[libvirt] [PATCH 05/15] nodedev: Move the sysfs-related cap handling to node_device_conf.c

[Michal Privoznik]

On 01/25/2018 10:23 AM, Erik Skultety wrote:
> The capabilities are defined/parsed/formatted/queried from this module,
> no reason for 'update' not being part of the module as well. This also
> involves some module-specific prefix changes.
> This patch also drops the node_device_linux_sysfs module from the repo
> since:
> a) it only contained the capability handlers we just moved
> b) it's only linked with the driver (by design) and thus unreachable to
> other modules
> c) we touch sysfs across all the src/util modules so the module being
> deleted hasn't been serving its original intention for some time already.
> 
> Signed-off-by: Erik Skultety <eskultet at redhat.com>
> ---
>  src/Makefile.am                           |   4 +-
>  src/conf/node_device_conf.c               | 154 +++++++++++++++++++++-
>  src/conf/node_device_conf.h               |   7 +
>  src/libvirt_private.syms                  |   2 +
>  src/node_device/node_device_driver.c      |   7 +-
>  src/node_device/node_device_hal.c         |   1 -
>  src/node_device/node_device_linux_sysfs.c | 206 ------------------------------
>  src/node_device/node_device_linux_sysfs.h |  33 -----
>  src/node_device/node_device_udev.c        |   5 +-
>  9 files changed, 168 insertions(+), 251 deletions(-)
>  delete mode 100644 src/node_device/node_device_linux_sysfs.c
>  delete mode 100644 src/node_device/node_device_linux_sysfs.h
> 
> diff --git a/src/Makefile.am b/src/Makefile.am
> index 166c9a8e9..b8ecd8df8 100644
> --- a/src/Makefile.am
> +++ b/src/Makefile.am
> @@ -1168,9 +1168,7 @@ ACCESS_DRIVER_POLKIT_POLICY = \
>  
>  NODE_DEVICE_DRIVER_SOURCES = \
>  		node_device/node_device_driver.c \
> -		node_device/node_device_driver.h \
> -		node_device/node_device_linux_sysfs.c \
> -		node_device/node_device_linux_sysfs.h
> +		node_device/node_device_driver.h
>  
>  NODE_DEVICE_DRIVER_HAL_SOURCES = \
>  		node_device/node_device_hal.c \
> diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c
> index 70a753ebf..5b0af559a 100644
> --- a/src/conf/node_device_conf.c
> +++ b/src/conf/node_device_conf.c


>  int
> -virNodeDeviceGetSCSIHostCaps(virNodeDevCap)
> +virNodeDeviceGetPCIDynamicCaps(const char *sysfsPath,
> +                               virNodeDevCapPCIDevPtr pci_dev)
> +{

ATTRIBUTE_UNUSED to both. Also, it seems interesting that for rename of
SCSI version of the function you have a separate patch but for PCI you
do it in one run. I share your feelings towards PCI, SCSI is so much
better bus and thus deserves its own patch :-)

> +    return -1;
> +}
> +
> +
> +int virNodeDeviceGetSCSITargetCaps(const char *sysfsPath ATTRIBUTE_UNUSED,
> +                                   virNodeDevCapSCSITargetPtr scsi_target ATTRIBUTE_UNUSED)
>  {
>      return -1;
>  }
> diff --git a/src/conf/node_device_conf.h b/src/conf/node_device_conf.h
> index cf79773aa..4e3154875 100644
> --- a/src/conf/node_device_conf.h
> +++ b/src/conf/node_device_conf.h
> @@ -393,4 +393,11 @@ virNodeDeviceDeleteVport(virConnectPtr conn,
>  int
>  virNodeDeviceGetSCSIHostCaps(virNodeDevCapSCSIHostPtr scsi_host);
>  
> +int
> +virNodeDeviceGetSCSITargetCaps(const char *sysfsPath,
> +                               virNodeDevCapSCSITargetPtr scsi_target);
> +
> +int
> +virNodeDeviceGetPCIDynamicCaps(const char *sysfsPath,
> +                               virNodeDevCapPCIDevPtr pci_dev);
>  #endif /* __VIR_NODE_DEVICE_CONF_H__ */
> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index bc8cc1fba..0cd8086a6 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -707,7 +707,9 @@ virNodeDeviceDefParseNode;
>  virNodeDeviceDefParseString;
>  virNodeDeviceDeleteVport;
>  virNodeDeviceGetParentName;
> +virNodeDeviceGetPCIDynamicCaps;
>  virNodeDeviceGetSCSIHostCaps;
> +virNodeDeviceGetSCSITargetCaps;
>  virNodeDeviceGetWWNs;
>  
>  
> diff --git a/src/node_device/node_device_driver.c b/src/node_device/node_device_driver.c
> index a2f687942..2e42d3527 100644
> --- a/src/node_device/node_device_driver.c
> +++ b/src/node_device/node_device_driver.c
> @@ -38,7 +38,6 @@
>  #include "node_device_event.h"
>  #include "node_device_driver.h"
>  #include "node_device_hal.h"
> -#include "node_device_linux_sysfs.h"
>  #include "virvhba.h"
>  #include "viraccessapicheck.h"
>  #include "virnetdev.h"
> @@ -59,7 +58,7 @@ nodeDeviceUpdateCaps(virNodeDeviceDefPtr def)
>              virNodeDeviceGetSCSIHostCaps(&cap->data.scsi_host);
>              break;
>          case VIR_NODE_DEV_CAP_SCSI_TARGET:
> -            nodeDeviceSysfsGetSCSITargetCaps(def->sysfs_path,
> +            virNodeDeviceGetSCSITargetCaps(def->sysfs_path,
>                                               &cap->data.scsi_target);
>              break;
>          case VIR_NODE_DEV_CAP_NET:
> @@ -70,8 +69,8 @@ nodeDeviceUpdateCaps(virNodeDeviceDefPtr def)
>                  return -1;
>              break;
>          case VIR_NODE_DEV_CAP_PCI_DEV:
> -           if (nodeDeviceSysfsGetPCIRelatedDevCaps(def->sysfs_path,
> -                                                   &cap->data.pci_dev) < 0)
> +            if (virNodeDeviceGetPCIDynamicCaps(def->sysfs_path,
> +                                               &cap->data.pci_dev) < 0)
>                return -1;
>             break;
>  
> diff --git a/src/node_device/node_device_hal.c b/src/node_device/node_device_hal.c
> index 4c50f4613..f98fd0856 100644
> --- a/src/node_device/node_device_hal.c
> +++ b/src/node_device/node_device_hal.c
> @@ -31,7 +31,6 @@
>  #include "node_device_conf.h"
>  #include "node_device_driver.h"
>  #include "node_device_hal.h"
> -#include "node_device_linux_sysfs.h"
>  #include "virerror.h"
>  #include "driver.h"
>  #include "datatypes.h"

Ah, this is the place for the hunk I'm mentioning in the previous patch.

ACK if you fix it.

Michal