09.06.26 18:32, Kevin Wolf пишет:
Am 09.06.2026 um 14:10 hat Vladimir Sementsov-Ogievskiy geschrieben:
08.06.26 17:23, Stefan Hajnoczi пишет:
Hi Vladimir,
It looks like there is a race condition in qemu-iotests 205 when the
NBD server is shutting down:
+FAIL: test_remove_during_connect_safe_hard (__main__.TestNbdServerRemove)
+----------------------------------------------------------------------
+Traceback (most recent call last):
+ File "/builds/qemu-project/qemu/tests/qemu-iotests/205", line 149,
in test_remove_during_connect_safe_hard
+ self.assertExportNotFound('exp')
+ File "/builds/qemu-project/qemu/tests/qemu-iotests/205", line 63, in
assertExportNotFound
+ self.assert_qmp(result, 'error/desc', "Export 'exp' is not found")
+ File "/builds/qemu-project/qemu/tests/qemu-iotests/iotests.py", line
1246, in assert_qmp
+ self.assertEqual(result, value,
+AssertionError: "Export 'exp' is already shutting down" != "Export
'exp' is not found"
+- Export 'exp' is already shutting down
++ Export 'exp' is not found
+ : "error/desc" is "Export 'exp' is already shutting down", expected
"Export 'exp' is not found"
https://gitlab.com/qemu-project/qemu/-/jobs/14745043965#L328
I have not seen this CI failure before, so it might be rare and hard
to reproduce.
Stefan
Hi! Looks like a degradation in commit
commit 3c3bc462adeb561f5dfdcbb84ae691c95ccef916
Author: Kevin Wolf <[email protected]>
Date: Thu Sep 24 17:27:06 2020 +0200
block/export: Add block-export-del
Implement a new QMP command block-export-del and make nbd-server-remove
a wrapper around it.
Before that commit we search for export by nbd_export_find(), and it fails
with "Export 'exp' is not found" as expected, because prior
qmp_nbd_server_remove(mode=hard) does
nbd_export_remove() -> nbd_export_request_shutdown() ->
QTAILQ_REMOVE(&exports, exp, next);
when nbd_export_find() searches for that export exactly in this "exports" list
(static global in nbd/server.c)
Starting with 3c3bc462adeb5
qmp_nbd_server_remove() searches for export by blk_export_find(), which
searches in
block_exports list in block/export/export.c.
Previous qmp_nbd_server_remove(mode=hard) does
qmp_block_export_del() -> blk_exp_request_shutdown() ->
nbd_export_request_shutdown(),
which removes the export only from "exports" list in nbd/server.c.
How the export is removed from block_exports list? It is done in
blk_exp_delete_bh(),
scheduled by
void blk_exp_unref(BlockExport *exp)
{
assert(exp->refcount > 0);
if (--exp->refcount == 0) {
/* Touch the block_exports list only in the main thread */
aio_bh_schedule_oneshot(qemu_get_aio_context(), blk_exp_delete_bh,
exp);
}
}
---
I think, all the references should be removed druing qmp
nbd-server-remove(hard) call,
but probably, we still have a (small) chance of doing this check in test
self.assertExportNotFound('exp')
and get unexpected answer _before_ scheduled removal actually done.
---
blk_exp_unref() does scheduling to main thread since
commit bc4ee65b8c309ed6a726e3ea1b73f7fa31b4bb95
Author: Kevin Wolf <[email protected]>
Date: Thu Sep 24 17:27:03 2020 +0200
block/export: Add blk_exp_close_all(_type)
---
Not sure, how to properly fix it... Of course, better is to support old
behavior,
when action of nbd-server-remove(hard) was synchronous.
I guess you can always add one more AIO_WAIT_WHILE(), this time to
nbd-server-remove, but blocking QMP commands aren't really nice, so I'd
rather avoid that. In the case of nbd-server-remove, if you analysis is
correct, the current behaviour has been there for almost six years.
So my gut feeling is that it's better to fix the test case to wait for
the BLOCK_EXPORT_DELETED event.
And then I saw that nbd-server-add and nbd-server-remove have actually
been deprecated since QEMU 5.2, so maybe the best way forward is really
to just delete them? That will still require changing the test case, of
course.
Agree, these are strong arguments, thanks. I'll prepare patches for removing
deprecated commands and changeing the test.
--
Best regards,
Vladimir
