[PATCH v2 0/7] Migration deadcode removal

[PATCH v2 0/7] Migration deadcode removal

[dave]

From: "Dr. David Alan Gilbert" <dave@treblig.org>

  This is a set of deadcode removal around migration
found by looking for unused symbols.

v2
   Don't remove the zero-blocks capability yet
   add Fabiano's deprecation text patch.
   Use the uffd helpers in postcopy rather than
     removing most of them.
   Remove one.

Dave

Dr. David Alan Gilbert (6):
  migration: Remove migrate_cap_set
  migration: Remove unused migrate_zero_blocks
  migration: Remove unused socket_send_channel_create_sync
  util/userfaultfd: Return -errno on error
  migration/postcopy: Use uffd helpers
  util/userfaultfd: Remove unused uffd_poll_events

Fabiano Rosas (1):
  migration: Deprecate zero-blocks capability

 docs/about/deprecated.rst  |  6 +++++
 include/qemu/userfaultfd.h |  1 -
 migration/options.c        | 31 ++++--------------------
 migration/options.h        |  2 --
 migration/postcopy-ram.c   | 47 ++++++++++--------------------------
 migration/socket.c         | 18 --------------
 migration/socket.h         |  1 -
 qapi/migration.json        |  5 +++-
 util/userfaultfd.c         | 49 ++++++++++----------------------------
 9 files changed, 39 insertions(+), 121 deletions(-)

-- 
2.46.1

[PATCH v2 1/7] migration: Remove migrate_cap_set

[dave]

From: "Dr. David Alan Gilbert" <dave@treblig.org>

migrate_cap_set has been unused since
  18d154f575 ("migration: Remove 'blk/-b' option from migrate commands")

Remove it.

Signed-off-by: Dr. David Alan Gilbert <dave@treblig.org>
Reviewed-by: Fabiano Rosas <farosas@suse.de>
---
 migration/options.c | 20 --------------------
 migration/options.h |  1 -
 2 files changed, 21 deletions(-)

diff --git a/migration/options.c b/migration/options.c
index 147cd2b8fd..9460c5dee9 100644
--- a/migration/options.c
+++ b/migration/options.c
@@ -605,26 +605,6 @@ bool migrate_caps_check(bool *old_caps, bool *new_caps, Error **errp)
     return true;
 }
 
-bool migrate_cap_set(int cap, bool value, Error **errp)
-{
-    MigrationState *s = migrate_get_current();
-    bool new_caps[MIGRATION_CAPABILITY__MAX];
-
-    if (migration_is_running()) {
-        error_setg(errp, "There's a migration process in progress");
-        return false;
-    }
-
-    memcpy(new_caps, s->capabilities, sizeof(new_caps));
-    new_caps[cap] = value;
-
-    if (!migrate_caps_check(s->capabilities, new_caps, errp)) {
-        return false;
-    }
-    s->capabilities[cap] = value;
-    return true;
-}
-
 MigrationCapabilityStatusList *qmp_query_migrate_capabilities(Error **errp)
 {
     MigrationCapabilityStatusList *head = NULL, **tail = &head;
diff --git a/migration/options.h b/migration/options.h
index a0bd6edc06..36e7b3723f 100644
--- a/migration/options.h
+++ b/migration/options.h
@@ -58,7 +58,6 @@ bool migrate_tls(void);
 /* capabilities helpers */
 
 bool migrate_caps_check(bool *old_caps, bool *new_caps, Error **errp);
-bool migrate_cap_set(int cap, bool value, Error **errp);
 
 /* parameters */
 
-- 
2.46.1

[PATCH v2 2/7] migration: Remove unused migrate_zero_blocks

[dave]

From: "Dr. David Alan Gilbert" <dave@treblig.org>

migrate_zero_blocks is unused since
  eef0bae3a7 ("migration: Remove block migration")

Remove it.

Signed-off-by: Dr. David Alan Gilbert <dave@treblig.org>
---
 migration/options.c | 7 -------
 migration/options.h | 1 -
 2 files changed, 8 deletions(-)

diff --git a/migration/options.c b/migration/options.c
index 9460c5dee9..6f549984cb 100644
--- a/migration/options.c
+++ b/migration/options.c
@@ -339,13 +339,6 @@ bool migrate_xbzrle(void)
     return s->capabilities[MIGRATION_CAPABILITY_XBZRLE];
 }
 
-bool migrate_zero_blocks(void)
-{
-    MigrationState *s = migrate_get_current();
-
-    return s->capabilities[MIGRATION_CAPABILITY_ZERO_BLOCKS];
-}
-
 bool migrate_zero_copy_send(void)
 {
     MigrationState *s = migrate_get_current();
diff --git a/migration/options.h b/migration/options.h
index 36e7b3723f..79084eed0d 100644
--- a/migration/options.h
+++ b/migration/options.h
@@ -40,7 +40,6 @@ bool migrate_release_ram(void);
 bool migrate_return_path(void);
 bool migrate_validate_uuid(void);
 bool migrate_xbzrle(void);
-bool migrate_zero_blocks(void);
 bool migrate_zero_copy_send(void);
 
 /*
-- 
2.46.1

Re: [PATCH v2 2/7] migration: Remove unused migrate_zero_blocks

[Peter Xu]

On Thu, Sep 19, 2024 at 02:46:21PM +0100, dave@treblig.org wrote:
> From: "Dr. David Alan Gilbert" <dave@treblig.org>
> 
> migrate_zero_blocks is unused since
>   eef0bae3a7 ("migration: Remove block migration")
> 
> Remove it.
> 
> Signed-off-by: Dr. David Alan Gilbert <dave@treblig.org>

Reviewed-by: Peter Xu <peterx@redhat.com>

-- 
Peter Xu

[PATCH v2 3/7] migration: Deprecate zero-blocks capability

[dave]

From: Fabiano Rosas <farosas@suse.de>

The zero-blocks capability was meant to be used along with the block
migration, which has been removed already in commit eef0bae3a7
("migration: Remove block migration").

Setting zero-blocks is currently a noop, but the outright removal of
the capability would cause and error in case some users are still
setting it. Put the capability through the deprecation process.

Signed-off-by: Fabiano Rosas <farosas@suse.de>
---
 docs/about/deprecated.rst | 6 ++++++
 migration/options.c       | 4 ++++
 qapi/migration.json       | 5 ++++-
 3 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
index ed31d4b0b2..47cabb6fcc 100644
--- a/docs/about/deprecated.rst
+++ b/docs/about/deprecated.rst
@@ -476,3 +476,9 @@ usage of providing a file descriptor to a plain file has been
 deprecated in favor of explicitly using the ``file:`` URI with the
 file descriptor being passed as an ``fdset``. Refer to the ``add-fd``
 command documentation for details on the ``fdset`` usage.
+
+``zero-blocks`` capability (since 9.2)
+''''''''''''''''''''''''''''''''''''''
+
+The ``zero-blocks`` capability was part of the block migration which
+doesn't exist anymore since it was removed in QEMU v9.1.
diff --git a/migration/options.c b/migration/options.c
index 6f549984cb..ad8d6989a8 100644
--- a/migration/options.c
+++ b/migration/options.c
@@ -450,6 +450,10 @@ bool migrate_caps_check(bool *old_caps, bool *new_caps, Error **errp)
     ERRP_GUARD();
     MigrationIncomingState *mis = migration_incoming_get_current();
 
+    if (new_caps[MIGRATION_CAPABILITY_ZERO_BLOCKS]) {
+        warn_report("zero-blocks capability is deprecated");
+    }
+
 #ifndef CONFIG_REPLICATION
     if (new_caps[MIGRATION_CAPABILITY_X_COLO]) {
         error_setg(errp, "QEMU compiled without replication module"
diff --git a/qapi/migration.json b/qapi/migration.json
index b66cccf107..3af6aa1740 100644
--- a/qapi/migration.json
+++ b/qapi/migration.json
@@ -479,11 +479,14 @@
 # Features:
 #
 # @unstable: Members @x-colo and @x-ignore-shared are experimental.
+# @deprecated: Member @zero-blocks is deprecated as being part of
+#     block migration which was already removed.
 #
 # Since: 1.2
 ##
 { 'enum': 'MigrationCapability',
-  'data': ['xbzrle', 'rdma-pin-all', 'auto-converge', 'zero-blocks',
+  'data': ['xbzrle', 'rdma-pin-all', 'auto-converge',
+           { 'name': 'zero-blocks', 'features': [ 'deprecated' ] },
            'events', 'postcopy-ram',
            { 'name': 'x-colo', 'features': [ 'unstable' ] },
            'release-ram',
-- 
2.46.1

Re: [PATCH v2 3/7] migration: Deprecate zero-blocks capability

[Peter Xu]

On Thu, Sep 19, 2024 at 02:46:22PM +0100, dave@treblig.org wrote:
> From: Fabiano Rosas <farosas@suse.de>
> 
> The zero-blocks capability was meant to be used along with the block
> migration, which has been removed already in commit eef0bae3a7
> ("migration: Remove block migration").
> 
> Setting zero-blocks is currently a noop, but the outright removal of
> the capability would cause and error in case some users are still
> setting it. Put the capability through the deprecation process.
> 
> Signed-off-by: Fabiano Rosas <farosas@suse.de>

Reviewed-by: Peter Xu <peterx@redhat.com>

-- 
Peter Xu

[PATCH v2 4/7] migration: Remove unused socket_send_channel_create_sync

[dave]

From: "Dr. David Alan Gilbert" <dave@treblig.org>

socket_send_channel_create_sync only use was removed by
  d0edb8a173 ("migration: Create the postcopy preempt channel asynchronously")

Remove it.

Signed-off-by: Dr. David Alan Gilbert <dave@treblig.org>
Reviewed-by: Fabiano Rosas <farosas@suse.de>
---
 migration/socket.c | 18 ------------------
 migration/socket.h |  1 -
 2 files changed, 19 deletions(-)

diff --git a/migration/socket.c b/migration/socket.c
index 9ab89b1e08..5ec65b8c03 100644
--- a/migration/socket.c
+++ b/migration/socket.c
@@ -42,24 +42,6 @@ void socket_send_channel_create(QIOTaskFunc f, void *data)
                                      f, data, NULL, NULL);
 }
 
-QIOChannel *socket_send_channel_create_sync(Error **errp)
-{
-    QIOChannelSocket *sioc = qio_channel_socket_new();
-
-    if (!outgoing_args.saddr) {
-        object_unref(OBJECT(sioc));
-        error_setg(errp, "Initial sock address not set!");
-        return NULL;
-    }
-
-    if (qio_channel_socket_connect_sync(sioc, outgoing_args.saddr, errp) < 0) {
-        object_unref(OBJECT(sioc));
-        return NULL;
-    }
-
-    return QIO_CHANNEL(sioc);
-}
-
 struct SocketConnectData {
     MigrationState *s;
     char *hostname;
diff --git a/migration/socket.h b/migration/socket.h
index 46c233ecd2..04ebbe95a1 100644
--- a/migration/socket.h
+++ b/migration/socket.h
@@ -22,7 +22,6 @@
 #include "qemu/sockets.h"
 
 void socket_send_channel_create(QIOTaskFunc f, void *data);
-QIOChannel *socket_send_channel_create_sync(Error **errp);
 
 void socket_start_incoming_migration(SocketAddress *saddr, Error **errp);
 
-- 
2.46.1

[PATCH v2 5/7] util/userfaultfd: Return -errno on error

[dave]

From: "Dr. David Alan Gilbert" <dave@treblig.org>

Convert (the currently unused) uffd_wakeup, uffd_copy_page and
uffd_zero_page to return -errno on error rather than -1.

That will make it easier to reuse in postcopy.

Signed-off-by: Dr. David Alan Gilbert <dave@treblig.org>
---
 util/userfaultfd.c | 21 ++++++++++++---------
 1 file changed, 12 insertions(+), 9 deletions(-)

diff --git a/util/userfaultfd.c b/util/userfaultfd.c
index 1b2fa949d4..518d5c3586 100644
--- a/util/userfaultfd.c
+++ b/util/userfaultfd.c
@@ -240,7 +240,7 @@ int uffd_change_protection(int uffd_fd, void *addr, uint64_t length,
  * Copy range of source pages to the destination to resolve
  * missing page fault somewhere in the destination range.
  *
- * Returns 0 on success, negative value in case of an error
+ * Returns 0 on success, -errno in case of an error
  *
  * @uffd_fd: UFFD file descriptor
  * @dst_addr: destination base address
@@ -259,10 +259,11 @@ int uffd_copy_page(int uffd_fd, void *dst_addr, void *src_addr,
     uffd_copy.mode = dont_wake ? UFFDIO_COPY_MODE_DONTWAKE : 0;
 
     if (ioctl(uffd_fd, UFFDIO_COPY, &uffd_copy)) {
+        int e = errno;
         error_report("uffd_copy_page() failed: dst_addr=%p src_addr=%p length=%" PRIu64
                 " mode=%" PRIx64 " errno=%i", dst_addr, src_addr,
-                length, (uint64_t) uffd_copy.mode, errno);
-        return -1;
+                length, (uint64_t) uffd_copy.mode, e);
+        return -e;
     }
 
     return 0;
@@ -273,7 +274,7 @@ int uffd_copy_page(int uffd_fd, void *dst_addr, void *src_addr,
  *
  * Fill range pages with zeroes to resolve missing page fault within the range.
  *
- * Returns 0 on success, negative value in case of an error
+ * Returns 0 on success, -errno in case of an error
  *
  * @uffd_fd: UFFD file descriptor
  * @addr: base address
@@ -289,10 +290,11 @@ int uffd_zero_page(int uffd_fd, void *addr, uint64_t length, bool dont_wake)
     uffd_zeropage.mode = dont_wake ? UFFDIO_ZEROPAGE_MODE_DONTWAKE : 0;
 
     if (ioctl(uffd_fd, UFFDIO_ZEROPAGE, &uffd_zeropage)) {
+        int e = errno;
         error_report("uffd_zero_page() failed: addr=%p length=%" PRIu64
                 " mode=%" PRIx64 " errno=%i", addr, length,
-                (uint64_t) uffd_zeropage.mode, errno);
-        return -1;
+                (uint64_t) uffd_zeropage.mode, e);
+        return -e;
     }
 
     return 0;
@@ -306,7 +308,7 @@ int uffd_zero_page(int uffd_fd, void *addr, uint64_t length, bool dont_wake)
  * via UFFD-IO IOCTLs with MODE_DONTWAKE flag set, then after that all waits
  * for the whole memory range are satisfied in a single call to uffd_wakeup().
  *
- * Returns 0 on success, negative value in case of an error
+ * Returns 0 on success, -errno in case of an error
  *
  * @uffd_fd: UFFD file descriptor
  * @addr: base address
@@ -320,9 +322,10 @@ int uffd_wakeup(int uffd_fd, void *addr, uint64_t length)
     uffd_range.len = length;
 
     if (ioctl(uffd_fd, UFFDIO_WAKE, &uffd_range)) {
+        int e = errno;
         error_report("uffd_wakeup() failed: addr=%p length=%" PRIu64 " errno=%i",
-                addr, length, errno);
-        return -1;
+                addr, length, e);
+        return -e;
     }
 
     return 0;
-- 
2.46.1

Re: [PATCH v2 5/7] util/userfaultfd: Return -errno on error

[Peter Xu]

On Thu, Sep 19, 2024 at 02:46:24PM +0100, dave@treblig.org wrote:
> From: "Dr. David Alan Gilbert" <dave@treblig.org>
> 
> Convert (the currently unused) uffd_wakeup, uffd_copy_page and
> uffd_zero_page to return -errno on error rather than -1.
> 
> That will make it easier to reuse in postcopy.
> 
> Signed-off-by: Dr. David Alan Gilbert <dave@treblig.org>

Reviewed-by: Peter Xu <peterx@redhat.com>

-- 
Peter Xu

[PATCH v2 6/7] migration/postcopy: Use uffd helpers

[dave]

From: "Dr. David Alan Gilbert" <dave@treblig.org>

Use the uffd_copy_page, uffd_zero_page and uffd_wakeup helpers
rather than calling ioctl ourselves.

They return -errno on error, and print an error_report themselves.
I think this actually makes postcopy_place_page actually more
consistent in it's callers.

Signed-off-by: Dr. David Alan Gilbert <dave@treblig.org>
---
 migration/postcopy-ram.c | 47 +++++++++++-----------------------------
 1 file changed, 13 insertions(+), 34 deletions(-)

diff --git a/migration/postcopy-ram.c b/migration/postcopy-ram.c
index 1c374b7ea1..e2b318d3da 100644
--- a/migration/postcopy-ram.c
+++ b/migration/postcopy-ram.c
@@ -746,18 +746,9 @@ int postcopy_wake_shared(struct PostCopyFD *pcfd,
                          RAMBlock *rb)
 {
     size_t pagesize = qemu_ram_pagesize(rb);
-    struct uffdio_range range;
-    int ret;
     trace_postcopy_wake_shared(client_addr, qemu_ram_get_idstr(rb));
-    range.start = ROUND_DOWN(client_addr, pagesize);
-    range.len = pagesize;
-    ret = ioctl(pcfd->fd, UFFDIO_WAKE, &range);
-    if (ret) {
-        error_report("%s: Failed to wake: %zx in %s (%s)",
-                     __func__, (size_t)client_addr, qemu_ram_get_idstr(rb),
-                     strerror(errno));
-    }
-    return ret;
+    return uffd_wakeup(pcfd->fd, (void *)ROUND_DOWN(client_addr, pagesize),
+                       pagesize);
 }
 
 static int postcopy_request_page(MigrationIncomingState *mis, RAMBlock *rb,
@@ -1275,18 +1266,10 @@ static int qemu_ufd_copy_ioctl(MigrationIncomingState *mis, void *host_addr,
     int ret;
 
     if (from_addr) {
-        struct uffdio_copy copy_struct;
-        copy_struct.dst = (uint64_t)(uintptr_t)host_addr;
-        copy_struct.src = (uint64_t)(uintptr_t)from_addr;
-        copy_struct.len = pagesize;
-        copy_struct.mode = 0;
-        ret = ioctl(userfault_fd, UFFDIO_COPY, &copy_struct);
+        ret = uffd_copy_page(userfault_fd, host_addr, from_addr, pagesize,
+                             false);
     } else {
-        struct uffdio_zeropage zero_struct;
-        zero_struct.range.start = (uint64_t)(uintptr_t)host_addr;
-        zero_struct.range.len = pagesize;
-        zero_struct.mode = 0;
-        ret = ioctl(userfault_fd, UFFDIO_ZEROPAGE, &zero_struct);
+        ret = uffd_zero_page(userfault_fd, host_addr, pagesize, false);
     }
     if (!ret) {
         qemu_mutex_lock(&mis->page_request_mutex);
@@ -1343,18 +1326,16 @@ int postcopy_place_page(MigrationIncomingState *mis, void *host, void *from,
                         RAMBlock *rb)
 {
     size_t pagesize = qemu_ram_pagesize(rb);
+    int e;
 
     /* copy also acks to the kernel waking the stalled thread up
      * TODO: We can inhibit that ack and only do it if it was requested
      * which would be slightly cheaper, but we'd have to be careful
      * of the order of updating our page state.
      */
-    if (qemu_ufd_copy_ioctl(mis, host, from, pagesize, rb)) {
-        int e = errno;
-        error_report("%s: %s copy host: %p from: %p (size: %zd)",
-                     __func__, strerror(e), host, from, pagesize);
-
-        return -e;
+    e = qemu_ufd_copy_ioctl(mis, host, from, pagesize, rb);
+    if (e) {
+        return e;
     }
 
     trace_postcopy_place_page(host);
@@ -1376,12 +1357,10 @@ int postcopy_place_page_zero(MigrationIncomingState *mis, void *host,
      * but it's not available for everything (e.g. hugetlbpages)
      */
     if (qemu_ram_is_uf_zeroable(rb)) {
-        if (qemu_ufd_copy_ioctl(mis, host, NULL, pagesize, rb)) {
-            int e = errno;
-            error_report("%s: %s zero host: %p",
-                         __func__, strerror(e), host);
-
-            return -e;
+        int e;
+        e = qemu_ufd_copy_ioctl(mis, host, NULL, pagesize, rb);
+        if (e) {
+            return e;
         }
         return postcopy_notify_shared_wake(rb,
                                            qemu_ram_block_host_offset(rb,
-- 
2.46.1

Re: [PATCH v2 6/7] migration/postcopy: Use uffd helpers

[Peter Xu]

On Thu, Sep 19, 2024 at 02:46:25PM +0100, dave@treblig.org wrote:
> From: "Dr. David Alan Gilbert" <dave@treblig.org>
> 
> Use the uffd_copy_page, uffd_zero_page and uffd_wakeup helpers
> rather than calling ioctl ourselves.
> 
> They return -errno on error, and print an error_report themselves.
> I think this actually makes postcopy_place_page actually more
> consistent in it's callers.
> 
> Signed-off-by: Dr. David Alan Gilbert <dave@treblig.org>

Reviewed-by: Peter Xu <peterx@redhat.com>

-- 
Peter Xu

Re: [PATCH v2 6/7] migration/postcopy: Use uffd helpers

[Peter Xu]

On Thu, Sep 19, 2024 at 02:46:25PM +0100, dave@treblig.org wrote:
> From: "Dr. David Alan Gilbert" <dave@treblig.org>
> 
> Use the uffd_copy_page, uffd_zero_page and uffd_wakeup helpers
> rather than calling ioctl ourselves.
> 
> They return -errno on error, and print an error_report themselves.
> I think this actually makes postcopy_place_page actually more
> consistent in it's callers.
> 
> Signed-off-by: Dr. David Alan Gilbert <dave@treblig.org>
> ---
>  migration/postcopy-ram.c | 47 +++++++++++-----------------------------
>  1 file changed, 13 insertions(+), 34 deletions(-)
> 
> diff --git a/migration/postcopy-ram.c b/migration/postcopy-ram.c
> index 1c374b7ea1..e2b318d3da 100644
> --- a/migration/postcopy-ram.c
> +++ b/migration/postcopy-ram.c
> @@ -746,18 +746,9 @@ int postcopy_wake_shared(struct PostCopyFD *pcfd,
>                           RAMBlock *rb)
>  {
>      size_t pagesize = qemu_ram_pagesize(rb);
> -    struct uffdio_range range;
> -    int ret;
>      trace_postcopy_wake_shared(client_addr, qemu_ram_get_idstr(rb));
> -    range.start = ROUND_DOWN(client_addr, pagesize);
> -    range.len = pagesize;
> -    ret = ioctl(pcfd->fd, UFFDIO_WAKE, &range);
> -    if (ret) {
> -        error_report("%s: Failed to wake: %zx in %s (%s)",
> -                     __func__, (size_t)client_addr, qemu_ram_get_idstr(rb),
> -                     strerror(errno));
> -    }
> -    return ret;
> +    return uffd_wakeup(pcfd->fd, (void *)ROUND_DOWN(client_addr, pagesize),
> +                       pagesize);
>  }

There's a build issue on i386:

../migration/postcopy-ram.c: In function ‘postcopy_wake_shared’:
../migration/postcopy-ram.c:750:34: error: cast to pointer from integer of different size [-Werror=int-to-pointer-cast]
  750 |     return uffd_wakeup(pcfd->fd, (void *)ROUND_DOWN(client_addr, pagesize),
      |                                  ^

The plan is to squash below fix:

=========8<===========
diff --git a/migration/postcopy-ram.c b/migration/postcopy-ram.c
index 03a63ef5cd..83f6160a36 100644
--- a/migration/postcopy-ram.c
+++ b/migration/postcopy-ram.c
-@@ -747,7 +747,8 @@ int postcopy_wake_shared(struct PostCopyFD *pcfd,
 {
     size_t pagesize = qemu_ram_pagesize(rb);
     trace_postcopy_wake_shared(client_addr, qemu_ram_get_idstr(rb));
-    return uffd_wakeup(pcfd->fd, (void *)ROUND_DOWN(client_addr, pagesize),
+    return uffd_wakeup(pcfd->fd,
+                       (void *)(uintptr_t)ROUND_DOWN(client_addr, pagesize),
                        pagesize);
 }
=========8<===========

Thanks,

-- 
Peter Xu

Re: [PATCH v2 6/7] migration/postcopy: Use uffd helpers

[Dr. David Alan Gilbert]

* Peter Xu (peterx@redhat.com) wrote:
> On Thu, Sep 19, 2024 at 02:46:25PM +0100, dave@treblig.org wrote:
> > From: "Dr. David Alan Gilbert" <dave@treblig.org>
> > 
> > Use the uffd_copy_page, uffd_zero_page and uffd_wakeup helpers
> > rather than calling ioctl ourselves.
> > 
> > They return -errno on error, and print an error_report themselves.
> > I think this actually makes postcopy_place_page actually more
> > consistent in it's callers.
> > 
> > Signed-off-by: Dr. David Alan Gilbert <dave@treblig.org>
> > ---
> >  migration/postcopy-ram.c | 47 +++++++++++-----------------------------
> >  1 file changed, 13 insertions(+), 34 deletions(-)
> > 
> > diff --git a/migration/postcopy-ram.c b/migration/postcopy-ram.c
> > index 1c374b7ea1..e2b318d3da 100644
> > --- a/migration/postcopy-ram.c
> > +++ b/migration/postcopy-ram.c
> > @@ -746,18 +746,9 @@ int postcopy_wake_shared(struct PostCopyFD *pcfd,
> >                           RAMBlock *rb)
> >  {
> >      size_t pagesize = qemu_ram_pagesize(rb);
> > -    struct uffdio_range range;
> > -    int ret;
> >      trace_postcopy_wake_shared(client_addr, qemu_ram_get_idstr(rb));
> > -    range.start = ROUND_DOWN(client_addr, pagesize);
> > -    range.len = pagesize;
> > -    ret = ioctl(pcfd->fd, UFFDIO_WAKE, &range);
> > -    if (ret) {
> > -        error_report("%s: Failed to wake: %zx in %s (%s)",
> > -                     __func__, (size_t)client_addr, qemu_ram_get_idstr(rb),
> > -                     strerror(errno));
> > -    }
> > -    return ret;
> > +    return uffd_wakeup(pcfd->fd, (void *)ROUND_DOWN(client_addr, pagesize),
> > +                       pagesize);
> >  }
> 
> There's a build issue on i386:
> 
> ../migration/postcopy-ram.c: In function ‘postcopy_wake_shared’:
> ../migration/postcopy-ram.c:750:34: error: cast to pointer from integer of different size [-Werror=int-to-pointer-cast]
>   750 |     return uffd_wakeup(pcfd->fd, (void *)ROUND_DOWN(client_addr, pagesize),
>       |                                  ^
> 
> The plan is to squash below fix:

Thanks!

Dave

> =========8<===========
> diff --git a/migration/postcopy-ram.c b/migration/postcopy-ram.c
> index 03a63ef5cd..83f6160a36 100644
> --- a/migration/postcopy-ram.c
> +++ b/migration/postcopy-ram.c
> -@@ -747,7 +747,8 @@ int postcopy_wake_shared(struct PostCopyFD *pcfd,
>  {
>      size_t pagesize = qemu_ram_pagesize(rb);
>      trace_postcopy_wake_shared(client_addr, qemu_ram_get_idstr(rb));
> -    return uffd_wakeup(pcfd->fd, (void *)ROUND_DOWN(client_addr, pagesize),
> +    return uffd_wakeup(pcfd->fd,
> +                       (void *)(uintptr_t)ROUND_DOWN(client_addr, pagesize),
>                         pagesize);
>  }
> =========8<===========
> 
> Thanks,
> 
> -- 
> Peter Xu
> 
-- 
 -----Open up your eyes, open up your mind, open up your code -------   
/ Dr. David Alan Gilbert    |       Running GNU/Linux       | Happy  \ 
\        dave @ treblig.org |                               | In Hex /
 \ _________________________|_____ http://www.treblig.org   |_______/

[PATCH v2 7/7] util/userfaultfd: Remove unused uffd_poll_events

[dave]

From: "Dr. David Alan Gilbert" <dave@treblig.org>

uffd_poll_events has been unused since it was added; it's also
just a wrapper around a plain old poll call, so doesn't add anything.

Signed-off-by: Dr. David Alan Gilbert <dave@treblig.org>
---
 include/qemu/userfaultfd.h |  1 -
 util/userfaultfd.c         | 28 ----------------------------
 2 files changed, 29 deletions(-)

diff --git a/include/qemu/userfaultfd.h b/include/qemu/userfaultfd.h
index 18a4314212..a1979308d7 100644
--- a/include/qemu/userfaultfd.h
+++ b/include/qemu/userfaultfd.h
@@ -39,7 +39,6 @@ int uffd_copy_page(int uffd_fd, void *dst_addr, void *src_addr,
 int uffd_zero_page(int uffd_fd, void *addr, uint64_t length, bool dont_wake);
 int uffd_wakeup(int uffd_fd, void *addr, uint64_t length);
 int uffd_read_events(int uffd_fd, struct uffd_msg *msgs, int count);
-bool uffd_poll_events(int uffd_fd, int tmo);
 
 #endif /* CONFIG_LINUX */
 
diff --git a/util/userfaultfd.c b/util/userfaultfd.c
index 518d5c3586..2396104f23 100644
--- a/util/userfaultfd.c
+++ b/util/userfaultfd.c
@@ -358,31 +358,3 @@ int uffd_read_events(int uffd_fd, struct uffd_msg *msgs, int count)
 
     return (int) (res / sizeof(struct uffd_msg));
 }
-
-/**
- * uffd_poll_events: poll UFFD file descriptor for read
- *
- * Returns true if events are available for read, false otherwise
- *
- * @uffd_fd: UFFD file descriptor
- * @tmo: timeout value
- */
-bool uffd_poll_events(int uffd_fd, int tmo)
-{
-    int res;
-    struct pollfd poll_fd = { .fd = uffd_fd, .events = POLLIN, .revents = 0 };
-
-    do {
-        res = poll(&poll_fd, 1, tmo);
-    } while (res < 0 && errno == EINTR);
-
-    if (res == 0) {
-        return false;
-    }
-    if (res < 0) {
-        error_report("uffd_poll_events() failed: errno=%i", errno);
-        return false;
-    }
-
-    return (poll_fd.revents & POLLIN) != 0;
-}
-- 
2.46.1

Re: [PATCH v2 7/7] util/userfaultfd: Remove unused uffd_poll_events

[Peter Xu]

On Thu, Sep 19, 2024 at 02:46:26PM +0100, dave@treblig.org wrote:
> From: "Dr. David Alan Gilbert" <dave@treblig.org>
> 
> uffd_poll_events has been unused since it was added; it's also
> just a wrapper around a plain old poll call, so doesn't add anything.
> 
> Signed-off-by: Dr. David Alan Gilbert <dave@treblig.org>

Reviewed-by: Peter Xu <peterx@redhat.com>

-- 
Peter Xu

Re: [PATCH v2 0/7] Migration deadcode removal

[Peter Xu]

On Thu, Sep 19, 2024 at 02:46:19PM +0100, dave@treblig.org wrote:
> From: "Dr. David Alan Gilbert" <dave@treblig.org>
> 
>   This is a set of deadcode removal around migration
> found by looking for unused symbols.
> 
> v2
>    Don't remove the zero-blocks capability yet
>    add Fabiano's deprecation text patch.
>    Use the uffd helpers in postcopy rather than
>      removing most of them.
>    Remove one.
> 
> Dave
> 
> Dr. David Alan Gilbert (6):
>   migration: Remove migrate_cap_set
>   migration: Remove unused migrate_zero_blocks
>   migration: Remove unused socket_send_channel_create_sync
>   util/userfaultfd: Return -errno on error
>   migration/postcopy: Use uffd helpers
>   util/userfaultfd: Remove unused uffd_poll_events
> 
> Fabiano Rosas (1):
>   migration: Deprecate zero-blocks capability

Tentatively queued.  Markus/others, still feel free to comment or offer
tags, the PR will be at least a few days after people back from forum.

Thanks!

-- 
Peter Xu