[libvirt] [PATCH v2 1/7] dumpxml: add flag to virDomainGetXMLDesc

[Adam Litke]

On 18/11/14 09:33 -0700, Eric Blake wrote:
>On 11/18/2014 07:50 AM, Adam Litke wrote:
>>>
>>> diff --git a/include/libvirt/libvirt-domain.h
>>> b/include/libvirt/libvirt-domain.h
>>> index 1fac2a3..caf1f46 100644
>>> --- a/include/libvirt/libvirt-domain.h
>>> +++ b/include/libvirt/libvirt-domain.h
>>> @@ -1248,6 +1248,7 @@ typedef enum {
>>>     VIR_DOMAIN_XML_INACTIVE     = (1 << 1), /* dump inactive domain
>>> information */
>>>     VIR_DOMAIN_XML_UPDATE_CPU   = (1 << 2), /* update guest CPU
>>> requirements according to host CPU */
>>>     VIR_DOMAIN_XML_MIGRATABLE   = (1 << 3), /* dump XML suitable for
>>> migration */
>>> +    VIR_DOMAIN_XML_BLOCK_INFO   = (1 << 4), /* include storage volume
>>> information about each disk source */
>>
>> I'll admit I'm not a master API designer but this is a red flag for me
>> (pun most definitely intended).  Up to this point, the individual
>> virDomainXMLFlags are used to ask for XML for a certain purpose.  For
>> example, VIR_DOMAIN_XML_INACTIVE retrieves XML suitable for a call to
>> virDomainCreateXML whereas VIR_DOMAIN_XML_MIGRATABLE is used to "dump
>> XML suitable for migration".
>
>Not quite.  The VIR_DOMAIN_XML_SECURE and VIR_DOMAIN_XML_UPDATE_CPU
>flags both have the behavior of dumping additional information that is
>omitted by default.  And the VIR_DOMAIN_XML_INACTIVE flag really means
>'dump the xml that will be used on the next boot' if the domain is
>persistent (if the domain is transient, it says 'dump the xml that would
>be used on the next boot if I make the domain persistent').
>
>>
>> This new flag is somewhat arbitrarily slicing up some of the ACTIVE VM
>> information.  Slippery slope logic would lead us to a future API with
>> a proliferation of flags that turn various bits of info on and off
>> which would be very cumbersome to maintain and use.  Since this
>> information is really ACTIVE VM info, I'd prefer it to be provided
>> whenever VIR_DOMAIN_XML_INACTIVE is not set.  Callers who want a
>> tight XML document can always use VIR_DOMAIN_XML_INACTIVE.
>
>The thing is that this new flag is providing additional informative
>information that is NOT being stored as part of the domain itself, but
>which is being queried from the disks each time the xml is gathered.
>What's more, the information is valid for both active and inactive xml
>(it is not an active-only flag), and gathering the information
>potentially requires a monitor command for an active domain (none of the
>other XML information requires a monitor command).  It is not good to
>require a monitor command by default, which is why I felt that having
>the flag opt-in to the behavior is the best way to preserve back-compat
>for callers that don't need the extra information.

Then perhaps the right flag to add would be VIR_DOMAIN_XML_QUERY or
some such to indicate that extra data needs to be retrieved via
potentially expensive means.  Then, the flag (and its semantics) could
be reused in the future.


-- 
Adam Litke