[PATCH V4 0/2] migration file URI

[PATCH V4 0/2] migration file URI

[Steve Sistare]

Add the migration URI "file:filename[,offset=offset]".

Fabiano Rosas has submitted the unit tests in the series 
  migration: Test the new "file:" migration

Steve Sistare (2):
  migration: file URI
  migration: file URI offset

 migration/file.c       | 103 +++++++++++++++++++++++++++++++++++++++++++++++++
 migration/file.h       |  14 +++++++
 migration/meson.build  |   1 +
 migration/migration.c  |   5 +++
 migration/trace-events |   4 ++
 qemu-options.hx        |   7 +++-
 6 files changed, 133 insertions(+), 1 deletion(-)
 create mode 100644 migration/file.c
 create mode 100644 migration/file.h

-- 
1.8.3.1

[PATCH V4 1/2] migration: file URI

[Steve Sistare]

Extend the migration URI to support file:<filename>.  This can be used for
any migration scenario that does not require a reverse path.  It can be
used as an alternative to 'exec:cat > file' in minimized containers that
do not contain /bin/sh, and it is easier to use than the fd:<fdname> URI.
It can be used in HMP commands, and as a qemu command-line parameter.

For best performance, guest ram should be shared and x-ignore-shared
should be true, so guest pages are not written to the file, in which case
the guest may remain running.  If ram is not so configured, then the user
is advised to stop the guest first.  Otherwise, a busy guest may re-dirty
the same page, causing it to be appended to the file multiple times,
and the file may grow unboundedly.  That issue is being addressed in the
"fixed-ram" patch series.

Signed-off-by: Steve Sistare <steven.sistare@oracle.com>
Reviewed-by: Fabiano Rosas <farosas@suse.de>
Reviewed-by: Peter Xu <peterx@redhat.com>
---
 migration/file.c       | 62 ++++++++++++++++++++++++++++++++++++++++++++++++++
 migration/file.h       | 14 ++++++++++++
 migration/meson.build  |  1 +
 migration/migration.c  |  5 ++++
 migration/trace-events |  4 ++++
 qemu-options.hx        |  6 ++++-
 6 files changed, 91 insertions(+), 1 deletion(-)
 create mode 100644 migration/file.c
 create mode 100644 migration/file.h

diff --git a/migration/file.c b/migration/file.c
new file mode 100644
index 0000000..8e35827
--- /dev/null
+++ b/migration/file.c
@@ -0,0 +1,62 @@
+/*
+ * Copyright (c) 2021-2023 Oracle and/or its affiliates.
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2.
+ * See the COPYING file in the top-level directory.
+ */
+
+#include "qemu/osdep.h"
+#include "channel.h"
+#include "file.h"
+#include "migration.h"
+#include "io/channel-file.h"
+#include "io/channel-util.h"
+#include "trace.h"
+
+void file_start_outgoing_migration(MigrationState *s, const char *filename,
+                                   Error **errp)
+{
+    g_autoptr(QIOChannelFile) fioc = NULL;
+    QIOChannel *ioc;
+
+    trace_migration_file_outgoing(filename);
+
+    fioc = qio_channel_file_new_path(filename, O_CREAT | O_WRONLY | O_TRUNC,
+                                     0600, errp);
+    if (!fioc) {
+        return;
+    }
+
+    ioc = QIO_CHANNEL(fioc);
+    qio_channel_set_name(ioc, "migration-file-outgoing");
+    migration_channel_connect(s, ioc, NULL, NULL);
+}
+
+static gboolean file_accept_incoming_migration(QIOChannel *ioc,
+                                               GIOCondition condition,
+                                               gpointer opaque)
+{
+    migration_channel_process_incoming(ioc);
+    object_unref(OBJECT(ioc));
+    return G_SOURCE_REMOVE;
+}
+
+void file_start_incoming_migration(const char *filename, Error **errp)
+{
+    QIOChannelFile *fioc = NULL;
+    QIOChannel *ioc;
+
+    trace_migration_file_incoming(filename);
+
+    fioc = qio_channel_file_new_path(filename, O_RDONLY, 0, errp);
+    if (!fioc) {
+        return;
+    }
+
+    ioc = QIO_CHANNEL(fioc);
+    qio_channel_set_name(QIO_CHANNEL(ioc), "migration-file-incoming");
+    qio_channel_add_watch_full(ioc, G_IO_IN,
+                               file_accept_incoming_migration,
+                               NULL, NULL,
+                               g_main_context_get_thread_default());
+}
diff --git a/migration/file.h b/migration/file.h
new file mode 100644
index 0000000..841b94a
--- /dev/null
+++ b/migration/file.h
@@ -0,0 +1,14 @@
+/*
+ * Copyright (c) 2021-2023 Oracle and/or its affiliates.
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2.
+ * See the COPYING file in the top-level directory.
+ */
+
+#ifndef QEMU_MIGRATION_FILE_H
+#define QEMU_MIGRATION_FILE_H
+void file_start_incoming_migration(const char *filename, Error **errp);
+
+void file_start_outgoing_migration(MigrationState *s, const char *filename,
+                                   Error **errp);
+#endif
diff --git a/migration/meson.build b/migration/meson.build
index 1ae2852..92b1cc4 100644
--- a/migration/meson.build
+++ b/migration/meson.build
@@ -16,6 +16,7 @@ system_ss.add(files(
   'dirtyrate.c',
   'exec.c',
   'fd.c',
+  'file.c',
   'global_state.c',
   'migration-hmp-cmds.c',
   'migration.c',
diff --git a/migration/migration.c b/migration/migration.c
index dc05c6f..cfbde86 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -20,6 +20,7 @@
 #include "migration/blocker.h"
 #include "exec.h"
 #include "fd.h"
+#include "file.h"
 #include "socket.h"
 #include "sysemu/runstate.h"
 #include "sysemu/sysemu.h"
@@ -442,6 +443,8 @@ static void qemu_start_incoming_migration(const char *uri, Error **errp)
         exec_start_incoming_migration(p, errp);
     } else if (strstart(uri, "fd:", &p)) {
         fd_start_incoming_migration(p, errp);
+    } else if (strstart(uri, "file:", &p)) {
+        file_start_incoming_migration(p, errp);
     } else {
         error_setg(errp, "unknown migration protocol: %s", uri);
     }
@@ -1662,6 +1665,8 @@ void qmp_migrate(const char *uri, bool has_blk, bool blk,
         exec_start_outgoing_migration(s, p, &local_err);
     } else if (strstart(uri, "fd:", &p)) {
         fd_start_outgoing_migration(s, p, &local_err);
+    } else if (strstart(uri, "file:", &p)) {
+        file_start_outgoing_migration(s, p, &local_err);
     } else {
         if (!(has_resume && resume)) {
             yank_unregister_instance(MIGRATION_YANK_INSTANCE);
diff --git a/migration/trace-events b/migration/trace-events
index cdaef7a..c8c1771 100644
--- a/migration/trace-events
+++ b/migration/trace-events
@@ -307,6 +307,10 @@ migration_exec_incoming(const char *cmd) "cmd=%s"
 migration_fd_outgoing(int fd) "fd=%d"
 migration_fd_incoming(int fd) "fd=%d"
 
+# file.c
+migration_file_outgoing(const char *filename) "filename=%s"
+migration_file_incoming(const char *filename) "filename=%s"
+
 # socket.c
 migration_socket_incoming_accepted(void) ""
 migration_socket_outgoing_connected(const char *hostname) "hostname=%s"
diff --git a/qemu-options.hx b/qemu-options.hx
index b57489d..5aab8fb 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -4622,6 +4622,7 @@ DEF("incoming", HAS_ARG, QEMU_OPTION_incoming, \
     "                prepare for incoming migration, listen on\n" \
     "                specified protocol and socket address\n" \
     "-incoming fd:fd\n" \
+    "-incoming file:filename\n" \
     "-incoming exec:cmdline\n" \
     "                accept incoming migration on given file descriptor\n" \
     "                or from given external command\n" \
@@ -4638,7 +4639,10 @@ SRST
     Prepare for incoming migration, listen on a given unix socket.
 
 ``-incoming fd:fd``
-    Accept incoming migration from a given filedescriptor.
+    Accept incoming migration from a given file descriptor.
+
+``-incoming file:filename``
+    Accept incoming migration from a given file.
 
 ``-incoming exec:cmdline``
     Accept incoming migration as an output from specified external
-- 
1.8.3.1

[PATCH V4 2/2] migration: file URI offset

[Steve Sistare]

Allow an offset option to be specified as part of the file URI, in
the form "file:filename,offset=offset", where offset accepts the common
size suffixes, or the 0x prefix, but not both.  Migration data is written
to and read from the file starting at offset.  If unspecified, it defaults
to 0.

This is needed by libvirt to store its own data at the head of the file.

Suggested-by: Daniel P. Berrange <berrange@redhat.com>
Signed-off-by: Steve Sistare <steven.sistare@oracle.com>
Reviewed-by: Peter Xu <peterx@redhat.com>
---
 migration/file.c | 45 +++++++++++++++++++++++++++++++++++++++++++--
 qemu-options.hx  |  7 ++++---
 2 files changed, 47 insertions(+), 5 deletions(-)

diff --git a/migration/file.c b/migration/file.c
index 8e35827..61e2a37 100644
--- a/migration/file.c
+++ b/migration/file.c
@@ -6,6 +6,8 @@
  */
 
 #include "qemu/osdep.h"
+#include "qemu/cutils.h"
+#include "qapi/error.h"
 #include "channel.h"
 #include "file.h"
 #include "migration.h"
@@ -13,14 +15,41 @@
 #include "io/channel-util.h"
 #include "trace.h"
 
-void file_start_outgoing_migration(MigrationState *s, const char *filename,
+#define OFFSET_OPTION ",offset="
+
+/* Remove the offset option from @filespec and return it in @offsetp. */
+
+static int file_parse_offset(char *filespec, uint64_t *offsetp, Error **errp)
+{
+    char *option = strstr(filespec, OFFSET_OPTION);
+    int ret;
+
+    if (option) {
+        *option = 0;
+        option += sizeof(OFFSET_OPTION) - 1;
+        ret = qemu_strtosz(option, NULL, offsetp);
+        if (ret) {
+            error_setg_errno(errp, -ret, "file URI has bad offset %s", option);
+            return -1;
+        }
+    }
+    return 0;
+}
+
+void file_start_outgoing_migration(MigrationState *s, const char *filespec,
                                    Error **errp)
 {
+    g_autofree char *filename = g_strdup(filespec);
     g_autoptr(QIOChannelFile) fioc = NULL;
+    uint64_t offset = 0;
     QIOChannel *ioc;
 
     trace_migration_file_outgoing(filename);
 
+    if (file_parse_offset(filename, &offset, errp)) {
+        return;
+    }
+
     fioc = qio_channel_file_new_path(filename, O_CREAT | O_WRONLY | O_TRUNC,
                                      0600, errp);
     if (!fioc) {
@@ -28,6 +57,9 @@ void file_start_outgoing_migration(MigrationState *s, const char *filename,
     }
 
     ioc = QIO_CHANNEL(fioc);
+    if (offset && qio_channel_io_seek(ioc, offset, SEEK_SET, errp) < 0) {
+        return;
+    }
     qio_channel_set_name(ioc, "migration-file-outgoing");
     migration_channel_connect(s, ioc, NULL, NULL);
 }
@@ -41,19 +73,28 @@ static gboolean file_accept_incoming_migration(QIOChannel *ioc,
     return G_SOURCE_REMOVE;
 }
 
-void file_start_incoming_migration(const char *filename, Error **errp)
+void file_start_incoming_migration(const char *filespec, Error **errp)
 {
+    g_autofree char *filename = g_strdup(filespec);
     QIOChannelFile *fioc = NULL;
+    uint64_t offset = 0;
     QIOChannel *ioc;
 
     trace_migration_file_incoming(filename);
 
+    if (file_parse_offset(filename, &offset, errp)) {
+        return;
+    }
+
     fioc = qio_channel_file_new_path(filename, O_RDONLY, 0, errp);
     if (!fioc) {
         return;
     }
 
     ioc = QIO_CHANNEL(fioc);
+    if (offset && qio_channel_io_seek(ioc, offset, SEEK_SET, errp) < 0) {
+        return;
+    }
     qio_channel_set_name(QIO_CHANNEL(ioc), "migration-file-incoming");
     qio_channel_add_watch_full(ioc, G_IO_IN,
                                file_accept_incoming_migration,
diff --git a/qemu-options.hx b/qemu-options.hx
index 5aab8fb..5a92210 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -4622,7 +4622,7 @@ DEF("incoming", HAS_ARG, QEMU_OPTION_incoming, \
     "                prepare for incoming migration, listen on\n" \
     "                specified protocol and socket address\n" \
     "-incoming fd:fd\n" \
-    "-incoming file:filename\n" \
+    "-incoming file:filename[,offset=offset]\n" \
     "-incoming exec:cmdline\n" \
     "                accept incoming migration on given file descriptor\n" \
     "                or from given external command\n" \
@@ -4641,8 +4641,9 @@ SRST
 ``-incoming fd:fd``
     Accept incoming migration from a given file descriptor.
 
-``-incoming file:filename``
-    Accept incoming migration from a given file.
+``-incoming file:filename[,offset=offset]``
+    Accept incoming migration from a given file starting at offset.
+    offset allows the common size suffixes, or a 0x prefix, but not both.
 
 ``-incoming exec:cmdline``
     Accept incoming migration as an output from specified external
-- 
1.8.3.1

Re: [PATCH V4 0/2] migration file URI

[Michael Galaxy]

Tested-by: Michael Galaxy <mgalaxy@akamai.com>
Reviewed-by: Michael Galaxy <mgalaxy@akamai.com>

On 6/30/23 09:25, Steve Sistare wrote:
> Add the migration URI "file:filename[,offset=offset]".
>
> Fabiano Rosas has submitted the unit tests in the series
>    migration: Test the new "file:" migration
>
> Steve Sistare (2):
>    migration: file URI
>    migration: file URI offset
>
>   migration/file.c       | 103 +++++++++++++++++++++++++++++++++++++++++++++++++
>   migration/file.h       |  14 +++++++
>   migration/meson.build  |   1 +
>   migration/migration.c  |   5 +++
>   migration/trace-events |   4 ++
>   qemu-options.hx        |   7 +++-
>   6 files changed, 133 insertions(+), 1 deletion(-)
>   create mode 100644 migration/file.c
>   create mode 100644 migration/file.h
>

Re: [PATCH V4 0/2] migration file URI

[Claudio Fontana]

Hello,

this series is all reviewed,

and is needed as a precondition for further work to improve dramatically the performance of virsh save, virsh restore
when migrating to disk, can it be merged?

Thanks,

Claudio

On 6/30/23 16:25, Steve Sistare wrote:
> Add the migration URI "file:filename[,offset=offset]".
> 
> Fabiano Rosas has submitted the unit tests in the series 
>   migration: Test the new "file:" migration
> 
> Steve Sistare (2):
>   migration: file URI
>   migration: file URI offset
> 
>  migration/file.c       | 103 +++++++++++++++++++++++++++++++++++++++++++++++++
>  migration/file.h       |  14 +++++++
>  migration/meson.build  |   1 +
>  migration/migration.c  |   5 +++
>  migration/trace-events |   4 ++
>  qemu-options.hx        |   7 +++-
>  6 files changed, 133 insertions(+), 1 deletion(-)
>  create mode 100644 migration/file.c
>  create mode 100644 migration/file.h
> 

Re: [PATCH V4 0/2] migration file URI

[Philippe Mathieu-Daudé]

Hi Claudio,

On 22/8/23 14:00, Claudio Fontana wrote:
> Hello,
> 
> this series is all reviewed,
> 
> and is needed as a precondition for further work to improve dramatically the performance of virsh save, virsh restore
> when migrating to disk, can it be merged?

$ ./scripts/get_maintainer.pl -f migration/meson.build
Juan Quintela <quintela@redhat.com> (maintainer:Migration)
Peter Xu <peterx@redhat.com> (reviewer:Migration)
Leonardo Bras <leobras@redhat.com> (reviewer:Migration)
qemu-devel@nongnu.org (open list:All patches CC here)

One maintainer, one single point of failure. When the
maintainer is busy or offline (vacations?) then everybody
is stuck.

This is usually solved by adding co-maintainers.

Juan, would you accept having co-maintainers helping you
dealing with the merge process? I'm not volunteering, but
this can be a good opportunity to make a formal request to
the community.

Regards,

Phil.

Re: [PATCH V4 0/2] migration file URI

[Claudio Fontana]

On 8/22/23 15:25, Philippe Mathieu-Daudé wrote:
> Hi Claudio,
> 
> On 22/8/23 14:00, Claudio Fontana wrote:
>> Hello,
>>
>> this series is all reviewed,
>>
>> and is needed as a precondition for further work to improve dramatically the performance of virsh save, virsh restore
>> when migrating to disk, can it be merged?
> 
> $ ./scripts/get_maintainer.pl -f migration/meson.build
> Juan Quintela <quintela@redhat.com> (maintainer:Migration)
> Peter Xu <peterx@redhat.com> (reviewer:Migration)
> Leonardo Bras <leobras@redhat.com> (reviewer:Migration)
> qemu-devel@nongnu.org (open list:All patches CC here)
> 
> One maintainer, one single point of failure. When the
> maintainer is busy or offline (vacations?) then everybody
> is stuck.
> 
> This is usually solved by adding co-maintainers.
> 
> Juan, would you accept having co-maintainers helping you
> dealing with the merge process? I'm not volunteering, but
> this can be a good opportunity to make a formal request to
> the community.
> 
> Regards,
> 
> Phil.
> 

Thanks Philippe,

I would like to highlight to the QEMU community here how important this is for SUSE,

to see progress for all the series in the migration area that are currently still waiting and competing for attention.

The specific features and improvements we are developing or helping to review are a priority for us,
and we are concerned about the rate of progress with the existing governance processes.

Fabiano is investing a lot of his attention to this area, with features, bugfixes and reviews,
but of course it is up to the community as a whole to address the issue so that reviewed work is merged.

My attempt here is to make sure that it is recognized as a problem, so it can be hopefully be addressed by the community in a timely fashion,
and we can all benefit from the already developed and reviewed work that is pending.

Thanks!

CLaudio

Re: [PATCH V4 1/2] migration: file URI

[Daniel P. Berrangé]

On Fri, Jun 30, 2023 at 07:25:07AM -0700, Steve Sistare wrote:
> Extend the migration URI to support file:<filename>.  This can be used for
> any migration scenario that does not require a reverse path.  It can be
> used as an alternative to 'exec:cat > file' in minimized containers that
> do not contain /bin/sh, and it is easier to use than the fd:<fdname> URI.
> It can be used in HMP commands, and as a qemu command-line parameter.
> 
> For best performance, guest ram should be shared and x-ignore-shared
> should be true, so guest pages are not written to the file, in which case
> the guest may remain running.  If ram is not so configured, then the user
> is advised to stop the guest first.  Otherwise, a busy guest may re-dirty
> the same page, causing it to be appended to the file multiple times,
> and the file may grow unboundedly.  That issue is being addressed in the
> "fixed-ram" patch series.
> 
> Signed-off-by: Steve Sistare <steven.sistare@oracle.com>
> Reviewed-by: Fabiano Rosas <farosas@suse.de>
> Reviewed-by: Peter Xu <peterx@redhat.com>
> ---
>  migration/file.c       | 62 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  migration/file.h       | 14 ++++++++++++
>  migration/meson.build  |  1 +
>  migration/migration.c  |  5 ++++
>  migration/trace-events |  4 ++++
>  qemu-options.hx        |  6 ++++-
>  6 files changed, 91 insertions(+), 1 deletion(-)
>  create mode 100644 migration/file.c
>  create mode 100644 migration/file.h
> 
> diff --git a/migration/file.c b/migration/file.c
> new file mode 100644
> index 0000000..8e35827
> --- /dev/null
> +++ b/migration/file.c
> @@ -0,0 +1,62 @@
> +/*
> + * Copyright (c) 2021-2023 Oracle and/or its affiliates.
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2.

Was it an intentional decision to assign this under the version 2 *only* ?

QEMU's LICENSE file states

[quote]
As of July 2013, contributions under version 2 of the GNU General Public
License (and no later version) are only accepted for the following files
or directories: bsd-user/, linux-user/, hw/vfio/, hw/xen/xen_pt*.
[/quote]

Thus we'd expect this new file to be version 2, or later.

> + * See the COPYING file in the top-level directory.
> + */
> +
> +#include "qemu/osdep.h"
> +#include "channel.h"
> +#include "file.h"
> +#include "migration.h"
> +#include "io/channel-file.h"
> +#include "io/channel-util.h"
> +#include "trace.h"
> +
> +void file_start_outgoing_migration(MigrationState *s, const char *filename,
> +                                   Error **errp)
> +{
> +    g_autoptr(QIOChannelFile) fioc = NULL;
> +    QIOChannel *ioc;
> +
> +    trace_migration_file_outgoing(filename);
> +
> +    fioc = qio_channel_file_new_path(filename, O_CREAT | O_WRONLY | O_TRUNC,
> +                                     0600, errp);
> +    if (!fioc) {
> +        return;
> +    }
> +
> +    ioc = QIO_CHANNEL(fioc);
> +    qio_channel_set_name(ioc, "migration-file-outgoing");
> +    migration_channel_connect(s, ioc, NULL, NULL);
> +}
> +
> +static gboolean file_accept_incoming_migration(QIOChannel *ioc,
> +                                               GIOCondition condition,
> +                                               gpointer opaque)
> +{
> +    migration_channel_process_incoming(ioc);
> +    object_unref(OBJECT(ioc));
> +    return G_SOURCE_REMOVE;
> +}
> +
> +void file_start_incoming_migration(const char *filename, Error **errp)
> +{
> +    QIOChannelFile *fioc = NULL;
> +    QIOChannel *ioc;
> +
> +    trace_migration_file_incoming(filename);
> +
> +    fioc = qio_channel_file_new_path(filename, O_RDONLY, 0, errp);
> +    if (!fioc) {
> +        return;
> +    }
> +
> +    ioc = QIO_CHANNEL(fioc);
> +    qio_channel_set_name(QIO_CHANNEL(ioc), "migration-file-incoming");
> +    qio_channel_add_watch_full(ioc, G_IO_IN,
> +                               file_accept_incoming_migration,
> +                               NULL, NULL,
> +                               g_main_context_get_thread_default());
> +}
> diff --git a/migration/file.h b/migration/file.h
> new file mode 100644
> index 0000000..841b94a
> --- /dev/null
> +++ b/migration/file.h
> @@ -0,0 +1,14 @@
> +/*
> + * Copyright (c) 2021-2023 Oracle and/or its affiliates.
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2.
> + * See the COPYING file in the top-level directory.
> + */
> +
> +#ifndef QEMU_MIGRATION_FILE_H
> +#define QEMU_MIGRATION_FILE_H
> +void file_start_incoming_migration(const char *filename, Error **errp);
> +
> +void file_start_outgoing_migration(MigrationState *s, const char *filename,
> +                                   Error **errp);
> +#endif
> diff --git a/migration/meson.build b/migration/meson.build
> index 1ae2852..92b1cc4 100644
> --- a/migration/meson.build
> +++ b/migration/meson.build
> @@ -16,6 +16,7 @@ system_ss.add(files(
>    'dirtyrate.c',
>    'exec.c',
>    'fd.c',
> +  'file.c',
>    'global_state.c',
>    'migration-hmp-cmds.c',
>    'migration.c',
> diff --git a/migration/migration.c b/migration/migration.c
> index dc05c6f..cfbde86 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -20,6 +20,7 @@
>  #include "migration/blocker.h"
>  #include "exec.h"
>  #include "fd.h"
> +#include "file.h"
>  #include "socket.h"
>  #include "sysemu/runstate.h"
>  #include "sysemu/sysemu.h"
> @@ -442,6 +443,8 @@ static void qemu_start_incoming_migration(const char *uri, Error **errp)
>          exec_start_incoming_migration(p, errp);
>      } else if (strstart(uri, "fd:", &p)) {
>          fd_start_incoming_migration(p, errp);
> +    } else if (strstart(uri, "file:", &p)) {
> +        file_start_incoming_migration(p, errp);
>      } else {
>          error_setg(errp, "unknown migration protocol: %s", uri);
>      }
> @@ -1662,6 +1665,8 @@ void qmp_migrate(const char *uri, bool has_blk, bool blk,
>          exec_start_outgoing_migration(s, p, &local_err);
>      } else if (strstart(uri, "fd:", &p)) {
>          fd_start_outgoing_migration(s, p, &local_err);
> +    } else if (strstart(uri, "file:", &p)) {
> +        file_start_outgoing_migration(s, p, &local_err);
>      } else {
>          if (!(has_resume && resume)) {
>              yank_unregister_instance(MIGRATION_YANK_INSTANCE);

This section will clash with the other pending reviewed series
that introduces a formall QAPI schema for migration addresses.

Either this or that series will need an update depending on
which Juan decides to merge first.  The changes should be
fairly simple to resolve so not a big deal.

> diff --git a/migration/trace-events b/migration/trace-events
> index cdaef7a..c8c1771 100644
> --- a/migration/trace-events
> +++ b/migration/trace-events
> @@ -307,6 +307,10 @@ migration_exec_incoming(const char *cmd) "cmd=%s"
>  migration_fd_outgoing(int fd) "fd=%d"
>  migration_fd_incoming(int fd) "fd=%d"
>  
> +# file.c
> +migration_file_outgoing(const char *filename) "filename=%s"
> +migration_file_incoming(const char *filename) "filename=%s"
> +
>  # socket.c
>  migration_socket_incoming_accepted(void) ""
>  migration_socket_outgoing_connected(const char *hostname) "hostname=%s"
> diff --git a/qemu-options.hx b/qemu-options.hx
> index b57489d..5aab8fb 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -4622,6 +4622,7 @@ DEF("incoming", HAS_ARG, QEMU_OPTION_incoming, \
>      "                prepare for incoming migration, listen on\n" \
>      "                specified protocol and socket address\n" \
>      "-incoming fd:fd\n" \
> +    "-incoming file:filename\n" \
>      "-incoming exec:cmdline\n" \
>      "                accept incoming migration on given file descriptor\n" \
>      "                or from given external command\n" \
> @@ -4638,7 +4639,10 @@ SRST
>      Prepare for incoming migration, listen on a given unix socket.
>  
>  ``-incoming fd:fd``
> -    Accept incoming migration from a given filedescriptor.
> +    Accept incoming migration from a given file descriptor.
> +
> +``-incoming file:filename``
> +    Accept incoming migration from a given file.
>  
>  ``-incoming exec:cmdline``
>      Accept incoming migration as an output from specified external
> -- 
> 1.8.3.1
> 

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 :|

Re: [PATCH V4 1/2] migration: file URI

[Steven Sistare]

On 8/30/2023 9:16 AM, Daniel P. Berrangé wrote:
> On Fri, Jun 30, 2023 at 07:25:07AM -0700, Steve Sistare wrote:
>> Extend the migration URI to support file:<filename>.  This can be used for
>> any migration scenario that does not require a reverse path.  It can be
>> used as an alternative to 'exec:cat > file' in minimized containers that
>> do not contain /bin/sh, and it is easier to use than the fd:<fdname> URI.
>> It can be used in HMP commands, and as a qemu command-line parameter.
>>
>> For best performance, guest ram should be shared and x-ignore-shared
>> should be true, so guest pages are not written to the file, in which case
>> the guest may remain running.  If ram is not so configured, then the user
>> is advised to stop the guest first.  Otherwise, a busy guest may re-dirty
>> the same page, causing it to be appended to the file multiple times,
>> and the file may grow unboundedly.  That issue is being addressed in the
>> "fixed-ram" patch series.
>>
>> Signed-off-by: Steve Sistare <steven.sistare@oracle.com>
>> Reviewed-by: Fabiano Rosas <farosas@suse.de>
>> Reviewed-by: Peter Xu <peterx@redhat.com>
>> ---
>>  migration/file.c       | 62 ++++++++++++++++++++++++++++++++++++++++++++++++++
>>  migration/file.h       | 14 ++++++++++++
>>  migration/meson.build  |  1 +
>>  migration/migration.c  |  5 ++++
>>  migration/trace-events |  4 ++++
>>  qemu-options.hx        |  6 ++++-
>>  6 files changed, 91 insertions(+), 1 deletion(-)
>>  create mode 100644 migration/file.c
>>  create mode 100644 migration/file.h
>>
>> diff --git a/migration/file.c b/migration/file.c
>> new file mode 100644
>> index 0000000..8e35827
>> --- /dev/null
>> +++ b/migration/file.c
>> @@ -0,0 +1,62 @@
>> +/*
>> + * Copyright (c) 2021-2023 Oracle and/or its affiliates.
>> + *
>> + * This work is licensed under the terms of the GNU GPL, version 2.
> 
> Was it an intentional decision to assign this under the version 2 *only* ?
> 
> QEMU's LICENSE file states
> 
> [quote]
> As of July 2013, contributions under version 2 of the GNU General Public
> License (and no later version) are only accepted for the following files
> or directories: bsd-user/, linux-user/, hw/vfio/, hw/xen/xen_pt*.
> [/quote]
> 
> Thus we'd expect this new file to be version 2, or later.

My mistake, sorry.  It should say "GNU GPL, version 2 or later"

- Steve

>> + * See the COPYING file in the top-level directory.
>> + */
>> +
>> +#include "qemu/osdep.h"
>> +#include "channel.h"
>> +#include "file.h"
>> +#include "migration.h"
>> +#include "io/channel-file.h"
>> +#include "io/channel-util.h"
>> +#include "trace.h"
>> +
>> +void file_start_outgoing_migration(MigrationState *s, const char *filename,
>> +                                   Error **errp)
>> +{
>> +    g_autoptr(QIOChannelFile) fioc = NULL;
>> +    QIOChannel *ioc;
>> +
>> +    trace_migration_file_outgoing(filename);
>> +
>> +    fioc = qio_channel_file_new_path(filename, O_CREAT | O_WRONLY | O_TRUNC,
>> +                                     0600, errp);
>> +    if (!fioc) {
>> +        return;
>> +    }
>> +
>> +    ioc = QIO_CHANNEL(fioc);
>> +    qio_channel_set_name(ioc, "migration-file-outgoing");
>> +    migration_channel_connect(s, ioc, NULL, NULL);
>> +}
>> +
>> +static gboolean file_accept_incoming_migration(QIOChannel *ioc,
>> +                                               GIOCondition condition,
>> +                                               gpointer opaque)
>> +{
>> +    migration_channel_process_incoming(ioc);
>> +    object_unref(OBJECT(ioc));
>> +    return G_SOURCE_REMOVE;
>> +}
>> +
>> +void file_start_incoming_migration(const char *filename, Error **errp)
>> +{
>> +    QIOChannelFile *fioc = NULL;
>> +    QIOChannel *ioc;
>> +
>> +    trace_migration_file_incoming(filename);
>> +
>> +    fioc = qio_channel_file_new_path(filename, O_RDONLY, 0, errp);
>> +    if (!fioc) {
>> +        return;
>> +    }
>> +
>> +    ioc = QIO_CHANNEL(fioc);
>> +    qio_channel_set_name(QIO_CHANNEL(ioc), "migration-file-incoming");
>> +    qio_channel_add_watch_full(ioc, G_IO_IN,
>> +                               file_accept_incoming_migration,
>> +                               NULL, NULL,
>> +                               g_main_context_get_thread_default());
>> +}
>> diff --git a/migration/file.h b/migration/file.h
>> new file mode 100644
>> index 0000000..841b94a
>> --- /dev/null
>> +++ b/migration/file.h
>> @@ -0,0 +1,14 @@
>> +/*
>> + * Copyright (c) 2021-2023 Oracle and/or its affiliates.
>> + *
>> + * This work is licensed under the terms of the GNU GPL, version 2.
>> + * See the COPYING file in the top-level directory.
>> + */
>> +
>> +#ifndef QEMU_MIGRATION_FILE_H
>> +#define QEMU_MIGRATION_FILE_H
>> +void file_start_incoming_migration(const char *filename, Error **errp);
>> +
>> +void file_start_outgoing_migration(MigrationState *s, const char *filename,
>> +                                   Error **errp);
>> +#endif
>> diff --git a/migration/meson.build b/migration/meson.build
>> index 1ae2852..92b1cc4 100644
>> --- a/migration/meson.build
>> +++ b/migration/meson.build
>> @@ -16,6 +16,7 @@ system_ss.add(files(
>>    'dirtyrate.c',
>>    'exec.c',
>>    'fd.c',
>> +  'file.c',
>>    'global_state.c',
>>    'migration-hmp-cmds.c',
>>    'migration.c',
>> diff --git a/migration/migration.c b/migration/migration.c
>> index dc05c6f..cfbde86 100644
>> --- a/migration/migration.c
>> +++ b/migration/migration.c
>> @@ -20,6 +20,7 @@
>>  #include "migration/blocker.h"
>>  #include "exec.h"
>>  #include "fd.h"
>> +#include "file.h"
>>  #include "socket.h"
>>  #include "sysemu/runstate.h"
>>  #include "sysemu/sysemu.h"
>> @@ -442,6 +443,8 @@ static void qemu_start_incoming_migration(const char *uri, Error **errp)
>>          exec_start_incoming_migration(p, errp);
>>      } else if (strstart(uri, "fd:", &p)) {
>>          fd_start_incoming_migration(p, errp);
>> +    } else if (strstart(uri, "file:", &p)) {
>> +        file_start_incoming_migration(p, errp);
>>      } else {
>>          error_setg(errp, "unknown migration protocol: %s", uri);
>>      }
>> @@ -1662,6 +1665,8 @@ void qmp_migrate(const char *uri, bool has_blk, bool blk,
>>          exec_start_outgoing_migration(s, p, &local_err);
>>      } else if (strstart(uri, "fd:", &p)) {
>>          fd_start_outgoing_migration(s, p, &local_err);
>> +    } else if (strstart(uri, "file:", &p)) {
>> +        file_start_outgoing_migration(s, p, &local_err);
>>      } else {
>>          if (!(has_resume && resume)) {
>>              yank_unregister_instance(MIGRATION_YANK_INSTANCE);
> 
> This section will clash with the other pending reviewed series
> that introduces a formall QAPI schema for migration addresses.
> 
> Either this or that series will need an update depending on
> which Juan decides to merge first.  The changes should be
> fairly simple to resolve so not a big deal.
> 
>> diff --git a/migration/trace-events b/migration/trace-events
>> index cdaef7a..c8c1771 100644
>> --- a/migration/trace-events
>> +++ b/migration/trace-events
>> @@ -307,6 +307,10 @@ migration_exec_incoming(const char *cmd) "cmd=%s"
>>  migration_fd_outgoing(int fd) "fd=%d"
>>  migration_fd_incoming(int fd) "fd=%d"
>>  
>> +# file.c
>> +migration_file_outgoing(const char *filename) "filename=%s"
>> +migration_file_incoming(const char *filename) "filename=%s"
>> +
>>  # socket.c
>>  migration_socket_incoming_accepted(void) ""
>>  migration_socket_outgoing_connected(const char *hostname) "hostname=%s"
>> diff --git a/qemu-options.hx b/qemu-options.hx
>> index b57489d..5aab8fb 100644
>> --- a/qemu-options.hx
>> +++ b/qemu-options.hx
>> @@ -4622,6 +4622,7 @@ DEF("incoming", HAS_ARG, QEMU_OPTION_incoming, \
>>      "                prepare for incoming migration, listen on\n" \
>>      "                specified protocol and socket address\n" \
>>      "-incoming fd:fd\n" \
>> +    "-incoming file:filename\n" \
>>      "-incoming exec:cmdline\n" \
>>      "                accept incoming migration on given file descriptor\n" \
>>      "                or from given external command\n" \
>> @@ -4638,7 +4639,10 @@ SRST
>>      Prepare for incoming migration, listen on a given unix socket.
>>  
>>  ``-incoming fd:fd``
>> -    Accept incoming migration from a given filedescriptor.
>> +    Accept incoming migration from a given file descriptor.
>> +
>> +``-incoming file:filename``
>> +    Accept incoming migration from a given file.
>>  
>>  ``-incoming exec:cmdline``
>>      Accept incoming migration as an output from specified external
>> -- 
>> 1.8.3.1
>>
> 
> With regards,
> Daniel

Re: [PATCH V4 1/2] migration: file URI

[Daniel P. Berrangé]

On Wed, Aug 30, 2023 at 10:15:43AM -0400, Steven Sistare wrote:
> On 8/30/2023 9:16 AM, Daniel P. Berrangé wrote:
> > On Fri, Jun 30, 2023 at 07:25:07AM -0700, Steve Sistare wrote:
> >> Extend the migration URI to support file:<filename>.  This can be used for
> >> any migration scenario that does not require a reverse path.  It can be
> >> used as an alternative to 'exec:cat > file' in minimized containers that
> >> do not contain /bin/sh, and it is easier to use than the fd:<fdname> URI.
> >> It can be used in HMP commands, and as a qemu command-line parameter.
> >>
> >> For best performance, guest ram should be shared and x-ignore-shared
> >> should be true, so guest pages are not written to the file, in which case
> >> the guest may remain running.  If ram is not so configured, then the user
> >> is advised to stop the guest first.  Otherwise, a busy guest may re-dirty
> >> the same page, causing it to be appended to the file multiple times,
> >> and the file may grow unboundedly.  That issue is being addressed in the
> >> "fixed-ram" patch series.
> >>
> >> Signed-off-by: Steve Sistare <steven.sistare@oracle.com>
> >> Reviewed-by: Fabiano Rosas <farosas@suse.de>
> >> Reviewed-by: Peter Xu <peterx@redhat.com>
> >> ---
> >>  migration/file.c       | 62 ++++++++++++++++++++++++++++++++++++++++++++++++++
> >>  migration/file.h       | 14 ++++++++++++
> >>  migration/meson.build  |  1 +
> >>  migration/migration.c  |  5 ++++
> >>  migration/trace-events |  4 ++++
> >>  qemu-options.hx        |  6 ++++-
> >>  6 files changed, 91 insertions(+), 1 deletion(-)
> >>  create mode 100644 migration/file.c
> >>  create mode 100644 migration/file.h
> >>
> >> diff --git a/migration/file.c b/migration/file.c
> >> new file mode 100644
> >> index 0000000..8e35827
> >> --- /dev/null
> >> +++ b/migration/file.c
> >> @@ -0,0 +1,62 @@
> >> +/*
> >> + * Copyright (c) 2021-2023 Oracle and/or its affiliates.
> >> + *
> >> + * This work is licensed under the terms of the GNU GPL, version 2.
> > 
> > Was it an intentional decision to assign this under the version 2 *only* ?
> > 
> > QEMU's LICENSE file states
> > 
> > [quote]
> > As of July 2013, contributions under version 2 of the GNU General Public
> > License (and no later version) are only accepted for the following files
> > or directories: bsd-user/, linux-user/, hw/vfio/, hw/xen/xen_pt*.
> > [/quote]
> > 
> > Thus we'd expect this new file to be version 2, or later.
> 
> My mistake, sorry.  It should say "GNU GPL, version 2 or later"

Could you re-post, as aside from that, this series looks
ready for merge.


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 :|

Re: [PATCH V4 1/2] migration: file URI

[Steven Sistare]

On 9/8/2023 6:52 AM, Daniel P. Berrangé wrote:
> On Wed, Aug 30, 2023 at 10:15:43AM -0400, Steven Sistare wrote:
>> On 8/30/2023 9:16 AM, Daniel P. Berrangé wrote:
>>> On Fri, Jun 30, 2023 at 07:25:07AM -0700, Steve Sistare wrote:
>>>> Extend the migration URI to support file:<filename>.  This can be used for
>>>> any migration scenario that does not require a reverse path.  It can be
>>>> used as an alternative to 'exec:cat > file' in minimized containers that
>>>> do not contain /bin/sh, and it is easier to use than the fd:<fdname> URI.
>>>> It can be used in HMP commands, and as a qemu command-line parameter.
>>>>
>>>> For best performance, guest ram should be shared and x-ignore-shared
>>>> should be true, so guest pages are not written to the file, in which case
>>>> the guest may remain running.  If ram is not so configured, then the user
>>>> is advised to stop the guest first.  Otherwise, a busy guest may re-dirty
>>>> the same page, causing it to be appended to the file multiple times,
>>>> and the file may grow unboundedly.  That issue is being addressed in the
>>>> "fixed-ram" patch series.
>>>>
>>>> Signed-off-by: Steve Sistare <steven.sistare@oracle.com>
>>>> Reviewed-by: Fabiano Rosas <farosas@suse.de>
>>>> Reviewed-by: Peter Xu <peterx@redhat.com>
>>>> ---
>>>>  migration/file.c       | 62 ++++++++++++++++++++++++++++++++++++++++++++++++++
>>>>  migration/file.h       | 14 ++++++++++++
>>>>  migration/meson.build  |  1 +
>>>>  migration/migration.c  |  5 ++++
>>>>  migration/trace-events |  4 ++++
>>>>  qemu-options.hx        |  6 ++++-
>>>>  6 files changed, 91 insertions(+), 1 deletion(-)
>>>>  create mode 100644 migration/file.c
>>>>  create mode 100644 migration/file.h
>>>>
>>>> diff --git a/migration/file.c b/migration/file.c
>>>> new file mode 100644
>>>> index 0000000..8e35827
>>>> --- /dev/null
>>>> +++ b/migration/file.c
>>>> @@ -0,0 +1,62 @@
>>>> +/*
>>>> + * Copyright (c) 2021-2023 Oracle and/or its affiliates.
>>>> + *
>>>> + * This work is licensed under the terms of the GNU GPL, version 2.
>>>
>>> Was it an intentional decision to assign this under the version 2 *only* ?
>>>
>>> QEMU's LICENSE file states
>>>
>>> [quote]
>>> As of July 2013, contributions under version 2 of the GNU General Public
>>> License (and no later version) are only accepted for the following files
>>> or directories: bsd-user/, linux-user/, hw/vfio/, hw/xen/xen_pt*.
>>> [/quote]
>>>
>>> Thus we'd expect this new file to be version 2, or later.
>>
>> My mistake, sorry.  It should say "GNU GPL, version 2 or later"
> 
> Could you re-post, as aside from that, this series looks
> ready for merge.

Done, thanks, see V5.

- Steve

Re: [PATCH V4 0/2] migration file URI

[Claudio Fontana]

On 8/30/23 15:09, Claudio Fontana wrote:
> On 8/22/23 15:25, Philippe Mathieu-Daudé wrote:
>> Hi Claudio,
>>
>> On 22/8/23 14:00, Claudio Fontana wrote:
>>> Hello,
>>>
>>> this series is all reviewed,
>>>
>>> and is needed as a precondition for further work to improve dramatically the performance of virsh save, virsh restore
>>> when migrating to disk, can it be merged?
>>
>> $ ./scripts/get_maintainer.pl -f migration/meson.build
>> Juan Quintela <quintela@redhat.com> (maintainer:Migration)
>> Peter Xu <peterx@redhat.com> (reviewer:Migration)
>> Leonardo Bras <leobras@redhat.com> (reviewer:Migration)
>> qemu-devel@nongnu.org (open list:All patches CC here)
>>
>> One maintainer, one single point of failure. When the
>> maintainer is busy or offline (vacations?) then everybody
>> is stuck.
>>
>> This is usually solved by adding co-maintainers.
>>
>> Juan, would you accept having co-maintainers helping you
>> dealing with the merge process? I'm not volunteering, but
>> this can be a good opportunity to make a formal request to
>> the community.
>>
>> Regards,
>>
>> Phil.
>>
> 
> Thanks Philippe,
> 
> I would like to highlight to the QEMU community here how important this is for SUSE,
> 
> to see progress for all the series in the migration area that are currently still waiting and competing for attention.
> 
> The specific features and improvements we are developing or helping to review are a priority for us,
> and we are concerned about the rate of progress with the existing governance processes.
> 
> Fabiano is investing a lot of his attention to this area, with features, bugfixes and reviews,
> but of course it is up to the community as a whole to address the issue so that reviewed work is merged.
> 
> My attempt here is to make sure that it is recognized as a problem, so it can be hopefully be addressed by the community in a timely fashion,
> and we can all benefit from the already developed and reviewed work that is pending.
> 
> Thanks!
> 
> CLaudio
> 
> 

Hi Juan, any comments? Would additional help for co-maintenance of live migration help?
How can we make things proceed?

From our (SUSE) side we are willing to help as much as we can, and all options are open,
whatever it takes to get the multiple series currently waiting to flow again.

Thanks,

Claudio

migration maintenance, governance [Was: Re: [PATCH V4 0/2] migration file URI]

[Claudio Fontana]

On 9/13/23 15:00, Claudio Fontana wrote:
> On 8/30/23 15:09, Claudio Fontana wrote:
>> On 8/22/23 15:25, Philippe Mathieu-Daudé wrote:
>>> Hi Claudio,
>>>
>>> On 22/8/23 14:00, Claudio Fontana wrote:
>>>> Hello,
>>>>
>>>> this series is all reviewed,
>>>>
>>>> and is needed as a precondition for further work to improve dramatically the performance of virsh save, virsh restore
>>>> when migrating to disk, can it be merged?
>>>
>>> $ ./scripts/get_maintainer.pl -f migration/meson.build
>>> Juan Quintela <quintela@redhat.com> (maintainer:Migration)
>>> Peter Xu <peterx@redhat.com> (reviewer:Migration)
>>> Leonardo Bras <leobras@redhat.com> (reviewer:Migration)
>>> qemu-devel@nongnu.org (open list:All patches CC here)
>>>
>>> One maintainer, one single point of failure. When the
>>> maintainer is busy or offline (vacations?) then everybody
>>> is stuck.
>>>
>>> This is usually solved by adding co-maintainers.
>>>
>>> Juan, would you accept having co-maintainers helping you
>>> dealing with the merge process? I'm not volunteering, but
>>> this can be a good opportunity to make a formal request to
>>> the community.
>>>
>>> Regards,
>>>
>>> Phil.
>>>
>>
>> Thanks Philippe,
>>
>> I would like to highlight to the QEMU community here how important this is for SUSE,
>>
>> to see progress for all the series in the migration area that are currently still waiting and competing for attention.
>>
>> The specific features and improvements we are developing or helping to review are a priority for us,
>> and we are concerned about the rate of progress with the existing governance processes.
>>
>> Fabiano is investing a lot of his attention to this area, with features, bugfixes and reviews,
>> but of course it is up to the community as a whole to address the issue so that reviewed work is merged.
>>
>> My attempt here is to make sure that it is recognized as a problem, so it can be hopefully be addressed by the community in a timely fashion,
>> and we can all benefit from the already developed and reviewed work that is pending.
>>
>> Thanks!
>>
>> CLaudio
>>
>>
> 
> Hi Juan, any comments? Would additional help for co-maintenance of live migration help?
> How can we make things proceed?
> 
> From our (SUSE) side we are willing to help as much as we can, and all options are open,
> whatever it takes to get the multiple series currently waiting to flow again.
> 
> Thanks,
> 
> Claudio

Hi any news here?

We continue to see the migration area of QEMU as basically blocked;

there are a number of reviewed series out and little progress in merging them,
as well as a number of improvements not posted to the list at all due to the situation being completely bottlenecked.

Would a co-maintenance of migration/ from Fabiano be an option?

Should we organize a call to discuss this if the mailing list does not suffice to discuss this topic?

Thank you,

Claudio

Re: migration maintenance, governance [Was: Re: [PATCH V4 0/2] migration file URI]

[Peter Xu]

On Wed, Sep 27, 2023 at 03:14:29PM +0200, Claudio Fontana wrote:
> On 9/13/23 15:00, Claudio Fontana wrote:
> > On 8/30/23 15:09, Claudio Fontana wrote:
> >> On 8/22/23 15:25, Philippe Mathieu-Daudé wrote:
> >>> Hi Claudio,
> >>>
> >>> On 22/8/23 14:00, Claudio Fontana wrote:
> >>>> Hello,
> >>>>
> >>>> this series is all reviewed,
> >>>>
> >>>> and is needed as a precondition for further work to improve dramatically the performance of virsh save, virsh restore
> >>>> when migrating to disk, can it be merged?
> >>>
> >>> $ ./scripts/get_maintainer.pl -f migration/meson.build
> >>> Juan Quintela <quintela@redhat.com> (maintainer:Migration)
> >>> Peter Xu <peterx@redhat.com> (reviewer:Migration)
> >>> Leonardo Bras <leobras@redhat.com> (reviewer:Migration)
> >>> qemu-devel@nongnu.org (open list:All patches CC here)
> >>>
> >>> One maintainer, one single point of failure. When the
> >>> maintainer is busy or offline (vacations?) then everybody
> >>> is stuck.
> >>>
> >>> This is usually solved by adding co-maintainers.
> >>>
> >>> Juan, would you accept having co-maintainers helping you
> >>> dealing with the merge process? I'm not volunteering, but
> >>> this can be a good opportunity to make a formal request to
> >>> the community.
> >>>
> >>> Regards,
> >>>
> >>> Phil.
> >>>
> >>
> >> Thanks Philippe,
> >>
> >> I would like to highlight to the QEMU community here how important this is for SUSE,
> >>
> >> to see progress for all the series in the migration area that are currently still waiting and competing for attention.
> >>
> >> The specific features and improvements we are developing or helping to review are a priority for us,
> >> and we are concerned about the rate of progress with the existing governance processes.
> >>
> >> Fabiano is investing a lot of his attention to this area, with features, bugfixes and reviews,
> >> but of course it is up to the community as a whole to address the issue so that reviewed work is merged.
> >>
> >> My attempt here is to make sure that it is recognized as a problem, so it can be hopefully be addressed by the community in a timely fashion,
> >> and we can all benefit from the already developed and reviewed work that is pending.
> >>
> >> Thanks!
> >>
> >> CLaudio
> >>
> >>
> > 
> > Hi Juan, any comments? Would additional help for co-maintenance of live migration help?
> > How can we make things proceed?
> > 
> > From our (SUSE) side we are willing to help as much as we can, and all options are open,
> > whatever it takes to get the multiple series currently waiting to flow again.
> > 
> > Thanks,
> > 
> > Claudio
> 
> Hi any news here?
> 
> We continue to see the migration area of QEMU as basically blocked;
> 
> there are a number of reviewed series out and little progress in merging them,
> as well as a number of improvements not posted to the list at all due to the situation being completely bottlenecked.
> 
> Would a co-maintenance of migration/ from Fabiano be an option?
> 
> Should we organize a call to discuss this if the mailing list does not suffice to discuss this topic?

Yes we can.

Juan will be back next Monday.  Before he left, he told me that he's
preparing the pull.

If next week there's still no pull from migration side then I suppose
someone should start to pick up patches and send PR. I would volunteer
myself already; I should have already perpared a pull a long time ago if I
got ack from Juan.  If Fabiano would like to that'll also be nice.  I don't
know how that works without delegation from the current (solo) maintainer,
though, for either of us to become a maintainer.

Before that, Fabiano can already propose a patch as at least a Reviewer
without the need of any delegation from anyone, as he already did lots of
work in that regard, assume he'll keep doing so to support the migration
subsystem.

I hope Juan will start to figure things out next week.

Thanks,

-- 
Peter Xu

Re: migration maintenance, governance [Was: Re: [PATCH V4 0/2] migration file URI]

[Daniel P. Berrangé]

On Wed, Sep 27, 2023 at 03:14:29PM +0200, Claudio Fontana wrote:
> On 9/13/23 15:00, Claudio Fontana wrote:
> > On 8/30/23 15:09, Claudio Fontana wrote:
> >> On 8/22/23 15:25, Philippe Mathieu-Daudé wrote:
> >>> Hi Claudio,
> >>>
> >>> On 22/8/23 14:00, Claudio Fontana wrote:
> >>>> Hello,
> >>>>
> >>>> this series is all reviewed,
> >>>>
> >>>> and is needed as a precondition for further work to improve dramatically the performance of virsh save, virsh restore
> >>>> when migrating to disk, can it be merged?
> >>>
> >>> $ ./scripts/get_maintainer.pl -f migration/meson.build
> >>> Juan Quintela <quintela@redhat.com> (maintainer:Migration)
> >>> Peter Xu <peterx@redhat.com> (reviewer:Migration)
> >>> Leonardo Bras <leobras@redhat.com> (reviewer:Migration)
> >>> qemu-devel@nongnu.org (open list:All patches CC here)
> >>>
> >>> One maintainer, one single point of failure. When the
> >>> maintainer is busy or offline (vacations?) then everybody
> >>> is stuck.
> >>>
> >>> This is usually solved by adding co-maintainers.
> >>>
> >>> Juan, would you accept having co-maintainers helping you
> >>> dealing with the merge process? I'm not volunteering, but
> >>> this can be a good opportunity to make a formal request to
> >>> the community.
> >>
> >> I would like to highlight to the QEMU community here how important this is for SUSE,
> >>
> >> to see progress for all the series in the migration area that are currently still waiting and competing for attention.
> >>
> >> The specific features and improvements we are developing or helping to review are a priority for us,
> >> and we are concerned about the rate of progress with the existing governance processes.
> >>
> >> Fabiano is investing a lot of his attention to this area, with features, bugfixes and reviews,
> >> but of course it is up to the community as a whole to address the issue so that reviewed work is merged.
> >>
> >> My attempt here is to make sure that it is recognized as a problem, so it can be hopefully be addressed by the community in a timely fashion,
> >> and we can all benefit from the already developed and reviewed work that is pending.
> >>
> > 
> > Hi Juan, any comments? Would additional help for co-maintenance of live migration help?
> > How can we make things proceed?
> > 
> > From our (SUSE) side we are willing to help as much as we can, and all options are open,
> > whatever it takes to get the multiple series currently waiting to flow again.
> > 
> > Thanks,
> > 
> > Claudio
> 
> Hi any news here?
> 
> We continue to see the migration area of QEMU as basically blocked;
> 
> there are a number of reviewed series out and little progress in merging them,
> as well as a number of improvements not posted to the list at all due to the situation being completely bottlenecked.
> 
> Would a co-maintenance of migration/ from Fabiano be an option?
> 
> Should we organize a call to discuss this if the mailing list does not suffice to discuss this topic?

FWIW, in response to your previous mail, I had suggested to Peter Xu in
private that he would make a good candidate of co-maintainer for migration,
given he is already a nominated reviewer and has been actively reviewing
migration patches for a long while.

Migration did have 2 maintainers until earlier in this year when David
Gilbert moved on to other things, and I think we can all see that this
has had an unfortunate impact on productivity in the migration subsystem
that needs to be solved.

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 :|

Re: migration maintenance, governance [Was: Re: [PATCH V4 0/2] migration file URI]

[Daniel P. Berrangé]

On Wed, Sep 27, 2023 at 09:56:44AM -0400, Peter Xu wrote:
> Juan will be back next Monday.  Before he left, he told me that he's
> preparing the pull.
> 
> If next week there's still no pull from migration side then I suppose
> someone should start to pick up patches and send PR. I would volunteer
> myself already; I should have already perpared a pull a long time ago if I
> got ack from Juan.  If Fabiano would like to that'll also be nice.  I don't
> know how that works without delegation from the current (solo) maintainer,
> though, for either of us to become a maintainer.

Even if not actively sending a PR, a possible starting point that could
be done today, would be for someone to put up a gitlab.com branch that
contains all the outstanding patch series that are considered ready
to merge and validate a CI pipeline. That would both serve as a base for
further work, and might be useful to Juan in assembling the next pull
request.

> Before that, Fabiano can already propose a patch as at least a Reviewer
> without the need of any delegation from anyone, as he already did lots of
> work in that regard, assume he'll keep doing so to support the migration
> subsystem.

Yes, anyone is free to self-propose themselves as reviewers.

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 :|

Re: migration maintenance, governance [Was: Re: [PATCH V4 0/2] migration file URI]

[Peter Xu]

On Wed, Sep 27, 2023 at 03:36:16PM +0100, Daniel P. Berrangé wrote:
> Even if not actively sending a PR, a possible starting point that could
> be done today, would be for someone to put up a gitlab.com branch that
> contains all the outstanding patch series that are considered ready
> to merge and validate a CI pipeline. That would both serve as a base for
> further work, and might be useful to Juan in assembling the next pull
> request.

Right.

Though that'll be slightly conter-productive if Juan already has a branch
that contains everything for a pull covering patches until two weeks ago.

I think before he left he has that on hand, maybe also tested a bit over
the branch.  I actually talked to him on taking that branch over for this
time if he's busy, but unfortunately that conversation discontinued..

Having another branch has the risk that we'll have two base branches, if
Juan's branch will be sent out as the final pull next week and get merged.

But I do agree things need to be done if the pull doesn't come next week.

Thanks,

-- 
Peter Xu