From 5bb8327b655dbce10a91ef809acb0875dd0ee0ed Mon Sep 17 00:00:00 2001 From: Greg Kurz Date: Fri, 12 Mar 2021 10:22:12 +0100 Subject: [PATCH 1/9] virtiofsd: Release vu_dispatch_lock when stopping queue QEMU can stop a virtqueue by sending a VHOST_USER_GET_VRING_BASE request to virtiofsd. As with all other vhost-user protocol messages, the thread that runs the main event loop in virtiofsd takes the vu_dispatch lock in write mode. This ensures that no other thread can access virtqueues or memory tables at the same time. In the case of VHOST_USER_GET_VRING_BASE, the main thread basically notifies the queue thread that it should terminate and waits for its termination: main() virtio_loop() vu_dispatch_wrlock() vu_dispatch() vu_process_message() vu_get_vring_base_exec() fv_queue_cleanup_thread() pthread_join() Unfortunately, the queue thread ends up calling virtio_send_msg() at some point, which itself needs to grab the lock: fv_queue_thread() g_list_foreach() fv_queue_worker() fuse_session_process_buf_int() do_release() lo_release() fuse_reply_err() send_reply() send_reply_iov() fuse_send_reply_iov_nofree() fuse_send_msg() virtio_send_msg() vu_dispatch_rdlock() <-- Deadlock ! Simply have the main thread to release the lock before going to sleep and take it back afterwards. A very similar patch was already sent by Vivek Goyal sometime back: https://listman.redhat.com/archives/virtio-fs/2021-January/msg00073.html The only difference here is that this done in fv_queue_set_started() because fv_queue_cleanup_thread() can also be called from virtio_loop() without the lock being held. Signed-off-by: Greg Kurz Reviewed-by: Vivek Goyal Reviewed-by: Stefan Hajnoczi Message-Id: <20210312092212.782255-8-groug@kaod.org> Signed-off-by: Dr. David Alan Gilbert --- tools/virtiofsd/fuse_virtio.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/tools/virtiofsd/fuse_virtio.c b/tools/virtiofsd/fuse_virtio.c index 523ee64fb7..3e13997406 100644 --- a/tools/virtiofsd/fuse_virtio.c +++ b/tools/virtiofsd/fuse_virtio.c @@ -792,7 +792,13 @@ static void fv_queue_set_started(VuDev *dev, int qidx, bool started) assert(0); } } else { + /* + * Temporarily drop write-lock taken in virtio_loop() so that + * the queue thread doesn't block in virtio_send_msg(). + */ + vu_dispatch_unlock(vud); fv_queue_cleanup_thread(vud, qidx); + vu_dispatch_wrlock(vud); } } From 6d118c4349966a1890d00bbbdc42001f173c6e4d Mon Sep 17 00:00:00 2001 From: Vivek Goyal Date: Wed, 3 Mar 2021 14:53:39 -0500 Subject: [PATCH 2/9] virtiofsd: Add qemu version and copyright info Option "-V" currently displays the fuse protocol version virtiofsd is using. For example, I see this. $ ./virtiofsd -V "using FUSE kernel interface version 7.33" People also want to know software version of virtiofsd so that they can figure out if a certain fix is part of currently running virtiofsd or not. Eric Ernst ran into this issue. David Gilbert thinks that it probably is best that we simply carry the qemu version and display that information given we are part of qemu tree. So this patch enhances version information and also adds qemu version and copyright info. Not sure if copyright information is supposed to be displayed along with version info. Given qemu-storage-daemon and other utilities are doing it, so I continued with same pattern. This is how now output looks like. $ ./virtiofsd -V virtiofsd version 5.2.50 (v5.2.0-2357-gcbcf09872a-dirty) Copyright (c) 2003-2020 Fabrice Bellard and the QEMU Project developers using FUSE kernel interface version 7.33 Reported-by: Eric Ernst Signed-off-by: Vivek Goyal Message-Id: <20210303195339.GB3793@redhat.com> Reviewed-by: Dr. David Alan Gilbert Reviewed-by: Sergio Lopez Signed-off-by: Dr. David Alan Gilbert --- tools/virtiofsd/passthrough_ll.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/tools/virtiofsd/passthrough_ll.c b/tools/virtiofsd/passthrough_ll.c index fc7e1b1e8e..851c25ef20 100644 --- a/tools/virtiofsd/passthrough_ll.c +++ b/tools/virtiofsd/passthrough_ll.c @@ -37,6 +37,8 @@ #include "qemu/osdep.h" #include "qemu/timer.h" +#include "qemu-version.h" +#include "qemu-common.h" #include "fuse_virtio.h" #include "fuse_log.h" #include "fuse_lowlevel.h" @@ -3666,6 +3668,11 @@ static void fuse_lo_data_cleanup(struct lo_data *lo) free(lo->source); } +static void qemu_version(void) +{ + printf("virtiofsd version " QEMU_FULL_VERSION "\n" QEMU_COPYRIGHT "\n"); +} + int main(int argc, char *argv[]) { struct fuse_args args = FUSE_ARGS_INIT(argc, argv); @@ -3737,6 +3744,7 @@ int main(int argc, char *argv[]) ret = 0; goto err_out1; } else if (opts.show_version) { + qemu_version(); fuse_lowlevel_version(); ret = 0; goto err_out1; From 28d1ad0ea41342472afda15b515d95671eac4030 Mon Sep 17 00:00:00 2001 From: Greg Kurz Date: Fri, 12 Mar 2021 15:10:03 +0100 Subject: [PATCH 3/9] virtiofsd: Don't allow empty filenames POSIX.1-2017 clearly stipulates that empty filenames aren't allowed ([1] and [2]). Since virtiofsd is supposed to mirror the host file system hierarchy and the host can be assumed to be linux, we don't really expect clients to pass requests with an empty path in it. If they do so anyway, this would eventually cause an error when trying to create/lookup the actual inode on the underlying POSIX filesystem. But this could still confuse some code that wouldn't be ready to cope with this. Filter out empty names coming from the client at the top level, so that the rest doesn't have to care about it. This is done everywhere we already call is_safe_path_component(), but in a separate helper since the usual error for empty path names is ENOENT instead of EINVAL. [1] https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap03.html#tag_03_170 [2] https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap04.html#tag_04_13 Signed-off-by: Greg Kurz Message-Id: <20210312141003.819108-4-groug@kaod.org> Reviewed-by: Connor Kuehl Signed-off-by: Dr. David Alan Gilbert --- tools/virtiofsd/passthrough_ll.c | 35 ++++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/tools/virtiofsd/passthrough_ll.c b/tools/virtiofsd/passthrough_ll.c index 851c25ef20..b07101d8eb 100644 --- a/tools/virtiofsd/passthrough_ll.c +++ b/tools/virtiofsd/passthrough_ll.c @@ -239,6 +239,11 @@ static int is_safe_path_component(const char *path) return !is_dot_or_dotdot(path); } +static bool is_empty(const char *name) +{ + return name[0] == '\0'; +} + static struct lo_data *lo_data(fuse_req_t req) { return (struct lo_data *)fuse_req_userdata(req); @@ -1085,6 +1090,11 @@ static void lo_lookup(fuse_req_t req, fuse_ino_t parent, const char *name) fuse_log(FUSE_LOG_DEBUG, "lo_lookup(parent=%" PRIu64 ", name=%s)\n", parent, name); + if (is_empty(name)) { + fuse_reply_err(req, ENOENT); + return; + } + /* * Don't use is_safe_path_component(), allow "." and ".." for NFS export * support. @@ -1176,6 +1186,11 @@ static void lo_mknod_symlink(fuse_req_t req, fuse_ino_t parent, struct fuse_entry_param e; struct lo_cred old = {}; + if (is_empty(name)) { + fuse_reply_err(req, ENOENT); + return; + } + if (!is_safe_path_component(name)) { fuse_reply_err(req, EINVAL); return; @@ -1248,6 +1263,11 @@ static void lo_link(fuse_req_t req, fuse_ino_t ino, fuse_ino_t parent, char procname[64]; int saverr; + if (is_empty(name)) { + fuse_reply_err(req, ENOENT); + return; + } + if (!is_safe_path_component(name)) { fuse_reply_err(req, EINVAL); return; @@ -1326,6 +1346,11 @@ static void lo_rmdir(fuse_req_t req, fuse_ino_t parent, const char *name) struct lo_inode *inode; struct lo_data *lo = lo_data(req); + if (is_empty(name)) { + fuse_reply_err(req, ENOENT); + return; + } + if (!is_safe_path_component(name)) { fuse_reply_err(req, EINVAL); return; @@ -1355,6 +1380,11 @@ static void lo_rename(fuse_req_t req, fuse_ino_t parent, const char *name, struct lo_inode *newinode = NULL; struct lo_data *lo = lo_data(req); + if (is_empty(name) || is_empty(newname)) { + fuse_reply_err(req, ENOENT); + return; + } + if (!is_safe_path_component(name) || !is_safe_path_component(newname)) { fuse_reply_err(req, EINVAL); return; @@ -1408,6 +1438,11 @@ static void lo_unlink(fuse_req_t req, fuse_ino_t parent, const char *name) struct lo_inode *inode; struct lo_data *lo = lo_data(req); + if (is_empty(name)) { + fuse_reply_err(req, ENOENT); + return; + } + if (!is_safe_path_component(name)) { fuse_reply_err(req, EINVAL); return; From 20afcc23b3212784c84fb06062f66d9d2ce6865d Mon Sep 17 00:00:00 2001 From: Greg Kurz Date: Fri, 12 Mar 2021 15:10:01 +0100 Subject: [PATCH 4/9] virtiofsd: Don't allow empty paths in lookup_name() When passed an empty filename, lookup_name() returns the inode of the parent directory, unless the parent is the root in which case the st_dev doesn't match and lo_find() returns NULL. This is because lookup_name() passes AT_EMPTY_PATH down to fstatat() or statx(). This behavior doesn't quite make sense because users of lookup_name() then pass the name to unlinkat(), renameat() or renameat2(), all of which will always fail on empty names. Drop AT_EMPTY_PATH from the flags in lookup_name() so that it has the consistent behavior of "returning an existing child inode or NULL" for all directories. Signed-off-by: Greg Kurz Message-Id: <20210312141003.819108-2-groug@kaod.org> Reviewed-by: Connor Kuehl Reviewed-by: Vivek Goyal Signed-off-by: Dr. David Alan Gilbert --- tools/virtiofsd/passthrough_ll.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tools/virtiofsd/passthrough_ll.c b/tools/virtiofsd/passthrough_ll.c index b07101d8eb..cf453eea9b 100644 --- a/tools/virtiofsd/passthrough_ll.c +++ b/tools/virtiofsd/passthrough_ll.c @@ -1330,8 +1330,7 @@ static struct lo_inode *lookup_name(fuse_req_t req, fuse_ino_t parent, return NULL; } - res = do_statx(lo, dir->fd, name, &attr, - AT_EMPTY_PATH | AT_SYMLINK_NOFOLLOW, &mnt_id); + res = do_statx(lo, dir->fd, name, &attr, AT_SYMLINK_NOFOLLOW, &mnt_id); lo_inode_put(lo, &dir); if (res == -1) { return NULL; From 03ccaaae48fe1bd3ee0842717008fe74d7745680 Mon Sep 17 00:00:00 2001 From: Greg Kurz Date: Fri, 12 Mar 2021 15:10:02 +0100 Subject: [PATCH 5/9] virtiofsd: Convert some functions to return bool Both currently only return 0 or 1. Signed-off-by: Greg Kurz Message-Id: <20210312141003.819108-3-groug@kaod.org> Reviewed-by: Connor Kuehl Reviewed-by: Vivek Goyal Signed-off-by: Dr. David Alan Gilbert --- tools/virtiofsd/passthrough_ll.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tools/virtiofsd/passthrough_ll.c b/tools/virtiofsd/passthrough_ll.c index cf453eea9b..b144320e48 100644 --- a/tools/virtiofsd/passthrough_ll.c +++ b/tools/virtiofsd/passthrough_ll.c @@ -223,17 +223,17 @@ static struct lo_inode *lo_find(struct lo_data *lo, struct stat *st, static int xattr_map_client(const struct lo_data *lo, const char *client_name, char **out_name); -static int is_dot_or_dotdot(const char *name) +static bool is_dot_or_dotdot(const char *name) { return name[0] == '.' && (name[1] == '\0' || (name[1] == '.' && name[2] == '\0')); } /* Is `path` a single path component that is not "." or ".."? */ -static int is_safe_path_component(const char *path) +static bool is_safe_path_component(const char *path) { if (strchr(path, '/')) { - return 0; + return false; } return !is_dot_or_dotdot(path); From a339149afa50578380bf8a7c1ed5ae7061431db4 Mon Sep 17 00:00:00 2001 From: Hao Wang Date: Tue, 9 Feb 2021 18:42:36 +0800 Subject: [PATCH 6/9] migration/tls: fix inverted semantics in multifd_channel_connect MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Function multifd_channel_connect() return "true" to indicate failure, which is rather confusing. Fix that. Signed-off-by: Hao Wang Message-Id: <20210209104237.2250941-2-wanghao232@huawei.com> Reviewed-by: Daniel P. Berrangé Reviewed-by: Chuan Zheng Signed-off-by: Dr. David Alan Gilbert --- migration/multifd.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/migration/multifd.c b/migration/multifd.c index 1a1e589064..2a1ea85ade 100644 --- a/migration/multifd.c +++ b/migration/multifd.c @@ -798,9 +798,9 @@ static bool multifd_channel_connect(MultiFDSendParams *p, * function after the TLS handshake, * so we mustn't call multifd_send_thread until then */ - return false; - } else { return true; + } else { + return false; } } else { /* update for tls qio channel */ @@ -808,10 +808,10 @@ static bool multifd_channel_connect(MultiFDSendParams *p, qemu_thread_create(&p->thread, p->name, multifd_send_thread, p, QEMU_THREAD_JOINABLE); } - return false; + return true; } - return true; + return false; } static void multifd_new_send_channel_cleanup(MultiFDSendParams *p, @@ -844,7 +844,7 @@ static void multifd_new_send_channel_async(QIOTask *task, gpointer opaque) p->c = QIO_CHANNEL(sioc); qio_channel_set_delay(p->c, false); p->running = true; - if (multifd_channel_connect(p, sioc, local_err)) { + if (!multifd_channel_connect(p, sioc, local_err)) { goto cleanup; } return; From fca676429ca7f309b5d492c7675d35fec484197c Mon Sep 17 00:00:00 2001 From: Hao Wang Date: Tue, 9 Feb 2021 18:42:37 +0800 Subject: [PATCH 7/9] migration/tls: add error handling in multifd_tls_handshake_thread MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit If any error happens during multifd send thread creating (e.g. channel broke because new domain is destroyed by the dst), multifd_tls_handshake_thread may exit silently, leaving main migration thread hanging (ram_save_setup -> multifd_send_sync_main -> qemu_sem_wait(&p->sem_sync)). Fix that by adding error handling in multifd_tls_handshake_thread. Signed-off-by: Hao Wang Message-Id: <20210209104237.2250941-3-wanghao232@huawei.com> Reviewed-by: Daniel P. Berrangé Reviewed-by: Chuan Zheng Signed-off-by: Dr. David Alan Gilbert --- migration/multifd.c | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/migration/multifd.c b/migration/multifd.c index 2a1ea85ade..03527c564c 100644 --- a/migration/multifd.c +++ b/migration/multifd.c @@ -739,7 +739,16 @@ static void multifd_tls_outgoing_handshake(QIOTask *task, } else { trace_multifd_tls_outgoing_handshake_complete(ioc); } - multifd_channel_connect(p, ioc, err); + + if (!multifd_channel_connect(p, ioc, err)) { + /* + * Error happen, mark multifd_send_thread status as 'quit' although it + * is not created, and then tell who pay attention to me. + */ + p->quit = true; + qemu_sem_post(&multifd_send_state->channels_ready); + qemu_sem_post(&p->sem_sync); + } } static void *multifd_tls_handshake_thread(void *opaque) From a8e2ab5db2181b68f371ee794e1a0fe7ca6f5e24 Mon Sep 17 00:00:00 2001 From: Mahmoud Mandour Date: Thu, 11 Mar 2021 05:15:34 +0200 Subject: [PATCH 8/9] monitor: Replaced qemu_mutex_lock calls with QEMU_LOCK_GUARD Removed various qemu_mutex_lock and their respective qemu_mutex_unlock calls and used lock guard macros (QEMU_LOCK_GUARD and WITH_QEMU_LOCK_GUARD). This simplifies the code by eliminating qemu_mutex_unlock calls. Signed-off-by: Mahmoud Mandour Message-Id: <20210311031538.5325-6-ma.mandourr@gmail.com> Reviewed-by: Dr. David Alan Gilbert Signed-off-by: Dr. David Alan Gilbert --- monitor/monitor.c | 8 ++------ monitor/qmp.c | 51 ++++++++++++++++++++++------------------------- 2 files changed, 26 insertions(+), 33 deletions(-) diff --git a/monitor/monitor.c b/monitor/monitor.c index e94f532cf5..640496e562 100644 --- a/monitor/monitor.c +++ b/monitor/monitor.c @@ -349,7 +349,7 @@ monitor_qapi_event_queue_no_reenter(QAPIEvent event, QDict *qdict) evconf = &monitor_qapi_event_conf[event]; trace_monitor_protocol_event_queue(event, qdict, evconf->rate); - qemu_mutex_lock(&monitor_lock); + QEMU_LOCK_GUARD(&monitor_lock); if (!evconf->rate) { /* Unthrottled event */ @@ -391,8 +391,6 @@ monitor_qapi_event_queue_no_reenter(QAPIEvent event, QDict *qdict) timer_mod_ns(evstate->timer, now + evconf->rate); } } - - qemu_mutex_unlock(&monitor_lock); } void qapi_event_emit(QAPIEvent event, QDict *qdict) @@ -447,7 +445,7 @@ static void monitor_qapi_event_handler(void *opaque) MonitorQAPIEventConf *evconf = &monitor_qapi_event_conf[evstate->event]; trace_monitor_protocol_event_handler(evstate->event, evstate->qdict); - qemu_mutex_lock(&monitor_lock); + QEMU_LOCK_GUARD(&monitor_lock); if (evstate->qdict) { int64_t now = qemu_clock_get_ns(monitor_get_event_clock()); @@ -462,8 +460,6 @@ static void monitor_qapi_event_handler(void *opaque) timer_free(evstate->timer); g_free(evstate); } - - qemu_mutex_unlock(&monitor_lock); } static unsigned int qapi_event_throttle_hash(const void *key) diff --git a/monitor/qmp.c b/monitor/qmp.c index 2326bd7f9b..2b0308f933 100644 --- a/monitor/qmp.c +++ b/monitor/qmp.c @@ -76,7 +76,7 @@ static void monitor_qmp_cleanup_req_queue_locked(MonitorQMP *mon) static void monitor_qmp_cleanup_queue_and_resume(MonitorQMP *mon) { - qemu_mutex_lock(&mon->qmp_queue_lock); + QEMU_LOCK_GUARD(&mon->qmp_queue_lock); /* * Same condition as in monitor_qmp_dispatcher_co(), but before @@ -103,7 +103,6 @@ static void monitor_qmp_cleanup_queue_and_resume(MonitorQMP *mon) monitor_resume(&mon->common); } - qemu_mutex_unlock(&mon->qmp_queue_lock); } void qmp_send_response(MonitorQMP *mon, const QDict *rsp) @@ -179,7 +178,7 @@ static QMPRequest *monitor_qmp_requests_pop_any_with_lock(void) Monitor *mon; MonitorQMP *qmp_mon; - qemu_mutex_lock(&monitor_lock); + QEMU_LOCK_GUARD(&monitor_lock); QTAILQ_FOREACH(mon, &mon_list, entry) { if (!monitor_is_qmp(mon)) { @@ -205,8 +204,6 @@ static QMPRequest *monitor_qmp_requests_pop_any_with_lock(void) QTAILQ_INSERT_TAIL(&mon_list, mon, entry); } - qemu_mutex_unlock(&monitor_lock); - return req_obj; } @@ -376,31 +373,31 @@ static void handle_qmp_command(void *opaque, QObject *req, Error *err) req_obj->err = err; /* Protect qmp_requests and fetching its length. */ - qemu_mutex_lock(&mon->qmp_queue_lock); + WITH_QEMU_LOCK_GUARD(&mon->qmp_queue_lock) { - /* - * Suspend the monitor when we can't queue more requests after - * this one. Dequeuing in monitor_qmp_dispatcher_co() or - * monitor_qmp_cleanup_queue_and_resume() will resume it. - * Note that when OOB is disabled, we queue at most one command, - * for backward compatibility. - */ - if (!qmp_oob_enabled(mon) || - mon->qmp_requests->length == QMP_REQ_QUEUE_LEN_MAX - 1) { - monitor_suspend(&mon->common); + /* + * Suspend the monitor when we can't queue more requests after + * this one. Dequeuing in monitor_qmp_dispatcher_co() or + * monitor_qmp_cleanup_queue_and_resume() will resume it. + * Note that when OOB is disabled, we queue at most one command, + * for backward compatibility. + */ + if (!qmp_oob_enabled(mon) || + mon->qmp_requests->length == QMP_REQ_QUEUE_LEN_MAX - 1) { + monitor_suspend(&mon->common); + } + + /* + * Put the request to the end of queue so that requests will be + * handled in time order. Ownership for req_obj, req, + * etc. will be delivered to the handler side. + */ + trace_monitor_qmp_in_band_enqueue(req_obj, mon, + mon->qmp_requests->length); + assert(mon->qmp_requests->length < QMP_REQ_QUEUE_LEN_MAX); + g_queue_push_tail(mon->qmp_requests, req_obj); } - /* - * Put the request to the end of queue so that requests will be - * handled in time order. Ownership for req_obj, req, - * etc. will be delivered to the handler side. - */ - trace_monitor_qmp_in_band_enqueue(req_obj, mon, - mon->qmp_requests->length); - assert(mon->qmp_requests->length < QMP_REQ_QUEUE_LEN_MAX); - g_queue_push_tail(mon->qmp_requests, req_obj); - qemu_mutex_unlock(&mon->qmp_queue_lock); - /* Kick the dispatcher routine */ if (!qatomic_xchg(&qmp_dispatcher_co_busy, true)) { aio_co_wake(qmp_dispatcher_co); From 373969507a3dc7de2d291da7e1bd03acf46ec643 Mon Sep 17 00:00:00 2001 From: Mahmoud Mandour Date: Thu, 11 Mar 2021 05:15:35 +0200 Subject: [PATCH 9/9] migration: Replaced qemu_mutex_lock calls with QEMU_LOCK_GUARD Replaced various qemu_mutex_lock calls and their respective qemu_mutex_unlock calls with QEMU_LOCK_GUARD macro. This simplifies the code by eliminating the respective qemu_mutex_unlock calls. Signed-off-by: Mahmoud Mandour Message-Id: <20210311031538.5325-7-ma.mandourr@gmail.com> Reviewed-by: Dr. David Alan Gilbert Signed-off-by: Dr. David Alan Gilbert --- migration/migration.c | 6 ++---- migration/ram.c | 6 ++---- 2 files changed, 4 insertions(+), 8 deletions(-) diff --git a/migration/migration.c b/migration/migration.c index a5ddf43559..36768391b6 100644 --- a/migration/migration.c +++ b/migration/migration.c @@ -323,7 +323,7 @@ static int migrate_send_rp_message(MigrationIncomingState *mis, int ret = 0; trace_migrate_send_rp_message((int)message_type, len); - qemu_mutex_lock(&mis->rp_mutex); + QEMU_LOCK_GUARD(&mis->rp_mutex); /* * It's possible that the file handle got lost due to network @@ -331,7 +331,7 @@ static int migrate_send_rp_message(MigrationIncomingState *mis, */ if (!mis->to_src_file) { ret = -EIO; - goto error; + return ret; } qemu_put_be16(mis->to_src_file, (unsigned int)message_type); @@ -342,8 +342,6 @@ static int migrate_send_rp_message(MigrationIncomingState *mis, /* It's possible that qemu file got error during sending */ ret = qemu_file_get_error(mis->to_src_file); -error: - qemu_mutex_unlock(&mis->rp_mutex); return ret; } diff --git a/migration/ram.c b/migration/ram.c index 72143da0ac..52537f14ac 100644 --- a/migration/ram.c +++ b/migration/ram.c @@ -819,7 +819,7 @@ static inline bool migration_bitmap_clear_dirty(RAMState *rs, { bool ret; - qemu_mutex_lock(&rs->bitmap_mutex); + QEMU_LOCK_GUARD(&rs->bitmap_mutex); /* * Clear dirty bitmap if needed. This _must_ be called before we @@ -852,7 +852,6 @@ static inline bool migration_bitmap_clear_dirty(RAMState *rs, if (ret) { rs->migration_dirty_pages--; } - qemu_mutex_unlock(&rs->bitmap_mutex); return ret; } @@ -3275,7 +3274,7 @@ static void decompress_data_with_multi_threads(QEMUFile *f, int idx, thread_count; thread_count = migrate_decompress_threads(); - qemu_mutex_lock(&decomp_done_lock); + QEMU_LOCK_GUARD(&decomp_done_lock); while (true) { for (idx = 0; idx < thread_count; idx++) { if (decomp_param[idx].done) { @@ -3295,7 +3294,6 @@ static void decompress_data_with_multi_threads(QEMUFile *f, qemu_cond_wait(&decomp_done_cond, &decomp_done_lock); } } - qemu_mutex_unlock(&decomp_done_lock); } /*