Re: [PATCH v4 01/23] xl / libxl: s/pcidev/pci and remove DEFINE_DEVICE_TYPE_STRUCT_X

[Oleksandr Andrushchenko <Oleksandr_Andrushchenko@xxxxxxxx>]

Hi, Paul!

On 11/24/20 10:01 AM, Paul Durrant wrote:
> From: Paul Durrant <pdurrant@xxxxxxxxxx>
>
> The seemingly arbitrary use of 'pci' and 'pcidev' in the code in libxl_pci.c
> is confusing and also compromises use of some macros used for other device
> types. Indeed it seems that DEFINE_DEVICE_TYPE_STRUCT_X exists solely because
> of this duality.
>
> This patch purges use of 'pcidev' from the libxl code, allowing evaluation of
> DEFINE_DEVICE_TYPE_STRUCT_X to be replaced with DEFINE_DEVICE_TYPE_STRUCT,
> hence allowing removal of the former.
>
> For consistency the xl and libs/util code is also modified, but in this case
> it is purely cosmetic.
>
> NOTE: Some of the more gross formatting errors (such as lack of spaces after
>        keywords) that came into context have been fixed in libxl_pci.c.
>
> Signed-off-by: Paul Durrant <pdurrant@xxxxxxxxxx>
> ---
> Cc: Ian Jackson <iwj@xxxxxxxxxxxxxx>
> Cc: Wei Liu <wl@xxxxxxx>
> Cc: Anthony PERARD <anthony.perard@xxxxxxxxxx>
> ---
>   tools/include/libxl.h             |  17 +-
>   tools/libs/light/libxl_create.c   |   6 +-
>   tools/libs/light/libxl_dm.c       |  18 +-
>   tools/libs/light/libxl_internal.h |  45 ++-
>   tools/libs/light/libxl_pci.c      | 582 
> +++++++++++++++++++-------------------
>   tools/libs/light/libxl_types.idl  |   2 +-
>   tools/libs/util/libxlu_pci.c      |  36 +--
>   tools/xl/xl_parse.c               |  28 +-
>   tools/xl/xl_pci.c                 |  68 ++---
>   tools/xl/xl_sxp.c                 |  12 +-
>   10 files changed, 409 insertions(+), 405 deletions(-)
>
> diff --git a/tools/include/libxl.h b/tools/include/libxl.h
> index 1ea5b4f446..fbe4c81ba5 100644
> --- a/tools/include/libxl.h
> +++ b/tools/include/libxl.h
> @@ -445,6 +445,13 @@
>   #define LIBXL_HAVE_DISK_SAFE_REMOVE 1
>   
[snip]
> -/* Scan through /sys/.../pciback/slots looking for pcidev's BDF */
> -static int pciback_dev_has_slot(libxl__gc *gc, libxl_device_pci *pcidev)
> +/* Scan through /sys/.../pciback/slots looking for pci's BDF */
> +static int pciback_dev_has_slot(libxl__gc *gc, libxl_device_pci *pci)
>   {
>       FILE *f;
>       int rc = 0;
> @@ -635,11 +635,11 @@ static int pciback_dev_has_slot(libxl__gc *gc, 
> libxl_device_pci *pcidev)
>           return ERROR_FAIL;
>       }
>   
> -    while(fscanf(f, "%x:%x:%x.%d\n", &dom, &bus, &dev, &func)==4) {
> -        if(dom == pcidev->domain
> -           && bus == pcidev->bus
> -           && dev == pcidev->dev
> -           && func == pcidev->func) {
> +    while (fscanf(f, "%x:%x:%x.%d\n", &dom, &bus, &dev, &func)==4) {
So, then you can probably put spaces around "4" if touching this line
> +        if (dom == pci->domain
> +            && bus == pci->bus
> +            && dev == pci->dev
> +            && func == pci->func) {
>               rc = 1;
>               goto out;
>           }
> @@ -649,7 +649,7 @@ out:
>       return rc;
>   }
>   

Reviewed-by: Oleksandr Andrushchenko <oleksandr_andrushchenko@xxxxxxxx>

Thank you,

Oleksandr