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

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

> > /* Generate string for arch-specific '-device' parameter */ > @@ -142,7 +153,8 @@ char * qemuBuildUSBHostdevDevStr(virDomainHostdevDefPtr dev, > virQEMUCapsPtr qemuCaps); > > char * qemuBuildSCSIHostdevDrvStr(virDomainHostdevDefPtr dev, > - virQEMUCapsPtr qemuCaps); > + virQEMUCapsPtr qemuCaps, > + qemuBuildCommandLineCallbacksPtr callbacks); ...the same here.