Add a multifd test for mapped-ram with passing of fds into QEMU. This
is how libvirt will consume the feature.
There are a couple of details to the fdset mechanism:
- multifd needs two distinct file descriptors (not duplicated with
dup()) so it can enable O_DIRECT only on the channels that do
aligned IO. The dup() system call creates file descriptors that
share status flags, of which O_DIRECT is one.
- the open() access mode flags used for the fds passed into QEMU need
to match the flags QEMU uses to open the file. Currently O_WRONLY
for src and O_RDONLY for dst.
Note that fdset code goes under _WIN32 because fd passing is not
supported on Windows.
Signed-off-by: Fabiano Rosas <farosas@suse.de>
---
- dropped Peter's r-b
- stopped removing the fdset at the end of the tests. The migration
code should always cleanup after itself.
---
tests/qtest/migration-test.c | 98 ++++++++++++++++++++++++++++++++++--
1 file changed, 95 insertions(+), 3 deletions(-)
diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
index 928f0ca6ce..85a21ff5e9 100644
--- a/tests/qtest/migration-test.c
+++ b/tests/qtest/migration-test.c
@@ -2024,11 +2024,18 @@ static void test_precopy_file(void)
#ifndef _WIN32
static void fdset_add_fds(QTestState *qts, const char *file, int flags,
- int num_fds)
+ int num_fds, bool direct_io)
{
for (int i = 0; i < num_fds; i++) {
int fd;
+#ifdef O_DIRECT
+ /* only secondary channels can use direct-io */
+ if (direct_io && i != 0) {
+ flags |= O_DIRECT;
+ }
+#endif
+
fd = open(file, flags, 0660);
assert(fd != -1);
@@ -2042,8 +2049,8 @@ static void *file_offset_fdset_start_hook(QTestState *from, QTestState *to)
{
g_autofree char *file = g_strdup_printf("%s/%s", tmpfs, FILE_TEST_FILENAME);
- fdset_add_fds(from, file, O_WRONLY, 1);
- fdset_add_fds(to, file, O_RDONLY, 1);
+ fdset_add_fds(from, file, O_WRONLY, 1, false);
+ fdset_add_fds(to, file, O_RDONLY, 1, false);
return NULL;
}
@@ -2215,6 +2222,84 @@ static void test_multifd_file_mapped_ram_dio(void)
test_file_common(&args, true);
}
+#ifndef _WIN32
+static void multifd_mapped_ram_fdset_end(QTestState *from, QTestState *to,
+ void *opaque)
+{
+ QDict *resp;
+ QList *fdsets;
+
+ /*
+ * Make sure no fdsets are left after migration, otherwise a
+ * second migration would fail due fdset reuse.
+ */
+ resp = qtest_qmp(from, "{'execute': 'query-fdsets', "
+ "'arguments': {}}");
+ g_assert(qdict_haskey(resp, "return"));
+ fdsets = qdict_get_qlist(resp, "return");
+ g_assert(fdsets && qlist_empty(fdsets));
+}
+
+static void *multifd_mapped_ram_fdset_dio(QTestState *from, QTestState *to)
+{
+ g_autofree char *file = g_strdup_printf("%s/%s", tmpfs, FILE_TEST_FILENAME);
+
+ fdset_add_fds(from, file, O_WRONLY, 2, true);
+ fdset_add_fds(to, file, O_RDONLY, 2, true);
+
+ migrate_multifd_mapped_ram_start(from, to);
+ migrate_set_parameter_bool(from, "direct-io", true);
+ migrate_set_parameter_bool(to, "direct-io", true);
+
+ return NULL;
+}
+
+static void *multifd_mapped_ram_fdset(QTestState *from, QTestState *to)
+{
+ g_autofree char *file = g_strdup_printf("%s/%s", tmpfs, FILE_TEST_FILENAME);
+
+ fdset_add_fds(from, file, O_WRONLY, 2, false);
+ fdset_add_fds(to, file, O_RDONLY, 2, false);
+
+ migrate_multifd_mapped_ram_start(from, to);
+
+ return NULL;
+}
+
+static void test_multifd_file_mapped_ram_fdset(void)
+{
+ g_autofree char *uri = g_strdup_printf("file:/dev/fdset/1,offset=%d",
+ FILE_TEST_OFFSET);
+ MigrateCommon args = {
+ .connect_uri = uri,
+ .listen_uri = "defer",
+ .start_hook = multifd_mapped_ram_fdset,
+ .finish_hook = multifd_mapped_ram_fdset_end,
+ };
+
+ test_file_common(&args, true);
+}
+
+static void test_multifd_file_mapped_ram_fdset_dio(void)
+{
+ g_autofree char *uri = g_strdup_printf("file:/dev/fdset/1,offset=%d",
+ FILE_TEST_OFFSET);
+ MigrateCommon args = {
+ .connect_uri = uri,
+ .listen_uri = "defer",
+ .start_hook = multifd_mapped_ram_fdset_dio,
+ .finish_hook = multifd_mapped_ram_fdset_end,
+ };
+
+ if (!probe_o_direct_support(tmpfs)) {
+ g_test_skip("Filesystem does not support O_DIRECT");
+ return;
+ }
+
+ test_file_common(&args, true);
+}
+#endif /* !_WIN32 */
+
static void test_precopy_tcp_plain(void)
{
MigrateCommon args = {
@@ -3654,6 +3739,13 @@ int main(int argc, char **argv)
migration_test_add("/migration/multifd/file/mapped-ram/dio",
test_multifd_file_mapped_ram_dio);
+#ifndef _WIN32
+ migration_test_add("/migration/multifd/file/mapped-ram/fdset",
+ test_multifd_file_mapped_ram_fdset);
+ migration_test_add("/migration/multifd/file/mapped-ram/fdset/dio",
+ test_multifd_file_mapped_ram_fdset_dio);
+#endif
+
#ifdef CONFIG_GNUTLS
migration_test_add("/migration/precopy/unix/tls/psk",
test_precopy_unix_tls_psk);
--
2.35.3On Mon, Jun 17, 2024 at 03:57:31PM -0300, Fabiano Rosas wrote: > Add a multifd test for mapped-ram with passing of fds into QEMU. This > is how libvirt will consume the feature. > > There are a couple of details to the fdset mechanism: > > - multifd needs two distinct file descriptors (not duplicated with > dup()) so it can enable O_DIRECT only on the channels that do > aligned IO. The dup() system call creates file descriptors that > share status flags, of which O_DIRECT is one. > > - the open() access mode flags used for the fds passed into QEMU need > to match the flags QEMU uses to open the file. Currently O_WRONLY > for src and O_RDONLY for dst. > > Note that fdset code goes under _WIN32 because fd passing is not > supported on Windows. > > Signed-off-by: Fabiano Rosas <farosas@suse.de> > --- > - dropped Peter's r-b > > - stopped removing the fdset at the end of the tests. The migration > code should always cleanup after itself. Ah, that looks also ok. Reviewed-by: Peter Xu <peterx@redhat.com> -- Peter Xu
Peter Xu <peterx@redhat.com> writes: > On Mon, Jun 17, 2024 at 03:57:31PM -0300, Fabiano Rosas wrote: >> Add a multifd test for mapped-ram with passing of fds into QEMU. This >> is how libvirt will consume the feature. >> >> There are a couple of details to the fdset mechanism: >> >> - multifd needs two distinct file descriptors (not duplicated with >> dup()) so it can enable O_DIRECT only on the channels that do >> aligned IO. The dup() system call creates file descriptors that >> share status flags, of which O_DIRECT is one. >> >> - the open() access mode flags used for the fds passed into QEMU need >> to match the flags QEMU uses to open the file. Currently O_WRONLY >> for src and O_RDONLY for dst. >> >> Note that fdset code goes under _WIN32 because fd passing is not >> supported on Windows. >> >> Signed-off-by: Fabiano Rosas <farosas@suse.de> >> --- >> - dropped Peter's r-b >> >> - stopped removing the fdset at the end of the tests. The migration >> code should always cleanup after itself. > > Ah, that looks also ok. I made a mistake here. We still need to require that the management layer explicitly removes the fds they added by calling qmp_remove_fd(). The reason I thought it was ok to not remove the fds was that after your suggestion to use monitor_fdset_free_if_empty() in patch 7, I mistakenly put monitor_fdset_free() instead. So the qmp_remove_fd() was not finding any fdsets and I thought that was QEMU removing everything. Which is silly, because the whole purpose of the patch is to not do that. I think I'll just fix this in the migration tree. It's just a revert to v2 of this patch and the correction to patch 7.
On Fri, Jun 21, 2024 at 09:33:48AM -0300, Fabiano Rosas wrote: > Peter Xu <peterx@redhat.com> writes: > > > On Mon, Jun 17, 2024 at 03:57:31PM -0300, Fabiano Rosas wrote: > >> Add a multifd test for mapped-ram with passing of fds into QEMU. This > >> is how libvirt will consume the feature. > >> > >> There are a couple of details to the fdset mechanism: > >> > >> - multifd needs two distinct file descriptors (not duplicated with > >> dup()) so it can enable O_DIRECT only on the channels that do > >> aligned IO. The dup() system call creates file descriptors that > >> share status flags, of which O_DIRECT is one. > >> > >> - the open() access mode flags used for the fds passed into QEMU need > >> to match the flags QEMU uses to open the file. Currently O_WRONLY > >> for src and O_RDONLY for dst. > >> > >> Note that fdset code goes under _WIN32 because fd passing is not > >> supported on Windows. > >> > >> Signed-off-by: Fabiano Rosas <farosas@suse.de> > >> --- > >> - dropped Peter's r-b > >> > >> - stopped removing the fdset at the end of the tests. The migration > >> code should always cleanup after itself. > > > > Ah, that looks also ok. > > I made a mistake here. We still need to require that the management > layer explicitly removes the fds they added by calling qmp_remove_fd(). > > The reason I thought it was ok to not remove the fds was that after your > suggestion to use monitor_fdset_free_if_empty() in patch 7, I mistakenly > put monitor_fdset_free() instead. So the qmp_remove_fd() was not finding > any fdsets and I thought that was QEMU removing everything. Which is > silly, because the whole purpose of the patch is to not do that. > > I think I'll just fix this in the migration tree. It's just a revert to > v2 of this patch and the correction to patch 7. Ah OK. I thought you were talking about when QEMU exit()s, which should cleanup everything too. Personally I guess I kind of ignored that remove-fd api anyway being there or not, as it seems nobody understand why it needs to exist in the first place.. -- Peter Xu
© 2016 - 2025 Red Hat, Inc.