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

[Wei Liu <wl@xxxxxxx>]

On Fri, Dec 04, 2020 at 11:19:47AM -0000, Paul Durrant wrote:
> > -----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.

Yes if you can add some internal shims to handle it that would be
great. Otherwise you will need to at least fix libvirt.

Wei.