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
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list