[libvirt] [PATCHv4 3/5] domifaddr: Implement the API for qemu
Osier Yang
jyang at redhat.com
Tue Aug 27 09:33:35 UTC 2013
On 25/08/13 07:15, Nehal J Wani 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
>
> tests/qemuagenttest.c:
> * Add new test: testQemuAgentGetInterfaces
>
> ---
> src/qemu/qemu_agent.c | 153 +++++++++++++++++++++++++++++++++++++++++++++++++
> src/qemu/qemu_agent.h | 3 +
> src/qemu/qemu_driver.c | 55 ++++++++++++++++++
> tests/qemuagenttest.c | 113 ++++++++++++++++++++++++++++++++++++
> 4 files changed, 324 insertions(+)
>
> diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c
> index 2cd0ccc..11f5467 100644
> --- a/src/qemu/qemu_agent.c
> +++ b/src/qemu/qemu_agent.c
> @@ -1319,6 +1319,159 @@ 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++)
> + 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..71e7949 100644
> --- a/tests/qemuagenttest.c
> +++ b/tests/qemuagenttest.c
> @@ -271,6 +271,7 @@ cleanup:
> }
>
>
> +
Meaningless new blank line.
> static int
> testQemuAgentShutdown(const void *data)
> {
> @@ -576,6 +577,117 @@ 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;
> + }
No need for the {}.
[1]
> + if (STRNEQ(ifaces[0]->name, "lo") ||
> + STRNEQ(ifaces[0]->addrs[0].addr, "127.0.0.1") ||
> + ifaces[0]->addrs[1].prefix != 128) {
Better to check "naddrs" of ifaces[i] before indexing them, assuming
that ifaces[0] only has 1 "addrs" here. I'd like write the tests like:
> + virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> + "unexpected return values for interface: lo");
> + goto cleanup;
> + }
> +
> + if (STRNEQ(ifaces[1]->hwaddr, "52:54:00:89:ad:35") ||
> + ifaces[1]->addrs[0].prefix != 24 ||
> + ifaces[1]->addrs[1].type != VIR_IP_ADDR_TYPE_IPV6) {
> + virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> + "unexpected return values for interface: eth0");
> + goto cleanup;
> + }
> +
> + if (ifaces[2]->naddrs != 2 ||
> + ifaces[2]->addrs[0].type != VIR_IP_ADDR_TYPE_IPV4 ||
> + STRNEQ(ifaces[2]->addrs[1].addr, "fe80::5054:ff:fed3:39ee")) {
> + virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> + "unexpected return values for interface: eth1");
> + goto cleanup;
> + }
> +
> + if (ifaces_count != 3) {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + "expected 3 interfaces, got %d", ret);
> + goto cleanup;
> + }
> +
Not too necessary, since it's the test, but still better to move this
checking to
[1], assuming that qemuAgentGetInterfaces returns 0, it will lead to crash.
So I'd like write the tests like:
if (ifaces_count != 3) {
virReportError(...);
goto cleanup;
}
if (ifaces[0]->naddrs != 2 ||
ifaces[1]->naddrs != 2 ||
ifaces[2]->naddrs !=2) {
virReportError(...);
goto cleanup;
}
if (STRNEQ(ifaces[0]->name, "lo") ||
STRNEQ(ifaces[0]->addrs[0].addr, "127.0.0.1") ||
ifaces[0]->addrs[1].prefix != 128) {
virReportError(...);
goto cleanup;
}
if (STRNEQ(ifaces[1]->hwaddr, "52:54:00:89:ad:35") ||
ifaces[1]->addrs[0].prefix != 24 ||
ifaces[1]->addrs[1].type != VIR_IP_ADDR_TYPE_IPV6) {
virReportError(...);
goto cleanup;
}
if (ifaces[2]->addrs[0].type != VIR_IP_ADDR_TYPE_IPV4 ||
STRNEQ(ifaces[2]->addrs[1].addr, "fe80::5054:ff:fed3:39ee")) {
virReportError(...);
goto cleanup;
}
> + ret = 0;
> +
> +cleanup:
> + qemuMonitorTestFree(test);
> + size_t i;
Generally we do this in the beginning of function.
> + for (i = 0; i < ifaces_count; i++)
> + virDomainInterfaceFree(ifaces[i]);
> + VIR_FREE(ifaces);
> + return ret;
> +}
>
> static int
> mymain(void)
> @@ -605,6 +717,7 @@ mymain(void)
> DO_TEST(Shutdown);
> DO_TEST(CPU);
> DO_TEST(ArbitraryCommand);
> + DO_TEST(GetInterfaces);
>
> DO_TEST(Timeout); /* Timeout should always be called last */
>
More information about the libvir-list
mailing list