[libvirt] [PATCHv2 15/27] uml: reject unknown flags

Matthias Bolte matthias.bolte at googlemail.com
Thu Jul 14 07:21:33 UTC 2011


2011/7/13 Eric Blake <eblake at redhat.com>:
> On 07/13/2011 06:41 AM, Matthias Bolte wrote:
>> 2011/7/8 Eric Blake <eblake at redhat.com>:
>>> * src/uml/uml_driver.c (umlOpen, umlDomainGetXMLDesc)
>>> (umlDomainBlockPeek): Reject unknown flags.
>>> ---
>>>  src/uml/uml_driver.c |   14 +++++++++++---
>>>  1 files changed, 11 insertions(+), 3 deletions(-)
>>>
>>
>>> @@ -1559,11 +1562,14 @@ cleanup:
>>>
>>>
>>>  static char *umlDomainGetXMLDesc(virDomainPtr dom,
>>> -                                 unsigned int flags ATTRIBUTE_UNUSED) {
>>> +                                 unsigned int flags)
>>> +{
>>>     struct uml_driver *driver = dom->conn->privateData;
>>>     virDomainObjPtr vm;
>>>     char *ret = NULL;
>>>
>>> +    virCheckFlags(0, NULL);
>>> +
>>>     umlDriverLock(driver);
>>>     vm = virDomainFindByUUID(&driver->domains, dom->uuid);
>>>     umlDriverUnlock(driver);
>>
>> Don't get fooled by the ATTRIBUTE_UNUSED again. All *DomainGetXMLDesc
>> use virDomainDefFormat and have to accept all flags that
>> virDomainDefFormat accepts. I suggest to recheck your series for this
>> pattern, here it's just the first time that I notice it.
>
> Ouch.  Good catch, and I'll have to fix that shortly.  I'll post the
> patch before I commit it, but it should be considered a trivial
> regression fix, so I'll commit it without waiting for review.
>
>>
>> ACK, with that virCheckFlags loosened correctly.
>
> I squashed this in for this patch, and now I'm working on using the same
> fix for the regression committed in all the prior pushed patches.
>
> diff --git i/src/uml/uml_driver.c w/src/uml/uml_driver.c
> index 132aef1..da91687 100644
> --- i/src/uml/uml_driver.c
> +++ w/src/uml/uml_driver.c
> @@ -1599,7 +1599,9 @@ static char *umlDomainGetXMLDesc(virDomainPtr dom,
>     virDomainObjPtr vm;
>     char *ret = NULL;
>
> -    virCheckFlags(0, NULL);
> +    virCheckFlags(VIR_DOMAIN_XML_SECURE |
> +                  VIR_DOMAIN_XML_INACTIVE |
> +                  VIR_DOMAIN_XML_UPDATE_CPU, NULL);
>
>     umlDriverLock(driver);
>     vm = virDomainFindByUUID(&driver->domains, dom->uuid);

I'm not sure if it's a good idea to add the flags check for this set
of flags at the driver level. Maybe it should be moved one level down
into virDomainDefFormat. This avoids touching all drivers should we
ever add a new VIR_DOMAIN_XML_* flag.

-- 
Matthias Bolte
http://photron.blogspot.com




More information about the libvir-list mailing list