Re: [Qemu-devel] [PATCH v8 3/4] guest agent: qemu-ga daemon

[Michael Roth <mdroth@linux.vnet.ibm.com>]

On 07/20/2011 03:01 PM, Michael Roth wrote:
> On 07/20/2011 02:31 PM, Luiz Capitulino wrote:
>> On Wed, 20 Jul 2011 14:25:06 -0500
>> Michael Roth<mdroth@linux.vnet.ibm.com> wrote:
>>
>>> On 07/20/2011 12:56 PM, Luiz Capitulino wrote:
>>>> On Tue, 19 Jul 2011 15:41:54 -0500
>>>> Michael Roth<mdroth@linux.vnet.ibm.com> wrote:
>>>>
>>>>> This is the actual guest daemon, it listens for requests over a
>>>>> virtio-serial/isa-serial/unix socket channel and routes them through
>>>>> to dispatch routines, and writes the results back to the channel in
>>>>> a manner similar to QMP.
>>>>>
>>>>> A shorthand invocation:
>>>>>
>>>>> qemu-ga -d
>>>>>
>>>>> Is equivalent to:
>>>>>
>>>>> qemu-ga -m virtio-serial -p /dev/virtio-ports/org.qemu.guest_agent.0 \
>>>>> -f /var/run/qemu-ga.pid -d
>>>>>
>>>>> Signed-off-by: Michael Roth<mdroth@linux.vnet.ibm.com>
>>>>> ---
>>>>> Makefile | 8 +-
>>>>> configure | 1 +
>>>>> qemu-ga.c | 667 ++++++++++++++++++++++++++++++++++++++++++++++++
>>>>> qga/guest-agent-core.h | 4 +
>>>>> 4 files changed, 677 insertions(+), 3 deletions(-)
>>>>> create mode 100644 qemu-ga.c
>>>>>
>>>>> diff --git a/Makefile b/Makefile
>>>>> index b8cdf0e..0d2e33d 100644
>>>>> --- a/Makefile
>>>>> +++ b/Makefile
>>>>> @@ -140,7 +140,7 @@ endif
>>>>> ######################################################################
>>>>>
>>>>> qemu-img.o: qemu-img-cmds.h
>>>>> -qemu-img.o qemu-tool.o qemu-nbd.o qemu-io.o cmd.o:
>>>>> $(GENERATED_HEADERS)
>>>>> +qemu-img.o qemu-tool.o qemu-nbd.o qemu-io.o cmd.o qemu-ga.o:
>>>>> $(GENERATED_HEADERS)
>>>>>
>>>>> qemu-img$(EXESUF): qemu-img.o qemu-tool.o qemu-error.o
>>>>> $(oslib-obj-y) $(trace-obj-y) $(block-obj-y) $(qobject-obj-y)
>>>>> $(version-obj-y) qemu-timer-common.o
>>>>>
>>>>> @@ -184,13 +184,15 @@ test-qmp-commands: test-qmp-commands.o
>>>>> qfloat.o qint.o qdict.o qstring.o qlist.o
>>>>>
>>>>> QGALIB=qga/guest-agent-command-state.o
>>>>>
>>>>> +qemu-ga$(EXESUF): qemu-ga.o $(QGALIB) qemu-tool.o qemu-error.o
>>>>> error.o $(oslib-obj-y) $(trace-obj-y) $(block-obj-y)
>>>>> $(qobject-obj-y) $(version-obj-y) $(qapi-obj-y) qemu-timer-common.o
>>>>> qemu-sockets.o module.o qapi/qmp-dispatch.o qapi/qmp-registry.o
>>>>> +
>>>>> QEMULIBS=libhw32 libhw64 libuser libdis libdis-user
>>>>>
>>>>> clean:
>>>>> # avoid old build problems by removing potentially incorrect old files
>>>>> rm -f config.mak op-i386.h opc-i386.h gen-op-i386.h op-arm.h
>>>>> opc-arm.h gen-op-arm.h
>>>>> rm -f qemu-options.def
>>>>> - rm -f *.o *.d *.a *.lo $(TOOLS) TAGS cscope.* *.pod *~ */*~
>>>>> + rm -f *.o *.d *.a *.lo $(TOOLS) qemu-ga TAGS cscope.* *.pod *~ */*~
>>>>> rm -Rf .libs
>>>>> rm -f slirp/*.o slirp/*.d audio/*.o audio/*.d block/*.o block/*.d
>>>>> net/*.o net/*.d fsdev/*.o fsdev/*.d ui/*.o ui/*.d qapi/*.o qapi/*.d
>>>>> qga/*.o qga/*.d
>>>>> rm -f qemu-img-cmds.h
>>>>> @@ -386,4 +388,4 @@ tarbin:
>>>>> $(mandir)/man8/qemu-nbd.8
>>>>>
>>>>> # Include automatically generated dependency files
>>>>> --include $(wildcard *.d audio/*.d slirp/*.d block/*.d net/*.d
>>>>> ui/*.d qapi/*.d)
>>>>> +-include $(wildcard *.d audio/*.d slirp/*.d block/*.d net/*.d
>>>>> ui/*.d qapi/*.d qga/*.d)
>>>>> diff --git a/configure b/configure
>>>>> index 6a03002..445f674 100755
>>>>> --- a/configure
>>>>> +++ b/configure
>>>>> @@ -2532,6 +2532,7 @@ if test "$softmmu" = yes ; then
>>>>> tools="qemu-img\$(EXESUF) qemu-io\$(EXESUF) $tools"
>>>>> if [ "$linux" = "yes" -o "$bsd" = "yes" -o "$solaris" = "yes" ] ; then
>>>>> tools="qemu-nbd\$(EXESUF) $tools"
>>>>> + tools="qemu-ga\$(EXESUF) $tools"
>>>>> if [ "$check_utests" = "yes" ]; then
>>>>> tools="check-qint check-qstring check-qdict check-qlist $tools"
>>>>> tools="check-qfloat check-qjson $tools"
>>>>> diff --git a/qemu-ga.c b/qemu-ga.c
>>>>> new file mode 100644
>>>>> index 0000000..eb09100
>>>>> --- /dev/null
>>>>> +++ b/qemu-ga.c
>>>>> @@ -0,0 +1,667 @@
>>>>> +/*
>>>>> + * QEMU Guest Agent
>>>>> + *
>>>>> + * Copyright IBM Corp. 2011
>>>>> + *
>>>>> + * Authors:
>>>>> + * Adam Litke<aglitke@linux.vnet.ibm.com>
>>>>> + * Michael Roth<mdroth@linux.vnet.ibm.com>
>>>>> + *
>>>>> + * This work is licensed under the terms of the GNU GPL, version 2
>>>>> or later.
>>>>> + * See the COPYING file in the top-level directory.
>>>>> + */
>>>>> +#include<stdlib.h>
>>>>> +#include<stdio.h>
>>>>> +#include<stdbool.h>
>>>>> +#include<glib.h>
>>>>> +#include<gio/gio.h>
>>>>> +#include<getopt.h>
>>>>> +#include<termios.h>
>>>>> +#include<syslog.h>
>>>>> +#include "qemu_socket.h"
>>>>> +#include "json-streamer.h"
>>>>> +#include "json-parser.h"
>>>>> +#include "qint.h"
>>>>> +#include "qjson.h"
>>>>> +#include "qga/guest-agent-core.h"
>>>>> +#include "module.h"
>>>>> +#include "signal.h"
>>>>> +#include "qerror.h"
>>>>> +#include "error_int.h"
>>>>> +
>>>>> +#define QGA_VIRTIO_PATH_DEFAULT
>>>>> "/dev/virtio-ports/org.qemu.guest_agent.0"
>>>>> +#define QGA_PIDFILE_DEFAULT "/var/run/qemu-ga.pid"
>>>>> +#define QGA_BAUDRATE_DEFAULT B38400 /* for isa-serial channels */
>>>>> +#define QGA_TIMEOUT_DEFAULT 30*1000 /* ms */
>>>>> +
>>>>> +struct GAState {
>>>>> + JSONMessageParser parser;
>>>>> + GMainLoop *main_loop;
>>>>> + guint conn_id;
>>>>> + GSocket *conn_sock;
>>>>> + GIOChannel *conn_channel;
>>>>> + guint listen_id;
>>>>> + GSocket *listen_sock;
>>>>> + GIOChannel *listen_channel;
>>>>> + const char *path;
>>>>> + const char *method;
>>>>> + bool virtio; /* fastpath to check for virtio to deal with poll()
>>>>> quirks */
>>>>> + GACommandState *command_state;
>>>>> + GLogLevelFlags log_level;
>>>>> + FILE *log_file;
>>>>> + bool logging_enabled;
>>>>> +};
>>>>> +
>>>>> +static struct GAState *ga_state;
>>>>> +
>>>>> +static void quit_handler(int sig)
>>>>> +{
>>>>> + g_debug("recieved signal num %d, quitting", sig);
>>>>> +
>>>>> + if (g_main_loop_is_running(ga_state->main_loop)) {
>>>>> + g_main_loop_quit(ga_state->main_loop);
>>>>> + }
>>>>> +}
>>>>> +
>>>>> +static void register_signal_handlers(void)
>>>>> +{
>>>>> + struct sigaction sigact;
>>>>> + int ret;
>>>>> +
>>>>> + memset(&sigact, 0, sizeof(struct sigaction));
>>>>> + sigact.sa_handler = quit_handler;
>>>>> +
>>>>> + ret = sigaction(SIGINT,&sigact, NULL);
>>>>> + if (ret == -1) {
>>>>> + g_error("error configuring signal handler: %s", strerror(errno));
>>>>> + exit(EXIT_FAILURE);
>>>>> + }
>>>>> + ret = sigaction(SIGTERM,&sigact, NULL);
>>>>> + if (ret == -1) {
>>>>> + g_error("error configuring signal handler: %s", strerror(errno));
>>>>> + }
>>>>> +}
>>>>> +
>>>>> +static void usage(const char *cmd)
>>>>> +{
>>>>> + printf(
>>>>> +"Usage: %s -c<channel_opts>\n"
>>>>> +"QEMU Guest Agent %s\n"
>>>>> +"\n"
>>>>> +" -m, --method transport method: one of unix-listen,
>>>>> virtio-serial, or\n"
>>>>> +" isa-serial (virtio-serial is the default)\n"
>>>>> +" -p, --path device/socket path (%s is the default for
>>>>> virtio-serial)\n"
>>>>> +" -l, --logfile set logfile path, logs to stderr by default\n"
>>>>> +" -f, --pidfile specify pidfile (default is %s)\n"
>>>>> +" -v, --verbose log extra debugging information\n"
>>>>> +" -V, --version print version information and exit\n"
>>>>> +" -d, --daemonize become a daemon\n"
>>>>> +" -h, --help display this help and exit\n"
>>>>> +"\n"
>>>>> +"Report bugs to<mdroth@linux.vnet.ibm.com>\n"
>>>>> + , cmd, QGA_VERSION, QGA_VIRTIO_PATH_DEFAULT, QGA_PIDFILE_DEFAULT);
>>>>> +}
>>>>> +
>>>>> +static void conn_channel_close(GAState *s);
>>>>> +
>>>>> +static const char *ga_log_level_str(GLogLevelFlags level)
>>>>> +{
>>>>> + switch (level& G_LOG_LEVEL_MASK) {
>>>>> + case G_LOG_LEVEL_ERROR:
>>>>> + return "error";
>>>>> + case G_LOG_LEVEL_CRITICAL:
>>>>> + return "critical";
>>>>> + case G_LOG_LEVEL_WARNING:
>>>>> + return "warning";
>>>>> + case G_LOG_LEVEL_MESSAGE:
>>>>> + return "message";
>>>>> + case G_LOG_LEVEL_INFO:
>>>>> + return "info";
>>>>> + case G_LOG_LEVEL_DEBUG:
>>>>> + return "debug";
>>>>> + default:
>>>>> + return "user";
>>>>> + }
>>>>> +}
>>>>> +
>>>>> +bool ga_logging_enabled(GAState *s)
>>>>> +{
>>>>> + return s->logging_enabled;
>>>>> +}
>>>>> +
>>>>> +void ga_disable_logging(GAState *s)
>>>>> +{
>>>>> + s->logging_enabled = false;
>>>>> +}
>>>>> +
>>>>> +void ga_enable_logging(GAState *s)
>>>>> +{
>>>>> + s->logging_enabled = true;
>>>>> +}
>>>>> +
>>>>> +static void ga_log(const gchar *domain, GLogLevelFlags level,
>>>>> + const gchar *msg, gpointer opaque)
>>>>> +{
>>>>> + GAState *s = opaque;
>>>>> + GTimeVal time;
>>>>> + const char *level_str = ga_log_level_str(level);
>>>>> +
>>>>> + if (!ga_logging_enabled(s)) {
>>>>> + return;
>>>>> + }
>>>>> +
>>>>> + level&= G_LOG_LEVEL_MASK;
>>>>> + if (g_strcmp0(domain, "syslog") == 0) {
>>>>> + syslog(LOG_INFO, "%s: %s", level_str, msg);
>>>>> + } else if (level& s->log_level) {
>>>>> + g_get_current_time(&time);
>>>>> + fprintf(s->log_file,
>>>>> + "%lu.%lu: %s: %s\n", time.tv_sec, time.tv_usec, level_str, msg);
>>>>> + fflush(s->log_file);
>>>>> + }
>>>>> +}
>>>>> +
>>>>> +static void become_daemon(const char *pidfile)
>>>>> +{
>>>>> + pid_t pid, sid;
>>>>> + int pidfd;
>>>>> + char *pidstr = NULL;
>>>>> +
>>>>> + pid = fork();
>>>>> + if (pid< 0) {
>>>>> + exit(EXIT_FAILURE);
>>>>> + }
>>>>> + if (pid> 0) {
>>>>> + exit(EXIT_SUCCESS);
>>>>> + }
>>>>> +
>>>>> + pidfd = open(pidfile, O_CREAT|O_WRONLY|O_EXCL, S_IRUSR|S_IWUSR);
>>>>> + if (pidfd == -1) {
>>>>> + g_critical("Cannot create pid file, %s", strerror(errno));
>>>>> + exit(EXIT_FAILURE);
>>>>> + }
>>>>> +
>>>>> + if (asprintf(&pidstr, "%d", getpid()) == -1) {
>>>>> + g_critical("Cannot allocate memory");
>>>>> + goto fail;
>>>>> + }
>>>>> + if (write(pidfd, pidstr, strlen(pidstr)) != strlen(pidstr)) {
>>>>> + free(pidstr);
>>>>> + g_critical("Failed to write pid file");
>>>>> + goto fail;
>>>>> + }
>>>>> +
>>>>> + umask(0);
>>>>> + sid = setsid();
>>>>> + if (sid< 0) {
>>>>> + goto fail;
>>>>> + }
>>>>> + if ((chdir("/"))< 0) {
>>>>> + goto fail;
>>>>> + }
>>>>> +
>>>>> + close(STDIN_FILENO);
>>>>> + close(STDOUT_FILENO);
>>>>> + close(STDERR_FILENO);
>>>>> + free(pidstr);
>>>>> + return;
>>>>> +
>>>>> +fail:
>>>>> + unlink(pidfile);
>>>>> + g_critical("failed to daemonize");
>>>>> + exit(EXIT_FAILURE);
>>>>> +}
>>>>> +
>>>>> +static int conn_channel_send_buf(GIOChannel *channel, const char
>>>>> *buf,
>>>>> + gsize count)
>>>>> +{
>>>>> + GError *err = NULL;
>>>>> + gsize written = 0;
>>>>> + GIOStatus status;
>>>>> +
>>>>> + while (count) {
>>>>> + status = g_io_channel_write_chars(channel, buf,
>>>>> count,&written,&err);
>>>>> + g_debug("sending data, count: %d", (int)count);
>>>>> + if (err != NULL) {
>>>>> + g_warning("error sending newline: %s", err->message);
>>>>> + return err->code;
>>>>> + }
>>>>> + if (status == G_IO_STATUS_ERROR || status == G_IO_STATUS_EOF) {
>>>>> + return -EPIPE;
>>>>> + }
>>>>> +
>>>>> + if (status == G_IO_STATUS_NORMAL) {
>>>>> + count -= written;
>>>>> + }
>>>>> + }
>>>>> +
>>>>> + return 0;
>>>>> +}
>>>>> +
>>>>> +static int conn_channel_send_payload(GIOChannel *channel, QObject
>>>>> *payload)
>>>>> +{
>>>>> + int ret = 0;
>>>>> + const char *buf;
>>>>> + QString *payload_qstr;
>>>>> + GError *err = NULL;
>>>>> +
>>>>> + g_assert(payload&& channel);
>>>>> +
>>>>> + payload_qstr = qobject_to_json(payload);
>>>>> + if (!payload_qstr) {
>>>>> + return -EINVAL;
>>>>> + }
>>>>> +
>>>>> + qstring_append_chr(payload_qstr, '\n');
>>>>> + buf = qstring_get_str(payload_qstr);
>>>>> + ret = conn_channel_send_buf(channel, buf, strlen(buf));
>>>>> + if (ret) {
>>>>> + goto out_free;
>>>>> + }
>>>>> +
>>>>> + g_io_channel_flush(channel,&err);
>>>>> + if (err != NULL) {
>>>>> + g_warning("error flushing payload: %s", err->message);
>>>>> + ret = err->code;
>>>>> + goto out_free;
>>>>> + }
>>>>> +
>>>>> +out_free:
>>>>> + QDECREF(payload_qstr);
>>>>> + if (err) {
>>>>> + g_error_free(err);
>>>>> + }
>>>>> + return ret;
>>>>> +}
>>>>> +
>>>>> +static void process_command(GAState *s, QDict *req)
>>>>> +{
>>>>> + QObject *rsp = NULL;
>>>>> + int ret;
>>>>> +
>>>>> + g_assert(req);
>>>>> + g_debug("processing command");
>>>>> + rsp = qmp_dispatch(QOBJECT(req));
>>>>> + if (rsp) {
>>>>> + ret = conn_channel_send_payload(s->conn_channel, rsp);
>>>>> + if (ret) {
>>>>> + g_warning("error sending payload: %s", strerror(ret));
>>>>> + }
>>>>> + qobject_decref(rsp);
>>>>> + } else {
>>>>> + g_warning("error getting response");
>>>>> + }
>>>>> +}
>>>>> +
>>>>> +/* handle requests/control events coming in over the channel */
>>>>> +static void process_event(JSONMessageParser *parser, QList *tokens)
>>>>> +{
>>>>> + GAState *s = container_of(parser, GAState, parser);
>>>>> + QObject *obj;
>>>>> + QDict *qdict;
>>>>> + Error *err = NULL;
>>>>> + int ret;
>>>>> +
>>>>> + g_assert(s&& parser);
>>>>> +
>>>>> + g_debug("process_event: called");
>>>>> + obj = json_parser_parse_err(tokens, NULL,&err);
>>>>> + if (err || !obj || qobject_type(obj) != QTYPE_QDICT) {
>>>>> + qobject_decref(obj);
>>>>> + qdict = qdict_new();
>>>>> + if (!err) {
>>>>> + g_warning("failed to parse event: unknown error");
>>>>> + error_set(&err, QERR_JSON_PARSING);
>>>>> + } else {
>>>>> + g_warning("failed to parse event: %s", error_get_pretty(err));
>>>>> + }
>>>>> + qdict_put_obj(qdict, "error", error_get_qobject(err));
>>>>> + error_free(err);
>>>>> + } else {
>>>>> + qdict = qobject_to_qdict(obj);
>>>>> + }
>>>>> +
>>>>> + g_assert(qdict);
>>>>> +
>>>>> + /* handle host->guest commands */
>>>>> + if (qdict_haskey(qdict, "execute")) {
>>>>> + process_command(s, qdict);
>>>>> + } else {
>>>>> + if (!qdict_haskey(qdict, "error")) {
>>>>> + QDECREF(qdict);
>>>>> + qdict = qdict_new();
>>>>> + g_warning("unrecognized payload format");
>>>>> + error_set(&err, QERR_UNSUPPORTED);
>>>>> + qdict_put_obj(qdict, "error", error_get_qobject(err));
>>>>> + error_free(err);
>>>>> + }
>>>>> + ret = conn_channel_send_payload(s->conn_channel, QOBJECT(qdict));
>>>>> + if (ret) {
>>>>> + g_warning("error sending payload: %s", strerror(ret));
>>>>> + }
>>>>> + }
>>>>> +
>>>>> + QDECREF(qdict);
>>>>> +}
>>>>> +
>>>>> +static gboolean conn_channel_read(GIOChannel *channel,
>>>>> GIOCondition condition,
>>>>> + gpointer data)
>>>>> +{
>>>>> + GAState *s = data;
>>>>> + gchar buf[1024];
>>>>> + gsize count;
>>>>> + GError *err = NULL;
>>>>> + memset(buf, 0, 1024);
>>>>> + GIOStatus status = g_io_channel_read_chars(channel, buf, 1024,
>>>>> +&count,&err);
>>>>> + if (err != NULL) {
>>>>> + g_warning("error reading channel: %s", err->message);
>>>>> + conn_channel_close(s);
>>>>> + g_error_free(err);
>>>>> + return false;
>>>>> + }
>>>>> + switch (status) {
>>>>> + case G_IO_STATUS_ERROR:
>>>>> + g_warning("problem");
>>>>> + return false;
>>>>> + case G_IO_STATUS_NORMAL:
>>>>> + g_debug("read data, count: %d, data: %s", (int)count, buf);
>>>>> + json_message_parser_feed(&s->parser, (char *)buf, (int)count);
>>>>> + case G_IO_STATUS_AGAIN:
>>>>> + /* virtio causes us to spin here when no process is attached to
>>>>> + * host-side chardev. sleep a bit to mitigate this
>>>>> + */
>>>>> + if (s->virtio) {
>>>>> + usleep(100*1000);
>>>>> + }
>>>>> + return true;
>>>>> + case G_IO_STATUS_EOF:
>>>>> + g_debug("received EOF");
>>>>> + conn_channel_close(s);
>>>>> + if (s->virtio) {
>>>>> + return true;
>>>>> + }
>>>>> + return false;
>>>>> + default:
>>>>> + g_warning("unknown channel read status, closing");
>>>>> + conn_channel_close(s);
>>>>> + return false;
>>>>> + }
>>>>> + return true;
>>>>> +}
>>>>> +
>>>>> +static int conn_channel_add(GAState *s, int fd)
>>>>> +{
>>>>> + GIOChannel *conn_channel;
>>>>> + guint conn_id;
>>>>> + GError *err = NULL;
>>>>> +
>>>>> + g_assert(s&& !s->conn_channel);
>>>>> + conn_channel = g_io_channel_unix_new(fd);
>>>>> + g_assert(conn_channel);
>>>>> + g_io_channel_set_encoding(conn_channel, NULL,&err);
>>>>> + if (err != NULL) {
>>>>> + g_warning("error setting channel encoding to binary");
>>>>> + g_error_free(err);
>>>>> + return -1;
>>>>> + }
>>>>> + conn_id = g_io_add_watch(conn_channel, G_IO_IN | G_IO_HUP,
>>>>> + conn_channel_read, s);
>>>>> + if (err != NULL) {
>>>>> + g_warning("error adding io watch: %s", err->message);
>>>>> + g_error_free(err);
>>>>> + return -1;
>>>>> + }
>>>>> + s->conn_channel = conn_channel;
>>>>> + s->conn_id = conn_id;
>>>>> + return 0;
>>>>> +}
>>>>> +
>>>>> +static gboolean listen_channel_accept(GIOChannel *channel,
>>>>> + GIOCondition condition, gpointer data)
>>>>> +{
>>>>> + GAState *s = data;
>>>>> + GError *err = NULL;
>>>>> + g_assert(channel != NULL);
>>>>> + int ret;
>>>>> + bool accepted = false;
>>>>> +
>>>>> + s->conn_sock = g_socket_accept(s->listen_sock, NULL,&err);
>>>>> + if (err != NULL) {
>>>>> + g_warning("error converting fd to gsocket: %s", err->message);
>>>>> + g_error_free(err);
>>>>> + goto out;
>>>>> + }
>>>>> + ret = conn_channel_add(s, g_socket_get_fd(s->conn_sock));
>>>>> + if (ret) {
>>>>> + g_warning("error setting up connection");
>>>>> + goto out;
>>>>> + }
>>>>> + accepted = true;
>>>>> +
>>>>> +out:
>>>>> + /* only accept 1 connection at a time */
>>>>> + return !accepted;
>>>>> +}
>>>>> +
>>>>> +/* start polling for readable events on listen fd, new==true
>>>>> + * indicates we should use the existing s->listen_channel
>>>>> + */
>>>>> +static int listen_channel_add(GAState *s, int listen_fd, bool new)
>>>>> +{
>>>>> + GError *err = NULL;
>>>>> + guint listen_id;
>>>>> +
>>>>> + if (new) {
>>>>> + s->listen_channel = g_io_channel_unix_new(listen_fd);
>>>>> + if (s->listen_sock) {
>>>>> + g_object_unref(s->listen_sock);
>>>>> + }
>>>>> + s->listen_sock = g_socket_new_from_fd(listen_fd,&err);
>>>>> + if (err != NULL) {
>>>>> + g_warning("error converting fd to gsocket: %s", err->message);
>>>>> + g_error_free(err);
>>>>> + return -1;
>>>>> + }
>>>>> + }
>>>>> + listen_id = g_io_add_watch(s->listen_channel, G_IO_IN,
>>>>> + listen_channel_accept, s);
>>>>> + if (err != NULL) {
>>>>> + g_warning("error adding io watch: %s", err->message);
>>>>> + g_error_free(err);
>>>>> + return -1;
>>>>> + }
>>>>> + return 0;
>>>>
>>>> I'm getting the following error when building this on a F15 guest:
>>>>
>>>> qemu-ga.c: In function ‘listen_channel_add’:
>>>> qemu-ga.c:453:11: error: variable ‘listen_id’ set but not used
>>>> [-Werror=unused-but-set-variable]
>>>> cc1: all warnings being treated as errors
>>>>
>>>> g_io_add_watch() doesn't seem to return an error. So if the fix is
>>>> just to
>>>> drop 'listen_id' and the last if statement altogether I can do it
>>>> myself,
>>>> otherwise you can send me only this patch.
>>>>
>>>> The series looks good otherwise.
>>>>
>>>
>>> Doh, sorry for not catching this. I did the same thing with conn_id as
>>> well. I made a patch available here:
>>>
>>> http://repo.or.cz/w/qemu/mdroth.git/blobdiff/a8d38e5bb2d558918ac0bad1ee618472ae34fe7a..581922e148f37e8f5844eeb11257eef8e9302709:/qemu-ga.c
>>>
>>>
>>> I tested the patch on a 32-bit F15 guest and it seems to do the trick.
>>>
>>> If you'd like you can just pull from
>>>
>>> git://repo.or.cz/qemu/mdroth.git qga-dev
>>>
>>> Which is this series (set3-v8) plus the patch right now.
>>
>> Can you resend the patch to the list please? If it's just this patch
>> (ie. patch 3/4) that was rebased, then you can submit only it. Otherwise
>> would be better to submit a new version of the series.
>>
>
> Patch sent. It's just against 3/4
>

Or did I misunderstand, and you actually want the revised patch 3/4 with 
these changes pre-applied? If so let me know and I'll fire that off.

>>>
>>>>> +}
>>>>> +
>>>>> +/* cleanup state for closed connection/session, start accepting new
>>>>> + * connections if we're in listening mode
>>>>> + */
>>>>> +static void conn_channel_close(GAState *s)
>>>>> +{
>>>>> + if (strcmp(s->method, "unix-listen") == 0) {
>>>>> + g_io_channel_shutdown(s->conn_channel, true, NULL);
>>>>> + g_object_unref(s->conn_sock);
>>>>> + s->conn_sock = NULL;
>>>>> + listen_channel_add(s, 0, false);
>>>>> + } else if (strcmp(s->method, "virtio-serial") == 0) {
>>>>> + /* we spin on EOF for virtio-serial, so back off a bit. also,
>>>>> + * dont close the connection in this case, it'll resume normal
>>>>> + * operation when another process connects to host chardev
>>>>> + */
>>>>> + usleep(100*1000);
>>>>> + goto out_noclose;
>>>>> + }
>>>>> + g_io_channel_unref(s->conn_channel);
>>>>> + s->conn_channel = NULL;
>>>>> + s->conn_id = 0;
>>>>> +out_noclose:
>>>>> + return;
>>>>> +}
>>>>> +
>>>>> +static void init_guest_agent(GAState *s)
>>>>> +{
>>>>> + struct termios tio;
>>>>> + int ret, fd;
>>>>> +
>>>>> + if (s->method == NULL) {
>>>>> + /* try virtio-serial as our default */
>>>>> + s->method = "virtio-serial";
>>>>> + }
>>>>> +
>>>>> + if (s->path == NULL) {
>>>>> + if (strcmp(s->method, "virtio-serial") != 0) {
>>>>> + g_critical("must specify a path for this channel");
>>>>> + exit(EXIT_FAILURE);
>>>>> + }
>>>>> + /* try the default path for the virtio-serial port */
>>>>> + s->path = QGA_VIRTIO_PATH_DEFAULT;
>>>>> + }
>>>>> +
>>>>> + if (strcmp(s->method, "virtio-serial") == 0) {
>>>>> + s->virtio = true;
>>>>> + fd = qemu_open(s->path, O_RDWR | O_NONBLOCK | O_ASYNC);
>>>>> + if (fd == -1) {
>>>>> + g_critical("error opening channel: %s", strerror(errno));
>>>>> + exit(EXIT_FAILURE);
>>>>> + }
>>>>> + ret = conn_channel_add(s, fd);
>>>>> + if (ret) {
>>>>> + g_critical("error adding channel to main loop");
>>>>> + exit(EXIT_FAILURE);
>>>>> + }
>>>>> + } else if (strcmp(s->method, "isa-serial") == 0) {
>>>>> + fd = qemu_open(s->path, O_RDWR | O_NOCTTY);
>>>>> + if (fd == -1) {
>>>>> + g_critical("error opening channel: %s", strerror(errno));
>>>>> + exit(EXIT_FAILURE);
>>>>> + }
>>>>> + tcgetattr(fd,&tio);
>>>>> + /* set up serial port for non-canonical, dumb byte streaming */
>>>>> + tio.c_iflag&= ~(IGNBRK | BRKINT | IGNPAR | PARMRK | INPCK | ISTRIP |
>>>>> + INLCR | IGNCR | ICRNL | IXON | IXOFF | IXANY |
>>>>> + IMAXBEL);
>>>>> + tio.c_oflag = 0;
>>>>> + tio.c_lflag = 0;
>>>>> + tio.c_cflag |= QGA_BAUDRATE_DEFAULT;
>>>>> + /* 1 available byte min or reads will block (we'll set non-blocking
>>>>> + * elsewhere, else we have to deal with read()=0 instead)
>>>>> + */
>>>>> + tio.c_cc[VMIN] = 1;
>>>>> + tio.c_cc[VTIME] = 0;
>>>>> + /* flush everything waiting for read/xmit, it's garbage at this
>>>>> point */
>>>>> + tcflush(fd, TCIFLUSH);
>>>>> + tcsetattr(fd, TCSANOW,&tio);
>>>>> + ret = conn_channel_add(s, fd);
>>>>> + if (ret) {
>>>>> + g_error("error adding channel to main loop");
>>>>> + }
>>>>> + } else if (strcmp(s->method, "unix-listen") == 0) {
>>>>> + fd = unix_listen(s->path, NULL, strlen(s->path));
>>>>> + if (fd == -1) {
>>>>> + g_critical("error opening path: %s", strerror(errno));
>>>>> + exit(EXIT_FAILURE);
>>>>> + }
>>>>> + ret = listen_channel_add(s, fd, true);
>>>>> + if (ret) {
>>>>> + g_critical("error binding/listening to specified socket");
>>>>> + exit(EXIT_FAILURE);
>>>>> + }
>>>>> + } else {
>>>>> + g_critical("unsupported channel method/type: %s", s->method);
>>>>> + exit(EXIT_FAILURE);
>>>>> + }
>>>>> +
>>>>> + json_message_parser_init(&s->parser, process_event);
>>>>> + s->main_loop = g_main_loop_new(NULL, false);
>>>>> +}
>>>>> +
>>>>> +int main(int argc, char **argv)
>>>>> +{
>>>>> + const char *sopt = "hVvdm:p:l:f:";
>>>>> + const char *method = NULL, *path = NULL, *pidfile =
>>>>> QGA_PIDFILE_DEFAULT;
>>>>> + const struct option lopt[] = {
>>>>> + { "help", 0, NULL, 'h' },
>>>>> + { "version", 0, NULL, 'V' },
>>>>> + { "logfile", 0, NULL, 'l' },
>>>>> + { "pidfile", 0, NULL, 'f' },
>>>>> + { "verbose", 0, NULL, 'v' },
>>>>> + { "method", 0, NULL, 'm' },
>>>>> + { "path", 0, NULL, 'p' },
>>>>> + { "daemonize", 0, NULL, 'd' },
>>>>> + { NULL, 0, NULL, 0 }
>>>>> + };
>>>>> + int opt_ind = 0, ch, daemonize = 0;
>>>>> + GLogLevelFlags log_level = G_LOG_LEVEL_ERROR | G_LOG_LEVEL_CRITICAL;
>>>>> + FILE *log_file = stderr;
>>>>> + GAState *s;
>>>>> +
>>>>> + while ((ch = getopt_long(argc, argv, sopt, lopt,&opt_ind)) != -1) {
>>>>> + switch (ch) {
>>>>> + case 'm':
>>>>> + method = optarg;
>>>>> + break;
>>>>> + case 'p':
>>>>> + path = optarg;
>>>>> + break;
>>>>> + case 'l':
>>>>> + log_file = fopen(optarg, "a");
>>>>> + if (!log_file) {
>>>>> + g_critical("unable to open specified log file: %s",
>>>>> + strerror(errno));
>>>>> + return EXIT_FAILURE;
>>>>> + }
>>>>> + break;
>>>>> + case 'f':
>>>>> + pidfile = optarg;
>>>>> + break;
>>>>> + case 'v':
>>>>> + /* enable all log levels */
>>>>> + log_level = G_LOG_LEVEL_MASK;
>>>>> + break;
>>>>> + case 'V':
>>>>> + printf("QEMU Guest Agent %s\n", QGA_VERSION);
>>>>> + return 0;
>>>>> + case 'd':
>>>>> + daemonize = 1;
>>>>> + break;
>>>>> + case 'h':
>>>>> + usage(argv[0]);
>>>>> + return 0;
>>>>> + case '?':
>>>>> + g_print("Unknown option, try '%s --help' for more information.\n",
>>>>> + argv[0]);
>>>>> + return EXIT_FAILURE;
>>>>> + }
>>>>> + }
>>>>> +
>>>>> + if (daemonize) {
>>>>> + g_debug("starting daemon");
>>>>> + become_daemon(pidfile);
>>>>> + }
>>>>> +
>>>>> + g_type_init();
>>>>> + g_thread_init(NULL);
>>>>> +
>>>>> + s = qemu_mallocz(sizeof(GAState));
>>>>> + s->conn_id = 0;
>>>>> + s->conn_channel = NULL;
>>>>> + s->path = path;
>>>>> + s->method = method;
>>>>> + s->log_file = log_file;
>>>>> + s->log_level = log_level;
>>>>> + g_log_set_default_handler(ga_log, s);
>>>>> + g_log_set_fatal_mask(NULL, G_LOG_LEVEL_ERROR);
>>>>> + s->logging_enabled = true;
>>>>> + ga_state = s;
>>>>> +
>>>>> + module_call_init(MODULE_INIT_QAPI);
>>>>> + init_guest_agent(ga_state);
>>>>> + register_signal_handlers();
>>>>> +
>>>>> + g_main_loop_run(ga_state->main_loop);
>>>>> +
>>>>> + unlink(pidfile);
>>>>> +
>>>>> + return 0;
>>>>> +}
>>>>> diff --git a/qga/guest-agent-core.h b/qga/guest-agent-core.h
>>>>> index 688f120..66d1729 100644
>>>>> --- a/qga/guest-agent-core.h
>>>>> +++ b/qga/guest-agent-core.h
>>>>> @@ -15,6 +15,7 @@
>>>>>
>>>>> #define QGA_VERSION "1.0"
>>>>>
>>>>> +typedef struct GAState GAState;
>>>>> typedef struct GACommandState GACommandState;
>>>>>
>>>>> void ga_command_state_add(GACommandState *cs,
>>>>> @@ -23,3 +24,6 @@ void ga_command_state_add(GACommandState *cs,
>>>>> void ga_command_state_init_all(GACommandState *cs);
>>>>> void ga_command_state_cleanup_all(GACommandState *cs);
>>>>> GACommandState *ga_command_state_new(void);
>>>>> +bool ga_logging_enabled(GAState *s);
>>>>> +void ga_disable_logging(GAState *s);
>>>>> +void ga_enable_logging(GAState *s);
>>>>
>>>
>>
>