From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:43797) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QfIM8-0005U0-Nk for qemu-devel@nongnu.org; Fri, 08 Jul 2011 17:13:03 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1QfIM5-0006X3-JZ for qemu-devel@nongnu.org; Fri, 08 Jul 2011 17:13:00 -0400 Received: from e38.co.us.ibm.com ([32.97.110.159]:34058) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QfIM4-0006Ww-Oq for qemu-devel@nongnu.org; Fri, 08 Jul 2011 17:12:57 -0400 Received: from d03relay01.boulder.ibm.com (d03relay01.boulder.ibm.com [9.17.195.226]) by e38.co.us.ibm.com (8.14.4/8.13.1) with ESMTP id p68L4K3p018321 for ; Fri, 8 Jul 2011 15:04:20 -0600 Received: from d03av05.boulder.ibm.com (d03av05.boulder.ibm.com [9.17.195.85]) by d03relay01.boulder.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id p68LCtJa205682 for ; Fri, 8 Jul 2011 15:12:55 -0600 Received: from d03av05.boulder.ibm.com (loopback [127.0.0.1]) by d03av05.boulder.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id p68LCsru007306 for ; Fri, 8 Jul 2011 15:12:54 -0600 Message-ID: <4E1772D3.8000401@linux.vnet.ibm.com> Date: Fri, 08 Jul 2011 16:12:51 -0500 From: Michael Roth MIME-Version: 1.0 References: <1309872100-27912-1-git-send-email-mdroth@linux.vnet.ibm.com> <1309872100-27912-3-git-send-email-mdroth@linux.vnet.ibm.com> <20110708113625.49369c53@doriath> In-Reply-To: <20110708113625.49369c53@doriath> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v6 2/4] guest agent: qemu-ga daemon List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Luiz Capitulino Cc: aliguori@linux.vnet.ibm.com, agl@linux.vnet.ibm.com, qemu-devel@nongnu.org, Jes.Sorensen@redhat.com On 07/08/2011 09:36 AM, Luiz Capitulino wrote: > On Tue, 5 Jul 2011 08:21:38 -0500 > Michael Roth 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 -c virtio-serial -p /dev/virtio-ports/org.qemu.guest_agent \ >> -p /var/run/qemu-guest-agent.pid -d > > I think you meant -f /var/run/qemu-guest-agent.pid > Yup, sorry. >> >> Signed-off-by: Michael Roth >> --- >> Makefile | 10 +- >> qemu-ga.c | 651 ++++++++++++++++++++++++++++++++++++++++++++++++ >> qga/guest-agent-core.h | 4 + >> 3 files changed, 661 insertions(+), 4 deletions(-) >> create mode 100644 qemu-ga.c >> >> diff --git a/Makefile b/Makefile >> index 6c3ba71..b2e8593 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 >> >> @@ -163,7 +163,7 @@ check-qfloat: check-qfloat.o qfloat.o $(CHECK_PROG_DEPS) >> check-qjson: check-qjson.o qfloat.o qint.o qdict.o qstring.o qlist.o qbool.o qjson.o json-streamer.o json-lexer.o json-parser.o error.o qerror.o qemu-error.o $(CHECK_PROG_DEPS) >> >> qapi-dir := qapi-generated >> -$(qapi-obj-y) test-visitor.o test-qmp-commands.o: QEMU_CFLAGS += -I $(qapi-dir) >> +$(qapi-obj-y) test-visitor.o test-qmp-commands.o qemu-ga$(EXESUF): QEMU_CFLAGS += -I $(qapi-dir) >> >> $(qapi-dir)/test-qapi-types.c: $(qapi-dir)/test-qapi-types.h >> $(qapi-dir)/test-qapi-types.h: $(SRC_PATH)/qapi-schema-test.json $(SRC_PATH)/scripts/qapi-types.py >> @@ -183,13 +183,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 >> @@ -385,4 +387,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/qemu-ga.c b/qemu-ga.c >> new file mode 100644 >> index 0000000..649c16a >> --- /dev/null >> +++ b/qemu-ga.c >> @@ -0,0 +1,651 @@ >> +/* >> + * QEMU Guest Agent >> + * >> + * Copyright IBM Corp. 2011 >> + * >> + * Authors: >> + * Adam Litke >> + * Michael Roth >> + * >> + * 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 >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> +#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" >> + >> +#define QGA_VIRTIO_PATH_DEFAULT "/dev/virtio-ports/org.qemu.guest_agent" >> +#define QGA_PIDFILE_DEFAULT "/var/run/qemu-va.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"); >> + >> + 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; >> + >> + sigact.sa_handler = quit_handler; >> + >> + ret = sigaction(SIGINT,&sigact, NULL); >> + if (ret == -1) { >> + g_error("error configuring signal handler: %s", strerror(errno)); >> + } >> + 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\n" >> +"QEMU Guest Agent %s\n" >> +"\n" >> +" -c, --channel channel method: one of unix-connect, virtio-serial, or\n" >> +" isa-serial (virtio-serial is the default)\n" >> +" -p, --path channel 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\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_error("Cannot create pid file, %s", strerror(errno)); >> + } >> + >> + 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_error("failed to daemonize"); >> +} >> + >> +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; >> + } > > You can do: > > qstring_append_chr(payload_qstr, '\n'); > > so that you avoid the additional conn_channel_send_buf() call below. > Nice! >> + >> + buf = qstring_get_str(payload_qstr); >> + ret = conn_channel_send_buf(channel, buf, strlen(buf)); >> + if (ret) { >> + goto out_free; >> + } >> + >> + ret = conn_channel_send_buf(channel, "\n", 1); >> + 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; >> + >> + g_assert(s&& parser); >> + >> + g_debug("process_event: called"); >> + obj = json_parser_parse_err(tokens, NULL,&err); >> + if (!obj || qobject_type(obj) != QTYPE_QDICT) { >> + qobject_decref(obj); >> + g_warning("failed to parse event"); > > No error is returned to the client, is this intended? > Hmm, nope... originally the errors got rolled into the response in qmp_dispatch(). But when I reworked qmp_dispatch() to take a pre-parsed QDict instead of a token list this functionality was lost.. Which makes me wonder if I should move the parsing back into qmp_dispatch()... I'll take a look. But yah, these errors should get returned. >> + return; >> + } >> + >> + if (err) { >> + error_free(err); >> + g_warning("failed to parse event: %s", error_get_pretty(err)); > > Same here. > >> + return; >> + } >> + >> + g_debug("parse successful"); >> + qdict = qobject_to_qdict(obj); >> + g_assert(qdict); >> + >> + /* handle host->guest commands */ >> + if (qdict_haskey(qdict, "execute")) { >> + process_command(s, qdict); >> + } else { > > And here. > >> + g_warning("unrecognized payload format, ignoring"); >> + } >> + >> + 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; >> +} >> + >> +/* 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_error("must specify a path for this channel"); >> + } >> + /* 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); >> + if (fd == -1) { >> + g_error("error opening channel: %s", strerror(errno)); >> + } >> + ret = fcntl(fd, F_GETFL); >> + if (ret< 0) { >> + g_error("error getting channel flags: %s", strerror(errno)); >> + } >> + ret = fcntl(fd, F_SETFL, ret | O_NONBLOCK | O_ASYNC); >> + if (ret< 0) { >> + g_error("error setting channel flags: %s", strerror(errno)); >> + } > > Can't O_NONBLOCK and O_ASYNC be set as open() flags? > >> + ret = conn_channel_add(s, fd); >> + if (ret) { >> + g_error("error adding channel to main loop"); >> + } >> + } else if (strcmp(s->method, "isa-serial") == 0) { >> + fd = qemu_open(s->path, O_RDWR | O_NOCTTY); >> + if (fd == -1) { >> + g_error("error opening channel: %s", strerror(errno)); >> + } >> + 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_error("error opening path: %s", strerror(errno)); >> + } >> + ret = listen_channel_add(s, fd, true); >> + if (ret) { >> + g_error("error binding/listening to specified socket"); >> + } >> + } else { >> + g_error("unsupported channel method/type: %s", s->method); >> + } >> + >> + 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 = "hVvdc:p:l:f:"; >> + const char *method = NULL, *path = NULL, *pidfile = QGA_PIDFILE_DEFAULT; >> + struct option lopt[] = { > > Can be const. > >> + { "help", 0, NULL, 'h' }, >> + { "version", 0, NULL, 'V' }, >> + { "logfile", 0, NULL, 'l' }, >> + { "pidfile", 0, NULL, 'f' }, >> + { "verbose", 0, NULL, 'v' }, >> + { "channel", 0, NULL, 'c' }, >> + { "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 'c': >> + method = optarg; > > Why don't you call this option 'm' ('method') then? > That's probably better. >> + break; >> + case 'p': >> + path = optarg; >> + break; >> + case 'l': >> + log_file = fopen(optarg, "a"); >> + if (!log_file) { >> + g_error("unable to open specified log file: %s", >> + strerror(errno)); >> + } >> + 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_error("Unknown option, try '%s --help' for more information.", >> + argv[0]); > > g_error() documentation from: > > http://developer.gnome.org/glib/2.29/glib-Message-Logging.html#g-error > > Says: > > "This function will result in a core dump; don't use it for errors you expect. > Using this function indicates a bug in your program, i.e. an assertion failure." > > But I see it's being used beyond assertions. > I've been using it to bail on unrecoverable error states. Might be a bit extreme for unknown options though. >> + } >> + } >> + >> + 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); >