The local 9p driver in virtio-9p-test.c its temporary dir right at the start of qos-test (via virtio_9p_create_local_test_dir()) and only deletes it after qos-test is finished (via virtio_9p_remove_local_test_dir()).
This means that any qos-test machine that ends up running virtio-9p-test local tests more than once will end up re-using the same temp dir. This is what's happening in [1] after we introduced the riscv machine nodes: if we enable slow tests with the '-m slow' flag using qemu-system-riscv64, this is what happens: - a temp dir is created, e.g. qtest-9p-local-WZLDL2; - virtio-9p-device tests will run virtio-9p-test successfully; - virtio-9p-pci tests will run virtio-9p-test, and fail right at the first slow test at fs_create_dir() because the "01" file was already created by fs_create_dir() test when running with the virtio-9p-device. We can fix it by making every test clean up their changes in the filesystem after they're done. But we don't need every test either: what fs_create_file() does is already exercised in fs_unlinkat_dir(), i.e. a dir is created, verified to be created, and then removed. Fixing fs_create_file() would turn it into fs_unlikat_dir(), so we don't need both. The same theme follows every test in virtio-9p-test.c, where the 'unlikat' variant does the same thing the 'create' does but with some cleaning in the end. Consolide some tests as follows: - fs_create_dir() is removed. fs_unlinkat_dir() is renamed to fs_create_unlinkat_dir(); - fs_create_file() is removed. fs_unlinkat_file() is renamed to fs_create_unlinkat_file(). The "04" dir it uses is now being removed; - fs_symlink_file() is removed. fs_unlinkat_symlink() is renamed to fs_create_unlinkat_symlink(). Both "real_file" and the "06" dir it creates is now being removed. We're still missing the 'hardlink' tests. We'll do it in the next patch since it's less trivial to consolidate than these. [1] https://mail.gnu.org/archive/html/qemu-devel/2024-03/msg05807.html Reported-by: Thomas Huth <th...@redhat.com> Signed-off-by: Daniel Henrique Barboza <dbarb...@ventanamicro.com> --- tests/qtest/virtio-9p-test.c | 97 +++++++++++------------------------- 1 file changed, 29 insertions(+), 68 deletions(-) diff --git a/tests/qtest/virtio-9p-test.c b/tests/qtest/virtio-9p-test.c index 65e69491e5..cdbe3e78ea 100644 --- a/tests/qtest/virtio-9p-test.c +++ b/tests/qtest/virtio-9p-test.c @@ -506,26 +506,8 @@ static void fs_readdir_split_512(void *obj, void *data, /* tests using the 9pfs 'local' fs driver */ -static void fs_create_dir(void *obj, void *data, QGuestAllocator *t_alloc) -{ - QVirtio9P *v9p = obj; - v9fs_set_allocator(t_alloc); - struct stat st; - g_autofree char *root_path = virtio_9p_test_path(""); - g_autofree char *new_dir = virtio_9p_test_path("01"); - - g_assert(root_path != NULL); - - tattach({ .client = v9p }); - tmkdir({ .client = v9p, .atPath = "/", .name = "01" }); - - /* check if created directory really exists now ... */ - g_assert(stat(new_dir, &st) == 0); - /* ... and is actually a directory */ - g_assert((st.st_mode & S_IFMT) == S_IFDIR); -} - -static void fs_unlinkat_dir(void *obj, void *data, QGuestAllocator *t_alloc) +static void fs_create_unlinkat_dir(void *obj, void *data, + QGuestAllocator *t_alloc) { QVirtio9P *v9p = obj; v9fs_set_allocator(t_alloc); @@ -551,28 +533,13 @@ static void fs_unlinkat_dir(void *obj, void *data, QGuestAllocator *t_alloc) g_assert(stat(new_dir, &st) != 0); } -static void fs_create_file(void *obj, void *data, QGuestAllocator *t_alloc) -{ - QVirtio9P *v9p = obj; - v9fs_set_allocator(t_alloc); - struct stat st; - g_autofree char *new_file = virtio_9p_test_path("03/1st_file"); - - tattach({ .client = v9p }); - tmkdir({ .client = v9p, .atPath = "/", .name = "03" }); - tlcreate({ .client = v9p, .atPath = "03", .name = "1st_file" }); - - /* check if created file exists now ... */ - g_assert(stat(new_file, &st) == 0); - /* ... and is a regular file */ - g_assert((st.st_mode & S_IFMT) == S_IFREG); -} - -static void fs_unlinkat_file(void *obj, void *data, QGuestAllocator *t_alloc) +static void fs_create_unlinkat_file(void *obj, void *data, + QGuestAllocator *t_alloc) { QVirtio9P *v9p = obj; v9fs_set_allocator(t_alloc); struct stat st; + g_autofree char *new_dir = virtio_9p_test_path("04"); g_autofree char *new_file = virtio_9p_test_path("04/doa_file"); tattach({ .client = v9p }); @@ -587,37 +554,22 @@ static void fs_unlinkat_file(void *obj, void *data, QGuestAllocator *t_alloc) tunlinkat({ .client = v9p, .atPath = "04", .name = "doa_file" }); /* file should be gone now */ g_assert(stat(new_file, &st) != 0); -} - -static void fs_symlink_file(void *obj, void *data, QGuestAllocator *t_alloc) -{ - QVirtio9P *v9p = obj; - v9fs_set_allocator(t_alloc); - struct stat st; - g_autofree char *real_file = virtio_9p_test_path("05/real_file"); - g_autofree char *symlink_file = virtio_9p_test_path("05/symlink_file"); - tattach({ .client = v9p }); - tmkdir({ .client = v9p, .atPath = "/", .name = "05" }); - tlcreate({ .client = v9p, .atPath = "05", .name = "real_file" }); - g_assert(stat(real_file, &st) == 0); - g_assert((st.st_mode & S_IFMT) == S_IFREG); - - tsymlink({ - .client = v9p, .atPath = "05", .name = "symlink_file", - .symtgt = "real_file" + /* also cleanup dir*/ + tunlinkat({ + .client = v9p, .atPath = "/", .name = "04", + .flags = P9_DOTL_AT_REMOVEDIR }); - - /* check if created link exists now */ - g_assert(stat(symlink_file, &st) == 0); + g_assert(stat(new_dir, &st) != 0); } -static void fs_unlinkat_symlink(void *obj, void *data, - QGuestAllocator *t_alloc) +static void fs_create_unlinkat_symlink(void *obj, void *data, + QGuestAllocator *t_alloc) { QVirtio9P *v9p = obj; v9fs_set_allocator(t_alloc); struct stat st; + g_autofree char *new_dir = virtio_9p_test_path("06"); g_autofree char *real_file = virtio_9p_test_path("06/real_file"); g_autofree char *symlink_file = virtio_9p_test_path("06/symlink_file"); @@ -636,6 +588,16 @@ static void fs_unlinkat_symlink(void *obj, void *data, tunlinkat({ .client = v9p, .atPath = "06", .name = "symlink_file" }); /* symlink should be gone now */ g_assert(stat(symlink_file, &st) != 0); + + /* remove real file and dir */ + tunlinkat({ .client = v9p, .atPath = "06", .name = "real_file" }); + g_assert(stat(real_file, &st) != 0); + + tunlinkat({ + .client = v9p, .atPath = "/", .name = "06", + .flags = P9_DOTL_AT_REMOVEDIR + }); + g_assert(stat(new_dir, &st) != 0); } static void fs_hardlink_file(void *obj, void *data, QGuestAllocator *t_alloc) @@ -746,13 +708,12 @@ static void register_virtio_9p_test(void) opts.before = assign_9p_local_driver; qos_add_test("local/config", "virtio-9p", pci_config, &opts); - qos_add_test("local/create_dir", "virtio-9p", fs_create_dir, &opts); - qos_add_test("local/unlinkat_dir", "virtio-9p", fs_unlinkat_dir, &opts); - qos_add_test("local/create_file", "virtio-9p", fs_create_file, &opts); - qos_add_test("local/unlinkat_file", "virtio-9p", fs_unlinkat_file, &opts); - qos_add_test("local/symlink_file", "virtio-9p", fs_symlink_file, &opts); - qos_add_test("local/unlinkat_symlink", "virtio-9p", fs_unlinkat_symlink, - &opts); + qos_add_test("local/create_unlinkat_dir", "virtio-9p", + fs_create_unlinkat_dir, &opts); + qos_add_test("local/create_unlinkat_file", "virtio-9p", + fs_create_unlinkat_file, &opts); + qos_add_test("local/create_unlinkat_symlink", "virtio-9p", + fs_create_unlinkat_symlink, &opts); qos_add_test("local/hardlink_file", "virtio-9p", fs_hardlink_file, &opts); qos_add_test("local/unlinkat_hardlink", "virtio-9p", fs_unlinkat_hardlink, &opts); -- 2.44.0
