On 4/12/22 7:23 PM, Ani Sinha wrote:
> On Tue, Apr 12, 2022 at 10:09 PM Ani Sinha <ani@xxxxxxxxxxx> wrote:
>
>> On Tue, Apr 12, 2022 at 2:48 PM Claudio Fontana <cfontana@xxxxxxx> 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@xxxxxxx>
>>> ---
>>> 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.
I don't think so. I don't think it is obvious, so can you be more specific?
