[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH v2 14/15] vbox: Process empty removable disks in dumpxml




On 10/24/2017 03:35 PM, Dawid Zamirski wrote:
> 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

XML

> names to stay in 'sync' between domain and snapshot XML dumps.
> ---
>  src/vbox/vbox_common.c | 128 ++++++++++++++++++++++++++++---------------------
>  1 file changed, 74 insertions(+), 54 deletions(-)
> 
> diff --git a/src/vbox/vbox_common.c b/src/vbox/vbox_common.c
> index ee6421aae..d1d8804c7 100644
> --- a/src/vbox/vbox_common.c
> +++ b/src/vbox/vbox_common.c
> @@ -3213,7 +3213,6 @@ vboxDumpDisks(virDomainDefPtr def, vboxDriverPtr data, IMachine *machine)
>      virDomainDiskDefPtr disk = NULL;
>      char *mediumLocUtf8 = NULL;
>      size_t sdCount = 0, i;
> -    int diskCount = 0;
>      int ret = -1;
>  
>      def->ndisks = 0;
> @@ -3226,11 +3225,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 */
> @@ -3246,7 +3249,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;
> @@ -3258,7 +3261,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;
> @@ -3270,9 +3273,6 @@ vboxDumpDisks(virDomainDefPtr def, vboxDriverPtr data, IMachine *machine)
>              goto cleanup;
>          }
>  
> -        if (!medium)
> -            continue;
> -
>          rc = gVBoxAPI.UIMediumAttachment.GetController(mediumAttachment,
>                                                    &controllerName);
>          if (NS_FAILED(rc)) {
> @@ -3292,22 +3292,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,
> @@ -3333,11 +3317,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,
> @@ -3375,8 +3378,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);
> @@ -5814,6 +5815,14 @@ 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 ||
> +                storageBus == StorageBus_SAS)
> +                sdCount++;
> +
>              VBOX_RELEASE(storageController);
>              continue;
>          }
> @@ -5982,14 +5991,6 @@ vboxSnapshotGetReadOnlyDisks(virDomainSnapshotPtr snapshot,
>          IMediumAttachment *imediumattach = mediumAttachments.items[i];
>          if (!imediumattach)
>              continue;
> -        rc = gVBoxAPI.UIMediumAttachment.GetMedium(imediumattach, &disk);
> -        if (NS_FAILED(rc)) {
> -            virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> -                           _("Cannot get medium"));
> -            goto cleanup;
> -        }
> -        if (!disk)
> -            continue;

Can the changes that are moving the medium code to be all together go
into a patch just before this one?  Then the hunk here just becomes
adding the sdCount++ adjustment when !disk - just like all that changed
for vboxSnapshotGetReadWriteDisks.

Tks -

John

>          rc = gVBoxAPI.UIMediumAttachment.GetController(imediumattach, &storageControllerName);
>          if (NS_FAILED(rc)) {
>              virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> @@ -6009,19 +6010,6 @@ vboxSnapshotGetReadOnlyDisks(virDomainSnapshotPtr snapshot,
>          }
>          if (!storageController)
>              continue;
> -        rc = gVBoxAPI.UIMedium.GetLocation(disk, &mediumLocUtf16);
> -        if (NS_FAILED(rc)) {
> -            virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> -                           _("Cannot get disk location"));
> -            goto cleanup;
> -        }
> -        VBOX_UTF16_TO_UTF8(mediumLocUtf16, &mediumLocUtf8);
> -        VBOX_UTF16_FREE(mediumLocUtf16);
> -        if (VIR_STRDUP(def->dom->disks[diskCount]->src->path, mediumLocUtf8) < 0)
> -            goto cleanup;
> -
> -        VBOX_UTF8_FREE(mediumLocUtf8);
> -
>          rc = gVBoxAPI.UIStorageController.GetBus(storageController, &storageBus);
>          if (NS_FAILED(rc)) {
>              virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> @@ -6040,6 +6028,38 @@ vboxSnapshotGetReadOnlyDisks(virDomainSnapshotPtr snapshot,
>                             _("Cannot get device slot"));
>              goto cleanup;
>          }
> +
> +        rc = gVBoxAPI.UIMediumAttachment.GetMedium(imediumattach, &disk);
> +        if (NS_FAILED(rc)) {
> +            virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                           _("Cannot get medium"));
> +            goto cleanup;
> +        }
> +
> +        /* 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 ||
> +                storageBus == StorageBus_SAS)
> +                sdCount++;
> +            continue;
> +        }
> +
> +        rc = gVBoxAPI.UIMedium.GetLocation(disk, &mediumLocUtf16);
> +        if (NS_FAILED(rc)) {
> +            virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                           _("Cannot get disk location"));
> +            goto cleanup;
> +        }
> +        VBOX_UTF16_TO_UTF8(mediumLocUtf16, &mediumLocUtf8);
> +        VBOX_UTF16_FREE(mediumLocUtf16);
> +        if (VIR_STRDUP(def->dom->disks[diskCount]->src->path, mediumLocUtf8) < 0)
> +            goto cleanup;
> +
> +        VBOX_UTF8_FREE(mediumLocUtf8);
>          rc = gVBoxAPI.UIMedium.GetReadOnly(disk, &readOnly);
>          if (NS_FAILED(rc)) {
>              virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> 


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]