qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: "Daniel P. Berrangé" <berrange@redhat.com>
To: qemu-devel@nongnu.org
Cc: "Marc-André Lureau" <marcandre.lureau@redhat.com>,
	"Jagannathan Raman" <jag.raman@oracle.com>,
	"Zhao Liu" <zhao1.liu@intel.com>,
	"Eric Blake" <eblake@redhat.com>,
	"Stefan Hajnoczi" <stefanha@redhat.com>,
	"Michael S. Tsirkin" <mst@redhat.com>,
	"Hanna Reitz" <hreitz@redhat.com>,
	"Gustavo Romero" <gustavo.romero@linaro.org>,
	"Thanos Makatos" <thanos.makatos@nutanix.com>,
	"Cédric Le Goater" <clg@redhat.com>,
	"Darren Kenny" <darren.kenny@oracle.com>,
	"Stefano Garzarella" <sgarzare@redhat.com>,
	"Kevin Wolf" <kwolf@redhat.com>,
	"Fabiano Rosas" <farosas@suse.de>,
	qemu-block@nongnu.org, "Peter Xu" <peterx@redhat.com>,
	"Laurent Vivier" <lvivier@redhat.com>,
	"Jason Wang" <jasowang@redhat.com>,
	"Elena Ufimtseva" <elena.ufimtseva@oracle.com>,
	"John Levon" <john.levon@nutanix.com>,
	"Fam Zheng" <fam@euphon.net>, "Alexander Bulekov" <alxndr@bu.edu>,
	"Stefan Weil" <sw@weilnetz.de>,
	"Gerd Hoffmann" <kraxel@redhat.com>,
	"Coiby Xu" <Coiby.Xu@gmail.com>,
	"Daniel P. Berrangé" <berrange@redhat.com>,
	"Qiuhao Li" <Qiuhao.Li@outlook.com>,
	"Michael Roth" <michael.roth@amd.com>,
	"Vladimir Sementsov-Ogievskiy" <vsementsov@yandex-team.ru>,
	"Paolo Bonzini" <pbonzini@redhat.com>,
	"Bandan Das" <bsd@redhat.com>,
	"Kostiantyn Kostiuk" <kkostiuk@redhat.com>,
	"Hailiang Zhang" <zhanghailiang@xfusion.com>
Subject: [PULL 08/16] migration: qemu_file_set_blocking(): add errp parameter
Date: Fri, 19 Sep 2025 12:50:09 +0100	[thread overview]
Message-ID: <20250919115017.1536203-9-berrange@redhat.com> (raw)
In-Reply-To: <20250919115017.1536203-1-berrange@redhat.com>

From: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>

qemu_file_set_blocking() is a wrapper on qio_channel_set_blocking(),
so let's passthrough the errp.

Note the migration should not be using &error_abort in these calls,
however, this is done to expedite the API conversion.

The original code would have eventually ended up calling either
qemu_socket_set_nonblock which would asset on Linux, or
g_unix_set_fd_nonblocking which would propagate errors. We never
saw asserts in practice, and conceptually they should not happen,
but ideally this code will be later adapted to remove use of
&error_abort.

Acked-by: Peter Xu <peterx@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 migration/colo.c         | 5 ++++-
 migration/migration.c    | 8 +++++---
 migration/postcopy-ram.c | 2 +-
 migration/qemu-file.c    | 4 ++--
 migration/qemu-file.h    | 2 +-
 migration/savevm.c       | 4 ++--
 6 files changed, 15 insertions(+), 10 deletions(-)

diff --git a/migration/colo.c b/migration/colo.c
index e0f713c837..cf4d71d9ed 100644
--- a/migration/colo.c
+++ b/migration/colo.c
@@ -859,7 +859,10 @@ static void *colo_process_incoming_thread(void *opaque)
      * coroutine, and here we are in the COLO incoming thread, so it is ok to
      * set the fd back to blocked.
      */
-    qemu_file_set_blocking(mis->from_src_file, true);
+    if (!qemu_file_set_blocking(mis->from_src_file, true, &local_err)) {
+        error_report_err(local_err);
+        goto out;
+    }
 
     colo_incoming_start_dirty_log();
 
diff --git a/migration/migration.c b/migration/migration.c
index 10c216d25d..e1ac4d73c2 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -951,7 +951,7 @@ static void migration_incoming_setup(QEMUFile *f)
 
     assert(!mis->from_src_file);
     mis->from_src_file = f;
-    qemu_file_set_blocking(f, false);
+    qemu_file_set_blocking(f, false, &error_abort);
 }
 
 void migration_incoming_process(void)
@@ -971,7 +971,7 @@ static bool postcopy_try_recover(void)
         /* This should be set already in migration_incoming_setup() */
         assert(mis->from_src_file);
         /* Postcopy has standalone thread to do vm load */
-        qemu_file_set_blocking(mis->from_src_file, true);
+        qemu_file_set_blocking(mis->from_src_file, true, &error_abort);
 
         /* Re-configure the return path */
         mis->to_src_file = qemu_file_get_return_path(mis->from_src_file);
@@ -4002,7 +4002,9 @@ void migration_connect(MigrationState *s, Error *error_in)
     }
 
     migration_rate_set(rate_limit);
-    qemu_file_set_blocking(s->to_dst_file, true);
+    if (!qemu_file_set_blocking(s->to_dst_file, true, &local_err)) {
+        goto fail;
+    }
 
     /*
      * Open the return path. For postcopy, it is used exclusively. For
diff --git a/migration/postcopy-ram.c b/migration/postcopy-ram.c
index 45af9a361e..0172172343 100644
--- a/migration/postcopy-ram.c
+++ b/migration/postcopy-ram.c
@@ -1909,7 +1909,7 @@ void postcopy_preempt_new_channel(MigrationIncomingState *mis, QEMUFile *file)
      * The new loading channel has its own threads, so it needs to be
      * blocked too.  It's by default true, just be explicit.
      */
-    qemu_file_set_blocking(file, true);
+    qemu_file_set_blocking(file, true, &error_abort);
     mis->postcopy_qemufile_dst = file;
     qemu_sem_post(&mis->postcopy_qemufile_dst_done);
     trace_postcopy_preempt_new_channel();
diff --git a/migration/qemu-file.c b/migration/qemu-file.c
index d5c6e7ec61..0f4280df21 100644
--- a/migration/qemu-file.c
+++ b/migration/qemu-file.c
@@ -888,9 +888,9 @@ void qemu_put_counted_string(QEMUFile *f, const char *str)
  *       both directions, and thus changing the blocking on the main
  *       QEMUFile can also affect the return path.
  */
-void qemu_file_set_blocking(QEMUFile *f, bool block)
+bool qemu_file_set_blocking(QEMUFile *f, bool block, Error **errp)
 {
-    qio_channel_set_blocking(f->ioc, block, NULL);
+    return qio_channel_set_blocking(f->ioc, block, errp);
 }
 
 /*
diff --git a/migration/qemu-file.h b/migration/qemu-file.h
index f5b9f430e0..c13c967167 100644
--- a/migration/qemu-file.h
+++ b/migration/qemu-file.h
@@ -71,7 +71,7 @@ void qemu_file_set_error(QEMUFile *f, int ret);
 int qemu_file_shutdown(QEMUFile *f);
 QEMUFile *qemu_file_get_return_path(QEMUFile *f);
 int qemu_fflush(QEMUFile *f);
-void qemu_file_set_blocking(QEMUFile *f, bool block);
+bool qemu_file_set_blocking(QEMUFile *f, bool block, Error **errp);
 int qemu_file_get_to_fd(QEMUFile *f, int fd, size_t size);
 void qemu_set_offset(QEMUFile *f, off_t off, int whence);
 off_t qemu_get_offset(QEMUFile *f);
diff --git a/migration/savevm.c b/migration/savevm.c
index fabbeb296a..abe0547f9b 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -2095,7 +2095,7 @@ static void *postcopy_ram_listen_thread(void *opaque)
      * Because we're a thread and not a coroutine we can't yield
      * in qemu_file, and thus we must be blocking now.
      */
-    qemu_file_set_blocking(f, true);
+    qemu_file_set_blocking(f, true, &error_fatal);
 
     /* TODO: sanity check that only postcopiable data will be loaded here */
     load_res = qemu_loadvm_state_main(f, mis);
@@ -2108,7 +2108,7 @@ static void *postcopy_ram_listen_thread(void *opaque)
     f = mis->from_src_file;
 
     /* And non-blocking again so we don't block in any cleanup */
-    qemu_file_set_blocking(f, false);
+    qemu_file_set_blocking(f, false, &error_fatal);
 
     trace_postcopy_ram_listen_thread_exit();
     if (load_res < 0) {
-- 
2.50.1



  parent reply	other threads:[~2025-09-19 11:55 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-09-19 11:50 [PULL 00/16] Treewide I/O handle cleanup Daniel P. Berrangé
2025-09-19 11:50 ` [PULL 01/16] MAINTAINERS: list qemu-security@nongnu.org as security contact Daniel P. Berrangé
2025-09-19 11:50 ` [PULL 02/16] migration/qemu-file: don't make incoming fds blocking again Daniel P. Berrangé
2025-09-19 11:50 ` [PULL 03/16] io/channel: document how qio_channel_readv_full() handles fds Daniel P. Berrangé
2025-09-19 11:50 ` [PULL 04/16] char-socket: tcp_chr_recv(): drop extra _set_(block, cloexec) Daniel P. Berrangé via
2025-09-19 11:50 ` [PULL 05/16] char-socket: tcp_chr_recv(): add comment Daniel P. Berrangé
2025-09-19 11:50 ` [PULL 06/16] util: add qemu_set_blocking() function Daniel P. Berrangé
2025-09-19 11:50 ` [PULL 07/16] treewide: handle result of qio_channel_set_blocking() Daniel P. Berrangé
2025-09-19 11:50 ` Daniel P. Berrangé [this message]
2025-09-19 11:50 ` [PULL 09/16] util: drop qemu_socket_set_nonblock() Daniel P. Berrangé
2025-09-19 11:50 ` [PULL 10/16] util: drop qemu_socket_try_set_nonblock() Daniel P. Berrangé
2025-09-19 11:50 ` [PULL 11/16] io/channel-socket: rework qio_channel_socket_copy_fds() Daniel P. Berrangé
2025-09-19 11:50 ` [PULL 12/16] util: drop qemu_socket_set_block() Daniel P. Berrangé
2025-09-19 11:50 ` [PULL 13/16] treewide: use qemu_set_blocking instead of g_unix_set_fd_nonblocking Daniel P. Berrangé
2025-09-22 13:40   ` Peter Maydell
2025-09-22 14:19     ` Vladimir Sementsov-Ogievskiy
2025-09-19 11:50 ` [PULL 14/16] chardev: qemu_chr_open_fd(): add errp Daniel P. Berrangé
2025-09-22 13:45   ` Peter Maydell
2025-09-22 14:27     ` Vladimir Sementsov-Ogievskiy
2025-10-14 12:52       ` Peter Maydell
2025-10-14 13:20         ` Vladimir Sementsov-Ogievskiy
2025-09-22 14:29     ` Daniel P. Berrangé
2025-09-19 11:50 ` [PULL 15/16] chardev: close an fd on failure path Daniel P. Berrangé
2025-09-19 11:50 ` [PULL 16/16] util/vhost-user-server: vu_message_read(): improve error handling Daniel P. Berrangé
2025-09-19 20:47 ` [PULL 00/16] Treewide I/O handle cleanup Richard Henderson

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=20250919115017.1536203-9-berrange@redhat.com \
    --to=berrange@redhat.com \
    --cc=Coiby.Xu@gmail.com \
    --cc=Qiuhao.Li@outlook.com \
    --cc=alxndr@bu.edu \
    --cc=bsd@redhat.com \
    --cc=clg@redhat.com \
    --cc=darren.kenny@oracle.com \
    --cc=eblake@redhat.com \
    --cc=elena.ufimtseva@oracle.com \
    --cc=fam@euphon.net \
    --cc=farosas@suse.de \
    --cc=gustavo.romero@linaro.org \
    --cc=hreitz@redhat.com \
    --cc=jag.raman@oracle.com \
    --cc=jasowang@redhat.com \
    --cc=john.levon@nutanix.com \
    --cc=kkostiuk@redhat.com \
    --cc=kraxel@redhat.com \
    --cc=kwolf@redhat.com \
    --cc=lvivier@redhat.com \
    --cc=marcandre.lureau@redhat.com \
    --cc=michael.roth@amd.com \
    --cc=mst@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=peterx@redhat.com \
    --cc=qemu-block@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    --cc=sgarzare@redhat.com \
    --cc=stefanha@redhat.com \
    --cc=sw@weilnetz.de \
    --cc=thanos.makatos@nutanix.com \
    --cc=vsementsov@yandex-team.ru \
    --cc=zhanghailiang@xfusion.com \
    --cc=zhao1.liu@intel.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).