[PATCH RFC 18/21] migration: Allow postcopy_register_shared_ufd() to fail

Peter Xu posted 21 patches 3 years ago
Maintainers: David Hildenbrand <david@redhat.com>, Igor Mammedov <imammedo@redhat.com>, "Michael S. Tsirkin" <mst@redhat.com>, Richard Henderson <richard.henderson@linaro.org>, Paolo Bonzini <pbonzini@redhat.com>, Peter Xu <peterx@redhat.com>, "Philippe Mathieu-Daudé" <philmd@linaro.org>, Cornelia Huck <cohuck@redhat.com>, Juan Quintela <quintela@redhat.com>, "Dr. David Alan Gilbert" <dgilbert@redhat.com>, Eric Blake <eblake@redhat.com>, Markus Armbruster <armbru@redhat.com>
[PATCH RFC 18/21] migration: Allow postcopy_register_shared_ufd() to fail
Posted by Peter Xu 3 years ago
Let's fail double-map for vhost-user and any potential users that can have
a remote userfaultfd for now.

Signed-off-by: Peter Xu <peterx@redhat.com>
---
 hw/virtio/vhost-user.c   | 9 ++++++++-
 migration/postcopy-ram.c | 9 +++++++--
 migration/postcopy-ram.h | 4 ++--
 3 files changed, 17 insertions(+), 5 deletions(-)

diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
index d9ce0501b2..00351bd67a 100644
--- a/hw/virtio/vhost-user.c
+++ b/hw/virtio/vhost-user.c
@@ -1952,7 +1952,14 @@ static int vhost_user_postcopy_advise(struct vhost_dev *dev, Error **errp)
     u->postcopy_fd.handler = vhost_user_postcopy_fault_handler;
     u->postcopy_fd.waker = vhost_user_postcopy_waker;
     u->postcopy_fd.idstr = "vhost-user"; /* Need to find unique name */
-    postcopy_register_shared_ufd(&u->postcopy_fd);
+
+    ret = postcopy_register_shared_ufd(&u->postcopy_fd);
+    if (ret) {
+        error_setg(errp, "%s: Register of shared userfaultfd failed: %s",
+                   __func__, strerror(ret));
+        return ret;
+    }
+
     return 0;
 #else
     error_setg(errp, "Postcopy not supported on non-Linux systems");
diff --git a/migration/postcopy-ram.c b/migration/postcopy-ram.c
index dbc7e54e4a..0cfe5174a5 100644
--- a/migration/postcopy-ram.c
+++ b/migration/postcopy-ram.c
@@ -1582,14 +1582,19 @@ PostcopyState postcopy_state_set(PostcopyState new_state)
 }
 
 /* Register a handler for external shared memory postcopy
- * called on the destination.
+ * called on the destination.  Returns 0 if success, <0 for err.
  */
-void postcopy_register_shared_ufd(struct PostCopyFD *pcfd)
+int postcopy_register_shared_ufd(struct PostCopyFD *pcfd)
 {
     MigrationIncomingState *mis = migration_incoming_get_current();
 
+    if (migrate_hugetlb_doublemap()) {
+        return -EINVAL;
+    }
+
     mis->postcopy_remote_fds = g_array_append_val(mis->postcopy_remote_fds,
                                                   *pcfd);
+    return 0;
 }
 
 /* Unregister a handler for external shared memory postcopy
diff --git a/migration/postcopy-ram.h b/migration/postcopy-ram.h
index 32734d2340..94adad6fb8 100644
--- a/migration/postcopy-ram.h
+++ b/migration/postcopy-ram.h
@@ -161,9 +161,9 @@ struct PostCopyFD {
 };
 
 /* Register a userfaultfd owned by an external process for
- * shared memory.
+ * shared memory.  Returns 0 if succeeded, <0 if error.
  */
-void postcopy_register_shared_ufd(struct PostCopyFD *pcfd);
+int postcopy_register_shared_ufd(struct PostCopyFD *pcfd);
 void postcopy_unregister_shared_ufd(struct PostCopyFD *pcfd);
 /* Call each of the shared 'waker's registered telling them of
  * availability of a block.
-- 
2.37.3
Re: [PATCH RFC 18/21] migration: Allow postcopy_register_shared_ufd() to fail
Posted by Juan Quintela 3 years ago
Peter Xu <peterx@redhat.com> wrote:
> Let's fail double-map for vhost-user and any potential users that can have
> a remote userfaultfd for now.
>
> Signed-off-by: Peter Xu <peterx@redhat.com>

Reviewed-by: Juan Quintela <quintela@redhat.com>

But

> -void postcopy_register_shared_ufd(struct PostCopyFD *pcfd)
> +int postcopy_register_shared_ufd(struct PostCopyFD *pcfd)
>  {
>      MigrationIncomingState *mis = migration_incoming_get_current();
>  
> +    if (migrate_hugetlb_doublemap()) {
> +        return -EINVAL;

I am not sure that -EINVAL is the best answer here.
There is not a problem with the value.  The problem is that both
features together don't work.

As an alternative:

ENOSYS 38 Function not implemented

But I am not sure that this is much better :-(

Later, Juan.