On Wed, Apr 27, 2022 at 11:13:25PM +0200, Claudio Fontana wrote:
> add new API in order to be able to extend parameters to the domain
> save operation. We will use it to fit the existing arguments of
> VirDomainSaveFlags, and then add parallel saves functionality.
Regardless of my thoughts on the parallel save to multiple files,
I think this API is conceptually a benefit, given it is more
extensible long term. Likewise for the restore counterpart.
Could you split off the new save/restore APIs into a standalone
patch series, that simply wires them up with the existing
functionality. That way we can get the APIs merged without being
blocked on any ongoing parallel save design debate.
IOW, this patch and the next 5 that follow could be merged
fairly easily, if the VIR_DOMAIN_SAVE_PARALLEL flag is added
in a later patch.
>
> Signed-off-by: Claudio Fontana <cfont...@suse.de>
> ---
> include/libvirt/libvirt-domain.h | 45 ++++++++++++++++++++++++++++
> src/driver-hypervisor.h | 7 +++++
> src/libvirt-domain.c | 51 ++++++++++++++++++++++++++++++++
> src/libvirt_public.syms | 5 ++++
> 4 files changed, 108 insertions(+)
>
> diff --git a/include/libvirt/libvirt-domain.h
> b/include/libvirt/libvirt-domain.h
> index 9aa214f3df..b870a73b64 100644
> --- a/include/libvirt/libvirt-domain.h
> +++ b/include/libvirt/libvirt-domain.h
> @@ -1481,6 +1481,8 @@ typedef enum {
> VIR_DOMAIN_SAVE_RUNNING = 1 << 1, /* Favor running over paused */
> VIR_DOMAIN_SAVE_PAUSED = 1 << 2, /* Favor paused over running */
> VIR_DOMAIN_SAVE_RESET_NVRAM = 1 << 3, /* Re-initialize NVRAM from
> template */
> + /** Since: v8.3.0 */
> + VIR_DOMAIN_SAVE_PARALLEL = 1 << 4, /* Parallel Save/Restore to
> multiple files */
> } virDomainSaveRestoreFlags;
>
> int virDomainSave (virDomainPtr domain,
> @@ -1489,6 +1491,10 @@ int virDomainSaveFlags
> (virDomainPtr domain,
> const char *to,
> const char *dxml,
> unsigned int flags);
> +int virDomainSaveParametersFlags (virDomainPtr domain,
> + virTypedParameterPtr
> params,
> + int nparams,
> + unsigned int flags);
> int virDomainRestore (virConnectPtr conn,
> const char *from);
> int virDomainRestoreFlags (virConnectPtr conn,
> @@ -1496,6 +1502,45 @@ int virDomainRestoreFlags
> (virConnectPtr conn,
> const char *dxml,
> unsigned int flags);
>
> +/**
> + * VIR_SAVE_PARAM_FILE:
> + *
> + * the parameter used to specify the savestate file to save to or restore
> from.
> + * For parallel saves, this is the main file, with the extra connections
> adding suffix
> + * .1 .2 .3 ... up to VIR_SAVE_PARAM_PARALLEL_CONNECTIONS.
> + *
> + * Since: v8.3.0
> + */
> +# define VIR_SAVE_PARAM_FILE "file"
> +
> +/**
> + * VIR_SAVE_PARAM_DXML:
> + *
> + * an optional parameter used to adjust guest xml on restore.
> + * If the hypervisor supports it, it can be used to alter
> + * host-specific portions of the domain XML that will be used when
> + * restoring an image. For example, it is possible to alter the
> + * device while the domain is stopped.
> + *
> + * Since: v8.3.0
> + */
> +# define VIR_SAVE_PARAM_DXML "dxml"
> +
> +/**
> + * VIR_SAVE_PARAM_PARALLEL_CONNECTIONS:
> + *
> + * this optional parameter mirrors the migration parameter
> + * VIR_MIGRATE_PARAM_PARALLEL_CONNECTIONS.
> + *
> + * This parameter is used when saving or restoring state files
> + * in parallel using the flag VIR_DOMAIN_SAVE_PARALLEL .
> + * It specifies the number of extra files to save/restore
> + * using parallel connections.
> + *
> + * Since: v8.3.0
> + */
> +# define VIR_SAVE_PARAM_PARALLEL_CONNECTIONS "parallel.connections"
> +
> /* See below for virDomainSaveImageXMLFlags */
> char * virDomainSaveImageGetXMLDesc (virConnectPtr conn,
> const char *file,
> diff --git a/src/driver-hypervisor.h b/src/driver-hypervisor.h
> index 4423eb0885..a4e1d21e76 100644
> --- a/src/driver-hypervisor.h
> +++ b/src/driver-hypervisor.h
> @@ -240,6 +240,12 @@ typedef int
> const char *dxml,
> unsigned int flags);
>
> +typedef int
> +(*virDrvDomainSaveParametersFlags)(virDomainPtr domain,
> + virTypedParameterPtr params,
> + int nparams,
> + unsigned int flags);
> +
> typedef int
> (*virDrvDomainRestore)(virConnectPtr conn,
> const char *from);
> @@ -1489,6 +1495,7 @@ struct _virHypervisorDriver {
> virDrvDomainGetControlInfo domainGetControlInfo;
> virDrvDomainSave domainSave;
> virDrvDomainSaveFlags domainSaveFlags;
> + virDrvDomainSaveParametersFlags domainSaveParametersFlags;
> virDrvDomainRestore domainRestore;
> virDrvDomainRestoreFlags domainRestoreFlags;
> virDrvDomainSaveImageGetXMLDesc domainSaveImageGetXMLDesc;
> diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
> index cbd7902d2d..9e4fcfd022 100644
> --- a/src/libvirt-domain.c
> +++ b/src/libvirt-domain.c
> @@ -959,6 +959,57 @@ virDomainSaveFlags(virDomainPtr domain, const char *to,
> return -1;
> }
>
> +/**
> + * virDomainSaveParametersFlags:
> + * @domain: a domain object
> + * @params: save parameters
> + * @nparams: number of save parameters
> + * @flags: bitwise-OR of virDomainSaveRestoreFlags
> + *
> + * This method extends virDomainSaveFlags by adding parameters to Save.
> + *
> + * If @flags includes VIR_DOMAIN_SAVE_PARALLEL, then libvirt will
> + * attempt to trigger a parallel transfer to multiple files,
> + * where the number of extra files is determined by the parameter
> + * VIR_SAVE_PARAM_PARALLEL_CONNECTIONS.
> + *
> + * Returns 0 in case of success and -1 in case of failure.
> + */
> +int
> +virDomainSaveParametersFlags(virDomainPtr domain,
> + virTypedParameterPtr params, int nparams,
> + unsigned int flags)
> +{
> + virConnectPtr conn;
> +
> + VIR_DOMAIN_DEBUG(domain, "params=%p, nparams=%d, flags=0x%x",
> + params, nparams, flags);
> + VIR_TYPED_PARAMS_DEBUG(params, nparams);
> +
> + virResetLastError();
> +
> + virCheckDomainReturn(domain, -1);
> + conn = domain->conn;
> +
> + virCheckReadOnlyGoto(conn->flags, error);
> +
> + VIR_EXCLUSIVE_FLAGS_GOTO(VIR_DOMAIN_SAVE_RUNNING,
> + VIR_DOMAIN_SAVE_PAUSED,
> + error);
> +
> + if (conn->driver->domainSaveParametersFlags) {
> + if (conn->driver->domainSaveParametersFlags(domain, params, nparams,
> flags) < 0)
> + goto error;
> + return 0;
> + }
> +
> + virReportUnsupportedError();
> +
> + error:
> + virDispatchError(domain->conn);
> + return -1;
> +}
> +
>
> /**
> * virDomainRestore:
> diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms
> index f93692c427..eb3a7afb75 100644
> --- a/src/libvirt_public.syms
> +++ b/src/libvirt_public.syms
> @@ -916,4 +916,9 @@ LIBVIRT_8.0.0 {
> virDomainSetLaunchSecurityState;
> } LIBVIRT_7.8.0;
>
> +LIBVIRT_8.3.0 {
> + global:
> + virDomainSaveParametersFlags;
> +} LIBVIRT_8.0.0;
> +
> # .... define new API here using predicted next version number ....
> --
> 2.34.1
>
With regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
