From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:47901) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QXgXy-0000y0-OZ for qemu-devel@nongnu.org; Fri, 17 Jun 2011 17:25:49 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1QXgXv-0006eq-Ep for qemu-devel@nongnu.org; Fri, 17 Jun 2011 17:25:46 -0400 Received: from e34.co.us.ibm.com ([32.97.110.152]:51831) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QXgXu-0006ek-5W for qemu-devel@nongnu.org; Fri, 17 Jun 2011 17:25:43 -0400 Received: from d03relay03.boulder.ibm.com (d03relay03.boulder.ibm.com [9.17.195.228]) by e34.co.us.ibm.com (8.14.4/8.13.1) with ESMTP id p5HLCkMW023152 for ; Fri, 17 Jun 2011 15:12:46 -0600 Received: from d03av05.boulder.ibm.com (d03av05.boulder.ibm.com [9.17.195.85]) by d03relay03.boulder.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id p5HLPZm8169098 for ; Fri, 17 Jun 2011 15:25:35 -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 p5HLPZV0009950 for ; Fri, 17 Jun 2011 15:25:35 -0600 Message-ID: <4DFBC64C.2020007@linux.vnet.ibm.com> Date: Fri, 17 Jun 2011 16:25:32 -0500 From: Michael Roth MIME-Version: 1.0 References: <1308081985-32394-1-git-send-email-mdroth@linux.vnet.ibm.com> <1308081985-32394-3-git-send-email-mdroth@linux.vnet.ibm.com> <20110616154221.6e6d848e@doriath> <4DFBA93B.4040104@linux.vnet.ibm.com> <20110617171301.1db09794@doriath> In-Reply-To: <20110617171301.1db09794@doriath> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v5 2/5] 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 06/17/2011 03:13 PM, Luiz Capitulino wrote: > On Fri, 17 Jun 2011 14:21:31 -0500 > Michael Roth wrote: > >> On 06/16/2011 01:42 PM, Luiz Capitulino wrote: >>> On Tue, 14 Jun 2011 15:06:22 -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 >>>> >>>> Signed-off-by: Michael Roth >>> >>> Would be nice to have a more complete description, like explaining how to >>> do a simple test. >>> >>> And this can't be built... >>> >>>> --- >>>> qemu-ga.c | 631 ++++++++++++++++++++++++++++++++++++++++++++++++ >>>> qga/guest-agent-core.h | 4 + >>>> 2 files changed, 635 insertions(+), 0 deletions(-) >>>> create mode 100644 qemu-ga.c >>>> >>>> diff --git a/qemu-ga.c b/qemu-ga.c >>>> new file mode 100644 >>>> index 0000000..df08d8c >>>> --- /dev/null >>>> +++ b/qemu-ga.c >>>> @@ -0,0 +1,631 @@ >>>> +/* >>>> + * 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 "qga-qmp-commands.h" >>>> +#include "module.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 { >>>> + const char *proxy_path; >>> >>> Where is this used? >>> >> >> Nowhere actually. Will remove. >> >>>> + 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 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; >>>> +} >>> >>> Just to check I got this right, this is needed because of the fsfreeze >>> command, correct? Isn't it better to have a more descriptive name, like >>> fsfrozen? >>> >>> First I thought this was about a log file. Then I realized this was >>> probably about letting the user log in, but we don't seem to have this >>> functionality so I got confused. >>> >> >> Yup, this is currently due to fsfreeze support. From the perspective of >> the fsfreeze command the explicit "is_frozen" check makes more sense, >> but the reason it affects other RPCs is because because we can't log >> stuff in that state. If an RPC shoots itself in the foot by writing to a >> frozen filesystem we don't really care so much, and up until recently >> that case was handled with a pthread_cancel timeout mechanism (was >> removed for the time being, will re-implement using a child process most >> likely). >> >> What we don't want to do is give a host a way to bypass the expectation >> we set for guest owners by allowing RPCs to be logged. So that's what >> the check is based on, rather than lower level details like *why* >> logging is disabled. Also, I'd really like to avoid a precedence where a >> single RPC can place restrictions on all the others, so the logging >> aspect seemed general enough that it doesn't necessarily provide that >> precedence. >> >> This has come up a few times without any real consensus. I can probably >> go either way, but I think the check for logging is easier to set >> expectations with: "if logging is important from an auditing >> perspective, don't execute this if logging is disabled". beyond that, >> same behavior/symantics you'd get with anything that causes a write() on >> a filesystem in a frozen state: block. > > While I understand your arguments, I still find the current behavior > confusing: you issue a guest-open-file command and get a QgaLoggingFailed > error. The first question is: what does a log write fail have to do with me > opening a file? The second question is: what should I do? Try again? Give up? > I agree it's a better solution for the client there, but at the same time: guest_privileged_ping(): if fsfreeze.status == FROZEN: syslog("privileged ping, thought you should know") else: return "error, filesystem frozen" Seems silly to the client as well. "Why does a ping command care about the filesystem state?" Inability to log is the true error. That's also the case for the guest-file-open command. There's nothing wrong with opening a read-only file on a frozen filesystem, it's the fact that we can't log the open that causes us to bail out.. So, what if we just munge the 2 to give the user the proper clues to fix things, and instead return an error like: "Guest agent failed to log RPC (is the filesystem frozen?)"? > IMHO, making fsfrozen a global state makes more sense. It's the true guest > state after all. This way a client will clearly know what's happening and > what it has to do in order to make its command successful. > > Another possible solution is to have the same semantics of a non-blocking > I/O, that's, return EWOULDBLOCK. I find this less confusing than the > current error. > >> >>>> + >>>> +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_RDWR, S_IRUSR|S_IWUSR); >>> >>> O_WRONLY is enough. >>> >>>> + if (!pidfd || lockf(pidfd, F_TLOCK, 0)) { >>>> + g_error("Cannot lock pid file"); >>>> + } >>> >>> Are you sure we need lockf() here? I think using O_EXCL is enough for >>> pid files. >>> >> >> O_EXCL + O_CREAT? Wouldn't we have issues if the pid file wasn't cleaned >> up from a previous run? > > Yes, but that's the intention (and that's how most daemons work afaik). The > only reason for a daemon not to cleanup its pid file is when it exists > abnormally. If this happens, then only the sysadmin can ensure that it's safe > to run another daemon instance. > > We could add a -r (--override-existing-pid-file) option, but we should not > do it silently. > > Btw, we need to implement a signal handler for SIGTERM. > >>>> + >>>> + if (ftruncate(pidfd, 0) || lseek(pidfd, 0, SEEK_SET)) { >>>> + g_critical("Cannot truncate pid file"); >>>> + goto fail; >>> >>> You can use O_TRUNC and the file pointer is already positioned at the >>> beginning of the file, no need to call lseek(). >>> >>>> + } >>>> + if (asprintf(&pidstr, "%d", getpid()) == -1) { >>>> + g_critical("Cannot allocate memory"); >>>> + goto fail; >>>> + } >>>> + if (write(pidfd, pidstr, strlen(pidstr)) != strlen(pidstr)) { >>>> + g_critical("Failed to write pid file"); >>>> + goto fail; >>>> + } >>> >>> You're not freeing pidstr, nor closing the file (although I think you >>> do this because of the lockf() call()). >>> >> >> pidstr gets free'd in the fail: block. And yeah, closing would give up >> the lock prematurely. > > pidstr leaks on a successful execution. > >> >>>> + >>>> + umask(0); >>>> + sid = setsid(); >>>> + if (sid< 0) { >>>> + goto fail; >>>> + } >>>> + if ((chdir("/"))< 0) { >>>> + goto fail; >>>> + } >>>> + >>>> + close(STDIN_FILENO); >>>> + close(STDOUT_FILENO); >>>> + close(STDERR_FILENO); >>>> + return; >>>> + >>>> +fail: >>>> + if (pidstr) { >>>> + free(pidstr); >>>> + } >>> >>> This check is not needed. >>> >>>> + unlink(pidfile); >>>> + g_error("failed to daemonize"); >>>> +} >>>> + >>>> +static int conn_channel_send_payload(GIOChannel *channel, QObject *payload) >>>> +{ >>>> + gsize count, written = 0; >>>> + int ret; >>>> + const char *buf; >>>> + QString *payload_qstr; >>>> + GIOStatus status; >>>> + GError *err = NULL; >>>> + >>>> + if (!payload || !channel) { >>>> + ret = -EINVAL; >>>> + goto out; >>>> + } >>> >>> Just do "return -EINVAL" instead of using goto, but I wonder if this should >>> be an assert() instead. >>> >> >> Yah, looks like it. >> >>>> + >>>> + payload_qstr = qobject_to_json(payload); >>>> + if (!payload_qstr) { >>>> + ret = -EINVAL; >>>> + goto out; >>>> + } >>> >>> return -EINVAL. >>> >>>> + >>>> + buf = qstring_get_str(payload_qstr); >>>> + count = strlen(buf); >>>> + >>>> + while (count) { >>>> + g_debug("sending data, count: %d", (int)count); >>>> + status = g_io_channel_write_chars(channel, buf, count,&written,&err); >>>> + if (err != NULL) { >>>> + g_warning("error writing payload to channel: %s", err->message); >>>> + ret = err->code; >>>> + goto out_free; >>>> + } >>>> + if (status == G_IO_STATUS_NORMAL) { >>>> + count -= written; >>>> + } else if (status == G_IO_STATUS_ERROR || status == G_IO_STATUS_EOF) { >>>> + ret = -EPIPE; >>>> + goto out_free; >>>> + } >>>> + } >>>> + >>>> + status = g_io_channel_write_chars(channel, (char *)"\n", 1,&written,&err); >>>> + if (err != NULL) { >>>> + g_warning("error sending newline: %s", err->message); >>>> + ret = err->code; >>>> + goto out_free; >>>> + } else if (status == G_IO_STATUS_ERROR || status == G_IO_STATUS_EOF) { >>>> + ret = -EPIPE; >>>> + goto out_free; >>>> + } >>> >>> This wants to be a function. >>> >>>> + >>>> + g_io_channel_flush(channel,&err); >>>> + if (err != NULL) { >>>> + g_warning("error flushing payload: %s", err->message); >>>> + ret = err->code; >>>> + goto out_free; >>>> + } >>>> + >>>> + ret = 0; >>>> + >>>> +out_free: >>>> + QDECREF(payload_qstr); >>>> + if (err != NULL) { >>>> + g_error_free(err); >>>> + } >>> >>> No need to check against NULL. >>> >>>> +out: >>>> + return ret; >>>> +} >>>> + >>>> +static void process_command(GAState *s, QDict *req) >>>> +{ >>>> + QObject *rsp = NULL; >>>> + Error *err = NULL; >>>> + int ret; >>>> + >>>> + g_assert(req); >>>> + g_debug("processing command"); >>>> + rsp = qmp_dispatch(QOBJECT(req)); >>>> + if (rsp) { >>>> + if (err) { >>> >>> Apart from its initialization, 'err' is read only. Either, you want to >>> use qmp_dispatch_err() here or you have to modify qmp_dispatch() to also >>> accept an Error argument. >>> >>> If you want to send a "return" or "error" QMP kind of response, then you >>> have to do the latter. >>> >> >> qmp_dispatch() does the Error -> QMP error conversion. The unused err is >> cruft from when a seperate worker thread did the dispatch, will remove it. >> >>>> + g_warning("command failed: %s", error_get_pretty(err)); >>>> + } >>>> + 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"); >>>> + if (err) { >>>> + g_warning("dispatch failed: %s", error_get_pretty(err)); >>>> + } >>>> + } >>>> +} >>>> + >>>> +/* 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) { >>>> + g_warning("failed to parse event"); >>> >>> Two possible leaks here. You have to call qobject_decref() if obj != NULL >>> and there's a missing call to error_free(). But you don't use 'err' anyway, >>> so you could pass NULL there. >>> >>> Oh, btw, the guest agent doesn't seem to report errors back to its client... >>> Not even json error messages, is this intended? >>> >> >> Hmm, yes it's intended, but I probably should send those to the client... >> >> You do get non-parse errors though: missing arguments, etc. >> >> I'll fix this to do what QMP does here. >> >>>> + return; >>>> + } else { >>>> + g_debug("parse successful"); >>>> + qdict = qobject_to_qdict(obj); >>>> + g_assert(qdict); >>>> + } >>> >>> Superfluous else clause. >>> >>>> + >>>> + /* handle host->guest commands */ >>>> + if (qdict_haskey(qdict, "execute")) { >>>> + process_command(s, qdict); >>>> + } else { >>>> + 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, listen_fd=0 >>>> + * indicates we should use the existing s->listen_channel >>>> + */ >>>> +static int listen_channel_add(GAState *s, int listen_fd) >>>> +{ >>>> + GError *err = NULL; >>>> + guint listen_id; >>>> + >>>> + if (listen_fd) { >>>> + 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); >>>> + } 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)); >>>> + } >>>> + 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); >>>> + 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[] = { >>>> + { "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; >>>> + >>>> + g_type_init(); >>>> + g_thread_init(NULL); >>>> + >>>> + while ((ch = getopt_long(argc, argv, sopt, lopt,&opt_ind)) != -1) { >>>> + switch (ch) { >>>> + case 'c': >>>> + method = optarg; >>>> + 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]); >>>> + } >>>> + } >>>> + >>>> + if (daemonize) { >>>> + g_debug("starting daemon"); >>>> + become_daemon(pidfile); >>>> + } >>>> + >>>> + s = g_malloc0(sizeof(GAState)); >>>> + s->conn_id = 0; >>>> + s->conn_channel = NULL; >>>> + s->path = path; >>>> + s->method = method; >>>> + s->command_state = ga_command_state_new(); >>>> + ga_command_state_init(s, s->command_state); >>>> + ga_command_state_init_all(s->command_state); >>>> + 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; >>>> + >>>> + module_call_init(MODULE_INIT_QAPI); >>>> + init_guest_agent(s); >>>> + >>>> + g_main_loop_run(s->main_loop); >>>> + >>>> + ga_command_state_cleanup_all(s->command_state); >>>> + >>>> + 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 >>> >>> No license text. >>> >>>> @@ -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); >>> >> >