On Tue, Apr 12, 2022 at 10:09 PM Ani Sinha <ani@anisinha.ca> wrote:
On Tue, Apr 12, 2022 at 2:48 PM Claudio Fontana <cfontana@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@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;
> +    }

There is an obvious issue with this code. We are trying to close and
free a file descriptor that we have not opened when
VIR_DOMAIN_SAVE_BYPASS_CACHE is set in flags.

I meant *not* set in flags.



>
>      if (virQEMUSaveDataWrite(data, fd, path) < 0)
>          goto cleanup;
> --
> 2.34.1
>