[Libvirt-cim] [PATCH 1 of 3] Add VirtualSystemSnapshotService

[Kaitlin Rupert]

Dan Smith wrote:
> # HG changeset patch
> # User Dan Smith <danms at us.ibm.com>
> # Date 1204053596 28800
> # Node ID fce1a31a53090b42dbfbd788a6db2a1164636197
> # Parent  bb0530f50ea8d4a75ec34cd54f3bc2daebbda556
> Add VirtualSystemSnapshotService
> 
> Changes:
>  - Added header block
> 
> Signed-off-by: Dan Smith <danms at us.ibm.com>
> 
> diff -r bb0530f50ea8 -r fce1a31a5309 src/Virt_VirtualSystemSnapshotService.c


> +static CMPI_THREAD_RETURN snapshot_thread(struct snap_context *ctx)
> +{
> +        CMPIStatus s;

We don't use this, which is fine.  However, leads to a question.. why 
the decision to have do_snapshot() return void?

> +
> +char *vsss_get_save_path(const char *domname)

The function is defined as vss_get_save_path() here, but called as 
get_save_path() later on.

> +{
> +        int ret;
> +        char *path = NULL;
> +
> +        ret = asprintf(&path,
> +                       "/var/lib/libvirt/%s.save", domname);

I assume this path will be valid on all systems / distros?

> +
> +static struct snap_context *new_context(const char *name,
> +                                        CMPIStatus *s)
> +{
> +        struct snap_context *ctx;
> +        uuid_t uuid;
> +
> +        ctx = calloc(1, sizeof(*ctx));
> +        if (ctx == NULL) {
> +                CU_DEBUG("Failed to alloc snapshot context");
> +                goto out;
> +        }
> +
> +        ctx->domain = strdup(name);
> +
> +        uuid_generate(uuid);
> +        uuid_unparse(uuid, ctx->uuid);
> +
> +        ctx->save_path = get_save_path(ctx->domain);

Needs to be vsss_get_save_path()

> +        if (ctx->save_path == NULL) {
> +                cu_statusf(_BROKER, s,
> +                           CMPI_RC_ERR_FAILED,
> +                           "Unable to get save_path");
> +                goto out;
> +        }
> +
> +        cu_statusf(_BROKER, s,
> +                   CMPI_RC_OK,
> +                   "");
> + out:
> +        if (s->rc != CMPI_RC_OK) {
> +                snap_job_free(ctx);
> +                ctx = NULL;
> +        }
> +
> +        return ctx;
> +}
> +
> +static CMPIStatus start_snapshot_job(const CMPIObjectPath *ref,
> +                                     const CMPIContext *context,
> +                                     const char *name,
> +                                     uint16_t type)
> +{
> +        struct snap_context *ctx;
> +        CMPIStatus s;

Need to initialize s here.  Or make sure to set the status when the 
calloc fails in new_context().

> +        CMPIObjectPath *job;
> +
> +        ctx = new_context(name, &s);
> +        if (ctx == NULL)
> +                goto out;
> +
> +        ctx->save = (type != 0);
> +        ctx->restore = (type != VIR_VSSS_SNAPSHOT_MEM);
> +
> +        s = create_job(context, ref, ctx, &job);
> +
> + out:
> +        return s;
> +}
> +

> +
> +static CMPIStatus destroy_snapshot(CMPIMethodMI *self,
> +                                   const CMPIContext *context,
> +                                   const CMPIResult *results,
> +                                   const CMPIObjectPath *reference,
> +                                   const CMPIArgs *argsin,
> +                                   CMPIArgs *argsout)
> +{
> +        CMPIStatus s = {CMPI_RC_OK, NULL};
> +        CMPIObjectPath *snap;
> +        char *name = NULL;
> +        char *path = NULL;

> +        path = get_save_path(name);

Needs to be vsss_get_save_path()

-- 
Kaitlin Rupert
IBM Linux Technology Center
kaitlin at linux.vnet.ibm.com