[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