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",
>
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
