* [PATCH 01/20] io: add a QIOChannelNull equivalent to /dev/null
2022-05-24 11:02 [PATCH 00/20] migration: remove QEMUFileOps concept and assume use of QIOChannel Daniel P. Berrangé
@ 2022-05-24 11:02 ` Daniel P. Berrangé
2022-05-24 21:14 ` Eric Blake
2022-05-24 11:02 ` [PATCH 02/20] migration: switch to use QIOChannelNull for dummy channel Daniel P. Berrangé
` (18 subsequent siblings)
19 siblings, 1 reply; 51+ messages in thread
From: Daniel P. Berrangé @ 2022-05-24 11:02 UTC (permalink / raw)
To: qemu-devel
Cc: Daniel P. Berrangé,
Dr. David Alan Gilbert, qemu-block, Stefan Hajnoczi,
Hailiang Zhang, Juan Quintela, Fam Zheng
This is for code which needs a portable equivalent to a QIOChannelFile
connected to /dev/null.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
include/io/channel-null.h | 55 +++++++
io/channel-null.c | 237 ++++++++++++++++++++++++++++++
io/meson.build | 1 +
io/trace-events | 3 +
tests/unit/meson.build | 1 +
tests/unit/test-io-channel-null.c | 95 ++++++++++++
6 files changed, 392 insertions(+)
create mode 100644 include/io/channel-null.h
create mode 100644 io/channel-null.c
create mode 100644 tests/unit/test-io-channel-null.c
diff --git a/include/io/channel-null.h b/include/io/channel-null.h
new file mode 100644
index 0000000000..d7c950e494
--- /dev/null
+++ b/include/io/channel-null.h
@@ -0,0 +1,55 @@
+/*
+ * QEMU I/O channels null driver
+ *
+ * Copyright (c) 2022 Red Hat, Inc.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+#ifndef QIO_CHANNEL_FILE_H
+#define QIO_CHANNEL_FILE_H
+
+#include "io/channel.h"
+#include "qom/object.h"
+
+#define TYPE_QIO_CHANNEL_NULL "qio-channel-null"
+OBJECT_DECLARE_SIMPLE_TYPE(QIOChannelNull, QIO_CHANNEL_NULL)
+
+
+/**
+ * QIOChannelNull:
+ *
+ * The QIOChannelNull object provides a channel implementation
+ * that discards all writes and returns zero bytes for all reads.
+ */
+
+struct QIOChannelNull {
+ QIOChannel parent;
+ bool closed;
+};
+
+
+/**
+ * qio_channel_null_new:
+ *
+ * Create a new IO channel object that discards all writes
+ * and returns zero bytes for all reads.
+ *
+ * Returns: the new channel object
+ */
+QIOChannelNull *
+qio_channel_null_new(void);
+
+#endif /* QIO_CHANNEL_NULL_H */
diff --git a/io/channel-null.c b/io/channel-null.c
new file mode 100644
index 0000000000..75e3781507
--- /dev/null
+++ b/io/channel-null.c
@@ -0,0 +1,237 @@
+/*
+ * QEMU I/O channels null driver
+ *
+ * Copyright (c) 2022 Red Hat, Inc.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+#include "qemu/osdep.h"
+#include "io/channel-null.h"
+#include "io/channel-watch.h"
+#include "qapi/error.h"
+#include "trace.h"
+#include "qemu/iov.h"
+
+typedef struct QIOChannelNullSource QIOChannelNullSource;
+struct QIOChannelNullSource {
+ GSource parent;
+ QIOChannel *ioc;
+ GIOCondition condition;
+};
+
+
+QIOChannelNull *
+qio_channel_null_new(void)
+{
+ QIOChannelNull *ioc;
+
+ ioc = QIO_CHANNEL_NULL(object_new(TYPE_QIO_CHANNEL_NULL));
+
+ trace_qio_channel_null_new(ioc);
+
+ return ioc;
+}
+
+
+static void
+qio_channel_null_init(Object *obj)
+{
+ QIOChannelNull *ioc = QIO_CHANNEL_NULL(obj);
+ ioc->closed = false;
+}
+
+
+static ssize_t
+qio_channel_null_readv(QIOChannel *ioc,
+ const struct iovec *iov,
+ size_t niov,
+ int **fds G_GNUC_UNUSED,
+ size_t *nfds G_GNUC_UNUSED,
+ Error **errp)
+{
+ QIOChannelNull *nioc = QIO_CHANNEL_NULL(ioc);
+
+ if (nioc->closed) {
+ error_setg_errno(errp, EINVAL,
+ "Channel is closed");
+ return -1;
+ }
+
+ return 0;
+}
+
+
+static ssize_t
+qio_channel_null_writev(QIOChannel *ioc,
+ const struct iovec *iov,
+ size_t niov,
+ int *fds G_GNUC_UNUSED,
+ size_t nfds G_GNUC_UNUSED,
+ int flags G_GNUC_UNUSED,
+ Error **errp)
+{
+ QIOChannelNull *nioc = QIO_CHANNEL_NULL(ioc);
+
+ if (nioc->closed) {
+ error_setg_errno(errp, EINVAL,
+ "Channel is closed");
+ return -1;
+ }
+
+ return iov_size(iov, niov);
+}
+
+
+static int
+qio_channel_null_set_blocking(QIOChannel *ioc G_GNUC_UNUSED,
+ bool enabled G_GNUC_UNUSED,
+ Error **errp G_GNUC_UNUSED)
+{
+ return 0;
+}
+
+
+static off_t
+qio_channel_null_seek(QIOChannel *ioc G_GNUC_UNUSED,
+ off_t offset G_GNUC_UNUSED,
+ int whence G_GNUC_UNUSED,
+ Error **errp G_GNUC_UNUSED)
+{
+ return 0;
+}
+
+
+static int
+qio_channel_null_close(QIOChannel *ioc,
+ Error **errp G_GNUC_UNUSED)
+{
+ QIOChannelNull *nioc = QIO_CHANNEL_NULL(ioc);
+
+ nioc->closed = true;
+ return 0;
+}
+
+
+static void
+qio_channel_null_set_aio_fd_handler(QIOChannel *ioc G_GNUC_UNUSED,
+ AioContext *ctx G_GNUC_UNUSED,
+ IOHandler *io_read G_GNUC_UNUSED,
+ IOHandler *io_write G_GNUC_UNUSED,
+ void *opaque G_GNUC_UNUSED)
+{
+}
+
+
+static gboolean
+qio_channel_null_source_prepare(GSource *source G_GNUC_UNUSED,
+ gint *timeout)
+{
+ *timeout = -1;
+
+ return TRUE;
+}
+
+
+static gboolean
+qio_channel_null_source_check(GSource *source G_GNUC_UNUSED)
+{
+ return TRUE;
+}
+
+
+static gboolean
+qio_channel_null_source_dispatch(GSource *source,
+ GSourceFunc callback,
+ gpointer user_data)
+{
+ QIOChannelFunc func = (QIOChannelFunc)callback;
+ QIOChannelNullSource *ssource = (QIOChannelNullSource *)source;
+
+ return (*func)(ssource->ioc,
+ ssource->condition,
+ user_data);
+}
+
+
+static void
+qio_channel_null_source_finalize(GSource *source)
+{
+ QIOChannelNullSource *ssource = (QIOChannelNullSource *)source;
+
+ object_unref(OBJECT(ssource->ioc));
+}
+
+
+GSourceFuncs qio_channel_null_source_funcs = {
+ qio_channel_null_source_prepare,
+ qio_channel_null_source_check,
+ qio_channel_null_source_dispatch,
+ qio_channel_null_source_finalize
+};
+
+
+static GSource *
+qio_channel_null_create_watch(QIOChannel *ioc,
+ GIOCondition condition)
+{
+ GSource *source;
+ QIOChannelNullSource *ssource;
+
+ source = g_source_new(&qio_channel_null_source_funcs,
+ sizeof(QIOChannelNullSource));
+ ssource = (QIOChannelNullSource *)source;
+
+ ssource->ioc = ioc;
+ object_ref(OBJECT(ioc));
+
+ ssource->condition = condition;
+
+ return source;
+}
+
+
+static void
+qio_channel_null_class_init(ObjectClass *klass,
+ void *class_data G_GNUC_UNUSED)
+{
+ QIOChannelClass *ioc_klass = QIO_CHANNEL_CLASS(klass);
+
+ ioc_klass->io_writev = qio_channel_null_writev;
+ ioc_klass->io_readv = qio_channel_null_readv;
+ ioc_klass->io_set_blocking = qio_channel_null_set_blocking;
+ ioc_klass->io_seek = qio_channel_null_seek;
+ ioc_klass->io_close = qio_channel_null_close;
+ ioc_klass->io_create_watch = qio_channel_null_create_watch;
+ ioc_klass->io_set_aio_fd_handler = qio_channel_null_set_aio_fd_handler;
+}
+
+
+static const TypeInfo qio_channel_null_info = {
+ .parent = TYPE_QIO_CHANNEL,
+ .name = TYPE_QIO_CHANNEL_NULL,
+ .instance_size = sizeof(QIOChannelNull),
+ .instance_init = qio_channel_null_init,
+ .class_init = qio_channel_null_class_init,
+};
+
+
+static void
+qio_channel_null_register_types(void)
+{
+ type_register_static(&qio_channel_null_info);
+}
+
+type_init(qio_channel_null_register_types);
diff --git a/io/meson.build b/io/meson.build
index bbcd3c53a4..283b9b2bdb 100644
--- a/io/meson.build
+++ b/io/meson.build
@@ -3,6 +3,7 @@ io_ss.add(files(
'channel-buffer.c',
'channel-command.c',
'channel-file.c',
+ 'channel-null.c',
'channel-socket.c',
'channel-tls.c',
'channel-util.c',
diff --git a/io/trace-events b/io/trace-events
index c5e814eb44..3cc5cf1efd 100644
--- a/io/trace-events
+++ b/io/trace-events
@@ -10,6 +10,9 @@ qio_task_thread_result(void *task) "Task thread result task=%p"
qio_task_thread_source_attach(void *task, void *source) "Task thread source attach task=%p source=%p"
qio_task_thread_source_cancel(void *task, void *source) "Task thread source cancel task=%p source=%p"
+# channel-null.c
+qio_channel_null_new(void *ioc) "Null new ioc=%p"
+
# channel-socket.c
qio_channel_socket_new(void *ioc) "Socket new ioc=%p"
qio_channel_socket_new_fd(void *ioc, int fd) "Socket new ioc=%p fd=%d"
diff --git a/tests/unit/meson.build b/tests/unit/meson.build
index 264f2bc0c8..6074ce51aa 100644
--- a/tests/unit/meson.build
+++ b/tests/unit/meson.build
@@ -86,6 +86,7 @@ if have_block
'test-io-channel-file': ['io-channel-helpers.c', io],
'test-io-channel-command': ['io-channel-helpers.c', io],
'test-io-channel-buffer': ['io-channel-helpers.c', io],
+ 'test-io-channel-null': [io],
'test-crypto-ivgen': [io],
'test-crypto-afsplit': [io],
'test-crypto-block': [io],
diff --git a/tests/unit/test-io-channel-null.c b/tests/unit/test-io-channel-null.c
new file mode 100644
index 0000000000..b3aab17ccc
--- /dev/null
+++ b/tests/unit/test-io-channel-null.c
@@ -0,0 +1,95 @@
+/*
+ * QEMU I/O channel null test
+ *
+ * Copyright (c) 2022 Red Hat, Inc.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+#include "qemu/osdep.h"
+#include "io/channel-null.h"
+#include "qapi/error.h"
+
+static gboolean test_io_channel_watch(QIOChannel *ioc,
+ GIOCondition condition,
+ gpointer opaque)
+{
+ GIOCondition *gotcond = opaque;
+ *gotcond = condition;
+ return G_SOURCE_REMOVE;
+}
+
+static void test_io_channel_null_io(void)
+{
+ g_autoptr(QIOChannelNull) null = qio_channel_null_new();
+ char buf[1024];
+ GIOCondition gotcond = 0;
+ Error *local_err = NULL;
+
+ g_assert(qio_channel_write(QIO_CHANNEL(null),
+ "Hello World", 11,
+ &error_abort) == 11);
+
+ g_assert(qio_channel_read(QIO_CHANNEL(null),
+ buf, sizeof(buf),
+ &error_abort) == 0);
+
+ qio_channel_add_watch(QIO_CHANNEL(null),
+ G_IO_IN,
+ test_io_channel_watch,
+ &gotcond,
+ NULL);
+
+ g_main_context_iteration(NULL, false);
+
+ g_assert(gotcond == G_IO_IN);
+
+ qio_channel_add_watch(QIO_CHANNEL(null),
+ G_IO_IN | G_IO_OUT,
+ test_io_channel_watch,
+ &gotcond,
+ NULL);
+
+ g_main_context_iteration(NULL, false);
+
+ g_assert(gotcond == (G_IO_IN | G_IO_OUT));
+
+ qio_channel_close(QIO_CHANNEL(null), &error_abort);
+
+ g_assert(qio_channel_write(QIO_CHANNEL(null),
+ "Hello World", 11,
+ &local_err) == -1);
+ g_assert_nonnull(local_err);
+
+ g_clear_pointer(&local_err, error_free);
+
+ g_assert(qio_channel_read(QIO_CHANNEL(null),
+ buf, sizeof(buf),
+ &local_err) == -1);
+ g_assert_nonnull(local_err);
+
+ g_clear_pointer(&local_err, error_free);
+}
+
+int main(int argc, char **argv)
+{
+ module_call_init(MODULE_INIT_QOM);
+
+ g_test_init(&argc, &argv, NULL);
+
+ g_test_add_func("/io/channel/null/io", test_io_channel_null_io);
+
+ return g_test_run();
+}
--
2.36.1
^ permalink raw reply related [flat|nested] 51+ messages in thread* Re: [PATCH 01/20] io: add a QIOChannelNull equivalent to /dev/null
2022-05-24 11:02 ` [PATCH 01/20] io: add a QIOChannelNull equivalent to /dev/null Daniel P. Berrangé
@ 2022-05-24 21:14 ` Eric Blake
2022-06-16 16:26 ` Daniel P. Berrangé
0 siblings, 1 reply; 51+ messages in thread
From: Eric Blake @ 2022-05-24 21:14 UTC (permalink / raw)
To: Daniel P. Berrangé
Cc: qemu-devel, Dr. David Alan Gilbert, qemu-block, Stefan Hajnoczi,
Hailiang Zhang, Juan Quintela, Fam Zheng
On Tue, May 24, 2022 at 12:02:16PM +0100, Daniel P. Berrangé wrote:
> This is for code which needs a portable equivalent to a QIOChannelFile
> connected to /dev/null.
>
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
> include/io/channel-null.h | 55 +++++++
> io/channel-null.c | 237 ++++++++++++++++++++++++++++++
> io/meson.build | 1 +
> io/trace-events | 3 +
> tests/unit/meson.build | 1 +
> tests/unit/test-io-channel-null.c | 95 ++++++++++++
> 6 files changed, 392 insertions(+)
> create mode 100644 include/io/channel-null.h
> create mode 100644 io/channel-null.c
> create mode 100644 tests/unit/test-io-channel-null.c
> +/**
> + * QIOChannelNull:
> + *
> + * The QIOChannelNull object provides a channel implementation
> + * that discards all writes and returns zero bytes for all reads.
That describes the behavior of /dev/zero, not /dev/null, where reads
always fail with EOF.
> + */
> +
> +struct QIOChannelNull {
> + QIOChannel parent;
> + bool closed;
> +};
> +
> diff --git a/io/channel-null.c b/io/channel-null.c
> +
> +static ssize_t
> +qio_channel_null_readv(QIOChannel *ioc,
> + const struct iovec *iov,
> + size_t niov,
> + int **fds G_GNUC_UNUSED,
> + size_t *nfds G_GNUC_UNUSED,
> + Error **errp)
> +{
> + QIOChannelNull *nioc = QIO_CHANNEL_NULL(ioc);
> +
> + if (nioc->closed) {
> + error_setg_errno(errp, EINVAL,
> + "Channel is closed");
> + return -1;
> + }
> +
> + return 0;
> +}
But this behavior is returning early EOF instead of using iov_memset()
to read all zeroes the way /dev/zero would.
> +++ b/tests/unit/test-io-channel-null.c
> +static void test_io_channel_null_io(void)
> +{
> + g_autoptr(QIOChannelNull) null = qio_channel_null_new();
> + char buf[1024];
> + GIOCondition gotcond = 0;
> + Error *local_err = NULL;
> +
> + g_assert(qio_channel_write(QIO_CHANNEL(null),
> + "Hello World", 11,
> + &error_abort) == 11);
I still cringe seeing tests inside g_assert(), but this is not the
first instance of it.
> +
> + g_assert(qio_channel_read(QIO_CHANNEL(null),
> + buf, sizeof(buf),
> + &error_abort) == 0);
Okay, you're testing for /dev/null behavior of early EOF.
Other than misleading comments, this looks reasonable. But those
comments are core enough as to what this channel does that I don't
feel comfortable giving R-b yet.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
^ permalink raw reply [flat|nested] 51+ messages in thread* Re: [PATCH 01/20] io: add a QIOChannelNull equivalent to /dev/null
2022-05-24 21:14 ` Eric Blake
@ 2022-06-16 16:26 ` Daniel P. Berrangé
0 siblings, 0 replies; 51+ messages in thread
From: Daniel P. Berrangé @ 2022-06-16 16:26 UTC (permalink / raw)
To: Eric Blake
Cc: qemu-devel, Dr. David Alan Gilbert, qemu-block, Stefan Hajnoczi,
Hailiang Zhang, Juan Quintela, Fam Zheng
On Tue, May 24, 2022 at 04:14:31PM -0500, Eric Blake wrote:
> On Tue, May 24, 2022 at 12:02:16PM +0100, Daniel P. Berrangé wrote:
> > This is for code which needs a portable equivalent to a QIOChannelFile
> > connected to /dev/null.
> >
> > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> > ---
> > include/io/channel-null.h | 55 +++++++
> > io/channel-null.c | 237 ++++++++++++++++++++++++++++++
> > io/meson.build | 1 +
> > io/trace-events | 3 +
> > tests/unit/meson.build | 1 +
> > tests/unit/test-io-channel-null.c | 95 ++++++++++++
> > 6 files changed, 392 insertions(+)
> > create mode 100644 include/io/channel-null.h
> > create mode 100644 io/channel-null.c
> > create mode 100644 tests/unit/test-io-channel-null.c
>
> > +/**
> > + * QIOChannelNull:
> > + *
> > + * The QIOChannelNull object provides a channel implementation
> > + * that discards all writes and returns zero bytes for all reads.
>
> That describes the behavior of /dev/zero, not /dev/null, where reads
> always fail with EOF.
Nb, I wrote 'zero bytes' (as in "byte count equal to zero") not
'zeroed bytes' (as in "set all bytes to the value zero").
I'll reword it to make it less confusing though.
>
> > + */
> > +
> > +struct QIOChannelNull {
> > + QIOChannel parent;
> > + bool closed;
> > +};
> > +
>
> > diff --git a/io/channel-null.c b/io/channel-null.c
>
> > +
> > +static ssize_t
> > +qio_channel_null_readv(QIOChannel *ioc,
> > + const struct iovec *iov,
> > + size_t niov,
> > + int **fds G_GNUC_UNUSED,
> > + size_t *nfds G_GNUC_UNUSED,
> > + Error **errp)
> > +{
> > + QIOChannelNull *nioc = QIO_CHANNEL_NULL(ioc);
> > +
> > + if (nioc->closed) {
> > + error_setg_errno(errp, EINVAL,
> > + "Channel is closed");
> > + return -1;
> > + }
> > +
> > + return 0;
> > +}
>
> But this behavior is returning early EOF instead of using iov_memset()
> to read all zeroes the way /dev/zero would.
>
> > +++ b/tests/unit/test-io-channel-null.c
>
> > +static void test_io_channel_null_io(void)
> > +{
> > + g_autoptr(QIOChannelNull) null = qio_channel_null_new();
> > + char buf[1024];
> > + GIOCondition gotcond = 0;
> > + Error *local_err = NULL;
> > +
> > + g_assert(qio_channel_write(QIO_CHANNEL(null),
> > + "Hello World", 11,
> > + &error_abort) == 11);
>
> I still cringe seeing tests inside g_assert(), but this is not the
> first instance of it.
>
> > +
> > + g_assert(qio_channel_read(QIO_CHANNEL(null),
> > + buf, sizeof(buf),
> > + &error_abort) == 0);
>
> Okay, you're testing for /dev/null behavior of early EOF.
>
> Other than misleading comments, this looks reasonable. But those
> comments are core enough as to what this channel does that I don't
> feel comfortable giving R-b yet.
>
> --
> Eric Blake, Principal Software Engineer
> Red Hat, Inc. +1-919-301-3266
> Virtualization: qemu.org | libvirt.org
>
With regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
^ permalink raw reply [flat|nested] 51+ messages in thread
* [PATCH 02/20] migration: switch to use QIOChannelNull for dummy channel
2022-05-24 11:02 [PATCH 00/20] migration: remove QEMUFileOps concept and assume use of QIOChannel Daniel P. Berrangé
2022-05-24 11:02 ` [PATCH 01/20] io: add a QIOChannelNull equivalent to /dev/null Daniel P. Berrangé
@ 2022-05-24 11:02 ` Daniel P. Berrangé
2022-05-24 21:15 ` Eric Blake
2022-05-24 11:02 ` [PATCH 03/20] migration: remove unreachble RDMA code in save_hook impl Daniel P. Berrangé
` (17 subsequent siblings)
19 siblings, 1 reply; 51+ messages in thread
From: Daniel P. Berrangé @ 2022-05-24 11:02 UTC (permalink / raw)
To: qemu-devel
Cc: Daniel P. Berrangé,
Dr. David Alan Gilbert, qemu-block, Stefan Hajnoczi,
Hailiang Zhang, Juan Quintela, Fam Zheng
This removes one further custom impl of QEMUFile, in favour of a
QIOChannel based impl.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
migration/ram.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/migration/ram.c b/migration/ram.c
index 5f5e37f64d..89082716d6 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -32,11 +32,13 @@
#include "qemu/bitmap.h"
#include "qemu/madvise.h"
#include "qemu/main-loop.h"
+#include "io/channel-null.h"
#include "xbzrle.h"
#include "ram.h"
#include "migration.h"
#include "migration/register.h"
#include "migration/misc.h"
+#include "migration/qemu-file-channel.h"
#include "qemu-file.h"
#include "postcopy-ram.h"
#include "page_cache.h"
@@ -457,8 +459,6 @@ static QemuThread *compress_threads;
*/
static QemuMutex comp_done_lock;
static QemuCond comp_done_cond;
-/* The empty QEMUFileOps will be used by file in CompressParam */
-static const QEMUFileOps empty_ops = { };
static QEMUFile *decomp_file;
static DecompressParam *decomp_param;
@@ -569,7 +569,8 @@ static int compress_threads_save_setup(void)
/* comp_param[i].file is just used as a dummy buffer to save data,
* set its ops to empty.
*/
- comp_param[i].file = qemu_fopen_ops(NULL, &empty_ops, false);
+ comp_param[i].file = qemu_fopen_channel_output(
+ QIO_CHANNEL(qio_channel_null_new()));
comp_param[i].done = true;
comp_param[i].quit = false;
qemu_mutex_init(&comp_param[i].mutex);
--
2.36.1
^ permalink raw reply related [flat|nested] 51+ messages in thread* Re: [PATCH 02/20] migration: switch to use QIOChannelNull for dummy channel
2022-05-24 11:02 ` [PATCH 02/20] migration: switch to use QIOChannelNull for dummy channel Daniel P. Berrangé
@ 2022-05-24 21:15 ` Eric Blake
0 siblings, 0 replies; 51+ messages in thread
From: Eric Blake @ 2022-05-24 21:15 UTC (permalink / raw)
To: Daniel P. Berrangé
Cc: qemu-devel, Dr. David Alan Gilbert, qemu-block, Stefan Hajnoczi,
Hailiang Zhang, Juan Quintela, Fam Zheng
On Tue, May 24, 2022 at 12:02:17PM +0100, Daniel P. Berrangé wrote:
> This removes one further custom impl of QEMUFile, in favour of a
> QIOChannel based impl.
>
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
> migration/ram.c | 7 ++++---
> 1 file changed, 4 insertions(+), 3 deletions(-)
>
Reviewed-by: Eric Blake <eblake@redhat.com>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
^ permalink raw reply [flat|nested] 51+ messages in thread
* [PATCH 03/20] migration: remove unreachble RDMA code in save_hook impl
2022-05-24 11:02 [PATCH 00/20] migration: remove QEMUFileOps concept and assume use of QIOChannel Daniel P. Berrangé
2022-05-24 11:02 ` [PATCH 01/20] io: add a QIOChannelNull equivalent to /dev/null Daniel P. Berrangé
2022-05-24 11:02 ` [PATCH 02/20] migration: switch to use QIOChannelNull for dummy channel Daniel P. Berrangé
@ 2022-05-24 11:02 ` Daniel P. Berrangé
2022-05-25 12:29 ` Eric Blake
2022-06-08 17:59 ` Dr. David Alan Gilbert
2022-05-24 11:02 ` [PATCH 04/20] migration: rename rate limiting fields in QEMUFile Daniel P. Berrangé
` (16 subsequent siblings)
19 siblings, 2 replies; 51+ messages in thread
From: Daniel P. Berrangé @ 2022-05-24 11:02 UTC (permalink / raw)
To: qemu-devel
Cc: Daniel P. Berrangé,
Dr. David Alan Gilbert, qemu-block, Stefan Hajnoczi,
Hailiang Zhang, Juan Quintela, Fam Zheng
The QEMUFile 'save_hook' callback has a 'size_t size' parameter.
The RDMA impl of this has logic that takes different actions
depending on whether the value is zero or non-zero. It has
commented out logic that would have taken further actions
if the value was negative.
The only place where the 'save_hook' callback is invoked is
the ram_control_save_page() method, which passes 'size'
through from its caller. The only caller of this method is
in turn control_save_page(). This method unconditionally
passes the 'TARGET_PAGE_SIZE' constant for the 'size' parameter.
IOW, the only scenario for 'size' that can execute in the
qemu_rdma_save_page method is 'size > 0'. The remaining code
has been unreachable since RDMA support was first introduced
9 years ago.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
migration/rdma.c | 120 +++++++++--------------------------------------
1 file changed, 21 insertions(+), 99 deletions(-)
diff --git a/migration/rdma.c b/migration/rdma.c
index 672d1958a9..6e7756bee7 100644
--- a/migration/rdma.c
+++ b/migration/rdma.c
@@ -1486,34 +1486,6 @@ static uint64_t qemu_rdma_make_wrid(uint64_t wr_id, uint64_t index,
return result;
}
-/*
- * Set bit for unregistration in the next iteration.
- * We cannot transmit right here, but will unpin later.
- */
-static void qemu_rdma_signal_unregister(RDMAContext *rdma, uint64_t index,
- uint64_t chunk, uint64_t wr_id)
-{
- if (rdma->unregistrations[rdma->unregister_next] != 0) {
- error_report("rdma migration: queue is full");
- } else {
- RDMALocalBlock *block = &(rdma->local_ram_blocks.block[index]);
-
- if (!test_and_set_bit(chunk, block->unregister_bitmap)) {
- trace_qemu_rdma_signal_unregister_append(chunk,
- rdma->unregister_next);
-
- rdma->unregistrations[rdma->unregister_next++] =
- qemu_rdma_make_wrid(wr_id, index, chunk);
-
- if (rdma->unregister_next == RDMA_SIGNALED_SEND_MAX) {
- rdma->unregister_next = 0;
- }
- } else {
- trace_qemu_rdma_signal_unregister_already(chunk);
- }
- }
-}
-
/*
* Consult the connection manager to see a work request
* (of any kind) has completed.
@@ -3278,23 +3250,7 @@ qio_channel_rdma_shutdown(QIOChannel *ioc,
* Offset is an offset to be added to block_offset and used
* to also lookup the corresponding RAMBlock.
*
- * @size > 0 :
- * Initiate an transfer this size.
- *
- * @size == 0 :
- * A 'hint' or 'advice' that means that we wish to speculatively
- * and asynchronously unregister this memory. In this case, there is no
- * guarantee that the unregister will actually happen, for example,
- * if the memory is being actively transmitted. Additionally, the memory
- * may be re-registered at any future time if a write within the same
- * chunk was requested again, even if you attempted to unregister it
- * here.
- *
- * @size < 0 : TODO, not yet supported
- * Unregister the memory NOW. This means that the caller does not
- * expect there to be any future RDMA transfers and we just want to clean
- * things up. This is used in case the upper layer owns the memory and
- * cannot wait for qemu_fclose() to occur.
+ * @size : Number of bytes to transfer
*
* @bytes_sent : User-specificed pointer to indicate how many bytes were
* sent. Usually, this will not be more than a few bytes of
@@ -3323,61 +3279,27 @@ static size_t qemu_rdma_save_page(QEMUFile *f, void *opaque,
qemu_fflush(f);
- if (size > 0) {
- /*
- * Add this page to the current 'chunk'. If the chunk
- * is full, or the page doesn't belong to the current chunk,
- * an actual RDMA write will occur and a new chunk will be formed.
- */
- ret = qemu_rdma_write(f, rdma, block_offset, offset, size);
- if (ret < 0) {
- error_report("rdma migration: write error! %d", ret);
- goto err;
- }
-
- /*
- * We always return 1 bytes because the RDMA
- * protocol is completely asynchronous. We do not yet know
- * whether an identified chunk is zero or not because we're
- * waiting for other pages to potentially be merged with
- * the current chunk. So, we have to call qemu_update_position()
- * later on when the actual write occurs.
- */
- if (bytes_sent) {
- *bytes_sent = 1;
- }
- } else {
- uint64_t index, chunk;
-
- /* TODO: Change QEMUFileOps prototype to be signed: size_t => long
- if (size < 0) {
- ret = qemu_rdma_drain_cq(f, rdma);
- if (ret < 0) {
- fprintf(stderr, "rdma: failed to synchronously drain"
- " completion queue before unregistration.\n");
- goto err;
- }
- }
- */
-
- ret = qemu_rdma_search_ram_block(rdma, block_offset,
- offset, size, &index, &chunk);
-
- if (ret) {
- error_report("ram block search failed");
- goto err;
- }
-
- qemu_rdma_signal_unregister(rdma, index, chunk, 0);
+ /*
+ * Add this page to the current 'chunk'. If the chunk
+ * is full, or the page doesn't belong to the current chunk,
+ * an actual RDMA write will occur and a new chunk will be formed.
+ */
+ ret = qemu_rdma_write(f, rdma, block_offset, offset, size);
+ if (ret < 0) {
+ error_report("rdma migration: write error! %d", ret);
+ goto err;
+ }
- /*
- * TODO: Synchronous, guaranteed unregistration (should not occur during
- * fast-path). Otherwise, unregisters will process on the next call to
- * qemu_rdma_drain_cq()
- if (size < 0) {
- qemu_rdma_unregister_waiting(rdma);
- }
- */
+ /*
+ * We always return 1 bytes because the RDMA
+ * protocol is completely asynchronous. We do not yet know
+ * whether an identified chunk is zero or not because we're
+ * waiting for other pages to potentially be merged with
+ * the current chunk. So, we have to call qemu_update_position()
+ * later on when the actual write occurs.
+ */
+ if (bytes_sent) {
+ *bytes_sent = 1;
}
/*
--
2.36.1
^ permalink raw reply related [flat|nested] 51+ messages in thread* Re: [PATCH 03/20] migration: remove unreachble RDMA code in save_hook impl
2022-05-24 11:02 ` [PATCH 03/20] migration: remove unreachble RDMA code in save_hook impl Daniel P. Berrangé
@ 2022-05-25 12:29 ` Eric Blake
2022-06-08 17:59 ` Dr. David Alan Gilbert
1 sibling, 0 replies; 51+ messages in thread
From: Eric Blake @ 2022-05-25 12:29 UTC (permalink / raw)
To: Daniel P. Berrangé
Cc: qemu-devel, Dr. David Alan Gilbert, qemu-block, Stefan Hajnoczi,
Hailiang Zhang, Juan Quintela, Fam Zheng
On Tue, May 24, 2022 at 12:02:18PM +0100, Daniel P. Berrangé wrote:
> The QEMUFile 'save_hook' callback has a 'size_t size' parameter.
>
> The RDMA impl of this has logic that takes different actions
> depending on whether the value is zero or non-zero. It has
> commented out logic that would have taken further actions
> if the value was negative.
>
> The only place where the 'save_hook' callback is invoked is
> the ram_control_save_page() method, which passes 'size'
> through from its caller. The only caller of this method is
> in turn control_save_page(). This method unconditionally
> passes the 'TARGET_PAGE_SIZE' constant for the 'size' parameter.
>
> IOW, the only scenario for 'size' that can execute in the
> qemu_rdma_save_page method is 'size > 0'. The remaining code
> has been unreachable since RDMA support was first introduced
> 9 years ago.
>
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
> migration/rdma.c | 120 +++++++++--------------------------------------
> 1 file changed, 21 insertions(+), 99 deletions(-)
>
Reviewed-by: Eric Blake <eblake@redhat.com>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
^ permalink raw reply [flat|nested] 51+ messages in thread
* Re: [PATCH 03/20] migration: remove unreachble RDMA code in save_hook impl
2022-05-24 11:02 ` [PATCH 03/20] migration: remove unreachble RDMA code in save_hook impl Daniel P. Berrangé
2022-05-25 12:29 ` Eric Blake
@ 2022-06-08 17:59 ` Dr. David Alan Gilbert
1 sibling, 0 replies; 51+ messages in thread
From: Dr. David Alan Gilbert @ 2022-06-08 17:59 UTC (permalink / raw)
To: Daniel P. Berrangé
Cc: qemu-devel, qemu-block, Stefan Hajnoczi, Hailiang Zhang,
Juan Quintela, Fam Zheng
* Daniel P. Berrangé (berrange@redhat.com) wrote:
> The QEMUFile 'save_hook' callback has a 'size_t size' parameter.
>
> The RDMA impl of this has logic that takes different actions
> depending on whether the value is zero or non-zero. It has
> commented out logic that would have taken further actions
> if the value was negative.
>
> The only place where the 'save_hook' callback is invoked is
> the ram_control_save_page() method, which passes 'size'
> through from its caller. The only caller of this method is
> in turn control_save_page(). This method unconditionally
> passes the 'TARGET_PAGE_SIZE' constant for the 'size' parameter.
>
> IOW, the only scenario for 'size' that can execute in the
> qemu_rdma_save_page method is 'size > 0'. The remaining code
> has been unreachable since RDMA support was first introduced
> 9 years ago.
>
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Ah good; less rdma code!
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> ---
> migration/rdma.c | 120 +++++++++--------------------------------------
> 1 file changed, 21 insertions(+), 99 deletions(-)
>
> diff --git a/migration/rdma.c b/migration/rdma.c
> index 672d1958a9..6e7756bee7 100644
> --- a/migration/rdma.c
> +++ b/migration/rdma.c
> @@ -1486,34 +1486,6 @@ static uint64_t qemu_rdma_make_wrid(uint64_t wr_id, uint64_t index,
> return result;
> }
>
> -/*
> - * Set bit for unregistration in the next iteration.
> - * We cannot transmit right here, but will unpin later.
> - */
> -static void qemu_rdma_signal_unregister(RDMAContext *rdma, uint64_t index,
> - uint64_t chunk, uint64_t wr_id)
> -{
> - if (rdma->unregistrations[rdma->unregister_next] != 0) {
> - error_report("rdma migration: queue is full");
> - } else {
> - RDMALocalBlock *block = &(rdma->local_ram_blocks.block[index]);
> -
> - if (!test_and_set_bit(chunk, block->unregister_bitmap)) {
> - trace_qemu_rdma_signal_unregister_append(chunk,
> - rdma->unregister_next);
> -
> - rdma->unregistrations[rdma->unregister_next++] =
> - qemu_rdma_make_wrid(wr_id, index, chunk);
> -
> - if (rdma->unregister_next == RDMA_SIGNALED_SEND_MAX) {
> - rdma->unregister_next = 0;
> - }
> - } else {
> - trace_qemu_rdma_signal_unregister_already(chunk);
> - }
> - }
> -}
> -
> /*
> * Consult the connection manager to see a work request
> * (of any kind) has completed.
> @@ -3278,23 +3250,7 @@ qio_channel_rdma_shutdown(QIOChannel *ioc,
> * Offset is an offset to be added to block_offset and used
> * to also lookup the corresponding RAMBlock.
> *
> - * @size > 0 :
> - * Initiate an transfer this size.
> - *
> - * @size == 0 :
> - * A 'hint' or 'advice' that means that we wish to speculatively
> - * and asynchronously unregister this memory. In this case, there is no
> - * guarantee that the unregister will actually happen, for example,
> - * if the memory is being actively transmitted. Additionally, the memory
> - * may be re-registered at any future time if a write within the same
> - * chunk was requested again, even if you attempted to unregister it
> - * here.
> - *
> - * @size < 0 : TODO, not yet supported
> - * Unregister the memory NOW. This means that the caller does not
> - * expect there to be any future RDMA transfers and we just want to clean
> - * things up. This is used in case the upper layer owns the memory and
> - * cannot wait for qemu_fclose() to occur.
> + * @size : Number of bytes to transfer
> *
> * @bytes_sent : User-specificed pointer to indicate how many bytes were
> * sent. Usually, this will not be more than a few bytes of
> @@ -3323,61 +3279,27 @@ static size_t qemu_rdma_save_page(QEMUFile *f, void *opaque,
>
> qemu_fflush(f);
>
> - if (size > 0) {
> - /*
> - * Add this page to the current 'chunk'. If the chunk
> - * is full, or the page doesn't belong to the current chunk,
> - * an actual RDMA write will occur and a new chunk will be formed.
> - */
> - ret = qemu_rdma_write(f, rdma, block_offset, offset, size);
> - if (ret < 0) {
> - error_report("rdma migration: write error! %d", ret);
> - goto err;
> - }
> -
> - /*
> - * We always return 1 bytes because the RDMA
> - * protocol is completely asynchronous. We do not yet know
> - * whether an identified chunk is zero or not because we're
> - * waiting for other pages to potentially be merged with
> - * the current chunk. So, we have to call qemu_update_position()
> - * later on when the actual write occurs.
> - */
> - if (bytes_sent) {
> - *bytes_sent = 1;
> - }
> - } else {
> - uint64_t index, chunk;
> -
> - /* TODO: Change QEMUFileOps prototype to be signed: size_t => long
> - if (size < 0) {
> - ret = qemu_rdma_drain_cq(f, rdma);
> - if (ret < 0) {
> - fprintf(stderr, "rdma: failed to synchronously drain"
> - " completion queue before unregistration.\n");
> - goto err;
> - }
> - }
> - */
> -
> - ret = qemu_rdma_search_ram_block(rdma, block_offset,
> - offset, size, &index, &chunk);
> -
> - if (ret) {
> - error_report("ram block search failed");
> - goto err;
> - }
> -
> - qemu_rdma_signal_unregister(rdma, index, chunk, 0);
> + /*
> + * Add this page to the current 'chunk'. If the chunk
> + * is full, or the page doesn't belong to the current chunk,
> + * an actual RDMA write will occur and a new chunk will be formed.
> + */
> + ret = qemu_rdma_write(f, rdma, block_offset, offset, size);
> + if (ret < 0) {
> + error_report("rdma migration: write error! %d", ret);
> + goto err;
> + }
>
> - /*
> - * TODO: Synchronous, guaranteed unregistration (should not occur during
> - * fast-path). Otherwise, unregisters will process on the next call to
> - * qemu_rdma_drain_cq()
> - if (size < 0) {
> - qemu_rdma_unregister_waiting(rdma);
> - }
> - */
> + /*
> + * We always return 1 bytes because the RDMA
> + * protocol is completely asynchronous. We do not yet know
> + * whether an identified chunk is zero or not because we're
> + * waiting for other pages to potentially be merged with
> + * the current chunk. So, we have to call qemu_update_position()
> + * later on when the actual write occurs.
> + */
> + if (bytes_sent) {
> + *bytes_sent = 1;
> }
>
> /*
> --
> 2.36.1
>
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
^ permalink raw reply [flat|nested] 51+ messages in thread
* [PATCH 04/20] migration: rename rate limiting fields in QEMUFile
2022-05-24 11:02 [PATCH 00/20] migration: remove QEMUFileOps concept and assume use of QIOChannel Daniel P. Berrangé
` (2 preceding siblings ...)
2022-05-24 11:02 ` [PATCH 03/20] migration: remove unreachble RDMA code in save_hook impl Daniel P. Berrangé
@ 2022-05-24 11:02 ` Daniel P. Berrangé
2022-06-09 9:29 ` Dr. David Alan Gilbert
2022-05-24 11:02 ` [PATCH 05/20] migration: rename 'pos' field in QEMUFile to 'bytes_processed' Daniel P. Berrangé
` (15 subsequent siblings)
19 siblings, 1 reply; 51+ messages in thread
From: Daniel P. Berrangé @ 2022-05-24 11:02 UTC (permalink / raw)
To: qemu-devel
Cc: Daniel P. Berrangé,
Dr. David Alan Gilbert, qemu-block, Stefan Hajnoczi,
Hailiang Zhang, Juan Quintela, Fam Zheng
This renames the following QEMUFile fields
* bytes_xfer -> rate_limit_used
* xfer_limit -> rate_limit_max
The intent is to make it clear that 'bytes_xfer' is specifically related
to rate limiting of data and applies to data queued, which need not have
been transferred on the wire yet if a flush hasn't taken place.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
migration/qemu-file.c | 30 +++++++++++++++++++-----------
1 file changed, 19 insertions(+), 11 deletions(-)
diff --git a/migration/qemu-file.c b/migration/qemu-file.c
index 1479cddad9..03f0b13a55 100644
--- a/migration/qemu-file.c
+++ b/migration/qemu-file.c
@@ -39,8 +39,16 @@ struct QEMUFile {
const QEMUFileHooks *hooks;
void *opaque;
- int64_t bytes_xfer;
- int64_t xfer_limit;
+ /*
+ * Maximum amount of data in bytes to transfer during one
+ * rate limiting time window
+ */
+ int64_t rate_limit_max;
+ /*
+ * Total amount of data in bytes queued for transfer
+ * during this rate limiting time window
+ */
+ int64_t rate_limit_used;
int64_t pos; /* start of buffer when writing, end of buffer
when reading */
@@ -304,7 +312,7 @@ size_t ram_control_save_page(QEMUFile *f, ram_addr_t block_offset,
int ret = f->hooks->save_page(f, f->opaque, block_offset,
offset, size, bytes_sent);
if (ret != RAM_SAVE_CONTROL_NOT_SUPP) {
- f->bytes_xfer += size;
+ f->rate_limit_used += size;
}
if (ret != RAM_SAVE_CONTROL_DELAYED &&
@@ -457,7 +465,7 @@ void qemu_put_buffer_async(QEMUFile *f, const uint8_t *buf, size_t size,
return;
}
- f->bytes_xfer += size;
+ f->rate_limit_used += size;
add_to_iovec(f, buf, size, may_free);
}
@@ -475,7 +483,7 @@ void qemu_put_buffer(QEMUFile *f, const uint8_t *buf, size_t size)
l = size;
}
memcpy(f->buf + f->buf_index, buf, l);
- f->bytes_xfer += l;
+ f->rate_limit_used += l;
add_buf_to_iovec(f, l);
if (qemu_file_get_error(f)) {
break;
@@ -492,7 +500,7 @@ void qemu_put_byte(QEMUFile *f, int v)
}
f->buf[f->buf_index] = v;
- f->bytes_xfer++;
+ f->rate_limit_used++;
add_buf_to_iovec(f, 1);
}
@@ -674,7 +682,7 @@ int qemu_file_rate_limit(QEMUFile *f)
if (qemu_file_get_error(f)) {
return 1;
}
- if (f->xfer_limit > 0 && f->bytes_xfer > f->xfer_limit) {
+ if (f->rate_limit_max > 0 && f->rate_limit_used > f->rate_limit_max) {
return 1;
}
return 0;
@@ -682,22 +690,22 @@ int qemu_file_rate_limit(QEMUFile *f)
int64_t qemu_file_get_rate_limit(QEMUFile *f)
{
- return f->xfer_limit;
+ return f->rate_limit_max;
}
void qemu_file_set_rate_limit(QEMUFile *f, int64_t limit)
{
- f->xfer_limit = limit;
+ f->rate_limit_max = limit;
}
void qemu_file_reset_rate_limit(QEMUFile *f)
{
- f->bytes_xfer = 0;
+ f->rate_limit_used = 0;
}
void qemu_file_update_transfer(QEMUFile *f, int64_t len)
{
- f->bytes_xfer += len;
+ f->rate_limit_used += len;
}
void qemu_put_be16(QEMUFile *f, unsigned int v)
--
2.36.1
^ permalink raw reply related [flat|nested] 51+ messages in thread* Re: [PATCH 04/20] migration: rename rate limiting fields in QEMUFile
2022-05-24 11:02 ` [PATCH 04/20] migration: rename rate limiting fields in QEMUFile Daniel P. Berrangé
@ 2022-06-09 9:29 ` Dr. David Alan Gilbert
0 siblings, 0 replies; 51+ messages in thread
From: Dr. David Alan Gilbert @ 2022-06-09 9:29 UTC (permalink / raw)
To: Daniel P. Berrangé
Cc: qemu-devel, qemu-block, Stefan Hajnoczi, Hailiang Zhang,
Juan Quintela, Fam Zheng
* Daniel P. Berrangé (berrange@redhat.com) wrote:
> This renames the following QEMUFile fields
>
> * bytes_xfer -> rate_limit_used
> * xfer_limit -> rate_limit_max
>
> The intent is to make it clear that 'bytes_xfer' is specifically related
> to rate limiting of data and applies to data queued, which need not have
> been transferred on the wire yet if a flush hasn't taken place.
>
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> ---
> migration/qemu-file.c | 30 +++++++++++++++++++-----------
> 1 file changed, 19 insertions(+), 11 deletions(-)
>
> diff --git a/migration/qemu-file.c b/migration/qemu-file.c
> index 1479cddad9..03f0b13a55 100644
> --- a/migration/qemu-file.c
> +++ b/migration/qemu-file.c
> @@ -39,8 +39,16 @@ struct QEMUFile {
> const QEMUFileHooks *hooks;
> void *opaque;
>
> - int64_t bytes_xfer;
> - int64_t xfer_limit;
> + /*
> + * Maximum amount of data in bytes to transfer during one
> + * rate limiting time window
> + */
> + int64_t rate_limit_max;
> + /*
> + * Total amount of data in bytes queued for transfer
> + * during this rate limiting time window
> + */
> + int64_t rate_limit_used;
>
> int64_t pos; /* start of buffer when writing, end of buffer
> when reading */
> @@ -304,7 +312,7 @@ size_t ram_control_save_page(QEMUFile *f, ram_addr_t block_offset,
> int ret = f->hooks->save_page(f, f->opaque, block_offset,
> offset, size, bytes_sent);
> if (ret != RAM_SAVE_CONTROL_NOT_SUPP) {
> - f->bytes_xfer += size;
> + f->rate_limit_used += size;
> }
>
> if (ret != RAM_SAVE_CONTROL_DELAYED &&
> @@ -457,7 +465,7 @@ void qemu_put_buffer_async(QEMUFile *f, const uint8_t *buf, size_t size,
> return;
> }
>
> - f->bytes_xfer += size;
> + f->rate_limit_used += size;
> add_to_iovec(f, buf, size, may_free);
> }
>
> @@ -475,7 +483,7 @@ void qemu_put_buffer(QEMUFile *f, const uint8_t *buf, size_t size)
> l = size;
> }
> memcpy(f->buf + f->buf_index, buf, l);
> - f->bytes_xfer += l;
> + f->rate_limit_used += l;
> add_buf_to_iovec(f, l);
> if (qemu_file_get_error(f)) {
> break;
> @@ -492,7 +500,7 @@ void qemu_put_byte(QEMUFile *f, int v)
> }
>
> f->buf[f->buf_index] = v;
> - f->bytes_xfer++;
> + f->rate_limit_used++;
> add_buf_to_iovec(f, 1);
> }
>
> @@ -674,7 +682,7 @@ int qemu_file_rate_limit(QEMUFile *f)
> if (qemu_file_get_error(f)) {
> return 1;
> }
> - if (f->xfer_limit > 0 && f->bytes_xfer > f->xfer_limit) {
> + if (f->rate_limit_max > 0 && f->rate_limit_used > f->rate_limit_max) {
> return 1;
> }
> return 0;
> @@ -682,22 +690,22 @@ int qemu_file_rate_limit(QEMUFile *f)
>
> int64_t qemu_file_get_rate_limit(QEMUFile *f)
> {
> - return f->xfer_limit;
> + return f->rate_limit_max;
> }
>
> void qemu_file_set_rate_limit(QEMUFile *f, int64_t limit)
> {
> - f->xfer_limit = limit;
> + f->rate_limit_max = limit;
> }
>
> void qemu_file_reset_rate_limit(QEMUFile *f)
> {
> - f->bytes_xfer = 0;
> + f->rate_limit_used = 0;
> }
>
> void qemu_file_update_transfer(QEMUFile *f, int64_t len)
> {
> - f->bytes_xfer += len;
> + f->rate_limit_used += len;
> }
>
> void qemu_put_be16(QEMUFile *f, unsigned int v)
> --
> 2.36.1
>
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
^ permalink raw reply [flat|nested] 51+ messages in thread
* [PATCH 05/20] migration: rename 'pos' field in QEMUFile to 'bytes_processed'
2022-05-24 11:02 [PATCH 00/20] migration: remove QEMUFileOps concept and assume use of QIOChannel Daniel P. Berrangé
` (3 preceding siblings ...)
2022-05-24 11:02 ` [PATCH 04/20] migration: rename rate limiting fields in QEMUFile Daniel P. Berrangé
@ 2022-05-24 11:02 ` Daniel P. Berrangé
2022-06-09 9:51 ` Dr. David Alan Gilbert
2022-05-24 11:02 ` [PATCH 06/20] migration: rename qemu_ftell to qemu_file_total_transferred Daniel P. Berrangé
` (14 subsequent siblings)
19 siblings, 1 reply; 51+ messages in thread
From: Daniel P. Berrangé @ 2022-05-24 11:02 UTC (permalink / raw)
To: qemu-devel
Cc: Daniel P. Berrangé,
Dr. David Alan Gilbert, qemu-block, Stefan Hajnoczi,
Hailiang Zhang, Juan Quintela, Fam Zheng
This makes the field name align with the newly introduced method
names in the previous commit.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
migration/qemu-file.c | 19 ++++++++++---------
1 file changed, 10 insertions(+), 9 deletions(-)
diff --git a/migration/qemu-file.c b/migration/qemu-file.c
index 03f0b13a55..b21da4c5bf 100644
--- a/migration/qemu-file.c
+++ b/migration/qemu-file.c
@@ -50,8 +50,9 @@ struct QEMUFile {
*/
int64_t rate_limit_used;
- int64_t pos; /* start of buffer when writing, end of buffer
- when reading */
+ /* The sum of bytes transferred on the wire */
+ int64_t total_transferred;
+
int buf_index;
int buf_size; /* 0 when writing */
uint8_t buf[IO_BUF_SIZE];
@@ -241,14 +242,14 @@ void qemu_fflush(QEMUFile *f)
}
if (f->iovcnt > 0) {
expect = iov_size(f->iov, f->iovcnt);
- ret = f->ops->writev_buffer(f->opaque, f->iov, f->iovcnt, f->pos,
+ ret = f->ops->writev_buffer(f->opaque, f->iov, f->iovcnt, f->total_transferred,
&local_error);
qemu_iovec_release_ram(f);
}
if (ret >= 0) {
- f->pos += ret;
+ f->total_transferred += ret;
}
/* We expect the QEMUFile write impl to send the full
* data set we requested, so sanity check that.
@@ -357,11 +358,11 @@ static ssize_t qemu_fill_buffer(QEMUFile *f)
return 0;
}
- len = f->ops->get_buffer(f->opaque, f->buf + pending, f->pos,
+ len = f->ops->get_buffer(f->opaque, f->buf + pending, f->total_transferred,
IO_BUF_SIZE - pending, &local_error);
if (len > 0) {
f->buf_size += len;
- f->pos += len;
+ f->total_transferred += len;
} else if (len == 0) {
qemu_file_set_error_obj(f, -EIO, local_error);
} else if (len != -EAGAIN) {
@@ -375,7 +376,7 @@ static ssize_t qemu_fill_buffer(QEMUFile *f)
void qemu_update_position(QEMUFile *f, size_t size)
{
- f->pos += size;
+ f->total_transferred += size;
}
/** Closes the file
@@ -658,7 +659,7 @@ int qemu_get_byte(QEMUFile *f)
int64_t qemu_ftell_fast(QEMUFile *f)
{
- int64_t ret = f->pos;
+ int64_t ret = f->total_transferred;
int i;
for (i = 0; i < f->iovcnt; i++) {
@@ -671,7 +672,7 @@ int64_t qemu_ftell_fast(QEMUFile *f)
int64_t qemu_ftell(QEMUFile *f)
{
qemu_fflush(f);
- return f->pos;
+ return f->total_transferred;
}
int qemu_file_rate_limit(QEMUFile *f)
--
2.36.1
^ permalink raw reply related [flat|nested] 51+ messages in thread* Re: [PATCH 05/20] migration: rename 'pos' field in QEMUFile to 'bytes_processed'
2022-05-24 11:02 ` [PATCH 05/20] migration: rename 'pos' field in QEMUFile to 'bytes_processed' Daniel P. Berrangé
@ 2022-06-09 9:51 ` Dr. David Alan Gilbert
2022-06-09 9:57 ` Daniel P. Berrangé
0 siblings, 1 reply; 51+ messages in thread
From: Dr. David Alan Gilbert @ 2022-06-09 9:51 UTC (permalink / raw)
To: Daniel P. Berrangé
Cc: qemu-devel, qemu-block, Stefan Hajnoczi, Hailiang Zhang,
Juan Quintela, Fam Zheng
* Daniel P. Berrangé (berrange@redhat.com) wrote:
> This makes the field name align with the newly introduced method
> names in the previous commit.
I think that's the method in the following commits?
tbh I'm not sure about this; 'pos' is still passed to writev_buffer
and get_buffer to say where the data is - and that makes it a 'pos'
still rather than a simple stats counter.
Dave
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
> migration/qemu-file.c | 19 ++++++++++---------
> 1 file changed, 10 insertions(+), 9 deletions(-)
>
> diff --git a/migration/qemu-file.c b/migration/qemu-file.c
> index 03f0b13a55..b21da4c5bf 100644
> --- a/migration/qemu-file.c
> +++ b/migration/qemu-file.c
> @@ -50,8 +50,9 @@ struct QEMUFile {
> */
> int64_t rate_limit_used;
>
> - int64_t pos; /* start of buffer when writing, end of buffer
> - when reading */
> + /* The sum of bytes transferred on the wire */
> + int64_t total_transferred;
> +
> int buf_index;
> int buf_size; /* 0 when writing */
> uint8_t buf[IO_BUF_SIZE];
> @@ -241,14 +242,14 @@ void qemu_fflush(QEMUFile *f)
> }
> if (f->iovcnt > 0) {
> expect = iov_size(f->iov, f->iovcnt);
> - ret = f->ops->writev_buffer(f->opaque, f->iov, f->iovcnt, f->pos,
> + ret = f->ops->writev_buffer(f->opaque, f->iov, f->iovcnt, f->total_transferred,
> &local_error);
>
> qemu_iovec_release_ram(f);
> }
>
> if (ret >= 0) {
> - f->pos += ret;
> + f->total_transferred += ret;
> }
> /* We expect the QEMUFile write impl to send the full
> * data set we requested, so sanity check that.
> @@ -357,11 +358,11 @@ static ssize_t qemu_fill_buffer(QEMUFile *f)
> return 0;
> }
>
> - len = f->ops->get_buffer(f->opaque, f->buf + pending, f->pos,
> + len = f->ops->get_buffer(f->opaque, f->buf + pending, f->total_transferred,
> IO_BUF_SIZE - pending, &local_error);
> if (len > 0) {
> f->buf_size += len;
> - f->pos += len;
> + f->total_transferred += len;
> } else if (len == 0) {
> qemu_file_set_error_obj(f, -EIO, local_error);
> } else if (len != -EAGAIN) {
> @@ -375,7 +376,7 @@ static ssize_t qemu_fill_buffer(QEMUFile *f)
>
> void qemu_update_position(QEMUFile *f, size_t size)
> {
> - f->pos += size;
> + f->total_transferred += size;
> }
>
> /** Closes the file
> @@ -658,7 +659,7 @@ int qemu_get_byte(QEMUFile *f)
>
> int64_t qemu_ftell_fast(QEMUFile *f)
> {
> - int64_t ret = f->pos;
> + int64_t ret = f->total_transferred;
> int i;
>
> for (i = 0; i < f->iovcnt; i++) {
> @@ -671,7 +672,7 @@ int64_t qemu_ftell_fast(QEMUFile *f)
> int64_t qemu_ftell(QEMUFile *f)
> {
> qemu_fflush(f);
> - return f->pos;
> + return f->total_transferred;
> }
>
> int qemu_file_rate_limit(QEMUFile *f)
> --
> 2.36.1
>
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
^ permalink raw reply [flat|nested] 51+ messages in thread* Re: [PATCH 05/20] migration: rename 'pos' field in QEMUFile to 'bytes_processed'
2022-06-09 9:51 ` Dr. David Alan Gilbert
@ 2022-06-09 9:57 ` Daniel P. Berrangé
2022-06-09 9:59 ` Dr. David Alan Gilbert
0 siblings, 1 reply; 51+ messages in thread
From: Daniel P. Berrangé @ 2022-06-09 9:57 UTC (permalink / raw)
To: Dr. David Alan Gilbert
Cc: qemu-devel, qemu-block, Stefan Hajnoczi, Hailiang Zhang,
Juan Quintela, Fam Zheng
On Thu, Jun 09, 2022 at 10:51:27AM +0100, Dr. David Alan Gilbert wrote:
> * Daniel P. Berrangé (berrange@redhat.com) wrote:
> > This makes the field name align with the newly introduced method
> > names in the previous commit.
>
> I think that's the method in the following commits?
Opps, yeah, I did re-arrange this series a few times to get the
most attractive diffs.
> tbh I'm not sure about this; 'pos' is still passed to writev_buffer
> and get_buffer to say where the data is - and that makes it a 'pos'
> still rather than a simple stats counter.
Note every QIOChannel backed impl of QEMUFile is ignoring the
'pos' field.
The only QEMUFile impl using 'pos' is the one during I/O is
the block device vmstate. A later patch is introducing a
QIOChannel impl for the vmstate, and to handle this it is
tracking a file offset itself internally to the QIOChannel
impl. So when we eliminate the QEMUFileOps callbacks later,
this 'pos' goes away.
So I guess my description here is a little ahead of itself.
> > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> > ---
> > migration/qemu-file.c | 19 ++++++++++---------
> > 1 file changed, 10 insertions(+), 9 deletions(-)
> >
> > diff --git a/migration/qemu-file.c b/migration/qemu-file.c
> > index 03f0b13a55..b21da4c5bf 100644
> > --- a/migration/qemu-file.c
> > +++ b/migration/qemu-file.c
> > @@ -50,8 +50,9 @@ struct QEMUFile {
> > */
> > int64_t rate_limit_used;
> >
> > - int64_t pos; /* start of buffer when writing, end of buffer
> > - when reading */
> > + /* The sum of bytes transferred on the wire */
> > + int64_t total_transferred;
> > +
> > int buf_index;
> > int buf_size; /* 0 when writing */
> > uint8_t buf[IO_BUF_SIZE];
> > @@ -241,14 +242,14 @@ void qemu_fflush(QEMUFile *f)
> > }
> > if (f->iovcnt > 0) {
> > expect = iov_size(f->iov, f->iovcnt);
> > - ret = f->ops->writev_buffer(f->opaque, f->iov, f->iovcnt, f->pos,
> > + ret = f->ops->writev_buffer(f->opaque, f->iov, f->iovcnt, f->total_transferred,
> > &local_error);
> >
> > qemu_iovec_release_ram(f);
> > }
> >
> > if (ret >= 0) {
> > - f->pos += ret;
> > + f->total_transferred += ret;
> > }
> > /* We expect the QEMUFile write impl to send the full
> > * data set we requested, so sanity check that.
> > @@ -357,11 +358,11 @@ static ssize_t qemu_fill_buffer(QEMUFile *f)
> > return 0;
> > }
> >
> > - len = f->ops->get_buffer(f->opaque, f->buf + pending, f->pos,
> > + len = f->ops->get_buffer(f->opaque, f->buf + pending, f->total_transferred,
> > IO_BUF_SIZE - pending, &local_error);
> > if (len > 0) {
> > f->buf_size += len;
> > - f->pos += len;
> > + f->total_transferred += len;
> > } else if (len == 0) {
> > qemu_file_set_error_obj(f, -EIO, local_error);
> > } else if (len != -EAGAIN) {
> > @@ -375,7 +376,7 @@ static ssize_t qemu_fill_buffer(QEMUFile *f)
> >
> > void qemu_update_position(QEMUFile *f, size_t size)
> > {
> > - f->pos += size;
> > + f->total_transferred += size;
> > }
> >
> > /** Closes the file
> > @@ -658,7 +659,7 @@ int qemu_get_byte(QEMUFile *f)
> >
> > int64_t qemu_ftell_fast(QEMUFile *f)
> > {
> > - int64_t ret = f->pos;
> > + int64_t ret = f->total_transferred;
> > int i;
> >
> > for (i = 0; i < f->iovcnt; i++) {
> > @@ -671,7 +672,7 @@ int64_t qemu_ftell_fast(QEMUFile *f)
> > int64_t qemu_ftell(QEMUFile *f)
> > {
> > qemu_fflush(f);
> > - return f->pos;
> > + return f->total_transferred;
> > }
> >
> > int qemu_file_rate_limit(QEMUFile *f)
> > --
> > 2.36.1
> >
> --
> Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
>
With regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
^ permalink raw reply [flat|nested] 51+ messages in thread* Re: [PATCH 05/20] migration: rename 'pos' field in QEMUFile to 'bytes_processed'
2022-06-09 9:57 ` Daniel P. Berrangé
@ 2022-06-09 9:59 ` Dr. David Alan Gilbert
0 siblings, 0 replies; 51+ messages in thread
From: Dr. David Alan Gilbert @ 2022-06-09 9:59 UTC (permalink / raw)
To: Daniel P. Berrangé
Cc: qemu-devel, qemu-block, Stefan Hajnoczi, Hailiang Zhang,
Juan Quintela, Fam Zheng
* Daniel P. Berrangé (berrange@redhat.com) wrote:
> On Thu, Jun 09, 2022 at 10:51:27AM +0100, Dr. David Alan Gilbert wrote:
> > * Daniel P. Berrangé (berrange@redhat.com) wrote:
> > > This makes the field name align with the newly introduced method
> > > names in the previous commit.
> >
> > I think that's the method in the following commits?
>
> Opps, yeah, I did re-arrange this series a few times to get the
> most attractive diffs.
>
> > tbh I'm not sure about this; 'pos' is still passed to writev_buffer
> > and get_buffer to say where the data is - and that makes it a 'pos'
> > still rather than a simple stats counter.
>
> Note every QIOChannel backed impl of QEMUFile is ignoring the
> 'pos' field.
>
> The only QEMUFile impl using 'pos' is the one during I/O is
> the block device vmstate. A later patch is introducing a
> QIOChannel impl for the vmstate, and to handle this it is
> tracking a file offset itself internally to the QIOChannel
> impl. So when we eliminate the QEMUFileOps callbacks later,
> this 'pos' goes away.
Ah! Put that description in the commit message and:
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> So I guess my description here is a little ahead of itself.
>
>
> > > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> > > ---
> > > migration/qemu-file.c | 19 ++++++++++---------
> > > 1 file changed, 10 insertions(+), 9 deletions(-)
> > >
> > > diff --git a/migration/qemu-file.c b/migration/qemu-file.c
> > > index 03f0b13a55..b21da4c5bf 100644
> > > --- a/migration/qemu-file.c
> > > +++ b/migration/qemu-file.c
> > > @@ -50,8 +50,9 @@ struct QEMUFile {
> > > */
> > > int64_t rate_limit_used;
> > >
> > > - int64_t pos; /* start of buffer when writing, end of buffer
> > > - when reading */
> > > + /* The sum of bytes transferred on the wire */
> > > + int64_t total_transferred;
> > > +
> > > int buf_index;
> > > int buf_size; /* 0 when writing */
> > > uint8_t buf[IO_BUF_SIZE];
> > > @@ -241,14 +242,14 @@ void qemu_fflush(QEMUFile *f)
> > > }
> > > if (f->iovcnt > 0) {
> > > expect = iov_size(f->iov, f->iovcnt);
> > > - ret = f->ops->writev_buffer(f->opaque, f->iov, f->iovcnt, f->pos,
> > > + ret = f->ops->writev_buffer(f->opaque, f->iov, f->iovcnt, f->total_transferred,
> > > &local_error);
> > >
> > > qemu_iovec_release_ram(f);
> > > }
> > >
> > > if (ret >= 0) {
> > > - f->pos += ret;
> > > + f->total_transferred += ret;
> > > }
> > > /* We expect the QEMUFile write impl to send the full
> > > * data set we requested, so sanity check that.
> > > @@ -357,11 +358,11 @@ static ssize_t qemu_fill_buffer(QEMUFile *f)
> > > return 0;
> > > }
> > >
> > > - len = f->ops->get_buffer(f->opaque, f->buf + pending, f->pos,
> > > + len = f->ops->get_buffer(f->opaque, f->buf + pending, f->total_transferred,
> > > IO_BUF_SIZE - pending, &local_error);
> > > if (len > 0) {
> > > f->buf_size += len;
> > > - f->pos += len;
> > > + f->total_transferred += len;
> > > } else if (len == 0) {
> > > qemu_file_set_error_obj(f, -EIO, local_error);
> > > } else if (len != -EAGAIN) {
> > > @@ -375,7 +376,7 @@ static ssize_t qemu_fill_buffer(QEMUFile *f)
> > >
> > > void qemu_update_position(QEMUFile *f, size_t size)
> > > {
> > > - f->pos += size;
> > > + f->total_transferred += size;
> > > }
> > >
> > > /** Closes the file
> > > @@ -658,7 +659,7 @@ int qemu_get_byte(QEMUFile *f)
> > >
> > > int64_t qemu_ftell_fast(QEMUFile *f)
> > > {
> > > - int64_t ret = f->pos;
> > > + int64_t ret = f->total_transferred;
> > > int i;
> > >
> > > for (i = 0; i < f->iovcnt; i++) {
> > > @@ -671,7 +672,7 @@ int64_t qemu_ftell_fast(QEMUFile *f)
> > > int64_t qemu_ftell(QEMUFile *f)
> > > {
> > > qemu_fflush(f);
> > > - return f->pos;
> > > + return f->total_transferred;
> > > }
> > >
> > > int qemu_file_rate_limit(QEMUFile *f)
> > > --
> > > 2.36.1
> > >
> > --
> > Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
> >
>
> With regards,
> Daniel
> --
> |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
> |: https://libvirt.org -o- https://fstop138.berrange.com :|
> |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
>
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
^ permalink raw reply [flat|nested] 51+ messages in thread
* [PATCH 06/20] migration: rename qemu_ftell to qemu_file_total_transferred
2022-05-24 11:02 [PATCH 00/20] migration: remove QEMUFileOps concept and assume use of QIOChannel Daniel P. Berrangé
` (4 preceding siblings ...)
2022-05-24 11:02 ` [PATCH 05/20] migration: rename 'pos' field in QEMUFile to 'bytes_processed' Daniel P. Berrangé
@ 2022-05-24 11:02 ` Daniel P. Berrangé
2022-06-09 10:23 ` Dr. David Alan Gilbert
2022-05-24 11:02 ` [PATCH 07/20] migration: rename qemu_update_position to qemu_file_credit_transfer Daniel P. Berrangé
` (13 subsequent siblings)
19 siblings, 1 reply; 51+ messages in thread
From: Daniel P. Berrangé @ 2022-05-24 11:02 UTC (permalink / raw)
To: qemu-devel
Cc: Daniel P. Berrangé,
Dr. David Alan Gilbert, qemu-block, Stefan Hajnoczi,
Hailiang Zhang, Juan Quintela, Fam Zheng
The name 'ftell' gives the misleading impression that the QEMUFile
objects are seekable. This is not the case, as in general we just
have an opaque stream. The users of this method are only interested
in the total bytes processed. This switches to a new name that
reflects the intended usage.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
migration/block.c | 10 +++++-----
migration/migration.c | 2 +-
migration/qemu-file.c | 4 ++--
migration/qemu-file.h | 40 ++++++++++++++++++++++++++++++++++++++--
migration/savevm.c | 6 +++---
migration/vmstate.c | 4 ++--
6 files changed, 51 insertions(+), 15 deletions(-)
diff --git a/migration/block.c b/migration/block.c
index 077a413325..823453c977 100644
--- a/migration/block.c
+++ b/migration/block.c
@@ -756,8 +756,8 @@ static int block_save_setup(QEMUFile *f, void *opaque)
static int block_save_iterate(QEMUFile *f, void *opaque)
{
int ret;
- int64_t last_ftell = qemu_ftell(f);
- int64_t delta_ftell;
+ int64_t last_bytes = qemu_file_total_transferred(f);
+ int64_t delta_bytes;
trace_migration_block_save("iterate", block_mig_state.submitted,
block_mig_state.transferred);
@@ -809,10 +809,10 @@ static int block_save_iterate(QEMUFile *f, void *opaque)
}
qemu_put_be64(f, BLK_MIG_FLAG_EOS);
- delta_ftell = qemu_ftell(f) - last_ftell;
- if (delta_ftell > 0) {
+ delta_bytes = qemu_file_total_transferred(f) - last_bytes;
+ if (delta_bytes > 0) {
return 1;
- } else if (delta_ftell < 0) {
+ } else if (delta_bytes < 0) {
return -1;
} else {
return 0;
diff --git a/migration/migration.c b/migration/migration.c
index 31739b2af9..ab1e9610ef 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -3544,7 +3544,7 @@ static MigThrError migration_detect_error(MigrationState *s)
/* How many bytes have we transferred since the beginning of the migration */
static uint64_t migration_total_bytes(MigrationState *s)
{
- return qemu_ftell(s->to_dst_file) + ram_counters.multifd_bytes;
+ return qemu_file_total_transferred(s->to_dst_file) + ram_counters.multifd_bytes;
}
static void migration_calculate_complete(MigrationState *s)
diff --git a/migration/qemu-file.c b/migration/qemu-file.c
index b21da4c5bf..664ac77067 100644
--- a/migration/qemu-file.c
+++ b/migration/qemu-file.c
@@ -657,7 +657,7 @@ int qemu_get_byte(QEMUFile *f)
return result;
}
-int64_t qemu_ftell_fast(QEMUFile *f)
+int64_t qemu_file_total_transferred_fast(QEMUFile *f)
{
int64_t ret = f->total_transferred;
int i;
@@ -669,7 +669,7 @@ int64_t qemu_ftell_fast(QEMUFile *f)
return ret;
}
-int64_t qemu_ftell(QEMUFile *f)
+int64_t qemu_file_total_transferred(QEMUFile *f)
{
qemu_fflush(f);
return f->total_transferred;
diff --git a/migration/qemu-file.h b/migration/qemu-file.h
index 3f36d4dc8c..febc961aa9 100644
--- a/migration/qemu-file.h
+++ b/migration/qemu-file.h
@@ -124,8 +124,44 @@ QEMUFile *qemu_fopen_ops(void *opaque, const QEMUFileOps *ops, bool has_ioc);
void qemu_file_set_hooks(QEMUFile *f, const QEMUFileHooks *hooks);
int qemu_get_fd(QEMUFile *f);
int qemu_fclose(QEMUFile *f);
-int64_t qemu_ftell(QEMUFile *f);
-int64_t qemu_ftell_fast(QEMUFile *f);
+
+/*
+ * qemu_file_total_transferred:
+ *
+ * Report the total number of bytes transferred with
+ * this file.
+ *
+ * For writable files, any pending buffers will be
+ * flushed, so the reported value will be equal to
+ * the number of bytes transferred on the wire.
+ *
+ * For readable files, the reported value will be
+ * equal to the number of bytes transferred on the
+ * wire.
+ *
+ * Returns: the total bytes transferred
+ */
+int64_t qemu_file_total_transferred(QEMUFile *f);
+
+/*
+ * qemu_file_total_transferred_fast:
+ *
+ * Report the total number of bytes transferred with
+ * this file.
+ *
+ * For writable files, no pending buffers will be
+ * flushed, but the reported value will include the
+ * size of any queued buffers, on top of the amount
+ * actually transferred.
+ *
+ * For readable files, the reported value will be
+ * equal to the number of bytes transferred on the
+ * wire.
+ *
+ * Returns: the total bytes transferred and queued
+ */
+int64_t qemu_file_total_transferred_fast(QEMUFile *f);
+
/*
* put_buffer without copying the buffer.
* The buffer should be available till it is sent asynchronously.
diff --git a/migration/savevm.c b/migration/savevm.c
index d9076897b8..75d05f1a84 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -916,9 +916,9 @@ static void vmstate_save_old_style(QEMUFile *f, SaveStateEntry *se,
{
int64_t old_offset, size;
- old_offset = qemu_ftell_fast(f);
+ old_offset = qemu_file_total_transferred_fast(f);
se->ops->save_state(f, se->opaque);
- size = qemu_ftell_fast(f) - old_offset;
+ size = qemu_file_total_transferred_fast(f) - old_offset;
if (vmdesc) {
json_writer_int64(vmdesc, "size", size);
@@ -2887,7 +2887,7 @@ bool save_snapshot(const char *name, bool overwrite, const char *vmstate,
goto the_end;
}
ret = qemu_savevm_state(f, errp);
- vm_state_size = qemu_ftell(f);
+ vm_state_size = qemu_file_total_transferred(f);
ret2 = qemu_fclose(f);
if (ret < 0) {
goto the_end;
diff --git a/migration/vmstate.c b/migration/vmstate.c
index 36ae8b9e19..b0551e82c6 100644
--- a/migration/vmstate.c
+++ b/migration/vmstate.c
@@ -360,7 +360,7 @@ int vmstate_save_state_v(QEMUFile *f, const VMStateDescription *vmsd,
void *curr_elem = first_elem + size * i;
vmsd_desc_field_start(vmsd, vmdesc_loop, field, i, n_elems);
- old_offset = qemu_ftell_fast(f);
+ old_offset = qemu_file_total_transferred_fast(f);
if (field->flags & VMS_ARRAY_OF_POINTER) {
assert(curr_elem);
curr_elem = *(void **)curr_elem;
@@ -390,7 +390,7 @@ int vmstate_save_state_v(QEMUFile *f, const VMStateDescription *vmsd,
return ret;
}
- written_bytes = qemu_ftell_fast(f) - old_offset;
+ written_bytes = qemu_file_total_transferred_fast(f) - old_offset;
vmsd_desc_field_end(vmsd, vmdesc_loop, field, written_bytes, i);
/* Compressed arrays only care about the first element */
--
2.36.1
^ permalink raw reply related [flat|nested] 51+ messages in thread* Re: [PATCH 06/20] migration: rename qemu_ftell to qemu_file_total_transferred
2022-05-24 11:02 ` [PATCH 06/20] migration: rename qemu_ftell to qemu_file_total_transferred Daniel P. Berrangé
@ 2022-06-09 10:23 ` Dr. David Alan Gilbert
0 siblings, 0 replies; 51+ messages in thread
From: Dr. David Alan Gilbert @ 2022-06-09 10:23 UTC (permalink / raw)
To: Daniel P. Berrangé
Cc: qemu-devel, qemu-block, Stefan Hajnoczi, Hailiang Zhang,
Juan Quintela, Fam Zheng
* Daniel P. Berrangé (berrange@redhat.com) wrote:
> The name 'ftell' gives the misleading impression that the QEMUFile
> objects are seekable. This is not the case, as in general we just
> have an opaque stream. The users of this method are only interested
> in the total bytes processed. This switches to a new name that
> reflects the intended usage.
>
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Mostly,
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Two nits below:
> ---
> migration/block.c | 10 +++++-----
> migration/migration.c | 2 +-
> migration/qemu-file.c | 4 ++--
> migration/qemu-file.h | 40 ++++++++++++++++++++++++++++++++++++++--
> migration/savevm.c | 6 +++---
> migration/vmstate.c | 4 ++--
> 6 files changed, 51 insertions(+), 15 deletions(-)
>
> diff --git a/migration/block.c b/migration/block.c
> index 077a413325..823453c977 100644
> --- a/migration/block.c
> +++ b/migration/block.c
> @@ -756,8 +756,8 @@ static int block_save_setup(QEMUFile *f, void *opaque)
> static int block_save_iterate(QEMUFile *f, void *opaque)
> {
> int ret;
> - int64_t last_ftell = qemu_ftell(f);
> - int64_t delta_ftell;
> + int64_t last_bytes = qemu_file_total_transferred(f);
> + int64_t delta_bytes;
>
> trace_migration_block_save("iterate", block_mig_state.submitted,
> block_mig_state.transferred);
> @@ -809,10 +809,10 @@ static int block_save_iterate(QEMUFile *f, void *opaque)
> }
>
> qemu_put_be64(f, BLK_MIG_FLAG_EOS);
> - delta_ftell = qemu_ftell(f) - last_ftell;
> - if (delta_ftell > 0) {
> + delta_bytes = qemu_file_total_transferred(f) - last_bytes;
> + if (delta_bytes > 0) {
> return 1;
> - } else if (delta_ftell < 0) {
> + } else if (delta_bytes < 0) {
> return -1;
> } else {
> return 0;
> diff --git a/migration/migration.c b/migration/migration.c
> index 31739b2af9..ab1e9610ef 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -3544,7 +3544,7 @@ static MigThrError migration_detect_error(MigrationState *s)
> /* How many bytes have we transferred since the beginning of the migration */
> static uint64_t migration_total_bytes(MigrationState *s)
> {
> - return qemu_ftell(s->to_dst_file) + ram_counters.multifd_bytes;
> + return qemu_file_total_transferred(s->to_dst_file) + ram_counters.multifd_bytes;
I think that's hit line length limits.
> }
>
> static void migration_calculate_complete(MigrationState *s)
> diff --git a/migration/qemu-file.c b/migration/qemu-file.c
> index b21da4c5bf..664ac77067 100644
> --- a/migration/qemu-file.c
> +++ b/migration/qemu-file.c
> @@ -657,7 +657,7 @@ int qemu_get_byte(QEMUFile *f)
> return result;
> }
>
> -int64_t qemu_ftell_fast(QEMUFile *f)
> +int64_t qemu_file_total_transferred_fast(QEMUFile *f)
> {
> int64_t ret = f->total_transferred;
> int i;
> @@ -669,7 +669,7 @@ int64_t qemu_ftell_fast(QEMUFile *f)
> return ret;
> }
>
> -int64_t qemu_ftell(QEMUFile *f)
> +int64_t qemu_file_total_transferred(QEMUFile *f)
> {
> qemu_fflush(f);
> return f->total_transferred;
> diff --git a/migration/qemu-file.h b/migration/qemu-file.h
> index 3f36d4dc8c..febc961aa9 100644
> --- a/migration/qemu-file.h
> +++ b/migration/qemu-file.h
> @@ -124,8 +124,44 @@ QEMUFile *qemu_fopen_ops(void *opaque, const QEMUFileOps *ops, bool has_ioc);
> void qemu_file_set_hooks(QEMUFile *f, const QEMUFileHooks *hooks);
> int qemu_get_fd(QEMUFile *f);
> int qemu_fclose(QEMUFile *f);
> -int64_t qemu_ftell(QEMUFile *f);
> -int64_t qemu_ftell_fast(QEMUFile *f);
> +
> +/*
> + * qemu_file_total_transferred:
> + *
> + * Report the total number of bytes transferred with
> + * this file.
> + *
> + * For writable files, any pending buffers will be
> + * flushed, so the reported value will be equal to
> + * the number of bytes transferred on the wire.
> + *
> + * For readable files, the reported value will be
> + * equal to the number of bytes transferred on the
> + * wire.
> + *
> + * Returns: the total bytes transferred
> + */
> +int64_t qemu_file_total_transferred(QEMUFile *f);
> +
> +/*
> + * qemu_file_total_transferred_fast:
> + *
> + * Report the total number of bytes transferred with
> + * this file.
> + *
> + * For writable files, no pending buffers will be
> + * flushed, but the reported value will include the
> + * size of any queued buffers, on top of the amount
> + * actually transferred.
> + *
> + * For readable files, the reported value will be
> + * equal to the number of bytes transferred on the
> + * wire.
> + *
> + * Returns: the total bytes transferred and queued
I think this would be clearer if it just said:
* qemu_file_total_transferred_fast:
As qemu_file_total_transferred except for writable
files, where no flush is performed and the reported
amount will include the size of any queued buffers,
on top of the amount actually transferred.
Otherwise you're left eyeballing the two to try and spot
the difference.
Dave
> + */
> +int64_t qemu_file_total_transferred_fast(QEMUFile *f);
> +
> /*
> * put_buffer without copying the buffer.
> * The buffer should be available till it is sent asynchronously.
> diff --git a/migration/savevm.c b/migration/savevm.c
> index d9076897b8..75d05f1a84 100644
> --- a/migration/savevm.c
> +++ b/migration/savevm.c
> @@ -916,9 +916,9 @@ static void vmstate_save_old_style(QEMUFile *f, SaveStateEntry *se,
> {
> int64_t old_offset, size;
>
> - old_offset = qemu_ftell_fast(f);
> + old_offset = qemu_file_total_transferred_fast(f);
> se->ops->save_state(f, se->opaque);
> - size = qemu_ftell_fast(f) - old_offset;
> + size = qemu_file_total_transferred_fast(f) - old_offset;
>
> if (vmdesc) {
> json_writer_int64(vmdesc, "size", size);
> @@ -2887,7 +2887,7 @@ bool save_snapshot(const char *name, bool overwrite, const char *vmstate,
> goto the_end;
> }
> ret = qemu_savevm_state(f, errp);
> - vm_state_size = qemu_ftell(f);
> + vm_state_size = qemu_file_total_transferred(f);
> ret2 = qemu_fclose(f);
> if (ret < 0) {
> goto the_end;
> diff --git a/migration/vmstate.c b/migration/vmstate.c
> index 36ae8b9e19..b0551e82c6 100644
> --- a/migration/vmstate.c
> +++ b/migration/vmstate.c
> @@ -360,7 +360,7 @@ int vmstate_save_state_v(QEMUFile *f, const VMStateDescription *vmsd,
> void *curr_elem = first_elem + size * i;
>
> vmsd_desc_field_start(vmsd, vmdesc_loop, field, i, n_elems);
> - old_offset = qemu_ftell_fast(f);
> + old_offset = qemu_file_total_transferred_fast(f);
> if (field->flags & VMS_ARRAY_OF_POINTER) {
> assert(curr_elem);
> curr_elem = *(void **)curr_elem;
> @@ -390,7 +390,7 @@ int vmstate_save_state_v(QEMUFile *f, const VMStateDescription *vmsd,
> return ret;
> }
>
> - written_bytes = qemu_ftell_fast(f) - old_offset;
> + written_bytes = qemu_file_total_transferred_fast(f) - old_offset;
> vmsd_desc_field_end(vmsd, vmdesc_loop, field, written_bytes, i);
>
> /* Compressed arrays only care about the first element */
> --
> 2.36.1
>
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
^ permalink raw reply [flat|nested] 51+ messages in thread
* [PATCH 07/20] migration: rename qemu_update_position to qemu_file_credit_transfer
2022-05-24 11:02 [PATCH 00/20] migration: remove QEMUFileOps concept and assume use of QIOChannel Daniel P. Berrangé
` (5 preceding siblings ...)
2022-05-24 11:02 ` [PATCH 06/20] migration: rename qemu_ftell to qemu_file_total_transferred Daniel P. Berrangé
@ 2022-05-24 11:02 ` Daniel P. Berrangé
2022-06-09 10:29 ` Dr. David Alan Gilbert
2022-06-09 12:56 ` Peter Maydell
2022-05-24 11:02 ` [PATCH 08/20] migration: introduce a QIOChannel impl for BlockDriverState VMState Daniel P. Berrangé
` (12 subsequent siblings)
19 siblings, 2 replies; 51+ messages in thread
From: Daniel P. Berrangé @ 2022-05-24 11:02 UTC (permalink / raw)
To: qemu-devel
Cc: Daniel P. Berrangé,
Dr. David Alan Gilbert, qemu-block, Stefan Hajnoczi,
Hailiang Zhang, Juan Quintela, Fam Zheng
The qemu_update_position method name gives the misleading impression
that it is changing the current file offset. Most of the files are
just streams, however, so there's no concept of a file offset in the
general case.
What this method is actually used for is to report on the number of
bytes that have been transferred out of band from the main I/O methods.
This new name better reflects this purpose.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
migration/qemu-file.c | 4 ++--
migration/qemu-file.h | 8 +++++++-
migration/ram.c | 2 +-
3 files changed, 10 insertions(+), 4 deletions(-)
diff --git a/migration/qemu-file.c b/migration/qemu-file.c
index 664ac77067..9a7f715e17 100644
--- a/migration/qemu-file.c
+++ b/migration/qemu-file.c
@@ -319,7 +319,7 @@ size_t ram_control_save_page(QEMUFile *f, ram_addr_t block_offset,
if (ret != RAM_SAVE_CONTROL_DELAYED &&
ret != RAM_SAVE_CONTROL_NOT_SUPP) {
if (bytes_sent && *bytes_sent > 0) {
- qemu_update_position(f, *bytes_sent);
+ qemu_file_credit_transfer(f, *bytes_sent);
} else if (ret < 0) {
qemu_file_set_error(f, ret);
}
@@ -374,7 +374,7 @@ static ssize_t qemu_fill_buffer(QEMUFile *f)
return len;
}
-void qemu_update_position(QEMUFile *f, size_t size)
+void qemu_file_credit_transfer(QEMUFile *f, size_t size)
{
f->total_transferred += size;
}
diff --git a/migration/qemu-file.h b/migration/qemu-file.h
index febc961aa9..81f6fd7db8 100644
--- a/migration/qemu-file.h
+++ b/migration/qemu-file.h
@@ -186,7 +186,13 @@ int qemu_put_qemu_file(QEMUFile *f_des, QEMUFile *f_src);
*/
int qemu_peek_byte(QEMUFile *f, int offset);
void qemu_file_skip(QEMUFile *f, int size);
-void qemu_update_position(QEMUFile *f, size_t size);
+/*
+ * qemu_file_credit_transfer:
+ *
+ * Report on a number of bytes that have been transferred
+ * out of band from the main file object I/O methods.
+ */
+void qemu_file_credit_transfer(QEMUFile *f, size_t size);
void qemu_file_reset_rate_limit(QEMUFile *f);
void qemu_file_update_transfer(QEMUFile *f, int64_t len);
void qemu_file_set_rate_limit(QEMUFile *f, int64_t new_rate);
diff --git a/migration/ram.c b/migration/ram.c
index 89082716d6..bf321e1e72 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -2301,7 +2301,7 @@ void acct_update_position(QEMUFile *f, size_t size, bool zero)
} else {
ram_counters.normal += pages;
ram_transferred_add(size);
- qemu_update_position(f, size);
+ qemu_file_credit_transfer(f, size);
}
}
--
2.36.1
^ permalink raw reply related [flat|nested] 51+ messages in thread* Re: [PATCH 07/20] migration: rename qemu_update_position to qemu_file_credit_transfer
2022-05-24 11:02 ` [PATCH 07/20] migration: rename qemu_update_position to qemu_file_credit_transfer Daniel P. Berrangé
@ 2022-06-09 10:29 ` Dr. David Alan Gilbert
2022-06-09 12:56 ` Peter Maydell
1 sibling, 0 replies; 51+ messages in thread
From: Dr. David Alan Gilbert @ 2022-06-09 10:29 UTC (permalink / raw)
To: Daniel P. Berrangé
Cc: qemu-devel, qemu-block, Stefan Hajnoczi, Hailiang Zhang,
Juan Quintela, Fam Zheng
* Daniel P. Berrangé (berrange@redhat.com) wrote:
> The qemu_update_position method name gives the misleading impression
> that it is changing the current file offset. Most of the files are
> just streams, however, so there's no concept of a file offset in the
> general case.
>
> What this method is actually used for is to report on the number of
> bytes that have been transferred out of band from the main I/O methods.
> This new name better reflects this purpose.
>
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> ---
> migration/qemu-file.c | 4 ++--
> migration/qemu-file.h | 8 +++++++-
> migration/ram.c | 2 +-
> 3 files changed, 10 insertions(+), 4 deletions(-)
>
> diff --git a/migration/qemu-file.c b/migration/qemu-file.c
> index 664ac77067..9a7f715e17 100644
> --- a/migration/qemu-file.c
> +++ b/migration/qemu-file.c
> @@ -319,7 +319,7 @@ size_t ram_control_save_page(QEMUFile *f, ram_addr_t block_offset,
> if (ret != RAM_SAVE_CONTROL_DELAYED &&
> ret != RAM_SAVE_CONTROL_NOT_SUPP) {
> if (bytes_sent && *bytes_sent > 0) {
> - qemu_update_position(f, *bytes_sent);
> + qemu_file_credit_transfer(f, *bytes_sent);
> } else if (ret < 0) {
> qemu_file_set_error(f, ret);
> }
> @@ -374,7 +374,7 @@ static ssize_t qemu_fill_buffer(QEMUFile *f)
> return len;
> }
>
> -void qemu_update_position(QEMUFile *f, size_t size)
> +void qemu_file_credit_transfer(QEMUFile *f, size_t size)
> {
> f->total_transferred += size;
> }
> diff --git a/migration/qemu-file.h b/migration/qemu-file.h
> index febc961aa9..81f6fd7db8 100644
> --- a/migration/qemu-file.h
> +++ b/migration/qemu-file.h
> @@ -186,7 +186,13 @@ int qemu_put_qemu_file(QEMUFile *f_des, QEMUFile *f_src);
> */
> int qemu_peek_byte(QEMUFile *f, int offset);
> void qemu_file_skip(QEMUFile *f, int size);
> -void qemu_update_position(QEMUFile *f, size_t size);
> +/*
> + * qemu_file_credit_transfer:
> + *
> + * Report on a number of bytes that have been transferred
> + * out of band from the main file object I/O methods.
> + */
> +void qemu_file_credit_transfer(QEMUFile *f, size_t size);
> void qemu_file_reset_rate_limit(QEMUFile *f);
> void qemu_file_update_transfer(QEMUFile *f, int64_t len);
> void qemu_file_set_rate_limit(QEMUFile *f, int64_t new_rate);
> diff --git a/migration/ram.c b/migration/ram.c
> index 89082716d6..bf321e1e72 100644
> --- a/migration/ram.c
> +++ b/migration/ram.c
> @@ -2301,7 +2301,7 @@ void acct_update_position(QEMUFile *f, size_t size, bool zero)
> } else {
> ram_counters.normal += pages;
> ram_transferred_add(size);
> - qemu_update_position(f, size);
> + qemu_file_credit_transfer(f, size);
> }
> }
>
> --
> 2.36.1
>
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
^ permalink raw reply [flat|nested] 51+ messages in thread* Re: [PATCH 07/20] migration: rename qemu_update_position to qemu_file_credit_transfer
2022-05-24 11:02 ` [PATCH 07/20] migration: rename qemu_update_position to qemu_file_credit_transfer Daniel P. Berrangé
2022-06-09 10:29 ` Dr. David Alan Gilbert
@ 2022-06-09 12:56 ` Peter Maydell
2022-06-09 13:02 ` Daniel P. Berrangé
1 sibling, 1 reply; 51+ messages in thread
From: Peter Maydell @ 2022-06-09 12:56 UTC (permalink / raw)
To: Daniel P. Berrangé
Cc: qemu-devel, Dr. David Alan Gilbert, qemu-block, Stefan Hajnoczi,
Hailiang Zhang, Juan Quintela, Fam Zheng
On Tue, 24 May 2022 at 12:46, Daniel P. Berrangé <berrange@redhat.com> wrote:
>
> The qemu_update_position method name gives the misleading impression
> that it is changing the current file offset. Most of the files are
> just streams, however, so there's no concept of a file offset in the
> general case.
>
> What this method is actually used for is to report on the number of
> bytes that have been transferred out of band from the main I/O methods.
> This new name better reflects this purpose.
>
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> int qemu_peek_byte(QEMUFile *f, int offset);
> void qemu_file_skip(QEMUFile *f, int size);
> -void qemu_update_position(QEMUFile *f, size_t size);
> +/*
> + * qemu_file_credit_transfer:
> + *
> + * Report on a number of bytes that have been transferred
> + * out of band from the main file object I/O methods.
> + */
> +void qemu_file_credit_transfer(QEMUFile *f, size_t size);
> void qemu_file_reset_rate_limit(QEMUFile *f);
> void qemu_file_update_transfer(QEMUFile *f, int64_t len);
> void qemu_file_set_rate_limit(QEMUFile *f, int64_t new_rate);
What's the difference between "credit transfer" and "update
transfer" ? The latter also seems to just be adding a number
to a count of bytes-transferred...
-- PMM
^ permalink raw reply [flat|nested] 51+ messages in thread
* Re: [PATCH 07/20] migration: rename qemu_update_position to qemu_file_credit_transfer
2022-06-09 12:56 ` Peter Maydell
@ 2022-06-09 13:02 ` Daniel P. Berrangé
2022-06-09 13:15 ` Dr. David Alan Gilbert
0 siblings, 1 reply; 51+ messages in thread
From: Daniel P. Berrangé @ 2022-06-09 13:02 UTC (permalink / raw)
To: Peter Maydell
Cc: qemu-devel, Dr. David Alan Gilbert, qemu-block, Stefan Hajnoczi,
Hailiang Zhang, Juan Quintela, Fam Zheng
On Thu, Jun 09, 2022 at 01:56:00PM +0100, Peter Maydell wrote:
> On Tue, 24 May 2022 at 12:46, Daniel P. Berrangé <berrange@redhat.com> wrote:
> >
> > The qemu_update_position method name gives the misleading impression
> > that it is changing the current file offset. Most of the files are
> > just streams, however, so there's no concept of a file offset in the
> > general case.
> >
> > What this method is actually used for is to report on the number of
> > bytes that have been transferred out of band from the main I/O methods.
> > This new name better reflects this purpose.
> >
> > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
>
> > int qemu_peek_byte(QEMUFile *f, int offset);
> > void qemu_file_skip(QEMUFile *f, int size);
> > -void qemu_update_position(QEMUFile *f, size_t size);
> > +/*
> > + * qemu_file_credit_transfer:
> > + *
> > + * Report on a number of bytes that have been transferred
> > + * out of band from the main file object I/O methods.
> > + */
> > +void qemu_file_credit_transfer(QEMUFile *f, size_t size);
> > void qemu_file_reset_rate_limit(QEMUFile *f);
> > void qemu_file_update_transfer(QEMUFile *f, int64_t len);
> > void qemu_file_set_rate_limit(QEMUFile *f, int64_t new_rate);
>
> What's the difference between "credit transfer" and "update
> transfer" ? The latter also seems to just be adding a number
> to a count of bytes-transferred...
The other method is merely related to the rate limiting, and so
probably ought to have 'rate_limit' included in its name too.
With regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
^ permalink raw reply [flat|nested] 51+ messages in thread
* Re: [PATCH 07/20] migration: rename qemu_update_position to qemu_file_credit_transfer
2022-06-09 13:02 ` Daniel P. Berrangé
@ 2022-06-09 13:15 ` Dr. David Alan Gilbert
0 siblings, 0 replies; 51+ messages in thread
From: Dr. David Alan Gilbert @ 2022-06-09 13:15 UTC (permalink / raw)
To: Daniel P. Berrangé
Cc: Peter Maydell, qemu-devel, qemu-block, Stefan Hajnoczi,
Hailiang Zhang, Juan Quintela, Fam Zheng
* Daniel P. Berrangé (berrange@redhat.com) wrote:
> On Thu, Jun 09, 2022 at 01:56:00PM +0100, Peter Maydell wrote:
> > On Tue, 24 May 2022 at 12:46, Daniel P. Berrangé <berrange@redhat.com> wrote:
> > >
> > > The qemu_update_position method name gives the misleading impression
> > > that it is changing the current file offset. Most of the files are
> > > just streams, however, so there's no concept of a file offset in the
> > > general case.
> > >
> > > What this method is actually used for is to report on the number of
> > > bytes that have been transferred out of band from the main I/O methods.
> > > This new name better reflects this purpose.
> > >
> > > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> >
> > > int qemu_peek_byte(QEMUFile *f, int offset);
> > > void qemu_file_skip(QEMUFile *f, int size);
> > > -void qemu_update_position(QEMUFile *f, size_t size);
> > > +/*
> > > + * qemu_file_credit_transfer:
> > > + *
> > > + * Report on a number of bytes that have been transferred
> > > + * out of band from the main file object I/O methods.
> > > + */
> > > +void qemu_file_credit_transfer(QEMUFile *f, size_t size);
> > > void qemu_file_reset_rate_limit(QEMUFile *f);
> > > void qemu_file_update_transfer(QEMUFile *f, int64_t len);
> > > void qemu_file_set_rate_limit(QEMUFile *f, int64_t new_rate);
> >
> > What's the difference between "credit transfer" and "update
> > transfer" ? The latter also seems to just be adding a number
> > to a count of bytes-transferred...
>
> The other method is merely related to the rate limiting, and so
> probably ought to have 'rate_limit' included in its name too.
Bleh that's messy; I see update_transfer is used in the multifd case
as well, so makes sense for stats only and not a position in a stream
that only makes sense for a single fd.
(But now doesn't make any sense any more with these changes either)
Dave
> With regards,
> Daniel
> --
> |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
> |: https://libvirt.org -o- https://fstop138.berrange.com :|
> |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
>
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
^ permalink raw reply [flat|nested] 51+ messages in thread
* [PATCH 08/20] migration: introduce a QIOChannel impl for BlockDriverState VMState
2022-05-24 11:02 [PATCH 00/20] migration: remove QEMUFileOps concept and assume use of QIOChannel Daniel P. Berrangé
` (6 preceding siblings ...)
2022-05-24 11:02 ` [PATCH 07/20] migration: rename qemu_update_position to qemu_file_credit_transfer Daniel P. Berrangé
@ 2022-05-24 11:02 ` Daniel P. Berrangé
2022-05-24 11:02 ` [PATCH 09/20] migration: convert savevm to use QIOChannelBlock for VMState Daniel P. Berrangé
` (11 subsequent siblings)
19 siblings, 0 replies; 51+ messages in thread
From: Daniel P. Berrangé @ 2022-05-24 11:02 UTC (permalink / raw)
To: qemu-devel
Cc: Daniel P. Berrangé,
Dr. David Alan Gilbert, qemu-block, Stefan Hajnoczi,
Hailiang Zhang, Juan Quintela, Fam Zheng
Introduce a QIOChannelBlock class that exposes the BlockDriverState
VMState region for I/O.
This is kept in the migration/ directory rather than io/, to avoid
a mutual dependancy between block/ <-> io/ directories. Also the
VMState should only be used by the migration code.
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
---
migration/channel-block.c | 195 ++++++++++++++++++++++++++++++++++++++
migration/channel-block.h | 59 ++++++++++++
migration/meson.build | 1 +
3 files changed, 255 insertions(+)
create mode 100644 migration/channel-block.c
create mode 100644 migration/channel-block.h
diff --git a/migration/channel-block.c b/migration/channel-block.c
new file mode 100644
index 0000000000..ad52342c10
--- /dev/null
+++ b/migration/channel-block.c
@@ -0,0 +1,195 @@
+/*
+ * QEMU I/O channels block driver
+ *
+ * Copyright (c) 2022 Red Hat, Inc.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+#include "qemu/osdep.h"
+#include "migration/channel-block.h"
+#include "qapi/error.h"
+#include "block/block.h"
+#include "trace.h"
+
+QIOChannelBlock *
+qio_channel_block_new(BlockDriverState *bs)
+{
+ QIOChannelBlock *ioc;
+
+ ioc = QIO_CHANNEL_BLOCK(object_new(TYPE_QIO_CHANNEL_BLOCK));
+
+ bdrv_ref(bs);
+ ioc->bs = bs;
+
+ return ioc;
+}
+
+
+static void
+qio_channel_block_finalize(Object *obj)
+{
+ QIOChannelBlock *ioc = QIO_CHANNEL_BLOCK(obj);
+
+ g_clear_pointer(&ioc->bs, bdrv_unref);
+}
+
+
+static ssize_t
+qio_channel_block_readv(QIOChannel *ioc,
+ const struct iovec *iov,
+ size_t niov,
+ int **fds,
+ size_t *nfds,
+ Error **errp)
+{
+ QIOChannelBlock *bioc = QIO_CHANNEL_BLOCK(ioc);
+ QEMUIOVector qiov;
+ int ret;
+
+ qemu_iovec_init_external(&qiov, (struct iovec *)iov, niov);
+ ret = bdrv_readv_vmstate(bioc->bs, &qiov, bioc->offset);
+ if (ret < 0) {
+ return ret;
+ }
+
+ bioc->offset += qiov.size;
+ return qiov.size;
+}
+
+
+static ssize_t
+qio_channel_block_writev(QIOChannel *ioc,
+ const struct iovec *iov,
+ size_t niov,
+ int *fds,
+ size_t nfds,
+ int flags,
+ Error **errp)
+{
+ QIOChannelBlock *bioc = QIO_CHANNEL_BLOCK(ioc);
+ QEMUIOVector qiov;
+ int ret;
+
+ qemu_iovec_init_external(&qiov, (struct iovec *)iov, niov);
+ ret = bdrv_writev_vmstate(bioc->bs, &qiov, bioc->offset);
+ if (ret < 0) {
+ return ret;
+ }
+
+ bioc->offset += qiov.size;
+ return qiov.size;
+}
+
+
+static int
+qio_channel_block_set_blocking(QIOChannel *ioc,
+ bool enabled,
+ Error **errp)
+{
+ if (!enabled) {
+ error_setg(errp, "Non-blocking mode not supported for block devices");
+ return -1;
+ }
+ return 0;
+}
+
+
+static off_t
+qio_channel_block_seek(QIOChannel *ioc,
+ off_t offset,
+ int whence,
+ Error **errp)
+{
+ QIOChannelBlock *bioc = QIO_CHANNEL_BLOCK(ioc);
+
+ switch (whence) {
+ case SEEK_SET:
+ bioc->offset = offset;
+ break;
+ case SEEK_CUR:
+ bioc->offset += whence;
+ break;
+ case SEEK_END:
+ error_setg(errp, "Size of VMstate region is unknown");
+ return (off_t)-1;
+ default:
+ g_assert_not_reached();
+ }
+
+ return bioc->offset;
+}
+
+
+static int
+qio_channel_block_close(QIOChannel *ioc,
+ Error **errp)
+{
+ QIOChannelBlock *bioc = QIO_CHANNEL_BLOCK(ioc);
+ int rv;
+
+ if ((rv = bdrv_flush(bioc->bs)) < 0) {
+ error_setg_errno(errp, -rv,
+ "Unable to flush VMState");
+ return -1;
+ }
+
+ g_clear_pointer(&bioc->bs, bdrv_unref);
+ bioc->offset = 0;
+
+ return 0;
+}
+
+
+static void
+qio_channel_block_set_aio_fd_handler(QIOChannel *ioc,
+ AioContext *ctx,
+ IOHandler *io_read,
+ IOHandler *io_write,
+ void *opaque)
+{
+ /* XXX anything we can do here ? */
+}
+
+
+static void
+qio_channel_block_class_init(ObjectClass *klass,
+ void *class_data G_GNUC_UNUSED)
+{
+ QIOChannelClass *ioc_klass = QIO_CHANNEL_CLASS(klass);
+
+ ioc_klass->io_writev = qio_channel_block_writev;
+ ioc_klass->io_readv = qio_channel_block_readv;
+ ioc_klass->io_set_blocking = qio_channel_block_set_blocking;
+ ioc_klass->io_seek = qio_channel_block_seek;
+ ioc_klass->io_close = qio_channel_block_close;
+ ioc_klass->io_set_aio_fd_handler = qio_channel_block_set_aio_fd_handler;
+}
+
+static const TypeInfo qio_channel_block_info = {
+ .parent = TYPE_QIO_CHANNEL,
+ .name = TYPE_QIO_CHANNEL_BLOCK,
+ .instance_size = sizeof(QIOChannelBlock),
+ .instance_finalize = qio_channel_block_finalize,
+ .class_init = qio_channel_block_class_init,
+};
+
+static void
+qio_channel_block_register_types(void)
+{
+ type_register_static(&qio_channel_block_info);
+}
+
+type_init(qio_channel_block_register_types);
diff --git a/migration/channel-block.h b/migration/channel-block.h
new file mode 100644
index 0000000000..31673824e6
--- /dev/null
+++ b/migration/channel-block.h
@@ -0,0 +1,59 @@
+/*
+ * QEMU I/O channels block driver
+ *
+ * Copyright (c) 2022 Red Hat, Inc.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+#ifndef QIO_CHANNEL_BLOCK_H
+#define QIO_CHANNEL_BLOCK_H
+
+#include "io/channel.h"
+#include "qom/object.h"
+
+#define TYPE_QIO_CHANNEL_BLOCK "qio-channel-block"
+OBJECT_DECLARE_SIMPLE_TYPE(QIOChannelBlock, QIO_CHANNEL_BLOCK)
+
+
+/**
+ * QIOChannelBlock:
+ *
+ * The QIOChannelBlock object provides a channel implementation
+ * that is able to perform I/O on the BlockDriverState objects
+ * to the VMState region.
+ */
+
+struct QIOChannelBlock {
+ QIOChannel parent;
+ BlockDriverState *bs;
+ off_t offset;
+};
+
+
+/**
+ * qio_channel_block_new:
+ * @bs: the block driver state
+ *
+ * Create a new IO channel object that can perform
+ * I/O on a BlockDriverState object to the VMState
+ * region
+ *
+ * Returns: the new channel object
+ */
+QIOChannelBlock *
+qio_channel_block_new(BlockDriverState *bs);
+
+#endif /* QIO_CHANNEL_BLOCK_H */
diff --git a/migration/meson.build b/migration/meson.build
index 6880b61b10..8d309f5849 100644
--- a/migration/meson.build
+++ b/migration/meson.build
@@ -13,6 +13,7 @@ softmmu_ss.add(migration_files)
softmmu_ss.add(files(
'block-dirty-bitmap.c',
'channel.c',
+ 'channel-block.c',
'colo-failover.c',
'colo.c',
'exec.c',
--
2.36.1
^ permalink raw reply related [flat|nested] 51+ messages in thread* [PATCH 09/20] migration: convert savevm to use QIOChannelBlock for VMState
2022-05-24 11:02 [PATCH 00/20] migration: remove QEMUFileOps concept and assume use of QIOChannel Daniel P. Berrangé
` (7 preceding siblings ...)
2022-05-24 11:02 ` [PATCH 08/20] migration: introduce a QIOChannel impl for BlockDriverState VMState Daniel P. Berrangé
@ 2022-05-24 11:02 ` Daniel P. Berrangé
2022-06-09 14:57 ` Dr. David Alan Gilbert
2022-05-24 11:02 ` [PATCH 10/20] migration: stop passing 'opaque' parameter to QEMUFile hooks Daniel P. Berrangé
` (10 subsequent siblings)
19 siblings, 1 reply; 51+ messages in thread
From: Daniel P. Berrangé @ 2022-05-24 11:02 UTC (permalink / raw)
To: qemu-devel
Cc: Daniel P. Berrangé,
Dr. David Alan Gilbert, qemu-block, Stefan Hajnoczi,
Hailiang Zhang, Juan Quintela, Fam Zheng
With this change, all QEMUFile usage is backed by QIOChannel at
last.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
migration/savevm.c | 42 ++++--------------------------------------
1 file changed, 4 insertions(+), 38 deletions(-)
diff --git a/migration/savevm.c b/migration/savevm.c
index 75d05f1a84..24a50376dc 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -35,6 +35,7 @@
#include "migration/misc.h"
#include "migration/register.h"
#include "migration/global_state.h"
+#include "migration/channel-block.h"
#include "ram.h"
#include "qemu-file-channel.h"
#include "qemu-file.h"
@@ -130,48 +131,13 @@ static struct mig_cmd_args {
/***********************************************************/
/* savevm/loadvm support */
-static ssize_t block_writev_buffer(void *opaque, struct iovec *iov, int iovcnt,
- int64_t pos, Error **errp)
-{
- int ret;
- QEMUIOVector qiov;
-
- qemu_iovec_init_external(&qiov, iov, iovcnt);
- ret = bdrv_writev_vmstate(opaque, &qiov, pos);
- if (ret < 0) {
- return ret;
- }
-
- return qiov.size;
-}
-
-static ssize_t block_get_buffer(void *opaque, uint8_t *buf, int64_t pos,
- size_t size, Error **errp)
-{
- return bdrv_load_vmstate(opaque, buf, pos, size);
-}
-
-static int bdrv_fclose(void *opaque, Error **errp)
-{
- return bdrv_flush(opaque);
-}
-
-static const QEMUFileOps bdrv_read_ops = {
- .get_buffer = block_get_buffer,
- .close = bdrv_fclose
-};
-
-static const QEMUFileOps bdrv_write_ops = {
- .writev_buffer = block_writev_buffer,
- .close = bdrv_fclose
-};
-
static QEMUFile *qemu_fopen_bdrv(BlockDriverState *bs, int is_writable)
{
if (is_writable) {
- return qemu_fopen_ops(bs, &bdrv_write_ops, false);
+ return qemu_fopen_channel_output(QIO_CHANNEL(qio_channel_block_new(bs)));
+ } else {
+ return qemu_fopen_channel_input(QIO_CHANNEL(qio_channel_block_new(bs)));
}
- return qemu_fopen_ops(bs, &bdrv_read_ops, false);
}
--
2.36.1
^ permalink raw reply related [flat|nested] 51+ messages in thread* Re: [PATCH 09/20] migration: convert savevm to use QIOChannelBlock for VMState
2022-05-24 11:02 ` [PATCH 09/20] migration: convert savevm to use QIOChannelBlock for VMState Daniel P. Berrangé
@ 2022-06-09 14:57 ` Dr. David Alan Gilbert
0 siblings, 0 replies; 51+ messages in thread
From: Dr. David Alan Gilbert @ 2022-06-09 14:57 UTC (permalink / raw)
To: Daniel P. Berrangé
Cc: qemu-devel, qemu-block, Stefan Hajnoczi, Hailiang Zhang,
Juan Quintela, Fam Zheng
* Daniel P. Berrangé (berrange@redhat.com) wrote:
> With this change, all QEMUFile usage is backed by QIOChannel at
> last.
>
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> ---
> migration/savevm.c | 42 ++++--------------------------------------
> 1 file changed, 4 insertions(+), 38 deletions(-)
>
> diff --git a/migration/savevm.c b/migration/savevm.c
> index 75d05f1a84..24a50376dc 100644
> --- a/migration/savevm.c
> +++ b/migration/savevm.c
> @@ -35,6 +35,7 @@
> #include "migration/misc.h"
> #include "migration/register.h"
> #include "migration/global_state.h"
> +#include "migration/channel-block.h"
> #include "ram.h"
> #include "qemu-file-channel.h"
> #include "qemu-file.h"
> @@ -130,48 +131,13 @@ static struct mig_cmd_args {
> /***********************************************************/
> /* savevm/loadvm support */
>
> -static ssize_t block_writev_buffer(void *opaque, struct iovec *iov, int iovcnt,
> - int64_t pos, Error **errp)
> -{
> - int ret;
> - QEMUIOVector qiov;
> -
> - qemu_iovec_init_external(&qiov, iov, iovcnt);
> - ret = bdrv_writev_vmstate(opaque, &qiov, pos);
> - if (ret < 0) {
> - return ret;
> - }
> -
> - return qiov.size;
> -}
> -
> -static ssize_t block_get_buffer(void *opaque, uint8_t *buf, int64_t pos,
> - size_t size, Error **errp)
> -{
> - return bdrv_load_vmstate(opaque, buf, pos, size);
> -}
> -
> -static int bdrv_fclose(void *opaque, Error **errp)
> -{
> - return bdrv_flush(opaque);
> -}
> -
> -static const QEMUFileOps bdrv_read_ops = {
> - .get_buffer = block_get_buffer,
> - .close = bdrv_fclose
> -};
> -
> -static const QEMUFileOps bdrv_write_ops = {
> - .writev_buffer = block_writev_buffer,
> - .close = bdrv_fclose
> -};
> -
> static QEMUFile *qemu_fopen_bdrv(BlockDriverState *bs, int is_writable)
> {
> if (is_writable) {
> - return qemu_fopen_ops(bs, &bdrv_write_ops, false);
> + return qemu_fopen_channel_output(QIO_CHANNEL(qio_channel_block_new(bs)));
> + } else {
> + return qemu_fopen_channel_input(QIO_CHANNEL(qio_channel_block_new(bs)));
> }
> - return qemu_fopen_ops(bs, &bdrv_read_ops, false);
> }
>
>
> --
> 2.36.1
>
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
^ permalink raw reply [flat|nested] 51+ messages in thread
* [PATCH 10/20] migration: stop passing 'opaque' parameter to QEMUFile hooks
2022-05-24 11:02 [PATCH 00/20] migration: remove QEMUFileOps concept and assume use of QIOChannel Daniel P. Berrangé
` (8 preceding siblings ...)
2022-05-24 11:02 ` [PATCH 09/20] migration: convert savevm to use QIOChannelBlock for VMState Daniel P. Berrangé
@ 2022-05-24 11:02 ` Daniel P. Berrangé
2022-06-09 15:00 ` Dr. David Alan Gilbert
2022-05-24 11:02 ` [PATCH 11/20] migration: hardcode assumption that QEMUFile is backed with QIOChannel Daniel P. Berrangé
` (9 subsequent siblings)
19 siblings, 1 reply; 51+ messages in thread
From: Daniel P. Berrangé @ 2022-05-24 11:02 UTC (permalink / raw)
To: qemu-devel
Cc: Daniel P. Berrangé,
Dr. David Alan Gilbert, qemu-block, Stefan Hajnoczi,
Hailiang Zhang, Juan Quintela, Fam Zheng
The only user of the hooks is RDMA which provides a QIOChannel backed
impl of QEMUFile. It can thus use the qemu_file_get_ioc() method.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
migration/qemu-file.c | 8 ++++----
migration/qemu-file.h | 14 ++++++--------
migration/rdma.c | 19 ++++++++++---------
3 files changed, 20 insertions(+), 21 deletions(-)
diff --git a/migration/qemu-file.c b/migration/qemu-file.c
index 9a7f715e17..6badc8b0ec 100644
--- a/migration/qemu-file.c
+++ b/migration/qemu-file.c
@@ -266,7 +266,7 @@ void ram_control_before_iterate(QEMUFile *f, uint64_t flags)
int ret = 0;
if (f->hooks && f->hooks->before_ram_iterate) {
- ret = f->hooks->before_ram_iterate(f, f->opaque, flags, NULL);
+ ret = f->hooks->before_ram_iterate(f, flags, NULL);
if (ret < 0) {
qemu_file_set_error(f, ret);
}
@@ -278,7 +278,7 @@ void ram_control_after_iterate(QEMUFile *f, uint64_t flags)
int ret = 0;
if (f->hooks && f->hooks->after_ram_iterate) {
- ret = f->hooks->after_ram_iterate(f, f->opaque, flags, NULL);
+ ret = f->hooks->after_ram_iterate(f, flags, NULL);
if (ret < 0) {
qemu_file_set_error(f, ret);
}
@@ -290,7 +290,7 @@ void ram_control_load_hook(QEMUFile *f, uint64_t flags, void *data)
int ret = -EINVAL;
if (f->hooks && f->hooks->hook_ram_load) {
- ret = f->hooks->hook_ram_load(f, f->opaque, flags, data);
+ ret = f->hooks->hook_ram_load(f, flags, data);
if (ret < 0) {
qemu_file_set_error(f, ret);
}
@@ -310,7 +310,7 @@ size_t ram_control_save_page(QEMUFile *f, ram_addr_t block_offset,
uint64_t *bytes_sent)
{
if (f->hooks && f->hooks->save_page) {
- int ret = f->hooks->save_page(f, f->opaque, block_offset,
+ int ret = f->hooks->save_page(f, block_offset,
offset, size, bytes_sent);
if (ret != RAM_SAVE_CONTROL_NOT_SUPP) {
f->rate_limit_used += size;
diff --git a/migration/qemu-file.h b/migration/qemu-file.h
index 81f6fd7db8..6310071f90 100644
--- a/migration/qemu-file.h
+++ b/migration/qemu-file.h
@@ -65,11 +65,9 @@ typedef ssize_t (QEMUFileWritevBufferFunc)(void *opaque, struct iovec *iov,
/*
* This function provides hooks around different
* stages of RAM migration.
- * 'opaque' is the backend specific data in QEMUFile
* 'data' is call specific data associated with the 'flags' value
*/
-typedef int (QEMURamHookFunc)(QEMUFile *f, void *opaque, uint64_t flags,
- void *data);
+typedef int (QEMURamHookFunc)(QEMUFile *f, uint64_t flags, void *data);
/*
* Constants used by ram_control_* hooks
@@ -84,11 +82,11 @@ typedef int (QEMURamHookFunc)(QEMUFile *f, void *opaque, uint64_t flags,
* This function allows override of where the RAM page
* is saved (such as RDMA, for example.)
*/
-typedef size_t (QEMURamSaveFunc)(QEMUFile *f, void *opaque,
- ram_addr_t block_offset,
- ram_addr_t offset,
- size_t size,
- uint64_t *bytes_sent);
+typedef size_t (QEMURamSaveFunc)(QEMUFile *f,
+ ram_addr_t block_offset,
+ ram_addr_t offset,
+ size_t size,
+ uint64_t *bytes_sent);
/*
* Return a QEMUFile for comms in the opposite direction
diff --git a/migration/rdma.c b/migration/rdma.c
index 6e7756bee7..83265513d9 100644
--- a/migration/rdma.c
+++ b/migration/rdma.c
@@ -3256,11 +3256,11 @@ qio_channel_rdma_shutdown(QIOChannel *ioc,
* sent. Usually, this will not be more than a few bytes of
* the protocol because most transfers are sent asynchronously.
*/
-static size_t qemu_rdma_save_page(QEMUFile *f, void *opaque,
+static size_t qemu_rdma_save_page(QEMUFile *f,
ram_addr_t block_offset, ram_addr_t offset,
size_t size, uint64_t *bytes_sent)
{
- QIOChannelRDMA *rioc = QIO_CHANNEL_RDMA(opaque);
+ QIOChannelRDMA *rioc = QIO_CHANNEL_RDMA(qemu_file_get_ioc(f));
RDMAContext *rdma;
int ret;
@@ -3872,14 +3872,15 @@ rdma_block_notification_handle(QIOChannelRDMA *rioc, const char *name)
return 0;
}
-static int rdma_load_hook(QEMUFile *f, void *opaque, uint64_t flags, void *data)
+static int rdma_load_hook(QEMUFile *f, uint64_t flags, void *data)
{
+ QIOChannelRDMA *rioc = QIO_CHANNEL_RDMA(qemu_file_get_ioc(f));
switch (flags) {
case RAM_CONTROL_BLOCK_REG:
- return rdma_block_notification_handle(opaque, data);
+ return rdma_block_notification_handle(rioc, data);
case RAM_CONTROL_HOOK:
- return qemu_rdma_registration_handle(f, opaque);
+ return qemu_rdma_registration_handle(f, rioc);
default:
/* Shouldn't be called with any other values */
@@ -3887,10 +3888,10 @@ static int rdma_load_hook(QEMUFile *f, void *opaque, uint64_t flags, void *data)
}
}
-static int qemu_rdma_registration_start(QEMUFile *f, void *opaque,
+static int qemu_rdma_registration_start(QEMUFile *f,
uint64_t flags, void *data)
{
- QIOChannelRDMA *rioc = QIO_CHANNEL_RDMA(opaque);
+ QIOChannelRDMA *rioc = QIO_CHANNEL_RDMA(qemu_file_get_ioc(f));
RDMAContext *rdma;
RCU_READ_LOCK_GUARD();
@@ -3916,10 +3917,10 @@ static int qemu_rdma_registration_start(QEMUFile *f, void *opaque,
* Inform dest that dynamic registrations are done for now.
* First, flush writes, if any.
*/
-static int qemu_rdma_registration_stop(QEMUFile *f, void *opaque,
+static int qemu_rdma_registration_stop(QEMUFile *f,
uint64_t flags, void *data)
{
- QIOChannelRDMA *rioc = QIO_CHANNEL_RDMA(opaque);
+ QIOChannelRDMA *rioc = QIO_CHANNEL_RDMA(qemu_file_get_ioc(f));
RDMAContext *rdma;
RDMAControlHeader head = { .len = 0, .repeat = 1 };
int ret = 0;
--
2.36.1
^ permalink raw reply related [flat|nested] 51+ messages in thread* Re: [PATCH 10/20] migration: stop passing 'opaque' parameter to QEMUFile hooks
2022-05-24 11:02 ` [PATCH 10/20] migration: stop passing 'opaque' parameter to QEMUFile hooks Daniel P. Berrangé
@ 2022-06-09 15:00 ` Dr. David Alan Gilbert
0 siblings, 0 replies; 51+ messages in thread
From: Dr. David Alan Gilbert @ 2022-06-09 15:00 UTC (permalink / raw)
To: Daniel P. Berrangé
Cc: qemu-devel, qemu-block, Stefan Hajnoczi, Hailiang Zhang,
Juan Quintela, Fam Zheng
* Daniel P. Berrangé (berrange@redhat.com) wrote:
> The only user of the hooks is RDMA which provides a QIOChannel backed
> impl of QEMUFile. It can thus use the qemu_file_get_ioc() method.
>
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> ---
> migration/qemu-file.c | 8 ++++----
> migration/qemu-file.h | 14 ++++++--------
> migration/rdma.c | 19 ++++++++++---------
> 3 files changed, 20 insertions(+), 21 deletions(-)
>
> diff --git a/migration/qemu-file.c b/migration/qemu-file.c
> index 9a7f715e17..6badc8b0ec 100644
> --- a/migration/qemu-file.c
> +++ b/migration/qemu-file.c
> @@ -266,7 +266,7 @@ void ram_control_before_iterate(QEMUFile *f, uint64_t flags)
> int ret = 0;
>
> if (f->hooks && f->hooks->before_ram_iterate) {
> - ret = f->hooks->before_ram_iterate(f, f->opaque, flags, NULL);
> + ret = f->hooks->before_ram_iterate(f, flags, NULL);
> if (ret < 0) {
> qemu_file_set_error(f, ret);
> }
> @@ -278,7 +278,7 @@ void ram_control_after_iterate(QEMUFile *f, uint64_t flags)
> int ret = 0;
>
> if (f->hooks && f->hooks->after_ram_iterate) {
> - ret = f->hooks->after_ram_iterate(f, f->opaque, flags, NULL);
> + ret = f->hooks->after_ram_iterate(f, flags, NULL);
> if (ret < 0) {
> qemu_file_set_error(f, ret);
> }
> @@ -290,7 +290,7 @@ void ram_control_load_hook(QEMUFile *f, uint64_t flags, void *data)
> int ret = -EINVAL;
>
> if (f->hooks && f->hooks->hook_ram_load) {
> - ret = f->hooks->hook_ram_load(f, f->opaque, flags, data);
> + ret = f->hooks->hook_ram_load(f, flags, data);
> if (ret < 0) {
> qemu_file_set_error(f, ret);
> }
> @@ -310,7 +310,7 @@ size_t ram_control_save_page(QEMUFile *f, ram_addr_t block_offset,
> uint64_t *bytes_sent)
> {
> if (f->hooks && f->hooks->save_page) {
> - int ret = f->hooks->save_page(f, f->opaque, block_offset,
> + int ret = f->hooks->save_page(f, block_offset,
> offset, size, bytes_sent);
> if (ret != RAM_SAVE_CONTROL_NOT_SUPP) {
> f->rate_limit_used += size;
> diff --git a/migration/qemu-file.h b/migration/qemu-file.h
> index 81f6fd7db8..6310071f90 100644
> --- a/migration/qemu-file.h
> +++ b/migration/qemu-file.h
> @@ -65,11 +65,9 @@ typedef ssize_t (QEMUFileWritevBufferFunc)(void *opaque, struct iovec *iov,
> /*
> * This function provides hooks around different
> * stages of RAM migration.
> - * 'opaque' is the backend specific data in QEMUFile
> * 'data' is call specific data associated with the 'flags' value
> */
> -typedef int (QEMURamHookFunc)(QEMUFile *f, void *opaque, uint64_t flags,
> - void *data);
> +typedef int (QEMURamHookFunc)(QEMUFile *f, uint64_t flags, void *data);
>
> /*
> * Constants used by ram_control_* hooks
> @@ -84,11 +82,11 @@ typedef int (QEMURamHookFunc)(QEMUFile *f, void *opaque, uint64_t flags,
> * This function allows override of where the RAM page
> * is saved (such as RDMA, for example.)
> */
> -typedef size_t (QEMURamSaveFunc)(QEMUFile *f, void *opaque,
> - ram_addr_t block_offset,
> - ram_addr_t offset,
> - size_t size,
> - uint64_t *bytes_sent);
> +typedef size_t (QEMURamSaveFunc)(QEMUFile *f,
> + ram_addr_t block_offset,
> + ram_addr_t offset,
> + size_t size,
> + uint64_t *bytes_sent);
>
> /*
> * Return a QEMUFile for comms in the opposite direction
> diff --git a/migration/rdma.c b/migration/rdma.c
> index 6e7756bee7..83265513d9 100644
> --- a/migration/rdma.c
> +++ b/migration/rdma.c
> @@ -3256,11 +3256,11 @@ qio_channel_rdma_shutdown(QIOChannel *ioc,
> * sent. Usually, this will not be more than a few bytes of
> * the protocol because most transfers are sent asynchronously.
> */
> -static size_t qemu_rdma_save_page(QEMUFile *f, void *opaque,
> +static size_t qemu_rdma_save_page(QEMUFile *f,
> ram_addr_t block_offset, ram_addr_t offset,
> size_t size, uint64_t *bytes_sent)
> {
> - QIOChannelRDMA *rioc = QIO_CHANNEL_RDMA(opaque);
> + QIOChannelRDMA *rioc = QIO_CHANNEL_RDMA(qemu_file_get_ioc(f));
> RDMAContext *rdma;
> int ret;
>
> @@ -3872,14 +3872,15 @@ rdma_block_notification_handle(QIOChannelRDMA *rioc, const char *name)
> return 0;
> }
>
> -static int rdma_load_hook(QEMUFile *f, void *opaque, uint64_t flags, void *data)
> +static int rdma_load_hook(QEMUFile *f, uint64_t flags, void *data)
> {
> + QIOChannelRDMA *rioc = QIO_CHANNEL_RDMA(qemu_file_get_ioc(f));
> switch (flags) {
> case RAM_CONTROL_BLOCK_REG:
> - return rdma_block_notification_handle(opaque, data);
> + return rdma_block_notification_handle(rioc, data);
>
> case RAM_CONTROL_HOOK:
> - return qemu_rdma_registration_handle(f, opaque);
> + return qemu_rdma_registration_handle(f, rioc);
>
> default:
> /* Shouldn't be called with any other values */
> @@ -3887,10 +3888,10 @@ static int rdma_load_hook(QEMUFile *f, void *opaque, uint64_t flags, void *data)
> }
> }
>
> -static int qemu_rdma_registration_start(QEMUFile *f, void *opaque,
> +static int qemu_rdma_registration_start(QEMUFile *f,
> uint64_t flags, void *data)
> {
> - QIOChannelRDMA *rioc = QIO_CHANNEL_RDMA(opaque);
> + QIOChannelRDMA *rioc = QIO_CHANNEL_RDMA(qemu_file_get_ioc(f));
> RDMAContext *rdma;
>
> RCU_READ_LOCK_GUARD();
> @@ -3916,10 +3917,10 @@ static int qemu_rdma_registration_start(QEMUFile *f, void *opaque,
> * Inform dest that dynamic registrations are done for now.
> * First, flush writes, if any.
> */
> -static int qemu_rdma_registration_stop(QEMUFile *f, void *opaque,
> +static int qemu_rdma_registration_stop(QEMUFile *f,
> uint64_t flags, void *data)
> {
> - QIOChannelRDMA *rioc = QIO_CHANNEL_RDMA(opaque);
> + QIOChannelRDMA *rioc = QIO_CHANNEL_RDMA(qemu_file_get_ioc(f));
> RDMAContext *rdma;
> RDMAControlHeader head = { .len = 0, .repeat = 1 };
> int ret = 0;
> --
> 2.36.1
>
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
^ permalink raw reply [flat|nested] 51+ messages in thread
* [PATCH 11/20] migration: hardcode assumption that QEMUFile is backed with QIOChannel
2022-05-24 11:02 [PATCH 00/20] migration: remove QEMUFileOps concept and assume use of QIOChannel Daniel P. Berrangé
` (9 preceding siblings ...)
2022-05-24 11:02 ` [PATCH 10/20] migration: stop passing 'opaque' parameter to QEMUFile hooks Daniel P. Berrangé
@ 2022-05-24 11:02 ` Daniel P. Berrangé
2022-06-09 15:01 ` Dr. David Alan Gilbert
2022-05-24 11:02 ` [PATCH 12/20] migration: introduce new constructors for QEMUFile Daniel P. Berrangé
` (8 subsequent siblings)
19 siblings, 1 reply; 51+ messages in thread
From: Daniel P. Berrangé @ 2022-05-24 11:02 UTC (permalink / raw)
To: qemu-devel
Cc: Daniel P. Berrangé,
Dr. David Alan Gilbert, qemu-block, Stefan Hajnoczi,
Hailiang Zhang, Juan Quintela, Fam Zheng
The only callers of qemu_fopen_ops pass 'true' for the 'has_ioc'
parameter, so hardcode this assumption in QEMUFile, by passing in
the QIOChannel object as a non-opaque parameter.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
migration/qemu-file-channel.c | 4 ++--
migration/qemu-file.c | 35 +++++++++++++++++------------------
migration/qemu-file.h | 2 +-
3 files changed, 20 insertions(+), 21 deletions(-)
diff --git a/migration/qemu-file-channel.c b/migration/qemu-file-channel.c
index bb5a5752df..ce8eced417 100644
--- a/migration/qemu-file-channel.c
+++ b/migration/qemu-file-channel.c
@@ -184,11 +184,11 @@ static const QEMUFileOps channel_output_ops = {
QEMUFile *qemu_fopen_channel_input(QIOChannel *ioc)
{
object_ref(OBJECT(ioc));
- return qemu_fopen_ops(ioc, &channel_input_ops, true);
+ return qemu_fopen_ops(ioc, &channel_input_ops);
}
QEMUFile *qemu_fopen_channel_output(QIOChannel *ioc)
{
object_ref(OBJECT(ioc));
- return qemu_fopen_ops(ioc, &channel_output_ops, true);
+ return qemu_fopen_ops(ioc, &channel_output_ops);
}
diff --git a/migration/qemu-file.c b/migration/qemu-file.c
index 6badc8b0ec..ea1e8da0cb 100644
--- a/migration/qemu-file.c
+++ b/migration/qemu-file.c
@@ -37,7 +37,7 @@
struct QEMUFile {
const QEMUFileOps *ops;
const QEMUFileHooks *hooks;
- void *opaque;
+ QIOChannel *ioc;
/*
* Maximum amount of data in bytes to transfer during one
@@ -65,8 +65,6 @@ struct QEMUFile {
Error *last_error_obj;
/* has the file has been shutdown */
bool shutdown;
- /* Whether opaque points to a QIOChannel */
- bool has_ioc;
};
/*
@@ -81,7 +79,7 @@ int qemu_file_shutdown(QEMUFile *f)
if (!f->ops->shut_down) {
return -ENOSYS;
}
- ret = f->ops->shut_down(f->opaque, true, true, NULL);
+ ret = f->ops->shut_down(f->ioc, true, true, NULL);
if (!f->last_error) {
qemu_file_set_error(f, -EIO);
@@ -98,7 +96,7 @@ QEMUFile *qemu_file_get_return_path(QEMUFile *f)
if (!f->ops->get_return_path) {
return NULL;
}
- return f->ops->get_return_path(f->opaque);
+ return f->ops->get_return_path(f->ioc);
}
bool qemu_file_mode_is_not_valid(const char *mode)
@@ -113,15 +111,15 @@ bool qemu_file_mode_is_not_valid(const char *mode)
return false;
}
-QEMUFile *qemu_fopen_ops(void *opaque, const QEMUFileOps *ops, bool has_ioc)
+QEMUFile *qemu_fopen_ops(QIOChannel *ioc, const QEMUFileOps *ops)
{
QEMUFile *f;
f = g_new0(QEMUFile, 1);
- f->opaque = opaque;
+ f->ioc = ioc;
f->ops = ops;
- f->has_ioc = has_ioc;
+
return f;
}
@@ -242,7 +240,7 @@ void qemu_fflush(QEMUFile *f)
}
if (f->iovcnt > 0) {
expect = iov_size(f->iov, f->iovcnt);
- ret = f->ops->writev_buffer(f->opaque, f->iov, f->iovcnt, f->total_transferred,
+ ret = f->ops->writev_buffer(f->ioc, f->iov, f->iovcnt, f->total_transferred,
&local_error);
qemu_iovec_release_ram(f);
@@ -358,7 +356,7 @@ static ssize_t qemu_fill_buffer(QEMUFile *f)
return 0;
}
- len = f->ops->get_buffer(f->opaque, f->buf + pending, f->total_transferred,
+ len = f->ops->get_buffer(f->ioc, f->buf + pending, f->total_transferred,
IO_BUF_SIZE - pending, &local_error);
if (len > 0) {
f->buf_size += len;
@@ -394,7 +392,7 @@ int qemu_fclose(QEMUFile *f)
ret = qemu_file_get_error(f);
if (f->ops->close) {
- int ret2 = f->ops->close(f->opaque, NULL);
+ int ret2 = f->ops->close(f->ioc, NULL);
if (ret >= 0) {
ret = ret2;
}
@@ -861,18 +859,19 @@ void qemu_put_counted_string(QEMUFile *f, const char *str)
void qemu_file_set_blocking(QEMUFile *f, bool block)
{
if (f->ops->set_blocking) {
- f->ops->set_blocking(f->opaque, block, NULL);
+ f->ops->set_blocking(f->ioc, block, NULL);
}
}
/*
- * Return the ioc object if it's a migration channel. Note: it can return NULL
- * for callers passing in a non-migration qemufile. E.g. see qemu_fopen_bdrv()
- * and its usage in e.g. load_snapshot(). So we need to check against NULL
- * before using it. If without the check, migration_incoming_state_destroy()
- * could fail for load_snapshot().
+ * qemu_file_get_ioc:
+ *
+ * Get the ioc object for the file, without incrementing
+ * the reference count.
+ *
+ * Returns: the ioc object
*/
QIOChannel *qemu_file_get_ioc(QEMUFile *file)
{
- return file->has_ioc ? QIO_CHANNEL(file->opaque) : NULL;
+ return file->ioc;
}
diff --git a/migration/qemu-file.h b/migration/qemu-file.h
index 6310071f90..0458b1d3b6 100644
--- a/migration/qemu-file.h
+++ b/migration/qemu-file.h
@@ -118,7 +118,7 @@ typedef struct QEMUFileHooks {
QEMURamSaveFunc *save_page;
} QEMUFileHooks;
-QEMUFile *qemu_fopen_ops(void *opaque, const QEMUFileOps *ops, bool has_ioc);
+QEMUFile *qemu_fopen_ops(QIOChannel *ioc, const QEMUFileOps *ops);
void qemu_file_set_hooks(QEMUFile *f, const QEMUFileHooks *hooks);
int qemu_get_fd(QEMUFile *f);
int qemu_fclose(QEMUFile *f);
--
2.36.1
^ permalink raw reply related [flat|nested] 51+ messages in thread* Re: [PATCH 11/20] migration: hardcode assumption that QEMUFile is backed with QIOChannel
2022-05-24 11:02 ` [PATCH 11/20] migration: hardcode assumption that QEMUFile is backed with QIOChannel Daniel P. Berrangé
@ 2022-06-09 15:01 ` Dr. David Alan Gilbert
0 siblings, 0 replies; 51+ messages in thread
From: Dr. David Alan Gilbert @ 2022-06-09 15:01 UTC (permalink / raw)
To: Daniel P. Berrangé
Cc: qemu-devel, qemu-block, Stefan Hajnoczi, Hailiang Zhang,
Juan Quintela, Fam Zheng
* Daniel P. Berrangé (berrange@redhat.com) wrote:
> The only callers of qemu_fopen_ops pass 'true' for the 'has_ioc'
> parameter, so hardcode this assumption in QEMUFile, by passing in
> the QIOChannel object as a non-opaque parameter.
>
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> ---
> migration/qemu-file-channel.c | 4 ++--
> migration/qemu-file.c | 35 +++++++++++++++++------------------
> migration/qemu-file.h | 2 +-
> 3 files changed, 20 insertions(+), 21 deletions(-)
>
> diff --git a/migration/qemu-file-channel.c b/migration/qemu-file-channel.c
> index bb5a5752df..ce8eced417 100644
> --- a/migration/qemu-file-channel.c
> +++ b/migration/qemu-file-channel.c
> @@ -184,11 +184,11 @@ static const QEMUFileOps channel_output_ops = {
> QEMUFile *qemu_fopen_channel_input(QIOChannel *ioc)
> {
> object_ref(OBJECT(ioc));
> - return qemu_fopen_ops(ioc, &channel_input_ops, true);
> + return qemu_fopen_ops(ioc, &channel_input_ops);
> }
>
> QEMUFile *qemu_fopen_channel_output(QIOChannel *ioc)
> {
> object_ref(OBJECT(ioc));
> - return qemu_fopen_ops(ioc, &channel_output_ops, true);
> + return qemu_fopen_ops(ioc, &channel_output_ops);
> }
> diff --git a/migration/qemu-file.c b/migration/qemu-file.c
> index 6badc8b0ec..ea1e8da0cb 100644
> --- a/migration/qemu-file.c
> +++ b/migration/qemu-file.c
> @@ -37,7 +37,7 @@
> struct QEMUFile {
> const QEMUFileOps *ops;
> const QEMUFileHooks *hooks;
> - void *opaque;
> + QIOChannel *ioc;
>
> /*
> * Maximum amount of data in bytes to transfer during one
> @@ -65,8 +65,6 @@ struct QEMUFile {
> Error *last_error_obj;
> /* has the file has been shutdown */
> bool shutdown;
> - /* Whether opaque points to a QIOChannel */
> - bool has_ioc;
> };
>
> /*
> @@ -81,7 +79,7 @@ int qemu_file_shutdown(QEMUFile *f)
> if (!f->ops->shut_down) {
> return -ENOSYS;
> }
> - ret = f->ops->shut_down(f->opaque, true, true, NULL);
> + ret = f->ops->shut_down(f->ioc, true, true, NULL);
>
> if (!f->last_error) {
> qemu_file_set_error(f, -EIO);
> @@ -98,7 +96,7 @@ QEMUFile *qemu_file_get_return_path(QEMUFile *f)
> if (!f->ops->get_return_path) {
> return NULL;
> }
> - return f->ops->get_return_path(f->opaque);
> + return f->ops->get_return_path(f->ioc);
> }
>
> bool qemu_file_mode_is_not_valid(const char *mode)
> @@ -113,15 +111,15 @@ bool qemu_file_mode_is_not_valid(const char *mode)
> return false;
> }
>
> -QEMUFile *qemu_fopen_ops(void *opaque, const QEMUFileOps *ops, bool has_ioc)
> +QEMUFile *qemu_fopen_ops(QIOChannel *ioc, const QEMUFileOps *ops)
> {
> QEMUFile *f;
>
> f = g_new0(QEMUFile, 1);
>
> - f->opaque = opaque;
> + f->ioc = ioc;
> f->ops = ops;
> - f->has_ioc = has_ioc;
> +
> return f;
> }
>
> @@ -242,7 +240,7 @@ void qemu_fflush(QEMUFile *f)
> }
> if (f->iovcnt > 0) {
> expect = iov_size(f->iov, f->iovcnt);
> - ret = f->ops->writev_buffer(f->opaque, f->iov, f->iovcnt, f->total_transferred,
> + ret = f->ops->writev_buffer(f->ioc, f->iov, f->iovcnt, f->total_transferred,
> &local_error);
>
> qemu_iovec_release_ram(f);
> @@ -358,7 +356,7 @@ static ssize_t qemu_fill_buffer(QEMUFile *f)
> return 0;
> }
>
> - len = f->ops->get_buffer(f->opaque, f->buf + pending, f->total_transferred,
> + len = f->ops->get_buffer(f->ioc, f->buf + pending, f->total_transferred,
> IO_BUF_SIZE - pending, &local_error);
> if (len > 0) {
> f->buf_size += len;
> @@ -394,7 +392,7 @@ int qemu_fclose(QEMUFile *f)
> ret = qemu_file_get_error(f);
>
> if (f->ops->close) {
> - int ret2 = f->ops->close(f->opaque, NULL);
> + int ret2 = f->ops->close(f->ioc, NULL);
> if (ret >= 0) {
> ret = ret2;
> }
> @@ -861,18 +859,19 @@ void qemu_put_counted_string(QEMUFile *f, const char *str)
> void qemu_file_set_blocking(QEMUFile *f, bool block)
> {
> if (f->ops->set_blocking) {
> - f->ops->set_blocking(f->opaque, block, NULL);
> + f->ops->set_blocking(f->ioc, block, NULL);
> }
> }
>
> /*
> - * Return the ioc object if it's a migration channel. Note: it can return NULL
> - * for callers passing in a non-migration qemufile. E.g. see qemu_fopen_bdrv()
> - * and its usage in e.g. load_snapshot(). So we need to check against NULL
> - * before using it. If without the check, migration_incoming_state_destroy()
> - * could fail for load_snapshot().
> + * qemu_file_get_ioc:
> + *
> + * Get the ioc object for the file, without incrementing
> + * the reference count.
> + *
> + * Returns: the ioc object
> */
> QIOChannel *qemu_file_get_ioc(QEMUFile *file)
> {
> - return file->has_ioc ? QIO_CHANNEL(file->opaque) : NULL;
> + return file->ioc;
> }
> diff --git a/migration/qemu-file.h b/migration/qemu-file.h
> index 6310071f90..0458b1d3b6 100644
> --- a/migration/qemu-file.h
> +++ b/migration/qemu-file.h
> @@ -118,7 +118,7 @@ typedef struct QEMUFileHooks {
> QEMURamSaveFunc *save_page;
> } QEMUFileHooks;
>
> -QEMUFile *qemu_fopen_ops(void *opaque, const QEMUFileOps *ops, bool has_ioc);
> +QEMUFile *qemu_fopen_ops(QIOChannel *ioc, const QEMUFileOps *ops);
> void qemu_file_set_hooks(QEMUFile *f, const QEMUFileHooks *hooks);
> int qemu_get_fd(QEMUFile *f);
> int qemu_fclose(QEMUFile *f);
> --
> 2.36.1
>
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
^ permalink raw reply [flat|nested] 51+ messages in thread
* [PATCH 12/20] migration: introduce new constructors for QEMUFile
2022-05-24 11:02 [PATCH 00/20] migration: remove QEMUFileOps concept and assume use of QIOChannel Daniel P. Berrangé
` (10 preceding siblings ...)
2022-05-24 11:02 ` [PATCH 11/20] migration: hardcode assumption that QEMUFile is backed with QIOChannel Daniel P. Berrangé
@ 2022-05-24 11:02 ` Daniel P. Berrangé
2022-06-09 15:33 ` Dr. David Alan Gilbert
2022-05-24 11:02 ` [PATCH 13/20] migration: remove unused QEMUFileGetFD typedef Daniel P. Berrangé
` (7 subsequent siblings)
19 siblings, 1 reply; 51+ messages in thread
From: Daniel P. Berrangé @ 2022-05-24 11:02 UTC (permalink / raw)
To: qemu-devel
Cc: Daniel P. Berrangé,
Dr. David Alan Gilbert, qemu-block, Stefan Hajnoczi,
Hailiang Zhang, Juan Quintela, Fam Zheng
Prepare for the elimination of QEMUFileOps by introducing a pair of new
constructors. This lets us distinguish between an input and output file
object explicitly rather than via the existance of specific callbacks.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
migration/qemu-file-channel.c | 4 ++--
migration/qemu-file.c | 18 ++++++++++++++++--
migration/qemu-file.h | 3 ++-
3 files changed, 20 insertions(+), 5 deletions(-)
diff --git a/migration/qemu-file-channel.c b/migration/qemu-file-channel.c
index ce8eced417..5cb8ac93c0 100644
--- a/migration/qemu-file-channel.c
+++ b/migration/qemu-file-channel.c
@@ -184,11 +184,11 @@ static const QEMUFileOps channel_output_ops = {
QEMUFile *qemu_fopen_channel_input(QIOChannel *ioc)
{
object_ref(OBJECT(ioc));
- return qemu_fopen_ops(ioc, &channel_input_ops);
+ return qemu_file_new_input(ioc, &channel_input_ops);
}
QEMUFile *qemu_fopen_channel_output(QIOChannel *ioc)
{
object_ref(OBJECT(ioc));
- return qemu_fopen_ops(ioc, &channel_output_ops);
+ return qemu_file_new_output(ioc, &channel_output_ops);
}
diff --git a/migration/qemu-file.c b/migration/qemu-file.c
index ea1e8da0cb..5548e1abf3 100644
--- a/migration/qemu-file.c
+++ b/migration/qemu-file.c
@@ -38,6 +38,7 @@ struct QEMUFile {
const QEMUFileOps *ops;
const QEMUFileHooks *hooks;
QIOChannel *ioc;
+ bool is_writable;
/*
* Maximum amount of data in bytes to transfer during one
@@ -111,7 +112,9 @@ bool qemu_file_mode_is_not_valid(const char *mode)
return false;
}
-QEMUFile *qemu_fopen_ops(QIOChannel *ioc, const QEMUFileOps *ops)
+static QEMUFile *qemu_file_new_impl(QIOChannel *ioc,
+ const QEMUFileOps *ops,
+ bool is_writable)
{
QEMUFile *f;
@@ -119,10 +122,21 @@ QEMUFile *qemu_fopen_ops(QIOChannel *ioc, const QEMUFileOps *ops)
f->ioc = ioc;
f->ops = ops;
+ f->is_writable = is_writable;
return f;
}
+QEMUFile *qemu_file_new_output(QIOChannel *ioc, const QEMUFileOps *ops)
+{
+ return qemu_file_new_impl(ioc, ops, true);
+}
+
+QEMUFile *qemu_file_new_input(QIOChannel *ioc, const QEMUFileOps *ops)
+{
+ return qemu_file_new_impl(ioc, ops, false);
+}
+
void qemu_file_set_hooks(QEMUFile *f, const QEMUFileHooks *hooks)
{
@@ -181,7 +195,7 @@ void qemu_file_set_error(QEMUFile *f, int ret)
bool qemu_file_is_writable(QEMUFile *f)
{
- return f->ops->writev_buffer;
+ return f->is_writable;
}
static void qemu_iovec_release_ram(QEMUFile *f)
diff --git a/migration/qemu-file.h b/migration/qemu-file.h
index 0458b1d3b6..07c86bfea3 100644
--- a/migration/qemu-file.h
+++ b/migration/qemu-file.h
@@ -118,7 +118,8 @@ typedef struct QEMUFileHooks {
QEMURamSaveFunc *save_page;
} QEMUFileHooks;
-QEMUFile *qemu_fopen_ops(QIOChannel *ioc, const QEMUFileOps *ops);
+QEMUFile *qemu_file_new_input(QIOChannel *ioc, const QEMUFileOps *ops);
+QEMUFile *qemu_file_new_output(QIOChannel *ioc, const QEMUFileOps *ops);
void qemu_file_set_hooks(QEMUFile *f, const QEMUFileHooks *hooks);
int qemu_get_fd(QEMUFile *f);
int qemu_fclose(QEMUFile *f);
--
2.36.1
^ permalink raw reply related [flat|nested] 51+ messages in thread* Re: [PATCH 12/20] migration: introduce new constructors for QEMUFile
2022-05-24 11:02 ` [PATCH 12/20] migration: introduce new constructors for QEMUFile Daniel P. Berrangé
@ 2022-06-09 15:33 ` Dr. David Alan Gilbert
0 siblings, 0 replies; 51+ messages in thread
From: Dr. David Alan Gilbert @ 2022-06-09 15:33 UTC (permalink / raw)
To: Daniel P. Berrangé
Cc: qemu-devel, qemu-block, Stefan Hajnoczi, Hailiang Zhang,
Juan Quintela, Fam Zheng
* Daniel P. Berrangé (berrange@redhat.com) wrote:
> Prepare for the elimination of QEMUFileOps by introducing a pair of new
> constructors. This lets us distinguish between an input and output file
> object explicitly rather than via the existance of specific callbacks.
>
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> ---
> migration/qemu-file-channel.c | 4 ++--
> migration/qemu-file.c | 18 ++++++++++++++++--
> migration/qemu-file.h | 3 ++-
> 3 files changed, 20 insertions(+), 5 deletions(-)
>
> diff --git a/migration/qemu-file-channel.c b/migration/qemu-file-channel.c
> index ce8eced417..5cb8ac93c0 100644
> --- a/migration/qemu-file-channel.c
> +++ b/migration/qemu-file-channel.c
> @@ -184,11 +184,11 @@ static const QEMUFileOps channel_output_ops = {
> QEMUFile *qemu_fopen_channel_input(QIOChannel *ioc)
> {
> object_ref(OBJECT(ioc));
> - return qemu_fopen_ops(ioc, &channel_input_ops);
> + return qemu_file_new_input(ioc, &channel_input_ops);
> }
>
> QEMUFile *qemu_fopen_channel_output(QIOChannel *ioc)
> {
> object_ref(OBJECT(ioc));
> - return qemu_fopen_ops(ioc, &channel_output_ops);
> + return qemu_file_new_output(ioc, &channel_output_ops);
> }
> diff --git a/migration/qemu-file.c b/migration/qemu-file.c
> index ea1e8da0cb..5548e1abf3 100644
> --- a/migration/qemu-file.c
> +++ b/migration/qemu-file.c
> @@ -38,6 +38,7 @@ struct QEMUFile {
> const QEMUFileOps *ops;
> const QEMUFileHooks *hooks;
> QIOChannel *ioc;
> + bool is_writable;
>
> /*
> * Maximum amount of data in bytes to transfer during one
> @@ -111,7 +112,9 @@ bool qemu_file_mode_is_not_valid(const char *mode)
> return false;
> }
>
> -QEMUFile *qemu_fopen_ops(QIOChannel *ioc, const QEMUFileOps *ops)
> +static QEMUFile *qemu_file_new_impl(QIOChannel *ioc,
> + const QEMUFileOps *ops,
> + bool is_writable)
> {
> QEMUFile *f;
>
> @@ -119,10 +122,21 @@ QEMUFile *qemu_fopen_ops(QIOChannel *ioc, const QEMUFileOps *ops)
>
> f->ioc = ioc;
> f->ops = ops;
> + f->is_writable = is_writable;
>
> return f;
> }
>
> +QEMUFile *qemu_file_new_output(QIOChannel *ioc, const QEMUFileOps *ops)
> +{
> + return qemu_file_new_impl(ioc, ops, true);
> +}
> +
> +QEMUFile *qemu_file_new_input(QIOChannel *ioc, const QEMUFileOps *ops)
> +{
> + return qemu_file_new_impl(ioc, ops, false);
> +}
> +
>
> void qemu_file_set_hooks(QEMUFile *f, const QEMUFileHooks *hooks)
> {
> @@ -181,7 +195,7 @@ void qemu_file_set_error(QEMUFile *f, int ret)
>
> bool qemu_file_is_writable(QEMUFile *f)
> {
> - return f->ops->writev_buffer;
> + return f->is_writable;
> }
>
> static void qemu_iovec_release_ram(QEMUFile *f)
> diff --git a/migration/qemu-file.h b/migration/qemu-file.h
> index 0458b1d3b6..07c86bfea3 100644
> --- a/migration/qemu-file.h
> +++ b/migration/qemu-file.h
> @@ -118,7 +118,8 @@ typedef struct QEMUFileHooks {
> QEMURamSaveFunc *save_page;
> } QEMUFileHooks;
>
> -QEMUFile *qemu_fopen_ops(QIOChannel *ioc, const QEMUFileOps *ops);
> +QEMUFile *qemu_file_new_input(QIOChannel *ioc, const QEMUFileOps *ops);
> +QEMUFile *qemu_file_new_output(QIOChannel *ioc, const QEMUFileOps *ops);
> void qemu_file_set_hooks(QEMUFile *f, const QEMUFileHooks *hooks);
> int qemu_get_fd(QEMUFile *f);
> int qemu_fclose(QEMUFile *f);
> --
> 2.36.1
>
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
^ permalink raw reply [flat|nested] 51+ messages in thread
* [PATCH 13/20] migration: remove unused QEMUFileGetFD typedef
2022-05-24 11:02 [PATCH 00/20] migration: remove QEMUFileOps concept and assume use of QIOChannel Daniel P. Berrangé
` (11 preceding siblings ...)
2022-05-24 11:02 ` [PATCH 12/20] migration: introduce new constructors for QEMUFile Daniel P. Berrangé
@ 2022-05-24 11:02 ` Daniel P. Berrangé
2022-06-09 16:03 ` Dr. David Alan Gilbert
2022-05-24 11:02 ` [PATCH 14/20] migration: remove the QEMUFileOps 'shut_down' callback Daniel P. Berrangé
` (6 subsequent siblings)
19 siblings, 1 reply; 51+ messages in thread
From: Daniel P. Berrangé @ 2022-05-24 11:02 UTC (permalink / raw)
To: qemu-devel
Cc: Daniel P. Berrangé,
Dr. David Alan Gilbert, qemu-block, Stefan Hajnoczi,
Hailiang Zhang, Juan Quintela, Fam Zheng
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
migration/qemu-file.h | 4 ----
1 file changed, 4 deletions(-)
diff --git a/migration/qemu-file.h b/migration/qemu-file.h
index 07c86bfea3..674c2c409b 100644
--- a/migration/qemu-file.h
+++ b/migration/qemu-file.h
@@ -46,10 +46,6 @@ typedef ssize_t (QEMUFileGetBufferFunc)(void *opaque, uint8_t *buf,
*/
typedef int (QEMUFileCloseFunc)(void *opaque, Error **errp);
-/* Called to return the OS file descriptor associated to the QEMUFile.
- */
-typedef int (QEMUFileGetFD)(void *opaque);
-
/* Called to change the blocking mode of the file
*/
typedef int (QEMUFileSetBlocking)(void *opaque, bool enabled, Error **errp);
--
2.36.1
^ permalink raw reply related [flat|nested] 51+ messages in thread* Re: [PATCH 13/20] migration: remove unused QEMUFileGetFD typedef
2022-05-24 11:02 ` [PATCH 13/20] migration: remove unused QEMUFileGetFD typedef Daniel P. Berrangé
@ 2022-06-09 16:03 ` Dr. David Alan Gilbert
0 siblings, 0 replies; 51+ messages in thread
From: Dr. David Alan Gilbert @ 2022-06-09 16:03 UTC (permalink / raw)
To: Daniel P. Berrangé
Cc: qemu-devel, qemu-block, Stefan Hajnoczi, Hailiang Zhang,
Juan Quintela, Fam Zheng
* Daniel P. Berrangé (berrange@redhat.com) wrote:
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> ---
> migration/qemu-file.h | 4 ----
> 1 file changed, 4 deletions(-)
>
> diff --git a/migration/qemu-file.h b/migration/qemu-file.h
> index 07c86bfea3..674c2c409b 100644
> --- a/migration/qemu-file.h
> +++ b/migration/qemu-file.h
> @@ -46,10 +46,6 @@ typedef ssize_t (QEMUFileGetBufferFunc)(void *opaque, uint8_t *buf,
> */
> typedef int (QEMUFileCloseFunc)(void *opaque, Error **errp);
>
> -/* Called to return the OS file descriptor associated to the QEMUFile.
> - */
> -typedef int (QEMUFileGetFD)(void *opaque);
> -
> /* Called to change the blocking mode of the file
> */
> typedef int (QEMUFileSetBlocking)(void *opaque, bool enabled, Error **errp);
> --
> 2.36.1
>
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
^ permalink raw reply [flat|nested] 51+ messages in thread
* [PATCH 14/20] migration: remove the QEMUFileOps 'shut_down' callback
2022-05-24 11:02 [PATCH 00/20] migration: remove QEMUFileOps concept and assume use of QIOChannel Daniel P. Berrangé
` (12 preceding siblings ...)
2022-05-24 11:02 ` [PATCH 13/20] migration: remove unused QEMUFileGetFD typedef Daniel P. Berrangé
@ 2022-05-24 11:02 ` Daniel P. Berrangé
2022-06-09 16:12 ` Dr. David Alan Gilbert
2022-05-24 11:02 ` [PATCH 15/20] migration: remove the QEMUFileOps 'set_blocking' callback Daniel P. Berrangé
` (5 subsequent siblings)
19 siblings, 1 reply; 51+ messages in thread
From: Daniel P. Berrangé @ 2022-05-24 11:02 UTC (permalink / raw)
To: qemu-devel
Cc: Daniel P. Berrangé,
Dr. David Alan Gilbert, qemu-block, Stefan Hajnoczi,
Hailiang Zhang, Juan Quintela, Fam Zheng
This directly implements the shutdown logic using QIOChannel APIs.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
migration/qemu-file-channel.c | 27 ---------------------------
migration/qemu-file.c | 10 +++++++---
migration/qemu-file.h | 10 ----------
3 files changed, 7 insertions(+), 40 deletions(-)
diff --git a/migration/qemu-file-channel.c b/migration/qemu-file-channel.c
index 5cb8ac93c0..80f05dc371 100644
--- a/migration/qemu-file-channel.c
+++ b/migration/qemu-file-channel.c
@@ -112,31 +112,6 @@ static int channel_close(void *opaque, Error **errp)
}
-static int channel_shutdown(void *opaque,
- bool rd,
- bool wr,
- Error **errp)
-{
- QIOChannel *ioc = QIO_CHANNEL(opaque);
-
- if (qio_channel_has_feature(ioc,
- QIO_CHANNEL_FEATURE_SHUTDOWN)) {
- QIOChannelShutdown mode;
- if (rd && wr) {
- mode = QIO_CHANNEL_SHUTDOWN_BOTH;
- } else if (rd) {
- mode = QIO_CHANNEL_SHUTDOWN_READ;
- } else {
- mode = QIO_CHANNEL_SHUTDOWN_WRITE;
- }
- if (qio_channel_shutdown(ioc, mode, errp) < 0) {
- return -EIO;
- }
- }
- return 0;
-}
-
-
static int channel_set_blocking(void *opaque,
bool enabled,
Error **errp)
@@ -166,7 +141,6 @@ static QEMUFile *channel_get_output_return_path(void *opaque)
static const QEMUFileOps channel_input_ops = {
.get_buffer = channel_get_buffer,
.close = channel_close,
- .shut_down = channel_shutdown,
.set_blocking = channel_set_blocking,
.get_return_path = channel_get_input_return_path,
};
@@ -175,7 +149,6 @@ static const QEMUFileOps channel_input_ops = {
static const QEMUFileOps channel_output_ops = {
.writev_buffer = channel_writev_buffer,
.close = channel_close,
- .shut_down = channel_shutdown,
.set_blocking = channel_set_blocking,
.get_return_path = channel_get_output_return_path,
};
diff --git a/migration/qemu-file.c b/migration/qemu-file.c
index 5548e1abf3..fd9f060c02 100644
--- a/migration/qemu-file.c
+++ b/migration/qemu-file.c
@@ -74,13 +74,17 @@ struct QEMUFile {
*/
int qemu_file_shutdown(QEMUFile *f)
{
- int ret;
+ int ret = 0;
f->shutdown = true;
- if (!f->ops->shut_down) {
+ if (!qio_channel_has_feature(f->ioc,
+ QIO_CHANNEL_FEATURE_SHUTDOWN)) {
return -ENOSYS;
}
- ret = f->ops->shut_down(f->ioc, true, true, NULL);
+
+ if (qio_channel_shutdown(f->ioc, QIO_CHANNEL_SHUTDOWN_BOTH, NULL) < 0) {
+ ret = -EIO;
+ }
if (!f->last_error) {
qemu_file_set_error(f, -EIO);
diff --git a/migration/qemu-file.h b/migration/qemu-file.h
index 674c2c409b..2049dfe7e4 100644
--- a/migration/qemu-file.h
+++ b/migration/qemu-file.h
@@ -89,22 +89,12 @@ typedef size_t (QEMURamSaveFunc)(QEMUFile *f,
*/
typedef QEMUFile *(QEMURetPathFunc)(void *opaque);
-/*
- * Stop any read or write (depending on flags) on the underlying
- * transport on the QEMUFile.
- * Existing blocking reads/writes must be woken
- * Returns 0 on success, -err on error
- */
-typedef int (QEMUFileShutdownFunc)(void *opaque, bool rd, bool wr,
- Error **errp);
-
typedef struct QEMUFileOps {
QEMUFileGetBufferFunc *get_buffer;
QEMUFileCloseFunc *close;
QEMUFileSetBlocking *set_blocking;
QEMUFileWritevBufferFunc *writev_buffer;
QEMURetPathFunc *get_return_path;
- QEMUFileShutdownFunc *shut_down;
} QEMUFileOps;
typedef struct QEMUFileHooks {
--
2.36.1
^ permalink raw reply related [flat|nested] 51+ messages in thread* Re: [PATCH 14/20] migration: remove the QEMUFileOps 'shut_down' callback
2022-05-24 11:02 ` [PATCH 14/20] migration: remove the QEMUFileOps 'shut_down' callback Daniel P. Berrangé
@ 2022-06-09 16:12 ` Dr. David Alan Gilbert
2022-06-09 16:14 ` Daniel P. Berrangé
0 siblings, 1 reply; 51+ messages in thread
From: Dr. David Alan Gilbert @ 2022-06-09 16:12 UTC (permalink / raw)
To: Daniel P. Berrangé
Cc: qemu-devel, qemu-block, Stefan Hajnoczi, Hailiang Zhang,
Juan Quintela, Fam Zheng
* Daniel P. Berrangé (berrange@redhat.com) wrote:
> This directly implements the shutdown logic using QIOChannel APIs.
>
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
> migration/qemu-file-channel.c | 27 ---------------------------
> migration/qemu-file.c | 10 +++++++---
> migration/qemu-file.h | 10 ----------
> 3 files changed, 7 insertions(+), 40 deletions(-)
>
> diff --git a/migration/qemu-file-channel.c b/migration/qemu-file-channel.c
> index 5cb8ac93c0..80f05dc371 100644
> --- a/migration/qemu-file-channel.c
> +++ b/migration/qemu-file-channel.c
> @@ -112,31 +112,6 @@ static int channel_close(void *opaque, Error **errp)
> }
>
>
> -static int channel_shutdown(void *opaque,
> - bool rd,
> - bool wr,
> - Error **errp)
> -{
> - QIOChannel *ioc = QIO_CHANNEL(opaque);
> -
> - if (qio_channel_has_feature(ioc,
> - QIO_CHANNEL_FEATURE_SHUTDOWN)) {
> - QIOChannelShutdown mode;
> - if (rd && wr) {
> - mode = QIO_CHANNEL_SHUTDOWN_BOTH;
> - } else if (rd) {
> - mode = QIO_CHANNEL_SHUTDOWN_READ;
> - } else {
> - mode = QIO_CHANNEL_SHUTDOWN_WRITE;
> - }
> - if (qio_channel_shutdown(ioc, mode, errp) < 0) {
> - return -EIO;
> - }
> - }
> - return 0;
> -}
> -
> -
> static int channel_set_blocking(void *opaque,
> bool enabled,
> Error **errp)
> @@ -166,7 +141,6 @@ static QEMUFile *channel_get_output_return_path(void *opaque)
> static const QEMUFileOps channel_input_ops = {
> .get_buffer = channel_get_buffer,
> .close = channel_close,
> - .shut_down = channel_shutdown,
> .set_blocking = channel_set_blocking,
> .get_return_path = channel_get_input_return_path,
> };
> @@ -175,7 +149,6 @@ static const QEMUFileOps channel_input_ops = {
> static const QEMUFileOps channel_output_ops = {
> .writev_buffer = channel_writev_buffer,
> .close = channel_close,
> - .shut_down = channel_shutdown,
> .set_blocking = channel_set_blocking,
> .get_return_path = channel_get_output_return_path,
> };
> diff --git a/migration/qemu-file.c b/migration/qemu-file.c
> index 5548e1abf3..fd9f060c02 100644
> --- a/migration/qemu-file.c
> +++ b/migration/qemu-file.c
> @@ -74,13 +74,17 @@ struct QEMUFile {
> */
> int qemu_file_shutdown(QEMUFile *f)
> {
> - int ret;
> + int ret = 0;
>
> f->shutdown = true;
> - if (!f->ops->shut_down) {
> + if (!qio_channel_has_feature(f->ioc,
> + QIO_CHANNEL_FEATURE_SHUTDOWN)) {
> return -ENOSYS;
> }
> - ret = f->ops->shut_down(f->ioc, true, true, NULL);
> +
> + if (qio_channel_shutdown(f->ioc, QIO_CHANNEL_SHUTDOWN_BOTH, NULL) < 0) {
> + ret = -EIO;
> + }
OK, so this is following the code you're flattening; so:
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
I wonder if there's any reason it doesn't just pass the return value through to ret rather
than flattening it to -EIO?
> if (!f->last_error) {
> qemu_file_set_error(f, -EIO);
> diff --git a/migration/qemu-file.h b/migration/qemu-file.h
> index 674c2c409b..2049dfe7e4 100644
> --- a/migration/qemu-file.h
> +++ b/migration/qemu-file.h
> @@ -89,22 +89,12 @@ typedef size_t (QEMURamSaveFunc)(QEMUFile *f,
> */
> typedef QEMUFile *(QEMURetPathFunc)(void *opaque);
>
> -/*
> - * Stop any read or write (depending on flags) on the underlying
> - * transport on the QEMUFile.
> - * Existing blocking reads/writes must be woken
> - * Returns 0 on success, -err on error
> - */
> -typedef int (QEMUFileShutdownFunc)(void *opaque, bool rd, bool wr,
> - Error **errp);
> -
> typedef struct QEMUFileOps {
> QEMUFileGetBufferFunc *get_buffer;
> QEMUFileCloseFunc *close;
> QEMUFileSetBlocking *set_blocking;
> QEMUFileWritevBufferFunc *writev_buffer;
> QEMURetPathFunc *get_return_path;
> - QEMUFileShutdownFunc *shut_down;
> } QEMUFileOps;
>
> typedef struct QEMUFileHooks {
> --
> 2.36.1
>
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
^ permalink raw reply [flat|nested] 51+ messages in thread* Re: [PATCH 14/20] migration: remove the QEMUFileOps 'shut_down' callback
2022-06-09 16:12 ` Dr. David Alan Gilbert
@ 2022-06-09 16:14 ` Daniel P. Berrangé
2022-06-09 16:17 ` Dr. David Alan Gilbert
0 siblings, 1 reply; 51+ messages in thread
From: Daniel P. Berrangé @ 2022-06-09 16:14 UTC (permalink / raw)
To: Dr. David Alan Gilbert
Cc: qemu-devel, qemu-block, Stefan Hajnoczi, Hailiang Zhang,
Juan Quintela, Fam Zheng
On Thu, Jun 09, 2022 at 05:12:41PM +0100, Dr. David Alan Gilbert wrote:
> * Daniel P. Berrangé (berrange@redhat.com) wrote:
> > This directly implements the shutdown logic using QIOChannel APIs.
> >
> > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> > ---
> > migration/qemu-file-channel.c | 27 ---------------------------
> > migration/qemu-file.c | 10 +++++++---
> > migration/qemu-file.h | 10 ----------
> > 3 files changed, 7 insertions(+), 40 deletions(-)
> >
> > diff --git a/migration/qemu-file-channel.c b/migration/qemu-file-channel.c
> > index 5cb8ac93c0..80f05dc371 100644
> > --- a/migration/qemu-file-channel.c
> > +++ b/migration/qemu-file-channel.c
> > @@ -112,31 +112,6 @@ static int channel_close(void *opaque, Error **errp)
> > }
> >
> >
> > -static int channel_shutdown(void *opaque,
> > - bool rd,
> > - bool wr,
> > - Error **errp)
> > -{
> > - QIOChannel *ioc = QIO_CHANNEL(opaque);
> > -
> > - if (qio_channel_has_feature(ioc,
> > - QIO_CHANNEL_FEATURE_SHUTDOWN)) {
> > - QIOChannelShutdown mode;
> > - if (rd && wr) {
> > - mode = QIO_CHANNEL_SHUTDOWN_BOTH;
> > - } else if (rd) {
> > - mode = QIO_CHANNEL_SHUTDOWN_READ;
> > - } else {
> > - mode = QIO_CHANNEL_SHUTDOWN_WRITE;
> > - }
> > - if (qio_channel_shutdown(ioc, mode, errp) < 0) {
> > - return -EIO;
> > - }
> > - }
> > - return 0;
> > -}
> > -
> > -
> > static int channel_set_blocking(void *opaque,
> > bool enabled,
> > Error **errp)
> > @@ -166,7 +141,6 @@ static QEMUFile *channel_get_output_return_path(void *opaque)
> > static const QEMUFileOps channel_input_ops = {
> > .get_buffer = channel_get_buffer,
> > .close = channel_close,
> > - .shut_down = channel_shutdown,
> > .set_blocking = channel_set_blocking,
> > .get_return_path = channel_get_input_return_path,
> > };
> > @@ -175,7 +149,6 @@ static const QEMUFileOps channel_input_ops = {
> > static const QEMUFileOps channel_output_ops = {
> > .writev_buffer = channel_writev_buffer,
> > .close = channel_close,
> > - .shut_down = channel_shutdown,
> > .set_blocking = channel_set_blocking,
> > .get_return_path = channel_get_output_return_path,
> > };
> > diff --git a/migration/qemu-file.c b/migration/qemu-file.c
> > index 5548e1abf3..fd9f060c02 100644
> > --- a/migration/qemu-file.c
> > +++ b/migration/qemu-file.c
> > @@ -74,13 +74,17 @@ struct QEMUFile {
> > */
> > int qemu_file_shutdown(QEMUFile *f)
> > {
> > - int ret;
> > + int ret = 0;
> >
> > f->shutdown = true;
> > - if (!f->ops->shut_down) {
> > + if (!qio_channel_has_feature(f->ioc,
> > + QIO_CHANNEL_FEATURE_SHUTDOWN)) {
> > return -ENOSYS;
> > }
> > - ret = f->ops->shut_down(f->ioc, true, true, NULL);
> > +
> > + if (qio_channel_shutdown(f->ioc, QIO_CHANNEL_SHUTDOWN_BOTH, NULL) < 0) {
> > + ret = -EIO;
> > + }
>
> OK, so this is following the code you're flattening; so:
>
> Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
>
> I wonder if there's any reason it doesn't just pass the return value through to ret rather
> than flattening it to -EIO?
qio methods never return errno values just positive integer or -1.
Since qemu_file_shutdown seems to want an errno, I picked EIO
Better would be for qemu_file_shutdown to have an Error **errp
param instead but that could come later.
With regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
^ permalink raw reply [flat|nested] 51+ messages in thread* Re: [PATCH 14/20] migration: remove the QEMUFileOps 'shut_down' callback
2022-06-09 16:14 ` Daniel P. Berrangé
@ 2022-06-09 16:17 ` Dr. David Alan Gilbert
0 siblings, 0 replies; 51+ messages in thread
From: Dr. David Alan Gilbert @ 2022-06-09 16:17 UTC (permalink / raw)
To: Daniel P. Berrangé
Cc: qemu-devel, qemu-block, Stefan Hajnoczi, Hailiang Zhang,
Juan Quintela, Fam Zheng
* Daniel P. Berrangé (berrange@redhat.com) wrote:
> On Thu, Jun 09, 2022 at 05:12:41PM +0100, Dr. David Alan Gilbert wrote:
> > * Daniel P. Berrangé (berrange@redhat.com) wrote:
> > > This directly implements the shutdown logic using QIOChannel APIs.
> > >
> > > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> > > ---
> > > migration/qemu-file-channel.c | 27 ---------------------------
> > > migration/qemu-file.c | 10 +++++++---
> > > migration/qemu-file.h | 10 ----------
> > > 3 files changed, 7 insertions(+), 40 deletions(-)
> > >
> > > diff --git a/migration/qemu-file-channel.c b/migration/qemu-file-channel.c
> > > index 5cb8ac93c0..80f05dc371 100644
> > > --- a/migration/qemu-file-channel.c
> > > +++ b/migration/qemu-file-channel.c
> > > @@ -112,31 +112,6 @@ static int channel_close(void *opaque, Error **errp)
> > > }
> > >
> > >
> > > -static int channel_shutdown(void *opaque,
> > > - bool rd,
> > > - bool wr,
> > > - Error **errp)
> > > -{
> > > - QIOChannel *ioc = QIO_CHANNEL(opaque);
> > > -
> > > - if (qio_channel_has_feature(ioc,
> > > - QIO_CHANNEL_FEATURE_SHUTDOWN)) {
> > > - QIOChannelShutdown mode;
> > > - if (rd && wr) {
> > > - mode = QIO_CHANNEL_SHUTDOWN_BOTH;
> > > - } else if (rd) {
> > > - mode = QIO_CHANNEL_SHUTDOWN_READ;
> > > - } else {
> > > - mode = QIO_CHANNEL_SHUTDOWN_WRITE;
> > > - }
> > > - if (qio_channel_shutdown(ioc, mode, errp) < 0) {
> > > - return -EIO;
> > > - }
> > > - }
> > > - return 0;
> > > -}
> > > -
> > > -
> > > static int channel_set_blocking(void *opaque,
> > > bool enabled,
> > > Error **errp)
> > > @@ -166,7 +141,6 @@ static QEMUFile *channel_get_output_return_path(void *opaque)
> > > static const QEMUFileOps channel_input_ops = {
> > > .get_buffer = channel_get_buffer,
> > > .close = channel_close,
> > > - .shut_down = channel_shutdown,
> > > .set_blocking = channel_set_blocking,
> > > .get_return_path = channel_get_input_return_path,
> > > };
> > > @@ -175,7 +149,6 @@ static const QEMUFileOps channel_input_ops = {
> > > static const QEMUFileOps channel_output_ops = {
> > > .writev_buffer = channel_writev_buffer,
> > > .close = channel_close,
> > > - .shut_down = channel_shutdown,
> > > .set_blocking = channel_set_blocking,
> > > .get_return_path = channel_get_output_return_path,
> > > };
> > > diff --git a/migration/qemu-file.c b/migration/qemu-file.c
> > > index 5548e1abf3..fd9f060c02 100644
> > > --- a/migration/qemu-file.c
> > > +++ b/migration/qemu-file.c
> > > @@ -74,13 +74,17 @@ struct QEMUFile {
> > > */
> > > int qemu_file_shutdown(QEMUFile *f)
> > > {
> > > - int ret;
> > > + int ret = 0;
> > >
> > > f->shutdown = true;
> > > - if (!f->ops->shut_down) {
> > > + if (!qio_channel_has_feature(f->ioc,
> > > + QIO_CHANNEL_FEATURE_SHUTDOWN)) {
> > > return -ENOSYS;
> > > }
> > > - ret = f->ops->shut_down(f->ioc, true, true, NULL);
> > > +
> > > + if (qio_channel_shutdown(f->ioc, QIO_CHANNEL_SHUTDOWN_BOTH, NULL) < 0) {
> > > + ret = -EIO;
> > > + }
> >
> > OK, so this is following the code you're flattening; so:
> >
> > Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> >
> > I wonder if there's any reason it doesn't just pass the return value through to ret rather
> > than flattening it to -EIO?
>
> qio methods never return errno values just positive integer or -1.
>
> Since qemu_file_shutdown seems to want an errno, I picked EIO
>
> Better would be for qemu_file_shutdown to have an Error **errp
> param instead but that could come later.
Ah OK.
Dave
>
> With regards,
> Daniel
> --
> |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
> |: https://libvirt.org -o- https://fstop138.berrange.com :|
> |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
>
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
^ permalink raw reply [flat|nested] 51+ messages in thread
* [PATCH 15/20] migration: remove the QEMUFileOps 'set_blocking' callback
2022-05-24 11:02 [PATCH 00/20] migration: remove QEMUFileOps concept and assume use of QIOChannel Daniel P. Berrangé
` (13 preceding siblings ...)
2022-05-24 11:02 ` [PATCH 14/20] migration: remove the QEMUFileOps 'shut_down' callback Daniel P. Berrangé
@ 2022-05-24 11:02 ` Daniel P. Berrangé
2022-06-09 16:21 ` Dr. David Alan Gilbert
2022-05-24 11:02 ` [PATCH 16/20] migration: remove the QEMUFileOps 'close' callback Daniel P. Berrangé
` (4 subsequent siblings)
19 siblings, 1 reply; 51+ messages in thread
From: Daniel P. Berrangé @ 2022-05-24 11:02 UTC (permalink / raw)
To: qemu-devel
Cc: Daniel P. Berrangé,
Dr. David Alan Gilbert, qemu-block, Stefan Hajnoczi,
Hailiang Zhang, Juan Quintela, Fam Zheng
This directly implements the set_blocking logic using QIOChannel APIs.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
migration/qemu-file-channel.c | 14 --------------
migration/qemu-file.c | 4 +---
migration/qemu-file.h | 5 -----
3 files changed, 1 insertion(+), 22 deletions(-)
diff --git a/migration/qemu-file-channel.c b/migration/qemu-file-channel.c
index 80f05dc371..0350d367ec 100644
--- a/migration/qemu-file-channel.c
+++ b/migration/qemu-file-channel.c
@@ -112,18 +112,6 @@ static int channel_close(void *opaque, Error **errp)
}
-static int channel_set_blocking(void *opaque,
- bool enabled,
- Error **errp)
-{
- QIOChannel *ioc = QIO_CHANNEL(opaque);
-
- if (qio_channel_set_blocking(ioc, enabled, errp) < 0) {
- return -1;
- }
- return 0;
-}
-
static QEMUFile *channel_get_input_return_path(void *opaque)
{
QIOChannel *ioc = QIO_CHANNEL(opaque);
@@ -141,7 +129,6 @@ static QEMUFile *channel_get_output_return_path(void *opaque)
static const QEMUFileOps channel_input_ops = {
.get_buffer = channel_get_buffer,
.close = channel_close,
- .set_blocking = channel_set_blocking,
.get_return_path = channel_get_input_return_path,
};
@@ -149,7 +136,6 @@ static const QEMUFileOps channel_input_ops = {
static const QEMUFileOps channel_output_ops = {
.writev_buffer = channel_writev_buffer,
.close = channel_close,
- .set_blocking = channel_set_blocking,
.get_return_path = channel_get_output_return_path,
};
diff --git a/migration/qemu-file.c b/migration/qemu-file.c
index fd9f060c02..171b9f85bf 100644
--- a/migration/qemu-file.c
+++ b/migration/qemu-file.c
@@ -876,9 +876,7 @@ void qemu_put_counted_string(QEMUFile *f, const char *str)
*/
void qemu_file_set_blocking(QEMUFile *f, bool block)
{
- if (f->ops->set_blocking) {
- f->ops->set_blocking(f->ioc, block, NULL);
- }
+ qio_channel_set_blocking(f->ioc, block, NULL);
}
/*
diff --git a/migration/qemu-file.h b/migration/qemu-file.h
index 2049dfe7e4..6db4bb9fdb 100644
--- a/migration/qemu-file.h
+++ b/migration/qemu-file.h
@@ -46,10 +46,6 @@ typedef ssize_t (QEMUFileGetBufferFunc)(void *opaque, uint8_t *buf,
*/
typedef int (QEMUFileCloseFunc)(void *opaque, Error **errp);
-/* Called to change the blocking mode of the file
- */
-typedef int (QEMUFileSetBlocking)(void *opaque, bool enabled, Error **errp);
-
/*
* This function writes an iovec to file. The handler must write all
* of the data or return a negative errno value.
@@ -92,7 +88,6 @@ typedef QEMUFile *(QEMURetPathFunc)(void *opaque);
typedef struct QEMUFileOps {
QEMUFileGetBufferFunc *get_buffer;
QEMUFileCloseFunc *close;
- QEMUFileSetBlocking *set_blocking;
QEMUFileWritevBufferFunc *writev_buffer;
QEMURetPathFunc *get_return_path;
} QEMUFileOps;
--
2.36.1
^ permalink raw reply related [flat|nested] 51+ messages in thread* Re: [PATCH 15/20] migration: remove the QEMUFileOps 'set_blocking' callback
2022-05-24 11:02 ` [PATCH 15/20] migration: remove the QEMUFileOps 'set_blocking' callback Daniel P. Berrangé
@ 2022-06-09 16:21 ` Dr. David Alan Gilbert
0 siblings, 0 replies; 51+ messages in thread
From: Dr. David Alan Gilbert @ 2022-06-09 16:21 UTC (permalink / raw)
To: Daniel P. Berrangé
Cc: qemu-devel, qemu-block, Stefan Hajnoczi, Juan Quintela, Fam Zheng
* Daniel P. Berrangé (berrange@redhat.com) wrote:
> This directly implements the set_blocking logic using QIOChannel APIs.
>
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> ---
> migration/qemu-file-channel.c | 14 --------------
> migration/qemu-file.c | 4 +---
> migration/qemu-file.h | 5 -----
> 3 files changed, 1 insertion(+), 22 deletions(-)
>
> diff --git a/migration/qemu-file-channel.c b/migration/qemu-file-channel.c
> index 80f05dc371..0350d367ec 100644
> --- a/migration/qemu-file-channel.c
> +++ b/migration/qemu-file-channel.c
> @@ -112,18 +112,6 @@ static int channel_close(void *opaque, Error **errp)
> }
>
>
> -static int channel_set_blocking(void *opaque,
> - bool enabled,
> - Error **errp)
> -{
> - QIOChannel *ioc = QIO_CHANNEL(opaque);
> -
> - if (qio_channel_set_blocking(ioc, enabled, errp) < 0) {
> - return -1;
> - }
> - return 0;
> -}
> -
> static QEMUFile *channel_get_input_return_path(void *opaque)
> {
> QIOChannel *ioc = QIO_CHANNEL(opaque);
> @@ -141,7 +129,6 @@ static QEMUFile *channel_get_output_return_path(void *opaque)
> static const QEMUFileOps channel_input_ops = {
> .get_buffer = channel_get_buffer,
> .close = channel_close,
> - .set_blocking = channel_set_blocking,
> .get_return_path = channel_get_input_return_path,
> };
>
> @@ -149,7 +136,6 @@ static const QEMUFileOps channel_input_ops = {
> static const QEMUFileOps channel_output_ops = {
> .writev_buffer = channel_writev_buffer,
> .close = channel_close,
> - .set_blocking = channel_set_blocking,
> .get_return_path = channel_get_output_return_path,
> };
>
> diff --git a/migration/qemu-file.c b/migration/qemu-file.c
> index fd9f060c02..171b9f85bf 100644
> --- a/migration/qemu-file.c
> +++ b/migration/qemu-file.c
> @@ -876,9 +876,7 @@ void qemu_put_counted_string(QEMUFile *f, const char *str)
> */
> void qemu_file_set_blocking(QEMUFile *f, bool block)
> {
> - if (f->ops->set_blocking) {
> - f->ops->set_blocking(f->ioc, block, NULL);
> - }
> + qio_channel_set_blocking(f->ioc, block, NULL);
> }
>
> /*
> diff --git a/migration/qemu-file.h b/migration/qemu-file.h
> index 2049dfe7e4..6db4bb9fdb 100644
> --- a/migration/qemu-file.h
> +++ b/migration/qemu-file.h
> @@ -46,10 +46,6 @@ typedef ssize_t (QEMUFileGetBufferFunc)(void *opaque, uint8_t *buf,
> */
> typedef int (QEMUFileCloseFunc)(void *opaque, Error **errp);
>
> -/* Called to change the blocking mode of the file
> - */
> -typedef int (QEMUFileSetBlocking)(void *opaque, bool enabled, Error **errp);
> -
> /*
> * This function writes an iovec to file. The handler must write all
> * of the data or return a negative errno value.
> @@ -92,7 +88,6 @@ typedef QEMUFile *(QEMURetPathFunc)(void *opaque);
> typedef struct QEMUFileOps {
> QEMUFileGetBufferFunc *get_buffer;
> QEMUFileCloseFunc *close;
> - QEMUFileSetBlocking *set_blocking;
> QEMUFileWritevBufferFunc *writev_buffer;
> QEMURetPathFunc *get_return_path;
> } QEMUFileOps;
> --
> 2.36.1
>
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
^ permalink raw reply [flat|nested] 51+ messages in thread
* [PATCH 16/20] migration: remove the QEMUFileOps 'close' callback
2022-05-24 11:02 [PATCH 00/20] migration: remove QEMUFileOps concept and assume use of QIOChannel Daniel P. Berrangé
` (14 preceding siblings ...)
2022-05-24 11:02 ` [PATCH 15/20] migration: remove the QEMUFileOps 'set_blocking' callback Daniel P. Berrangé
@ 2022-05-24 11:02 ` Daniel P. Berrangé
2022-06-09 16:40 ` Dr. David Alan Gilbert
2022-05-24 11:02 ` [PATCH 17/20] migration: remove the QEMUFileOps 'get_buffer' callback Daniel P. Berrangé
` (3 subsequent siblings)
19 siblings, 1 reply; 51+ messages in thread
From: Daniel P. Berrangé @ 2022-05-24 11:02 UTC (permalink / raw)
To: qemu-devel
Cc: Daniel P. Berrangé,
Dr. David Alan Gilbert, qemu-block, Stefan Hajnoczi,
Hailiang Zhang, Juan Quintela, Fam Zheng
This directly implements the close logic using QIOChannel APIs.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
migration/qemu-file-channel.c | 12 ------------
migration/qemu-file.c | 12 ++++++------
migration/qemu-file.h | 10 ----------
3 files changed, 6 insertions(+), 28 deletions(-)
diff --git a/migration/qemu-file-channel.c b/migration/qemu-file-channel.c
index 0350d367ec..8ff58e81f9 100644
--- a/migration/qemu-file-channel.c
+++ b/migration/qemu-file-channel.c
@@ -102,16 +102,6 @@ static ssize_t channel_get_buffer(void *opaque,
}
-static int channel_close(void *opaque, Error **errp)
-{
- int ret;
- QIOChannel *ioc = QIO_CHANNEL(opaque);
- ret = qio_channel_close(ioc, errp);
- object_unref(OBJECT(ioc));
- return ret;
-}
-
-
static QEMUFile *channel_get_input_return_path(void *opaque)
{
QIOChannel *ioc = QIO_CHANNEL(opaque);
@@ -128,14 +118,12 @@ static QEMUFile *channel_get_output_return_path(void *opaque)
static const QEMUFileOps channel_input_ops = {
.get_buffer = channel_get_buffer,
- .close = channel_close,
.get_return_path = channel_get_input_return_path,
};
static const QEMUFileOps channel_output_ops = {
.writev_buffer = channel_writev_buffer,
- .close = channel_close,
.get_return_path = channel_get_output_return_path,
};
diff --git a/migration/qemu-file.c b/migration/qemu-file.c
index 171b9f85bf..a855ce33dc 100644
--- a/migration/qemu-file.c
+++ b/migration/qemu-file.c
@@ -405,16 +405,16 @@ void qemu_file_credit_transfer(QEMUFile *f, size_t size)
*/
int qemu_fclose(QEMUFile *f)
{
- int ret;
+ int ret, ret2;
qemu_fflush(f);
ret = qemu_file_get_error(f);
- if (f->ops->close) {
- int ret2 = f->ops->close(f->ioc, NULL);
- if (ret >= 0) {
- ret = ret2;
- }
+ ret2 = qio_channel_close(f->ioc, NULL);
+ if (ret >= 0) {
+ ret = ret2;
}
+ g_clear_pointer(&f->ioc, object_unref);
+
/* If any error was spotted before closing, we should report it
* instead of the close() return value.
*/
diff --git a/migration/qemu-file.h b/migration/qemu-file.h
index 6db4bb9fdb..7ec105bf96 100644
--- a/migration/qemu-file.h
+++ b/migration/qemu-file.h
@@ -37,15 +37,6 @@ typedef ssize_t (QEMUFileGetBufferFunc)(void *opaque, uint8_t *buf,
int64_t pos, size_t size,
Error **errp);
-/* Close a file
- *
- * Return negative error number on error, 0 or positive value on success.
- *
- * The meaning of return value on success depends on the specific back-end being
- * used.
- */
-typedef int (QEMUFileCloseFunc)(void *opaque, Error **errp);
-
/*
* This function writes an iovec to file. The handler must write all
* of the data or return a negative errno value.
@@ -87,7 +78,6 @@ typedef QEMUFile *(QEMURetPathFunc)(void *opaque);
typedef struct QEMUFileOps {
QEMUFileGetBufferFunc *get_buffer;
- QEMUFileCloseFunc *close;
QEMUFileWritevBufferFunc *writev_buffer;
QEMURetPathFunc *get_return_path;
} QEMUFileOps;
--
2.36.1
^ permalink raw reply related [flat|nested] 51+ messages in thread* Re: [PATCH 16/20] migration: remove the QEMUFileOps 'close' callback
2022-05-24 11:02 ` [PATCH 16/20] migration: remove the QEMUFileOps 'close' callback Daniel P. Berrangé
@ 2022-06-09 16:40 ` Dr. David Alan Gilbert
0 siblings, 0 replies; 51+ messages in thread
From: Dr. David Alan Gilbert @ 2022-06-09 16:40 UTC (permalink / raw)
To: Daniel P. Berrangé
Cc: qemu-devel, qemu-block, Stefan Hajnoczi, Juan Quintela, Fam Zheng
* Daniel P. Berrangé (berrange@redhat.com) wrote:
> This directly implements the close logic using QIOChannel APIs.
>
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> ---
> migration/qemu-file-channel.c | 12 ------------
> migration/qemu-file.c | 12 ++++++------
> migration/qemu-file.h | 10 ----------
> 3 files changed, 6 insertions(+), 28 deletions(-)
>
> diff --git a/migration/qemu-file-channel.c b/migration/qemu-file-channel.c
> index 0350d367ec..8ff58e81f9 100644
> --- a/migration/qemu-file-channel.c
> +++ b/migration/qemu-file-channel.c
> @@ -102,16 +102,6 @@ static ssize_t channel_get_buffer(void *opaque,
> }
>
>
> -static int channel_close(void *opaque, Error **errp)
> -{
> - int ret;
> - QIOChannel *ioc = QIO_CHANNEL(opaque);
> - ret = qio_channel_close(ioc, errp);
> - object_unref(OBJECT(ioc));
> - return ret;
> -}
> -
> -
> static QEMUFile *channel_get_input_return_path(void *opaque)
> {
> QIOChannel *ioc = QIO_CHANNEL(opaque);
> @@ -128,14 +118,12 @@ static QEMUFile *channel_get_output_return_path(void *opaque)
>
> static const QEMUFileOps channel_input_ops = {
> .get_buffer = channel_get_buffer,
> - .close = channel_close,
> .get_return_path = channel_get_input_return_path,
> };
>
>
> static const QEMUFileOps channel_output_ops = {
> .writev_buffer = channel_writev_buffer,
> - .close = channel_close,
> .get_return_path = channel_get_output_return_path,
> };
>
> diff --git a/migration/qemu-file.c b/migration/qemu-file.c
> index 171b9f85bf..a855ce33dc 100644
> --- a/migration/qemu-file.c
> +++ b/migration/qemu-file.c
> @@ -405,16 +405,16 @@ void qemu_file_credit_transfer(QEMUFile *f, size_t size)
> */
> int qemu_fclose(QEMUFile *f)
> {
> - int ret;
> + int ret, ret2;
> qemu_fflush(f);
> ret = qemu_file_get_error(f);
>
> - if (f->ops->close) {
> - int ret2 = f->ops->close(f->ioc, NULL);
> - if (ret >= 0) {
> - ret = ret2;
> - }
> + ret2 = qio_channel_close(f->ioc, NULL);
> + if (ret >= 0) {
> + ret = ret2;
> }
> + g_clear_pointer(&f->ioc, object_unref);
> +
> /* If any error was spotted before closing, we should report it
> * instead of the close() return value.
> */
> diff --git a/migration/qemu-file.h b/migration/qemu-file.h
> index 6db4bb9fdb..7ec105bf96 100644
> --- a/migration/qemu-file.h
> +++ b/migration/qemu-file.h
> @@ -37,15 +37,6 @@ typedef ssize_t (QEMUFileGetBufferFunc)(void *opaque, uint8_t *buf,
> int64_t pos, size_t size,
> Error **errp);
>
> -/* Close a file
> - *
> - * Return negative error number on error, 0 or positive value on success.
> - *
> - * The meaning of return value on success depends on the specific back-end being
> - * used.
> - */
> -typedef int (QEMUFileCloseFunc)(void *opaque, Error **errp);
> -
> /*
> * This function writes an iovec to file. The handler must write all
> * of the data or return a negative errno value.
> @@ -87,7 +78,6 @@ typedef QEMUFile *(QEMURetPathFunc)(void *opaque);
>
> typedef struct QEMUFileOps {
> QEMUFileGetBufferFunc *get_buffer;
> - QEMUFileCloseFunc *close;
> QEMUFileWritevBufferFunc *writev_buffer;
> QEMURetPathFunc *get_return_path;
> } QEMUFileOps;
> --
> 2.36.1
>
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
^ permalink raw reply [flat|nested] 51+ messages in thread
* [PATCH 17/20] migration: remove the QEMUFileOps 'get_buffer' callback
2022-05-24 11:02 [PATCH 00/20] migration: remove QEMUFileOps concept and assume use of QIOChannel Daniel P. Berrangé
` (15 preceding siblings ...)
2022-05-24 11:02 ` [PATCH 16/20] migration: remove the QEMUFileOps 'close' callback Daniel P. Berrangé
@ 2022-05-24 11:02 ` Daniel P. Berrangé
2022-06-09 16:46 ` Dr. David Alan Gilbert
2022-05-24 11:02 ` [PATCH 18/20] migration: remove the QEMUFileOps 'writev_buffer' callback Daniel P. Berrangé
` (2 subsequent siblings)
19 siblings, 1 reply; 51+ messages in thread
From: Daniel P. Berrangé @ 2022-05-24 11:02 UTC (permalink / raw)
To: qemu-devel
Cc: Daniel P. Berrangé,
Dr. David Alan Gilbert, qemu-block, Stefan Hajnoczi,
Hailiang Zhang, Juan Quintela, Fam Zheng
This directly implements the get_buffer logic using QIOChannel APIs.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
migration/qemu-file-channel.c | 29 -----------------------------
migration/qemu-file.c | 18 ++++++++++++++++--
migration/qemu-file.h | 9 ---------
3 files changed, 16 insertions(+), 40 deletions(-)
diff --git a/migration/qemu-file-channel.c b/migration/qemu-file-channel.c
index 8ff58e81f9..7b32831752 100644
--- a/migration/qemu-file-channel.c
+++ b/migration/qemu-file-channel.c
@@ -74,34 +74,6 @@ static ssize_t channel_writev_buffer(void *opaque,
}
-static ssize_t channel_get_buffer(void *opaque,
- uint8_t *buf,
- int64_t pos,
- size_t size,
- Error **errp)
-{
- QIOChannel *ioc = QIO_CHANNEL(opaque);
- ssize_t ret;
-
- do {
- ret = qio_channel_read(ioc, (char *)buf, size, errp);
- if (ret < 0) {
- if (ret == QIO_CHANNEL_ERR_BLOCK) {
- if (qemu_in_coroutine()) {
- qio_channel_yield(ioc, G_IO_IN);
- } else {
- qio_channel_wait(ioc, G_IO_IN);
- }
- } else {
- return -EIO;
- }
- }
- } while (ret == QIO_CHANNEL_ERR_BLOCK);
-
- return ret;
-}
-
-
static QEMUFile *channel_get_input_return_path(void *opaque)
{
QIOChannel *ioc = QIO_CHANNEL(opaque);
@@ -117,7 +89,6 @@ static QEMUFile *channel_get_output_return_path(void *opaque)
}
static const QEMUFileOps channel_input_ops = {
- .get_buffer = channel_get_buffer,
.get_return_path = channel_get_input_return_path,
};
diff --git a/migration/qemu-file.c b/migration/qemu-file.c
index a855ce33dc..e024b43851 100644
--- a/migration/qemu-file.c
+++ b/migration/qemu-file.c
@@ -374,8 +374,22 @@ static ssize_t qemu_fill_buffer(QEMUFile *f)
return 0;
}
- len = f->ops->get_buffer(f->ioc, f->buf + pending, f->total_transferred,
- IO_BUF_SIZE - pending, &local_error);
+ do {
+ len = qio_channel_read(f->ioc,
+ (char *)f->buf + pending,
+ IO_BUF_SIZE - pending,
+ &local_error);
+ if (len == QIO_CHANNEL_ERR_BLOCK) {
+ if (qemu_in_coroutine()) {
+ qio_channel_yield(f->ioc, G_IO_IN);
+ } else {
+ qio_channel_wait(f->ioc, G_IO_IN);
+ }
+ } else if (len < 0) {
+ len = EIO;
+ }
+ } while (len == QIO_CHANNEL_ERR_BLOCK);
+
if (len > 0) {
f->buf_size += len;
f->total_transferred += len;
diff --git a/migration/qemu-file.h b/migration/qemu-file.h
index 7ec105bf96..cd49184c00 100644
--- a/migration/qemu-file.h
+++ b/migration/qemu-file.h
@@ -29,14 +29,6 @@
#include "exec/cpu-common.h"
#include "io/channel.h"
-/* Read a chunk of data from a file at the given position. The pos argument
- * can be ignored if the file is only be used for streaming. The number of
- * bytes actually read should be returned.
- */
-typedef ssize_t (QEMUFileGetBufferFunc)(void *opaque, uint8_t *buf,
- int64_t pos, size_t size,
- Error **errp);
-
/*
* This function writes an iovec to file. The handler must write all
* of the data or return a negative errno value.
@@ -77,7 +69,6 @@ typedef size_t (QEMURamSaveFunc)(QEMUFile *f,
typedef QEMUFile *(QEMURetPathFunc)(void *opaque);
typedef struct QEMUFileOps {
- QEMUFileGetBufferFunc *get_buffer;
QEMUFileWritevBufferFunc *writev_buffer;
QEMURetPathFunc *get_return_path;
} QEMUFileOps;
--
2.36.1
^ permalink raw reply related [flat|nested] 51+ messages in thread* Re: [PATCH 17/20] migration: remove the QEMUFileOps 'get_buffer' callback
2022-05-24 11:02 ` [PATCH 17/20] migration: remove the QEMUFileOps 'get_buffer' callback Daniel P. Berrangé
@ 2022-06-09 16:46 ` Dr. David Alan Gilbert
2022-06-09 17:09 ` Daniel P. Berrangé
0 siblings, 1 reply; 51+ messages in thread
From: Dr. David Alan Gilbert @ 2022-06-09 16:46 UTC (permalink / raw)
To: Daniel P. Berrangé
Cc: qemu-devel, qemu-block, Stefan Hajnoczi, Hailiang Zhang,
Juan Quintela, Fam Zheng
* Daniel P. Berrangé (berrange@redhat.com) wrote:
> This directly implements the get_buffer logic using QIOChannel APIs.
>
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
> migration/qemu-file-channel.c | 29 -----------------------------
> migration/qemu-file.c | 18 ++++++++++++++++--
> migration/qemu-file.h | 9 ---------
> 3 files changed, 16 insertions(+), 40 deletions(-)
>
> diff --git a/migration/qemu-file-channel.c b/migration/qemu-file-channel.c
> index 8ff58e81f9..7b32831752 100644
> --- a/migration/qemu-file-channel.c
> +++ b/migration/qemu-file-channel.c
> @@ -74,34 +74,6 @@ static ssize_t channel_writev_buffer(void *opaque,
> }
>
>
> -static ssize_t channel_get_buffer(void *opaque,
> - uint8_t *buf,
> - int64_t pos,
> - size_t size,
> - Error **errp)
> -{
> - QIOChannel *ioc = QIO_CHANNEL(opaque);
> - ssize_t ret;
> -
> - do {
> - ret = qio_channel_read(ioc, (char *)buf, size, errp);
> - if (ret < 0) {
> - if (ret == QIO_CHANNEL_ERR_BLOCK) {
> - if (qemu_in_coroutine()) {
> - qio_channel_yield(ioc, G_IO_IN);
> - } else {
> - qio_channel_wait(ioc, G_IO_IN);
> - }
> - } else {
> - return -EIO;
> - }
> - }
> - } while (ret == QIO_CHANNEL_ERR_BLOCK);
> -
> - return ret;
> -}
> -
> -
> static QEMUFile *channel_get_input_return_path(void *opaque)
> {
> QIOChannel *ioc = QIO_CHANNEL(opaque);
> @@ -117,7 +89,6 @@ static QEMUFile *channel_get_output_return_path(void *opaque)
> }
>
> static const QEMUFileOps channel_input_ops = {
> - .get_buffer = channel_get_buffer,
> .get_return_path = channel_get_input_return_path,
> };
>
> diff --git a/migration/qemu-file.c b/migration/qemu-file.c
> index a855ce33dc..e024b43851 100644
> --- a/migration/qemu-file.c
> +++ b/migration/qemu-file.c
> @@ -374,8 +374,22 @@ static ssize_t qemu_fill_buffer(QEMUFile *f)
> return 0;
> }
>
> - len = f->ops->get_buffer(f->ioc, f->buf + pending, f->total_transferred,
> - IO_BUF_SIZE - pending, &local_error);
> + do {
> + len = qio_channel_read(f->ioc,
Yes, I think that's OK - not that 'len' is an int where 'ret'
was a ssize_t; but I think our buffers are guranteed to be 'small'.
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> + (char *)f->buf + pending,
> + IO_BUF_SIZE - pending,
> + &local_error);
> + if (len == QIO_CHANNEL_ERR_BLOCK) {
> + if (qemu_in_coroutine()) {
> + qio_channel_yield(f->ioc, G_IO_IN);
> + } else {
> + qio_channel_wait(f->ioc, G_IO_IN);
> + }
> + } else if (len < 0) {
> + len = EIO;
> + }
> + } while (len == QIO_CHANNEL_ERR_BLOCK);
> +
> if (len > 0) {
> f->buf_size += len;
> f->total_transferred += len;
> diff --git a/migration/qemu-file.h b/migration/qemu-file.h
> index 7ec105bf96..cd49184c00 100644
> --- a/migration/qemu-file.h
> +++ b/migration/qemu-file.h
> @@ -29,14 +29,6 @@
> #include "exec/cpu-common.h"
> #include "io/channel.h"
>
> -/* Read a chunk of data from a file at the given position. The pos argument
> - * can be ignored if the file is only be used for streaming. The number of
> - * bytes actually read should be returned.
> - */
> -typedef ssize_t (QEMUFileGetBufferFunc)(void *opaque, uint8_t *buf,
> - int64_t pos, size_t size,
> - Error **errp);
> -
> /*
> * This function writes an iovec to file. The handler must write all
> * of the data or return a negative errno value.
> @@ -77,7 +69,6 @@ typedef size_t (QEMURamSaveFunc)(QEMUFile *f,
> typedef QEMUFile *(QEMURetPathFunc)(void *opaque);
>
> typedef struct QEMUFileOps {
> - QEMUFileGetBufferFunc *get_buffer;
> QEMUFileWritevBufferFunc *writev_buffer;
> QEMURetPathFunc *get_return_path;
> } QEMUFileOps;
> --
> 2.36.1
>
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
^ permalink raw reply [flat|nested] 51+ messages in thread* Re: [PATCH 17/20] migration: remove the QEMUFileOps 'get_buffer' callback
2022-06-09 16:46 ` Dr. David Alan Gilbert
@ 2022-06-09 17:09 ` Daniel P. Berrangé
0 siblings, 0 replies; 51+ messages in thread
From: Daniel P. Berrangé @ 2022-06-09 17:09 UTC (permalink / raw)
To: Dr. David Alan Gilbert
Cc: qemu-devel, qemu-block, Stefan Hajnoczi, Hailiang Zhang,
Juan Quintela, Fam Zheng
On Thu, Jun 09, 2022 at 05:46:29PM +0100, Dr. David Alan Gilbert wrote:
> * Daniel P. Berrangé (berrange@redhat.com) wrote:
> > This directly implements the get_buffer logic using QIOChannel APIs.
> >
> > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> > ---
> > migration/qemu-file-channel.c | 29 -----------------------------
> > migration/qemu-file.c | 18 ++++++++++++++++--
> > migration/qemu-file.h | 9 ---------
> > 3 files changed, 16 insertions(+), 40 deletions(-)
> >
> > diff --git a/migration/qemu-file-channel.c b/migration/qemu-file-channel.c
> > index 8ff58e81f9..7b32831752 100644
> > --- a/migration/qemu-file-channel.c
> > +++ b/migration/qemu-file-channel.c
> > @@ -74,34 +74,6 @@ static ssize_t channel_writev_buffer(void *opaque,
> > }
> >
> >
> > -static ssize_t channel_get_buffer(void *opaque,
> > - uint8_t *buf,
> > - int64_t pos,
> > - size_t size,
> > - Error **errp)
> > -{
> > - QIOChannel *ioc = QIO_CHANNEL(opaque);
> > - ssize_t ret;
> > -
> > - do {
> > - ret = qio_channel_read(ioc, (char *)buf, size, errp);
> > - if (ret < 0) {
> > - if (ret == QIO_CHANNEL_ERR_BLOCK) {
> > - if (qemu_in_coroutine()) {
> > - qio_channel_yield(ioc, G_IO_IN);
> > - } else {
> > - qio_channel_wait(ioc, G_IO_IN);
> > - }
> > - } else {
> > - return -EIO;
> > - }
> > - }
> > - } while (ret == QIO_CHANNEL_ERR_BLOCK);
> > -
> > - return ret;
> > -}
> > -
> > -
> > static QEMUFile *channel_get_input_return_path(void *opaque)
> > {
> > QIOChannel *ioc = QIO_CHANNEL(opaque);
> > @@ -117,7 +89,6 @@ static QEMUFile *channel_get_output_return_path(void *opaque)
> > }
> >
> > static const QEMUFileOps channel_input_ops = {
> > - .get_buffer = channel_get_buffer,
> > .get_return_path = channel_get_input_return_path,
> > };
> >
> > diff --git a/migration/qemu-file.c b/migration/qemu-file.c
> > index a855ce33dc..e024b43851 100644
> > --- a/migration/qemu-file.c
> > +++ b/migration/qemu-file.c
> > @@ -374,8 +374,22 @@ static ssize_t qemu_fill_buffer(QEMUFile *f)
> > return 0;
> > }
> >
> > - len = f->ops->get_buffer(f->ioc, f->buf + pending, f->total_transferred,
> > - IO_BUF_SIZE - pending, &local_error);
> > + do {
> > + len = qio_channel_read(f->ioc,
>
> Yes, I think that's OK - not that 'len' is an int where 'ret'
> was a ssize_t; but I think our buffers are guranteed to be 'small'.
There are a few places in qemu-file.c where we're fast & loose
with int rather than size_t, that are probably worth cleaning.
> Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
With regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
^ permalink raw reply [flat|nested] 51+ messages in thread
* [PATCH 18/20] migration: remove the QEMUFileOps 'writev_buffer' callback
2022-05-24 11:02 [PATCH 00/20] migration: remove QEMUFileOps concept and assume use of QIOChannel Daniel P. Berrangé
` (16 preceding siblings ...)
2022-05-24 11:02 ` [PATCH 17/20] migration: remove the QEMUFileOps 'get_buffer' callback Daniel P. Berrangé
@ 2022-05-24 11:02 ` Daniel P. Berrangé
2022-06-09 16:51 ` Dr. David Alan Gilbert
2022-05-24 11:02 ` [PATCH 19/20] migration: remove the QEMUFileOps 'get_return_path' callback Daniel P. Berrangé
2022-05-24 11:02 ` [PATCH 20/20] migration: remove the QEMUFileOps abstraction Daniel P. Berrangé
19 siblings, 1 reply; 51+ messages in thread
From: Daniel P. Berrangé @ 2022-05-24 11:02 UTC (permalink / raw)
To: qemu-devel
Cc: Daniel P. Berrangé,
Dr. David Alan Gilbert, qemu-block, Stefan Hajnoczi,
Hailiang Zhang, Juan Quintela, Fam Zheng
This directly implements the writev_buffer logic using QIOChannel APIs.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
migration/qemu-file-channel.c | 43 -----------------------------------
migration/qemu-file.c | 24 +++++++------------
migration/qemu-file.h | 9 --------
3 files changed, 8 insertions(+), 68 deletions(-)
diff --git a/migration/qemu-file-channel.c b/migration/qemu-file-channel.c
index 7b32831752..2e139f7bcd 100644
--- a/migration/qemu-file-channel.c
+++ b/migration/qemu-file-channel.c
@@ -32,48 +32,6 @@
#include "yank_functions.h"
-static ssize_t channel_writev_buffer(void *opaque,
- struct iovec *iov,
- int iovcnt,
- int64_t pos,
- Error **errp)
-{
- QIOChannel *ioc = QIO_CHANNEL(opaque);
- ssize_t done = 0;
- struct iovec *local_iov = g_new(struct iovec, iovcnt);
- struct iovec *local_iov_head = local_iov;
- unsigned int nlocal_iov = iovcnt;
-
- nlocal_iov = iov_copy(local_iov, nlocal_iov,
- iov, iovcnt,
- 0, iov_size(iov, iovcnt));
-
- while (nlocal_iov > 0) {
- ssize_t len;
- len = qio_channel_writev(ioc, local_iov, nlocal_iov, errp);
- if (len == QIO_CHANNEL_ERR_BLOCK) {
- if (qemu_in_coroutine()) {
- qio_channel_yield(ioc, G_IO_OUT);
- } else {
- qio_channel_wait(ioc, G_IO_OUT);
- }
- continue;
- }
- if (len < 0) {
- done = -EIO;
- goto cleanup;
- }
-
- iov_discard_front(&local_iov, &nlocal_iov, len);
- done += len;
- }
-
- cleanup:
- g_free(local_iov_head);
- return done;
-}
-
-
static QEMUFile *channel_get_input_return_path(void *opaque)
{
QIOChannel *ioc = QIO_CHANNEL(opaque);
@@ -94,7 +52,6 @@ static const QEMUFileOps channel_input_ops = {
static const QEMUFileOps channel_output_ops = {
- .writev_buffer = channel_writev_buffer,
.get_return_path = channel_get_output_return_path,
};
diff --git a/migration/qemu-file.c b/migration/qemu-file.c
index e024b43851..72a6f58af5 100644
--- a/migration/qemu-file.c
+++ b/migration/qemu-file.c
@@ -245,10 +245,6 @@ static void qemu_iovec_release_ram(QEMUFile *f)
*/
void qemu_fflush(QEMUFile *f)
{
- ssize_t ret = 0;
- ssize_t expect = 0;
- Error *local_error = NULL;
-
if (!qemu_file_is_writable(f)) {
return;
}
@@ -257,22 +253,18 @@ void qemu_fflush(QEMUFile *f)
return;
}
if (f->iovcnt > 0) {
- expect = iov_size(f->iov, f->iovcnt);
- ret = f->ops->writev_buffer(f->ioc, f->iov, f->iovcnt, f->total_transferred,
- &local_error);
+ Error *local_error = NULL;
+ if (qio_channel_writev_all(f->ioc,
+ f->iov, f->iovcnt,
+ &local_error) < 0) {
+ qemu_file_set_error_obj(f, -EIO, local_error);
+ } else {
+ f->total_transferred += iov_size(f->iov, f->iovcnt);
+ }
qemu_iovec_release_ram(f);
}
- if (ret >= 0) {
- f->total_transferred += ret;
- }
- /* We expect the QEMUFile write impl to send the full
- * data set we requested, so sanity check that.
- */
- if (ret != expect) {
- qemu_file_set_error_obj(f, ret < 0 ? ret : -EIO, local_error);
- }
f->buf_index = 0;
f->iovcnt = 0;
}
diff --git a/migration/qemu-file.h b/migration/qemu-file.h
index cd49184c00..542c637934 100644
--- a/migration/qemu-file.h
+++ b/migration/qemu-file.h
@@ -29,14 +29,6 @@
#include "exec/cpu-common.h"
#include "io/channel.h"
-/*
- * This function writes an iovec to file. The handler must write all
- * of the data or return a negative errno value.
- */
-typedef ssize_t (QEMUFileWritevBufferFunc)(void *opaque, struct iovec *iov,
- int iovcnt, int64_t pos,
- Error **errp);
-
/*
* This function provides hooks around different
* stages of RAM migration.
@@ -69,7 +61,6 @@ typedef size_t (QEMURamSaveFunc)(QEMUFile *f,
typedef QEMUFile *(QEMURetPathFunc)(void *opaque);
typedef struct QEMUFileOps {
- QEMUFileWritevBufferFunc *writev_buffer;
QEMURetPathFunc *get_return_path;
} QEMUFileOps;
--
2.36.1
^ permalink raw reply related [flat|nested] 51+ messages in thread* Re: [PATCH 18/20] migration: remove the QEMUFileOps 'writev_buffer' callback
2022-05-24 11:02 ` [PATCH 18/20] migration: remove the QEMUFileOps 'writev_buffer' callback Daniel P. Berrangé
@ 2022-06-09 16:51 ` Dr. David Alan Gilbert
0 siblings, 0 replies; 51+ messages in thread
From: Dr. David Alan Gilbert @ 2022-06-09 16:51 UTC (permalink / raw)
To: Daniel P. Berrangé
Cc: qemu-devel, qemu-block, Stefan Hajnoczi, Hailiang Zhang,
Juan Quintela, Fam Zheng
* Daniel P. Berrangé (berrange@redhat.com) wrote:
> This directly implements the writev_buffer logic using QIOChannel APIs.
>
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
> migration/qemu-file-channel.c | 43 -----------------------------------
> migration/qemu-file.c | 24 +++++++------------
> migration/qemu-file.h | 9 --------
> 3 files changed, 8 insertions(+), 68 deletions(-)
>
> diff --git a/migration/qemu-file-channel.c b/migration/qemu-file-channel.c
> index 7b32831752..2e139f7bcd 100644
> --- a/migration/qemu-file-channel.c
> +++ b/migration/qemu-file-channel.c
> @@ -32,48 +32,6 @@
> #include "yank_functions.h"
>
>
> -static ssize_t channel_writev_buffer(void *opaque,
> - struct iovec *iov,
> - int iovcnt,
> - int64_t pos,
> - Error **errp)
> -{
> - QIOChannel *ioc = QIO_CHANNEL(opaque);
> - ssize_t done = 0;
> - struct iovec *local_iov = g_new(struct iovec, iovcnt);
> - struct iovec *local_iov_head = local_iov;
> - unsigned int nlocal_iov = iovcnt;
> -
> - nlocal_iov = iov_copy(local_iov, nlocal_iov,
> - iov, iovcnt,
> - 0, iov_size(iov, iovcnt));
> -
> - while (nlocal_iov > 0) {
> - ssize_t len;
> - len = qio_channel_writev(ioc, local_iov, nlocal_iov, errp);
> - if (len == QIO_CHANNEL_ERR_BLOCK) {
> - if (qemu_in_coroutine()) {
> - qio_channel_yield(ioc, G_IO_OUT);
> - } else {
> - qio_channel_wait(ioc, G_IO_OUT);
> - }
> - continue;
I wondered where that code went, but it turns out it's already copied
into qio_channel_writev_full_all, so:
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> - }
> - if (len < 0) {
> - done = -EIO;
> - goto cleanup;
> - }
> -
> - iov_discard_front(&local_iov, &nlocal_iov, len);
> - done += len;
> - }
> -
> - cleanup:
> - g_free(local_iov_head);
> - return done;
> -}
> -
> -
> static QEMUFile *channel_get_input_return_path(void *opaque)
> {
> QIOChannel *ioc = QIO_CHANNEL(opaque);
> @@ -94,7 +52,6 @@ static const QEMUFileOps channel_input_ops = {
>
>
> static const QEMUFileOps channel_output_ops = {
> - .writev_buffer = channel_writev_buffer,
> .get_return_path = channel_get_output_return_path,
> };
>
> diff --git a/migration/qemu-file.c b/migration/qemu-file.c
> index e024b43851..72a6f58af5 100644
> --- a/migration/qemu-file.c
> +++ b/migration/qemu-file.c
> @@ -245,10 +245,6 @@ static void qemu_iovec_release_ram(QEMUFile *f)
> */
> void qemu_fflush(QEMUFile *f)
> {
> - ssize_t ret = 0;
> - ssize_t expect = 0;
> - Error *local_error = NULL;
> -
> if (!qemu_file_is_writable(f)) {
> return;
> }
> @@ -257,22 +253,18 @@ void qemu_fflush(QEMUFile *f)
> return;
> }
> if (f->iovcnt > 0) {
> - expect = iov_size(f->iov, f->iovcnt);
> - ret = f->ops->writev_buffer(f->ioc, f->iov, f->iovcnt, f->total_transferred,
> - &local_error);
> + Error *local_error = NULL;
> + if (qio_channel_writev_all(f->ioc,
> + f->iov, f->iovcnt,
> + &local_error) < 0) {
> + qemu_file_set_error_obj(f, -EIO, local_error);
> + } else {
> + f->total_transferred += iov_size(f->iov, f->iovcnt);
> + }
>
> qemu_iovec_release_ram(f);
> }
>
> - if (ret >= 0) {
> - f->total_transferred += ret;
> - }
> - /* We expect the QEMUFile write impl to send the full
> - * data set we requested, so sanity check that.
> - */
> - if (ret != expect) {
> - qemu_file_set_error_obj(f, ret < 0 ? ret : -EIO, local_error);
> - }
> f->buf_index = 0;
> f->iovcnt = 0;
> }
> diff --git a/migration/qemu-file.h b/migration/qemu-file.h
> index cd49184c00..542c637934 100644
> --- a/migration/qemu-file.h
> +++ b/migration/qemu-file.h
> @@ -29,14 +29,6 @@
> #include "exec/cpu-common.h"
> #include "io/channel.h"
>
> -/*
> - * This function writes an iovec to file. The handler must write all
> - * of the data or return a negative errno value.
> - */
> -typedef ssize_t (QEMUFileWritevBufferFunc)(void *opaque, struct iovec *iov,
> - int iovcnt, int64_t pos,
> - Error **errp);
> -
> /*
> * This function provides hooks around different
> * stages of RAM migration.
> @@ -69,7 +61,6 @@ typedef size_t (QEMURamSaveFunc)(QEMUFile *f,
> typedef QEMUFile *(QEMURetPathFunc)(void *opaque);
>
> typedef struct QEMUFileOps {
> - QEMUFileWritevBufferFunc *writev_buffer;
> QEMURetPathFunc *get_return_path;
> } QEMUFileOps;
>
> --
> 2.36.1
>
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
^ permalink raw reply [flat|nested] 51+ messages in thread
* [PATCH 19/20] migration: remove the QEMUFileOps 'get_return_path' callback
2022-05-24 11:02 [PATCH 00/20] migration: remove QEMUFileOps concept and assume use of QIOChannel Daniel P. Berrangé
` (17 preceding siblings ...)
2022-05-24 11:02 ` [PATCH 18/20] migration: remove the QEMUFileOps 'writev_buffer' callback Daniel P. Berrangé
@ 2022-05-24 11:02 ` Daniel P. Berrangé
2022-06-09 16:54 ` Dr. David Alan Gilbert
2022-05-24 11:02 ` [PATCH 20/20] migration: remove the QEMUFileOps abstraction Daniel P. Berrangé
19 siblings, 1 reply; 51+ messages in thread
From: Daniel P. Berrangé @ 2022-05-24 11:02 UTC (permalink / raw)
To: qemu-devel
Cc: Daniel P. Berrangé,
Dr. David Alan Gilbert, qemu-block, Stefan Hajnoczi,
Hailiang Zhang, Juan Quintela, Fam Zheng
This directly implements the get_return_path logic using QIOChannel APIs.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
migration/qemu-file-channel.c | 16 ----------------
migration/qemu-file.c | 22 ++++++++++------------
migration/qemu-file.h | 6 ------
3 files changed, 10 insertions(+), 34 deletions(-)
diff --git a/migration/qemu-file-channel.c b/migration/qemu-file-channel.c
index 2e139f7bcd..51717c1137 100644
--- a/migration/qemu-file-channel.c
+++ b/migration/qemu-file-channel.c
@@ -32,27 +32,11 @@
#include "yank_functions.h"
-static QEMUFile *channel_get_input_return_path(void *opaque)
-{
- QIOChannel *ioc = QIO_CHANNEL(opaque);
-
- return qemu_fopen_channel_output(ioc);
-}
-
-static QEMUFile *channel_get_output_return_path(void *opaque)
-{
- QIOChannel *ioc = QIO_CHANNEL(opaque);
-
- return qemu_fopen_channel_input(ioc);
-}
-
static const QEMUFileOps channel_input_ops = {
- .get_return_path = channel_get_input_return_path,
};
static const QEMUFileOps channel_output_ops = {
- .get_return_path = channel_get_output_return_path,
};
diff --git a/migration/qemu-file.c b/migration/qemu-file.c
index 72a6f58af5..dfee808924 100644
--- a/migration/qemu-file.c
+++ b/migration/qemu-file.c
@@ -92,18 +92,6 @@ int qemu_file_shutdown(QEMUFile *f)
return ret;
}
-/*
- * Result: QEMUFile* for a 'return path' for comms in the opposite direction
- * NULL if not available
- */
-QEMUFile *qemu_file_get_return_path(QEMUFile *f)
-{
- if (!f->ops->get_return_path) {
- return NULL;
- }
- return f->ops->get_return_path(f->ioc);
-}
-
bool qemu_file_mode_is_not_valid(const char *mode)
{
if (mode == NULL ||
@@ -131,6 +119,16 @@ static QEMUFile *qemu_file_new_impl(QIOChannel *ioc,
return f;
}
+/*
+ * Result: QEMUFile* for a 'return path' for comms in the opposite direction
+ * NULL if not available
+ */
+QEMUFile *qemu_file_get_return_path(QEMUFile *f)
+{
+ object_ref(f->ioc);
+ return qemu_file_new_impl(f->ioc, f->ops, !f->is_writable);
+}
+
QEMUFile *qemu_file_new_output(QIOChannel *ioc, const QEMUFileOps *ops)
{
return qemu_file_new_impl(ioc, ops, true);
diff --git a/migration/qemu-file.h b/migration/qemu-file.h
index 542c637934..5370e4e5ec 100644
--- a/migration/qemu-file.h
+++ b/migration/qemu-file.h
@@ -55,13 +55,7 @@ typedef size_t (QEMURamSaveFunc)(QEMUFile *f,
size_t size,
uint64_t *bytes_sent);
-/*
- * Return a QEMUFile for comms in the opposite direction
- */
-typedef QEMUFile *(QEMURetPathFunc)(void *opaque);
-
typedef struct QEMUFileOps {
- QEMURetPathFunc *get_return_path;
} QEMUFileOps;
typedef struct QEMUFileHooks {
--
2.36.1
^ permalink raw reply related [flat|nested] 51+ messages in thread* Re: [PATCH 19/20] migration: remove the QEMUFileOps 'get_return_path' callback
2022-05-24 11:02 ` [PATCH 19/20] migration: remove the QEMUFileOps 'get_return_path' callback Daniel P. Berrangé
@ 2022-06-09 16:54 ` Dr. David Alan Gilbert
0 siblings, 0 replies; 51+ messages in thread
From: Dr. David Alan Gilbert @ 2022-06-09 16:54 UTC (permalink / raw)
To: Daniel P. Berrangé
Cc: qemu-devel, qemu-block, Stefan Hajnoczi, Hailiang Zhang,
Juan Quintela, Fam Zheng
* Daniel P. Berrangé (berrange@redhat.com) wrote:
> This directly implements the get_return_path logic using QIOChannel APIs.
>
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> ---
> migration/qemu-file-channel.c | 16 ----------------
> migration/qemu-file.c | 22 ++++++++++------------
> migration/qemu-file.h | 6 ------
> 3 files changed, 10 insertions(+), 34 deletions(-)
>
> diff --git a/migration/qemu-file-channel.c b/migration/qemu-file-channel.c
> index 2e139f7bcd..51717c1137 100644
> --- a/migration/qemu-file-channel.c
> +++ b/migration/qemu-file-channel.c
> @@ -32,27 +32,11 @@
> #include "yank_functions.h"
>
>
> -static QEMUFile *channel_get_input_return_path(void *opaque)
> -{
> - QIOChannel *ioc = QIO_CHANNEL(opaque);
> -
> - return qemu_fopen_channel_output(ioc);
> -}
> -
> -static QEMUFile *channel_get_output_return_path(void *opaque)
> -{
> - QIOChannel *ioc = QIO_CHANNEL(opaque);
> -
> - return qemu_fopen_channel_input(ioc);
> -}
> -
> static const QEMUFileOps channel_input_ops = {
> - .get_return_path = channel_get_input_return_path,
> };
>
>
> static const QEMUFileOps channel_output_ops = {
> - .get_return_path = channel_get_output_return_path,
> };
>
>
> diff --git a/migration/qemu-file.c b/migration/qemu-file.c
> index 72a6f58af5..dfee808924 100644
> --- a/migration/qemu-file.c
> +++ b/migration/qemu-file.c
> @@ -92,18 +92,6 @@ int qemu_file_shutdown(QEMUFile *f)
> return ret;
> }
>
> -/*
> - * Result: QEMUFile* for a 'return path' for comms in the opposite direction
> - * NULL if not available
> - */
> -QEMUFile *qemu_file_get_return_path(QEMUFile *f)
> -{
> - if (!f->ops->get_return_path) {
> - return NULL;
> - }
> - return f->ops->get_return_path(f->ioc);
> -}
> -
> bool qemu_file_mode_is_not_valid(const char *mode)
> {
> if (mode == NULL ||
> @@ -131,6 +119,16 @@ static QEMUFile *qemu_file_new_impl(QIOChannel *ioc,
> return f;
> }
>
> +/*
> + * Result: QEMUFile* for a 'return path' for comms in the opposite direction
> + * NULL if not available
> + */
> +QEMUFile *qemu_file_get_return_path(QEMUFile *f)
> +{
> + object_ref(f->ioc);
> + return qemu_file_new_impl(f->ioc, f->ops, !f->is_writable);
> +}
> +
> QEMUFile *qemu_file_new_output(QIOChannel *ioc, const QEMUFileOps *ops)
> {
> return qemu_file_new_impl(ioc, ops, true);
> diff --git a/migration/qemu-file.h b/migration/qemu-file.h
> index 542c637934..5370e4e5ec 100644
> --- a/migration/qemu-file.h
> +++ b/migration/qemu-file.h
> @@ -55,13 +55,7 @@ typedef size_t (QEMURamSaveFunc)(QEMUFile *f,
> size_t size,
> uint64_t *bytes_sent);
>
> -/*
> - * Return a QEMUFile for comms in the opposite direction
> - */
> -typedef QEMUFile *(QEMURetPathFunc)(void *opaque);
> -
> typedef struct QEMUFileOps {
> - QEMURetPathFunc *get_return_path;
> } QEMUFileOps;
>
> typedef struct QEMUFileHooks {
> --
> 2.36.1
>
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
^ permalink raw reply [flat|nested] 51+ messages in thread
* [PATCH 20/20] migration: remove the QEMUFileOps abstraction
2022-05-24 11:02 [PATCH 00/20] migration: remove QEMUFileOps concept and assume use of QIOChannel Daniel P. Berrangé
` (18 preceding siblings ...)
2022-05-24 11:02 ` [PATCH 19/20] migration: remove the QEMUFileOps 'get_return_path' callback Daniel P. Berrangé
@ 2022-05-24 11:02 ` Daniel P. Berrangé
2022-06-09 16:59 ` Dr. David Alan Gilbert
19 siblings, 1 reply; 51+ messages in thread
From: Daniel P. Berrangé @ 2022-05-24 11:02 UTC (permalink / raw)
To: qemu-devel
Cc: Daniel P. Berrangé,
Dr. David Alan Gilbert, qemu-block, Stefan Hajnoczi,
Hailiang Zhang, Juan Quintela, Fam Zheng
Now that all QEMUFile callbacks are removed, the entire concept can be
deleted.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
migration/channel.c | 4 +--
migration/colo.c | 5 ++--
migration/meson.build | 1 -
migration/migration.c | 7 ++---
migration/qemu-file-channel.c | 53 -----------------------------------
migration/qemu-file-channel.h | 32 ---------------------
migration/qemu-file.c | 20 ++++++-------
migration/qemu-file.h | 8 ++----
migration/ram.c | 3 +-
migration/rdma.c | 5 ++--
migration/savevm.c | 11 ++++----
tests/unit/test-vmstate.c | 5 ++--
12 files changed, 27 insertions(+), 127 deletions(-)
delete mode 100644 migration/qemu-file-channel.c
delete mode 100644 migration/qemu-file-channel.h
diff --git a/migration/channel.c b/migration/channel.c
index a162d00fea..90087d8986 100644
--- a/migration/channel.c
+++ b/migration/channel.c
@@ -14,7 +14,7 @@
#include "channel.h"
#include "tls.h"
#include "migration.h"
-#include "qemu-file-channel.h"
+#include "qemu-file.h"
#include "trace.h"
#include "qapi/error.h"
#include "io/channel-tls.h"
@@ -85,7 +85,7 @@ void migration_channel_connect(MigrationState *s,
return;
}
} else {
- QEMUFile *f = qemu_fopen_channel_output(ioc);
+ QEMUFile *f = qemu_file_new_output(ioc);
migration_ioc_register_yank(ioc);
diff --git a/migration/colo.c b/migration/colo.c
index 5f7071b3cd..2b71722fd6 100644
--- a/migration/colo.c
+++ b/migration/colo.c
@@ -14,7 +14,6 @@
#include "sysemu/sysemu.h"
#include "qapi/error.h"
#include "qapi/qapi-commands-migration.h"
-#include "qemu-file-channel.h"
#include "migration.h"
#include "qemu-file.h"
#include "savevm.h"
@@ -559,7 +558,7 @@ static void colo_process_checkpoint(MigrationState *s)
goto out;
}
bioc = qio_channel_buffer_new(COLO_BUFFER_BASE_SIZE);
- fb = qemu_fopen_channel_output(QIO_CHANNEL(bioc));
+ fb = qemu_file_new_output(QIO_CHANNEL(bioc));
object_unref(OBJECT(bioc));
qemu_mutex_lock_iothread();
@@ -873,7 +872,7 @@ void *colo_process_incoming_thread(void *opaque)
colo_incoming_start_dirty_log();
bioc = qio_channel_buffer_new(COLO_BUFFER_BASE_SIZE);
- fb = qemu_fopen_channel_input(QIO_CHANNEL(bioc));
+ fb = qemu_file_new_input(QIO_CHANNEL(bioc));
object_unref(OBJECT(bioc));
qemu_mutex_lock_iothread();
diff --git a/migration/meson.build b/migration/meson.build
index 8d309f5849..690487cf1a 100644
--- a/migration/meson.build
+++ b/migration/meson.build
@@ -4,7 +4,6 @@ migration_files = files(
'xbzrle.c',
'vmstate-types.c',
'vmstate.c',
- 'qemu-file-channel.c',
'qemu-file.c',
'yank_functions.c',
)
diff --git a/migration/migration.c b/migration/migration.c
index ab1e9610ef..8a30ef17d9 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -30,7 +30,6 @@
#include "migration/misc.h"
#include "migration.h"
#include "savevm.h"
-#include "qemu-file-channel.h"
#include "qemu-file.h"
#include "migration/vmstate.h"
#include "block/block.h"
@@ -722,7 +721,7 @@ void migration_ioc_process_incoming(QIOChannel *ioc, Error **errp)
if (!mis->from_src_file) {
/* The first connection (multifd may have multiple) */
- QEMUFile *f = qemu_fopen_channel_input(ioc);
+ QEMUFile *f = qemu_file_new_input(ioc);
if (!migration_incoming_setup(f, errp)) {
return;
@@ -3081,7 +3080,7 @@ static int postcopy_start(MigrationState *ms)
*/
bioc = qio_channel_buffer_new(4096);
qio_channel_set_name(QIO_CHANNEL(bioc), "migration-postcopy-buffer");
- fb = qemu_fopen_channel_output(QIO_CHANNEL(bioc));
+ fb = qemu_file_new_output(QIO_CHANNEL(bioc));
object_unref(OBJECT(bioc));
/*
@@ -3970,7 +3969,7 @@ static void *bg_migration_thread(void *opaque)
*/
s->bioc = qio_channel_buffer_new(512 * 1024);
qio_channel_set_name(QIO_CHANNEL(s->bioc), "vmstate-buffer");
- fb = qemu_fopen_channel_output(QIO_CHANNEL(s->bioc));
+ fb = qemu_file_new_output(QIO_CHANNEL(s->bioc));
object_unref(OBJECT(s->bioc));
update_iteration_initial_status(s);
diff --git a/migration/qemu-file-channel.c b/migration/qemu-file-channel.c
deleted file mode 100644
index 51717c1137..0000000000
--- a/migration/qemu-file-channel.c
+++ /dev/null
@@ -1,53 +0,0 @@
-/*
- * QEMUFile backend for QIOChannel objects
- *
- * Copyright (c) 2015-2016 Red Hat, Inc
- *
- * Permission is hereby granted, free of charge, to any person obtaining a copy
- * of this software and associated documentation files (the "Software"), to deal
- * in the Software without restriction, including without limitation the rights
- * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
- * copies of the Software, and to permit persons to whom the Software is
- * furnished to do so, subject to the following conditions:
- *
- * The above copyright notice and this permission notice shall be included in
- * all copies or substantial portions of the Software.
- *
- * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
- * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
- * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
- * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
- * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
- * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
- * THE SOFTWARE.
- */
-
-#include "qemu/osdep.h"
-#include "qemu-file-channel.h"
-#include "qemu-file.h"
-#include "io/channel-socket.h"
-#include "io/channel-tls.h"
-#include "qemu/iov.h"
-#include "qemu/yank.h"
-#include "yank_functions.h"
-
-
-static const QEMUFileOps channel_input_ops = {
-};
-
-
-static const QEMUFileOps channel_output_ops = {
-};
-
-
-QEMUFile *qemu_fopen_channel_input(QIOChannel *ioc)
-{
- object_ref(OBJECT(ioc));
- return qemu_file_new_input(ioc, &channel_input_ops);
-}
-
-QEMUFile *qemu_fopen_channel_output(QIOChannel *ioc)
-{
- object_ref(OBJECT(ioc));
- return qemu_file_new_output(ioc, &channel_output_ops);
-}
diff --git a/migration/qemu-file-channel.h b/migration/qemu-file-channel.h
deleted file mode 100644
index 0028a09eb6..0000000000
--- a/migration/qemu-file-channel.h
+++ /dev/null
@@ -1,32 +0,0 @@
-/*
- * QEMUFile backend for QIOChannel objects
- *
- * Copyright (c) 2015-2016 Red Hat, Inc
- *
- * Permission is hereby granted, free of charge, to any person obtaining a copy
- * of this software and associated documentation files (the "Software"), to deal
- * in the Software without restriction, including without limitation the rights
- * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
- * copies of the Software, and to permit persons to whom the Software is
- * furnished to do so, subject to the following conditions:
- *
- * The above copyright notice and this permission notice shall be included in
- * all copies or substantial portions of the Software.
- *
- * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
- * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
- * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
- * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
- * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
- * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
- * THE SOFTWARE.
- */
-
-#ifndef QEMU_FILE_CHANNEL_H
-#define QEMU_FILE_CHANNEL_H
-
-#include "io/channel.h"
-
-QEMUFile *qemu_fopen_channel_input(QIOChannel *ioc);
-QEMUFile *qemu_fopen_channel_output(QIOChannel *ioc);
-#endif
diff --git a/migration/qemu-file.c b/migration/qemu-file.c
index dfee808924..81688b7a23 100644
--- a/migration/qemu-file.c
+++ b/migration/qemu-file.c
@@ -35,7 +35,6 @@
#define MAX_IOV_SIZE MIN_CONST(IOV_MAX, 64)
struct QEMUFile {
- const QEMUFileOps *ops;
const QEMUFileHooks *hooks;
QIOChannel *ioc;
bool is_writable;
@@ -104,16 +103,14 @@ bool qemu_file_mode_is_not_valid(const char *mode)
return false;
}
-static QEMUFile *qemu_file_new_impl(QIOChannel *ioc,
- const QEMUFileOps *ops,
- bool is_writable)
+static QEMUFile *qemu_file_new_impl(QIOChannel *ioc, bool is_writable)
{
QEMUFile *f;
f = g_new0(QEMUFile, 1);
+ object_ref(ioc);
f->ioc = ioc;
- f->ops = ops;
f->is_writable = is_writable;
return f;
@@ -125,21 +122,19 @@ static QEMUFile *qemu_file_new_impl(QIOChannel *ioc,
*/
QEMUFile *qemu_file_get_return_path(QEMUFile *f)
{
- object_ref(f->ioc);
- return qemu_file_new_impl(f->ioc, f->ops, !f->is_writable);
+ return qemu_file_new_impl(f->ioc, !f->is_writable);
}
-QEMUFile *qemu_file_new_output(QIOChannel *ioc, const QEMUFileOps *ops)
+QEMUFile *qemu_file_new_output(QIOChannel *ioc)
{
- return qemu_file_new_impl(ioc, ops, true);
+ return qemu_file_new_impl(ioc, true);
}
-QEMUFile *qemu_file_new_input(QIOChannel *ioc, const QEMUFileOps *ops)
+QEMUFile *qemu_file_new_input(QIOChannel *ioc)
{
- return qemu_file_new_impl(ioc, ops, false);
+ return qemu_file_new_impl(ioc, false);
}
-
void qemu_file_set_hooks(QEMUFile *f, const QEMUFileHooks *hooks)
{
f->hooks = hooks;
@@ -235,6 +230,7 @@ static void qemu_iovec_release_ram(QEMUFile *f)
memset(f->may_free, 0, sizeof(f->may_free));
}
+
/**
* Flushes QEMUFile buffer
*
diff --git a/migration/qemu-file.h b/migration/qemu-file.h
index 5370e4e5ec..12e625f96a 100644
--- a/migration/qemu-file.h
+++ b/migration/qemu-file.h
@@ -55,9 +55,6 @@ typedef size_t (QEMURamSaveFunc)(QEMUFile *f,
size_t size,
uint64_t *bytes_sent);
-typedef struct QEMUFileOps {
-} QEMUFileOps;
-
typedef struct QEMUFileHooks {
QEMURamHookFunc *before_ram_iterate;
QEMURamHookFunc *after_ram_iterate;
@@ -65,10 +62,9 @@ typedef struct QEMUFileHooks {
QEMURamSaveFunc *save_page;
} QEMUFileHooks;
-QEMUFile *qemu_file_new_input(QIOChannel *ioc, const QEMUFileOps *ops);
-QEMUFile *qemu_file_new_output(QIOChannel *ioc, const QEMUFileOps *ops);
+QEMUFile *qemu_file_new_input(QIOChannel *ioc);
+QEMUFile *qemu_file_new_output(QIOChannel *ioc);
void qemu_file_set_hooks(QEMUFile *f, const QEMUFileHooks *hooks);
-int qemu_get_fd(QEMUFile *f);
int qemu_fclose(QEMUFile *f);
/*
diff --git a/migration/ram.c b/migration/ram.c
index bf321e1e72..01f9cc1d72 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -38,7 +38,6 @@
#include "migration.h"
#include "migration/register.h"
#include "migration/misc.h"
-#include "migration/qemu-file-channel.h"
#include "qemu-file.h"
#include "postcopy-ram.h"
#include "page_cache.h"
@@ -569,7 +568,7 @@ static int compress_threads_save_setup(void)
/* comp_param[i].file is just used as a dummy buffer to save data,
* set its ops to empty.
*/
- comp_param[i].file = qemu_fopen_channel_output(
+ comp_param[i].file = qemu_file_new_output(
QIO_CHANNEL(qio_channel_null_new()));
comp_param[i].done = true;
comp_param[i].quit = false;
diff --git a/migration/rdma.c b/migration/rdma.c
index 83265513d9..49e6dba323 100644
--- a/migration/rdma.c
+++ b/migration/rdma.c
@@ -21,7 +21,6 @@
#include "migration.h"
#include "qemu-file.h"
#include "ram.h"
-#include "qemu-file-channel.h"
#include "qemu/error-report.h"
#include "qemu/main-loop.h"
#include "qemu/module.h"
@@ -4093,12 +4092,12 @@ static QEMUFile *qemu_fopen_rdma(RDMAContext *rdma, const char *mode)
rioc = QIO_CHANNEL_RDMA(object_new(TYPE_QIO_CHANNEL_RDMA));
if (mode[0] == 'w') {
- rioc->file = qemu_fopen_channel_output(QIO_CHANNEL(rioc));
+ rioc->file = qemu_file_new_output(QIO_CHANNEL(rioc));
rioc->rdmaout = rdma;
rioc->rdmain = rdma->return_path;
qemu_file_set_hooks(rioc->file, &rdma_write_hooks);
} else {
- rioc->file = qemu_fopen_channel_input(QIO_CHANNEL(rioc));
+ rioc->file = qemu_file_new_input(QIO_CHANNEL(rioc));
rioc->rdmain = rdma;
rioc->rdmaout = rdma->return_path;
qemu_file_set_hooks(rioc->file, &rdma_read_hooks);
diff --git a/migration/savevm.c b/migration/savevm.c
index 24a50376dc..e8a1b96fcd 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -37,7 +37,6 @@
#include "migration/global_state.h"
#include "migration/channel-block.h"
#include "ram.h"
-#include "qemu-file-channel.h"
#include "qemu-file.h"
#include "savevm.h"
#include "postcopy-ram.h"
@@ -134,9 +133,9 @@ static struct mig_cmd_args {
static QEMUFile *qemu_fopen_bdrv(BlockDriverState *bs, int is_writable)
{
if (is_writable) {
- return qemu_fopen_channel_output(QIO_CHANNEL(qio_channel_block_new(bs)));
+ return qemu_file_new_output(QIO_CHANNEL(qio_channel_block_new(bs)));
} else {
- return qemu_fopen_channel_input(QIO_CHANNEL(qio_channel_block_new(bs)));
+ return qemu_file_new_input(QIO_CHANNEL(qio_channel_block_new(bs)));
}
}
@@ -2159,7 +2158,7 @@ static int loadvm_handle_cmd_packaged(MigrationIncomingState *mis)
bioc->usage += length;
trace_loadvm_handle_cmd_packaged_received(ret);
- QEMUFile *packf = qemu_fopen_channel_input(QIO_CHANNEL(bioc));
+ QEMUFile *packf = qemu_file_new_input(QIO_CHANNEL(bioc));
ret = qemu_loadvm_state_main(packf, mis);
trace_loadvm_handle_cmd_packaged_main(ret);
@@ -2917,7 +2916,7 @@ void qmp_xen_save_devices_state(const char *filename, bool has_live, bool live,
goto the_end;
}
qio_channel_set_name(QIO_CHANNEL(ioc), "migration-xen-save-state");
- f = qemu_fopen_channel_output(QIO_CHANNEL(ioc));
+ f = qemu_file_new_output(QIO_CHANNEL(ioc));
object_unref(OBJECT(ioc));
ret = qemu_save_device_state(f);
if (ret < 0 || qemu_fclose(f) < 0) {
@@ -2964,7 +2963,7 @@ void qmp_xen_load_devices_state(const char *filename, Error **errp)
return;
}
qio_channel_set_name(QIO_CHANNEL(ioc), "migration-xen-load-state");
- f = qemu_fopen_channel_input(QIO_CHANNEL(ioc));
+ f = qemu_file_new_input(QIO_CHANNEL(ioc));
object_unref(OBJECT(ioc));
ret = qemu_loadvm_state(f);
diff --git a/tests/unit/test-vmstate.c b/tests/unit/test-vmstate.c
index 6a417bb102..72077b5780 100644
--- a/tests/unit/test-vmstate.c
+++ b/tests/unit/test-vmstate.c
@@ -28,7 +28,6 @@
#include "migration/vmstate.h"
#include "migration/qemu-file-types.h"
#include "../migration/qemu-file.h"
-#include "../migration/qemu-file-channel.h"
#include "../migration/savevm.h"
#include "qemu/coroutine.h"
#include "qemu/module.h"
@@ -52,9 +51,9 @@ static QEMUFile *open_test_file(bool write)
}
ioc = QIO_CHANNEL(qio_channel_file_new_fd(fd));
if (write) {
- f = qemu_fopen_channel_output(ioc);
+ f = qemu_file_new_output(ioc);
} else {
- f = qemu_fopen_channel_input(ioc);
+ f = qemu_file_new_input(ioc);
}
object_unref(OBJECT(ioc));
return f;
--
2.36.1
^ permalink raw reply related [flat|nested] 51+ messages in thread* Re: [PATCH 20/20] migration: remove the QEMUFileOps abstraction
2022-05-24 11:02 ` [PATCH 20/20] migration: remove the QEMUFileOps abstraction Daniel P. Berrangé
@ 2022-06-09 16:59 ` Dr. David Alan Gilbert
2022-06-09 17:10 ` Daniel P. Berrangé
0 siblings, 1 reply; 51+ messages in thread
From: Dr. David Alan Gilbert @ 2022-06-09 16:59 UTC (permalink / raw)
To: Daniel P. Berrangé
Cc: qemu-devel, qemu-block, Stefan Hajnoczi, Hailiang Zhang,
Juan Quintela, Fam Zheng
* Daniel P. Berrangé (berrange@redhat.com) wrote:
> Now that all QEMUFile callbacks are removed, the entire concept can be
> deleted.
>
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
I think that's OK, there's one nit - you remove qemu_get_fd from one of
the headers; I think that probably belongs in an earlier patch.
Other than that,
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> ---
> migration/channel.c | 4 +--
> migration/colo.c | 5 ++--
> migration/meson.build | 1 -
> migration/migration.c | 7 ++---
> migration/qemu-file-channel.c | 53 -----------------------------------
> migration/qemu-file-channel.h | 32 ---------------------
> migration/qemu-file.c | 20 ++++++-------
> migration/qemu-file.h | 8 ++----
> migration/ram.c | 3 +-
> migration/rdma.c | 5 ++--
> migration/savevm.c | 11 ++++----
> tests/unit/test-vmstate.c | 5 ++--
> 12 files changed, 27 insertions(+), 127 deletions(-)
> delete mode 100644 migration/qemu-file-channel.c
> delete mode 100644 migration/qemu-file-channel.h
>
> diff --git a/migration/channel.c b/migration/channel.c
> index a162d00fea..90087d8986 100644
> --- a/migration/channel.c
> +++ b/migration/channel.c
> @@ -14,7 +14,7 @@
> #include "channel.h"
> #include "tls.h"
> #include "migration.h"
> -#include "qemu-file-channel.h"
> +#include "qemu-file.h"
> #include "trace.h"
> #include "qapi/error.h"
> #include "io/channel-tls.h"
> @@ -85,7 +85,7 @@ void migration_channel_connect(MigrationState *s,
> return;
> }
> } else {
> - QEMUFile *f = qemu_fopen_channel_output(ioc);
> + QEMUFile *f = qemu_file_new_output(ioc);
>
> migration_ioc_register_yank(ioc);
>
> diff --git a/migration/colo.c b/migration/colo.c
> index 5f7071b3cd..2b71722fd6 100644
> --- a/migration/colo.c
> +++ b/migration/colo.c
> @@ -14,7 +14,6 @@
> #include "sysemu/sysemu.h"
> #include "qapi/error.h"
> #include "qapi/qapi-commands-migration.h"
> -#include "qemu-file-channel.h"
> #include "migration.h"
> #include "qemu-file.h"
> #include "savevm.h"
> @@ -559,7 +558,7 @@ static void colo_process_checkpoint(MigrationState *s)
> goto out;
> }
> bioc = qio_channel_buffer_new(COLO_BUFFER_BASE_SIZE);
> - fb = qemu_fopen_channel_output(QIO_CHANNEL(bioc));
> + fb = qemu_file_new_output(QIO_CHANNEL(bioc));
> object_unref(OBJECT(bioc));
>
> qemu_mutex_lock_iothread();
> @@ -873,7 +872,7 @@ void *colo_process_incoming_thread(void *opaque)
> colo_incoming_start_dirty_log();
>
> bioc = qio_channel_buffer_new(COLO_BUFFER_BASE_SIZE);
> - fb = qemu_fopen_channel_input(QIO_CHANNEL(bioc));
> + fb = qemu_file_new_input(QIO_CHANNEL(bioc));
> object_unref(OBJECT(bioc));
>
> qemu_mutex_lock_iothread();
> diff --git a/migration/meson.build b/migration/meson.build
> index 8d309f5849..690487cf1a 100644
> --- a/migration/meson.build
> +++ b/migration/meson.build
> @@ -4,7 +4,6 @@ migration_files = files(
> 'xbzrle.c',
> 'vmstate-types.c',
> 'vmstate.c',
> - 'qemu-file-channel.c',
> 'qemu-file.c',
> 'yank_functions.c',
> )
> diff --git a/migration/migration.c b/migration/migration.c
> index ab1e9610ef..8a30ef17d9 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -30,7 +30,6 @@
> #include "migration/misc.h"
> #include "migration.h"
> #include "savevm.h"
> -#include "qemu-file-channel.h"
> #include "qemu-file.h"
> #include "migration/vmstate.h"
> #include "block/block.h"
> @@ -722,7 +721,7 @@ void migration_ioc_process_incoming(QIOChannel *ioc, Error **errp)
>
> if (!mis->from_src_file) {
> /* The first connection (multifd may have multiple) */
> - QEMUFile *f = qemu_fopen_channel_input(ioc);
> + QEMUFile *f = qemu_file_new_input(ioc);
>
> if (!migration_incoming_setup(f, errp)) {
> return;
> @@ -3081,7 +3080,7 @@ static int postcopy_start(MigrationState *ms)
> */
> bioc = qio_channel_buffer_new(4096);
> qio_channel_set_name(QIO_CHANNEL(bioc), "migration-postcopy-buffer");
> - fb = qemu_fopen_channel_output(QIO_CHANNEL(bioc));
> + fb = qemu_file_new_output(QIO_CHANNEL(bioc));
> object_unref(OBJECT(bioc));
>
> /*
> @@ -3970,7 +3969,7 @@ static void *bg_migration_thread(void *opaque)
> */
> s->bioc = qio_channel_buffer_new(512 * 1024);
> qio_channel_set_name(QIO_CHANNEL(s->bioc), "vmstate-buffer");
> - fb = qemu_fopen_channel_output(QIO_CHANNEL(s->bioc));
> + fb = qemu_file_new_output(QIO_CHANNEL(s->bioc));
> object_unref(OBJECT(s->bioc));
>
> update_iteration_initial_status(s);
> diff --git a/migration/qemu-file-channel.c b/migration/qemu-file-channel.c
> deleted file mode 100644
> index 51717c1137..0000000000
> --- a/migration/qemu-file-channel.c
> +++ /dev/null
> @@ -1,53 +0,0 @@
> -/*
> - * QEMUFile backend for QIOChannel objects
> - *
> - * Copyright (c) 2015-2016 Red Hat, Inc
> - *
> - * Permission is hereby granted, free of charge, to any person obtaining a copy
> - * of this software and associated documentation files (the "Software"), to deal
> - * in the Software without restriction, including without limitation the rights
> - * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
> - * copies of the Software, and to permit persons to whom the Software is
> - * furnished to do so, subject to the following conditions:
> - *
> - * The above copyright notice and this permission notice shall be included in
> - * all copies or substantial portions of the Software.
> - *
> - * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> - * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> - * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> - * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> - * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
> - * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
> - * THE SOFTWARE.
> - */
> -
> -#include "qemu/osdep.h"
> -#include "qemu-file-channel.h"
> -#include "qemu-file.h"
> -#include "io/channel-socket.h"
> -#include "io/channel-tls.h"
> -#include "qemu/iov.h"
> -#include "qemu/yank.h"
> -#include "yank_functions.h"
> -
> -
> -static const QEMUFileOps channel_input_ops = {
> -};
> -
> -
> -static const QEMUFileOps channel_output_ops = {
> -};
> -
> -
> -QEMUFile *qemu_fopen_channel_input(QIOChannel *ioc)
> -{
> - object_ref(OBJECT(ioc));
> - return qemu_file_new_input(ioc, &channel_input_ops);
> -}
> -
> -QEMUFile *qemu_fopen_channel_output(QIOChannel *ioc)
> -{
> - object_ref(OBJECT(ioc));
> - return qemu_file_new_output(ioc, &channel_output_ops);
> -}
> diff --git a/migration/qemu-file-channel.h b/migration/qemu-file-channel.h
> deleted file mode 100644
> index 0028a09eb6..0000000000
> --- a/migration/qemu-file-channel.h
> +++ /dev/null
> @@ -1,32 +0,0 @@
> -/*
> - * QEMUFile backend for QIOChannel objects
> - *
> - * Copyright (c) 2015-2016 Red Hat, Inc
> - *
> - * Permission is hereby granted, free of charge, to any person obtaining a copy
> - * of this software and associated documentation files (the "Software"), to deal
> - * in the Software without restriction, including without limitation the rights
> - * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
> - * copies of the Software, and to permit persons to whom the Software is
> - * furnished to do so, subject to the following conditions:
> - *
> - * The above copyright notice and this permission notice shall be included in
> - * all copies or substantial portions of the Software.
> - *
> - * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> - * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> - * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> - * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> - * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
> - * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
> - * THE SOFTWARE.
> - */
> -
> -#ifndef QEMU_FILE_CHANNEL_H
> -#define QEMU_FILE_CHANNEL_H
> -
> -#include "io/channel.h"
> -
> -QEMUFile *qemu_fopen_channel_input(QIOChannel *ioc);
> -QEMUFile *qemu_fopen_channel_output(QIOChannel *ioc);
> -#endif
> diff --git a/migration/qemu-file.c b/migration/qemu-file.c
> index dfee808924..81688b7a23 100644
> --- a/migration/qemu-file.c
> +++ b/migration/qemu-file.c
> @@ -35,7 +35,6 @@
> #define MAX_IOV_SIZE MIN_CONST(IOV_MAX, 64)
>
> struct QEMUFile {
> - const QEMUFileOps *ops;
> const QEMUFileHooks *hooks;
> QIOChannel *ioc;
> bool is_writable;
> @@ -104,16 +103,14 @@ bool qemu_file_mode_is_not_valid(const char *mode)
> return false;
> }
>
> -static QEMUFile *qemu_file_new_impl(QIOChannel *ioc,
> - const QEMUFileOps *ops,
> - bool is_writable)
> +static QEMUFile *qemu_file_new_impl(QIOChannel *ioc, bool is_writable)
> {
> QEMUFile *f;
>
> f = g_new0(QEMUFile, 1);
>
> + object_ref(ioc);
> f->ioc = ioc;
> - f->ops = ops;
> f->is_writable = is_writable;
>
> return f;
> @@ -125,21 +122,19 @@ static QEMUFile *qemu_file_new_impl(QIOChannel *ioc,
> */
> QEMUFile *qemu_file_get_return_path(QEMUFile *f)
> {
> - object_ref(f->ioc);
> - return qemu_file_new_impl(f->ioc, f->ops, !f->is_writable);
> + return qemu_file_new_impl(f->ioc, !f->is_writable);
> }
>
> -QEMUFile *qemu_file_new_output(QIOChannel *ioc, const QEMUFileOps *ops)
> +QEMUFile *qemu_file_new_output(QIOChannel *ioc)
> {
> - return qemu_file_new_impl(ioc, ops, true);
> + return qemu_file_new_impl(ioc, true);
> }
>
> -QEMUFile *qemu_file_new_input(QIOChannel *ioc, const QEMUFileOps *ops)
> +QEMUFile *qemu_file_new_input(QIOChannel *ioc)
> {
> - return qemu_file_new_impl(ioc, ops, false);
> + return qemu_file_new_impl(ioc, false);
> }
>
> -
> void qemu_file_set_hooks(QEMUFile *f, const QEMUFileHooks *hooks)
> {
> f->hooks = hooks;
> @@ -235,6 +230,7 @@ static void qemu_iovec_release_ram(QEMUFile *f)
> memset(f->may_free, 0, sizeof(f->may_free));
> }
>
> +
> /**
> * Flushes QEMUFile buffer
> *
> diff --git a/migration/qemu-file.h b/migration/qemu-file.h
> index 5370e4e5ec..12e625f96a 100644
> --- a/migration/qemu-file.h
> +++ b/migration/qemu-file.h
> @@ -55,9 +55,6 @@ typedef size_t (QEMURamSaveFunc)(QEMUFile *f,
> size_t size,
> uint64_t *bytes_sent);
>
> -typedef struct QEMUFileOps {
> -} QEMUFileOps;
> -
> typedef struct QEMUFileHooks {
> QEMURamHookFunc *before_ram_iterate;
> QEMURamHookFunc *after_ram_iterate;
> @@ -65,10 +62,9 @@ typedef struct QEMUFileHooks {
> QEMURamSaveFunc *save_page;
> } QEMUFileHooks;
>
> -QEMUFile *qemu_file_new_input(QIOChannel *ioc, const QEMUFileOps *ops);
> -QEMUFile *qemu_file_new_output(QIOChannel *ioc, const QEMUFileOps *ops);
> +QEMUFile *qemu_file_new_input(QIOChannel *ioc);
> +QEMUFile *qemu_file_new_output(QIOChannel *ioc);
> void qemu_file_set_hooks(QEMUFile *f, const QEMUFileHooks *hooks);
> -int qemu_get_fd(QEMUFile *f);
> int qemu_fclose(QEMUFile *f);
>
> /*
> diff --git a/migration/ram.c b/migration/ram.c
> index bf321e1e72..01f9cc1d72 100644
> --- a/migration/ram.c
> +++ b/migration/ram.c
> @@ -38,7 +38,6 @@
> #include "migration.h"
> #include "migration/register.h"
> #include "migration/misc.h"
> -#include "migration/qemu-file-channel.h"
> #include "qemu-file.h"
> #include "postcopy-ram.h"
> #include "page_cache.h"
> @@ -569,7 +568,7 @@ static int compress_threads_save_setup(void)
> /* comp_param[i].file is just used as a dummy buffer to save data,
> * set its ops to empty.
> */
> - comp_param[i].file = qemu_fopen_channel_output(
> + comp_param[i].file = qemu_file_new_output(
> QIO_CHANNEL(qio_channel_null_new()));
> comp_param[i].done = true;
> comp_param[i].quit = false;
> diff --git a/migration/rdma.c b/migration/rdma.c
> index 83265513d9..49e6dba323 100644
> --- a/migration/rdma.c
> +++ b/migration/rdma.c
> @@ -21,7 +21,6 @@
> #include "migration.h"
> #include "qemu-file.h"
> #include "ram.h"
> -#include "qemu-file-channel.h"
> #include "qemu/error-report.h"
> #include "qemu/main-loop.h"
> #include "qemu/module.h"
> @@ -4093,12 +4092,12 @@ static QEMUFile *qemu_fopen_rdma(RDMAContext *rdma, const char *mode)
> rioc = QIO_CHANNEL_RDMA(object_new(TYPE_QIO_CHANNEL_RDMA));
>
> if (mode[0] == 'w') {
> - rioc->file = qemu_fopen_channel_output(QIO_CHANNEL(rioc));
> + rioc->file = qemu_file_new_output(QIO_CHANNEL(rioc));
> rioc->rdmaout = rdma;
> rioc->rdmain = rdma->return_path;
> qemu_file_set_hooks(rioc->file, &rdma_write_hooks);
> } else {
> - rioc->file = qemu_fopen_channel_input(QIO_CHANNEL(rioc));
> + rioc->file = qemu_file_new_input(QIO_CHANNEL(rioc));
> rioc->rdmain = rdma;
> rioc->rdmaout = rdma->return_path;
> qemu_file_set_hooks(rioc->file, &rdma_read_hooks);
> diff --git a/migration/savevm.c b/migration/savevm.c
> index 24a50376dc..e8a1b96fcd 100644
> --- a/migration/savevm.c
> +++ b/migration/savevm.c
> @@ -37,7 +37,6 @@
> #include "migration/global_state.h"
> #include "migration/channel-block.h"
> #include "ram.h"
> -#include "qemu-file-channel.h"
> #include "qemu-file.h"
> #include "savevm.h"
> #include "postcopy-ram.h"
> @@ -134,9 +133,9 @@ static struct mig_cmd_args {
> static QEMUFile *qemu_fopen_bdrv(BlockDriverState *bs, int is_writable)
> {
> if (is_writable) {
> - return qemu_fopen_channel_output(QIO_CHANNEL(qio_channel_block_new(bs)));
> + return qemu_file_new_output(QIO_CHANNEL(qio_channel_block_new(bs)));
> } else {
> - return qemu_fopen_channel_input(QIO_CHANNEL(qio_channel_block_new(bs)));
> + return qemu_file_new_input(QIO_CHANNEL(qio_channel_block_new(bs)));
> }
> }
>
> @@ -2159,7 +2158,7 @@ static int loadvm_handle_cmd_packaged(MigrationIncomingState *mis)
> bioc->usage += length;
> trace_loadvm_handle_cmd_packaged_received(ret);
>
> - QEMUFile *packf = qemu_fopen_channel_input(QIO_CHANNEL(bioc));
> + QEMUFile *packf = qemu_file_new_input(QIO_CHANNEL(bioc));
>
> ret = qemu_loadvm_state_main(packf, mis);
> trace_loadvm_handle_cmd_packaged_main(ret);
> @@ -2917,7 +2916,7 @@ void qmp_xen_save_devices_state(const char *filename, bool has_live, bool live,
> goto the_end;
> }
> qio_channel_set_name(QIO_CHANNEL(ioc), "migration-xen-save-state");
> - f = qemu_fopen_channel_output(QIO_CHANNEL(ioc));
> + f = qemu_file_new_output(QIO_CHANNEL(ioc));
> object_unref(OBJECT(ioc));
> ret = qemu_save_device_state(f);
> if (ret < 0 || qemu_fclose(f) < 0) {
> @@ -2964,7 +2963,7 @@ void qmp_xen_load_devices_state(const char *filename, Error **errp)
> return;
> }
> qio_channel_set_name(QIO_CHANNEL(ioc), "migration-xen-load-state");
> - f = qemu_fopen_channel_input(QIO_CHANNEL(ioc));
> + f = qemu_file_new_input(QIO_CHANNEL(ioc));
> object_unref(OBJECT(ioc));
>
> ret = qemu_loadvm_state(f);
> diff --git a/tests/unit/test-vmstate.c b/tests/unit/test-vmstate.c
> index 6a417bb102..72077b5780 100644
> --- a/tests/unit/test-vmstate.c
> +++ b/tests/unit/test-vmstate.c
> @@ -28,7 +28,6 @@
> #include "migration/vmstate.h"
> #include "migration/qemu-file-types.h"
> #include "../migration/qemu-file.h"
> -#include "../migration/qemu-file-channel.h"
> #include "../migration/savevm.h"
> #include "qemu/coroutine.h"
> #include "qemu/module.h"
> @@ -52,9 +51,9 @@ static QEMUFile *open_test_file(bool write)
> }
> ioc = QIO_CHANNEL(qio_channel_file_new_fd(fd));
> if (write) {
> - f = qemu_fopen_channel_output(ioc);
> + f = qemu_file_new_output(ioc);
> } else {
> - f = qemu_fopen_channel_input(ioc);
> + f = qemu_file_new_input(ioc);
> }
> object_unref(OBJECT(ioc));
> return f;
> --
> 2.36.1
>
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
^ permalink raw reply [flat|nested] 51+ messages in thread* Re: [PATCH 20/20] migration: remove the QEMUFileOps abstraction
2022-06-09 16:59 ` Dr. David Alan Gilbert
@ 2022-06-09 17:10 ` Daniel P. Berrangé
0 siblings, 0 replies; 51+ messages in thread
From: Daniel P. Berrangé @ 2022-06-09 17:10 UTC (permalink / raw)
To: Dr. David Alan Gilbert
Cc: qemu-devel, qemu-block, Stefan Hajnoczi, Hailiang Zhang,
Juan Quintela, Fam Zheng
On Thu, Jun 09, 2022 at 05:59:00PM +0100, Dr. David Alan Gilbert wrote:
> * Daniel P. Berrangé (berrange@redhat.com) wrote:
> > Now that all QEMUFile callbacks are removed, the entire concept can be
> > deleted.
> >
> > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
>
> I think that's OK, there's one nit - you remove qemu_get_fd from one of
> the headers; I think that probably belongs in an earlier patch.
Oh should probably be squashed back into patch 13, which removes
the corresponding unused callback typedef.
>
> Other than that,
>
>
> Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
>
> > ---
> > migration/channel.c | 4 +--
> > migration/colo.c | 5 ++--
> > migration/meson.build | 1 -
> > migration/migration.c | 7 ++---
> > migration/qemu-file-channel.c | 53 -----------------------------------
> > migration/qemu-file-channel.h | 32 ---------------------
> > migration/qemu-file.c | 20 ++++++-------
> > migration/qemu-file.h | 8 ++----
> > migration/ram.c | 3 +-
> > migration/rdma.c | 5 ++--
> > migration/savevm.c | 11 ++++----
> > tests/unit/test-vmstate.c | 5 ++--
> > 12 files changed, 27 insertions(+), 127 deletions(-)
> > delete mode 100644 migration/qemu-file-channel.c
> > delete mode 100644 migration/qemu-file-channel.h
> >
> > diff --git a/migration/channel.c b/migration/channel.c
> > index a162d00fea..90087d8986 100644
> > --- a/migration/channel.c
> > +++ b/migration/channel.c
> > @@ -14,7 +14,7 @@
> > #include "channel.h"
> > #include "tls.h"
> > #include "migration.h"
> > -#include "qemu-file-channel.h"
> > +#include "qemu-file.h"
> > #include "trace.h"
> > #include "qapi/error.h"
> > #include "io/channel-tls.h"
> > @@ -85,7 +85,7 @@ void migration_channel_connect(MigrationState *s,
> > return;
> > }
> > } else {
> > - QEMUFile *f = qemu_fopen_channel_output(ioc);
> > + QEMUFile *f = qemu_file_new_output(ioc);
> >
> > migration_ioc_register_yank(ioc);
> >
> > diff --git a/migration/colo.c b/migration/colo.c
> > index 5f7071b3cd..2b71722fd6 100644
> > --- a/migration/colo.c
> > +++ b/migration/colo.c
> > @@ -14,7 +14,6 @@
> > #include "sysemu/sysemu.h"
> > #include "qapi/error.h"
> > #include "qapi/qapi-commands-migration.h"
> > -#include "qemu-file-channel.h"
> > #include "migration.h"
> > #include "qemu-file.h"
> > #include "savevm.h"
> > @@ -559,7 +558,7 @@ static void colo_process_checkpoint(MigrationState *s)
> > goto out;
> > }
> > bioc = qio_channel_buffer_new(COLO_BUFFER_BASE_SIZE);
> > - fb = qemu_fopen_channel_output(QIO_CHANNEL(bioc));
> > + fb = qemu_file_new_output(QIO_CHANNEL(bioc));
> > object_unref(OBJECT(bioc));
> >
> > qemu_mutex_lock_iothread();
> > @@ -873,7 +872,7 @@ void *colo_process_incoming_thread(void *opaque)
> > colo_incoming_start_dirty_log();
> >
> > bioc = qio_channel_buffer_new(COLO_BUFFER_BASE_SIZE);
> > - fb = qemu_fopen_channel_input(QIO_CHANNEL(bioc));
> > + fb = qemu_file_new_input(QIO_CHANNEL(bioc));
> > object_unref(OBJECT(bioc));
> >
> > qemu_mutex_lock_iothread();
> > diff --git a/migration/meson.build b/migration/meson.build
> > index 8d309f5849..690487cf1a 100644
> > --- a/migration/meson.build
> > +++ b/migration/meson.build
> > @@ -4,7 +4,6 @@ migration_files = files(
> > 'xbzrle.c',
> > 'vmstate-types.c',
> > 'vmstate.c',
> > - 'qemu-file-channel.c',
> > 'qemu-file.c',
> > 'yank_functions.c',
> > )
> > diff --git a/migration/migration.c b/migration/migration.c
> > index ab1e9610ef..8a30ef17d9 100644
> > --- a/migration/migration.c
> > +++ b/migration/migration.c
> > @@ -30,7 +30,6 @@
> > #include "migration/misc.h"
> > #include "migration.h"
> > #include "savevm.h"
> > -#include "qemu-file-channel.h"
> > #include "qemu-file.h"
> > #include "migration/vmstate.h"
> > #include "block/block.h"
> > @@ -722,7 +721,7 @@ void migration_ioc_process_incoming(QIOChannel *ioc, Error **errp)
> >
> > if (!mis->from_src_file) {
> > /* The first connection (multifd may have multiple) */
> > - QEMUFile *f = qemu_fopen_channel_input(ioc);
> > + QEMUFile *f = qemu_file_new_input(ioc);
> >
> > if (!migration_incoming_setup(f, errp)) {
> > return;
> > @@ -3081,7 +3080,7 @@ static int postcopy_start(MigrationState *ms)
> > */
> > bioc = qio_channel_buffer_new(4096);
> > qio_channel_set_name(QIO_CHANNEL(bioc), "migration-postcopy-buffer");
> > - fb = qemu_fopen_channel_output(QIO_CHANNEL(bioc));
> > + fb = qemu_file_new_output(QIO_CHANNEL(bioc));
> > object_unref(OBJECT(bioc));
> >
> > /*
> > @@ -3970,7 +3969,7 @@ static void *bg_migration_thread(void *opaque)
> > */
> > s->bioc = qio_channel_buffer_new(512 * 1024);
> > qio_channel_set_name(QIO_CHANNEL(s->bioc), "vmstate-buffer");
> > - fb = qemu_fopen_channel_output(QIO_CHANNEL(s->bioc));
> > + fb = qemu_file_new_output(QIO_CHANNEL(s->bioc));
> > object_unref(OBJECT(s->bioc));
> >
> > update_iteration_initial_status(s);
> > diff --git a/migration/qemu-file-channel.c b/migration/qemu-file-channel.c
> > deleted file mode 100644
> > index 51717c1137..0000000000
> > --- a/migration/qemu-file-channel.c
> > +++ /dev/null
> > @@ -1,53 +0,0 @@
> > -/*
> > - * QEMUFile backend for QIOChannel objects
> > - *
> > - * Copyright (c) 2015-2016 Red Hat, Inc
> > - *
> > - * Permission is hereby granted, free of charge, to any person obtaining a copy
> > - * of this software and associated documentation files (the "Software"), to deal
> > - * in the Software without restriction, including without limitation the rights
> > - * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
> > - * copies of the Software, and to permit persons to whom the Software is
> > - * furnished to do so, subject to the following conditions:
> > - *
> > - * The above copyright notice and this permission notice shall be included in
> > - * all copies or substantial portions of the Software.
> > - *
> > - * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> > - * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> > - * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> > - * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> > - * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
> > - * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
> > - * THE SOFTWARE.
> > - */
> > -
> > -#include "qemu/osdep.h"
> > -#include "qemu-file-channel.h"
> > -#include "qemu-file.h"
> > -#include "io/channel-socket.h"
> > -#include "io/channel-tls.h"
> > -#include "qemu/iov.h"
> > -#include "qemu/yank.h"
> > -#include "yank_functions.h"
> > -
> > -
> > -static const QEMUFileOps channel_input_ops = {
> > -};
> > -
> > -
> > -static const QEMUFileOps channel_output_ops = {
> > -};
> > -
> > -
> > -QEMUFile *qemu_fopen_channel_input(QIOChannel *ioc)
> > -{
> > - object_ref(OBJECT(ioc));
> > - return qemu_file_new_input(ioc, &channel_input_ops);
> > -}
> > -
> > -QEMUFile *qemu_fopen_channel_output(QIOChannel *ioc)
> > -{
> > - object_ref(OBJECT(ioc));
> > - return qemu_file_new_output(ioc, &channel_output_ops);
> > -}
> > diff --git a/migration/qemu-file-channel.h b/migration/qemu-file-channel.h
> > deleted file mode 100644
> > index 0028a09eb6..0000000000
> > --- a/migration/qemu-file-channel.h
> > +++ /dev/null
> > @@ -1,32 +0,0 @@
> > -/*
> > - * QEMUFile backend for QIOChannel objects
> > - *
> > - * Copyright (c) 2015-2016 Red Hat, Inc
> > - *
> > - * Permission is hereby granted, free of charge, to any person obtaining a copy
> > - * of this software and associated documentation files (the "Software"), to deal
> > - * in the Software without restriction, including without limitation the rights
> > - * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
> > - * copies of the Software, and to permit persons to whom the Software is
> > - * furnished to do so, subject to the following conditions:
> > - *
> > - * The above copyright notice and this permission notice shall be included in
> > - * all copies or substantial portions of the Software.
> > - *
> > - * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> > - * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> > - * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> > - * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> > - * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
> > - * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
> > - * THE SOFTWARE.
> > - */
> > -
> > -#ifndef QEMU_FILE_CHANNEL_H
> > -#define QEMU_FILE_CHANNEL_H
> > -
> > -#include "io/channel.h"
> > -
> > -QEMUFile *qemu_fopen_channel_input(QIOChannel *ioc);
> > -QEMUFile *qemu_fopen_channel_output(QIOChannel *ioc);
> > -#endif
> > diff --git a/migration/qemu-file.c b/migration/qemu-file.c
> > index dfee808924..81688b7a23 100644
> > --- a/migration/qemu-file.c
> > +++ b/migration/qemu-file.c
> > @@ -35,7 +35,6 @@
> > #define MAX_IOV_SIZE MIN_CONST(IOV_MAX, 64)
> >
> > struct QEMUFile {
> > - const QEMUFileOps *ops;
> > const QEMUFileHooks *hooks;
> > QIOChannel *ioc;
> > bool is_writable;
> > @@ -104,16 +103,14 @@ bool qemu_file_mode_is_not_valid(const char *mode)
> > return false;
> > }
> >
> > -static QEMUFile *qemu_file_new_impl(QIOChannel *ioc,
> > - const QEMUFileOps *ops,
> > - bool is_writable)
> > +static QEMUFile *qemu_file_new_impl(QIOChannel *ioc, bool is_writable)
> > {
> > QEMUFile *f;
> >
> > f = g_new0(QEMUFile, 1);
> >
> > + object_ref(ioc);
> > f->ioc = ioc;
> > - f->ops = ops;
> > f->is_writable = is_writable;
> >
> > return f;
> > @@ -125,21 +122,19 @@ static QEMUFile *qemu_file_new_impl(QIOChannel *ioc,
> > */
> > QEMUFile *qemu_file_get_return_path(QEMUFile *f)
> > {
> > - object_ref(f->ioc);
> > - return qemu_file_new_impl(f->ioc, f->ops, !f->is_writable);
> > + return qemu_file_new_impl(f->ioc, !f->is_writable);
> > }
> >
> > -QEMUFile *qemu_file_new_output(QIOChannel *ioc, const QEMUFileOps *ops)
> > +QEMUFile *qemu_file_new_output(QIOChannel *ioc)
> > {
> > - return qemu_file_new_impl(ioc, ops, true);
> > + return qemu_file_new_impl(ioc, true);
> > }
> >
> > -QEMUFile *qemu_file_new_input(QIOChannel *ioc, const QEMUFileOps *ops)
> > +QEMUFile *qemu_file_new_input(QIOChannel *ioc)
> > {
> > - return qemu_file_new_impl(ioc, ops, false);
> > + return qemu_file_new_impl(ioc, false);
> > }
> >
> > -
> > void qemu_file_set_hooks(QEMUFile *f, const QEMUFileHooks *hooks)
> > {
> > f->hooks = hooks;
> > @@ -235,6 +230,7 @@ static void qemu_iovec_release_ram(QEMUFile *f)
> > memset(f->may_free, 0, sizeof(f->may_free));
> > }
> >
> > +
> > /**
> > * Flushes QEMUFile buffer
> > *
> > diff --git a/migration/qemu-file.h b/migration/qemu-file.h
> > index 5370e4e5ec..12e625f96a 100644
> > --- a/migration/qemu-file.h
> > +++ b/migration/qemu-file.h
> > @@ -55,9 +55,6 @@ typedef size_t (QEMURamSaveFunc)(QEMUFile *f,
> > size_t size,
> > uint64_t *bytes_sent);
> >
> > -typedef struct QEMUFileOps {
> > -} QEMUFileOps;
> > -
> > typedef struct QEMUFileHooks {
> > QEMURamHookFunc *before_ram_iterate;
> > QEMURamHookFunc *after_ram_iterate;
> > @@ -65,10 +62,9 @@ typedef struct QEMUFileHooks {
> > QEMURamSaveFunc *save_page;
> > } QEMUFileHooks;
> >
> > -QEMUFile *qemu_file_new_input(QIOChannel *ioc, const QEMUFileOps *ops);
> > -QEMUFile *qemu_file_new_output(QIOChannel *ioc, const QEMUFileOps *ops);
> > +QEMUFile *qemu_file_new_input(QIOChannel *ioc);
> > +QEMUFile *qemu_file_new_output(QIOChannel *ioc);
> > void qemu_file_set_hooks(QEMUFile *f, const QEMUFileHooks *hooks);
> > -int qemu_get_fd(QEMUFile *f);
> > int qemu_fclose(QEMUFile *f);
> >
> > /*
> > diff --git a/migration/ram.c b/migration/ram.c
> > index bf321e1e72..01f9cc1d72 100644
> > --- a/migration/ram.c
> > +++ b/migration/ram.c
> > @@ -38,7 +38,6 @@
> > #include "migration.h"
> > #include "migration/register.h"
> > #include "migration/misc.h"
> > -#include "migration/qemu-file-channel.h"
> > #include "qemu-file.h"
> > #include "postcopy-ram.h"
> > #include "page_cache.h"
> > @@ -569,7 +568,7 @@ static int compress_threads_save_setup(void)
> > /* comp_param[i].file is just used as a dummy buffer to save data,
> > * set its ops to empty.
> > */
> > - comp_param[i].file = qemu_fopen_channel_output(
> > + comp_param[i].file = qemu_file_new_output(
> > QIO_CHANNEL(qio_channel_null_new()));
> > comp_param[i].done = true;
> > comp_param[i].quit = false;
> > diff --git a/migration/rdma.c b/migration/rdma.c
> > index 83265513d9..49e6dba323 100644
> > --- a/migration/rdma.c
> > +++ b/migration/rdma.c
> > @@ -21,7 +21,6 @@
> > #include "migration.h"
> > #include "qemu-file.h"
> > #include "ram.h"
> > -#include "qemu-file-channel.h"
> > #include "qemu/error-report.h"
> > #include "qemu/main-loop.h"
> > #include "qemu/module.h"
> > @@ -4093,12 +4092,12 @@ static QEMUFile *qemu_fopen_rdma(RDMAContext *rdma, const char *mode)
> > rioc = QIO_CHANNEL_RDMA(object_new(TYPE_QIO_CHANNEL_RDMA));
> >
> > if (mode[0] == 'w') {
> > - rioc->file = qemu_fopen_channel_output(QIO_CHANNEL(rioc));
> > + rioc->file = qemu_file_new_output(QIO_CHANNEL(rioc));
> > rioc->rdmaout = rdma;
> > rioc->rdmain = rdma->return_path;
> > qemu_file_set_hooks(rioc->file, &rdma_write_hooks);
> > } else {
> > - rioc->file = qemu_fopen_channel_input(QIO_CHANNEL(rioc));
> > + rioc->file = qemu_file_new_input(QIO_CHANNEL(rioc));
> > rioc->rdmain = rdma;
> > rioc->rdmaout = rdma->return_path;
> > qemu_file_set_hooks(rioc->file, &rdma_read_hooks);
> > diff --git a/migration/savevm.c b/migration/savevm.c
> > index 24a50376dc..e8a1b96fcd 100644
> > --- a/migration/savevm.c
> > +++ b/migration/savevm.c
> > @@ -37,7 +37,6 @@
> > #include "migration/global_state.h"
> > #include "migration/channel-block.h"
> > #include "ram.h"
> > -#include "qemu-file-channel.h"
> > #include "qemu-file.h"
> > #include "savevm.h"
> > #include "postcopy-ram.h"
> > @@ -134,9 +133,9 @@ static struct mig_cmd_args {
> > static QEMUFile *qemu_fopen_bdrv(BlockDriverState *bs, int is_writable)
> > {
> > if (is_writable) {
> > - return qemu_fopen_channel_output(QIO_CHANNEL(qio_channel_block_new(bs)));
> > + return qemu_file_new_output(QIO_CHANNEL(qio_channel_block_new(bs)));
> > } else {
> > - return qemu_fopen_channel_input(QIO_CHANNEL(qio_channel_block_new(bs)));
> > + return qemu_file_new_input(QIO_CHANNEL(qio_channel_block_new(bs)));
> > }
> > }
> >
> > @@ -2159,7 +2158,7 @@ static int loadvm_handle_cmd_packaged(MigrationIncomingState *mis)
> > bioc->usage += length;
> > trace_loadvm_handle_cmd_packaged_received(ret);
> >
> > - QEMUFile *packf = qemu_fopen_channel_input(QIO_CHANNEL(bioc));
> > + QEMUFile *packf = qemu_file_new_input(QIO_CHANNEL(bioc));
> >
> > ret = qemu_loadvm_state_main(packf, mis);
> > trace_loadvm_handle_cmd_packaged_main(ret);
> > @@ -2917,7 +2916,7 @@ void qmp_xen_save_devices_state(const char *filename, bool has_live, bool live,
> > goto the_end;
> > }
> > qio_channel_set_name(QIO_CHANNEL(ioc), "migration-xen-save-state");
> > - f = qemu_fopen_channel_output(QIO_CHANNEL(ioc));
> > + f = qemu_file_new_output(QIO_CHANNEL(ioc));
> > object_unref(OBJECT(ioc));
> > ret = qemu_save_device_state(f);
> > if (ret < 0 || qemu_fclose(f) < 0) {
> > @@ -2964,7 +2963,7 @@ void qmp_xen_load_devices_state(const char *filename, Error **errp)
> > return;
> > }
> > qio_channel_set_name(QIO_CHANNEL(ioc), "migration-xen-load-state");
> > - f = qemu_fopen_channel_input(QIO_CHANNEL(ioc));
> > + f = qemu_file_new_input(QIO_CHANNEL(ioc));
> > object_unref(OBJECT(ioc));
> >
> > ret = qemu_loadvm_state(f);
> > diff --git a/tests/unit/test-vmstate.c b/tests/unit/test-vmstate.c
> > index 6a417bb102..72077b5780 100644
> > --- a/tests/unit/test-vmstate.c
> > +++ b/tests/unit/test-vmstate.c
> > @@ -28,7 +28,6 @@
> > #include "migration/vmstate.h"
> > #include "migration/qemu-file-types.h"
> > #include "../migration/qemu-file.h"
> > -#include "../migration/qemu-file-channel.h"
> > #include "../migration/savevm.h"
> > #include "qemu/coroutine.h"
> > #include "qemu/module.h"
> > @@ -52,9 +51,9 @@ static QEMUFile *open_test_file(bool write)
> > }
> > ioc = QIO_CHANNEL(qio_channel_file_new_fd(fd));
> > if (write) {
> > - f = qemu_fopen_channel_output(ioc);
> > + f = qemu_file_new_output(ioc);
> > } else {
> > - f = qemu_fopen_channel_input(ioc);
> > + f = qemu_file_new_input(ioc);
> > }
> > object_unref(OBJECT(ioc));
> > return f;
> > --
> > 2.36.1
> >
> --
> Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
>
With regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
^ permalink raw reply [flat|nested] 51+ messages in thread