Re: [PATCH v3 17/20] vhost_user: Make source quit after reporting migration state

[David Gibson <david@gibson.dropbear.id.au>]

[-- Attachment #1: Type: text/plain, Size: 2930 bytes --]

On Fri, Jan 31, 2025 at 08:39:50PM +0100, Stefano Brivio wrote:
> On migration, the source process asks passt-helper to set TCP sockets
> in repair mode, dumps the information we need to migrate connections,
> and closes them.
> 
> At this point, we can't pass them back to passt-helper using
> SCM_RIGHTS, because they are closed, from that perspective, and
> sendmsg() will give us EBADF. But if we don't clear repair mode, the
> port they are bound to will not be available for binding in the
> target.
> 
> Terminate once we're done with the migration and we reported the
> state. This is equivalent to clearing repair mode on the sockets we
> just closed.

As noted on the passt-repair patch, I think this is based on a
misinterpreation of the situation.  I think the problem is that the
sockets aren't closed in passt-repair, so the additional handle copy
is keeping the underlying socket open.  This appears to work, because
it is causing passt-repair to also terminate.

That said, we probably want to terminate on the source side after a
succesful migrate anyway.  At the very least we need to close() all
our sockets, and delete the corresponding flows, because we don't own
them any more.  Quitting is probably the simplest way to do that.

Which also makes me realise, on a *failed* outbound migration, we _do_
need to turn repair mode off on everything again.  Is that implemented
yet?

> 
> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
> ---
>  vhost_user.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/vhost_user.c b/vhost_user.c
> index 1092387..19ede8a 100644
> --- a/vhost_user.c
> +++ b/vhost_user.c
> @@ -997,6 +997,8 @@ static bool vu_send_rarp_exec(struct vu_dev *vdev,
>  	return false;
>  }
>  
> +static bool quit_on_device_state = false;
> +
>  /**
>   * vu_set_device_state_fd_exec() - Set the device state migration channel
>   * @vdev:	vhost-user device
> @@ -1024,6 +1026,9 @@ static bool vu_set_device_state_fd_exec(struct vu_dev *vdev,
>  	migrate_request(vdev->context, msg->fds[0],
>  			direction == VHOST_USER_TRANSFER_STATE_DIRECTION_LOAD);
>  
> +	if (direction == VHOST_USER_TRANSFER_STATE_DIRECTION_SAVE)
> +		quit_on_device_state = true;
> +
>  	/* We don't provide a new fd for the data transfer */
>  	vmsg_set_reply_u64(msg, VHOST_USER_VRING_NOFD_MASK);
>  
> @@ -1201,4 +1206,10 @@ void vu_control_handler(struct vu_dev *vdev, int fd, uint32_t events)
>  
>  	if (reply_requested)
>  		vu_send_reply(fd, &msg);
> +
> +	if (quit_on_device_state &&
> +	    msg.hdr.request == VHOST_USER_CHECK_DEVICE_STATE) {
> +		info("Migration complete, exiting");
> +		exit(EXIT_SUCCESS);
> +	}
>  }

-- 
David Gibson (he or they)	| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you, not the other way
				| around.
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]