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

Re: [libvirt] [PATCH v2 1/9] test: Allow specifying object runstate in driver XML



On 08/30/2013 12:43 PM, Eric Blake wrote:
> On 08/30/2013 10:03 AM, Cole Robinson wrote:
>> When passing in custom driver XML, allow a block like
>>
>> <domain xmlns:test='http://libvirt.org/schemas/domain/test/1.0'>
>>   ...
>>   <test:runstate>5</test:runstate>
> 
> Since the enum virDomainState is part of our public API, I'm not worried
> about the numbers ever being tied to the wrong state.  But wouldn't it
> be nicer for the XML to use a string conversion of a name, rather than
> just a raw number?  On the other hand, this is just for testing
> purposes, so I can live with it.
> 

Yeah I think for something like this the only people who will use it are
capable of getting the domain state numbers, and it keeps the code simpler.

>> </domain>
>>
>> This is only read at initial driver start time, and sets the initial
>> run state of the object. This is handy for UI testing.
>>
>> It's only wired up for domains, since that's the only conf/
>> infrastructure that supports namespaces at the moment.
>> ---
>>  src/test/test_driver.c | 91 ++++++++++++++++++++++++++++++++++++++++++++++----
>>  1 file changed, 84 insertions(+), 7 deletions(-)
>>
> 
>> +
>> +    tmp = virXPathUInt("string(./test:runstate)", ctxt, &tmpuint);
>> +    if (tmp == 0) {
>> +        if (tmpuint >= VIR_DOMAIN_LAST) {
>> +            virReportError(VIR_ERR_XML_ERROR,
>> +                           _("runstate '%d' out of range'"), tmpuint);
>> +            goto error;
>> +        }
>> +        nsdata->runstate = tmpuint;
> 
> Should we also reject VIR_DOMAIN_NOSTATE?

But isn't it a valid state? I know xen used to return it quite a bit but that
was ages ago. But if any libvirt version returned it then it's useful for apps
to test.

> 
>> +    };
>> +
>> +    /* All our XML extensions are write only, so we only need to parse */
> 
> Maybe s/write only/input only/
> 
> ACK, but not until after the release.
> 

Thanks, pushed now with that change.

- Cole


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