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

[Wei Liu <wei.liu2%citrix.com@localhost>]

On Mon, Feb 16, 2015 at 10:54:11AM -0500, Boris Ostrovsky wrote:
[...]
> >>+    if (valid_devs == 0)
> >>+        printf("No device topology data available\n");
> >>+
> >>+    libxl_pcitopology_list_free(pciinfo, nr);
> >>+#endif
> >>
> >And for implementation too, I think.
> 
> 
> I am not sure what the standard practice is for LIBXL_HAVE_ macros. I see a
> couple of examples where '#ifdef LIBXL_HAVE_*" is used in libxl code, which
> is why I have it here as well.
> 
> This is probably question for maintainers.
> 

They are not needed inside libxl. They are also not needed in xl, if
your new feature works on all architectures etc, because xl is shipped
with libxl and will always use the latest API.

Sorry for not mentioning this earlier.

> 
> >
> >In fact, if I'm not missing something obvious, since we're always
> >distributing xl and libxl together, this will always be "#ifdef 1",
> >wouldn't it?
> >
> >Also, ISTR that the first change that actually changes the API should
> >bump the MAJOR in libxl's Makefile. I'm not sure this change qualifies
> >as such, as you're adding stuff, not altering existing data structs
> >(e.g., by adding fields, etc). Personally, I think it does, but I'm
> >leaving this to tools maintainers.
> 
> libxl.h seems to suggest that API version is changed only when we make an
> incompatible change to the library. In my mind new interface is does not
> break compatibility so I didn't think a bump would be necessary.
> 
> -boris
>