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

Re: [libvirt] [RFC 1/6] conf: Get rid of virDomainCapsDevice



On Wed, 2016-03-30 at 13:19 -0400, John Ferlan wrote:
> On 03/21/2016 01:28 PM, Andrea Bolognani wrote:
> > 
> > The struct contains a single boolean field which can be
> > applied to domain capabilities that do not represent device
> > availability.
> > 
> > Instead of trying to come up with a more generic name just
> > get rid of the struct altogether.
> > ---
> >  src/conf/domain_capabilities.c |  6 +++---
> >  src/conf/domain_capabilities.h | 14 ++++----------
> >  src/qemu/qemu_capabilities.c   |  8 ++++----
> >  tests/domaincapstest.c         |  8 ++++----
> >  4 files changed, 15 insertions(+), 21 deletions(-)
> 
> The construct was added as part of commit id '614581f32' - not sure if
> Michal felt more bool's were going to be added to virDomainCapsDevice.

The problem is that, as you noted, patch 4 adds a
virDomainCapsFeatureGIC structure - if I were to follow the
pattern established with eg. virDomainCapsDeviceDisk, I would
have to introduce something like

  struct _virDomainCapsFeature {
    bool supported;
  };

and then use it as the first member of virDomainCapsFeatureGIC;
however, that would clash with the virDomainCapsFeature that's
already being defined in domain_conf.h.

Moreover, the existing FORMAT_PROLOGUE() macro would not be
usable for virDomainCapsFeatureGIC, because it checks for
item->device.supported and we would probably use something
like item->feature.supported instead.

Last but not least, the current usage is questionable: neither
virDomainCapsOS nor virDomainCapsLoader end up being inside the
<devices> element, yet both include virDomainCapsDevice as
their first member...

> I see this affects patch 4 - I think it would be a good idea to see if
> Michal had "other designs" in mind before changing this. That could be
> separate too...

CCing Michal so he can voice his opinion on the matter.

Cheers.


PS: Don't worry, I have no idea what I was trying to say with
    the first paragraph of the commit message either :)
-- 
Andrea Bolognani
Software Engineer - Virtualization Team


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