date:20101115

On Mon, Nov 15, 2010 at 09:52:19PM -0500, Kevin O'Connor wrote:
> On Mon, Nov 15, 2010 at 03:36:25PM +0200, Gleb Natapov wrote:
> > On Mon, Nov 15, 2010 at 08:26:35AM -0500, Kevin O'Connor wrote:
> > > On Mon, Nov 15, 2010 at 09:40:08AM +0200, Gleb Natapov wrote:
> > > > On Sun, Nov 14, 2010 at 10:40:33PM -0500, Kevin O'Connor wrote:
> > > > > Why not just return a newline separated list that is null terminated?
> > > > > 
> > > > Doing it like this will needlessly complicate firmware side. How do you
> > > > know how much memory to allocate before reading device list?
> > > 
> > > My preference would be for the size to be exposed via the
> > > QEMU_CFG_FILE_DIR selector.  (My preference would be for all objects
> > > in fw_cfg to have entries in QEMU_CFG_FILE_DIR describing their size
> > > in a reliable manner.)
> > > 
> > Will interface suggested by Blue will be good for you? The one with two
> > fw_cfg ids. BOOTINDEX_LEN for len and BOOTINDEX_DATA for device list. I
> 
> I dislike how different fw_cfg objects pass the length in different
> ways (eg, QEMU_CFG_E820_TABLE passes length as first 4 bytes).  This
> is a common problem - I'd prefer if we could adopt one uniform way of
> passing length.  I think QEMU_CFG_FILE_DIR solves this problem well.
>
Looking at available fw cfg option I see that _SIZE _DATA is also a
common pattern. The problem with QEMU_CFG_FILE_DIR is that we have very
little available slots right now. If we a going to require everything to
use it we better grow number of available slots considerably now while
it is easily done (no option defined above file slots yet).

I personally do not have preferences one way or the other. Blue are you
OK with using QEMU_CFG_FILE_DIR?

> I also have an ulterior motive here.  If the boot order is exposed as
> a newline separated list via an entry in QEMU_CFG_FILE_DIR, then this
> becomes free for coreboot users as well.  (On coreboot, the boot order
> could be placed in a "file" in flash with no change to the seabios
> code.)
> 
You can define get_boot_order() function and implement it differently
for qemu and coreboot. For coreboot it will be one linear. Just call
cbfs_copyfile("bootorder"). BTW why newline separation is important? 

--
Gleb.

[Qemu-devel] [PATCH 3/4] virtio-net: Convert fprintf() to error_report()

Signed-off-by: Stefan Hajnoczi 
---
 hw/virtio-net.c |   41 -
 1 files changed, 20 insertions(+), 21 deletions(-)

diff --git a/hw/virtio-net.c b/hw/virtio-net.c
index 7e1688c..1d61f19 100644
--- a/hw/virtio-net.c
+++ b/hw/virtio-net.c
@@ -120,8 +120,8 @@ static void virtio_net_set_status(struct VirtIODevice 
*vdev, uint8_t status)
 if (!n->vhost_started) {
 int r = vhost_net_start(tap_get_vhost_net(n->nic->nc.peer), &n->vdev);
 if (r < 0) {
-fprintf(stderr, "unable to start vhost net: %d: "
-"falling back on userspace virtio\n", -r);
+error_report("unable to start vhost net: %d: "
+ "falling back on userspace virtio", -r);
 } else {
 n->vhost_started = 1;
 }
@@ -271,7 +271,7 @@ static int virtio_net_handle_rx_mode(VirtIONet *n, uint8_t 
cmd,
 uint8_t on;
 
 if (elem->out_num != 2 || elem->out_sg[1].iov_len != sizeof(on)) {
-fprintf(stderr, "virtio-net ctrl invalid rx mode command\n");
+error_report("virtio-net ctrl invalid rx mode command");
 exit(1);
 }
 
@@ -353,7 +353,7 @@ static int virtio_net_handle_vlan_table(VirtIONet *n, 
uint8_t cmd,
 uint16_t vid;
 
 if (elem->out_num != 2 || elem->out_sg[1].iov_len != sizeof(vid)) {
-fprintf(stderr, "virtio-net ctrl invalid vlan command\n");
+error_report("virtio-net ctrl invalid vlan command");
 return VIRTIO_NET_ERR;
 }
 
@@ -381,13 +381,13 @@ static void virtio_net_handle_ctrl(VirtIODevice *vdev, 
VirtQueue *vq)
 
 while (virtqueue_pop(vq, &elem)) {
 if ((elem.in_num < 1) || (elem.out_num < 1)) {
-fprintf(stderr, "virtio-net ctrl missing headers\n");
+error_report("virtio-net ctrl missing headers");
 exit(1);
 }
 
 if (elem.out_sg[0].iov_len < sizeof(ctrl) ||
 elem.in_sg[elem.in_num - 1].iov_len < sizeof(status)) {
-fprintf(stderr, "virtio-net ctrl header not in correct element\n");
+error_report("virtio-net ctrl header not in correct element");
 exit(1);
 }
 
@@ -591,21 +591,21 @@ static ssize_t virtio_net_receive(VLANClientState *nc, 
const uint8_t *buf, size_
 if (virtqueue_pop(n->rx_vq, &elem) == 0) {
 if (i == 0)
 return -1;
-fprintf(stderr, "virtio-net unexpected empty queue: "
+error_report("virtio-net unexpected empty queue: "
 "i %zd mergeable %d offset %zd, size %zd, "
-"guest hdr len %zd, host hdr len %zd guest features 
0x%x\n",
+"guest hdr len %zd, host hdr len %zd guest features 0x%x",
 i, n->mergeable_rx_bufs, offset, size,
 guest_hdr_len, host_hdr_len, n->vdev.guest_features);
 exit(1);
 }
 
 if (elem.in_num < 1) {
-fprintf(stderr, "virtio-net receive queue contains no in 
buffers\n");
+error_report("virtio-net receive queue contains no in buffers");
 exit(1);
 }
 
 if (!n->mergeable_rx_bufs && elem.in_sg[0].iov_len != guest_hdr_len) {
-fprintf(stderr, "virtio-net header not in first element\n");
+error_report("virtio-net header not in first element");
 exit(1);
 }
 
@@ -630,12 +630,11 @@ static ssize_t virtio_net_receive(VLANClientState *nc, 
const uint8_t *buf, size_
  * Otherwise, drop it. */
 if (!n->mergeable_rx_bufs && offset < size) {
 #if 0
-fprintf(stderr, "virtio-net truncated non-mergeable packet: "
-
-"i %zd mergeable %d offset %zd, size %zd, "
-"guest hdr len %zd, host hdr len %zd\n",
-i, n->mergeable_rx_bufs,
-offset, size, guest_hdr_len, host_hdr_len);
+error_report("virtio-net truncated non-mergeable packet: "
+ "i %zd mergeable %d offset %zd, size %zd, "
+ "guest hdr len %zd, host hdr len %zd",
+ i, n->mergeable_rx_bufs,
+ offset, size, guest_hdr_len, host_hdr_len);
 #endif
 return size;
 }
@@ -695,7 +694,7 @@ static int32_t virtio_net_flush_tx(VirtIONet *n, VirtQueue 
*vq)
 sizeof(struct virtio_net_hdr);
 
 if (out_num < 1 || out_sg->iov_len != hdr_len) {
-fprintf(stderr, "virtio-net header not in first element\n");
+error_report("virtio-net header not in first element");
 exit(1);
 }
 
@@ -981,10 +980,10 @@ VirtIODevice *virtio_net_init(DeviceState *dev, NICConf 
*conf,
 n->rx_vq = virtio_add_queue(&n->vdev, 256, virtio_net_handle_rx);
 
 if (net->tx && strcmp(net->tx, "timer") && strcmp(net->tx, "bh")) {
-fprintf(stderr, "virtio-net: "
-"Unknown option tx=%s, valid options: \"t

[Qemu-devel] Re: [PATCHv4 15/15] Pass boot device list to firmware.

2010-11-15 Thread Kevin O'Connor

On Mon, Nov 15, 2010 at 03:36:25PM +0200, Gleb Natapov wrote:
> On Mon, Nov 15, 2010 at 08:26:35AM -0500, Kevin O'Connor wrote:
> > On Mon, Nov 15, 2010 at 09:40:08AM +0200, Gleb Natapov wrote:
> > > On Sun, Nov 14, 2010 at 10:40:33PM -0500, Kevin O'Connor wrote:
> > > > Why not just return a newline separated list that is null terminated?
> > > > 
> > > Doing it like this will needlessly complicate firmware side. How do you
> > > know how much memory to allocate before reading device list?
> > 
> > My preference would be for the size to be exposed via the
> > QEMU_CFG_FILE_DIR selector.  (My preference would be for all objects
> > in fw_cfg to have entries in QEMU_CFG_FILE_DIR describing their size
> > in a reliable manner.)
> > 
> Will interface suggested by Blue will be good for you? The one with two
> fw_cfg ids. BOOTINDEX_LEN for len and BOOTINDEX_DATA for device list. I

I dislike how different fw_cfg objects pass the length in different
ways (eg, QEMU_CFG_E820_TABLE passes length as first 4 bytes).  This
is a common problem - I'd prefer if we could adopt one uniform way of
passing length.  I think QEMU_CFG_FILE_DIR solves this problem well.

I also have an ulterior motive here.  If the boot order is exposed as
a newline separated list via an entry in QEMU_CFG_FILE_DIR, then this
becomes free for coreboot users as well.  (On coreboot, the boot order
could be placed in a "file" in flash with no change to the seabios
code.)

-Kevin

[Qemu-devel] [PATCH 4/4] virtio-pci: Convert fprintf() to error_report()

Signed-off-by: Stefan Hajnoczi 
---
 hw/virtio-pci.c |4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/virtio-pci.c b/hw/virtio-pci.c
index 729917d..b9d0349 100644
--- a/hw/virtio-pci.c
+++ b/hw/virtio-pci.c
@@ -254,8 +254,8 @@ static void virtio_ioport_write(void *opaque, uint32_t 
addr, uint32_t val)
 virtio_queue_set_vector(vdev, vdev->queue_sel, val);
 break;
 default:
-fprintf(stderr, "%s: unexpected address 0x%x value 0x%x\n",
-__func__, addr, val);
+error_report("%s: unexpected address 0x%x value 0x%x",
+ __func__, addr, val);
 break;
 }
 }
-- 
1.7.2.3

[Qemu-devel] [RFC][PATCH v3 21/21] virtproxy: Makefile/configure changes to build qemu-vp


Signed-off-by: Michael Roth 
---
 .gitignore |1 +
 Makefile   |4 +++-
 configure  |1 +
 3 files changed, 5 insertions(+), 1 deletions(-)

diff --git a/.gitignore b/.gitignore
index a43e4d1..da307d2 100644
--- a/.gitignore
+++ b/.gitignore
@@ -31,6 +31,7 @@ qemu-img-cmds.texi
 qemu-img-cmds.h
 qemu-io
 qemu-monitor.texi
+qemu-vp
 QMP/qmp-commands.txt
 .gdbinit
 *.a
diff --git a/Makefile b/Makefile
index 02698e9..b4f93eb 100644
--- a/Makefile
+++ b/Makefile
@@ -129,7 +129,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: $(GENERATED_HEADERS)
+qemu-img.o qemu-tool.o qemu-nbd.o qemu-io.o qemu-vp.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
 
@@ -137,6 +137,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-vp$(EXESUF): qemu-vp.o virtproxy.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-img-cmds.h: $(SRC_PATH)/qemu-img-cmds.hx
$(call quiet-command,sh $(SRC_PATH)/hxtool -h < $< > $@,"  GEN   $@")
 
diff --git a/configure b/configure
index 7025d2b..01bde83 100755
--- a/configure
+++ b/configure
@@ -2250,6 +2250,7 @@ if test "$softmmu" = yes ; then
   tools="qemu-img\$(EXESUF) qemu-io\$(EXESUF) $tools"
   if [ "$linux" = "yes" -o "$bsd" = "yes" -o "$solaris" = "yes" ] ; then
   tools="qemu-nbd\$(EXESUF) $tools"
+  tools="qemu-vp\$(EXESUF) $tools"
 if [ "$check_utests" = "yes" ]; then
   tools="check-qint check-qstring check-qdict check-qlist $tools"
   tools="check-qfloat check-qjson $tools"
-- 
1.7.0.4

Re: [Qemu-devel] How to detect a stopped guest os?

2010-11-15 Thread Mulyadi Santosa

On Tue, Nov 16, 2010 at 02:54, Wilhelm  wrote:
> Hello,
>
> I know, this is not the right place to ask, but I wasn't abled to find a
> users mailing list.
>
> The question: is there any qemu (monitor) command to detect if the guest
> os has stopped / poweroff?

have you checked "info status"?

-- 
regards,

Mulyadi Santosa
Freelance Linux trainer and consultant

blog: the-hydra.blogspot.com
training: mulyaditraining.blogspot.com

[Qemu-devel] [RFC][PATCH v3 15/21] virtproxy: add read handler for proxied connections

reads data from client/server connections as they become readable, then
sends the data over the channel

Signed-off-by: Michael Roth 
---
 virtproxy.c |  114 +++
 virtproxy.h |1 +
 2 files changed, 115 insertions(+), 0 deletions(-)

diff --git a/virtproxy.c b/virtproxy.c
index 9e1745f..f1f6859 100644
--- a/virtproxy.c
+++ b/virtproxy.c
@@ -37,6 +37,40 @@
 #define VP_CHAN_DATA_LEN 4096   /* max bytes channel can send at a time */
 #define VP_MAGIC 0x1F374059
 
+QemuOptsList vp_opts = {
+.name = "vpargs",
+.head = QTAILQ_HEAD_INITIALIZER(vp_opts.head),
+.desc = {
+{
+.name = "service_id",
+.type = QEMU_OPT_STRING,
+},{
+.name = "channel_method",
+.type = QEMU_OPT_STRING,
+},{
+.name = "index",
+.type = QEMU_OPT_NUMBER,
+},{
+.name = "path",
+.type = QEMU_OPT_STRING,
+},{
+.name = "host",
+.type = QEMU_OPT_STRING,
+},{
+.name = "port",
+.type = QEMU_OPT_STRING,
+},{
+.name = "ipv4",
+.type = QEMU_OPT_BOOL,
+},{
+.name = "ipv6",
+.type = QEMU_OPT_BOOL,
+},
+{ /* end of list */ }
+},
+};
+
+
 /* listening fd, one for each service we're forwarding to remote end */
 typedef struct VPOForward {
 VPDriver *drv;
@@ -233,6 +267,86 @@ static VPIForward *get_iforward(const VPDriver *drv, const 
char *service_id)
 return NULL;
 }
 
+/* read handler for proxied connections */
+static void vp_conn_read(void *opaque)
+{
+VPConn *conn = opaque;
+VPDriver *drv = conn->drv;
+VPPacket pkt;
+char buf[VP_CONN_DATA_LEN];
+int fd, count, ret;
+bool client;
+
+TRACE("called with opaque: %p, drv: %p", opaque, drv);
+
+if (conn->state != VP_STATE_CONNECTED) {
+LOG("invalid connection state");
+return;
+}
+
+if (conn->type != VP_CONN_CLIENT && conn->type != VP_CONN_SERVER) {
+LOG("invalid connection type");
+return;
+}
+
+/* TODO: all fields should be explicitly set so we shouldn't
+ * need to memset. this might hurt if we beef up VPPacket size
+ */
+memset(&pkt, 0, sizeof(VPPacket));
+pkt.magic = VP_MAGIC;
+
+if (conn->type == VP_CONN_CLIENT) {
+client = true;
+fd = conn->client_fd;
+} else {
+client = false;
+fd = conn->server_fd;
+}
+
+count = read(fd, buf, VP_CONN_DATA_LEN);
+if (count == -1) {
+LOG("read() failed: %s", strerror(errno));
+return;
+} else if (count == 0) {
+/* connection closed, tell remote end to clean up */
+TRACE("connection closed");
+pkt.type = VP_PKT_CONTROL;
+pkt.payload.msg.type = VP_CONTROL_CLOSE;
+if (client) {
+/* we're closing the client, have remote close the server conn */
+TRACE("closing connection for client fd %d", conn->client_fd);
+pkt.payload.msg.args.close.client_fd = -1;
+pkt.payload.msg.args.close.server_fd = conn->server_fd;
+} else {
+TRACE("closing connection for server fd %d", conn->server_fd);
+pkt.payload.msg.args.close.server_fd = -1;
+pkt.payload.msg.args.close.client_fd = conn->client_fd;;
+}
+/* clean up things on our end */
+closesocket(fd);
+vp_set_fd_handler(fd, NULL, NULL, NULL);
+QLIST_REMOVE(conn, next);
+qemu_free(conn);
+} else {
+TRACE("data read");
+pkt.type = client ? VP_PKT_CLIENT : VP_PKT_SERVER;
+pkt.payload.proxied.client_fd = conn->client_fd;
+pkt.payload.proxied.server_fd = conn->server_fd;
+memcpy(pkt.payload.proxied.data, buf, count);
+pkt.payload.proxied.bytes = count;
+}
+
+ret = vp_channel_send_all(drv, (uint8_t*)&pkt, sizeof(VPPacket));
+if (ret == -1) {
+LOG("error sending data over channel: %s", strerror(errno));
+return;
+}
+if (ret != sizeof(VPPacket)) {
+TRACE("buffer full?");
+return;
+}
+}
+
 /* accept handler for communication channel
  *
  * accept()s connection to communication channel (for sockets), and sets
diff --git a/virtproxy.h b/virtproxy.h
index 83cc905..ef168b0 100644
--- a/virtproxy.h
+++ b/virtproxy.h
@@ -23,6 +23,7 @@ enum vp_context {
 VP_CTX_CHARDEV, /* in qemu/host, channel is a virtproxy chardev */
 VP_CTX_FD,  /* in guest, channel is an FD */
 };
+extern QemuOptsList vp_opts;
 
 /* wrappers for s/vp/qemu/ functions we need */
 int vp_send_all(int fd, const void *buf, int len1);
-- 
1.7.0.4

[Qemu-devel] [RFC][PATCH v3 08/21] virtproxy: add vp_new() VPDriver constructor


Signed-off-by: Michael Roth 
---
 virtproxy.c |   40 
 virtproxy.h |6 ++
 2 files changed, 46 insertions(+), 0 deletions(-)

diff --git a/virtproxy.c b/virtproxy.c
index 091a223..401d51c 100644
--- a/virtproxy.c
+++ b/virtproxy.c
@@ -359,3 +359,43 @@ static void vp_channel_read(void *opaque)
 LOG("error handling packet stream");
 }
 }
+
+VPDriver *vp_new(enum vp_context ctx, CharDriverState *s, int fd, bool listen)
+{
+VPDriver *drv = NULL;
+
+drv = qemu_mallocz(sizeof(VPDriver));
+drv->listen_fd = -1;
+drv->channel_fd = -1;
+drv->chr = s;
+drv->ctx = ctx;
+QLIST_INIT(&drv->oforwards);
+QLIST_INIT(&drv->conns);
+
+if (ctx == VP_CTX_CHARDEV) {
+if (drv->chr == NULL) {
+LOG("invalid virtproxy chardev");
+goto out_bad;
+}
+} else if (ctx == VP_CTX_FD) {
+if (fd <= 0) {
+LOG("invalid FD");
+goto out_bad;
+} else if (listen) {
+/* provided FD is to be listened on for channel connection */
+drv->listen_fd = fd;
+vp_set_fd_handler(drv->listen_fd, vp_channel_accept, NULL, drv);
+} else {
+drv->channel_fd = fd;
+vp_set_fd_handler(drv->channel_fd, vp_channel_read, NULL, drv);
+}
+} else {
+LOG("invalid context");
+goto out_bad;
+}
+
+return drv;
+out_bad:
+qemu_free(drv);
+return NULL;
+}
diff --git a/virtproxy.h b/virtproxy.h
index 8fa0142..7cd2f76 100644
--- a/virtproxy.h
+++ b/virtproxy.h
@@ -16,8 +16,13 @@
 
 #include "qemu-common.h"
 #include "qemu-queue.h"
+#include "qemu-char.h"
 
 typedef struct VPDriver VPDriver;
+enum vp_context {
+VP_CTX_CHARDEV, /* in qemu/host, channel is a virtproxy chardev */
+VP_CTX_FD,  /* in guest, channel is an FD */
+};
 
 /* wrappers for s/vp/qemu/ functions we need */
 int vp_send_all(int fd, const void *buf, int len1);
@@ -33,6 +38,7 @@ int vp_set_fd_handler(int fd,
 void vp_chr_read(CharDriverState *s, uint8_t *buf, int len);
 
 /* virtproxy interface */
+VPDriver *vp_new(enum vp_context ctx, CharDriverState *s, int fd, bool listen);
 int vp_handle_packet_buf(VPDriver *drv, const void *buf, int count);
 
 #endif /* VIRTPROXY_H */
-- 
1.7.0.4

[Qemu-devel] [RFC][PATCH v3 05/21] virtproxy, add vp_channel_send_all

This handles sending of data to channel fd (qemu-vp in guest) or the
device associated with the virtproxy chardev for the host depending on
the context. vp_chr_read() wraps qemu_chr_read(), it'll be defined later
in virtproxy-builtin.c, and noop'd in the guest agent via qemu-vp.c.

Signed-off-by: Michael Roth 
---
 virtproxy.c |   31 +++
 virtproxy.h |1 +
 2 files changed, 32 insertions(+), 0 deletions(-)

diff --git a/virtproxy.c b/virtproxy.c
index 2cfd905..edca62e 100644
--- a/virtproxy.c
+++ b/virtproxy.c
@@ -152,6 +152,37 @@ static QemuOptsList vp_socket_opts = {
 },
 };
 
+static int vp_channel_send_all(VPDriver *drv, uint8_t *buf, int count)
+{
+int ret;
+CharDriverState *chr = drv->chr;
+
+if (drv->chr != NULL) {
+/* send data to guest via channel device's read handler */
+vp_chr_read(chr, buf, count);
+/* TODO: we assume here the full buffer was written to device
+ * due to the dev write handler being a void function.
+ * can we confirm? Do we need to?
+ */
+ret = count;
+} else if (drv->channel_fd != -1) {
+/* send data to host via channel fd */
+ret = vp_send_all(drv->channel_fd, buf, count);
+if (ret == -1) {
+LOG("error sending data");
+goto out_bad;
+}
+} else {
+LOG("driver in unknown state");
+goto out_bad;
+}
+
+return ret;
+out_bad:
+LOG("unable to send to channel");
+return -1;
+}
+
 /* get VPConn by fd, "client" denotes whether to look for client or server */
 static VPConn *get_conn(const VPDriver *drv, int fd, bool client)
 {
diff --git a/virtproxy.h b/virtproxy.h
index 0203421..1a5e56a 100644
--- a/virtproxy.h
+++ b/virtproxy.h
@@ -30,5 +30,6 @@ int vp_set_fd_handler(int fd,
 IOHandler *fd_read,
 IOHandler *fd_write,
 void *opaque);
+void vp_chr_read(CharDriverState *s, uint8_t *buf, int len);
 
 #endif /* VIRTPROXY_H */
-- 
1.7.0.4

[Qemu-devel] [RFC][PATCH v3 18/21] virtproxy: qemu integration, add virtproxy chardev

This allows us to create a virtproxy instance via a chardev. It can now
be created with something like:

qemu -chardev virtproxy,id=vp1 \
 -device virtio-serial \
 -device virtserialport,chardev=vp1

In the future the ability to add oforwards/iforwards in the command-line
invocation and the monitor will be added. For now we leave it to users
of virtproxy (currently only virtagent) to set up the forwarding
sockets/ports they need via direct virtproxy API calls.

Signed-off-by: Michael Roth 
---
 qemu-char.c   |  130 +
 qemu-config.c |6 +++
 2 files changed, 136 insertions(+), 0 deletions(-)

diff --git a/qemu-char.c b/qemu-char.c
index 88997f9..bc7925c 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -1902,6 +1902,135 @@ return_err:
 }
 
 /***/
+/* Virtproxy chardev driver */
+
+#include "virtproxy.h"
+
+static int vp_init_oforward(VPDriver *drv, QemuOpts *opts)
+{
+int ret, fd;
+const char *service_id;
+
+if (qemu_opt_get(opts, "host") != NULL) {
+fd = inet_listen_opts(opts, 0);
+} else if (qemu_opt_get(opts, "path") != NULL) {
+fd = unix_listen_opts(opts);
+} else {
+fprintf(stderr, "unable to find listening socket host/addr info");
+return -1;
+}
+
+if (fd == -1) {
+fprintf(stderr, "failed to create FD");
+return -1;
+}
+
+service_id = qemu_opt_get(opts, "service_id");
+if (service_id == NULL) {
+fprintf(stderr, "no service_id specified");
+return -1;
+}
+
+ret = vp_set_oforward(drv, fd, service_id);
+if (ret != 0) {
+fprintf(stderr, "error adding iforward");
+return -1;
+}
+
+return 0;
+}
+
+static int vp_init_iforward(VPDriver *drv, QemuOpts *opts)
+{
+const char *service_id, *addr, *port;
+bool ipv6;
+int ret;
+
+service_id = qemu_opt_get(opts, "service_id");
+if (service_id == NULL) {
+fprintf(stderr, "no service_id specified");
+return -1;
+}
+addr = qemu_opt_get(opts, "path");
+port = NULL;
+if (addr == NULL) {
+/* map service to a network socket instead */
+addr = qemu_opt_get(opts, "host");
+port = qemu_opt_get(opts, "port");
+}
+
+ipv6 = qemu_opt_get_bool(opts, "ipv6", 0) ?
+   true : false;
+
+ret = vp_set_iforward(drv, service_id, addr, port, ipv6);
+if (ret != 0) {
+fprintf(stderr, "error adding iforward");
+return -1;
+}
+
+return 0;
+}
+
+static int vp_init_forwards(const char *name, const char *value, void *opaque)
+{
+QemuOpts *opts = qemu_opts_create(&vp_opts, NULL, 0);
+VPDriver *drv = opaque;
+int ret;
+
+if (strcmp(name, "oforward") != 0 && strcmp(name, "iforward") != 0) {
+return 0;
+}
+
+/* parse opt string into QemuOpts */
+ret = vp_parse(opts, value, 0);
+if (ret < 0) {
+fprintf(stderr, "error parsing virtproxy arguments");
+return -1;
+}
+
+if (strcmp(name, "oforward") == 0) {
+return vp_init_oforward(drv, opts);
+} else if (strcmp(name, "iforward")) {
+return vp_init_iforward(drv, opts);
+}
+
+return -1;
+}
+
+static int vp_chr_write(CharDriverState *chr, const uint8_t *buf, int len)
+{
+VPDriver *drv = chr->opaque;
+int ret;
+
+fprintf(stderr, "called, opaque: %p\n", chr);
+
+ret = vp_handle_packet_buf(drv, buf, len);
+if (ret == -1) {
+fprintf(stderr, "error handling data from virtproxy channel");
+}
+
+return 0;
+}
+
+static CharDriverState *qemu_chr_open_virtproxy(QemuOpts *opts)
+{
+CharDriverState *chr = qemu_mallocz(sizeof(CharDriverState));
+VPDriver *drv = vp_new(VP_CTX_CHARDEV, chr, 0, 0);
+
+chr->opaque = drv;
+chr->chr_write = vp_chr_write;
+
+qemu_chr_generic_open(chr);
+
+/* parse socket forwarding options */
+qemu_opt_foreach(opts, vp_init_forwards, drv, 1);
+
+/* for "info chardev" monitor command */
+chr->filename = NULL;
+return chr;
+}
+
+/***/
 /* TCP Net console */
 
 typedef struct {
@@ -2408,6 +2537,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 = "virtproxy", .open = qemu_chr_open_virtproxy },
 #ifdef _WIN32
 { .name = "file",  .open = qemu_chr_open_win_file_out },
 { .name = "pipe",  .open = qemu_chr_open_win_pipe },
diff --git a/qemu-config.c b/qemu-config.c
index 52f18be..400e61a 100644
--- a/qemu-config.c
+++ b/qemu-config.c
@@ -146,6 +146,12 @@ static QemuOptsList qemu_chardev_opts = {
 },{
 .name = "signal",
 .type = QEMU_OPT_BOOL,
+},{
+.name = "oforward",
+.type = QEMU_OPT_STRING,
+},{
+

[Qemu-devel] [RFC][PATCH v3 17/21] virtproxy: add virtproxy-builtin.c for compat defs

Virtproxy relies on routines defined within qemu-vp which mirror various
i/o related operations in qemu to provide similar functionality for the
guest daemon. When building virtproxy as part of qemu rather than
qemu-vp we need these definitions to provide those functions in terms of
the original qemu functions.

Signed-off-by: Michael Roth 
---
 virtproxy-builtin.c |   38 ++
 1 files changed, 38 insertions(+), 0 deletions(-)
 create mode 100644 virtproxy-builtin.c

diff --git a/virtproxy-builtin.c b/virtproxy-builtin.c
new file mode 100644
index 000..71fc5bc
--- /dev/null
+++ b/virtproxy-builtin.c
@@ -0,0 +1,38 @@
+/*
+ * virt-proxy - host/guest communication layer builtin definitions
+ *
+ * Copyright IBM Corp. 2010
+ *
+ * Authors:
+ *  Adam Litke
+ *  Michael Roth  
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ *
+ */
+
+/* the following are functions we define in terms of qemu when linked
+ * against qemu/vl.c. these will be added on an as-needed basis
+ */
+
+#include "qemu-char.h"
+#include "qemu_socket.h"
+#include "virtproxy.h"
+
+int vp_set_fd_handler(int fd,
+IOHandler *fd_read,
+IOHandler *fd_write,
+void *opaque)
+{
+return qemu_set_fd_handler(fd, fd_read, fd_write, opaque);
+}
+
+int vp_send_all(int fd, const void *buf, int len)
+{
+return send_all(fd, buf, len);
+}
+
+void vp_chr_read(CharDriverState *s, uint8_t *buf, int len) {
+return qemu_chr_read(s, buf, len);
+}
-- 
1.7.0.4

[Qemu-devel] [RFC][PATCH v3 20/21] virtproxy: qemu-vp, main logic


Signed-off-by: Michael Roth 
---
 qemu-vp.c   |  368 ++-
 virtproxy.c |   24 
 2 files changed, 367 insertions(+), 25 deletions(-)

diff --git a/qemu-vp.c b/qemu-vp.c
index 5075cdc..cfd2a69 100644
--- a/qemu-vp.c
+++ b/qemu-vp.c
@@ -9,10 +9,54 @@
  * 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 
+#include 
+#include "qemu-option.h"
+#include "qemu_socket.h"
 #include "virtproxy.h"
 
+static bool verbose_enabled = 0;
+#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-related code for standalone daemon */
 typedef struct IOHandlerRecord {
 int fd;
@@ -92,13 +136,19 @@ int vp_send_all(int fd, const void *buf, int len1)
 return len1 - len;
 }
 
+/* qemu_chr_read doesn't get used in guest so noop our wrapper function */
+void vp_chr_read(CharDriverState *s, uint8_t *buf, int len)
+{
+return;
+}
+
 static void main_loop_wait(int nonblocking)
 {
 IOHandlerRecord *ioh;
 fd_set rfds, wfds, xfds;
 int ret, nfds;
 struct timeval tv;
-int timeout = 1000;
+int timeout = 10;
 
 if (nonblocking) {
 timeout = 0;
@@ -149,3 +199,319 @@ static void main_loop_wait(int nonblocking)
 }
 }
 }
+
+#define VP_ARG_LEN 256
+
+typedef struct VPData {
+QemuOpts *opts;
+void *opaque;
+QTAILQ_ENTRY(VPData) next;
+} VPData;
+
+static QTAILQ_HEAD(, VPData) iforwards;
+static QTAILQ_HEAD(, VPData) oforwards;
+static QTAILQ_HEAD(, VPData) channels;
+
+static void usage(const char *cmd)
+{
+printf(
+"Usage: %s -c  [-c ... ] [-i  ...] "
+"[-o  ...]\n"
+"QEMU virt-proxy communication channel\n"
+"\n"
+"  -c, --channelchannel options of the form:\n"
+"   ::[:channel_id]\n"
+"  -o, --oforward   oforward options of the form:\n"
+"   ::[:channel_id]\n"
+"  -i, --iforward   iforward options of the form:\n"
+"   ::[:channel_id]\n"
+"  -v, --verbosedisplay extra debugging information\n"
+"  -h, --help   display this help and exit\n"
+"\n"
+"  channels are used to establish a data connection between 2 end-points in\n"
+"  the host or the guest (connection method specified by ).\n"
+"  oforwards specify a socket to listen for new connections on, outgoing\n"
+"  data from which is tagged with  before being sent over the\n"
+"  channel. iforwards specify a socket to route incoming data/connections\n"
+"  with a specific  to. The positional parameters for\n"
+"  channels/iforwards/oforwards are:\n"
+"\n"
+"  : one of unix-connect, unix-listen, tcp-connect, tcp-listen,\n"
+"virtserial-open\n"
+"  :   path of unix socket or virtserial port, or IP of host, to\n"
+"connect/bind to\n"
+"  :   port to bind/connect to, or '-' if addr is a path\n"
+"  : an identifier used to properly route connections to the\n"
+"corresponding host or guest daemon socket.\n"
+"  : numerical id to identify what channel to use for an 
iforward\n"
+"or oforward. (default is 0)\n"
+"\n"
+"Report bugs to \n"
+, cmd);
+}
+
+static VPDriver *get_channel_drv(int index) {
+VPData *data;
+VPDriver *drv;
+int cindex;
+
+QTAILQ_FOREACH(data, &channels, next) {
+cindex = qemu_opt_get_number(data->opts, "index", 0);
+if (cindex == index) {
+drv = data->opaque;
+

[Qemu-devel] [RFC][PATCH v3 16/21] virtproxy: add option parser helper vp_parse()

This routine will parse iforward/oforward options for both qemu-vp and
the virtproxy chardev

Signed-off-by: Michael Roth 
---
 virtproxy.c |   77 +++
 virtproxy.h |1 +
 2 files changed, 78 insertions(+), 0 deletions(-)

diff --git a/virtproxy.c b/virtproxy.c
index f1f6859..78cda04 100644
--- a/virtproxy.c
+++ b/virtproxy.c
@@ -895,3 +895,80 @@ int vp_set_iforward(VPDriver *drv, const char *service_id, 
const char *addr,
 
 return 0;
 }
+
+/* utility function to parse iforward/oforward options for qemu-vp
+ * or virtproxy chardev and put them into QemuOpts
+ */
+int vp_parse(QemuOpts *opts, const char *str, bool is_channel)
+{
+/* TODO: use VP_SERVICE_ID_LEN, bring it into virtproxy.h */
+char service_id[32];
+char channel_method[32];
+char index[10];
+char *addr;
+char port[33];
+int pos, ret;
+
+if (is_channel == false) {
+/* parse service id */
+ret = sscanf(str,"%32[^:]:%n",service_id,&pos);
+if (ret != 1) {
+LOG("error parsing service id");
+return -1;
+}
+qemu_opt_set(opts, "service_id", service_id);
+} else {
+/* parse connection type */
+ret = sscanf(str,"%32[^:]:%n",channel_method,&pos);
+if (ret != 1) {
+LOG("error parsing channel method");
+return -1;
+}
+qemu_opt_set(opts, "channel_method", channel_method);
+}
+str += pos;
+pos = 0;
+
+/* parse path/addr and port */
+if (str[0] == '[') {
+/* ipv6 formatted */
+ret = sscanf(str,"[%a[^]:]]:%32[^:]%n",&addr,port,&pos);
+qemu_opt_set(opts, "ipv6", "on");
+} else {
+ret = sscanf(str,"%a[^:]:%32[^:]%n",&addr,port,&pos);
+qemu_opt_set(opts, "ipv4", "on");
+}
+
+if (ret != 2) {
+LOG("error parsing path/addr/port");
+return -1;
+} else if (port[0] == '-') {
+/* no port given, assume unix path */
+qemu_opt_set(opts, "path", addr);
+} else {
+qemu_opt_set(opts, "host", addr);
+qemu_opt_set(opts, "port", port);
+qemu_free(addr);
+}
+str += pos;
+pos = 0;
+
+if (str[0] == ':') {
+/* parse optional index parameter */
+ret = sscanf(str,":%10[^:]%n",index,&pos);
+} else {
+qemu_opt_set(opts, "index", "0");
+return 0;
+}
+
+if (ret != 1) {
+LOG("error parsing index");
+return -1;
+} else {
+qemu_opt_set(opts, "index", index);
+}
+str += pos;
+pos = 0;
+
+return 0;
+}
diff --git a/virtproxy.h b/virtproxy.h
index ef168b0..6a53487 100644
--- a/virtproxy.h
+++ b/virtproxy.h
@@ -44,5 +44,6 @@ int vp_handle_packet_buf(VPDriver *drv, const void *buf, int 
count);
 int vp_set_oforward(VPDriver *drv, int fd, const char *service_id);
 int vp_set_iforward(VPDriver *drv, const char *service_id, const char *addr,
 const char *port, bool ipv6);
+int vp_parse(QemuOpts *opts, const char *str, bool is_channel);
 
 #endif /* VIRTPROXY_H */
-- 
1.7.0.4

[Qemu-devel] [RFC][PATCH v3 11/21] virtproxy: add handler for control packet

Process control packets coming in over the channel. This entails setting
up/tearing down connections to local services initiated from the other
end of the channel.

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

diff --git a/virtproxy.c b/virtproxy.c
index a0bbe7f..0cc8950 100644
--- a/virtproxy.c
+++ b/virtproxy.c
@@ -268,6 +268,160 @@ static void vp_channel_accept(void *opaque)
 vp_set_fd_handler(drv->listen_fd, NULL, NULL, NULL);
 }
 
+/* handle control packets
+ *
+ * process VPPackets containing control messages
+ */
+static int vp_handle_control_packet(VPDriver *drv, const VPPacket *pkt)
+{
+const VPControlMsg *msg = &pkt->payload.msg;
+int ret;
+
+TRACE("called with drv: %p", drv);
+
+switch (msg->type) {
+case VP_CONTROL_CONNECT_INIT: {
+int client_fd = msg->args.connect_init.client_fd;
+int server_fd;
+char service_id[VP_SERVICE_ID_LEN];
+VPPacket resp_pkt;
+VPConn *new_conn;
+VPIForward *iforward;
+
+pstrcpy(service_id, VP_SERVICE_ID_LEN,
+ msg->args.connect_init.service_id);
+TRACE("setting up connection for service id %s", service_id);
+
+/* create server connection on behalf of remote end */
+iforward = get_iforward(drv, service_id);
+if (iforward == NULL) {
+LOG("no forwarder configured for service id");
+return -1;
+}
+
+qemu_opts_print(iforward->socket_opts, NULL);
+if (qemu_opt_get(iforward->socket_opts, "host") != NULL) {
+server_fd = inet_connect_opts(iforward->socket_opts);
+} else if (qemu_opt_get(iforward->socket_opts, "path") != NULL) {
+server_fd = unix_connect_opts(iforward->socket_opts);
+} else {
+LOG("unable to find listening socket host/addr info");
+return -1;
+}
+
+if (server_fd == -1) {
+LOG("failed to create connection to service with id %s",
+service_id);
+}
+TRACE("server_fd: %d", server_fd);
+
+new_conn = qemu_mallocz(sizeof(VPConn));
+if (!new_conn) {
+LOG("memory allocation failed");
+return -1;
+}
+
+/* send a connect_ack back over the channel */
+/* TODO: all fields should be explicitly set so we shouldn't
+ * need to memset. this might hurt if we beef up VPPacket size
+ */
+memset(&resp_pkt, 0, sizeof(resp_pkt));
+resp_pkt.type = VP_PKT_CONTROL;
+resp_pkt.payload.msg.type = VP_CONTROL_CONNECT_ACK;
+resp_pkt.payload.msg.args.connect_ack.server_fd = server_fd;
+resp_pkt.payload.msg.args.connect_ack.client_fd = client_fd;
+resp_pkt.magic = VP_MAGIC;
+
+/* TODO: can this potentially block or cause a deadlock with
+ * the remote end? need to look into potentially buffering these
+ * if it looks like the remote end is waiting for us to read data
+ * off the channel.
+ */
+if (!drv->chr && drv->channel_fd == -1) {
+TRACE("channel no longer connected, ignoring packet");
+return -1;
+}
+
+ret = vp_channel_send_all(drv, (void *)&resp_pkt, sizeof(resp_pkt));
+if (ret == -1) {
+LOG("error sending data over channel");
+return -1;
+}
+if (ret != sizeof(resp_pkt)) {
+TRACE("buffer full? %d bytes remaining", ret);
+return -1;
+}
+
+/* add new VPConn to list and set a read handler for it */
+new_conn->drv = drv;
+new_conn->client_fd = client_fd;
+new_conn->server_fd = server_fd;
+new_conn->type = VP_CONN_SERVER;
+new_conn->state = VP_STATE_CONNECTED;
+QLIST_INSERT_HEAD(&drv->conns, new_conn, next);
+vp_set_fd_handler(server_fd, vp_conn_read, NULL, new_conn);
+
+break;
+}
+case VP_CONTROL_CONNECT_ACK: {
+int client_fd = msg->args.connect_ack.client_fd;
+int server_fd = msg->args.connect_ack.server_fd;
+VPConn *conn;
+
+TRACE("recieved ack from remote end for client fd %d", client_fd);
+
+if (server_fd <= 0) {
+LOG("remote end sent invalid server fd");
+return -1;
+}
+
+conn = get_conn(drv, client_fd, true);
+
+if (conn == NULL) {
+LOG("failed to find connection with client_fd %d", client_fd);
+return -1;
+}
+
+conn->server_fd = server_fd;
+conn->state = VP_STATE_CONNECTED;
+vp_set_fd_handler(client_fd, vp_conn_read, NULL, conn);
+
+break;
+}
+case VP_CONTROL_CLOSE: {
+int fd;
+VPConn *conn;
+
+TRACE("closing connection on behalf of remote end");
+
+if (msg->args.close.client_fd >= 0) {
+fd = msg->args.cl

[Qemu-devel] [RFC][PATCH v3 19/21] virtproxy: qemu integration, add virtproxy to Makefile.targets


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

diff --git a/Makefile.target b/Makefile.target
index 91e6e74..f08c435 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -164,7 +164,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 virtproxy.o 
virtproxy-builtin.o
 # virtio has to be here due to weird dependency between PCI and virtio-net.
 # need to fix this properly
 obj-y += virtio-blk.o virtio-balloon.o virtio-net.o virtio-serial-bus.o
-- 
1.7.0.4

[Qemu-devel] [RFC][PATCH v3 07/21] virtproxy: add read handler for communication channel

Handle data coming in over the channel as VPPackets: Process control
messages and forward data from remote client/server connections to the
appropriate server/client FD on our end. We also provide here a helper
function to process a stream of packets from the channel.

Signed-off-by: Michael Roth 
---
 virtproxy.c |   96 +++
 virtproxy.h |3 ++
 2 files changed, 99 insertions(+), 0 deletions(-)

diff --git a/virtproxy.c b/virtproxy.c
index 770b57b..091a223 100644
--- a/virtproxy.c
+++ b/virtproxy.c
@@ -33,6 +33,7 @@
 #define VP_SERVICE_ID_LEN 32/* max length of service id string */
 #define VP_PKT_DATA_LEN 1024/* max proxied bytes per VPPacket */
 #define VP_CONN_DATA_LEN 1024   /* max bytes conns can send at a time */
+#define VP_CHAN_DATA_LEN 4096   /* max bytes channel can send at a time */
 #define VP_MAGIC 0x1F374059
 
 /* listening fd, one for each service we're forwarding to remote end */
@@ -152,6 +153,8 @@ static QemuOptsList vp_socket_opts = {
 },
 };
 
+static void vp_channel_read(void *opaque);
+
 static int vp_channel_send_all(VPDriver *drv, uint8_t *buf, int count)
 {
 int ret;
@@ -263,3 +266,96 @@ static void vp_channel_accept(void *opaque)
 /* dont accept anymore connections until channel_fd is closed */
 vp_set_fd_handler(drv->listen_fd, NULL, NULL, NULL);
 }
+
+/* process a stream of packets coming in from the channel */
+int vp_handle_packet_buf(VPDriver *drv, const void *buf, int count)
+{
+VPPacket pkt;
+int ret, buf_offset;
+char *pkt_ptr;
+const char *buf_ptr;
+
+if (drv->buflen + count >= sizeof(VPPacket)) {
+TRACE("initial packet, drv->buflen: %d", drv->buflen);
+pkt_ptr = (char *)&pkt;
+memcpy(pkt_ptr, drv->buf, drv->buflen);
+pkt_ptr += drv->buflen;
+memcpy(pkt_ptr, buf, sizeof(VPPacket) - drv->buflen);
+/* handle first packet */
+ret = vp_handle_packet(drv, &pkt);
+if (ret != 0) {
+LOG("error handling packet");
+}
+/* handle the rest of the buffer */
+buf_offset = sizeof(VPPacket) - drv->buflen;
+drv->buflen = 0;
+buf_ptr = buf + buf_offset;
+count -= buf_offset;
+while (count > 0) {
+if (count >= sizeof(VPPacket)) {
+/* handle full packet */
+TRACE("additional packet, drv->buflen: %d", drv->buflen);
+memcpy((void *)&pkt, buf_ptr, sizeof(VPPacket));
+ret = vp_handle_packet(drv, &pkt);
+if (ret != 0) {
+LOG("error handling packet");
+}
+count -= sizeof(VPPacket);
+buf_ptr += sizeof(VPPacket);
+} else {
+/* buffer the remainder */
+TRACE("buffering packet, drv->buflen: %d", drv->buflen);
+memcpy(drv->buf, buf_ptr, count);
+drv->buflen = count;
+break;
+}
+}
+} else {
+/* haven't got a full VPPacket yet, buffer for later */
+TRACE("buffering packet, drv->buflen: %d", drv->buflen);
+memcpy(drv->buf + drv->buflen, buf, count);
+drv->buflen += count;
+}
+return 0;
+}
+
+/* read handler for communication channel
+ *
+ * de-multiplexes data coming in over the channel. for control messages
+ * we process them here, for data destined for a service or client we
+ * send it to the appropriate FD.
+ */
+static void vp_channel_read(void *opaque)
+{
+VPDriver *drv = opaque;
+int count, ret;
+char buf[VP_CHAN_DATA_LEN];
+
+TRACE("called with opaque: %p", drv);
+
+count = read(drv->channel_fd, buf, sizeof(buf));
+
+if (count == -1) {
+LOG("read() failed: %s", strerror(errno));
+return;
+} else if (count == 0) {
+/* TODO: channel closed, this probably shouldn't happen for guest-side
+ * serial/virtio-serial connections, but need to confirm and consider
+ * what should happen in this case. as it stands this virtproxy 
instance
+ * is basically defunct at this point, same goes for "client" instances
+ * of virtproxy where the remote end has hung-up.
+ */
+LOG("channel connection closed");
+vp_set_fd_handler(drv->channel_fd, NULL, NULL, drv);
+drv->channel_fd = -1;
+if (drv->listen_fd) {
+vp_set_fd_handler(drv->listen_fd, vp_channel_accept, NULL, drv);
+}
+/* TODO: should close/remove/delete all existing VPConns here */
+}
+
+ret = vp_handle_packet_buf(drv, buf, count);
+if (ret != 0) {
+LOG("error handling packet stream");
+}
+}
diff --git a/virtproxy.h b/virtproxy.h
index 1a5e56a..8fa0142 100644
--- a/virtproxy.h
+++ b/virtproxy.h
@@ -32,4 +32,7 @@ int vp_set_fd_handler(int fd,
 void *opaque);
 void vp_chr_read(CharDriverState *s, uint8_t *buf, int len);
 
+/*

[Qemu-devel] [RFC][PATCH v3 14/21] virtproxy: use new option list in virtproxy.c


Signed-off-by: Michael Roth 
---
 virtproxy.c |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/virtproxy.c b/virtproxy.c
index b683ee6..9e1745f 100644
--- a/virtproxy.c
+++ b/virtproxy.c
@@ -764,7 +764,7 @@ int vp_set_iforward(VPDriver *drv, const char *service_id, 
const char *addr,
 /* stick socket-related options in a QemuOpts so we can
  * utilize qemu socket utility functions directly
  */
-f->socket_opts = qemu_opts_create(&vp_socket_opts, NULL, 0);
+f->socket_opts = qemu_opts_create(&vp_opts, NULL, 0);
 if (port == NULL) {
 /* no port given, assume unix path */
 qemu_opt_set(f->socket_opts, "path", addr);
-- 
1.7.0.4

[Qemu-devel] [RFC][PATCH v3 06/21] virtproxy: add accept handler for communication channel

This accept()'s connections to the socket we told virtproxy to listen
for the channel connection on and sets the appropriate read handler for
the resulting FD. This is only used only for network-based channels and
will most likely be dropped with the introduction of the virtproxy
chardev.

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

diff --git a/virtproxy.c b/virtproxy.c
index edca62e..770b57b 100644
--- a/virtproxy.c
+++ b/virtproxy.c
@@ -199,6 +199,8 @@ static VPConn *get_conn(const VPDriver *drv, int fd, bool 
client)
 return NULL;
 }
 
+static void vp_channel_accept(void *opaque);
+
 /* get VPOForward by service_id */
 static VPOForward *get_oforward(const VPDriver *drv, const char *service_id)
 {
@@ -226,3 +228,38 @@ static VPIForward *get_iforward(const VPDriver *drv, const 
char *service_id)
 
 return NULL;
 }
+
+/* accept handler for communication channel
+ *
+ * accept()s connection to communication channel (for sockets), and sets
+ * up the read handler for resulting FD.
+ */
+static void vp_channel_accept(void *opaque)
+{
+VPDriver *drv = opaque;
+struct sockaddr_in saddr;
+struct sockaddr *addr;
+socklen_t len;
+int fd;
+
+TRACE("called with opaque: %p", drv);
+
+for(;;) {
+len = sizeof(saddr);
+addr = (struct sockaddr *)&saddr;
+fd = qemu_accept(drv->listen_fd, addr, &len);
+
+if (fd < 0 && errno != EINTR) {
+TRACE("accept() failed");
+return;
+} else if (fd >= 0) {
+TRACE("accepted connection");
+break;
+}
+}
+
+drv->channel_fd = fd;
+vp_set_fd_handler(drv->channel_fd, vp_channel_read, NULL, drv);
+/* dont accept anymore connections until channel_fd is closed */
+vp_set_fd_handler(drv->listen_fd, NULL, NULL, NULL);
+}
-- 
1.7.0.4

[Qemu-devel] [RFC][PATCH v3 12/21] virtproxy: add vp_handle_packet()


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

diff --git a/virtproxy.c b/virtproxy.c
index 0cc8950..01a36c2 100644
--- a/virtproxy.c
+++ b/virtproxy.c
@@ -464,6 +464,29 @@ static int vp_handle_data_packet(void *drv, const VPPacket 
*pkt)
 return 0;
 }
 
+static inline int vp_handle_packet(VPDriver *drv, const VPPacket *pkt)
+{
+int ret;
+
+TRACE("called with drv: %p", drv);
+
+if (pkt->magic != VP_MAGIC) {
+LOG("invalid packet magic field");
+return -1;
+}
+
+if (pkt->type == VP_PKT_CONTROL) {
+ret = vp_handle_control_packet(drv, pkt);
+} else if (pkt->type == VP_PKT_CLIENT || pkt->type == VP_PKT_SERVER) {
+ret = vp_handle_data_packet(drv, pkt);
+} else {
+LOG("invalid packet type");
+return -1;
+}
+
+return ret;
+}
+
 /* process a stream of packets coming in from the channel */
 int vp_handle_packet_buf(VPDriver *drv, const void *buf, int count)
 {
-- 
1.7.0.4

[Qemu-devel] [RFC][PATCH v3 13/21] virtproxy: interfaces to set/remove VPIForwards


Signed-off-by: Michael Roth 
---
 virtproxy.c |   59 +++
 virtproxy.h |2 ++
 2 files changed, 61 insertions(+), 0 deletions(-)

diff --git a/virtproxy.c b/virtproxy.c
index 01a36c2..b683ee6 100644
--- a/virtproxy.c
+++ b/virtproxy.c
@@ -722,3 +722,62 @@ int vp_set_oforward(VPDriver *drv, int fd, const char 
*service_id)
 
 return 0;
 }
+
+/* add/modify a service_id -> net/unix socket mapping
+ *
+ * "service_id" is a user-defined id for the service. this is what the
+ * remote end will use to proxy connections to a specific service on
+ * our end.
+ *
+ * if "port" is NULL, "addr" is the address of the net socket the
+ * service is running on. otherwise, addr is the path to the unix socket
+ * the service is running on.
+ *
+ * if "port" AND "addr" are NULL, find and remove the current iforward
+ * for this "service_id" if it exists.
+ *
+ * "ipv6" is a bool denoting whether or not to use ipv6
+ */
+int vp_set_iforward(VPDriver *drv, const char *service_id, const char *addr,
+const char *port, bool ipv6)
+{
+VPIForward *f = get_iforward(drv, service_id);
+
+if (addr == NULL && port == NULL) {
+if (f != NULL) {
+qemu_opts_del(f->socket_opts);
+QLIST_REMOVE(f, next);
+qemu_free(f);
+}
+return 0;
+}
+
+if (f == NULL) {
+f = qemu_mallocz(sizeof(VPIForward));
+f->drv = drv;
+strncpy(f->service_id, service_id, VP_SERVICE_ID_LEN);
+QLIST_INSERT_HEAD(&drv->iforwards, f, next);
+} else {
+qemu_opts_del(f->socket_opts);
+}
+
+/* stick socket-related options in a QemuOpts so we can
+ * utilize qemu socket utility functions directly
+ */
+f->socket_opts = qemu_opts_create(&vp_socket_opts, NULL, 0);
+if (port == NULL) {
+/* no port given, assume unix path */
+qemu_opt_set(f->socket_opts, "path", addr);
+} else {
+qemu_opt_set(f->socket_opts, "host", addr);
+qemu_opt_set(f->socket_opts, "port", port);
+}
+
+if (ipv6) {
+qemu_opt_set(f->socket_opts, "ipv6", "on");
+} else {
+qemu_opt_set(f->socket_opts, "ipv4", "on");
+}
+
+return 0;
+}
diff --git a/virtproxy.h b/virtproxy.h
index e901600..83cc905 100644
--- a/virtproxy.h
+++ b/virtproxy.h
@@ -41,5 +41,7 @@ void vp_chr_read(CharDriverState *s, uint8_t *buf, int len);
 VPDriver *vp_new(enum vp_context ctx, CharDriverState *s, int fd, bool listen);
 int vp_handle_packet_buf(VPDriver *drv, const void *buf, int count);
 int vp_set_oforward(VPDriver *drv, int fd, const char *service_id);
+int vp_set_iforward(VPDriver *drv, const char *service_id, const char *addr,
+const char *port, bool ipv6);
 
 #endif /* VIRTPROXY_H */
-- 
1.7.0.4

[Qemu-devel] [RFC][PATCH v3 02/21] virtproxy: qemu-vp, standalone daemon skeleton

Daemon to be run in guest, or on host in standalone mode.
(re-)implements some qemu utility functions used by core virtproxy.c
code via wrapper functions. For built-in virtproxy code we will define
these wrapper functions in terms of qemu's built-in implementations.

Main logic will come in a later patch.

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

diff --git a/qemu-vp.c b/qemu-vp.c
new file mode 100644
index 000..5075cdc
--- /dev/null
+++ b/qemu-vp.c
@@ -0,0 +1,151 @@
+/*
+ * virt-proxy - host/guest communication daemon
+ *
+ * Copyright IBM Corp. 2010
+ *
+ * Authors:
+ *  Michael Roth  
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ *
+ */
+
+#include "virtproxy.h"
+
+/* mirror qemu I/O-related code for standalone daemon */
+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);
+
+int vp_set_fd_handler2(int fd,
+ IOCanReadHandler *fd_read_poll,
+ IOHandler *fd_read,
+ 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;
+}
+
+int vp_set_fd_handler(int fd,
+IOHandler *fd_read,
+IOHandler *fd_write,
+void *opaque)
+{
+return vp_set_fd_handler2(fd, NULL, fd_read, fd_write, opaque);
+}
+
+int vp_send_all(int fd, const void *buf, int len1)
+{
+int ret, len;
+
+len = len1;
+while (len > 0) {
+ret = write(fd, buf, len);
+if (ret < 0) {
+if (errno != EINTR && errno != EAGAIN) {
+warn("write() failed");
+return -1;
+}
+} else if (ret == 0) {
+break;
+} else {
+buf += ret;
+len -= ret;
+}
+}
+return len1 - len;
+}
+
+static void main_loop_wait(int nonblocking)
+{
+IOHandlerRecord *ioh;
+fd_set rfds, wfds, xfds;
+int ret, nfds;
+struct timeval tv;
+int timeout = 1000;
+
+if (nonblocking) {
+timeout = 0;
+}
+
+/* poll any events */
+nfds = -1;
+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;
+}
+}
+
+tv.tv_sec = timeout / 1000;
+tv.tv_usec = (timeout % 1000) * 1000;
+
+ret = select(nfds + 1, &rfds, &wfds, &xfds, &tv);
+
+if (ret > 0) {
+IOHandlerRecord *pioh;
+
+QLIST_FOREACH_SAFE(ioh, &io_handlers, next, pioh) {
+if (ioh->deleted) {
+QLIST_REMOVE(ioh, next);
+qemu_free(ioh);
+continue;
+}
+if (ioh->fd_read && FD_ISSET(ioh->fd, &rfds)) {
+ioh->fd_read(ioh->opaque);
+}
+if (ioh->fd_write && FD_ISSET(ioh->fd, &wfds)) {
+ioh->fd_write(ioh->opaque);
+}
+}
+}
+}
-- 
1.7.0.4

[Qemu-devel] [RFC][PATCH v3 10/21] virtproxy: add handler for data packets

Process VPPackets coming in from channel and send them to the
appropriate server/client connections.

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

diff --git a/virtproxy.c b/virtproxy.c
index f189e2c..a0bbe7f 100644
--- a/virtproxy.c
+++ b/virtproxy.c
@@ -268,6 +268,48 @@ static void vp_channel_accept(void *opaque)
 vp_set_fd_handler(drv->listen_fd, NULL, NULL, NULL);
 }
 
+/* handle data packets
+ *
+ * process VPPackets containing data and send them to the corresponding
+ * FDs
+ */
+static int vp_handle_data_packet(void *drv, const VPPacket *pkt)
+{
+int fd, ret;
+
+TRACE("called with drv: %p", drv);
+
+if (pkt->type == VP_PKT_CLIENT) {
+TRACE("recieved client packet, client fd: %d, server fd: %d",
+  pkt->payload.proxied.client_fd, pkt->payload.proxied.server_fd);
+fd = pkt->payload.proxied.server_fd;
+} else if (pkt->type == VP_PKT_SERVER) {
+TRACE("recieved server packet, client fd: %d, server fd: %d",
+  pkt->payload.proxied.client_fd, pkt->payload.proxied.server_fd);
+fd = pkt->payload.proxied.client_fd;
+} else {
+TRACE("unknown packet type");
+return -1;
+}
+
+/* TODO: proxied in non-blocking mode can causes us to spin here
+ * for slow servers/clients. need to use write()'s and maintain
+ * a per-conn write queue that we clear out before sending any
+ * more data to the fd
+ */
+ret = vp_send_all(fd, (void *)pkt->payload.proxied.data,
+pkt->payload.proxied.bytes);
+if (ret == -1) {
+LOG("error sending data over channel");
+return -1;
+} else if (ret != pkt->payload.proxied.bytes) {
+TRACE("buffer full?");
+return -1;
+}
+
+return 0;
+}
+
 /* process a stream of packets coming in from the channel */
 int vp_handle_packet_buf(VPDriver *drv, const void *buf, int count)
 {
-- 
1.7.0.4

[Qemu-devel] [RFC][PATCH v3 01/21] virtproxy: base data structures and constants


Signed-off-by: Michael Roth 
---
 virtproxy.c |  136 +++
 virtproxy.h |   34 +++
 2 files changed, 170 insertions(+), 0 deletions(-)
 create mode 100644 virtproxy.c
 create mode 100644 virtproxy.h

diff --git a/virtproxy.c b/virtproxy.c
new file mode 100644
index 000..8f18d83
--- /dev/null
+++ b/virtproxy.c
@@ -0,0 +1,136 @@
+/*
+ * virt-proxy - host/guest communication layer
+ *
+ * Copyright IBM Corp. 2010
+ *
+ * Authors:
+ *  Michael Roth  
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ *
+ */
+
+#include "virtproxy.h"
+
+#define VP_SERVICE_ID_LEN 32/* max length of service id string */
+#define VP_PKT_DATA_LEN 1024/* max proxied bytes per VPPacket */
+#define VP_CONN_DATA_LEN 1024   /* max bytes conns can send at a time */
+#define VP_MAGIC 0x1F374059
+
+/* listening fd, one for each service we're forwarding to remote end */
+typedef struct VPOForward {
+VPDriver *drv;
+int listen_fd;
+char service_id[VP_SERVICE_ID_LEN];
+QLIST_ENTRY(VPOForward) next;
+} VPOForward;
+
+/* service_id->path/port mapping of each service forwarded from remote end */
+typedef struct VPIForward {
+VPDriver *drv;
+char service_id[VP_SERVICE_ID_LEN];
+QemuOpts *socket_opts;
+QLIST_ENTRY(VPIForward) next;
+} VPIForward;
+
+/* proxied client/server connected states */
+typedef struct VPConn {
+VPDriver *drv;
+int client_fd;
+int server_fd;
+enum {
+VP_CONN_CLIENT = 1,
+VP_CONN_SERVER,
+} type;
+enum {
+VP_STATE_NEW = 1,   /* accept()'d and registered fd */
+VP_STATE_INIT,  /* sent init pkt to remote end, waiting for ack */
+VP_STATE_CONNECTED, /* client and server connected */
+} state;
+QLIST_ENTRY(VPConn) next;
+} VPConn;
+
+typedef struct VPControlMsg {
+enum {
+VP_CONTROL_CONNECT_INIT = 1,
+VP_CONTROL_CONNECT_ACK,
+VP_CONTROL_CLOSE,
+} type;
+union {
+/* tell remote end connect to server and map client_fd to it */
+struct {
+int client_fd;
+char service_id[VP_SERVICE_ID_LEN];
+} connect_init;
+/* tell remote end we've created the connection to the server,
+ * and give them the corresponding fd to use so we don't have
+ * to do a reverse lookup everytime
+ */
+struct {
+int client_fd;
+int server_fd;
+} connect_ack;
+/* tell remote end to close fd in question, presumably because
+ * connection was closed on our end
+ */
+struct {
+int client_fd;
+int server_fd;
+} close;
+} args;
+} VPControlMsg;
+
+typedef struct VPPacket {
+enum {
+VP_PKT_CONTROL = 1,
+VP_PKT_CLIENT,
+VP_PKT_SERVER,
+} type;
+union {
+VPControlMsg msg;
+struct {
+int client_fd;
+int server_fd;
+int bytes;
+char data[VP_PKT_DATA_LEN];
+} proxied;
+} payload;
+int magic;
+} __attribute__((__packed__)) VPPacket;
+
+struct VPDriver {
+enum vp_context ctx;
+int channel_fd;
+int listen_fd;
+CharDriverState *chr;
+char buf[sizeof(VPPacket)];
+int buflen;
+QLIST_HEAD(, VPOForward) oforwards;
+QLIST_HEAD(, VPIForward) iforwards;
+QLIST_HEAD(, VPConn) conns;
+};
+
+static QemuOptsList vp_socket_opts = {
+.name = "vp_socket_opts",
+.head = QTAILQ_HEAD_INITIALIZER(vp_socket_opts.head),
+.desc = {
+{
+.name = "path",
+.type = QEMU_OPT_STRING,
+},{
+.name = "host",
+.type = QEMU_OPT_STRING,
+},{
+.name = "port",
+.type = QEMU_OPT_STRING,
+},{
+.name = "ipv4",
+.type = QEMU_OPT_BOOL,
+},{
+.name = "ipv6",
+.type = QEMU_OPT_BOOL,
+},
+{ /* end if list */ }
+},
+};
diff --git a/virtproxy.h b/virtproxy.h
new file mode 100644
index 000..0203421
--- /dev/null
+++ b/virtproxy.h
@@ -0,0 +1,34 @@
+/*
+ * virt-proxy - host/guest communication layer
+ *
+ * Copyright IBM Corp. 2010
+ *
+ * Authors:
+ *  Michael Roth  
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ *
+ */
+
+#ifndef VIRTPROXY_H
+#define VIRTPROXY_H
+
+#include "qemu-common.h"
+#include "qemu-queue.h"
+
+typedef struct VPDriver VPDriver;
+
+/* wrappers for s/vp/qemu/ functions we need */
+int vp_send_all(int fd, const void *buf, int len1);
+int vp_set_fd_handler2(int fd,
+ IOCanReadHandler *fd_read_poll,
+ IOHandler *fd_read,
+ IOHandler *fd_write,
+ void *opaque);
+int vp_set_fd_handler(int

[Qemu-devel] [RFC][PATCH v3 03/21] virtproxy: add debug functions for virtproxy core


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

diff --git a/virtproxy.c b/virtproxy.c
index 8f18d83..3686c77 100644
--- a/virtproxy.c
+++ b/virtproxy.c
@@ -13,6 +13,23 @@
 
 #include "virtproxy.h"
 
+#define DEBUG_VP
+
+#ifdef DEBUG_VP
+#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 VP_SERVICE_ID_LEN 32/* max length of service id string */
 #define VP_PKT_DATA_LEN 1024/* max proxied bytes per VPPacket */
 #define VP_CONN_DATA_LEN 1024   /* max bytes conns can send at a time */
-- 
1.7.0.4

[Qemu-devel] [RFC][PATCH v3 09/21] virtproxy: interfaces to set/remove/handle VPOForwards

Functions to add listener FDs (oforwards) which set up proxied connections
to associated service, and the corresponding handler function to process
to new connections to said FDs and initialize new client connections to
the associated remote server over the channel

Signed-off-by: Michael Roth 
---
 virtproxy.c |  104 +++
 virtproxy.h |1 +
 2 files changed, 105 insertions(+), 0 deletions(-)

diff --git a/virtproxy.c b/virtproxy.c
index 401d51c..f189e2c 100644
--- a/virtproxy.c
+++ b/virtproxy.c
@@ -12,6 +12,7 @@
  */
 
 #include "virtproxy.h"
+#include "qemu_socket.h"
 
 #define DEBUG_VP
 
@@ -360,6 +361,71 @@ static void vp_channel_read(void *opaque)
 }
 }
 
+/* handler to accept() and init new client connections */
+static void vp_oforward_accept(void *opaque)
+{
+VPOForward *f = opaque;
+VPDriver *drv = f->drv;
+
+struct sockaddr_in saddr;
+struct sockaddr *addr;
+socklen_t len;
+int fd, ret;
+VPConn *conn = NULL;
+VPPacket pkt;
+VPControlMsg msg;
+
+TRACE("called with opaque: %p, drv: %p", f, drv);
+
+for(;;) {
+len = sizeof(saddr);
+addr = (struct sockaddr *)&saddr;
+fd = qemu_accept(f->listen_fd, addr, &len);
+
+if (fd < 0 && errno != EINTR) {
+TRACE("accept() failed");
+return;
+} else if (fd >= 0) {
+TRACE("accepted connection");
+break;
+}
+}
+
+if (!drv->chr && drv->channel_fd == -1) {
+TRACE("communication channel not open, closing connection");
+closesocket(fd);
+return;
+}
+
+/* send init packet over channel */
+memset(&msg, 0, sizeof(VPControlMsg));
+msg.type = VP_CONTROL_CONNECT_INIT;
+msg.args.connect_init.client_fd = fd;
+pstrcpy(msg.args.connect_init.service_id, VP_SERVICE_ID_LEN, 
f->service_id);
+
+memset(&pkt, 0, sizeof(VPPacket));
+pkt.type = VP_PKT_CONTROL;
+pkt.payload.msg = msg;
+pkt.magic = VP_MAGIC;
+
+ret = vp_channel_send_all(drv, (uint8_t*)&pkt, sizeof(VPPacket));
+if (ret == -1) {
+LOG("error sending to channel");
+return;
+}
+
+/* create new VPConn for client */
+conn = qemu_mallocz(sizeof(VPConn));
+conn->drv = drv;
+conn->client_fd = fd;
+conn->type = VP_CONN_CLIENT;
+conn->state = VP_STATE_NEW;
+QLIST_INSERT_HEAD(&drv->conns, conn, next);
+
+socket_set_nonblock(fd);
+}
+
+/* create/init VPDriver object */
 VPDriver *vp_new(enum vp_context ctx, CharDriverState *s, int fd, bool listen)
 {
 VPDriver *drv = NULL;
@@ -399,3 +465,41 @@ out_bad:
 qemu_free(drv);
 return NULL;
 }
+
+/* set/modify/remove a service_id -> net/unix listening socket mapping
+ *
+ * "service_id" is a user-defined id for the service. this is what the
+ * client end will tag it's connections with so that the remote end can
+ * route it to the proper socket on the remote end.
+ *
+ * "fd" is a listen()'ing socket we want virtproxy to listen for new
+ * connections of this service type on. set "fd" to -1 to remove the 
+ * existing listening socket for this "service_id"
+ */
+int vp_set_oforward(VPDriver *drv, int fd, const char *service_id)
+{
+VPOForward *f = get_oforward(drv, service_id);
+
+if (fd == -1) {
+if (f != NULL) {
+vp_set_fd_handler(f->listen_fd, NULL, NULL, NULL);
+QLIST_REMOVE(f, next);
+qemu_free(f);
+}
+return 0;
+}
+
+if (f == NULL) {
+f = qemu_mallocz(sizeof(VPOForward));
+f->drv = drv;
+strncpy(f->service_id, service_id, VP_SERVICE_ID_LEN);
+QLIST_INSERT_HEAD(&drv->oforwards, f, next);
+} else {
+closesocket(f->listen_fd);
+}
+
+f->listen_fd = fd;
+vp_set_fd_handler(f->listen_fd, vp_oforward_accept, NULL, f);
+
+return 0;
+}
diff --git a/virtproxy.h b/virtproxy.h
index 7cd2f76..e901600 100644
--- a/virtproxy.h
+++ b/virtproxy.h
@@ -40,5 +40,6 @@ void vp_chr_read(CharDriverState *s, uint8_t *buf, int len);
 /* virtproxy interface */
 VPDriver *vp_new(enum vp_context ctx, CharDriverState *s, int fd, bool listen);
 int vp_handle_packet_buf(VPDriver *drv, const void *buf, int count);
+int vp_set_oforward(VPDriver *drv, int fd, const char *service_id);
 
 #endif /* VIRTPROXY_H */
-- 
1.7.0.4

[Qemu-devel] [RFC][PATCH v3 04/21] virtproxy: list look-up functions conns/oforwards/iforwards


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

diff --git a/virtproxy.c b/virtproxy.c
index 3686c77..2cfd905 100644
--- a/virtproxy.c
+++ b/virtproxy.c
@@ -151,3 +151,47 @@ static QemuOptsList vp_socket_opts = {
 { /* end if list */ }
 },
 };
+
+/* get VPConn by fd, "client" denotes whether to look for client or server */
+static VPConn *get_conn(const VPDriver *drv, int fd, bool client)
+{
+VPConn *c = NULL;
+int cur_fd;
+
+QLIST_FOREACH(c, &drv->conns, next) {
+cur_fd = client ? c->client_fd : c->server_fd;
+if (cur_fd == fd) {
+return c;
+}
+}
+
+return NULL;
+}
+
+/* get VPOForward by service_id */
+static VPOForward *get_oforward(const VPDriver *drv, const char *service_id)
+{
+VPOForward *f = NULL;
+
+QLIST_FOREACH(f, &drv->oforwards, next) {
+if (strncmp(f->service_id, service_id, VP_SERVICE_ID_LEN) == 0) {
+return f;
+}
+}
+
+return NULL;
+}
+
+/* get VPIForward by service_id */
+static VPIForward *get_iforward(const VPDriver *drv, const char *service_id)
+{
+VPIForward *f = NULL;
+
+QLIST_FOREACH(f, &drv->iforwards, next) {
+if (strncmp(f->service_id, service_id, VP_SERVICE_ID_LEN) == 0) {
+return f;
+}
+}
+
+return NULL;
+}
-- 
1.7.0.4

[Qemu-devel] [RFC][PATCH v3 00/21] virtproxy: host/guest communication layer

OVERVIEW:

Virtproxy proxies and multiplexes network/unix socket streams over a data
channel between a host and a guest (currently network connections, emulated
serial, or virtio-serial channels are supported). This allows for services such
as guest data collection agents, host/guest file transfer, and event
generation/handling to be implemented/deployed as basic socket-based daemons,
independently of the actual data channel. This is especially useful in
environments where there is no network connectivity between the host and the
guest, allowing for standard services to be utilized without modification or
knowledge about the nature of the data connection.

This code is intended to provide a channel-independent abstraction layer for
communicating with a QEMU-specific guest agent (in particular, the virtagent
RPC guest agent which will follow this in a seperate patchset), but may have
general utility beyond this (for instance: ssh/sftp/other guest agents/etc over
isa/virtio serial), and so is submitted here as a seperate patchset.

It should be noted that the proposed guest agent, virtagent, is not
fundamentally dependent on this code, but by leveraging it we provide both the
benefits of a closely-integrated QEMU guest agent as well as the flexibility to
utilize other services in network-limited environments with this single guest
daemon.

CHANGES IN V3:
- virtproxy chardev options added to support specification of forwarding
ports/sockets (examples below). code base is testable again now.
- various cleanups

CHANGES IN V2:
- host daemon deprecated, virtproxy now integrated into qemu via a virtproxy
chardev. The command-line options have not yet been converted over so for now
the virtproxy layer is no longer testable directly. This will be addressed
soon. Virtagent will hook into this chardev via a boolean chardev option. That
patchset will follow this one shortly.
- deadlocking issue resolved

BUILD/USAGE INFO:
# build guest daemon
make qemu-vp
./qemu-vp -h

EXAMPLE USAGE:

note: oforward/iforward chardev options have not yet been converted over from
original standalone host daemon implementation so this won't work till then.
The examples however have been updated for reference.

- Proxy http and ssh connections from a host to a guest over a virtio-serial
connection:
# start guest with virtio-serial. for example (RHEL6s13):
qemu \
-device virtio-serial \
-chardev virtproxy,id=test0, \
oforward=http:127.0.0.1:9080,oforward=ssh:127.0.0.1:22 \
-device virtconsole,chardev=test0,name=test0 \
...
# in the guest:
./qemu-vp -c virtserial-open:/dev/virtio-ports/test2:- -i http:127.0.0.1:80
\
-i ssh:127.0.0.1:22

# from host, access guest http server
wget http://locahost:9080
# from host, access guest ssh server
ssh localhost -p 9022

By specifying -i and -o options in the host and guest, respectively, the
channel can also be used to establish connections from a guest to a host. Unix
sockets can be specified by providing a pathname in place of the hostname/ip,
and a '-' in place of the port number.

KNOWN ISSUES:

- Sync issues with virtio-serial: This may or may not be related to the issue
above, but I noticed some cases where proxied ssh sessions from the guest to
the host would "lag" by a few bytes. For instance typing "top" would result in
"to" being displayed, and the "p" wouldn't show up till I hit another key. This
could be related to how I'm handling the buffering, but I haven't been able to
reproduce using a network-based channel. UPDATE: this issue seems to be fixed
here:

http://git.kernel.org/?p=linux/kernel/git/rusty/linux-2.6-for-linus.git;a=commit;h=6df7aadcd9290807c464675098b5dd2dc9da5075

TODO:

- Better channel negotiation to gracefully handle guest reboots/disconnects/etc
- Add monitor commands to add/remove virtproxy channels/oforwards/iforwards on
the fly

[Qemu-devel] [PATCH 1/4] virtio-blk: Convert fprintf() to error_report()

Errors should be logged using error_report() so they go to the
appropriate monitor.

Signed-off-by: Stefan Hajnoczi 
---
 hw/virtio-blk.c |4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/virtio-blk.c b/hw/virtio-blk.c
index 49528a9..e5f9b27 100644
--- a/hw/virtio-blk.c
+++ b/hw/virtio-blk.c
@@ -324,13 +324,13 @@ static void virtio_blk_handle_request(VirtIOBlockReq *req,
 MultiReqBuffer *mrb)
 {
 if (req->elem.out_num < 1 || req->elem.in_num < 1) {
-fprintf(stderr, "virtio-blk missing headers\n");
+error_report("virtio-blk missing headers");
 exit(1);
 }
 
 if (req->elem.out_sg[0].iov_len < sizeof(*req->out) ||
 req->elem.in_sg[req->elem.in_num - 1].iov_len < sizeof(*req->in)) {
-fprintf(stderr, "virtio-blk header not in correct element\n");
+error_report("virtio-blk header not in correct element");
 exit(1);
 }
 
-- 
1.7.2.3

Re: [Qemu-devel] Re: [PATCHv2 2/2] tap: mark fd handler as device


On 11/15/2010 04:44 PM, Michael S. Tsirkin wrote:

So I'm not sure I understand what is proposed here.
Care posting a patch?
   


I *think* just adding:

if (vm_running == 0) {
return qemu_net_queue_append(queue, sender, flags, data, size, NULL);
}

To qemu_net_queue_send() along with a state notifier to kick the queue 
on start would do the trick.


Regards,

Anthony Liguori

[Qemu-devel] Re: [PATCH] device-assignment: register a reset function

2010-11-15 Thread Jan Kiszka

[Wrong list, it's not upstream yet. I'm migrating the thread to kvm.]

Am 15.11.2010 12:33, Bernhard Kohl wrote:
> This is necessary because during reboot of a VM the assigned devices
> continue DMA transfers which causes memory corruption.
> 
> Signed-off-by: Thomas Ostler 
> Signed-off-by: Bernhard Kohl 
> ---
> Sorry for for the long delay. Finally we added Alex' suggestions
> and rebased the patch.
> 
> Thanks
> Bernhard
> ---
>  hw/device-assignment.c |   12 
>  1 files changed, 12 insertions(+), 0 deletions(-)
> 
> diff --git a/hw/device-assignment.c b/hw/device-assignment.c
> index 5f5bde1..3f8de66 100644
> --- a/hw/device-assignment.c
> +++ b/hw/device-assignment.c
> @@ -1434,6 +1434,17 @@ static void 
> assigned_dev_unregister_msix_mmio(AssignedDevice *dev)
>  dev->msix_table_page = NULL;
>  }
>  
> +static void reset_assigned_device(DeviceState *dev)
> +{
> +PCIDevice *d = DO_UPCAST(PCIDevice, qdev, dev);
> +uint32_t conf;
> +
> +/* reset the bus master bit to avoid further DMA transfers */
> +conf = assigned_dev_pci_read_config(d, PCI_COMMAND, 2);
> +conf &= ~PCI_COMMAND_MASTER;
> +assigned_dev_pci_write_config(d, PCI_COMMAND, conf, 2);

What about writing to /sys/bus/pci/devices/$DEVICE/reset? You probably
still need to put the command word into the reset state (ie. no RMW in
any case, just write 0), but the hardware should receive a reset as well
- if it is capable of doing a function-level reset, but we should at
least try.

> +}
> +
>  static int assigned_initfn(struct PCIDevice *pci_dev)
>  {
>  AssignedDevice *dev = DO_UPCAST(AssignedDevice, dev, pci_dev);
> @@ -1544,6 +1555,7 @@ static PCIDeviceInfo assign_info = {
>  .qdev.name= "pci-assign",
>  .qdev.desc= "pass through host pci devices to the guest",
>  .qdev.size= sizeof(AssignedDevice),
> +.qdev.reset   = reset_assigned_device,
>  .init = assigned_initfn,
>  .exit = assigned_exitfn,
>  .config_read  = assigned_dev_pci_read_config,

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux

Re: [Qemu-devel] Re: [PATCHv2 2/2] tap: mark fd handler as device

On Mon, Nov 15, 2010 at 03:05:39PM -0600, Anthony Liguori wrote:
> On 11/15/2010 02:53 PM, Stefan Hajnoczi wrote:
> >
> >vhost has a solution for this: register a VMChangeStateHandler
> >function that stops ioeventfd while vm_stop(0) is in effect.  Then
> >poll to see if there is work pending.
> >
> >I will add equivalent functionality to virtio-ioeventfd.
> 
> I still think that stopping this at the net/block layer is going to
> be a lot more robust in the long term.
>  All net/block traffic flows
> through a single set of interfaces whereas getting the devices to
> completely stop themselves requires touching every device and making
> sure that noone adds back vmstop-unfriendly behavior down the road.
> 
> Regards,
> 
> Anthony Liguori

So I'm not sure I understand what is proposed here.
Care posting a patch?

-- 
MST

Re: [Qemu-devel] Re: [Try2][PATCH] Initial implementation of a mpeg1 layer2 streaming audio driver.

2010-11-15 Thread François Revol


Le 15 nov. 2010 à 20:30, Anthony Liguori a écrit :

>> I'll see if I can buffer a bit more in the twolame code and if it helps, 
>> then I'll try to merge with the failed attempts I have around at using 
>> external progs.
>>   
> 
> Okay, but my thinking was that we'd do something like:
> 
> audio_capture "capture_command -opt=val -opt2=val" ...
> 
> Which would make it very easy to tie into lame, oggenc, etc.
> 
> Having simple alias commands like mp3_capture and ogg_capture that invoke 
> common tools with reasonable options also would be a bonus.

You mean in the console ? I hardly use it (beside the idea is to not have it 
accessible to avoid the user committing snapshots as it's just for demos).

Btw, we should really convert all those audio env vars to QemuOpts someday, it 
seems like a duplicate. It should still be possible to map env vars to them for 
the time being.

Anyway I'm still having a hard time getting sound to work reliably in Haiku 
guests regardless the output or emulated card. OTH it works perfectly in 
VirtualBox with almost no cpu overhead. If anyone has a clue...
It might be related to the use of RDTSC as primary time source, still it's the 
same in vbox.
We do have some support for HPET, I should try it someday, maybe.
Maybe it has to do with vbox using VT-x and me not even building QEMU with 
kvm... (does it even work on OSX ?)

François.

[Qemu-devel] [PATCH] slirp: Remove unused code for bad sprintf

Neither DECLARE_SPRINTF nor BAD_SPRINTF are needed for QEMU.

QEMU won't support systems with missing or bad declarations
for sprintf. The unused code was detected while looking for
functions with missing format checking. Instead of adding
GCC_FMT_ATTR, the unused code was removed.

Cc: Blue Swirl 
Signed-off-by: Stefan Weil 
---
 slirp/misc.c |   42 --
 slirp/slirp.h|   14 --
 slirp/slirp_config.h |6 --
 3 files changed, 0 insertions(+), 62 deletions(-)

diff --git a/slirp/misc.c b/slirp/misc.c
index 1aeb401..19dbec4 100644
--- a/slirp/misc.c
+++ b/slirp/misc.c
@@ -264,48 +264,6 @@ void lprint(const char *format, ...)
 va_end(args);
 }
 
-#ifdef BAD_SPRINTF
-
-#undef vsprintf
-#undef sprintf
-
-/*
- * Some BSD-derived systems have a sprintf which returns char *
- */
-
-int
-vsprintf_len(string, format, args)
-   char *string;
-   const char *format;
-   va_list args;
-{
-   vsprintf(string, format, args);
-   return strlen(string);
-}
-
-int
-#ifdef __STDC__
-sprintf_len(char *string, const char *format, ...)
-#else
-sprintf_len(va_alist) va_dcl
-#endif
-{
-   va_list args;
-#ifdef __STDC__
-   va_start(args, format);
-#else
-   char *string;
-   char *format;
-   va_start(args);
-   string = va_arg(args, char *);
-   format = va_arg(args, char *);
-#endif
-   vsprintf(string, format, args);
-   return strlen(string);
-}
-
-#endif
-
 void
 u_sleep(int usec)
 {
diff --git a/slirp/slirp.h b/slirp/slirp.h
index 462292d..dfd977a 100644
--- a/slirp/slirp.h
+++ b/slirp/slirp.h
@@ -237,20 +237,6 @@ void if_start(Slirp *);
 void if_start(struct ttys *);
 #endif
 
-#ifdef BAD_SPRINTF
-# define vsprintf vsprintf_len
-# define sprintf sprintf_len
- extern int vsprintf_len(char *, const char *, va_list);
- extern int sprintf_len(char *, const char *, ...);
-#endif
-
-#ifdef DECLARE_SPRINTF
-# ifndef BAD_SPRINTF
- extern int vsprintf(char *, const char *, va_list);
-# endif
- extern int vfprintf(FILE *, const char *, va_list);
-#endif
-
 #ifndef HAVE_STRERROR
  extern char *strerror(int error);
 #endif
diff --git a/slirp/slirp_config.h b/slirp/slirp_config.h
index f19c703..18db45c 100644
--- a/slirp/slirp_config.h
+++ b/slirp/slirp_config.h
@@ -85,9 +85,6 @@
 /* Define if the machine is big endian */
 //#undef HOST_WORDS_BIGENDIAN
 
-/* Define if your sprintf returns char * instead of int */
-#undef BAD_SPRINTF
-
 /* Define if you have readv */
 #undef HAVE_READV
 
@@ -97,9 +94,6 @@
 #define DECLARE_IOVEC
 #endif
 
-/* Define if a declaration of sprintf/fprintf is needed */
-#undef DECLARE_SPRINTF
-
 /* Define if you have a POSIX.1 sys/wait.h */
 #undef HAVE_SYS_WAIT_H
 
-- 
1.7.2.3

Re: [Qemu-devel] [PATCH] add a command line option to specify the interface to send multicast packets on

2010-11-15 Thread Mike Ryan

On Mon, Nov 15, 2010 at 03:07:51PM -0600, Anthony Liguori wrote:

> Thanks.  Second question is how portable is SIOCGIFADDR?  I suspect
> that's very Linux-centric..

It ostensibly exists in FreeBSD, but I can't get master to compile under
7.3-RELEASE or 8.1-STABLE.

Given that it is unavailable on at least one platform (mingw32) it
sounds like it should be detected in configure. However configure
doesn't appear to be generated from autoconf/automake. Is it a custom
build system?

Re: [Qemu-devel] [PATCH] add a command line option to specify the interface to send multicast packets on


Am 15.11.2010 22:07, schrieb Anthony Liguori:

On 11/15/2010 01:52 PM, Mike Ryan wrote:

I'll clarify/elaborate a bit:

When using a multicast socket, the OS chooses a default physical
interface to send packets. The patch I've supplied allows the user to
select the interface.

Suppose you have a setup like so:

BoxA --- BoxB --- BoxC

You wish to run virtual machines on BoxB and BoxC and network them using
a multicast UDP socket.

BoxB has two network interfaces, and the default multicast interface may
be the link between BoxA and BoxB. In this situation, BoxC will not
receive any multicast packets from BoxB and networking between the boxes
is therefore impossible.

The utility of a multicast socket is obviously limited in my simplified
example. Generalize BoxC to a LAN of physical machines all running
virtual machines you wish to network and the use case should become a
bit clearer.


Thanks.  Second question is how portable is SIOCGIFADDR?  I suspect 
that's very Linux-centric..


Regards,

Anthony Liguori



Yes. It is unavailable for mingw32 (windows).

Regards,

Stefan Weil

Re: [Qemu-devel] virtio-blk broken after system reset


On 11/13/2010 04:09 AM, Jan Kiszka wrote:


There is also real hw out there that goes into an error state if it's
misprogrammed.

I think we have to remove all those premature exits. They also prevent
handing the device inside the guest to an untrusted driver (relevant
once we have IOMMU emulation).
   


I think the key to achieving this is to isolate the device within QEMU.

IOW, have all fd callbacks, bottom halves, etc. tagged with a device 
context.  Have a mechanism that raises an error on a device that can 
then be used to stop issuing any type of callback to the device until reset.


Obviously, we can fix some of these by just simple code refactoring.

Regards,

Anthony Liguori


Why it is trying to print things to stderr is a different
matter, it should be using a proper error-reporting routine,
but this is a different story.
 

Jep. Even worse: the above message is not dumped to the console as the
stream isn't flushed on exit.

Jan

[Qemu-devel] KVM call agenda for Nov 16

2010-11-15 Thread Chris Wright

Please send in any agenda items you are interested in covering.

thanks,
-chris

[Qemu-devel] Re: [PATCH] configure: Add compiler option -Wmissing-format-attribute


Am 15.11.2010 21:22, schrieb Stefan Weil:

With the previous patches, hopefully all functions with
printf like arguments use gcc's format checking.

This was tested with default build configuration on linux
and windows hosts (including some cross compilations),
so chances are good that there remain few (if any) functions
without format checking.

Cc: Blue Swirl 
Signed-off-by: Stefan Weil 
---

[snip]

Hi,

to make testing and committing of this and the previous
patches easier, I have now set up a git repository which
contains all of them (more at the bottom of this mail).

I'd appreciate if someone could test compilation especially
with these environment (which I could not test):

* BSD / Darwin host
* Special configure options

This was my configuration (+ cross compilations for windows / mips / 
powerpc):


$ ./configure --enable-debug-tcg --trace-backend=simple 
--audio-drv-list="oss,alsa,sdl,esd,pa" --enable-attr

Install prefix/usr/local
BIOS directory/usr/local/share/qemu
binary directory  /usr/local/bin
config directory  /usr/local/etc
Manual directory  /usr/local/share/man
ELF interp prefix /usr/gnemul/qemu-%M
Source path   /home/stefan/src/qemu
C compilergcc
Host C compiler   gcc
CFLAGS-O2 -g
QEMU_CFLAGS   -Werror -m64 -I. -I$(SRC_PATH) -D_FORTIFY_SOURCE=2 
-D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE 
-Wstrict-prototypes -Wredundant-decls -Wmissing-format-attribute -Wall 
-Wundef -Wendif-labels -Wwrite-strings -Wmissing-prototypes 
-fno-strict-aliasing  -fstack-protector-all -Wempty-body 
-Wnested-externs -Wformat-security -Wformat-y2k -Winit-self 
-Wignored-qualifiers -Wold-style-declaration -Wold-style-definition 
-Wtype-limits

LDFLAGS   -Wl,--warn-common -m64 -g
make  make
install   install
host CPU  x86_64
host big endian   no
target list   i386-softmmu x86_64-softmmu arm-softmmu cris-softmmu 
m68k-softmmu microblaze-softmmu mips-softmmu mipsel-softmmu 
mips64-softmmu mips64el-softmmu ppc-softmmu ppcemb-softmmu ppc64-softmmu 
sh4-softmmu sh4eb-softmmu sparc-softmmu sparc64-softmmu i386-linux-user 
x86_64-linux-user alpha-linux-user arm-linux-user armeb-linux-user 
cris-linux-user m68k-linux-user microblaze-linux-user mips-linux-user 
mipsel-linux-user ppc-linux-user ppc64-linux-user ppc64abi32-linux-user 
sh4-linux-user sh4eb-linux-user sparc-linux-user sparc64-linux-user 
sparc32plus-linux-user

tcg debug enabled yes
Mon debug enabled no
gprof enabled no
sparse enabledno
strip binariesyes
profiler  no
static build  no
-Werror enabled   yes
SDL support   yes
curses supportyes
curl support  yes
check support no
mingw32 support   no
Audio drivers oss alsa sdl esd pa
Extra audio cards ac97 es1370 sb16 hda
Block whitelist
Mixer emulation   no
VNC TLS support   no
VNC SASL support  no
VNC JPEG support  yes
VNC PNG support   yes
VNC threadno
xen support   no
brlapi supportno
bluez  supportno
Documentation yes
NPTL support  yes
GUEST_BASEyes
PIE user targets  no
vde support   yes
IO thread no
Linux AIO support yes
ATTR/XATTR support yes
Install blobs yes
KVM support   yes
fdt support   no
preadv supportyes
fdatasync yes
madvise   yes
posix_madvise yes
uuid support  yes
vhost-net support no
Trace backend simple
Trace output file trace-
spice support no

Regards,
Stefan



The following changes since commit cf2c1839a955482f2e208d7400594bf076c222f2:

  add copyright to spiceaudio (2010-11-11 17:59:25 +0300)

are available in the git repository at:
  git://git.weilnetz.de/git/qemu for-blueswirl

Stefan Weil (7):
  *-dis: Replace fprintf_ftype by fprintf_function (format checking)
  target-sparc: Use fprintf_function (format checking)
  trace: Use fprintf_function (format checking)
  audio: Use GCC_FMT_ATTR (format checking)
  darwin-user: Use GCC_FMT_ATTR (format checking)
  slirp: Remove unused code for bad sprintf
  configure: Add compiler option -Wmissing-format-attribute

 HACKING|3 ---
 alpha-dis.c|3 +++
 arm-dis.c  |   14 +++---
 audio/audio_pt_int.c   |3 ++-
 configure  |1 +
 darwin-user/machload.c |2 +-
 darwin-user/qemu.h |2 +-
 dis-asm.h  |   10 ++
 m68k-dis.c |2 +-
 microblaze-dis.c   |2 +-
 mips-dis.c |2 --
 sh4-dis.c  |   16 +---
 simpletrace.h  |6 +++---
 slirp/misc.c   |   42 --
 slirp/slirp.h  |   14 --
 slirp/slirp_config.h   |6 --
 target-sparc/cpu.h |4 ++--
 17 files changed, 29 insertions(+), 103 deletions(-)

[Qemu-devel] [PATCH] trace: Use fprintf_function (format checking)

fprintf_function adds format checking with GCC_FMT_ATTR.

Cc: Blue Swirl 
Signed-off-by: Stefan Weil 
---
 simpletrace.h |6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/simpletrace.h b/simpletrace.h
index 72614ec..2f44ed3 100644
--- a/simpletrace.h
+++ b/simpletrace.h
@@ -29,10 +29,10 @@ void trace3(TraceEventID event, uint64_t x1, uint64_t x2, 
uint64_t x3);
 void trace4(TraceEventID event, uint64_t x1, uint64_t x2, uint64_t x3, 
uint64_t x4);
 void trace5(TraceEventID event, uint64_t x1, uint64_t x2, uint64_t x3, 
uint64_t x4, uint64_t x5);
 void trace6(TraceEventID event, uint64_t x1, uint64_t x2, uint64_t x3, 
uint64_t x4, uint64_t x5, uint64_t x6);
-void st_print_trace(FILE *stream, int (*stream_printf)(FILE *stream, const 
char *fmt, ...));
-void st_print_trace_events(FILE *stream, int (*stream_printf)(FILE *stream, 
const char *fmt, ...));
+void st_print_trace(FILE *stream, fprintf_function stream_printf);
+void st_print_trace_events(FILE *stream, fprintf_function stream_printf);
 bool st_change_trace_event_state(const char *tname, bool tstate);
-void st_print_trace_file_status(FILE *stream, int (*stream_printf)(FILE 
*stream, const char *fmt, ...));
+void st_print_trace_file_status(FILE *stream, fprintf_function stream_printf);
 void st_set_trace_file_enabled(bool enable);
 bool st_set_trace_file(const char *file);
 void st_flush_trace_buffer(void);
-- 
1.7.2.3

Re: [Qemu-devel] [PATCH] *-dis: Replace fprintf_ftype by fprintf_function (format checking)


Am 15.11.2010 21:01, schrieb Andreas Färber:

Am 15.11.2010 um 19:39 schrieb Stefan Weil:


This patch adds more printf format checking.

Additional modifications were needed for this code change:

* alpha-dis.c: The local definition of MAX conflicts with
 a previous definition from osdep.h, so add an #undef.

* dis-asm.h: Add include for fprintf_function (qemu-common.h).
 The standard (now redundant) includes are removed.

* mis-dis.c:


:-)


Thanks. Hopefully nobody will mis-interpret the missing 'p' :-)

Maybe Blue Swirl can add it, or simply commit it as it is
(so future readers of the commit log have fun, too).

[Qemu-devel] Re: [PATCHv4 15/15] Pass boot device list to firmware.

2010-11-15 Thread Blue Swirl

2010/11/15 Gleb Natapov :
> On Sun, Nov 14, 2010 at 10:50:13PM +, Blue Swirl wrote:
>> On Sun, Nov 14, 2010 at 3:39 PM, Gleb Natapov  wrote:
>> >
>> > Signed-off-by: Gleb Natapov 
>> > ---
>> >  hw/fw_cfg.c |   14 ++
>> >  hw/fw_cfg.h |    4 +++-
>> >  sysemu.h    |    1 +
>> >  vl.c        |   51 +++
>> >  4 files changed, 69 insertions(+), 1 deletions(-)
>> >
>> > diff --git a/hw/fw_cfg.c b/hw/fw_cfg.c
>> > index 7b9434f..f6a67db 100644
>> > --- a/hw/fw_cfg.c
>> > +++ b/hw/fw_cfg.c
>> > @@ -53,6 +53,7 @@ struct FWCfgState {
>> >     FWCfgFiles *files;
>> >     uint16_t cur_entry;
>> >     uint32_t cur_offset;
>> > +    Notifier machine_ready;
>> >  };
>> >
>> >  static void fw_cfg_write(FWCfgState *s, uint8_t value)
>> > @@ -315,6 +316,15 @@ int fw_cfg_add_file(FWCfgState *s,  const char 
>> > *filename, uint8_t *data,
>> >     return 1;
>> >  }
>> >
>> > +static void fw_cfg_machine_ready(struct Notifier* n)
>> > +{
>> > +    uint32_t len;
>> > +    char *bootindex = get_boot_devices_list(&len);
>> > +
>> > +    fw_cfg_add_bytes(container_of(n, FWCfgState, machine_ready),
>> > +                     FW_CFG_BOOTINDEX, (uint8_t*)bootindex, len);
>> > +}
>> > +
>> >  FWCfgState *fw_cfg_init(uint32_t ctl_port, uint32_t data_port,
>> >                         target_phys_addr_t ctl_addr, target_phys_addr_t 
>> > data_addr)
>> >  {
>> > @@ -343,6 +353,10 @@ FWCfgState *fw_cfg_init(uint32_t ctl_port, uint32_t 
>> > data_port,
>> >     fw_cfg_add_i16(s, FW_CFG_MAX_CPUS, (uint16_t)max_cpus);
>> >     fw_cfg_add_i16(s, FW_CFG_BOOT_MENU, (uint16_t)boot_menu);
>> >
>> > +
>> > +    s->machine_ready.notify = fw_cfg_machine_ready;
>> > +    qemu_add_machine_init_done_notifier(&s->machine_ready);
>> > +
>> >     return s;
>> >  }
>> >
>> > diff --git a/hw/fw_cfg.h b/hw/fw_cfg.h
>> > index 856bf91..4d61410 100644
>> > --- a/hw/fw_cfg.h
>> > +++ b/hw/fw_cfg.h
>> > @@ -30,7 +30,9 @@
>> >
>> >  #define FW_CFG_FILE_FIRST       0x20
>> >  #define FW_CFG_FILE_SLOTS       0x10
>> > -#define FW_CFG_MAX_ENTRY        (FW_CFG_FILE_FIRST+FW_CFG_FILE_SLOTS)
>> > +#define FW_CFG_FILE_LAST_SLOT   (FW_CFG_FILE_FIRST+FW_CFG_FILE_SLOTS)
>> > +#define FW_CFG_BOOTINDEX        (FW_CFG_FILE_LAST_SLOT + 1)
>> > +#define FW_CFG_MAX_ENTRY        FW_CFG_BOOTINDEX
>>
>> This should be
>> #define FW_CFG_MAX_ENTRY        (FW_CFG_BOOTINDEX + 1)
>> because the check is like this:
>>     if ((key & FW_CFG_ENTRY_MASK) >= FW_CFG_MAX_ENTRY) {
>>         s->cur_entry = FW_CFG_INVALID;
>>
> Yeah, will fix.
>
>> With that change, I got the bootindex passed to OpenBIOS:
>> OpenBIOS for Sparc64
>> Configuration device id QEMU version 1 machine id 0
>> kernel cmdline
>> CPUs: 1 x SUNW,UltraSPARC-IIi
>> UUID: ----
>> bootindex num_strings 1
>> bootindex /p...@01fe/i...@5/dr...@1/d...@0
>>
>> The device path does not match exactly, but it's close:
>> /p...@1fe,0/pci-...@5/i...@600/d...@0
>
> pbm->pci should be solvable by the patch at the end. Were in the spec
> it is allowed to abbreviate 1fe as 1fe,0? Spec allows to drop
> starting zeroes but TARGET_FMT_plx definition in targphys.h has 0 after
> %. I can define another one without leading zeroes. Can you suggest
> a name?

I think OpenBIOS for Sparc64 is not correct here, so it may be a bad
reference architecture. OBP on a real Ultra-5 used a path like this:
/p...@1f,0/p...@1,1/i...@3/d...@0,0

p...@1f,0 specifies the PCI host bridge at UPA bus port ID of 0x1f.
p...@1,1 specifies a PCI-PCI bridge.

> TARGET_FMT_lx is poisoned. As of ATA there is no open firmware
> binding spec for ATA, so everyone does what he pleases. I based my
> implementation on what open firmware showing when running on qemu x86.
> "pci-ata" should be "ide" according to PCI binding spec :)

Yes, for example there is no ATA in the Ultra-5 tree but in UltraAX it exists:
/p...@1f,4000/i...@3/a...@0,0/c...@0,0

> diff --git a/hw/apb_pci.c b/hw/apb_pci.c
> index c619112..643aa49 100644
> --- a/hw/apb_pci.c
> +++ b/hw/apb_pci.c
> @@ -453,6 +453,7 @@ static PCIDeviceInfo pbm_pci_host_info = {
>
>  static SysBusDeviceInfo pbm_host_info = {
>     .qdev.name = "pbm",
> +    .qdev.fw_name = "pci",

Perhaps the FW path should use device class names if no name is specified.

I'll try Sparc32 to see how this fits there.

[Qemu-devel] [PATCH] configure: Add compiler option -Wmissing-format-attribute

With the previous patches, hopefully all functions with
printf like arguments use gcc's format checking.

This was tested with default build configuration on linux
and windows hosts (including some cross compilations),
so chances are good that there remain few (if any) functions
without format checking.

Cc: Blue Swirl 
Signed-off-by: Stefan Weil 
---
 HACKING   |3 ---
 configure |1 +
 2 files changed, 1 insertions(+), 3 deletions(-)

diff --git a/HACKING b/HACKING
index 6ba9d7e..3af53fd 100644
--- a/HACKING
+++ b/HACKING
@@ -120,6 +120,3 @@ gcc's printf attribute directive in the prototype.
 This makes it so gcc's -Wformat and -Wformat-security options can do
 their jobs and cross-check format strings with the number and types
 of arguments.
-
-Currently many functions in QEMU are not following this rule but
-patches to add the attribute would be very much appreciated.
diff --git a/configure b/configure
index 7025d2b..d4c983a 100755
--- a/configure
+++ b/configure
@@ -140,6 +140,7 @@ windres="${cross_prefix}${windres}"
 QEMU_CFLAGS="-fno-strict-aliasing $QEMU_CFLAGS"
 CFLAGS="-g $CFLAGS"
 QEMU_CFLAGS="-Wall -Wundef -Wendif-labels -Wwrite-strings -Wmissing-prototypes 
$QEMU_CFLAGS"
+QEMU_CFLAGS="-Wmissing-format-attribute $QEMU_CFLAGS"
 QEMU_CFLAGS="-Wstrict-prototypes -Wredundant-decls $QEMU_CFLAGS"
 QEMU_CFLAGS="-D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE 
$QEMU_CFLAGS"
 QEMU_CFLAGS="-D_FORTIFY_SOURCE=2 $QEMU_CFLAGS"
-- 
1.7.2.3

Re: [Qemu-devel] Re: [PATCHv2 2/2] tap: mark fd handler as device

On Mon, Nov 15, 2010 at 4:09 PM, Anthony Liguori  wrote:
> On 11/15/2010 09:18 AM, Michael S. Tsirkin wrote:
>>
>> On Mon, Nov 15, 2010 at 08:55:07AM -0600, Anthony Liguori wrote:
>>
>>>
>>> On 11/15/2010 08:52 AM, Juan Quintela wrote:
>>>

 "Michael S. Tsirkin"   wrote:

>
> There's no reason for tap to run when VM is stopped.
> If we let it, it confuses the bridge on TX
> and corrupts DMA memory on RX.
>
> Signed-off-by: Michael S. Tsirkin
>

 once here, what handlers make sense to run while stopped?
 /me can think of the normal console, non live migration, loadvm and not
 much more.  Perhaps it is easier to just move the other way around?

>>>
>>> I'm not sure I concur that this is really a problem.
>>> Semantically, I don't think that stop has to imply that the guest
>>> memory no longer changes.
>>>
>>> Regards,
>>>
>>> Anthony Liguori
>>>
>>>

 Later, Juan.

>>
>> Well, I do not really know about vmstop that is not for migration.
>>
>
> They are separate.  For instance, we don't rely on stop to pause pending
> disk I/O because we don't serialize pending disk I/O operations.  Instead,
> we flush all pending I/O and rely on the fact that disk I/O requests are
> always submitted in the context of a vCPU operation.  This assumption breaks
> down though with ioeventfd so we need to revisit it.

vhost has a solution for this: register a VMChangeStateHandler
function that stops ioeventfd while vm_stop(0) is in effect.  Then
poll to see if there is work pending.

I will add equivalent functionality to virtio-ioeventfd.

Stefan

[Qemu-devel] Re: [PATCHv2 2/2] tap: mark fd handler as device

On Mon, Nov 15, 2010 at 10:09:29AM -0600, Anthony Liguori wrote:
> On 11/15/2010 09:18 AM, Michael S. Tsirkin wrote:
> >On Mon, Nov 15, 2010 at 08:55:07AM -0600, Anthony Liguori wrote:
> >>On 11/15/2010 08:52 AM, Juan Quintela wrote:
> >>>"Michael S. Tsirkin"   wrote:
> There's no reason for tap to run when VM is stopped.
> If we let it, it confuses the bridge on TX
> and corrupts DMA memory on RX.
> 
> Signed-off-by: Michael S. Tsirkin
> >>>once here, what handlers make sense to run while stopped?
> >>>/me can think of the normal console, non live migration, loadvm and not
> >>>much more.  Perhaps it is easier to just move the other way around?
> >>I'm not sure I concur that this is really a problem.
> >>Semantically, I don't think that stop has to imply that the guest
> >>memory no longer changes.
> >>
> >>Regards,
> >>
> >>Anthony Liguori
> >>
> >>>Later, Juan.
> >Well, I do not really know about vmstop that is not for migration.
> 
> They are separate.  For instance, we don't rely on stop to pause
> pending disk I/O because we don't serialize pending disk I/O
> operations.  Instead, we flush all pending I/O and rely on the fact
> that disk I/O requests are always submitted in the context of a vCPU
> operation.  This assumption breaks down though with ioeventfd so we
> need to revisit it.
> 
> >For most vmstop calls are for migration. And there, the problems are very
> >real.
> >
> >First, it's not just memory.  At least for network transmit, sending out
> >packets with the same MAC from two locations is a problem. See?
> 
> I agree it's a problem but I'm not sure that just marking fd
> handlers really helps.
> 
> Bottom halves can also trigger transmits.

Are there any system ones?
Can we just stop processing them?

>  I think that if we put
> something in the network layer that just queued packets if the vm is
> stopped, it would be a more robust solution to the problem.

Will only work for -net. The problem is for anything
that can trigger activity when vm is stopped.

> >For memory, it is much worse:  any memory changes can either get
> >discarded or not.  This breaks consistency guarantees that guest relies
> >upon. Imagine virtio index getting updated but content not being
> >updated.  See?
> 
> If you suppress any I/O then the memory changes don't matter because
> the same changes will happen on the destination too.

They matter, and same changes won't happen.
Example:

virtio used index is in page 1, it can point at data in page 2.
device writes into data, *then* into index. Order matters,
but won't be preserved: migration assumes memory does not
change after vmstop, and so it might send old values for
data but new values for index. Result will be invalid
data coming into guest.

On the destination guest will pick up the index and
get bad (stale) data.


> I think this basic problem is the same as Kemari.  We can either
> attempt to totally freeze a guest which means stopping all callbacks
> that are device related or we can prevent I/O from happening which
> should introduce enough determinism to fix the problem in practice.
> 
> Regards,
> 
> Anthony Liguori


See above. IMO it's a different problem. Unlike Kemari,
I don't really see any drawbacks to stop all callbacks.
Do you?

-- 
MST

Re: [Qemu-devel] [PATCH] add a command line option to specify the interface to send multicast packets on


On 11/15/2010 01:52 PM, Mike Ryan wrote:

I'll clarify/elaborate a bit:

When using a multicast socket, the OS chooses a default physical
interface to send packets. The patch I've supplied allows the user to
select the interface.

Suppose you have a setup like so:

BoxA --- BoxB --- BoxC

You wish to run virtual machines on BoxB and BoxC and network them using
a multicast UDP socket.

BoxB has two network interfaces, and the default multicast interface may
be the link between BoxA and BoxB. In this situation, BoxC will not
receive any multicast packets from BoxB and networking between the boxes
is therefore impossible.

The utility of a multicast socket is obviously limited in my simplified
example. Generalize BoxC to a LAN of physical machines all running
virtual machines you wish to network and the use case should become a
bit clearer.
   


Thanks.  Second question is how portable is SIOCGIFADDR?  I suspect 
that's very Linux-centric..


Regards,

Anthony Liguori


On Mon, Nov 15, 2010 at 01:36:28PM -0600, Anthony Liguori wrote:
   

On 11/15/2010 12:54 PM, Mike Ryan wrote:
 

Anyone care to comment?
   

I must admit, I don't understand the use-case well enough to really
give an appropriate amount of review as to whether this is the best
solution to the problem.

Michael, do you have any thoughts?

Regards,

Anthony Liguori

 

On Wed, Nov 10, 2010 at 05:47:35PM -0800, Mike Ryan wrote:
   

Add an option to specify the host interface to send multicast packets on
when using a multicast socket for networking. The option takes the name
of a host interface (e.g., eth0) and sets the IP_MULTICAST_IF socket
option, which causes the packets to use that interface as an egress.

This is useful if the host machine has several interfaces with several
virtual networks across disparate interfaces.
---
  net.c   |4 
  net/socket.c|   48 
  qemu-common.h   |1 +
  qemu-options.hx |   11 +--
  qemu_socket.h   |1 +
  5 files changed, 51 insertions(+), 14 deletions(-)

diff --git a/net.c b/net.c
index c5e6063..ff9eb27 100644
--- a/net.c
+++ b/net.c
@@ -1050,6 +1050,10 @@ static const struct {
  .name = "mcast",
  .type = QEMU_OPT_STRING,
  .help = "UDP multicast address and port number",
+}, {
+.name = "interface",
+.type = QEMU_OPT_STRING,
+.help = "interface to send multicast packets on",
  },
  { /* end of list */ }
  },
diff --git a/net/socket.c b/net/socket.c
index 1c4e153..ed7cd12 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -149,11 +149,13 @@ static void net_socket_send_dgram(void *opaque)
  qemu_send_packet(&s->nc, s->buf, size);
  }

-static int net_socket_mcast_create(struct sockaddr_in *mcastaddr)
+static int net_socket_mcast_create(struct sockaddr_in *mcastaddr, const char 
*interface)
  {
  struct ip_mreq imr;
  int fd;
  int val, ret;
+struct in_addr maddr;
+struct ifreq ifr;
  if (!IN_MULTICAST(ntohl(mcastaddr->sin_addr.s_addr))) {
fprintf(stderr, "qemu: error: specified mcastaddr \"%s\" (0x%08x) does not 
contain a multicast address\n",
inet_ntoa(mcastaddr->sin_addr),
@@ -201,6 +203,23 @@ static int net_socket_mcast_create(struct sockaddr_in 
*mcastaddr)
goto fail;
  }

+/* If an interface name is given, only send packets out that interface */
+if (interface != NULL) {
+strncpy(ifr.ifr_name, interface, IFNAMSIZ);
+ret = ioctl(fd, SIOCGIFADDR,&ifr);
+if (ret<   0) {
+fprintf(stderr, "qemu: error: specified interface \"%s\" does not 
exist\n", interface);
+goto fail;
+}
+
+maddr = ((struct sockaddr_in *)&ifr.ifr_addr)->sin_addr;
+ret = setsockopt(fd, IPPROTO_IP, IP_MULTICAST_IF,&maddr, 
sizeof(maddr));
+if (ret<   0) {
+perror("setsockopt(IP_MULTICAST_IF)");
+goto fail;
+}
+}
+
  socket_set_nonblock(fd);
  return fd;
  fail:
@@ -248,7 +267,7 @@ static NetSocketState *net_socket_fd_init_dgram(VLANState 
*vlan,
return NULL;
}
/* clone dgram socket */
-   newfd = net_socket_mcast_create(&saddr);
+   newfd = net_socket_mcast_create(&saddr, NULL);
if (newfd<   0) {
/* error already reported by net_socket_mcast_create() */
close(fd);
@@ -468,7 +487,8 @@ static int net_socket_connect_init(VLANState *vlan,
  static int net_socket_mcast_init(VLANState *vlan,
   const char *model,
   const char *name,
- const char *host_str)
+ const char *host_str,
+ const char *interface)
  {
  NetSocketState *s;
  int fd;
@@ -478,7 +498,7

Re: [Qemu-devel] [PATCH 1/3] Make paio subsystem use threadlets infrastructure


On 11/15/2010 11:53 AM, Arun R Bharadwaj wrote:

From: Gautham R Shenoy

This patch creates a generic asynchronous-task-offloading infrastructure named
threadlets.

The patch creates a global queue on-to which subsystems can queue their tasks to
be executed asynchronously.

The patch also provides API's that allow a subsystem to create a private queue
with an associated pool of threads.

The patch has been tested with fstress.
   


This patch is very hard to review.  Can you generize this via a serial 
of incremental changes instead of a big rewrite?


This code has been historically fickle so doing the extra work to 
simplify review is worth it.


Regards,

Anthony Liguori


Signed-off-by: Arun R Bharadwaj
Signed-off-by: Aneesh Kumar K.V
Signed-off-by: Gautham R Shenoy
Signed-off-by: Sripathi Kodi
---
  Makefile.objs  |2
  configure  |2
  posix-aio-compat.c |  354 +++-
  3 files changed, 211 insertions(+), 147 deletions(-)

diff --git a/Makefile.objs b/Makefile.objs
index cd5a24b..3b7ec27 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -9,6 +9,7 @@ qobject-obj-y += qerror.o

  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 osdep.o qemu-config.o
+block-obj-$(CONFIG_POSIX) += qemu-thread.o
  block-obj-$(CONFIG_POSIX) += posix-aio-compat.o
  block-obj-$(CONFIG_LINUX_AIO) += linux-aio.o

@@ -124,7 +125,6 @@ endif
  common-obj-y += $(addprefix ui/, $(ui-obj-y))

  common-obj-y += iov.o acl.o
-common-obj-$(CONFIG_THREAD) += qemu-thread.o
  common-obj-y += notify.o event_notifier.o
  common-obj-y += qemu-timer.o

diff --git a/configure b/configure
index a079a49..addf733 100755
--- a/configure
+++ b/configure
@@ -2456,7 +2456,6 @@ if test "$vnc_png" != "no" ; then
  fi
  if test "$vnc_thread" != "no" ; then
echo "CONFIG_VNC_THREAD=y">>  $config_host_mak
-  echo "CONFIG_THREAD=y">>  $config_host_mak
  fi
  if test "$fnmatch" = "yes" ; then
echo "CONFIG_FNMATCH=y">>  $config_host_mak
@@ -2534,7 +2533,6 @@ if test "$xen" = "yes" ; then
  fi
  if test "$io_thread" = "yes" ; then
echo "CONFIG_IOTHREAD=y">>  $config_host_mak
-  echo "CONFIG_THREAD=y">>  $config_host_mak
  fi
  if test "$linux_aio" = "yes" ; then
echo "CONFIG_LINUX_AIO=y">>  $config_host_mak
diff --git a/posix-aio-compat.c b/posix-aio-compat.c
index 7b862b5..e1812fc 100644
--- a/posix-aio-compat.c
+++ b/posix-aio-compat.c
@@ -29,7 +29,33 @@
  #include "block_int.h"

  #include "block/raw-posix-aio.h"
+#include "qemu-thread.h"

+#define MAX_GLOBAL_THREADS  64
+#define MIN_GLOBAL_THREADS   8
+
+static QemuMutex aiocb_mutex;
+static QemuCond aiocb_completion;
+
+typedef struct ThreadletQueue
+{
+QemuMutex lock;
+QemuCond cond;
+int max_threads;
+int min_threads;
+int cur_threads;
+int idle_threads;
+QTAILQ_HEAD(, ThreadletWork) request_list;
+} ThreadletQueue;
+
+typedef struct ThreadletWork
+{
+QTAILQ_ENTRY(ThreadletWork) node;
+void (*func)(struct ThreadletWork *work);
+} ThreadletWork;
+
+static ThreadletQueue globalqueue;
+static int globalqueue_init;

  struct qemu_paiocb {
  BlockDriverAIOCB common;
@@ -44,13 +70,12 @@ struct qemu_paiocb {
  int ev_signo;
  off_t aio_offset;

-QTAILQ_ENTRY(qemu_paiocb) node;
  int aio_type;
  ssize_t ret;
-int active;
  struct qemu_paiocb *next;

  int async_context_id;
+ThreadletWork work;
  };

  typedef struct PosixAioState {
@@ -58,64 +83,169 @@ typedef struct PosixAioState {
  struct qemu_paiocb *first_aio;
  } PosixAioState;

+static void *threadlet_worker(void *data)
+{
+ThreadletQueue *queue = data;

-static pthread_mutex_t lock = PTHREAD_MUTEX_INITIALIZER;
-static pthread_cond_t cond = PTHREAD_COND_INITIALIZER;
-static pthread_t thread_id;
-static pthread_attr_t attr;
-static int max_threads = 64;
-static int cur_threads = 0;
-static int idle_threads = 0;
-static QTAILQ_HEAD(, qemu_paiocb) request_list;
+qemu_mutex_lock(&queue->lock);
+while (1) {
+ThreadletWork *work;
+int ret = 0;
+
+while (QTAILQ_EMPTY(&queue->request_list)&&
+   (ret != ETIMEDOUT)) {
+/* wait for cond to be signalled or broadcast for 1000s */
+ret = qemu_cond_timedwait((&queue->cond),
+&(queue->lock), 10*10);
+}

-#ifdef CONFIG_PREADV
-static int preadv_present = 1;
-#else
-static int preadv_present = 0;
-#endif
+assert(queue->idle_threads != 0);
+if (QTAILQ_EMPTY(&queue->request_list)) {
+if (queue->cur_threads>  queue->min_threads) {
+/* We retain the minimum number of threads */
+break;
+}
+} else {
+work = QTAILQ_FIRST(&queue->request_list);
+QTAILQ_REMOVE(&queue->request_list, work, node);

-static void die2(int err, const char *what)
-{
-fprintf(stderr, "%s failed: %s\n", what, strerror(err));
-abort();
+

Re: [Qemu-devel] Re: [PATCHv2 2/2] tap: mark fd handler as device


On 11/15/2010 02:53 PM, Stefan Hajnoczi wrote:


vhost has a solution for this: register a VMChangeStateHandler
function that stops ioeventfd while vm_stop(0) is in effect.  Then
poll to see if there is work pending.

I will add equivalent functionality to virtio-ioeventfd.
   


I still think that stopping this at the net/block layer is going to be a 
lot more robust in the long term.  All net/block traffic flows through a 
single set of interfaces whereas getting the devices to completely stop 
themselves requires touching every device and making sure that noone 
adds back vmstop-unfriendly behavior down the road.


Regards,

Anthony Liguori


Stefan

[Qemu-devel] Re: [PATCHv2 2/2] tap: mark fd handler as device


On 11/15/2010 02:26 PM, Michael S. Tsirkin wrote:

Are there any system ones?
   


There's one in qemu-char.c.  Not sure it's easy to just say for the 
future that there can't be BHs that get delivered during vmstop.



Can we just stop processing them?
   


We ran into a lot of problems trying to do this with emulating 
synchronous I/O in the block layer.  If we can avoid the 
stop-dispatching handlers approach, I think we'll have far fewer problems.



  I think that if we put
something in the network layer that just queued packets if the vm is
stopped, it would be a more robust solution to the problem.
 

Will only work for -net. The problem is for anything
that can trigger activity when vm is stopped.
   


Activity or external I/O?

My assertion is that if we can stop things from hitting the disk, 
escaping the network, or leaving a character device, we're solid.



For memory, it is much worse:  any memory changes can either get
discarded or not.  This breaks consistency guarantees that guest relies
upon. Imagine virtio index getting updated but content not being
updated.  See?
   

If you suppress any I/O then the memory changes don't matter because
the same changes will happen on the destination too.
 

They matter, and same changes won't happen.
Example:

virtio used index is in page 1, it can point at data in page 2.
device writes into data, *then* into index. Order matters,
but won't be preserved: migration assumes memory does not
change after vmstop, and so it might send old values for
data but new values for index. Result will be invalid
data coming into guest.
   


No, this scenario is invalid.

Migration copies data while the guest is live and whenever a change 
happens, updates the dirty bitmap (that's why cpu_physical_memory_rw 
matters).


Once the VM is stopped, we never return to the main loop before 
completing migration so nothing else gets an opportunity to run.  This 
means when we send a page, we guarantee it won't be made dirty against 
until after the migration completes.


Once the migration completes, the contents of memory may change but 
that's okay.  As long as you stop packets from being sent, if you need 
to resume the guest, it'll pick up where it left off.



On the destination guest will pick up the index and
get bad (stale) data.
   


If you're seeing this happen with vhost, you aren't updating dirty bits 
correctly.  AFAICT, this cannot happen with userspace device models.


   

I think this basic problem is the same as Kemari.  We can either
attempt to totally freeze a guest which means stopping all callbacks
that are device related or we can prevent I/O from happening which
should introduce enough determinism to fix the problem in practice.

Regards,

Anthony Liguori
 


See above. IMO it's a different problem. Unlike Kemari,
I don't really see any drawbacks to stop all callbacks.
Do you?
   


I think it's going to be extremely difficult to get right and keep 
right.  A bunch of code needs to be converted to make us safe and then 
becomes brittle as it's very simple to change things in such a way that 
we're broken again.


Regards,

Anthony Liguori

[Qemu-devel] [PATCH 0/4] virtio: Convert fprintf() to error_report()

The virtio hardware emulation code uses fprintf(stderr, ...) error messages.
Improve things slightly by moving to error_report() so error messages will be
printed to the monitor, if present.

We want to handle device error states properly instead of bailing out with
exit(1) but this series does not attempt to fix that yet.

Leave virtio-9p for now where there are many cases of fprintf(stderr, ...) and
development is still very active.

[Qemu-devel] [PATCH 2/4] virtio: Convert fprintf() to error_report()

Signed-off-by: Stefan Hajnoczi 
---
 hw/virtio.c |   35 ++-
 1 files changed, 18 insertions(+), 17 deletions(-)

diff --git a/hw/virtio.c b/hw/virtio.c
index a2a657e..849a60f 100644
--- a/hw/virtio.c
+++ b/hw/virtio.c
@@ -14,6 +14,7 @@
 #include 
 
 #include "trace.h"
+#include "qemu-error.h"
 #include "virtio.h"
 #include "sysemu.h"
 
@@ -253,8 +254,8 @@ static int virtqueue_num_heads(VirtQueue *vq, unsigned int 
idx)
 
 /* Check it isn't doing very strange things with descriptor numbers. */
 if (num_heads > vq->vring.num) {
-fprintf(stderr, "Guest moved used index from %u to %u",
-idx, vring_avail_idx(vq));
+error_report("Guest moved used index from %u to %u",
+ idx, vring_avail_idx(vq));
 exit(1);
 }
 
@@ -271,7 +272,7 @@ static unsigned int virtqueue_get_head(VirtQueue *vq, 
unsigned int idx)
 
 /* If their number is silly, that's a fatal mistake. */
 if (head >= vq->vring.num) {
-fprintf(stderr, "Guest says index %u is available", head);
+error_report("Guest says index %u is available", head);
 exit(1);
 }
 
@@ -293,7 +294,7 @@ static unsigned virtqueue_next_desc(target_phys_addr_t 
desc_pa,
 wmb();
 
 if (next >= max) {
-fprintf(stderr, "Desc next is %u", next);
+error_report("Desc next is %u", next);
 exit(1);
 }
 
@@ -320,13 +321,13 @@ int virtqueue_avail_bytes(VirtQueue *vq, int in_bytes, 
int out_bytes)
 
 if (vring_desc_flags(desc_pa, i) & VRING_DESC_F_INDIRECT) {
 if (vring_desc_len(desc_pa, i) % sizeof(VRingDesc)) {
-fprintf(stderr, "Invalid size for indirect buffer table\n");
+error_report("Invalid size for indirect buffer table");
 exit(1);
 }
 
 /* If we've got too many, that implies a descriptor loop. */
 if (num_bufs >= max) {
-fprintf(stderr, "Looped descriptor");
+error_report("Looped descriptor");
 exit(1);
 }
 
@@ -340,7 +341,7 @@ int virtqueue_avail_bytes(VirtQueue *vq, int in_bytes, int 
out_bytes)
 do {
 /* If we've got too many, that implies a descriptor loop. */
 if (++num_bufs > max) {
-fprintf(stderr, "Looped descriptor");
+error_report("Looped descriptor");
 exit(1);
 }
 
@@ -374,7 +375,7 @@ void virtqueue_map_sg(struct iovec *sg, target_phys_addr_t 
*addr,
 len = sg[i].iov_len;
 sg[i].iov_base = cpu_physical_memory_map(addr[i], &len, is_write);
 if (sg[i].iov_base == NULL || len != sg[i].iov_len) {
-fprintf(stderr, "virtio: trying to map MMIO memory\n");
+error_report("virtio: trying to map MMIO memory");
 exit(1);
 }
 }
@@ -397,7 +398,7 @@ int virtqueue_pop(VirtQueue *vq, VirtQueueElement *elem)
 
 if (vring_desc_flags(desc_pa, i) & VRING_DESC_F_INDIRECT) {
 if (vring_desc_len(desc_pa, i) % sizeof(VRingDesc)) {
-fprintf(stderr, "Invalid size for indirect buffer table\n");
+error_report("Invalid size for indirect buffer table");
 exit(1);
 }
 
@@ -423,7 +424,7 @@ int virtqueue_pop(VirtQueue *vq, VirtQueueElement *elem)
 
 /* If we've got too many, that implies a descriptor loop. */
 if ((elem->in_num + elem->out_num) > max) {
-fprintf(stderr, "Looped descriptor");
+error_report("Looped descriptor");
 exit(1);
 }
 } while ((i = virtqueue_next_desc(desc_pa, i, max)) != max);
@@ -694,8 +695,8 @@ int virtio_load(VirtIODevice *vdev, QEMUFile *f)
 qemu_get_be16s(f, &vdev->queue_sel);
 qemu_get_be32s(f, &features);
 if (features & ~supported_features) {
-fprintf(stderr, "Features 0x%x unsupported. Allowed features: 0x%x\n",
-features, supported_features);
+error_report("Features 0x%x unsupported. Allowed features: 0x%x",
+ features, supported_features);
 return -1;
 }
 if (vdev->set_features)
@@ -717,11 +718,11 @@ int virtio_load(VirtIODevice *vdev, QEMUFile *f)
num_heads = vring_avail_idx(&vdev->vq[i]) - vdev->vq[i].last_avail_idx;
/* Check it isn't doing very strange things with descriptor numbers. */
if (num_heads > vdev->vq[i].vring.num) {
-   fprintf(stderr, "VQ %d size 0x%x Guest index 0x%x "
-"inconsistent with Host index 0x%x: delta 0x%x\n",
-   i, vdev->vq[i].vring.num,
-vring_avail_idx(&vdev->vq[i]),
-vdev->vq[i].last_avail_idx, num_heads);
+   error_report("VQ %d size 0x%x Guest index 0x%x "
+"inconsistent with Host index 0x%x: delta 0x%x",
+i, vdev->vq[i].vring.num,
+

[Qemu-devel] Re: [PATCH RESENT] msix: allow byte and word reading from mmio

On Mon, Nov 15, 2010 at 05:40:30PM +0100, Bernhard Kohl wrote:
> Am 15.11.2010 11:42, schrieb ext Michael S. Tsirkin:
> >On Thu, Aug 19, 2010 at 02:56:51PM +0200, Bernhard Kohl wrote:
> >>It's legal that the guest reads a single byte or word from mmio.
> >Interesting. The spec seems to say this:
> >
> > For all accesses to MSI-X Table and MSI-X PBA fields, software must use
> > aligned full DWORD or aligned full QWORD transactions; otherwise, the
> > result is undefined.
> 
> I will remove the first statement from the commit message and add
> something like the comment you proposed below.
> 
> >>I have an OS which reads single bytes and it works fine on real
> >>hardware. Maybe this happens due to casting.
> >What do you mean by casting?
> 
> I did not yet locate the line of code where this happens in the guest.
> As all other accesses are dword, I assume that there is some masking,
> shifting or casting to a byte variable in the source code, and the
> compiler generates a byte access from this.

And I presume it's too late to fix the guest in question?

> >>Signed-off-by: Bernhard Kohl
> >I guess we can merge this, but we need a comment I think since this
> >seems to contradict the spec, and people were sending patches relying on
> >this.
> >
> >Does something like the following describe the situation correctly?
> >
> >/* Note: PCI spec requires that all MSI-X table accesses
> >are either DWORD or QWORD, size aligned.  Some guests seem to violate
> >this rule for read accesses, performing single byte reads.
> >Since it's easy to support this, let's do so.
> >Also support 16 bit size aligned reads, just in case.
> >  */
> 
> Yes, that's is exactly the situation with my guest.
> I will add this comment.
> 
> >Does you guest also do 16 bit reads?  Are accesses at least aligned?
> 
> I will check this with my guest and the readw function disabled
> in the patch. This will take some time.
> 
> >>---
> >>  hw/msix.c |   20 
> >>  1 files changed, 16 insertions(+), 4 deletions(-)
> >>
> >>diff --git a/hw/msix.c b/hw/msix.c
> >>index d99403a..7dac7f7 100644
> >>--- a/hw/msix.c
> >>+++ b/hw/msix.c
> >>@@ -100,10 +100,22 @@ static uint32_t msix_mmio_readl(void *opaque, 
> >>target_phys_addr_t addr)
> >>  return pci_get_long(page + offset);
> >>  }
> >>
> >>-static uint32_t msix_mmio_read_unallowed(void *opaque, target_phys_addr_t 
> >>addr)
> >>+static uint32_t msix_mmio_readw(void *opaque, target_phys_addr_t addr)
> >>  {
> >>-fprintf(stderr, "MSI-X: only dword read is allowed!\n");
> >>-return 0;
> >>+PCIDevice *dev = opaque;
> >>+unsigned int offset = addr&  (MSIX_PAGE_SIZE - 1)&  ~0x1;
> >>+void *page = dev->msix_table_page;
> >>+
> >>+return pci_get_word(page + offset);
> >>+}
> >>+
> >>+static uint32_t msix_mmio_readb(void *opaque, target_phys_addr_t addr)
> >>+{
> >>+PCIDevice *dev = opaque;
> >>+unsigned int offset = addr&  (MSIX_PAGE_SIZE - 1);
> >>+void *page = dev->msix_table_page;
> >>+
> >>+return pci_get_byte(page + offset);
> >>  }
> >>
> >>  static uint8_t msix_pending_mask(int vector)
> >>@@ -198,7 +210,7 @@ static CPUWriteMemoryFunc * const msix_mmio_write[] = {
> >>  };
> >>
> >>  static CPUReadMemoryFunc * const msix_mmio_read[] = {
> >>-msix_mmio_read_unallowed, msix_mmio_read_unallowed, msix_mmio_readl
> >>+msix_mmio_readb, msix_mmio_readw, msix_mmio_readl
> >>  };
> >>
> >>  /* Should be called from device's map method. */
> >>-- 
> >>1.7.2.1

[Qemu-devel] [PATCH] darwin-user: Use GCC_FMT_ATTR (format checking)

The redundant forward declaration of qerror in machload.c
is removed because it should be taken from qemu.h.

Please note that this patch is untested because
I have no matching environment to compile it.

Cc: Blue Swirl 
Signed-off-by: Stefan Weil 
---
 darwin-user/machload.c |2 +-
 darwin-user/qemu.h |2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/darwin-user/machload.c b/darwin-user/machload.c
index 4bb5c72..3bc3b65 100644
--- a/darwin-user/machload.c
+++ b/darwin-user/machload.c
@@ -82,7 +82,7 @@ void *macho_text_sect = 0;
 int   macho_offset = 0;
 
 int load_object(const char *filename, struct target_pt_regs * regs, void ** 
mh);
-void qerror(const char *format, ...);
+
 #ifdef TARGET_I386
 typedef struct mach_i386_thread_state {
 unsigned inteax;
diff --git a/darwin-user/qemu.h b/darwin-user/qemu.h
index 0c5081b..b6d3e6c 100644
--- a/darwin-user/qemu.h
+++ b/darwin-user/qemu.h
@@ -100,7 +100,7 @@ int do_sigaction(int sig, const struct sigaction *act,
 int do_sigaltstack(const struct sigaltstack *ss, struct sigaltstack *oss);
 
 void gemu_log(const char *fmt, ...) GCC_FMT_ATTR(1, 2);
-void qerror(const char *fmt, ...);
+void qerror(const char *fmt, ...) GCC_FMT_ATTR(1, 2);
 
 void write_dt(void *ptr, unsigned long addr, unsigned long limit, int flags);
 
-- 
1.7.2.3

Re: [Qemu-devel] [PATCH] *-dis: Replace fprintf_ftype by fprintf_function (format checking)

2010-11-15 Thread Andreas Färber


Am 15.11.2010 um 19:39 schrieb Stefan Weil:


This patch adds more printf format checking.

Additional modifications were needed for this code change:

* alpha-dis.c: The local definition of MAX conflicts with
 a previous definition from osdep.h, so add an #undef.

* dis-asm.h: Add include for fprintf_function (qemu-common.h).
 The standard (now redundant) includes are removed.

* mis-dis.c:


:-)

[...]

alpha-dis.c  |3 +++
arm-dis.c|   14 +++---
dis-asm.h|   10 ++
m68k-dis.c   |2 +-
microblaze-dis.c |2 +-
mips-dis.c   |2 --
sh4-dis.c|   16 +---
7 files changed, 19 insertions(+), 30 deletions(-)

Re: [Qemu-devel] [PATCH] make trace options use autoconfy names

2010-11-15 Thread Andreas Färber


Am 15.11.2010 um 16:48 schrieb Paolo Bonzini:


On 11/15/2010 03:17 PM, Stefan Hajnoczi wrote:
On Sun, Nov 14, 2010 at 1:52 PM, Paolo  
Bonzini  wrote:

On 11/14/2010 02:38 PM, Andreas Färber wrote:


- --trace-file=*) trace_file="$optarg"
+ --enable-trace-file=*) trace_file="$optarg"
;;


but this should be --with-trace-file=... please. It is not being
enabled, just set to a different value.


--with-* should be reserved for library paths, but I can change it  
if people

prefer it that way.


I did think of the argument as a file name, sort of a relative path...

Actually I think we have something similar to overriding --prefix  
here

:).  It's a path that you can set at ./configure time.


Yeah, that's true.  However...


So is it not okay to use --trace-file=?


... Autoconf would not allow unknown options not starting with -- 
enable- or --with-.


The rationale to avoid incompatible options in QEMU is this: suppose  
you have a project using Autoconf (e.g. GCC) and you want to drop  
QEMU as a subdirectory in there, e.g. to run the GCC testsuite under  
QEMU usermode emulation (GCC can already do this for other  
simulators).  To pass options to QEMU's configure, you can include  
them in GCC's commandline.  The script will simply pass the option  
down to QEMU and it will be processed there.  However, if you pass -- 
trace-file to GCC's configure script, it will complain and stop.


Probably I would use something like --enable-trace- 
backend=simple:trace- if I was adding something similar to an  
autoconfiscated project.  But unless it provides some additional  
benefit (as is the case with cross-compilation support) I want to  
keep the syntactic changes in my patches to the minimum.



But I know nothing of autoconf and --enable-* or --with-* sort of
make sense too.


Whatever, I have to repost the other series anyway, so I'll change  
to --with-.


Thinking more about it, what about --enable-simple-trace=...,  
callapsing the two options into one?


Another autoconf way to pass this stuff would we ./configure ...  
TRACE_FILE=...


I wouldn't mind either way though, just noticed that the --enable- 
trace-file suggestion by autoconf convention would allow --disable- 
trace-file. Similar issue for --without-trace-file though.


Andreas

[Qemu-devel] [PATCH] audio: Use GCC_FMT_ATTR (format checking)

Cc: Blue Swirl 
Signed-off-by: Stefan Weil 
---
 audio/audio_pt_int.c |3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/audio/audio_pt_int.c b/audio/audio_pt_int.c
index f15cc70..908c569 100644
--- a/audio/audio_pt_int.c
+++ b/audio/audio_pt_int.c
@@ -8,7 +8,8 @@
 
 #include 
 
-static void logerr (struct audio_pt *pt, int err, const char *fmt, ...)
+static void GCC_FMT_ATTR(3, 4) logerr (struct audio_pt *pt, int err,
+   const char *fmt, ...)
 {
 va_list ap;
 
-- 
1.7.2.3

[Qemu-devel] How to detect a stopped guest os?

2010-11-15 Thread Wilhelm

Hello,

I know, this is not the right place to ask, but I wasn't abled to find a
users mailing list.

The question: is there any qemu (monitor) command to detect if the guest
os has stopped / poweroff?

-- 
Wilhelm

Re: [Qemu-devel] [PATCH] add a command line option to specify the interface to send multicast packets on

2010-11-15 Thread Mike Ryan

I'll clarify/elaborate a bit:

When using a multicast socket, the OS chooses a default physical
interface to send packets. The patch I've supplied allows the user to
select the interface.

Suppose you have a setup like so:

BoxA --- BoxB --- BoxC

You wish to run virtual machines on BoxB and BoxC and network them using
a multicast UDP socket.

BoxB has two network interfaces, and the default multicast interface may
be the link between BoxA and BoxB. In this situation, BoxC will not
receive any multicast packets from BoxB and networking between the boxes
is therefore impossible.

The utility of a multicast socket is obviously limited in my simplified
example. Generalize BoxC to a LAN of physical machines all running
virtual machines you wish to network and the use case should become a
bit clearer.

On Mon, Nov 15, 2010 at 01:36:28PM -0600, Anthony Liguori wrote:
> On 11/15/2010 12:54 PM, Mike Ryan wrote:
> >Anyone care to comment?
> 
> I must admit, I don't understand the use-case well enough to really
> give an appropriate amount of review as to whether this is the best
> solution to the problem.
> 
> Michael, do you have any thoughts?
> 
> Regards,
> 
> Anthony Liguori
> 
> >On Wed, Nov 10, 2010 at 05:47:35PM -0800, Mike Ryan wrote:
> >>Add an option to specify the host interface to send multicast packets on
> >>when using a multicast socket for networking. The option takes the name
> >>of a host interface (e.g., eth0) and sets the IP_MULTICAST_IF socket
> >>option, which causes the packets to use that interface as an egress.
> >>
> >>This is useful if the host machine has several interfaces with several
> >>virtual networks across disparate interfaces.
> >>---
> >>  net.c   |4 
> >>  net/socket.c|   48 
> >>  qemu-common.h   |1 +
> >>  qemu-options.hx |   11 +--
> >>  qemu_socket.h   |1 +
> >>  5 files changed, 51 insertions(+), 14 deletions(-)
> >>
> >>diff --git a/net.c b/net.c
> >>index c5e6063..ff9eb27 100644
> >>--- a/net.c
> >>+++ b/net.c
> >>@@ -1050,6 +1050,10 @@ static const struct {
> >>  .name = "mcast",
> >>  .type = QEMU_OPT_STRING,
> >>  .help = "UDP multicast address and port number",
> >>+}, {
> >>+.name = "interface",
> >>+.type = QEMU_OPT_STRING,
> >>+.help = "interface to send multicast packets on",
> >>  },
> >>  { /* end of list */ }
> >>  },
> >>diff --git a/net/socket.c b/net/socket.c
> >>index 1c4e153..ed7cd12 100644
> >>--- a/net/socket.c
> >>+++ b/net/socket.c
> >>@@ -149,11 +149,13 @@ static void net_socket_send_dgram(void *opaque)
> >>  qemu_send_packet(&s->nc, s->buf, size);
> >>  }
> >>
> >>-static int net_socket_mcast_create(struct sockaddr_in *mcastaddr)
> >>+static int net_socket_mcast_create(struct sockaddr_in *mcastaddr, const 
> >>char *interface)
> >>  {
> >>  struct ip_mreq imr;
> >>  int fd;
> >>  int val, ret;
> >>+struct in_addr maddr;
> >>+struct ifreq ifr;
> >>  if (!IN_MULTICAST(ntohl(mcastaddr->sin_addr.s_addr))) {
> >>fprintf(stderr, "qemu: error: specified mcastaddr \"%s\" (0x%08x) does 
> >> not contain a multicast address\n",
> >>inet_ntoa(mcastaddr->sin_addr),
> >>@@ -201,6 +203,23 @@ static int net_socket_mcast_create(struct sockaddr_in 
> >>*mcastaddr)
> >>goto fail;
> >>  }
> >>
> >>+/* If an interface name is given, only send packets out that interface 
> >>*/
> >>+if (interface != NULL) {
> >>+strncpy(ifr.ifr_name, interface, IFNAMSIZ);
> >>+ret = ioctl(fd, SIOCGIFADDR,&ifr);
> >>+if (ret<  0) {
> >>+fprintf(stderr, "qemu: error: specified interface \"%s\" does 
> >>not exist\n", interface);
> >>+goto fail;
> >>+}
> >>+
> >>+maddr = ((struct sockaddr_in *)&ifr.ifr_addr)->sin_addr;
> >>+ret = setsockopt(fd, IPPROTO_IP, IP_MULTICAST_IF,&maddr, 
> >>sizeof(maddr));
> >>+if (ret<  0) {
> >>+perror("setsockopt(IP_MULTICAST_IF)");
> >>+goto fail;
> >>+}
> >>+}
> >>+
> >>  socket_set_nonblock(fd);
> >>  return fd;
> >>  fail:
> >>@@ -248,7 +267,7 @@ static NetSocketState 
> >>*net_socket_fd_init_dgram(VLANState *vlan,
> >>return NULL;
> >>}
> >>/* clone dgram socket */
> >>-   newfd = net_socket_mcast_create(&saddr);
> >>+   newfd = net_socket_mcast_create(&saddr, NULL);
> >>if (newfd<  0) {
> >>/* error already reported by net_socket_mcast_create() */
> >>close(fd);
> >>@@ -468,7 +487,8 @@ static int net_socket_connect_init(VLANState *vlan,
> >>  static int net_socket_mcast_init(VLANState *vlan,
> >>   const char *model,
> >>   const char *name,
> >>- const char *host_str)
> >>+

Re: [Qemu-devel] [PATCH] add a command line option to specify the interface to send multicast packets on


On 11/15/2010 12:54 PM, Mike Ryan wrote:

Anyone care to comment?
   


I must admit, I don't understand the use-case well enough to really give 
an appropriate amount of review as to whether this is the best solution 
to the problem.


Michael, do you have any thoughts?

Regards,

Anthony Liguori


On Wed, Nov 10, 2010 at 05:47:35PM -0800, Mike Ryan wrote:
   

Add an option to specify the host interface to send multicast packets on
when using a multicast socket for networking. The option takes the name
of a host interface (e.g., eth0) and sets the IP_MULTICAST_IF socket
option, which causes the packets to use that interface as an egress.

This is useful if the host machine has several interfaces with several
virtual networks across disparate interfaces.
---
  net.c   |4 
  net/socket.c|   48 
  qemu-common.h   |1 +
  qemu-options.hx |   11 +--
  qemu_socket.h   |1 +
  5 files changed, 51 insertions(+), 14 deletions(-)

diff --git a/net.c b/net.c
index c5e6063..ff9eb27 100644
--- a/net.c
+++ b/net.c
@@ -1050,6 +1050,10 @@ static const struct {
  .name = "mcast",
  .type = QEMU_OPT_STRING,
  .help = "UDP multicast address and port number",
+}, {
+.name = "interface",
+.type = QEMU_OPT_STRING,
+.help = "interface to send multicast packets on",
  },
  { /* end of list */ }
  },
diff --git a/net/socket.c b/net/socket.c
index 1c4e153..ed7cd12 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -149,11 +149,13 @@ static void net_socket_send_dgram(void *opaque)
  qemu_send_packet(&s->nc, s->buf, size);
  }

-static int net_socket_mcast_create(struct sockaddr_in *mcastaddr)
+static int net_socket_mcast_create(struct sockaddr_in *mcastaddr, const char 
*interface)
  {
  struct ip_mreq imr;
  int fd;
  int val, ret;
+struct in_addr maddr;
+struct ifreq ifr;
  if (!IN_MULTICAST(ntohl(mcastaddr->sin_addr.s_addr))) {
fprintf(stderr, "qemu: error: specified mcastaddr \"%s\" (0x%08x) does not 
contain a multicast address\n",
inet_ntoa(mcastaddr->sin_addr),
@@ -201,6 +203,23 @@ static int net_socket_mcast_create(struct sockaddr_in 
*mcastaddr)
goto fail;
  }

+/* If an interface name is given, only send packets out that interface */
+if (interface != NULL) {
+strncpy(ifr.ifr_name, interface, IFNAMSIZ);
+ret = ioctl(fd, SIOCGIFADDR,&ifr);
+if (ret<  0) {
+fprintf(stderr, "qemu: error: specified interface \"%s\" does not 
exist\n", interface);
+goto fail;
+}
+
+maddr = ((struct sockaddr_in *)&ifr.ifr_addr)->sin_addr;
+ret = setsockopt(fd, IPPROTO_IP, IP_MULTICAST_IF,&maddr, 
sizeof(maddr));
+if (ret<  0) {
+perror("setsockopt(IP_MULTICAST_IF)");
+goto fail;
+}
+}
+
  socket_set_nonblock(fd);
  return fd;
  fail:
@@ -248,7 +267,7 @@ static NetSocketState *net_socket_fd_init_dgram(VLANState 
*vlan,
return NULL;
}
/* clone dgram socket */
-   newfd = net_socket_mcast_create(&saddr);
+   newfd = net_socket_mcast_create(&saddr, NULL);
if (newfd<  0) {
/* error already reported by net_socket_mcast_create() */
close(fd);
@@ -468,7 +487,8 @@ static int net_socket_connect_init(VLANState *vlan,
  static int net_socket_mcast_init(VLANState *vlan,
   const char *model,
   const char *name,
- const char *host_str)
+ const char *host_str,
+ const char *interface)
  {
  NetSocketState *s;
  int fd;
@@ -478,7 +498,7 @@ static int net_socket_mcast_init(VLANState *vlan,
  return -1;


-fd = net_socket_mcast_create(&saddr);
+fd = net_socket_mcast_create(&saddr, interface);
  if (fd<  0)
return -1;

@@ -505,8 +525,9 @@ int net_init_socket(QemuOpts *opts,

  if (qemu_opt_get(opts, "listen") ||
  qemu_opt_get(opts, "connect") ||
-qemu_opt_get(opts, "mcast")) {
-error_report("listen=, connect= and mcast= is invalid with fd=");
+qemu_opt_get(opts, "mcast") ||
+qemu_opt_get(opts, "interface")) {
+error_report("listen=, connect=, mcast= and interface= is invalid with 
fd=\n");
  return -1;
  }

@@ -524,8 +545,9 @@ int net_init_socket(QemuOpts *opts,

  if (qemu_opt_get(opts, "fd") ||
  qemu_opt_get(opts, "connect") ||
-qemu_opt_get(opts, "mcast")) {
-error_report("fd=, connect= and mcast= is invalid with listen=");
+qemu_opt_get(opts, "mcast") ||
+qemu_opt_get(opts, "interface")) {
+

Re: [Qemu-devel] Re: [Try2][PATCH] Initial implementation of a mpeg1 layer2 streaming audio driver.


On 11/12/2010 10:54 AM, François Revol wrote:

Le 12 nov. 2010 à 15:32, Anthony Liguori a écrit :

   

I did try years ago, but at least the current wav driver really didn't like 
fifos back then. I recall trying for hours to get it pipe to ffmpeg or others 
without much luck.

Also, this poses several problems about the control of the external process 
(respawn on listener disconnection, close on exit...).
   

Doing encoding in QEMU is going to starve the guest pretty bad.  For an x86 
guest, that's going to result in issues with time drift, etc.

Making reencoding with an external process work a bit more nicely also has the 
advantage of making this work with other formats like Ogg which are a bit more 
Open Source friendly.
 

The current patch uses a separate thread, but since I clone this part from the 
esdaudio code I didn't check what it was doing. It seems it's not really 
queueing anything though except the single mix buffer, which kind of defeats 
the purpose of having a separate thread. This might explain why it lags so much 
here... I tried increasing the buffer size and lowering the threshold but it 
doesn't seem to help.

I agree it'd be better to use external programs when possible but as I said 
it's a bit harder to handle the errors and such, and I wanted to have something 
working.
Also, it requires more work to set it up for users, they must install the 
externals, figure out the command line options...
Possibly we can provide default templates for known programs, either text files 
with % escaping for args, or just a shell script passing env vars maybe...

Besides, external or not, IIRC a pipe is by default 4kB max, which isn't much 
better for decoupling the processing on its own, if the encoder is too slow it 
will still starve the audio thread, and the rest. Also it all requires more 
context switching and IPC, which increase the total processing time.

So I think it might be interesting to have both.

I'll see if I can buffer a bit more in the twolame code and if it helps, then 
I'll try to merge with the failed attempts I have around at using external 
progs.
   


Okay, but my thinking was that we'd do something like:

audio_capture "capture_command -opt=val -opt2=val" ...

Which would make it very easy to tie into lame, oggenc, etc.

Having simple alias commands like mp3_capture and ogg_capture that 
invoke common tools with reasonable options also would be a bonus.


Regards,

Anthony Liguori


François.

[Qemu-devel] [PATCH] target-sparc: Use fprintf_function (format checking)

This change was missing in commit
9a78eead0c74333a394c0f7bbfc4423ac746fcd5.

Cc: Blue Swirl 
Signed-off-by: Stefan Weil 
---
 target-sparc/cpu.h |4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/target-sparc/cpu.h b/target-sparc/cpu.h
index 7e0d17c..eb5d9c1 100644
--- a/target-sparc/cpu.h
+++ b/target-sparc/cpu.h
@@ -2,6 +2,7 @@
 #define CPU_SPARC_H
 
 #include "config.h"
+#include "qemu-common.h"
 
 #if !defined(TARGET_SPARC64)
 #define TARGET_LONG_BITS 32
@@ -442,8 +443,7 @@ typedef struct CPUSPARCState {
 /* helper.c */
 CPUSPARCState *cpu_sparc_init(const char *cpu_model);
 void cpu_sparc_set_id(CPUSPARCState *env, unsigned int cpu);
-void sparc_cpu_list (FILE *f, int (*cpu_fprintf)(FILE *f, const char *fmt,
- ...));
+void sparc_cpu_list(FILE *f, fprintf_function cpu_fprintf);
 void cpu_lock(void);
 void cpu_unlock(void);
 int cpu_sparc_handle_mmu_fault(CPUSPARCState *env1, target_ulong address, int 
rw,
-- 
1.7.2.3

Re: [Qemu-devel] [PATCH] add a command line option to specify the interface to send multicast packets on

2010-11-15 Thread Mike Ryan

Anyone care to comment?

On Wed, Nov 10, 2010 at 05:47:35PM -0800, Mike Ryan wrote:
> Add an option to specify the host interface to send multicast packets on
> when using a multicast socket for networking. The option takes the name
> of a host interface (e.g., eth0) and sets the IP_MULTICAST_IF socket
> option, which causes the packets to use that interface as an egress.
> 
> This is useful if the host machine has several interfaces with several
> virtual networks across disparate interfaces.
> ---
>  net.c   |4 
>  net/socket.c|   48 
>  qemu-common.h   |1 +
>  qemu-options.hx |   11 +--
>  qemu_socket.h   |1 +
>  5 files changed, 51 insertions(+), 14 deletions(-)
> 
> diff --git a/net.c b/net.c
> index c5e6063..ff9eb27 100644
> --- a/net.c
> +++ b/net.c
> @@ -1050,6 +1050,10 @@ static const struct {
>  .name = "mcast",
>  .type = QEMU_OPT_STRING,
>  .help = "UDP multicast address and port number",
> +}, {
> +.name = "interface",
> +.type = QEMU_OPT_STRING,
> +.help = "interface to send multicast packets on",
>  },
>  { /* end of list */ }
>  },
> diff --git a/net/socket.c b/net/socket.c
> index 1c4e153..ed7cd12 100644
> --- a/net/socket.c
> +++ b/net/socket.c
> @@ -149,11 +149,13 @@ static void net_socket_send_dgram(void *opaque)
>  qemu_send_packet(&s->nc, s->buf, size);
>  }
>  
> -static int net_socket_mcast_create(struct sockaddr_in *mcastaddr)
> +static int net_socket_mcast_create(struct sockaddr_in *mcastaddr, const char 
> *interface)
>  {
>  struct ip_mreq imr;
>  int fd;
>  int val, ret;
> +struct in_addr maddr;
> +struct ifreq ifr;
>  if (!IN_MULTICAST(ntohl(mcastaddr->sin_addr.s_addr))) {
>   fprintf(stderr, "qemu: error: specified mcastaddr \"%s\" (0x%08x) does 
> not contain a multicast address\n",
>   inet_ntoa(mcastaddr->sin_addr),
> @@ -201,6 +203,23 @@ static int net_socket_mcast_create(struct sockaddr_in 
> *mcastaddr)
>   goto fail;
>  }
>  
> +/* If an interface name is given, only send packets out that interface */
> +if (interface != NULL) {
> +strncpy(ifr.ifr_name, interface, IFNAMSIZ);
> +ret = ioctl(fd, SIOCGIFADDR, &ifr);
> +if (ret < 0) {
> +fprintf(stderr, "qemu: error: specified interface \"%s\" does 
> not exist\n", interface);
> +goto fail;
> +}
> +
> +maddr = ((struct sockaddr_in *)&ifr.ifr_addr)->sin_addr;
> +ret = setsockopt(fd, IPPROTO_IP, IP_MULTICAST_IF, &maddr, 
> sizeof(maddr));
> +if (ret < 0) {
> +perror("setsockopt(IP_MULTICAST_IF)");
> +goto fail;
> +}
> +}
> +
>  socket_set_nonblock(fd);
>  return fd;
>  fail:
> @@ -248,7 +267,7 @@ static NetSocketState *net_socket_fd_init_dgram(VLANState 
> *vlan,
>   return NULL;
>   }
>   /* clone dgram socket */
> - newfd = net_socket_mcast_create(&saddr);
> + newfd = net_socket_mcast_create(&saddr, NULL);
>   if (newfd < 0) {
>   /* error already reported by net_socket_mcast_create() */
>   close(fd);
> @@ -468,7 +487,8 @@ static int net_socket_connect_init(VLANState *vlan,
>  static int net_socket_mcast_init(VLANState *vlan,
>   const char *model,
>   const char *name,
> - const char *host_str)
> + const char *host_str,
> + const char *interface)
>  {
>  NetSocketState *s;
>  int fd;
> @@ -478,7 +498,7 @@ static int net_socket_mcast_init(VLANState *vlan,
>  return -1;
>  
>  
> -fd = net_socket_mcast_create(&saddr);
> +fd = net_socket_mcast_create(&saddr, interface);
>  if (fd < 0)
>   return -1;
>  
> @@ -505,8 +525,9 @@ int net_init_socket(QemuOpts *opts,
>  
>  if (qemu_opt_get(opts, "listen") ||
>  qemu_opt_get(opts, "connect") ||
> -qemu_opt_get(opts, "mcast")) {
> -error_report("listen=, connect= and mcast= is invalid with fd=");
> +qemu_opt_get(opts, "mcast") ||
> +qemu_opt_get(opts, "interface")) {
> +error_report("listen=, connect=, mcast= and interface= is 
> invalid with fd=\n");
>  return -1;
>  }
>  
> @@ -524,8 +545,9 @@ int net_init_socket(QemuOpts *opts,
>  
>  if (qemu_opt_get(opts, "fd") ||
>  qemu_opt_get(opts, "connect") ||
> -qemu_opt_get(opts, "mcast")) {
> -error_report("fd=, connect= and mcast= is invalid with listen=");
> +qemu_opt_get(opts, "mcast") ||
> +qemu_opt_get(opts, "interface")) {
> +error_report("fd=, connect=, mcast= and interface= is invalid 
>

[Qemu-devel] [PATCH] *-dis: Replace fprintf_ftype by fprintf_function (format checking)

This patch adds more printf format checking.

Additional modifications were needed for this code change:

* alpha-dis.c: The local definition of MAX conflicts with
  a previous definition from osdep.h, so add an #undef.

* dis-asm.h: Add include for fprintf_function (qemu-common.h).
  The standard (now redundant) includes are removed.

* mis-dis.c: The definition of ARRAY_SIZE is no longer needed
  and must be removed (conflict with previous definition from
  qemu-common.h).

* sh4-dis.c: Remove some unneeded forward declarations.

Cc: Blue Swirl 
Signed-off-by: Stefan Weil 
---
 alpha-dis.c  |3 +++
 arm-dis.c|   14 +++---
 dis-asm.h|   10 ++
 m68k-dis.c   |2 +-
 microblaze-dis.c |2 +-
 mips-dis.c   |2 --
 sh4-dis.c|   16 +---
 7 files changed, 19 insertions(+), 30 deletions(-)

diff --git a/alpha-dis.c b/alpha-dis.c
index 970da5b..8a2411e 100644
--- a/alpha-dis.c
+++ b/alpha-dis.c
@@ -22,6 +22,9 @@ along with this file; see the file COPYING.  If not, see
 #include 
 #include "dis-asm.h"
 
+/* MAX is redefined below, so remove any previous definition. */
+#undef MAX
+
 /* The opcode table is an array of struct alpha_opcode.  */
 
 struct alpha_opcode
diff --git a/arm-dis.c b/arm-dis.c
index fe7ac99..af21739 100644
--- a/arm-dis.c
+++ b/arm-dis.c
@@ -1587,7 +1587,7 @@ arm_decode_bitfield (const char *ptr, unsigned long insn,
 }
 
 static void
-arm_decode_shift (long given, fprintf_ftype func, void *stream,
+arm_decode_shift (long given, fprintf_function func, void *stream,
  int print_shift)
 {
   func (stream, "%s", arm_regnames[given & 0xf]);
@@ -1633,7 +1633,7 @@ print_insn_coprocessor (bfd_vma pc, struct 
disassemble_info *info, long given,
 {
   const struct opcode32 *insn;
   void *stream = info->stream;
-  fprintf_ftype func = info->fprintf_func;
+  fprintf_function func = info->fprintf_func;
   unsigned long mask;
   unsigned long value;
   int cond;
@@ -2127,7 +2127,7 @@ static void
 print_arm_address (bfd_vma pc, struct disassemble_info *info, long given)
 {
   void *stream = info->stream;
-  fprintf_ftype func = info->fprintf_func;
+  fprintf_function func = info->fprintf_func;
 
   if (((given & 0x000f) == 0x000f)
   && ((given & 0x0200) == 0))
@@ -,7 +,7 @@ print_insn_neon (struct disassemble_info *info, long 
given, bfd_boolean thumb)
 {
   const struct opcode32 *insn;
   void *stream = info->stream;
-  fprintf_ftype func = info->fprintf_func;
+  fprintf_function func = info->fprintf_func;
 
   if (thumb)
 {
@@ -2676,7 +2676,7 @@ print_insn_arm_internal (bfd_vma pc, struct 
disassemble_info *info, long given)
 {
   const struct opcode32 *insn;
   void *stream = info->stream;
-  fprintf_ftype func = info->fprintf_func;
+  fprintf_function func = info->fprintf_func;
 
   if (print_insn_coprocessor (pc, info, given, false))
 return;
@@ -3036,7 +3036,7 @@ print_insn_thumb16 (bfd_vma pc, struct disassemble_info 
*info, long given)
 {
   const struct opcode16 *insn;
   void *stream = info->stream;
-  fprintf_ftype func = info->fprintf_func;
+  fprintf_function func = info->fprintf_func;
 
   for (insn = thumb_opcodes; insn->assembler; insn++)
 if ((given & insn->mask) == insn->value)
@@ -3312,7 +3312,7 @@ print_insn_thumb32 (bfd_vma pc, struct disassemble_info 
*info, long given)
 {
   const struct opcode32 *insn;
   void *stream = info->stream;
-  fprintf_ftype func = info->fprintf_func;
+  fprintf_function func = info->fprintf_func;
 
   if (print_insn_coprocessor (pc, info, given, true))
 return;
diff --git a/dis-asm.h b/dis-asm.h
index 9b9657e..3fb4838 100644
--- a/dis-asm.h
+++ b/dis-asm.h
@@ -9,11 +9,7 @@
 #ifndef DIS_ASM_H
 #define DIS_ASM_H
 
-#include 
-#include 
-#include 
-#include 
-#include 
+#include "qemu-common.h"
 
 typedef void *PTR;
 typedef uint64_t bfd_vma;
@@ -237,8 +233,6 @@ typedef struct symbol_cache_entry
 } udata;
 } asymbol;
 
-typedef int (*fprintf_ftype) (FILE*, const char*, ...);
-
 enum dis_insn_type {
   dis_noninsn, /* Not a valid instruction */
   dis_nonbranch,   /* Not a branch instruction */
@@ -261,7 +255,7 @@ enum dis_insn_type {
by hand, or using one of the initialization macros below.  */
 
 typedef struct disassemble_info {
-  fprintf_ftype fprintf_func;
+  fprintf_function fprintf_func;
   FILE *stream;
   PTR application_data;
 
diff --git a/m68k-dis.c b/m68k-dis.c
index d93943e..04f837a 100644
--- a/m68k-dis.c
+++ b/m68k-dis.c
@@ -1732,7 +1732,7 @@ match_insn_m68k (bfd_vma memaddr,
   const char *d;
 
   bfd_byte *buffer = priv->the_buffer;
-  fprintf_ftype save_printer = info->fprintf_func;
+  fprintf_function save_printer = info->fprintf_func;
   void (* save_print_address) (bfd_vma, struct disassemble_info *)
 = info->print_address_func;
 
diff --git a/microblaze-dis.c b/microblaze-dis.c
index 7694a43..16c312f 100644
--- a/microblaze-dis.c
+++ b/microblaze-dis.c
@@ -789,7 +789,7

[Qemu-devel] [PATCH 3/3] Add helper functions to enable virtio-9p make use of the threadlets

From: Gautham R Shenoy 

infrastructure for offloading blocking tasks such as making posix calls on
to the helper threads and handle the post_posix_operations() from the
context of the iothread. This frees the vcpu thread to process any other guest
operations while the processing of v9fs_io is in progress.

Signed-off-by: Gautham R Shenoy 
Signed-off-by: Sripathi Kodi 
Signed-off-by: Arun R Bharadwaj 
---
 hw/virtio-9p.c |  164 
 posix-aio-compat.c |   30 +++---
 qemu-threadlets.c  |   21 +++
 qemu-threadlets.h  |1 
 vl.c   |3 +
 5 files changed, 196 insertions(+), 23 deletions(-)

diff --git a/hw/virtio-9p.c b/hw/virtio-9p.c
index 7c59988..88da20f 100644
--- a/hw/virtio-9p.c
+++ b/hw/virtio-9p.c
@@ -18,6 +18,7 @@
 #include "fsdev/qemu-fsdev.h"
 #include "virtio-9p-debug.h"
 #include "virtio-9p-xattr.h"
+#include "qemu-threadlets.h"
 
 int debug_9p_pdu;
 
@@ -33,6 +34,149 @@ enum {
 Oappend = 0x80,
 };
 
+typedef struct V9fsPostOp {
+QTAILQ_ENTRY(V9fsPostOp) node;
+void (*func)(void *arg);
+void *arg;
+} V9fsPostOp;
+
+static struct {
+int rfd;
+int wfd;
+QemuMutex lock;
+QTAILQ_HEAD(, V9fsPostOp) post_op_list;
+} v9fs_async_struct;
+
+static void die2(int err, const char *what)
+{
+fprintf(stderr, "%s failed: %s\n", what, strerror(err));
+abort();
+}
+
+static void die(const char *what)
+{
+die2(errno, what);
+}
+
+#define ASYNC_MAX_PROCESS   5
+
+/**
+ * v9fs_process_post_ops: Process any pending v9fs_post_posix_operation
+ * @arg: Not used.
+ *
+ * This function serves as a callback to the iothread to be called into 
whenever
+ * the v9fs_async_struct.wfd is written into. This thread goes through the list
+ * of v9fs_post_posix_operations() and executes them. In the process, it might
+ * queue more job on the asynchronous thread pool.
+ */
+static void v9fs_process_post_ops(void *arg)
+{
+int count = 0;
+struct V9fsPostOp *post_op;
+int ret;
+char byte;
+
+do {
+ret = read(v9fs_async_struct.rfd, &byte, sizeof(byte));
+} while (ret > 0 || (ret == -1 && errno == EINTR));
+
+qemu_mutex_lock(&v9fs_async_struct.lock);
+for (count = 0; count < ASYNC_MAX_PROCESS; count++) {
+if (QTAILQ_EMPTY(&(v9fs_async_struct.post_op_list))) {
+break;
+}
+post_op = QTAILQ_FIRST(&(v9fs_async_struct.post_op_list));
+QTAILQ_REMOVE(&(v9fs_async_struct.post_op_list), post_op, node);
+
+qemu_mutex_unlock(&v9fs_async_struct.lock);
+post_op->func(post_op->arg);
+qemu_free(post_op);
+qemu_mutex_lock(&v9fs_async_struct.lock);
+}
+qemu_mutex_unlock(&v9fs_async_struct.lock);
+}
+
+/**
+ * v9fs_async_signal: Inform the io-thread of completion of async job.
+ *
+ * This function is used to inform the iothread that a particular
+ * async-operation pertaining to v9fs has been completed and that the io thread
+ * can handle the v9fs_post_posix_operation.
+ *
+ * This is based on the aio_signal_handler
+ */
+static inline void v9fs_async_signal(void)
+{
+char byte = 0;
+ssize_t ret;
+int tries = 0;
+
+qemu_mutex_lock(&v9fs_async_struct.lock);
+do {
+assert(tries != 100);
+   ret = write(v9fs_async_struct.wfd, &byte, sizeof(byte));
+tries++;
+} while (ret < 0 && errno == EAGAIN);
+qemu_mutex_unlock(&v9fs_async_struct.lock);
+
+if (ret < 0 && errno != EAGAIN) {
+die("write() in v9fs");
+}
+
+if (kill(getpid(), SIGUSR2)) {
+die("kill failed");
+}
+}
+
+/**
+ * v9fs_async_helper_done: Marks the completion of the v9fs_async job
+ * @func: v9fs_post_posix_func() for post-processing invoked in the context of
+ *the io-thread
+ * @arg: Argument to func.
+ *
+ * This function is called from the context of one of the asynchronous threads
+ * in the thread pool. This is called when the asynchronous thread has finished
+ * executing a v9fs_posix_operation. It's purpose is to initiate the process of
+ * informing the io-thread that the v9fs_posix_operation has completed.
+ */
+static void v9fs_async_helper_done(void (*func)(void *arg), void *arg)
+{
+struct V9fsPostOp *post_op;
+
+post_op = qemu_mallocz(sizeof(*post_op));
+post_op->func = func;
+post_op->arg = arg;
+
+qemu_mutex_lock(&v9fs_async_struct.lock);
+QTAILQ_INSERT_TAIL(&(v9fs_async_struct.post_op_list), post_op, node);
+qemu_mutex_unlock(&v9fs_async_struct.lock);
+
+v9fs_async_signal();
+}
+
+/**
+ * v9fs_do_async_posix: Offload v9fs_posix_operation onto async thread.
+ * @vs: V9fsOPState variable for the OP operation.
+ * @posix_fn: The posix function which has to be offloaded onto async thread.
+ * @post_fn_ptr: Address of the location to hold the post_fn corresponding to
+ * the posix_fn
+ * @post_fn: The post processing function corresponding to the posix_fn.
+ *
+ * This function is a helper to offload posix_operation on to t

[Qemu-devel] [PATCH 2/3] Move threadlets infrastructure to qemu-threadlets.c

The reason for creating this generic infrastructure is so that other subsystems,
such as virtio-9p could make use of it for offloading tasks that could block.

Signed-off-by: Arun R Bharadwaj 
Signed-off-by: Aneesh Kumar K.V 
Signed-off-by: Gautham R Shenoy 
Signed-off-by: Sripathi Kodi 

Acked-by: Stefan Hajnoczi 
---
 Makefile.objs  |1 
 docs/async-support.txt |  141 +++
 posix-aio-compat.c |  173 
 qemu-threadlets.c  |  168 +++
 qemu-threadlets.h  |   46 +
 5 files changed, 357 insertions(+), 172 deletions(-)
 create mode 100644 docs/async-support.txt
 create mode 100644 qemu-threadlets.c
 create mode 100644 qemu-threadlets.h

diff --git a/Makefile.objs b/Makefile.objs
index 3b7ec27..2cf8aba 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -10,6 +10,7 @@ qobject-obj-y += qerror.o
 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 osdep.o qemu-config.o
 block-obj-$(CONFIG_POSIX) += qemu-thread.o
+block-obj-$(CONFIG_POSIX) += qemu-threadlets.o
 block-obj-$(CONFIG_POSIX) += posix-aio-compat.o
 block-obj-$(CONFIG_LINUX_AIO) += linux-aio.o
 
diff --git a/docs/async-support.txt b/docs/async-support.txt
new file mode 100644
index 000..9f22b9a
--- /dev/null
+++ b/docs/async-support.txt
@@ -0,0 +1,141 @@
+== How to use the threadlets infrastructure supported in Qemu ==
+
+== Threadlets ==
+
+Q.1: What are threadlets ?
+A.1: Threadlets is an infrastructure within QEMU that allows other subsystems
+ to offload possibly blocking work to a queue to be processed by a pool
+ of threads asynchronously.
+
+Q.2: When would one want to use threadlets ?
+A.2: Threadlets are useful when there are operations that can be performed
+ outside the context of the VCPU/IO threads inorder to free these latter
+ to service any other guest requests.
+
+Q.3: I have some work that can be executed in an asynchronous context. How
+ should I go about it ?
+A.3: One could follow the steps listed below:
+
+ - Define a function which would do the asynchronous work.
+   static void my_threadlet_func(ThreadletWork *work)
+   {
+   }
+
+ - Declare an object of type ThreadletWork;
+   ThreadletWork work;
+
+
+ - Assign a value to the "func" member of ThreadletWork object.
+   work.func = my_threadlet_func;
+
+ - Submit the threadlet to the global queue.
+   submit_threadletwork(&work);
+
+ - Continue servicing some other guest operations.
+
+Q.4: I want to my_threadlet_func to access some non-global data. How do I do
+ that ?
+A.4: Suppose you want my_threadlet_func to access some non-global data-object
+ of type myPrivateData. In that case one could follow the following steps.
+
+ - Define a member of the type ThreadletWork within myPrivateData.
+   typedef struct MyPrivateData {
+   ...;
+   ...;
+   ...;
+   ThreadletWork work;
+   } MyPrivateData;
+
+   MyPrivateData my_data;
+
+ - Initialize myData.work as described in A.3
+   myData.work.func = my_threadlet_func;
+   submit_threadletwork(&myData.work);
+
+ - Access the myData object inside my_threadlet_func() using container_of
+   primitive
+   static void my_threadlet_func(ThreadletWork *work)
+   {
+   myPrivateData *mydata_ptr;
+   mydata_ptr = container_of(work, myPrivateData, work);
+
+   /* mydata_ptr now points to myData object */
+   }
+
+Q.5: Are there any precautions one must take while sharing data with the
+ Asynchronous thread-pool ?
+A.5: Yes, make sure that the helper function of the type my_threadlet_func()
+ does not access/modify data when it can be accessed or modified in the
+ context of VCPU thread or IO thread. This is because the asynchronous
+ threads in the pool can run in parallel with the VCPU/IOThreads as shown
+ in the figure.
+
+ A typical workflow is as follows:
+
+  VCPU/IOThread
+   |
+   | (1)
+   |
+   V
+Offload work  (2)
+  |---> to threadlets -> Helper thread
+  ||   |
+  ||   |
+  || (3)   | (4)
+  ||   |
+  | Handle other Guest requests|
+  ||   |
+  ||   V
+  || (3)  Signal the I/O Thread
+  |(6) |

[Qemu-devel] [PATCH 1/3] Make paio subsystem use threadlets infrastructure

From: Gautham R Shenoy 

This patch creates a generic asynchronous-task-offloading infrastructure named
threadlets.

The patch creates a global queue on-to which subsystems can queue their tasks to
be executed asynchronously.

The patch also provides API's that allow a subsystem to create a private queue
with an associated pool of threads.

The patch has been tested with fstress.

Signed-off-by: Arun R Bharadwaj 
Signed-off-by: Aneesh Kumar K.V 
Signed-off-by: Gautham R Shenoy 
Signed-off-by: Sripathi Kodi 
---
 Makefile.objs  |2 
 configure  |2 
 posix-aio-compat.c |  354 +++-
 3 files changed, 211 insertions(+), 147 deletions(-)

diff --git a/Makefile.objs b/Makefile.objs
index cd5a24b..3b7ec27 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -9,6 +9,7 @@ qobject-obj-y += qerror.o
 
 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 osdep.o qemu-config.o
+block-obj-$(CONFIG_POSIX) += qemu-thread.o
 block-obj-$(CONFIG_POSIX) += posix-aio-compat.o
 block-obj-$(CONFIG_LINUX_AIO) += linux-aio.o
 
@@ -124,7 +125,6 @@ endif
 common-obj-y += $(addprefix ui/, $(ui-obj-y))
 
 common-obj-y += iov.o acl.o
-common-obj-$(CONFIG_THREAD) += qemu-thread.o
 common-obj-y += notify.o event_notifier.o
 common-obj-y += qemu-timer.o
 
diff --git a/configure b/configure
index a079a49..addf733 100755
--- a/configure
+++ b/configure
@@ -2456,7 +2456,6 @@ if test "$vnc_png" != "no" ; then
 fi
 if test "$vnc_thread" != "no" ; then
   echo "CONFIG_VNC_THREAD=y" >> $config_host_mak
-  echo "CONFIG_THREAD=y" >> $config_host_mak
 fi
 if test "$fnmatch" = "yes" ; then
   echo "CONFIG_FNMATCH=y" >> $config_host_mak
@@ -2534,7 +2533,6 @@ if test "$xen" = "yes" ; then
 fi
 if test "$io_thread" = "yes" ; then
   echo "CONFIG_IOTHREAD=y" >> $config_host_mak
-  echo "CONFIG_THREAD=y" >> $config_host_mak
 fi
 if test "$linux_aio" = "yes" ; then
   echo "CONFIG_LINUX_AIO=y" >> $config_host_mak
diff --git a/posix-aio-compat.c b/posix-aio-compat.c
index 7b862b5..e1812fc 100644
--- a/posix-aio-compat.c
+++ b/posix-aio-compat.c
@@ -29,7 +29,33 @@
 #include "block_int.h"
 
 #include "block/raw-posix-aio.h"
+#include "qemu-thread.h"
 
+#define MAX_GLOBAL_THREADS  64
+#define MIN_GLOBAL_THREADS   8
+
+static QemuMutex aiocb_mutex;
+static QemuCond aiocb_completion;
+
+typedef struct ThreadletQueue
+{
+QemuMutex lock;
+QemuCond cond;
+int max_threads;
+int min_threads;
+int cur_threads;
+int idle_threads;
+QTAILQ_HEAD(, ThreadletWork) request_list;
+} ThreadletQueue;
+
+typedef struct ThreadletWork
+{
+QTAILQ_ENTRY(ThreadletWork) node;
+void (*func)(struct ThreadletWork *work);
+} ThreadletWork;
+
+static ThreadletQueue globalqueue;
+static int globalqueue_init;
 
 struct qemu_paiocb {
 BlockDriverAIOCB common;
@@ -44,13 +70,12 @@ struct qemu_paiocb {
 int ev_signo;
 off_t aio_offset;
 
-QTAILQ_ENTRY(qemu_paiocb) node;
 int aio_type;
 ssize_t ret;
-int active;
 struct qemu_paiocb *next;
 
 int async_context_id;
+ThreadletWork work;
 };
 
 typedef struct PosixAioState {
@@ -58,64 +83,169 @@ typedef struct PosixAioState {
 struct qemu_paiocb *first_aio;
 } PosixAioState;
 
+static void *threadlet_worker(void *data)
+{
+ThreadletQueue *queue = data;
 
-static pthread_mutex_t lock = PTHREAD_MUTEX_INITIALIZER;
-static pthread_cond_t cond = PTHREAD_COND_INITIALIZER;
-static pthread_t thread_id;
-static pthread_attr_t attr;
-static int max_threads = 64;
-static int cur_threads = 0;
-static int idle_threads = 0;
-static QTAILQ_HEAD(, qemu_paiocb) request_list;
+qemu_mutex_lock(&queue->lock);
+while (1) {
+ThreadletWork *work;
+int ret = 0;
+
+while (QTAILQ_EMPTY(&queue->request_list) &&
+   (ret != ETIMEDOUT)) {
+/* wait for cond to be signalled or broadcast for 1000s */
+ret = qemu_cond_timedwait((&queue->cond),
+ &(queue->lock), 10*10);
+}
 
-#ifdef CONFIG_PREADV
-static int preadv_present = 1;
-#else
-static int preadv_present = 0;
-#endif
+assert(queue->idle_threads != 0);
+if (QTAILQ_EMPTY(&queue->request_list)) {
+if (queue->cur_threads > queue->min_threads) {
+/* We retain the minimum number of threads */
+break;
+}
+} else {
+work = QTAILQ_FIRST(&queue->request_list);
+QTAILQ_REMOVE(&queue->request_list, work, node);
 
-static void die2(int err, const char *what)
-{
-fprintf(stderr, "%s failed: %s\n", what, strerror(err));
-abort();
+queue->idle_threads--;
+qemu_mutex_unlock(&queue->lock);
+
+/* execute the work function */
+work->func(work);
+
+qemu_mutex_lock(&queue->lock);
+queue->idle_threads++;
+}
+}
+
+queue->idle_th

[Qemu-devel] [PATCH 0/3] v11: Threadlets: A generic task offloading framework

Hi,

This is the v11 of the refactored patch-series to have a generic
asynchronous task offloading framework (called threadlets)
within qemu.

I have run KVM autotest suite with this patch. This test suite
ran successfully for the following tests:
-connecthon
-ebizzy
-dbench
-fsstress
-disktest
-cpu_hotplug
-hackbench
-sleeptest

Changelog:

* Moved the qemu_cond_broadcast to the right place.
* Removed unnecessary extern qualifiers.



The following series implements...

---

Arun R Bharadwaj (1):
  Move threadlets infrastructure to qemu-threadlets.c

Gautham R Shenoy (2):
  Make paio subsystem use threadlets infrastructure
  Add helper functions to enable virtio-9p make use of the threadlets


 Makefile.objs  |3 -
 configure  |2 
 docs/async-support.txt |  141 ++
 hw/virtio-9p.c |  164 ++-
 posix-aio-compat.c |  227 +++-
 qemu-threadlets.c  |  189 
 qemu-threadlets.h  |   47 ++
 vl.c   |3 +
 8 files changed, 599 insertions(+), 177 deletions(-)
 create mode 100644 docs/async-support.txt
 create mode 100644 qemu-threadlets.c
 create mode 100644 qemu-threadlets.h

-- 
arun

[Qemu-devel] Re: [PATCH] pci: allow hotplug removal of cold-plugged devices

2010-11-15 Thread Cam Macdonell

On Sun, Nov 14, 2010 at 7:18 AM, Michael S. Tsirkin  wrote:
> This patch fixes 5beb8ad503c88a76f2b8106c3b74b4ce485a60e1
> which broke hotplug removal of cold plugged devices:
>
> - pass addition/removal state to hotplug callbacks
> - use that in piix and pcie
>
> This also fixes an assert on hotplug removal of coldplugged
> express devices.
>
> Reported-by: by Cam Macdonell .
> Signed-off-by: Isaku Yamahata 
> Signed-off-by: Michael S. Tsirkin 

Acked-by: Cam Macdonell 

> ---
>
> So I think the below would be the cleanest way
> to fix the bug as we keep the hot/cold plug logic
> in a central palce. Untested. Comments? Cam?

Yes, it seems to fix the problem.

>
>
> diff --git a/hw/acpi_piix4.c b/hw/acpi_piix4.c
> index 66c7885..f549089 100644
> --- a/hw/acpi_piix4.c
> +++ b/hw/acpi_piix4.c
> @@ -585,7 +585,8 @@ static void pciej_write(void *opaque, uint32_t addr, 
> uint32_t val)
>     PIIX4_DPRINTF("pciej write %x <== %d\n", addr, val);
>  }
>
> -static int piix4_device_hotplug(DeviceState *qdev, PCIDevice *dev, int 
> state);
> +static int piix4_device_hotplug(DeviceState *qdev, PCIDevice *dev,
> +                                PCIHotplugState state);
>
>  static void piix4_acpi_system_hot_add_init(PCIBus *bus, PIIX4PMState *s)
>  {
> @@ -615,18 +616,23 @@ static void disable_device(PIIX4PMState *s, int slot)
>     s->pci0_status.down |= (1 << slot);
>  }
>
> -static int piix4_device_hotplug(DeviceState *qdev, PCIDevice *dev, int state)
> +static int piix4_device_hotplug(DeviceState *qdev, PCIDevice *dev,
> +                               PCIHotplugState state)
>  {
>     int slot = PCI_SLOT(dev->devfn);
>     PIIX4PMState *s = DO_UPCAST(PIIX4PMState, dev,
>                                 DO_UPCAST(PCIDevice, qdev, qdev));
>
> -    if (!dev->qdev.hotplugged)
> +    /* Don't send event when device is enabled during qemu machine creation:
> +     * it is present on boot, no hotplug event is necessary. We do send an
> +     * event when the device is disabled later. */
> +    if (state == PCI_COLDPLUG_ENABLED) {
>         return 0;
> +    }
>
>     s->pci0_status.up = 0;
>     s->pci0_status.down = 0;
> -    if (state) {
> +    if (state == PCI_HOTPLUG_ENABLED) {
>         enable_device(s, slot);
>     } else {
>         disable_device(s, slot);
> diff --git a/hw/pci.c b/hw/pci.c
> index 30e1603..316b24f 100644
> --- a/hw/pci.c
> +++ b/hw/pci.c
> @@ -1566,8 +1566,11 @@ static int pci_qdev_init(DeviceState *qdev, DeviceInfo 
> *base)
>     pci_add_option_rom(pci_dev);
>
>     if (bus->hotplug) {
> -        /* lower layer must check qdev->hotplugged */
> -        rc = bus->hotplug(bus->hotplug_qdev, pci_dev, 1);
> +        /* Let buses differentiate between hotplug and when device is
> +         * enabled during qemu machine creation. */
> +        rc = bus->hotplug(bus->hotplug_qdev, pci_dev,
> +                          qdev->hotplugged ? PCI_HOTPLUG_ENABLED:
> +                          PCI_COLDPLUG_ENABLED);
>         if (rc != 0) {
>             int r = pci_unregister_device(&pci_dev->qdev);
>             assert(!r);
> @@ -1581,7 +1584,8 @@ static int pci_unplug_device(DeviceState *qdev)
>  {
>     PCIDevice *dev = DO_UPCAST(PCIDevice, qdev, qdev);
>
> -    return dev->bus->hotplug(dev->bus->hotplug_qdev, dev, 0);
> +    return dev->bus->hotplug(dev->bus->hotplug_qdev, dev,
> +                             PCI_HOTPLUG_DISABLED);
>  }
>
>  void pci_qdev_register(PCIDeviceInfo *info)
> diff --git a/hw/pci.h b/hw/pci.h
> index 7100804..09b3e4c 100644
> --- a/hw/pci.h
> +++ b/hw/pci.h
> @@ -214,7 +214,15 @@ int pci_device_load(PCIDevice *s, QEMUFile *f);
>
>  typedef void (*pci_set_irq_fn)(void *opaque, int irq_num, int level);
>  typedef int (*pci_map_irq_fn)(PCIDevice *pci_dev, int irq_num);
> -typedef int (*pci_hotplug_fn)(DeviceState *qdev, PCIDevice *pci_dev, int 
> state);
> +
> +typedef enum {
> +    PCI_HOTPLUG_DISABLED,
> +    PCI_HOTPLUG_ENABLED,
> +    PCI_COLDPLUG_ENABLED,
> +} PCIHotplugState;
> +
> +typedef int (*pci_hotplug_fn)(DeviceState *qdev, PCIDevice *pci_dev,
> +                              PCIHotplugState state);
>  void pci_bus_new_inplace(PCIBus *bus, DeviceState *parent,
>                          const char *name, int devfn_min);
>  PCIBus *pci_bus_new(DeviceState *parent, const char *name, int devfn_min);
> diff --git a/hw/pcie.c b/hw/pcie.c
> index 35918f7..4df48b8 100644
> --- a/hw/pcie.c
> +++ b/hw/pcie.c
> @@ -192,14 +192,16 @@ static void pcie_cap_slot_event(PCIDevice *dev, 
> PCIExpressHotPlugEvent event)
>  }
>
>  static int pcie_cap_slot_hotplug(DeviceState *qdev,
> -                                 PCIDevice *pci_dev, int state)
> +                                 PCIDevice *pci_dev, PCIHotplugState state)
>  {
>     PCIDevice *d = DO_UPCAST(PCIDevice, qdev, qdev);
>     uint8_t *exp_cap = d->config + d->exp.exp_cap;
>     uint16_t sltsta = pci_get_word(exp_cap + PCI_EXP_SLTSTA);
>
> -    if (!pci_dev->qdev.hotplugged) {
> -        assert(state); /* this case only happe

[Qemu-devel] [Bug 241119] Re: usb_add of a Creative ZEN unrecognized in guest

2010-11-15 Thread Alexandrosg

Hi. I have a similar problem, with a simple JetFlash usb drive.

After adding it with usb_add, "info usb" shows nothing and I start getting the 
following message in the console:
husb: config #1 need -1
husb: 1 interfaces claimed for configuration 1
husb: grabbed usb device 1.3
usb_linux_update_endp_table: Protocol error

This is under Maverick using qemu-kvm 0.12.5+noroms-0ubuntu7

The same operation works fine in the same pc under my gentoo system,
currently running qemu-kvm-0.13.0, but it has been working now for a
year, without ever giving me problems.

-- 
usb_add of a Creative ZEN unrecognized in guest
https://bugs.launchpad.net/bugs/241119
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.

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

Bug description:
Binary package hint: kvm

This happens when I add my Creative ZEN to a virtual machine running XP. The 
device is recognised well at first and drivers are installed correctly. But 
when trying to connect windows crashes with the classic blue screen It 
complains about something like usbohci.sys, I can't read well because it 
crashes too fast.
I have also tried with another virtual machine running Vista, same results.
Any help would be really appreciated!

I'm using the module kvm-amd with Ubuntu 8.04

The USB device has the following ID: 041e:4157 Creative Technology, Ltd

kvm:
  Installed: 1:62+dfsg-0ubuntu7
  Candidate: 1:62+dfsg-0ubuntu7
  Version table:
 *** 1:62+dfsg-0ubuntu7 0
500 http://archive.ubuntu.com hardy/main Packages
100 /var/lib/dpkg/status

[Qemu-devel] [Bug 611142] Re: seabios should have native scsi support

2010-11-15 Thread Serge Hallyn

(Clearly this must be done upstream first, so marking Triaged for the Ubuntu
task)

** Changed in: qemu-kvm (Ubuntu)
   Status: New => Triaged

-- 
seabios should have native scsi support
https://bugs.launchpad.net/bugs/611142
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.

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

Bug description:
Binary package hint: seabios

Currently when a grub multiboot image is booted with 'kvm -kernel' and 
'biosdisk' module, it will see block devices of type IDE or virtio.  It will 
not see scsi devices.

To demonstrate this:
$ qemu-img create -f qcow2 disk.img 1G
$ grub-mkrescue --output=rescue.iso
$ grub-mkimage -O i386-pc --output=grub-mb.img biosdisk minicmd part_msdos

An 'ls' inside the grub prompt will show hard disks (hd0) on if:
a.) -drive uses interface of virtio or scsi
or
b.) kvm boots boot from a cdrom or floppy 

For example, with these commands, grub will see a '(hd0)'
$ kvm -drive file=disk.img,if=scsi,boot=on -cdrom rescue.iso -boot d
$ kvm -drive file=disk.img,if=scsi,boot=on -floppy rescue.iso -boot a
$ kvm -drive file=disk.img,if=virtio,boot=on -cdrom rescue.iso -boot d
$ kvm -drive file=disk.img,if=ide,boot=on -cdrom rescue.iso -boot d
$ kvm -drive file=disk.img,if=virtio,boot=on -kernel grub-mb.img 
$ kvm -drive file=disk.img,if=ide,boot=on -kernel grub-mb.img 

But the following will not:
$ kvm -drive file=disk.img,if=scsi,boot=on -kernel grub-mb.img

ProblemType: Bug
DistroRelease: Ubuntu 10.10
Package: seabios 0.6.0-0ubuntu1
ProcVersionSignature: User Name 2.6.32-305.9-ec2 2.6.32.11+drm33.2
Uname: Linux 2.6.32-305-ec2 i686
Architecture: i386
Date: Thu Jul 29 03:21:21 2010
Dependencies:
 
Ec2AMI: ami-e930db80
Ec2AMIManifest: 
ubuntu-images-testing-us/ubuntu-maverick-daily-i386-server-20100727.manifest.xml
Ec2AvailabilityZone: us-east-1b
Ec2InstanceType: m1.small
Ec2Kernel: aki-407d9529
Ec2Ramdisk: unavailable
PackageArchitecture: all
ProcEnviron:
 PATH=(custom, user)
 LANG=en_US.UTF-8
 SHELL=/bin/bash
SourcePackage: seabios

[Qemu-devel] Re: [PATCH] ceph/rbd block driver for qemu-kvm (v7)

2010-11-15 Thread Yehuda Sadeh Weinraub

On Mon, Nov 15, 2010 at 9:04 AM, Christian Brunner
 wrote:
> Hi Stefan,
>
> thanks for your feedback. Yehuda and Sage have already committed some
> pathes to our git repository.
>
> What I'm not sure about is the rados_(de)initialization for multiple
> rbd images. I suspect that _deinitialize should only be called for the
> last rbd image.
>
> Yehuda and Sage know librados a lot better than me. I pretty sure,
> that they will give some feedback about this remaining issue. After
> that we will send an updated patch.
>
> Regards,
> Christian
>
> 2010/11/11 Stefan Hajnoczi :
>> On Fri, Oct 15, 2010 at 8:54 PM, Christian Brunner  wrote:
>>> [...]
>>> +
>>> +    if ((r = rados_initialize(0, NULL)) < 0) {
>>> +        error_report("error initializing");
>>> +        return r;
>>> +    }
>>
>> Does rados_initialize() work when called multiple times?  This would happen 
>> if
>> the VM has several rbd devices attached.
>>
>> [...]
>>
>> Stefan
>> --
>> To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
>> the body of a message to majord...@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>
> --
> To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

The rados (de)initialization is refcounted and it is safe to call it
multiple times.

[Qemu-devel] Re: [PATCH] make-release: fix mtime for a wider range of git versions

2010-11-15 Thread Bernhard Kohl


Am 15.11.2010 12:48, schrieb Bernhard Kohl:

With the latest git versions, e.g. 1.7.2.3, git still prints out
the tag info in addition to the requested format. So let's simply
fetch the first line from the output.

In addition I use the --pretty option instead of --format which
is not recognized in very old git versions, e.g. 1.5.5.6.

Tested with git versions 1.5.5.6 and 1.7.2.3.

Signed-off-by: Bernhard Kohl
   


Sorry, I sent this to the wrong list. Resent to kvm!

Bernhard


---
  kvm/scripts/make-release |2 +-
  1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/kvm/scripts/make-release b/kvm/scripts/make-release
index 56302c3..2d050fc 100755
--- a/kvm/scripts/make-release
+++ b/kvm/scripts/make-release
@@ -51,7 +51,7 @@ cd "$(dirname "$0")"/../..
  mkdir -p "$(dirname "$tarball")"
  git archive --prefix="$name/" --format=tar "$commit">  "$tarball"

-mtime=`git show --format=%ct "$commit""^{commit}" --`
+mtime=`git show --pretty=format:%ct "$commit""^{commit}" -- | head -n 1`
  tarargs="--owner=root --group=root"

  mkdir -p "$tmpdir/$name"

[Qemu-devel] Re: Where's gpxe-eepro100-80862449.rom ?


Am 25.10.2010 18:54, schrieb Michael S. Tsirkin:

On Mon, Oct 25, 2010 at 06:23:24PM +0200, Stefan Weil wrote:
   

Am 25.10.2010 14:11, schrieb Markus Armbruster:
 

Stefan Weil  writes:

   

Am 13.10.2010 09:13, schrieb Markus Armbruster:
 

Stefan Weil  writes:
   

[...]
   

Do you think there is urgent need for a
gpxe-eepro100-80862449.rom binary?

 

Well, "-device i82801" complains because it misses this binary. Do we
want to ship that way?

   

I just sent two patches which create the rom data on load.
So -device i82801 no longer complains but gets the boot
information from dhcp (tested with i386-softmmu).

Your build tree will be made smaller by at least 50 KB :-)

Do you think these patches should be added to stable-0.13, too?
 

If the gain for "-device i82801" is worth the risk, which depends on the
patch. Have we reached consensus on how to fix it in master?
   


The latest patch version fixes rom data only for the default
roms, so the risk is minimized and full tests are possible.

There is still no consensus whether we need a new qdev property
(which allows users to have their rom data fixed, too) or not.

My patch does not prevent adding that new qdev property,
so I suggest applying my patch now and adding the property
later (if it is ever needed).

Regards
Stefan
 

Fair enough, I guess.
   


So is there consensus that both patches can be committed to qemu master?

[Qemu-devel] Re: [PATCH] ceph/rbd block driver for qemu-kvm (v7)

2010-11-15 Thread Christian Brunner

Hi Stefan,

thanks for your feedback. Yehuda and Sage have already committed some
pathes to our git repository.

What I'm not sure about is the rados_(de)initialization for multiple
rbd images. I suspect that _deinitialize should only be called for the
last rbd image.

Yehuda and Sage know librados a lot better than me. I pretty sure,
that they will give some feedback about this remaining issue. After
that we will send an updated patch.

Regards,
Christian

2010/11/11 Stefan Hajnoczi :
> On Fri, Oct 15, 2010 at 8:54 PM, Christian Brunner  wrote:
>> [...]
>> +
>> +    if ((r = rados_initialize(0, NULL)) < 0) {
>> +        error_report("error initializing");
>> +        return r;
>> +    }
>
> Does rados_initialize() work when called multiple times?  This would happen if
> the VM has several rbd devices attached.
>
> [...]
>
> Stefan
> --
> To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

Re: [Qemu-devel] [PATCH] Introduce -accel command option.


On 11/15/2010 09:45 AM, anthony.per...@citrix.com wrote:

From: Anthony PERARD

This option gives the ability to switch one "accelerator" like kvm, xen
or the default one tcg. We can specify more than one accelerator by
separate them by a comma. QEMU will try each one and use the first whose
works.

So,

-accel xen,kvm,tcg

which would try Xen support first, then KVM and finaly tcg if none of
the other works.
   


Should use QemuOpts instead of parsing by hand.  I'd rather it be 
presented as a -machine option too with accel=xen:kvm:tcg to specify order.


Regards,

Anthony Liguori


Signed-off-by: Anthony PERARD
---
  arch_init.c |5 +++
  arch_init.h |1 +
  qemu-options.hx |   10 ++
  vl.c|   85 +++---
  4 files changed, 90 insertions(+), 11 deletions(-)

diff --git a/arch_init.c b/arch_init.c
index 4486925..e0d7a4c 100644
--- a/arch_init.c
+++ b/arch_init.c
@@ -639,6 +639,11 @@ int audio_available(void)
  #endif
  }

+int tcg_available(void)
+{
+return 1;
+}
+
  int kvm_available(void)
  {
  #ifdef CONFIG_KVM
diff --git a/arch_init.h b/arch_init.h
index 682890c..f0fb6a0 100644
--- a/arch_init.h
+++ b/arch_init.h
@@ -27,6 +27,7 @@ void do_acpitable_option(const char *optarg);
  void do_smbios_option(const char *optarg);
  void cpudef_init(void);
  int audio_available(void);
+int tcg_available(void);
  int kvm_available(void);
  int xen_available(void);

diff --git a/qemu-options.hx b/qemu-options.hx
index 4d99a58..958d126 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -1975,6 +1975,16 @@ Enable KVM full virtualization support. This option is 
only available
  if KVM support is enabled when compiling.
  ETEXI

+DEF("accel", HAS_ARG, QEMU_OPTION_accel, \
+"-accel acceluse an accelerator (kvm,xen,tcg), default is tcg\n", 
QEMU_ARCH_ALL)
+STEXI
+...@item -accel @var{accel}[,@var{accel}[,...]]
+...@findex -accel
+This is use to enable an accelerator, in kvm,xen,tcg.
+By default, it use only tcg. If there a more than one accelerator
+specified, the next one is used if the first don't work.
+ETEXI
+
  DEF("xen-domid", HAS_ARG, QEMU_OPTION_xen_domid,
  "-xen-domid id   specify xen guest domain id\n", QEMU_ARCH_ALL)
  DEF("xen-create", 0, QEMU_OPTION_xen_create,
diff --git a/vl.c b/vl.c
index c58583d..2917e32 100644
--- a/vl.c
+++ b/vl.c
@@ -242,6 +242,7 @@ static void *boot_set_opaque;
  static NotifierList exit_notifiers =
  NOTIFIER_LIST_INITIALIZER(exit_notifiers);

+static int tcg_allowed = 1;
  int kvm_allowed = 0;
  uint32_t xen_domid;
  enum xen_mode xen_mode = XEN_EMULATE;
@@ -1723,6 +1724,72 @@ static int debugcon_parse(const char *devname)
  return 0;
  }

+static int tcg_init(int smp_cpus)
+{
+return 0;
+}
+
+static struct {
+const char *opt_name;
+const char *name;
+int (*available)(void);
+int (*init)(int smp_cpus);
+int *allowed;
+} accel_list[] = {
+{ "tcg", "tcg", tcg_available, tcg_init,&tcg_allowed },
+{ "kvm", "KVM", kvm_available, kvm_init,&kvm_allowed },
+};
+
+static int accel_parse_init(const char *opts)
+{
+const char *p = opts;
+char buf[10];
+int i, ret;
+bool accel_initalised = 0;
+bool init_failed = 0;
+
+while (!accel_initalised&&  *p != '\0') {
+if (*p == ',') {
+p++;
+}
+p = get_opt_name(buf, sizeof (buf), p, ',');
+for (i = 0; i<  ARRAY_SIZE(accel_list); i++) {
+if (strcmp(accel_list[i].opt_name, buf) == 0) {
+ret = accel_list[i].init(smp_cpus);
+if (ret<  0) {
+init_failed = 1;
+if (!accel_list[i].available()) {
+printf("%s not supported for this target\n",
+   accel_list[i].name);
+} else {
+fprintf(stderr, "failed to initialize %s: %s\n",
+accel_list[i].name,
+strerror(-ret));
+}
+} else {
+accel_initalised = 1;
+*(accel_list[i].allowed) = 1;
+}
+break;
+}
+}
+if (i == ARRAY_SIZE(accel_list)) {
+fprintf(stderr, "\"%s\" accelerator does not exist.\n", buf);
+}
+}
+
+if (!accel_initalised) {
+fprintf(stderr, "No accelerator found!\n");
+exit(1);
+}
+
+if (init_failed) {
+fprintf(stderr, "Back to %s accelerator.\n", accel_list[i].name);
+}
+
+return !accel_initalised;
+}
+
  void qemu_add_exit_notifier(Notifier *notify)
  {
  notifier_list_add(&exit_notifiers, notify);
@@ -1802,6 +1869,7 @@ int main(int argc, char **argv, char **envp)
  const char *incoming = NULL;
  int show_vnc_port = 0;
  int defconfig = 1;
+const char *accel_list_opts = "tcg";

  #ifdef CONFIG_SIMPLE_TRACE
  const char *trace_file = NULL;

Re: [Qemu-devel] [PATCH 1/2] virtio-9p: Use chroot to safely access files in passthrough model

On Mon, Nov 15, 2010 at 2:52 PM, M. Mohan Kumar  wrote:
> In passthrough security model, following symbolic links in the server
> side could result in accessing files outside guest's export path.This
> could happen under two conditions:
> 1) If a modified guest kernel is sending symbolic link as part of the
> file path and when resolving that symbolic link at server side, it could
> result in accessing files outside export path.
> 2) If a same path is exported to multiple guests and if guest1 tries to
> open a file "a/b/c/passwd" and meanwhile guest2 did this "rm -rf a/b/c;
> cd a/b; ln -s ../../etc c". If guest1 lookup happened and guest2
> completed these operations just before guest1 opening the file, this
> operation could result in opening host's /etc/passwd.
>
> Following approach is used to avoid the security issue involved in
> following symbolic links in the passthrough model. Create a sub-process
> which will chroot into export path, so that even if there is a symbolic
> link in the path it could never go beyond the share path.
>
> When qemu is started with passthrough security model, a process is
> forked and this sub-process process takes care of accessing files in the
> passthrough share path. It does
>  * Create socketpair
>  * Chroot into share path
>  * Read file open request from socket descriptor
>  * Open request contains file path, flags, mode, uid, gid, dev etc
>  * Based on the request type it creates/opens file/directory/device node
>  * Return the file descriptor to main process using socket with
>   SCM_RIGHTS as cmsg type.
>
> Main process when ever there is a request for a resource to be
> opened/created, it constructs the open request and writes that into
> its socket descriptor and reads from chroot process socket to get the
> file descriptor.

How does the child process exit cleanly?  If QEMU crashes will the
child process be orphaned?

>
> This patch implements chroot enviroment, provides necessary functions
> that can be used by the passthrough function calls.
>
> Signed-off-by: M. Mohan Kumar 
> ---
>  Makefile.objs         |    1 +
>  hw/file-op-9p.h       |    2 +
>  hw/virtio-9p-chroot.c |  275 
> +
>  hw/virtio-9p.c        |   20 
>  hw/virtio-9p.h        |   21 
>  5 files changed, 319 insertions(+), 0 deletions(-)
>  create mode 100644 hw/virtio-9p-chroot.c
>
> diff --git a/Makefile.objs b/Makefile.objs
> index cd5a24b..134da8e 100644
> --- a/Makefile.objs
> +++ b/Makefile.objs
> @@ -251,6 +251,7 @@ hw-obj-$(CONFIG_SOUND) += $(sound-obj-y)
>
>  hw-obj-$(CONFIG_VIRTFS) += virtio-9p-debug.o virtio-9p-local.o 
> virtio-9p-xattr.o
>  hw-obj-$(CONFIG_VIRTFS) += virtio-9p-xattr-user.o virtio-9p-posix-acl.o
> +hw-obj-$(CONFIG_VIRTFS) += virtio-9p-chroot.o
>
>  ##
>  # libdis
> diff --git a/hw/file-op-9p.h b/hw/file-op-9p.h
> index c7731c2..149a915 100644
> --- a/hw/file-op-9p.h
> +++ b/hw/file-op-9p.h
> @@ -55,6 +55,8 @@ typedef struct FsContext
>     SecModel fs_sm;
>     uid_t uid;
>     struct xattr_operations **xops;
> +    pthread_mutex_t chroot_mutex;
> +    int chroot_socket;
>  } FsContext;
>
>  extern void cred_init(FsCred *);
> diff --git a/hw/virtio-9p-chroot.c b/hw/virtio-9p-chroot.c
> new file mode 100644
> index 000..50e28a1
> --- /dev/null
> +++ b/hw/virtio-9p-chroot.c
> @@ -0,0 +1,275 @@
> +/*
> + * Virtio 9p chroot environment for secured access to exported file
> + * system
> + *
> + * Copyright IBM, Corp. 2010
> + *
> + * Authors:
> + * M. Mohan Kumar 
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2. See
> + * the copying file in the top-level directory
> + *
> + */
> +
> +#include "virtio.h"
> +#include "qemu_socket.h"
> +#include "qemu-thread.h"
> +#include "virtio-9p.h"
> +#include 
> +#include 
> +
> +/* Structure used to transfer file descriptor and error info to the main
> + * process. fd will be zero if there was an error(in this case error
> + * will hold the errno). error will be zero and fd will be a valid
> + * identifier when open was success
> + */
> +typedef struct {
> +    int fd;
> +    int error;
> +} FdInfo;
> +
> +static int sendfd(int sockfd, FdInfo fd_info)
> +{
> +    struct msghdr msg = { };
> +    struct iovec iov;
> +    union {
> +        struct cmsghdr cmsg;
> +        char control[CMSG_SPACE(sizeof(int))];
> +    } msg_control;
> +    struct cmsghdr *cmsg;
> +
> +    iov.iov_base = &fd_info;
> +    iov.iov_len = sizeof(fd_info);
> +
> +    memset(&msg, 0, sizeof(msg));
> +    msg.msg_iov = &iov;
> +    msg.msg_iovlen = 1;
> +    msg.msg_control = &msg_control;
> +    msg.msg_controllen = sizeof(msg_control);
> +
> +    cmsg = &msg_control.cmsg;
> +    cmsg->cmsg_len = CMSG_LEN(sizeof(fd_info.fd));
> +    cmsg->cmsg_level = SOL_SOCKET;
> +    cmsg->cmsg_type = SCM_RIGHTS;
> +    memcpy(CMSG_DATA(cmsg), &fd_info.fd, sizeof(fd_info.fd));
> +
> +    return sendmsg(sockfd, &msg, 0

[Qemu-devel] Re: [PATCH v3] virtio-9p: fix build on !CONFIG_UTIMENSAT

2010-11-15 Thread M. Mohan Kumar

> This patch introduce a fallback mechanism for old systems that do not
> support utimensat().  This fix build failure with following warnings:
> 
> hw/virtio-9p-local.c: In function 'local_utimensat':
> hw/virtio-9p-local.c:479: warning: implicit declaration of function
> 'utimensat' hw/virtio-9p-local.c:479: warning: nested extern declaration
> of 'utimensat'
> 
> and:
> 
> hw/virtio-9p.c: In function 'v9fs_setattr_post_chmod':
> hw/virtio-9p.c:1410: error: 'UTIME_NOW' undeclared (first use in this
> function) hw/virtio-9p.c:1410: error: (Each undeclared identifier is
> reported only once hw/virtio-9p.c:1410: error: for each function it
> appears in.)
> hw/virtio-9p.c:1413: error: 'UTIME_OMIT' undeclared (first use in this
> function) hw/virtio-9p.c: In function 'v9fs_wstat_post_chmod':
> hw/virtio-9p.c:2905: error: 'UTIME_OMIT' undeclared (first use in this
> function)
> 
> v3:
>   - Use better alternative handling for UTIME_NOW/OMIT
>   - Move qemu_utimensat() to cutils.c
> V2:
>   - Introduce qemu_utimensat()
> 
> Signed-off-by: Hidetoshi Seto 

Looks good to me.

Acked-by: M. Mohan Kumar

[Qemu-devel] Re: [PATCH RESENT] msix: allow byte and word reading from mmio

2010-11-15 Thread Bernhard Kohl


Am 15.11.2010 11:42, schrieb ext Michael S. Tsirkin:

On Thu, Aug 19, 2010 at 02:56:51PM +0200, Bernhard Kohl wrote:
   

It's legal that the guest reads a single byte or word from mmio.
 

Interesting. The spec seems to say this:

For all accesses to MSI-X Table and MSI-X PBA fields, software must use
aligned full DWORD or aligned full QWORD transactions; otherwise, the
result is undefined.
   


I will remove the first statement from the commit message and add
something like the comment you proposed below.

   

I have an OS which reads single bytes and it works fine on real
hardware. Maybe this happens due to casting.
 

What do you mean by casting?
   


I did not yet locate the line of code where this happens in the guest.
As all other accesses are dword, I assume that there is some masking,
shifting or casting to a byte variable in the source code, and the
compiler generates a byte access from this.

   

Signed-off-by: Bernhard Kohl
 

I guess we can merge this, but we need a comment I think since this
seems to contradict the spec, and people were sending patches relying on
this.

Does something like the following describe the situation correctly?

/* Note: PCI spec requires that all MSI-X table accesses
are either DWORD or QWORD, size aligned.  Some guests seem to violate
this rule for read accesses, performing single byte reads.
Since it's easy to support this, let's do so.
Also support 16 bit size aligned reads, just in case.
  */
   


Yes, that's is exactly the situation with my guest.
I will add this comment.


Does you guest also do 16 bit reads?  Are accesses at least aligned?
   


I will check this with my guest and the readw function disabled
in the patch. This will take some time.

   

---
  hw/msix.c |   20 
  1 files changed, 16 insertions(+), 4 deletions(-)

diff --git a/hw/msix.c b/hw/msix.c
index d99403a..7dac7f7 100644
--- a/hw/msix.c
+++ b/hw/msix.c
@@ -100,10 +100,22 @@ static uint32_t msix_mmio_readl(void *opaque, 
target_phys_addr_t addr)
  return pci_get_long(page + offset);
  }

-static uint32_t msix_mmio_read_unallowed(void *opaque, target_phys_addr_t addr)
+static uint32_t msix_mmio_readw(void *opaque, target_phys_addr_t addr)
  {
-fprintf(stderr, "MSI-X: only dword read is allowed!\n");
-return 0;
+PCIDevice *dev = opaque;
+unsigned int offset = addr&  (MSIX_PAGE_SIZE - 1)&  ~0x1;
+void *page = dev->msix_table_page;
+
+return pci_get_word(page + offset);
+}
+
+static uint32_t msix_mmio_readb(void *opaque, target_phys_addr_t addr)
+{
+PCIDevice *dev = opaque;
+unsigned int offset = addr&  (MSIX_PAGE_SIZE - 1);
+void *page = dev->msix_table_page;
+
+return pci_get_byte(page + offset);
  }

  static uint8_t msix_pending_mask(int vector)
@@ -198,7 +210,7 @@ static CPUWriteMemoryFunc * const msix_mmio_write[] = {
  };

  static CPUReadMemoryFunc * const msix_mmio_read[] = {
-msix_mmio_read_unallowed, msix_mmio_read_unallowed, msix_mmio_readl
+msix_mmio_readb, msix_mmio_readw, msix_mmio_readl
  };

  /* Should be called from device's map method. */
--
1.7.2.1

Re: [Qemu-devel] Re: [PATCH] pc: disable the BOCHS BIOS panic port

2010-11-15 Thread Bernhard Kohl


Am 15.11.2010 11:09, schrieb ext Alexander Graf:

On 15.11.2010, at 10:53, Bernhard Kohl wrote:

   

Am 01.09.2010 16:44, schrieb Bernhard Kohl:
 

We have an OS which writes to port 0x400 when probing for special hardware.
This causes an exit of the VM. With SeaBIOS this port isn't used anyway.

Signed-off-by: Bernhard Kohl
---
  hw/pc.c |2 --
  1 files changed, 0 insertions(+), 2 deletions(-)

diff --git a/hw/pc.c b/hw/pc.c
index 69b13bf..3f81229 100644
--- a/hw/pc.c
+++ b/hw/pc.c
@@ -430,8 +430,6 @@ static void bochs_bios_write(void *opaque, uint32_t addr, 
uint32_t val)
  /* Bochs BIOS messages */
  case 0x400:
  case 0x401:
-fprintf(stderr, "BIOS panic at rombios.c, line %d\n", val);
-exit(1);
  case 0x402:
  case 0x403:
  #ifdef DEBUG_BIOS

   

Hi,

could you please look at this?
 

This patch makes that port that was silent before print debug output if 
DEBUG_BIOS is enabled which might be confusing.

How about something like

case 0x400:
case 0x401:
 /* used to be panic, now unused */
 break;


Alex


   

Yes, you are right. I will take your proposal in patch v2.

Bernhard

[Qemu-devel] Re: [PATCHv2 2/2] tap: mark fd handler as device


On 11/15/2010 09:18 AM, Michael S. Tsirkin wrote:

On Mon, Nov 15, 2010 at 08:55:07AM -0600, Anthony Liguori wrote:
   

On 11/15/2010 08:52 AM, Juan Quintela wrote:
 

"Michael S. Tsirkin"   wrote:
   

There's no reason for tap to run when VM is stopped.
If we let it, it confuses the bridge on TX
and corrupts DMA memory on RX.

Signed-off-by: Michael S. Tsirkin
 

once here, what handlers make sense to run while stopped?
/me can think of the normal console, non live migration, loadvm and not
much more.  Perhaps it is easier to just move the other way around?
   

I'm not sure I concur that this is really a problem.
Semantically, I don't think that stop has to imply that the guest
memory no longer changes.

Regards,

Anthony Liguori

 

Later, Juan.
   

Well, I do not really know about vmstop that is not for migration.
   


They are separate.  For instance, we don't rely on stop to pause pending 
disk I/O because we don't serialize pending disk I/O operations.  
Instead, we flush all pending I/O and rely on the fact that disk I/O 
requests are always submitted in the context of a vCPU operation.  This 
assumption breaks down though with ioeventfd so we need to revisit it.



For most vmstop calls are for migration. And there, the problems are very
real.

First, it's not just memory.  At least for network transmit, sending out
packets with the same MAC from two locations is a problem. See?
   


I agree it's a problem but I'm not sure that just marking fd handlers 
really helps.


Bottom halves can also trigger transmits.  I think that if we put 
something in the network layer that just queued packets if the vm is 
stopped, it would be a more robust solution to the problem.



For memory, it is much worse:  any memory changes can either get
discarded or not.  This breaks consistency guarantees that guest relies
upon. Imagine virtio index getting updated but content not being
updated.  See?
   


If you suppress any I/O then the memory changes don't matter because the 
same changes will happen on the destination too.


I think this basic problem is the same as Kemari.  We can either attempt 
to totally freeze a guest which means stopping all callbacks that are 
device related or we can prevent I/O from happening which should 
introduce enough determinism to fix the problem in practice.


Regards,

Anthony Liguori

Re: [Qemu-devel] [PATCH] make trace options use autoconfy names

2010-11-15 Thread Paolo Bonzini


On 11/15/2010 03:17 PM, Stefan Hajnoczi wrote:

On Sun, Nov 14, 2010 at 1:52 PM, Paolo Bonzini  wrote:

On 11/14/2010 02:38 PM, Andreas Färber wrote:


- --trace-file=*) trace_file="$optarg"
+ --enable-trace-file=*) trace_file="$optarg"
;;


but this should be --with-trace-file=... please. It is not being
enabled, just set to a different value.


--with-* should be reserved for library paths, but I can change it if people
prefer it that way.


Actually I think we have something similar to overriding --prefix here
:).  It's a path that you can set at ./configure time.


Yeah, that's true.  However...


So is it not okay to use --trace-file=?


... Autoconf would not allow unknown options not starting with --enable- 
or --with-.


The rationale to avoid incompatible options in QEMU is this: suppose you 
have a project using Autoconf (e.g. GCC) and you want to drop QEMU as a 
subdirectory in there, e.g. to run the GCC testsuite under QEMU usermode 
emulation (GCC can already do this for other simulators).  To pass 
options to QEMU's configure, you can include them in GCC's commandline. 
 The script will simply pass the option down to QEMU and it will be 
processed there.  However, if you pass --trace-file to GCC's configure 
script, it will complain and stop.


Probably I would use something like --enable-trace-backend=simple:trace- 
if I was adding something similar to an autoconfiscated project.  But 
unless it provides some additional benefit (as is the case with 
cross-compilation support) I want to keep the syntactic changes in my 
patches to the minimum.



But I know nothing of autoconf and --enable-* or --with-* sort of
make sense too.


Whatever, I have to repost the other series anyway, so I'll change to 
--with-.


Paolo

[Qemu-devel] [PATCH] Introduce -accel command option.

2010-11-15 Thread anthony . perard

From: Anthony PERARD 

This option gives the ability to switch one "accelerator" like kvm, xen
or the default one tcg. We can specify more than one accelerator by
separate them by a comma. QEMU will try each one and use the first whose
works.

So,

-accel xen,kvm,tcg

which would try Xen support first, then KVM and finaly tcg if none of
the other works.

Signed-off-by: Anthony PERARD 
---
 arch_init.c |5 +++
 arch_init.h |1 +
 qemu-options.hx |   10 ++
 vl.c|   85 +++---
 4 files changed, 90 insertions(+), 11 deletions(-)

diff --git a/arch_init.c b/arch_init.c
index 4486925..e0d7a4c 100644
--- a/arch_init.c
+++ b/arch_init.c
@@ -639,6 +639,11 @@ int audio_available(void)
 #endif
 }
 
+int tcg_available(void)
+{
+return 1;
+}
+
 int kvm_available(void)
 {
 #ifdef CONFIG_KVM
diff --git a/arch_init.h b/arch_init.h
index 682890c..f0fb6a0 100644
--- a/arch_init.h
+++ b/arch_init.h
@@ -27,6 +27,7 @@ void do_acpitable_option(const char *optarg);
 void do_smbios_option(const char *optarg);
 void cpudef_init(void);
 int audio_available(void);
+int tcg_available(void);
 int kvm_available(void);
 int xen_available(void);
 
diff --git a/qemu-options.hx b/qemu-options.hx
index 4d99a58..958d126 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -1975,6 +1975,16 @@ Enable KVM full virtualization support. This option is 
only available
 if KVM support is enabled when compiling.
 ETEXI
 
+DEF("accel", HAS_ARG, QEMU_OPTION_accel, \
+"-accel acceluse an accelerator (kvm,xen,tcg), default is tcg\n", 
QEMU_ARCH_ALL)
+STEXI
+...@item -accel @var{accel}[,@var{accel}[,...]]
+...@findex -accel
+This is use to enable an accelerator, in kvm,xen,tcg.
+By default, it use only tcg. If there a more than one accelerator
+specified, the next one is used if the first don't work.
+ETEXI
+
 DEF("xen-domid", HAS_ARG, QEMU_OPTION_xen_domid,
 "-xen-domid id   specify xen guest domain id\n", QEMU_ARCH_ALL)
 DEF("xen-create", 0, QEMU_OPTION_xen_create,
diff --git a/vl.c b/vl.c
index c58583d..2917e32 100644
--- a/vl.c
+++ b/vl.c
@@ -242,6 +242,7 @@ static void *boot_set_opaque;
 static NotifierList exit_notifiers =
 NOTIFIER_LIST_INITIALIZER(exit_notifiers);
 
+static int tcg_allowed = 1;
 int kvm_allowed = 0;
 uint32_t xen_domid;
 enum xen_mode xen_mode = XEN_EMULATE;
@@ -1723,6 +1724,72 @@ static int debugcon_parse(const char *devname)
 return 0;
 }
 
+static int tcg_init(int smp_cpus)
+{
+return 0;
+}
+
+static struct {
+const char *opt_name;
+const char *name;
+int (*available)(void);
+int (*init)(int smp_cpus);
+int *allowed;
+} accel_list[] = {
+{ "tcg", "tcg", tcg_available, tcg_init, &tcg_allowed },
+{ "kvm", "KVM", kvm_available, kvm_init, &kvm_allowed },
+};
+
+static int accel_parse_init(const char *opts)
+{
+const char *p = opts;
+char buf[10];
+int i, ret;
+bool accel_initalised = 0;
+bool init_failed = 0;
+
+while (!accel_initalised && *p != '\0') {
+if (*p == ',') {
+p++;
+}
+p = get_opt_name(buf, sizeof (buf), p, ',');
+for (i = 0; i < ARRAY_SIZE(accel_list); i++) {
+if (strcmp(accel_list[i].opt_name, buf) == 0) {
+ret = accel_list[i].init(smp_cpus);
+if (ret < 0) {
+init_failed = 1;
+if (!accel_list[i].available()) {
+printf("%s not supported for this target\n",
+   accel_list[i].name);
+} else {
+fprintf(stderr, "failed to initialize %s: %s\n",
+accel_list[i].name,
+strerror(-ret));
+}
+} else {
+accel_initalised = 1;
+*(accel_list[i].allowed) = 1;
+}
+break;
+}
+}
+if (i == ARRAY_SIZE(accel_list)) {
+fprintf(stderr, "\"%s\" accelerator does not exist.\n", buf);
+}
+}
+
+if (!accel_initalised) {
+fprintf(stderr, "No accelerator found!\n");
+exit(1);
+}
+
+if (init_failed) {
+fprintf(stderr, "Back to %s accelerator.\n", accel_list[i].name);
+}
+
+return !accel_initalised;
+}
+
 void qemu_add_exit_notifier(Notifier *notify)
 {
 notifier_list_add(&exit_notifiers, notify);
@@ -1802,6 +1869,7 @@ int main(int argc, char **argv, char **envp)
 const char *incoming = NULL;
 int show_vnc_port = 0;
 int defconfig = 1;
+const char *accel_list_opts = "tcg";
 
 #ifdef CONFIG_SIMPLE_TRACE
 const char *trace_file = NULL;
@@ -2409,7 +2477,10 @@ int main(int argc, char **argv, char **envp)
 do_smbios_option(optarg);
 break;
 case QEMU_OPTION_enable_kvm:
-kvm_allowed = 1;
+accel_list_opts = "kvm";
+

[Qemu-devel] Re: [PATCHv2 2/2] tap: mark fd handler as device

On Mon, Nov 15, 2010 at 03:52:54PM +0100, Juan Quintela wrote:
> "Michael S. Tsirkin"  wrote:
> > There's no reason for tap to run when VM is stopped.
> > If we let it, it confuses the bridge on TX
> > and corrupts DMA memory on RX.
> >
> > Signed-off-by: Michael S. Tsirkin 
> 
> once here, what handlers make sense to run while stopped?
> /me can think of the normal console, non live migration, loadvm and not
> much more.  Perhaps it is easier to just move the other way around?
> 
> Later, Juan.

vnc? SDL?
Yes, more devices need to be stopped than not, but I
tread carefully to avoid breaking existing functionality.
If you could solve it for all devices in one swoop, that'd be
great. I'm not up to it.

-- 
MST

[Qemu-devel] Re: [PATCHv2 2/2] tap: mark fd handler as device

On Mon, Nov 15, 2010 at 08:55:07AM -0600, Anthony Liguori wrote:
> On 11/15/2010 08:52 AM, Juan Quintela wrote:
> >"Michael S. Tsirkin"  wrote:
> >>There's no reason for tap to run when VM is stopped.
> >>If we let it, it confuses the bridge on TX
> >>and corrupts DMA memory on RX.
> >>
> >>Signed-off-by: Michael S. Tsirkin
> >once here, what handlers make sense to run while stopped?
> >/me can think of the normal console, non live migration, loadvm and not
> >much more.  Perhaps it is easier to just move the other way around?
> 
> I'm not sure I concur that this is really a problem.
> Semantically, I don't think that stop has to imply that the guest
> memory no longer changes.
> 
> Regards,
> 
> Anthony Liguori
> 
> >Later, Juan.

Well, I do not really know about vmstop that is not for migration.
For most vmstop calls are for migration. And there, the problems are very
real.

First, it's not just memory.  At least for network transmit, sending out
packets with the same MAC from two locations is a problem. See?

For memory, it is much worse:  any memory changes can either get
discarded or not.  This breaks consistency guarantees that guest relies
upon. Imagine virtio index getting updated but content not being
updated.  See?

-- 
MST

[Qemu-devel] [PATCH 2/2] virtio-9p: Use chroot interface in passthrough model

2010-11-15 Thread M. Mohan Kumar

Make use of chroot interfaces for passthrough security model to fix the
vulnerability in following symbolic links.

Signed-off-by: M. Mohan Kumar 
---
 hw/virtio-9p-local.c |  284 ++
 1 files changed, 218 insertions(+), 66 deletions(-)

diff --git a/hw/virtio-9p-local.c b/hw/virtio-9p-local.c
index 656bfb3..4b72dec 100644
--- a/hw/virtio-9p-local.c
+++ b/hw/virtio-9p-local.c
@@ -19,16 +19,91 @@
 #include 
 #include 
 #include 
+#include 
+
+static int get_fd(FsContext *fs_ctx, const char *path, int flags, FsCred 
*credp)
+{
+V9fsOpenRequest request;
+int fd, error = 0;
+
+memset(&request, 0, sizeof(request));
+request.data.path_len = strlen(path);
+request.path.path = qemu_strdup(path);
+request.data.flags = flags;
+if (credp) {
+request.data.mode = credp->fc_mode;
+request.data.uid = credp->fc_uid;
+request.data.gid = credp->fc_gid;
+request.data.dev = credp->fc_rdev;
+}
+fd = v9fs_getfd(&request, &error, fs_ctx);
+if (error) {
+errno = error;
+} else {
+errno = error;
+}
+qemu_strdup(request.path.path);
+return fd;
+}
+
+static int get_pfd(FsContext *fs_ctx, const char *path)
+{
+V9fsOpenRequest request;
+int fd, error = 0;
+char *dpath = qemu_strdup(path);
+
+memset(&request, 0, sizeof(request));
+request.path.path = dirname(dpath);
+request.data.path_len = strlen(request.path.path);
+request.data.flags = O_RDONLY | O_DIRECTORY | O_NOFOLLOW;
+fd = v9fs_getfd(&request, &error, fs_ctx);
+if (error) {
+errno = error;
+} else {
+errno = 0;
+}
+qemu_free(dpath);
+return fd;
+}
 
+static int do_symlink(FsContext *fs_ctx, const char *oldpath,
+const char *newpath, FsCred *credp)
+{
+V9fsOpenRequest request;
+int fd, error = 0;
+
+memset(&request, 0, sizeof(request));
+request.data.path_len = strlen(newpath);
+request.path.path = qemu_strdup(newpath);
+request.data.oldpath_len = strlen(oldpath);
+request.path.old_path = qemu_strdup(oldpath);
+request.data.flags = S_IFLNK | O_CREAT;
+
+if (credp) {
+request.data.mode = credp->fc_mode;
+request.data.uid = credp->fc_uid;
+request.data.gid = credp->fc_gid;
+request.data.dev = credp->fc_rdev;
+}
+fd = v9fs_getfd(&request, &error, fs_ctx);
+if (error) {
+errno = error;
+} else {
+errno = error;
+}
+qemu_strdup(request.path.path);
+return fd;
+}
 
 static int local_lstat(FsContext *fs_ctx, const char *path, struct stat *stbuf)
 {
 int err;
-err =  lstat(rpath(fs_ctx, path), stbuf);
-if (err) {
-return err;
-}
+
 if (fs_ctx->fs_sm == SM_MAPPED) {
+err =  lstat(rpath(fs_ctx, path), stbuf);
+if (err) {
+return err;
+}
 /* Actual credentials are part of extended attrs */
 uid_t tmp_uid;
 gid_t tmp_gid;
@@ -50,6 +125,22 @@ static int local_lstat(FsContext *fs_ctx, const char *path, 
struct stat *stbuf)
 sizeof(dev_t)) > 0) {
 stbuf->st_rdev = tmp_dev;
 }
+} else if (fs_ctx->fs_sm == SM_PASSTHROUGH) {
+int pfd;
+char *base, *basep;
+
+base = qemu_strdup(path);
+basep = basename(base);
+
+pfd = get_pfd(fs_ctx, path);
+err = fstatat(pfd, basep, stbuf, AT_SYMLINK_NOFOLLOW);
+close(pfd);
+free(base);
+} else {
+err =  lstat(rpath(fs_ctx, path), stbuf);
+if (err) {
+return err;
+}
 }
 return err;
 }
@@ -88,21 +179,13 @@ static int local_set_xattr(const char *path, FsCred *credp)
 return 0;
 }
 
-static int local_post_create_passthrough(FsContext *fs_ctx, const char *path,
-FsCred *credp)
+static int local_post_create_none(FsContext *fs_ctx, const char *path,
+FsCred *credp)
 {
 if (chmod(rpath(fs_ctx, path), credp->fc_mode & 0) < 0) {
 return -1;
 }
-if (lchown(rpath(fs_ctx, path), credp->fc_uid, credp->fc_gid) < 0) {
-/*
- * If we fail to change ownership and if we are
- * using security model none. Ignore the error
- */
-if (fs_ctx->fs_sm != SM_NONE) {
-return -1;
-}
-}
+lchown(rpath(fs_ctx, path), credp->fc_uid, credp->fc_gid);
 return 0;
 }
 
@@ -121,9 +204,16 @@ static ssize_t local_readlink(FsContext *fs_ctx, const 
char *path,
 } while (tsize == -1 && errno == EINTR);
 close(fd);
 return tsize;
-} else if ((fs_ctx->fs_sm == SM_PASSTHROUGH) ||
-   (fs_ctx->fs_sm == SM_NONE)) {
+} else if (fs_ctx->fs_sm == SM_NONE) {
 tsize = readlink(rpath(fs_ctx, path), buf, bufsz);
+} else if (fs_ctx->fs_sm == SM_PASSTHROUGH) {
+int pfd;
+char *base;
+base = qemu_strdup(path);
+pfd =

[Qemu-devel] Re: [PATCHv2 2/2] tap: mark fd handler as device


On 11/15/2010 08:52 AM, Juan Quintela wrote:

"Michael S. Tsirkin"  wrote:
   

There's no reason for tap to run when VM is stopped.
If we let it, it confuses the bridge on TX
and corrupts DMA memory on RX.

Signed-off-by: Michael S. Tsirkin
 

once here, what handlers make sense to run while stopped?
/me can think of the normal console, non live migration, loadvm and not
much more.  Perhaps it is easier to just move the other way around?
   


I'm not sure I concur that this is really a problem.

Semantically, I don't think that stop has to imply that the guest memory 
no longer changes.


Regards,

Anthony Liguori


Later, Juan.

[Qemu-devel] [PATCH 1/2] virtio-9p: Use chroot to safely access files in passthrough model

2010-11-15 Thread M. Mohan Kumar

In passthrough security model, following symbolic links in the server
side could result in accessing files outside guest's export path.This
could happen under two conditions:
1) If a modified guest kernel is sending symbolic link as part of the
file path and when resolving that symbolic link at server side, it could
result in accessing files outside export path.
2) If a same path is exported to multiple guests and if guest1 tries to
open a file "a/b/c/passwd" and meanwhile guest2 did this "rm -rf a/b/c;
cd a/b; ln -s ../../etc c". If guest1 lookup happened and guest2
completed these operations just before guest1 opening the file, this
operation could result in opening host's /etc/passwd.

Following approach is used to avoid the security issue involved in
following symbolic links in the passthrough model. Create a sub-process
which will chroot into export path, so that even if there is a symbolic
link in the path it could never go beyond the share path.

When qemu is started with passthrough security model, a process is
forked and this sub-process process takes care of accessing files in the
passthrough share path. It does
 * Create socketpair
 * Chroot into share path
 * Read file open request from socket descriptor
 * Open request contains file path, flags, mode, uid, gid, dev etc
 * Based on the request type it creates/opens file/directory/device node
 * Return the file descriptor to main process using socket with
   SCM_RIGHTS as cmsg type.

Main process when ever there is a request for a resource to be
opened/created, it constructs the open request and writes that into
its socket descriptor and reads from chroot process socket to get the
file descriptor.

This patch implements chroot enviroment, provides necessary functions
that can be used by the passthrough function calls.

Signed-off-by: M. Mohan Kumar 
---
 Makefile.objs |1 +
 hw/file-op-9p.h   |2 +
 hw/virtio-9p-chroot.c |  275 +
 hw/virtio-9p.c|   20 
 hw/virtio-9p.h|   21 
 5 files changed, 319 insertions(+), 0 deletions(-)
 create mode 100644 hw/virtio-9p-chroot.c

diff --git a/Makefile.objs b/Makefile.objs
index cd5a24b..134da8e 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -251,6 +251,7 @@ hw-obj-$(CONFIG_SOUND) += $(sound-obj-y)
 
 hw-obj-$(CONFIG_VIRTFS) += virtio-9p-debug.o virtio-9p-local.o 
virtio-9p-xattr.o
 hw-obj-$(CONFIG_VIRTFS) += virtio-9p-xattr-user.o virtio-9p-posix-acl.o
+hw-obj-$(CONFIG_VIRTFS) += virtio-9p-chroot.o
 
 ##
 # libdis
diff --git a/hw/file-op-9p.h b/hw/file-op-9p.h
index c7731c2..149a915 100644
--- a/hw/file-op-9p.h
+++ b/hw/file-op-9p.h
@@ -55,6 +55,8 @@ typedef struct FsContext
 SecModel fs_sm;
 uid_t uid;
 struct xattr_operations **xops;
+pthread_mutex_t chroot_mutex;
+int chroot_socket;
 } FsContext;
 
 extern void cred_init(FsCred *);
diff --git a/hw/virtio-9p-chroot.c b/hw/virtio-9p-chroot.c
new file mode 100644
index 000..50e28a1
--- /dev/null
+++ b/hw/virtio-9p-chroot.c
@@ -0,0 +1,275 @@
+/*
+ * Virtio 9p chroot environment for secured access to exported file
+ * system
+ *
+ * Copyright IBM, Corp. 2010
+ *
+ * Authors:
+ * M. Mohan Kumar 
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2. See
+ * the copying file in the top-level directory
+ *
+ */
+
+#include "virtio.h"
+#include "qemu_socket.h"
+#include "qemu-thread.h"
+#include "virtio-9p.h"
+#include 
+#include 
+
+/* Structure used to transfer file descriptor and error info to the main
+ * process. fd will be zero if there was an error(in this case error
+ * will hold the errno). error will be zero and fd will be a valid
+ * identifier when open was success
+ */
+typedef struct {
+int fd;
+int error;
+} FdInfo;
+
+static int sendfd(int sockfd, FdInfo fd_info)
+{
+struct msghdr msg = { };
+struct iovec iov;
+union {
+struct cmsghdr cmsg;
+char control[CMSG_SPACE(sizeof(int))];
+} msg_control;
+struct cmsghdr *cmsg;
+
+iov.iov_base = &fd_info;
+iov.iov_len = sizeof(fd_info);
+
+memset(&msg, 0, sizeof(msg));
+msg.msg_iov = &iov;
+msg.msg_iovlen = 1;
+msg.msg_control = &msg_control;
+msg.msg_controllen = sizeof(msg_control);
+
+cmsg = &msg_control.cmsg;
+cmsg->cmsg_len = CMSG_LEN(sizeof(fd_info.fd));
+cmsg->cmsg_level = SOL_SOCKET;
+cmsg->cmsg_type = SCM_RIGHTS;
+memcpy(CMSG_DATA(cmsg), &fd_info.fd, sizeof(fd_info.fd));
+
+return sendmsg(sockfd, &msg, 0);
+}
+
+static int getfd(int sockfd, int *error)
+{
+struct msghdr msg = { };
+struct iovec iov;
+union {
+struct cmsghdr cmsg;
+char control[CMSG_SPACE(sizeof(int))];
+} msg_control;
+struct cmsghdr *cmsg;
+int retval, fd;
+FdInfo fd_info;
+
+iov.iov_base = &fd_info;
+iov.iov_len = sizeof(fd_info);
+
+memset(&msg, 0, sizeof(msg));
+msg.msg_iov =

[Qemu-devel] Re: [PATCHv2 2/2] tap: mark fd handler as device

2010-11-15 Thread Juan Quintela

"Michael S. Tsirkin"  wrote:
> There's no reason for tap to run when VM is stopped.
> If we let it, it confuses the bridge on TX
> and corrupts DMA memory on RX.
>
> Signed-off-by: Michael S. Tsirkin 

once here, what handlers make sense to run while stopped?
/me can think of the normal console, non live migration, loadvm and not
much more.  Perhaps it is easier to just move the other way around?

Later, Juan.

Re: [Qemu-devel] [PATCH V6 04/15] Introduce -accel command option.

2010-11-15 Thread Anthony PERARD

On Mon, 15 Nov 2010, Anthony PERARD wrote:

> On Mon, 15 Nov 2010, Alexander Graf wrote:
>
> >
> > On 21.10.2010, at 19:36, anthony.per...@citrix.com wrote:
> >
> > > From: Anthony PERARD 
> > >
> > > This option gives the ability to switch one "accelerator" like kvm, xen
> > > or the default one tcg. We can specify more than one accelerator by
> > > separate them by a comma. QEMU will try each one and use the first whose
> > > works.
> > >
> > > So,
> > >
> > > -accel xen,kvm,tcg
> > >
> > > which would try Xen support first, then KVM and finaly tcg if none of
> > > the other works.
> > >
> > > Signed-off-by: Anthony PERARD 
> > > ---
> > > qemu-options.hx |   10 ++
> > > vl.c|   86 
> > > ---
> > > 2 files changed, 85 insertions(+), 11 deletions(-)
> > >
> > > diff --git a/vl.c b/vl.c
> > > index df414ef..40a26ee 100644
> > > --- a/vl.c
> > > +++ b/vl.c
> > > @@ -1750,6 +1750,74 @@ static int debugcon_parse(const char *devname)
> > > return 0;
> > > }
> > >
> > > +static struct {
> > > +const char *opt_name;
> > > +const char *name;
> > > +int (*available)(void);
> > > +int (*init)(int smp_cpus);
> > > +int *allowed;
> > > +} accel_list[] = {
> > > +{ "tcg", "tcg", NULL, NULL, NULL },
> > > +{ "kvm", "KVM", kvm_available, kvm_init, &kvm_allowed },
> >
> > Thinking about this a bit more ...
> >
> > kvm_available is a function pointer that gets #defined to (0) when we don't 
> > have KVM available. I can imagine some compiler might throw a warning on us 
> > for this one day.
> >
> > Is there a valid reason not to do
> >
> > static inline int kvm_enabled()
> > {
> > #ifdef CONFIG_KVM
> > return kvm_allowed;
> > #else
> > return 0;
> > #endif
> > }
> >
> > That should compile into the exact same code but be valid for function 
> > pointers.
>
> I will do this change, as well as for the two others patches.

Actually, kvm_available is already a function, not a define.

kvm_enable can be change from define to function, but not in this patch,
because I don't use it.

-- 
Anthony PERARD

Re: [Qemu-devel] [PATCH V6 04/15] Introduce -accel command option.

2010-11-15 Thread Alexander Graf

On 15.11.2010, at 15:47, Anthony PERARD wrote:

> On Mon, 15 Nov 2010, Anthony PERARD wrote:
> 
>> On Mon, 15 Nov 2010, Alexander Graf wrote:
>> 
>>> 
>>> On 21.10.2010, at 19:36, anthony.per...@citrix.com wrote:
>>> 
 From: Anthony PERARD 

 This option gives the ability to switch one "accelerator" like kvm, xen
 or the default one tcg. We can specify more than one accelerator by
 separate them by a comma. QEMU will try each one and use the first whose
 works.

 So,

 -accel xen,kvm,tcg

 which would try Xen support first, then KVM and finaly tcg if none of
 the other works.

 Signed-off-by: Anthony PERARD 
 ---
 qemu-options.hx |   10 ++
 vl.c|   86 
 ---
 2 files changed, 85 insertions(+), 11 deletions(-)

 diff --git a/vl.c b/vl.c
 index df414ef..40a26ee 100644
 --- a/vl.c
 +++ b/vl.c
 @@ -1750,6 +1750,74 @@ static int debugcon_parse(const char *devname)
return 0;
 }

 +static struct {
 +const char *opt_name;
 +const char *name;
 +int (*available)(void);
 +int (*init)(int smp_cpus);
 +int *allowed;
 +} accel_list[] = {
 +{ "tcg", "tcg", NULL, NULL, NULL },
 +{ "kvm", "KVM", kvm_available, kvm_init, &kvm_allowed },
>>> 
>>> Thinking about this a bit more ...
>>> 
>>> kvm_available is a function pointer that gets #defined to (0) when we don't 
>>> have KVM available. I can imagine some compiler might throw a warning on us 
>>> for this one day.
>>> 
>>> Is there a valid reason not to do
>>> 
>>> static inline int kvm_enabled()
>>> {
>>> #ifdef CONFIG_KVM
>>>return kvm_allowed;
>>> #else
>>>return 0;
>>> #endif
>>> }
>>> 
>>> That should compile into the exact same code but be valid for function 
>>> pointers.
>> 
>> I will do this change, as well as for the two others patches.
> 
> Actually, kvm_available is already a function, not a define.
> 
> kvm_enable can be change from define to function, but not in this patch,
> because I don't use it.

Oh no worries for stuff you don't use. As long as kvm_available is a function, 
just make sure that xen_available is one too :). The main incentive is to get 
rid of all those if (!x->available) parts, as we can guarantee there to always 
be a function behind. 

Treating tcg the same way would also help in the long term for non-tcg build of 
qemu which some people are interested in ;).

Alex

[Qemu-devel] [PATCHv5 14/15] Add notifier that will be called when machine is fully created.

Action that depends on fully initialized device model should register
with this notifier chain.

Signed-off-by: Gleb Natapov 
---
 sysemu.h |2 ++
 vl.c |   15 +++
 2 files changed, 17 insertions(+), 0 deletions(-)

diff --git a/sysemu.h b/sysemu.h
index 48f8eee..c42f33a 100644
--- a/sysemu.h
+++ b/sysemu.h
@@ -60,6 +60,8 @@ void qemu_system_reset(void);
 void qemu_add_exit_notifier(Notifier *notify);
 void qemu_remove_exit_notifier(Notifier *notify);
 
+void qemu_add_machine_init_done_notifier(Notifier *notify);
+
 void do_savevm(Monitor *mon, const QDict *qdict);
 int load_vmstate(const char *name);
 void do_delvm(Monitor *mon, const QDict *qdict);
diff --git a/vl.c b/vl.c
index e8ada75..918d988 100644
--- a/vl.c
+++ b/vl.c
@@ -253,6 +253,9 @@ static void *boot_set_opaque;
 static NotifierList exit_notifiers =
 NOTIFIER_LIST_INITIALIZER(exit_notifiers);
 
+static NotifierList machine_init_done_notifiers =
+NOTIFIER_LIST_INITIALIZER(machine_init_done_notifiers);
+
 int kvm_allowed = 0;
 uint32_t xen_domid;
 enum xen_mode xen_mode = XEN_EMULATE;
@@ -1778,6 +1781,16 @@ static void qemu_run_exit_notifiers(void)
 notifier_list_notify(&exit_notifiers);
 }
 
+void qemu_add_machine_init_done_notifier(Notifier *notify)
+{
+notifier_list_add(&machine_init_done_notifiers, notify);
+}
+
+static void qemu_run_machine_init_done_notifiers(void)
+{
+notifier_list_notify(&machine_init_done_notifiers);
+}
+
 static const QEMUOption *lookup_opt(int argc, char **argv,
 const char **poptarg, int *poptind)
 {
@@ -3023,6 +3036,8 @@ int main(int argc, char **argv, char **envp)
 exit(1);
 }
 
+qemu_run_machine_init_done_notifiers();
+
 qemu_system_reset();
 if (loadvm) {
 if (load_vmstate(loadvm) < 0) {
-- 
1.7.1

[Qemu-devel] [PATCHv5 11/15] Add bootindex parameter to net/block/fd device

If bootindex is specified on command line a string that describes device
in firmware readable way is added into sorted list. Later this list will
be passed into firmware to control boot order.

Signed-off-by: Gleb Natapov 
---
 block_int.h |4 +++-
 hw/e1000.c  |4 
 hw/eepro100.c   |3 +++
 hw/fdc.c|8 
 hw/ide/qdev.c   |5 +
 hw/ne2000.c |3 +++
 hw/pcnet.c  |4 
 hw/qdev.c   |   32 
 hw/qdev.h   |1 +
 hw/rtl8139.c|4 
 hw/usb-net.c|2 ++
 hw/virtio-blk.c |2 ++
 hw/virtio-net.c |2 ++
 net.h   |4 +++-
 sysemu.h|2 ++
 vl.c|   40 
 16 files changed, 118 insertions(+), 2 deletions(-)

diff --git a/block_int.h b/block_int.h
index 3c3adb5..0a0e47d 100644
--- a/block_int.h
+++ b/block_int.h
@@ -227,6 +227,7 @@ typedef struct BlockConf {
 uint16_t logical_block_size;
 uint16_t min_io_size;
 uint32_t opt_io_size;
+int32_t bootindex;
 } BlockConf;
 
 static inline unsigned int get_physical_block_exp(BlockConf *conf)
@@ -249,6 +250,7 @@ static inline unsigned int get_physical_block_exp(BlockConf 
*conf)
 DEFINE_PROP_UINT16("physical_block_size", _state,   \
_conf.physical_block_size, 512), \
 DEFINE_PROP_UINT16("min_io_size", _state, _conf.min_io_size, 0),  \
-DEFINE_PROP_UINT32("opt_io_size", _state, _conf.opt_io_size, 0)
+DEFINE_PROP_UINT32("opt_io_size", _state, _conf.opt_io_size, 0),\
+DEFINE_PROP_INT32("bootindex", _state, _conf.bootindex, -1) \
 
 #endif /* BLOCK_INT_H */
diff --git a/hw/e1000.c b/hw/e1000.c
index 532efdc..053f33e 100644
--- a/hw/e1000.c
+++ b/hw/e1000.c
@@ -30,6 +30,7 @@
 #include "net.h"
 #include "net/checksum.h"
 #include "loader.h"
+#include "sysemu.h"
 
 #include "e1000_hw.h"
 
@@ -1148,6 +1149,9 @@ static int pci_e1000_init(PCIDevice *pci_dev)
   d->dev.qdev.info->name, d->dev.qdev.id, d);
 
 qemu_format_nic_info_str(&d->nic->nc, macaddr);
+
+add_boot_device_path(d->conf.bootindex, &pci_dev->qdev, "ethernet-...@0");
+
 return 0;
 }
 
diff --git a/hw/eepro100.c b/hw/eepro100.c
index 41d792a..80adac6 100644
--- a/hw/eepro100.c
+++ b/hw/eepro100.c
@@ -46,6 +46,7 @@
 #include "pci.h"
 #include "net.h"
 #include "eeprom93xx.h"
+#include "sysemu.h"
 
 #define KiB 1024
 
@@ -1907,6 +1908,8 @@ static int e100_nic_init(PCIDevice *pci_dev)
 s->vmstate->name = s->nic->nc.model;
 vmstate_register(&pci_dev->qdev, -1, s->vmstate, s);
 
+add_boot_device_path(s->conf.bootindex, &pci_dev->qdev, "ethernet-...@0");
+
 return 0;
 }
 
diff --git a/hw/fdc.c b/hw/fdc.c
index 5ab754b..7b1349f 100644
--- a/hw/fdc.c
+++ b/hw/fdc.c
@@ -35,6 +35,7 @@
 #include "sysbus.h"
 #include "qdev-addr.h"
 #include "blockdev.h"
+#include "sysemu.h"
 
 //
 /* debug Floppy devices */
@@ -523,6 +524,8 @@ typedef struct FDCtrlSysBus {
 typedef struct FDCtrlISABus {
 ISADevice busdev;
 struct FDCtrl state;
+int32_t bootindexA;
+int32_t bootindexB;
 } FDCtrlISABus;
 
 static uint32_t fdctrl_read (void *opaque, uint32_t reg)
@@ -1992,6 +1995,9 @@ static int isabus_fdc_init1(ISADevice *dev)
 qdev_set_legacy_instance_id(&dev->qdev, iobase, 2);
 ret = fdctrl_init_common(fdctrl);
 
+add_boot_device_path(isa->bootindexA, &dev->qdev, "flo...@0");
+add_boot_device_path(isa->bootindexB, &dev->qdev, "flo...@1");
+
 return ret;
 }
 
@@ -2051,6 +2057,8 @@ static ISADeviceInfo isa_fdc_info = {
 .qdev.props = (Property[]) {
 DEFINE_PROP_DRIVE("driveA", FDCtrlISABus, state.drives[0].bs),
 DEFINE_PROP_DRIVE("driveB", FDCtrlISABus, state.drives[1].bs),
+DEFINE_PROP_INT32("bootindexA", FDCtrlISABus, bootindexA, -1),
+DEFINE_PROP_INT32("bootindexB", FDCtrlISABus, bootindexB, -1),
 DEFINE_PROP_END_OF_LIST(),
 },
 };
diff --git a/hw/ide/qdev.c b/hw/ide/qdev.c
index 01a181b..69a00e2 100644
--- a/hw/ide/qdev.c
+++ b/hw/ide/qdev.c
@@ -21,6 +21,7 @@
 #include "qemu-error.h"
 #include 
 #include "blockdev.h"
+#include "sysemu.h"
 
 /* - */
 
@@ -143,6 +144,10 @@ static int ide_drive_initfn(IDEDevice *dev)
 if (!dev->serial) {
 dev->serial = qemu_strdup(s->drive_serial_str);
 }
+
+add_boot_device_path(dev->conf.bootindex, &dev->qdev,
+ dev->unit ? "d...@1" : "d...@0");
+
 return 0;
 }
 
diff --git a/hw/ne2000.c b/hw/ne2000.c
index 126e7cf..f4bbac2 100644
--- a/hw/ne2000.c
+++ b/hw/ne2000.c
@@ -26,6 +26,7 @@
 #include "net.h"
 #include "ne2000.h"
 #include "loader.h"
+#include "sysemu.h"
 
 /* debug NE2000 card */
 //#define DEBUG_NE2000
@@ -746,6 +747,8 @@ static int pci_ne2000_init(PCIDevice *pci_dev)
 }
 }
 
+add_boot_device_path(s->c.bootindex, &pci_dev->qdev, "

[Qemu-devel] [PATCHv5 09/15] Record which USBDevice USBPort belongs too.

Ports on root hub will have NULL here. This is needed to reconstruct
path from device to its root hub to build device path.

Signed-off-by: Gleb Natapov 
---
 hw/usb-bus.c  |3 ++-
 hw/usb-hub.c  |2 +-
 hw/usb-musb.c |2 +-
 hw/usb-ohci.c |2 +-
 hw/usb-uhci.c |2 +-
 hw/usb.h  |3 ++-
 6 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/hw/usb-bus.c b/hw/usb-bus.c
index b692503..256b881 100644
--- a/hw/usb-bus.c
+++ b/hw/usb-bus.c
@@ -110,11 +110,12 @@ USBDevice *usb_create_simple(USBBus *bus, const char 
*name)
 }
 
 void usb_register_port(USBBus *bus, USBPort *port, void *opaque, int index,
-   usb_attachfn attach)
+   USBDevice *pdev, usb_attachfn attach)
 {
 port->opaque = opaque;
 port->index = index;
 port->attach = attach;
+port->pdev = pdev;
 QTAILQ_INSERT_TAIL(&bus->free, port, next);
 bus->nfree++;
 }
diff --git a/hw/usb-hub.c b/hw/usb-hub.c
index 8e3a96b..8a3f829 100644
--- a/hw/usb-hub.c
+++ b/hw/usb-hub.c
@@ -535,7 +535,7 @@ static int usb_hub_initfn(USBDevice *dev)
 for (i = 0; i < s->nb_ports; i++) {
 port = &s->ports[i];
 usb_register_port(usb_bus_from_device(dev),
-  &port->port, s, i, usb_hub_attach);
+  &port->port, s, i, &s->dev, usb_hub_attach);
 port->wPortStatus = PORT_STAT_POWER;
 port->wPortChange = 0;
 }
diff --git a/hw/usb-musb.c b/hw/usb-musb.c
index 7f15842..9efe7a6 100644
--- a/hw/usb-musb.c
+++ b/hw/usb-musb.c
@@ -343,7 +343,7 @@ struct MUSBState {
 }
 
 usb_bus_new(&s->bus, NULL /* FIXME */);
-usb_register_port(&s->bus, &s->port, s, 0, musb_attach);
+usb_register_port(&s->bus, &s->port, s, 0, NULL, musb_attach);
 
 return s;
 }
diff --git a/hw/usb-ohci.c b/hw/usb-ohci.c
index c60fd8d..59604cf 100644
--- a/hw/usb-ohci.c
+++ b/hw/usb-ohci.c
@@ -1705,7 +1705,7 @@ static void usb_ohci_init(OHCIState *ohci, DeviceState 
*dev,
 usb_bus_new(&ohci->bus, dev);
 ohci->num_ports = num_ports;
 for (i = 0; i < num_ports; i++) {
-usb_register_port(&ohci->bus, &ohci->rhport[i].port, ohci, i, 
ohci_attach);
+usb_register_port(&ohci->bus, &ohci->rhport[i].port, ohci, i, NULL, 
ohci_attach);
 }
 
 ohci->async_td = 0;
diff --git a/hw/usb-uhci.c b/hw/usb-uhci.c
index 1d83400..b9b822f 100644
--- a/hw/usb-uhci.c
+++ b/hw/usb-uhci.c
@@ -1115,7 +1115,7 @@ static int usb_uhci_common_initfn(UHCIState *s)
 
 usb_bus_new(&s->bus, &s->dev.qdev);
 for(i = 0; i < NB_PORTS; i++) {
-usb_register_port(&s->bus, &s->ports[i].port, s, i, uhci_attach);
+usb_register_port(&s->bus, &s->ports[i].port, s, i, NULL, uhci_attach);
 }
 s->frame_timer = qemu_new_timer(vm_clock, uhci_frame_timer, s);
 s->expire_time = qemu_get_clock(vm_clock) +
diff --git a/hw/usb.h b/hw/usb.h
index 00d2802..0b32d77 100644
--- a/hw/usb.h
+++ b/hw/usb.h
@@ -203,6 +203,7 @@ struct USBPort {
 USBDevice *dev;
 usb_attachfn attach;
 void *opaque;
+USBDevice *pdev;
 int index; /* internal port index, may be used with the opaque */
 QTAILQ_ENTRY(USBPort) next;
 };
@@ -312,7 +313,7 @@ USBDevice *usb_create(USBBus *bus, const char *name);
 USBDevice *usb_create_simple(USBBus *bus, const char *name);
 USBDevice *usbdevice_create(const char *cmdline);
 void usb_register_port(USBBus *bus, USBPort *port, void *opaque, int index,
-   usb_attachfn attach);
+   USBDevice *pdev, usb_attachfn attach);
 void usb_unregister_port(USBBus *bus, USBPort *port);
 int usb_device_attach(USBDevice *dev);
 int usb_device_detach(USBDevice *dev);
-- 
1.7.1

[Qemu-devel] [PATCHv5 05/15] Store IDE bus id in IDEBus structure for easy access.


Signed-off-by: Gleb Natapov 
---
 hw/ide/cmd646.c   |4 ++--
 hw/ide/internal.h |3 ++-
 hw/ide/isa.c  |2 +-
 hw/ide/piix.c |4 ++--
 hw/ide/qdev.c |3 ++-
 hw/ide/via.c  |4 ++--
 6 files changed, 11 insertions(+), 9 deletions(-)

diff --git a/hw/ide/cmd646.c b/hw/ide/cmd646.c
index ff80dd5..b2cbdbc 100644
--- a/hw/ide/cmd646.c
+++ b/hw/ide/cmd646.c
@@ -257,8 +257,8 @@ static int pci_cmd646_ide_initfn(PCIDevice *dev)
 pci_conf[PCI_INTERRUPT_PIN] = 0x01; // interrupt on pin 1
 
 irq = qemu_allocate_irqs(cmd646_set_irq, d, 2);
-ide_bus_new(&d->bus[0], &d->dev.qdev);
-ide_bus_new(&d->bus[1], &d->dev.qdev);
+ide_bus_new(&d->bus[0], &d->dev.qdev, 0);
+ide_bus_new(&d->bus[1], &d->dev.qdev, 1);
 ide_init2(&d->bus[0], irq[0]);
 ide_init2(&d->bus[1], irq[1]);
 
diff --git a/hw/ide/internal.h b/hw/ide/internal.h
index d652e06..c0a1abc 100644
--- a/hw/ide/internal.h
+++ b/hw/ide/internal.h
@@ -448,6 +448,7 @@ struct IDEBus {
 IDEDevice *slave;
 BMDMAState *bmdma;
 IDEState ifs[2];
+int bus_id;
 uint8_t unit;
 uint8_t cmd;
 qemu_irq irq;
@@ -565,7 +566,7 @@ void ide_init2_with_non_qdev_drives(IDEBus *bus, DriveInfo 
*hd0,
 void ide_init_ioport(IDEBus *bus, int iobase, int iobase2);
 
 /* hw/ide/qdev.c */
-void ide_bus_new(IDEBus *idebus, DeviceState *dev);
+void ide_bus_new(IDEBus *idebus, DeviceState *dev, int bus_id);
 IDEDevice *ide_create_drive(IDEBus *bus, int unit, DriveInfo *drive);
 
 #endif /* HW_IDE_INTERNAL_H */
diff --git a/hw/ide/isa.c b/hw/ide/isa.c
index 4206afd..8c59c5a 100644
--- a/hw/ide/isa.c
+++ b/hw/ide/isa.c
@@ -67,7 +67,7 @@ static int isa_ide_initfn(ISADevice *dev)
 {
 ISAIDEState *s = DO_UPCAST(ISAIDEState, dev, dev);
 
-ide_bus_new(&s->bus, &s->dev.qdev);
+ide_bus_new(&s->bus, &s->dev.qdev, 0);
 ide_init_ioport(&s->bus, s->iobase, s->iobase2);
 isa_init_irq(dev, &s->irq, s->isairq);
 isa_init_ioport_range(dev, s->iobase, 8);
diff --git a/hw/ide/piix.c b/hw/ide/piix.c
index 07483e8..d0b04a3 100644
--- a/hw/ide/piix.c
+++ b/hw/ide/piix.c
@@ -129,8 +129,8 @@ static int pci_piix_ide_initfn(PCIIDEState *d)
 
 vmstate_register(&d->dev.qdev, 0, &vmstate_ide_pci, d);
 
-ide_bus_new(&d->bus[0], &d->dev.qdev);
-ide_bus_new(&d->bus[1], &d->dev.qdev);
+ide_bus_new(&d->bus[0], &d->dev.qdev, 0);
+ide_bus_new(&d->bus[1], &d->dev.qdev, 1);
 ide_init_ioport(&d->bus[0], 0x1f0, 0x3f6);
 ide_init_ioport(&d->bus[1], 0x170, 0x376);
 
diff --git a/hw/ide/qdev.c b/hw/ide/qdev.c
index 6d27b60..88ff657 100644
--- a/hw/ide/qdev.c
+++ b/hw/ide/qdev.c
@@ -29,9 +29,10 @@ static struct BusInfo ide_bus_info = {
 .size  = sizeof(IDEBus),
 };
 
-void ide_bus_new(IDEBus *idebus, DeviceState *dev)
+void ide_bus_new(IDEBus *idebus, DeviceState *dev, int bus_id)
 {
 qbus_create_inplace(&idebus->qbus, &ide_bus_info, dev, NULL);
+idebus->bus_id = bus_id;
 }
 
 static int ide_qdev_init(DeviceState *qdev, DeviceInfo *base)
diff --git a/hw/ide/via.c b/hw/ide/via.c
index b2c7cad..cc48b2b 100644
--- a/hw/ide/via.c
+++ b/hw/ide/via.c
@@ -158,8 +158,8 @@ static int vt82c686b_ide_initfn(PCIDevice *dev)
 
 vmstate_register(&dev->qdev, 0, &vmstate_ide_pci, d);
 
-ide_bus_new(&d->bus[0], &d->dev.qdev);
-ide_bus_new(&d->bus[1], &d->dev.qdev);
+ide_bus_new(&d->bus[0], &d->dev.qdev, 0);
+ide_bus_new(&d->bus[1], &d->dev.qdev, 1);
 ide_init2(&d->bus[0], isa_reserve_irq(14));
 ide_init2(&d->bus[1], isa_reserve_irq(15));
 ide_init_ioport(&d->bus[0], 0x1f0, 0x3f6);
-- 
1.7.1

[Qemu-devel] [PATCHv5 03/15] Keep track of ISA ports ISA device is using in qdev.

Store all io ports used by device in ISADevice structure.

Signed-off-by: Gleb Natapov 
---
 hw/cs4231a.c |1 +
 hw/fdc.c |3 +++
 hw/gus.c |4 
 hw/ide/isa.c |2 ++
 hw/isa-bus.c |   25 +
 hw/isa.h |4 
 hw/m48t59.c  |1 +
 hw/mc146818rtc.c |1 +
 hw/ne2000-isa.c  |3 +++
 hw/parallel.c|5 +
 hw/pckbd.c   |3 +++
 hw/sb16.c|4 
 hw/serial.c  |1 +
 13 files changed, 57 insertions(+), 0 deletions(-)

diff --git a/hw/cs4231a.c b/hw/cs4231a.c
index 4d5ce5c..598f032 100644
--- a/hw/cs4231a.c
+++ b/hw/cs4231a.c
@@ -645,6 +645,7 @@ static int cs4231a_initfn (ISADevice *dev)
 isa_init_irq (dev, &s->pic, s->irq);
 
 for (i = 0; i < 4; i++) {
+isa_init_ioport(dev, i);
 register_ioport_write (s->port + i, 1, 1, cs_write, s);
 register_ioport_read (s->port + i, 1, 1, cs_read, s);
 }
diff --git a/hw/fdc.c b/hw/fdc.c
index a467c4b..5ab754b 100644
--- a/hw/fdc.c
+++ b/hw/fdc.c
@@ -1983,6 +1983,9 @@ static int isabus_fdc_init1(ISADevice *dev)
   &fdctrl_write_port, fdctrl);
 register_ioport_write(iobase + 0x07, 1, 1,
   &fdctrl_write_port, fdctrl);
+isa_init_ioport_range(dev, iobase + 1, 5);
+isa_init_ioport(dev, iobase + 7);
+
 isa_init_irq(&isa->busdev, &fdctrl->irq, isairq);
 fdctrl->dma_chann = dma_chann;
 
diff --git a/hw/gus.c b/hw/gus.c
index e9016d8..ff9e7c7 100644
--- a/hw/gus.c
+++ b/hw/gus.c
@@ -264,20 +264,24 @@ static int gus_initfn (ISADevice *dev)
 
 register_ioport_write (s->port, 1, 1, gus_writeb, s);
 register_ioport_write (s->port, 1, 2, gus_writew, s);
+isa_init_ioport_range(dev, s->port, 2);
 
 register_ioport_read ((s->port + 0x100) & 0xf00, 1, 1, gus_readb, s);
 register_ioport_read ((s->port + 0x100) & 0xf00, 1, 2, gus_readw, s);
+isa_init_ioport_range(dev, (s->port + 0x100) & 0xf00, 2);
 
 register_ioport_write (s->port + 6, 10, 1, gus_writeb, s);
 register_ioport_write (s->port + 6, 10, 2, gus_writew, s);
 register_ioport_read (s->port + 6, 10, 1, gus_readb, s);
 register_ioport_read (s->port + 6, 10, 2, gus_readw, s);
+isa_init_ioport_range(dev, s->port + 6, 10);
 
 
 register_ioport_write (s->port + 0x100, 8, 1, gus_writeb, s);
 register_ioport_write (s->port + 0x100, 8, 2, gus_writew, s);
 register_ioport_read (s->port + 0x100, 8, 1, gus_readb, s);
 register_ioport_read (s->port + 0x100, 8, 2, gus_readw, s);
+isa_init_ioport_range(dev, s->port + 0x100, 8);
 
 DMA_register_channel (s->emu.gusdma, GUS_read_DMA, s);
 s->emu.himemaddr = s->himem;
diff --git a/hw/ide/isa.c b/hw/ide/isa.c
index 9856435..4206afd 100644
--- a/hw/ide/isa.c
+++ b/hw/ide/isa.c
@@ -70,6 +70,8 @@ static int isa_ide_initfn(ISADevice *dev)
 ide_bus_new(&s->bus, &s->dev.qdev);
 ide_init_ioport(&s->bus, s->iobase, s->iobase2);
 isa_init_irq(dev, &s->irq, s->isairq);
+isa_init_ioport_range(dev, s->iobase, 8);
+isa_init_ioport(dev, s->iobase2);
 ide_init2(&s->bus, s->irq);
 vmstate_register(&dev->qdev, 0, &vmstate_ide_isa, s);
 return 0;
diff --git a/hw/isa-bus.c b/hw/isa-bus.c
index 26036e0..c0ac7e9 100644
--- a/hw/isa-bus.c
+++ b/hw/isa-bus.c
@@ -92,6 +92,31 @@ void isa_init_irq(ISADevice *dev, qemu_irq *p, int isairq)
 dev->nirqs++;
 }
 
+static void isa_init_ioport_one(ISADevice *dev, uint16_t ioport)
+{
+assert(dev->nioports < ARRAY_SIZE(dev->ioports));
+dev->ioports[dev->nioports++] = ioport;
+}
+
+static int isa_cmp_ports(const void *p1, const void *p2)
+{
+return *(uint16_t*)p1 - *(uint16_t*)p2;
+}
+
+void isa_init_ioport_range(ISADevice *dev, uint16_t start, uint16_t length)
+{
+int i;
+for (i = start; i < start + length; i++) {
+isa_init_ioport_one(dev, i);
+}
+qsort(dev->ioports, dev->nioports, sizeof(dev->ioports[0]), isa_cmp_ports);
+}
+
+void isa_init_ioport(ISADevice *dev, uint16_t ioport)
+{
+isa_init_ioport_range(dev, ioport, 1);
+}
+
 static int isa_qdev_init(DeviceState *qdev, DeviceInfo *base)
 {
 ISADevice *dev = DO_UPCAST(ISADevice, qdev, qdev);
diff --git a/hw/isa.h b/hw/isa.h
index aaf0272..4794b76 100644
--- a/hw/isa.h
+++ b/hw/isa.h
@@ -14,6 +14,8 @@ struct ISADevice {
 DeviceState qdev;
 uint32_t isairq[2];
 int nirqs;
+uint16_t ioports[32];
+int nioports;
 };
 
 typedef int (*isa_qdev_initfn)(ISADevice *dev);
@@ -26,6 +28,8 @@ ISABus *isa_bus_new(DeviceState *dev);
 void isa_bus_irqs(qemu_irq *irqs);
 qemu_irq isa_reserve_irq(int isairq);
 void isa_init_irq(ISADevice *dev, qemu_irq *p, int isairq);
+void isa_init_ioport(ISADevice *dev, uint16_t ioport);
+void isa_init_ioport_range(ISADevice *dev, uint16_t start, uint16_t length);
 void isa_qdev_register(ISADeviceInfo *info);
 ISADevice *isa_create(const char *name);
 ISADevice *isa_create_simple(const char *name);
diff --git a/hw/m48t59.c b/hw/m

[Qemu-devel] [PATCHv5 08/15] Add get_fw_dev_path callback for pci bus.


Signed-off-by: Gleb Natapov 
---
 hw/pci.c |  108 -
 1 files changed, 85 insertions(+), 23 deletions(-)

diff --git a/hw/pci.c b/hw/pci.c
index 962886e..114b435 100644
--- a/hw/pci.c
+++ b/hw/pci.c
@@ -43,12 +43,14 @@
 
 static void pcibus_dev_print(Monitor *mon, DeviceState *dev, int indent);
 static char *pcibus_get_dev_path(DeviceState *dev);
+static char *pcibus_get_fw_dev_path(DeviceState *dev);
 
 struct BusInfo pci_bus_info = {
 .name   = "PCI",
 .size   = sizeof(PCIBus),
 .print_dev  = pcibus_dev_print,
 .get_dev_path = pcibus_get_dev_path,
+.get_fw_dev_path = pcibus_get_fw_dev_path,
 .props  = (Property[]) {
 DEFINE_PROP_PCI_DEVFN("addr", PCIDevice, devfn, -1),
 DEFINE_PROP_STRING("romfile", PCIDevice, romfile),
@@ -1061,45 +1063,63 @@ void pci_msi_notify(PCIDevice *dev, unsigned int vector)
 typedef struct {
 uint16_t class;
 const char *desc;
+const char *fw_name;
+uint16_t fw_ign_bits;
 } pci_class_desc;
 
 static const pci_class_desc pci_class_descriptions[] =
 {
-{ 0x0100, "SCSI controller"},
-{ 0x0101, "IDE controller"},
-{ 0x0102, "Floppy controller"},
-{ 0x0103, "IPI controller"},
-{ 0x0104, "RAID controller"},
+{ 0x0001, "VGA controller", "display"},
+{ 0x0100, "SCSI controller", "scsi"},
+{ 0x0101, "IDE controller", "ide"},
+{ 0x0102, "Floppy controller", "fdc"},
+{ 0x0103, "IPI controller", "ipi"},
+{ 0x0104, "RAID controller", "raid"},
 { 0x0106, "SATA controller"},
 { 0x0107, "SAS controller"},
 { 0x0180, "Storage controller"},
-{ 0x0200, "Ethernet controller"},
-{ 0x0201, "Token Ring controller"},
-{ 0x0202, "FDDI controller"},
-{ 0x0203, "ATM controller"},
+{ 0x0200, "Ethernet controller", "ethernet"},
+{ 0x0201, "Token Ring controller", "token-ring"},
+{ 0x0202, "FDDI controller", "fddi"},
+{ 0x0203, "ATM controller", "atm"},
 { 0x0280, "Network controller"},
-{ 0x0300, "VGA controller"},
+{ 0x0300, "VGA controller", "display", 0x00ff},
 { 0x0301, "XGA controller"},
 { 0x0302, "3D controller"},
 { 0x0380, "Display controller"},
-{ 0x0400, "Video controller"},
-{ 0x0401, "Audio controller"},
+{ 0x0400, "Video controller", "video"},
+{ 0x0401, "Audio controller", "sound"},
 { 0x0402, "Phone"},
 { 0x0480, "Multimedia controller"},
-{ 0x0500, "RAM controller"},
-{ 0x0501, "Flash controller"},
+{ 0x0500, "RAM controller", "memory"},
+{ 0x0501, "Flash controller", "flash"},
 { 0x0580, "Memory controller"},
-{ 0x0600, "Host bridge"},
-{ 0x0601, "ISA bridge"},
-{ 0x0602, "EISA bridge"},
-{ 0x0603, "MC bridge"},
-{ 0x0604, "PCI bridge"},
-{ 0x0605, "PCMCIA bridge"},
-{ 0x0606, "NUBUS bridge"},
-{ 0x0607, "CARDBUS bridge"},
+{ 0x0600, "Host bridge", "host"},
+{ 0x0601, "ISA bridge", "isa"},
+{ 0x0602, "EISA bridge", "eisa"},
+{ 0x0603, "MC bridge", "mca"},
+{ 0x0604, "PCI bridge", "pci"},
+{ 0x0605, "PCMCIA bridge", "pcmcia"},
+{ 0x0606, "NUBUS bridge", "nubus"},
+{ 0x0607, "CARDBUS bridge", "cardbus"},
 { 0x0608, "RACEWAY bridge"},
 { 0x0680, "Bridge"},
-{ 0x0c03, "USB controller"},
+{ 0x0700, "Serial port", "serial"},
+{ 0x0701, "Parallel port", "parallel"},
+{ 0x0800, "Interrupt controller", "interrupt-controller"},
+{ 0x0801, "DMA controller", "dma-controller"},
+{ 0x0802, "Timer", "timer"},
+{ 0x0803, "RTC", "rtc"},
+{ 0x0900, "Keyboard", "keyboard"},
+{ 0x0901, "Pen", "pen"},
+{ 0x0902, "Mouse", "mouse"},
+{ 0x0A00, "Dock station", "dock", 0x00ff},
+{ 0x0B00, "i386 cpu", "cpu", 0x00ff},
+{ 0x0c00, "Fireware contorller", "fireware"},
+{ 0x0c01, "Access bus controller", "access-bus"},
+{ 0x0c02, "SSA controller", "ssa"},
+{ 0x0c03, "USB controller", "usb"},
+{ 0x0c04, "Fibre channel controller", "fibre-channel"},
 { 0, NULL}
 };
 
@@ -1825,6 +1845,48 @@ static void pcibus_dev_print(Monitor *mon, DeviceState 
*dev, int indent)
 }
 }
 
+static char *pci_dev_fw_name(DeviceState *dev, char *buf, int len)
+{
+PCIDevice *d = (PCIDevice *)dev;
+const char *name = NULL;
+const pci_class_desc *desc =  pci_class_descriptions;
+int class = pci_get_word(d->config + PCI_CLASS_DEVICE);
+
+while (desc->desc &&
+  (class & ~desc->fw_ign_bits) !=
+  (desc->class & ~desc->fw_ign_bits)) {
+desc++;
+}
+
+if (desc->desc) {
+name = desc->fw_name;
+}
+
+if (name) {
+pstrcpy(buf, len, name);
+} else {
+snprintf(buf, len, "pci%04x,%04x",
+ pci_get_word(d->config + PCI_VENDOR_ID),
+ pci_get_word(d->config + PCI_DEVICE_ID));
+}
+
+return buf;
+}
+
+static char *pcibus_get_fw_dev_path(DeviceState *dev)
+{
+PCIDevice *d = (PCIDevice *)dev;
+char path[50], na

[Qemu-devel] [PATCHv5 00/15] boot order specification

I am using open firmware naming scheme to specify device path names.
In this version: fixed compilation problem, changed how device list
is passed into firmware.

Names look like this on pci machine:
/p...@i0cf8/i...@1,1/dr...@1/d...@0
/p...@i0cf8/i...@1/f...@03f1/flo...@1
/p...@i0cf8/i...@1/f...@03f1/flo...@0
/p...@i0cf8/i...@1,1/dr...@1/d...@1
/p...@i0cf8/i...@1,1/dr...@0/d...@0
/p...@i0cf8/s...@3/d...@0
/p...@i0cf8/ether...@4/ethernet-...@0
/p...@i0cf8/ether...@5/ethernet-...@0
/p...@i0cf8/i...@1,1/dr...@0/d...@1
/p...@i0cf8/i...@1/i...@01e8/dr...@0/d...@0
/p...@i0cf8/u...@1,2/netw...@0/ether...@0
/p...@i0cf8/u...@1,2/h...@1/netw...@0/ether...@0
/r...@genroms/linuxboot.bin

and on isa machine:
/isa/i...@0170/dr...@0/d...@0
/isa/f...@03f1/flo...@1
/isa/f...@03f1/flo...@0
/isa/i...@0170/dr...@0/d...@1


Instead of using get_dev_path() callback I introduces another one
get_fw_dev_path. Unfortunately the way get_dev_path() callback is used
in migration code makes it hard to reuse it for other purposes. First
of all it is not called recursively so caller expects it to provide
unique name by itself. Device path though is inherently recursive. Each
individual element may not be unique, but the whole path will be. On
the other hand to call get_dev_path() recursively in migration code we
should implement it for all possible buses first. Other problem is
compatibility. If we change get_dev_path() output format now we will not
be able to migrate from old qemu to new one without some additional
compatibility layer.

Gleb Natapov (15):
  Introduce fw_name field to DeviceInfo structure.
  Introduce new BusInfo callback get_fw_dev_path.
  Keep track of ISA ports ISA device is using in qdev.
  Add get_fw_dev_path callback to ISA bus in qdev.
  Store IDE bus id in IDEBus structure for easy access.
  Add get_fw_dev_path callback to IDE bus.
  Add get_dev_path callback for system bus.
  Add get_fw_dev_path callback for pci bus.
  Record which USBDevice USBPort belongs too.
  Add get_dev_path callback for usb bus.
  Add bootindex parameter to net/block/fd device
  Change fw_cfg_add_file() to get full file path as a parameter.
  Add bootindex for option roms.
  Add notifier that will be called when machine is fully created.
  Pass boot device list to firmware.

 block_int.h   |4 +-
 hw/cs4231a.c  |1 +
 hw/e1000.c|4 ++
 hw/eepro100.c |3 +
 hw/fdc.c  |   12 ++
 hw/fw_cfg.c   |   31 +--
 hw/fw_cfg.h   |9 +++-
 hw/gus.c  |4 ++
 hw/ide/cmd646.c   |4 +-
 hw/ide/internal.h |3 +-
 hw/ide/isa.c  |5 ++-
 hw/ide/piix.c |4 +-
 hw/ide/qdev.c |   22 ++-
 hw/ide/via.c  |4 +-
 hw/isa-bus.c  |   42 +++
 hw/isa.h  |4 ++
 hw/lance.c|1 +
 hw/loader.c   |   32 +++---
 hw/loader.h   |8 ++--
 hw/m48t59.c   |1 +
 hw/mc146818rtc.c  |1 +
 hw/multiboot.c|3 +-
 hw/ne2000-isa.c   |3 +
 hw/ne2000.c   |5 ++-
 hw/nseries.c  |4 +-
 hw/palm.c |6 +-
 hw/parallel.c |5 ++
 hw/pc.c   |7 ++-
 hw/pci.c  |  110 +++---
 hw/pci_host.c |2 +
 hw/pckbd.c|3 +
 hw/pcnet.c|6 ++-
 hw/piix_pci.c |1 +
 hw/qdev.c |   32 +++
 hw/qdev.h |9 
 hw/rtl8139.c  |4 ++
 hw/sb16.c |4 ++
 hw/serial.c   |1 +
 hw/sysbus.c   |   30 ++
 hw/sysbus.h   |4 ++
 hw/usb-bus.c  |   45 -
 hw/usb-hub.c  |3 +-
 hw/usb-musb.c |2 +-
 hw/usb-net.c  |3 +
 hw/usb-ohci.c |2 +-
 hw/usb-uhci.c |2 +-
 hw/usb.h  |3 +-
 hw/virtio-blk.c   |2 +
 hw/virtio-net.c   |2 +
 hw/virtio-pci.c   |1 +
 net.h |4 +-
 qemu-config.c |   17 
 sysemu.h  |   11 +-
 vl.c  |  115 -
 54 files changed, 569 insertions(+), 81 deletions(-)

[Qemu-devel] [PATCHv5 15/15] Pass boot device list to firmware.


Signed-off-by: Gleb Natapov 
---
 hw/fw_cfg.c |   15 +++
 hw/fw_cfg.h |5 -
 sysemu.h|1 +
 vl.c|   49 +
 4 files changed, 69 insertions(+), 1 deletions(-)

diff --git a/hw/fw_cfg.c b/hw/fw_cfg.c
index 7b9434f..4eea338 100644
--- a/hw/fw_cfg.c
+++ b/hw/fw_cfg.c
@@ -53,6 +53,7 @@ struct FWCfgState {
 FWCfgFiles *files;
 uint16_t cur_entry;
 uint32_t cur_offset;
+Notifier machine_ready;
 };
 
 static void fw_cfg_write(FWCfgState *s, uint8_t value)
@@ -315,6 +316,16 @@ int fw_cfg_add_file(FWCfgState *s,  const char *filename, 
uint8_t *data,
 return 1;
 }
 
+static void fw_cfg_machine_ready(struct Notifier* n)
+{
+uint32_t len;
+FWCfgState *s = container_of(n, FWCfgState, machine_ready);
+char *bootindex = get_boot_devices_list(&len);
+
+fw_cfg_add_i32(s, FW_CFG_BOOTINDEX_LEN, len);
+fw_cfg_add_bytes(s, FW_CFG_BOOTINDEX_DATA, (uint8_t*)bootindex, len);
+}
+
 FWCfgState *fw_cfg_init(uint32_t ctl_port, uint32_t data_port,
 target_phys_addr_t ctl_addr, target_phys_addr_t 
data_addr)
 {
@@ -343,6 +354,10 @@ FWCfgState *fw_cfg_init(uint32_t ctl_port, uint32_t 
data_port,
 fw_cfg_add_i16(s, FW_CFG_MAX_CPUS, (uint16_t)max_cpus);
 fw_cfg_add_i16(s, FW_CFG_BOOT_MENU, (uint16_t)boot_menu);
 
+
+s->machine_ready.notify = fw_cfg_machine_ready;
+qemu_add_machine_init_done_notifier(&s->machine_ready);
+
 return s;
 }
 
diff --git a/hw/fw_cfg.h b/hw/fw_cfg.h
index 856bf91..b951f6b 100644
--- a/hw/fw_cfg.h
+++ b/hw/fw_cfg.h
@@ -30,7 +30,10 @@
 
 #define FW_CFG_FILE_FIRST   0x20
 #define FW_CFG_FILE_SLOTS   0x10
-#define FW_CFG_MAX_ENTRY(FW_CFG_FILE_FIRST+FW_CFG_FILE_SLOTS)
+#define FW_CFG_FILE_LAST_SLOT   (FW_CFG_FILE_FIRST+FW_CFG_FILE_SLOTS)
+#define FW_CFG_BOOTINDEX_LEN(FW_CFG_FILE_LAST_SLOT + 1)
+#define FW_CFG_BOOTINDEX_DATA   (FW_CFG_FILE_LAST_SLOT + 2)
+#define FW_CFG_MAX_ENTRYFW_CFG_BOOTINDEX_DATA + 1 
 
 #define FW_CFG_WRITE_CHANNEL0x4000
 #define FW_CFG_ARCH_LOCAL   0x8000
diff --git a/sysemu.h b/sysemu.h
index c42f33a..38a20a3 100644
--- a/sysemu.h
+++ b/sysemu.h
@@ -196,4 +196,5 @@ void register_devices(void);
 
 void add_boot_device_path(int32_t bootindex, DeviceState *dev,
   const char *suffix);
+char *get_boot_devices_list(uint32_t *size);
 #endif
diff --git a/vl.c b/vl.c
index 918d988..ab36f9f 100644
--- a/vl.c
+++ b/vl.c
@@ -735,6 +735,55 @@ void add_boot_device_path(int32_t bootindex, DeviceState 
*dev,
 QTAILQ_INSERT_TAIL(&fw_boot_order, node, link);
 }
 
+/*
+ * This function returns device list as an array in a below format:
+ * +---+-+-
+ * | devpath1  |  devpath2   | ...
+ * +---+-+-
+ * where:
+ *   devpath - null terminated string representing one device path
+ *
+ *  memory pointed by "size" is assigned total length of teh array in bytes
+ *
+ */
+char *get_boot_devices_list(uint32_t *size)
+{
+FWBootEntry *i;
+uint32_t total = 0;
+char *list = NULL;
+
+QTAILQ_FOREACH(i, &fw_boot_order, link) {
+char *devpath = NULL, *bootpath;
+int len;
+
+if (i->dev) {
+devpath = qdev_get_fw_dev_path(i->dev);
+assert(devpath);
+}
+
+if (i->suffix && devpath) {
+bootpath = qemu_malloc(strlen(devpath) + strlen(i->suffix) + 2);
+sprintf(bootpath, "%s/%s", devpath, i->suffix);
+qemu_free(devpath);
+} else if (devpath) {
+bootpath = devpath;
+} else {
+bootpath = strdup(i->suffix);
+assert(bootpath);
+}
+
+len = strlen(bootpath) + 1;
+list = qemu_realloc(list, total + len);
+memcpy(&list[total], bootpath, len);
+total += len;
+qemu_free(bootpath);
+}
+
+*size = total;
+
+return list;
+}
+
 static void numa_add(const char *optarg)
 {
 char option[128];
-- 
1.7.1

[Qemu-devel] [PATCHv5 12/15] Change fw_cfg_add_file() to get full file path as a parameter.

Change fw_cfg_add_file() to get full file path as a parameter instead
of building one internally. Two reasons for that. First caller may need
to know how file is named. Second this moves policy of file naming out
from fw_cfg. Platform may want to use more then two levels of
directories for instance.

Signed-off-by: Gleb Natapov 
---
 hw/fw_cfg.c |   16 
 hw/fw_cfg.h |4 ++--
 hw/loader.c |   16 ++--
 3 files changed, 20 insertions(+), 16 deletions(-)

diff --git a/hw/fw_cfg.c b/hw/fw_cfg.c
index 72866ae..7b9434f 100644
--- a/hw/fw_cfg.c
+++ b/hw/fw_cfg.c
@@ -277,10 +277,9 @@ int fw_cfg_add_callback(FWCfgState *s, uint16_t key, 
FWCfgCallback callback,
 return 1;
 }
 
-int fw_cfg_add_file(FWCfgState *s,  const char *dir, const char *filename,
-uint8_t *data, uint32_t len)
+int fw_cfg_add_file(FWCfgState *s,  const char *filename, uint8_t *data,
+uint32_t len)
 {
-const char *basename;
 int i, index;
 
 if (!s->files) {
@@ -297,15 +296,8 @@ int fw_cfg_add_file(FWCfgState *s,  const char *dir, const 
char *filename,
 
 fw_cfg_add_bytes(s, FW_CFG_FILE_FIRST + index, data, len);
 
-basename = strrchr(filename, '/');
-if (basename) {
-basename++;
-} else {
-basename = filename;
-}
-
-snprintf(s->files->f[index].name, sizeof(s->files->f[index].name),
- "%s/%s", dir, basename);
+pstrcpy(s->files->f[index].name, sizeof(s->files->f[index].name),
+filename);
 for (i = 0; i < index; i++) {
 if (strcmp(s->files->f[index].name, s->files->f[i].name) == 0) {
 FW_CFG_DPRINTF("%s: skip duplicate: %s\n", __FUNCTION__,
diff --git a/hw/fw_cfg.h b/hw/fw_cfg.h
index 4d13a4f..856bf91 100644
--- a/hw/fw_cfg.h
+++ b/hw/fw_cfg.h
@@ -60,8 +60,8 @@ int fw_cfg_add_i32(FWCfgState *s, uint16_t key, uint32_t 
value);
 int fw_cfg_add_i64(FWCfgState *s, uint16_t key, uint64_t value);
 int fw_cfg_add_callback(FWCfgState *s, uint16_t key, FWCfgCallback callback,
 void *callback_opaque, uint8_t *data, size_t len);
-int fw_cfg_add_file(FWCfgState *s, const char *dir, const char *filename,
-uint8_t *data, uint32_t len);
+int fw_cfg_add_file(FWCfgState *s, const char *filename, uint8_t *data,
+uint32_t len);
 FWCfgState *fw_cfg_init(uint32_t ctl_port, uint32_t data_port,
 target_phys_addr_t crl_addr, target_phys_addr_t 
data_addr);
 
diff --git a/hw/loader.c b/hw/loader.c
index 49ac1fa..1e98326 100644
--- a/hw/loader.c
+++ b/hw/loader.c
@@ -592,8 +592,20 @@ int rom_add_file(const char *file, const char *fw_dir,
 }
 close(fd);
 rom_insert(rom);
-if (rom->fw_file && fw_cfg)
-fw_cfg_add_file(fw_cfg, rom->fw_dir, rom->fw_file, rom->data, 
rom->romsize);
+if (rom->fw_file && fw_cfg) {
+const char *basename;
+char fw_file_name[56];
+
+basename = strrchr(rom->fw_file, '/');
+if (basename) {
+basename++;
+} else {
+basename = rom->fw_file;
+}
+snprintf(fw_file_name, sizeof(fw_file_name), "%s/%s", rom->fw_dir,
+ basename);
+fw_cfg_add_file(fw_cfg, fw_file_name, rom->data, rom->romsize);
+}
 return 0;
 
 err:
-- 
1.7.1

[Qemu-devel] [PATCHv5 02/15] Introduce new BusInfo callback get_fw_dev_path.

New get_fw_dev_path callback will be used for build device path usable
by firmware in contrast to qdev qemu internal device path.

Signed-off-by: Gleb Natapov 
---
 hw/qdev.h |2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/hw/qdev.h b/hw/qdev.h
index 9f90efe..dc669b3 100644
--- a/hw/qdev.h
+++ b/hw/qdev.h
@@ -49,12 +49,14 @@ struct DeviceState {
 
 typedef void (*bus_dev_printfn)(Monitor *mon, DeviceState *dev, int indent);
 typedef char *(*bus_get_dev_path)(DeviceState *dev);
+typedef char *(*bus_get_fw_dev_path)(DeviceState *dev);
 
 struct BusInfo {
 const char *name;
 size_t size;
 bus_dev_printfn print_dev;
 bus_get_dev_path get_dev_path;
+bus_get_fw_dev_path get_fw_dev_path;
 Property *props;
 };
 
-- 
1.7.1

[Qemu-devel] [PATCHv5 10/15] Add get_dev_path callback for usb bus.