On Wed, Oct 31, 2012 at 03:45:19PM -0200, Luiz Capitulino wrote:
> This commit prepares ga_channel_new(), ga_channel_read() and
> ga_channel_open() for isa-serial support.
>
> Signed-off-by: Luiz Capitulino <lcapitul...@redhat.com>
Reviewed-by: Michael Roth <mdr...@linux.vnet.ibm.com>
Though I'm wondering if it'd be nicer to:
1) Add a GAChannelOps struct with pointers for init/read/write/write_all
2) Move all the method-specific init stuff from ga_channel_new() into
method-specific init functions (which all use ga_channel_open() as
a general helper function, or maybe roll the CreateFile() into the
init routines with the proper flags hard-coded
3) Have ga_channel_new() simply hang the proper GAChannelOps* off of
the GAChannel it create and then call GAChannel->ops->init(self)
4) Make ga_channel_read/write/write_all(self) wrappers around
GAChannel->ops->read/write/write_all(self)
> ---
> qga/channel-win32.c | 75
> ++++++++++++++++++++++++++++++-----------------------
> 1 file changed, 42 insertions(+), 33 deletions(-)
>
> diff --git a/qga/channel-win32.c b/qga/channel-win32.c
> index 8e259c3..c173270 100644
> --- a/qga/channel-win32.c
> +++ b/qga/channel-win32.c
> @@ -213,14 +213,20 @@ GIOStatus ga_channel_read(GAChannel *c, char *buf,
> size_t size, gsize *count)
> return G_IO_STATUS_ERROR;
> }
>
> - *count = to_read = MIN(size, rs->pending);
> - if (to_read) {
> - memcpy(buf, rs->buf + rs->cur, to_read);
> - rs->cur += to_read;
> - rs->pending -= to_read;
> - status = G_IO_STATUS_NORMAL;
> - } else {
> - status = G_IO_STATUS_AGAIN;
> + switch (c->method) {
> + case GA_CHANNEL_VIRTIO_SERIAL:
> + *count = to_read = MIN(size, rs->pending);
> + if (to_read) {
> + memcpy(buf, rs->buf + rs->cur, to_read);
> + rs->cur += to_read;
> + rs->pending -= to_read;
> + status = G_IO_STATUS_NORMAL;
> + } else {
> + status = G_IO_STATUS_AGAIN;
> + }
> + break;
> + default:
> + abort(); /* impossible */
> }
>
> return status;
> @@ -285,23 +291,11 @@ GIOStatus ga_channel_write_all(GAChannel *c, const char
> *buf, size_t size)
> return status;
> }
>
> -static gboolean ga_channel_open(GAChannel *c, GAChannelMethod method,
> - const gchar *path)
> +static gboolean ga_channel_open(GAChannel *c, const gchar *path, int flags)
> {
> - if (!method == GA_CHANNEL_VIRTIO_SERIAL) {
> - g_critical("unsupported communication method");
> - return false;
> - }
> -
> c->handle = CreateFile(path, GENERIC_READ | GENERIC_WRITE, 0, NULL,
> - OPEN_EXISTING,
> - FILE_FLAG_NO_BUFFERING | FILE_FLAG_OVERLAPPED,
> NULL);
> - if (c->handle == INVALID_HANDLE_VALUE) {
> - g_critical("error opening path");
> - return false;
> - }
> -
> - return true;
> + OPEN_EXISTING, FILE_FLAG_NO_BUFFERING | flags,
> NULL);
> + return (c->handle == INVALID_HANDLE_VALUE) ? false : true;
> }
>
> GAChannel *ga_channel_new(GAChannelMethod method, const gchar *path,
> @@ -310,27 +304,42 @@ GAChannel *ga_channel_new(GAChannelMethod method, const
> gchar *path,
> GAChannel *c = g_malloc0(sizeof(GAChannel));
> SECURITY_ATTRIBUTES sec_attrs;
>
> - if (!ga_channel_open(c, method, path)) {
> - g_critical("error opening channel");
> - g_free(c);
> - return NULL;
> + switch (method) {
> + case GA_CHANNEL_VIRTIO_SERIAL:
> + if (!ga_channel_open(c, path, FILE_FLAG_OVERLAPPED)) {
> + g_critical("error opening channel (path: %s)", path);
> + goto out_err;
> + }
> +
> + sec_attrs.nLength = sizeof(SECURITY_ATTRIBUTES);
> + sec_attrs.lpSecurityDescriptor = NULL;
> + sec_attrs.bInheritHandle = false;
> + c->rstate.ov.hEvent = CreateEvent(&sec_attrs, FALSE, FALSE, NULL);
> + if (!c->rstate.ov.hEvent) {
> + g_critical("can't create event");
> + goto out_err;
> + }
> +
> + c->source = ga_channel_create_watch_ov(c);
> + break;
> + default:
> + g_critical("unsupported communication method");
> + goto out_err;
> }
>
> c->cb = cb;
> c->user_data = opaque;
> c->method = method;
>
> - sec_attrs.nLength = sizeof(SECURITY_ATTRIBUTES);
> - sec_attrs.lpSecurityDescriptor = NULL;
> - sec_attrs.bInheritHandle = false;
> -
> c->rstate.buf_size = QGA_READ_COUNT_DEFAULT;
> c->rstate.buf = g_malloc(QGA_READ_COUNT_DEFAULT);
> - c->rstate.ov.hEvent = CreateEvent(&sec_attrs, FALSE, FALSE, NULL);
>
> - c->source = ga_channel_create_watch_ov(c);
> g_source_attach(c->source, NULL);
> return c;
> +
> +out_err:
> + g_free(c);
> + return NULL;
> }
>
> void ga_channel_free(GAChannel *c)
> --
> 1.7.12.315.g682ce8b
>
