Re: [Qemu-devel] Re: [RFC][PATCH v1 00/11] QEMU Guest Agent: QMP-based host/guest communication (virtagent)

[Michael Roth]

On 03/25/2011 05:03 PM, Anthony Liguori wrote:

On 03/25/2011 03:42 PM, Michael Roth wrote:

On 03/25/2011 02:47 PM, Michael Roth wrote:

These apply on top of Anthony's glib tree, commit
03d5927deb5e6baebaade1b4c8ff2428a85e125c currently, and can also be
obtained from:
git://repo.or.cz/qemu/mdroth.git qga_v1

The QGA-specific patches are in pretty rough shape, and there are
some outstanding issues that I'll note below. I just wanted to put
the general approach out there for consideration. Patch-level
comments/review are still much-appreciated though.

However, patches 1-5 are general json/QAPI-related fixes. Anthony,
please consider pulling these into your glib tree. The json fix-ups
may need further evaluation, but I'm confident they're at least an
improvement. The QAPI ones are trivial fix-ups.

ISSUES/TODOS:

- QMP's async callbacks expect the command results to be
de-marshalled beforehand. This is completely infeasible to attempt
outside of the code generator, so this is a big area that needs to be
addressed. So for now, only the 'guest-ping' command works, since it
has no return value. The dummy "guest-view-file" command will cause
an error to be reported to the client.


Well, not completely de-marshalled.


I don't follow. Are you talking about async callbacks within QEMU?
Because that should be totally transparent to you. You'll receive a
qobject and you can do whatever you need with it.



Heh, I was terribly confused when I initially noted that issue...must've 
been looking at a function further up the return chain and convinced 
myself something outside qmp was expected to de-marshal, but that was 
handled elsewhere in qmp-marshal.c


The follow-up was with regard to the much-lesser issue of dealing with 
something like:


static void qmp_guest_view_file_cb(void *qmp__opaque, QObject 
*qmp__retval, Error *qmp__err)


The generated code expects that to be a QObject of a certain type. On 
the QmpProxy side, we have a response from the guest in the form of a 
qdict, with the retval stored with the "return" key:


{ "return":  }

So you need to pull  out to pass to the callback. I 
was under the impression that there was no generic qdict_get(), and that 
you were expected to know the type in advance. But...



Basically just need a way to pull whatever is stored in the response
qdict's "return" field out without specifying the QTYPE in advance...
which exists in qdict.c:qdict_get_obj(), it's just not currently
exposed to outside callers.


Just use qdict_get()--but I still don't understand what the problem is.


Argh! So that solves the "problem" completely. Thanks :)



Regards,

Anthony Liguori

[Qemu-devel] Re: [RFC][PATCH v1 08/12] qemu-char: add qmp_proxy chardev

[Michael Roth]

On 03/25/2011 04:29 PM, Anthony Liguori wrote:

On 03/25/2011 02:47 PM, Michael Roth wrote:

This allows qemu to be started with guest agent support via:

qemu -chardev qmp_proxy,path=,id=qmp_proxy \
-device ...,chardev=qmp_proxy

It is essentially a wrapper for -chardev socket, which takes the extra
step of setting required defaults and initializing the qmp proxy by
passing the path argument along.

Not sure if this is the most elegant approach, but in terms of the
command-line invocation it seems to be the most consistent way to do
it.

Signed-off-by: Michael Roth
---
qemu-char.c | 46 ++
1 files changed, 46 insertions(+), 0 deletions(-)

diff --git a/qemu-char.c b/qemu-char.c
index d301925..6ff7698 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -2275,6 +2275,51 @@ static CharDriverState
*qemu_chr_open_socket(QemuOpts *opts)
return NULL;
}

+#include "qmp-proxy-core.h"
+
+extern QmpProxy *qmp_proxy_default;
+
+static CharDriverState *qemu_chr_open_qmp_proxy(QemuOpts *opts)
+{
+ CharDriverState *chr;
+ QmpProxy *p;
+ const char *path;
+
+ /* revert to/enforce default socket chardev options for qmp proxy */
+ path = qemu_opt_get(opts, "path");
+ if (path == NULL) {
+ path = QMP_PROXY_PATH_DEFAULT;
+ qemu_opt_set_qerr(opts, "path", path);
+ }
+ /* required options for qmp proxy */
+ qemu_opt_set_qerr(opts, "server", "on");
+ qemu_opt_set_qerr(opts, "wait", "off");
+ qemu_opt_set_qerr(opts, "telnet", "off");


Why are these options required?


The qmp_proxy_new() constructor expects a path to a socket it can 
connect() to. Not sure about telnet, but the other options are required 
for this. Well...server=on at least, wait=off needs to be set as well 
because that's the only way to have the socket chardev set the listening 
socket to non-block, since qmp_proxy_new() calls connect() on it before 
we return to the main I/O loop.


Although, we probably shouldn't just silently switch out explicitly set 
options; an error would probably be more appropriate here.




Regards,

Anthony Liguori

Re: [Qemu-devel] Re: [RFC][PATCH v1 00/11] QEMU Guest Agent: QMP-based host/guest communication (virtagent)

[Anthony Liguori]

On 03/25/2011 03:42 PM, Michael Roth wrote:

On 03/25/2011 02:47 PM, Michael Roth wrote:
These apply on top of Anthony's glib tree, commit 
03d5927deb5e6baebaade1b4c8ff2428a85e125c currently, and can also be 
obtained from:

git://repo.or.cz/qemu/mdroth.git qga_v1

The QGA-specific patches are in pretty rough shape, and there are 
some outstanding issues that I'll note below. I just wanted to put 
the general approach out there for consideration. Patch-level 
comments/review are still much-appreciated though.


However, patches 1-5 are general json/QAPI-related fixes. Anthony, 
please consider pulling these into your glib tree. The json fix-ups 
may need further evaluation, but I'm confident they're at least an 
improvement. The QAPI ones are trivial fix-ups.


ISSUES/TODOS:

  - QMP's async callbacks expect the command results to be 
de-marshalled beforehand. This is completely infeasible to attempt 
outside of the code generator, so this is a big area that needs to be 
addressed. So for now, only the 'guest-ping' command works, since it 
has no return value. The dummy "guest-view-file" command will cause 
an error to be reported to the client.


Well, not completely de-marshalled. 


I don't follow.  Are you talking about async callbacks within QEMU?  
Because that should be totally transparent to you.  You'll receive a 
qobject and you can do whatever you need with it.


Basically just need a way to pull whatever is stored in the response 
qdict's "return" field out without specifying the QTYPE in advance... 
which exists in qdict.c:qdict_get_obj(), it's just not currently 
exposed to outside callers.


Just use qdict_get()--but I still don't understand what the problem is.

Regards,

Anthony Liguori


Alternatively, the callback function can take in the entire response's 
qdict and pull the value out using knowledge from the schema. Will 
look into this more, but not nearly involved as I first thought.


  - qemu-ga guest daemon is currently not working over virtio-serial 
or isa-serial. This is probably an issue with how I'm using glib's io 
channel interfaces, still investigating. This means the only way to 
currently test is by invocing qemu-ga with "-c unix-listen 
-p", then doing something like `socat /dev/ttyS0,raw,echo=0 
unix-connect:`.
  - guest-view-file is a stub, and will be broken out into an 
open/read/close set of RPCs, possibly with a high-level interface 
built around those.

[Qemu-devel] Re: [RFC][PATCH v1 07/12] qmp proxy: core code for proxying qmp requests to guest

[Michael Roth]

On 03/25/2011 04:27 PM, Anthony Liguori wrote:

On 03/25/2011 02:47 PM, Michael Roth wrote:

This provides a QmpProxy class, 1 instance of which is shared by all QMP
servers/sessions to send/receive QMP requests/responses between QEMU and
the QEMU guest agent.

A single qmp_proxy_send_request() is the only interface currently needed
by a QMP session, QAPI/QMP's existing async support handles all the work
of doing callbacks and routing responses to the proper session.

Currently the class requires a path to a listening socket that either
corresponds to the chardev that the guest agent is communicating
through, or a local socket so we can communicate with a host-side
"guest" agent for testing purposes.

A subsequent patch will introduce a new chardev that sets up the
socket chardev and initializes the QmpProxy instance to abstract this
away from the user. Unifying this with local "guest" agent support may
not be feasible, so another command-line option may be needed support
host-side-only testing.

Signed-off-by: Michael Roth
---
qmp-core.c | 8 ++
qmp-core.h | 7 +-
qmp-proxy-core.h | 20 
qmp-proxy.c | 335 ++
vl.c | 1 +
5 files changed, 365 insertions(+), 6 deletions(-)
create mode 100644 qmp-proxy-core.h
create mode 100644 qmp-proxy.c

diff --git a/qmp-core.c b/qmp-core.c
index 9f3d182..dab50a1 100644
--- a/qmp-core.c
+++ b/qmp-core.c
@@ -937,7 +937,15 @@ void qmp_async_complete_command(QmpCommandState
*cmd, QObject *retval, Error *er
qemu_free(cmd);
}

+extern QmpProxy *qmp_proxy_default;
+
void qmp_guest_dispatch(const char *name, const QDict *args, Error
**errp,
QmpGuestCompletionFunc *cb, void *opaque)
{
+ if (!qmp_proxy_default) {
+ /* TODO: should set errp here */
+ fprintf(stderr, "qmp proxy: no guest proxy found\n");
+ return;
+ }
+ qmp_proxy_send_request(qmp_proxy_default, name, args, errp, cb,
opaque);
}
diff --git a/qmp-core.h b/qmp-core.h
index b676020..114d290 100644
--- a/qmp-core.h
+++ b/qmp-core.h
@@ -4,6 +4,7 @@
#include "monitor.h"
#include "qmp-marshal-types.h"
#include "error_int.h"
+#include "qmp-proxy-core.h"

struct QmpCommandState
{
@@ -85,11 +86,5 @@ int qmp_state_get_fd(QmpState *sess);
} \
} while(0)

-typedef void (QmpGuestCompletionFunc)(void *opaque, QObject
*ret_data, Error *err);
-
-void qmp_guest_dispatch(const char *name, const QDict *args, Error
**errp,
- QmpGuestCompletionFunc *cb, void *opaque);
-
-
#endif

diff --git a/qmp-proxy-core.h b/qmp-proxy-core.h
new file mode 100644
index 000..47ac85d
--- /dev/null
+++ b/qmp-proxy-core.h
@@ -0,0 +1,20 @@
+#ifndef QMP_PROXY_CORE_H
+#define QMP_PROXY_CORE_H
+
+#define QMP_PROXY_PATH_DEFAULT "/tmp/qmp-proxy.sock"
+
+typedef void (QmpGuestCompletionFunc)(void *opaque, QObject *ret_data,
+ Error *err);
+
+void qmp_guest_dispatch(const char *name, const QDict *args, Error
**errp,
+ QmpGuestCompletionFunc *cb, void *opaque);
+
+typedef struct QmpProxy QmpProxy;
+
+void qmp_proxy_send_request(QmpProxy *p, const char *name,
+ const QDict *args, Error **errp,
+ QmpGuestCompletionFunc *cb, void *opaque);
+QmpProxy *qmp_proxy_new(const char *channel_path);
+void qmp_proxy_close(QmpProxy *p);
+
+#endif
diff --git a/qmp-proxy.c b/qmp-proxy.c
new file mode 100644
index 000..eaa6e6e
--- /dev/null
+++ b/qmp-proxy.c
@@ -0,0 +1,335 @@
+/*
+ * QMP definitions for communicating with guest agent
+ *
+ * Copyright IBM Corp. 2011
+ *
+ * Authors:
+ * Michael Roth
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or
later.
+ * See the COPYING file in the top-level directory.
+ *
+ */
+
+#include "qmp.h"
+#include "qmp-core.h"
+#include "qemu-queue.h"
+#include "json-parser.h"
+#include "json-streamer.h"
+#include "qemu_socket.h"
+
+#define QMP_SENTINEL 0xFF
+
+typedef struct QmpProxyRequest {
+ const char *name;
+ const QDict *args;
+ QmpGuestCompletionFunc *cb;
+ void *opaque;
+ QString *json;
+ QTAILQ_ENTRY(QmpProxyRequest) entry;
+} QmpProxyRequest;
+
+typedef struct QmpProxyWriteState {
+ QmpProxyRequest *current_request;
+ const char *buf;
+ size_t size;
+ size_t pos;
+ bool use_sentinel;
+} QmpProxyWriteState;
+
+typedef struct QmpProxyReadState {
+ char *buf;
+ size_t size;
+ size_t pos;
+} QmpProxyReadState;


I suspect you should use GStrings for both the rx and tx buffers. See
the QmpUnixSession for an example.


+struct QmpProxy {
+ int fd;
+ const char *path;
+ QmpProxyWriteState write_state;
+ QmpProxyReadState read_state;
+ JSONMessageParser parser;
+ QTAILQ_HEAD(, QmpProxyRequest) sent_requests;
+ QTAILQ_HEAD(, QmpProxyRequest) queued_requests;
+ QString *xport_event;
+ QString *xport_event_sending;
+};
+
+static void qmp_proxy_read_handler(void *opaque);
+static void qmp_proxy_write_handler(void *opaque);
+
+static int qmp_proxy_cancel_request(QmpProxy *p, QmpProxyRequest *r)
+{
+ if (r->name) {
+ if (r->cb) {
+ r->cb(r->opaque, NULL, NULL);
+ }
+ }
+
+ return 0;
+}
+
+static int qmp_proxy_cancel_all(QmpProxy *p)
+{
+ QmpProxyRequest *r, *tmp;
+ QTAILQ

[Qemu-devel] Re: [RFC][PATCH v1 08/12] qemu-char: add qmp_proxy chardev

[Anthony Liguori]

On 03/25/2011 02:47 PM, Michael Roth wrote:

This allows qemu to be started with guest agent support via:

qemu -chardev qmp_proxy,path=,id=qmp_proxy \
   -device ...,chardev=qmp_proxy

It is essentially a wrapper for -chardev socket, which takes the extra
step of setting required defaults and initializing the qmp proxy by
passing the path argument along.

Not sure if this is the most elegant approach, but in terms of the
command-line invocation it seems to be the most consistent way to do
it.

Signed-off-by: Michael Roth
---
  qemu-char.c |   46 ++
  1 files changed, 46 insertions(+), 0 deletions(-)

diff --git a/qemu-char.c b/qemu-char.c
index d301925..6ff7698 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -2275,6 +2275,51 @@ static CharDriverState *qemu_chr_open_socket(QemuOpts 
*opts)
  return NULL;
  }

+#include "qmp-proxy-core.h"
+
+extern QmpProxy *qmp_proxy_default;
+
+static CharDriverState *qemu_chr_open_qmp_proxy(QemuOpts *opts)
+{
+CharDriverState *chr;
+QmpProxy *p;
+const char *path;
+
+/* revert to/enforce default socket chardev options for qmp proxy */
+path = qemu_opt_get(opts, "path");
+if (path == NULL) {
+path = QMP_PROXY_PATH_DEFAULT;
+qemu_opt_set_qerr(opts, "path", path);
+}
+/* required options for qmp proxy */
+qemu_opt_set_qerr(opts, "server", "on");
+qemu_opt_set_qerr(opts, "wait", "off");
+qemu_opt_set_qerr(opts, "telnet", "off");


Why are these options required?

Regards,

Anthony Liguori


+
+chr = qemu_chr_open_socket(opts);
+if (chr == NULL) {
+goto err;
+}
+
+/* initialize virtagent using the socket we just set up */
+if (qmp_proxy_default) {
+fprintf(stderr, "error, multiple qmp guest proxies are not allowed\n");
+}
+p = qmp_proxy_new(path);
+if (p == NULL) {
+fprintf(stderr, "error initializing qmp guest proxy\n");
+goto err;
+}
+qmp_proxy_default = p;
+
+return chr;
+err:
+if (chr) {
+qemu_free(chr);
+}
+return NULL;
+}
+
  /***/
  /* Memory chardev */
  typedef struct {
@@ -2495,6 +2540,7 @@ static const struct {
  || defined(__FreeBSD_kernel__)
  { .name = "parport",   .open = qemu_chr_open_pp },
  #endif
+{ .name = "qmp_proxy", .open = qemu_chr_open_qmp_proxy },
  };

  CharDriverState *qemu_chr_open_opts(QemuOpts *opts,

[Qemu-devel] Re: [RFC][PATCH v1 07/12] qmp proxy: core code for proxying qmp requests to guest

[Anthony Liguori]

On 03/25/2011 02:47 PM, Michael Roth wrote:

This provides a QmpProxy class, 1 instance of which is shared by all QMP
servers/sessions to send/receive QMP requests/responses between QEMU and
the QEMU guest agent.

A single qmp_proxy_send_request() is the only interface currently needed
by a QMP session, QAPI/QMP's existing async support handles all the work
of doing callbacks and routing responses to the proper session.

Currently the class requires a path to a listening socket that either
corresponds to the chardev that the guest agent is communicating
through, or a local socket so we can communicate with a host-side
"guest" agent for testing purposes.

A subsequent patch will introduce a new chardev that sets up the
socket chardev and initializes the QmpProxy instance to abstract this
away from the user. Unifying this with local "guest" agent support may
not be feasible, so another command-line option may be needed support
host-side-only testing.

Signed-off-by: Michael Roth
---
  qmp-core.c   |8 ++
  qmp-core.h   |7 +-
  qmp-proxy-core.h |   20 
  qmp-proxy.c  |  335 ++
  vl.c |1 +
  5 files changed, 365 insertions(+), 6 deletions(-)
  create mode 100644 qmp-proxy-core.h
  create mode 100644 qmp-proxy.c

diff --git a/qmp-core.c b/qmp-core.c
index 9f3d182..dab50a1 100644
--- a/qmp-core.c
+++ b/qmp-core.c
@@ -937,7 +937,15 @@ void qmp_async_complete_command(QmpCommandState *cmd, 
QObject *retval, Error *er
  qemu_free(cmd);
  }

+extern QmpProxy *qmp_proxy_default;
+
  void qmp_guest_dispatch(const char *name, const QDict *args, Error **errp,
  QmpGuestCompletionFunc *cb, void *opaque)
  {
+if (!qmp_proxy_default) {
+/* TODO: should set errp here */
+fprintf(stderr, "qmp proxy: no guest proxy found\n");
+return;
+}
+qmp_proxy_send_request(qmp_proxy_default, name, args, errp, cb, opaque);
  }
diff --git a/qmp-core.h b/qmp-core.h
index b676020..114d290 100644
--- a/qmp-core.h
+++ b/qmp-core.h
@@ -4,6 +4,7 @@
  #include "monitor.h"
  #include "qmp-marshal-types.h"
  #include "error_int.h"
+#include "qmp-proxy-core.h"

  struct QmpCommandState
  {
@@ -85,11 +86,5 @@ int qmp_state_get_fd(QmpState *sess);
  }\
  } while(0)

-typedef void (QmpGuestCompletionFunc)(void *opaque, QObject *ret_data, Error 
*err);
-
-void qmp_guest_dispatch(const char *name, const QDict *args, Error **errp,
-QmpGuestCompletionFunc *cb, void *opaque);
-
-
  #endif

diff --git a/qmp-proxy-core.h b/qmp-proxy-core.h
new file mode 100644
index 000..47ac85d
--- /dev/null
+++ b/qmp-proxy-core.h
@@ -0,0 +1,20 @@
+#ifndef QMP_PROXY_CORE_H
+#define QMP_PROXY_CORE_H
+
+#define QMP_PROXY_PATH_DEFAULT "/tmp/qmp-proxy.sock"
+
+typedef void (QmpGuestCompletionFunc)(void *opaque, QObject *ret_data,
+  Error *err);
+
+void qmp_guest_dispatch(const char *name, const QDict *args, Error **errp,
+QmpGuestCompletionFunc *cb, void *opaque);
+
+typedef struct QmpProxy QmpProxy;
+
+void qmp_proxy_send_request(QmpProxy *p, const char *name,
+const QDict *args, Error **errp,
+QmpGuestCompletionFunc *cb, void *opaque);
+QmpProxy *qmp_proxy_new(const char *channel_path);
+void qmp_proxy_close(QmpProxy *p);
+
+#endif
diff --git a/qmp-proxy.c b/qmp-proxy.c
new file mode 100644
index 000..eaa6e6e
--- /dev/null
+++ b/qmp-proxy.c
@@ -0,0 +1,335 @@
+/*
+ * QMP definitions for communicating with guest agent
+ *
+ * Copyright IBM Corp. 2011
+ *
+ * Authors:
+ *  Michael Roth
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ *
+ */
+
+#include "qmp.h"
+#include "qmp-core.h"
+#include "qemu-queue.h"
+#include "json-parser.h"
+#include "json-streamer.h"
+#include "qemu_socket.h"
+
+#define QMP_SENTINEL 0xFF
+
+typedef struct QmpProxyRequest {
+const char *name;
+const QDict *args;
+QmpGuestCompletionFunc *cb;
+void *opaque;
+QString *json;
+QTAILQ_ENTRY(QmpProxyRequest) entry;
+} QmpProxyRequest;
+
+typedef struct QmpProxyWriteState {
+QmpProxyRequest *current_request;
+const char *buf;
+size_t size;
+size_t pos;
+bool use_sentinel;
+} QmpProxyWriteState;
+
+typedef struct QmpProxyReadState {
+char *buf;
+size_t size;
+size_t pos;
+} QmpProxyReadState;


I suspect you should use GStrings for both the rx and tx buffers.  See 
the QmpUnixSession for an example.



+struct QmpProxy {
+int fd;
+const char *path;
+QmpProxyWriteState write_state;
+QmpProxyReadState read_state;
+JSONMessageParser parser;
+QTAILQ_HEAD(, QmpProxyRequest) sent_requests;
+QTAILQ_HEAD(, QmpProxyRequest) queued_requests;
+QString *xport_event;
+QString *xport_event_sen

[Qemu-devel] Re: [RFC][PATCH v1 05/12] qapi: fix handling for null-return async callbacks

[Anthony Liguori]

On 03/25/2011 02:47 PM, Michael Roth wrote:

Async commands like 'guest-ping' have NULL retvals. Handle these by
inserting an empty dictionary in the response's "return" field.

Signed-off-by: Michael Roth
---
  qmp-core.c |5 -
  1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/qmp-core.c b/qmp-core.c
index e33f7a4..9f3d182 100644
--- a/qmp-core.c
+++ b/qmp-core.c
@@ -922,9 +922,12 @@ void qmp_async_complete_command(QmpCommandState *cmd, 
QObject *retval, Error *er
  rsp = qdict_new();
  if (err) {
  qdict_put_obj(rsp, "error", error_get_qobject(err));
-} else {
+} else if (retval) {
  qobject_incref(retval);
  qdict_put_obj(rsp, "return", retval);
+} else {
+/* add empty "return" dict, this is the standard for NULL returns */
+qdict_put_obj(rsp, "return", QOBJECT(qdict_new()));


Luiz, I know we decided to return empty dicts because it lets us extend 
things better, but did we want to rule out the use of a 'null' return 
value entirely?


For a command like this, I can't imagine ever wanting to extend the 
return value...


Regards,

Anthony Liguori


  }
  if (cmd->tag) {
  qdict_put_obj(rsp, "tag", cmd->tag);

[Qemu-devel] Re: [RFC][PATCH v1 01/12] json-lexer: make lexer error-recovery more deterministic

[Anthony Liguori]

On 03/25/2011 02:47 PM, Michael Roth wrote:

Currently when we reach an error state we effectively flush everything
fed to the lexer, which can put us in a state where we keep feeding
tokens into the parser at arbitrary offsets in the stream. This makes it
difficult for the lexer/tokenizer/parser to get back in sync when bad
input is made by the client.

With these changes we emit an error state/token up to the tokenizer as
soon as we reach an error state, and continue processing any data passed
in rather than bailing out. The reset token will be used to reset the
tokenizer and parser, such that they'll recover state as soon as the
lexer begins generating valid token sequences again.

We also map chr(0xFF) to an error state here, since it's an invalid
UTF-8 character. QMP guest proxy/agent use this to force a flush/reset
of previous input for reliable delivery of certain events, so also we
document that thoroughly here.

Signed-off-by: Michael Roth
---
  json-lexer.c |   22 ++
  json-lexer.h |1 +
  2 files changed, 19 insertions(+), 4 deletions(-)

diff --git a/json-lexer.c b/json-lexer.c
index 3462c89..21aa03a 100644
--- a/json-lexer.c
+++ b/json-lexer.c
@@ -105,7 +105,7 @@ static const uint8_t json_lexer[][256] =  {
  ['u'] = IN_DQ_UCODE0,
  },
  [IN_DQ_STRING] = {
-[1 ... 0xFF] = IN_DQ_STRING,
+[1 ... 0xFE] = IN_DQ_STRING,


We also need to exclude 192, 193, 245-254 as these are all invalid bytes 
in a UTF-8 sequence.  See http://en.wikipedia.org/wiki/UTF-8#Codepage_layout


We probably ought to actually handle UTF-8 extend byte sequences in the 
lexer but we can keep this as a future exercise.



  ['\\'] = IN_DQ_STRING_ESCAPE,
  ['"'] = JSON_STRING,
  },
@@ -144,7 +144,7 @@ static const uint8_t json_lexer[][256] =  {
  ['u'] = IN_SQ_UCODE0,
  },
  [IN_SQ_STRING] = {
-[1 ... 0xFF] = IN_SQ_STRING,
+[1 ... 0xFE] = IN_SQ_STRING,
  ['\\'] = IN_SQ_STRING_ESCAPE,
  ['\''] = JSON_STRING,
  },
@@ -305,10 +305,25 @@ static int json_lexer_feed_char(JSONLexer *lexer, char ch)
  new_state = IN_START;
  break;
  case ERROR:
+/* XXX: To avoid having previous bad input leaving the parser in an
+ * unresponsive state where we consume unpredictable amounts of
+ * subsequent "good" input, percolate this error state up to the
+ * tokenizer/parser by forcing a NULL object to be emitted, then
+ * reset state.
+ *
+ * Also note that this handling is required for reliable channel
+ * negotiation between QMP and the guest agent, since chr(0xFF)
+ * is placed at the beginning of certain events to ensure proper
+ * delivery when the channel is in an unknown state. chr(0xFF) is
+ * never a valid ASCII/UTF-8 sequence, so this should reliably
+ * induce an error/flush state.
+ */
+lexer->emit(lexer, lexer->token, JSON_ERROR, lexer->x, lexer->y);
  QDECREF(lexer->token);
  lexer->token = qstring_new();
  new_state = IN_START;
-return -EINVAL;
+lexer->state = new_state;
+return 0;
  default:
  break;
  }
@@ -334,7 +349,6 @@ int json_lexer_feed(JSONLexer *lexer, const char *buffer, 
size_t size)

  for (i = 0; i<  size; i++) {
  int err;
-


This whitespace change slipped in FWIW.

Regards,

Anthony Liguori


  err = json_lexer_feed_char(lexer, buffer[i]);
  if (err<  0) {
  return err;
diff --git a/json-lexer.h b/json-lexer.h
index 3b50c46..10bc0a7 100644
--- a/json-lexer.h
+++ b/json-lexer.h
@@ -25,6 +25,7 @@ typedef enum json_token_type {
  JSON_STRING,
  JSON_ESCAPE,
  JSON_SKIP,
+JSON_ERROR,
  } JSONTokenType;

  typedef struct JSONLexer JSONLexer;

[Qemu-devel] Re: [RFC][PATCH v1 00/11] QEMU Guest Agent: QMP-based host/guest communication (virtagent)

[Michael Roth]

On 03/25/2011 02:47 PM, Michael Roth wrote:

These apply on top of Anthony's glib tree, commit 
03d5927deb5e6baebaade1b4c8ff2428a85e125c currently, and can also be obtained 
from:
git://repo.or.cz/qemu/mdroth.git qga_v1

The QGA-specific patches are in pretty rough shape, and there are some 
outstanding issues that I'll note below. I just wanted to put the general 
approach out there for consideration. Patch-level comments/review are still 
much-appreciated though.

However, patches 1-5 are general json/QAPI-related fixes. Anthony, please 
consider pulling these into your glib tree. The json fix-ups may need further 
evaluation, but I'm confident they're at least an improvement. The QAPI ones 
are trivial fix-ups.

ISSUES/TODOS:

  - QMP's async callbacks expect the command results to be de-marshalled beforehand. This 
is completely infeasible to attempt outside of the code generator, so this is a big area 
that needs to be addressed. So for now, only the 'guest-ping' command works, since it has 
no return value. The dummy "guest-view-file" command will cause an error to be 
reported to the client.


Well, not completely de-marshalled. Basically just need a way to pull 
whatever is stored in the response qdict's "return" field out without 
specifying the QTYPE in advance... which exists in 
qdict.c:qdict_get_obj(), it's just not currently exposed to outside 
callers. Alternatively, the callback function can take in the entire 
response's qdict and pull the value out using knowledge from the schema. 
Will look into this more, but not nearly involved as I first thought.



  - qemu-ga guest daemon is currently not working over virtio-serial or isa-serial. This is probably 
an issue with how I'm using glib's io channel interfaces, still investigating. This means the only 
way to currently test is by invocing qemu-ga with "-c unix-listen -p", then 
doing something like `socat /dev/ttyS0,raw,echo=0 unix-connect:`.
  - guest-view-file is a stub, and will be broken out into an open/read/close 
set of RPCs, possibly with a high-level interface built around those.

[Qemu-devel] [Bug 741887] Re: virsh snapshot-create too slow (kvm, qcow2, savevm)

[mkopta]

I confirm that without 'cache' option, I have got from iostat those
result while doing 'savevm'

Device: sda
rrqm/s: 0.00
wrqm/s: 316.00
r/s: 0.00
w/s: 94.80
rkB/s: 0.00
wkB/s: 1541.60
avgrq-sz: 32.52
avgqu-sz: 0.98
await: 10.32
svctm: 10.10
%util: 95.76

I also confirm, that when option 'cache=unsafe' is used, snapshot (from
qemu monitor) is done as quickly as it should (few seconds).

I am not sure if this is a solution or workaround or just a closer
description of a bug.

http://libvirt.org/formatdomain.html#elementsDisks describes option
'cache'. When I use that (cache="none") it spits out:

error: Failed to create domain from vm.xml
error: internal error process exited while connecting to monitor: kvm: -drive 
file=/home/dum8d0g/vms/deb.qcow2,if=none,id=drive-ide0-0-0,boot=on,format=qcow2,cache=none:
 could not open disk image /home/dum8d0g/vms/deb.qcow2: Invalid argument

When that option is removed, domain is created successfuly. I guess I
have another bugreport to fill.

So, for me, the issue is somehow solved from the qemu side. I think,
this could be marked as wontfix.

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/741887

Title:
  virsh snapshot-create too slow (kvm, qcow2, savevm)

Status in QEMU:
  New
Status in “qemu-kvm” package in Ubuntu:
  Confirmed

Bug description:
  Action
  ==
  # time virsh snapshot-create 1

  * Taking snapshot of a running KVM virtual machine

  Result
  ==
  Domain snapshot 1300983161 created
  real4m46.994s
  user0m0.000s
  sys 0m0.010s

  Expected result
  ===
  * Snapshot taken after few seconds instead of minutes.

  Environment
  ===
  * Ubuntu Natty Narwhal upgraded from Lucid and Meerkat, fully updated.

  * Stock natty packages of libvirt and qemu installed (libvirt-bin
  0.8.8-1ubuntu5; libvirt0 0.8.8-1ubuntu5; qemu-common 0.14.0+noroms-
  0ubuntu3; qemu-kvm 0.14.0+noroms-0ubuntu3).

  * Virtual machine disk format is qcow2 (debian 5 installed)
  image: /storage/debian.qcow2
  file format: qcow2
  virtual size: 10G (10737418240 bytes)
  disk size: 1.2G
  cluster_size: 65536
  Snapshot list:
  IDTAG VM SIZEDATE   VM CLOCK
  1 snap01  48M 2011-03-24 09:46:33   00:00:58.899
  2 1300979368  58M 2011-03-24 11:09:28   00:01:03.589
  3 1300983161  57M 2011-03-24 12:12:41   00:00:51.905

  * qcow2 disk is stored on ext4 filesystem, without RAID or LVM or any
  special setup.

  * running guest VM takes about 40M RAM from inside, from outside 576M
  are given to that machine

  * host has fast dual-core pentium cpu with virtualization support,
  around 8G of RAM and 7200rpm harddrive (dd from urandom to file gives
  about 20M/s)

  * running processes: sshd, atd (empty), crond (empty), libvirtd, tmux,
  bash, rsyslogd, upstart-socket-bridge, udevd, dnsmasq, iotop (python)

  * networking is done by bridging and bonding

  
  Detail description
  ==

  * Under root, command 'virsh create-snapshot 1' is issued on booted
  and running KVM machine with debian inside.

  * After about four minutes, the process is done.

  * 'iotop' shows two 'kvm' processes reading/writing to disk. First one
  has IO around 1500 K/s, second one has around 400 K/s. That takes
  about three minutes. Then first process grabs about 3 M/s of IO and
  suddenly dissapears (1-2 sec). Then second process does about 7.5 M/s
  of IO for around a 1-2 minutes.

  * Snapshot is successfuly created and is usable for reverting or
  extracting.

  * Pretty much the same behaviour occurs when command 'savevm' is
  issued directly from qemu monitor, without using libvirt at all
  (actually, virsh snapshot-create just calls 'savevm' to the monitor
  socket).

  * This behaviour was observed on lucid, meerkat, natty and even with
  git version of libvirt (f44bfb7fb978c9313ce050a1c4149bf04aa0a670).
  Also slowsave packages from
  https://bugs.launchpad.net/ubuntu/+source/libvirt/+bug/524447 gave
  this issue.

  
  Thank you for helping to solve this issue!

  ProblemType: Bug
  DistroRelease: Ubuntu 11.04
  Package: libvirt-bin 0.8.8-1ubuntu5
  ProcVersionSignature: Ubuntu 2.6.38-7.38-server 2.6.38
  Uname: Linux 2.6.38-7-server x86_64
  Architecture: amd64
  Date: Thu Mar 24 12:19:41 2011
  InstallationMedia: Ubuntu-Server 10.04.2 LTS "Lucid Lynx" - Release amd64 
(20110211.1)
  ProcEnviron:
   LANG=en_US.UTF-8
   SHELL=/bin/bash
  SourcePackage: libvirt
  UpgradeStatus: No upgrade log present (probably fresh install)

[Qemu-devel] [RFC][PATCH v1 09/12] guest agent: core marshal/dispatch interfaces

[Michael Roth]

These are basically a stripped-down, guest-side analogue to what's in
qmp_core.c: definitions used by qmp-gen.py-generated marshalling code to
handle dispatch and a registration of command->function mappings (minus
all the bits related to Qmp sessions/server/events).

As a result of that, there is a little bit of code duplication here.
It wouldn't be difficult to make the common bits shareable, but there
isn't much.

The guest agent will use these interfaces to handle dispatch/execution
of requests it gets from the proxy.

Signed-off-by: Michael Roth 
---
 guest-agent-core.c |  143 
 guest-agent-core.h |   21 
 2 files changed, 164 insertions(+), 0 deletions(-)
 create mode 100644 guest-agent-core.c
 create mode 100644 guest-agent-core.h

diff --git a/guest-agent-core.c b/guest-agent-core.c
new file mode 100644
index 000..6c3b031
--- /dev/null
+++ b/guest-agent-core.c
@@ -0,0 +1,143 @@
+/*
+ * QEMU Guest Agent core functions
+ *
+ * Copyright IBM Corp. 2011
+ *
+ * Authors:
+ *  Adam Litke
+ *  Michael Roth  
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+#include "guest-agent-core.h"
+
+typedef enum QmpCommandType
+{
+QCT_NORMAL,
+QCT_ASYNC,
+} QmpCommandType;
+
+typedef struct QmpCommand
+{
+const char *name;
+QmpCommandType type;
+QmpCommandFunc *fn;
+QmpAsyncCommandFunc *afn;
+QTAILQ_ENTRY(QmpCommand) node;
+} QmpCommand;
+
+static QTAILQ_HEAD(, QmpCommand) qmp_commands =
+QTAILQ_HEAD_INITIALIZER(qmp_commands);
+
+QmpMarshalState *qmp_state_get_mstate(QmpState *sess)
+{
+return NULL;
+}
+
+void qga_register_command(const char *name, QmpCommandFunc *fn)
+{
+QmpCommand *cmd = qemu_mallocz(sizeof(*cmd));
+
+cmd->name = name;
+cmd->type = QCT_NORMAL;
+cmd->fn = fn;
+QTAILQ_INSERT_TAIL(&qmp_commands, cmd, node);
+}
+
+static QmpCommand *qmp_find_command(const char *name)
+{
+QmpCommand *i;
+
+QTAILQ_FOREACH(i, &qmp_commands, node) {
+if (strcmp(i->name, name) == 0) {
+return i;
+}
+}
+return NULL;
+}
+
+static QObject *qga_dispatch_err(QObject *request, Error **errp)
+{
+const char *command;
+QDict *args, *dict;
+QmpCommand *cmd;
+QObject *ret = NULL;
+
+if (qobject_type(request) != QTYPE_QDICT) {
+error_set(errp, QERR_JSON_PARSE_ERROR, "request is not a dictionary");
+goto out;
+}
+
+dict = qobject_to_qdict(request);
+if (!qdict_haskey(dict, "execute")) {
+error_set(errp, QERR_JSON_PARSE_ERROR, "no execute key");
+goto out;
+}
+
+command = qdict_get_str(dict, "execute");
+cmd = qmp_find_command(command);
+if (cmd == NULL) {
+error_set(errp, QERR_COMMAND_NOT_FOUND, command);
+goto out;
+}
+
+if (!qdict_haskey(dict, "arguments")) {
+args = qdict_new();
+} else {
+args = qdict_get_qdict(dict, "arguments");
+QINCREF(args);
+}
+
+switch (cmd->type) {
+case QCT_NORMAL:
+cmd->fn(NULL, args, &ret, errp);
+if (ret == NULL) {
+ret = QOBJECT(qdict_new());
+}
+break;
+case QCT_ASYNC: {
+QmpCommandState *s = qemu_mallocz(sizeof(*s));
+// FIXME save async commands and do something
+// smart if disconnect occurs before completion
+//s->state = state;
+s->tag = NULL;
+if (qdict_haskey(dict, "tag")) {
+s->tag = qdict_get(dict, "tag");
+qobject_incref(s->tag);
+}
+cmd->afn(args, errp, s);
+ret = NULL;
+}
+break;
+}
+
+QDECREF(args);
+
+out:
+qobject_decref(request);
+
+return ret;
+}
+
+QObject *qga_dispatch(QObject *request, Error **errp)
+{
+Error *err = NULL;
+QObject *ret;
+QDict *rsp;
+
+ret = qga_dispatch_err(request, &err);
+
+rsp = qdict_new();
+if (err) {
+qdict_put_obj(rsp, "error", error_get_qobject(err));
+error_free(err);
+} else if (ret) {
+qdict_put_obj(rsp, "return", ret);
+} else {
+QDECREF(rsp);
+return NULL;
+}
+
+return QOBJECT(rsp);
+}
diff --git a/guest-agent-core.h b/guest-agent-core.h
new file mode 100644
index 000..18126b0
--- /dev/null
+++ b/guest-agent-core.h
@@ -0,0 +1,21 @@
+/*
+ * QEMU Guest Agent core declarations
+ *
+ * Copyright IBM Corp. 2011
+ *
+ * Authors:
+ *  Adam Litke
+ *  Michael Roth  
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+#include "qmp-core.h"
+#include "qapi-types-core.h"
+#include "qmp-marshal-types-core.h"
+#include "qemu-common.h"
+#include "qdict.h"
+
+QObject *qga_dispatch(QObject *obj, Error **errp);
+void qga_register_command(const char *name, QmpCommandFunc *fn);
+void qga_init_marshal(void);
-- 
1.7.

[Qemu-devel] [RFC][PATCH v1 11/12] guest agent: guest-side command implementations

[Michael Roth]

This is where the actual commands/RPCs are defined. This patch is mostly
just to serve as an example, but guest-ping actually does everything it
needs to.

view_file will soon be replaced with a stateful open/read/close interface,
and shutdown will be ported over from virtagent soon as well.

Signed-off-by: Michael Roth 
---
 guest-agent-commands.c |   24 
 1 files changed, 24 insertions(+), 0 deletions(-)
 create mode 100644 guest-agent-commands.c

diff --git a/guest-agent-commands.c b/guest-agent-commands.c
new file mode 100644
index 000..ca8a894
--- /dev/null
+++ b/guest-agent-commands.c
@@ -0,0 +1,24 @@
+/*
+ * QEMU Guest Agent commands
+ *
+ * Copyright IBM Corp. 2011
+ *
+ * Authors:
+ *  Michael Roth  
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#include "guest-agent.h"
+
+void qga_guest_ping(Error **err)
+{
+}
+
+char *qga_guest_view_file(const char *filename, Error **err)
+{
+char *test_response = qemu_mallocz(512);
+strcpy(test_response, "this is some text");
+return test_response;
+}
-- 
1.7.0.4

[Qemu-devel] [RFC][PATCH v1 03/12] json-parser: add handling for NULL token list

[Michael Roth]

Currently a NULL token list will crash the parser, instead we have it
pass back a NULL QObject.

Signed-off-by: Michael Roth 
---
 json-parser.c |6 +-
 1 files changed, 5 insertions(+), 1 deletions(-)

diff --git a/json-parser.c b/json-parser.c
index 58e973b..849e215 100644
--- a/json-parser.c
+++ b/json-parser.c
@@ -633,9 +633,13 @@ QObject *json_parser_parse(QList *tokens, va_list *ap)
 QObject *json_parser_parse_err(QList *tokens, va_list *ap, Error **errp)
 {
 JSONParserContext ctxt = {};
-QList *working = qlist_copy(tokens);
+QList *working;
 QObject *result;
 
+if (!tokens) {
+return NULL;
+}
+working = qlist_copy(tokens);
 result = parse_value(&ctxt, &working, ap);
 
 QDECREF(working);
-- 
1.7.0.4

[Qemu-devel] [RFC][PATCH v1 10/12] guest agent: qemu-ga daemon

[Michael Roth]

This is the actual guest daemon, it listens for requests over a
virtio-serial/isa-serial/unix socket channel and routes them through
to dispatch routines, and writes the results back to the channel in
a manner similar to Qmp.

This is currently horribly broken, only the unix-listen channel method
is working at the moment (likely due to mis-use of gio channel
interfaces), and the code is in overall rough shape.

Signed-off-by: Michael Roth 
---
 qemu-ga.c |  522 +
 1 files changed, 522 insertions(+), 0 deletions(-)
 create mode 100644 qemu-ga.c

diff --git a/qemu-ga.c b/qemu-ga.c
new file mode 100644
index 000..435a1fc
--- /dev/null
+++ b/qemu-ga.c
@@ -0,0 +1,522 @@
+/*
+ * QEMU Guest Agent
+ *
+ * Copyright IBM Corp. 2011
+ *
+ * Authors:
+ *  Adam Litke
+ *  Michael Roth  
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include "qemu_socket.h"
+#include "json-streamer.h"
+#include "json-parser.h"
+#include "guest-agent.h"
+
+#define QGA_VERSION "1.0"
+#define QGA_GUEST_PATH_VIRTIO_DEFAULT "/dev/virtio-ports/va"
+#define QGA_PIDFILE_DEFAULT "/var/run/qemu-va.pid"
+#define QGA_BAUDRATE_DEFAULT B38400 /* for isa-serial channels */
+
+bool verbose_enabled = false;
+
+typedef struct GAState {
+bool active;
+int session_id;
+const char *proxy_path;
+JSONMessageParser parser;
+GMainLoop *main_loop;
+guint conn_id;
+GSocket *conn_sock;
+GIOChannel *conn_channel;
+guint listen_id;
+GSocket *listen_sock;
+GIOChannel *listen_channel;
+const char *path;
+const char *method;
+} GAState;
+
+static void usage(const char *cmd)
+{
+printf(
+"Usage: %s -c \n"
+"QEMU virtagent guest agent %s\n"
+"\n"
+"  -c, --channel channel method: one of unix-connect, virtio-serial, or\n"
+"isa-serial\n"
+"  -p, --pathchannel path\n"
+"  -v, --verbose display extra debugging information\n"
+"  -d, --daemonize   become a daemon\n"
+"  -h, --helpdisplay this help and exit\n"
+"\n"
+"Report bugs to \n"
+, cmd, QGA_VERSION);
+}
+
+static void conn_channel_close(GAState *s);
+
+static void become_daemon(void)
+{
+pid_t pid, sid;
+int pidfd;
+char *pidstr;
+
+pid = fork();
+if (pid < 0)
+exit(EXIT_FAILURE);
+if (pid > 0) {
+exit(EXIT_SUCCESS);
+}
+
+pidfd = open(QGA_PIDFILE_DEFAULT, O_CREAT|O_RDWR, S_IRUSR|S_IWUSR);
+if (!pidfd || lockf(pidfd, F_TLOCK, 0))
+g_error("Cannot lock pid file");
+
+if (ftruncate(pidfd, 0) || lseek(pidfd, 0, SEEK_SET))
+   g_error("Cannot truncate pid file");
+if (asprintf(&pidstr, "%d", getpid()) == -1)
+g_error("Cannot allocate memory");
+if (write(pidfd, pidstr, strlen(pidstr)) != strlen(pidstr))
+g_error("Failed to write pid file");
+free(pidstr);
+
+umask(0);
+sid = setsid();
+if (sid < 0)
+goto fail;
+if ((chdir("/")) < 0)
+goto fail;
+
+close(STDIN_FILENO);
+close(STDOUT_FILENO);
+close(STDERR_FILENO);
+return;
+
+fail:
+unlink(QGA_PIDFILE_DEFAULT);
+g_error("failed to daemonize");
+}
+
+static int conn_channel_send_payload(GIOChannel *channel, QObject *payload)
+{
+gsize count, written=0;
+const char *buf;
+QString *payload_qstr;
+GIOStatus status;
+GError *err = NULL;
+
+if (!payload || !channel) {
+return -EINVAL;
+}
+
+payload_qstr = qobject_to_json(payload);
+if (!payload_qstr) {
+return -EINVAL;
+}
+
+buf = qstring_get_str(payload_qstr);
+count = strlen(buf);
+
+while (count) {
+g_warning("sending, count: %d", (int)count);
+status = g_io_channel_write_chars(channel, buf, count, &written, &err);
+if (err != NULL) {
+g_warning("error sending payload: %s", err->message);
+return err->code;
+}
+if (status == G_IO_STATUS_NORMAL) {
+count -= written;
+} else if (status == G_IO_STATUS_ERROR || status == G_IO_STATUS_EOF) {
+return -EPIPE;
+}
+}
+g_io_channel_flush(channel, &err);
+if (err != NULL) {
+g_warning("error flushing payload: %s", err->message);
+return err->code;
+}
+return 0;
+}
+
+/* keep reading channel, but ignore all data until we see an appropriate
+ * ack from the host
+ */
+static void start_negotiation(GAState *s)
+{
+QDict *payload = qdict_new();
+int ret;
+
+g_debug("[re]negotiating connection");
+
+g_assert(s && s->conn_channel);
+s->active = false;
+s->session_id = g_random_int_range(1, G_MAXINT32);
+
+qdict_put_obj(payload, "_xport_event",
+  QOBJECT(qstring_from_str("guest_init")));
+qdict_put_obj(payload, "_xport_arg_sid",
+  

[Qemu-devel] [RFC][PATCH v1 07/12] qmp proxy: core code for proxying qmp requests to guest

[Michael Roth]

This provides a QmpProxy class, 1 instance of which is shared by all QMP
servers/sessions to send/receive QMP requests/responses between QEMU and
the QEMU guest agent.

A single qmp_proxy_send_request() is the only interface currently needed
by a QMP session, QAPI/QMP's existing async support handles all the work
of doing callbacks and routing responses to the proper session.

Currently the class requires a path to a listening socket that either
corresponds to the chardev that the guest agent is communicating
through, or a local socket so we can communicate with a host-side
"guest" agent for testing purposes.

A subsequent patch will introduce a new chardev that sets up the
socket chardev and initializes the QmpProxy instance to abstract this
away from the user. Unifying this with local "guest" agent support may
not be feasible, so another command-line option may be needed support
host-side-only testing.

Signed-off-by: Michael Roth 
---
 qmp-core.c   |8 ++
 qmp-core.h   |7 +-
 qmp-proxy-core.h |   20 
 qmp-proxy.c  |  335 ++
 vl.c |1 +
 5 files changed, 365 insertions(+), 6 deletions(-)
 create mode 100644 qmp-proxy-core.h
 create mode 100644 qmp-proxy.c

diff --git a/qmp-core.c b/qmp-core.c
index 9f3d182..dab50a1 100644
--- a/qmp-core.c
+++ b/qmp-core.c
@@ -937,7 +937,15 @@ void qmp_async_complete_command(QmpCommandState *cmd, 
QObject *retval, Error *er
 qemu_free(cmd);
 }
 
+extern QmpProxy *qmp_proxy_default;
+
 void qmp_guest_dispatch(const char *name, const QDict *args, Error **errp,
 QmpGuestCompletionFunc *cb, void *opaque)
 {
+if (!qmp_proxy_default) {
+/* TODO: should set errp here */
+fprintf(stderr, "qmp proxy: no guest proxy found\n");
+return;
+}
+qmp_proxy_send_request(qmp_proxy_default, name, args, errp, cb, opaque);
 }
diff --git a/qmp-core.h b/qmp-core.h
index b676020..114d290 100644
--- a/qmp-core.h
+++ b/qmp-core.h
@@ -4,6 +4,7 @@
 #include "monitor.h"
 #include "qmp-marshal-types.h"
 #include "error_int.h"
+#include "qmp-proxy-core.h"
 
 struct QmpCommandState
 {
@@ -85,11 +86,5 @@ int qmp_state_get_fd(QmpState *sess);
 }\
 } while(0)
 
-typedef void (QmpGuestCompletionFunc)(void *opaque, QObject *ret_data, Error 
*err);
-
-void qmp_guest_dispatch(const char *name, const QDict *args, Error **errp,
-QmpGuestCompletionFunc *cb, void *opaque);
-
-
 #endif
 
diff --git a/qmp-proxy-core.h b/qmp-proxy-core.h
new file mode 100644
index 000..47ac85d
--- /dev/null
+++ b/qmp-proxy-core.h
@@ -0,0 +1,20 @@
+#ifndef QMP_PROXY_CORE_H
+#define QMP_PROXY_CORE_H
+
+#define QMP_PROXY_PATH_DEFAULT "/tmp/qmp-proxy.sock"
+
+typedef void (QmpGuestCompletionFunc)(void *opaque, QObject *ret_data,
+  Error *err);
+
+void qmp_guest_dispatch(const char *name, const QDict *args, Error **errp,
+QmpGuestCompletionFunc *cb, void *opaque);
+
+typedef struct QmpProxy QmpProxy;
+
+void qmp_proxy_send_request(QmpProxy *p, const char *name,
+const QDict *args, Error **errp,
+QmpGuestCompletionFunc *cb, void *opaque);
+QmpProxy *qmp_proxy_new(const char *channel_path);
+void qmp_proxy_close(QmpProxy *p);
+
+#endif
diff --git a/qmp-proxy.c b/qmp-proxy.c
new file mode 100644
index 000..eaa6e6e
--- /dev/null
+++ b/qmp-proxy.c
@@ -0,0 +1,335 @@
+/*
+ * QMP definitions for communicating with guest agent
+ *
+ * Copyright IBM Corp. 2011
+ *
+ * Authors:
+ *  Michael Roth  
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ *
+ */
+
+#include "qmp.h"
+#include "qmp-core.h"
+#include "qemu-queue.h"
+#include "json-parser.h"
+#include "json-streamer.h"
+#include "qemu_socket.h"
+
+#define QMP_SENTINEL 0xFF
+
+typedef struct QmpProxyRequest {
+const char *name;
+const QDict *args;
+QmpGuestCompletionFunc *cb;
+void *opaque;
+QString *json;
+QTAILQ_ENTRY(QmpProxyRequest) entry;
+} QmpProxyRequest;
+
+typedef struct QmpProxyWriteState {
+QmpProxyRequest *current_request;
+const char *buf;
+size_t size;
+size_t pos;
+bool use_sentinel;
+} QmpProxyWriteState;
+
+typedef struct QmpProxyReadState {
+char *buf;
+size_t size;
+size_t pos;
+} QmpProxyReadState;
+
+struct QmpProxy {
+int fd;
+const char *path;
+QmpProxyWriteState write_state;
+QmpProxyReadState read_state;
+JSONMessageParser parser;
+QTAILQ_HEAD(, QmpProxyRequest) sent_requests;
+QTAILQ_HEAD(, QmpProxyRequest) queued_requests;
+QString *xport_event;
+QString *xport_event_sending;
+};
+
+static void qmp_proxy_read_handler(void *opaque);
+static void qmp_proxy_write_handler(void *opaque);
+
+static int qmp_proxy_cancel_request(QmpProxy *p, 

[Qemu-devel] [RFC][PATCH v1 06/12] qmp proxy: build qemu with guest proxy dependency

[Michael Roth]

Signed-off-by: Michael Roth 
---
 Makefile.objs |1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/Makefile.objs b/Makefile.objs
index df7e670..f143bd8 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -103,6 +103,7 @@ common-obj-y += block-migration.o
 common-obj-y += pflib.o
 common-obj-y += qmp-marshal.o qmp-marshal-types.o
 common-obj-y += qmp-marshal-types-core.o qmp-core.o
+common-obj-y += qmp-proxy.o
 common-obj-y += hmp.o
 
 common-obj-$(CONFIG_BRLAPI) += baum.o
-- 
1.7.0.4

[Qemu-devel] [RFC][PATCH v1 12/12] guest agent: build qemu-ga, add QEMU-wide gio dep

[Michael Roth]

This allows us to build qemu-ga with "make qemu-ga". It pulls in the
qemu-tools deps, but does not currently build by default. This may
change to avoid bitrot and help with host-side-only unit tests.

This also pulls in gio dependences for all of qemu, currently we only
pull in gthread. In general this brings in gio, gmodule, and gobject.

Can limit this to only the guest agent, but it's expected that all of
these will be needed as we start relying more on glib throughout qemu,
so leaving for now.

Signed-off-by: Michael Roth 
---
 Makefile  |5 -
 configure |6 +++---
 2 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/Makefile b/Makefile
index 6dc71a0..e8aa817 100644
--- a/Makefile
+++ b/Makefile
@@ -200,6 +200,7 @@ qcfg-opts.c: $(SRC_PATH)/qapi-schema.json 
$(SRC_PATH)/qmp-gen.py
 qmp/schema.py: $(SRC_PATH)/qapi-schema.json $(SRC_PATH)/qmp-gen.py
$(call quiet-command,python $(SRC_PATH)/qmp-gen.py --pybinding $@ < $<, 
"  GEN   $@")
 
+guest-agent.o: guest-agent.c guest-agent.h
 qmp-marshal.o: qmp-marshal.c qmp.h qapi-types.h
 qapi-types.o: qapi-types.c qapi-types.h
 libqmp.o: libqmp.c libqmp.h qapi-types.h
@@ -214,7 +215,9 @@ version-obj-$(CONFIG_WIN32) += version.o
 ##
 
 qemu-img.o: qemu-img-cmds.h
-qemu-img.o qemu-tool.o qemu-nbd.o qemu-io.o cmd.o: $(GENERATED_HEADERS)
+qemu-img.o qemu-tool.o qemu-nbd.o qemu-io.o cmd.o qemu-ga.o: 
$(GENERATED_HEADERS)
+
+qemu-ga$(EXESUF): qemu-ga.o qemu-tool.o qemu-error.o $(oslib-obj-y) 
$(trace-obj-y) $(block-obj-y) $(qobject-obj-y) $(version-obj-y) 
qemu-timer-common.o qemu-sockets.o guest-agent.o guest-agent-core.o 
qmp-marshal-types-core.o guest-agent-commands.c
 
 qemu-img$(EXESUF): qemu-img.o qemu-tool.o qemu-error.o $(oslib-obj-y) 
$(trace-obj-y) $(block-obj-y) $(qobject-obj-y) $(version-obj-y) 
qemu-timer-common.o
 
diff --git a/configure b/configure
index 1af6b44..608fa63 100755
--- a/configure
+++ b/configure
@@ -1659,9 +1659,9 @@ fi
 
 ##
 # glib support probe
-if $pkg_config --modversion gthread-2.0 > /dev/null 2>&1 ; then
-glib_cflags=`$pkg_config --cflags gthread-2.0 2>/dev/null`
-glib_libs=`$pkg_config --libs gthread-2.0 2>/dev/null`
+if $pkg_config --modversion gthread-2.0 gio-2.0 > /dev/null 2>&1 ; then
+glib_cflags=`$pkg_config --cflags gthread-2.0 gio-2.0 2>/dev/null`
+glib_libs=`$pkg_config --libs gthread-2.0 gio-2.0 2>/dev/null`
 libs_softmmu="$glib_libs $libs_softmmu"
 libs_tools="$glib_libs $libs_tools"
 else
-- 
1.7.0.4

[Qemu-devel] [RFC][PATCH v1 05/12] qapi: fix handling for null-return async callbacks

[Michael Roth]

Async commands like 'guest-ping' have NULL retvals. Handle these by
inserting an empty dictionary in the response's "return" field.

Signed-off-by: Michael Roth 
---
 qmp-core.c |5 -
 1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/qmp-core.c b/qmp-core.c
index e33f7a4..9f3d182 100644
--- a/qmp-core.c
+++ b/qmp-core.c
@@ -922,9 +922,12 @@ void qmp_async_complete_command(QmpCommandState *cmd, 
QObject *retval, Error *er
 rsp = qdict_new();
 if (err) {
 qdict_put_obj(rsp, "error", error_get_qobject(err));
-} else {
+} else if (retval) {
 qobject_incref(retval);
 qdict_put_obj(rsp, "return", retval);
+} else {
+/* add empty "return" dict, this is the standard for NULL returns */
+qdict_put_obj(rsp, "return", QOBJECT(qdict_new()));
 }
 if (cmd->tag) {
 qdict_put_obj(rsp, "tag", cmd->tag);
-- 
1.7.0.4

[Qemu-devel] [RFC][PATCH v1 02/12] json-streamer: add handling for JSON_ERROR token/state

[Michael Roth]

This allows a JSON_ERROR state to be passed to the streamer to force a
flush of the current tokens and pass a NULL token list to the parser
rather that have it churn on bad data. (Alternatively we could just not
pass it to the parser at all, but it may be useful to push there errors
up the stack. NULL token lists are not currently handled by the parser,
the next patch will address that)

Signed-off-by: Michael Roth 
---
 json-streamer.c |   35 +++
 1 files changed, 23 insertions(+), 12 deletions(-)

diff --git a/json-streamer.c b/json-streamer.c
index a6cb28f..659e3f0 100644
--- a/json-streamer.c
+++ b/json-streamer.c
@@ -56,29 +56,40 @@ static void json_message_process_token(JSONLexer *lexer, 
QString *token, JSONTok
 
 qlist_append(parser->tokens, dict);
 
-if (parser->brace_count < 0 ||
+if (type == JSON_ERROR) {
+goto out_emit_bad;
+} else if (parser->brace_count < 0 ||
 parser->bracket_count < 0 ||
 (parser->brace_count == 0 &&
  parser->bracket_count == 0)) {
-parser->brace_count = 0;
-parser->bracket_count = 0;
-parser->emit(parser, parser->tokens);
-QDECREF(parser->tokens);
-parser->tokens = qlist_new();
-parser->token_size = 0;
+goto out_emit;
 } else if (parser->token_size > MAX_TOKEN_SIZE ||
parser->bracket_count > MAX_NESTING ||
parser->brace_count > MAX_NESTING) {
 /* Security consideration, we limit total memory allocated per object
  * and the maximum recursion depth that a message can force.
  */
-parser->brace_count = 0;
-parser->bracket_count = 0;
-parser->emit(parser, parser->tokens);
+goto out_emit;
+}
+
+return;
+
+out_emit_bad:
+/* clear out token list and tell the parser to emit and error
+ * indication by passing it a NULL list
+ */ 
+QDECREF(parser->tokens);
+parser->tokens = NULL;
+out_emit:
+/* send current list of tokens to parser and reset tokenizer */
+parser->brace_count = 0;
+parser->bracket_count = 0;
+parser->emit(parser, parser->tokens);
+if (parser->tokens) {
 QDECREF(parser->tokens);
-parser->tokens = qlist_new();
-parser->token_size = 0;
 }
+parser->tokens = qlist_new();
+parser->token_size = 0;
 }
 
 void json_message_parser_init(JSONMessageParser *parser,
-- 
1.7.0.4

[Qemu-devel] [RFC][PATCH v1 04/12] qapi: fix function name typo in qmp-gen.py

[Michael Roth]

Signed-off-by: Michael Roth 
---
 qmp-gen.py |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/qmp-gen.py b/qmp-gen.py
index 90069ca..4164692 100644
--- a/qmp-gen.py
+++ b/qmp-gen.py
@@ -2328,7 +2328,7 @@ void qga_init_marshal(void)
 if not s.has_key('command'):
 continue
 name = s['command']
-if qmp_is_proxy_command(name):
+if qmp_is_proxy_cmd(name):
 ret += mcgen('''
 qga_register_command("%(name)s", &qmp_marshal_%(c_name)s);
 ''',
-- 
1.7.0.4

[Qemu-devel] [RFC][PATCH v1 08/12] qemu-char: add qmp_proxy chardev

[Michael Roth]

This allows qemu to be started with guest agent support via:

qemu -chardev qmp_proxy,path=,id=qmp_proxy \
  -device ...,chardev=qmp_proxy

It is essentially a wrapper for -chardev socket, which takes the extra
step of setting required defaults and initializing the qmp proxy by
passing the path argument along.

Not sure if this is the most elegant approach, but in terms of the
command-line invocation it seems to be the most consistent way to do
it.

Signed-off-by: Michael Roth 
---
 qemu-char.c |   46 ++
 1 files changed, 46 insertions(+), 0 deletions(-)

diff --git a/qemu-char.c b/qemu-char.c
index d301925..6ff7698 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -2275,6 +2275,51 @@ static CharDriverState *qemu_chr_open_socket(QemuOpts 
*opts)
 return NULL;
 }
 
+#include "qmp-proxy-core.h"
+
+extern QmpProxy *qmp_proxy_default;
+
+static CharDriverState *qemu_chr_open_qmp_proxy(QemuOpts *opts)
+{
+CharDriverState *chr;
+QmpProxy *p;
+const char *path;
+
+/* revert to/enforce default socket chardev options for qmp proxy */
+path = qemu_opt_get(opts, "path");
+if (path == NULL) {
+path = QMP_PROXY_PATH_DEFAULT;
+qemu_opt_set_qerr(opts, "path", path);
+}
+/* required options for qmp proxy */
+qemu_opt_set_qerr(opts, "server", "on");
+qemu_opt_set_qerr(opts, "wait", "off");
+qemu_opt_set_qerr(opts, "telnet", "off");
+
+chr = qemu_chr_open_socket(opts);
+if (chr == NULL) {
+goto err;
+}
+
+/* initialize virtagent using the socket we just set up */
+if (qmp_proxy_default) {
+fprintf(stderr, "error, multiple qmp guest proxies are not allowed\n");
+}
+p = qmp_proxy_new(path);
+if (p == NULL) {
+fprintf(stderr, "error initializing qmp guest proxy\n");
+goto err;
+}
+qmp_proxy_default = p;
+
+return chr;
+err:
+if (chr) {
+qemu_free(chr);
+}
+return NULL;
+}
+
 /***/
 /* Memory chardev */
 typedef struct {
@@ -2495,6 +2540,7 @@ static const struct {
 || defined(__FreeBSD_kernel__)
 { .name = "parport",   .open = qemu_chr_open_pp },
 #endif
+{ .name = "qmp_proxy", .open = qemu_chr_open_qmp_proxy },
 };
 
 CharDriverState *qemu_chr_open_opts(QemuOpts *opts,
-- 
1.7.0.4

[Qemu-devel] [RFC][PATCH v1 01/12] json-lexer: make lexer error-recovery more deterministic

[Michael Roth]

Currently when we reach an error state we effectively flush everything
fed to the lexer, which can put us in a state where we keep feeding
tokens into the parser at arbitrary offsets in the stream. This makes it
difficult for the lexer/tokenizer/parser to get back in sync when bad
input is made by the client.

With these changes we emit an error state/token up to the tokenizer as
soon as we reach an error state, and continue processing any data passed
in rather than bailing out. The reset token will be used to reset the
tokenizer and parser, such that they'll recover state as soon as the
lexer begins generating valid token sequences again.

We also map chr(0xFF) to an error state here, since it's an invalid
UTF-8 character. QMP guest proxy/agent use this to force a flush/reset
of previous input for reliable delivery of certain events, so also we
document that thoroughly here.

Signed-off-by: Michael Roth 
---
 json-lexer.c |   22 ++
 json-lexer.h |1 +
 2 files changed, 19 insertions(+), 4 deletions(-)

diff --git a/json-lexer.c b/json-lexer.c
index 3462c89..21aa03a 100644
--- a/json-lexer.c
+++ b/json-lexer.c
@@ -105,7 +105,7 @@ static const uint8_t json_lexer[][256] =  {
 ['u'] = IN_DQ_UCODE0,
 },
 [IN_DQ_STRING] = {
-[1 ... 0xFF] = IN_DQ_STRING,
+[1 ... 0xFE] = IN_DQ_STRING,
 ['\\'] = IN_DQ_STRING_ESCAPE,
 ['"'] = JSON_STRING,
 },
@@ -144,7 +144,7 @@ static const uint8_t json_lexer[][256] =  {
 ['u'] = IN_SQ_UCODE0,
 },
 [IN_SQ_STRING] = {
-[1 ... 0xFF] = IN_SQ_STRING,
+[1 ... 0xFE] = IN_SQ_STRING,
 ['\\'] = IN_SQ_STRING_ESCAPE,
 ['\''] = JSON_STRING,
 },
@@ -305,10 +305,25 @@ static int json_lexer_feed_char(JSONLexer *lexer, char ch)
 new_state = IN_START;
 break;
 case ERROR:
+/* XXX: To avoid having previous bad input leaving the parser in an
+ * unresponsive state where we consume unpredictable amounts of
+ * subsequent "good" input, percolate this error state up to the
+ * tokenizer/parser by forcing a NULL object to be emitted, then
+ * reset state.
+ *
+ * Also note that this handling is required for reliable channel
+ * negotiation between QMP and the guest agent, since chr(0xFF)
+ * is placed at the beginning of certain events to ensure proper
+ * delivery when the channel is in an unknown state. chr(0xFF) is
+ * never a valid ASCII/UTF-8 sequence, so this should reliably
+ * induce an error/flush state.
+ */
+lexer->emit(lexer, lexer->token, JSON_ERROR, lexer->x, lexer->y);
 QDECREF(lexer->token);
 lexer->token = qstring_new();
 new_state = IN_START;
-return -EINVAL;
+lexer->state = new_state;
+return 0;
 default:
 break;
 }
@@ -334,7 +349,6 @@ int json_lexer_feed(JSONLexer *lexer, const char *buffer, 
size_t size)
 
 for (i = 0; i < size; i++) {
 int err;
-
 err = json_lexer_feed_char(lexer, buffer[i]);
 if (err < 0) {
 return err;
diff --git a/json-lexer.h b/json-lexer.h
index 3b50c46..10bc0a7 100644
--- a/json-lexer.h
+++ b/json-lexer.h
@@ -25,6 +25,7 @@ typedef enum json_token_type {
 JSON_STRING,
 JSON_ESCAPE,
 JSON_SKIP,
+JSON_ERROR,
 } JSONTokenType;
 
 typedef struct JSONLexer JSONLexer;
-- 
1.7.0.4

[Qemu-devel] [RFC][PATCH v1 00/11] QEMU Guest Agent: QMP-based host/guest communication (virtagent)

[Michael Roth]

These apply on top of Anthony's glib tree, commit 
03d5927deb5e6baebaade1b4c8ff2428a85e125c currently, and can also be obtained 
from:
git://repo.or.cz/qemu/mdroth.git qga_v1

The QGA-specific patches are in pretty rough shape, and there are some 
outstanding issues that I'll note below. I just wanted to put the general 
approach out there for consideration. Patch-level comments/review are still 
much-appreciated though.

However, patches 1-5 are general json/QAPI-related fixes. Anthony, please 
consider pulling these into your glib tree. The json fix-ups may need further 
evaluation, but I'm confident they're at least an improvement. The QAPI ones 
are trivial fix-ups.

ISSUES/TODOS:

 - QMP's async callbacks expect the command results to be de-marshalled 
beforehand. This is completely infeasible to attempt outside of the code 
generator, so this is a big area that needs to be addressed. So for now, only 
the 'guest-ping' command works, since it has no return value. The dummy 
"guest-view-file" command will cause an error to be reported to the client.
 - qemu-ga guest daemon is currently not working over virtio-serial or 
isa-serial. This is probably an issue with how I'm using glib's io channel 
interfaces, still investigating. This means the only way to currently test is 
by invocing qemu-ga with "-c unix-listen -p ", then doing something 
like `socat /dev/ttyS0,raw,echo=0 unix-connect:`.
 - guest-view-file is a stub, and will be broken out into an open/read/close 
set of RPCs, possibly with a high-level interface built around those.

OVERVIEW

For a better overview of what these patches are meant to accomplish, please 
reference the RFC for virtagent:

http://comments.gmane.org/gmane.comp.emulators.qemu/96096

These patches integrate the previous virtagent guest agent work directly in 
QAPI/QMP to leverage it's auto-generated marshalling code. This has numerous 
benefits:

 - addresses previous concerns over relying on external libraries to handle 
data encapsulation
 - reduces the need for manual unmarshalling of requests/responses, which makes 
adding new RPCs much safer/less error-prone, as well as cutting down on 
redundant code
 - QAPI documentation aligns completely with guest-side RPC implementation
 - is Just Better (TM)

BUILD/USAGE

build:
  ./configure --target-list=x86_64-softmmu
  make
  make qemu-ga #should be built on|for target guest

start guest:
  qemu \
  -drive file=/home/mdroth/vm/rhel6_64_base.raw,snapshot=off,if=virtio \
  -net nic,model=virtio,macaddr=52:54:00:12:34:00 \
  -net tap,script=/etc/qemu-ifup \
  -vnc :1 -m 1024 --enable-kvm \
  -chardev socket,path=/tmp/mon-qmp.sock,server,nowait,id=mon-qmp \
  -qmp mon-qmp \
  -chardev qmp_proxy,path=/tmp/qmp-proxy2.sock,server,nowait,id=qmp_proxy \ 
  -device virtio-serial \
  -device virtserialport,chardev=qmp_proxy,name=qcg"

use guest agent:
  ./qemu-ga -h
  ./qemu-ga -c virtio-serial -p /dev/virtio-ports/qcg

start/use qmp:
  mdroth@illuin:~$ sudo socat unix-connect:/tmp/mon-qmp.sock readline
  {"QMP": {"version": {"qemu": {"micro": 50, "minor": 13, "major": 0}, 
"package": ""}, "capabilities": []}}
  {"execute":"guest-ping"}
  {"return": {}}

__

 Makefile   |5 +-
 Makefile.objs  |1 +
 configure  |6 +-
 guest-agent-commands.c |   24 +++
 guest-agent-core.c |  143 +
 guest-agent-core.h |   21 ++
 json-lexer.c   |   22 ++-
 json-lexer.h   |1 +
 json-parser.c  |6 +-
 json-streamer.c|   35 ++-
 qemu-char.c|   46 +
 qemu-ga.c  |  522 
 qmp-core.c |   13 +-
 qmp-core.h |7 +-
 qmp-gen.py |2 +-
 qmp-proxy-core.h   |   20 ++
 qmp-proxy.c|  335 +++
 vl.c   |1 +
 18 files changed, 1181 insertions(+), 29 deletions(-)

[Qemu-devel] [PATCH] m68k: Replace gen_im32() by tcg_const_i32()

[Laurent Vivier]

Signed-off-by: Laurent Vivier 
---
 target-m68k/translate.c |   43 ---
 1 files changed, 20 insertions(+), 23 deletions(-)

diff --git a/target-m68k/translate.c b/target-m68k/translate.c
index 6f72a2b..be56aaa 100644
--- a/target-m68k/translate.c
+++ b/target-m68k/translate.c
@@ -171,9 +171,6 @@ typedef void (*disas_proc)(DisasContext *, uint16_t);
   static void disas_##name (DisasContext *s, uint16_t insn)
 #endif
 
-/* FIXME: Remove this.  */
-#define gen_im32(val) tcg_const_i32(val)
-
 /* Generate a load from the specified address.  Narrow values are
sign extended to full register width.  */
 static inline TCGv gen_load(DisasContext * s, int opsize, TCGv addr, int sign)
@@ -339,7 +336,7 @@ static TCGv gen_lea_indexed(DisasContext *s, int opsize, 
TCGv base)
 if ((ext & 0x80) == 0) {
 /* base not suppressed */
 if (IS_NULL_QREG(base)) {
-base = gen_im32(offset + bd);
+base = tcg_const_i32(offset + bd);
 bd = 0;
 }
 if (!IS_NULL_QREG(add)) {
@@ -355,7 +352,7 @@ static TCGv gen_lea_indexed(DisasContext *s, int opsize, 
TCGv base)
 add = tmp;
 }
 } else {
-add = gen_im32(bd);
+add = tcg_const_i32(bd);
 }
 if ((ext & 3) != 0) {
 /* memory indirect */
@@ -536,15 +533,15 @@ static TCGv gen_lea(DisasContext *s, uint16_t insn, int 
opsize)
 case 0: /* Absolute short.  */
 offset = ldsw_code(s->pc);
 s->pc += 2;
-return gen_im32(offset);
+return tcg_const_i32(offset);
 case 1: /* Absolute long.  */
 offset = read_im32(s);
-return gen_im32(offset);
+return tcg_const_i32(offset);
 case 2: /* pc displacement  */
 offset = s->pc;
 offset += ldsw_code(s->pc);
 s->pc += 2;
-return gen_im32(offset);
+return tcg_const_i32(offset);
 case 3: /* pc index+displacement.  */
 return gen_lea_indexed(s, opsize, NULL_QREG);
 case 4: /* Immediate.  */
@@ -1209,16 +1206,16 @@ DISAS_INSN(arith_im)
 break;
 case 2: /* subi */
 tcg_gen_mov_i32(dest, src1);
-gen_helper_xflag_lt(QREG_CC_X, dest, gen_im32(im));
+gen_helper_xflag_lt(QREG_CC_X, dest, tcg_const_i32(im));
 tcg_gen_subi_i32(dest, dest, im);
-gen_update_cc_add(dest, gen_im32(im));
+gen_update_cc_add(dest, tcg_const_i32(im));
 s->cc_op = CC_OP_SUB;
 break;
 case 3: /* addi */
 tcg_gen_mov_i32(dest, src1);
 tcg_gen_addi_i32(dest, dest, im);
-gen_update_cc_add(dest, gen_im32(im));
-gen_helper_xflag_lt(QREG_CC_X, dest, gen_im32(im));
+gen_update_cc_add(dest, tcg_const_i32(im));
+gen_helper_xflag_lt(QREG_CC_X, dest, tcg_const_i32(im));
 s->cc_op = CC_OP_ADD;
 break;
 case 5: /* eori */
@@ -1228,7 +1225,7 @@ DISAS_INSN(arith_im)
 case 6: /* cmpi */
 tcg_gen_mov_i32(dest, src1);
 tcg_gen_subi_i32(dest, dest, im);
-gen_update_cc_add(dest, gen_im32(im));
+gen_update_cc_add(dest, tcg_const_i32(im));
 s->cc_op = CC_OP_SUB;
 break;
 default:
@@ -1324,8 +1321,8 @@ DISAS_INSN(clr)
 default:
 abort();
 }
-DEST_EA(insn, opsize, gen_im32(0), NULL);
-gen_logic_cc(s, gen_im32(0));
+DEST_EA(insn, opsize, tcg_const_i32(0), NULL);
+gen_logic_cc(s, tcg_const_i32(0));
 }
 
 static TCGv gen_get_ccr(DisasContext *s)
@@ -1589,7 +1586,7 @@ DISAS_INSN(jump)
 }
 if ((insn & 0x40) == 0) {
 /* jsr */
-gen_push(s, gen_im32(s->pc));
+gen_push(s, tcg_const_i32(s->pc));
 }
 gen_jmp(s, tmp);
 }
@@ -1617,7 +1614,7 @@ DISAS_INSN(addsubq)
 tcg_gen_addi_i32(dest, dest, val);
 }
 } else {
-src2 = gen_im32(val);
+src2 = tcg_const_i32(val);
 if (insn & 0x0100) {
 gen_helper_xflag_lt(QREG_CC_X, dest, src2);
 tcg_gen_subi_i32(dest, dest, val);
@@ -1666,7 +1663,7 @@ DISAS_INSN(branch)
 }
 if (op == 1) {
 /* bsr */
-gen_push(s, gen_im32(s->pc));
+gen_push(s, tcg_const_i32(s->pc));
 }
 gen_flush_cc_op(s);
 if (op > 1) {
@@ -1757,7 +1754,7 @@ DISAS_INSN(mov3q)
 val = (insn >> 9) & 7;
 if (val == 0)
 val = -1;
-src = gen_im32(val);
+src = tcg_const_i32(val);
 gen_logic_cc(s, src);
 DEST_EA(insn, OS_LONG, src, NULL);
 }
@@ -1883,7 +1880,7 @@ DISAS_INSN(shift_im)
 tmp = (insn >> 9) & 7;
 if (tmp == 0)
 tmp = 8;
-shift = gen_im32(tmp);
+shift = tcg_const_i32(tmp);
 /* No need to flush flags becuse we know we will set C flag.  */
 if (insn & 0x100) {
 gen_helper_shl_cc(reg, cpu_env, reg, shift);
@@ -2191,7 +2188,7 @@ DISAS_INSN(fpu)
 switch ((

Re: [Qemu-devel] [PATCH 27/27] Add SLOF-based partition firmware for pSeries machine, allowing more boot options

[Anthony Liguori]

On 03/24/2011 10:21 PM, David Gibson wrote:

Currently, the emulated pSeries machine requires the use of the
-kernel parameter in order to explicitly load a guest kernel.  This
means booting from the virtual disk, cdrom or network is not possible.

This patch addresses this limitation by inserting a within-partition
firmware image (derived from the "SLOF" free Open Firmware project).
If -kernel is not specified, qemu will now load the SLOF image, which
has access to the qemu boot device list through the device tree, and
can boot from any of the usual virtual devices.

In order to support the new firmware, an extension to the emulated
machine/hypervisor is necessary.  Unlike Linux, which expects
multi-CPU entry to be handled kexec() style, the SLOF firmware expects
only one CPU to be active at entry, and to use a hypervisor RTAS
method to enable the other CPUs one by one.

This patch also implements this 'start-cpu' method, so that SLOF can
start the secondary CPUs and marshal them into the kexec() holding
pattern ready for entry into the guest OS.  Linux should, and in the
future might directly use the start-cpu method to enable initially
disabled CPUs, but for now it does require kexec() entry.

Signed-off-by: Benjamin Herrenschmidt
Signed-off-by: Paul Mackerras
Signed-off-by: David Gibson


We should pull in SLOF via a git submodule.  That ensures we ship the 
source code along with the binary.


Regards,

Anthony Liguori

[Qemu-devel] [Bug 741887] Re: virsh snapshot-create too slow (kvm, qcow2, savevm)

[Serge Hallyn]

Confirmed that doing


  kvm -drive file=lucid.img,cache=unsafe,index=0,boot=on -m 512M -smp 2 -vnc :1 
-monitor stdio

and doing 'savevm savevm5'

takes about 2 seconds.

So, for fast savevm, 'cache=unsafe' is the workaround.  Shoudl this bug
then be marked invalid, or 'wontfix'?

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/741887

Title:
  virsh snapshot-create too slow (kvm, qcow2, savevm)

Status in QEMU:
  New
Status in “qemu-kvm” package in Ubuntu:
  Confirmed

Bug description:
  Action
  ==
  # time virsh snapshot-create 1

  * Taking snapshot of a running KVM virtual machine

  Result
  ==
  Domain snapshot 1300983161 created
  real4m46.994s
  user0m0.000s
  sys 0m0.010s

  Expected result
  ===
  * Snapshot taken after few seconds instead of minutes.

  Environment
  ===
  * Ubuntu Natty Narwhal upgraded from Lucid and Meerkat, fully updated.

  * Stock natty packages of libvirt and qemu installed (libvirt-bin
  0.8.8-1ubuntu5; libvirt0 0.8.8-1ubuntu5; qemu-common 0.14.0+noroms-
  0ubuntu3; qemu-kvm 0.14.0+noroms-0ubuntu3).

  * Virtual machine disk format is qcow2 (debian 5 installed)
  image: /storage/debian.qcow2
  file format: qcow2
  virtual size: 10G (10737418240 bytes)
  disk size: 1.2G
  cluster_size: 65536
  Snapshot list:
  IDTAG VM SIZEDATE   VM CLOCK
  1 snap01  48M 2011-03-24 09:46:33   00:00:58.899
  2 1300979368  58M 2011-03-24 11:09:28   00:01:03.589
  3 1300983161  57M 2011-03-24 12:12:41   00:00:51.905

  * qcow2 disk is stored on ext4 filesystem, without RAID or LVM or any
  special setup.

  * running guest VM takes about 40M RAM from inside, from outside 576M
  are given to that machine

  * host has fast dual-core pentium cpu with virtualization support,
  around 8G of RAM and 7200rpm harddrive (dd from urandom to file gives
  about 20M/s)

  * running processes: sshd, atd (empty), crond (empty), libvirtd, tmux,
  bash, rsyslogd, upstart-socket-bridge, udevd, dnsmasq, iotop (python)

  * networking is done by bridging and bonding

  
  Detail description
  ==

  * Under root, command 'virsh create-snapshot 1' is issued on booted
  and running KVM machine with debian inside.

  * After about four minutes, the process is done.

  * 'iotop' shows two 'kvm' processes reading/writing to disk. First one
  has IO around 1500 K/s, second one has around 400 K/s. That takes
  about three minutes. Then first process grabs about 3 M/s of IO and
  suddenly dissapears (1-2 sec). Then second process does about 7.5 M/s
  of IO for around a 1-2 minutes.

  * Snapshot is successfuly created and is usable for reverting or
  extracting.

  * Pretty much the same behaviour occurs when command 'savevm' is
  issued directly from qemu monitor, without using libvirt at all
  (actually, virsh snapshot-create just calls 'savevm' to the monitor
  socket).

  * This behaviour was observed on lucid, meerkat, natty and even with
  git version of libvirt (f44bfb7fb978c9313ce050a1c4149bf04aa0a670).
  Also slowsave packages from
  https://bugs.launchpad.net/ubuntu/+source/libvirt/+bug/524447 gave
  this issue.

  
  Thank you for helping to solve this issue!

  ProblemType: Bug
  DistroRelease: Ubuntu 11.04
  Package: libvirt-bin 0.8.8-1ubuntu5
  ProcVersionSignature: Ubuntu 2.6.38-7.38-server 2.6.38
  Uname: Linux 2.6.38-7-server x86_64
  Architecture: amd64
  Date: Thu Mar 24 12:19:41 2011
  InstallationMedia: Ubuntu-Server 10.04.2 LTS "Lucid Lynx" - Release amd64 
(20110211.1)
  ProcEnviron:
   LANG=en_US.UTF-8
   SHELL=/bin/bash
  SourcePackage: libvirt
  UpgradeStatus: No upgrade log present (probably fresh install)

[Qemu-devel] [Bug 741887] Re: virsh snapshot-create too slow (kvm, qcow2, savevm)

[Michael Tokarev]

savevm _is_ slow, because it's writing to a qcow2 file with full
(meta)data allocation which is terrible slow since 0.13 (and 0.12.5)
unless you use cache=unsafe.  It's the same slowdown as observed with
default cache mode when performing an operating system install into a
freshly created qcow2 - it may take several hours.  To verify, run
`iostat -dkx 5' and see how busy (the last column) your disk is during
the save - I suspect it'll be about 100%.

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/741887

Title:
  virsh snapshot-create too slow (kvm, qcow2, savevm)

Status in QEMU:
  New
Status in “qemu-kvm” package in Ubuntu:
  Confirmed

Bug description:
  Action
  ==
  # time virsh snapshot-create 1

  * Taking snapshot of a running KVM virtual machine

  Result
  ==
  Domain snapshot 1300983161 created
  real4m46.994s
  user0m0.000s
  sys 0m0.010s

  Expected result
  ===
  * Snapshot taken after few seconds instead of minutes.

  Environment
  ===
  * Ubuntu Natty Narwhal upgraded from Lucid and Meerkat, fully updated.

  * Stock natty packages of libvirt and qemu installed (libvirt-bin
  0.8.8-1ubuntu5; libvirt0 0.8.8-1ubuntu5; qemu-common 0.14.0+noroms-
  0ubuntu3; qemu-kvm 0.14.0+noroms-0ubuntu3).

  * Virtual machine disk format is qcow2 (debian 5 installed)
  image: /storage/debian.qcow2
  file format: qcow2
  virtual size: 10G (10737418240 bytes)
  disk size: 1.2G
  cluster_size: 65536
  Snapshot list:
  IDTAG VM SIZEDATE   VM CLOCK
  1 snap01  48M 2011-03-24 09:46:33   00:00:58.899
  2 1300979368  58M 2011-03-24 11:09:28   00:01:03.589
  3 1300983161  57M 2011-03-24 12:12:41   00:00:51.905

  * qcow2 disk is stored on ext4 filesystem, without RAID or LVM or any
  special setup.

  * running guest VM takes about 40M RAM from inside, from outside 576M
  are given to that machine

  * host has fast dual-core pentium cpu with virtualization support,
  around 8G of RAM and 7200rpm harddrive (dd from urandom to file gives
  about 20M/s)

  * running processes: sshd, atd (empty), crond (empty), libvirtd, tmux,
  bash, rsyslogd, upstart-socket-bridge, udevd, dnsmasq, iotop (python)

  * networking is done by bridging and bonding

  
  Detail description
  ==

  * Under root, command 'virsh create-snapshot 1' is issued on booted
  and running KVM machine with debian inside.

  * After about four minutes, the process is done.

  * 'iotop' shows two 'kvm' processes reading/writing to disk. First one
  has IO around 1500 K/s, second one has around 400 K/s. That takes
  about three minutes. Then first process grabs about 3 M/s of IO and
  suddenly dissapears (1-2 sec). Then second process does about 7.5 M/s
  of IO for around a 1-2 minutes.

  * Snapshot is successfuly created and is usable for reverting or
  extracting.

  * Pretty much the same behaviour occurs when command 'savevm' is
  issued directly from qemu monitor, without using libvirt at all
  (actually, virsh snapshot-create just calls 'savevm' to the monitor
  socket).

  * This behaviour was observed on lucid, meerkat, natty and even with
  git version of libvirt (f44bfb7fb978c9313ce050a1c4149bf04aa0a670).
  Also slowsave packages from
  https://bugs.launchpad.net/ubuntu/+source/libvirt/+bug/524447 gave
  this issue.

  
  Thank you for helping to solve this issue!

  ProblemType: Bug
  DistroRelease: Ubuntu 11.04
  Package: libvirt-bin 0.8.8-1ubuntu5
  ProcVersionSignature: Ubuntu 2.6.38-7.38-server 2.6.38
  Uname: Linux 2.6.38-7-server x86_64
  Architecture: amd64
  Date: Thu Mar 24 12:19:41 2011
  InstallationMedia: Ubuntu-Server 10.04.2 LTS "Lucid Lynx" - Release amd64 
(20110211.1)
  ProcEnviron:
   LANG=en_US.UTF-8
   SHELL=/bin/bash
  SourcePackage: libvirt
  UpgradeStatus: No upgrade log present (probably fresh install)

Re: [Qemu-devel] [PATCH 1/3] arm: basic support for ARMv4/ARMv4T emulation

[Peter Maydell]

On 24 March 2011 22:07, Dmitry Eremin-Solenikov  wrote:
> Currently target-arm/ assumes at least ARMv5 core. Add support for
> handling also ARMv4/ARMv4T. This changes the following instructions:

Mostly looks good; comments below.

> @@ -161,6 +179,8 @@ static void cpu_reset_model_id(CPUARMState *env, uint32_t 
> id)
>         break;
>     case ARM_CPUID_TI915T:
>     case ARM_CPUID_TI925T:
> +        set_feature(env, ARM_FEATURE_V4T);
> +        set_feature(env, ARM_FEATURE_V5);
>         set_feature(env, ARM_FEATURE_OMAPCP);
>         env->cp15.c0_cpuid = ARM_CPUID_TI925T; /* Depends on wiring.  */
>         env->cp15.c0_cachetype = 0x5109149;

As far as I can tell from google these are based on the ARM9TDMI
which means they're ARMv4T and so shouldn't have the V5 feature set.
(You can legitimately feel disgruntled that whoever added these didn't
do the v4T stuff properly :-))

> @@ -6129,6 +6131,7 @@ static void disas_arm_insn(CPUState * env, DisasContext 
> *s)
>                 }
>             }
>             /* Otherwise PLD; v5TE+ */
> +            ARCH(5);
>             return;
>         }
>         if (((insn & 0x0f70f000) == 0x0450f000) ||

Rather than adding ARCH() lines here and in some of the following
hunks it would be simpler to change the

if (cond == 0xf){
/* Unconditional instructions.  */

to:

if (cond == 0xf) {
 /* In ARMv3 and v4 the NV condition is UNPREDICTABLE; we
  * choose to UNDEF. In ARMv5 and above the space is used
  * for miscellaneous unconditional instructions.
  */
 ARCH(5);

Some bits that are missing from this patch:

You need to guard the Thumb BKPT and BLX decodes
with ARCH(5) as they're not in v4T.

The CPSR Q bit needs to RAZ/WI on v4 and v4T.

For v4 you need to make sure that the core can't get into
thumb mode at all. So feature guards in gen_bx_imm() and
gen_bx(), make sure PSR masks prevent the T bit getting set,
and check helper.c for anything that sets env->thumb from
somewhere else...

-- PMM

[Qemu-devel] [Bug 741887] Re: virsh snapshot-create too slow (kvm, qcow2, savevm)

[Serge Hallyn]

The current upstream qemu.git from git://git.savannah.nongnu.org/qemu.git
also has the slow savevm.  However, it's loadvm takes only a few seconds.


** Also affects: qemu
   Importance: Undecided
   Status: New

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/741887

Title:
  virsh snapshot-create too slow (kvm, qcow2, savevm)

Status in QEMU:
  New
Status in “qemu-kvm” package in Ubuntu:
  Confirmed

Bug description:
  Action
  ==
  # time virsh snapshot-create 1

  * Taking snapshot of a running KVM virtual machine

  Result
  ==
  Domain snapshot 1300983161 created
  real4m46.994s
  user0m0.000s
  sys 0m0.010s

  Expected result
  ===
  * Snapshot taken after few seconds instead of minutes.

  Environment
  ===
  * Ubuntu Natty Narwhal upgraded from Lucid and Meerkat, fully updated.

  * Stock natty packages of libvirt and qemu installed (libvirt-bin
  0.8.8-1ubuntu5; libvirt0 0.8.8-1ubuntu5; qemu-common 0.14.0+noroms-
  0ubuntu3; qemu-kvm 0.14.0+noroms-0ubuntu3).

  * Virtual machine disk format is qcow2 (debian 5 installed)
  image: /storage/debian.qcow2
  file format: qcow2
  virtual size: 10G (10737418240 bytes)
  disk size: 1.2G
  cluster_size: 65536
  Snapshot list:
  IDTAG VM SIZEDATE   VM CLOCK
  1 snap01  48M 2011-03-24 09:46:33   00:00:58.899
  2 1300979368  58M 2011-03-24 11:09:28   00:01:03.589
  3 1300983161  57M 2011-03-24 12:12:41   00:00:51.905

  * qcow2 disk is stored on ext4 filesystem, without RAID or LVM or any
  special setup.

  * running guest VM takes about 40M RAM from inside, from outside 576M
  are given to that machine

  * host has fast dual-core pentium cpu with virtualization support,
  around 8G of RAM and 7200rpm harddrive (dd from urandom to file gives
  about 20M/s)

  * running processes: sshd, atd (empty), crond (empty), libvirtd, tmux,
  bash, rsyslogd, upstart-socket-bridge, udevd, dnsmasq, iotop (python)

  * networking is done by bridging and bonding

  
  Detail description
  ==

  * Under root, command 'virsh create-snapshot 1' is issued on booted
  and running KVM machine with debian inside.

  * After about four minutes, the process is done.

  * 'iotop' shows two 'kvm' processes reading/writing to disk. First one
  has IO around 1500 K/s, second one has around 400 K/s. That takes
  about three minutes. Then first process grabs about 3 M/s of IO and
  suddenly dissapears (1-2 sec). Then second process does about 7.5 M/s
  of IO for around a 1-2 minutes.

  * Snapshot is successfuly created and is usable for reverting or
  extracting.

  * Pretty much the same behaviour occurs when command 'savevm' is
  issued directly from qemu monitor, without using libvirt at all
  (actually, virsh snapshot-create just calls 'savevm' to the monitor
  socket).

  * This behaviour was observed on lucid, meerkat, natty and even with
  git version of libvirt (f44bfb7fb978c9313ce050a1c4149bf04aa0a670).
  Also slowsave packages from
  https://bugs.launchpad.net/ubuntu/+source/libvirt/+bug/524447 gave
  this issue.

  
  Thank you for helping to solve this issue!

  ProblemType: Bug
  DistroRelease: Ubuntu 11.04
  Package: libvirt-bin 0.8.8-1ubuntu5
  ProcVersionSignature: Ubuntu 2.6.38-7.38-server 2.6.38
  Uname: Linux 2.6.38-7-server x86_64
  Architecture: amd64
  Date: Thu Mar 24 12:19:41 2011
  InstallationMedia: Ubuntu-Server 10.04.2 LTS "Lucid Lynx" - Release amd64 
(20110211.1)
  ProcEnviron:
   LANG=en_US.UTF-8
   SHELL=/bin/bash
  SourcePackage: libvirt
  UpgradeStatus: No upgrade log present (probably fresh install)

Re: [Qemu-devel] ARM: BKPT instructions should raise prefetch aborts with IFSR type 00010

[Alex Zuepke]

Hi Peter,

Peter Maydell schrieb:
> On 25 March 2011 10:54, Alex Zuepke  wrote:
>> while digging through some problems with BKPT exceptions on ARM, I
>> discovered that QEMU does not update IFSR on prefetch aborts. This
>> should be done since ARMv6 according to ARM docs. Please include.
> 
> This patch is the wrong approach to fixing this bug -- the
> updating of the IFSR needs to be done when the exception
> is taken, not when we translate the breakpoint instruction.

--- qemu-0.14.0.orig/target-arm/helper.c2011-02-16 15:44:05.0 
+0100
+++ qemu-0.14.0/target-arm/helper.c 2011-03-25 14:00:31.0 +0100
@@ -808,6 +808,8 @@ void do_interrupt(CPUARMState *env)
 return;
 }
 }
+/* indicate debug exception in IFSR */
+env->cp15.c5_insn = 2;
 /* Fall through to prefetch abort.  */
 case EXCP_PREFETCH_ABORT:
 new_mode = ARM_CPU_MODE_ABT;


Something like this? This neither looks good ...

> I'll put this on my todo list. If you happen to have a convenient
> test case demonstrating the problem, that would make a fix happen
> faster ;-)

Testcase is attached.

$ gunzip tc.elf.gz
$ qemu-system-arm.orig -nographic --cpu cortex-a8 -kernel tc.elf
testcase: IFSR undefined on QEMU
got prefetch abort, IFSR is 12345678
test: failed
HALT
Killed
$ qemu-system-arm.fixed -nographic --cpu cortex-a8 -kernel tc.elf
testcase: IFSR undefined on QEMU
got prefetch abort, IFSR is 0002
test: OK
HALT
Killed

Best Regards,
Alex

-- 
Alexander Zuepkeazue...@sysgo.com
SYSGO AG ~ Am Pfaffenstein 14 ~ 55270 Klein-Winternheim ~ Germany


tc.elf.gz
Description: application/gzip

Re: [Qemu-devel] [PATCH v2] severe memory leak caused by broken palette_destroy() function

[Anthony Liguori]

On 03/25/2011 04:31 AM, Stefan Hajnoczi wrote:

On Fri, Mar 25, 2011 at 8:45 AM, Ulrich Obergfell  wrote:

This is version 2 of the patch that I originally posted in:

http://lists.gnu.org/archive/html/qemu-devel/2011-03/msg02063.html

[Sorry, I missed to include the keyword 'PATCH' in the subject
  of the original post.]

The following commit breaks the code of the function palette_destroy().

http://git.kernel.org/?p=virt/kvm/qemu-kvm.git;a=commit;h=e31e3694afef58ba191cbcc6875ec243e5971268

The broken code causes a severe memory leak of 'VncPalette' structures
because it never frees anything:

 70 void palette_destroy(VncPalette *palette)
 71 {
 72 if (palette == NULL) {
 73 qemu_free(palette);
 74 }
 75 }

Version 2 of the patch calls qemu_free() unconditionally.

Signed-off-by: Ulrich Obergfell

Looks good.


Applied.  Thanks.

Regards,

Anthony Liguori


Reviewed-by: Stefan Hajnoczi

Re: [Qemu-devel] Compile errors in vl.c on git head

[Anthony Liguori]

On 03/25/2011 03:10 AM, Stefan Hajnoczi wrote:

On Fri, Mar 25, 2011 at 7:58 AM, Peter Maydell  wrote:

This is fixed by this patch:
http://patchwork.ozlabs.org/patch/88098/
sent Wednesday but hasn't made it into git yet.

(The patch got an ack from Jes although patchwork seems
to have lost it.)

CCed Anthony so this patch can be merged.


Applied.  Thanks.

Regards,

Anthony Liguori


Stefan

Re: [Qemu-devel] [PATCH 10/17] s390x: Adjust GDB stub

[Alexander Graf]

On 25.03.2011, at 13:07, Nathan Froyd wrote:

> On Thu, Mar 24, 2011 at 04:58:46PM +0100, Alexander Graf wrote:
>> We have successfully lazilized cc computation, so we need to manually
>> trigger its calculation when gdb wants to fetch it. We also changed the
>> variable name, so writing it writes into a different field now.
> 
> Wouldn't you want to:
> 
> a) change the variable name in the earlier commit and just do the
>   manual triggering in this commit, so as not to break bisect; or
> b) combine this change with the previous lazification?

I mostly did the variable name change so I can easily track all users of cc and 
adjust them accordingly.

The nice position we're in with S/390 is that nobody is building it currently. 
And even if they were, it would just break as compilation has been broken for 
quite a while now (ivshmem). So I basically assume that the target is only 
enabled for real bisecting as of the last patch which enables its configuration 
in the default target list.


Alex

Re: [Qemu-devel] [PATCH 10/17] s390x: Adjust GDB stub

[Nathan Froyd]

On Thu, Mar 24, 2011 at 04:58:46PM +0100, Alexander Graf wrote:
> We have successfully lazilized cc computation, so we need to manually
> trigger its calculation when gdb wants to fetch it. We also changed the
> variable name, so writing it writes into a different field now.

Wouldn't you want to:

a) change the variable name in the earlier commit and just do the
   manual triggering in this commit, so as not to break bisect; or
b) combine this change with the previous lazification?

-Nathan

Re: [Qemu-devel] [PATCHv3] report that QEMU process was killed by a signal

[Gleb Natapov]

Ping?

On Tue, Mar 15, 2011 at 01:56:04PM +0200, Gleb Natapov wrote:
> Currently when rogue script kills QEMU process (using TERM/INT/HUP
> signal) it looks indistinguishable from system shutdown. Lets report
> that QEMU was killed and leave some clues about the killer identity.
>   
> Signed-off-by: Gleb Natapov 
> ---
>   v1->v2:
> - print message from a main loop instead of signal handler
>   v2->v3
> - use pid_t to store pid instead of int 
> 
> diff --git a/os-posix.c b/os-posix.c
> index 38c29d1..36d937c 100644
> --- a/os-posix.c
> +++ b/os-posix.c
> @@ -61,9 +61,9 @@ void os_setup_early_signal_handling(void)
>  sigaction(SIGPIPE, &act, NULL);
>  }
>  
> -static void termsig_handler(int signal)
> +static void termsig_handler(int signal, siginfo_t *info, void *c)
>  {
> -qemu_system_shutdown_request();
> +qemu_system_killed(info->si_signo, info->si_pid);
>  }
>  
>  static void sigchld_handler(int signal)
> @@ -76,7 +76,8 @@ void os_setup_signal_handling(void)
>  struct sigaction act;
>  
>  memset(&act, 0, sizeof(act));
> -act.sa_handler = termsig_handler;
> +act.sa_sigaction = termsig_handler;
> +act.sa_flags = SA_SIGINFO;
>  sigaction(SIGINT,  &act, NULL);
>  sigaction(SIGHUP,  &act, NULL);
>  sigaction(SIGTERM, &act, NULL);
> diff --git a/sysemu.h b/sysemu.h
> index 0a83ab9..f868500 100644
> --- a/sysemu.h
> +++ b/sysemu.h
> @@ -66,6 +66,8 @@ void qemu_system_vmstop_request(int reason);
>  int qemu_shutdown_requested(void);
>  int qemu_reset_requested(void);
>  int qemu_powerdown_requested(void);
> +void qemu_system_killed(int signal, pid_t pid);
> +void qemu_kill_report(void);
>  extern qemu_irq qemu_system_powerdown;
>  void qemu_system_reset(void);
>  
> diff --git a/vl.c b/vl.c
> index 5e007a7..000c845 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -1213,7 +1213,8 @@ typedef struct QEMUResetEntry {
>  static QTAILQ_HEAD(reset_handlers, QEMUResetEntry) reset_handlers =
>  QTAILQ_HEAD_INITIALIZER(reset_handlers);
>  static int reset_requested;
> -static int shutdown_requested;
> +static int shutdown_requested, shutdown_signal = -1;
> +static pid_t shutdown_pid;
>  static int powerdown_requested;
>  static int debug_requested;
>  static int vmstop_requested;
> @@ -1225,6 +1226,15 @@ int qemu_shutdown_requested(void)
>  return r;
>  }
>  
> +void qemu_kill_report(void)
> +{
> +if (shutdown_signal != -1) {
> +fprintf(stderr, "Got signal %d from pid %d\n",
> + shutdown_signal, shutdown_pid);
> +shutdown_signal = -1;
> +}
> +}
> +
>  int qemu_reset_requested(void)
>  {
>  int r = reset_requested;
> @@ -1298,6 +1308,13 @@ void qemu_system_reset_request(void)
>  qemu_notify_event();
>  }
>  
> +void qemu_system_killed(int signal, pid_t pid)
> +{
> +shutdown_signal = signal;
> +shutdown_pid = pid;
> +qemu_system_shutdown_request();
> +}
> +
>  void qemu_system_shutdown_request(void)
>  {
>  shutdown_requested = 1;
> @@ -1441,6 +1458,7 @@ static void main_loop(void)
>  vm_stop(VMSTOP_DEBUG);
>  }
>  if (qemu_shutdown_requested()) {
> +qemu_kill_report();
>  monitor_protocol_event(QEVENT_SHUTDOWN, NULL);
>  if (no_shutdown) {
>  vm_stop(VMSTOP_SHUTDOWN);
> --
>   Gleb.

--
Gleb.

[Qemu-devel] Re: [PATCH] floppy: save and restore DIR register

[Paolo Bonzini]

On 03/25/2011 07:27 AM, Jason Wang wrote:

We need to keep DIR register unchanged across migration, but currently it
depends on the media_changed flags from block layer and we do not save/restore
it which could let the guest driver think the floppy have changed after
migration. To fix this, a new filed media_changed in FDrive strcutre was
introduced in order to save and restore the it from block layer through
pre_save/post_load callbacks.


I guess you can avoid saving if the media changed flag is zero, too 
(which should be the common case after the guest has booted, right?).


Paolo

Re: [Qemu-devel] ARM: BKPT instructions should raise prefetch aborts with IFSR type 00010

[Peter Maydell]

On 25 March 2011 10:54, Alex Zuepke  wrote:
> while digging through some problems with BKPT exceptions on ARM, I
> discovered that QEMU does not update IFSR on prefetch aborts. This
> should be done since ARMv6 according to ARM docs. Please include.

This patch is the wrong approach to fixing this bug -- the
updating of the IFSR needs to be done when the exception
is taken, not when we translate the breakpoint instruction.

I'll put this on my todo list. If you happen to have a convenient
test case demonstrating the problem, that would make a fix happen
faster ;-)

-- PMM

[Qemu-devel] Re: [PATCH, RFC] virtio_blk: add cache control support

[Rusty Russell]

On Thu, 24 Mar 2011 10:54:05 +0100, Christian Borntraeger 
 wrote:
> Am 24.03.2011 04:05, schrieb Anthony Liguori:
> >> ie. lguest and S/390 don't trap writes to config space.
> >>
> >> Or perhaps they should?  But we should be explicit about needing it...
> > I don't think we ever operated on the assumption that config space writes 
> > would trap.
> > 
> > I don't think adding it is the right thing either because you can do byte 
> > access to the config space which makes atomicity difficult.
> 
> There is the additional problem, that s390 has no MMIO and,therefore,
> there is no real HW support for trapping writes to an area. You can
> use page faults, or read-only faults on newer systems, but this is 
> expensive. In addition, page faults only deliver the page frame, but
> not the offset within a page.

That's not *really* a problem, since you have control over the
config_set operation and could do whatever you wanted.

But I wanted to make sure we're all on the same page: you *can't* rely
on the host knowing immediately what you write to the config space.  If
you want that, an actual queued request is necessary...

Thanks,
Rusty.

[Qemu-devel] [PATCH 1/4] acpi, acpi_piix, vt82c686: factor out PM_TMR logic

[Isaku Yamahata]

factor out PM_TMR logic. Later This will be used by ich9 acpi.
Also fixes the same bug in vt82c686.c that was fixed by the following
commits.

> commit 055479feab63607b8042bb8ebb2e0523f17cbc4e
> Author: aliguori 
> Date:   Wed Jan 21 16:31:20 2009 +
>
> Always return latest pmsts instead of the old one (Xiantao Zhang)
>
> It may lead to the issue when booting windows guests with acpi=1
> if return the old pmsts.
>
> Signed-off-by: Xiantao Zhang 
> Signed-off-by: Anthony Liguori 

Cc: Blue Swirl 
Cc: Huacai Chen 
Cc: Aurelien Jarno 
Signed-off-by: Isaku Yamahata 
---
Change v8 -> v9:
- qemu_get_clock_ns() and so on.

Changes v7 -> v8:
- vt82c686.c
---
 hw/acpi.c   |   45 +
 hw/acpi.h   |   24 
 hw/acpi_piix4.c |   45 -
 hw/vt82c686.c   |   47 +++
 4 files changed, 96 insertions(+), 65 deletions(-)

diff --git a/hw/acpi.c b/hw/acpi.c
index 8071e7b..08cb126 100644
--- a/hw/acpi.c
+++ b/hw/acpi.c
@@ -197,3 +197,48 @@ out:
 }
 return -1;
 }
+
+/* ACPI PM_TMR */
+void acpi_pm_tmr_update(ACPIPMTimer *tmr, bool enable)
+{
+int64_t expire_time;
+
+/* schedule a timer interruption if needed */
+if (enable) {
+expire_time = muldiv64(tmr->overflow_time, get_ticks_per_sec(),
+   PM_TIMER_FREQUENCY);
+qemu_mod_timer(tmr->timer, expire_time);
+} else {
+qemu_del_timer(tmr->timer);
+}
+}
+
+void acpi_pm_tmr_calc_overflow_time(ACPIPMTimer *tmr)
+{
+int64_t d = acpi_pm_tmr_get_clock();
+tmr->overflow_time = (d + 0x80LL) & ~0x7fLL;
+}
+
+uint32_t acpi_pm_tmr_get(ACPIPMTimer *tmr)
+{
+uint32_t d = acpi_pm_tmr_get_clock();;
+return d & 0xff;
+}
+
+static void acpi_pm_tmr_timer(void *opaque)
+{
+ACPIPMTimer *tmr = opaque;
+tmr->update_sci(tmr);
+}
+
+void acpi_pm_tmr_init(ACPIPMTimer *tmr, acpi_update_sci_fn update_sci)
+{
+tmr->update_sci = update_sci;
+tmr->timer = qemu_new_timer_ns(vm_clock, acpi_pm_tmr_timer, tmr);
+}
+
+void acpi_pm_tmr_reset(ACPIPMTimer *tmr)
+{
+tmr->overflow_time = 0;
+qemu_del_timer(tmr->timer);
+}
diff --git a/hw/acpi.h b/hw/acpi.h
index 5949958..fc42501 100644
--- a/hw/acpi.h
+++ b/hw/acpi.h
@@ -74,5 +74,29 @@
 #define ACPI_BITMASK_ARB_DISABLE0x0001
 
 /* PM_TMR */
+struct ACPIPMTimer;
+typedef struct ACPIPMTimer ACPIPMTimer;
+
+typedef void (*acpi_update_sci_fn)(ACPIPMTimer *tmr);
+
+struct ACPIPMTimer {
+QEMUTimer *timer;
+int64_t overflow_time;
+
+acpi_update_sci_fn update_sci;
+};
+
+void acpi_pm_tmr_update(ACPIPMTimer *tmr, bool enable);
+void acpi_pm_tmr_calc_overflow_time(ACPIPMTimer *tmr);
+uint32_t acpi_pm_tmr_get(ACPIPMTimer *tmr);
+void acpi_pm_tmr_init(ACPIPMTimer *tmr, acpi_update_sci_fn update_sci);
+void acpi_pm_tmr_reset(ACPIPMTimer *tmr);
+
+#include "qemu-timer.h"
+static inline int64_t acpi_pm_tmr_get_clock(void)
+{
+return muldiv64(qemu_get_clock_ns(vm_clock), PM_TIMER_FREQUENCY,
+get_ticks_per_sec());
+}
 
 #endif /* !QEMU_HW_ACPI_H */
diff --git a/hw/acpi_piix4.c b/hw/acpi_piix4.c
index 0b2bc97..d5f631a 100644
--- a/hw/acpi_piix4.c
+++ b/hw/acpi_piix4.c
@@ -60,8 +60,7 @@ typedef struct PIIX4PMState {
 
 APMState apm;
 
-QEMUTimer *tmr_timer;
-int64_t tmr_overflow_time;
+ACPIPMTimer tmr;
 
 PMSMBus smb;
 uint32_t smb_io_base;
@@ -82,20 +81,10 @@ static void piix4_acpi_system_hot_add_init(PCIBus *bus, 
PIIX4PMState *s);
 #define ACPI_ENABLE 0xf1
 #define ACPI_DISABLE 0xf0
 
-static uint32_t get_pmtmr(PIIX4PMState *s)
-{
-uint32_t d;
-d = muldiv64(qemu_get_clock_ns(vm_clock), PM_TIMER_FREQUENCY, 
get_ticks_per_sec());
-return d & 0xff;
-}
-
 static int get_pmsts(PIIX4PMState *s)
 {
-int64_t d;
-
-d = muldiv64(qemu_get_clock_ns(vm_clock), PM_TIMER_FREQUENCY,
- get_ticks_per_sec());
-if (d >= s->tmr_overflow_time)
+int64_t d = acpi_pm_tmr_get_clock();
+if (d >= s->tmr.overflow_time)
 s->pmsts |= ACPI_BITMASK_TIMER_STATUS;
 return s->pmsts;
 }
@@ -103,7 +92,6 @@ static int get_pmsts(PIIX4PMState *s)
 static void pm_update_sci(PIIX4PMState *s)
 {
 int sci_level, pmsts;
-int64_t expire_time;
 
 pmsts = get_pmsts(s);
 sci_level = (((pmsts & s->pmen) &
@@ -115,19 +103,13 @@ static void pm_update_sci(PIIX4PMState *s)
 
 qemu_set_irq(s->irq, sci_level);
 /* schedule a timer interruption if needed */
-if ((s->pmen & ACPI_BITMASK_TIMER_ENABLE) &&
-!(pmsts & ACPI_BITMASK_TIMER_STATUS)) {
-expire_time = muldiv64(s->tmr_overflow_time, get_ticks_per_sec(),
-   PM_TIMER_FREQUENCY);
-qemu_mod_timer(s->tmr_timer, expire_time);
-} else {
-qemu_del_timer(s->tmr_timer);
-}
+acpi_pm_tmr_update(&s->tmr, (s->pmen & ACPI_BITMASK_TIMER_ENABLE) &&
+  

[Qemu-devel] Re: [PATCH 01/17] Only build ivshmem when CONFIG_PCI && CONFIG_KVM

[Alexander Graf]

On 24.03.2011, at 22:46, Juan Quintela wrote:

> Alexander Graf  wrote:
>> The ivshmem depends on PCI and KVM, not only KVM. Reflect this
>> in the Makefile, so we don't get build errors on s390x.
>> 
>> Signed-off-by: Alexander Graf 
>> ---
>> Makefile.target |8 +++-
>> 1 files changed, 7 insertions(+), 1 deletions(-)
>> 
>> diff --git a/Makefile.target b/Makefile.target
>> index f0df98e..17ad396 100644
>> --- a/Makefile.target
>> +++ b/Makefile.target
>> @@ -209,7 +209,13 @@ QEMU_CFLAGS += $(VNC_PNG_CFLAGS)
>> obj-$(CONFIG_XEN) += xen_machine_pv.o xen_domainbuild.o
>> 
>> # Inter-VM PCI shared memory
>> -obj-$(CONFIG_KVM) += ivshmem.o
>> +CONFIG_IVSHMEM =
>> +ifeq ($(CONFIG_KVM), y)
>> +  ifeq ($(CONFIG_PCI), y)
>> +CONFIG_IVSHMEM = y
>> +  endif
>> +endif
>> +obj-$(CONFIG_IVSHMEM) += ivshmem.o
> 
> This shouldn't be here.  Proper place is at ./configure, or better yet
> at defaults/x86_64-softmmu.mak
> 
> CONFIG_IVSHMEM=y
> 
> It is complicated though, because we depend on PCI and KVM.

So what would you recommend? The current dependency is just plain wrong :).


Alex

[Qemu-devel] [PATCH 3/4] acpi, acpi_piix, vt82c686: factor out PM1_CNT logic

[Isaku Yamahata]

factor out ACPI PM1_CNT logic. This will be used by ich9 acpi.

Cc: Blue Swirl 
Cc: Huacai Chen 
Cc: Aurelien Jarno 
Signed-off-by: Isaku Yamahata 
---
Changes v7 -> v8:
- vt82c686
---
 hw/acpi.c   |   49 +
 hw/acpi.h   |   14 ++
 hw/acpi_piix4.c |   40 ++--
 hw/vt82c686.c   |   23 +--
 4 files changed, 74 insertions(+), 52 deletions(-)

diff --git a/hw/acpi.c b/hw/acpi.c
index 07283be..2879eea 100644
--- a/hw/acpi.c
+++ b/hw/acpi.c
@@ -279,3 +279,52 @@ void acpi_pm_tmr_reset(ACPIPMTimer *tmr)
 tmr->overflow_time = 0;
 qemu_del_timer(tmr->timer);
 }
+
+/* ACPI PM1aCNT */
+void acpi_pm1_cnt_init(ACPIPM1CNT *pm1_cnt, qemu_irq cmos_s3)
+{
+pm1_cnt->cmos_s3 = cmos_s3;
+}
+
+void acpi_pm1_cnt_write(ACPIPM1EVT *pm1a, ACPIPM1CNT *pm1_cnt, uint16_t val)
+{
+pm1_cnt->cnt = val & ~(ACPI_BITMASK_SLEEP_ENABLE);
+
+if (val & ACPI_BITMASK_SLEEP_ENABLE) {
+/* change suspend type */
+uint16_t sus_typ = (val >> 10) & 7;
+switch(sus_typ) {
+case 0: /* soft power off */
+qemu_system_shutdown_request();
+break;
+case 1:
+/* ACPI_BITMASK_WAKE_STATUS should be set on resume.
+   Pretend that resume was caused by power button */
+pm1a->sts |=
+(ACPI_BITMASK_WAKE_STATUS | ACPI_BITMASK_POWER_BUTTON_STATUS);
+qemu_system_reset_request();
+qemu_irq_raise(pm1_cnt->cmos_s3);
+default:
+break;
+}
+}
+}
+
+void acpi_pm1_cnt_update(ACPIPM1CNT *pm1_cnt,
+ bool sci_enable, bool sci_disable)
+{
+/* ACPI specs 3.0, 4.7.2.5 */
+if (sci_enable) {
+pm1_cnt->cnt |= ACPI_BITMASK_SCI_ENABLE;
+} else if (sci_disable) {
+pm1_cnt->cnt &= ~ACPI_BITMASK_SCI_ENABLE;
+}
+}
+
+void acpi_pm1_cnt_reset(ACPIPM1CNT *pm1_cnt)
+{
+pm1_cnt->cnt = 0;
+if (pm1_cnt->cmos_s3) {
+qemu_irq_lower(pm1_cnt->cmos_s3);
+}
+}
diff --git a/hw/acpi.h b/hw/acpi.h
index c286d7d..836459e 100644
--- a/hw/acpi.h
+++ b/hw/acpi.h
@@ -112,4 +112,18 @@ void acpi_pm1_evt_write_sts(ACPIPM1EVT *pm1, ACPIPMTimer 
*tmr, uint16_t val);
 void acpi_pm1_evt_power_down(ACPIPM1EVT *pm1, ACPIPMTimer *tmr);
 void acpi_pm1_evt_reset(ACPIPM1EVT *pm1);
 
+/* PM1a_CNT: piix and ich9 don't implement PM1b CNT. */
+struct ACPIPM1CNT {
+uint16_t cnt;
+
+qemu_irq cmos_s3;
+};
+typedef struct ACPIPM1CNT ACPIPM1CNT;
+
+void acpi_pm1_cnt_init(ACPIPM1CNT *pm1_cnt, qemu_irq cmos_s3);
+void acpi_pm1_cnt_write(ACPIPM1EVT *pm1a, ACPIPM1CNT *pm1_cnt, uint16_t val);
+void acpi_pm1_cnt_update(ACPIPM1CNT *pm1_cnt,
+ bool sci_enable, bool sci_disable);
+void acpi_pm1_cnt_reset(ACPIPM1CNT *pm1_cnt);
+
 #endif /* !QEMU_HW_ACPI_H */
diff --git a/hw/acpi_piix4.c b/hw/acpi_piix4.c
index 5b4eef5..ce86ee3 100644
--- a/hw/acpi_piix4.c
+++ b/hw/acpi_piix4.c
@@ -55,7 +55,7 @@ typedef struct PIIX4PMState {
 PCIDevice dev;
 IORange ioport;
 ACPIPM1EVT pm1a;
-uint16_t pmcntrl;
+ACPIPM1CNT pm1_cnt;
 
 APMState apm;
 
@@ -65,7 +65,6 @@ typedef struct PIIX4PMState {
 uint32_t smb_io_base;
 
 qemu_irq irq;
-qemu_irq cmos_s3;
 qemu_irq smi_irq;
 int kvm_enabled;
 
@@ -124,30 +123,7 @@ static void pm_ioport_write(IORange *ioport, uint64_t 
addr, unsigned width,
 pm_update_sci(s);
 break;
 case 0x04:
-{
-int sus_typ;
-s->pmcntrl = val & ~(ACPI_BITMASK_SLEEP_ENABLE);
-if (val & ACPI_BITMASK_SLEEP_ENABLE) {
-/* change suspend type */
-sus_typ = (val >> 10) & 7;
-switch(sus_typ) {
-case 0: /* soft power off */
-qemu_system_shutdown_request();
-break;
-case 1:
-/* ACPI_BITMASK_WAKE_STATUS should be set on resume.
-   Pretend that resume was caused by power button */
-s->pm1a.sts |= (ACPI_BITMASK_WAKE_STATUS |
-ACPI_BITMASK_POWER_BUTTON_STATUS);
-qemu_system_reset_request();
-if (s->cmos_s3) {
-qemu_irq_raise(s->cmos_s3);
-}
-default:
-break;
-}
-}
-}
+acpi_pm1_cnt_write(&s->pm1a, &s->pm1_cnt, val);
 break;
 default:
 break;
@@ -169,7 +145,7 @@ static void pm_ioport_read(IORange *ioport, uint64_t addr, 
unsigned width,
 val = s->pm1a.en;
 break;
 case 0x04:
-val = s->pmcntrl;
+val = s->pm1_cnt.cnt;
 break;
 case 0x08:
 val = acpi_pm_tmr_get(&s->tmr);
@@ -192,11 +168,7 @@ static void apm_ctrl_changed(uint32_t val, void *arg)
 PIIX4PMState *s = arg;
 
 /* ACPI specs 3.

[Qemu-devel] [PATCH 4/4] acpi, acpi_piix: factor out GPE logic

[Isaku Yamahata]

factor out ACPI GPE logic. Later it will be used by ICH9 ACPI.

Signed-off-by: Isaku Yamahata 
---
 hw/acpi.c   |   66 ++
 hw/acpi.h   |   17 ++
 hw/acpi_piix4.c |   95 ++
 3 files changed, 108 insertions(+), 70 deletions(-)

diff --git a/hw/acpi.c b/hw/acpi.c
index 2879eea..e372474 100644
--- a/hw/acpi.c
+++ b/hw/acpi.c
@@ -328,3 +328,69 @@ void acpi_pm1_cnt_reset(ACPIPM1CNT *pm1_cnt)
 qemu_irq_lower(pm1_cnt->cmos_s3);
 }
 }
+
+/* ACPI GPE */
+void acpi_gpe_init(ACPIGPE *gpe, uint8_t len)
+{
+gpe->len = len;
+gpe->sts = qemu_mallocz(len / 2);
+gpe->en = qemu_mallocz(len / 2);
+}
+
+void acpi_gpe_blk(ACPIGPE *gpe, uint32_t blk)
+{
+gpe->blk = blk;
+}
+
+void acpi_gpe_reset(ACPIGPE *gpe)
+{
+memset(gpe->sts, 0, gpe->len / 2);
+memset(gpe->en, 0, gpe->len / 2);
+}
+
+static uint8_t *acpi_gpe_ioport_get_ptr(ACPIGPE *gpe, uint32_t addr)
+{
+uint8_t *cur = NULL;
+
+if (addr < gpe->len / 2) {
+cur = gpe->sts + addr;
+} else if (addr < gpe->len) {
+cur = gpe->en + addr;
+} else {
+abort();
+}
+
+return cur;
+}
+
+void acpi_gpe_ioport_writeb(ACPIGPE *gpe, uint32_t addr, uint32_t val)
+{
+uint8_t *cur;
+
+addr -= gpe->blk;
+cur = acpi_gpe_ioport_get_ptr(gpe, addr);
+if (addr < gpe->len / 2) {
+/* GPE_STS */
+*cur = (*cur) & ~val;
+} else if (addr < gpe->len) {
+/* GPE_EN */
+*cur = val;
+} else {
+abort();
+}
+}
+
+uint32_t acpi_gpe_ioport_readb(ACPIGPE *gpe, uint32_t addr)
+{
+uint8_t *cur;
+uint32_t val;
+
+addr -= gpe->blk;
+cur = acpi_gpe_ioport_get_ptr(gpe, addr);
+val = 0;
+if (cur != NULL) {
+val = *cur;
+}
+
+return val;
+}
diff --git a/hw/acpi.h b/hw/acpi.h
index 836459e..c141e65 100644
--- a/hw/acpi.h
+++ b/hw/acpi.h
@@ -126,4 +126,21 @@ void acpi_pm1_cnt_update(ACPIPM1CNT *pm1_cnt,
  bool sci_enable, bool sci_disable);
 void acpi_pm1_cnt_reset(ACPIPM1CNT *pm1_cnt);
 
+/* GPE0 */
+struct ACPIGPE {
+uint32_t blk;
+uint8_t len;
+
+uint8_t *sts;
+uint8_t *en;
+};
+typedef struct ACPIGPE ACPIGPE;
+
+void acpi_gpe_init(ACPIGPE *gpe, uint8_t len);
+void acpi_gpe_blk(ACPIGPE *gpe, uint32_t blk);
+void acpi_gpe_reset(ACPIGPE *gpe);
+
+void acpi_gpe_ioport_writeb(ACPIGPE *gpe, uint32_t addr, uint32_t val);
+uint32_t acpi_gpe_ioport_readb(ACPIGPE *gpe, uint32_t addr);
+
 #endif /* !QEMU_HW_ACPI_H */
diff --git a/hw/acpi_piix4.c b/hw/acpi_piix4.c
index ce86ee3..a60eb57 100644
--- a/hw/acpi_piix4.c
+++ b/hw/acpi_piix4.c
@@ -35,17 +35,13 @@
 #define ACPI_DBG_IO_ADDR  0xb044
 
 #define GPE_BASE 0xafe0
+#define GPE_LEN 4
 #define PCI_BASE 0xae00
 #define PCI_EJ_BASE 0xae08
 #define PCI_RMV_BASE 0xae0c
 
 #define PIIX4_PCI_HOTPLUG_STATUS 2
 
-struct gpe_regs {
-uint16_t sts; /* status */
-uint16_t en;  /* enabled */
-};
-
 struct pci_status {
 uint32_t up;
 uint32_t down;
@@ -69,7 +65,7 @@ typedef struct PIIX4PMState {
 int kvm_enabled;
 
 /* for pci hotplug */
-struct gpe_regs gpe;
+ACPIGPE gpe;
 struct pci_status pci0_status;
 uint32_t pci0_hotplug_enable;
 } PIIX4PMState;
@@ -89,7 +85,7 @@ static void pm_update_sci(PIIX4PMState *s)
ACPI_BITMASK_POWER_BUTTON_ENABLE |
ACPI_BITMASK_GLOBAL_LOCK_ENABLE |
ACPI_BITMASK_TIMER_ENABLE)) != 0) ||
-(((s->gpe.sts & s->gpe.en) & PIIX4_PCI_HOTPLUG_STATUS) != 0);
+(((s->gpe.sts[0] & s->gpe.en[0]) & PIIX4_PCI_HOTPLUG_STATUS) != 0);
 
 qemu_set_irq(s->irq, sci_level);
 /* schedule a timer interruption if needed */
@@ -213,14 +209,25 @@ static int vmstate_acpi_post_load(void *opaque, int 
version_id)
 return 0;
 }
 
+#define VMSTATE_GPE_ARRAY(_field, _state)\
+ {   \
+ .name   = (stringify(_field)),  \
+ .version_id = 0,\
+ .num= GPE_LEN,  \
+ .info   = &vmstate_info_uint16, \
+ .size   = sizeof(uint16_t), \
+ .flags  = VMS_ARRAY | VMS_POINTER,  \
+ .offset = vmstate_offset_pointer(_state, _field, uint8_t),  \
+ }
+
 static const VMStateDescription vmstate_gpe = {
 .name = "gpe",
 .version_id = 1,
 .minimum_version_id = 1,
 .minimum_version_id_old = 1,
 .fields  = (VMStateField []) {
-VMSTATE_UINT16(sts, struct gpe_regs),
-VMSTATE_UINT16(en, struct gpe_regs),
+VMSTATE_GPE_ARRAY(sts, ACPIGPE),
+VMSTATE_GPE_ARRAY(en, ACPIGPE),
 VMSTATE_END_OF_LIST()
 }
 };
@@ -251,7 +258,7 @@ static const VMStateDescription vmstate_acpi = {
 

[Qemu-devel] [PATCH 2/4] acpi, acpi_piix, vt82c686: factor out PM1a EVT logic

[Isaku Yamahata]

factor out ACPI PM1a EVT logic.
Later this will be used by ich9 acpi.

Cc: Blue Swirl 
Cc: Huacai Chen 
Cc: Aurelien Jarno 
Signed-off-by: Isaku Yamahata 
---
Changes v7 -> v8:
- vt82c686
---
 hw/acpi.c   |   37 +
 hw/acpi.h   |   13 +
 hw/acpi_piix4.c |   52 
 hw/vt82c686.c   |   54 +++---
 4 files changed, 81 insertions(+), 75 deletions(-)

diff --git a/hw/acpi.c b/hw/acpi.c
index 08cb126..07283be 100644
--- a/hw/acpi.c
+++ b/hw/acpi.c
@@ -15,6 +15,7 @@
  * You should have received a copy of the GNU Lesser General Public
  * License along with this library; if not, see 
  */
+#include "sysemu.h"
 #include "hw.h"
 #include "pc.h"
 #include "acpi.h"
@@ -198,6 +199,42 @@ out:
 return -1;
 }
 
+/* ACPI PM1a EVT */
+uint16_t acpi_pm1_evt_get_sts(ACPIPM1EVT *pm1, int64_t overflow_time)
+{
+int64_t d = acpi_pm_tmr_get_clock();
+if (d >= overflow_time) {
+pm1->sts |= ACPI_BITMASK_TIMER_STATUS;
+}
+return pm1->sts;
+}
+
+void acpi_pm1_evt_write_sts(ACPIPM1EVT *pm1, ACPIPMTimer *tmr, uint16_t val)
+{
+uint16_t pm1_sts = acpi_pm1_evt_get_sts(pm1, tmr->overflow_time);
+if (pm1_sts & val & ACPI_BITMASK_TIMER_STATUS) {
+/* if TMRSTS is reset, then compute the new overflow time */
+acpi_pm_tmr_calc_overflow_time(tmr);
+}
+pm1->sts &= ~val;
+}
+
+void acpi_pm1_evt_power_down(ACPIPM1EVT *pm1, ACPIPMTimer *tmr)
+{
+if (!pm1) {
+qemu_system_shutdown_request();
+} else if (pm1->en & ACPI_BITMASK_POWER_BUTTON_ENABLE) {
+pm1->sts |= ACPI_BITMASK_POWER_BUTTON_STATUS;
+tmr->update_sci(tmr);
+}
+}
+
+void acpi_pm1_evt_reset(ACPIPM1EVT *pm1)
+{
+pm1->sts = 0;
+pm1->en = 0;
+}
+
 /* ACPI PM_TMR */
 void acpi_pm_tmr_update(ACPIPMTimer *tmr, bool enable)
 {
diff --git a/hw/acpi.h b/hw/acpi.h
index fc42501..c286d7d 100644
--- a/hw/acpi.h
+++ b/hw/acpi.h
@@ -99,4 +99,17 @@ static inline int64_t acpi_pm_tmr_get_clock(void)
 get_ticks_per_sec());
 }
 
+/* PM1a_EVT: piix and ich9 don't implement PM1b. */
+struct ACPIPM1EVT
+{
+uint16_t sts;
+uint16_t en;
+};
+typedef struct ACPIPM1EVT ACPIPM1EVT;
+
+uint16_t acpi_pm1_evt_get_sts(ACPIPM1EVT *pm1, int64_t overflow_time);
+void acpi_pm1_evt_write_sts(ACPIPM1EVT *pm1, ACPIPMTimer *tmr, uint16_t val);
+void acpi_pm1_evt_power_down(ACPIPM1EVT *pm1, ACPIPMTimer *tmr);
+void acpi_pm1_evt_reset(ACPIPM1EVT *pm1);
+
 #endif /* !QEMU_HW_ACPI_H */
diff --git a/hw/acpi_piix4.c b/hw/acpi_piix4.c
index d5f631a..5b4eef5 100644
--- a/hw/acpi_piix4.c
+++ b/hw/acpi_piix4.c
@@ -54,8 +54,7 @@ struct pci_status {
 typedef struct PIIX4PMState {
 PCIDevice dev;
 IORange ioport;
-uint16_t pmsts;
-uint16_t pmen;
+ACPIPM1EVT pm1a;
 uint16_t pmcntrl;
 
 APMState apm;
@@ -81,20 +80,12 @@ static void piix4_acpi_system_hot_add_init(PCIBus *bus, 
PIIX4PMState *s);
 #define ACPI_ENABLE 0xf1
 #define ACPI_DISABLE 0xf0
 
-static int get_pmsts(PIIX4PMState *s)
-{
-int64_t d = acpi_pm_tmr_get_clock();
-if (d >= s->tmr.overflow_time)
-s->pmsts |= ACPI_BITMASK_TIMER_STATUS;
-return s->pmsts;
-}
-
 static void pm_update_sci(PIIX4PMState *s)
 {
 int sci_level, pmsts;
 
-pmsts = get_pmsts(s);
-sci_level = (((pmsts & s->pmen) &
+pmsts = acpi_pm1_evt_get_sts(&s->pm1a, s->tmr.overflow_time);
+sci_level = (((pmsts & s->pm1a.en) &
   (ACPI_BITMASK_RT_CLOCK_ENABLE |
ACPI_BITMASK_POWER_BUTTON_ENABLE |
ACPI_BITMASK_GLOBAL_LOCK_ENABLE |
@@ -103,7 +94,7 @@ static void pm_update_sci(PIIX4PMState *s)
 
 qemu_set_irq(s->irq, sci_level);
 /* schedule a timer interruption if needed */
-acpi_pm_tmr_update(&s->tmr, (s->pmen & ACPI_BITMASK_TIMER_ENABLE) &&
+acpi_pm_tmr_update(&s->tmr, (s->pm1a.en & ACPI_BITMASK_TIMER_ENABLE) &&
!(pmsts & ACPI_BITMASK_TIMER_STATUS));
 }
 
@@ -125,19 +116,11 @@ static void pm_ioport_write(IORange *ioport, uint64_t 
addr, unsigned width,
 
 switch(addr) {
 case 0x00:
-{
-int pmsts;
-pmsts = get_pmsts(s);
-if (pmsts & val & ACPI_BITMASK_TIMER_STATUS) {
-/* if TMRSTS is reset, then compute the new overflow time */
-acpi_pm_tmr_calc_overflow_time(&s->tmr);
-}
-s->pmsts &= ~val;
-pm_update_sci(s);
-}
+acpi_pm1_evt_write_sts(&s->pm1a, &s->tmr, val);
+pm_update_sci(s);
 break;
 case 0x02:
-s->pmen = val;
+s->pm1a.en = val;
 pm_update_sci(s);
 break;
 case 0x04:
@@ -154,8 +137,8 @@ static void pm_ioport_write(IORange *ioport, uint64_t addr, 
unsigned width,
 case 1:
 /* ACPI_BITMASK_WAKE_STATUS should be set on resume.

[Qemu-devel] [PATCH 0/4] acpi: factor out fixed hardware logic

[Isaku Yamahata]

So far acpi fixed hardware logic (PM TMR, PM1a_EVT, PM1_CNT and GPE0)
are embedded in each device.(acpi_piix.c and vt82c686).
This patch series factors out the logic and consolidate them.
This was the part of q35 chipset patch series which also implements
acpi fixed hardware.
But I made the acpi part independent patch series to make the merge easy.

Isaku Yamahata (4):
  acpi, acpi_piix, vt82c686: factor out PM_TMR logic
  acpi, acpi_piix, vt82c686: factor out PM1a EVT logic
  acpi, acpi_piix, vt82c686: factor out PM1_CNT logic
  acpi, acpi_piix: factor out GPE logic

 hw/acpi.c   |  197 +
 hw/acpi.h   |   68 +
 hw/acpi_piix4.c |  220 +-
 hw/vt82c686.c   |  110 +++
 4 files changed, 346 insertions(+), 249 deletions(-)

[Qemu-devel] ARM: BKPT instructions should raise prefetch aborts with IFSR type 00010

[Alex Zuepke]

Hi,

while digging through some problems with BKPT exceptions on ARM, I
discovered that QEMU does not update IFSR on prefetch aborts. This
should be done since ARMv6 according to ARM docs. Please include.

Best Regards,
Alex

-- 
Alexander Zuepkeazue...@sysgo.com
SYSGO AG ~ Am Pfaffenstein 14 ~ 55270 Klein-Winternheim ~ Germany
 target-arm: BKPT instructions should raise prefetch aborts with IFSR type 00010
 diff against qemu 0.14.0
 Signed-off-by: Alex Zuepke 
--- qemu-0.14.0.orig/target-arm/translate.c	2011-02-16 15:44:05.0 +0100
+++ qemu-0.14.0/target-arm/translate.c	2011-03-25 11:22:03.0 +0100
@@ -6389,6 +6389,7 @@
 goto illegal_op;
 }
 /* bkpt */
+env->cp15.c5_insn = 2;
 gen_exception_insn(s, 4, EXCP_BKPT);
 break;
 case 0x8: /* signed multiply */
@@ -8930,6 +8931,7 @@
 break;
 
 case 0xe: /* bkpt */
+env->cp15.c5_insn = 2;
 gen_exception_insn(s, 2, EXCP_BKPT);
 break;
 

Re: [Qemu-devel] [PATCH v2] severe memory leak caused by broken palette_destroy() function

[Stefan Hajnoczi]

On Fri, Mar 25, 2011 at 8:45 AM, Ulrich Obergfell  wrote:
>
> This is version 2 of the patch that I originally posted in:
>
> http://lists.gnu.org/archive/html/qemu-devel/2011-03/msg02063.html
>
> [Sorry, I missed to include the keyword 'PATCH' in the subject
>  of the original post.]
>
> The following commit breaks the code of the function palette_destroy().
>
> http://git.kernel.org/?p=virt/kvm/qemu-kvm.git;a=commit;h=e31e3694afef58ba191cbcc6875ec243e5971268
>
> The broken code causes a severe memory leak of 'VncPalette' structures
> because it never frees anything:
>
>     70 void palette_destroy(VncPalette *palette)
>     71 {
>     72     if (palette == NULL) {
>     73         qemu_free(palette);
>     74     }
>     75 }
>
> Version 2 of the patch calls qemu_free() unconditionally.
>
> Signed-off-by: Ulrich Obergfell 

Looks good.

Reviewed-by: Stefan Hajnoczi 

[Qemu-devel] Re: [PATCH v2] virtio-serial: Print out reason for aborting before calling abort()

[Amit Shah]

On (Fri) 25 Mar 2011 [10:23:16], Juan Quintela wrote:
> Amit Shah  wrote:
> > When a port returns an error for not consuming data, we can only handle
> > the -EAGAIN error type.  Any other error isn't handled.  Print out a
> > message indicating this and the error returned.
> >
> > Signed-off-by: Amit Shah 
> > ---
> >  hw/virtio-serial-bus.c |2 ++
> >  1 files changed, 2 insertions(+), 0 deletions(-)
> >
> > diff --git a/hw/virtio-serial-bus.c b/hw/virtio-serial-bus.c
> > index a82fbe9..8b715b2 100644
> > --- a/hw/virtio-serial-bus.c
> > +++ b/hw/virtio-serial-bus.c
> > @@ -155,6 +155,8 @@ static void do_flush_queued_data(VirtIOSerialPort 
> > *port, VirtQueue *vq,
> >  buf_size);
> >  if (ret < 0 && ret != -EAGAIN) {
> >  /* We don't handle any other type of errors here */
> > +error_report("%s: unexpected return %zd, aborting.\n",
> > + __func__, ret);
> >  abort();
> >  }
> >  if (ret == -EAGAIN || (ret >= 0 && ret < buf_size)) {
> 
> Reviewed-by: Juan Quintela 
> 
> I agree that change is a step in the right direction.  But I don't think
> the "abort" way of handling errors.  Not that this can be improved
> without changing all virtual queues functions to allow for return codes
> :-(

Yes :-(

BTW I'm working on more patches here: first step is to have
implementations of have_data() take the right call in other error
conditions (e.g. chardev closed would give -EPIPE, so those type of
things should be handled there sanely.)

Amit

[Qemu-devel] Re: [PATCH v2] severe memory leak caused by broken palette_destroy() function

[Juan Quintela]

Ulrich Obergfell  wrote:
> This is version 2 of the patch that I originally posted in:
>
> http://lists.gnu.org/archive/html/qemu-devel/2011-03/msg02063.html
>
> [Sorry, I missed to include the keyword 'PATCH' in the subject
>  of the original post.]
>
> The following commit breaks the code of the function palette_destroy().
>
> http://git.kernel.org/?p=virt/kvm/qemu-kvm.git;a=commit;h=e31e3694afef58ba191cbcc6875ec243e5971268
>
> The broken code causes a severe memory leak of 'VncPalette' structures
> because it never frees anything:
>
>  70 void palette_destroy(VncPalette *palette)
>  71 {
>  72 if (palette == NULL) {
>  73 qemu_free(palette);
>  74 }
>  75 }
>
> Version 2 of the patch calls qemu_free() unconditionally.
>
> Signed-off-by: Ulrich Obergfell 


Ouchhh

Reviewed-by: Juan Quintela 

A new reason to never ever test if pointer is != NULL before calling
free.

Good catch.

[Qemu-devel] Re: [PATCH] floppy: save and restore DIR register

[Juan Quintela]

Jason Wang  wrote:
> We need to keep DIR register unchanged across migration, but currently it
> depends on the media_changed flags from block layer and we do not save/restore
> it which could let the guest driver think the floppy have changed after
> migration. To fix this, a new filed media_changed in FDrive strcutre was
> introduced in order to save and restore the it from block layer through
> pre_save/post_load callbacks.
>
> Signed-off-by: Jason Wang 
> ---
>  hw/fdc.c |   46 ++
>  1 files changed, 46 insertions(+), 0 deletions(-)
>
> diff --git a/hw/fdc.c b/hw/fdc.c
> index 9fdbc75..8257d51 100644
> --- a/hw/fdc.c
> +++ b/hw/fdc.c
> @@ -36,6 +36,7 @@
>  #include "qdev-addr.h"
>  #include "blockdev.h"
>  #include "sysemu.h"
> +#include "block_int.h"
>  
>  //
>  /* debug Floppy devices */
> @@ -82,6 +83,7 @@ typedef struct FDrive {
>  uint8_t max_track;/* Nb of tracks   */
>  uint16_t bps; /* Bytes per sector   */
>  uint8_t ro;   /* Is read-only   */
> +uint8_t media_changed;/* Is media changed   */
>  } FDrive;
>  
>  static void fd_init(FDrive *drv)
> @@ -533,6 +535,42 @@ static CPUWriteMemoryFunc * const 
> fdctrl_mem_write_strict[3] = {
>  NULL,
>  };
>  
> +static void fdrive_media_changed_pre_save(void *opaque)
> +{
> +FDrive *drive = opaque;
> +
> +drive->media_changed = drive->bs->media_changed;
> +}
> +
> +static int fdrive_media_changed_post_load(void *opaque, int version_id)
> +{
> +FDrive *drive = opaque;
> +
> +drive->bs->media_changed = drive->media_changed;

shouldn't this be something like:

if (!drive->bs)
   return 1;

drive->bs->media_changed = drive->media_changed.

???

Otherwise, we can can have troubles if floppy is not opened in the other side.


Rest of code looks nice, thanks.

Later, Juan.

[Qemu-devel] Re: [PATCH v2] virtio-serial: Print out reason for aborting before calling abort()

[Juan Quintela]

Amit Shah  wrote:
> When a port returns an error for not consuming data, we can only handle
> the -EAGAIN error type.  Any other error isn't handled.  Print out a
> message indicating this and the error returned.
>
> Signed-off-by: Amit Shah 
> ---
>  hw/virtio-serial-bus.c |2 ++
>  1 files changed, 2 insertions(+), 0 deletions(-)
>
> diff --git a/hw/virtio-serial-bus.c b/hw/virtio-serial-bus.c
> index a82fbe9..8b715b2 100644
> --- a/hw/virtio-serial-bus.c
> +++ b/hw/virtio-serial-bus.c
> @@ -155,6 +155,8 @@ static void do_flush_queued_data(VirtIOSerialPort *port, 
> VirtQueue *vq,
>  buf_size);
>  if (ret < 0 && ret != -EAGAIN) {
>  /* We don't handle any other type of errors here */
> +error_report("%s: unexpected return %zd, aborting.\n",
> + __func__, ret);
>  abort();
>  }
>  if (ret == -EAGAIN || (ret >= 0 && ret < buf_size)) {

Reviewed-by: Juan Quintela 

I agree that change is a step in the right direction.  But I don't think
the "abort" way of handling errors.  Not that this can be improved
without changing all virtual queues functions to allow for return codes
:-(

[Qemu-devel] [PATCH v2] severe memory leak caused by broken palette_destroy() function

[Ulrich Obergfell]

This is version 2 of the patch that I originally posted in:

http://lists.gnu.org/archive/html/qemu-devel/2011-03/msg02063.html

[Sorry, I missed to include the keyword 'PATCH' in the subject
 of the original post.]

The following commit breaks the code of the function palette_destroy().

http://git.kernel.org/?p=virt/kvm/qemu-kvm.git;a=commit;h=e31e3694afef58ba191cbcc6875ec243e5971268

The broken code causes a severe memory leak of 'VncPalette' structures
because it never frees anything:

 70 void palette_destroy(VncPalette *palette)
 71 {
 72 if (palette == NULL) {
 73 qemu_free(palette);
 74 }
 75 }

Version 2 of the patch calls qemu_free() unconditionally.

Signed-off-by: Ulrich Obergfell 


diff -up ./ui/vnc-palette.c.orig0 ./ui/vnc-palette.c
--- ./ui/vnc-palette.c.orig02011-03-15 03:53:22.0 +0100
+++ ./ui/vnc-palette.c  2011-03-21 20:19:02.736948725 +0100
@@ -69,9 +69,7 @@ void palette_init(VncPalette *palette, s
 
 void palette_destroy(VncPalette *palette)
 {
-if (palette == NULL) {
-qemu_free(palette);
-}
+qemu_free(palette);
 }
 
 int palette_put(VncPalette *palette, uint32_t color)

[Qemu-devel] Request for suggestions: USB Interface Association Descriptors

[Brad Hards]

Hi,

I've just started the learning process on QEMU, and am trying to create a USB 
webcam virtual device (targetting the Linux UVC driver). So far, I'm able to 
create the device, plug it in (thanks to those involved in qdev), and inspect 
the descriptor.

The problem is building the descriptor, and particularly how to do an 
Interface Association Descriptor. This is covered in Section 9.6.4 of the USB 
3.0 spec, but there is a better description in the Intel IAD whitepaper 
(www.usb.org/developers/whitepapers/iadclasscode_r10.pdf).

The short version is that IAD is an extra descriptor type that appears before 
a group (two or more) interface descriptors, that explains which interface 
descriptors make up a virtual device.  So it could look like:
Config Desc
IAD#0
Iface#0
Iface#1
Iface#2
IAD#1
Iface#3
Iface#4

[Check the diagram in the Intel IAD whitepaper if that makes no sense]

I've managed to make the USB descriptor code produce my descriptor, but the 
change has (at least) two problems I'd appreciate suggestions on how to fix:
1. I can only do the simple case where there is 0 or 1 IADs.
2. The change impacts on every USB configuration descriptor, even though IAD 
isn't very common.

Here is what I'm currently doing in any case (not complete - I haven't 
converted over all the existing device to have the .niad = 0 bit, since I got 
a bad feeling about it.  Not sure what the problem is - it just feels a bit 
wrong).

Suggestions?

Brad

--- a/hw/usb-desc.c
+++ b/hw/usb-desc.c
@@ -91,6 +91,15 @@ int usb_desc_config(const USBDescConfig *conf, uint8_t 
*dest, 
size_t len)
 dest[0x08] = conf->bMaxPower;
 wTotalLength += bLength;
 
+/* handle Interface Association Descriptor, if present */
+if (conf->niad != 0) {
+rc = usb_desc_iad(conf->iad, dest + wTotalLength, len - 
wTotalLength);
+if (rc < 0) {
+return rc;
+}
+wTotalLength += rc;
+}
+
 count = conf->nif ? conf->nif : conf->bNumInterfaces;
 for (i = 0; i < count; i++) {
 rc = usb_desc_iface(conf->ifs + i, dest + wTotalLength, len - 
wTotalLength);
@@ -105,6 +114,26 @@ int usb_desc_config(const USBDescConfig *conf, uint8_t 
*dest, size_t len)
 return wTotalLength;
 }
 
+int usb_desc_iad(const USBDescIfaceAssoc *iad, uint8_t *dest, size_t len)
+{
+uint8_t bLength = 0x08;
+
+if (len < bLength) {
+return -1;
+}
+
+dest[0x00] = bLength;
+dest[0x01] = USB_DT_INTERFACE_ASSOC;
+dest[0x02] = iad->bFirstInterface;
+dest[0x03] = iad->bInterfaceCount;
+dest[0x04] = iad->bFunctionClass;
+dest[0x05] = iad->bFunctionSubClass;
+dest[0x06] = iad->bFunctionProtocol;
+dest[0x07] = iad->iFunction;
+
+return bLength;
+}
+
 int usb_desc_iface(const USBDescIface *iface, uint8_t *dest, size_t len)
 {
 uint8_t bLength = 0x09;
diff --git a/hw/usb-desc.h b/hw/usb-desc.h
index ac734ab..cf5f794 100644
--- a/hw/usb-desc.h
+++ b/hw/usb-desc.h
@@ -30,10 +30,22 @@ struct USBDescConfig {
 uint8_t   bmAttributes;
 uint8_t   bMaxPower;
 
+uint8_t   niad; /* 0 or 1 for now */
+const USBDescIfaceAssoc   *iad;
+
 uint8_t   nif;
 const USBDescIface*ifs;
 };
 
+struct USBDescIfaceAssoc {
+uint8_t   bFirstInterface;
+uint8_t   bInterfaceCount;
+uint8_t   bFunctionClass;
+uint8_t   bFunctionSubClass;
+uint8_t   bFunctionProtocol;
+uint8_t   iFunction;
+};
+
 struct USBDescIface {
 uint8_t   bInterfaceNumber;
 uint8_t   bAlternateSetting;
@@ -75,6 +87,7 @@ int usb_desc_device(const USBDescID *id, const USBDescDevice 
*dev,
 int usb_desc_device_qualifier(const USBDescDevice *dev,
   uint8_t *dest, size_t len);
 int usb_desc_config(const USBDescConfig *conf, uint8_t *dest, size_t len);
+int usb_desc_iad(const USBDescIfaceAssoc *iad, uint8_t *dest, size_t len);
 int usb_desc_iface(const USBDescIface *iface, uint8_t *dest, size_t len);
 int usb_desc_endpoint(const USBDescEndpoint *ep, uint8_t *dest, size_t len);
 int usb_desc_other(const USBDescOther *desc, uint8_t *dest, size_t len);
diff --git a/hw/usb-hid.c b/hw/usb-hid.c
index c25362c..acbcddc 100644
--- a/hw/usb-hid.c
+++ b/hw/usb-hid.c
@@ -211,6 +211,7 @@ static const USBDescDevice desc_device_mouse = {
 .iConfiguration= STR_CONFIG_MOUSE,
 .bmAttributes  = 0xa0,
 .bMaxPower = 50,
+.niad  = 0,
 .ifs = &desc_iface_mouse,
 },
 },
@@ -227,6 +228,7 @@ static const USBDescDevice desc_device_tablet = {
 .iConfiguration= STR_CONFIG_TABLET,
 .bmAttributes  = 0xa0,
 .bMaxPower = 50,
+.niad  = 0,
 .ifs =

Re: [Qemu-devel] Compile errors in vl.c on git head

[Stefan Hajnoczi]

On Fri, Mar 25, 2011 at 7:58 AM, Peter Maydell  wrote:
> On 25 March 2011 07:17, Stefan Hajnoczi  wrote:
>> On Fri, Mar 25, 2011 at 7:05 AM, Gerhard Wiesinger  
>> wrote:
>>> I can't compile:
>>>  CC    libhw64/vl.o
>>> cc1: warnings being treated as errors
>>> /root/download/qemu/git/qemu/vl.c: In function .select_display.:
>>> /root/download/qemu/git/qemu/vl.c:1645: error: label .invalid_display.
>>> defined but not used
>
>> Temporary workaround:
>> $ ./configure --disable-werror [...]
>> $ make
>>
>> Jes: This is a CONFIG_SDL issue.  I think you're the expert! :)
>
> This is fixed by this patch:
> http://patchwork.ozlabs.org/patch/88098/
> sent Wednesday but hasn't made it into git yet.
>
> (The patch got an ack from Jes although patchwork seems
> to have lost it.)

CCed Anthony so this patch can be merged.

Stefan

Re: [Qemu-devel] Compile errors in vl.c on git head

[Peter Maydell]

On 25 March 2011 07:17, Stefan Hajnoczi  wrote:
> On Fri, Mar 25, 2011 at 7:05 AM, Gerhard Wiesinger  
> wrote:
>> I can't compile:
>>  CC    libhw64/vl.o
>> cc1: warnings being treated as errors
>> /root/download/qemu/git/qemu/vl.c: In function .select_display.:
>> /root/download/qemu/git/qemu/vl.c:1645: error: label .invalid_display.
>> defined but not used

> Temporary workaround:
> $ ./configure --disable-werror [...]
> $ make
>
> Jes: This is a CONFIG_SDL issue.  I think you're the expert! :)

This is fixed by this patch:
http://patchwork.ozlabs.org/patch/88098/
sent Wednesday but hasn't made it into git yet.

(The patch got an ack from Jes although patchwork seems
to have lost it.)

-- PMM

Re: [Qemu-devel] Compile errors in vl.c on git head

[Stefan Hajnoczi]

On Fri, Mar 25, 2011 at 7:05 AM, Gerhard Wiesinger  wrote:
> I can't compile:
>  CC    libhw64/vl.o
> cc1: warnings being treated as errors
> /root/download/qemu/git/qemu/vl.c: In function .select_display.:
> /root/download/qemu/git/qemu/vl.c:1645: error: label .invalid_display.
> defined but not used
> make[1]: *** [vl.o] Error 1
> make: *** [subdir-libhw64] Error 2

Temporary workaround:
$ ./configure --disable-werror [...]
$ make

Jes: This is a CONFIG_SDL issue.  I think you're the expert! :)

Stefan

[Qemu-devel] Compile errors in vl.c on git head

[Gerhard Wiesinger]

Hello,

I can't compile:
  CClibhw64/vl.o
cc1: warnings being treated as errors
/root/download/qemu/git/qemu/vl.c: In function .select_display.:
/root/download/qemu/git/qemu/vl.c:1645: error: label .invalid_display. 
defined but not used

make[1]: *** [vl.o] Error 1
make: *** [subdir-libhw64] Error 2

gcc (GCC) 4.4.5 20101112 (Red Hat 4.4.5-2)

Thnx.

Ciao,
Gerhard

--
http://www.wiesinger.com/