On 4/28/22 11:05 AM, Daniel P. Berrangé wrote:
> 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.
Absolutely, will work on it.
>
>>
>> Signed-off-by: Claudio Fontana <cfontana@xxxxxxx>
>> ---
>> 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
>
