On 23/08/13 06:18, nehaljwani wrote:

Define a 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

You introduced virDomainInterfaceFree in the new series, the commit log should be changed to be consistent.

--- include/libvirt/libvirt.h.in | 33 +++++++++++++ python/generator.py | 3 ++ src/driver.h | 6 +++ src/libvirt.c | 110 +++++++++++++++++++++++++++++++++++++++++++ src/libvirt_public.syms | 6 +++ 5 files changed, 158 insertions(+)

diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index a47e33c..deb1e1f 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -2044,6 +2044,39 @@ 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 naddrs; /* number of items in @addrs */ + virDomainIPAddressPtr addrs; /* array of IP addresses */ +}; + +int virDomainInterfacesAddresses (virDomainPtr dom, + virDomainInterfacePtr **ifaces, + unsigned int flags); + +void virDomainInterfaceFree (virDomainInterfacePtr iface); + /* Management of domain block devices */

int virDomainBlockPeek (virDomainPtr dom, diff --git a/python/generator.py b/python/generator.py index 427cebc..4f3c8ee 100755 --- a/python/generator.py +++ b/python/generator.py @@ -458,6 +458,7 @@ skip_impl = ( 'virNodeGetMemoryParameters', 'virNodeSetMemoryParameters', 'virNodeGetCPUMap', + 'virDomainInterfacesAddresses', 'virDomainMigrate3', 'virDomainMigrateToURI3', ) @@ -560,6 +561,8 @@ skip_function = ( "virTypedParamsGetString", "virTypedParamsGetUInt", "virTypedParamsGetULLong", + + "virDomainInterfaceFree", #Only for convenience in C

Consider changing the comment into "# Only useful in C".

)

lxc_skip_function = ( diff --git a/src/driver.h b/src/driver.h index be64333..8b6182e 100644 --- a/src/driver.h +++ b/src/driver.h @@ -518,6 +518,11 @@ typedef int unsigned int flags);

typedef int +(*virDrvDomainInterfacesAddresses)(virDomainPtr dom, + virDomainInterfacePtr **ifaces, + unsigned int flags); + +typedef int (*virDrvDomainMemoryStats)(virDomainPtr domain, struct _virDomainMemoryStat *stats, unsigned int nr_stats, @@ -1238,6 +1243,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 07a3fd5..2d4b0bf 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -8643,6 +8643,97 @@ error: return -1; }

+ /** + * virDomainInterfacesAddresses: + * @dom: domain object + * @ifaces: array of @dom interfaces

Not correct. Note that you changed the API interface in this series.

+ * @flags: extra flags; not used yet, so callers should always pass 0 + * + * Return an array of interfaces present in given domain along with

Like above, this should be changed too.

+ * 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.

In this new implementation, there is now @ifaces_count, the count of the interfaces is returned as the return value.

+ * + * 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; + * int ifaces_count = 0; + * size_t i, j; + * virDomainPtr dom = ... obtain a domain here ...; + * + * if ((ifaces_count = virDomainInterfacesAddresses(dom, &ifaces, 0)) < 0)

The argument type is "virDomainInterfacePtr **" now. So you have an error here.

+ * goto cleanup; + * + * ... do something with returned values, for example: + * for (i = 0; i < ifaces_count; i++) { + * printf("name: %s", ifaces[i].name);

And you will want ifaces[i]->name instead. Similiar for other memebers accessing.

+ * if (ifaces[i].hwaddr) + * printf(" hwaddr: %s", ifaces[i].hwaddr); + * + * for (j = 0; j < ifaces[i].naddrs; j++) { + * virDomainIPAddressPtr ip_addr = ifaces[i].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++) { + * if(ifaces[i]) + * virDomainInterfaceFree(ifaces[i]);

Indention problem.

+ * } + * free(ifaces); + * + * Returns 0 on success, -1 in case of error. + */ +int +virDomainInterfacesAddresses(virDomainPtr dom, + virDomainInterfacePtr **ifaces, + unsigned int flags) +{ + virConnectPtr conn; + + VIR_DOMAIN_DEBUG(dom, "ifaces=%p, flags=%x", ifaces, flags); + + virResetLastError(); + + if (!VIR_IS_CONNECTED_DOMAIN(dom)) { + virLibDomainError(VIR_ERR_INVALID_DOMAIN, __FUNCTION__); + goto error; + } + + conn = dom->conn; + + virCheckNonNullArgGoto(ifaces, error); + + if (conn->driver->domainInterfacesAddresses) { + int ret; + ret = conn->driver->domainInterfacesAddresses(dom, ifaces, 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 @@ -21961,3 +22052,22 @@ error: virDispatchError(dom->conn); return -1; } + +/** + * virDomainInterfaceFree: + * @iface: an interface object + * + * Free the interface object. The data structure is + * freed and should not be used thereafter. + * + */ +void +virDomainInterfaceFree(virDomainInterfacePtr iface) +{ + size_t i; + VIR_FREE((*iface).name);

Why not "iface->name"? also you need to check if "iface" is NULL or not, otherwise it could cause the crash, unless the user checking it himself before calling virDomainInterfaceFree, but that says you introduce a bad api then.

+ VIR_FREE((*iface).hwaddr); + for (i = 0; i < (*iface).naddrs; i++) + VIR_FREE((*iface).addrs[i].addr); + VIR_FREE((*iface).addrs); +} diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms index bbdf78a..df1c166 100644 --- a/src/libvirt_public.syms +++ b/src/libvirt_public.syms @@ -634,4 +634,10 @@ LIBVIRT_1.1.1 { virDomainSetMemoryStatsPeriod; } LIBVIRT_1.1.0;

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

On 23/08/13 06:18, 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

New tests are added in new series.

--- src/qemu/qemu_agent.c | 157 +++++++++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_agent.h | 3 + src/qemu/qemu_driver.c | 55 +++++++++++++++++ tests/qemuagenttest.c | 90 +++++++++++++++++++++++++++- 4 files changed, 304 insertions(+), 1 deletion(-)

diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c index 2cd0ccc..015e752 100644 --- a/src/qemu/qemu_agent.c +++ b/src/qemu/qemu_agent.c @@ -844,6 +844,7 @@ static int qemuAgentSend(qemuAgentPtr mon, VIR_DEBUG("Attempt to send command while error is set %s", NULLSTR(mon->lastError.message)); virSetError(&mon->lastError); + printf("Upper\n");

Self debug purpose? guess you forgot to remove it.

return -1; }

@@ -862,6 +863,7 @@ static int qemuAgentSend(qemuAgentPtr mon, while (!mon->msg->finished) { if ((then && virCondWaitUntil(&mon->notify, &mon->parent.lock, then) < 0) || (!then && virCondWait(&mon->notify, &mon->parent.lock) < 0)) { +

Meaningless addition.

if (errno == ETIMEDOUT) { virReportError(VIR_ERR_AGENT_UNRESPONSIVE, "%s", _("Guest agent not available for now")); @@ -1319,6 +1321,161 @@ cleanup: return ret; }

+ +int +qemuAgentGetInterfaces(qemuAgentPtr mon, + virDomainInterfacePtr **ifaces) +{ + int ret = -1; + size_t i, j; + int size = -1; + virJSONValuePtr cmd = NULL; + virJSONValuePtr reply = NULL; + virJSONValuePtr ret_array = NULL; + int ifaces_count = 0; + virDomainInterfacePtr *ifaces_ret = NULL; + + if (!(cmd = qemuAgentMakeCommand("guest-network-get-interfaces", NULL))) + return -1; + + if (qemuAgentCommand(mon, cmd, &reply, VIR_DOMAIN_QEMU_AGENT_COMMAND_BLOCK) < 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_ret, 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; + + if (VIR_ALLOC(ifaces_ret[i]) < 0) + goto cleanup; + + /* 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 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 error; + } + + if (VIR_STRDUP(ifaces_ret[i]->name, name) < 0) + goto error; + + /* hwaddr might be omitted */ + hwaddr = virJSONValueObjectGetString(tmp_iface, "hardware-address"); + if (hwaddr && VIR_STRDUP(ifaces_ret[i]->hwaddr, hwaddr) < 0) + goto error; + + /* 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_ret)[i]).naddrs = (unsigned int) ip_addr_arr_size; + + if (VIR_ALLOC_N((*(ifaces_ret)[i]).addrs, ip_addr_arr_size) < 0) + goto error; + + for (j = 0; j < ip_addr_arr_size; j++) { + virJSONValuePtr ip_addr_obj = virJSONValueArrayGet(ip_addr_arr, j); + virDomainIPAddressPtr ip_addr = &ifaces_ret[i]->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 error; + } + + 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 error; + } 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 error; + } + + 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 error; + } + if (VIR_STRDUP(ip_addr->addr, addr) < 0) + goto error; + + if (virJSONValueObjectGetNumberInt(ip_addr_obj, "prefix", + &ip_addr->prefix) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("malformed 'prefix' field")); + goto error; + } + } + } + + *ifaces = ifaces_ret; + ifaces_ret = NULL; + ret = ifaces_count; + +cleanup: + virJSONValueFree(cmd); + virJSONValueFree(reply); + return ret; + +error: + if (ifaces_ret) { + for (i = 0; i < ifaces_count; i++) { + if (ifaces_ret[i]) + virDomainInterfaceFree(ifaces_ret[i]); + } + } + VIR_FREE(ifaces_ret); + goto cleanup; +} + + /* * qemuAgentFSThaw: * @mon: Agent diff --git a/src/qemu/qemu_agent.h b/src/qemu/qemu_agent.h index 5fbacdb..58b56fd 100644 --- a/src/qemu/qemu_agent.h +++ b/src/qemu/qemu_agent.h @@ -76,6 +76,9 @@ int qemuAgentFSThaw(qemuAgentPtr mon); int qemuAgentSuspend(qemuAgentPtr mon, unsigned int target);

+int qemuAgentGetInterfaces(qemuAgentPtr mon, + virDomainInterfacePtr **ifaces); + int qemuAgentArbitraryCommand(qemuAgentPtr mon, const char *cmd, char **result, diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 0a8e518..0cd266f 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -16035,6 +16035,60 @@ qemuNodeSuspendForDuration(virConnectPtr conn, return nodeSuspendForDuration(target, duration, flags); }

+static int +qemuDomainInterfacesAddresses(virDomainPtr dom, + virDomainInterfacePtr **ifaces, + 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 cleanup; + } + + 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); + qemuDomainObjExitAgent(vm); + + if (qemuDomainObjEndJob(driver, vm) == 0) + vm = NULL; + +cleanup: + if (vm) + virObjectUnlock(vm); + return ret; +}

static virDriver qemuDriver = { .no = VIR_DRV_QEMU, @@ -16130,6 +16184,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 */ diff --git a/tests/qemuagenttest.c b/tests/qemuagenttest.c index 4e27981..6b6c47f 100644 --- a/tests/qemuagenttest.c +++ b/tests/qemuagenttest.c @@ -31,6 +31,8 @@

#define VIR_FROM_THIS VIR_FROM_NONE

+ +

Two many blank lines. Add 1 or don't add it.

static int testQemuAgentFSFreeze(const void *data) { @@ -271,6 +273,7 @@ cleanup: }

+ static int testQemuAgentShutdown(const void *data) { @@ -496,7 +499,7 @@ testQemuAgentArbitraryCommand(const void *data) goto cleanup;

if (qemuAgentArbitraryCommand(qemuMonitorTestGetAgent(test), - "{\"execute\":\"ble\"}", + "{\"execute\":\"ble\"}",

Indention.

&reply, VIR_DOMAIN_QEMU_AGENT_COMMAND_BLOCK) < 0) goto cleanup; @@ -576,6 +579,90 @@ cleanup: return ret; }

+static const char testQemuAgentGetInterfacesResponse[] = + "{\"return\": " + " [" + " {\"name\":\"lo\"," + " \"ip-addresses\":" + " [" + " {\"ip-address-type\":\"ipv4\"," + " \"ip-address\":\"127.0.0.1\"," + " \"prefix\":8" + " }," + " {\"ip-address-type\":\"ipv6\"," + " \"ip-address\":\"::1\"," + " \"prefix\":128" + " }" + " ]," + " \"hardware-address\":\"00:00:00:00:00:00\"" + " }," + " {\"name\":\"eth0\"," + " \"ip-addresses\":" + " [" + " {\"ip-address-type\":\"ipv4\"," + " \"ip-address\":\"192.168.102.142\"," + " \"prefix\":24" + " }," + " {\"ip-address-type\":\"ipv6\"," + " \"ip-address\":\"fe80::5054:ff:fe89:ad35\"," + " \"prefix\":64" + " }" + " ]," + " \"hardware-address\":\"52:54:00:89:ad:35\"" + " }," + " {\"name\":\"eth1\"," + " \"ip-addresses\":" + " [" + " {\"ip-address-type\":\"ipv4\"," + " \"ip-address\":\"192.168.103.83\"," + " \"prefix\":24" + " }," + " {\"ip-address-type\":\"ipv6\"," + " \"ip-address\":\"fe80::5054:ff:fed3:39ee\"," + " \"prefix\":64" + " }" + " ]," + " \"hardware-address\":\"52:54:00:d3:39:ee\"" + " }" + " ]" + "}"; + +static int +testQemuAgentGetInterfaces(const void *data) +{ + virDomainXMLOptionPtr xmlopt = (virDomainXMLOptionPtr)data; + qemuMonitorTestPtr test = qemuMonitorTestNewAgent(xmlopt); + int ret = -1; + int ifaces_count = 0; + virDomainInterfacePtr *ifaces; + + if (!test) + return -1; + + if (qemuMonitorTestAddAgentSyncResponse(test) < 0) + goto cleanup; + + if (qemuMonitorTestAddItem(test, "guest-network-get-interfaces", + testQemuAgentGetInterfacesResponse) < 0) + goto cleanup; + + if ((ifaces_count = qemuAgentGetInterfaces(qemuMonitorTestGetAgent(test), + &ifaces)) < 0) { + goto cleanup; + } + + if (ifaces_count != 3) { + virReportError(VIR_ERR_INTERNAL_ERROR, + "expected 3 interfaces, got %d", ret);

I think you want to test more than just checking the interface count, such as lo's MAC, eth1's ip address count, eth0's second ip address prefix, etc.

+ goto cleanup; + } + + ret = 0; + +cleanup: + qemuMonitorTestFree(test);

"ifaces" is leaked.

+ return ret; +}

static int mymain(void) @@ -605,6 +692,7 @@ mymain(void) DO_TEST(Shutdown); DO_TEST(CPU); DO_TEST(ArbitraryCommand); + DO_TEST(GetInterfaces);

DO_TEST(Timeout); /* Timeout should always be called last */

On 23/08/13 06:18, nehaljwani wrote:

Use virDomainInterfacesAddresses in virsh

tools/virsh-domain-monitor.c * Introduce new command : domifaddr

virsh # domifaddr f18 Name MAC address IP address --------------------------------------------------- lo 00:00:00:00:00:00 127.0.0.1/8 ::1/128 eth0 52:54:00:89:ad:35 192.168.102.142/24 fe80::5054:ff:fe89:ad35/64

Deserved to have two fields for IP address, one is IPv4, the other is IPv6,

eth1 52:54:00:d3:39:ee 192.168.103.183/24 fe80::5054:ff:fed3:39ee/64 eth2 52:54:00:fe:4c:4f 192.168.101.197/24 fe80::5054:ff:fefe:4c4f/64 eth3 52:54:00:89:4e:97 192.168.101.130/24 fe80::5054:ff:fe89:4e97/64

tools/virsh.pod * Document new command

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

diff --git a/tools/virsh-domain-monitor.c b/tools/virsh-domain-monitor.c index b29b82a..91efa71 100644 --- a/tools/virsh-domain-monitor.c +++ b/tools/virsh-domain-monitor.c @@ -1871,6 +1871,99 @@ cleanup: } #undef FILTER

+/* "domifaddr" command + */ +static const vshCmdInfo info_domifaddr[] = { + {"help", N_("Get network interfaces' addresses for a running 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")}, + {"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; + int ifaces_count = 0; + unsigned int flags = 0; + bool ret = false; + + if (!(dom = vshCommandOptDomain(ctl, cmd, NULL))) + return false; + + if (vshCommandOptString(cmd, "interface", &interface) < 0) { + goto cleanup; + } + + if ((ifaces_count = virDomainInterfacesAddresses(dom, &ifaces, 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(interface, iface->name)) { + virBufferFreeAndReset(&buf); + continue; + } + + if (iface->hwaddr) + hwaddr = iface->hwaddr; + + for (j = 0; j < iface->naddrs; j++) { + if (j) + virBufferAddChar(&buf, ' '); + virBufferAsprintf(&buf, "%s/%d", + iface->addrs[j].addr, + iface->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);

Indention. I think there is no changes on this patch, so just had a glance at the code. Osier

On Fri, Aug 23, 2013 at 8:16 PM, Doug Goldstein <cardoe@gentoo.org> wrote:

On Thu, Aug 22, 2013 at 5:18 PM, nehaljwani <nehaljw.kkd1@gmail.com>

wrote:

...

Use virDomainInterfacesAddresses in virsh

tools/virsh-domain-monitor.c * Introduce new command : domifaddr

virsh # domifaddr f18 Name MAC address IP address --------------------------------------------------- lo 00:00:00:00:00:00 127.0.0.1/8 ::1/128 eth0 52:54:00:89:ad:35 192.168.102.142/24fe80::5054:ff:fe89:ad35/64 eth1 52:54:00:d3:39:ee 192.168.103.183/24fe80::5054:ff:fed3:39ee/64 eth2 52:54:00:fe:4c:4f 192.168.101.197/24fe80::5054:ff:fefe:4c4f/64 eth3 52:54:00:89:4e:97 192.168.101.130/24fe80::5054:ff:fe89:4e97/64

Not a review, but a question. How does this work with IP aliases? Will it show all the aliases 1 per line or just show the primary address?

...

tools/virsh.pod * Document new command

--- tools/virsh-domain-monitor.c | 99

++++++++++++++++++++++++++++++++++++++++++++

...

tools/virsh.pod | 10 +++++ 2 files changed, 109 insertions(+)

diff --git a/tools/virsh-domain-monitor.c b/tools/virsh-domain-monitor.c index b29b82a..91efa71 100644 --- a/tools/virsh-domain-monitor.c +++ b/tools/virsh-domain-monitor.c @@ -1871,6 +1871,99 @@ cleanup: } #undef FILTER

+/* "domifaddr" command + */ +static const vshCmdInfo info_domifaddr[] = { + {"help", N_("Get network interfaces' addresses for a running 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")}, + {"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; + int ifaces_count = 0; + unsigned int flags = 0; + bool ret = false; + + if (!(dom = vshCommandOptDomain(ctl, cmd, NULL))) + return false; + + if (vshCommandOptString(cmd, "interface", &interface) < 0) { + goto cleanup; + } + + if ((ifaces_count = virDomainInterfacesAddresses(dom, &ifaces, 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(interface, iface->name)) { + virBufferFreeAndReset(&buf); + continue; + } + + if (iface->hwaddr) + hwaddr = iface->hwaddr; + + for (j = 0; j < iface->naddrs; j++) { + if (j) + virBufferAddChar(&buf, ' '); + virBufferAsprintf(&buf, "%s/%d", + iface->addrs[j].addr, + iface->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++) { + if (ifaces[i]) + virDomainInterfaceFree(ifaces[i]); + } + VIR_FREE(ifaces); + + virDomainFree(dom); + return ret; +} + const vshCmdDef domMonitoringCmds[] = { {.name = "domblkerror", .handler = cmdDomBlkError, @@ -1944,5 +2037,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 0ae5178..008ffea 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -636,6 +636,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>] + +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> is +specified. Note, that I<interface> can be driver dependent, it can 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. -- 1.7.11.7

-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list

-- Doug Goldstein

The leases method will be applied to this API when virNetworkGetDHCPLeases() API is accepted, which is the next step after this. Refer: http://www.redhat.com/archives/libvir-list/2013-July/msg01603.html and its followups. The output from the virNetworkGetDHCPLeases will be something of this kind: virsh # net-dhcp-leases default Virtual Network Expiry Time MAC address IP address Hostname ClientId ------------------------------------------------------------ ---------------------------------------- default 16-08-2013 03:53:11 52:54:00:89:4e:97 192.168.101.130 f18 * default 16-08-2013 03:45:20 52:54:00:fe:4c:4f 192.168.101.197 * * -- Nehal J Wani UG3, BTech CS+MS(CL) IIIT-Hyderabad http://commandlinewani.blogspot.com

On Sat, Aug 24, 2013 at 6:46 AM, Doug Goldstein <cardoe@gentoo.org> wrote:

On Fri, Aug 23, 2013 at 10:45 AM, Nehal J Wani <nehaljw.kkd1@gmail.com>

...

On Fri, Aug 23, 2013 at 8:16 PM, Doug Goldstein <cardoe@gentoo.org>

wrote:

...

...

On Thu, Aug 22, 2013 at 5:18 PM, nehaljwani <nehaljw.kkd1@gmail.com>

wrote:

...

...

Use virDomainInterfacesAddresses in virsh

tools/virsh-domain-monitor.c * Introduce new command : domifaddr

virsh # domifaddr f18 Name MAC address IP address --------------------------------------------------- lo 00:00:00:00:00:00 127.0.0.1/8 ::1/128 eth0 52:54:00:89:ad:35 192.168.102.142/24fe80::5054:ff:fe89:ad35/64 eth1 52:54:00:d3:39:ee 192.168.103.183/24fe80::5054:ff:fed3:39ee/64 eth2 52:54:00:fe:4c:4f 192.168.101.197/24fe80::5054:ff:fefe:4c4f/64 eth3 52:54:00:89:4e:97 192.168.101.130/24fe80::5054:ff:fe89:4e97/64

Not a review, but a question. How does this work with IP aliases? Will it show all the aliases 1 per line or just show the primary address?

...

tools/virsh.pod * Document new command

--- tools/virsh-domain-monitor.c | 99

++++++++++++++++++++++++++++++++++++++++++++

...

tools/virsh.pod | 10 +++++ 2 files changed, 109 insertions(+)

diff --git a/tools/virsh-domain-monitor.c b/tools/virsh-domain-monitor.c index b29b82a..91efa71 100644 --- a/tools/virsh-domain-monitor.c +++ b/tools/virsh-domain-monitor.c @@ -1871,6 +1871,99 @@ cleanup: } #undef FILTER

+/* "domifaddr" command + */ +static const vshCmdInfo info_domifaddr[] = { + {"help", N_("Get network interfaces' addresses for a running 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")}, + {"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; + int ifaces_count = 0; + unsigned int flags = 0; + bool ret = false; + + if (!(dom = vshCommandOptDomain(ctl, cmd, NULL))) + return false; + + if (vshCommandOptString(cmd, "interface", &interface) < 0) { + goto cleanup; + } + + if ((ifaces_count = virDomainInterfacesAddresses(dom, &ifaces, 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(interface, iface->name)) { + virBufferFreeAndReset(&buf); + continue; + } + + if (iface->hwaddr) + hwaddr = iface->hwaddr; + + for (j = 0; j < iface->naddrs; j++) { + if (j) + virBufferAddChar(&buf, ' '); + virBufferAsprintf(&buf, "%s/%d", + iface->addrs[j].addr, + iface->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++) { + if (ifaces[i]) + virDomainInterfaceFree(ifaces[i]); + } + VIR_FREE(ifaces); + + virDomainFree(dom); + return ret; +} + const vshCmdDef domMonitoringCmds[] = { {.name = "domblkerror", .handler = cmdDomBlkError, @@ -1944,5 +2037,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 0ae5178..008ffea 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -636,6 +636,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>] + +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> is +specified. Note, that I<interface> can be driver dependent, it can 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. -- 1.7.11.7

-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list

-- Doug Goldstein

The leases method will be applied to this API when virNetworkGetDHCPLeases() API is accepted, which is the next step after

wrote: this.

...

Refer: http://www.redhat.com/archives/libvir-list/2013-July/msg01603.htmland its followups.

The output from the virNetworkGetDHCPLeases will be something of this kind:

virsh # net-dhcp-leases default

Virtual Network Expiry Time MAC address IP address Hostname ClientId

...

default 16-08-2013 03:53:11 52:54:00:89:4e:97

192.168.101.130 f18 *

...

default 16-08-2013 03:45:20 52:54:00:fe:4c:4f

192.168.101.197 * *

...

My question was how does virDomainInterfacesAddresses() and virsh domifaddr handle aliased IP addresses where the same interface and same MAC address have multiple IP addresses.

Oh. I misread your question. In the case of aliases, it will be shown like this (one alias per line): virsh # domifaddr f18 Name MAC address IPv4 address IPv6 address --------------------------------------------------- lo 00:00:00:00:00:00 127.0.0.1/8 ::1/128 eth0 52:54:00:89:4e:97 192.168.101.130/24fe80::5054:ff:fe89:4e97/64 eth0:2 52:54:00:89:4e:97 192.168.101.132/24 eth0:1 52:54:00:89:4e:97 192.168.101.133/24 eth1 52:54:00:89:ad:35 192.168.102.142/24fe80::5054:ff:fe89:ad35/64 eth1:2 52:54:00:89:ad:35 192.168.102.143/24 eth2:0 52:54:00:d3:39:ee 192.168.103.184/24 eth2 52:54:00:d3:39:ee 192.168.103.183/24fe80::5054:ff:fed3:39ee/64 eth2:1 52:54:00:d3:39:ee 192.168.103.185/24 eth3 52:54:00:fe:4c:4f 192.168.101.197/24fe80::5054:ff:fefe:4c4f/64 eth3:1 52:54:00:fe:4c:4f 192.168.101.198/24

-- Doug Goldstein

-- Nehal J Wani UG3, BTech CS+MS(CL) IIIT-Hyderabad http://commandlinewani.blogspot.com

On 23/08/13 06:18, 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 | 114 ++++++++++++++++++++++++++++++++++++++++ 5 files changed, 173 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 diff --git a/python/libvirt-override-api.xml b/python/libvirt-override-api.xml index 9a88215..f5c6e83 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 a dictionary 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 along with their MAC and IP addresses"/> + </function> +</symbols> </api> diff --git a/python/libvirt-override.c b/python/libvirt-override.c index d16b9a2..50bb7d1 100644 --- a/python/libvirt-override.c +++ b/python/libvirt-override.c @@ -4761,6 +4761,119 @@ 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; + 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; + ifaces_count = virDomainInterfacesAddresses(domain, &ifaces, flags); + ret = ifaces_count; + 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->naddrs) { + if (!(py_ip_addrs = PyList_New(iface->naddrs))) { + Py_DECREF(py_iface); + goto no_memory; + } + } else { + py_ip_addrs = VIR_PY_NONE; + } + + for (j = 0; j < iface->naddrs; j++) { + virDomainIPAddressPtr ip_addr = &(iface->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 && ifaces[i]) { + virDomainInterfaceFree(ifaces[i]);

Remove the "{}"