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

[Eric Blake]

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.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 539 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20141118/65d27884/attachment-0001.sig>