On 24/07/13 19:07, nehaljwani wrote:

Define an new API virDomainInterfacesAddresses, which returns the address information of a running domain's interfaces(s). If no interface name is specified, it returns the information of all interfaces, otherwise it only returns the information of the specificed interface. The address information includes the MAC and IP addresses.

The API is going to provide multiple methods by flags, e.g.

* Query guest agent * Parse lease file of dnsmasq * DHCP snooping

But at this stage, it will only work with guest agent, and flags won't be supported.

include/libvirt/libvirt.h.in: * Define virDomainInterfacesAddresses * Define structs virDomainInterface, virDomainIPAddress

python/generator.py: * Skip the auto-generation for virDomainInterfacesAddresses

src/driver.h: * Define domainInterfacesAddresses

src/libvirt.c: * Implement virDomainInterfacesAddresses

src/libvirt_public.syms: * Export the new symbol

--- include/libvirt/libvirt.h.in | 32 ++++++++++++++ python/generator.py | 1 + src/driver.h | 7 ++++ src/libvirt.c | 99 ++++++++++++++++++++++++++++++++++++++++++++ src/libvirt_public.syms | 5 +++ 5 files changed, 144 insertions(+)

diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index c0eb25b..183efba 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -2045,6 +2045,38 @@ int virDomainGetInterfaceParameters (virDomainPtr dom, virTypedParameterPtr params, int *nparams, unsigned int flags);

+typedef enum { + VIR_IP_ADDR_TYPE_IPV4, + VIR_IP_ADDR_TYPE_IPV6, + +#ifdef VIR_ENUM_SENTINELS + VIR_IP_ADDR_TYPE_LAST +#endif +} virIPAddrType; + + +typedef struct _virDomainInterfaceIPAddress virDomainIPAddress; +typedef virDomainIPAddress *virDomainIPAddressPtr; +struct _virDomainInterfaceIPAddress { + int type; /* virIPAddrType */ + char *addr; /* IP address */ + int prefix; /* IP address prefix */ +}; + +typedef struct _virDomainInterface virDomainInterface; +typedef virDomainInterface *virDomainInterfacePtr; +struct _virDomainInterface { + char *name; /* interface name */ + char *hwaddr; /* hardware address */ + unsigned int ip_addrs_count; /* number of items in @ip_addr */ + virDomainIPAddressPtr ip_addrs; /* array of IP addresses */ +}; + +int virDomainInterfacesAddresses (virDomainPtr dom, + virDomainInterfacePtr *ifaces, + unsigned int *ifaces_count, + unsigned int flags); + /* Management of domain block devices */

int virDomainBlockPeek (virDomainPtr dom, diff --git a/python/generator.py b/python/generator.py index 427cebc..ceef9a7 100755 --- a/python/generator.py +++ b/python/generator.py @@ -458,6 +458,7 @@ skip_impl = ( 'virNodeGetMemoryParameters', 'virNodeSetMemoryParameters', 'virNodeGetCPUMap', + 'virDomainInterfacesAddresses', 'virDomainMigrate3', 'virDomainMigrateToURI3', ) diff --git a/src/driver.h b/src/driver.h index cc03e9f..771dc35 100644 --- a/src/driver.h +++ b/src/driver.h @@ -518,6 +518,12 @@ typedef int unsigned int flags);

typedef int +(*virDrvDomainInterfacesAddresses)(virDomainPtr dom, + virDomainInterfacePtr *ifaces, + unsigned int *ifaces_count, + unsigned int flags); + +typedef int (*virDrvDomainMemoryStats)(virDomainPtr domain, struct _virDomainMemoryStat *stats, unsigned int nr_stats, @@ -1238,6 +1244,7 @@ struct _virDriver { virDrvDomainInterfaceStats domainInterfaceStats; virDrvDomainSetInterfaceParameters domainSetInterfaceParameters; virDrvDomainGetInterfaceParameters domainGetInterfaceParameters; + virDrvDomainInterfacesAddresses domainInterfacesAddresses; virDrvDomainMemoryStats domainMemoryStats; virDrvDomainBlockPeek domainBlockPeek; virDrvDomainMemoryPeek domainMemoryPeek; diff --git a/src/libvirt.c b/src/libvirt.c index 444c1c3..cb5639c 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -8629,6 +8629,105 @@ error: return -1; }

+ /** + * virDomainInterfacesAddresses: + * @dom: domain object + * @ifaces: array of @dom interfaces + * @ifaces_count: number of items in @ifaces + * @flags: extra flags; not used yet, so callers should always pass 0 + * + * Return an array of interfaces present in given domain along with + * their IP and MAC addresses. Note that single interface can have + * multiple or even 0 IP address. + * + * This API dynamically allocates the virDomainInterfacePtr struct + * based on how many interfaces domain @dom has, usually there's + * 1:1 correlation. The count of elements allocated is stored in + * @ifaces_count. + * + * Note that for some hypervisors, a configured guest agent is needed + * for successful return from this API. Moreover, if guest agent is + * used then the interface name is the one seen by guest OS. To match + * such interface with the one from @dom XML use MAC address or IP range. + * + * @ifaces->name is never NULL, @ifaces->hwaddr might be NULL. + * + * The caller *must* free @ifaces when no longer needed. Usual use + * case looks like this: + * + * virDomainInterfacePtr ifaces = NULL; + * unsigned int ifaces_count = 0; + * size_t i, j; + * virDomainPtr dom = ... obtain a domain here ...; + * + * if (virDomainInterfacesAddresses(dom, &ifaces, &ifaces_count, 0) < 0) + * goto cleanup; + * + * ... do something with returned values, for example: + * for (i = 0; i < ifaces_count; i++) { + * printf("name: %s", ifaces[i].name); + * if (ifaces[i].hwaddr) + * printf(" hwaddr: %s", ifaces[i].hwaddr); + * + * for (j = 0; j < ifaces[i].ip_addrs_count; j++) { + * virDomainIPAddressPtr ip_addr = ifaces[i].ip_addrs + j; + * printf("[addr: %s prefix: %d type: %d]", + * ip_addr->addr, ip_addr->prefix, ip_addr->type); + * } + * printf("\n"); + * } + * + * cleanup: + * for (i = 0; i < ifaces_count; i++) { + * free(ifaces[i].name); + * free(ifaces[i].hwaddr); + * for (j = 0; j < ifaces[i].ip_addrs_count; j++) + * free(ifaces[i].ip_addrs[j].addr); + * free(ifaces[i].ip_addrs); + * } + * free(ifaces); + * + * Returns 0 on success, -1 in case of error. + */ +int +virDomainInterfacesAddresses(virDomainPtr dom, + virDomainInterfacePtr *ifaces, + unsigned int *ifaces_count, + unsigned int flags) +{ + virConnectPtr conn; + + VIR_DOMAIN_DEBUG(dom, "ifaces=%p, ifaces_count=%p, flags=%x", + ifaces, ifaces_count, flags); + + virResetLastError(); + + if (!VIR_IS_CONNECTED_DOMAIN(dom)) { + virLibDomainError(VIR_ERR_INVALID_DOMAIN, __FUNCTION__); + goto error; + } + + conn = dom->conn; + + virCheckNonNullArgGoto(ifaces, error); + virCheckNonNullArgGoto(ifaces_count, error); + + if (conn->driver->domainInterfacesAddresses) { + int ret; + ret = conn->driver->domainInterfacesAddresses(dom, ifaces, + ifaces_count, flags); + if (ret < 0) + goto error; + return ret; + } + + virLibConnError(VIR_ERR_NO_SUPPORT, __FUNCTION__); + +error: + virDispatchError(dom ? dom->conn : NULL); + return -1; +} + /** * virDomainMemoryStats: * @dom: pointer to the domain object diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms index bbdf78a..6cbcfb4 100644 --- a/src/libvirt_public.syms +++ b/src/libvirt_public.syms @@ -634,4 +634,9 @@ LIBVIRT_1.1.1 { virDomainSetMemoryStatsPeriod; } LIBVIRT_1.1.0;

+LIBVIRT_1.1.2 { + global: + virDomainInterfacesAddresses; +} LIBVIRT_1.1.1; + # .... define new API here using predicted next version number ....

The set was reviewed by me several rounds privately, and this has no changes, ACK. Osier

On 24/07/13 19:07, nehaljwani wrote:

Implement RPC calls for virDomainInterfacesAddresses

daemon/remote.c * Define remoteSerializeDomainInterfacePtr, remoteDispatchDomainInterfacesAddresses

src/remote/remote_driver.c * Define remoteDomainInterfacesAddresses

src/remote/remote_protocol.x * New RPC procedure: REMOTE_PROC_DOMAIN_INTERFACES_ADDRESSES * Define structs remote_domain_ip_addr, remote_domain_interface, remote_domain_interfaces_addresses_args, remote_domain_interfaces_addresses_ret

src/remote_protocol-structs * New structs added --- daemon/remote.c | 123 +++++++++++++++++++++++++++++++++++++++++++ src/remote/remote_driver.c | 93 ++++++++++++++++++++++++++++++++ src/remote/remote_protocol.x | 32 ++++++++++- src/remote_protocol-structs | 24 +++++++++ 4 files changed, 270 insertions(+), 2 deletions(-)

diff --git a/daemon/remote.c b/daemon/remote.c index 03d5557..4c3bb4f 100644 --- a/daemon/remote.c +++ b/daemon/remote.c @@ -5025,6 +5025,129 @@ cleanup: return rv; }

+static int +remoteSerializeDomainInterfacePtr(virDomainInterfacePtr ifaces, + unsigned int ifaces_count, + remote_domain_interfaces_addresses_ret *ret) +{ + size_t i, j; + + if (VIR_ALLOC_N(ret->ifaces.ifaces_val, ifaces_count) < 0) + return -1; + + ret->ifaces.ifaces_len = ifaces_count; + + for (i = 0; i < ifaces_count; i++) { + virDomainInterfacePtr iface = &(ifaces[i]); + remote_domain_interface *iface_ret = &(ret->ifaces.ifaces_val[i]); + + if ((VIR_STRDUP(iface_ret->name, iface->name)) < 0) + goto no_memory; + + if (iface->hwaddr) { + char **hwaddr_p = NULL; + if (VIR_ALLOC(hwaddr_p) < 0) + goto no_memory; + if (VIR_STRDUP(*hwaddr_p, iface->hwaddr) < 0) + goto no_memory;

memory leak here, if VIR_STRDUP fails, hwaddr_p is leaked.

+ + iface_ret->hwaddr = hwaddr_p; + } + + if (VIR_ALLOC_N(iface_ret->ip_addrs.ip_addrs_val, + iface->ip_addrs_count) < 0) + goto no_memory; + + iface_ret->ip_addrs.ip_addrs_len = iface->ip_addrs_count; + + for (j = 0; j < iface->ip_addrs_count; j++) { + virDomainIPAddressPtr ip_addr = &(iface->ip_addrs[j]); + remote_domain_ip_addr *ip_addr_ret = + &(iface_ret->ip_addrs.ip_addrs_val[j]); + + if (VIR_STRDUP(ip_addr_ret->addr, ip_addr->addr) < 0) + goto no_memory; + + ip_addr_ret->prefix = ip_addr->prefix; + ip_addr_ret->type = ip_addr->type; + } + } + + return 0; + +no_memory: + if (ret->ifaces.ifaces_val) { + for (i = 0; i < ifaces_count; i++) { + remote_domain_interface *iface_ret = &(ret->ifaces.ifaces_val[i]); + VIR_FREE(iface_ret->name); + VIR_FREE(iface_ret->hwaddr); + for (j = 0; j < iface_ret->ip_addrs.ip_addrs_len; j++) { + remote_domain_ip_addr *ip_addr = + &(iface_ret->ip_addrs.ip_addrs_val[j]); + VIR_FREE(ip_addr->addr); + } + VIR_FREE(iface_ret); + } + VIR_FREE(ret->ifaces.ifaces_val); + } + + return -1; +} + +static int +remoteDispatchDomainInterfacesAddresses( + virNetServerPtr server ATTRIBUTE_UNUSED, + virNetServerClientPtr client, + virNetMessagePtr msg ATTRIBUTE_UNUSED, + virNetMessageErrorPtr rerr, + remote_domain_interfaces_addresses_args *args, + remote_domain_interfaces_addresses_ret *ret) +{ + int rv = -1; + virDomainPtr dom = NULL; + virDomainInterfacePtr ifaces = NULL; + unsigned int ifaces_count = 0; + struct daemonClientPrivate *priv = + virNetServerClientGetPrivateData(client); + + if (!priv->conn) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not open")); + goto cleanup; + } + + if (!(dom = get_nonnull_domain(priv->conn, args->dom))) + goto cleanup; + + if (virDomainInterfacesAddresses(dom, &ifaces, + &ifaces_count, args->flags) < 0) + goto cleanup; + + if (remoteSerializeDomainInterfacePtr(ifaces, ifaces_count, ret) < 0) + goto cleanup; + + rv = 0; + +cleanup: + if (rv < 0) + virNetMessageSaveError(rerr); + if (dom) + virDomainFree(dom); + if (ifaces) { + size_t i, j; + + for (i = 0; i < ifaces_count; i++) { + VIR_FREE(ifaces[i].name); + VIR_FREE(ifaces[i].hwaddr); + for (j = 0; j < ifaces[i].ip_addrs_count; j++) + VIR_FREE(ifaces[i].ip_addrs[j].addr); + VIR_FREE(ifaces[i].ip_addrs); + } + VIR_FREE(ifaces); + } + return rv; +} + +

/*----- Helpers. -----*/ diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index f828eef..18f18c7 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -6476,6 +6476,98 @@ done: return rv; }

+static int +remoteDomainInterfacesAddresses(virDomainPtr dom, + virDomainInterfacePtr *ifaces, + unsigned int *ifaces_count, + unsigned int flags) +{ + int rv = -1; + remote_domain_interfaces_addresses_args args; + remote_domain_interfaces_addresses_ret ret; + struct private_data *priv = dom->conn->privateData; + size_t i, j; + + memset(&ret, 0, sizeof(ret)); + args.flags = flags; + make_nonnull_domain(&args.dom, dom); + + remoteDriverLock(priv); + + if (call(dom->conn, priv, 0, REMOTE_PROC_DOMAIN_INTERFACES_ADDRESSES, + (xdrproc_t)xdr_remote_domain_interfaces_addresses_args, + (char *)&args, + (xdrproc_t)xdr_remote_domain_interfaces_addresses_ret, + (char *)&ret) == -1) { + goto done; + } + + if (ret.ifaces.ifaces_len && + VIR_ALLOC_N(*ifaces, ret.ifaces.ifaces_len) < 0) { + virReportOOMError();

you don't need virReportOOMError here.

+ goto cleanup; + } + + *ifaces_count = ret.ifaces.ifaces_len; + + for (i = 0; i < *ifaces_count; i++) { + virDomainInterfacePtr iface = &((*ifaces)[i]); + remote_domain_interface *iface_ret = &(ret.ifaces.ifaces_val[i]); + + if (VIR_STRDUP(iface->name, iface_ret->name) < 0) { + virReportOOMError();

Likewise.

+ goto cleanup; + } + + if (iface_ret->hwaddr && + VIR_STRDUP(iface->hwaddr, *iface_ret->hwaddr) < 0) { + virReportOOMError();

Likewise.

+ goto cleanup; + } + + iface->ip_addrs_count = iface_ret->ip_addrs.ip_addrs_len; + + if (iface->ip_addrs_count) { + if (VIR_ALLOC_N(iface->ip_addrs, iface->ip_addrs_count) < 0) { + virReportOOMError();

Likewise.

+ goto cleanup; + } + + for (j = 0; j < iface->ip_addrs_count; j++) { + virDomainIPAddressPtr ip_addr = &(iface->ip_addrs[j]); + remote_domain_ip_addr *ip_addr_ret = + &(iface_ret->ip_addrs.ip_addrs_val[j]); + + if (VIR_STRDUP(ip_addr->addr, ip_addr_ret->addr) < 0) { + virReportOOMError();

likewise.

+ goto cleanup; + } + ip_addr->prefix = ip_addr_ret->prefix; + ip_addr->type = ip_addr_ret->type; + } + } + } + + rv = 0; + +cleanup: + if (rv < 0) { + for (i = 0; i < *ifaces_count; i++) { + VIR_FREE((*ifaces)[i].name); + VIR_FREE((*ifaces)[i].hwaddr); + for (j = 0; j < (*ifaces)[i].ip_addrs_count; j++) + VIR_FREE((*ifaces)[i].ip_addrs[j].addr); + VIR_FREE((*ifaces)[i].ip_addrs); + } + VIR_FREE(*ifaces); + } + xdr_free((xdrproc_t)xdr_remote_domain_interfaces_addresses_ret, + (char *) &ret); +done: + remoteDriverUnlock(priv); + return rv; +} + static void remoteDomainEventQueue(struct private_data *priv, virDomainEventPtr event) { @@ -6810,6 +6902,7 @@ static virDriver remote_driver = { .domainMigratePerform3Params = remoteDomainMigratePerform3Params, /* 1.1.0 */ .domainMigrateFinish3Params = remoteDomainMigrateFinish3Params, /* 1.1.0 */ .domainMigrateConfirm3Params = remoteDomainMigrateConfirm3Params, /* 1.1.0 */ + .domainInterfacesAddresses = remoteDomainInterfacesAddresses, /* 1.1.2 */ };

static virNetworkDriver network_driver = { diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x index 7cfebdf..27f3634 100644 --- a/src/remote/remote_protocol.x +++ b/src/remote/remote_protocol.x @@ -1,5 +1,5 @@ /* -*- c -*- - * remote_protocol.x: private protocol for communicating between + * remote_protocoa.x: private protocol for communicating between

Mistake.

* remote_internal driver and libvirtd. This protocol is * internal and may change at any time. * @@ -2837,6 +2837,27 @@ struct remote_domain_event_device_removed_msg { remote_nonnull_string devAlias; };

+struct remote_domain_ip_addr { + int type; + remote_nonnull_string addr; + int prefix; +}; + +struct remote_domain_interface { + remote_nonnull_string name; + remote_string hwaddr; + remote_domain_ip_addr ip_addrs<>; +}; + +struct remote_domain_interfaces_addresses_args { + remote_nonnull_domain dom; + unsigned int flags; +}; + +struct remote_domain_interfaces_addresses_ret { + remote_domain_interface ifaces<>; +}; + /*----- Protocol. -----*/

/* Define the program number, protocol version and procedure numbers here. */ @@ -5000,5 +5021,12 @@ enum remote_procedure { * @generate: both * @acl: none */ - REMOTE_PROC_DOMAIN_EVENT_DEVICE_REMOVED = 311 + REMOTE_PROC_DOMAIN_EVENT_DEVICE_REMOVED = 311, + + /** + * @generate: none + * @acl: domain:read + */ + REMOTE_PROC_DOMAIN_INTERFACES_ADDRESSES = 312 + }; diff --git a/src/remote_protocol-structs b/src/remote_protocol-structs index 4e27aae..3919530 100644 --- a/src/remote_protocol-structs +++ b/src/remote_protocol-structs @@ -2316,6 +2316,29 @@ struct remote_domain_event_device_removed_msg { remote_nonnull_domain dom; remote_nonnull_string devAlias; }; +struct remote_domain_ip_addr { + int type; + remote_nonnull_string addr; + int prefix; +}; +struct remote_domain_interface { + remote_nonnull_string name; + remote_string hwaddr; + struct { + u_int ip_addrs_len; + remote_domain_ip_addr * ip_addrs_val; + } ip_addrs; +}; +struct remote_domain_interfaces_addresses_args { + remote_nonnull_domain dom; + u_int flags; +}; +struct remote_domain_interfaces_addresses_ret { + struct { + u_int ifaces_len; + remote_domain_interface * ifaces_val; + } ifaces; +}; enum remote_procedure { REMOTE_PROC_CONNECT_OPEN = 1, REMOTE_PROC_CONNECT_CLOSE = 2, @@ -2628,4 +2651,5 @@ enum remote_procedure { REMOTE_PROC_DOMAIN_CREATE_XML_WITH_FILES = 309, REMOTE_PROC_DOMAIN_CREATE_WITH_FILES = 310, REMOTE_PROC_DOMAIN_EVENT_DEVICE_REMOVED = 311, + REMOTE_PROC_DOMAIN_INTERFACES_ADDRESSES = 312, };

This needs a new version...

On 24/07/13 19:07, nehaljwani wrote:

By querying the qemu guest agent with the QMP command "guest-network-get-interfaces" and converting the received JSON output to structured objects.

src/qemu/qemu_agent.h: * Define qemuAgentGetInterfaces

src/qemu/qemu_agent.c: * Implement qemuAgentGetInterface

src/qemu/qemu_driver.c: * New function qemuDomainInterfacesAddresses

src/remote_protocol-sructs: * Define new structs

--- src/qemu/qemu_agent.c | 150 +++++++++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_agent.h | 4 ++ src/qemu/qemu_driver.c | 57 +++++++++++++++++++ 3 files changed, 211 insertions(+)

diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c index 72bf211..6889195 100644 --- a/src/qemu/qemu_agent.c +++ b/src/qemu/qemu_agent.c @@ -1323,6 +1323,156 @@ cleanup: return ret; }

+ +int +qemuAgentGetInterfaces(qemuAgentPtr mon, + virDomainInterfacePtr *ifaces, + unsigned int *ifaces_count) +{ + int ret = -1; + size_t i, j; + int size = -1; + virJSONValuePtr cmd; + virJSONValuePtr reply = NULL; + virJSONValuePtr ret_array = NULL; + + cmd = qemuAgentMakeCommand("guest-network-get-interfaces", NULL); + + if (!cmd) + return -1; + + if (qemuAgentCommand(mon, cmd, &reply, 1) < 0 || + qemuAgentCheckError(cmd, reply) < 0) + goto cleanup; + + if (!(ret_array = virJSONValueObjectGet(reply, "return"))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("qemu agent didn't provide 'return' field")); + goto cleanup; + } + + if ((size = virJSONValueArraySize(ret_array)) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("qemu agent didn't return an array of interfaces")); + goto cleanup; + } + + *ifaces_count = (unsigned int) size; + + if (VIR_ALLOC_N(*ifaces, size) < 0) + goto cleanup; + + 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) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("something has went really wrong")); + goto cleanup;

see [1], from here, it should be changed into: goto error;

+ } + + /* interface name is required to be presented */ + name = virJSONValueObjectGetString(tmp_iface, "name"); + if (!name) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("qemu agent didn't provide 'name' field")); + goto cleanup; + } + + if (VIR_STRDUP((*ifaces)[i].name, name) < 0) + goto cleanup; + + /* hwaddr might be omitted */ + hwaddr = virJSONValueObjectGetString(tmp_iface, "hardware-address"); + if (hwaddr && VIR_STRDUP((*ifaces)[i].hwaddr, hwaddr) < 0) + goto cleanup; + + /* as well as IP address which - moreover - + * can be presented multiple times */ + ip_addr_arr = virJSONValueObjectGet(tmp_iface, "ip-addresses"); + if (!ip_addr_arr) + continue; + + if ((ip_addr_arr_size = virJSONValueArraySize(ip_addr_arr)) < 0) + /* Mmm, empty 'ip-address'? */ + continue; + + (*ifaces)[i].ip_addrs_count = (unsigned int) ip_addr_arr_size; + + if (VIR_ALLOC_N((*ifaces)[i].ip_addrs, ip_addr_arr_size) < 0) + goto cleanup; + + for (j = 0; j < ip_addr_arr_size; j++) { + virJSONValuePtr ip_addr_obj = virJSONValueArrayGet(ip_addr_arr, j); + virDomainIPAddressPtr ip_addr = &(*ifaces)[i].ip_addrs[j]; + const char *type, *addr; + + /* Shouldn't happen but doesn't hurt to check neither */ + if (!ip_addr_obj) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("something has went really wrong")); + goto cleanup; + } + + type = virJSONValueObjectGetString(ip_addr_obj, "ip-address-type"); + if (!type) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("qemu agent didn't provide 'ip-address-type'" + " field for interface '%s'"), name); + goto cleanup; + } else if (STREQ(type, "ipv4")) { + ip_addr->type = VIR_IP_ADDR_TYPE_IPV4; + } else if (STREQ(type, "ipv6")) { + ip_addr->type = VIR_IP_ADDR_TYPE_IPV6; + } else { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("unknown ip address type '%s'"), + type); + goto cleanup; + } + + addr = virJSONValueObjectGetString(ip_addr_obj, "ip-address"); + if (!addr) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("qemu agent didn't provide 'ip-address'" + " field for interface '%s'"), name); + goto cleanup; + } + if (VIR_STRDUP(ip_addr->addr, addr) < 0) + goto cleanup; + + if (virJSONValueObjectGetNumberInt(ip_addr_obj, "prefix", + &ip_addr->prefix) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("malformed 'prefix' field")); + goto cleanup; + } + } + } + + ret = 0; + +cleanup: + if (ret < 0 && *ifaces && size> 0) { + for (i = 0; i < size; i++) { + VIR_FREE((*ifaces)[i].name); + VIR_FREE((*ifaces)[i].hwaddr); + for (j = 0; j < (*ifaces)[i].ip_addrs_count; j++) + VIR_FREE((*ifaces)[i].ip_addrs[j].addr); + VIR_FREE((*ifaces)[i].ip_addrs); + } + VIR_FREE(*ifaces); + }

[1] using two lables here will make things more clear, e.g. ret = 0; cleanup: virJSONValueFree(cmd); virJSONValueFree(reply); return ret; error: for (i = 0; i < *ifaces_count; i++) { /* free the stuffs here */ } goto cleanup; Note that I use "*ifaces_count" instead of "size", they have same value, but "*ifaces_count" makes more sense, it make the code more readable, as you are freeing the *ifaces. Also you don't need to do checking like the old code, *ifaces must be not NULL as long as the code go through to "error" label.

+ virJSONValueFree(cmd); + virJSONValueFree(reply); + return ret; +} + + /* * qemuAgentFSThaw: * @mon: Agent diff --git a/src/qemu/qemu_agent.h b/src/qemu/qemu_agent.h index cf70653..4afc028 100644 --- a/src/qemu/qemu_agent.h +++ b/src/qemu/qemu_agent.h @@ -76,6 +76,10 @@ int qemuAgentFSThaw(qemuAgentPtr mon); int qemuAgentSuspend(qemuAgentPtr mon, unsigned int target);

+int qemuAgentGetInterfaces(qemuAgentPtr mon, + virDomainInterfacePtr *ifaces, + unsigned int *ifaces_count); + int qemuAgentArbitraryCommand(qemuAgentPtr mon, const char *cmd, char **result, diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 96f87cd..1d204dc 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -16050,6 +16050,62 @@ qemuNodeSuspendForDuration(virConnectPtr conn, return nodeSuspendForDuration(target, duration, flags); }

+static int +qemuDomainInterfacesAddresses(virDomainPtr dom, + virDomainInterfacePtr *ifaces, + unsigned int *ifaces_count, + unsigned int flags) +{ + virQEMUDriverPtr driver = dom->conn->privateData; + qemuDomainObjPrivatePtr priv = NULL; + virDomainObjPtr vm = NULL; + int ret = -1; + + virCheckFlags(0, -1); + + if (!(vm = qemuDomObjFromDomain(dom))) + goto cleanup; + + if (!virDomainObjIsActive(vm)) { + virReportError(VIR_ERR_OPERATION_INVALID, "%s", + _("domain is not running")); + goto endjob; + } + + priv = vm->privateData; + + if (virDomainInterfacesAddressesEnsureACL(dom->conn, vm->def) < 0) + goto cleanup; + + if (priv->agentError) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("QEMU guest agent is not " + "available due to an error")); + goto cleanup; + } + + if (!priv->agent) { + virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s", + _("QEMU guest agent is not configured")); + goto cleanup; + } + + if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_QUERY) < 0) + goto cleanup; + + qemuDomainObjEnterAgent(vm); + ret = qemuAgentGetInterfaces(priv->agent, ifaces, ifaces_count); + qemuDomainObjExitAgent(vm); + +endjob: + if (qemuDomainObjEndJob(driver, vm) == 0) + vm = NULL;

"endjob" label is never used. simply removeing it is fine.

+ +cleanup: + if (vm) + virObjectUnlock(vm); + return ret; +}

static virDriver qemuDriver = { .no = VIR_DRV_QEMU, @@ -16145,6 +16201,7 @@ static virDriver qemuDriver = { .domainMemoryPeek = qemuDomainMemoryPeek, /* 0.4.4 */ .domainGetBlockInfo = qemuDomainGetBlockInfo, /* 0.8.1 */ .nodeGetCPUStats = qemuNodeGetCPUStats, /* 0.9.3 */ + .domainInterfacesAddresses = qemuDomainInterfacesAddresses, /* 1.1.2 */ .nodeGetMemoryStats = qemuNodeGetMemoryStats, /* 0.9.3 */ .nodeGetCellsFreeMemory = qemuNodeGetCellsFreeMemory, /* 0.4.4 */ .nodeGetFreeMemory = qemuNodeGetFreeMemory, /* 0.4.4 */

On 24/07/13 19:07, nehaljwani wrote:

Use virDomainInterfacesAddresses in virsh

tools/virsh-domain-monitor.c * Introduce new command : domifaddr Example Usage: domifaddr <domain-name> <interface-name (optional)>

As a habit, we indicate the optional optinon of command like: domifaddr <domain> [interface]

tools/virsh.pod * Document new command

--- tools/virsh-domain-monitor.c | 103 +++++++++++++++++++++++++++++++++++++++++++ tools/virsh.pod | 10 +++++ 2 files changed, 113 insertions(+)

diff --git a/tools/virsh-domain-monitor.c b/tools/virsh-domain-monitor.c index 773f96d..d85dfbd 100644 --- a/tools/virsh-domain-monitor.c +++ b/tools/virsh-domain-monitor.c @@ -1873,6 +1873,103 @@ cleanup: } #undef FILTER

+/* "domifaddr" command + */ +static const vshCmdInfo info_domifaddr[] = { + {"help", N_("Get network interfaces addresses for a running domain")},

not sure if it should be interfaces' instead of interfaces.

+ {"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")}, + {"interface", VSH_OT_DATA, VSH_OFLAG_NONE, N_("network interface name")}, + {NULL, 0, 0, NULL} +}; + +static bool +cmdDomIfAddr(vshControl *ctl, const vshCmd *cmd) +{ + virDomainPtr dom = NULL; + const char *interface = NULL; + virDomainInterfacePtr ifaces = NULL; + size_t i, j; + unsigned int ifaces_count = 0; + unsigned int flags = 0; + bool ret = false; + + if (!(dom = vshCommandOptDomain(ctl, cmd, NULL))) + return false; + [1]

+ if (vshCommandOptString(cmd, "interface", &interface) < 0) { + goto cleanup; + } + + if (virDomainInterfacesAddresses(dom, &ifaces, &ifaces_count, flags) < 0) { + vshError(ctl, _("Failed to query for interfaces addresses")); + goto cleanup; + } + + vshPrintExtra(ctl, " %-10s %-17s %s\n%s\n", + _("Name"), _("MAC address"), _("IP address"), + "---------------------------------------------------"); + + for (i = 0; i < ifaces_count; i++) { + virDomainInterfacePtr iface = &(ifaces[i]); + virBuffer buf = VIR_BUFFER_INITIALIZER; + const char *hwaddr = ""; + const char *ip_addr_str = NULL; + + if (interface && STRNEQ_NULLABLE(interface, iface->name)) {

My bad, I shouldn't tell you to use STRNEQ_NULLABLE here, because iface->name must be NOT NULL, and checking interface is NULL or not has logical meaning (wheter [interface] is specified, it makes code more readable). This sentense should be written as: if (interface && STRNEQ(interface, iface->name) continue;

+ virBufferFreeAndReset(&buf); + continue; + } + + if (iface->hwaddr) + hwaddr = iface->hwaddr; + + for (j = 0; j < iface->ip_addrs_count; j++) { + if (j) + virBufferAddChar(&buf, ' '); + virBufferAsprintf(&buf, "%s/%d", + iface->ip_addrs[j].addr, + iface->ip_addrs[j].prefix); + } + + if (virBufferError(&buf)) { + virBufferFreeAndReset(&buf); + virReportOOMError(); + return ret; + } + + ip_addr_str = virBufferContentAndReset(&buf); + + if (!ip_addr_str) + ip_addr_str = ""; + + vshPrintExtra(ctl, " %-10s %-17s %s\n", + iface->name, hwaddr, ip_addr_str); + + virBufferFreeAndReset(&buf); + } + + ret = true; + +cleanup: + for (i = 0; i < ifaces_count; i++) { + VIR_FREE(ifaces[i].name); + VIR_FREE(ifaces[i].hwaddr); + for (j = 0; j < ifaces[i].ip_addrs_count; j++) + VIR_FREE(ifaces[i].ip_addrs[j].addr); + VIR_FREE(ifaces[i].ip_addrs); + } + VIR_FREE(ifaces); + + if (dom) + virDomainFree(dom);

see [1], "dom" must be not NULL as long as it went to here. So no need for the checking.

+ return ret; +} + const vshCmdDef domMonitoringCmds[] = { {.name = "domblkerror", .handler = cmdDomBlkError, @@ -1946,5 +2043,11 @@ const vshCmdDef domMonitoringCmds[] = { .info = info_list, .flags = 0 }, + {.name = "domifaddr", + .handler = cmdDomIfAddr, + .opts = opts_domifaddr, + .info = info_domifaddr, + .flags = 0 + }, {.name = NULL} }; diff --git a/tools/virsh.pod b/tools/virsh.pod index 3ff6da1..bf13be4 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -633,6 +633,16 @@ B<Explanation of fields> (fields appear in the following order): flush_total_times - total time flush operations took (ns) <-- other fields provided by hypervisor -->

+ +=item B<domifaddr> I<domain> [I<interface-device>]

s/interface-device/interface/, since you are using "interface" instead of "interface-device" in the code. +static const vshCmdOptDef opts_domifaddr[] = { + {"domain", VSH_OT_DATA, VSH_OFLAG_REQ, N_("domain name, id or uuid")}, + {"interface", VSH_OT_DATA, VSH_OFLAG_NONE, N_("network interface name")}, + {NULL, 0, 0, NULL} Otherwise, the help document from "virsh help domifaddr" and in virsh.pod are different.

+ +Get a list of interfaces of a running domain along with their IP and MAC +addresses, or limited output just for one interface if I<interface-device>

Like above.

+is specified. Note, that I<interface-device> can be driver dependent, it can

Like above.

+be the 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. + =item B<domifstat> I<domain> I<interface-device>

Get network interface stats for a running domain.

On 24/07/13 19:07, nehaljwani wrote:

Expose virDomainInterfacesAddresses to python binding

examples/python/Makefile.am: * Add new file domipaddrs.py

examples/python/README: * Add documentation for the python example

python/libvirt-override-api.xml: * Add new symbol for virDomainInterfacesAddresses

python/libvirt-override.c: * Hand written python api

--- examples/python/Makefile.am | 2 +- examples/python/README | 1 + examples/python/domipaddrs.py | 50 +++++++++++++++++ python/libvirt-override-api.xml | 8 ++- python/libvirt-override.c | 117 ++++++++++++++++++++++++++++++++++++++++ 5 files changed, 176 insertions(+), 2 deletions(-) create mode 100755 examples/python/domipaddrs.py

diff --git a/examples/python/Makefile.am b/examples/python/Makefile.am index 2cacfa1..d33ee17 100644 --- a/examples/python/Makefile.am +++ b/examples/python/Makefile.am @@ -17,4 +17,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..1285d52 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 along with their MAC 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 100755 index 0000000..cc16c67 --- /dev/null +++ b/examples/python/domipaddrs.py @@ -0,0 +1,50 @@ +#!/usr/bin/env python +# domipaddrds - print domain interfaces along with their MAC and IP addresses + +import libvirt +import sys + +def usage(): + print "Usage: %s [URI] DOMAIN" % sys.argv[0] + print " Print domain interfaces along with their MAC and IP addresses" + +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) + +print " {0:10} {1:17} {2}".format("Interface", "MAC address", "IP Address") + +for (name, val) in ifaces.iteritems(): + print " {0:10} {1:17}".format(name, val['hwaddr']), + + if (val['ip_addrs'] <> None): + print " ", + for addr in val['ip_addrs']: + print "{0}/{1} ".format(addr['addr'], addr['prefix']), + + print

Glad to see you added an example. Good habit.

diff --git a/python/libvirt-override-api.xml b/python/libvirt-override-api.xml index 9a88215..1fcfa82 100644 --- a/python/libvirt-override-api.xml +++ b/python/libvirt-override-api.xml @@ -602,5 +602,11 @@ <arg name='conn' type='virConnectPtr' info='pointer to the hypervisor connection'/> <arg name='flags' type='int' info='unused, pass 0'/> </function> - </symbols> + <function name='virDomainInterfacesAddresses' file='python'> + <info>returns an array of domain interfaces along with their MAC and IP addresses</info> + <arg name='dom' type='virDomainPtr' info='pointer to the domain'/> + <arg name='flags' type='unsigned int' info='extra flags; not used yet, so callers should always pass 0'/> + <return type='virDomainInterfacePtr' info="dictionary of domain interfaces among with their MAC and IP addresses"/>

hm, regardless of it's an array or dictionary, it should be consistent. Though I think it's a dictionary.

+ </function> +</symbols> </api> diff --git a/python/libvirt-override.c b/python/libvirt-override.c index d16b9a2..11f3570 100644 --- a/python/libvirt-override.c +++ b/python/libvirt-override.c @@ -4761,6 +4761,122 @@ cleanup: return py_retval; }

+ +static PyObject * +libvirt_virDomainInterfacesAddresses(PyObject *self ATTRIBUTE_UNUSED, + PyObject *args) +{ + PyObject *py_retval = VIR_PY_NONE; + virDomainPtr domain; + PyObject *pyobj_domain; + unsigned int flags; + virDomainInterfacePtr ifaces = NULL; + unsigned int ifaces_count = 0; + size_t i, j; + int ret; + bool full_free = true; + + if (!PyArg_ParseTuple(args, (char *) "Oi:virDomainInterfacePtr", + &pyobj_domain, &flags)) + return NULL; + + domain = (virDomainPtr) PyvirDomain_Get(pyobj_domain); + + LIBVIRT_BEGIN_ALLOW_THREADS; + ret = virDomainInterfacesAddresses(domain, &ifaces, &ifaces_count, flags); + LIBVIRT_END_ALLOW_THREADS; + if (ret < 0) + goto cleanup; + + if (!(py_retval = PyDict_New())) + goto no_memory; + + for (i = 0; i < ifaces_count; i++) { + virDomainInterfacePtr iface = &(ifaces[i]); + PyObject *py_ip_addrs = NULL; + PyObject *py_iface = NULL; + + if (!(py_iface = PyDict_New())) + goto no_memory; + + if (iface->ip_addrs_count) { + if (!(py_ip_addrs = PyList_New(iface->ip_addrs_count))) { + Py_DECREF(py_iface); + goto no_memory; + } + } else { + py_ip_addrs = VIR_PY_NONE; + } + + for (j = 0; j < iface->ip_addrs_count; j++) { + virDomainIPAddressPtr ip_addr = &(iface->ip_addrs[j]); + PyObject *py_addr = PyDict_New(); + const char *type; + + if (!py_addr) { + Py_DECREF(py_iface); + Py_DECREF(py_ip_addrs); + goto no_memory; + } + + switch (ip_addr->type) { + case VIR_IP_ADDR_TYPE_IPV4: + type = "ipv4"; + break; + case VIR_IP_ADDR_TYPE_IPV6: + type = "ipv6"; + break; + default: + type = "unknown"; + break; + } + + PyDict_SetItem(py_addr, libvirt_constcharPtrWrap("addr"), + PyString_FromString(ip_addr->addr)); + PyDict_SetItem(py_addr, libvirt_constcharPtrWrap("prefix"), + libvirt_intWrap(ip_addr->prefix)); + PyDict_SetItem(py_addr, libvirt_constcharPtrWrap("type"), + PyString_FromString(type)); + + PyList_SetItem(py_ip_addrs, j, py_addr); + } + + PyDict_SetItem(py_iface, libvirt_constcharPtrWrap("ip_addrs"), + py_ip_addrs); + PyDict_SetItem(py_iface, libvirt_constcharPtrWrap("hwaddr"), + libvirt_charPtrWrap(iface->hwaddr)); + + PyDict_SetItem(py_retval, libvirt_charPtrWrap(iface->name), + py_iface); + } + + full_free = false; + +cleanup: + for (i = 0; i < ifaces_count; i++) { + /* + * We don't want to free values we've just shared with python variables unless + * there was an error and hence we are returning PY_NONE or equivalent + */ + if (full_free) { + VIR_FREE(ifaces[i].name); + VIR_FREE(ifaces[i].hwaddr); + for (j = 0; j < ifaces[i].ip_addrs_count; j++) + VIR_FREE(ifaces[i].ip_addrs[j].addr); + } + VIR_FREE(ifaces[i].ip_addrs); + } + VIR_FREE(ifaces); + + return py_retval; + +no_memory: + Py_XDECREF(py_retval); + py_retval = PyErr_NoMemory(); + goto cleanup; +} + + /******************************************* * Helper functions to avoid importing modules * for every callback @@ -7284,6 +7400,7 @@ static PyMethodDef libvirtMethods[] = { {(char *) "virDomainBlockPeek", libvirt_virDomainBlockPeek, METH_VARARGS, NULL}, {(char *) "virDomainMemoryPeek", libvirt_virDomainMemoryPeek, METH_VARARGS, NULL}, {(char *) "virDomainGetDiskErrors", libvirt_virDomainGetDiskErrors, METH_VARARGS, NULL}, + {(char *) "virDomainInterfacesAddresses", libvirt_virDomainInterfacesAddresses, METH_VARARGS, NULL}, {(char *) "virNodeGetMemoryParameters", libvirt_virNodeGetMemoryParameters, METH_VARARGS, NULL}, {(char *) "virNodeSetMemoryParameters", libvirt_virNodeSetMemoryParameters, METH_VARARGS, NULL}, {(char *) "virNodeGetCPUMap", libvirt_virNodeGetCPUMap, METH_VARARGS, NULL},

Others look fine to me.