Any chance this series gets reviewed before freeze? Or should I just abandon it (and the close the associated bz)? Tks, John On 06/10/2014 03:03 PM, John Ferlan wrote: > 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 > -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list