On Wed, Sep 27, 2017 at 12:44 PM Frediano Ziglio <fziglio@xxxxxxxxxx> wrote:
>
> From: Victor Toso <me@xxxxxxxxxxxxxx>
>
> This patch export two existing functions `udscs_do_read()` and
> `udscs_do_write()` and also creates a new one `udscs_client_get_fd()`.
>
> The intention of this functions is to allow vdagent to check if
> connection's socket is ready to read or write. This will be done
> together with GMainLoop integration in a followup patch.
>
> Signed-off-by: Victor Toso <victortoso@xxxxxxxxxx>
This patch starts exporting lot of information from udscs and
the event handling is done half in vdagent and half in udscs
code creating a circular dependency from some component to vdagent.
Would not be better if udscs know GLib (we already are including
glib.h in this patch) context/loop and deal directly with it?
Just pass a GMainLoop building udscs and move the source in
udscs. In this way in 11/11 you won't have to change all udscs_write
calls and include vdagent.h.
OK, so what you are proposing is:
- add optional GMainLoop argument to udscs_connect()
- move g_io_add_watch(G_IO_IN) to udscs_connect()
- move conn_channel and conn_out_source_id from VDAgent object to udscs_connection struct
- move conn_channel_io_callback() to udscs
- move g_io_add_watch(G_IO_OUT) from spice_vdagent_write_msg() to udscs_write()
and remove spice_vdagent_write_msg()
I think that could work, the only problem I see is that udscs_write() takes udscs_connection * as argument.
We have to call g_io_add_watch(G_IO_OUT) in udscs_write(), passing udscs_connection ** so that
when conn_channel_io_cb() is called, it can destroy the connection.
So we would have to add udscs_connection ** in the udscs_connection struct, or
develop a mechanism to determine whether the GMainLoop was quit by udscs.c or not.
Is that correct?
Potentially you can also avoit 6/11 entirely.
How? The individual VDAGENTD_ messages should be handled in vdagent.c
Since we removed most of the global variables, we need to pass the VDAgent object
to daemon_read_complete().
> ---
> src/udscs.c | 17 +++++++++--------
> src/udscs.h | 17 +++++++++++++++++
> 2 files changed, 26 insertions(+), 8 deletions(-)
>
> diff --git a/src/udscs.c b/src/udscs.c
> index f67d0a0..2761cbb 100644
> --- a/src/udscs.c
> +++ b/src/udscs.c
> @@ -31,6 +31,7 @@
> #include <errno.h>
> #include <sys/socket.h>
> #include <sys/un.h>
> +#include <glib.h>
> #include "udscs.h"
>
> struct udscs_buf {
> @@ -246,8 +247,7 @@ static void udscs_read_complete(struct udscs_connection
> **connp)
> conn->header_read = 0;
> }
>
> -/* A helper for udscs_client_handle_fds() */
> -static void udscs_do_read(struct udscs_connection **connp)
> +void udscs_do_read(struct udscs_connection **connp)
> {
> ssize_t n;
> size_t to_read;
> @@ -298,19 +298,15 @@ static void udscs_do_read(struct udscs_connection
> **connp)
> }
>
> /* A helper for udscs_client_handle_fds() */
> -static void udscs_do_write(struct udscs_connection **connp)
> +void udscs_do_write(struct udscs_connection **connp)
> {
> ssize_t n;
> size_t to_write;
> struct udscs_connection *conn = *connp;
>
> struct udscs_buf* wbuf = conn->write_buf;
> - if (!wbuf) {
> - syslog(LOG_ERR,
> - "%p do_write called on a connection without a write buf ?!",
> - conn);
> + if (!wbuf)
> return;
> - }
>
> to_write = wbuf->size - wbuf->pos;
> n = write(conn->fd, wbuf->buf + wbuf->pos, to_write);
> @@ -357,6 +353,11 @@ int udscs_client_fill_fds(struct udscs_connection *conn,
> fd_set *readfds,
> return conn->fd + 1;
> }
>
> +int udscs_client_get_fd(struct udscs_connection *conn)
> +{
> + g_return_val_if_fail(conn != NULL, -1);
> + return conn->fd;
> +}
>
> #ifndef UDSCS_NO_SERVER
>
> diff --git a/src/udscs.h b/src/udscs.h
> index 04377ba..08e71c8 100644
> --- a/src/udscs.h
> +++ b/src/udscs.h
> @@ -109,6 +109,23 @@ void udscs_set_user_data(struct udscs_connection *conn,
> void *data);
> */
> void *udscs_get_user_data(struct udscs_connection *conn);
>
> +/* Get fd from connection or return -1 if NULL */
> +int udscs_client_get_fd(struct udscs_connection *conn);
> +
> +/* Performs a read in socket's connection. Note that fd should be ready for
> read
> + * otherwise it will destroy the connection. Use udscs_client_fill_fds(),
> + * select() and udscs_client_handle_fds() to delegate those checks to udscs.
> + */
> +void udscs_do_read(struct udscs_connection **connp);
> +
> +/* Performs a write in socket's connection. Note that fd should be ready for
> + * write otherwise it will destroy the connection. Use
> udscs_client_fill_fds(),
> + * select() and udscs_client_handle_fds() to delegate those checks to udscs.
> + * If no buffer is ready to be written from udscs side, this function simply
> + * returns.
> + */
> +void udscs_do_write(struct udscs_connection **connp);
> +
>
> #ifndef UDSCS_NO_SERVER
>
Frediano
Cheers,
Jakub
_______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/spice-devel