cpr-transfer with caveats

cpr-transfer with caveats

[Steven Sistare]

Hi Peter, are you OK if we proceed with cpr-transfer as is, without the
precreate phase?  Here are the problems that motivated it:

* migration test wants to enable migration events on the dest.
   fix: enable on dest qemu using -global.  only for the test.

* migration test needs to fetch the dynamically assigned migration
     listen port number
   Fix: require unix domain socket for cpr-transfer, or a fixed port
   number. Document it.

* migration test hangs connecting to the qtest socket.
   fix: in the qtest code, defer connection.

Document that one cannot set migration caps or params on the dest
for cpr-transfer.

Document that for -incoming defer, mgmt must send the migrate command
to the src first (so dest reads cpr state and progresses to start the
monitor), then send the hotplug monitor commands to the dest.

Daniel, are you OK with that last bit?

- Steve

Re: cpr-transfer with caveats

[Peter Xu]

On Fri, Oct 25, 2024 at 11:01:27AM -0400, Steven Sistare wrote:
> Hi Peter, are you OK if we proceed with cpr-transfer as is, without the
> precreate phase?  Here are the problems that motivated it:
> 
> * migration test wants to enable migration events on the dest.
>   fix: enable on dest qemu using -global.  only for the test.

If it's to be documented that cpr-transfer allows no migration parameter /
cap setup, then the test case should follow, IMHO, rather than enabling
anything from cmdline.

I hope that's trivial if we can have MigrateCommon.disable_events, for
example, then only cpr selects it.

> 
> * migration test needs to fetch the dynamically assigned migration
>     listen port number
>   Fix: require unix domain socket for cpr-transfer, or a fixed port
>   number. Document it.

Fixed port is probably not a good idea.. requires unix sockets looks fine.

But then again the whole point previously having -cpr-uri together with
-incoming (IIRC, from your previous email) is to have that flexibility to
use non-unix too.  Now I am not sure whether it is still needed to be
separate just that you'll still need to rely on SIGHUP just because
precreate is gone.

> 
> * migration test hangs connecting to the qtest socket.
>   fix: in the qtest code, defer connection.

Not sure how much change here, hopefully still manageable.

From use case POV shouldn't be a huge issue, if we don't use -qtest in
production anyway.

> 
> Document that one cannot set migration caps or params on the dest
> for cpr-transfer.

This also needs to be reviewed by Dan, on impact of Libvirt initiating the
dest QEMU instance with QMP all dead before a "migrate" on src.  I'm not
sure whether Libvirt will also do early setup like what our qtest does to
enable events and so on, assuming QMP available since the start.

> 
> Document that for -incoming defer, mgmt must send the migrate command
> to the src first (so dest reads cpr state and progresses to start the
> monitor), then send the hotplug monitor commands to the dest.
> 
> Daniel, are you OK with that last bit?
> 
> - Steve
> 

-- 
Peter Xu

Re: cpr-transfer with caveats

[Daniel P. Berrangé]

On Fri, Oct 25, 2024 at 11:01:27AM -0400, Steven Sistare wrote:
> Hi Peter, are you OK if we proceed with cpr-transfer as is, without the
> precreate phase?  Here are the problems that motivated it:
> 
> * migration test wants to enable migration events on the dest.
>   fix: enable on dest qemu using -global.  only for the test.
> 
> * migration test needs to fetch the dynamically assigned migration
>     listen port number
>   Fix: require unix domain socket for cpr-transfer, or a fixed port
>   number. Document it.
> 
> * migration test hangs connecting to the qtest socket.
>   fix: in the qtest code, defer connection.
> 
> Document that one cannot set migration caps or params on the dest
> for cpr-transfer.
> 
> Document that for -incoming defer, mgmt must send the migrate command
> to the src first (so dest reads cpr state and progresses to start the
> monitor), then send the hotplug monitor commands to the dest.
> 
> Daniel, are you OK with that last bit?

I guess it depends on what happens inside QEMU between reading the
cpr state and libvirt being able to access the monitor. Libvirt does
various things with the monitor during QEMU startup, before guest
vCPUs start. Mostly this is around host resource placement/mgmt
that needs to be done before the guest CPUs start.

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

Re: cpr-transfer with caveats

[Steven Sistare]

On 10/25/2024 11:47 AM, Peter Xu wrote:
> On Fri, Oct 25, 2024 at 11:01:27AM -0400, Steven Sistare wrote:
>> Hi Peter, are you OK if we proceed with cpr-transfer as is, without the
>> precreate phase?  Here are the problems that motivated it:
>>
>> * migration test wants to enable migration events on the dest.
>>    fix: enable on dest qemu using -global.  only for the test.
> 
> If it's to be documented that cpr-transfer allows no migration parameter /
> cap setup, then the test case should follow, IMHO, rather than enabling
> anything from cmdline.
> 
> I hope that's trivial if we can have MigrateCommon.disable_events, for
> example, then only cpr selects it.

Enabling and waiting for events makes the test more robust, and means
the same recipe test_precopy_common() can be used for cpr and migration.
All good things I think, so I would prefer to use -global to enable
events, if you do not object.  With a comment saying "do not do this
outside of the tests".

>> * migration test needs to fetch the dynamically assigned migration
>>      listen port number
>>    Fix: require unix domain socket for cpr-transfer, or a fixed port
>>    number. Document it.
> 
> Fixed port is probably not a good idea.. requires unix sockets looks fine.

Daniel mentioned that libvirt chooses and uses a fixed port.

> But then again the whole point previously having -cpr-uri together with
> -incoming (IIRC, from your previous email) is to have that flexibility to
> use non-unix too.  Now I am not sure whether it is still needed to be
> separate just that you'll still need to rely on SIGHUP just because
> precreate is gone.

My "fix" goes too far.  It should restrict minimally:
   "Disallow dynamic tcp port for cpr transfer"

>> * migration test hangs connecting to the qtest socket.
>>    fix: in the qtest code, defer connection.
> 
> Not sure how much change here, hopefully still manageable.

I already coded that before going on the precreate tangent.
Not a huge change.

- Steve

>  From use case POV shouldn't be a huge issue, if we don't use -qtest in
> production anyway.
> 
>>
>> Document that one cannot set migration caps or params on the dest
>> for cpr-transfer.
> 
> This also needs to be reviewed by Dan, on impact of Libvirt initiating the
> dest QEMU instance with QMP all dead before a "migrate" on src.  I'm not
> sure whether Libvirt will also do early setup like what our qtest does to
> enable events and so on, assuming QMP available since the start.
> 
>>
>> Document that for -incoming defer, mgmt must send the migrate command
>> to the src first (so dest reads cpr state and progresses to start the
>> monitor), then send the hotplug monitor commands to the dest.
>>
>> Daniel, are you OK with that last bit?
>>
>> - Steve
>>
> 

Re: cpr-transfer with caveats

[Steven Sistare]

On 10/25/2024 11:54 AM, Daniel P. Berrangé wrote:
> On Fri, Oct 25, 2024 at 11:01:27AM -0400, Steven Sistare wrote:
>> Hi Peter, are you OK if we proceed with cpr-transfer as is, without the
>> precreate phase?  Here are the problems that motivated it:
>>
>> * migration test wants to enable migration events on the dest.
>>    fix: enable on dest qemu using -global.  only for the test.
>>
>> * migration test needs to fetch the dynamically assigned migration
>>      listen port number
>>    Fix: require unix domain socket for cpr-transfer, or a fixed port
>>    number. Document it.
>>
>> * migration test hangs connecting to the qtest socket.
>>    fix: in the qtest code, defer connection.
>>
>> Document that one cannot set migration caps or params on the dest
>> for cpr-transfer.
>>
>> Document that for -incoming defer, mgmt must send the migrate command
>> to the src first (so dest reads cpr state and progresses to start the
>> monitor), then send the hotplug monitor commands to the dest.
>>
>> Daniel, are you OK with that last bit?
> 
> I guess it depends on what happens inside QEMU between reading the
> cpr state and libvirt being able to access the monitor. Libvirt does
> various things with the monitor during QEMU startup, before guest
> vCPUs start. Mostly this is around host resource placement/mgmt
> that needs to be done before the guest CPUs start.

When the monitor starts, there will be no difference in observable VM state.
The only difference is how you get there -- libvirt must issue the migrate
command up front, rather than issuing it after issuing migrate_incoming.

- Steve