On 5/13/22 14:57, Claudio Fontana wrote:
> 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])
That's likely caused by virsh. I haven't checked your patches yet, but
depending on how the argument is queried for in virsh you may get a NULL
or an empty string. However, empty string doesn't matter as it produces
sensible error message:
No such file or directory: ''
Michal
