[libvirt] [PATCH v2 13/15] vbox: Cleanup vboxDumpDisks implementation
Dawid Zamirski
dzamirski at datto.com
Tue Oct 24 19:35:36 UTC 2017
Primer the code for further changes:
* move variable declarations to the top of the function
* group together free/release statements
* error check and report VBOX API calls used
---
src/vbox/vbox_common.c | 188 +++++++++++++++++++++++++++++++------------------
1 file changed, 120 insertions(+), 68 deletions(-)
diff --git a/src/vbox/vbox_common.c b/src/vbox/vbox_common.c
index 9dc36a1b2..ee6421aae 100644
--- a/src/vbox/vbox_common.c
+++ b/src/vbox/vbox_common.c
@@ -3202,6 +3202,16 @@ static int
vboxDumpDisks(virDomainDefPtr def, vboxDriverPtr data, IMachine *machine)
{
vboxArray mediumAttachments = VBOX_ARRAY_INITIALIZER;
+ IMediumAttachment *mediumAttachment = NULL;
+ IMedium *medium = NULL;
+ IStorageController *controller = NULL;
+ PRUnichar *controllerName = NULL, *mediumLocUtf16 = NULL;
+ PRUint32 deviceType, storageBus;
+ PRInt32 devicePort, deviceSlot;
+ PRBool readOnly;
+ nsresult rc;
+ virDomainDiskDefPtr disk = NULL;
+ char *mediumLocUtf8 = NULL;
size_t sdCount = 0, i;
int diskCount = 0;
int ret = -1;
@@ -3212,15 +3222,14 @@ vboxDumpDisks(virDomainDefPtr def, vboxDriverPtr data, IMachine *machine)
/* get the number of attachments */
for (i = 0; i < mediumAttachments.count; i++) {
- IMediumAttachment *imediumattach = mediumAttachments.items[i];
- if (imediumattach) {
- IMedium *medium = NULL;
+ mediumAttachment = mediumAttachments.items[i];
+ if (!mediumAttachment)
+ continue;
- gVBoxAPI.UIMediumAttachment.GetMedium(imediumattach, &medium);
- if (medium) {
- def->ndisks++;
- VBOX_RELEASE(medium);
- }
+ gVBoxAPI.UIMediumAttachment.GetMedium(mediumAttachment, &medium);
+ if (medium) {
+ def->ndisks++;
+ VBOX_RELEASE(medium);
}
}
@@ -3229,7 +3238,7 @@ vboxDumpDisks(virDomainDefPtr def, vboxDriverPtr data, IMachine *machine)
goto cleanup;
for (i = 0; i < def->ndisks; i++) {
- virDomainDiskDefPtr disk = virDomainDiskDefNew(NULL);
+ disk = virDomainDiskDefNew(NULL);
if (!disk)
goto cleanup;
@@ -3238,104 +3247,141 @@ vboxDumpDisks(virDomainDefPtr def, vboxDriverPtr data, IMachine *machine)
/* get the attachment details here */
for (i = 0; i < mediumAttachments.count && diskCount < def->ndisks; i++) {
- IMediumAttachment *imediumattach = mediumAttachments.items[i];
- IStorageController *storageController = NULL;
- PRUnichar *storageControllerName = NULL;
- PRUint32 deviceType = DeviceType_Null;
- PRUint32 storageBus = StorageBus_Null;
- PRBool readOnly = PR_FALSE;
- IMedium *medium = NULL;
- PRUnichar *mediumLocUtf16 = NULL;
- char *mediumLocUtf8 = NULL;
- PRInt32 devicePort = 0;
- PRInt32 deviceSlot = 0;
-
- if (!imediumattach)
+ mediumAttachment = mediumAttachments.items[i];
+ controller = NULL;
+ controllerName = NULL;
+ deviceType = DeviceType_Null;
+ storageBus = StorageBus_Null;
+ readOnly = PR_FALSE;
+ medium = NULL;
+ mediumLocUtf16 = NULL;
+ mediumLocUtf8 = NULL;
+ devicePort = 0;
+ deviceSlot = 0;
+ disk = def->disks[diskCount];
+
+ if (!mediumAttachment)
continue;
- gVBoxAPI.UIMediumAttachment.GetMedium(imediumattach, &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;
+ }
+
if (!medium)
continue;
- gVBoxAPI.UIMediumAttachment.GetController(imediumattach, &storageControllerName);
- if (!storageControllerName) {
- VBOX_RELEASE(medium);
- continue;
+ rc = gVBoxAPI.UIMediumAttachment.GetController(mediumAttachment,
+ &controllerName);
+ if (NS_FAILED(rc)) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Failed to get storage controller name, rc=%08x"),
+ rc);
+ goto cleanup;
}
- gVBoxAPI.UIMachine.GetStorageControllerByName(machine,
- storageControllerName,
- &storageController);
- VBOX_UTF16_FREE(storageControllerName);
- if (!storageController) {
- VBOX_RELEASE(medium);
- continue;
+ rc = gVBoxAPI.UIMachine.GetStorageControllerByName(machine,
+ controllerName,
+ &controller);
+ if (NS_FAILED(rc)) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Could not get storage controller by name, rc=%08x"),
+ rc);
+ 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;
}
- gVBoxAPI.UIMedium.GetLocation(medium, &mediumLocUtf16);
VBOX_UTF16_TO_UTF8(mediumLocUtf16, &mediumLocUtf8);
- VBOX_UTF16_FREE(mediumLocUtf16);
- ignore_value(virDomainDiskSetSource(def->disks[diskCount],
- mediumLocUtf8));
- VBOX_UTF8_FREE(mediumLocUtf8);
- if (!virDomainDiskGetSource(def->disks[diskCount])) {
- VBOX_RELEASE(medium);
- VBOX_RELEASE(storageController);
+ 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,
+ _("Could not get device type, rc=%08x"), rc);
+ goto cleanup;
+ }
+ rc = gVBoxAPI.UIMediumAttachment.GetPort(mediumAttachment, &devicePort);
+ if (NS_FAILED(rc)) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Could not get device port, rc=%08x"), rc);
+ goto cleanup;
+ }
+ rc = gVBoxAPI.UIMediumAttachment.GetDevice(mediumAttachment, &deviceSlot);
+ if (NS_FAILED(rc)) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Could not get device slot, rc=%08x"), rc);
+ goto cleanup;
+ }
+ rc = gVBoxAPI.UIStorageController.GetBus(controller, &storageBus);
+ if (NS_FAILED(rc)) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Could not get storage controller bus, rc=%08x"),
+ 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;
}
- gVBoxAPI.UIStorageController.GetBus(storageController, &storageBus);
- gVBoxAPI.UIMediumAttachment.GetPort(imediumattach, &devicePort);
- gVBoxAPI.UIMediumAttachment.GetDevice(imediumattach, &deviceSlot);
+ disk->dst = vboxGenerateMediumName(storageBus, devicePort, deviceSlot,
+ sdCount);
- def->disks[diskCount]->dst = vboxGenerateMediumName(storageBus,
- devicePort,
- deviceSlot,
- sdCount);
- if (!def->disks[diskCount]->dst) {
+ if (!disk->dst) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("Could not generate medium name for the disk "
"at: port:%d, slot:%d"), devicePort, deviceSlot);
- VBOX_RELEASE(medium);
- VBOX_RELEASE(storageController);
-
goto cleanup;
}
if (storageBus == StorageBus_IDE) {
- def->disks[diskCount]->bus = VIR_DOMAIN_DISK_BUS_IDE;
+ disk->bus = VIR_DOMAIN_DISK_BUS_IDE;
} else if (storageBus == StorageBus_SATA) {
sdCount++;
- def->disks[diskCount]->bus = VIR_DOMAIN_DISK_BUS_SATA;
+ disk->bus = VIR_DOMAIN_DISK_BUS_SATA;
} else if (storageBus == StorageBus_SCSI ||
storageBus == StorageBus_SAS) {
sdCount++;
- def->disks[diskCount]->bus = VIR_DOMAIN_DISK_BUS_SCSI;
+ disk->bus = VIR_DOMAIN_DISK_BUS_SCSI;
} else if (storageBus == StorageBus_Floppy) {
- def->disks[diskCount]->bus = VIR_DOMAIN_DISK_BUS_FDC;
+ disk->bus = VIR_DOMAIN_DISK_BUS_FDC;
}
- gVBoxAPI.UIMediumAttachment.GetType(imediumattach, &deviceType);
if (deviceType == DeviceType_HardDisk)
- def->disks[diskCount]->device = VIR_DOMAIN_DISK_DEVICE_DISK;
+ disk->device = VIR_DOMAIN_DISK_DEVICE_DISK;
else if (deviceType == DeviceType_Floppy)
- def->disks[diskCount]->device = VIR_DOMAIN_DISK_DEVICE_FLOPPY;
+ disk->device = VIR_DOMAIN_DISK_DEVICE_FLOPPY;
else if (deviceType == DeviceType_DVD)
- def->disks[diskCount]->device = VIR_DOMAIN_DISK_DEVICE_CDROM;
+ disk->device = VIR_DOMAIN_DISK_DEVICE_CDROM;
-
- gVBoxAPI.UIMedium.GetReadOnly(medium, &readOnly);
if (readOnly == PR_TRUE)
- def->disks[diskCount]->src->readonly = true;
+ disk->src->readonly = true;
- virDomainDiskSetType(def->disks[diskCount],
- VIR_STORAGE_TYPE_FILE);
+ virDomainDiskSetType(disk, VIR_STORAGE_TYPE_FILE);
- VBOX_RELEASE(medium);
- VBOX_RELEASE(storageController);
diskCount++;
+
+ VBOX_UTF16_FREE(controllerName);
+ VBOX_UTF8_FREE(mediumLocUtf8);
+ VBOX_UTF16_FREE(mediumLocUtf16);
+ VBOX_RELEASE(medium);
+ VBOX_RELEASE(controller);
}
ret = 0;
@@ -3345,6 +3391,12 @@ vboxDumpDisks(virDomainDefPtr def, vboxDriverPtr data, IMachine *machine)
/* cleanup on error */
if (ret < 0) {
+ VBOX_UTF16_FREE(controllerName);
+ VBOX_UTF8_FREE(mediumLocUtf8);
+ VBOX_UTF16_FREE(mediumLocUtf16);
+ VBOX_RELEASE(medium);
+ VBOX_RELEASE(controller);
+
for (i = 0; i < def->ndisks; i++)
VIR_FREE(def->disks[i]);
VIR_FREE(def->disks);
--
2.14.2
More information about the libvir-list
mailing list