The function at first validates the top image of the chain, then
traverses the chain as declared in the XML (if any) and then procedes to
detect the rest of the chain from images. All of the steps have their
own temporary iterator.

Clarify the use scope of the steps by introducing a new temp variable
holding the top level source and adding comments.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
---
 src/qemu/qemu_domain.c | 40 +++++++++++++++++++++++-----------------
 1 file changed, 23 insertions(+), 17 deletions(-)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 32a43f2064..8e3d0dd374 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -8945,43 +8945,49 @@ qemuDomainDetermineDiskChain(virQEMUDriverPtr driver,
                              bool report_broken)
 {
     virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
-    virStorageSourcePtr src = disk->src;
-    virStorageSourcePtr n;
+    virStorageSourcePtr disksrc = NULL; /* disk source */
+    virStorageSourcePtr src; /* iterator for the backing chain declared in XML */
+    virStorageSourcePtr n; /* iterator for the backing chain detected from disk */
     qemuDomainObjPrivatePtr priv = vm->privateData;
     int ret = -1;
     uid_t uid;
     gid_t gid;

-    if (virStorageSourceIsEmpty(src)) {
+    if (!disksrc)
+        disksrc = disk->src;
+
+    src = disksrc;
+
+    if (virStorageSourceIsEmpty(disksrc)) {
         ret = 0;
         goto cleanup;
     }

     /* 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 (virStorageSourceIsLocalStorage(disksrc) &&
+        disksrc->format > VIR_STORAGE_FILE_NONE &&
+        disksrc->format < VIR_STORAGE_FILE_BACKING) {

-        if (!virFileExists(src->path)) {
+        if (!virFileExists(disksrc->path)) {
             if (report_broken)
-                virStorageFileReportBrokenChain(errno, src, disk->src);
+                virStorageFileReportBrokenChain(errno, disksrc, disksrc);

             goto cleanup;
         }

         /* terminate the chain for such images as the code below would do */
-        if (!src->backingStore &&
-            VIR_ALLOC(src->backingStore) < 0)
+        if (!disksrc->backingStore &&
+            VIR_ALLOC(disksrc->backingStore) < 0)
             goto cleanup;

         /* host cdrom requires special treatment in qemu, so we need to check
          * whether a block device is a cdrom */
         if (disk->device == VIR_DOMAIN_DISK_DEVICE_CDROM &&
-            src->format == VIR_STORAGE_FILE_RAW &&
-            virStorageSourceIsBlockLocal(src) &&
-            virFileIsCDROM(src->path) == 1)
-            src->hostcdrom = true;
+            disksrc->format == VIR_STORAGE_FILE_RAW &&
+            virStorageSourceIsBlockLocal(disksrc) &&
+            virFileIsCDROM(disksrc->path) == 1)
+            disksrc->hostcdrom = true;

         ret = 0;
         goto cleanup;
@@ -8996,11 +9002,11 @@ qemuDomainDetermineDiskChain(virQEMUDriverPtr driver,
                 goto cleanup;

             if (rv > 0) {
-                if (qemuDomainStorageFileInit(driver, vm, src, disk->src) < 0)
+                if (qemuDomainStorageFileInit(driver, vm, src, disksrc) < 0)
                     goto cleanup;

                 if (virStorageFileAccess(src, F_OK) < 0) {
-                    virStorageFileReportBrokenChain(errno, src, disk->src);
+                    virStorageFileReportBrokenChain(errno, src, disksrc);
                     virStorageFileDeinit(src);
                     goto cleanup;
                 }
@@ -9018,7 +9024,7 @@ qemuDomainDetermineDiskChain(virQEMUDriverPtr driver,
         goto cleanup;
     }

-    qemuDomainGetImageIds(cfg, vm, src, disk->src, &uid, &gid);
+    qemuDomainGetImageIds(cfg, vm, src, disksrc, &uid, &gid);

     if (virStorageFileGetMetadata(src, uid, gid, report_broken) < 0)
         goto cleanup;
-- 
2.20.1

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

On 1/23/19 11:10 AM, Peter Krempa wrote:
> The function at first validates the top image of the chain, then
> traverses the chain as declared in the XML (if any) and then procedes to
> detect the rest of the chain from images. All of the steps have their
> own temporary iterator.
> 
> Clarify the use scope of the steps by introducing a new temp variable
> holding the top level source and adding comments.
> 
> Signed-off-by: Peter Krempa <pkrempa@redhat.com>
> ---
>  src/qemu/qemu_domain.c | 40 +++++++++++++++++++++++-----------------
>  1 file changed, 23 insertions(+), 17 deletions(-)
> 
> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> index 32a43f2064..8e3d0dd374 100644
> --- a/src/qemu/qemu_domain.c
> +++ b/src/qemu/qemu_domain.c
> @@ -8945,43 +8945,49 @@ qemuDomainDetermineDiskChain(virQEMUDriverPtr driver,
>                               bool report_broken)
>  {
>      virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
> -    virStorageSourcePtr src = disk->src;
> -    virStorageSourcePtr n;
> +    virStorageSourcePtr disksrc = NULL; /* disk source */

NIT: Chould be disksrc = disk->src here and then in the next patch it's
removed and [1] becomes part of the next patch.

> +    virStorageSourcePtr src; /* iterator for the backing chain declared in XML */
> +    virStorageSourcePtr n; /* iterator for the backing chain detected from disk */
>      qemuDomainObjPrivatePtr priv = vm->privateData;
>      int ret = -1;
>      uid_t uid;
>      gid_t gid;
> 
> -    if (virStorageSourceIsEmpty(src)) {
> +    if (!disksrc)
> +        disksrc = disk->src;

[1] always going to happen with this patch...

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

John

[...]

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