* [PULL 1/4] file-posix: probe discard alignment on Linux block devices
2025-04-25 17:52 [PULL 0/4] Block layer patches Kevin Wolf
@ 2025-04-25 17:52 ` Kevin Wolf
2025-04-25 17:52 ` [PULL 2/4] block/io: skip head/tail requests on EINVAL Kevin Wolf
` (3 subsequent siblings)
4 siblings, 0 replies; 11+ messages in thread
From: Kevin Wolf @ 2025-04-25 17:52 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, qemu-devel
From: Stefan Hajnoczi <stefanha@redhat.com>
Populate the pdiscard_alignment block limit so the block layer is able
align discard requests correctly.
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-ID: <20250417150528.76470-2-stefanha@redhat.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
block/file-posix.c | 67 +++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 66 insertions(+), 1 deletion(-)
diff --git a/block/file-posix.c b/block/file-posix.c
index 56d1972d15..0d6e12f880 100644
--- a/block/file-posix.c
+++ b/block/file-posix.c
@@ -1276,10 +1276,10 @@ static int get_sysfs_zoned_model(struct stat *st, BlockZoneModel *zoned)
}
#endif /* defined(CONFIG_BLKZONED) */
+#ifdef CONFIG_LINUX
/*
* Get a sysfs attribute value as a long integer.
*/
-#ifdef CONFIG_LINUX
static long get_sysfs_long_val(struct stat *st, const char *attribute)
{
g_autofree char *str = NULL;
@@ -1299,6 +1299,30 @@ static long get_sysfs_long_val(struct stat *st, const char *attribute)
}
return ret;
}
+
+/*
+ * Get a sysfs attribute value as a uint32_t.
+ */
+static int get_sysfs_u32_val(struct stat *st, const char *attribute,
+ uint32_t *u32)
+{
+ g_autofree char *str = NULL;
+ const char *end;
+ unsigned int val;
+ int ret;
+
+ ret = get_sysfs_str_val(st, attribute, &str);
+ if (ret < 0) {
+ return ret;
+ }
+
+ /* The file is ended with '\n', pass 'end' to accept that. */
+ ret = qemu_strtoui(str, &end, 10, &val);
+ if (ret == 0 && end && *end == '\0') {
+ *u32 = val;
+ }
+ return ret;
+}
#endif
static int hdev_get_max_segments(int fd, struct stat *st)
@@ -1318,6 +1342,23 @@ static int hdev_get_max_segments(int fd, struct stat *st)
#endif
}
+/*
+ * Fills in *dalign with the discard alignment and returns 0 on success,
+ * -errno otherwise.
+ */
+static int hdev_get_pdiscard_alignment(struct stat *st, uint32_t *dalign)
+{
+#ifdef CONFIG_LINUX
+ /*
+ * Note that Linux "discard_granularity" is QEMU "discard_alignment". Linux
+ * "discard_alignment" is something else.
+ */
+ return get_sysfs_u32_val(st, "discard_granularity", dalign);
+#else
+ return -ENOTSUP;
+#endif
+}
+
#if defined(CONFIG_BLKZONED)
/*
* If the reset_all flag is true, then the wps of zone whose state is
@@ -1527,6 +1568,30 @@ static void raw_refresh_limits(BlockDriverState *bs, Error **errp)
}
}
+ if (S_ISBLK(st.st_mode)) {
+ uint32_t dalign = 0;
+ int ret;
+
+ ret = hdev_get_pdiscard_alignment(&st, &dalign);
+ if (ret == 0) {
+ uint32_t ralign = bs->bl.request_alignment;
+
+ /* Probably never happens, but handle it just in case */
+ if (dalign < ralign && (ralign % dalign == 0)) {
+ dalign = ralign;
+ }
+
+ /* The block layer requires a multiple of request_alignment */
+ if (dalign % ralign != 0) {
+ error_setg(errp, "Invalid pdiscard_alignment limit %u is not a "
+ "multiple of request_alignment %u", dalign, ralign);
+ return;
+ }
+
+ bs->bl.pdiscard_alignment = dalign;
+ }
+ }
+
raw_refresh_zoned_limits(bs, &st, errp);
}
--
2.49.0
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PULL 2/4] block/io: skip head/tail requests on EINVAL
2025-04-25 17:52 [PULL 0/4] Block layer patches Kevin Wolf
2025-04-25 17:52 ` [PULL 1/4] file-posix: probe discard alignment on Linux block devices Kevin Wolf
@ 2025-04-25 17:52 ` Kevin Wolf
2025-04-25 17:52 ` [PULL 3/4] block: Remove unused callback function *bdrv_aio_pdiscard Kevin Wolf
` (2 subsequent siblings)
4 siblings, 0 replies; 11+ messages in thread
From: Kevin Wolf @ 2025-04-25 17:52 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, qemu-devel
From: Stefan Hajnoczi <stefanha@redhat.com>
When guests send misaligned discard requests, the block layer breaks
them up into a misaligned head, an aligned main body, and a misaligned
tail.
The file-posix block driver on Linux returns -EINVAL on misaligned
discard requests. This causes bdrv_co_pdiscard() to fail and guests
configured with werror=stop will pause.
Add a special case for misaligned head/tail requests. Simply continue
when EINVAL is encountered so that the aligned main body of the request
can be completed and the guest is not paused. This is the best we can do
when guest discard limits do not match the host discard limits.
Fixes: https://issues.redhat.com/browse/RHEL-86032
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Hanna Czenczek <hreitz@redhat.com>
Message-ID: <20250417150528.76470-3-stefanha@redhat.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
block/io.c | 15 ++++++++++-----
1 file changed, 10 insertions(+), 5 deletions(-)
diff --git a/block/io.c b/block/io.c
index 1ba8d1aeea..ccec11386b 100644
--- a/block/io.c
+++ b/block/io.c
@@ -3109,11 +3109,12 @@ int coroutine_fn bdrv_co_pdiscard(BdrvChild *child, int64_t offset,
/* Invalidate the cached block-status data range if this discard overlaps */
bdrv_bsc_invalidate_range(bs, offset, bytes);
- /* Discard is advisory, but some devices track and coalesce
+ /*
+ * Discard is advisory, but some devices track and coalesce
* unaligned requests, so we must pass everything down rather than
- * round here. Still, most devices will just silently ignore
- * unaligned requests (by returning -ENOTSUP), so we must fragment
- * the request accordingly. */
+ * round here. Still, most devices reject unaligned requests with
+ * -EINVAL or -ENOTSUP, so we must fragment the request accordingly.
+ */
align = MAX(bs->bl.pdiscard_alignment, bs->bl.request_alignment);
assert(align % bs->bl.request_alignment == 0);
head = offset % align;
@@ -3180,7 +3181,11 @@ int coroutine_fn bdrv_co_pdiscard(BdrvChild *child, int64_t offset,
}
}
if (ret && ret != -ENOTSUP) {
- goto out;
+ if (ret == -EINVAL && (offset % align != 0 || num % align != 0)) {
+ /* Silently skip rejected unaligned head/tail requests */
+ } else {
+ goto out; /* bail out */
+ }
}
offset += num;
--
2.49.0
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PULL 3/4] block: Remove unused callback function *bdrv_aio_pdiscard
2025-04-25 17:52 [PULL 0/4] Block layer patches Kevin Wolf
2025-04-25 17:52 ` [PULL 1/4] file-posix: probe discard alignment on Linux block devices Kevin Wolf
2025-04-25 17:52 ` [PULL 2/4] block/io: skip head/tail requests on EINVAL Kevin Wolf
@ 2025-04-25 17:52 ` Kevin Wolf
2025-04-25 17:52 ` [PULL 4/4] qemu-img: improve queue depth validation in img_bench Kevin Wolf
2025-04-28 17:56 ` [PULL 0/4] Block layer patches Stefan Hajnoczi
4 siblings, 0 replies; 11+ messages in thread
From: Kevin Wolf @ 2025-04-25 17:52 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, qemu-devel
From: Sunny Zhu <sunnyzhyy@qq.com>
The bytes type in *bdrv_aio_pdiscard should be int64_t rather than int.
There are no drivers implementing the *bdrv_aio_pdiscard() callback,
it appears to be an unused function. Therefore, we'll simply remove it
instead of fixing it.
Additionally, coroutine-based callbacks are preferred. If someone needs
to implement bdrv_aio_pdiscard, a coroutine-based version would be
straightforward to implement.
Signed-off-by: Sunny Zhu <sunnyzhyy@qq.com>
Message-ID: <tencent_7140D2E54157D98CF3D9E64B1A007A1A7906@qq.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
include/block/block_int-common.h | 4 ----
block/io.c | 22 +++-------------------
2 files changed, 3 insertions(+), 23 deletions(-)
diff --git a/include/block/block_int-common.h b/include/block/block_int-common.h
index ebb4e56a50..0d8187f656 100644
--- a/include/block/block_int-common.h
+++ b/include/block/block_int-common.h
@@ -506,10 +506,6 @@ struct BlockDriver {
BlockAIOCB * GRAPH_RDLOCK_PTR (*bdrv_aio_flush)(
BlockDriverState *bs, BlockCompletionFunc *cb, void *opaque);
- BlockAIOCB * GRAPH_RDLOCK_PTR (*bdrv_aio_pdiscard)(
- BlockDriverState *bs, int64_t offset, int bytes,
- BlockCompletionFunc *cb, void *opaque);
-
int coroutine_fn GRAPH_RDLOCK_PTR (*bdrv_co_readv)(BlockDriverState *bs,
int64_t sector_num, int nb_sectors, QEMUIOVector *qiov);
diff --git a/block/io.c b/block/io.c
index ccec11386b..6d98b0abb9 100644
--- a/block/io.c
+++ b/block/io.c
@@ -3102,7 +3102,7 @@ int coroutine_fn bdrv_co_pdiscard(BdrvChild *child, int64_t offset,
return 0;
}
- if (!bs->drv->bdrv_co_pdiscard && !bs->drv->bdrv_aio_pdiscard) {
+ if (!bs->drv->bdrv_co_pdiscard) {
return 0;
}
@@ -3162,24 +3162,8 @@ int coroutine_fn bdrv_co_pdiscard(BdrvChild *child, int64_t offset,
ret = -ENOMEDIUM;
goto out;
}
- if (bs->drv->bdrv_co_pdiscard) {
- ret = bs->drv->bdrv_co_pdiscard(bs, offset, num);
- } else {
- BlockAIOCB *acb;
- CoroutineIOCompletion co = {
- .coroutine = qemu_coroutine_self(),
- };
-
- acb = bs->drv->bdrv_aio_pdiscard(bs, offset, num,
- bdrv_co_io_em_complete, &co);
- if (acb == NULL) {
- ret = -EIO;
- goto out;
- } else {
- qemu_coroutine_yield();
- ret = co.ret;
- }
- }
+
+ ret = bs->drv->bdrv_co_pdiscard(bs, offset, num);
if (ret && ret != -ENOTSUP) {
if (ret == -EINVAL && (offset % align != 0 || num % align != 0)) {
/* Silently skip rejected unaligned head/tail requests */
--
2.49.0
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PULL 4/4] qemu-img: improve queue depth validation in img_bench
2025-04-25 17:52 [PULL 0/4] Block layer patches Kevin Wolf
` (2 preceding siblings ...)
2025-04-25 17:52 ` [PULL 3/4] block: Remove unused callback function *bdrv_aio_pdiscard Kevin Wolf
@ 2025-04-25 17:52 ` Kevin Wolf
2025-04-28 13:54 ` Michael Tokarev
2025-04-28 17:56 ` [PULL 0/4] Block layer patches Stefan Hajnoczi
4 siblings, 1 reply; 11+ messages in thread
From: Kevin Wolf @ 2025-04-25 17:52 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, qemu-devel
From: Denis Rastyogin <gerben@altlinux.org>
This error was discovered by fuzzing qemu-img.
Currently, running `qemu-img bench -d 0` in img_bench is allowed,
which is a pointless operation and causes qemu-img to hang.
Signed-off-by: Denis Rastyogin <gerben@altlinux.org>
Message-ID: <20250327162423.25154-5-gerben@altlinux.org>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
qemu-img.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/qemu-img.c b/qemu-img.c
index 2044c22a4c..76ac5d3028 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -4571,7 +4571,7 @@ static int img_bench(int argc, char **argv)
{
unsigned long res;
- if (qemu_strtoul(optarg, NULL, 0, &res) < 0 || res > INT_MAX) {
+ if (qemu_strtoul(optarg, NULL, 0, &res) <= 0 || res > INT_MAX) {
error_report("Invalid queue depth specified");
return 1;
}
--
2.49.0
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PULL 4/4] qemu-img: improve queue depth validation in img_bench
2025-04-25 17:52 ` [PULL 4/4] qemu-img: improve queue depth validation in img_bench Kevin Wolf
@ 2025-04-28 13:54 ` Michael Tokarev
2025-04-28 13:58 ` Michael Tokarev
0 siblings, 1 reply; 11+ messages in thread
From: Michael Tokarev @ 2025-04-28 13:54 UTC (permalink / raw)
To: Kevin Wolf, qemu-block; +Cc: qemu-devel
25.04.2025 20:52, Kevin Wolf wrote:
> From: Denis Rastyogin <gerben@altlinux.org>
>
> This error was discovered by fuzzing qemu-img.
>
> Currently, running `qemu-img bench -d 0` in img_bench is allowed,
> which is a pointless operation and causes qemu-img to hang.
>
> Signed-off-by: Denis Rastyogin <gerben@altlinux.org>
> Message-ID: <20250327162423.25154-5-gerben@altlinux.org>
> Reviewed-by: Kevin Wolf <kwolf@redhat.com>
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
> qemu-img.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/qemu-img.c b/qemu-img.c
> index 2044c22a4c..76ac5d3028 100644
> --- a/qemu-img.c
> +++ b/qemu-img.c
> @@ -4571,7 +4571,7 @@ static int img_bench(int argc, char **argv)
> {
> unsigned long res;
>
> - if (qemu_strtoul(optarg, NULL, 0, &res) < 0 || res > INT_MAX) {
> + if (qemu_strtoul(optarg, NULL, 0, &res) <= 0 || res > INT_MAX) {
> error_report("Invalid queue depth specified");
> return 1;
> }
FWIW, it's been covered by my qemu-img options patches for way over a year.
/mjt
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PULL 4/4] qemu-img: improve queue depth validation in img_bench
2025-04-28 13:54 ` Michael Tokarev
@ 2025-04-28 13:58 ` Michael Tokarev
2025-05-13 15:06 ` Kevin Wolf
0 siblings, 1 reply; 11+ messages in thread
From: Michael Tokarev @ 2025-04-28 13:58 UTC (permalink / raw)
To: Kevin Wolf, qemu-block; +Cc: qemu-devel
28.04.2025 16:54, Michael Tokarev пишет:
> 25.04.2025 20:52, Kevin Wolf wrote:
>> From: Denis Rastyogin <gerben@altlinux.org>
>>
>> This error was discovered by fuzzing qemu-img.
>>
>> Currently, running `qemu-img bench -d 0` in img_bench is allowed,
>> which is a pointless operation and causes qemu-img to hang.
>>
>> Signed-off-by: Denis Rastyogin <gerben@altlinux.org>
>> Message-ID: <20250327162423.25154-5-gerben@altlinux.org>
>> Reviewed-by: Kevin Wolf <kwolf@redhat.com>
>> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
>> ---
>> qemu-img.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/qemu-img.c b/qemu-img.c
>> index 2044c22a4c..76ac5d3028 100644
>> --- a/qemu-img.c
>> +++ b/qemu-img.c
>> @@ -4571,7 +4571,7 @@ static int img_bench(int argc, char **argv)
>> {
>> unsigned long res;
>> - if (qemu_strtoul(optarg, NULL, 0, &res) < 0 || res > INT_MAX) {
>> + if (qemu_strtoul(optarg, NULL, 0, &res) <= 0 || res > INT_MAX) {
>> error_report("Invalid queue depth specified");
>> return 1;
>> }
>
> FWIW, it's been covered by my qemu-img options patches for way over a year.
In particular:
https://lore.kernel.org/qemu-devel/20240927061121.573271-28-mjt@tls.msk.ru/
I'm still waiting for some feedback from these patches - heard neither ACK
nor NACK for this rather large work.
/mjt
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PULL 4/4] qemu-img: improve queue depth validation in img_bench
2025-04-28 13:58 ` Michael Tokarev
@ 2025-05-13 15:06 ` Kevin Wolf
2025-05-14 9:28 ` Michael Tokarev
0 siblings, 1 reply; 11+ messages in thread
From: Kevin Wolf @ 2025-05-13 15:06 UTC (permalink / raw)
To: Michael Tokarev; +Cc: qemu-block, qemu-devel
Am 28.04.2025 um 15:58 hat Michael Tokarev geschrieben:
> 28.04.2025 16:54, Michael Tokarev пишет:
> > 25.04.2025 20:52, Kevin Wolf wrote:
> > > From: Denis Rastyogin <gerben@altlinux.org>
> > >
> > > This error was discovered by fuzzing qemu-img.
> > >
> > > Currently, running `qemu-img bench -d 0` in img_bench is allowed,
> > > which is a pointless operation and causes qemu-img to hang.
> > >
> > > Signed-off-by: Denis Rastyogin <gerben@altlinux.org>
> > > Message-ID: <20250327162423.25154-5-gerben@altlinux.org>
> > > Reviewed-by: Kevin Wolf <kwolf@redhat.com>
> > > Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> > > ---
> > > qemu-img.c | 2 +-
> > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/qemu-img.c b/qemu-img.c
> > > index 2044c22a4c..76ac5d3028 100644
> > > --- a/qemu-img.c
> > > +++ b/qemu-img.c
> > > @@ -4571,7 +4571,7 @@ static int img_bench(int argc, char **argv)
> > > {
> > > unsigned long res;
> > > - if (qemu_strtoul(optarg, NULL, 0, &res) < 0 || res > INT_MAX) {
> > > + if (qemu_strtoul(optarg, NULL, 0, &res) <= 0 || res > INT_MAX) {
> > > error_report("Invalid queue depth specified");
> > > return 1;
> > > }
> >
> > FWIW, it's been covered by my qemu-img options patches for way over a year.
>
> In particular:
>
> https://lore.kernel.org/qemu-devel/20240927061121.573271-28-mjt@tls.msk.ru/
>
> I'm still waiting for some feedback from these patches - heard neither ACK
> nor NACK for this rather large work.
Oops, seems I never continued review after patch 5. I'll get back to it.
However, I don't see the above hunk in that series. Am I missing it or
is there another series of yours waiting for review?
Kevin
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PULL 4/4] qemu-img: improve queue depth validation in img_bench
2025-05-13 15:06 ` Kevin Wolf
@ 2025-05-14 9:28 ` Michael Tokarev
2025-05-14 9:31 ` Michael Tokarev
0 siblings, 1 reply; 11+ messages in thread
From: Michael Tokarev @ 2025-05-14 9:28 UTC (permalink / raw)
To: Kevin Wolf; +Cc: qemu-block, qemu-devel
On 13.05.2025 18:06, Kevin Wolf wrote:
> Am 28.04.2025 um 15:58 hat Michael Tokarev geschrieben:
>> 28.04.2025 16:54, Michael Tokarev пишет:
>>> 25.04.2025 20:52, Kevin Wolf wrote:
>>>> From: Denis Rastyogin <gerben@altlinux.org>
>>>>
>>>> This error was discovered by fuzzing qemu-img.
>>>>
>>>> Currently, running `qemu-img bench -d 0` in img_bench is allowed,
>>>> which is a pointless operation and causes qemu-img to hang.
>>>>
>>>> Signed-off-by: Denis Rastyogin <gerben@altlinux.org>
>>>> Message-ID: <20250327162423.25154-5-gerben@altlinux.org>
>>>> Reviewed-by: Kevin Wolf <kwolf@redhat.com>
>>>> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
>>>> ---
>>>> qemu-img.c | 2 +-
>>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/qemu-img.c b/qemu-img.c
>>>> index 2044c22a4c..76ac5d3028 100644
>>>> --- a/qemu-img.c
>>>> +++ b/qemu-img.c
>>>> @@ -4571,7 +4571,7 @@ static int img_bench(int argc, char **argv)
>>>> {
>>>> unsigned long res;
>>>> - if (qemu_strtoul(optarg, NULL, 0, &res) < 0 || res > INT_MAX) {
>>>> + if (qemu_strtoul(optarg, NULL, 0, &res) <= 0 || res > INT_MAX) {
>>>> error_report("Invalid queue depth specified");
>>>> return 1;
>>>> }
>>>
>>> FWIW, it's been covered by my qemu-img options patches for way over a year.
>>
>> In particular:
>>
>> https://lore.kernel.org/qemu-devel/20240927061121.573271-28-mjt@tls.msk.ru/
>>
>> I'm still waiting for some feedback from these patches - heard neither ACK
>> nor NACK for this rather large work.
>
> Oops, seems I never continued review after patch 5. I'll get back to it.
>
> However, I don't see the above hunk in that series. Am I missing it or
> is there another series of yours waiting for review?
This one:
@@ -4791,27 +4788,17 @@ static int img_bench(const img_cmd_t *ccmd, int
argc, char **argv)
);
break;
case 'c':
- {
- unsigned long res;
-
- if (qemu_strtoul(optarg, NULL, 0, &res) < 0 || res > INT_MAX) {
- error_report("Invalid request count specified");
+ count = cvtnum_full("request count", optarg, false, 1,
INT_MAX);
+ if (count < 0) {
return 1;
}
Thanks,
/mjt
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PULL 4/4] qemu-img: improve queue depth validation in img_bench
2025-05-14 9:28 ` Michael Tokarev
@ 2025-05-14 9:31 ` Michael Tokarev
0 siblings, 0 replies; 11+ messages in thread
From: Michael Tokarev @ 2025-05-14 9:31 UTC (permalink / raw)
To: Kevin Wolf; +Cc: qemu-block, qemu-devel
On 14.05.2025 12:28, Michael Tokarev wrote:
> On 13.05.2025 18:06, Kevin Wolf wrote:
>> Am 28.04.2025 um 15:58 hat Michael Tokarev geschrieben:
>>> 28.04.2025 16:54, Michael Tokarev пишет:
>>>> 25.04.2025 20:52, Kevin Wolf wrote:
>>>>> From: Denis Rastyogin <gerben@altlinux.org>
>>>>>
>>>>> This error was discovered by fuzzing qemu-img.
>>>>>
>>>>> Currently, running `qemu-img bench -d 0` in img_bench is allowed,
>>>>> which is a pointless operation and causes qemu-img to hang.
>>>>>
>>>>> Signed-off-by: Denis Rastyogin <gerben@altlinux.org>
>>>>> Message-ID: <20250327162423.25154-5-gerben@altlinux.org>
>>>>> Reviewed-by: Kevin Wolf <kwolf@redhat.com>
>>>>> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
>>>>> ---
>>>>> qemu-img.c | 2 +-
>>>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/qemu-img.c b/qemu-img.c
>>>>> index 2044c22a4c..76ac5d3028 100644
>>>>> --- a/qemu-img.c
>>>>> +++ b/qemu-img.c
>>>>> @@ -4571,7 +4571,7 @@ static int img_bench(int argc, char **argv)
>>>>> {
>>>>> unsigned long res;
>>>>> - if (qemu_strtoul(optarg, NULL, 0, &res) < 0 || res >
>>>>> INT_MAX) {
>>>>> + if (qemu_strtoul(optarg, NULL, 0, &res) <= 0 || res >
>>>>> INT_MAX) {
>>>>> error_report("Invalid queue depth specified");
>>>>> return 1;
>>>>> }
>>>>
>>>> FWIW, it's been covered by my qemu-img options patches for way over
>>>> a year.
>>>
>>> In particular:
>>>
>>> https://lore.kernel.org/qemu-devel/20240927061121.573271-28-
>>> mjt@tls.msk.ru/
>>>
>>> I'm still waiting for some feedback from these patches - heard
>>> neither ACK
>>> nor NACK for this rather large work.
>>
>> Oops, seems I never continued review after patch 5. I'll get back to it.
>>
>> However, I don't see the above hunk in that series. Am I missing it or
>> is there another series of yours waiting for review?
>
> This one:
Actually it is the next very similar hunk. But doesn't matter anymore,
since this 4/4 partial patch has been applied already.
/mjt
> @@ -4791,27 +4788,17 @@ static int img_bench(const img_cmd_t *ccmd, int
> argc, char **argv)
> );
> break;
> case 'c':
> - {
> - unsigned long res;
> -
> - if (qemu_strtoul(optarg, NULL, 0, &res) < 0 || res >
> INT_MAX) {
> - error_report("Invalid request count specified");
> + count = cvtnum_full("request count", optarg, false, 1,
> INT_MAX);
> + if (count < 0) {
> return 1;
> }
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PULL 0/4] Block layer patches
2025-04-25 17:52 [PULL 0/4] Block layer patches Kevin Wolf
` (3 preceding siblings ...)
2025-04-25 17:52 ` [PULL 4/4] qemu-img: improve queue depth validation in img_bench Kevin Wolf
@ 2025-04-28 17:56 ` Stefan Hajnoczi
4 siblings, 0 replies; 11+ messages in thread
From: Stefan Hajnoczi @ 2025-04-28 17:56 UTC (permalink / raw)
To: Kevin Wolf; +Cc: qemu-block, kwolf, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 116 bytes --]
Applied, thanks.
Please update the changelog at https://wiki.qemu.org/ChangeLog/10.0 for any user-visible changes.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 11+ messages in thread