[Spice-devel] [PATCH spice-gtk] spice-session: Fix SWAP_STR macro

[Frediano Ziglio]

> 
> Really swap "x" and "y", not temporary copies.
> The issue was introduced by 01c6343 "Use macro to swap
> data in spice_session_start_migrating()".
> 
> Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
> ---
>  src/spice-session.c | 9 +++------
>  1 file changed, 3 insertions(+), 6 deletions(-)
> 
> Removed RFC.
> Tested, the original session is updated with the new values
> after all connections are established.
> As usually there are no other connection after these the
> problem is not noted.
> 

The patch is working. I manage to test it setting up 2 VMs with iSCSI target,
quite easy setup not requiring ages to do it, with NFS I had an issue and
system was having problems shutting down.

By the way. The "swap" is quite weird, looks like an hack to avoid changing
SpiceSession (also also similar code for SpiceChannel) pointers but on
the other side it make the code harder to understand. For instance for channel
is more complicated, you copy the base SpiceChannel (see the problem of "object
slicing" in C++) but then you have to reset the channel in order to put the
reset of the state to a proper one. On the server, IMHO more correctly, new
channel objects (also because the old object are in another machine!) replace
old ones.

Another problem that I noted is at protocol level. The message from main channel
(DstInfo basically) contains new:
- hostname
- port
- secure port
- certificate
This however does not cover:
- accesses through ssh
- accesses through proxy
- using unix socket (if one host is local)
Maybe would be worth adding a generic URL? Note that in case of proxy the
URL should be provided externally (maybe depending on the client).

I remember also Victor had some additional need to extend the protocol
due to other migration limitations.

> diff --git a/src/spice-session.c b/src/spice-session.c
> index 04ba124a..d0d9e541 100644
> --- a/src/spice-session.c
> +++ b/src/spice-session.c
> @@ -1742,12 +1742,9 @@ void spice_session_switching_disconnect(SpiceSession
> *self)
>  }
>  
>  #define SWAP_STR(x, y) G_STMT_START { \
> -    const gchar *tmp;                 \
> -    const gchar *a = x;               \
> -    const gchar *b = y;               \
> -    tmp = a;                          \
> -    a = b;                            \
> -    b = tmp;                          \
> +    gchar *tmp = x;                   \
> +    x = y;                            \
> +    y = tmp;                          \
>  } G_STMT_END
>  
>  G_GNUC_INTERNAL
> --
> 2.20.1
> 
>