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

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

On Fri, Jun 06, 2025 at 10:37:28AM -0500, JAEHOON KIM wrote:
> 
> On 6/6/2025 10:12 AM, Steven Sistare wrote:
> > On 6/6/2025 11:06 AM, JAEHOON KIM wrote:
> > > On 6/6/2025 9:14 AM, Steven Sistare wrote:
> > > > On 6/6/2025 9:53 AM, Daniel P. Berrangé wrote:
> > > > > 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.
> > > > 
> > > > I agree with Daniel, and unfortunately it is not just a test issue;
> > > > every management framework that supports cpr-transfer must add logic to
> > > > wait for the cpr socket to appear in the target before proceeding.
> > > > 
> > > > This is analogous to waiting for the monitor socket to appear before
> > > > connecting to it.
> > > > 
> > > > - Steve
> > > 
> > > Thank you very much for your valuable review and feedback.
> > > 
> > > Just to clarify, the added cpr_validate_socket_path() function is
> > > not limited to the test framework.
> > > It is part of the actual CPR implementation and is intended to
> > > ensure correct behavior in all cases, including outside of tests.
> > > 
> > > I mentioned the qtest failure simply as an example where this issue
> > > became apparent.
> > 
> > Yes, I understand that you understand :)
> > Are you willing to move your fix to the qtest?
> > 
> > - Steve
> 
> Thank you for your question and feedback.
> 
> I agree that the issue could be addressed within the qtest framework to
> improve stability.
> 
> However, this socket readiness check is a fundamental part of CPR transfer
> process,
> and I believe that incorporating cpr_validate_socket_path() directly into
> the CPR implementation helps ensure more reliable behavior
> across all environments - not only during testing.
> 
> Just from my perspective, adding this logic to the CPR code does not
> introduce significant overhead or side effects.
> I would appreciate if you could share more details about your concerns, so I
> can better address them.

Requiring a busy wait like this is a sign of a design problem.

There needs to be a way to setup the incoming socket listener
without resorting to busy waiting - that's showing a lack of
synchronization.


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 :|