[libvirt] [PATCH v4] qemu: Add callback struct for qemuBuildCommandLine
Osier Yang
jyang at redhat.com
Mon May 20 12:19:56 UTC 2013
On 20/05/13 19:36, Daniel P. Berrange wrote:
> On Fri, May 17, 2013 at 06:34:24PM +0800, Osier Yang wrote:
>> Since 0d70656afded, it starts to access the sysfs files to build
>> the qemu command line (by virSCSIDeviceGetSgName, which is to find
>> out the scsi generic device name by adpater:bus:target:unit), there
>> is no way to work around, qemu wants to see the scsi generic device
>> like "/dev/sg6" anyway.
>>
>> And there might be other places which need to access sysfs files
>> when building qemu command line in future.
>>
>> Instead of increasing the arguments of qemuBuildCommandLine, this
>> introduces a new callback for qemuBuildCommandLine, and thus tests
>> can register their own callbacks for sysfs test input files accessing.
>>
>> * src/qemu/qemu_command.h: (New callback struct
>> qemuBuildCommandLineCallbacks;
>> extern buildCommandLineCallbacks)
>> * src/qemu/qemu_command.c: (wire up the callback struct)
>> * src/qemu/qemu_driver.c: (Use the new syntax of qemuBuildCommandLine)
>> * src/qemu/qemu_hotplug.c: Likewise
>> * src/qemu/qemu_process.c: Likewise
>> * tests/qemuxml2argvtest.c: (Helper testSCSIDeviceGetSgName;
>> callback struct testCallbacks;
>> Use the callback struct)
>> * src/tests/qemuxmlnstest.c: (Like above)
>> ---
>> src/qemu/qemu_command.c | 22 ++++++++++++++--------
>> src/qemu/qemu_command.h | 19 ++++++++++++++++---
>> src/qemu/qemu_driver.c | 3 ++-
>> src/qemu/qemu_hotplug.c | 6 ++++--
>> src/qemu/qemu_process.c | 3 ++-
>> tests/qemuxml2argvtest.c | 21 ++++++++++++++++++++-
>> tests/qemuxmlnstest.c | 21 ++++++++++++++++++++-
>> 7 files changed, 78 insertions(+), 17 deletions(-)
>>
>> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
>> index c268a3a..33a1910 100644
>> --- a/src/qemu/qemu_command.c
>> +++ b/src/qemu/qemu_command.c
>> @@ -4757,17 +4757,18 @@ qemuBuildUSBHostdevUsbDevStr(virDomainHostdevDefPtr dev)
>>
>> char *
>> qemuBuildSCSIHostdevDrvStr(virDomainHostdevDefPtr dev,
>> - virQEMUCapsPtr qemuCaps ATTRIBUTE_UNUSED)
>> + virQEMUCapsPtr qemuCaps ATTRIBUTE_UNUSED,
>> + qemuBuildCommandLineCallbacksPtr callbacks)
>> {
>> virBuffer buf = VIR_BUFFER_INITIALIZER;
>> char *sg = NULL;
>>
>> - if (!(sg = virSCSIDeviceGetSgName(dev->source.subsys.u.scsi.adapter,
>> - dev->source.subsys.u.scsi.bus,
>> - dev->source.subsys.u.scsi.target,
>> - dev->source.subsys.u.scsi.unit))) {
>> + sg = (callbacks->qemuGetSCSIDeviceSgName)(dev->source.subsys.u.scsi.adapter,
>> + dev->source.subsys.u.scsi.bus,
>> + dev->source.subsys.u.scsi.target,
>> + dev->source.subsys.u.scsi.unit);
>> + if (!sg)
>> goto error;
>> - }
>>
>> virBufferAsprintf(&buf, "file=/dev/%s,if=none", sg);
>> virBufferAsprintf(&buf, ",id=%s-%s",
>> @@ -6405,6 +6406,10 @@ qemuBuildGraphicsCommandLine(virQEMUDriverConfigPtr cfg,
>> return 0;
>> }
>>
>> +qemuBuildCommandLineCallbacks buildCommandLineCallbacks = {
>> + .qemuGetSCSIDeviceSgName = virSCSIDeviceGetSgName,
>> +};
>> +
>> /*
>> * Constructs a argv suitable for launching qemu with config defined
>> * for a given virtual machine.
>> @@ -6422,7 +6427,8 @@ qemuBuildCommandLine(virConnectPtr conn,
>> const char *migrateFrom,
>> int migrateFd,
>> virDomainSnapshotObjPtr snapshot,
>> - enum virNetDevVPortProfileOp vmop)
>> + enum virNetDevVPortProfileOp vmop,
>> + qemuBuildCommandLineCallbacksPtr callbacks)
>> {
>> virErrorPtr originalError = NULL;
>> int i, j;
>> @@ -8254,7 +8260,7 @@ qemuBuildCommandLine(virConnectPtr conn,
>> char *drvstr;
>>
>> virCommandAddArg(cmd, "-drive");
>> - if (!(drvstr = qemuBuildSCSIHostdevDrvStr(hostdev, qemuCaps)))
>> + if (!(drvstr = qemuBuildSCSIHostdevDrvStr(hostdev, qemuCaps, callbacks)))
>> goto error;
>> virCommandAddArg(cmd, drvstr);
>> VIR_FREE(drvstr);
>> diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h
>> index 36dfa6b..133e0b2 100644
>> --- a/src/qemu/qemu_command.h
>> +++ b/src/qemu/qemu_command.h
>> @@ -51,6 +51,16 @@
>> # define QEMU_WEBSOCKET_PORT_MIN 5700
>> # define QEMU_WEBSOCKET_PORT_MAX 65535
>>
>> +typedef struct _qemuBuildCommandLineCallbacks qemuBuildCommandLineCallbacks;
>> +typedef qemuBuildCommandLineCallbacks *qemuBuildCommandLineCallbacksPtr;
>> +struct _qemuBuildCommandLineCallbacks {
>> + char * (*qemuGetSCSIDeviceSgName) (const char *adapter,
>> + unsigned int bus,
>> + unsigned int target,
>> + unsigned int unit);
>> +};
>> +
>> +extern qemuBuildCommandLineCallbacks buildCommandLineCallbacks;
>>
>> virCommandPtr qemuBuildCommandLine(virConnectPtr conn,
>> virQEMUDriverPtr driver,
>> @@ -61,8 +71,9 @@ virCommandPtr qemuBuildCommandLine(virConnectPtr conn,
>> const char *migrateFrom,
>> int migrateFd,
>> virDomainSnapshotObjPtr current_snapshot,
>> - enum virNetDevVPortProfileOp vmop)
>> - ATTRIBUTE_NONNULL(1);
>> + enum virNetDevVPortProfileOp vmop,
>> + qemuBuildCommandLineCallbacksPtr callbacks)
>> + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(11);
>>
>> /* Generate string for arch-specific '-device' parameter */
>> char *
>> @@ -142,7 +153,9 @@ char * qemuBuildUSBHostdevDevStr(virDomainHostdevDefPtr dev,
>> virQEMUCapsPtr qemuCaps);
>>
>> char * qemuBuildSCSIHostdevDrvStr(virDomainHostdevDefPtr dev,
>> - virQEMUCapsPtr qemuCaps);
>> + virQEMUCapsPtr qemuCaps,
>> + qemuBuildCommandLineCallbacksPtr callbacks)
>> + ATTRIBUTE_NONNULL(3);
>> char * qemuBuildSCSIHostdevDevStr(virDomainDefPtr def,
>> virDomainHostdevDefPtr dev,
>> virQEMUCapsPtr qemuCaps);
>> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
>> index 203cc6d..0665131 100644
>> --- a/src/qemu/qemu_driver.c
>> +++ b/src/qemu/qemu_driver.c
>> @@ -5362,7 +5362,8 @@ static char *qemuConnectDomainXMLToNative(virConnectPtr conn,
>>
>> if (!(cmd = qemuBuildCommandLine(conn, driver, def,
>> &monConfig, monitor_json, qemuCaps,
>> - NULL, -1, NULL, VIR_NETDEV_VPORT_PROFILE_OP_NO_OP)))
>> + NULL, -1, NULL, VIR_NETDEV_VPORT_PROFILE_OP_NO_OP,
>> + &buildCommandLineCallbacks)))
>> goto cleanup;
>>
>> ret = virCommandToString(cmd);
>> diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
>> index d037c9d..77d9f4f 100644
>> --- a/src/qemu/qemu_hotplug.c
>> +++ b/src/qemu/qemu_hotplug.c
>> @@ -1226,7 +1226,8 @@ qemuDomainAttachHostScsiDevice(virQEMUDriverPtr driver,
>> if (qemuAssignDeviceHostdevAlias(vm->def, hostdev, 0) < 0)
>> goto cleanup;
>>
>> - if (!(drvstr = qemuBuildSCSIHostdevDrvStr(hostdev, priv->qemuCaps)))
>> + if (!(drvstr = qemuBuildSCSIHostdevDrvStr(hostdev, priv->qemuCaps,
>> + &buildCommandLineCallbacks)))
>> goto cleanup;
>>
>> if (!(devstr = qemuBuildSCSIHostdevDevStr(vm->def, hostdev, priv->qemuCaps)))
>> @@ -2543,7 +2544,8 @@ qemuDomainDetachHostScsiDevice(virQEMUDriverPtr driver,
>> return -1;
>> }
>>
>> - if (!(drvstr = qemuBuildSCSIHostdevDrvStr(detach, priv->qemuCaps)))
>> + if (!(drvstr = qemuBuildSCSIHostdevDrvStr(detach, priv->qemuCaps,
>> + &buildCommandLineCallbacks)))
>> goto cleanup;
>> if (!(devstr = qemuBuildSCSIHostdevDevStr(vm->def, detach, priv->qemuCaps)))
>> goto cleanup;
>> diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
>> index 4a7c612..95e712c 100644
>> --- a/src/qemu/qemu_process.c
>> +++ b/src/qemu/qemu_process.c
>> @@ -3605,7 +3605,8 @@ int qemuProcessStart(virConnectPtr conn,
>> VIR_DEBUG("Building emulator command line");
>> if (!(cmd = qemuBuildCommandLine(conn, driver, vm->def, priv->monConfig,
>> priv->monJSON, priv->qemuCaps,
>> - migrateFrom, stdin_fd, snapshot, vmop)))
>> + migrateFrom, stdin_fd, snapshot, vmop,
>> + &buildCommandLineCallbacks)))
>> goto cleanup;
>>
>> /* now that we know it is about to start call the hook if present */
>> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
>> index d853308..e103925 100644
>> --- a/tests/qemuxml2argvtest.c
>> +++ b/tests/qemuxml2argvtest.c
>> @@ -82,6 +82,24 @@ typedef enum {
>> FLAG_JSON = 1 << 3,
>> } virQemuXML2ArgvTestFlags;
>>
>> +static char *
>> +testSCSIDeviceGetSgName(const char *adapter ATTRIBUTE_UNUSED,
>> + unsigned int bus ATTRIBUTE_UNUSED,
>> + unsigned int target ATTRIBUTE_UNUSED,
>> + unsigned int unit ATTRIBUTE_UNUSED)
>> +{
>> + char *sg = NULL;
>> +
>> + if (VIR_STRDUP(sg, "sg0") < 0)
>> + return NULL;
>> +
>> + return sg;
>> +}
>> +
>> +static qemuBuildCommandLineCallbacks testCallbacks = {
>> + .qemuGetSCSIDeviceSgName = testSCSIDeviceGetSgName,
>> +};
> I think I suggested putting this 'qemuBuildCommandLineCallbacks' instance
> in testutilsqemu.{c,h} so that it could be share in all test cases, instead
> of them having to duplicate it as you've done here:
>
Oh, right, I pushed it with the following diff:
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index e103925..fb61017 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -82,24 +82,6 @@ typedef enum {
FLAG_JSON = 1 << 3,
} virQemuXML2ArgvTestFlags;
-static char *
-testSCSIDeviceGetSgName(const char *adapter ATTRIBUTE_UNUSED,
- unsigned int bus ATTRIBUTE_UNUSED,
- unsigned int target ATTRIBUTE_UNUSED,
- unsigned int unit ATTRIBUTE_UNUSED)
-{
- char *sg = NULL;
-
- if (VIR_STRDUP(sg, "sg0") < 0)
- return NULL;
-
- return sg;
-}
-
-static qemuBuildCommandLineCallbacks testCallbacks = {
- .qemuGetSCSIDeviceSgName = testSCSIDeviceGetSgName,
-};
-
static int testCompareXMLToArgvFiles(const char *xml,
const char *cmdline,
virQEMUCapsPtr extraFlags,
diff --git a/tests/qemuxmlnstest.c b/tests/qemuxmlnstest.c
index 961405b..9f4f442 100644
--- a/tests/qemuxmlnstest.c
+++ b/tests/qemuxmlnstest.c
@@ -26,24 +26,6 @@
static const char *abs_top_srcdir;
static virQEMUDriver driver;
-static char *
-testSCSIDeviceGetSgName(const char *adapter ATTRIBUTE_UNUSED,
- unsigned int bus ATTRIBUTE_UNUSED,
- unsigned int target ATTRIBUTE_UNUSED,
- unsigned int unit ATTRIBUTE_UNUSED)
-{
- char *sg = NULL;
-
- if (VIR_STRDUP(sg, "dummy") < 0)
- return NULL;
-
- return sg;
-}
-
-static qemuBuildCommandLineCallbacks testCallbacks = {
- .qemuGetSCSIDeviceSgName = testSCSIDeviceGetSgName,
-};
-
static int testCompareXMLToArgvFiles(const char *xml,
const char *cmdline,
virQEMUCapsPtr extraFlags,
diff --git a/tests/testutilsqemu.c b/tests/testutilsqemu.c
index 45ca6fe..fac83b2 100644
--- a/tests/testutilsqemu.c
+++ b/tests/testutilsqemu.c
@@ -289,4 +289,23 @@ cleanup:
virObjectUnref(caps);
return NULL;
}
+
+
+static char *
+testSCSIDeviceGetSgName(const char *adapter ATTRIBUTE_UNUSED,
+ unsigned int bus ATTRIBUTE_UNUSED,
+ unsigned int target ATTRIBUTE_UNUSED,
+ unsigned int unit ATTRIBUTE_UNUSED)
+{
+ char *sg = NULL;
+
+ if (VIR_STRDUP(sg, "sg0") < 0)
+ return NULL;
+
+ return sg;
+}
+
+qemuBuildCommandLineCallbacks testCallbacks = {
+ .qemuGetSCSIDeviceSgName = testSCSIDeviceGetSgName,
+};
#endif
diff --git a/tests/testutilsqemu.h b/tests/testutilsqemu.h
index d7d5c0a..2ca42ab 100644
--- a/tests/testutilsqemu.h
+++ b/tests/testutilsqemu.h
@@ -1,6 +1,8 @@
#include "capabilities.h"
#include "domain_conf.h"
+#include "qemu/qemu_command.h"
virCapsPtr testQemuCapsInit(void);
virDomainXMLOptionPtr testQemuXMLConfInit(void);
+extern qemuBuildCommandLineCallbacks testCallbacks;
Thanks,
Osier
More information about the libvir-list
mailing list