[Qemu-devel] [PATCH 1/4] char: check for initial_reset_issued unnecessary
At init, qemu_chr_reset is always called with initial_reset_issued set to 1.
So checking for it to be set is not necessary.
Signed-off-by: Amit Shah amit.s...@redhat.com
---
qemu-char.c |5 +
1 files changed, 1 insertions(+), 4 deletions(-)
diff --git a/qemu-char.c b/qemu-char.c
index 8084a67..8f7f81c 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -108,7 +108,6 @@
static QTAILQ_HEAD(CharDriverStateHead, CharDriverState) chardevs =
QTAILQ_HEAD_INITIALIZER(chardevs);
-static int initial_reset_issued;
static void qemu_chr_event(CharDriverState *s, int event)
{
@@ -127,7 +126,7 @@ static void qemu_chr_reset_bh(void *opaque)
void qemu_chr_reset(CharDriverState *s)
{
-if (s-bh == NULL initial_reset_issued) {
+if (s-bh == NULL) {
s-bh = qemu_bh_new(qemu_chr_reset_bh, s);
qemu_bh_schedule(s-bh);
}
@@ -137,8 +136,6 @@ void qemu_chr_initial_reset(void)
{
CharDriverState *chr;
-initial_reset_issued = 1;
-
QTAILQ_FOREACH(chr, chardevs, next) {
qemu_chr_reset(chr);
}
--
1.6.2.5
[Qemu-devel] [PATCH 2/4] char: rename CHR_EVENT_RESET to CHR_EVENT_OPENED
The char event RESET is emitted when a char device is opened.
Give it a better name.
Signed-off-by: Amit Shah amit.s...@redhat.com
---
gdbstub.c |2 +-
hw/baum.c |2 +-
hw/usb-serial.c |2 +-
monitor.c |2 +-
qemu-char.c |2 +-
qemu-char.h |2 +-
6 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/gdbstub.c b/gdbstub.c
index 315f606..055093f 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -2447,7 +2447,7 @@ static void gdb_chr_receive(void *opaque, const uint8_t
*buf, int size)
static void gdb_chr_event(void *opaque, int event)
{
switch (event) {
-case CHR_EVENT_RESET:
+case CHR_EVENT_OPENED:
vm_stop(EXCP_INTERRUPT);
gdb_has_xml = 0;
break;
diff --git a/hw/baum.c b/hw/baum.c
index 665deab..c66e737 100644
--- a/hw/baum.c
+++ b/hw/baum.c
@@ -475,7 +475,7 @@ static void baum_send_event(CharDriverState *chr, int event)
switch (event) {
case CHR_EVENT_BREAK:
break;
-case CHR_EVENT_RESET:
+case CHR_EVENT_OPENED:
/* Reset state */
baum-in_buf_used = 0;
break;
diff --git a/hw/usb-serial.c b/hw/usb-serial.c
index e2379c4..d02f6b2 100644
--- a/hw/usb-serial.c
+++ b/hw/usb-serial.c
@@ -516,7 +516,7 @@ static void usb_serial_event(void *opaque, int event)
break;
case CHR_EVENT_FOCUS:
break;
-case CHR_EVENT_RESET:
+case CHR_EVENT_OPENED:
usb_serial_reset(s);
/* TODO: Reset USB port */
break;
diff --git a/monitor.c b/monitor.c
index 3424e60..2566f4a 100644
--- a/monitor.c
+++ b/monitor.c
@@ -3470,7 +3470,7 @@ static void monitor_event(void *opaque, int event)
mon-mux_out = 1;
break;
-case CHR_EVENT_RESET:
+case CHR_EVENT_OPENED:
monitor_printf(mon, QEMU %s monitor - type 'help' for more
information\n, QEMU_VERSION);
if (!mon-mux_out) {
diff --git a/qemu-char.c b/qemu-char.c
index 8f7f81c..4757689 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -119,7 +119,7 @@ static void qemu_chr_event(CharDriverState *s, int event)
static void qemu_chr_reset_bh(void *opaque)
{
CharDriverState *s = opaque;
-qemu_chr_event(s, CHR_EVENT_RESET);
+qemu_chr_event(s, CHR_EVENT_OPENED);
qemu_bh_delete(s-bh);
s-bh = NULL;
}
diff --git a/qemu-char.h b/qemu-char.h
index c0654bc..05fe15d 100644
--- a/qemu-char.h
+++ b/qemu-char.h
@@ -10,7 +10,7 @@
#define CHR_EVENT_BREAK 0 /* serial break char */
#define CHR_EVENT_FOCUS 1 /* focus to this terminal (modal input needed) */
-#define CHR_EVENT_RESET 2 /* new connection established */
+#define CHR_EVENT_OPENED 2 /* new connection established */
#define CHR_EVENT_MUX_IN 3 /* mux-focus was set to this terminal */
#define CHR_EVENT_MUX_OUT 4 /* mux-focus will move on */
#define CHR_EVENT_CLOSED 5 /* connection closed */
--
1.6.2.5
[Qemu-devel] [PATCH 3/4] console: call qemu_chr_reset() in text_console_init
text_console_init is called as the initialiser function for the 'vc' char backend. This function doesn't call qemu_chr_reset(), that emits the OPENED event. This fixes the help text and initial prompt display when the monitor is started in its default options -- as a vc backend to sdl or vnc. Signed-off-by: Amit Shah amit.s...@redhat.com --- console.c |1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/console.c b/console.c index 9bbef59..a4e0d64 100644 --- a/console.c +++ b/console.c @@ -1403,6 +1403,7 @@ CharDriverState *text_console_init(QemuOpts *opts) text_console_opts[n_text_consoles] = opts; n_text_consoles++; +qemu_chr_reset(chr); return chr; } -- 1.6.2.5
[Qemu-devel] [PATCH 4/4] char: emit the OPENED event only when a new char connection is opened
The OPENED event gets sent also when qemu resets its state initially.
The consumers of the event aren't interested in receiving this event
on reset.
Signed-off-by: Amit Shah amit.s...@redhat.com
---
qemu-char.c |7 ++-
qemu-char.h |2 ++
2 files changed, 8 insertions(+), 1 deletions(-)
diff --git a/qemu-char.c b/qemu-char.c
index 4757689..0fd402c 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -119,7 +119,12 @@ static void qemu_chr_event(CharDriverState *s, int event)
static void qemu_chr_reset_bh(void *opaque)
{
CharDriverState *s = opaque;
-qemu_chr_event(s, CHR_EVENT_OPENED);
+
+if (s-initial_reset_issued) {
+qemu_chr_event(s, CHR_EVENT_OPENED);
+} else {
+s-initial_reset_issued = true;
+}
qemu_bh_delete(s-bh);
s-bh = NULL;
}
diff --git a/qemu-char.h b/qemu-char.h
index 05fe15d..409961d 100644
--- a/qemu-char.h
+++ b/qemu-char.h
@@ -1,6 +1,7 @@
#ifndef QEMU_CHAR_H
#define QEMU_CHAR_H
+#include stdbool.h
#include qemu-common.h
#include qemu-queue.h
#include qemu-option.h
@@ -66,6 +67,7 @@ struct CharDriverState {
QEMUBH *bh;
char *label;
char *filename;
+bool initial_reset_issued;
QTAILQ_ENTRY(CharDriverState) next;
};
--
1.6.2.5
[Qemu-devel] [PATCH v9 0/3] virtio-console: Add support for multiple ports for generic guest-host communication
Hello, This patch series fixes a few problems since the last send, mainly in the save/restore code and a few bugs shown by the automated test suite (located in a separate git repo, link below). The automated test suite and a standalone interactive test program are located at http://fedorapeople.org/gitweb?p=amitshah/public_git/test-virtserial.git These patches are based on top of the char patches I've sent previously. Amit Shah (2): virtio-console: Add a virtio-serial bus, support for multiple ports virtio-console: Add a new virtserialport device for generic serial port support Gerd Hoffmann (1): qdev: add string property. Makefile.target|2 +- hw/pc.c|9 - hw/ppc440_bamboo.c |7 - hw/qdev-properties.c | 28 ++ hw/qdev.c |8 +- hw/qdev.h |4 + hw/virtio-console.c| 206 +++--- hw/virtio-console.h| 19 -- hw/virtio-pci.c|8 +- hw/virtio-serial-bus.c | 764 hw/virtio-serial.h | 227 ++ hw/virtio.h|2 +- qemu-options.hx|6 +- sysemu.h |6 - vl.c | 65 +++-- 15 files changed, 1182 insertions(+), 179 deletions(-) delete mode 100644 hw/virtio-console.h create mode 100644 hw/virtio-serial-bus.c create mode 100644 hw/virtio-serial.h
[Qemu-devel] [PATCH v9 1/3] qdev: add string property.
From: Gerd Hoffmann kra...@redhat.com
Signed-off-by: Gerd Hoffmann kra...@redhat.com
Signed-off-by: Amit Shah amit.s...@redhat.com
---
hw/qdev-properties.c | 28
hw/qdev.h|4
2 files changed, 32 insertions(+), 0 deletions(-)
diff --git a/hw/qdev-properties.c b/hw/qdev-properties.c
index 5c627fa..8ca345a 100644
--- a/hw/qdev-properties.c
+++ b/hw/qdev-properties.c
@@ -193,6 +193,34 @@ PropertyInfo qdev_prop_hex64 = {
.print = print_hex64,
};
+/* --- string --- */
+
+static int parse_string(DeviceState *dev, Property *prop, const char *str)
+{
+char **ptr = qdev_get_prop_ptr(dev, prop);
+
+if (*ptr)
+qemu_free(*ptr);
+*ptr = qemu_strdup(str);
+return 0;
+}
+
+static int print_string(DeviceState *dev, Property *prop, char *dest, size_t
len)
+{
+char **ptr = qdev_get_prop_ptr(dev, prop);
+if (!*ptr)
+return snprintf(dest, len, null);
+return snprintf(dest, len, \%s\, *ptr);
+}
+
+PropertyInfo qdev_prop_string = {
+.name = string,
+.type = PROP_TYPE_STRING,
+.size = sizeof(char*),
+.parse = parse_string,
+.print = print_string,
+};
+
/* --- drive --- */
static int parse_drive(DeviceState *dev, Property *prop, const char *str)
diff --git a/hw/qdev.h b/hw/qdev.h
index 6d61b3a..b79f3e3 100644
--- a/hw/qdev.h
+++ b/hw/qdev.h
@@ -79,6 +79,7 @@ enum PropertyType {
PROP_TYPE_MACADDR,
PROP_TYPE_DRIVE,
PROP_TYPE_CHR,
+PROP_TYPE_STRING,
PROP_TYPE_PTR,
};
@@ -186,6 +187,7 @@ extern PropertyInfo qdev_prop_int32;
extern PropertyInfo qdev_prop_uint64;
extern PropertyInfo qdev_prop_hex32;
extern PropertyInfo qdev_prop_hex64;
+extern PropertyInfo qdev_prop_string;
extern PropertyInfo qdev_prop_chr;
extern PropertyInfo qdev_prop_ptr;
extern PropertyInfo qdev_prop_macaddr;
@@ -227,6 +229,8 @@ extern PropertyInfo qdev_prop_pci_devfn;
DEFINE_PROP(_n, _s, _f, qdev_prop_ptr, void*)
#define DEFINE_PROP_CHR(_n, _s, _f) \
DEFINE_PROP(_n, _s, _f, qdev_prop_chr, CharDriverState*)
+#define DEFINE_PROP_STRING(_n, _s, _f) \
+DEFINE_PROP(_n, _s, _f, qdev_prop_string, char*)
#define DEFINE_PROP_DRIVE(_n, _s, _f) \
DEFINE_PROP(_n, _s, _f, qdev_prop_drive, DriveInfo*)
#define DEFINE_PROP_MACADDR(_n, _s, _f) \
--
1.6.2.5
[Qemu-devel] [PATCH v9 2/3] virtio-console: Add a virtio-serial bus, support for multiple ports
This patch migrates virtio-console to the qdev infrastructure and
creates a new virtio-serial bus on which multiple ports are exposed as
devices. The bulk of the code now resides in a new file with
virtio-console.c being just a simple qdev device.
This interface enables spawning of multiple virtio consoles as well as generic
serial ports.
The older -virtconsole argument still works, but when using the new
functionality, it is recommended to use
-device virtio-serial-pci -device virtconsole,...
The virtconsole device type accepts a chardev as an argument and a 'name'
argument to identify the corresponding consoles on the host as well as the
guest. The name, if given, is exposed via the 'name' sysfs attribute.
Care has been taken to ensure compatibility with kernels that do not
support multiple ports as well as accepting incoming migrations from older
qemu versions.
Signed-off-by: Amit Shah amit.s...@redhat.com
---
Makefile.target|2 +-
hw/pc.c|9 -
hw/ppc440_bamboo.c |7 -
hw/qdev.c |8 +-
hw/virtio-console.c| 180 +---
hw/virtio-console.h| 19 --
hw/virtio-pci.c|8 +-
hw/virtio-serial-bus.c | 764
hw/virtio-serial.h | 227 ++
hw/virtio.h|2 +-
qemu-options.hx|6 +-
sysemu.h |6 -
vl.c | 65 +++--
13 files changed, 1118 insertions(+), 185 deletions(-)
delete mode 100644 hw/virtio-console.h
create mode 100644 hw/virtio-serial-bus.c
create mode 100644 hw/virtio-serial.h
diff --git a/Makefile.target b/Makefile.target
index 8d146c5..fd86eeb 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -157,7 +157,7 @@ ifdef CONFIG_SOFTMMU
obj-y = vl.o monitor.o pci.o machine.o gdbstub.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-console.o
virtio-pci.o
+obj-y += virtio-blk.o virtio-balloon.o virtio-net.o virtio-serial-bus.o
virtio-console.o virtio-pci.o
obj-$(CONFIG_KVM) += kvm.o kvm-all.o
obj-$(CONFIG_ISA_MMIO) += isa_mmio.o
LIBS+=-lz
diff --git a/hw/pc.c b/hw/pc.c
index 408d6d6..3dbe718 100644
--- a/hw/pc.c
+++ b/hw/pc.c
@@ -1319,15 +1319,6 @@ static void pc_init1(ram_addr_t ram_size,
pci_create_simple(pci_bus, -1, lsi53c895a);
}
}
-
-/* Add virtio console devices */
-if (pci_enabled) {
-for(i = 0; i MAX_VIRTIO_CONSOLES; i++) {
-if (virtcon_hds[i]) {
-pci_create_simple(pci_bus, -1, virtio-console-pci);
-}
-}
-}
}
static void pc_init_pci(ram_addr_t ram_size,
diff --git a/hw/ppc440_bamboo.c b/hw/ppc440_bamboo.c
index a488240..1ab9872 100644
--- a/hw/ppc440_bamboo.c
+++ b/hw/ppc440_bamboo.c
@@ -108,13 +108,6 @@ static void bamboo_init(ram_addr_t ram_size,
env = ppc440ep_init(ram_size, pcibus, pci_irq_nrs, 1, cpu_model);
if (pcibus) {
-/* Add virtio console devices */
-for(i = 0; i MAX_VIRTIO_CONSOLES; i++) {
-if (virtcon_hds[i]) {
-pci_create_simple(pcibus, -1, virtio-console-pci);
-}
-}
-
/* Register network interfaces. */
for (i = 0; i nb_nics; i++) {
/* There are no PCI NICs on the Bamboo board, but there are
diff --git a/hw/qdev.c b/hw/qdev.c
index 20f931c..374d2d0 100644
--- a/hw/qdev.c
+++ b/hw/qdev.c
@@ -321,13 +321,9 @@ void qdev_machine_creation_done(void)
CharDriverState *qdev_init_chardev(DeviceState *dev)
{
static int next_serial;
-static int next_virtconsole;
+
/* FIXME: This is a nasty hack that needs to go away. */
-if (strncmp(dev-info-name, virtio, 6) == 0) {
-return virtcon_hds[next_virtconsole++];
-} else {
-return serial_hds[next_serial++];
-}
+return serial_hds[next_serial++];
}
BusState *qdev_get_parent_bus(DeviceState *dev)
diff --git a/hw/virtio-console.c b/hw/virtio-console.c
index 57f8f89..ef32820 100644
--- a/hw/virtio-console.c
+++ b/hw/virtio-console.c
@@ -1,143 +1,109 @@
/*
- * Virtio Console Device
+ * Virtio Console and Generic Port Devices
*
- * Copyright IBM, Corp. 2008
+ * Copyright Red Hat, Inc. 2009
*
* Authors:
- * Christian Ehrhardt ehrha...@linux.vnet.ibm.com
+ * Amit Shah amit.s...@redhat.com
*
* This work is licensed under the terms of the GNU GPL, version 2. See
* the COPYING file in the top-level directory.
- *
*/
-#include hw.h
#include qemu-char.h
-#include virtio.h
-#include virtio-console.h
-
+#include virtio-serial.h
-typedef struct VirtIOConsole
-{
-VirtIODevice vdev;
-VirtQueue *ivq, *ovq;
+typedef struct VirtConsole {
+VirtIOSerialPort port;
CharDriverState *chr;
-} VirtIOConsole;
+} VirtConsole;
-static VirtIOConsole *to_virtio_console(VirtIODevice *vdev)
-{
-return (VirtIOConsole *)vdev;
-}
-static void
[Qemu-devel] [PATCH v9 3/3] virtio-console: Add a new virtserialport device for generic serial port support
This patch adds generic serial ports over the virtio serial bus.
These ports have a few more options that are not relevant for
virtio console ports: the ability to cache buffers that are
received for a port while it's disconnected, setting of limits
to the bytes that are cached so as to prevent OOM conditions,
etc.
Sample uses for such a device can be obtaining info from the
guest like the file systems used, apps installed, etc. for
offline usage and logged-in users, clipboard copy-paste, etc.
for online usage.
For requirements, use-cases, test cases and some history see
http://www.linux-kvm.org/page/VMchannel_Requirements
Signed-off-by: Amit Shah amit.s...@redhat.com
---
hw/virtio-console.c | 38 ++
1 files changed, 38 insertions(+), 0 deletions(-)
diff --git a/hw/virtio-console.c b/hw/virtio-console.c
index ef32820..fec346c 100644
--- a/hw/virtio-console.c
+++ b/hw/virtio-console.c
@@ -107,3 +107,41 @@ static void virtcon_register(void)
virtio_serial_port_qdev_register(virtcon_info);
}
device_init(virtcon_register)
+
+
+/* Generic Virtio Serial Ports */
+static int virtserial_port_initfn(VirtIOSerialDevice *dev)
+{
+VirtIOSerialPort *port = DO_UPCAST(VirtIOSerialPort, dev, dev-qdev);
+VirtConsole *vcon = DO_UPCAST(VirtConsole, port, port);
+
+port-info = dev-info;
+
+if (vcon-chr) {
+qemu_chr_add_handlers(vcon-chr, chr_can_read, chr_read, chr_event,
+ vcon);
+}
+return 0;
+}
+
+static VirtIOSerialPortInfo virtserial_port_info = {
+.qdev.name = virtserialport,
+.qdev.size = sizeof(VirtConsole),
+.init = virtserial_port_initfn,
+.have_data = flush_buf,
+.qdev.props = (Property[]) {
+DEFINE_PROP_CHR(chardev, VirtConsole, chr),
+DEFINE_PROP_STRING(name, VirtIOSerialPort, name),
+DEFINE_PROP_INT32(cache_buffers, VirtIOSerialPort, cache_buffers, 1),
+DEFINE_PROP_UINT64(byte_limit, VirtIOSerialPort, byte_limit, 0),
+DEFINE_PROP_UINT64(guest_byte_limit, VirtIOSerialPort,
+ guest_byte_limit, 0),
+DEFINE_PROP_END_OF_LIST(),
+},
+};
+
+static void virtserial_port_register(void)
+{
+virtio_serial_port_qdev_register(virtserial_port_info);
+}
+device_init(virtserial_port_register)
--
1.6.2.5
Re: [Qemu-devel] [PATCH v9 3/3] virtio-console: Add a new virtserialport device for generic serial port support
On (Tue) Oct 20 2009 [10:51:30], Gerd Hoffmann wrote:
@@ -107,3 +107,41 @@ static void virtcon_register(void)
virtio_serial_port_qdev_register(virtcon_info);
}
device_init(virtcon_register)
+static VirtIOSerialPortInfo virtserial_port_info = {
+.qdev.name = virtserialport,
+.qdev.size = sizeof(VirtConsole),
+.init = virtserial_port_initfn,
+.have_data = flush_buf,
+.qdev.props = (Property[]) {
+DEFINE_PROP_CHR(chardev, VirtConsole, chr),
+DEFINE_PROP_STRING(name, VirtIOSerialPort, name),
likewise: DEFINE_PROP_STRING(name, VirtConsole, port.name),
The 'name' field is common to all ports, so it makes sense to put it in
the VirtIOSerialPort struct. (Internal users can pre-define it to their
liking, eg, org.qemu.vnc)
+static void virtserial_port_register(void)
+{
+virtio_serial_port_qdev_register(virtserial_port_info);
+}
+device_init(virtserial_port_register)
You can simply stick two register calls into the existing init function ...
The console init function has extra init'ing to do, like disable buffer
caching, set is_console to true, etc., so there are two init fns.
Amit
Re: [Qemu-devel] [PATCH v9 0/3] virtio-console: Add support for multiple ports for generic guest-host communication
On (Tue) Oct 20 2009 [10:56:40], Gerd Hoffmann wrote: Hi, This patch series fixes a few problems since the last send, mainly in the save/restore code and a few bugs shown by the automated test suite (located in a separate git repo, link below). A bit hard to review in this form, especially the virtio-console.c changes, because you put everything upside down in that file. Hard to do better though given the massive code reorganization ... Top-down is the usual way of writing code, isn't it (avoids fwd declarations too). Also, I've arranged the code according to some grouping: functions useful to outside users first, then functions using some outside facilities, and then intialisation functions. So I applied the bits and looked at the resulting tree instead. Looks good overall, just a few minor nits, check the replies to the individual patches. I think we are ready to go as soon as the linux kernel side is on the way to mainline. Yeah; waiting for Rusty's comments. Amit
Re: [Qemu-devel] [PATCH v9 2/3] virtio-console: Add a virtio-serial bus, support for multiple ports
On (Tue) Oct 20 2009 [12:08:06], Richard W.M. Jones wrote:
On Tue, Oct 20, 2009 at 12:43:44PM +0530, Amit Shah wrote:
DEF(virtioconsole, HAS_ARG, QEMU_OPTION_virtiocon, \
-virtioconsole c\n \
-set virtio console\n)
+define virtio console\n)
It would be much better to add a detectable string here so we
(libguestfs) can tell if multiport is supported. Something like:
set virtio console (multiport)\n
How about (deprecated)? (We need to have this in some general policy of
deprecating command-line options.)
STEXI
@item -virtioconsole @var{c}
Set virtio console.
+
+This option is maintained for backward compatibility.
+
+Please use @code{-device virtioconsole} for the new way of invocation
I think you mean -device virtconsole here.
Yeah; thanks. Will fix.
Amit
Re: [Qemu-devel] [PATCH v9 0/3] virtio-console: Add support for multiple ports for generic guest-host communication
On (Tue) Oct 20 2009 [13:54:33], Dor Laor wrote: On 10/20/2009 09:13 AM, Amit Shah wrote: Hello, This patch series fixes a few problems since the last send, mainly in the save/restore code and a few bugs shown by the automated test suite (located in a separate git repo, link below). The automated test suite and a standalone interactive test program are located at http://fedorapeople.org/gitweb?p=amitshah/public_git/test-virtserial.git Why not keep it in-tree? Out of tree it will soon break, especially for new releases and it'll be harder for autotest to pick it up. The code is best integrated within autotest and I'm talking with Lucas for integrating it. BTW if a test breaks, it's generally not a good sign :-) Amit
Re: [Qemu-devel] [PATCH v9 2/3] virtio-console: Add a virtio-serial bus, support for multiple ports
On (Tue) Oct 20 2009 [13:05:59], Richard W.M. Jones wrote: On Tue, Oct 20, 2009 at 05:31:23PM +0530, Amit Shah wrote: How about (deprecated)? (We need to have this in some general policy of deprecating command-line options.) No, the important thing is that we can detect somehow that multiport virtio console is possible for some random version of qemu that we have to work with. The only feasible way we've been able to discover is to poke qemu with various arguments (usually, qemu -help) and then match on substrings in the output. Hm, probing with -device '?' will get you the new devices: -device virtio-serial-pci and -device virtserialport are two new devices added (along with -device virtconsole). If there's a better way to do this, please let me know. I'm not sure of that, but there should be. But for now, having (multiport) there allows us to detect that multiport virtio console is supported. But suggesting -virtioconsole supports multiport is misleading and further usage of -virtioconsole is not to be encouraged as well.. I think you mean -device virtconsole here. Yeah; thanks. Will fix. So is this documentation correct or not? http://www.linux-kvm.org/page/VMchannel_Requirements#Invocation Yes, that's updated very recently and is according to what we have now. Amit
Re: [Qemu-devel] [PATCH v9 3/3] virtio-console: Add a new virtserialport device for generic serial port support
On (Tue) Oct 20 2009 [16:02:16], Gerd Hoffmann wrote:
On 10/20/09 11:48, Amit Shah wrote:
On (Tue) Oct 20 2009 [10:51:30], Gerd Hoffmann wrote:
@@ -107,3 +107,41 @@ static void virtcon_register(void)
virtio_serial_port_qdev_register(virtcon_info);
}
device_init(virtcon_register)
+static VirtIOSerialPortInfo virtserial_port_info = {
+.qdev.name = virtserialport,
+.qdev.size = sizeof(VirtConsole),
+.init = virtserial_port_initfn,
+.have_data = flush_buf,
+.qdev.props = (Property[]) {
+DEFINE_PROP_CHR(chardev, VirtConsole, chr),
+DEFINE_PROP_STRING(name, VirtIOSerialPort, name),
likewise: DEFINE_PROP_STRING(name, VirtConsole, port.name),
The 'name' field is common to all ports, so it makes sense to put it in
the VirtIOSerialPort struct. (Internal users can pre-define it to their
liking, eg, org.qemu.vnc)
Sure. I don't want to move it out there. But the driver state struct
is 'VirtConsole' so all properties should use that. Note that the field
changed too: from name to port.name, so everything keeps working ;)
Ah; I understand it now. Sorry for the confusion.
Thanks Gerd for the reviews.
Amit
Re: [Qemu-devel] [PATCH 02/11] Add support for qfloat
On (Mon) Oct 19 2009 [09:18:19], Anthony Liguori wrote: + * This work is licensed under the terms of the GNU GPL, version 2. See + * the COPYING file in the top-level directory. + * + * Copyright IBM, Corp. 2009 + * + * Authors: + * Anthony Liguori aligu...@us.ibm.com + * + * This work is licensed under the terms of the GNU LGPL, version 2.1 or later. + * See the COPYING.LIB file in the top-level directory. + * + */ I see you're double licensing it to respect my initial choice, but you can leave only LGPL for this new code. I will send patches changing the ones written by me. Great, I'm hoping we can make the whole thing LGPL 2.1+ so that we can allow libraries like libvirt to use it. Why 'or later'? Let's just stick to a version that we know and accept. If you end up later liking version 3, you can change the text then. Amit
Re: [Qemu-devel] [PATCH 02/11] Add support for qfloat
On (Thu) Oct 22 2009 [09:01:48], Anthony Liguori wrote: Amit Shah wrote: On (Mon) Oct 19 2009 [09:18:19], Anthony Liguori wrote: + * This work is licensed under the terms of the GNU GPL, version 2. See + * the COPYING file in the top-level directory. + * + * Copyright IBM, Corp. 2009 + * + * Authors: + * Anthony Liguori aligu...@us.ibm.com + * + * This work is licensed under the terms of the GNU LGPL, version 2.1 or later. + * See the COPYING.LIB file in the top-level directory. + * + */ I see you're double licensing it to respect my initial choice, but you can leave only LGPL for this new code. I will send patches changing the ones written by me. Great, I'm hoping we can make the whole thing LGPL 2.1+ so that we can allow libraries like libvirt to use it. Why 'or later'? Let's just stick to a version that we know and accept. If you end up later liking version 3, you can change the text then. It really doesn't matter in the context of LGPL so I don't mind if we do 2.1 only. Yeah; better a known evil than an unknown one :-) Amit
Re: [Qemu-devel] Re: [PATCH v2 3/3] char: emit the OPENED event only when a new char connection is opened
On (Sat) Oct 24 2009 [12:36:54], Jan Kiszka wrote: Amit Shah wrote: The OPENED event gets sent also when qemu resets its state initially. The consumers of the event aren't interested in receiving this event on reset. The monitor was. Now its initial prompt on activation is broken. The patch in Anthony's queue, titled 'console: call qemu_chr_reset() in text_console_init' fixed that. However, with the qcow2 synchronous patch, the monitor prompt doesn't come up again -- which shows there is a problem with the way the bhs work and also the initial resets. I think the initial resets are a hack to work around something from my reading of it; do you have a better idea of why it's there and how it's all supposed to work? Does this patch fix/improve something for a different user? If not, please let us revert it. There's another question too: is a separate 'reset' event needed in addition to an 'opened' event? I have a few apps (that are coming as part of the virtio-console work) that need just an 'opened' event and are not interested in the 'reset' event. Amit
Re: [Qemu-devel] Re: [PATCH v2 3/3] char: emit the OPENED event only when a new char connection is opened
On (Mon) Oct 26 2009 [08:40:12], Jan Kiszka wrote: Amit Shah wrote: On (Sat) Oct 24 2009 [12:36:54], Jan Kiszka wrote: Amit Shah wrote: The OPENED event gets sent also when qemu resets its state initially. The consumers of the event aren't interested in receiving this event on reset. The monitor was. Now its initial prompt on activation is broken. The patch in Anthony's queue, titled 'console: call qemu_chr_reset() in text_console_init' You may also want to rename qemu_chr_reset - unless there is still a need for real reset. Yeah; there isn't a need after my patches -- I've been slowing working towards renaming it all. fixed that. However, with the qcow2 synchronous patch, the monitor prompt doesn't come up again -- which shows there is a problem with the way the bhs work and also the initial resets. Then the qcow2 patch is already in? At least applying your patch doesn't change the picture. Yeah; that patch went in just before my patches. Can you try reverting ef845c3bf421290153154635dc18eaa677cecb43 I think the initial resets are a hack to work around something from my reading of it; do you have a better idea of why it's there and how it's all supposed to work? From the monitor's POV, it's not a hack, it's simply the requirement to receive an indication that the console was opened. Just an indication that the monitor was opened -- agreed. But git history shows you added that as 'reset', so I'm wondering if maybe you wanted it to do something else as well (or you did it that way just because of the way qemu's bhs are handled?). Does this patch fix/improve something for a different user? If not, please let us revert it. There's another question too: is a separate 'reset' event needed in addition to an 'opened' event? Not for the monitor, but I cannot speak for other users. I think it would be good to check them in details before changing the reset/open semantic. As far as I could see in the git history, the 'reset' was added for the monitor. And the others could live with the double 'reset' events they were getting -- one for the reset and one when the device was opened. Amit
[Qemu-devel] [PATCH v10 0/2] virtio-console: Add support for multiple ports for generic guest-host communication
Hello, This iteration of the series fixes a style bug pointed out by Gerd and a typo noticed by Rich. In addition to that, it also introduces a few debug prints in the 'info qtree' output that shows some per-port data that could be helpful for debugging. I've been testing all the features that are presented here from the testsuite at http://fedorapeople.org/gitweb?p=amitshah/public_git/test-virtserial.git The testsuite tests for host-guest interaction as well as qemu chardev-virtioserial interaction. Amit Shah (2): virtio-console: Add a virtio-serial bus, support for multiple ports virtio-console: Add a new virtserialport device for generic serial port support Makefile.target|2 +- hw/pc.c|9 - hw/ppc440_bamboo.c |7 - hw/qdev.c |8 +- hw/virtio-console.c| 206 +++-- hw/virtio-console.h| 19 -- hw/virtio-pci.c|8 +- hw/virtio-serial-bus.c | 785 hw/virtio-serial.h | 227 ++ hw/virtio.h|2 +- qemu-options.hx|6 +- sysemu.h |6 - vl.c | 65 +++-- 13 files changed, 1171 insertions(+), 179 deletions(-) delete mode 100644 hw/virtio-console.h create mode 100644 hw/virtio-serial-bus.c create mode 100644 hw/virtio-serial.h
[Qemu-devel] [PATCH v10 2/2] virtio-console: Add a new virtserialport device for generic serial port support
This patch adds generic serial ports over the virtio serial bus.
These ports have a few more options that are not relevant for
virtio console ports: the ability to cache buffers that are
received for a port while it's disconnected, setting of limits
to the bytes that are cached so as to prevent OOM conditions,
etc.
Sample uses for such a device can be obtaining info from the
guest like the file systems used, apps installed, etc. for
offline usage and logged-in users, clipboard copy-paste, etc.
for online usage.
For requirements, use-cases, test cases and some history see
http://www.linux-kvm.org/page/VMchannel_Requirements
Signed-off-by: Amit Shah amit.s...@redhat.com
---
hw/virtio-console.c | 38 ++
1 files changed, 38 insertions(+), 0 deletions(-)
diff --git a/hw/virtio-console.c b/hw/virtio-console.c
index a14133b..c27e556 100644
--- a/hw/virtio-console.c
+++ b/hw/virtio-console.c
@@ -107,3 +107,41 @@ static void virtcon_register(void)
virtio_serial_port_qdev_register(virtcon_info);
}
device_init(virtcon_register)
+
+
+/* Generic Virtio Serial Ports */
+static int virtserial_port_initfn(VirtIOSerialDevice *dev)
+{
+VirtIOSerialPort *port = DO_UPCAST(VirtIOSerialPort, dev, dev-qdev);
+VirtConsole *vcon = DO_UPCAST(VirtConsole, port, port);
+
+port-info = dev-info;
+
+if (vcon-chr) {
+qemu_chr_add_handlers(vcon-chr, chr_can_read, chr_read, chr_event,
+ vcon);
+}
+return 0;
+}
+
+static VirtIOSerialPortInfo virtserial_port_info = {
+.qdev.name = virtserialport,
+.qdev.size = sizeof(VirtConsole),
+.init = virtserial_port_initfn,
+.have_data = flush_buf,
+.qdev.props = (Property[]) {
+DEFINE_PROP_CHR(chardev, VirtConsole, chr),
+DEFINE_PROP_STRING(name, VirtConsole, port.name),
+DEFINE_PROP_INT32(cache_buffers, VirtConsole, port.cache_buffers, 1),
+DEFINE_PROP_UINT64(byte_limit, VirtConsole, port.byte_limit, 0),
+DEFINE_PROP_UINT64(guest_byte_limit, VirtConsole,
+ port.guest_byte_limit, 0),
+DEFINE_PROP_END_OF_LIST(),
+},
+};
+
+static void virtserial_port_register(void)
+{
+virtio_serial_port_qdev_register(virtserial_port_info);
+}
+device_init(virtserial_port_register)
--
1.6.2.5
[Qemu-devel] [PATCH v10 1/2] virtio-console: Add a virtio-serial bus, support for multiple ports
This patch migrates virtio-console to the qdev infrastructure and
creates a new virtio-serial bus on which multiple ports are exposed as
devices. The bulk of the code now resides in a new file with
virtio-console.c being just a simple qdev device.
This interface enables spawning of multiple virtio consoles as well as generic
serial ports.
The older -virtconsole argument still works, but when using the new
functionality, it is recommended to use
-device virtio-serial-pci -device virtconsole,...
The virtconsole device type accepts a chardev as an argument and a 'name'
argument to identify the corresponding consoles on the host as well as the
guest. The name, if given, is exposed via the 'name' sysfs attribute.
Care has been taken to ensure compatibility with kernels that do not
support multiple ports as well as accepting incoming migrations from older
qemu versions.
Signed-off-by: Amit Shah amit.s...@redhat.com
---
Makefile.target|2 +-
hw/pc.c|9 -
hw/ppc440_bamboo.c |7 -
hw/qdev.c |8 +-
hw/virtio-console.c| 180 +---
hw/virtio-console.h| 19 --
hw/virtio-pci.c|8 +-
hw/virtio-serial-bus.c | 785
hw/virtio-serial.h | 227 ++
hw/virtio.h|2 +-
qemu-options.hx|6 +-
sysemu.h |6 -
vl.c | 65 +++--
13 files changed, 1139 insertions(+), 185 deletions(-)
delete mode 100644 hw/virtio-console.h
create mode 100644 hw/virtio-serial-bus.c
create mode 100644 hw/virtio-serial.h
diff --git a/Makefile.target b/Makefile.target
index 8d146c5..fd86eeb 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -157,7 +157,7 @@ ifdef CONFIG_SOFTMMU
obj-y = vl.o monitor.o pci.o machine.o gdbstub.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-console.o
virtio-pci.o
+obj-y += virtio-blk.o virtio-balloon.o virtio-net.o virtio-serial-bus.o
virtio-console.o virtio-pci.o
obj-$(CONFIG_KVM) += kvm.o kvm-all.o
obj-$(CONFIG_ISA_MMIO) += isa_mmio.o
LIBS+=-lz
diff --git a/hw/pc.c b/hw/pc.c
index 408d6d6..3dbe718 100644
--- a/hw/pc.c
+++ b/hw/pc.c
@@ -1319,15 +1319,6 @@ static void pc_init1(ram_addr_t ram_size,
pci_create_simple(pci_bus, -1, lsi53c895a);
}
}
-
-/* Add virtio console devices */
-if (pci_enabled) {
-for(i = 0; i MAX_VIRTIO_CONSOLES; i++) {
-if (virtcon_hds[i]) {
-pci_create_simple(pci_bus, -1, virtio-console-pci);
-}
-}
-}
}
static void pc_init_pci(ram_addr_t ram_size,
diff --git a/hw/ppc440_bamboo.c b/hw/ppc440_bamboo.c
index a488240..1ab9872 100644
--- a/hw/ppc440_bamboo.c
+++ b/hw/ppc440_bamboo.c
@@ -108,13 +108,6 @@ static void bamboo_init(ram_addr_t ram_size,
env = ppc440ep_init(ram_size, pcibus, pci_irq_nrs, 1, cpu_model);
if (pcibus) {
-/* Add virtio console devices */
-for(i = 0; i MAX_VIRTIO_CONSOLES; i++) {
-if (virtcon_hds[i]) {
-pci_create_simple(pcibus, -1, virtio-console-pci);
-}
-}
-
/* Register network interfaces. */
for (i = 0; i nb_nics; i++) {
/* There are no PCI NICs on the Bamboo board, but there are
diff --git a/hw/qdev.c b/hw/qdev.c
index 20f931c..374d2d0 100644
--- a/hw/qdev.c
+++ b/hw/qdev.c
@@ -321,13 +321,9 @@ void qdev_machine_creation_done(void)
CharDriverState *qdev_init_chardev(DeviceState *dev)
{
static int next_serial;
-static int next_virtconsole;
+
/* FIXME: This is a nasty hack that needs to go away. */
-if (strncmp(dev-info-name, virtio, 6) == 0) {
-return virtcon_hds[next_virtconsole++];
-} else {
-return serial_hds[next_serial++];
-}
+return serial_hds[next_serial++];
}
BusState *qdev_get_parent_bus(DeviceState *dev)
diff --git a/hw/virtio-console.c b/hw/virtio-console.c
index 57f8f89..a14133b 100644
--- a/hw/virtio-console.c
+++ b/hw/virtio-console.c
@@ -1,143 +1,109 @@
/*
- * Virtio Console Device
+ * Virtio Console and Generic Port Devices
*
- * Copyright IBM, Corp. 2008
+ * Copyright Red Hat, Inc. 2009
*
* Authors:
- * Christian Ehrhardt ehrha...@linux.vnet.ibm.com
+ * Amit Shah amit.s...@redhat.com
*
* This work is licensed under the terms of the GNU GPL, version 2. See
* the COPYING file in the top-level directory.
- *
*/
-#include hw.h
#include qemu-char.h
-#include virtio.h
-#include virtio-console.h
-
+#include virtio-serial.h
-typedef struct VirtIOConsole
-{
-VirtIODevice vdev;
-VirtQueue *ivq, *ovq;
+typedef struct VirtConsole {
+VirtIOSerialPort port;
CharDriverState *chr;
-} VirtIOConsole;
+} VirtConsole;
-static VirtIOConsole *to_virtio_console(VirtIODevice *vdev)
-{
-return (VirtIOConsole *)vdev;
-}
-static void
Re: [Qemu-devel] Re: [PATCH v2 3/3] char: emit the OPENED event only when a new char connection is opened
On (Mon) Oct 26 2009 [21:15:57], Jan Kiszka wrote: However, with the qcow2 synchronous patch, the monitor prompt doesn't come up again -- which shows there is a problem with the way the bhs work and also the initial resets. Then the qcow2 patch is already in? At least applying your patch doesn't change the picture. Yeah; that patch went in just before my patches. Can you try reverting ef845c3bf421290153154635dc18eaa677cecb43 Makes no difference either - but it also does not touch any code that code impact the open/reset signaling. What happens is the BHs that are run get run in a different order. Let me explain the two scenarios: 1. Current qemu tree, plus my patch to fix this issue that's in Anthony's queue plus reverting the qcow2 patch -- the monitor prompt appears fine. 2. Current qemu tree plus my patch from Anthony's queue - the monitor prompt doesn't appear. The difference is in the order the BHs are scheduled. In the case without the qcow2 patch, the bhs get scheduled early on and results in initial_reset_issued getting set. Later, when the char devs are initialsed, the OPENED event is sent out. In the case with the qcow2 patch, the bhs are run in a different order -- the bhs are scheduled but not run, and when the char init happens, the condition if (s-bh == NULL) is false and the bhs in effect get scheduled only once, not emitting the opened event. Of course, the qcow2 patch just causes some conditions for the bh handling to happen differently which I've not examined yet. I just tracked why this was happening. All that said, I'm ok with reverting that patch now till I find some kind of a solution to this. Amit
Re: [Qemu-devel] Re: [PATCH v2 3/3] char: emit the OPENED event only when a new char connection is opened
On (Tue) Oct 27 2009 [09:40:27], Kevin Wolf wrote: All that said, I'm ok with reverting that patch now till I find some kind of a solution to this. Which patch do you want to revert? You're aware that the qcow2 patch is a data corruption fix? Ah, no. Reverting my patch that causes this problem. I know the qcow2 patch only exposes the bh handling issue. I intend to fix that appropriately elsewhere :-) Amit
Re: [Qemu-devel] Re: [PATCH v2 3/3] char: emit the OPENED event only when a new char connection is opened
On (Tue) Oct 27 2009 [09:04:50], Anthony Liguori wrote: Amit Shah wrote: On (Tue) Oct 27 2009 [09:40:27], Kevin Wolf wrote: All that said, I'm ok with reverting that patch now till I find some kind of a solution to this. Which patch do you want to revert? You're aware that the qcow2 patch is a data corruption fix? Ah, no. Reverting my patch that causes this problem. I know the qcow2 patch only exposes the bh handling issue. I intend to fix that appropriately elsewhere :-) How does Kevin's latest patches that introduces new BHs semantics affect all of this? I explained that a couple of mails ago in this thread in a reply to Jan. Basically, the order in which the BHs run causes the problem, so it's a race, and Kevin's patch make the BHs run later. Amit
[Qemu-devel] [PATCH] qdev: Check if unplug handler exists before calling it
A bus may have hotplugging enabled but not have the 'unplug'
callback defined, which would lead to a crash on trying to
unplug a device on the bus.
Fix by checking if the callback is valid
Signed-off-by: Amit Shah amit.s...@redhat.com
---
hw/qdev.c |5 +
1 files changed, 5 insertions(+), 0 deletions(-)
diff --git a/hw/qdev.c b/hw/qdev.c
index 374d2d0..1a9eb2e 100644
--- a/hw/qdev.c
+++ b/hw/qdev.c
@@ -258,6 +258,11 @@ int qdev_unplug(DeviceState *dev)
dev-parent_bus-name);
return -1;
}
+if (!dev-info-unplug) {
+qemu_error(Unplug event for bus %s not defined\n,
+ dev-parent_bus-name);
+return -1;
+}
return dev-info-unplug(dev);
}
--
1.6.2.5
[Qemu-devel] [PATCH] qdev: Check if unplug handler exists before calling it
A bus may have hotplugging enabled but not have the 'unplug' callback defined, which would lead to a crash on trying to unplug a device on the bus. Fix by introducing an assert to check if the callback is valid. Signed-off-by: Amit Shah amit.s...@redhat.com --- hw/qdev.c |2 ++ 1 files changed, 2 insertions(+), 0 deletions(-) diff --git a/hw/qdev.c b/hw/qdev.c index c7884d0..d19d531 100644 --- a/hw/qdev.c +++ b/hw/qdev.c @@ -258,6 +258,8 @@ int qdev_unplug(DeviceState *dev) dev-parent_bus-name); return -1; } +assert(dev-info-unplug != NULL); + return dev-info-unplug(dev); } -- 1.6.2.5
[Qemu-devel] [PATCH] char: tcp: increase size of buffer that holds data to be sent out
1k is too less; at least send out 4k of data from a chardev.
Signed-off-by: Amit Shah amit.s...@redhat.com
---
qemu-char.c |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/qemu-char.c b/qemu-char.c
index 0fd402c..34c0a63 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -1996,7 +1996,7 @@ static void tcp_chr_read(void *opaque)
{
CharDriverState *chr = opaque;
TCPCharDriver *s = chr-opaque;
-uint8_t buf[1024];
+uint8_t buf[4096];
int len, size;
if (!s-connected || s-max_size = 0)
--
1.6.2.5
[Qemu-devel] Re: [PATCH] char: tcp: increase size of buffer that holds data to be sent out
On (Mon) Nov 02 2009 [21:59:58], Amit Shah wrote:
1k is too less; at least send out 4k of data from a chardev.
This one only touches unix/tcp sockets; the others use a 1k buffer as
well; I'll send a new patch that converts all the users to a consistent
buffer size.
Signed-off-by: Amit Shah amit.s...@redhat.com
---
qemu-char.c |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/qemu-char.c b/qemu-char.c
index 0fd402c..34c0a63 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -1996,7 +1996,7 @@ static void tcp_chr_read(void *opaque)
{
CharDriverState *chr = opaque;
TCPCharDriver *s = chr-opaque;
-uint8_t buf[1024];
+uint8_t buf[4096];
int len, size;
if (!s-connected || s-max_size = 0)
--
1.6.2.5
Amit
Re: [Qemu-devel] [PATCH] char: tcp: increase size of buffer that holds data to be sent out
On (Mon) Nov 02 2009 [10:52:02], Anthony Liguori wrote: Amit Shah wrote: 1k is too less; at least send out 4k of data from a chardev. Why is 1k too small? Definitely depends on the apps that pump in the data. If an app has data to pump, it will keep pumping as much data as possible (eg, Linux syscalls issue read / write requests for 32k bytes of data). The char devices have a 'can_read' call to determine how much the backend can accept. We're artificially limiting to 1k in this code, even if the app can pump and the backend can receive, resulting in multiple splits. In the case of virtio, 4k is available if the guest is able to receive data. Amit
[Qemu-devel] char: remove init_reset handling, more data per write()
Hello, These patches, all independent of each other, make the char backend a little better by removing the initial_reset handling that's unnecessary, bump up the amount of data that's sent per write() call to 4k from the current 1k and also renames the generic char open() command to reflect reality. Please apply. Amit
[Qemu-devel] [PATCH 1/3] char: don't limit data sent to backends to 1k per buffer
chardevs have a 'can_read' function via which backends specify
the amount of data they can receive. When can_read returns 0,
apps can start sending data. However, each chardev driver here
allows a max. of 1k bytes inspite of the backend being able to
receive more.
The best we can do here is to allocate s-max_size bytes from
the heap on each call (which is the number returned by the
backend from the can_read call).
This is an intermediate step to bump up the bytes written in
each call to 4k.
Signed-off-by: Amit Shah amit.s...@redhat.com
---
qemu-char.c | 14 --
1 files changed, 8 insertions(+), 6 deletions(-)
diff --git a/qemu-char.c b/qemu-char.c
index 40bd7e8..1f63019 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -97,6 +97,8 @@
#include qemu_socket.h
+#define READ_BUF_LEN 4096
+
/***/
/* character device */
@@ -172,7 +174,7 @@ void qemu_chr_accept_input(CharDriverState *s)
void qemu_chr_printf(CharDriverState *s, const char *fmt, ...)
{
-char buf[4096];
+char buf[READ_BUF_LEN];
va_list ap;
va_start(ap, fmt);
vsnprintf(buf, sizeof(buf), fmt, ap);
@@ -555,7 +557,7 @@ static void fd_chr_read(void *opaque)
CharDriverState *chr = opaque;
FDCharDriver *s = chr-opaque;
int size, len;
-uint8_t buf[1024];
+uint8_t buf[READ_BUF_LEN];
len = sizeof(buf);
if (len s-max_size)
@@ -866,7 +868,7 @@ static void pty_chr_read(void *opaque)
CharDriverState *chr = opaque;
PtyCharDriver *s = chr-opaque;
int size, len;
-uint8_t buf[1024];
+uint8_t buf[READ_BUF_LEN];
len = sizeof(buf);
if (len s-read_bytes)
@@ -1554,7 +1556,7 @@ static void win_chr_readfile(CharDriverState *chr)
{
WinCharState *s = chr-opaque;
int ret, err;
-uint8_t buf[1024];
+uint8_t buf[READ_BUF_LEN];
DWORD size;
ZeroMemory(s-orecv, sizeof(s-orecv));
@@ -1760,7 +1762,7 @@ static CharDriverState
*qemu_chr_open_win_file_out(QemuOpts *opts)
typedef struct {
int fd;
-uint8_t buf[1024];
+uint8_t buf[READ_BUF_LEN];
int bufcnt;
int bufptr;
int max_size;
@@ -2020,7 +2022,7 @@ static void tcp_chr_read(void *opaque)
{
CharDriverState *chr = opaque;
TCPCharDriver *s = chr-opaque;
-uint8_t buf[1024];
+uint8_t buf[READ_BUF_LEN];
int len, size;
if (!s-connected || s-max_size = 0)
--
1.6.2.5
[Qemu-devel] [PATCH 2/3] char: Remove special init_reset handling
The initial_reset sent to chardevs doesn't do much other than setting
a bool to true. Char devices are interested in the open event and
that gets sent whenever the device is opened.
Moreover, the reset logic breaks as and when qemu's bh scheduling
changes.
Signed-off-by: Amit Shah amit.s...@redhat.com
---
qemu-char.c |9 -
qemu-char.h |1 -
vl.c|1 -
3 files changed, 0 insertions(+), 11 deletions(-)
diff --git a/qemu-char.c b/qemu-char.c
index 1f63019..d78bae3 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -128,15 +128,6 @@ void qemu_chr_reset(CharDriverState *s)
}
}
-void qemu_chr_initial_reset(void)
-{
-CharDriverState *chr;
-
-QTAILQ_FOREACH(chr, chardevs, next) {
-qemu_chr_reset(chr);
-}
-}
-
int qemu_chr_write(CharDriverState *s, const uint8_t *buf, int len)
{
return s-chr_write(s, buf, len);
diff --git a/qemu-char.h b/qemu-char.h
index 05fe15d..b420111 100644
--- a/qemu-char.h
+++ b/qemu-char.h
@@ -83,7 +83,6 @@ void qemu_chr_add_handlers(CharDriverState *s,
void *opaque);
int qemu_chr_ioctl(CharDriverState *s, int cmd, void *arg);
void qemu_chr_reset(CharDriverState *s);
-void qemu_chr_initial_reset(void);
int qemu_chr_can_read(CharDriverState *s);
void qemu_chr_read(CharDriverState *s, uint8_t *buf, int len);
int qemu_chr_get_msgfd(CharDriverState *s);
diff --git a/vl.c b/vl.c
index e57f58f..5ad4e42 100644
--- a/vl.c
+++ b/vl.c
@@ -5750,7 +5750,6 @@ int main(int argc, char **argv, char **envp)
}
text_consoles_set_display(display_state);
-qemu_chr_initial_reset();
for (i = 0; i MAX_MONITOR_DEVICES; i++) {
if (monitor_devices[i] monitor_hds[i]) {
--
1.6.2.5
[Qemu-devel] [PATCH 3/3] char: rename qemu_chr_reset to qemu_chr_generic_open
This function sends out the OPENED event to backends that
have drive the chardevs. The 'reset' is now a historical
artifact and we can now just call the function for what it
is.
Signed-off-by: Amit Shah amit.s...@redhat.com
---
console.c |2 +-
hw/baum.c |2 +-
qemu-char.c | 22 +++---
qemu-char.h |2 +-
4 files changed, 14 insertions(+), 14 deletions(-)
diff --git a/console.c b/console.c
index 9bbef59..82ddbe4 100644
--- a/console.c
+++ b/console.c
@@ -1384,7 +1384,7 @@ static void text_console_do_init(CharDriverState *chr,
DisplayState *ds, QemuOpt
s-t_attrib = s-t_attrib_default;
text_console_resize(s);
-qemu_chr_reset(chr);
+qemu_chr_generic_open(chr);
if (chr-init)
chr-init(chr);
}
diff --git a/hw/baum.c b/hw/baum.c
index 8a12985..fa356ec 100644
--- a/hw/baum.c
+++ b/hw/baum.c
@@ -614,7 +614,7 @@ CharDriverState *chr_baum_init(QemuOpts *opts)
qemu_set_fd_handler(baum-brlapi_fd, baum_chr_read, NULL, baum);
-qemu_chr_reset(chr);
+qemu_chr_generic_open(chr);
return chr;
diff --git a/qemu-char.c b/qemu-char.c
index d78bae3..5a81e8f 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -112,7 +112,7 @@ static void qemu_chr_event(CharDriverState *s, int event)
s-chr_event(s-handler_opaque, event);
}
-static void qemu_chr_reset_bh(void *opaque)
+static void qemu_chr_generic_open_bh(void *opaque)
{
CharDriverState *s = opaque;
qemu_chr_event(s, CHR_EVENT_OPENED);
@@ -120,10 +120,10 @@ static void qemu_chr_reset_bh(void *opaque)
s-bh = NULL;
}
-void qemu_chr_reset(CharDriverState *s)
+void qemu_chr_generic_open(CharDriverState *s)
{
if (s-bh == NULL) {
- s-bh = qemu_bh_new(qemu_chr_reset_bh, s);
+ s-bh = qemu_bh_new(qemu_chr_generic_open_bh, s);
qemu_bh_schedule(s-bh);
}
}
@@ -610,7 +610,7 @@ static CharDriverState *qemu_chr_open_fd(int fd_in, int
fd_out)
chr-chr_update_read_handler = fd_chr_update_read_handler;
chr-chr_close = fd_chr_close;
-qemu_chr_reset(chr);
+qemu_chr_generic_open(chr);
return chr;
}
@@ -910,7 +910,7 @@ static void pty_chr_state(CharDriverState *chr, int
connected)
qemu_mod_timer(s-timer, qemu_get_clock(rt_clock) + 1000);
} else {
if (!s-connected)
-qemu_chr_reset(chr);
+qemu_chr_generic_open(chr);
s-connected = 1;
}
}
@@ -1184,7 +1184,7 @@ static CharDriverState *qemu_chr_open_tty(QemuOpts *opts)
return NULL;
}
chr-chr_ioctl = tty_serial_ioctl;
-qemu_chr_reset(chr);
+qemu_chr_generic_open(chr);
return chr;
}
#else /* ! __linux__ ! __sun__ */
@@ -1330,7 +1330,7 @@ static CharDriverState *qemu_chr_open_pp(QemuOpts *opts)
chr-chr_close = pp_close;
chr-opaque = drv;
-qemu_chr_reset(chr);
+qemu_chr_generic_open(chr);
return chr;
}
@@ -1611,7 +1611,7 @@ static CharDriverState *qemu_chr_open_win(QemuOpts *opts)
free(chr);
return NULL;
}
-qemu_chr_reset(chr);
+qemu_chr_generic_open(chr);
return chr;
}
@@ -1711,7 +1711,7 @@ static CharDriverState *qemu_chr_open_win_pipe(QemuOpts
*opts)
free(chr);
return NULL;
}
-qemu_chr_reset(chr);
+qemu_chr_generic_open(chr);
return chr;
}
@@ -1725,7 +1725,7 @@ static CharDriverState *qemu_chr_open_win_file(HANDLE
fd_out)
s-hcom = fd_out;
chr-opaque = s;
chr-chr_write = win_chr_write;
-qemu_chr_reset(chr);
+qemu_chr_generic_open(chr);
return chr;
}
@@ -2052,7 +2052,7 @@ static void tcp_chr_connect(void *opaque)
s-connected = 1;
qemu_set_fd_handler2(s-fd, tcp_chr_read_poll,
tcp_chr_read, NULL, chr);
-qemu_chr_reset(chr);
+qemu_chr_generic_open(chr);
}
#define IACSET(x,a,b,c) x[0] = a; x[1] = b; x[2] = c;
diff --git a/qemu-char.h b/qemu-char.h
index b420111..9957db1 100644
--- a/qemu-char.h
+++ b/qemu-char.h
@@ -82,7 +82,7 @@ void qemu_chr_add_handlers(CharDriverState *s,
IOEventHandler *fd_event,
void *opaque);
int qemu_chr_ioctl(CharDriverState *s, int cmd, void *arg);
-void qemu_chr_reset(CharDriverState *s);
+void qemu_chr_generic_open(CharDriverState *s);
int qemu_chr_can_read(CharDriverState *s);
void qemu_chr_read(CharDriverState *s, uint8_t *buf, int len);
int qemu_chr_get_msgfd(CharDriverState *s);
--
1.6.2.5
[Qemu-devel] Re: [PATCH 2/3] char: Remove special init_reset handling
On (Tue) Nov 03 2009 [18:08:57], Jan Kiszka wrote: Amit Shah wrote: The initial_reset sent to chardevs doesn't do much other than setting a bool to true. Char devices are interested in the open event and that gets sent whenever the device is opened. Have you checked with the monitor in all use cases (dedicated muxed console, stdio SDL console, etc.)? It was introduced once to fix a I've checked with -monitor stdio, monitor in SDL and also chardevs using unix sockets. I've not tried mux yet; I'll try that and report back. BTW if it ends up not working with this patch, it'd be broken in the current master as well. corner case, I think it's even documented... Hm; I'll have to look really deep; haven't found any such thing yet. Amit
[Qemu-devel] Re: [PATCH 2/3] char: Remove special init_reset handling
On (Tue) Nov 03 2009 [23:25:52], Amit Shah wrote: On (Tue) Nov 03 2009 [18:08:57], Jan Kiszka wrote: Amit Shah wrote: The initial_reset sent to chardevs doesn't do much other than setting a bool to true. Char devices are interested in the open event and that gets sent whenever the device is opened. Have you checked with the monitor in all use cases (dedicated muxed console, stdio SDL console, etc.)? It was introduced once to fix a I've checked with -monitor stdio, monitor in SDL and also chardevs using unix sockets. I've not tried mux yet; I'll try that and report back. BTW if it ends up not working with this patch, it'd be broken in the current master as well. I tried with: -chardev stdio,mux=on,id=mux -monitor chardev:mux -serial chardev:mux The monitor prompt shows up as does the serial output. (btw I've also tried closing and opening char devs and that works fine too) Amit
[Qemu-devel] Re: [PATCH 2/3] char: Remove special init_reset handling
On (Tue) Nov 03 2009 [19:53:43], Jan Kiszka wrote: Amit Shah wrote: On (Tue) Nov 03 2009 [23:25:52], Amit Shah wrote: On (Tue) Nov 03 2009 [18:08:57], Jan Kiszka wrote: Amit Shah wrote: The initial_reset sent to chardevs doesn't do much other than setting a bool to true. Char devices are interested in the open event and that gets sent whenever the device is opened. Have you checked with the monitor in all use cases (dedicated muxed console, stdio SDL console, etc.)? It was introduced once to fix a I've checked with -monitor stdio, monitor in SDL and also chardevs using unix sockets. I've not tried mux yet; I'll try that and report back. BTW if it ends up not working with this patch, it'd be broken in the current master as well. I tried with: -chardev stdio,mux=on,id=mux -monitor chardev:mux -serial chardev:mux The monitor prompt shows up as does the serial output. (btw I've also tried closing and opening char devs and that works fine too) That sounds good. Then something must have changed since 2970a6c943, do you see what? I think that depended on the resets being sent. I've now removed the need for resets altogether. Amit
[Qemu-devel] Re: [PATCH 2/3] char: Remove special init_reset handling
On (Wed) Nov 04 2009 [10:39:39], Jan Kiszka wrote: Amit Shah wrote: On (Tue) Nov 03 2009 [19:53:43], Jan Kiszka wrote: Amit Shah wrote: On (Tue) Nov 03 2009 [23:25:52], Amit Shah wrote: On (Tue) Nov 03 2009 [18:08:57], Jan Kiszka wrote: Amit Shah wrote: The initial_reset sent to chardevs doesn't do much other than setting a bool to true. Char devices are interested in the open event and that gets sent whenever the device is opened. Have you checked with the monitor in all use cases (dedicated muxed console, stdio SDL console, etc.)? It was introduced once to fix a I've checked with -monitor stdio, monitor in SDL and also chardevs using unix sockets. I've not tried mux yet; I'll try that and report back. BTW if it ends up not working with this patch, it'd be broken in the current master as well. I tried with: -chardev stdio,mux=on,id=mux -monitor chardev:mux -serial chardev:mux The monitor prompt shows up as does the serial output. (btw I've also tried closing and opening char devs and that works fine too) That sounds good. Then something must have changed since 2970a6c943, do you see what? I think that depended on the resets being sent. I've now removed the need for resets altogether. No, this is in fact the reason why we no longer need it: 9a1e948129 (Introduce contexts for asynchronous callbacks) As the initial reset of the char device that is marked pending on open is now no longer consumed by the IDE initialization, we can actually drop the later regeneration via qemu_chr_initial_reset. I just hope this stays like it is... I tested this even on a tree that doesn't have this patch. I haven't really delved deep to see why this was added earlier -- the commit log only says very little. Plus my testing with the current tree works fine so I'm happy to mention these things in the commit log. Amit
Re: [Qemu-devel] virtio-rng
On (Mon) Nov 16 2009 [17:58:22], Ian Molton wrote: -BEGIN PGP SIGNED MESSAGE- Hash: SHA1 Gerd Hoffmann wrote: Maybe ... -chardev socket,id=egd,host=egd.domain.tld,port=whatever -device virtio-rng,chardev=egd I've had a go at modifying virtio-console.c to use these semantics, attached below. I'd appreciate it if you could let me know if this is 'the right way'. I'm doing that as part of revamping of the virtio-console driver. You can see the latest patch I sent out here: http://patchwork.ozlabs.org/patch/36901/ Amit
Re: [Qemu-devel] virtio-rng
On (Tue) Nov 17 2009 [11:10:32], Ian Molton wrote: -BEGIN PGP SIGNED MESSAGE- Hash: SHA1 Amit Shah wrote: (Any reason to take this off-list?) None other than hitting reply rather than reply-all. CCing list once more :-) Either way, you still need to specify the properties - they've just moved into the console driver in your patch by the look of it. Yeah; there are two ways of setting properties -- from the command line or from code. What you're doing is something like -virtiorng,a=foo,b=bar Ah, I see why we're at cross purposes here - I thought we'd moved to discussing virtio-console. I've already moved to qdev based init for virtio-rng - 'fixing' virtio-console was where I learnt about the 'new way' (qdev). I actually meant to talk about virtio-console but had the virtio-rng example ready in mind. I thought you did the same thing for console too? Sorry I've not really looked at the patch in detail so you can disregard that comment. I'd prefer to do the same for virtio-rng, Here I refer to you having (and I didnt apply the patch so I may have misread) dropped the virtio-pci proxy from virtio-console. Once I've got a tree I can apply your patch to I'll take another look :-) It slightly works differently now: virtio is supposed to be pci-agnostic. So what now happens is: virtio-console hooks on to the virtio-serial-bus via -device virtconsole virtio-serial-bus hooks on to the virtio-pci-proxy via -device virtio-serial-pci Yes, I saw that. Would it not be better to generate the device / chardev pair dynamically though, rather than preserve the icky old array? I don't understand what you mean by 'dynamically'. Rather than have that array virtcon_hds at all, create the pairs of host side data and qdev properties during parsing the commandline - no need to store them up and iterate through the later, or to set an arbitrary limit on how many can be specified in that way. qdev won't understand the old-style commandline syntax; so once -virtioconsole is encounters, all parsing of the arguments for that param are to be done by the code that handles -virtioconsole. The array is maintained because multiple virtio-consoles could be spawned, upto a max. of MAX_VIRTIO_CONSOLES. But, as it stands, MAX_VIRTIO_CONSOLES is 1 and so the array can be dropped, but a local var. would still be needed to fetch the chardev and then init it properly using qemu_opt. Might be an idea for qemu to warn people that this syntax will be deprecated, too... Yes, that should be done (maybe for 1-2 releases). Amit
Re: [Qemu-devel] [PATCH 09/10] qdev: Use QError for 'device not found' error
On (Wed) Nov 18 2009 [16:17:02], Markus Armbruster wrote:
Luiz Capitulino lcapitul...@redhat.com writes:
@@ -176,8 +177,7 @@ DeviceState *qdev_device_add(QemuOpts *opts)
/* find driver */
info = qdev_find_info(NULL, driver);
if (!info) {
-qemu_error(Device \%s\ not found. Try -device '?' for a
list.\n,
- driver);
+qemu_error_new(QERR_DEVICE_NOT_FOUND, driver);
return NULL;
}
if (info-no_user) {
Not obvious from this patch, but we lose the Try -device '?' for a
list hint here. In PATCH 7/10:
BTW that hint isn't always appropriate as it's printed on the monitor
when doing 'device_add' as well.
Amit
Re: [Qemu-devel] [PATCH] qemu-io: Fix memory leak
On (Wed) Nov 18 2009 [10:42:59], Kevin Wolf wrote:
Signed-off-by: Kevin Wolf kw...@redhat.com
---
qemu-io.c | 10 ++
1 files changed, 6 insertions(+), 4 deletions(-)
diff --git a/qemu-io.c b/qemu-io.c
index cac72e9..c84b361 100644
--- a/qemu-io.c
+++ b/qemu-io.c
@@ -129,7 +129,8 @@ create_iovec(QEMUIOVector *qiov, char **argv, int nr_iov,
int pattern)
{
size_t *sizes = calloc(nr_iov, sizeof(size_t));
size_t count = 0;
- void *buf, *p;
+ void *buf = NULL;
+ void *p;
int i;
I'd prefer the init to happen after the declarations -- brings in
consistent style, puts declarations in one blob and makes
initialisations explicit.
Amit
[Qemu-devel] [PATCH 1/2] qdev: Add a DEV_NVECTORS_UNSPECIFIED enum for unspecified nr of MSI vectors
net.c used a constant to signify no MSI vectors were specified. Extend
that to all qdev devices.
Signed-off-by: Amit Shah amit.s...@redhat.com
Reported-by: Michael S. Tsirkin m...@redhat.com
---
hw/qdev.c |2 +-
hw/qdev.h |4
net.c |6 --
net.h |3 ---
4 files changed, 9 insertions(+), 6 deletions(-)
diff --git a/hw/qdev.c b/hw/qdev.c
index d0052d4..b634890 100644
--- a/hw/qdev.c
+++ b/hw/qdev.c
@@ -387,7 +387,7 @@ void qdev_set_nic_properties(DeviceState *dev, NICInfo *nd)
qdev_prop_set_vlan(dev, vlan, nd-vlan);
if (nd-netdev)
qdev_prop_set_netdev(dev, netdev, nd-netdev);
-if (nd-nvectors != NIC_NVECTORS_UNSPECIFIED
+if (nd-nvectors != DEV_NVECTORS_UNSPECIFIED
qdev_prop_exists(dev, vectors)) {
qdev_prop_set_uint32(dev, vectors, nd-nvectors);
}
diff --git a/hw/qdev.h b/hw/qdev.h
index 0eb45b0..adfcf79 100644
--- a/hw/qdev.h
+++ b/hw/qdev.h
@@ -24,6 +24,10 @@ enum DevState {
DEV_STATE_INITIALIZED,
};
+enum {
+DEV_NVECTORS_UNSPECIFIED = -1,
+};
+
/* This structure should not be accessed directly. We declare it here
so that it can be embedded in individual device state structures. */
struct DeviceState {
diff --git a/net.c b/net.c
index a1bf49f..dd3962e 100644
--- a/net.c
+++ b/net.c
@@ -35,6 +35,7 @@
#include sysemu.h
#include qemu-common.h
#include qemu_socket.h
+#include hw/qdev.h
static QTAILQ_HEAD(, VLANState) vlans;
static QTAILQ_HEAD(, VLANClientState) non_vlan_clients;
@@ -804,8 +805,9 @@ static int net_init_nic(QemuOpts *opts,
return -1;
}
-nd-nvectors = qemu_opt_get_number(opts, vectors,
NIC_NVECTORS_UNSPECIFIED);
-if (nd-nvectors != NIC_NVECTORS_UNSPECIFIED
+nd-nvectors = qemu_opt_get_number(opts, vectors,
+ DEV_NVECTORS_UNSPECIFIED);
+if (nd-nvectors != DEV_NVECTORS_UNSPECIFIED
(nd-nvectors 0 || nd-nvectors 0x7ff)) {
qemu_error(invalid # of vectors: %d\n, nd-nvectors);
return -1;
diff --git a/net.h b/net.h
index 33a1eaf..16f19c5 100644
--- a/net.h
+++ b/net.h
@@ -123,9 +123,6 @@ void do_set_link(Monitor *mon, const QDict *qdict);
/* NIC info */
#define MAX_NICS 8
-enum {
- NIC_NVECTORS_UNSPECIFIED = -1
-};
struct NICInfo {
uint8_t macaddr[6];
--
1.6.2.5
[Qemu-devel] [PATCH 2/2] virtio-pci: Use DEV_NVECTORS_UNSPECIFIED instead of -1 for virtio-serial
Use the named constant instead of -1.
Signed-off-by: Amit Shah amit.s...@redhat.com
Reported-by: Michael S. Tsirkin m...@redhat.com
---
hw/virtio-pci.c |8 +---
1 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/hw/virtio-pci.c b/hw/virtio-pci.c
index bcd40f7..799f664 100644
--- a/hw/virtio-pci.c
+++ b/hw/virtio-pci.c
@@ -500,8 +500,9 @@ static int virtio_serial_init_pci(PCIDevice *pci_dev)
if (!vdev) {
return -1;
}
-vdev-nvectors = proxy-nvectors == -1 ? proxy-max_virtserial_ports + 1
- : proxy-nvectors;
+vdev-nvectors = proxy-nvectors == DEV_NVECTORS_UNSPECIFIED
+? proxy-max_virtserial_ports + 1
+: proxy-nvectors;
virtio_init_pci(proxy, vdev,
PCI_VENDOR_ID_REDHAT_QUMRANET,
PCI_DEVICE_ID_VIRTIO_CONSOLE,
@@ -585,7 +586,8 @@ static PCIDeviceInfo virtio_info[] = {
.init = virtio_serial_init_pci,
.exit = virtio_exit_pci,
.qdev.props = (Property[]) {
-DEFINE_PROP_UINT32(vectors, VirtIOPCIProxy, nvectors, -1),
+DEFINE_PROP_UINT32(vectors, VirtIOPCIProxy, nvectors,
+ DEV_NVECTORS_UNSPECIFIED),
DEFINE_PROP_HEX32(class, VirtIOPCIProxy, class_code, 0),
DEFINE_VIRTIO_COMMON_FEATURES(VirtIOPCIProxy, host_features),
DEFINE_PROP_UINT32(max_ports, VirtIOPCIProxy,
max_virtserial_ports,
--
1.6.2.5
[Qemu-devel] [PATCH 0/3] s390: Fix compile, add 0.12 machine type, backward compat
Hello, This series fixes a compile error and adds 0.12 and 0.13 machine types, disabling newer virtio-serial features for 0.12. Note: The name for the machine type changes from s390-virtio to s390-virtio-vXX, this may not be compatible for 0.12. Amit Shah (3): s390-virtio: Fix compile error for virtio-block init s390: Add a 0.12 machine type for compatibility s390-virtio: Disable new virtio-serial features for 0.12 machine type hw/s390-virtio-bus.c |2 +- hw/s390-virtio.c | 26 +- 2 files changed, 26 insertions(+), 2 deletions(-)
[Qemu-devel] [PATCH 2/3] s390: Add a 0.12 machine type for compatibility
Add a 0.12 s390-virtio machine type for compatibility with older
versions.
Signed-off-by: Amit Shah amit.s...@redhat.com
---
hw/s390-virtio.c | 14 +-
1 files changed, 13 insertions(+), 1 deletions(-)
diff --git a/hw/s390-virtio.c b/hw/s390-virtio.c
index 3582728..a3cbfb6 100644
--- a/hw/s390-virtio.c
+++ b/hw/s390-virtio.c
@@ -242,7 +242,7 @@ static void s390_init(ram_addr_t ram_size,
}
static QEMUMachine s390_machine = {
-.name = s390-virtio,
+.name = s390-virtio-0.13,
.alias = s390,
.desc = VirtIO based S390 machine,
.init = s390_init,
@@ -254,9 +254,21 @@ static QEMUMachine s390_machine = {
.is_default = 1,
};
+static QEMUMachine s390_machine_v0_12 = {
+.name = s390-virtio-0.12,
+.desc = VirtIO based S390 machine for v0.12 compatibility,
+.init = s390_init,
+.no_serial = 1,
+.no_parallel = 1,
+.use_virtcon = 1,
+.no_vga = 1,
+.max_cpus = 255,
+};
+
static void s390_machine_init(void)
{
qemu_register_machine(s390_machine);
+qemu_register_machine(s390_machine_v0_12);
}
machine_init(s390_machine_init);
--
1.6.2.5
[Qemu-devel] [PATCH 3/3] s390-virtio: Disable new virtio-serial features for 0.12 machine type
Disable the MULTIPORT feature and MSI vectors for the 0.12 machine
types; those features are added only for 0.13 onwards.
Signed-off-by: Amit Shah amit.s...@redhat.com
---
hw/s390-virtio.c | 12
1 files changed, 12 insertions(+), 0 deletions(-)
diff --git a/hw/s390-virtio.c b/hw/s390-virtio.c
index a3cbfb6..f027521 100644
--- a/hw/s390-virtio.c
+++ b/hw/s390-virtio.c
@@ -263,6 +263,18 @@ static QEMUMachine s390_machine_v0_12 = {
.use_virtcon = 1,
.no_vga = 1,
.max_cpus = 255,
+.compat_props = (GlobalProperty[]) {
+{
+.driver = virtio-serial-s390,
+.property = max_nr_ports,
+.value= stringify(1),
+},{
+.driver = virtio-serial-s390,
+.property = vectors,
+.value= stringify(0),
+},
+{ /* end of list */ }
+ },
};
static void s390_machine_init(void)
--
1.6.2.5
[Qemu-devel] [PATCH 1/3] s390-virtio: Fix compile error for virtio-block init
Commit 428c149b0be790b440e1cbee185b152cdb22feec modified the argument
that virtio_blk_init takes. Update the s390 bus code that calls this
function.
Signed-off-by: Amit Shah amit.s...@redhat.com
CC: Christoph Hellwig h...@lst.de
---
hw/s390-virtio-bus.c |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/hw/s390-virtio-bus.c b/hw/s390-virtio-bus.c
index fa0a74f..9fc01e9 100644
--- a/hw/s390-virtio-bus.c
+++ b/hw/s390-virtio-bus.c
@@ -123,7 +123,7 @@ static int s390_virtio_blk_init(VirtIOS390Device *dev)
{
VirtIODevice *vdev;
-vdev = virtio_blk_init((DeviceState *)dev, dev-block.dinfo);
+vdev = virtio_blk_init((DeviceState *)dev, dev-block);
if (!vdev) {
return -1;
}
--
1.6.2.5
[Qemu-devel] [PATCH 1/2] ppc440_bamboo: Add 0.12 and 0.13 machine types for backward compat
Add a 0.12 machine type for compatibility with older versions. Mark the
default one as 0.13.
Signed-off-by: Amit Shah amit.s...@redhat.com
---
hw/ppc440_bamboo.c | 11 ++-
1 files changed, 10 insertions(+), 1 deletions(-)
diff --git a/hw/ppc440_bamboo.c b/hw/ppc440_bamboo.c
index 1ab9872..aa9f594 100644
--- a/hw/ppc440_bamboo.c
+++ b/hw/ppc440_bamboo.c
@@ -172,7 +172,15 @@ static void bamboo_init(ram_addr_t ram_size,
}
static QEMUMachine bamboo_machine = {
-.name = bamboo,
+.name = bamboo-0.13,
+.alias = bamboo,
+.desc = bamboo,
+.init = bamboo_init,
+.is_default = 1,
+};
+
+static QEMUMachine bamboo_machine_v0_12 = {
+.name = bamboo-0.12,
.desc = bamboo,
.init = bamboo_init,
};
@@ -180,6 +188,7 @@ static QEMUMachine bamboo_machine = {
static void bamboo_machine_init(void)
{
qemu_register_machine(bamboo_machine);
+qemu_register_machine(bamboo_machine_v0_12);
}
machine_init(bamboo_machine_init);
--
1.6.2.5
[Qemu-devel] [PATCH 2/2] ppc440_bamboo: Disable new virtio-serial features for 0.12 machine type
Disable the MULTIPORT feature and MSI vectors for the 0.12 machine
types; those features are added only for 0.13 onwards.
Signed-off-by: Amit Shah amit.s...@redhat.com
---
hw/ppc440_bamboo.c | 12
1 files changed, 12 insertions(+), 0 deletions(-)
diff --git a/hw/ppc440_bamboo.c b/hw/ppc440_bamboo.c
index aa9f594..db96f14 100644
--- a/hw/ppc440_bamboo.c
+++ b/hw/ppc440_bamboo.c
@@ -183,6 +183,18 @@ static QEMUMachine bamboo_machine_v0_12 = {
.name = bamboo-0.12,
.desc = bamboo,
.init = bamboo_init,
+.compat_props = (GlobalProperty[]) {
+{
+.driver = virtio-serial-pci,
+.property = max_nr_ports,
+.value= stringify(1),
+},{
+.driver = virtio-serial-pci,
+.property = vectors,
+.value= stringify(0),
+},
+{ /* end of list */ }
+},
};
static void bamboo_machine_init(void)
--
1.6.2.5
[Qemu-devel] [PATCH] Fix 'make install' from non-srcdir build
Commit b5ec5ce0 broke 'make install' from non source-dir build. Fix. Signed-off-by: Amit Shah amit.s...@redhat.com --- Makefile |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/Makefile b/Makefile index 296f500..14c1930 100644 --- a/Makefile +++ b/Makefile @@ -207,7 +207,7 @@ endif install-sysconfig: $(INSTALL_DIR) $(sysconfdir)/qemu - $(INSTALL_DATA) sysconfigs/target/target-x86_64.conf $(sysconfdir)/qemu + $(INSTALL_DATA) $(SRC_PATH)/sysconfigs/target/target-x86_64.conf $(sysconfdir)/qemu install: all $(if $(BUILD_DOCS),install-doc) install-sysconfig $(INSTALL_DIR) $(DESTDIR)$(bindir) -- 1.6.2.5
Re: [Qemu-devel] [PATCHv4 07/12] virtio: move typedef to qemu-common
On (Wed) Mar 03 2010 [19:16:28], Michael S. Tsirkin wrote: make it possible to use type without header include Why? Signed-off-by: Michael S. Tsirkin m...@redhat.com --- hw/virtio.h |1 - qemu-common.h |1 + 2 files changed, 1 insertions(+), 1 deletions(-) diff --git a/hw/virtio.h b/hw/virtio.h index 58b06bf..6f2fab0 100644 --- a/hw/virtio.h +++ b/hw/virtio.h @@ -69,7 +69,6 @@ static inline target_phys_addr_t vring_align(target_phys_addr_t addr, } typedef struct VirtQueue VirtQueue; -typedef struct VirtIODevice VirtIODevice; #define VIRTQUEUE_MAX_SIZE 1024 diff --git a/qemu-common.h b/qemu-common.h index f12a8f5..90ca3b8 100644 --- a/qemu-common.h +++ b/qemu-common.h @@ -228,6 +228,7 @@ typedef struct I2SCodec I2SCodec; typedef struct DeviceState DeviceState; typedef struct SSIBus SSIBus; typedef struct EventNotifier EventNotifier; +typedef struct VirtIODevice VirtIODevice; typedef uint64_t pcibus_t; Amit
Re: [Qemu-devel] [PATCHv4 07/12] virtio: move typedef to qemu-common
On (Thu) Mar 04 2010 [14:19:58], Michael S. Tsirkin wrote: On Thu, Mar 04, 2010 at 05:50:19PM +0530, Amit Shah wrote: On (Wed) Mar 03 2010 [19:16:28], Michael S. Tsirkin wrote: make it possible to use type without header include Why? So that vhost.h does not need to include virtio.h But what's the benefit in doing that? Either way, a better commit msg would help. Amit
[Qemu-devel] Re: [PATCHv4 04/12] virtio: add notifier support
On (Wed) Mar 03 2010 [19:16:04], Michael S. Tsirkin wrote:
Add binding API to set host/guest notifiers.
Will be used by vhost.
I've already mentioned this on IRC, but I'll put it here too, for
tracking.
Signed-off-by: Michael S. Tsirkin m...@redhat.com
---
hw/virtio.c |5 -
hw/virtio.h |3 +++
2 files changed, 7 insertions(+), 1 deletions(-)
diff --git a/hw/virtio.c b/hw/virtio.c
index 7c020a3..1f5e7be 100644
--- a/hw/virtio.c
+++ b/hw/virtio.c
@@ -73,6 +73,7 @@ struct VirtQueue
int inuse;
uint16_t vector;
void (*handle_output)(VirtIODevice *vdev, VirtQueue *vq);
+VirtIODevice *vdev;
};
/* virt queue functions */
@@ -714,8 +715,10 @@ VirtIODevice *virtio_common_init(const char *name,
uint16_t device_id,
vdev-queue_sel = 0;
vdev-config_vector = VIRTIO_NO_VECTOR;
vdev-vq = qemu_mallocz(sizeof(VirtQueue) * VIRTIO_PCI_QUEUE_MAX);
-for(i = 0; i VIRTIO_PCI_QUEUE_MAX; i++)
+for(i = 0; i VIRTIO_PCI_QUEUE_MAX; i++) {
vdev-vq[i].vector = VIRTIO_NO_VECTOR;
+vdev-vq[i].vdev = vdev;
+}
These two hunks don't belong to this patchset (and are not part of the
commit description too..)
vdev-name = name;
vdev-config_len = config_size;
diff --git a/hw/virtio.h b/hw/virtio.h
index 3baa2a3..af87889 100644
--- a/hw/virtio.h
+++ b/hw/virtio.h
@@ -19,6 +19,7 @@
#include qdev.h
#include sysemu.h
#include block_int.h
+#include notifier.h
/* from Linux's linux/virtio_config.h */
@@ -89,6 +90,8 @@ typedef struct {
int (*load_config)(void * opaque, QEMUFile *f);
int (*load_queue)(void * opaque, int n, QEMUFile *f);
unsigned (*get_features)(void * opaque);
+int (*guest_notifier)(void * opaque, int n, bool assigned);
+int (*host_notifier)(void * opaque, int n, bool assigned);
} VirtIOBindings;
#define VIRTIO_PCI_QUEUE_MAX 64
And I think this whole patch could be merged with the next one.
Amit
[Qemu-devel] Re: [PATCHv4 05/12] virtio: add APIs for queue fields
On (Wed) Mar 03 2010 [19:16:09], Michael S. Tsirkin wrote:
vhost needs physical addresses for ring and other queue fields,
so add APIs for these.
Already discussed on IRC, but mentioning here so that it doesn't get
lost:
+target_phys_addr_t virtio_queue_get_desc(VirtIODevice *vdev, int n)
+{
+return vdev-vq[n].vring.desc;
+}
+
+target_phys_addr_t virtio_queue_get_avail(VirtIODevice *vdev, int n)
+{
+return vdev-vq[n].vring.avail;
+}
+
+target_phys_addr_t virtio_queue_get_used(VirtIODevice *vdev, int n)
+{
+return vdev-vq[n].vring.used;
+}
+
+target_phys_addr_t virtio_queue_get_ring(VirtIODevice *vdev, int n)
+{
+return vdev-vq[n].vring.desc;
+}
All these functions return the start address of these fields; any better
way to name them?
eg, just by looking at, eg, 'virtio_queue_get_used()', I'd expect that
the function returns the number of used buffers in the ring, not the
start address of the used buffers.
+target_phys_addr_t virtio_queue_get_desc_size(VirtIODevice *vdev, int n)
+{
+return sizeof(VRingDesc) * vdev-vq[n].vring.num;
+}
+
+target_phys_addr_t virtio_queue_get_avail_size(VirtIODevice *vdev, int n)
+{
+return offsetof(VRingAvail, ring) +
+sizeof(u_int64_t) * vdev-vq[n].vring.num;
+}
+
+target_phys_addr_t virtio_queue_get_used_size(VirtIODevice *vdev, int n)
+{
+return offsetof(VRingUsed, ring) +
+sizeof(VRingUsedElem) * vdev-vq[n].vring.num;
+}
+
+
Extra newline
+target_phys_addr_t virtio_queue_get_ring_size(VirtIODevice *vdev, int n)
+{
+return vdev-vq[n].vring.used - vdev-vq[n].vring.desc +
+ virtio_queue_get_used_size(vdev, n);
+}
+
+uint16_t virtio_queue_last_avail_idx(VirtIODevice *vdev, int n)
+{
+return vdev-vq[n].last_avail_idx;
+}
+
+void virtio_queue_set_last_avail_idx(VirtIODevice *vdev, int n, uint16_t idx)
+{
+vdev-vq[n].last_avail_idx = idx;
+}
virtio_queue_last_avail_idx() does make sense, but since you have a
'set_last_avail_idx', better name the previous one 'get_..'?
+VirtQueue *virtio_queue(VirtIODevice *vdev, int n)
+{
+return vdev-vq + n;
+}
This really doesn't mean anything; I suggest virtio_queue_get_vq().
+EventNotifier *virtio_queue_guest_notifier(VirtQueue *vq)
+{
+return vq-guest_notifier;
+}
+EventNotifier *virtio_queue_host_notifier(VirtQueue *vq)
+{
+return vq-host_notifier;
+}
Why drop the 'get_' for these functions?
virtio_queue_get_guest_notifier()
and
virtio_queue_get_host_notifier()
might be better.
Amit
[Qemu-devel] Re: [PATCHv4 09/12] vhost: vhost net support
On (Wed) Mar 03 2010 [19:16:35], Michael S. Tsirkin wrote:
+static int vhost_virtqueue_init(struct vhost_dev *dev,
+struct VirtIODevice *vdev,
+struct vhost_virtqueue *vq,
+unsigned idx)
+{
+target_phys_addr_t s, l, a;
+int r;
+struct vhost_vring_file file = {
+.index = idx,
+};
+struct vhost_vring_state state = {
+.index = idx,
+};
+struct VirtQueue *q = virtio_queue(vdev, idx);
Why depart from using 'vq' for VirtQueue? Why not use 'hvq' for
vhost_virtqueue? That'll make reading through this code easier... Also,
'hvdev' for vhost_dev will be apt as well.
+vq-num = state.num = virtio_queue_get_num(vdev, idx);
I think this should be named 'virtio_queue_get_vq_num' for clarity.
+r = ioctl(dev-control, VHOST_SET_VRING_NUM, state);
+if (r) {
+return -errno;
+}
+
+state.num = virtio_queue_last_avail_idx(vdev, idx);
+r = ioctl(dev-control, VHOST_SET_VRING_BASE, state);
+if (r) {
+return -errno;
+}
+
+s = l = virtio_queue_get_desc_size(vdev, idx);
+a = virtio_queue_get_desc(vdev, idx);
+vq-desc = cpu_physical_memory_map(a, l, 0);
+if (!vq-desc || l != s) {
+r = -ENOMEM;
+goto fail_alloc_desc;
+}
+s = l = virtio_queue_get_avail_size(vdev, idx);
+a = virtio_queue_get_avail(vdev, idx);
+vq-avail = cpu_physical_memory_map(a, l, 0);
+if (!vq-avail || l != s) {
+r = -ENOMEM;
+goto fail_alloc_avail;
+}
+vq-used_size = s = l = virtio_queue_get_used_size(vdev, idx);
+vq-used_phys = a = virtio_queue_get_used(vdev, idx);
+vq-used = cpu_physical_memory_map(a, l, 1);
+if (!vq-used || l != s) {
+r = -ENOMEM;
+goto fail_alloc_used;
+}
+
+vq-ring_size = s = l = virtio_queue_get_ring_size(vdev, idx);
+vq-ring_phys = a = virtio_queue_get_ring(vdev, idx);
+vq-ring = cpu_physical_memory_map(a, l, 1);
+if (!vq-ring || l != s) {
+r = -ENOMEM;
+goto fail_alloc_ring;
+}
+
+r = vhost_virtqueue_set_addr(dev, vq, idx, dev-log_enabled);
+if (r 0) {
+r = -errno;
+goto fail_alloc;
+}
+if (!vdev-binding-guest_notifier || !vdev-binding-host_notifier) {
+fprintf(stderr, binding does not support irqfd/queuefd\n);
+r = -ENOSYS;
+goto fail_alloc;
+}
This could be checked much earlier on in the function; so that we avoid
doing all that stuff above and the cleanup.
Amit
[Qemu-devel] Re: [PATCHv4 05/12] virtio: add APIs for queue fields
On (Sat) Mar 06 2010 [21:07:57], Michael S. Tsirkin wrote:
On Fri, Mar 05, 2010 at 06:38:35PM +0530, Amit Shah wrote:
On (Wed) Mar 03 2010 [19:16:09], Michael S. Tsirkin wrote:
+
+EventNotifier *virtio_queue_guest_notifier(VirtQueue *vq)
+{
+return vq-guest_notifier;
+}
+EventNotifier *virtio_queue_host_notifier(VirtQueue *vq)
+{
+return vq-host_notifier;
+}
Where are these host_notifier and guest_notifier fields set? Am I
completely missing it?
What do you mean by set?
You get a pointer, you can e.g. init it.
I mean where is vq-host_notifier initialised?
Amit
[Qemu-devel] Re: [PATCHv4 09/12] vhost: vhost net support
On (Sat) Mar 06 2010 [21:06:35], Michael S. Tsirkin wrote:
+r = vhost_virtqueue_set_addr(dev, vq, idx, dev-log_enabled);
+if (r 0) {
+r = -errno;
+goto fail_alloc;
+}
+if (!vdev-binding-guest_notifier || !vdev-binding-host_notifier)
{
+fprintf(stderr, binding does not support irqfd/queuefd\n);
+r = -ENOSYS;
+goto fail_alloc;
+}
This could be checked much earlier on in the function; so that we avoid
doing all that stuff above and the cleanup.
Whatever order we put checks in, we'll have to undo stuff
done beforehand on error.
Not if you do this check before any ioctls or allocations.
!vdev-binding-guest_notifier is not dependent on anything you do above
it in this function, so just checking for this first thing in the
function will not need any cleanup.
Amit
Re: [Qemu-devel] Latest CVS build of qemu-system-ppc not boot debian_lenny_powerpc_small.qcow
On (Mon) Mar 08 2010 [12:38:37], Aurelien Jarno wrote: On Sun, Mar 07, 2010 at 11:07:40AM +0800, tielian wrote: qemu-system-ppc -hda debian_lenny_powerpc_small.qcow qemu: fatal: Trying to execute code outside RAM or ROM at 0x NIP LR CTR XER MSR HID0 0300 HF idx 0 Segmentation fault It has been broken by the following patch: commit 977b6b91cee1132f8c7b12d22f4b273091598e44 Author: Amit Shah amit.s...@redhat.com Date: Thu Feb 25 19:26:11 2010 +0530 ppc440_bamboo: Add 0.12 and 0.13 machine types for backward compat Add a 0.12 machine type for compatibility with older versions. Mark the default one as 0.13. Signed-off-by: Amit Shah amit.s...@redhat.com Signed-off-by: Aurelien Jarno aurel...@aurel32.net This patch change the default PPC machine to bamboo, while it was previously g3beige. I haven't noticed the problem, as my test script sets the machine with the -M argument. BTW, as in the s390 case, does this patch make sense for powerpc? If no one uses such a thing, those two patches should be reverted.. Amit
[Qemu-devel] Re: virtio-serial NULL deference
On (Tue) Mar 09 2010 [14:15:45], Juan Quintela wrote: Hi Amit Hey Juan, Checking migration, I just found this problem: I don't know what to put there. a return -EINVAL or continue? Looking more at the code, I am not sure what checks: a- that bus-max_nr_ports is the same in both sides (or at least bigger on migration destination) Yes, we should check for this. b- We sent the value of config.nr_ports, but ... we assign it back on destination, instead of checking that they are the same. This is done to accomodate for hot-plug/unplug. nr_ports will go up / down after those operations. (Current implementation only increases this value, on hotplug operations. On hot-unplug, this value is not decremented.) c- port-id is taken from nr_ports again, and nothing checks that ports appear in the same order in source and destination. Actually, this has me thinking about how would this work: - start vm with 3 ports - hotplug 2 more ports - migrate Would the destination have 5 ports, or would it have 3? I thought qdev would take care of this scenario (hotplug). I don't think I've tested this, so I'll do this soon, but in case anyone knows the answer, please let me know. [snipped patch that's necessary in case qdev doesn't handle this kind of hotplug] Amit -- http://log.amitshah.net/
[Qemu-devel] Re: [patch 2/2] virtio-serial-bus: wake up iothread upon guest read notification
On (Thu) Mar 11 2010 [23:45:51], Marcelo Tosatti wrote:
Wake up iothread when buffers are consumed.
Signed-off-by: Marcelo Tosatti mtosa...@redhat.com
Index: qemu-ioworker/hw/virtio-serial-bus.c
===
--- qemu-ioworker.orig/hw/virtio-serial-bus.c
+++ qemu-ioworker/hw/virtio-serial-bus.c
@@ -331,6 +331,7 @@ static void handle_output(VirtIODevice *
static void handle_input(VirtIODevice *vdev, VirtQueue *vq)
{
+qemu_notify_event(main_io_worker);
}
ACK, the host lets us know buffers are consumed and new buffers have
been added to the pool so that we can start sending more data.
Before this patch my tests took 16m18s to run.
After this patch my tests take 1m17s to run.
Both tests done with just one buffer made available in the virtio-queues.
Amit
[Qemu-devel] Re: [PATCHv5 04/11] virtio: notifier support + APIs for queue fields
On (Tue) Mar 16 2010 [19:10:58], Michael S. Tsirkin wrote:
diff --git a/hw/virtio.c b/hw/virtio.c
index 7c020a3..f54129f 100644
--- a/hw/virtio.c
+++ b/hw/virtio.c
@@ -73,6 +73,9 @@ struct VirtQueue
int inuse;
uint16_t vector;
void (*handle_output)(VirtIODevice *vdev, VirtQueue *vq);
+VirtIODevice *vdev;
+EventNotifier guest_notifier;
+EventNotifier host_notifier;
Another thing that terribly confused me was this: in the vq struct you
have guest_notifier and host_notifier of type EventNotifier and in the
virtio binding structs you have guest_notifier and host_notifier which
are function callbacks.
Also, these vq-{guest_|host_}notifier assignments weren't easily found
using grep because they're assigned by assigning a pointer value and
initialising that pointer. Thanks to Juan for finding that for me :-)
Amit
--
http://log.amitshah.net/
[Qemu-devel] Re: virtio-serial NULL deference
On (Tue) Mar 09 2010 [20:59:58], Amit Shah wrote: On (Tue) Mar 09 2010 [14:15:45], Juan Quintela wrote: Hi Amit Hey Juan, Checking migration, I just found this problem: I don't know what to put there. a return -EINVAL or continue? Looking more at the code, I am not sure what checks: a- that bus-max_nr_ports is the same in both sides (or at least bigger on migration destination) Yes, we should check for this. I've added this check in my local tree. b- We sent the value of config.nr_ports, but ... we assign it back on destination, instead of checking that they are the same. This is done to accomodate for hot-plug/unplug. nr_ports will go up / down after those operations. (Current implementation only increases this value, on hotplug operations. On hot-unplug, this value is not decremented.) For now I've added a check that tests nr_ports == s-config.nr_ports. If that's not true, then return with -EINVAL. These two were small changes, no problem. c- port-id is taken from nr_ports again, and nothing checks that ports appear in the same order in source and destination. Actually, this has me thinking about how would this work: - start vm with 3 ports - hotplug 2 more ports - migrate Would the destination have 5 ports, or would it have 3? I thought qdev would take care of this scenario (hotplug). I don't think I've tested this, so I'll do this soon, but in case anyone knows the answer, please let me know. I spoke with Dan and he confirmed libvirt starts qemu instances on the destination computer with the appropriate devices, taking into consideration the hotplug/unplug status. However, the only problem here is virtio-serial ports are allocated an 'id', ie., a port number, and this is auto-assigned when a new port is found by adding 1 to the previous id. To make this whole thing independent of the order in which ports are added / removed, we'll have to accept the port id on the command line. This also means that we'll have to let the kernel know of the id because the control messages that the kernel and qemu exchange contain the port id. So basically some design changes will have to be incorporated both, in the kernel as well as in qemu to accomodate this. If this sounds right, I'll get to this right away. If there's an easier solution, please let me know. Amit
[Qemu-devel] [PATCH 1/9] virtio-serial-bus: save/load: Ensure target has enough ports
The target could be started with max_nr_ports for a virtio-serial device
lesser than what was available on the source machine. Fail the migration
in such a case.
Signed-off-by: Amit Shah amit.s...@redhat.com
Reported-by: Juan Quintela quint...@redhat.com
---
hw/virtio-serial-bus.c | 10 +-
1 files changed, 9 insertions(+), 1 deletions(-)
diff --git a/hw/virtio-serial-bus.c b/hw/virtio-serial-bus.c
index 17c1ec1..36985a1 100644
--- a/hw/virtio-serial-bus.c
+++ b/hw/virtio-serial-bus.c
@@ -374,6 +374,8 @@ static void virtio_serial_save(QEMUFile *f, void *opaque)
/* Items in struct VirtIOSerial */
+qemu_put_be32s(f, s-bus-max_nr_ports);
+
/* Do this because we might have hot-unplugged some ports */
nr_active_ports = 0;
QTAILQ_FOREACH(port, s-ports, next)
@@ -399,7 +401,7 @@ static int virtio_serial_load(QEMUFile *f, void *opaque,
int version_id)
{
VirtIOSerial *s = opaque;
VirtIOSerialPort *port;
-uint32_t nr_active_ports;
+uint32_t max_nr_ports, nr_active_ports;
unsigned int i;
if (version_id 2) {
@@ -420,6 +422,12 @@ static int virtio_serial_load(QEMUFile *f, void *opaque,
int version_id)
/* Items in struct VirtIOSerial */
+qemu_get_be32s(f, max_nr_ports);
+if (max_nr_ports s-bus-max_nr_ports) {
+/* Source could have more ports than us. Fail migration. */
+return -EINVAL;
+}
+
qemu_get_be32s(f, nr_active_ports);
/* Items in struct VirtIOSerialPort */
--
1.6.2.5
[Qemu-devel] [PATCH 2/9] virtio-serial-bus: save/load: Ensure nr_ports on src and dest are same.
The number of ports on the source as well as the destination machines
should match. If they don't, it means some ports that got hotplugged on
the source aren't instantiated on the destination. Or that ports that
were hot-unplugged on the source are created on the destination.
Signed-off-by: Amit Shah amit.s...@redhat.com
Reported-by: Juan Quintela quint...@redhat.com
---
hw/virtio-serial-bus.c | 18 --
1 files changed, 16 insertions(+), 2 deletions(-)
diff --git a/hw/virtio-serial-bus.c b/hw/virtio-serial-bus.c
index 36985a1..f43d1fc 100644
--- a/hw/virtio-serial-bus.c
+++ b/hw/virtio-serial-bus.c
@@ -401,7 +401,7 @@ static int virtio_serial_load(QEMUFile *f, void *opaque,
int version_id)
{
VirtIOSerial *s = opaque;
VirtIOSerialPort *port;
-uint32_t max_nr_ports, nr_active_ports;
+uint32_t max_nr_ports, nr_active_ports, nr_ports;
unsigned int i;
if (version_id 2) {
@@ -418,7 +418,21 @@ static int virtio_serial_load(QEMUFile *f, void *opaque,
int version_id)
/* The config space */
qemu_get_be16s(f, s-config.cols);
qemu_get_be16s(f, s-config.rows);
-s-config.nr_ports = qemu_get_be32(f);
+nr_ports = qemu_get_be32(f);
+
+if (nr_ports != s-config.nr_ports) {
+/*
+ * Source hot-plugged/unplugged ports and we don't have all of
+ * them here.
+ *
+ * Note: This condition cannot check for all hotplug/unplug
+ * events: eg, if one port was hot-plugged and one was
+ * unplugged, the nr_ports remains the same but the port id's
+ * would have changed and we won't catch it here. A later
+ * check for !find_port_by_id() will confirm if this happened.
+ */
+return -EINVAL;
+}
/* Items in struct VirtIOSerial */
--
1.6.2.5
[Qemu-devel] [PATCH 0/9] virtio-serial fixes, ABI updates
Hello, This series fixes a few issues pointed out by Avi and Juan. Avi pointed out we should do full scatter/gather processing of guest data even if current (well-behaved) guests don't send multiple iovs per element. Juan pointed out a few migration-related bugs. In handling the migration fixes, I noticed hot-plug/unplug isn't handled perfectly for the migration case: ports are enumerated and the port numbering has to be consistent with the guest's numbering. If there's a mismatch, control messages meant for one port could be interpreted for another. To solve this issue, I go back to maintaining a bitmap in the config space for active ports. Hot-plug and unplug can be added easily via the config space as a result. The kernel driver has to be changed as well so that the changes are in sync with the changes here. I've tested these patches on my test suite that tests for correctness and also hot-plug/unplug cases and fixes presented here. Amit Shah (9): virtio-serial-bus: save/load: Ensure target has enough ports virtio-serial-bus: save/load: Ensure nr_ports on src and dest are same. virtio-serial: save/load: Ensure we have hot-plugged ports instantiated virtio-serial: Handle scatter-gather buffers for control messages virtio-serial: Handle scatter/gather input from the guest virtio-serial: Remove redundant check for 0-sized write request virtio-serial: Update copyright year to 2010 virtio-serial-bus: Use a bitmap in virtio config space for active ports virtio-serial-bus: Let the guest know of host connection changes after migration hw/virtio-console.c|4 +- hw/virtio-serial-bus.c | 205 hw/virtio-serial.h |8 +- 3 files changed, 161 insertions(+), 56 deletions(-)
[Qemu-devel] [PATCH 3/9] virtio-serial: save/load: Ensure we have hot-plugged ports instantiated
If some ports that were hot-plugged on the source are not available on
the destination, fail migration instead of trying to deref a NULL
pointer.
Signed-off-by: Amit Shah amit.s...@redhat.com
Reported-by: Juan Quintela quint...@redhat.com
---
hw/virtio-serial-bus.c |7 +++
1 files changed, 7 insertions(+), 0 deletions(-)
diff --git a/hw/virtio-serial-bus.c b/hw/virtio-serial-bus.c
index f43d1fc..830eb75 100644
--- a/hw/virtio-serial-bus.c
+++ b/hw/virtio-serial-bus.c
@@ -450,6 +450,13 @@ static int virtio_serial_load(QEMUFile *f, void *opaque,
int version_id)
id = qemu_get_be32(f);
port = find_port_by_id(s, id);
+if (!port) {
+/*
+ * The requested port was hot-plugged on the source but we
+ * don't have it
+ */
+return -EINVAL;
+}
port-guest_connected = qemu_get_byte(f);
}
--
1.6.2.5
[Qemu-devel] [PATCH 4/9] virtio-serial: Handle scatter-gather buffers for control messages
Current control messages are small enough to not be split into multiple
buffers but we could run into such a situation in the future or a
malicious guest could cause such a situation.
So handle the entire iov request for control messages.
Also ensure the size of the control request is = what we expect
otherwise we risk accessing memory that we don't own.
Signed-off-by: Amit Shah amit.s...@redhat.com
CC: Avi Kivity a...@redhat.com
Reported-by: Avi Kivity a...@redhat.com
---
hw/virtio-serial-bus.c | 43 ---
1 files changed, 40 insertions(+), 3 deletions(-)
diff --git a/hw/virtio-serial-bus.c b/hw/virtio-serial-bus.c
index 830eb75..d5887ab 100644
--- a/hw/virtio-serial-bus.c
+++ b/hw/virtio-serial-bus.c
@@ -200,7 +200,7 @@ size_t virtio_serial_guest_ready(VirtIOSerialPort *port)
}
/* Guest wants to notify us of some event */
-static void handle_control_message(VirtIOSerial *vser, void *buf)
+static void handle_control_message(VirtIOSerial *vser, void *buf, size_t len)
{
struct VirtIOSerialPort *port;
struct virtio_console_control cpkt, *gcpkt;
@@ -208,6 +208,10 @@ static void handle_control_message(VirtIOSerial *vser,
void *buf)
size_t buffer_len;
gcpkt = buf;
+if (len sizeof(cpkt)) {
+/* The guest sent an invalid control packet */
+return;
+}
port = find_port_by_id(vser, ldl_p(gcpkt-id));
if (!port)
return;
@@ -281,12 +285,45 @@ static void control_out(VirtIODevice *vdev, VirtQueue *vq)
{
VirtQueueElement elem;
VirtIOSerial *vser;
+uint8_t *buf;
+size_t len;
vser = DO_UPCAST(VirtIOSerial, vdev, vdev);
+len = 0;
+buf = NULL;
while (virtqueue_pop(vq, elem)) {
-handle_control_message(vser, elem.out_sg[0].iov_base);
-virtqueue_push(vq, elem, elem.out_sg[0].iov_len);
+unsigned int i;
+size_t cur_len, offset;
+
+cur_len = 0;
+for (i = 0; i elem.out_num; i++) {
+cur_len += elem.out_sg[i].iov_len;
+}
+/*
+ * Allocate a new buf only if we didn't have one previously or
+ * if the size of the buf differs
+ */
+if (cur_len != len) {
+if (len) {
+qemu_free(buf);
+}
+buf = qemu_malloc(cur_len);
+}
+
+offset = 0;
+for (i = 0; i elem.out_num; i++) {
+memcpy(buf + offset, elem.out_sg[i].iov_base,
+ elem.out_sg[i].iov_len);
+offset += elem.out_sg[i].iov_len;
+}
+len = cur_len;
+
+handle_control_message(vser, buf, len);
+virtqueue_push(vq, elem, len);
+}
+if (len) {
+qemu_free(buf);
}
virtio_notify(vdev, vq);
}
--
1.6.2.5
[Qemu-devel] [PATCH 6/9] virtio-serial: Remove redundant check for 0-sized write request
The check for a 0-sized write request to a guest port is not necessary;
the while loop below won't be executed in this case and all will be
fine.
Signed-off-by: Amit Shah amit.s...@redhat.com
---
hw/virtio-serial-bus.c |3 ---
1 files changed, 0 insertions(+), 3 deletions(-)
diff --git a/hw/virtio-serial-bus.c b/hw/virtio-serial-bus.c
index 19e8c59..2603dcd 100644
--- a/hw/virtio-serial-bus.c
+++ b/hw/virtio-serial-bus.c
@@ -83,9 +83,6 @@ static size_t write_to_port(VirtIOSerialPort *port,
if (!virtio_queue_ready(vq)) {
return 0;
}
-if (!size) {
-return 0;
-}
while (offset size) {
int i;
--
1.6.2.5
[Qemu-devel] [PATCH 7/9] virtio-serial: Update copyright year to 2010
Signed-off-by: Amit Shah amit.s...@redhat.com --- hw/virtio-console.c|2 +- hw/virtio-serial-bus.c |2 +- hw/virtio-serial.h |2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/hw/virtio-console.c b/hw/virtio-console.c index bd44ec6..e915491 100644 --- a/hw/virtio-console.c +++ b/hw/virtio-console.c @@ -1,7 +1,7 @@ /* * Virtio Console and Generic Serial Port Devices * - * Copyright Red Hat, Inc. 2009 + * Copyright Red Hat, Inc. 2009, 2010 * * Authors: * Amit Shah amit.s...@redhat.com diff --git a/hw/virtio-serial-bus.c b/hw/virtio-serial-bus.c index 2603dcd..b682b77 100644 --- a/hw/virtio-serial-bus.c +++ b/hw/virtio-serial-bus.c @@ -1,7 +1,7 @@ /* * A bus for connecting virtio serial and console ports * - * Copyright (C) 2009 Red Hat, Inc. + * Copyright (C) 2009, 2010 Red Hat, Inc. * * Author(s): * Amit Shah amit.s...@redhat.com diff --git a/hw/virtio-serial.h b/hw/virtio-serial.h index f297b00..632d31b 100644 --- a/hw/virtio-serial.h +++ b/hw/virtio-serial.h @@ -2,7 +2,7 @@ * Virtio Serial / Console Support * * Copyright IBM, Corp. 2008 - * Copyright Red Hat, Inc. 2009 + * Copyright Red Hat, Inc. 2009, 2010 * * Authors: * Christian Ehrhardt ehrha...@linux.vnet.ibm.com -- 1.6.2.5
[Qemu-devel] [PATCH 9/9] virtio-serial-bus: Let the guest know of host connection changes after migration
If the host connection to a port is closed on the destination machine
after migration, when the connection was open on the source, the host
has to be informed of that.
Similar for a host connection open on the destination.
Signed-off-by: Amit Shah amit.s...@redhat.com
---
hw/virtio-serial-bus.c | 11 +++
1 files changed, 11 insertions(+), 0 deletions(-)
diff --git a/hw/virtio-serial-bus.c b/hw/virtio-serial-bus.c
index 2242edf..f5ea37e 100644
--- a/hw/virtio-serial-bus.c
+++ b/hw/virtio-serial-bus.c
@@ -435,6 +435,7 @@ static void virtio_serial_save(QEMUFile *f, void *opaque)
QTAILQ_FOREACH(port, s-ports, next) {
qemu_put_be32s(f, port-id);
qemu_put_byte(f, port-guest_connected);
+qemu_put_byte(f, port-host_connected);
}
}
@@ -488,11 +489,21 @@ static int virtio_serial_load(QEMUFile *f, void *opaque,
int version_id)
/* Items in struct VirtIOSerialPort */
for (i = 0; i nr_active_ports; i++) {
uint32_t id;
+bool host_connected;
id = qemu_get_be32(f);
port = find_port_by_id(s, id);
port-guest_connected = qemu_get_byte(f);
+host_connected = qemu_get_byte(f);
+if (host_connected != port-host_connected) {
+/*
+ * We have to let the guest know of the host connection
+ * status change
+ */
+send_control_event(port, VIRTIO_CONSOLE_PORT_OPEN,
+ port-host_connected);
+}
}
return 0;
}
--
1.6.2.5
[Qemu-devel] [PATCH 8/9] virtio-serial-bus: Use a bitmap in virtio config space for active ports
Allow the port 'id's to be set by a user on the command line. This is
needed by management apps that will want a stable port numbering scheme
for hot-plug/unplug and migration.
Since the port numbers are shared with the guest (to identify ports in
control messages), the config space now maintains a bitmap for active
ports instead of a number indicating active ports.
This also means we can just update the config for port hot-plug and
hot-unplug.
Signed-off-by: Amit Shah amit.s...@redhat.com
---
hw/virtio-console.c|2 +
hw/virtio-serial-bus.c | 141 +++-
hw/virtio-serial.h |6 ++-
3 files changed, 86 insertions(+), 63 deletions(-)
diff --git a/hw/virtio-console.c b/hw/virtio-console.c
index e915491..6b8 100644
--- a/hw/virtio-console.c
+++ b/hw/virtio-console.c
@@ -99,6 +99,7 @@ static VirtIOSerialPortInfo virtconsole_info = {
.exit = virtconsole_exitfn,
.qdev.props = (Property[]) {
DEFINE_PROP_UINT8(is_console, VirtConsole, port.is_console, 1),
+DEFINE_PROP_UINT32(nr, VirtConsole, port.id, VIRTIO_CONSOLE_BAD_ID),
DEFINE_PROP_CHR(chardev, VirtConsole, chr),
DEFINE_PROP_STRING(name, VirtConsole, port.name),
DEFINE_PROP_END_OF_LIST(),
@@ -133,6 +134,7 @@ static VirtIOSerialPortInfo virtserialport_info = {
.init = virtserialport_initfn,
.exit = virtconsole_exitfn,
.qdev.props = (Property[]) {
+DEFINE_PROP_UINT32(nr, VirtConsole, port.id, VIRTIO_CONSOLE_BAD_ID),
DEFINE_PROP_CHR(chardev, VirtConsole, chr),
DEFINE_PROP_STRING(name, VirtConsole, port.name),
DEFINE_PROP_END_OF_LIST(),
diff --git a/hw/virtio-serial-bus.c b/hw/virtio-serial-bus.c
index b682b77..2242edf 100644
--- a/hw/virtio-serial-bus.c
+++ b/hw/virtio-serial-bus.c
@@ -48,6 +48,10 @@ static VirtIOSerialPort *find_port_by_id(VirtIOSerial *vser,
uint32_t id)
{
VirtIOSerialPort *port;
+if (id == VIRTIO_CONSOLE_BAD_ID) {
+return NULL;
+}
+
QTAILQ_FOREACH(port, vser-ports, next) {
if (port-id == id)
return port;
@@ -412,13 +416,13 @@ static void virtio_serial_save(QEMUFile *f, void *opaque)
/* The config space */
qemu_put_be16s(f, s-config.cols);
qemu_put_be16s(f, s-config.rows);
-qemu_put_be32s(f, s-config.nr_ports);
-/* Items in struct VirtIOSerial */
+qemu_put_be32s(f, s-config.max_nr_ports);
+qemu_put_buffer(f, (uint8_t *)s-config.ports_map,
+sizeof(uint32_t) * (s-config.max_nr_ports + 31) / 32);
-qemu_put_be32s(f, s-bus-max_nr_ports);
+/* Ports */
-/* Do this because we might have hot-unplugged some ports */
nr_active_ports = 0;
QTAILQ_FOREACH(port, s-ports, next)
nr_active_ports++;
@@ -429,11 +433,6 @@ static void virtio_serial_save(QEMUFile *f, void *opaque)
* Items in struct VirtIOSerialPort.
*/
QTAILQ_FOREACH(port, s-ports, next) {
-/*
- * We put the port number because we may not have an active
- * port at id 0 that's reserved for a console port, or in case
- * of ports that might have gotten unplugged
- */
qemu_put_be32s(f, port-id);
qemu_put_byte(f, port-guest_connected);
}
@@ -443,7 +442,8 @@ static int virtio_serial_load(QEMUFile *f, void *opaque,
int version_id)
{
VirtIOSerial *s = opaque;
VirtIOSerialPort *port;
-uint32_t max_nr_ports, nr_active_ports, nr_ports;
+size_t ports_map_size;
+uint32_t max_nr_ports, nr_active_ports, *ports_map;
unsigned int i;
if (version_id 2) {
@@ -460,29 +460,28 @@ static int virtio_serial_load(QEMUFile *f, void *opaque,
int version_id)
/* The config space */
qemu_get_be16s(f, s-config.cols);
qemu_get_be16s(f, s-config.rows);
-nr_ports = qemu_get_be32(f);
-if (nr_ports != s-config.nr_ports) {
-/*
- * Source hot-plugged/unplugged ports and we don't have all of
- * them here.
- *
- * Note: This condition cannot check for all hotplug/unplug
- * events: eg, if one port was hot-plugged and one was
- * unplugged, the nr_ports remains the same but the port id's
- * would have changed and we won't catch it here. A later
- * check for !find_port_by_id() will confirm if this happened.
- */
+qemu_get_be32s(f, max_nr_ports);
+if (max_nr_ports s-config.max_nr_ports) {
+/* Source could have had more ports than us. Fail migration. */
return -EINVAL;
}
-/* Items in struct VirtIOSerial */
+ports_map_size = sizeof(uint32_t) * (max_nr_ports + 31) / 32;
+ports_map = qemu_malloc(ports_map_size);
+qemu_get_buffer(f, (uint8_t *)ports_map, ports_map_size);
-qemu_get_be32s(f, max_nr_ports);
-if (max_nr_ports s-bus-max_nr_ports) {
-/* Source could have more ports than us. Fail migration. */
-return -EINVAL
[Qemu-devel] Re: [PATCH 0/9] virtio-serial fixes, ABI updates
On (Sun) Mar 21 2010 [15:47:53], Michael S. Tsirkin wrote: On Fri, Mar 19, 2010 at 05:28:37PM +0530, Amit Shah wrote: Hello, This series fixes a few issues pointed out by Avi and Juan. Avi pointed out we should do full scatter/gather processing of guest data even if current (well-behaved) guests don't send multiple iovs per element. Juan pointed out a few migration-related bugs. In handling the migration fixes, I noticed hot-plug/unplug isn't handled perfectly for the migration case: ports are enumerated and the port numbering has to be consistent with the guest's numbering. If there's a mismatch, control messages meant for one port could be interpreted for another. BTW, I think virtio serial migration code needs to be fixed to be backwards compatible with old qemu if multiport feature is off. This is already done; the savevm version number is bumped up. To solve this issue, I go back to maintaining a bitmap in the config space for active ports. Hot-plug and unplug can be added easily via the config space as a result. As I commented on the kernel driver, I'm not sure this is a good choice. I've replied to that comment; please let me know if it's a concern. Also, what do others think? Amit
Re: [Qemu-devel] Re: [PATCH 4/9] virtio-serial: Handle scatter-gather buffers for control messages
On (Sat) Mar 20 2010 [09:40:50], Avi Kivity wrote:
On 03/19/2010 01:58 PM, Amit Shah wrote:
+
+offset = 0;
+for (i = 0; i elem.out_num; i++) {
+memcpy(buf + offset, elem.out_sg[i].iov_base,
+ elem.out_sg[i].iov_len);
+offset += elem.out_sg[i].iov_len;
+}
+len = cur_len;
+
+handle_control_message(vser, buf, len);
+virtqueue_push(vq,elem, len);
+}
+if (len) {
+qemu_free(buf);
}
virtio_notify(vdev, vq);
}
Isn't there some virtio function to linearize requests?
I don't see one.
Amit
[Qemu-devel] Re: hi, may I ask some help on the paravirtualization of KVM?
Hello, [any reason you dropped the CC list? CC'ing qemu-devel, where this is relevant.] On (Tue) Mar 23 2010 [21:46:28], Liang YANG wrote: I check the 'lspci -v' result, only find RTL-8139 realtek ethernet. I think the option model=virtio does't make effect. Then something is wrong. I get a virtio ethernet device using model=virtio. Which qemu version are you using? Simultaneously, I have the a look at the code. The function pc_init1() seem not change the network model as the option assigned. Do you ever meet this case? I've not seen this. On Tue, Mar 23, 2010 at 9:08 PM, Amit Shah amit.s...@redhat.com wrote: On (Sun) Mar 21 2010 [20:18:53], Liang YANG wrote: I want to set up the virtio-net for the GuestOS on KVM. Following is my steps: 1.Compile the kvm-88 and make, make install. 2.Compile the GuestOS(redhat) with kernel version 2.6.27.45(with virtio support). The required option are all selected. Â Â Â Â Â o CONFIG_VIRTIO_PCI=y (Virtualization - PCI driver for virtio devices) Â Â Â Â Â o CONFIG_VIRTIO_BALLOON=y (Virtualization - Virtio balloon driver) Â Â Â Â Â o CONFIG_VIRTIO_BLK=y (Device Drivers - Block - Virtio block driver) Â Â Â Â Â o CONFIG_VIRTIO_NET=y (Device Drivers - Network device support - Virtio network driver) Â Â Â Â Â o CONFIG_VIRTIO=y (automatically selected) Â Â Â Â Â o CONFIG_VIRTIO_RING=y (automatically selected) 3.Then start up the GuestOS by such command: Â Â Â Â Â x86_64-softmmu/qemu-system-x86_64 Â -m 1024 /root/redhat.img -net nic,model=virtio -net tap,script=/etc/kvm/qemu-ifup 4.Result is this: Â Â Â Â Â * The Guest OS start up. Â Â Â Â Â * But the network not, no eth-X device found. Â Â Â Â Â * lsmod | grep virtio get none module about virtio Since you selected the virtio options as 'y', they are directly compiled into the kernel and hence you won't see the modules. Check your 'lspci -v' output. You should see the virtio device there. You can also try some usual things like 'ifconfig eth0 up', etc., to get the interface. Â Â Â Â Â Â Â Â Amit -- http://log.amitshah.net/ -- BestRegards. YangLiang _ Department of Computer Science . School of Electronics Engineering Computer Science . _ Amit -- http://log.amitshah.net/
[Qemu-devel] [PATCH 0/9] v2: Fixes, new way of discovering ports
Hello, These patches rework the way ports are announced to the guests. A control message is used to let the guest know a new port is added. Initial port discovery and port hot-plug work via this way now. This was done to have the host and guest port numbering in sync to avoid surprises after several hotplug/unplug operations and migrations. The ability to assign a particular port number to ports is also added so that management software can control the placement of ports. The other patches to handle scatter/gather for guest data and migration fixes remain the same. Please review. Amit Shah (9): virtio-serial-bus: save/load: Ensure target has enough ports virtio-serial-bus: save/load: Ensure nr_ports on src and dest are same. virtio-serial: Remove redundant check for 0-sized write request virtio-serial: Update copyright year to 2010 virtio-serial: save/load: Ensure we have hot-plugged ports instantiated virtio-serial-bus: Use control messages to notify guest of new ports virtio-serial-bus: Let the guest know of host connection changes after migration virtio-serial: Handle scatter-gather buffers for control messages virtio-serial: Handle scatter/gather input from the guest hw/virtio-console.c|4 +- hw/virtio-serial-bus.c | 245 +--- hw/virtio-serial.h | 19 ++-- 3 files changed, 205 insertions(+), 63 deletions(-)
[Qemu-devel] [PATCH 1/9] virtio-serial-bus: save/load: Ensure target has enough ports
The target could be started with max_nr_ports for a virtio-serial device
lesser than what was available on the source machine. Fail the migration
in such a case.
Signed-off-by: Amit Shah amit.s...@redhat.com
Reported-by: Juan Quintela quint...@redhat.com
---
hw/virtio-serial-bus.c | 10 +-
1 files changed, 9 insertions(+), 1 deletions(-)
diff --git a/hw/virtio-serial-bus.c b/hw/virtio-serial-bus.c
index 17c1ec1..36985a1 100644
--- a/hw/virtio-serial-bus.c
+++ b/hw/virtio-serial-bus.c
@@ -374,6 +374,8 @@ static void virtio_serial_save(QEMUFile *f, void *opaque)
/* Items in struct VirtIOSerial */
+qemu_put_be32s(f, s-bus-max_nr_ports);
+
/* Do this because we might have hot-unplugged some ports */
nr_active_ports = 0;
QTAILQ_FOREACH(port, s-ports, next)
@@ -399,7 +401,7 @@ static int virtio_serial_load(QEMUFile *f, void *opaque,
int version_id)
{
VirtIOSerial *s = opaque;
VirtIOSerialPort *port;
-uint32_t nr_active_ports;
+uint32_t max_nr_ports, nr_active_ports;
unsigned int i;
if (version_id 2) {
@@ -420,6 +422,12 @@ static int virtio_serial_load(QEMUFile *f, void *opaque,
int version_id)
/* Items in struct VirtIOSerial */
+qemu_get_be32s(f, max_nr_ports);
+if (max_nr_ports s-bus-max_nr_ports) {
+/* Source could have more ports than us. Fail migration. */
+return -EINVAL;
+}
+
qemu_get_be32s(f, nr_active_ports);
/* Items in struct VirtIOSerialPort */
--
1.6.2.5
[Qemu-devel] [PATCH 2/9] virtio-serial-bus: save/load: Ensure nr_ports on src and dest are same.
The number of ports on the source as well as the destination machines
should match. If they don't, it means some ports that got hotplugged on
the source aren't instantiated on the destination. Or that ports that
were hot-unplugged on the source are created on the destination.
Signed-off-by: Amit Shah amit.s...@redhat.com
Reported-by: Juan Quintela quint...@redhat.com
---
hw/virtio-serial-bus.c | 18 --
1 files changed, 16 insertions(+), 2 deletions(-)
diff --git a/hw/virtio-serial-bus.c b/hw/virtio-serial-bus.c
index 36985a1..f43d1fc 100644
--- a/hw/virtio-serial-bus.c
+++ b/hw/virtio-serial-bus.c
@@ -401,7 +401,7 @@ static int virtio_serial_load(QEMUFile *f, void *opaque,
int version_id)
{
VirtIOSerial *s = opaque;
VirtIOSerialPort *port;
-uint32_t max_nr_ports, nr_active_ports;
+uint32_t max_nr_ports, nr_active_ports, nr_ports;
unsigned int i;
if (version_id 2) {
@@ -418,7 +418,21 @@ static int virtio_serial_load(QEMUFile *f, void *opaque,
int version_id)
/* The config space */
qemu_get_be16s(f, s-config.cols);
qemu_get_be16s(f, s-config.rows);
-s-config.nr_ports = qemu_get_be32(f);
+nr_ports = qemu_get_be32(f);
+
+if (nr_ports != s-config.nr_ports) {
+/*
+ * Source hot-plugged/unplugged ports and we don't have all of
+ * them here.
+ *
+ * Note: This condition cannot check for all hotplug/unplug
+ * events: eg, if one port was hot-plugged and one was
+ * unplugged, the nr_ports remains the same but the port id's
+ * would have changed and we won't catch it here. A later
+ * check for !find_port_by_id() will confirm if this happened.
+ */
+return -EINVAL;
+}
/* Items in struct VirtIOSerial */
--
1.6.2.5
[Qemu-devel] [PATCH 3/9] virtio-serial: Remove redundant check for 0-sized write request
The check for a 0-sized write request to a guest port is not necessary;
the while loop below won't be executed in this case and all will be
fine.
Signed-off-by: Amit Shah amit.s...@redhat.com
---
hw/virtio-serial-bus.c |3 ---
1 files changed, 0 insertions(+), 3 deletions(-)
diff --git a/hw/virtio-serial-bus.c b/hw/virtio-serial-bus.c
index f43d1fc..7e9df96 100644
--- a/hw/virtio-serial-bus.c
+++ b/hw/virtio-serial-bus.c
@@ -83,9 +83,6 @@ static size_t write_to_port(VirtIOSerialPort *port,
if (!virtio_queue_ready(vq)) {
return 0;
}
-if (!size) {
-return 0;
-}
while (offset size) {
int i;
--
1.6.2.5
[Qemu-devel] [PATCH 4/9] virtio-serial: Update copyright year to 2010
Signed-off-by: Amit Shah amit.s...@redhat.com --- hw/virtio-console.c|2 +- hw/virtio-serial-bus.c |2 +- hw/virtio-serial.h |2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/hw/virtio-console.c b/hw/virtio-console.c index bd44ec6..e915491 100644 --- a/hw/virtio-console.c +++ b/hw/virtio-console.c @@ -1,7 +1,7 @@ /* * Virtio Console and Generic Serial Port Devices * - * Copyright Red Hat, Inc. 2009 + * Copyright Red Hat, Inc. 2009, 2010 * * Authors: * Amit Shah amit.s...@redhat.com diff --git a/hw/virtio-serial-bus.c b/hw/virtio-serial-bus.c index 7e9df96..bf7899c 100644 --- a/hw/virtio-serial-bus.c +++ b/hw/virtio-serial-bus.c @@ -1,7 +1,7 @@ /* * A bus for connecting virtio serial and console ports * - * Copyright (C) 2009 Red Hat, Inc. + * Copyright (C) 2009, 2010 Red Hat, Inc. * * Author(s): * Amit Shah amit.s...@redhat.com diff --git a/hw/virtio-serial.h b/hw/virtio-serial.h index f297b00..632d31b 100644 --- a/hw/virtio-serial.h +++ b/hw/virtio-serial.h @@ -2,7 +2,7 @@ * Virtio Serial / Console Support * * Copyright IBM, Corp. 2008 - * Copyright Red Hat, Inc. 2009 + * Copyright Red Hat, Inc. 2009, 2010 * * Authors: * Christian Ehrhardt ehrha...@linux.vnet.ibm.com -- 1.6.2.5
[Qemu-devel] [PATCH 5/9] virtio-serial: save/load: Ensure we have hot-plugged ports instantiated
If some ports that were hot-plugged on the source are not available on
the destination, fail migration instead of trying to deref a NULL
pointer.
Signed-off-by: Amit Shah amit.s...@redhat.com
Reported-by: Juan Quintela quint...@redhat.com
---
hw/virtio-serial-bus.c |7 +++
1 files changed, 7 insertions(+), 0 deletions(-)
diff --git a/hw/virtio-serial-bus.c b/hw/virtio-serial-bus.c
index bf7899c..e8eb5aa 100644
--- a/hw/virtio-serial-bus.c
+++ b/hw/virtio-serial-bus.c
@@ -447,6 +447,13 @@ static int virtio_serial_load(QEMUFile *f, void *opaque,
int version_id)
id = qemu_get_be32(f);
port = find_port_by_id(s, id);
+if (!port) {
+/*
+ * The requested port was hot-plugged on the source but we
+ * don't have it
+ */
+return -EINVAL;
+}
port-guest_connected = qemu_get_byte(f);
}
--
1.6.2.5
[Qemu-devel] [PATCH 6/9] virtio-serial-bus: Use control messages to notify guest of new ports
Allow the port 'id's to be set by a user on the command line. This is
needed by management apps that will want a stable port numbering scheme
for hot-plug/unplug and migration.
Since the port numbers are shared with the guest (to identify ports in
control messages), we just send a control message to the guest
indicating addition of new ports (hot-plug) or notifying the guest of
the available ports when the guest sends us a DEVICE_READY control
message.
Signed-off-by: Amit Shah amit.s...@redhat.com
---
hw/virtio-console.c|2 +
hw/virtio-serial-bus.c | 182 +++-
hw/virtio-serial.h | 17 +++--
3 files changed, 130 insertions(+), 71 deletions(-)
diff --git a/hw/virtio-console.c b/hw/virtio-console.c
index e915491..6b8 100644
--- a/hw/virtio-console.c
+++ b/hw/virtio-console.c
@@ -99,6 +99,7 @@ static VirtIOSerialPortInfo virtconsole_info = {
.exit = virtconsole_exitfn,
.qdev.props = (Property[]) {
DEFINE_PROP_UINT8(is_console, VirtConsole, port.is_console, 1),
+DEFINE_PROP_UINT32(nr, VirtConsole, port.id, VIRTIO_CONSOLE_BAD_ID),
DEFINE_PROP_CHR(chardev, VirtConsole, chr),
DEFINE_PROP_STRING(name, VirtConsole, port.name),
DEFINE_PROP_END_OF_LIST(),
@@ -133,6 +134,7 @@ static VirtIOSerialPortInfo virtserialport_info = {
.init = virtserialport_initfn,
.exit = virtconsole_exitfn,
.qdev.props = (Property[]) {
+DEFINE_PROP_UINT32(nr, VirtConsole, port.id, VIRTIO_CONSOLE_BAD_ID),
DEFINE_PROP_CHR(chardev, VirtConsole, chr),
DEFINE_PROP_STRING(name, VirtConsole, port.name),
DEFINE_PROP_END_OF_LIST(),
diff --git a/hw/virtio-serial-bus.c b/hw/virtio-serial-bus.c
index e8eb5aa..dd50f2d 100644
--- a/hw/virtio-serial-bus.c
+++ b/hw/virtio-serial-bus.c
@@ -41,6 +41,10 @@ struct VirtIOSerial {
VirtIOSerialBus *bus;
QTAILQ_HEAD(, VirtIOSerialPort) ports;
+
+/* bitmap for identifying active ports */
+uint32_t *ports_map;
+
struct virtio_console_config config;
};
@@ -48,6 +52,10 @@ static VirtIOSerialPort *find_port_by_id(VirtIOSerial *vser,
uint32_t id)
{
VirtIOSerialPort *port;
+if (id == VIRTIO_CONSOLE_BAD_ID) {
+return NULL;
+}
+
QTAILQ_FOREACH(port, vser-ports, next) {
if (port-id == id)
return port;
@@ -205,14 +213,25 @@ static void handle_control_message(VirtIOSerial *vser,
void *buf)
size_t buffer_len;
gcpkt = buf;
-port = find_port_by_id(vser, ldl_p(gcpkt-id));
-if (!port)
-return;
cpkt.event = lduw_p(gcpkt-event);
cpkt.value = lduw_p(gcpkt-value);
+port = find_port_by_id(vser, ldl_p(gcpkt-id));
+if (!port cpkt.event != VIRTIO_CONSOLE_DEVICE_READY)
+return;
+
switch(cpkt.event) {
+case VIRTIO_CONSOLE_DEVICE_READY:
+/*
+ * The device is up, we can now tell the device about all the
+ * ports we have here.
+ */
+QTAILQ_FOREACH(port, vser-ports, next) {
+send_control_event(port, VIRTIO_CONSOLE_PORT_ADD, 1);
+}
+break;
+
case VIRTIO_CONSOLE_PORT_READY:
/*
* Now that we know the guest asked for the port name, we're
@@ -367,13 +386,16 @@ static void virtio_serial_save(QEMUFile *f, void *opaque)
/* The config space */
qemu_put_be16s(f, s-config.cols);
qemu_put_be16s(f, s-config.rows);
-qemu_put_be32s(f, s-config.nr_ports);
-/* Items in struct VirtIOSerial */
+qemu_put_be32s(f, s-config.max_nr_ports);
+
+/* The ports map */
-qemu_put_be32s(f, s-bus-max_nr_ports);
+qemu_put_buffer(f, (uint8_t *)s-ports_map,
+sizeof(uint32_t) * (s-config.max_nr_ports + 31) / 32);
+
+/* Ports */
-/* Do this because we might have hot-unplugged some ports */
nr_active_ports = 0;
QTAILQ_FOREACH(port, s-ports, next)
nr_active_ports++;
@@ -384,11 +406,6 @@ static void virtio_serial_save(QEMUFile *f, void *opaque)
* Items in struct VirtIOSerialPort.
*/
QTAILQ_FOREACH(port, s-ports, next) {
-/*
- * We put the port number because we may not have an active
- * port at id 0 that's reserved for a console port, or in case
- * of ports that might have gotten unplugged
- */
qemu_put_be32s(f, port-id);
qemu_put_byte(f, port-guest_connected);
}
@@ -398,7 +415,8 @@ static int virtio_serial_load(QEMUFile *f, void *opaque,
int version_id)
{
VirtIOSerial *s = opaque;
VirtIOSerialPort *port;
-uint32_t max_nr_ports, nr_active_ports, nr_ports;
+size_t ports_map_size;
+uint32_t max_nr_ports, nr_active_ports, *ports_map;
unsigned int i;
if (version_id 2) {
@@ -415,29 +433,28 @@ static int virtio_serial_load(QEMUFile *f, void *opaque,
int version_id)
/* The config space */
qemu_get_be16s(f, s-config.cols);
qemu_get_be16s
[Qemu-devel] [PATCH 8/9] virtio-serial: Handle scatter-gather buffers for control messages
Current control messages are small enough to not be split into multiple
buffers but we could run into such a situation in the future or a
malicious guest could cause such a situation.
So handle the entire iov request for control messages.
Also ensure the size of the control request is = what we expect
otherwise we risk accessing memory that we don't own.
Signed-off-by: Amit Shah amit.s...@redhat.com
CC: Avi Kivity a...@redhat.com
Reported-by: Avi Kivity a...@redhat.com
---
hw/virtio-serial-bus.c | 44 +---
1 files changed, 41 insertions(+), 3 deletions(-)
diff --git a/hw/virtio-serial-bus.c b/hw/virtio-serial-bus.c
index 6d12c10..fe976ec 100644
--- a/hw/virtio-serial-bus.c
+++ b/hw/virtio-serial-bus.c
@@ -205,7 +205,7 @@ size_t virtio_serial_guest_ready(VirtIOSerialPort *port)
}
/* Guest wants to notify us of some event */
-static void handle_control_message(VirtIOSerial *vser, void *buf)
+static void handle_control_message(VirtIOSerial *vser, void *buf, size_t len)
{
struct VirtIOSerialPort *port;
struct virtio_console_control cpkt, *gcpkt;
@@ -214,6 +214,11 @@ static void handle_control_message(VirtIOSerial *vser,
void *buf)
gcpkt = buf;
+if (len sizeof(cpkt)) {
+/* The guest sent an invalid control packet */
+return;
+}
+
cpkt.event = lduw_p(gcpkt-event);
cpkt.value = lduw_p(gcpkt-value);
@@ -297,12 +302,45 @@ static void control_out(VirtIODevice *vdev, VirtQueue *vq)
{
VirtQueueElement elem;
VirtIOSerial *vser;
+uint8_t *buf;
+size_t len;
vser = DO_UPCAST(VirtIOSerial, vdev, vdev);
+len = 0;
+buf = NULL;
while (virtqueue_pop(vq, elem)) {
-handle_control_message(vser, elem.out_sg[0].iov_base);
-virtqueue_push(vq, elem, elem.out_sg[0].iov_len);
+unsigned int i;
+size_t cur_len, offset;
+
+cur_len = 0;
+for (i = 0; i elem.out_num; i++) {
+cur_len += elem.out_sg[i].iov_len;
+}
+/*
+ * Allocate a new buf only if we didn't have one previously or
+ * if the size of the buf differs
+ */
+if (cur_len != len) {
+if (len) {
+qemu_free(buf);
+}
+buf = qemu_malloc(cur_len);
+}
+
+offset = 0;
+for (i = 0; i elem.out_num; i++) {
+memcpy(buf + offset, elem.out_sg[i].iov_base,
+ elem.out_sg[i].iov_len);
+offset += elem.out_sg[i].iov_len;
+}
+len = cur_len;
+
+handle_control_message(vser, buf, len);
+virtqueue_push(vq, elem, len);
+}
+if (len) {
+qemu_free(buf);
}
virtio_notify(vdev, vq);
}
--
1.6.2.5
[Qemu-devel] [PATCH 9/9] virtio-serial: Handle scatter/gather input from the guest
Current guests don't send more than one iov but it can change later.
Ensure we handle that case.
Signed-off-by: Amit Shah amit.s...@redhat.com
CC: Avi Kivity a...@redhat.com
---
hw/virtio-serial-bus.c | 22 +++---
1 files changed, 15 insertions(+), 7 deletions(-)
diff --git a/hw/virtio-serial-bus.c b/hw/virtio-serial-bus.c
index fe976ec..2ca3c0a 100644
--- a/hw/virtio-serial-bus.c
+++ b/hw/virtio-serial-bus.c
@@ -355,11 +355,12 @@ static void handle_output(VirtIODevice *vdev, VirtQueue
*vq)
while (virtqueue_pop(vq, elem)) {
VirtIOSerialPort *port;
-size_t ret;
+size_t len;
+unsigned int i;
+len = 0;
port = find_port_by_vq(vser, vq);
if (!port) {
-ret = 0;
goto next_buf;
}
@@ -369,16 +370,23 @@ static void handle_output(VirtIODevice *vdev, VirtQueue
*vq)
* with it. Just ignore the data in that case.
*/
if (!port-info-have_data) {
-ret = 0;
goto next_buf;
}
-/* The guest always sends only one sg */
-ret = port-info-have_data(port, elem.out_sg[0].iov_base,
-elem.out_sg[0].iov_len);
+for (i = 0; i elem.out_num; i++) {
+size_t ret;
+
+ret = port-info-have_data(port, elem.out_sg[0].iov_base,
+elem.out_sg[0].iov_len);
+if (ret elem.out_sg[0].iov_len) {
+/* We couldn't write the entire iov; stop processing now */
+break;
+}
+len += ret;
+}
next_buf:
-virtqueue_push(vq, elem, ret);
+virtqueue_push(vq, elem, len);
}
virtio_notify(vdev, vq);
}
--
1.6.2.5
[Qemu-devel] Re: [PATCH 9/9] virtio-serial: Handle scatter/gather input from the guest
On (Tue) Mar 23 2010 [20:00:19], Amit Shah wrote:
@@ -369,16 +370,23 @@ static void handle_output(VirtIODevice *vdev, VirtQueue
*vq)
* with it. Just ignore the data in that case.
*/
if (!port-info-have_data) {
-ret = 0;
goto next_buf;
}
-/* The guest always sends only one sg */
-ret = port-info-have_data(port, elem.out_sg[0].iov_base,
-elem.out_sg[0].iov_len);
+for (i = 0; i elem.out_num; i++) {
+size_t ret;
+
+ret = port-info-have_data(port, elem.out_sg[0].iov_base,
+elem.out_sg[0].iov_len);
+if (ret elem.out_sg[0].iov_len) {
+/* We couldn't write the entire iov; stop processing now */
+break;
We should increment len here if ret 0.
I'll post a followup patch that does this.
+}
+len += ret;
+}
next_buf:
-virtqueue_push(vq, elem, ret);
+virtqueue_push(vq, elem, len);
}
virtio_notify(vdev, vq);
}
--
1.6.2.5
Amit
Re: [Qemu-devel] Re: [PATCH 4/9] virtio-serial: Handle scatter-gather buffers for control messages
On (Tue) Mar 23 2010 [17:51:26], Michael S. Tsirkin wrote:
On Mon, Mar 22, 2010 at 10:48:02AM +0530, Amit Shah wrote:
On (Sat) Mar 20 2010 [09:40:50], Avi Kivity wrote:
On 03/19/2010 01:58 PM, Amit Shah wrote:
+
+offset = 0;
+for (i = 0; i elem.out_num; i++) {
+memcpy(buf + offset, elem.out_sg[i].iov_base,
+ elem.out_sg[i].iov_len);
+offset += elem.out_sg[i].iov_len;
+}
+len = cur_len;
+
+handle_control_message(vser, buf, len);
+virtqueue_push(vq,elem, len);
+}
+if (len) {
+qemu_free(buf);
}
virtio_notify(vdev, vq);
}
Isn't there some virtio function to linearize requests?
I don't see one.
virtio-net has iov_fill. Reuse it?
Hm, yeah. Any ideas on how to share it? Put it in some common file?
Just copying it seems good for now..
Amit
[Qemu-devel] Re: hi, may I ask some help on the paravirtualization of KVM?
On (Wed) Mar 24 2010 [11:35:09], Liang YANG wrote: Last, start the image Guest use qemu-system-x86_64 -net nic, model=virtio. This time I can see virtio_net module in GuestOS, but it doesn't work. The qemu didn't emulate the virtio net device. So I update the code located in pc_init1(), where the net model is set. After this time update, I can start the kvm-paravirtualization successfully. So this is a bug; can you please post the patch you made? I don't think anyone else has complained of this so must be specific to your setup. Amit -- http://log.amitshah.net/
[Qemu-devel] [PATCH 01/15] virtio-serial: save/load: Ensure target has enough ports
The target could be started with max_nr_ports for a virtio-serial device
lesser than what was available on the source machine. Fail the migration
in such a case.
Signed-off-by: Amit Shah amit.s...@redhat.com
Reported-by: Juan Quintela quint...@redhat.com
---
hw/virtio-serial-bus.c | 13 +++--
1 files changed, 11 insertions(+), 2 deletions(-)
diff --git a/hw/virtio-serial-bus.c b/hw/virtio-serial-bus.c
index 17c1ec1..9a7f0c1 100644
--- a/hw/virtio-serial-bus.c
+++ b/hw/virtio-serial-bus.c
@@ -374,10 +374,13 @@ static void virtio_serial_save(QEMUFile *f, void *opaque)
/* Items in struct VirtIOSerial */
+qemu_put_be32s(f, s-bus-max_nr_ports);
+
/* Do this because we might have hot-unplugged some ports */
nr_active_ports = 0;
-QTAILQ_FOREACH(port, s-ports, next)
+QTAILQ_FOREACH(port, s-ports, next) {
nr_active_ports++;
+}
qemu_put_be32s(f, nr_active_ports);
@@ -399,7 +402,7 @@ static int virtio_serial_load(QEMUFile *f, void *opaque,
int version_id)
{
VirtIOSerial *s = opaque;
VirtIOSerialPort *port;
-uint32_t nr_active_ports;
+uint32_t max_nr_ports, nr_active_ports;
unsigned int i;
if (version_id 2) {
@@ -420,6 +423,12 @@ static int virtio_serial_load(QEMUFile *f, void *opaque,
int version_id)
/* Items in struct VirtIOSerial */
+qemu_get_be32s(f, max_nr_ports);
+if (max_nr_ports s-bus-max_nr_ports) {
+/* Source could have more ports than us. Fail migration. */
+return -EINVAL;
+}
+
qemu_get_be32s(f, nr_active_ports);
/* Items in struct VirtIOSerialPort */
--
1.6.2.5
[Qemu-devel] [PATCH 02/15] virtio-serial: save/load: Ensure nr_ports on src and dest are same.
The number of ports on the source as well as the destination machines
should match. If they don't, it means some ports that got hotplugged on
the source aren't instantiated on the destination. Or that ports that
were hot-unplugged on the source are created on the destination.
Signed-off-by: Amit Shah amit.s...@redhat.com
Reported-by: Juan Quintela quint...@redhat.com
---
hw/virtio-serial-bus.c | 18 --
1 files changed, 16 insertions(+), 2 deletions(-)
diff --git a/hw/virtio-serial-bus.c b/hw/virtio-serial-bus.c
index 9a7f0c1..d31e62d 100644
--- a/hw/virtio-serial-bus.c
+++ b/hw/virtio-serial-bus.c
@@ -402,7 +402,7 @@ static int virtio_serial_load(QEMUFile *f, void *opaque,
int version_id)
{
VirtIOSerial *s = opaque;
VirtIOSerialPort *port;
-uint32_t max_nr_ports, nr_active_ports;
+uint32_t max_nr_ports, nr_active_ports, nr_ports;
unsigned int i;
if (version_id 2) {
@@ -419,7 +419,21 @@ static int virtio_serial_load(QEMUFile *f, void *opaque,
int version_id)
/* The config space */
qemu_get_be16s(f, s-config.cols);
qemu_get_be16s(f, s-config.rows);
-s-config.nr_ports = qemu_get_be32(f);
+nr_ports = qemu_get_be32(f);
+
+if (nr_ports != s-config.nr_ports) {
+/*
+ * Source hot-plugged/unplugged ports and we don't have all of
+ * them here.
+ *
+ * Note: This condition cannot check for all hotplug/unplug
+ * events: eg, if one port was hot-plugged and one was
+ * unplugged, the nr_ports remains the same but the port id's
+ * would have changed and we won't catch it here. A later
+ * check for !find_port_by_id() will confirm if this happened.
+ */
+return -EINVAL;
+}
/* Items in struct VirtIOSerial */
--
1.6.2.5
[Qemu-devel] [PATCH 04/15] virtio-serial: save/load: Send target host connection status if different
If the host connection to a port is closed on the destination machine
after migration, whereas the connection was open on the source, the
guest has to be informed of that.
Similar for a host connection open on the destination.
Signed-off-by: Amit Shah amit.s...@redhat.com
---
hw/virtio-serial-bus.c | 11 +++
1 files changed, 11 insertions(+), 0 deletions(-)
diff --git a/hw/virtio-serial-bus.c b/hw/virtio-serial-bus.c
index 5316ef6..484dc94 100644
--- a/hw/virtio-serial-bus.c
+++ b/hw/virtio-serial-bus.c
@@ -395,6 +395,7 @@ static void virtio_serial_save(QEMUFile *f, void *opaque)
*/
qemu_put_be32s(f, port-id);
qemu_put_byte(f, port-guest_connected);
+qemu_put_byte(f, port-host_connected);
}
}
@@ -448,6 +449,7 @@ static int virtio_serial_load(QEMUFile *f, void *opaque,
int version_id)
/* Items in struct VirtIOSerialPort */
for (i = 0; i nr_active_ports; i++) {
uint32_t id;
+bool host_connected;
id = qemu_get_be32(f);
port = find_port_by_id(s, id);
@@ -460,6 +462,15 @@ static int virtio_serial_load(QEMUFile *f, void *opaque,
int version_id)
}
port-guest_connected = qemu_get_byte(f);
+host_connected = qemu_get_byte(f);
+if (host_connected != port-host_connected) {
+/*
+ * We have to let the guest know of the host connection
+ * status change
+ */
+send_control_event(port, VIRTIO_CONSOLE_PORT_OPEN,
+ port-host_connected);
+}
}
return 0;
--
1.6.2.5
[Qemu-devel] [PATCH 05/15] virtio-serial: Use control messages to notify guest of new ports
Allow the port 'id's to be set by a user on the command line. This is
needed by management apps that will want a stable port numbering scheme
for hot-plug/unplug and migration.
Since the port numbers are shared with the guest (to identify ports in
control messages), we just send a control message to the guest
indicating addition of new ports (hot-plug) or notifying the guest of
the available ports when the guest sends us a DEVICE_READY control
message.
Signed-off-by: Amit Shah amit.s...@redhat.com
---
hw/virtio-console.c|2 +
hw/virtio-serial-bus.c | 181 +++-
hw/virtio-serial.h | 17 +++--
3 files changed, 130 insertions(+), 70 deletions(-)
diff --git a/hw/virtio-console.c b/hw/virtio-console.c
index bd44ec6..17b221d 100644
--- a/hw/virtio-console.c
+++ b/hw/virtio-console.c
@@ -99,6 +99,7 @@ static VirtIOSerialPortInfo virtconsole_info = {
.exit = virtconsole_exitfn,
.qdev.props = (Property[]) {
DEFINE_PROP_UINT8(is_console, VirtConsole, port.is_console, 1),
+DEFINE_PROP_UINT32(nr, VirtConsole, port.id, VIRTIO_CONSOLE_BAD_ID),
DEFINE_PROP_CHR(chardev, VirtConsole, chr),
DEFINE_PROP_STRING(name, VirtConsole, port.name),
DEFINE_PROP_END_OF_LIST(),
@@ -133,6 +134,7 @@ static VirtIOSerialPortInfo virtserialport_info = {
.init = virtserialport_initfn,
.exit = virtconsole_exitfn,
.qdev.props = (Property[]) {
+DEFINE_PROP_UINT32(nr, VirtConsole, port.id, VIRTIO_CONSOLE_BAD_ID),
DEFINE_PROP_CHR(chardev, VirtConsole, chr),
DEFINE_PROP_STRING(name, VirtConsole, port.name),
DEFINE_PROP_END_OF_LIST(),
diff --git a/hw/virtio-serial-bus.c b/hw/virtio-serial-bus.c
index 484dc94..00e8616 100644
--- a/hw/virtio-serial-bus.c
+++ b/hw/virtio-serial-bus.c
@@ -41,6 +41,10 @@ struct VirtIOSerial {
VirtIOSerialBus *bus;
QTAILQ_HEAD(, VirtIOSerialPort) ports;
+
+/* bitmap for identifying active ports */
+uint32_t *ports_map;
+
struct virtio_console_config config;
};
@@ -48,6 +52,10 @@ static VirtIOSerialPort *find_port_by_id(VirtIOSerial *vser,
uint32_t id)
{
VirtIOSerialPort *port;
+if (id == VIRTIO_CONSOLE_BAD_ID) {
+return NULL;
+}
+
QTAILQ_FOREACH(port, vser-ports, next) {
if (port-id == id)
return port;
@@ -208,14 +216,25 @@ static void handle_control_message(VirtIOSerial *vser,
void *buf)
size_t buffer_len;
gcpkt = buf;
-port = find_port_by_id(vser, ldl_p(gcpkt-id));
-if (!port)
-return;
cpkt.event = lduw_p(gcpkt-event);
cpkt.value = lduw_p(gcpkt-value);
+port = find_port_by_id(vser, ldl_p(gcpkt-id));
+if (!port cpkt.event != VIRTIO_CONSOLE_DEVICE_READY)
+return;
+
switch(cpkt.event) {
+case VIRTIO_CONSOLE_DEVICE_READY:
+/*
+ * The device is up, we can now tell the device about all the
+ * ports we have here.
+ */
+QTAILQ_FOREACH(port, vser-ports, next) {
+send_control_event(port, VIRTIO_CONSOLE_PORT_ADD, 1);
+}
+break;
+
case VIRTIO_CONSOLE_PORT_READY:
/*
* Now that we know the guest asked for the port name, we're
@@ -370,13 +389,16 @@ static void virtio_serial_save(QEMUFile *f, void *opaque)
/* The config space */
qemu_put_be16s(f, s-config.cols);
qemu_put_be16s(f, s-config.rows);
-qemu_put_be32s(f, s-config.nr_ports);
-/* Items in struct VirtIOSerial */
+qemu_put_be32s(f, s-config.max_nr_ports);
+
+/* The ports map */
-qemu_put_be32s(f, s-bus-max_nr_ports);
+qemu_put_buffer(f, (uint8_t *)s-ports_map,
+sizeof(uint32_t) * (s-config.max_nr_ports + 31) / 32);
+
+/* Ports */
-/* Do this because we might have hot-unplugged some ports */
nr_active_ports = 0;
QTAILQ_FOREACH(port, s-ports, next) {
nr_active_ports++;
@@ -388,11 +410,6 @@ static void virtio_serial_save(QEMUFile *f, void *opaque)
* Items in struct VirtIOSerialPort.
*/
QTAILQ_FOREACH(port, s-ports, next) {
-/*
- * We put the port number because we may not have an active
- * port at id 0 that's reserved for a console port, or in case
- * of ports that might have gotten unplugged
- */
qemu_put_be32s(f, port-id);
qemu_put_byte(f, port-guest_connected);
qemu_put_byte(f, port-host_connected);
@@ -403,7 +420,8 @@ static int virtio_serial_load(QEMUFile *f, void *opaque,
int version_id)
{
VirtIOSerial *s = opaque;
VirtIOSerialPort *port;
-uint32_t max_nr_ports, nr_active_ports, nr_ports;
+size_t ports_map_size;
+uint32_t max_nr_ports, nr_active_ports, *ports_map;
unsigned int i;
if (version_id 2) {
@@ -420,29 +438,28 @@ static int virtio_serial_load(QEMUFile *f, void *opaque,
int version_id)
/* The config space
[Qemu-devel] [PATCH 06/15] virtio-serial: whitespace: match surrounding code
The virtio-serial code doesn't mix declarations and definitions, so
separate them out on different lines.
Signed-off-by: Amit Shah amit.s...@redhat.com
---
hw/virtio-serial-bus.c |5 -
1 files changed, 4 insertions(+), 1 deletions(-)
diff --git a/hw/virtio-serial-bus.c b/hw/virtio-serial-bus.c
index 00e8616..80f0259 100644
--- a/hw/virtio-serial-bus.c
+++ b/hw/virtio-serial-bus.c
@@ -354,7 +354,10 @@ static void handle_input(VirtIODevice *vdev, VirtQueue *vq)
static uint32_t get_features(VirtIODevice *vdev, uint32_t features)
{
-VirtIOSerial *vser = DO_UPCAST(VirtIOSerial, vdev, vdev);
+VirtIOSerial *vser;
+
+vser = DO_UPCAST(VirtIOSerial, vdev, vdev);
+
if (vser-bus-max_nr_ports 1) {
features |= (1 VIRTIO_CONSOLE_F_MULTIPORT);
}
--
1.6.2.5
[Qemu-devel] [PATCH 08/15] virtio-serial: Update copyright year to 2010
Signed-off-by: Amit Shah amit.s...@redhat.com --- hw/virtio-console.c|2 +- hw/virtio-serial-bus.c |2 +- hw/virtio-serial.h |2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/hw/virtio-console.c b/hw/virtio-console.c index 17b221d..6b8 100644 --- a/hw/virtio-console.c +++ b/hw/virtio-console.c @@ -1,7 +1,7 @@ /* * Virtio Console and Generic Serial Port Devices * - * Copyright Red Hat, Inc. 2009 + * Copyright Red Hat, Inc. 2009, 2010 * * Authors: * Amit Shah amit.s...@redhat.com diff --git a/hw/virtio-serial-bus.c b/hw/virtio-serial-bus.c index 4435c62..a19e751 100644 --- a/hw/virtio-serial-bus.c +++ b/hw/virtio-serial-bus.c @@ -1,7 +1,7 @@ /* * A bus for connecting virtio serial and console ports * - * Copyright (C) 2009 Red Hat, Inc. + * Copyright (C) 2009, 2010 Red Hat, Inc. * * Author(s): * Amit Shah amit.s...@redhat.com diff --git a/hw/virtio-serial.h b/hw/virtio-serial.h index 0548689..f023873 100644 --- a/hw/virtio-serial.h +++ b/hw/virtio-serial.h @@ -2,7 +2,7 @@ * Virtio Serial / Console Support * * Copyright IBM, Corp. 2008 - * Copyright Red Hat, Inc. 2009 + * Copyright Red Hat, Inc. 2009, 2010 * * Authors: * Christian Ehrhardt ehrha...@linux.vnet.ibm.com -- 1.6.2.5
[Qemu-devel] [PATCH 09/15] virtio-serial: Propagate errors in initialising ports / devices in guest
If adding of ports or devices in the guest fails we can send out a QMP
event so that management software can deal with it.
Signed-off-by: Amit Shah amit.s...@redhat.com
---
hw/virtio-serial-bus.c | 10 ++
1 files changed, 10 insertions(+), 0 deletions(-)
diff --git a/hw/virtio-serial-bus.c b/hw/virtio-serial-bus.c
index a19e751..33083af 100644
--- a/hw/virtio-serial-bus.c
+++ b/hw/virtio-serial-bus.c
@@ -223,6 +223,11 @@ static void handle_control_message(VirtIOSerial *vser,
void *buf)
switch(cpkt.event) {
case VIRTIO_CONSOLE_DEVICE_READY:
+if (!cpkt.value) {
+error_report(virtio-serial-bus: Guest failure in adding device
%s\n,
+vser-bus-qbus.name);
+break;
+}
/*
* The device is up, we can now tell the device about all the
* ports we have here.
@@ -233,6 +238,11 @@ static void handle_control_message(VirtIOSerial *vser,
void *buf)
break;
case VIRTIO_CONSOLE_PORT_READY:
+if (!cpkt.value) {
+error_report(virtio-serial-bus: Guest failure in adding port %u
for device %s\n,
+ port-id, vser-bus-qbus.name);
+break;
+}
/*
* Now that we know the guest asked for the port name, we're
* sure the guest has initialised whatever state is necessary
--
1.6.2.5
[Qemu-devel] [PATCH 07/15] virtio-serial: Remove redundant check for 0-sized write request
The check for a 0-sized write request to a guest port is not necessary;
the while loop below won't be executed in this case and all will be
fine.
Signed-off-by: Amit Shah amit.s...@redhat.com
---
hw/virtio-serial-bus.c |3 ---
1 files changed, 0 insertions(+), 3 deletions(-)
diff --git a/hw/virtio-serial-bus.c b/hw/virtio-serial-bus.c
index 80f0259..4435c62 100644
--- a/hw/virtio-serial-bus.c
+++ b/hw/virtio-serial-bus.c
@@ -91,9 +91,6 @@ static size_t write_to_port(VirtIOSerialPort *port,
if (!virtio_queue_ready(vq)) {
return 0;
}
-if (!size) {
-return 0;
-}
while (offset size) {
int i;
--
1.6.2.5
[Qemu-devel] [PATCH 10/15] virtio-serial: Add QMP events for failed port/device add
When adding a port or a device to the guest fails, management software
might be interested in knowing and then cleaning up the host-side of the
port. Introduce QMP events to signal such errors.
Signed-off-by: Amit Shah amit.s...@redhat.com
CC: Luiz Capitulino lcapitul...@redhat.com
---
QMP/qmp-events.txt | 48
hw/virtio-serial-bus.c | 15 +++
monitor.c |3 +++
monitor.h |1 +
4 files changed, 67 insertions(+), 0 deletions(-)
diff --git a/QMP/qmp-events.txt b/QMP/qmp-events.txt
index a94e9b4..f13cf45 100644
--- a/QMP/qmp-events.txt
+++ b/QMP/qmp-events.txt
@@ -188,3 +188,51 @@ Example:
Note: If action is reset, shutdown, or pause the WATCHDOG event is
followed respectively by the RESET, SHUTDOWN, or STOP events.
+
+VIRTIO_SERIAL
+-
+
+Emitted when errors occur in guest port add or guest device add.
+
+Data:
+
+- device: The virtio-serial device that triggered the event {json-string}
+ This is the name given to the bus on the command line:
+-device virtio-serial,id=foo
+ here, the device name is foo
+
+- port: The port number that triggered the event {json-number}
+ This is the number given to the port on the command line:
+-device virtserialport,nr=2
+ here, the port number is 2. If not mentioned on the command
+ line, the number is auto-assigned.
+
+- result: The result of the operation {json-string}
+ This is one of the following:
+ pass, fail
+
+- operation: The operation that triggered the event {json-sring}
+ This is one of the following:
+ port_add, device_add
+
+Example:
+
+Port 0 add failure in the guest:
+
+{ timestamp: {seconds: 1269438649, microseconds: 851170},
+ event: VIRTIO_SERIAL,
+ data: {
+device: virtio-serial-bus.0,
+port: 0,
+result: fail,
+operation: port_add } }
+
+Device add failure in the guest:
+
+{ timestamp: {seconds: 1269438702, microseconds: 78114},
+ event: VIRTIO_SERIAL,
+ data: {
+device: virtio-serial-bus.0,
+port: 4294967295,
+result: fail,
+operation: device_add } }
diff --git a/hw/virtio-serial-bus.c b/hw/virtio-serial-bus.c
index 33083af..efcc66c 100644
--- a/hw/virtio-serial-bus.c
+++ b/hw/virtio-serial-bus.c
@@ -16,6 +16,7 @@
*/
#include monitor.h
+#include qemu-objects.h
#include qemu-queue.h
#include sysbus.h
#include virtio-serial.h
@@ -204,6 +205,17 @@ size_t virtio_serial_guest_ready(VirtIOSerialPort *port)
return 0;
}
+static void mon_event(const char *device, const uint32_t port_id,
+ const char *operation, const char *result)
+{
+QObject *data;
+
+data = qobject_from_jsonf({ 'device': %s, 'port': %ld, 'operation': %s,
'result': %s },
+ device, (int64_t)port_id, operation, result);
+monitor_protocol_event(QEVENT_VIRTIO_SERIAL, data);
+qobject_decref(data);
+}
+
/* Guest wants to notify us of some event */
static void handle_control_message(VirtIOSerial *vser, void *buf)
{
@@ -226,6 +238,8 @@ static void handle_control_message(VirtIOSerial *vser, void
*buf)
if (!cpkt.value) {
error_report(virtio-serial-bus: Guest failure in adding device
%s\n,
vser-bus-qbus.name);
+mon_event(vser-bus-qbus.name, VIRTIO_CONSOLE_BAD_ID,
+ device_add, fail);
break;
}
/*
@@ -241,6 +255,7 @@ static void handle_control_message(VirtIOSerial *vser, void
*buf)
if (!cpkt.value) {
error_report(virtio-serial-bus: Guest failure in adding port %u
for device %s\n,
port-id, vser-bus-qbus.name);
+mon_event(vser-bus-qbus.name, port-id, port_add, fail);
break;
}
/*
diff --git a/monitor.c b/monitor.c
index 0448a70..9e5bfe7 100644
--- a/monitor.c
+++ b/monitor.c
@@ -441,6 +441,9 @@ void monitor_protocol_event(MonitorEvent event, QObject
*data)
case QEVENT_WATCHDOG:
event_name = WATCHDOG;
break;
+case QEVENT_VIRTIO_SERIAL:
+event_name = VIRTIO_SERIAL;
+break;
default:
abort();
break;
diff --git a/monitor.h b/monitor.h
index bd4ae34..ea4df8a 100644
--- a/monitor.h
+++ b/monitor.h
@@ -28,6 +28,7 @@ typedef enum MonitorEvent {
QEVENT_BLOCK_IO_ERROR,
QEVENT_RTC_CHANGE,
QEVENT_WATCHDOG,
+QEVENT_VIRTIO_SERIAL,
QEVENT_MAX,
} MonitorEvent;
--
1.6.2.5
[Qemu-devel] [PATCH 11/15] virtio-serial: Send out guest data to ports only if port is opened
Data should be written only when ports are open.
Signed-off-by: Amit Shah amit.s...@redhat.com
---
hw/virtio-serial-bus.c |5 +
1 files changed, 5 insertions(+), 0 deletions(-)
diff --git a/hw/virtio-serial-bus.c b/hw/virtio-serial-bus.c
index efcc66c..80fbff4 100644
--- a/hw/virtio-serial-bus.c
+++ b/hw/virtio-serial-bus.c
@@ -350,6 +350,11 @@ static void handle_output(VirtIODevice *vdev, VirtQueue
*vq)
goto next_buf;
}
+ if (!port-host_connected) {
+ret = 0;
+goto next_buf;
+}
+
/*
* A port may not have any handler registered for consuming the
* data that the guest sends or it may not have a chardev associated
--
1.6.2.5
[Qemu-devel] [PATCH 12/15] iov: Introduce a new file for helpers around iovs, add iov_from_buf()
The virtio-net code uses iov_fill() which fills an iov from a linear
buffer. The virtio-serial-bus code does something similar in an
open-coded function.
Create a new iov.c file that has iov_from_buf().
Convert virtio-net and virtio-serial-bus over to use this functionality.
virtio-net used ints to hold sizes, the new function is going to use
size_t types.
Later commits will add the opposite functionality -- going from an iov
to a linear buffer.
Signed-off-by: Amit Shah amit.s...@redhat.com
---
Makefile.objs |1 +
hw/iov.c | 33 +
hw/iov.h | 16
hw/virtio-net.c| 20 +++-
hw/virtio-serial-bus.c | 15 +++
5 files changed, 60 insertions(+), 25 deletions(-)
create mode 100644 hw/iov.c
create mode 100644 hw/iov.h
diff --git a/Makefile.objs b/Makefile.objs
index 281f7a6..212ae1d 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -127,6 +127,7 @@ user-obj-y += cutils.o cache-utils.o
# libhw
hw-obj-y =
+hw-obj-y += iov.o
hw-obj-y += loader.o
hw-obj-y += virtio.o virtio-console.o virtio-pci.o
hw-obj-y += fw_cfg.o pci.o pci_host.o pcie_host.o
diff --git a/hw/iov.c b/hw/iov.c
new file mode 100644
index 000..07bd499
--- /dev/null
+++ b/hw/iov.c
@@ -0,0 +1,33 @@
+/*
+ * Helpers for getting linearized buffers from iov / filling buffers into iovs
+ *
+ * Copyright IBM, Corp. 2007, 2008
+ * Copyright (C) 2010 Red Hat, Inc.
+ *
+ * Author(s):
+ * Anthony Liguori aligu...@us.ibm.com
+ * Amit Shah amit.s...@redhat.com
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2. See
+ * the COPYING file in the top-level directory.
+ */
+
+#include iov.h
+
+size_t iov_from_buf(struct iovec *iov, unsigned int iovcnt,
+const void *buf, size_t size)
+{
+size_t offset;
+unsigned int i;
+
+offset = 0;
+for (i = 0; offset size i iovcnt; i++) {
+size_t len;
+
+len = MIN(iov[i].iov_len, size - offset);
+
+memcpy(iov[i].iov_base, buf + offset, len);
+offset += len;
+}
+return offset;
+}
diff --git a/hw/iov.h b/hw/iov.h
new file mode 100644
index 000..5e3e541
--- /dev/null
+++ b/hw/iov.h
@@ -0,0 +1,16 @@
+/*
+ * Helpers for getting linearized buffers from iov / filling buffers into iovs
+ *
+ * Copyright (C) 2010 Red Hat, Inc.
+ *
+ * Author(s):
+ * Amit Shah amit.s...@redhat.com
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2. See
+ * the COPYING file in the top-level directory.
+ */
+
+#include qemu-common.h
+
+size_t iov_from_buf(struct iovec *iov, unsigned int iovcnt,
+const void *buf, size_t size);
diff --git a/hw/virtio-net.c b/hw/virtio-net.c
index be33c68..273e7f9 100644
--- a/hw/virtio-net.c
+++ b/hw/virtio-net.c
@@ -11,6 +11,7 @@
*
*/
+#include iov.h
#include virtio.h
#include net.h
#include net/checksum.h
@@ -423,21 +424,6 @@ static void work_around_broken_dhclient(struct
virtio_net_hdr *hdr,
}
}
-static int iov_fill(struct iovec *iov, int iovcnt, const void *buf, int count)
-{
-int offset, i;
-
-offset = i = 0;
-while (offset count i iovcnt) {
-int len = MIN(iov[i].iov_len, count - offset);
-memcpy(iov[i].iov_base, buf + offset, len);
-offset += len;
-i++;
-}
-
-return offset;
-}
-
static int receive_header(VirtIONet *n, struct iovec *iov, int iovcnt,
const void *buf, size_t size, size_t hdr_len)
{
@@ -573,8 +559,8 @@ static ssize_t virtio_net_receive(VLANClientState *nc,
const uint8_t *buf, size_
}
/* copy in packet. ugh */
-len = iov_fill(sg, elem.in_num,
- buf + offset, size - offset);
+len = iov_from_buf(sg, elem.in_num,
+ buf + offset, size - offset);
total += len;
/* signal other side */
diff --git a/hw/virtio-serial-bus.c b/hw/virtio-serial-bus.c
index 80fbff4..bd1223e 100644
--- a/hw/virtio-serial-bus.c
+++ b/hw/virtio-serial-bus.c
@@ -15,6 +15,7 @@
* the COPYING file in the top-level directory.
*/
+#include iov.h
#include monitor.h
#include qemu-objects.h
#include qemu-queue.h
@@ -85,27 +86,25 @@ static size_t write_to_port(VirtIOSerialPort *port,
{
VirtQueueElement elem;
VirtQueue *vq;
-size_t offset = 0;
-size_t len = 0;
+size_t offset;
vq = port-ivq;
if (!virtio_queue_ready(vq)) {
return 0;
}
+offset = 0;
while (offset size) {
-int i;
+size_t len;
if (!virtqueue_pop(vq, elem)) {
break;
}
-for (i = 0; offset size i elem.in_num; i++) {
-len = MIN(elem.in_sg[i].iov_len, size - offset);
+len = iov_from_buf(elem.in_sg, elem.in_num,
+ buf + offset, size - offset);
+offset += len;
-memcpy(elem.in_sg[i].iov_base, buf + offset
[Qemu-devel] [PATCH 13/15] iov: Add iov_to_buf and iov_size helpers
iov_to_buf() puts the buffer contents in the iov in a linearized buffer.
iov_size() gets the length of the contents in the iov.
The iov_to_buf() function is the memcpy_to_iovec() function that was
used in virtio-ballon.c.
Signed-off-by: Amit Shah amit.s...@redhat.com
---
hw/iov.c| 37 +
hw/iov.h|3 +++
hw/virtio-balloon.c | 35 ---
3 files changed, 44 insertions(+), 31 deletions(-)
diff --git a/hw/iov.c b/hw/iov.c
index 07bd499..d4013cd 100644
--- a/hw/iov.c
+++ b/hw/iov.c
@@ -31,3 +31,40 @@ size_t iov_from_buf(struct iovec *iov, unsigned int iovcnt,
}
return offset;
}
+
+size_t iov_to_buf(struct iovec *iov, unsigned int iovcnt,
+ void *buf, size_t offset, size_t size)
+{
+uint8_t *ptr;
+size_t iov_off, buf_off;
+unsigned int i;
+
+ptr = buf;
+iov_off = 0;
+buf_off = 0;
+for (i = 0; i iovcnt size; i++) {
+if (offset (iov_off + iov[i].iov_len)) {
+size_t len = MIN((iov_off + iov[i].iov_len) - offset , size);
+
+memcpy(ptr + buf_off, iov[i].iov_base + (offset - iov_off), len);
+
+buf_off += len;
+offset += len;
+size -= len;
+}
+iov_off += iov[i].iov_len;
+}
+return buf_off;
+}
+
+size_t iov_size(struct iovec *iov, unsigned int iovcnt)
+{
+size_t len;
+unsigned int i;
+
+len = 0;
+for (i = 0; i iovcnt; i++) {
+len += iov[i].iov_len;
+}
+return len;
+}
diff --git a/hw/iov.h b/hw/iov.h
index 5e3e541..c977ff1 100644
--- a/hw/iov.h
+++ b/hw/iov.h
@@ -14,3 +14,6 @@
size_t iov_from_buf(struct iovec *iov, unsigned int iovcnt,
const void *buf, size_t size);
+size_t iov_to_buf(struct iovec *iov, unsigned int iovcnt,
+ void *buf, size_t offset, size_t size);
+size_t iov_size(struct iovec *iov, unsigned int iovcnt);
diff --git a/hw/virtio-balloon.c b/hw/virtio-balloon.c
index 6d12024..4414eae 100644
--- a/hw/virtio-balloon.c
+++ b/hw/virtio-balloon.c
@@ -11,6 +11,7 @@
*
*/
+#include iov.h
#include qemu-common.h
#include virtio.h
#include pc.h
@@ -91,33 +92,6 @@ static QObject *get_stats_qobject(VirtIOBalloon *dev)
return QOBJECT(dict);
}
-/* FIXME: once we do a virtio refactoring, this will get subsumed into common
- * code */
-static size_t memcpy_from_iovector(void *data, size_t offset, size_t size,
- struct iovec *iov, int iovlen)
-{
-int i;
-uint8_t *ptr = data;
-size_t iov_off = 0;
-size_t data_off = 0;
-
-for (i = 0; i iovlen size; i++) {
-if (offset (iov_off + iov[i].iov_len)) {
-size_t len = MIN((iov_off + iov[i].iov_len) - offset , size);
-
-memcpy(ptr + data_off, iov[i].iov_base + (offset - iov_off), len);
-
-data_off += len;
-offset += len;
-size -= len;
-}
-
-iov_off += iov[i].iov_len;
-}
-
-return data_off;
-}
-
static void virtio_balloon_handle_output(VirtIODevice *vdev, VirtQueue *vq)
{
VirtIOBalloon *s = to_virtio_balloon(vdev);
@@ -127,8 +101,7 @@ static void virtio_balloon_handle_output(VirtIODevice
*vdev, VirtQueue *vq)
size_t offset = 0;
uint32_t pfn;
-while (memcpy_from_iovector(pfn, offset, 4,
-elem.out_sg, elem.out_num) == 4) {
+while (iov_to_buf(elem.out_sg, elem.out_num, pfn, offset, 4) == 4) {
ram_addr_t pa;
ram_addr_t addr;
@@ -180,8 +153,8 @@ static void virtio_balloon_receive_stats(VirtIODevice
*vdev, VirtQueue *vq)
*/
reset_stats(s);
-while (memcpy_from_iovector(stat, offset, sizeof(stat), elem-out_sg,
-elem-out_num) == sizeof(stat)) {
+while (iov_to_buf(elem-out_sg, elem-out_num, stat, offset, sizeof(stat))
+ == sizeof(stat)) {
uint16_t tag = tswap16(stat.tag);
uint64_t val = tswap64(stat.val);
--
1.6.2.5
[Qemu-devel] [PATCH 14/15] virtio-serial: Handle scatter-gather buffers for control messages
Current control messages are small enough to not be split into multiple
buffers but we could run into such a situation in the future or a
malicious guest could cause such a situation.
So handle the entire iov request for control messages.
Also ensure the size of the control request is = what we expect
otherwise we risk accessing memory that we don't own.
Signed-off-by: Amit Shah amit.s...@redhat.com
CC: Avi Kivity a...@redhat.com
Reported-by: Avi Kivity a...@redhat.com
---
hw/virtio-serial-bus.c | 34 +++---
1 files changed, 31 insertions(+), 3 deletions(-)
diff --git a/hw/virtio-serial-bus.c b/hw/virtio-serial-bus.c
index bd1223e..3edfeca 100644
--- a/hw/virtio-serial-bus.c
+++ b/hw/virtio-serial-bus.c
@@ -216,7 +216,7 @@ static void mon_event(const char *device, const uint32_t
port_id,
}
/* Guest wants to notify us of some event */
-static void handle_control_message(VirtIOSerial *vser, void *buf)
+static void handle_control_message(VirtIOSerial *vser, void *buf, size_t len)
{
struct VirtIOSerialPort *port;
struct virtio_console_control cpkt, *gcpkt;
@@ -225,6 +225,11 @@ static void handle_control_message(VirtIOSerial *vser,
void *buf)
gcpkt = buf;
+if (len sizeof(cpkt)) {
+/* The guest sent an invalid control packet */
+return;
+}
+
cpkt.event = lduw_p(gcpkt-event);
cpkt.value = lduw_p(gcpkt-value);
@@ -321,12 +326,35 @@ static void control_out(VirtIODevice *vdev, VirtQueue *vq)
{
VirtQueueElement elem;
VirtIOSerial *vser;
+uint8_t *buf;
+size_t len;
vser = DO_UPCAST(VirtIOSerial, vdev, vdev);
+len = 0;
+buf = NULL;
while (virtqueue_pop(vq, elem)) {
-handle_control_message(vser, elem.out_sg[0].iov_base);
-virtqueue_push(vq, elem, elem.out_sg[0].iov_len);
+size_t cur_len, copied;
+
+cur_len = iov_size(elem.out_sg, elem.out_num);
+/*
+ * Allocate a new buf only if we didn't have one previously or
+ * if the size of the buf differs
+ */
+if (cur_len != len) {
+if (len) {
+qemu_free(buf);
+}
+buf = qemu_malloc(cur_len);
+len = cur_len;
+}
+copied = iov_to_buf(elem.out_sg, elem.out_num, buf, 0, len);
+
+handle_control_message(vser, buf, copied);
+virtqueue_push(vq, elem, copied);
+}
+if (len) {
+qemu_free(buf);
}
virtio_notify(vdev, vq);
}
--
1.6.2.5
[Qemu-devel] [PATCH 15/15] virtio-serial: Handle scatter/gather input from the guest
Current guests don't send more than one iov but it can change later.
Ensure we handle that case.
Signed-off-by: Amit Shah amit.s...@redhat.com
CC: Avi Kivity a...@redhat.com
---
hw/virtio-serial-bus.c | 12
1 files changed, 8 insertions(+), 4 deletions(-)
diff --git a/hw/virtio-serial-bus.c b/hw/virtio-serial-bus.c
index 3edfeca..42e4ed0 100644
--- a/hw/virtio-serial-bus.c
+++ b/hw/virtio-serial-bus.c
@@ -369,7 +369,8 @@ static void handle_output(VirtIODevice *vdev, VirtQueue *vq)
while (virtqueue_pop(vq, elem)) {
VirtIOSerialPort *port;
-size_t ret;
+uint8_t *buf;
+size_t ret, buf_size;
port = find_port_by_vq(vser, vq);
if (!port) {
@@ -392,9 +393,12 @@ static void handle_output(VirtIODevice *vdev, VirtQueue
*vq)
goto next_buf;
}
-/* The guest always sends only one sg */
-ret = port-info-have_data(port, elem.out_sg[0].iov_base,
-elem.out_sg[0].iov_len);
+buf_size = iov_size(elem.out_sg, elem.out_num);
+buf = qemu_malloc(buf_size);
+ret = iov_to_buf(elem.out_sg, elem.out_num, buf, 0, buf_size);
+
+ret = port-info-have_data(port, buf, ret);
+qemu_free(buf);
next_buf:
virtqueue_push(vq, elem, ret);
--
1.6.2.5
