[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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



2011/7/13 Eric Blake <eblake redhat com>:
> On 07/13/2011 06:41 AM, Matthias Bolte wrote:
>> 2011/7/8 Eric Blake <eblake 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


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]