Re: [pve-devel] [PATCH v1 pve-qemu 1/1] add block-commit-replaces option patch

["Fabian Grünbichler" <f.gruenbichler@proxmox.com>]

> Alexandre Derumier via pve-devel <pve-devel@lists.proxmox.com> hat am 16.12.2024 10:12 CET geschrieben:

> This is needed for external snapshot live commit,
> when the top blocknode is not the fmt-node.
> (in our case, the throttle-group node is the topnode)

so this is needed to workaround a limitation in block-commit? I think if we need this it should probably be submitted upstream for inclusion, or we provide our own copy of block-commit with it in the meantime?

> 
> Signed-off-by: Alexandre Derumier <alexandre.derumier@groupe-cyllene.com>
> ---
>  ...052-block-commit-add-replaces-option.patch | 137 ++++++++++++++++++
>  debian/patches/series                         |   1 +
>  2 files changed, 138 insertions(+)
>  create mode 100644 debian/patches/pve/0052-block-commit-add-replaces-option.patch
> 
> diff --git a/debian/patches/pve/0052-block-commit-add-replaces-option.patch b/debian/patches/pve/0052-block-commit-add-replaces-option.patch
> new file mode 100644
> index 0000000..2488b5b
> --- /dev/null
> +++ b/debian/patches/pve/0052-block-commit-add-replaces-option.patch
> @@ -0,0 +1,137 @@
> +From ae39fd3bb72db440cf380978af9bf5693c12ac6c Mon Sep 17 00:00:00 2001
> +From: Alexandre Derumier <alexandre.derumier@groupe-cyllene.com>
> +Date: Wed, 11 Dec 2024 16:20:25 +0100
> +Subject: [PATCH] block-commit: add replaces option
> +
> +This use same code than drive-mirror for live commit, but the option
> +is not send currently.
> +
> +Allow to replaces a different node than the root node after the block-commit
> +(as we use throttle-group as root, and not the drive)
> +---
> + block/mirror.c                         | 4 ++--
> + block/replication.c                    | 2 +-
> + blockdev.c                             | 4 ++--
> + include/block/block_int-global-state.h | 4 +++-
> + qapi/block-core.json                   | 5 ++++-
> + qemu-img.c                             | 2 +-
> + 6 files changed, 13 insertions(+), 8 deletions(-)
> +
> +diff --git a/block/mirror.c b/block/mirror.c
> +index 2f12238..1a5e528 100644
> +--- a/block/mirror.c
> ++++ b/block/mirror.c
> +@@ -2086,7 +2086,7 @@ BlockJob *commit_active_start(const char *job_id, BlockDriverState *bs,
> +                               int64_t speed, BlockdevOnError on_error,
> +                               const char *filter_node_name,
> +                               BlockCompletionFunc *cb, void *opaque,
> +-                              bool auto_complete, Error **errp)
> ++                              bool auto_complete, const char *replaces, Error **errp)
> + {
> +     bool base_read_only;
> +     BlockJob *job;
> +@@ -2102,7 +2102,7 @@ BlockJob *commit_active_start(const char *job_id, BlockDriverState *bs,
> +     }
> + 
> +     job = mirror_start_job(
> +-                     job_id, bs, creation_flags, base, NULL, speed, 0, 0,
> ++                     job_id, bs, creation_flags, base, replaces, speed, 0, 0,
> +                      MIRROR_LEAVE_BACKING_CHAIN, false,
> +                      on_error, on_error, true, cb, opaque,
> +                      &commit_active_job_driver, MIRROR_SYNC_MODE_FULL,
> +diff --git a/block/replication.c b/block/replication.c
> +index 0415a5e..debbe25 100644
> +--- a/block/replication.c
> ++++ b/block/replication.c
> +@@ -711,7 +711,7 @@ static void replication_stop(ReplicationState *rs, bool failover, Error **errp)
> +         s->commit_job = commit_active_start(
> +                             NULL, bs->file->bs, s->secondary_disk->bs,
> +                             JOB_INTERNAL, 0, BLOCKDEV_ON_ERROR_REPORT,
> +-                            NULL, replication_done, bs, true, errp);
> ++                            NULL, replication_done, bs, true, NULL, errp);
> +         bdrv_graph_rdunlock_main_loop();
> +         break;
> +     default:
> +diff --git a/blockdev.c b/blockdev.c
> +index cbe2243..349fb71 100644
> +--- a/blockdev.c
> ++++ b/blockdev.c
> +@@ -2435,7 +2435,7 @@ void qmp_block_commit(const char *job_id, const char *device,
> +                       const char *filter_node_name,
> +                       bool has_auto_finalize, bool auto_finalize,
> +                       bool has_auto_dismiss, bool auto_dismiss,
> +-                      Error **errp)
> ++                      const char *replaces, Error **errp)
> + {
> +     BlockDriverState *bs;
> +     BlockDriverState *iter;
> +@@ -2596,7 +2596,7 @@ void qmp_block_commit(const char *job_id, const char *device,
> +             job_id = bdrv_get_device_name(bs);
> +         }
> +         commit_active_start(job_id, top_bs, base_bs, job_flags, speed, on_error,
> +-                            filter_node_name, NULL, NULL, false, &local_err);
> ++                            filter_node_name, NULL, NULL, false, replaces, &local_err);
> +     } else {
> +         BlockDriverState *overlay_bs = bdrv_find_overlay(bs, top_bs);
> +         if (bdrv_op_is_blocked(overlay_bs, BLOCK_OP_TYPE_COMMIT_TARGET, errp)) {
> +diff --git a/include/block/block_int-global-state.h b/include/block/block_int-global-state.h
> +index f0c642b..194b580 100644
> +--- a/include/block/block_int-global-state.h
> ++++ b/include/block/block_int-global-state.h
> +@@ -115,6 +115,7 @@ void commit_start(const char *job_id, BlockDriverState *bs,
> +  * @cb: Completion function for the job.
> +  * @opaque: Opaque pointer value passed to @cb.
> +  * @auto_complete: Auto complete the job.
> ++ * @replaces: Block graph node name to replace once the commit is done.
> +  * @errp: Error object.
> +  *
> +  */
> +@@ -123,7 +124,8 @@ BlockJob *commit_active_start(const char *job_id, BlockDriverState *bs,
> +                               int64_t speed, BlockdevOnError on_error,
> +                               const char *filter_node_name,
> +                               BlockCompletionFunc *cb, void *opaque,
> +-                              bool auto_complete, Error **errp);
> ++                              bool auto_complete, const char *replaces,
> ++                              Error **errp);
> + /*
> +  * mirror_start:
> +  * @job_id: The id of the newly-created job, or %NULL to use the
> +diff --git a/qapi/block-core.json b/qapi/block-core.json
> +index ff441d4..50564c7 100644
> +--- a/qapi/block-core.json
> ++++ b/qapi/block-core.json
> +@@ -2098,6 +2098,8 @@
> + #     disappear from the query list without user intervention.
> + #     Defaults to true.  (Since 3.1)
> + #
> ++# @replaces: graph node name to be replaced base image node.
> ++#
> + # Features:
> + #
> + # @deprecated: Members @base and @top are deprecated.  Use @base-node
> +@@ -2125,7 +2127,8 @@
> +             '*speed': 'int',
> +             '*on-error': 'BlockdevOnError',
> +             '*filter-node-name': 'str',
> +-            '*auto-finalize': 'bool', '*auto-dismiss': 'bool' },
> ++            '*auto-finalize': 'bool', '*auto-dismiss': 'bool',
> ++            '*replaces': 'str' },
> +   'allow-preconfig': true }
> + 
> + ##
> +diff --git a/qemu-img.c b/qemu-img.c
> +index a6c88e0..f6c59bc 100644
> +--- a/qemu-img.c
> ++++ b/qemu-img.c
> +@@ -1079,7 +1079,7 @@ static int img_commit(int argc, char **argv)
> + 
> +     commit_active_start("commit", bs, base_bs, JOB_DEFAULT, rate_limit,
> +                         BLOCKDEV_ON_ERROR_REPORT, NULL, common_block_job_cb,
> +-                        &cbi, false, &local_err);
> ++                        &cbi, false, NULL, &local_err);
> +     if (local_err) {
> +         goto done;
> +     }
> +-- 
> +2.39.5
> +
> diff --git a/debian/patches/series b/debian/patches/series
> index 93c97bf..e604a23 100644
> --- a/debian/patches/series
> +++ b/debian/patches/series
> @@ -92,3 +92,4 @@ pve/0048-PVE-backup-fixup-error-handling-for-fleecing.patch
>  pve/0049-PVE-backup-factor-out-setting-up-snapshot-access-for.patch
>  pve/0050-PVE-backup-save-device-name-in-device-info-structure.patch
>  pve/0051-PVE-backup-include-device-name-in-error-when-setting.patch
> +pve/0052-block-commit-add-replaces-option.patch
> -- 
> 2.39.5


_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel