From: Peter Xu <peterx@redhat.com>
To: qemu-devel@nongnu.org
Cc: Peter Maydell <peter.maydell@linaro.org>,
Lukas Straub <lukasstraub2@web.de>,
"Daniel P . Berrange" <berrange@redhat.com>,
Juan Quintela <quintela@redhat.com>,
"Dr . David Alan Gilbert" <dgilbert@redhat.com>,
peterx@redhat.com,
Leonardo Bras Soares Passos <lsoaresp@redhat.com>
Subject: [PATCH v2 5/5] migration: Move the yank unregister of channel_close out
Date: Wed, 21 Jul 2021 15:34:09 -0400 [thread overview]
Message-ID: <20210721193409.910462-6-peterx@redhat.com> (raw)
In-Reply-To: <20210721193409.910462-1-peterx@redhat.com>
It's efficient, but hackish to call yank unregister calls in channel_close(),
especially it'll be hard to debug when qemu crashed with some yank function
leaked.
Remove that hack, but instead explicitly unregister yank functions at the
places where needed, they are:
(on src)
- migrate_fd_cleanup
- postcopy_pause
(on dst)
- migration_incoming_state_destroy
- postcopy_pause_incoming
Signed-off-by: Peter Xu <peterx@redhat.com>
---
migration/migration.c | 5 +++++
migration/qemu-file-channel.c | 3 ---
migration/savevm.c | 7 +++++++
migration/yank_functions.c | 14 ++++++++++++++
migration/yank_functions.h | 1 +
5 files changed, 27 insertions(+), 3 deletions(-)
diff --git a/migration/migration.c b/migration/migration.c
index fa70400f98..bfeb65b8f7 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -59,6 +59,7 @@
#include "multifd.h"
#include "qemu/yank.h"
#include "sysemu/cpus.h"
+#include "yank_functions.h"
#define MAX_THROTTLE (128 << 20) /* Migration transfer speed throttling */
@@ -273,6 +274,7 @@ void migration_incoming_state_destroy(void)
}
if (mis->from_src_file) {
+ migration_ioc_unregister_yank_from_file(mis->from_src_file);
qemu_fclose(mis->from_src_file);
mis->from_src_file = NULL;
}
@@ -1811,6 +1813,7 @@ static void migrate_fd_cleanup(MigrationState *s)
* Close the file handle without the lock to make sure the
* critical section won't block for long.
*/
+ migration_ioc_unregister_yank_from_file(tmp);
qemu_fclose(tmp);
}
@@ -3352,6 +3355,8 @@ static MigThrError postcopy_pause(MigrationState *s)
/* Current channel is possibly broken. Release it. */
assert(s->to_dst_file);
+ /* Unregister yank for current channel */
+ migration_ioc_unregister_yank_from_file(s->to_dst_file);
qemu_mutex_lock(&s->qemu_file_lock);
file = s->to_dst_file;
s->to_dst_file = NULL;
diff --git a/migration/qemu-file-channel.c b/migration/qemu-file-channel.c
index 2f8b1fcd46..bb5a5752df 100644
--- a/migration/qemu-file-channel.c
+++ b/migration/qemu-file-channel.c
@@ -107,9 +107,6 @@ static int channel_close(void *opaque, Error **errp)
int ret;
QIOChannel *ioc = QIO_CHANNEL(opaque);
ret = qio_channel_close(ioc, errp);
- if (OBJECT(ioc)->ref == 1) {
- migration_ioc_unregister_yank(ioc);
- }
object_unref(OBJECT(ioc));
return ret;
}
diff --git a/migration/savevm.c b/migration/savevm.c
index 96b5e5d639..7b7b64bd13 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -65,6 +65,7 @@
#include "qemu/bitmap.h"
#include "net/announce.h"
#include "qemu/yank.h"
+#include "yank_functions.h"
const unsigned int postcopy_ram_discard_version;
@@ -2568,6 +2569,12 @@ static bool postcopy_pause_incoming(MigrationIncomingState *mis)
/* Clear the triggered bit to allow one recovery */
mis->postcopy_recover_triggered = false;
+ /*
+ * Unregister yank with either from/to src would work, since ioc behind it
+ * is the same
+ */
+ migration_ioc_unregister_yank_from_file(mis->from_src_file);
+
assert(mis->from_src_file);
qemu_file_shutdown(mis->from_src_file);
qemu_fclose(mis->from_src_file);
diff --git a/migration/yank_functions.c b/migration/yank_functions.c
index 23697173ae..8c08aef14a 100644
--- a/migration/yank_functions.c
+++ b/migration/yank_functions.c
@@ -14,6 +14,7 @@
#include "qemu/yank.h"
#include "io/channel-socket.h"
#include "io/channel-tls.h"
+#include "qemu-file.h"
void migration_yank_iochannel(void *opaque)
{
@@ -46,3 +47,16 @@ void migration_ioc_unregister_yank(QIOChannel *ioc)
QIO_CHANNEL(ioc));
}
}
+
+void migration_ioc_unregister_yank_from_file(QEMUFile *file)
+{
+ QIOChannel *ioc = qemu_file_get_ioc(file);
+
+ if (ioc) {
+ /*
+ * For migration qemufiles, we'll always reach here. Though we'll skip
+ * calls from e.g. savevm/loadvm as they don't use yank.
+ */
+ migration_ioc_unregister_yank(ioc);
+ }
+}
diff --git a/migration/yank_functions.h b/migration/yank_functions.h
index 74c7f18c91..a7577955ed 100644
--- a/migration/yank_functions.h
+++ b/migration/yank_functions.h
@@ -17,3 +17,4 @@
void migration_yank_iochannel(void *opaque);
void migration_ioc_register_yank(QIOChannel *ioc);
void migration_ioc_unregister_yank(QIOChannel *ioc);
+void migration_ioc_unregister_yank_from_file(QEMUFile *file);
--
2.31.1
next prev parent reply other threads:[~2021-07-21 19:41 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-07-21 19:34 [PATCH v2 0/5] migrations: Fix potential rare race of migration-test after yank Peter Xu
2021-07-21 19:34 ` [PATCH v2 1/5] migration: Fix missing join() of rp_thread Peter Xu
2021-07-21 19:34 ` [PATCH v2 2/5] migration: Make from_dst_file accesses thread-safe Peter Xu
2021-07-21 21:15 ` Eric Blake
2021-07-22 14:44 ` Peter Xu
2021-07-22 15:19 ` Dr. David Alan Gilbert
2021-07-21 19:34 ` [PATCH v2 3/5] migration: Introduce migration_ioc_[un]register_yank() Peter Xu
2021-07-21 19:34 ` [PATCH v2 4/5] migration: Teach QEMUFile to be QIOChannel-aware Peter Xu
2021-07-22 15:21 ` Dr. David Alan Gilbert
2021-07-21 19:34 ` Peter Xu [this message]
2021-07-22 15:27 ` [PATCH v2 5/5] migration: Move the yank unregister of channel_close out Dr. David Alan Gilbert
2021-07-22 16:19 ` Peter Xu
2021-07-22 17:09 ` Dr. David Alan Gilbert
2021-07-22 17:35 ` Peter Xu
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=20210721193409.910462-6-peterx@redhat.com \
--to=peterx@redhat.com \
--cc=berrange@redhat.com \
--cc=dgilbert@redhat.com \
--cc=lsoaresp@redhat.com \
--cc=lukasstraub2@web.de \
--cc=peter.maydell@linaro.org \
--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).