Re: [libvirt] [PATCH v10 3/4] domifaddr: Implement the API for qemu

On 03/11/2015 07:09 AM, Daniel P. Berrange wrote: > From: Nehal J Wani <nehaljw.kkd1(a)gmail.com&gt; > > By querying the qemu guest agent with the QMP command > "guest-network-get-interfaces" and converting the received JSON > output to structured objects. > > Although "ifconfig" is deprecated, IP aliases created by "ifconfig" > are supported by this API. The legacy syntax of an IP alias is: > "<ifname>:<alias-name>". Since we want all aliases to be clubbed > under parent interface, simply stripping ":<alias-name>" suffices. > Note that IP aliases formed by "ip" aren't visible to "ifconfig", > and aliases created by "ip" do not have any specific name. But > we are lucky, as qemu guest agent detects aliases created by both. > > src/qemu/qemu_agent.h: > * Define qemuAgentGetInterfaces > > src/qemu/qemu_agent.c: > * Implement qemuAgentGetInterface > > src/qemu/qemu_driver.c: > * New function qemuGetDHCPInterfaces > * New function qemuDomainInterfaceAddresses > > src/remote_protocol-sructs: > * Define new structs > > tests/qemuagenttest.c: > * Add new test: testQemuAgentGetInterfaces > Test cases for IP aliases, 0 or multiple ipv4/ipv6 address(es) > > Signed-off-by: Nehal J Wani <nehaljw.kkd1(a)gmail.com&gt; > --- > src/qemu/qemu_agent.c | 204 +++++++++++++++++++++++++++++++++++++++++++++++++ > src/qemu/qemu_agent.h | 4 + > src/qemu/qemu_driver.c | 175 ++++++++++++++++++++++++++++++++++++++++++ > tests/qemuagenttest.c | 188 +++++++++++++++++++++++++++++++++++++++++++++ > 4 files changed, 571 insertions(+) > ACK - there's a NIT and a comment that follows, I'll let you decide how to handle. John > diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c > index 5fcc40f..8155006 100644 > --- a/src/qemu/qemu_agent.c > +++ b/src/qemu/qemu_agent.c > @@ -1953,3 +1953,207 @@ qemuAgentGetFSInfo(qemuAgentPtr mon, virDomainFSInfoPtr **info, > virJSONValueFree(reply); > return ret; > } > + > +/* > + * qemuAgentGetInterfaces: > + * @mon: Agent monitor > + * @ifaces: pointer to an array of pointers pointing to interface objects > + * > + * Issue guest-network-get-interfaces to guest agent, which returns a > + * list of interfaces of a running domain along with their IP and MAC > + * addresses. > + * > + * Returns: number of interfaces on success, -1 on error. > + */ > +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; > + size_t ifaces_count = 0; > + size_t addrs_count = 0; > + virDomainInterfacePtr *ifaces_ret = NULL; > + virHashTablePtr ifaces_store = NULL; > + char **ifname = NULL; > + > + /* Hash table to handle the interface alias */ > + if (!(ifaces_store = virHashCreate(ifaces_count, NULL))) { > + virHashFree(ifaces_store); > + return -1; > + } > + > + if (!(cmd = qemuAgentMakeCommand("guest-network-get-interfaces", NULL))) > + goto cleanup; > + > + if (qemuAgentCommand(mon, cmd, &reply, false, 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; > + } > + > + for (i = 0; i < size; i++) { > + virJSONValuePtr tmp_iface = virJSONValueArrayGet(ret_array, i); > + virJSONValuePtr ip_addr_arr = NULL; > + const char *hwaddr, *ifname_s, *name = NULL; > + int ip_addr_arr_size; > + virDomainInterfacePtr iface = NULL; > + > + /* 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; > + } > + > + /* Handle interface alias (<ifname>:<alias>) */ > + ifname = virStringSplit(name, ":", 2); > + ifname_s = ifname[0]; > + > + iface = virHashLookup(ifaces_store, ifname_s); > + > + /* If the hash table doesn't contain this iface, add it */ > + if (!iface) { > + if (VIR_EXPAND_N(ifaces_ret, ifaces_count, 1) < 0) > + goto error; > + > + if (VIR_ALLOC(ifaces_ret[ifaces_count - 1]) < 0) > + goto error; > + > + if (virHashAddEntry(ifaces_store, ifname_s, > + ifaces_ret[ifaces_count - 1]) < 0) > + goto error; > + > + iface = ifaces_ret[ifaces_count - 1]; > + iface->naddrs = 0; > + > + if (VIR_STRDUP(iface->name, ifname_s) < 0) > + goto error; > + > + hwaddr = virJSONValueObjectGetString(tmp_iface, "hardware-address"); > + if (!hwaddr) { > + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", > + _("qemu agent didn't provide" > + " 'hardware-address' field")); > + goto error; > + } > + > + if (VIR_STRDUP(iface->hwaddr, hwaddr) < 0) > + goto error; > + } > + > + /* Has to be freed for each interface. */ > + virStringFreeList(ifname); > + > + /* 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; > + NIT: Extra line here > + > + if ((ip_addr_arr_size = virJSONValueArraySize(ip_addr_arr)) < 0) > + /* Mmm, empty 'ip-address'? */ > + goto error; > + > + /* If current iface already exists, continue with the count */ > + addrs_count = iface->naddrs; > + > + for (j = 0; j < ip_addr_arr_size; j++) { > + const char *type, *addr; > + virJSONValuePtr ip_addr_obj = virJSONValueArrayGet(ip_addr_arr, j); > + virDomainIPAddressPtr ip_addr; > + > + if (VIR_EXPAND_N(iface->addrs, addrs_count, 1) < 0) > + goto error; > + > + ip_addr = &iface->addrs[addrs_count - 1]; > + > + /* 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")); Cannot remember if this text caught my attention the first time, but it is humorous...