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