The following changes since commit bf159f0bdc7b8e7aa8342dedb3829ca744c1b612:

Merge remote-tracking branch 'remotes/edgar/tags/edgar/xilinx-next-2021-01-27.for-upstream' into staging (2021-01-27 17:40:25 +0000)

for you to fetch changes up to a44be0334beae3a9affb4a3a92cc6852993d7a84:

iotests: rename and move 169 and 199 tests (2021-01-27 20:53:14 +0100)

- Fix crash on write to read-only devices

- iotests: Rewrite 'check' in Python, get rid of 'groups' and allow

non-numeric test case names

Kevin Wolf (2):

block: Separate blk_is_writable() and blk_supports_write_perm()

virtio-scsi-test: Test writing to scsi-cd device

Vladimir Sementsov-Ogievskiy (6):

iotests: 146: drop extra whitespaces from .out file

iotests: add findtests.py

iotests: add testenv.py

iotests: add testrunner.py

iotests: rewrite check into python

iotests: rename and move 169 and 199 tests

docs/devel/testing.rst | 50 +-

include/sysemu/block-backend.h | 3 +-

block/block-backend.c | 19 +-

hw/block/dataplane/xen-block.c | 2 +-

hw/block/fdc.c | 9 +-

hw/block/m25p80.c | 6 +-

hw/block/nand.c | 2 +-

hw/block/nvme-ns.c | 7 +-

hw/block/onenand.c | 2 +-

hw/block/pflash_cfi01.c | 2 +-

hw/block/pflash_cfi02.c | 2 +-

hw/block/swim.c | 6 +-

hw/block/virtio-blk.c | 6 +-

hw/block/xen-block.c | 2 +-

hw/ide/core.c | 2 +-

hw/misc/sifive_u_otp.c | 2 +-

hw/ppc/pnv_pnor.c | 2 +-

hw/scsi/scsi-disk.c | 10 +-

hw/scsi/scsi-generic.c | 4 +-

hw/sd/sd.c | 6 +-

hw/usb/dev-storage.c | 4 +-

tests/qtest/virtio-scsi-test.c | 39 +

tests/qemu-iotests/findtests.py | 159 +++

tests/qemu-iotests/iotests.py | 8 +

tests/qemu-iotests/testenv.py | 281 +++++

tests/qemu-iotests/testrunner.py | 367 +++++++

Makefile | 1 -

tests/check-block.sh | 3 +-

tests/qemu-iotests/146.out | 780 +++++++-------

tests/qemu-iotests/check | 1095 +++-----------------

tests/qemu-iotests/common.env.in | 3 -

tests/qemu-iotests/group | 323 ------

tests/qemu-iotests/meson.build | 3 -

.../{199 => tests/migrate-bitmaps-postcopy-test} | 0

.../migrate-bitmaps-postcopy-test.out} | 0

.../{169 => tests/migrate-bitmaps-test} | 0

.../{169.out => tests/migrate-bitmaps-test.out} | 0

37 files changed, 1481 insertions(+), 1729 deletions(-)

create mode 100644 tests/qemu-iotests/findtests.py

create mode 100644 tests/qemu-iotests/testenv.py

create mode 100644 tests/qemu-iotests/testrunner.py

delete mode 100644 tests/qemu-iotests/common.env.in

delete mode 100644 tests/qemu-iotests/group

rename tests/qemu-iotests/{199 => tests/migrate-bitmaps-postcopy-test} (100%)

rename tests/qemu-iotests/{199.out => tests/migrate-bitmaps-postcopy-test.out} (100%)

rename tests/qemu-iotests/{169 => tests/migrate-bitmaps-test} (100%)

rename tests/qemu-iotests/{169.out => tests/migrate-bitmaps-test.out} (100%)

Rename bitmaps migration tests and move them to tests subdirectory to

demonstrate new human-friendly test naming.

Message-Id: <20210125185056.129513-7-vsementsov@virtuozzo.com>

tests/qemu-iotests/{199 => tests/migrate-bitmaps-postcopy-test} | 0

.../{199.out => tests/migrate-bitmaps-postcopy-test.out} | 0

tests/qemu-iotests/{169 => tests/migrate-bitmaps-test} | 0

tests/qemu-iotests/{169.out => tests/migrate-bitmaps-test.out} | 0

4 files changed, 0 insertions(+), 0 deletions(-)

rename tests/qemu-iotests/{199 => tests/migrate-bitmaps-postcopy-test} (100%)

rename tests/qemu-iotests/{199.out => tests/migrate-bitmaps-postcopy-test.out} (100%)

rename tests/qemu-iotests/{169 => tests/migrate-bitmaps-test} (100%)

rename tests/qemu-iotests/{169.out => tests/migrate-bitmaps-test.out} (100%)

diff --git a/tests/qemu-iotests/199 b/tests/qemu-iotests/tests/migrate-bitmaps-postcopy-test

similarity index 100%

rename from tests/qemu-iotests/199

rename to tests/qemu-iotests/tests/migrate-bitmaps-postcopy-test

diff --git a/tests/qemu-iotests/199.out b/tests/qemu-iotests/tests/migrate-bitmaps-postcopy-test.out

similarity index 100%

rename from tests/qemu-iotests/199.out

rename to tests/qemu-iotests/tests/migrate-bitmaps-postcopy-test.out

diff --git a/tests/qemu-iotests/169 b/tests/qemu-iotests/tests/migrate-bitmaps-test

similarity index 100%

rename from tests/qemu-iotests/169

rename to tests/qemu-iotests/tests/migrate-bitmaps-test

diff --git a/tests/qemu-iotests/169.out b/tests/qemu-iotests/tests/migrate-bitmaps-test.out

similarity index 100%

rename from tests/qemu-iotests/169.out

rename to tests/qemu-iotests/tests/migrate-bitmaps-test.out

Currently, blk_is_read_only() tells whether a given BlockBackend can

only be used in read-only mode because its root node is read-only. Some

callers actually try to answer a slightly different question: Is the

BlockBackend configured to be writable, by taking write permissions on

the root node?

This can differ, for example, for CD-ROM devices which don't take write

permissions, but may be backed by a writable image file. scsi-cd allows

write requests to the drive if blk_is_read_only() returns false.

However, the write request will immediately run into an assertion

failure because the write permission is missing.

This patch introduces separate functions for both questions.

blk_supports_write_perm() answers the question whether the block

node/image file can support writable devices, whereas blk_is_writable()

tells whether the BlockBackend is currently configured to be writable.

All calls of blk_is_read_only() are converted to one of the two new

functions.

Fixes: https://bugs.launchpad.net/bugs/1906693

Cc: qemu-stable@nongnu.org

Signed-off-by: Kevin Wolf <kwolf@redhat.com>

Message-Id: <20210118123448.307825-2-kwolf@redhat.com>

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Reviewed-by: Max Reitz <mreitz@redhat.com>

include/sysemu/block-backend.h | 3 ++-

block/block-backend.c | 19 ++++++++++++++++---

hw/block/dataplane/xen-block.c | 2 +-

hw/block/fdc.c | 9 +++++----

hw/block/m25p80.c | 6 +++---

hw/block/nand.c | 2 +-

hw/block/nvme-ns.c | 7 ++++---

hw/block/onenand.c | 2 +-

hw/block/pflash_cfi01.c | 2 +-

hw/block/pflash_cfi02.c | 2 +-

hw/block/swim.c | 6 +++---

hw/block/virtio-blk.c | 6 +++---

hw/block/xen-block.c | 2 +-

hw/ide/core.c | 2 +-

hw/misc/sifive_u_otp.c | 2 +-

hw/ppc/pnv_pnor.c | 2 +-

hw/scsi/scsi-disk.c | 10 +++++-----

hw/scsi/scsi-generic.c | 4 ++--

hw/sd/sd.c | 6 +++---

hw/usb/dev-storage.c | 4 ++--

20 files changed, 57 insertions(+), 41 deletions(-)

diff --git a/include/sysemu/block-backend.h b/include/sysemu/block-backend.h

--- a/include/sysemu/block-backend.h

+++ b/include/sysemu/block-backend.h

@@ -XXX,XX +XXX,XX @@ BlockErrorAction blk_get_error_action(BlockBackend *blk, bool is_read,

int error);

void blk_error_action(BlockBackend *blk, BlockErrorAction action,

bool is_read, int error);

-bool blk_is_read_only(BlockBackend *blk);

+bool blk_supports_write_perm(BlockBackend *blk);

+bool blk_is_writable(BlockBackend *blk);

bool blk_is_sg(BlockBackend *blk);

bool blk_enable_write_cache(BlockBackend *blk);

void blk_set_enable_write_cache(BlockBackend *blk, bool wce);

diff --git a/block/block-backend.c b/block/block-backend.c

index XXXXXXX..XXXXXXX 100644

--- a/block/block-backend.c

+++ b/block/block-backend.c

@@ -XXX,XX +XXX,XX @@ void blk_error_action(BlockBackend *blk, BlockErrorAction action,

}

-bool blk_is_read_only(BlockBackend *blk)

+/*

+ * Returns true if the BlockBackend can support taking write permissions

+ * (because its root node is not read-only).

+ */

+bool blk_supports_write_perm(BlockBackend *blk)

{

BlockDriverState *bs = blk_bs(blk);

if (bs) {

- return bdrv_is_read_only(bs);

+ return !bdrv_is_read_only(bs);

} else {

- return blk->root_state.read_only;

+ return !blk->root_state.read_only;

}

+/*

+ * Returns true if the BlockBackend can be written to in its current

+ * configuration (i.e. if write permission have been requested)

+ */

+bool blk_is_writable(BlockBackend *blk)

+{

+ return blk->perm & BLK_PERM_WRITE;

+}

+

bool blk_is_sg(BlockBackend *blk)

{

BlockDriverState *bs = blk_bs(blk);

diff --git a/hw/block/dataplane/xen-block.c b/hw/block/dataplane/xen-block.c

index XXXXXXX..XXXXXXX 100644

--- a/hw/block/dataplane/xen-block.c

+++ b/hw/block/dataplane/xen-block.c

@@ -XXX,XX +XXX,XX @@ static int xen_block_parse_request(XenBlockRequest *request)

};

if (request->req.operation != BLKIF_OP_READ &&

- blk_is_read_only(dataplane->blk)) {

+ !blk_is_writable(dataplane->blk)) {

error_report("error: write req for ro device");

goto err;

}

diff --git a/hw/block/fdc.c b/hw/block/fdc.c

index XXXXXXX..XXXXXXX 100644

--- a/hw/block/fdc.c

+++ b/hw/block/fdc.c

@@ -XXX,XX +XXX,XX @@ static void fd_revalidate(FDrive *drv)

FLOPPY_DPRINTF("revalidate\n");

if (drv->blk != NULL) {

- drv->ro = blk_is_read_only(drv->blk);

+ drv->ro = !blk_is_writable(drv->blk);

if (!blk_is_inserted(drv->blk)) {

FLOPPY_DPRINTF("No disk in drive\n");

drv->disk = FLOPPY_DRIVE_TYPE_NONE;

@@ -XXX,XX +XXX,XX @@ static void fd_change_cb(void *opaque, bool load, Error **errp)

blk_set_perm(drive->blk, 0, BLK_PERM_ALL, &error_abort);

} else {

if (!blkconf_apply_backend_options(drive->conf,

- blk_is_read_only(drive->blk), false,

- errp)) {

+ !blk_supports_write_perm(drive->blk),

+ false, errp)) {

return;

}

}

@@ -XXX,XX +XXX,XX @@ static void floppy_drive_realize(DeviceState *qdev, Error **errp)

* read-only node later */

read_only = true;

} else {

- read_only = !blk_bs(dev->conf.blk) || blk_is_read_only(dev->conf.blk);

+ read_only = !blk_bs(dev->conf.blk) ||

+ !blk_supports_write_perm(dev->conf.blk);

}

if (!blkconf_blocksizes(&dev->conf, errp)) {

diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c

index XXXXXXX..XXXXXXX 100644

--- a/hw/block/m25p80.c

+++ b/hw/block/m25p80.c

@@ -XXX,XX +XXX,XX @@ static void flash_sync_page(Flash *s, int page)

{

QEMUIOVector *iov;

- if (!s->blk || blk_is_read_only(s->blk)) {

+ if (!s->blk || !blk_is_writable(s->blk)) {

return;

}

@@ -XXX,XX +XXX,XX @@ static inline void flash_sync_area(Flash *s, int64_t off, int64_t len)

{

QEMUIOVector *iov;

- if (!s->blk || blk_is_read_only(s->blk)) {

+ if (!s->blk || !blk_is_writable(s->blk)) {

return;

}

@@ -XXX,XX +XXX,XX @@ static void m25p80_realize(SSIPeripheral *ss, Error **errp)

if (s->blk) {

uint64_t perm = BLK_PERM_CONSISTENT_READ |

- (blk_is_read_only(s->blk) ? 0 : BLK_PERM_WRITE);

+ (blk_supports_write_perm(s->blk) ? BLK_PERM_WRITE : 0);

ret = blk_set_perm(s->blk, perm, BLK_PERM_ALL, errp);

if (ret < 0) {

return;

diff --git a/hw/block/nand.c b/hw/block/nand.c

index XXXXXXX..XXXXXXX 100644

--- a/hw/block/nand.c

+++ b/hw/block/nand.c

@@ -XXX,XX +XXX,XX @@ static void nand_realize(DeviceState *dev, Error **errp)

pagesize = 1 << s->oob_shift;

s->mem_oob = 1;

if (s->blk) {

- if (blk_is_read_only(s->blk)) {

+ if (!blk_supports_write_perm(s->blk)) {

error_setg(errp, "Can't use a read-only drive");

return;

}

diff --git a/hw/block/nvme-ns.c b/hw/block/nvme-ns.c

index XXXXXXX..XXXXXXX 100644

--- a/hw/block/nvme-ns.c

+++ b/hw/block/nvme-ns.c

@@ -XXX,XX +XXX,XX @@ static void nvme_ns_init(NvmeNamespace *ns)

static int nvme_ns_init_blk(NvmeCtrl *n, NvmeNamespace *ns, Error **errp)

{

+ bool read_only;

+

if (!blkconf_blocksizes(&ns->blkconf, errp)) {

return -1;

}

- if (!blkconf_apply_backend_options(&ns->blkconf,

- blk_is_read_only(ns->blkconf.blk),

- false, errp)) {

+ read_only = !blk_supports_write_perm(ns->blkconf.blk);

+ if (!blkconf_apply_backend_options(&ns->blkconf, read_only, false, errp)) {

return -1;

}

diff --git a/hw/block/onenand.c b/hw/block/onenand.c

index XXXXXXX..XXXXXXX 100644

--- a/hw/block/onenand.c

+++ b/hw/block/onenand.c

@@ -XXX,XX +XXX,XX @@ static void onenand_realize(DeviceState *dev, Error **errp)

s->image = memset(g_malloc(size + (size >> 5)),

0xff, size + (size >> 5));

} else {

- if (blk_is_read_only(s->blk)) {

+ if (!blk_supports_write_perm(s->blk)) {

error_setg(errp, "Can't use a read-only drive");

return;

}

diff --git a/hw/block/pflash_cfi01.c b/hw/block/pflash_cfi01.c

index XXXXXXX..XXXXXXX 100644

--- a/hw/block/pflash_cfi01.c

+++ b/hw/block/pflash_cfi01.c

@@ -XXX,XX +XXX,XX @@ static void pflash_cfi01_realize(DeviceState *dev, Error **errp)

if (pfl->blk) {

uint64_t perm;

- pfl->ro = blk_is_read_only(pfl->blk);

+ pfl->ro = !blk_supports_write_perm(pfl->blk);

perm = BLK_PERM_CONSISTENT_READ | (pfl->ro ? 0 : BLK_PERM_WRITE);

ret = blk_set_perm(pfl->blk, perm, BLK_PERM_ALL, errp);

if (ret < 0) {

diff --git a/hw/block/pflash_cfi02.c b/hw/block/pflash_cfi02.c

index XXXXXXX..XXXXXXX 100644

--- a/hw/block/pflash_cfi02.c

+++ b/hw/block/pflash_cfi02.c

@@ -XXX,XX +XXX,XX @@ static void pflash_cfi02_realize(DeviceState *dev, Error **errp)

if (pfl->blk) {

uint64_t perm;

- pfl->ro = blk_is_read_only(pfl->blk);

+ pfl->ro = !blk_supports_write_perm(pfl->blk);

perm = BLK_PERM_CONSISTENT_READ | (pfl->ro ? 0 : BLK_PERM_WRITE);

ret = blk_set_perm(pfl->blk, perm, BLK_PERM_ALL, errp);

if (ret < 0) {

diff --git a/hw/block/swim.c b/hw/block/swim.c

index XXXXXXX..XXXXXXX 100644

--- a/hw/block/swim.c

+++ b/hw/block/swim.c

@@ -XXX,XX +XXX,XX @@ static void swim_change_cb(void *opaque, bool load, Error **errp)

blk_set_perm(drive->blk, 0, BLK_PERM_ALL, &error_abort);

} else {

if (!blkconf_apply_backend_options(drive->conf,

- blk_is_read_only(drive->blk), false,

- errp)) {

+ !blk_supports_write_perm(drive->blk),

+ false, errp)) {

return;

}

}

@@ -XXX,XX +XXX,XX @@ static void swim_drive_realize(DeviceState *qdev, Error **errp)

dev->conf.werror = BLOCKDEV_ON_ERROR_AUTO;

if (!blkconf_apply_backend_options(&dev->conf,

- blk_is_read_only(dev->conf.blk),

+ !blk_supports_write_perm(dev->conf.blk),

false, errp)) {

return;

}

diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c

index XXXXXXX..XXXXXXX 100644

--- a/hw/block/virtio-blk.c

+++ b/hw/block/virtio-blk.c

@@ -XXX,XX +XXX,XX @@ static uint64_t virtio_blk_get_features(VirtIODevice *vdev, uint64_t features,

virtio_has_feature(features, VIRTIO_BLK_F_CONFIG_WCE))) {

virtio_add_feature(&features, VIRTIO_BLK_F_WCE);

}

- if (blk_is_read_only(s->blk)) {

+ if (!blk_is_writable(s->blk)) {

virtio_add_feature(&features, VIRTIO_BLK_F_RO);

}

if (s->conf.num_queues > 1) {

@@ -XXX,XX +XXX,XX @@ static void virtio_blk_device_realize(DeviceState *dev, Error **errp)

}

if (!blkconf_apply_backend_options(&conf->conf,

- blk_is_read_only(conf->conf.blk), true,

- errp)) {

+ !blk_supports_write_perm(conf->conf.blk),

+ true, errp)) {

return;

}

s->original_wce = blk_enable_write_cache(conf->conf.blk);

diff --git a/hw/block/xen-block.c b/hw/block/xen-block.c

index XXXXXXX..XXXXXXX 100644

--- a/hw/block/xen-block.c

+++ b/hw/block/xen-block.c

@@ -XXX,XX +XXX,XX @@ static void xen_disk_realize(XenBlockDevice *blockdev, Error **errp)

return;

}

- blockdev->info = blk_is_read_only(conf->blk) ? VDISK_READONLY : 0;

+ blockdev->info = blk_supports_write_perm(conf->blk) ? 0 : VDISK_READONLY;

}

static void xen_disk_class_init(ObjectClass *class, void *data)

diff --git a/hw/ide/core.c b/hw/ide/core.c

index XXXXXXX..XXXXXXX 100644

--- a/hw/ide/core.c

+++ b/hw/ide/core.c

@@ -XXX,XX +XXX,XX @@ int ide_init_drive(IDEState *s, BlockBackend *blk, IDEDriveKind kind,

error_setg(errp, "Device needs media, but drive is empty");

return -1;

}

- if (blk_is_read_only(blk)) {

+ if (!blk_is_writable(blk)) {

error_setg(errp, "Can't use a read-only drive");

return -1;

}

diff --git a/hw/misc/sifive_u_otp.c b/hw/misc/sifive_u_otp.c

index XXXXXXX..XXXXXXX 100644

--- a/hw/misc/sifive_u_otp.c

+++ b/hw/misc/sifive_u_otp.c

@@ -XXX,XX +XXX,XX @@ static void sifive_u_otp_realize(DeviceState *dev, Error **errp)

if (s->blk) {

perm = BLK_PERM_CONSISTENT_READ |

- (blk_is_read_only(s->blk) ? 0 : BLK_PERM_WRITE);

+ (blk_supports_write_perm(s->blk) ? BLK_PERM_WRITE : 0);

ret = blk_set_perm(s->blk, perm, BLK_PERM_ALL, errp);

if (ret < 0) {

return;

diff --git a/hw/ppc/pnv_pnor.c b/hw/ppc/pnv_pnor.c

index XXXXXXX..XXXXXXX 100644

--- a/hw/ppc/pnv_pnor.c

+++ b/hw/ppc/pnv_pnor.c

@@ -XXX,XX +XXX,XX @@ static void pnv_pnor_realize(DeviceState *dev, Error **errp)

if (s->blk) {

uint64_t perm = BLK_PERM_CONSISTENT_READ |

- (blk_is_read_only(s->blk) ? 0 : BLK_PERM_WRITE);

+ (blk_supports_write_perm(s->blk) ? BLK_PERM_WRITE : 0);

ret = blk_set_perm(s->blk, perm, BLK_PERM_ALL, errp);

if (ret < 0) {

return;

diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c

index XXXXXXX..XXXXXXX 100644

--- a/hw/scsi/scsi-disk.c

+++ b/hw/scsi/scsi-disk.c

@@ -XXX,XX +XXX,XX @@ static int scsi_disk_emulate_mode_sense(SCSIDiskReq *r, uint8_t *outbuf)

if (s->qdev.type == TYPE_DISK) {

dev_specific_param = s->features & (1 << SCSI_DISK_F_DPOFUA) ? 0x10 : 0;

- if (blk_is_read_only(s->qdev.conf.blk)) {

+ if (!blk_is_writable(s->qdev.conf.blk)) {

dev_specific_param |= 0x80; /* Readonly. */

}

} else {

@@ -XXX,XX +XXX,XX @@ static void scsi_disk_emulate_unmap(SCSIDiskReq *r, uint8_t *inbuf)

goto invalid_param_len;

}

- if (blk_is_read_only(s->qdev.conf.blk)) {

+ if (!blk_is_writable(s->qdev.conf.blk)) {

block_acct_invalid(blk_get_stats(s->qdev.conf.blk), BLOCK_ACCT_UNMAP);

scsi_check_condition(r, SENSE_CODE(WRITE_PROTECTED));

return;

@@ -XXX,XX +XXX,XX @@ static void scsi_disk_emulate_write_same(SCSIDiskReq *r, uint8_t *inbuf)

return;

}

- if (blk_is_read_only(s->qdev.conf.blk)) {

+ if (!blk_is_writable(s->qdev.conf.blk)) {

scsi_check_condition(r, SENSE_CODE(WRITE_PROTECTED));

return;

}

@@ -XXX,XX +XXX,XX @@ static int32_t scsi_disk_dma_command(SCSIRequest *req, uint8_t *buf)

case WRITE_VERIFY_10:

case WRITE_VERIFY_12:

case WRITE_VERIFY_16:

- if (blk_is_read_only(s->qdev.conf.blk)) {

+ if (!blk_is_writable(s->qdev.conf.blk)) {

scsi_check_condition(r, SENSE_CODE(WRITE_PROTECTED));

return 0;

}

@@ -XXX,XX +XXX,XX @@ static void scsi_realize(SCSIDevice *dev, Error **errp)

}

}

- read_only = blk_is_read_only(s->qdev.conf.blk);

+ read_only = !blk_supports_write_perm(s->qdev.conf.blk);

if (dev->type == TYPE_ROM) {

read_only = true;

}

diff --git a/hw/scsi/scsi-generic.c b/hw/scsi/scsi-generic.c

index XXXXXXX..XXXXXXX 100644

--- a/hw/scsi/scsi-generic.c

+++ b/hw/scsi/scsi-generic.c

@@ -XXX,XX +XXX,XX @@ static void scsi_read_complete(void * opaque, int ret)

* readonly.

*/

if ((s->type == TYPE_DISK || s->type == TYPE_TAPE || s->type == TYPE_ZBC) &&

- blk_is_read_only(s->conf.blk) &&

+ !blk_is_writable(s->conf.blk) &&

(r->req.cmd.buf[0] == MODE_SENSE ||

r->req.cmd.buf[0] == MODE_SENSE_10) &&

(r->req.cmd.buf[1] & 0x8) == 0) {

@@ -XXX,XX +XXX,XX @@ static void scsi_generic_realize(SCSIDevice *s, Error **errp)

return;

}

if (!blkconf_apply_backend_options(&s->conf,

- blk_is_read_only(s->conf.blk),

+ !blk_supports_write_perm(s->conf.blk),

true, errp)) {

return;

}

diff --git a/hw/sd/sd.c b/hw/sd/sd.c

index XXXXXXX..XXXXXXX 100644

--- a/hw/sd/sd.c

+++ b/hw/sd/sd.c

@@ -XXX,XX +XXX,XX @@ static void sd_reset(DeviceState *dev)

sd_set_sdstatus(sd);

g_free(sd->wp_groups);

- sd->wp_switch = sd->blk ? blk_is_read_only(sd->blk) : false;

+ sd->wp_switch = sd->blk ? !blk_is_writable(sd->blk) : false;

sd->wpgrps_size = sect;

sd->wp_groups = bitmap_new(sd->wpgrps_size);

memset(sd->function_group, 0, sizeof(sd->function_group));

@@ -XXX,XX +XXX,XX @@ void sd_set_cb(SDState *sd, qemu_irq readonly, qemu_irq insert)

{

sd->readonly_cb = readonly;

sd->inserted_cb = insert;

- qemu_set_irq(readonly, sd->blk ? blk_is_read_only(sd->blk) : 0);

+ qemu_set_irq(readonly, sd->blk ? !blk_is_writable(sd->blk) : 0);

qemu_set_irq(insert, sd->blk ? blk_is_inserted(sd->blk) : 0);

}

@@ -XXX,XX +XXX,XX @@ static void sd_realize(DeviceState *dev, Error **errp)

if (sd->blk) {

int64_t blk_size;

- if (blk_is_read_only(sd->blk)) {

+ if (!blk_supports_write_perm(sd->blk)) {

error_setg(errp, "Cannot use read-only drive as SD card");

return;

}

diff --git a/hw/usb/dev-storage.c b/hw/usb/dev-storage.c

index XXXXXXX..XXXXXXX 100644

--- a/hw/usb/dev-storage.c

+++ b/hw/usb/dev-storage.c

@@ -XXX,XX +XXX,XX @@ static void usb_msd_storage_realize(USBDevice *dev, Error **errp)

return;

}

- if (!blkconf_apply_backend_options(&s->conf, blk_is_read_only(blk), true,

- errp)) {

+ if (!blkconf_apply_backend_options(&s->conf, !blk_supports_write_perm(blk),

+ true, errp)) {

return;

}

This tests that trying to write to a (read-only) scsi-cd device backed

by a read-write image file doesn't crash and results in the correct

error.

This is a regression test for https://bugs.launchpad.net/bugs/1906693.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>

Message-Id: <20210118123448.307825-3-kwolf@redhat.com>

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Reviewed-by: Max Reitz <mreitz@redhat.com>

tests/qtest/virtio-scsi-test.c | 39 ++++++++++++++++++++++++++++++++++

1 file changed, 39 insertions(+)

diff --git a/tests/qtest/virtio-scsi-test.c b/tests/qtest/virtio-scsi-test.c

--- a/tests/qtest/virtio-scsi-test.c

+++ b/tests/qtest/virtio-scsi-test.c

@@ -XXX,XX +XXX,XX @@ static void test_unaligned_write_same(void *obj, void *data,

qvirtio_scsi_pci_free(vs);

+static void test_write_to_cdrom(void *obj, void *data,

+ QGuestAllocator *t_alloc)

+{

+ QVirtioSCSI *scsi = obj;

+ QVirtioSCSIQueues *vs;

+ uint8_t buf[2048] = { 0 };

+ const uint8_t write_cdb[VIRTIO_SCSI_CDB_SIZE] = {

+ /* WRITE(10) to LBA 0, transfer length 1 */

+ 0x2a, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01, 0x00

+ };

+ struct virtio_scsi_cmd_resp resp;

+ alloc = t_alloc;

+ vs = qvirtio_scsi_init(scsi->vdev);

+

+ virtio_scsi_do_command(vs, write_cdb, NULL, 0, buf, 2048, &resp);

+ g_assert_cmphex(resp.response, ==, 0);

+ g_assert_cmphex(resp.status, ==, CHECK_CONDITION);

+ g_assert_cmphex(resp.sense[0], ==, 0x70);

+ g_assert_cmphex(resp.sense[2], ==, DATA_PROTECT);

+ g_assert_cmphex(resp.sense[12], ==, 0x27); /* WRITE PROTECTED */

+ g_assert_cmphex(resp.sense[13], ==, 0x00); /* WRITE PROTECTED */

+

+ qvirtio_scsi_pci_free(vs);

+}

+

static void test_iothread_attach_node(void *obj, void *data,

QGuestAllocator *t_alloc)

{

@@ -XXX,XX +XXX,XX @@ static void *virtio_scsi_setup(GString *cmd_line, void *arg)

return arg;

}

+static void *virtio_scsi_setup_cd(GString *cmd_line, void *arg)

+{

+ g_string_append(cmd_line,

+ " -drive file=null-co://,"

+ "file.read-zeroes=on,"

+ "if=none,id=dr1,format=raw "

+ "-device scsi-cd,drive=dr1,lun=0,scsi-id=1");

+ return arg;

+}

+

static void *virtio_scsi_setup_iothread(GString *cmd_line, void *arg)

{

g_string_append(cmd_line,

@@ -XXX,XX +XXX,XX @@ 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_cd;

+ qos_add_test("write-to-cdrom", "virtio-scsi", test_write_to_cdrom, &opts);

+

opts.before = virtio_scsi_setup_iothread;

opts.edge = (QOSGraphEdgeOptions) {

.extra_device_opts = "iothread=thread0",

Add python script with new logic of searching for tests:

Current ./check behavior:

- tests are named [0-9][0-9][0-9]

- tests must be registered in group file (even if test doesn't belong

to any group, like 142)

Behavior of findtests.py:

- group file is dropped

- tests are all files in tests/ subdirectory (except for .out files),

so it's not needed more to "register the test", just create it with

appropriate name in tests/ subdirectory. Old names like

[0-9][0-9][0-9] (in root iotests directory) are supported too, but

not recommended for new tests

- groups are parsed from '# group: ' line inside test files

- optional file group.local may be used to define some additional

groups for downstreams

- 'disabled' group is used to temporary disable tests. So instead of

commenting tests in old 'group' file you now can add them to

disabled group with help of 'group.local' file

- selecting test ranges like 5-15 are not supported more

(to support restarting failed ./check command from the middle of the

process, new argument is added: --start-from)

Benefits:

- no rebase conflicts in group file on patch porting from branch to

branch

- no conflicts in upstream, when different series want to occupy same

test number

- meaningful names for test files

For example, with digital number, when some person wants to add some

test about block-stream, he most probably will just create a new

test. But if there would be test-block-stream test already, he will

at first look at it and may be just add a test-case into it.

And anyway meaningful names are better.

This commit doesn't update check behavior (which will be done in

further commit), still, the documentation changed like new behavior is

already here. Let's live with this small inconsistency for the

following few commits, until final change.

Reviewed-by: Eric Blake <eblake@redhat.com>

Message-Id: <20210125185056.129513-3-vsementsov@virtuozzo.com>

docs/devel/testing.rst | 50 +++++++++-

tests/qemu-iotests/findtests.py | 159 ++++++++++++++++++++++++++++++++

2 files changed, 208 insertions(+), 1 deletion(-)

create mode 100644 tests/qemu-iotests/findtests.py

diff --git a/docs/devel/testing.rst b/docs/devel/testing.rst

--- a/docs/devel/testing.rst

+++ b/docs/devel/testing.rst

@@ -XXX,XX +XXX,XX @@ check-block

-----------

``make check-block`` runs a subset of the block layer iotests (the tests that

-are in the "auto" group in ``tests/qemu-iotests/group``).

+are in the "auto" group).

See the "QEMU iotests" section below for more information.

GCC gcov support

@@ -XXX,XX +XXX,XX @@ another application on the host may have locked the file, possibly leading to a

test failure. If using such devices are explicitly desired, consider adding

``locking=off`` option to disable image locking.

+Test case groups

+----------------

+

+"Tests may belong to one or more test groups, which are defined in the form

+of a comment in the test source file. By convention, test groups are listed

+in the second line of the test file, after the "#!/..." line, like this:

+

+.. code::

+

+ #!/usr/bin/env python3

+ # group: auto quick

+ #

+ ...

+

+Another way of defining groups is creating the tests/qemu-iotests/group.local

+file. This should be used only for downstream (this file should never appear

+in upstream). This file may be used for defining some downstream test groups

+or for temporarily disabling tests, like this:

+

+.. code::

+

+ # groups for some company downstream process

+ #

+ # ci - tests to run on build

+ # down - our downstream tests, not for upstream

+ #

+ # Format of each line is:

+ # TEST_NAME TEST_GROUP [TEST_GROUP ]...

+

+ 013 ci

+ 210 disabled

+ 215 disabled

+ our-ugly-workaround-test down ci

+

+Note that the following group names have a special meaning:

+

+- quick: Tests in this group should finish within a few seconds.

+

+- auto: Tests in this group are used during "make check" and should be

+ runnable in any case. That means they should run with every QEMU binary

+ (also non-x86), with every QEMU configuration (i.e. must not fail if

+ an optional feature is not compiled in - but reporting a "skip" is ok),

+ work at least with the qcow2 file format, work with all kind of host

+ filesystems and users (e.g. "nobody" or "root") and must not take too

+ much memory and disk space (since CI pipelines tend to fail otherwise).

+

+- disabled: Tests in this group are disabled and ignored by check.

+

.. _docker-ref:

Docker based tests

diff --git a/tests/qemu-iotests/findtests.py b/tests/qemu-iotests/findtests.py

new file mode 100644

index XXXXXXX..XXXXXXX

--- /dev/null

+++ b/tests/qemu-iotests/findtests.py

@@ -XXX,XX +XXX,XX @@

+# TestFinder class, define set of tests to run.

+#

+# Copyright (c) 2020-2021 Virtuozzo International GmbH

+#

+# This program is free software; you can redistribute it and/or modify

+# it under the terms of the GNU General Public License as published by

+# the Free Software Foundation; either version 2 of the License, or

+# (at your option) any later version.

+#

+# This program is distributed in the hope that it will be useful,

+# but WITHOUT ANY WARRANTY; without even the implied warranty of

+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the

+# GNU General Public License for more details.

+#

+# You should have received a copy of the GNU General Public License

+# along with this program. If not, see <http://www.gnu.org/licenses/>.

+#

+

+import os

+import glob

+import re

+from collections import defaultdict

+from contextlib import contextmanager

+from typing import Optional, List, Iterator, Set

+

+

+@contextmanager

+def chdir(path: Optional[str] = None) -> Iterator[None]:

+ if path is None:

+ yield

+ return

+

+ saved_dir = os.getcwd()

+ os.chdir(path)

+ try:

+ yield

+ finally:

+ os.chdir(saved_dir)

+

+

+class TestFinder:

+ def __init__(self, test_dir: Optional[str] = None) -> None:

+ self.groups = defaultdict(set)

+

+ with chdir(test_dir):

+ self.all_tests = glob.glob('[0-9][0-9][0-9]')

+ self.all_tests += [f for f in glob.iglob('tests/*')

+ if not f.endswith('.out') and

+ os.path.isfile(f + '.out')]

+

+ for t in self.all_tests:

+ with open(t, encoding="utf-8") as f:

+ for line in f:

+ if line.startswith('# group: '):

+ for g in line.split()[2:]:

+ self.groups[g].add(t)

+ break

+

+ def add_group_file(self, fname: str) -> None:

+ with open(fname, encoding="utf-8") as f:

+ for line in f:

+ line = line.strip()

+

+ if (not line) or line[0] == '#':

+ continue

+

+ words = line.split()

+ test_file = self.parse_test_name(words[0])

+ groups = words[1:]

+

+ for g in groups:

+ self.groups[g].add(test_file)

+

+ def parse_test_name(self, name: str) -> str:

+ if '/' in name:

+ raise ValueError('Paths are unsupported for test selection, '

+ f'requiring "{name}" is wrong')

+

+ if re.fullmatch(r'\d+', name):

+ # Numbered tests are old naming convention. We should convert them

+ # to three-digit-length, like 1 --> 001.

+ name = f'{int(name):03}'

+ else:

+ # Named tests all should be in tests/ subdirectory

+ name = os.path.join('tests', name)

+

+ if name not in self.all_tests:

+ raise ValueError(f'Test "{name}" is not found')

+

+ return name

+

+ def find_tests(self, groups: Optional[List[str]] = None,

+ exclude_groups: Optional[List[str]] = None,

+ tests: Optional[List[str]] = None,

+ start_from: Optional[str] = None) -> List[str]:

+ """Find tests

+

+ Algorithm:

+

+ 1. a. if some @groups specified

+ a.1 Take all tests from @groups

+ a.2 Drop tests, which are in at least one of @exclude_groups or in

+ 'disabled' group (if 'disabled' is not listed in @groups)

+ a.3 Add tests from @tests (don't exclude anything from them)

+

+ b. else, if some @tests specified:

+ b.1 exclude_groups must be not specified, so just take @tests

+

+ c. else (only @exclude_groups list is non-empty):

+ c.1 Take all tests

+ c.2 Drop tests, which are in at least one of @exclude_groups or in

+ 'disabled' group

+

+ 2. sort

+

+ 3. If start_from specified, drop tests from first one to @start_from

+ (not inclusive)

+ """

+ if groups is None:

+ groups = []

+ if exclude_groups is None:

+ exclude_groups = []

+ if tests is None:

+ tests = []

+

+ res: Set[str] = set()

+ if groups:

+ # Some groups specified. exclude_groups supported, additionally

+ # selecting some individual tests supported as well.

+ res.update(*(self.groups[g] for g in groups))

+ elif tests:

+ # Some individual tests specified, but no groups. In this case

+ # we don't support exclude_groups.

+ if exclude_groups:

+ raise ValueError("Can't exclude from individually specified "

+ "tests.")

+ else:

+ # No tests no groups: start from all tests, exclude_groups

+ # supported.

+ res.update(self.all_tests)

+

+ if 'disabled' not in groups and 'disabled' not in exclude_groups:

+ # Don't want to modify function argument, so create new list.

+ exclude_groups = exclude_groups + ['disabled']

+

+ res = res.difference(*(self.groups[g] for g in exclude_groups))

+

+ # We want to add @tests. But for compatibility with old test names,

+ # we should convert any number < 100 to number padded by

+ # leading zeroes, like 1 -> 001 and 23 -> 023.

+ for t in tests:

+ res.add(self.parse_test_name(t))

+

+ sequence = sorted(res)

+

+ if start_from is not None:

+ del sequence[:sequence.index(self.parse_test_name(start_from))]

+

+ return sequence

From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>

Add TestEnv class, which will handle test environment in a new python

iotests running framework.

Don't add compat=1.1 for qcow2 IMGOPTS, as v3 is default anyway.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>

Message-Id: <20210125185056.129513-4-vsementsov@virtuozzo.com>

tests/qemu-iotests/testenv.py | 281 ++++++++++++++++++++++++++++++++++

1 file changed, 281 insertions(+)

create mode 100644 tests/qemu-iotests/testenv.py

new file mode 100644

index XXXXXXX..XXXXXXX

--- /dev/null

@@ -XXX,XX +XXX,XX @@

+# TestEnv class to manage test environment variables.

+#

+# Copyright (c) 2020-2021 Virtuozzo International GmbH

+#

+# This program is free software; you can redistribute it and/or modify

+# it under the terms of the GNU General Public License as published by

+# the Free Software Foundation; either version 2 of the License, or

+# (at your option) any later version.

+#

+# This program is distributed in the hope that it will be useful,

+# but WITHOUT ANY WARRANTY; without even the implied warranty of

+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the

+# GNU General Public License for more details.

+#

+# You should have received a copy of the GNU General Public License

+# along with this program. If not, see <http://www.gnu.org/licenses/>.

+#

+

+import os

+import sys

+import tempfile

+from pathlib import Path

+import shutil

+import collections

+import random

+import subprocess

+import glob

+from typing import Dict, Any, Optional, ContextManager

+

+

+def isxfile(path: str) -> bool:

+ return os.path.isfile(path) and os.access(path, os.X_OK)

+

+

+def get_default_machine(qemu_prog: str) -> str:

+ outp = subprocess.run([qemu_prog, '-machine', 'help'], check=True,

+ universal_newlines=True,

+ stdout=subprocess.PIPE).stdout

+

+ machines = outp.split('\n')

+ try:

+ default_machine = next(m for m in machines if m.endswith(' (default)'))

+ except StopIteration:

+ return ''

+ default_machine = default_machine.split(' ', 1)[0]

+

+ alias_suf = ' (alias of {})'.format(default_machine)

+ alias = next((m for m in machines if m.endswith(alias_suf)), None)

+ if alias is not None:

+ default_machine = alias.split(' ', 1)[0]

+

+ return default_machine

+

+

+class TestEnv(ContextManager['TestEnv']):

+ """

+ Manage system environment for running tests

+

+ The following variables are supported/provided. They are represented by

+ lower-cased TestEnv attributes.

+ """

+

+ # We store environment variables as instance attributes, and there are a

+ # lot of them. Silence pylint:

+ # pylint: disable=too-many-instance-attributes

+

+ env_variables = ['PYTHONPATH', 'TEST_DIR', 'SOCK_DIR', 'SAMPLE_IMG_DIR',

+ 'OUTPUT_DIR', 'PYTHON', 'QEMU_PROG', 'QEMU_IMG_PROG',

+ 'QEMU_IO_PROG', 'QEMU_NBD_PROG', 'QSD_PROG',

+ 'SOCKET_SCM_HELPER', 'QEMU_OPTIONS', 'QEMU_IMG_OPTIONS',

+ 'QEMU_IO_OPTIONS', 'QEMU_IO_OPTIONS_NO_FMT',

+ 'QEMU_NBD_OPTIONS', 'IMGOPTS', 'IMGFMT', 'IMGPROTO',

+ 'AIOMODE', 'CACHEMODE', 'VALGRIND_QEMU',

+ 'CACHEMODE_IS_DEFAULT', 'IMGFMT_GENERIC', 'IMGOPTSSYNTAX',

+ 'IMGKEYSECRET', 'QEMU_DEFAULT_MACHINE', 'MALLOC_PERTURB_']

+

+ def get_env(self) -> Dict[str, str]:

+ env = {}

+ for v in self.env_variables:

+ val = getattr(self, v.lower(), None)

+ if val is not None:

+ env[v] = val

+

+ return env

+

+ def init_directories(self) -> None:

+ """Init directory variables:

+ PYTHONPATH

+ TEST_DIR

+ SOCK_DIR

+ SAMPLE_IMG_DIR

+ OUTPUT_DIR

+ """

+ self.pythonpath = os.getenv('PYTHONPATH')

+ if self.pythonpath:

+ self.pythonpath = self.source_iotests + os.pathsep + \

+ self.pythonpath

+ else:

+ self.pythonpath = self.source_iotests

+

+ self.test_dir = os.getenv('TEST_DIR',

+ os.path.join(os.getcwd(), 'scratch'))

+ Path(self.test_dir).mkdir(parents=True, exist_ok=True)

+

+ try:

+ self.sock_dir = os.environ['SOCK_DIR']

+ self.tmp_sock_dir = False

+ Path(self.test_dir).mkdir(parents=True, exist_ok=True)

+ except KeyError:

+ self.sock_dir = tempfile.mkdtemp()

+ self.tmp_sock_dir = True

+

+ self.sample_img_dir = os.getenv('SAMPLE_IMG_DIR',

+ os.path.join(self.source_iotests,

+ 'sample_images'))

+

+ self.output_dir = os.getcwd() # OUTPUT_DIR

+

+ def init_binaries(self) -> None:

+ """Init binary path variables:

+ PYTHON (for bash tests)

+ QEMU_PROG, QEMU_IMG_PROG, QEMU_IO_PROG, QEMU_NBD_PROG, QSD_PROG

+ SOCKET_SCM_HELPER

+ """

+ self.python = sys.executable

+

+ def root(*names: str) -> str:

+ return os.path.join(self.build_root, *names)

+

+ arch = os.uname().machine

+ if 'ppc64' in arch:

+ arch = 'ppc64'

+

+ self.qemu_prog = os.getenv('QEMU_PROG', root(f'qemu-system-{arch}'))

+ if not os.path.exists(self.qemu_prog):

+ pattern = root('qemu-system-*')

+ try:

+ progs = glob.iglob(pattern)

+ self.qemu_prog = next(p for p in progs if isxfile(p))

+ except StopIteration:

+ sys.exit("Not found any Qemu executable binary by pattern "

+ f"'{pattern}'")

+

+ self.qemu_img_prog = os.getenv('QEMU_IMG_PROG', root('qemu-img'))

+ self.qemu_io_prog = os.getenv('QEMU_IO_PROG', root('qemu-io'))

+ self.qemu_nbd_prog = os.getenv('QEMU_NBD_PROG', root('qemu-nbd'))

+ self.qsd_prog = os.getenv('QSD_PROG', root('storage-daemon',

+ 'qemu-storage-daemon'))

+

+ for b in [self.qemu_img_prog, self.qemu_io_prog, self.qemu_nbd_prog,

+ self.qemu_prog, self.qsd_prog]:

+ if not os.path.exists(b):

+ sys.exit('No such file: ' + b)

+ if not isxfile(b):

+ sys.exit('Not executable: ' + b)

+

+ helper_path = os.path.join(self.build_iotests, 'socket_scm_helper')

+ if isxfile(helper_path):

+ self.socket_scm_helper = helper_path # SOCKET_SCM_HELPER

+

+ def __init__(self, imgfmt: str, imgproto: str, aiomode: str,

+ cachemode: Optional[str] = None,

+ imgopts: Optional[str] = None,

+ misalign: bool = False,

+ debug: bool = False,

+ valgrind: bool = False) -> None:

+ self.imgfmt = imgfmt

+ self.imgproto = imgproto

+ self.aiomode = aiomode

+ self.imgopts = imgopts

+ self.misalign = misalign

+ self.debug = debug

+

+ if valgrind:

+ self.valgrind_qemu = 'y'

+

+ if cachemode is None:

+ self.cachemode_is_default = 'true'

+ self.cachemode = 'writeback'

+ else:

+ self.cachemode_is_default = 'false'

+ self.cachemode = cachemode

+

+ # Initialize generic paths: build_root, build_iotests, source_iotests,

+ # which are needed to initialize some environment variables. They are

+ # used by init_*() functions as well.

+

+ if os.path.islink(sys.argv[0]):

+ # called from the build tree

+ self.source_iotests = os.path.dirname(os.readlink(sys.argv[0]))

+ self.build_iotests = os.path.dirname(os.path.abspath(sys.argv[0]))

+ else:

+ # called from the source tree

+ self.source_iotests = os.getcwd()

+ self.build_iotests = self.source_iotests

+