Re: [PATCH v6 02/12] monitor: Use getter/setter functions for cur_mon

2020-08-04 Thread Markus Armbruster
Markus Armbruster writes: > Kevin Wolf writes: > >> cur_mon really needs to be coroutine-local as soon as we move monitor >> command handlers to coroutines and let them yield. As a first step, just >> remove all direct accesses to cur_mon so that we can implement this in >> the getter function l

Re: [PATCH] qcow2: flush qcow2 l2 meta for new allocated clusters

2020-08-04 Thread no-reply
Patchew URL: https://patchew.org/QEMU/20200805023826.184-1-fangyi...@huawei.com/ Hi, This series failed the docker-quick@centos7 build test. Please find the testing commands and their output below. If you have Docker installed, you can probably reproduce it locally. The full log is avail

[PATCH] qcow2: flush qcow2 l2 meta for new allocated clusters

2020-08-04 Thread Ying Fang
From: fangying When qemu or qemu-nbd process uses a qcow2 image and configured with 'cache = none', it will write to the qcow2 image with a cache to cache L2 tables, however the process will not use L2 tables without explicitly calling the flush command or closing the mirror flash into the disk.

Re: cleanups with long-term benefits (was Re: [PATCH] schemas: Add vim modeline)

2020-08-04 Thread John Snow
On 8/4/20 4:03 AM, Markus Armbruster wrote: The pain of tweaking the parser is likely dwarved several times over by the pain of the flag day. You mention this often; I wonder if I misunderstand the critique, because the pain of a "flag day" for a new file format seems negligible to me. I do

Re: [PATCH 3/3] aio-posix: keep aio_notify_me disabled during polling

2020-08-04 Thread Paolo Bonzini
On 04/08/20 12:29, Stefan Hajnoczi wrote: > On Tue, Aug 04, 2020 at 06:28:04AM +0100, Stefan Hajnoczi wrote: >> @@ -597,15 +574,38 @@ bool aio_poll(AioContext *ctx, bool blocking) >> * system call---a single round of run_poll_handlers_once suffices. >> */ >> if (timeout || ctx->fdm

Re: [PATCH v6 02/12] monitor: Use getter/setter functions for cur_mon

2020-08-04 Thread Kevin Wolf
Am 04.08.2020 um 14:46 hat Markus Armbruster geschrieben: > > diff --git a/monitor/hmp.c b/monitor/hmp.c > > index d598dd02bb..f609fcf75b 100644 > > --- a/monitor/hmp.c > > +++ b/monitor/hmp.c > > @@ -1301,11 +1301,11 @@ cleanup: > > static void monitor_read(void *opaque, const uint8_t *buf, int s

Re: [PATCH v6 06/12] monitor: Make current monitor a per-coroutine property

2020-08-04 Thread Daniel P . Berrangé
On Tue, Aug 04, 2020 at 03:50:54PM +0200, Markus Armbruster wrote: > Kevin Wolf writes: > > > This way, a monitor command handler will still be able to access the > > current monitor, but when it yields, all other code code will correctly > > get NULL from monitor_cur(). > > > > Outside of corout

Re: [PATCH v6 06/12] monitor: Make current monitor a per-coroutine property

2020-08-04 Thread Kevin Wolf
Am 04.08.2020 um 15:50 hat Markus Armbruster geschrieben: > Kevin Wolf writes: > > > This way, a monitor command handler will still be able to access the > > current monitor, but when it yields, all other code code will correctly > > get NULL from monitor_cur(). > > > > Outside of coroutine conte

Re: [PATCH v1 0/7] vhost-user-blk: fix the migration issue and enhance qtests

2020-08-04 Thread Dima Stepanov
On Tue, Aug 04, 2020 at 10:19:17AM -0400, Michael S. Tsirkin wrote: > On Tue, Aug 04, 2020 at 01:36:45PM +0300, Dima Stepanov wrote: > > Reference e-mail threads: > > - https://lists.gnu.org/archive/html/qemu-devel/2020-05/msg01509.html > > - https://lists.gnu.org/archive/html/qemu-devel/2020-0

Re: [PATCH v1 0/7] vhost-user-blk: fix the migration issue and enhance qtests

2020-08-04 Thread Michael S. Tsirkin
On Tue, Aug 04, 2020 at 01:36:45PM +0300, Dima Stepanov wrote: > Reference e-mail threads: > - https://lists.gnu.org/archive/html/qemu-devel/2020-05/msg01509.html > - https://lists.gnu.org/archive/html/qemu-devel/2020-05/msg05241.html > > If vhost-user daemon is used as a backend for the vhost

Re: [PATCH v6 06/12] monitor: Make current monitor a per-coroutine property

2020-08-04 Thread Markus Armbruster
Kevin Wolf writes: > This way, a monitor command handler will still be able to access the > current monitor, but when it yields, all other code code will correctly > get NULL from monitor_cur(). > > Outside of coroutine context, qemu_coroutine_self() returns the leader > coroutine of the current

Re: [PATCH v6 05/12] qmp: Call monitor_set_cur() only in qmp_dispatch()

2020-08-04 Thread Markus Armbruster
Kevin Wolf writes: > The correct way to set the current monitor for a coroutine handler is > different that for a blocking handler, so monitor_set_cur() can only be will be different > called in qmp_dispatch(). needs to be called in > > Signed-off-by: Kevin Wolf > --- > include/qapi/qmp/dis

Re: [PATCH v6 04/12] qmp: Assert that no other monitor is active

2020-08-04 Thread Markus Armbruster
Kevin Wolf writes: > monitor_qmp_dispatch() is never supposed to be called in the context of > another monitor, so assert that monitor_cur() is NULL instead of saving > and restoring it. > > Signed-off-by: Kevin Wolf > --- > monitor/qmp.c | 5 ++--- > 1 file changed, 2 insertions(+), 3 deletion

Re: [PATCH v6 03/12] hmp: Set cur_mon only in handle_hmp_command()

2020-08-04 Thread Markus Armbruster
Kevin Wolf writes: > cur_mon is updated relatively early in the command handling code even @cur_mon doesn't exist anymore (you renamed it to @cur_monitor in the previous patch). Either say "The current monitor", or use the actual variable name. > though only the command handler actually needs

Re: [PATCH v6 02/12] monitor: Use getter/setter functions for cur_mon

2020-08-04 Thread Markus Armbruster
Kevin Wolf writes: > cur_mon really needs to be coroutine-local as soon as we move monitor > command handlers to coroutines and let them yield. As a first step, just > remove all direct accesses to cur_mon so that we can implement this in > the getter function later. > > Signed-off-by: Kevin Wolf

Re: [PATCH v6 01/12] monitor: Add Monitor parameter to monitor_set_cpu()

2020-08-04 Thread Markus Armbruster
Kevin Wolf writes: > Most callers actually don't have to rely on cur_mon, but already know > for which monitor they call monitor_set_cpu(). > > Signed-off-by: Kevin Wolf > --- > include/monitor/monitor.h | 2 +- > monitor/hmp-cmds.c| 2 +- > monitor/misc.c| 10 +- >

Re: [PATCH v6 00/12] monitor: Optionally run handlers in coroutines

2020-08-04 Thread Markus Armbruster
I let this series slide to get my Error API rework done, along with much else. My sincere apologies! Unsurprisingly, it needs a rebase now. I suggest to let me review it as is first.

[PATCH v1 7/7] tests/qtest/vhost-user-test: enable the reconnect tests

2020-08-04 Thread Dima Stepanov
For now a QTEST_VHOST_USER_FIXME environment variable is used to separate reconnect tests for the vhost-user-net device. Looks like the reconnect functionality is pretty stable, so this separation is deprecated. Remove it and enable these tests for the default run. Signed-off-by: Dima Stepanov --

[PATCH v1 5/7] tests/qtest/vhost-user-test: add support for the vhost-user-blk device

2020-08-04 Thread Dima Stepanov
Add vhost_user_ops structure for the vhost-user-blk device class. Add the test_reconnect and test_migrate tests for this device. Signed-off-by: Dima Stepanov --- tests/qtest/vhost-user-test.c | 140 +- 1 file changed, 138 insertions(+), 2 deletions(-) dif

[PATCH v1 3/7] tests/qtest/vhost-user-test: prepare the tests for adding new dev class

2020-08-04 Thread Dima Stepanov
For now only vhost-user-net device is supported by the test. Other vhost-user devices are not tested. As a first step make source code refactoring so new devices can reuse the same test routines. To make this provide a new vhost_user_ops structure with the methods to initialize device, its command

[PATCH v1 6/7] tests/qtest/vhost-user-test: add migrate_reconnect test

2020-08-04 Thread Dima Stepanov
Add new migrate_reconnect test for the vhost-user-blk device. Perform a disconnect after sending response for the VHOST_USER_SET_LOG_BASE command. Signed-off-by: Dima Stepanov --- tests/qtest/vhost-user-test.c | 25 + 1 file changed, 25 insertions(+) diff --git a/tests/q

[PATCH v1 0/7] vhost-user-blk: fix the migration issue and enhance qtests

2020-08-04 Thread Dima Stepanov
Reference e-mail threads: - https://lists.gnu.org/archive/html/qemu-devel/2020-05/msg01509.html - https://lists.gnu.org/archive/html/qemu-devel/2020-05/msg05241.html If vhost-user daemon is used as a backend for the vhost device, then we should consider a possibility of disconnect at any momen

[PATCH v1 4/7] tests/qtest/libqos/virtio-blk: add support for vhost-user-blk

2020-08-04 Thread Dima Stepanov
Add support for the vhost-user-blk-pci device. This node can be used by the vhost-user-blk tests. Tests for the vhost-user-blk device are added in the following patches. Signed-off-by: Dima Stepanov --- tests/qtest/libqos/virtio-blk.c | 14 ++ 1 file changed, 14 insertions(+) diff -

[PATCH v1 2/7] vhost: check queue state in the vhost_dev_set_log routine

2020-08-04 Thread Dima Stepanov
If the vhost-user-blk daemon provides only one virtqueue, but device was added with several queues, then QEMU will send more VHOST-USER command than expected by daemon side. The vhost_virtqueue_start() routine handles such case by checking the return value from the virtio_queue_get_desc_addr() func

[PATCH v1 1/7] vhost: recheck dev state in the vhost_migration_log routine

2020-08-04 Thread Dima Stepanov
vhost-user devices can get a disconnect in the middle of the VHOST-USER handshake on the migration start. If disconnect event happened right before sending next VHOST-USER command, then the vhost_dev_set_log() call in the vhost_migration_log() function will return error. This error will lead to the

Re: [PATCH 3/3] aio-posix: keep aio_notify_me disabled during polling

2020-08-04 Thread Stefan Hajnoczi
On Tue, Aug 04, 2020 at 06:28:04AM +0100, Stefan Hajnoczi wrote: > @@ -597,15 +574,38 @@ bool aio_poll(AioContext *ctx, bool blocking) > * system call---a single round of run_poll_handlers_once suffices. > */ > if (timeout || ctx->fdmon_ops->need_wait(ctx)) { > +/* > +

Re: [PATCH 2/3] async: always set ctx->notified in aio_notify()

2020-08-04 Thread Stefan Hajnoczi
On Tue, Aug 04, 2020 at 09:12:46AM +0200, Paolo Bonzini wrote: > On 04/08/20 07:28, Stefan Hajnoczi wrote: > > @@ -425,19 +425,14 @@ void aio_notify(AioContext *ctx) > > smp_mb(); > > if (atomic_read(&ctx->notify_me)) { > > event_notifier_set(&ctx->notifier); > > -atomic_

Re: [PATCH] schemas: Add vim modeline

2020-08-04 Thread Alex Bennée
Daniel P. Berrangé writes: > On Thu, Jul 30, 2020 at 11:07:26AM +0200, Markus Armbruster wrote: >> Andrea Bolognani writes: >> >> > The various schemas included in QEMU use a JSON-based format which >> > is, however, strictly speaking not valid JSON. >> > >> > As a consequence, when vim tries

[PATCH v7 7/8] MAINTAINERS: Add myself as maintainer for yank feature

2020-08-04 Thread Lukas Straub
I'll maintain this for now as the colo usecase is the first user of this functionality. Signed-off-by: Lukas Straub Reviewed-by: Daniel P. Berrangé Acked-by: Stefan Hajnoczi --- MAINTAINERS | 6 ++ 1 file changed, 6 insertions(+) diff --git a/MAINTAINERS b/MAINTAINERS index 0886eb3d2b..bf

[PATCH v7 6/8] io: Document thread-safety of qio_channel_shutdown

2020-08-04 Thread Lukas Straub
Migration and yank code assume that qio_channel_shutdown is thread -safe. Document this after checking the code. Signed-off-by: Lukas Straub Reviewed-by: Daniel P. Berrangé Acked-by: Stefan Hajnoczi --- include/io/channel.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/include/io/chann

[PATCH v7 5/8] io/channel-tls.c: make qio_channel_tls_shutdown thread-safe

2020-08-04 Thread Lukas Straub
Make qio_channel_tls_shutdown thread-safe by using atomics when accessing tioc->shutdown. Signed-off-by: Lukas Straub Reviewed-by: Daniel P. Berrangé Acked-by: Stefan Hajnoczi --- io/channel-tls.c | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/io/channel-tls.c b/io/c

[PATCH v7 3/8] chardev/char-socket.c: Add yank feature

2020-08-04 Thread Lukas Straub
Register a yank function to shutdown the socket on yank. Signed-off-by: Lukas Straub Acked-by: Stefan Hajnoczi --- chardev/char-socket.c | 31 +++ 1 file changed, 31 insertions(+) diff --git a/chardev/char-socket.c b/chardev/char-socket.c index ef62dbf3d7..8e2865ca8

[PATCH v7 8/8] tests/test-char.c: Wait for the chardev to connect in char_socket_client_dupid_test

2020-08-04 Thread Lukas Straub
A connecting chardev object has an additional reference by the connecting thread, so if the chardev is still connecting by the end of the test, then the chardev object won't be freed. This in turn means that the yank instance won't be unregistered and when running the next test-case yank_register_i

[PATCH v7 1/8] Introduce yank feature

2020-08-04 Thread Lukas Straub
The yank feature allows to recover from hanging qemu by "yanking" at various parts. Other qemu systems can register themselves and multiple yank functions. Then all yank functions for selected instances can be called by the 'yank' out-of-band qmp command. Available instances can be queried by a 'qu

[PATCH v7 2/8] block/nbd.c: Add yank feature

2020-08-04 Thread Lukas Straub
Register a yank function which shuts down the socket and sets s->state = NBD_CLIENT_QUIT. This is the same behaviour as if an error occured. Signed-off-by: Lukas Straub Acked-by: Stefan Hajnoczi --- block/nbd.c | 129 1 file changed, 80 inser

[PATCH v7 4/8] migration: Add yank feature

2020-08-04 Thread Lukas Straub
Register yank functions on sockets to shut them down. Signed-off-by: Lukas Straub Acked-by: Stefan Hajnoczi --- migration/channel.c | 12 migration/migration.c | 25 - migration/multifd.c | 10 ++ migration/qemu-file-chann

[PATCH v7 0/8] Introduce 'yank' oob qmp command to recover from hanging qemu

2020-08-04 Thread Lukas Straub
Hello Everyone, In many cases, if qemu has a network connection (qmp, migration, chardev, etc.) to some other server and that server dies or hangs, qemu hangs too. These patches introduce the new 'yank' out-of-band qmp command to recover from these kinds of hangs. The different subsystems register

Re: cleanups with long-term benefits (was Re: [PATCH] schemas: Add vim modeline)

2020-08-04 Thread Markus Armbruster
Paolo Bonzini writes: > On 03/08/20 18:03, Markus Armbruster wrote: >>> In general it seems like a good idea to use a standard file format and >>> not "a standard file format except for two characters". :) We also >>> wouldn't be having discussions on editors. >> >> No argument. But towards wh

Re: [PATCH] schemas: Add vim modeline

2020-08-04 Thread Markus Armbruster
Paolo Bonzini writes: > On 01/08/20 01:12, Nir Soffer wrote: >> I think inventing DSLs and developing tools is wrong. Use standard >> format and tools and spend time on the core of the project. > > Please don't apply 2020 standards to choices that were made in 2009. Or > if you do, be ready to

Re: cleanups with long-term benefits (was Re: [PATCH] schemas: Add vim modeline)

2020-08-04 Thread Markus Armbruster
Kevin Wolf writes: > Am 03.08.2020 um 18:03 hat Markus Armbruster geschrieben: >> Paolo Bonzini writes: >> > This means the two parts might be considered separately: >> > >> > - replacing single-quote with double-quote strings >> > >> > - replacing # comments with // >> >> If all we want is dec

Re: [PATCH 0/3] aio-posix: keep aio_notify_me disabled during polling

2020-08-04 Thread Paolo Bonzini
On 04/08/20 07:28, Stefan Hajnoczi wrote: > This patch series eliminates ctx->notifier EventNotifier activity when > aio_poll() is in polling mode. There is no need to use the EventNotifier since > a polling handler can detect that aio_notify() has been called by monitoring a > field in memory inst

Re: [PATCH 2/3] async: always set ctx->notified in aio_notify()

2020-08-04 Thread Paolo Bonzini
On 04/08/20 07:28, Stefan Hajnoczi wrote: > @@ -425,19 +425,14 @@ void aio_notify(AioContext *ctx) > smp_mb(); > if (atomic_read(&ctx->notify_me)) { > event_notifier_set(&ctx->notifier); > -atomic_mb_set(&ctx->notified, true); > } > + > +atomic_mb_set(&ctx->notif