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 <[EMAIL PROTECTED]> > Atsushi SAKAI <[EMAIL PROTECTED]>
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/ [EMAIL PROTECTED] | libxml GNOME XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/ -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
