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 6406292254 for ; Fri, 6 Oct 2023 13:02:30 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 5585C32AD7 for ; Fri, 6 Oct 2023 13:01: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 for ; Fri, 6 Oct 2023 13:01:57 +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 7B3B148F10 for ; Fri, 6 Oct 2023 13:01:54 +0200 (CEST) From: Fiona Ebner To: pve-devel@lists.proxmox.com Date: Fri, 6 Oct 2023 13:01:47 +0200 Message-Id: <20231006110148.154914-9-f.ebner@proxmox.com> X-Mailer: git-send-email 2.39.2 In-Reply-To: <20231006110148.154914-1-f.ebner@proxmox.com> References: <20231006110148.154914-1-f.ebner@proxmox.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-SPAM-LEVEL: Spam detection results: 0 AWL -0.207 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 KAM_LOTSOFHASH 0.25 Emails with lots of hash-like gibberish SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Subject: [pve-devel] [PATCH v2 qemu 8/9] cherry-pick stable fixes to avoid crash in IO error scenarios 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: Fri, 06 Oct 2023 11:02:30 -0000 Upstream report of the issue [0]. I ran into it too by chance by filling up my NFS storage with the VM's qcow2 disk. [0]: https://bugzilla.redhat.com/show_bug.cgi?id=2234374 Signed-off-by: Fiona Ebner --- No changes in v2. ...ile-posix-Clear-bs-bl.zoned-on-error.patch | 87 +++++++++++++++++++ ...osix-Check-bs-bl.zoned-for-zone-info.patch | 59 +++++++++++++ ...ix-Fix-zone-update-in-I-O-error-path.patch | 33 +++++++ ...-Simplify-raw_co_prw-s-out-zone-code.patch | 58 +++++++++++++ ...k-file-change-locking-default-to-off.patch | 2 +- ...le-posix-make-locking-optiono-on-cre.patch | 14 +-- debian/patches/series | 4 + 7 files changed, 249 insertions(+), 8 deletions(-) create mode 100644 debian/patches/extra/0008-file-posix-Clear-bs-bl.zoned-on-error.patch create mode 100644 debian/patches/extra/0009-file-posix-Check-bs-bl.zoned-for-zone-info.patch create mode 100644 debian/patches/extra/0010-file-posix-Fix-zone-update-in-I-O-error-path.patch create mode 100644 debian/patches/extra/0011-file-posix-Simplify-raw_co_prw-s-out-zone-code.patch diff --git a/debian/patches/extra/0008-file-posix-Clear-bs-bl.zoned-on-error.patch b/debian/patches/extra/0008-file-posix-Clear-bs-bl.zoned-on-error.patch new file mode 100644 index 0000000..0e3dc3e --- /dev/null +++ b/debian/patches/extra/0008-file-posix-Clear-bs-bl.zoned-on-error.patch @@ -0,0 +1,87 @@ +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 +From: Hanna Czenczek +Date: Thu, 24 Aug 2023 17:53:40 +0200 +Subject: [PATCH] file-posix: Clear bs->bl.zoned on error + +bs->bl.zoned is what indicates whether the zone information is present +and valid; it is the only thing that raw_refresh_zoned_limits() sets if +CONFIG_BLKZONED is not defined, and it is also the only thing that it +sets if CONFIG_BLKZONED is defined, but there are no zones. + +Make sure that it is always set to BLK_Z_NONE if there is an error +anywhere in raw_refresh_zoned_limits() so that we do not accidentally +announce zones while our information is incomplete or invalid. + +This also fixes a memory leak in the last error path in +raw_refresh_zoned_limits(). + +Signed-off-by: Hanna Czenczek +Message-Id: <20230824155345.109765-2-hreitz@redhat.com> +Reviewed-by: Sam Li +(cherry-picked from commit 56d1a022a77ea2125564913665eeadf3e303a671) +Signed-off-by: Fiona Ebner +--- + block/file-posix.c | 21 ++++++++++++--------- + 1 file changed, 12 insertions(+), 9 deletions(-) + +diff --git a/block/file-posix.c b/block/file-posix.c +index b16e9c21a1..2b88b9eefa 100644 +--- a/block/file-posix.c ++++ b/block/file-posix.c +@@ -1412,11 +1412,9 @@ static void raw_refresh_zoned_limits(BlockDriverState *bs, struct stat *st, + BlockZoneModel zoned; + int ret; + +- bs->bl.zoned = BLK_Z_NONE; +- + ret = get_sysfs_zoned_model(st, &zoned); + if (ret < 0 || zoned == BLK_Z_NONE) { +- return; ++ goto no_zoned; + } + bs->bl.zoned = zoned; + +@@ -1437,10 +1435,10 @@ static void raw_refresh_zoned_limits(BlockDriverState *bs, struct stat *st, + if (ret < 0) { + error_setg_errno(errp, -ret, "Unable to read chunk_sectors " + "sysfs attribute"); +- return; ++ goto no_zoned; + } else if (!ret) { + error_setg(errp, "Read 0 from chunk_sectors sysfs attribute"); +- return; ++ goto no_zoned; + } + bs->bl.zone_size = ret << BDRV_SECTOR_BITS; + +@@ -1448,10 +1446,10 @@ static void raw_refresh_zoned_limits(BlockDriverState *bs, struct stat *st, + if (ret < 0) { + error_setg_errno(errp, -ret, "Unable to read nr_zones " + "sysfs attribute"); +- return; ++ goto no_zoned; + } else if (!ret) { + error_setg(errp, "Read 0 from nr_zones sysfs attribute"); +- return; ++ goto no_zoned; + } + bs->bl.nr_zones = ret; + +@@ -1472,10 +1470,15 @@ static void raw_refresh_zoned_limits(BlockDriverState *bs, struct stat *st, + ret = get_zones_wp(bs, s->fd, 0, bs->bl.nr_zones, 0); + if (ret < 0) { + error_setg_errno(errp, -ret, "report wps failed"); +- bs->wps = NULL; +- return; ++ goto no_zoned; + } + qemu_co_mutex_init(&bs->wps->colock); ++ return; ++ ++no_zoned: ++ bs->bl.zoned = BLK_Z_NONE; ++ g_free(bs->wps); ++ bs->wps = NULL; + } + #else /* !defined(CONFIG_BLKZONED) */ + static void raw_refresh_zoned_limits(BlockDriverState *bs, struct stat *st, diff --git a/debian/patches/extra/0009-file-posix-Check-bs-bl.zoned-for-zone-info.patch b/debian/patches/extra/0009-file-posix-Check-bs-bl.zoned-for-zone-info.patch new file mode 100644 index 0000000..a505816 --- /dev/null +++ b/debian/patches/extra/0009-file-posix-Check-bs-bl.zoned-for-zone-info.patch @@ -0,0 +1,59 @@ +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 +From: Hanna Czenczek +Date: Thu, 24 Aug 2023 17:53:41 +0200 +Subject: [PATCH] file-posix: Check bs->bl.zoned for zone info + +Instead of checking bs->wps or bs->bl.zone_size for whether zone +information is present, check bs->bl.zoned. That is the flag that +raw_refresh_zoned_limits() reliably sets to indicate zone support. If +it is set to something other than BLK_Z_NONE, other values and objects +like bs->wps and bs->bl.zone_size must be non-null/zero and valid; if it +is not, we cannot rely on their validity. + +Signed-off-by: Hanna Czenczek +Message-Id: <20230824155345.109765-3-hreitz@redhat.com> +Reviewed-by: Sam Li +(cherry-picked from commit 4b5d80f3d02096a9bb1f651f6b3401ba40877159) +Signed-off-by: Fiona Ebner +--- + block/file-posix.c | 12 +++++++----- + 1 file changed, 7 insertions(+), 5 deletions(-) + +diff --git a/block/file-posix.c b/block/file-posix.c +index 2b88b9eefa..46e22403fe 100644 +--- a/block/file-posix.c ++++ b/block/file-posix.c +@@ -2455,9 +2455,10 @@ static int coroutine_fn raw_co_prw(BlockDriverState *bs, uint64_t offset, + if (fd_open(bs) < 0) + return -EIO; + #if defined(CONFIG_BLKZONED) +- if ((type & (QEMU_AIO_WRITE | QEMU_AIO_ZONE_APPEND)) && bs->wps) { ++ if ((type & (QEMU_AIO_WRITE | QEMU_AIO_ZONE_APPEND)) && ++ bs->bl.zoned != BLK_Z_NONE) { + qemu_co_mutex_lock(&bs->wps->colock); +- if (type & QEMU_AIO_ZONE_APPEND && bs->bl.zone_size) { ++ if (type & QEMU_AIO_ZONE_APPEND) { + int index = offset / bs->bl.zone_size; + offset = bs->wps->wp[index]; + } +@@ -2508,8 +2509,8 @@ out: + { + BlockZoneWps *wps = bs->wps; + if (ret == 0) { +- if ((type & (QEMU_AIO_WRITE | QEMU_AIO_ZONE_APPEND)) +- && wps && bs->bl.zone_size) { ++ if ((type & (QEMU_AIO_WRITE | QEMU_AIO_ZONE_APPEND)) && ++ bs->bl.zoned != BLK_Z_NONE) { + uint64_t *wp = &wps->wp[offset / bs->bl.zone_size]; + if (!BDRV_ZT_IS_CONV(*wp)) { + if (type & QEMU_AIO_ZONE_APPEND) { +@@ -2529,7 +2530,8 @@ out: + } + } + +- if ((type & (QEMU_AIO_WRITE | QEMU_AIO_ZONE_APPEND)) && wps) { ++ if ((type & (QEMU_AIO_WRITE | QEMU_AIO_ZONE_APPEND)) && ++ bs->blk.zoned != BLK_Z_NONE) { + qemu_co_mutex_unlock(&wps->colock); + } + } diff --git a/debian/patches/extra/0010-file-posix-Fix-zone-update-in-I-O-error-path.patch b/debian/patches/extra/0010-file-posix-Fix-zone-update-in-I-O-error-path.patch new file mode 100644 index 0000000..fb32c62 --- /dev/null +++ b/debian/patches/extra/0010-file-posix-Fix-zone-update-in-I-O-error-path.patch @@ -0,0 +1,33 @@ +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 +From: Hanna Czenczek +Date: Thu, 24 Aug 2023 17:53:42 +0200 +Subject: [PATCH] file-posix: Fix zone update in I/O error path + +We must check that zone information is present before running +update_zones_wp(). + +Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=2234374 +Fixes: Coverity CID 1512459 +Signed-off-by: Hanna Czenczek +Message-Id: <20230824155345.109765-4-hreitz@redhat.com> +Reviewed-by: Sam Li +(cherry-picked from commit deab5c9a4ed74f76a713008a42527762b30a7e84) +Signed-off-by: Fiona Ebner +--- + block/file-posix.c | 3 ++- + 1 file changed, 2 insertions(+), 1 deletion(-) + +diff --git a/block/file-posix.c b/block/file-posix.c +index 46e22403fe..a050682e97 100644 +--- a/block/file-posix.c ++++ b/block/file-posix.c +@@ -2525,7 +2525,8 @@ out: + } + } + } else { +- if (type & (QEMU_AIO_WRITE | QEMU_AIO_ZONE_APPEND)) { ++ if ((type & (QEMU_AIO_WRITE | QEMU_AIO_ZONE_APPEND)) && ++ bs->bl.zoned != BLK_Z_NONE) { + update_zones_wp(bs, s->fd, 0, 1); + } + } diff --git a/debian/patches/extra/0011-file-posix-Simplify-raw_co_prw-s-out-zone-code.patch b/debian/patches/extra/0011-file-posix-Simplify-raw_co_prw-s-out-zone-code.patch new file mode 100644 index 0000000..6164160 --- /dev/null +++ b/debian/patches/extra/0011-file-posix-Simplify-raw_co_prw-s-out-zone-code.patch @@ -0,0 +1,58 @@ +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 +From: Hanna Czenczek +Date: Thu, 24 Aug 2023 17:53:43 +0200 +Subject: [PATCH] file-posix: Simplify raw_co_prw's 'out' zone code + +We duplicate the same condition three times here, pull it out to the top +level. + +Signed-off-by: Hanna Czenczek +Message-Id: <20230824155345.109765-5-hreitz@redhat.com> +Reviewed-by: Sam Li +(cherry-picked from commit d31b50a15dd25a560749b25fc40b6484fd1a57b7) +Signed-off-by: Fiona Ebner +--- + block/file-posix.c | 18 +++++------------- + 1 file changed, 5 insertions(+), 13 deletions(-) + +diff --git a/block/file-posix.c b/block/file-posix.c +index a050682e97..aa89789737 100644 +--- a/block/file-posix.c ++++ b/block/file-posix.c +@@ -2506,11 +2506,10 @@ static int coroutine_fn raw_co_prw(BlockDriverState *bs, uint64_t offset, + + out: + #if defined(CONFIG_BLKZONED) +-{ +- BlockZoneWps *wps = bs->wps; +- if (ret == 0) { +- if ((type & (QEMU_AIO_WRITE | QEMU_AIO_ZONE_APPEND)) && +- bs->bl.zoned != BLK_Z_NONE) { ++ if ((type & (QEMU_AIO_WRITE | QEMU_AIO_ZONE_APPEND)) && ++ bs->bl.zoned != BLK_Z_NONE) { ++ BlockZoneWps *wps = bs->wps; ++ if (ret == 0) { + uint64_t *wp = &wps->wp[offset / bs->bl.zone_size]; + if (!BDRV_ZT_IS_CONV(*wp)) { + if (type & QEMU_AIO_ZONE_APPEND) { +@@ -2523,19 +2522,12 @@ out: + *wp = offset + bytes; + } + } +- } +- } else { +- if ((type & (QEMU_AIO_WRITE | QEMU_AIO_ZONE_APPEND)) && +- bs->bl.zoned != BLK_Z_NONE) { ++ } else { + update_zones_wp(bs, s->fd, 0, 1); + } +- } + +- if ((type & (QEMU_AIO_WRITE | QEMU_AIO_ZONE_APPEND)) && +- bs->blk.zoned != BLK_Z_NONE) { + qemu_co_mutex_unlock(&wps->colock); + } +-} + #endif + return ret; + } diff --git a/debian/patches/pve/0001-PVE-Config-block-file-change-locking-default-to-off.patch b/debian/patches/pve/0001-PVE-Config-block-file-change-locking-default-to-off.patch index 1c3d08c..3d8785c 100644 --- a/debian/patches/pve/0001-PVE-Config-block-file-change-locking-default-to-off.patch +++ b/debian/patches/pve/0001-PVE-Config-block-file-change-locking-default-to-off.patch @@ -14,7 +14,7 @@ Signed-off-by: Thomas Lamprecht 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/block/file-posix.c b/block/file-posix.c -index b16e9c21a1..26aa0172b4 100644 +index aa89789737..0db366a851 100644 --- a/block/file-posix.c +++ b/block/file-posix.c @@ -564,7 +564,7 @@ static QemuOptsList raw_runtime_opts = { diff --git a/debian/patches/pve/0022-PVE-Up-Config-file-posix-make-locking-optiono-on-cre.patch b/debian/patches/pve/0022-PVE-Up-Config-file-posix-make-locking-optiono-on-cre.patch index 904f76e..766c4f9 100644 --- a/debian/patches/pve/0022-PVE-Up-Config-file-posix-make-locking-optiono-on-cre.patch +++ b/debian/patches/pve/0022-PVE-Up-Config-file-posix-make-locking-optiono-on-cre.patch @@ -13,10 +13,10 @@ Signed-off-by: Thomas Lamprecht 2 files changed, 42 insertions(+), 20 deletions(-) diff --git a/block/file-posix.c b/block/file-posix.c -index 26aa0172b4..504c6ed316 100644 +index 0db366a851..46f1ee38ae 100644 --- a/block/file-posix.c +++ b/block/file-posix.c -@@ -2872,6 +2872,7 @@ raw_co_create(BlockdevCreateOptions *options, Error **errp) +@@ -2870,6 +2870,7 @@ raw_co_create(BlockdevCreateOptions *options, Error **errp) int fd; uint64_t perm, shared; int result = 0; @@ -24,7 +24,7 @@ index 26aa0172b4..504c6ed316 100644 /* Validate options and set default values */ assert(options->driver == BLOCKDEV_DRIVER_FILE); -@@ -2912,19 +2913,22 @@ raw_co_create(BlockdevCreateOptions *options, Error **errp) +@@ -2910,19 +2911,22 @@ raw_co_create(BlockdevCreateOptions *options, Error **errp) perm = BLK_PERM_WRITE | BLK_PERM_RESIZE; shared = BLK_PERM_ALL & ~BLK_PERM_RESIZE; @@ -59,7 +59,7 @@ index 26aa0172b4..504c6ed316 100644 } /* Clear the file by truncating it to 0 */ -@@ -2978,13 +2982,15 @@ raw_co_create(BlockdevCreateOptions *options, Error **errp) +@@ -2976,13 +2980,15 @@ raw_co_create(BlockdevCreateOptions *options, Error **errp) } out_unlock: @@ -82,7 +82,7 @@ index 26aa0172b4..504c6ed316 100644 } out_close: -@@ -3008,6 +3014,7 @@ raw_co_create_opts(BlockDriver *drv, const char *filename, +@@ -3006,6 +3012,7 @@ raw_co_create_opts(BlockDriver *drv, const char *filename, PreallocMode prealloc; char *buf = NULL; Error *local_err = NULL; @@ -90,7 +90,7 @@ index 26aa0172b4..504c6ed316 100644 /* Skip file: protocol prefix */ strstart(filename, "file:", &filename); -@@ -3030,6 +3037,18 @@ raw_co_create_opts(BlockDriver *drv, const char *filename, +@@ -3028,6 +3035,18 @@ raw_co_create_opts(BlockDriver *drv, const char *filename, return -EINVAL; } @@ -109,7 +109,7 @@ index 26aa0172b4..504c6ed316 100644 options = (BlockdevCreateOptions) { .driver = BLOCKDEV_DRIVER_FILE, .u.file = { -@@ -3041,6 +3060,8 @@ raw_co_create_opts(BlockDriver *drv, const char *filename, +@@ -3039,6 +3058,8 @@ raw_co_create_opts(BlockDriver *drv, const char *filename, .nocow = nocow, .has_extent_size_hint = has_extent_size_hint, .extent_size_hint = extent_size_hint, diff --git a/debian/patches/series b/debian/patches/series index c27c245..71f7e01 100644 --- a/debian/patches/series +++ b/debian/patches/series @@ -5,6 +5,10 @@ extra/0004-migration-block-dirty-bitmap-fix-loading-bitmap-when.patch extra/0005-hw-ide-reset-cancel-async-DMA-operation-before-reset.patch extra/0006-Revert-Revert-graph-lock-Disable-locking-for-now.patch extra/0007-migration-states-workaround-snapshot-performance-reg.patch +extra/0008-file-posix-Clear-bs-bl.zoned-on-error.patch +extra/0009-file-posix-Check-bs-bl.zoned-for-zone-info.patch +extra/0010-file-posix-Fix-zone-update-in-I-O-error-path.patch +extra/0011-file-posix-Simplify-raw_co_prw-s-out-zone-code.patch bitmap-mirror/0001-drive-mirror-add-support-for-sync-bitmap-mode-never.patch bitmap-mirror/0002-drive-mirror-add-support-for-conditional-and-always-.patch bitmap-mirror/0003-mirror-add-check-for-bitmap-mode-without-bitmap.patch -- 2.39.2