From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by lists.proxmox.com (Postfix) with ESMTPS id EBCD6942F5 for ; Wed, 10 Apr 2024 13:38:35 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id D3D4BE63F for ; Wed, 10 Apr 2024 13:38:35 +0200 (CEST) Received: from proxmox-new.maurer-it.com (proxmox-new.maurer-it.com [94.136.29.106]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by firstgate.proxmox.com (Proxmox) with ESMTPS for ; Wed, 10 Apr 2024 13:38:35 +0200 (CEST) Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1]) by proxmox-new.maurer-it.com (Proxmox) with ESMTP id 218BC43AB9 for ; Wed, 10 Apr 2024 13:38:35 +0200 (CEST) Date: Wed, 10 Apr 2024 13:38:34 +0200 From: Wolfgang Bumiller To: Fiona Ebner Cc: pve-devel@lists.proxmox.com Message-ID: <2k2aeucv6tx6ruxczfockfplgvlzi7zwm5ag4ktzuob24adaey@vidu2kcw6nvx> References: <20240315102502.84163-1-f.ebner@proxmox.com> <20240315102502.84163-8-f.ebner@proxmox.com> <553f7d33-d739-49d0-a65e-b572d3985da2@proxmox.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <553f7d33-d739-49d0-a65e-b572d3985da2@proxmox.com> X-SPAM-LEVEL: Spam detection results: 0 AWL 0.086 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 Subject: Re: [pve-devel] [PATCH qemu v2 07/21] PVE backup: add fleecing option 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: , X-List-Received-Date: Wed, 10 Apr 2024 11:38:36 -0000 On Wed, Apr 10, 2024 at 11:30:59AM +0200, Fiona Ebner wrote: > Am 08.04.24 um 14:45 schrieb Wolfgang Bumiller: > > On Fri, Mar 15, 2024 at 11:24:48AM +0100, Fiona Ebner wrote: > >> @@ -581,6 +682,14 @@ static void create_backup_jobs_bh(void *opaque) { > >> aio_co_enter(data->ctx, data->co); > >> } > >> > >> +/* > >> + * EFI disk and TPM state are small and it's just not worth setting up fleecing for them. > >> + */ > >> +static bool device_uses_fleecing(const char *device_id) > > > > Do we really want this? > > > > IMO we already have enough code trying to distinguish "real" disks from > > efidisks and tpmstate files. > > > > AFAICT we do check whether the hmp command to *create* the fleecing > > drives actually works, so... (see below) > > > >> +{ > >> + return strncmp(device_id, "drive-efidisk", 13) && strncmp(device_id, "drive-tpmstate", 14); > >> +} > >> + > >> /* > >> * Returns a list of device infos, which needs to be freed by the caller. In > >> * case of an error, errp will be set, but the returned value might still be a > >> @@ -588,6 +697,7 @@ static void create_backup_jobs_bh(void *opaque) { > >> */ > >> static GList coroutine_fn *get_device_info( > >> const char *devlist, > >> + bool fleecing, > >> Error **errp) > >> { > >> gchar **devs = NULL; > >> @@ -611,6 +721,31 @@ static GList coroutine_fn *get_device_info( > >> } > >> PVEBackupDevInfo *di = g_new0(PVEBackupDevInfo, 1); > >> di->bs = bs; > >> + > >> + if (fleecing && device_uses_fleecing(*d)) { > >> + g_autofree gchar *fleecing_devid = g_strconcat(*d, "-fleecing", NULL); > >> + BlockBackend *fleecing_blk = blk_by_name(fleecing_devid); > >> + if (!fleecing_blk) { > > > > ...so instead of this, we could just treat the absence of a fleecing > > BlockBackend *not* as an error, but as deliberate? > > > > Yes, we could. But the check gives protection against potential (future) > bugs where we can't find the fleecing device for some other reason. > Without the check, we'd just quietly continue and it would be hard to > notice that something is wrong. So I'm slightly in favor of keeping it. > If you still want me to remove it, I'll do it in v3. Mh, makes sense. Let's keep it then.