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.
--
Best regards,
Vladimir
