[libvirt RFC] qemu_saveimage: only use wrapper when using --bypass-cache

[Claudio Fontana]

a ping on this one;

this change makes sense to me, and makes performance better in specific cases, by avoiding --bypass-cache before reaching the "cache trashing" state.

However, before virsh save was guarenteed to go through iohelper, now it is not.

The iohelper was guaranteeing a virFileDataSync before exiting,
while after the change the data will still be in-flight between the kernel and the device as virsh save returns to the prompt.

Seems more efficient, but wanted to mention this as a user-visible change.

Thoughts?

Claudio


On 4/13/22 9:18 AM, Ani Sinha wrote:
> On Tue, Apr 12, 2022 at 2:48 PM Claudio Fontana <cfontana at suse.de> wrote:
>>
>> align the "save" with the "restore" code,
>> by only using the wrapper when using --bypass-cache.
>>
>> This avoids a copy, resulting in better performance.
>>
>> Signed-off-by: Claudio Fontana <cfontana at suse.de>
> 
> Reviewed-by: Ani Sinha <ani at anisinha.ca>
> 
>> ---
>>  src/qemu/qemu_saveimage.c | 6 ++++--
>>  1 file changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/src/qemu/qemu_saveimage.c b/src/qemu/qemu_saveimage.c
>> index 4fd4c5cfcd..5ea1b2fbcc 100644
>> --- a/src/qemu/qemu_saveimage.c
>> +++ b/src/qemu/qemu_saveimage.c
>> @@ -289,8 +289,10 @@ qemuSaveImageCreate(virQEMUDriver *driver,
>>      if (qemuSecuritySetImageFDLabel(driver->securityManager, vm->def, fd) < 0)
>>          goto cleanup;
>>
>> -    if (!(wrapperFd = virFileWrapperFdNew(&fd, path, wrapperFlags)))
>> -        goto cleanup;
>> +    if ((flags & VIR_DOMAIN_SAVE_BYPASS_CACHE)) {
>> +        if (!(wrapperFd = virFileWrapperFdNew(&fd, path, wrapperFlags)))
>> +            goto cleanup;
>> +    }
>>
>>      if (virQEMUSaveDataWrite(data, fd, path) < 0)
>>          goto cleanup;
>> --
>> 2.34.1
>>
>