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(a)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(a)suse.de>
Reviewed-by: Ani Sinha <ani(a)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
>