Re: [Libvir] [PATCH] check file format in virsh attach/detach-device
On Tue, Jul 31, 2007 at 06:46:19PM +0900, 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.
Signed-off-by: Masayuki Sunou <fj1826dm(a)aa.jp.fujitsu.com>
Atsushi SAKAI <sakaia(a)jp.fujitsu.com>
Sorry I will have to reject this ! You are breaking the layering, and
the API.
http://libvirt.org/html/libvirt-libvirt.html#virDomainAttachDevice
Returns: 0 in case of success, -1 in case of failure.
your patch makes it return -2 you are breaking the API, this is not acceptable.
An error should be described as precisely as possible when detected. This
should not be handled in an application specific way, so to me this is wrong
in 2 ways.
The routine inside the Xen driver should indicate *why* it could not load the
file, and be as precise as possible, virParseXMLDevice should be refined
to provide a better error report instead. We already have a specific error
code for this VIR_ERR_XML_ERROR, just use it.
Daniel
--
Red Hat Virtualization group http://redhat.com/virtualization/
Daniel Veillard | virtualization library http://libvirt.org/
veillard(a)redhat.com | libxml GNOME XML XSLT toolkit http://xmlsoft.org/
http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/