Re: [PATCH v1] migration: Wait for cpr.sock file to appear before connecting

["Daniel P. Berrangé" <berrange@redhat.com>]

On Thu, Jun 05, 2025 at 06:08:08PM -0500, Jaehoon Kim wrote:
> When the source VM attempts to connect to the destination VM's Unix
> domain socket(cpr.sock) during CPR transfer, the socket file might not
> yet be exist if the destination side hasn't completed the bind
> operation. This can lead to connection failures when running tests with
> the qtest framework.

This sounds like a flawed test impl to me - whatever is initiating
the cpr operation on the source has done so prematurely - it should
ensure the dest is ready before starting the operation.

> To address this, add cpr_validate_socket_path(), which wait for the
> socket file to appear. This avoids intermittent qtest failures caused by
> early connection attempts.

IMHO it is dubious to special case cpr in this way.

> 
> Signed-off-by: Jaehoon Kim <jhkim@linux.ibm.com>
> Reviewed-by: Jason J. Herne <jjherne@linux.ibm.com>
> ---
>  include/migration/cpr.h  |  1 +
>  migration/cpr-transfer.c | 35 +++++++++++++++++++++++++++++++++++
>  2 files changed, 36 insertions(+)
> 
> diff --git a/include/migration/cpr.h b/include/migration/cpr.h
> index 7561fc75ad..cc9384b4f9 100644
> --- a/include/migration/cpr.h
> +++ b/include/migration/cpr.h
> @@ -23,6 +23,7 @@ MigMode cpr_get_incoming_mode(void);
>  void cpr_set_incoming_mode(MigMode mode);
>  bool cpr_is_incoming(void);
>  
> +bool cpr_validate_socket_path(const char *path, Error **errp);
>  int cpr_state_save(MigrationChannel *channel, Error **errp);
>  int cpr_state_load(MigrationChannel *channel, Error **errp);
>  void cpr_state_close(void);
> diff --git a/migration/cpr-transfer.c b/migration/cpr-transfer.c
> index e1f140359c..3088ed323f 100644
> --- a/migration/cpr-transfer.c
> +++ b/migration/cpr-transfer.c
> @@ -17,6 +17,33 @@
>  #include "migration/vmstate.h"
>  #include "trace.h"
>  
> +#define CPR_MAX_RETRIES     50     /* Retry for up to 5 seconds */
> +#define CPR_RETRY_DELAY_US  100000 /* 100 ms per retry */
> +
> +bool cpr_validate_socket_path(const char *path, Error **errp)
> +{
> +    struct stat st;
> +    int retries = CPR_MAX_RETRIES;
> +
> +    do {
> +        if (!stat(path, &st) && S_ISSOCK(st.st_mode)) {
> +            return true;
> +        }
> +
> +        if (errno == ENOENT) {
> +            usleep(CPR_RETRY_DELAY_US);
> +        } else {
> +            error_setg_errno(errp, errno,
> +                "Unable to check status of socket path '%s'", path);
> +            return false;
> +        }
> +    } while (--retries > 0);
> +
> +    error_setg(errp, "Socket path '%s' not found after %d retries",
> +                                            path, CPR_MAX_RETRIES);
> +    return false;
> +}
> +
>  QEMUFile *cpr_transfer_output(MigrationChannel *channel, Error **errp)
>  {
>      MigrationAddress *addr = channel->addr;
> @@ -28,6 +55,14 @@ QEMUFile *cpr_transfer_output(MigrationChannel *channel, Error **errp)
>          QIOChannel *ioc = QIO_CHANNEL(sioc);
>          SocketAddress *saddr = &addr->u.socket;
>  
> +        /*
> +         * Verify that the cpr.sock Unix domain socket file exists and is ready
> +         * before proceeding with the connection.
> +         */
> +        if (!cpr_validate_socket_path(addr->u.socket.u.q_unix.path, errp)) {
> +            return NULL;
> +        }
> +
>          if (qio_channel_socket_connect_sync(sioc, saddr, errp) < 0) {
>              return NULL;
>          }
> -- 
> 2.49.0
> 
> 

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|