Re: [PATCH 1/2] block/raw: added support of persistent dirty bitmaps

[Kevin Wolf]

Am 20.03.2021 um 10:32 hat Patrik Janoušek geschrieben:
> Current implementation of dirty bitmaps for raw format is very
> limited, because those bitmaps cannot be persistent. Basically it
> makes sense, because the raw format doesn't have space where could
> be dirty bitmap stored when QEMU is offline. This patch solves it
> by storing content of every dirty bitmap in separate file on the
> host filesystem.
> 
> However, this only solves one part of the problem. We also have to
> store information about the existence of the dirty bitmap. This is
> solved by adding custom options, that stores all required metadata
> about dirty bitmap (filename where is the bitmap stored on the
> host filesystem, granularity, persistence, etc.).
> 
> Signed-off-by: Patrik Janoušek <pj@patrikjanousek.cz>

I'm not sure if you're going to try to change your thesis to use qcow2
with an external data file, but in case you're still using this patch
for your thesis, let's imagine for a moment that we all agreed on adding
the functionality to raw.

After skimming over the patch, I see two major things that we would ask
to change:

1. You need to change BlockdevOptions to the QAPI schema in
   qapi/block-core.json, because if you don't add new options there,
   they won't be accessible with -blockdev and QMP blockdev-add.

   Among others, this means you would describe RawDirtyBitmapOpts in the
   schema and would get the structure and some helpers automatically
   generated, simplifying your code.

   Instead of processing QemuOpts manually, I would then probably try to
   use the QAPI visitors to get RawDirtyBitmapOpts from the input, which
   could potentially further simplify the code.

2. Instead of having a 'filename': 'str' option and working on the
   bitmap files with stdio.h functions (which block the guest instead of
   allowing asynchronous operation in the background), we would probably
   want something like 'file': 'BlockdevRef', so you get another
   BlockDriverState for each bitmap file that you access with the normal
   QEMU block layer I/O functions.

   The advantage is not only that this isn't blocking, but it also
   allows you to configure more details than just the filename (think
   cache mode, AIO mode, locking, etc.). In fact, it would even allow
   you to store the metadata not in a file, but in any place that can be
   accessed with a protocol supported by QEMU (like NBD, iscsi, ssh,
   whatever).

Kevin