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

[Osier Yang]

On 2011年12月20日 02:51, Eric Blake wrote:
> On 12/19/2011 11:38 AM, Eric Blake wrote:
>> 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.

Yes, I'd say commit ea7182c29 might be not a quite good idea for
we do somethings special internally, just like for the address
conversion, it's not quite good idea to do things like early
comparison in virsh, but since it's already pushed, the better
way is to find some way to work around.

>>
>> 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.
>
> Maybe we need a new API, something like:
>
> char *virConnectNormalizeXML(virConnectPtr conn, const char *xml,
> unsigned int flags)
>
> that takes incoming xml, passes it through the internal parser, and
> spits back out the resulting xml (or diagnoses the xml as invalid by
> returning NULL), all without modifying any existing domain, network,
> storage pool, or other object managed by xml.
>

Sounds great idea, but the only question in my mind is how useful
the API will be. Seems it will be useful only for the app which
does things like virsh, or some one wants to see the dry-run XML
pieces.

And it inspires me perhaps it will be great if we support dry-run
XML not only for the XML pieces, but also for the whole domain,
network, storage, interface, ... configs too?

Regards,
Osier