[libvirt] [PATCHv5 3/3] API: add trivial qemu support for VIR_TYPED_PARAM_STRING

Eric Blake eblake at redhat.com
Wed Nov 2 18:27:50 UTC 2011


On 11/02/2011 05:12 AM, Stefan Berger wrote:
> On 11/01/2011 07:47 PM, Eric Blake wrote:
>> Qemu will be the first driver to make use of a typed string in the
>> next round of additions. Separate out the trivial addition.
>>
>> * src/qemu/qemu_driver.c (qemudSupportsFeature): Advertise feature.
>> (qemuDomainGetBlkioParameters, qemuDomainGetMemoryParameters)
>> (qemuGetSchedulerParametersFlags, qemudDomainBlockStatsFlags):
>> Allow typed strings flag where trivially supported.

>> virCheckFlags(VIR_DOMAIN_AFFECT_LIVE |
>> - VIR_DOMAIN_AFFECT_CONFIG, -1);
>> + VIR_DOMAIN_AFFECT_CONFIG |
>> + VIR_TYPED_PARAM_STRING_OKAY, -1);
> Here you seem to be mixing flags of different 'enum types', though they
> don't step on each other. Couldn't you make the
> VIR_TYPED_PARAM_STRING_OKAY be of the same type as the
> VIR_DOMAIN_AFFECT_LIVE?

Well, in patch 1/3, libvirt.h.in specifically stated that these enum 
values must not overlap, but didn't do anything to guarantee that; it's 
probably worth me adding some verify() codes to ensure sane growth of 
these enums.

>
> Or do something like this here to prevent the two types of flags from
> ever stepping on each other:
>
> typedef enum {
> VIR_TYPED_PARAM_STRING_OKAY = (VIR_DOMAIN_AFFECT_LAST<< 0),
>
> } virTypedParameterFlags;
>
> with VIR_DOMAIN_AFFECT_LAST = (1 << 2).

No, that's not extensible.  VIR_DOMAIN_AFFECT_LAST might grow in the 
future, but VIR_TYPED_PARAM_STRING_OKAY must _not_ change in value, 
since it is publicly documented.  Rather, if we ever add to 
virDomainModificationImpact, that addition must not overwrite 
VIR_TYPED_PARAM_STRING_OKAY.

Hmm, I'll have to think about how to redo patch 1/3 as a result, to make 
our intentions clear as to the future growth of either enum value.

> Rest looks good. ACK

I'll do some more testing (in particular, actually use the new 
TYPED_PARAM_STRING by finishing the work on Hu's v4 series to expose 
cgroups.weight_device through blkio parameters), before posting a v2, 
since you turned up some good bugs that I should have seen if I had 
actually used the new code :)

-- 
Eric Blake   eblake at redhat.com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org




More information about the libvir-list mailing list