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

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

 



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

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]