mbox series

[v3,0/2] backup: allow specifying minimum cluster size

Message ID 20240711120915.310243-1-f.ebner@proxmox.com (mailing list archive)
Headers show
Series backup: allow specifying minimum cluster size | expand

Message

Fiona Ebner July 11, 2024, 12:09 p.m. UTC
Discussion for v2:
https://lore.kernel.org/qemu-devel/20240528120114.344416-1-f.ebner@proxmox.com/

Changes in v3:
* Pass min_cluster_size option directly without checking
  has_min_cluster_size, because the default is 0 anyways.
* Calculate maximum of passed-in argument and default once at the
  beginning of block_copy_calculate_cluster_size()
* Update warning message to reflect actual value used
* Do not leak qdict in error case
* Use PRI{i,u}64 macros

Discussion for v1:
https://lore.kernel.org/qemu-devel/20240308155158.830258-1-f.ebner@proxmox.com/
-
Changes in v2:
* Use 'size' type in QAPI.
* Remove option in cbw_parse_options(), i.e. before parsing generic
  blockdev options.
* Reword commit messages hoping to describe the issue in a more
  straight-forward way.

In the context of backup fleecing, discarding the source will not work
when the fleecing image has a larger granularity than the one used for
block-copy operations (can happen if the backup target has smaller
cluster size), because cbw_co_pdiscard_snapshot() will align down the
discard requests and thus effectively ignore then.

To make @discard-source work in such a scenario, allow specifying the
minimum cluster size used for block-copy operations and thus in
particular also the granularity for discard requests to the source.

Fiona Ebner (2):
  copy-before-write: allow specifying minimum cluster size
  backup: add minimum cluster size to performance options

 block/backup.c             |  2 +-
 block/block-copy.c         | 36 ++++++++++++++++++++++++++----------
 block/copy-before-write.c  | 14 +++++++++++++-
 block/copy-before-write.h  |  1 +
 blockdev.c                 |  3 +++
 include/block/block-copy.h |  1 +
 qapi/block-core.json       | 17 ++++++++++++++---
 7 files changed, 59 insertions(+), 15 deletions(-)

Comments

Vladimir Sementsov-Ogievskiy July 12, 2024, 1:02 p.m. UTC | #1
On 11.07.24 15:09, Fiona Ebner wrote:
> Discussion for v2:
> https://lore.kernel.org/qemu-devel/20240528120114.344416-1-f.ebner@proxmox.com/
> 
> Changes in v3:
> * Pass min_cluster_size option directly without checking
>    has_min_cluster_size, because the default is 0 anyways.
> * Calculate maximum of passed-in argument and default once at the
>    beginning of block_copy_calculate_cluster_size()
> * Update warning message to reflect actual value used
> * Do not leak qdict in error case
> * Use PRI{i,u}64 macros
> 
> Discussion for v1:
> https://lore.kernel.org/qemu-devel/20240308155158.830258-1-f.ebner@proxmox.com/
> -
> Changes in v2:
> * Use 'size' type in QAPI.
> * Remove option in cbw_parse_options(), i.e. before parsing generic
>    blockdev options.
> * Reword commit messages hoping to describe the issue in a more
>    straight-forward way.
> 
> In the context of backup fleecing, discarding the source will not work
> when the fleecing image has a larger granularity than the one used for
> block-copy operations (can happen if the backup target has smaller
> cluster size), because cbw_co_pdiscard_snapshot() will align down the
> discard requests and thus effectively ignore then.
> 
> To make @discard-source work in such a scenario, allow specifying the
> minimum cluster size used for block-copy operations and thus in
> particular also the granularity for discard requests to the source.
> 
> Fiona Ebner (2):
>    copy-before-write: allow specifying minimum cluster size
>    backup: add minimum cluster size to performance options
> 
>   block/backup.c             |  2 +-
>   block/block-copy.c         | 36 ++++++++++++++++++++++++++----------
>   block/copy-before-write.c  | 14 +++++++++++++-
>   block/copy-before-write.h  |  1 +
>   blockdev.c                 |  3 +++
>   include/block/block-copy.h |  1 +
>   qapi/block-core.json       | 17 ++++++++++++++---
>   7 files changed, 59 insertions(+), 15 deletions(-)
> 

Thanks, applied to my block branch.