[Spice-devel] [PATCH spice-server] red-channel-client: Reduce indentation of some code

2019-05-23 Thread Frediano Ziglio
Just a style change, return earlier to avoid some indentation.

Signed-off-by: Frediano Ziglio 
---
 server/red-channel-client.c | 37 +++--
 1 file changed, 15 insertions(+), 22 deletions(-)

diff --git a/server/red-channel-client.c b/server/red-channel-client.c
index 4978f3567..3fd51d78b 100644
--- a/server/red-channel-client.c
+++ b/server/red-channel-client.c
@@ -690,25 +690,21 @@ static void red_channel_client_ping_timer(void *opaque)
 red_channel_client_cancel_ping_timer(rcc);
 
 #ifdef HAVE_LINUX_SOCKIOS_H /* SIOCOUTQ is a Linux only ioctl on sockets. */
-{
-int so_unsent_size = 0;
+int so_unsent_size = 0;
 
-/* retrieving the occupied size of the socket's tcp snd buffer 
(unacked + unsent) */
-if (ioctl(rcc->priv->stream->socket, SIOCOUTQ, _unsent_size) == -1) 
{
-red_channel_warning(red_channel_client_get_channel(rcc),
-"ioctl(SIOCOUTQ) failed, %s", strerror(errno));
-}
-if (so_unsent_size > 0) {
-/* tcp snd buffer is still occupied. rescheduling ping */
-red_channel_client_start_ping_timer(rcc, 
PING_TEST_IDLE_NET_TIMEOUT_MS);
-} else {
-red_channel_client_push_ping(rcc);
-}
+/* retrieving the occupied size of the socket's tcp snd buffer (unacked + 
unsent) */
+if (ioctl(rcc->priv->stream->socket, SIOCOUTQ, _unsent_size) == -1) {
+red_channel_warning(red_channel_client_get_channel(rcc),
+"ioctl(SIOCOUTQ) failed, %s", strerror(errno));
 }
-#else /* ifdef HAVE_LINUX_SOCKIOS_H */
+if (so_unsent_size > 0) {
+/* tcp snd buffer is still occupied. rescheduling ping */
+red_channel_client_start_ping_timer(rcc, 
PING_TEST_IDLE_NET_TIMEOUT_MS);
+return;
+}
+#endif /* ifdef HAVE_LINUX_SOCKIOS_H */
 /* More portable alternative code path (less accurate but avoids bogus 
ioctls)*/
 red_channel_client_push_ping(rcc);
-#endif /* ifdef HAVE_LINUX_SOCKIOS_H */
 }
 
 static inline int red_channel_client_waiting_for_ack(RedChannelClient *rcc)
@@ -1140,16 +1136,13 @@ static int red_peer_receive(RedStream *stream, uint8_t 
*buf, uint32_t size)
 break;
 } else if (errno == EINTR) {
 continue;
-} else if (errno == EPIPE) {
-return -1;
-} else {
+} else if (errno != EPIPE) {
 g_warning("%s", strerror(errno));
-return -1;
 }
-} else {
-size -= now;
-pos += now;
+return -1;
 }
+size -= now;
+pos += now;
 }
 return pos - buf;
 }
-- 
2.20.1

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

[Spice-devel] [PATCH spice-server] inputs-channel-client: Remove unused declarations

2019-05-23 Thread Frediano Ziglio
Signed-off-by: Frediano Ziglio 
---
 server/inputs-channel-client.h | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/server/inputs-channel-client.h b/server/inputs-channel-client.h
index c22288980..1afc22bd4 100644
--- a/server/inputs-channel-client.h
+++ b/server/inputs-channel-client.h
@@ -61,9 +61,6 @@ RedChannelClient* inputs_channel_client_create(RedChannel 
*channel,
RedStream *stream,
RedChannelCapabilities *caps);
 
-uint16_t inputs_channel_client_get_motion_count(InputsChannelClient* self);
-/* only for migration */
-void inputs_channel_client_set_motion_count(InputsChannelClient* self, 
uint16_t count);
 void inputs_channel_client_on_mouse_motion(InputsChannelClient* self);
 void inputs_channel_client_send_migrate_data(RedChannelClient *rcc,
  SpiceMarshaller *m, RedPipeItem 
*item);
-- 
2.20.1

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

[Spice-devel] [PATCH spice-server] red-channel: Remove unused declaration

2019-05-23 Thread Frediano Ziglio
Signed-off-by: Frediano Ziglio 
---
 server/red-channel.h | 1 -
 1 file changed, 1 deletion(-)

diff --git a/server/red-channel.h b/server/red-channel.h
index 4bfd81ee1..eb16bd4b8 100644
--- a/server/red-channel.h
+++ b/server/red-channel.h
@@ -47,7 +47,6 @@ typedef struct MainChannelClient MainChannelClient;
 
 typedef bool (*channel_handle_message_proc)(RedChannelClient *rcc, uint16_t 
type,
 uint32_t size, void *msg);
-typedef bool (*channel_configure_socket_proc)(RedChannelClient *rcc);
 typedef void (*channel_send_pipe_item_proc)(RedChannelClient *rcc, RedPipeItem 
*item);
 
 typedef bool (*channel_handle_migrate_flush_mark_proc)(RedChannelClient *base);
-- 
2.20.1

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

[Spice-devel] [PATCH spice-server] dispatcher: Use a new API to handle events

2019-05-23 Thread Frediano Ziglio
Instead of having to manually register the file descriptor and
than need to call dispatcher_handle_recv_read just provide a single
API to create the watch.
This has some advantage:
- replace 2 API with 1;
- code reuse for handling the event (removed 2 functions);
- avoid the caller to use the file descriptor;
- avoid the caller to register wrong events.

Signed-off-by: Frediano Ziglio 
---
 server/dispatcher.c  | 15 +--
 server/dispatcher.h  | 25 +
 server/main-dispatcher.c | 12 +---
 server/red-worker.c  | 10 +-
 4 files changed, 16 insertions(+), 46 deletions(-)

diff --git a/server/dispatcher.c b/server/dispatcher.c
index 602f30a8e..bd74b6f35 100644
--- a/server/dispatcher.c
+++ b/server/dispatcher.c
@@ -317,11 +317,13 @@ static int dispatcher_handle_single_read(Dispatcher 
*dispatcher)
 }
 
 /*
- * dispatcher_handle_recv_read
+ * dispatcher_handle_event
  * doesn't handle being in the middle of a message. all reads are blocking.
  */
-void dispatcher_handle_recv_read(Dispatcher *dispatcher)
+static void dispatcher_handle_event(int fd, int event, void *opaque)
 {
+Dispatcher *dispatcher = opaque;
+
 while (dispatcher_handle_single_read(dispatcher)) {
 }
 }
@@ -430,14 +432,15 @@ static void setup_dummy_signal_handler(void)
 }
 #endif
 
-void dispatcher_set_opaque(Dispatcher *self, void *opaque)
+SpiceWatch *dispatcher_create_watch(Dispatcher *dispatcher, 
SpiceCoreInterfaceInternal *core)
 {
-self->priv->opaque = opaque;
+return core->watch_add(core, dispatcher->priv->recv_fd,
+   SPICE_WATCH_EVENT_READ, dispatcher_handle_event, 
dispatcher);
 }
 
-int dispatcher_get_recv_fd(Dispatcher *dispatcher)
+void dispatcher_set_opaque(Dispatcher *self, void *opaque)
 {
-return dispatcher->priv->recv_fd;
+self->priv->opaque = opaque;
 }
 
 pthread_t dispatcher_get_thread_id(Dispatcher *self)
diff --git a/server/dispatcher.h b/server/dispatcher.h
index 49215782a..5bd0b1055 100644
--- a/server/dispatcher.h
+++ b/server/dispatcher.h
@@ -148,29 +148,14 @@ void dispatcher_register_handler(Dispatcher *dispatcher, 
uint32_t message_type,
 void dispatcher_register_universal_handler(Dispatcher *dispatcher,
 dispatcher_handle_any_message handler);
 
-/* dispatcher_handle_recv_read
+/* dispatcher_create_watch
  *
- * A convenience function that is intended to be called by the receiving thread
- * to handle all incoming messages and execute any handlers for those messages.
- * This function will handle all incoming messages until there is no more data
- * to read, so multiple handlers may be executed from a single call to
- * dispatcher_handle_recv_read().
+ * Create a new watch to handle events for the dispatcher.
+ * You should release it before releasing the dispatcher.
  *
- * @dispatcher: Dispatcher instance
- */
-void dispatcher_handle_recv_read(Dispatcher *);
-
-/* dispatcher_get_recv_fd
- *
- * This function returns the file descriptor that is used by the receiving
- * thread to listen for incoming messages. You should not read or write
- * directly to this fd, but should only use it to watch for read events. When
- * there is a read event, you should use dispatcher_handle_recv_read() to
- * handle the incoming messages.
- *
- * @return: receive file descriptor of the dispatcher
+ * @return: newly created watch
  */
-int dispatcher_get_recv_fd(Dispatcher *);
+SpiceWatch *dispatcher_create_watch(Dispatcher *dispatcher, 
SpiceCoreInterfaceInternal *core);
 
 /* dispatcher_set_opaque
  *
diff --git a/server/main-dispatcher.c b/server/main-dispatcher.c
index 839e7242c..2ca68a4d1 100644
--- a/server/main-dispatcher.c
+++ b/server/main-dispatcher.c
@@ -247,13 +247,6 @@ void main_dispatcher_client_disconnect(MainDispatcher 
*self, RedClient *client)
 }
 }
 
-static void dispatcher_handle_read(int fd, int event, void *opaque)
-{
-Dispatcher *dispatcher = opaque;
-
-dispatcher_handle_recv_read(dispatcher);
-}
-
 /*
  * FIXME:
  * Reds routines shouldn't be exposed. Instead reds.c should register the 
callbacks,
@@ -276,10 +269,7 @@ void main_dispatcher_constructed(GObject *object)
 dispatcher_set_opaque(DISPATCHER(self), self->priv->reds);
 
 self->priv->watch =
-reds_core_watch_add(self->priv->reds,
-dispatcher_get_recv_fd(DISPATCHER(self)),
-SPICE_WATCH_EVENT_READ, dispatcher_handle_read,
-DISPATCHER(self));
+dispatcher_create_watch(DISPATCHER(self), 
reds_get_core_interface(self->priv->reds));
 dispatcher_register_handler(DISPATCHER(self), 
MAIN_DISPATCHER_CHANNEL_EVENT,
 main_dispatcher_handle_channel_event,
 sizeof(MainDispatcherChannelEventMessage), 
false);
diff --git a/server/red-worker.c b/server/red-worker.c
index d64c26e83..1ed5a05cb 100644
--- a/server/red-worker.c
+++ 

Re: [Spice-devel] [spice-gtk v1 3/4] usb-redirection: isolate usage of libusb and usbredirhost

2019-05-23 Thread Yuri Benditovich
On Tue, May 21, 2019 at 1:45 PM Victor Toso  wrote:
>
> Hi,
>
> So this is the core of the series and we have a lot going on
> here. The goal of this email is to start a discussion on current
> design and proposed design and what can still be improved.
>
> To be able to provide good enough feedback I had to actually
> understand as much as possible, how things work together as we
> have so many files, objects and roles for both win and linux
> based systems.
>
> What I actually found and discussed a bit with Frediano is that
> we could do, in the current code, a bit better on design. But
> this patch plans to change some of the design itself so, we have
> two possibilities:
>
>  A. refactor existing code and rebase/rework this patch series
>  B. get this patch in shape and do more refactor later
>
> I started to play with some possible changes for (A) like [0] and
> [1] and if I could spend 100% time on this I might keep going but
> it'll likely take good amount of time and testing so I'm inclined
> to work with you on (B) approach.
>
> [0] https://lists.freedesktop.org/archives/spice-devel/2019-May/049071.html
> [1] https://lists.freedesktop.org/archives/spice-devel/2019-May/049087.html
>



> So, with this introduction in mind, let's go.
>
> On Tue, May 21, 2019 at 09:05:51AM +, Victor Toso wrote:
> > From: Yuri Benditovich 
> >
> > As a step toward possibility to present emulated USB devices
> > to the guest, we remove the knowledge about libusb and
> > usbredirhost (which depends on libusb) from all the modules
> > and concentrate it in one (usb backend) which presents
> > abstract USB objects and internal API to all other modules.
>
> This is a good summary but it does so much in terms of code and
> not much is explained here whatsoever.
>
> One thing that makes me uncomfortable with current code but also
> suggested code can be explained with the following type renames:
>
> [var: context] libusb_context -> SpiceUsbBackend
> [var: device ] libusb_device  -> SpiceUsbBackendDevice
> [var: host   ] usbredirhost   -> SpiceUsbBackendChannel
>
> Just renaming the types don't make anything simpler, IMHO but
> what I really want to discuss is by introducing usb-backend,
> can't we reduce some of the complexity, that is, instead of
> dealing with 3 types we can interact with some SpiceUsbBacked
> type alone that should handle context/usbredirhost per device?

The usbredirhost object shall be created before the usb-redir-channel is
used for redirection of any specific device (there is initial data
exchange to introduce options and filters).
On other hand, UsbBackendDevice objects exist regardless of redirection
channels and represent existing USB devices.
So, currently I do not see how we can combine UsbBackendChannel and
UsbBackendDevice.

If you ask my opinion, I would prefer to prioritize things as:
1. Incorporate existing series of patches (what must be changed for that?)
2. Add ability of cd-sharing
3. Decide on GUI related to cd-sharing and add it
4. Review the possibilities of design simplification.

>
> SpiceUsbBackend's types should be handling libusb_context,
> libusb_device and usbredir so I take that what channel-usbredir
> might need from device can be reduce to some internal APIs and
> existing functionality can be dropped instead of handling around
> those new wrappers, eg:
>
> * What's the role of spice_usbredir_channel_set_context() with
>   this patch in? Comment in line on the function.
>
> I don't find it constructive for me to reply to all parts of this
> patch considering that I have doubts on how we should handle the
> code design itself so, my suggestion is:
>
> 1) If you like my FIXUP, let's merge that and remove one patch :)

No problem, let's merge it.
I do not see why it is better than initial one, but I agree with every thing
that let us move forward.

>
> 2) Let's clarify the future roles of channel-usbredir,
>usb-device-manager and usb-backend types. Note that this kind
>of thing probably must be on commit log.
>

I agree in advance with any changes/additions to commit message.
There are no changes in roles of these types - the goal of commit(s) is to
concentrate all the knowledge about libusb/usbredir in one module and
make the changes readable.

> 3) Agree what should be ideal and what we can compromise to do
>later, that is, what are the necessary changes in your patches
>to be merged so we can go back to (A) because I'm not really
>trying to make you refactor all the code yourself with this,
>just asking your help with design itself ;)
>
> > Signed-off-by: Yuri Benditovich 
> > ---
> >  src/Makefile.am   |   2 +
> >  src/channel-usbredir-priv.h   |   9 +-
> >  src/channel-usbredir.c| 216 +++-
> >  src/meson.build   |   2 +
> >  src/usb-backend.c | 622 ++
> >  src/usb-backend.h | 110 ++
> >  src/usb-device-manager-priv.h |   1 -
> >  

Re: [Spice-devel] [PATCH phodav 00/13] Miscellaneous series

2019-05-23 Thread Marc-André Lureau
Hi

On Thu, May 23, 2019 at 10:37 AM Jakub Janků  wrote:
>
> Hi,
>
> this series contains mostly fixes of some things
> that I came across while reading the spice-webdavd.c code,
> without any order.
>
> Cheers,
>
> Jakub Janků (13):
>   spice: remove G_SOURCE_{REMOVE,CONTINUE} definitions
>   spice: rename my_input_channel
>   spice: handle SIGINT properly
>   spice: quit service quickly
>   spice: clear loop pointer on unref
>   spice: print err when g_open fails
>   spice: unref GSocketAddress
>   spice-win: define SERVICE_NAME
>   spice-win: remove MapDriveEnum
>   spice-win: don't assign drive_letter on error
>   spice: move OutputQueue to file
>   rename README to README.md
>   rename NEWS to NEWS.md

Looks good overall,

Phodav is hosted on GNOME gitlab, please open a MR there, so CI can run.

Thanks

>  NEWS => NEWS.md   |   0
>  README => README.md   |  21 ++--
>  spice/meson.build |   8 +-
>  spice/output-queue.c  | 164 +
>  spice/output-queue.h  |  38 ++
>  spice/spice-webdavd.c | 280 ++
>  6 files changed, 286 insertions(+), 225 deletions(-)
>  rename NEWS => NEWS.md (100%)
>  rename README => README.md (56%)
>  create mode 100644 spice/output-queue.c
>  create mode 100644 spice/output-queue.h
>
> --
> 2.21.0
>
> ___
> Spice-devel mailing list
> Spice-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel



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

Re: [Spice-devel] [PATCH phodav 13/13] rename NEWS to NEWS.md

2019-05-23 Thread Marc-André Lureau
On Thu, May 23, 2019 at 10:38 AM Jakub Janků  wrote:
>
> The filename needs the proper extension for GitLab to handle
> the file as a Markdown file.
>
> Signed-off-by: Jakub Janků 

small nack,

I am not sure how GNOME ftprelease will handle that change, at the
moment they document NEWS file only:

https://wiki.gnome.org/MaintainersCorner/Releasing

> ---
>  NEWS => NEWS.md | 0
>  1 file changed, 0 insertions(+), 0 deletions(-)
>  rename NEWS => NEWS.md (100%)
>
> diff --git a/NEWS b/NEWS.md
> similarity index 100%
> rename from NEWS
> rename to NEWS.md
> --
> 2.21.0
>
> ___
> Spice-devel mailing list
> Spice-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel



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

Re: [Spice-devel] [PATCH phodav 12/13] rename README to README.md

2019-05-23 Thread Marc-André Lureau
On Thu, May 23, 2019 at 10:37 AM Jakub Janků  wrote:
>
> The filename needs the proper extension for GitLab to handle
> the file as a Markdown file.
>
> Fix some style issues so that the file renders correctly.
>
> Signed-off-by: Jakub Janků 

why not, ack

> ---
>  README => README.md | 21 ++---
>  1 file changed, 10 insertions(+), 11 deletions(-)
>  rename README => README.md (56%)
>
> diff --git a/README b/README.md
> similarity index 56%
> rename from README
> rename to README.md
> index fa82acd..297756f 100644
> --- a/README
> +++ b/README.md
> @@ -1,31 +1,30 @@
> -phởdav
> -==
> +# phởdav
>
> -= About
> +## About
>
>  phởdav is a WebDav server implementation using libsoup (RFC 4918).
>
> -Releases: http://ftp.gnome.org/pub/GNOME/sources/phodav/
> +Releases: http://ftp.gnome.org/pub/GNOME/sources/phodav/
>
> -= Tools
> +## Tools
>
> -chezdav, allows to share a particual directory (with optional
> -digest authentication)
> +* chezdav, allows to share a particual directory (with optional
> +  digest authentication)
>
> -= Notes
> +## Notes
>
>  phởdav was initially developped as a filesharing mechanism for Spice,
>  but it is generic enough to be reused in other projects, in particular
>  in the GNOME desktop. Further integration work would be a welcome
>  contribution!
>
> -= Contributing
> +## Contributing
>
>  git clone git://git.gnome.org/phodav
>
> -http://lists.freedesktop.org/mailman/listinfo/spice-devel
> +http://lists.freedesktop.org/mailman/listinfo/spice-devel
>
>  Please report bug there:
>  https://bugzilla.gnome.org/enter_bug.cgi?product=phodav
>
> -Forking and sending github pull requests is also welcome.
> \ No newline at end of file
> +Forking and sending pull requests is also welcome.
> \ No newline at end of file
> --
> 2.21.0
>
> ___
> Spice-devel mailing list
> Spice-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel



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

Re: [Spice-devel] [PATCH phodav 11/13] spice: move OutputQueue to file

2019-05-23 Thread Marc-André Lureau
Hi

On Thu, May 23, 2019 at 10:37 AM Jakub Janků  wrote:
>
> OutputQueue is a self-contained unit and as such can be put in
> a separate file to make the spice-webdavd.c less cluttered.
>
> Also, as the current implementation defines output_queue_{ref, unref},
> turn OutputQueue into a GObject which can handle these for us.
>
> Signed-off-by: Jakub Janků 

ack in principle, minor coding style issues

The phodav source code tries to follow glib code style. Can you indent
accordingly?

> ---
>  spice/meson.build |   8 ++-
>  spice/output-queue.c  | 164 ++
>  spice/output-queue.h  |  38 ++
>  spice/spice-webdavd.c | 162 ++---
>  4 files changed, 214 insertions(+), 158 deletions(-)
>  create mode 100644 spice/output-queue.c
>  create mode 100644 spice/output-queue.h
>
> diff --git a/spice/meson.build b/spice/meson.build
> index 6db22cc..06d20e6 100644
> --- a/spice/meson.build
> +++ b/spice/meson.build
> @@ -4,9 +4,15 @@ if host_machine.system() == 'windows'
>win32_deps += compiler.find_library('mpr')
>  endif
>
> +sources = [
> +  'spice-webdavd.c',
> +  'output-queue.c',
> +  'output-queue.h'
> +]
> +
>  executable(
>'spice-webdavd',
> -  [ 'spice-webdavd.c' ],
> +  sources,
>install_dir : sbindir,
>include_directories : incdir,
>dependencies : win32_deps + avahi_deps + deps,
> diff --git a/spice/output-queue.c b/spice/output-queue.c
> new file mode 100644
> index 000..6991493
> --- /dev/null
> +++ b/spice/output-queue.c
> @@ -0,0 +1,164 @@
> +/*
> + * Copyright (C) 2019 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 
> .
> + */
> +#include 
> +
> +#include "output-queue.h"
> +
> +typedef struct _OutputQueueElem
> +{
> +  OutputQueue  *queue;
> +  const guint8 *buf;
> +  gsize size;
> +  PushedCb  cb;
> +  gpointer  user_data;
> +} OutputQueueElem;
> +
> +struct _OutputQueue
> +{
> +  GObjectparent_instance;
> +  GOutputStream *output;
> +  gboolean   flushing;
> +  guint  idle_id;
> +  GQueue*queue;
> +  GCancellable  *cancel;
> +};
> +
> +G_DEFINE_TYPE(OutputQueue, output_queue, G_TYPE_OBJECT);
> +
> +static void output_queue_init(OutputQueue *self)
> +{
> +self->queue = g_queue_new ();
> +}
> +
> +static void output_queue_finalize(GObject *obj)
> +{
> +OutputQueue *self = OUTPUT_QUEUE(obj);
> +
> +g_warn_if_fail (g_queue_get_length (self->queue) == 0);
> +g_warn_if_fail (!self->flushing);
> +g_warn_if_fail (!self->idle_id);
> +
> +g_queue_free_full (self->queue, g_free);
> +g_object_unref (self->output);
> +g_object_unref (self->cancel);
> +
> +G_OBJECT_CLASS(output_queue_parent_class)->finalize(obj);
> +}
> +
> +static void output_queue_class_init(OutputQueueClass *klass)
> +{
> +GObjectClass *gobject_class = G_OBJECT_CLASS(klass);
> +gobject_class->finalize = output_queue_finalize;
> +}
> +
> +OutputQueue* output_queue_new (GOutputStream *output, GCancellable *cancel)
> +{
> +  OutputQueue *self = g_object_new(OUTPUT_TYPE_QUEUE, NULL);
> +  self->output = g_object_ref (output);
> +  self->cancel = g_object_ref (cancel);
> +  return self;
> +}
> +
> +static gboolean output_queue_idle (gpointer user_data);
> +
> +static void
> +output_queue_flush_cb (GObject  *source_object,
> +   GAsyncResult *res,
> +   gpointer  user_data)
> +{
> +  GError *error = NULL;
> +  OutputQueueElem *e = user_data;
> +  OutputQueue *q = e->queue;
> +
> +  g_debug ("flushed");
> +  q->flushing = FALSE;
> +  g_output_stream_flush_finish (G_OUTPUT_STREAM (source_object),
> +res, );
> +  if (error)
> +g_warning ("error: %s", error->message);
> +
> +  g_clear_error ();
> +
> +  if (!q->idle_id)
> +q->idle_id = g_idle_add (output_queue_idle, g_object_ref (q));
> +
> +  g_free (e);
> +  g_object_unref (q);
> +}
> +
> +static gboolean
> +output_queue_idle (gpointer user_data)
> +{
> +  OutputQueue *q = user_data;
> +  OutputQueueElem *e = NULL;
> +  GError *error = NULL;
> +
> +  if (q->flushing)
> +{
> +  g_debug ("already flushing");
> +  goto end;
> +}
> +
> +  e = g_queue_pop_head (q->queue);
> +  if (!e)
> +{
> +  g_debug ("No more data to flush");
> +  

Re: [Spice-devel] [PATCH phodav 10/13] spice-win: don't assign drive_letter on error

2019-05-23 Thread Marc-André Lureau
On Thu, May 23, 2019 at 10:37 AM Jakub Janků  wrote:
>
> drive_letter should be assigned only when the mapping was successful.
>
> Signed-off-by: Jakub Janků 

ack

>  spice/spice-webdavd.c | 12 +---
>  1 file changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/spice/spice-webdavd.c b/spice/spice-webdavd.c
> index 29058e7..f2c7f07 100644
> --- a/spice/spice-webdavd.c
> +++ b/spice/spice-webdavd.c
> @@ -910,14 +910,20 @@ map_drive_cb(GTask *task,
>break;
>  }
>
> -  if (map_drive (drive_letter) != ERROR_ALREADY_ASSIGNED)
> +  ret = map_drive (drive_letter);
> +  if (ret == ERROR_ALREADY_ASSIGNED)
>  {
> -  break;
> +  /* try again with another letter */
> +  continue;
> +}
> +  if (ret != NO_ERROR)
> +{
> +  drive_letter = 0;
>  }
> +  break;
>//TODO: After mapping, rename network drive from 
> \\localhost@PORT\DavWWWRoot
>//  to something like SPICE Shared Folder
>  }
> -
>g_mutex_lock(_drive_data->service_data->mutex);
>map_drive_data->service_data->drive_letter = drive_letter;
>g_mutex_unlock(_drive_data->service_data->mutex);
> --
> 2.21.0
>
> ___
> Spice-devel mailing list
> Spice-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel



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

Re: [Spice-devel] [PATCH phodav 09/13] spice-win: remove MapDriveEnum

2019-05-23 Thread Marc-André Lureau
On Thu, May 23, 2019 at 10:37 AM Jakub Janků  wrote:
>
> The enum doesn't add much value, let's remove it.
>
> Signed-off-by: Jakub Janků 

ack

> ---
>  spice/spice-webdavd.c | 19 +++
>  1 file changed, 7 insertions(+), 12 deletions(-)
>
> diff --git a/spice/spice-webdavd.c b/spice/spice-webdavd.c
> index c77c870..29058e7 100644
> --- a/spice/spice-webdavd.c
> +++ b/spice/spice-webdavd.c
> @@ -738,12 +738,6 @@ open_mux_path (const char *path)
>  #ifdef G_OS_WIN32
>  #define MAX_SHARED_FOLDER_NAME_SIZE 64
>  #define MAX_DRIVE_LETTER_SIZE 3
> -typedef enum _MapDriveEnum
> -{
> -  MAP_DRIVE_OK,
> -  MAP_DRIVE_TRY_AGAIN,
> -  MAP_DRIVE_ERROR
> -} MapDriveEnum;
>
>  typedef struct _MapDriveData
>  {
> @@ -828,7 +822,7 @@ netresource_free(NETRESOURCE *net_resource)
>g_free(net_resource->lpRemoteName);
>  }
>
> -static MapDriveEnum
> +static guint32
>  map_drive(const gchar drive_letter)
>  {
>NETRESOURCE net_resource;
> @@ -841,16 +835,17 @@ map_drive(const gchar drive_letter)
>if (errn == NO_ERROR)
>  {
>g_debug ("Shared folder mapped to %c succesfully", drive_letter);
> -  return MAP_DRIVE_OK;
>  }
>else if (errn == ERROR_ALREADY_ASSIGNED)
>  {
>g_debug ("Drive letter %c is already assigned", drive_letter);
> -  return MAP_DRIVE_TRY_AGAIN;
> +}
> +  else
> +{
> +  g_warning ("map_drive error %d", errn);
>  }
>
> -  g_warning ("map_drive error %d", errn);
> -  return MAP_DRIVE_ERROR;
> +  return errn;
>  }
>
>  static void
> @@ -915,7 +910,7 @@ map_drive_cb(GTask *task,
>break;
>  }
>
> -  if (map_drive (drive_letter) != MAP_DRIVE_TRY_AGAIN)
> +  if (map_drive (drive_letter) != ERROR_ALREADY_ASSIGNED)
>  {
>break;
>  }
> --
> 2.21.0
>
> ___
> Spice-devel mailing list
> Spice-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel



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

Re: [Spice-devel] [PATCH phodav 08/13] spice-win: define SERVICE_NAME

2019-05-23 Thread Marc-André Lureau
On Thu, May 23, 2019 at 10:37 AM Jakub Janků  wrote:
>
> Signed-off-by: Jakub Janků 

ack

> ---
>  spice/spice-webdavd.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/spice/spice-webdavd.c b/spice/spice-webdavd.c
> index 681e909..c77c870 100644
> --- a/spice/spice-webdavd.c
> +++ b/spice/spice-webdavd.c
> @@ -31,6 +31,7 @@
>  #include 
>  #include 
>  #include 
> +#define SERVICE_NAME "spice-webdavd"
>  #endif
>
>  #ifdef WITH_AVAHI
> @@ -1062,7 +1063,7 @@ service_main (DWORD argc, TCHAR *argv[])
>g_mutex_init(_data.mutex);
>
>service_status_handle =
> -RegisterServiceCtrlHandlerEx ("spice-webdavd", service_ctrl_handler, 
> _data);
> +RegisterServiceCtrlHandlerEx (SERVICE_NAME, service_ctrl_handler, 
> _data);
>
>g_return_if_fail (service_status_handle != 0);
>
> @@ -1154,7 +1155,7 @@ main (int argc, char *argv[])
>
>SERVICE_TABLE_ENTRY service_table[] =
>  {
> -  { (char *)"spice-webdavd", service_main }, { NULL, NULL }
> +  { SERVICE_NAME, service_main }, { NULL, NULL }
>  };
>if (!no_service && !getenv("DEBUG"))
>  {
> --
> 2.21.0
>
> ___
> Spice-devel mailing list
> Spice-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel



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

Re: [Spice-devel] [PATCH phodav 07/13] spice: unref GSocketAddress

2019-05-23 Thread Marc-André Lureau
On Thu, May 23, 2019 at 10:37 AM Jakub Janků  wrote:
>
> No need to keep it around after calling
> g_socket_listener_add_address().
>
> Signed-off-by: Jakub Janků 

ack

> ---
>  spice/spice-webdavd.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/spice/spice-webdavd.c b/spice/spice-webdavd.c
> index d9b1fae..681e909 100644
> --- a/spice/spice-webdavd.c
> +++ b/spice/spice-webdavd.c
> @@ -1137,6 +1137,7 @@ main (int argc, char *argv[])
>   NULL,
>   NULL,
>   );
> +  g_object_unref (saddr);
>if (error)
>  {
>g_printerr ("%s\n", error->message);
> --
> 2.21.0
>
> ___
> Spice-devel mailing list
> Spice-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel



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

Re: [Spice-devel] [PATCH phodav 06/13] spice: print err when g_open fails

2019-05-23 Thread Marc-André Lureau
On Thu, May 23, 2019 at 10:37 AM Jakub Janků  wrote:
>
> Don't fail silently when we cannot open the webdav virtio port.
>
> Signed-off-by: Jakub Janků 

ack

> ---
>  spice/spice-webdavd.c | 5 +
>  1 file changed, 5 insertions(+)
>
> diff --git a/spice/spice-webdavd.c b/spice/spice-webdavd.c
> index 3fac28b..d9b1fae 100644
> --- a/spice/spice-webdavd.c
> +++ b/spice/spice-webdavd.c
> @@ -24,6 +24,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #endif
>
>  #ifdef G_OS_WIN32
> @@ -703,8 +704,12 @@ open_mux_path (const char *path)
>g_debug ("Open %s", path);
>  #ifdef G_OS_UNIX
>port_fd = g_open (path, O_RDWR);
> +  gint errsv = errno;
>if (port_fd == -1)
> +{
> +  g_printerr("Failed to open %s: %s\n", path, g_strerror(errsv));
>exit (1);
> +}
>
>wait_for_virtio_host (port_fd);
>
> --
> 2.21.0
>
> ___
> Spice-devel mailing list
> Spice-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel



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

Re: [Spice-devel] [PATCH phodav 05/13] spice: clear loop pointer on unref

2019-05-23 Thread Marc-André Lureau
On Thu, May 23, 2019 at 10:37 AM Jakub Janků  wrote:
>
> The pointer to loop must be set to NULL on unref.
>
> Quit signal handler can be called at any time,
> using g_main_loop_quit() on a freed loop could lead to segfault.

I am not sure if this is enough to solve the race you describe
(handler could still be called before loop is set to null, right?),
but the change seems to go in the right direction.

ack

>
> Signed-off-by: Jakub Janků 
> ---
>  spice/spice-webdavd.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/spice/spice-webdavd.c b/spice/spice-webdavd.c
> index 6ad63c5..3fac28b 100644
> --- a/spice/spice-webdavd.c
> +++ b/spice/spice-webdavd.c
> @@ -243,7 +243,8 @@ quit (int sig)
>if (sig == SIGINT || sig == SIGTERM)
>quit_service = TRUE;
>
> -  g_main_loop_quit (loop);
> +  if (loop)
> +g_main_loop_quit (loop);
>  }
>
>  static Client *
> @@ -982,7 +983,7 @@ run_service (ServiceData *service_data)
>
>start_mux_read (mux_istream);
>g_main_loop_run (loop);
> -  g_main_loop_unref (loop);
> +  g_clear_pointer (, g_main_loop_unref);
>
>  #ifdef G_OS_WIN32
>g_cancellable_cancel (map_drive_data.cancel_map);
> --
> 2.21.0
>
> ___
> Spice-devel mailing list
> Spice-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel



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

Re: [Spice-devel] [PATCH phodav 04/13] spice: quit service quickly

2019-05-23 Thread Marc-André Lureau
On Thu, May 23, 2019 at 10:37 AM Jakub Janků  wrote:
>
> We shouldn't wait for 1 second if the service is supposed to stop.
>
> Signed-off-by: Jakub Janků 

ack

> ---
>  spice/spice-webdavd.c | 15 +--
>  1 file changed, 9 insertions(+), 6 deletions(-)
>
> diff --git a/spice/spice-webdavd.c b/spice/spice-webdavd.c
> index cdfa73d..6ad63c5 100644
> --- a/spice/spice-webdavd.c
> +++ b/spice/spice-webdavd.c
> @@ -923,11 +923,15 @@ map_drive_cb(GTask *task,
>
>  #endif
>
> -static void
> +/* returns FALSE if the service should quit */
> +static gboolean
>  run_service (ServiceData *service_data)
>  {
>g_debug ("Run service");
>
> +  if (quit_service)
> +return FALSE;
> +
>  #ifdef G_OS_WIN32
>MapDriveData map_drive_data;
>map_drive_data.cancel_map = g_cancellable_new ();
> @@ -1003,6 +1007,7 @@ run_service (ServiceData *service_data)
>  #else
>close (port_fd);
>  #endif
> +  return !quit_service;
>  }
>
>  #ifdef G_OS_WIN32
> @@ -1064,9 +1069,8 @@ service_main (DWORD argc, TCHAR *argv[])
>service_status.dwWaitHint = 0;
>SetServiceStatus (service_status_handle, _status);
>
> -  while (!quit_service) {
> -  run_service (_data);
> -  g_usleep (G_USEC_PER_SEC);
> +  while (run_service(_data)) {
> +g_usleep(G_USEC_PER_SEC);
>}
>
>service_status.dwCurrentState = SERVICE_STOPPED;
> @@ -1154,8 +1158,7 @@ main (int argc, char *argv[])
>  }
>  } else
>  #endif
> -  while (!quit_service) {
> -run_service (_data);
> +  while (run_service(_data)) {
>  g_usleep (G_USEC_PER_SEC);
>}
>
> --
> 2.21.0
>
> ___
> Spice-devel mailing list
> Spice-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel



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

Re: [Spice-devel] [PATCH phodav 03/13] spice: handle SIGINT properly

2019-05-23 Thread Marc-André Lureau
Hi

On Thu, May 23, 2019 at 10:37 AM Jakub Janků  wrote:
>
> According to [0], g_debug should not be used in a signal handler.
> So, to avoid reentrancy, do not print debug message when quit is
> called with SIGINT.
>
> [0] 
> https://docs.microsoft.com/en-us/cpp/c-runtime-library/reference/signal?view=vs-2019
>
> Signed-off-by: Jakub Janků 
> ---
>  spice/spice-webdavd.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/spice/spice-webdavd.c b/spice/spice-webdavd.c
> index e494692..cdfa73d 100644
> --- a/spice/spice-webdavd.c
> +++ b/spice/spice-webdavd.c
> @@ -237,7 +237,8 @@ static void mdns_unregister_service (void);
>  static void
>  quit (int sig)
>  {
> -  g_debug ("quit %d", sig);
> +  if (sig != SIGINT)
> +g_debug ("quit %d", sig);
>

I would simply remove the g_debug() call then.

(maybe we should have a different function for the signal handler)

>if (sig == SIGINT || sig == SIGTERM)
>quit_service = TRUE;
> --
> 2.21.0
>
> ___
> Spice-devel mailing list
> Spice-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel



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

Re: [Spice-devel] [PATCH phodav 02/13] spice: rename my_input_channel

2019-05-23 Thread Marc-André Lureau
Hi

On Thu, May 23, 2019 at 10:37 AM Jakub Janků  wrote:
>
> my_input_channel_* isn't saying much,
> let's rename it to input_channel_*_thread,
> which is more fitting.
>
> Signed-off-by: Jakub Janků 
> ---
>  spice/spice-webdavd.c | 48 +--
>  1 file changed, 24 insertions(+), 24 deletions(-)
>
> diff --git a/spice/spice-webdavd.c b/spice/spice-webdavd.c
> index 81fe6be..e494692 100644
> --- a/spice/spice-webdavd.c
> +++ b/spice/spice-webdavd.c
> @@ -307,11 +307,11 @@ read_thread (GTask *task,
>
>data = g_task_get_task_data (task);
>
> -  g_debug ("my read %" G_GSIZE_FORMAT, data->count);
> +  g_debug ("thread read %" G_GSIZE_FORMAT, data->count);
>g_input_stream_read_all (stream,
> data->buffer, data->count, ,
> cancellable, );
> -  g_debug ("my read result %" G_GSIZE_FORMAT, bread);
> +  g_debug ("thread read result %" G_GSIZE_FORMAT, bread);
>
>if (error)
>  {
> @@ -329,13 +329,13 @@ read_thread (GTask *task,
>  }
>
>  static void
> -my_input_stream_read_async (GInputStream   *stream,
> -void   *buffer,
> -gsize   count,
> -int io_priority,
> -GCancellable   *cancellable,
> -GAsyncReadyCallback callback,
> -gpointeruser_data)
> +input_stream_read_async_thread (GInputStream   *stream,
> +void   *buffer,
> +gsize   count,
> +int io_priority,
> +GCancellable   *cancellable,
> +GAsyncReadyCallback callback,
> +gpointeruser_data)

for consistency, keep the suffix _async (& _finish)

ack otherwise

>  {
>GTask *task;
>ReadData *data = g_new (ReadData, 1);
> @@ -351,9 +351,9 @@ my_input_stream_read_async (GInputStream   *stream,
>  }
>
>  static gssize
> -my_input_stream_read_finish (GInputStream *stream,
> - GAsyncResult *result,
> - GError  **error)
> +input_stream_read_thread_finish (GInputStream *stream,
> + GAsyncResult *result,
> + GError  **error)
>  {
>g_return_val_if_fail (g_task_is_valid (result, stream), -1);
>
> @@ -392,7 +392,7 @@ mux_data_read_cb (GObject  *source_object,
>GError *error = NULL;
>gssize size;
>
> -  size = my_input_stream_read_finish (G_INPUT_STREAM (source_object), res, 
> );
> +  size = input_stream_read_thread_finish (G_INPUT_STREAM (source_object), 
> res, );
>g_return_if_fail (size == demux.size);
>if (error)
>  {
> @@ -422,13 +422,13 @@ mux_size_read_cb (GObject  *source_object,
>GError *error = NULL;
>gssize size;
>
> -  size = my_input_stream_read_finish (G_INPUT_STREAM (source_object), res, 
> );
> +  size = input_stream_read_thread_finish (G_INPUT_STREAM (source_object), 
> res, );
>if (error || size != sizeof (guint16))
>  goto end;
>
> -  my_input_stream_read_async (istream,
> -  , demux.size, G_PRIORITY_DEFAULT,
> -  cancel, mux_data_read_cb, NULL);
> +  input_stream_read_async_thread (istream,
> +  , demux.size, G_PRIORITY_DEFAULT,
> +  cancel, mux_data_read_cb, NULL);
>return;
>
>  end:
> @@ -450,13 +450,13 @@ mux_client_read_cb (GObject  *source_object,
>GError *error = NULL;
>gssize size;
>
> -  size = my_input_stream_read_finish (G_INPUT_STREAM (source_object), res, 
> );
> +  size = input_stream_read_thread_finish (G_INPUT_STREAM (source_object), 
> res, );
>g_debug ("read %" G_GSSIZE_FORMAT, size);
>if (error || size != sizeof (gint64))
>  goto end;
> -  my_input_stream_read_async (istream,
> -  , sizeof (guint16), 
> G_PRIORITY_DEFAULT,
> -  cancel, mux_size_read_cb, NULL);
> +  input_stream_read_async_thread (istream,
> +  , sizeof (guint16), 
> G_PRIORITY_DEFAULT,
> +  cancel, mux_size_read_cb, NULL);
>return;
>
>  end:
> @@ -475,9 +475,9 @@ end:
>  static void
>  start_mux_read (GInputStream *istream)
>  {
> -  my_input_stream_read_async (istream,
> -  , sizeof (gint64), 
> G_PRIORITY_DEFAULT,
> -  cancel, mux_client_read_cb, NULL);
> +  input_stream_read_async_thread (istream,
> +  , sizeof (gint64), 
> G_PRIORITY_DEFAULT,
> +  cancel, mux_client_read_cb, NULL);
>  }
>
>  static void client_start_read 

Re: [Spice-devel] [PATCH phodav 01/13] spice: remove G_SOURCE_{REMOVE, CONTINUE} definitions

2019-05-23 Thread Marc-André Lureau
On Thu, May 23, 2019 at 10:37 AM Jakub Janků  wrote:
>
> Other symbols that were added after G_SOURCE_{REMOVE, CONTINUE}
> are already being used in the rest of the file - e.g. g_task_new,
> so there's no need to define these.
>
> Signed-off-by: Jakub Janků 

ack,
we should bump glib dep to 2.36 at least (gtask)

> ---
>  spice/spice-webdavd.c | 7 ---
>  1 file changed, 7 deletions(-)
>
> diff --git a/spice/spice-webdavd.c b/spice/spice-webdavd.c
> index cd0029b..81fe6be 100644
> --- a/spice/spice-webdavd.c
> +++ b/spice/spice-webdavd.c
> @@ -665,13 +665,6 @@ mdns_state_changed (GaClient *client, GaClientState 
> state, gpointer user_data)
>  }
>  #endif
>
> -#ifndef G_SOURCE_REMOVE
> -#define G_SOURCE_REMOVE FALSE
> -#endif
> -#ifndef G_SOURCE_CONTINUE
> -#define G_SOURCE_CONTINUE TRUE
> -#endif
> -
>  #ifdef G_OS_UNIX
>  static void
>  wait_for_virtio_host (gint fd)
> --
> 2.21.0
>
> ___
> Spice-devel mailing list
> Spice-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel



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

Re: [Spice-devel] [PATCH v2 spice-gtk] Adjust to window scaling

2019-05-23 Thread Victor Toso
Hi,

On Thu, May 23, 2019 at 01:01:12PM +0300, Snir Sheriber wrote:
> Hi,
> 
> On 5/22/19 6:02 PM, Marc-André Lureau wrote:
> > Hi
> > 
> > On Sun, Mar 17, 2019 at 4:28 PM Snir Sheriber  wrote:
> > > When GDK_SCALE is != 1 and egl is used, the image presented does not
> > > fit to the window (scale of 2 is often used with hidpi monitors).
> > > Usually this is not a problem since all components are adjusted by
> > > gdk/gtk but with egl, pixel-based data is not being scaled. In this
> > > case window's scale value can be used in order to determine whether
> > > to use a pixel resource with higher resolution data.
> > > 
> > > In order to reproduce the problem set spice with virgl/Intel-vGPU
> > > and run spice-gtk with GDK_SCALE=2
> > > ---
> > > Changes from v1:
> > > -commit msg
> > > -replace var naming (ws with win_scale)
> > > 
> > > 
> > > This patch is kind of RFC, it fixes the issue, but it's a bit hacky
> > > and specific. I didn't come across other scale issues but it is likely
> > > that more of these exist and better and generic fix is needed.
> > > 
> > > ---
> > >   src/spice-widget-egl.c  | 15 +--
> > >   src/spice-widget-priv.h |  1 +
> > >   2 files changed, 14 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/src/spice-widget-egl.c b/src/spice-widget-egl.c
> > > index 43fccd7..600c87a 100644
> > > --- a/src/spice-widget-egl.c
> > > +++ b/src/spice-widget-egl.c
> > > @@ -326,6 +326,8 @@ static gboolean 
> > > spice_widget_init_egl_win(SpiceDisplay *display, GdkWindow *win,
> > >   if (d->egl.surface)
> > >   return TRUE;
> > > 
> > > +d->egl.scale = gdk_window_get_scale_factor(win);
> > Why not use gtk_widget_get_scale_factor() directly from
> > spice_egl_resize_display?
> 
> There is no special objection for that, just because i adjust
> scaling also in spice_egl_update_display and i assumed scaling
> is not being changed frequently.

But it can be changed, right? In that case, d->egl.scale would
not have the right value in spice_egl_resize_display()

Note that user tested that this patches fixes so I'd add that to
commit log:

https://gitlab.freedesktop.org/spice/spice-gtk/issues/99

Cheers,

> Snir.
> 
> > > +
> > >   #ifdef GDK_WINDOWING_X11
> > >   if (GDK_IS_X11_WINDOW(win)) {
> > >   native = (EGLNativeWindowType)GDK_WINDOW_XID(win);
> > > @@ -431,15 +433,17 @@ void spice_egl_resize_display(SpiceDisplay 
> > > *display, int w, int h)
> > >   {
> > >   SpiceDisplayPrivate *d = display->priv;
> > >   int prog;
> > > +gint win_scale;
> > > 
> > >   if (!gl_make_current(display, NULL))
> > >   return;
> > > 
> > > +win_scale = d->egl.scale;
> > >   glGetIntegerv(GL_CURRENT_PROGRAM, );
> > > 
> > >   glUseProgram(d->egl.prog);
> > > -apply_ortho(d->egl.mproj, 0, w, 0, h, -1, 1);
> > > -glViewport(0, 0, w, h);
> > > +apply_ortho(d->egl.mproj, 0, w * win_scale , 0, h * win_scale, -1, 
> > > 1);
> > > +glViewport(0, 0, w * win_scale, h * win_scale);
> > > 
> > >   if (d->ready)
> > >   spice_egl_update_display(display);
> > > @@ -559,6 +563,13 @@ void spice_egl_update_display(SpiceDisplay *display)
> > > 
> > >   spice_display_get_scaling(display, , , , , );
> > > 
> > > +// Adjust to gdk scale
> > > +s *= d->egl.scale;
> > > +x *= d->egl.scale;
> > > +y *= d->egl.scale;
> > > +w *= d->egl.scale;
> > > +h *= d->egl.scale;
> > > +
> > >   glClearColor(0.0f, 0.0f, 0.0f, 0.0f);
> > >   glClear(GL_COLOR_BUFFER_BIT);
> > > 
> > > diff --git a/src/spice-widget-priv.h b/src/spice-widget-priv.h
> > > index 65eb404..8f110ac 100644
> > > --- a/src/spice-widget-priv.h
> > > +++ b/src/spice-widget-priv.h
> > > @@ -149,6 +149,7 @@ struct _SpiceDisplayPrivate {
> > >   EGLImageKHR image;
> > >   gbooleancall_draw_done;
> > >   SpiceGlScanout  scanout;
> > > +gintscale;
> > >   } egl;
> > >   #endif // HAVE_EGL
> > >   double scroll_delta_y;
> > > --
> > > 2.19.1
> > > 
> > > ___
> > > 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


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 spice-gtk] Adjust to window scaling

2019-05-23 Thread Snir Sheriber

Hi,

On 5/22/19 6:02 PM, Marc-André Lureau wrote:

Hi

On Sun, Mar 17, 2019 at 4:28 PM Snir Sheriber  wrote:

When GDK_SCALE is != 1 and egl is used, the image presented does not
fit to the window (scale of 2 is often used with hidpi monitors).
Usually this is not a problem since all components are adjusted by
gdk/gtk but with egl, pixel-based data is not being scaled. In this
case window's scale value can be used in order to determine whether
to use a pixel resource with higher resolution data.

In order to reproduce the problem set spice with virgl/Intel-vGPU
and run spice-gtk with GDK_SCALE=2
---
Changes from v1:
-commit msg
-replace var naming (ws with win_scale)


This patch is kind of RFC, it fixes the issue, but it's a bit hacky
and specific. I didn't come across other scale issues but it is likely
that more of these exist and better and generic fix is needed.

---
  src/spice-widget-egl.c  | 15 +--
  src/spice-widget-priv.h |  1 +
  2 files changed, 14 insertions(+), 2 deletions(-)

diff --git a/src/spice-widget-egl.c b/src/spice-widget-egl.c
index 43fccd7..600c87a 100644
--- a/src/spice-widget-egl.c
+++ b/src/spice-widget-egl.c
@@ -326,6 +326,8 @@ static gboolean spice_widget_init_egl_win(SpiceDisplay 
*display, GdkWindow *win,
  if (d->egl.surface)
  return TRUE;

+d->egl.scale = gdk_window_get_scale_factor(win);

Why not use gtk_widget_get_scale_factor() directly from
spice_egl_resize_display?



There is no special objection for that, just because i adjust scaling 
also in spice_egl_update_display

and i assumed scaling is not being changed frequently.


Snir.


+
  #ifdef GDK_WINDOWING_X11
  if (GDK_IS_X11_WINDOW(win)) {
  native = (EGLNativeWindowType)GDK_WINDOW_XID(win);
@@ -431,15 +433,17 @@ void spice_egl_resize_display(SpiceDisplay *display, int 
w, int h)
  {
  SpiceDisplayPrivate *d = display->priv;
  int prog;
+gint win_scale;

  if (!gl_make_current(display, NULL))
  return;

+win_scale = d->egl.scale;
  glGetIntegerv(GL_CURRENT_PROGRAM, );

  glUseProgram(d->egl.prog);
-apply_ortho(d->egl.mproj, 0, w, 0, h, -1, 1);
-glViewport(0, 0, w, h);
+apply_ortho(d->egl.mproj, 0, w * win_scale , 0, h * win_scale, -1, 1);
+glViewport(0, 0, w * win_scale, h * win_scale);

  if (d->ready)
  spice_egl_update_display(display);
@@ -559,6 +563,13 @@ void spice_egl_update_display(SpiceDisplay *display)

  spice_display_get_scaling(display, , , , , );

+// Adjust to gdk scale
+s *= d->egl.scale;
+x *= d->egl.scale;
+y *= d->egl.scale;
+w *= d->egl.scale;
+h *= d->egl.scale;
+
  glClearColor(0.0f, 0.0f, 0.0f, 0.0f);
  glClear(GL_COLOR_BUFFER_BIT);

diff --git a/src/spice-widget-priv.h b/src/spice-widget-priv.h
index 65eb404..8f110ac 100644
--- a/src/spice-widget-priv.h
+++ b/src/spice-widget-priv.h
@@ -149,6 +149,7 @@ struct _SpiceDisplayPrivate {
  EGLImageKHR image;
  gbooleancall_draw_done;
  SpiceGlScanout  scanout;
+gintscale;
  } egl;
  #endif // HAVE_EGL
  double scroll_delta_y;
--
2.19.1

___
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] use spice proxy

2019-05-23 Thread Victor Toso
Hi,

On Thu, May 23, 2019 at 03:38:32PM +0800, zap83q wrote:
> Hi,
>When I try to use proxy to connect spice server, it return a
>virt-viewer warning: "donnot allow http proxy connect", why?
> I do config like https://www.spice-space.org/spice-proxy.html

Could you please post:
- Spice-gtk/virt-viewer versions
- Are you on windows or linux client?
- Would be possible to run remote-viewer with --spice-debug and
  paste or attach those logs here?

Thanks,
Victor


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

[Spice-devel] use spice proxy

2019-05-23 Thread zap83q
Hi,
   When I try to use proxy to connect spice server, it return a virt-viewer 
warning: "donnot allow http proxy connect", why?
I do config like https://www.spice-space.org/spice-proxy.html___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

[Spice-devel] [PATCH phodav 03/13] spice: handle SIGINT properly

2019-05-23 Thread Jakub Janků
According to [0], g_debug should not be used in a signal handler.
So, to avoid reentrancy, do not print debug message when quit is
called with SIGINT.

[0] 
https://docs.microsoft.com/en-us/cpp/c-runtime-library/reference/signal?view=vs-2019

Signed-off-by: Jakub Janků 
---
 spice/spice-webdavd.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/spice/spice-webdavd.c b/spice/spice-webdavd.c
index e494692..cdfa73d 100644
--- a/spice/spice-webdavd.c
+++ b/spice/spice-webdavd.c
@@ -237,7 +237,8 @@ static void mdns_unregister_service (void);
 static void
 quit (int sig)
 {
-  g_debug ("quit %d", sig);
+  if (sig != SIGINT)
+g_debug ("quit %d", sig);
 
   if (sig == SIGINT || sig == SIGTERM)
   quit_service = TRUE;
-- 
2.21.0

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

[Spice-devel] [PATCH phodav 07/13] spice: unref GSocketAddress

2019-05-23 Thread Jakub Janků
No need to keep it around after calling
g_socket_listener_add_address().

Signed-off-by: Jakub Janků 
---
 spice/spice-webdavd.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/spice/spice-webdavd.c b/spice/spice-webdavd.c
index d9b1fae..681e909 100644
--- a/spice/spice-webdavd.c
+++ b/spice/spice-webdavd.c
@@ -1137,6 +1137,7 @@ main (int argc, char *argv[])
  NULL,
  NULL,
  );
+  g_object_unref (saddr);
   if (error)
 {
   g_printerr ("%s\n", error->message);
-- 
2.21.0

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

[Spice-devel] [PATCH phodav 02/13] spice: rename my_input_channel

2019-05-23 Thread Jakub Janků
my_input_channel_* isn't saying much,
let's rename it to input_channel_*_thread,
which is more fitting.

Signed-off-by: Jakub Janků 
---
 spice/spice-webdavd.c | 48 +--
 1 file changed, 24 insertions(+), 24 deletions(-)

diff --git a/spice/spice-webdavd.c b/spice/spice-webdavd.c
index 81fe6be..e494692 100644
--- a/spice/spice-webdavd.c
+++ b/spice/spice-webdavd.c
@@ -307,11 +307,11 @@ read_thread (GTask *task,
 
   data = g_task_get_task_data (task);
 
-  g_debug ("my read %" G_GSIZE_FORMAT, data->count);
+  g_debug ("thread read %" G_GSIZE_FORMAT, data->count);
   g_input_stream_read_all (stream,
data->buffer, data->count, ,
cancellable, );
-  g_debug ("my read result %" G_GSIZE_FORMAT, bread);
+  g_debug ("thread read result %" G_GSIZE_FORMAT, bread);
 
   if (error)
 {
@@ -329,13 +329,13 @@ read_thread (GTask *task,
 }
 
 static void
-my_input_stream_read_async (GInputStream   *stream,
-void   *buffer,
-gsize   count,
-int io_priority,
-GCancellable   *cancellable,
-GAsyncReadyCallback callback,
-gpointeruser_data)
+input_stream_read_async_thread (GInputStream   *stream,
+void   *buffer,
+gsize   count,
+int io_priority,
+GCancellable   *cancellable,
+GAsyncReadyCallback callback,
+gpointeruser_data)
 {
   GTask *task;
   ReadData *data = g_new (ReadData, 1);
@@ -351,9 +351,9 @@ my_input_stream_read_async (GInputStream   *stream,
 }
 
 static gssize
-my_input_stream_read_finish (GInputStream *stream,
- GAsyncResult *result,
- GError  **error)
+input_stream_read_thread_finish (GInputStream *stream,
+ GAsyncResult *result,
+ GError  **error)
 {
   g_return_val_if_fail (g_task_is_valid (result, stream), -1);
 
@@ -392,7 +392,7 @@ mux_data_read_cb (GObject  *source_object,
   GError *error = NULL;
   gssize size;
 
-  size = my_input_stream_read_finish (G_INPUT_STREAM (source_object), res, 
);
+  size = input_stream_read_thread_finish (G_INPUT_STREAM (source_object), res, 
);
   g_return_if_fail (size == demux.size);
   if (error)
 {
@@ -422,13 +422,13 @@ mux_size_read_cb (GObject  *source_object,
   GError *error = NULL;
   gssize size;
 
-  size = my_input_stream_read_finish (G_INPUT_STREAM (source_object), res, 
);
+  size = input_stream_read_thread_finish (G_INPUT_STREAM (source_object), res, 
);
   if (error || size != sizeof (guint16))
 goto end;
 
-  my_input_stream_read_async (istream,
-  , demux.size, G_PRIORITY_DEFAULT,
-  cancel, mux_data_read_cb, NULL);
+  input_stream_read_async_thread (istream,
+  , demux.size, G_PRIORITY_DEFAULT,
+  cancel, mux_data_read_cb, NULL);
   return;
 
 end:
@@ -450,13 +450,13 @@ mux_client_read_cb (GObject  *source_object,
   GError *error = NULL;
   gssize size;
 
-  size = my_input_stream_read_finish (G_INPUT_STREAM (source_object), res, 
);
+  size = input_stream_read_thread_finish (G_INPUT_STREAM (source_object), res, 
);
   g_debug ("read %" G_GSSIZE_FORMAT, size);
   if (error || size != sizeof (gint64))
 goto end;
-  my_input_stream_read_async (istream,
-  , sizeof (guint16), 
G_PRIORITY_DEFAULT,
-  cancel, mux_size_read_cb, NULL);
+  input_stream_read_async_thread (istream,
+  , sizeof (guint16), 
G_PRIORITY_DEFAULT,
+  cancel, mux_size_read_cb, NULL);
   return;
 
 end:
@@ -475,9 +475,9 @@ end:
 static void
 start_mux_read (GInputStream *istream)
 {
-  my_input_stream_read_async (istream,
-  , sizeof (gint64), 
G_PRIORITY_DEFAULT,
-  cancel, mux_client_read_cb, NULL);
+  input_stream_read_async_thread (istream,
+  , sizeof (gint64), 
G_PRIORITY_DEFAULT,
+  cancel, mux_client_read_cb, NULL);
 }
 
 static void client_start_read (Client *client);
-- 
2.21.0

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

[Spice-devel] [PATCH phodav 06/13] spice: print err when g_open fails

2019-05-23 Thread Jakub Janků
Don't fail silently when we cannot open the webdav virtio port.

Signed-off-by: Jakub Janků 
---
 spice/spice-webdavd.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/spice/spice-webdavd.c b/spice/spice-webdavd.c
index 3fac28b..d9b1fae 100644
--- a/spice/spice-webdavd.c
+++ b/spice/spice-webdavd.c
@@ -24,6 +24,7 @@
 #include 
 #include 
 #include 
+#include 
 #endif
 
 #ifdef G_OS_WIN32
@@ -703,8 +704,12 @@ open_mux_path (const char *path)
   g_debug ("Open %s", path);
 #ifdef G_OS_UNIX
   port_fd = g_open (path, O_RDWR);
+  gint errsv = errno;
   if (port_fd == -1)
+{
+  g_printerr("Failed to open %s: %s\n", path, g_strerror(errsv));
   exit (1);
+}
 
   wait_for_virtio_host (port_fd);
 
-- 
2.21.0

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

[Spice-devel] [PATCH phodav 11/13] spice: move OutputQueue to file

2019-05-23 Thread Jakub Janků
OutputQueue is a self-contained unit and as such can be put in
a separate file to make the spice-webdavd.c less cluttered.

Also, as the current implementation defines output_queue_{ref, unref},
turn OutputQueue into a GObject which can handle these for us.

Signed-off-by: Jakub Janků 
---
 spice/meson.build |   8 ++-
 spice/output-queue.c  | 164 ++
 spice/output-queue.h  |  38 ++
 spice/spice-webdavd.c | 162 ++---
 4 files changed, 214 insertions(+), 158 deletions(-)
 create mode 100644 spice/output-queue.c
 create mode 100644 spice/output-queue.h

diff --git a/spice/meson.build b/spice/meson.build
index 6db22cc..06d20e6 100644
--- a/spice/meson.build
+++ b/spice/meson.build
@@ -4,9 +4,15 @@ if host_machine.system() == 'windows'
   win32_deps += compiler.find_library('mpr')
 endif
 
+sources = [
+  'spice-webdavd.c',
+  'output-queue.c',
+  'output-queue.h'
+]
+
 executable(
   'spice-webdavd',
-  [ 'spice-webdavd.c' ],
+  sources,
   install_dir : sbindir,
   include_directories : incdir,
   dependencies : win32_deps + avahi_deps + deps,
diff --git a/spice/output-queue.c b/spice/output-queue.c
new file mode 100644
index 000..6991493
--- /dev/null
+++ b/spice/output-queue.c
@@ -0,0 +1,164 @@
+/*
+ * Copyright (C) 2019 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 .
+ */
+#include 
+
+#include "output-queue.h"
+
+typedef struct _OutputQueueElem
+{
+  OutputQueue  *queue;
+  const guint8 *buf;
+  gsize size;
+  PushedCb  cb;
+  gpointer  user_data;
+} OutputQueueElem;
+
+struct _OutputQueue
+{
+  GObjectparent_instance;
+  GOutputStream *output;
+  gboolean   flushing;
+  guint  idle_id;
+  GQueue*queue;
+  GCancellable  *cancel;
+};
+
+G_DEFINE_TYPE(OutputQueue, output_queue, G_TYPE_OBJECT);
+
+static void output_queue_init(OutputQueue *self)
+{
+self->queue = g_queue_new ();
+}
+
+static void output_queue_finalize(GObject *obj)
+{
+OutputQueue *self = OUTPUT_QUEUE(obj);
+
+g_warn_if_fail (g_queue_get_length (self->queue) == 0);
+g_warn_if_fail (!self->flushing);
+g_warn_if_fail (!self->idle_id);
+
+g_queue_free_full (self->queue, g_free);
+g_object_unref (self->output);
+g_object_unref (self->cancel);
+
+G_OBJECT_CLASS(output_queue_parent_class)->finalize(obj);
+}
+
+static void output_queue_class_init(OutputQueueClass *klass)
+{
+GObjectClass *gobject_class = G_OBJECT_CLASS(klass);
+gobject_class->finalize = output_queue_finalize;
+}
+
+OutputQueue* output_queue_new (GOutputStream *output, GCancellable *cancel)
+{
+  OutputQueue *self = g_object_new(OUTPUT_TYPE_QUEUE, NULL);
+  self->output = g_object_ref (output);
+  self->cancel = g_object_ref (cancel);
+  return self;
+}
+
+static gboolean output_queue_idle (gpointer user_data);
+
+static void
+output_queue_flush_cb (GObject  *source_object,
+   GAsyncResult *res,
+   gpointer  user_data)
+{
+  GError *error = NULL;
+  OutputQueueElem *e = user_data;
+  OutputQueue *q = e->queue;
+
+  g_debug ("flushed");
+  q->flushing = FALSE;
+  g_output_stream_flush_finish (G_OUTPUT_STREAM (source_object),
+res, );
+  if (error)
+g_warning ("error: %s", error->message);
+
+  g_clear_error ();
+
+  if (!q->idle_id)
+q->idle_id = g_idle_add (output_queue_idle, g_object_ref (q));
+
+  g_free (e);
+  g_object_unref (q);
+}
+
+static gboolean
+output_queue_idle (gpointer user_data)
+{
+  OutputQueue *q = user_data;
+  OutputQueueElem *e = NULL;
+  GError *error = NULL;
+
+  if (q->flushing)
+{
+  g_debug ("already flushing");
+  goto end;
+}
+
+  e = g_queue_pop_head (q->queue);
+  if (!e)
+{
+  g_debug ("No more data to flush");
+  goto end;
+}
+
+  g_debug ("flushing %" G_GSIZE_FORMAT, e->size);
+  g_output_stream_write_all (q->output, e->buf, e->size, NULL, q->cancel, 
);
+  if (e->cb)
+e->cb (q, e->user_data, error);
+
+  if (error)
+  goto end;
+
+  q->flushing = TRUE;
+  g_output_stream_flush_async (q->output, G_PRIORITY_DEFAULT, q->cancel, 
output_queue_flush_cb, e);
+
+  q->idle_id = 0;
+  return FALSE;
+
+end:
+  g_clear_error ();
+  q->idle_id = 0;
+  g_free (e);
+  g_object_unref (q);
+
+  return FALSE;
+}
+
+void

[Spice-devel] [PATCH phodav 04/13] spice: quit service quickly

2019-05-23 Thread Jakub Janků
We shouldn't wait for 1 second if the service is supposed to stop.

Signed-off-by: Jakub Janků 
---
 spice/spice-webdavd.c | 15 +--
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/spice/spice-webdavd.c b/spice/spice-webdavd.c
index cdfa73d..6ad63c5 100644
--- a/spice/spice-webdavd.c
+++ b/spice/spice-webdavd.c
@@ -923,11 +923,15 @@ map_drive_cb(GTask *task,
 
 #endif
 
-static void
+/* returns FALSE if the service should quit */
+static gboolean
 run_service (ServiceData *service_data)
 {
   g_debug ("Run service");
 
+  if (quit_service)
+return FALSE;
+
 #ifdef G_OS_WIN32
   MapDriveData map_drive_data;
   map_drive_data.cancel_map = g_cancellable_new ();
@@ -1003,6 +1007,7 @@ run_service (ServiceData *service_data)
 #else
   close (port_fd);
 #endif
+  return !quit_service;
 }
 
 #ifdef G_OS_WIN32
@@ -1064,9 +1069,8 @@ service_main (DWORD argc, TCHAR *argv[])
   service_status.dwWaitHint = 0;
   SetServiceStatus (service_status_handle, _status);
 
-  while (!quit_service) {
-  run_service (_data);
-  g_usleep (G_USEC_PER_SEC);
+  while (run_service(_data)) {
+g_usleep(G_USEC_PER_SEC);
   }
 
   service_status.dwCurrentState = SERVICE_STOPPED;
@@ -1154,8 +1158,7 @@ main (int argc, char *argv[])
 }
 } else
 #endif
-  while (!quit_service) {
-run_service (_data);
+  while (run_service(_data)) {
 g_usleep (G_USEC_PER_SEC);
   }
 
-- 
2.21.0

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

[Spice-devel] [PATCH phodav 00/13] Miscellaneous series

2019-05-23 Thread Jakub Janků
Hi,

this series contains mostly fixes of some things 
that I came across while reading the spice-webdavd.c code,
without any order.

Cheers,

Jakub Janků (13):
  spice: remove G_SOURCE_{REMOVE,CONTINUE} definitions
  spice: rename my_input_channel
  spice: handle SIGINT properly
  spice: quit service quickly
  spice: clear loop pointer on unref
  spice: print err when g_open fails
  spice: unref GSocketAddress
  spice-win: define SERVICE_NAME
  spice-win: remove MapDriveEnum
  spice-win: don't assign drive_letter on error
  spice: move OutputQueue to file
  rename README to README.md
  rename NEWS to NEWS.md

 NEWS => NEWS.md   |   0
 README => README.md   |  21 ++--
 spice/meson.build |   8 +-
 spice/output-queue.c  | 164 +
 spice/output-queue.h  |  38 ++
 spice/spice-webdavd.c | 280 ++
 6 files changed, 286 insertions(+), 225 deletions(-)
 rename NEWS => NEWS.md (100%)
 rename README => README.md (56%)
 create mode 100644 spice/output-queue.c
 create mode 100644 spice/output-queue.h

-- 
2.21.0

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

[Spice-devel] [PATCH phodav 12/13] rename README to README.md

2019-05-23 Thread Jakub Janků
The filename needs the proper extension for GitLab to handle
the file as a Markdown file.

Fix some style issues so that the file renders correctly.

Signed-off-by: Jakub Janků 
---
 README => README.md | 21 ++---
 1 file changed, 10 insertions(+), 11 deletions(-)
 rename README => README.md (56%)

diff --git a/README b/README.md
similarity index 56%
rename from README
rename to README.md
index fa82acd..297756f 100644
--- a/README
+++ b/README.md
@@ -1,31 +1,30 @@
-phởdav
-==
+# phởdav
 
-= About
+## About
 
 phởdav is a WebDav server implementation using libsoup (RFC 4918).
 
-Releases: http://ftp.gnome.org/pub/GNOME/sources/phodav/
+Releases: http://ftp.gnome.org/pub/GNOME/sources/phodav/
 
-= Tools
+## Tools
 
-chezdav, allows to share a particual directory (with optional
-digest authentication)
+* chezdav, allows to share a particual directory (with optional
+  digest authentication)
 
-= Notes
+## Notes
 
 phởdav was initially developped as a filesharing mechanism for Spice,
 but it is generic enough to be reused in other projects, in particular
 in the GNOME desktop. Further integration work would be a welcome
 contribution!
 
-= Contributing
+## Contributing
 
 git clone git://git.gnome.org/phodav
 
-http://lists.freedesktop.org/mailman/listinfo/spice-devel
+http://lists.freedesktop.org/mailman/listinfo/spice-devel
 
 Please report bug there:
 https://bugzilla.gnome.org/enter_bug.cgi?product=phodav
 
-Forking and sending github pull requests is also welcome.
\ No newline at end of file
+Forking and sending pull requests is also welcome.
\ No newline at end of file
-- 
2.21.0

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

[Spice-devel] [PATCH phodav 13/13] rename NEWS to NEWS.md

2019-05-23 Thread Jakub Janků
The filename needs the proper extension for GitLab to handle
the file as a Markdown file.

Signed-off-by: Jakub Janků 
---
 NEWS => NEWS.md | 0
 1 file changed, 0 insertions(+), 0 deletions(-)
 rename NEWS => NEWS.md (100%)

diff --git a/NEWS b/NEWS.md
similarity index 100%
rename from NEWS
rename to NEWS.md
-- 
2.21.0

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

[Spice-devel] [PATCH phodav 05/13] spice: clear loop pointer on unref

2019-05-23 Thread Jakub Janků
The pointer to loop must be set to NULL on unref.

Quit signal handler can be called at any time,
using g_main_loop_quit() on a freed loop could lead to segfault.

Signed-off-by: Jakub Janků 
---
 spice/spice-webdavd.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/spice/spice-webdavd.c b/spice/spice-webdavd.c
index 6ad63c5..3fac28b 100644
--- a/spice/spice-webdavd.c
+++ b/spice/spice-webdavd.c
@@ -243,7 +243,8 @@ quit (int sig)
   if (sig == SIGINT || sig == SIGTERM)
   quit_service = TRUE;
 
-  g_main_loop_quit (loop);
+  if (loop)
+g_main_loop_quit (loop);
 }
 
 static Client *
@@ -982,7 +983,7 @@ run_service (ServiceData *service_data)
 
   start_mux_read (mux_istream);
   g_main_loop_run (loop);
-  g_main_loop_unref (loop);
+  g_clear_pointer (, g_main_loop_unref);
 
 #ifdef G_OS_WIN32
   g_cancellable_cancel (map_drive_data.cancel_map);
-- 
2.21.0

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

[Spice-devel] [PATCH phodav 01/13] spice: remove G_SOURCE_{REMOVE, CONTINUE} definitions

2019-05-23 Thread Jakub Janků
Other symbols that were added after G_SOURCE_{REMOVE, CONTINUE}
are already being used in the rest of the file - e.g. g_task_new,
so there's no need to define these.

Signed-off-by: Jakub Janků 
---
 spice/spice-webdavd.c | 7 ---
 1 file changed, 7 deletions(-)

diff --git a/spice/spice-webdavd.c b/spice/spice-webdavd.c
index cd0029b..81fe6be 100644
--- a/spice/spice-webdavd.c
+++ b/spice/spice-webdavd.c
@@ -665,13 +665,6 @@ mdns_state_changed (GaClient *client, GaClientState state, 
gpointer user_data)
 }
 #endif
 
-#ifndef G_SOURCE_REMOVE
-#define G_SOURCE_REMOVE FALSE
-#endif
-#ifndef G_SOURCE_CONTINUE
-#define G_SOURCE_CONTINUE TRUE
-#endif
-
 #ifdef G_OS_UNIX
 static void
 wait_for_virtio_host (gint fd)
-- 
2.21.0

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

[Spice-devel] [PATCH phodav 09/13] spice-win: remove MapDriveEnum

2019-05-23 Thread Jakub Janků
The enum doesn't add much value, let's remove it.

Signed-off-by: Jakub Janků 
---
 spice/spice-webdavd.c | 19 +++
 1 file changed, 7 insertions(+), 12 deletions(-)

diff --git a/spice/spice-webdavd.c b/spice/spice-webdavd.c
index c77c870..29058e7 100644
--- a/spice/spice-webdavd.c
+++ b/spice/spice-webdavd.c
@@ -738,12 +738,6 @@ open_mux_path (const char *path)
 #ifdef G_OS_WIN32
 #define MAX_SHARED_FOLDER_NAME_SIZE 64
 #define MAX_DRIVE_LETTER_SIZE 3
-typedef enum _MapDriveEnum
-{
-  MAP_DRIVE_OK,
-  MAP_DRIVE_TRY_AGAIN,
-  MAP_DRIVE_ERROR
-} MapDriveEnum;
 
 typedef struct _MapDriveData
 {
@@ -828,7 +822,7 @@ netresource_free(NETRESOURCE *net_resource)
   g_free(net_resource->lpRemoteName);
 }
 
-static MapDriveEnum
+static guint32
 map_drive(const gchar drive_letter)
 {
   NETRESOURCE net_resource;
@@ -841,16 +835,17 @@ map_drive(const gchar drive_letter)
   if (errn == NO_ERROR)
 {
   g_debug ("Shared folder mapped to %c succesfully", drive_letter);
-  return MAP_DRIVE_OK;
 }
   else if (errn == ERROR_ALREADY_ASSIGNED)
 {
   g_debug ("Drive letter %c is already assigned", drive_letter);
-  return MAP_DRIVE_TRY_AGAIN;
+}
+  else
+{
+  g_warning ("map_drive error %d", errn);
 }
 
-  g_warning ("map_drive error %d", errn);
-  return MAP_DRIVE_ERROR;
+  return errn;
 }
 
 static void
@@ -915,7 +910,7 @@ map_drive_cb(GTask *task,
   break;
 }
 
-  if (map_drive (drive_letter) != MAP_DRIVE_TRY_AGAIN)
+  if (map_drive (drive_letter) != ERROR_ALREADY_ASSIGNED)
 {
   break;
 }
-- 
2.21.0

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

[Spice-devel] [PATCH phodav 08/13] spice-win: define SERVICE_NAME

2019-05-23 Thread Jakub Janků
Signed-off-by: Jakub Janků 
---
 spice/spice-webdavd.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/spice/spice-webdavd.c b/spice/spice-webdavd.c
index 681e909..c77c870 100644
--- a/spice/spice-webdavd.c
+++ b/spice/spice-webdavd.c
@@ -31,6 +31,7 @@
 #include 
 #include 
 #include 
+#define SERVICE_NAME "spice-webdavd"
 #endif
 
 #ifdef WITH_AVAHI
@@ -1062,7 +1063,7 @@ service_main (DWORD argc, TCHAR *argv[])
   g_mutex_init(_data.mutex);
 
   service_status_handle =
-RegisterServiceCtrlHandlerEx ("spice-webdavd", service_ctrl_handler, 
_data);
+RegisterServiceCtrlHandlerEx (SERVICE_NAME, service_ctrl_handler, 
_data);
 
   g_return_if_fail (service_status_handle != 0);
 
@@ -1154,7 +1155,7 @@ main (int argc, char *argv[])
 
   SERVICE_TABLE_ENTRY service_table[] =
 {
-  { (char *)"spice-webdavd", service_main }, { NULL, NULL }
+  { SERVICE_NAME, service_main }, { NULL, NULL }
 };
   if (!no_service && !getenv("DEBUG"))
 {
-- 
2.21.0

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

[Spice-devel] [PATCH phodav 10/13] spice-win: don't assign drive_letter on error

2019-05-23 Thread Jakub Janků
drive_letter should be assigned only when the mapping was successful.

Signed-off-by: Jakub Janků 
---
 spice/spice-webdavd.c | 12 +---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/spice/spice-webdavd.c b/spice/spice-webdavd.c
index 29058e7..f2c7f07 100644
--- a/spice/spice-webdavd.c
+++ b/spice/spice-webdavd.c
@@ -910,14 +910,20 @@ map_drive_cb(GTask *task,
   break;
 }
 
-  if (map_drive (drive_letter) != ERROR_ALREADY_ASSIGNED)
+  ret = map_drive (drive_letter);
+  if (ret == ERROR_ALREADY_ASSIGNED)
 {
-  break;
+  /* try again with another letter */
+  continue;
+}
+  if (ret != NO_ERROR)
+{
+  drive_letter = 0;
 }
+  break;
   //TODO: After mapping, rename network drive from 
\\localhost@PORT\DavWWWRoot
   //  to something like SPICE Shared Folder
 }
-
   g_mutex_lock(_drive_data->service_data->mutex);
   map_drive_data->service_data->drive_letter = drive_letter;
   g_mutex_unlock(_drive_data->service_data->mutex);
-- 
2.21.0

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