RE: [PATCH v5 01/23] xl / libxl: s/pcidev/pci and remove DEFINE_DEVICE_TYPE_STRUCT_X

[Paul Durrant <xadimgnik@xxxxxxxxx>]

> -----Original Message-----
> From: Wei Liu <wl@xxxxxxx>
> Sent: 04 December 2020 11:13
> To: Paul Durrant <paul@xxxxxxx>
> Cc: xen-devel@xxxxxxxxxxxxxxxxxxxx; Paul Durrant <pdurrant@xxxxxxxxxx>; 
> Oleksandr Andrushchenko
> <oleksandr_andrushchenko@xxxxxxxx>; Ian Jackson <iwj@xxxxxxxxxxxxxx>; Wei Liu 
> <wl@xxxxxxx>; Anthony
> PERARD <anthony.perard@xxxxxxxxxx>
> Subject: Re: [PATCH v5 01/23] xl / libxl: s/pcidev/pci and remove 
> DEFINE_DEVICE_TYPE_STRUCT_X
> 
> On Thu, Dec 03, 2020 at 02:25:12PM +0000, 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>
> > Reviewed-by: Oleksandr Andrushchenko <oleksandr_andrushchenko@xxxxxxxx>
> > ---
> > Cc: Ian Jackson <iwj@xxxxxxxxxxxxxx>
> > Cc: Wei Liu <wl@xxxxxxx>
> > Cc: Anthony PERARD <anthony.perard@xxxxxxxxxx>
> >
> 
> This is going to break libxl callers because the name "pcidev" is
> visible from the public header.
> 
> I agree this is confusing and inconsistent, but we didn't go extra
> length to maintain the inconsistency for no reason.
> 
> If you really want to change it, I won't stand in the way. In fact, I'm
> all for consistency. I think the flag you added should help alleviate
> the fallout.

Yes, I thought that was the idea... we can make API changes if we add a flag. I 
could see about adding shims to translate the names
and keep the internal code clean.

  Paul

> Also, you will need to submit patches to libvirt (the only
> user I know of) to make use of the flag and switch to the new name and
> then request such patch(es) be backported to all maintained version of
> libvirt.
> 
> See
> https://github.com/libvirt/libvirt/blob/0d05d51b715390e08cd112f83e03b6776412aaeb/src/libxl/libxl_conf.
> c#L2273
> 
> Wei.