qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH v2 0/8] qemu-ga: add support for Windows
@ 2012-02-02 19:58 Michael Roth
  2012-02-02 19:58 ` [Qemu-devel] [PATCH v2 1/8] qemu-ga: Add schema documentation for types Michael Roth
                   ` (8 more replies)
  0 siblings, 9 replies; 21+ messages in thread
From: Michael Roth @ 2012-02-02 19:58 UTC (permalink / raw)
  To: qemu-devel; +Cc: ghammer, aliguori, mdroth, lcapitulino

These patches apply on top of qemu.git master, and can also be obtained from:
git://github.com/mdroth/qemu.git qga-win32-v2

Luiz/Gal, I decided not to roll the suspend/hibernate stuff into this series
since the s3 situation isn't fully sorted out yet. The file structure is a
little different now, posix/linux-specific stuff goes in qga/commands-posix.c,
win32-specific stuff in qga/commands-win32.c, but other than that it should be
a straightforward rebase if this gets merged first.

CHANGES SINCE V1:

- Dropped guest-set-support-level patch dependency
- Rebased on master and re-tested
- Spelling/grammar fixes in commits/comments

OVERVIEW:

These patches add support for Windows to the QEMU guest agent. With these
patches the following guest agent commands are supported on Windows:

guest-ping
guest-info
guest-sync
guest-shutdown

The guest-file* commands can essentially be enabled for Windows as-is, but since
mingw does not honor the O_NONBLOCK flag, they'll need to be reworked if we're
to retain the current non-blocking behavior.

The rest of the commands are currently stubbed out for Windows (qemu-ga will
return an "unsupported" error), but it should be easy to implement these going
forward with basic Windows support/infrastructure in place.

The build was tested using Fedora15 with a MinGW cross-build target via:

configure --enable-guest-agent --cross-prefix=i686-pc-mingw32-
make qemu-ga.exe

The executable was tested using Windows XP SP3, and partially tested using
Windows Server 2008 and Windows 7 (no I/O for the latter 2, having issues with
virtio-win drivers). GLib 2.28+ for Windows is required. You can install qemu-ga
as a load-on-boot service by running:

./qemu-ga --service install

And start/stop manually via:

net start qemu-ga
net stop qemu-ga

Many thanks to Gal Hammer for contributing the service integration and shutdown
code.

 Makefile                   |    2 +-
 Makefile.objs              |    8 +-
 configure                  |    2 +-
 qapi-schema-guest.json     |  118 ++++++++--
 qemu-ga.c                  |  413 ++++++++++++++-----------------
 qga/channel-posix.c        |  246 +++++++++++++++++++
 qga/channel-win32.c        |  337 +++++++++++++++++++++++++
 qga/channel.h              |   33 +++
 qga/commands-posix.c       |  528 +++++++++++++++++++++++++++++++++++++++
 qga/commands-win32.c       |  130 ++++++++++
 qga/commands.c             |   73 ++++++
 qga/guest-agent-commands.c |  585 --------------------------------------------
 qga/guest-agent-core.h     |    3 +-
 qga/service-win32.c        |  114 +++++++++
 qga/service-win32.h        |   30 +++
 15 files changed, 1782 insertions(+), 840 deletions(-)

^ permalink raw reply	[flat|nested] 21+ messages in thread

* [Qemu-devel] [PATCH v2 1/8] qemu-ga: Add schema documentation for types
  2012-02-02 19:58 [Qemu-devel] [PATCH v2 0/8] qemu-ga: add support for Windows Michael Roth
@ 2012-02-02 19:58 ` Michael Roth
  2012-02-02 19:58 ` [Qemu-devel] [PATCH v2 2/8] qemu-ga: move channel/transport functionality into wrapper class Michael Roth
                   ` (7 subsequent siblings)
  8 siblings, 0 replies; 21+ messages in thread
From: Michael Roth @ 2012-02-02 19:58 UTC (permalink / raw)
  To: qemu-devel; +Cc: ghammer, aliguori, mdroth, lcapitulino

Document guest agent schema types in similar fashion as qmp schema
types.

Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
 qapi-schema-guest.json |  118 +++++++++++++++++++++++++++++++++++++++---------
 1 files changed, 97 insertions(+), 21 deletions(-)

diff --git a/qapi-schema-guest.json b/qapi-schema-guest.json
index 5f8a18d..706925d 100644
--- a/qapi-schema-guest.json
+++ b/qapi-schema-guest.json
@@ -37,17 +37,42 @@
 { 'command': 'guest-ping' }
 
 ##
-# @guest-info:
+# @GuestAgentCommandInfo:
 #
-# Get some information about the guest agent.
+# Information about guest agent commands.
 #
-# Since: 0.15.0
+# @name: name of the command
+#
+# @enabled: whether command is currently enabled by guest admin
+#
+# Since 1.1.0
 ##
 { 'type': 'GuestAgentCommandInfo',
   'data': { 'name': 'str', 'enabled': 'bool' } }
+
+##
+# @GuestAgentInfo
+#
+# Information about guest agent.
+#
+# @version: guest agent version
+#
+# @supported_commands: Information about guest agent commands
+#
+# Since 0.15.0
+##
 { 'type': 'GuestAgentInfo',
   'data': { 'version': 'str',
             'supported_commands': ['GuestAgentCommandInfo'] } }
+##
+# @guest-info:
+#
+# Get some information about the guest agent.
+#
+# Returns: @GuestAgentInfo
+#
+# Since: 0.15.0
+##
 { 'command': 'guest-info',
   'returns': 'GuestAgentInfo' }
 
@@ -98,6 +123,23 @@
   'data': { 'handle': 'int' } }
 
 ##
+# @GuestFileRead
+#
+# Result of guest agent file-read operation
+#
+# @count: number of bytes read (note: count is *before*
+#         base64-encoding is applied)
+#
+# @buf-b64: base64-encoded bytes read
+#
+# @eof: whether EOF was encountered during read operation.
+#
+# Since: 0.15.0
+##
+{ 'type': 'GuestFileRead',
+  'data': { 'count': 'int', 'buf-b64': 'str', 'eof': 'bool' } }
+
+##
 # @guest-file-read:
 #
 # Read from an open file in the guest. Data will be base64-encoded
@@ -106,19 +148,30 @@
 #
 # @count: #optional maximum number of bytes to read (default is 4KB)
 #
-# Returns: GuestFileRead on success. Note: count is number of bytes read
-#          *before* base64 encoding bytes read.
+# Returns: @GuestFileRead on success.
 #
 # Since: 0.15.0
 ##
-{ 'type': 'GuestFileRead',
-  'data': { 'count': 'int', 'buf-b64': 'str', 'eof': 'bool' } }
-
 { 'command': 'guest-file-read',
   'data':    { 'handle': 'int', '*count': 'int' },
   'returns': 'GuestFileRead' }
 
 ##
+# @GuestFileWrite
+#
+# Result of guest agent file-write operation
+#
+# @count: number of bytes written (note: count is actual bytes
+#         written, after base64-decoding of provided buffer)
+#
+# @eof: whether EOF was encountered during write operation.
+#
+# Since: 0.15.0
+##
+{ 'type': 'GuestFileWrite',
+  'data': { 'count': 'int', 'eof': 'bool' } }
+
+##
 # @guest-file-write:
 #
 # Write to an open file in the guest.
@@ -130,17 +183,29 @@
 # @count: #optional bytes to write (actual bytes, after base64-decode),
 #         default is all content in buf-b64 buffer after base64 decoding
 #
-# Returns: GuestFileWrite on success. Note: count is the number of bytes
-#          base64-decoded bytes written
+# Returns: @GuestFileWrite on success.
 #
 # Since: 0.15.0
 ##
-{ 'type': 'GuestFileWrite',
-  'data': { 'count': 'int', 'eof': 'bool' } }
 { 'command': 'guest-file-write',
   'data':    { 'handle': 'int', 'buf-b64': 'str', '*count': 'int' },
   'returns': 'GuestFileWrite' }
 
+
+##
+# @GuestFileSeek
+#
+# Result of guest agent file-seek operation
+#
+# @position: current file position
+#
+# @eof: whether EOF was encountered during file seek
+#
+# Since: 0.15.0
+##
+{ 'type': 'GuestFileSeek',
+  'data': { 'position': 'int', 'eof': 'bool' } }
+
 ##
 # @guest-file-seek:
 #
@@ -154,13 +219,10 @@
 #
 # @whence: SEEK_SET, SEEK_CUR, or SEEK_END, as with fseek()
 #
-# Returns: GuestFileSeek on success.
+# Returns: @GuestFileSeek on success.
 #
 # Since: 0.15.0
 ##
-{ 'type': 'GuestFileSeek',
-  'data': { 'position': 'int', 'eof': 'bool' } }
-
 { 'command': 'guest-file-seek',
   'data':    { 'handle': 'int', 'offset': 'int', 'whence': 'int' },
   'returns': 'GuestFileSeek' }
@@ -180,18 +242,32 @@
   'data': { 'handle': 'int' } }
 
 ##
-# @guest-fsfreeze-status:
+# @GuestFsFreezeStatus
 #
-# Get guest fsfreeze state. error state indicates failure to thaw 1 or more
-# previously frozen filesystems, or failure to open a previously cached
-# filesytem (filesystem unmounted/directory changes, etc).
+# An enumation of filesystem freeze states
 #
-# Returns: GuestFsfreezeStatus ("thawed", "frozen", etc., as defined below)
+# @thawed: filesystems thawed/unfrozen
+#
+# @frozen: all non-network guest filesystems frozen
+#
+# @error: failure to thaw 1 or more
+#         previously frozen filesystems, or failure to open a previously
+#         cached filesytem (filesystem unmounted/directory changes, etc).
 #
 # Since: 0.15.0
 ##
 { 'enum': 'GuestFsfreezeStatus',
   'data': [ 'thawed', 'frozen', 'error' ] }
+
+##
+# @guest-fsfreeze-status:
+#
+# Get guest fsfreeze state. error state indicates
+#
+# Returns: GuestFsfreezeStatus ("thawed", "frozen", etc., as defined below)
+#
+# Since: 0.15.0
+##
 { 'command': 'guest-fsfreeze-status',
   'returns': 'GuestFsfreezeStatus' }
 
-- 
1.7.4.1

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [Qemu-devel] [PATCH v2 2/8] qemu-ga: move channel/transport functionality into wrapper class
  2012-02-02 19:58 [Qemu-devel] [PATCH v2 0/8] qemu-ga: add support for Windows Michael Roth
  2012-02-02 19:58 ` [Qemu-devel] [PATCH v2 1/8] qemu-ga: Add schema documentation for types Michael Roth
@ 2012-02-02 19:58 ` Michael Roth
  2012-02-02 19:58 ` [Qemu-devel] [PATCH v2 3/8] qemu-ga: separate out common commands from posix-specific ones Michael Roth
                   ` (6 subsequent siblings)
  8 siblings, 0 replies; 21+ messages in thread
From: Michael Roth @ 2012-02-02 19:58 UTC (permalink / raw)
  To: qemu-devel; +Cc: ghammer, aliguori, mdroth, lcapitulino

This is mostly in preparation for the win32 port, which won't use
GIO channels for reasons that will be made clearer later. Here the
GAChannel class is just a loose wrapper around GIOChannel
calls/callbacks, but we also roll in the logic/configuration for
various channel types and managing unix socket connections, which makes
the abstraction much more complete and further aids in the win32 port
since isa-serial/unix-listen will not be supported initially.

There's also a bit of refactoring in the main logic to consolidate the
exit paths so we can do common cleanup for things like pid files, which
weren't always cleaned up previously.

Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
 Makefile.objs          |    1 +
 qemu-ga.c              |  306 ++++++++++++------------------------------------
 qga/channel-posix.c    |  246 ++++++++++++++++++++++++++++++++++++++
 qga/channel.h          |   33 +++++
 qga/guest-agent-core.h |    2 +-
 5 files changed, 355 insertions(+), 233 deletions(-)
 create mode 100644 qga/channel-posix.c
 create mode 100644 qga/channel.h

diff --git a/Makefile.objs b/Makefile.objs
index b942625..27ff919 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -425,6 +425,7 @@ common-obj-y += qmp.o hmp.o
 # guest agent
 
 qga-nested-y = guest-agent-commands.o guest-agent-command-state.o
+qga-nested-y += channel-posix.o
 qga-obj-y = $(addprefix qga/, $(qga-nested-y))
 qga-obj-y += qemu-ga.o qemu-sockets.o module.o qemu-option.o
 qga-obj-$(CONFIG_WIN32) += oslib-win32.o
diff --git a/qemu-ga.c b/qemu-ga.c
index 29e4f64..2e8af02 100644
--- a/qemu-ga.c
+++ b/qemu-ga.c
@@ -15,9 +15,7 @@
 #include <stdbool.h>
 #include <glib.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"
@@ -28,19 +26,15 @@
 #include "qerror.h"
 #include "error_int.h"
 #include "qapi/qmp-core.h"
+#include "qga/channel.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;
-    GIOChannel *conn_channel;
-    GIOChannel *listen_channel;
-    const char *path;
-    const char *method;
+    GAChannel *channel;
     bool virtio; /* fastpath to check for virtio to deal with poll() quirks */
     GACommandState *command_state;
     GLogLevelFlags log_level;
@@ -59,7 +53,7 @@ static void quit_handler(int sig)
     }
 }
 
-static void register_signal_handlers(void)
+static gboolean register_signal_handlers(void)
 {
     struct sigaction sigact;
     int ret;
@@ -70,12 +64,14 @@ static void register_signal_handlers(void)
     ret = sigaction(SIGINT, &sigact, NULL);
     if (ret == -1) {
         g_error("error configuring signal handler: %s", strerror(errno));
-        exit(EXIT_FAILURE);
+        return false;
     }
     ret = sigaction(SIGTERM, &sigact, NULL);
     if (ret == -1) {
         g_error("error configuring signal handler: %s", strerror(errno));
+        return false;
     }
+    return true;
 }
 
 static void usage(const char *cmd)
@@ -100,8 +96,6 @@ static void usage(const char *cmd)
     , 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) {
@@ -210,40 +204,13 @@ fail:
     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)
+static int send_response(GAState *s, QObject *payload)
 {
-    int ret = 0;
     const char *buf;
     QString *payload_qstr;
-    GError *err = NULL;
+    GIOStatus status;
 
-    g_assert(payload && channel);
+    g_assert(payload && s->channel);
 
     payload_qstr = qobject_to_json(payload);
     if (!payload_qstr) {
@@ -252,24 +219,13 @@ static int conn_channel_send_payload(GIOChannel *channel, QObject *payload)
 
     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:
+    status = ga_channel_write_all(s->channel, buf, strlen(buf));
     QDECREF(payload_qstr);
-    if (err) {
-        g_error_free(err);
+    if (status != G_IO_STATUS_NORMAL) {
+        return -EIO;
     }
-    return ret;
+
+    return 0;
 }
 
 static void process_command(GAState *s, QDict *req)
@@ -281,9 +237,9 @@ static void process_command(GAState *s, QDict *req)
     g_debug("processing command");
     rsp = qmp_dispatch(QOBJECT(req));
     if (rsp) {
-        ret = conn_channel_send_payload(s->conn_channel, rsp);
+        ret = send_response(s, rsp);
         if (ret) {
-            g_warning("error sending payload: %s", strerror(ret));
+            g_warning("error sending response: %s", strerror(ret));
         }
         qobject_decref(rsp);
     } else {
@@ -333,38 +289,42 @@ static void process_event(JSONMessageParser *parser, QList *tokens)
             qdict_put_obj(qdict, "error", error_get_qobject(err));
             error_free(err);
         }
-        ret = conn_channel_send_payload(s->conn_channel, QOBJECT(qdict));
+        ret = send_response(s, QOBJECT(qdict));
         if (ret) {
-            g_warning("error sending payload: %s", strerror(ret));
+            g_warning("error sending error response: %s", strerror(ret));
         }
     }
 
     QDECREF(qdict);
 }
 
-static gboolean conn_channel_read(GIOChannel *channel, GIOCondition condition,
-                                  gpointer data)
+/* false return signals GAChannel to close the current client connection */
+static gboolean channel_event_cb(GIOCondition condition, gpointer data)
 {
     GAState *s = data;
-    gchar buf[1024];
+    gchar buf[QGA_READ_COUNT_DEFAULT+1];
     gsize count;
     GError *err = NULL;
-    memset(buf, 0, 1024);
-    GIOStatus status = g_io_channel_read_chars(channel, buf, 1024,
-                                               &count, &err);
+    GIOStatus status = ga_channel_read(s->channel, buf, QGA_READ_COUNT_DEFAULT, &count);
     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");
+        g_warning("error reading channel");
         return false;
     case G_IO_STATUS_NORMAL:
+        buf[count] = 0;
         g_debug("read data, count: %d, data: %s", (int)count, buf);
         json_message_parser_feed(&s->parser, (char *)buf, (int)count);
+        break;
+    case G_IO_STATUS_EOF:
+        g_debug("received EOF");
+        if (!s->virtio) {
+            return false;
+        }
     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
@@ -373,180 +333,49 @@ static gboolean conn_channel_read(GIOChannel *channel, GIOCondition condition,
             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;
-    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;
-    }
-    g_io_add_watch(conn_channel, G_IO_IN | G_IO_HUP,
-                   conn_channel_read, s);
-    s->conn_channel = conn_channel;
-    return 0;
-}
-
-static gboolean listen_channel_accept(GIOChannel *channel,
-                                      GIOCondition condition, gpointer data)
-{
-    GAState *s = data;
-    g_assert(channel != NULL);
-    int ret, conn_fd;
-    bool accepted = false;
-    struct sockaddr_un addr;
-    socklen_t addrlen = sizeof(addr);
-
-    conn_fd = qemu_accept(g_io_channel_unix_get_fd(s->listen_channel),
-                             (struct sockaddr *)&addr, &addrlen);
-    if (conn_fd == -1) {
-        g_warning("error converting fd to gsocket: %s", strerror(errno));
-        goto out;
-    }
-    fcntl(conn_fd, F_SETFL, O_NONBLOCK);
-    ret = conn_channel_add(s, conn_fd);
-    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)
-{
-    if (new) {
-        s->listen_channel = g_io_channel_unix_new(listen_fd);
-    }
-    g_io_add_watch(s->listen_channel, G_IO_IN,
-                   listen_channel_accept, s);
-    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);
-        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;
-out_noclose:
-    return;
-}
-
-static void init_guest_agent(GAState *s)
+static gboolean channel_init(GAState *s, const gchar *method, const gchar *path)
 {
-    struct termios tio;
-    int ret, fd;
+    GAChannelMethod channel_method;
 
-    if (s->method == NULL) {
-        /* try virtio-serial as our default */
-        s->method = "virtio-serial";
+    if (method == NULL) {
+        method = "virtio-serial";
     }
 
-    if (s->path == NULL) {
-        if (strcmp(s->method, "virtio-serial") != 0) {
+    if (path == NULL) {
+        if (strcmp(method, "virtio-serial") != 0) {
             g_critical("must specify a path for this channel");
-            exit(EXIT_FAILURE);
+            return false;
         }
         /* try the default path for the virtio-serial port */
-        s->path = QGA_VIRTIO_PATH_DEFAULT;
+        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);
-        }
+    if (strcmp(method, "virtio-serial") == 0) {
+        s->virtio = true; /* virtio requires special handling in some cases */
+        channel_method = GA_CHANNEL_VIRTIO_SERIAL;
+    } else if (strcmp(method, "isa-serial") == 0) {
+        channel_method = GA_CHANNEL_ISA_SERIAL;
+    } else if (strcmp(method, "unix-listen") == 0) {
+        channel_method = GA_CHANNEL_UNIX_LISTEN;
     } else {
-        g_critical("unsupported channel method/type: %s", s->method);
-        exit(EXIT_FAILURE);
+        g_critical("unsupported channel method/type: %s", method);
+        return false;
     }
 
-    json_message_parser_init(&s->parser, process_event);
-    s->main_loop = g_main_loop_new(NULL, false);
+    s->channel = ga_channel_new(channel_method, path, channel_event_cb, s);
+    if (!s->channel) {
+        g_critical("failed to create guest agent channel");
+        return false;
+    }
+
+    return true;
 }
 
 int main(int argc, char **argv)
@@ -643,9 +472,6 @@ int main(int argc, char **argv)
     }
 
     s = g_malloc0(sizeof(GAState));
-    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);
@@ -654,15 +480,31 @@ int main(int argc, char **argv)
     s->command_state = ga_command_state_new();
     ga_command_state_init(s, s->command_state);
     ga_command_state_init_all(s->command_state);
+    json_message_parser_init(&s->parser, process_event);
     ga_state = s;
+    if (!register_signal_handlers()) {
+        g_critical("failed to register signal handlers");
+        goto out_bad;
+    }
 
-    init_guest_agent(ga_state);
-    register_signal_handlers();
-
+    s->main_loop = g_main_loop_new(NULL, false);
+    if (!channel_init(ga_state, method, path)) {
+        g_critical("failed to initialize guest agent channel");
+        goto out_bad;
+    }
     g_main_loop_run(ga_state->main_loop);
 
     ga_command_state_cleanup_all(ga_state->command_state);
-    unlink(pidfile);
+    ga_channel_free(ga_state->channel);
 
+    if (daemonize) {
+        unlink(pidfile);
+    }
     return 0;
+
+out_bad:
+    if (daemonize) {
+        unlink(pidfile);
+    }
+    return EXIT_FAILURE;
 }
diff --git a/qga/channel-posix.c b/qga/channel-posix.c
new file mode 100644
index 0000000..40f7658
--- /dev/null
+++ b/qga/channel-posix.c
@@ -0,0 +1,246 @@
+#include <glib.h>
+#include <termios.h>
+#include "qemu_socket.h"
+#include "qga/channel.h"
+
+#define GA_CHANNEL_BAUDRATE_DEFAULT B38400 /* for isa-serial channels */
+
+struct GAChannel {
+    GIOChannel *listen_channel;
+    GIOChannel *client_channel;
+    GAChannelMethod method;
+    GAChannelCallback event_cb;
+    gpointer user_data;
+};
+
+static int ga_channel_client_add(GAChannel *c, int fd);
+
+static gboolean ga_channel_listen_accept(GIOChannel *channel,
+                                         GIOCondition condition, gpointer data)
+{
+    GAChannel *c = data;
+    int ret, client_fd;
+    bool accepted = false;
+    struct sockaddr_un addr;
+    socklen_t addrlen = sizeof(addr);
+
+    g_assert(channel != NULL);
+
+    client_fd = qemu_accept(g_io_channel_unix_get_fd(channel),
+                            (struct sockaddr *)&addr, &addrlen);
+    if (client_fd == -1) {
+        g_warning("error converting fd to gsocket: %s", strerror(errno));
+        goto out;
+    }
+    fcntl(client_fd, F_SETFL, O_NONBLOCK);
+    ret = ga_channel_client_add(c, client_fd);
+    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 void ga_channel_listen_add(GAChannel *c, int listen_fd, bool create)
+{
+    if (create) {
+        c->listen_channel = g_io_channel_unix_new(listen_fd);
+    }
+    g_io_add_watch(c->listen_channel, G_IO_IN, ga_channel_listen_accept, c);
+}
+
+static void ga_channel_listen_close(GAChannel *c)
+{
+    g_assert(c->method == GA_CHANNEL_UNIX_LISTEN);
+    g_assert(c->listen_channel);
+    g_io_channel_shutdown(c->listen_channel, true, NULL);
+    g_io_channel_unref(c->listen_channel);
+    c->listen_channel = NULL;
+}
+
+/* cleanup state for closed connection/session, start accepting new
+ * connections if we're in listening mode
+ */
+static void ga_channel_client_close(GAChannel *c)
+{
+    g_assert(c->client_channel);
+    g_io_channel_shutdown(c->client_channel, true, NULL);
+    g_io_channel_unref(c->client_channel);
+    c->client_channel = NULL;
+    if (c->method == GA_CHANNEL_UNIX_LISTEN && c->listen_channel) {
+        ga_channel_listen_add(c, 0, false);
+    }
+}
+
+static gboolean ga_channel_client_event(GIOChannel *channel,
+                                        GIOCondition condition, gpointer data)
+{
+    GAChannel *c = data;
+    gboolean client_cont;
+
+    g_assert(c);
+    if (c->event_cb) {
+        client_cont = c->event_cb(condition, c->user_data);
+        if (!client_cont) {
+            ga_channel_client_close(c);
+            return false;
+        }
+    }
+    return true;
+}
+
+static int ga_channel_client_add(GAChannel *c, int fd)
+{
+    GIOChannel *client_channel;
+    GError *err = NULL;
+
+    g_assert(c && !c->client_channel);
+    client_channel = g_io_channel_unix_new(fd);
+    g_assert(client_channel);
+    g_io_channel_set_encoding(client_channel, NULL, &err);
+    if (err != NULL) {
+        g_warning("error setting channel encoding to binary");
+        g_error_free(err);
+        return -1;
+    }
+    g_io_add_watch(client_channel, G_IO_IN | G_IO_HUP,
+                   ga_channel_client_event, c);
+    c->client_channel = client_channel;
+    return 0;
+}
+
+static gboolean ga_channel_open(GAChannel *c, const gchar *path, GAChannelMethod method)
+{
+    int ret;
+    c->method = method;
+
+    switch (c->method) {
+    case GA_CHANNEL_VIRTIO_SERIAL: {
+        int fd = qemu_open(path, O_RDWR | O_NONBLOCK | O_ASYNC);
+        if (fd == -1) {
+            g_critical("error opening channel: %s", strerror(errno));
+            exit(EXIT_FAILURE);
+        }
+        ret = ga_channel_client_add(c, fd);
+        if (ret) {
+            g_critical("error adding channel to main loop");
+            return false;
+        }
+        break;
+    }
+    case GA_CHANNEL_ISA_SERIAL: {
+        struct termios tio;
+        int fd = qemu_open(path, O_RDWR | O_NOCTTY | O_NONBLOCK);
+        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 |= GA_CHANNEL_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 = ga_channel_client_add(c, fd);
+        if (ret) {
+            g_error("error adding channel to main loop");
+        }
+        break;
+    }
+    case GA_CHANNEL_UNIX_LISTEN: {
+        int fd = unix_listen(path, NULL, strlen(path));
+        if (fd == -1) {
+            g_critical("error opening path: %s", strerror(errno));
+            return false;
+        }
+        ga_channel_listen_add(c, fd, true);
+        break;
+    }
+    default:
+        g_critical("error binding/listening to specified socket");
+        return false;
+    }
+
+    return true;
+}
+
+GIOStatus ga_channel_write_all(GAChannel *c, const gchar *buf, gsize size)
+{
+    GError *err = NULL;
+    gsize written = 0;
+    GIOStatus status = G_IO_STATUS_NORMAL;
+
+    while (size) {
+        status = g_io_channel_write_chars(c->client_channel, buf, size,
+                                          &written, &err);
+        g_debug("sending data, count: %d", (int)size);
+        if (err != NULL) {
+            g_warning("error writing to channel: %s", err->message);
+            return G_IO_STATUS_ERROR;
+        }
+        if (status != G_IO_STATUS_NORMAL) {
+            break;
+        }
+        size -= written;
+    }
+
+    if (status == G_IO_STATUS_NORMAL) {
+        status = g_io_channel_flush(c->client_channel, &err);
+        if (err != NULL) {
+            g_warning("error flushing channel: %s", err->message);
+            return G_IO_STATUS_ERROR;
+        }
+    }
+
+    return status;
+}
+
+GIOStatus ga_channel_read(GAChannel *c, gchar *buf, gsize size, gsize *count)
+{
+    return g_io_channel_read_chars(c->client_channel, buf, size, count, NULL);
+}
+
+GAChannel *ga_channel_new(GAChannelMethod method, const gchar *path,
+                          GAChannelCallback cb, gpointer opaque)
+{
+    GAChannel *c = g_malloc0(sizeof(GAChannel));
+    c->event_cb = cb;
+    c->user_data = opaque;
+
+    if (!ga_channel_open(c, path, method)) {
+        g_critical("error opening channel");
+        ga_channel_free(c);
+        return NULL;
+    }
+
+    return c;
+}
+
+void ga_channel_free(GAChannel *c)
+{
+    if (c->method == GA_CHANNEL_UNIX_LISTEN
+        && c->listen_channel) {
+        ga_channel_listen_close(c);
+    }
+    if (c->client_channel) {
+        ga_channel_client_close(c);
+    }
+    g_free(c);
+}
diff --git a/qga/channel.h b/qga/channel.h
new file mode 100644
index 0000000..3704ea9
--- /dev/null
+++ b/qga/channel.h
@@ -0,0 +1,33 @@
+/*
+ * QEMU Guest Agent channel declarations
+ *
+ * Copyright IBM Corp. 2012
+ *
+ * Authors:
+ *  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.
+ */
+#ifndef QGA_CHANNEL_H
+#define QGA_CHANNEL_H
+
+#include <glib.h>
+
+typedef struct GAChannel GAChannel;
+
+typedef enum {
+    GA_CHANNEL_VIRTIO_SERIAL,
+    GA_CHANNEL_ISA_SERIAL,
+    GA_CHANNEL_UNIX_LISTEN,
+} GAChannelMethod;
+
+typedef gboolean (*GAChannelCallback)(GIOCondition condition, gpointer opaque);
+
+GAChannel *ga_channel_new(GAChannelMethod method, const gchar *path,
+                          GAChannelCallback cb, gpointer opaque);
+void ga_channel_free(GAChannel *c);
+GIOStatus ga_channel_read(GAChannel *c, gchar *buf, gsize size, gsize *count);
+GIOStatus ga_channel_write_all(GAChannel *c, const gchar *buf, gsize size);
+
+#endif
diff --git a/qga/guest-agent-core.h b/qga/guest-agent-core.h
index e42b91d..6148d10 100644
--- a/qga/guest-agent-core.h
+++ b/qga/guest-agent-core.h
@@ -14,7 +14,7 @@
 #include "qemu-common.h"
 
 #define QGA_VERSION "1.0"
-#define QGA_READ_COUNT_DEFAULT 4 << 10
+#define QGA_READ_COUNT_DEFAULT 4096
 
 typedef struct GAState GAState;
 typedef struct GACommandState GACommandState;
-- 
1.7.4.1

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [Qemu-devel] [PATCH v2 3/8] qemu-ga: separate out common commands from posix-specific ones
  2012-02-02 19:58 [Qemu-devel] [PATCH v2 0/8] qemu-ga: add support for Windows Michael Roth
  2012-02-02 19:58 ` [Qemu-devel] [PATCH v2 1/8] qemu-ga: Add schema documentation for types Michael Roth
  2012-02-02 19:58 ` [Qemu-devel] [PATCH v2 2/8] qemu-ga: move channel/transport functionality into wrapper class Michael Roth
@ 2012-02-02 19:58 ` Michael Roth
  2012-02-02 19:58 ` [Qemu-devel] [PATCH v2 4/8] qemu-ga: rename guest-agent-commands.c -> commands-posix.c Michael Roth
                   ` (5 subsequent siblings)
  8 siblings, 0 replies; 21+ messages in thread
From: Michael Roth @ 2012-02-02 19:58 UTC (permalink / raw)
  To: qemu-devel; +Cc: ghammer, aliguori, mdroth, lcapitulino

Many of the current RPC implementations are very much POSIX-specific
and require complete re-writes for Windows. There are however a small
set of core guest agent commands that are common to both, and other
commands such as guest-file-* which *may* be portable. So we introduce
commands.c for the latter, and will rename guest-agent-commands.c to
commands-posix.c in a future commit. Windows implementations will go in
commands-win32.c, eventually.

Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
 Makefile.objs              |    2 +-
 qga/commands.c             |   73 ++++++++++++++++++++++++++++++++++++++++++++
 qga/guest-agent-commands.c |   59 +-----------------------------------
 qga/guest-agent-core.h     |    1 +
 4 files changed, 76 insertions(+), 59 deletions(-)
 create mode 100644 qga/commands.c

diff --git a/Makefile.objs b/Makefile.objs
index 27ff919..d70cebe 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -424,7 +424,7 @@ common-obj-y += qmp.o hmp.o
 ######################################################################
 # guest agent
 
-qga-nested-y = guest-agent-commands.o guest-agent-command-state.o
+qga-nested-y = commands.o guest-agent-commands.o guest-agent-command-state.o
 qga-nested-y += channel-posix.o
 qga-obj-y = $(addprefix qga/, $(qga-nested-y))
 qga-obj-y += qemu-ga.o qemu-sockets.o module.o qemu-option.o
diff --git a/qga/commands.c b/qga/commands.c
new file mode 100644
index 0000000..b27407d
--- /dev/null
+++ b/qga/commands.c
@@ -0,0 +1,73 @@
+/*
+ * QEMU Guest Agent common/cross-platform command implementations
+ *
+ * Copyright IBM Corp. 2012
+ *
+ * Authors:
+ *  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 <glib.h>
+#include "qga/guest-agent-core.h"
+#include "qga-qmp-commands.h"
+#include "qerror.h"
+
+/* Note: in some situations, like with the fsfreeze, logging may be
+ * temporarilly disabled. if it is necessary that a command be able
+ * to log for accounting purposes, check ga_logging_enabled() beforehand,
+ * and use the QERR_QGA_LOGGING_DISABLED to generate an error
+ */
+void slog(const gchar *fmt, ...)
+{
+    va_list ap;
+
+    va_start(ap, fmt);
+    g_logv("syslog", G_LOG_LEVEL_INFO, fmt, ap);
+    va_end(ap);
+}
+
+int64_t qmp_guest_sync(int64_t id, Error **errp)
+{
+    return id;
+}
+
+void qmp_guest_ping(Error **err)
+{
+    slog("guest-ping called");
+}
+
+struct GuestAgentInfo *qmp_guest_info(Error **err)
+{
+    GuestAgentInfo *info = g_malloc0(sizeof(GuestAgentInfo));
+    GuestAgentCommandInfo *cmd_info;
+    GuestAgentCommandInfoList *cmd_info_list;
+    char **cmd_list_head, **cmd_list;
+
+    info->version = g_strdup(QGA_VERSION);
+
+    cmd_list_head = cmd_list = qmp_get_command_list();
+    if (*cmd_list_head == NULL) {
+        goto out;
+    }
+
+    while (*cmd_list) {
+        cmd_info = g_malloc0(sizeof(GuestAgentCommandInfo));
+        cmd_info->name = strdup(*cmd_list);
+        cmd_info->enabled = qmp_command_is_enabled(cmd_info->name);
+
+        cmd_info_list = g_malloc0(sizeof(GuestAgentCommandInfoList));
+        cmd_info_list->value = cmd_info;
+        cmd_info_list->next = info->supported_commands;
+        info->supported_commands = cmd_info_list;
+
+        g_free(*cmd_list);
+        cmd_list++;
+    }
+
+out:
+    g_free(cmd_list_head);
+    return info;
+}
diff --git a/qga/guest-agent-commands.c b/qga/guest-agent-commands.c
index a09c8ca..126127a 100644
--- a/qga/guest-agent-commands.c
+++ b/qga/guest-agent-commands.c
@@ -1,5 +1,5 @@
 /*
- * QEMU Guest Agent commands
+ * QEMU Guest Agent POSIX-specific command implementations
  *
  * Copyright IBM Corp. 2011
  *
@@ -30,63 +30,6 @@
 
 static GAState *ga_state;
 
-/* Note: in some situations, like with the fsfreeze, logging may be
- * temporarilly disabled. if it is necessary that a command be able
- * to log for accounting purposes, check ga_logging_enabled() beforehand,
- * and use the QERR_QGA_LOGGING_DISABLED to generate an error
- */
-static void slog(const char *fmt, ...)
-{
-    va_list ap;
-
-    va_start(ap, fmt);
-    g_logv("syslog", G_LOG_LEVEL_INFO, fmt, ap);
-    va_end(ap);
-}
-
-int64_t qmp_guest_sync(int64_t id, Error **errp)
-{
-    return id;
-}
-
-void qmp_guest_ping(Error **err)
-{
-    slog("guest-ping called");
-}
-
-struct GuestAgentInfo *qmp_guest_info(Error **err)
-{
-    GuestAgentInfo *info = g_malloc0(sizeof(GuestAgentInfo));
-    GuestAgentCommandInfo *cmd_info;
-    GuestAgentCommandInfoList *cmd_info_list;
-    char **cmd_list_head, **cmd_list;
-
-    info->version = g_strdup(QGA_VERSION);
-
-    cmd_list_head = cmd_list = qmp_get_command_list();
-    if (*cmd_list_head == NULL) {
-        goto out;
-    }
-
-    while (*cmd_list) {
-        cmd_info = g_malloc0(sizeof(GuestAgentCommandInfo));
-        cmd_info->name = strdup(*cmd_list);
-        cmd_info->enabled = qmp_command_is_enabled(cmd_info->name);
-
-        cmd_info_list = g_malloc0(sizeof(GuestAgentCommandInfoList));
-        cmd_info_list->value = cmd_info;
-        cmd_info_list->next = info->supported_commands;
-        info->supported_commands = cmd_info_list;
-
-        g_free(*cmd_list);
-        cmd_list++;
-    }
-
-out:
-    g_free(cmd_list_head);
-    return info;
-}
-
 void qmp_guest_shutdown(bool has_mode, const char *mode, Error **err)
 {
     int ret;
diff --git a/qga/guest-agent-core.h b/qga/guest-agent-core.h
index 6148d10..b5dfa5b 100644
--- a/qga/guest-agent-core.h
+++ b/qga/guest-agent-core.h
@@ -29,3 +29,4 @@ GACommandState *ga_command_state_new(void);
 bool ga_logging_enabled(GAState *s);
 void ga_disable_logging(GAState *s);
 void ga_enable_logging(GAState *s);
+void slog(const gchar *fmt, ...);
-- 
1.7.4.1

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [Qemu-devel] [PATCH v2 4/8] qemu-ga: rename guest-agent-commands.c -> commands-posix.c
  2012-02-02 19:58 [Qemu-devel] [PATCH v2 0/8] qemu-ga: add support for Windows Michael Roth
                   ` (2 preceding siblings ...)
  2012-02-02 19:58 ` [Qemu-devel] [PATCH v2 3/8] qemu-ga: separate out common commands from posix-specific ones Michael Roth
@ 2012-02-02 19:58 ` Michael Roth
  2012-02-02 19:58 ` [Qemu-devel] [PATCH v2 5/8] qemu-ga: fixes for win32 build of qemu-ga Michael Roth
                   ` (4 subsequent siblings)
  8 siblings, 0 replies; 21+ messages in thread
From: Michael Roth @ 2012-02-02 19:58 UTC (permalink / raw)
  To: qemu-devel; +Cc: ghammer, aliguori, mdroth, lcapitulino


Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
 Makefile.objs              |    2 +-
 qga/commands-posix.c       |  528 ++++++++++++++++++++++++++++++++++++++++++++
 qga/guest-agent-commands.c |  528 --------------------------------------------
 3 files changed, 529 insertions(+), 529 deletions(-)
 create mode 100644 qga/commands-posix.c
 delete mode 100644 qga/guest-agent-commands.c

diff --git a/Makefile.objs b/Makefile.objs
index d70cebe..2e2efb4 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -424,7 +424,7 @@ common-obj-y += qmp.o hmp.o
 ######################################################################
 # guest agent
 
-qga-nested-y = commands.o guest-agent-commands.o guest-agent-command-state.o
+qga-nested-y = commands.o commands-posix.o guest-agent-command-state.o
 qga-nested-y += channel-posix.o
 qga-obj-y = $(addprefix qga/, $(qga-nested-y))
 qga-obj-y += qemu-ga.o qemu-sockets.o module.o qemu-option.o
diff --git a/qga/commands-posix.c b/qga/commands-posix.c
new file mode 100644
index 0000000..126127a
--- /dev/null
+++ b/qga/commands-posix.c
@@ -0,0 +1,528 @@
+/*
+ * QEMU Guest Agent POSIX-specific command implementations
+ *
+ * Copyright IBM Corp. 2011
+ *
+ * Authors:
+ *  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 <glib.h>
+
+#if defined(__linux__)
+#include <mntent.h>
+#include <linux/fs.h>
+
+#if defined(__linux__) && defined(FIFREEZE)
+#define CONFIG_FSFREEZE
+#endif
+#endif
+
+#include <sys/types.h>
+#include <sys/ioctl.h>
+#include "qga/guest-agent-core.h"
+#include "qga-qmp-commands.h"
+#include "qerror.h"
+#include "qemu-queue.h"
+
+static GAState *ga_state;
+
+void qmp_guest_shutdown(bool has_mode, const char *mode, Error **err)
+{
+    int ret;
+    const char *shutdown_flag;
+
+    slog("guest-shutdown called, mode: %s", mode);
+    if (!has_mode || strcmp(mode, "powerdown") == 0) {
+        shutdown_flag = "-P";
+    } else if (strcmp(mode, "halt") == 0) {
+        shutdown_flag = "-H";
+    } else if (strcmp(mode, "reboot") == 0) {
+        shutdown_flag = "-r";
+    } else {
+        error_set(err, QERR_INVALID_PARAMETER_VALUE, "mode",
+                  "halt|powerdown|reboot");
+        return;
+    }
+
+    ret = fork();
+    if (ret == 0) {
+        /* child, start the shutdown */
+        setsid();
+        fclose(stdin);
+        fclose(stdout);
+        fclose(stderr);
+
+        ret = execl("/sbin/shutdown", "shutdown", shutdown_flag, "+0",
+                    "hypervisor initiated shutdown", (char*)NULL);
+        if (ret) {
+            slog("guest-shutdown failed: %s", strerror(errno));
+        }
+        exit(!!ret);
+    } else if (ret < 0) {
+        error_set(err, QERR_UNDEFINED_ERROR);
+    }
+}
+
+typedef struct GuestFileHandle {
+    uint64_t id;
+    FILE *fh;
+    QTAILQ_ENTRY(GuestFileHandle) next;
+} GuestFileHandle;
+
+static struct {
+    QTAILQ_HEAD(, GuestFileHandle) filehandles;
+} guest_file_state;
+
+static void guest_file_handle_add(FILE *fh)
+{
+    GuestFileHandle *gfh;
+
+    gfh = g_malloc0(sizeof(GuestFileHandle));
+    gfh->id = fileno(fh);
+    gfh->fh = fh;
+    QTAILQ_INSERT_TAIL(&guest_file_state.filehandles, gfh, next);
+}
+
+static GuestFileHandle *guest_file_handle_find(int64_t id)
+{
+    GuestFileHandle *gfh;
+
+    QTAILQ_FOREACH(gfh, &guest_file_state.filehandles, next)
+    {
+        if (gfh->id == id) {
+            return gfh;
+        }
+    }
+
+    return NULL;
+}
+
+int64_t qmp_guest_file_open(const char *path, bool has_mode, const char *mode, Error **err)
+{
+    FILE *fh;
+    int fd;
+    int64_t ret = -1;
+
+    if (!has_mode) {
+        mode = "r";
+    }
+    slog("guest-file-open called, filepath: %s, mode: %s", path, mode);
+    fh = fopen(path, mode);
+    if (!fh) {
+        error_set(err, QERR_OPEN_FILE_FAILED, path);
+        return -1;
+    }
+
+    /* set fd non-blocking to avoid common use cases (like reading from a
+     * named pipe) from hanging the agent
+     */
+    fd = fileno(fh);
+    ret = fcntl(fd, F_GETFL);
+    ret = fcntl(fd, F_SETFL, ret | O_NONBLOCK);
+    if (ret == -1) {
+        error_set(err, QERR_QGA_COMMAND_FAILED, "fcntl() failed");
+        fclose(fh);
+        return -1;
+    }
+
+    guest_file_handle_add(fh);
+    slog("guest-file-open, handle: %d", fd);
+    return fd;
+}
+
+void qmp_guest_file_close(int64_t handle, Error **err)
+{
+    GuestFileHandle *gfh = guest_file_handle_find(handle);
+    int ret;
+
+    slog("guest-file-close called, handle: %ld", handle);
+    if (!gfh) {
+        error_set(err, QERR_FD_NOT_FOUND, "handle");
+        return;
+    }
+
+    ret = fclose(gfh->fh);
+    if (ret == -1) {
+        error_set(err, QERR_QGA_COMMAND_FAILED, "fclose() failed");
+        return;
+    }
+
+    QTAILQ_REMOVE(&guest_file_state.filehandles, gfh, next);
+    g_free(gfh);
+}
+
+struct GuestFileRead *qmp_guest_file_read(int64_t handle, bool has_count,
+                                          int64_t count, Error **err)
+{
+    GuestFileHandle *gfh = guest_file_handle_find(handle);
+    GuestFileRead *read_data = NULL;
+    guchar *buf;
+    FILE *fh;
+    size_t read_count;
+
+    if (!gfh) {
+        error_set(err, QERR_FD_NOT_FOUND, "handle");
+        return NULL;
+    }
+
+    if (!has_count) {
+        count = QGA_READ_COUNT_DEFAULT;
+    } else if (count < 0) {
+        error_set(err, QERR_INVALID_PARAMETER, "count");
+        return NULL;
+    }
+
+    fh = gfh->fh;
+    buf = g_malloc0(count+1);
+    read_count = fread(buf, 1, count, fh);
+    if (ferror(fh)) {
+        slog("guest-file-read failed, handle: %ld", handle);
+        error_set(err, QERR_QGA_COMMAND_FAILED, "fread() failed");
+    } else {
+        buf[read_count] = 0;
+        read_data = g_malloc0(sizeof(GuestFileRead));
+        read_data->count = read_count;
+        read_data->eof = feof(fh);
+        if (read_count) {
+            read_data->buf_b64 = g_base64_encode(buf, read_count);
+        }
+    }
+    g_free(buf);
+    clearerr(fh);
+
+    return read_data;
+}
+
+GuestFileWrite *qmp_guest_file_write(int64_t handle, const char *buf_b64,
+                                     bool has_count, int64_t count, Error **err)
+{
+    GuestFileWrite *write_data = NULL;
+    guchar *buf;
+    gsize buf_len;
+    int write_count;
+    GuestFileHandle *gfh = guest_file_handle_find(handle);
+    FILE *fh;
+
+    if (!gfh) {
+        error_set(err, QERR_FD_NOT_FOUND, "handle");
+        return NULL;
+    }
+
+    fh = gfh->fh;
+    buf = g_base64_decode(buf_b64, &buf_len);
+
+    if (!has_count) {
+        count = buf_len;
+    } else if (count < 0 || count > buf_len) {
+        g_free(buf);
+        error_set(err, QERR_INVALID_PARAMETER, "count");
+        return NULL;
+    }
+
+    write_count = fwrite(buf, 1, count, fh);
+    if (ferror(fh)) {
+        slog("guest-file-write failed, handle: %ld", handle);
+        error_set(err, QERR_QGA_COMMAND_FAILED, "fwrite() error");
+    } else {
+        write_data = g_malloc0(sizeof(GuestFileWrite));
+        write_data->count = write_count;
+        write_data->eof = feof(fh);
+    }
+    g_free(buf);
+    clearerr(fh);
+
+    return write_data;
+}
+
+struct GuestFileSeek *qmp_guest_file_seek(int64_t handle, int64_t offset,
+                                          int64_t whence, Error **err)
+{
+    GuestFileHandle *gfh = guest_file_handle_find(handle);
+    GuestFileSeek *seek_data = NULL;
+    FILE *fh;
+    int ret;
+
+    if (!gfh) {
+        error_set(err, QERR_FD_NOT_FOUND, "handle");
+        return NULL;
+    }
+
+    fh = gfh->fh;
+    ret = fseek(fh, offset, whence);
+    if (ret == -1) {
+        error_set(err, QERR_QGA_COMMAND_FAILED, strerror(errno));
+    } else {
+        seek_data = g_malloc0(sizeof(GuestFileRead));
+        seek_data->position = ftell(fh);
+        seek_data->eof = feof(fh);
+    }
+    clearerr(fh);
+
+    return seek_data;
+}
+
+void qmp_guest_file_flush(int64_t handle, Error **err)
+{
+    GuestFileHandle *gfh = guest_file_handle_find(handle);
+    FILE *fh;
+    int ret;
+
+    if (!gfh) {
+        error_set(err, QERR_FD_NOT_FOUND, "handle");
+        return;
+    }
+
+    fh = gfh->fh;
+    ret = fflush(fh);
+    if (ret == EOF) {
+        error_set(err, QERR_QGA_COMMAND_FAILED, strerror(errno));
+    }
+}
+
+static void guest_file_init(void)
+{
+    QTAILQ_INIT(&guest_file_state.filehandles);
+}
+
+#if defined(CONFIG_FSFREEZE)
+static void disable_logging(void)
+{
+    ga_disable_logging(ga_state);
+}
+
+static void enable_logging(void)
+{
+    ga_enable_logging(ga_state);
+}
+
+typedef struct GuestFsfreezeMount {
+    char *dirname;
+    char *devtype;
+    QTAILQ_ENTRY(GuestFsfreezeMount) next;
+} GuestFsfreezeMount;
+
+struct {
+    GuestFsfreezeStatus status;
+    QTAILQ_HEAD(, GuestFsfreezeMount) mount_list;
+} guest_fsfreeze_state;
+
+/*
+ * Walk the mount table and build a list of local file systems
+ */
+static int guest_fsfreeze_build_mount_list(void)
+{
+    struct mntent *ment;
+    GuestFsfreezeMount *mount, *temp;
+    char const *mtab = MOUNTED;
+    FILE *fp;
+
+    QTAILQ_FOREACH_SAFE(mount, &guest_fsfreeze_state.mount_list, next, temp) {
+        QTAILQ_REMOVE(&guest_fsfreeze_state.mount_list, mount, next);
+        g_free(mount->dirname);
+        g_free(mount->devtype);
+        g_free(mount);
+    }
+
+    fp = setmntent(mtab, "r");
+    if (!fp) {
+        g_warning("fsfreeze: unable to read mtab");
+        return -1;
+    }
+
+    while ((ment = getmntent(fp))) {
+        /*
+         * An entry which device name doesn't start with a '/' is
+         * either a dummy file system or a network file system.
+         * Add special handling for smbfs and cifs as is done by
+         * coreutils as well.
+         */
+        if ((ment->mnt_fsname[0] != '/') ||
+            (strcmp(ment->mnt_type, "smbfs") == 0) ||
+            (strcmp(ment->mnt_type, "cifs") == 0)) {
+            continue;
+        }
+
+        mount = g_malloc0(sizeof(GuestFsfreezeMount));
+        mount->dirname = g_strdup(ment->mnt_dir);
+        mount->devtype = g_strdup(ment->mnt_type);
+
+        QTAILQ_INSERT_TAIL(&guest_fsfreeze_state.mount_list, mount, next);
+    }
+
+    endmntent(fp);
+
+    return 0;
+}
+
+/*
+ * Return status of freeze/thaw
+ */
+GuestFsfreezeStatus qmp_guest_fsfreeze_status(Error **err)
+{
+    return guest_fsfreeze_state.status;
+}
+
+/*
+ * Walk list of mounted file systems in the guest, and freeze the ones which
+ * are real local file systems.
+ */
+int64_t qmp_guest_fsfreeze_freeze(Error **err)
+{
+    int ret = 0, i = 0;
+    struct GuestFsfreezeMount *mount, *temp;
+    int fd;
+    char err_msg[512];
+
+    slog("guest-fsfreeze called");
+
+    if (guest_fsfreeze_state.status == GUEST_FSFREEZE_STATUS_FROZEN) {
+        return 0;
+    }
+
+    ret = guest_fsfreeze_build_mount_list();
+    if (ret < 0) {
+        return ret;
+    }
+
+    /* cannot risk guest agent blocking itself on a write in this state */
+    disable_logging();
+
+    QTAILQ_FOREACH_SAFE(mount, &guest_fsfreeze_state.mount_list, next, temp) {
+        fd = qemu_open(mount->dirname, O_RDONLY);
+        if (fd == -1) {
+            sprintf(err_msg, "failed to open %s, %s", mount->dirname, strerror(errno));
+            error_set(err, QERR_QGA_COMMAND_FAILED, err_msg);
+            goto error;
+        }
+
+        /* we try to cull filesytems we know won't work in advance, but other
+         * filesytems may not implement fsfreeze for less obvious reasons.
+         * these will report EOPNOTSUPP, so we simply ignore them. when
+         * thawing, these filesystems will return an EINVAL instead, due to
+         * not being in a frozen state. Other filesystem-specific
+         * errors may result in EINVAL, however, so the user should check the
+         * number * of filesystems returned here against those returned by the
+         * thaw operation to determine whether everything completed
+         * successfully
+         */
+        ret = ioctl(fd, FIFREEZE);
+        if (ret < 0 && errno != EOPNOTSUPP) {
+            sprintf(err_msg, "failed to freeze %s, %s", mount->dirname, strerror(errno));
+            error_set(err, QERR_QGA_COMMAND_FAILED, err_msg);
+            close(fd);
+            goto error;
+        }
+        close(fd);
+
+        i++;
+    }
+
+    guest_fsfreeze_state.status = GUEST_FSFREEZE_STATUS_FROZEN;
+    return i;
+
+error:
+    if (i > 0) {
+        qmp_guest_fsfreeze_thaw(NULL);
+    }
+    return 0;
+}
+
+/*
+ * Walk list of frozen file systems in the guest, and thaw them.
+ */
+int64_t qmp_guest_fsfreeze_thaw(Error **err)
+{
+    int ret;
+    GuestFsfreezeMount *mount, *temp;
+    int fd, i = 0;
+    bool has_error = false;
+
+    QTAILQ_FOREACH_SAFE(mount, &guest_fsfreeze_state.mount_list, next, temp) {
+        fd = qemu_open(mount->dirname, O_RDONLY);
+        if (fd == -1) {
+            has_error = true;
+            continue;
+        }
+        ret = ioctl(fd, FITHAW);
+        if (ret < 0 && errno != EOPNOTSUPP && errno != EINVAL) {
+            has_error = true;
+            close(fd);
+            continue;
+        }
+        close(fd);
+        i++;
+    }
+
+    if (has_error) {
+        guest_fsfreeze_state.status = GUEST_FSFREEZE_STATUS_ERROR;
+    } else {
+        guest_fsfreeze_state.status = GUEST_FSFREEZE_STATUS_THAWED;
+    }
+    enable_logging();
+    return i;
+}
+
+static void guest_fsfreeze_init(void)
+{
+    guest_fsfreeze_state.status = GUEST_FSFREEZE_STATUS_THAWED;
+    QTAILQ_INIT(&guest_fsfreeze_state.mount_list);
+}
+
+static void guest_fsfreeze_cleanup(void)
+{
+    int64_t ret;
+    Error *err = NULL;
+
+    if (guest_fsfreeze_state.status == GUEST_FSFREEZE_STATUS_FROZEN) {
+        ret = qmp_guest_fsfreeze_thaw(&err);
+        if (ret < 0 || err) {
+            slog("failed to clean up frozen filesystems");
+        }
+    }
+}
+#else
+/*
+ * Return status of freeze/thaw
+ */
+GuestFsfreezeStatus qmp_guest_fsfreeze_status(Error **err)
+{
+    error_set(err, QERR_UNSUPPORTED);
+
+    return 0;
+}
+
+/*
+ * Walk list of mounted file systems in the guest, and freeze the ones which
+ * are real local file systems.
+ */
+int64_t qmp_guest_fsfreeze_freeze(Error **err)
+{
+    error_set(err, QERR_UNSUPPORTED);
+
+    return 0;
+}
+
+/*
+ * Walk list of frozen file systems in the guest, and thaw them.
+ */
+int64_t qmp_guest_fsfreeze_thaw(Error **err)
+{
+    error_set(err, QERR_UNSUPPORTED);
+
+    return 0;
+}
+#endif
+
+/* register init/cleanup routines for stateful command groups */
+void ga_command_state_init(GAState *s, GACommandState *cs)
+{
+    ga_state = s;
+#if defined(CONFIG_FSFREEZE)
+    ga_command_state_add(cs, guest_fsfreeze_init, guest_fsfreeze_cleanup);
+#endif
+    ga_command_state_add(cs, guest_file_init, NULL);
+}
diff --git a/qga/guest-agent-commands.c b/qga/guest-agent-commands.c
deleted file mode 100644
index 126127a..0000000
--- a/qga/guest-agent-commands.c
+++ /dev/null
@@ -1,528 +0,0 @@
-/*
- * QEMU Guest Agent POSIX-specific command implementations
- *
- * Copyright IBM Corp. 2011
- *
- * Authors:
- *  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 <glib.h>
-
-#if defined(__linux__)
-#include <mntent.h>
-#include <linux/fs.h>
-
-#if defined(__linux__) && defined(FIFREEZE)
-#define CONFIG_FSFREEZE
-#endif
-#endif
-
-#include <sys/types.h>
-#include <sys/ioctl.h>
-#include "qga/guest-agent-core.h"
-#include "qga-qmp-commands.h"
-#include "qerror.h"
-#include "qemu-queue.h"
-
-static GAState *ga_state;
-
-void qmp_guest_shutdown(bool has_mode, const char *mode, Error **err)
-{
-    int ret;
-    const char *shutdown_flag;
-
-    slog("guest-shutdown called, mode: %s", mode);
-    if (!has_mode || strcmp(mode, "powerdown") == 0) {
-        shutdown_flag = "-P";
-    } else if (strcmp(mode, "halt") == 0) {
-        shutdown_flag = "-H";
-    } else if (strcmp(mode, "reboot") == 0) {
-        shutdown_flag = "-r";
-    } else {
-        error_set(err, QERR_INVALID_PARAMETER_VALUE, "mode",
-                  "halt|powerdown|reboot");
-        return;
-    }
-
-    ret = fork();
-    if (ret == 0) {
-        /* child, start the shutdown */
-        setsid();
-        fclose(stdin);
-        fclose(stdout);
-        fclose(stderr);
-
-        ret = execl("/sbin/shutdown", "shutdown", shutdown_flag, "+0",
-                    "hypervisor initiated shutdown", (char*)NULL);
-        if (ret) {
-            slog("guest-shutdown failed: %s", strerror(errno));
-        }
-        exit(!!ret);
-    } else if (ret < 0) {
-        error_set(err, QERR_UNDEFINED_ERROR);
-    }
-}
-
-typedef struct GuestFileHandle {
-    uint64_t id;
-    FILE *fh;
-    QTAILQ_ENTRY(GuestFileHandle) next;
-} GuestFileHandle;
-
-static struct {
-    QTAILQ_HEAD(, GuestFileHandle) filehandles;
-} guest_file_state;
-
-static void guest_file_handle_add(FILE *fh)
-{
-    GuestFileHandle *gfh;
-
-    gfh = g_malloc0(sizeof(GuestFileHandle));
-    gfh->id = fileno(fh);
-    gfh->fh = fh;
-    QTAILQ_INSERT_TAIL(&guest_file_state.filehandles, gfh, next);
-}
-
-static GuestFileHandle *guest_file_handle_find(int64_t id)
-{
-    GuestFileHandle *gfh;
-
-    QTAILQ_FOREACH(gfh, &guest_file_state.filehandles, next)
-    {
-        if (gfh->id == id) {
-            return gfh;
-        }
-    }
-
-    return NULL;
-}
-
-int64_t qmp_guest_file_open(const char *path, bool has_mode, const char *mode, Error **err)
-{
-    FILE *fh;
-    int fd;
-    int64_t ret = -1;
-
-    if (!has_mode) {
-        mode = "r";
-    }
-    slog("guest-file-open called, filepath: %s, mode: %s", path, mode);
-    fh = fopen(path, mode);
-    if (!fh) {
-        error_set(err, QERR_OPEN_FILE_FAILED, path);
-        return -1;
-    }
-
-    /* set fd non-blocking to avoid common use cases (like reading from a
-     * named pipe) from hanging the agent
-     */
-    fd = fileno(fh);
-    ret = fcntl(fd, F_GETFL);
-    ret = fcntl(fd, F_SETFL, ret | O_NONBLOCK);
-    if (ret == -1) {
-        error_set(err, QERR_QGA_COMMAND_FAILED, "fcntl() failed");
-        fclose(fh);
-        return -1;
-    }
-
-    guest_file_handle_add(fh);
-    slog("guest-file-open, handle: %d", fd);
-    return fd;
-}
-
-void qmp_guest_file_close(int64_t handle, Error **err)
-{
-    GuestFileHandle *gfh = guest_file_handle_find(handle);
-    int ret;
-
-    slog("guest-file-close called, handle: %ld", handle);
-    if (!gfh) {
-        error_set(err, QERR_FD_NOT_FOUND, "handle");
-        return;
-    }
-
-    ret = fclose(gfh->fh);
-    if (ret == -1) {
-        error_set(err, QERR_QGA_COMMAND_FAILED, "fclose() failed");
-        return;
-    }
-
-    QTAILQ_REMOVE(&guest_file_state.filehandles, gfh, next);
-    g_free(gfh);
-}
-
-struct GuestFileRead *qmp_guest_file_read(int64_t handle, bool has_count,
-                                          int64_t count, Error **err)
-{
-    GuestFileHandle *gfh = guest_file_handle_find(handle);
-    GuestFileRead *read_data = NULL;
-    guchar *buf;
-    FILE *fh;
-    size_t read_count;
-
-    if (!gfh) {
-        error_set(err, QERR_FD_NOT_FOUND, "handle");
-        return NULL;
-    }
-
-    if (!has_count) {
-        count = QGA_READ_COUNT_DEFAULT;
-    } else if (count < 0) {
-        error_set(err, QERR_INVALID_PARAMETER, "count");
-        return NULL;
-    }
-
-    fh = gfh->fh;
-    buf = g_malloc0(count+1);
-    read_count = fread(buf, 1, count, fh);
-    if (ferror(fh)) {
-        slog("guest-file-read failed, handle: %ld", handle);
-        error_set(err, QERR_QGA_COMMAND_FAILED, "fread() failed");
-    } else {
-        buf[read_count] = 0;
-        read_data = g_malloc0(sizeof(GuestFileRead));
-        read_data->count = read_count;
-        read_data->eof = feof(fh);
-        if (read_count) {
-            read_data->buf_b64 = g_base64_encode(buf, read_count);
-        }
-    }
-    g_free(buf);
-    clearerr(fh);
-
-    return read_data;
-}
-
-GuestFileWrite *qmp_guest_file_write(int64_t handle, const char *buf_b64,
-                                     bool has_count, int64_t count, Error **err)
-{
-    GuestFileWrite *write_data = NULL;
-    guchar *buf;
-    gsize buf_len;
-    int write_count;
-    GuestFileHandle *gfh = guest_file_handle_find(handle);
-    FILE *fh;
-
-    if (!gfh) {
-        error_set(err, QERR_FD_NOT_FOUND, "handle");
-        return NULL;
-    }
-
-    fh = gfh->fh;
-    buf = g_base64_decode(buf_b64, &buf_len);
-
-    if (!has_count) {
-        count = buf_len;
-    } else if (count < 0 || count > buf_len) {
-        g_free(buf);
-        error_set(err, QERR_INVALID_PARAMETER, "count");
-        return NULL;
-    }
-
-    write_count = fwrite(buf, 1, count, fh);
-    if (ferror(fh)) {
-        slog("guest-file-write failed, handle: %ld", handle);
-        error_set(err, QERR_QGA_COMMAND_FAILED, "fwrite() error");
-    } else {
-        write_data = g_malloc0(sizeof(GuestFileWrite));
-        write_data->count = write_count;
-        write_data->eof = feof(fh);
-    }
-    g_free(buf);
-    clearerr(fh);
-
-    return write_data;
-}
-
-struct GuestFileSeek *qmp_guest_file_seek(int64_t handle, int64_t offset,
-                                          int64_t whence, Error **err)
-{
-    GuestFileHandle *gfh = guest_file_handle_find(handle);
-    GuestFileSeek *seek_data = NULL;
-    FILE *fh;
-    int ret;
-
-    if (!gfh) {
-        error_set(err, QERR_FD_NOT_FOUND, "handle");
-        return NULL;
-    }
-
-    fh = gfh->fh;
-    ret = fseek(fh, offset, whence);
-    if (ret == -1) {
-        error_set(err, QERR_QGA_COMMAND_FAILED, strerror(errno));
-    } else {
-        seek_data = g_malloc0(sizeof(GuestFileRead));
-        seek_data->position = ftell(fh);
-        seek_data->eof = feof(fh);
-    }
-    clearerr(fh);
-
-    return seek_data;
-}
-
-void qmp_guest_file_flush(int64_t handle, Error **err)
-{
-    GuestFileHandle *gfh = guest_file_handle_find(handle);
-    FILE *fh;
-    int ret;
-
-    if (!gfh) {
-        error_set(err, QERR_FD_NOT_FOUND, "handle");
-        return;
-    }
-
-    fh = gfh->fh;
-    ret = fflush(fh);
-    if (ret == EOF) {
-        error_set(err, QERR_QGA_COMMAND_FAILED, strerror(errno));
-    }
-}
-
-static void guest_file_init(void)
-{
-    QTAILQ_INIT(&guest_file_state.filehandles);
-}
-
-#if defined(CONFIG_FSFREEZE)
-static void disable_logging(void)
-{
-    ga_disable_logging(ga_state);
-}
-
-static void enable_logging(void)
-{
-    ga_enable_logging(ga_state);
-}
-
-typedef struct GuestFsfreezeMount {
-    char *dirname;
-    char *devtype;
-    QTAILQ_ENTRY(GuestFsfreezeMount) next;
-} GuestFsfreezeMount;
-
-struct {
-    GuestFsfreezeStatus status;
-    QTAILQ_HEAD(, GuestFsfreezeMount) mount_list;
-} guest_fsfreeze_state;
-
-/*
- * Walk the mount table and build a list of local file systems
- */
-static int guest_fsfreeze_build_mount_list(void)
-{
-    struct mntent *ment;
-    GuestFsfreezeMount *mount, *temp;
-    char const *mtab = MOUNTED;
-    FILE *fp;
-
-    QTAILQ_FOREACH_SAFE(mount, &guest_fsfreeze_state.mount_list, next, temp) {
-        QTAILQ_REMOVE(&guest_fsfreeze_state.mount_list, mount, next);
-        g_free(mount->dirname);
-        g_free(mount->devtype);
-        g_free(mount);
-    }
-
-    fp = setmntent(mtab, "r");
-    if (!fp) {
-        g_warning("fsfreeze: unable to read mtab");
-        return -1;
-    }
-
-    while ((ment = getmntent(fp))) {
-        /*
-         * An entry which device name doesn't start with a '/' is
-         * either a dummy file system or a network file system.
-         * Add special handling for smbfs and cifs as is done by
-         * coreutils as well.
-         */
-        if ((ment->mnt_fsname[0] != '/') ||
-            (strcmp(ment->mnt_type, "smbfs") == 0) ||
-            (strcmp(ment->mnt_type, "cifs") == 0)) {
-            continue;
-        }
-
-        mount = g_malloc0(sizeof(GuestFsfreezeMount));
-        mount->dirname = g_strdup(ment->mnt_dir);
-        mount->devtype = g_strdup(ment->mnt_type);
-
-        QTAILQ_INSERT_TAIL(&guest_fsfreeze_state.mount_list, mount, next);
-    }
-
-    endmntent(fp);
-
-    return 0;
-}
-
-/*
- * Return status of freeze/thaw
- */
-GuestFsfreezeStatus qmp_guest_fsfreeze_status(Error **err)
-{
-    return guest_fsfreeze_state.status;
-}
-
-/*
- * Walk list of mounted file systems in the guest, and freeze the ones which
- * are real local file systems.
- */
-int64_t qmp_guest_fsfreeze_freeze(Error **err)
-{
-    int ret = 0, i = 0;
-    struct GuestFsfreezeMount *mount, *temp;
-    int fd;
-    char err_msg[512];
-
-    slog("guest-fsfreeze called");
-
-    if (guest_fsfreeze_state.status == GUEST_FSFREEZE_STATUS_FROZEN) {
-        return 0;
-    }
-
-    ret = guest_fsfreeze_build_mount_list();
-    if (ret < 0) {
-        return ret;
-    }
-
-    /* cannot risk guest agent blocking itself on a write in this state */
-    disable_logging();
-
-    QTAILQ_FOREACH_SAFE(mount, &guest_fsfreeze_state.mount_list, next, temp) {
-        fd = qemu_open(mount->dirname, O_RDONLY);
-        if (fd == -1) {
-            sprintf(err_msg, "failed to open %s, %s", mount->dirname, strerror(errno));
-            error_set(err, QERR_QGA_COMMAND_FAILED, err_msg);
-            goto error;
-        }
-
-        /* we try to cull filesytems we know won't work in advance, but other
-         * filesytems may not implement fsfreeze for less obvious reasons.
-         * these will report EOPNOTSUPP, so we simply ignore them. when
-         * thawing, these filesystems will return an EINVAL instead, due to
-         * not being in a frozen state. Other filesystem-specific
-         * errors may result in EINVAL, however, so the user should check the
-         * number * of filesystems returned here against those returned by the
-         * thaw operation to determine whether everything completed
-         * successfully
-         */
-        ret = ioctl(fd, FIFREEZE);
-        if (ret < 0 && errno != EOPNOTSUPP) {
-            sprintf(err_msg, "failed to freeze %s, %s", mount->dirname, strerror(errno));
-            error_set(err, QERR_QGA_COMMAND_FAILED, err_msg);
-            close(fd);
-            goto error;
-        }
-        close(fd);
-
-        i++;
-    }
-
-    guest_fsfreeze_state.status = GUEST_FSFREEZE_STATUS_FROZEN;
-    return i;
-
-error:
-    if (i > 0) {
-        qmp_guest_fsfreeze_thaw(NULL);
-    }
-    return 0;
-}
-
-/*
- * Walk list of frozen file systems in the guest, and thaw them.
- */
-int64_t qmp_guest_fsfreeze_thaw(Error **err)
-{
-    int ret;
-    GuestFsfreezeMount *mount, *temp;
-    int fd, i = 0;
-    bool has_error = false;
-
-    QTAILQ_FOREACH_SAFE(mount, &guest_fsfreeze_state.mount_list, next, temp) {
-        fd = qemu_open(mount->dirname, O_RDONLY);
-        if (fd == -1) {
-            has_error = true;
-            continue;
-        }
-        ret = ioctl(fd, FITHAW);
-        if (ret < 0 && errno != EOPNOTSUPP && errno != EINVAL) {
-            has_error = true;
-            close(fd);
-            continue;
-        }
-        close(fd);
-        i++;
-    }
-
-    if (has_error) {
-        guest_fsfreeze_state.status = GUEST_FSFREEZE_STATUS_ERROR;
-    } else {
-        guest_fsfreeze_state.status = GUEST_FSFREEZE_STATUS_THAWED;
-    }
-    enable_logging();
-    return i;
-}
-
-static void guest_fsfreeze_init(void)
-{
-    guest_fsfreeze_state.status = GUEST_FSFREEZE_STATUS_THAWED;
-    QTAILQ_INIT(&guest_fsfreeze_state.mount_list);
-}
-
-static void guest_fsfreeze_cleanup(void)
-{
-    int64_t ret;
-    Error *err = NULL;
-
-    if (guest_fsfreeze_state.status == GUEST_FSFREEZE_STATUS_FROZEN) {
-        ret = qmp_guest_fsfreeze_thaw(&err);
-        if (ret < 0 || err) {
-            slog("failed to clean up frozen filesystems");
-        }
-    }
-}
-#else
-/*
- * Return status of freeze/thaw
- */
-GuestFsfreezeStatus qmp_guest_fsfreeze_status(Error **err)
-{
-    error_set(err, QERR_UNSUPPORTED);
-
-    return 0;
-}
-
-/*
- * Walk list of mounted file systems in the guest, and freeze the ones which
- * are real local file systems.
- */
-int64_t qmp_guest_fsfreeze_freeze(Error **err)
-{
-    error_set(err, QERR_UNSUPPORTED);
-
-    return 0;
-}
-
-/*
- * Walk list of frozen file systems in the guest, and thaw them.
- */
-int64_t qmp_guest_fsfreeze_thaw(Error **err)
-{
-    error_set(err, QERR_UNSUPPORTED);
-
-    return 0;
-}
-#endif
-
-/* register init/cleanup routines for stateful command groups */
-void ga_command_state_init(GAState *s, GACommandState *cs)
-{
-    ga_state = s;
-#if defined(CONFIG_FSFREEZE)
-    ga_command_state_add(cs, guest_fsfreeze_init, guest_fsfreeze_cleanup);
-#endif
-    ga_command_state_add(cs, guest_file_init, NULL);
-}
-- 
1.7.4.1

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [Qemu-devel] [PATCH v2 5/8] qemu-ga: fixes for win32 build of qemu-ga
  2012-02-02 19:58 [Qemu-devel] [PATCH v2 0/8] qemu-ga: add support for Windows Michael Roth
                   ` (3 preceding siblings ...)
  2012-02-02 19:58 ` [Qemu-devel] [PATCH v2 4/8] qemu-ga: rename guest-agent-commands.c -> commands-posix.c Michael Roth
@ 2012-02-02 19:58 ` Michael Roth
  2012-02-02 19:58 ` [Qemu-devel] [PATCH v2 6/8] qemu-ga: add initial win32 support Michael Roth
                   ` (3 subsequent siblings)
  8 siblings, 0 replies; 21+ messages in thread
From: Michael Roth @ 2012-02-02 19:58 UTC (permalink / raw)
  To: qemu-devel; +Cc: ghammer, aliguori, mdroth, lcapitulino

Various stubs and #ifdefs to compile for Windows using mingw
cross-build. Still has 1 linker error due to a dependency on the
forthcoming win32 versions of the GAChannel/transport class.

Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
 Makefile             |    2 +-
 Makefile.objs        |    9 +++--
 configure            |    2 +-
 qemu-ga.c            |   16 +++++++++
 qga/commands-win32.c |   91 ++++++++++++++++++++++++++++++++++++++++++++++++++
 5 files changed, 114 insertions(+), 6 deletions(-)
 create mode 100644 qga/commands-win32.c

diff --git a/Makefile b/Makefile
index 2560b59..9baa532 100644
--- a/Makefile
+++ b/Makefile
@@ -199,7 +199,7 @@ QGALIB_GEN=$(addprefix $(qapi-dir)/, qga-qapi-types.h qga-qapi-visit.h qga-qmp-c
 $(QGALIB_OBJ): $(QGALIB_GEN) $(GENERATED_HEADERS)
 $(qga-obj-y) qemu-ga.o: $(QGALIB_GEN) $(GENERATED_HEADERS)
 
-qemu-ga$(EXESUF): qemu-ga.o $(qga-obj-y) $(qapi-obj-y) $(tools-obj-y) $(qobject-obj-y) $(version-obj-y) $(QGALIB_OBJ)
+qemu-ga$(EXESUF): qemu-ga.o $(qga-obj-y) $(tools-obj-y) $(qapi-obj-y) $(qobject-obj-y) $(version-obj-y) $(QGALIB_OBJ)
 
 QEMULIBS=libhw32 libhw64 libuser libdis libdis-user
 
diff --git a/Makefile.objs b/Makefile.objs
index 2e2efb4..18e79ce 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -424,12 +424,13 @@ common-obj-y += qmp.o hmp.o
 ######################################################################
 # guest agent
 
-qga-nested-y = commands.o commands-posix.o guest-agent-command-state.o
-qga-nested-y += channel-posix.o
+qga-nested-y = commands.o guest-agent-command-state.o
+qga-nested-$(CONFIG_POSIX) += commands-posix.o channel-posix.o
+qga-nested-$(CONFIG_WIN32) += commands-win32.o
 qga-obj-y = $(addprefix qga/, $(qga-nested-y))
-qga-obj-y += qemu-ga.o qemu-sockets.o module.o qemu-option.o
+qga-obj-y += qemu-ga.o module.o
 qga-obj-$(CONFIG_WIN32) += oslib-win32.o
-qga-obj-$(CONFIG_POSIX) += oslib-posix.o
+qga-obj-$(CONFIG_POSIX) += oslib-posix.o qemu-sockets.o qemu-option.o
 
 vl.o: QEMU_CFLAGS+=$(GPROF_CFLAGS)
 
diff --git a/configure b/configure
index 3b0b300..aaf8171 100755
--- a/configure
+++ b/configure
@@ -508,7 +508,7 @@ if test "$mingw32" = "yes" ; then
   bindir="\${prefix}"
   sysconfdir="\${prefix}"
   confsuffix=""
-  guest_agent="no"
+  libs_qga="-lws2_32 -lwinmm $lib_qga"
 fi
 
 werror=""
diff --git a/qemu-ga.c b/qemu-ga.c
index 2e8af02..93ebc3e 100644
--- a/qemu-ga.c
+++ b/qemu-ga.c
@@ -15,7 +15,9 @@
 #include <stdbool.h>
 #include <glib.h>
 #include <getopt.h>
+#ifndef _WIN32
 #include <syslog.h>
+#endif
 #include "json-streamer.h"
 #include "json-parser.h"
 #include "qint.h"
@@ -44,6 +46,7 @@ struct GAState {
 
 static struct GAState *ga_state;
 
+#ifndef _WIN32
 static void quit_handler(int sig)
 {
     g_debug("received signal num %d, quitting", sig);
@@ -73,6 +76,7 @@ static gboolean register_signal_handlers(void)
     }
     return true;
 }
+#endif
 
 static void usage(const char *cmd)
 {
@@ -87,7 +91,9 @@ static void usage(const char *cmd)
 "  -f, --pidfile     specify pidfile (default is %s)\n"
 "  -v, --verbose     log extra debugging information\n"
 "  -V, --version     print version information and exit\n"
+#ifndef _WIN32
 "  -d, --daemonize   become a daemon\n"
+#endif
 "  -b, --blacklist   comma-separated list of RPCs to disable (no spaces, \"?\""
 "                    to list available RPCs)\n"
 "  -h, --help        display this help and exit\n"
@@ -143,9 +149,13 @@ static void ga_log(const gchar *domain, GLogLevelFlags level,
     }
 
     level &= G_LOG_LEVEL_MASK;
+#ifndef _WIN32
     if (domain && strcmp(domain, "syslog") == 0) {
         syslog(LOG_INFO, "%s: %s", level_str, msg);
     } else if (level & s->log_level) {
+#else
+    if (level & s->log_level) {
+#endif
         g_get_current_time(&time);
         fprintf(s->log_file,
                 "%lu.%lu: %s: %s\n", time.tv_sec, time.tv_usec, level_str, msg);
@@ -153,6 +163,7 @@ static void ga_log(const gchar *domain, GLogLevelFlags level,
     }
 }
 
+#ifndef _WIN32
 static void become_daemon(const char *pidfile)
 {
     pid_t pid, sid;
@@ -203,6 +214,7 @@ fail:
     g_critical("failed to daemonize");
     exit(EXIT_FAILURE);
 }
+#endif
 
 static int send_response(GAState *s, QObject *payload)
 {
@@ -466,10 +478,12 @@ int main(int argc, char **argv)
         }
     }
 
+#ifndef _WIN32
     if (daemonize) {
         g_debug("starting daemon");
         become_daemon(pidfile);
     }
+#endif
 
     s = g_malloc0(sizeof(GAState));
     s->log_file = log_file;
@@ -482,10 +496,12 @@ int main(int argc, char **argv)
     ga_command_state_init_all(s->command_state);
     json_message_parser_init(&s->parser, process_event);
     ga_state = s;
+#ifndef _WIN32
     if (!register_signal_handlers()) {
         g_critical("failed to register signal handlers");
         goto out_bad;
     }
+#endif
 
     s->main_loop = g_main_loop_new(NULL, false);
     if (!channel_init(ga_state, method, path)) {
diff --git a/qga/commands-win32.c b/qga/commands-win32.c
new file mode 100644
index 0000000..d96f1ad
--- /dev/null
+++ b/qga/commands-win32.c
@@ -0,0 +1,91 @@
+/*
+ * QEMU Guest Agent win32-specific command implementations
+ *
+ * Copyright IBM Corp. 2012
+ *
+ * Authors:
+ *  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 <glib.h>
+#include "qga/guest-agent-core.h"
+#include "qga-qmp-commands.h"
+#include "qerror.h"
+
+void qmp_guest_shutdown(bool has_mode, const char *mode, Error **err)
+{
+    error_set(err, QERR_UNSUPPORTED);
+}
+
+int64_t qmp_guest_file_open(const char *path, bool has_mode, const char *mode, Error **err)
+{
+    error_set(err, QERR_UNSUPPORTED);
+    return 0;
+}
+
+void qmp_guest_file_close(int64_t handle, Error **err)
+{
+    error_set(err, QERR_UNSUPPORTED);
+}
+
+GuestFileRead *qmp_guest_file_read(int64_t handle, bool has_count,
+                                   int64_t count, Error **err)
+{
+    error_set(err, QERR_UNSUPPORTED);
+    return 0;
+}
+
+GuestFileWrite *qmp_guest_file_write(int64_t handle, const char *buf_b64,
+                                     bool has_count, int64_t count, Error **err)
+{
+    error_set(err, QERR_UNSUPPORTED);
+    return 0;
+}
+
+GuestFileSeek *qmp_guest_file_seek(int64_t handle, int64_t offset,
+                                   int64_t whence, Error **err)
+{
+    error_set(err, QERR_UNSUPPORTED);
+    return 0;
+}
+
+void qmp_guest_file_flush(int64_t handle, Error **err)
+{
+    error_set(err, QERR_UNSUPPORTED);
+}
+
+/*
+ * Return status of freeze/thaw
+ */
+GuestFsfreezeStatus qmp_guest_fsfreeze_status(Error **err)
+{
+    error_set(err, QERR_UNSUPPORTED);
+    return 0;
+}
+
+/*
+ * Walk list of mounted file systems in the guest, and freeze the ones which
+ * are real local file systems.
+ */
+int64_t qmp_guest_fsfreeze_freeze(Error **err)
+{
+    error_set(err, QERR_UNSUPPORTED);
+    return 0;
+}
+
+/*
+ * Walk list of frozen file systems in the guest, and thaw them.
+ */
+int64_t qmp_guest_fsfreeze_thaw(Error **err)
+{
+    error_set(err, QERR_UNSUPPORTED);
+    return 0;
+}
+
+/* register init/cleanup routines for stateful command groups */
+void ga_command_state_init(GAState *s, GACommandState *cs)
+{
+}
-- 
1.7.4.1

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [Qemu-devel] [PATCH v2 6/8] qemu-ga: add initial win32 support
  2012-02-02 19:58 [Qemu-devel] [PATCH v2 0/8] qemu-ga: add support for Windows Michael Roth
                   ` (4 preceding siblings ...)
  2012-02-02 19:58 ` [Qemu-devel] [PATCH v2 5/8] qemu-ga: fixes for win32 build of qemu-ga Michael Roth
@ 2012-02-02 19:58 ` Michael Roth
  2012-02-02 19:58 ` [Qemu-devel] [PATCH v2 7/8] qemu-ga: add Windows service integration Michael Roth
                   ` (2 subsequent siblings)
  8 siblings, 0 replies; 21+ messages in thread
From: Michael Roth @ 2012-02-02 19:58 UTC (permalink / raw)
  To: qemu-devel; +Cc: ghammer, aliguori, mdroth, lcapitulino

This adds a win32 channel implementation that makes qemu-ga functional
on Windows using virtio-serial (unix-listen/isa-serial not currently
implemented). Unlike with the posix implementation, we do not use
GIOChannel for the following reasons:

 - glib calls stat() on an fd to check whether S_IFCHR is set, which is
   the case for virtio-serial on win32. Because of that, a one-time
   check to determine whether the channel is readable is done by making
   a call to PeekConsoleInput(), which reports the underlying handle is
   not a valid console handle, and thus we can never read from the
   channel.

 - if one goes as far as to "trick" glib into thinking it is a normal
   file descripter, the buffering is done in such a way that data
   written to the output stream will subsequently result in that same
   data being read back as if it were input, causing an error loop.
   furthermore, a forced flush of the channel only moves the data into a
   secondary buffer managed by glib, so there's no way to prevent output
   from getting read back as input.

The implementation here ties into the glib main loop by implementing a
custom GSource that continually submits asynchronous/overlapped I/O to
fill an GAChannel-managed read buffer, and tells glib to poll the
corresponding event handle for a completion whenever there is no
data/RPC in the read buffer to notify the main application about.

Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
 Makefile.objs       |    2 +-
 qemu-ga.c           |    4 +
 qga/channel-win32.c |  337 +++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 342 insertions(+), 1 deletions(-)
 create mode 100644 qga/channel-win32.c

diff --git a/Makefile.objs b/Makefile.objs
index 18e79ce..e1cb54a 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -426,7 +426,7 @@ common-obj-y += qmp.o hmp.o
 
 qga-nested-y = commands.o guest-agent-command-state.o
 qga-nested-$(CONFIG_POSIX) += commands-posix.o channel-posix.o
-qga-nested-$(CONFIG_WIN32) += commands-win32.o
+qga-nested-$(CONFIG_WIN32) += commands-win32.o channel-win32.o
 qga-obj-y = $(addprefix qga/, $(qga-nested-y))
 qga-obj-y += qemu-ga.o module.o
 qga-obj-$(CONFIG_WIN32) += oslib-win32.o
diff --git a/qemu-ga.c b/qemu-ga.c
index 93ebc3e..8e517b5 100644
--- a/qemu-ga.c
+++ b/qemu-ga.c
@@ -30,7 +30,11 @@
 #include "qapi/qmp-core.h"
 #include "qga/channel.h"
 
+#ifndef _WIN32
 #define QGA_VIRTIO_PATH_DEFAULT "/dev/virtio-ports/org.qemu.guest_agent.0"
+#else
+#define QGA_VIRTIO_PATH_DEFAULT "\\\\.\\Global\\org.qemu.guest_agent.0"
+#endif
 #define QGA_PIDFILE_DEFAULT "/var/run/qemu-ga.pid"
 
 struct GAState {
diff --git a/qga/channel-win32.c b/qga/channel-win32.c
new file mode 100644
index 0000000..9d8601a
--- /dev/null
+++ b/qga/channel-win32.c
@@ -0,0 +1,337 @@
+#include <stdlib.h>
+#include <stdio.h>
+#include <stdbool.h>
+#include <glib.h>
+#include <windows.h>
+#include <errno.h>
+#include <io.h>
+#include "qga/guest-agent-core.h"
+#include "qga/channel.h"
+
+typedef struct GAChannelReadState {
+    guint thread_id;
+    uint8_t *buf;
+    size_t buf_size;
+    size_t cur; /* current buffer start */
+    size_t pending; /* pending buffered bytes to read */
+    OVERLAPPED ov;
+    bool ov_pending; /* whether on async read is outstanding */
+} GAChannelReadState;
+
+struct GAChannel {
+    HANDLE handle;
+    GAChannelCallback cb;
+    gpointer user_data;
+    GAChannelReadState rstate;
+    GIOCondition pending_events; /* TODO: use GAWatch.pollfd.revents */
+    GSource *source;
+};
+
+typedef struct GAWatch {
+    GSource source;
+    GPollFD pollfd;
+    GAChannel *channel;
+    GIOCondition events_mask;
+} GAWatch;
+
+/*
+ * Called by glib prior to polling to set up poll events if polling is needed.
+ *
+ */
+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, count_to_read = 0;
+    bool success;
+    GIOCondition new_events = 0;
+
+    g_debug("prepare");
+    /* go ahead and submit another read if there's room in the buffer
+     * and no previous reads are outstanding
+     */
+    if (!rs->ov_pending) {
+        if (rs->cur + rs->pending >= rs->buf_size) {
+            if (rs->cur) {
+                memmove(rs->buf, rs->buf + rs->cur, rs->pending);
+                rs->cur = 0;
+            }
+        }
+        count_to_read = rs->buf_size - rs->cur - rs->pending;
+    }
+
+    if (rs->ov_pending || count_to_read <= 0) {
+            goto out;
+    }
+
+    /* submit the read */
+    success = ReadFile(c->handle, rs->buf + rs->cur + rs->pending,
+                       count_to_read, &count_read, &rs->ov);
+    if (success) {
+        rs->pending += count_read;
+        rs->ov_pending = false;
+    } else {
+        if (GetLastError() == ERROR_IO_PENDING) {
+            rs->ov_pending = true;
+        } else {
+            new_events |= G_IO_ERR;
+        }
+    }
+
+out:
+    /* dont block forever, iterate the main loop every once and a while */
+    *timeout_ms = 500;
+    /* if there's data in the read buffer, or another event is pending,
+     * skip polling and issue user cb.
+     */
+    if (rs->pending) {
+        new_events |= G_IO_IN;
+    }
+    c->pending_events |= new_events;
+    return !!c->pending_events;
+}
+
+/*
+ * Called by glib after an outstanding read request is completed.
+ */
+static gboolean ga_channel_check(GSource *source)
+{
+    GAWatch *watch = (GAWatch *)source;
+    GAChannel *c = (GAChannel *)watch->channel;
+    GAChannelReadState *rs = &c->rstate;
+    DWORD count_read, error;
+    BOOL success;
+
+    GIOCondition new_events = 0;
+
+    g_debug("check");
+
+    /* failing this implies we issued a read that completed immediately,
+     * yet no data was placed into the buffer (and thus we did not skip
+     * polling). but since EOF is not obtainable until we retrieve an
+     * overlapped result, it must be the case that there was data placed
+     * into the buffer, or an error was generated by Readfile(). in either
+     * case, we should've skipped the polling for this round.
+     */
+    g_assert(rs->ov_pending);
+
+    success = GetOverlappedResult(c->handle, &rs->ov, &count_read, FALSE);
+    if (success) {
+        g_debug("thread: overlapped result, count_read: %d", (int)count_read);
+        rs->pending += count_read;
+        new_events |= G_IO_IN;
+    } else {
+        error = GetLastError();
+        if (error == 0 || error == ERROR_HANDLE_EOF ||
+            error == ERROR_NO_SYSTEM_RESOURCES) {
+            /* note: On WinXP SP3 with virtio-win 0.1-15 vioser drivers,
+             * ENSR seems to be synonymous with when we'd normally expect
+             * ERROR_HANDLE_EOF. So treat it as such. Microsoft's
+             * recommendation for ERROR_NO_SYSTEM_RESOURCES is to
+             * retry the read, so this happens to work out anyway.
+             */
+            new_events |= G_IO_HUP;
+        } else if (error != ERROR_IO_INCOMPLETE) {
+            g_critical("error retrieving overlapped result: %d", (int)error);
+            new_events |= G_IO_ERR;
+        }
+    }
+
+    if (new_events) {
+        rs->ov_pending = 0;
+    }
+    c->pending_events |= new_events;
+
+    return !!c->pending_events;
+}
+
+/*
+ * Called by glib after either prepare or check routines signal readiness
+ */
+static gboolean ga_channel_dispatch(GSource *source, GSourceFunc unused,
+                                    gpointer user_data)
+{
+    GAWatch *watch = (GAWatch *)source;
+    GAChannel *c = (GAChannel *)watch->channel;
+    GAChannelReadState *rs = &c->rstate;
+    gboolean success;
+
+    g_debug("dispatch");
+    success = c->cb(watch->pollfd.revents, c->user_data);
+
+    if (c->pending_events & G_IO_ERR) {
+        g_critical("channel error, removing source");
+        return false;
+    }
+
+    /* TODO: replace rs->pending with watch->revents */
+    c->pending_events &= ~G_IO_HUP;
+    if (!rs->pending) {
+        c->pending_events &= ~G_IO_IN;
+    } else {
+        c->pending_events = 0;
+    }
+    return success;
+}
+
+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 = (gintptr) c->rstate.ov.hEvent;
+    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;
+    GIOStatus status;
+    size_t to_read = 0;
+
+    if (c->pending_events & G_IO_ERR) {
+        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;
+    }
+
+    return status;
+}
+
+static GIOStatus ga_channel_write(GAChannel *c, const char *buf, size_t size,
+                                  size_t *count)
+{
+    GIOStatus status;
+    OVERLAPPED ov = {0};
+    BOOL ret;
+    DWORD written;
+
+    ov.hEvent = CreateEvent(NULL, FALSE, FALSE, NULL);
+    ret = WriteFile(c->handle, buf, size, &written, &ov);
+    if (!ret) {
+        if (GetLastError() == ERROR_IO_PENDING) {
+            /* write is pending */
+            ret = GetOverlappedResult(c->handle, &ov, &written, TRUE);
+            if (!ret) {
+                if (!GetLastError()) {
+                    status = G_IO_STATUS_AGAIN;
+                } else {
+                    status = G_IO_STATUS_ERROR;
+                }
+            } else {
+                /* write is complete */
+                status = G_IO_STATUS_NORMAL;
+                *count = written;
+            }
+        } else {
+            status = G_IO_STATUS_ERROR;
+        }
+    } else {
+        /* write returned immediately */
+        status = G_IO_STATUS_NORMAL;
+        *count = written;
+    }
+
+    return status;
+}
+
+GIOStatus ga_channel_write_all(GAChannel *c, const char *buf, size_t size)
+{
+    GIOStatus status = G_IO_STATUS_NORMAL;;
+    size_t count;
+
+    while (size) {
+        status = ga_channel_write(c, buf, size, &count);
+        if (status == G_IO_STATUS_NORMAL) {
+            size -= count;
+            buf += count;
+        } else if (status != G_IO_STATUS_AGAIN) {
+            break;
+        }
+    }
+
+    return status;
+}
+
+static gboolean ga_channel_open(GAChannel *c, GAChannelMethod method,
+                                const gchar *path)
+{
+    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;
+}
+
+GAChannel *ga_channel_new(GAChannelMethod method, const gchar *path,
+                          GAChannelCallback cb, gpointer opaque)
+{
+    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;
+    }
+
+    c->cb = cb;
+    c->user_data = opaque;
+
+    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(c);
+    g_source_attach(c->source, NULL);
+    return c;
+}
+
+void ga_channel_free(GAChannel *c)
+{
+    if (c->source) {
+        g_source_destroy(c->source);
+    }
+    if (c->rstate.ov.hEvent) {
+        CloseHandle(c->rstate.ov.hEvent);
+    }
+    g_free(c->rstate.buf);
+    g_free(c);
+}
-- 
1.7.4.1

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [Qemu-devel] [PATCH v2 7/8] qemu-ga: add Windows service integration
  2012-02-02 19:58 [Qemu-devel] [PATCH v2 0/8] qemu-ga: add support for Windows Michael Roth
                   ` (5 preceding siblings ...)
  2012-02-02 19:58 ` [Qemu-devel] [PATCH v2 6/8] qemu-ga: add initial win32 support Michael Roth
@ 2012-02-02 19:58 ` Michael Roth
  2012-02-02 19:59 ` [Qemu-devel] [PATCH v2 8/8] qemu-ga: add win32 guest-shutdown command Michael Roth
  2012-02-03 14:18 ` [Qemu-devel] [PATCH v2 0/8] qemu-ga: add support for Windows Luiz Capitulino
  8 siblings, 0 replies; 21+ messages in thread
From: Michael Roth @ 2012-02-02 19:58 UTC (permalink / raw)
  To: qemu-devel; +Cc: ghammer, aliguori, mdroth, lcapitulino

This allows qemu-ga to function as a Windows service:

 - to install the service (will auto-start on boot):
     qemu-ga --service install
 - to start the service:
     net start qemu-ga
 - to stop the service:
     net stop qemu-ga
 - to uninstall service:
     qemu-ga --service uninstall

Original patch by Gal Hammer <ghammer@redhat.com>

Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
 Makefile.objs       |    2 +-
 qemu-ga.c           |  103 ++++++++++++++++++++++++++++++++++++++++++++--
 qga/service-win32.c |  114 +++++++++++++++++++++++++++++++++++++++++++++++++++
 qga/service-win32.h |   30 +++++++++++++
 4 files changed, 244 insertions(+), 5 deletions(-)
 create mode 100644 qga/service-win32.c
 create mode 100644 qga/service-win32.h

diff --git a/Makefile.objs b/Makefile.objs
index e1cb54a..3b08e70 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -426,7 +426,7 @@ common-obj-y += qmp.o hmp.o
 
 qga-nested-y = commands.o guest-agent-command-state.o
 qga-nested-$(CONFIG_POSIX) += commands-posix.o channel-posix.o
-qga-nested-$(CONFIG_WIN32) += commands-win32.o channel-win32.o
+qga-nested-$(CONFIG_WIN32) += commands-win32.o channel-win32.o service-win32.o
 qga-obj-y = $(addprefix qga/, $(qga-nested-y))
 qga-obj-y += qemu-ga.o module.o
 qga-obj-$(CONFIG_WIN32) += oslib-win32.o
diff --git a/qemu-ga.c b/qemu-ga.c
index 8e517b5..92f81ed 100644
--- a/qemu-ga.c
+++ b/qemu-ga.c
@@ -29,6 +29,10 @@
 #include "error_int.h"
 #include "qapi/qmp-core.h"
 #include "qga/channel.h"
+#ifdef _WIN32
+#include "qga/service-win32.h"
+#include <windows.h>
+#endif
 
 #ifndef _WIN32
 #define QGA_VIRTIO_PATH_DEFAULT "/dev/virtio-ports/org.qemu.guest_agent.0"
@@ -46,11 +50,19 @@ struct GAState {
     GLogLevelFlags log_level;
     FILE *log_file;
     bool logging_enabled;
+#ifdef _WIN32
+    GAService service;
+#endif
 };
 
 static struct GAState *ga_state;
 
-#ifndef _WIN32
+#ifdef _WIN32
+DWORD WINAPI service_ctrl_handler(DWORD ctrl, DWORD type, LPVOID data,
+                                  LPVOID ctx);
+VOID WINAPI service_main(DWORD argc, TCHAR *argv[]);
+#endif
+
 static void quit_handler(int sig)
 {
     g_debug("received signal num %d, quitting", sig);
@@ -60,6 +72,7 @@ static void quit_handler(int sig)
     }
 }
 
+#ifndef _WIN32
 static gboolean register_signal_handlers(void)
 {
     struct sigaction sigact;
@@ -95,8 +108,9 @@ static void usage(const char *cmd)
 "  -f, --pidfile     specify pidfile (default is %s)\n"
 "  -v, --verbose     log extra debugging information\n"
 "  -V, --version     print version information and exit\n"
-#ifndef _WIN32
 "  -d, --daemonize   become a daemon\n"
+#ifdef _WIN32
+"  -s, --service     service commands: install, uninstall\n"
 #endif
 "  -b, --blacklist   comma-separated list of RPCs to disable (no spaces, \"?\""
 "                    to list available RPCs)\n"
@@ -394,10 +408,64 @@ static gboolean channel_init(GAState *s, const gchar *method, const gchar *path)
     return true;
 }
 
+#ifdef _WIN32
+DWORD WINAPI service_ctrl_handler(DWORD ctrl, DWORD type, LPVOID data,
+                                  LPVOID ctx)
+{
+    DWORD ret = NO_ERROR;
+    GAService *service = &ga_state->service;
+
+    switch (ctrl)
+    {
+        case SERVICE_CONTROL_STOP:
+        case SERVICE_CONTROL_SHUTDOWN:
+            quit_handler(SIGTERM);
+            service->status.dwCurrentState = SERVICE_STOP_PENDING;
+            SetServiceStatus(service->status_handle, &service->status);
+            break;
+
+        default:
+            ret = ERROR_CALL_NOT_IMPLEMENTED;
+    }
+    return ret;
+}
+
+VOID WINAPI service_main(DWORD argc, TCHAR *argv[])
+{
+    GAService *service = &ga_state->service;
+
+    service->status_handle = RegisterServiceCtrlHandlerEx(QGA_SERVICE_NAME,
+        service_ctrl_handler, NULL);
+
+    if (service->status_handle == 0) {
+        g_critical("Failed to register extended requests function!\n");
+        return;
+    }
+
+    service->status.dwServiceType = SERVICE_WIN32;
+    service->status.dwCurrentState = SERVICE_RUNNING;
+    service->status.dwControlsAccepted = SERVICE_ACCEPT_STOP | SERVICE_ACCEPT_SHUTDOWN;
+    service->status.dwWin32ExitCode = NO_ERROR;
+    service->status.dwServiceSpecificExitCode = NO_ERROR;
+    service->status.dwCheckPoint = 0;
+    service->status.dwWaitHint = 0;
+    SetServiceStatus(service->status_handle, &service->status);
+
+    g_main_loop_run(ga_state->main_loop);
+
+    service->status.dwCurrentState = SERVICE_STOPPED;
+    SetServiceStatus(service->status_handle, &service->status);
+}
+#endif
+
 int main(int argc, char **argv)
 {
-    const char *sopt = "hVvdm:p:l:f:b:";
+    const char *sopt = "hVvdm:p:l:f:b:s:";
     const char *method = NULL, *path = NULL, *pidfile = QGA_PIDFILE_DEFAULT;
+    const char *log_file_name = NULL;
+#ifdef _WIN32
+    const char *service = NULL;
+#endif
     const struct option lopt[] = {
         { "help", 0, NULL, 'h' },
         { "version", 0, NULL, 'V' },
@@ -408,6 +476,9 @@ int main(int argc, char **argv)
         { "path", 0, NULL, 'p' },
         { "daemonize", 0, NULL, 'd' },
         { "blacklist", 0, NULL, 'b' },
+#ifdef _WIN32
+        { "service", 0, NULL, 's' },
+#endif        
         { NULL, 0, NULL, 0 }
     };
     int opt_ind = 0, ch, daemonize = 0, i, j, len;
@@ -426,7 +497,8 @@ int main(int argc, char **argv)
             path = optarg;
             break;
         case 'l':
-            log_file = fopen(optarg, "a");
+            log_file_name = optarg;
+            log_file = fopen(log_file_name, "a");
             if (!log_file) {
                 g_critical("unable to open specified log file: %s",
                            strerror(errno));
@@ -472,6 +544,19 @@ int main(int argc, char **argv)
             }
             break;
         }
+#ifdef _WIN32
+        case 's':
+            service = optarg;
+            if (strcmp(service, "install") == 0) {
+                return ga_install_service(path, log_file_name);
+            } else if (strcmp(service, "uninstall") == 0) {
+                return ga_uninstall_service();
+            } else {
+                printf("Unknown service command.\n");
+                return EXIT_FAILURE;
+            }
+            break;
+#endif
         case 'h':
             usage(argv[0]);
             return 0;
@@ -512,7 +597,17 @@ int main(int argc, char **argv)
         g_critical("failed to initialize guest agent channel");
         goto out_bad;
     }
+#ifndef _WIN32
     g_main_loop_run(ga_state->main_loop);
+#else
+    if (daemonize) {
+        SERVICE_TABLE_ENTRY service_table[] = {
+            { (char *)QGA_SERVICE_NAME, service_main }, { NULL, NULL } };
+        StartServiceCtrlDispatcher(service_table);
+    } else {
+        g_main_loop_run(ga_state->main_loop);
+    }
+#endif
 
     ga_command_state_cleanup_all(ga_state->command_state);
     ga_channel_free(ga_state->channel);
diff --git a/qga/service-win32.c b/qga/service-win32.c
new file mode 100644
index 0000000..0905456
--- /dev/null
+++ b/qga/service-win32.c
@@ -0,0 +1,114 @@
+/*
+ * QEMU Guest Agent helpers for win32 service management
+ *
+ * Copyright IBM Corp. 2012
+ *
+ * Authors:
+ *  Gal Hammer        <ghammer@redhat.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 <glib.h>
+#include <windows.h>
+#include "qga/service-win32.h"
+
+static int printf_win_error(const char *text)
+{
+    DWORD err = GetLastError();
+    char *message;
+    int n;
+
+    FormatMessage(FORMAT_MESSAGE_ALLOCATE_BUFFER |
+        FORMAT_MESSAGE_FROM_SYSTEM | FORMAT_MESSAGE_IGNORE_INSERTS,
+        NULL,
+        err,
+        MAKELANGID(LANG_NEUTRAL, SUBLANG_DEFAULT),
+        (char *)&message, 0,
+        NULL);
+    n = printf("%s. (Error: %d) %s", text, err, message);
+    LocalFree(message);
+
+    return n;
+}
+
+int ga_install_service(const char *path, const char *logfile)
+{
+    SC_HANDLE manager;
+    SC_HANDLE service;
+    TCHAR cmdline[MAX_PATH];
+
+    if (GetModuleFileName(NULL, cmdline, MAX_PATH) == 0) {
+        printf_win_error("No full path to service's executable");
+        return EXIT_FAILURE;
+    }
+
+    _snprintf(cmdline, MAX_PATH - strlen(cmdline), "%s -d", cmdline);
+
+    if (path) {
+        _snprintf(cmdline, MAX_PATH - strlen(cmdline), "%s -p %s", cmdline, path);
+    }
+    if (logfile) {
+        _snprintf(cmdline, MAX_PATH - strlen(cmdline), "%s -l %s -v",
+            cmdline, logfile);
+    }
+
+    g_debug("service's cmdline: %s", cmdline);
+
+    manager = OpenSCManager(NULL, NULL, SC_MANAGER_ALL_ACCESS);
+    if (manager == NULL) {
+        printf_win_error("No handle to service control manager");
+        return EXIT_FAILURE;
+    }
+
+    service = CreateService(manager, QGA_SERVICE_NAME, QGA_SERVICE_DISPLAY_NAME,
+        SERVICE_ALL_ACCESS, SERVICE_WIN32_OWN_PROCESS, SERVICE_AUTO_START,
+        SERVICE_ERROR_NORMAL, cmdline, NULL, NULL, NULL, NULL, NULL);
+
+    if (service) {
+        SERVICE_DESCRIPTION desc = { (char *)QGA_SERVICE_DESCRIPTION };
+        ChangeServiceConfig2(service, SERVICE_CONFIG_DESCRIPTION, &desc);
+
+        printf("Service was installed successfully.\n");
+    } else {
+        printf_win_error("Failed to install service");
+    }
+
+    CloseServiceHandle(service);
+    CloseServiceHandle(manager);
+
+    return (service == NULL);
+}
+
+int ga_uninstall_service(void)
+{
+    SC_HANDLE manager;
+    SC_HANDLE service;
+
+    manager = OpenSCManager(NULL, NULL, SC_MANAGER_ALL_ACCESS);
+    if (manager == NULL) {
+        printf_win_error("No handle to service control manager");
+        return EXIT_FAILURE;
+    }
+
+    service = OpenService(manager, QGA_SERVICE_NAME, DELETE);
+    if (service == NULL) {
+        printf_win_error("No handle to service");
+        CloseServiceHandle(manager);
+        return EXIT_FAILURE;
+    }
+
+    if (DeleteService(service) == FALSE) {
+        printf_win_error("Failed to delete service");
+    } else {
+        printf("Service was deleted successfully.\n");
+    }
+
+    CloseServiceHandle(service);
+    CloseServiceHandle(manager);
+
+    return EXIT_SUCCESS;
+}
diff --git a/qga/service-win32.h b/qga/service-win32.h
new file mode 100644
index 0000000..99dfc53
--- /dev/null
+++ b/qga/service-win32.h
@@ -0,0 +1,30 @@
+/*
+ * QEMU Guest Agent helpers for win32 service management
+ *
+ * Copyright IBM Corp. 2012
+ *
+ * Authors:
+ *  Gal Hammer        <ghammer@redhat.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.
+ */
+#ifndef QGA_SERVICE_H
+#define QGA_SERVICE_H
+
+#include <windows.h>
+
+#define QGA_SERVICE_DISPLAY_NAME "QEMU Guest Agent"
+#define QGA_SERVICE_NAME         "qemu-ga"
+#define QGA_SERVICE_DESCRIPTION  "Enables integration with QEMU machine emulator and virtualizer."
+
+typedef struct GAService {
+    SERVICE_STATUS status;
+    SERVICE_STATUS_HANDLE status_handle;
+} GAService;
+
+int ga_install_service(const char *path, const char *logfile);
+int ga_uninstall_service(void);
+
+#endif
-- 
1.7.4.1

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [Qemu-devel] [PATCH v2 8/8] qemu-ga: add win32 guest-shutdown command
  2012-02-02 19:58 [Qemu-devel] [PATCH v2 0/8] qemu-ga: add support for Windows Michael Roth
                   ` (6 preceding siblings ...)
  2012-02-02 19:58 ` [Qemu-devel] [PATCH v2 7/8] qemu-ga: add Windows service integration Michael Roth
@ 2012-02-02 19:59 ` Michael Roth
  2012-02-03 14:18 ` [Qemu-devel] [PATCH v2 0/8] qemu-ga: add support for Windows Luiz Capitulino
  8 siblings, 0 replies; 21+ messages in thread
From: Michael Roth @ 2012-02-02 19:59 UTC (permalink / raw)
  To: qemu-devel; +Cc: ghammer, aliguori, mdroth, lcapitulino

Implement guest-shutdown RPC for Windows. Functionally this should be
equivalent to the posix implementation.

Original patch by Gal Hammer <ghammer@redhat.com>

Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
 qga/commands-win32.c |   41 ++++++++++++++++++++++++++++++++++++++++-
 1 files changed, 40 insertions(+), 1 deletions(-)

diff --git a/qga/commands-win32.c b/qga/commands-win32.c
index d96f1ad..4aa0f0d 100644
--- a/qga/commands-win32.c
+++ b/qga/commands-win32.c
@@ -15,9 +15,48 @@
 #include "qga-qmp-commands.h"
 #include "qerror.h"
 
+#ifndef SHTDN_REASON_FLAG_PLANNED
+#define SHTDN_REASON_FLAG_PLANNED 0x80000000
+#endif
+
 void qmp_guest_shutdown(bool has_mode, const char *mode, Error **err)
 {
-    error_set(err, QERR_UNSUPPORTED);
+    HANDLE token;
+    TOKEN_PRIVILEGES priv;
+    UINT shutdown_flag = EWX_FORCE;
+
+    slog("guest-shutdown called, mode: %s", mode);
+
+    if (!has_mode || strcmp(mode, "powerdown") == 0) {
+        shutdown_flag |= EWX_POWEROFF;
+    } else if (strcmp(mode, "halt") == 0) {
+        shutdown_flag |= EWX_SHUTDOWN;
+    } else if (strcmp(mode, "reboot") == 0) {
+        shutdown_flag |= EWX_REBOOT;
+    } else {
+        error_set(err, QERR_INVALID_PARAMETER_VALUE, "mode",
+                  "halt|powerdown|reboot");
+        return;
+    }
+
+    /* Request a shutdown privilege, but try to shut down the system
+       anyway. */
+    if (OpenProcessToken(GetCurrentProcess(),
+        TOKEN_ADJUST_PRIVILEGES|TOKEN_QUERY, &token))
+    {
+        LookupPrivilegeValue(NULL, SE_SHUTDOWN_NAME,
+            &priv.Privileges[0].Luid);
+
+        priv.PrivilegeCount = 1;
+        priv.Privileges[0].Attributes = SE_PRIVILEGE_ENABLED;
+
+        AdjustTokenPrivileges(token, FALSE, &priv, 0, NULL, 0);
+    }
+
+    if (!ExitWindowsEx(shutdown_flag, SHTDN_REASON_FLAG_PLANNED)) {
+        slog("guest-shutdown failed: %d", GetLastError());
+        error_set(err, QERR_UNDEFINED_ERROR);
+    }
 }
 
 int64_t qmp_guest_file_open(const char *path, bool has_mode, const char *mode, Error **err)
-- 
1.7.4.1

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* Re: [Qemu-devel] [PATCH v2 0/8] qemu-ga: add support for Windows
  2012-02-02 19:58 [Qemu-devel] [PATCH v2 0/8] qemu-ga: add support for Windows Michael Roth
                   ` (7 preceding siblings ...)
  2012-02-02 19:59 ` [Qemu-devel] [PATCH v2 8/8] qemu-ga: add win32 guest-shutdown command Michael Roth
@ 2012-02-03 14:18 ` Luiz Capitulino
  2012-02-03 16:37   ` Michael Roth
  8 siblings, 1 reply; 21+ messages in thread
From: Luiz Capitulino @ 2012-02-03 14:18 UTC (permalink / raw)
  To: Michael Roth; +Cc: ghammer, aliguori, qemu-devel

On Thu,  2 Feb 2012 13:58:52 -0600
Michael Roth <mdroth@linux.vnet.ibm.com> wrote:

> These patches apply on top of qemu.git master, and can also be obtained from:
> git://github.com/mdroth/qemu.git qga-win32-v2
> 
> Luiz/Gal, I decided not to roll the suspend/hibernate stuff into this series
> since the s3 situation isn't fully sorted out yet. The file structure is a
> little different now, posix/linux-specific stuff goes in qga/commands-posix.c,
> win32-specific stuff in qga/commands-win32.c, but other than that it should be
> a straightforward rebase if this gets merged first.

I think I'll have to rebase my series on top of this one, when do you plan to
merge this?

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [Qemu-devel] [PATCH v2 0/8] qemu-ga: add support for Windows
  2012-02-03 14:18 ` [Qemu-devel] [PATCH v2 0/8] qemu-ga: add support for Windows Luiz Capitulino
@ 2012-02-03 16:37   ` Michael Roth
  2012-02-03 16:45     ` Luiz Capitulino
  0 siblings, 1 reply; 21+ messages in thread
From: Michael Roth @ 2012-02-03 16:37 UTC (permalink / raw)
  To: Luiz Capitulino; +Cc: ghammer, aliguori, qemu-devel

On 02/03/2012 08:18 AM, Luiz Capitulino wrote:
> On Thu,  2 Feb 2012 13:58:52 -0600
> Michael Roth<mdroth@linux.vnet.ibm.com>  wrote:
>
>> These patches apply on top of qemu.git master, and can also be obtained from:
>> git://github.com/mdroth/qemu.git qga-win32-v2
>>
>> Luiz/Gal, I decided not to roll the suspend/hibernate stuff into this series
>> since the s3 situation isn't fully sorted out yet. The file structure is a
>> little different now, posix/linux-specific stuff goes in qga/commands-posix.c,
>> win32-specific stuff in qga/commands-win32.c, but other than that it should be
>> a straightforward rebase if this gets merged first.
>
> I think I'll have to rebase my series on top of this one, when do you plan to
> merge this?
>

Hopefully soon, was planning on waiting for the suspend/hibernate bits 
but we seem to be blocked on the s3 issues and I have other patches 
accumulating on top of win32 (hesitant to base those on master since 
this patchset does a lot of refactoring that might affect them), so I 
figured I'd push this for merge since it doesn't have any dependencies 
outside master.

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [Qemu-devel] [PATCH v2 0/8] qemu-ga: add support for Windows
  2012-02-03 16:37   ` Michael Roth
@ 2012-02-03 16:45     ` Luiz Capitulino
  2012-02-03 17:23       ` Michael Roth
  0 siblings, 1 reply; 21+ messages in thread
From: Luiz Capitulino @ 2012-02-03 16:45 UTC (permalink / raw)
  To: Michael Roth; +Cc: ghammer, aliguori, qemu-devel

On Fri, 03 Feb 2012 10:37:25 -0600
Michael Roth <mdroth@linux.vnet.ibm.com> wrote:

> On 02/03/2012 08:18 AM, Luiz Capitulino wrote:
> > On Thu,  2 Feb 2012 13:58:52 -0600
> > Michael Roth<mdroth@linux.vnet.ibm.com>  wrote:
> >
> >> These patches apply on top of qemu.git master, and can also be obtained from:
> >> git://github.com/mdroth/qemu.git qga-win32-v2
> >>
> >> Luiz/Gal, I decided not to roll the suspend/hibernate stuff into this series
> >> since the s3 situation isn't fully sorted out yet. The file structure is a
> >> little different now, posix/linux-specific stuff goes in qga/commands-posix.c,
> >> win32-specific stuff in qga/commands-win32.c, but other than that it should be
> >> a straightforward rebase if this gets merged first.
> >
> > I think I'll have to rebase my series on top of this one, when do you plan to
> > merge this?
> >
> 
> Hopefully soon, was planning on waiting for the suspend/hibernate bits 
> but we seem to be blocked on the s3 issues and I have other patches 
> accumulating on top of win32 (hesitant to base those on master since 
> this patchset does a lot of refactoring that might affect them), so I 
> figured I'd push this for merge since it doesn't have any dependencies 
> outside master.

The S3 issues seem sorted to me, but I don't oppose having this series in first.

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [Qemu-devel] [PATCH v2 0/8] qemu-ga: add support for Windows
  2012-02-03 16:45     ` Luiz Capitulino
@ 2012-02-03 17:23       ` Michael Roth
  2012-02-03 19:16         ` Luiz Capitulino
  0 siblings, 1 reply; 21+ messages in thread
From: Michael Roth @ 2012-02-03 17:23 UTC (permalink / raw)
  To: Luiz Capitulino; +Cc: ghammer, aliguori, qemu-devel

On 02/03/2012 10:45 AM, Luiz Capitulino wrote:
> On Fri, 03 Feb 2012 10:37:25 -0600
> Michael Roth<mdroth@linux.vnet.ibm.com>  wrote:
>
>> On 02/03/2012 08:18 AM, Luiz Capitulino wrote:
>>> On Thu,  2 Feb 2012 13:58:52 -0600
>>> Michael Roth<mdroth@linux.vnet.ibm.com>   wrote:
>>>
>>>> These patches apply on top of qemu.git master, and can also be obtained from:
>>>> git://github.com/mdroth/qemu.git qga-win32-v2
>>>>
>>>> Luiz/Gal, I decided not to roll the suspend/hibernate stuff into this series
>>>> since the s3 situation isn't fully sorted out yet. The file structure is a
>>>> little different now, posix/linux-specific stuff goes in qga/commands-posix.c,
>>>> win32-specific stuff in qga/commands-win32.c, but other than that it should be
>>>> a straightforward rebase if this gets merged first.
>>>
>>> I think I'll have to rebase my series on top of this one, when do you plan to
>>> merge this?
>>>
>>
>> Hopefully soon, was planning on waiting for the suspend/hibernate bits
>> but we seem to be blocked on the s3 issues and I have other patches
>> accumulating on top of win32 (hesitant to base those on master since
>> this patchset does a lot of refactoring that might affect them), so I
>> figured I'd push this for merge since it doesn't have any dependencies
>> outside master.
>
> The S3 issues seem sorted to me, but I don't oppose having this series in first.
>

Thanks, in retrospect I probably should've just gotten these out of the 
way weeks ago since they'd immediately clobber git blame.

I'd been tracking Gerd's QMP wakeup series as the s3 resolution we need 
for guest-suspend, is that still the case? I guess those are coming 
through your QMP queue?

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [Qemu-devel] [PATCH v2 0/8] qemu-ga: add support for Windows
  2012-02-03 17:23       ` Michael Roth
@ 2012-02-03 19:16         ` Luiz Capitulino
  2012-02-04 15:34           ` Kevin O'Connor
  0 siblings, 1 reply; 21+ messages in thread
From: Luiz Capitulino @ 2012-02-03 19:16 UTC (permalink / raw)
  To: Michael Roth; +Cc: ghammer, aliguori, qemu-devel

On Fri, 03 Feb 2012 11:23:05 -0600
Michael Roth <mdroth@linux.vnet.ibm.com> wrote:

> On 02/03/2012 10:45 AM, Luiz Capitulino wrote:
> > On Fri, 03 Feb 2012 10:37:25 -0600
> > Michael Roth<mdroth@linux.vnet.ibm.com>  wrote:
> >
> >> On 02/03/2012 08:18 AM, Luiz Capitulino wrote:
> >>> On Thu,  2 Feb 2012 13:58:52 -0600
> >>> Michael Roth<mdroth@linux.vnet.ibm.com>   wrote:
> >>>
> >>>> These patches apply on top of qemu.git master, and can also be obtained from:
> >>>> git://github.com/mdroth/qemu.git qga-win32-v2
> >>>>
> >>>> Luiz/Gal, I decided not to roll the suspend/hibernate stuff into this series
> >>>> since the s3 situation isn't fully sorted out yet. The file structure is a
> >>>> little different now, posix/linux-specific stuff goes in qga/commands-posix.c,
> >>>> win32-specific stuff in qga/commands-win32.c, but other than that it should be
> >>>> a straightforward rebase if this gets merged first.
> >>>
> >>> I think I'll have to rebase my series on top of this one, when do you plan to
> >>> merge this?
> >>>
> >>
> >> Hopefully soon, was planning on waiting for the suspend/hibernate bits
> >> but we seem to be blocked on the s3 issues and I have other patches
> >> accumulating on top of win32 (hesitant to base those on master since
> >> this patchset does a lot of refactoring that might affect them), so I
> >> figured I'd push this for merge since it doesn't have any dependencies
> >> outside master.
> >
> > The S3 issues seem sorted to me, but I don't oppose having this series in first.
> >
> 
> Thanks, in retrospect I probably should've just gotten these out of the 
> way weeks ago since they'd immediately clobber git blame.
> 
> I'd been tracking Gerd's QMP wakeup series as the s3 resolution we need 
> for guest-suspend, is that still the case?

Yes. But now I remembered about a seabios bug with S3... Need to check if
it were already addressed.

> I guess those are coming 
> through your QMP queue?

Oh, as the QMP part is trivial I thought someone else would pick them up,
but I can do that.

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [Qemu-devel] [PATCH v2 0/8] qemu-ga: add support for Windows
  2012-02-03 19:16         ` Luiz Capitulino
@ 2012-02-04 15:34           ` Kevin O'Connor
  2012-02-06 15:43             ` Luiz Capitulino
  0 siblings, 1 reply; 21+ messages in thread
From: Kevin O'Connor @ 2012-02-04 15:34 UTC (permalink / raw)
  To: Luiz Capitulino; +Cc: ghammer, aliguori, Michael Roth, qemu-devel

On Fri, Feb 03, 2012 at 05:16:27PM -0200, Luiz Capitulino wrote:
> On Fri, 03 Feb 2012 11:23:05 -0600
> Michael Roth <mdroth@linux.vnet.ibm.com> wrote:
> > I'd been tracking Gerd's QMP wakeup series as the s3 resolution we need 
> > for guest-suspend, is that still the case?
> 
> Yes. But now I remembered about a seabios bug with S3... Need to check if
> it were already addressed.

Hi Luiz,

I'm not aware of any recent S3 defects in SeaBIOS.  If there is a
defect, please let me know.

(I am aware of recent discussions on SeaBIOS and it running the
vgabios on s3-resume, but I would not classify that issue as a
defect.)

-Kevin

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [Qemu-devel] [PATCH v2 0/8] qemu-ga: add support for Windows
  2012-02-04 15:34           ` Kevin O'Connor
@ 2012-02-06 15:43             ` Luiz Capitulino
  2012-02-07  0:09               ` Kevin O'Connor
  0 siblings, 1 reply; 21+ messages in thread
From: Luiz Capitulino @ 2012-02-06 15:43 UTC (permalink / raw)
  To: Kevin O'Connor
  Cc: ghammer, aliguori, Michael Roth, Gleb Natapov, qemu-devel

On Sat, 4 Feb 2012 10:34:32 -0500
"Kevin O'Connor" <kevin@koconnor.net> wrote:

> On Fri, Feb 03, 2012 at 05:16:27PM -0200, Luiz Capitulino wrote:
> > On Fri, 03 Feb 2012 11:23:05 -0600
> > Michael Roth <mdroth@linux.vnet.ibm.com> wrote:
> > > I'd been tracking Gerd's QMP wakeup series as the s3 resolution we need 
> > > for guest-suspend, is that still the case?
> > 
> > Yes. But now I remembered about a seabios bug with S3... Need to check if
> > it were already addressed.
> 
> Hi Luiz,
> 
> I'm not aware of any recent S3 defects in SeaBIOS.  If there is a
> defect, please let me know.
> 
> (I am aware of recent discussions on SeaBIOS and it running the
> vgabios on s3-resume, but I would not classify that issue as a
> defect.)

The problem is that, the screen goes black after resuming from S3. Gleb
debugged it a bit and he said that it was caused by a change in seabios.

Please, take a look at the last three comments in this bz:

 https://bugzilla.redhat.com/show_bug.cgi?id=772614

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [Qemu-devel] [PATCH v2 0/8] qemu-ga: add support for Windows
  2012-02-06 15:43             ` Luiz Capitulino
@ 2012-02-07  0:09               ` Kevin O'Connor
  2012-02-07  8:44                 ` Gleb Natapov
  0 siblings, 1 reply; 21+ messages in thread
From: Kevin O'Connor @ 2012-02-07  0:09 UTC (permalink / raw)
  To: Luiz Capitulino; +Cc: ghammer, aliguori, Michael Roth, Gleb Natapov, qemu-devel

On Mon, Feb 06, 2012 at 01:43:42PM -0200, Luiz Capitulino wrote:
> "Kevin O'Connor" <kevin@koconnor.net> wrote:
> > On Fri, Feb 03, 2012 at 05:16:27PM -0200, Luiz Capitulino wrote:
> > > On Fri, 03 Feb 2012 11:23:05 -0600
> > > Michael Roth <mdroth@linux.vnet.ibm.com> wrote:
> > > > I'd been tracking Gerd's QMP wakeup series as the s3 resolution we need 
> > > > for guest-suspend, is that still the case?
> > > 
> > > Yes. But now I remembered about a seabios bug with S3... Need to check if
> > > it were already addressed.
> > 
> > I'm not aware of any recent S3 defects in SeaBIOS.  If there is a
> > defect, please let me know.
> > 
> > (I am aware of recent discussions on SeaBIOS and it running the
> > vgabios on s3-resume, but I would not classify that issue as a
> > defect.)
> 
> The problem is that, the screen goes black after resuming from S3. Gleb
> debugged it a bit and he said that it was caused by a change in seabios.
> 
> Please, take a look at the last three comments in this bz:
> 
>  https://bugzilla.redhat.com/show_bug.cgi?id=772614

Perhaps a semantic distinction, but I don't consider that to be a
seabios defect.

In any case, I don't think this was addressed.  Gerd published a patch
that can address this in qemu:
http://www.seabios.org/pipermail/seabios/2012-January/002944.html

-Kevin

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [Qemu-devel] [PATCH v2 0/8] qemu-ga: add support for Windows
  2012-02-07  0:09               ` Kevin O'Connor
@ 2012-02-07  8:44                 ` Gleb Natapov
  2012-02-08  0:35                   ` Kevin O'Connor
  0 siblings, 1 reply; 21+ messages in thread
From: Gleb Natapov @ 2012-02-07  8:44 UTC (permalink / raw)
  To: Kevin O'Connor
  Cc: ghammer, aliguori, qemu-devel, Michael Roth, Luiz Capitulino

On Mon, Feb 06, 2012 at 07:09:42PM -0500, Kevin O'Connor wrote:
> On Mon, Feb 06, 2012 at 01:43:42PM -0200, Luiz Capitulino wrote:
> > "Kevin O'Connor" <kevin@koconnor.net> wrote:
> > > On Fri, Feb 03, 2012 at 05:16:27PM -0200, Luiz Capitulino wrote:
> > > > On Fri, 03 Feb 2012 11:23:05 -0600
> > > > Michael Roth <mdroth@linux.vnet.ibm.com> wrote:
> > > > > I'd been tracking Gerd's QMP wakeup series as the s3 resolution we need 
> > > > > for guest-suspend, is that still the case?
> > > > 
> > > > Yes. But now I remembered about a seabios bug with S3... Need to check if
> > > > it were already addressed.
> > > 
> > > I'm not aware of any recent S3 defects in SeaBIOS.  If there is a
> > > defect, please let me know.
> > > 
> > > (I am aware of recent discussions on SeaBIOS and it running the
> > > vgabios on s3-resume, but I would not classify that issue as a
> > > defect.)
> > 
> > The problem is that, the screen goes black after resuming from S3. Gleb
> > debugged it a bit and he said that it was caused by a change in seabios.
> > 
> > Please, take a look at the last three comments in this bz:
> > 
> >  https://bugzilla.redhat.com/show_bug.cgi?id=772614
> 
> Perhaps a semantic distinction, but I don't consider that to be a
> seabios defect.
> 
Non optimal default. The default didn't change BTW, but it was config
parameter before and we changed it for RHEL. Now config parameter is
gone.

> In any case, I don't think this was addressed.  Gerd published a patch
> that can address this in qemu:
> http://www.seabios.org/pipermail/seabios/2012-January/002944.html
> 
Strictly speaking the patch is incorrect since it introduces the file
for all architectures, but I do not think qemu is the right place to
tune SeaBIOS defaults. I propose this patch instead:
---

Run vgabios during S3 resume by default on QEMU. QEMU still able to modify
SeaBIOS behavior if it wishes so by providing etc/s3-resume-vga-init file.

Gleb Natapov <gleb@redhat.com>
diff --git a/src/optionroms.c b/src/optionroms.c
index 27cfffd..06db1c1 100644
--- a/src/optionroms.c
+++ b/src/optionroms.c
@@ -423,7 +423,7 @@ vga_setup(void)
 
     // Load some config settings that impact VGA.
     EnforceChecksum = romfile_loadint("etc/optionroms-checksum", 1);
-    S3ResumeVgaInit = romfile_loadint("etc/s3-resume-vga-init", 0);
+    S3ResumeVgaInit = romfile_loadint("etc/s3-resume-vga-init", !CONFIG_COREBOOT);
     ScreenAndDebug = romfile_loadint("etc/screen-and-debug", 1);
 
     if (CONFIG_OPTIONROMS_DEPLOYED) {
--
			Gleb.

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* Re: [Qemu-devel] [PATCH v2 0/8] qemu-ga: add support for Windows
  2012-02-07  8:44                 ` Gleb Natapov
@ 2012-02-08  0:35                   ` Kevin O'Connor
  2012-02-08  7:18                     ` Gleb Natapov
  0 siblings, 1 reply; 21+ messages in thread
From: Kevin O'Connor @ 2012-02-08  0:35 UTC (permalink / raw)
  To: Gleb Natapov; +Cc: ghammer, aliguori, qemu-devel, Michael Roth, Luiz Capitulino

On Tue, Feb 07, 2012 at 10:44:39AM +0200, Gleb Natapov wrote:
> On Mon, Feb 06, 2012 at 07:09:42PM -0500, Kevin O'Connor wrote:
> > Perhaps a semantic distinction, but I don't consider that to be a
> > seabios defect.
> > 
> Non optimal default. The default didn't change BTW, but it was config
> parameter before and we changed it for RHEL. Now config parameter is
> gone.

Config parameter moved from compile time to runtime.  But I agree it
is unfortunate that knowledge of that didn't get to the right people.

> > In any case, I don't think this was addressed.  Gerd published a patch
> > that can address this in qemu:
> > http://www.seabios.org/pipermail/seabios/2012-January/002944.html
> > 
> Strictly speaking the patch is incorrect since it introduces the file
> for all architectures, but I do not think qemu is the right place to
> tune SeaBIOS defaults. I propose this patch instead:
[...]
>      // Load some config settings that impact VGA.
>      EnforceChecksum = romfile_loadint("etc/optionroms-checksum", 1);
> -    S3ResumeVgaInit = romfile_loadint("etc/s3-resume-vga-init", 0);
> +    S3ResumeVgaInit = romfile_loadint("etc/s3-resume-vga-init", !CONFIG_COREBOOT);
>      ScreenAndDebug = romfile_loadint("etc/screen-and-debug", 1);

I'm concerned about the VGA passthrough case.  (I know that's not
common and has other issues, but I also know several people have been
working with it.)  As near as I can tell, running the VGA rom on S3
resume has as much chance of breaking things as helping things.  It's
fine for the cirrus/bochsvga vgaroms that are totally under our
control, but it'd be an open guess for any third-party code.  (Again,
if someone has documentation to the contrary please let me know.)

So, compiling this into SeaBIOS doesn't seems like the right choice to
me.

-Kevin

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [Qemu-devel] [PATCH v2 0/8] qemu-ga: add support for Windows
  2012-02-08  0:35                   ` Kevin O'Connor
@ 2012-02-08  7:18                     ` Gleb Natapov
  2012-02-08 13:25                       ` Kevin O'Connor
  0 siblings, 1 reply; 21+ messages in thread
From: Gleb Natapov @ 2012-02-08  7:18 UTC (permalink / raw)
  To: Kevin O'Connor
  Cc: ghammer, aliguori, qemu-devel, Michael Roth, Luiz Capitulino

On Tue, Feb 07, 2012 at 07:35:34PM -0500, Kevin O'Connor wrote:
> > > In any case, I don't think this was addressed.  Gerd published a patch
> > > that can address this in qemu:
> > > http://www.seabios.org/pipermail/seabios/2012-January/002944.html
> > > 
> > Strictly speaking the patch is incorrect since it introduces the file
> > for all architectures, but I do not think qemu is the right place to
> > tune SeaBIOS defaults. I propose this patch instead:
> [...]
> >      // Load some config settings that impact VGA.
> >      EnforceChecksum = romfile_loadint("etc/optionroms-checksum", 1);
> > -    S3ResumeVgaInit = romfile_loadint("etc/s3-resume-vga-init", 0);
> > +    S3ResumeVgaInit = romfile_loadint("etc/s3-resume-vga-init", !CONFIG_COREBOOT);
> >      ScreenAndDebug = romfile_loadint("etc/screen-and-debug", 1);
> 
> I'm concerned about the VGA passthrough case.  (I know that's not
> common and has other issues, but I also know several people have been
> working with it.)  As near as I can tell, running the VGA rom on S3
> resume has as much chance of breaking things as helping things.  It's
> fine for the cirrus/bochsvga vgaroms that are totally under our
> control, but it'd be an open guess for any third-party code.  (Again,
> if someone has documentation to the contrary please let me know.)
> 
VGA passthrough does not work with QEMU without code changes. Whoever
works on it will have to provide etc/s3-resume-vga-init file with
appropriate value. My patch above does not remove run time selection, it
only changes the default.

> So, compiling this into SeaBIOS doesn't seems like the right choice to
> me.
> 
It is still run time selectable. I think it is best of both worlds.

--
			Gleb.

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [Qemu-devel] [PATCH v2 0/8] qemu-ga: add support for Windows
  2012-02-08  7:18                     ` Gleb Natapov
@ 2012-02-08 13:25                       ` Kevin O'Connor
  0 siblings, 0 replies; 21+ messages in thread
From: Kevin O'Connor @ 2012-02-08 13:25 UTC (permalink / raw)
  To: Gleb Natapov; +Cc: ghammer, aliguori, qemu-devel, Michael Roth, Luiz Capitulino

On Wed, Feb 08, 2012 at 09:18:24AM +0200, Gleb Natapov wrote:
> On Tue, Feb 07, 2012 at 07:35:34PM -0500, Kevin O'Connor wrote:
> > I'm concerned about the VGA passthrough case.  (I know that's not
> > common and has other issues, but I also know several people have been
> > working with it.)  As near as I can tell, running the VGA rom on S3
> > resume has as much chance of breaking things as helping things.  It's
> > fine for the cirrus/bochsvga vgaroms that are totally under our
> > control, but it'd be an open guess for any third-party code.  (Again,
> > if someone has documentation to the contrary please let me know.)
> > 
> VGA passthrough does not work with QEMU without code changes. Whoever
> works on it will have to provide etc/s3-resume-vga-init file with
> appropriate value. My patch above does not remove run time selection, it
> only changes the default.

True.

I view running the vgabios on s3 a hack and think an explicit "please
apply hack" flag is nicer than the inverse.

However, it's clear this hack helps the majority of qemu/kvm users.
So, I'm okay with changing the default.  It is a change of default
though (upstream kvm/qemu has never run the vgabios on s3 resume
before).  So, we need to make sure there's proper notice of the change
and assuming no objection I'll go forward with it.

-Kevin

^ permalink raw reply	[flat|nested] 21+ messages in thread

end of thread, other threads:[~2012-02-08 13:26 UTC | newest]

Thread overview: 21+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-02-02 19:58 [Qemu-devel] [PATCH v2 0/8] qemu-ga: add support for Windows Michael Roth
2012-02-02 19:58 ` [Qemu-devel] [PATCH v2 1/8] qemu-ga: Add schema documentation for types Michael Roth
2012-02-02 19:58 ` [Qemu-devel] [PATCH v2 2/8] qemu-ga: move channel/transport functionality into wrapper class Michael Roth
2012-02-02 19:58 ` [Qemu-devel] [PATCH v2 3/8] qemu-ga: separate out common commands from posix-specific ones Michael Roth
2012-02-02 19:58 ` [Qemu-devel] [PATCH v2 4/8] qemu-ga: rename guest-agent-commands.c -> commands-posix.c Michael Roth
2012-02-02 19:58 ` [Qemu-devel] [PATCH v2 5/8] qemu-ga: fixes for win32 build of qemu-ga Michael Roth
2012-02-02 19:58 ` [Qemu-devel] [PATCH v2 6/8] qemu-ga: add initial win32 support Michael Roth
2012-02-02 19:58 ` [Qemu-devel] [PATCH v2 7/8] qemu-ga: add Windows service integration Michael Roth
2012-02-02 19:59 ` [Qemu-devel] [PATCH v2 8/8] qemu-ga: add win32 guest-shutdown command Michael Roth
2012-02-03 14:18 ` [Qemu-devel] [PATCH v2 0/8] qemu-ga: add support for Windows Luiz Capitulino
2012-02-03 16:37   ` Michael Roth
2012-02-03 16:45     ` Luiz Capitulino
2012-02-03 17:23       ` Michael Roth
2012-02-03 19:16         ` Luiz Capitulino
2012-02-04 15:34           ` Kevin O'Connor
2012-02-06 15:43             ` Luiz Capitulino
2012-02-07  0:09               ` Kevin O'Connor
2012-02-07  8:44                 ` Gleb Natapov
2012-02-08  0:35                   ` Kevin O'Connor
2012-02-08  7:18                     ` Gleb Natapov
2012-02-08 13:25                       ` Kevin O'Connor

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).