Preparing the storage source prior to assigning the alias will not work as the names of the certain objects depend on the alias for the legacy hotplug case as we generate the object names for the secrets based on the alias. This reverts commit 192fdaa614e3800255048a8a70c1292ccf18397a. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/qemu/qemu_hotplug.c | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 4558a3c02d..ccf3336633 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -781,6 +781,7 @@ qemuDomainAttachDiskGeneric(virQEMUDriverPtr driver, qemuDomainObjPrivatePtr priv = vm->privateData; qemuHotplugDiskSourceDataPtr diskdata = NULL; char *devstr = NULL; + virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); if (qemuHotplugPrepareDiskAccess(driver, vm, disk, NULL, false) < 0) goto cleanup; @@ -788,6 +789,9 @@ qemuDomainAttachDiskGeneric(virQEMUDriverPtr driver, if (qemuAssignDeviceDiskAlias(vm->def, disk, priv->qemuCaps) < 0) goto error; + if (qemuDomainPrepareDiskSource(disk, priv, cfg) < 0) + goto error; + if (!(diskdata = qemuHotplugDiskSourceAttachPrepare(disk, priv->qemuCaps))) goto error; @@ -822,6 +826,7 @@ qemuDomainAttachDiskGeneric(virQEMUDriverPtr driver, qemuHotplugDiskSourceDataFree(diskdata); qemuDomainSecretDiskDestroy(disk); VIR_FREE(devstr); + virObjectUnref(cfg); return ret; exit_monitor: @@ -1062,8 +1067,6 @@ qemuDomainAttachDeviceDiskLive(virQEMUDriverPtr driver, bool forceMediaChange) { size_t i; - virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); - qemuDomainObjPrivatePtr priv = vm->privateData; virDomainDiskDefPtr disk = dev->data.disk; virDomainDiskDefPtr orig_disk = NULL; int ret = -1; @@ -1080,9 +1083,6 @@ qemuDomainAttachDeviceDiskLive(virQEMUDriverPtr driver, if (qemuDomainDetermineDiskChain(driver, vm, disk, true) < 0) goto cleanup; - if (qemuDomainPrepareDiskSource(disk, priv, cfg) < 0) - goto cleanup; - switch ((virDomainDiskDevice) disk->device) { case VIR_DOMAIN_DISK_DEVICE_CDROM: case VIR_DOMAIN_DISK_DEVICE_FLOPPY: @@ -1153,7 +1153,6 @@ qemuDomainAttachDeviceDiskLive(virQEMUDriverPtr driver, cleanup: if (ret != 0) ignore_value(qemuRemoveSharedDevice(driver, dev, vm->def->name)); - virObjectUnref(cfg); return ret; } -- 2.17.1

While the idea was good the implementation not so much as we need to take into account the old disk data and the new source. The code will be consolidated later in a different way. This reverts commit 663b1d55de652201b19d875f0eff730dc28e689e. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/qemu/qemu_driver.c | 20 ++++++++++++++++++-- src/qemu/qemu_hotplug.c | 18 +++--------------- src/qemu/qemu_hotplug.h | 9 +++++++-- tests/qemuhotplugtest.c | 2 +- 4 files changed, 29 insertions(+), 20 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index b238309852..64f0ad33e8 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -7600,7 +7600,7 @@ qemuDomainAttachDeviceLive(virDomainObjPtr vm, switch ((virDomainDeviceType)dev->type) { case VIR_DOMAIN_DEVICE_DISK: qemuDomainObjCheckDiskTaint(driver, vm, dev->data.disk, NULL); - ret = qemuDomainAttachDeviceDiskLive(driver, vm, dev, false); + ret = qemuDomainAttachDeviceDiskLive(driver, vm, dev); if (!ret) { alias = dev->data.disk->info.alias; dev->data.disk = NULL; @@ -7851,6 +7851,12 @@ qemuDomainChangeDiskLive(virDomainObjPtr vm, virDomainDeviceDef oldDev = { .type = dev->type }; int ret = -1; + if (virDomainDiskTranslateSourcePool(disk) < 0) + goto cleanup; + + if (qemuDomainDetermineDiskChain(driver, vm, disk, true) < 0) + goto cleanup; + if (!(orig_disk = virDomainDiskFindByBusAndDst(vm->def, disk->bus, disk->dst))) { virReportError(VIR_ERR_INTERNAL_ERROR, @@ -7879,8 +7885,18 @@ qemuDomainChangeDiskLive(virDomainObjPtr vm, goto cleanup; } - if (qemuDomainAttachDeviceDiskLive(driver, vm, dev, force) < 0) + /* Add the new disk src into shared disk hash table */ + if (qemuAddSharedDevice(driver, dev, vm->def->name) < 0) goto cleanup; + + if (qemuDomainChangeEjectableMedia(driver, vm, orig_disk, + dev->data.disk->src, force) < 0) { + ignore_value(qemuRemoveSharedDisk(driver, dev->data.disk, + vm->def->name)); + goto cleanup; + } + + dev->data.disk->src = NULL; } orig_disk->startupPolicy = dev->data.disk->startupPolicy; diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index ccf3336633..86afda636e 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -722,7 +722,7 @@ qemuDomainChangeMediaBlockdev(virQEMUDriverPtr driver, * * Returns 0 on success, -1 on error and reports libvirt error */ -static int +int qemuDomainChangeEjectableMedia(virQEMUDriverPtr driver, virDomainObjPtr vm, virDomainDiskDefPtr disk, @@ -1049,22 +1049,10 @@ qemuDomainAttachUSBMassStorageDevice(virQEMUDriverPtr driver, } -/** - * qemuDomainAttachDeviceDiskLive: - * @driver: qemu driver struct - * @vm: domain object - * @dev: device to attach (expected type is DISK) - * @forceMediaChange: Forcibly open the drive if changing media - * - * Attach a new disk or in case of cdroms/floppies change the media in the drive. - * This function handles all the necessary steps to attach a new storage source - * to the VM. If @forceMediaChange is true the drive is opened forcibly. - */ int qemuDomainAttachDeviceDiskLive(virQEMUDriverPtr driver, virDomainObjPtr vm, - virDomainDeviceDefPtr dev, - bool forceMediaChange) + virDomainDeviceDefPtr dev) { size_t i; virDomainDiskDefPtr disk = dev->data.disk; @@ -1098,7 +1086,7 @@ qemuDomainAttachDeviceDiskLive(virQEMUDriverPtr driver, } if (qemuDomainChangeEjectableMedia(driver, vm, orig_disk, - disk->src, forceMediaChange) < 0) + disk->src, false) < 0) goto cleanup; disk->src = NULL; diff --git a/src/qemu/qemu_hotplug.h b/src/qemu/qemu_hotplug.h index c085c45082..0297e42a98 100644 --- a/src/qemu/qemu_hotplug.h +++ b/src/qemu/qemu_hotplug.h @@ -28,6 +28,12 @@ # include "qemu_domain.h" # include "domain_conf.h" +int qemuDomainChangeEjectableMedia(virQEMUDriverPtr driver, + virDomainObjPtr vm, + virDomainDiskDefPtr disk, + virStorageSourcePtr newsrc, + bool force); + void qemuDomainDelTLSObjects(virQEMUDriverPtr driver, virDomainObjPtr vm, qemuDomainAsyncJob asyncJob, @@ -54,8 +60,7 @@ int qemuDomainAttachControllerDevice(virQEMUDriverPtr driver, virDomainControllerDefPtr controller); int qemuDomainAttachDeviceDiskLive(virQEMUDriverPtr driver, virDomainObjPtr vm, - virDomainDeviceDefPtr dev, - bool forceMediaChange); + virDomainDeviceDefPtr dev); int qemuDomainAttachNetDevice(virQEMUDriverPtr driver, virDomainObjPtr vm, virDomainNetDefPtr net); diff --git a/tests/qemuhotplugtest.c b/tests/qemuhotplugtest.c index e0e248556f..5b1e0db104 100644 --- a/tests/qemuhotplugtest.c +++ b/tests/qemuhotplugtest.c @@ -120,7 +120,7 @@ testQemuHotplugAttach(virDomainObjPtr vm, /* conn in only used for storage pool and secrets lookup so as long * as we don't use any of them, passing NULL should be safe */ - ret = qemuDomainAttachDeviceDiskLive(&driver, vm, dev, false); + ret = qemuDomainAttachDeviceDiskLive(&driver, vm, dev); break; case VIR_DOMAIN_DEVICE_CHR: ret = qemuDomainAttachChrDevice(&driver, vm, dev->data.chr); -- 2.17.1

Some internal helpers use only a virDomainDiskDef to do their job. If we want to change source for the disk we need to update it to the new source in the disk definition for those to work correctly. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/qemu/qemu_hotplug.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 86afda636e..3043b3257b 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -730,9 +730,12 @@ qemuDomainChangeEjectableMedia(virQEMUDriverPtr driver, bool force) { qemuDomainObjPrivatePtr priv = vm->privateData; + virStorageSourcePtr oldsrc = disk->src; int ret = -1; int rc; + disk->src = newsrc; + if (qemuHotplugPrepareDiskAccess(driver, vm, disk, newsrc, false) < 0) goto cleanup; @@ -744,7 +747,7 @@ qemuDomainChangeEjectableMedia(virQEMUDriverPtr driver, else rc = qemuDomainChangeMediaLegacy(driver, vm, disk, newsrc, force); - virDomainAuditDisk(vm, disk->src, newsrc, "update", rc >= 0); + virDomainAuditDisk(vm, oldsrc, newsrc, "update", rc >= 0); if (rc < 0) { ignore_value(qemuHotplugPrepareDiskAccess(driver, vm, disk, newsrc, true)); @@ -755,7 +758,8 @@ qemuDomainChangeEjectableMedia(virQEMUDriverPtr driver, ignore_value(qemuRemoveSharedDisk(driver, disk, vm->def->name)); ignore_value(qemuHotplugPrepareDiskAccess(driver, vm, disk, NULL, true)); - virStorageSourceFree(disk->src); + virStorageSourceFree(oldsrc); + oldsrc = NULL; VIR_STEAL_PTR(disk->src, newsrc); ignore_value(qemuHotplugRemoveManagedPR(driver, vm, QEMU_ASYNC_JOB_NONE)); @@ -763,6 +767,9 @@ qemuDomainChangeEjectableMedia(virQEMUDriverPtr driver, ret = 0; cleanup: + if (oldsrc) + disk->src = oldsrc; + return ret; } -- 2.17.1

On Tue, Oct 02, 2018 at 08:15:07 -0400, John Ferlan wrote: Umm, well it's kind of pointless to pass in newsrc, but it's the exactly expected behaviour. We want to prepare 'newsrc' but half of the functions take disk->src directly. Hmm, yeah, the blockdev version actually does care what the old source was, so that would not work. The non-blockdev version does not care so it does'nt matter much. I'll have to change qemuDomainChangeMediaBlockdev to take 'oldsrc' instead of 'newsrc'. As explained above, the only thing that happens is that disk->src is replaced and reverted again, thus 3 extra assignments.

The disk storage source needs to be prepared if we want to use -blockdev or secrets for the new media image. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/qemu/qemu_hotplug.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 3043b3257b..1ec696e2aa 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -729,6 +729,7 @@ qemuDomainChangeEjectableMedia(virQEMUDriverPtr driver, virStorageSourcePtr newsrc, bool force) { + virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); qemuDomainObjPrivatePtr priv = vm->privateData; virStorageSourcePtr oldsrc = disk->src; int ret = -1; @@ -736,6 +737,9 @@ qemuDomainChangeEjectableMedia(virQEMUDriverPtr driver, disk->src = newsrc; + if (qemuDomainPrepareDiskSource(disk, priv, cfg) < 0) + goto cleanup; + if (qemuHotplugPrepareDiskAccess(driver, vm, disk, newsrc, false) < 0) goto cleanup; @@ -770,6 +774,7 @@ qemuDomainChangeEjectableMedia(virQEMUDriverPtr driver, if (oldsrc) disk->src = oldsrc; + virObjectUnref(cfg); return ret; } -- 2.17.1

The disk hotplug code also overloads media change which is not ideal. This will allow splitting out of the media change code. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/qemu/qemu_hotplug.c | 27 +++++++++++++++++++++++---- 1 file changed, 23 insertions(+), 4 deletions(-) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 1ec696e2aa..6227a130da 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -1061,10 +1061,10 @@ qemuDomainAttachUSBMassStorageDevice(virQEMUDriverPtr driver, } -int -qemuDomainAttachDeviceDiskLive(virQEMUDriverPtr driver, - virDomainObjPtr vm, - virDomainDeviceDefPtr dev) +static int +qemuDomainAttachDeviceDiskLiveInternal(virQEMUDriverPtr driver, + virDomainObjPtr vm, + virDomainDeviceDefPtr dev) { size_t i; virDomainDiskDefPtr disk = dev->data.disk; @@ -1157,6 +1157,25 @@ qemuDomainAttachDeviceDiskLive(virQEMUDriverPtr driver, } +/** + * qemuDomainAttachDeviceDiskLive: + * @driver: qemu driver struct + * @vm: domain object + * @dev: device to attach (expected type is DISK) + * + * Attach a new disk or in case of cdroms/floppies change the media in the drive. + * This function handles all the necessary steps to attach a new storage source + * to the VM. + */ +int +qemuDomainAttachDeviceDiskLive(virQEMUDriverPtr driver, + virDomainObjPtr vm, + virDomainDeviceDefPtr dev) +{ + return qemuDomainAttachDeviceDiskLiveInternal(driver, vm, dev); +} + + static void qemuDomainNetDeviceVportRemove(virDomainNetDefPtr net) { -- 2.17.1

In cases where we know the device is a disk we can avoid using the full device definition. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/qemu/qemu_conf.c | 2 +- src/qemu/qemu_conf.h | 5 +++++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index 33508174cb..6cdc0f407a 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -1411,7 +1411,7 @@ qemuSharedDeviceEntryInsert(virQEMUDriverPtr driver, * records all the domains that use the shared device if the entry * already exists, otherwise add a new entry. */ -static int +int qemuAddSharedDisk(virQEMUDriverPtr driver, virDomainDiskDefPtr disk, const char *name) diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h index c227ac72cc..f876f9117c 100644 --- a/src/qemu/qemu_conf.h +++ b/src/qemu/qemu_conf.h @@ -339,6 +339,11 @@ char *qemuGetSharedDeviceKey(const char *disk_path) void qemuSharedDeviceEntryFree(void *payload, const void *name); +int qemuAddSharedDisk(virQEMUDriverPtr driver, + virDomainDiskDefPtr disk, + const char *name) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3); + int qemuAddSharedDevice(virQEMUDriverPtr driver, virDomainDeviceDefPtr dev, const char *name) -- 2.17.1

Disk hotplug has slightly different semantics from media changing. Move the media change code out and add proper initialization of the new source object. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/qemu/qemu_driver.c | 15 +---------- src/qemu/qemu_hotplug.c | 56 +++++++++++++++++++++++++---------------- 2 files changed, 35 insertions(+), 36 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 64f0ad33e8..f609151152 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -7851,12 +7851,6 @@ qemuDomainChangeDiskLive(virDomainObjPtr vm, virDomainDeviceDef oldDev = { .type = dev->type }; int ret = -1; - if (virDomainDiskTranslateSourcePool(disk) < 0) - goto cleanup; - - if (qemuDomainDetermineDiskChain(driver, vm, disk, true) < 0) - goto cleanup; - if (!(orig_disk = virDomainDiskFindByBusAndDst(vm->def, disk->bus, disk->dst))) { virReportError(VIR_ERR_INTERNAL_ERROR, @@ -7885,16 +7879,9 @@ qemuDomainChangeDiskLive(virDomainObjPtr vm, goto cleanup; } - /* Add the new disk src into shared disk hash table */ - if (qemuAddSharedDevice(driver, dev, vm->def->name) < 0) - goto cleanup; - if (qemuDomainChangeEjectableMedia(driver, vm, orig_disk, - dev->data.disk->src, force) < 0) { - ignore_value(qemuRemoveSharedDisk(driver, dev->data.disk, - vm->def->name)); + dev->data.disk->src, force) < 0) goto cleanup; - } dev->data.disk->src = NULL; } diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 6227a130da..ef3f0cd8b3 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -737,6 +737,15 @@ qemuDomainChangeEjectableMedia(virQEMUDriverPtr driver, disk->src = newsrc; + if (virDomainDiskTranslateSourcePool(disk) < 0) + goto cleanup; + + if (qemuAddSharedDisk(driver, disk, vm->def->name) < 0) + goto cleanup; + + if (qemuDomainDetermineDiskChain(driver, vm, disk, true) < 0) + goto cleanup; + if (qemuDomainPrepareDiskSource(disk, priv, cfg) < 0) goto cleanup; @@ -1068,9 +1077,15 @@ qemuDomainAttachDeviceDiskLiveInternal(virQEMUDriverPtr driver, { size_t i; virDomainDiskDefPtr disk = dev->data.disk; - virDomainDiskDefPtr orig_disk = NULL; int ret = -1; + if (disk->device == VIR_DOMAIN_DISK_DEVICE_CDROM || + disk->device == VIR_DOMAIN_DISK_DEVICE_FLOPPY) { + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", + _("cdrom/floppy device hotplug isn't supported")); + return -1; + } + if (virDomainDiskTranslateSourcePool(disk) < 0) goto cleanup; @@ -1084,27 +1099,6 @@ qemuDomainAttachDeviceDiskLiveInternal(virQEMUDriverPtr driver, goto cleanup; switch ((virDomainDiskDevice) disk->device) { - case VIR_DOMAIN_DISK_DEVICE_CDROM: - case VIR_DOMAIN_DISK_DEVICE_FLOPPY: - if (!(orig_disk = virDomainDiskFindByBusAndDst(vm->def, - disk->bus, disk->dst))) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("No device with bus '%s' and target '%s'. " - "cdrom and floppy device hotplug isn't supported " - "by libvirt"), - virDomainDiskBusTypeToString(disk->bus), - disk->dst); - goto cleanup; - } - - if (qemuDomainChangeEjectableMedia(driver, vm, orig_disk, - disk->src, false) < 0) - goto cleanup; - - disk->src = NULL; - ret = 0; - break; - case VIR_DOMAIN_DISK_DEVICE_DISK: case VIR_DOMAIN_DISK_DEVICE_LUN: for (i = 0; i < vm->def->ndisks; i++) { @@ -1146,6 +1140,8 @@ qemuDomainAttachDeviceDiskLiveInternal(virQEMUDriverPtr driver, } break; + case VIR_DOMAIN_DISK_DEVICE_CDROM: + case VIR_DOMAIN_DISK_DEVICE_FLOPPY: case VIR_DOMAIN_DISK_DEVICE_LAST: break; } @@ -1172,6 +1168,22 @@ qemuDomainAttachDeviceDiskLive(virQEMUDriverPtr driver, virDomainObjPtr vm, virDomainDeviceDefPtr dev) { + virDomainDiskDefPtr disk = dev->data.disk; + virDomainDiskDefPtr orig_disk = NULL; + + /* this API overloads media change semantics on disk hotplug + * for devices supporting media changes */ + if ((disk->device == VIR_DOMAIN_DISK_DEVICE_CDROM || + disk->device == VIR_DOMAIN_DISK_DEVICE_FLOPPY) && + (orig_disk = virDomainDiskFindByBusAndDst(vm->def, disk->bus, disk->dst))) { + if (qemuDomainChangeEjectableMedia(driver, vm, orig_disk, + disk->src, false) < 0) + return -1; + + disk->src = NULL; + return 0; + } + return qemuDomainAttachDeviceDiskLiveInternal(driver, vm, dev); } -- 2.17.1

We now explicitly handle media change elsewhere so we can drop the switch statement. This will also make it more intuitive once CDROM device hotplug might be supported. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/qemu/qemu_hotplug.c | 70 ++++++++++++++++++----------------------- 1 file changed, 30 insertions(+), 40 deletions(-) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index ef3f0cd8b3..d5e1e2a039 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -1098,52 +1098,42 @@ qemuDomainAttachDeviceDiskLiveInternal(virQEMUDriverPtr driver, if (qemuDomainDetermineDiskChain(driver, vm, disk, true) < 0) goto cleanup; - switch ((virDomainDiskDevice) disk->device) { - case VIR_DOMAIN_DISK_DEVICE_DISK: - case VIR_DOMAIN_DISK_DEVICE_LUN: - for (i = 0; i < vm->def->ndisks; i++) { - if (virDomainDiskDefCheckDuplicateInfo(vm->def->disks[i], disk) < 0) - goto cleanup; - } - - switch ((virDomainDiskBus) disk->bus) { - case VIR_DOMAIN_DISK_BUS_USB: - if (disk->device == VIR_DOMAIN_DISK_DEVICE_LUN) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("disk device='lun' is not supported for usb bus")); - break; - } - ret = qemuDomainAttachUSBMassStorageDevice(driver, vm, disk); - break; - - case VIR_DOMAIN_DISK_BUS_VIRTIO: - ret = qemuDomainAttachVirtioDiskDevice(driver, vm, disk); - break; + for (i = 0; i < vm->def->ndisks; i++) { + if (virDomainDiskDefCheckDuplicateInfo(vm->def->disks[i], disk) < 0) + goto cleanup; + } - case VIR_DOMAIN_DISK_BUS_SCSI: - ret = qemuDomainAttachSCSIDisk(driver, vm, disk); + switch ((virDomainDiskBus) disk->bus) { + case VIR_DOMAIN_DISK_BUS_USB: + if (disk->device == VIR_DOMAIN_DISK_DEVICE_LUN) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("disk device='lun' is not supported for usb bus")); break; - - case VIR_DOMAIN_DISK_BUS_IDE: - case VIR_DOMAIN_DISK_BUS_FDC: - case VIR_DOMAIN_DISK_BUS_XEN: - case VIR_DOMAIN_DISK_BUS_UML: - case VIR_DOMAIN_DISK_BUS_SATA: - case VIR_DOMAIN_DISK_BUS_SD: - /* Note that SD card hotplug support should be added only once - * they support '-device' (don't require -drive only). - * See also: qemuDiskBusNeedsDriveArg */ - case VIR_DOMAIN_DISK_BUS_LAST: - virReportError(VIR_ERR_OPERATION_UNSUPPORTED, - _("disk bus '%s' cannot be hotplugged."), - virDomainDiskBusTypeToString(disk->bus)); } + ret = qemuDomainAttachUSBMassStorageDevice(driver, vm, disk); + break; + + case VIR_DOMAIN_DISK_BUS_VIRTIO: + ret = qemuDomainAttachVirtioDiskDevice(driver, vm, disk); break; - case VIR_DOMAIN_DISK_DEVICE_CDROM: - case VIR_DOMAIN_DISK_DEVICE_FLOPPY: - case VIR_DOMAIN_DISK_DEVICE_LAST: + case VIR_DOMAIN_DISK_BUS_SCSI: + ret = qemuDomainAttachSCSIDisk(driver, vm, disk); break; + + case VIR_DOMAIN_DISK_BUS_IDE: + case VIR_DOMAIN_DISK_BUS_FDC: + case VIR_DOMAIN_DISK_BUS_XEN: + case VIR_DOMAIN_DISK_BUS_UML: + case VIR_DOMAIN_DISK_BUS_SATA: + case VIR_DOMAIN_DISK_BUS_SD: + /* Note that SD card hotplug support should be added only once + * they support '-device' (don't require -drive only). + * See also: qemuDiskBusNeedsDriveArg */ + case VIR_DOMAIN_DISK_BUS_LAST: + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, + _("disk bus '%s' cannot be hotplugged."), + virDomainDiskBusTypeToString(disk->bus)); } cleanup: -- 2.17.1