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
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
