Re: [libvirt] [PATCH 1/2] qemu: Add callback struct for qemuBuildCommandLine

On 15/05/13 23:32, Daniel P. Berrange wrote: >On Wed, May 15, 2013 at 11:25:19PM +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/util/virscsi.c: (Allow to specify the sysfs root for >> virSCSIDeviceGetSgName) >>* src/util/virscsi.h: (Change the definition of virSCSIDeviceGetSgName) >>* tests/sysfsroot/* : (New test input files) >>* tests/Makefile.am: (Add "sysfsroot" in EXTRA_DIST) >You shouldn't need any of these changes if.... > >>diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c >>index ac31d2a..3d22491 100644 >>--- a/tests/qemuxml2argvtest.c >>+++ b/tests/qemuxml2argvtest.c >>@@ -82,6 +82,21 @@ typedef enum { >> FLAG_JSON = 1 << 3, >> } virQemuXML2ArgvTestFlags; >>+# define TEST_SYSFS_ROOT "./sysfsroot" >>+ >>+static char * >>+testSCSIDeviceGetSgName(const char *adapter, >>+ unsigned int bus, >>+ unsigned int target, >>+ unsigned int unit) >>+{ >>+ return virSCSIDeviceGetSgName(adapter, bus, target, unit, TEST_SYSFS_ROOT); >>+} >...you change this to just be > > return strdup("/dev/sg2") > >or whatever it needs to be. No need to call into the virSCSI apis >from the test suite I thought calling the virSCSIDeviceGetSgName is better, as it let us make sure things work fine to qemu command line building in real practice, And it's not overkill to have a single file path as the test input.