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:
> diff --git a/tests/qemuxmlnstest.c b/tests/qemuxmlnstest.c
> index 952b8e2..961405b 100644
> --- a/tests/qemuxmlnstest.c
> +++ b/tests/qemuxmlnstest.c
> @@ -26,6 +26,24 @@
> 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,
> +};
> +
ACK either way
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list