[libvirt] [PATCH v2 3/8] qemu_agent: Implement 'guest-network-get-interfaces' command handling

[Ján Tomko]

On 01/03/13 14:46, Michal Privoznik wrote:
> This command returns an array of all guest interfaces among
> with their IP and HW addresses.
> ---
>  src/qemu/qemu_agent.c | 171 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  src/qemu/qemu_agent.h |   2 +
>  2 files changed, 173 insertions(+)
> 
> diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c
> index bb421bd..409ba04 100644
> --- a/src/qemu/qemu_agent.c
> +++ b/src/qemu/qemu_agent.c
> @@ -1474,3 +1474,174 @@ qemuAgentFSTrim(qemuAgentPtr mon,
>      virJSONValueFree(reply);
>      return ret;
>  }
> +
> +static int
> +getInterfaces(virJSONValuePtr reply,
> +              virDomainInterfacePtr **ifaces)
> +{
> +    int ret = -1;
> +    int i, size = 0;
> +    virJSONValuePtr replyArray = NULL;
> +
> +    if (!(replyArray = virJSONValueObjectGet(reply, "return"))) {
> +        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                       _("qemu agent did not provide 'return' object"));
> +        goto cleanup;
> +    }
> +
> +    if ((size = virJSONValueArraySize(replyArray)) < 0) {
> +        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                       _("qemu agent did not provide any interface"));
> +        goto cleanup;
> +    }
> +
> +    if (size && VIR_ALLOC_N(*ifaces, size * sizeof(virDomainInterfacePtr)) < 0) {
> +        virReportOOMError();

size is > 0 now and *ifaces = NULL, if you jump to cleanup you'll
dereference it. I'd suggest just returning -1 here and above.

> +        goto cleanup;
> +    }
> +
> +    for (i = 0; i < size; i++) {
> +        virJSONValuePtr jsonIface = virJSONValueArrayGet(replyArray, i);
> +        virDomainInterfacePtr tmpIface = NULL;
> +        virJSONValuePtr jsonIpAddrArr = NULL;
> +        int j, jsonIpAddrArrSize = 0;
> +        const char *name, *hwaddr;
> +
> +        if (VIR_ALLOC(tmpIface) < 0) {
> +            virReportOOMError();
> +            goto cleanup;
> +        }
> +
> +        (*ifaces)[i] = tmpIface;
> +        /* should not happen, but doesn't hurt to check */
> +        if (!jsonIface) {
> +            virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                           _("Something went really wrong while processing "
> +                             "guest agent reply"));
> +            goto cleanup;
> +        }
> +
> +        name = virJSONValueObjectGetString(jsonIface, "name");
> +        if (!name) {
> +            virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                           _("qemu agent did not provide 'name' object"));
> +            goto cleanup;
> +        }
> +
> +        if (!(tmpIface->name = strdup(name))) {
> +            virReportOOMError();
> +            goto cleanup;
> +        }
> +
> +        /* hwaddr might be omitted */
> +        hwaddr = virJSONValueObjectGetString(jsonIface, "hardware-address");
> +        if (hwaddr && !(tmpIface->hwaddr = strdup(hwaddr))) {
> +            virReportOOMError();
> +            goto cleanup;
> +        }
> +
> +        /* as well as ip-addresses */
> +        jsonIpAddrArr = virJSONValueObjectGet(jsonIface, "ip-addresses");
> +        if (!jsonIpAddrArr)
> +            continue;
> +
> +        if ((jsonIpAddrArrSize = virJSONValueArraySize(jsonIpAddrArr)) < 0) {
> +            virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                           _("qemu agent provided malformed "
> +                             "ip-addresses field"));
> +            goto cleanup;
> +        }
> +
> +        if (VIR_ALLOC_N(tmpIface->ip_addrs, jsonIpAddrArrSize) < 0) {
> +            virReportOOMError();
> +            goto cleanup;
> +        }
> +        tmpIface->ip_addrs_count = jsonIpAddrArrSize;
> +
> +        for (j = 0; j <  jsonIpAddrArrSize; j++) {

extra space

> +            virJSONValuePtr jsonIpAddr = virJSONValueArrayGet(jsonIpAddrArr, j);
> +            virDomainIPAddressPtr tmpIpAddr = &(tmpIface->ip_addrs[j]);
> +            const char *ipAddr, *ipAddrType;
> +
> +            if (!(ipAddr = virJSONValueObjectGetString(jsonIpAddr,
> +                                                       "ip-address"))) {
> +                virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                               _("qemu-agent didn't provided "
> +                                 "an ip-address field"));

didn't provide

> +                goto cleanup;
> +            }
> +
> +            if (!(tmpIpAddr->addr = strdup(ipAddr))) {
> +                virReportOOMError();
> +                goto cleanup;
> +            }
> +
> +            if (!(ipAddrType = virJSONValueObjectGetString(jsonIpAddr,
> +                                                           "ip-address-type"))) {
> +                virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                               _("qemu-agent didn't provided "
> +                                 "an ip-address-type field"));

here too

> +                goto cleanup;
> +            }
> +
> +            if (STREQ(ipAddrType, "ipv4"))
> +                tmpIpAddr->type = VIR_IP_ADDR_TYPE_IPV4;
> +            else if (STREQ(ipAddrType, "ipv6"))
> +                tmpIpAddr->type = VIR_IP_ADDR_TYPE_IPV6;
> +            else {
> +                virReportError(VIR_ERR_INTERNAL_ERROR,
> +                               _("qemu agent provided unknown "
> +                                 "ip-address-type '%s'"),
> +                               ipAddrType);
> +                goto cleanup;
> +            }
> +
> +            if (virJSONValueObjectGetNumberInt(jsonIpAddr, "prefix",
> +                                               &(tmpIpAddr->prefix)) < 0) {
> +                virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                               _("qemu agent provided "
> +                                 "malformed prefix field"));
> +                goto cleanup;
> +            }
> +
> +            /* Nor broadcast address is reported */
> +        }
> +    }
> +
> +    ret = size;
> +
> +cleanup:
> +    if (ret < 0) {
> +        for (i = 0; i < size; i++)
> +            virDomainInterfaceFree((*ifaces)[i]);

(*ifaces)[i] might be NULL here, but virDomainInterfaceFree doesn't
check for that.

> +        VIR_FREE(*ifaces);
> +    }
> +    return ret;
> +}
> +