Re: RFC: Qemu backup interface plans

[Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>]

17.05.2021 15:07, Vladimir Sementsov-Ogievskiy wrote:
> 3.1 and make it possible to modify the bitmap externally, so that consumer of fleecing can say to backup-top filter: I've already copied these blocks, don't bother with copying them to temp image". This is to solve [2].
> 
> Still, how consumer of fleecing will reset shared bitmap after copying blocks? I have the following idea: we make a "discard-bitmap-filter" filter driver, that own some bitmap and on discard request unset corresponding bits. Also, on read, if read from the region with unset bits the EINVAL returned immediately. This way both consumers (backup job and NBD client) are able to use this interface:
> 
> Backup job can simply call discard on source, we can add an option for this.
> External backup tool will send TRIM request after reading some region. This way disk space will be freed and no extra copy-before-write operations will be done. I also have a side idea that we can implement READ_ONCE flag, so that READ and TRIM can be done in one NBD command. But this works only for clients that don't want to implement any kind of retrying.
> 
> 
> 
> So, finally, how will it look (here I call backup-top with a new name, and "file" child is used instead of "backing", as this change I propose in "[PATCH 00/21] block: publish backup-top filter"):
> 
> Pull backup:
> 
> ┌────────────────────────────────────┐
> │             NBD export             │
> └────────────────────────────────────┘
>    │
>    │
> ┌────────────────────────────────────┐  file   ┌───────────────────────────────────────┐  backing   ┌─────────────┐
> │ discard-bitmap filter (bitmap=bm0) │ ──────▶ │            temp qcow2 img             │ ─────────▶ │ active disk │
> └────────────────────────────────────┘         └───────────────────────────────────────┘            └─────────────┘
>                                                   ▲                                                    ▲
>                                                   │ target                                             │
>                                                   │                                                    │
> ┌────────────────────────────────────┐         ┌───────────────────────────────────────┐  file        │
> │             guest blk              │ ──────▶ │ copy-before-write filter (bitmap=bm0) │ ─────────────┘
> └────────────────────────────────────┘         └───────────────────────────────────────┘


Now I think that discard-bitmap is not really good idea:

1. If it is separate of internal BlockCopyState::copy_bitmap, than we'll have to consider both bitmaps inside block-copy code, it's not a small complication.

2. Using BlockCopyState::copy_bitmap directly seems possible:

User creates copy_bitmap by hand, and pass to copy-before-write filter as an option, block-copy will use this bitmap directly

Same bitmap is passed to discard-bitmap filter, so that it can clear bits on discards.

Pros:
  - we don't make block-copy code more complicated

Note then, that BlockCopyState::copy_bitmap is used in block-copy process as follows:

  - copy tasks are created to copy dirty areas
  - when task is created, corresponding dirty area is cleared (so that creating tasks on same area can't happen)
  - if task failed, corresponding area becomes dirty again (so that it can be retried)

So, we have
Cons:
  - if discard comes when task is in-flight, bits are already clean. Then if task failed bits become dirty again. That's wrong. Not very bad, and not worth doing coplications of [1]. But still, keep it in mind [*]
  - we have to use separate bitmap in discard-filter to fail reads from non-dirty areas (because BlockCopyState::copy_bitmap is cleared by block-copy process, not only by discards). That is worse.. It just means that discard-filter is strange thing and we don't have good understanding of what should it do. Good documentation will help of course, but...

...this all leads me to new idea:

Let's go without discard-bitmap filter with the simple logic:

If discard is done on block-copy target, let's inform block-copy about it. So, block-copy can:

  - clear corresponding area in its internal bitmap
  - [not really improtant, but it addresses [*]]: cancel in-flight tasks in discarded area.

Pros: avoid extra filter

-- 
Best regards,
Vladimir