Re: [PATCH] postocpy: Check that postocpy fd's are not NULL

["Dr. David Alan Gilbert" <dgilbert@redhat.com>]

* Juan Quintela (quintela@redhat.com) wrote:
> If postcopy has finished, it frees the array.
> But vhost-user unregister it at cleanup time.

I must admit to being confused as the double migrate case vs migrate
once and shutdown. It might just be an ordering thing?

I notice that  'vhost_user_backend_cleanup' does:
    if (u->postcopy_fd.handler) {
        postcopy_unregister_shared_ufd(&u->postcopy_fd);
        close(u->postcopy_fd.fd);
        u->postcopy_fd.handler = NULL;
    }

where as the other caller, 'vhost_user_postcopy_end'
does:
    postcopy_unregister_shared_ufd(&u->postcopy_fd);
    close(u->postcopy_fd.fd);
    u->postcopy_fd.handler = NULL;


maybe it would be better to fix them to do the same if check?

(Also note 'post*o*cpy' typo in title, and probably worth a
  Fixes: c4f7538 ?)

Dave


> Signed-off-by: Juan Quintela <quintela@redhat.com>
> ---
>  migration/postcopy-ram.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/migration/postcopy-ram.c b/migration/postcopy-ram.c
> index e721f69d0f..d18b5d05b2 100644
> --- a/migration/postcopy-ram.c
> +++ b/migration/postcopy-ram.c
> @@ -1457,6 +1457,10 @@ void postcopy_unregister_shared_ufd(struct PostCopyFD *pcfd)
>      MigrationIncomingState *mis = migration_incoming_get_current();
>      GArray *pcrfds = mis->postcopy_remote_fds;
>  
> +    if (!pcrfds) {
> +        /* migration has already finished and freed the array */
> +        return;
> +    }
>      for (i = 0; i < pcrfds->len; i++) {
>          struct PostCopyFD *cur = &g_array_index(pcrfds, struct PostCopyFD, i);
>          if (cur->fd == pcfd->fd) {
> -- 
> 2.33.1
> 
-- 
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK