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

[John Ferlan]

Ping?

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
>