Re: [libvirt] [PATCH v2 07/15] vbox: Support empty removable drives.

[John Ferlan]

On 10/24/2017 03:35 PM, Dawid Zamirski wrote:
> Original code was checking for non empty disk source before proceeding
> to actually attach disk device to VM. This prevented from creating
> empty removable devices like DVD or floppy. Therefore, this patch
> re-organizes the loop work-flow to allow such configurations as well as
> makes the code follow better libvirt practices. Additionally, adjusted
> debug logs to be more helpful - removed old ones and added new which
> give more valuable info for troubleshooting.
> ---
>  src/vbox/vbox_common.c | 206 
> +++--
>  1 file changed, 130 insertions(+), 76 deletions(-)
> 

There's a lot going on here, but I'm not sure how much is separable
other than perhaps the debug stuff, so even though there's a lot going
on ...

Reviewed-by: John Ferlan 

John

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

[libvirt] [PATCH v2 07/15] vbox: Support empty removable drives.

[Dawid Zamirski]

Original code was checking for non empty disk source before proceeding
to actually attach disk device to VM. This prevented from creating
empty removable devices like DVD or floppy. Therefore, this patch
re-organizes the loop work-flow to allow such configurations as well as
makes the code follow better libvirt practices. Additionally, adjusted
debug logs to be more helpful - removed old ones and added new which
give more valuable info for troubleshooting.
---
 src/vbox/vbox_common.c | 206 +++--
 1 file changed, 130 insertions(+), 76 deletions(-)

diff --git a/src/vbox/vbox_common.c b/src/vbox/vbox_common.c
index 9f4bf18a3..2bd891efb 100644
--- a/src/vbox/vbox_common.c
+++ b/src/vbox/vbox_common.c
@@ -959,11 +959,12 @@ static int
 vboxAttachDrives(virDomainDefPtr def, vboxDriverPtr data, IMachine *machine)
 {
 size_t i;
-int type, format, ret = 0;
+int type, ret = 0;
 const char *src = NULL;
 nsresult rc = 0;
 virDomainDiskDefPtr disk = NULL;
 PRUnichar *storageCtlName = NULL;
+char *controllerName = NULL;
 IMedium *medium = NULL;
 PRUnichar *mediumFileUtf16 = NULL;
 PRUint32 devicePort, deviceSlot, deviceType, accessMode;
@@ -1015,47 +1016,104 @@ vboxAttachDrives(virDomainDefPtr def, vboxDriverPtr 
data, IMachine *machine)
 disk = def->disks[i];
 src = virDomainDiskGetSource(disk);
 type = virDomainDiskGetType(disk);
-format = virDomainDiskGetFormat(disk);
 deviceType = DeviceType_Null;
 accessMode = AccessMode_ReadOnly;
 devicePort = disk->info.addr.drive.unit;
 deviceSlot = disk->info.addr.drive.bus;
 
-VIR_DEBUG("disk(%zu) type:   %d", i, type);
-VIR_DEBUG("disk(%zu) device: %d", i, disk->device);
-VIR_DEBUG("disk(%zu) bus:%d", i, disk->bus);
-VIR_DEBUG("disk(%zu) src:%s", i, src);
-VIR_DEBUG("disk(%zu) dst:%s", i, disk->dst);
-VIR_DEBUG("disk(%zu) driverName: %s", i,
-  virDomainDiskGetDriver(disk));
-VIR_DEBUG("disk(%zu) driverType: %s", i,
-  virStorageFileFormatTypeToString(format));
-VIR_DEBUG("disk(%zu) cachemode:  %d", i, disk->cachemode);
-VIR_DEBUG("disk(%zu) readonly:   %s", i, (disk->src->readonly
- ? "True" : "False"));
-VIR_DEBUG("disk(%zu) shared: %s", i, (disk->src->shared
- ? "True" : "False"));
-
-if (type == VIR_STORAGE_TYPE_FILE && src) {
-VBOX_UTF8_TO_UTF16(src, );
+if (type != VIR_STORAGE_TYPE_FILE) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+   _("Unsupported storage type %s, the only supported "
+ "type is %s"),
+   virStorageTypeToString(type),
+   virStorageTypeToString(VIR_STORAGE_TYPE_FILE));
+ret = -1;
+goto cleanup;
+}
 
-if (disk->device == VIR_DOMAIN_DISK_DEVICE_DISK) {
-deviceType = DeviceType_HardDisk;
-accessMode = AccessMode_ReadWrite;
-} else if (disk->device == VIR_DOMAIN_DISK_DEVICE_CDROM) {
-deviceType = DeviceType_DVD;
-accessMode = AccessMode_ReadOnly;
-} else if (disk->device == VIR_DOMAIN_DISK_DEVICE_FLOPPY) {
-deviceType = DeviceType_Floppy;
-accessMode = AccessMode_ReadWrite;
-} else {
+switch ((virDomainDiskDevice) disk->device) {
+case VIR_DOMAIN_DISK_DEVICE_DISK:
+if (!src) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+   _("Missing disk source file path"));
 ret = -1;
 goto cleanup;
 }
 
-gVBoxAPI.UIVirtualBox.FindHardDisk(data->vboxObj, mediumFileUtf16,
-   deviceType, accessMode, 
);
+deviceType = DeviceType_HardDisk;
+accessMode = AccessMode_ReadWrite;
+
+break;
+
+case VIR_DOMAIN_DISK_DEVICE_CDROM:
+deviceType = DeviceType_DVD;
+accessMode = AccessMode_ReadOnly;
+
+break;
+case VIR_DOMAIN_DISK_DEVICE_FLOPPY:
+deviceType = DeviceType_Floppy;
+accessMode = AccessMode_ReadWrite;
+
+break;
+case VIR_DOMAIN_DISK_DEVICE_LUN:
+case VIR_DOMAIN_DISK_DEVICE_LAST:
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+   _("The vbox driver does not support %s disk 
device"),
+   virDomainDiskDeviceTypeToString(disk->device));
+ret = -1;
+goto cleanup;
+}
+
+switch ((virDomainDiskBus) disk->bus) {
+case VIR_DOMAIN_DISK_BUS_IDE:
+