diff mbox

qemu: command: Ignore QEMU_CAPS_DEVICE when building drive alias

Message ID 7f1e34f1ed8abe5bc7e4dc0100c3bfdabf51b9b8.1463346235.git.crobinso@redhat.com
State Accepted
Commit c7d6c13989932d32257b78b9062f1a39dcc83385
Headers show

Commit Message

Cole Robinson May 15, 2016, 9:03 p.m. UTC
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(-)

-- 
2.7.4

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

Comments

Cole Robinson May 16, 2016, 1:03 p.m. UTC | #1
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_qemuDeviceDriveHostAlias.28.29

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

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

Patch

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)))