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

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.