diff mbox series

[v2,1/4] tests/qtest/migration-test: Use regular file file for shared-memory tests

Message ID 20240530095408.31608-2-npiggin@gmail.com
State New
Headers show
Series tests/qtest/migration-test: Improve shared | expand

Commit Message

Nicholas Piggin May 30, 2024, 9:54 a.m. UTC 
There is no need to use /dev/shm for file-backed memory devices, and
on Gitlab CI the tmpfs mount is too small to be usable for migration
tests. Switch to using a regular file in /tmp/ which will usually have
more space available.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 tests/qtest/migration-test.c | 34 +++++++++++++++-------------------
 1 file changed, 15 insertions(+), 19 deletions(-)

Comments

Prasad Pandit May 31, 2024, 7:03 a.m. UTC | #1 
On Thu, 30 May 2024 at 15:25, Nicholas Piggin <npiggin@gmail.com> wrote:
> There is no need to use /dev/shm for file-backed memory devices, and
> on Gitlab CI the tmpfs mount is too small to be usable for migration
> tests. Switch to using a regular file in /tmp/ which will usually have
> more space available.
...
> -    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(
...
>      g_autofree char *uri = g_strdup_printf("unix:%s/migsocket", tmpfs);
>      QTestState *from, *to;
...
>      g_autofree char *uri = g_strdup_printf("file:%s/%s", tmpfs, FILE_TEST_FILENAME);

* Maybe the 'tmpfs' variable needs to be renamed to indicate that it
uses '/tmp/' or '/var/tmp' directory to create temporary files. And it
is not in memory tmpfs(5) used for shared memory '/dev/shm'. Commit
message above says 'tmpfs mount is too small' and above calls continue
to use 'tmpfs' variable to create temporary files. It's a little
confusing.

Otherwise patch looks okay.
Reviewed-by: Prasad Pandit <pjp@fedoraproject.org>

Thank you.
---
  - Prasad
diff mbox series

Patch

diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
index d6f5ceed80..8bbf45313d 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,21 +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;
-        }
-    }
-
     dst_state = (QTestMigrationState) { };
     src_state = (QTestMigrationState) { };
     bootfile_create(tmpfs, args->suspend_me);
@@ -746,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) {
@@ -780,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) {
@@ -802,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);
@@ -814,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;
@@ -1868,6 +1861,9 @@  static void test_ignore_shared(void)
 {
     g_autofree char *uri = g_strdup_printf("unix:%s/migsocket", tmpfs);
     QTestState *from, *to;
+    MigrateStart args = {
+        .use_memfile = true,
+    };
 
     if (test_migrate_start(&from, &to, uri, false, true, NULL, NULL)) {
         return;
@@ -2024,7 +2020,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