Re: [libvirt] [PATCH 0/8] storage_scsi: Stable SCSI host addressing

Just over a year ago, Osier Yang submitted some patches to provide stable SCSI host addressing support: http://www.redhat.com/archives/libvir-list/2013-June/msg00396.html Although reviewed - none of the patches were ever pushed and Osier never got back to the work. I eventually inherited the changes and now after languishing in the todo list - I took the time to rework things. There is a bz associated: https://bugzilla.redhat.com/show_bug.cgi?id=963817 Prior to leaving Red Hat, Osier and I spent some time revisiting the patch series and hashing out new/different ideas to come up with the same answer. My focus was to avoid the "generic recursive directory search" (patch 3), to not require having to know whether the scsi_host was using udev or hal, and to simplify as much as possible using existing data/information. The generic directory search I believe was less generic than intended and over complicated things. The old patches essentially used the output of a 'virsh nodedev-list scsi_host' and then selecting a resulting entry ran a 'virsh nodedev-dumpxml' to grab/use the resulting <parent> value. Taking that value and optionally adding a unique_id value was the basis for the design that would generate a PCI address (either in udev or hal format) and recursively search through /sys/devices looking for a matching address in either udev or hal format. These patches replace the bulk of the directory traversal logic with a more direct (and already in use) approach to scan the /sys/class/scsi_host directories looking for a matching PCI address found in the symlink of the files in the directory. The changed logic will add a new XML element 'parentaddr' to describe the scsi_host by it's PCI address. Additionally, the 'unique_id' has become a required attribute. The code reuses the virDevicePCIAddressParseXML() in order parse the required 'address' element. The 'address' will be in the expect PCI Address format like other described host devices. In order to help view the required unique_id value, the nodedev-dumpxml output has been adjusted to provide the <unique_id> value. This value will be an optional value on input. The documentation is updated to describe how to generate the address from the nodedev-dumpxml output. The new scsihosttest creates the expected PCI infrastructure on the fly since adding files with colons (':') is prohibited. There are multiple directories and hosts within each to ensure the search logic worked as expected. Reviewer notes: Patch 1 is new - it's just forcing the specific adapter.type checking Patches 2 & 3 are the former patches 1 & 2 with some edits to adjust for new XML syntax (and changed associated structure) Patch 4 is new - it's just utilizing the 'LINUX_SYSFS_SCSI_HOST_PREFIX' definition rather than using the hardcoded value Patches 5 & 6 are new. They handle the optional unique_id value (including using the new virNodeDevCapsDefParseIntOptional()) Patch 7 is similar to the former patch 5 insomuch as it's where the comparison of the PCI directory path and unique_id file is made. Patch 8 is similar to the former patch 7 insomuch as it's where the link is made between the scsi_host host# and the loading/refreshing of the scsi_host adapter. Former patches 3, 4, 6, and 8-11 are no longer used Please check my XML schema (patch 3) - I think I figured out the right syntax to use regarding using either "<name>" or "<parentaddr>" where "<parentaddr>" is an element. The syntax passes the make check, but it's not an area of expertise for me. Using parentaddr was preferred over overloading the parent attribute. John Ferlan (6): getAdapterName: check for SCSI_HOST scsi_backend: Use existing LINUX_SYSFS_SCSI_HOST_PREFIX definition virutil: Introduce virReadSCSIUniqueId Add unique_id to nodedev output scsi_host: Introduce virFindSCSIHostByPCI getAdapterName: Lookup stable scsi_host Osier Yang (2): virStoragePoolSourceAdapter: Refine the SCSI_HOST adapter name storage: Introduce parentaddr into virStoragePoolSourceAdapter docs/formatnode.html.in | 11 + docs/formatstorage.html.in | 130 +++++++-- docs/schemas/basictypes.rng | 24 +- docs/schemas/nodedev.rng | 6 + src/conf/node_device_conf.c | 23 +- src/conf/node_device_conf.h | 1 + src/conf/storage_conf.c | 111 +++++++- src/conf/storage_conf.h | 8 +- src/libvirt_private.syms | 2 + src/node_device/node_device_linux_sysfs.c | 6 + src/phyp/phyp_driver.c | 8 +- src/storage/storage_backend_scsi.c | 53 +++- src/test/test_driver.c | 5 +- src/util/virutil.c | 154 +++++++++++ src/util/virutil.h | 8 + tests/Makefile.am | 7 + .../pci_8086_27c5_scsi_host_0_unique_id.xml | 8 + tests/nodedevxml2xmltest.c | 1 + tests/scsihosttest.c | 308 +++++++++++++++++++++ .../pool-scsi-type-scsi-host-stable.xml | 19 ++ .../pool-scsi-type-scsi-host-stable.xml | 22 ++ tests/storagepoolxml2xmltest.c | 1 + 22 files changed, 855 insertions(+), 61 deletions(-) create mode 100644 tests/nodedevschemadata/pci_8086_27c5_scsi_host_0_unique_id.xml create mode 100644 tests/scsihosttest.c create mode 100644 tests/storagepoolxml2xmlin/pool-scsi-type-scsi-host-stable.xml create mode 100644 tests/storagepoolxml2xmlout/pool-scsi-type-scsi-host-stable.xml