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 D69778487 for ; Mon, 25 Apr 2022 08:37:59 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id CDF4F42E4 for ; Mon, 25 Apr 2022 08:37:59 +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 id 4482A42D8 for ; Mon, 25 Apr 2022 08:37:59 +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 178BC428A4 for ; Mon, 25 Apr 2022 08:37:59 +0200 (CEST) Date: Mon, 25 Apr 2022 08:37:58 +0200 From: Wolfgang Bumiller To: Fabian Ebner Cc: pve-devel@lists.proxmox.com Message-ID: <20220425063758.jfgrpoblou2tern2@olga.proxmox.com> References: <20220421112659.74011-1-f.ebner@proxmox.com> <20220421112659.74011-2-f.ebner@proxmox.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20220421112659.74011-2-f.ebner@proxmox.com> X-SPAM-LEVEL: Spam detection results: 0 AWL 0.328 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% 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 v2 qemu 1/2] vma: restore: call blk_unref for all opened block devices 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: Mon, 25 Apr 2022 06:37:59 -0000 lgtm While looking at the code again, I do find the error handling in `extract_content` generally a bit lacking, though, eg. the `blk` var moved down below is only set if `errp` is not set, but we still go ahead with everything else, making me wonder if we shouldn't bail out then instead... But that's out of scope of this patch, this definitely fixes what appears to be buggy cleanup code. On Thu, Apr 21, 2022 at 01:26:47PM +0200, Fabian Ebner wrote: > rather than just the last one. This is the only caller registering > block devices with the reader, so other callers are already fine. > > Signed-off-by: Fabian Ebner > --- > > New in v2. > > Best squashed into 0026-PVE-Backup-add-vma-backup-format-code.patch > > vma-reader.c | 3 +++ > vma.c | 6 ++---- > 2 files changed, 5 insertions(+), 4 deletions(-) > > diff --git a/vma-reader.c b/vma-reader.c > index 2b1d1cdab3..4f4ee2b47b 100644 > --- a/vma-reader.c > +++ b/vma-reader.c > @@ -192,6 +192,9 @@ void vma_reader_destroy(VmaReader *vmar) > if (vmar->rstate[i].bitmap) { > g_free(vmar->rstate[i].bitmap); > } > + if (vmar->rstate[i].target) { > + blk_unref(vmar->rstate[i].target); > + } > } > > if (vmar->md5csum) { > diff --git a/vma.c b/vma.c > index df542b7732..89440733b1 100644 > --- a/vma.c > +++ b/vma.c > @@ -309,8 +309,6 @@ static int extract_content(int argc, char **argv) > int vmstate_fd = -1; > guint8 vmstate_stream = 0; > > - BlockBackend *blk = NULL; > - > for (i = 1; i < 255; i++) { > VmaDeviceInfo *di = vma_reader_get_device_info(vmar, i); > if (di && (strcmp(di->devname, "vmstate") == 0)) { > @@ -331,6 +329,8 @@ static int extract_content(int argc, char **argv) > int flags = BDRV_O_RDWR; > bool write_zero = true; > > + BlockBackend *blk = NULL; > + > if (readmap) { > RestoreMap *map; > map = (RestoreMap *)g_hash_table_lookup(devmap, di->devname); > @@ -443,8 +443,6 @@ static int extract_content(int argc, char **argv) > > vma_reader_destroy(vmar); > > - blk_unref(blk); > - > bdrv_close_all(); > > return ret; > -- > 2.30.2