New subject: [libvirt] [PATCH 01/15] conf: nodedev: Rename virNodeDevObjHasCap to virNodeDevObjHasCapStr

25 Jan 2018

> -----Original Message-----
> From: libvir-list-bounces@redhat.com [mailto:libvir-list-
> bounces@redhat.com] On Behalf Of Erik Skultety
> Sent: Thursday, January 25, 2018 5:24 PM
> To: libvir-list@redhat.com
> Cc: Erik Skultety <eskultet@redhat.com>
> Subject: [libvirt] [PATCH 07/15] nodedev: Introduce
> virNodeDeviceCapsListExport
> 
> Whether asking for a number of capabilities supported by a device or
> listing them, it's handled essentially by a copy-paste code, so extract
> the common stuff into this new helper which also updates all capabilities
> just before touching them.
> 
> Signed-off-by: Erik Skultety <eskultet@redhat.com>
> ---
>  src/conf/node_device_conf.c          | 73
> +++++++++++++++++++++++++++++++++++
>  src/conf/node_device_conf.h          |  5 +++
>  src/libvirt_private.syms             |  1 +
>  src/node_device/node_device_driver.c | 75 +++++++++----------------------
> -----
>  4 files changed, 97 insertions(+), 57 deletions(-)
> 
> diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c
> index 217673a56..9467bb415 100644
> --- a/src/conf/node_device_conf.c
> +++ b/src/conf/node_device_conf.c
> @@ -2487,6 +2487,79 @@ virNodeDeviceUpdateCaps(virNodeDeviceDefPtr def)  }
> 
> 
> +/**
> + * virNodeDeviceCapsListExport:
> + * @def: node device definition
> + * @list: pointer to an array to store all supported capabilities by a
> +device
> + *
> + * Takes the definition, scans through all the capabilities that the
> +device
> + * supports (including the nested caps) and populates a newly allocated
> +list
> + * with them. Caller is responsible for freeing the list.
> + * If NULL is passed to @list, only the number of caps will be returned.
> + *
> + * Returns the number of capabilities the device supports, -1 on error.
> + */
> +int
> +virNodeDeviceCapsListExport(virNodeDeviceDefPtr def,
> +                            virNodeDevCapType **list) {
> +    virNodeDevCapsDefPtr caps = NULL;
> +    virNodeDevCapType *tmp = NULL;
> +    bool want_list = !!list;
> +    int ncaps = 0;
> +    int ret = -1;
> +
> +#define MAYBE_ADD_CAP(cap) \
> +    do { \
> +        if (want_list) \
> +            tmp[ncaps] = cap; \
> +    } while (0)
> +
> +    if (want_list && VIR_ALLOC_N(tmp, VIR_NODE_DEV_CAP_LAST - 1) < 0)
> +        goto cleanup;
> +
> +    for (caps = def->caps; caps; caps = caps->next) {
> +        unsigned int flags;
> +
> +        MAYBE_ADD_CAP(caps->data.type);
> +        ncaps++;
> +
> +        /* check nested caps for a given type as well */
> +        if (caps->data.type == VIR_NODE_DEV_CAP_SCSI_HOST) {
> +            flags = caps->data.scsi_host.flags;
> +
> +            if (flags & VIR_NODE_DEV_CAP_FLAG_HBA_FC_HOST) {
> +                MAYBE_ADD_CAP(VIR_NODE_DEV_CAP_FC_HOST);
> +                ncaps++;
> +            }
> +
> +            if (flags  & VIR_NODE_DEV_CAP_FLAG_HBA_VPORT_OPS) {
> +                MAYBE_ADD_CAP(VIR_NODE_DEV_CAP_VPORTS);
> +                ncaps++;
> +            }
> +        }
> +
> +        if (caps->data.type == VIR_NODE_DEV_CAP_PCI_DEV) {
> +            flags = caps->data.pci_dev.flags;
> +
> +            if (flags & VIR_NODE_DEV_CAP_FLAG_PCI_MDEV) {
> +                MAYBE_ADD_CAP(VIR_NODE_DEV_CAP_MDEV_TYPES);
> +                ncaps++;
> +            }
> +        }
> +    }
> +
> +#undef MAYBE_ADD_CAP
> +
> +    if (want_list)
> +        VIR_STEAL_PTR(*list, tmp);
> +    ret = ncaps;
> + cleanup:
> +    VIR_FREE(tmp);
> +    return ret;
> +}
> +
> +
>  #ifdef __linux__
> 
>  int
> diff --git a/src/conf/node_device_conf.h b/src/conf/node_device_conf.h
> index 7e32f5c05..53cdfdb01 100644
> --- a/src/conf/node_device_conf.h
> +++ b/src/conf/node_device_conf.h
> @@ -403,4 +403,9 @@ virNodeDeviceGetPCIDynamicCaps(const char *sysfsPath,
> 
>  int
>  virNodeDeviceUpdateCaps(virNodeDeviceDefPtr def);
> +
> +int
> +virNodeDeviceCapsListExport(virNodeDeviceDefPtr def,
> +                            virNodeDevCapType **list);
> +
>  #endif /* __VIR_NODE_DEVICE_CONF_H__ */ diff --git
> a/src/libvirt_private.syms b/src/libvirt_private.syms index
> 6098cf121..1698e6227 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -699,6 +699,7 @@ virNodeDevCapMdevTypeFree;  virNodeDevCapsDefFree;
> virNodeDevCapTypeFromString;  virNodeDevCapTypeToString;
> +virNodeDeviceCapsListExport;
>  virNodeDeviceCreateVport;
>  virNodeDeviceDefFormat;
>  virNodeDeviceDefFree;
> diff --git a/src/node_device/node_device_driver.c
> b/src/node_device/node_device_driver.c
> index 48f45474c..8fb08742b 100644
> --- a/src/node_device/node_device_driver.c
> +++ b/src/node_device/node_device_driver.c
> @@ -306,8 +306,6 @@ nodeDeviceNumOfCaps(virNodeDevicePtr device)  {
>      virNodeDeviceObjPtr obj;
>      virNodeDeviceDefPtr def;
> -    virNodeDevCapsDefPtr caps;
> -    int ncaps = 0;
>      int ret = -1;
> 
>      if (!(obj = nodeDeviceObjFindByName(device->name)))
> @@ -317,27 +315,7 @@ nodeDeviceNumOfCaps(virNodeDevicePtr device)
>      if (virNodeDeviceNumOfCapsEnsureACL(device->conn, def) < 0)
>          goto cleanup;
> 
> -    for (caps = def->caps; caps; caps = caps->next) {
> -        ++ncaps;
> -
> -        if (caps->data.type == VIR_NODE_DEV_CAP_SCSI_HOST) {
> -            if (caps->data.scsi_host.flags &
> -                VIR_NODE_DEV_CAP_FLAG_HBA_FC_HOST)
> -                ncaps++;
> -
> -            if (caps->data.scsi_host.flags &
> -                VIR_NODE_DEV_CAP_FLAG_HBA_VPORT_OPS)
> -                ncaps++;
> -        }
> -        if (caps->data.type == VIR_NODE_DEV_CAP_PCI_DEV) {
> -            if (caps->data.pci_dev.flags &
> -                VIR_NODE_DEV_CAP_FLAG_PCI_MDEV)
> -                ncaps++;
> -        }
> -
> -    }
> -
> -    ret = ncaps;
> +    ret = virNodeDeviceCapsListExport(def, NULL);
> 
>   cleanup:
>      virNodeDeviceObjEndAPI(&obj);
> @@ -353,9 +331,10 @@ nodeDeviceListCaps(virNodeDevicePtr device,  {
>      virNodeDeviceObjPtr obj;
>      virNodeDeviceDefPtr def;
> -    virNodeDevCapsDefPtr caps;
> +    virNodeDevCapType *list = NULL;
>      int ncaps = 0;
>      int ret = -1;
> +    size_t i;

variable i should be initialized. Or the act is uncertain when cleanup.
Except this, this series of patches work well for me.

> 
>      if (!(obj = nodeDeviceObjFindByName(device->name)))
>          return -1;
> @@ -364,46 +343,28 @@ nodeDeviceListCaps(virNodeDevicePtr device,
>      if (virNodeDeviceListCapsEnsureACL(device->conn, def) < 0)
>          goto cleanup;
> 
> -    for (caps = def->caps; caps && ncaps < maxnames; caps = caps->next) {
> -        if (VIR_STRDUP(names[ncaps++], virNodeDevCapTypeToString(caps-
> >data.type)) < 0)
> +    if ((ncaps = virNodeDeviceCapsListExport(def, &list)) < 0)
> +        goto cleanup;
> +
> +    if (ncaps > maxnames)
> +        ncaps = maxnames;
> +
> +    for (i = 0; i < ncaps; i++) {
> +        if (VIR_STRDUP(names[i], virNodeDevCapTypeToString(list[i])) <
> + 0)
>              goto cleanup;
> -
> -        if (caps->data.type == VIR_NODE_DEV_CAP_SCSI_HOST) {
> -            if (ncaps < maxnames &&
> -                caps->data.scsi_host.flags &
> -                VIR_NODE_DEV_CAP_FLAG_HBA_FC_HOST) {
> -                if (VIR_STRDUP(names[ncaps++],
> -
> virNodeDevCapTypeToString(VIR_NODE_DEV_CAP_FC_HOST)) < 0)
> -                    goto cleanup;
> -            }
> -
> -            if (ncaps < maxnames &&
> -                caps->data.scsi_host.flags &
> -                VIR_NODE_DEV_CAP_FLAG_HBA_VPORT_OPS) {
> -                if (VIR_STRDUP(names[ncaps++],
> -
> virNodeDevCapTypeToString(VIR_NODE_DEV_CAP_VPORTS)) < 0)
> -                    goto cleanup;
> -            }
> -        }
> -        if (caps->data.type == VIR_NODE_DEV_CAP_PCI_DEV) {
> -            if (ncaps < maxnames &&
> -                caps->data.pci_dev.flags &
> -                VIR_NODE_DEV_CAP_FLAG_PCI_MDEV) {
> -                if (VIR_STRDUP(names[ncaps++],
> -
> virNodeDevCapTypeToString(VIR_NODE_DEV_CAP_MDEV_TYPES)) < 0)
> -                    goto cleanup;
> -            }
> -        }
>      }
> +
>      ret = ncaps;
> 
>   cleanup:
>      virNodeDeviceObjEndAPI(&obj);
> -    if (ret == -1) {
> -        --ncaps;
> -        while (--ncaps >= 0)
> -            VIR_FREE(names[ncaps]);
> +    if (ret < 0) {
> +        size_t j;
> +        for (j = 0; j < i; j++)
> +            VIR_FREE(names[j]);
>      }
> +
> +    VIR_FREE(list);
>      return ret;
>  }
> 
> --
> 2.13.6
> 
> --
> libvir-list mailing list
> libvir-list@redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH 00/15] Nodedev-mdev overhaul

Wuzongyong (Euler Dept)

tags

participants (3)