[libvirt] [PATCH v8 2/8] parallels: add functions to list domains and get info

Peter Krempa pkrempa at redhat.com
Thu Jul 12 13:05:12 UTC 2012


On 07/04/12 19:42, Dmitry Guryanov wrote:
> PARALLELS driver is 'stateless', like vmware or openvz drivers.
> It collects information about domains during startup using
> command-line utility prlctl. VMs in PARALLELS are identified by UUIDs
> or unique names, which can be used as respective fields in
> virDomainDef structure. Currently only basic info, like
> description, virtual cpus number and memory amount, is implemented.
> Querying devices information will be added in the next patches.
>
> PARALLELS doesn't support non-persistent domains - you can't run
> a domain having only disk image, it must always be registered
> in system.
>
> Functions for querying domain info have been just copied from
> test driver with some changes - they extract needed data from
> previously created list of virDomainObj objects.
>
> Signed-off-by: Dmitry Guryanov <dguryanov at parallels.com>

I'm reviewing this without the changes I've suggested in 1/8 review as 
this patch doesn't apply then.


> ---
>   po/POTFILES.in                   |    2 +
>   src/Makefile.am                  |    3 +-
>   src/parallels/parallels_driver.c |  540 +++++++++++++++++++++++++++++++++++++-
>   src/parallels/parallels_driver.h |   14 +
>   src/parallels/parallels_utils.c  |   89 +++++++
>   5 files changed, 646 insertions(+), 2 deletions(-)
>   create mode 100644 src/parallels/parallels_utils.c
>
> diff --git a/po/POTFILES.in b/po/POTFILES.in
> index 1917899..dcb0813 100644
> --- a/po/POTFILES.in
> +++ b/po/POTFILES.in
> @@ -64,6 +64,8 @@ src/openvz/openvz_driver.c
>   src/openvz/openvz_util.c
>   src/phyp/phyp_driver.c
>   src/parallels/parallels_driver.c
> +src/parallels/parallels_driver.h

This won't be needed if you move the error macros into the C file as we 
don't want to expose them from the parallells driver. (See below)

> +src/parallels/parallels_utils.c
>   src/qemu/qemu_agent.c
>   src/qemu/qemu_bridge_filter.c
>   src/qemu/qemu_capabilities.c
> diff --git a/src/Makefile.am b/src/Makefile.am
> index 3aa2a18..93989c8 100644
> --- a/src/Makefile.am
> +++ b/src/Makefile.am
> @@ -481,7 +481,8 @@ HYPERV_DRIVER_EXTRA_DIST =							\
>
>   PARALLELS_DRIVER_SOURCES =					\
>   		parallels/parallels_driver.h			\
> -		parallels/parallels_driver.c
> +		parallels/parallels_driver.c			\
> +		parallels/parallels_utils.c
>
>   NETWORK_DRIVER_SOURCES =					\
>   		network/bridge_driver.h network/bridge_driver.c
> diff --git a/src/parallels/parallels_driver.c b/src/parallels/parallels_driver.c
> index 614f78f..7424c20 100644
> --- a/src/parallels/parallels_driver.c
> +++ b/src/parallels/parallels_driver.c
> @@ -50,12 +50,13 @@
>   #include "configmake.h"
>   #include "storage_file.h"
>   #include "nodeinfo.h"
> -#include "json.h"
> +#include "domain_conf.h"
>
>   #include "parallels_driver.h"
>
>   #define VIR_FROM_THIS VIR_FROM_PARALLELS
>
> +static void parallelsFreeDomObj(void *p);

A prototype is not needed for static funcs.

>   static virCapsPtr parallelsBuildCapabilities(void);
>   static int parallelsClose(virConnectPtr conn);
>
> @@ -77,6 +78,12 @@ parallelsDefaultConsoleType(const char *ostype ATTRIBUTE_UNUSED)
>       return VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SERIAL;
>   }
>
> +static void
> +parallelsFreeDomObj(void *p)

I'd rename this to parallelsDomObjFreePrivate or similar to clarify that 
this serves to free the private data in libvirt's domain object mechanism.

> +{
> +    VIR_FREE(p);

Also later on in parallelsLoadDomain you strdup a string into one of 
members of this struct. This function needs to free that memory, 
otherwise it's a memory leak.


> +};
> +
>   static virCapsPtr
>   parallelsBuildCapabilities(void)
>   {
> @@ -125,6 +132,221 @@ parallelsGetCapabilities(virConnectPtr conn)
>       return xml;
>   }
>
> +/*
> + * Must be called with privconn->lock held
> + */
> +static virDomainObjPtr
> +parallelsLoadDomain(parallelsConnPtr privconn, virJSONValuePtr jobj)
> +{
> +    virDomainObjPtr dom = NULL;
> +    virDomainDefPtr def = NULL;
> +    parallelsDomObjPtr pdom = NULL;
> +    virJSONValuePtr jobj2, jobj3;
> +    const char *tmp;
> +    char *endptr;
> +    unsigned long mem;
> +    unsigned int x;
> +
> +    if (VIR_ALLOC(def) < 0)
> +        goto no_memory;
> +
> +    def->virtType = VIR_DOMAIN_VIRT_PARALLELS;
> +    def->id = -1;
> +
> +    tmp = virJSONValueObjectGetString(jobj, "Name");

I prefer if (!(tmp = virJSONValueObjectGetString(jobj, "Name"))) { if 
the line is short enough. (Here and below)

> +    if (!tmp) {
> +        parallelsParseError();
> +        goto cleanup;
> +    }
> +    if (!(def->name = strdup(tmp)))
> +        goto no_memory;
> +
> +    tmp = virJSONValueObjectGetString(jobj, "ID");
> +    if (!tmp) {
> +        parallelsParseError();
> +        goto cleanup;
> +    }
> +
> +    if (virUUIDParse(tmp, def->uuid) < 0) {
> +        parallelsError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                 _("UUID in config file malformed"));

Missaligned

> +        goto cleanup;
> +    }
> +
> +    tmp = virJSONValueObjectGetString(jobj, "Description");
> +    if (!tmp) {
> +        parallelsParseError();
> +        goto cleanup;
> +    }
> +    if (!(def->description = strdup(tmp)))
> +        goto no_memory;
> +
> +    jobj2 = virJSONValueObjectGet(jobj, "Hardware");
> +    if (!jobj2) {
> +        parallelsParseError();
> +        goto cleanup;
> +    }
> +
> +    jobj3 = virJSONValueObjectGet(jobj2, "cpu");
> +    if (!jobj3) {
> +        parallelsParseError();
> +        goto cleanup;
> +    }
> +
> +    if (virJSONValueObjectGetNumberUint(jobj3, "cpus", &x) < 0) {
> +        parallelsParseError();
> +        goto cleanup;
> +    }
> +    def->vcpus = x;
> +    def->maxvcpus = x;
> +
> +    jobj3 = virJSONValueObjectGet(jobj2, "memory");
> +    if (!jobj3) {
> +        parallelsParseError();
> +        goto cleanup;
> +    }
> +
> +    tmp = virJSONValueObjectGetString(jobj3, "size");
> +
> +    if (virStrToLong_ul(tmp, &endptr, 10, &mem) < 0) {
> +        parallelsParseError();
> +        goto cleanup;
> +    }
> +
> +    if (!STREQ(endptr, "Mb")) {
> +        parallelsParseError();
> +        goto cleanup;
> +    }
> +
> +    def->mem.max_balloon = mem;
> +    def->mem.max_balloon <<= 10;
> +    def->mem.cur_balloon = def->mem.max_balloon;
> +
> +    if (!(def->os.type = strdup("hvm")))
> +        goto no_memory;
> +
> +    if (!(def->os.init = strdup("/sbin/init")))
> +        goto no_memory;
> +
> +    if (!(dom = virDomainAssignDef(privconn->caps,
> +                                   &privconn->domains, def, false)))
> +        goto cleanup;
> +    /* dom is locked here */
> +
> +    if (VIR_ALLOC(pdom) < 0)
> +        goto no_memory_unlock;
> +    dom->privateDataFreeFunc = parallelsFreeDomObj;
> +    dom->privateData = pdom;
> +
> +    if (virJSONValueObjectGetNumberUint(jobj, "EnvID", &x) < 0)
> +        goto cleanup_unlock;
> +    pdom->id = x;
> +    tmp = virJSONValueObjectGetString(jobj, "ID");
> +    if (!tmp) {
> +        parallelsParseError();
> +        goto cleanup_unlock;
> +    }
> +    if (!(pdom->uuid = strdup(tmp)))

Here you assign allocated memory that doesn't get freed in the private 
data free function.

> +        goto no_memory_unlock;
> +
> +    tmp = virJSONValueObjectGetString(jobj, "OS");
> +    if (!tmp)
> +        goto cleanup_unlock;
> +    if (!(pdom->os = strdup(tmp)))

Also here.

> +        goto no_memory_unlock;
> +
> +    dom->persistent = 1;
> +
> +    tmp = virJSONValueObjectGetString(jobj, "State");
> +    if (!tmp) {
> +        parallelsParseError();
> +        goto cleanup_unlock;
> +    }
> +
> +    /* TODO: handle all possible states */
> +    if (STREQ(tmp, "running")) {
> +        virDomainObjSetState(dom, VIR_DOMAIN_RUNNING,
> +                             VIR_DOMAIN_RUNNING_BOOTED);
> +        def->id = pdom->id;
> +    }
> +
> +    tmp = virJSONValueObjectGetString(jobj, "Autostart");
> +    if (!tmp) {
> +        parallelsParseError();
> +        goto cleanup_unlock;
> +    }
> +    if (STREQ(tmp, "on"))
> +        dom->autostart = 1;
> +    else
> +        dom->autostart = 0;
> +
> +    virDomainObjUnlock(dom);
> +
> +    return dom;
> +
> +  no_memory_unlock:
> +    virReportOOMError();
> +  cleanup_unlock:
> +    virDomainObjUnlock(dom);
> +    /* domain list was locked, so nobody could get 'dom'. It has only
> +     * one reference and virDomainObjUnref return 0 here */
> +    if (virDomainObjUnref(dom))

This actualy isn't right: There's a reference in the domain list that is 
added by

 > +    if (!(dom = virDomainAssignDef(privconn->caps,
 > +                                   &privconn->domains, def, false)))

and you can't just remove the underlying data as the domail would still 
exist in terms of looking up the previous pointer and you would then 
access unalocated memory.

The solution would be either to move the call to virDomainAssignDef to a 
place where failure can't happen and then assign all other values, or 
you need to remove the entry from the &privconn->domains hash.


> +        parallelsError(VIR_ERR_INTERNAL_ERROR, _("Can't free virDomainObj"));
> +    return NULL;
> +  no_memory:
> +    virReportOOMError();
> +  cleanup:
> +    virDomainDefFree(def);
> +    return NULL;
> +}
> +
> +/*
> + * Must be called with privconn->lock held
> + *
> + * if domain_name is NULL - load information about all
> + * registered domains.
> + */
> +static int
> +parallelsLoadDomains(parallelsConnPtr privconn, const char *domain_name)
> +{
> +    int count, i;
> +    virJSONValuePtr jobj;
> +    virJSONValuePtr jobj2;
> +    virDomainObjPtr dom = NULL;
> +    int ret = -1;
> +
> +    jobj = parallelsParseOutput(PRLCTL, "list", "-j", "-a",
> +                          "-i", "-H", "--vmtype", "vm", domain_name, NULL);

Bad alignment.

> +    if (!jobj) {
> +        parallelsParseError();
> +        goto cleanup;
> +    }
> +
> +    count = virJSONValueArraySize(jobj);

If I understand this correctly, you require at least one domain to be 
defined in the hypervisor, otherwise you get an error. This behavior is 
a little bit unfortunate. You probably want to check count against 0.

> +    if (count < 1) {
> +        parallelsParseError();
> +        goto cleanup;
> +    }
> +
> +    for (i = 0; i < count; i++) {
> +        jobj2 = virJSONValueArrayGet(jobj, i);
> +        if (!jobj2) {
> +            parallelsParseError();
> +            goto cleanup;
> +        }
> +
> +        dom = parallelsLoadDomain(privconn, jobj2);
> +        if (!dom)
> +            goto cleanup;
> +    }
> +
> +    ret = 0;
> +
> +  cleanup:
> +    virJSONValueFree(jobj);
> +    return ret;
> +}
> +
>   static int
>   parallelsOpenDefault(virConnectPtr conn)
>   {
> @@ -150,6 +372,9 @@ parallelsOpenDefault(virConnectPtr conn)
>       if (virDomainObjListInit(&privconn->domains) < 0)
>           goto error;
>
> +    if (parallelsLoadDomains(privconn, NULL))
> +        goto error;
> +
>       return VIR_DRV_OPEN_SUCCESS;
>
>     error:
> @@ -236,6 +461,306 @@ parallelsGetVersion(virConnectPtr conn ATTRIBUTE_UNUSED, unsigned long *hvVer)
>       return 0;
>   }
>
> +static int
> +parallelsListDomains(virConnectPtr conn, int *ids, int maxids)
> +{
> +    parallelsConnPtr privconn = conn->privateData;
> +    int n;
> +
> +    parallelsDriverLock(privconn);
> +    n = virDomainObjListGetActiveIDs(&privconn->domains, ids, maxids);
> +    parallelsDriverUnlock(privconn);
> +
> +    return n;
> +}
> +
> +static int
> +parallelsNumOfDomains(virConnectPtr conn)
> +{
> +    parallelsConnPtr privconn = conn->privateData;
> +    int count;
> +
> +    parallelsDriverLock(privconn);
> +    count = virDomainObjListNumOfDomains(&privconn->domains, 1);
> +    parallelsDriverUnlock(privconn);
> +
> +    return count;
> +}
> +
> +static int
> +parallelsListDefinedDomains(virConnectPtr conn, char **const names, int maxnames)
> +{
> +    parallelsConnPtr privconn = conn->privateData;
> +    int n;
> +
> +    parallelsDriverLock(privconn);
> +    memset(names, 0, sizeof(*names) * maxnames);
> +    n = virDomainObjListGetInactiveNames(&privconn->domains, names,
> +                                         maxnames);
> +    parallelsDriverUnlock(privconn);
> +
> +    return n;
> +}
> +
> +static int
> +parallelsNumOfDefinedDomains(virConnectPtr conn)
> +{
> +    parallelsConnPtr privconn = conn->privateData;
> +    int count;
> +
> +    parallelsDriverLock(privconn);
> +    count = virDomainObjListNumOfDomains(&privconn->domains, 0);
> +    parallelsDriverUnlock(privconn);
> +
> +    return count;
> +}

I'd suggest adding a handler also for virConnectListAllDomains. You're 
using the virDomainObj code to store machines, so the implementation is 
actualy trivial. See commit 33dc8cf018e41c0e973a4a46a108d5ff30cbbf49

http://libvirt.org/git/?p=libvirt.git;a=commitdiff;h=33dc8cf018e41c0e973a4a46a108d5ff30cbbf49

> +
> +static virDomainPtr
> +parallelsLookupDomainByID(virConnectPtr conn, int id)
> +{
> +    parallelsConnPtr privconn = conn->privateData;
> +    virDomainPtr ret = NULL;
> +    virDomainObjPtr dom;
> +
> +    parallelsDriverLock(privconn);
> +    dom = virDomainFindByID(&privconn->domains, id);
> +    parallelsDriverUnlock(privconn);
> +
> +    if (dom == NULL) {
> +        parallelsError(VIR_ERR_NO_DOMAIN, NULL);
> +        goto cleanup;
> +    }
> +
> +    ret = virGetDomain(conn, dom->def->name, dom->def->uuid);
> +    if (ret)
> +        ret->id = dom->def->id;
> +
> +  cleanup:
> +    if (dom)
> +        virDomainObjUnlock(dom);
> +    return ret;
> +}
> +
> +static virDomainPtr
> +parallelsLookupDomainByUUID(virConnectPtr conn, const unsigned char *uuid)
> +{
> +    parallelsConnPtr privconn = conn->privateData;
> +    virDomainPtr ret = NULL;
> +    virDomainObjPtr dom;
> +
> +    parallelsDriverLock(privconn);
> +    dom = virDomainFindByUUID(&privconn->domains, uuid);
> +    parallelsDriverUnlock(privconn);
> +
> +    if (dom == NULL) {
> +        parallelsError(VIR_ERR_NO_DOMAIN, NULL);
> +        goto cleanup;
> +    }
> +
> +    ret = virGetDomain(conn, dom->def->name, dom->def->uuid);
> +    if (ret)
> +        ret->id = dom->def->id;
> +
> +  cleanup:
> +    if (dom)
> +        virDomainObjUnlock(dom);
> +    return ret;
> +}
> +
> +static virDomainPtr
> +parallelsLookupDomainByName(virConnectPtr conn, const char *name)
> +{
> +    parallelsConnPtr privconn = conn->privateData;
> +    virDomainPtr ret = NULL;
> +    virDomainObjPtr dom;
> +
> +    parallelsDriverLock(privconn);
> +    dom = virDomainFindByName(&privconn->domains, name);
> +    parallelsDriverUnlock(privconn);
> +
> +    if (dom == NULL) {
> +        parallelsError(VIR_ERR_NO_DOMAIN, NULL);
> +        goto cleanup;
> +    }
> +
> +    ret = virGetDomain(conn, dom->def->name, dom->def->uuid);
> +    if (ret)
> +        ret->id = dom->def->id;
> +
> +  cleanup:
> +    if (dom)
> +        virDomainObjUnlock(dom);
> +    return ret;
> +}
> +
> +static int
> +parallelsGetDomainInfo(virDomainPtr domain, virDomainInfoPtr info)
> +{
> +    parallelsConnPtr privconn = domain->conn->privateData;
> +    virDomainObjPtr privdom;
> +    int ret = -1;
> +
> +    parallelsDriverLock(privconn);
> +    privdom = virDomainFindByName(&privconn->domains, domain->name);
> +    parallelsDriverUnlock(privconn);
> +
> +    if (privdom == NULL) {
> +        parallelsError(VIR_ERR_NO_DOMAIN,
> +                 _("no domain with matching name '%s'"), domain->name);

Bad alignment.

> +        goto cleanup;
> +    }
> +
> +    info->state = virDomainObjGetState(privdom, NULL);
> +    info->memory = privdom->def->mem.cur_balloon;
> +    info->maxMem = privdom->def->mem.max_balloon;
> +    info->nrVirtCpu = privdom->def->vcpus;
> +    info->cpuTime = 0;
> +    ret = 0;
> +
> +  cleanup:
> +    if (privdom)
> +        virDomainObjUnlock(privdom);
> +    return ret;
> +}
> +
> +static char *
> +parallelsGetOSType(virDomainPtr dom)
> +{
> +    parallelsConnPtr privconn = dom->conn->privateData;
> +    virDomainObjPtr privdom;
> +    parallelsDomObjPtr pdom;
> +
> +    char *ret = NULL;
> +
> +    parallelsDriverLock(privconn);
> +    privdom = virDomainFindByName(&privconn->domains, dom->name);
> +    if (privdom == NULL) {
> +        parallelsError(VIR_ERR_NO_DOMAIN,
> +                 _("no domain with matching name '%s'"), dom->name);

Bad alignment.

> +        goto cleanup;
> +    }
> +
> +    pdom = privdom->privateData;
> +
> +    if (!(ret = strdup(pdom->os)))
> +        virReportOOMError();
> +
> +  cleanup:
> +    if (privdom)
> +        virDomainObjUnlock(privdom);
> +    parallelsDriverUnlock(privconn);
> +    return ret;
> +}
> +
> +static int
> +parallelsDomainIsPersistent(virDomainPtr dom)
> +{
> +    parallelsConnPtr privconn = dom->conn->privateData;
> +    virDomainObjPtr privdom;
> +    int ret = -1;
> +
> +    parallelsDriverLock(privconn);
> +    privdom = virDomainFindByName(&privconn->domains, dom->name);
> +    if (privdom == NULL) {
> +        parallelsError(VIR_ERR_NO_DOMAIN,
> +                 _("no domain with matching name '%s'"), dom->name);

Bad alignment.

> +        goto cleanup;
> +    }
> +
> +    ret = 1;
> +
> +  cleanup:
> +    if (privdom)
> +        virDomainObjUnlock(privdom);
> +    parallelsDriverUnlock(privconn);
> +    return ret;
> +}
> +
> +static int
> +parallelsDomainGetState(virDomainPtr domain,
> +                  int *state, int *reason, unsigned int flags)
> +{
> +    parallelsConnPtr privconn = domain->conn->privateData;
> +    virDomainObjPtr privdom;
> +    int ret = -1;
> +    virCheckFlags(0, -1);
> +
> +    parallelsDriverLock(privconn);
> +    privdom = virDomainFindByName(&privconn->domains, domain->name);
> +    parallelsDriverUnlock(privconn);
> +
> +    if (privdom == NULL) {
> +        parallelsError(VIR_ERR_NO_DOMAIN,
> +                 _("no domain with matching name '%s'"), domain->name);

Bad alignment.

> +        goto cleanup;
> +    }
> +
> +    *state = virDomainObjGetState(privdom, reason);
> +    ret = 0;
> +
> +  cleanup:
> +    if (privdom)
> +        virDomainObjUnlock(privdom);
> +    return ret;
> +}
> +
> +static char *
> +parallelsDomainGetXMLDesc(virDomainPtr domain, unsigned int flags)
> +{
> +    parallelsConnPtr privconn = domain->conn->privateData;
> +    virDomainDefPtr def;
> +    virDomainObjPtr privdom;
> +    char *ret = NULL;
> +
> +    /* Flags checked by virDomainDefFormat */
> +
> +    parallelsDriverLock(privconn);
> +    privdom = virDomainFindByName(&privconn->domains, domain->name);
> +    parallelsDriverUnlock(privconn);
> +
> +    if (privdom == NULL) {
> +        parallelsError(VIR_ERR_NO_DOMAIN,
> +                 _("no domain with matching name '%s'"), domain->name);

Bad alignment.

> +        goto cleanup;
> +    }
> +
> +    def = (flags & VIR_DOMAIN_XML_INACTIVE) &&
> +        privdom->newDef ? privdom->newDef : privdom->def;
> +
> +    ret = virDomainDefFormat(def, flags);
> +
> +  cleanup:
> +    if (privdom)
> +        virDomainObjUnlock(privdom);
> +    return ret;
> +}
> +
> +static int
> +parallelsDomainGetAutostart(virDomainPtr domain, int *autostart)
> +{
> +    parallelsConnPtr privconn = domain->conn->privateData;
> +    virDomainObjPtr privdom;
> +    int ret = -1;
> +
> +    parallelsDriverLock(privconn);
> +    privdom = virDomainFindByName(&privconn->domains, domain->name);
> +    parallelsDriverUnlock(privconn);
> +
> +    if (privdom == NULL) {
> +        parallelsError(VIR_ERR_NO_DOMAIN,
> +                 _("no domain with matching name '%s'"), domain->name);

Bad alignment.

> +        goto cleanup;
> +    }
> +
> +    *autostart = privdom->autostart;
> +    ret = 0;
> +
> +  cleanup:
> +    if (privdom)
> +        virDomainObjUnlock(privdom);
> +    return ret;
> +}
> +
>   static virDriver parallelsDriver = {
>       .no = VIR_DRV_PARALLELS,
>       .name = "PARALLELS",
> @@ -245,6 +770,19 @@ static virDriver parallelsDriver = {
>       .getHostname = virGetHostname,      /* 0.10.0 */
>       .nodeGetInfo = nodeGetInfo,      /* 0.10.0 */
>       .getCapabilities = parallelsGetCapabilities,      /* 0.10.0 */
> +    .listDomains = parallelsListDomains,      /* 0.10.0 */
> +    .numOfDomains = parallelsNumOfDomains,    /* 0.10.0 */
> +    .listDefinedDomains = parallelsListDefinedDomains,        /* 0.10.0 */
> +    .numOfDefinedDomains = parallelsNumOfDefinedDomains,      /* 0.10.0 */
> +    .domainLookupByID = parallelsLookupDomainByID,    /* 0.10.0 */
> +    .domainLookupByUUID = parallelsLookupDomainByUUID,        /* 0.10.0 */
> +    .domainLookupByName = parallelsLookupDomainByName,        /* 0.10.0 */
> +    .domainGetOSType = parallelsGetOSType,    /* 0.10.0 */
> +    .domainGetInfo = parallelsGetDomainInfo,  /* 0.10.0 */
> +    .domainGetState = parallelsDomainGetState,        /* 0.10.0 */
> +    .domainGetXMLDesc = parallelsDomainGetXMLDesc,    /* 0.10.0 */
> +    .domainIsPersistent = parallelsDomainIsPersistent,        /* 0.10.0 */
> +    .domainGetAutostart = parallelsDomainGetAutostart,        /* 0.10.0 */
>   };
>
>   /**
> diff --git a/src/parallels/parallels_driver.h b/src/parallels/parallels_driver.h
> index c04db35..8398c02 100644
> --- a/src/parallels/parallels_driver.h
> +++ b/src/parallels/parallels_driver.h
> @@ -28,11 +28,23 @@
>   # include "storage_conf.h"
>   # include "domain_event.h"
>
> +# include "json.h"
> +
>   # define parallelsError(code, ...)                                         \
>           virReportErrorHelper(VIR_FROM_TEST, code, __FILE__,         \
>                                __FUNCTION__, __LINE__, __VA_ARGS__)
>   # define PRLCTL      "prlctl"
> +# define parallelsParseError()                                                          \
> +        virReportErrorHelper(VIR_FROM_TEST, VIR_ERR_OPERATION_FAILED, __FILE__,  \
> +                             __FUNCTION__, __LINE__, _("Can't parse prlctl output"))
> +
> +struct parallelsDomObj {
> +    int id;
> +    char *uuid;
> +    char *os;
> +};
>
> +typedef struct parallelsDomObj *parallelsDomObjPtr;

The data and macros above are private to the parallels driver. They 
should reside in the corresponding C files, or a separate header that 
doesn't get exposed to the rest of libvirt.

>
>   struct _parallelsConn {
>       virMutex lock;
> @@ -48,4 +60,6 @@ typedef struct _parallelsConn *parallelsConnPtr;
>
>   int parallelsRegister(void);
>
> +virJSONValuePtr parallelsParseOutput(const char *binary, ...) ATTRIBUTE_NONNULL(1) ATTRIBUTE_SENTINEL;
> +

Also this function shouldn't be used outside of this driver.

>   #endif
> diff --git a/src/parallels/parallels_utils.c b/src/parallels/parallels_utils.c
> new file mode 100644
> index 0000000..845adf4
> --- /dev/null
> +++ b/src/parallels/parallels_utils.c
> @@ -0,0 +1,89 @@
> +/*
> + * parallels_utils.c: core driver functions for managing
> + * Parallels Virtuozzo Server hosts
> + *
> + * Copyright (C) 2012 Parallels, Inc.
> + *
> + * This library is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * This library is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with this library; if not, write to the Free Software
> + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307  USA
> + *
> + */
> +
> +#include <config.h>
> +
> +#include <stdarg.h>
> +
> +#include "command.h"
> +#include "virterror_internal.h"
> +#include "memory.h"
> +
> +#include "parallels_driver.h"
> +
> +static int
> +parallelsDoCmdRun(char **outbuf, const char *binary, va_list list)
> +{
> +    virCommandPtr cmd = virCommandNew(binary);
> +    const char *arg;
> +    char *scmd = NULL;
> +    int ret = -1;
> +
> +    while ((arg = va_arg(list, const char *)) != NULL)
> +        virCommandAddArg(cmd, arg);
> +
> +    if (outbuf)
> +        virCommandSetOutputBuffer(cmd, outbuf);
> +
> +    scmd = virCommandToString(cmd);
> +    if (!scmd)
> +        goto cleanup;
> +
> +    if (virCommandRun(cmd, NULL))
> +        goto cleanup;
> +
> +    ret = 0;
> +
> +  cleanup:
> +    VIR_FREE(scmd);
> +    virCommandFree(cmd);
> +    if (ret)
> +        VIR_FREE(*outbuf);
> +    return ret;
> +}
> +
> +/*
> + * Run command and parse its JSON output, return
> + * pointer to virJSONValue or NULL in case of error.
> + */
> +virJSONValuePtr
> +parallelsParseOutput(const char *binary, ...)
> +{
> +    char *outbuf;
> +    virJSONValuePtr jobj = NULL;
> +    va_list list;
> +    int ret;
> +
> +    va_start(list, binary);
> +    ret = parallelsDoCmdRun(&outbuf, binary, list);
> +    va_end(list);
> +    if (ret)
> +        return NULL;
> +
> +    jobj = virJSONValueFromString(outbuf);
> +    if (!jobj)
> +        parallelsError(VIR_ERR_INTERNAL_ERROR, "%s: %s",
> +                 _("invalid output from prlctl"), outbuf);

Bad alignment.

> +
> +    VIR_FREE(outbuf);
> +    return jobj;
> +}
>





More information about the libvir-list mailing list