9:56 a.m.
This is the 2nd iteration of this patchset incorporating danpb's
suggestions. Along with a couple other minor things, the type attribute
is again a required attribute, and netcf support will be omitted in
the build unless you have netcf-devel-0.1.3 or greater. Since there is
no netcf-devel-0.1.3 yet, I tested this by changing the appropriate
strings to 0.1.2. 0.1.3 will be released "soon" (I want to try and get
IPv6 address reporting in before lutter cuts another release). I just
wanted to get the patches to the list early to get some visual review
in case I screwed up anything new ;-)
Note that (unless you're building/installing netcf from the head of
git) if you try to fake out the version of netcf yourself you'll get
runtime errors from virInterfaceGetXMLDesc (iface-dumpxml) due to
there being no "type" attribute in the interface xml.
9:56 a.m.
New subject: [libvirt] [PATCH 1/3] Make startmode optional in toplevel interface definitions.
From: Laine Stump <laine(a)laine.org>
The minimal XML returned from ncf_if_xml_state() doesn't contain this
attribute (which makes no sense in the case of reporting current
status of the interface), and it was preventing it from passing
through the parse/format step.
---
src/conf/interface_conf.c | 11 +++++------
src/conf/interface_conf.h | 3 ++-
2 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/src/conf/interface_conf.c b/src/conf/interface_conf.c
index e646351..540c931 100644
--- a/src/conf/interface_conf.c
+++ b/src/conf/interface_conf.c
@@ -128,12 +128,9 @@ virInterfaceDefParseStartMode(virConnectPtr conn, virInterfaceDefPtr
def,
char *tmp;
tmp = virXPathString(conn, "string(./start/@mode)", ctxt);
- if (tmp == NULL) {
- virInterfaceReportError(conn, VIR_ERR_XML_ERROR,
- "%s", _("interface misses the start mode
attribute"));
- return(-1);
- }
- if (STREQ(tmp, "onboot"))
+ if (tmp == NULL)
+ def->startmode = VIR_INTERFACE_START_UNSPECIFIED;
+ else if (STREQ(tmp, "onboot"))
def->startmode = VIR_INTERFACE_START_ONBOOT;
else if (STREQ(tmp, "hotplug"))
def->startmode = VIR_INTERFACE_START_HOTPLUG;
@@ -1039,6 +1036,8 @@ virInterfaceStartmodeDefFormat(virConnectPtr conn, virBufferPtr
buf,
enum virInterfaceStartMode startmode) {
const char *mode;
switch (startmode) {
+ case VIR_INTERFACE_START_UNSPECIFIED:
+ return 0;
case VIR_INTERFACE_START_NONE:
mode = "none";
break;
diff --git a/src/conf/interface_conf.h b/src/conf/interface_conf.h
index bb9dce4..20d9771 100644
--- a/src/conf/interface_conf.h
+++ b/src/conf/interface_conf.h
@@ -48,7 +48,8 @@ VIR_ENUM_DECL(virInterface)
/* types of start mode */
enum virInterfaceStartMode {
- VIR_INTERFACE_START_NONE = 0, /* not defined */
+ VIR_INTERFACE_START_UNSPECIFIED = 0, /* not given in config */
+ VIR_INTERFACE_START_NONE, /* specified as not defined */
VIR_INTERFACE_START_ONBOOT, /* startup at boot */
VIR_INTERFACE_START_HOTPLUG, /* on hotplug */
};
--
1.6.5.15.gc274d
11:22 a.m.
New subject: [libvirt] [PATCH 1/3] Make startmode optional in toplevel interface definitions.
On Fri, Oct 16, 2009 at 10:56:17AM -0400, laine(a)laine.org wrote:
From: Laine Stump <laine(a)laine.org>
The minimal XML returned from ncf_if_xml_state() doesn't contain this
attribute (which makes no sense in the case of reporting current
status of the interface), and it was preventing it from passing
through the parse/format step.
---
src/conf/interface_conf.c | 11 +++++------
src/conf/interface_conf.h | 3 ++-
2 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/src/conf/interface_conf.c b/src/conf/interface_conf.c
index e646351..540c931 100644
--- a/src/conf/interface_conf.c
+++ b/src/conf/interface_conf.c
@@ -128,12 +128,9 @@ virInterfaceDefParseStartMode(virConnectPtr conn, virInterfaceDefPtr
def,
char *tmp;
tmp = virXPathString(conn, "string(./start/@mode)", ctxt);
- if (tmp == NULL) {
- virInterfaceReportError(conn, VIR_ERR_XML_ERROR,
- "%s", _("interface misses the start mode
attribute"));
- return(-1);
- }
- if (STREQ(tmp, "onboot"))
+ if (tmp == NULL)
+ def->startmode = VIR_INTERFACE_START_UNSPECIFIED;
+ else if (STREQ(tmp, "onboot"))
def->startmode = VIR_INTERFACE_START_ONBOOT;
else if (STREQ(tmp, "hotplug"))
def->startmode = VIR_INTERFACE_START_HOTPLUG;
@@ -1039,6 +1036,8 @@ virInterfaceStartmodeDefFormat(virConnectPtr conn, virBufferPtr
buf,
enum virInterfaceStartMode startmode) {
const char *mode;
switch (startmode) {
+ case VIR_INTERFACE_START_UNSPECIFIED:
+ return 0;
case VIR_INTERFACE_START_NONE:
mode = "none";
break;
diff --git a/src/conf/interface_conf.h b/src/conf/interface_conf.h
index bb9dce4..20d9771 100644
--- a/src/conf/interface_conf.h
+++ b/src/conf/interface_conf.h
@@ -48,7 +48,8 @@ VIR_ENUM_DECL(virInterface)
/* types of start mode */
enum virInterfaceStartMode {
- VIR_INTERFACE_START_NONE = 0, /* not defined */
+ VIR_INTERFACE_START_UNSPECIFIED = 0, /* not given in config */
+ VIR_INTERFACE_START_NONE, /* specified as not defined */
VIR_INTERFACE_START_ONBOOT, /* startup at boot */
VIR_INTERFACE_START_HOTPLUG, /* on hotplug */
};
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 :|
3:58 a.m.
New subject: [libvirt] [PATCH 1/3] Make startmode optional in toplevel interface definitions.
On Fri, Oct 16, 2009 at 10:56:17AM -0400, laine(a)laine.org wrote:
From: Laine Stump <laine(a)laine.org>
The minimal XML returned from ncf_if_xml_state() doesn't contain this
attribute (which makes no sense in the case of reporting current
status of the interface), and it was preventing it from passing
through the parse/format step.
---
src/conf/interface_conf.c | 11 +++++------
src/conf/interface_conf.h | 3 ++-
2 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/src/conf/interface_conf.c b/src/conf/interface_conf.c
index e646351..540c931 100644
--- a/src/conf/interface_conf.c
+++ b/src/conf/interface_conf.c
@@ -128,12 +128,9 @@ virInterfaceDefParseStartMode(virConnectPtr conn, virInterfaceDefPtr
def,
char *tmp;
tmp = virXPathString(conn, "string(./start/@mode)", ctxt);
- if (tmp == NULL) {
- virInterfaceReportError(conn, VIR_ERR_XML_ERROR,
- "%s", _("interface misses the start mode
attribute"));
- return(-1);
- }
- if (STREQ(tmp, "onboot"))
+ if (tmp == NULL)
+ def->startmode = VIR_INTERFACE_START_UNSPECIFIED;
+ else if (STREQ(tmp, "onboot"))
def->startmode = VIR_INTERFACE_START_ONBOOT;
else if (STREQ(tmp, "hotplug"))
def->startmode = VIR_INTERFACE_START_HOTPLUG;
@@ -1039,6 +1036,8 @@ virInterfaceStartmodeDefFormat(virConnectPtr conn, virBufferPtr
buf,
enum virInterfaceStartMode startmode) {
const char *mode;
switch (startmode) {
+ case VIR_INTERFACE_START_UNSPECIFIED:
+ return 0;
case VIR_INTERFACE_START_NONE:
mode = "none";
break;
diff --git a/src/conf/interface_conf.h b/src/conf/interface_conf.h
index bb9dce4..20d9771 100644
--- a/src/conf/interface_conf.h
+++ b/src/conf/interface_conf.h
@@ -48,7 +48,8 @@ VIR_ENUM_DECL(virInterface)
/* types of start mode */
enum virInterfaceStartMode {
- VIR_INTERFACE_START_NONE = 0, /* not defined */
+ VIR_INTERFACE_START_UNSPECIFIED = 0, /* not given in config */
+ VIR_INTERFACE_START_NONE, /* specified as not defined */
VIR_INTERFACE_START_ONBOOT, /* startup at boot */
VIR_INTERFACE_START_HOTPLUG, /* on hotplug */
};
Okay, I usually prefer when enum numbering is not shuffled in patches,
but in that case that should have no consequence, so ACK,
I'm pushing this, thanks !
Daniel
--
Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/
daniel(a)veillard.com | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library http://libvirt.org/
9:56 a.m.
New subject: [libvirt] [PATCH 2/3] Support reporting live interface IP/netmask.
From: Laine Stump <laine(a)laine.org>
This patch adds the flag VIR_INTERFACE_XML_INACTIVE to
virInterfaceGetXMLDesc's flags. When it is*not* set (the default), the
live interface info will be returned in the XML (in particular, the IP
address(es) and netmask(s) will be retrieved by querying the interface
directly, rather than reporting what's in the config file). The
backend of this is in netcf's ncf_if_xml_state() function.
Note that after this patch is applied, netcf support will be omitted
from the build unless you have netcf-devel-0.1.3 or better installed.
---
configure.in | 2 +-
include/libvirt/libvirt.h.in | 4 +++
libvirt.spec.in | 2 +-
src/conf/interface_conf.c | 53 +++++++++++++++++++-----------------------
src/interface/netcf_driver.c | 8 ++++-
src/libvirt.c | 15 ++++++++---
tools/virsh.c | 10 ++++++-
7 files changed, 55 insertions(+), 39 deletions(-)
diff --git a/configure.in b/configure.in
index 2f9db72..3402b4f 100644
--- a/configure.in
+++ b/configure.in
@@ -33,7 +33,7 @@ GNUTLS_REQUIRED="1.0.25"
AVAHI_REQUIRED="0.6.0"
POLKIT_REQUIRED="0.6"
PARTED_REQUIRED="1.8.0"
-NETCF_REQUIRED="0.0.1"
+NETCF_REQUIRED="0.1.3"
dnl Checks for C compiler.
AC_PROG_CC
diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
index 6186d4e..7e75bee 100644
--- a/include/libvirt/libvirt.h.in
+++ b/include/libvirt/libvirt.h.in
@@ -930,6 +930,10 @@ virInterfacePtr virInterfaceLookupByMACString (virConnectPtr
conn,
const char* virInterfaceGetName (virInterfacePtr iface);
const char* virInterfaceGetMACString (virInterfacePtr iface);
+typedef enum {
+ VIR_INTERFACE_XML_INACTIVE = 1 /* dump inactive interface information */
+} virInterfaceXMLFlags;
+
char * virInterfaceGetXMLDesc (virInterfacePtr iface,
unsigned int flags);
virInterfacePtr virInterfaceDefineXML (virConnectPtr conn,
diff --git a/libvirt.spec.in b/libvirt.spec.in
index 6cd0888..32596d4 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -308,7 +308,7 @@ BuildRequires: libcap-ng-devel >= 0.5.0
BuildRequires: libssh2-devel
%endif
%if %{with_netcf}
-BuildRequires: netcf-devel
+BuildRequires: netcf-devel >= 0.1.3
%endif
# Fedora build root suckage
diff --git a/src/conf/interface_conf.c b/src/conf/interface_conf.c
index 540c931..d46f7ac 100644
--- a/src/conf/interface_conf.c
+++ b/src/conf/interface_conf.c
@@ -279,22 +279,19 @@ virInterfaceDefParseIp(virConnectPtr conn, virInterfaceDefPtr def,
static int
virInterfaceDefParseProtoIPv4(virConnectPtr conn, virInterfaceDefPtr def,
xmlXPathContextPtr ctxt) {
- xmlNodePtr cur;
- int ret;
+ xmlNodePtr dhcp, ip;
+ int ret = 0;
- cur = virXPathNode(conn, "./dhcp", ctxt);
- if (cur != NULL)
- ret = virInterfaceDefParseDhcp(conn, def, cur, ctxt);
- else {
- cur = virXPathNode(conn, "./ip", ctxt);
- if (cur != NULL)
- ret = virInterfaceDefParseIp(conn, def, cur, ctxt);
- else {
- virInterfaceReportError(conn, VIR_ERR_XML_ERROR,
- "%s", _("interface miss dhcp or ip
adressing"));
- ret = -1;
- }
- }
+ dhcp = virXPathNode(conn, "./dhcp", ctxt);
+ if (dhcp != NULL)
+ ret = virInterfaceDefParseDhcp(conn, def, dhcp, ctxt);
+
+ if (ret != 0)
+ return(ret);
+
+ ip = virXPathNode(conn, "./ip", ctxt);
+ if (ip != NULL)
+ ret = virInterfaceDefParseIp(conn, def, ip, ctxt);
return(ret);
}
@@ -1012,20 +1009,18 @@ virInterfaceProtocolDefFormat(virConnectPtr conn
ATTRIBUTE_UNUSED,
virBufferAddLit(buf, " <dhcp peerdns='yes'/>\n");
else
virBufferAddLit(buf, " <dhcp/>\n");
- } else {
- /* theorically if we don't have dhcp we should have an address */
- if (def->proto.address != NULL) {
- if (def->proto.prefix != 0)
- virBufferVSprintf(buf, " <ip address='%s'
prefix='%d'/>\n",
- def->proto.address, def->proto.prefix);
- else
- virBufferVSprintf(buf, " <ip
address='%s'/>\n",
- def->proto.address);
- }
- if (def->proto.gateway != NULL) {
- virBufferVSprintf(buf, " <route
gateway='%s'/>\n",
- def->proto.gateway);
- }
+ }
+ if (def->proto.address != NULL) {
+ if (def->proto.prefix != 0)
+ virBufferVSprintf(buf, " <ip address='%s'
prefix='%d'/>\n",
+ def->proto.address, def->proto.prefix);
+ else
+ virBufferVSprintf(buf, " <ip address='%s'/>\n",
+ def->proto.address);
+ }
+ if (def->proto.gateway != NULL) {
+ virBufferVSprintf(buf, " <route gateway='%s'/>\n",
+ def->proto.gateway);
}
virBufferAddLit(buf, " </protocol>\n");
return(0);
diff --git a/src/interface/netcf_driver.c b/src/interface/netcf_driver.c
index ca14fb0..b5c3664 100644
--- a/src/interface/netcf_driver.c
+++ b/src/interface/netcf_driver.c
@@ -326,7 +326,7 @@ cleanup:
}
static char *interfaceGetXMLDesc(virInterfacePtr ifinfo,
- unsigned int flags ATTRIBUTE_UNUSED)
+ unsigned int flags)
{
struct interface_driver *driver = ifinfo->conn->interfacePrivateData;
struct netcf_if *iface = NULL;
@@ -342,7 +342,11 @@ static char *interfaceGetXMLDesc(virInterfacePtr ifinfo,
goto cleanup;
}
- xmlstr = ncf_if_xml_desc(iface);
+ if ((flags & VIR_INTERFACE_XML_INACTIVE)) {
+ xmlstr = ncf_if_xml_desc(iface);
+ } else {
+ xmlstr = ncf_if_xml_state(iface);
+ }
if (!xmlstr) {
const char *errmsg, *details;
int errcode = ncf_error(driver->netcf, &errmsg, &details);
diff --git a/src/libvirt.c b/src/libvirt.c
index 92a1eaa..c7d6bcc 100644
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -6198,10 +6198,17 @@ virInterfaceGetMACString(virInterfacePtr iface)
/**
* virInterfaceGetXMLDesc:
* @iface: an interface object
- * @flags: an OR'ed set of extraction flags, not used yet
+ * @flags: an OR'ed set of extraction flags. Current valid bits:
+ *
+ * VIR_INTERFACE_XML_INACTIVE - return the static configuration,
+ * suitable for use redefining the
+ * interface via virInterfaceDefineXML()
*
- * Provide an XML description of the interface. The description may be reused
- * later to redefine the interface with virInterfaceDefineXML().
+ * Provide an XML description of the interface. If
+ * VIR_INTERFACE_XML_INACTIVE is set, the description may be reused
+ * later to redefine the interface with virInterfaceDefineXML(). If it
+ * is not set, the ip address and netmask will be the current live
+ * setting of the interface, not the settings from the config files.
*
* Returns a 0 terminated UTF-8 encoded XML instance, or NULL in case of error.
* the caller must free() the returned value.
@@ -6218,7 +6225,7 @@ virInterfaceGetXMLDesc(virInterfacePtr iface, unsigned int flags)
virLibInterfaceError(NULL, VIR_ERR_INVALID_INTERFACE, __FUNCTION__);
return (NULL);
}
- if (flags != 0) {
+ if ((flags & ~VIR_INTERFACE_XML_INACTIVE) != 0) {
virLibInterfaceError(iface, VIR_ERR_INVALID_ARG, __FUNCTION__);
goto error;
}
diff --git a/tools/virsh.c b/tools/virsh.c
index 47122d5..322de30 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -2792,7 +2792,7 @@ cmdInterfaceEdit (vshControl *ctl, const vshCmd *cmd)
char *doc = NULL;
char *doc_edited = NULL;
char *doc_reread = NULL;
- int flags = 0;
+ int flags = VIR_INTERFACE_XML_INACTIVE;
if (!vshConnectionUsability(ctl, ctl->conn, TRUE))
goto cleanup;
@@ -3326,6 +3326,7 @@ static const vshCmdInfo info_interface_dumpxml[] = {
static const vshCmdOptDef opts_interface_dumpxml[] = {
{"interface", VSH_OT_DATA, VSH_OFLAG_REQ, gettext_noop("interface name
or MAC address")},
+ {"inactive", VSH_OT_BOOL, 0, gettext_noop("show inactive defined
XML")},
{NULL, 0, 0, NULL}
};
@@ -3335,6 +3336,11 @@ cmdInterfaceDumpXML(vshControl *ctl, const vshCmd *cmd)
virInterfacePtr iface;
int ret = TRUE;
char *dump;
+ int flags = 0;
+ int inactive = vshCommandOptBool(cmd, "inactive");
+
+ if (inactive)
+ flags |= VIR_INTERFACE_XML_INACTIVE;
if (!vshConnectionUsability(ctl, ctl->conn, TRUE))
return FALSE;
@@ -3342,7 +3348,7 @@ cmdInterfaceDumpXML(vshControl *ctl, const vshCmd *cmd)
if (!(iface = vshCommandOptInterface(ctl, cmd, NULL)))
return FALSE;
- dump = virInterfaceGetXMLDesc(iface, 0);
+ dump = virInterfaceGetXMLDesc(iface, flags);
if (dump != NULL) {
printf("%s", dump);
free(dump);
--
1.6.5.15.gc274d
11:22 a.m.
New subject: [libvirt] [PATCH 2/3] Support reporting live interface IP/netmask.
On Fri, Oct 16, 2009 at 10:56:18AM -0400, laine(a)laine.org wrote:
From: Laine Stump <laine(a)laine.org>
This patch adds the flag VIR_INTERFACE_XML_INACTIVE to
virInterfaceGetXMLDesc's flags. When it is*not* set (the default), the
live interface info will be returned in the XML (in particular, the IP
address(es) and netmask(s) will be retrieved by querying the interface
directly, rather than reporting what's in the config file). The
backend of this is in netcf's ncf_if_xml_state() function.
Note that after this patch is applied, netcf support will be omitted
from the build unless you have netcf-devel-0.1.3 or better installed.
---
configure.in | 2 +-
include/libvirt/libvirt.h.in | 4 +++
libvirt.spec.in | 2 +-
src/conf/interface_conf.c | 53 +++++++++++++++++++-----------------------
src/interface/netcf_driver.c | 8 ++++-
src/libvirt.c | 15 ++++++++---
tools/virsh.c | 10 ++++++-
7 files changed, 55 insertions(+), 39 deletions(-)
diff --git a/configure.in b/configure.in
index 2f9db72..3402b4f 100644
--- a/configure.in
+++ b/configure.in
@@ -33,7 +33,7 @@ GNUTLS_REQUIRED="1.0.25"
AVAHI_REQUIRED="0.6.0"
POLKIT_REQUIRED="0.6"
PARTED_REQUIRED="1.8.0"
-NETCF_REQUIRED="0.0.1"
+NETCF_REQUIRED="0.1.3"
dnl Checks for C compiler.
AC_PROG_CC
diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
index 6186d4e..7e75bee 100644
--- a/include/libvirt/libvirt.h.in
+++ b/include/libvirt/libvirt.h.in
@@ -930,6 +930,10 @@ virInterfacePtr virInterfaceLookupByMACString (virConnectPtr
conn,
const char* virInterfaceGetName (virInterfacePtr iface);
const char* virInterfaceGetMACString (virInterfacePtr iface);
+typedef enum {
+ VIR_INTERFACE_XML_INACTIVE = 1 /* dump inactive interface information */
+} virInterfaceXMLFlags;
+
char * virInterfaceGetXMLDesc (virInterfacePtr iface,
unsigned int flags);
virInterfacePtr virInterfaceDefineXML (virConnectPtr conn,
diff --git a/libvirt.spec.in b/libvirt.spec.in
index 6cd0888..32596d4 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -308,7 +308,7 @@ BuildRequires: libcap-ng-devel >= 0.5.0
BuildRequires: libssh2-devel
%endif
%if %{with_netcf}
-BuildRequires: netcf-devel
+BuildRequires: netcf-devel >= 0.1.3
%endif
# Fedora build root suckage
diff --git a/src/conf/interface_conf.c b/src/conf/interface_conf.c
index 540c931..d46f7ac 100644
--- a/src/conf/interface_conf.c
+++ b/src/conf/interface_conf.c
@@ -279,22 +279,19 @@ virInterfaceDefParseIp(virConnectPtr conn, virInterfaceDefPtr def,
static int
virInterfaceDefParseProtoIPv4(virConnectPtr conn, virInterfaceDefPtr def,
xmlXPathContextPtr ctxt) {
- xmlNodePtr cur;
- int ret;
+ xmlNodePtr dhcp, ip;
+ int ret = 0;
- cur = virXPathNode(conn, "./dhcp", ctxt);
- if (cur != NULL)
- ret = virInterfaceDefParseDhcp(conn, def, cur, ctxt);
- else {
- cur = virXPathNode(conn, "./ip", ctxt);
- if (cur != NULL)
- ret = virInterfaceDefParseIp(conn, def, cur, ctxt);
- else {
- virInterfaceReportError(conn, VIR_ERR_XML_ERROR,
- "%s", _("interface miss dhcp or ip
adressing"));
- ret = -1;
- }
- }
+ dhcp = virXPathNode(conn, "./dhcp", ctxt);
+ if (dhcp != NULL)
+ ret = virInterfaceDefParseDhcp(conn, def, dhcp, ctxt);
+
+ if (ret != 0)
+ return(ret);
+
+ ip = virXPathNode(conn, "./ip", ctxt);
+ if (ip != NULL)
+ ret = virInterfaceDefParseIp(conn, def, ip, ctxt);
return(ret);
}
@@ -1012,20 +1009,18 @@ virInterfaceProtocolDefFormat(virConnectPtr conn
ATTRIBUTE_UNUSED,
virBufferAddLit(buf, " <dhcp
peerdns='yes'/>\n");
else
virBufferAddLit(buf, " <dhcp/>\n");
- } else {
- /* theorically if we don't have dhcp we should have an address */
- if (def->proto.address != NULL) {
- if (def->proto.prefix != 0)
- virBufferVSprintf(buf, " <ip address='%s'
prefix='%d'/>\n",
- def->proto.address, def->proto.prefix);
- else
- virBufferVSprintf(buf, " <ip
address='%s'/>\n",
- def->proto.address);
- }
- if (def->proto.gateway != NULL) {
- virBufferVSprintf(buf, " <route
gateway='%s'/>\n",
- def->proto.gateway);
- }
+ }
+ if (def->proto.address != NULL) {
+ if (def->proto.prefix != 0)
+ virBufferVSprintf(buf, " <ip address='%s'
prefix='%d'/>\n",
+ def->proto.address, def->proto.prefix);
+ else
+ virBufferVSprintf(buf, " <ip address='%s'/>\n",
+ def->proto.address);
+ }
+ if (def->proto.gateway != NULL) {
+ virBufferVSprintf(buf, " <route gateway='%s'/>\n",
+ def->proto.gateway);
}
virBufferAddLit(buf, " </protocol>\n");
return(0);
diff --git a/src/interface/netcf_driver.c b/src/interface/netcf_driver.c
index ca14fb0..b5c3664 100644
--- a/src/interface/netcf_driver.c
+++ b/src/interface/netcf_driver.c
@@ -326,7 +326,7 @@ cleanup:
}
static char *interfaceGetXMLDesc(virInterfacePtr ifinfo,
- unsigned int flags ATTRIBUTE_UNUSED)
+ unsigned int flags)
{
struct interface_driver *driver = ifinfo->conn->interfacePrivateData;
struct netcf_if *iface = NULL;
@@ -342,7 +342,11 @@ static char *interfaceGetXMLDesc(virInterfacePtr ifinfo,
goto cleanup;
}
- xmlstr = ncf_if_xml_desc(iface);
+ if ((flags & VIR_INTERFACE_XML_INACTIVE)) {
+ xmlstr = ncf_if_xml_desc(iface);
+ } else {
+ xmlstr = ncf_if_xml_state(iface);
+ }
if (!xmlstr) {
const char *errmsg, *details;
int errcode = ncf_error(driver->netcf, &errmsg, &details);
diff --git a/src/libvirt.c b/src/libvirt.c
index 92a1eaa..c7d6bcc 100644
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -6198,10 +6198,17 @@ virInterfaceGetMACString(virInterfacePtr iface)
/**
* virInterfaceGetXMLDesc:
* @iface: an interface object
- * @flags: an OR'ed set of extraction flags, not used yet
+ * @flags: an OR'ed set of extraction flags. Current valid bits:
+ *
+ * VIR_INTERFACE_XML_INACTIVE - return the static configuration,
+ * suitable for use redefining the
+ * interface via virInterfaceDefineXML()
*
- * Provide an XML description of the interface. The description may be reused
- * later to redefine the interface with virInterfaceDefineXML().
+ * Provide an XML description of the interface. If
+ * VIR_INTERFACE_XML_INACTIVE is set, the description may be reused
+ * later to redefine the interface with virInterfaceDefineXML(). If it
+ * is not set, the ip address and netmask will be the current live
+ * setting of the interface, not the settings from the config files.
*
* Returns a 0 terminated UTF-8 encoded XML instance, or NULL in case of error.
* the caller must free() the returned value.
@@ -6218,7 +6225,7 @@ virInterfaceGetXMLDesc(virInterfacePtr iface, unsigned int flags)
virLibInterfaceError(NULL, VIR_ERR_INVALID_INTERFACE, __FUNCTION__);
return (NULL);
}
- if (flags != 0) {
+ if ((flags & ~VIR_INTERFACE_XML_INACTIVE) != 0) {
virLibInterfaceError(iface, VIR_ERR_INVALID_ARG, __FUNCTION__);
goto error;
}
diff --git a/tools/virsh.c b/tools/virsh.c
index 47122d5..322de30 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -2792,7 +2792,7 @@ cmdInterfaceEdit (vshControl *ctl, const vshCmd *cmd)
char *doc = NULL;
char *doc_edited = NULL;
char *doc_reread = NULL;
- int flags = 0;
+ int flags = VIR_INTERFACE_XML_INACTIVE;
if (!vshConnectionUsability(ctl, ctl->conn, TRUE))
goto cleanup;
@@ -3326,6 +3326,7 @@ static const vshCmdInfo info_interface_dumpxml[] = {
static const vshCmdOptDef opts_interface_dumpxml[] = {
{"interface", VSH_OT_DATA, VSH_OFLAG_REQ, gettext_noop("interface
name or MAC address")},
+ {"inactive", VSH_OT_BOOL, 0, gettext_noop("show inactive defined
XML")},
{NULL, 0, 0, NULL}
};
@@ -3335,6 +3336,11 @@ cmdInterfaceDumpXML(vshControl *ctl, const vshCmd *cmd)
virInterfacePtr iface;
int ret = TRUE;
char *dump;
+ int flags = 0;
+ int inactive = vshCommandOptBool(cmd, "inactive");
+
+ if (inactive)
+ flags |= VIR_INTERFACE_XML_INACTIVE;
if (!vshConnectionUsability(ctl, ctl->conn, TRUE))
return FALSE;
@@ -3342,7 +3348,7 @@ cmdInterfaceDumpXML(vshControl *ctl, const vshCmd *cmd)
if (!(iface = vshCommandOptInterface(ctl, cmd, NULL)))
return FALSE;
- dump = virInterfaceGetXMLDesc(iface, 0);
+ dump = virInterfaceGetXMLDesc(iface, flags);
if (dump != NULL) {
printf("%s", dump);
free(dump);
--
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 :|
4:48 a.m.
New subject: [libvirt] [PATCH 2/3] Support reporting live interface IP/netmask.
On Fri, Oct 16, 2009 at 10:56:18AM -0400, laine(a)laine.org wrote:
From: Laine Stump <laine(a)laine.org>
This patch adds the flag VIR_INTERFACE_XML_INACTIVE to
virInterfaceGetXMLDesc's flags. When it is*not* set (the default), the
live interface info will be returned in the XML (in particular, the IP
address(es) and netmask(s) will be retrieved by querying the interface
directly, rather than reporting what's in the config file). The
backend of this is in netcf's ncf_if_xml_state() function.
Okay, looks fine, applied.
Note that after this patch is applied, netcf support will be omitted
from the build unless you have netcf-devel-0.1.3 or better installed.
yup, it's not worth messing the code to handle older releases at this
point I guess. The only problem will be with people using F-12 or
versions based on netcf 0.1.x x < 3 willing to recompile a newer version
and loosing the interface functionality.
Daniel
--
Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/
daniel(a)veillard.com | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library http://libvirt.org/
9:56 a.m.
New subject: [libvirt] [PATCH 3/3] Support for IPv6 / multiple addresses per interface in virInterface
From: Laine Stump <laine(a)laine.org>
This patch updates the xml parsing and formatting, and the associated
virInterfaceDef data structure to support IPv6, along the way adding
support for multiple protocols per interface, and multiple IP
addresses per protocol.
---
src/conf/interface_conf.c | 292 ++++++++++++++++++++++++++++++++++----------
src/conf/interface_conf.h | 19 ++-
2 files changed, 239 insertions(+), 72 deletions(-)
diff --git a/src/conf/interface_conf.c b/src/conf/interface_conf.c
index d46f7ac..ae0c0cb 100644
--- a/src/conf/interface_conf.c
+++ b/src/conf/interface_conf.c
@@ -53,9 +53,31 @@ void virInterfaceBareDefFree(virInterfaceBareDefPtr def) {
VIR_FREE(def);
}
+static
+void virInterfaceIpDefFree(virInterfaceIpDefPtr def) {
+ if (def == NULL)
+ return;
+ VIR_FREE(def->address);
+}
+
+static
+void virInterfaceProtocolDefFree(virInterfaceProtocolDefPtr def) {
+ int ii;
+
+ if (def == NULL)
+ return;
+ for (ii = 0; ii < def->nips; ii++) {
+ virInterfaceIpDefFree(def->ips[ii]);
+ }
+ VIR_FREE(def->ips);
+ VIR_FREE(def->family);
+ VIR_FREE(def->gateway);
+ VIR_FREE(def);
+}
+
void virInterfaceDefFree(virInterfaceDefPtr def)
{
- int i;
+ int i, pp;
if (def == NULL)
return;
@@ -89,10 +111,11 @@ void virInterfaceDefFree(virInterfaceDefPtr def)
break;
}
- VIR_FREE(def->proto.family);
- VIR_FREE(def->proto.address);
- VIR_FREE(def->proto.gateway);
-
+ /* free all protos */
+ for (pp = 0; pp < def->nprotos; pp++) {
+ virInterfaceProtocolDefFree(def->protos[pp]);
+ }
+ VIR_FREE(def->protos);
VIR_FREE(def);
}
@@ -222,22 +245,22 @@ virInterfaceDefParseBondArpValid(virConnectPtr conn,
xmlXPathContextPtr ctxt) {
}
static int
-virInterfaceDefParseDhcp(virConnectPtr conn, virInterfaceDefPtr def,
+virInterfaceDefParseDhcp(virConnectPtr conn, virInterfaceProtocolDefPtr def,
xmlNodePtr dhcp, xmlXPathContextPtr ctxt) {
xmlNodePtr save;
char *tmp;
int ret = 0;
- def->proto.dhcp = 1;
+ def->dhcp = 1;
save = ctxt->node;
ctxt->node = dhcp;
/* Not much to do in the current version */
tmp = virXPathString(conn, "string(./@peerdns)", ctxt);
if (tmp) {
if (STREQ(tmp, "yes"))
- def->proto.peerdns = 1;
+ def->peerdns = 1;
else if (STREQ(tmp, "no"))
- def->proto.peerdns = 0;
+ def->peerdns = 0;
else {
virInterfaceReportError(conn, VIR_ERR_XML_ERROR,
_("unknown dhcp peerdns value %s"), tmp);
@@ -245,86 +268,207 @@ virInterfaceDefParseDhcp(virConnectPtr conn, virInterfaceDefPtr
def,
}
VIR_FREE(tmp);
} else
- def->proto.peerdns = -1;
+ def->peerdns = -1;
ctxt->node = save;
return(ret);
}
static int
-virInterfaceDefParseIp(virConnectPtr conn, virInterfaceDefPtr def,
- xmlNodePtr ip ATTRIBUTE_UNUSED, xmlXPathContextPtr ctxt) {
+virInterfaceDefParseIp(virConnectPtr conn, virInterfaceIpDefPtr def,
+ xmlXPathContextPtr ctxt) {
int ret = 0;
char *tmp;
long l;
- tmp = virXPathString(conn, "string(./ip[1]/@address)", ctxt);
- def->proto.address = tmp;
+ tmp = virXPathString(conn, "string(./@address)", ctxt);
+ def->address = tmp;
if (tmp != NULL) {
- ret = virXPathLong(conn, "string(./ip[1]/@prefix)", ctxt, &l);
+ ret = virXPathLong(conn, "string(./@prefix)", ctxt, &l);
if (ret == 0)
- def->proto.prefix = (int) l;
+ def->prefix = (int) l;
else if (ret == -2) {
virInterfaceReportError(conn, VIR_ERR_XML_ERROR,
"%s", _("Invalid ip address prefix
value"));
return(-1);
}
}
- tmp = virXPathString(conn, "string(./route[1]/@gateway)", ctxt);
- def->proto.gateway = tmp;
return(0);
}
static int
-virInterfaceDefParseProtoIPv4(virConnectPtr conn, virInterfaceDefPtr def,
+virInterfaceDefParseProtoIPv4(virConnectPtr conn, virInterfaceProtocolDefPtr def,
xmlXPathContextPtr ctxt) {
- xmlNodePtr dhcp, ip;
- int ret = 0;
+ xmlNodePtr dhcp;
+ xmlNodePtr *ipNodes = NULL;
+ int nIpNodes, ii, ret = 0;
+ char *tmp;
+
+ tmp = virXPathString(conn, "string(./route[1]/@gateway)", ctxt);
+ def->gateway = tmp;
dhcp = virXPathNode(conn, "./dhcp", ctxt);
if (dhcp != NULL)
ret = virInterfaceDefParseDhcp(conn, def, dhcp, ctxt);
+ if (ret != 0)
+ return(ret);
+
+ nIpNodes = virXPathNodeSet(conn, "./ip", ctxt, &ipNodes);
+ if (ipNodes == NULL)
+ return 0;
+
+ if (VIR_ALLOC_N(def->ips, nIpNodes) < 0) {
+ virReportOOMError(conn);
+ ret = -1;
+ goto error;
+ }
+
+ def->nips = 0;
+ for (ii = 0; ii < nIpNodes; ii++) {
+ virInterfaceIpDefPtr ip;
+
+ if (VIR_ALLOC(ip) < 0) {
+ virReportOOMError(conn);
+ break;
+ }
+
+ ctxt->node = ipNodes[ii];
+ ret = virInterfaceDefParseIp(conn, ip, ctxt);
+ if (ret != 0) {
+ virInterfaceIpDefFree(ip);
+ break;
+ }
+ def->ips[def->nips++] = ip;
+ }
+
+error:
+ VIR_FREE(ipNodes);
+ return(ret);
+}
+
+static int
+virInterfaceDefParseProtoIPv6(virConnectPtr conn, virInterfaceProtocolDefPtr def,
+ xmlXPathContextPtr ctxt) {
+ xmlNodePtr dhcp, autoconf;
+ xmlNodePtr *ipNodes = NULL;
+ int nIpNodes, ii, ret = 0;
+ char *tmp;
+
+ tmp = virXPathString(conn, "string(./route[1]/@gateway)", ctxt);
+ def->gateway = tmp;
+
+ autoconf = virXPathNode(conn, "./autoconf", ctxt);
+ if (autoconf != NULL)
+ def->autoconf = 1;
+
+ dhcp = virXPathNode(conn, "./dhcp", ctxt);
+ if (dhcp != NULL)
+ ret = virInterfaceDefParseDhcp(conn, def, dhcp, ctxt);
if (ret != 0)
return(ret);
- ip = virXPathNode(conn, "./ip", ctxt);
- if (ip != NULL)
- ret = virInterfaceDefParseIp(conn, def, ip, ctxt);
+ nIpNodes = virXPathNodeSet(conn, "./ip", ctxt, &ipNodes);
+ if (ipNodes == NULL)
+ return 0;
+
+ if (VIR_ALLOC_N(def->ips, nIpNodes) < 0) {
+ virReportOOMError(conn);
+ ret = -1;
+ goto error;
+ }
+
+ def->nips = 0;
+ for (ii = 0; ii < nIpNodes; ii++) {
+
+ virInterfaceIpDefPtr ip;
+
+ if (VIR_ALLOC(ip) < 0) {
+ virReportOOMError(conn);
+ break;
+ }
+
+ ctxt->node = ipNodes[ii];
+ ret = virInterfaceDefParseIp(conn, ip, ctxt);
+ if (ret != 0) {
+ virInterfaceIpDefFree(ip);
+ break;
+ }
+ def->ips[def->nips++] = ip;
+ }
+
+error:
+ VIR_FREE(ipNodes);
return(ret);
}
static int
virInterfaceDefParseIfAdressing(virConnectPtr conn, virInterfaceDefPtr def,
xmlXPathContextPtr ctxt) {
- xmlNodePtr cur, save;
- int ret;
+ xmlNodePtr save;
+ xmlNodePtr *protoNodes = NULL;
+ int nProtoNodes, pp, ret = 0;
char *tmp;
- cur = virXPathNode(conn, "./protocol[1]", ctxt);
- if (cur == NULL)
- return(0);
save = ctxt->node;
- ctxt->node = cur;
- tmp = virXPathString(conn, "string(./@family)", ctxt);
- if (tmp == NULL) {
- virInterfaceReportError(conn, VIR_ERR_XML_ERROR,
- "%s", _("protocol misses the family
attribute"));
- ret = -1;
- goto done;
+
+ nProtoNodes = virXPathNodeSet(conn, "./protocol", ctxt, &protoNodes);
+ if (nProtoNodes <= 0) {
+ /* no protocols is an acceptable outcome */
+ return 0;
}
- if (STREQ(tmp, "ipv4")) {
- def->proto.family = tmp;
- ret = virInterfaceDefParseProtoIPv4(conn, def, ctxt);
- } else {
- virInterfaceReportError(conn, VIR_ERR_XML_ERROR,
- _("unsupported protocol family '%s'"),
tmp);
+
+ if (VIR_ALLOC_N(def->protos, nProtoNodes) < 0) {
+ virReportOOMError(conn);
ret = -1;
- VIR_FREE(tmp);
+ goto error;
}
-done:
+ def->nprotos = 0;
+ for (pp = 0; pp < nProtoNodes; pp++) {
+
+ virInterfaceProtocolDefPtr proto;
+
+ if (VIR_ALLOC(proto) < 0) {
+ virReportOOMError(conn);
+ break;
+ }
+
+ ctxt->node = protoNodes[pp];
+ tmp = virXPathString(conn, "string(./@family)", ctxt);
+ if (tmp == NULL) {
+ virInterfaceReportError(conn, VIR_ERR_XML_ERROR,
+ "%s", _("protocol misses the family
attribute"));
+ virInterfaceProtocolDefFree(proto);
+ ret = -1;
+ break;
+ }
+ proto->family = tmp;
+ if (STREQ(tmp, "ipv4")) {
+ ret = virInterfaceDefParseProtoIPv4(conn, proto, ctxt);
+ if (ret != 0) {
+ virInterfaceProtocolDefFree(proto);
+ break;
+ }
+ } else if (STREQ(tmp, "ipv6")) {
+ ret = virInterfaceDefParseProtoIPv6(conn, proto, ctxt);
+ if (ret != 0) {
+ virInterfaceProtocolDefFree(proto);
+ break;
+ }
+ } else {
+ virInterfaceReportError(conn, VIR_ERR_XML_ERROR,
+ _("unsupported protocol family
'%s'"), tmp);
+ ret = -1;
+ virInterfaceProtocolDefFree(proto);
+ break;
+ }
+ def->protos[def->nprotos++] = proto;
+ }
+
+error:
+ VIR_FREE(protoNodes);
ctxt->node = save;
return(ret);
@@ -999,30 +1143,44 @@ virInterfaceVlanDefFormat(virConnectPtr conn, virBufferPtr buf,
static int
virInterfaceProtocolDefFormat(virConnectPtr conn ATTRIBUTE_UNUSED,
virBufferPtr buf, const virInterfaceDefPtr def) {
- if (def->proto.family == NULL)
- return(0);
- virBufferVSprintf(buf, " <protocol family='%s'>\n",
def->proto.family);
- if (def->proto.dhcp) {
- if (def->proto.peerdns == 0)
- virBufferAddLit(buf, " <dhcp peerdns='no'/>\n");
- else if (def->proto.peerdns == 1)
- virBufferAddLit(buf, " <dhcp peerdns='yes'/>\n");
- else
- virBufferAddLit(buf, " <dhcp/>\n");
- }
- if (def->proto.address != NULL) {
- if (def->proto.prefix != 0)
- virBufferVSprintf(buf, " <ip address='%s'
prefix='%d'/>\n",
- def->proto.address, def->proto.prefix);
- else
- virBufferVSprintf(buf, " <ip address='%s'/>\n",
- def->proto.address);
- }
- if (def->proto.gateway != NULL) {
- virBufferVSprintf(buf, " <route gateway='%s'/>\n",
- def->proto.gateway);
+ int pp, ii;
+
+ for (pp = 0; pp < def->nprotos; pp++) {
+
+ virBufferVSprintf(buf, " <protocol family='%s'>\n",
def->protos[pp]->family);
+
+ if (def->protos[pp]->autoconf) {
+ virBufferAddLit(buf, " <autoconf/>\n");
+ }
+
+ if (def->protos[pp]->dhcp) {
+ if (def->protos[pp]->peerdns == 0)
+ virBufferAddLit(buf, " <dhcp
peerdns='no'/>\n");
+ else if (def->protos[pp]->peerdns == 1)
+ virBufferAddLit(buf, " <dhcp
peerdns='yes'/>\n");
+ else
+ virBufferAddLit(buf, " <dhcp/>\n");
+ }
+
+ for (ii = 0; ii < def->protos[pp]->nips; ii++) {
+ if (def->protos[pp]->ips[ii]->address != NULL) {
+
+ virBufferVSprintf(buf, " <ip address='%s'",
+ def->protos[pp]->ips[ii]->address);
+ if (def->protos[pp]->ips[ii]->prefix != 0) {
+ virBufferVSprintf(buf, " prefix='%d'",
+ def->protos[pp]->ips[ii]->prefix);
+ }
+ virBufferAddLit(buf, "/>\n");
+ }
+ }
+ if (def->protos[pp]->gateway != NULL) {
+ virBufferVSprintf(buf, " <route
gateway='%s'/>\n",
+ def->protos[pp]->gateway);
+ }
+
+ virBufferAddLit(buf, " </protocol>\n");
}
- virBufferAddLit(buf, " </protocol>\n");
return(0);
}
diff --git a/src/conf/interface_conf.h b/src/conf/interface_conf.h
index 20d9771..c5f1023 100644
--- a/src/conf/interface_conf.h
+++ b/src/conf/interface_conf.h
@@ -124,14 +124,23 @@ struct _virInterfaceVlanDef {
char *devname; /* device name for vlan */
};
+typedef struct _virInterfaceIpDef virInterfaceIpDef;
+typedef virInterfaceIpDef *virInterfaceIpDefPtr;
+struct _virInterfaceIpDef {
+ char *address; /* ip address */
+ int prefix; /* ip prefix */
+};
+
+
typedef struct _virInterfaceProtocolDef virInterfaceProtocolDef;
typedef virInterfaceProtocolDef *virInterfaceProtocolDefPtr;
struct _virInterfaceProtocolDef {
- char *family; /* ipv4 only right now */
+ char *family; /* ipv4 or ipv6 */
int dhcp; /* use dhcp */
int peerdns; /* dhcp peerdns ? */
- char *address; /* ip address */
- int prefix; /* ip prefix */
+ int autoconf; /* only useful if family is ipv6 */
+ int nips;
+ virInterfaceIpDefPtr *ips; /* ptr to array of ips[nips] */
char *gateway; /* route gateway */
};
@@ -152,8 +161,8 @@ struct _virInterfaceDef {
virInterfaceBondDef bond;
} data;
- /* separated as we may allow multiple of those in the future */
- virInterfaceProtocolDef proto;
+ int nprotos;
+ virInterfaceProtocolDefPtr *protos; /* ptr to array of protos[nprotos] */
};
typedef struct _virInterfaceObj virInterfaceObj;
--
1.6.5.15.gc274d
11:27 a.m.
New subject: [libvirt] [PATCH 3/3] Support for IPv6 / multiple addresses per interface in virInterface
On Fri, Oct 16, 2009 at 10:56:19AM -0400, laine(a)laine.org wrote:
static int
-virInterfaceDefParseProtoIPv4(virConnectPtr conn, virInterfaceDefPtr def,
+virInterfaceDefParseProtoIPv4(virConnectPtr conn, virInterfaceProtocolDefPtr def,
xmlXPathContextPtr ctxt) {
- xmlNodePtr dhcp, ip;
- int ret = 0;
+ xmlNodePtr dhcp;
+ xmlNodePtr *ipNodes = NULL;
+ int nIpNodes, ii, ret = 0;
+ char *tmp;
+
+ tmp = virXPathString(conn, "string(./route[1]/@gateway)", ctxt);
+ def->gateway = tmp;
dhcp = virXPathNode(conn, "./dhcp", ctxt);
if (dhcp != NULL)
ret = virInterfaceDefParseDhcp(conn, def, dhcp, ctxt);
+ if (ret != 0)
+ return(ret);
+
+ nIpNodes = virXPathNodeSet(conn, "./ip", ctxt, &ipNodes);
+ if (ipNodes == NULL)
+ return 0;
+
+ if (VIR_ALLOC_N(def->ips, nIpNodes) < 0) {
+ virReportOOMError(conn);
+ ret = -1;
+ goto error;
+ }
+
+ def->nips = 0;
+ for (ii = 0; ii < nIpNodes; ii++) {
+ virInterfaceIpDefPtr ip;
+
+ if (VIR_ALLOC(ip) < 0) {
+ virReportOOMError(conn);
+ break;
+ }
+
+ ctxt->node = ipNodes[ii];
+ ret = virInterfaceDefParseIp(conn, ip, ctxt);
+ if (ret != 0) {
+ virInterfaceIpDefFree(ip);
+ break;
+ }
+ def->ips[def->nips++] = ip;
+ }
+
+error:
+ VIR_FREE(ipNodes);
+ return(ret);
+}
I don't think the error reporting is quite right here. At the time of
declaration you initialize 'ret = 0', so those two error cases inside
the for loop which break out will end up returning 0. I think it is
better to initialize 'ret = -1', change the 'break' to 'goto
error',
and after the for loop put 'ret = 0' to mark success.
+
+static int
+virInterfaceDefParseProtoIPv6(virConnectPtr conn, virInterfaceProtocolDefPtr def,
+ xmlXPathContextPtr ctxt) {
+ xmlNodePtr dhcp, autoconf;
+ xmlNodePtr *ipNodes = NULL;
+ int nIpNodes, ii, ret = 0;
+ char *tmp;
+
+ tmp = virXPathString(conn, "string(./route[1]/@gateway)", ctxt);
+ def->gateway = tmp;
+
+ autoconf = virXPathNode(conn, "./autoconf", ctxt);
+ if (autoconf != NULL)
+ def->autoconf = 1;
+
+ dhcp = virXPathNode(conn, "./dhcp", ctxt);
+ if (dhcp != NULL)
+ ret = virInterfaceDefParseDhcp(conn, def, dhcp, ctxt);
if (ret != 0)
return(ret);
- ip = virXPathNode(conn, "./ip", ctxt);
- if (ip != NULL)
- ret = virInterfaceDefParseIp(conn, def, ip, ctxt);
+ nIpNodes = virXPathNodeSet(conn, "./ip", ctxt, &ipNodes);
+ if (ipNodes == NULL)
+ return 0;
+
+ if (VIR_ALLOC_N(def->ips, nIpNodes) < 0) {
+ virReportOOMError(conn);
+ ret = -1;
+ goto error;
+ }
+
+ def->nips = 0;
+ for (ii = 0; ii < nIpNodes; ii++) {
+
+ virInterfaceIpDefPtr ip;
+
+ if (VIR_ALLOC(ip) < 0) {
+ virReportOOMError(conn);
+ break;
+ }
+
+ ctxt->node = ipNodes[ii];
+ ret = virInterfaceDefParseIp(conn, ip, ctxt);
+ if (ret != 0) {
+ virInterfaceIpDefFree(ip);
+ break;
+ }
+ def->ips[def->nips++] = ip;
+ }
+
+error:
+ VIR_FREE(ipNodes);
return(ret);
}
Same here - that looks like it'll return 0 for several error
cases.
Regards,
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 :|
12:31 p.m.
New subject: [libvirt] [PATCH 3/3] (REVISED) Support for IPv6 / multiple addresses per interface in virInterface
(How's this?)
This patch updates the xml parsing and formatting, and the associated
virInterfaceDef data structure to support IPv6, along the way adding
support for multiple protocols per interface, and multiple IP
addresses per protocol.
---
src/conf/interface_conf.c | 295 ++++++++++++++++++++++++++++++++++-----------
src/conf/interface_conf.h | 19 ++-
2 files changed, 241 insertions(+), 73 deletions(-)
diff --git a/src/conf/interface_conf.c b/src/conf/interface_conf.c
index d46f7ac..7cb71ed 100644
--- a/src/conf/interface_conf.c
+++ b/src/conf/interface_conf.c
@@ -53,9 +53,31 @@ void virInterfaceBareDefFree(virInterfaceBareDefPtr def) {
VIR_FREE(def);
}
+static
+void virInterfaceIpDefFree(virInterfaceIpDefPtr def) {
+ if (def == NULL)
+ return;
+ VIR_FREE(def->address);
+}
+
+static
+void virInterfaceProtocolDefFree(virInterfaceProtocolDefPtr def) {
+ int ii;
+
+ if (def == NULL)
+ return;
+ for (ii = 0; ii < def->nips; ii++) {
+ virInterfaceIpDefFree(def->ips[ii]);
+ }
+ VIR_FREE(def->ips);
+ VIR_FREE(def->family);
+ VIR_FREE(def->gateway);
+ VIR_FREE(def);
+}
+
void virInterfaceDefFree(virInterfaceDefPtr def)
{
- int i;
+ int i, pp;
if (def == NULL)
return;
@@ -89,10 +111,11 @@ void virInterfaceDefFree(virInterfaceDefPtr def)
break;
}
- VIR_FREE(def->proto.family);
- VIR_FREE(def->proto.address);
- VIR_FREE(def->proto.gateway);
-
+ /* free all protos */
+ for (pp = 0; pp < def->nprotos; pp++) {
+ virInterfaceProtocolDefFree(def->protos[pp]);
+ }
+ VIR_FREE(def->protos);
VIR_FREE(def);
}
@@ -222,22 +245,22 @@ virInterfaceDefParseBondArpValid(virConnectPtr conn,
xmlXPathContextPtr ctxt) {
}
static int
-virInterfaceDefParseDhcp(virConnectPtr conn, virInterfaceDefPtr def,
+virInterfaceDefParseDhcp(virConnectPtr conn, virInterfaceProtocolDefPtr def,
xmlNodePtr dhcp, xmlXPathContextPtr ctxt) {
xmlNodePtr save;
char *tmp;
int ret = 0;
- def->proto.dhcp = 1;
+ def->dhcp = 1;
save = ctxt->node;
ctxt->node = dhcp;
/* Not much to do in the current version */
tmp = virXPathString(conn, "string(./@peerdns)", ctxt);
if (tmp) {
if (STREQ(tmp, "yes"))
- def->proto.peerdns = 1;
+ def->peerdns = 1;
else if (STREQ(tmp, "no"))
- def->proto.peerdns = 0;
+ def->peerdns = 0;
else {
virInterfaceReportError(conn, VIR_ERR_XML_ERROR,
_("unknown dhcp peerdns value %s"), tmp);
@@ -245,86 +268,208 @@ virInterfaceDefParseDhcp(virConnectPtr conn, virInterfaceDefPtr
def,
}
VIR_FREE(tmp);
} else
- def->proto.peerdns = -1;
+ def->peerdns = -1;
ctxt->node = save;
return(ret);
}
static int
-virInterfaceDefParseIp(virConnectPtr conn, virInterfaceDefPtr def,
- xmlNodePtr ip ATTRIBUTE_UNUSED, xmlXPathContextPtr ctxt) {
+virInterfaceDefParseIp(virConnectPtr conn, virInterfaceIpDefPtr def,
+ xmlXPathContextPtr ctxt) {
int ret = 0;
char *tmp;
long l;
- tmp = virXPathString(conn, "string(./ip[1]/@address)", ctxt);
- def->proto.address = tmp;
+ tmp = virXPathString(conn, "string(./@address)", ctxt);
+ def->address = tmp;
if (tmp != NULL) {
- ret = virXPathLong(conn, "string(./ip[1]/@prefix)", ctxt, &l);
+ ret = virXPathLong(conn, "string(./@prefix)", ctxt, &l);
if (ret == 0)
- def->proto.prefix = (int) l;
+ def->prefix = (int) l;
else if (ret == -2) {
virInterfaceReportError(conn, VIR_ERR_XML_ERROR,
"%s", _("Invalid ip address prefix
value"));
return(-1);
}
}
- tmp = virXPathString(conn, "string(./route[1]/@gateway)", ctxt);
- def->proto.gateway = tmp;
return(0);
}
static int
-virInterfaceDefParseProtoIPv4(virConnectPtr conn, virInterfaceDefPtr def,
+virInterfaceDefParseProtoIPv4(virConnectPtr conn, virInterfaceProtocolDefPtr def,
xmlXPathContextPtr ctxt) {
- xmlNodePtr dhcp, ip;
- int ret = 0;
+ xmlNodePtr dhcp;
+ xmlNodePtr *ipNodes = NULL;
+ int nIpNodes, ii, ret = -1;
+ char *tmp;
+
+ tmp = virXPathString(conn, "string(./route[1]/@gateway)", ctxt);
+ def->gateway = tmp;
dhcp = virXPathNode(conn, "./dhcp", ctxt);
if (dhcp != NULL)
ret = virInterfaceDefParseDhcp(conn, def, dhcp, ctxt);
+ if (ret != 0)
+ return(ret);
+
+ nIpNodes = virXPathNodeSet(conn, "./ip", ctxt, &ipNodes);
+ if (ipNodes == NULL)
+ return 0;
+
+ if (VIR_ALLOC_N(def->ips, nIpNodes) < 0) {
+ virReportOOMError(conn);
+ goto error;
+ }
+
+ def->nips = 0;
+ for (ii = 0; ii < nIpNodes; ii++) {
+
+ virInterfaceIpDefPtr ip;
+ if (VIR_ALLOC(ip) < 0) {
+ virReportOOMError(conn);
+ goto error;
+ }
+
+ ctxt->node = ipNodes[ii];
+ ret = virInterfaceDefParseIp(conn, ip, ctxt);
+ if (ret != 0) {
+ virInterfaceIpDefFree(ip);
+ goto error;
+ }
+ def->ips[def->nips++] = ip;
+ }
+
+ ret = 0;
+
+error:
+ VIR_FREE(ipNodes);
+ return(ret);
+}
+
+static int
+virInterfaceDefParseProtoIPv6(virConnectPtr conn, virInterfaceProtocolDefPtr def,
+ xmlXPathContextPtr ctxt) {
+ xmlNodePtr dhcp, autoconf;
+ xmlNodePtr *ipNodes = NULL;
+ int nIpNodes, ii, ret = -1;
+ char *tmp;
+
+ tmp = virXPathString(conn, "string(./route[1]/@gateway)", ctxt);
+ def->gateway = tmp;
+
+ autoconf = virXPathNode(conn, "./autoconf", ctxt);
+ if (autoconf != NULL)
+ def->autoconf = 1;
+
+ dhcp = virXPathNode(conn, "./dhcp", ctxt);
+ if (dhcp != NULL)
+ ret = virInterfaceDefParseDhcp(conn, def, dhcp, ctxt);
if (ret != 0)
return(ret);
- ip = virXPathNode(conn, "./ip", ctxt);
- if (ip != NULL)
- ret = virInterfaceDefParseIp(conn, def, ip, ctxt);
+ nIpNodes = virXPathNodeSet(conn, "./ip", ctxt, &ipNodes);
+ if (ipNodes == NULL)
+ return 0;
+
+ if (VIR_ALLOC_N(def->ips, nIpNodes) < 0) {
+ virReportOOMError(conn);
+ goto error;
+ }
+
+ def->nips = 0;
+ for (ii = 0; ii < nIpNodes; ii++) {
+
+ virInterfaceIpDefPtr ip;
+
+ if (VIR_ALLOC(ip) < 0) {
+ virReportOOMError(conn);
+ goto error;
+ }
+
+ ctxt->node = ipNodes[ii];
+ ret = virInterfaceDefParseIp(conn, ip, ctxt);
+ if (ret != 0) {
+ virInterfaceIpDefFree(ip);
+ goto error;
+ }
+ def->ips[def->nips++] = ip;
+ }
+
+ ret = 0;
+
+error:
+ VIR_FREE(ipNodes);
return(ret);
}
static int
virInterfaceDefParseIfAdressing(virConnectPtr conn, virInterfaceDefPtr def,
xmlXPathContextPtr ctxt) {
- xmlNodePtr cur, save;
- int ret;
+ xmlNodePtr save;
+ xmlNodePtr *protoNodes = NULL;
+ int nProtoNodes, pp, ret = -1;
char *tmp;
- cur = virXPathNode(conn, "./protocol[1]", ctxt);
- if (cur == NULL)
- return(0);
save = ctxt->node;
- ctxt->node = cur;
- tmp = virXPathString(conn, "string(./@family)", ctxt);
- if (tmp == NULL) {
- virInterfaceReportError(conn, VIR_ERR_XML_ERROR,
- "%s", _("protocol misses the family
attribute"));
- ret = -1;
- goto done;
+
+ nProtoNodes = virXPathNodeSet(conn, "./protocol", ctxt, &protoNodes);
+ if (nProtoNodes <= 0) {
+ /* no protocols is an acceptable outcome */
+ return 0;
}
- if (STREQ(tmp, "ipv4")) {
- def->proto.family = tmp;
- ret = virInterfaceDefParseProtoIPv4(conn, def, ctxt);
- } else {
- virInterfaceReportError(conn, VIR_ERR_XML_ERROR,
- _("unsupported protocol family '%s'"),
tmp);
- ret = -1;
- VIR_FREE(tmp);
+
+ if (VIR_ALLOC_N(def->protos, nProtoNodes) < 0) {
+ virReportOOMError(conn);
+ goto error;
}
-done:
+ def->nprotos = 0;
+ for (pp = 0; pp < nProtoNodes; pp++) {
+
+ virInterfaceProtocolDefPtr proto;
+
+ if (VIR_ALLOC(proto) < 0) {
+ virReportOOMError(conn);
+ goto error;
+ }
+
+ ctxt->node = protoNodes[pp];
+ tmp = virXPathString(conn, "string(./@family)", ctxt);
+ if (tmp == NULL) {
+ virInterfaceReportError(conn, VIR_ERR_XML_ERROR,
+ "%s", _("protocol misses the family
attribute"));
+ virInterfaceProtocolDefFree(proto);
+ goto error;
+ }
+ proto->family = tmp;
+ if (STREQ(tmp, "ipv4")) {
+ ret = virInterfaceDefParseProtoIPv4(conn, proto, ctxt);
+ if (ret != 0) {
+ virInterfaceProtocolDefFree(proto);
+ goto error;
+ }
+ } else if (STREQ(tmp, "ipv6")) {
+ ret = virInterfaceDefParseProtoIPv6(conn, proto, ctxt);
+ if (ret != 0) {
+ virInterfaceProtocolDefFree(proto);
+ goto error;
+ }
+ } else {
+ virInterfaceReportError(conn, VIR_ERR_XML_ERROR,
+ _("unsupported protocol family
'%s'"), tmp);
+ virInterfaceProtocolDefFree(proto);
+ goto error;
+ }
+ def->protos[def->nprotos++] = proto;
+ }
+
+ ret = 0;
+
+error:
+ VIR_FREE(protoNodes);
ctxt->node = save;
return(ret);
@@ -999,30 +1144,44 @@ virInterfaceVlanDefFormat(virConnectPtr conn, virBufferPtr buf,
static int
virInterfaceProtocolDefFormat(virConnectPtr conn ATTRIBUTE_UNUSED,
virBufferPtr buf, const virInterfaceDefPtr def) {
- if (def->proto.family == NULL)
- return(0);
- virBufferVSprintf(buf, " <protocol family='%s'>\n",
def->proto.family);
- if (def->proto.dhcp) {
- if (def->proto.peerdns == 0)
- virBufferAddLit(buf, " <dhcp peerdns='no'/>\n");
- else if (def->proto.peerdns == 1)
- virBufferAddLit(buf, " <dhcp peerdns='yes'/>\n");
- else
- virBufferAddLit(buf, " <dhcp/>\n");
- }
- if (def->proto.address != NULL) {
- if (def->proto.prefix != 0)
- virBufferVSprintf(buf, " <ip address='%s'
prefix='%d'/>\n",
- def->proto.address, def->proto.prefix);
- else
- virBufferVSprintf(buf, " <ip address='%s'/>\n",
- def->proto.address);
- }
- if (def->proto.gateway != NULL) {
- virBufferVSprintf(buf, " <route gateway='%s'/>\n",
- def->proto.gateway);
+ int pp, ii;
+
+ for (pp = 0; pp < def->nprotos; pp++) {
+
+ virBufferVSprintf(buf, " <protocol family='%s'>\n",
def->protos[pp]->family);
+
+ if (def->protos[pp]->autoconf) {
+ virBufferAddLit(buf, " <autoconf/>\n");
+ }
+
+ if (def->protos[pp]->dhcp) {
+ if (def->protos[pp]->peerdns == 0)
+ virBufferAddLit(buf, " <dhcp
peerdns='no'/>\n");
+ else if (def->protos[pp]->peerdns == 1)
+ virBufferAddLit(buf, " <dhcp
peerdns='yes'/>\n");
+ else
+ virBufferAddLit(buf, " <dhcp/>\n");
+ }
+
+ for (ii = 0; ii < def->protos[pp]->nips; ii++) {
+ if (def->protos[pp]->ips[ii]->address != NULL) {
+
+ virBufferVSprintf(buf, " <ip address='%s'",
+ def->protos[pp]->ips[ii]->address);
+ if (def->protos[pp]->ips[ii]->prefix != 0) {
+ virBufferVSprintf(buf, " prefix='%d'",
+ def->protos[pp]->ips[ii]->prefix);
+ }
+ virBufferAddLit(buf, "/>\n");
+ }
+ }
+ if (def->protos[pp]->gateway != NULL) {
+ virBufferVSprintf(buf, " <route
gateway='%s'/>\n",
+ def->protos[pp]->gateway);
+ }
+
+ virBufferAddLit(buf, " </protocol>\n");
}
- virBufferAddLit(buf, " </protocol>\n");
return(0);
}
diff --git a/src/conf/interface_conf.h b/src/conf/interface_conf.h
index 20d9771..c5f1023 100644
--- a/src/conf/interface_conf.h
+++ b/src/conf/interface_conf.h
@@ -124,14 +124,23 @@ struct _virInterfaceVlanDef {
char *devname; /* device name for vlan */
};
+typedef struct _virInterfaceIpDef virInterfaceIpDef;
+typedef virInterfaceIpDef *virInterfaceIpDefPtr;
+struct _virInterfaceIpDef {
+ char *address; /* ip address */
+ int prefix; /* ip prefix */
+};
+
+
typedef struct _virInterfaceProtocolDef virInterfaceProtocolDef;
typedef virInterfaceProtocolDef *virInterfaceProtocolDefPtr;
struct _virInterfaceProtocolDef {
- char *family; /* ipv4 only right now */
+ char *family; /* ipv4 or ipv6 */
int dhcp; /* use dhcp */
int peerdns; /* dhcp peerdns ? */
- char *address; /* ip address */
- int prefix; /* ip prefix */
+ int autoconf; /* only useful if family is ipv6 */
+ int nips;
+ virInterfaceIpDefPtr *ips; /* ptr to array of ips[nips] */
char *gateway; /* route gateway */
};
@@ -152,8 +161,8 @@ struct _virInterfaceDef {
virInterfaceBondDef bond;
} data;
- /* separated as we may allow multiple of those in the future */
- virInterfaceProtocolDef proto;
+ int nprotos;
+ virInterfaceProtocolDefPtr *protos; /* ptr to array of protos[nprotos] */
};
typedef struct _virInterfaceObj virInterfaceObj;
--
1.6.5.15.gc274d
2:23 p.m.
New subject: [libvirt] [PATCH 3/3] (REVISED) Support for IPv6 / multiple addresses per interface in virInterface
On Fri, Oct 23, 2009 at 01:31:19PM -0400, Laine Stump wrote:
(How's this?)
This patch updates the xml parsing and formatting, and the associated
virInterfaceDef data structure to support IPv6, along the way adding
support for multiple protocols per interface, and multiple IP
addresses per protocol.
---
src/conf/interface_conf.c | 295 ++++++++++++++++++++++++++++++++++-----------
src/conf/interface_conf.h | 19 ++-
2 files changed, 241 insertions(+), 73 deletions(-)
ACK, looks good now.
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 :|
6:06 a.m.
New subject: [libvirt] [PATCH 3/3] (REVISED) Support for IPv6 / multiple addresses per interface in virInterface
On Fri, Oct 23, 2009 at 01:31:19PM -0400, Laine Stump wrote:
(How's this?)
This patch updates the xml parsing and formatting, and the associated
virInterfaceDef data structure to support IPv6, along the way adding
support for multiple protocols per interface, and multiple IP
addresses per protocol.
---
src/conf/interface_conf.c | 295 ++++++++++++++++++++++++++++++++++-----------
src/conf/interface_conf.h | 19 ++-
2 files changed, 241 insertions(+), 73 deletions(-)
diff --git a/src/conf/interface_conf.c b/src/conf/interface_conf.c
index d46f7ac..7cb71ed 100644
--- a/src/conf/interface_conf.c
+++ b/src/conf/interface_conf.c
@@ -53,9 +53,31 @@ void virInterfaceBareDefFree(virInterfaceBareDefPtr def) {
VIR_FREE(def);
}
+static
+void virInterfaceIpDefFree(virInterfaceIpDefPtr def) {
+ if (def == NULL)
+ return;
+ VIR_FREE(def->address);
+}
Hum ... shouldn't def be freed too there ???
[...]
+ def->nips = 0;
+ for (ii = 0; ii < nIpNodes; ii++) {
+
+ virInterfaceIpDefPtr ip;
+ if (VIR_ALLOC(ip) < 0) {
+ virReportOOMError(conn);
+ goto error;
+ }
hum, yes I really think the matching FREE is missing, but let's do
that as a separate patch
+ ctxt->node = ipNodes[ii];
+ ret = virInterfaceDefParseIp(conn, ip, ctxt);
+ if (ret != 0) {
+ virInterfaceIpDefFree(ip);
+ goto error;
Hum ....
+ }
+ def->ips[def->nips++] = ip;
+ }
+
+ ret = 0;
+
+error:
+ VIR_FREE(ipNodes);
+ return(ret);
+}
It's hard to be sure we aren't leaking there too. On second reading
this looks okay because def is passed by the caller and we are storing
the data there.
[...]
+ nProtoNodes = virXPathNodeSet(conn, "./protocol",
ctxt, &protoNodes);
+ if (nProtoNodes <= 0) {
+ /* no protocols is an acceptable outcome */
+ return 0;
}
+
+ if (VIR_ALLOC_N(def->protos, nProtoNodes) < 0) {
+ virReportOOMError(conn);
+ goto error;
}
+ def->nprotos = 0;
+ for (pp = 0; pp < nProtoNodes; pp++) {
+
+ virInterfaceProtocolDefPtr proto;
+
+ if (VIR_ALLOC(proto) < 0) {
+ virReportOOMError(conn);
+ goto error;
+ }
+
+ ctxt->node = protoNodes[pp];
+ tmp = virXPathString(conn, "string(./@family)", ctxt);
+ if (tmp == NULL) {
+ virInterfaceReportError(conn, VIR_ERR_XML_ERROR,
+ "%s", _("protocol misses the family
attribute"));
+ virInterfaceProtocolDefFree(proto);
+ goto error;
+ }
+ proto->family = tmp;
+ if (STREQ(tmp, "ipv4")) {
+ ret = virInterfaceDefParseProtoIPv4(conn, proto, ctxt);
+ if (ret != 0) {
+ virInterfaceProtocolDefFree(proto);
+ goto error;
+ }
+ } else if (STREQ(tmp, "ipv6")) {
+ ret = virInterfaceDefParseProtoIPv6(conn, proto, ctxt);
+ if (ret != 0) {
+ virInterfaceProtocolDefFree(proto);
+ goto error;
+ }
+ } else {
+ virInterfaceReportError(conn, VIR_ERR_XML_ERROR,
+ _("unsupported protocol family
'%s'"), tmp);
+ virInterfaceProtocolDefFree(proto);
+ goto error;
+ }
+ def->protos[def->nprotos++] = proto;
+ }
+
+ ret = 0;
+
+error:
+ VIR_FREE(protoNodes);
ctxt->node = save;
return(ret);
Hum, we don't check there that an ipv6/4 protocol is not defined
multiple time. Maybe this could be slightly refactored to search
for 1 protocol node with type IPv4 and the one protocol node for type
Ipv6 something like
v4 = virXPathNode(conn, "./protocol[@family = 'ipv4']", ctxt)
v6 = virXPathNode(conn, "./protocol[@family = 'ipv6']", ctxt)
and drop the loop.
But current code works too, just does less checking IMHO.
I think we need to patch the previous leak.
But the main problem I faced when running the regression tests.
I had installed netcf-0.1.3 and a number of thing just break with
this patch. For example when libvirtd exits in the
"testing with corrupted config" libvirtd it just segfaults:
Starting program: /u/veillard/libvirt/daemon/libvirtd --config=in1.conf
[...]
Program received signal SIGSEGV, Segmentation fault.
drv_close (ncf=0x7f0740) at drv_initscripts.c:511
511 xsltFreeStylesheet(ncf->driver->get);
(gdb) p ncf->driver
$2 = (struct driver *) 0x0
(gdb) p *ncf
$3 = {ref = 1, root = 0x7f0780 "/",
data_dir = 0x7ffff77626b8 "/usr/share/netcf", errcode = NETCF_NOERROR,
errdetails = 0x0, driver = 0x0, debug = 0}
(gdb)
IMHO it's a bug in netcf since it should protect against acessing
uninitialized fields. The libvirtd side is:
/* open netcf */
if (ncf_init(&driverState->netcf, NULL) != 0)
{
/* what error to report? */
goto netcf_error;
}
conn->interfacePrivateData = driverState;
return 0;
netcf_error:
if (driverState->netcf)
{
ncf_close(driverState->netcf);
}
maybe if ncf_init() failed we should not call ncf_close()
but we can see that there is at least a string in that driver which need
to be deallocated.
I ran the test on Fedora 11 updated with netcf-0.1.3:
What I don't understand is why that patch affecting only
src/conf/interface_conf.[ch] is the one breaking this...
Hum, I reverted that patch and still get the error, maybe it's just
the netcf update ! I will try to clarify where things went wrong.
I also noted that the interface XML parsing regression tests also failed
which might be normal though since no reg test was updated.
Daniel
--
Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/
daniel(a)veillard.com | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library http://libvirt.org/
8:25 a.m.
New subject: [libvirt] [PATCH 3/3] (REVISED) Support for IPv6 / multiple addresses per interface in virInterface
On Wed, Oct 28, 2009 at 12:06:24PM +0100, Daniel Veillard wrote:
[...]
But the main problem I faced when running the regression tests.
I had installed netcf-0.1.3 and a number of thing just break with
this patch. For example when libvirtd exits in the
"testing with corrupted config" libvirtd it just segfaults:
Starting program: /u/veillard/libvirt/daemon/libvirtd --config=in1.conf
[...]
Program received signal SIGSEGV, Segmentation fault.
drv_close (ncf=0x7f0740) at drv_initscripts.c:511
511 xsltFreeStylesheet(ncf->driver->get);
(gdb) p ncf->driver
$2 = (struct driver *) 0x0
(gdb) p *ncf
$3 = {ref = 1, root = 0x7f0780 "/",
data_dir = 0x7ffff77626b8 "/usr/share/netcf", errcode = NETCF_NOERROR,
errdetails = 0x0, driver = 0x0, debug = 0}
(gdb)
Okay, I modified netcf to incorporate the patch below to avoid the
crash, but I still get a ton of regression tests failures on F-11 +
0.1.3 + your 3 patches when running make check as non-root.
If I checkout master without your patches in the same environment
make check passes fine.
So I'm postponing commiting your patches until we have an additional
patch fixing the regressions I see,
sorry,
Daniel
--
Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/
daniel(a)veillard.com | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library http://libvirt.org/
8:27 a.m.
New subject: [libvirt] [PATCH 3/3] (REVISED) Support for IPv6 / multiple addresses per interface in virInterface
On Wed, Oct 28, 2009 at 02:25:38PM +0100, Daniel Veillard wrote:
On Wed, Oct 28, 2009 at 12:06:24PM +0100, Daniel Veillard wrote:
[...]
> But the main problem I faced when running the regression tests.
> I had installed netcf-0.1.3 and a number of thing just break with
> this patch. For example when libvirtd exits in the
> "testing with corrupted config" libvirtd it just segfaults:
>
> Starting program: /u/veillard/libvirt/daemon/libvirtd --config=in1.conf
> [...]
> Program received signal SIGSEGV, Segmentation fault.
> drv_close (ncf=0x7f0740) at drv_initscripts.c:511
> 511 xsltFreeStylesheet(ncf->driver->get);
> (gdb) p ncf->driver
> $2 = (struct driver *) 0x0
> (gdb) p *ncf
> $3 = {ref = 1, root = 0x7f0780 "/",
> data_dir = 0x7ffff77626b8 "/usr/share/netcf", errcode = NETCF_NOERROR,
> errdetails = 0x0, driver = 0x0, debug = 0}
> (gdb)
Okay, I modified netcf to incorporate the patch below to avoid the
crash, but I still get a ton of regression tests failures on F-11 +
urgh, forgot it :-\
Daniel
--
Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/
daniel(a)veillard.com | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library http://libvirt.org/
1:53 a.m.
New subject: [libvirt] [PATCH 3/3] (REVISED) Support for IPv6 / multiple addresses per interface in virInterface
On 10/28/2009 07:06 AM, Daniel Veillard wrote:
On Fri, Oct 23, 2009 at 01:31:19PM -0400, Laine Stump wrote:
> diff --git a/src/conf/interface_conf.c b/src/conf/interface_conf.c
> index d46f7ac..7cb71ed 100644
> --- a/src/conf/interface_conf.c
> +++ b/src/conf/interface_conf.c
> @@ -53,9 +53,31 @@ void virInterfaceBareDefFree(virInterfaceBareDefPtr def) {
> VIR_FREE(def);
> }
>
> +static
> +void virInterfaceIpDefFree(virInterfaceIpDefPtr def) {
> + if (def == NULL)
> + return;
> + VIR_FREE(def->address);
> +}
>
Hum ... shouldn't def be freed too there ???
Yes, you are correct. It will be fixed in the redo.
> + ctxt->node = ipNodes[ii];
> + ret = virInterfaceDefParseIp(conn, ip, ctxt);
> + if (ret != 0) {
> + virInterfaceIpDefFree(ip);
> + goto error;
>
Hum ....
> + }
> + def->ips[def->nips++] = ip;
> + }
> +
> + ret = 0;
> +
> +error:
> + VIR_FREE(ipNodes);
> + return(ret);
> +}
>
It's hard to be sure we aren't leaking there too. On second reading
this looks okay because def is passed by the caller and we are storing
the data there.
Correct. The caller will clean up if there's a failure.
Hum, we don't check there that an ipv6/4 protocol is not
defined
multiple time. Maybe this could be slightly refactored to search
for 1 protocol node with type IPv4 and the one protocol node for type
Ipv6 something like
v4 = virXPathNode(conn, "./protocol[@family = 'ipv4']", ctxt)
v6 = virXPathNode(conn, "./protocol[@family = 'ipv6']", ctxt)
Yeah, I can do that.
In the meantime, I've sent a netcf patch that gets vlan xml passing the
libvirt parse, but I'm having trouble finding the info for bridges and
bonds either in sysfs or netlink, and my brain is finished for the day.
Basically, for a bridge to pass, we need the name of the physical
interface, and for bond to pass, we need an miimon node with freq
property, or an arpmode node with interval and target properties, in
addition to the names of the physical interfaces. I'm not finding those
anywhere.
9:34 a.m.
New subject: [libvirt] [PATCH 3/3] (REVISED) Support for IPv6 / multiple addresses per interface in virInterface
On 10/29/2009 02:53 AM, Laine Stump wrote:
On 10/28/2009 07:06 AM, Daniel Veillard wrote:
> On Fri, Oct 23, 2009 at 01:31:19PM -0400, Laine Stump wrote:
>
> Hum, we don't check there that an ipv6/4 protocol is not defined
> multiple time. Maybe this could be slightly refactored to search
> for 1 protocol node with type IPv4 and the one protocol node for type
> Ipv6 something like
> v4 = virXPathNode(conn, "./protocol[@family = 'ipv4']", ctxt)
> v6 = virXPathNode(conn, "./protocol[@family = 'ipv6']", ctxt)
Yeah, I can do that.
Now that I'm actually doing it, I'm having doubts about this. Making
this change leads to many other changes (mostly making the code simpler,
but still changed lines are changed lines, and could lead to new bugs).
More important, though, I notice that if we do it this way we lose the
ability to report an error if someone botches up the family of a
protocol - with this new method of parsing it will just be ignored; with
the current setup in my patch, an error will be reported if there's a
protocol that is missing family, or has family set to anything other
than "ipv4" or "ipv6".
Would it maybe be better to still cycle through all the protocol nodes
in the document with a loop as currently, but report an error if a
particular protocol family is encountered a 2nd time?
If so, can/should that be submitted as a separate patch, rather than
trying to get it into the patch that's already working? (If that's the
case, I'll clean up and resend the patches sooner rather than later).
10:33 a.m.
New subject: [libvirt] [PATCH 3/3] (REVISED) Support for IPv6 / multiple addresses per interface in virInterface
On Fri, Oct 30, 2009 at 10:34:03AM -0400, Laine Stump wrote:
On 10/29/2009 02:53 AM, Laine Stump wrote:
> On 10/28/2009 07:06 AM, Daniel Veillard wrote:
>> On Fri, Oct 23, 2009 at 01:31:19PM -0400, Laine Stump wrote:
>>
>
>> Hum, we don't check there that an ipv6/4 protocol is not defined
>> multiple time. Maybe this could be slightly refactored to search
>> for 1 protocol node with type IPv4 and the one protocol node for type
>> Ipv6 something like
>> v4 = virXPathNode(conn, "./protocol[@family = 'ipv4']",
ctxt)
>> v6 = virXPathNode(conn, "./protocol[@family = 'ipv6']",
ctxt)
>
> Yeah, I can do that.
Now that I'm actually doing it, I'm having doubts about this. Making
this change leads to many other changes (mostly making the code simpler,
but still changed lines are changed lines, and could lead to new bugs).
More important, though, I notice that if we do it this way we lose the
ability to report an error if someone botches up the family of a
protocol - with this new method of parsing it will just be ignored; with
the current setup in my patch, an error will be reported if there's a
protocol that is missing family, or has family set to anything other
than "ipv4" or "ipv6".
Ah right, I didn't though about that side of the checking
Would it maybe be better to still cycle through all the protocol
nodes
in the document with a loop as currently, but report an error if a
particular protocol family is encountered a 2nd time?
yup,
If so, can/should that be submitted as a separate patch, rather than
trying to get it into the patch that's already working? (If that's the
case, I'll clean up and resend the patches sooner rather than later).
Let's just make a cleanup patch on top of the current one !
Daniel
--
Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/
daniel(a)veillard.com | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library http://libvirt.org/
7:34 a.m.
New subject: [libvirt] [PATCH] Fix virInterfaceIpDefPtr leak during virInterfaceIpDefFree().
This is the only update to the patchset already sent. All other problems
were either in netcf code, or will be cleaned up later.
A patch has been sent to netcf-devel which, when applied, allows make
check of libvirt plus the patch series to complete with no errors.
There is still one issue, affecting bond interfaces, which is not
revealed by make check. I'll describe that in a separate email.
---
src/conf/interface_conf.c | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/src/conf/interface_conf.c b/src/conf/interface_conf.c
index fc18eba..31af957 100644
--- a/src/conf/interface_conf.c
+++ b/src/conf/interface_conf.c
@@ -58,6 +58,7 @@ void virInterfaceIpDefFree(virInterfaceIpDefPtr def) {
if (def == NULL)
return;
VIR_FREE(def->address);
+ VIR_FREE(def);
}
static
--
1.6.5.15.gc274d
8:05 a.m.
New subject: [libvirt] [PATCH] Fix virInterfaceIpDefPtr leak during virInterfaceIpDefFree().
On Mon, Nov 02, 2009 at 08:34:50AM -0500, Laine Stump wrote:
This is the only update to the patchset already sent. All other
problems
were either in netcf code, or will be cleaned up later.
A patch has been sent to netcf-devel which, when applied, allows make
check of libvirt plus the patch series to complete with no errors.
There is still one issue, affecting bond interfaces, which is not
revealed by make check. I'll describe that in a separate email.
---
src/conf/interface_conf.c | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/src/conf/interface_conf.c b/src/conf/interface_conf.c
index fc18eba..31af957 100644
--- a/src/conf/interface_conf.c
+++ b/src/conf/interface_conf.c
@@ -58,6 +58,7 @@ void virInterfaceIpDefFree(virInterfaceIpDefPtr def) {
if (def == NULL)
return;
VIR_FREE(def->address);
+ VIR_FREE(def);
}
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 :|
1:08 p.m.
New subject: [libvirt] [PATCH] Fix virInterfaceIpDefPtr leak during virInterfaceIpDefFree().
On Mon, Nov 02, 2009 at 08:34:50AM -0500, Laine Stump wrote:
This is the only update to the patchset already sent. All other
problems
were either in netcf code, or will be cleaned up later.
A patch has been sent to netcf-devel which, when applied, allows make
check of libvirt plus the patch series to complete with no errors.
There is still one issue, affecting bond interfaces, which is not
revealed by make check. I'll describe that in a separate email.
---
src/conf/interface_conf.c | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/src/conf/interface_conf.c b/src/conf/interface_conf.c
index fc18eba..31af957 100644
--- a/src/conf/interface_conf.c
+++ b/src/conf/interface_conf.c
@@ -58,6 +58,7 @@ void virInterfaceIpDefFree(virInterfaceIpDefPtr def) {
if (def == NULL)
return;
VIR_FREE(def->address);
+ VIR_FREE(def);
}
Okay, applied !
I have the single netcf patch to avoid a crash, but when running the
regression tests I still see:
9) Interface XML-2-XML bond
... FAILED
10) Interface XML-2-XML bond-arp
... FAILED
FAIL: interfacexml2xmltest
=====================
12 of 43 tests failed
=====================
make[3]: *** [check-TESTS] Error 1
So what am I missing ?
Daniel
--
Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/
daniel(a)veillard.com | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library http://libvirt.org/
2:07 p.m.
New subject: [libvirt] [PATCH] Fix virInterfaceIpDefPtr leak during virInterfaceIpDefFree().
On Tue, Nov 03, 2009 at 08:08:00PM +0100, Daniel Veillard wrote:
On Mon, Nov 02, 2009 at 08:34:50AM -0500, Laine Stump wrote:
> This is the only update to the patchset already sent. All other problems
> were either in netcf code, or will be cleaned up later.
>
> A patch has been sent to netcf-devel which, when applied, allows make
> check of libvirt plus the patch series to complete with no errors.
>
> There is still one issue, affecting bond interfaces, which is not
> revealed by make check. I'll describe that in a separate email.
> ---
> src/conf/interface_conf.c | 1 +
> 1 files changed, 1 insertions(+), 0 deletions(-)
>
> diff --git a/src/conf/interface_conf.c b/src/conf/interface_conf.c
> index fc18eba..31af957 100644
> --- a/src/conf/interface_conf.c
> +++ b/src/conf/interface_conf.c
> @@ -58,6 +58,7 @@ void virInterfaceIpDefFree(virInterfaceIpDefPtr def) {
> if (def == NULL)
> return;
> VIR_FREE(def->address);
> + VIR_FREE(def);
> }
>
Okay, applied !
I have the single netcf patch to avoid a crash, but when running the
regression tests I still see:
9) Interface XML-2-XML bond
... FAILED
10) Interface XML-2-XML bond-arp
... FAILED
FAIL: interfacexml2xmltest
=====================
12 of 43 tests failed
=====================
make[3]: *** [check-TESTS] Error 1
So what am I missing ?
I was missing patch
https://www.redhat.com/archives/libvir-list/2009-October/msg00760.html
all 5 patches are now pushed,
thanks !
Daniel
--
Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/
daniel(a)veillard.com | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library http://libvirt.org/
9:20 a.m.
New subject: [libvirt] [PATCH 0/3] IPv6 / multi address / report live config in virInterface
The related patch is now available on the netcf-devel mailing list,
making it possible to review this set ;-)
On 10/16/2009 10:56 AM, laine(a)laine.org wrote:
This is the 2nd iteration of this patchset incorporating danpb's
suggestions. Along with a couple other minor things, the type attribute
is again a required attribute, and netcf support will be omitted in
the build unless you have netcf-devel-0.1.3 or greater. Since there is
no netcf-devel-0.1.3 yet, I tested this by changing the appropriate
strings to 0.1.2. 0.1.3 will be released "soon" (I want to try and get
IPv6 address reporting in before lutter cuts another release). I just
wanted to get the patches to the list early to get some visual review
in case I screwed up anything new ;-)
Note that (unless you're building/installing netcf from the head of
git) if you try to fake out the version of netcf yourself you'll get
runtime errors from virInterfaceGetXMLDesc (iface-dumpxml) due to
there being no "type" attribute in the interface xml.
--
Libvir-list mailing list
Libvir-list(a)redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
5311
days inactive
5329
days old
21 comments
4 participants
participants (4)
-
Daniel P. Berrange -
Daniel Veillard -
Laine Stump -
laine＠laine.org