* Alexey Perevalov (a.pereva...@samsung.com) wrote:
> Userfaultfd mechanism is able to provide process thread id,
> in case when client request it with UFDD_API ioctl.
>
> Signed-off-by: Alexey Perevalov <a.pereva...@samsung.com>
There seem to be two parts to this:
a) Adding the mis parameter to ufd_version_check
b) Asking for the feature
Please split it into two patches.
Also....
> ---
> include/migration/postcopy-ram.h | 2 +-
> migration/migration.c | 2 +-
> migration/postcopy-ram.c | 12 ++++++------
> migration/savevm.c | 2 +-
> 4 files changed, 9 insertions(+), 9 deletions(-)
>
> diff --git a/include/migration/postcopy-ram.h
> b/include/migration/postcopy-ram.h
> index 8e036b9..809f6db 100644
> --- a/include/migration/postcopy-ram.h
> +++ b/include/migration/postcopy-ram.h
> @@ -14,7 +14,7 @@
> #define QEMU_POSTCOPY_RAM_H
>
> /* Return true if the host supports everything we need to do postcopy-ram */
> -bool postcopy_ram_supported_by_host(void);
> +bool postcopy_ram_supported_by_host(MigrationIncomingState *mis);
>
> /*
> * Make all of RAM sensitive to accesses to areas that haven't yet been
> written
> diff --git a/migration/migration.c b/migration/migration.c
> index ad4036f..79f6425 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -802,7 +802,7 @@ void
> qmp_migrate_set_capabilities(MigrationCapabilityStatusList *params,
> * special support.
> */
> if (!old_postcopy_cap && runstate_check(RUN_STATE_INMIGRATE) &&
> - !postcopy_ram_supported_by_host()) {
> + !postcopy_ram_supported_by_host(NULL)) {
> /* postcopy_ram_supported_by_host will have emitted a more
> * detailed message
> */
> diff --git a/migration/postcopy-ram.c b/migration/postcopy-ram.c
> index dc80dbb..70f0480 100644
> --- a/migration/postcopy-ram.c
> +++ b/migration/postcopy-ram.c
> @@ -60,13 +60,13 @@ struct PostcopyDiscardState {
> #include <sys/eventfd.h>
> #include <linux/userfaultfd.h>
>
> -static bool ufd_version_check(int ufd)
> +static bool ufd_version_check(int ufd, MigrationIncomingState *mis)
> {
> struct uffdio_api api_struct;
> uint64_t ioctl_mask;
>
> api_struct.api = UFFD_API;
> - api_struct.features = 0;
> + api_struct.features = UFFD_FEATURE_THREAD_ID;
> if (ioctl(ufd, UFFDIO_API, &api_struct)) {
> error_report("postcopy_ram_supported_by_host: UFFDIO_API failed: %s",
> strerror(errno));
You're not actually using the 'mis' here - what I'd expected was
something that was going to check if the UFFDIO_API return said that it really
had the feature, and if so store a flag in the MIS somewhere.
Also, I'm not sure it's right to set 'api_struct.features' on the input - what
happens if this is run on an old kernel - we don't want postcopy to fail on
an old kernel without your feature.
I'm not 100% sure of the interface, but I think the way it works is you set
features = 0 before the call, and then check the api_struct.features in the
return - in the same way that I check for UFFD_FEATURE_MISSING_HUGETLBFS.
Dave
> @@ -113,7 +113,7 @@ static int test_range_shared(const char *block_name, void
> *host_addr,
> * normally fine since if the postcopy succeeds it gets turned back on at the
> * end.
> */
> -bool postcopy_ram_supported_by_host(void)
> +bool postcopy_ram_supported_by_host(MigrationIncomingState *mis)
> {
> long pagesize = getpagesize();
> int ufd = -1;
> @@ -136,7 +136,7 @@ bool postcopy_ram_supported_by_host(void)
> }
>
> /* Version and features check */
> - if (!ufd_version_check(ufd)) {
> + if (!ufd_version_check(ufd, mis)) {
> goto out;
> }
>
> @@ -515,7 +515,7 @@ int postcopy_ram_enable_notify(MigrationIncomingState
> *mis)
> * Although the host check already tested the API, we need to
> * do the check again as an ABI handshake on the new fd.
> */
> - if (!ufd_version_check(mis->userfault_fd)) {
> + if (!ufd_version_check(mis->userfault_fd, mis)) {
> return -1;
> }
>
> @@ -653,7 +653,7 @@ void *postcopy_get_tmp_page(MigrationIncomingState *mis)
>
> #else
> /* No target OS support, stubs just fail */
> -bool postcopy_ram_supported_by_host(void)
> +bool postcopy_ram_supported_by_host(MigrationIncomingState *mis)
> {
> error_report("%s: No OS support", __func__);
> return false;
> diff --git a/migration/savevm.c b/migration/savevm.c
> index 3b19a4a..f01e418 100644
> --- a/migration/savevm.c
> +++ b/migration/savevm.c
> @@ -1360,7 +1360,7 @@ static int
> loadvm_postcopy_handle_advise(MigrationIncomingState *mis)
> return -1;
> }
>
> - if (!postcopy_ram_supported_by_host()) {
> + if (!postcopy_ram_supported_by_host(mis)) {
> postcopy_state_set(POSTCOPY_INCOMING_NONE);
> return -1;
> }
> --
> 1.8.3.1
>
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
