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

[Claudio Fontana]

On 4/21/22 6:52 PM, Daniel P. Berrangé wrote:
> On Thu, Apr 21, 2022 at 05:27:09PM +0100, Daniel P. Berrangé wrote:
>> On Tue, Apr 12, 2022 at 11:18:15AM +0200, Claudio Fontana 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>
>>> ---
>>>  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;
>>> +    }
>>
>> This effectively reverts:
>>
>>   commit c4caab538effb57411ad787fedb61e80d557caae
>>   Author: Jiri Denemark <jdenemar at redhat.com>
>>   Date:   Wed Feb 8 14:08:54 2012 +0100
>>
>>     qemu: Always use iohelper for domain save
>>     
>>     This is probably not strictly needed as save operation is not live but
>>     we may have other reasons to avoid blocking qemu's main loop.
>>
>> As the commit message mentions, using a plain file FD results
>> in the QEMU thread being blocked, even when O_NONBLOCK is used.
> 

Then we are stuck with the extra copy apparently, if the helper cannot be taken out of the picture.

The problem is, the only way currently to get the maximum bandwidth out for a regular save (non-multifd), is to skip libvirt entirely,
which is very hard to maintain.

Maybe we need a --skip-iohelper option?


> Oh and the bit about 'save operation is not live' is actually
> outdated. The VM CPUs are stopped when using 'virsh save'
> but are not stopped when using 'virsh snapshot-create' for
> live snapshots.

So --skip-iohelper should be an option for only save and managedsave?

> 
> With regards,
> Daniel
> 

Thanks,

Claudio