Re: [PATCH v2 0/4] Fix parsing our own XMLs

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, 2016-04-15 at 06:40 +0200, Martin Kletzander wrote:
> On Thu, Apr 14, 2016 at 06:24:26PM +0200, Andrea Bolognani wrote:
> > On Wed, 2016-04-13 at 16:37 +0200, Martin Kletzander wrote:
> > > 
> > > v2:
> > >   - Just a rebase
> > >   - I did *not* use virPCIDeviceAddress wording instead as discussed in
> > >     the v1 thread.  That's because we have lot of functions working
> > >     with virDevicePCIAddress named exactly after that and renaming
> > >     those would be ugly IMHO.
> > 
> > Sorry, but I feel pretty strongly the other way around: if
> > it's defined in virpci.h, it should be called virPCI*.
> > 
> > virDevicePCIAddress is used a lot but AFAICT the number of
> > functions whose name is derived from it is just six.
> > 
> > Moreover, we don't have other virDevice*Address types (or
> > even just virDevice*) to set a precedent, but we have a
> > bunch of virPCI* stuff including virPCIDevice, which happens
> > to have a virPCIDeviceAddress among its members.
> > 
> > Bikeshedding, I know, but there you have it :)
> 
> No problem, I'm not against discussing this.  Feel free to try it out if
> you want. I actually should've posted something like 2a/4 and 2b/4,
> i.e. two versions of patches so that it's visible what's nice and what's
> not :)  If we don't reach a conclusion, I might do that later.

ACK series with the stuff pointed out in 4/4 taken care of
on the condition that you *do not* rename stuff from
virPCIDeviceAddress to virDevicePCIAddress.

If you're not okay with that condition we can discuss the
matter further next week :)

Cheers.

-- 
Andrea Bolognani
Software Engineer - Virtualization Team

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]