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