Re: [PATCH spice-gtk] spice-session: Fix SWAP_STR macro

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



> 
> 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@xxxxxxxxxx>
> ---
>  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
> 
> 
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel




[Index of Archives]     [Linux Virtualization]     [Linux Virtualization]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]     [Monitors]