[libvirt] [PATCH] qemu: command: Ignore QEMU_CAPS_DEVICE when building drive alias
QEMU_CAPS_DEVICE is always set nowadays, so we can drop the non-DEVICE code paths --- src/qemu/qemu_command.c | 12 ++++-------- src/qemu/qemu_command.h | 3 +-- src/qemu/qemu_hotplug.c | 4 ++-- 3 files changed, 7 insertions(+), 12 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 0d6d5f8..4286999 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -377,17 +377,13 @@ qemuBuildObjectCommandlineFromJSON(const char *type, } -char *qemuDeviceDriveHostAlias(virDomainDiskDefPtr disk, - virQEMUCapsPtr qemuCaps) +char *qemuDeviceDriveHostAlias(virDomainDiskDefPtr disk) { char *ret; - if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) { - ignore_value(virAsprintf(&ret, "%s%s", QEMU_DRIVE_HOST_PREFIX, - disk->info.alias)); - } else { - ignore_value(VIR_STRDUP(ret, disk->info.alias)); - } + if (virAsprintf(&ret, "%s%s", + QEMU_DRIVE_HOST_PREFIX, disk->info.alias) < 0) + return NULL; return ret; } diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h index d5ad1b2..c777701 100644 --- a/src/qemu/qemu_command.h +++ b/src/qemu/qemu_command.h @@ -96,8 +96,7 @@ char *qemuBuildNicDevStr(virDomainDefPtr def, size_t vhostfdSize, virQEMUCapsPtr qemuCaps); -char *qemuDeviceDriveHostAlias(virDomainDiskDefPtr disk, - virQEMUCapsPtr qemuCaps); +char *qemuDeviceDriveHostAlias(virDomainDiskDefPtr disk); /* Both legacy & current support */ char *qemuBuildDriveStr(virDomainDiskDefPtr disk, diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index f40b34d..f8ab095 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -192,7 +192,7 @@ qemuDomainChangeEjectableMedia(virQEMUDriverPtr driver, if (qemuDomainPrepareDisk(driver, vm, disk, newsrc, false) < 0) goto cleanup; - if (!(driveAlias = qemuDeviceDriveHostAlias(disk, priv->qemuCaps))) + if (!(driveAlias = qemuDeviceDriveHostAlias(disk))) goto error; do { @@ -376,7 +376,7 @@ qemuDomainAttachVirtioDiskDevice(virConnectPtr conn, if (!(drivestr = qemuBuildDriveStr(disk, false, priv->qemuCaps))) goto error; - if (!(drivealias = qemuDeviceDriveHostAlias(disk, priv->qemuCaps))) + if (!(drivealias = qemuDeviceDriveHostAlias(disk))) goto error; if (!(devstr = qemuBuildDriveDevStr(vm->def, disk, 0, priv->qemuCaps))) -- 2.7.4
On Sun, May 15, 2016 at 17:03:55 -0400, Cole Robinson wrote:
QEMU_CAPS_DEVICE is always set nowadays, so we can drop the non-DEVICE code paths --- src/qemu/qemu_command.c | 12 ++++-------- src/qemu/qemu_command.h | 3 +-- src/qemu/qemu_hotplug.c | 4 ++-- 3 files changed, 7 insertions(+), 12 deletions(-)
[...]
diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h index d5ad1b2..c777701 100644 --- a/src/qemu/qemu_command.h +++ b/src/qemu/qemu_command.h @@ -96,8 +96,7 @@ char *qemuBuildNicDevStr(virDomainDefPtr def, size_t vhostfdSize, virQEMUCapsPtr qemuCaps);
-char *qemuDeviceDriveHostAlias(virDomainDiskDefPtr disk, - virQEMUCapsPtr qemuCaps); +char *qemuDeviceDriveHostAlias(virDomainDiskDefPtr disk);
ACK, although it's weird that this function is only used in the hotplug code. This has probably to do with the very weird command line generator code for drives which actually clears QEMU_CAPS_DEVICE in certain cases and then sets it back once finished. I have to finish the patches getting rid of that oddity. Peter
On 05/16/2016 02:42 AM, Peter Krempa wrote:
On Sun, May 15, 2016 at 17:03:55 -0400, Cole Robinson wrote:
QEMU_CAPS_DEVICE is always set nowadays, so we can drop the non-DEVICE code paths --- src/qemu/qemu_command.c | 12 ++++-------- src/qemu/qemu_command.h | 3 +-- src/qemu/qemu_hotplug.c | 4 ++-- 3 files changed, 7 insertions(+), 12 deletions(-)
[...]
diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h index d5ad1b2..c777701 100644 --- a/src/qemu/qemu_command.h +++ b/src/qemu/qemu_command.h @@ -96,8 +96,7 @@ char *qemuBuildNicDevStr(virDomainDefPtr def, size_t vhostfdSize, virQEMUCapsPtr qemuCaps);
-char *qemuDeviceDriveHostAlias(virDomainDiskDefPtr disk, - virQEMUCapsPtr qemuCaps); +char *qemuDeviceDriveHostAlias(virDomainDiskDefPtr disk);
ACK, although it's weird that this function is only used in the hotplug code.
Yep, and even outside qemu_command.c it's not consistently used. After sending this I added a BiteSizedTask to look for places to convert: http://wiki.libvirt.org/page/BiteSizedTasks#More_usage_of_qemuDeviceDriveHos... This has probably to do with the very weird command line generator
code for drives which actually clears QEMU_CAPS_DEVICE in certain cases and then sets it back once finished.
Yes, I was scratching my head over that code again recently. I had patches for it at one point but I doubt they apply any longer and I won't get back to them anytime soon, so if you take a stab at it I'll help review. http://www.redhat.com/archives/libvir-list/2016-January/msg00995.html Patch pushed now, thanks - Cole
participants (2)
-
Cole Robinson -
Peter Krempa