2:25 a.m.
When testing all the different possibilities of interfaces with the
new netcf release + my latest libvirt patches (ACKed, but not yet
committed), I came across some problems, addressed by the following
two patches.
The first patch is for a bug that crept in due to a last minute change
suggested in danpb's review - I missed changing a couple places to
account for changing the initial value of "ret". It could alternately
be squashed into this patch, which hasn't yet been committed:
https://www.redhat.com/archives/libvir-list/2009-October/msg00689.html
The second patch loosens up the parsing of vlan, bond, and bridge
interface XML because the live config XML doesn't know how to get
those items yet. I believe the necessary items can be learned and
included by the time for the *next* libvirt release, but think that
the new functionality there already is too useful (and too needy of
more hands to test) to delay the entire set for that long.
2:25 a.m.
New subject: [libvirt] [PATCH 1/2] Fix improper error return in virInterfaceDefParseProtoIPvX
---
src/conf/interface_conf.c | 14 ++++++++------
1 files changed, 8 insertions(+), 6 deletions(-)
diff --git a/src/conf/interface_conf.c b/src/conf/interface_conf.c
index 7cb71ed..fc18eba 100644
--- a/src/conf/interface_conf.c
+++ b/src/conf/interface_conf.c
@@ -309,10 +309,11 @@ virInterfaceDefParseProtoIPv4(virConnectPtr conn,
virInterfaceProtocolDefPtr def
def->gateway = tmp;
dhcp = virXPathNode(conn, "./dhcp", ctxt);
- if (dhcp != NULL)
+ if (dhcp != NULL) {
ret = virInterfaceDefParseDhcp(conn, def, dhcp, ctxt);
- if (ret != 0)
- return(ret);
+ if (ret != 0)
+ return(ret);
+ }
nIpNodes = virXPathNodeSet(conn, "./ip", ctxt, &ipNodes);
if (ipNodes == NULL)
@@ -365,10 +366,11 @@ virInterfaceDefParseProtoIPv6(virConnectPtr conn,
virInterfaceProtocolDefPtr def
def->autoconf = 1;
dhcp = virXPathNode(conn, "./dhcp", ctxt);
- if (dhcp != NULL)
+ if (dhcp != NULL) {
ret = virInterfaceDefParseDhcp(conn, def, dhcp, ctxt);
- if (ret != 0)
- return(ret);
+ if (ret != 0)
+ return(ret);
+ }
nIpNodes = virXPathNodeSet(conn, "./ip", ctxt, &ipNodes);
if (ipNodes == NULL)
--
1.6.5.15.gc274d
5:41 a.m.
New subject: [libvirt] [PATCH 1/2] Fix improper error return in virInterfaceDefParseProtoIPvX
On Wed, Oct 28, 2009 at 03:25:43AM -0400, Laine Stump wrote:
---
src/conf/interface_conf.c | 14 ++++++++------
1 files changed, 8 insertions(+), 6 deletions(-)
diff --git a/src/conf/interface_conf.c b/src/conf/interface_conf.c
index 7cb71ed..fc18eba 100644
--- a/src/conf/interface_conf.c
+++ b/src/conf/interface_conf.c
@@ -309,10 +309,11 @@ virInterfaceDefParseProtoIPv4(virConnectPtr conn,
virInterfaceProtocolDefPtr def
def->gateway = tmp;
dhcp = virXPathNode(conn, "./dhcp", ctxt);
- if (dhcp != NULL)
+ if (dhcp != NULL) {
ret = virInterfaceDefParseDhcp(conn, def, dhcp, ctxt);
- if (ret != 0)
- return(ret);
+ if (ret != 0)
+ return(ret);
+ }
nIpNodes = virXPathNodeSet(conn, "./ip", ctxt, &ipNodes);
if (ipNodes == NULL)
@@ -365,10 +366,11 @@ virInterfaceDefParseProtoIPv6(virConnectPtr conn,
virInterfaceProtocolDefPtr def
def->autoconf = 1;
dhcp = virXPathNode(conn, "./dhcp", ctxt);
- if (dhcp != NULL)
+ if (dhcp != NULL) {
ret = virInterfaceDefParseDhcp(conn, def, dhcp, ctxt);
- if (ret != 0)
- return(ret);
+ if (ret != 0)
+ return(ret);
+ }
nIpNodes = virXPathNodeSet(conn, "./ip", ctxt, &ipNodes);
if (ipNodes == NULL)
ACK
Daniel
--
|: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
2:25 a.m.
New subject: [libvirt] [PATCH 2/2] Make some vlan, bond, and bridge properties optional in the interface XML.
This is necessary because netcf does not yet know how to get this
information from the kernel, so the live config XML doesn't contain
it, leading to errors when passing it through the current libvirt parsers.
---
src/conf/interface_conf.c | 86 ++++++++++++++++++--------------------------
1 files changed, 35 insertions(+), 51 deletions(-)
diff --git a/src/conf/interface_conf.c b/src/conf/interface_conf.c
index fc18eba..8926804 100644
--- a/src/conf/interface_conf.c
+++ b/src/conf/interface_conf.c
@@ -782,30 +782,27 @@ virInterfaceDefParseXML(virConnectPtr conn, xmlXPathContextPtr ctxt)
{
if (virInterfaceDefParseIfAdressing(conn, def, ctxt) < 0)
goto error;
- bridge = virXPathNode(conn, "./bridge[1]", ctxt);
- if (bridge == NULL) {
- virInterfaceReportError(conn, VIR_ERR_XML_ERROR,
- "%s", _("bridge interface misses
the bridge element"));
- goto error;
- }
- tmp = virXMLPropString(bridge, "stp");
def->data.bridge.stp = -1;
- if (tmp != NULL) {
- if (STREQ(tmp, "on")) {
- def->data.bridge.stp = 1;
- } else if (STREQ(tmp, "off")) {
- def->data.bridge.stp = 0;
- } else {
- virInterfaceReportError(conn, VIR_ERR_XML_ERROR,
- _("bridge interface stp should be on or off got
%s"),
- tmp);
+ bridge = virXPathNode(conn, "./bridge[1]", ctxt);
+ if (bridge != NULL) {
+ tmp = virXMLPropString(bridge, "stp");
+ if (tmp != NULL) {
+ if (STREQ(tmp, "on")) {
+ def->data.bridge.stp = 1;
+ } else if (STREQ(tmp, "off")) {
+ def->data.bridge.stp = 0;
+ } else {
+ virInterfaceReportError(conn, VIR_ERR_XML_ERROR,
+ _("bridge interface stp should be on
or off got %s"),
+ tmp);
+ VIR_FREE(tmp);
+ goto error;
+ }
VIR_FREE(tmp);
- goto error;
}
- VIR_FREE(tmp);
+ ctxt->node = bridge;
+ virInterfaceDefParseBridge(conn, def, ctxt);
}
- ctxt->node = bridge;
- virInterfaceDefParseBridge(conn, def, ctxt);
break;
}
case VIR_INTERFACE_TYPE_BOND: {
@@ -818,14 +815,11 @@ virInterfaceDefParseXML(virConnectPtr conn, xmlXPathContextPtr ctxt)
{
if (virInterfaceDefParseIfAdressing(conn, def, ctxt) < 0)
goto error;
bond = virXPathNode(conn, "./bond[1]", ctxt);
- if (bond == NULL) {
- virInterfaceReportError(conn, VIR_ERR_XML_ERROR,
- "%s", _("bond interface misses the bond
element"));
- goto error;
+ if (bond != NULL) {
+ ctxt->node = bond;
+ if (virInterfaceDefParseBond(conn, def, ctxt) < 0)
+ goto error;
}
- ctxt->node = bond;
- if (virInterfaceDefParseBond(conn, def, ctxt) < 0)
- goto error;
break;
}
case VIR_INTERFACE_TYPE_VLAN: {
@@ -839,14 +833,11 @@ virInterfaceDefParseXML(virConnectPtr conn, xmlXPathContextPtr ctxt)
{
if (virInterfaceDefParseIfAdressing(conn, def, ctxt) < 0)
goto error;
vlan = virXPathNode(conn, "./vlan[1]", ctxt);
- if (vlan == NULL) {
- virInterfaceReportError(conn, VIR_ERR_XML_ERROR,
- "%s", _("vlan interface misses the vlan
element"));
- goto error;
+ if (vlan != NULL) {
+ ctxt->node = vlan;
+ if (virInterfaceDefParseVlan(conn, def, ctxt) < 0)
+ goto error;
}
- ctxt->node = vlan;
- if (virInterfaceDefParseVlan(conn, def, ctxt) < 0)
- goto error;
break;
}
@@ -1108,12 +1099,8 @@ virInterfaceBondDefFormat(virConnectPtr conn, virBufferPtr buf,
else if (def->data.bond.validate == VIR_INTERFACE_BOND_ARP_ALL)
virBufferAddLit(buf, " validate='all'");
virBufferAddLit(buf, "/>\n");
- } else {
- virInterfaceReportError(conn, VIR_ERR_INTERNAL_ERROR,
- _("bond monitoring type %d unknown"),
- def->data.bond.monit);
- return(-1);
}
+
for (i = 0;i < def->data.bond.nbItf;i++) {
if (virInterfaceBareDevDefFormat(conn, buf, def->data.bond.itf[i]) < 0)
ret = -1;
@@ -1126,20 +1113,17 @@ virInterfaceBondDefFormat(virConnectPtr conn, virBufferPtr buf,
static int
virInterfaceVlanDefFormat(virConnectPtr conn, virBufferPtr buf,
const virInterfaceDefPtr def) {
- if (def->data.vlan.tag == NULL) {
- virInterfaceReportError(conn, VIR_ERR_INTERNAL_ERROR,
- "%s", _("vlan misses the tag
name"));
- return(-1);
+ if (def->data.vlan.tag != NULL) {
+ virBufferVSprintf(buf, " <vlan tag='%s'",
def->data.vlan.tag);
+ if (def->data.vlan.devname != NULL) {
+ virBufferAddLit(buf, ">\n");
+ virBufferVSprintf(buf, " <interface
name='%s'/>\n",
+ def->data.vlan.devname);
+ virBufferAddLit(buf, " </vlan>\n");
+ } else
+ virBufferAddLit(buf, "/>\n");
}
- virBufferVSprintf(buf, " <vlan tag='%s'",
def->data.vlan.tag);
- if (def->data.vlan.devname != NULL) {
- virBufferAddLit(buf, ">\n");
- virBufferVSprintf(buf, " <interface name='%s'/>\n",
- def->data.vlan.devname);
- virBufferAddLit(buf, " </vlan>\n");
- } else
- virBufferAddLit(buf, "/>\n");
return(0);
}
--
1.6.5.15.gc274d
5:40 a.m.
New subject: [libvirt] [PATCH 2/2] Make some vlan, bond, and bridge properties optional in the interface XML.
On Wed, Oct 28, 2009 at 03:25:44AM -0400, Laine Stump wrote:
This is necessary because netcf does not yet know how to get this
information from the kernel, so the live config XML doesn't contain
it, leading to errors when passing it through the current libvirt parsers.
Omitting this information makes the XML pretty useless. It is easy
to get the name of the enslaved interface for a bond/bridge out of
the sysfs, so I'd rather we just fixed netcf than make this stuff
optional.
Daniel
--
|: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
5505
days inactive
5505
days old
4 comments
2 participants
participants (2)
-
Daniel P. Berrange -
Laine Stump