From 5a7f21efaf99c60614fe1967be1c0f9aa46c526e Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Fri, 1 Dec 2023 15:25:19 +0100 Subject: vl: Improve error message for conflicting -incoming and -loadvm Currently, the conflict between -incoming and -loadvm is only detected when loading the snapshot fails because the image is still inactive for the incoming migration. This results in a suboptimal error message: $ ./qemu-system-x86_64 -hda /tmp/test.qcow2 -loadvm foo -incoming defer qemu-system-x86_64: Device 'ide0-hd0' is writable but does not support snapshots Catch the situation already in qemu_validate_options() to improve the message: $ ./qemu-system-x86_64 -hda /tmp/test.qcow2 -loadvm foo -incoming defer qemu-system-x86_64: 'incoming' and 'loadvm' options are mutually exclusive Signed-off-by: Kevin Wolf Message-ID: <20231201142520.32255-3-kwolf@redhat.com> Signed-off-by: Kevin Wolf --- system/vl.c | 4 ++++ 1 file changed, 4 insertions(+) (limited to 'system') diff --git a/system/vl.c b/system/vl.c index 2bcd9ef..6b87bfa 100644 --- a/system/vl.c +++ b/system/vl.c @@ -2426,6 +2426,10 @@ static void qemu_validate_options(const QDict *machine_opts) } } + if (loadvm && incoming) { + error_report("'incoming' and 'loadvm' options are mutually exclusive"); + exit(EXIT_FAILURE); + } if (loadvm && preconfig_requested) { error_report("'preconfig' and 'loadvm' options are " "mutually exclusive"); -- cgit v1.1 From e661a2470342d6fa873369c81988a19b1bb7b3f4 Mon Sep 17 00:00:00 2001 From: Stefan Hajnoczi Date: Mon, 4 Dec 2023 11:42:59 -0500 Subject: dma-helpers: don't lock AioContext in dma_blk_cb() Commit abfcd2760b3e ("dma-helpers: prevent dma_blk_cb() vs dma_aio_cancel() race") acquired the AioContext lock inside dma_blk_cb() to avoid a race with scsi_device_purge_requests() running in the main loop thread. The SCSI code no longer calls dma_aio_cancel() from the main loop thread while I/O is running in the IOThread AioContext. Therefore it is no longer necessary to take this lock to protect DMAAIOCB fields. The ->cb() function also does not require the lock because blk_aio_*() and friends do not need the AioContext lock. Both hw/ide/core.c and hw/ide/macio.c also call dma_blk_io() but don't rely on it taking the AioContext lock, so this change is safe. Signed-off-by: Stefan Hajnoczi Reviewed-by: Eric Blake Reviewed-by: Kevin Wolf Message-ID: <20231204164259.1515217-5-stefanha@redhat.com> Signed-off-by: Kevin Wolf --- system/dma-helpers.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) (limited to 'system') diff --git a/system/dma-helpers.c b/system/dma-helpers.c index 36211ac..528117f 100644 --- a/system/dma-helpers.c +++ b/system/dma-helpers.c @@ -119,13 +119,12 @@ static void dma_blk_cb(void *opaque, int ret) trace_dma_blk_cb(dbs, ret); - aio_context_acquire(ctx); dbs->acb = NULL; dbs->offset += dbs->iov.size; if (dbs->sg_cur_index == dbs->sg->nsg || ret < 0) { dma_complete(dbs, ret); - goto out; + return; } dma_blk_unmap(dbs); @@ -168,7 +167,7 @@ static void dma_blk_cb(void *opaque, int ret) trace_dma_map_wait(dbs); dbs->bh = aio_bh_new(ctx, reschedule_dma, dbs); cpu_register_map_client(dbs->bh); - goto out; + return; } if (!QEMU_IS_ALIGNED(dbs->iov.size, dbs->align)) { @@ -179,8 +178,6 @@ static void dma_blk_cb(void *opaque, int ret) dbs->acb = dbs->io_func(dbs->offset, &dbs->iov, dma_blk_cb, dbs, dbs->io_func_opaque); assert(dbs->acb); -out: - aio_context_release(ctx); } static void dma_aio_cancel(BlockAIOCB *acb) -- cgit v1.1 From 10bcb0d996634aec642b000f05a72c93b652b2e6 Mon Sep 17 00:00:00 2001 From: Stefan Hajnoczi Date: Tue, 5 Dec 2023 13:19:59 -0500 Subject: scsi: assert that callbacks run in the correct AioContext Since the removal of AioContext locking, the correctness of the code relies on running requests from a single AioContext at any given time. Add assertions that verify that callbacks are invoked in the correct AioContext. Signed-off-by: Stefan Hajnoczi Message-ID: <20231205182011.1976568-3-stefanha@redhat.com> Reviewed-by: Kevin Wolf Signed-off-by: Kevin Wolf --- system/dma-helpers.c | 3 +++ 1 file changed, 3 insertions(+) (limited to 'system') diff --git a/system/dma-helpers.c b/system/dma-helpers.c index 528117f..9b221cf 100644 --- a/system/dma-helpers.c +++ b/system/dma-helpers.c @@ -119,6 +119,9 @@ static void dma_blk_cb(void *opaque, int ret) trace_dma_blk_cb(dbs, ret); + /* DMAAIOCB is not thread-safe and must be accessed only from dbs->ctx */ + assert(ctx == qemu_get_current_aio_context()); + dbs->acb = NULL; dbs->offset += dbs->iov.size; -- cgit v1.1