On Thu, May 16, 2013 at 12:33:56AM +0800, Osier Yang wrote: > 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. Well we really need a complete standalone test suite for all the virSCSI APIs, so we can validate their actions explicitly. 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