Re: [RFC 4/6] migration: Deprecate -incoming

2023-06-23 Thread Daniel P . Berrangé
On Fri, Jun 23, 2023 at 10:51:53AM -0400, Peter Xu wrote:
> On Fri, Jun 23, 2023 at 09:23:18AM +0100, Daniel P. Berrangé wrote:
> > On Thu, Jun 22, 2023 at 11:54:43AM -0400, Peter Xu wrote:
> > > On Thu, Jun 22, 2023 at 10:59:58AM +0100, Daniel P. Berrangé wrote:
> > > > I've mentioned several times before that the user should never need to
> > > > set this multifd-channels parameter (nor many other parameters) on the
> > > > destination in the first place.
> > > > 
> > > > The QEMU migration stream should be changed to add a full
> > > > bi-directional handshake, with negotiation of most parameters.
> > > > IOW, the src QEMU should be configured with 16 channels, and
> > > > it should connect the primary control channel, and then directly
> > > > tell the dest that it wants to use 16 multifd channels.
> > > > 
> > > > If we're expecting the user to pass this info across to the dest
> > > > manually we've already spectacularly failed wrt user friendliness.
> > > 
> > > I can try to move the todo even higher.  Trying to list the initial goals
> > > here:
> > > 
> > > - One extra phase of handshake between src/dst (maybe the time to boost
> > >   QEMU_VM_FILE_VERSION) before anything else happens.
> > > 
> > > - Dest shouldn't need to apply any cap/param, it should get all from src.
> > >   Dest still need to be setup with an URI and that should be all it needs.
> > 
> > There are a few that the dest will still need set explicitly. Specifically
> > the TLS parameters - tls-authz and tls-creds, because those are both
> > related to --object parameters configured on the dst QEMU. Potentially
> > there's an argument to be made for the TLS parameters to be part fo the
> > initial 'migrate' and 'migrate-incoming' command data, as they're
> > specifically related to the connection establishment, while (most) of
> > the other params are related to the migration protocol running inside
> > the connection.
> 
> Ideally we can even make tls options to be after the main connection is
> established, IOW the gnutls handshake can be part of the generic handshake.
> But yeah I agree that may contain much more work, so we may start with
> assuming the v2 handshake just happen on the tls channel built for now.
> 
> I think the new protocol should allow extension so when we want to move the
> tls handshake into it v2 protocol should be able to first detect src/dst
> binary support of that, and switch to that if we want - then we can even
> got a src qemu migration failure which tells "dest qemu forget to setup tls
> credentials in cmdlines", or anything wrong on dest during tls setup.

Doing negotiated "upgrades" from plain to TLS mode is generally frowned
upon, as it opens up potentially dangerous attack routes which can prevent
the upgrade from happening.

If the user/app controlling the client and server side of a connection
both know they want TLS, the best practice is for a connection to start
in TLS mode *immediately*, never sending any data in the clear. We have
this ability in QEMU right now, with libvirt explicitly enabling TLS
mode on src + dst, and we should keep that in any v2 migration protocol.


> > A few other parameters are also related to the connection establishment,
> > most notably the enablement multifd, postcopy and postcopy-pre-empt.
> 
> As I mentioned in the list, I plan to make this part of the default v2
> where v2 handshake will take care of managing the connections rather than
> relying on the old code.  I'm not sure how complicated it'll be, but the v2
> protocol just sounds a good fit for having such a major change on how we
> setup the channels, and chance we get all things alright from the start.
> 
> > 
> > I think with those ones we don't need to set them on the src either.
> > With the new migration handshake we should probably use multifd
> > codepaths unconditionally, with a single channel.
> 
> The v2 handshake will be beneficial to !multifd as well.  Right now I tend
> to make it also work for !multifd, e.g., it always makes sense to do a
> device tree comparision before migration, even if someone used special
> tunneling so multifd may not be able to be enabled for whatever reason, but
> as long as a return path is available so they can talk.
> 
> > By matching with the introduction of new protocol, we have a nice point
> > against which to deprecate the old non-multifd codepaths. We'll need to
> > keep the non-multifd code around *alot* longer than the normal
> > deprecation cycle though, as we need mig to/from very old QEMUs.
> 
> I actually had a feeling that we should always keep it..  I'm not sure
> whether we must combine a new handshake to "making multifd the default".  I
> do think we can make the !multifd path very simple though, e.g., I'd think
> we should start considering deprecate things like !multifd+compressions etc
> earlier than that.


My thought is that right now QEMU has effectively has two completely
distinct migration protocols (even though they share the 

Re: [RFC 4/6] migration: Deprecate -incoming

2023-06-23 Thread Peter Xu
On Fri, Jun 23, 2023 at 09:23:18AM +0100, Daniel P. Berrangé wrote:
> On Thu, Jun 22, 2023 at 11:54:43AM -0400, Peter Xu wrote:
> > On Thu, Jun 22, 2023 at 10:59:58AM +0100, Daniel P. Berrangé wrote:
> > > I've mentioned several times before that the user should never need to
> > > set this multifd-channels parameter (nor many other parameters) on the
> > > destination in the first place.
> > > 
> > > The QEMU migration stream should be changed to add a full
> > > bi-directional handshake, with negotiation of most parameters.
> > > IOW, the src QEMU should be configured with 16 channels, and
> > > it should connect the primary control channel, and then directly
> > > tell the dest that it wants to use 16 multifd channels.
> > > 
> > > If we're expecting the user to pass this info across to the dest
> > > manually we've already spectacularly failed wrt user friendliness.
> > 
> > I can try to move the todo even higher.  Trying to list the initial goals
> > here:
> > 
> > - One extra phase of handshake between src/dst (maybe the time to boost
> >   QEMU_VM_FILE_VERSION) before anything else happens.
> > 
> > - Dest shouldn't need to apply any cap/param, it should get all from src.
> >   Dest still need to be setup with an URI and that should be all it needs.
> 
> There are a few that the dest will still need set explicitly. Specifically
> the TLS parameters - tls-authz and tls-creds, because those are both
> related to --object parameters configured on the dst QEMU. Potentially
> there's an argument to be made for the TLS parameters to be part fo the
> initial 'migrate' and 'migrate-incoming' command data, as they're
> specifically related to the connection establishment, while (most) of
> the other params are related to the migration protocol running inside
> the connection.

Ideally we can even make tls options to be after the main connection is
established, IOW the gnutls handshake can be part of the generic handshake.
But yeah I agree that may contain much more work, so we may start with
assuming the v2 handshake just happen on the tls channel built for now.

I think the new protocol should allow extension so when we want to move the
tls handshake into it v2 protocol should be able to first detect src/dst
binary support of that, and switch to that if we want - then we can even
got a src qemu migration failure which tells "dest qemu forget to setup tls
credentials in cmdlines", or anything wrong on dest during tls setup.

> 
> A few other parameters are also related to the connection establishment,
> most notably the enablement multifd, postcopy and postcopy-pre-empt.

As I mentioned in the list, I plan to make this part of the default v2
where v2 handshake will take care of managing the connections rather than
relying on the old code.  I'm not sure how complicated it'll be, but the v2
protocol just sounds a good fit for having such a major change on how we
setup the channels, and chance we get all things alright from the start.

> 
> I think with those ones we don't need to set them on the src either.
> With the new migration handshake we should probably use multifd
> codepaths unconditionally, with a single channel.

The v2 handshake will be beneficial to !multifd as well.  Right now I tend
to make it also work for !multifd, e.g., it always makes sense to do a
device tree comparision before migration, even if someone used special
tunneling so multifd may not be able to be enabled for whatever reason, but
as long as a return path is available so they can talk.

> By matching with the introduction of new protocol, we have a nice point
> against which to deprecate the old non-multifd codepaths. We'll need to
> keep the non-multifd code around *alot* longer than the normal
> deprecation cycle though, as we need mig to/from very old QEMUs.

I actually had a feeling that we should always keep it..  I'm not sure
whether we must combine a new handshake to "making multifd the default".  I
do think we can make the !multifd path very simple though, e.g., I'd think
we should start considering deprecate things like !multifd+compressions etc
earlier than that.

> 
> The enablement of postcopy could be automatic too - src & dst can
> both detect if their host OS supports it. That would make all
> migrations post-copy capable. The mgmt app just needs to trigger
> the switch to post-copy mode *if* they want to use it.

Sounds doable.

> 
> Likewise we can just always assume postcopy-pre-empt is available.
> 
> I think 'return-path' becomes another one we can just assume too.

Right, handshake cap (or with the new QAPI of URI replacement) should imply
return path already.

Thanks,

-- 
Peter Xu




Re: [RFC 4/6] migration: Deprecate -incoming

2023-06-23 Thread Peter Xu
On Fri, Jun 23, 2023 at 08:17:46AM +0100, Daniel P. Berrangé wrote:
> On Thu, Jun 22, 2023 at 03:20:01PM -0400, Peter Xu wrote:
> > On Thu, Jun 22, 2023 at 05:33:29PM +0100, Daniel P. Berrangé wrote:
> > > On Thu, Jun 22, 2023 at 11:54:43AM -0400, Peter Xu wrote:
> > > > I can try to move the todo even higher.  Trying to list the initial 
> > > > goals
> > > > here:
> > > > 
> > > > - One extra phase of handshake between src/dst (maybe the time to boost
> > > >   QEMU_VM_FILE_VERSION) before anything else happens.
> > > > 
> > > > - Dest shouldn't need to apply any cap/param, it should get all from 
> > > > src.
> > > >   Dest still need to be setup with an URI and that should be all it 
> > > > needs.
> > > > 
> > > > - Src shouldn't need to worry on the binary version of dst anymore as 
> > > > long
> > > >   as dest qemu supports handshake, because src can fetch it from dest.
> > > 
> > > I'm not sure that works in general. Even if we have a handshake and
> > > bi-directional comms for live migration, we still haave the save/restore
> > > to file codepath to deal with. The dst QEMU doesn't exist at the time
> > > the save process is done, so we can't add logic to VMSate handling that
> > > assumes knowledge of the dst version at time of serialization.
> > 
> > My current thought was still based on a new cap or anything the user would
> > need to specify first on both sides (but hopefully the last cap to set on
> > dest).
> > 
> > E.g. if with a new handshake cap we shouldn't set it on a exec: or file:
> > protocol migration, and it should just fail on qmp_migrate() telling that
> > the URI is not supported if the cap is set.  Return path is definitely
> > required here.
> 
> exec can support bi-directional migration - we have both stdin + stdout
> for the command. For exec it is mostly a documentation problem - you
> can't merely use 'cat' for example, but if you used 'socat' that could
> be made to work bi-directionally.

Okay.  Just an example that the handshake just cannot work for all the
cases, and it should always be able to fail.

So when exec doesn't properly provide return path, I think with
handshake=on we should get a timeout of not getting response properly and
fail the migration after the timeout, then.

There're a bunch of implications and details that need to be investigated
around such a handshake if it'll be proposed for real, so I'm not yet sure
whether there's something that may be surprising.  For channeltypes it
seems all fine for now.  Hopefully nothing obvious overlooked.

> 
> > > I don't think its possible for QEMU to validate that it has a fully
> > > bi-directional channel, without adding timeouts to its detection which I
> > > think we should strive to avoid.
> > > 
> > > I don't think we actually need self-bootstrapping anyway.
> > > 
> > > I think the mgmt app can just indicate the new v2 bi-directional
> > > protocol when issuing the 'migrate' and 'migrate-incoming'
> > > commands.  This becomes trivial when Het's refactoring of the
> > > migrate address QAPI is accepted:
> > > 
> > >   https://lists.gnu.org/archive/html/qemu-devel/2023-05/msg04851.html
> > > 
> > > eg:
> > > 
> > > { "execute": "migrate",
> > >   "arguments": {
> > >   "channels": [ { "channeltype": "main",
> > >   "addr": { "transport": "socket", "type": "inet",
> > >"host": "10.12.34.9",
> > > "port": "1050" } } ] } }
> > > 
> > > note the 'channeltype' parameter here. If we declare the 'main'
> > > refers to the existing migration protocol, then we merely need
> > > to define a new 'channeltype' to use as an indicator for the
> > > v2 migration handshake protocol.
> > 
> > Using a new channeltype would also work at least on src qemu, but I'm not
> > sure on how dest qemu would know that it needs a handshake in that case,
> > because it knows nothing until the connection is established.
> 
> In Het's series the 'migrate_incoming' command similarly has a chaneltype
> parameter.

Oh, yeah then that'll just work.  Thanks.

-- 
Peter Xu




Re: [RFC 4/6] migration: Deprecate -incoming

2023-06-23 Thread Daniel P . Berrangé
On Thu, Jun 22, 2023 at 11:54:43AM -0400, Peter Xu wrote:
> On Thu, Jun 22, 2023 at 10:59:58AM +0100, Daniel P. Berrangé wrote:
> > I've mentioned several times before that the user should never need to
> > set this multifd-channels parameter (nor many other parameters) on the
> > destination in the first place.
> > 
> > The QEMU migration stream should be changed to add a full
> > bi-directional handshake, with negotiation of most parameters.
> > IOW, the src QEMU should be configured with 16 channels, and
> > it should connect the primary control channel, and then directly
> > tell the dest that it wants to use 16 multifd channels.
> > 
> > If we're expecting the user to pass this info across to the dest
> > manually we've already spectacularly failed wrt user friendliness.
> 
> I can try to move the todo even higher.  Trying to list the initial goals
> here:
> 
> - One extra phase of handshake between src/dst (maybe the time to boost
>   QEMU_VM_FILE_VERSION) before anything else happens.
> 
> - Dest shouldn't need to apply any cap/param, it should get all from src.
>   Dest still need to be setup with an URI and that should be all it needs.

There are a few that the dest will still need set explicitly. Specifically
the TLS parameters - tls-authz and tls-creds, because those are both
related to --object parameters configured on the dst QEMU. Potentially
there's an argument to be made for the TLS parameters to be part fo the
initial 'migrate' and 'migrate-incoming' command data, as they're
specifically related to the connection establishment, while (most) of
the other params are related to the migration protocol running inside
the connection.

A few other parameters are also related to the connection establishment,
most notably the enablement multifd, postcopy and postcopy-pre-empt.

I think with those ones we don't need to set them on the src either.
With the new migration handshake we should probably use multifd
codepaths unconditionally, with a single channel. By matching with
the introduction of new protocol, we have a nice point against which
to deprecate the old non-multifd codepaths. We'll need to keep the
non-multifd code around *alot* longer than the normal deprecation
cycle though, as we need mig to/from very old QEMUs.

The enablement of postcopy could be automatic too - src & dst can
both detect if their host OS supports it. That would make all
migrations post-copy capable. The mgmt app just needs to trigger
the switch to post-copy mode *if* they want to use it.

Likewise we can just always assume postcopy-pre-empt is available.

I think 'return-path' becomes another one we can just assume too.

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: [PULL 00/30] Next patches

2023-06-23 Thread Juan Quintela
Richard Henderson  wrote:
> On 6/22/23 18:54, Juan Quintela wrote:
>> The following changes since commit b455ce4c2f300c8ba47cba7232dd03261368a4cb:
>>Merge tag 'q800-for-8.1-pull-request'
>> ofhttps://github.com/vivier/qemu-m68k  into staging (2023-06-22
>> 10:18:32 +0200)
>> are available in the Git repository at:
>>https://gitlab.com/juan.quintela/qemu.git  tags/next-pull-request
>> for you to fetch changes up to
>> 23e4307eadc1497bd0a11ca91041768f15963b68:
>>migration/rdma: Split qemu_fopen_rdma() into input/output
>> functions (2023-06-22 18:11:58 +0200)
>> 
>> Migration Pull request (20230621) take 2
>> In this pull request the only change is fixing 32 bits complitaion
>> issue.
>> Please apply.
>> [take 1]
>> - fix for multifd thread creation (fabiano)
>> - dirtylimity (hyman)
>>* migration-test will go on next PULL request, as it has failures.
>> - Improve error description (tejus)
>> - improve -incoming and set parameters before calling incoming (wei)
>> - migration atomic counters reviewed patches (quintela)
>> - migration-test refacttoring reviewed (quintela)

I had the feeling when I wake up that today was going to be a great day.
Confirmed.

> New failure with check-cfi-x86_64:

Aha. CFI.  Something that I don't know what it is failing on me.

/me googles.

/me enables cfi+lto and compiles with clang.

50/491] Compiling C object 
subprojects/berkeley-testfloat-3/libtestfloat.a.p/source_test_az_f128_rx.c.o
[51/491] Compiling C object 
subprojects/berkeley-testfloat-3/libtestfloat.a.p/source_test_az_f128.c.o
[52/491] Compiling C object 
subprojects/berkeley-testfloat-3/libtestfloat.a.p/source_test_abz_f128.c.o
[53/491] Compiling C object 
subprojects/berkeley-testfloat-3/libtestfloat.a.p/source_test_abcz_f128.c.o
[54/491] Compiling C object 
subprojects/berkeley-testfloat-3/libtestfloat.a.p/source_test_ab_f128_z_bool.c.o
[55/491] Linking target qemu-system-x86_64
FAILED: qemu-system-x86_64 
clang++ -m64 -mcx16 @qemu-system-x86_64.rsp
/usr/bin/ld: cannot find libchardev.fa: Too many open files
/usr/bin/ld: cannot find libqmp.fa: Too many open files
/usr/bin/ld: cannot find libpage-vary-common.a: Too many open files
/usr/bin/ld: cannot find libqemuutil.a: Too many open files
/usr/bin/ld: cannot find subprojects/libvhost-user/libvhost-user-glib.a: Too 
many open files
/usr/bin/ld: cannot find subprojects/libvhost-user/libvhost-user.a: Too many 
open files
/usr/bin/ld: cannot find tcg/libtcg_softmmu.fa: Too many open files
/usr/bin/ld: cannot find libmigration.fa: Too many open files
/usr/bin/ld: cannot find libhwcore.fa: Too many open files
/usr/bin/ld: cannot find libqom.fa: Too many open files
/usr/bin/ld: cannot find gdbstub/libgdb_softmmu.fa: Too many open files
/usr/bin/ld: cannot find libio.fa: Too many open files
/usr/bin/ld: cannot find libcrypto.fa: Too many open files
/usr/bin/ld: cannot find libauthz.fa: Too many open files
/usr/bin/ld: cannot find libblockdev.fa: Too many open files
/usr/bin/ld: cannot find libblock.fa: Too many open files
/usr/bin/ld: cannot find libchardev.fa: Too many open files
/usr/bin/ld: cannot find libqmp.fa: Too many open files
/usr/bin/ld: cannot find /usr/lib64/libpixman-1.so: Too many open files
/usr/bin/ld: cannot find /usr/lib64/libepoxy.so: Too many open files
/usr/bin/ld: cannot find /usr/lib64/libxenctrl.so: Too many open files
/usr/bin/ld: cannot find /usr/lib64/libxenstore.so: Too many open files
/usr/bin/ld: cannot find /usr/lib64/libxenforeignmemory.so: Too many open files
/usr/bin/ld: cannot find /usr/lib64/libxengnttab.so: Too many open files
/usr/bin/ld: cannot find /usr/lib64/libxenevtchn.so: Too many open files

Confirmed, today is going to be a great day.

No check-cfi target for me.

/me investigates what is going on.  Found this and retries.

AR=llvm-ar CC=clang CXX=clang++ /mnt/code/qemu/full/configure
--enable-cfi --target-list=x86_64-softmmu

Gives the same error.

After a while of desesperation trying to disable features, etc, etc
Just doing a plain ulimit -n 4096 fixed the problem.

Here we go.

Later, Juan.




Re: [RFC 4/6] migration: Deprecate -incoming

2023-06-23 Thread Daniel P . Berrangé
On Thu, Jun 22, 2023 at 03:20:01PM -0400, Peter Xu wrote:
> On Thu, Jun 22, 2023 at 05:33:29PM +0100, Daniel P. Berrangé wrote:
> > On Thu, Jun 22, 2023 at 11:54:43AM -0400, Peter Xu wrote:
> > > I can try to move the todo even higher.  Trying to list the initial goals
> > > here:
> > > 
> > > - One extra phase of handshake between src/dst (maybe the time to boost
> > >   QEMU_VM_FILE_VERSION) before anything else happens.
> > > 
> > > - Dest shouldn't need to apply any cap/param, it should get all from src.
> > >   Dest still need to be setup with an URI and that should be all it needs.
> > > 
> > > - Src shouldn't need to worry on the binary version of dst anymore as long
> > >   as dest qemu supports handshake, because src can fetch it from dest.
> > 
> > I'm not sure that works in general. Even if we have a handshake and
> > bi-directional comms for live migration, we still haave the save/restore
> > to file codepath to deal with. The dst QEMU doesn't exist at the time
> > the save process is done, so we can't add logic to VMSate handling that
> > assumes knowledge of the dst version at time of serialization.
> 
> My current thought was still based on a new cap or anything the user would
> need to specify first on both sides (but hopefully the last cap to set on
> dest).
> 
> E.g. if with a new handshake cap we shouldn't set it on a exec: or file:
> protocol migration, and it should just fail on qmp_migrate() telling that
> the URI is not supported if the cap is set.  Return path is definitely
> required here.

exec can support bi-directional migration - we have both stdin + stdout
for the command. For exec it is mostly a documentation problem - you
can't merely use 'cat' for example, but if you used 'socat' that could
be made to work bi-directionally.

> > I don't think its possible for QEMU to validate that it has a fully
> > bi-directional channel, without adding timeouts to its detection which I
> > think we should strive to avoid.
> > 
> > I don't think we actually need self-bootstrapping anyway.
> > 
> > I think the mgmt app can just indicate the new v2 bi-directional
> > protocol when issuing the 'migrate' and 'migrate-incoming'
> > commands.  This becomes trivial when Het's refactoring of the
> > migrate address QAPI is accepted:
> > 
> >   https://lists.gnu.org/archive/html/qemu-devel/2023-05/msg04851.html
> > 
> > eg:
> > 
> > { "execute": "migrate",
> >   "arguments": {
> >   "channels": [ { "channeltype": "main",
> >   "addr": { "transport": "socket", "type": "inet",
> >"host": "10.12.34.9",
> > "port": "1050" } } ] } }
> > 
> > note the 'channeltype' parameter here. If we declare the 'main'
> > refers to the existing migration protocol, then we merely need
> > to define a new 'channeltype' to use as an indicator for the
> > v2 migration handshake protocol.
> 
> Using a new channeltype would also work at least on src qemu, but I'm not
> sure on how dest qemu would know that it needs a handshake in that case,
> because it knows nothing until the connection is established.

In Het's series the 'migrate_incoming' command similarly has a chaneltype
parameter.

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