[Libvir] [PATCH] Cut out "type=ioemu" on Xen3.1

Hi, The definition of (type ioemu) is *not* required by Xen3.1. So I cut out it when xendConfigVersion is 4 or later. Signed-off-by: Saori Fukuta <fukuta.saori@jp.fujitsu.com> Thanks, Saori Fukuta. Index: xml.c =================================================================== RCS file: /data/cvs/libvirt/src/xml.c,v retrieving revision 1.73 diff -u -p -r1.73 xml.c --- xml.c 23 Apr 2007 07:41:23 -0000 1.73 +++ xml.c 31 May 2007 07:02:01 -0000 @@ -1047,6 +1047,7 @@ virDomainParseXMLDiskDesc(virConnectPtr * @conn: pointer to the hypervisor connection * @node: node containing the interface description * @buf: a buffer for the result S-Expr + * @xendConfigVersion: xend configuration file format * * Parse the one interface the XML description and add it to the S-Expr in buf * This is a temporary interface as the S-Expr interface @@ -1056,7 +1057,7 @@ virDomainParseXMLDiskDesc(virConnectPtr * Returns 0 in case of success, -1 in case of error. */ static int -virDomainParseXMLIfDesc(virConnectPtr conn ATTRIBUTE_UNUSED, xmlNodePtr node, virBufferPtr buf, int hvm) +virDomainParseXMLIfDesc(virConnectPtr conn ATTRIBUTE_UNUSED, xmlNodePtr node, virBufferPtr buf, int hvm, int xendConfigVersion) { xmlNodePtr cur; xmlChar *type = NULL; @@ -1129,7 +1130,8 @@ virDomainParseXMLIfDesc(virConnectPtr co virBufferVSprintf(buf, "(script '%s')", script); if (ip != NULL) virBufferVSprintf(buf, "(ip '%s')", ip); - if (hvm) + /* HVM type ioemu config for xen < 3.1 */ + if (hvm && xendConfigVersion < 4) virBufferAdd(buf, "(type ioemu)", 12); virBufferAdd(buf, ")", 1); @@ -1335,7 +1337,7 @@ virDomainParseXMLDesc(virConnectPtr conn if (nb_nodes > 0) { for (i = 0; i < nb_nodes; i++) { virBufferAdd(&buf, "(device ", 8); - res = virDomainParseXMLIfDesc(conn, nodes[i], &buf, hvm); + res = virDomainParseXMLIfDesc(conn, nodes[i], &buf, hvm, xendConfigVersion); if (res != 0) { free(nodes); goto error; @@ -1494,7 +1496,7 @@ virParseXMLDevice(virConnectPtr conn, ch goto error; } else if (xmlStrEqual(node->name, BAD_CAST "interface")) { - if (virDomainParseXMLIfDesc(conn, node, &buf, hvm) != 0) + if (virDomainParseXMLIfDesc(conn, node, &buf, hvm, xendConfigVersion) != 0) goto error; } cleanup:

On Thu, May 31, 2007 at 05:38:02PM +0900, Saori Fukuta wrote:
Hi,
The definition of (type ioemu) is *not* required by Xen3.1. So I cut out it when xendConfigVersion is 4 or later.
okay, the patch looks fine, but what happens when you leave (type ioemu) in 3.1 expressions ? Does it fail ? If it doesn't fail, maybe we can avoid the complexity added by this patch, right ? Daniel -- Red Hat Virtualization group http://redhat.com/virtualization/ Daniel Veillard | virtualization library http://libvirt.org/ veillard@redhat.com | libxml GNOME XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/

On Fri, 1 Jun 2007 05:47:51 -0400 Daniel Veillard wrote:
okay, the patch looks fine, but what happens when you leave (type ioemu) in 3.1 expressions ? Does it fail ? If it doesn't fail, maybe we can avoid the complexity added by this patch, right ?
It does not fail on 3.1 even if I remove (type ioemu) because it is not necessary to add specially. Thanks, Saori Fukuta

On Fri, Jun 01, 2007 at 07:29:25PM +0900, Saori Fukuta wrote:
On Fri, 1 Jun 2007 05:47:51 -0400 Daniel Veillard wrote:
okay, the patch looks fine, but what happens when you leave (type ioemu) in 3.1 expressions ? Does it fail ? If it doesn't fail, maybe we can avoid the complexity added by this patch, right ?
It does not fail on 3.1 even if I remove (type ioemu) because it is not necessary to add specially.
okay, then there is enough places where we must tweak the code depending on the version of Xend running, this makes the code harder, less maintainable, so unless we actually get a problem by still adding this unecessary extra data, I think it is better to keep the code simpler. I just added a comment to log that information in the code: if (ip != NULL) virBufferVSprintf(buf, "(ip '%s')", ip); /* apparently not needed any more for xen >= 3.1 but harmless */ if (hvm) virBufferAdd(buf, "(type ioemu)", 12); thanks, Daniel -- Red Hat Virtualization group http://redhat.com/virtualization/ Daniel Veillard | virtualization library http://libvirt.org/ veillard@redhat.com | libxml GNOME XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/

Hi, Daniel If we want to use PV-driver(VNIF), We should remove the "type=ioemu". Please apply Saori's patch. Of course If you use only HVM w/o PV-driver(VNIF). "type=ioemu" is required. Thanks Atsushi SAKAI Daniel Veillard <veillard@redhat.com> wrote:
On Fri, Jun 01, 2007 at 07:29:25PM +0900, Saori Fukuta wrote:
On Fri, 1 Jun 2007 05:47:51 -0400 Daniel Veillard wrote:
okay, the patch looks fine, but what happens when you leave (type ioemu) in 3.1 expressions ? Does it fail ? If it doesn't fail, maybe we can avoid the complexity added by this patch, right ?
It does not fail on 3.1 even if I remove (type ioemu) because it is not necessary to add specially.
okay, then there is enough places where we must tweak the code depending on the version of Xend running, this makes the code harder, less maintainable, so unless we actually get a problem by still adding this unecessary extra data, I think it is better to keep the code simpler. I just added a comment to log that information in the code:
if (ip != NULL) virBufferVSprintf(buf, "(ip '%s')", ip); /* apparently not needed any more for xen >= 3.1 but harmless */ if (hvm) virBufferAdd(buf, "(type ioemu)", 12);
thanks,
Daniel
-- Red Hat Virtualization group http://redhat.com/virtualization/ Daniel Veillard | virtualization library http://libvirt.org/ veillard@redhat.com | 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

On Wed, Jun 06, 2007 at 01:38:46PM +0900, Atsushi SAKAI wrote:
Hi, Daniel
If we want to use PV-driver(VNIF), We should remove the "type=ioemu". Please apply Saori's patch. Of course If you use only HVM w/o PV-driver(VNIF). "type=ioemu" is required.
Ah, so adding (type ioemu) actually breaks something, i.e. paravirt drivers in HVM setups. Okay, applied, thanks, Daniel -- Red Hat Virtualization group http://redhat.com/virtualization/ Daniel Veillard | virtualization library http://libvirt.org/ veillard@redhat.com | libxml GNOME XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/

On Wed, 6 Jun 2007 10:39:09 -0400 Daniel Veillard wrote:
On Wed, Jun 06, 2007 at 01:38:46PM +0900, Atsushi SAKAI wrote:
Hi, Daniel
If we want to use PV-driver(VNIF), We should remove the "type=ioemu". Please apply Saori's patch. Of course If you use only HVM w/o PV-driver(VNIF). "type=ioemu" is required.
Ah, so adding (type ioemu) actually breaks something, i.e. paravirt drivers in HVM setups. Okay, applied,
Thank you for committing, and I appreciate Atsushi's help. It works fine ! thanks a lot, Saori Fukuta
participants (3)
-
Atsushi SAKAI
-
Daniel Veillard
-
Saori Fukuta