From f7afa7daa08c09d7c8435a95a13f6bd9dd11255e Mon Sep 17 00:00:00 2001 From: Connor Kuehl Date: Wed, 21 Apr 2021 16:23:42 -0500 Subject: [PATCH 01/19] iotests/231: Update expected deprecation message The deprecation message in the expected output has technically been wrong since the wrong version of a patch was applied to it. Because of this, the test fails. Correct the expected output so that it passes. Signed-off-by: Connor Kuehl Reviewed-by: Max Reitz Reviewed-by: Stefano Garzarella Message-Id: <20210421212343.85524-2-ckuehl@redhat.com> Signed-off-by: Max Reitz --- tests/qemu-iotests/231.out | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/tests/qemu-iotests/231.out b/tests/qemu-iotests/231.out index 579ba11c16..747dd221bb 100644 --- a/tests/qemu-iotests/231.out +++ b/tests/qemu-iotests/231.out @@ -1,9 +1,7 @@ QA output created by 231 -qemu-img: RBD options encoded in the filename as keyvalue pairs is deprecated. Future versions may cease to parse these options in the future. +qemu-img: warning: RBD options encoded in the filename as keyvalue pairs is deprecated unable to get monitor info from DNS SRV with service name: ceph-mon -no monitors specified to connect to. qemu-img: Could not open 'json:{'file.driver':'rbd','file.filename':'rbd:rbd/bogus:conf=BOGUS_CONF'}': error connecting: No such file or directory unable to get monitor info from DNS SRV with service name: ceph-mon -no monitors specified to connect to. qemu-img: Could not open 'json:{'file.driver':'rbd','file.pool':'rbd','file.image':'bogus','file.conf':'BOGUS_CONF'}': error connecting: No such file or directory *** done From 2b99cfce08da53a07e86271747fe465556ac7eb4 Mon Sep 17 00:00:00 2001 From: Connor Kuehl Date: Wed, 21 Apr 2021 16:23:43 -0500 Subject: [PATCH 02/19] block/rbd: Add an escape-aware strchr helper Sometimes the parser needs to further split a token it has collected from the token input stream. Right now, it does a cursory check to see if the relevant characters appear in the token to determine if it should break it down further. However, qemu_rbd_next_tok() will escape characters as it removes tokens from the token stream and plain strchr() won't. This can make the initial strchr() check slightly misleading since it implies qemu_rbd_next_tok() will find the token and split on it, except the reality is that qemu_rbd_next_tok() will pass over it if it is escaped. Use a custom strchr to avoid mixing escaped and unescaped string operations. Furthermore, this code is identical to how qemu_rbd_next_tok() seeks its next token, so incorporate this custom strchr into the body of that function to reduce duplication. Reported-by: Han Han Fixes: https://bugzilla.redhat.com/1873913 Signed-off-by: Connor Kuehl Message-Id: <20210421212343.85524-3-ckuehl@redhat.com> Reviewed-by: Stefano Garzarella Signed-off-by: Max Reitz --- block/rbd.c | 32 +++++++++++++++++++++----------- tests/qemu-iotests/231 | 4 ++++ tests/qemu-iotests/231.out | 3 +++ 3 files changed, 28 insertions(+), 11 deletions(-) diff --git a/block/rbd.c b/block/rbd.c index f098a89c7b..26f64cce7c 100644 --- a/block/rbd.c +++ b/block/rbd.c @@ -113,21 +113,31 @@ static int qemu_rbd_connect(rados_t *cluster, rados_ioctx_t *io_ctx, const char *keypairs, const char *secretid, Error **errp); +static char *qemu_rbd_strchr(char *src, char delim) +{ + char *p; + + for (p = src; *p; ++p) { + if (*p == delim) { + return p; + } + if (*p == '\\' && p[1] != '\0') { + ++p; + } + } + + return NULL; +} + + static char *qemu_rbd_next_tok(char *src, char delim, char **p) { char *end; *p = NULL; - for (end = src; *end; ++end) { - if (*end == delim) { - break; - } - if (*end == '\\' && end[1] != '\0') { - end++; - } - } - if (*end == delim) { + end = qemu_rbd_strchr(src, delim); + if (end) { *p = end + 1; *end = '\0'; } @@ -171,7 +181,7 @@ static void qemu_rbd_parse_filename(const char *filename, QDict *options, qemu_rbd_unescape(found_str); qdict_put_str(options, "pool", found_str); - if (strchr(p, '@')) { + if (qemu_rbd_strchr(p, '@')) { image_name = qemu_rbd_next_tok(p, '@', &p); found_str = qemu_rbd_next_tok(p, ':', &p); @@ -181,7 +191,7 @@ static void qemu_rbd_parse_filename(const char *filename, QDict *options, image_name = qemu_rbd_next_tok(p, ':', &p); } /* Check for namespace in the image_name */ - if (strchr(image_name, '/')) { + if (qemu_rbd_strchr(image_name, '/')) { found_str = qemu_rbd_next_tok(image_name, '/', &image_name); qemu_rbd_unescape(found_str); qdict_put_str(options, "namespace", found_str); diff --git a/tests/qemu-iotests/231 b/tests/qemu-iotests/231 index 0f66d0ca36..8e6c6447c1 100755 --- a/tests/qemu-iotests/231 +++ b/tests/qemu-iotests/231 @@ -55,6 +55,10 @@ _filter_conf() $QEMU_IMG info "json:{'file.driver':'rbd','file.filename':'rbd:rbd/bogus:conf=${BOGUS_CONF}'}" 2>&1 | _filter_conf $QEMU_IMG info "json:{'file.driver':'rbd','file.pool':'rbd','file.image':'bogus','file.conf':'${BOGUS_CONF}'}" 2>&1 | _filter_conf +# Regression test: the qemu-img invocation is expected to fail, but it should +# not seg fault the parser. +$QEMU_IMG create "rbd:rbd/aa\/bb:conf=${BOGUS_CONF}" 1M 2>&1 | _filter_conf + # success, all done echo "*** done" rm -f $seq.full diff --git a/tests/qemu-iotests/231.out b/tests/qemu-iotests/231.out index 747dd221bb..a785a6e859 100644 --- a/tests/qemu-iotests/231.out +++ b/tests/qemu-iotests/231.out @@ -4,4 +4,7 @@ unable to get monitor info from DNS SRV with service name: ceph-mon qemu-img: Could not open 'json:{'file.driver':'rbd','file.filename':'rbd:rbd/bogus:conf=BOGUS_CONF'}': error connecting: No such file or directory unable to get monitor info from DNS SRV with service name: ceph-mon qemu-img: Could not open 'json:{'file.driver':'rbd','file.pool':'rbd','file.image':'bogus','file.conf':'BOGUS_CONF'}': error connecting: No such file or directory +Formatting 'rbd:rbd/aa\/bb:conf=BOGUS_CONF', fmt=raw size=1048576 +unable to get monitor info from DNS SRV with service name: ceph-mon +qemu-img: rbd:rbd/aa\/bb:conf=BOGUS_CONF: error connecting: No such file or directory *** done From 78632a3d1685454fec83f83586c675e7f2a1a84a Mon Sep 17 00:00:00 2001 From: Vladimir Sementsov-Ogievskiy Date: Fri, 23 Apr 2021 16:42:33 +0300 Subject: [PATCH 03/19] monitor: hmp_qemu_io: acquire aio contex, fix crash Max reported the following bug: $ ./qemu-img create -f raw src.img 1G $ ./qemu-img create -f raw dst.img 1G $ (echo ' {"execute":"qmp_capabilities"} {"execute":"blockdev-mirror", "arguments":{"job-id":"mirror", "device":"source", "target":"target", "sync":"full", "filter-node-name":"mirror-top"}} '; sleep 3; echo ' {"execute":"human-monitor-command", "arguments":{"command-line": "qemu-io mirror-top \"write 0 1G\""}}') \ | x86_64-softmmu/qemu-system-x86_64 \ -qmp stdio \ -blockdev file,node-name=source,filename=src.img \ -blockdev file,node-name=target,filename=dst.img \ -object iothread,id=iothr0 \ -device virtio-blk,drive=source,iothread=iothr0 crashes: 0 raise () at /usr/lib/libc.so.6 1 abort () at /usr/lib/libc.so.6 2 error_exit (err=, msg=msg@entry=0x55fbb1634790 <__func__.27> "qemu_mutex_unlock_impl") at ../util/qemu-thread-posix.c:37 3 qemu_mutex_unlock_impl (mutex=mutex@entry=0x55fbb25ab6e0, file=file@entry=0x55fbb1636957 "../util/async.c", line=line@entry=650) at ../util/qemu-thread-posix.c:109 4 aio_context_release (ctx=ctx@entry=0x55fbb25ab680) at ../util/async.c:650 5 bdrv_do_drained_begin (bs=bs@entry=0x55fbb3a87000, recursive=recursive@entry=false, parent=parent@entry=0x0, ignore_bds_parents=ignore_bds_parents@entry=false, poll=poll@entry=true) at ../block/io.c:441 6 bdrv_do_drained_begin (poll=true, ignore_bds_parents=false, parent=0x0, recursive=false, bs=0x55fbb3a87000) at ../block/io.c:448 7 blk_drain (blk=0x55fbb26c5a00) at ../block/block-backend.c:1718 8 blk_unref (blk=0x55fbb26c5a00) at ../block/block-backend.c:498 9 blk_unref (blk=0x55fbb26c5a00) at ../block/block-backend.c:491 10 hmp_qemu_io (mon=0x7fffaf3fc7d0, qdict=) at ../block/monitor/block-hmp-cmds.c:628 man pthread_mutex_unlock ... EPERM The mutex type is PTHREAD_MUTEX_ERRORCHECK or PTHREAD_MUTEX_RECURSIVE, or the mutex is a robust mutex, and the current thread does not own the mutex. So, thread doesn't own the mutex. And we have iothread here. Next, note that AIO_WAIT_WHILE() documents that ctx must be acquired exactly once by caller. But where is it acquired in the call stack? Seems nowhere. qemuio_command do acquire aio context.. But we need context acquired around blk_unref() as well and actually around blk_insert_bs() too. Let's refactor qemuio_command so that it doesn't acquire aio context but callers do that instead. This way we can cleanly acquire aio context in hmp_qemu_io() around all three calls. Reported-by: Max Reitz Signed-off-by: Vladimir Sementsov-Ogievskiy Message-Id: <20210423134233.51495-1-vsementsov@virtuozzo.com> [mreitz: Fixed comment] Signed-off-by: Max Reitz --- block/monitor/block-hmp-cmds.c | 31 +++++++++++++++++++++---------- qemu-io-cmds.c | 8 ++++---- qemu-io.c | 17 +++++++++++++++-- 3 files changed, 40 insertions(+), 16 deletions(-) diff --git a/block/monitor/block-hmp-cmds.c b/block/monitor/block-hmp-cmds.c index ebf1033f31..3e6670c963 100644 --- a/block/monitor/block-hmp-cmds.c +++ b/block/monitor/block-hmp-cmds.c @@ -557,8 +557,10 @@ void hmp_eject(Monitor *mon, const QDict *qdict) void hmp_qemu_io(Monitor *mon, const QDict *qdict) { - BlockBackend *blk; + BlockBackend *blk = NULL; + BlockDriverState *bs = NULL; BlockBackend *local_blk = NULL; + AioContext *ctx = NULL; bool qdev = qdict_get_try_bool(qdict, "qdev", false); const char *device = qdict_get_str(qdict, "device"); const char *command = qdict_get_str(qdict, "command"); @@ -573,20 +575,24 @@ void hmp_qemu_io(Monitor *mon, const QDict *qdict) } else { blk = blk_by_name(device); if (!blk) { - BlockDriverState *bs = bdrv_lookup_bs(NULL, device, &err); - if (bs) { - blk = local_blk = blk_new(bdrv_get_aio_context(bs), - 0, BLK_PERM_ALL); - ret = blk_insert_bs(blk, bs, &err); - if (ret < 0) { - goto fail; - } - } else { + bs = bdrv_lookup_bs(NULL, device, &err); + if (!bs) { goto fail; } } } + ctx = blk ? blk_get_aio_context(blk) : bdrv_get_aio_context(bs); + aio_context_acquire(ctx); + + if (bs) { + blk = local_blk = blk_new(bdrv_get_aio_context(bs), 0, BLK_PERM_ALL); + ret = blk_insert_bs(blk, bs, &err); + if (ret < 0) { + goto fail; + } + } + /* * Notably absent: Proper permission management. This is sad, but it seems * almost impossible to achieve without changing the semantics and thereby @@ -616,6 +622,11 @@ void hmp_qemu_io(Monitor *mon, const QDict *qdict) fail: blk_unref(local_blk); + + if (ctx) { + aio_context_release(ctx); + } + hmp_handle_error(mon, err); } diff --git a/qemu-io-cmds.c b/qemu-io-cmds.c index 97611969cb..998b67186d 100644 --- a/qemu-io-cmds.c +++ b/qemu-io-cmds.c @@ -2457,9 +2457,12 @@ static const cmdinfo_t help_cmd = { .oneline = "help for one or all commands", }; +/* + * Called with aio context of blk acquired. Or with qemu_get_aio_context() + * context acquired if blk is NULL. + */ int qemuio_command(BlockBackend *blk, const char *cmd) { - AioContext *ctx; char *input; const cmdinfo_t *ct; char **v; @@ -2471,10 +2474,7 @@ int qemuio_command(BlockBackend *blk, const char *cmd) if (c) { ct = find_command(v[0]); if (ct) { - ctx = blk ? blk_get_aio_context(blk) : qemu_get_aio_context(); - aio_context_acquire(ctx); ret = command(blk, ct, c, v); - aio_context_release(ctx); } else { fprintf(stderr, "command \"%s\" not found\n", v[0]); ret = -EINVAL; diff --git a/qemu-io.c b/qemu-io.c index bf902302e9..57f07501df 100644 --- a/qemu-io.c +++ b/qemu-io.c @@ -411,6 +411,19 @@ static void prep_fetchline(void *opaque) *fetchable= 1; } +static int do_qemuio_command(const char *cmd) +{ + int ret; + AioContext *ctx = + qemuio_blk ? blk_get_aio_context(qemuio_blk) : qemu_get_aio_context(); + + aio_context_acquire(ctx); + ret = qemuio_command(qemuio_blk, cmd); + aio_context_release(ctx); + + return ret; +} + static int command_loop(void) { int i, fetchable = 0, prompted = 0; @@ -418,7 +431,7 @@ static int command_loop(void) char *input; for (i = 0; !quit_qemu_io && i < ncmdline; i++) { - ret = qemuio_command(qemuio_blk, cmdline[i]); + ret = do_qemuio_command(cmdline[i]); if (ret < 0) { last_error = ret; } @@ -446,7 +459,7 @@ static int command_loop(void) if (input == NULL) { break; } - ret = qemuio_command(qemuio_blk, input); + ret = do_qemuio_command(input); g_free(input); if (ret < 0) { From 9c785cd714b3c368503ba3aed4266a77056cae29 Mon Sep 17 00:00:00 2001 From: Vladimir Sementsov-Ogievskiy Date: Wed, 21 Apr 2021 10:58:58 +0300 Subject: [PATCH 04/19] mirror: stop cancelling in-flight requests on non-force cancel in READY If mirror is READY than cancel operation is not discarding the whole result of the operation, but instead it's a documented way get a point-in-time snapshot of source disk. So, we should not cancel any requests if mirror is READ and force=false. Let's fix that case. Note, that bug that we have before this commit is not critical, as the only .bdrv_cancel_in_flight implementation is nbd_cancel_in_flight() and it cancels only requests waiting for reconnection, so it should be rare case. Fixes: 521ff8b779b11c394dbdc43f02e158dd99df308a Signed-off-by: Vladimir Sementsov-Ogievskiy Message-Id: <20210421075858.40197-1-vsementsov@virtuozzo.com> Signed-off-by: Max Reitz --- block/backup.c | 2 +- block/mirror.c | 6 ++++-- include/block/block_int.h | 2 +- include/qemu/job.h | 2 +- job.c | 2 +- tests/qemu-iotests/264 | 2 +- 6 files changed, 9 insertions(+), 7 deletions(-) diff --git a/block/backup.c b/block/backup.c index 6cf2f974aa..bd3614ce70 100644 --- a/block/backup.c +++ b/block/backup.c @@ -331,7 +331,7 @@ static void coroutine_fn backup_set_speed(BlockJob *job, int64_t speed) } } -static void backup_cancel(Job *job) +static void backup_cancel(Job *job, bool force) { BackupBlockJob *s = container_of(job, BackupBlockJob, common.job); diff --git a/block/mirror.c b/block/mirror.c index 840b8e8c15..019f6deaa5 100644 --- a/block/mirror.c +++ b/block/mirror.c @@ -1178,12 +1178,14 @@ static bool mirror_drained_poll(BlockJob *job) return !!s->in_flight; } -static void mirror_cancel(Job *job) +static void mirror_cancel(Job *job, bool force) { MirrorBlockJob *s = container_of(job, MirrorBlockJob, common.job); BlockDriverState *target = blk_bs(s->target); - bdrv_cancel_in_flight(target); + if (force || !job_is_ready(job)) { + bdrv_cancel_in_flight(target); + } } static const BlockJobDriver mirror_job_driver = { diff --git a/include/block/block_int.h b/include/block/block_int.h index c823f5b1b3..731ffedb27 100644 --- a/include/block/block_int.h +++ b/include/block/block_int.h @@ -357,7 +357,7 @@ struct BlockDriver { * of in-flight requests, so don't waste the time if possible. * * One example usage is to avoid waiting for an nbd target node reconnect - * timeout during job-cancel. + * timeout during job-cancel with force=true. */ void (*bdrv_cancel_in_flight)(BlockDriverState *bs); diff --git a/include/qemu/job.h b/include/qemu/job.h index efc6fa7544..41162ed494 100644 --- a/include/qemu/job.h +++ b/include/qemu/job.h @@ -254,7 +254,7 @@ struct JobDriver { /** * If the callback is not NULL, it will be invoked in job_cancel_async */ - void (*cancel)(Job *job); + void (*cancel)(Job *job, bool force); /** Called when the job is freed */ diff --git a/job.c b/job.c index 4aff13d95a..8775c1803b 100644 --- a/job.c +++ b/job.c @@ -716,7 +716,7 @@ static int job_finalize_single(Job *job) static void job_cancel_async(Job *job, bool force) { if (job->driver->cancel) { - job->driver->cancel(job); + job->driver->cancel(job, force); } if (job->user_paused) { /* Do not call job_enter here, the caller will handle it. */ diff --git a/tests/qemu-iotests/264 b/tests/qemu-iotests/264 index 4f96825a22..bc431d1a19 100755 --- a/tests/qemu-iotests/264 +++ b/tests/qemu-iotests/264 @@ -95,7 +95,7 @@ class TestNbdReconnect(iotests.QMPTestCase): self.assert_qmp(result, 'return', {}) def cancel_job(self): - result = self.vm.qmp('block-job-cancel', device='drive0') + result = self.vm.qmp('block-job-cancel', device='drive0', force=True) self.assert_qmp(result, 'return', {}) start_t = time.time() From f29f4c25eb9f11d78d62185b69456681f9c703b2 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Mon, 3 May 2021 13:01:06 +0200 Subject: [PATCH 05/19] qemu-iotests: do not buffer the test output Instead of buffering the test output into a StringIO, patch it on the fly by wrapping sys.stdout's write method. This can be done unconditionally, even if using -d, which makes execute_unittest a bit simpler. Signed-off-by: Paolo Bonzini Reviewed-by: Vladimir Sementsov-Ogievskiy Tested-by: Emanuele Giuseppe Esposito Message-Id: <20210323181928.311862-2-pbonzini@redhat.com> Message-Id: <20210503110110.476887-2-pbonzini@redhat.com> Signed-off-by: Max Reitz --- tests/qemu-iotests/240.out | 8 ++-- tests/qemu-iotests/245.out | 8 ++-- tests/qemu-iotests/295.out | 6 +-- tests/qemu-iotests/296.out | 8 ++-- tests/qemu-iotests/iotests.py | 70 ++++++++++++++++++++--------------- 5 files changed, 56 insertions(+), 44 deletions(-) diff --git a/tests/qemu-iotests/240.out b/tests/qemu-iotests/240.out index e0982831ae..89ed25e506 100644 --- a/tests/qemu-iotests/240.out +++ b/tests/qemu-iotests/240.out @@ -15,7 +15,7 @@ {"return": {}} {"execute": "blockdev-del", "arguments": {"node-name": "hd0"}} {"return": {}} -==Attach two SCSI disks using the same block device and the same iothread== +.==Attach two SCSI disks using the same block device and the same iothread== {"execute": "blockdev-add", "arguments": {"driver": "null-co", "node-name": "hd0", "read-only": true, "read-zeroes": true}} {"return": {}} {"execute": "object-add", "arguments": {"id": "iothread0", "qom-type": "iothread"}} @@ -32,7 +32,7 @@ {"return": {}} {"execute": "blockdev-del", "arguments": {"node-name": "hd0"}} {"return": {}} -==Attach two SCSI disks using the same block device but different iothreads== +.==Attach two SCSI disks using the same block device but different iothreads== {"execute": "blockdev-add", "arguments": {"driver": "null-co", "node-name": "hd0", "read-only": true, "read-zeroes": true}} {"return": {}} {"execute": "object-add", "arguments": {"id": "iothread0", "qom-type": "iothread"}} @@ -55,7 +55,7 @@ {"return": {}} {"execute": "blockdev-del", "arguments": {"node-name": "hd0"}} {"return": {}} -==Attach a SCSI disks using the same block device as a NBD server== +.==Attach a SCSI disks using the same block device as a NBD server== {"execute": "blockdev-add", "arguments": {"driver": "null-co", "node-name": "hd0", "read-only": true, "read-zeroes": true}} {"return": {}} {"execute": "nbd-server-start", "arguments": {"addr": {"data": {"path": "SOCK_DIR/PID-nbd.sock"}, "type": "unix"}}} @@ -68,7 +68,7 @@ {"return": {}} {"execute": "device_add", "arguments": {"drive": "hd0", "driver": "scsi-hd", "id": "scsi-hd0"}} {"return": {}} -.... +. ---------------------------------------------------------------------- Ran 4 tests diff --git a/tests/qemu-iotests/245.out b/tests/qemu-iotests/245.out index 4b33dcaf5c..99c12f4f98 100644 --- a/tests/qemu-iotests/245.out +++ b/tests/qemu-iotests/245.out @@ -1,16 +1,16 @@ -{"execute": "job-finalize", "arguments": {"id": "commit0"}} +..{"execute": "job-finalize", "arguments": {"id": "commit0"}} {"return": {}} {"data": {"id": "commit0", "type": "commit"}, "event": "BLOCK_JOB_PENDING", "timestamp": {"microseconds": "USECS", "seconds": "SECS"}} {"data": {"device": "commit0", "len": 3145728, "offset": 3145728, "speed": 0, "type": "commit"}, "event": "BLOCK_JOB_COMPLETED", "timestamp": {"microseconds": "USECS", "seconds": "SECS"}} -{"execute": "job-finalize", "arguments": {"id": "stream0"}} +...{"execute": "job-finalize", "arguments": {"id": "stream0"}} {"return": {}} {"data": {"id": "stream0", "type": "stream"}, "event": "BLOCK_JOB_PENDING", "timestamp": {"microseconds": "USECS", "seconds": "SECS"}} {"data": {"device": "stream0", "len": 3145728, "offset": 3145728, "speed": 0, "type": "stream"}, "event": "BLOCK_JOB_COMPLETED", "timestamp": {"microseconds": "USECS", "seconds": "SECS"}} -{"execute": "job-finalize", "arguments": {"id": "stream0"}} +.{"execute": "job-finalize", "arguments": {"id": "stream0"}} {"return": {}} {"data": {"id": "stream0", "type": "stream"}, "event": "BLOCK_JOB_PENDING", "timestamp": {"microseconds": "USECS", "seconds": "SECS"}} {"data": {"device": "stream0", "len": 3145728, "offset": 3145728, "speed": 0, "type": "stream"}, "event": "BLOCK_JOB_COMPLETED", "timestamp": {"microseconds": "USECS", "seconds": "SECS"}} -..................... +............... ---------------------------------------------------------------------- Ran 21 tests diff --git a/tests/qemu-iotests/295.out b/tests/qemu-iotests/295.out index ad34b2ca2c..5ff91f116c 100644 --- a/tests/qemu-iotests/295.out +++ b/tests/qemu-iotests/295.out @@ -4,7 +4,7 @@ {"return": {}} {"execute": "job-dismiss", "arguments": {"id": "job_erase_key"}} {"return": {}} -{"execute": "job-dismiss", "arguments": {"id": "job_add_key"}} +.{"execute": "job-dismiss", "arguments": {"id": "job_add_key"}} {"return": {}} {"execute": "job-dismiss", "arguments": {"id": "job_erase_key"}} {"return": {}} @@ -13,7 +13,7 @@ Job failed: Invalid password, cannot unlock any keyslot {"return": {}} {"execute": "job-dismiss", "arguments": {"id": "job_add_key"}} {"return": {}} -{"execute": "job-dismiss", "arguments": {"id": "job_add_key"}} +.{"execute": "job-dismiss", "arguments": {"id": "job_add_key"}} {"return": {}} {"execute": "job-dismiss", "arguments": {"id": "job_add_key"}} {"return": {}} @@ -33,7 +33,7 @@ Job failed: All the active keyslots match the (old) password that was given and {"return": {}} {"execute": "job-dismiss", "arguments": {"id": "job_erase_key"}} {"return": {}} -... +. ---------------------------------------------------------------------- Ran 3 tests diff --git a/tests/qemu-iotests/296.out b/tests/qemu-iotests/296.out index cb2859a15c..6c69735604 100644 --- a/tests/qemu-iotests/296.out +++ b/tests/qemu-iotests/296.out @@ -13,7 +13,7 @@ Job failed: Failed to get shared "consistent read" lock qemu-img: Failed to get shared "consistent read" lock Is another process using the image [TEST_DIR/test.img]? -Formatting 'TEST_DIR/test.img', fmt=luks size=1048576 key-secret=keysec0 iter-time=10 +.Formatting 'TEST_DIR/test.img', fmt=luks size=1048576 key-secret=keysec0 iter-time=10 Job failed: Block node is read-only {"execute": "job-dismiss", "arguments": {"id": "job0"}} @@ -26,15 +26,15 @@ Job failed: Failed to get shared "consistent read" lock {"return": {}} {"execute": "job-dismiss", "arguments": {"id": "job0"}} {"return": {}} -Formatting 'TEST_DIR/test.img', fmt=luks size=1048576 key-secret=keysec0 iter-time=10 +.Formatting 'TEST_DIR/test.img', fmt=luks size=1048576 key-secret=keysec0 iter-time=10 {"return": {}} {"error": {"class": "GenericError", "desc": "Failed to get \"write\" lock"}} -Formatting 'TEST_DIR/test.img', fmt=luks size=1048576 key-secret=keysec0 iter-time=10 +.Formatting 'TEST_DIR/test.img', fmt=luks size=1048576 key-secret=keysec0 iter-time=10 {"return": {}} {"return": {}} -.... +. ---------------------------------------------------------------------- Ran 4 tests diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py index 5af0182895..55a017577f 100644 --- a/tests/qemu-iotests/iotests.py +++ b/tests/qemu-iotests/iotests.py @@ -20,7 +20,6 @@ import atexit import bz2 from collections import OrderedDict import faulthandler -import io import json import logging import os @@ -32,7 +31,7 @@ import subprocess import sys import time from typing import (Any, Callable, Dict, Iterable, - List, Optional, Sequence, Tuple, TypeVar) + List, Optional, Sequence, TextIO, Tuple, Type, TypeVar) import unittest from contextlib import contextmanager @@ -1271,37 +1270,50 @@ def skip_if_user_is_root(func): return func(*args, **kwargs) return func_wrapper +# We need to filter out the time taken from the output so that +# qemu-iotest can reliably diff the results against master output, +# and hide skipped tests from the reference output. + +class ReproducibleTestResult(unittest.TextTestResult): + def addSkip(self, test, reason): + # Same as TextTestResult, but print dot instead of "s" + unittest.TestResult.addSkip(self, test, reason) + if self.showAll: + self.stream.writeln("skipped {0!r}".format(reason)) + elif self.dots: + self.stream.write(".") + self.stream.flush() + +class ReproducibleStreamWrapper: + def __init__(self, stream: TextIO): + self.stream = stream + + def __getattr__(self, attr): + if attr in ('stream', '__getstate__'): + raise AttributeError(attr) + return getattr(self.stream, attr) + + def write(self, arg=None): + arg = re.sub(r'Ran (\d+) tests? in [\d.]+s', r'Ran \1 tests', arg) + arg = re.sub(r' \(skipped=\d+\)', r'', arg) + self.stream.write(arg) + +class ReproducibleTestRunner(unittest.TextTestRunner): + def __init__(self, stream: Optional[TextIO] = None, + resultclass: Type[unittest.TestResult] = ReproducibleTestResult, + **kwargs: Any) -> None: + rstream = ReproducibleStreamWrapper(stream or sys.stdout) + super().__init__(stream=rstream, # type: ignore + descriptions=True, + resultclass=resultclass, + **kwargs) + def execute_unittest(debug=False): """Executes unittests within the calling module.""" verbosity = 2 if debug else 1 - - if debug: - output = sys.stdout - else: - # We need to filter out the time taken from the output so that - # qemu-iotest can reliably diff the results against master output. - output = io.StringIO() - - runner = unittest.TextTestRunner(stream=output, descriptions=True, - verbosity=verbosity) - try: - # unittest.main() will use sys.exit(); so expect a SystemExit - # exception - unittest.main(testRunner=runner) - finally: - # We need to filter out the time taken from the output so that - # qemu-iotest can reliably diff the results against master output. - if not debug: - out = output.getvalue() - out = re.sub(r'Ran (\d+) tests? in [\d.]+s', r'Ran \1 tests', out) - - # Hide skipped tests from the reference output - out = re.sub(r'OK \(skipped=\d+\)', 'OK', out) - out_first_line, out_rest = out.split('\n', 1) - out = out_first_line.replace('s', '.') + '\n' + out_rest - - sys.stderr.write(out) + runner = ReproducibleTestRunner(verbosity=verbosity) + unittest.main(testRunner=runner) def execute_setup_common(supported_fmts: Sequence[str] = (), supported_platforms: Sequence[str] = (), From 00dbc85e0efd863498d52408b248039816c10532 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Mon, 3 May 2021 13:01:07 +0200 Subject: [PATCH 06/19] qemu-iotests: allow passing unittest.main arguments to the test scripts Python test scripts that use unittest consist of multiple tests. unittest.main allows selecting which tests to run, but currently this is not possible because the iotests wrapper ignores sys.argv. unittest.main command line options also allow the user to pick the desired options for verbosity, failfast mode, etc. While "-d" is currently translated to "-v", it also enables extra debug output, and other options are not available at all. These command line options only work if the unittest.main testRunner argument is a type, rather than a TestRunner instance. Therefore, pass the class name and "verbosity" argument to unittest.main, and adjust for the different default warnings between TextTestRunner and unittest.main. Signed-off-by: Paolo Bonzini Reviewed-by: Vladimir Sementsov-Ogievskiy Tested-by: Emanuele Giuseppe Esposito Message-Id: <20210323181928.311862-3-pbonzini@redhat.com> Message-Id: <20210503110110.476887-3-pbonzini@redhat.com> Signed-off-by: Max Reitz --- tests/qemu-iotests/iotests.py | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py index 55a017577f..5ead94229f 100644 --- a/tests/qemu-iotests/iotests.py +++ b/tests/qemu-iotests/iotests.py @@ -1308,12 +1308,16 @@ class ReproducibleTestRunner(unittest.TextTestRunner): resultclass=resultclass, **kwargs) -def execute_unittest(debug=False): +def execute_unittest(argv: List[str], debug: bool = False) -> None: """Executes unittests within the calling module.""" - verbosity = 2 if debug else 1 - runner = ReproducibleTestRunner(verbosity=verbosity) - unittest.main(testRunner=runner) + # Some tests have warnings, especially ResourceWarnings for unclosed + # files and sockets. Ignore them for now to ensure reproducibility of + # the test output. + unittest.main(argv=argv, + testRunner=ReproducibleTestRunner, + verbosity=2 if debug else 1, + warnings=None if sys.warnoptions else 'ignore') def execute_setup_common(supported_fmts: Sequence[str] = (), supported_platforms: Sequence[str] = (), @@ -1350,7 +1354,7 @@ def execute_test(*args, test_function=None, **kwargs): debug = execute_setup_common(*args, **kwargs) if not test_function: - execute_unittest(debug) + execute_unittest(sys.argv, debug) else: test_function() From c64430d2386d9968342a8e1ae00ed34ff0b98bbb Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Mon, 3 May 2021 13:01:08 +0200 Subject: [PATCH 07/19] qemu-iotests: move command line and environment handling from TestRunner to TestEnv In the next patch, "check" will learn how to execute a test script without going through TestRunner. To enable this, keep only the text output and subprocess handling in the TestRunner; move into TestEnv the logic to prepare for running a subprocess. Reviewed-by: Vladimir Sementsov-Ogievskiy Signed-off-by: Paolo Bonzini Tested-by: Emanuele Giuseppe Esposito Message-Id: <20210323181928.311862-4-pbonzini@redhat.com> Message-Id: <20210503110110.476887-4-pbonzini@redhat.com> Signed-off-by: Max Reitz --- tests/qemu-iotests/testenv.py | 17 ++++++++++++++++- tests/qemu-iotests/testrunner.py | 14 +------------- 2 files changed, 17 insertions(+), 14 deletions(-) diff --git a/tests/qemu-iotests/testenv.py b/tests/qemu-iotests/testenv.py index 6d27712617..fca3a609e0 100644 --- a/tests/qemu-iotests/testenv.py +++ b/tests/qemu-iotests/testenv.py @@ -25,7 +25,7 @@ import collections import random import subprocess import glob -from typing import Dict, Any, Optional, ContextManager +from typing import List, Dict, Any, Optional, ContextManager def isxfile(path: str) -> bool: @@ -74,6 +74,21 @@ class TestEnv(ContextManager['TestEnv']): 'CACHEMODE_IS_DEFAULT', 'IMGFMT_GENERIC', 'IMGOPTSSYNTAX', 'IMGKEYSECRET', 'QEMU_DEFAULT_MACHINE', 'MALLOC_PERTURB_'] + def prepare_subprocess(self, args: List[str]) -> Dict[str, str]: + if self.debug: + args.append('-d') + + with open(args[0], encoding="utf-8") as f: + try: + if f.readline().rstrip() == '#!/usr/bin/env python3': + args.insert(0, self.python) + except UnicodeDecodeError: # binary test? for future. + pass + + os_env = os.environ.copy() + os_env.update(self.get_env()) + return os_env + def get_env(self) -> Dict[str, str]: env = {} for v in self.env_variables: diff --git a/tests/qemu-iotests/testrunner.py b/tests/qemu-iotests/testrunner.py index 1fc61fcaa3..519924dc81 100644 --- a/tests/qemu-iotests/testrunner.py +++ b/tests/qemu-iotests/testrunner.py @@ -129,7 +129,6 @@ class TestRunner(ContextManager['TestRunner']): def __init__(self, env: TestEnv, makecheck: bool = False, color: str = 'auto') -> None: self.env = env - self.test_run_env = self.env.get_env() self.makecheck = makecheck self.last_elapsed = LastElapsedTime('.last-elapsed-cache', env) @@ -243,18 +242,7 @@ class TestRunner(ContextManager['TestRunner']): silent_unlink(p) args = [str(f_test.resolve())] - if self.env.debug: - args.append('-d') - - with f_test.open(encoding="utf-8") as f: - try: - if f.readline().rstrip() == '#!/usr/bin/env python3': - args.insert(0, self.env.python) - except UnicodeDecodeError: # binary test? for future. - pass - - env = os.environ.copy() - env.update(self.test_run_env) + env = self.env.prepare_subprocess(args) t0 = time.time() with f_bad.open('w', encoding="utf-8") as f: From 480b75ee1423ee6d8aba59cb8090d60eb97676ff Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Mon, 3 May 2021 13:01:09 +0200 Subject: [PATCH 08/19] qemu-iotests: let "check" spawn an arbitrary test command Right now there is no easy way for "check" to print a reproducer command. Because such a reproducer command line would be huge, we can instead teach check to start a command of our choice. This can be for example a Python unit test with arguments to only run a specific subtest. Move the trailing empty line to print_env(), since it always looks better and one caller was not adding it. Signed-off-by: Paolo Bonzini Reviewed-by: Vladimir Sementsov-Ogievskiy Tested-by: Emanuele Giuseppe Esposito Message-Id: <20210323181928.311862-5-pbonzini@redhat.com> Message-Id: <20210503110110.476887-5-pbonzini@redhat.com> Signed-off-by: Max Reitz --- tests/qemu-iotests/check | 19 ++++++++++++++++++- tests/qemu-iotests/testenv.py | 3 ++- tests/qemu-iotests/testrunner.py | 1 - 3 files changed, 20 insertions(+), 3 deletions(-) diff --git a/tests/qemu-iotests/check b/tests/qemu-iotests/check index 08f51366f1..2dd529eb75 100755 --- a/tests/qemu-iotests/check +++ b/tests/qemu-iotests/check @@ -19,6 +19,9 @@ import os import sys import argparse +import shutil +from pathlib import Path + from findtests import TestFinder from testenv import TestEnv from testrunner import TestRunner @@ -100,7 +103,7 @@ def make_argparser() -> argparse.ArgumentParser: 'rerun failed ./check command, starting from the ' 'middle of the process.') g_sel.add_argument('tests', metavar='TEST_FILES', nargs='*', - help='tests to run') + help='tests to run, or "--" followed by a command') return p @@ -113,6 +116,20 @@ if __name__ == '__main__': imgopts=args.imgopts, misalign=args.misalign, debug=args.debug, valgrind=args.valgrind) + if len(sys.argv) > 1 and sys.argv[-len(args.tests)-1] == '--': + if not args.tests: + sys.exit("missing command after '--'") + cmd = args.tests + env.print_env() + exec_pathstr = shutil.which(cmd[0]) + if exec_pathstr is None: + sys.exit('command not found: ' + cmd[0]) + exec_path = Path(exec_pathstr).resolve() + cmd[0] = str(exec_path) + full_env = env.prepare_subprocess(cmd) + os.chdir(exec_path.parent) + os.execve(cmd[0], cmd, full_env) + testfinder = TestFinder(test_dir=env.source_iotests) groups = args.groups.split(',') if args.groups else None diff --git a/tests/qemu-iotests/testenv.py b/tests/qemu-iotests/testenv.py index fca3a609e0..cd0e39b789 100644 --- a/tests/qemu-iotests/testenv.py +++ b/tests/qemu-iotests/testenv.py @@ -284,7 +284,8 @@ IMGPROTO -- {IMGPROTO} PLATFORM -- {platform} TEST_DIR -- {TEST_DIR} SOCK_DIR -- {SOCK_DIR} -SOCKET_SCM_HELPER -- {SOCKET_SCM_HELPER}""" +SOCKET_SCM_HELPER -- {SOCKET_SCM_HELPER} +""" args = collections.defaultdict(str, self.get_env()) diff --git a/tests/qemu-iotests/testrunner.py b/tests/qemu-iotests/testrunner.py index 519924dc81..2f56ac545d 100644 --- a/tests/qemu-iotests/testrunner.py +++ b/tests/qemu-iotests/testrunner.py @@ -316,7 +316,6 @@ class TestRunner(ContextManager['TestRunner']): if not self.makecheck: self.env.print_env() - print() test_field_width = max(len(os.path.basename(t)) for t in tests) + 2 From c3d479aab9d72fa0d6a3aafdaf0729140e6eae51 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Mon, 3 May 2021 13:01:10 +0200 Subject: [PATCH 09/19] qemu-iotests: fix case of SOCK_DIR already in the environment Due to a typo, in this case the SOCK_DIR was not being created. Reviewed-by: Vladimir Sementsov-Ogievskiy Signed-off-by: Paolo Bonzini Tested-by: Emanuele Giuseppe Esposito Message-Id: <20210323181928.311862-6-pbonzini@redhat.com> Message-Id: <20210503110110.476887-6-pbonzini@redhat.com> Signed-off-by: Max Reitz --- tests/qemu-iotests/testenv.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/qemu-iotests/testenv.py b/tests/qemu-iotests/testenv.py index cd0e39b789..0c3fe75636 100644 --- a/tests/qemu-iotests/testenv.py +++ b/tests/qemu-iotests/testenv.py @@ -120,7 +120,7 @@ class TestEnv(ContextManager['TestEnv']): try: self.sock_dir = os.environ['SOCK_DIR'] self.tmp_sock_dir = False - Path(self.test_dir).mkdir(parents=True, exist_ok=True) + Path(self.sock_dir).mkdir(parents=True, exist_ok=True) except KeyError: self.sock_dir = tempfile.mkdtemp() self.tmp_sock_dir = True From d65173f924ef0a170693fc59692a542d38f31879 Mon Sep 17 00:00:00 2001 From: Connor Kuehl Date: Wed, 5 May 2021 14:55:12 -0500 Subject: [PATCH 10/19] Document qemu-img options data_file and data_file_raw The contents of this patch were initially developed and posted by Han Han[1], however, it appears the original patch was not applied. Since then, the relevant documentation has been moved and adapted to a new format. I've taken most of the original wording and tweaked it according to some of the feedback from the original patch submission. I've also adapted it to restructured text, which is the format the documentation currently uses. [1] https://lists.nongnu.org/archive/html/qemu-block/2019-10/msg01253.html Fixes: https://bugzilla.redhat.com/1763105 Signed-off-by: Han Han Suggested-by: Max Reitz [ Max: provided description of data_file_raw behavior ] Signed-off-by: Connor Kuehl Message-Id: <20210505195512.391128-1-ckuehl@redhat.com> Signed-off-by: Max Reitz --- docs/tools/qemu-img.rst | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/docs/tools/qemu-img.rst b/docs/tools/qemu-img.rst index c9efcfaefc..cfe1147879 100644 --- a/docs/tools/qemu-img.rst +++ b/docs/tools/qemu-img.rst @@ -866,6 +866,37 @@ Supported image file formats: issue ``lsattr filename`` to check if the NOCOW flag is set or not (Capital 'C' is NOCOW flag). + ``data_file`` + Filename where all guest data will be stored. If this option is used, + the qcow2 file will only contain the image's metadata. + + Note: Data loss will occur if the given filename already exists when + using this option with ``qemu-img create`` since ``qemu-img`` will create + the data file anew, overwriting the file's original contents. To simply + update the reference to point to the given pre-existing file, use + ``qemu-img amend``. + + ``data_file_raw`` + If this option is set to ``on``, QEMU will always keep the external data + file consistent as a standalone read-only raw image. + + It does this by forwarding all write accesses to the qcow2 file through to + the raw data file, including their offsets. Therefore, data that is visible + on the qcow2 node (i.e., to the guest) at some offset is visible at the same + offset in the raw data file. This results in a read-only raw image. Writes + that bypass the qcow2 metadata may corrupt the qcow2 metadata because the + out-of-band writes may result in the metadata falling out of sync with the + raw image. + + If this option is ``off``, QEMU will use the data file to store data in an + arbitrary manner. The file’s content will not make sense without the + accompanying qcow2 metadata. Where data is written will have no relation to + its offset as seen by the guest, and some writes (specifically zero writes) + may not be forwarded to the data file at all, but will only be handled by + modifying qcow2 metadata. + + This option can only be enabled if ``data_file`` is set. + ``Other`` QEMU also supports various other image file formats for From bcc8584c832f7a52fd8c64483dd452c1baf01db7 Mon Sep 17 00:00:00 2001 From: Vladimir Sementsov-Ogievskiy Date: Thu, 6 May 2021 22:41:43 +0300 Subject: [PATCH 11/19] block/copy-on-read: use bdrv_drop_filter() and drop s->active Now, after huge update of block graph permission update algorithm, we don't need this workaround with active state of the filter. Drop it and use new smart bdrv_drop_filter() function. Signed-off-by: Vladimir Sementsov-Ogievskiy Message-Id: <20210506194143.394141-1-vsementsov@virtuozzo.com> Signed-off-by: Max Reitz --- block/copy-on-read.c | 33 +-------------------------------- 1 file changed, 1 insertion(+), 32 deletions(-) diff --git a/block/copy-on-read.c b/block/copy-on-read.c index 9cad9e1b8c..c428682272 100644 --- a/block/copy-on-read.c +++ b/block/copy-on-read.c @@ -29,7 +29,6 @@ typedef struct BDRVStateCOR { - bool active; BlockDriverState *bottom_bs; bool chain_frozen; } BDRVStateCOR; @@ -89,7 +88,6 @@ static int cor_open(BlockDriverState *bs, QDict *options, int flags, */ bdrv_ref(bottom_bs); } - state->active = true; state->bottom_bs = bottom_bs; /* @@ -112,17 +110,6 @@ static void cor_child_perm(BlockDriverState *bs, BdrvChild *c, uint64_t perm, uint64_t shared, uint64_t *nperm, uint64_t *nshared) { - BDRVStateCOR *s = bs->opaque; - - if (!s->active) { - /* - * While the filter is being removed - */ - *nperm = 0; - *nshared = BLK_PERM_ALL; - return; - } - *nperm = perm & PERM_PASSTHROUGH; *nshared = (shared & PERM_PASSTHROUGH) | PERM_UNCHANGED; @@ -280,32 +267,14 @@ static BlockDriver bdrv_copy_on_read = { void bdrv_cor_filter_drop(BlockDriverState *cor_filter_bs) { - BdrvChild *child; - BlockDriverState *bs; BDRVStateCOR *s = cor_filter_bs->opaque; - child = bdrv_filter_child(cor_filter_bs); - if (!child) { - return; - } - bs = child->bs; - - /* Retain the BDS until we complete the graph change. */ - bdrv_ref(bs); - /* Hold a guest back from writing while permissions are being reset. */ - bdrv_drained_begin(bs); - /* Drop permissions before the graph change. */ - s->active = false; /* unfreeze, as otherwise bdrv_replace_node() will fail */ if (s->chain_frozen) { s->chain_frozen = false; bdrv_unfreeze_backing_chain(cor_filter_bs, s->bottom_bs); } - bdrv_child_refresh_perms(cor_filter_bs, child, &error_abort); - bdrv_replace_node(cor_filter_bs, bs, &error_abort); - - bdrv_drained_end(bs); - bdrv_unref(bs); + bdrv_drop_filter(cor_filter_bs, &error_abort); bdrv_unref(cor_filter_bs); } From ac4e14f5dc93d6b4bc5d4849bb61810023330380 Mon Sep 17 00:00:00 2001 From: Emanuele Giuseppe Esposito Date: Mon, 10 May 2021 21:04:49 +0200 Subject: [PATCH 12/19] qemu-iotests: fix pylint 2.8 consider-using-with error pylint 2.8 introduces consider-using-with error, suggesting to use the 'with' block statement when possible. Modify all subprocess.Popen call to use the 'with' statement, except the one in __init__ of QemuIoInteractive class, since it is assigned to a class field and used in other methods. Signed-off-by: Emanuele Giuseppe Esposito Message-Id: <20210510190449.65948-1-eesposit@redhat.com> [mreitz: Disable bad-option-value warning in the iotests' pylintrc, so that disabling consider-using-with in QemuIoInteractive will not produce a warning in pre-2.8 pylint versions] Signed-off-by: Max Reitz --- tests/qemu-iotests/iotests.py | 63 ++++++++++++++++---------------- tests/qemu-iotests/pylintrc | 3 ++ tests/qemu-iotests/testrunner.py | 22 +++++------ 3 files changed, 46 insertions(+), 42 deletions(-) diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py index 5ead94229f..777fa2ec0e 100644 --- a/tests/qemu-iotests/iotests.py +++ b/tests/qemu-iotests/iotests.py @@ -112,15 +112,14 @@ def qemu_tool_pipe_and_status(tool: str, args: Sequence[str], Run a tool and return both its output and its exit code """ stderr = subprocess.STDOUT if connect_stderr else None - subp = subprocess.Popen(args, - stdout=subprocess.PIPE, - stderr=stderr, - universal_newlines=True) - output = subp.communicate()[0] - if subp.returncode < 0: - cmd = ' '.join(args) - sys.stderr.write(f'{tool} received signal {-subp.returncode}: {cmd}\n') - return (output, subp.returncode) + with subprocess.Popen(args, stdout=subprocess.PIPE, + stderr=stderr, universal_newlines=True) as subp: + output = subp.communicate()[0] + if subp.returncode < 0: + cmd = ' '.join(args) + sys.stderr.write(f'{tool} received signal \ + {-subp.returncode}: {cmd}\n') + return (output, subp.returncode) def qemu_img_pipe_and_status(*args: str) -> Tuple[str, int]: """ @@ -236,6 +235,9 @@ def qemu_io_silent_check(*args): class QemuIoInteractive: def __init__(self, *args): self.args = qemu_io_args_no_fmt + list(args) + # We need to keep the Popen objext around, and not + # close it immediately. Therefore, disable the pylint check: + # pylint: disable=consider-using-with self._p = subprocess.Popen(self.args, stdin=subprocess.PIPE, stdout=subprocess.PIPE, stderr=subprocess.STDOUT, @@ -309,22 +311,22 @@ def qemu_nbd_popen(*args): cmd.extend(args) log('Start NBD server') - p = subprocess.Popen(cmd) - try: - while not os.path.exists(pid_file): - if p.poll() is not None: - raise RuntimeError( - "qemu-nbd terminated with exit code {}: {}" - .format(p.returncode, ' '.join(cmd))) + with subprocess.Popen(cmd) as p: + try: + while not os.path.exists(pid_file): + if p.poll() is not None: + raise RuntimeError( + "qemu-nbd terminated with exit code {}: {}" + .format(p.returncode, ' '.join(cmd))) - time.sleep(0.01) - yield - finally: - if os.path.exists(pid_file): - os.remove(pid_file) - log('Kill NBD server') - p.kill() - p.wait() + time.sleep(0.01) + yield + finally: + if os.path.exists(pid_file): + os.remove(pid_file) + log('Kill NBD server') + p.kill() + p.wait() def compare_images(img1, img2, fmt1=imgfmt, fmt2=imgfmt): '''Return True if two image files are identical''' @@ -333,13 +335,12 @@ def compare_images(img1, img2, fmt1=imgfmt, fmt2=imgfmt): def create_image(name, size): '''Create a fully-allocated raw image with sector markers''' - file = open(name, 'wb') - i = 0 - while i < size: - sector = struct.pack('>l504xl', i // 512, i // 512) - file.write(sector) - i = i + 512 - file.close() + with open(name, 'wb') as file: + i = 0 + while i < size: + sector = struct.pack('>l504xl', i // 512, i // 512) + file.write(sector) + i = i + 512 def image_size(img): '''Return image's virtual size''' diff --git a/tests/qemu-iotests/pylintrc b/tests/qemu-iotests/pylintrc index 7a6c0a9474..f2c0b522ac 100644 --- a/tests/qemu-iotests/pylintrc +++ b/tests/qemu-iotests/pylintrc @@ -19,6 +19,9 @@ disable=invalid-name, too-many-public-methods, # pylint warns about Optional[] etc. as unsubscriptable in 3.9 unsubscriptable-object, + # Sometimes we need to disable a newly introduced pylint warning. + # Doing so should not produce a warning in older versions of pylint. + bad-option-value, # These are temporary, and should be removed: missing-docstring, too-many-return-statements, diff --git a/tests/qemu-iotests/testrunner.py b/tests/qemu-iotests/testrunner.py index 2f56ac545d..4a6ec421ed 100644 --- a/tests/qemu-iotests/testrunner.py +++ b/tests/qemu-iotests/testrunner.py @@ -246,17 +246,17 @@ class TestRunner(ContextManager['TestRunner']): t0 = time.time() with f_bad.open('w', encoding="utf-8") as f: - proc = subprocess.Popen(args, cwd=str(f_test.parent), env=env, - stdout=f, stderr=subprocess.STDOUT) - try: - proc.wait() - except KeyboardInterrupt: - proc.terminate() - proc.wait() - return TestResult(status='not run', - description='Interrupted by user', - interrupted=True) - ret = proc.returncode + with subprocess.Popen(args, cwd=str(f_test.parent), env=env, + stdout=f, stderr=subprocess.STDOUT) as proc: + try: + proc.wait() + except KeyboardInterrupt: + proc.terminate() + proc.wait() + return TestResult(status='not run', + description='Interrupted by user', + interrupted=True) + ret = proc.returncode elapsed = round(time.time() - t0, 1) From 94783301b8cb3f2b4fd871badd923fb3b9d2e7bf Mon Sep 17 00:00:00 2001 From: Vladimir Sementsov-Ogievskiy Date: Thu, 6 May 2021 12:06:14 +0300 Subject: [PATCH 13/19] block/write-threshold: don't use write notifiers write-notifiers are used only for write-threshold. New code for such purpose should create filters. Let's better special-case write-threshold and drop write notifiers at all. (Actually, write-threshold is special-cased anyway, as the only user of write-notifiers) So, create a new direct interface for bdrv_co_write_req_prepare() and drop all write-notifier related logic from write-threshold.c. Signed-off-by: Vladimir Sementsov-Ogievskiy Reviewed-by: Max Reitz Message-Id: <20210506090621.11848-2-vsementsov@virtuozzo.com> Reviewed-by: Eric Blake Reviewed-by: Stefan Hajnoczi [mreitz: Adjusted comment as per Eric's suggestion] Signed-off-by: Max Reitz --- block/io.c | 5 ++- block/write-threshold.c | 70 +++++++-------------------------- include/block/block_int.h | 1 - include/block/write-threshold.h | 9 +++++ 4 files changed, 27 insertions(+), 58 deletions(-) diff --git a/block/io.c b/block/io.c index 35b6c56efc..3520de51bb 100644 --- a/block/io.c +++ b/block/io.c @@ -30,6 +30,7 @@ #include "block/blockjob_int.h" #include "block/block_int.h" #include "block/coroutines.h" +#include "block/write-threshold.h" #include "qemu/cutils.h" #include "qapi/error.h" #include "qemu/error-report.h" @@ -2008,8 +2009,8 @@ bdrv_co_write_req_prepare(BdrvChild *child, int64_t offset, int64_t bytes, } else { assert(child->perm & BLK_PERM_WRITE); } - return notifier_with_return_list_notify(&bs->before_write_notifiers, - req); + bdrv_write_threshold_check_write(bs, offset, bytes); + return 0; case BDRV_TRACKED_TRUNCATE: assert(child->perm & BLK_PERM_RESIZE); return 0; diff --git a/block/write-threshold.c b/block/write-threshold.c index 85b78dc2a9..71df3c434f 100644 --- a/block/write-threshold.c +++ b/block/write-threshold.c @@ -29,14 +29,6 @@ bool bdrv_write_threshold_is_set(const BlockDriverState *bs) return bs->write_threshold_offset > 0; } -static void write_threshold_disable(BlockDriverState *bs) -{ - if (bdrv_write_threshold_is_set(bs)) { - notifier_with_return_remove(&bs->write_threshold_notifier); - bs->write_threshold_offset = 0; - } -} - uint64_t bdrv_write_threshold_exceeded(const BlockDriverState *bs, const BdrvTrackedRequest *req) { @@ -51,55 +43,9 @@ uint64_t bdrv_write_threshold_exceeded(const BlockDriverState *bs, return 0; } -static int coroutine_fn before_write_notify(NotifierWithReturn *notifier, - void *opaque) -{ - BdrvTrackedRequest *req = opaque; - BlockDriverState *bs = req->bs; - uint64_t amount = 0; - - amount = bdrv_write_threshold_exceeded(bs, req); - if (amount > 0) { - qapi_event_send_block_write_threshold( - bs->node_name, - amount, - bs->write_threshold_offset); - - /* autodisable to avoid flooding the monitor */ - write_threshold_disable(bs); - } - - return 0; /* should always let other notifiers run */ -} - -static void write_threshold_register_notifier(BlockDriverState *bs) -{ - bs->write_threshold_notifier.notify = before_write_notify; - bdrv_add_before_write_notifier(bs, &bs->write_threshold_notifier); -} - -static void write_threshold_update(BlockDriverState *bs, - int64_t threshold_bytes) -{ - bs->write_threshold_offset = threshold_bytes; -} - void bdrv_write_threshold_set(BlockDriverState *bs, uint64_t threshold_bytes) { - if (bdrv_write_threshold_is_set(bs)) { - if (threshold_bytes > 0) { - write_threshold_update(bs, threshold_bytes); - } else { - write_threshold_disable(bs); - } - } else { - if (threshold_bytes > 0) { - /* avoid multiple registration */ - write_threshold_register_notifier(bs); - write_threshold_update(bs, threshold_bytes); - } - /* discard bogus disable request */ - } + bs->write_threshold_offset = threshold_bytes; } void qmp_block_set_write_threshold(const char *node_name, @@ -122,3 +68,17 @@ void qmp_block_set_write_threshold(const char *node_name, aio_context_release(aio_context); } + +void bdrv_write_threshold_check_write(BlockDriverState *bs, int64_t offset, + int64_t bytes) +{ + int64_t end = offset + bytes; + uint64_t wtr = bs->write_threshold_offset; + + if (wtr > 0 && end > wtr) { + qapi_event_send_block_write_threshold(bs->node_name, end - wtr, wtr); + + /* autodisable to avoid flooding the monitor */ + bdrv_write_threshold_set(bs, 0); + } +} diff --git a/include/block/block_int.h b/include/block/block_int.h index 731ffedb27..aff948fb63 100644 --- a/include/block/block_int.h +++ b/include/block/block_int.h @@ -959,7 +959,6 @@ struct BlockDriverState { /* threshold limit for writes, in bytes. "High water mark". */ uint64_t write_threshold_offset; - NotifierWithReturn write_threshold_notifier; /* Writing to the list requires the BQL _and_ the dirty_bitmap_mutex. * Reading from the list can be done with either the BQL or the diff --git a/include/block/write-threshold.h b/include/block/write-threshold.h index c646f267a4..848a5dde85 100644 --- a/include/block/write-threshold.h +++ b/include/block/write-threshold.h @@ -59,4 +59,13 @@ bool bdrv_write_threshold_is_set(const BlockDriverState *bs); uint64_t bdrv_write_threshold_exceeded(const BlockDriverState *bs, const BdrvTrackedRequest *req); +/* + * bdrv_write_threshold_check_write + * + * Check whether the specified request exceeds the write threshold. + * If so, send a corresponding event and disable write threshold checking. + */ +void bdrv_write_threshold_check_write(BlockDriverState *bs, int64_t offset, + int64_t bytes); + #endif From ad578c56d596a3250a62ae8687005ca6909d2688 Mon Sep 17 00:00:00 2001 From: Vladimir Sementsov-Ogievskiy Date: Thu, 6 May 2021 12:06:15 +0300 Subject: [PATCH 14/19] block: drop write notifiers They are unused now. Signed-off-by: Vladimir Sementsov-Ogievskiy Reviewed-by: Max Reitz Message-Id: <20210506090621.11848-3-vsementsov@virtuozzo.com> Reviewed-by: Stefan Hajnoczi Signed-off-by: Max Reitz --- block.c | 1 - block/io.c | 6 ------ include/block/block_int.h | 12 ------------ 3 files changed, 19 deletions(-) diff --git a/block.c b/block.c index 9ad725d205..75a82af641 100644 --- a/block.c +++ b/block.c @@ -400,7 +400,6 @@ BlockDriverState *bdrv_new(void) for (i = 0; i < BLOCK_OP_TYPE_MAX; i++) { QLIST_INIT(&bs->op_blockers[i]); } - notifier_with_return_list_init(&bs->before_write_notifiers); qemu_co_mutex_init(&bs->reqs_lock); qemu_mutex_init(&bs->dirty_bitmap_mutex); bs->refcnt = 1; diff --git a/block/io.c b/block/io.c index 3520de51bb..1e826ba9e8 100644 --- a/block/io.c +++ b/block/io.c @@ -3165,12 +3165,6 @@ bool bdrv_qiov_is_aligned(BlockDriverState *bs, QEMUIOVector *qiov) return true; } -void bdrv_add_before_write_notifier(BlockDriverState *bs, - NotifierWithReturn *notifier) -{ - notifier_with_return_list_add(&bs->before_write_notifiers, notifier); -} - void bdrv_io_plug(BlockDriverState *bs) { BdrvChild *child; diff --git a/include/block/block_int.h b/include/block/block_int.h index aff948fb63..b2c8b09d0f 100644 --- a/include/block/block_int.h +++ b/include/block/block_int.h @@ -954,9 +954,6 @@ struct BlockDriverState { */ int64_t total_sectors; - /* Callback before write request is processed */ - NotifierWithReturnList before_write_notifiers; - /* threshold limit for writes, in bytes. "High water mark". */ uint64_t write_threshold_offset; @@ -1083,15 +1080,6 @@ void bdrv_parse_filename_strip_prefix(const char *filename, const char *prefix, bool bdrv_backing_overridden(BlockDriverState *bs); -/** - * bdrv_add_before_write_notifier: - * - * Register a callback that is invoked before write requests are processed but - * after any throttling or waiting for overlapping requests. - */ -void bdrv_add_before_write_notifier(BlockDriverState *bs, - NotifierWithReturn *notifier); - /** * bdrv_add_aio_context_notifier: * From e46354a8aeefe4637c689de27532bc00712f9c60 Mon Sep 17 00:00:00 2001 From: Vladimir Sementsov-Ogievskiy Date: Thu, 6 May 2021 12:06:16 +0300 Subject: [PATCH 15/19] test-write-threshold: rewrite test_threshold_(not_)trigger tests These tests use bdrv_write_threshold_exceeded() API, which is used only for test (since pre-previous commit). Better is testing real API, which is used in block.c as well. So, let's call bdrv_write_threshold_check_write(), and check is bs->write_threshold_offset cleared or not (it's cleared iff threshold triggered). Also we get rid of BdrvTrackedRequest use here. Note, that paranoiac bdrv_check_request() calls were added in 8b1170012b1 to protect BdrvTrackedRequest. Drop them now. Signed-off-by: Vladimir Sementsov-Ogievskiy Reviewed-by: Max Reitz Message-Id: <20210506090621.11848-4-vsementsov@virtuozzo.com> Reviewed-by: Stefan Hajnoczi Signed-off-by: Max Reitz --- tests/unit/test-write-threshold.c | 22 ++++------------------ 1 file changed, 4 insertions(+), 18 deletions(-) diff --git a/tests/unit/test-write-threshold.c b/tests/unit/test-write-threshold.c index fc1c45a2eb..fd40a815b8 100644 --- a/tests/unit/test-write-threshold.c +++ b/tests/unit/test-write-threshold.c @@ -55,41 +55,27 @@ static void test_threshold_multi_set_get(void) static void test_threshold_not_trigger(void) { - uint64_t amount = 0; uint64_t threshold = 4 * 1024 * 1024; BlockDriverState bs; - BdrvTrackedRequest req; memset(&bs, 0, sizeof(bs)); - memset(&req, 0, sizeof(req)); - req.offset = 1024; - req.bytes = 1024; - - bdrv_check_request(req.offset, req.bytes, &error_abort); bdrv_write_threshold_set(&bs, threshold); - amount = bdrv_write_threshold_exceeded(&bs, &req); - g_assert_cmpuint(amount, ==, 0); + bdrv_write_threshold_check_write(&bs, 1024, 1024); + g_assert_cmpuint(bdrv_write_threshold_get(&bs), ==, threshold); } static void test_threshold_trigger(void) { - uint64_t amount = 0; uint64_t threshold = 4 * 1024 * 1024; BlockDriverState bs; - BdrvTrackedRequest req; memset(&bs, 0, sizeof(bs)); - memset(&req, 0, sizeof(req)); - req.offset = (4 * 1024 * 1024) - 1024; - req.bytes = 2 * 1024; - - bdrv_check_request(req.offset, req.bytes, &error_abort); bdrv_write_threshold_set(&bs, threshold); - amount = bdrv_write_threshold_exceeded(&bs, &req); - g_assert_cmpuint(amount, >=, 1024); + bdrv_write_threshold_check_write(&bs, threshold - 1024, 2 * 1024); + g_assert_cmpuint(bdrv_write_threshold_get(&bs), ==, 0); } typedef struct TestStruct { From 2e0e9cbd897078fea2ad3772106e0cbd1f0c239a Mon Sep 17 00:00:00 2001 From: Vladimir Sementsov-Ogievskiy Date: Thu, 6 May 2021 12:06:17 +0300 Subject: [PATCH 16/19] block/write-threshold: drop extra APIs bdrv_write_threshold_exceeded() is unused. bdrv_write_threshold_is_set() is used only to double check the value of bs->write_threshold_offset in tests. No real sense in it (both tests do check real value with help of bdrv_write_threshold_get()) Signed-off-by: Vladimir Sementsov-Ogievskiy Reviewed-by: Max Reitz Message-Id: <20210506090621.11848-5-vsementsov@virtuozzo.com> Reviewed-by: Eric Blake Reviewed-by: Stefan Hajnoczi [mreitz: Adjusted commit message as per Eric's suggestion] Signed-off-by: Max Reitz --- block/write-threshold.c | 19 ------------------- include/block/write-threshold.h | 24 ------------------------ tests/unit/test-write-threshold.c | 4 ---- 3 files changed, 47 deletions(-) diff --git a/block/write-threshold.c b/block/write-threshold.c index 71df3c434f..65a6acd142 100644 --- a/block/write-threshold.c +++ b/block/write-threshold.c @@ -24,25 +24,6 @@ uint64_t bdrv_write_threshold_get(const BlockDriverState *bs) return bs->write_threshold_offset; } -bool bdrv_write_threshold_is_set(const BlockDriverState *bs) -{ - return bs->write_threshold_offset > 0; -} - -uint64_t bdrv_write_threshold_exceeded(const BlockDriverState *bs, - const BdrvTrackedRequest *req) -{ - if (bdrv_write_threshold_is_set(bs)) { - if (req->offset > bs->write_threshold_offset) { - return (req->offset - bs->write_threshold_offset) + req->bytes; - } - if ((req->offset + req->bytes) > bs->write_threshold_offset) { - return (req->offset + req->bytes) - bs->write_threshold_offset; - } - } - return 0; -} - void bdrv_write_threshold_set(BlockDriverState *bs, uint64_t threshold_bytes) { bs->write_threshold_offset = threshold_bytes; diff --git a/include/block/write-threshold.h b/include/block/write-threshold.h index 848a5dde85..a03ee1cacd 100644 --- a/include/block/write-threshold.h +++ b/include/block/write-threshold.h @@ -35,30 +35,6 @@ void bdrv_write_threshold_set(BlockDriverState *bs, uint64_t threshold_bytes); */ uint64_t bdrv_write_threshold_get(const BlockDriverState *bs); -/* - * bdrv_write_threshold_is_set - * - * Tell if a write threshold is set for a given BDS. - */ -bool bdrv_write_threshold_is_set(const BlockDriverState *bs); - -/* - * bdrv_write_threshold_exceeded - * - * Return the extent of a write request that exceeded the threshold, - * or zero if the request is below the threshold. - * Return zero also if the threshold was not set. - * - * NOTE: here we assume the following holds for each request this code - * deals with: - * - * assert((req->offset + req->bytes) <= UINT64_MAX) - * - * Please not there is *not* an actual C assert(). - */ -uint64_t bdrv_write_threshold_exceeded(const BlockDriverState *bs, - const BdrvTrackedRequest *req); - /* * bdrv_write_threshold_check_write * diff --git a/tests/unit/test-write-threshold.c b/tests/unit/test-write-threshold.c index fd40a815b8..bb5c1a5217 100644 --- a/tests/unit/test-write-threshold.c +++ b/tests/unit/test-write-threshold.c @@ -18,8 +18,6 @@ static void test_threshold_not_set_on_init(void) BlockDriverState bs; memset(&bs, 0, sizeof(bs)); - g_assert(!bdrv_write_threshold_is_set(&bs)); - res = bdrv_write_threshold_get(&bs); g_assert_cmpint(res, ==, 0); } @@ -33,8 +31,6 @@ static void test_threshold_set_get(void) bdrv_write_threshold_set(&bs, threshold); - g_assert(bdrv_write_threshold_is_set(&bs)); - res = bdrv_write_threshold_get(&bs); g_assert_cmpint(res, ==, threshold); } From 935129223ce2a2eb61c59b585869724570d3a349 Mon Sep 17 00:00:00 2001 From: Vladimir Sementsov-Ogievskiy Date: Thu, 6 May 2021 12:06:19 +0300 Subject: [PATCH 17/19] test-write-threshold: drop extra tests Testing set/get of one 64bit variable doesn't seem necessary. We have a lot of such variables. Also remaining tests do test set/get anyway. Signed-off-by: Vladimir Sementsov-Ogievskiy Reviewed-by: Max Reitz Message-Id: <20210506090621.11848-7-vsementsov@virtuozzo.com> Reviewed-by: Stefan Hajnoczi Signed-off-by: Max Reitz --- tests/unit/test-write-threshold.c | 43 ------------------------------- 1 file changed, 43 deletions(-) diff --git a/tests/unit/test-write-threshold.c b/tests/unit/test-write-threshold.c index bb5c1a5217..9e9986aefc 100644 --- a/tests/unit/test-write-threshold.c +++ b/tests/unit/test-write-threshold.c @@ -12,43 +12,6 @@ #include "block/write-threshold.h" -static void test_threshold_not_set_on_init(void) -{ - uint64_t res; - BlockDriverState bs; - memset(&bs, 0, sizeof(bs)); - - res = bdrv_write_threshold_get(&bs); - g_assert_cmpint(res, ==, 0); -} - -static void test_threshold_set_get(void) -{ - uint64_t threshold = 4 * 1024 * 1024; - uint64_t res; - BlockDriverState bs; - memset(&bs, 0, sizeof(bs)); - - bdrv_write_threshold_set(&bs, threshold); - - res = bdrv_write_threshold_get(&bs); - g_assert_cmpint(res, ==, threshold); -} - -static void test_threshold_multi_set_get(void) -{ - uint64_t threshold1 = 4 * 1024 * 1024; - uint64_t threshold2 = 15 * 1024 * 1024; - uint64_t res; - BlockDriverState bs; - memset(&bs, 0, sizeof(bs)); - - bdrv_write_threshold_set(&bs, threshold1); - bdrv_write_threshold_set(&bs, threshold2); - res = bdrv_write_threshold_get(&bs); - g_assert_cmpint(res, ==, threshold2); -} - static void test_threshold_not_trigger(void) { uint64_t threshold = 4 * 1024 * 1024; @@ -84,12 +47,6 @@ int main(int argc, char **argv) { size_t i; TestStruct tests[] = { - { "/write-threshold/not-set-on-init", - test_threshold_not_set_on_init }, - { "/write-threshold/set-get", - test_threshold_set_get }, - { "/write-threshold/multi-set-get", - test_threshold_multi_set_get }, { "/write-threshold/not-trigger", test_threshold_not_trigger }, { "/write-threshold/trigger", From 23357b93c7bbeec65aef75f798cacbcda0ca5f4d Mon Sep 17 00:00:00 2001 From: Vladimir Sementsov-Ogievskiy Date: Thu, 6 May 2021 12:06:20 +0300 Subject: [PATCH 18/19] test-write-threshold: drop extra TestStruct structure We don't need this extra logic: it doesn't make code simpler. Signed-off-by: Vladimir Sementsov-Ogievskiy Reviewed-by: Max Reitz Message-Id: <20210506090621.11848-8-vsementsov@virtuozzo.com> Reviewed-by: Stefan Hajnoczi Signed-off-by: Max Reitz --- tests/unit/test-write-threshold.c | 20 +++----------------- 1 file changed, 3 insertions(+), 17 deletions(-) diff --git a/tests/unit/test-write-threshold.c b/tests/unit/test-write-threshold.c index 9e9986aefc..49b1ef7a20 100644 --- a/tests/unit/test-write-threshold.c +++ b/tests/unit/test-write-threshold.c @@ -37,26 +37,12 @@ static void test_threshold_trigger(void) g_assert_cmpuint(bdrv_write_threshold_get(&bs), ==, 0); } -typedef struct TestStruct { - const char *name; - void (*func)(void); -} TestStruct; - int main(int argc, char **argv) { - size_t i; - TestStruct tests[] = { - { "/write-threshold/not-trigger", - test_threshold_not_trigger }, - { "/write-threshold/trigger", - test_threshold_trigger }, - { NULL, NULL } - }; - g_test_init(&argc, &argv, NULL); - for (i = 0; tests[i].name != NULL; i++) { - g_test_add_func(tests[i].name, tests[i].func); - } + g_test_add_func("/write-threshold/not-trigger", test_threshold_not_trigger); + g_test_add_func("/write-threshold/trigger", test_threshold_trigger); + return g_test_run(); } From c61ebf362d0abf288ce266845519d5a550a1d89f Mon Sep 17 00:00:00 2001 From: Vladimir Sementsov-Ogievskiy Date: Thu, 6 May 2021 12:06:21 +0300 Subject: [PATCH 19/19] write-threshold: deal with includes "qemu/typedefs.h" is enough for include/block/write-threshold.h header with forward declaration of BlockDriverState. Also drop extra includes from block/write-threshold.c and tests/unit/test-write-threshold.c Signed-off-by: Vladimir Sementsov-Ogievskiy Message-Id: <20210506090621.11848-9-vsementsov@virtuozzo.com> Reviewed-by: Stefan Hajnoczi Signed-off-by: Max Reitz --- block/write-threshold.c | 2 -- include/block/write-threshold.h | 2 +- tests/unit/test-write-threshold.c | 1 - 3 files changed, 1 insertion(+), 4 deletions(-) diff --git a/block/write-threshold.c b/block/write-threshold.c index 65a6acd142..35cafbc22d 100644 --- a/block/write-threshold.c +++ b/block/write-threshold.c @@ -12,9 +12,7 @@ #include "qemu/osdep.h" #include "block/block_int.h" -#include "qemu/coroutine.h" #include "block/write-threshold.h" -#include "qemu/notify.h" #include "qapi/error.h" #include "qapi/qapi-commands-block-core.h" #include "qapi/qapi-events-block-core.h" diff --git a/include/block/write-threshold.h b/include/block/write-threshold.h index a03ee1cacd..f50f923e7e 100644 --- a/include/block/write-threshold.h +++ b/include/block/write-threshold.h @@ -13,7 +13,7 @@ #ifndef BLOCK_WRITE_THRESHOLD_H #define BLOCK_WRITE_THRESHOLD_H -#include "block/block_int.h" +#include "qemu/typedefs.h" /* * bdrv_write_threshold_set: diff --git a/tests/unit/test-write-threshold.c b/tests/unit/test-write-threshold.c index 49b1ef7a20..0158e4637a 100644 --- a/tests/unit/test-write-threshold.c +++ b/tests/unit/test-write-threshold.c @@ -7,7 +7,6 @@ */ #include "qemu/osdep.h" -#include "qapi/error.h" #include "block/block_int.h" #include "block/write-threshold.h"