On 5/13/22 9:54 AM, Michal Privoznik wrote:
> When no VIR_DOMAIN_SAVE_PARAM_FILE typed param is set when
> calling virDomainSaveParams() then in turn virQEMUFileOpenAs()
> tries to open a NULL path.
>
> We have two options now:
> 1) require the typed param, which in turn may be promoted to a
> regular argument, or
>
> 2) use this opportunity to make the API behave like
> virDomainManagedSave() and use typed params to pass extra
> arguments, instead of having to invent new managed save API
> with typed params.
>
> Let's go with option 2, as it is more future proof.
>
> Signed-off-by: Michal Privoznik <mprivozn@xxxxxxxxxx>
> ---
> src/libvirt-domain.c | 2 ++
> src/qemu/qemu_driver.c | 33 ++++++++++++++++++++-------------
> 2 files changed, 22 insertions(+), 13 deletions(-)
>
> diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
> index d1d62daa71..208c2438fe 100644
> --- a/src/libvirt-domain.c
> +++ b/src/libvirt-domain.c
> @@ -1007,6 +1007,8 @@ virDomainSaveFlags(virDomainPtr domain, const char *to,
> * @flags: bitwise-OR of virDomainSaveRestoreFlags
> *
> * This method extends virDomainSaveFlags by adding parameters.
> + * If VIR_DOMAIN_SAVE_PARAM_FILE is not provided then a managed save is
> + * performed (see virDomainManagedSave).
> *
> * Returns 0 in case of success and -1 in case of failure.
> *
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 7d8c7176d9..0b31c73bb9 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -2772,6 +2772,7 @@ qemuDomainManagedSavePath(virQEMUDriver *driver, virDomainObj *vm)
> static int
> qemuDomainManagedSaveHelper(virQEMUDriver *driver,
> virDomainObj *vm,
> + const char *dxml,
> unsigned int flags)
> {
> g_autoptr(virQEMUDriverConfig) cfg = NULL;
> @@ -2799,7 +2800,7 @@ qemuDomainManagedSaveHelper(virQEMUDriver *driver,
> VIR_INFO("Saving state of domain '%s' to '%s'", vm->def->name, path);
>
> if (qemuDomainSaveInternal(driver, vm, path, compressed,
> - compressor, NULL, flags) < 0)
> + compressor, dxml, flags) < 0)
> return -1;
>
> vm->hasManagedSave = true;
> @@ -2853,17 +2854,18 @@ qemuDomainSave(virDomainPtr dom, const char *path)
>
> static int
> qemuDomainSaveParams(virDomainPtr dom,
> - virTypedParameterPtr params, int nparams,
> + virTypedParameterPtr params,
> + int nparams,
> unsigned int flags)
> {
> + virQEMUDriver *driver = dom->conn->privateData;
> + g_autoptr(virQEMUDriverConfig) cfg = NULL;
> + virDomainObj *vm = NULL;
> + g_autoptr(virCommand) compressor = NULL;
> const char *to = NULL;
> const char *dxml = NULL;
> - virQEMUDriver *driver = dom->conn->privateData;
> int compressed;
> - g_autoptr(virCommand) compressor = NULL;
> int ret = -1;
> - virDomainObj *vm = NULL;
> - g_autoptr(virQEMUDriverConfig) cfg = NULL;
>
> virCheckFlags(VIR_DOMAIN_SAVE_BYPASS_CACHE |
> VIR_DOMAIN_SAVE_RUNNING |
> @@ -2884,18 +2886,23 @@ qemuDomainSaveParams(virDomainPtr dom,
> VIR_DOMAIN_SAVE_PARAM_DXML, &dxml) < 0)
> return -1;
>
> - cfg = virQEMUDriverGetConfig(driver);
> - if ((compressed = qemuSaveImageGetCompressionProgram(cfg->saveImageFormat,
> - &compressor,
> - "save", false)) < 0)
> - goto cleanup;
> -
> if (!(vm = qemuDomainObjFromDomain(dom)))
> goto cleanup;
>
> if (virDomainSaveParamsEnsureACL(dom->conn, vm->def) < 0)
> goto cleanup;
>
> + if (!to) {
> + /* If no save path was provided then this behaves as managed save. */
> + return qemuDomainManagedSaveHelper(driver, vm, dxml, flags);
> + }
Is this sufficient? In some cases I am seeing that omitted string parameters are passed as the empty string.
In my multifd series I encountered this with the optional --parallel-compression argument,
that in this case would mean:
if (!to || !to[0])
Thanks,
Claudio
> +
> + cfg = virQEMUDriverGetConfig(driver);
> + if ((compressed = qemuSaveImageGetCompressionProgram(cfg->saveImageFormat,
> + &compressor,
> + "save", false)) < 0)
> + goto cleanup;
> +
> if (virDomainObjCheckActive(vm) < 0)
> goto cleanup;
>
> @@ -2925,7 +2932,7 @@ qemuDomainManagedSave(virDomainPtr dom, unsigned int flags)
> if (virDomainManagedSaveEnsureACL(dom->conn, vm->def) < 0)
> goto cleanup;
>
> - ret = qemuDomainManagedSaveHelper(driver, vm, flags);
> + ret = qemuDomainManagedSaveHelper(driver, vm, NULL, flags);
>
> cleanup:
> virDomainObjEndAPI(&vm);
>
