[libvirt] [PATCH] virsh: Fix a regression of detaching device

[Eric Blake]

On 12/19/2011 04:01 AM, Osier Yang wrote:
> Since for node <address>, the values for attrs "domain", "bus", "slot",
> "function" are all converted to heximal when parsing the XML. However
> commit ea7182c29 tries to examine if the device represented by the XML
> from user exists in the domain XML earlier before the XML parsing, and
> thus if the user use different base to represent the device address, e.g.
>   <address type='pci' domain='0' bus='0' slot='7'/>
> 
> vshCompleteXMLFromDomain won't find the device, and quit with error
> "no such device" even if the device does exist.
> ---
>  tools/virsh.c |   41 +++++++++++++++++++++++++++++++++++------
>  1 files changed, 35 insertions(+), 6 deletions(-)

Hmm.  I agree that we have a problem, but I think this patch is taking
the wrong approach to fixing it.  By special casing which pieces of XML
should be parsed as an integer and compared by integer value rather than
by string equality, we effectively end up having to add more and more
exceptions every time someone else points out another field that needs
to be treated as an integer.

It would be a lot better to read the user's input, and normalize it into
a single form, then compare the normalized forms; rather than this
patch's attempt to normalize individual fields during the comparison.
But I'm not sure how we would go about doing that, short of reusing our
XML parsing code from domain_conf.c which does the normalization by
flattening XML into a consistent in-memory representation, followed by
the code that re-generates XML from that in-memory representation.

-- 
Eric Blake   eblake at 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: 620 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20111219/de8e8388/attachment-0001.sig>