[libvirt] [PATCH] Support reporting live interface IP/netmask.
From: root <root@vlap.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 device directly, rather than just reporting what's in the config file. The backend of this is in netcf's new ncf_if_xml_state() function. Any live interface ip address info in the xml will have the property "source" set to "device", eg: <ip address='10.24.0.1' prefix='24' source='device'/> Also, if an interface is currently inactive, no ip addresses will be returned, since an inactive interface device can't be queried for IP addresses (effectively it has none). A difference in the XML from previously - it is now acceptable to have both a dhcp *and* an ip node (or neither) within the protocol node. Before you had to have one or the other, but couldn't have both. Note that you need at least netcf 0.1.2 for this to build and work, and an upcoming release (patches submitted today) for it to work exactly as described above. --- include/libvirt/libvirt.h.in | 4 +++ src/conf/interface_conf.c | 62 ++++++++++++++++++++++------------------- src/conf/interface_conf.h | 1 + src/interface/netcf_driver.c | 8 ++++- src/libvirt.c | 15 +++++++--- tools/virsh.c | 10 +++++- 6 files changed, 63 insertions(+), 37 deletions(-) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 4e63e48..fd0c90b 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -922,6 +922,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/src/conf/interface_conf.c b/src/conf/interface_conf.c index e646351..b422464 100644 --- a/src/conf/interface_conf.c +++ b/src/conf/interface_conf.c @@ -92,6 +92,7 @@ void virInterfaceDefFree(virInterfaceDefPtr def) VIR_FREE(def->proto.family); VIR_FREE(def->proto.address); VIR_FREE(def->proto.gateway); + VIR_FREE(def->proto.source); VIR_FREE(def); } @@ -272,6 +273,8 @@ virInterfaceDefParseIp(virConnectPtr conn, virInterfaceDefPtr def, "%s", _("Invalid ip address prefix value")); return(-1); } + tmp = virXPathString(conn, "string(./ip[1]/@source)", ctxt); + def->proto.source = tmp; } tmp = virXPathString(conn, "string(./route[1]/@gateway)", ctxt); def->proto.gateway = tmp; @@ -282,22 +285,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); } @@ -1005,6 +1005,7 @@ virInterfaceVlanDefFormat(virConnectPtr conn, virBufferPtr buf, static int virInterfaceProtocolDefFormat(virConnectPtr conn ATTRIBUTE_UNUSED, virBufferPtr buf, const virInterfaceDefPtr def) { + char prefixStr[16]; if (def->proto.family == NULL) return(0); virBufferVSprintf(buf, " <protocol family='%s'>\n", def->proto.family); @@ -1015,20 +1016,23 @@ 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) + snprintf(prefixStr, sizeof(prefixStr), "%d", def->proto.prefix); + + virBufferVSprintf(buf, " <ip address='%s'%s%s%s%s%s%s/>\n", + def->proto.address, + def->proto.prefix ? " prefix='" : "", + def->proto.prefix ? prefixStr : "", + def->proto.prefix ? "'" : "", + def->proto.source ? " source='" : "", + def->proto.source ? def->proto.source : "", + def->proto.source ? "'" : ""); + } + if (def->proto.gateway != NULL) { + virBufferVSprintf(buf, " <route gateway='%s'/>\n", + def->proto.gateway); } virBufferAddLit(buf, " </protocol>\n"); return(0); diff --git a/src/conf/interface_conf.h b/src/conf/interface_conf.h index bb9dce4..df871aa 100644 --- a/src/conf/interface_conf.h +++ b/src/conf/interface_conf.h @@ -132,6 +132,7 @@ struct _virInterfaceProtocolDef { char *address; /* ip address */ int prefix; /* ip prefix */ char *gateway; /* route gateway */ + char *source; /* source of address - "device" or "config" (default) */ }; 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 bcb89e1..bd712dc 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -5915,10 +5915,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. @@ -5935,7 +5942,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 3482389..77f1d70 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -2763,7 +2763,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; @@ -3297,6 +3297,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} }; @@ -3306,6 +3307,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; @@ -3313,7 +3319,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.2.5
On 09/29/2009 04:02 PM, Laine Stump wrote:
From: root<root@vlap.laine.org>
Note that I accidentally did the local commit as root, and didn't notice it until now. If this patch gets committed, please change the From: first.
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 device directly, rather than just reporting what's in the config file. The backend of this is in netcf's new ncf_if_xml_state() function.
Any live interface ip address info in the xml will have the property "source" set to "device", eg:
<ip address='10.24.0.1' prefix='24' source='device'/>
Also, if an interface is currently inactive, no ip addresses will be returned, since an inactive interface device can't be queried for IP addresses (effectively it has none).
A difference in the XML from previously - it is now acceptable to have both a dhcp *and* an ip node (or neither) within the protocol node. Before you had to have one or the other, but couldn't have both.
Note that you need at least netcf 0.1.2 for this to build and work, and an upcoming release (patches submitted today) for it to work exactly as described above.
--- include/libvirt/libvirt.h.in | 4 +++ src/conf/interface_conf.c | 62 ++++++++++++++++++++++------------------- src/conf/interface_conf.h | 1 + src/interface/netcf_driver.c | 8 ++++- src/libvirt.c | 15 +++++++--- tools/virsh.c | 10 +++++- 6 files changed, 63 insertions(+), 37 deletions(-)
diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 4e63e48..fd0c90b 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -922,6 +922,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/src/conf/interface_conf.c b/src/conf/interface_conf.c index e646351..b422464 100644 --- a/src/conf/interface_conf.c +++ b/src/conf/interface_conf.c @@ -92,6 +92,7 @@ void virInterfaceDefFree(virInterfaceDefPtr def) VIR_FREE(def->proto.family); VIR_FREE(def->proto.address); VIR_FREE(def->proto.gateway); + VIR_FREE(def->proto.source);
VIR_FREE(def); } @@ -272,6 +273,8 @@ virInterfaceDefParseIp(virConnectPtr conn, virInterfaceDefPtr def, "%s", _("Invalid ip address prefix value")); return(-1); } + tmp = virXPathString(conn, "string(./ip[1]/@source)", ctxt); + def->proto.source = tmp; } tmp = virXPathString(conn, "string(./route[1]/@gateway)", ctxt); def->proto.gateway = tmp; @@ -282,22 +285,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); }
@@ -1005,6 +1005,7 @@ virInterfaceVlanDefFormat(virConnectPtr conn, virBufferPtr buf, static int virInterfaceProtocolDefFormat(virConnectPtr conn ATTRIBUTE_UNUSED, virBufferPtr buf, const virInterfaceDefPtr def) { + char prefixStr[16]; if (def->proto.family == NULL) return(0); virBufferVSprintf(buf, "<protocol family='%s'>\n", def->proto.family); @@ -1015,20 +1016,23 @@ 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) + snprintf(prefixStr, sizeof(prefixStr), "%d", def->proto.prefix); + + virBufferVSprintf(buf, "<ip address='%s'%s%s%s%s%s%s/>\n", + def->proto.address, + def->proto.prefix ? " prefix='" : "", + def->proto.prefix ? prefixStr : "", + def->proto.prefix ? "'" : "", + def->proto.source ? " source='" : "", + def->proto.source ? def->proto.source : "", + def->proto.source ? "'" : ""); + } + if (def->proto.gateway != NULL) { + virBufferVSprintf(buf, "<route gateway='%s'/>\n", + def->proto.gateway); } virBufferAddLit(buf, "</protocol>\n"); return(0); diff --git a/src/conf/interface_conf.h b/src/conf/interface_conf.h index bb9dce4..df871aa 100644 --- a/src/conf/interface_conf.h +++ b/src/conf/interface_conf.h @@ -132,6 +132,7 @@ struct _virInterfaceProtocolDef { char *address; /* ip address */ int prefix; /* ip prefix */ char *gateway; /* route gateway */ + char *source; /* source of address - "device" or "config" (default) */ };
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 bcb89e1..bd712dc 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -5915,10 +5915,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. @@ -5935,7 +5942,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 3482389..77f1d70 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -2763,7 +2763,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; @@ -3297,6 +3297,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} };
@@ -3306,6 +3307,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; @@ -3313,7 +3319,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);
On 09/29/2009 10:02 PM, Laine Stump wrote:
From: root<root@vlap.laine.org>
This patch adds the flag VIR_INTERFACE_XML_INACTIVE to virInterfaceGetXMLDesc's flags.
What about calling it instead VIR_INTERFACE_XML_CONFIG and adding a dummy (0) VIR_INTERFACE_XML_DEVICE, and corresponding --config/--device flags to the virsh command? This would match the source attribute. I'll let others override me happily, though. Paolo
On Tue, Sep 29, 2009 at 11:13:56PM +0200, Paolo Bonzini wrote:
On 09/29/2009 10:02 PM, Laine Stump wrote:
From: root<root@vlap.laine.org>
This patch adds the flag VIR_INTERFACE_XML_INACTIVE to virInterfaceGetXMLDesc's flags.
What about calling it instead VIR_INTERFACE_XML_CONFIG and adding a dummy (0) VIR_INTERFACE_XML_DEVICE, and corresponding --config/--device flags to the virsh command? This would match the source attribute.
I'll let others override me happily, though.
Well it's to keep coherent naming with the APIs for other kind of objects, for example there is an VIR_DOMAIN_XML_INACTIVE in libvirt.h to dump the inactive domain informations (when a domain is running/active). Similary 'virsh dumpxml --inactive' is about dumping the XML for the domain as it was defined. W.r.t. providing accessors for each informations we provide as part of an xml dump in virsh, I could try to add something generic, for example as a post processing step based on XPath if people really feel disarmed by grabbing something from XML output. But was that really the sense of your query ? Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/
This patch adds the flag VIR_INTERFACE_XML_INACTIVE to virInterfaceGetXMLDesc's flags.
What about calling it instead VIR_INTERFACE_XML_CONFIG and adding a dummy (0) VIR_INTERFACE_XML_DEVICE, and corresponding --config/--device flags to the virsh command? This would match the source attribute.
I'll let others override me happily, though.
Well it's to keep coherent naming with the APIs for other kind of objects, for example there is an VIR_DOMAIN_XML_INACTIVE in libvirt.h to dump the inactive domain informations (when a domain is running/active). Similary 'virsh dumpxml --inactive' is about dumping the XML for the domain as it was defined.
Then I'm "happily overridden". :-)
W.r.t. providing accessors for each informations we provide as part of an xml dump in virsh, I could try to add something generic, for example as a post processing step based on XPath if people really feel disarmed by grabbing something from XML output. But was that really the sense of your query ?
No, "matching the source attribute" was just my rationale for naming the options --config/--device (and the corresponding enum names). Paolo
On Tue, Sep 29, 2009 at 04:02:30PM -0400, Laine Stump wrote:
From: root <root@vlap.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 device directly, rather than just reporting what's in the config file. The backend of this is in netcf's new ncf_if_xml_state() function.
Any live interface ip address info in the xml will have the property "source" set to "device", eg:
<ip address='10.24.0.1' prefix='24' source='device'/>
This new 'source' attribute is bogus. The fact that the XML is the showing the device details, and not the config file details is inherent in the fact that we're querying the live interface. This distinction applies to the entire XML dump, not just the <ip> tag.
Also, if an interface is currently inactive, no ip addresses will be returned, since an inactive interface device can't be queried for IP addresses (effectively it has none).
A difference in the XML from previously - it is now acceptable to have both a dhcp *and* an ip node (or neither) within the protocol node. Before you had to have one or the other, but couldn't have both.
Note that you need at least netcf 0.1.2 for this to build and work, and an upcoming release (patches submitted today) for it to work exactly as described above.
With the exception of the new 'source' attribute on <ip>, the rest of this pach looks good. 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 :|
On 10/05/2009 07:02 AM, Daniel P. Berrange wrote:
On Tue, Sep 29, 2009 at 04:02:30PM -0400, Laine Stump wrote:
From: root<root@vlap.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 device directly, rather than just reporting what's in the config file. The backend of this is in netcf's new ncf_if_xml_state() function.
Any live interface ip address info in the xml will have the property "source" set to "device", eg:
<ip address='10.24.0.1' prefix='24' source='device'/>
This new 'source' attribute is bogus. The fact that the XML is the showing the device details, and not the config file details is inherent in the fact that we're querying the live interface. This distinction applies to the entire XML dump, not just the<ip> tag.
The 'source' attribute came out of the exchange at the bottom of these messages: https://fedorahosted.org/pipermail/netcf-devel/2009-September/000258.html https://fedorahosted.org/pipermail/netcf-devel/2009-September/000259.html https://fedorahosted.org/pipermail/netcf-devel/2009-September/000260.html (I included all the messages in case too much context was removed in the replies). Some/most of the stuff in the XML still comes from the config files even though we're querying the "live interface', because some info isn't available reliably in any other way (for example, it's useful to know that dhcp is setup for the interface, but I don't believe there's any way to query dhclient to see if that truly is the case). The idea was to put some sort of tag on the particular items that were really coming from the device. I will say that, if nothing else, having this extra attribute has mmade it easier for me to verify that my patches are actually doing something ;-) In the end, I looked at it kindly because it didn't seem to hurt anything (most people just ignore extra stuff in the xml), and might help someone. This attribute is already in the latest netcf release, but of course if required we could ignore it in libvirt (it would be a shame to have netcf and libvirt's xml diverge so soon, though. I figured we'd get at least a year or so before we had to start worring about that :-/)
On Mon, Oct 05, 2009 at 12:18:33PM -0400, Laine Stump wrote:
On 10/05/2009 07:02 AM, Daniel P. Berrange wrote:
On Tue, Sep 29, 2009 at 04:02:30PM -0400, Laine Stump wrote:
From: root<root@vlap.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 device directly, rather than just reporting what's in the config file. The backend of this is in netcf's new ncf_if_xml_state() function.
Any live interface ip address info in the xml will have the property "source" set to "device", eg:
<ip address='10.24.0.1' prefix='24' source='device'/>
This new 'source' attribute is bogus. The fact that the XML is the showing the device details, and not the config file details is inherent in the fact that we're querying the live interface. This distinction applies to the entire XML dump, not just the<ip> tag.
The 'source' attribute came out of the exchange at the bottom of these messages:
https://fedorahosted.org/pipermail/netcf-devel/2009-September/000258.html https://fedorahosted.org/pipermail/netcf-devel/2009-September/000259.html https://fedorahosted.org/pipermail/netcf-devel/2009-September/000260.html
(I included all the messages in case too much context was removed in the replies).
Some/most of the stuff in the XML still comes from the config files even though we're querying the "live interface', because some info isn't available reliably in any other way (for example, it's useful to know that dhcp is setup for the interface, but I don't believe there's any way to query dhclient to see if that truly is the case). The idea was to put some sort of tag on the particular items that were really coming from the device. I will say that, if nothing else, having this extra attribute has mmade it easier for me to verify that my patches are actually doing something ;-) In the end, I looked at it kindly because it didn't seem to hurt anything (most people just ignore extra stuff in the xml), and might help someone.
I'm not really convinced by that thread. If the virInterfaceDumpXML API only ever exposed the live XML config, then there' would be a compelling need to identify whether the <ip> info were from the DHCP or the persistent config. The whole point of having the extra VIR_INTERFACE_XML_INACIVE flag though, is to allow the caller to access the full persistent XML config, independantly of the live XML config. This is much more powerful because they can compare every aspect of the XML description, not merely that one element. With the domain XML format, we did have a few abortive attempts at indicating in the live XML, whether an attribute was from the persistent config, vs dynamically added to live config, but it all ended up as rather a mess. Eventually we introduced the extra VIR_DOMAIN_XML_INACTIVE flag to allow the full distinction/comparison to be made, rather than doing lots of small hacks in the XML each time we case across a new special case. I'd really like us to avoid making this mistake again in the interface XML, and just use the VIR_INTERFACE_XML_INACTIVE flag for this scenario. 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 :|
On Mon, 2009-10-05 at 17:34 +0100, Daniel P. Berrange wrote:
With the domain XML format, we did have a few abortive attempts at indicating in the live XML, whether an attribute was from the persistent config, vs dynamically added to live config, but it all ended up as rather a mess.
Here's a concrete example where this leads to headscratching, at a minimum, with the netowrk configs. When the live XML contains <protocol family="ipv4"> <dhcp/> <ip address="192.168.0.5" prefix="24"/> <route gateway="192.168.0.1"/> </protocol> you at least think that the interface is configured to use DHCP, and currently has the indicated address. Of course, if we ever poke around to see if the interface _currently_ gets its address from DHCP, we have no way to indicate that anymore, since we would indicate 'configured to use DHCP' and 'currently gets its address from DHCP' in exactly the same way. Also, US $5 to anybody who knows whether the <route/> tag above says 'is configured to use that gateway' or 'routing is through that gateway right now'. To belabor this point, when the live XML says <protocol family="ipv4"> <ip address="192.168.0.5" prefix="24"/> <ip address="192.168.0.6" prefix="24"/> </protocol> it's completely unclear what addresses are in the config files (maybe both?) and which ones are actually assigned to the interface right now. David
On Mon, Oct 05, 2009 at 02:34:05PM -0700, David Lutterkort wrote:
On Mon, 2009-10-05 at 17:34 +0100, Daniel P. Berrange wrote:
With the domain XML format, we did have a few abortive attempts at indicating in the live XML, whether an attribute was from the persistent config, vs dynamically added to live config, but it all ended up as rather a mess.
Here's a concrete example where this leads to headscratching, at a minimum, with the netowrk configs. When the live XML contains
<protocol family="ipv4"> <dhcp/> <ip address="192.168.0.5" prefix="24"/> <route gateway="192.168.0.1"/> </protocol>
you at least think that the interface is configured to use DHCP, and currently has the indicated address. Of course, if we ever poke around to see if the interface _currently_ gets its address from DHCP, we have no way to indicate that anymore, since we would indicate 'configured to use DHCP' and 'currently gets its address from DHCP' in exactly the same way. Also, US $5 to anybody who knows whether the <route/> tag above says 'is configured to use that gateway' or 'routing is through that gateway right now'.
The live XML should *always* say what the inteface is doing right now.
To belabor this point, when the live XML says
<protocol family="ipv4"> <ip address="192.168.0.5" prefix="24"/> <ip address="192.168.0.6" prefix="24"/> </protocol>
it's completely unclear what addresses are in the config files (maybe both?) and which ones are actually assigned to the interface right now.
I think there is a disconnect here about the behaviour we should be exposing. In that example of live XML above with 2 IP addresses, both of those IP address *must* be currently assigned to the interface, since this is the live XML. To determine which (if any) of those addresses are in the config file, the VIR_INTEFACE_XML_INACTIVE flag would be used. Looking at netcf, it does not appear to implement this correctly. If I query the live XML of an interface, it gives back a mixture of both the config file and active addresses. Indeed, if using a static IP address, it'll list the same IP address twice in the XML once it pulls it from the config file, and once from the interface. 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 :|
On 10/06/2009 05:13 AM, Daniel P. Berrange wrote:
On Mon, Oct 05, 2009 at 02:34:05PM -0700, David Lutterkort wrote:
On Mon, 2009-10-05 at 17:34 +0100, Daniel P. Berrange wrote:
With the domain XML format, we did have a few abortive attempts at indicating in the live XML, whether an attribute was from the persistent config, vs dynamically added to live config, but it all ended up as rather a mess.
Here's a concrete example where this leads to headscratching, at a minimum, with the netowrk configs. When the live XML contains
<protocol family="ipv4"> <dhcp/> <ip address="192.168.0.5" prefix="24"/> <route gateway="192.168.0.1"/> </protocol>
you at least think that the interface is configured to use DHCP, and currently has the indicated address. Of course, if we ever poke around to see if the interface _currently_ gets its address from DHCP, we have no way to indicate that anymore, since we would indicate 'configured to use DHCP' and 'currently gets its address from DHCP' in exactly the same way.
Well, it's actually problematic no matter what we do, because it's not easy to differentiate between an IP address on an interface that was put there by dhclient, and one that was put there manually (and might be replaced as soon as dhclient decide's it's time to renew its lease, or might stay there forever, if the dhclient process has died or been killed). We *could* parse dhclient.leases to find what dhclient believes to be the address, and compare it to what was returned from the ioctl, however my past experience with dhclient.leases was that there tended to be a lot of old leases in there, making it a real pain to find the correct lease. Maybe what we *really* need is an attribute in the 'state' XML to indicate where the currently in-use addresses originated, ie source='dhcp', source='static-config', source='manual', or whatever. I don't know how much success we'd have at getting these correct, though...
Also, US $5 to anybody who knows whether the<route/> tag above says 'is configured to use that gateway' or 'routing is through that gateway right now'.
Once I get all the address reporting correct, I'm going to see what I can do about reporting routes directly from the kernel, and once that is the case, it will always mean "routing is through that gateway right now". I don't think the current "route" node within the interface protocol will be adequate, though - for example, it has no destination/netmask, no metric, and no method to indicate if the route comes from a routing protocol, or is static. It's really only intended for setting up the default route. (Anyway, I don't know that the best way to report the current route table is by mixing it in with interface state - a separate API may be more appropriate)
The live XML should *always* say what the inteface is doing right now.
Up to the point that is possible, that's my intent.
To belabor this point, when the live XML says
<protocol family="ipv4"> <ip address="192.168.0.5" prefix="24"/> <ip address="192.168.0.6" prefix="24"/> </protocol>
it's completely unclear what addresses are in the config files (maybe both?) and which ones are actually assigned to the interface right now.
I think there is a disconnect here about the behaviour we should be exposing. In that example of live XML above with 2 IP addresses, both of those IP address *must* be currently assigned to the interface, since this is the live XML. To determine which (if any) of those addresses are in the config file, the VIR_INTEFACE_XML_INACTIVE flag would be used.
The summary of the two methods: 1) info about whether an address is the "live" address vs. from the config file is explicitly indicated by presence/absence of the "source='device'" attribute. 2) info about whether an address is the "live" address vs. from the config file is implied by which function was called / option flag was given. Of course in both cases, no config-derived addresses are returned in the XML (when the INACTIVE flag isn't set), so it is guaranteed that any addresses will be "live". I'm tending to lean towards (2) at the moment - having the extra attribute seems a bit redundant, as long as we guarantee that we're removing all the config-derived items before presentation. If we were going to try and combine both live and config-derived items in the same XML, I would vote for (1). (We actually had a short discussion about whether or not to mix live and config data on the netcf mailing list, but didn't get much participation...)
Looking at netcf, it does not appear to implement this correctly. If I query the live XML of an interface, it gives back a mixture of both the config file and active addresses. Indeed, if using a static IP address, it'll list the same IP address twice in the XML once it pulls it from the config file, and once from the interface.
This was a bug in the original patch, corrected by this set of patches: https://fedorahosted.org/pipermail/netcf-devel/2009-September/000280.html Once those patches are applied, only the IP (v4) addresses currently on the device are returned (IPv6 address reporting will work rsn.)
On Tue, Oct 06, 2009 at 09:55:06AM -0400, Laine Stump wrote:
On 10/06/2009 05:13 AM, Daniel P. Berrange wrote:
On Mon, Oct 05, 2009 at 02:34:05PM -0700, David Lutterkort wrote:
On Mon, 2009-10-05 at 17:34 +0100, Daniel P. Berrange wrote:
With the domain XML format, we did have a few abortive attempts at indicating in the live XML, whether an attribute was from the persistent config, vs dynamically added to live config, but it all ended up as rather a mess.
Here's a concrete example where this leads to headscratching, at a minimum, with the netowrk configs. When the live XML contains
<protocol family="ipv4"> <dhcp/> <ip address="192.168.0.5" prefix="24"/> <route gateway="192.168.0.1"/> </protocol>
you at least think that the interface is configured to use DHCP, and currently has the indicated address. Of course, if we ever poke around to see if the interface _currently_ gets its address from DHCP, we have no way to indicate that anymore, since we would indicate 'configured to use DHCP' and 'currently gets its address from DHCP' in exactly the same way.
Well, it's actually problematic no matter what we do, because it's not easy to differentiate between an IP address on an interface that was put there by dhclient, and one that was put there manually (and might be replaced as soon as dhclient decide's it's time to renew its lease, or might stay there forever, if the dhclient process has died or been killed). We *could* parse dhclient.leases to find what dhclient believes to be the address, and compare it to what was returned from the ioctl, however my past experience with dhclient.leases was that there tended to be a lot of old leases in there, making it a real pain to find the correct lease.
Maybe what we *really* need is an attribute in the 'state' XML to indicate where the currently in-use addresses originated, ie source='dhcp', source='static-config', source='manual', or whatever. I don't know how much success we'd have at getting these correct, though...
I don't think it is important where the address on a interface came from. As you say it is essentially impossible to determine tha after the fact, so its not worth trying to figure that out. Apps that really care that much would be fully managing networrking on the OS so likely already know where the addresses are coming from The libvirt interface XML should just focus on what's present, not where it came from.
The live XML should *always* say what the inteface is doing right now.
Up to the point that is possible, that's my intent.
If you restrict scope to reporting what is currently present, and not worry about where they came from, it should be pretty straightforward, after all, it is just presenting the same info as ifconfig / ip commands would in that scenario
To belabor this point, when the live XML says
<protocol family="ipv4"> <ip address="192.168.0.5" prefix="24"/> <ip address="192.168.0.6" prefix="24"/> </protocol>
it's completely unclear what addresses are in the config files (maybe both?) and which ones are actually assigned to the interface right now.
I think there is a disconnect here about the behaviour we should be exposing. In that example of live XML above with 2 IP addresses, both of those IP address *must* be currently assigned to the interface, since this is the live XML. To determine which (if any) of those addresses are in the config file, the VIR_INTEFACE_XML_INACTIVE flag would be used.
The summary of the two methods:
1) info about whether an address is the "live" address vs. from the config file is explicitly indicated by presence/absence of the "source='device'" attribute.
2) info about whether an address is the "live" address vs. from the config file is implied by which function was called / option flag was given.
Of course in both cases, no config-derived addresses are returned in the XML (when the INACTIVE flag isn't set), so it is guaranteed that any addresses will be "live".
[snip]
Looking at netcf, it does not appear to implement this correctly. If I query the live XML of an interface, it gives back a mixture of both the config file and active addresses. Indeed, if using a static IP address, it'll list the same IP address twice in the XML once it pulls it from the config file, and once from the interface.
This was a bug in the original patch, corrected by this set of patches:
https://fedorahosted.org/pipermail/netcf-devel/2009-September/000280.html
Once those patches are applied, only the IP (v4) addresses currently on the device are returned (IPv6 address reporting will work rsn.)
Ah, so that's not in the netcf 0.12 release then ? That's what I've been testing this stuff with & seeing the behaviour of mixed config and live addresses 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 :|
On 10/06/2009 10:13 AM, Daniel P. Berrange wrote:
This was a bug in the original patch, corrected by this set of patches:
https://fedorahosted.org/pipermail/netcf-devel/2009-September/000280.html
Once those patches are applied, only the IP (v4) addresses currently on the device are returned (IPv6 address reporting will work rsn.)
Ah, so that's not in the netcf 0.12 release then ? That's what I've been testing this stuff with& seeing the behaviour of mixed config and live addresses
Correct. David noticed the bug as he applied the first patches and made the release, and I submitted the fix just before he left for "puppet boot camp" or something like that (G20 Summit?), so it hasn't been committed yet.
On Tue, 2009-10-06 at 10:13 +0100, Daniel P. Berrange wrote:
The live XML should *always* say what the inteface is doing right now.
And from the rest of this exchange, I think the word 'only' should be added to that sentence. In other words, the XML for live setups contains *only* what we have figured out from the live setup, completely disregarding config files - that also leaves us room to add more live info as we gather more. For example, the entire live XML for any interface will now look like <interface name="eth0"> <protocol family="ipv4"> <ip address="192.168.0.52" prefix="24"/> </protocol> </interface> Note that that doesn't even indicate the type of the interface (which we may add at some point, but since we do not have that info right now, we won't pretend we know what the interface is) David
On 10/06/2009 01:36 PM, David Lutterkort wrote:
On Tue, 2009-10-06 at 10:13 +0100, Daniel P. Berrange wrote:
The live XML should *always* say what the inteface is doing right now.
And from the rest of this exchange, I think the word 'only' should be added to that sentence. In other words, the XML for live setups contains *only* what we have figured out from the live setup, completely disregarding config files - that also leaves us room to add more live info as we gather more. For example, the entire live XML for any interface will now look like
<interface name="eth0"> <protocol family="ipv4"> <ip address="192.168.0.52" prefix="24"/> </protocol> </interface>
Note that that doesn't even indicate the type of the interface (which we may add at some point, but since we do not have that info right now, we won't pretend we know what the interface is)
Interesting extrapolation. I can't say that I hadn't thought of it once or twice, but figured it would be more convenient to have one-stop shopping. (Also it will require a separate codepath for parse/format, but maybe that's better anyway.) Should I code this up and see how it flies?
On Tue, 2009-10-06 at 14:17 -0400, Laine Stump wrote:
On 10/06/2009 01:36 PM, David Lutterkort wrote:
On Tue, 2009-10-06 at 10:13 +0100, Daniel P. Berrange wrote:
The live XML should *always* say what the inteface is doing right now.
And from the rest of this exchange, I think the word 'only' should be added to that sentence. In other words, the XML for live setups contains *only* what we have figured out from the live setup, completely disregarding config files - that also leaves us room to add more live info as we gather more. For example, the entire live XML for any interface will now look like
<interface name="eth0"> <protocol family="ipv4"> <ip address="192.168.0.52" prefix="24"/> </protocol> </interface>
Note that that doesn't even indicate the type of the interface (which we may add at some point, but since we do not have that info right now, we won't pretend we know what the interface is)
Interesting extrapolation. I can't say that I hadn't thought of it once or twice, but figured it would be more convenient to have one-stop shopping. (Also it will require a separate codepath for parse/format, but maybe that's better anyway.)
Not necessarily - you just have to be mindful when parsing/formatting that many things the schema says are mandatory are in fact optional.
Should I code this up and see how it flies?
That's what I would do. David
On Mon, 2009-10-05 at 12:02 +0100, Daniel P. Berrange wrote:
On Tue, Sep 29, 2009 at 04:02:30PM -0400, Laine Stump wrote:
From: root <root@vlap.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 device directly, rather than just reporting what's in the config file. The backend of this is in netcf's new ncf_if_xml_state() function.
Any live interface ip address info in the xml will have the property "source" set to "device", eg:
<ip address='10.24.0.1' prefix='24' source='device'/>
This new 'source' attribute is bogus. The fact that the XML is the showing the device details, and not the config file details is inherent in the fact that we're querying the live interface. This distinction applies to the entire XML dump, not just the <ip> tag.
That's what I meant when I said 'I am not sure how this will be used in practice'. The options for this API call are (1) report configured and live setup side-by-side in the same XML, and mark the live setup somehow (e.g., with the source='device' attribute) (2) _only_ report live setup from this call, if you want configured setup use the VIR_INTERFACE_XML_INACTIVE flag. The advantage of (2) is that you can reuse any parsing/processsing code you might already have for the configured setup variant; the disadvantage is that you may have to handle the other variant's XML doc, too, and correlate the two. David
participants (5)
-
Daniel P. Berrange -
Daniel Veillard -
David Lutterkort -
Laine Stump -
Paolo Bonzini