* [PATCH 0/3] testing block device blocksizes
@ 2021-05-25 12:47 Kit Westneat
2021-05-25 12:47 ` [PATCH 1/3] block/blkdebug: add blocksize parameter Kit Westneat
` (3 more replies)
0 siblings, 4 replies; 7+ messages in thread
From: Kit Westneat @ 2021-05-25 12:47 UTC (permalink / raw)
To: qemu-devel; +Cc: pbonzini, Kit Westneat
These patches add a few parameters to blkdebug to allow modification of
the block device block sizes, both logical and physical. It also adds a
test that uses the parameter to verify correct UNMAP behavior in devices
with 4k blocks.
Kit Westneat (3):
block/blkdebug: add blocksize parameter
tests/qtest/virtio-scsi-test: add unmap large LBA with 4k blocks test
block/blkdebug: add log-blocksize and phys-blocksize parameters
block/blkdebug.c | 53 ++++++++++++++++++++++++++++++++++
tests/qtest/virtio-scsi-test.c | 48 ++++++++++++++++++++++++++++++
2 files changed, 101 insertions(+)
--
2.26.3
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 1/3] block/blkdebug: add blocksize parameter
2021-05-25 12:47 [PATCH 0/3] testing block device blocksizes Kit Westneat
@ 2021-05-25 12:47 ` Kit Westneat
2021-05-25 14:46 ` Eric Blake
2021-05-25 12:47 ` [PATCH 2/3] tests/qtest/virtio-scsi-test: add unmap large LBA with 4k blocks test Kit Westneat
` (2 subsequent siblings)
3 siblings, 1 reply; 7+ messages in thread
From: Kit Westneat @ 2021-05-25 12:47 UTC (permalink / raw)
To: qemu-devel; +Cc: pbonzini, Kit Westneat
Allow users to specify the block size of the qdev for testing purposes.
Signed-off-by: Kit Westneat <kit.westneat@gmail.com>
---
block/blkdebug.c | 27 +++++++++++++++++++++++++++
1 file changed, 27 insertions(+)
diff --git a/block/blkdebug.c b/block/blkdebug.c
index 2c0b9b0ee8..c7500746a8 100644
--- a/block/blkdebug.c
+++ b/block/blkdebug.c
@@ -47,6 +47,7 @@ typedef struct BDRVBlkdebugState {
uint64_t max_write_zero;
uint64_t opt_discard;
uint64_t max_discard;
+ uint64_t blocksize;
uint64_t take_child_perms;
uint64_t unshare_child_perms;
@@ -455,6 +456,11 @@ static QemuOptsList runtime_opts = {
.type = QEMU_OPT_SIZE,
.help = "Maximum discard size in bytes",
},
+ {
+ .name = "blocksize",
+ .type = QEMU_OPT_SIZE,
+ .help = "Blocksize of device",
+ },
{ /* end of list */ }
},
};
@@ -562,6 +568,13 @@ static int blkdebug_open(BlockDriverState *bs, QDict *options, int flags,
goto out;
}
+ s->blocksize = qemu_opt_get_size(opts, "blocksize", 512);
+ if (s->blocksize && (s->blocksize >= INT_MAX || !is_power_of_2(s->blocksize))) {
+ error_setg(errp, "Cannot meet constraints with blocksize %" PRIu64,
+ s->blocksize);
+ goto out;
+ }
+
bdrv_debug_event(bs, BLKDBG_NONE);
ret = 0;
@@ -984,6 +997,18 @@ static void blkdebug_refresh_limits(BlockDriverState *bs, Error **errp)
}
}
+static int blkdebug_probe_blocksizes(BlockDriverState *bs, BlockSizes *bsz)
+{
+ BDRVBlkdebugState *s = bs->opaque;
+
+ if (!s->blocksize)
+ return 0;
+
+ bsz->phys = s->blocksize;
+ bsz->log = s->blocksize;
+ return 0;
+}
+
static int blkdebug_reopen_prepare(BDRVReopenState *reopen_state,
BlockReopenQueue *queue, Error **errp)
{
@@ -1010,6 +1035,7 @@ static const char *const blkdebug_strong_runtime_opts[] = {
"inject-error.",
"set-state.",
"align",
+ "blocksize",
"max-transfer",
"opt-write-zero",
"max-write-zero",
@@ -1034,6 +1060,7 @@ static BlockDriver bdrv_blkdebug = {
.bdrv_getlength = blkdebug_getlength,
.bdrv_refresh_filename = blkdebug_refresh_filename,
.bdrv_refresh_limits = blkdebug_refresh_limits,
+ .bdrv_probe_blocksizes = blkdebug_probe_blocksizes,
.bdrv_co_preadv = blkdebug_co_preadv,
.bdrv_co_pwritev = blkdebug_co_pwritev,
--
2.26.3
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 2/3] tests/qtest/virtio-scsi-test: add unmap large LBA with 4k blocks test
2021-05-25 12:47 [PATCH 0/3] testing block device blocksizes Kit Westneat
2021-05-25 12:47 ` [PATCH 1/3] block/blkdebug: add blocksize parameter Kit Westneat
@ 2021-05-25 12:47 ` Kit Westneat
2021-05-25 12:47 ` [PATCH 3/3] block/blkdebug: add log-blocksize and phys-blocksize parameters Kit Westneat
2021-05-25 12:54 ` [PATCH 0/3] testing block device blocksizes no-reply
3 siblings, 0 replies; 7+ messages in thread
From: Kit Westneat @ 2021-05-25 12:47 UTC (permalink / raw)
To: qemu-devel; +Cc: pbonzini, Kit Westneat
Add test for issue #345
Signed-off-by: Kit Westneat <kit.westneat@gmail.com>
---
tests/qtest/virtio-scsi-test.c | 48 ++++++++++++++++++++++++++++++++++
1 file changed, 48 insertions(+)
diff --git a/tests/qtest/virtio-scsi-test.c b/tests/qtest/virtio-scsi-test.c
index 1b7ecc1c8f..6557e4f422 100644
--- a/tests/qtest/virtio-scsi-test.c
+++ b/tests/qtest/virtio-scsi-test.c
@@ -200,6 +200,40 @@ static void test_unaligned_write_same(void *obj, void *data,
qvirtio_scsi_pci_free(vs);
}
+/* Test UNMAP with a large LBA, issue #345 */
+static void test_unmap_large_lba(void *obj, void *data,
+ QGuestAllocator *t_alloc)
+{
+ QVirtioSCSI *scsi = obj;
+ QVirtioSCSIQueues *vs;
+ const uint8_t unmap[VIRTIO_SCSI_CDB_SIZE] = {
+ 0x42, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x18, 0x00
+ };
+
+ // default null-co device size is 2**30
+ // LBA 0x7fff is ~ 1/8 into device, with 4k blocks
+ // if check_lba_range incorrectly using 512 bytes, will trigger sense error
+ uint8_t unmap_params[0x18] = {
+ 0x00, 0x16, // unmap data length
+ 0x00, 0x10, // unmap block descriptor data length
+ 0x00, 0x00, 0x00, 0x00, // reserved
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x7f, 0xff, // LBA
+ 0x00, 0x00, 0x03, 0xff, // sector count
+ 0x00, 0x00, 0x00, 0x00, //reserved
+ };
+ struct virtio_scsi_cmd_resp resp;
+
+ alloc = t_alloc;
+ vs = qvirtio_scsi_init(scsi->vdev);
+
+ virtio_scsi_do_command(vs, unmap, NULL, 0, unmap_params,
+ sizeof(unmap_params), &resp);
+ g_assert_cmphex(resp.response, ==, 0);
+ g_assert_cmphex(resp.status, !=, CHECK_CONDITION);
+
+ qvirtio_scsi_pci_free(vs);
+}
+
static void test_write_to_cdrom(void *obj, void *data,
QGuestAllocator *t_alloc)
{
@@ -293,6 +327,16 @@ static void *virtio_scsi_setup(GString *cmd_line, void *arg)
return arg;
}
+static void *virtio_scsi_setup_4k(GString *cmd_line, void *arg)
+{
+ g_string_append(cmd_line,
+ " -drive file=blkdebug::null-co://,"
+ "file.image.read-zeroes=on,"
+ "if=none,id=dr1,format=raw,file.blocksize=4k "
+ "-device scsi-hd,drive=dr1,lun=0,scsi-id=1");
+ return arg;
+}
+
static void *virtio_scsi_setup_cd(GString *cmd_line, void *arg)
{
g_string_append(cmd_line,
@@ -323,6 +367,10 @@ static void register_virtio_scsi_test(void)
qos_add_test("unaligned-write-same", "virtio-scsi",
test_unaligned_write_same, &opts);
+ opts.before = virtio_scsi_setup_4k;
+ qos_add_test("large-lba-unmap", "virtio-scsi",
+ test_unmap_large_lba, &opts);
+
opts.before = virtio_scsi_setup_cd;
qos_add_test("write-to-cdrom", "virtio-scsi", test_write_to_cdrom, &opts);
--
2.26.3
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 3/3] block/blkdebug: add log-blocksize and phys-blocksize parameters
2021-05-25 12:47 [PATCH 0/3] testing block device blocksizes Kit Westneat
2021-05-25 12:47 ` [PATCH 1/3] block/blkdebug: add blocksize parameter Kit Westneat
2021-05-25 12:47 ` [PATCH 2/3] tests/qtest/virtio-scsi-test: add unmap large LBA with 4k blocks test Kit Westneat
@ 2021-05-25 12:47 ` Kit Westneat
2021-05-25 12:54 ` [PATCH 0/3] testing block device blocksizes no-reply
3 siblings, 0 replies; 7+ messages in thread
From: Kit Westneat @ 2021-05-25 12:47 UTC (permalink / raw)
To: qemu-devel; +Cc: pbonzini, Kit Westneat
Allow users to specify the logical and physical block sizes of the
qdev for testing purposes.
Signed-off-by: Kit Westneat <kit.westneat@gmail.com>
---
block/blkdebug.c | 36 +++++++++++++++++++++++++++++++-----
1 file changed, 31 insertions(+), 5 deletions(-)
diff --git a/block/blkdebug.c b/block/blkdebug.c
index c7500746a8..e5cfdab32b 100644
--- a/block/blkdebug.c
+++ b/block/blkdebug.c
@@ -48,6 +48,8 @@ typedef struct BDRVBlkdebugState {
uint64_t opt_discard;
uint64_t max_discard;
uint64_t blocksize;
+ uint64_t phys_blocksize;
+ uint64_t log_blocksize;
uint64_t take_child_perms;
uint64_t unshare_child_perms;
@@ -459,7 +461,17 @@ static QemuOptsList runtime_opts = {
{
.name = "blocksize",
.type = QEMU_OPT_SIZE,
- .help = "Blocksize of device",
+ .help = "Blocksize of device (512 default)",
+ },
+ {
+ .name = "phys-blocksize",
+ .type = QEMU_OPT_SIZE,
+ .help = "Physical blocksize of device (Defaults to 'blocksize')",
+ },
+ {
+ .name = "log-blocksize",
+ .type = QEMU_OPT_SIZE,
+ .help = "Logical blocksize of device (Defaults to 'blocksize')",
},
{ /* end of list */ }
},
@@ -575,6 +587,20 @@ static int blkdebug_open(BlockDriverState *bs, QDict *options, int flags,
goto out;
}
+ s->phys_blocksize = qemu_opt_get_size(opts, "phys-blocksize", 0);
+ if (s->phys_blocksize && (s->phys_blocksize >= INT_MAX || !is_power_of_2(s->phys_blocksize))) {
+ error_setg(errp, "Cannot meet constraints with phys-blocksize %" PRIu64,
+ s->phys_blocksize);
+ goto out;
+ }
+
+ s->log_blocksize = qemu_opt_get_size(opts, "log-blocksize", 0);
+ if (s->log_blocksize && (s->log_blocksize >= INT_MAX || !is_power_of_2(s->log_blocksize))) {
+ error_setg(errp, "Cannot meet constraints with log-blocksize %" PRIu64,
+ s->log_blocksize);
+ goto out;
+ }
+
bdrv_debug_event(bs, BLKDBG_NONE);
ret = 0;
@@ -1001,11 +1027,9 @@ static int blkdebug_probe_blocksizes(BlockDriverState *bs, BlockSizes *bsz)
{
BDRVBlkdebugState *s = bs->opaque;
- if (!s->blocksize)
- return 0;
+ bsz->phys = s->phys_blocksize ? s->phys_blocksize : s->blocksize;
+ bsz->log = s->log_blocksize ? s->log_blocksize : s->blocksize;
- bsz->phys = s->blocksize;
- bsz->log = s->blocksize;
return 0;
}
@@ -1036,6 +1060,8 @@ static const char *const blkdebug_strong_runtime_opts[] = {
"set-state.",
"align",
"blocksize",
+ "phys-blocksize",
+ "log-blocksize",
"max-transfer",
"opt-write-zero",
"max-write-zero",
--
2.26.3
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 0/3] testing block device blocksizes
2021-05-25 12:47 [PATCH 0/3] testing block device blocksizes Kit Westneat
` (2 preceding siblings ...)
2021-05-25 12:47 ` [PATCH 3/3] block/blkdebug: add log-blocksize and phys-blocksize parameters Kit Westneat
@ 2021-05-25 12:54 ` no-reply
3 siblings, 0 replies; 7+ messages in thread
From: no-reply @ 2021-05-25 12:54 UTC (permalink / raw)
To: kit.westneat; +Cc: pbonzini, kit.westneat, qemu-devel
Patchew URL: https://patchew.org/QEMU/20210525124753.528516-1-kit.westneat@gmail.com/
Hi,
This series seems to have some coding style problems. See output below for
more information:
Type: series
Message-id: 20210525124753.528516-1-kit.westneat@gmail.com
Subject: [PATCH 0/3] testing block device blocksizes
=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===
Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
* [new tag] patchew/20210525124753.528516-1-kit.westneat@gmail.com -> patchew/20210525124753.528516-1-kit.westneat@gmail.com
Switched to a new branch 'test'
8319440 block/blkdebug: add log-blocksize and phys-blocksize parameters
e09eac6 tests/qtest/virtio-scsi-test: add unmap large LBA with 4k blocks test
0442070 block/blkdebug: add blocksize parameter
=== OUTPUT BEGIN ===
1/3 Checking commit 0442070571e5 (block/blkdebug: add blocksize parameter)
WARNING: line over 80 characters
#44: FILE: block/blkdebug.c:572:
+ if (s->blocksize && (s->blocksize >= INT_MAX || !is_power_of_2(s->blocksize))) {
ERROR: braces {} are necessary for all arms of this statement
#61: FILE: block/blkdebug.c:1004:
+ if (!s->blocksize)
[...]
total: 1 errors, 1 warnings, 63 lines checked
Patch 1/3 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
2/3 Checking commit e09eac6b4197 (tests/qtest/virtio-scsi-test: add unmap large LBA with 4k blocks test)
ERROR: do not use C99 // comments
#33: FILE: tests/qtest/virtio-scsi-test.c:213:
+ // default null-co device size is 2**30
ERROR: do not use C99 // comments
#34: FILE: tests/qtest/virtio-scsi-test.c:214:
+ // LBA 0x7fff is ~ 1/8 into device, with 4k blocks
ERROR: do not use C99 // comments
#35: FILE: tests/qtest/virtio-scsi-test.c:215:
+ // if check_lba_range incorrectly using 512 bytes, will trigger sense error
ERROR: do not use C99 // comments
#37: FILE: tests/qtest/virtio-scsi-test.c:217:
+ 0x00, 0x16, // unmap data length
ERROR: do not use C99 // comments
#38: FILE: tests/qtest/virtio-scsi-test.c:218:
+ 0x00, 0x10, // unmap block descriptor data length
ERROR: do not use C99 // comments
#39: FILE: tests/qtest/virtio-scsi-test.c:219:
+ 0x00, 0x00, 0x00, 0x00, // reserved
ERROR: do not use C99 // comments
#40: FILE: tests/qtest/virtio-scsi-test.c:220:
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x7f, 0xff, // LBA
ERROR: do not use C99 // comments
#41: FILE: tests/qtest/virtio-scsi-test.c:221:
+ 0x00, 0x00, 0x03, 0xff, // sector count
ERROR: do not use C99 // comments
#42: FILE: tests/qtest/virtio-scsi-test.c:222:
+ 0x00, 0x00, 0x00, 0x00, //reserved
total: 9 errors, 0 warnings, 66 lines checked
Patch 2/3 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
3/3 Checking commit 8319440a28b2 (block/blkdebug: add log-blocksize and phys-blocksize parameters)
ERROR: line over 90 characters
#53: FILE: block/blkdebug.c:591:
+ if (s->phys_blocksize && (s->phys_blocksize >= INT_MAX || !is_power_of_2(s->phys_blocksize))) {
ERROR: line over 90 characters
#60: FILE: block/blkdebug.c:598:
+ if (s->log_blocksize && (s->log_blocksize >= INT_MAX || !is_power_of_2(s->log_blocksize))) {
total: 2 errors, 0 warnings, 67 lines checked
Patch 3/3 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
=== OUTPUT END ===
Test command exited with code: 1
The full log is available at
http://patchew.org/logs/20210525124753.528516-1-kit.westneat@gmail.com/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/3] block/blkdebug: add blocksize parameter
2021-05-25 12:47 ` [PATCH 1/3] block/blkdebug: add blocksize parameter Kit Westneat
@ 2021-05-25 14:46 ` Eric Blake
2021-05-25 17:59 ` Kit Westneat
0 siblings, 1 reply; 7+ messages in thread
From: Eric Blake @ 2021-05-25 14:46 UTC (permalink / raw)
To: Kit Westneat; +Cc: pbonzini, qemu-devel
On Tue, May 25, 2021 at 12:47:51PM +0000, Kit Westneat wrote:
> Allow users to specify the block size of the qdev for testing purposes.
>
> Signed-off-by: Kit Westneat <kit.westneat@gmail.com>
> ---
> block/blkdebug.c | 27 +++++++++++++++++++++++++++
> 1 file changed, 27 insertions(+)
Should this also be selectable when using QAPI to create a blkdebug
device over QMP?
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/3] block/blkdebug: add blocksize parameter
2021-05-25 14:46 ` Eric Blake
@ 2021-05-25 17:59 ` Kit Westneat
0 siblings, 0 replies; 7+ messages in thread
From: Kit Westneat @ 2021-05-25 17:59 UTC (permalink / raw)
To: Kit Westneat, qemu-devel, pbonzini
[-- Attachment #1: Type: text/plain, Size: 723 bytes --]
Makes sense. Sorry for the basic question, but is that just a matter of
adding it to qapi/block-core.json?
Thanks,
Kit
On Tue, May 25, 2021 at 10:46 AM Eric Blake <eblake@redhat.com> wrote:
> On Tue, May 25, 2021 at 12:47:51PM +0000, Kit Westneat wrote:
> > Allow users to specify the block size of the qdev for testing purposes.
> >
> > Signed-off-by: Kit Westneat <kit.westneat@gmail.com>
> > ---
> > block/blkdebug.c | 27 +++++++++++++++++++++++++++
> > 1 file changed, 27 insertions(+)
>
> Should this also be selectable when using QAPI to create a blkdebug
> device over QMP?
>
> --
> Eric Blake, Principal Software Engineer
> Red Hat, Inc. +1-919-301-3266
> Virtualization: qemu.org | libvirt.org
>
>
[-- Attachment #2: Type: text/html, Size: 1322 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2021-05-25 18:01 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-25 12:47 [PATCH 0/3] testing block device blocksizes Kit Westneat
2021-05-25 12:47 ` [PATCH 1/3] block/blkdebug: add blocksize parameter Kit Westneat
2021-05-25 14:46 ` Eric Blake
2021-05-25 17:59 ` Kit Westneat
2021-05-25 12:47 ` [PATCH 2/3] tests/qtest/virtio-scsi-test: add unmap large LBA with 4k blocks test Kit Westneat
2021-05-25 12:47 ` [PATCH 3/3] block/blkdebug: add log-blocksize and phys-blocksize parameters Kit Westneat
2021-05-25 12:54 ` [PATCH 0/3] testing block device blocksizes no-reply
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.