Re: [PATCH v3 7/7] libxl: Add interface for querying hypervisor about PCI topology

[Ian Campbell <ian.campbell@citrix.com>]

On Mon, 2015-02-09 at 15:04 -0500, Boris Ostrovsky wrote:
> diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
> index 9c1c949..c3b6c8a 100644
> --- a/tools/libxl/libxl.c
> +++ b/tools/libxl/libxl.c
> @@ -5107,6 +5107,55 @@ libxl_cputopology *libxl_get_cpu_topology(libxl_ctx *ctx, int *nb_cpu_out)
>      return ret;
>  }
>  
> +#ifdef  LIBXL_HAVE_PCITOPO

No need for this conditional anywhere within libxl itself, nor in
libxl.h or in xl_cmdimpl.c either, it's for external app convenience
only.

> +/*
> + * LIBXL_HAVE_PCITOPO
> + *
> + * If this is defined, we have interfaces to query hypervisor about PCI device
> + * topology
> + */
> +#define  LIBXL_HAVE_PCITOPO 1

Can we call it PCI_TOPOLOGY here please.

> diff --git a/tools/libxl/libxl_linux.c b/tools/libxl/libxl_linux.c
> index ea5d8c1..f558dba 100644
> --- a/tools/libxl/libxl_linux.c
> +++ b/tools/libxl/libxl_linux.c
> @@ -279,3 +279,77 @@ libxl_device_model_version libxl__default_device_model(libxl__gc *gc)
>  {
>      return LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN;
>  }
> +
> +#ifdef  LIBXL_HAVE_PCITOPO
> +/* These two routines are "inspired" by pciutils */

Unless you've actually copied code (with the attention to licensing
which that requires) then please just drop this comment since it's only
purpose is to make me suspicious about the licensing status of these
functions.

Or if you did copy (although you previously said not) then the comment
should be expanded to clarify the apparent licensing disparity.

Ian.