[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 5/5] qemu-ga: win32: add isa-serial support
From: |
Luiz Capitulino |
Subject: |
Re: [Qemu-devel] [PATCH 5/5] qemu-ga: win32: add isa-serial support |
Date: |
Thu, 1 Nov 2012 14:55:20 -0200 |
On Thu, 1 Nov 2012 11:31:05 -0500
Michael Roth <address@hidden> wrote:
> On Wed, Oct 31, 2012 at 03:45:20PM -0200, Luiz Capitulino wrote:
> > It's implemented by opening the serial port in "non-blocking" mode
> > and using the GSourceFuncs API in the following way:
> >
> > o prepare(): issue a read request. If something has been read, it's
> > stored in rs->buf and rs->pending will store the number
> > of bytes read. If there wasn't any bytes available, we
> > set the timeout to 500 ms and return
> >
> > o check(): returns true if prepare() was able to read anything,
> > otherwise return false
> >
> > o finalize(): does nothing
> >
> > At dispatch() time we simply copy the bytes read to the buffer passed
> > to ga_channel_read().
> >
> > Signed-off-by: Luiz Capitulino <address@hidden>
> > ---
> > qga/channel-win32.c | 87
> > +++++++++++++++++++++++++++++++++++++++++++++++++++++
> > 1 file changed, 87 insertions(+)
> >
> > diff --git a/qga/channel-win32.c b/qga/channel-win32.c
> > index c173270..887fe5f 100644
> > --- a/qga/channel-win32.c
> > +++ b/qga/channel-win32.c
> > @@ -179,6 +179,46 @@ static gboolean ga_channel_dispatch_ov(GSource
> > *source, GSourceFunc unused,
> > return success;
> > }
> >
> > +static gboolean ga_channel_prepare(GSource *source, gint *timeout_ms)
> > +{
> > + GAWatch *watch = (GAWatch *)source;
> > + GAChannel *c = (GAChannel *)watch->channel;
> > + GAChannelReadState *rs = &c->rstate;
> > + DWORD count_read = 0;
> > + bool success;
> > +
> > + success = ReadFile(c->handle, rs->buf, rs->buf_size, &count_read,
> > NULL);
> > + if (success) {
> > + if (count_read == 0) {
> > + *timeout_ms = 500;
> > + return false;
> > + }
> > + rs->pending = count_read;
> > + return true;
> > + }
> > +
> > + return false;
> > +}
> > +
> > +static gboolean ga_channel_check(GSource *source)
> > +{
> > + GAWatch *watch = (GAWatch *)source;
> > + GAChannel *c = (GAChannel *)watch->channel;
> > + GAChannelReadState *rs = &c->rstate;
> > +
> > + return !!rs->pending;
> > +}
> > +
> > +static gboolean ga_channel_dispatch(GSource *source, GSourceFunc unused,
> > + gpointer user_data)
> > +{
> > + GAWatch *watch = (GAWatch *)source;
> > + GAChannel *c = (GAChannel *)watch->channel;
> > +
> > + g_assert(c->cb);
> > + return c->cb(0, c->user_data);
> > +}
> > +
> > static void ga_channel_finalize_ov(GSource *source)
> > {
> > g_debug("finalize");
> > @@ -203,6 +243,30 @@ static GSource *ga_channel_create_watch_ov(GAChannel
> > *c)
> > return source;
> > }
> >
> > +static void ga_channel_finalize(GSource *source)
> > +{
> > + g_debug("finalize");
> > +}
> > +
> > +GSourceFuncs ga_channel_watch_funcs = {
> > + ga_channel_prepare,
> > + ga_channel_check,
> > + ga_channel_dispatch,
> > + ga_channel_finalize
> > +};
> > +
> > +static GSource *ga_channel_create_watch(GAChannel *c)
> > +{
> > + GSource *source = g_source_new(&ga_channel_watch_funcs,
> > sizeof(GAWatch));
> > + GAWatch *watch = (GAWatch *)source;
> > +
> > + watch->channel = c;
> > + watch->pollfd.fd = -1;
> > + g_source_add_poll(source, &watch->pollfd);
> > +
> > + return source;
> > +}
> > +
> > GIOStatus ga_channel_read(GAChannel *c, char *buf, size_t size, gsize
> > *count)
> > {
> > GAChannelReadState *rs = &c->rstate;
> > @@ -225,6 +289,12 @@ GIOStatus ga_channel_read(GAChannel *c, char *buf,
> > size_t size, gsize *count)
> > status = G_IO_STATUS_AGAIN;
> > }
> > break;
> > + case GA_CHANNEL_ISA_SERIAL:
> > + memcpy(buf, rs->buf, rs->pending);
>
> Do we really want to ignore size? Seems like we can overrun their buffer
> if it's smaller than ours. You can use rs->start to track the number of
> bytes theyve read so far and offset from that in prepare()
size == rs->buf_size, so we're ok. Although this is sensible, as both buffers
are not tied...
I think that the intermediate buffer handling complicate things, and for
isa-serial this is not needed. What about returning the buffer from
ga_channel_read()?
I mean, instead of:
gchar buf[QGA_READ_COUNT_DEFAULT+1];
gsize count;
...
ga_channel_read(s->channel, buf, QGA_READ_COUNT_DEFAULT, &count);
We could have:
gchar *buf;
gsize count;
...
ga_channel_read(s->channel, &buf, &count);
>
> > + *count = rs->pending;
> > + rs->pending = 0;
> > + status = G_IO_STATUS_NORMAL;
> > + break;
> > default:
> > abort(); /* impossible */
> > }
> > @@ -303,6 +373,7 @@ GAChannel *ga_channel_new(GAChannelMethod method, const
> > gchar *path,
> > {
> > GAChannel *c = g_malloc0(sizeof(GAChannel));
> > SECURITY_ATTRIBUTES sec_attrs;
> > + COMMTIMEOUTS timeouts;
> >
> > switch (method) {
> > case GA_CHANNEL_VIRTIO_SERIAL:
> > @@ -322,6 +393,22 @@ GAChannel *ga_channel_new(GAChannelMethod method,
> > const gchar *path,
> >
> > c->source = ga_channel_create_watch_ov(c);
> > break;
> > + case GA_CHANNEL_ISA_SERIAL:
> > + if (!ga_channel_open(c, path, 0)) {
> > + g_critical("error opening channel (path: %s)", path);
> > + goto out_err;
> > + }
> > +
> > + /* non-blocking I/O */
> > + memset(&timeouts, 0, sizeof(timeouts));
> > + timeouts.ReadIntervalTimeout = MAXDWORD;
> > + if (!SetCommTimeouts(c->handle, &timeouts)) {
> > + CloseHandle(c->handle);
> > + goto out_err;
> > + }
> > +
> > + c->source = ga_channel_create_watch(c);
> > + break;
> > default:
> > g_critical("unsupported communication method");
> > goto out_err;
> > --
> > 1.7.12.315.g682ce8b
> >
> >
>