06.03.2019 18:36, Eric Blake wrote:
> On 3/6/19 9:33 AM, Vladimir Sementsov-Ogievskiy wrote:
>
>>> + /* Flush bitmaps */
>>> + if (s->nb_bitmaps) {
>>> + Error *local_err = NULL;
>>> +
>>> + /* Usually, bitmaps get resized after this call.
>>> + * Force it earlier so we can make the metadata consistent. */
>>> + bdrv_dirty_bitmap_truncate(bs, offset);
>>> + qcow2_flush_persistent_dirty_bitmaps(bs, &local_err);
>>> + if (local_err) {
>>> + ret = -EINVAL;
>>> + goto fail;
>>> + }
>>> + }
>>
>> Why to flush after resize? Bitmaps will be IN_USE in the image anyway...
>>
>> Could we implement resize without flush first, it would be one patch +
>> test? And then consider
>> flushing in separate?
>
> What happens with migration if we don't flush the new size to disk, so
> the on-disk format has a different size than the in-memory version?
>
migration works fine, as we store bitmap not on close but on inactivate.
Bitmaps may be migrated in two ways:
1. dirty-bitmaps migration capability enabled: they migrate through migration
channel, RAM to RAM,
BdrvDirtyBitmap to BdrvDirtyBitmap, stored bitmaps are unrelated and they
are marked IN_USE anyway
2. dirty-bitmaps migration capability disabled: they migrate through storage,
so they are stored in
inactivation on source and then loaded in invalidation on target.
A good chunk of information is also in a huge comment inside qcow2_do_open()
function.
Note: we use the following hack for resize, I never tried to send it to the
list:
in qcow2_co_truncated, instead of "Can't resize an image which has bitmaps":
if (s->nb_bitmaps) {
/* FIXME: not loaded bitmaps will be lost */
Error *local_err = NULL;
qcow2_remove_all_persistent_dirty_bitmaps(bs, &local_err);
if (local_err) {
error_propagate(errp, local_err);
return -EINVAL;
}
}
--
Best regards,
Vladimir
