[libvirt] [PATCH 3/5] domifaddr: Implement the API for qemu
Osier Yang
jyang at redhat.com
Wed Aug 14 07:56:41 UTC 2013
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 */
More information about the libvir-list
mailing list