From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) by lore.proxmox.com (Postfix) with ESMTPS id B09A61FF2B0 for ; Fri, 5 Jul 2024 11:30:40 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id DD0973F187; Fri, 5 Jul 2024 11:30:57 +0200 (CEST) MIME-Version: 1.0 In-Reply-To: <20240610125942.116985-4-f.ebner@proxmox.com> References: <20240610125942.116985-1-f.ebner@proxmox.com> <20240610125942.116985-4-f.ebner@proxmox.com> From: Fabian =?utf-8?q?Gr=C3=BCnbichler?= To: Proxmox VE development discussion Date: Fri, 05 Jul 2024 11:30:48 +0200 Message-ID: <172017184865.125161.8672737858177971283@yuna.proxmox.com> User-Agent: alot/0.10 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.051 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record URIBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to URIBL was blocked. See http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more information. [proxmox.com] Subject: Re: [pve-devel] [RFC qemu 3/7] block/backup: allow passing additional options for copy-before-write upon job creation X-BeenThere: pve-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox VE development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: Proxmox VE development discussion Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: pve-devel-bounces@lists.proxmox.com Sender: "pve-devel" Quoting Fiona Ebner (2024-06-10 14:59:38) > In particular, useful for setting the 'on-cbw-error' and 'cbw-timeout' > options (see BlockdevOptionsCbw in QAPI). > > Signed-off-by: Fiona Ebner > --- > block/backup.c | 10 +++++++--- > block/replication.c | 2 +- > blockdev.c | 2 +- > include/block/block_int-global-state.h | 2 ++ > pve-backup.c | 2 +- > 5 files changed, 12 insertions(+), 6 deletions(-) > > diff --git a/block/backup.c b/block/backup.c > index e0acfe6758..82fedf1680 100644 > --- a/block/backup.c > +++ b/block/backup.c > @@ -336,6 +336,7 @@ BlockJob *backup_job_create(const char *job_id, BlockDriverState *bs, > bool compress, bool discard_source, > const char *filter_node_name, > BackupPerf *perf, > + QDict *cbw_opts, > BlockdevOnError on_source_error, > BlockdevOnError on_target_error, > int creation_flags, > @@ -347,7 +348,6 @@ BlockJob *backup_job_create(const char *job_id, BlockDriverState *bs, > int64_t cluster_size; > BlockDriverState *cbw = NULL; > BlockCopyState *bcs = NULL; > - QDict *cbw_opts = NULL; > > assert(bs); > assert(target); > @@ -436,8 +436,12 @@ BlockJob *backup_job_create(const char *job_id, BlockDriverState *bs, > } > > if (perf->has_min_cluster_size) { > - cbw_opts = qdict_new(); > - qdict_put_int(cbw_opts, "min-cluster-size", perf->min_cluster_size); > + if (!cbw_opts) { > + cbw_opts = qdict_new(); > + } > + if (!qdict_haskey(cbw_opts, "min-cluster-size")) { > + qdict_put_int(cbw_opts, "min-cluster-size", perf->min_cluster_size); so here cbw_opts "wins" without any indication that this happens > + } > } > > cbw = bdrv_cbw_append(bs, target, filter_node_name, discard_source, > diff --git a/block/replication.c b/block/replication.c > index 0415a5e8b7..c5a27f593e 100644 > --- a/block/replication.c > +++ b/block/replication.c > @@ -583,7 +583,7 @@ static void replication_start(ReplicationState *rs, ReplicationMode mode, > s->backup_job = backup_job_create( > NULL, s->secondary_disk->bs, s->hidden_disk->bs, > 0, MIRROR_SYNC_MODE_NONE, NULL, 0, false, false, > - NULL, &perf, > + NULL, &perf, NULL, > BLOCKDEV_ON_ERROR_REPORT, > BLOCKDEV_ON_ERROR_REPORT, JOB_INTERNAL, > backup_job_completed, bs, NULL, &local_err); > diff --git a/blockdev.c b/blockdev.c > index cbe224387b..55ca967430 100644 > --- a/blockdev.c > +++ b/blockdev.c > @@ -2732,7 +2732,7 @@ static BlockJob *do_backup_common(BackupCommon *backup, > backup->sync, bmap, backup->bitmap_mode, > backup->compress, backup->discard_source, > backup->filter_node_name, > - &perf, > + &perf, NULL, > backup->on_source_error, > backup->on_target_error, > job_flags, NULL, NULL, txn, errp); > diff --git a/include/block/block_int-global-state.h b/include/block/block_int-global-state.h > index f0c642b194..7332680087 100644 > --- a/include/block/block_int-global-state.h > +++ b/include/block/block_int-global-state.h > @@ -179,6 +179,7 @@ void mirror_start(const char *job_id, BlockDriverState *bs, > * @bitmap_mode: The bitmap synchronization policy to use. > * @perf: Performance options. All actual fields assumed to be present, > * all ".has_*" fields are ignored. > + * @cbw_opts: Additional options to configure cbw filter with. from this, I'd have guessed the other way round ;) should the precedence be made explicit somewhere? or, for this particular option, should the higher value win? but such logic might quickly get complicated once more parameters need to be handled.. > * @on_source_error: The action to take upon error reading from the source. > * @on_target_error: The action to take upon error writing to the target. > * @creation_flags: Flags that control the behavior of the Job lifetime. > @@ -198,6 +199,7 @@ BlockJob *backup_job_create(const char *job_id, BlockDriverState *bs, > bool compress, bool discard_source, > const char *filter_node_name, > BackupPerf *perf, > + QDict *cbw_opts, > BlockdevOnError on_source_error, > BlockdevOnError on_target_error, > int creation_flags, > diff --git a/pve-backup.c b/pve-backup.c > index 4e80a9f283..108e185a20 100644 > --- a/pve-backup.c > +++ b/pve-backup.c > @@ -635,7 +635,7 @@ static void create_backup_jobs_bh(void *opaque) { > > BlockJob *job = backup_job_create( > job_id, source_bs, di->target, backup_state.speed, sync_mode, di->bitmap, > - bitmap_mode, false, discard_source, NULL, &perf, BLOCKDEV_ON_ERROR_REPORT, > + bitmap_mode, false, discard_source, NULL, &perf, NULL, BLOCKDEV_ON_ERROR_REPORT, > BLOCKDEV_ON_ERROR_REPORT, JOB_DEFAULT, pvebackup_complete_cb, di, backup_state.txn, > &local_err); > > -- > 2.39.2 > > > > _______________________________________________ > pve-devel mailing list > pve-devel@lists.proxmox.com > https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel > > _______________________________________________ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel