[libvirt] [PATCH 0/2 v2] Fix the build failure
As exposed by [1], qemuxml2argvtest fails because of it tries to access non-existing sysfs files when building qemu command for scsi-generic device. This creates "tests/sysfsroot", uses it as the fake sysfs root, any future sysfs test input files should be put into it. [1] https://www.redhat.com/archives/libvir-list/2013-May/msg00923.html v1 - v2: * Use callbacks instead of increasing the arguments Osier Yang (2): qemu: Add callback struct for qemuBuildCommandLine tests: Move fchostdata into sysfsroot src/qemu/qemu_command.c | 31 ++++++++++++++----- src/qemu/qemu_command.h | 16 ++++++++-- src/qemu/qemu_driver.c | 3 +- src/qemu/qemu_hotplug.c | 6 ++-- src/qemu/qemu_process.c | 3 +- src/util/virscsi.c | 35 ++++++++++++++++++---- src/util/virscsi.h | 3 +- tests/Makefile.am | 2 +- tests/fchostdata/fc_host/host4/fabric_name | 1 - tests/fchostdata/fc_host/host4/max_npiv_vports | 1 - tests/fchostdata/fc_host/host4/node_name | 1 - tests/fchostdata/fc_host/host4/npiv_vports_inuse | 1 - tests/fchostdata/fc_host/host4/port_name | 1 - tests/fchostdata/fc_host/host4/port_state | 1 - tests/fchostdata/fc_host/host4/vport_create | 0 tests/fchostdata/fc_host/host4/vport_delete | 0 tests/fchostdata/fc_host/host5/fabric_name | 1 - tests/fchostdata/fc_host/host5/max_npiv_vports | 1 - tests/fchostdata/fc_host/host5/node_name | 1 - tests/fchostdata/fc_host/host5/npiv_vports_inuse | 1 - tests/fchostdata/fc_host/host5/port_name | 1 - tests/fchostdata/fc_host/host5/port_state | 1 - tests/fchostdata/fc_host/host5/vport_create | 0 tests/fchostdata/fc_host/host5/vport_delete | 0 tests/fchosttest.c | 2 +- tests/qemuxml2argvtest.c | 18 ++++++++++- tests/qemuxmlnstest.c | 3 +- .../bus/scsi/devices/0:0:0:0/scsi_generic/sg0/dev | 1 + .../class/fchostdata/fc_host/host4/fabric_name | 1 + .../class/fchostdata/fc_host/host4/max_npiv_vports | 1 + .../class/fchostdata/fc_host/host4/node_name | 1 + .../fchostdata/fc_host/host4/npiv_vports_inuse | 1 + .../class/fchostdata/fc_host/host4/port_name | 1 + .../class/fchostdata/fc_host/host4/port_state | 1 + .../class/fchostdata/fc_host/host4/vport_create | 0 .../class/fchostdata/fc_host/host4/vport_delete | 0 .../class/fchostdata/fc_host/host5/fabric_name | 1 + .../class/fchostdata/fc_host/host5/max_npiv_vports | 1 + .../class/fchostdata/fc_host/host5/node_name | 1 + .../fchostdata/fc_host/host5/npiv_vports_inuse | 1 + .../class/fchostdata/fc_host/host5/port_name | 1 + .../class/fchostdata/fc_host/host5/port_state | 1 + .../class/fchostdata/fc_host/host5/vport_create | 0 .../class/fchostdata/fc_host/host5/vport_delete | 0 44 files changed, 110 insertions(+), 37 deletions(-) delete mode 100644 tests/fchostdata/fc_host/host4/fabric_name delete mode 100644 tests/fchostdata/fc_host/host4/max_npiv_vports delete mode 100644 tests/fchostdata/fc_host/host4/node_name delete mode 100644 tests/fchostdata/fc_host/host4/npiv_vports_inuse delete mode 100644 tests/fchostdata/fc_host/host4/port_name delete mode 100644 tests/fchostdata/fc_host/host4/port_state delete mode 100644 tests/fchostdata/fc_host/host4/vport_create delete mode 100644 tests/fchostdata/fc_host/host4/vport_delete delete mode 100644 tests/fchostdata/fc_host/host5/fabric_name delete mode 100644 tests/fchostdata/fc_host/host5/max_npiv_vports delete mode 100644 tests/fchostdata/fc_host/host5/node_name delete mode 100644 tests/fchostdata/fc_host/host5/npiv_vports_inuse delete mode 100644 tests/fchostdata/fc_host/host5/port_name delete mode 100644 tests/fchostdata/fc_host/host5/port_state delete mode 100644 tests/fchostdata/fc_host/host5/vport_create delete mode 100644 tests/fchostdata/fc_host/host5/vport_delete create mode 100644 tests/sysfsroot/bus/scsi/devices/0:0:0:0/scsi_generic/sg0/dev create mode 100644 tests/sysfsroot/class/fchostdata/fc_host/host4/fabric_name create mode 100644 tests/sysfsroot/class/fchostdata/fc_host/host4/max_npiv_vports create mode 100644 tests/sysfsroot/class/fchostdata/fc_host/host4/node_name create mode 100644 tests/sysfsroot/class/fchostdata/fc_host/host4/npiv_vports_inuse create mode 100644 tests/sysfsroot/class/fchostdata/fc_host/host4/port_name create mode 100644 tests/sysfsroot/class/fchostdata/fc_host/host4/port_state create mode 100644 tests/sysfsroot/class/fchostdata/fc_host/host4/vport_create create mode 100644 tests/sysfsroot/class/fchostdata/fc_host/host4/vport_delete create mode 100644 tests/sysfsroot/class/fchostdata/fc_host/host5/fabric_name create mode 100644 tests/sysfsroot/class/fchostdata/fc_host/host5/max_npiv_vports create mode 100644 tests/sysfsroot/class/fchostdata/fc_host/host5/node_name create mode 100644 tests/sysfsroot/class/fchostdata/fc_host/host5/npiv_vports_inuse create mode 100644 tests/sysfsroot/class/fchostdata/fc_host/host5/port_name create mode 100644 tests/sysfsroot/class/fchostdata/fc_host/host5/port_state create mode 100644 tests/sysfsroot/class/fchostdata/fc_host/host5/vport_create create mode 100644 tests/sysfsroot/class/fchostdata/fc_host/host5/vport_delete -- 1.8.1.4
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. "tests/sysfsroot" is created to hold the sysfs related test input files. * src/qemu/qemu_command.h: (New callback struct qemuBuildCommandLineCallbacks; extern buildCommandLineCallbacks) * src/qemu/qemu_command.c: (Helper qemuSCSIDeviceGetSgName; 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 * src/util/virscsi.c: (Allow to specify the sysfs root for virSCSIDeviceGetSgName) * src/util/virscsi.h: (Change the definition of virSCSIDeviceGetSgName) * tests/qemuxml2argvtest.c: (Helper testSCSIDeviceGetSgName; callback struct testCallbacks; Use the callback struct) * src/tests/qemuxmlnstest.c :( Use the new syntax of qemuBuildCommandLine) * tests/sysfsroot/* : (New test input files) * tests/Makefile.am: (Add "sysfsroot" in EXTRA_DIST) --- src/qemu/qemu_command.c | 31 ++++++++++++++----- src/qemu/qemu_command.h | 16 ++++++++-- src/qemu/qemu_driver.c | 3 +- src/qemu/qemu_hotplug.c | 6 ++-- src/qemu/qemu_process.c | 3 +- src/util/virscsi.c | 35 ++++++++++++++++++---- src/util/virscsi.h | 3 +- tests/Makefile.am | 1 + tests/qemuxml2argvtest.c | 18 ++++++++++- tests/qemuxmlnstest.c | 3 +- .../bus/scsi/devices/0:0:0:0/scsi_generic/sg0/dev | 1 + 11 files changed, 97 insertions(+), 23 deletions(-) create mode 100644 tests/sysfsroot/bus/scsi/devices/0:0:0:0/scsi_generic/sg0/dev diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 5b95c07..b71a3b9 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,19 @@ qemuBuildGraphicsCommandLine(virQEMUDriverConfigPtr cfg, return 0; } +static char * +qemuSCSIDeviceGetSgName(const char *adapter, + unsigned int bus, + unsigned int target, + unsigned int unit) +{ + return virSCSIDeviceGetSgName(adapter, bus, target, unit, NULL); +} + +qemuBuildCommandLineCallbacks buildCommandLineCallbacks = { + .qemuGetSCSIDeviceSgName = qemuSCSIDeviceGetSgName, +}; + /* * Constructs a argv suitable for launching qemu with config defined * for a given virtual machine. @@ -6422,7 +6436,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; @@ -8243,7 +8258,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..23b6af3 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,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); /* 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); char * qemuBuildSCSIHostdevDevStr(virDomainDefPtr def, virDomainHostdevDefPtr dev, virQEMUCapsPtr qemuCaps); diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index b1630f8..cb46276 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 dbbb7bf..3b247a5 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -3638,7 +3638,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/src/util/virscsi.c b/src/util/virscsi.c index d6685fa..56a2cbf 100644 --- a/src/util/virscsi.c +++ b/src/util/virscsi.c @@ -100,26 +100,48 @@ virSCSIDeviceGetAdapterId(const char *adapter, return 0; } +static char * +virSCSIDeviceSysfsPrefix(const char *sysfs_root) +{ + char *ret = NULL; + + if (sysfs_root) { + if (virAsprintf(&ret, "%s/bus/scsi/devices", sysfs_root) < 0) { + virReportOOMError(); + return NULL; + } + } else { + if (VIR_STRDUP(ret, SYSFS_SCSI_DEVICES) < 0) + return NULL; + } + + return ret; +} + char * virSCSIDeviceGetSgName(const char *adapter, unsigned int bus, unsigned int target, - unsigned int unit) + unsigned int unit, + const char *sysfs_root) { DIR *dir = NULL; struct dirent *entry; char *path = NULL; char *sg = NULL; unsigned int adapter_id; + char *sysfs_prefix = NULL; if (virSCSIDeviceGetAdapterId(adapter, &adapter_id) < 0) return NULL; - if (virAsprintf(&path, - SYSFS_SCSI_DEVICES "/%d:%d:%d:%d/scsi_generic", - adapter_id, bus, target, unit) < 0) { - virReportOOMError(); + if (!(sysfs_prefix = virSCSIDeviceSysfsPrefix(sysfs_root))) return NULL; + + if (virAsprintf(&path, "%s/%d:%d:%d:%d/scsi_generic", + sysfs_prefix, adapter_id, bus, target, unit) < 0) { + virReportOOMError(); + goto cleanup; } if (!(dir = opendir(path))) { @@ -138,6 +160,7 @@ virSCSIDeviceGetSgName(const char *adapter, cleanup: closedir(dir); + VIR_FREE(sysfs_prefix); VIR_FREE(path); return sg; } @@ -166,7 +189,7 @@ virSCSIDeviceNew(const char *adapter, dev->unit = unit; dev->readonly = readonly; - if (!(sg = virSCSIDeviceGetSgName(adapter, bus, target, unit))) + if (!(sg = virSCSIDeviceGetSgName(adapter, bus, target, unit, NULL))) goto cleanup; if (virSCSIDeviceGetAdapterId(adapter, &dev->adapter) < 0) diff --git a/src/util/virscsi.h b/src/util/virscsi.h index 8268cdf..749e8d9 100644 --- a/src/util/virscsi.h +++ b/src/util/virscsi.h @@ -36,7 +36,8 @@ typedef virSCSIDeviceList *virSCSIDeviceListPtr; char *virSCSIDeviceGetSgName(const char *adapter, unsigned int bus, unsigned int target, - unsigned int unit); + unsigned int unit, + const char *sysfs_root); virSCSIDevicePtr virSCSIDeviceNew(const char *adapter, unsigned int bus, diff --git a/tests/Makefile.am b/tests/Makefile.am index 41c4067..a6cc5bf 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -80,6 +80,7 @@ EXTRA_DIST = \ storagevolschematest \ storagevolxml2xmlin \ storagevolxml2xmlout \ + sysfsroot \ sysinfodata \ test-lib.sh \ vmx2xmldata \ 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); +} + +static qemuBuildCommandLineCallbacks testCallbacks = { + .qemuGetSCSIDeviceSgName = testSCSIDeviceGetSgName, +}; + static int testCompareXMLToArgvFiles(const char *xml, const char *cmdline, virQEMUCapsPtr extraFlags, @@ -157,7 +172,8 @@ static int testCompareXMLToArgvFiles(const char *xml, if (!(cmd = qemuBuildCommandLine(conn, &driver, vmdef, &monitor_chr, (flags & FLAG_JSON), extraFlags, migrateFrom, migrateFd, NULL, - VIR_NETDEV_VPORT_PROFILE_OP_NO_OP))) { + VIR_NETDEV_VPORT_PROFILE_OP_NO_OP, + &testCallbacks))) { if (flags & FLAG_EXPECT_FAILURE) { ret = 0; if (virTestGetDebug() > 1) diff --git a/tests/qemuxmlnstest.c b/tests/qemuxmlnstest.c index 952b8e2..9d3d2a0 100644 --- a/tests/qemuxmlnstest.c +++ b/tests/qemuxmlnstest.c @@ -113,7 +113,8 @@ static int testCompareXMLToArgvFiles(const char *xml, if (!(cmd = qemuBuildCommandLine(conn, &driver, vmdef, &monitor_chr, json, extraFlags, migrateFrom, migrateFd, NULL, - VIR_NETDEV_VPORT_PROFILE_OP_NO_OP))) + VIR_NETDEV_VPORT_PROFILE_OP_NO_OP, + NULL))) goto fail; if (!!virGetLastError() != expectError) { diff --git a/tests/sysfsroot/bus/scsi/devices/0:0:0:0/scsi_generic/sg0/dev b/tests/sysfsroot/bus/scsi/devices/0:0:0:0/scsi_generic/sg0/dev new file mode 100644 index 0000000..992e920 --- /dev/null +++ b/tests/sysfsroot/bus/scsi/devices/0:0:0:0/scsi_generic/sg0/dev @@ -0,0 +1 @@ +21:0 -- 1.8.1.4
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 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 :|
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. Osier
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 :|
On 05/15/2013 11:25 AM, 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.
Then instead of the very specific callback function you've added, why not add a "sysfsroot" that would be accessed by any commandline-building function that needed access to sysfs? This patch is needed to fix a regression, so this may not be the best time to be discussing what is the optimum design, but we should try to keep it as simple as possible, and having a callback object that must be completely filled in by any caller seems complicated (I notice you didn't check for the possibility of the callback pointers being NULL). Has anyone enumerated exactly what are the actions currently performed inside qemuBuildCommandLine() that shouldn't be there?
On 16/05/13 01:23, Laine Stump wrote:
On 05/15/2013 11:25 AM, 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. Then instead of the very specific callback function you've added, why not add a "sysfsroot" that would be accessed by any commandline-building function that needed access to sysfs?
That's what I did in v1, or I misunderstood you (because I saw you looked at this after v1, from the time)
This patch is needed to fix a regression, so this may not be the best time to be discussing what is the optimum design, but we should try to keep it as simple as possible, and having a callback object that must be completely filled in by any caller seems complicated (I notice you didn't check for the possibility of the callback pointers being NULL).
The only place needs to check if the callback pointer is NULL or not is qemuBuildSCSIHostdevDrvStr. I can make the change if we get an agreement.
Has anyone enumerated exactly what are the actions currently performed inside qemuBuildCommandLine() that shouldn't be there?
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
"fchostdata" is also about sysfs files, it can go home now. --- tests/Makefile.am | 1 - tests/fchostdata/fc_host/host4/fabric_name | 1 - tests/fchostdata/fc_host/host4/max_npiv_vports | 1 - tests/fchostdata/fc_host/host4/node_name | 1 - tests/fchostdata/fc_host/host4/npiv_vports_inuse | 1 - tests/fchostdata/fc_host/host4/port_name | 1 - tests/fchostdata/fc_host/host4/port_state | 1 - tests/fchostdata/fc_host/host4/vport_create | 0 tests/fchostdata/fc_host/host4/vport_delete | 0 tests/fchostdata/fc_host/host5/fabric_name | 1 - tests/fchostdata/fc_host/host5/max_npiv_vports | 1 - tests/fchostdata/fc_host/host5/node_name | 1 - tests/fchostdata/fc_host/host5/npiv_vports_inuse | 1 - tests/fchostdata/fc_host/host5/port_name | 1 - tests/fchostdata/fc_host/host5/port_state | 1 - tests/fchostdata/fc_host/host5/vport_create | 0 tests/fchostdata/fc_host/host5/vport_delete | 0 tests/fchosttest.c | 2 +- tests/sysfsroot/class/fc_host/host4/fabric_name | 1 + tests/sysfsroot/class/fc_host/host4/max_npiv_vports | 1 + tests/sysfsroot/class/fc_host/host4/node_name | 1 + tests/sysfsroot/class/fc_host/host4/npiv_vports_inuse | 1 + tests/sysfsroot/class/fc_host/host4/port_name | 1 + tests/sysfsroot/class/fc_host/host4/port_state | 1 + tests/sysfsroot/class/fc_host/host4/vport_create | 0 tests/sysfsroot/class/fc_host/host4/vport_delete | 0 tests/sysfsroot/class/fc_host/host5/fabric_name | 1 + tests/sysfsroot/class/fc_host/host5/max_npiv_vports | 1 + tests/sysfsroot/class/fc_host/host5/node_name | 1 + tests/sysfsroot/class/fc_host/host5/npiv_vports_inuse | 1 + tests/sysfsroot/class/fc_host/host5/port_name | 1 + tests/sysfsroot/class/fc_host/host5/port_state | 1 + tests/sysfsroot/class/fc_host/host5/vport_create | 0 tests/sysfsroot/class/fc_host/host5/vport_delete | 0 34 files changed, 13 insertions(+), 14 deletions(-) delete mode 100644 tests/fchostdata/fc_host/host4/fabric_name delete mode 100644 tests/fchostdata/fc_host/host4/max_npiv_vports delete mode 100644 tests/fchostdata/fc_host/host4/node_name delete mode 100644 tests/fchostdata/fc_host/host4/npiv_vports_inuse delete mode 100644 tests/fchostdata/fc_host/host4/port_name delete mode 100644 tests/fchostdata/fc_host/host4/port_state delete mode 100644 tests/fchostdata/fc_host/host4/vport_create delete mode 100644 tests/fchostdata/fc_host/host4/vport_delete delete mode 100644 tests/fchostdata/fc_host/host5/fabric_name delete mode 100644 tests/fchostdata/fc_host/host5/max_npiv_vports delete mode 100644 tests/fchostdata/fc_host/host5/node_name delete mode 100644 tests/fchostdata/fc_host/host5/npiv_vports_inuse delete mode 100644 tests/fchostdata/fc_host/host5/port_name delete mode 100644 tests/fchostdata/fc_host/host5/port_state delete mode 100644 tests/fchostdata/fc_host/host5/vport_create delete mode 100644 tests/fchostdata/fc_host/host5/vport_delete create mode 100644 tests/sysfsroot/class/fc_host/host4/fabric_name create mode 100644 tests/sysfsroot/class/fc_host/host4/max_npiv_vports create mode 100644 tests/sysfsroot/class/fc_host/host4/node_name create mode 100644 tests/sysfsroot/class/fc_host/host4/npiv_vports_inuse create mode 100644 tests/sysfsroot/class/fc_host/host4/port_name create mode 100644 tests/sysfsroot/class/fc_host/host4/port_state create mode 100644 tests/sysfsroot/class/fc_host/host4/vport_create create mode 100644 tests/sysfsroot/class/fc_host/host4/vport_delete create mode 100644 tests/sysfsroot/class/fc_host/host5/fabric_name create mode 100644 tests/sysfsroot/class/fc_host/host5/max_npiv_vports create mode 100644 tests/sysfsroot/class/fc_host/host5/node_name create mode 100644 tests/sysfsroot/class/fc_host/host5/npiv_vports_inuse create mode 100644 tests/sysfsroot/class/fc_host/host5/port_name create mode 100644 tests/sysfsroot/class/fc_host/host5/port_state create mode 100644 tests/sysfsroot/class/fc_host/host5/vport_create create mode 100644 tests/sysfsroot/class/fc_host/host5/vport_delete diff --git a/tests/Makefile.am b/tests/Makefile.am index a6cc5bf..3ae9015 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -52,7 +52,6 @@ EXTRA_DIST = \ domainsnapshotschematest \ domainsnapshotxml2xmlin \ domainsnapshotxml2xmlout \ - fchostdata \ interfaceschemadata \ lxcxml2xmldata \ networkschematest \ diff --git a/tests/fchostdata/fc_host/host4/fabric_name b/tests/fchostdata/fc_host/host4/fabric_name deleted file mode 100644 index f587b68..0000000 --- a/tests/fchostdata/fc_host/host4/fabric_name +++ /dev/null @@ -1 +0,0 @@ -0xffffffffffffffff diff --git a/tests/fchostdata/fc_host/host4/max_npiv_vports b/tests/fchostdata/fc_host/host4/max_npiv_vports deleted file mode 100644 index c75acbe..0000000 --- a/tests/fchostdata/fc_host/host4/max_npiv_vports +++ /dev/null @@ -1 +0,0 @@ -127 diff --git a/tests/fchostdata/fc_host/host4/node_name b/tests/fchostdata/fc_host/host4/node_name deleted file mode 100644 index e8c1e1a..0000000 --- a/tests/fchostdata/fc_host/host4/node_name +++ /dev/null @@ -1 +0,0 @@ -0x2000001b3289da4e diff --git a/tests/fchostdata/fc_host/host4/npiv_vports_inuse b/tests/fchostdata/fc_host/host4/npiv_vports_inuse deleted file mode 100644 index 573541a..0000000 --- a/tests/fchostdata/fc_host/host4/npiv_vports_inuse +++ /dev/null @@ -1 +0,0 @@ -0 diff --git a/tests/fchostdata/fc_host/host4/port_name b/tests/fchostdata/fc_host/host4/port_name deleted file mode 100644 index ee2d399..0000000 --- a/tests/fchostdata/fc_host/host4/port_name +++ /dev/null @@ -1 +0,0 @@ -0x2100001b3289da4e diff --git a/tests/fchostdata/fc_host/host4/port_state b/tests/fchostdata/fc_host/host4/port_state deleted file mode 100644 index bd1e6d5..0000000 --- a/tests/fchostdata/fc_host/host4/port_state +++ /dev/null @@ -1 +0,0 @@ -Linkdown diff --git a/tests/fchostdata/fc_host/host4/vport_create b/tests/fchostdata/fc_host/host4/vport_create deleted file mode 100644 index e69de29..0000000 diff --git a/tests/fchostdata/fc_host/host4/vport_delete b/tests/fchostdata/fc_host/host4/vport_delete deleted file mode 100644 index e69de29..0000000 diff --git a/tests/fchostdata/fc_host/host5/fabric_name b/tests/fchostdata/fc_host/host5/fabric_name deleted file mode 100644 index 4128070..0000000 --- a/tests/fchostdata/fc_host/host5/fabric_name +++ /dev/null @@ -1 +0,0 @@ -0x2001000dec9877c1 diff --git a/tests/fchostdata/fc_host/host5/max_npiv_vports b/tests/fchostdata/fc_host/host5/max_npiv_vports deleted file mode 100644 index c75acbe..0000000 --- a/tests/fchostdata/fc_host/host5/max_npiv_vports +++ /dev/null @@ -1 +0,0 @@ -127 diff --git a/tests/fchostdata/fc_host/host5/node_name b/tests/fchostdata/fc_host/host5/node_name deleted file mode 100644 index 91a0a05..0000000 --- a/tests/fchostdata/fc_host/host5/node_name +++ /dev/null @@ -1 +0,0 @@ -0x2001001b32a9da4e diff --git a/tests/fchostdata/fc_host/host5/npiv_vports_inuse b/tests/fchostdata/fc_host/host5/npiv_vports_inuse deleted file mode 100644 index 573541a..0000000 --- a/tests/fchostdata/fc_host/host5/npiv_vports_inuse +++ /dev/null @@ -1 +0,0 @@ -0 diff --git a/tests/fchostdata/fc_host/host5/port_name b/tests/fchostdata/fc_host/host5/port_name deleted file mode 100644 index c37f618..0000000 --- a/tests/fchostdata/fc_host/host5/port_name +++ /dev/null @@ -1 +0,0 @@ -0x2101001b32a9da4e diff --git a/tests/fchostdata/fc_host/host5/port_state b/tests/fchostdata/fc_host/host5/port_state deleted file mode 100644 index b73dd46..0000000 --- a/tests/fchostdata/fc_host/host5/port_state +++ /dev/null @@ -1 +0,0 @@ -Online diff --git a/tests/fchostdata/fc_host/host5/vport_create b/tests/fchostdata/fc_host/host5/vport_create deleted file mode 100644 index e69de29..0000000 diff --git a/tests/fchostdata/fc_host/host5/vport_delete b/tests/fchostdata/fc_host/host5/vport_delete deleted file mode 100644 index e69de29..0000000 diff --git a/tests/fchosttest.c b/tests/fchosttest.c index 035f721..3d64671 100644 --- a/tests/fchosttest.c +++ b/tests/fchosttest.c @@ -22,7 +22,7 @@ #include "virutil.h" #include "testutils.h" -#define TEST_FC_HOST_PREFIX "./fchostdata/fc_host/" +#define TEST_FC_HOST_PREFIX "./sysfsroot/class/fc_host/" #define TEST_FC_HOST_NUM 5 /* Test virIsCapableFCHost */ diff --git a/tests/sysfsroot/class/fc_host/host4/fabric_name b/tests/sysfsroot/class/fc_host/host4/fabric_name new file mode 100644 index 0000000..f587b68 --- /dev/null +++ b/tests/sysfsroot/class/fc_host/host4/fabric_name @@ -0,0 +1 @@ +0xffffffffffffffff diff --git a/tests/sysfsroot/class/fc_host/host4/max_npiv_vports b/tests/sysfsroot/class/fc_host/host4/max_npiv_vports new file mode 100644 index 0000000..c75acbe --- /dev/null +++ b/tests/sysfsroot/class/fc_host/host4/max_npiv_vports @@ -0,0 +1 @@ +127 diff --git a/tests/sysfsroot/class/fc_host/host4/node_name b/tests/sysfsroot/class/fc_host/host4/node_name new file mode 100644 index 0000000..e8c1e1a --- /dev/null +++ b/tests/sysfsroot/class/fc_host/host4/node_name @@ -0,0 +1 @@ +0x2000001b3289da4e diff --git a/tests/sysfsroot/class/fc_host/host4/npiv_vports_inuse b/tests/sysfsroot/class/fc_host/host4/npiv_vports_inuse new file mode 100644 index 0000000..573541a --- /dev/null +++ b/tests/sysfsroot/class/fc_host/host4/npiv_vports_inuse @@ -0,0 +1 @@ +0 diff --git a/tests/sysfsroot/class/fc_host/host4/port_name b/tests/sysfsroot/class/fc_host/host4/port_name new file mode 100644 index 0000000..ee2d399 --- /dev/null +++ b/tests/sysfsroot/class/fc_host/host4/port_name @@ -0,0 +1 @@ +0x2100001b3289da4e diff --git a/tests/sysfsroot/class/fc_host/host4/port_state b/tests/sysfsroot/class/fc_host/host4/port_state new file mode 100644 index 0000000..bd1e6d5 --- /dev/null +++ b/tests/sysfsroot/class/fc_host/host4/port_state @@ -0,0 +1 @@ +Linkdown diff --git a/tests/sysfsroot/class/fc_host/host4/vport_create b/tests/sysfsroot/class/fc_host/host4/vport_create new file mode 100644 index 0000000..e69de29 diff --git a/tests/sysfsroot/class/fc_host/host4/vport_delete b/tests/sysfsroot/class/fc_host/host4/vport_delete new file mode 100644 index 0000000..e69de29 diff --git a/tests/sysfsroot/class/fc_host/host5/fabric_name b/tests/sysfsroot/class/fc_host/host5/fabric_name new file mode 100644 index 0000000..4128070 --- /dev/null +++ b/tests/sysfsroot/class/fc_host/host5/fabric_name @@ -0,0 +1 @@ +0x2001000dec9877c1 diff --git a/tests/sysfsroot/class/fc_host/host5/max_npiv_vports b/tests/sysfsroot/class/fc_host/host5/max_npiv_vports new file mode 100644 index 0000000..c75acbe --- /dev/null +++ b/tests/sysfsroot/class/fc_host/host5/max_npiv_vports @@ -0,0 +1 @@ +127 diff --git a/tests/sysfsroot/class/fc_host/host5/node_name b/tests/sysfsroot/class/fc_host/host5/node_name new file mode 100644 index 0000000..91a0a05 --- /dev/null +++ b/tests/sysfsroot/class/fc_host/host5/node_name @@ -0,0 +1 @@ +0x2001001b32a9da4e diff --git a/tests/sysfsroot/class/fc_host/host5/npiv_vports_inuse b/tests/sysfsroot/class/fc_host/host5/npiv_vports_inuse new file mode 100644 index 0000000..573541a --- /dev/null +++ b/tests/sysfsroot/class/fc_host/host5/npiv_vports_inuse @@ -0,0 +1 @@ +0 diff --git a/tests/sysfsroot/class/fc_host/host5/port_name b/tests/sysfsroot/class/fc_host/host5/port_name new file mode 100644 index 0000000..c37f618 --- /dev/null +++ b/tests/sysfsroot/class/fc_host/host5/port_name @@ -0,0 +1 @@ +0x2101001b32a9da4e diff --git a/tests/sysfsroot/class/fc_host/host5/port_state b/tests/sysfsroot/class/fc_host/host5/port_state new file mode 100644 index 0000000..b73dd46 --- /dev/null +++ b/tests/sysfsroot/class/fc_host/host5/port_state @@ -0,0 +1 @@ +Online diff --git a/tests/sysfsroot/class/fc_host/host5/vport_create b/tests/sysfsroot/class/fc_host/host5/vport_create new file mode 100644 index 0000000..e69de29 diff --git a/tests/sysfsroot/class/fc_host/host5/vport_delete b/tests/sysfsroot/class/fc_host/host5/vport_delete new file mode 100644 index 0000000..e69de29 -- 1.8.1.4
On Wed, May 15, 2013 at 11:25:20PM +0800, Osier Yang wrote:
"fchostdata" is also about sysfs files, it can go home now. --- tests/Makefile.am | 1 - tests/fchostdata/fc_host/host4/fabric_name | 1 - tests/fchostdata/fc_host/host4/max_npiv_vports | 1 - tests/fchostdata/fc_host/host4/node_name | 1 - tests/fchostdata/fc_host/host4/npiv_vports_inuse | 1 - tests/fchostdata/fc_host/host4/port_name | 1 - tests/fchostdata/fc_host/host4/port_state | 1 - tests/fchostdata/fc_host/host4/vport_create | 0 tests/fchostdata/fc_host/host4/vport_delete | 0 tests/fchostdata/fc_host/host5/fabric_name | 1 - tests/fchostdata/fc_host/host5/max_npiv_vports | 1 - tests/fchostdata/fc_host/host5/node_name | 1 - tests/fchostdata/fc_host/host5/npiv_vports_inuse | 1 - tests/fchostdata/fc_host/host5/port_name | 1 - tests/fchostdata/fc_host/host5/port_state | 1 - tests/fchostdata/fc_host/host5/vport_create | 0 tests/fchostdata/fc_host/host5/vport_delete | 0 tests/fchosttest.c | 2 +- tests/sysfsroot/class/fc_host/host4/fabric_name | 1 + tests/sysfsroot/class/fc_host/host4/max_npiv_vports | 1 + tests/sysfsroot/class/fc_host/host4/node_name | 1 + tests/sysfsroot/class/fc_host/host4/npiv_vports_inuse | 1 + tests/sysfsroot/class/fc_host/host4/port_name | 1 + tests/sysfsroot/class/fc_host/host4/port_state | 1 + tests/sysfsroot/class/fc_host/host4/vport_create | 0 tests/sysfsroot/class/fc_host/host4/vport_delete | 0 tests/sysfsroot/class/fc_host/host5/fabric_name | 1 + tests/sysfsroot/class/fc_host/host5/max_npiv_vports | 1 + tests/sysfsroot/class/fc_host/host5/node_name | 1 + tests/sysfsroot/class/fc_host/host5/npiv_vports_inuse | 1 + tests/sysfsroot/class/fc_host/host5/port_name | 1 + tests/sysfsroot/class/fc_host/host5/port_state | 1 + tests/sysfsroot/class/fc_host/host5/vport_create | 0 tests/sysfsroot/class/fc_host/host5/vport_delete | 0 34 files changed, 13 insertions(+), 14 deletions(-) delete mode 100644 tests/fchostdata/fc_host/host4/fabric_name delete mode 100644 tests/fchostdata/fc_host/host4/max_npiv_vports delete mode 100644 tests/fchostdata/fc_host/host4/node_name delete mode 100644 tests/fchostdata/fc_host/host4/npiv_vports_inuse delete mode 100644 tests/fchostdata/fc_host/host4/port_name delete mode 100644 tests/fchostdata/fc_host/host4/port_state delete mode 100644 tests/fchostdata/fc_host/host4/vport_create delete mode 100644 tests/fchostdata/fc_host/host4/vport_delete delete mode 100644 tests/fchostdata/fc_host/host5/fabric_name delete mode 100644 tests/fchostdata/fc_host/host5/max_npiv_vports delete mode 100644 tests/fchostdata/fc_host/host5/node_name delete mode 100644 tests/fchostdata/fc_host/host5/npiv_vports_inuse delete mode 100644 tests/fchostdata/fc_host/host5/port_name delete mode 100644 tests/fchostdata/fc_host/host5/port_state delete mode 100644 tests/fchostdata/fc_host/host5/vport_create delete mode 100644 tests/fchostdata/fc_host/host5/vport_delete create mode 100644 tests/sysfsroot/class/fc_host/host4/fabric_name create mode 100644 tests/sysfsroot/class/fc_host/host4/max_npiv_vports create mode 100644 tests/sysfsroot/class/fc_host/host4/node_name create mode 100644 tests/sysfsroot/class/fc_host/host4/npiv_vports_inuse create mode 100644 tests/sysfsroot/class/fc_host/host4/port_name create mode 100644 tests/sysfsroot/class/fc_host/host4/port_state create mode 100644 tests/sysfsroot/class/fc_host/host4/vport_create create mode 100644 tests/sysfsroot/class/fc_host/host4/vport_delete create mode 100644 tests/sysfsroot/class/fc_host/host5/fabric_name create mode 100644 tests/sysfsroot/class/fc_host/host5/max_npiv_vports create mode 100644 tests/sysfsroot/class/fc_host/host5/node_name create mode 100644 tests/sysfsroot/class/fc_host/host5/npiv_vports_inuse create mode 100644 tests/sysfsroot/class/fc_host/host5/port_name create mode 100644 tests/sysfsroot/class/fc_host/host5/port_state create mode 100644 tests/sysfsroot/class/fc_host/host5/vport_create create mode 100644 tests/sysfsroot/class/fc_host/host5/vport_delete
Not sure I really agree with this change. The 'fchostdata' files are used by the 'fchosttest.c' test program. So that existing naming is following our conventions of naming the data dir to match the test program. 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 :|
On 15/05/13 23:36, Daniel P. Berrange wrote:
On Wed, May 15, 2013 at 11:25:20PM +0800, Osier Yang wrote:
"fchostdata" is also about sysfs files, it can go home now. --- tests/Makefile.am | 1 - tests/fchostdata/fc_host/host4/fabric_name | 1 - tests/fchostdata/fc_host/host4/max_npiv_vports | 1 - tests/fchostdata/fc_host/host4/node_name | 1 - tests/fchostdata/fc_host/host4/npiv_vports_inuse | 1 - tests/fchostdata/fc_host/host4/port_name | 1 - tests/fchostdata/fc_host/host4/port_state | 1 - tests/fchostdata/fc_host/host4/vport_create | 0 tests/fchostdata/fc_host/host4/vport_delete | 0 tests/fchostdata/fc_host/host5/fabric_name | 1 - tests/fchostdata/fc_host/host5/max_npiv_vports | 1 - tests/fchostdata/fc_host/host5/node_name | 1 - tests/fchostdata/fc_host/host5/npiv_vports_inuse | 1 - tests/fchostdata/fc_host/host5/port_name | 1 - tests/fchostdata/fc_host/host5/port_state | 1 - tests/fchostdata/fc_host/host5/vport_create | 0 tests/fchostdata/fc_host/host5/vport_delete | 0 tests/fchosttest.c | 2 +- tests/sysfsroot/class/fc_host/host4/fabric_name | 1 + tests/sysfsroot/class/fc_host/host4/max_npiv_vports | 1 + tests/sysfsroot/class/fc_host/host4/node_name | 1 + tests/sysfsroot/class/fc_host/host4/npiv_vports_inuse | 1 + tests/sysfsroot/class/fc_host/host4/port_name | 1 + tests/sysfsroot/class/fc_host/host4/port_state | 1 + tests/sysfsroot/class/fc_host/host4/vport_create | 0 tests/sysfsroot/class/fc_host/host4/vport_delete | 0 tests/sysfsroot/class/fc_host/host5/fabric_name | 1 + tests/sysfsroot/class/fc_host/host5/max_npiv_vports | 1 + tests/sysfsroot/class/fc_host/host5/node_name | 1 + tests/sysfsroot/class/fc_host/host5/npiv_vports_inuse | 1 + tests/sysfsroot/class/fc_host/host5/port_name | 1 + tests/sysfsroot/class/fc_host/host5/port_state | 1 + tests/sysfsroot/class/fc_host/host5/vport_create | 0 tests/sysfsroot/class/fc_host/host5/vport_delete | 0 34 files changed, 13 insertions(+), 14 deletions(-) delete mode 100644 tests/fchostdata/fc_host/host4/fabric_name delete mode 100644 tests/fchostdata/fc_host/host4/max_npiv_vports delete mode 100644 tests/fchostdata/fc_host/host4/node_name delete mode 100644 tests/fchostdata/fc_host/host4/npiv_vports_inuse delete mode 100644 tests/fchostdata/fc_host/host4/port_name delete mode 100644 tests/fchostdata/fc_host/host4/port_state delete mode 100644 tests/fchostdata/fc_host/host4/vport_create delete mode 100644 tests/fchostdata/fc_host/host4/vport_delete delete mode 100644 tests/fchostdata/fc_host/host5/fabric_name delete mode 100644 tests/fchostdata/fc_host/host5/max_npiv_vports delete mode 100644 tests/fchostdata/fc_host/host5/node_name delete mode 100644 tests/fchostdata/fc_host/host5/npiv_vports_inuse delete mode 100644 tests/fchostdata/fc_host/host5/port_name delete mode 100644 tests/fchostdata/fc_host/host5/port_state delete mode 100644 tests/fchostdata/fc_host/host5/vport_create delete mode 100644 tests/fchostdata/fc_host/host5/vport_delete create mode 100644 tests/sysfsroot/class/fc_host/host4/fabric_name create mode 100644 tests/sysfsroot/class/fc_host/host4/max_npiv_vports create mode 100644 tests/sysfsroot/class/fc_host/host4/node_name create mode 100644 tests/sysfsroot/class/fc_host/host4/npiv_vports_inuse create mode 100644 tests/sysfsroot/class/fc_host/host4/port_name create mode 100644 tests/sysfsroot/class/fc_host/host4/port_state create mode 100644 tests/sysfsroot/class/fc_host/host4/vport_create create mode 100644 tests/sysfsroot/class/fc_host/host4/vport_delete create mode 100644 tests/sysfsroot/class/fc_host/host5/fabric_name create mode 100644 tests/sysfsroot/class/fc_host/host5/max_npiv_vports create mode 100644 tests/sysfsroot/class/fc_host/host5/node_name create mode 100644 tests/sysfsroot/class/fc_host/host5/npiv_vports_inuse create mode 100644 tests/sysfsroot/class/fc_host/host5/port_name create mode 100644 tests/sysfsroot/class/fc_host/host5/port_state create mode 100644 tests/sysfsroot/class/fc_host/host5/vport_create create mode 100644 tests/sysfsroot/class/fc_host/host5/vport_delete
Not sure I really agree with this change. The 'fchostdata' files are used by the 'fchosttest.c' test program. So that existing naming is following our conventions of naming the data dir to match the test program.
My intention was to put all the sysfs test input files together in a directory, and keep the tree similar with real "sysfs". IMHO it's better than having different sysfs test input files here and there, and orgnize the tree of each of them in different manner? Osier
On Thu, May 16, 2013 at 12:37:23AM +0800, Osier Yang wrote:
On 15/05/13 23:36, Daniel P. Berrange wrote:
On Wed, May 15, 2013 at 11:25:20PM +0800, Osier Yang wrote:
"fchostdata" is also about sysfs files, it can go home now. ---
Not sure I really agree with this change. The 'fchostdata' files are used by the 'fchosttest.c' test program. So that existing naming is following our conventions of naming the data dir to match the test program.
My intention was to put all the sysfs test input files together in a directory, and keep the tree similar with real "sysfs". IMHO it's better than having different sysfs test input files here and there, and orgnize the tree of each of them in different manner?
The different trees of files are specific to each particular test case though. Unless there are multiple test cases which are all using the same file, they should be named to match the test case 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 :|
Hi Osier, On Thu, May 16, 2013 at 12:37:23AM +0800, Osier Yang wrote: [..snip..]
Not sure I really agree with this change. The 'fchostdata' files are used by the 'fchosttest.c' test program. So that existing naming is following our conventions of naming the data dir to match the test program.
My intention was to put all the sysfs test input files together in a directory, and keep the tree similar with real "sysfs". IMHO it's better than having different sysfs test input files here and there, and orgnize the tree of each of them in different manner?
I think there's pros and cons for both. With different minimal sysfs trees we'd have a minimal test fixture so we can be sure other test data doesn't influence the outcome. So I kind of agree with Daniel's PoV here. -- Guido
participants (4)
-
Daniel P. Berrange -
Guido Günther -
Laine Stump -
Osier Yang