Re: [Qemu-devel] [Qemu-block] Q: Report of leaked clusters with qcow2 when disk is resized with a live VM

[Darren Kenny <darren.kenny@oracle.com>]

Hi Kevin,

Thanks for getting back to me so quickly.

Kevin Wolf wrote:
> Am 13.09.2017 um 14:00 hat Darren Kenny geschrieben:
>> [Cross-posted from qemu-devel, meant to send here first]
>
> Just keep both lists in the CC for the same email.

Will do.
> There is an issue here, which is that you are accessing the image at 
> the same time from two separate processes. qemu is using writeback 
> caches in order to improve performance, so only after the guest has 
> issued a flush command to its disk or after you shut down or at least 
> pause qemu, the changes are fully written to the image file. In qemu 
> 2.10, you would probably see this instead: $ qemu-img check 
> ./test.qcow2 qemu-img: Could not open './test.qcow2': Failed to get 
> shared "write" lock Is another process using the image? This lock can 
> be overridden, but at least it shows clearly that you are doing 
> something that you probably shouldn't be doing.

Hmm, I've just updated to the HEAD of the Git repo, and I didn't see this
locking behaviour, it still did the same thing as before.

Does the disk need to be formatted/mounted before it's seen as locked?
Or even a configure option?

The version that have is:

     $ qemu-img --version
     qemu-img version 2.10.50 (v2.10.0-476-g04ef330)
     Copyright (c) 2003-2017 Fabrice Bellard and the QEMU Project developers

     $ qemu-system-x86_64 --version
     QEMU emulator version 2.10.50 (v2.10.0-476-g04ef330)
     Copyright (c) 2003-2017 Fabrice Bellard and the QEMU Project developers

The last commit I have is (as in the version string):

     04ef330 tcg/tci: do not use ldst label (never implemented)

>> What I observed, then was that if I powered down the VM, or even just
>> quit the VM, that the subsequent check of the disk would say that
>> everything was just fine, and there no longer were leaked clusters.
>
> Yes, quitting the VM writes out all of the cached data, so the on-disk
> state and the in-memory state become fully consistent again.

OK

>> In looking at the code in qcow2_truncate() it would appear that in the case
>> where prealloc has the value PREALLOC_MODE_OFF, that we don't flush the
>> metadata to disk - which seems to be the case here.
>>
>> If I ignore the if test, and always execute the block in block/qcow2.c,
>> lines 3250 to 3258:
>>
>>    if (prealloc != PREALLOC_MODE_OFF) {
>>        /* Flush metadata before actually changing the image size */
>>        ret = bdrv_flush(bs);
>>        if (ret<  0) {
>>            error_setg_errno(errp, -ret,
>>                             "Failed to flush the preallocated area to disk");
>>            return ret;
>>        }
>>    }
>>
>> causing the flush to always be done, then the check will succeed when
>> the VM is still running.
>>
>> While I know that this resolves the issue, I can only imagine that
>> there was some reason that this check for !PREALLOC_MODE_OFF was being
>> done in the first place.
>>
>> So, I'm hoping that someone here might be able to explain to me why
>> that check is needed, but also why it might be wrong to do the flush
>> regardless of the value of prealloc here.
>
> Doing a flush here wouldn't be wrong, but it's also unnecessary and
> would slow down the operation a bit.
Sure, but how often does a resize/truncate get done? Would seem like a
small impact to do it - but I agree w.r.t. the single-process access
as a better solution.
> For the preallocation case, the goal that is achieved with the flush is
> that the header update with the new image size is only written if the
> preallocation succeeded, so that in error cases you don't end up with an
> image that seems to be resized, but isn't actually preallocated as
> requested.
Understood, thanks.
>> If it is wrong to do that flush here, then would anyone have
>> suggestions as to an alternative solution to this issue?
>
> Don't call 'qemu-img check' (or basically any other operation on the
> image) while a VM is using the image.
Heh, sure - if the locking was working as you describe then I think
I would be fine with that :)

Thanks,

Darren.