[Libvir] [PATCH] check file format in virsh attach/detach-device

[Richard W.M. Jones]

Masayuki Sunou wrote:
> Hi
> 
> virsh attach/detach-devich does not check file format now.
> 
> This patch checks config file format is XML or not.
> (in virsh attach/detach-device)
> If file format is not XML, just return the error with message.

Hello Masayuki,

To summarise what your patch does:

(1) It changes the semantics of virDomainAttachDevice and 
virDomainDetachDevice so that returning -2 as an error (instead of -1) 
means "the XML is invalid".

(2) It changes the implementation of the xenDaemon{Attach,Detach}Device 
functions in xend_internal.c so that it returns -2 instead of -1 if 
virParseXMLDevice fails.

My problems are:

(1) is an ABI change (and undocumented!).  In particular some of my own 
code depends on -1 meaning error (not just < 0).  Really we can't accept 
patches which make ABI changes like this.  You'll have seen a patch 
which I submitted last week get rejected because it made an egregious 
ABI change.

(2) isn't quite correct.  The function virParseXMLDevice can fail for 
many reasons (eg. out of memory).

So I think a better solution would look like either:

(a) Make virsh parse the XML file and do some simple sanity checks on 
it.  I realise that virsh cannot do a full check on the XML, but it can 
at least check things like: Is it an XML file?  Is the root node <disk> 
or <interface>?  That should be enough to catch simple command-line errors.

or:

(b) Fix the error handling in virParseXMLDevice so that it generates 
reasonable errors.  At the moment the error handling is mostly missing, 
and so error don't get propagated back to the user.

or:

(c) Modify libvirt to add a virDomainVerifyDeviceXML call which returns 
0 (correct) or -1 (error) depending on whether the  Device XML passed is 
reasonable.  Then virsh can do this call before it does the 
attach/detach call.

I hope that is helpful.

Rich.

-- 
Emerging Technologies, Red Hat - http://et.redhat.com/~rjones/
Registered Address: Red Hat UK Ltd, Amberley Place, 107-111 Peascod
Street, Windsor, Berkshire, SL4 1TE, United Kingdom.  Registered in
England and Wales under Company Registration No. 03798903
-------------- next part --------------
A non-text attachment was scrubbed...
Name: smime.p7s
Type: application/x-pkcs7-signature
Size: 3237 bytes
Desc: S/MIME Cryptographic Signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20070731/efbe8265/attachment-0001.bin>