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

Re: [libvirt] [RFC PATCH 0/6] New API to normalize the device XML



On Mon, Jan 09, 2012 at 03:53:25PM -0700, Eric Blake wrote:
> > I don't really understand what the use case is for this API, from
> > the description above.  Can you give an example of how, for example,
> > virt-manager would use this API todo something ?
> 
> I see several potential uses:
> 
> 1. XML feature probe.  For example, libvirt 0.9.9 just added the XML
> feature of having a <seclabel> attached to a specific <disk>; older
> libvirt silently ignores that label.  Right now, I have no way to know
> if I use <seclabel> if I will get a per-disk override; running
> virt-xml-validate on my machine doesn't help if the libvirt on the
> remote server is at a different libvirt version, and if my only
> connection to the remote machine is via libvirt connections rather than
> a direct ssh, then I can't run virt-xml-validate on the destination
> machine.  But with this new API, I could:
> construct a candidate XML
> run it through the API
> compare the resulting XML with my candidate
> if <seclabel> is still intact, then the feature is supported; if it was
> stripped, then I am talking to an older libvirt that didn't know the feature
> 
> 2. XML subset matching.  Things like 'virsh detach-device' want to be
> permissive in what the user passes in, by taking minimal XML from the
> user and expanding it to the exact XML present in the domain.  But since
> we allow interleaves, it's not easy to do the comparison from user XML
> to the order that the XML is generated by 'virsh dumpxml'.  With the new
> API, virsh could be coded to run any candidate XML through the
> normalization, prior to doing subset comparisons against the real domain
> XML, and have the assurance that all known elements and attributes
> appear in the same order.

These 2 things feel like overloading the API with two different
semantics. On the one hand I see a simple  XML -> struct -> XML
transformation, which is driver indepedant and does not need to
get any HV state.  I call this canonicalization

The other case I see   XML -> struct -> munge struct info -> XML
which makes changes based on the HV state. I call this XML
enhancement. I'm not even convinced this is desirable. IMHO the way
virsh detach device is going about this task is just plain wrong.
It should be just requesting the full XML document, and then doing
an XPath query on it, to pull out the <interface> element it
needs. This removes the need to create any XML from scratch.


> 3. XML validation.  It seems pretty easy to expand this API into taking
> a flag that says to compare the incoming XML against the RNG, and return
> NULL on failure to validate.  This would make implementation of 'virsh
> edit' smarter, by being able to validate the user's edits prior to
> trying to apply them.
>
> 4. libvirt-gconfig seems like an obvious client for taking user input
> and normalizing it into the same form that libvirt would output.

libvirt-gconfig actually attempts to never re-write the XML it receives
either from libvirt or the user. The code is designed so it just parses
the XML doc it is given, and keeps the xmlDocPtr DOM in memory, and just
reads/writes the DOM directly for all APIs. This way we preserve all
data that the user has in the XML, including stuff we don't understand
ourselves.

> However, I think the API needs to be a bit more flexible than what Osier
> proposed.  It needs to be an API on the virConnectPtr, not a
> virDomainPtr, since it is NOT tied to a specific domain, but to the
> parsing abilities of the connection in general.  I also think that the
> API can handle multiple XML types in a single API.  (Oh yuck - I just
> realized that VIR_DOMAIN_XML_INACTIVE==2, but
> VIR_INTERFACE_XML_INACTIVE==1, so we _can't_ have a common
> VIR_NORMALIZE_XML_INACTIVE).  I envision something more like:

>   VIR_NORMALIZE_XML_VALIDATE = (1 << 30), /* perform RNG validation */
> } virConnectNormalizeXMLFlags;
> 
> /**
>  * virConnectNormalizeXML:
>  * @conn: Pointer to connection
>  * @xml: XML to be normalized
>  * @type: one value from virConnectNormalizeXMLTypes, documenting
>  *   expected top-level element of @xml
>  * @flags: bitwise-OR of virConnectNormalizeXMLFlags
>  *
>  * This function parses the incoming @xml, with root element
>  * according to @type, and returns the same XML normalized to the
>  * same form it would appear via the corresponding vir*GetXMLDesc
>  * function if it had described a real libvirt object.  No
>  * machine state is checked or changed, so while the XML may be
>  * valid, subsequent use of the XML might still fail for other
>  * reasons such as an attempt to reuse a UUID.
>  *
>  * The normalization process may reorder attributes and elements,
>  * and may add new elements describing default states. By default,
>  * unrecognized elements and attributes are silently removed, but
>  * if @flags includes VIR_NORMALIZE_XML_VALIDATE, an error is
>  * raised if any unknown input is encountered.
>  *
>  * Depending on @type, additional bits in @flags may be understood.
>  * For example, if @type specifies a domain or sub-element of a
>  * domain, the flags can include virDomainXMLFlags such as
>  * VIR_DOMAIN_XML_INACTIVE; whereas if @type specifies a host
>  * interface, the flags can include virInterfaceXMLFlags.
>  *
>  * Returns the normalized XML (caller must free), or NULL if the
>  * XML could not be normalized.
>  */
> char *
> virConnectNormalizeXML(virConnectPtr conn,
>                        const char *xml,
>                        unsigned int type,
>                        unsigned int flags);

This does look more useful, but as mentioned above, IMHO the normalization
should be purely syntactic normalization, and not rely on any semantic
state from the hypervisor drivers.

ie, basically round trip virDomainParseDef+virDomainFormatDef + optional
RNG validation.

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|


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