[libvirt] [PATCH v8 4/8] parallels: get info about serial ports

[Peter Krempa]

On 07/04/12 19:42, Dmitry Guryanov wrote:
> Add support of collecting information about serial
> ports. This change is needed mostly as an example,
> support of other devices will be added later.
>
> Signed-off-by: Dmitry Guryanov <dguryanov at parallels.com>
> ---
>   src/parallels/parallels_driver.c |  119 ++++++++++++++++++++++++++++++++++++++
>   1 files changed, 119 insertions(+), 0 deletions(-)
>
> diff --git a/src/parallels/parallels_driver.c b/src/parallels/parallels_driver.c
> index 8fadd79..270ac21 100644
> --- a/src/parallels/parallels_driver.c
> +++ b/src/parallels/parallels_driver.c
> @@ -143,6 +143,122 @@ parallelsGetCapabilities(virConnectPtr conn)
>       return xml;
>   }
>
> +static int
> +parallelsGetSerialInfo(virDomainChrDefPtr chr,
> +                 const char *name, virJSONValuePtr value)
> +{
> +    const char *tmp;
> +
> +    chr->deviceType = VIR_DOMAIN_CHR_DEVICE_TYPE_SERIAL;
> +    chr->targetType = VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SERIAL;
> +    if (virStrToLong_i(name + strlen("serial"),
> +                       NULL, 10, &chr->target.port) < 0) {
> +        parallelsParseError();
> +        return -1;
> +    }
> +
> +    if (virJSONValueObjectHasKey(value, "output")) {
> +        chr->source.type = VIR_DOMAIN_CHR_TYPE_FILE;
> +
> +        tmp = virJSONValueObjectGetString(value, "output");
> +        if (!tmp) {
> +            parallelsParseError();
> +            return -1;
> +        }
> +
> +        if (!(chr->source.data.file.path = strdup(tmp)))
> +            goto no_memory;
> +    } else if (virJSONValueObjectHasKey(value, "socket")) {
> +        chr->source.type = VIR_DOMAIN_CHR_TYPE_UNIX;
> +
> +        tmp = virJSONValueObjectGetString(value, "socket");
> +        if (!tmp) {
> +            parallelsParseError();
> +            return -1;
> +        }
> +
> +        if (!(chr->source.data.nix.path = strdup(tmp)))
> +            goto no_memory;
> +        chr->source.data.nix.listen = false;
> +    } else if (virJSONValueObjectHasKey(value, "real")) {
> +        chr->source.type = VIR_DOMAIN_CHR_TYPE_DEV;
> +
> +        tmp = virJSONValueObjectGetString(value, "real");
> +        if (!tmp) {
> +            parallelsParseError();
> +            return -1;
> +        }
> +
> +        if (!(chr->source.data.file.path = strdup(tmp)))
> +            goto no_memory;
> +    } else {
> +        parallelsParseError();

Maybe sometimes a more specific error message would be great, but not a 
show stopper.

> +        return -1;
> +    }
> +
> +    return 0;
> +
> +  no_memory:
> +    virReportOOMError();
> +    return -1;
> +}
> +
> +static int
> +parallelsAddSerialInfo(virDomainObjPtr dom,
> +                 const char *key, virJSONValuePtr value)
> +{
> +    virDomainDefPtr def = dom->def;
> +    virDomainChrDefPtr chr = NULL;
> +
> +    if (!(chr = virDomainChrDefNew()))
> +        goto no_memory;
> +
> +    if (parallelsGetSerialInfo(chr, key, value))
> +        goto cleanup;
> +
> +    if (VIR_REALLOC_N(def->serials, def->nserials + 1) < 0) {
> +        virDomainChrDefFree(chr);

Freeing chr is not necessary as the next goto solves that.

> +        goto no_memory;
> +    }
> +
> +    def->serials[def->nserials++] = chr;
> +
> +    return 0;
> +
> +  no_memory:
> +    virReportOOMError();
> +  cleanup:
> +    virDomainChrDefFree(chr);
> +    return -1;
> +}
> +
> +static int
> +parallelsAddDomainHardware(virDomainObjPtr dom, virJSONValuePtr jobj)
> +{
> +    int n, i;
> +    virJSONValuePtr value;
> +    const char *key;
> +
> +    n = virJSONValueObjectKeysNumber(jobj);
> +    if (n < 1)
> +        goto cleanup;
> +
> +    for (i = 0; i < n; i++) {
> +        key = virJSONValueObjectGetKey(jobj, i);
> +        value = virJSONValueObjectGetValue(jobj, i);
> +
> +        if (STRPREFIX(key, "serial")) {
> +            if (parallelsAddSerialInfo(dom, key, value))
> +                goto cleanup;
> +        }
> +    }
> +
> +    return 0;
> +
> +  cleanup:
> +    return -1;
> +}
> +
>   /*
>    * Must be called with privconn->lock held
>    */
> @@ -291,6 +407,9 @@ parallelsLoadDomain(parallelsConnPtr privconn, virJSONValuePtr jobj)
>       else
>           dom->autostart = 0;
>
> +    if (parallelsAddDomainHardware(dom, jobj2) < 0)
> +        goto cleanup_unlock;
> +
>       virDomainObjUnlock(dom);
>
>       return dom;
>

Otherwise looks good to me.

Peter