[PATCH] migration: fix QIOChannelFile leak on error in file_connect_outgoing

Trieu Huynh posted 1 patch 5 days ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20260328121215.159532-1-vikingtc4@gmail.com
Maintainers: Peter Xu <peterx@redhat.com>, Fabiano Rosas <farosas@suse.de>, Laurent Vivier <lvivier@redhat.com>, Paolo Bonzini <pbonzini@redhat.com>
migration/file.c                   |  2 ++
tests/qtest/migration/file-tests.c | 48 ++++++++++++++++++++++++++++++
2 files changed, 50 insertions(+)
[PATCH] migration: fix QIOChannelFile leak on error in file_connect_outgoing
Posted by Trieu Huynh 5 days ago
Commit 03a680c978 changed g_autoptr(QIOChannelFile) to a plain pointer
but failed to restore the necessary object_unref() calls on error paths.
Previously, these were handled implicitly by the g_autoptr cleanup
mechanism.

Two error paths currently leak the QIOChannelFile object and its
underlying file descriptor:

  1. When ftruncate() fails (e.g., on character or block devices).
  2. When qio_channel_io_seek() fails after the channel is created.

In environments that retry migration automatically (e.g., libvirt),
these FDs accumulate until QEMU hits RLIMIT_NOFILE and fails with
EMFILE (Too many open files).

Add the missing object_unref() calls to both error paths to ensure
resources are properly released.

Signed-off-by: Trieu Huynh <vikingtc4@gmail.com>
---
 migration/file.c                   |  2 ++
 tests/qtest/migration/file-tests.c | 48 ++++++++++++++++++++++++++++++
 2 files changed, 50 insertions(+)

diff --git a/migration/file.c b/migration/file.c
index 5618aced49..962b4b3a1b 100644
--- a/migration/file.c
+++ b/migration/file.c
@@ -112,6 +112,7 @@ QIOChannel *file_connect_outgoing(MigrationState *s,
         error_setg_errno(errp, errno,
                          "failed to truncate migration file to offset %" PRIx64,
                          offset);
+        object_unref(OBJECT(fioc));
         goto out;
     }
 
@@ -119,6 +120,7 @@ QIOChannel *file_connect_outgoing(MigrationState *s,
 
     ioc = QIO_CHANNEL(fioc);
     if (offset && qio_channel_io_seek(ioc, offset, SEEK_SET, errp) < 0) {
+        object_unref(OBJECT(fioc));
         ioc = NULL;
         goto out;
     }
diff --git a/tests/qtest/migration/file-tests.c b/tests/qtest/migration/file-tests.c
index 5d1b861cf6..fef172068f 100644
--- a/tests/qtest/migration/file-tests.c
+++ b/tests/qtest/migration/file-tests.c
@@ -20,6 +20,51 @@
 
 static char *tmpfs;
 
+static int count_proc_fds(pid_t pid)
+{
+    g_autofree char *fddir = g_strdup_printf("/proc/%d/fd", (int)pid);
+    GDir *dir = g_dir_open(fddir, 0, NULL);
+    int count = 0;
+
+    if (!dir) {
+        return -1;
+    }
+    while (g_dir_read_name(dir)) {
+        count++;
+    }
+    g_dir_close(dir);
+    return count;
+}
+
+static void test_file_connect_outgoing_fd_leak(char *name, MigrateCommon *args)
+{
+    QTestState *from, *to;
+    int fd_before, fd_after;
+    const int retries = 5;
+    int i;
+    if (!g_file_test("/dev/full", G_FILE_TEST_EXISTS)) {
+        g_test_skip("/dev/full not available");
+        return;
+    }
+
+    args->listen_uri = "defer";
+    if (migrate_start(&from, &to, args->listen_uri, &args->start)) {
+        return;
+    }
+
+    fd_before = count_proc_fds(qtest_pid(from));
+    g_assert_cmpint(fd_before, >, 0);
+    for (i = 0; i < retries; i++) {
+        migrate_qmp_fail(from, "file:/dev/full", NULL, "{}");
+        migration_event_wait(from, "failed");
+    }
+
+    fd_after = count_proc_fds(qtest_pid(from));
+    g_assert_cmpint(fd_after, >, 0);
+    g_assert_cmpint(fd_after, ==, fd_before);
+    migrate_end(from, to, false);
+}
+
 static void test_precopy_file(char *name, MigrateCommon *args)
 {
     g_autofree char *uri = g_strdup_printf("file:%s/%s", tmpfs,
@@ -314,6 +359,9 @@ void migration_test_add_file(MigrationTestEnv *env)
     migration_test_add("/migration/precopy/file/offset/bad",
                        test_precopy_file_offset_bad);
 
+    migration_test_add("/migration/precopy/file/connect-outgoing-fd-leak",
+                       test_file_connect_outgoing_fd_leak);
+
     migration_test_add("/migration/precopy/file/mapped-ram",
                        test_precopy_file_mapped_ram);
     migration_test_add("/migration/precopy/file/mapped-ram/live",
-- 
2.43.0
Re: [PATCH] migration: fix QIOChannelFile leak on error in file_connect_outgoing
Posted by Peter Xu 2 days, 15 hours ago
On Sat, Mar 28, 2026 at 09:12:14PM +0900, Trieu Huynh wrote:
> Commit 03a680c978 changed g_autoptr(QIOChannelFile) to a plain pointer
> but failed to restore the necessary object_unref() calls on error paths.
> Previously, these were handled implicitly by the g_autoptr cleanup
> mechanism.
> 
> Two error paths currently leak the QIOChannelFile object and its
> underlying file descriptor:
> 
>   1. When ftruncate() fails (e.g., on character or block devices).
>   2. When qio_channel_io_seek() fails after the channel is created.
> 
> In environments that retry migration automatically (e.g., libvirt),
> these FDs accumulate until QEMU hits RLIMIT_NOFILE and fails with
> EMFILE (Too many open files).
> 
> Add the missing object_unref() calls to both error paths to ensure
> resources are properly released.
> 
> Signed-off-by: Trieu Huynh <vikingtc4@gmail.com>

Good catch.

Reviewed-by: Peter Xu <peterx@redhat.com>

I'm just curious, how did you find it?

-- 
Peter Xu
Re: [PATCH] migration: fix QIOChannelFile leak on error in file_connect_outgoing
Posted by Trieu Huynh 16 hours ago
On Mon, Mar 30, 2026 at 04:52:42PM -0400, Peter Xu wrote:
> On Sat, Mar 28, 2026 at 09:12:14PM +0900, Trieu Huynh wrote:
> > Commit 03a680c978 changed g_autoptr(QIOChannelFile) to a plain pointer
> > but failed to restore the necessary object_unref() calls on error paths.
> > Previously, these were handled implicitly by the g_autoptr cleanup
> > mechanism.
> > 
> > Two error paths currently leak the QIOChannelFile object and its
> > underlying file descriptor:
> > 
> >   1. When ftruncate() fails (e.g., on character or block devices).
> >   2. When qio_channel_io_seek() fails after the channel is created.
> > 
> > In environments that retry migration automatically (e.g., libvirt),
> > these FDs accumulate until QEMU hits RLIMIT_NOFILE and fails with
> > EMFILE (Too many open files).
> > 
> > Add the missing object_unref() calls to both error paths to ensure
> > resources are properly released.
> > 
> > Signed-off-by: Trieu Huynh <vikingtc4@gmail.com>
> 
> Good catch.
> 
> Reviewed-by: Peter Xu <peterx@redhat.com>
> 
> I'm just curious, how did you find it?
> 
Thanks, just by change, while researching the codebase and blame
some locs :)
> -- 
> Peter Xu
>