From 7c0a85e5be4990d2789ffe7d380fa41d97523beb Mon Sep 17 00:00:00 2001
From: Dawid Zamirski <dzamirski@datto.com>
Date: Tue, 7 Nov 2017 13:49:27 -0500
Subject: [PATCH] vbox: Process empty removable disks in dumpxml

Previously any removable storage device without media attached was
omitted from domain XML dump. They're still (rightfully) omitted in
snapshot XML dump but need to be accounted properly to for the device
names to stay in 'sync' between domain and snapshot XML dumps.
---
 src/vbox/vbox_common.c | 82 +++++++++++++++++++++++++-----------------
 1 file changed, 49 insertions(+), 33 deletions(-)

diff --git a/src/vbox/vbox_common.c b/src/vbox/vbox_common.c
index 248f315ebe..661e09a27a 100644
--- a/src/vbox/vbox_common.c
+++ b/src/vbox/vbox_common.c
@@ -3185,7 +3185,7 @@ static int
 vboxDumpDisks(virDomainDefPtr def, vboxDriverPtr data, IMachine *machine)
 {
     vboxArray mediumAttachments = VBOX_ARRAY_INITIALIZER;
-    int ret = -1, diskCount = 0;
+    int ret = -1;
     IMediumAttachment *mediumAttachment = NULL;
     IMedium *medium = NULL;
     IStorageController *controller = NULL;
@@ -3208,11 +3208,15 @@ vboxDumpDisks(virDomainDefPtr def, vboxDriverPtr data, IMachine *machine)
         if (!mediumAttachment)
             continue;
 
-        gVBoxAPI.UIMediumAttachment.GetMedium(mediumAttachment, &medium);
-        if (medium) {
-            def->ndisks++;
-            VBOX_RELEASE(medium);
+        rc = gVBoxAPI.UIMediumAttachment.GetMedium(mediumAttachment, &medium);
+        if (NS_FAILED(rc)) {
+            virReportError(VIR_ERR_INTERNAL_ERROR,
+                           _("Could not get IMedium, rc=%08x"), rc);
+            goto cleanup;
         }
+
+        def->ndisks++;
+        VBOX_RELEASE(medium);
     }
 
     /* Allocate mem, if fails return error */
@@ -3228,7 +3232,7 @@ vboxDumpDisks(virDomainDefPtr def, vboxDriverPtr data, IMachine *machine)
     }
 
     /* get the attachment details here */
-    for (i = 0; i < mediumAttachments.count && diskCount < def->ndisks; i++) {
+    for (i = 0; i < mediumAttachments.count; i++) {
         mediumAttachment = mediumAttachments.items[i];
         controller = NULL;
         controllerName = NULL;
@@ -3240,7 +3244,7 @@ vboxDumpDisks(virDomainDefPtr def, vboxDriverPtr data, IMachine *machine)
         mediumLocUtf8 = NULL;
         devicePort = 0;
         deviceSlot = 0;
-        disk = def->disks[diskCount];
+        disk = def->disks[i];
 
         if (!mediumAttachment)
             continue;
@@ -3252,9 +3256,6 @@ vboxDumpDisks(virDomainDefPtr def, vboxDriverPtr data, IMachine *machine)
             goto cleanup;
         }
 
-        if (!medium)
-            continue;
-
         rc = gVBoxAPI.UIMediumAttachment.GetController(mediumAttachment,
                                                        &controllerName);
         if (NS_FAILED(rc)) {
@@ -3274,22 +3275,6 @@ vboxDumpDisks(virDomainDefPtr def, vboxDriverPtr data, IMachine *machine)
             goto cleanup;
         }
 
-        rc = gVBoxAPI.UIMedium.GetLocation(medium, &mediumLocUtf16);
-        if (NS_FAILED(rc)) {
-            virReportError(VIR_ERR_INTERNAL_ERROR,
-                           _("Could not get medium storage location, rc=%08x"),
-                           rc);
-            goto cleanup;
-        }
-
-        VBOX_UTF16_TO_UTF8(mediumLocUtf16, &mediumLocUtf8);
-
-        if (virDomainDiskSetSource(disk, mediumLocUtf8) < 0) {
-            virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
-                           _("Could not set disk source"));
-            goto cleanup;
-        }
-
         rc = gVBoxAPI.UIMediumAttachment.GetType(mediumAttachment, &deviceType);
         if (NS_FAILED(rc)) {
             virReportError(VIR_ERR_INTERNAL_ERROR,
@@ -3315,11 +3300,30 @@ vboxDumpDisks(virDomainDefPtr def, vboxDriverPtr data, IMachine *machine)
                            rc);
             goto cleanup;
         }
-        rc = gVBoxAPI.UIMedium.GetReadOnly(medium, &readOnly);
-        if (NS_FAILED(rc)) {
-            virReportError(VIR_ERR_INTERNAL_ERROR,
-                           _("Could not get read only state, rc=%08x"), rc);
-            goto cleanup;
+
+        if (medium) {
+            rc = gVBoxAPI.UIMedium.GetLocation(medium, &mediumLocUtf16);
+            if (NS_FAILED(rc)) {
+                virReportError(VIR_ERR_INTERNAL_ERROR,
+                               _("Could not get medium storage location, rc=%08x"),
+                               rc);
+                goto cleanup;
+            }
+
+            VBOX_UTF16_TO_UTF8(mediumLocUtf16, &mediumLocUtf8);
+
+            if (virDomainDiskSetSource(disk, mediumLocUtf8) < 0) {
+                virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+                               _("Could not set disk source"));
+                goto cleanup;
+            }
+
+            rc = gVBoxAPI.UIMedium.GetReadOnly(medium, &readOnly);
+            if (NS_FAILED(rc)) {
+                virReportError(VIR_ERR_INTERNAL_ERROR,
+                               _("Could not get read only state, rc=%08x"), rc);
+                goto cleanup;
+            }
         }
 
         disk->dst = vboxGenerateMediumName(storageBus, devicePort, deviceSlot,
@@ -3356,8 +3360,6 @@ vboxDumpDisks(virDomainDefPtr def, vboxDriverPtr data, IMachine *machine)
 
         virDomainDiskSetType(disk, VIR_STORAGE_TYPE_FILE);
 
-        diskCount++;
-
         VBOX_UTF16_FREE(controllerName);
         VBOX_UTF8_FREE(mediumLocUtf8);
         VBOX_UTF16_FREE(mediumLocUtf16);
@@ -5788,6 +5790,13 @@ vboxSnapshotGetReadWriteDisks(virDomainSnapshotDefPtr def,
 
         /* skip empty removable disk */
         if (!disk) {
+            /* removable disks with empty (ejected) media won't be displayed
+             * in XML, but we need to update "sdCount" so that device names match
+             * in domain dumpxml and snapshot dumpxml
+             */
+            if (storageBus == StorageBus_SATA || storageBus == StorageBus_SCSI)
+                sdCount++;
+
             VBOX_RELEASE(storageController);
             continue;
         }
@@ -5996,6 +6005,13 @@ vboxSnapshotGetReadOnlyDisks(virDomainSnapshotDefPtr def,
 
         /* skip empty removable disk */
         if (!disk) {
+            /* removable disks with empty (ejected) media won't be displayed
+             * in XML, but we need to update "sdCount" so that device names match
+             * in domain dumpxml and snapshot dumpxml
+             */
+            if (storageBus == StorageBus_SATA || storageBus == StorageBus_SCSI)
+                sdCount++;
+
             VBOX_RELEASE(storageController);
             continue;
         }