[libvirt] [PATCH v2 0/5] Introduce API for dumping domain IP addresses

This feature has been requested for a very long time. However, we had to wait for guest agent to obtain reliable results as user might create totally different structure of interfaces than seen from outside (e.g. bonding, virtual interfaces, etc.). That's the main reason why sniffing for domain traffic can return bogus results. Fortunately, qemu guest agent implement requested part for a while so nothing holds us back anymore. To make matters worse, guest OS can assign whatever name to an interface and changing MAC inside guest isn't propagated to the host which in the end see original one. Therefore, finding correlation between interface within guest and the host side end is left as exercise for mgmt applications. This API is called virDomainGetInterfacesAddresses (okay, maybe too many plurals) and returns a XML document containing all interesting data. diff to v1: -switch from struct to XML doc Michal Privoznik (5): Introduce virDomainGetInterfacesAddresses API virsh: Expose virDomainGetInterfacesAddresses qemu_agent: Implement 'guest-network-get-interfaces' command handling qemu: Implement virDomainInterfacesAddresses python: create example for dumping domain IP addresses docs/schemas/interfaces.rng | 57 +++++++++++++++++ examples/python/Makefile.am | 2 +- examples/python/README | 1 + examples/python/domipaddrs.py | 62 +++++++++++++++++++ include/libvirt/libvirt.h.in | 2 + src/driver.h | 4 + src/libvirt.c | 49 +++++++++++++++ src/libvirt_public.syms | 1 + src/qemu/qemu_agent.c | 135 +++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_agent.h | 2 + src/qemu/qemu_driver.c | 68 +++++++++++++++++++++ src/remote/remote_driver.c | 1 + src/remote/remote_protocol.x | 12 +++- src/remote_protocol-structs | 8 +++ tools/virsh.c | 41 ++++++++++++ tools/virsh.pod | 9 +++ 16 files changed, 452 insertions(+), 2 deletions(-) create mode 100644 docs/schemas/interfaces.rng create mode 100644 examples/python/domipaddrs.py -- 1.7.8.5

This API returns dynamically allocated XML document showing IP addresses for all domain interfaces. --- docs/schemas/interfaces.rng | 57 ++++++++++++++++++++++++++++++++++++++++++ include/libvirt/libvirt.h.in | 2 + src/driver.h | 4 +++ src/libvirt.c | 49 ++++++++++++++++++++++++++++++++++++ src/libvirt_public.syms | 1 + src/remote/remote_driver.c | 1 + src/remote/remote_protocol.x | 12 ++++++++- src/remote_protocol-structs | 8 ++++++ 8 files changed, 133 insertions(+), 1 deletions(-) create mode 100644 docs/schemas/interfaces.rng diff --git a/docs/schemas/interfaces.rng b/docs/schemas/interfaces.rng new file mode 100644 index 0000000..95c939e --- /dev/null +++ b/docs/schemas/interfaces.rng @@ -0,0 +1,57 @@ +<!-- A Relax NG schema for the libvirt interfaces (return of + virDomainGetInterfacesAddresses) XML format --> +<grammar xmlns="http://relaxng.org/ns/structure/1.0" + datatypeLibrary="http://www.w3.org/2001/XMLSchema-datatypes"> + <include href='basictypes.rng'/> + <start> + <ref name='interfaces'/> + </start> + + + <define name='interfaces'> + <element name='interfaces'> + <zeroOrMore> + <ref name='interface'/> + </zeroOrMore> + </element> + </define> + + <define name='interface'> + <element name='interface'> + <element name='name'> + <text/> + </element> + <optional> + <element name='hwaddr'> + <text/> + </element> + </optional> + <optional> + <ref name='addresses'/> + </optional> + </element> + </define> + + <define name='addresses'> + <element name='addresses'> + <zeroOrMore> + <ref name='ip'/> + </zeroOrMore> + </element> + </define> + + <define name='ip'> + <element name='ip'> + <attribute name='type'> + <choice> + <value>ipv4</value> + <value>ipv6</value> + </choice> + </attribute> + <attribute name='prefix'> + <ref name='unsignedInt'/> + </attribute> + <text/> + </element> + </define> +</grammar> diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 024c4ec..90660d1 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -1659,6 +1659,8 @@ int virDomainGetInterfaceParameters (virDomainPtr dom, const char *device, virTypedParameterPtr params, int *nparams, unsigned int flags); +char * virDomainGetInterfacesAddresses (virDomainPtr dom, + unsigned int flags); /* Management of domain block devices */ diff --git a/src/driver.h b/src/driver.h index b3c1740..b2ed4b0 100644 --- a/src/driver.h +++ b/src/driver.h @@ -398,6 +398,9 @@ typedef int const char *device, virTypedParameterPtr params, int *nparams, unsigned int flags); +typedef char * + (*virDrvDomainGetInterfacesAddresses) (virDomainPtr dom, + unsigned int flags); typedef int (*virDrvDomainMemoryStats) @@ -966,6 +969,7 @@ struct _virDriver { virDrvDomainInterfaceStats domainInterfaceStats; virDrvDomainSetInterfaceParameters domainSetInterfaceParameters; virDrvDomainGetInterfaceParameters domainGetInterfaceParameters; + virDrvDomainGetInterfacesAddresses domainGetInterfacesAddresses; virDrvDomainMemoryStats domainMemoryStats; virDrvDomainBlockPeek domainBlockPeek; virDrvDomainMemoryPeek domainMemoryPeek; diff --git a/src/libvirt.c b/src/libvirt.c index 0aa50cb..2dadd6f 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -7357,6 +7357,55 @@ error: } /** + * virDomainGetInterfacesAddresses: + * @domain: domain object + * @flags: extra flags; not used yet, so callers should always pass 0 + * + * Return an XML document representing domain interfaces among with their IP + * addresses assigned. It's worth noticing that one interface can have multiple + * addresses or even none. + * + * For some hypervisors (e.g. qemu) a configured guest agent is needed. If + * guest agent is used, then the interface name will be the as seen by guest + * OS. To match such interface with the one from @domain XML us HW address or + * IP range. + * + * Returns an XML document or NULL on error. + * Callers *must* free returned pointer. + */ +char * +virDomainGetInterfacesAddresses(virDomainPtr domain, + unsigned int flags) +{ + virConnectPtr conn; + + VIR_DOMAIN_DEBUG(domain, "flags=%x", flags); + + virResetLastError(); + + if (!VIR_IS_CONNECTED_DOMAIN(domain)) { + virLibDomainError(VIR_ERR_INVALID_DOMAIN, __FUNCTION__); + goto error; + } + + conn = domain->conn; + + if (conn->driver->domainGetInterfacesAddresses) { + char *ret; + ret = conn->driver->domainGetInterfacesAddresses(domain, flags); + if (!ret) + goto error; + return ret; + } + + virLibConnError(VIR_ERR_NO_SUPPORT, __FUNCTION__); + +error: + virDispatchError(domain ? domain->conn : NULL); + return NULL; +} + +/** * virDomainMemoryStats: * @dom: pointer to the domain object * @stats: nr_stats-sized array of stat structures (returned) diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms index 2913a81..20dd08d 100644 --- a/src/libvirt_public.syms +++ b/src/libvirt_public.syms @@ -542,6 +542,7 @@ LIBVIRT_0.9.13 { virDomainSnapshotIsCurrent; virDomainSnapshotListAllChildren; virDomainSnapshotRef; + virDomainGetInterfacesAddresses; } LIBVIRT_0.9.11; # .... define new API here using predicted next version number .... diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index 6f53264..40a7a9c 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -5226,6 +5226,7 @@ static virDriver remote_driver = { .domainInterfaceStats = remoteDomainInterfaceStats, /* 0.3.2 */ .domainSetInterfaceParameters = remoteDomainSetInterfaceParameters, /* 0.9.9 */ .domainGetInterfaceParameters = remoteDomainGetInterfaceParameters, /* 0.9.9 */ + .domainGetInterfacesAddresses = remoteDomainGetInterfacesAddresses, /* 0.9.13 */ .domainMemoryStats = remoteDomainMemoryStats, /* 0.7.5 */ .domainBlockPeek = remoteDomainBlockPeek, /* 0.4.2 */ .domainMemoryPeek = remoteDomainMemoryPeek, /* 0.4.2 */ diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x index 1da9f3e..ea37270 100644 --- a/src/remote/remote_protocol.x +++ b/src/remote/remote_protocol.x @@ -653,6 +653,15 @@ struct remote_domain_get_interface_parameters_ret { int nparams; }; +struct remote_domain_get_interfaces_addresses_args { + remote_nonnull_domain dom; + unsigned int flags; +}; + +struct remote_domain_get_interfaces_addresses_ret { + remote_nonnull_string xml; +}; + struct remote_domain_memory_stats_args { remote_nonnull_domain dom; unsigned int maxStats; @@ -2838,7 +2847,8 @@ enum remote_procedure { REMOTE_PROC_DOMAIN_SNAPSHOT_HAS_METADATA = 272, /* autogen autogen */ REMOTE_PROC_CONNECT_LIST_ALL_DOMAINS = 273, /* skipgen skipgen priority:high */ REMOTE_PROC_DOMAIN_LIST_ALL_SNAPSHOTS = 274, /* skipgen skipgen priority:high */ - REMOTE_PROC_DOMAIN_SNAPSHOT_LIST_ALL_CHILDREN = 275 /* skipgen skipgen priority:high */ + REMOTE_PROC_DOMAIN_SNAPSHOT_LIST_ALL_CHILDREN = 275, /* skipgen skipgen priority:high */ + REMOTE_PROC_DOMAIN_GET_INTERFACES_ADDRESSES = 276 /* autogen autogen */ /* * Notice how the entries are grouped in sets of 10 ? diff --git a/src/remote_protocol-structs b/src/remote_protocol-structs index b667527..fadff38 100644 --- a/src/remote_protocol-structs +++ b/src/remote_protocol-structs @@ -354,6 +354,13 @@ struct remote_domain_get_interface_parameters_ret { } params; int nparams; }; +struct remote_domain_get_interfaces_addresses_args { + remote_nonnull_domain dom; + u_int flags; +}; +struct remote_domain_get_interfaces_addresses_ret { + remote_nonnull_string xml; +}; struct remote_domain_memory_stats_args { remote_nonnull_domain dom; u_int maxStats; @@ -2246,4 +2253,5 @@ enum remote_procedure { REMOTE_PROC_CONNECT_LIST_ALL_DOMAINS = 273, REMOTE_PROC_DOMAIN_LIST_ALL_SNAPSHOTS = 274, REMOTE_PROC_DOMAIN_SNAPSHOT_LIST_ALL_CHILDREN = 275, + REMOTE_PROC_DOMAIN_GET_INTERFACES_ADDRESSES = 276, }; -- 1.7.8.5

On 06/21/12 15:55, Michal Privoznik wrote:
This API returns dynamically allocated XML document showing IP addresses for all domain interfaces. --- docs/schemas/interfaces.rng | 57 ++++++++++++++++++++++++++++++++++++++++++ include/libvirt/libvirt.h.in | 2 + src/driver.h | 4 +++ src/libvirt.c | 49 ++++++++++++++++++++++++++++++++++++ src/libvirt_public.syms | 1 + src/remote/remote_driver.c | 1 + src/remote/remote_protocol.x | 12 ++++++++- src/remote_protocol-structs | 8 ++++++ 8 files changed, 133 insertions(+), 1 deletions(-) create mode 100644 docs/schemas/interfaces.rng
diff --git a/docs/schemas/interfaces.rng b/docs/schemas/interfaces.rng new file mode 100644 index 0000000..95c939e --- /dev/null +++ b/docs/schemas/interfaces.rng @@ -0,0 +1,57 @@
Missing XML doc header: <?xml version="1.0"?>
+<!-- A Relax NG schema for the libvirt interfaces (return of + virDomainGetInterfacesAddresses) XML format --> +<grammar xmlns="http://relaxng.org/ns/structure/1.0" + datatypeLibrary="http://www.w3.org/2001/XMLSchema-datatypes"> + <include href='basictypes.rng'/> + <start> + <ref name='interfaces'/> + </start> + + + <define name='interfaces'> + <element name='interfaces'> + <zeroOrMore> + <ref name='interface'/> + </zeroOrMore> + </element> + </define> + + <define name='interface'> + <element name='interface'> + <element name='name'> + <text/> + </element> + <optional> + <element name='hwaddr'> + <text/> + </element> + </optional> + <optional> + <ref name='addresses'/> + </optional> + </element> + </define> + + <define name='addresses'> + <element name='addresses'> + <zeroOrMore> + <ref name='ip'/> + </zeroOrMore> + </element> + </define> + + <define name='ip'> + <element name='ip'> + <attribute name='type'> + <choice> + <value>ipv4</value> + <value>ipv6</value> + </choice> + </attribute> + <attribute name='prefix'> + <ref name='unsignedInt'/> + </attribute> + <text/> + </element> + </define> +</grammar>
The XML looks reasonable to me, but I'd appreciate some other opinions on this as this is a architectural decision that we'll have to support forever. (Or maybe just until 21.12.2012? :))
diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 024c4ec..90660d1 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -1659,6 +1659,8 @@ int virDomainGetInterfaceParameters (virDomainPtr dom, const char *device, virTypedParameterPtr params, int *nparams, unsigned int flags); +char * virDomainGetInterfacesAddresses (virDomainPtr dom, + unsigned int flags);
Bad indent: missing one space. Looking at the file, I'm taking this objection back as the problem is worthy for separate cleanup.
/* Management of domain block devices */
diff --git a/src/driver.h b/src/driver.h index b3c1740..b2ed4b0 100644 --- a/src/driver.h +++ b/src/driver.h @@ -398,6 +398,9 @@ typedef int const char *device, virTypedParameterPtr params, int *nparams, unsigned int flags); +typedef char * + (*virDrvDomainGetInterfacesAddresses) (virDomainPtr dom, + unsigned int flags);
Bad indent: missing one space. Looking at the file, there are more of such problems. My cleanup of driver.h wasn't thorough enough.
typedef int (*virDrvDomainMemoryStats) diff --git a/src/libvirt.c b/src/libvirt.c index 0aa50cb..2dadd6f 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -7357,6 +7357,55 @@ error: }
/** + * virDomainGetInterfacesAddresses: + * @domain: domain object + * @flags: extra flags; not used yet, so callers should always pass 0 + * + * Return an XML document representing domain interfaces among with their IP + * addresses assigned. It's worth noticing that one interface can have multiple
I'd rephrase the beginning of the sentence to "Note that an interface can have multiple addresses or even none."
+ * addresses or even none. + * + * For some hypervisors (e.g. qemu) a configured guest agent is needed. If + * guest agent is used, then the interface name will be the as seen by guest + * OS. To match such interface with the one from @domain XML us HW address or + * IP range.
I'd rephrase this paragraph: Some hypervisors (e.g. qemu) require a configured guest agent instance running in the guest. The guest agent may return interface names as seen by the guest operating system. To match them to host interfaces use the HW addresses and IP ranges from domain's XML.
+ * + * Returns an XML document or NULL on error. + * Callers *must* free returned pointer. + */ +
+/** * virDomainMemoryStats: * @dom: pointer to the domain object * @stats: nr_stats-sized array of stat structures (returned) diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms index 2913a81..20dd08d 100644 --- a/src/libvirt_public.syms +++ b/src/libvirt_public.syms @@ -542,6 +542,7 @@ LIBVIRT_0.9.13 { virDomainSnapshotIsCurrent; virDomainSnapshotListAllChildren; virDomainSnapshotRef; + virDomainGetInterfacesAddresses;
Hopefully it'll be sorted out until the release, otherwise you'll have to bump these numbers.
} LIBVIRT_0.9.11;
# .... define new API here using predicted next version number ....
I ACK (with the XML header added) the technical part of this patch but I'd prefer if someone could do a architectural and language review. Peter

--- tools/virsh.c | 41 +++++++++++++++++++++++++++++++++++++++++ tools/virsh.pod | 9 +++++++++ 2 files changed, 50 insertions(+), 0 deletions(-) diff --git a/tools/virsh.c b/tools/virsh.c index a117424..fe52af0 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -1637,6 +1637,46 @@ cleanup: } #undef DOMBLKSTAT_LEGACY_PRINT +/* "domifaddr" command + */ +static const vshCmdInfo info_domifaddr[] = { + {"help", N_("get network interfaces addresses for a domain")}, + {"desc", N_("Get network interfaces addresses for a running domain")}, + {NULL, NULL} +}; + +static const vshCmdOptDef opts_domifaddr[] = { + {"domain", VSH_OT_DATA, VSH_OFLAG_REQ, N_("domain name, id or uuid")}, + {NULL, 0, 0, NULL} +}; + +static bool +cmdDomIfAddr(vshControl *ctl, const vshCmd *cmd) +{ + virDomainPtr dom = NULL; + unsigned int flags = 0; + char *xml = NULL; + bool ret = false; + + if (!vshConnectionUsability(ctl, ctl->conn)) + return false; + + if (!(dom = vshCommandOptDomain(ctl, cmd, NULL))) + return false; + + if (!(xml = virDomainGetInterfacesAddresses(dom, flags))) + goto cleanup; + + vshPrint(ctl, "%s", xml); + + ret = true; +cleanup: + VIR_FREE(xml); + virDomainFree(dom); + return ret; +} + + /* "domifstat" command */ static const vshCmdInfo info_domifstat[] = { @@ -17841,6 +17881,7 @@ static const vshCmdDef domMonitoringCmds[] = { {"domblkstat", cmdDomblkstat, opts_domblkstat, info_domblkstat, 0}, {"domcontrol", cmdDomControl, opts_domcontrol, info_domcontrol, 0}, {"domif-getlink", cmdDomIfGetLink, opts_domif_getlink, info_domif_getlink, 0}, + {"domifaddr", cmdDomIfAddr, opts_domifaddr, info_domifaddr, 0}, {"domiflist", cmdDomiflist, opts_domiflist, info_domiflist, 0}, {"domifstat", cmdDomIfstat, opts_domifstat, info_domifstat, 0}, {"dominfo", cmdDominfo, opts_dominfo, info_dominfo, 0}, diff --git a/tools/virsh.pod b/tools/virsh.pod index 839c156..b53ef8c 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -563,6 +563,15 @@ B<Explanation of fields> (fields appear in the folowing order): flush_total_times - total time flush operations took (ns) <-- other fields provided by hypervisor --> +=item B<domifaddr> I<domain> + +Get a list of interfaces of domain among with their IP and hardware addresses. +Note, that interface name can be driver dependent meaning it can be name within +guest OS or the name you would see in domain XML. Moreover, the whole command +may require a guest agent to be configured for the queried domain under some +drivers, notably qemu. To match interfaces from guest to host use MAC address +or IP address range. + =item B<domifstat> I<domain> I<interface-device> Get network interface stats for a running domain. -- 1.7.8.5

On 06/21/12 15:55, Michal Privoznik wrote:
--- tools/virsh.c | 41 +++++++++++++++++++++++++++++++++++++++++ tools/virsh.pod | 9 +++++++++ 2 files changed, 50 insertions(+), 0 deletions(-)
/* "domifstat" command */ static const vshCmdInfo info_domifstat[] = { @@ -17841,6 +17881,7 @@ static const vshCmdDef domMonitoringCmds[] = { {"domblkstat", cmdDomblkstat, opts_domblkstat, info_domblkstat, 0}, {"domcontrol", cmdDomControl, opts_domcontrol, info_domcontrol, 0}, {"domif-getlink", cmdDomIfGetLink, opts_domif_getlink, info_domif_getlink, 0}, + {"domifaddr", cmdDomIfAddr, opts_domifaddr, info_domifaddr, 0}, {"domiflist", cmdDomiflist, opts_domiflist, info_domiflist, 0}, {"domifstat", cmdDomIfstat, opts_domifstat, info_domifstat, 0}, {"dominfo", cmdDominfo, opts_dominfo, info_dominfo, 0},
Pretty straightforward expousure of the API. I'd probably go for a name that contains XML to denote what we're returning. It's because I'd like to see a tabular output like other commands and maybe return the XML with flag for that command. ACK for correctness of this patch, but I'll look into doing the tabular output after reviewing the rest of the series. Peter

This command returns an array of all guest interfaces among with their IP and HW addresses. --- src/qemu/qemu_agent.c | 135 +++++++++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_agent.h | 2 + 2 files changed, 137 insertions(+), 0 deletions(-) diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c index 7a0381c..cab02a0 100644 --- a/src/qemu/qemu_agent.c +++ b/src/qemu/qemu_agent.c @@ -1410,3 +1410,138 @@ qemuAgentSuspend(qemuAgentPtr mon, virJSONValueFree(reply); return ret; } + +char * +qemuAgentGetInterfaces(qemuAgentPtr mon) +{ + char *ret = NULL; + int i, j, size = -1; + virJSONValuePtr cmd; + virJSONValuePtr reply = NULL; + virJSONValuePtr ret_array = NULL; + virBuffer buf = VIR_BUFFER_INITIALIZER; + + cmd = qemuAgentMakeCommand("guest-network-get-interfaces", NULL); + + if (!cmd) + return NULL; + + if (qemuAgentCommand(mon, cmd, &reply) < 0 || + qemuAgentCheckError(cmd, reply) < 0) + goto cleanup; + + if (!(ret_array = virJSONValueObjectGet(reply, "return"))) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("qemu agent didn't provide 'return' field")); + goto cleanup; + } + + if ((size = virJSONValueArraySize(ret_array)) < 0) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("qemu agent didn't return an array of interfaces")); + goto cleanup; + } + + virBufferAddLit(&buf, "<interfaces>\n"); + virBufferAdjustIndent(&buf, 2); + + for (i = 0; i < size; i++) { + virJSONValuePtr tmp_iface = virJSONValueArrayGet(ret_array, i); + virJSONValuePtr ip_addr_arr = NULL; + const char *name, *hwaddr; + int ip_addr_arr_size; + + /* Shouldn't happen but doesn't hurt to check neither */ + if (!tmp_iface) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("something has went really wrong")); + goto cleanup; + } + + virBufferAddLit(&buf, "<interface>\n"); + virBufferAdjustIndent(&buf, 2); + + /* interface name is required to be presented */ + name = virJSONValueObjectGetString(tmp_iface, "name"); + if (!name) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("qemu agent didn't provide 'name' field")); + goto cleanup; + } + virBufferAsprintf(&buf, "<name>%s</name>\n", name); + + /* hwaddr might be omitted */ + hwaddr = virJSONValueObjectGetString(tmp_iface, "hardware-address"); + if (hwaddr) + virBufferAsprintf(&buf, "<hwaddr>%s</hwaddr>\n", hwaddr); + + /* as well as IP address which - moreover - + * can be presented multiple times */ + ip_addr_arr = virJSONValueObjectGet(tmp_iface, "ip-addresses"); + if (!ip_addr_arr) + goto interface_cont; + + if ((ip_addr_arr_size = virJSONValueArraySize(ip_addr_arr)) < 0) { + /* Mmm, empty 'ip-address'? */ + goto interface_cont; + } + + virBufferAddLit(&buf, "<addresses>\n"); + virBufferAdjustIndent(&buf, 2); + for (j = 0; j < ip_addr_arr_size; j++) { + virJSONValuePtr ip_addr_obj = virJSONValueArrayGet(ip_addr_arr, j); + const char *type, *addr; + int prefix; + + /* Shouldn't happen but doesn't hurt to check neither */ + if (!ip_addr_obj) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("something has went really wrong")); + goto cleanup; + } + + type = virJSONValueObjectGetString(ip_addr_obj, "ip-address-type"); + if (!type) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("qemu agent didn't provide 'ip-address-type'" + " field for interface '%s'"), name); + goto cleanup; + } + + addr = virJSONValueObjectGetString(ip_addr_obj, "ip-address"); + if (!addr) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("qemu agent didn't provide 'ip-address'" + " field for interface '%s'"), name); + goto cleanup; + } + + if (virJSONValueObjectGetNumberInt(ip_addr_obj, "prefix", + &prefix) < 0) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("malformed 'prefix' field")); + goto cleanup; + } + virBufferAsprintf(&buf, "<ip type='%s' prefix='%d'>%s</ip>\n", + type, prefix, addr); + } + + virBufferAdjustIndent(&buf, -2); + virBufferAddLit(&buf, "</addresses>\n"); + +interface_cont: + virBufferAdjustIndent(&buf, -2); + virBufferAddLit(&buf, "</interface>\n"); + } + + virBufferAdjustIndent(&buf, -2); + virBufferAddLit(&buf, "</interfaces>\n"); + + ret = virBufferContentAndReset(&buf); + +cleanup: + virBufferFreeAndReset(&buf); + virJSONValueFree(cmd); + virJSONValueFree(reply); + return ret; +} diff --git a/src/qemu/qemu_agent.h b/src/qemu/qemu_agent.h index 0816d90..1561652 100644 --- a/src/qemu/qemu_agent.h +++ b/src/qemu/qemu_agent.h @@ -80,4 +80,6 @@ int qemuAgentFSThaw(qemuAgentPtr mon); int qemuAgentSuspend(qemuAgentPtr mon, unsigned int target); + +char *qemuAgentGetInterfaces(qemuAgentPtr mon); #endif /* __QEMU_AGENT_H__ */ -- 1.7.8.5

On 06/21/12 15:56, Michal Privoznik wrote:
This command returns an array of all guest interfaces among with their IP and HW addresses. --- src/qemu/qemu_agent.c | 135 +++++++++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_agent.h | 2 + 2 files changed, 137 insertions(+), 0 deletions(-)
diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c index 7a0381c..cab02a0 100644 --- a/src/qemu/qemu_agent.c +++ b/src/qemu/qemu_agent.c @@ -1410,3 +1410,138 @@ qemuAgentSuspend(qemuAgentPtr mon, virJSONValueFree(reply); return ret; } + +char * +qemuAgentGetInterfaces(qemuAgentPtr mon) +{ + char *ret = NULL; + int i, j, size = -1; + virJSONValuePtr cmd; + virJSONValuePtr reply = NULL; + virJSONValuePtr ret_array = NULL; + virBuffer buf = VIR_BUFFER_INITIALIZER; + + cmd = qemuAgentMakeCommand("guest-network-get-interfaces", NULL); + + if (!cmd) + return NULL; + + if (qemuAgentCommand(mon, cmd, &reply) < 0 || + qemuAgentCheckError(cmd, reply) < 0) + goto cleanup; + + if (!(ret_array = virJSONValueObjectGet(reply, "return"))) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("qemu agent didn't provide 'return' field")); + goto cleanup; + } + + if ((size = virJSONValueArraySize(ret_array)) < 0) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("qemu agent didn't return an array of interfaces")); + goto cleanup; + } + + virBufferAddLit(&buf, "<interfaces>\n"); + virBufferAdjustIndent(&buf, 2); + + for (i = 0; i < size; i++) { + virJSONValuePtr tmp_iface = virJSONValueArrayGet(ret_array, i); + virJSONValuePtr ip_addr_arr = NULL; + const char *name, *hwaddr; + int ip_addr_arr_size; + + /* Shouldn't happen but doesn't hurt to check neither */ + if (!tmp_iface) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("something has went really wrong"));
If you check for this to happen, you should provide a more helpful message for both the user and for you if somebody complains: "failed to extract interface information" perhaps?
+ goto cleanup; + } + + virBufferAddLit(&buf, "<interface>\n"); + virBufferAdjustIndent(&buf, 2); + + /* interface name is required to be presented */ + name = virJSONValueObjectGetString(tmp_iface, "name"); + if (!name) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("qemu agent didn't provide 'name' field")); + goto cleanup; + } + virBufferAsprintf(&buf, "<name>%s</name>\n", name); + + /* hwaddr might be omitted */ + hwaddr = virJSONValueObjectGetString(tmp_iface, "hardware-address"); + if (hwaddr) + virBufferAsprintf(&buf, "<hwaddr>%s</hwaddr>\n", hwaddr); + + /* as well as IP address which - moreover - + * can be presented multiple times */ + ip_addr_arr = virJSONValueObjectGet(tmp_iface, "ip-addresses"); + if (!ip_addr_arr) + goto interface_cont; + + if ((ip_addr_arr_size = virJSONValueArraySize(ip_addr_arr)) < 0) { + /* Mmm, empty 'ip-address'? */ + goto interface_cont; + } + + virBufferAddLit(&buf, "<addresses>\n"); + virBufferAdjustIndent(&buf, 2); + for (j = 0; j < ip_addr_arr_size; j++) { + virJSONValuePtr ip_addr_obj = virJSONValueArrayGet(ip_addr_arr, j); + const char *type, *addr; + int prefix; + + /* Shouldn't happen but doesn't hurt to check neither */ + if (!ip_addr_obj) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("something has went really wrong"));
Same as above. A bit more explanatory error message would be great.
+ goto cleanup; + } + + type = virJSONValueObjectGetString(ip_addr_obj, "ip-address-type"); + if (!type) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("qemu agent didn't provide 'ip-address-type'" + " field for interface '%s'"), name); + goto cleanup; + } + + addr = virJSONValueObjectGetString(ip_addr_obj, "ip-address"); + if (!addr) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("qemu agent didn't provide 'ip-address'" + " field for interface '%s'"), name); + goto cleanup; + } + + if (virJSONValueObjectGetNumberInt(ip_addr_obj, "prefix", + &prefix) < 0) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("malformed 'prefix' field")); + goto cleanup; + } + virBufferAsprintf(&buf, "<ip type='%s' prefix='%d'>%s</ip>\n", + type, prefix, addr); + } + + virBufferAdjustIndent(&buf, -2); + virBufferAddLit(&buf, "</addresses>\n"); + +interface_cont: + virBufferAdjustIndent(&buf, -2); + virBufferAddLit(&buf, "</interface>\n"); + } + + virBufferAdjustIndent(&buf, -2); + virBufferAddLit(&buf, "</interfaces>\n"); + + ret = virBufferContentAndReset(&buf); + +cleanup: + virBufferFreeAndReset(&buf); + virJSONValueFree(cmd); + virJSONValueFree(reply); + return ret; +}
ACK with error messages tweaked. Peter

--- src/qemu/qemu_driver.c | 68 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 68 insertions(+), 0 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 2177c30..7387911 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -13157,6 +13157,73 @@ qemuListAllDomains(virConnectPtr conn, return ret; } +static char * +qemuDomainGetInterfacesAddresses(virDomainPtr dom, + unsigned int flags) +{ + struct qemud_driver *driver = dom->conn->privateData; + qemuDomainObjPrivatePtr priv = NULL; + virDomainObjPtr vm = NULL; + char *ret = NULL; + + virCheckFlags(0, NULL); + + qemuDriverLock(driver); + vm = virDomainFindByUUID(&driver->domains, dom->uuid); + qemuDriverUnlock(driver); + + if (!vm) { + char uuidstr[VIR_UUID_STRING_BUFLEN]; + virUUIDFormat(dom->uuid, uuidstr); + qemuReportError(VIR_ERR_NO_DOMAIN, + _("no domain with matching uuid '%s'"), uuidstr); + goto cleanup; + } + + if (!virDomainObjIsActive(vm)) { + qemuReportError(VIR_ERR_OPERATION_INVALID, "%s", + _("domain is not running")); + goto cleanup; + } + + priv = vm->privateData; + + if (priv->agentError) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("QEMU guest agent is not " + "available due to an error")); + goto cleanup; + } + + if (!priv->agent) { + qemuReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s", + _("QEMU guest agent is not configured")); + goto cleanup; + } + + if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_QUERY) < 0) + goto cleanup; + + if (!virDomainObjIsActive(vm)) { + qemuReportError(VIR_ERR_OPERATION_INVALID, "%s", + _("domain is not running")); + goto endjob; + } + + qemuDomainObjEnterAgent(driver, vm); + ret = qemuAgentGetInterfaces(priv->agent); + qemuDomainObjExitAgent(driver, vm); + +endjob: + if (qemuDomainObjEndJob(driver, vm) == 0) + vm = NULL; + +cleanup: + if (vm) + virDomainObjUnlock(vm); + return ret; +} + static virDriver qemuDriver = { .no = VIR_DRV_QEMU, .name = QEMU_DRIVER_NAME, @@ -13322,6 +13389,7 @@ static virDriver qemuDriver = { .domainPMSuspendForDuration = qemuDomainPMSuspendForDuration, /* 0.9.11 */ .domainPMWakeup = qemuDomainPMWakeup, /* 0.9.11 */ .domainGetCPUStats = qemuDomainGetCPUStats, /* 0.9.11 */ + .domainGetInterfacesAddresses = qemuDomainGetInterfacesAddresses, /* 0.9.13 */ }; -- 1.7.8.5

On 06/21/12 15:56, Michal Privoznik wrote:
--- src/qemu/qemu_driver.c | 68 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 68 insertions(+), 0 deletions(-)
+ static virDriver qemuDriver = { .no = VIR_DRV_QEMU, .name = QEMU_DRIVER_NAME, @@ -13322,6 +13389,7 @@ static virDriver qemuDriver = { .domainPMSuspendForDuration = qemuDomainPMSuspendForDuration, /* 0.9.11 */ .domainPMWakeup = qemuDomainPMWakeup, /* 0.9.11 */ .domainGetCPUStats = qemuDomainGetCPUStats, /* 0.9.11 */ + .domainGetInterfacesAddresses = qemuDomainGetInterfacesAddresses, /* 0.9.13 */ };
In the driver struct you've ordered the function between similar functions but here's at the end. diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index 6f53264..40a7a9c 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -5226,6 +5226,7 @@ static virDriver remote_driver = { .domainInterfaceStats = remoteDomainInterfaceStats, /* 0.3.2 */ .domainSetInterfaceParameters = remoteDomainSetInterfaceParameters, /* 0.9.9 */ .domainGetInterfaceParameters = remoteDomainGetInterfaceParameters, /* 0.9.9 */ + .domainGetInterfacesAddresses = remoteDomainGetInterfacesAddresses, /* 0.9.13 */ .domainMemoryStats = remoteDomainMemoryStats, /* 0.7.5 */ .domainBlockPeek = remoteDomainBlockPeek, /* 0.4.2 */ .domainMemoryPeek = remoteDomainMemoryPeek, /* 0.4.2 */ ACK with reordered driver struct initialization. Peter

--- examples/python/Makefile.am | 2 +- examples/python/README | 1 + examples/python/domipaddrs.py | 62 +++++++++++++++++++++++++++++++++++++++++ 3 files changed, 64 insertions(+), 1 deletions(-) create mode 100644 examples/python/domipaddrs.py diff --git a/examples/python/Makefile.am b/examples/python/Makefile.am index b04d126..b51f729 100644 --- a/examples/python/Makefile.am +++ b/examples/python/Makefile.am @@ -4,4 +4,4 @@ EXTRA_DIST= \ README \ consolecallback.py \ - dominfo.py domrestore.py domsave.py domstart.py esxlist.py + dominfo.py domrestore.py domsave.py domstart.py esxlist.py domipaddrs.py diff --git a/examples/python/README b/examples/python/README index f4db76c..d895740 100644 --- a/examples/python/README +++ b/examples/python/README @@ -10,6 +10,7 @@ domsave.py - save all running domU's into a directory domrestore.py - restore domU's from their saved files in a directory esxlist.py - list active domains of an VMware ESX host and print some info. also demonstrates how to use the libvirt.openAuth() method +domipaddrs.py - print domain interfaces among with their HW and IP addresses The XML files in this directory are examples of the XML format that libvirt expects, and will have to be adapted for your setup. They are only needed diff --git a/examples/python/domipaddrs.py b/examples/python/domipaddrs.py new file mode 100644 index 0000000..fd2d201 --- /dev/null +++ b/examples/python/domipaddrs.py @@ -0,0 +1,62 @@ +#!/usr/bin/env python +# domipaddrds - print IP addresses for running domain + +import libxml2 +import libvirt +import sys + +def usage(): + print "Usage: %s [URI] DOMAIN" % sys.argv[0] + print " Print IP addresses assigned to domain interfaces" + +uri = None +name = None +args = len(sys.argv) + +if args == 2: + name = sys.argv[1] +elif args == 3: + uri = sys.argv[1] + name = sys.argv[2] +else: + usage() + sys.exit(2) + +conn = libvirt.openReadOnly(uri) +if conn == None: + print "Unable to open connection to libvirt" + sys.exit(1) + +try: + dom = conn.lookupByName(name) +except libvirt.libvirtError: + print "Domain %s not found" % name + sys.exit(0) + +ifaces = dom.interfacesAddresses(0) +if (ifaces == None): + print "Failed to get domain interfaces" + sys.exit(0) + +doc = libxml2.parseDoc(ifaces) +ctxt = doc.xpathNewContext() +res = ctxt.xpathEval("/interfaces/interface/name/text()") + +print " {0:10} {1:17} {2}".format("Interface", "HW address", "IP Address") + +for interface in res: + hwaddr = ctxt.xpathEval("string(/interfaces/interface[descendant::name/text()='%s']/hwaddr/text())" % interface) + ip_addrs = ctxt.xpathEval("/interfaces/interface[descendant::name/text()='%s']/addresses/ip" % interface) + print " {0:10} {1:17}".format(interface, hwaddr), + + print " ", + for ip in ip_addrs: + prefix = "" + for i in ip.properties: + if i.name == "prefix": + prefix = i.content + print " {0}/{1}".format(ip.content, prefix), + + print + +doc.freeDoc() -- 1.7.8.5

On Thu, Jun 21, 2012 at 03:55:57PM +0200, Michal Privoznik wrote:
This feature has been requested for a very long time. However, we had to wait for guest agent to obtain reliable results as user might create totally different structure of interfaces than seen from outside (e.g. bonding, virtual interfaces, etc.). That's the main reason why sniffing for domain traffic can return bogus results. Fortunately, qemu guest agent implement requested part for a while so nothing holds us back anymore.
To make matters worse, guest OS can assign whatever name to an interface and changing MAC inside guest isn't propagated to the host which in the end see original one.
Therefore, finding correlation between interface within guest and the host side end is left as exercise for mgmt applications.
This API is called virDomainGetInterfacesAddresses (okay, maybe too many plurals) and returns a XML document containing all interesting data.
As I mentioned before, I'm not really seeing a compelling need or benefit for returning an XML document, instead of a struct. I'd only see the point if we anticipated that this XML document would expand to cover full interface config, as seen with netcf, but in that case, I'd expect the XML document to follow the network interface schema, rather than be a brand new schema. Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

On 21.06.2012 15:55, Michal Privoznik wrote:
This feature has been requested for a very long time. However, we had to wait for guest agent to obtain reliable results as user might create totally different structure of interfaces than seen from outside (e.g. bonding, virtual interfaces, etc.). That's the main reason why sniffing for domain traffic can return bogus results. Fortunately, qemu guest agent implement requested part for a while so nothing holds us back anymore.
To make matters worse, guest OS can assign whatever name to an interface and changing MAC inside guest isn't propagated to the host which in the end see original one.
Therefore, finding correlation between interface within guest and the host side end is left as exercise for mgmt applications.
This API is called virDomainGetInterfacesAddresses (okay, maybe too many plurals) and returns a XML document containing all interesting data.
diff to v1: -switch from struct to XML doc
Michal Privoznik (5): Introduce virDomainGetInterfacesAddresses API virsh: Expose virDomainGetInterfacesAddresses qemu_agent: Implement 'guest-network-get-interfaces' command handling qemu: Implement virDomainInterfacesAddresses python: create example for dumping domain IP addresses
docs/schemas/interfaces.rng | 57 +++++++++++++++++ examples/python/Makefile.am | 2 +- examples/python/README | 1 + examples/python/domipaddrs.py | 62 +++++++++++++++++++ include/libvirt/libvirt.h.in | 2 + src/driver.h | 4 + src/libvirt.c | 49 +++++++++++++++ src/libvirt_public.syms | 1 + src/qemu/qemu_agent.c | 135 +++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_agent.h | 2 + src/qemu/qemu_driver.c | 68 +++++++++++++++++++++ src/remote/remote_driver.c | 1 + src/remote/remote_protocol.x | 12 +++- src/remote_protocol-structs | 8 +++ tools/virsh.c | 41 ++++++++++++ tools/virsh.pod | 9 +++ 16 files changed, 452 insertions(+), 2 deletions(-) create mode 100644 docs/schemas/interfaces.rng create mode 100644 examples/python/domipaddrs.py
Ping? So do we have a consensus on this? The first version expose guest IP via public struct, while this one returns an XML doc. Which one should we prefer?

On Mon, Jul 30, 2012 at 12:59:27PM +0200, Michal Privoznik wrote:
On 21.06.2012 15:55, Michal Privoznik wrote:
This feature has been requested for a very long time. However, we had to wait for guest agent to obtain reliable results as user might create totally different structure of interfaces than seen from outside (e.g. bonding, virtual interfaces, etc.). That's the main reason why sniffing for domain traffic can return bogus results. Fortunately, qemu guest agent implement requested part for a while so nothing holds us back anymore.
To make matters worse, guest OS can assign whatever name to an interface and changing MAC inside guest isn't propagated to the host which in the end see original one.
Therefore, finding correlation between interface within guest and the host side end is left as exercise for mgmt applications.
This API is called virDomainGetInterfacesAddresses (okay, maybe too many plurals) and returns a XML document containing all interesting data.
diff to v1: -switch from struct to XML doc
Michal Privoznik (5): Introduce virDomainGetInterfacesAddresses API virsh: Expose virDomainGetInterfacesAddresses qemu_agent: Implement 'guest-network-get-interfaces' command handling qemu: Implement virDomainInterfacesAddresses python: create example for dumping domain IP addresses
docs/schemas/interfaces.rng | 57 +++++++++++++++++ examples/python/Makefile.am | 2 +- examples/python/README | 1 + examples/python/domipaddrs.py | 62 +++++++++++++++++++ include/libvirt/libvirt.h.in | 2 + src/driver.h | 4 + src/libvirt.c | 49 +++++++++++++++ src/libvirt_public.syms | 1 + src/qemu/qemu_agent.c | 135 +++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_agent.h | 2 + src/qemu/qemu_driver.c | 68 +++++++++++++++++++++ src/remote/remote_driver.c | 1 + src/remote/remote_protocol.x | 12 +++- src/remote_protocol-structs | 8 +++ tools/virsh.c | 41 ++++++++++++ tools/virsh.pod | 9 +++ 16 files changed, 452 insertions(+), 2 deletions(-) create mode 100644 docs/schemas/interfaces.rng create mode 100644 examples/python/domipaddrs.py
Ping?
So do we have a consensus on this? The first version expose guest IP via public struct, while this one returns an XML doc. Which one should we prefer?
It seems to me people suggesting either ways 1/ struct if we just return the IP adress[es] 2/ XML if we want to expand all the other informations about the interface At this point I would think we are not really in a good situation to extract and expose all the interface settings as seen from the guest and as Dan said it's more something to be done on the guest agent, that interface is a backup in the absence of agent or a way to bootstrap communication with the guest. I think v1 does this fine, though I would agree with Eric feedback on the API changes, I would also suggest -1 as being the error failure code for this API. Then later if we can get full client interfaces viewpoint, as seen from the guest (is there an API for this in VMWare, we ought to be able to do it for LXC) then providing a second extensible API returning an XML could be added, but I think the core API need now it be able to easilly extract the IP(s) of the guest, and XML while being more flexible for future use would get in the way. So my take would be to refine v1, and based on feedback of v1, availability of informations in various drivers, then provide an XML extensible version if there is a need and a good way to provide the full information set. 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/

On Tue, Jul 31, 2012 at 11:22:09AM +0800, Daniel Veillard wrote:
On Mon, Jul 30, 2012 at 12:59:27PM +0200, Michal Privoznik wrote:
On 21.06.2012 15:55, Michal Privoznik wrote:
This feature has been requested for a very long time. However, we had to wait for guest agent to obtain reliable results as user might create totally different structure of interfaces than seen from outside (e.g. bonding, virtual interfaces, etc.). That's the main reason why sniffing for domain traffic can return bogus results. Fortunately, qemu guest agent implement requested part for a while so nothing holds us back anymore.
To make matters worse, guest OS can assign whatever name to an interface and changing MAC inside guest isn't propagated to the host which in the end see original one.
Therefore, finding correlation between interface within guest and the host side end is left as exercise for mgmt applications.
This API is called virDomainGetInterfacesAddresses (okay, maybe too many plurals) and returns a XML document containing all interesting data.
diff to v1: -switch from struct to XML doc
Michal Privoznik (5): Introduce virDomainGetInterfacesAddresses API virsh: Expose virDomainGetInterfacesAddresses qemu_agent: Implement 'guest-network-get-interfaces' command handling qemu: Implement virDomainInterfacesAddresses python: create example for dumping domain IP addresses
docs/schemas/interfaces.rng | 57 +++++++++++++++++ examples/python/Makefile.am | 2 +- examples/python/README | 1 + examples/python/domipaddrs.py | 62 +++++++++++++++++++ include/libvirt/libvirt.h.in | 2 + src/driver.h | 4 + src/libvirt.c | 49 +++++++++++++++ src/libvirt_public.syms | 1 + src/qemu/qemu_agent.c | 135 +++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_agent.h | 2 + src/qemu/qemu_driver.c | 68 +++++++++++++++++++++ src/remote/remote_driver.c | 1 + src/remote/remote_protocol.x | 12 +++- src/remote_protocol-structs | 8 +++ tools/virsh.c | 41 ++++++++++++ tools/virsh.pod | 9 +++ 16 files changed, 452 insertions(+), 2 deletions(-) create mode 100644 docs/schemas/interfaces.rng create mode 100644 examples/python/domipaddrs.py
Ping?
So do we have a consensus on this? The first version expose guest IP via public struct, while this one returns an XML doc. Which one should we prefer?
It seems to me people suggesting either ways 1/ struct if we just return the IP adress[es] 2/ XML if we want to expand all the other informations about the interface
At this point I would think we are not really in a good situation to extract and expose all the interface settings as seen from the guest and as Dan said it's more something to be done on the guest agent, that interface is a backup in the absence of agent or a way to bootstrap communication with the guest. I think v1 does this fine, though I would agree with Eric feedback on the API changes, I would also suggest -1 as being the error failure code for this API. Then later if we can get full client interfaces viewpoint, as seen from the guest (is there an API for this in VMWare, we ought to be able to do it for LXC) then providing a second extensible API returning an XML could be added, but I think the core API need now it be able to easilly extract the IP(s) of the guest, and XML while being more flexible for future use would get in the way.
So my take would be to refine v1, and based on feedback of v1, availability of informations in various drivers, then provide an XML extensible version if there is a need and a good way to provide the full information set.
I think I've already mentioned I had a preference for v1 struct approach. If we did go for th v2 XML approach, then I'd want us to align with the virInteface XML schema for reporting interface configuration, instead of creating a new schema. FWIW, I don't think it would be the end of the world if we added a struct based API now, and then *also* added a XML based API at a later date (should it become required) Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

On Tue, Jul 31, 2012 at 05:38:31PM +0100, Daniel P. Berrange wrote:
On Tue, Jul 31, 2012 at 11:22:09AM +0800, Daniel Veillard wrote:
So do we have a consensus on this? The first version expose guest IP via public struct, while this one returns an XML doc. Which one should we prefer?
It seems to me people suggesting either ways 1/ struct if we just return the IP adress[es] 2/ XML if we want to expand all the other informations about the interface
At this point I would think we are not really in a good situation to extract and expose all the interface settings as seen from the guest and as Dan said it's more something to be done on the guest agent, that interface is a backup in the absence of agent or a way to bootstrap communication with the guest. I think v1 does this fine, though I would agree with Eric feedback on the API changes, I would also suggest -1 as being the error failure code for this API. Then later if we can get full client interfaces viewpoint, as seen from the guest (is there an API for this in VMWare, we ought to be able to do it for LXC) then providing a second extensible API returning an XML could be added, but I think the core API need now it be able to easilly extract the IP(s) of the guest, and XML while being more flexible for future use would get in the way.
So my take would be to refine v1, and based on feedback of v1, availability of informations in various drivers, then provide an XML extensible version if there is a need and a good way to provide the full information set.
I think I've already mentioned I had a preference for v1 struct approach. If we did go for th v2 XML approach, then I'd want us to align with the virInteface XML schema for reporting interface configuration, instead of creating a new schema.
FWIW, I don't think it would be the end of the world if we added a struct based API now, and then *also* added a XML based API at a later date (should it become required)
okay, we agree, 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/
participants (4)
-
Daniel P. Berrange
-
Daniel Veillard
-
Michal Privoznik
-
Peter Krempa