| Message ID | 3d6449d4df25bcdd3e807eff169f46f1385e5257.1732465720.git.qemu_oss@crudebyte.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | 9pfs: fix fstat() after unlink() (with a Linux guest) | expand |
Hi Christian, On Wed, 21 Feb 2024 15:13:13 +0100 Christian Schoenebeck <qemu_oss@crudebyte.com> wrote: > After removing a file from the file system, we should still be able to > work with the file if we already had it open before removal. > > As a first step we verify that it is possible to write to an unlinked > file, as this is what already works. This test is extended later on > after having fixed other use cases after unlink that are not working > yet. > > Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com> > --- Test looks good but make sure it is merged last to preserve bisect. Reviewed-by: Greg Kurz <groug@kaod.org> > tests/qtest/virtio-9p-test.c | 41 ++++++++++++++++++++++++++++++++++++ > 1 file changed, 41 insertions(+) > > diff --git a/tests/qtest/virtio-9p-test.c b/tests/qtest/virtio-9p-test.c > index 3c8cd235cf..f6d7400a87 100644 > --- a/tests/qtest/virtio-9p-test.c > +++ b/tests/qtest/virtio-9p-test.c > @@ -693,6 +693,45 @@ static void fs_unlinkat_hardlink(void *obj, void *data, > g_assert(stat(real_file, &st_real) == 0); > } > > +static void fs_use_after_unlink(void *obj, void *data, > + QGuestAllocator *t_alloc) > +{ > + QVirtio9P *v9p = obj; > + v9fs_set_allocator(t_alloc); > + static const uint32_t write_count = P9_MAX_SIZE / 2; > + g_autofree char *real_file = virtio_9p_test_path("09/doa_file"); > + g_autofree char *buf = g_malloc0(write_count); > + struct stat st_file; > + uint32_t fid_file; > + uint32_t count; > + > + tattach({ .client = v9p }); > + > + /* create a file "09/doa_file" and make sure it exists and is regular */ > + tmkdir({ .client = v9p, .atPath = "/", .name = "09" }); > + tlcreate({ .client = v9p, .atPath = "09", .name = "doa_file" }); > + g_assert(stat(real_file, &st_file) == 0); > + g_assert((st_file.st_mode & S_IFMT) == S_IFREG); > + > + /* request a FID for that regular file that we can work with next */ > + fid_file = twalk({ > + .client = v9p, .fid = 0, .path = "09/doa_file" > + }).newfid; > + g_assert(fid_file != 0); > + > + /* now first open the file in write mode before ... */ > + tlopen({ .client = v9p, .fid = fid_file, .flags = O_WRONLY }); > + /* ... removing the file from file system */ > + tunlinkat({ .client = v9p, .atPath = "09", .name = "doa_file" }); > + > + /* file is removed, but we still have it open, so this should succeed */ > + count = twrite({ > + .client = v9p, .fid = fid_file, .offset = 0, .count = write_count, > + .data = buf > + }).count; > + g_assert_cmpint(count, ==, write_count); > +} > + > static void cleanup_9p_local_driver(void *data) > { > /* remove previously created test dir when test is completed */ > @@ -758,6 +797,8 @@ static void register_virtio_9p_test(void) > qos_add_test("local/hardlink_file", "virtio-9p", fs_hardlink_file, &opts); > qos_add_test("local/unlinkat_hardlink", "virtio-9p", fs_unlinkat_hardlink, > &opts); > + qos_add_test("local/use_after_unlink", "virtio-9p", fs_use_after_unlink, > + &opts); > } > > libqos_init(register_virtio_9p_test); Cheers,
On Monday, November 25, 2024 9:47:17 AM CET Greg Kurz wrote: > Hi Christian, > > On Wed, 21 Feb 2024 15:13:13 +0100 > Christian Schoenebeck <qemu_oss@crudebyte.com> wrote: > > > After removing a file from the file system, we should still be able to > > work with the file if we already had it open before removal. > > > > As a first step we verify that it is possible to write to an unlinked > > file, as this is what already works. This test is extended later on > > after having fixed other use cases after unlink that are not working > > yet. > > > > Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com> > > --- > > Test looks good but make sure it is merged last to preserve bisect. I think there is a misapprehension: this test already passed! So no need to move this patch. What this test does is verifying the scenario open-unlink-write. I already sent this patch in February and was surprised by myself that this idiom already works: https://lore.kernel.org/all/E1rcnYJ-0004KK-LV@lizzy.crudebyte.com/ What this entire series (i.e. patch 5) rather fixes is the idiom open-unlink-fstat, and the test for this idiom is the last patch, not this first one here. So bisect is already fine. > Reviewed-by: Greg Kurz <groug@kaod.org> Thanks! /Christian
diff --git a/tests/qtest/virtio-9p-test.c b/tests/qtest/virtio-9p-test.c index 3c8cd235cf..f6d7400a87 100644 --- a/tests/qtest/virtio-9p-test.c +++ b/tests/qtest/virtio-9p-test.c @@ -693,6 +693,45 @@ static void fs_unlinkat_hardlink(void *obj, void *data, g_assert(stat(real_file, &st_real) == 0); } +static void fs_use_after_unlink(void *obj, void *data, + QGuestAllocator *t_alloc) +{ + QVirtio9P *v9p = obj; + v9fs_set_allocator(t_alloc); + static const uint32_t write_count = P9_MAX_SIZE / 2; + g_autofree char *real_file = virtio_9p_test_path("09/doa_file"); + g_autofree char *buf = g_malloc0(write_count); + struct stat st_file; + uint32_t fid_file; + uint32_t count; + + tattach({ .client = v9p }); + + /* create a file "09/doa_file" and make sure it exists and is regular */ + tmkdir({ .client = v9p, .atPath = "/", .name = "09" }); + tlcreate({ .client = v9p, .atPath = "09", .name = "doa_file" }); + g_assert(stat(real_file, &st_file) == 0); + g_assert((st_file.st_mode & S_IFMT) == S_IFREG); + + /* request a FID for that regular file that we can work with next */ + fid_file = twalk({ + .client = v9p, .fid = 0, .path = "09/doa_file" + }).newfid; + g_assert(fid_file != 0); + + /* now first open the file in write mode before ... */ + tlopen({ .client = v9p, .fid = fid_file, .flags = O_WRONLY }); + /* ... removing the file from file system */ + tunlinkat({ .client = v9p, .atPath = "09", .name = "doa_file" }); + + /* file is removed, but we still have it open, so this should succeed */ + count = twrite({ + .client = v9p, .fid = fid_file, .offset = 0, .count = write_count, + .data = buf + }).count; + g_assert_cmpint(count, ==, write_count); +} + static void cleanup_9p_local_driver(void *data) { /* remove previously created test dir when test is completed */ @@ -758,6 +797,8 @@ static void register_virtio_9p_test(void) qos_add_test("local/hardlink_file", "virtio-9p", fs_hardlink_file, &opts); qos_add_test("local/unlinkat_hardlink", "virtio-9p", fs_unlinkat_hardlink, &opts); + qos_add_test("local/use_after_unlink", "virtio-9p", fs_use_after_unlink, + &opts); } libqos_init(register_virtio_9p_test);
After removing a file from the file system, we should still be able to work with the file if we already had it open before removal. As a first step we verify that it is possible to write to an unlinked file, as this is what already works. This test is extended later on after having fixed other use cases after unlink that are not working yet. Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com> --- tests/qtest/virtio-9p-test.c | 41 ++++++++++++++++++++++++++++++++++++ 1 file changed, 41 insertions(+)