qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Fabiano Rosas <farosas@suse.de>
To: qemu-devel@nongnu.org
Cc: Juan Quintela <quintela@redhat.com>, Peter Xu <peterx@redhat.com>,
	Lukas Straub <lukasstraub2@web.de>,
	Leonardo Bras <leobras@redhat.com>
Subject: [PATCH v6 09/10] migration/yank: Keep track of registered yank instances
Date: Mon, 11 Sep 2023 14:13:19 -0300	[thread overview]
Message-ID: <20230911171320.24372-10-farosas@suse.de> (raw)
In-Reply-To: <20230911171320.24372-1-farosas@suse.de>

The core yank code is strict about balanced registering and
unregistering of yank functions.

This creates a difficulty because the migration code registers one
yank function per QIOChannel, but each QIOChannel can be referenced by
more than one QEMUFile. The yank function should not be removed until
all QEMUFiles have been closed.

Keep a reference count of how many QEMUFiles are using a QIOChannel
that has a yank function. Only unregister the yank function when all
QEMUFiles have been closed.

This improves the current code by removing the need for the programmer
to know which QEMUFile is the last one to be cleaned up and fixes the
theoretical issue of removing the yank function while another QEMUFile
could still be using the ioc and require a yank.

Signed-off-by: Fabiano Rosas <farosas@suse.de>
---
 migration/yank_functions.c | 81 ++++++++++++++++++++++++++++++++++----
 migration/yank_functions.h |  8 ++++
 2 files changed, 81 insertions(+), 8 deletions(-)

diff --git a/migration/yank_functions.c b/migration/yank_functions.c
index 979e60c762..afdeef30ff 100644
--- a/migration/yank_functions.c
+++ b/migration/yank_functions.c
@@ -10,9 +10,32 @@
 #include "qemu/osdep.h"
 #include "io/channel.h"
 #include "yank_functions.h"
+#include "qemu/lockable.h"
 #include "qemu/yank.h"
 #include "qemu-file.h"
 
+static QemuMutex ioc_list_lock;
+static QLIST_HEAD(, Yankable) yankable_ioc_list
+    = QLIST_HEAD_INITIALIZER(yankable_ioc_list);
+
+static void __attribute__((constructor)) ioc_list_lock_init(void)
+{
+    qemu_mutex_init(&ioc_list_lock);
+}
+
+static void yankable_ref(Yankable *yankable)
+{
+    assert(yankable->refcnt > 0);
+    yankable->refcnt++;
+    assert(yankable->refcnt < INT_MAX);
+}
+
+static void yankable_unref(Yankable *yankable)
+{
+    assert(yankable->refcnt > 0);
+    yankable->refcnt--;
+}
+
 void migration_yank_iochannel(void *opaque)
 {
     QIOChannel *ioc = QIO_CHANNEL(opaque);
@@ -28,20 +51,62 @@ static bool migration_ioc_yank_supported(QIOChannel *ioc)
 
 void migration_ioc_register_yank(QIOChannel *ioc)
 {
-    if (migration_ioc_yank_supported(ioc)) {
-        yank_register_function(MIGRATION_YANK_INSTANCE,
-                               migration_yank_iochannel,
-                               ioc);
+    Yankable *new, *entry;
+
+    if (!ioc || !migration_ioc_yank_supported(ioc)) {
+        return;
     }
+
+    WITH_QEMU_LOCK_GUARD(&ioc_list_lock) {
+        QLIST_FOREACH(entry, &yankable_ioc_list, next) {
+            if (entry->opaque == ioc) {
+                yankable_ref(entry);
+                return;
+            }
+        }
+
+        new = g_new0(Yankable, 1);
+        new->refcnt = 1;
+        new->opaque = ioc;
+        object_ref(ioc);
+
+        QLIST_INSERT_HEAD(&yankable_ioc_list, new, next);
+    }
+
+    yank_register_function(MIGRATION_YANK_INSTANCE,
+                           migration_yank_iochannel,
+                           ioc);
 }
 
 void migration_ioc_unregister_yank(QIOChannel *ioc)
 {
-    if (migration_ioc_yank_supported(ioc)) {
-        yank_unregister_function(MIGRATION_YANK_INSTANCE,
-                                 migration_yank_iochannel,
-                                 ioc);
+    Yankable *entry, *tmp;
+
+    if (!ioc || !migration_ioc_yank_supported(ioc)) {
+        return;
     }
+
+    WITH_QEMU_LOCK_GUARD(&ioc_list_lock) {
+        QLIST_FOREACH_SAFE(entry, &yankable_ioc_list, next, tmp) {
+            if (entry->opaque == ioc) {
+                yankable_unref(entry);
+
+                if (!entry->refcnt) {
+                    QLIST_REMOVE(entry, next);
+                    g_free(entry);
+                    goto unreg;
+                }
+            }
+        }
+    }
+
+    return;
+
+unreg:
+    yank_unregister_function(MIGRATION_YANK_INSTANCE,
+                             migration_yank_iochannel,
+                             ioc);
+    object_unref(ioc);
 }
 
 void migration_ioc_unregister_yank_from_file(QEMUFile *file)
diff --git a/migration/yank_functions.h b/migration/yank_functions.h
index a7577955ed..c928a46f68 100644
--- a/migration/yank_functions.h
+++ b/migration/yank_functions.h
@@ -7,6 +7,14 @@
  * See the COPYING file in the top-level directory.
  */
 
+struct Yankable {
+    void *opaque;
+    int refcnt;
+    QLIST_ENTRY(Yankable) next;
+};
+
+typedef struct Yankable Yankable;
+
 /**
  * migration_yank_iochannel: yank function for iochannel
  *
-- 
2.35.3



  parent reply	other threads:[~2023-09-11 17:16 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-09-11 17:13 [PATCH v6 00/10] Fix segfault on migration return path Fabiano Rosas
2023-09-11 17:13 ` [PATCH v6 01/10] migration: Fix possible race when setting rp_state.error Fabiano Rosas
2023-09-11 17:13 ` [PATCH v6 02/10] migration: Fix possible races when shutting down the return path Fabiano Rosas
2023-09-11 17:13 ` [PATCH v6 03/10] migration: Fix possible race when shutting down to_dst_file Fabiano Rosas
2023-09-11 17:13 ` [PATCH v6 04/10] migration: Remove redundant cleanup of postcopy_qemufile_src Fabiano Rosas
2023-09-11 17:13 ` [PATCH v6 05/10] migration: Consolidate return path closing code Fabiano Rosas
2023-09-11 17:13 ` [PATCH v6 06/10] migration: Replace the return path retry logic Fabiano Rosas
2023-09-11 17:13 ` [PATCH v6 07/10] migration: Move return path cleanup to main migration thread Fabiano Rosas
2023-09-11 17:13 ` [PATCH v6 08/10] migration/yank: Use channel features Fabiano Rosas
2023-09-11 20:46   ` Philippe Mathieu-Daudé
2023-09-13 20:05   ` Peter Xu
2024-01-22 20:08     ` Fabiano Rosas
2024-01-23  1:27       ` Peter Xu
2023-09-11 17:13 ` Fabiano Rosas [this message]
2023-09-13 20:13   ` [PATCH v6 09/10] migration/yank: Keep track of registered yank instances Peter Xu
2023-09-13 21:53     ` Fabiano Rosas
2023-09-13 23:48       ` Peter Xu
2023-09-14 13:23         ` Fabiano Rosas
2023-09-14 14:57           ` Peter Xu
2023-09-25  7:38             ` Lukas Straub
2023-09-25 12:20               ` Fabiano Rosas
2023-09-25 15:32                 ` Lukas Straub
2023-09-11 17:13 ` [PATCH v6 10/10] migration: Add a wrapper to cleanup migration files Fabiano Rosas

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20230911171320.24372-10-farosas@suse.de \
    --to=farosas@suse.de \
    --cc=leobras@redhat.com \
    --cc=lukasstraub2@web.de \
    --cc=peterx@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=quintela@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).