[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH 2/8] Basic framework for lock manager plugins



> +/**
> + * virLockManagerNew:
> + * @man: the lock manager context
> + * @type: the type of process to be supervised
> + * @flags: optional flags, currently unused
> + *
> + * Initialize a new context to supervise a process, usually
> + * a virtual machine. If the lock driver requested a
> + * private data, the <code>privateData</code> field of
> + * <code>man</code> will be initialized with a suitable
> + * pointer.
> + *
> + * Returns 0 if successful initialized a new context, -1 on error
> + */
> +typedef int (*virLockDriverNew)(virLockManagerPtr man,
> +                                unsigned int type,
> +                                unsigned int flags);

So I don't malloc man->privateData...

> +/**
> + * virLockDriverFree:
> + * @manager: the lock manager context
> + *
> + * Release any resources associated with the lock manager
> + * context private data
> + */
> +typedef void (*virLockDriverFree)(virLockManagerPtr man);

but I do free it?

> +/**
> + * virLockDriverSetParameter:
> + * @manager: the lock manager context
> + * @key: the parameter name
> + * @value: the parameter value
> + *
> + * Set a configuration parameter for the managed process.
> + * A process of VIR_LOCK_MANAGER_START_DOMAIN will be
> + * given at least 3 parameters:
> + *
> + * - id: the domain unique id
> + * - uuid: the domain uuid
> + * - name: the domain name
> + *
> + * There may be other parameters specific to the lock manager
> + * plugin that are provided for the managed process
> + *
> + * This should only be called prior to the supervised process
> + * being started.
> + *
> + * Returns 0 on success, or -1 on failure
> + */
> +typedef int (*virLockDriverSetParameter)(virLockManagerPtr man,
> +                                         const char *key,
> +                                         const char *value);

I think we may want to set parameters on resources as well.

> +/**
> + * virLockDriverAcquireObject:
> + * @manager: the lock manager context
> + * @state: the current lock state
> + * @flags: optional flags, currently unused
> + *
> + * Start managing resources for the object. If the
> + * object is being transferred from another location
> + * the current lock state may be passed in. This
> + * must be called from the PID that represents the
> + * object to be managed. If the lock is lost at any
> + * time, the PID will be killed off by the lock manager.
> + *
> + * Returns 0 on success, or -1 on failure
> + */
> +typedef int (*virLockDriverAcquireObject)(virLockManagerPtr man,
> +                                          const char *state,
> +                                          unsigned int flags);

I know that the migration part is still pending, but I assume one of the
flags here would indicate "incoming"?

> +/**
> + * virLockDriverGetState:
> + * @manager: the lock manager context
> + * @state: pointer to be filled with lock state
> + * @flags: optional flags, currently unused
> + *
> + * Retrieve the current lock state. The returned
> + * lock state may be NULL if none is required. The
> + * caller is responsible for freeing the lock
> + * state string when it is no longer required
> + *
> + * Returns 0 on success, or -1 on failure.
> + */
> +typedef int (*virLockDriverGetState)(virLockManagerPtr man,
> +                                     char **state,
> +                                     unsigned int flags);

Is there a reason that "state" should be a string instead of binary data
with size?

This may have more uses than migration, but for migration it may be nice
to return the state from the first migrate function on the source.

> +/**
> + * virLockDriverAcquireResource:
> + * @manager: the lock manager context
> + * @type: the resource type virLockDriverResourceType
> + * @name: the resource name
> + * @flags: the resource access flags
> + *
> + * Assign a resource to a managed object. This will
> + * only be called when the object is already locked
> + * and active. eg, to hotplug a disk into a VM.
> + * The format of @name varies according to
> + * the resource @type. A VIR_LOCK_MANAGER_RESOURCE_TYPE_DISK
> + * will have a fully qualified file path.
> + *
> + * If no flags are given, the resource is assumed to be
> + * used in exclusive, read-write mode. Access can be
> + * relaxed to readonly, or shared read-write.
> + *
> + * Returns 0 on success, or -1 on failure
> + */
> +typedef int (*virLockDriverAcquireResource)(virLockManagerPtr man,
> +                                            unsigned int type,
> +                                            const char *name,
> +                                            unsigned int flags);

This doesn't work with my suggestion above about setting parameters on
resources between adding them and acquiring them, if that's something we
want to do.  It also won't allow locking multiple resources together like
the original acquire does, i.e. acquire all these locks or none.

Dave


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]