Re: [libvirt] [PATCH v3] qemu: Add callback struct for qemuBuildCommandLine

On 17/05/13 18:12, Daniel P. Berrange wrote: >On Thu, May 16, 2013 at 11:02:00PM +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 Use the new syntax of qemuBuildCommandLine) >>--- >> src/qemu/qemu_command.c | 28 ++++++++++++++++++++-------- >> src/qemu/qemu_command.h | 16 ++++++++++++++-- >> 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 | 3 ++- >> 7 files changed, 64 insertions(+), 16 deletions(-) >> >>diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c >>index 5b95c07..e775479 100644 >>--- a/src/qemu/qemu_command.c >>+++ b/src/qemu/qemu_command.c >>@@ -4757,18 +4757,25 @@ 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))) { >>- goto error; >>+ if (!callbacks) { >>+ virReportError(VIR_ERR_INVALID_ARG, "%s", >>+ _("callbacks cannot be NULL")); >>+ return NULL; >I think I'd remove this check and.... > > >>diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h >>index 36dfa6b..23b6af3 100644 >>--- a/src/qemu/qemu_command.h >>+++ b/src/qemu/qemu_command.h >>@@ -61,7 +71,8 @@ virCommandPtr qemuBuildCommandLine(virConnectPtr conn, >> const char *migrateFrom, >> int migrateFd, >> virDomainSnapshotObjPtr current_snapshot, >>- enum virNetDevVPortProfileOp vmop) >>+ enum virNetDevVPortProfileOp vmop, >>+ qemuBuildCommandLineCallbacksPtr callbacks) >> ATTRIBUTE_NONNULL(1); >...add another ATTRIBUTE_NONNULL here and... Do we need ATTRIBUT_NONULL for qemuBuildCommandLine too? the callbacks could be NULL in case of it won't call qemuBuildSCSIHostdevDrvStr, or something else in future.

If it should be NONNULL, I have to set up the callback from qemuxmlnstest.c too, the callbacks is meaningless for it though.