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:22
> To: paul@xxxxxxx
> Cc: 'Wei Liu' <wl@xxxxxxx>; xen-devel@xxxxxxxxxxxxxxxxxxxx; 'Paul Durrant' 
> <pdurrant@xxxxxxxxxx>;
> 'Oleksandr Andrushchenko' <oleksandr_andrushchenko@xxxxxxxx>; 'Ian Jackson' 
> <iwj@xxxxxxxxxxxxxx>;
> 'Anthony PERARD' <anthony.perard@xxxxxxxxxx>
> Subject: Re: [PATCH v5 01/23] xl / libxl: s/pcidev/pci and remove 
> DEFINE_DEVICE_TYPE_STRUCT_X
> 
> 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.
> 

I think shims are safest. We don't know what other callers are lurking out 
there :-)

  Paul

> Wei.