Re: [Spice-devel] Getting involved with implementation of remote spice for 3D accelerated VMs

2017-01-11 Thread Victor Toso
Hi,

On Mon, Jan 09, 2017 at 05:20:10PM +0100, Behrooz Shabani wrote:
> Thanks for the info Victor.
>
> I will try to set it up and because I am new to the code base and have no
> idea where to start, I will ping you on IRC for help if you don't mind
> (assuming you're `toso`).

As discussed by IRC in regards to the preferred video-codec message,
I've pushed it to my gitlab, you should be able to build with master
spice-protocol and the stream-preference branches from:
- https://gitlab.com/victortoso/spice-common
- https://gitlab.com/victortoso/spice-gtk
- https://gitlab.com/victortoso/spice

Let me know if you have any issues and comments/suggestions are welcome!

Cheers,
  toso

>
> On Mon, Jan 9, 2017 at 5:02 PM, Victor Toso  wrote:
> 
> > Hi,
> >
> > On Mon, Jan 09, 2017 at 04:27:35AM -0500, Frediano Ziglio wrote:
> > > The patches for Virgl are working and require only tweaks.  We wanted
> > > to have some working PoC for all next 3D software, that include GPU
> > > passtrough to make sure that the Virgl doesn't complicate the overall
> > > design. We plan to have these definitive PoCs in a couple of months.
> > >
> > > About video streaming acceleration for the client somebody are doing
> > > some stuff. Recent client versions are using gstreamer to handle
> > > decoding so potentially this could be a question of choosing the right
> > > plugins, test and package them correctly
> >
> > Yes, I'm trying to play with it now and then in the client using
> > gstreamer1-vaapi it works but we don't handle hw decoding specifically.
> > If you have all the right bits installed, you can try enabling
> > SPICE_GSTVIDEO_AUTO which uses decodebin. It should try vaapi elemets
> > (and hw decoding) if possible.
> >
> > But to have this working nicely, we should have an easy way to setup
> > spice-sever to use vp8 or h264 which broadens the hw decoding
> > possibilities (and makes much easier to test too!) - So, I've started
> > playing with that first [0]
> >
> > [0] https://lists.freedesktop.org/archives/spice-devel/2017-
> > January/034815.html
> >
> > With that in, working on improving the client-side to handle hw decoding
> > will be easier. If you are interested in the decoding side, feel free to
> > jump in :)
> >
> > > (this is more challenging for Windows for instance).
> >
> > Indeed
> >
> > >
> > > Frediano
> > >
> > > - Original Message -
> > >
> > > > From: "Behrooz Shabani" 
> > > > To: "spice-devel" 
> > > > Cc: "Armin Ranjbar" 
> > > > Sent: Sunday, January 8, 2017 12:39:52 PM
> > > > Subject: Re: [Spice-devel] Getting involved with implementation of
> > remote
> > > > spice for 3D accelerated VMs
> > >
> > > > Thanks Gerd and hello mailing list :-),
> > >
> > > > As it is obvious from my previous email, I am looking for some
> > guidance to
> > > > make some contribution while I am working on my specialization.
> > >
> > > > At this moment, I know what I like to be able to do with Spice:
> > >
> > > > * Remote access with 3D acceleration
> > > > * Decoding the video stream via GPU on the client.
> > > > However, I have no idea what their status is and where I should begin
> > to
> > > > contribute. Therefore, any information towards making me useful to
> > spice (as
> > > > I want to do more than just basing my research on virtgl/spice), would
> > be
> > > > very helpful.
> > >
> > > > Thanks in advance.
> > >
> > > > P.S. I have joined the IRC channel with the nickname `behrooz` as well.
> > >
> > > > On Fri, Jan 6, 2017 at 10:45 AM, Gerd Hoffmann < kra...@redhat.com >
> > wrote:
> > >
> > > > > On Mi, 2017-01-04 at 15:40 +0100, Behrooz Shabani wrote:
> > > >
> > > > > > Hi Gerd,
> > > >
> > > > > >
> > > >
> > > > > >
> > > >
> > > > > > I hope you are doing well.
> > > >
> > > > > >
> > > >
> > > > > >
> > > >
> > > > > > I watched one of your talks about virgl and spice, great work!
> > > >
> > > > > >
> > > >
> > > > > >
> > > >
> > > > > > Currently, I am busy with my specialization at Saxion university
> > which
> > > >
> > > > > > is about virtualization of graphic cards.
> > > >
> > > > > >
> > > >
> > > > > >
> > > >
> > > > > > I was wondering how I can get involved to implement 3D accelerated
> > VM
> > > >
> > > > > > over network with spice. And after that, maybe, with using the GPU
> > of
> > > >
> > > > > > the system that runs spice to decode the video stream.
> > Essentially, I
> > > >
> > > > > > am looking for your guidance to understand what needs to be done
> > so I
> > > >
> > > > > > can get on with implementing it.
> > > >
> > >
> > > > > That question is asked best on the spice-devel mainling list (added
> > to
> > > >
> > > > > Cc:).
> > > >
> > >
> > > > > Improving video encoding support in spice is making progress, and
> > remote
> > > >
> > > > > support for virtio-gpu (and intel-vgpu) will build on top of that.
> > > >
> > >
> > > > > But I don't know in detail what the 

Re: [Spice-devel] [PATCH spice-gtk v5 3/9] Update spice-common

2017-01-11 Thread Victor Toso
Hi,

On Fri, Jan 06, 2017 at 10:22:08AM -0500, Frediano Ziglio wrote:
> > 
> > > 
> > > From: Victor Toso 
> > > 
> > > Victor Toso (1):
> > >   protocol: add preferred video codec message
> > > 
> > > Signed-off-by: Victor Toso 
> > > ---
> > >  spice-common | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/spice-common b/spice-common
> > > index 86dcd22..67cea5e 16
> > > --- a/spice-common
> > > +++ b/spice-common
> > > @@ -1 +1 @@
> > > -Subproject commit 86dcd22d334a4992718780417bc574623bd61826
> > > +Subproject commit 67cea5ee8cafe3d7eb119a420849affa518eddd5
> > 
> > Acked-by: Frediano Ziglio 
> > 
> > Frediano
> 
> Sorry, confused with 8/9 (which has the same subject).
> 
> Frediano

Still, as the protocol patch was pushed and spice-server's spice-common
has been updated to have this, I'll be pushing this one in spice-gtk.

Cheers,



signature.asc
Description: PGP signature
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel


Re: [Spice-devel] [spice-gtk v2 3/3] ssl: Use accessors rather than direct struct access

2017-01-11 Thread Christophe Fergeau
On Wed, Jan 11, 2017 at 04:37:01PM +0100, Pavel Grunt wrote:
> On Wed, 2017-01-11 at 10:50 +0100, Christophe Fergeau wrote:
> > From: Sebastian Andrzej Siewior 
> > 
> > In OpenSSL 1.1.0, the struct fields are private so we can no longer
> > directly access them.
> > 
> > The accessors are not available in previous OpenSSL releases, so we
> > need
> > to add compat helpers.
> > ---
> >  src/bio-gio.c   | 108
> > 
> >  src/spice-channel.c |  11 +-
> >  2 files changed, 102 insertions(+), 17 deletions(-)
> > 
> > diff --git a/src/bio-gio.c b/src/bio-gio.c
> > index 0f8b415..ef47cd3 100644
> > --- a/src/bio-gio.c
> > +++ b/src/bio-gio.c
> > @@ -83,30 +152,37 @@ static int bio_gio_puts(BIO *bio, const char
> > *str)
> >  return ret;
> >  }
> >  
> > -#define BIO_TYPE_START 128
> > +static BIO_METHOD *bio_gio_method;
> >  
> >  G_GNUC_INTERNAL
> >  BIO* bio_new_giostream(GIOStream *stream)
> >  {
> >  BIO *bio;
> > -static BIO_METHOD bio_gio_method;
> >  
> > -if (bio_gio_method.name == NULL) {
> > -bio_gio_method.type = BIO_TYPE_START |
> > BIO_TYPE_SOURCE_SINK;
> > -bio_gio_method.name = "gio stream";
> > +if (!bio_gio_method) {
> > +int index = BIO_get_new_index();
> > +g_warning("index: %i", index);
> 
> probably a leftover ^

Oops.. Removed these 2 lines!

Christophe


signature.asc
Description: PGP signature
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel


Re: [Spice-devel] [spice-gtk v2 0/3] ssl: Add support for OpenSSL 1.1.0

2017-01-11 Thread Pavel Grunt
On Wed, 2017-01-11 at 15:26 +0100, Christophe Fergeau wrote:
> On Wed, Jan 11, 2017 at 03:05:45PM +0100, Fabian Grünbichler wrote:
> > AFAIK the situation regarding OpenSSL in Sid/Stretch is rather
> > complicated - upstream / the OpenSSL maintainers / .. are pushing
> > for a
> > transition to 1.1.0 (currently 1.1.0c), but there was quite a
> > backlash
> > from various maintainers because of the ABI breakage and not-yet-
> > updated
> > upstreams. Now there are two -dev packages, libssl-dev (previously
> > 1.0.X, now 1.1.0c) and libssl1.0-dev and a boatload of bug reports
> > and
> > patches for various packages to transition to 1.1.0 in a backwards
> > compatible way. Whether the move to 1.1.0 for Stretch will really
> > happen
> > was unclear the last time I checked, but that was some time in
> > December
> > (or maybe even November?). You can check the debian-devel archives
> > if
> > you are in the mood for reading long back and forth threads ;)
> 
> Hehe, ok, thanks for the details, I think I'll skip the mailing list
> archives ;)
> 
> > 
> > Anyhow, the spice client packages in Debian Stretch are still
> > compiled
> > against libssl1.0-dev (which is 1.0.2j-4), the ones in Debian Sid
> > are
> > compiled against libssl-dev (which is 1.1.0c-2).
> > 
> > > 
> > > I assume you are getting the same results if you use --spice-ca-
> > > file
> > > instead of .spicec/spice_truststore.pem?
> > > By any chance, would it be possible for you to try the latest
> > > version of
> > > these patches rather than what they have in debian, as they are
> > > a bit
> > > different at this point?
> > > 
> > 
> > Compiling the current spice-gtk master with v2 of this patch set
> > 
> > - against libssl-dev on Sid works as expected
> > - against libssl1.0-dev on Sid works as expected (which matches
> > with
> >   your result)
> > 
> > Compiling the current spice-gtk master without the patch set
> > - against libssl-dev on Sid does not work (hence the patch set ;))
> > - against libssl1.0-dev on Sid works as expected
> > 
> > Replacing the old patch in Debian Sid's source package with v2 of
> > this
> > patch set and rebuilding also works as expected. I'll open a bug
> > report
> > on the Debian side to get the current patch set included in the
> > Debian
> > package, unless there are any objections (or an upcoming v3) from
> > your
> > side?
> 
> I think the current iteration should be fine, especially with the
> additional testing you just did ;) Thanks for the investigation and
> getting to the bottom of it!
> 
> Christophe

Thanks for testing, patches look good to me

Pavel

> ___
> Spice-devel mailing list
> Spice-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel


Re: [Spice-devel] [spice-gtk v2 3/3] ssl: Use accessors rather than direct struct access

2017-01-11 Thread Pavel Grunt
On Wed, 2017-01-11 at 10:50 +0100, Christophe Fergeau wrote:
> From: Sebastian Andrzej Siewior 
> 
> In OpenSSL 1.1.0, the struct fields are private so we can no longer
> directly access them.
> 
> The accessors are not available in previous OpenSSL releases, so we
> need
> to add compat helpers.
> ---
>  src/bio-gio.c   | 108
> 
>  src/spice-channel.c |  11 +-
>  2 files changed, 102 insertions(+), 17 deletions(-)
> 
> diff --git a/src/bio-gio.c b/src/bio-gio.c
> index 0f8b415..ef47cd3 100644
> --- a/src/bio-gio.c
> +++ b/src/bio-gio.c
> @@ -23,6 +23,75 @@
>  #include "spice-util.h"
>  #include "bio-gio.h"
>  
> +#if OPENSSL_VERSION_NUMBER < 0x1010
> +static BIO_METHOD one_static_bio;
> +
> +static int BIO_meth_set_read(BIO_METHOD *biom,
> + int (*bread) (BIO *, char *, int))
> +{
> +biom->bread = bread;
> +return 1;
> +}
> +
> +static int BIO_meth_set_write(BIO_METHOD *biom,
> +  int (*bwrite) (BIO *, const char *,
> int))
> +{
> +biom->bwrite = bwrite;
> +return 1;
> +}
> +
> +static int BIO_meth_set_puts(BIO_METHOD *biom,
> + int (*bputs) (BIO *, const char *))
> +{
> +biom->bputs = bputs;
> +return 1;
> +}
> +
> +static int BIO_meth_set_ctrl(BIO_METHOD *biom,
> + long (*ctrl) (BIO *, int, long, void
> *))
> +{
> +biom->ctrl = ctrl;
> +return 1;
> +}
> +
> +#define BIO_TYPE_START 128
> +
> +static int BIO_get_new_index(void)
> +{
> +static int bio_index = BIO_TYPE_START;
> +return bio_index++;
> +}
> +
> +static void BIO_set_init(BIO *a, int init)
> +{
> + a->init = init;
> +}
> +
> +static void BIO_set_data(BIO *a, void *ptr)
> +{
> +a->ptr = ptr;
> +}
> +
> +static void *BIO_get_data(BIO *a)
> +{
> +return a->ptr;
> +}
> +
> +static BIO_METHOD *BIO_meth_new(int type, const char *name)
> +{
> +BIO_METHOD *biom = _static_bio;
> +
> +biom->type = type;
> +biom->name = name;
> +return biom;
> +}
> +
> +static void BIO_meth_free(BIO_METHOD *biom)
> +{
> +}
> +
> +#endif
> +
>  static long bio_gio_ctrl(G_GNUC_UNUSED BIO *b,
>   int cmd,
>   G_GNUC_UNUSED long num,
> @@ -37,7 +106,7 @@ static int bio_gio_write(BIO *bio, const char
> *in, int inl)
>  gssize ret;
>  GError *error = NULL;
>  
> -stream = g_io_stream_get_output_stream(bio->ptr);
> +stream = g_io_stream_get_output_stream(BIO_get_data(bio));
>  ret =
> g_pollable_output_stream_write_nonblocking(G_POLLABLE_OUTPUT_STREAM(
> stream),
>   in, inl, NULL,
> );
>  BIO_clear_retry_flags(bio);
> @@ -58,7 +127,7 @@ static int bio_gio_read(BIO *bio, char *out, int
> outl)
>  gssize ret;
>  GError *error = NULL;
>  
> -stream = g_io_stream_get_input_stream(bio->ptr);
> +stream = g_io_stream_get_input_stream(BIO_get_data(bio));
>  ret =
> g_pollable_input_stream_read_nonblocking(G_POLLABLE_INPUT_STREAM(str
> eam),
> out, outl, NULL,
> );
>  BIO_clear_retry_flags(bio);
> @@ -83,30 +152,37 @@ static int bio_gio_puts(BIO *bio, const char
> *str)
>  return ret;
>  }
>  
> -#define BIO_TYPE_START 128
> +static BIO_METHOD *bio_gio_method;
>  
>  G_GNUC_INTERNAL
>  BIO* bio_new_giostream(GIOStream *stream)
>  {
>  BIO *bio;
> -static BIO_METHOD bio_gio_method;
>  
> -if (bio_gio_method.name == NULL) {
> -bio_gio_method.type = BIO_TYPE_START |
> BIO_TYPE_SOURCE_SINK;
> -bio_gio_method.name = "gio stream";
> +if (!bio_gio_method) {
> +int index = BIO_get_new_index();
> +g_warning("index: %i", index);

probably a leftover ^

> +bio_gio_method = BIO_meth_new(BIO_get_new_index() |
> +  BIO_TYPE_SOURCE_SINK,
> +  "gio stream");
> +if (!bio_gio_method)
> +return NULL;
> +
> +if (!BIO_meth_set_write(bio_gio_method, bio_gio_write) ||
> +!BIO_meth_set_read(bio_gio_method, bio_gio_read) ||
> +!BIO_meth_set_puts(bio_gio_method, bio_gio_puts) ||
> +!BIO_meth_set_ctrl(bio_gio_method, bio_gio_ctrl)) {
> +BIO_meth_free(bio_gio_method);
> +bio_gio_method = NULL;
> +return NULL;
> +}
>  }
>  
> -bio = BIO_new(_gio_method);
> +bio = BIO_new(bio_gio_method);
>  if (!bio)
>  return NULL;
>  
> -bio->method->bwrite = bio_gio_write;
> -bio->method->bread = bio_gio_read;
> -bio->method->bputs = bio_gio_puts;
> -bio->method->ctrl = bio_gio_ctrl;
> -
> -bio->init = 1;
> -bio->ptr = stream;
> -
> +BIO_set_init(bio, 1);
> +BIO_set_data(bio, stream);
>  return bio;
>  }
> diff --git a/src/spice-channel.c b/src/spice-channel.c
> index 

Re: [Spice-devel] [spice-server 11/17] sound: Prefer snd_set_command() over snd_*_send_*()

2017-01-11 Thread Frediano Ziglio
> 
> snd_set_command()/snd_send() are higher level methods which take care of
> scheduling calls to the corresponding snd_*_send_*() methods when
> appropriate. This commit switches a few direct snd_*_send_*() calls to
> snd_set_command()/snd_send().
> 
> Based on a patch from Frediano Ziglio 
> 
> Signed-off-by: Christophe Fergeau 
> ---
>  server/sound.c | 12 
>  1 file changed, 4 insertions(+), 8 deletions(-)
> 
> diff --git a/server/sound.c b/server/sound.c
> index d45e4b9..b0a1367 100644
> --- a/server/sound.c
> +++ b/server/sound.c
> @@ -1094,7 +1094,6 @@ SPICE_GNUC_VISIBLE void
> spice_server_playback_set_volume(SpicePlaybackInstance *
>  {
>  SpiceVolumeState *st = >st->channel.volume;
>  SndChannelClient *client = sin->st->channel.connection;
> -PlaybackChannelClient *playback_client = SPICE_CONTAINEROF(client,
> PlaybackChannelClient, base);
>  
>  st->volume_nchannels = nchannels;
>  free(st->volume);
> @@ -1103,21 +1102,20 @@ SPICE_GNUC_VISIBLE void
> spice_server_playback_set_volume(SpicePlaybackInstance *
>  if (!client || nchannels == 0)
>  return;
>  
> -snd_playback_send_volume(playback_client);
> +snd_set_command(client, SND_VOLUME_MASK);
>  }
>  
>  SPICE_GNUC_VISIBLE void spice_server_playback_set_mute(SpicePlaybackInstance
>  *sin, uint8_t mute)
>  {
>  SpiceVolumeState *st = >st->channel.volume;
>  SndChannelClient *client = sin->st->channel.connection;
> -PlaybackChannelClient *playback_client = SPICE_CONTAINEROF(client,
> PlaybackChannelClient, base);
>  
>  st->mute = mute;
>  
>  if (!client)
>  return;
>  
> -snd_playback_send_mute(playback_client);
> +snd_set_command(client, SND_MUTE_MASK);

For compatibility you need to send both MUTE and VOLUME so SND_VOLUME_MUTE_MASK.

>  }
>  
>  static void snd_playback_start(SndChannel *channel)
> @@ -1378,7 +1376,6 @@ SPICE_GNUC_VISIBLE void
> spice_server_record_set_volume(SpiceRecordInstance *sin,
>  {
>  SpiceVolumeState *st = >st->channel.volume;
>  SndChannelClient *client = sin->st->channel.connection;
> -RecordChannelClient *record_client = SPICE_CONTAINEROF(client,
> RecordChannelClient, base);
>  
>  st->volume_nchannels = nchannels;
>  free(st->volume);
> @@ -1387,21 +1384,20 @@ SPICE_GNUC_VISIBLE void
> spice_server_record_set_volume(SpiceRecordInstance *sin,
>  if (!client || nchannels == 0)
>  return;
>  
> -snd_record_send_volume(record_client);
> +snd_set_command(client, SND_VOLUME_MASK);
>  }
>  
>  SPICE_GNUC_VISIBLE void spice_server_record_set_mute(SpiceRecordInstance
>  *sin, uint8_t mute)
>  {
>  SpiceVolumeState *st = >st->channel.volume;
>  SndChannelClient *client = sin->st->channel.connection;
> -RecordChannelClient *record_client = SPICE_CONTAINEROF(client,
> RecordChannelClient, base);
>  
>  st->mute = mute;
>  
>  if (!client)
>  return;
>  
> -snd_record_send_mute(record_client);
> +snd_set_command(client, SND_MUTE_MASK);
>  }
>  
>  static void snd_record_start(SndChannel *channel)

Frediano
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel


Re: [Spice-devel] [spice-gtk v2 0/3] ssl: Add support for OpenSSL 1.1.0

2017-01-11 Thread Christophe Fergeau
On Wed, Jan 11, 2017 at 03:05:45PM +0100, Fabian Grünbichler wrote:
> AFAIK the situation regarding OpenSSL in Sid/Stretch is rather
> complicated - upstream / the OpenSSL maintainers / .. are pushing for a
> transition to 1.1.0 (currently 1.1.0c), but there was quite a backlash
> from various maintainers because of the ABI breakage and not-yet-updated
> upstreams. Now there are two -dev packages, libssl-dev (previously
> 1.0.X, now 1.1.0c) and libssl1.0-dev and a boatload of bug reports and
> patches for various packages to transition to 1.1.0 in a backwards
> compatible way. Whether the move to 1.1.0 for Stretch will really happen
> was unclear the last time I checked, but that was some time in December
> (or maybe even November?). You can check the debian-devel archives if
> you are in the mood for reading long back and forth threads ;)

Hehe, ok, thanks for the details, I think I'll skip the mailing list
archives ;)

> 
> Anyhow, the spice client packages in Debian Stretch are still compiled
> against libssl1.0-dev (which is 1.0.2j-4), the ones in Debian Sid are
> compiled against libssl-dev (which is 1.1.0c-2).
> 
> > 
> > I assume you are getting the same results if you use --spice-ca-file
> > instead of .spicec/spice_truststore.pem?
> > By any chance, would it be possible for you to try the latest version of
> > these patches rather than what they have in debian, as they are a bit
> > different at this point?
> > 
> 
> Compiling the current spice-gtk master with v2 of this patch set
> 
> - against libssl-dev on Sid works as expected
> - against libssl1.0-dev on Sid works as expected (which matches with
>   your result)
> 
> Compiling the current spice-gtk master without the patch set
> - against libssl-dev on Sid does not work (hence the patch set ;))
> - against libssl1.0-dev on Sid works as expected
> 
> Replacing the old patch in Debian Sid's source package with v2 of this
> patch set and rebuilding also works as expected. I'll open a bug report
> on the Debian side to get the current patch set included in the Debian
> package, unless there are any objections (or an upcoming v3) from your
> side?

I think the current iteration should be fine, especially with the
additional testing you just did ;) Thanks for the investigation and
getting to the bottom of it!

Christophe


signature.asc
Description: PGP signature
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel


Re: [Spice-devel] [PATCH v2 2/2] Quiet uninitialized variable warning.

2017-01-11 Thread Christophe Fergeau
On Mon, Jan 09, 2017 at 02:11:02PM +0100, Michal Suchánek wrote:
> On Mon, 09 Jan 2017 10:51:34 +0100
> Pavel Grunt  wrote:
> 
> Hello,
> 
> > Hi Michal,
> > 
> > On Fri, 2017-01-06 at 14:25 +0100, Michal Suchanek wrote:
> > > Signed-off-by: Michal Suchanek 
> > > ---
> > >  src/vdagentd/vdagentd.c | 1 +
> > >  1 file changed, 1 insertion(+)
> > > 
> > > diff --git a/src/vdagentd/vdagentd.c b/src/vdagentd/vdagentd.c
> > > index 991514e..60a866e 100644
> > > --- a/src/vdagentd/vdagentd.c
> > > +++ b/src/vdagentd/vdagentd.c
> > > @@ -334,6 +334,7 @@ static void do_client_file_xfer(struct
> > > vdagent_virtio_port *vport,
> > >  id = d->id;
> > >  break;
> > >  }
> > > +default: return; /* quiet uninitialized variable warning */  
> > I would go for 'g_return_if_reached' - it will log a warning, also it
> > should be on a new line. I can do the changes and push if you agree.
> > 
> 
> I use this locally to be able to build with gcc 6. gcc 4.8.5 does not
> detect the issue and gcc 7 should be able to detect that the function
> is called from a switch statement where only the values already tested
> above are allowed. 

For what it's worth, I'm not seeing this warning with gcc6 (gcc (GCC)
6.3.1 20161221 (Red Hat 6.3.1-1))

Christophe


signature.asc
Description: PGP signature
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel


Re: [Spice-devel] [spice-gtk v2 0/3] ssl: Add support for OpenSSL 1.1.0

2017-01-11 Thread Christophe Fergeau
Hey Fabian,

On Wed, Jan 11, 2017 at 12:21:35PM +0100, Fabian Grünbichler wrote:
> On Wed, Jan 11, 2017 at 10:50:30AM +0100, Christophe Fergeau wrote:
> > Sebastian sent these patches privately a while ago, I've run some tests on 
> > them
> > and helped split them. They add support for OpenSSL 1.1.0 which makes some 
> > of the
> > structures we were directly accessing private. This also keeps support with 
> > older
> > OpenSSL releases by adding some compat helpers.
> > 
> > These patches have been tested against a RHEV instance, and against 
> > manually configured
> > QEMU+SPICE+TLS.
> 
> Might be our (unusual) setup, but this and the previous version of this
> patch included in Debian Sid seem to break connecting to Spice servers
> listening only on TLS.

Thanks for the report!

> 
> starting a qemu VM with
> 
>  -chardev spicevmc,id=charchannel0,name=vdagent \
>  -device 
> virtserialport,bus=virtio-serial0.0,nr=1,chardev=charchannel0,id=channel0,name=com.redhat.spice.0
>  \
>  -spice 
> tls-port=5901,addr=AAA.BBB.CCC.DDD,disable-ticketing,x509-dir=/etc/pki/libvirt-spice,image-compression=off,seamless-migration=on
> 
> and QXL makes qemu listen on port 5901 with TLS only (tested with qemu
> and libvirt from Debian Sid, although we noticed the original problem
> when using Proxmox VE 4.4 on the server side, which is based on Debian
> Jessie and uses a custom Qemu).
> 
> connecting using spicy on Debian Sid (with appropriately set
> ~/.spicec/spice_truststore.pem and correctly generated self-signed
> certificates) doesn't work:
> 
> # dpkg --list | grep spice
> ii  gir1.2-spice-client-glib-2.0  0.33-3.2  
> amd64GObject for communicating with Spice servers 
> (GObject-Introspection)
> ii  gir1.2-spice-client-gtk-3.0   0.33-3.2  
> amd64GTK3 widget for SPICE clients (GObject-Introspection)
> ii  libspice-client-glib-2.0-8:amd64  0.33-3.2  
> amd64GObject for communicating with Spice servers (runtime library)
> ii  libspice-client-gtk-3.0-5:amd64   0.33-3.2  
> amd64GTK3 widget for SPICE clients (runtime library)
> ii  libspice-protocol-dev 0.12.12-1 
> all  SPICE protocol headers
> ii  libspice-server1:amd640.12.8-2  
> amd64Implements the server side of the SPICE protocol
> ii  spice-client-glib-usb-acl-helper  0.33-3.2  
> amd64Helper tool to validate usb ACLs
> ii  spice-client-gtk  0.33-3.2  
> amd64Simple clients for interacting with SPICE servers 
> 
> $ spicy -h AAA.BBB.CCC.DDD -s 5901 --spice-debug &> spicy-broken.log
> 
> see attachment for full log, excerpt:
> 
> GSpice-WARNING **: main-1:0: SSL_connect: 
> error:0001:lib(0):func(0):reason(1)
> GSpice-Message: main channel: failed to connect
> 
> the qemu process logs the following error message:
> (/usr/bin/qemu-system-x86_64:7758): Spice-Warning **: 
> reds_stream.c:379:reds_stream_ssl_accept: SSL_accept failed, error=5
> 
> downgrading the spice client packages to Stretch makes everything work
> again:
> 
> # apt-get install  gir1.2-spice-client-glib-2.0=0.33-3 
> gir1.2-spice-client-gtk-3.0=0.33-3  libspice-client-glib-2.0-8=0.33-3 
> libspice-client-gtk-3.0-5=0.33-3 spice-client-glib-usb-acl-helper
> =0.33-3 spice-client-gtk=0.33-3
> 
> $ spicy -h AAA.BBB.CCC.DDD -s 5901 --spice-debug &> spicy-works.log
> 
> also attached.
> 
> a TLS connection attempt with
> 
> $ openssl -debug -connect AAA.BBB.CCC.DDD:5901
> 
> shows that ECDHE-RSA-AES256-GCM-SHA384 is negotiated, but I did not
> verify whether this is also true for the connection made by spicy.
> 
> any idea what's going amiss here?
> 

I've tried to reproduce this with the latest version of the patches. I
setup a TLS-only SPICE VM on a RHEL6 host (using libvirt), and connected to it
with spicy, both with --spice-ca-file, and with a
.spicec/spice_truststore.pem  file, and in both cases this worked fine.
Is sid using openssl 1.1.0 or older? I've tested with
openssl-1.0.2j-3.fc25.x86_64

I assume you are getting the same results if you use --spice-ca-file
instead of .spicec/spice_truststore.pem?
By any chance, would it be possible for you to try the latest version of
these patches rather than what they have in debian, as they are a bit
different at this point?

Thanks,

Christophe


signature.asc
Description: PGP signature
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel


Re: [Spice-devel] [spice-server 04/17] sound: Remove duplicate AudioFrame typedef

2017-01-11 Thread Christophe Fergeau
On Wed, Jan 11, 2017 at 05:58:55AM -0500, Frediano Ziglio wrote:
> > 
> > It's already defined before in the same source file.
> > 
> > Based on a patch from Frediano Ziglio 
> > 
> > Signed-off-by: Christophe Fergeau 
> > ---
> >  server/sound.c | 1 -
> >  1 file changed, 1 deletion(-)
> > 
> > diff --git a/server/sound.c b/server/sound.c
> > index 1f6c243..d24a0d9 100644
> > --- a/server/sound.c
> > +++ b/server/sound.c
> > @@ -123,7 +123,6 @@ struct SndChannelClient {
> >  snd_channel_cleanup_channel_proc cleanup;
> >  };
> >  
> > -typedef struct AudioFrame AudioFrame;
> >  struct AudioFrame {
> >  uint32_t time;
> >  uint32_t samples[SND_CODEC_MAX_FRAME_SIZE];
> 
> Can I ack my hunk or should I consider your signed-off like
> an ack? I usually prefer a sign A + sign B + ack A

Dunno :) I'm personally fine with considering everything up to
"sound: Add sanity checks in snd_{playback,record}_send" (included) as

Acked-by: Christophe Fergeau 

though I would not ack a patch I'm also marked as the git author, so
having your ack on top makes sense I guess.

Christophe


signature.asc
Description: PGP signature
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel


Re: [Spice-devel] [spice-server 08/17] sound: Implement snd_channel_config_socket

2017-01-11 Thread Frediano Ziglio
> 
> This is in preparation for switching SndChannelClient into a proper
> RedChannelClient. The prototype of the new helper matches what is
> expected from the RedChannel::config_socket vfunc.
> 
> To be able to achieve that, this commit associates the sound channel
> RedsStream instance with the DummyChannelClient instance we have, and
> then call snd_channel_config_socket() on that instance.
> 
> Based on a patch from Frediano Ziglio 
> 
> Signed-off-by: Christophe Fergeau 
> ---
>  server/dummy-channel-client.c |  2 +
>  server/dummy-channel-client.h |  1 +
>  server/sound.c| 96
>  ++-
>  3 files changed, 53 insertions(+), 46 deletions(-)
> 
> diff --git a/server/dummy-channel-client.c b/server/dummy-channel-client.c
> index 61d5683..4efaa31 100644
> --- a/server/dummy-channel-client.c
> +++ b/server/dummy-channel-client.c
> @@ -104,6 +104,7 @@ dummy_channel_client_init(DummyChannelClient *self)
>  
>  RedChannelClient* dummy_channel_client_create(RedChannel *channel,
>RedClient  *client,
> +  RedsStream *stream,
>int num_common_caps,
>uint32_t *common_caps,
>int num_caps, uint32_t *caps)
> @@ -125,6 +126,7 @@ RedChannelClient* dummy_channel_client_create(RedChannel
> *channel,
>   NULL, NULL,
>   "channel", channel,
>   "client", client,
> + "stream", stream,
>   "caps", caps_array,
>   "common-caps", common_caps_array,
>   NULL);
> diff --git a/server/dummy-channel-client.h b/server/dummy-channel-client.h
> index 8013aa2..54e0e6c 100644
> --- a/server/dummy-channel-client.h
> +++ b/server/dummy-channel-client.h
> @@ -56,6 +56,7 @@ GType dummy_channel_client_get_type(void) G_GNUC_CONST;
>  
>  RedChannelClient *dummy_channel_client_create(RedChannel *channel,
>RedClient  *client,
> +  RedsStream *stream,
>int num_common_caps, uint32_t
>*common_caps,
>int num_caps, uint32_t *caps);
>  
> diff --git a/server/sound.c b/server/sound.c
> index bd9bb66..e1d6805 100644
> --- a/server/sound.c
> +++ b/server/sound.c
> @@ -925,6 +925,8 @@ static void snd_record_send(void* data)
>  }
>  }
>  
> +static int snd_channel_config_socket(RedChannelClient *rcc);
> +
>  static SndChannelClient *__new_channel(SndChannel *channel, int size,
>  uint32_t channel_id,
> RedClient *red_client,
> RedsStream *stream,
> @@ -936,50 +938,8 @@ static SndChannelClient *__new_channel(SndChannel
> *channel, int size, uint32_t c
> uint32_t *caps, int num_caps)
>  {
>  SndChannelClient *client;
> -int delay_val;
> -int flags;
> -#ifdef SO_PRIORITY
> -int priority;
> -#endif
> -int tos;
> -MainChannelClient *mcc = red_client_get_main(red_client);
>  RedsState *reds = red_channel_get_server(RED_CHANNEL(channel));
>  
> -spice_assert(stream);
> -if ((flags = fcntl(stream->socket, F_GETFL)) == -1) {
> -spice_printerr("accept failed, %s", strerror(errno));
> -goto error1;
> -}
> -
> -#ifdef SO_PRIORITY
> -priority = 6;
> -if (setsockopt(stream->socket, SOL_SOCKET, SO_PRIORITY,
> (void*),
> -   sizeof(priority)) == -1) {
> -if (errno != ENOTSUP) {
> -spice_printerr("setsockopt failed, %s", strerror(errno));
> -}
> -}
> -#endif
> -
> -tos = IPTOS_LOWDELAY;
> -if (setsockopt(stream->socket, IPPROTO_IP, IP_TOS, (void*),
> sizeof(tos)) == -1) {
> -if (errno != ENOTSUP) {
> -spice_printerr("setsockopt failed, %s", strerror(errno));
> -}
> -}
> -
> -delay_val = main_channel_client_is_low_bandwidth(mcc) ? 0 : 1;
> -if (setsockopt(stream->socket, IPPROTO_TCP, TCP_NODELAY, _val,
> sizeof(delay_val)) == -1) {
> -if (errno != ENOTSUP) {
> -spice_printerr("setsockopt failed, %s", strerror(errno));
> -}
> -}
> -
> -if (fcntl(stream->socket, F_SETFL, flags | O_NONBLOCK) == -1) {
> -spice_printerr("accept failed, %s", strerror(errno));
> -goto error1;
> -}
> -
>  spice_assert(size >= sizeof(*client));
>  client = spice_malloc0(size);
>  client->refs = 1;
> @@ -1004,24 +964,68 @@ static SndChannelClient *__new_channel(SndChannel
> *channel, int size, uint32_t c
>  client->cleanup = 

Re: [Spice-devel] [spice-server 03/17] sound: Remove extraneous whitespace

2017-01-11 Thread Christophe Fergeau
On Wed, Jan 11, 2017 at 05:50:43AM -0500, Frediano Ziglio wrote:
> > 
> > No need for this whitespace before ';'
> > ---
> >  server/sound.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/server/sound.c b/server/sound.c
> > index f154465..1f6c243 100644
> > --- a/server/sound.c
> > +++ b/server/sound.c
> > @@ -1508,7 +1508,7 @@ static void on_new_record_channel_client(SndChannel
> > *channel, SndChannelClient *
> >  {
> >  spice_assert(client);
> >  
> > -channel->connection = client ;
> > +channel->connection = client;
> >  if (channel->volume.volume_nchannels) {
> >  snd_set_command(client, SND_VOLUME_MASK);
> >  }
> 
> Acked-by: Frediano Ziglio 
> 
> A bit picky I would say.

This was part of that big patch, but did not really fit well anywhere,
so I just split it.

Christophe


signature.asc
Description: PGP signature
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel


Re: [Spice-devel] [spice-server 06/17] sound: Don't dereference pointer before NULL check

2017-01-11 Thread Frediano Ziglio
> 
> Based on a patch from Frediano Ziglio 
> 
> Signed-off-by: Christophe Fergeau 

Acked-by: Frediano Ziglio 

Frediano

> ---
>  server/sound.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/server/sound.c b/server/sound.c
> index 7ebea90..1eab75b 100644
> --- a/server/sound.c
> +++ b/server/sound.c
> @@ -1125,11 +1125,11 @@ SPICE_GNUC_VISIBLE void
> spice_server_playback_start(SpicePlaybackInstance *sin)
>  SPICE_GNUC_VISIBLE void spice_server_playback_stop(SpicePlaybackInstance
>  *sin)
>  {
>  SndChannelClient *client = sin->st->channel.connection;
> -PlaybackChannelClient *playback_client = SPICE_CONTAINEROF(client,
> PlaybackChannelClient, base);
>  
>  sin->st->channel.active = 0;
>  if (!client)
>  return;
> +PlaybackChannelClient *playback_client = SPICE_CONTAINEROF(client,
> PlaybackChannelClient, base);
>  spice_assert(client->active);
>  reds_enable_mm_time(snd_channel_get_server(client));
>  client->active = FALSE;
> @@ -1383,11 +1383,11 @@ SPICE_GNUC_VISIBLE void
> spice_server_record_set_mute(SpiceRecordInstance *sin, u
>  static void snd_record_start(SndChannel *channel)
>  {
>  SndChannelClient *client = channel->connection;
> -RecordChannelClient *record_client = SPICE_CONTAINEROF(client,
> RecordChannelClient, base);
>  
>  channel->active = 1;
>  if (!client)
>  return;
> +RecordChannelClient *record_client = SPICE_CONTAINEROF(client,
> RecordChannelClient, base);
>  spice_assert(!client->active);
>  record_client->read_pos = record_client->write_pos = 0;   //todo:
>  improve by
>//stream
>generation
> @@ -1426,13 +1426,13 @@ SPICE_GNUC_VISIBLE uint32_t
> spice_server_record_get_samples(SpiceRecordInstance
>  uint32_t
>  *samples,
>  uint32_t
>  bufsize)
>  {
>  SndChannelClient *client = sin->st->channel.connection;
> -RecordChannelClient *record_client = SPICE_CONTAINEROF(client,
> RecordChannelClient, base);
>  uint32_t read_pos;
>  uint32_t now;
>  uint32_t len;
>  
>  if (!client)
>  return 0;
> +RecordChannelClient *record_client = SPICE_CONTAINEROF(client,
> RecordChannelClient, base);
>  spice_assert(client->active);
>  
>  if (record_client->write_pos < RECORD_SAMPLES_SIZE / 2) {

___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel


Re: [Spice-devel] [spice-server 05/17] sound: Remove unused __new_channel() argument

2017-01-11 Thread Frediano Ziglio
> 
> It became unused in 26027036c 'red_channel: remove unused migrate flag
> from RedChannel' but was never removed from the function prototype.
> 
> Signed-off-by: Christophe Fergeau 

Acked-by: Frediano Ziglio 

Frediano

> ---
>  server/sound.c | 9 -
>  1 file changed, 4 insertions(+), 5 deletions(-)
> 
> diff --git a/server/sound.c b/server/sound.c
> index d24a0d9..7ebea90 100644
> --- a/server/sound.c
> +++ b/server/sound.c
> @@ -928,7 +928,6 @@ static void snd_record_send(void* data)
>  static SndChannelClient *__new_channel(SndChannel *channel, int size,
>  uint32_t channel_id,
> RedClient *red_client,
> RedsStream *stream,
> -   int migrate,
> snd_channel_send_messages_proc
> send_messages,
> snd_channel_handle_message_proc
> handle_message,
> snd_channel_on_message_done_proc
> on_message_done,
> @@ -1281,7 +1280,8 @@ static void snd_playback_cleanup(SndChannelClient
> *client)
>  }
>  
>  static void snd_set_playback_peer(RedChannel *red_channel, RedClient
>  *client, RedsStream *stream,
> -  int migration, int num_common_caps,
> uint32_t *common_caps,
> +  G_GNUC_UNUSED int migration,
> +  int num_common_caps, uint32_t
> *common_caps,
>int num_caps, uint32_t *caps)
>  {
>  SndChannel *channel = SND_CHANNEL(red_channel);
> @@ -1294,7 +1294,6 @@ static void snd_set_playback_peer(RedChannel
> *red_channel, RedClient *client, Re
> 
> SPICE_CHANNEL_PLAYBACK,
> client,
> stream,
> -
> migration,
> 
> snd_playback_send,
> 
> snd_playback_handle_message,
> 
> snd_playback_on_message_done,
> @@ -1523,7 +1522,8 @@ static void snd_record_cleanup(SndChannelClient
> *client)
>  }
>  
>  static void snd_set_record_peer(RedChannel *red_channel, RedClient *client,
>  RedsStream *stream,
> -int migration, int num_common_caps, uint32_t
> *common_caps,
> +G_GNUC_UNUSED int migration,
> +int num_common_caps, uint32_t *common_caps,
>  int num_caps, uint32_t *caps)
>  {
>  SndChannel *channel = SND_CHANNEL(red_channel);
> @@ -1536,7 +1536,6 @@ static void snd_set_record_peer(RedChannel
> *red_channel, RedClient *client, Reds
> 
> SPICE_CHANNEL_RECORD,
> client,
> stream,
> -   migration,
> 
> snd_record_send,
> 
> snd_record_handle_message,
> 
> snd_record_on_message_done,

___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel


Re: [Spice-devel] [spice-server 04/17] sound: Remove duplicate AudioFrame typedef

2017-01-11 Thread Frediano Ziglio
> 
> It's already defined before in the same source file.
> 
> Based on a patch from Frediano Ziglio 
> 
> Signed-off-by: Christophe Fergeau 
> ---
>  server/sound.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/server/sound.c b/server/sound.c
> index 1f6c243..d24a0d9 100644
> --- a/server/sound.c
> +++ b/server/sound.c
> @@ -123,7 +123,6 @@ struct SndChannelClient {
>  snd_channel_cleanup_channel_proc cleanup;
>  };
>  
> -typedef struct AudioFrame AudioFrame;
>  struct AudioFrame {
>  uint32_t time;
>  uint32_t samples[SND_CODEC_MAX_FRAME_SIZE];

Can I ack my hunk or should I consider your signed-off like
an ack? I usually prefer a sign A + sign B + ack A

Frediano
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel


Re: [Spice-devel] [spice-server 02/17] sound: Remove dummy-channel.[ch]

2017-01-11 Thread Frediano Ziglio
> 
> This is no longer used since "sound: Convert SndChannel to GObject"
> 
> Signed-off-by: Christophe Fergeau 

Didn't notice could be already removed.
However this looks like a partial patch I had in my list, some credit
is missing.
Also the other part of this initial patch ended up merged in your
"sound: Convert SndChannelClient to RedChannelClient" patch,
perhaps for coherence would be better to split also the other patch

Frediano

> ---
>  server/Makefile.am |  2 --
>  server/dummy-channel.c | 94
>  --
>  server/dummy-channel.h | 60 
>  server/sound.c |  1 -
>  4 files changed, 157 deletions(-)
>  delete mode 100644 server/dummy-channel.c
>  delete mode 100644 server/dummy-channel.h
> 
> diff --git a/server/Makefile.am b/server/Makefile.am
> index ae79ed7..90ff779 100644
> --- a/server/Makefile.am
> +++ b/server/Makefile.am
> @@ -104,8 +104,6 @@ libserver_la_SOURCES =\
>   red-channel-client-private.h\
>   red-client.c\
>   red-client.h\
> - dummy-channel.c \
> - dummy-channel.h \
>   dummy-channel-client.c  \
>   dummy-channel-client.h  \
>   red-common.h\
> diff --git a/server/dummy-channel.c b/server/dummy-channel.c
> deleted file mode 100644
> index aecd3a6..000
> --- a/server/dummy-channel.c
> +++ /dev/null
> @@ -1,94 +0,0 @@
> -/*
> -   Copyright (C) 2016 Red Hat, Inc.
> -
> -   This library is free software; you can redistribute it and/or
> -   modify it under the terms of the GNU Lesser General Public
> -   License as published by the Free Software Foundation; either
> -   version 2.1 of the License, or (at your option) any later version.
> -
> -   This library is distributed in the hope that it will be useful,
> -   but WITHOUT ANY WARRANTY; without even the implied warranty of
> -   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> -   Lesser General Public License for more details.
> -
> -   You should have received a copy of the GNU Lesser General Public
> -   License along with this library; if not, see
> .
> -*/
> -#ifdef HAVE_CONFIG_H
> -#include 
> -#endif
> -
> -#include "dummy-channel.h"
> -
> -G_DEFINE_TYPE(DummyChannel, dummy_channel, RED_TYPE_CHANNEL)
> -
> -static int dummy_config_socket(RedChannelClient *self)
> -{
> -return 1;
> -}
> -
> -static void dummy_on_disconnect(RedChannelClient *self)
> -{
> -}
> -
> -static uint8_t* dummy_alloc_recv_buf(RedChannelClient *self,
> - uint16_t type,
> - uint32_t size)
> -{
> -return NULL;
> -}
> -
> -static void dummy_release_recv_buf(RedChannelClient *self,
> -   uint16_t type,
> -   uint32_t size,
> -   uint8_t *msg)
> -{
> -}
> -
> -static void
> -dummy_channel_class_init(DummyChannelClass *klass)
> -{
> -RedChannelClass *channel_class = RED_CHANNEL_CLASS(klass);
> -channel_class->config_socket = dummy_config_socket;
> -channel_class->on_disconnect = dummy_on_disconnect;
> -channel_class->alloc_recv_buf = dummy_alloc_recv_buf;
> -channel_class->release_recv_buf = dummy_release_recv_buf;
> -}
> -
> -static void
> -dummy_channel_init(DummyChannel *self)
> -{
> -}
> -
> -// TODO: red_worker can use this one
> -static void dummy_watch_update_mask(const SpiceCoreInterfaceInternal *iface,
> -SpiceWatch *watch, int event_mask)
> -{
> -}
> -
> -static SpiceWatch *dummy_watch_add(const SpiceCoreInterfaceInternal *iface,
> -   int fd, int event_mask, SpiceWatchFunc
> func, void *opaque)
> -{
> -return NULL; // apparently allowed?
> -}
> -
> -static void dummy_watch_remove(const SpiceCoreInterfaceInternal *iface,
> SpiceWatch *watch)
> -{
> -}
> -
> -// TODO: actually, since I also use channel_client_dummy, no need for core.
> Can be NULL
> -static const SpiceCoreInterfaceInternal dummy_core = {
> -.watch_update_mask = dummy_watch_update_mask,
> -.watch_add = dummy_watch_add,
> -.watch_remove = dummy_watch_remove,
> -};
> -
> -RedChannel *dummy_channel_new(RedsState *reds, uint32_t type, uint32_t id)
> -{
> -return g_object_new(TYPE_DUMMY_CHANNEL,
> -"spice-server", reds,
> -"core-interface", _core,
> -"channel-type", type,
> -"id", id,
> -NULL);
> -}
> diff --git a/server/dummy-channel.h b/server/dummy-channel.h
> deleted file mode 100644
> index 9527633..000
> --- a/server/dummy-channel.h
> +++ /dev/null
> @@ -1,60 +0,0 @@
> -/*
> -   Copyright (C) 

Re: [Spice-devel] [spice-server 03/17] sound: Remove extraneous whitespace

2017-01-11 Thread Frediano Ziglio
> 
> No need for this whitespace before ';'
> ---
>  server/sound.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/server/sound.c b/server/sound.c
> index f154465..1f6c243 100644
> --- a/server/sound.c
> +++ b/server/sound.c
> @@ -1508,7 +1508,7 @@ static void on_new_record_channel_client(SndChannel
> *channel, SndChannelClient *
>  {
>  spice_assert(client);
>  
> -channel->connection = client ;
> +channel->connection = client;
>  if (channel->volume.volume_nchannels) {
>  snd_set_command(client, SND_VOLUME_MASK);
>  }

Acked-by: Frediano Ziglio 

A bit picky I would say.

Frediano
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel


Re: [Spice-devel] [spice-server 01/17] channel: Remove commented out function prototype

2017-01-11 Thread Frediano Ziglio

> This became obsolete when RedChannel became GObject-based.
> 
> Signed-off-by: Christophe Fergeau 
> ---
>  server/red-channel.h | 12 
>  1 file changed, 12 deletions(-)
> 
> diff --git a/server/red-channel.h b/server/red-channel.h
> index 11a4088..f2866f5 100644
> --- a/server/red-channel.h
> +++ b/server/red-channel.h
> @@ -204,18 +204,6 @@ typedef struct RedChannelCapabilities {
>  
>  GType red_channel_get_type(void) G_GNUC_CONST;
>  
> -/* alternative constructor, meant for marshaller based (inputs,main)
> channels,
> - * will become default eventually */
> -/*
> -RedChannel *red_channel_create_parser(int size,
> -  RedsState *reds,
> -  const SpiceCoreInterfaceInternal
> *core,
> -  uint32_t type, uint32_t id,
> -  gboolean handle_acks,
> -  spice_parse_channel_func_t parser,
> -  channel_handle_parsed_proc
> handle_parsed,
> -  uint32_t migration_flags);
> -  */
>  void red_channel_add_client(RedChannel *channel, RedChannelClient *rcc);
>  void red_channel_remove_client(RedChannel *channel, RedChannelClient *rcc);
>  

OT but 

Acked-by: Frediano Ziglio 

Frediano
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel


[Spice-devel] [spice-gtk v2 3/3] ssl: Use accessors rather than direct struct access

2017-01-11 Thread Christophe Fergeau
From: Sebastian Andrzej Siewior 

In OpenSSL 1.1.0, the struct fields are private so we can no longer
directly access them.

The accessors are not available in previous OpenSSL releases, so we need
to add compat helpers.
---
 src/bio-gio.c   | 108 
 src/spice-channel.c |  11 +-
 2 files changed, 102 insertions(+), 17 deletions(-)

diff --git a/src/bio-gio.c b/src/bio-gio.c
index 0f8b415..ef47cd3 100644
--- a/src/bio-gio.c
+++ b/src/bio-gio.c
@@ -23,6 +23,75 @@
 #include "spice-util.h"
 #include "bio-gio.h"
 
+#if OPENSSL_VERSION_NUMBER < 0x1010
+static BIO_METHOD one_static_bio;
+
+static int BIO_meth_set_read(BIO_METHOD *biom,
+ int (*bread) (BIO *, char *, int))
+{
+biom->bread = bread;
+return 1;
+}
+
+static int BIO_meth_set_write(BIO_METHOD *biom,
+  int (*bwrite) (BIO *, const char *, int))
+{
+biom->bwrite = bwrite;
+return 1;
+}
+
+static int BIO_meth_set_puts(BIO_METHOD *biom,
+ int (*bputs) (BIO *, const char *))
+{
+biom->bputs = bputs;
+return 1;
+}
+
+static int BIO_meth_set_ctrl(BIO_METHOD *biom,
+ long (*ctrl) (BIO *, int, long, void *))
+{
+biom->ctrl = ctrl;
+return 1;
+}
+
+#define BIO_TYPE_START 128
+
+static int BIO_get_new_index(void)
+{
+static int bio_index = BIO_TYPE_START;
+return bio_index++;
+}
+
+static void BIO_set_init(BIO *a, int init)
+{
+   a->init = init;
+}
+
+static void BIO_set_data(BIO *a, void *ptr)
+{
+a->ptr = ptr;
+}
+
+static void *BIO_get_data(BIO *a)
+{
+return a->ptr;
+}
+
+static BIO_METHOD *BIO_meth_new(int type, const char *name)
+{
+BIO_METHOD *biom = _static_bio;
+
+biom->type = type;
+biom->name = name;
+return biom;
+}
+
+static void BIO_meth_free(BIO_METHOD *biom)
+{
+}
+
+#endif
+
 static long bio_gio_ctrl(G_GNUC_UNUSED BIO *b,
  int cmd,
  G_GNUC_UNUSED long num,
@@ -37,7 +106,7 @@ static int bio_gio_write(BIO *bio, const char *in, int inl)
 gssize ret;
 GError *error = NULL;
 
-stream = g_io_stream_get_output_stream(bio->ptr);
+stream = g_io_stream_get_output_stream(BIO_get_data(bio));
 ret = 
g_pollable_output_stream_write_nonblocking(G_POLLABLE_OUTPUT_STREAM(stream),
  in, inl, NULL, );
 BIO_clear_retry_flags(bio);
@@ -58,7 +127,7 @@ static int bio_gio_read(BIO *bio, char *out, int outl)
 gssize ret;
 GError *error = NULL;
 
-stream = g_io_stream_get_input_stream(bio->ptr);
+stream = g_io_stream_get_input_stream(BIO_get_data(bio));
 ret = 
g_pollable_input_stream_read_nonblocking(G_POLLABLE_INPUT_STREAM(stream),
out, outl, NULL, );
 BIO_clear_retry_flags(bio);
@@ -83,30 +152,37 @@ static int bio_gio_puts(BIO *bio, const char *str)
 return ret;
 }
 
-#define BIO_TYPE_START 128
+static BIO_METHOD *bio_gio_method;
 
 G_GNUC_INTERNAL
 BIO* bio_new_giostream(GIOStream *stream)
 {
 BIO *bio;
-static BIO_METHOD bio_gio_method;
 
-if (bio_gio_method.name == NULL) {
-bio_gio_method.type = BIO_TYPE_START | BIO_TYPE_SOURCE_SINK;
-bio_gio_method.name = "gio stream";
+if (!bio_gio_method) {
+int index = BIO_get_new_index();
+g_warning("index: %i", index);
+bio_gio_method = BIO_meth_new(BIO_get_new_index() |
+  BIO_TYPE_SOURCE_SINK,
+  "gio stream");
+if (!bio_gio_method)
+return NULL;
+
+if (!BIO_meth_set_write(bio_gio_method, bio_gio_write) ||
+!BIO_meth_set_read(bio_gio_method, bio_gio_read) ||
+!BIO_meth_set_puts(bio_gio_method, bio_gio_puts) ||
+!BIO_meth_set_ctrl(bio_gio_method, bio_gio_ctrl)) {
+BIO_meth_free(bio_gio_method);
+bio_gio_method = NULL;
+return NULL;
+}
 }
 
-bio = BIO_new(_gio_method);
+bio = BIO_new(bio_gio_method);
 if (!bio)
 return NULL;
 
-bio->method->bwrite = bio_gio_write;
-bio->method->bread = bio_gio_read;
-bio->method->bputs = bio_gio_puts;
-bio->method->ctrl = bio_gio_ctrl;
-
-bio->init = 1;
-bio->ptr = stream;
-
+BIO_set_init(bio, 1);
+BIO_set_data(bio, stream);
 return bio;
 }
diff --git a/src/spice-channel.c b/src/spice-channel.c
index 6a911a6..6556db3 100644
--- a/src/spice-channel.c
+++ b/src/spice-channel.c
@@ -55,6 +55,15 @@ static void spice_channel_reset_capabilities(SpiceChannel 
*channel);
 static void spice_channel_send_migration_handshake(SpiceChannel *channel);
 static gboolean channel_connect(SpiceChannel *channel, gboolean tls);
 
+#if OPENSSL_VERSION_NUMBER < 0x1010
+static RSA *EVP_PKEY_get0_RSA(EVP_PKEY *pkey)
+{
+if (pkey->type != EVP_PKEY_RSA) {

[Spice-devel] [spice-gtk v2 0/3] ssl: Add support for OpenSSL 1.1.0

2017-01-11 Thread Christophe Fergeau
Sebastian sent these patches privately a while ago, I've run some tests on them
and helped split them. They add support for OpenSSL 1.1.0 which makes some of 
the
structures we were directly accessing private. This also keeps support with 
older
OpenSSL releases by adding some compat helpers.

These patches have been tested against a RHEV instance, and against manually 
configured
QEMU+SPICE+TLS.

This second version of the series addresses the review comments made by Pavel.

Christophe

Sebastian Andrzej Siewior (3):
  ssl: Stop creating our own X509_LOOKUP_METHOD
  ssl: Rework our custom BIO type
  ssl: Use accessors rather than direct struct access

 src/bio-gio.c   | 139 
 src/spice-channel.c |  24 +
 2 files changed, 121 insertions(+), 42 deletions(-)

-- 
2.9.3

___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel


[Spice-devel] [spice-gtk v2 1/3] ssl: Stop creating our own X509_LOOKUP_METHOD

2017-01-11 Thread Christophe Fergeau
From: Sebastian Andrzej Siewior 

OpenSSL 1.1.0 does not seem to provide API to do that anymore.

There is no need to create a custom lookup to begin with. This method
here has no callbacks implemented and is doing nothing. The way I
understand it, it is used to retrieve a `lookup' object which provides a
certificate store.  The SSL ctx provides also such a store.

Acked-by: Christophe Fergeau 
Acked-by: Pavel Grunt 
---
 src/spice-channel.c | 13 -
 1 file changed, 4 insertions(+), 9 deletions(-)

diff --git a/src/spice-channel.c b/src/spice-channel.c
index 95662f3..6a911a6 100644
--- a/src/spice-channel.c
+++ b/src/spice-channel.c
@@ -2352,17 +2352,12 @@ static gboolean spice_channel_delayed_unref(gpointer 
data)
 return FALSE;
 }
 
-static X509_LOOKUP_METHOD spice_x509_mem_lookup = {
-"spice_x509_mem_lookup",
-0
-};
-
 static int spice_channel_load_ca(SpiceChannel *channel)
 {
 SpiceChannelPrivate *c = channel->priv;
 STACK_OF(X509_INFO) *inf;
 X509_INFO *itmp;
-X509_LOOKUP *lookup;
+X509_STORE *store;
 BIO *in;
 int i, count = 0;
 guint8 *ca;
@@ -2372,13 +2367,13 @@ static int spice_channel_load_ca(SpiceChannel *channel)
 
 g_return_val_if_fail(c->ctx != NULL, 0);
 
-lookup = X509_STORE_add_lookup(c->ctx->cert_store, _x509_mem_lookup);
 ca_file = spice_session_get_ca_file(c->session);
 spice_session_get_ca(c->session, , );
 
 CHANNEL_DEBUG(channel, "Load CA, file: %s, data: %p", ca_file, ca);
 
 if (ca != NULL) {
+store = SSL_CTX_get_cert_store(c->ctx);
 in = BIO_new_mem_buf(ca, size);
 inf = PEM_X509_INFO_read_bio(in, NULL, NULL, NULL);
 BIO_free(in);
@@ -2386,11 +2381,11 @@ static int spice_channel_load_ca(SpiceChannel *channel)
 for (i = 0; i < sk_X509_INFO_num(inf); i++) {
 itmp = sk_X509_INFO_value(inf, i);
 if (itmp->x509) {
-X509_STORE_add_cert(lookup->store_ctx, itmp->x509);
+X509_STORE_add_cert(store, itmp->x509);
 count++;
 }
 if (itmp->crl) {
-X509_STORE_add_crl(lookup->store_ctx, itmp->crl);
+X509_STORE_add_crl(store, itmp->crl);
 count++;
 }
 }
-- 
2.9.3

___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel


[Spice-devel] [spice-gtk v2 2/3] ssl: Rework our custom BIO type

2017-01-11 Thread Christophe Fergeau
From: Sebastian Andrzej Siewior 

This commit changes to an actual new BIO method rather than reusing an
existing BIO method, and overriding only the fields that we need.
The approach before this commit would be causing issues with OpenSSL
1.1.0 as some of the fields we access have become opaque.
---
 src/bio-gio.c | 57 -
 1 file changed, 28 insertions(+), 29 deletions(-)

diff --git a/src/bio-gio.c b/src/bio-gio.c
index b310c97..0f8b415 100644
--- a/src/bio-gio.c
+++ b/src/bio-gio.c
@@ -23,21 +23,22 @@
 #include "spice-util.h"
 #include "bio-gio.h"
 
-typedef struct bio_gsocket_method {
-BIO_METHOD method;
-GIOStream *stream;
-} bio_gsocket_method;
-
-#define BIO_GET_GSOCKET(bio)  (((bio_gsocket_method*)bio->method)->gsocket)
-#define BIO_GET_ISTREAM(bio)  
(g_io_stream_get_input_stream(((bio_gsocket_method*)bio->method)->stream))
-#define BIO_GET_OSTREAM(bio)  
(g_io_stream_get_output_stream(((bio_gsocket_method*)bio->method)->stream))
+static long bio_gio_ctrl(G_GNUC_UNUSED BIO *b,
+ int cmd,
+ G_GNUC_UNUSED long num,
+ G_GNUC_UNUSED void *ptr)
+{
+return (cmd == BIO_CTRL_FLUSH);
+}
 
 static int bio_gio_write(BIO *bio, const char *in, int inl)
 {
+GOutputStream *stream;
 gssize ret;
 GError *error = NULL;
 
-ret = 
g_pollable_output_stream_write_nonblocking(G_POLLABLE_OUTPUT_STREAM(BIO_GET_OSTREAM(bio)),
+stream = g_io_stream_get_output_stream(bio->ptr);
+ret = 
g_pollable_output_stream_write_nonblocking(G_POLLABLE_OUTPUT_STREAM(stream),
  in, inl, NULL, );
 BIO_clear_retry_flags(bio);
 
@@ -53,10 +54,12 @@ static int bio_gio_write(BIO *bio, const char *in, int inl)
 
 static int bio_gio_read(BIO *bio, char *out, int outl)
 {
+GInputStream *stream;
 gssize ret;
 GError *error = NULL;
 
-ret = 
g_pollable_input_stream_read_nonblocking(G_POLLABLE_INPUT_STREAM(BIO_GET_ISTREAM(bio)),
+stream = g_io_stream_get_input_stream(bio->ptr);
+ret = 
g_pollable_input_stream_read_nonblocking(G_POLLABLE_INPUT_STREAM(stream),
out, outl, NULL, );
 BIO_clear_retry_flags(bio);
 
@@ -70,17 +73,6 @@ static int bio_gio_read(BIO *bio, char *out, int outl)
 return ret;
 }
 
-static int bio_gio_destroy(BIO *bio)
-{
-if (bio == NULL || bio->method == NULL)
-return 0;
-
-SPICE_DEBUG("bio gsocket destroy");
-g_clear_pointer(>method, g_free);
-
-return 1;
-}
-
 static int bio_gio_puts(BIO *bio, const char *str)
 {
 int n, ret;
@@ -91,23 +83,30 @@ static int bio_gio_puts(BIO *bio, const char *str)
 return ret;
 }
 
+#define BIO_TYPE_START 128
+
 G_GNUC_INTERNAL
 BIO* bio_new_giostream(GIOStream *stream)
 {
-// TODO: make an actual new BIO type, or just switch to GTls already...
-BIO *bio = BIO_new_socket(-1, BIO_NOCLOSE);
+BIO *bio;
+static BIO_METHOD bio_gio_method;
 
-bio_gsocket_method *bio_method = g_new(bio_gsocket_method, 1);
-bio_method->method = *bio->method;
-bio_method->stream = stream;
+if (bio_gio_method.name == NULL) {
+bio_gio_method.type = BIO_TYPE_START | BIO_TYPE_SOURCE_SINK;
+bio_gio_method.name = "gio stream";
+}
 
-bio->method->destroy(bio);
-bio->method = (BIO_METHOD*)bio_method;
+bio = BIO_new(_gio_method);
+if (!bio)
+return NULL;
 
 bio->method->bwrite = bio_gio_write;
 bio->method->bread = bio_gio_read;
 bio->method->bputs = bio_gio_puts;
-bio->method->destroy = bio_gio_destroy;
+bio->method->ctrl = bio_gio_ctrl;
+
+bio->init = 1;
+bio->ptr = stream;
 
 return bio;
 }
-- 
2.9.3

___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel


Re: [Spice-devel] [spice-gtk 3/3] ssl: Use accessors rather than direct struct access

2017-01-11 Thread Pavel Grunt
On Wed, 2017-01-11 at 10:15 +0100, Christophe Fergeau wrote:
> On Mon, Jan 09, 2017 at 03:38:43PM +0100, Pavel Grunt wrote:
> > > +static int BIO_get_new_index(void)
> > > +{
> > > +return 128;
> > 
> > looking at openssl implementation
> > https://github.com/openssl/openssl/commit/8b8d963db5bb619fbada014f
> > 294f
> > d09a855a2650
> > 
> > it uses BIO_TYPE_START and increments when called
> > 
> 
> Are you suggesting that a constant and incremented static are
> introduced? Or is it fine to just keep things as they are now?

I am suggesting to introduce that, I would always think why "128" and
why is the new index same as the previous.

diff --git a/src/bio-gio.c b/src/bio-gio.c
index 1c8d267..6a9637c 100644
--- a/src/bio-gio.c
+++ b/src/bio-gio.c
@@ -54,9 +54,10 @@ static int BIO_meth_set_ctrl(BIO_METHOD *biom,
 return 1;
 }
 
+static int bio_index = BIO_TYPE_START;
 static int BIO_get_new_index(void)
 {
-return 128;
+return ++bio_index;
 }
 
 static void BIO_set_init(BIO *a, int init)




Pavel
> 
> Christophe
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel


Re: [Spice-devel] [spice-gtk 3/3] ssl: Use accessors rather than direct struct access

2017-01-11 Thread Christophe Fergeau
On Mon, Jan 09, 2017 at 03:38:43PM +0100, Pavel Grunt wrote:
> > +static int BIO_get_new_index(void)
> > +{
> > +return 128;
> 
> looking at openssl implementation
> https://github.com/openssl/openssl/commit/8b8d963db5bb619fbada014f294f
> d09a855a2650
> 
> it uses BIO_TYPE_START and increments when called
> 

Are you suggesting that a constant and incremented static are
introduced? Or is it fine to just keep things as they are now?

Christophe


signature.asc
Description: PGP signature
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel


[Spice-devel] [spice-server 09/17] sound: Add separate SND_MUTE_MASK

2017-01-11 Thread Christophe Fergeau
Currently MUTE and VOLUME commands use the same VOLUME mask. This commit
introduces a separate SND_MUTE_MASK for MUTE commands to make things
a bit more clear.

Based on a patch from Frediano Ziglio 

Signed-off-by: Christophe Fergeau 
---
 server/sound.c | 25 +++--
 1 file changed, 19 insertions(+), 6 deletions(-)

diff --git a/server/sound.c b/server/sound.c
index e1d6805..7941ea2 100644
--- a/server/sound.c
+++ b/server/sound.c
@@ -55,6 +55,7 @@ enum SndCommand {
 SND_MIGRATE,
 SND_CTRL,
 SND_VOLUME,
+SND_MUTE,
 SND_END_COMMAND,
 };
 
@@ -67,6 +68,8 @@ enum PlaybackCommand {
 #define SND_MIGRATE_MASK (1 << SND_MIGRATE)
 #define SND_CTRL_MASK (1 << SND_CTRL)
 #define SND_VOLUME_MASK (1 << SND_VOLUME)
+#define SND_MUTE_MASK (1 << SND_MUTE)
+#define SND_VOLUME_MUTE_MASK (SND_VOLUME_MASK|SND_MUTE_MASK)
 
 #define SND_PLAYBACK_MODE_MASK (1 << SND_PLAYBACK_MODE)
 #define SND_PLAYBACK_PCM_MASK (1 << SND_PLAYBACK_PCM)
@@ -872,12 +875,17 @@ static void snd_playback_send(void* data)
 client->command &= ~SND_CTRL_MASK;
 }
 if (client->command & SND_VOLUME_MASK) {
-if (!snd_playback_send_volume(playback_client) ||
-!snd_playback_send_mute(playback_client)) {
+if (!snd_playback_send_volume(playback_client)) {
 return;
 }
 client->command &= ~SND_VOLUME_MASK;
 }
+if (client->command & SND_MUTE_MASK) {
+if (!snd_playback_send_mute(playback_client)) {
+return;
+}
+client->command &= ~SND_MUTE_MASK;
+}
 if (client->command & SND_MIGRATE_MASK) {
 if (!snd_playback_send_migrate(playback_client)) {
 return;
@@ -910,12 +918,17 @@ static void snd_record_send(void* data)
 client->command &= ~SND_CTRL_MASK;
 }
 if (client->command & SND_VOLUME_MASK) {
-if (!snd_record_send_volume(record_client) ||
-!snd_record_send_mute(record_client)) {
+if (!snd_record_send_volume(record_client)) {
 return;
 }
 client->command &= ~SND_VOLUME_MASK;
 }
+if (client->command & SND_MUTE_MASK) {
+if (!snd_record_send_mute(record_client)) {
+return;
+}
+client->command &= ~SND_MUTE_MASK;
+}
 if (client->command & SND_MIGRATE_MASK) {
 if (!snd_record_send_migrate(record_client)) {
 return;
@@ -1259,7 +1272,7 @@ static void on_new_playback_channel_client(SndChannel 
*channel, SndChannelClient
 snd_set_command(client, SND_CTRL_MASK);
 }
 if (channel->volume.volume_nchannels) {
-snd_set_command(client, SND_VOLUME_MASK);
+snd_set_command(client, SND_VOLUME_MUTE_MASK);
 }
 if (client->active) {
 reds_disable_mm_time(reds);
@@ -1515,7 +1528,7 @@ static void on_new_record_channel_client(SndChannel 
*channel, SndChannelClient *
 
 channel->connection = client;
 if (channel->volume.volume_nchannels) {
-snd_set_command(client, SND_VOLUME_MASK);
+snd_set_command(client, SND_VOLUME_MUTE_MASK);
 }
 if (client->active) {
 snd_set_command(client, SND_CTRL_MASK);
-- 
2.9.3

___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel


[Spice-devel] [spice-server 01/17] channel: Remove commented out function prototype

2017-01-11 Thread Christophe Fergeau
This became obsolete when RedChannel became GObject-based.

Signed-off-by: Christophe Fergeau 
---
 server/red-channel.h | 12 
 1 file changed, 12 deletions(-)

diff --git a/server/red-channel.h b/server/red-channel.h
index 11a4088..f2866f5 100644
--- a/server/red-channel.h
+++ b/server/red-channel.h
@@ -204,18 +204,6 @@ typedef struct RedChannelCapabilities {
 
 GType red_channel_get_type(void) G_GNUC_CONST;
 
-/* alternative constructor, meant for marshaller based (inputs,main) channels,
- * will become default eventually */
-/*
-RedChannel *red_channel_create_parser(int size,
-  RedsState *reds,
-  const SpiceCoreInterfaceInternal *core,
-  uint32_t type, uint32_t id,
-  gboolean handle_acks,
-  spice_parse_channel_func_t parser,
-  channel_handle_parsed_proc handle_parsed,
-  uint32_t migration_flags);
-  */
 void red_channel_add_client(RedChannel *channel, RedChannelClient *rcc);
 void red_channel_remove_client(RedChannel *channel, RedChannelClient *rcc);
 
-- 
2.9.3

___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel


[Spice-devel] [spice-server 14/17] sound: Remove code from spice_server_record_get_samples()

2017-01-11 Thread Christophe Fergeau
It's ok to remove it because ?

Based on a patch from Frediano Ziglio 

Signed-off-by: Christophe Fergeau 
---
Not sure what to say in this commit log either.


 server/sound.c | 9 -
 1 file changed, 9 deletions(-)

diff --git a/server/sound.c b/server/sound.c
index 1a5f70c..434350a 100644
--- a/server/sound.c
+++ b/server/sound.c
@@ -1469,15 +1469,6 @@ SPICE_GNUC_VISIBLE uint32_t 
spice_server_record_get_samples(SpiceRecordInstance
 
 len = MIN(record_client->write_pos - record_client->read_pos, bufsize);
 
-if (len < bufsize) {
-SndChannel *channel = 
SND_CHANNEL(red_channel_client_get_channel(client->channel_client));
-snd_receive(client);
-if (!channel->connection) {
-return 0;
-}
-len = MIN(record_client->write_pos - record_client->read_pos, bufsize);
-}
-
 read_pos = record_client->read_pos % RECORD_SAMPLES_SIZE;
 record_client->read_pos += len;
 now = MIN(len, RECORD_SAMPLES_SIZE - read_pos);
-- 
2.9.3

___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel


[Spice-devel] [spice-server 15/17] sound: Use RedChannelClient to receive/send data

2017-01-11 Thread Christophe Fergeau
You can see that SndChannelClient has much less field
as the code to read/write from/to client is reused from
RedChannelClient instead of creating a fake RedChannelClient
just to make the system happy.

One of the different between the old sound code and all other
RedChannelClient objects was that the sound channel don't use
a queue while RedChannelClient use RedPipeItem object. This was
the main reason why RedChannelClient was not used. To implement
the old behaviour a "persistent_pipe_item" is used. This RedPipeItem
will be queued to RedChannelClient (only one!) so signal code we
have data to send. The {playback,record}_channel_send_item will
then send the messages to the client using RedChannelClient functions.
For this reason snd_reset_send_data is replaced by a call to
red_channel_client_init_send_data and snd_begin_send_message is
replaced by red_channel_client_begin_send_message.

Signed-off-by: Frediano Ziglio 
Signed-off-by: Christophe Fergeau 
---

This is one of the big changes this series introduces. I have the feeling that
it might be possible to make something smaller
({playback,record}_channel_send_item are very similar to
snd_{playback,record}_send), but I had already spent too much time splitting 
this,
and it was not obvious to me if this was possible to split this too.



 server/sound.c | 663 ++---
 1 file changed, 249 insertions(+), 414 deletions(-)

diff --git a/server/sound.c b/server/sound.c
index 434350a..0992cf4 100644
--- a/server/sound.c
+++ b/server/sound.c
@@ -82,8 +82,6 @@ typedef struct AudioFrameContainer AudioFrameContainer;
 typedef struct SpicePlaybackState PlaybackChannel;
 typedef struct SpiceRecordState RecordChannel;
 
-typedef void (*snd_channel_send_messages_proc)(void *in_channel);
-typedef int (*snd_channel_handle_message_proc)(SndChannelClient *client, 
size_t size, uint32_t type, void *message);
 typedef void (*snd_channel_on_message_done_proc)(SndChannelClient *client);
 typedef void (*snd_channel_cleanup_channel_proc)(SndChannelClient *client);
 
@@ -91,37 +89,30 @@ typedef void 
(*snd_channel_cleanup_channel_proc)(SndChannelClient *client);
 
 /* Connects an audio client to a Spice client */
 struct SndChannelClient {
-RedsStream *stream;
-spice_parse_channel_func_t parser;
 int refs;
 
 RedChannelClient *channel_client;
 
 int active;
 int client_active;
-int blocked;
 
 uint32_t command;
 
-struct {
-uint64_t serial;
-uint32_t size;
-uint32_t pos;
-} send_data;
+/* we don't expect very big messages so don't allocate too much
+ * bytes, data will be cached in RecordChannelClient::samples */
+uint8_t receive_buf[SND_CODEC_MAX_FRAME_BYTES + 64];
+RedPipeItem persistent_pipe_item;
 
-struct {
-uint8_t buf[SND_RECEIVE_BUF_SIZE];
-uint8_t *message_start;
-uint8_t *now;
-uint8_t *end;
-} receive_data;
-
-snd_channel_send_messages_proc send_messages;
-snd_channel_handle_message_proc handle_message;
 snd_channel_on_message_done_proc on_message_done;
 snd_channel_cleanup_channel_proc cleanup;
 };
 
+
+enum {
+RED_PIPE_ITEM_PERSISTENT = RED_PIPE_ITEM_TYPE_CHANNEL_BASE,
+};
+
+
 struct AudioFrame {
 uint32_t time;
 uint32_t samples[SND_CODEC_MAX_FRAME_SIZE];
@@ -226,10 +217,10 @@ struct RecordChannelClient {
 /* A list of all Spice{Playback,Record}State objects */
 static SndChannel *snd_channels;
 
-static void snd_receive(SndChannelClient *client);
 static void snd_playback_start(SndChannel *channel);
 static void snd_record_start(SndChannel *channel);
 static void snd_playback_alloc_frames(PlaybackChannelClient *playback);
+static void snd_send(SndChannelClient * client);
 
 static SndChannelClient *snd_channel_unref(SndChannelClient *client)
 {
@@ -241,6 +232,16 @@ static SndChannelClient 
*snd_channel_unref(SndChannelClient *client)
 return client;
 }
 
+static SndChannelClient *snd_channel_client_from_dummy(RedChannelClient *dummy)
+{
+SndChannelClient *sound_client;
+
+g_assert(IS_DUMMY_CHANNEL_CLIENT(dummy));
+sound_client =  g_object_get_data(G_OBJECT(dummy), "sound-channel-client");
+
+return sound_client;
+}
+
 static RedsState* snd_channel_get_server(SndChannelClient *client)
 {
 g_return_val_if_fail(client != NULL, NULL);
@@ -250,16 +251,14 @@ static RedsState* snd_channel_get_server(SndChannelClient 
*client)
 static void snd_disconnect_channel(SndChannelClient *client)
 {
 SndChannel *channel;
-RedsState *reds;
 RedChannel *red_channel;
 uint32_t type;
 
-if (!client || !client->stream) {
+if (!client || !red_channel_client_is_connected(client->channel_client)) {
 spice_debug("not connected");
 return;
 }
 red_channel = red_channel_client_get_channel(client->channel_client);
-reds = snd_channel_get_server(client);
 

[Spice-devel] [spice-server 07/17] sound: Rework spice_server_playback_get_buffer() error handling

2017-01-11 Thread Christophe Fergeau
Missing detailed log...

Based on a patch from Frediano Ziglio 

Signed-off-by: Christophe Fergeau 
---
As the log indicates, a better log would be needed for this one.

Christophe


 server/sound.c | 11 +++
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/server/sound.c b/server/sound.c
index 1eab75b..bd9bb66 100644
--- a/server/sound.c
+++ b/server/sound.c
@@ -1153,11 +1153,14 @@ SPICE_GNUC_VISIBLE void 
spice_server_playback_get_buffer(SpicePlaybackInstance *
  uint32_t **frame, 
uint32_t *num_samples)
 {
 SndChannelClient *client = sin->st->channel.connection;
-PlaybackChannelClient *playback_client = SPICE_CONTAINEROF(client, 
PlaybackChannelClient, base);
 
-if (!client || !playback_client->free_frames) {
-*frame = NULL;
-*num_samples = 0;
+*frame = NULL;
+*num_samples = 0;
+if (!client) {
+return;
+}
+PlaybackChannelClient *playback_client = SPICE_CONTAINEROF(client, 
PlaybackChannelClient, base);
+if (!playback_client->free_frames) {
 return;
 }
 spice_assert(client->active);
-- 
2.9.3

___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel


[Spice-devel] [spice-server 13/17] sound: Remove SndChannelClient::channel

2017-01-11 Thread Christophe Fergeau
We can get it from our DummyChannelClient rather than storing it in
SndChannelClient.

Based on a patch from Frediano Ziglio 

Signed-off-by: Christophe Fergeau 
---
 server/sound.c | 21 +++--
 1 file changed, 11 insertions(+), 10 deletions(-)

diff --git a/server/sound.c b/server/sound.c
index 45594fb..1a5f70c 100644
--- a/server/sound.c
+++ b/server/sound.c
@@ -92,7 +92,6 @@ typedef void 
(*snd_channel_cleanup_channel_proc)(SndChannelClient *client);
 /* Connects an audio client to a Spice client */
 struct SndChannelClient {
 RedsStream *stream;
-SndChannel *channel;
 spice_parse_channel_func_t parser;
 int refs;
 
@@ -245,7 +244,7 @@ static SndChannelClient *snd_channel_unref(SndChannelClient 
*client)
 static RedsState* snd_channel_get_server(SndChannelClient *client)
 {
 g_return_val_if_fail(client != NULL, NULL);
-return red_channel_get_server(RED_CHANNEL(client->channel));
+return 
red_channel_get_server(red_channel_client_get_channel(client->channel_client));
 }
 
 static void snd_disconnect_channel(SndChannelClient *client)
@@ -264,7 +263,7 @@ static void snd_disconnect_channel(SndChannelClient *client)
 g_object_get(red_channel, "channel-type", , NULL);
 spice_debug("SndChannelClient=%p rcc=%p type=%d",
  client, client->channel_client, type);
-channel = client->channel;
+channel = SND_CHANNEL(red_channel);
 client->cleanup(client);
 red_channel_client_disconnect(channel->connection->channel_client);
 channel->connection->channel_client = NULL;
@@ -420,6 +419,7 @@ static int snd_playback_handle_message(SndChannelClient 
*client, size_t size, ui
 static int snd_record_handle_message(SndChannelClient *client, size_t size, 
uint32_t type, void *message)
 {
 RecordChannelClient *record_client = (RecordChannelClient *)client;
+RedChannelClient *rcc = client->channel_client;
 
 if (!client) {
 return FALSE;
@@ -429,7 +429,7 @@ static int snd_record_handle_message(SndChannelClient 
*client, size_t size, uint
 return snd_record_handle_write(record_client, size, message);
 case SPICE_MSGC_RECORD_MODE: {
 SpiceMsgcRecordMode *mode = (SpiceMsgcRecordMode *)message;
-SndChannel *channel = client->channel;
+SndChannel *channel = SND_CHANNEL(red_channel_client_get_channel(rcc));
 record_client->mode_time = mode->time;
 if (mode->mode != SPICE_AUDIO_DATA_MODE_RAW) {
 if (snd_codec_is_capable(mode->mode, channel->frequency)) {
@@ -622,7 +622,8 @@ static int snd_send_volume(SndChannelClient *client, 
uint32_t cap, int msg)
 uint8_t c;
 RedChannelClient *rcc = client->channel_client;
 SpiceMarshaller *m = red_channel_client_get_marshaller(rcc);
-SpiceVolumeState *st = >channel->volume;
+SndChannel *channel = SND_CHANNEL(red_channel_client_get_channel(rcc));
+SpiceVolumeState *st = >volume;
 
 if (!red_channel_client_test_remote_cap(client->channel_client, cap)) {
 return TRUE;
@@ -653,7 +654,8 @@ static int snd_send_mute(SndChannelClient *client, uint32_t 
cap, int msg)
 SpiceMsgAudioMute mute;
 RedChannelClient *rcc = client->channel_client;
 SpiceMarshaller *m = red_channel_client_get_marshaller(rcc);
-SpiceVolumeState *st = >channel->volume;
+SndChannel *channel = SND_CHANNEL(red_channel_client_get_channel(rcc));
+SpiceVolumeState *st = >volume;
 
 if (!red_channel_client_test_remote_cap(client->channel_client, cap)) {
 return TRUE;
@@ -702,7 +704,7 @@ static int snd_playback_send_start(PlaybackChannelClient 
*playback_client)
 }
 
 start.channels = SPICE_INTERFACE_PLAYBACK_CHAN;
-start.frequency = client->channel->frequency;
+start.frequency = 
SND_CHANNEL(red_channel_client_get_channel(rcc))->frequency;
 spice_assert(SPICE_INTERFACE_PLAYBACK_FMT == 
SPICE_INTERFACE_AUDIO_FMT_S16);
 start.format = SPICE_AUDIO_FMT_S16;
 start.time = reds_get_mm_time();
@@ -745,7 +747,7 @@ static int snd_record_send_start(RecordChannelClient 
*record_client)
 }
 
 start.channels = SPICE_INTERFACE_RECORD_CHAN;
-start.frequency = client->channel->frequency;
+start.frequency = 
SND_CHANNEL(red_channel_client_get_channel(rcc))->frequency;
 spice_assert(SPICE_INTERFACE_RECORD_FMT == SPICE_INTERFACE_AUDIO_FMT_S16);
 start.format = SPICE_AUDIO_FMT_S16;
 spice_marshall_msg_record_start(m, );
@@ -970,7 +972,6 @@ static SndChannelClient *__new_channel(SndChannel *channel, 
int size, uint32_t c
 client->refs = 1;
 client->parser = spice_get_client_channel_parser(channel_id, NULL);
 client->stream = stream;
-client->channel = channel;
 client->receive_data.message_start = client->receive_data.buf;
 client->receive_data.now = client->receive_data.buf;
 client->receive_data.end = client->receive_data.buf + 
sizeof(client->receive_data.buf);
@@ -1469,7 +1470,7 @@ 

[Spice-devel] [spice-server 00/17] Convert SndChannelClient to GObject

2017-01-11 Thread Christophe Fergeau
Hey,

This series is an attempt at splitting "sound: Convert SndChannelClient to 
GObject"
https://lists.freedesktop.org/archives/spice-devel/2017-January/034773.html
in much smaller chunks to ease reviews, and make git history more usable.
This might be a bit artificial at times (especially the conversion to GObject
before converting to RedChannelClient), but in my opinion most of the patches
make sense on their own.
The series should build and audio should still be functional with each of these
patches, though I've only tested playback, not recording.

A few patches would need slightly better logs, Frediano, it would be nice if
you could help with that.

Christophe

___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel


[Spice-devel] [spice-server 04/17] sound: Remove duplicate AudioFrame typedef

2017-01-11 Thread Christophe Fergeau
It's already defined before in the same source file.

Based on a patch from Frediano Ziglio 

Signed-off-by: Christophe Fergeau 
---
 server/sound.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/server/sound.c b/server/sound.c
index 1f6c243..d24a0d9 100644
--- a/server/sound.c
+++ b/server/sound.c
@@ -123,7 +123,6 @@ struct SndChannelClient {
 snd_channel_cleanup_channel_proc cleanup;
 };
 
-typedef struct AudioFrame AudioFrame;
 struct AudioFrame {
 uint32_t time;
 uint32_t samples[SND_CODEC_MAX_FRAME_SIZE];
-- 
2.9.3

___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel


[Spice-devel] [spice-server 06/17] sound: Don't dereference pointer before NULL check

2017-01-11 Thread Christophe Fergeau
Based on a patch from Frediano Ziglio 

Signed-off-by: Christophe Fergeau 
---
 server/sound.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/server/sound.c b/server/sound.c
index 7ebea90..1eab75b 100644
--- a/server/sound.c
+++ b/server/sound.c
@@ -1125,11 +1125,11 @@ SPICE_GNUC_VISIBLE void 
spice_server_playback_start(SpicePlaybackInstance *sin)
 SPICE_GNUC_VISIBLE void spice_server_playback_stop(SpicePlaybackInstance *sin)
 {
 SndChannelClient *client = sin->st->channel.connection;
-PlaybackChannelClient *playback_client = SPICE_CONTAINEROF(client, 
PlaybackChannelClient, base);
 
 sin->st->channel.active = 0;
 if (!client)
 return;
+PlaybackChannelClient *playback_client = SPICE_CONTAINEROF(client, 
PlaybackChannelClient, base);
 spice_assert(client->active);
 reds_enable_mm_time(snd_channel_get_server(client));
 client->active = FALSE;
@@ -1383,11 +1383,11 @@ SPICE_GNUC_VISIBLE void 
spice_server_record_set_mute(SpiceRecordInstance *sin, u
 static void snd_record_start(SndChannel *channel)
 {
 SndChannelClient *client = channel->connection;
-RecordChannelClient *record_client = SPICE_CONTAINEROF(client, 
RecordChannelClient, base);
 
 channel->active = 1;
 if (!client)
 return;
+RecordChannelClient *record_client = SPICE_CONTAINEROF(client, 
RecordChannelClient, base);
 spice_assert(!client->active);
 record_client->read_pos = record_client->write_pos = 0;   //todo: improve 
by
   //stream 
generation
@@ -1426,13 +1426,13 @@ SPICE_GNUC_VISIBLE uint32_t 
spice_server_record_get_samples(SpiceRecordInstance
 uint32_t *samples, 
uint32_t bufsize)
 {
 SndChannelClient *client = sin->st->channel.connection;
-RecordChannelClient *record_client = SPICE_CONTAINEROF(client, 
RecordChannelClient, base);
 uint32_t read_pos;
 uint32_t now;
 uint32_t len;
 
 if (!client)
 return 0;
+RecordChannelClient *record_client = SPICE_CONTAINEROF(client, 
RecordChannelClient, base);
 spice_assert(client->active);
 
 if (record_client->write_pos < RECORD_SAMPLES_SIZE / 2) {
-- 
2.9.3

___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel


[Spice-devel] [spice-server 10/17] sound: Add sanity checks in snd_{playback, record}_send

2017-01-11 Thread Christophe Fergeau
Filter out commands which should not happen. Should it be a
g_warn_if_fail() or such instead?

Based on a patch from Frediano Ziglio 

Signed-off-by: Christophe Fergeau 
---
 server/sound.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/server/sound.c b/server/sound.c
index 7941ea2..d45e4b9 100644
--- a/server/sound.c
+++ b/server/sound.c
@@ -851,6 +851,9 @@ static void snd_playback_send(void* data)
 return;
 }
 
+client->command &= SND_PLAYBACK_MODE_MASK|SND_PLAYBACK_PCM_MASK|
+   SND_CTRL_MASK|SND_VOLUME_MUTE_MASK|
+   SND_MIGRATE_MASK|SND_PLAYBACK_LATENCY_MASK;
 while (client->command) {
 if (client->command & SND_PLAYBACK_MODE_MASK) {
 if (!playback_send_mode(playback_client)) {
@@ -910,6 +913,7 @@ static void snd_record_send(void* data)
 return;
 }
 
+client->command &= SND_CTRL_MASK|SND_VOLUME_MUTE_MASK|SND_MIGRATE_MASK;
 while (client->command) {
 if (client->command & SND_CTRL_MASK) {
 if (!snd_record_send_ctl(record_client)) {
-- 
2.9.3

___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel


[Spice-devel] [spice-server 17/17] sound: Convert SndChannelClient to RedChannelClient

2017-01-11 Thread Christophe Fergeau
SndChannelClient is now inheriting from GObject, and has switched
from using its own code for sending data to using RedChannelClient.
This commit makes it directly inherit from RedChannelClient rather than
having a channel_client field. This allows to get rid of the whole
DummyChannel/DummyChannelClient code.

Based on a patch from Frediano Ziglio 

Signed-off-by: Christophe Fergeau 
---
 server/Makefile.am|   2 -
 server/dummy-channel-client.c | 140 ---
 server/dummy-channel-client.h |  65 ---
 server/sound.c| 258 +-
 4 files changed, 103 insertions(+), 362 deletions(-)
 delete mode 100644 server/dummy-channel-client.c
 delete mode 100644 server/dummy-channel-client.h

diff --git a/server/Makefile.am b/server/Makefile.am
index 90ff779..1442bf1 100644
--- a/server/Makefile.am
+++ b/server/Makefile.am
@@ -104,8 +104,6 @@ libserver_la_SOURCES =  \
red-channel-client-private.h\
red-client.c\
red-client.h\
-   dummy-channel-client.c  \
-   dummy-channel-client.h  \
red-common.h\
dispatcher.c\
dispatcher.h\
diff --git a/server/dummy-channel-client.c b/server/dummy-channel-client.c
deleted file mode 100644
index 4efaa31..000
--- a/server/dummy-channel-client.c
+++ /dev/null
@@ -1,140 +0,0 @@
-/*
-   Copyright (C) 2009-2015 Red Hat, Inc.
-
-   This library is free software; you can redistribute it and/or
-   modify it under the terms of the GNU Lesser General Public
-   License as published by the Free Software Foundation; either
-   version 2.1 of the License, or (at your option) any later version.
-
-   This library is distributed in the hope that it will be useful,
-   but WITHOUT ANY WARRANTY; without even the implied warranty of
-   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
-   Lesser General Public License for more details.
-
-   You should have received a copy of the GNU Lesser General Public
-   License along with this library; if not, see .
-*/
-#ifdef HAVE_CONFIG_H
-#include 
-#endif
-
-#include "dummy-channel-client.h"
-#include "red-channel.h"
-#include "red-client.h"
-
-static void dummy_channel_client_initable_interface_init(GInitableIface 
*iface);
-
-G_DEFINE_TYPE_WITH_CODE(DummyChannelClient, dummy_channel_client, 
RED_TYPE_CHANNEL_CLIENT,
-G_IMPLEMENT_INTERFACE(G_TYPE_INITABLE,
-  
dummy_channel_client_initable_interface_init))
-
-#define DUMMY_CHANNEL_CLIENT_PRIVATE(o) \
-(G_TYPE_INSTANCE_GET_PRIVATE((o), TYPE_DUMMY_CHANNEL_CLIENT, 
DummyChannelClientPrivate))
-
-struct DummyChannelClientPrivate
-{
-gboolean connected;
-};
-
-static gboolean dummy_channel_client_initable_init(GInitable *initable,
-   GCancellable *cancellable,
-   GError **error)
-{
-GError *local_error = NULL;
-RedChannelClient *rcc = RED_CHANNEL_CLIENT(initable);
-RedClient *client = red_channel_client_get_client(rcc);
-RedChannel *channel = red_channel_client_get_channel(rcc);
-
-red_channel_add_client(channel, rcc);
-if (!red_client_add_channel(client, rcc, _error)) {
-red_channel_remove_client(channel, rcc);
-}
-
-if (local_error) {
-g_warning("Failed to create channel client: %s", local_error->message);
-g_propagate_error(error, local_error);
-}
-return local_error == NULL;
-}
-
-static void dummy_channel_client_initable_interface_init(GInitableIface *iface)
-{
-iface->init = dummy_channel_client_initable_init;
-}
-
-static gboolean dummy_channel_client_is_connected(RedChannelClient *rcc)
-{
-return DUMMY_CHANNEL_CLIENT(rcc)->priv->connected;
-}
-
-static void dummy_channel_client_disconnect(RedChannelClient *rcc)
-{
-DummyChannelClient *self = DUMMY_CHANNEL_CLIENT(rcc);
-RedChannel *channel = red_channel_client_get_channel(rcc);
-GList *link;
-uint32_t type, id;
-
-if (channel && (link = g_list_find(red_channel_get_clients(channel), 
rcc))) {
-g_object_get(channel, "channel-type", , "id", , NULL);
-spice_printerr("rcc=%p (channel=%p type=%d id=%d)", rcc, channel,
-   type, id);
-red_channel_remove_client(channel, link->data);
-}
-self->priv->connected = FALSE;
-}
-
-static void
-dummy_channel_client_class_init(DummyChannelClientClass *klass)
-{
-RedChannelClientClass *cc_class = RED_CHANNEL_CLIENT_CLASS(klass);
-
-g_type_class_add_private(klass, sizeof(DummyChannelClientPrivate));
-
-cc_class->is_connected = dummy_channel_client_is_connected;
- 

[Spice-devel] [spice-server 02/17] sound: Remove dummy-channel.[ch]

2017-01-11 Thread Christophe Fergeau
This is no longer used since "sound: Convert SndChannel to GObject"

Signed-off-by: Christophe Fergeau 
---
 server/Makefile.am |  2 --
 server/dummy-channel.c | 94 --
 server/dummy-channel.h | 60 
 server/sound.c |  1 -
 4 files changed, 157 deletions(-)
 delete mode 100644 server/dummy-channel.c
 delete mode 100644 server/dummy-channel.h

diff --git a/server/Makefile.am b/server/Makefile.am
index ae79ed7..90ff779 100644
--- a/server/Makefile.am
+++ b/server/Makefile.am
@@ -104,8 +104,6 @@ libserver_la_SOURCES =  \
red-channel-client-private.h\
red-client.c\
red-client.h\
-   dummy-channel.c \
-   dummy-channel.h \
dummy-channel-client.c  \
dummy-channel-client.h  \
red-common.h\
diff --git a/server/dummy-channel.c b/server/dummy-channel.c
deleted file mode 100644
index aecd3a6..000
--- a/server/dummy-channel.c
+++ /dev/null
@@ -1,94 +0,0 @@
-/*
-   Copyright (C) 2016 Red Hat, Inc.
-
-   This library is free software; you can redistribute it and/or
-   modify it under the terms of the GNU Lesser General Public
-   License as published by the Free Software Foundation; either
-   version 2.1 of the License, or (at your option) any later version.
-
-   This library is distributed in the hope that it will be useful,
-   but WITHOUT ANY WARRANTY; without even the implied warranty of
-   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
-   Lesser General Public License for more details.
-
-   You should have received a copy of the GNU Lesser General Public
-   License along with this library; if not, see .
-*/
-#ifdef HAVE_CONFIG_H
-#include 
-#endif
-
-#include "dummy-channel.h"
-
-G_DEFINE_TYPE(DummyChannel, dummy_channel, RED_TYPE_CHANNEL)
-
-static int dummy_config_socket(RedChannelClient *self)
-{
-return 1;
-}
-
-static void dummy_on_disconnect(RedChannelClient *self)
-{
-}
-
-static uint8_t* dummy_alloc_recv_buf(RedChannelClient *self,
- uint16_t type,
- uint32_t size)
-{
-return NULL;
-}
-
-static void dummy_release_recv_buf(RedChannelClient *self,
-   uint16_t type,
-   uint32_t size,
-   uint8_t *msg)
-{
-}
-
-static void
-dummy_channel_class_init(DummyChannelClass *klass)
-{
-RedChannelClass *channel_class = RED_CHANNEL_CLASS(klass);
-channel_class->config_socket = dummy_config_socket;
-channel_class->on_disconnect = dummy_on_disconnect;
-channel_class->alloc_recv_buf = dummy_alloc_recv_buf;
-channel_class->release_recv_buf = dummy_release_recv_buf;
-}
-
-static void
-dummy_channel_init(DummyChannel *self)
-{
-}
-
-// TODO: red_worker can use this one
-static void dummy_watch_update_mask(const SpiceCoreInterfaceInternal *iface,
-SpiceWatch *watch, int event_mask)
-{
-}
-
-static SpiceWatch *dummy_watch_add(const SpiceCoreInterfaceInternal *iface,
-   int fd, int event_mask, SpiceWatchFunc 
func, void *opaque)
-{
-return NULL; // apparently allowed?
-}
-
-static void dummy_watch_remove(const SpiceCoreInterfaceInternal *iface, 
SpiceWatch *watch)
-{
-}
-
-// TODO: actually, since I also use channel_client_dummy, no need for core. 
Can be NULL
-static const SpiceCoreInterfaceInternal dummy_core = {
-.watch_update_mask = dummy_watch_update_mask,
-.watch_add = dummy_watch_add,
-.watch_remove = dummy_watch_remove,
-};
-
-RedChannel *dummy_channel_new(RedsState *reds, uint32_t type, uint32_t id)
-{
-return g_object_new(TYPE_DUMMY_CHANNEL,
-"spice-server", reds,
-"core-interface", _core,
-"channel-type", type,
-"id", id,
-NULL);
-}
diff --git a/server/dummy-channel.h b/server/dummy-channel.h
deleted file mode 100644
index 9527633..000
--- a/server/dummy-channel.h
+++ /dev/null
@@ -1,60 +0,0 @@
-/*
-   Copyright (C) 2009-2015 Red Hat, Inc.
-
-   This library is free software; you can redistribute it and/or
-   modify it under the terms of the GNU Lesser General Public
-   License as published by the Free Software Foundation; either
-   version 2.1 of the License, or (at your option) any later version.
-
-   This library is distributed in the hope that it will be useful,
-   but WITHOUT ANY WARRANTY; without even the implied warranty of
-   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
-   Lesser General Public License for more details.
-
-   You should have received a copy 

[Spice-devel] [spice-server 16/17] sound: Turn {Playback, Record}ChannelClient into GObjects

2017-01-11 Thread Christophe Fergeau
This is in preparation for making them inherit from RedChannelClient.
Doing it in one go would result in a very huge commit, so this commit
starts by turning these into GObjects, while still using a
DummyChannelClient instance for sending the data.

Based on a patch from Frediano Ziglio 

Signed-off-by: Christophe Fergeau 
---
 server/sound.c | 322 -
 1 file changed, 207 insertions(+), 115 deletions(-)

diff --git a/server/sound.c b/server/sound.c
index 0992cf4..d397bfb 100644
--- a/server/sound.c
+++ b/server/sound.c
@@ -83,14 +83,17 @@ typedef struct SpicePlaybackState PlaybackChannel;
 typedef struct SpiceRecordState RecordChannel;
 
 typedef void (*snd_channel_on_message_done_proc)(SndChannelClient *client);
-typedef void (*snd_channel_cleanup_channel_proc)(SndChannelClient *client);
 
-#define SND_CHANNEL_CLIENT(obj) (&(obj)->base)
+#define TYPE_SND_CHANNEL_CLIENT snd_channel_client_get_type()
+#define SND_CHANNEL_CLIENT(obj) \
+(G_TYPE_CHECK_INSTANCE_CAST((obj), TYPE_SND_CHANNEL_CLIENT, 
SndChannelClient))
+#define IS_SND_CHANNEL_CLIENT(obj) \
+(G_TYPE_CHECK_INSTANCE_TYPE ((obj), TYPE_SND_CHANNEL_CLIENT))
+GType snd_channel_client_get_type(void) G_GNUC_CONST;
 
 /* Connects an audio client to a Spice client */
 struct SndChannelClient {
-int refs;
-
+GObject parent;
 RedChannelClient *channel_client;
 
 int active;
@@ -104,9 +107,14 @@ struct SndChannelClient {
 RedPipeItem persistent_pipe_item;
 
 snd_channel_on_message_done_proc on_message_done;
-snd_channel_cleanup_channel_proc cleanup;
 };
 
+typedef struct SndChannelClientClass {
+GObjectClass parent_class;
+} SndChannelClientClass;
+
+G_DEFINE_TYPE(SndChannelClient, snd_channel_client, G_TYPE_OBJECT)
+
 
 enum {
 RED_PIPE_ITEM_PERSISTENT = RED_PIPE_ITEM_TYPE_CHANNEL_BASE,
@@ -129,8 +137,13 @@ struct AudioFrameContainer
 AudioFrame items[NUM_AUDIO_FRAMES];
 };
 
+#define TYPE_PLAYBACK_CHANNEL_CLIENT playback_channel_client_get_type()
+#define PLAYBACK_CHANNEL_CLIENT(obj) \
+(G_TYPE_CHECK_INSTANCE_CAST((obj), TYPE_PLAYBACK_CHANNEL_CLIENT, 
PlaybackChannelClient))
+GType playback_channel_client_get_type(void) G_GNUC_CONST;
+
 struct PlaybackChannelClient {
-SndChannelClient base;
+SndChannelClient parent;
 
 AudioFrameContainer *frames;
 AudioFrame *free_frames;
@@ -142,6 +155,13 @@ struct PlaybackChannelClient {
 uint8_t  encode_buf[SND_CODEC_MAX_COMPRESSED_BYTES];
 };
 
+typedef struct PlaybackChannelClientClass {
+SndChannelClientClass parent_class;
+} PlaybackChannelClientClass;
+
+G_DEFINE_TYPE(PlaybackChannelClient, playback_channel_client, 
TYPE_SND_CHANNEL_CLIENT)
+
+
 typedef struct SpiceVolumeState {
 uint16_t *volume;
 uint8_t volume_nchannels;
@@ -202,8 +222,13 @@ typedef struct RecordChannelClass {
 G_DEFINE_TYPE(RecordChannel, record_channel, TYPE_SND_CHANNEL)
 
 
+#define TYPE_RECORD_CHANNEL_CLIENT record_channel_client_get_type()
+#define RECORD_CHANNEL_CLIENT(obj) \
+(G_TYPE_CHECK_INSTANCE_CAST((obj), TYPE_RECORD_CHANNEL_CLIENT, 
RecordChannelClient))
+GType record_channel_client_get_type(void) G_GNUC_CONST;
+
 struct RecordChannelClient {
-SndChannelClient base;
+SndChannelClient parent;
 uint32_t samples[RECORD_SAMPLES_SIZE];
 uint32_t write_pos;
 uint32_t read_pos;
@@ -214,22 +239,41 @@ struct RecordChannelClient {
 uint8_t  decode_buf[SND_CODEC_MAX_FRAME_BYTES];
 };
 
+typedef struct RecordChannelClientClass {
+SndChannelClientClass parent_class;
+} RecordChannelClientClass;
+
+G_DEFINE_TYPE(RecordChannelClient, record_channel_client, 
TYPE_SND_CHANNEL_CLIENT)
+
+
 /* A list of all Spice{Playback,Record}State objects */
 static SndChannel *snd_channels;
 
 static void snd_playback_start(SndChannel *channel);
 static void snd_record_start(SndChannel *channel);
-static void snd_playback_alloc_frames(PlaybackChannelClient *playback);
 static void snd_send(SndChannelClient * client);
 
-static SndChannelClient *snd_channel_unref(SndChannelClient *client)
+enum {
+PROP0,
+PROP_CHANNEL_CLIENT
+};
+
+static void
+snd_channel_client_set_property(GObject *object,
+guint property_id,
+const GValue *value,
+GParamSpec *pspec)
 {
-if (!--client->refs) {
-spice_printerr("SndChannelClient=%p freed", client);
-free(client);
-return NULL;
+SndChannelClient *self = SND_CHANNEL_CLIENT(object);
+
+switch (property_id)
+{
+case PROP_CHANNEL_CLIENT:
+self->channel_client = g_value_dup_object(value);
+break;
+default:
+G_OBJECT_WARN_INVALID_PROPERTY_ID(object, property_id, pspec);
 }
-return client;
 }
 
 static SndChannelClient *snd_channel_client_from_dummy(RedChannelClient *dummy)
@@ -238,6 +282,7 @@ static SndChannelClient 

[Spice-devel] [spice-server 11/17] sound: Prefer snd_set_command() over snd_*_send_*()

2017-01-11 Thread Christophe Fergeau
snd_set_command()/snd_send() are higher level methods which take care of
scheduling calls to the corresponding snd_*_send_*() methods when
appropriate. This commit switches a few direct snd_*_send_*() calls to
snd_set_command()/snd_send().

Based on a patch from Frediano Ziglio 

Signed-off-by: Christophe Fergeau 
---
 server/sound.c | 12 
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/server/sound.c b/server/sound.c
index d45e4b9..b0a1367 100644
--- a/server/sound.c
+++ b/server/sound.c
@@ -1094,7 +1094,6 @@ SPICE_GNUC_VISIBLE void 
spice_server_playback_set_volume(SpicePlaybackInstance *
 {
 SpiceVolumeState *st = >st->channel.volume;
 SndChannelClient *client = sin->st->channel.connection;
-PlaybackChannelClient *playback_client = SPICE_CONTAINEROF(client, 
PlaybackChannelClient, base);
 
 st->volume_nchannels = nchannels;
 free(st->volume);
@@ -1103,21 +1102,20 @@ SPICE_GNUC_VISIBLE void 
spice_server_playback_set_volume(SpicePlaybackInstance *
 if (!client || nchannels == 0)
 return;
 
-snd_playback_send_volume(playback_client);
+snd_set_command(client, SND_VOLUME_MASK);
 }
 
 SPICE_GNUC_VISIBLE void spice_server_playback_set_mute(SpicePlaybackInstance 
*sin, uint8_t mute)
 {
 SpiceVolumeState *st = >st->channel.volume;
 SndChannelClient *client = sin->st->channel.connection;
-PlaybackChannelClient *playback_client = SPICE_CONTAINEROF(client, 
PlaybackChannelClient, base);
 
 st->mute = mute;
 
 if (!client)
 return;
 
-snd_playback_send_mute(playback_client);
+snd_set_command(client, SND_MUTE_MASK);
 }
 
 static void snd_playback_start(SndChannel *channel)
@@ -1378,7 +1376,6 @@ SPICE_GNUC_VISIBLE void 
spice_server_record_set_volume(SpiceRecordInstance *sin,
 {
 SpiceVolumeState *st = >st->channel.volume;
 SndChannelClient *client = sin->st->channel.connection;
-RecordChannelClient *record_client = SPICE_CONTAINEROF(client, 
RecordChannelClient, base);
 
 st->volume_nchannels = nchannels;
 free(st->volume);
@@ -1387,21 +1384,20 @@ SPICE_GNUC_VISIBLE void 
spice_server_record_set_volume(SpiceRecordInstance *sin,
 if (!client || nchannels == 0)
 return;
 
-snd_record_send_volume(record_client);
+snd_set_command(client, SND_VOLUME_MASK);
 }
 
 SPICE_GNUC_VISIBLE void spice_server_record_set_mute(SpiceRecordInstance *sin, 
uint8_t mute)
 {
 SpiceVolumeState *st = >st->channel.volume;
 SndChannelClient *client = sin->st->channel.connection;
-RecordChannelClient *record_client = SPICE_CONTAINEROF(client, 
RecordChannelClient, base);
 
 st->mute = mute;
 
 if (!client)
 return;
 
-snd_record_send_mute(record_client);
+snd_set_command(client, SND_MUTE_MASK);
 }
 
 static void snd_record_start(SndChannel *channel)
-- 
2.9.3

___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel


[Spice-devel] [spice-server 05/17] sound: Remove unused __new_channel() argument

2017-01-11 Thread Christophe Fergeau
It became unused in 26027036c 'red_channel: remove unused migrate flag
from RedChannel' but was never removed from the function prototype.

Signed-off-by: Christophe Fergeau 
---
 server/sound.c | 9 -
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/server/sound.c b/server/sound.c
index d24a0d9..7ebea90 100644
--- a/server/sound.c
+++ b/server/sound.c
@@ -928,7 +928,6 @@ static void snd_record_send(void* data)
 static SndChannelClient *__new_channel(SndChannel *channel, int size, uint32_t 
channel_id,
RedClient *red_client,
RedsStream *stream,
-   int migrate,
snd_channel_send_messages_proc 
send_messages,
snd_channel_handle_message_proc 
handle_message,
snd_channel_on_message_done_proc 
on_message_done,
@@ -1281,7 +1280,8 @@ static void snd_playback_cleanup(SndChannelClient *client)
 }
 
 static void snd_set_playback_peer(RedChannel *red_channel, RedClient *client, 
RedsStream *stream,
-  int migration, int num_common_caps, uint32_t 
*common_caps,
+  G_GNUC_UNUSED int migration,
+  int num_common_caps, uint32_t *common_caps,
   int num_caps, uint32_t *caps)
 {
 SndChannel *channel = SND_CHANNEL(red_channel);
@@ -1294,7 +1294,6 @@ static void snd_set_playback_peer(RedChannel 
*red_channel, RedClient *client, Re

SPICE_CHANNEL_PLAYBACK,
client,
stream,
-   migration,

snd_playback_send,

snd_playback_handle_message,

snd_playback_on_message_done,
@@ -1523,7 +1522,8 @@ static void snd_record_cleanup(SndChannelClient *client)
 }
 
 static void snd_set_record_peer(RedChannel *red_channel, RedClient *client, 
RedsStream *stream,
-int migration, int num_common_caps, uint32_t 
*common_caps,
+G_GNUC_UNUSED int migration,
+int num_common_caps, uint32_t *common_caps,
 int num_caps, uint32_t *caps)
 {
 SndChannel *channel = SND_CHANNEL(red_channel);
@@ -1536,7 +1536,6 @@ static void snd_set_record_peer(RedChannel *red_channel, 
RedClient *client, Reds

SPICE_CHANNEL_RECORD,
client,
stream,
-   migration,
snd_record_send,

snd_record_handle_message,

snd_record_on_message_done,
-- 
2.9.3

___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel


[Spice-devel] [spice-server 08/17] sound: Implement snd_channel_config_socket

2017-01-11 Thread Christophe Fergeau
This is in preparation for switching SndChannelClient into a proper
RedChannelClient. The prototype of the new helper matches what is
expected from the RedChannel::config_socket vfunc.

To be able to achieve that, this commit associates the sound channel
RedsStream instance with the DummyChannelClient instance we have, and
then call snd_channel_config_socket() on that instance.

Based on a patch from Frediano Ziglio 

Signed-off-by: Christophe Fergeau 
---
 server/dummy-channel-client.c |  2 +
 server/dummy-channel-client.h |  1 +
 server/sound.c| 96 ++-
 3 files changed, 53 insertions(+), 46 deletions(-)

diff --git a/server/dummy-channel-client.c b/server/dummy-channel-client.c
index 61d5683..4efaa31 100644
--- a/server/dummy-channel-client.c
+++ b/server/dummy-channel-client.c
@@ -104,6 +104,7 @@ dummy_channel_client_init(DummyChannelClient *self)
 
 RedChannelClient* dummy_channel_client_create(RedChannel *channel,
   RedClient  *client,
+  RedsStream *stream,
   int num_common_caps,
   uint32_t *common_caps,
   int num_caps, uint32_t *caps)
@@ -125,6 +126,7 @@ RedChannelClient* dummy_channel_client_create(RedChannel 
*channel,
  NULL, NULL,
  "channel", channel,
  "client", client,
+ "stream", stream,
  "caps", caps_array,
  "common-caps", common_caps_array,
  NULL);
diff --git a/server/dummy-channel-client.h b/server/dummy-channel-client.h
index 8013aa2..54e0e6c 100644
--- a/server/dummy-channel-client.h
+++ b/server/dummy-channel-client.h
@@ -56,6 +56,7 @@ GType dummy_channel_client_get_type(void) G_GNUC_CONST;
 
 RedChannelClient *dummy_channel_client_create(RedChannel *channel,
   RedClient  *client,
+  RedsStream *stream,
   int num_common_caps, uint32_t 
*common_caps,
   int num_caps, uint32_t *caps);
 
diff --git a/server/sound.c b/server/sound.c
index bd9bb66..e1d6805 100644
--- a/server/sound.c
+++ b/server/sound.c
@@ -925,6 +925,8 @@ static void snd_record_send(void* data)
 }
 }
 
+static int snd_channel_config_socket(RedChannelClient *rcc);
+
 static SndChannelClient *__new_channel(SndChannel *channel, int size, uint32_t 
channel_id,
RedClient *red_client,
RedsStream *stream,
@@ -936,50 +938,8 @@ static SndChannelClient *__new_channel(SndChannel 
*channel, int size, uint32_t c
uint32_t *caps, int num_caps)
 {
 SndChannelClient *client;
-int delay_val;
-int flags;
-#ifdef SO_PRIORITY
-int priority;
-#endif
-int tos;
-MainChannelClient *mcc = red_client_get_main(red_client);
 RedsState *reds = red_channel_get_server(RED_CHANNEL(channel));
 
-spice_assert(stream);
-if ((flags = fcntl(stream->socket, F_GETFL)) == -1) {
-spice_printerr("accept failed, %s", strerror(errno));
-goto error1;
-}
-
-#ifdef SO_PRIORITY
-priority = 6;
-if (setsockopt(stream->socket, SOL_SOCKET, SO_PRIORITY, (void*),
-   sizeof(priority)) == -1) {
-if (errno != ENOTSUP) {
-spice_printerr("setsockopt failed, %s", strerror(errno));
-}
-}
-#endif
-
-tos = IPTOS_LOWDELAY;
-if (setsockopt(stream->socket, IPPROTO_IP, IP_TOS, (void*), 
sizeof(tos)) == -1) {
-if (errno != ENOTSUP) {
-spice_printerr("setsockopt failed, %s", strerror(errno));
-}
-}
-
-delay_val = main_channel_client_is_low_bandwidth(mcc) ? 0 : 1;
-if (setsockopt(stream->socket, IPPROTO_TCP, TCP_NODELAY, _val, 
sizeof(delay_val)) == -1) {
-if (errno != ENOTSUP) {
-spice_printerr("setsockopt failed, %s", strerror(errno));
-}
-}
-
-if (fcntl(stream->socket, F_SETFL, flags | O_NONBLOCK) == -1) {
-spice_printerr("accept failed, %s", strerror(errno));
-goto error1;
-}
-
 spice_assert(size >= sizeof(*client));
 client = spice_malloc0(size);
 client->refs = 1;
@@ -1004,24 +964,68 @@ static SndChannelClient *__new_channel(SndChannel 
*channel, int size, uint32_t c
 client->cleanup = cleanup;
 
 client->channel_client =
-dummy_channel_client_create(RED_CHANNEL(channel), red_client,
+dummy_channel_client_create(RED_CHANNEL(channel), red_client, stream,
 num_common_caps, common_caps, num_caps, 
caps);
 if 

[Spice-devel] [spice-server 12/17] sound: Remove SndChannelClient::send_data::marshaller

2017-01-11 Thread Christophe Fergeau
We can use the marshaller provided by the dummy RedChannelClient
associated with SndChannelClient.

Based on a patch from Frediano Ziglio 

Signed-off-by: Christophe Fergeau 
---
 server/sound.c | 49 -
 1 file changed, 28 insertions(+), 21 deletions(-)

diff --git a/server/sound.c b/server/sound.c
index b0a1367..45594fb 100644
--- a/server/sound.c
+++ b/server/sound.c
@@ -26,7 +26,6 @@
 #include 
 #include 
 
-#include 
 #include 
 #include 
 
@@ -41,7 +40,6 @@
 #include "red-channel-client-private.h"
 #include "red-client.h"
 #include "sound.h"
-#include "demarshallers.h"
 #include "main-channel-client.h"
 
 #ifndef IOV_MAX
@@ -108,7 +106,6 @@ struct SndChannelClient {
 
 struct {
 uint64_t serial;
-SpiceMarshaller *marshaller;
 uint32_t size;
 uint32_t pos;
 } send_data;
@@ -275,7 +272,6 @@ static void snd_disconnect_channel(SndChannelClient *client)
 client->stream->watch = NULL;
 reds_stream_free(client->stream);
 client->stream = NULL;
-spice_marshaller_destroy(client->send_data.marshaller);
 snd_channel_unref(client);
 channel->connection = NULL;
 }
@@ -306,6 +302,8 @@ static void snd_record_on_message_done(SndChannelClient 
*client)
 static int snd_send_data(SndChannelClient *client)
 {
 uint32_t n;
+RedChannelClient *rcc = client->channel_client;
+SpiceMarshaller *m = red_channel_client_get_marshaller(rcc);
 
 if (!client) {
 return FALSE;
@@ -330,7 +328,7 @@ static int snd_send_data(SndChannelClient *client)
 break;
 }
 
-vec_size = spice_marshaller_fill_iovec(client->send_data.marshaller,
+vec_size = spice_marshaller_fill_iovec(m,
vec, IOV_MAX, 
client->send_data.pos);
 n = reds_stream_writev(client->stream, vec, vec_size);
 if (n == -1) {
@@ -562,17 +560,17 @@ static void snd_event(int fd, int event, void *data)
 static inline int snd_reset_send_data(SndChannelClient *client, uint16_t verb)
 {
 SpiceDataHeaderOpaque *header;
+RedChannelClient *rcc = client->channel_client;
+SpiceMarshaller *m = red_channel_client_get_marshaller(rcc);
 
 if (!client) {
 return FALSE;
 }
 
 header = >channel_client->priv->send_data.header;
-spice_marshaller_reset(client->send_data.marshaller);
-header->data = spice_marshaller_reserve_space(client->send_data.marshaller,
-  header->header_size);
-spice_marshaller_set_base(client->send_data.marshaller,
-  header->header_size);
+spice_marshaller_reset(m);
+header->data = spice_marshaller_reserve_space(m, header->header_size);
+spice_marshaller_set_base(m, header->header_size);
 client->send_data.pos = 0;
 header->set_msg_size(header, 0);
 header->set_msg_type(header, verb);
@@ -588,16 +586,19 @@ static inline int snd_reset_send_data(SndChannelClient 
*client, uint16_t verb)
 static int snd_begin_send_message(SndChannelClient *client)
 {
 SpiceDataHeaderOpaque *header = 
>channel_client->priv->send_data.header;
+RedChannelClient *rcc = client->channel_client;
+SpiceMarshaller *m = red_channel_client_get_marshaller(rcc);
 
-spice_marshaller_flush(client->send_data.marshaller);
-client->send_data.size = 
spice_marshaller_get_total_size(client->send_data.marshaller);
+spice_marshaller_flush(m);
+client->send_data.size = spice_marshaller_get_total_size(m);
 header->set_msg_size(header, client->send_data.size - header->header_size);
 return snd_send_data(client);
 }
 
 static int snd_channel_send_migrate(SndChannelClient *client)
 {
-SpiceMarshaller *m = client->send_data.marshaller;
+RedChannelClient *rcc = client->channel_client;
+SpiceMarshaller *m = red_channel_client_get_marshaller(rcc);
 SpiceMsgMigrate migrate;
 
 if (!snd_reset_send_data(client, SPICE_MSG_MIGRATE)) {
@@ -619,8 +620,9 @@ static int snd_send_volume(SndChannelClient *client, 
uint32_t cap, int msg)
 {
 SpiceMsgAudioVolume *vol;
 uint8_t c;
+RedChannelClient *rcc = client->channel_client;
+SpiceMarshaller *m = red_channel_client_get_marshaller(rcc);
 SpiceVolumeState *st = >channel->volume;
-SpiceMarshaller *m = client->send_data.marshaller;
 
 if (!red_channel_client_test_remote_cap(client->channel_client, cap)) {
 return TRUE;
@@ -649,8 +651,9 @@ static int snd_playback_send_volume(PlaybackChannelClient 
*playback_client)
 static int snd_send_mute(SndChannelClient *client, uint32_t cap, int msg)
 {
 SpiceMsgAudioMute mute;
+RedChannelClient *rcc = client->channel_client;
+SpiceMarshaller *m = red_channel_client_get_marshaller(rcc);
 SpiceVolumeState *st = >channel->volume;
-SpiceMarshaller *m = client->send_data.marshaller;
 
 if 

[Spice-devel] [spice-server 03/17] sound: Remove extraneous whitespace

2017-01-11 Thread Christophe Fergeau
No need for this whitespace before ';'
---
 server/sound.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/server/sound.c b/server/sound.c
index f154465..1f6c243 100644
--- a/server/sound.c
+++ b/server/sound.c
@@ -1508,7 +1508,7 @@ static void on_new_record_channel_client(SndChannel 
*channel, SndChannelClient *
 {
 spice_assert(client);
 
-channel->connection = client ;
+channel->connection = client;
 if (channel->volume.volume_nchannels) {
 snd_set_command(client, SND_VOLUME_MASK);
 }
-- 
2.9.3

___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel