* [Qemu-devel] [RFC][PATCH v7 00/16] virtagent: host/guest communication agent
@ 2011-03-07 20:10 Michael Roth
2011-03-07 20:10 ` [Qemu-devel] [RFC][PATCH v7 01/16] Move code related to fd handlers into utility functions Michael Roth
` (16 more replies)
0 siblings, 17 replies; 41+ messages in thread
From: Michael Roth @ 2011-03-07 20:10 UTC (permalink / raw)
To: qemu-devel
Cc: agl, stefanha, Jes.Sorensen, mdroth, markus_mueller, aliguori,
abeekhof
These patches apply to master (3-07-2011), and can also be obtained from:
git://repo.or.cz/qemu/mdroth.git virtagent_v7
CHANGES IN V7:
- Removed dependency on xmlrpc-c for data transport. Now using JSON via QEMU's qjson qobject<->json conversion routines. Binary encoding mechanisms such as Protocol Buffers and ASN.1/BER were considered, but due to limited library support, and limitations of isa/virtio serial transport that would have required an additional layer of encoding to reliably determine RPC boundaries during transport (more here: http://www.mail-archive.com/qemu-devel@nongnu.org/msg56237.html), qobject<->json seemed to be the most prudent route.
- Logic to handle management/scheduling of bi-directional RPCs is now decoupled from transport layer for readability and better support for future additions such as session-level agents and threaded execution of RPCs.
- Added thorough documentation of virtagent protocol to virtagent-manager.h
- Improved documentation for RPCs
- Fixes for guest agent lockfile handling
- Removed viewdmesg/viewfile, will be replacing these with a more robust getfile RPC/command shortly
- Workaround for guests that, in certain situations, fail to retrieve a pushed buffer from virtqueue in timely fashion (Rusty submitted patch several months ago, don't know the status)
- Added guest agent support for switching isa-serial ports to RAW mode (previously reliant on socat intermediary)
- Added tagging of RPCs to allow for multiple in-flight requests
- qemu-va guest agent no longer builds as part of host tools target
- Various other bug fixes and cleanups
CHANGES IN V6:
- Added a sentinel value to reliably detect the start of an "http" hdr. Used to skip past partially sent http content from previous "sessions"
- Added http hdr tag (currently hardcoded for testing, will switch to uuid) to filter out valid-but-unexpected content in channel from previous "sessions"
- Added timeout mechanism to avoid hanging monitor when agent isn't running
- Added timed back-off on read's from a virtio-serial that result in ret=0 to avoid spinning if host isn't connected.
- Added daemonize flags to qemu-va
- Added sane defaults for channel type and virtio-serial port path
- Various bug fixes for state machine/job handling logic
CHANGES IN V5:
- Dependency on virtproxy dropped, virtagent now handles transport and multiplexing of bi-directional RPCs internally
- Removed duplification of qemu_set_fd_handler()-centered i/o code. Support for interacting with objects that use qemu_set_fd_handler() now available to tools via qemu-tools.c and a set of generalized utility functions
- Fixed memory leaks in client/monitor functions
- Various cleanups
CHANGES IN V4:
- Added guest agent capabilities negotiation
- Added RPC/monitor command to invoke guest shutdown/reboot/powerdown
- Added RPC/monitor command to the guest agent
- Added guest startup notification ("hello")
- Added syslog()'ing of guest agent RPCs
- Various cleanups
CHANGES IN V3:
- Integrated virtagent server into virtproxy chardev. Usage examples below.
- Consolidated RPC server/client setup into a pair of init routines
- Fixed buffer overflow in agent_viewfile() and various memory leaks
CHANGES IN V2:
- All RPC communication is now done using asynchronous/non-blocking read/write handlers
- Previously fork()'d RPC server loop is now integrated into qemu-vp/virtproxy i/o loop
- Cleanups/suggestions from previous RFC
OVERVIEW:
There are a wide range of use cases motivating the need for a guest agent of some sort to extend the functionality/usability/control offered by QEMU. Some examples include graceful guest shutdown/reboot and notifications thereof, copy/paste syncing between host/guest, guest statistics gathering, file access, etc.
Ideally these would all be served by a single, easilly extensible agent that can be deployed in a wide range of guests. Virtagent is an JSON RPC server, integrated into QEMU and a simple guest daemon, aimed at providing this type of functionality.
DESIGN:
There are actually 2 RPC servers:
1) a server in the guest agent which handles RPC requests from QEMU
2) a server in the host, integrated into the virtagent chardev, to handle RPC requests sent by the guest agent (mainly for handling asynchronous events reported by the agent).
Communication is done via RPCs (JSON/HTTP between host and guest), albeit with a non-standard implementation that allows for multiplexing server/client RPC over a single virtio/isa serial channel.
EXAMPLE USAGE:
- Build into host:
./configure --target-list=x86_64-softmmu --enable-io-thread
make
- Build guest agent (in guest):
./configure --target-list=x86_64-softmmu --enabled-io-thread
make qemu-va
- Configure guest agent to talk to host via virtio-serial
# start guest with virtio-serial/virtagent. for example (RHEL6):
qemu \
-chardev virtagent,id=test0 \
-device virtio-serial \
-device virtserialport,chardev=test0,name=virtagent0 \
-monitor stdio
...
# in the guest:
sudo ./qemu-va -c virtio-serial -p /dev/virtio-ports/virtagent0
...
# monitor commands
(qemu) va_ping
status: success
(qemu) va_viewfile /proc/meminfo
MemTotal: 3985488 kB
MemFree: 400524 kB
Buffers: 220556 kB
Cached: 2073160 kB
SwapCached: 0 kB
...
Hugepagesize: 2048 kB
DirectMap4k: 8896 kB
DirectMap2M: 4110336 kB
(qemu) va_shutdown powerdown
(qemu)
KNOWN ISSUES/PLANS:
- Implement RPCs for stateful open/read/close of guest files.
- Implement RPC for guest script/command execution
- Scan for sentinel value while reading http content as well to immediately detect truncated requests/responses and avoid accidentally consuming new ones
- switch to standard logging/trace mechanisms
- the client socket that qemu connects to send RPCs is a hardcoded filepath. This is unacceptable as the socket is channel/process specific and things will break when multiple guests are started.
Makefile | 4 +-
Makefile.objs | 2 +-
Makefile.target | 2 +-
cpus.c | 83 +-------
hmp-commands.hx | 48 +++++
monitor.c | 1 +
qemu-char.c | 44 ++++
qemu-char.h | 4 +
qemu-ioh.c | 210 ++++++++++++++++++++
qemu-ioh.h | 43 ++++
qemu-tool.c | 115 +++++++++++-
qemu-tool.h | 26 +++
qemu-va.c | 247 +++++++++++++++++++++++
qerror.c | 8 +
qerror.h | 6 +
qmp-commands.hx | 97 +++++++++
roms/seabios | 2 +-
virtagent-common.c | 206 +++++++++++++++++++
virtagent-common.h | 95 +++++++++
virtagent-manager.c | 326 ++++++++++++++++++++++++++++++
virtagent-manager.h | 130 ++++++++++++
virtagent-server.c | 387 ++++++++++++++++++++++++++++++++++++
virtagent-server.h | 40 ++++
virtagent-transport.c | 432 ++++++++++++++++++++++++++++++++++++++++
virtagent.c | 524 +++++++++++++++++++++++++++++++++++++++++++++++++
virtagent.h | 51 +++++
vl.c | 86 ++-------
27 files changed, 3071 insertions(+), 148 deletions(-)
^ permalink raw reply [flat|nested] 41+ messages in thread
* [Qemu-devel] [RFC][PATCH v7 01/16] Move code related to fd handlers into utility functions
2011-03-07 20:10 [Qemu-devel] [RFC][PATCH v7 00/16] virtagent: host/guest communication agent Michael Roth
@ 2011-03-07 20:10 ` Michael Roth
2011-03-09 13:58 ` [Qemu-devel] " Paolo Bonzini
2011-03-09 14:09 ` Paolo Bonzini
2011-03-07 20:10 ` [Qemu-devel] [RFC][PATCH v7 02/16] Add qemu_set_fd_handler() wrappers to qemu-tools.c Michael Roth
` (15 subsequent siblings)
16 siblings, 2 replies; 41+ messages in thread
From: Michael Roth @ 2011-03-07 20:10 UTC (permalink / raw)
To: qemu-devel
Cc: agl, stefanha, Jes.Sorensen, mdroth, markus_mueller, aliguori,
abeekhof
This allows us to implement an i/o loop outside of vl.c that can
interact with objects that use qemu_set_fd_handler()
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
Makefile.objs | 2 +-
qemu-char.h | 4 ++
qemu-ioh.c | 115 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
qemu-ioh.h | 34 +++++++++++++++++
vl.c | 86 ++++++++----------------------------------
5 files changed, 170 insertions(+), 71 deletions(-)
create mode 100644 qemu-ioh.c
create mode 100644 qemu-ioh.h
diff --git a/Makefile.objs b/Makefile.objs
index 9e98a66..4303b95 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -14,7 +14,7 @@ oslib-obj-$(CONFIG_POSIX) += oslib-posix.o
# block-obj-y is code used by both qemu system emulation and qemu-img
block-obj-y = cutils.o cache-utils.o qemu-malloc.o qemu-option.o module.o
-block-obj-y += nbd.o block.o aio.o aes.o qemu-config.o
+block-obj-y += nbd.o block.o aio.o aes.o qemu-config.o qemu-ioh.o
block-obj-$(CONFIG_POSIX) += posix-aio-compat.o
block-obj-$(CONFIG_LINUX_AIO) += linux-aio.o
diff --git a/qemu-char.h b/qemu-char.h
index 56d9954..34936a7 100644
--- a/qemu-char.h
+++ b/qemu-char.h
@@ -7,6 +7,7 @@
#include "qemu-config.h"
#include "qobject.h"
#include "qstring.h"
+#include "qemu-ioh.h"
/* character device */
@@ -120,4 +121,7 @@ int qemu_set_fd_handler(int fd,
IOHandler *fd_read,
IOHandler *fd_write,
void *opaque);
+void qemu_get_fdset(int *nfds, fd_set *rfds, fd_set *wfds, fd_set *xfds);
+void qemu_process_fd_handlers(const fd_set *rfds, const fd_set *wfds,
+ const fd_set *xfds);
#endif
diff --git a/qemu-ioh.c b/qemu-ioh.c
new file mode 100644
index 0000000..cc71470
--- /dev/null
+++ b/qemu-ioh.c
@@ -0,0 +1,115 @@
+/*
+ * QEMU System Emulator
+ *
+ * Copyright (c) 2003-2008 Fabrice Bellard
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+#include "qemu-ioh.h"
+#include "qlist.h"
+
+/* XXX: fd_read_poll should be suppressed, but an API change is
+ necessary in the character devices to suppress fd_can_read(). */
+int qemu_set_fd_handler3(void *ioh_record_list,
+ int fd,
+ IOCanReadHandler *fd_read_poll,
+ IOHandler *fd_read,
+ IOHandler *fd_write,
+ void *opaque)
+{
+ QLIST_HEAD(, IOHandlerRecord) *io_handlers_ptr = ioh_record_list;
+ IOHandlerRecord *ioh;
+
+ if (!fd_read && !fd_write) {
+ QLIST_FOREACH(ioh, io_handlers_ptr, next) {
+ if (ioh->fd == fd) {
+ ioh->deleted = 1;
+ break;
+ }
+ }
+ } else {
+ QLIST_FOREACH(ioh, io_handlers_ptr, next) {
+ if (ioh->fd == fd)
+ goto found;
+ }
+ ioh = qemu_mallocz(sizeof(IOHandlerRecord));
+ QLIST_INSERT_HEAD(io_handlers_ptr, ioh, next);
+ found:
+ ioh->fd = fd;
+ ioh->fd_read_poll = fd_read_poll;
+ ioh->fd_read = fd_read;
+ ioh->fd_write = fd_write;
+ ioh->opaque = opaque;
+ ioh->deleted = 0;
+ }
+ return 0;
+}
+
+/* add entries from ioh record list to fd sets. nfds and fd sets
+ * should be cleared/reset by caller if desired. set a particular
+ * fdset to NULL to ignore fd events of that type
+ */
+void qemu_get_fdset2(void *ioh_record_list, int *nfds, fd_set *rfds,
+ fd_set *wfds, fd_set *xfds)
+{
+ QLIST_HEAD(, IOHandlerRecord) *io_handlers = ioh_record_list;
+ IOHandlerRecord *ioh;
+
+ QLIST_FOREACH(ioh, io_handlers, next) {
+ if (ioh->deleted)
+ continue;
+ if ((rfds != NULL && ioh->fd_read) &&
+ (!ioh->fd_read_poll ||
+ ioh->fd_read_poll(ioh->opaque) != 0)) {
+ FD_SET(ioh->fd, rfds);
+ if (ioh->fd > *nfds)
+ *nfds = ioh->fd;
+ }
+ if (wfds != NULL && ioh->fd_write) {
+ FD_SET(ioh->fd, wfds);
+ if (ioh->fd > *nfds)
+ *nfds = ioh->fd;
+ }
+ }
+}
+
+/* execute registered handlers for r/w events in the provided fdsets. unset
+ * handlers are cleaned up here as well
+ */
+void qemu_process_fd_handlers2(void *ioh_record_list, const fd_set *rfds,
+ const fd_set *wfds, const fd_set *xfds)
+{
+ QLIST_HEAD(, IOHandlerRecord) *io_handlers = ioh_record_list;
+ IOHandlerRecord *ioh, *pioh;
+
+ QLIST_FOREACH_SAFE(ioh, io_handlers, next, pioh) {
+ if (!ioh->deleted && ioh->fd_read && FD_ISSET(ioh->fd, rfds)) {
+ ioh->fd_read(ioh->opaque);
+ }
+ if (!ioh->deleted && ioh->fd_write && FD_ISSET(ioh->fd, wfds)) {
+ ioh->fd_write(ioh->opaque);
+ }
+
+ /* Do this last in case read/write handlers marked it for deletion */
+ if (ioh->deleted) {
+ QLIST_REMOVE(ioh, next);
+ qemu_free(ioh);
+ }
+ }
+}
diff --git a/qemu-ioh.h b/qemu-ioh.h
new file mode 100644
index 0000000..7c6e833
--- /dev/null
+++ b/qemu-ioh.h
@@ -0,0 +1,34 @@
+#ifndef QEMU_IOH_H
+#define QEMU_IOH_H
+
+#include "qemu-common.h"
+#include "qlist.h"
+
+/* common i/o loop definitions */
+
+typedef struct IOHandlerRecord {
+ int fd;
+ IOCanReadHandler *fd_read_poll;
+ IOHandler *fd_read;
+ IOHandler *fd_write;
+ int deleted;
+ void *opaque;
+ /* temporary data */
+ struct pollfd *ufd;
+ QLIST_ENTRY(IOHandlerRecord) next;
+} IOHandlerRecord;
+
+/* XXX: fd_read_poll should be suppressed, but an API change is
+ necessary in the character devices to suppress fd_can_read(). */
+int qemu_set_fd_handler3(void *io_handlers_ptr,
+ int fd,
+ IOCanReadHandler *fd_read_poll,
+ IOHandler *fd_read,
+ IOHandler *fd_write,
+ void *opaque);
+void qemu_get_fdset2(void *ioh_record_list, int *nfds, fd_set *rfds,
+ fd_set *wfds, fd_set *xfds);
+void qemu_process_fd_handlers2(void *ioh_record_list, const fd_set *rfds,
+ const fd_set *wfds, const fd_set *xfds);
+
+#endif
diff --git a/vl.c b/vl.c
index b436952..dc90774 100644
--- a/vl.c
+++ b/vl.c
@@ -148,6 +148,7 @@ int main(int argc, char **argv)
#include "qemu-config.h"
#include "qemu-objects.h"
#include "qemu-options.h"
+#include "qemu-ioh.h"
#ifdef CONFIG_VIRTFS
#include "fsdev/qemu-fsdev.h"
#endif
@@ -1025,18 +1026,6 @@ void pcmcia_info(Monitor *mon)
/***********************************************************/
/* I/O handling */
-typedef struct IOHandlerRecord {
- int fd;
- IOCanReadHandler *fd_read_poll;
- IOHandler *fd_read;
- IOHandler *fd_write;
- int deleted;
- void *opaque;
- /* temporary data */
- struct pollfd *ufd;
- QLIST_ENTRY(IOHandlerRecord) next;
-} IOHandlerRecord;
-
static QLIST_HEAD(, IOHandlerRecord) io_handlers =
QLIST_HEAD_INITIALIZER(io_handlers);
@@ -1049,31 +1038,8 @@ int qemu_set_fd_handler2(int fd,
IOHandler *fd_write,
void *opaque)
{
- IOHandlerRecord *ioh;
-
- if (!fd_read && !fd_write) {
- QLIST_FOREACH(ioh, &io_handlers, next) {
- if (ioh->fd == fd) {
- ioh->deleted = 1;
- break;
- }
- }
- } else {
- QLIST_FOREACH(ioh, &io_handlers, next) {
- if (ioh->fd == fd)
- goto found;
- }
- ioh = qemu_mallocz(sizeof(IOHandlerRecord));
- QLIST_INSERT_HEAD(&io_handlers, ioh, next);
- found:
- ioh->fd = fd;
- ioh->fd_read_poll = fd_read_poll;
- ioh->fd_read = fd_read;
- ioh->fd_write = fd_write;
- ioh->opaque = opaque;
- ioh->deleted = 0;
- }
- return 0;
+ return qemu_set_fd_handler3(&io_handlers, fd, fd_read_poll, fd_read,
+ fd_write, opaque);
}
int qemu_set_fd_handler(int fd,
@@ -1084,6 +1050,17 @@ int qemu_set_fd_handler(int fd,
return qemu_set_fd_handler2(fd, NULL, fd_read, fd_write, opaque);
}
+void qemu_get_fdset(int *nfds, fd_set *rfds, fd_set *wfds, fd_set *xfds)
+{
+ return qemu_get_fdset2(&io_handlers, nfds, rfds, wfds, xfds);
+}
+
+void qemu_process_fd_handlers(const fd_set *rfds, const fd_set *wfds,
+ const fd_set *xfds)
+{
+ return qemu_process_fd_handlers2(&io_handlers, rfds, wfds, xfds);
+}
+
/***********************************************************/
/* machine registration */
@@ -1326,7 +1303,6 @@ void qemu_system_vmstop_request(int reason)
void main_loop_wait(int nonblocking)
{
- IOHandlerRecord *ioh;
fd_set rfds, wfds, xfds;
int ret, nfds;
struct timeval tv;
@@ -1347,22 +1323,7 @@ void main_loop_wait(int nonblocking)
FD_ZERO(&rfds);
FD_ZERO(&wfds);
FD_ZERO(&xfds);
- QLIST_FOREACH(ioh, &io_handlers, next) {
- if (ioh->deleted)
- continue;
- if (ioh->fd_read &&
- (!ioh->fd_read_poll ||
- ioh->fd_read_poll(ioh->opaque) != 0)) {
- FD_SET(ioh->fd, &rfds);
- if (ioh->fd > nfds)
- nfds = ioh->fd;
- }
- if (ioh->fd_write) {
- FD_SET(ioh->fd, &wfds);
- if (ioh->fd > nfds)
- nfds = ioh->fd;
- }
- }
+ qemu_get_fdset(&nfds, &rfds, &wfds, &xfds);
tv.tv_sec = timeout / 1000;
tv.tv_usec = (timeout % 1000) * 1000;
@@ -1373,22 +1334,7 @@ void main_loop_wait(int nonblocking)
ret = select(nfds + 1, &rfds, &wfds, &xfds, &tv);
qemu_mutex_lock_iothread();
if (ret > 0) {
- IOHandlerRecord *pioh;
-
- QLIST_FOREACH_SAFE(ioh, &io_handlers, next, pioh) {
- if (!ioh->deleted && ioh->fd_read && FD_ISSET(ioh->fd, &rfds)) {
- ioh->fd_read(ioh->opaque);
- }
- if (!ioh->deleted && ioh->fd_write && FD_ISSET(ioh->fd, &wfds)) {
- ioh->fd_write(ioh->opaque);
- }
-
- /* Do this last in case read/write handlers marked it for deletion */
- if (ioh->deleted) {
- QLIST_REMOVE(ioh, next);
- qemu_free(ioh);
- }
- }
+ qemu_process_fd_handlers(&rfds, &wfds, &xfds);
}
slirp_select_poll(&rfds, &wfds, &xfds, (ret < 0));
--
1.7.0.4
^ permalink raw reply related [flat|nested] 41+ messages in thread
* [Qemu-devel] [RFC][PATCH v7 02/16] Add qemu_set_fd_handler() wrappers to qemu-tools.c
2011-03-07 20:10 [Qemu-devel] [RFC][PATCH v7 00/16] virtagent: host/guest communication agent Michael Roth
2011-03-07 20:10 ` [Qemu-devel] [RFC][PATCH v7 01/16] Move code related to fd handlers into utility functions Michael Roth
@ 2011-03-07 20:10 ` Michael Roth
2011-03-07 20:10 ` [Qemu-devel] [RFC][PATCH v7 03/16] Make qemu timers available for tools Michael Roth
` (14 subsequent siblings)
16 siblings, 0 replies; 41+ messages in thread
From: Michael Roth @ 2011-03-07 20:10 UTC (permalink / raw)
To: qemu-devel
Cc: agl, stefanha, Jes.Sorensen, mdroth, markus_mueller, aliguori,
abeekhof
This adds state information for managing fd handlers to qemu-tools.c so
that tools that build against it can implement an I/O loop for
interacting with objects that use qemu_set_fd_handler()
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
qemu-tool.c | 25 ++++++++++++++++++++++++-
1 files changed, 24 insertions(+), 1 deletions(-)
diff --git a/qemu-tool.c b/qemu-tool.c
index 392e1c9..78d3532 100644
--- a/qemu-tool.c
+++ b/qemu-tool.c
@@ -22,6 +22,8 @@
QEMUClock *rt_clock;
FILE *logfile;
+static QLIST_HEAD(, IOHandlerRecord) io_handlers =
+ QLIST_HEAD_INITIALIZER(io_handlers);
struct QEMUBH
{
@@ -103,11 +105,32 @@ void qemu_bh_delete(QEMUBH *bh)
qemu_free(bh);
}
+/* definitions to implement i/o loop for fd handlers in tools */
int qemu_set_fd_handler2(int fd,
IOCanReadHandler *fd_read_poll,
IOHandler *fd_read,
IOHandler *fd_write,
void *opaque)
{
- return 0;
+ return qemu_set_fd_handler3(&io_handlers, fd, fd_read_poll, fd_read,
+ fd_write, opaque);
+}
+
+int qemu_set_fd_handler(int fd,
+ IOHandler *fd_read,
+ IOHandler *fd_write,
+ void *opaque)
+{
+ return qemu_set_fd_handler2(fd, NULL, fd_read, fd_write, opaque);
+}
+
+void qemu_get_fdset(int *nfds, fd_set *rfds, fd_set *wfds, fd_set *xfds)
+{
+ return qemu_get_fdset2(&io_handlers, nfds, rfds, wfds, xfds);
+}
+
+void qemu_process_fd_handlers(const fd_set *rfds, const fd_set *wfds,
+ const fd_set *xfds)
+{
+ return qemu_process_fd_handlers2(&io_handlers, rfds, wfds, xfds);
}
--
1.7.0.4
^ permalink raw reply related [flat|nested] 41+ messages in thread
* [Qemu-devel] [RFC][PATCH v7 03/16] Make qemu timers available for tools
2011-03-07 20:10 [Qemu-devel] [RFC][PATCH v7 00/16] virtagent: host/guest communication agent Michael Roth
2011-03-07 20:10 ` [Qemu-devel] [RFC][PATCH v7 01/16] Move code related to fd handlers into utility functions Michael Roth
2011-03-07 20:10 ` [Qemu-devel] [RFC][PATCH v7 02/16] Add qemu_set_fd_handler() wrappers to qemu-tools.c Michael Roth
@ 2011-03-07 20:10 ` Michael Roth
2011-03-09 10:33 ` [Qemu-devel] " Jes Sorensen
2011-03-07 20:10 ` [Qemu-devel] [RFC][PATCH v7 04/16] virtagent: bi-directional RPC handling logic Michael Roth
` (13 subsequent siblings)
16 siblings, 1 reply; 41+ messages in thread
From: Michael Roth @ 2011-03-07 20:10 UTC (permalink / raw)
To: qemu-devel
Cc: agl, stefanha, Jes.Sorensen, mdroth, markus_mueller, aliguori,
abeekhof
To be able to use qemu_mod_timer() and friends to register timeout
events for virtagent's qemu-va tool, we need to do the following:
Move several blocks of code out of cpus.c that handle initialization
of qemu's io_thread_fd and working with it via
qemu_notify_event()/qemu_event_read()/etc, and make them accessible
as backend functions to both the emulator code and qemu-tool.c via
wrapper functions within cpus.c and qemu-tool.c, respectively. These
have been added to qemu-ioh.c, where similar treatment was given to
qemu_set_fd_handler() and friends.
Some of these wrapper functions lack declarations when being
built into tools, so we add those via qemu-tool.h, which can be included
by a tool to access them. With these changes we can drive timers in a
tool linking it against qemu-timer.o and then implementing something
similar to the main i/o loop in vl.c:
init_clocks();
configure_alarms("dynticks");
if (init_timer_alarm() < 0) {
errx(EXIT_FAILURE, "could not initialize alarm timer");
}
while (running) {
//do work
qemu_run_all_timers();
}
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
cpus.c | 83 +++++++--------------------------------------------
qemu-ioh.c | 95 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
qemu-ioh.h | 9 +++++
qemu-tool.c | 92 +++++++++++++++++++++++++++++++++++++++++++++++++++++++-
qemu-tool.h | 26 ++++++++++++++++
5 files changed, 231 insertions(+), 74 deletions(-)
create mode 100644 qemu-tool.h
diff --git a/cpus.c b/cpus.c
index 0f33945..507a660 100644
--- a/cpus.c
+++ b/cpus.c
@@ -246,64 +246,12 @@ static int io_thread_fd = -1;
static void qemu_event_increment(void)
{
- /* Write 8 bytes to be compatible with eventfd. */
- static const uint64_t val = 1;
- ssize_t ret;
-
- if (io_thread_fd == -1) {
- return;
- }
- do {
- ret = write(io_thread_fd, &val, sizeof(val));
- } while (ret < 0 && errno == EINTR);
-
- /* EAGAIN is fine, a read must be pending. */
- if (ret < 0 && errno != EAGAIN) {
- fprintf(stderr, "qemu_event_increment: write() filed: %s\n",
- strerror(errno));
- exit (1);
- }
-}
-
-static void qemu_event_read(void *opaque)
-{
- int fd = (unsigned long)opaque;
- ssize_t len;
- char buffer[512];
-
- /* Drain the notify pipe. For eventfd, only 8 bytes will be read. */
- do {
- len = read(fd, buffer, sizeof(buffer));
- } while ((len == -1 && errno == EINTR) || len == sizeof(buffer));
+ return iothread_event_increment(&io_thread_fd);
}
static int qemu_event_init(void)
{
- int err;
- int fds[2];
-
- err = qemu_eventfd(fds);
- if (err == -1) {
- return -errno;
- }
- err = fcntl_setfl(fds[0], O_NONBLOCK);
- if (err < 0) {
- goto fail;
- }
- err = fcntl_setfl(fds[1], O_NONBLOCK);
- if (err < 0) {
- goto fail;
- }
- qemu_set_fd_handler2(fds[0], NULL, qemu_event_read, NULL,
- (void *)(unsigned long)fds[0]);
-
- io_thread_fd = fds[1];
- return 0;
-
-fail:
- close(fds[0]);
- close(fds[1]);
- return err;
+ return iothread_event_init(&io_thread_fd);
}
static void dummy_signal(int sig)
@@ -410,28 +358,14 @@ static void qemu_kvm_eat_signals(CPUState *env)
HANDLE qemu_event_handle;
-static void dummy_event_handler(void *opaque)
-{
-}
-
static int qemu_event_init(void)
{
- qemu_event_handle = CreateEvent(NULL, FALSE, FALSE, NULL);
- if (!qemu_event_handle) {
- fprintf(stderr, "Failed CreateEvent: %ld\n", GetLastError());
- return -1;
- }
- qemu_add_wait_object(qemu_event_handle, dummy_event_handler, NULL);
- return 0;
+ return win32_event_init(&qemu_event_handle);
}
static void qemu_event_increment(void)
{
- if (!SetEvent(qemu_event_handle)) {
- fprintf(stderr, "qemu_event_increment: SetEvent failed: %ld\n",
- GetLastError());
- exit (1);
- }
+ win32_event_increment(&qemu_event_handle);
}
static void qemu_kvm_eat_signals(CPUState *env)
@@ -564,11 +498,10 @@ void qemu_cpu_kick_self(void)
#endif
}
-void qemu_notify_event(void)
+static void qemu_stop_all_vcpus(void)
{
CPUState *env = cpu_single_env;
- qemu_event_increment ();
if (env) {
cpu_exit(env);
}
@@ -578,6 +511,12 @@ void qemu_notify_event(void)
exit_request = 1;
}
+void qemu_notify_event(void)
+{
+ qemu_event_increment();
+ qemu_stop_all_vcpus();
+}
+
void qemu_mutex_lock_iothread(void) {}
void qemu_mutex_unlock_iothread(void) {}
diff --git a/qemu-ioh.c b/qemu-ioh.c
index cc71470..5c3f94c 100644
--- a/qemu-ioh.c
+++ b/qemu-ioh.c
@@ -22,7 +22,11 @@
* THE SOFTWARE.
*/
#include "qemu-ioh.h"
+#include "qemu-char.h"
#include "qlist.h"
+#ifdef CONFIG_EVENTFD
+#include <sys/eventfd.h>
+#endif
/* XXX: fd_read_poll should be suppressed, but an API change is
necessary in the character devices to suppress fd_can_read(). */
@@ -113,3 +117,94 @@ void qemu_process_fd_handlers2(void *ioh_record_list, const fd_set *rfds,
}
}
}
+
+#ifndef _WIN32
+void iothread_event_increment(int *io_thread_fd)
+{
+ /* Write 8 bytes to be compatible with eventfd. */
+ static const uint64_t val = 1;
+ ssize_t ret;
+
+ if (*io_thread_fd == -1) {
+ return;
+ }
+
+ do {
+ ret = write(*io_thread_fd, &val, sizeof(val));
+ } while (ret < 0 && errno == EINTR);
+
+ /* EAGAIN is fine, a read must be pending. */
+ if (ret < 0 && errno != EAGAIN) {
+ fprintf(stderr, "qemu_event_increment: write() filed: %s\n",
+ strerror(errno));
+ exit (1);
+ }
+}
+
+static void qemu_event_read(void *opaque)
+{
+ int fd = (unsigned long)opaque;
+ ssize_t len;
+ char buffer[512];
+
+ /* Drain the notify pipe. For eventfd, only 8 bytes will be read. */
+ do {
+ len = read(fd, buffer, sizeof(buffer));
+ } while (len == -1 && errno == EINTR);
+}
+
+
+int iothread_event_init(int *io_thread_fd)
+{
+ int err;
+ int fds[2];
+
+ err = qemu_eventfd(fds);
+ if (err == -1)
+ return -errno;
+
+ err = fcntl_setfl(fds[0], O_NONBLOCK);
+ if (err < 0)
+ goto fail;
+
+ err = fcntl_setfl(fds[1], O_NONBLOCK);
+ if (err < 0) {
+ goto fail;
+ }
+
+ qemu_set_fd_handler2(fds[0], NULL, qemu_event_read, NULL,
+ (void *)(unsigned long)fds[0]);
+
+ *io_thread_fd = fds[1];
+ return 0;
+
+fail:
+ close(fds[0]);
+ close(fds[1]);
+ return err;
+}
+#else
+static void dummy_event_handler(void *opaque)
+{
+}
+
+int win32_event_init(HANDLE *qemu_event_handle)
+{
+ *qemu_event_handle = CreateEvent(NULL, FALSE, FALSE, NULL);
+ if (!qemu_event_handle) {
+ fprintf(stderr, "Failed CreateEvent: %ld\n", GetLastError());
+ return -1;
+ }
+ qemu_add_wait_object(*qemu_event_handle, dummy_event_handler, NULL);
+ return 0;
+}
+
+void win32_event_increment(HANDLE *qemu_event_handle)
+{
+ if (!SetEvent(*qemu_event_handle)) {
+ fprintf(stderr, "qemu_event_increment: SetEvent failed: %ld\n",
+ GetLastError());
+ exit (1);
+ }
+}
+#endif
diff --git a/qemu-ioh.h b/qemu-ioh.h
index 7c6e833..2c714a9 100644
--- a/qemu-ioh.h
+++ b/qemu-ioh.h
@@ -31,4 +31,13 @@ void qemu_get_fdset2(void *ioh_record_list, int *nfds, fd_set *rfds,
void qemu_process_fd_handlers2(void *ioh_record_list, const fd_set *rfds,
const fd_set *wfds, const fd_set *xfds);
+
+#ifndef _WIN32
+void iothread_event_increment(int *io_thread_fd);
+int iothread_event_init(int *io_thread_fd);
+#else
+int win32_event_init(HANDLE *qemu_event_handle);
+void win32_event_increment(HANDLE *qemu_event_handle);
+#endif
+
#endif
diff --git a/qemu-tool.c b/qemu-tool.c
index 78d3532..027ea31 100644
--- a/qemu-tool.c
+++ b/qemu-tool.c
@@ -12,6 +12,7 @@
*/
#include "qemu-common.h"
+#include "qemu-tool.h"
#include "monitor.h"
#include "qemu-timer.h"
#include "qemu-log.h"
@@ -19,12 +20,11 @@
#include <sys/time.h>
-QEMUClock *rt_clock;
+QEMUClock *rtc_clock;
FILE *logfile;
static QLIST_HEAD(, IOHandlerRecord) io_handlers =
QLIST_HEAD_INITIALIZER(io_handlers);
-
struct QEMUBH
{
QEMUBHFunc *cb;
@@ -134,3 +134,91 @@ void qemu_process_fd_handlers(const fd_set *rfds, const fd_set *wfds,
{
return qemu_process_fd_handlers2(&io_handlers, rfds, wfds, xfds);
}
+
+#ifndef _WIN32
+static int io_thread_fd = -1;
+
+void qemu_event_increment(void)
+{
+ return iothread_event_increment(&io_thread_fd);
+}
+
+int qemu_event_init(void)
+{
+ return iothread_event_init(&io_thread_fd);
+}
+#else
+HANDLE qemu_event_handle;
+
+int qemu_event_init(void)
+{
+ return win32_event_init(&qemu_event_handle);
+}
+
+void qemu_event_increment(void)
+{
+ win32_event_increment(&qemu_event_handle);
+}
+#endif
+
+void qemu_notify_event(void)
+{
+ qemu_event_increment ();
+}
+
+/*
+ * Creates an eventfd that looks like a pipe and has EFD_CLOEXEC set.
+ */
+int qemu_eventfd(int fds[2])
+{
+#ifdef CONFIG_EVENTFD
+ int ret;
+
+ ret = eventfd(0, 0);
+ if (ret >= 0) {
+ fds[0] = ret;
+ qemu_set_cloexec(ret);
+ if ((fds[1] = dup(ret)) == -1) {
+ close(ret);
+ return -1;
+ }
+ qemu_set_cloexec(fds[1]);
+ return 0;
+ }
+
+ if (errno != ENOSYS) {
+ return -1;
+ }
+#endif
+
+ return qemu_pipe(fds);
+}
+
+void qemu_put_be64(QEMUFile *f, uint64_t v)
+{
+}
+
+uint64_t qemu_get_be64(QEMUFile *f)
+{
+ return 0;
+}
+
+const VMStateInfo vmstate_info_int64;
+int use_icount = 0;
+int vm_running = 1;
+int64_t qemu_icount;
+
+int vmstate_register(DeviceState *dev, int instance_id,
+ const VMStateDescription *vmsd, void *opaque)
+{
+ return 0;
+}
+int64_t cpu_get_icount(void) {
+ return 0;
+}
+
+VMChangeStateEntry *qemu_add_vm_change_state_handler(VMChangeStateHandler *cb,
+ void *opaque)
+{
+ return NULL;
+}
diff --git a/qemu-tool.h b/qemu-tool.h
new file mode 100644
index 0000000..fd693cf
--- /dev/null
+++ b/qemu-tool.h
@@ -0,0 +1,26 @@
+#ifndef QEMU_TOOL_H
+#define QEMU_TOOL_H
+
+#include "qemu-common.h"
+
+#ifdef CONFIG_EVENTFD
+#include <sys/eventfd.h>
+#endif
+
+typedef void VMStateDescription;
+typedef int VMStateInfo;
+
+#ifndef _WIN32
+void qemu_event_increment(void);
+int qemu_event_init(void);
+#else
+int qemu_event_init(void);
+void qemu_event_increment(void);
+#endif
+
+void qemu_put_be64(QEMUFile *f, uint64_t v);
+uint64_t qemu_get_be64(QEMUFile *f);
+int vmstate_register(DeviceState *dev, int instance_id,
+ const VMStateDescription *vmsd, void *opaque);
+
+#endif
--
1.7.0.4
^ permalink raw reply related [flat|nested] 41+ messages in thread
* [Qemu-devel] [RFC][PATCH v7 04/16] virtagent: bi-directional RPC handling logic
2011-03-07 20:10 [Qemu-devel] [RFC][PATCH v7 00/16] virtagent: host/guest communication agent Michael Roth
` (2 preceding siblings ...)
2011-03-07 20:10 ` [Qemu-devel] [RFC][PATCH v7 03/16] Make qemu timers available for tools Michael Roth
@ 2011-03-07 20:10 ` Michael Roth
2011-03-07 21:24 ` [Qemu-devel] " Adam Litke
2011-03-07 20:10 ` [Qemu-devel] [RFC][PATCH v7 05/16] virtagent: common helpers and init routines Michael Roth
` (12 subsequent siblings)
16 siblings, 1 reply; 41+ messages in thread
From: Michael Roth @ 2011-03-07 20:10 UTC (permalink / raw)
To: qemu-devel
Cc: agl, stefanha, Jes.Sorensen, mdroth, markus_mueller, aliguori,
abeekhof
This implements the state machine/logic used to manage
send/receive/execute phases of RPCs we send or receive. It does so using
a set of abstract methods we implement with the application and
transport level code which will follow.
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
virtagent-manager.c | 326 +++++++++++++++++++++++++++++++++++++++++++++++++++
virtagent-manager.h | 130 ++++++++++++++++++++
2 files changed, 456 insertions(+), 0 deletions(-)
create mode 100644 virtagent-manager.c
create mode 100644 virtagent-manager.h
diff --git a/virtagent-manager.c b/virtagent-manager.c
new file mode 100644
index 0000000..51d26a3
--- /dev/null
+++ b/virtagent-manager.c
@@ -0,0 +1,326 @@
+/*
+ * virtagent - job queue management
+ *
+ * Copyright IBM Corp. 2011
+ *
+ * Authors:
+ * Michael Roth <mdroth@linux.vnet.ibm.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ *
+ */
+
+#include "virtagent-common.h"
+
+typedef struct VAServerJob {
+ char tag[64];
+ void *opaque;
+ VAServerJobOps ops;
+ QTAILQ_ENTRY(VAServerJob) next;
+ enum {
+ VA_SERVER_JOB_STATE_NEW = 0,
+ VA_SERVER_JOB_STATE_BUSY,
+ VA_SERVER_JOB_STATE_EXECUTED,
+ VA_SERVER_JOB_STATE_SENT,
+ VA_SERVER_JOB_STATE_DONE,
+ } state;
+} VAServerJob;
+
+typedef struct VAClientJob {
+ char tag[64];
+ void *opaque;
+ void *resp_opaque;
+ VAClientJobOps ops;
+ QTAILQ_ENTRY(VAClientJob) next;
+ enum {
+ VA_CLIENT_JOB_STATE_NEW = 0,
+ VA_CLIENT_JOB_STATE_BUSY,
+ VA_CLIENT_JOB_STATE_SENT,
+ VA_CLIENT_JOB_STATE_READ,
+ VA_CLIENT_JOB_STATE_DONE,
+ } state;
+} VAClientJob;
+
+#define SEND_COUNT_MAX 1
+#define EXECUTE_COUNT_MAX 4
+
+struct VAManager {
+ int send_count; /* sends in flight */
+ int execute_count; /* number of jobs currently executing */
+ QTAILQ_HEAD(, VAServerJob) server_jobs;
+ QTAILQ_HEAD(, VAClientJob) client_jobs;
+};
+
+/* server job operations/helpers */
+
+static VAServerJob *va_server_job_by_tag(VAManager *m, const char *tag)
+{
+ VAServerJob *j;
+ QTAILQ_FOREACH(j, &m->server_jobs, next) {
+ if (strcmp(j->tag, tag) == 0) {
+ return j;
+ }
+ }
+ return NULL;
+}
+
+int va_server_job_add(VAManager *m, const char *tag, void *opaque,
+ VAServerJobOps ops)
+{
+ VAServerJob *j = qemu_mallocz(sizeof(VAServerJob));
+ TRACE("called");
+ j->state = VA_SERVER_JOB_STATE_NEW;
+ j->ops = ops;
+ j->opaque = opaque;
+ memset(j->tag, 0, 64);
+ pstrcpy(j->tag, 63, tag);
+ QTAILQ_INSERT_TAIL(&m->server_jobs, j, next);
+ va_kick(m);
+ return 0;
+}
+
+static void va_server_job_execute(VAServerJob *j)
+{
+ TRACE("called");
+ j->state = VA_SERVER_JOB_STATE_BUSY;
+ j->ops.execute(j->opaque, j->tag);
+}
+
+/* TODO: need a way to pass information back */
+void va_server_job_execute_done(VAManager *m, const char *tag)
+{
+ VAServerJob *j = va_server_job_by_tag(m, tag);
+ TRACE("called");
+ if (!j) {
+ LOG("server job with tag \"%s\" not found", tag);
+ return;
+ }
+ j->state = VA_SERVER_JOB_STATE_EXECUTED;
+ va_kick(m);
+}
+
+static void va_server_job_send(VAServerJob *j)
+{
+ TRACE("called");
+ j->state = VA_SERVER_JOB_STATE_BUSY;
+ j->ops.send(j->opaque, j->tag);
+}
+
+void va_server_job_send_done(VAManager *m, const char *tag)
+{
+ VAServerJob *j = va_server_job_by_tag(m, tag);
+ TRACE("called");
+ if (!j) {
+ LOG("server job with tag \"%s\" not found", tag);
+ return;
+ }
+ j->state = VA_SERVER_JOB_STATE_SENT;
+ m->send_count--;
+ va_kick(m);
+}
+
+static void va_server_job_callback(VAServerJob *j)
+{
+ TRACE("called");
+ j->state = VA_SERVER_JOB_STATE_BUSY;
+ if (j->ops.callback) {
+ j->ops.callback(j->opaque, j->tag);
+ }
+ j->state = VA_SERVER_JOB_STATE_DONE;
+}
+
+void va_server_job_cancel(VAManager *m, const char *tag)
+{
+ VAServerJob *j = va_server_job_by_tag(m, tag);
+ TRACE("called");
+ if (!j) {
+ LOG("server job with tag \"%s\" not found", tag);
+ return;
+ }
+ /* TODO: need to decrement sends/execs in flight appropriately */
+ /* make callback and move to done state, kick() will handle cleanup */
+ va_server_job_callback(j);
+ va_kick(m);
+}
+
+/* client job operations */
+
+static VAClientJob *va_client_job_by_tag(VAManager *m, const char *tag)
+{
+ VAClientJob *j;
+ QTAILQ_FOREACH(j, &m->client_jobs, next) {
+ if (strcmp(j->tag, tag) == 0) {
+ return j;
+ }
+ }
+ return NULL;
+}
+
+int va_client_job_add(VAManager *m, const char *tag, void *opaque,
+ VAClientJobOps ops)
+{
+ VAClientJob *j = qemu_mallocz(sizeof(VAClientJob));
+ TRACE("called");
+ j->ops = ops;
+ j->opaque = opaque;
+ memset(j->tag, 0, 64);
+ pstrcpy(j->tag, 63, tag);
+ QTAILQ_INSERT_TAIL(&m->client_jobs, j, next);
+ va_kick(m);
+ return 0;
+}
+
+static void va_client_job_send(VAClientJob *j)
+{
+ TRACE("called");
+ j->state = VA_CLIENT_JOB_STATE_BUSY;
+ j->ops.send(j->opaque, j->tag);
+}
+
+void va_client_job_send_done(VAManager *m, const char *tag)
+{
+ VAClientJob *j = va_client_job_by_tag(m, tag);
+ TRACE("called");
+ if (!j) {
+ LOG("client job with tag \"%s\" not found", tag);
+ return;
+ }
+ j->state = VA_CLIENT_JOB_STATE_SENT;
+ m->send_count--;
+ va_kick(m);
+}
+
+void va_client_job_read_done(VAManager *m, const char *tag, void *resp)
+{
+ VAClientJob *j = va_client_job_by_tag(m, tag);
+ TRACE("called");
+ if (!j) {
+ LOG("client job with tag \"%s\" not found", tag);
+ return;
+ }
+ j->state = VA_CLIENT_JOB_STATE_READ;
+ j->resp_opaque = resp;
+ va_kick(m);
+}
+
+static void va_client_job_callback(VAClientJob *j)
+{
+ TRACE("called");
+ j->state = VA_CLIENT_JOB_STATE_BUSY;
+ if (j->ops.callback) {
+ j->ops.callback(j->opaque, j->resp_opaque, j->tag);
+ }
+ j->state = VA_CLIENT_JOB_STATE_DONE;
+}
+
+void va_client_job_cancel(VAManager *m, const char *tag)
+{
+ VAClientJob *j = va_client_job_by_tag(m, tag);
+ TRACE("called");
+ if (!j) {
+ LOG("client job with tag \"%s\" not found", tag);
+ return;
+ }
+ /* TODO: need to decrement sends/execs in flight appropriately */
+ /* make callback and move to done state, kick() will handle cleanup */
+ va_client_job_callback(j);
+ va_kick(m);
+}
+
+/* general management functions */
+
+VAManager *va_manager_new(void)
+{
+ VAManager *m = qemu_mallocz(sizeof(VAManager));
+ QTAILQ_INIT(&m->client_jobs);
+ QTAILQ_INIT(&m->server_jobs);
+ return m;
+}
+
+static void va_process_server_job(VAManager *m, VAServerJob *sj)
+{
+ switch (sj->state) {
+ case VA_SERVER_JOB_STATE_NEW:
+ TRACE("marker");
+ va_server_job_execute(sj);
+ break;
+ case VA_SERVER_JOB_STATE_EXECUTED:
+ TRACE("marker");
+ if (m->send_count < SEND_COUNT_MAX) {
+ TRACE("marker");
+ va_server_job_send(sj);
+ m->send_count++;
+ }
+ break;
+ case VA_SERVER_JOB_STATE_SENT:
+ TRACE("marker");
+ va_server_job_callback(sj);
+ break;
+ case VA_SERVER_JOB_STATE_BUSY:
+ TRACE("marker, server job currently busy");
+ break;
+ case VA_SERVER_JOB_STATE_DONE:
+ TRACE("marker");
+ QTAILQ_REMOVE(&m->server_jobs, sj, next);
+ break;
+ default:
+ LOG("error, unknown server job state");
+ break;
+ }
+}
+
+static void va_process_client_job(VAManager *m, VAClientJob *cj)
+{
+ switch (cj->state) {
+ case VA_CLIENT_JOB_STATE_NEW:
+ TRACE("marker");
+ if (m->send_count < SEND_COUNT_MAX) {
+ TRACE("marker");
+ va_client_job_send(cj);
+ m->send_count++;
+ }
+ break;
+ case VA_CLIENT_JOB_STATE_SENT:
+ TRACE("marker");
+ //nothing to do here, awaiting read_done()
+ break;
+ case VA_CLIENT_JOB_STATE_READ:
+ TRACE("marker");
+ va_client_job_callback(cj);
+ break;
+ case VA_CLIENT_JOB_STATE_DONE:
+ TRACE("marker");
+ QTAILQ_REMOVE(&m->client_jobs, cj, next);
+ break;
+ case VA_CLIENT_JOB_STATE_BUSY:
+ TRACE("marker, client job currently busy");
+ break;
+ default:
+ LOG("error, unknown client job state");
+ break;
+ }
+}
+
+void va_kick(VAManager *m)
+{
+ VAServerJob *sj, *sj_tmp;
+ VAClientJob *cj, *cj_tmp;
+
+ TRACE("called");
+ TRACE("send_count: %u, execute_count: %u", m->send_count, m->execute_count);
+
+ /* TODO: make sure there is no starvation of jobs/operations here */
+
+ /* look for any work to be done among pending server jobs */
+ QTAILQ_FOREACH_SAFE(sj, &m->server_jobs, next, sj_tmp) {
+ TRACE("marker, server tag: %s", sj->tag);
+ va_process_server_job(m, sj);
+ }
+
+ /* look for work to be done among pending client jobs */
+ QTAILQ_FOREACH_SAFE(cj, &m->client_jobs, next, cj_tmp) {
+ TRACE("marker, client tag: %s", cj->tag);
+ va_process_client_job(m, cj);
+ }
+}
diff --git a/virtagent-manager.h b/virtagent-manager.h
new file mode 100644
index 0000000..7b463fb
--- /dev/null
+++ b/virtagent-manager.h
@@ -0,0 +1,130 @@
+#ifndef VIRTAGENT_MANAGER_H
+#define VIRTAGENT_MANAGER_H
+
+#include "qemu-common.h"
+#include "qemu-queue.h"
+
+/*
+ * Protocol Overview:
+ *
+ * The virtagent protocol depends on a state machine to manage communication
+ * over a single connection stream, currently a virtio or isa serial channel.
+ * The basic characterization of the work being done is that clients
+ * send/handle client jobs locally, which are then read/handled remotely as
+ * server jobs. A client job consists of a request which is sent, and a
+ * response which is eventually recieved. A server job consists of a request
+ * which is recieved from the other end, and a response which is sent back.
+ *
+ * Server jobs are given priority over client jobs, i.e. if we send a client
+ * job (our request) and recieve a server job (their request), rather than
+ * await a response to the client job, we immediately begin processing the
+ * server job and then send back the response. This prevents us from being
+ * deadlocked in a situation where both sides have sent a client job and are
+ * awaiting the response before handling the other side's client job.
+ *
+ * Multiple in-flight requests are supported, but high request rates can
+ * potentially starve out the other side's client jobs / requests, so we'll
+ * behaved participants should periodically backoff on high request rates, or
+ * limit themselves to 1 request at a time (anything more than 1 can still
+ * potentionally remove any window for the other end to service it's own
+ * client jobs, since we can begin sending the next request before it begins
+ * send the response for the 2nd).
+ *
+ * On a related note, in the future, bidirectional user/session-level guest
+ * agents may also be supported via a forwarding service made available
+ * through the system-level guest agent. In this case it is up to the
+ * system-level agent to handle forwarding requests in such a way that we
+ * don't starve the host-side service out sheerly by having too many
+ * sessions/users trying to send RPCs at a constant rate. This would be
+ * supported through this job Manager via an additional "forwarder" job type.
+ *
+ * To encapsulate some of this logic, we define here a "Manager" class, which
+ * provides an abstract interface to a state machine which handles most of
+ * the above logic transparently to the transport/application-level code.
+ * This also makes it possible to utilize alternative
+ * transport/application-level protocols in the future.
+ *
+ */
+
+/*
+ * Two types of jobs are generated from various components of virtagent.
+ * Each job type has a priority, and a set of prioritized functions as well.
+ *
+ * The read handler generates new server jobs as it recieves requests from
+ * the channel. Server jobs make progress through the following operations.
+ *
+ * EXECUTE->EXECUTE_DONE->SEND->SEND_DONE
+ *
+ * EXECUTE (provided by user, manager calls)
+ * When server jobs are added, eventually (as execution slots become
+ * available) an execute() will be called to begin executing the job. An
+ * error value will be returned if there is no room in the queue for another
+ * server job.
+ *
+ * EXECUTE_DONE (provided by manager, user calls)
+ * As server jobs complete, execute_completed() is called to update execution
+ * status of that job (failure/success), inject the payload, and kick off the
+ * next operation.
+ *
+ * SEND (provided by user, manager calls)
+ * Eventually the send() operation is made. This will cause the send handler
+ * to begin sending the response.
+ *
+ * SEND_DONE (provided by manager, user calls)
+ * Upon completion of that send, the send_completed() operation will be
+ * called. This will free up the job, and kick off the next operation.
+ */
+typedef int (va_job_op)(void *opaque, const char *tag);
+typedef struct VAServerJobOps {
+ va_job_op *execute;
+ va_job_op *send;
+ va_job_op *callback;
+} VAServerJobOps;
+
+/*
+ * The client component generates new client jobs as they're made by
+ * virtagent in response to monitored events or user-issued commands.
+ * Client jobs progress via the following operations.
+ *
+ * SEND->SEND_DONE->READ_DONE
+ *
+ * SEND (provided by user, called by manager)
+ * After client jobs are added, send() will eventually be called to queue
+ * the job up for xmit over the channel.
+ *
+ * SEND_DONE (provided by manager, called by user)
+ * Upon completion of the send, send_completed() should be called with
+ * failure/success indication.
+ *
+ * READ_DONE (provided by manager, called by user)
+ * When a response for the request is read back via the transport layer,
+ * read_done() will be called by the user to indicate success/failure,
+ * inject the response, and make the associated callback.
+ */
+typedef int (va_client_job_cb)(void *opaque, void *resp_opaque,
+ const char *tag);
+typedef struct VAClientJobOps {
+ va_job_op *send;
+ va_client_job_cb *callback;
+} VAClientJobOps;
+
+typedef struct VAManager VAManager;
+
+VAManager *va_manager_new(void);
+void va_kick(VAManager *m);
+
+/* interfaces for server jobs */
+int va_server_job_add(VAManager *m, const char *tag, void *opaque,
+ VAServerJobOps ops);
+void va_server_job_execute_done(VAManager *m, const char *tag);
+void va_server_job_send_done(VAManager *m, const char *tag);
+void va_server_job_cancel(VAManager *m, const char *tag);
+
+/* interfaces for client jobs */
+int va_client_job_add(VAManager *m, const char *tag, void *opaque,
+ VAClientJobOps ops);
+void va_client_job_cancel(VAManager *m, const char *tag);
+void va_client_job_send_done(VAManager *m, const char *tag);
+void va_client_job_read_done(VAManager *m, const char *tag, void *resp);
+
+#endif /* VIRTAGENT_MANAGER_H */
--
1.7.0.4
^ permalink raw reply related [flat|nested] 41+ messages in thread
* [Qemu-devel] [RFC][PATCH v7 05/16] virtagent: common helpers and init routines
2011-03-07 20:10 [Qemu-devel] [RFC][PATCH v7 00/16] virtagent: host/guest communication agent Michael Roth
` (3 preceding siblings ...)
2011-03-07 20:10 ` [Qemu-devel] [RFC][PATCH v7 04/16] virtagent: bi-directional RPC handling logic Michael Roth
@ 2011-03-07 20:10 ` Michael Roth
2011-03-09 10:38 ` [Qemu-devel] " Jes Sorensen
2011-03-07 20:10 ` [Qemu-devel] [RFC][PATCH v7 06/16] virtagent: transport definitions Michael Roth
` (11 subsequent siblings)
16 siblings, 1 reply; 41+ messages in thread
From: Michael Roth @ 2011-03-07 20:10 UTC (permalink / raw)
To: qemu-devel
Cc: agl, stefanha, Jes.Sorensen, mdroth, markus_mueller, aliguori,
abeekhof
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
virtagent-common.c | 206 ++++++++++++++++++++++++++++++++++++++++++++++++++++
virtagent-common.h | 95 ++++++++++++++++++++++++
2 files changed, 301 insertions(+), 0 deletions(-)
create mode 100644 virtagent-common.c
create mode 100644 virtagent-common.h
diff --git a/virtagent-common.c b/virtagent-common.c
new file mode 100644
index 0000000..4b13ee8
--- /dev/null
+++ b/virtagent-common.c
@@ -0,0 +1,206 @@
+/*
+ * virtagent - common host/guest functions
+ *
+ * Copyright IBM Corp. 2010
+ *
+ * Authors:
+ * Adam Litke <aglitke@linux.vnet.ibm.com>
+ * Michael Roth <mdroth@linux.vnet.ibm.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ *
+ */
+
+#include "virtagent-common.h"
+
+VAState *va_state;
+
+/* helper to avoid tedious key/type checking on QDict entries */
+bool va_qdict_haskey_with_type(const QDict *qdict, const char *key,
+ qtype_code type)
+{
+ QObject *qobj;
+ if (!qdict) {
+ return false;
+ }
+ if (!qdict_haskey(qdict, key)) {
+ return false;
+ }
+ qobj = qdict_get(qdict, key);
+ if (qobject_type(qobj) != type) {
+ return false;
+ }
+
+ return true;
+}
+
+static void va_qdict_insert(const char *key, QObject *entry, void *opaque)
+{
+ QDict *dict = opaque;
+
+ if (key && entry) {
+ qdict_put_obj(dict, key, entry);
+ }
+}
+
+QDict *va_qdict_copy(const QDict *old)
+{
+ QDict *new;
+
+ if (!old) {
+ return NULL;
+ }
+
+ new = qdict_new();
+ qdict_iter(old, va_qdict_insert, new);
+
+ return new;
+}
+
+static int va_connect(void)
+{
+ QemuOpts *opts;
+ int fd, ret = 0;
+
+ TRACE("called");
+ if (va_state->channel_method == NULL) {
+ LOG("no channel method specified");
+ return -EINVAL;
+ }
+ if (va_state->channel_path == NULL) {
+ LOG("no channel path specified");
+ return -EINVAL;
+ }
+
+ if (strcmp(va_state->channel_method, "unix-connect") == 0) {
+ TRACE("connecting to %s", va_state->channel_path);
+ opts = qemu_opts_create(qemu_find_opts("chardev"), NULL, 0);
+ qemu_opt_set(opts, "path", va_state->channel_path);
+ fd = unix_connect_opts(opts);
+ if (fd == -1) {
+ qemu_opts_del(opts);
+ LOG("error opening channel: %s", strerror(errno));
+ return -errno;
+ }
+ qemu_opts_del(opts);
+ socket_set_nonblock(fd);
+ } else if (strcmp(va_state->channel_method, "virtio-serial") == 0) {
+ if (va_state->is_host) {
+ LOG("specified channel method not available for host");
+ return -EINVAL;
+ }
+ if (va_state->channel_path == NULL) {
+ va_state->channel_path = VA_GUEST_PATH_VIRTIO_DEFAULT;
+ }
+ TRACE("opening %s", va_state->channel_path);
+ fd = qemu_open(va_state->channel_path, O_RDWR);
+ if (fd == -1) {
+ LOG("error opening channel: %s", strerror(errno));
+ return -errno;
+ }
+ ret = fcntl(fd, F_GETFL);
+ if (ret < 0) {
+ LOG("error getting channel flags: %s", strerror(errno));
+ return -errno;
+ }
+ ret = fcntl(fd, F_SETFL, ret | O_ASYNC | O_NONBLOCK);
+ if (ret < 0) {
+ LOG("error setting channel flags: %s", strerror(errno));
+ return -errno;
+ }
+ } else if (strcmp(va_state->channel_method, "isa-serial") == 0) {
+ struct termios tio;
+ if (va_state->is_host) {
+ LOG("specified channel method not available for host");
+ return -EINVAL;
+ }
+ if (va_state->channel_path == NULL) {
+ LOG("you must specify the path of the serial device to use");
+ return -EINVAL;
+ }
+ TRACE("opening %s", va_state->channel_path);
+ fd = qemu_open(va_state->channel_path, O_RDWR | O_NOCTTY);
+ if (fd == -1) {
+ LOG("error opening channel: %s", strerror(errno));
+ return -errno;
+ }
+ tcgetattr(fd, &tio);
+ /* set up serial port for non-canonical, dumb byte streaming */
+ tio.c_iflag &= ~(IGNBRK | BRKINT | IGNPAR | PARMRK | INPCK | ISTRIP |
+ INLCR | IGNCR | ICRNL | IXON | IXOFF | IXANY | IMAXBEL);
+ tio.c_oflag = 0;
+ tio.c_lflag = 0;
+ tio.c_cflag |= VA_BAUDRATE;
+ /* 1 available byte min, else reads will block (we'll set non-blocking
+ * elsewhere, else we'd have to deal with read()=0 instead)
+ */
+ tio.c_cc[VMIN] = 1;
+ tio.c_cc[VTIME] = 0;
+ /* flush everything waiting for read/xmit, it's garbage at this point */
+ tcflush(fd, TCIFLUSH);
+ tcsetattr(fd, TCSANOW, &tio);
+ } else {
+ LOG("invalid channel method");
+ return -EINVAL;
+ }
+
+ va_state->fd = fd;
+ return 0;
+}
+
+int va_init(VAContext ctx)
+{
+ VAState *s;
+ VAManager *m;
+ int ret;
+
+ TRACE("called");
+ if (va_state) {
+ LOG("virtagent already initialized");
+ return -EPERM;
+ }
+
+ s = qemu_mallocz(sizeof(VAState));
+ m = va_manager_new();
+
+ ret = va_server_init(m, &s->server_data, ctx.is_host);
+ if (ret) {
+ LOG("error initializing virtagent server");
+ goto out_bad;
+ }
+ ret = va_client_init(m, &s->client_data);
+ if (ret) {
+ LOG("error initializing virtagent client");
+ goto out_bad;
+ }
+
+ s->client_job_count = 0;
+ s->client_jobs_in_flight = 0;
+ s->server_job_count = 0;
+ s->channel_method = ctx.channel_method;
+ s->channel_path = ctx.channel_path;
+ s->is_host = ctx.is_host;
+ s->manager = m;
+ va_state = s;
+
+ /* connect to our end of the channel */
+ ret = va_connect();
+ if (ret) {
+ LOG("error connecting to channel");
+ goto out_bad;
+ }
+
+ /* start listening for requests/responses */
+ qemu_set_fd_handler(va_state->fd, va_http_read_handler, NULL, NULL);
+
+ if (!va_state->is_host) {
+ /* tell the host the agent is running */
+ va_send_hello();
+ }
+
+ return 0;
+out_bad:
+ qemu_free(s);
+ return ret;
+}
diff --git a/virtagent-common.h b/virtagent-common.h
new file mode 100644
index 0000000..5ae50d1
--- /dev/null
+++ b/virtagent-common.h
@@ -0,0 +1,95 @@
+/*
+ * virt-agent - host/guest RPC client functions
+ *
+ * Copyright IBM Corp. 2010
+ *
+ * Authors:
+ * Adam Litke <aglitke@linux.vnet.ibm.com>
+ * Michael Roth <mdroth@linux.vnet.ibm.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ *
+ */
+#ifndef VIRTAGENT_COMMON_H
+#define VIRTAGENT_COMMON_H
+
+#include <termios.h>
+#include "qemu-common.h"
+#include "qemu_socket.h"
+#include "qemu-timer.h"
+#include "monitor.h"
+#include "virtagent-manager.h"
+#include "virtagent-server.h"
+#include "virtagent.h"
+
+#define DEBUG_VA
+
+#ifdef DEBUG_VA
+#define TRACE(msg, ...) do { \
+ fprintf(stderr, "%s:%s():L%d: " msg "\n", \
+ __FILE__, __FUNCTION__, __LINE__, ## __VA_ARGS__); \
+} while(0)
+#else
+#define TRACE(msg, ...) \
+ do { } while (0)
+#endif
+
+#define LOG(msg, ...) do { \
+ fprintf(stderr, "%s:%s(): " msg "\n", \
+ __FILE__, __FUNCTION__, ## __VA_ARGS__); \
+} while(0)
+
+#define VA_VERSION "1.0"
+#define EOL "\r\n"
+
+#define VA_PIDFILE "/var/run/qemu-va.pid"
+#define VA_HDR_LEN_MAX 4096 /* http header limit */
+#define VA_CONTENT_LEN_MAX 2*1024*1024 /* rpc/http send limit */
+#define VA_CLIENT_JOBS_MAX 5 /* max client rpcs we can queue */
+#define VA_SERVER_JOBS_MAX 5 /* max server rpcs we can queue */
+#define VA_SERVER_TIMEOUT_MS 5 * 1000
+#define VA_CLIENT_TIMEOUT_MS 5 * 1000
+#define VA_SENTINEL 0xFF
+#define VA_BAUDRATE B38400 /* for isa-serial channels */
+
+typedef struct VAContext {
+ bool is_host;
+ const char *channel_method;
+ const char *channel_path;
+} VAContext;
+
+typedef struct VAState {
+ bool is_host;
+ const char *channel_method;
+ const char *channel_path;
+ int fd;
+ QEMUTimer *client_timer;
+ QEMUTimer *server_timer;
+ VAClientData client_data;
+ VAServerData server_data;
+ int client_job_count;
+ int client_jobs_in_flight;
+ int server_job_count;
+ VAManager *manager;
+} VAState;
+
+enum va_job_status {
+ VA_JOB_STATUS_PENDING = 0,
+ VA_JOB_STATUS_OK,
+ VA_JOB_STATUS_ERROR,
+ VA_JOB_STATUS_CANCELLED,
+};
+
+typedef void (VAHTSendCallback)(const void *opaque);
+
+int va_init(VAContext ctx);
+bool va_qdict_haskey_with_type(const QDict *qdict, const char *key,
+ qtype_code type);
+QDict *va_qdict_copy(const QDict *old);
+int va_xport_send_response(const char *content, size_t content_len, const char *tag,
+ const void *opaque, VAHTSendCallback cb);
+int va_xport_send_request(const char *content, size_t content_len, const char *tag,
+ const void *opaque, VAHTSendCallback cb);
+void va_http_read_handler(void *opaque);
+#endif /* VIRTAGENT_COMMON_H */
--
1.7.0.4
^ permalink raw reply related [flat|nested] 41+ messages in thread
* [Qemu-devel] [RFC][PATCH v7 06/16] virtagent: transport definitions
2011-03-07 20:10 [Qemu-devel] [RFC][PATCH v7 00/16] virtagent: host/guest communication agent Michael Roth
` (4 preceding siblings ...)
2011-03-07 20:10 ` [Qemu-devel] [RFC][PATCH v7 05/16] virtagent: common helpers and init routines Michael Roth
@ 2011-03-07 20:10 ` Michael Roth
2011-03-07 21:38 ` [Qemu-devel] " Adam Litke
2011-03-07 20:10 ` [Qemu-devel] [RFC][PATCH v7 07/16] virtagent: base RPC client definitions Michael Roth
` (10 subsequent siblings)
16 siblings, 1 reply; 41+ messages in thread
From: Michael Roth @ 2011-03-07 20:10 UTC (permalink / raw)
To: qemu-devel
Cc: agl, stefanha, Jes.Sorensen, mdroth, markus_mueller, aliguori,
abeekhof
This implements an HTTP-like transport for sending UTF-8 encoded RPC
requests/responses over the isa/virtio serial channel.
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
virtagent-transport.c | 432 +++++++++++++++++++++++++++++++++++++++++++++++++
1 files changed, 432 insertions(+), 0 deletions(-)
create mode 100644 virtagent-transport.c
diff --git a/virtagent-transport.c b/virtagent-transport.c
new file mode 100644
index 0000000..4f99e7e
--- /dev/null
+++ b/virtagent-transport.c
@@ -0,0 +1,432 @@
+/*
+ * virtagent - common host/guest RPC functions
+ *
+ * Copyright IBM Corp. 2010
+ *
+ * Authors:
+ * Adam Litke <aglitke@linux.vnet.ibm.com>
+ * Michael Roth <mdroth@linux.vnet.ibm.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ *
+ */
+
+#include "virtagent-common.h"
+
+enum va_http_status {
+ VA_HTTP_STATUS_NEW,
+ VA_HTTP_STATUS_OK,
+ VA_HTTP_STATUS_ERROR,
+};
+
+enum va_http_type {
+ VA_HTTP_TYPE_UNKNOWN = 1,
+ VA_HTTP_TYPE_REQUEST,
+ VA_HTTP_TYPE_RESPONSE,
+} va_http_type;
+
+typedef struct VAHTState {
+ enum {
+ VA_SEND_START = 0,
+ VA_SEND_HDR,
+ VA_SEND_BODY,
+ VA_SEND_COMPLETE,
+ VA_READ_START,
+ VA_READ_HDR,
+ VA_READ_BODY,
+ VA_READ_COMPLETE,
+ } state;
+ char hdr[VA_HDR_LEN_MAX];
+ char hdr_client_tag[64];
+ size_t hdr_len;
+ size_t hdr_pos;
+ char *content;
+ const char *send_content;
+ size_t content_len;
+ size_t content_pos;
+ const void *opaque;
+ VAHTSendCallback *send_cb;
+ enum va_http_type http_type;
+} VAHTState;
+
+extern VAState *va_state;
+VAHTState va_send_state = {
+ .state = VA_SEND_START,
+};
+VAHTState va_read_state = {
+ .state = VA_READ_START,
+};
+
+/* utility functions for handling http calls */
+
+static void va_http_hdr_init(VAHTState *s, enum va_http_type http_type)
+{
+ const char *preamble;
+
+ TRACE("called");
+ /* essentially ignored in the context of virtagent, but might as well */
+ if (http_type == VA_HTTP_TYPE_REQUEST) {
+ preamble = "POST /RPC2 HTTP/1.1";
+ } else if (http_type == VA_HTTP_TYPE_RESPONSE) {
+ preamble = "HTTP/1.1 200 OK";
+ } else {
+ LOG("unknown http type");
+ s->hdr_len = 0;
+ return;
+ }
+ memset(s->hdr, 0, VA_HDR_LEN_MAX);
+ s->hdr_len = sprintf(s->hdr,
+ "%c%s" EOL
+ "Content-Type: text/xml" EOL
+ "Content-Length: %u" EOL
+ "X-Virtagent-Client-Tag: %s" EOL EOL,
+ VA_SENTINEL,
+ preamble,
+ (uint32_t)s->content_len,
+ s->hdr_client_tag[0] ? s->hdr_client_tag : "none");
+}
+
+#define VA_LINE_LEN_MAX 1024
+static void va_rpc_parse_hdr(VAHTState *s)
+{
+ int i, line_pos = 0;
+ bool first_line = true;
+ char line_buf[VA_LINE_LEN_MAX];
+
+ TRACE("called");
+
+ for (i = 0; i < VA_HDR_LEN_MAX; ++i) {
+ if (s->hdr[i] == 0) {
+ /* end of header */
+ return;
+ }
+ if (s->hdr[i] != '\n') {
+ /* read line */
+ line_buf[line_pos++] = s->hdr[i];
+ } else {
+ /* process line */
+ if (first_line) {
+ if (strncmp(line_buf, "POST", 4) == 0) {
+ s->http_type = VA_HTTP_TYPE_REQUEST;
+ } else if (strncmp(line_buf, "HTTP", 4) == 0) {
+ s->http_type = VA_HTTP_TYPE_RESPONSE;
+ } else {
+ s->http_type = VA_HTTP_TYPE_UNKNOWN;
+ }
+ first_line = false;
+ }
+ if (strncmp(line_buf, "Content-Length: ", 16) == 0) {
+ s->content_len = atoi(&line_buf[16]);
+ }
+ if (strncmp(line_buf, "X-Virtagent-Client-Tag: ", 24) == 0) {
+ memcpy(s->hdr_client_tag, &line_buf[24], MIN(line_pos-25, 64));
+ //pstrcpy(s->hdr_client_tag, 64, &line_buf[24]);
+ TRACE("\nTAG<%s>\n", s->hdr_client_tag);
+ }
+ line_pos = 0;
+ memset(line_buf, 0, VA_LINE_LEN_MAX);
+ }
+ }
+}
+
+static int va_end_of_header(char *buf, int end_pos)
+{
+ return !strncmp(buf+(end_pos-2), "\n\r\n", 3);
+}
+
+static void va_http_read_handler_reset(void)
+{
+ VAHTState *s = &va_read_state;
+ TRACE("called");
+ s->state = VA_READ_START;
+ s->http_type = VA_HTTP_TYPE_UNKNOWN;
+ s->hdr_pos = 0;
+ s->content_len = 0;
+ s->content_pos = 0;
+ memset(s->hdr_client_tag, 0, 64);
+ strcpy(s->hdr_client_tag, "none");
+ s->content = NULL;
+}
+
+static void va_http_process(char *content, size_t content_len,
+ const char *tag, enum va_http_type type)
+{
+ int ret;
+ TRACE("marker");
+ if (type == VA_HTTP_TYPE_REQUEST) {
+ ret = va_server_job_create(content, content_len, tag);
+ if (ret < 0) {
+ LOG("error processing request: %s", strerror(-ret));
+ }
+ } else if (type == VA_HTTP_TYPE_RESPONSE) {
+ va_client_read_response_done(content, content_len, tag);
+ } else {
+ LOG("unknown http type");
+ }
+}
+
+/* read/send handlers */
+
+void va_http_read_handler(void *opaque)
+{
+ VAHTState *s = &va_read_state;
+ enum va_http_status http_status;
+ int fd = va_state->fd;
+ int ret;
+ uint8_t tmp;
+ static int bytes_skipped = 0;
+
+ TRACE("called with opaque: %p", opaque);
+
+ switch (s->state) {
+ case VA_READ_START:
+ /* we may have gotten here due to a http error, indicating
+ * a potential unclean state where we are not 'aligned' on http
+ * boundaries. we should read till we hit the next http preamble
+ * rather than assume we're at the start of an http header. since
+ * we control the transport layer on both sides, we'll use a
+ * more reliable sentinal character to mark/detect the start of
+ * the header
+ */
+ while((ret = read(fd, &tmp, 1) > 0) > 0) {
+ if (tmp == VA_SENTINEL) {
+ break;
+ }
+ bytes_skipped += ret;
+ }
+ if (ret == -1) {
+ if (errno == EAGAIN || errno == EWOULDBLOCK || errno == EINTR) {
+ return;
+ } else {
+ LOG("error reading connection: %s", strerror(errno));
+ goto out_bad;
+ }
+ } else if (ret == 0) {
+ LOG("connection closed unexpectedly");
+ goto out_bad_wait;
+ } else {
+ TRACE("found header, number of bytes skipped: %d",
+ bytes_skipped);
+ bytes_skipped = 0;
+ s->state = VA_READ_HDR;
+ }
+ case VA_READ_HDR:
+ while((ret = read(fd, s->hdr + s->hdr_pos, 1)) > 0
+ && s->hdr_pos < VA_HDR_LEN_MAX) {
+ if (s->hdr[s->hdr_pos] == (char)VA_SENTINEL) {
+ /* truncated header, toss it out and start over */
+ LOG("truncated header detected");
+ s->hdr_pos = 0;
+ } else {
+ s->hdr_pos += ret;
+ if (va_end_of_header(s->hdr, s->hdr_pos - 1)) {
+ break;
+ }
+ }
+ }
+ if (ret == -1) {
+ if (errno == EAGAIN || errno == EWOULDBLOCK || errno == EINTR) {
+ return;
+ } else {
+ LOG("error reading connection: %s", strerror(errno));
+ goto out_bad;
+ }
+ } else if (ret == 0) {
+ LOG("connection closed unexpectedly");
+ goto out_bad_wait;
+ } else if (s->hdr_pos >= VA_HDR_LEN_MAX) {
+ LOG("http header too long");
+ goto out_bad;
+ } else {
+ s->content_len = -1;
+ va_rpc_parse_hdr(s);
+ if (s->content_len == -1) {
+ LOG("malformed http header");
+ goto out_bad;
+ } else if (s->content_len > VA_CONTENT_LEN_MAX) {
+ LOG("http content length too long");
+ goto out_bad;
+ }
+ s->content = qemu_mallocz(s->content_len + 1);
+ s->state = VA_READ_BODY;
+ TRACE("read http header:\n<<<%s>>>\n", s->hdr);
+ }
+ case VA_READ_BODY:
+ while(s->content_pos < s->content_len) {
+ ret = read(fd, s->content + s->content_pos,
+ s->content_len - s->content_pos);
+ if (ret == -1) {
+ if (errno == EAGAIN || errno == EWOULDBLOCK
+ || errno == EINTR) {
+ return;
+ } else {
+ LOG("error reading connection: %s", strerror(errno));
+ goto out_bad;
+ }
+ } else if (ret == 0) {
+ LOG("connection closed unexpectedly:"
+ " read %u bytes, expected %u bytes",
+ (unsigned int)s->content_pos, (unsigned int)s->content_len);
+ goto out_bad_wait;
+ }
+ s->content_pos += ret;
+ }
+
+ TRACE("read http content:\n<<<%s>>>\n", s->content);
+ http_status = VA_HTTP_STATUS_OK;
+ s->content[s->content_len] = '\0';
+ goto out;
+ default:
+ LOG("unknown state");
+ goto out_bad;
+ }
+
+out_bad_wait:
+ /* We should only ever get a ret = 0 if we're a guest and the host is
+ * not connected. this would cause a guest to spin, and we can't do
+ * any work in the meantime, so sleep for a bit here. We also know
+ * we may go ahead and cancel any outstanding jobs at this point, though
+ * it should be noted that we're still ultimately reliant on per-job
+ * timeouts since we might not read EOF before host reconnect.
+ */
+ if (!va_state->is_host) {
+ usleep(100 * 1000);
+ }
+out_bad:
+ http_status = VA_HTTP_STATUS_ERROR;
+out:
+ s->state = VA_READ_COMPLETE;
+ /* handle the response or request we just read */
+ if (http_status == VA_HTTP_STATUS_OK) {
+ va_http_process(s->content, s->content_len, s->hdr_client_tag, s->http_type);
+ } else {
+ LOG("http read error");
+ }
+ /* restart read handler */
+ va_http_read_handler_reset();
+ http_status = VA_HTTP_STATUS_NEW;
+}
+
+static void va_http_send_handler(void *opaque)
+{
+ VAHTState *s = &va_send_state;
+ enum va_http_status http_status;
+ int fd = va_state->fd;
+ int ret;
+ char flush_char = VA_SENTINEL;
+
+ TRACE("called");
+
+ switch (s->state) {
+ case VA_SEND_START:
+ s->state = VA_SEND_HDR;
+ TRACE("preparing to send http header:\n<<<%s>>>", s->hdr);
+ case VA_SEND_HDR:
+ do {
+ ret = write(fd, s->hdr + s->hdr_pos, s->hdr_len - s->hdr_pos);
+ if (ret <= 0) {
+ break;
+ }
+ s->hdr_pos += ret;
+ } while (s->hdr_pos < s->hdr_len);
+ if (ret == -1) {
+ if (errno == EAGAIN || errno == EWOULDBLOCK || errno == EINTR) {
+ return;
+ } else {
+ LOG("error writing header: %s", strerror(errno));
+ goto out_bad;
+ }
+ } else if (ret == 0) {
+ LOG("connected closed unexpectedly");
+ goto out_bad;
+ } else {
+ s->state = VA_SEND_BODY;
+ TRACE("sent http header:\n<<<%s>>>", s->hdr);
+ TRACE("preparing to send http content:\n<<<%s>>>", s->send_content);
+ }
+ case VA_SEND_BODY:
+ do {
+ ret = write(fd, s->send_content + s->content_pos,
+ s->content_len - s->content_pos);
+ if (ret <= 0) {
+ break;
+ }
+ s->content_pos += ret;
+ } while (s->content_pos < s->content_len);
+ if (ret == -1) {
+ if (errno == EAGAIN || errno == EWOULDBLOCK || errno == EINTR) {
+ return;
+ } else {
+ LOG("error writing content: %s", strerror(errno));
+ goto out_bad;
+ }
+ } else if (ret == 0) {
+ LOG("connected closed unexpectedly");
+ goto out_bad;
+ } else {
+ http_status = VA_HTTP_STATUS_OK;
+ TRACE("sent http content:\n<<<%s>>>", s->send_content);
+ goto out;
+ }
+ default:
+ LOG("unknown state");
+ goto out_bad;
+ }
+
+out_bad:
+ http_status = VA_HTTP_STATUS_ERROR;
+out:
+ s->state = VA_SEND_COMPLETE;
+ qemu_set_fd_handler(fd, va_http_read_handler, NULL, NULL);
+ /* XXX: try to force flush to get around buggy guests */
+ ret = write(fd, &flush_char, 1);
+ s->send_cb(s->opaque);
+}
+
+static void va_send_handler_reset(void)
+{
+ TRACE("called");
+ assert(va_send_state.state == VA_SEND_START ||
+ va_send_state.state == VA_SEND_COMPLETE);
+ va_send_state.send_content = NULL;
+ va_send_state.content_len = 0;
+ va_send_state.content_pos = 0;
+ va_send_state.hdr_pos = 0;
+ va_send_state.state = VA_SEND_START;
+ memset(va_send_state.hdr_client_tag, 0, 64);
+}
+
+int va_xport_send_response(const char *content, size_t content_len, const char *tag,
+ const void *opaque, VAHTSendCallback cb)
+{
+ TRACE("called");
+ va_send_handler_reset();
+ va_send_state.send_content = content;
+ TRACE("sending response: %s", va_send_state.send_content);
+ va_send_state.content_len = content_len;
+ va_send_state.opaque = opaque;
+ va_send_state.send_cb = cb;
+ pstrcpy(va_send_state.hdr_client_tag, 63, tag);
+ va_http_hdr_init(&va_send_state, VA_HTTP_TYPE_RESPONSE);
+ qemu_set_fd_handler(va_state->fd, va_http_read_handler,
+ va_http_send_handler, NULL);
+ return 0;
+}
+
+int va_xport_send_request(const char *content, size_t content_len, const char *tag,
+ const void *opaque, VAHTSendCallback cb)
+{
+ TRACE("called");
+ va_send_handler_reset();
+ va_send_state.send_content = content;
+ TRACE("sending request: %s", va_send_state.send_content);
+ va_send_state.content_len = content_len;
+ va_send_state.opaque = opaque;
+ va_send_state.send_cb = cb;
+ pstrcpy(va_send_state.hdr_client_tag, 63, tag);
+ va_http_hdr_init(&va_send_state, VA_HTTP_TYPE_REQUEST);
+ qemu_set_fd_handler(va_state->fd, va_http_read_handler,
+ va_http_send_handler, NULL);
+ return 0;
+}
--
1.7.0.4
^ permalink raw reply related [flat|nested] 41+ messages in thread
* [Qemu-devel] [RFC][PATCH v7 07/16] virtagent: base RPC client definitions
2011-03-07 20:10 [Qemu-devel] [RFC][PATCH v7 00/16] virtagent: host/guest communication agent Michael Roth
` (5 preceding siblings ...)
2011-03-07 20:10 ` [Qemu-devel] [RFC][PATCH v7 06/16] virtagent: transport definitions Michael Roth
@ 2011-03-07 20:10 ` Michael Roth
2011-03-07 20:10 ` [Qemu-devel] [RFC][PATCH v7 08/16] virtagnet: base RPC server definitions Michael Roth
` (9 subsequent siblings)
16 siblings, 0 replies; 41+ messages in thread
From: Michael Roth @ 2011-03-07 20:10 UTC (permalink / raw)
To: qemu-devel
Cc: agl, stefanha, Jes.Sorensen, mdroth, markus_mueller, aliguori,
abeekhof
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
monitor.c | 1 +
qerror.c | 8 +
qerror.h | 6 +
virtagent.c | 455 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
virtagent.h | 46 ++++++
5 files changed, 516 insertions(+), 0 deletions(-)
create mode 100644 virtagent.c
create mode 100644 virtagent.h
diff --git a/monitor.c b/monitor.c
index 22ae3bb..44f5033 100644
--- a/monitor.c
+++ b/monitor.c
@@ -57,6 +57,7 @@
#include "json-parser.h"
#include "osdep.h"
#include "exec-all.h"
+#include "virtagent.h"
#ifdef CONFIG_SIMPLE_TRACE
#include "trace.h"
#endif
diff --git a/qerror.c b/qerror.c
index 4855604..741e0bc 100644
--- a/qerror.c
+++ b/qerror.c
@@ -209,6 +209,14 @@ static const QErrorStringTable qerror_table[] = {
.error_fmt = QERR_VNC_SERVER_FAILED,
.desc = "Could not start VNC server on %(target)",
},
+ {
+ .error_fmt = QERR_RPC_FAILED,
+ .desc = "An RPC error has occurred: %(message)",
+ },
+ {
+ .error_fmt = QERR_VA_FAILED,
+ .desc = "An error was reported by virtagent: %(message)",
+ },
{}
};
diff --git a/qerror.h b/qerror.h
index f732d45..f3322e7 100644
--- a/qerror.h
+++ b/qerror.h
@@ -171,4 +171,10 @@ QError *qobject_to_qerror(const QObject *obj);
#define QERR_VNC_SERVER_FAILED \
"{ 'class': 'VNCServerFailed', 'data': { 'target': %s } }"
+#define QERR_RPC_FAILED \
+ "{ 'class': 'RPCFailed', 'data': { 'code': %i, 'message': %s } }"
+
+#define QERR_VA_FAILED \
+ "{ 'class': 'VirtagentFailed', 'data': { 'code': %i, 'message': %s } }"
+
#endif /* QERROR_H */
diff --git a/virtagent.c b/virtagent.c
new file mode 100644
index 0000000..670309b
--- /dev/null
+++ b/virtagent.c
@@ -0,0 +1,455 @@
+/*
+ * virtagent - host/guest RPC client functions
+ *
+ * Copyright IBM Corp. 2010
+ *
+ * Authors:
+ * Adam Litke <aglitke@linux.vnet.ibm.com>
+ * Michael Roth <mdroth@linux.vnet.ibm.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ *
+ */
+
+#include "qemu_socket.h"
+#include "qjson.h"
+#include "qint.h"
+#include "monitor.h"
+#include "virtagent-common.h"
+
+static VAClientData *va_client_data;
+
+static void va_set_capabilities(QList *qlist)
+{
+ TRACE("called");
+
+ if (va_client_data == NULL) {
+ LOG("client is uninitialized, unable to set capabilities");
+ return;
+ }
+
+ if (va_client_data->supported_methods != NULL) {
+ qobject_decref(QOBJECT(va_client_data->supported_methods));
+ va_client_data->supported_methods = NULL;
+ TRACE("capabilities reset");
+ }
+
+ if (qlist != NULL) {
+ va_client_data->supported_methods = qlist_copy(qlist);
+ TRACE("capabilities set");
+ }
+}
+
+static void va_set_version_level(const char *version) {
+ if (version) {
+ pstrcpy(va_client_data->guest_version, 32, version);
+ }
+}
+
+typedef struct VACmpState {
+ const char *method;
+ bool found;
+} VACmpState;
+
+static void va_cmp_capability_iter(QObject *obj, void *opaque)
+{
+ QString *method = qobject_to_qstring(obj);
+ const char *method_str = NULL;
+ VACmpState *cmp_state = opaque;
+
+ if (method) {
+ method_str = qstring_get_str(method);
+ }
+
+ if (method_str && opaque) {
+ if (strcmp(method_str, cmp_state->method) == 0) {
+ cmp_state->found = 1;
+ }
+ }
+}
+
+static bool va_has_capability(const char *method)
+{
+ VACmpState cmp_state;
+
+ if (method == NULL) {
+ return false;
+ }
+
+ /* we can assume capabilities is available */
+ if (strcmp(method, "capabilities") == 0) {
+ return true;
+ }
+ /* assume hello is available to we can probe for/notify the host
+ * rpc server
+ */
+ if (strcmp(method, "hello") == 0) {
+ return true;
+ }
+
+ /* compare method against the last retrieved supported method list */
+ cmp_state.method = method;
+ cmp_state.found = false;
+ if (va_client_data->supported_methods) {
+ qlist_iter(va_client_data->supported_methods,
+ va_cmp_capability_iter,
+ (void *)&cmp_state);
+ }
+
+ return cmp_state.found;
+}
+
+int va_client_init(VAManager *m, VAClientData *client_data)
+{
+ client_data->supported_methods = NULL;
+ client_data->enabled = true;
+ client_data->manager = m;
+ va_client_data = client_data;
+
+ return 0;
+}
+
+static bool va_is_enabled(void)
+{
+ return va_client_data && va_client_data->enabled;
+}
+
+typedef struct VAClientRequest {
+ QString *payload;
+ char tag[64];
+ VAClientCallback *cb;
+ /* for use by QMP functions */
+ MonitorCompletion *mon_cb;
+ void *mon_data;
+ int timeout;
+ QEMUTimer *timer;
+} VAClientRequest;
+
+typedef struct VAClientResponse {
+ char *content;
+ size_t content_len;
+} VAClientResponse;
+
+static void va_client_timeout(void *opaque)
+{
+ VAClientRequest *req = opaque;
+ qemu_del_timer(req->timer);
+ req->timer = NULL;
+ va_client_job_cancel(va_client_data->manager, req->tag);
+}
+
+/* called by xport layer to indicate send completion to VAManager */
+static void va_send_request_cb(const void *opaque)
+{
+ const char *tag = opaque;
+ va_client_job_send_done(va_client_data->manager, tag);
+}
+
+/* called by VAManager to start send, in turn calls out to xport layer */
+static int va_send_request(void *opaque, const char *tag)
+{
+ VAClientRequest *req = opaque;
+ const char *payload_json;
+ int ret;
+
+ TRACE("called");
+ if (!req || !req->payload) {
+ TRACE("marker");
+ return -EINVAL;
+ }
+ payload_json = qstring_get_str(req->payload);
+ if (!payload_json) {
+ TRACE("marker");
+ return -EINVAL;
+ }
+ TRACE("marker");
+ ret = va_xport_send_request(payload_json, strlen(payload_json),
+ tag, tag, va_send_request_cb);
+ TRACE("marker");
+ /* register timeout */
+ if (req->timeout) {
+ TRACE("marker");
+ req->timer = qemu_new_timer(rt_clock, va_client_timeout, req);
+ qemu_mod_timer(req->timer, qemu_get_clock(rt_clock) + req->timeout);
+ }
+ TRACE("marker");
+ return ret;
+}
+
+/* called by xport layer to pass response to VAManager */
+void va_client_read_response_done(const char *content, size_t content_len, const char *tag)
+{
+ QDict *resp = NULL;
+ QObject *resp_qobject;
+
+ resp_qobject = qobject_from_json(content);
+ if (resp_qobject) {
+ resp = qobject_to_qdict(resp_qobject);
+ }
+ va_client_job_read_done(va_client_data->manager, tag, resp);
+}
+
+/* called by VAManager once RPC response is recieved */
+static int va_callback(void *opaque, void *resp_opaque, const char *tag)
+{
+ VAClientRequest *req = opaque;
+ QDict *resp = resp_opaque;
+
+ TRACE("called");
+
+ if (req->timer) {
+ qemu_del_timer(req->timer);
+ }
+
+ if (req->cb) {
+ if (resp) {
+ req->cb(resp, req->mon_cb, req->mon_data);
+ } else {
+ /* RPC did not complete */
+ req->cb(NULL, req->mon_cb, req->mon_data);
+ }
+ }
+
+ if (req) {
+ if (req->payload) {
+ QDECREF(req->payload);
+ }
+ qemu_free(req);
+ }
+
+ if (resp) {
+ QDECREF(resp);
+ }
+
+ return 0;
+}
+
+static VAClientJobOps client_job_ops = {
+ .send = va_send_request,
+ .callback = va_callback,
+};
+
+static int va_do_rpc(const char *method, const QDict *params,
+ VAClientCallback *cb, MonitorCompletion *mon_cb,
+ void *mon_data)
+{
+ VAClientRequest *req;
+ QDict *payload, *params_copy = NULL;
+ QString *payload_json;
+ struct timeval ts;
+ int ret;
+
+ if (!va_is_enabled()) {
+ LOG("virtagent not initialized");
+ ret = -ENOTCONN;
+ }
+
+ if (!va_has_capability(method)) {
+ LOG("guest agent does not have required capability: %s", method);
+ ret = -ENOTSUP;
+ goto out;
+ }
+
+ req = qemu_mallocz(sizeof(VAClientRequest));
+ req->cb = cb;
+ req->mon_cb = mon_cb;
+ req->mon_data = mon_data;
+ req->timeout = VA_CLIENT_TIMEOUT_MS;
+
+ /* add params and remote RPC method to call to payload */
+ payload = qdict_new();
+ qdict_put_obj(payload, "method",
+ QOBJECT(qstring_from_str(method)));
+ if (params) {
+ params_copy = va_qdict_copy(params);
+ if (!params_copy) {
+ LOG("error processing parameters");
+ QDECREF(payload);
+ ret = -EINVAL;
+ goto out_free;
+ }
+ qdict_put_obj(payload, "params", QOBJECT(params_copy));
+ }
+
+ /* convert payload to json */
+ payload_json = qobject_to_json(QOBJECT(payload));
+ QDECREF(payload);
+ if (!payload_json) {
+ LOG("error converting request to json");
+ ret = -EINVAL;
+ goto out_free;
+ }
+ req->payload = payload_json;
+
+ /* TODO: should switch to UUIDs eventually */
+ memset(req->tag, 0, 64);
+ gettimeofday(&ts, NULL);
+ sprintf(req->tag, "%u.%u", (uint32_t)ts.tv_sec, (uint32_t)ts.tv_usec);
+ TRACE("req->payload: %p, req->cb: %p, req->mon_cb: %p, req->mon_data: %p",
+ req->payload, req->cb, req->mon_cb, req->mon_data);
+
+ ret = va_client_job_add(va_client_data->manager, req->tag, req,
+ client_job_ops);
+ if (ret) {
+ TRACE("marker");
+ va_client_job_cancel(va_client_data->manager, req->tag);
+ goto out_free;
+ }
+
+out:
+ return ret;
+out_free:
+ qemu_free(req);
+ return ret;
+}
+
+/* validate the RPC response. if response indicates an error, log it
+ * to stderr/monitor. if return_data != NULL, return_data will be set
+ * to the response payload of the RPC if present, otherwise an error
+ * will be logged. if return_data == NULL, response payload is ignored,
+ * and only the RPC's error indicator is checked for success.
+ *
+ * XXX: The JSON that generates the response may originate from untrusted
+ * sources such as an unsupported/malicious guest agent, so we must take
+ * particular care to not make any assumptions about what the response
+ * contains. In particular, always check for key existence, and no blind
+ * qdict_get_<type>() calls since the value may be an unexpected type. This
+ * also applies to the return_data we pass back to callers.
+ */
+static bool va_check_response_ok(QDict *resp, QDict **return_data)
+{
+ int errnum;
+ const char *errstr = NULL;
+
+ TRACE("called");
+ /* TODO: not sure if errnum is of much use here */
+ if (!resp) {
+ errnum = ENOMSG;
+ errstr = "response is null";
+ goto out_bad;
+ }
+
+ if (va_qdict_haskey_with_type(resp, "errnum", QTYPE_QINT)) {
+ errnum = qdict_get_int(resp, "errnum");
+ if (errnum) {
+ if (va_qdict_haskey_with_type(resp, "errstr", QTYPE_QSTRING)) {
+ errstr = qdict_get_str(resp, "errstr");
+ }
+ goto out_bad;
+ }
+ } else {
+ errnum = EINVAL;
+ errstr = "response is missing error code";
+ goto out_bad;
+ }
+
+ if (return_data) {
+ if (va_qdict_haskey_with_type(resp, "return_data", QTYPE_QDICT)) {
+ TRACE("marker");
+ *return_data = qdict_get_qdict(resp, "return_data");
+ } else {
+ errnum = EINVAL;
+ errstr = "response indicates success, but missing expected retval";
+ goto out_bad;
+ }
+ }
+
+ return true;
+out_bad:
+ qerror_report(QERR_RPC_FAILED, errnum, errstr);
+ return false;
+}
+
+/* QMP/HMP RPC client functions and their helpers */
+
+static void va_print_capability_iter(QObject *obj, void *opaque)
+{
+ Monitor *mon = opaque;
+ QString *function = qobject_to_qstring(obj);
+ const char *function_str;
+
+ if (function) {
+ function_str = qstring_get_str(function);
+ monitor_printf(mon, "%s\n", function_str);
+ }
+}
+
+void do_va_capabilities_print(Monitor *mon, const QObject *data)
+{
+ QDict *ret = qobject_to_qdict(data);
+
+ TRACE("called");
+ if (!data) {
+ return;
+ }
+
+ monitor_printf(mon,
+ "guest agent version: %s\n"
+ "supported methods:\n", qdict_get_str(ret, "version"));
+ qlist_iter(qdict_get_qlist(ret, "methods"), va_print_capability_iter, mon);
+}
+
+static void do_va_capabilities_cb(QDict *resp,
+ MonitorCompletion *mon_cb,
+ void *mon_data)
+{
+ QDict *ret = NULL;
+ QObject *ret_qobject = NULL;
+
+ TRACE("called");
+ if (!va_check_response_ok(resp, &ret)) {
+ goto out;
+ }
+
+ if (!va_qdict_haskey_with_type(ret, "methods", QTYPE_QLIST) ||
+ !va_qdict_haskey_with_type(ret, "version", QTYPE_QSTRING)) {
+ qerror_report(QERR_VA_FAILED, -EINVAL,
+ "response does not contain required fields");
+ goto out;
+ }
+ va_set_capabilities(qdict_get_qlist(ret, "methods"));
+ va_set_version_level(qdict_get_str(ret, "version"));
+ ret_qobject = QOBJECT(ret);
+out:
+ if (mon_cb) {
+ mon_cb(mon_data, ret_qobject);
+ }
+}
+
+/*
+ * do_va_capabilities(): Fetch/re-negotiate guest agent capabilities
+ */
+int do_va_capabilities(Monitor *mon, const QDict *params,
+ MonitorCompletion cb, void *opaque)
+{
+ int ret = va_do_rpc("capabilities", params, do_va_capabilities_cb, cb,
+ opaque);
+ if (ret) {
+ qerror_report(QERR_VA_FAILED, ret, strerror(-ret));
+ }
+ return ret;
+}
+
+/* RPC client functions called outside of HMP/QMP */
+
+int va_client_init_capabilities(void)
+{
+ int ret = va_do_rpc("capabilities", NULL, do_va_capabilities_cb, NULL,
+ NULL);
+ if (ret) {
+ LOG("erroring negotiating capabilities: %s", strerror(-ret));
+ }
+
+ return 0;
+}
+
+int va_send_hello(void)
+{
+ int ret = va_do_rpc("hello", NULL, NULL, NULL, NULL);
+ if (ret) {
+ LOG("error sending start up notification to host: %s",
+ strerror(-ret));
+ }
+ return ret;
+}
diff --git a/virtagent.h b/virtagent.h
new file mode 100644
index 0000000..1652fdc
--- /dev/null
+++ b/virtagent.h
@@ -0,0 +1,46 @@
+/*
+ * virt-agent - host/guest RPC client functions
+ *
+ * Copyright IBM Corp. 2010
+ *
+ * Authors:
+ * Adam Litke <aglitke@linux.vnet.ibm.com>
+ * Michael Roth <mdroth@linux.vnet.ibm.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ *
+ */
+
+#ifndef VIRTAGENT_H
+#define VIRTAGENT_H
+
+#include "monitor.h"
+#include "virtagent-manager.h"
+
+#define VA_GUEST_PATH_VIRTIO_DEFAULT "/dev/virtio-ports/org.qemu.virtagent"
+#define VA_HOST_PATH_DEFAULT "/tmp/virtagent.sock"
+#define VA_MAX_CHUNK_SIZE 4096 /* max bytes at a time for get/send file */
+
+typedef void (VAClientCallback)(QDict *resp,
+ MonitorCompletion *mon_cb, void *mon_data);
+typedef struct VAClientData {
+ QList *supported_methods;
+ char guest_version[32];
+ bool enabled;
+ VAManager *manager;
+} VAClientData;
+
+int va_client_init(VAManager *m, VAClientData *client_data);
+int va_client_close(void);
+void va_client_read_response_done(const char *content, size_t content_len,
+ const char *tag);
+int va_client_init_capabilities(void);
+int va_send_hello(void);
+int do_agent_shutdown(Monitor *mon, const QDict *mon_params,
+ MonitorCompletion cb, void *opaque);
+void do_va_capabilities_print(Monitor *mon, const QObject *qobject);
+int do_va_capabilities(Monitor *mon, const QDict *mon_params,
+ MonitorCompletion cb, void *opaque);
+
+#endif /* VIRTAGENT_H */
--
1.7.0.4
^ permalink raw reply related [flat|nested] 41+ messages in thread
* [Qemu-devel] [RFC][PATCH v7 08/16] virtagnet: base RPC server definitions
2011-03-07 20:10 [Qemu-devel] [RFC][PATCH v7 00/16] virtagent: host/guest communication agent Michael Roth
` (6 preceding siblings ...)
2011-03-07 20:10 ` [Qemu-devel] [RFC][PATCH v7 07/16] virtagent: base RPC client definitions Michael Roth
@ 2011-03-07 20:10 ` Michael Roth
2011-03-07 20:10 ` [Qemu-devel] [RFC][PATCH v7 09/16] virtagent: add va_capabilities HMP/QMP command Michael Roth
` (8 subsequent siblings)
16 siblings, 0 replies; 41+ messages in thread
From: Michael Roth @ 2011-03-07 20:10 UTC (permalink / raw)
To: qemu-devel
Cc: agl, stefanha, Jes.Sorensen, mdroth, markus_mueller, aliguori,
abeekhof
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
virtagent-server.c | 313 ++++++++++++++++++++++++++++++++++++++++++++++++++++
virtagent-server.h | 40 +++++++
2 files changed, 353 insertions(+), 0 deletions(-)
create mode 100644 virtagent-server.c
create mode 100644 virtagent-server.h
diff --git a/virtagent-server.c b/virtagent-server.c
new file mode 100644
index 0000000..f84546b
--- /dev/null
+++ b/virtagent-server.c
@@ -0,0 +1,313 @@
+/*
+ * virtagent - host/guest RPC server functions
+ *
+ * Copyright IBM Corp. 2010
+ *
+ * Authors:
+ * Adam Litke <aglitke@linux.vnet.ibm.com>
+ * Michael Roth <mdroth@linux.vnet.ibm.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ *
+ */
+#include <syslog.h>
+#include "virtagent-common.h"
+#include "qemu_socket.h"
+#include "qjson.h"
+#include "qint.h"
+
+static VARPCFunction guest_functions[];
+static VARPCFunction host_functions[];
+static VAServerData *va_server_data;
+static bool va_enable_syslog = false; /* enable syslog'ing of RPCs */
+
+#define SLOG(msg, ...) do { \
+ char msg_buf[1024]; \
+ if (!va_enable_syslog) { \
+ break; \
+ } \
+ snprintf(msg_buf, 1024, msg, ## __VA_ARGS__); \
+ syslog(LOG_INFO, "virtagent, %s", msg_buf); \
+} while(0)
+
+/* helper functions for RPCs */
+
+static QDict *va_server_format_response(QDict *return_data, int errnum,
+ const char *errstr)
+{
+ QDict *response = qdict_new();
+
+ if (errnum == -1) {
+ if (!errstr) {
+ errstr = "unknown remote error handling RPC";
+ }
+ }
+ if (errstr) {
+ qdict_put_obj(response, "errstr",
+ QOBJECT(qstring_from_str(errstr)));
+ }
+ qdict_put_obj(response, "errnum", QOBJECT(qint_from_int(errnum)));
+ if (return_data) {
+ qdict_put_obj(response, "return_data", QOBJECT(return_data));
+ }
+
+ return response;
+}
+
+/* RPCs */
+
+/* va_hello(): handle client startup notification
+ * params/response qdict format (*=optional):
+ * response{error}: <error code>
+ * response{errstr}: <error description>
+ */
+static QDict *va_hello(const QDict *params)
+{
+ int ret;
+ TRACE("called");
+ SLOG("va_hello()");
+ ret = va_client_init_capabilities();
+ if (ret < 0) {
+ LOG("error setting initializing client capabilities");
+ }
+ return va_server_format_response(NULL, 0, NULL);
+}
+
+/* va_capabilities(): return server capabilities
+ * params/response qdict format (*=optional):
+ * response{error}: <error code>
+ * response{errstr}: <error description>
+ * response{return_data}{methods}: list of callable RPCs
+ * response{return_data}{version}: virtagent version
+ */
+static QDict *va_capabilities(const QDict *params)
+{
+ QList *functions = qlist_new();
+ QDict *ret = qdict_new();
+ int i;
+ const char *func_name;
+
+ TRACE("called");
+ SLOG("va_capabilities()");
+
+ for (i = 0; va_server_data->functions[i].func != NULL; ++i) {
+ func_name = va_server_data->functions[i].func_name;
+ qlist_append_obj(functions, QOBJECT(qstring_from_str(func_name)));
+ }
+ qdict_put_obj(ret, "methods", QOBJECT(functions));
+ qdict_put_obj(ret, "version", QOBJECT(qstring_from_str(VA_VERSION)));
+
+ return va_server_format_response(ret, 0, NULL);
+}
+
+static VARPCFunction guest_functions[] = {
+ { .func = va_capabilities,
+ .func_name = "capabilities" },
+ { NULL, NULL }
+};
+
+static VARPCFunction host_functions[] = {
+ { .func = va_hello,
+ .func_name = "hello" },
+ { NULL, NULL }
+};
+
+static bool va_server_is_enabled(void)
+{
+ return va_server_data && va_server_data->enabled;
+}
+
+typedef struct VARequestData {
+ QDict *request;
+ QString *response;
+} VARequestData;
+
+static int va_do_server_rpc(VARequestData *d, const char *tag)
+{
+ int ret = 0, i;
+ const char *func_name;
+ VARPCFunction *func_list = va_server_data->is_host ?
+ host_functions : guest_functions;
+ QDict *response = NULL, *params = NULL;
+ bool found;
+
+ TRACE("called");
+
+ if (!va_server_is_enabled()) {
+ ret = -EBUSY;
+ goto out;
+ }
+
+ if (!d->request) {
+ ret = -EINVAL;
+ goto out;
+ }
+
+ if (!va_qdict_haskey_with_type(d->request, "method", QTYPE_QSTRING)) {
+ ret = -EINVAL;
+ va_server_job_cancel(va_server_data->manager, tag);
+ goto out;
+ }
+ func_name = qdict_get_str(d->request, "method");
+ for (i = 0; func_list[i].func != NULL; ++i) {
+ if (strcmp(func_name, func_list[i].func_name) == 0) {
+ if (va_qdict_haskey_with_type(d->request, "params", QTYPE_QDICT)) {
+ params = qdict_get_qdict(d->request, "params");
+ }
+ response = func_list[i].func(params);
+ found = true;
+ break;
+ }
+ }
+
+ if (!response) {
+ if (found) {
+ response = va_server_format_response(NULL, -1,
+ "error executing rpc");
+ } else {
+ response = va_server_format_response(NULL, -1,
+ "unsupported rpc specified");
+ }
+ }
+ /* TODO: store the json rather than the QDict that generates it */
+ d->response = qobject_to_json(QOBJECT(response));
+ if (!d->response) {
+ ret = -EINVAL;
+ goto out;
+ }
+
+ va_server_job_execute_done(va_server_data->manager, tag);
+
+out:
+ return ret;
+}
+
+int va_server_init(VAManager *m, VAServerData *server_data, bool is_host)
+{
+ va_enable_syslog = !is_host; /* enable logging for guest agent */
+ server_data->functions = is_host ? host_functions : guest_functions;
+ server_data->enabled = true;
+ server_data->is_host = is_host;
+ server_data->manager = m;
+ va_server_data = server_data;
+
+ return 0;
+}
+
+int va_server_close(void)
+{
+ if (va_server_data != NULL) {
+ va_server_data = NULL;
+ }
+ return 0;
+}
+
+/* called by VAManager to start executing the RPC */
+static int va_execute(void *opaque, const char *tag)
+{
+ VARequestData *d = opaque;
+ int ret = va_do_server_rpc(d, tag);
+ if (ret < 0) {
+ LOG("error occurred executing RPC: %s", strerror(-ret));
+ }
+
+ return ret;
+}
+
+/* called by xport layer to indicate send completion to VAManager */
+static void va_send_response_cb(const void *opaque)
+{
+ const char *tag = opaque;
+ va_server_job_send_done(va_server_data->manager, tag);
+}
+
+/* called by VAManager to start send, in turn calls out to xport layer */
+static int va_send_response(void *opaque, const char *tag)
+{
+ VARequestData *d = opaque;
+ const char *json_resp;
+ int ret;
+
+ TRACE("called, request data d: %p", opaque);
+ if (!d->response) {
+ LOG("server generated null response");
+ ret = -EINVAL;
+ goto out_cancel;
+ }
+ json_resp = qstring_get_str(d->response);
+ if (!json_resp) {
+ ret = -EINVAL;
+ LOG("server generated invalid JSON response");
+ goto out_cancel;
+ }
+
+ ret = va_xport_send_response(json_resp, strlen(json_resp),
+ tag, tag, va_send_response_cb);
+ return ret;
+out_cancel:
+ va_server_job_cancel(va_server_data->manager, tag);
+ return ret;
+}
+
+static int va_cleanup(void *opaque, const char *tag)
+{
+ VARequestData *d = opaque;
+ if (d) {
+ if (d->request) {
+ QDECREF(d->request);
+ }
+ if (d->response) {
+ QDECREF(d->response);
+ }
+ qemu_free(d);
+ }
+ return 0;
+}
+
+static VAServerJobOps server_job_ops = {
+ .execute = va_execute,
+ .send = va_send_response,
+ .callback = va_cleanup,
+};
+
+/* create server jobs from requests read from xport layer */
+int va_server_job_create(const char *content, size_t content_len, const char *tag)
+{
+ VARequestData *d = qemu_mallocz(sizeof(VAServerData));
+ QObject *request_obj;
+
+ if (!content) {
+ LOG("recieved job with null request string");
+ goto out_bad;
+ }
+
+ request_obj = qobject_from_json(content);
+ if (!request_obj) {
+ LOG("unable to parse JSON arguments");
+ goto out_bad;
+ }
+
+ d->request = qobject_to_qdict(request_obj);
+ if (!d->request) {
+ LOG("recieved qobject of unexpected type: %d",
+ qobject_type(request_obj));
+ goto out_bad_free;
+ }
+
+ if (!va_qdict_haskey_with_type(d->request, "method", QTYPE_QSTRING)) {
+ LOG("RPC command not specified");
+ goto out_bad_free;
+ }
+
+ va_server_job_add(va_server_data->manager, tag, d, server_job_ops);
+
+ return 0;
+out_bad_free:
+ if (d->request) {
+ QDECREF(d->request);
+ }
+ qemu_free(d);
+out_bad:
+ return -EINVAL;
+}
diff --git a/virtagent-server.h b/virtagent-server.h
new file mode 100644
index 0000000..1f27577
--- /dev/null
+++ b/virtagent-server.h
@@ -0,0 +1,40 @@
+/*
+ * virt-agent - host/guest RPC daemon functions
+ *
+ * Copyright IBM Corp. 2010
+ *
+ * Authors:
+ * Michael Roth <mdroth@linux.vnet.ibm.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ *
+ */
+
+#include "virtagent-manager.h"
+#include "qdict.h"
+
+#define GUEST_AGENT_SERVICE_ID "virtagent"
+#define GUEST_AGENT_PATH "/tmp/virtagent-guest.sock"
+#define HOST_AGENT_SERVICE_ID "virtagent-host"
+#define HOST_AGENT_PATH "/tmp/virtagent-host.sock"
+#define VA_GETFILE_MAX 1 << 30
+#define VA_FILEBUF_LEN 16384
+#define VA_DMESG_LEN 16384
+
+typedef struct VARPCFunction {
+ QDict *(*func)(const QDict *params);
+ const char *func_name;
+} VARPCFunction;
+
+typedef struct VAServerData {
+ bool enabled;
+ bool is_host;
+ VARPCFunction *functions;
+ VAManager *manager;
+} VAServerData;
+
+int va_server_init(VAManager *m, VAServerData *server_data, bool is_host);
+int va_server_close(void);
+//int va_do_server_rpc(const char *content, size_t content_len, const char tag[64]);
+int va_server_job_create(const char *content, size_t content_len, const char *tag);
--
1.7.0.4
^ permalink raw reply related [flat|nested] 41+ messages in thread
* [Qemu-devel] [RFC][PATCH v7 09/16] virtagent: add va_capabilities HMP/QMP command
2011-03-07 20:10 [Qemu-devel] [RFC][PATCH v7 00/16] virtagent: host/guest communication agent Michael Roth
` (7 preceding siblings ...)
2011-03-07 20:10 ` [Qemu-devel] [RFC][PATCH v7 08/16] virtagnet: base RPC server definitions Michael Roth
@ 2011-03-07 20:10 ` Michael Roth
2011-03-07 20:10 ` [Qemu-devel] [RFC][PATCH v7 10/16] virtagent: add "ping" RPC to server Michael Roth
` (7 subsequent siblings)
16 siblings, 0 replies; 41+ messages in thread
From: Michael Roth @ 2011-03-07 20:10 UTC (permalink / raw)
To: qemu-devel
Cc: agl, stefanha, Jes.Sorensen, mdroth, markus_mueller, aliguori,
abeekhof
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
hmp-commands.hx | 16 ++++++++++++++++
qmp-commands.hx | 33 +++++++++++++++++++++++++++++++++
2 files changed, 49 insertions(+), 0 deletions(-)
diff --git a/hmp-commands.hx b/hmp-commands.hx
index 372bef4..86817e2 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -1364,6 +1364,22 @@ show available trace events and their state
ETEXI
#endif
+ {
+ .name = "va_capabilities",
+ .args_type = "",
+ .params = "",
+ .help = "Fetch and re-negotiate guest agent capabilties",
+ .user_print = do_va_capabilities_print,
+ .mhandler.cmd_async = do_va_capabilities,
+ .flags = MONITOR_CMD_ASYNC,
+ },
+
+STEXI
+@item va_capabilities
+@findex va_capabilities
+Fetch and re-negotiate guest agent capabilties
+ETEXI
+
STEXI
@end table
ETEXI
diff --git a/qmp-commands.hx b/qmp-commands.hx
index df40a3d..e1092dd 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -858,6 +858,39 @@ Example:
EQMP
{
+ .name = "va_capabilities",
+ .args_type = "",
+ .params = "",
+ .help = "Fetch and re-negotiate guest agent capabilities",
+ .user_print = monitor_user_noop,
+ .mhandler.cmd_async = do_va_capabilities,
+ .flags = MONITOR_CMD_ASYNC,
+ },
+
+STEXI
+@item va_capabilities
+@findex va_capabilities
+Fetch and re-negotiate guest agent capabilities
+ETEXI
+SQMP
+va_capabilities
+--------
+
+Fetch and re-negotiate guest agent capabilities
+
+Arguments:
+
+(none)
+
+Example:
+
+-> { "execute": "va_capabilities" }
+<- { "return": { "methods": ["capabilities", "shutdown", "ping", ... ],
+ "version": "1.0" }}
+
+EQMP
+
+ {
.name = "qmp_capabilities",
.args_type = "",
.params = "",
--
1.7.0.4
^ permalink raw reply related [flat|nested] 41+ messages in thread
* [Qemu-devel] [RFC][PATCH v7 10/16] virtagent: add "ping" RPC to server
2011-03-07 20:10 [Qemu-devel] [RFC][PATCH v7 00/16] virtagent: host/guest communication agent Michael Roth
` (8 preceding siblings ...)
2011-03-07 20:10 ` [Qemu-devel] [RFC][PATCH v7 09/16] virtagent: add va_capabilities HMP/QMP command Michael Roth
@ 2011-03-07 20:10 ` Michael Roth
2011-03-07 20:10 ` [Qemu-devel] [RFC][PATCH v7 11/16] virtagent: add va_ping HMP/QMP command Michael Roth
` (6 subsequent siblings)
16 siblings, 0 replies; 41+ messages in thread
From: Michael Roth @ 2011-03-07 20:10 UTC (permalink / raw)
To: qemu-devel
Cc: agl, stefanha, Jes.Sorensen, mdroth, markus_mueller, aliguori,
abeekhof
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
virtagent-server.c | 16 ++++++++++++++++
1 files changed, 16 insertions(+), 0 deletions(-)
diff --git a/virtagent-server.c b/virtagent-server.c
index f84546b..b0fc0c4 100644
--- a/virtagent-server.c
+++ b/virtagent-server.c
@@ -101,15 +101,31 @@ static QDict *va_capabilities(const QDict *params)
return va_server_format_response(ret, 0, NULL);
}
+/* va_ping(): respond to/pong to client.
+ * params/response qdict format (*=optional):
+ * response{error}: <error code>
+ * response{errstr}: <error description>
+ */
+static QDict *va_ping(const QDict *params)
+{
+ TRACE("called");
+ SLOG("va_ping()");
+ return va_server_format_response(NULL, 0, NULL);
+}
+
static VARPCFunction guest_functions[] = {
{ .func = va_capabilities,
.func_name = "capabilities" },
+ { .func = va_ping,
+ .func_name = "ping" },
{ NULL, NULL }
};
static VARPCFunction host_functions[] = {
{ .func = va_hello,
.func_name = "hello" },
+ { .func = va_ping,
+ .func_name = "ping" },
{ NULL, NULL }
};
--
1.7.0.4
^ permalink raw reply related [flat|nested] 41+ messages in thread
* [Qemu-devel] [RFC][PATCH v7 11/16] virtagent: add va_ping HMP/QMP command
2011-03-07 20:10 [Qemu-devel] [RFC][PATCH v7 00/16] virtagent: host/guest communication agent Michael Roth
` (9 preceding siblings ...)
2011-03-07 20:10 ` [Qemu-devel] [RFC][PATCH v7 10/16] virtagent: add "ping" RPC to server Michael Roth
@ 2011-03-07 20:10 ` Michael Roth
2011-03-07 20:10 ` [Qemu-devel] [RFC][PATCH v7 12/16] virtagent: add "shutdown" RPC to server Michael Roth
` (5 subsequent siblings)
16 siblings, 0 replies; 41+ messages in thread
From: Michael Roth @ 2011-03-07 20:10 UTC (permalink / raw)
To: qemu-devel
Cc: agl, stefanha, Jes.Sorensen, mdroth, markus_mueller, aliguori,
abeekhof
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
hmp-commands.hx | 16 ++++++++++++++++
qmp-commands.hx | 32 ++++++++++++++++++++++++++++++++
virtagent.c | 45 +++++++++++++++++++++++++++++++++++++++++++++
virtagent.h | 3 +++
4 files changed, 96 insertions(+), 0 deletions(-)
diff --git a/hmp-commands.hx b/hmp-commands.hx
index 86817e2..f22117a 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -1380,6 +1380,22 @@ STEXI
Fetch and re-negotiate guest agent capabilties
ETEXI
+ {
+ .name = "va_ping",
+ .args_type = "",
+ .params = "",
+ .help = "Ping the guest agent",
+ .user_print = do_va_ping_print,
+ .mhandler.cmd_async = do_va_ping,
+ .flags = MONITOR_CMD_ASYNC,
+ },
+
+STEXI
+@item va_ping
+@findex va_ping
+Ping the guest agent
+ETEXI
+
STEXI
@end table
ETEXI
diff --git a/qmp-commands.hx b/qmp-commands.hx
index e1092dd..0379b61 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -891,6 +891,38 @@ Example:
EQMP
{
+ .name = "va_ping",
+ .args_type = "",
+ .params = "",
+ .help = "Ping the guest agent",
+ .user_print = monitor_user_noop,
+ .mhandler.cmd_async = do_va_ping,
+ .flags = MONITOR_CMD_ASYNC,
+ },
+
+STEXI
+@item va_ping
+@findex va_ping
+Ping the guest agent
+ETEXI
+SQMP
+va_ping
+--------
+
+Ping the guest agent
+
+Arguments:
+
+(none)
+
+Example:
+
+-> { "execute": "va_ping" }
+<- { "return": {"status": "ok" }}
+
+EQMP
+
+ {
.name = "qmp_capabilities",
.args_type = "",
.params = "",
diff --git a/virtagent.c b/virtagent.c
index 670309b..baf3c0e 100644
--- a/virtagent.c
+++ b/virtagent.c
@@ -431,6 +431,51 @@ int do_va_capabilities(Monitor *mon, const QDict *params,
return ret;
}
+void do_va_ping_print(Monitor *mon, const QObject *data)
+{
+ QDict *ret = qobject_to_qdict(data);
+
+ TRACE("called");
+
+ if (!data) {
+ return;
+ }
+ monitor_printf(mon, "status: %s\n", qdict_get_str(ret, "status"));
+}
+
+static void do_va_ping_cb(QDict *resp,
+ MonitorCompletion *mon_cb,
+ void *mon_data)
+{
+ QDict *ret = qdict_new();
+ const char *status;
+
+ if (va_check_response_ok(resp, NULL)) {
+ status = "success";
+ } else {
+ status = "error or timeout";
+ }
+ qdict_put_obj(ret, "status", QOBJECT(qstring_from_str(status)));
+
+ if (mon_cb) {
+ mon_cb(mon_data, QOBJECT(ret));
+ }
+ QDECREF(ret);
+}
+
+/*
+ * do_va_ping(): Ping the guest agent
+ */
+int do_va_ping(Monitor *mon, const QDict *params,
+ MonitorCompletion cb, void *opaque)
+{
+ int ret = va_do_rpc("ping", params, do_va_ping_cb, cb, opaque);
+ if (ret) {
+ qerror_report(QERR_VA_FAILED, ret, strerror(-ret));
+ }
+ return ret;
+}
+
/* RPC client functions called outside of HMP/QMP */
int va_client_init_capabilities(void)
diff --git a/virtagent.h b/virtagent.h
index 1652fdc..a58d8ba 100644
--- a/virtagent.h
+++ b/virtagent.h
@@ -42,5 +42,8 @@ int do_agent_shutdown(Monitor *mon, const QDict *mon_params,
void do_va_capabilities_print(Monitor *mon, const QObject *qobject);
int do_va_capabilities(Monitor *mon, const QDict *mon_params,
MonitorCompletion cb, void *opaque);
+void do_va_ping_print(Monitor *mon, const QObject *qobject);
+int do_va_ping(Monitor *mon, const QDict *mon_params,
+ MonitorCompletion cb, void *opaque);
#endif /* VIRTAGENT_H */
--
1.7.0.4
^ permalink raw reply related [flat|nested] 41+ messages in thread
* [Qemu-devel] [RFC][PATCH v7 12/16] virtagent: add "shutdown" RPC to server
2011-03-07 20:10 [Qemu-devel] [RFC][PATCH v7 00/16] virtagent: host/guest communication agent Michael Roth
` (10 preceding siblings ...)
2011-03-07 20:10 ` [Qemu-devel] [RFC][PATCH v7 11/16] virtagent: add va_ping HMP/QMP command Michael Roth
@ 2011-03-07 20:10 ` Michael Roth
2011-03-07 20:10 ` [Qemu-devel] [RFC][PATCH v7 13/16] virtagent: add va_shutdown HMP/QMP command Michael Roth
` (4 subsequent siblings)
16 siblings, 0 replies; 41+ messages in thread
From: Michael Roth @ 2011-03-07 20:10 UTC (permalink / raw)
To: qemu-devel
Cc: agl, stefanha, Jes.Sorensen, mdroth, markus_mueller, aliguori,
abeekhof
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
virtagent-server.c | 58 ++++++++++++++++++++++++++++++++++++++++++++++++++++
1 files changed, 58 insertions(+), 0 deletions(-)
diff --git a/virtagent-server.c b/virtagent-server.c
index b0fc0c4..3c8c805 100644
--- a/virtagent-server.c
+++ b/virtagent-server.c
@@ -113,11 +113,69 @@ static QDict *va_ping(const QDict *params)
return va_server_format_response(NULL, 0, NULL);
}
+/* va_shutdown(): initiate guest shutdown
+ * params/response qdict format:
+ * params{shutdown_mode}: "reboot"|"powerdown"|"shutdown"
+ * response{error}: <error code>
+ * response{errstr}: <error description>
+ */
+static QDict *va_shutdown(const QDict *params)
+{
+ int ret;
+ const char *shutdown_mode, *shutdown_flag;
+
+ shutdown_mode = qdict_get_try_str(params, "shutdown_mode");
+ SLOG("va_shutdown(), shutdown_mode:%s", shutdown_mode);
+
+ if (!shutdown_mode) {
+ ret = -EINVAL;
+ LOG("missing shutdown argument");
+ goto out;
+ } else if (strcmp(shutdown_mode, "halt") == 0) {
+ shutdown_flag = "-H";
+ } else if (strcmp(shutdown_mode, "powerdown") == 0) {
+ shutdown_flag = "-P";
+ } else if (strcmp(shutdown_mode, "reboot") == 0) {
+ shutdown_flag = "-r";
+ } else {
+ ret = -EINVAL;
+ LOG("invalid shutdown argument");
+ goto out;
+ }
+
+ ret = fork();
+ if (ret == 0) {
+ /* child, start the shutdown */
+ setsid();
+ fclose(stdin);
+ fclose(stdout);
+ fclose(stderr);
+
+ sleep(5);
+ ret = execl("/sbin/shutdown", "shutdown", shutdown_flag, "+0",
+ "hypervisor initiated shutdown", (char*)NULL);
+ if (ret < 0) {
+ LOG("execl() failed: %s", strerror(errno));
+ exit(1);
+ }
+ exit(0);
+ } else if (ret < 0) {
+ LOG("fork() failed: %s", strerror(errno));
+ } else {
+ ret = 0;
+ }
+
+out:
+ return va_server_format_response(NULL, ret, strerror(errno));
+}
+
static VARPCFunction guest_functions[] = {
{ .func = va_capabilities,
.func_name = "capabilities" },
{ .func = va_ping,
.func_name = "ping" },
+ { .func = va_shutdown,
+ .func_name = "shutdown" },
{ NULL, NULL }
};
--
1.7.0.4
^ permalink raw reply related [flat|nested] 41+ messages in thread
* [Qemu-devel] [RFC][PATCH v7 13/16] virtagent: add va_shutdown HMP/QMP command
2011-03-07 20:10 [Qemu-devel] [RFC][PATCH v7 00/16] virtagent: host/guest communication agent Michael Roth
` (11 preceding siblings ...)
2011-03-07 20:10 ` [Qemu-devel] [RFC][PATCH v7 12/16] virtagent: add "shutdown" RPC to server Michael Roth
@ 2011-03-07 20:10 ` Michael Roth
2011-03-07 20:10 ` [Qemu-devel] [RFC][PATCH v7 14/16] virtagent: add virtagent chardev Michael Roth
` (3 subsequent siblings)
16 siblings, 0 replies; 41+ messages in thread
From: Michael Roth @ 2011-03-07 20:10 UTC (permalink / raw)
To: qemu-devel
Cc: agl, stefanha, Jes.Sorensen, mdroth, markus_mueller, aliguori,
abeekhof
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
hmp-commands.hx | 16 ++++++++++++++++
qmp-commands.hx | 32 ++++++++++++++++++++++++++++++++
virtagent.c | 24 ++++++++++++++++++++++++
virtagent.h | 2 ++
4 files changed, 74 insertions(+), 0 deletions(-)
diff --git a/hmp-commands.hx b/hmp-commands.hx
index f22117a..982ba25 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -1396,6 +1396,22 @@ STEXI
Ping the guest agent
ETEXI
+ {
+ .name = "va_shutdown",
+ .args_type = "shutdown_mode:s",
+ .params = "shutdown_mode",
+ .help = "Start guest-initiated reboot/halt/powerdown",
+ .user_print = monitor_user_noop,
+ .mhandler.cmd_async = do_va_shutdown,
+ .flags = MONITOR_CMD_ASYNC,
+ },
+
+STEXI
+@item va_shutdown
+@findex va_shutdown
+Start guest-initiated reboot/halt/powerdown
+ETEXI
+
STEXI
@end table
ETEXI
diff --git a/qmp-commands.hx b/qmp-commands.hx
index 0379b61..dc25021 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -923,6 +923,38 @@ Example:
EQMP
{
+ .name = "va_shutdown",
+ .args_type = "shutdown_mode:s",
+ .params = "shutdown_mode",
+ .help = "reboot|halt|powerdown the guest",
+ .user_print = monitor_user_noop,
+ .mhandler.cmd_async = do_va_shutdown,
+ .flags = MONITOR_CMD_ASYNC,
+ },
+
+STEXI
+@item va_shutdown
+@findex va_shutdown
+reboot|halt|powerdown the guest
+ETEXI
+SQMP
+va_shutdown
+--------
+
+reboot|halt|powerdown the guest
+
+Arguments:
+
+- "shutdown_mode": "reboot"|"halt"|"powerdown"
+
+Example:
+
+-> { "execute": "va_shutdown", "arguments": { "shutdown_mode": "reboot" } }
+<- { "return": {} }
+
+EQMP
+
+ {
.name = "qmp_capabilities",
.args_type = "",
.params = "",
diff --git a/virtagent.c b/virtagent.c
index baf3c0e..7d2566e 100644
--- a/virtagent.c
+++ b/virtagent.c
@@ -476,6 +476,30 @@ int do_va_ping(Monitor *mon, const QDict *params,
return ret;
}
+static void do_va_shutdown_cb(QDict *resp,
+ MonitorCompletion *mon_cb,
+ void *mon_data)
+{
+ TRACE("called");
+ va_check_response_ok(resp, NULL);
+ if (mon_cb) {
+ mon_cb(mon_data, NULL);
+ }
+}
+
+/*
+ * do_va_shutdown(): shutdown/powerdown/reboot the guest
+ */
+int do_va_shutdown(Monitor *mon, const QDict *params,
+ MonitorCompletion cb, void *opaque)
+{
+ int ret = va_do_rpc("shutdown", params, do_va_shutdown_cb, cb, opaque);
+ if (ret) {
+ qerror_report(QERR_VA_FAILED, ret, strerror(-ret));
+ }
+ return ret;
+}
+
/* RPC client functions called outside of HMP/QMP */
int va_client_init_capabilities(void)
diff --git a/virtagent.h b/virtagent.h
index a58d8ba..08c1004 100644
--- a/virtagent.h
+++ b/virtagent.h
@@ -45,5 +45,7 @@ int do_va_capabilities(Monitor *mon, const QDict *mon_params,
void do_va_ping_print(Monitor *mon, const QObject *qobject);
int do_va_ping(Monitor *mon, const QDict *mon_params,
MonitorCompletion cb, void *opaque);
+int do_va_shutdown(Monitor *mon, const QDict *params,
+ MonitorCompletion cb, void *opaque);
#endif /* VIRTAGENT_H */
--
1.7.0.4
^ permalink raw reply related [flat|nested] 41+ messages in thread
* [Qemu-devel] [RFC][PATCH v7 14/16] virtagent: add virtagent chardev
2011-03-07 20:10 [Qemu-devel] [RFC][PATCH v7 00/16] virtagent: host/guest communication agent Michael Roth
` (12 preceding siblings ...)
2011-03-07 20:10 ` [Qemu-devel] [RFC][PATCH v7 13/16] virtagent: add va_shutdown HMP/QMP command Michael Roth
@ 2011-03-07 20:10 ` Michael Roth
2011-03-07 20:10 ` [Qemu-devel] [RFC][PATCH v7 15/16] virtagent: qemu-va, system-level virtagent guest agent Michael Roth
` (2 subsequent siblings)
16 siblings, 0 replies; 41+ messages in thread
From: Michael Roth @ 2011-03-07 20:10 UTC (permalink / raw)
To: qemu-devel
Cc: agl, stefanha, Jes.Sorensen, mdroth, markus_mueller, aliguori,
abeekhof
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
qemu-char.c | 44 ++++++++++++++++++++++++++++++++++++++++++++
1 files changed, 44 insertions(+), 0 deletions(-)
diff --git a/qemu-char.c b/qemu-char.c
index bd4e944..ffdcadb 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -2458,6 +2458,49 @@ fail:
return NULL;
}
+#include "virtagent-common.h"
+
+static CharDriverState *qemu_chr_open_virtagent(QemuOpts *opts)
+{
+ CharDriverState *chr;
+ const char *path;
+ VAContext ctx;
+ int ret;
+
+ /* revert to/enforce default socket chardev options for virtagent */
+ path = qemu_opt_get(opts, "path");
+ if (path == NULL) {
+ path = VA_HOST_PATH_DEFAULT;
+ }
+ qemu_opt_set(opts, "path", path);
+ qemu_opt_set(opts, "server", "on");
+ qemu_opt_set(opts, "wait", "off");
+ qemu_opt_set(opts, "telnet", "off");
+
+ chr = qemu_chr_open_socket(opts);
+ if (chr == NULL) {
+ goto err;
+ }
+
+ /* initialize virtagent using the socket we just set up */
+ ctx.channel_method = "unix-connect";
+ ctx.channel_path = path;
+ ctx.is_host = true;
+ ret = va_init(ctx);
+ ret = 0;
+ if (ret != 0) {
+ fprintf(stderr, "error initializing virtagent");
+ goto err;
+ }
+
+ return chr;
+err:
+ if (chr) {
+ qemu_free(chr);
+ }
+ return NULL;
+}
+
static const struct {
const char *name;
CharDriverState *(*open)(QemuOpts *opts);
@@ -2467,6 +2510,7 @@ static const struct {
{ .name = "udp", .open = qemu_chr_open_udp },
{ .name = "msmouse", .open = qemu_chr_open_msmouse },
{ .name = "vc", .open = text_console_init },
+ { .name = "virtagent", .open = qemu_chr_open_virtagent },
#ifdef _WIN32
{ .name = "file", .open = qemu_chr_open_win_file_out },
{ .name = "pipe", .open = qemu_chr_open_win_pipe },
--
1.7.0.4
^ permalink raw reply related [flat|nested] 41+ messages in thread
* [Qemu-devel] [RFC][PATCH v7 15/16] virtagent: qemu-va, system-level virtagent guest agent
2011-03-07 20:10 [Qemu-devel] [RFC][PATCH v7 00/16] virtagent: host/guest communication agent Michael Roth
` (13 preceding siblings ...)
2011-03-07 20:10 ` [Qemu-devel] [RFC][PATCH v7 14/16] virtagent: add virtagent chardev Michael Roth
@ 2011-03-07 20:10 ` Michael Roth
2011-03-09 10:48 ` [Qemu-devel] " Jes Sorensen
2011-03-07 20:10 ` [Qemu-devel] [RFC][PATCH v7 16/16] virtagent: add bits to build virtagent host/guest components Michael Roth
2011-03-07 21:43 ` [Qemu-devel] Re: [RFC][PATCH v7 00/16] virtagent: host/guest communication agent Anthony Liguori
16 siblings, 1 reply; 41+ messages in thread
From: Michael Roth @ 2011-03-07 20:10 UTC (permalink / raw)
To: qemu-devel
Cc: agl, stefanha, Jes.Sorensen, mdroth, markus_mueller, aliguori,
abeekhof
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
qemu-va.c | 247 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
1 files changed, 247 insertions(+), 0 deletions(-)
create mode 100644 qemu-va.c
diff --git a/qemu-va.c b/qemu-va.c
new file mode 100644
index 0000000..a9ff56f
--- /dev/null
+++ b/qemu-va.c
@@ -0,0 +1,247 @@
+/*
+ * virtagent - QEMU guest agent
+ *
+ * Copyright IBM Corp. 2010
+ *
+ * Authors:
+ * Michael Roth <mdroth@linux.vnet.ibm.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ *
+ * QEMU System Emulator
+ *
+ * Copyright (c) 2003-2008 Fabrice Bellard
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#include <getopt.h>
+#include <err.h>
+#include "qemu-ioh.h"
+#include "qemu-tool.h"
+#include "virtagent-common.h"
+
+static bool verbose_enabled;
+#define DEBUG_ENABLED
+
+#ifdef DEBUG_ENABLED
+#define DEBUG(msg, ...) do { \
+ fprintf(stderr, "%s:%s():L%d: " msg "\n", \
+ __FILE__, __FUNCTION__, __LINE__, ## __VA_ARGS__); \
+} while(0)
+#else
+#define DEBUG(msg, ...) do {} while (0)
+#endif
+
+#define INFO(msg, ...) do { \
+ if (!verbose_enabled) { \
+ break; \
+ } \
+ warnx(msg, ## __VA_ARGS__); \
+} while(0)
+
+/* mirror qemu I/O loop for standalone daemon */
+static void main_loop_wait(int nonblocking)
+{
+ fd_set rfds, wfds, xfds;
+ int ret, nfds;
+ struct timeval tv;
+ int timeout = 100000;
+
+ if (nonblocking) {
+ timeout = 0;
+ }
+
+ /* poll any events */
+ nfds = -1;
+ FD_ZERO(&rfds);
+ FD_ZERO(&wfds);
+ FD_ZERO(&xfds);
+ qemu_get_fdset(&nfds, &rfds, &wfds, &xfds);
+
+ tv.tv_sec = timeout / 1000;
+ tv.tv_usec = (timeout % 1000) * 1000;
+
+ ret = select(nfds + 1, &rfds, &wfds, &xfds, &tv);
+
+ if (ret > 0) {
+ qemu_process_fd_handlers(&rfds, &wfds, &xfds);
+ }
+
+ DEBUG("running timers...");
+ qemu_run_all_timers();
+}
+
+static void usage(const char *cmd)
+{
+ printf(
+"Usage: %s -c <channel_opts>\n"
+"QEMU virtagent guest agent %s\n"
+"\n"
+" -c, --channel channel method: one of unix-connect, virtio-serial, or\n"
+" isa-serial\n"
+" -p, --path channel path\n"
+" -v, --verbose display extra debugging information\n"
+" -d, --daemonize become a daemon\n"
+" -h, --help display this help and exit\n"
+"\n"
+"Report bugs to <mdroth@linux.vnet.ibm.com>\n"
+ , cmd, VA_VERSION);
+}
+
+static int init_virtagent(const char *method, const char *path) {
+ VAContext ctx;
+ int ret;
+
+ INFO("initializing agent...");
+
+ if (method == NULL) {
+ /* try virtio-serial as our default */
+ method = "virtio-serial";
+ }
+
+ if (path == NULL) {
+ if (strcmp(method, "virtio-serial")) {
+ errx(EXIT_FAILURE, "must specify a path for this channel");
+ }
+ /* try the default name for the virtio-serial port */
+ path = VA_GUEST_PATH_VIRTIO_DEFAULT;
+ }
+
+ /* initialize virtagent */
+ ctx.is_host = false;
+ ctx.channel_method = method;
+ ctx.channel_path = path;
+ ret = va_init(ctx);
+ if (ret) {
+ errx(EXIT_FAILURE, "unable to initialize virtagent");
+ }
+
+ return 0;
+}
+
+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(VA_PIDFILE, O_CREAT|O_RDWR, S_IRUSR|S_IWUSR);
+ if (!pidfd || lockf(pidfd, F_TLOCK, 0))
+ errx(EXIT_FAILURE, "Cannot lock pid file");
+
+ if (ftruncate(pidfd, 0) || lseek(pidfd, 0, SEEK_SET))
+ errx(EXIT_FAILURE, "Cannot truncate pid file");
+ if (asprintf(&pidstr, "%d", getpid()) == -1)
+ errx(EXIT_FAILURE, "Cannot allocate memory");
+ if (write(pidfd, pidstr, strlen(pidstr)) != strlen(pidstr))
+ errx(EXIT_FAILURE, "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(VA_PIDFILE);
+ exit(EXIT_FAILURE);
+}
+
+int main(int argc, char **argv)
+{
+ const char *sopt = "hVvdc:p:", *channel_method = NULL, *channel_path = NULL;
+ struct option lopt[] = {
+ { "help", 0, NULL, 'h' },
+ { "version", 0, NULL, 'V' },
+ { "verbose", 0, NULL, 'v' },
+ { "channel", 0, NULL, 'c' },
+ { "path", 0, NULL, 'p' },
+ { "daemonize", 0, NULL, 'd' },
+ { NULL, 0, NULL, 0 }
+ };
+ int opt_ind = 0, ch, ret, daemonize = 0;
+
+ while ((ch = getopt_long(argc, argv, sopt, lopt, &opt_ind)) != -1) {
+ switch (ch) {
+ case 'c':
+ channel_method = optarg;
+ break;
+ case 'p':
+ channel_path = optarg;
+ break;
+ case 'v':
+ verbose_enabled = 1;
+ break;
+ case 'V':
+ printf("QEMU Virtagent %s\n", VA_VERSION);
+ return 0;
+ case 'd':
+ daemonize = 1;
+ break;
+ case 'h':
+ usage(argv[0]);
+ return 0;
+ case '?':
+ errx(EXIT_FAILURE, "Try '%s --help' for more information.",
+ argv[0]);
+ }
+ }
+
+ if (daemonize) {
+ become_daemon();
+ }
+
+ init_clocks();
+ configure_alarms("dynticks");
+ if (init_timer_alarm() < 0) {
+ errx(EXIT_FAILURE, "could not initialize alarm timer");
+ }
+
+ /* initialize virtagent */
+ ret = init_virtagent(channel_method, channel_path);
+ if (ret) {
+ errx(EXIT_FAILURE, "error initializing communication channel");
+ }
+
+ /* main i/o loop */
+ for (;;) {
+ DEBUG("entering main_loop_wait()");
+ main_loop_wait(0);
+ DEBUG("left main_loop_wait()");
+ }
+
+ unlink(VA_PIDFILE);
+ return 0;
+}
--
1.7.0.4
^ permalink raw reply related [flat|nested] 41+ messages in thread
* [Qemu-devel] [RFC][PATCH v7 16/16] virtagent: add bits to build virtagent host/guest components
2011-03-07 20:10 [Qemu-devel] [RFC][PATCH v7 00/16] virtagent: host/guest communication agent Michael Roth
` (14 preceding siblings ...)
2011-03-07 20:10 ` [Qemu-devel] [RFC][PATCH v7 15/16] virtagent: qemu-va, system-level virtagent guest agent Michael Roth
@ 2011-03-07 20:10 ` Michael Roth
2011-03-07 21:43 ` [Qemu-devel] Re: [RFC][PATCH v7 00/16] virtagent: host/guest communication agent Anthony Liguori
16 siblings, 0 replies; 41+ messages in thread
From: Michael Roth @ 2011-03-07 20:10 UTC (permalink / raw)
To: qemu-devel
Cc: agl, stefanha, Jes.Sorensen, mdroth, markus_mueller, aliguori,
abeekhof
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
Makefile | 4 +++-
Makefile.target | 2 +-
2 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/Makefile b/Makefile
index eca4c76..46f5730 100644
--- a/Makefile
+++ b/Makefile
@@ -151,7 +151,7 @@ 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-va.o: $(GENERATED_HEADERS)
qemu-img$(EXESUF): qemu-img.o qemu-tool.o qemu-error.o $(oslib-obj-y) $(trace-obj-y) $(block-obj-y) $(qobject-obj-y) $(version-obj-y) qemu-timer-common.o
@@ -159,6 +159,8 @@ qemu-nbd$(EXESUF): qemu-nbd.o qemu-tool.o qemu-error.o $(oslib-obj-y) $(trace-ob
qemu-io$(EXESUF): qemu-io.o cmd.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-va$(EXESUF): qemu-va.o virtagent.o virtagent-server.o virtagent-common.o virtagent-transport.o virtagent-manager.o qemu-tool.o qemu-error.o qemu-sockets.c $(oslib-obj-y) $(trace-obj-y) $(block-obj-y) $(qobject-obj-y) $(version-obj-y) qemu-timer-common.o qemu-timer.o
+
qemu-img-cmds.h: $(SRC_PATH)/qemu-img-cmds.hx
$(call quiet-command,sh $(SRC_PATH)/scripts/hxtool -h < $< > $@," GEN $@")
diff --git a/Makefile.target b/Makefile.target
index f0df98e..698e9a7 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -186,7 +186,7 @@ endif #CONFIG_BSD_USER
# System emulator target
ifdef CONFIG_SOFTMMU
-obj-y = arch_init.o cpus.o monitor.o machine.o gdbstub.o balloon.o
+obj-y = arch_init.o cpus.o monitor.o machine.o gdbstub.o balloon.o virtagent.o virtagent-server.o virtagent-common.o virtagent-transport.o virtagent-manager.o
# virtio has to be here due to weird dependency between PCI and virtio-net.
# need to fix this properly
obj-$(CONFIG_NO_PCI) += pci-stub.o
--
1.7.0.4
^ permalink raw reply related [flat|nested] 41+ messages in thread
* [Qemu-devel] Re: [RFC][PATCH v7 04/16] virtagent: bi-directional RPC handling logic
2011-03-07 20:10 ` [Qemu-devel] [RFC][PATCH v7 04/16] virtagent: bi-directional RPC handling logic Michael Roth
@ 2011-03-07 21:24 ` Adam Litke
2011-03-07 22:35 ` Michael Roth
0 siblings, 1 reply; 41+ messages in thread
From: Adam Litke @ 2011-03-07 21:24 UTC (permalink / raw)
To: Michael Roth
Cc: stefanha, markus_mueller, qemu-devel, abeekhof, aliguori,
Jes.Sorensen
On Mon, 2011-03-07 at 14:10 -0600, Michael Roth wrote:
> This implements the state machine/logic used to manage
> send/receive/execute phases of RPCs we send or receive. It does so using
> a set of abstract methods we implement with the application and
> transport level code which will follow.
>
> Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
> ---
> virtagent-manager.c | 326 +++++++++++++++++++++++++++++++++++++++++++++++++++
> virtagent-manager.h | 130 ++++++++++++++++++++
> 2 files changed, 456 insertions(+), 0 deletions(-)
> create mode 100644 virtagent-manager.c
> create mode 100644 virtagent-manager.h
>
> diff --git a/virtagent-manager.c b/virtagent-manager.c
> new file mode 100644
> index 0000000..51d26a3
> --- /dev/null
> +++ b/virtagent-manager.c
> @@ -0,0 +1,326 @@
> +/*
> + * virtagent - job queue management
> + *
> + * Copyright IBM Corp. 2011
> + *
> + * Authors:
> + * Michael Roth <mdroth@linux.vnet.ibm.com>
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> + * See the COPYING file in the top-level directory.
> + *
> + */
> +
> +#include "virtagent-common.h"
> +
> +typedef struct VAServerJob {
> + char tag[64];
> + void *opaque;
> + VAServerJobOps ops;
> + QTAILQ_ENTRY(VAServerJob) next;
> + enum {
> + VA_SERVER_JOB_STATE_NEW = 0,
> + VA_SERVER_JOB_STATE_BUSY,
> + VA_SERVER_JOB_STATE_EXECUTED,
> + VA_SERVER_JOB_STATE_SENT,
> + VA_SERVER_JOB_STATE_DONE,
> + } state;
> +} VAServerJob;
> +
> +typedef struct VAClientJob {
> + char tag[64];
> + void *opaque;
> + void *resp_opaque;
> + VAClientJobOps ops;
> + QTAILQ_ENTRY(VAClientJob) next;
> + enum {
> + VA_CLIENT_JOB_STATE_NEW = 0,
> + VA_CLIENT_JOB_STATE_BUSY,
> + VA_CLIENT_JOB_STATE_SENT,
> + VA_CLIENT_JOB_STATE_READ,
> + VA_CLIENT_JOB_STATE_DONE,
> + } state;
> +} VAClientJob;
> +
> +#define SEND_COUNT_MAX 1
> +#define EXECUTE_COUNT_MAX 4
It's not immediately clear what the difference between SEND_COUNT_MAX
and EXECUTE_COUNT_MAX is. Some comments would help. Also, will the
code work if these numbers are changed? If not, a note about what
someone needs to look at when changing these would seem appropriate.
> +
> +struct VAManager {
> + int send_count; /* sends in flight */
> + int execute_count; /* number of jobs currently executing */
> + QTAILQ_HEAD(, VAServerJob) server_jobs;
> + QTAILQ_HEAD(, VAClientJob) client_jobs;
> +};
> +
> +/* server job operations/helpers */
> +
> +static VAServerJob *va_server_job_by_tag(VAManager *m, const char *tag)
> +{
> + VAServerJob *j;
> + QTAILQ_FOREACH(j, &m->server_jobs, next) {
> + if (strcmp(j->tag, tag) == 0) {
> + return j;
> + }
> + }
> + return NULL;
> +}
> +
> +int va_server_job_add(VAManager *m, const char *tag, void *opaque,
> + VAServerJobOps ops)
> +{
> + VAServerJob *j = qemu_mallocz(sizeof(VAServerJob));
> + TRACE("called");
Qemu has a good tracing infrastructure. If this is trace point is
useful enough to keep around, it should try to use that. If it's not
that important, I'd remove it entirely. I believe this has been flagged
in an earlier RFC too.
> + j->state = VA_SERVER_JOB_STATE_NEW;
> + j->ops = ops;
> + j->opaque = opaque;
> + memset(j->tag, 0, 64);
> + pstrcpy(j->tag, 63, tag);
Magic numbers. Should use something like #define TAG_LEN 64
> + QTAILQ_INSERT_TAIL(&m->server_jobs, j, next);
> + va_kick(m);
> + return 0;
> +}
> +
> +static void va_server_job_execute(VAServerJob *j)
> +{
> + TRACE("called");
> + j->state = VA_SERVER_JOB_STATE_BUSY;
> + j->ops.execute(j->opaque, j->tag);
> +}
> +
> +/* TODO: need a way to pass information back */
> +void va_server_job_execute_done(VAManager *m, const char *tag)
> +{
> + VAServerJob *j = va_server_job_by_tag(m, tag);
> + TRACE("called");
> + if (!j) {
> + LOG("server job with tag \"%s\" not found", tag);
> + return;
> + }
> + j->state = VA_SERVER_JOB_STATE_EXECUTED;
> + va_kick(m);
> +}
> +
> +static void va_server_job_send(VAServerJob *j)
> +{
> + TRACE("called");
> + j->state = VA_SERVER_JOB_STATE_BUSY;
> + j->ops.send(j->opaque, j->tag);
> +}
> +
> +void va_server_job_send_done(VAManager *m, const char *tag)
> +{
> + VAServerJob *j = va_server_job_by_tag(m, tag);
> + TRACE("called");
> + if (!j) {
> + LOG("server job with tag \"%s\" not found", tag);
> + return;
> + }
> + j->state = VA_SERVER_JOB_STATE_SENT;
> + m->send_count--;
> + va_kick(m);
> +}
> +
> +static void va_server_job_callback(VAServerJob *j)
> +{
> + TRACE("called");
> + j->state = VA_SERVER_JOB_STATE_BUSY;
> + if (j->ops.callback) {
> + j->ops.callback(j->opaque, j->tag);
> + }
> + j->state = VA_SERVER_JOB_STATE_DONE;
> +}
> +
> +void va_server_job_cancel(VAManager *m, const char *tag)
> +{
> + VAServerJob *j = va_server_job_by_tag(m, tag);
> + TRACE("called");
> + if (!j) {
> + LOG("server job with tag \"%s\" not found", tag);
> + return;
> + }
> + /* TODO: need to decrement sends/execs in flight appropriately */
> + /* make callback and move to done state, kick() will handle cleanup */
> + va_server_job_callback(j);
> + va_kick(m);
> +}
> +
> +/* client job operations */
> +
> +static VAClientJob *va_client_job_by_tag(VAManager *m, const char *tag)
> +{
> + VAClientJob *j;
> + QTAILQ_FOREACH(j, &m->client_jobs, next) {
> + if (strcmp(j->tag, tag) == 0) {
> + return j;
> + }
> + }
> + return NULL;
> +}
> +
> +int va_client_job_add(VAManager *m, const char *tag, void *opaque,
> + VAClientJobOps ops)
> +{
> + VAClientJob *j = qemu_mallocz(sizeof(VAClientJob));
> + TRACE("called");
> + j->ops = ops;
> + j->opaque = opaque;
> + memset(j->tag, 0, 64);
> + pstrcpy(j->tag, 63, tag);
> + QTAILQ_INSERT_TAIL(&m->client_jobs, j, next);
> + va_kick(m);
> + return 0;
> +}
> +
> +static void va_client_job_send(VAClientJob *j)
> +{
> + TRACE("called");
> + j->state = VA_CLIENT_JOB_STATE_BUSY;
> + j->ops.send(j->opaque, j->tag);
> +}
> +
> +void va_client_job_send_done(VAManager *m, const char *tag)
> +{
> + VAClientJob *j = va_client_job_by_tag(m, tag);
> + TRACE("called");
> + if (!j) {
> + LOG("client job with tag \"%s\" not found", tag);
> + return;
> + }
> + j->state = VA_CLIENT_JOB_STATE_SENT;
> + m->send_count--;
> + va_kick(m);
> +}
> +
> +void va_client_job_read_done(VAManager *m, const char *tag, void *resp)
> +{
> + VAClientJob *j = va_client_job_by_tag(m, tag);
> + TRACE("called");
> + if (!j) {
> + LOG("client job with tag \"%s\" not found", tag);
> + return;
> + }
> + j->state = VA_CLIENT_JOB_STATE_READ;
> + j->resp_opaque = resp;
> + va_kick(m);
> +}
> +
> +static void va_client_job_callback(VAClientJob *j)
> +{
> + TRACE("called");
> + j->state = VA_CLIENT_JOB_STATE_BUSY;
> + if (j->ops.callback) {
> + j->ops.callback(j->opaque, j->resp_opaque, j->tag);
> + }
> + j->state = VA_CLIENT_JOB_STATE_DONE;
> +}
> +
> +void va_client_job_cancel(VAManager *m, const char *tag)
> +{
> + VAClientJob *j = va_client_job_by_tag(m, tag);
> + TRACE("called");
> + if (!j) {
> + LOG("client job with tag \"%s\" not found", tag);
> + return;
> + }
> + /* TODO: need to decrement sends/execs in flight appropriately */
> + /* make callback and move to done state, kick() will handle cleanup */
> + va_client_job_callback(j);
> + va_kick(m);
> +}
> +
> +/* general management functions */
> +
> +VAManager *va_manager_new(void)
> +{
> + VAManager *m = qemu_mallocz(sizeof(VAManager));
> + QTAILQ_INIT(&m->client_jobs);
> + QTAILQ_INIT(&m->server_jobs);
> + return m;
> +}
> +
> +static void va_process_server_job(VAManager *m, VAServerJob *sj)
> +{
> + switch (sj->state) {
> + case VA_SERVER_JOB_STATE_NEW:
> + TRACE("marker");
> + va_server_job_execute(sj);
> + break;
> + case VA_SERVER_JOB_STATE_EXECUTED:
> + TRACE("marker");
> + if (m->send_count < SEND_COUNT_MAX) {
> + TRACE("marker");
> + va_server_job_send(sj);
> + m->send_count++;
> + }
> + break;
> + case VA_SERVER_JOB_STATE_SENT:
> + TRACE("marker");
> + va_server_job_callback(sj);
> + break;
> + case VA_SERVER_JOB_STATE_BUSY:
> + TRACE("marker, server job currently busy");
> + break;
> + case VA_SERVER_JOB_STATE_DONE:
> + TRACE("marker");
> + QTAILQ_REMOVE(&m->server_jobs, sj, next);
> + break;
> + default:
> + LOG("error, unknown server job state");
> + break;
> + }
> +}
> +
> +static void va_process_client_job(VAManager *m, VAClientJob *cj)
> +{
> + switch (cj->state) {
> + case VA_CLIENT_JOB_STATE_NEW:
> + TRACE("marker");
> + if (m->send_count < SEND_COUNT_MAX) {
> + TRACE("marker");
> + va_client_job_send(cj);
> + m->send_count++;
> + }
> + break;
> + case VA_CLIENT_JOB_STATE_SENT:
> + TRACE("marker");
> + //nothing to do here, awaiting read_done()
> + break;
> + case VA_CLIENT_JOB_STATE_READ:
> + TRACE("marker");
> + va_client_job_callback(cj);
> + break;
> + case VA_CLIENT_JOB_STATE_DONE:
> + TRACE("marker");
> + QTAILQ_REMOVE(&m->client_jobs, cj, next);
> + break;
> + case VA_CLIENT_JOB_STATE_BUSY:
> + TRACE("marker, client job currently busy");
> + break;
> + default:
> + LOG("error, unknown client job state");
> + break;
> + }
> +}
> +
> +void va_kick(VAManager *m)
> +{
> + VAServerJob *sj, *sj_tmp;
> + VAClientJob *cj, *cj_tmp;
> +
> + TRACE("called");
> + TRACE("send_count: %u, execute_count: %u", m->send_count, m->execute_count);
> +
> + /* TODO: make sure there is no starvation of jobs/operations here */
> +
> + /* look for any work to be done among pending server jobs */
> + QTAILQ_FOREACH_SAFE(sj, &m->server_jobs, next, sj_tmp) {
> + TRACE("marker, server tag: %s", sj->tag);
> + va_process_server_job(m, sj);
> + }
> +
> + /* look for work to be done among pending client jobs */
> + QTAILQ_FOREACH_SAFE(cj, &m->client_jobs, next, cj_tmp) {
> + TRACE("marker, client tag: %s", cj->tag);
> + va_process_client_job(m, cj);
> + }
> +}
> diff --git a/virtagent-manager.h b/virtagent-manager.h
> new file mode 100644
> index 0000000..7b463fb
> --- /dev/null
> +++ b/virtagent-manager.h
> @@ -0,0 +1,130 @@
> +#ifndef VIRTAGENT_MANAGER_H
> +#define VIRTAGENT_MANAGER_H
> +
> +#include "qemu-common.h"
> +#include "qemu-queue.h"
> +
> +/*
> + * Protocol Overview:
> + *
> + * The virtagent protocol depends on a state machine to manage communication
> + * over a single connection stream, currently a virtio or isa serial channel.
> + * The basic characterization of the work being done is that clients
> + * send/handle client jobs locally, which are then read/handled remotely as
> + * server jobs. A client job consists of a request which is sent, and a
> + * response which is eventually recieved. A server job consists of a request
> + * which is recieved from the other end, and a response which is sent back.
"i before e, except after c ..." (I misspell receive all the time too).
> + *
> + * Server jobs are given priority over client jobs, i.e. if we send a client
> + * job (our request) and recieve a server job (their request), rather than
> + * await a response to the client job, we immediately begin processing the
> + * server job and then send back the response. This prevents us from being
> + * deadlocked in a situation where both sides have sent a client job and are
> + * awaiting the response before handling the other side's client job.
> + *
> + * Multiple in-flight requests are supported, but high request rates can
> + * potentially starve out the other side's client jobs / requests, so we'll
> + * behaved participants should periodically backoff on high request rates, or
> + * limit themselves to 1 request at a time (anything more than 1 can still
> + * potentionally remove any window for the other end to service it's own
> + * client jobs, since we can begin sending the next request before it begins
> + * send the response for the 2nd).
> + *
> + * On a related note, in the future, bidirectional user/session-level guest
> + * agents may also be supported via a forwarding service made available
> + * through the system-level guest agent. In this case it is up to the
> + * system-level agent to handle forwarding requests in such a way that we
> + * don't starve the host-side service out sheerly by having too many
> + * sessions/users trying to send RPCs at a constant rate. This would be
> + * supported through this job Manager via an additional "forwarder" job type.
> + *
> + * To encapsulate some of this logic, we define here a "Manager" class, which
> + * provides an abstract interface to a state machine which handles most of
> + * the above logic transparently to the transport/application-level code.
> + * This also makes it possible to utilize alternative
> + * transport/application-level protocols in the future.
> + *
> + */
> +
> +/*
> + * Two types of jobs are generated from various components of virtagent.
> + * Each job type has a priority, and a set of prioritized functions as well.
> + *
> + * The read handler generates new server jobs as it recieves requests from
> + * the channel. Server jobs make progress through the following operations.
> + *
> + * EXECUTE->EXECUTE_DONE->SEND->SEND_DONE
> + *
> + * EXECUTE (provided by user, manager calls)
> + * When server jobs are added, eventually (as execution slots become
> + * available) an execute() will be called to begin executing the job. An
> + * error value will be returned if there is no room in the queue for another
> + * server job.
> + *
> + * EXECUTE_DONE (provided by manager, user calls)
> + * As server jobs complete, execute_completed() is called to update execution
> + * status of that job (failure/success), inject the payload, and kick off the
> + * next operation.
> + *
> + * SEND (provided by user, manager calls)
> + * Eventually the send() operation is made. This will cause the send handler
> + * to begin sending the response.
> + *
> + * SEND_DONE (provided by manager, user calls)
> + * Upon completion of that send, the send_completed() operation will be
> + * called. This will free up the job, and kick off the next operation.
> + */
Very helpful protocol overview. Thanks for adding this.
> +typedef int (va_job_op)(void *opaque, const char *tag);
> +typedef struct VAServerJobOps {
> + va_job_op *execute;
> + va_job_op *send;
> + va_job_op *callback;
> +} VAServerJobOps;
> +
> +/*
> + * The client component generates new client jobs as they're made by
> + * virtagent in response to monitored events or user-issued commands.
> + * Client jobs progress via the following operations.
> + *
> + * SEND->SEND_DONE->READ_DONE
> + *
> + * SEND (provided by user, called by manager)
> + * After client jobs are added, send() will eventually be called to queue
> + * the job up for xmit over the channel.
> + *
> + * SEND_DONE (provided by manager, called by user)
> + * Upon completion of the send, send_completed() should be called with
> + * failure/success indication.
> + *
> + * READ_DONE (provided by manager, called by user)
> + * When a response for the request is read back via the transport layer,
> + * read_done() will be called by the user to indicate success/failure,
> + * inject the response, and make the associated callback.
> + */
> +typedef int (va_client_job_cb)(void *opaque, void *resp_opaque,
> + const char *tag);
> +typedef struct VAClientJobOps {
> + va_job_op *send;
> + va_client_job_cb *callback;
> +} VAClientJobOps;
> +
> +typedef struct VAManager VAManager;
> +
> +VAManager *va_manager_new(void);
> +void va_kick(VAManager *m);
> +
> +/* interfaces for server jobs */
> +int va_server_job_add(VAManager *m, const char *tag, void *opaque,
> + VAServerJobOps ops);
> +void va_server_job_execute_done(VAManager *m, const char *tag);
> +void va_server_job_send_done(VAManager *m, const char *tag);
> +void va_server_job_cancel(VAManager *m, const char *tag);
> +
> +/* interfaces for client jobs */
> +int va_client_job_add(VAManager *m, const char *tag, void *opaque,
> + VAClientJobOps ops);
> +void va_client_job_cancel(VAManager *m, const char *tag);
> +void va_client_job_send_done(VAManager *m, const char *tag);
> +void va_client_job_read_done(VAManager *m, const char *tag, void *resp);
> +
> +#endif /* VIRTAGENT_MANAGER_H */
--
Thanks,
Adam
^ permalink raw reply [flat|nested] 41+ messages in thread
* [Qemu-devel] Re: [RFC][PATCH v7 06/16] virtagent: transport definitions
2011-03-07 20:10 ` [Qemu-devel] [RFC][PATCH v7 06/16] virtagent: transport definitions Michael Roth
@ 2011-03-07 21:38 ` Adam Litke
0 siblings, 0 replies; 41+ messages in thread
From: Adam Litke @ 2011-03-07 21:38 UTC (permalink / raw)
To: Michael Roth
Cc: agl, stefanha, Jes.Sorensen, qemu-devel, markus_mueller, aliguori,
abeekhof
On Mon, 2011-03-07 at 14:10 -0600, Michael Roth wrote:
> +#define VA_LINE_LEN_MAX 1024
> +static void va_rpc_parse_hdr(VAHTState *s)
> +{
> + int i, line_pos = 0;
> + bool first_line = true;
> + char line_buf[VA_LINE_LEN_MAX];
> +
> + TRACE("called");
> +
> + for (i = 0; i < VA_HDR_LEN_MAX; ++i) {
> + if (s->hdr[i] == 0) {
> + /* end of header */
> + return;
> + }
> + if (s->hdr[i] != '\n') {
> + /* read line */
> + line_buf[line_pos++] = s->hdr[i];
> + } else {
> + /* process line */
> + if (first_line) {
> + if (strncmp(line_buf, "POST", 4) == 0) {
> + s->http_type = VA_HTTP_TYPE_REQUEST;
> + } else if (strncmp(line_buf, "HTTP", 4) == 0) {
> + s->http_type = VA_HTTP_TYPE_RESPONSE;
> + } else {
> + s->http_type = VA_HTTP_TYPE_UNKNOWN;
> + }
> + first_line = false;
> + }
> + if (strncmp(line_buf, "Content-Length: ", 16) == 0) {
> + s->content_len = atoi(&line_buf[16]);
> + }
> + if (strncmp(line_buf, "X-Virtagent-Client-Tag: ", 24) == 0) {
> + memcpy(s->hdr_client_tag, &line_buf[24], MIN(line_pos-25, 64));
> + //pstrcpy(s->hdr_client_tag, 64, &line_buf[24]);
Remove this commented code.
> + TRACE("\nTAG<%s>\n", s->hdr_client_tag);
> + }
> + line_pos = 0;
> + memset(line_buf, 0, VA_LINE_LEN_MAX);
> + }
> + }
> +}
--
Thanks,
Adam
^ permalink raw reply [flat|nested] 41+ messages in thread
* [Qemu-devel] Re: [RFC][PATCH v7 00/16] virtagent: host/guest communication agent
2011-03-07 20:10 [Qemu-devel] [RFC][PATCH v7 00/16] virtagent: host/guest communication agent Michael Roth
` (15 preceding siblings ...)
2011-03-07 20:10 ` [Qemu-devel] [RFC][PATCH v7 16/16] virtagent: add bits to build virtagent host/guest components Michael Roth
@ 2011-03-07 21:43 ` Anthony Liguori
2011-03-07 22:49 ` Michael Roth
16 siblings, 1 reply; 41+ messages in thread
From: Anthony Liguori @ 2011-03-07 21:43 UTC (permalink / raw)
To: Michael Roth
Cc: agl, stefanha, markus_mueller, qemu-devel, Jes.Sorensen, abeekhof
On 03/07/2011 02:10 PM, Michael Roth wrote:
> These patches apply to master (3-07-2011), and can also be obtained from:
> git://repo.or.cz/qemu/mdroth.git virtagent_v7
>
> CHANGES IN V7:
>
> - Removed dependency on xmlrpc-c for data transport. Now using JSON via QEMU's qjson qobject<->json conversion routines. Binary encoding mechanisms such as Protocol Buffers and ASN.1/BER were considered, but due to limited library support, and limitations of isa/virtio serial transport that would have required an additional layer of encoding to reliably determine RPC boundaries during transport (more here: http://www.mail-archive.com/qemu-devel@nongnu.org/msg56237.html), qobject<->json seemed to be the most prudent route.
Then it needs to be based on QAPI. No point in reinventing the wheel.
It won't be bidirectional though. The guest will only be able to post
events.
Regards,
Anthony Liguori
^ permalink raw reply [flat|nested] 41+ messages in thread
* [Qemu-devel] Re: [RFC][PATCH v7 04/16] virtagent: bi-directional RPC handling logic
2011-03-07 21:24 ` [Qemu-devel] " Adam Litke
@ 2011-03-07 22:35 ` Michael Roth
0 siblings, 0 replies; 41+ messages in thread
From: Michael Roth @ 2011-03-07 22:35 UTC (permalink / raw)
To: Adam Litke
Cc: stefanha, markus_mueller, qemu-devel, abeekhof, aliguori,
Jes.Sorensen
On 03/07/2011 03:24 PM, Adam Litke wrote:
> On Mon, 2011-03-07 at 14:10 -0600, Michael Roth wrote:
>> This implements the state machine/logic used to manage
>> send/receive/execute phases of RPCs we send or receive. It does so using
>> a set of abstract methods we implement with the application and
>> transport level code which will follow.
>>
>> Signed-off-by: Michael Roth<mdroth@linux.vnet.ibm.com>
>> ---
>> virtagent-manager.c | 326 +++++++++++++++++++++++++++++++++++++++++++++++++++
>> virtagent-manager.h | 130 ++++++++++++++++++++
>> 2 files changed, 456 insertions(+), 0 deletions(-)
>> create mode 100644 virtagent-manager.c
>> create mode 100644 virtagent-manager.h
>>
>> diff --git a/virtagent-manager.c b/virtagent-manager.c
>> new file mode 100644
>> index 0000000..51d26a3
>> --- /dev/null
>> +++ b/virtagent-manager.c
>> @@ -0,0 +1,326 @@
>> +/*
>> + * virtagent - job queue management
>> + *
>> + * Copyright IBM Corp. 2011
>> + *
>> + * Authors:
>> + * Michael Roth<mdroth@linux.vnet.ibm.com>
>> + *
>> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
>> + * See the COPYING file in the top-level directory.
>> + *
>> + */
>> +
>> +#include "virtagent-common.h"
>> +
>> +typedef struct VAServerJob {
>> + char tag[64];
>> + void *opaque;
>> + VAServerJobOps ops;
>> + QTAILQ_ENTRY(VAServerJob) next;
>> + enum {
>> + VA_SERVER_JOB_STATE_NEW = 0,
>> + VA_SERVER_JOB_STATE_BUSY,
>> + VA_SERVER_JOB_STATE_EXECUTED,
>> + VA_SERVER_JOB_STATE_SENT,
>> + VA_SERVER_JOB_STATE_DONE,
>> + } state;
>> +} VAServerJob;
>> +
>> +typedef struct VAClientJob {
>> + char tag[64];
>> + void *opaque;
>> + void *resp_opaque;
>> + VAClientJobOps ops;
>> + QTAILQ_ENTRY(VAClientJob) next;
>> + enum {
>> + VA_CLIENT_JOB_STATE_NEW = 0,
>> + VA_CLIENT_JOB_STATE_BUSY,
>> + VA_CLIENT_JOB_STATE_SENT,
>> + VA_CLIENT_JOB_STATE_READ,
>> + VA_CLIENT_JOB_STATE_DONE,
>> + } state;
>> +} VAClientJob;
>> +
>> +#define SEND_COUNT_MAX 1
>> +#define EXECUTE_COUNT_MAX 4
>
> It's not immediately clear what the difference between SEND_COUNT_MAX
> and EXECUTE_COUNT_MAX is. Some comments would help. Also, will the
> code work if these numbers are changed? If not, a note about what
> someone needs to look at when changing these would seem appropriate.
>
Basically the SEND_COUNT_MAX is the number of RPCs the client can have
in flight at a time. EXECUTE_COUNT_MAX is the number of jobs the server
can execute concurrently/asynchronously (execute as in actually do the
"execute corresponding RPC" phase of a server job's lifecycle).
These should be tweakable without much side-effect. These aren't
currently that important since a monitor tends to limit us to 1 RPC at a
time, and the guest agent doesn't make any substantial use of
guest->host RPCs atm, so SEND_COUNT_MAX has little impact.
We don't currently execute RPCs concurrently/asynchronously either, so
EXECUTE_COUNT_MAX doesn't do much. But when threaded RPC execution is
re-implemented this will come back into play. I'll make sure to add some
comments on this.
>> +
>> +struct VAManager {
>> + int send_count; /* sends in flight */
>> + int execute_count; /* number of jobs currently executing */
>> + QTAILQ_HEAD(, VAServerJob) server_jobs;
>> + QTAILQ_HEAD(, VAClientJob) client_jobs;
>> +};
>> +
>> +/* server job operations/helpers */
>> +
>> +static VAServerJob *va_server_job_by_tag(VAManager *m, const char *tag)
>> +{
>> + VAServerJob *j;
>> + QTAILQ_FOREACH(j,&m->server_jobs, next) {
>> + if (strcmp(j->tag, tag) == 0) {
>> + return j;
>> + }
>> + }
>> + return NULL;
>> +}
>> +
>> +int va_server_job_add(VAManager *m, const char *tag, void *opaque,
>> + VAServerJobOps ops)
>> +{
>> + VAServerJob *j = qemu_mallocz(sizeof(VAServerJob));
>> + TRACE("called");
>
> Qemu has a good tracing infrastructure. If this is trace point is
> useful enough to keep around, it should try to use that. If it's not
> that important, I'd remove it entirely. I believe this has been flagged
> in an earlier RFC too.
These are really just to aid in development. I plan on NOOPing these via
the DEBUG_VA flag before merge. Can also remove them if it's too nasty.
Only a very small subset of these would be useful for the trace
facility, I'll have a better idea of which ones once I stop relying on
the TRACE() stuff.
>
>> + j->state = VA_SERVER_JOB_STATE_NEW;
>> + j->ops = ops;
>> + j->opaque = opaque;
>> + memset(j->tag, 0, 64);
>> + pstrcpy(j->tag, 63, tag);
>
> Magic numbers. Should use something like #define TAG_LEN 64
>
>> + QTAILQ_INSERT_TAIL(&m->server_jobs, j, next);
>> + va_kick(m);
>> + return 0;
>> +}
>> +
>> +static void va_server_job_execute(VAServerJob *j)
>> +{
>> + TRACE("called");
>> + j->state = VA_SERVER_JOB_STATE_BUSY;
>> + j->ops.execute(j->opaque, j->tag);
>> +}
>> +
>> +/* TODO: need a way to pass information back */
>> +void va_server_job_execute_done(VAManager *m, const char *tag)
>> +{
>> + VAServerJob *j = va_server_job_by_tag(m, tag);
>> + TRACE("called");
>> + if (!j) {
>> + LOG("server job with tag \"%s\" not found", tag);
>> + return;
>> + }
>> + j->state = VA_SERVER_JOB_STATE_EXECUTED;
>> + va_kick(m);
>> +}
>> +
>> +static void va_server_job_send(VAServerJob *j)
>> +{
>> + TRACE("called");
>> + j->state = VA_SERVER_JOB_STATE_BUSY;
>> + j->ops.send(j->opaque, j->tag);
>> +}
>> +
>> +void va_server_job_send_done(VAManager *m, const char *tag)
>> +{
>> + VAServerJob *j = va_server_job_by_tag(m, tag);
>> + TRACE("called");
>> + if (!j) {
>> + LOG("server job with tag \"%s\" not found", tag);
>> + return;
>> + }
>> + j->state = VA_SERVER_JOB_STATE_SENT;
>> + m->send_count--;
>> + va_kick(m);
>> +}
>> +
>> +static void va_server_job_callback(VAServerJob *j)
>> +{
>> + TRACE("called");
>> + j->state = VA_SERVER_JOB_STATE_BUSY;
>> + if (j->ops.callback) {
>> + j->ops.callback(j->opaque, j->tag);
>> + }
>> + j->state = VA_SERVER_JOB_STATE_DONE;
>> +}
>> +
>> +void va_server_job_cancel(VAManager *m, const char *tag)
>> +{
>> + VAServerJob *j = va_server_job_by_tag(m, tag);
>> + TRACE("called");
>> + if (!j) {
>> + LOG("server job with tag \"%s\" not found", tag);
>> + return;
>> + }
>> + /* TODO: need to decrement sends/execs in flight appropriately */
>> + /* make callback and move to done state, kick() will handle cleanup */
>> + va_server_job_callback(j);
>> + va_kick(m);
>> +}
>> +
>> +/* client job operations */
>> +
>> +static VAClientJob *va_client_job_by_tag(VAManager *m, const char *tag)
>> +{
>> + VAClientJob *j;
>> + QTAILQ_FOREACH(j,&m->client_jobs, next) {
>> + if (strcmp(j->tag, tag) == 0) {
>> + return j;
>> + }
>> + }
>> + return NULL;
>> +}
>> +
>> +int va_client_job_add(VAManager *m, const char *tag, void *opaque,
>> + VAClientJobOps ops)
>> +{
>> + VAClientJob *j = qemu_mallocz(sizeof(VAClientJob));
>> + TRACE("called");
>> + j->ops = ops;
>> + j->opaque = opaque;
>> + memset(j->tag, 0, 64);
>> + pstrcpy(j->tag, 63, tag);
>> + QTAILQ_INSERT_TAIL(&m->client_jobs, j, next);
>> + va_kick(m);
>> + return 0;
>> +}
>> +
>> +static void va_client_job_send(VAClientJob *j)
>> +{
>> + TRACE("called");
>> + j->state = VA_CLIENT_JOB_STATE_BUSY;
>> + j->ops.send(j->opaque, j->tag);
>> +}
>> +
>> +void va_client_job_send_done(VAManager *m, const char *tag)
>> +{
>> + VAClientJob *j = va_client_job_by_tag(m, tag);
>> + TRACE("called");
>> + if (!j) {
>> + LOG("client job with tag \"%s\" not found", tag);
>> + return;
>> + }
>> + j->state = VA_CLIENT_JOB_STATE_SENT;
>> + m->send_count--;
>> + va_kick(m);
>> +}
>> +
>> +void va_client_job_read_done(VAManager *m, const char *tag, void *resp)
>> +{
>> + VAClientJob *j = va_client_job_by_tag(m, tag);
>> + TRACE("called");
>> + if (!j) {
>> + LOG("client job with tag \"%s\" not found", tag);
>> + return;
>> + }
>> + j->state = VA_CLIENT_JOB_STATE_READ;
>> + j->resp_opaque = resp;
>> + va_kick(m);
>> +}
>> +
>> +static void va_client_job_callback(VAClientJob *j)
>> +{
>> + TRACE("called");
>> + j->state = VA_CLIENT_JOB_STATE_BUSY;
>> + if (j->ops.callback) {
>> + j->ops.callback(j->opaque, j->resp_opaque, j->tag);
>> + }
>> + j->state = VA_CLIENT_JOB_STATE_DONE;
>> +}
>> +
>> +void va_client_job_cancel(VAManager *m, const char *tag)
>> +{
>> + VAClientJob *j = va_client_job_by_tag(m, tag);
>> + TRACE("called");
>> + if (!j) {
>> + LOG("client job with tag \"%s\" not found", tag);
>> + return;
>> + }
>> + /* TODO: need to decrement sends/execs in flight appropriately */
>> + /* make callback and move to done state, kick() will handle cleanup */
>> + va_client_job_callback(j);
>> + va_kick(m);
>> +}
>> +
>> +/* general management functions */
>> +
>> +VAManager *va_manager_new(void)
>> +{
>> + VAManager *m = qemu_mallocz(sizeof(VAManager));
>> + QTAILQ_INIT(&m->client_jobs);
>> + QTAILQ_INIT(&m->server_jobs);
>> + return m;
>> +}
>> +
>> +static void va_process_server_job(VAManager *m, VAServerJob *sj)
>> +{
>> + switch (sj->state) {
>> + case VA_SERVER_JOB_STATE_NEW:
>> + TRACE("marker");
>> + va_server_job_execute(sj);
>> + break;
>> + case VA_SERVER_JOB_STATE_EXECUTED:
>> + TRACE("marker");
>> + if (m->send_count< SEND_COUNT_MAX) {
>> + TRACE("marker");
>> + va_server_job_send(sj);
>> + m->send_count++;
>> + }
>> + break;
>> + case VA_SERVER_JOB_STATE_SENT:
>> + TRACE("marker");
>> + va_server_job_callback(sj);
>> + break;
>> + case VA_SERVER_JOB_STATE_BUSY:
>> + TRACE("marker, server job currently busy");
>> + break;
>> + case VA_SERVER_JOB_STATE_DONE:
>> + TRACE("marker");
>> + QTAILQ_REMOVE(&m->server_jobs, sj, next);
>> + break;
>> + default:
>> + LOG("error, unknown server job state");
>> + break;
>> + }
>> +}
>> +
>> +static void va_process_client_job(VAManager *m, VAClientJob *cj)
>> +{
>> + switch (cj->state) {
>> + case VA_CLIENT_JOB_STATE_NEW:
>> + TRACE("marker");
>> + if (m->send_count< SEND_COUNT_MAX) {
>> + TRACE("marker");
>> + va_client_job_send(cj);
>> + m->send_count++;
>> + }
>> + break;
>> + case VA_CLIENT_JOB_STATE_SENT:
>> + TRACE("marker");
>> + //nothing to do here, awaiting read_done()
>> + break;
>> + case VA_CLIENT_JOB_STATE_READ:
>> + TRACE("marker");
>> + va_client_job_callback(cj);
>> + break;
>> + case VA_CLIENT_JOB_STATE_DONE:
>> + TRACE("marker");
>> + QTAILQ_REMOVE(&m->client_jobs, cj, next);
>> + break;
>> + case VA_CLIENT_JOB_STATE_BUSY:
>> + TRACE("marker, client job currently busy");
>> + break;
>> + default:
>> + LOG("error, unknown client job state");
>> + break;
>> + }
>> +}
>> +
>> +void va_kick(VAManager *m)
>> +{
>> + VAServerJob *sj, *sj_tmp;
>> + VAClientJob *cj, *cj_tmp;
>> +
>> + TRACE("called");
>> + TRACE("send_count: %u, execute_count: %u", m->send_count, m->execute_count);
>> +
>> + /* TODO: make sure there is no starvation of jobs/operations here */
>> +
>> + /* look for any work to be done among pending server jobs */
>> + QTAILQ_FOREACH_SAFE(sj,&m->server_jobs, next, sj_tmp) {
>> + TRACE("marker, server tag: %s", sj->tag);
>> + va_process_server_job(m, sj);
>> + }
>> +
>> + /* look for work to be done among pending client jobs */
>> + QTAILQ_FOREACH_SAFE(cj,&m->client_jobs, next, cj_tmp) {
>> + TRACE("marker, client tag: %s", cj->tag);
>> + va_process_client_job(m, cj);
>> + }
>> +}
>> diff --git a/virtagent-manager.h b/virtagent-manager.h
>> new file mode 100644
>> index 0000000..7b463fb
>> --- /dev/null
>> +++ b/virtagent-manager.h
>> @@ -0,0 +1,130 @@
>> +#ifndef VIRTAGENT_MANAGER_H
>> +#define VIRTAGENT_MANAGER_H
>> +
>> +#include "qemu-common.h"
>> +#include "qemu-queue.h"
>> +
>> +/*
>> + * Protocol Overview:
>> + *
>> + * The virtagent protocol depends on a state machine to manage communication
>> + * over a single connection stream, currently a virtio or isa serial channel.
>> + * The basic characterization of the work being done is that clients
>> + * send/handle client jobs locally, which are then read/handled remotely as
>> + * server jobs. A client job consists of a request which is sent, and a
>> + * response which is eventually recieved. A server job consists of a request
>> + * which is recieved from the other end, and a response which is sent back.
>
> "i before e, except after c ..." (I misspell receive all the time too).
>
TIL about vim's spell check feature :)
>> + *
>> + * Server jobs are given priority over client jobs, i.e. if we send a client
>> + * job (our request) and recieve a server job (their request), rather than
>> + * await a response to the client job, we immediately begin processing the
>> + * server job and then send back the response. This prevents us from being
>> + * deadlocked in a situation where both sides have sent a client job and are
>> + * awaiting the response before handling the other side's client job.
>> + *
>> + * Multiple in-flight requests are supported, but high request rates can
>> + * potentially starve out the other side's client jobs / requests, so we'll
>> + * behaved participants should periodically backoff on high request rates, or
>> + * limit themselves to 1 request at a time (anything more than 1 can still
>> + * potentionally remove any window for the other end to service it's own
>> + * client jobs, since we can begin sending the next request before it begins
>> + * send the response for the 2nd).
>> + *
>> + * On a related note, in the future, bidirectional user/session-level guest
>> + * agents may also be supported via a forwarding service made available
>> + * through the system-level guest agent. In this case it is up to the
>> + * system-level agent to handle forwarding requests in such a way that we
>> + * don't starve the host-side service out sheerly by having too many
>> + * sessions/users trying to send RPCs at a constant rate. This would be
>> + * supported through this job Manager via an additional "forwarder" job type.
>> + *
>> + * To encapsulate some of this logic, we define here a "Manager" class, which
>> + * provides an abstract interface to a state machine which handles most of
>> + * the above logic transparently to the transport/application-level code.
>> + * This also makes it possible to utilize alternative
>> + * transport/application-level protocols in the future.
>> + *
>> + */
>> +
>> +/*
>> + * Two types of jobs are generated from various components of virtagent.
>> + * Each job type has a priority, and a set of prioritized functions as well.
>> + *
>> + * The read handler generates new server jobs as it recieves requests from
>> + * the channel. Server jobs make progress through the following operations.
>> + *
>> + * EXECUTE->EXECUTE_DONE->SEND->SEND_DONE
>> + *
>> + * EXECUTE (provided by user, manager calls)
>> + * When server jobs are added, eventually (as execution slots become
>> + * available) an execute() will be called to begin executing the job. An
>> + * error value will be returned if there is no room in the queue for another
>> + * server job.
>> + *
>> + * EXECUTE_DONE (provided by manager, user calls)
>> + * As server jobs complete, execute_completed() is called to update execution
>> + * status of that job (failure/success), inject the payload, and kick off the
>> + * next operation.
>> + *
>> + * SEND (provided by user, manager calls)
>> + * Eventually the send() operation is made. This will cause the send handler
>> + * to begin sending the response.
>> + *
>> + * SEND_DONE (provided by manager, user calls)
>> + * Upon completion of that send, the send_completed() operation will be
>> + * called. This will free up the job, and kick off the next operation.
>> + */
>
> Very helpful protocol overview. Thanks for adding this.
>
>> +typedef int (va_job_op)(void *opaque, const char *tag);
>> +typedef struct VAServerJobOps {
>> + va_job_op *execute;
>> + va_job_op *send;
>> + va_job_op *callback;
>> +} VAServerJobOps;
>> +
>> +/*
>> + * The client component generates new client jobs as they're made by
>> + * virtagent in response to monitored events or user-issued commands.
>> + * Client jobs progress via the following operations.
>> + *
>> + * SEND->SEND_DONE->READ_DONE
>> + *
>> + * SEND (provided by user, called by manager)
>> + * After client jobs are added, send() will eventually be called to queue
>> + * the job up for xmit over the channel.
>> + *
>> + * SEND_DONE (provided by manager, called by user)
>> + * Upon completion of the send, send_completed() should be called with
>> + * failure/success indication.
>> + *
>> + * READ_DONE (provided by manager, called by user)
>> + * When a response for the request is read back via the transport layer,
>> + * read_done() will be called by the user to indicate success/failure,
>> + * inject the response, and make the associated callback.
>> + */
>> +typedef int (va_client_job_cb)(void *opaque, void *resp_opaque,
>> + const char *tag);
>> +typedef struct VAClientJobOps {
>> + va_job_op *send;
>> + va_client_job_cb *callback;
>> +} VAClientJobOps;
>> +
>> +typedef struct VAManager VAManager;
>> +
>> +VAManager *va_manager_new(void);
>> +void va_kick(VAManager *m);
>> +
>> +/* interfaces for server jobs */
>> +int va_server_job_add(VAManager *m, const char *tag, void *opaque,
>> + VAServerJobOps ops);
>> +void va_server_job_execute_done(VAManager *m, const char *tag);
>> +void va_server_job_send_done(VAManager *m, const char *tag);
>> +void va_server_job_cancel(VAManager *m, const char *tag);
>> +
>> +/* interfaces for client jobs */
>> +int va_client_job_add(VAManager *m, const char *tag, void *opaque,
>> + VAClientJobOps ops);
>> +void va_client_job_cancel(VAManager *m, const char *tag);
>> +void va_client_job_send_done(VAManager *m, const char *tag);
>> +void va_client_job_read_done(VAManager *m, const char *tag, void *resp);
>> +
>> +#endif /* VIRTAGENT_MANAGER_H */
>
^ permalink raw reply [flat|nested] 41+ messages in thread
* [Qemu-devel] Re: [RFC][PATCH v7 00/16] virtagent: host/guest communication agent
2011-03-07 21:43 ` [Qemu-devel] Re: [RFC][PATCH v7 00/16] virtagent: host/guest communication agent Anthony Liguori
@ 2011-03-07 22:49 ` Michael Roth
2011-03-07 22:56 ` Anthony Liguori
0 siblings, 1 reply; 41+ messages in thread
From: Michael Roth @ 2011-03-07 22:49 UTC (permalink / raw)
To: Anthony Liguori
Cc: agl, stefanha, markus_mueller, qemu-devel, Jes.Sorensen, abeekhof
On 03/07/2011 03:43 PM, Anthony Liguori wrote:
> On 03/07/2011 02:10 PM, Michael Roth wrote:
>> These patches apply to master (3-07-2011), and can also be obtained from:
>> git://repo.or.cz/qemu/mdroth.git virtagent_v7
>>
>> CHANGES IN V7:
>>
>> - Removed dependency on xmlrpc-c for data transport. Now using JSON
>> via QEMU's qjson qobject<->json conversion routines. Binary encoding
>> mechanisms such as Protocol Buffers and ASN.1/BER were considered, but
>> due to limited library support, and limitations of isa/virtio serial
>> transport that would have required an additional layer of encoding to
>> reliably determine RPC boundaries during transport (more here:
>> http://www.mail-archive.com/qemu-devel@nongnu.org/msg56237.html),
>> qobject<->json seemed to be the most prudent route.
>
> Then it needs to be based on QAPI. No point in reinventing the wheel. It
> won't be bidirectional though. The guest will only be able to post events.
It's not really inventing anything. We've always started off with
qobject params, which we then pulled apart and stuck into xmlrpc params,
which when then turned into xml for transport. Now we just take the
qobjects and covert them to json directly. We've only cut out an
intermediate library and switched to a different UTF8-based encoding for
transport.
With QAPI we'd have the extra step of pulling function parameters into a
qobjects. Not any different from what the situation would've been using
xmlrpc or any of the other binary encoding that were considered.
It does look more similar to what QMP/QAPI is doing than previously, but
it doesn't need to be all or nothing.
>
> Regards,
>
> Anthony Liguori
>
^ permalink raw reply [flat|nested] 41+ messages in thread
* [Qemu-devel] Re: [RFC][PATCH v7 00/16] virtagent: host/guest communication agent
2011-03-07 22:49 ` Michael Roth
@ 2011-03-07 22:56 ` Anthony Liguori
2011-03-08 0:11 ` Michael Roth
0 siblings, 1 reply; 41+ messages in thread
From: Anthony Liguori @ 2011-03-07 22:56 UTC (permalink / raw)
To: Michael Roth
Cc: agl, stefanha, markus_mueller, qemu-devel, Jes.Sorensen, abeekhof
On 03/07/2011 04:49 PM, Michael Roth wrote:
> It's not really inventing anything. We've always started off with
> qobject params, which we then pulled apart and stuck into xmlrpc
> params, which when then turned into xml for transport. Now we just
> take the qobjects and covert them to json directly. We've only cut out
> an intermediate library and switched to a different UTF8-based
> encoding for transport.
>
> With QAPI we'd have the extra step of pulling function parameters into
> a qobjects. Not any different from what the situation would've been
> using xmlrpc or any of the other binary encoding that were considered.
>
> It does look more similar to what QMP/QAPI is doing than previously,
> but it doesn't need to be all or nothing.
I think I have a Clever Idea here but hacking together a prototype.
Basic thinking is to make guest commands part of the QMP namespace such
that a guest command looks like any other QMP command. The only role
QEMU plays in this model is validating the commands inputs and outputs
and then passing the command to the guest agent.
Stay tuned.
Regards,
Anthony Liguori
>
>>
>> Regards,
>>
>> Anthony Liguori
>>
>
^ permalink raw reply [flat|nested] 41+ messages in thread
* [Qemu-devel] Re: [RFC][PATCH v7 00/16] virtagent: host/guest communication agent
2011-03-07 22:56 ` Anthony Liguori
@ 2011-03-08 0:11 ` Michael Roth
2011-03-08 0:24 ` Anthony Liguori
0 siblings, 1 reply; 41+ messages in thread
From: Michael Roth @ 2011-03-08 0:11 UTC (permalink / raw)
To: Anthony Liguori
Cc: agl, stefanha, markus_mueller, qemu-devel, Jes.Sorensen, abeekhof
On 03/07/2011 04:56 PM, Anthony Liguori wrote:
> On 03/07/2011 04:49 PM, Michael Roth wrote:
>> It's not really inventing anything. We've always started off with
>> qobject params, which we then pulled apart and stuck into xmlrpc
>> params, which when then turned into xml for transport. Now we just
>> take the qobjects and covert them to json directly. We've only cut out
>> an intermediate library and switched to a different UTF8-based
>> encoding for transport.
>>
>> With QAPI we'd have the extra step of pulling function parameters into
>> a qobjects. Not any different from what the situation would've been
>> using xmlrpc or any of the other binary encoding that were considered.
>>
>> It does look more similar to what QMP/QAPI is doing than previously,
>> but it doesn't need to be all or nothing.
>
> I think I have a Clever Idea here but hacking together a prototype.
>
> Basic thinking is to make guest commands part of the QMP namespace such
> that a guest command looks like any other QMP command. The only role
> QEMU plays in this model is validating the commands inputs and outputs
> and then passing the command to the guest agent.
Hmm...this does sound nice. But keep in mind that not all parameters
passed in via QMP were intended solely for the guest. That just happens
to be the case for the RPCs we have implemented in this RFC.
For instance, one of QMP commands we intended to add for the new getfile
implementation, to address concerns over a hardcoded file size limit
while avoiding large memory allocations on the host or guest, was
chunked file transfer using a set of stateful RPCs, with a higher level
QMP command to wrap them:
qmp.getfile <local file> <remote file>
Where <local file> could be a normal file, or "-" for direct output to
monitor. Internally, the set of actual RPCs being executed are much
lower level. Something like:
offset = 0
local_fd = open(<local file>)
remote_fd = va.open(<remote file>)
while (((read_count, buf) = va.read(remote_fd, offset, 512*1024)) > 0):
write(local_fd, buf, read_count)
offset += read_count
va.close(remote_fd)
Any higher-level commands of this type would be still doable, but would
need to be pushed all the way up to the management stack, or done
programatically, so it does limit what can be done within an interactive
shell. Some might argue that's for the best, but there may be more of a
trade-off in other possible use cases.
>
> Stay tuned.
>
> Regards,
>
> Anthony Liguori
>
>>
>>>
>>> Regards,
>>>
>>> Anthony Liguori
>>>
>>
>
^ permalink raw reply [flat|nested] 41+ messages in thread
* [Qemu-devel] Re: [RFC][PATCH v7 00/16] virtagent: host/guest communication agent
2011-03-08 0:11 ` Michael Roth
@ 2011-03-08 0:24 ` Anthony Liguori
0 siblings, 0 replies; 41+ messages in thread
From: Anthony Liguori @ 2011-03-08 0:24 UTC (permalink / raw)
To: Michael Roth
Cc: agl, stefanha, markus_mueller, qemu-devel, Jes.Sorensen, abeekhof
On 03/07/2011 06:11 PM, Michael Roth wrote:
> On 03/07/2011 04:56 PM, Anthony Liguori wrote:
>> On 03/07/2011 04:49 PM, Michael Roth wrote:
>>> It's not really inventing anything. We've always started off with
>>> qobject params, which we then pulled apart and stuck into xmlrpc
>>> params, which when then turned into xml for transport. Now we just
>>> take the qobjects and covert them to json directly. We've only cut out
>>> an intermediate library and switched to a different UTF8-based
>>> encoding for transport.
>>>
>>> With QAPI we'd have the extra step of pulling function parameters into
>>> a qobjects. Not any different from what the situation would've been
>>> using xmlrpc or any of the other binary encoding that were considered.
>>>
>>> It does look more similar to what QMP/QAPI is doing than previously,
>>> but it doesn't need to be all or nothing.
>>
>> I think I have a Clever Idea here but hacking together a prototype.
>>
>> Basic thinking is to make guest commands part of the QMP namespace such
>> that a guest command looks like any other QMP command. The only role
>> QEMU plays in this model is validating the commands inputs and outputs
>> and then passing the command to the guest agent.
>
> Hmm...this does sound nice. But keep in mind that not all parameters
> passed in via QMP were intended solely for the guest. That just
> happens to be the case for the RPCs we have implemented in this RFC.
>
> For instance, one of QMP commands we intended to add for the new
> getfile implementation, to address concerns over a hardcoded file size
> limit while avoiding large memory allocations on the host or guest,
> was chunked file transfer using a set of stateful RPCs, with a higher
> level QMP command to wrap them:
Just as HMP commands can be implemented in terms of QMP commands, QMP
commands can be implemented in terms of QMP commands.
Regards,
Anthony Liguori
>
> qmp.getfile <local file> <remote file>
>
> Where <local file> could be a normal file, or "-" for direct output to
> monitor. Internally, the set of actual RPCs being executed are much
> lower level. Something like:
>
> offset = 0
> local_fd = open(<local file>)
> remote_fd = va.open(<remote file>)
> while (((read_count, buf) = va.read(remote_fd, offset, 512*1024)) > 0):
> write(local_fd, buf, read_count)
> offset += read_count
> va.close(remote_fd)
>
> Any higher-level commands of this type would be still doable, but
> would need to be pushed all the way up to the management stack, or
> done programatically, so it does limit what can be done within an
> interactive shell. Some might argue that's for the best, but there may
> be more of a trade-off in other possible use cases.
>
>>
>> Stay tuned.
>>
>> Regards,
>>
>> Anthony Liguori
>>
>>>
>>>>
>>>> Regards,
>>>>
>>>> Anthony Liguori
>>>>
>>>
>>
>
^ permalink raw reply [flat|nested] 41+ messages in thread
* [Qemu-devel] Re: [RFC][PATCH v7 03/16] Make qemu timers available for tools
2011-03-07 20:10 ` [Qemu-devel] [RFC][PATCH v7 03/16] Make qemu timers available for tools Michael Roth
@ 2011-03-09 10:33 ` Jes Sorensen
2011-03-09 13:04 ` Michael Roth
0 siblings, 1 reply; 41+ messages in thread
From: Jes Sorensen @ 2011-03-09 10:33 UTC (permalink / raw)
To: Michael Roth
Cc: agl, stefanha, markus_mueller, qemu-devel, aliguori, abeekhof
On 03/07/11 21:10, Michael Roth wrote:
> To be able to use qemu_mod_timer() and friends to register timeout
> events for virtagent's qemu-va tool, we need to do the following:
>
> Move several blocks of code out of cpus.c that handle initialization
> of qemu's io_thread_fd and working with it via
> qemu_notify_event()/qemu_event_read()/etc, and make them accessible
> as backend functions to both the emulator code and qemu-tool.c via
> wrapper functions within cpus.c and qemu-tool.c, respectively. These
> have been added to qemu-ioh.c, where similar treatment was given to
> qemu_set_fd_handler() and friends.
>
> Some of these wrapper functions lack declarations when being
> built into tools, so we add those via qemu-tool.h, which can be included
> by a tool to access them. With these changes we can drive timers in a
> tool linking it against qemu-timer.o and then implementing something
> similar to the main i/o loop in vl.c:
>
[snip]
> diff --git a/qemu-ioh.c b/qemu-ioh.c
> index cc71470..5c3f94c 100644
> --- a/qemu-ioh.c
> +++ b/qemu-ioh.c
> @@ -113,3 +117,94 @@ void qemu_process_fd_handlers2(void *ioh_record_list, const fd_set *rfds,
> }
> }
> }
> +
> +#ifndef _WIN32
> +void iothread_event_increment(int *io_thread_fd)
Please move these functions into posix/w32 specific files so we don't
get anymore ugly #ifdefs. It would be good if we could use a wrapper
struct as well to hide the different data types so we don't need #ifdefs
in the calling code as well.
Cheers,
Jes
^ permalink raw reply [flat|nested] 41+ messages in thread
* [Qemu-devel] Re: [RFC][PATCH v7 05/16] virtagent: common helpers and init routines
2011-03-07 20:10 ` [Qemu-devel] [RFC][PATCH v7 05/16] virtagent: common helpers and init routines Michael Roth
@ 2011-03-09 10:38 ` Jes Sorensen
2011-03-09 13:17 ` Michael Roth
0 siblings, 1 reply; 41+ messages in thread
From: Jes Sorensen @ 2011-03-09 10:38 UTC (permalink / raw)
To: Michael Roth
Cc: agl, stefanha, markus_mueller, qemu-devel, aliguori, abeekhof
On 03/07/11 21:10, Michael Roth wrote:
> +#define VA_PIDFILE "/var/run/qemu-va.pid"
> +#define VA_HDR_LEN_MAX 4096 /* http header limit */
> +#define VA_CONTENT_LEN_MAX 2*1024*1024 /* rpc/http send limit */
> +#define VA_CLIENT_JOBS_MAX 5 /* max client rpcs we can queue */
> +#define VA_SERVER_JOBS_MAX 5 /* max server rpcs we can queue */
> +#define VA_SERVER_TIMEOUT_MS 5 * 1000
> +#define VA_CLIENT_TIMEOUT_MS 5 * 1000
> +#define VA_SENTINEL 0xFF
> +#define VA_BAUDRATE B38400 /* for isa-serial channels */
> +
I've been after these before - please put the ones that make sense to
tune into a config file, and the same with the pidfile.
Cheers,
Jes
^ permalink raw reply [flat|nested] 41+ messages in thread
* [Qemu-devel] Re: [RFC][PATCH v7 15/16] virtagent: qemu-va, system-level virtagent guest agent
2011-03-07 20:10 ` [Qemu-devel] [RFC][PATCH v7 15/16] virtagent: qemu-va, system-level virtagent guest agent Michael Roth
@ 2011-03-09 10:48 ` Jes Sorensen
0 siblings, 0 replies; 41+ messages in thread
From: Jes Sorensen @ 2011-03-09 10:48 UTC (permalink / raw)
To: Michael Roth
Cc: agl, stefanha, markus_mueller, qemu-devel, aliguori, abeekhof
On 03/07/11 21:10, Michael Roth wrote:
> Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
> ---
> qemu-va.c | 247 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> 1 files changed, 247 insertions(+), 0 deletions(-)
> create mode 100644 qemu-va.c
>
> diff --git a/qemu-va.c b/qemu-va.c
> new file mode 100644
> index 0000000..a9ff56f
> --- /dev/null
> +++ b/qemu-va.c
> @@ -0,0 +1,247 @@
[snip]
> +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(VA_PIDFILE, O_CREAT|O_RDWR, S_IRUSR|S_IWUSR);
> + if (!pidfd || lockf(pidfd, F_TLOCK, 0))
> + errx(EXIT_FAILURE, "Cannot lock pid file");
> +
> + if (ftruncate(pidfd, 0) || lseek(pidfd, 0, SEEK_SET))
> + errx(EXIT_FAILURE, "Cannot truncate pid file");
> + if (asprintf(&pidstr, "%d", getpid()) == -1)
> + errx(EXIT_FAILURE, "Cannot allocate memory");
> + if (write(pidfd, pidstr, strlen(pidstr)) != strlen(pidstr))
> + errx(EXIT_FAILURE, "Failed to write pid file");
> + free(pidstr);
Coding style - this needs to be fixed.
> + umask(0);
> + sid = setsid();
> + if (sid < 0)
> + goto fail;
> + if ((chdir("/")) < 0)
> + goto fail;
and again
Cheers,
Jes
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [Qemu-devel] Re: [RFC][PATCH v7 03/16] Make qemu timers available for tools
2011-03-09 10:33 ` [Qemu-devel] " Jes Sorensen
@ 2011-03-09 13:04 ` Michael Roth
2011-03-09 13:06 ` Jes Sorensen
0 siblings, 1 reply; 41+ messages in thread
From: Michael Roth @ 2011-03-09 13:04 UTC (permalink / raw)
To: Jes Sorensen
Cc: agl, stefanha, abeekhof, qemu-devel, aliguori, markus_mueller
On 03/09/2011 04:33 AM, Jes Sorensen wrote:
> On 03/07/11 21:10, Michael Roth wrote:
>> To be able to use qemu_mod_timer() and friends to register timeout
>> events for virtagent's qemu-va tool, we need to do the following:
>>
>> Move several blocks of code out of cpus.c that handle initialization
>> of qemu's io_thread_fd and working with it via
>> qemu_notify_event()/qemu_event_read()/etc, and make them accessible
>> as backend functions to both the emulator code and qemu-tool.c via
>> wrapper functions within cpus.c and qemu-tool.c, respectively. These
>> have been added to qemu-ioh.c, where similar treatment was given to
>> qemu_set_fd_handler() and friends.
>>
>> Some of these wrapper functions lack declarations when being
>> built into tools, so we add those via qemu-tool.h, which can be included
>> by a tool to access them. With these changes we can drive timers in a
>> tool linking it against qemu-timer.o and then implementing something
>> similar to the main i/o loop in vl.c:
>>
>
> [snip]
>
>> diff --git a/qemu-ioh.c b/qemu-ioh.c
>> index cc71470..5c3f94c 100644
>> --- a/qemu-ioh.c
>> +++ b/qemu-ioh.c
>> @@ -113,3 +117,94 @@ void qemu_process_fd_handlers2(void *ioh_record_list, const fd_set *rfds,
>> }
>> }
>> }
>> +
>> +#ifndef _WIN32
>> +void iothread_event_increment(int *io_thread_fd)
>
> Please move these functions into posix/w32 specific files so we don't
> get anymore ugly #ifdefs. It would be good if we could use a wrapper
> struct as well to hide the different data types so we don't need #ifdefs
> in the calling code as well.
Yup, meant to add this to the TODO. I may end up sending these general
tools changes out in a separate patchset since they seem to be in
conflict with quite of few patches floating around the list. Either way
I'll make sure to get these cleaned up and tested a bit a more.
>
> Cheers,
> Jes
>
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [Qemu-devel] Re: [RFC][PATCH v7 03/16] Make qemu timers available for tools
2011-03-09 13:04 ` Michael Roth
@ 2011-03-09 13:06 ` Jes Sorensen
0 siblings, 0 replies; 41+ messages in thread
From: Jes Sorensen @ 2011-03-09 13:06 UTC (permalink / raw)
To: Michael Roth
Cc: agl, stefanha, abeekhof, qemu-devel, aliguori, markus_mueller
On 03/09/11 14:04, Michael Roth wrote:
> On 03/09/2011 04:33 AM, Jes Sorensen wrote:
>>> diff --git a/qemu-ioh.c b/qemu-ioh.c
>>> index cc71470..5c3f94c 100644
>>> --- a/qemu-ioh.c
>>> +++ b/qemu-ioh.c
>>> @@ -113,3 +117,94 @@ void qemu_process_fd_handlers2(void
>>> *ioh_record_list, const fd_set *rfds,
>>> }
>>> }
>>> }
>>> +
>>> +#ifndef _WIN32
>>> +void iothread_event_increment(int *io_thread_fd)
>>
>> Please move these functions into posix/w32 specific files so we don't
>> get anymore ugly #ifdefs. It would be good if we could use a wrapper
>> struct as well to hide the different data types so we don't need #ifdefs
>> in the calling code as well.
>
> Yup, meant to add this to the TODO. I may end up sending these general
> tools changes out in a separate patchset since they seem to be in
> conflict with quite of few patches floating around the list. Either way
> I'll make sure to get these cleaned up and tested a bit a more.
Sounds great! Since they are not directly part of virtagent you should
be able to push them in soon too.
Cheers,
Jes
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [Qemu-devel] Re: [RFC][PATCH v7 05/16] virtagent: common helpers and init routines
2011-03-09 10:38 ` [Qemu-devel] " Jes Sorensen
@ 2011-03-09 13:17 ` Michael Roth
0 siblings, 0 replies; 41+ messages in thread
From: Michael Roth @ 2011-03-09 13:17 UTC (permalink / raw)
To: Jes Sorensen
Cc: agl, stefanha, abeekhof, qemu-devel, aliguori, markus_mueller
On 03/09/2011 04:38 AM, Jes Sorensen wrote:
> On 03/07/11 21:10, Michael Roth wrote:
>> +#define VA_PIDFILE "/var/run/qemu-va.pid"
>> +#define VA_HDR_LEN_MAX 4096 /* http header limit */
>> +#define VA_CONTENT_LEN_MAX 2*1024*1024 /* rpc/http send limit */
>> +#define VA_CLIENT_JOBS_MAX 5 /* max client rpcs we can queue */
>> +#define VA_SERVER_JOBS_MAX 5 /* max server rpcs we can queue */
>> +#define VA_SERVER_TIMEOUT_MS 5 * 1000
>> +#define VA_CLIENT_TIMEOUT_MS 5 * 1000
>> +#define VA_SENTINEL 0xFF
>> +#define VA_BAUDRATE B38400 /* for isa-serial channels */
>> +
>
> I've been after these before - please put the ones that make sense to
> tune into a config file, and the same with the pidfile.
I think my contention last time was that most of these weren't meant to
be tweakable by an end-user, they're mainly just to avoid using magic
numbers everywhere.
For stuff that is, like the pid file and socket/port paths, these would
be the defaults, and the option to override them would be provided via
the command line (virtagent chardev options on the host, command options
on the guest).
I did plan to make the distinction between the 2 clearer though, by
adding a DEFAULT_* or something along that line. Will get those in for
the next pass.
>
> Cheers,
> Jes
>
>
^ permalink raw reply [flat|nested] 41+ messages in thread
* [Qemu-devel] Re: [RFC][PATCH v7 01/16] Move code related to fd handlers into utility functions
2011-03-07 20:10 ` [Qemu-devel] [RFC][PATCH v7 01/16] Move code related to fd handlers into utility functions Michael Roth
@ 2011-03-09 13:58 ` Paolo Bonzini
2011-03-09 14:11 ` Michael Roth
` (2 more replies)
2011-03-09 14:09 ` Paolo Bonzini
1 sibling, 3 replies; 41+ messages in thread
From: Paolo Bonzini @ 2011-03-09 13:58 UTC (permalink / raw)
To: Michael Roth
Cc: agl, stefanha, markus_mueller, qemu-devel, abeekhof, aliguori,
Jes.Sorensen
On 03/07/2011 09:10 PM, Michael Roth wrote:
> This allows us to implement an i/o loop outside of vl.c that can
> interact with objects that use qemu_set_fd_handler()
I must say I really dislike the patches 1..3. It's _really_ getting the
QEMU NIH worse. While it is not really possible to get a new shiny
mainloop infrastructure in QEMU like snapping fingers (and I'm not sure
the glib mainloop will ever happen there), there is no reason not to
adopt glib's infrastructure in virtagent. While cooperation between
QEMU and virtagent is close, it is IMHO a substantially separate project
that can afford starting from a clean slate.
If anybody disagrees, I'd be happy to hear their opinion anyway!
I'm sorry I'm saying this only now and I've been ignoring this series
until v7.
Paolo
^ permalink raw reply [flat|nested] 41+ messages in thread
* [Qemu-devel] Re: [RFC][PATCH v7 01/16] Move code related to fd handlers into utility functions
2011-03-07 20:10 ` [Qemu-devel] [RFC][PATCH v7 01/16] Move code related to fd handlers into utility functions Michael Roth
2011-03-09 13:58 ` [Qemu-devel] " Paolo Bonzini
@ 2011-03-09 14:09 ` Paolo Bonzini
1 sibling, 0 replies; 41+ messages in thread
From: Paolo Bonzini @ 2011-03-09 14:09 UTC (permalink / raw)
To: Michael Roth
Cc: agl, stefanha, markus_mueller, qemu-devel, abeekhof, aliguori,
Jes.Sorensen
On 03/07/2011 09:10 PM, Michael Roth wrote:
> +
> +/* XXX: fd_read_poll should be suppressed, but an API change is
> + necessary in the character devices to suppress fd_can_read(). */
> +int qemu_set_fd_handler3(void *ioh_record_list,
> + int fd,
> + IOCanReadHandler *fd_read_poll,
> + IOHandler *fd_read,
> + IOHandler *fd_write,
> + void *opaque)
What's the reason to introduce this additional indirection (and with a
void rather than opaque pointer)? A global iohandlers list would be
fine in qemu-ioh.c (and it would be a worthwhile patch anyway for QEMU).
Paolo
^ permalink raw reply [flat|nested] 41+ messages in thread
* [Qemu-devel] Re: [RFC][PATCH v7 01/16] Move code related to fd handlers into utility functions
2011-03-09 13:58 ` [Qemu-devel] " Paolo Bonzini
@ 2011-03-09 14:11 ` Michael Roth
2011-03-09 14:38 ` Paolo Bonzini
2011-03-09 14:28 ` Anthony Liguori
2011-03-09 14:40 ` Anthony Liguori
2 siblings, 1 reply; 41+ messages in thread
From: Michael Roth @ 2011-03-09 14:11 UTC (permalink / raw)
To: Paolo Bonzini
Cc: agl, stefanha, markus_mueller, qemu-devel, abeekhof, aliguori,
Jes.Sorensen
On 03/09/2011 07:58 AM, Paolo Bonzini wrote:
> On 03/07/2011 09:10 PM, Michael Roth wrote:
>> This allows us to implement an i/o loop outside of vl.c that can
>> interact with objects that use qemu_set_fd_handler()
>
> I must say I really dislike the patches 1..3. It's _really_ getting the
> QEMU NIH worse. While it is not really possible to get a new shiny
> mainloop infrastructure in QEMU like snapping fingers (and I'm not sure
> the glib mainloop will ever happen there), there is no reason not to
> adopt glib's infrastructure in virtagent. While cooperation between QEMU
> and virtagent is close, it is IMHO a substantially separate project that
> can afford starting from a clean slate.
>
> If anybody disagrees, I'd be happy to hear their opinion anyway!
>
> I'm sorry I'm saying this only now and I've been ignoring this series
> until v7.
In the context of virtagent I would agree. The only complication there
being that a large part of the event-driven code (the async read/write
handlers for instance) is shared between virtagent and the host.
Possibility this could be worked around with a set of wrappers..but it's
hard to say.
But more importantly, I wouldn't think of these changes as being
specific to virtagent though. Currently we have a lot of qemu tools that
stub out portions of the block code they pull in (qemu_set_fd_handler
and whatnot). I think it might be beneficial to future tools/test
utilities that they actually be able to drive things like aio and timer
events. We just keep stubbing more and more things out in these cases,
which I would argue is even worse because it can place artificial
constraints on how code is written that happens to get used by such tools.
>
> Paolo
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [Qemu-devel] Re: [RFC][PATCH v7 01/16] Move code related to fd handlers into utility functions
2011-03-09 13:58 ` [Qemu-devel] " Paolo Bonzini
2011-03-09 14:11 ` Michael Roth
@ 2011-03-09 14:28 ` Anthony Liguori
2011-03-09 14:40 ` Anthony Liguori
2 siblings, 0 replies; 41+ messages in thread
From: Anthony Liguori @ 2011-03-09 14:28 UTC (permalink / raw)
To: Paolo Bonzini
Cc: agl, stefanha, abeekhof, qemu-devel, Jes.Sorensen, aliguori,
markus_mueller
On 03/09/2011 07:58 AM, Paolo Bonzini wrote:
> On 03/07/2011 09:10 PM, Michael Roth wrote:
>> This allows us to implement an i/o loop outside of vl.c that can
>> interact with objects that use qemu_set_fd_handler()
>
> I must say I really dislike the patches 1..3. It's _really_ getting
> the QEMU NIH worse. While it is not really possible to get a new
> shiny mainloop infrastructure in QEMU like snapping fingers (and I'm
> not sure the glib mainloop will ever happen there), there is no reason
> not to adopt glib's infrastructure in virtagent.
I'm 90% in agreement with you but in terms of delivering a Windows guest
agent, instead of just having an exe, we're now talking about quite a
few extra DLLs. It's not a huge problem and probably makes a ton of
sense if virt-agent ever adopts more sophisticated functionality but I
wanted to at least raise this point.
Regards,
Anthony Liguori
> While cooperation between QEMU and virtagent is close, it is IMHO a
> substantially separate project that can afford starting from a clean
> slate.
>
> If anybody disagrees, I'd be happy to hear their opinion anyway!
>
> I'm sorry I'm saying this only now and I've been ignoring this series
> until v7.
>
> Paolo
>
>
^ permalink raw reply [flat|nested] 41+ messages in thread
* [Qemu-devel] Re: [RFC][PATCH v7 01/16] Move code related to fd handlers into utility functions
2011-03-09 14:11 ` Michael Roth
@ 2011-03-09 14:38 ` Paolo Bonzini
2011-03-09 15:01 ` Michael Roth
0 siblings, 1 reply; 41+ messages in thread
From: Paolo Bonzini @ 2011-03-09 14:38 UTC (permalink / raw)
To: Michael Roth
Cc: agl, stefanha, abeekhof, qemu-devel, Jes.Sorensen, aliguori,
markus_mueller
On 03/09/2011 03:11 PM, Michael Roth wrote:
>
> In the context of virtagent I would agree. The only complication there
> being that a large part of the event-driven code (the async read/write
> handlers for instance) is shared between virtagent and the host.
What exactly? The dependencies in 16/16 give:
qemu-tool.o qemu-error.o qemu-sockets.c $(oslib-obj-y) $(trace-obj-y)
$(block-obj-y) $(qobject-obj-y) $(version-obj-y) qemu-timer-common.o
qemu-timer.o
Compared to other tools, only qemu-sockets.c is added (and timers);
overall it is quite self contained and interfaces well with glib's
GIOChannels, which provide qemu_set_fd_handler-equivalent functionality.
In addition, qemu iohandlers have a lot of unwritten assumptions, for
example on Win32 they only work with sockets and not other kinds of file
descriptors.
Paolo
^ permalink raw reply [flat|nested] 41+ messages in thread
* [Qemu-devel] Re: [RFC][PATCH v7 01/16] Move code related to fd handlers into utility functions
2011-03-09 13:58 ` [Qemu-devel] " Paolo Bonzini
2011-03-09 14:11 ` Michael Roth
2011-03-09 14:28 ` Anthony Liguori
@ 2011-03-09 14:40 ` Anthony Liguori
2011-03-09 14:45 ` Paolo Bonzini
2 siblings, 1 reply; 41+ messages in thread
From: Anthony Liguori @ 2011-03-09 14:40 UTC (permalink / raw)
To: Paolo Bonzini
Cc: agl, stefanha, markus_mueller, Michael Roth, qemu-devel,
Jes.Sorensen, abeekhof
On 03/09/2011 07:58 AM, Paolo Bonzini wrote:
> On 03/07/2011 09:10 PM, Michael Roth wrote:
>> This allows us to implement an i/o loop outside of vl.c that can
>> interact with objects that use qemu_set_fd_handler()
>
> I must say I really dislike the patches 1..3. It's _really_ getting
> the QEMU NIH worse. While it is not really possible to get a new
> shiny mainloop infrastructure in QEMU like snapping fingers (and I'm
> not sure the glib mainloop will ever happen there
While it's not at the immediate top at my MUST DO list, it's still
pretty high FWIW. I think the benefits are huge because it means we can
refactor things like the VNC server to just interact with glib which
means it can become generally useful outside of QEMU.
Regards,
Anthony Liguori
> ), there is no reason not to adopt glib's infrastructure in
> virtagent. While cooperation between QEMU and virtagent is close, it
> is IMHO a substantially separate project that can afford starting from
> a clean slate.
>
> If anybody disagrees, I'd be happy to hear their opinion anyway!
>
> I'm sorry I'm saying this only now and I've been ignoring this series
> until v7.
>
> Paolo
^ permalink raw reply [flat|nested] 41+ messages in thread
* [Qemu-devel] Re: [RFC][PATCH v7 01/16] Move code related to fd handlers into utility functions
2011-03-09 14:40 ` Anthony Liguori
@ 2011-03-09 14:45 ` Paolo Bonzini
2011-03-09 15:39 ` Anthony Liguori
0 siblings, 1 reply; 41+ messages in thread
From: Paolo Bonzini @ 2011-03-09 14:45 UTC (permalink / raw)
To: Anthony Liguori
Cc: agl, stefanha, markus_mueller, Michael Roth, qemu-devel,
Jes.Sorensen, abeekhof
On 03/09/2011 03:40 PM, Anthony Liguori wrote:
>>
>> I must say I really dislike the patches 1..3. It's _really_ getting
>> the QEMU NIH worse. While it is not really possible to get a new
>> shiny mainloop infrastructure in QEMU like snapping fingers (and I'm
>> not sure the glib mainloop will ever happen there
>
> While it's not at the immediate top at my MUST DO list, it's still
> pretty high FWIW. I think the benefits are huge because it means we can
> refactor things like the VNC server to just interact with glib which
> means it can become generally useful outside of QEMU.
I actually agree, but there are a lot of cleanups to do to the code
before it becomes viable. I would be surprised to see it before 0.17
say (maybe a pleasant surprise, but still).
In any case, introducing more dependencies from the tools to core QEMU
would mean needing wrappers over wrappers over wrappers when QEMU itself
is refactored.
Perhaps for virtagent something like libnih would be more appropriate?
Not sure about its Win32 portability though.
Paolo
^ permalink raw reply [flat|nested] 41+ messages in thread
* [Qemu-devel] Re: [RFC][PATCH v7 01/16] Move code related to fd handlers into utility functions
2011-03-09 14:38 ` Paolo Bonzini
@ 2011-03-09 15:01 ` Michael Roth
2011-03-09 15:15 ` Paolo Bonzini
0 siblings, 1 reply; 41+ messages in thread
From: Michael Roth @ 2011-03-09 15:01 UTC (permalink / raw)
To: Paolo Bonzini
Cc: agl, stefanha, abeekhof, qemu-devel, Jes.Sorensen, aliguori,
markus_mueller
On 03/09/2011 08:38 AM, Paolo Bonzini wrote:
> On 03/09/2011 03:11 PM, Michael Roth wrote:
>>
>> In the context of virtagent I would agree. The only complication there
>> being that a large part of the event-driven code (the async read/write
>> handlers for instance) is shared between virtagent and the host.
>
> What exactly? The dependencies in 16/16 give:
>
> qemu-tool.o qemu-error.o qemu-sockets.c $(oslib-obj-y) $(trace-obj-y)
> $(block-obj-y) $(qobject-obj-y) $(version-obj-y) qemu-timer-common.o
> qemu-timer.o
These objs: virtagent.o virtagent-server.o virtagent-common.o
virtagent-transport.o virtagent-manager.o
Are shared by qemu and qemu-va. virtagent.o uses the common timer
infrastructure introduced in patch 3, and
virtagent-transport/virtagent-common use the iohandler stuff from patch 1/2.
On the host, qemu's event loop drives them, and on the guest, qemu-va's
event loop drives them.
Not sure what level of sharing we can maintain with 2 different event
loops. I'm sure it's doable, just not sure what it would end up looking
like.
I should note that initially all the qemu_set_fd_handler() stuff was
wrapped to provide compatibility between separate event loop
implementations in qemu/qemu-va. Sharing the event loop code was a
widely-held consensus from earlier reviews. I'm not sure glib is so nice
that it's worth back-peddling on that. And if we do eventually make
qemu's event loop glib-based, consumers of the common code here would
get migrated over for free.
>
> Compared to other tools, only qemu-sockets.c is added (and timers);
> overall it is quite self contained and interfaces well with glib's
> GIOChannels, which provide qemu_set_fd_handler-equivalent functionality.
>
> In addition, qemu iohandlers have a lot of unwritten assumptions, for
> example on Win32 they only work with sockets and not other kinds of file
> descriptors.
Hmm, that could be a problem... It seems like a more general one though,
that might benefit consumers other than virtagent. So if this is
addressed at some point, consumers of the common infrastructure proposed
here would all benefit.
>
> Paolo
^ permalink raw reply [flat|nested] 41+ messages in thread
* [Qemu-devel] Re: [RFC][PATCH v7 01/16] Move code related to fd handlers into utility functions
2011-03-09 15:01 ` Michael Roth
@ 2011-03-09 15:15 ` Paolo Bonzini
0 siblings, 0 replies; 41+ messages in thread
From: Paolo Bonzini @ 2011-03-09 15:15 UTC (permalink / raw)
To: Michael Roth
Cc: agl, stefanha, abeekhof, qemu-devel, Jes.Sorensen, aliguori,
markus_mueller
On 03/09/2011 04:01 PM, Michael Roth wrote:
>
> These objs: virtagent.o virtagent-server.o virtagent-common.o
> virtagent-transport.o virtagent-manager.o
>
> Are shared by qemu and qemu-va.
Okay, that's what I missed. Then I guess it's a pity but there's a good
reason.
> It seems like a more general one though, that might benefit consumers
> other than virtagent. So if this is addressed at some point,
> consumers of the common infrastructure proposed here would all
> benefit.
I doubt, Win32 sockets are almost unusable (QEMU does "select" on them
when it has an event from something else) and chardevs use a separate
polling mechanism.
Paolo
^ permalink raw reply [flat|nested] 41+ messages in thread
* [Qemu-devel] Re: [RFC][PATCH v7 01/16] Move code related to fd handlers into utility functions
2011-03-09 14:45 ` Paolo Bonzini
@ 2011-03-09 15:39 ` Anthony Liguori
0 siblings, 0 replies; 41+ messages in thread
From: Anthony Liguori @ 2011-03-09 15:39 UTC (permalink / raw)
To: Paolo Bonzini
Cc: agl, stefanha, markus_mueller, Michael Roth, qemu-devel,
Jes.Sorensen, abeekhof
On 03/09/2011 08:45 AM, Paolo Bonzini wrote:
> On 03/09/2011 03:40 PM, Anthony Liguori wrote:
>>>
>>> I must say I really dislike the patches 1..3. It's _really_ getting
>>> the QEMU NIH worse. While it is not really possible to get a new
>>> shiny mainloop infrastructure in QEMU like snapping fingers (and I'm
>>> not sure the glib mainloop will ever happen there
>>
>> While it's not at the immediate top at my MUST DO list, it's still
>> pretty high FWIW. I think the benefits are huge because it means we can
>> refactor things like the VNC server to just interact with glib which
>> means it can become generally useful outside of QEMU.
>
> I actually agree, but there are a lot of cleanups to do to the code
> before it becomes viable. I would be surprised to see it before 0.17
> say (maybe a pleasant surprise, but still).
>
> In any case, introducing more dependencies from the tools to core QEMU
> would mean needing wrappers over wrappers over wrappers when QEMU
> itself is refactored.
>
> Perhaps for virtagent something like libnih would be more appropriate?
> Not sure about its Win32 portability though.
If we do any lib, it should be glib. I'm just playing devil's advocate
here in pointing out that adding DLL dependences on Windows is a bit
painful although probably unavoidable.
Regards,
Anthony Liguori
> Paolo
^ permalink raw reply [flat|nested] 41+ messages in thread
end of thread, other threads:[~2011-03-09 15:39 UTC | newest]
Thread overview: 41+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-03-07 20:10 [Qemu-devel] [RFC][PATCH v7 00/16] virtagent: host/guest communication agent Michael Roth
2011-03-07 20:10 ` [Qemu-devel] [RFC][PATCH v7 01/16] Move code related to fd handlers into utility functions Michael Roth
2011-03-09 13:58 ` [Qemu-devel] " Paolo Bonzini
2011-03-09 14:11 ` Michael Roth
2011-03-09 14:38 ` Paolo Bonzini
2011-03-09 15:01 ` Michael Roth
2011-03-09 15:15 ` Paolo Bonzini
2011-03-09 14:28 ` Anthony Liguori
2011-03-09 14:40 ` Anthony Liguori
2011-03-09 14:45 ` Paolo Bonzini
2011-03-09 15:39 ` Anthony Liguori
2011-03-09 14:09 ` Paolo Bonzini
2011-03-07 20:10 ` [Qemu-devel] [RFC][PATCH v7 02/16] Add qemu_set_fd_handler() wrappers to qemu-tools.c Michael Roth
2011-03-07 20:10 ` [Qemu-devel] [RFC][PATCH v7 03/16] Make qemu timers available for tools Michael Roth
2011-03-09 10:33 ` [Qemu-devel] " Jes Sorensen
2011-03-09 13:04 ` Michael Roth
2011-03-09 13:06 ` Jes Sorensen
2011-03-07 20:10 ` [Qemu-devel] [RFC][PATCH v7 04/16] virtagent: bi-directional RPC handling logic Michael Roth
2011-03-07 21:24 ` [Qemu-devel] " Adam Litke
2011-03-07 22:35 ` Michael Roth
2011-03-07 20:10 ` [Qemu-devel] [RFC][PATCH v7 05/16] virtagent: common helpers and init routines Michael Roth
2011-03-09 10:38 ` [Qemu-devel] " Jes Sorensen
2011-03-09 13:17 ` Michael Roth
2011-03-07 20:10 ` [Qemu-devel] [RFC][PATCH v7 06/16] virtagent: transport definitions Michael Roth
2011-03-07 21:38 ` [Qemu-devel] " Adam Litke
2011-03-07 20:10 ` [Qemu-devel] [RFC][PATCH v7 07/16] virtagent: base RPC client definitions Michael Roth
2011-03-07 20:10 ` [Qemu-devel] [RFC][PATCH v7 08/16] virtagnet: base RPC server definitions Michael Roth
2011-03-07 20:10 ` [Qemu-devel] [RFC][PATCH v7 09/16] virtagent: add va_capabilities HMP/QMP command Michael Roth
2011-03-07 20:10 ` [Qemu-devel] [RFC][PATCH v7 10/16] virtagent: add "ping" RPC to server Michael Roth
2011-03-07 20:10 ` [Qemu-devel] [RFC][PATCH v7 11/16] virtagent: add va_ping HMP/QMP command Michael Roth
2011-03-07 20:10 ` [Qemu-devel] [RFC][PATCH v7 12/16] virtagent: add "shutdown" RPC to server Michael Roth
2011-03-07 20:10 ` [Qemu-devel] [RFC][PATCH v7 13/16] virtagent: add va_shutdown HMP/QMP command Michael Roth
2011-03-07 20:10 ` [Qemu-devel] [RFC][PATCH v7 14/16] virtagent: add virtagent chardev Michael Roth
2011-03-07 20:10 ` [Qemu-devel] [RFC][PATCH v7 15/16] virtagent: qemu-va, system-level virtagent guest agent Michael Roth
2011-03-09 10:48 ` [Qemu-devel] " Jes Sorensen
2011-03-07 20:10 ` [Qemu-devel] [RFC][PATCH v7 16/16] virtagent: add bits to build virtagent host/guest components Michael Roth
2011-03-07 21:43 ` [Qemu-devel] Re: [RFC][PATCH v7 00/16] virtagent: host/guest communication agent Anthony Liguori
2011-03-07 22:49 ` Michael Roth
2011-03-07 22:56 ` Anthony Liguori
2011-03-08 0:11 ` Michael Roth
2011-03-08 0:24 ` Anthony Liguori
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).