Re: [PATCH v5 21/23] xl / libxl: support naming of assignable devices

[Wei Liu <wl@xxxxxxx>]

On Thu, Dec 03, 2020 at 02:25:32PM +0000, Paul Durrant wrote:
> From: Paul Durrant <pdurrant@xxxxxxxxxx>
> 
> This patch modifies libxl_device_pci_assignable_add() to take an optional
> 'name' argument, which (if supplied) is saved into xenstore and can hence be
> used to refer to the now-assignable BDF in subsequent operations. To
> facilitate this, a new libxl_device_pci_assignable_name2bdf() function is
> added.
> 
> The xl code is modified to allow a name to be specified in the
> 'pci-assignable-add' operation and also allow an option to be specified to
> 'pci-assignable-list' requesting that names be displayed. The latter is
> facilitated by a new libxl_device_pci_assignable_bdf2name() function. Finally
> xl 'pci-assignable-remove' is modified to that either a name or BDF can be
> supplied. The supplied 'identifier' is first assumed to be a name, but if
> libxl_device_pci_assignable_name2bdf() fails to find a matching BDF the
> identifier itself will be parsed as a BDF. Names my only include printable
> characters and may not include whitespace.
> 
> Signed-off-by: Paul Durrant <pdurrant@xxxxxxxxxx>
> Acked-by: Christian Lindig <christian.lindig@xxxxxxxxxx>
> ---
> Cc: Ian Jackson <iwj@xxxxxxxxxxxxxx>
> Cc: Wei Liu <wl@xxxxxxx>
> Cc: David Scott <dave@xxxxxxxxxx>
> Cc: Anthony PERARD <anthony.perard@xxxxxxxxxx>
> 
> v4:
>  - Fix unitialized return value in libxl_device_pci_assignable_name2bdf()
>    that was discovered in CI
> ---
>  tools/include/libxl.h                | 19 +++++-
>  tools/libs/light/libxl_pci.c         | 86 ++++++++++++++++++++++++++--
>  tools/ocaml/libs/xl/xenlight_stubs.c |  3 +-
>  tools/xl/xl_cmdtable.c               | 12 ++--
>  tools/xl/xl_pci.c                    | 80 ++++++++++++++++++--------
>  5 files changed, 164 insertions(+), 36 deletions(-)
> 
> diff --git a/tools/include/libxl.h b/tools/include/libxl.h
> index 5703fdf367c5..4025d3a3d437 100644
> --- a/tools/include/libxl.h
> +++ b/tools/include/libxl.h
> @@ -476,6 +476,14 @@
>   */
>  #define LIBXL_HAVE_PCI_ASSIGNABLE_BDF 1
>  
> +/*
> + * LIBXL_HAVE_PCI_ASSIGNABLE_NAME indicates that the
> + * libxl_device_pci_assignable_add() function takes a 'name' argument
> + * and that the libxl_device_pci_assignable_name2bdf() and
> + * libxl_device_pci_assignable_bdf2name() functions are defined.
> + */
> +#define LIBXL_HAVE_PCI_ASSIGNABLE_NAME 1
> +
>  /*
>   * libxl ABI compatibility
>   *
> @@ -2385,11 +2393,18 @@ int libxl_device_events_handler(libxl_ctx *ctx,
>   * added or is not bound, the functions will emit a warning but return
>   * SUCCESS.
>   */
> -int libxl_device_pci_assignable_add(libxl_ctx *ctx, libxl_pci_bdf *pcibdf, 
> int rebind);
> -int libxl_device_pci_assignable_remove(libxl_ctx *ctx, libxl_pci_bdf 
> *pcibdf, int rebind);
> +int libxl_device_pci_assignable_add(libxl_ctx *ctx, libxl_pci_bdf *pcibdf,
> +                                    const char *name, int rebind);
> +int libxl_device_pci_assignable_remove(libxl_ctx *ctx, libxl_pci_bdf *pcibdf,
> +                                       int rebind);
>  libxl_pci_bdf *libxl_device_pci_assignable_list(libxl_ctx *ctx, int *num);
>  void libxl_device_pci_assignable_list_free(libxl_pci_bdf *list, int num);
>  
> +libxl_pci_bdf *libxl_device_pci_assignable_name2bdf(libxl_ctx *ctx,
> +                                                    const char *name);
> +char *libxl_device_pci_assignable_bdf2name(libxl_ctx *ctx,
> +                                           libxl_pci_bdf *pcibdf);

Again, these function require shims to be backward compatible.

Wei.