On Tue, May 28, 2024 at 02:27:57PM +1000, Nicholas Piggin wrote: > There is no need to use /dev/shm for file-backed memory devices, and > it is too small to be usable in gitlab CI. Switch to using a regular > file in /tmp/ which will usually have more space available. > > Signed-off-by: Nicholas Piggin <npig...@gmail.com> > --- > Am I missing something? AFAIKS there is not even any point using > /dev/shm aka tmpfs anyway, there is not much special about it as a > filesystem. This applies on top of the series just sent, and passes > gitlab CI qtests including aarch64.
I think it's just that /dev/shm guarantees shmem usage, while the var "tmpfs" implies g_dir_make_tmp() which may be another non-ram based file system, while that'll be slightly different comparing to what a real user would use - we don't suggest user to put guest RAM on things like btrfs. One real implication is if we add a postcopy test it'll fail with g_dir_make_tmp() when it is not pointing to a shmem mount, as UFFDIO_REGISTER will fail there. But that test doesn't yet exist as the QEMU paths should be the same even if Linux will trigger different paths when different types of mem is used (anonymous v.s. shmem). If the goal here is to properly handle the case where tmpfs doesn't have enough space, how about what I suggested in the other email? https://lore.kernel.org/r/ZlSppKDE6wzjCF--@x1n IOW, try populate the shmem region before starting the guest, skip if population failed. Would that work? Thanks, > > Thanks, > Nick > > tests/qtest/migration-test.c | 41 ++++++++++++------------------------ > 1 file changed, 13 insertions(+), 28 deletions(-) > > diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c > index 45830eb213..86eace354e 100644 > --- a/tests/qtest/migration-test.c > +++ b/tests/qtest/migration-test.c > @@ -552,7 +552,7 @@ typedef struct { > * unconditionally, because it means the user would like to be verbose. > */ > bool hide_stderr; > - bool use_shmem; > + bool use_memfile; > /* only launch the target process */ > bool only_target; > /* Use dirty ring if true; dirty logging otherwise */ > @@ -672,29 +672,14 @@ static int test_migrate_start(QTestState **from, > QTestState **to, > g_autofree gchar *cmd_source = NULL; > g_autofree gchar *cmd_target = NULL; > const gchar *ignore_stderr; > - g_autofree char *shmem_opts = NULL; > - g_autofree char *shmem_path = NULL; > + g_autofree char *memfile_opts = NULL; > + g_autofree char *memfile_path = NULL; > const char *kvm_opts = NULL; > const char *arch = qtest_get_arch(); > const char *memory_size; > const char *machine_alias, *machine_opts = ""; > g_autofree char *machine = NULL; > > - if (args->use_shmem) { > - if (!g_file_test("/dev/shm", G_FILE_TEST_IS_DIR)) { > - g_test_skip("/dev/shm is not supported"); > - return -1; > - } > - if (getenv("GITLAB_CI")) { > - /* > - * Gitlab runners are limited to 64MB shm size. See: > - * https://lore.kernel.org/all/87ttq5fvh7....@suse.de/ > - */ > - g_test_skip("/dev/shm is not supported in Gitlab CI > environment"); > - return -1; > - } > - } > - > dst_state = (QTestMigrationState) { }; > src_state = (QTestMigrationState) { }; > bootfile_create(tmpfs, args->suspend_me); > @@ -754,12 +739,12 @@ static int test_migrate_start(QTestState **from, > QTestState **to, > ignore_stderr = ""; > } > > - if (args->use_shmem) { > - shmem_path = g_strdup_printf("/dev/shm/qemu-%d", getpid()); > - shmem_opts = g_strdup_printf( > + if (args->use_memfile) { > + memfile_path = g_strdup_printf("/%s/qemu-%d", tmpfs, getpid()); > + memfile_opts = g_strdup_printf( > "-object memory-backend-file,id=mem0,size=%s" > ",mem-path=%s,share=on -numa node,memdev=mem0", > - memory_size, shmem_path); > + memory_size, memfile_path); > } > > if (args->use_dirty_ring) { > @@ -788,7 +773,7 @@ static int test_migrate_start(QTestState **from, > QTestState **to, > memory_size, tmpfs, > arch_opts ? arch_opts : "", > arch_source ? arch_source : "", > - shmem_opts ? shmem_opts : "", > + memfile_opts ? memfile_opts : "", > args->opts_source ? args->opts_source : "", > ignore_stderr); > if (!args->only_target) { > @@ -810,7 +795,7 @@ static int test_migrate_start(QTestState **from, > QTestState **to, > memory_size, tmpfs, uri, > arch_opts ? arch_opts : "", > arch_target ? arch_target : "", > - shmem_opts ? shmem_opts : "", > + memfile_opts ? memfile_opts : "", > args->opts_target ? args->opts_target : "", > ignore_stderr); > *to = qtest_init_with_env(QEMU_ENV_DST, cmd_target); > @@ -822,8 +807,8 @@ static int test_migrate_start(QTestState **from, > QTestState **to, > * Remove shmem file immediately to avoid memory leak in test failed > case. > * It's valid because QEMU has already opened this file > */ > - if (args->use_shmem) { > - unlink(shmem_path); > + if (args->use_memfile) { > + unlink(memfile_path); > } > > return 0; > @@ -1875,7 +1860,7 @@ static void test_ignore_shared(void) > g_autofree char *uri = g_strdup_printf("unix:%s/migsocket", tmpfs); > QTestState *from, *to; > MigrateStart args = { > - .use_shmem = true, > + .use_memfile = true, > }; > > if (test_migrate_start(&from, &to, uri, &args)) { > @@ -2033,7 +2018,7 @@ static void test_mode_reboot(void) > g_autofree char *uri = g_strdup_printf("file:%s/%s", tmpfs, > FILE_TEST_FILENAME); > MigrateCommon args = { > - .start.use_shmem = true, > + .start.use_memfile = true, > .connect_uri = uri, > .listen_uri = "defer", > .start_hook = test_mode_reboot_start > -- > 2.43.0 > -- Peter Xu
