Until now we would skip loading of the backing chain for files which
don't support backing chains only when starting up the VM. Move the
check from qemuProcessPrepareHostStorage with some adaptations so that's
always applied.
---
 src/qemu/qemu_domain.c  | 17 +++++++++++++++++
 src/qemu/qemu_process.c | 10 ----------
 2 files changed, 17 insertions(+), 10 deletions(-)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 0e6ebdc0a8..0cdcb11c37 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -6154,6 +6154,23 @@ qemuDomainDetermineDiskChain(virQEMUDriverPtr driver,
     if (force_probe)
         virStorageSourceBackingStoreClear(src);

+    /* There is no need to check the backing chain for disks without backing
+     * support */
+    if (virStorageSourceIsLocalStorage(src) &&
+        src->format > VIR_STORAGE_FILE_NONE &&
+        src->format < VIR_STORAGE_FILE_BACKING) {
+
+        if (!virFileExists(src->path)) {
+            if (report_broken)
+                virStorageFileReportBrokenChain(errno, src, disk->src);
+
+            goto cleanup;
+        }
+
+        ret = 0;
+        goto cleanup;
+    }
+
     /* skip to the end of the chain if there is any */
     while (virStorageSourceHasBacking(src)) {
         if (report_broken &&
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 8574f2b413..ea70885dd9 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -5555,20 +5555,10 @@ qemuProcessPrepareHostStorage(virQEMUDriverPtr driver,
     for (i = vm->def->ndisks; i > 0; i--) {
         size_t idx = i - 1;
         virDomainDiskDefPtr disk = vm->def->disks[idx];
-        virStorageFileFormat format = virDomainDiskGetFormat(disk);

         if (virStorageSourceIsEmpty(disk->src))
             continue;

-        /* There is no need to check the backing chain for disks
-         * without backing support, the fact that the file exists is
-         * more than enough */
-        if (virStorageSourceIsLocalStorage(disk->src) &&
-            format > VIR_STORAGE_FILE_NONE &&
-            format < VIR_STORAGE_FILE_BACKING &&
-            virFileExists(virDomainDiskGetSource(disk)))
-            continue;
-
         if (qemuDomainDetermineDiskChain(driver, vm, disk, true, true) >= 0)
             continue;

-- 
2.14.3

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

On 11/24/2017 07:21 AM, Peter Krempa wrote:
> Until now we would skip loading of the backing chain for files which
> don't support backing chains only when starting up the VM. Move the
> check from qemuProcessPrepareHostStorage with some adaptations so that's
> always applied.
> ---
>  src/qemu/qemu_domain.c  | 17 +++++++++++++++++
>  src/qemu/qemu_process.c | 10 ----------
>  2 files changed, 17 insertions(+), 10 deletions(-)
> 

Does it matter or should the check go before the
virStorageSourceBackingStoreClear... Up through this point we did it
before anyway.

Reviewed-by: John Ferlan <jferlan@redhat.com>

(for the code regardless of your decision on placement)

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

On Wed, Nov 29, 2017 at 21:25:45 -0500, John Ferlan wrote:
> 
> 
> On 11/24/2017 07:21 AM, Peter Krempa wrote:
> > Until now we would skip loading of the backing chain for files which
> > don't support backing chains only when starting up the VM. Move the
> > check from qemuProcessPrepareHostStorage with some adaptations so that's
> > always applied.
> > ---
> >  src/qemu/qemu_domain.c  | 17 +++++++++++++++++
> >  src/qemu/qemu_process.c | 10 ----------
> >  2 files changed, 17 insertions(+), 10 deletions(-)
> > 
> 
> Does it matter or should the check go before the
> virStorageSourceBackingStoreClear... Up through this point we did it
> before anyway.

Well. It does not matter at this precise point. If the file does not
exist (since it's the top level image), we would fail and clearing or
not clearing of the terminator does not matter.

It simplifies the code for the next patch though. If we are nuking the
rest of the backing chain (as we do in all cases today), we want to nuke
the rest of the chain for the raw disk (since it would be invalid).

After that we will always need to terminate the chain.

So yes, change in the ordering is desired.

> 
> Reviewed-by: John Ferlan <jferlan@redhat.com>
> 
> (for the code regardless of your decision on placement)
> 
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list