[Qemu-devel] [PULL] virtio-serial: use bh for unthrottling
The following changes since commit 0225e254ae81c5638463cda8f5730f31619113b6: usb-linux: Add missing break statement (2011-05-09 16:18:32 +0200) are available in the git repository at: git://git.kernel.org/pub/scm/virt/qemu/amit/virtio-serial.git for-anthony Alon Levy (1): virtio-serial-bus: use bh for unthrottling hw/virtio-serial-bus.c | 12 ++-- hw/virtio-serial.h |5 + 2 files changed, 15 insertions(+), 2 deletions(-) -- 1.7.4.4
[Qemu-devel] [PATCH 1/1] virtio-serial-bus: use bh for unthrottling
From: Alon Levy al...@redhat.com
Instead of calling flush_queued_data when unthrottling, schedule
a bh. That way we can return immediately to the caller, and the
flush uses the same call path as a have_data for callbackee.
No migration change is required because bh are called from vm_stop.
Signed-off-by: Alon Levy al...@redhat.com
Signed-off-by: Amit Shah amit.s...@redhat.com
---
hw/virtio-serial-bus.c | 12 ++--
hw/virtio-serial.h |5 +
2 files changed, 15 insertions(+), 2 deletions(-)
diff --git a/hw/virtio-serial-bus.c b/hw/virtio-serial-bus.c
index f10d48f..ca0581b 100644
--- a/hw/virtio-serial-bus.c
+++ b/hw/virtio-serial-bus.c
@@ -285,6 +285,13 @@ size_t virtio_serial_guest_ready(VirtIOSerialPort *port)
return 0;
}
+static void flush_queued_data_bh(void *opaque)
+{
+VirtIOSerialPort *port = opaque;
+
+flush_queued_data(port);
+}
+
void virtio_serial_throttle_port(VirtIOSerialPort *port, bool throttle)
{
if (!port) {
@@ -295,8 +302,7 @@ void virtio_serial_throttle_port(VirtIOSerialPort *port,
bool throttle)
if (throttle) {
return;
}
-
-flush_queued_data(port);
+qemu_bh_schedule(port-bh);
}
/* Guest wants to notify us of some event */
@@ -726,6 +732,7 @@ static int virtser_port_qdev_init(DeviceState *qdev,
DeviceInfo *base)
bool plugging_port0;
port-vser = bus-vser;
+port-bh = qemu_bh_new(flush_queued_data_bh, port);
/*
* Is the first console port we're seeing? If so, put it up at
@@ -792,6 +799,7 @@ static int virtser_port_qdev_exit(DeviceState *qdev)
VirtIOSerialPort *port = DO_UPCAST(VirtIOSerialPort, dev, qdev);
VirtIOSerial *vser = port-vser;
+qemu_bh_delete(port-bh);
remove_port(port-vser, port-id);
QTAILQ_REMOVE(vser-ports, port, next);
diff --git a/hw/virtio-serial.h b/hw/virtio-serial.h
index 5eb948e..0fa03d1 100644
--- a/hw/virtio-serial.h
+++ b/hw/virtio-serial.h
@@ -119,6 +119,11 @@ struct VirtIOSerialPort {
uint32_t iov_idx;
uint64_t iov_offset;
+/*
+ * When unthrottling we use a buttomhalf to call flush_queued_data.
+ */
+QEMUBH *bh;
+
/* Identify if this is a port that binds with hvc in the guest */
uint8_t is_console;
--
1.7.4.4
[Qemu-devel] [PatchV2] s390x: fix memory detection for guests 64GB
the s390 memory detection has a 16bit field that specifies the amount of
increments. This patch adopts the memory size to always fit into that
scheme. This also fixes virtio detection for these guests, since the
descriptor page is located after the main memory.
Signed-off-by: Christian Borntraeger borntrae...@de.ibm.com
---
target-s390x/op_helper.c |8 ++--
vl.c | 11 +++
2 files changed, 17 insertions(+), 2 deletions(-)
Index: b/target-s390x/op_helper.c
===
--- a/target-s390x/op_helper.c
+++ b/target-s390x/op_helper.c
@@ -2361,6 +2361,7 @@ static void ext_interrupt(CPUState *env,
int sclp_service_call(CPUState *env, uint32_t sccb, uint64_t code)
{
int r = 0;
+int shift = 0;
#ifdef DEBUG_HELPER
printf(sclp(0x%x, 0x% PRIx64 )\n, sccb, code);
@@ -2375,8 +2376,11 @@ int sclp_service_call(CPUState *env, uin
switch(code) {
case SCLP_CMDW_READ_SCP_INFO:
case SCLP_CMDW_READ_SCP_INFO_FORCED:
-stw_phys(sccb + SCP_MEM_CODE, ram_size 20);
-stb_phys(sccb + SCP_INCREMENT, 1);
+while ((ram_size (20 + shift)) 65535) {
+shift++;
+}
+stw_phys(sccb + SCP_MEM_CODE, ram_size (20 + shift));
+stb_phys(sccb + SCP_INCREMENT, 1 shift);
stw_phys(sccb + SCP_RESPONSE_CODE, 0x10);
if (kvm_enabled()) {
Index: b/vl.c
===
--- a/vl.c
+++ b/vl.c
@@ -2962,6 +2962,17 @@ int main(int argc, char **argv, char **e
if (ram_size == 0)
ram_size = DEFAULT_RAM_SIZE * 1024 * 1024;
+/* s390x ram size detection needs a 16bit multiplier + an increment. So
+ guests 64GB can be specified in 2MB steps etc */
+if (strstr(machine-name, s390)) {
+int shift = 0;
+
+while ((ram_size (20 + shift)) 65535) {
+shift++;
+}
+ram_size = ram_size (20 + shift) (20 + shift);
+}
+
/* init the dynamic translator */
cpu_exec_init_all(tb_size * 1024 * 1024);
Re: [Qemu-devel] [PatchV2] s390x: fix memory detection for guests 64GB
On 12.05.2011, at 09:50, Christian Borntraeger wrote:
the s390 memory detection has a 16bit field that specifies the amount of
increments. This patch adopts the memory size to always fit into that
scheme. This also fixes virtio detection for these guests, since the
descriptor page is located after the main memory.
Signed-off-by: Christian Borntraeger borntrae...@de.ibm.com
---
target-s390x/op_helper.c |8 ++--
vl.c | 11 +++
2 files changed, 17 insertions(+), 2 deletions(-)
Index: b/target-s390x/op_helper.c
===
--- a/target-s390x/op_helper.c
+++ b/target-s390x/op_helper.c
@@ -2361,6 +2361,7 @@ static void ext_interrupt(CPUState *env,
int sclp_service_call(CPUState *env, uint32_t sccb, uint64_t code)
{
int r = 0;
+int shift = 0;
#ifdef DEBUG_HELPER
printf(sclp(0x%x, 0x% PRIx64 )\n, sccb, code);
@@ -2375,8 +2376,11 @@ int sclp_service_call(CPUState *env, uin
switch(code) {
case SCLP_CMDW_READ_SCP_INFO:
case SCLP_CMDW_READ_SCP_INFO_FORCED:
-stw_phys(sccb + SCP_MEM_CODE, ram_size 20);
-stb_phys(sccb + SCP_INCREMENT, 1);
+while ((ram_size (20 + shift)) 65535) {
+shift++;
+}
+stw_phys(sccb + SCP_MEM_CODE, ram_size (20 + shift));
+stb_phys(sccb + SCP_INCREMENT, 1 shift);
stw_phys(sccb + SCP_RESPONSE_CODE, 0x10);
if (kvm_enabled()) {
Index: b/vl.c
===
--- a/vl.c
+++ b/vl.c
@@ -2962,6 +2962,17 @@ int main(int argc, char **argv, char **e
if (ram_size == 0)
ram_size = DEFAULT_RAM_SIZE * 1024 * 1024;
+/* s390x ram size detection needs a 16bit multiplier + an increment. So
+ guests 64GB can be specified in 2MB steps etc */
+if (strstr(machine-name, s390)) {
+int shift = 0;
+
+while ((ram_size (20 + shift)) 65535) {
+shift++;
+}
+ram_size = ram_size (20 + shift) (20 + shift);
This one really belongs to hw/s390-virtio.c. Just move the same code to
s390_init there and it should work out. The less hacks we can have in generic
code, the better :).
Alex
Re: [Qemu-devel] [PATCH 1/2] coroutine: introduce coroutines
Am 11.05.2011 21:12, schrieb Stefan Weil:
Am 11.05.2011 12:15, schrieb Stefan Hajnoczi:
From: Kevin Wolf kw...@redhat.com
Asynchronous code is becoming very complex. At the same time
synchronous code is growing because it is convenient to write.
Sometimes duplicate code paths are even added, one synchronous and the
other asynchronous. This patch introduces coroutines which allow code
that looks synchronous but is asynchronous under the covers.
A coroutine has its own stack and is therefore able to preserve state
across blocking operations, which traditionally require callback
functions and manual marshalling of parameters.
Creating and starting a coroutine is easy:
coroutine = qemu_coroutine_create(my_coroutine);
qemu_coroutine_enter(coroutine, my_data);
The coroutine then executes until it returns or yields:
void coroutine_fn my_coroutine(void *opaque) {
MyData *my_data = opaque;
/* do some work */
qemu_coroutine_yield();
/* do some more work */
}
Yielding switches control back to the caller of qemu_coroutine_enter().
This is typically used to switch back to the main thread's event loop
after issuing an asynchronous I/O request. The request callback will
then invoke qemu_coroutine_enter() once more to switch back to the
coroutine.
Note that coroutines never execute concurrently and should only be used
from threads which hold the global mutex. This restriction makes
programming with coroutines easier than with threads. Race conditions
cannot occur since only one coroutine may be active at any time. Other
coroutines can only run across yield.
This coroutines implementation is based on the gtk-vnc implementation
written by Anthony Liguori anth...@codemonkey.ws but it has been
significantly rewritten by Kevin Wolf kw...@redhat.com to use
setjmp()/longjmp() instead of the more expensive swapcontext().
Signed-off-by: Kevin Wolf kw...@redhat.com
Signed-off-by: Stefan Hajnoczi stefa...@linux.vnet.ibm.com
---
Hi Stefan,
you might want to add the following or a similar patch:
diff --git a/qemu-coroutine.c b/qemu-coroutine.c
index 0927f58..9cd0dd7 100644
--- a/qemu-coroutine.c
+++ b/qemu-coroutine.c
@@ -91,7 +91,10 @@ static void *coroutine_swap(Coroutine *from,
Coroutine *to, void *opaque)
case COROUTINE_TERMINATE:
current = to-caller;
qemu_coroutine_terminate(to);
-return to-data;
+opaque = to-data;
+qemu_free(to-stack);
+qemu_free(to);
+return opaque;
default:
/* Switch to called coroutine */
current = to;
No, this is wrong. qemu_coroutine_terminate() puts the coroutine back
into a pool, so you can't free it.
I tested your test code with Valgrind. Beside of the memory leaks which
are fixed
with the small modification shown above, Valgrind has a lot of complains.
Hm, for me Valgrind doesn't show any memory leaks, only some
uninitialised value of size 8 in the trampoline. I don't quite
understand these.
Maybe you can try it yourself, otherwise please wait until I have finished
analyzing the Valgrind results. At a first glance, I'm afraid that
debugging with gdb or Valgrind might become more difficult when coroutines
are used. This is different with threads: they are fully supported by gdb.
Yes, the debugging support is less than optimal. However, threads are
not the right comparison. Coroutines are meant to replace AIOCBs and
callbacks, and these are already hard to debug today. So I don't think
it becomes more difficult.
Kevin
Re: [Qemu-devel] [PatchV2] s390x: fix memory detection for guests 64GB
On 12/05/11 09:55, Alexander Graf wrote:
+/* s390x ram size detection needs a 16bit multiplier + an increment. So
+ guests 64GB can be specified in 2MB steps etc */
+if (strstr(machine-name, s390)) {
+int shift = 0;
+
+while ((ram_size (20 + shift)) 65535) {
+shift++;
+}
+ram_size = ram_size (20 + shift) (20 + shift);
This one really belongs to hw/s390-virtio.c. Just move the same code to
s390_init there and it should work out. The less hacks we can have in generic
code, the better :).
Unfortunately this does not work. The slcp op helper uses the global variable
ram_size fro vl.c. I could change the global ram_size in s390_init, but this
is also pretty ugly.
Christian
Re: [Qemu-devel] [PatchV2] s390x: fix memory detection for guests 64GB
On 12.05.2011, at 09:59, Christian Borntraeger wrote:
On 12/05/11 09:55, Alexander Graf wrote:
+/* s390x ram size detection needs a 16bit multiplier + an increment. So
+ guests 64GB can be specified in 2MB steps etc */
+if (strstr(machine-name, s390)) {
+int shift = 0;
+
+while ((ram_size (20 + shift)) 65535) {
+shift++;
+}
+ram_size = ram_size (20 + shift) (20 + shift);
This one really belongs to hw/s390-virtio.c. Just move the same code to
s390_init there and it should work out. The less hacks we can have in
generic code, the better :).
Unfortunately this does not work. The slcp op helper uses the global variable
ram_size fro vl.c. I could change the global ram_size in s390_init, but this
is also pretty ugly.
I'd definitely prefer that over the change on vl.c :)
Alex
Re: [Qemu-devel] [PATCH] Add warmup phase for live migration of large memory apps
On Wed, May 11, 2011 at 8:58 AM, Shribman, Aidan
aidan.shrib...@sap.com wrote:
From: Aidan Shribman aidan.shrib...@sap.com
[PATCH] Add warmup phase for live migration of large memory apps
By invoking migrate -w url we initiate a background
live-migration
transferring of dirty pages continuously until invocation
of migrate_end
which attempts to complete the live migration operation.
What is the purpose of this patch? How and when do I use it?
The warmup patch adds none-converging background update of guest memory during
live-migration such that on request of live-migration completion (via
migrate_end command) we get much faster response. This is especially needed
when running a payload of large enterprise applications which have high memory
demands.
Some nitpicks:
@@ -81,6 +83,11 @@ int do_migrate(Monitor *mon, const QDict
*qdict, QObject **ret_data)
int blk = qdict_get_try_bool(qdict, blk, 0);
int inc = qdict_get_try_bool(qdict, inc, 0);
const char *uri = qdict_get_str(qdict, uri);
+ is_migrate_warmup = qdict_get_try_bool(qdict, warmup, 0);
+
+ if (is_migrate_warmup) {
+ detach = 1; /* as we need migrate_end to complte */
s/complte/complete/
+ }
Please follow the coding style and put the closing curly brace on the
same column as the 'if' statement.
+int qemu_savevm_state_warmup(Monitor *mon, QEMUFile *f) {
+ int ret = 1;
1 is overwritten immediately.
A new patch follows with corrections:
---
From: Aidan Shribman aidan.shrib...@sap.com
Subject: Add warmup phase for live migration of large memory apps
By invoking migrate -w url we initiate a background live-migration
transferring of dirty pages continuously until invocation of migrate_end
which attempts to complete the live migration operation.
Qemu host: Ubuntu 10.10
Testing: live migration (with/without warmup phase) tested successfully.
Signed-off-by: Benoit Hudzia benoit.hud...@sap.com
Signed-off-by: Petter Svard pett...@cs.umu.se
Signed-off-by: Aidan Shribman aidan.shrib...@sap.com
---
hmp-commands.hx | 33 +
migration.c | 23 ++-
migration.h |2 ++
qmp-commands.hx | 41 ++---
savevm.c| 11 +++
sysemu.h|1 +
6 files changed, 95 insertions(+), 16 deletions(-)
diff --git a/hmp-commands.hx b/hmp-commands.hx
index e5585ba..215ea41 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -717,24 +717,28 @@ ETEXI
{
.name = migrate,
-.args_type = detach:-d,blk:-b,inc:-i,uri:s,
-.params = [-d] [-b] [-i] uri,
-.help = migrate to URI (using -d to not wait for completion)
- \n\t\t\t -b for migration without shared storage with
- full copy of disk\n\t\t\t -i for migration without
- shared storage with incremental copy of disk
- (base image shared between src and destination),
+.args_type = detach:-d,blk:-b,inc:-i,warmup:-w,uri:s,
+.params = [-d] [-b] [-i] [-w] uri,
+.help = migrate to URI
+ \n\t -d to not wait for completion
+ \n\t -b for migration without shared storage with
+ full copy of disk
+ \n\t -i for migration without
+ shared storage with incremental copy of disk
+ (base image shared between source and destination)
+ \n\t -w to enter warmup phase,
.user_print = monitor_user_noop,
.mhandler.cmd_new = do_migrate,
},
STEXI
-@item migrate [-d] [-b] [-i] @var{uri}
+@item migrate [-d] [-b] [-i] [-w] @var{uri}
@findex migrate
Migrate to @var{uri} (using -d to not wait for completion).
-b for migration with full copy of disk
-i for migration with incremental copy of disk (base image is shared)
+-w to enter warmup phase
ETEXI
{
@@ -753,6 +757,19 @@ Cancel the current VM migration.
ETEXI
{
+.name = migrate_end,
+.args_type = ,
+.params = ,
+.help = Complete warmup and move to full live migration,
+.mhandler.cmd = do_migrate_end,
+},
+
+STEXI
+@item migrate_end
+Complete warmup and move to full live migration.
+ETEXI
+
+{
.name = migrate_set_speed,
.args_type = value:o,
.params = value,
diff --git a/migration.c b/migration.c
index 9ee8b17..c178a77 100644
--- a/migration.c
+++ b/migration.c
@@ -31,6 +31,8 @@
do { } while (0)
#endif
+static int is_migrate_warmup = 0;
+
/* Migration speed throttling */
static uint32_t max_throttle = (32 20);
@@ -81,6 +83,11 @@ int do_migrate(Monitor *mon, const QDict *qdict, QObject
**ret_data)
int blk = qdict_get_try_bool(qdict, blk, 0);
int inc = qdict_get_try_bool(qdict,
Re: [Qemu-devel] [PatchV3] s390x: fix memory detection for guests 64GB
I'd definitely prefer that over the change on vl.c :)
the s390 memory detection has a 16bit field that specifies the amount of
increments. This patch adopts the memory size to always fit into that
scheme. This also fixes virtio detection for these guests, since the
descriptor page is located after the main memory.
Signed-off-by: Christian Borntraeger borntrae...@de.ibm.com
---
hw/s390-virtio.c | 20 +++-
target-s390x/op_helper.c |8 ++--
2 files changed, 21 insertions(+), 7 deletions(-)
Index: b/hw/s390-virtio.c
===
--- a/hw/s390-virtio.c
+++ b/hw/s390-virtio.c
@@ -133,7 +133,7 @@ int s390_virtio_hypercall(CPUState *env,
}
/* PC hardware initialisation */
-static void s390_init(ram_addr_t ram_size,
+static void s390_init(ram_addr_t my_ram_size,
const char *boot_device,
const char *kernel_filename,
const char *kernel_cmdline,
@@ -145,19 +145,29 @@ static void s390_init(ram_addr_t ram_siz
ram_addr_t kernel_size = 0;
ram_addr_t initrd_offset;
ram_addr_t initrd_size = 0;
+int shift = 0;
uint8_t *storage_keys;
int i;
+/* s390x ram size detection needs a 16bit multiplier + an increment. So
+ guests 64GB can be specified in 2MB steps etc. */
+while ((my_ram_size (20 + shift)) 65535) {
+shift++;
+}
+my_ram_size = my_ram_size (20 + shift) (20 + shift);
+
+/* lets propagate the changed ram size into the global variable. */
+ram_size = my_ram_size;
/* get a BUS */
-s390_bus = s390_virtio_bus_init(ram_size);
+s390_bus = s390_virtio_bus_init(my_ram_size);
/* allocate RAM */
-ram_addr = qemu_ram_alloc(NULL, s390.ram, ram_size);
-cpu_register_physical_memory(0, ram_size, ram_addr);
+ram_addr = qemu_ram_alloc(NULL, s390.ram, my_ram_size);
+cpu_register_physical_memory(0, my_ram_size, ram_addr);
/* allocate storage keys */
-storage_keys = qemu_mallocz(ram_size / TARGET_PAGE_SIZE);
+storage_keys = qemu_mallocz(my_ram_size / TARGET_PAGE_SIZE);
/* init CPUs */
if (cpu_model == NULL) {
Index: b/target-s390x/op_helper.c
===
--- a/target-s390x/op_helper.c
+++ b/target-s390x/op_helper.c
@@ -2361,6 +2361,7 @@ static void ext_interrupt(CPUState *env,
int sclp_service_call(CPUState *env, uint32_t sccb, uint64_t code)
{
int r = 0;
+int shift = 0;
#ifdef DEBUG_HELPER
printf(sclp(0x%x, 0x% PRIx64 )\n, sccb, code);
@@ -2375,8 +2376,11 @@ int sclp_service_call(CPUState *env, uin
switch(code) {
case SCLP_CMDW_READ_SCP_INFO:
case SCLP_CMDW_READ_SCP_INFO_FORCED:
-stw_phys(sccb + SCP_MEM_CODE, ram_size 20);
-stb_phys(sccb + SCP_INCREMENT, 1);
+while ((ram_size (20 + shift)) 65535) {
+shift++;
+}
+stw_phys(sccb + SCP_MEM_CODE, ram_size (20 + shift));
+stb_phys(sccb + SCP_INCREMENT, 1 shift);
stw_phys(sccb + SCP_RESPONSE_CODE, 0x10);
if (kvm_enabled()) {
Re: [Qemu-devel] [PATCH] Add an isa device for SGA
Glauber Costa glom...@redhat.com writes:
This patch adds a dummy legacy ISA device whose responsibility is to
deploy sgabios, an option rom for a serial graphics adapter.
The proposal is that this device is always-on when -nographics,
but can otherwise be enable in any setup when -device sga is used.
Signed-off-by: Glauber Costa glom...@redhat.com
---
Makefile.target |2 +-
hw/pc.c |2 ++
hw/pc.h |3 +++
hw/sga.c| 49 +
4 files changed, 55 insertions(+), 1 deletions(-)
create mode 100644 hw/sga.c
diff --git a/Makefile.target b/Makefile.target
index fdbdc6c..004ea7e 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -224,7 +224,7 @@ obj-$(CONFIG_KVM) += ivshmem.o
# Hardware support
obj-i386-y += vga.o
obj-i386-y += mc146818rtc.o i8259.o pc.o
-obj-i386-y += cirrus_vga.o apic.o ioapic.o piix_pci.o
+obj-i386-y += cirrus_vga.o sga.o apic.o ioapic.o piix_pci.o
obj-i386-y += vmport.o
obj-i386-y += device-hotplug.o pci-hotplug.o smbios.o wdt_ib700.o
obj-i386-y += extboot.o
diff --git a/hw/pc.c b/hw/pc.c
index 8d351ba..56c3887 100644
--- a/hw/pc.c
+++ b/hw/pc.c
@@ -1096,6 +1096,8 @@ void pc_vga_init(PCIBus *pci_bus)
isa_vga_init();
}
}
+
+isa_sga_init();
Please do this the qdev way (untested):
if (display_type == DT_NOGRAPHIC) {
dev = qdev_create(NULL, sga);
qdev_init_nofail(dev);
}
}
static void cpu_request_exit(void *opaque, int irq, int level)
diff --git a/hw/pc.h b/hw/pc.h
index 1291e2d..a00e054 100644
--- a/hw/pc.h
+++ b/hw/pc.h
@@ -219,6 +219,9 @@ int isa_vga_mm_init(target_phys_addr_t vram_base,
void pci_cirrus_vga_init(PCIBus *bus);
void isa_cirrus_vga_init(void);
+/* serial graphics */
+void isa_sga_init(void);
+
/* ne2000.c */
static inline bool isa_ne2000_init(int base, int irq, NICInfo *nd)
{
diff --git a/hw/sga.c b/hw/sga.c
new file mode 100644
index 000..411191b
--- /dev/null
+++ b/hw/sga.c
@@ -0,0 +1,49 @@
+#include pci.h
+#include pc.h
+#include loader.h
+#include sysemu.h
+
+#define SGABIOS_FILENAME sgabios.bin
+
+typedef struct ISAGAState {
+ISADevice dev;
+} ISASGAState;
+
+/* We can have both -device, and the initfn called, so better
+ * avoid to have the rom loaded twice */
+static void deploy_rom(void)
+{
+static int rom_deployed = 0;
+
+if (!rom_deployed++) {
+rom_add_vga(SGABIOS_FILENAME);
+}
+}
Is rom_deployed still needed with isa_sga_init() gone?
+
+static int isa_cirrus_vga_initfn(ISADevice *dev)
+{
+deploy_rom();
+
+return 0;
+}
+
+void isa_sga_init(void)
+{
+if (display_type == DT_NOGRAPHIC) {
+deploy_rom();
+}
+}
+
+static ISADeviceInfo sga_info = {
+.qdev.name= sga,
+.qdev.desc= Serial Graphics Adapter,
+.qdev.size= sizeof(ISASGAState),
+.init = isa_cirrus_vga_initfn,
+};
+
+static void sga_register(void)
+{
+ isa_qdev_register(sga_info);
+}
+
+device_init(sga_register);
Re: [Qemu-devel] [PATCH 1/2] usb-linux: fix device path aka physical port handling
On 05/11/11 10:52, Markus Armbruster wrote:
Good stuff, just a few questions.
Gerd Hoffmannkra...@redhat.com writes:
The device path isn't just a number. It specifies the physical port
the device is connected to and in case the device is connected via
usb hub you'll have two numbers there, like this: 5.1. The first
specifies the root port where the hub is plugged into, the second
specifies the port number of the hub where the device is plugged in.
With multiple hubs chained the string can become longer.
5.1? Do you mean 5-1?
No.
snprintf(dev-devpath, sizeof dev-devpath,
%s.%d, parent-devpath, port1);
^
$ ls /sys/bus/usb/devices
1-0:1.0 2-0:1.0 3-0:1.0 4-0:1.0 5-0:1.0 usb1 usb2 usb3 usb4 usb5
Boring, nothing plugged in here ;)
Laptop docking stations often have a usb hub built in. If you have one
place your laptop there, then connect a usb device to one of the ports
of the docking station. You'll see a file named like this:
1-5.3
where 1 is the bus number, 5 is the port number of the root hub and
3 is the port number of the docking station hub.
@@ -79,6 +79,7 @@ typedef int USBScanFunc(void *opaque, int bus_num, int addr,
int devpath,
#define USBPROCBUS_PATH /proc/bus/usb
#define PRODUCT_NAME_SZ 32
#define MAX_ENDPOINTS 15
+#define MAX_PORTLEN 8
Where does 8 come from?
Pulled out of thin air. Should be enougth, you can build chains with
three usb hubs (anyone ever did in practice? did the devices still
work?), getting port addresses like 1.2.3.4, and it still fits in.
For what it's worth, kernel's struct usb_device
has char devpath[16].
We can make that 16 too to be on the really safe side.
@@ -1672,8 +1674,8 @@ static int usb_host_scan_sys(void *opaque, USBScanFunc
*func)
if (!strncmp(de-d_name, usb, 3)) {
tmpstr += 3;
}
Sloppy parsing, but that's not your fault.
I think this can be zapped now, the new sscanf will fail on them and
skip the entries anyway.
-if (sscanf(tmpstr, %d-%d,bus_num,devpath) 1) {
-goto the_end;
+if (sscanf(tmpstr, %d-%7[0-9.],bus_num, port) 2) {
+continue;
}
if (!usb_host_read_file(line, sizeof(line), devnum,
de-d_name)) {
The old sscanf() succeeds if at least one item is assigned, i.e. tmpstr
starts with an integer. I suspect this is broken for roots. Consider
d_name usb1: tmpstr is 1, sscan() returns 1, and devpath remains
uninitialized. It's passed to the func() callback. Bug? If yes, the
commit message should mention it.
Indeed.
The new sscan() succeeds only if both items are assigned, i.e. tmpstr
starts with an integer, '-', and up to 7 of [0-9.]. Unlike the old one,
it fails for roots. Intentional?
Yes, now the roots are skipped. You can't assign them anyway, so it is
pretty pointless to loom at them and to list them in info usbhost.
cheers,
Gerd
[Qemu-devel] qdev device documentation (Re: [PATCH 0/2] usb-linux: physical port handling.)
Hi, Some additional docs (based on 2/2 hints) might be a useful addition. Indeed. Possible candidates: - docs/qdev-device-use.txt - qemu-doc.texi Hmm. qemu-doc.texi only documents the pre-qdev way to create devices. qdev-device-use.txt is a conversion guide. devices which can only be created via -device (or provide additional features via qdev attributes when created that way) are not documented anywhere now. Suggestions? Should we add a qdev section to qemu-doc.texi? Or better a separate qemu-devices.txt (or .texi)? What is the status of the qdev documentation patches btw.? Maybe we should just merge them, then start filling stuff and maybe also autogenerate documentation from that? cheers, Gerd
Re: [Qemu-devel] Bug #757654: UHCI fails to signal stall response patch
On 05/11/11 13:33, Jan Vesely wrote: glad I could help the original bug report is here: https://bugs.launchpad.net/qemu/+bug/757654 should I update it? or will it be updated when the patch reaches master? Better wait until the stuff hits master, that is less confusing and you can also add the git commit hash then. cheers, Gerd
Re: [Qemu-devel] adding search to dhcp
Michael Tokarev m...@tls.msk.ru writes: 12.05.2011 00:49, Jan Kiszka пишет: On 2011-05-11 18:08, Stefan Hajnoczi wrote: On Wed, May 11, 2011 at 4:22 PM, Carl Karsten c...@personnelware.com wrote: On Wed, May 11, 2011 at 6:01 AM, Markus Armbruster arm...@redhat.com wrote: Stefan Hajnoczi stefa...@gmail.com writes: On Tue, May 10, 2011 at 6:40 PM, Carl Karsten c...@personnelware.com wrote: I would expect the syntax to look like this: qemu -hda 1.qcow2 -net nick -net user,hostname=qemu,search=example.com,sales.example.com Comma escaping is needed but it seems like a reasonable feature to me. Comma escaping is ugly: -net user,hostname=qemu,search=example.com,,sales.example.com Could we have multiple search options instead? Like this: -net user,hostname=qemu,search=example.com,search=sales.example.com How about: -net user,hostname=qemu,search=example.com,sales.example.com That does not work the way you'd expect: $ echo asdf=asdf,ok=this,is,a,test asdf=asdf,ok=this,is,a,test Also, let's not get into the business of matching quotes and passing them escaped on the shell. That's just as ugly as escaping commas and more work. I think the two options are using QEMU's typical comma escaping ',,' or specifying the option multiple times. I'd go with comma escaping for consistency. I'm not aware of any other option in QEMU that is specified multiple times. -net user,hostfwd=...,hostfwd=... Let's got for multiple specification, ',,' is just ugly IMHO. I second this, just repeat the specification, please no double ,,. Or alternatively, search1=foo,search2=bar, but this is also sort of ugly. But I'm not sure why there's no way to use some other character, like colon (:) for example - it's used for protocol:details already, and for domain names it works well too... Too clever, in my opinion. Breaks down when option values may contain ':'. While that's not the case for domain names, it still means it's a special-purpose syntactic hack. Let's follow the hostfwd precedence and permit multiple search options.
Re: [Qemu-devel] [PATCH v4 5/5] hpet 'driftfix': add code in hpet_timer() to compensate delayed callbacks and coalesced interrupts
Hi Zachary,
1. re.:
+static void hpet_timer_driftfix_reset(HPETTimer *t)
+{
+if (t-state-driftfix timer_is_periodic(t)) {
+t-ticks_not_accounted = t-prev_period = t-period;
This is rather confusing. Clearly, ticks_not_accounted isn't actually
ticks not accounted, it's a different variable entirely which is based
on the current period.
If it were actually ticks_not_accounted, it should be reset to zero.
hpet_timer_driftfix_reset() is called at two sites before the periodic
timer is actually started via hpet_set_timer(). An interrupt shall be
delivered at the first callback of hpet_timer(), after t-period amount
of time has passed. The ticks are recorded as 'not accounted' until the
interrupt is delivered to the guest. The following message explains why
t-prev_period is needed in addition to t-period and how it is used.
http://lists.gnu.org/archive/html/qemu-devel/2011-05/msg00275.html
On entry to hpet_timer(), t-ticks_not_accounted is = t-prev_period,
and is it increased by t-period while the comparator register is being
advanced. This is also the place where we can detect whether the current
callback was delayed. If the period_count is 1, the delay was so large
that we missed to inject an interrupt (which needs to be compensated).
while (hpet_time_after(cur_tick, t-cmp)) {
t-cmp = (uint32_t)(t-cmp + t-period);
+t-ticks_not_accounted += t-period;
+period_count++;
}
Please note that period_count can be zero. This happens during callbacks
that inject additional interrupts 'inside of' a period interval in order
to compensate missed and coalesced interrupts.
t-ticks_not_accounted is decreased only if an interrupt was delivered
to the guest. If an interrupt could not be delivered, the ticks that are
represented by that interrupt remain recorded as 'not accounted' (this
also triggers compensation of coalesced interrupts).
+if (irq_delivered) {
+t-ticks_not_accounted -= t-prev_period;
+t-prev_period = t-period;
+} else {
+if (period_count) {
+t-irq_rate++;
+t-irq_rate = MIN(t-irq_rate, MAX_IRQ_RATE);
+}
+}
2. re.:
+if (period_count) {
+t-divisor = t-irq_rate;
+}
+diff /= t-divisor--;
Why subtracting from the divisor? Shouldn't the divisor and irq_rate
change in lockstep?
t-irq_rate is the rate at which interrupts are delivered to the guest,
relative to the period length. If t-irq_rate is 1, then one interrupt
shall be injected per period interval. If t-irq_rate is 1, we are in
'compensation mode' (trying to inject additional interrupts 'inside of'
an interval).
- If t-irq_rate is 2, then two interrupts shall be injected during a
period interval (one regular and one additional).
- If t-irq_rate is 3, then three interrupts shall be injected during a
period interval (one regular and two additional).
- etc.
A non-zero period_count marks the start of an interval, at which the
divisor is set to t-irq_rate. Let's take a look at an example where
t-divisor = t-irq_rate = 3.
- The current period starts at t(p), the next period starts at t(p+1).
We are now at t(p). The first additional interrupt shall be injected
at a(1), the second at a(2). Hence, the next callback is scheduled
to occur at a(1) = t(p) + diff / 3.
t(p) a(1) a(2) t(p+1)
++++ time
--- diff ---
diff / 3
- We are now in the callback at a(1). A new diff has been calculated,
which is equal to the remaining time in the interval from a(1) to
t(p+1). The second additional interrupt shall be injected at a(2).
Hence, the next callback is scheduled to occur at a(2) = a(1) + diff / 2.
a(1) a(2) t(p+1)
+++ time
-- diff ---
diff / 2
- We are now in the callback at a(2). A new diff has been calculated,
which is equal to the remaining time in the interval from a(2) to
t(p+1). The next callback marks the beginning of period t(p+1).
a(2) t(p+1)
++ time
-- diff --
diff / 1
At t(p), the divisor is set to irq_rate (= 3). diff is divided by 3
and the divisor is decremented by one. At a(1), the divisor is 2.
diff is divided by 2 and the divisor is decremented by one. At a(2),
the divisor is 1. diff is divided by
Re: [Qemu-devel] [PATCH 1/2] coroutine: introduce coroutines
On 2011-05-11 12:15, Stefan Hajnoczi wrote:
From: Kevin Wolf kw...@redhat.com
Asynchronous code is becoming very complex. At the same time
synchronous code is growing because it is convenient to write.
Sometimes duplicate code paths are even added, one synchronous and the
other asynchronous. This patch introduces coroutines which allow code
that looks synchronous but is asynchronous under the covers.
A coroutine has its own stack and is therefore able to preserve state
across blocking operations, which traditionally require callback
functions and manual marshalling of parameters.
Creating and starting a coroutine is easy:
coroutine = qemu_coroutine_create(my_coroutine);
qemu_coroutine_enter(coroutine, my_data);
The coroutine then executes until it returns or yields:
void coroutine_fn my_coroutine(void *opaque) {
MyData *my_data = opaque;
/* do some work */
qemu_coroutine_yield();
/* do some more work */
}
Yielding switches control back to the caller of qemu_coroutine_enter().
This is typically used to switch back to the main thread's event loop
after issuing an asynchronous I/O request. The request callback will
then invoke qemu_coroutine_enter() once more to switch back to the
coroutine.
Note that coroutines never execute concurrently and should only be used
from threads which hold the global mutex. This restriction makes
programming with coroutines easier than with threads. Race conditions
cannot occur since only one coroutine may be active at any time. Other
coroutines can only run across yield.
Mmh, is there anything that conceptually prevent fixing this limitation
later on? I would really like to remove such dependency long-term as
well to have VCPUs operate truly independently on independent device models.
Jan
--
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux
[Qemu-devel] [PATCH v2 4/4] coroutine: pool coroutines to speed up creation
This patch speeds up coroutine creation by reusing freed coroutines.
When a coroutine terminates it is placed in the pool instead of having
its resources freed. The next time a coroutine is created it can be
taken straight from the pool and requires no initialization.
Performance results on an Intel Core2 Duo T9400 (2.53GHz) for
./check-coroutine --benchmark-lifecycle 2000:
No pooling:19.5 sec
With pooling: 1.1 sec
Signed-off-by: Stefan Hajnoczi stefa...@linux.vnet.ibm.com
---
check-coroutine.c|2 ++
qemu-coroutine-int.h |2 ++
qemu-coroutine.c | 49 ++---
qemu-coroutine.h |9 +
vl.c |2 ++
5 files changed, 61 insertions(+), 3 deletions(-)
diff --git a/check-coroutine.c b/check-coroutine.c
index 5a42c49..223c50c 100644
--- a/check-coroutine.c
+++ b/check-coroutine.c
@@ -218,6 +218,8 @@ int main(int argc, char **argv)
};
int i;
+qemu_coroutine_init();
+
if (argc == 3 strcmp(argv[1], --benchmark-lifecycle) == 0) {
benchmark_lifecycle(argv[2]);
return EXIT_SUCCESS;
diff --git a/qemu-coroutine-int.h b/qemu-coroutine-int.h
index 71c6ee9..b264881 100644
--- a/qemu-coroutine-int.h
+++ b/qemu-coroutine-int.h
@@ -45,6 +45,8 @@ struct Coroutine {
void *data;
CoroutineEntry *entry;
+QLIST_ENTRY(Coroutine) pool_next;
+
jmp_buf env;
};
diff --git a/qemu-coroutine.c b/qemu-coroutine.c
index 80bed14..cff5a4f 100644
--- a/qemu-coroutine.c
+++ b/qemu-coroutine.c
@@ -24,16 +24,35 @@
#include qemu-coroutine.h
#include qemu-coroutine-int.h
+enum {
+/* Maximum free pool size prevents holding too many freed coroutines */
+POOL_MAX_SIZE = 64,
+};
+
+static QLIST_HEAD(, Coroutine) pool = QLIST_HEAD_INITIALIZER(pool);
+static unsigned int pool_size;
static __thread Coroutine leader;
static __thread Coroutine *current;
-static void coroutine_terminate(Coroutine *co)
+static void coroutine_delete(Coroutine *co)
{
-trace_qemu_coroutine_terminate(co);
qemu_free(co-stack);
qemu_free(co);
}
+static void coroutine_terminate(Coroutine *co)
+{
+trace_qemu_coroutine_terminate(co);
+
+if (pool_size POOL_MAX_SIZE) {
+QLIST_INSERT_HEAD(pool, co, pool_next);
+co-caller = NULL;
+pool_size++;
+} else {
+coroutine_delete(co);
+}
+}
+
static Coroutine *coroutine_new(void)
{
const size_t stack_size = 4 20;
@@ -49,7 +68,15 @@ Coroutine *qemu_coroutine_create(CoroutineEntry *entry)
{
Coroutine *co;
-co = coroutine_new();
+co = QLIST_FIRST(pool);
+
+if (co) {
+QLIST_REMOVE(co, pool_next);
+pool_size--;
+} else {
+co = coroutine_new();
+}
+
co-entry = entry;
return co;
@@ -123,3 +150,19 @@ void *coroutine_fn qemu_coroutine_yield(void)
self-caller = NULL;
return coroutine_swap(self, to, NULL);
}
+
+static void coroutine_free_pool(void)
+{
+Coroutine *co;
+Coroutine *tmp;
+
+QLIST_FOREACH_SAFE(co, pool, pool_next, tmp) {
+coroutine_delete(co);
+}
+pool_size = 0;
+}
+
+void qemu_coroutine_init(void)
+{
+atexit(coroutine_free_pool);
+}
diff --git a/qemu-coroutine.h b/qemu-coroutine.h
index b79b4bf..0fae0a4 100644
--- a/qemu-coroutine.h
+++ b/qemu-coroutine.h
@@ -17,6 +17,15 @@
#include stdbool.h
/**
+ * Initialize coroutine implementation
+ *
+ * This function should be called when the program is starting up. It installs
+ * an atexit(3) handler which will free pooled coroutines when the program
+ * exits. This keeps valgrind output clean.
+ */
+void qemu_coroutine_init(void);
+
+/**
* Mark a function that executes in coroutine context
*
* Functions that execute in coroutine context cannot be called directly from
diff --git a/vl.c b/vl.c
index 6b9a2f6..7092c9f 100644
--- a/vl.c
+++ b/vl.c
@@ -145,6 +145,7 @@ int main(int argc, char **argv)
#include qemu-config.h
#include qemu-objects.h
#include qemu-options.h
+#include qemu-coroutine.h
#ifdef CONFIG_VIRTFS
#include fsdev/qemu-fsdev.h
#endif
@@ -1975,6 +1976,7 @@ int main(int argc, char **argv, char **envp)
init_clocks();
qemu_cache_utils_init(envp);
+qemu_coroutine_init();
QLIST_INIT (vm_change_state_head);
os_setup_early_signal_handling();
--
1.7.4.4
[Qemu-devel] [PATCH v2 0/4] Coroutines for better asynchronous programming
QEMU is event-driven and suffers when blocking operations are performed because VM execution may be stopped until the operation completes. Therefore many operations that could block are performed asynchronously and a callback is invoked when the operation has completed. This allows QEMU to continue executing while the operation is pending. The downside to callbacks is that they split up code into many smaller functions, each of which is a single step in a state machine that quickly becomes complex and hard to understand. Callback functions also result in lots of noise as variables are packed and unpacked into temporary structs that pass state to the callback function. This patch series introduces coroutines as a solution for writing asynchronous code while still having a nice sequential control flow. The semantics are explained in the first patch. The second patch adds automated tests. A nice feature of coroutines is that it is relatively easy to take synchronous code and lift it into a coroutine to make it asynchronous. Work has been done to move qcow2 request processing into coroutines and thereby make it asynchronous (today qcow2 will perform synchronous metadata accesses). This qcow2 work is still ongoing and not quite ready for mainline yet. Coroutines are also being used for virtfs (virtio-9p) so I have submitted this patch now because virtfs patches that depend on coroutines will follow shortly. Other areas of QEMU that could take advantage of coroutines include the VNC server, migration, and qemu-tools. v2: * Added ./check-coroutine --lifecycle-benchmark for performance measurement * Split pooling into a separate patch with performance justification * Set maximum pool size to prevent holding onto too many free coroutines * Added atexit(3) handler to free pool * Coding style cleanups Makefile |3 +- Makefile.objs|7 ++ check-coroutine.c| 238 ++ coroutine-ucontext.c | 73 +++ coroutine-win32.c| 57 qemu-coroutine-int.h | 55 qemu-coroutine.c | 168 +++ qemu-coroutine.h | 91 +++ trace-events |5 + vl.c |2 + 10 files changed, 698 insertions(+), 1 deletions(-)
[Qemu-devel] [PATCH v2 1/4] coroutine: introduce coroutines
From: Kevin Wolf kw...@redhat.com
Asynchronous code is becoming very complex. At the same time
synchronous code is growing because it is convenient to write.
Sometimes duplicate code paths are even added, one synchronous and the
other asynchronous. This patch introduces coroutines which allow code
that looks synchronous but is asynchronous under the covers.
A coroutine has its own stack and is therefore able to preserve state
across blocking operations, which traditionally require callback
functions and manual marshalling of parameters.
Creating and starting a coroutine is easy:
coroutine = qemu_coroutine_create(my_coroutine);
qemu_coroutine_enter(coroutine, my_data);
The coroutine then executes until it returns or yields:
void coroutine_fn my_coroutine(void *opaque) {
MyData *my_data = opaque;
/* do some work */
qemu_coroutine_yield();
/* do some more work */
}
Yielding switches control back to the caller of qemu_coroutine_enter().
This is typically used to switch back to the main thread's event loop
after issuing an asynchronous I/O request. The request callback will
then invoke qemu_coroutine_enter() once more to switch back to the
coroutine.
Note that coroutines never execute concurrently and should only be used
from threads which hold the global mutex. This restriction makes
programming with coroutines easier than with threads. Race conditions
cannot occur since only one coroutine may be active at any time. Other
coroutines can only run across yield.
This coroutines implementation is based on the gtk-vnc implementation
written by Anthony Liguori anth...@codemonkey.ws but it has been
significantly rewritten by Kevin Wolf kw...@redhat.com to use
setjmp()/longjmp() instead of the more expensive swapcontext().
Signed-off-by: Kevin Wolf kw...@redhat.com
Signed-off-by: Stefan Hajnoczi stefa...@linux.vnet.ibm.com
---
Makefile.objs|7 +++
coroutine-ucontext.c | 73 +
coroutine-win32.c| 57 +++
qemu-coroutine-int.h | 53 +
qemu-coroutine.c | 125 ++
qemu-coroutine.h | 82 +
trace-events |5 ++
7 files changed, 402 insertions(+), 0 deletions(-)
create mode 100644 coroutine-ucontext.c
create mode 100644 coroutine-win32.c
create mode 100644 qemu-coroutine-int.h
create mode 100644 qemu-coroutine.c
create mode 100644 qemu-coroutine.h
diff --git a/Makefile.objs b/Makefile.objs
index 9d8851e..cba6c2b 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -11,6 +11,12 @@ oslib-obj-$(CONFIG_WIN32) += oslib-win32.o
oslib-obj-$(CONFIG_POSIX) += oslib-posix.o
###
+# coroutines
+coroutine-obj-y = qemu-coroutine.o
+coroutine-obj-$(CONFIG_POSIX) += coroutine-ucontext.o
+coroutine-obj-$(CONFIG_WIN32) += coroutine-win32.o
+
+###
# block-obj-y is code used by both qemu system emulation and qemu-img
block-obj-y = cutils.o cache-utils.o qemu-malloc.o qemu-option.o module.o
async.o
@@ -67,6 +73,7 @@ common-obj-y += readline.o console.o cursor.o qemu-error.o
common-obj-y += $(oslib-obj-y)
common-obj-$(CONFIG_WIN32) += os-win32.o
common-obj-$(CONFIG_POSIX) += os-posix.o
+common-obj-y += $(coroutine-obj-y)
common-obj-y += tcg-runtime.o host-utils.o
common-obj-y += irq.o ioport.o input.o
diff --git a/coroutine-ucontext.c b/coroutine-ucontext.c
new file mode 100644
index 000..3b14ebf
--- /dev/null
+++ b/coroutine-ucontext.c
@@ -0,0 +1,73 @@
+/*
+ * ucontext coroutine initialization code
+ *
+ * Copyright (C) 2006 Anthony Liguori anth...@codemonkey.ws
+ * Copyright (C) 2011 Kevin Wolf kw...@redhat.com
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.0 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301
USA
+ */
+
+/* XXX Is there a nicer way to disable glibc's stack check for longjmp? */
+#ifdef _FORTIFY_SOURCE
+#undef _FORTIFY_SOURCE
+#endif
+#include setjmp.h
+#include stdint.h
+#include ucontext.h
+#include qemu-coroutine-int.h
+
+static Coroutine *new_coroutine;
+
+static void continuation_trampoline(void)
+{
+Coroutine *co = new_coroutine;
+
+/* Initialize longjmp environment and switch
[Qemu-devel] [PATCH v2 2/4] coroutine: add check-coroutine automated tests
To run automated tests for coroutines:
make check-coroutine
./check-coroutine
On success the program terminates with exit status 0. On failure an
error message is written to stderr and the program exits with exit
status 1.
Signed-off-by: Stefan Hajnoczi stefa...@linux.vnet.ibm.com
---
Makefile |3 +-
check-coroutine.c | 188 +
2 files changed, 190 insertions(+), 1 deletions(-)
create mode 100644 check-coroutine.c
diff --git a/Makefile b/Makefile
index 67c0268..27ec6a1 100644
--- a/Makefile
+++ b/Makefile
@@ -130,7 +130,7 @@ qemu-io$(EXESUF): qemu-io.o cmd.o qemu-tool.o qemu-error.o
$(oslib-obj-y) $(trac
qemu-img-cmds.h: $(SRC_PATH)/qemu-img-cmds.hx
$(call quiet-command,sh $(SRC_PATH)/scripts/hxtool -h $ $@, GEN
$@)
-check-qint.o check-qstring.o check-qdict.o check-qlist.o check-qfloat.o
check-qjson.o: $(GENERATED_HEADERS)
+check-qint.o check-qstring.o check-qdict.o check-qlist.o check-qfloat.o
check-qjson.o check-coroutine.o: $(GENERATED_HEADERS)
CHECK_PROG_DEPS = qemu-malloc.o $(oslib-obj-y) $(trace-obj-y)
@@ -140,6 +140,7 @@ check-qdict: check-qdict.o qdict.o qfloat.o qint.o
qstring.o qbool.o qlist.o $(C
check-qlist: check-qlist.o qlist.o qint.o $(CHECK_PROG_DEPS)
check-qfloat: check-qfloat.o qfloat.o $(CHECK_PROG_DEPS)
check-qjson: check-qjson.o qfloat.o qint.o qdict.o qstring.o qlist.o qbool.o
qjson.o json-streamer.o json-lexer.o json-parser.o $(CHECK_PROG_DEPS)
+check-coroutine: check-coroutine.o $(coroutine-obj-y) $(CHECK_PROG_DEPS)
QEMULIBS=libhw32 libhw64 libuser libdis libdis-user
diff --git a/check-coroutine.c b/check-coroutine.c
new file mode 100644
index 000..f65ac2e
--- /dev/null
+++ b/check-coroutine.c
@@ -0,0 +1,188 @@
+/*
+ * Coroutine tests
+ *
+ * Copyright IBM, Corp. 2011
+ *
+ * Authors:
+ * Stefan Hajnoczistefa...@linux.vnet.ibm.com
+ *
+ * This work is licensed under the terms of the GNU LGPL, version 2 or later.
+ * See the COPYING.LIB file in the top-level directory.
+ *
+ */
+
+#include stdlib.h
+#include stdio.h
+#include qemu-coroutine.h
+
+static const char *cur_test_name;
+
+static void test_assert(bool condition, const char *msg)
+{
+if (!condition) {
+fprintf(stderr, %s: %s\n, cur_test_name, msg);
+exit(EXIT_FAILURE);
+}
+}
+
+/*
+ * Check that qemu_in_coroutine() works
+ */
+
+static void coroutine_fn verify_in_coroutine(void *opaque)
+{
+test_assert(qemu_in_coroutine(), expected coroutine context);
+}
+
+static void test_in_coroutine(void)
+{
+Coroutine *coroutine;
+
+test_assert(!qemu_in_coroutine(), expected no coroutine context);
+
+coroutine = qemu_coroutine_create(verify_in_coroutine);
+qemu_coroutine_enter(coroutine, NULL);
+}
+
+/*
+ * Check that qemu_coroutine_self() works
+ */
+
+static void coroutine_fn verify_self(void *opaque)
+{
+test_assert(qemu_coroutine_self() == opaque,
+qemu_coroutine_self() did not return this coroutine);
+}
+
+static void test_self(void)
+{
+Coroutine *coroutine;
+
+coroutine = qemu_coroutine_create(verify_self);
+qemu_coroutine_enter(coroutine, coroutine);
+}
+
+/*
+ * Check that coroutines may nest multiple levels
+ */
+
+typedef struct {
+unsigned int n_enter; /* num coroutines entered */
+unsigned int n_return; /* num coroutines returned */
+unsigned int max; /* maximum level of nesting */
+} NestData;
+
+static void coroutine_fn nest(void *opaque)
+{
+NestData *nd = opaque;
+
+nd-n_enter++;
+
+if (nd-n_enter nd-max) {
+Coroutine *child;
+
+child = qemu_coroutine_create(nest);
+qemu_coroutine_enter(child, nd);
+}
+
+nd-n_return++;
+}
+
+static void test_nesting(void)
+{
+Coroutine *root;
+NestData nd = {
+.n_enter = 0,
+.n_return = 0,
+.max = 1,
+};
+
+root = qemu_coroutine_create(nest);
+qemu_coroutine_enter(root, nd);
+
+test_assert(nd.n_enter == nd.max,
+failed entering to max nesting level);
+test_assert(nd.n_return == nd.max,
+failed returning from max nesting level);
+}
+
+/*
+ * Check that yield/enter transfer control correctly
+ */
+
+static void coroutine_fn yield_5_times(void *opaque)
+{
+bool *done = opaque;
+int i;
+
+for (i = 0; i 5; i++) {
+qemu_coroutine_yield();
+}
+*done = true;
+}
+
+static void test_yield(void)
+{
+Coroutine *coroutine;
+bool done = false;
+int i = -1; /* one extra time to return from coroutine */
+
+coroutine = qemu_coroutine_create(yield_5_times);
+while (!done) {
+qemu_coroutine_enter(coroutine, done);
+i++;
+}
+test_assert(i == 5, coroutine did not yield 5 times);
+}
+
+/*
+ * Check that creation, enter, and return work
+ */
+
+static void coroutine_fn set_and_exit(void *opaque)
+{
+bool *done = opaque;
+
+*done = true;
+}
+
+static void test_lifecycle(void)
+{
+
[Qemu-devel] [PATCH v2 3/4] coroutine: add check-coroutine --benchmark-lifecycle
Add a microbenchmark for coroutine create, enter, and return (aka
lifecycle). This is a useful benchmark because users are expected to
create many coroutines, one per I/O request for example, and we
therefore need to provide good performance in that scenario.
To run:
make check-coroutine
./check-coroutine --benchmark-lifecycle 2000
This will do 20,000,000 coroutine create, enter, return iterations and
print the resulting time.
Signed-off-by: Stefan Hajnoczi stefa...@linux.vnet.ibm.com
---
check-coroutine.c | 48
1 files changed, 48 insertions(+), 0 deletions(-)
diff --git a/check-coroutine.c b/check-coroutine.c
index f65ac2e..5a42c49 100644
--- a/check-coroutine.c
+++ b/check-coroutine.c
@@ -11,8 +11,10 @@
*
*/
+#include string.h
#include stdlib.h
#include stdio.h
+#include sys/time.h
#include qemu-coroutine.h
static const char *cur_test_name;
@@ -163,6 +165,43 @@ static void test_lifecycle(void)
test_assert(done, expected done to be true (second time));
}
+/*
+ * Lifecycle benchmark
+ */
+
+static void coroutine_fn empty_coroutine(void *opaque)
+{
+/* Do nothing */
+}
+
+static void benchmark_lifecycle(const char *iterations)
+{
+Coroutine *coroutine;
+unsigned int i, max;
+struct timeval start, finish;
+time_t dsec;
+suseconds_t dusec;
+
+max = atoi(iterations);
+
+gettimeofday(start, NULL);
+for (i = 0; i max; i++) {
+coroutine = qemu_coroutine_create(empty_coroutine);
+qemu_coroutine_enter(coroutine, NULL);
+}
+gettimeofday(finish, NULL);
+
+dsec = finish.tv_sec - start.tv_sec;
+if (finish.tv_usec start.tv_usec) {
+dsec--;
+dusec = finish.tv_usec + 100 - start.tv_usec;
+} else {
+dusec = finish.tv_usec - start.tv_usec;
+}
+printf(Lifecycle %u iterations: %lu sec %lu us\n,
+ max, dsec, dusec);
+}
+
#define TESTCASE(fn) { #fn, fn }
int main(int argc, char **argv)
{
@@ -179,6 +218,15 @@ int main(int argc, char **argv)
};
int i;
+if (argc == 3 strcmp(argv[1], --benchmark-lifecycle) == 0) {
+benchmark_lifecycle(argv[2]);
+return EXIT_SUCCESS;
+} else if (argc != 1) {
+fprintf(stderr, usage: %s [--benchmark-lifecycle iterations]\n,
+argv[0]);
+return EXIT_FAILURE;
+}
+
for (i = 0; testcases[i].name; i++) {
cur_test_name = testcases[i].name;
printf(%s\n, testcases[i].name);
--
1.7.4.4
Re: [Qemu-devel] [PATCH 1/2] coroutine: introduce coroutines
On Thu, May 12, 2011 at 10:51 AM, Jan Kiszka jan.kis...@siemens.com wrote:
On 2011-05-11 12:15, Stefan Hajnoczi wrote:
From: Kevin Wolf kw...@redhat.com
Asynchronous code is becoming very complex. At the same time
synchronous code is growing because it is convenient to write.
Sometimes duplicate code paths are even added, one synchronous and the
other asynchronous. This patch introduces coroutines which allow code
that looks synchronous but is asynchronous under the covers.
A coroutine has its own stack and is therefore able to preserve state
across blocking operations, which traditionally require callback
functions and manual marshalling of parameters.
Creating and starting a coroutine is easy:
coroutine = qemu_coroutine_create(my_coroutine);
qemu_coroutine_enter(coroutine, my_data);
The coroutine then executes until it returns or yields:
void coroutine_fn my_coroutine(void *opaque) {
MyData *my_data = opaque;
/* do some work */
qemu_coroutine_yield();
/* do some more work */
}
Yielding switches control back to the caller of qemu_coroutine_enter().
This is typically used to switch back to the main thread's event loop
after issuing an asynchronous I/O request. The request callback will
then invoke qemu_coroutine_enter() once more to switch back to the
coroutine.
Note that coroutines never execute concurrently and should only be used
from threads which hold the global mutex. This restriction makes
programming with coroutines easier than with threads. Race conditions
cannot occur since only one coroutine may be active at any time. Other
coroutines can only run across yield.
Mmh, is there anything that conceptually prevent fixing this limitation
later on? I would really like to remove such dependency long-term as
well to have VCPUs operate truly independently on independent device models.
The use case that has motivated coroutines is the block layer. It is
synchronous in many places and definitely not thread-safe. Coroutines
is a step that solves the synchronous part of the problem but does
not tackle the not thread-safe part.
It is possible to move from coroutines to threads but we need to
remove single-thread assumptions from all the block layer code, which
isn't a small task. Coroutines does not prevent us from making the
block layer thread-safe!
Stefan
Re: [Qemu-devel] [PATCH 1/2] coroutine: introduce coroutines
Am 12.05.2011 11:51, schrieb Jan Kiszka:
On 2011-05-11 12:15, Stefan Hajnoczi wrote:
From: Kevin Wolf kw...@redhat.com
Asynchronous code is becoming very complex. At the same time
synchronous code is growing because it is convenient to write.
Sometimes duplicate code paths are even added, one synchronous and the
other asynchronous. This patch introduces coroutines which allow code
that looks synchronous but is asynchronous under the covers.
A coroutine has its own stack and is therefore able to preserve state
across blocking operations, which traditionally require callback
functions and manual marshalling of parameters.
Creating and starting a coroutine is easy:
coroutine = qemu_coroutine_create(my_coroutine);
qemu_coroutine_enter(coroutine, my_data);
The coroutine then executes until it returns or yields:
void coroutine_fn my_coroutine(void *opaque) {
MyData *my_data = opaque;
/* do some work */
qemu_coroutine_yield();
/* do some more work */
}
Yielding switches control back to the caller of qemu_coroutine_enter().
This is typically used to switch back to the main thread's event loop
after issuing an asynchronous I/O request. The request callback will
then invoke qemu_coroutine_enter() once more to switch back to the
coroutine.
Note that coroutines never execute concurrently and should only be used
from threads which hold the global mutex. This restriction makes
programming with coroutines easier than with threads. Race conditions
cannot occur since only one coroutine may be active at any time. Other
coroutines can only run across yield.
Mmh, is there anything that conceptually prevent fixing this limitation
later on? I would really like to remove such dependency long-term as
well to have VCPUs operate truly independently on independent device models.
I think it's the defining property of coroutines. If you remove it, you
have full threads. Going from coroutines to threads may be an option in
the long term. The advantage of coroutines is that they provide an
incremental way forward from where we are today. This restriction is
really what makes the transition easy.
Kevin
Re: [Qemu-devel] [PATCH v2 4/4] coroutine: pool coroutines to speed up creation
terAm 12.05.2011 11:54, schrieb Stefan Hajnoczi: This patch speeds up coroutine creation by reusing freed coroutines. When a coroutine terminates it is placed in the pool instead of having its resources freed. The next time a coroutine is created it can be taken straight from the pool and requires no initialization. Performance results on an Intel Core2 Duo T9400 (2.53GHz) for ./check-coroutine --benchmark-lifecycle 2000: No pooling:19.5 sec With pooling: 1.1 sec Signed-off-by: Stefan Hajnoczi stefa...@linux.vnet.ibm.com --- check-coroutine.c|2 ++ qemu-coroutine-int.h |2 ++ qemu-coroutine.c | 49 ++--- qemu-coroutine.h |9 + vl.c |2 ++ 5 files changed, 61 insertions(+), 3 deletions(-) diff --git a/check-coroutine.c b/check-coroutine.c index 5a42c49..223c50c 100644 --- a/check-coroutine.c +++ b/check-coroutine.c @@ -218,6 +218,8 @@ int main(int argc, char **argv) }; int i; +qemu_coroutine_init(); Can we use module_init instead of adding an explicit call to main()? This would prevent forgetting to add it in qemu-img and qemu-io like in this patch. Kevin
Re: [Qemu-devel] [PATCH v2 4/4] coroutine: pool coroutines to speed up creation
On Thu, May 12, 2011 at 11:13 AM, Kevin Wolf kw...@redhat.com wrote: terAm 12.05.2011 11:54, schrieb Stefan Hajnoczi: This patch speeds up coroutine creation by reusing freed coroutines. When a coroutine terminates it is placed in the pool instead of having its resources freed. The next time a coroutine is created it can be taken straight from the pool and requires no initialization. Performance results on an Intel Core2 Duo T9400 (2.53GHz) for ./check-coroutine --benchmark-lifecycle 2000: No pooling: 19.5 sec With pooling: 1.1 sec Signed-off-by: Stefan Hajnoczi stefa...@linux.vnet.ibm.com --- check-coroutine.c | 2 ++ qemu-coroutine-int.h | 2 ++ qemu-coroutine.c | 49 ++--- qemu-coroutine.h | 9 + vl.c | 2 ++ 5 files changed, 61 insertions(+), 3 deletions(-) diff --git a/check-coroutine.c b/check-coroutine.c index 5a42c49..223c50c 100644 --- a/check-coroutine.c +++ b/check-coroutine.c @@ -218,6 +218,8 @@ int main(int argc, char **argv) }; int i; + qemu_coroutine_init(); Can we use module_init instead of adding an explicit call to main()? This would prevent forgetting to add it in qemu-img and qemu-io like in this patch. module_init what? :) qemu-img/qemu-io only init MODULE_INIT_BLOCK so we'd have to modify them anyway. I don't want to add qemu-img/qemu-io things yet because we don't have a block layer user for coroutines yet. The qcow2 patches should contain these changes. Stefan
Re: [Qemu-devel] [PATCH v2 4/4] coroutine: pool coroutines to speed up creation
Am 12.05.2011 12:22, schrieb Stefan Hajnoczi: On Thu, May 12, 2011 at 11:13 AM, Kevin Wolf kw...@redhat.com wrote: terAm 12.05.2011 11:54, schrieb Stefan Hajnoczi: This patch speeds up coroutine creation by reusing freed coroutines. When a coroutine terminates it is placed in the pool instead of having its resources freed. The next time a coroutine is created it can be taken straight from the pool and requires no initialization. Performance results on an Intel Core2 Duo T9400 (2.53GHz) for ./check-coroutine --benchmark-lifecycle 2000: No pooling:19.5 sec With pooling: 1.1 sec Signed-off-by: Stefan Hajnoczi stefa...@linux.vnet.ibm.com --- check-coroutine.c|2 ++ qemu-coroutine-int.h |2 ++ qemu-coroutine.c | 49 ++--- qemu-coroutine.h |9 + vl.c |2 ++ 5 files changed, 61 insertions(+), 3 deletions(-) diff --git a/check-coroutine.c b/check-coroutine.c index 5a42c49..223c50c 100644 --- a/check-coroutine.c +++ b/check-coroutine.c @@ -218,6 +218,8 @@ int main(int argc, char **argv) }; int i; +qemu_coroutine_init(); Can we use module_init instead of adding an explicit call to main()? This would prevent forgetting to add it in qemu-img and qemu-io like in this patch. module_init what? :) qemu-img/qemu-io only init MODULE_INIT_BLOCK so we'd have to modify them anyway. Right... I was thinking of block, but in fact coroutines are not limited to the block layer, so we would abuse it. I don't want to add qemu-img/qemu-io things yet because we don't have a block layer user for coroutines yet. The qcow2 patches should contain these changes. I hope we won't forget it. A missing atexit isn't a very obvious bug. Kevin
Re: [Qemu-devel] [PATCH] Add warmup phase for live migration of large memory apps
Shribman, Aidan aidan.shrib...@sap.com wrote: On Wed, May 11, 2011 at 8:58 AM, Shribman, Aidan aidan.shrib...@sap.com wrote: From: Aidan Shribman aidan.shrib...@sap.com [PATCH] Add warmup phase for live migration of large memory apps By invoking migrate -w url we initiate a background live-migration transferring of dirty pages continuously until invocation of migrate_end which attempts to complete the live migration operation. What is the purpose of this patch? How and when do I use it? The warmup patch adds none-converging background update of guest memory during live-migration such that on request of live-migration completion (via migrate_end command) we get much faster response. This is especially needed when running a payload of large enterprise applications which have high memory demands. We should integrate this with Kemari (Kemari is doing something like this, just that it has more requirements). Isaku, do you have any comments? BTW, what loads have you tested for this? if I setup an image with 1GB RAM and a DVD iso image, and do in the guest: while true; do find /media/cdrom -type f | xargs md5sum; done Migration never converges with current code (if you use more than 1GB memory, then all the DVD will be cached inside). So, I see this only useful for guests that are almost idle, and on that case, migration speed is not the bigger of your problems, no? Later, Juan.
Re: [Qemu-devel] [PATCH] Add warmup phase for live migration of large memory apps
On Thu, May 12, 2011 at 12:39:22PM +0200, Juan Quintela wrote: Shribman, Aidan aidan.shrib...@sap.com wrote: On Wed, May 11, 2011 at 8:58 AM, Shribman, Aidan aidan.shrib...@sap.com wrote: From: Aidan Shribman aidan.shrib...@sap.com [PATCH] Add warmup phase for live migration of large memory apps By invoking migrate -w url we initiate a background live-migration transferring of dirty pages continuously until invocation of migrate_end which attempts to complete the live migration operation. What is the purpose of this patch? How and when do I use it? The warmup patch adds none-converging background update of guest memory during live-migration such that on request of live-migration completion (via migrate_end command) we get much faster response. This is especially needed when running a payload of large enterprise applications which have high memory demands. We should integrate this with Kemari (Kemari is doing something like this, just that it has more requirements). Isaku, do you have any comments? Yochi and Kei are familiar with Kemari. Not me. Cced to them. BTW, what loads have you tested for this? if I setup an image with 1GB RAM and a DVD iso image, and do in the guest: while true; do find /media/cdrom -type f | xargs md5sum; done Migration never converges with current code (if you use more than 1GB memory, then all the DVD will be cached inside). So, I see this only useful for guests that are almost idle, and on that case, migration speed is not the bigger of your problems, no? Later, Juan. -- yamahata
Re: [Qemu-devel] [PATCH] Add warmup phase for live migration of large memory apps
Shribman, Aidan aidan.shrib...@sap.com wrote:
From: Aidan Shribman aidan.shrib...@sap.com
[PATCH] Add warmup phase for live migration of large memory apps
By invoking migrate -w url we initiate a background live-migration
transferring of dirty pages continuously until invocation of migrate_end
which attempts to complete the live migration operation.
Qemu host: Ubuntu 10.10
Codding style comments already commented by Stefan. Please use checkpatch.
+static int is_migrate_warmup = 0;
once here, this can be bool O:-)
And this should be part of FdMigrationState, not a global variable.
+ DPRINTF(iterate\n);
-if (qemu_savevm_state_iterate(s-mon, s-file) == 1) {
+if (is_migrate_warmup) {
+qemu_savevm_state_warmup(s-mon, s-file);
+ } else if (qemu_savevm_state_iterate(s-mon, s-file) == 1) {
Refactor this out? qemu_savevm_state_warmup is just calling
qemu_savevm_state_iterate() and you are just ignoring error return (what
is wrong).
Why not just do something like.
ret = qemu_savevm_state_iterate(s-mon, s-file);
if (ret 0) {
s-state = state;
notifier_list_notify(migration_state_notifiers);
return;
}
if (s-is_migrate_warmup) {
return;
}
if (ret == 1) {
int state;
int old_vm_running = vm_running;
/* and rest of old normal code */
}
+void do_migrate_end(Monitor *mon, const QDict *qdict)
+{
+if (!vm_running) {
+return;
+ }
+if (!is_migrate_warmup) {
+return;
+ }
+is_migrate_warmup = 0;
+}
If we add this, we should generalize it to always work. i.e. just
convert the current migration in non-live, or something like that.
diff --git a/savevm.c b/savevm.c
index 4e49765..521589c 100644
--- a/savevm.c
+++ b/savevm.c
@@ -1471,6 +1471,18 @@ int qemu_savevm_state_begin(Monitor *mon, QEMUFile *f,
int blk_enable,
return 0;
}
+int qemu_savevm_state_warmup(Monitor *mon, QEMUFile *f) {
+int ret = 1;
+
+ret = qemu_savevm_state_iterate(mon, f);
+
+if (ret == -EIO) {
+return ret;
+ }
+
+return 0;
+}
You can see here why I don't like this function, it is just a wrapper to
qemu_savevm_state_iterate() that does nothing, just detects the return
value equal to 1.
As stated by Anthony, I think that we can simulate this functionality
playing with max_downtime value. Why is that solution not enough for you?
Later, Juan.
Re: [Qemu-devel] [PATCH] Add warmup phase for live migration of large memory apps
Shribman, Aidan aidan.shrib...@sap.com wrote: If there are no additional dirty pages to be sent i.e. ram_save_remaining() == 0 then the migration *will* converge (even though we don't want it to) see: This should be a really idle guest to have that O:-) But if that is the whole problem, I would change the code to make the case of max_downtime == 0 to mean that migration didn't converge. Later, Juan.
Re: [Qemu-devel] qdev device documentation (Re: [PATCH 0/2] usb-linux: physical port handling.)
[note cc: Amit] Gerd Hoffmann kra...@redhat.com writes: Hi, Some additional docs (based on 2/2 hints) might be a useful addition. Indeed. Possible candidates: - docs/qdev-device-use.txt - qemu-doc.texi Hmm. qemu-doc.texi only documents the pre-qdev way to create devices. qdev-device-use.txt is a conversion guide. devices which can only be created via -device (or provide additional features via qdev attributes when created that way) are not documented anywhere now. Suggestions? Should we add a qdev section to qemu-doc.texi? Or better a separate qemu-devices.txt (or .texi)? What is the status of the qdev documentation patches btw.? http://lists.gnu.org/archive/html/qemu-devel/2011-02/msg02169.html Maybe we should just merge them, then start filling stuff and maybe also autogenerate documentation from that? Yes. Amit, once more into the breach?
Re: [Qemu-devel] [PATCH v2 4/4] coroutine: pool coroutines to speed up creation
On 05/12/2011 12:38 PM, Kevin Wolf wrote: I don't want to add qemu-img/qemu-io things yet because we don't have a block layer user for coroutines yet. The qcow2 patches should contain these changes. I hope we won't forget it. A missing atexit isn't a very obvious bug. I was going to reply that this atexit is actually useless, since memory is freed automatically at exit? Paolo
Re: [Qemu-devel] [PATCH v2 4/4] coroutine: pool coroutines to speed up creation
On Thu, May 12, 2011 at 12:12 PM, Paolo Bonzini pbonz...@redhat.com wrote: On 05/12/2011 12:38 PM, Kevin Wolf wrote: I don't want to add qemu-img/qemu-io things yet because we don't have a block layer user for coroutines yet. The qcow2 patches should contain these changes. I hope we won't forget it. A missing atexit isn't a very obvious bug. I was going to reply that this atexit is actually useless, since memory is freed automatically at exit? It's just for completeness to make tools like valgrind happy. Sure, the kernel will reclaim memory and we're just burning CPU by freeing this stuff ;). Stefan
Re: [Qemu-devel] [PATCH v2 4/4] coroutine: pool coroutines to speed up creation
On 05/12/2011 01:15 PM, Stefan Hajnoczi wrote: It's just for completeness to make tools like valgrind happy. Sure, the kernel will reclaim memory and we're just burning CPU by freeing this stuff;). But valgrind will not complain about reachable memory still allocated at exit, at least not with the default command-line options. Paolo
Re: [Qemu-devel] [PATCH] Add warmup phase for live migration of large memory apps
From: Juan Quintela [mailto:quint...@redhat.com] If there are no additional dirty pages to be sent i.e. ram_save_remaining() == 0 then the migration *will* converge (even though we don't want it to) see: This should be a really idle guest to have that O:-) But if that is the whole problem, I would change the code to make the case of max_downtime == 0 to mean that migration didn't converge. I agree this would be fine as surrogate for the warmup patch as I proposed. New patch based on your's / Anthony's suggestion: --- diff --git a/arch_init.c b/arch_init.c index 4486925..9a35411 100644 --- a/arch_init.c +++ b/arch_init.c @@ -295,7 +295,8 @@ int ram_save_live(Monitor *mon, QEMUFile *f, int stage, void *opaque) expected_time = ram_save_remaining() * TARGET_PAGE_SIZE / bwidth; -return (stage == 2) (expected_time = migrate_max_downtime()); +return (stage == 2) migrate_max_downtime() +(expected_time = migrate_max_downtime()); } static inline void *host_from_stream_offset(QEMUFile *f, --- Best, Aidan
Re: [Qemu-devel] [PATCH v2 4/4] coroutine: pool coroutines to speed up creation
On Thu, May 12, 2011 at 12:18 PM, Paolo Bonzini pbonz...@redhat.com wrote: On 05/12/2011 01:15 PM, Stefan Hajnoczi wrote: It's just for completeness to make tools like valgrind happy. Sure, the kernel will reclaim memory and we're just burning CPU by freeing this stuff;). But valgrind will not complain about reachable memory still allocated at exit, at least not with the default command-line options. /me is outsmarted by valgrind Then I will remove pool freeing in v3. Stefan
[Qemu-devel] [Bug 723871] Re: qemu-kvm-0.14.0 Aborts with -vga qxl
I386 build constantly fails in my PPA https://launchpad.net/~bderzhavets/+archive/git-spice. Build on local box ( Q9550,8 GB ) Ubuntu 11.04 (64-bit) debuild -rfakeroot -b -k930900E6 | tee build-v35.log doesn't have any problems with ./hw/qxl.c. I attached log and packages ** Attachment added: Buid log https://bugs.launchpad.net/qemu/+bug/723871/+attachment/2125644/+files/build-v35.log -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/723871 Title: qemu-kvm-0.14.0 Aborts with -vga qxl Status in QEMU: Confirmed Status in “libvirt” package in Ubuntu: Triaged Status in “qemu-kvm” package in Ubuntu: Fix Released Bug description: Host CPU is Core i7 Q820. KVM is from 2.6.35-gentoo-r5 kernel (x86_64). Host has spice-0.7.2 and spice-protocol-0.7.0. Guest is Windows XP SP3 with qxl driver 0.6.1, virtio-serial 1.1.6 and vdagent 0.6.3. qemu-kvm is started like so: qemu-system-x86_64 -cpu host -enable-kvm -pidfile /home/rick/qemu/hds/wxp.pid -drive file=/home/rick/qemu/hds/wxp.raw,if=virtio,media=disk,aio=native,snapshot=on -m 768 -name WinXP -net nic,model=virtio -net user -localtime -usb -vga qxl -device virtio-serial -chardev spicevmc,name=vdagent,id=vdagent -device virtserialport,chardev=vdagent,name=com.redhat.spice.0 -spice port=1234,disable-ticketing -monitor stdio and crashes with: qemu-system-x86_64: /home/rick/qemu/src/qemu-kvm-0.14.0/qemu-kvm.c:1724: kvm_mutex_unlock: Assertion `!cpu_single_env' failed. Aborted If I use -no-kvm, it works fine. If I use -vga std, it works fine. -enable-kvm and -vga qxl crashes.
[Qemu-devel] [Bug 723871] Re: qemu-kvm-0.14.0 Aborts with -vga qxl
** Attachment added: qemu-common_0.14.0+spice-v35-0ubuntu1_all.deb https://bugs.launchpad.net/qemu/+bug/723871/+attachment/2125646/+files/qemu-common_0.14.0%2Bspice-v35-0ubuntu1_all.deb -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/723871 Title: qemu-kvm-0.14.0 Aborts with -vga qxl Status in QEMU: Confirmed Status in “libvirt” package in Ubuntu: Triaged Status in “qemu-kvm” package in Ubuntu: Fix Released Bug description: Host CPU is Core i7 Q820. KVM is from 2.6.35-gentoo-r5 kernel (x86_64). Host has spice-0.7.2 and spice-protocol-0.7.0. Guest is Windows XP SP3 with qxl driver 0.6.1, virtio-serial 1.1.6 and vdagent 0.6.3. qemu-kvm is started like so: qemu-system-x86_64 -cpu host -enable-kvm -pidfile /home/rick/qemu/hds/wxp.pid -drive file=/home/rick/qemu/hds/wxp.raw,if=virtio,media=disk,aio=native,snapshot=on -m 768 -name WinXP -net nic,model=virtio -net user -localtime -usb -vga qxl -device virtio-serial -chardev spicevmc,name=vdagent,id=vdagent -device virtserialport,chardev=vdagent,name=com.redhat.spice.0 -spice port=1234,disable-ticketing -monitor stdio and crashes with: qemu-system-x86_64: /home/rick/qemu/src/qemu-kvm-0.14.0/qemu-kvm.c:1724: kvm_mutex_unlock: Assertion `!cpu_single_env' failed. Aborted If I use -no-kvm, it works fine. If I use -vga std, it works fine. -enable-kvm and -vga qxl crashes.
[Qemu-devel] [Bug 723871] Re: qemu-kvm-0.14.0 Aborts with -vga qxl
** Attachment added: qemu_0.14.0+spice-v35-0ubuntu1_amd64.deb https://bugs.launchpad.net/qemu/+bug/723871/+attachment/2125645/+files/qemu_0.14.0%2Bspice-v35-0ubuntu1_amd64.deb -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/723871 Title: qemu-kvm-0.14.0 Aborts with -vga qxl Status in QEMU: Confirmed Status in “libvirt” package in Ubuntu: Triaged Status in “qemu-kvm” package in Ubuntu: Fix Released Bug description: Host CPU is Core i7 Q820. KVM is from 2.6.35-gentoo-r5 kernel (x86_64). Host has spice-0.7.2 and spice-protocol-0.7.0. Guest is Windows XP SP3 with qxl driver 0.6.1, virtio-serial 1.1.6 and vdagent 0.6.3. qemu-kvm is started like so: qemu-system-x86_64 -cpu host -enable-kvm -pidfile /home/rick/qemu/hds/wxp.pid -drive file=/home/rick/qemu/hds/wxp.raw,if=virtio,media=disk,aio=native,snapshot=on -m 768 -name WinXP -net nic,model=virtio -net user -localtime -usb -vga qxl -device virtio-serial -chardev spicevmc,name=vdagent,id=vdagent -device virtserialport,chardev=vdagent,name=com.redhat.spice.0 -spice port=1234,disable-ticketing -monitor stdio and crashes with: qemu-system-x86_64: /home/rick/qemu/src/qemu-kvm-0.14.0/qemu-kvm.c:1724: kvm_mutex_unlock: Assertion `!cpu_single_env' failed. Aborted If I use -no-kvm, it works fine. If I use -vga std, it works fine. -enable-kvm and -vga qxl crashes.
[Qemu-devel] [Bug 723871] Re: qemu-kvm-0.14.0 Aborts with -vga qxl
** Attachment added: qemu-kvm_0.14.0+spice-v35-0ubuntu1_amd64.deb https://bugs.launchpad.net/qemu/+bug/723871/+attachment/2125647/+files/qemu-kvm_0.14.0%2Bspice-v35-0ubuntu1_amd64.deb -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/723871 Title: qemu-kvm-0.14.0 Aborts with -vga qxl Status in QEMU: Confirmed Status in “libvirt” package in Ubuntu: Triaged Status in “qemu-kvm” package in Ubuntu: Fix Released Bug description: Host CPU is Core i7 Q820. KVM is from 2.6.35-gentoo-r5 kernel (x86_64). Host has spice-0.7.2 and spice-protocol-0.7.0. Guest is Windows XP SP3 with qxl driver 0.6.1, virtio-serial 1.1.6 and vdagent 0.6.3. qemu-kvm is started like so: qemu-system-x86_64 -cpu host -enable-kvm -pidfile /home/rick/qemu/hds/wxp.pid -drive file=/home/rick/qemu/hds/wxp.raw,if=virtio,media=disk,aio=native,snapshot=on -m 768 -name WinXP -net nic,model=virtio -net user -localtime -usb -vga qxl -device virtio-serial -chardev spicevmc,name=vdagent,id=vdagent -device virtserialport,chardev=vdagent,name=com.redhat.spice.0 -spice port=1234,disable-ticketing -monitor stdio and crashes with: qemu-system-x86_64: /home/rick/qemu/src/qemu-kvm-0.14.0/qemu-kvm.c:1724: kvm_mutex_unlock: Assertion `!cpu_single_env' failed. Aborted If I use -no-kvm, it works fine. If I use -vga std, it works fine. -enable-kvm and -vga qxl crashes.
[Qemu-devel] [PATCH] Avoid segmentation fault for qdev device not found
qdev_try_create will cope well with a NULL bus, since it will assume
the main system bus by default. qdev_create, however, wants to print
a message, in which it instantiates the bus name. That simple and at
first inoffensive message will generate a segmentation found if the
reason for failure is a NULL bus.
I propose we avoid that - thus generating the normal hw_error by
always passing a valid bus to qdev_try_create - if none, be it the
main system bus.
The code for testing a NULL bus is not remove from qdev_try_create
because it is a externally visible function, and we want it to continue
working fine.
Signed-off-by: Glauber Costa glom...@redhat.com
---
hw/qdev.c |4
1 files changed, 4 insertions(+), 0 deletions(-)
diff --git a/hw/qdev.c b/hw/qdev.c
index 1aa1ea0..21ef075 100644
--- a/hw/qdev.c
+++ b/hw/qdev.c
@@ -108,6 +108,10 @@ DeviceState *qdev_create(BusState *bus, const char *name)
{
DeviceState *dev;
+if (!bus) {
+bus = sysbus_get_default();
+}
+
dev = qdev_try_create(bus, name);
if (!dev) {
hw_error(Unknown device '%s' for bus '%s'\n, name, bus-info-name);
--
1.7.4.2
[Qemu-devel] [PATCH v2] Add an isa device for SGA
This patch adds a dummy legacy ISA device whose responsibility is to
deploy sgabios, an option rom for a serial graphics adapter.
The proposal is that this device is always-on when -nographics,
but can otherwise be enable in any setup when -device sga is used.
[v2: suggestions on qdev by Markus ]
Signed-off-by: Glauber Costa glom...@redhat.com
---
Makefile.target |2 +-
hw/pc.c |4
hw/pc.h |3 +++
hw/sga.c| 31 +++
4 files changed, 39 insertions(+), 1 deletions(-)
create mode 100644 hw/sga.c
diff --git a/Makefile.target b/Makefile.target
index fdbdc6c..004ea7e 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -224,7 +224,7 @@ obj-$(CONFIG_KVM) += ivshmem.o
# Hardware support
obj-i386-y += vga.o
obj-i386-y += mc146818rtc.o i8259.o pc.o
-obj-i386-y += cirrus_vga.o apic.o ioapic.o piix_pci.o
+obj-i386-y += cirrus_vga.o sga.o apic.o ioapic.o piix_pci.o
obj-i386-y += vmport.o
obj-i386-y += device-hotplug.o pci-hotplug.o smbios.o wdt_ib700.o
obj-i386-y += extboot.o
diff --git a/hw/pc.c b/hw/pc.c
index 8d351ba..78bf7de 100644
--- a/hw/pc.c
+++ b/hw/pc.c
@@ -1096,6 +1096,10 @@ void pc_vga_init(PCIBus *pci_bus)
isa_vga_init();
}
}
+
+if (display_type == DT_NOGRAPHIC) {
+isa_create_simple(sga);
+}
}
static void cpu_request_exit(void *opaque, int irq, int level)
diff --git a/hw/pc.h b/hw/pc.h
index 1291e2d..a00e054 100644
--- a/hw/pc.h
+++ b/hw/pc.h
@@ -219,6 +219,9 @@ int isa_vga_mm_init(target_phys_addr_t vram_base,
void pci_cirrus_vga_init(PCIBus *bus);
void isa_cirrus_vga_init(void);
+/* serial graphics */
+void isa_sga_init(void);
+
/* ne2000.c */
static inline bool isa_ne2000_init(int base, int irq, NICInfo *nd)
{
diff --git a/hw/sga.c b/hw/sga.c
new file mode 100644
index 000..08cef56
--- /dev/null
+++ b/hw/sga.c
@@ -0,0 +1,31 @@
+#include pci.h
+#include pc.h
+#include loader.h
+#include sysemu.h
+
+#define SGABIOS_FILENAME sgabios.bin
+
+typedef struct ISAGAState {
+ISADevice dev;
+} ISASGAState;
+
+
+static int isa_cirrus_vga_initfn(ISADevice *dev)
+{
+rom_add_vga(SGABIOS_FILENAME);
+return 0;
+}
+
+static ISADeviceInfo sga_info = {
+.qdev.name= sga,
+.qdev.desc= Serial Graphics Adapter,
+.qdev.size= sizeof(ISASGAState),
+.init = isa_cirrus_vga_initfn,
+};
+
+static void sga_register(void)
+{
+ isa_qdev_register(sga_info);
+}
+
+device_init(sga_register);
--
1.7.4.2
Re: [Qemu-devel] [PULL] Trivial patches for April 26 to May 8 2011
On 05/08/2011 06:04 PM, Stefan Hajnoczi wrote: The following changes since commit 3964f535c35c08470ac69bd553282af500bc8bb0: Merge remote-tracking branch 'mst/for_anthony' into staging (2011-05-05 13:05:32 -0500) are available in the git repository at: git://repo.or.cz/qemu/stefanha.git trivial-patches Ping, can other committers pull this? Thanks! Paolo
Re: [Qemu-devel] [PATCH] Add warmup phase for live migration of large memory apps
On 05/12/2011 06:23 AM, Shribman, Aidan wrote: From: Juan Quintela [mailto:quint...@redhat.com] If there are no additional dirty pages to be sent i.e. ram_save_remaining() == 0 then the migration *will* converge (even though we don't want it to) see: This should be a really idle guest to have that O:-) But if that is the whole problem, I would change the code to make the case of max_downtime == 0 to mean that migration didn't converge. I agree this would be fine as surrogate for the warmup patch as I proposed. New patch based on your's / Anthony's suggestion: --- diff --git a/arch_init.c b/arch_init.c index 4486925..9a35411 100644 --- a/arch_init.c +++ b/arch_init.c @@ -295,7 +295,8 @@ int ram_save_live(Monitor *mon, QEMUFile *f, int stage, void *opaque) expected_time = ram_save_remaining() * TARGET_PAGE_SIZE / bwidth; -return (stage == 2) (expected_time= migrate_max_downtime()); +return (stage == 2) migrate_max_downtime() +(expected_time= migrate_max_downtime()); } This is better but let's make sure to document this behavior (and please include a Signed-off-by: in the next patch). Regards, Anthony Liguori static inline void *host_from_stream_offset(QEMUFile *f, --- Best, Aidan
Re: [Qemu-devel] -net interface association behavior change in current -git.
Hi, On Wed, May 11, 2011 at 22:39, Rob Landley r...@landley.net wrote: In 1.14.0, if I did this: qemu -net nic,blah -net user -net nic,blah -net tun,blah Then the first nic would be -net user, and the second nic would be -net tun. In current -git, -net user attaches to the second interface and -net tun attaches to the first, I.E. the order is reversed. Either way the first -nic becomes eth0 in Linux and the second becomes eth1 (I can manually assign mac addresses in order to confirm which is which), but eth0 used to be the -net user interface and now eth1 is the -net user interface. I bisected this to commit 60c07d933c66c4b30a83b but I don't know why it changed the behavior, and I can't find _documentation_ on having multiple interfaces transports hooked up to the same qemu instance anyway. (It used to work, but possibly that was an accident?) Any ideas? First of all, as you have 2 totally separated subnets in your setup, I think your command-line should use vlan= parameter to isolate them, else you will end up with some random routing/broadcasting (and random tends to change over time). I'm not using setup with multiple NICs but I would have written something like : qemu -net nic,vlan=1,blah -net user,vlan=1 -net nic,vlan=2,blah -net tun,vlan=2,blah In addition to this, which type of NIC are you using ? In my understanding, the Linux kernel might assign interface number depending on the order the interfaces are appearing. Without my change, when a packet arrives and should be distributed to multiple interfaces (that seems to be the case in your setup even though it is not intended) if one of the interface is not ready, the packet is only forwarded to the ready interface (and the other never receives it). This produces interesting timing effects where packets are routed according to obscure race conditions, but in your former setup, that might cause the packet to be routed to the interface you want. -- Vincent
Re: [Qemu-devel] [PULL] Xen HVM support
On 05/09/2011 12:39 PM, Alexander Graf wrote: Hi Anthony, These are Anthony's patches for Xen HVM support, nicely signed off, rebased to fit today's HEAD and compile tested. Please pull. Pulled. Thanks. Regards, Anthony Liguori Alex The following changes since commit 85097db6956bc86e2377b63a8309cb8b24d54139: Richard Henderson (1): irq: Privatize CPU_INTERRUPT_NMI. are available in the git repository at: git://repo.or.cz/qemu/agraf.git xen-next Anthony PERARD (16): Introduce -machine command option. machine, Add default_machine_opts to QEMUMachine. xen: Replace some tab-indents with spaces (clean-up). xen: Make Xen build once. xen: Support new libxc calls from xen unstable. xen: Add initialisation of Xen pc_memory_init: Move memory calculation to the caller. xen: Add xenfv machine pc, Disable vmport initialisation with Xen. piix_pci: Introduces Xen specific call for irq. xen: Introduce Xen Interrupt Controller Introduce qemu_put_ram_ptr configure: Always use 64bits target physical addresses with xen enabled. vl.c: Introduce getter for shutdown_requested and reset_requested. xen: Set running state in xenstore. xen: Add Xen hypercall for sleep state in the cmos_s3 callback. Arun Sharma (1): xen: Initialize event channels and io rings John Baboval (2): xen: Adds a cap to the number of map cache entries. pci: Use of qemu_put_ram_ptr in pci_add_option_rom. Jun Nakajima (1): xen: Introduce the Xen mapcache Makefile.target | 14 +- arch_init.c |5 + arch_init.h |1 + configure| 71 ++- cpu-common.h |1 + exec.c | 86 +++- hw/boards.h |1 + hw/pc.c | 28 +-- hw/pc.h | 11 +- hw/pc_piix.c | 71 ++- hw/pci.c |2 + hw/piix_pci.c| 49 - hw/xen.h | 41 hw/xen_backend.c | 421 +++ hw/xen_backend.h |6 +- hw/xen_common.h | 106 -- hw/xen_disk.c| 496 ++--- hw/xen_domainbuild.c |3 +- hw/xen_machine_pv.c |1 + hw/xen_nic.c | 265 -- qemu-config.c| 14 ++ qemu-options.hx | 10 + sysemu.h |2 + trace-events | 13 + vl.c | 136 ++- xen-all.c| 605 ++ xen-mapcache-stub.c | 44 xen-mapcache.c | 375 +++ xen-mapcache.h | 37 +++ xen-stub.c | 41 30 files changed, 2343 insertions(+), 613 deletions(-) create mode 100644 xen-all.c create mode 100644 xen-mapcache-stub.c create mode 100644 xen-mapcache.c create mode 100644 xen-mapcache.h create mode 100644 xen-stub.c
Re: [Qemu-devel] [PULL] Trivial patches for April 26 to May 8 2011
On 05/08/2011 11:04 AM, Stefan Hajnoczi wrote: The following changes since commit 3964f535c35c08470ac69bd553282af500bc8bb0: Merge remote-tracking branch 'mst/for_anthony' into staging (2011-05-05 13:05:32 -0500) are available in the git repository at: git://repo.or.cz/qemu/stefanha.git trivial-patches Pulled. Thanks. Sorry for the delay, I think I overlooked this pull request. Regards, Anthony Liguori Hannes Reinecke (1): lsi53c895a: Rename 'sense' to 'status' Paolo Bonzini (1): libcacard: add correct subdirectory dependencies Stefan Weil (22): Fix typo in code and comments Fix typos in comments (dependancy - dependency) Fix typos in comments (accross - across) Fix typos in comments (accessable - accessible, priveleged - privileged) Fix typo in comment (colum - column) Fix typo in comment (auxilliary - auxiliary) Fix typo in comment (embeded - embedded) Fix typo in comment (consistant - consistent) Fix typo in comment (dieing - dying) Fix typos in comments (imediately - immediately) Fix typos in comments (existance - existence) Fix typos in comments (interupt - interrupt) Fix typos in comments (instanciation - instantiation) Fix typos in comments (neccessary - necessary) Fix typos in comments and code (occured - occurred and related) Fix typo in comment (relevent - relevant) Fix typo in comment (responsiblity - responsibility) Fix typo in comment (truely - truly) Fix typos in comment (threshhold - threshold, mapp - map) ac97: Remove unused local variables Fix spelling in comments (intruction - instruction) linux-user: Replace deprecated function Changelog|2 +- Makefile |2 + Makefile.objs|9 +++ block.c |4 +- block/qcow2-refcount.c |2 +- block/sheepdog.c |4 +- bsd-user/main.c |2 +- bsd-user/qemu.h |2 +- cpu-all.h|2 +- cpu-exec.c |4 +- darwin-user/syscall.c|2 +- hppa-dis.c |2 +- hw/ac97.c|6 + hw/bt.h |2 +- hw/eepro100.c|2 +- hw/eeprom93xx.c | 10 hw/lan9118.c |2 +- hw/lsi53c895a.c | 18 hw/msi.c |2 +- hw/msix.c|2 +- hw/mst_fpga.c|2 +- hw/pci.c |6 ++-- hw/pci.h |2 +- hw/pci_regs.h|2 +- hw/pcie.c|2 +- hw/pcie.h|2 +- hw/pcie_aer.c|2 +- hw/pflash_cfi02.c|2 +- hw/pl031.c |2 +- hw/pl061.c |4 +- hw/ppc4xx_devs.c |2 +- hw/rtl8139.c | 44 ++--- hw/sh7750_regs.h |6 ++-- hw/ssd0303.c |2 +- hw/sun4m_iommu.c |2 +- hw/syborg_serial.c |2 +- hw/xilinx_axidma.c |4 +- libcacard/Makefile |9 +--- libcacard/vcard_emul_nss.c |2 +- libcacard/vscard_common.h|2 +- linux-user/main.c|2 +- linux-user/mmap.c|2 +- linux-user/qemu.h|2 +- linux-user/signal.c |2 +- linux-user/syscall.c |2 +- target-arm/translate.c | 10 target-cris/cpu.h|2 +- target-cris/translate_v10.c |2 +- target-m68k/helper.c |2 +- target-microblaze/helper.c |2 +- target-mips/translate_init.c |4 +- target-ppc/cpu.h |2 +- target-ppc/translate_init.c |2 +- tcg/tcg.h|4 +- tests/test-i386.c|2 +- tests/test-mmap.c|4 +- 56 files changed, 111 insertions(+), 117 deletions(-)
Re: [Qemu-devel] [PULL] Xen HVM support
On Thu, 12 May 2011, Anthony Liguori wrote: On 05/09/2011 12:39 PM, Alexander Graf wrote: Hi Anthony, These are Anthony's patches for Xen HVM support, nicely signed off, rebased to fit today's HEAD and compile tested. Please pull. Pulled. Thanks. popping champagne over here :-)
Re: [Qemu-devel] [RFC] live snapshot, live merge, live block migration
On Mon, May 09, 2011 at 10:23:03AM -0500, Anthony Liguori wrote: On 05/09/2011 08:40 AM, Dor Laor wrote: No patch here (sorry) but collection of thoughts about these features and their potential building blocks. Please review (also on http://wiki.qemu.org/Features/LiveBlockMigration) Future qemu is expected to support these features (some already implemented): * Live block copy Ability to copy 1+ virtual disk from the source backing file/block device to a new target that is accessible by the host. The copy supposed to be executed while the VM runs in a transparent way. Status: code exists (by Marcelo) today in qemu but needs refactoring due to a race condition at the end of the copy operation. We agreed that a re-implementation of the copy operation should take place that makes sure the image is completely mirrored until management decides what copy to keep. Live block copy is growing on me. It can actually be used (with an intermediate network storage) to do live block migration. * Live snapshots and live snapshot merge Live snapshot is already incorporated (by Jes) in qemu (still need qemu-agent work to freeze the guest FS). Live snapshot is unfortunately not really live. It runs a lot of operations synchronously which will cause the guest to incur downtime. We really need to refactor it to truly be live. * Copy on read (image streaming) Ability to start guest execution while the parent image reside remotely and each block access is replicated to a local copy (image format snapshot) It should be nice to have a general mechanism that will be used for all image formats. What about the protocol to access these blocks over the net? We can reuse existing ones (nbd/iscsi). I think the image format is really the best place to have this logic. Of course, if we have live snapshot merge, we could use a temporary QED/QCOW2 file and then merge afterwards. * Using external dirty block bitmap FVD has an option to use external dirty block bitmap file in addition to the regular mapping/data files. We can consider using it for live block migration and live merge too. It can also allow additional usages of 3rd party tools to calculate diffs between the snapshots. There is a big down side thought since it will make management complicated and there is the risky of the image and its bitmap file get out of sync. It's much better choice to have qemu-img tool to be the single interface to the dirty block bitmap data. Does the dirty block bitmap need to exist outside of QEMU? IOW, if it goes away after a guest shuts down, is that problematic? I think it potentially greatly simplifies the problem which makes it appealing from my perspective. One limitation of block copy is the need to rewrite data that differs from the base image on every merge. But this is a limitation of qcow2 external snapshots represented as files, not block copy itself (with external qcow2 snapshots, even a live block merge would require potentially copying large amounts of data). Only with snapshots internal to an image data copying can be avoided (and depending on the scenario, this can be a nasty limitation). Regards, Anthony Liguori
Re: [Qemu-devel] [PATCH 1/2] Add documentation for qemu_progres_print()
On 05/06/2011 06:10 PM, Markus Armbruster wrote: Here's my try: /* * Report progress. * @percent is how much progress we made. * If @max is zero, @percent is how much of the job is done. * Else, @percent is a progress delta since the last call, as a fraction * of @max. I.e. delta is @percent * @max / 100. This is for * convenience, it lets you account for @max% of the total work in some * function, and still count @percent from 0 to 100. */ My personal preference is to use fractions of 1 in the code and only covert to percentages during actual output. -- error compiling committee.c: too many arguments to function
[Qemu-devel] [PATCH v3 5/6] block: Remove type hint, it's guest matter, doesn't belong here
No users of bdrv_get_type_hint() left. bdrv_set_type_hint() can make
the media removable by side effect. Make that explicit.
Signed-off-by: Markus Armbruster arm...@redhat.com
---
block.c | 12
block.h |5 -
block_int.h |1 -
blockdev.c |4 ++--
4 files changed, 2 insertions(+), 20 deletions(-)
diff --git a/block.c b/block.c
index c6dc42a..cf3c8b7 100644
--- a/block.c
+++ b/block.c
@@ -1305,13 +1305,6 @@ void bdrv_set_geometry_hint(BlockDriverState *bs,
bs-secs = secs;
}
-void bdrv_set_type_hint(BlockDriverState *bs, int type)
-{
-bs-type = type;
-bs-removable = ((type == BDRV_TYPE_CDROM ||
- type == BDRV_TYPE_FLOPPY));
-}
-
void bdrv_set_translation_hint(BlockDriverState *bs, int translation)
{
bs-translation = translation;
@@ -1428,11 +1421,6 @@ void bdrv_get_floppy_geometry_hint(BlockDriverState *bs,
int *nb_heads,
}
}
-int bdrv_get_type_hint(BlockDriverState *bs)
-{
-return bs-type;
-}
-
int bdrv_get_translation_hint(BlockDriverState *bs)
{
return bs-translation;
diff --git a/block.h b/block.h
index 52e9cad..da7d39c 100644
--- a/block.h
+++ b/block.h
@@ -152,9 +152,6 @@ int bdrv_has_zero_init(BlockDriverState *bs);
int bdrv_is_allocated(BlockDriverState *bs, int64_t sector_num, int nb_sectors,
int *pnum);
-#define BDRV_TYPE_HD 0
-#define BDRV_TYPE_CDROM 1
-#define BDRV_TYPE_FLOPPY 2
#define BIOS_ATA_TRANSLATION_AUTO 0
#define BIOS_ATA_TRANSLATION_NONE 1
#define BIOS_ATA_TRANSLATION_LBA2
@@ -163,7 +160,6 @@ int bdrv_is_allocated(BlockDriverState *bs, int64_t
sector_num, int nb_sectors,
void bdrv_set_geometry_hint(BlockDriverState *bs,
int cyls, int heads, int secs);
-void bdrv_set_type_hint(BlockDriverState *bs, int type);
void bdrv_set_translation_hint(BlockDriverState *bs, int translation);
void bdrv_get_geometry_hint(BlockDriverState *bs,
int *pcyls, int *pheads, int *psecs);
@@ -177,7 +173,6 @@ typedef enum FDriveType {
void bdrv_get_floppy_geometry_hint(BlockDriverState *bs, int *nb_heads,
int *max_track, int *last_sect,
FDriveType drive_in, FDriveType *drive);
-int bdrv_get_type_hint(BlockDriverState *bs);
int bdrv_get_translation_hint(BlockDriverState *bs);
void bdrv_set_on_error(BlockDriverState *bs, BlockErrorAction on_read_error,
BlockErrorAction on_write_error);
diff --git a/block_int.h b/block_int.h
index 545ad11..fa91337 100644
--- a/block_int.h
+++ b/block_int.h
@@ -194,7 +194,6 @@ struct BlockDriverState {
/* NOTE: the following infos are only hints for real hardware
drivers. They are not used by the block driver */
int cyls, heads, secs, translation;
-int type;
BlockErrorAction on_read_error, on_write_error;
char device_name[32];
unsigned long *dirty_bitmap;
diff --git a/blockdev.c b/blockdev.c
index 28727df..6e0eb83 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -487,7 +487,7 @@ DriveInfo *drive_init(QemuOpts *opts, int default_to_scsi)
}
break;
case MEDIA_CDROM:
-bdrv_set_type_hint(dinfo-bdrv, BDRV_TYPE_CDROM);
+bdrv_set_removable(dinfo-bdrv, 1);
dinfo-media_cd = 1;
break;
}
@@ -496,7 +496,7 @@ DriveInfo *drive_init(QemuOpts *opts, int default_to_scsi)
/* FIXME: This isn't really a floppy, but it's a reasonable
approximation. */
case IF_FLOPPY:
-bdrv_set_type_hint(dinfo-bdrv, BDRV_TYPE_FLOPPY);
+bdrv_set_removable(dinfo-bdrv, 1);
break;
case IF_PFLASH:
case IF_MTD:
--
1.7.2.3
[Qemu-devel] [PATCH v3 3/6] block QMP: Drop query-block member type (type= in info block)
Its value is unreliable: a block device used as floppy has type
floppy if created with if=floppy, but type hd if created with
if=none.
That's because with if=none, the type is at best a declaration of
intent: the drive can be connected to any guest device. Its type is
really the guest device's business. Reporting it here is wrong.
Signed-off-by: Markus Armbruster arm...@redhat.com
---
block.c | 20 +++-
qmp-commands.hx |6 --
2 files changed, 3 insertions(+), 23 deletions(-)
diff --git a/block.c b/block.c
index f731c7a..c6dc42a 100644
--- a/block.c
+++ b/block.c
@@ -1704,9 +1704,8 @@ static void bdrv_print_dict(QObject *obj, void *opaque)
bs_dict = qobject_to_qdict(obj);
-monitor_printf(mon, %s: type=%s removable=%d,
+monitor_printf(mon, %s: removable=%d,
qdict_get_str(bs_dict, device),
-qdict_get_str(bs_dict, type),
qdict_get_bool(bs_dict, removable));
if (qdict_get_bool(bs_dict, removable)) {
@@ -1747,23 +1746,10 @@ void bdrv_info(Monitor *mon, QObject **ret_data)
QTAILQ_FOREACH(bs, bdrv_states, list) {
QObject *bs_obj;
-const char *type = unknown;
-
-switch(bs-type) {
-case BDRV_TYPE_HD:
-type = hd;
-break;
-case BDRV_TYPE_CDROM:
-type = cdrom;
-break;
-case BDRV_TYPE_FLOPPY:
-type = floppy;
-break;
-}
-bs_obj = qobject_from_jsonf({ 'device': %s, 'type': %s,
+bs_obj = qobject_from_jsonf({ 'device': %s,
'removable': %i, 'locked': %i },
-bs-device_name, type, bs-removable,
+bs-device_name, bs-removable,
bs-locked);
if (bs-drv) {
diff --git a/qmp-commands.hx b/qmp-commands.hx
index fbd98ee..b1c8206 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -1038,8 +1038,6 @@ is a json-array of all devices.
Each json-object contain the following:
- device: device name (json-string)
-- type: device type (json-string)
- - Possible values: hd, cdrom, floppy, unknown
- removable: true if the device is removable, false otherwise (json-bool)
- locked: true if the device is locked, false otherwise (json-bool)
- inserted: only present if the device is inserted, it is a json-object
@@ -1070,25 +1068,21 @@ Example:
encrypted:false,
file:disks/test.img
},
-type:hd
},
{
device:ide1-cd0,
locked:false,
removable:true,
-type:cdrom
},
{
device:floppy0,
locked:false,
removable:true,
-type: floppy
},
{
device:sd0,
locked:false,
removable:true,
-type:floppy
}
]
}
--
1.7.2.3
[Qemu-devel] [PATCH v3 2/6] scsi: Split qdev scsi-disk into scsi-hd and scsi-cd
A scsi-disk is either a hard disk or a CD-ROM, depending on the
associated BlockDriverState's type hint. Unclean; disk vs. CD belongs
to the guest part, not the host part.
Have separate qdevs scsi-hd and scsi-cd to model disk vs. CD in
the guest part.
Keep scsi-disk for backward compatibility.
Don't copy scsi-disk property removable to scsi-cd. It's not used and
always zero(!) there.
Signed-off-by: Markus Armbruster arm...@redhat.com
---
hw/scsi-disk.c | 136 ++--
1 files changed, 103 insertions(+), 33 deletions(-)
diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
index b05e654..8df8518 100644
--- a/hw/scsi-disk.c
+++ b/hw/scsi-disk.c
@@ -65,6 +65,8 @@ typedef struct SCSIDiskReq {
uint32_t status;
} SCSIDiskReq;
+typedef enum { SCSI_HD, SCSI_CD } SCSIDriveKind;
+
struct SCSIDiskState
{
SCSIDevice qdev;
@@ -78,6 +80,7 @@ struct SCSIDiskState
char *version;
char *serial;
SCSISense sense;
+SCSIDriveKind drive_kind;
};
static int scsi_handle_rw_error(SCSIDiskReq *r, int error, int type);
@@ -406,7 +409,7 @@ static int scsi_disk_emulate_inquiry(SCSIRequest *req,
uint8_t *outbuf)
return -1;
}
-if (bdrv_get_type_hint(s-bs) == BDRV_TYPE_CDROM) {
+if (s-drive_kind == SCSI_CD) {
outbuf[buflen++] = 5;
} else {
outbuf[buflen++] = 0;
@@ -424,7 +427,7 @@ static int scsi_disk_emulate_inquiry(SCSIRequest *req,
uint8_t *outbuf)
outbuf[buflen++] = 0x00; // list of supported pages (this page)
outbuf[buflen++] = 0x80; // unit serial number
outbuf[buflen++] = 0x83; // device identification
-if (bdrv_get_type_hint(s-bs) != BDRV_TYPE_CDROM) {
+if (s-drive_kind == SCSI_HD) {
outbuf[buflen++] = 0xb0; // block limits
outbuf[buflen++] = 0xb2; // thin provisioning
}
@@ -477,7 +480,7 @@ static int scsi_disk_emulate_inquiry(SCSIRequest *req,
uint8_t *outbuf)
unsigned int opt_io_size =
s-qdev.conf.opt_io_size / s-qdev.blocksize;
-if (bdrv_get_type_hint(s-bs) == BDRV_TYPE_CDROM) {
+if (s-drive_kind == SCSI_CD) {
DPRINTF(Inquiry (EVPD[%02X] not supported for CDROM\n,
page_code);
return -1;
@@ -547,7 +550,7 @@ static int scsi_disk_emulate_inquiry(SCSIRequest *req,
uint8_t *outbuf)
return buflen;
}
-if (bdrv_get_type_hint(s-bs) == BDRV_TYPE_CDROM) {
+if (s-drive_kind == SCSI_CD) {
outbuf[0] = 5;
outbuf[1] = 0x80;
memcpy(outbuf[16], QEMU CD-ROM , 16);
@@ -678,7 +681,7 @@ static int mode_sense_page(SCSIRequest *req, int page,
uint8_t *p,
return p[1] + 2;
case 0x2a: /* CD Capabilities and Mechanical Status page. */
-if (bdrv_get_type_hint(bdrv) != BDRV_TYPE_CDROM)
+if (s-drive_kind != SCSI_CD)
return 0;
p[0] = 0x2a;
p[1] = 0x14;
@@ -905,7 +908,7 @@ static int scsi_disk_emulate_command(SCSIDiskReq *r,
uint8_t *outbuf)
goto illegal_request;
break;
case START_STOP:
-if (bdrv_get_type_hint(s-bs) == BDRV_TYPE_CDROM (req-cmd.buf[4]
2)) {
+if (s-drive_kind == SCSI_CD (req-cmd.buf[4] 2)) {
/* load/eject medium */
bdrv_eject(s-bs, !(req-cmd.buf[4] 1));
}
@@ -1232,10 +1235,9 @@ static void scsi_destroy(SCSIDevice *dev)
blockdev_mark_auto_del(s-qdev.conf.bs);
}
-static int scsi_disk_initfn(SCSIDevice *dev)
+static int scsi_initfn(SCSIDevice *dev, SCSIDriveKind kind)
{
SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, dev);
-int is_cd;
DriveInfo *dinfo;
if (!s-qdev.conf.bs) {
@@ -1243,9 +1245,9 @@ static int scsi_disk_initfn(SCSIDevice *dev)
return -1;
}
s-bs = s-qdev.conf.bs;
-is_cd = bdrv_get_type_hint(s-bs) == BDRV_TYPE_CDROM;
+s-drive_kind = kind;
-if (!is_cd !bdrv_is_inserted(s-bs)) {
+if (kind == SCSI_HD !bdrv_is_inserted(s-bs)) {
error_report(Device needs media, but drive is empty);
return -1;
}
@@ -1265,7 +1267,7 @@ static int scsi_disk_initfn(SCSIDevice *dev)
return -1;
}
-if (is_cd) {
+if (kind == SCSI_CD) {
s-qdev.blocksize = 2048;
} else {
s-qdev.blocksize = s-qdev.conf.logical_block_size;
@@ -1275,35 +1277,103 @@ static int scsi_disk_initfn(SCSIDevice *dev)
s-qdev.type = TYPE_DISK;
qemu_add_vm_change_state_handler(scsi_dma_restart_cb, s);
-bdrv_set_removable(s-bs, is_cd);
+bdrv_set_removable(s-bs, kind == SCSI_CD);
add_boot_device_path(s-qdev.conf.bootindex, dev-qdev, ,0);
return 0;
}
-static SCSIDeviceInfo scsi_disk_info = {
-.qdev.name= scsi-disk,
-.qdev.fw_name = disk,
-.qdev.desc= virtual scsi disk or cdrom,
-.qdev.size= sizeof(SCSIDiskState),
-
[Qemu-devel] [PATCH v2 0/5] Split ide-drive and scsi-disk qdevs, and more
This patch series is about purging the type hint from the block layer. My previous series cleaned up improper uses it. Remaining uses are info block and qdevs ide-drive, scsi-disk. ide-drive and scsi-disk can either act as disk or as CD drive. They use their drive's type hint to decide between disk and CD. This is unclean. Disk vs. CD needs to be in qdev, not BlockDriverState, because it belongs to the drive's guest part. Split them into separate devices for disk and CD. Keep the old ones for backward compatibility. Remove the type hint from info block. Its value is unreliable anyway. libvirt doesn't use it. I posted v1 quite some time ago. Since we were working towards a release then, we decided to take only the bonus bug fixes (PATCH 1-3), and revisit the rest later. Which has turned out to be somewhat later than anticpiated. Sorry about that. v3: * Trivially rebased * fix if=ide: ide_create_drive() got HD vs. CD backwards (one-liner) * ide-cd and scsi-cd devices suppress default CD-ROM (PATCH 6/6) v2: * Rebased * Review comments addressed * Reordered so that potentially controversial parts come later in the series * More verbose commit messages v1: http://lists.gnu.org/archive/html/qemu-devel/2010-07/msg00304.html Markus Armbruster (6): ide: Split qdev ide-drive into ide-hd and ide-cd scsi: Split qdev scsi-disk into scsi-hd and scsi-cd block QMP: Drop query-block member type (type= in info block) blockdev: Store -drive option media in DriveInfo block: Remove type hint, it's guest matter, doesn't belong here defaults: ide-cd and scsi-cd devices suppress default CD-ROM block.c| 32 +--- block.h|5 -- block_int.h|1 - blockdev.c |5 +- blockdev.h |1 + hw/ide/core.c | 10 ++-- hw/ide/internal.h |2 +- hw/ide/qdev.c | 81 --- hw/scsi-disk.c | 137 +++- hw/xen_devconfig.c |2 +- qmp-commands.hx|6 -- vl.c |2 + 12 files changed, 185 insertions(+), 99 deletions(-) -- 1.7.2.3
Re: [Qemu-devel] [PATCH v2 0/5] Split ide-drive and scsi-disk qdevs, and more
Kevin Wolf kw...@redhat.com writes: Am 09.05.2011 11:51, schrieb Markus Armbruster: This patch series is about purging the type hint from the block layer. My previous series cleaned up improper uses it. Remaining uses are info block and qdevs ide-drive, scsi-disk. ide-drive and scsi-disk can either act as disk or as CD drive. They use their drive's type hint to decide between disk and CD. This is unclean. Disk vs. CD needs to be in qdev, not BlockDriverState, because it belongs to the drive's guest part. Split them into separate devices for disk and CD. Keep the old ones for backward compatibility. Remove the type hint from info block. Its value is unreliable anyway. libvirt doesn't use it. I posted v1 quite some time ago. Since we were working towards a release then, we decided to take only the bonus bug fixes (PATCH 1-3), and revisit the rest later. Which has turned out to be somewhat later than anticpiated. Sorry about that. Thanks, applied to the block branch. I found a bug, v3 sent.
[Qemu-devel] [PATCH v3 1/6] ide: Split qdev ide-drive into ide-hd and ide-cd
An ide-drive is either a hard disk or a CD-ROM, depending on the
associated BlockDriverState's type hint. Unclean; disk vs. CD belongs
to the guest part, not the host part.
Have separate qdevs ide-hd and ide-cd to model disk vs. CD in
the guest part.
Keep ide-drive for backward compatibility.
ide-disk would perhaps be a nicer name than ide-hd, but there's
already scsi-disk, which is like ide-drive, and will be likewise
split in the next commit. {ide,scsi}-{hd,cd} is the best consistent
set of names I could find within the backward compatibility
straightjacket.
Signed-off-by: Markus Armbruster arm...@redhat.com
---
hw/ide/core.c | 11 --
hw/ide/internal.h |2 +-
hw/ide/qdev.c | 83 ++---
3 files changed, 74 insertions(+), 22 deletions(-)
diff --git a/hw/ide/core.c b/hw/ide/core.c
index 90f553b..542ed65 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -1592,13 +1592,15 @@ void ide_bus_reset(IDEBus *bus)
bus-dma-ops-reset(bus-dma);
}
-int ide_init_drive(IDEState *s, BlockDriverState *bs,
+int ide_init_drive(IDEState *s, BlockDriverState *bs, IDEDriveKind kind,
const char *version, const char *serial)
{
int cylinders, heads, secs;
uint64_t nb_sectors;
s-bs = bs;
+s-drive_kind = kind;
+
bdrv_get_geometry(bs, nb_sectors);
bdrv_guess_geometry(bs, cylinders, heads, secs);
if (cylinders 1 || cylinders 16383) {
@@ -1623,8 +1625,7 @@ int ide_init_drive(IDEState *s, BlockDriverState *bs,
s-smart_autosave = 1;
s-smart_errors = 0;
s-smart_selftest_count = 0;
-if (bdrv_get_type_hint(bs) == BDRV_TYPE_CDROM) {
-s-drive_kind = IDE_CD;
+if (kind == IDE_CD) {
bdrv_set_change_cb(bs, cdrom_change_cb, s);
bs-buffer_alignment = 2048;
} else {
@@ -1729,7 +1730,9 @@ void ide_init2_with_non_qdev_drives(IDEBus *bus,
DriveInfo *hd0,
dinfo = i == 0 ? hd0 : hd1;
ide_init1(bus, i);
if (dinfo) {
-if (ide_init_drive(bus-ifs[i], dinfo-bdrv, NULL,
+if (ide_init_drive(bus-ifs[i], dinfo-bdrv,
+ bdrv_get_type_hint(dinfo-bdrv) ==
BDRV_TYPE_CDROM ? IDE_CD : IDE_HD,
+ NULL,
*dinfo-serial ? dinfo-serial : NULL) 0) {
error_report(Can't set up IDE drive %s, dinfo-id);
exit(1);
diff --git a/hw/ide/internal.h b/hw/ide/internal.h
index aa198b6..c2b35ec 100644
--- a/hw/ide/internal.h
+++ b/hw/ide/internal.h
@@ -558,7 +558,7 @@ uint32_t ide_data_readw(void *opaque, uint32_t addr);
void ide_data_writel(void *opaque, uint32_t addr, uint32_t val);
uint32_t ide_data_readl(void *opaque, uint32_t addr);
-int ide_init_drive(IDEState *s, BlockDriverState *bs,
+int ide_init_drive(IDEState *s, BlockDriverState *bs, IDEDriveKind kind,
const char *version, const char *serial);
void ide_init2(IDEBus *bus, qemu_irq irq);
void ide_init2_with_non_qdev_drives(IDEBus *bus, DriveInfo *hd0,
diff --git a/hw/ide/qdev.c b/hw/ide/qdev.c
index 2bb5c27..3bca726 100644
--- a/hw/ide/qdev.c
+++ b/hw/ide/qdev.c
@@ -98,7 +98,9 @@ IDEDevice *ide_create_drive(IDEBus *bus, int unit, DriveInfo
*drive)
{
DeviceState *dev;
-dev = qdev_create(bus-qbus, ide-drive);
+dev = qdev_create(bus-qbus,
+ bdrv_get_type_hint(drive-bdrv) == BDRV_TYPE_CDROM
+ ? ide-cd : ide-hd);
qdev_prop_set_uint32(dev, unit, unit);
qdev_prop_set_drive_nofail(dev, drive, drive-bdrv);
qdev_init_nofail(dev);
@@ -118,7 +120,7 @@ typedef struct IDEDrive {
IDEDevice dev;
} IDEDrive;
-static int ide_drive_initfn(IDEDevice *dev)
+static int ide_dev_initfn(IDEDevice *dev, IDEDriveKind kind)
{
IDEBus *bus = DO_UPCAST(IDEBus, qbus, dev-qdev.parent_bus);
IDEState *s = bus-ifs + dev-unit;
@@ -134,7 +136,7 @@ static int ide_drive_initfn(IDEDevice *dev)
}
}
-if (ide_init_drive(s, dev-conf.bs, dev-version, serial) 0) {
+if (ide_init_drive(s, dev-conf.bs, kind, dev-version, serial) 0) {
return -1;
}
@@ -151,22 +153,69 @@ static int ide_drive_initfn(IDEDevice *dev)
return 0;
}
-static IDEDeviceInfo ide_drive_info = {
-.qdev.name = ide-drive,
-.qdev.fw_name = drive,
-.qdev.size = sizeof(IDEDrive),
-.init = ide_drive_initfn,
-.qdev.props = (Property[]) {
-DEFINE_PROP_UINT32(unit, IDEDrive, dev.unit, -1),
-DEFINE_BLOCK_PROPERTIES(IDEDrive, dev.conf),
-DEFINE_PROP_STRING(ver, IDEDrive, dev.version),
-DEFINE_PROP_STRING(serial, IDEDrive, dev.serial),
-DEFINE_PROP_END_OF_LIST(),
+static int ide_hd_initfn(IDEDevice *dev)
+{
+return ide_dev_initfn(dev, IDE_HD);
+}
+
+static int ide_cd_initfn(IDEDevice *dev)
+{
+return ide_dev_initfn(dev, IDE_CD);
+}
+
+static int ide_drive_initfn(IDEDevice *dev)
+{
+return ide_dev_initfn(dev,
+
Re: [Qemu-devel] qdev device documentation (Re: [PATCH 0/2] usb-linux: physical port handling.)
Hi, What is the status of the qdev documentation patches btw.? http://lists.gnu.org/archive/html/qemu-devel/2011-02/msg02169.html What is the problem with the empty strings btw? The only way around I can see is having _DOC and _NODOC versions for all the property macros, but I'd prefer to not have _NODOC macros in the tree ... cheers, Gerd
[Qemu-devel] [PATCH] v5 revamp acpitable parsing and allow to specify complete (headerful) table
This patch almost rewrites acpi_table_add() function
(but still leaves it using old get_param_value() interface).
The result is that it's now possible to specify whole table
(together with a header) in an external file, instead of just
data portion, with a new file= parameter, but at the same time
it's still possible to specify header fields as before.
Now with the checkpatch.pl formatting fixes, thanks to
Stefan Hajnoczi for suggestions, with changes from
Isaku Yamahata, and with my further refinements.
v5: rediffed against current qemu/master.
Signed-off-by: Michael Tokarev m...@tls.msk.ru
---
hw/acpi.c | 292 ---
qemu-options.hx |7 +-
2 files changed, 175 insertions(+), 124 deletions(-)
diff --git a/hw/acpi.c b/hw/acpi.c
index ad40fb4..4316189 100644
--- a/hw/acpi.c
+++ b/hw/acpi.c
@@ -22,17 +22,29 @@
struct acpi_table_header
{
-char signature [4];/* ACPI signature (4 ASCII characters) */
+uint16_t _length; /* our length, not actual part of the hdr */
+ /* XXX why we have 2 length fields here? */
+char sig[4]; /* ACPI signature (4 ASCII characters) */
uint32_t length; /* Length of table, in bytes, including header */
uint8_t revision; /* ACPI Specification minor version # */
uint8_t checksum; /* To make sum of entire table == 0 */
-char oem_id [6]; /* OEM identification */
-char oem_table_id [8]; /* OEM table identification */
+char oem_id[6]; /* OEM identification */
+char oem_table_id[8]; /* OEM table identification */
uint32_t oem_revision;/* OEM revision number */
-char asl_compiler_id [4]; /* ASL compiler vendor ID */
+char asl_compiler_id[4]; /* ASL compiler vendor ID */
uint32_t asl_compiler_revision; /* ASL compiler revision number */
} __attribute__((packed));
+#define ACPI_TABLE_HDR_SIZE sizeof(struct acpi_table_header)
+#define ACPI_TABLE_PFX_SIZE sizeof(uint16_t) /* size of the extra prefix */
+
+static const char dfl_hdr[ACPI_TABLE_HDR_SIZE] =
+\0\0 /* fake _length (2) */
+QEMU\0\0\0\0\1\0 /* sig (4), len(4), revno (1), csum (1) */
+QEMUQEQEMUQEMU\1\0\0\0 /* OEM id (6), table (8), revno (4) */
+QEMU\1\0\0\0 /* ASL compiler ID (4), version (4) */
+;
+
char *acpi_tables;
size_t acpi_tables_len;
@@ -45,158 +57,192 @@ static int acpi_checksum(const uint8_t *data, int len)
return (-sum) 0xff;
}
+/* like strncpy() but zero-fills the tail of destination */
+static void strzcpy(char *dst, const char *src, size_t size)
+{
+size_t len = strlen(src);
+if (len = size) {
+len = size;
+} else {
+ memset(dst + len, 0, size - len);
+}
+memcpy(dst, src, len);
+}
+
+/* XXX fixme: this function uses obsolete argument parsing interface */
int acpi_table_add(const char *t)
{
-static const char *dfl_id = QEMUQEMU;
char buf[1024], *p, *f;
-struct acpi_table_header acpi_hdr;
unsigned long val;
-uint32_t length;
-struct acpi_table_header *acpi_hdr_p;
-size_t off;
+size_t len, start, allen;
+bool has_header;
+int changed;
+int r;
+struct acpi_table_header hdr;
+
+r = 0;
+r |= get_param_value(buf, sizeof(buf), data, t) ? 1 : 0;
+r |= get_param_value(buf, sizeof(buf), file, t) ? 2 : 0;
+switch (r) {
+case 0:
+buf[0] = '\0';
+case 1:
+has_header = false;
+break;
+case 2:
+has_header = true;
+break;
+default:
+fprintf(stderr, acpitable: both data and file are specified\n);
+return -1;
+}
+
+if (!acpi_tables) {
+allen = sizeof(uint16_t);
+acpi_tables = qemu_mallocz(allen);
+}
+else {
+allen = acpi_tables_len;
+}
+
+start = allen;
+acpi_tables = qemu_realloc(acpi_tables, start + ACPI_TABLE_HDR_SIZE);
+allen += has_header ? ACPI_TABLE_PFX_SIZE : ACPI_TABLE_HDR_SIZE;
+
+/* now read in the data files, reallocating buffer as needed */
+
+for (f = strtok(buf, :); f; f = strtok(NULL, :)) {
+int fd = open(f, O_RDONLY);
+
+if (fd 0) {
+fprintf(stderr, can't open file %s: %s\n, f, strerror(errno));
+return -1;
+}
+
+for (;;) {
+char data[8192];
+r = read(fd, data, sizeof(data));
+if (r == 0) {
+break;
+} else if (r 0) {
+acpi_tables = qemu_realloc(acpi_tables, allen + r);
+memcpy(acpi_tables + allen, data, r);
+allen += r;
+} else if (errno != EINTR) {
+fprintf(stderr, can't read file %s: %s\n,
+f, strerror(errno));
+close(fd);
+return -1;
+}
+}
+
+close(fd);
+}
+
+/* now fill in the header fields */
+
+f
Re: [Qemu-devel] [PATCH 0/4] introduce cpu_physical_memory_map_fast
On 05/03/2011 07:49 PM, Paolo Bonzini wrote: Paravirtualized devices (and also some real devices) can assume they are going to access RAM. For this reason, provide a fast-path function with the following properties: 1) it will never allocate a bounce buffer 2) it can be used for read-modify-write operations 3) unlike qemu_get_ram_ptr, it is safe because it recognizes short blocks Patches 3 and 4 use this function for virtio devices and the milkymist GPU. The latter is only compile-tested. Another function checks if it is possible to split a contiguous physical address range into multiple subranges, all of which use the fast path. I will introduce later a use for this function. Out of curiosity, what performance benefit do you see? For relatively constant mappings (like the ring) we can cache the mapping in structure and invalidate it when the memory map changes (using, say, rcu). That doesn't work for the actual buffers, or for indirect mappings. -- error compiling committee.c: too many arguments to function
Re: [Qemu-devel] [RFC] live snapshot, live merge, live block migration
On 05/09/11 15:40, Dor Laor wrote: Summary: * We need Marcelo's new (to come) block copy implementation * should work in parallel to migration and hotplug * General copy on read is desirable * Live snapshot merge to be implemented using block copy * Need to utilize a remote block access protocol (iscsi/nbd/other) Which one is the best? * Keep qemu-img the single interface for dirty block mappings. * Live block migration pre copy == live copy + block access protocol + live migration * Live block migration post copy == live migration + block access protocol/copy on read. Comments? I think we should add Jagane Sundar's Livebackup to the watch list here. It looks very interesting as an alternative way to reach some of the same goals. Cheers, Jes
Re: [Qemu-devel] qdev device documentation (Re: [PATCH 0/2] usb-linux: physical port handling.)
On 05/12/2011 10:25 AM, Gerd Hoffmann wrote: Hi, What is the status of the qdev documentation patches btw.? http://lists.gnu.org/archive/html/qemu-devel/2011-02/msg02169.html What is the problem with the empty strings btw? The only way around I can see is having _DOC and _NODOC versions for all the property macros, but I'd prefer to not have _NODOC macros in the tree ... Inline documentation is bad. Our documentation should be centralized. That's the only way to keep it consistent and thorough. There's no way to easily extract the inline docs in a complete way since some devices are built conditionally. Regards, Anthony Liguori cheers, Gerd
Re: [Qemu-devel] [RFC] live snapshot, live merge, live block migration
On 05/09/11 17:23, Anthony Liguori wrote: * Live snapshots and live snapshot merge Live snapshot is already incorporated (by Jes) in qemu (still need qemu-agent work to freeze the guest FS). Live snapshot is unfortunately not really live. It runs a lot of operations synchronously which will cause the guest to incur downtime. We really need to refactor it to truly be live. We keep having this discussion, but as pointed out in my last reply on this, you can pre-create your image if you so desire. The actual snapshot then becomes less in one command. Yes we can make it even nicer, but what we have now is far less bad than you make it out to be. Cheers, Jes
Re: [Qemu-devel] [PATCH v2 0/5] Split ide-drive and scsi-disk qdevs, and more
Pasto: this is PATCH v3. Sorry.
Re: [Qemu-devel] qdev device documentation (Re: [PATCH 0/2] usb-linux: physical port handling.)
Gerd Hoffmann kra...@redhat.com writes: Hi, What is the status of the qdev documentation patches btw.? http://lists.gnu.org/archive/html/qemu-devel/2011-02/msg02169.html What is the problem with the empty strings btw? Anthony ;) [...]
Re: [Qemu-devel] qdev device documentation (Re: [PATCH 0/2] usb-linux: physical port handling.)
On 05/12/2011 10:25 AM, Gerd Hoffmann wrote:
Hi,
What is the status of the qdev documentation patches btw.?
http://lists.gnu.org/archive/html/qemu-devel/2011-02/msg02169.html
What is the problem with the empty strings btw?
The only way around I can see is having _DOC and _NODOC versions for all
the property macros, but I'd prefer to not have _NODOC macros in the
tree ...
Here's an example of what I'm suggesting. I think we should just go
with this and add better output as we go.
But we need all of the qdev information.. not just a doc string for
each property.
Regards,
Anthony Liguori
cheers,
Gerd
From 130c817790880c61b79dbccf66f5863c406eb7d4 Mon Sep 17 00:00:00 2001
From: Anthony Liguori aligu...@us.ibm.com
Date: Thu, 12 May 2011 10:56:29 -0500
Subject: [PATCH] qdev: add centralized documentation for qdev
This is mostly a proof-of-concept.
Signed-off-by: Anthony Liguori aligu...@us.ibm.com
diff --git a/Makefile b/Makefile
index 2b0438c..fddb261 100644
--- a/Makefile
+++ b/Makefile
@@ -341,5 +341,7 @@ tarbin:
$(mandir)/man1/qemu-img.1 \
$(mandir)/man8/qemu-nbd.8
+include $(SRC_PATH)/Makefile.doc
+
# Include automatically generated dependency files
-include $(wildcard *.d audio/*.d slirp/*.d block/*.d net/*.d ui/*.d)
diff --git a/Makefile.doc b/Makefile.doc
new file mode 100644
index 000..f769b23
--- /dev/null
+++ b/Makefile.doc
@@ -0,0 +1,2 @@
+qdev-doc.html: $(SRC_PATH)/qdev-doc.json
+ python $(SRC_PATH)/scripts/qdev-doc-to-html.py $ $@
diff --git a/qdev-doc.json b/qdev-doc.json
new file mode 100644
index 000..c24630b
--- /dev/null
+++ b/qdev-doc.json
@@ -0,0 +1,14 @@
+# -*- Mode: Python -*-
+
+[ { device: isa-serial,
+parent: ISADevice,
+properties: {
+index: { type: uint32,
+ doc: A value from 0-3 that describes which IO regions to expose the device on. This sets appropriate values for iobase and irq. },
+iobase: { type: hex32,
+doc: The base IO address to expose the device on. },
+irq: { type: uint32,
+ doc: The IRQ to use for the device. },
+chardev: { type: chr,
+ doc: The name of a character device to specify. } } }
+ ]
diff --git a/scripts/qdev-doc-to-html.py b/scripts/qdev-doc-to-html.py
new file mode 100644
index 000..a25fe35
--- /dev/null
+++ b/scripts/qdev-doc-to-html.py
@@ -0,0 +1,40 @@
+#!/usr/bin/python
+
+import sys
+
+data = sys.stdin.read()
+
+docs = eval(data)
+
+sys.stdout.write('''
+html
+head
+titleQEMU device documentation/title
+/head
+body
+''')
+
+for item in docs:
+sys.stdout.write('''
+h2%(device)s :: %(parent)s/h2
+
+table border=1
+tr
+thName/ththType/ththComments/th
+/tr
+''' % item)
+for prop in item[properties]:
+sys.stdout.write('''
+tr
+td%s/tdtd%s/tdtd%s/td
+/tr
+''' % (prop, item[properties][prop]['type'], item[properties][prop]['doc']))
+
+sys.stdout.write('''
+/table
+''')
+
+sys.stdout.write('''
+/body
+/html
+''')
--
1.7.4.1
[Qemu-devel] [Bug 723871] Re: qemu-kvm-0.14.0 Aborts with -vga qxl
@Boris, The failure (in https://launchpadlibrarian.net/71569256/buildlog_ubuntu- natty-i386.qemu-kvm_0.14.0%2Bspice-v35-0ubuntu1_FAILEDTOBUILD.txt.gz) is: /build/buildd/qemu-kvm-0.14.0+spice-v35/hw/qxl.c: In function 'interface_release_resource': /build/buildd/qemu-kvm-0.14.0+spice-v35/hw/qxl.c:456:46: error: cast to pointer from integer of different size /build/buildd/qemu-kvm-0.14.0+spice-v35/hw/qxl.c: In function 'qxl_add_memslot': /build/buildd/qemu-kvm-0.14.0+spice-v35/hw/qxl.c:771:5: error: format '%llx' expects type 'long long unsigned int', but argument 5 has type 'long unsigned int' /build/buildd/qemu-kvm-0.14.0+spice-v35/hw/qxl.c:771:5: error: format '%llx' expects type 'long long unsigned int', but argument 6 has type 'long unsigned int' /build/buildd/qemu-kvm-0.14.0+spice-v35/hw/qxl.c: In function 'qxl_phys2virt': /build/buildd/qemu-kvm-0.14.0+spice-v35/hw/qxl.c:813:16: error: cast to pointer from integer of different size /build/buildd/qemu-kvm-0.14.0+spice-v35/hw/qxl.c: In function 'qxl_set_mode': /build/buildd/qemu-kvm-0.14.0+spice-v35/hw/qxl.c:892:5: error: format '%lx' expects type 'long unsigned int', but argument 8 has type 'uint64_t' /build/buildd/qemu-kvm-0.14.0+spice-v35/hw/qxl.c: In function 'qxl_map': /build/buildd/qemu-kvm-0.14.0+spice-v35/hw/qxl.c:1028:5: error: format '%lx' expects type 'long unsigned int', but argument 6 has type 'pcibus_t' /build/buildd/qemu-kvm-0.14.0+spice-v35/hw/qxl.c:1028:5: error: format '%lx' expects type 'long unsigned int', but argument 7 has type 'pcibus_t' make[2]: *** [qxl.o] Error 1 -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/723871 Title: qemu-kvm-0.14.0 Aborts with -vga qxl Status in QEMU: Confirmed Status in “libvirt” package in Ubuntu: Triaged Status in “qemu-kvm” package in Ubuntu: Fix Released Bug description: Host CPU is Core i7 Q820. KVM is from 2.6.35-gentoo-r5 kernel (x86_64). Host has spice-0.7.2 and spice-protocol-0.7.0. Guest is Windows XP SP3 with qxl driver 0.6.1, virtio-serial 1.1.6 and vdagent 0.6.3. qemu-kvm is started like so: qemu-system-x86_64 -cpu host -enable-kvm -pidfile /home/rick/qemu/hds/wxp.pid -drive file=/home/rick/qemu/hds/wxp.raw,if=virtio,media=disk,aio=native,snapshot=on -m 768 -name WinXP -net nic,model=virtio -net user -localtime -usb -vga qxl -device virtio-serial -chardev spicevmc,name=vdagent,id=vdagent -device virtserialport,chardev=vdagent,name=com.redhat.spice.0 -spice port=1234,disable-ticketing -monitor stdio and crashes with: qemu-system-x86_64: /home/rick/qemu/src/qemu-kvm-0.14.0/qemu-kvm.c:1724: kvm_mutex_unlock: Assertion `!cpu_single_env' failed. Aborted If I use -no-kvm, it works fine. If I use -vga std, it works fine. -enable-kvm and -vga qxl crashes.
Re: [Qemu-devel] qdev device documentation (Re: [PATCH 0/2] usb-linux: physical port handling.)
On 05/12/2011 10:56 AM, Markus Armbruster wrote: Gerd Hoffmannkra...@redhat.com writes: Hi, What is the status of the qdev documentation patches btw.? http://lists.gnu.org/archive/html/qemu-devel/2011-02/msg02169.html What is the problem with the empty strings btw? Anthony ;) 1) It doesn't help us get good written documentation without non-existent magic to extract a qdev schema. 2) It makes the documentation hidden in random places in the tree making it hard to audit what is undocumented. 3) It guarantees that documentation will vary drastically in language and quality. 4) I greatly fear that we're embarking down yet another introduce a new API that never gets fully converted path. I've sent a quick patch out with what I think is a significantly better approach. Regards, Anthony Liguori [...]
Re: [Qemu-devel] qdev device documentation (Re: [PATCH 0/2] usb-linux: physical port handling.)
Anthony Liguori anth...@codemonkey.ws writes: On 05/12/2011 10:25 AM, Gerd Hoffmann wrote: Hi, What is the status of the qdev documentation patches btw.? http://lists.gnu.org/archive/html/qemu-devel/2011-02/msg02169.html What is the problem with the empty strings btw? The only way around I can see is having _DOC and _NODOC versions for all the property macros, but I'd prefer to not have _NODOC macros in the tree ... Inline documentation is bad. Our documentation should be centralized. That's the only way to keep it consistent and thorough. External documentation of code details is bad. Our documentation should be next to the code. That's the only way to keep it up-to-date and consistent with the code. There's no way to easily extract the inline docs in a complete way since some devices are built conditionally. For each configured target: extract docs of the devices it builds Concatenate and discard the duplicates Yes, that means you don't get docs for devices none of your targets has. That's a feature. If you really want docs for all devices, build all targets.
Re: [Qemu-devel] qdev device documentation (Re: [PATCH 0/2] usb-linux: physical port handling.)
Anthony Liguori anth...@codemonkey.ws writes: On 05/12/2011 10:25 AM, Gerd Hoffmann wrote: Hi, What is the status of the qdev documentation patches btw.? http://lists.gnu.org/archive/html/qemu-devel/2011-02/msg02169.html What is the problem with the empty strings btw? The only way around I can see is having _DOC and _NODOC versions for all the property macros, but I'd prefer to not have _NODOC macros in the tree ... Here's an example of what I'm suggesting. I think we should just go with this and add better output as we go. But we need all of the qdev information.. not just a doc string for each property. Missing: make device_add ? show your device doc strings, and device_add NAME,? show your property doc strings. Missing: automated check qdev-doc.json matches the code. Keeping the docs far from the code is a bad idea even with such a check. [...]
Re: [Qemu-devel] qdev device documentation (Re: [PATCH 0/2] usb-linux: physical port handling.)
On 05/12/2011 11:08 AM, Markus Armbruster wrote: Anthony Liguorianth...@codemonkey.ws writes: On 05/12/2011 10:25 AM, Gerd Hoffmann wrote: Hi, What is the status of the qdev documentation patches btw.? http://lists.gnu.org/archive/html/qemu-devel/2011-02/msg02169.html What is the problem with the empty strings btw? The only way around I can see is having _DOC and _NODOC versions for all the property macros, but I'd prefer to not have _NODOC macros in the tree ... Inline documentation is bad. Our documentation should be centralized. That's the only way to keep it consistent and thorough. External documentation of code details is bad. Our documentation should be next to the code. That's the only way to keep it up-to-date and consistent with the code. qdev properties are *not* code details. It's a public user interface that we have to support for every. It should be disconnected from the internal implementation. And yes, the incestuous relationship that exists today is a problem, but it's one we're going to have to live with. There's no way to easily extract the inline docs in a complete way since some devices are built conditionally. For each configured target: extract docs of the devices it builds Concatenate and discard the duplicates Yes, that means you don't get docs for devices none of your targets has. That's a feature. If you really want docs for all devices, build all targets. But for things like Spice where the lack of libspice influences whether the device is available, how do I extract formal documentation to publish on qemu.org reliably? Regards, Anthony Liguori
Re: [Qemu-devel] qdev device documentation (Re: [PATCH 0/2] usb-linux: physical port handling.)
On 05/12/2011 11:18 AM, Markus Armbruster wrote: Anthony Liguorianth...@codemonkey.ws writes: On 05/12/2011 10:25 AM, Gerd Hoffmann wrote: Hi, What is the status of the qdev documentation patches btw.? http://lists.gnu.org/archive/html/qemu-devel/2011-02/msg02169.html What is the problem with the empty strings btw? The only way around I can see is having _DOC and _NODOC versions for all the property macros, but I'd prefer to not have _NODOC macros in the tree ... Here's an example of what I'm suggesting. I think we should just go with this and add better output as we go. But we need all of the qdev information.. not just a doc string for each property. Missing: make device_add ? show your device doc strings, and device_add NAME,? show your property doc strings. I really dislike overloading things for inline help. Introducing a device_help is fine and hopefully you realize that generating this is trivial. Missing: automated check qdev-doc.json matches the code. Keeping the docs far from the code is a bad idea even with such a check. I view this as a feature. What's documented is what's supported. Anything that isn't documented isn't supported. But yes, hopefully it's obvious that we can add automated checks to improve this. But if we're going to start somewhere, this is where I think we should start. Regards, Anthony Liguori [...]
Re: [Qemu-devel] [PATCH v3 3/6] block QMP: Drop query-block member type (type= in info block)
On Thu, 12 May 2011 17:05:12 +0200 Markus Armbruster arm...@redhat.com wrote: Its value is unreliable: a block device used as floppy has type floppy if created with if=floppy, but type hd if created with if=none. That's because with if=none, the type is at best a declaration of intent: the drive can be connected to any guest device. Its type is really the guest device's business. Reporting it here is wrong. It reports how the guest is using the device, right? I'd say that's what users/clients are interested in knowing. Also, we can't just drop it from QMP. We should first note it's deprecated.
Re: [Qemu-devel] [PATCH v3 3/6] block QMP: Drop query-block member type (type= in info block)
Luiz Capitulino lcapitul...@redhat.com writes: On Thu, 12 May 2011 17:05:12 +0200 Markus Armbruster arm...@redhat.com wrote: Its value is unreliable: a block device used as floppy has type floppy if created with if=floppy, but type hd if created with if=none. That's because with if=none, the type is at best a declaration of intent: the drive can be connected to any guest device. Its type is really the guest device's business. Reporting it here is wrong. It reports how the guest is using the device, right? I'd say that's what users/clients are interested in knowing. The value is *unreliable*. It may or may not match how the guest is using the device. I doubt users are interested in unreliable information. Also, we can't just drop it from QMP. We should first note it's deprecated. Would you accept a change to the more honest value unknown for the deprecation period?
Re: [Qemu-devel] qdev device documentation (Re: [PATCH 0/2] usb-linux: physical port handling.)
On 05/12/2011 11:18 AM, Markus Armbruster wrote:
Anthony Liguorianth...@codemonkey.ws writes:
On 05/12/2011 10:25 AM, Gerd Hoffmann wrote:
Hi,
What is the status of the qdev documentation patches btw.?
http://lists.gnu.org/archive/html/qemu-devel/2011-02/msg02169.html
What is the problem with the empty strings btw?
The only way around I can see is having _DOC and _NODOC versions for all
the property macros, but I'd prefer to not have _NODOC macros in the
tree ...
Here's an example of what I'm suggesting. I think we should just go
with this and add better output as we go.
But we need all of the qdev information.. not just a doc string for
each property.
Missing: make device_add ? show your device doc strings, and
device_add NAME,? show your property doc strings.
This is all it takes:
#!/usr/bin/python
import sys
data = sys.stdin.read()
docs = eval(data)
sys.stdout.write('DeviceStateDocumentation device_docs[] = {')
for item in docs:
sys.stdout.write('''
{
.name = %(device)s,
.properties = (PropertyDocumentation[])({''' % item)
for prop in item[properties]:
sys.stdout.write('''
{ %s, %s, %s },''' % (prop,
item[properties][prop]['type'], item[properties][prop]['doc']))
sys.stdout.write('''
{ },
},''')
sys.stdout.write('''
};
''')
Plus a little plumbing magic to add the actual command.
Missing: automated check qdev-doc.json matches the code. Keeping the
docs far from the code is a bad idea even with such a check.
If you walk the DeviceInfo list, you can validate that (1) each device
has a corresponding entry in device_docs (2) any field in device_docs is
present in device (3) the types match.
Regards,
Anthony Liguori
[...]
Re: [Qemu-devel] [PATCH v3 3/6] block QMP: Drop query-block member type (type= in info block)
On Thu, 12 May 2011 19:12:56 +0200 Markus Armbruster arm...@redhat.com wrote: Luiz Capitulino lcapitul...@redhat.com writes: On Thu, 12 May 2011 17:05:12 +0200 Markus Armbruster arm...@redhat.com wrote: Its value is unreliable: a block device used as floppy has type floppy if created with if=floppy, but type hd if created with if=none. That's because with if=none, the type is at best a declaration of intent: the drive can be connected to any guest device. Its type is really the guest device's business. Reporting it here is wrong. It reports how the guest is using the device, right? I'd say that's what users/clients are interested in knowing. The value is *unreliable*. It may or may not match how the guest is using the device. I doubt users are interested in unreliable information. Can't it be fixed? And how are users/clients supposed to find out how the guest is using its block devices? Also, we can't just drop it from QMP. We should first note it's deprecated. Would you accept a change to the more honest value unknown for the deprecation period? We have to avoid breaking the protocol. Changing something that has always been reported as 'cdrom' to 'unknown' will likely cause as many as damages as dropping the command. The best solution I can think of is noting in the documentation that the information is unreliable and explain what clients interested in knowing this info should do.
Re: [Qemu-devel] [PATCH] Add AACI audio playback support to the ARM Versatile/PB platform
On the other hand the current ac97.c implementation is a closely coupled combination of a PCI/ACLink bridge (Intel 82801AA) with a generic AC97 codec. This has prevent me to easily reuse this code. The milkymist-ac97 implementation is another case. It looks like a basic implementation with the AC97 registers directly mapped on the system bus. Using the ACLink bus I defined, it could be interesting to implement separately the PCI/ACLink bridge from ac97.c. Is it what you mean by saying this should be shared with the other AC97 devices ? Yes. The whole point of AClink is that it separates the host bridge from the codec. We now have at least three devices implementing this. Your aclink implementation is only used by one of these, which gives me little confidence it actually does what it claims. Paul
Re: [Qemu-devel] [PATCH v3 3/6] block QMP: Drop query-block member type (type= in info block)
Luiz Capitulino lcapitul...@redhat.com writes: On Thu, 12 May 2011 19:12:56 +0200 Markus Armbruster arm...@redhat.com wrote: Luiz Capitulino lcapitul...@redhat.com writes: On Thu, 12 May 2011 17:05:12 +0200 Markus Armbruster arm...@redhat.com wrote: Its value is unreliable: a block device used as floppy has type floppy if created with if=floppy, but type hd if created with if=none. That's because with if=none, the type is at best a declaration of intent: the drive can be connected to any guest device. Its type is really the guest device's business. Reporting it here is wrong. It reports how the guest is using the device, right? I'd say that's what users/clients are interested in knowing. The value is *unreliable*. It may or may not match how the guest is using the device. I doubt users are interested in unreliable information. Can't it be fixed? And how are users/clients supposed to find out how the guest is using its block devices? To find out more about the guest's devices, examine the guest's devices: info qtree. You don't expect to find the guest serial devices in in info chardev, either. query-block's type member is a mistake, because it mixes up guest device info with the host device info. Dropping it is a bug fix. The fact that its value is unreliable is merely icing on the cake. Also, we can't just drop it from QMP. We should first note it's deprecated. Would you accept a change to the more honest value unknown for the deprecation period? We have to avoid breaking the protocol. Changing something that has always been reported as 'cdrom' to 'unknown' will likely cause as many as damages as dropping the command. I can cause damage only if somebody is using it. Which I doubt. Remember, the value is unreliable. It's a *lie*. We can stop lying in two ways: shut up (drop member type), or tell the truth (change the value to unknown, which is a documented value of type). The best solution I can think of is noting in the documentation that the information is unreliable and explain what clients interested in knowing this info should do. I'd be much more willing to jump through compatibility hoops if there was *one* known user of this particular detail of QMP. But if you insist on us continuing to lie, I'll find a way to continue to lie. I'm resisting it, because I think it's a disservice to our users.
Re: [Qemu-devel] qdev device documentation (Re: [PATCH 0/2] usb-linux: physical port handling.)
Anthony Liguori anth...@codemonkey.ws writes: On 05/12/2011 11:08 AM, Markus Armbruster wrote: Anthony Liguorianth...@codemonkey.ws writes: On 05/12/2011 10:25 AM, Gerd Hoffmann wrote: Hi, What is the status of the qdev documentation patches btw.? http://lists.gnu.org/archive/html/qemu-devel/2011-02/msg02169.html What is the problem with the empty strings btw? The only way around I can see is having _DOC and _NODOC versions for all the property macros, but I'd prefer to not have _NODOC macros in the tree ... Inline documentation is bad. Our documentation should be centralized. That's the only way to keep it consistent and thorough. External documentation of code details is bad. Our documentation should be next to the code. That's the only way to keep it up-to-date and consistent with the code. qdev properties are *not* code details. It's a public user interface that we have to support for every. The fact that they are public user interface doesn't change the fact that they're code at all. They are *both*. It should be disconnected from the internal implementation. And yes, the incestuous relationship that exists today is a problem, but it's one we're going to have to live with. I doubt we'll ever reach consensus on this one. There's no way to easily extract the inline docs in a complete way since some devices are built conditionally. For each configured target: extract docs of the devices it builds Concatenate and discard the duplicates Yes, that means you don't get docs for devices none of your targets has. That's a feature. If you really want docs for all devices, build all targets. But for things like Spice where the lack of libspice influences whether the device is available, how do I extract formal documentation to publish on qemu.org reliably? If no maintainer of QEMU can build with Spice enabled, it got more serious problems than extracting its documentation.
Re: [Qemu-devel] qdev device documentation (Re: [PATCH 0/2] usb-linux: physical port handling.)
On 05/12/2011 12:58 PM, Markus Armbruster wrote: Anthony Liguorianth...@codemonkey.ws writes: On 05/12/2011 11:08 AM, Markus Armbruster wrote: Anthony Liguorianth...@codemonkey.ws writes: On 05/12/2011 10:25 AM, Gerd Hoffmann wrote: Hi, What is the status of the qdev documentation patches btw.? http://lists.gnu.org/archive/html/qemu-devel/2011-02/msg02169.html What is the problem with the empty strings btw? The only way around I can see is having _DOC and _NODOC versions for all the property macros, but I'd prefer to not have _NODOC macros in the tree ... Inline documentation is bad. Our documentation should be centralized. That's the only way to keep it consistent and thorough. External documentation of code details is bad. Our documentation should be next to the code. That's the only way to keep it up-to-date and consistent with the code. qdev properties are *not* code details. It's a public user interface that we have to support for every. The fact that they are public user interface doesn't change the fact that they're code at all. They are *both*. That's fine. But what better way to ensure a consistent and stable UI than having it centralized in one place. It should be disconnected from the internal implementation. And yes, the incestuous relationship that exists today is a problem, but it's one we're going to have to live with. I doubt we'll ever reach consensus on this one. I don't think that matters though. qdev properties are part of our UI and need to be stable. Just like we express our command line options centrally (with documentation) via qemu-options.hx, it seems reasonable to me to do it for qdev properties. I think the only downside is that we have to duplicate this the current DeviceInfo definitions but that's a harder problem that can be deferred for another day. Yes, that means you don't get docs for devices none of your targets has. That's a feature. If you really want docs for all devices, build all targets. But for things like Spice where the lack of libspice influences whether the device is available, how do I extract formal documentation to publish on qemu.org reliably? If no maintainer of QEMU can build with Spice enabled, it got more serious problems than extracting its documentation. It's not about Spice. But having documentation that is influenced by build options seems like a bad thing to me. But regardless, centralized documentation means more consistency in the documentation. I think this is a critically important point. Regards, Anthony Liguori
Re: [Qemu-devel] [PATCH v2 1/4] coroutine: introduce coroutines
On Thu, May 12, 2011 at 12:54 PM, Stefan Hajnoczi
stefa...@linux.vnet.ibm.com wrote:
From: Kevin Wolf kw...@redhat.com
Asynchronous code is becoming very complex. At the same time
synchronous code is growing because it is convenient to write.
Sometimes duplicate code paths are even added, one synchronous and the
other asynchronous. This patch introduces coroutines which allow code
that looks synchronous but is asynchronous under the covers.
A coroutine has its own stack and is therefore able to preserve state
across blocking operations, which traditionally require callback
functions and manual marshalling of parameters.
Creating and starting a coroutine is easy:
coroutine = qemu_coroutine_create(my_coroutine);
qemu_coroutine_enter(coroutine, my_data);
The coroutine then executes until it returns or yields:
void coroutine_fn my_coroutine(void *opaque) {
MyData *my_data = opaque;
/* do some work */
qemu_coroutine_yield();
/* do some more work */
}
Yielding switches control back to the caller of qemu_coroutine_enter().
This is typically used to switch back to the main thread's event loop
after issuing an asynchronous I/O request. The request callback will
then invoke qemu_coroutine_enter() once more to switch back to the
coroutine.
Note that coroutines never execute concurrently and should only be used
from threads which hold the global mutex. This restriction makes
programming with coroutines easier than with threads. Race conditions
cannot occur since only one coroutine may be active at any time. Other
coroutines can only run across yield.
This coroutines implementation is based on the gtk-vnc implementation
written by Anthony Liguori anth...@codemonkey.ws but it has been
significantly rewritten by Kevin Wolf kw...@redhat.com to use
setjmp()/longjmp() instead of the more expensive swapcontext().
Signed-off-by: Kevin Wolf kw...@redhat.com
Signed-off-by: Stefan Hajnoczi stefa...@linux.vnet.ibm.com
---
Makefile.objs | 7 +++
coroutine-ucontext.c | 73 +
coroutine-win32.c | 57 +++
qemu-coroutine-int.h | 53 +
qemu-coroutine.c | 125
++
qemu-coroutine.h | 82 +
trace-events | 5 ++
7 files changed, 402 insertions(+), 0 deletions(-)
create mode 100644 coroutine-ucontext.c
create mode 100644 coroutine-win32.c
create mode 100644 qemu-coroutine-int.h
create mode 100644 qemu-coroutine.c
create mode 100644 qemu-coroutine.h
diff --git a/Makefile.objs b/Makefile.objs
index 9d8851e..cba6c2b 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -11,6 +11,12 @@ oslib-obj-$(CONFIG_WIN32) += oslib-win32.o
oslib-obj-$(CONFIG_POSIX) += oslib-posix.o
###
+# coroutines
+coroutine-obj-y = qemu-coroutine.o
+coroutine-obj-$(CONFIG_POSIX) += coroutine-ucontext.o
+coroutine-obj-$(CONFIG_WIN32) += coroutine-win32.o
+
+###
# block-obj-y is code used by both qemu system emulation and qemu-img
block-obj-y = cutils.o cache-utils.o qemu-malloc.o qemu-option.o module.o
async.o
@@ -67,6 +73,7 @@ common-obj-y += readline.o console.o cursor.o qemu-error.o
common-obj-y += $(oslib-obj-y)
common-obj-$(CONFIG_WIN32) += os-win32.o
common-obj-$(CONFIG_POSIX) += os-posix.o
+common-obj-y += $(coroutine-obj-y)
common-obj-y += tcg-runtime.o host-utils.o
common-obj-y += irq.o ioport.o input.o
diff --git a/coroutine-ucontext.c b/coroutine-ucontext.c
new file mode 100644
index 000..3b14ebf
--- /dev/null
+++ b/coroutine-ucontext.c
@@ -0,0 +1,73 @@
+/*
+ * ucontext coroutine initialization code
+ *
+ * Copyright (C) 2006 Anthony Liguori anth...@codemonkey.ws
+ * Copyright (C) 2011 Kevin Wolf kw...@redhat.com
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.0 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301
USA
Please use the web link version.
+ */
+
+/* XXX Is there a nicer way to disable glibc's stack check for longjmp? */
What is the problem?
+#ifdef _FORTIFY_SOURCE
+#undef _FORTIFY_SOURCE
+#endif
+#include setjmp.h
Re: [Qemu-devel] qdev device documentation (Re: [PATCH 0/2] usb-linux: physical port handling.)
On 12 May 2011 19:58, Markus Armbruster arm...@redhat.com wrote: Anthony Liguori anth...@codemonkey.ws writes: But for things like Spice where the lack of libspice influences whether the device is available, how do I extract formal documentation to publish on qemu.org reliably? If no maintainer of QEMU can build with Spice enabled, it got more serious problems than extracting its documentation. The point isn't that you can arrange to build with option Foo enabled but that if the build environment changes accidentally there's not much warning that the official docs have suddenly lost some devices. The spice probe in configure will barf if you said --enable-spice and spice wasn't found, but I bet not all configure checks that influence availability of a device do that. And since there isn't currently an '--enable-all' option to configure the docs builder would have to keep track of every new --enable-foo switch and add it to the configure command... Which looks pretty ugly and not very reliable to me. -- PMM
Re: [Qemu-devel] qdev device documentation (Re: [PATCH 0/2] usb-linux: physical port handling.)
Anthony Liguori anth...@codemonkey.ws writes: On 05/12/2011 11:18 AM, Markus Armbruster wrote: Anthony Liguorianth...@codemonkey.ws writes: On 05/12/2011 10:25 AM, Gerd Hoffmann wrote: Hi, What is the status of the qdev documentation patches btw.? http://lists.gnu.org/archive/html/qemu-devel/2011-02/msg02169.html What is the problem with the empty strings btw? The only way around I can see is having _DOC and _NODOC versions for all the property macros, but I'd prefer to not have _NODOC macros in the tree ... Here's an example of what I'm suggesting. I think we should just go with this and add better output as we go. But we need all of the qdev information.. not just a doc string for each property. Missing: make device_add ? show your device doc strings, and device_add NAME,? show your property doc strings. I really dislike overloading things for inline help. Introducing a device_help is fine and hopefully you realize that generating this is trivial. I never liked device_add ? myself, but it's what we got. I'd support replacing it with something more decent. [...]
Re: [Qemu-devel] [PATCH v3 3/6] block QMP: Drop query-block member type (type= in info block)
On Thu, 12 May 2011 19:54:40 +0200 Markus Armbruster arm...@redhat.com wrote: Luiz Capitulino lcapitul...@redhat.com writes: On Thu, 12 May 2011 19:12:56 +0200 Markus Armbruster arm...@redhat.com wrote: Luiz Capitulino lcapitul...@redhat.com writes: On Thu, 12 May 2011 17:05:12 +0200 Markus Armbruster arm...@redhat.com wrote: Its value is unreliable: a block device used as floppy has type floppy if created with if=floppy, but type hd if created with if=none. That's because with if=none, the type is at best a declaration of intent: the drive can be connected to any guest device. Its type is really the guest device's business. Reporting it here is wrong. It reports how the guest is using the device, right? I'd say that's what users/clients are interested in knowing. The value is *unreliable*. It may or may not match how the guest is using the device. I doubt users are interested in unreliable information. Can't it be fixed? And how are users/clients supposed to find out how the guest is using its block devices? To find out more about the guest's devices, examine the guest's devices: info qtree. Which is not converted to QMP yet. You don't expect to find the guest serial devices in in info chardev, either. query-block's type member is a mistake, because it mixes up guest device info with the host device info. Dropping it is a bug fix. I understand why you're dropping it, what I don't want to do is to break working clients. For example, there might be clients out there using it in a way that it's expected to work (eg. if=floppy). Of course that I'm assuming that such a client exist. The fact that its value is unreliable is merely icing on the cake. Also, we can't just drop it from QMP. We should first note it's deprecated. Would you accept a change to the more honest value unknown for the deprecation period? We have to avoid breaking the protocol. Changing something that has always been reported as 'cdrom' to 'unknown' will likely cause as many as damages as dropping the command. I can cause damage only if somebody is using it. Which I doubt. Me too and I'd agree with this patch if I was 100% sure. But it's impossible to be sure, unless we do it by trial and error which is harmful. Remember, the value is unreliable. It's a *lie*. We can stop lying in two ways: shut up (drop member type), or tell the truth (change the value to unknown, which is a documented value of type). Can we set it to 'unknown' when if=none? The best solution I can think of is noting in the documentation that the information is unreliable and explain what clients interested in knowing this info should do. I'd be much more willing to jump through compatibility hoops if there was *one* known user of this particular detail of QMP. Ideally, yes, but it's the type of thing we'll never know. But if you insist on us continuing to lie, I'll find a way to continue to lie. I'm resisting it, because I think it's a disservice to our users. I also want to do the best for our users and I don't want to ignore the bug, but we don't know whether there are clients using the field. If we drop it and a client does use it, then we'll have failed in doing the best.
Re: [Qemu-devel] [PATCH v2 1/4] coroutine: introduce coroutines
On Thu, May 12, 2011 at 7:12 PM, Blue Swirl blauwir...@gmail.com wrote: On Thu, May 12, 2011 at 12:54 PM, Stefan Hajnoczi stefa...@linux.vnet.ibm.com wrote: diff --git a/coroutine-ucontext.c b/coroutine-ucontext.c new file mode 100644 index 000..3b14ebf --- /dev/null +++ b/coroutine-ucontext.c @@ -0,0 +1,73 @@ +/* + * ucontext coroutine initialization code + * + * Copyright (C) 2006 Anthony Liguori anth...@codemonkey.ws + * Copyright (C) 2011 Kevin Wolf kw...@redhat.com + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.0 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA Please use the web link version. Will update in v3. + */ + +/* XXX Is there a nicer way to disable glibc's stack check for longjmp? */ What is the problem? Kevin? +static __thread Coroutine leader; $ cat thread.c static __thread int sigusr1_wfd; $ gcc thread.c -c -pthread thread.c:1: error: thread-local storage not supported for this target $ gcc -v Reading specs from /usr/bin/../lib/gcc-lib/sparc64-unknown-openbsd4.9/4.2.1/specs Target: sparc64-unknown-openbsd4.9 Configured with: OpenBSD/sparc64 system compiler Thread model: posix gcc version 4.2.1 20070719 We had the thread-local variables in as part of the gtk-vnc history of this code. In QEMU we don't need it: Note that coroutines never execute concurrently and should only be used from threads which hold the global mutex. So I will remove the thread-local attribute. Thank you for testing that platform. diff --git a/trace-events b/trace-events index 4f965e2..2d4db05 100644 --- a/trace-events +++ b/trace-events @@ -361,3 +361,8 @@ disable milkymist_uart_pulse_irq_tx(void) Pulse IRQ TX # hw/milkymist-vgafb.c disable milkymist_vgafb_memory_read(uint32_t addr, uint32_t value) addr %08x value %08x disable milkymist_vgafb_memory_write(uint32_t addr, uint32_t value) addr %08x value %08x + +# qemu-coroutine.c +qemu_coroutine_enter(void *from, void *to, void *opaque) from %p to %p opaque %p +qemu_coroutine_yield(void *from, void *to) from %p to %p +qemu_coroutine_terminate(void *co) self %p Not disabled? Good catch. Stefan
Re: [Qemu-devel] qdev device documentation (Re: [PATCH 0/2] usb-linux: physical port handling.)
On Thu, May 12, 2011 at 08:15:43PM +0200, Peter Maydell wrote: On 12 May 2011 19:58, Markus Armbruster arm...@redhat.com wrote: Anthony Liguori anth...@codemonkey.ws writes: But for things like Spice where the lack of libspice influences whether the device is available, how do I extract formal documentation to publish on qemu.org reliably? If no maintainer of QEMU can build with Spice enabled, it got more serious problems than extracting its documentation. The point isn't that you can arrange to build with option Foo enabled but that if the build environment changes accidentally there's not much warning that the official docs have suddenly lost some devices. The spice probe in configure will barf if you said --enable-spice and spice wasn't found, but I bet not all configure checks that influence availability of a device do that. And since there isn't currently an '--enable-all' option to configure the docs builder would have to keep track of every new --enable-foo switch and add it to the configure command... Which looks pretty ugly and not very reliable to me. We could have a (just picking up the Spice example) spice-docs file that was checked in and updated periodically by generation by the maintainer. It's error prone, you could still update the source but forget to checkin the spice-docs and get an old version, but at least it would still be auto-generated, and it would not prevent the docs builder from working. -- PMM
Re: [Qemu-devel] qdev device documentation (Re: [PATCH 0/2] usb-linux: physical port handling.)
On 05/12/2011 02:32 PM, Alon Levy wrote: We could have a (just picking up the Spice example) spice-docs file that was checked in and updated periodically by generation by the maintainer. It's error prone, you could still update the source but forget to checkin the spice-docs and get an old version, but at least it would still be auto-generated, and it would not prevent the docs builder from working. It's not just Spice though. There are still a number of devices that only get built with certain targets. Doing it right is actually fairly complicated. It's far easier to use a central doc file, and then add mechanisms to ensure things don't get out of sync. Regards, Anthony Liguori -- PMM
[Qemu-devel] [PATCH 05/25] [virtio-9p] Move errno into v9fs_do_readlink
Signed-off-by: Venkateswararao Jujjuri jv...@linux.vnet.ibm.com
---
hw/9pfs/virtio-9p.c | 32
1 files changed, 16 insertions(+), 16 deletions(-)
diff --git a/hw/9pfs/virtio-9p.c b/hw/9pfs/virtio-9p.c
index c4d903a..a748c34 100644
--- a/hw/9pfs/virtio-9p.c
+++ b/hw/9pfs/virtio-9p.c
@@ -82,19 +82,21 @@ static int v9fs_do_lstat(V9fsState *s, V9fsString *path,
struct stat *stbuf)
return s-ops-lstat(s-ctx, path-data, stbuf);
}
-static ssize_t v9fs_do_readlink(V9fsState *s, V9fsString *path, V9fsString
*buf)
+static int v9fs_do_readlink(V9fsState *s, V9fsString *path, V9fsString *buf,
+ssize_t *len)
{
-ssize_t len;
-
+int err;
buf-data = qemu_malloc(1024);
-len = s-ops-readlink(s-ctx, path-data, buf-data, 1024 - 1);
-if (len -1) {
-buf-size = len;
-buf-data[len] = 0;
+*len = s-ops-readlink(s-ctx, path-data, buf-data, 1024 - 1);
+if (*len -1) {
+buf-size = *len;
+buf-data[*len] = 0;
+err = 0;
+} else {
+err = -errno;
}
-
-return len;
+return err;
}
static int v9fs_do_close(V9fsState *s, int fd)
@@ -1055,13 +1057,11 @@ static int stat_to_v9stat(V9fsState *s, V9fsString
*name,
v9fs_string_null(v9stat-extension);
if (v9stat-mode P9_STAT_MODE_SYMLINK) {
-err = v9fs_do_readlink(s, name, v9stat-extension);
-if (err == -1) {
-err = -errno;
+ssize_t symlink_len;
+err = v9fs_do_readlink(s, name, v9stat-extension, symlink_len);
+if (err 0) {
return err;
}
-v9stat-extension.data[err] = 0;
-v9stat-extension.size = err;
} else if (v9stat-mode P9_STAT_MODE_DEVICE) {
v9fs_string_sprintf(v9stat-extension, %c %u %u,
S_ISCHR(stbuf-st_mode) ? 'c' : 'b',
@@ -3645,6 +3645,7 @@ static void v9fs_readlink(void *opaque)
int32_t fid;
int err = 0;
V9fsFidState *fidp;
+ssize_t symlink_len;
pdu_unmarshal(copdu-pdu, offset, d, fid);
fidp = lookup_fid(copdu-s, fid);
@@ -3654,9 +3655,8 @@ static void v9fs_readlink(void *opaque)
}
v9fs_string_init(target);
-err = v9fs_do_readlink(copdu-s, fidp-path, target);
+err = v9fs_do_readlink(copdu-s, fidp-path, target, symlink_len);
if (err 0) {
-err = -errno;
goto out;
}
offset += pdu_marshal(copdu-pdu, offset, s, target);
--
1.7.1
[Qemu-devel] [PATCH 03/25] [virtio-9p] Remove post functions for v9fs_readlink.
In the process of preparation for coroutine threads, remove all post functions
and make the function more readable.
Signed-off-by: Venkateswararao Jujjuri jv...@linux.vnet.ibm.com
---
hw/9pfs/virtio-9p.c | 25 +++--
1 files changed, 7 insertions(+), 18 deletions(-)
diff --git a/hw/9pfs/virtio-9p.c b/hw/9pfs/virtio-9p.c
index e308e9b..0b38868 100644
--- a/hw/9pfs/virtio-9p.c
+++ b/hw/9pfs/virtio-9p.c
@@ -3637,21 +3637,6 @@ out:
qemu_free(copdu);
}
-static void v9fs_readlink_post_readlink(V9fsState *s, V9fsReadLinkState *vs,
-int err)
-{
-if (err 0) {
-err = -errno;
-goto out;
-}
-vs-offset += pdu_marshal(vs-pdu, vs-offset, s, vs-target);
-err = vs-offset;
-out:
-complete_pdu(s, vs-pdu, err);
-v9fs_string_free(vs-target);
-qemu_free(vs);
-}
-
static void v9fs_readlink(void *opaque)
{
V9fsCoPdu *copdu = opaque;
@@ -3667,7 +3652,6 @@ static void v9fs_readlink(void *opaque)
vs-offset = 7;
pdu_unmarshal(vs-pdu, vs-offset, d, fid);
-
fidp = lookup_fid(s, fid);
if (fidp == NULL) {
err = -ENOENT;
@@ -3676,8 +3660,13 @@ static void v9fs_readlink(void *opaque)
v9fs_string_init(vs-target);
err = v9fs_do_readlink(s, fidp-path, vs-target);
-v9fs_readlink_post_readlink(s, vs, err);
-return;
+if (err 0) {
+err = -errno;
+goto out;
+}
+vs-offset += pdu_marshal(vs-pdu, vs-offset, s, vs-target);
+err = vs-offset;
+v9fs_string_free(vs-target);
out:
complete_pdu(s, vs-pdu, err);
qemu_free(vs);
--
1.7.1
[Qemu-devel] [PATCH 04/25] [virtio-9p] clean up v9fs_readlink.
Rearrange the code so that we can avoid V9fsReadLinkState.
Signed-off-by: Venkateswararao Jujjuri jv...@linux.vnet.ibm.com
---
hw/9pfs/virtio-9p.c | 26 ++
hw/9pfs/virtio-9p.h |7 ---
2 files changed, 10 insertions(+), 23 deletions(-)
diff --git a/hw/9pfs/virtio-9p.c b/hw/9pfs/virtio-9p.c
index 0b38868..c4d903a 100644
--- a/hw/9pfs/virtio-9p.c
+++ b/hw/9pfs/virtio-9p.c
@@ -3640,36 +3640,30 @@ out:
static void v9fs_readlink(void *opaque)
{
V9fsCoPdu *copdu = opaque;
-V9fsState *s = copdu-s;
-V9fsPDU *pdu = copdu-pdu;
+size_t offset = 7;
+V9fsString target;
int32_t fid;
-V9fsReadLinkState *vs;
int err = 0;
V9fsFidState *fidp;
-vs = qemu_malloc(sizeof(*vs));
-vs-pdu = pdu;
-vs-offset = 7;
-
-pdu_unmarshal(vs-pdu, vs-offset, d, fid);
-fidp = lookup_fid(s, fid);
+pdu_unmarshal(copdu-pdu, offset, d, fid);
+fidp = lookup_fid(copdu-s, fid);
if (fidp == NULL) {
err = -ENOENT;
goto out;
}
-v9fs_string_init(vs-target);
-err = v9fs_do_readlink(s, fidp-path, vs-target);
+v9fs_string_init(target);
+err = v9fs_do_readlink(copdu-s, fidp-path, target);
if (err 0) {
err = -errno;
goto out;
}
-vs-offset += pdu_marshal(vs-pdu, vs-offset, s, vs-target);
-err = vs-offset;
-v9fs_string_free(vs-target);
+offset += pdu_marshal(copdu-pdu, offset, s, target);
+err = offset;
+v9fs_string_free(target);
out:
-complete_pdu(s, vs-pdu, err);
-qemu_free(vs);
+complete_pdu(copdu-s, copdu-pdu, err);
qemu_free(copdu);
}
diff --git a/hw/9pfs/virtio-9p.h b/hw/9pfs/virtio-9p.h
index 5021da8..bf8f64b 100644
--- a/hw/9pfs/virtio-9p.h
+++ b/hw/9pfs/virtio-9p.h
@@ -493,13 +493,6 @@ typedef struct V9fsGetlockState
V9fsGetlock *glock;
} V9fsGetlockState;
-typedef struct V9fsReadLinkState
-{
-V9fsPDU *pdu;
-size_t offset;
-V9fsString target;
-} V9fsReadLinkState;
-
size_t pdu_packunpack(void *addr, struct iovec *sg, int sg_count,
size_t offset, size_t size, int pack);
--
1.7.1
[Qemu-devel] [PATCH 02/25] [virtio-9p] Change all pdu handlers to coroutines.
This patch changes the top level handlers to coroutines and sets the base.
It will be followed up with series of patches to convert all filesystem
calls to threaded coroutines pushing all blocking clals in VirtFS out
of vcpu threads.
Signed-off-by: Venkateswararao Jujjuri jv...@linux.vnet.ibm.com
---
hw/9pfs/virtio-9p-coth.h |7 ++
hw/9pfs/virtio-9p.c | 194 --
hw/9pfs/virtio-9p.h |2 +-
3 files changed, 161 insertions(+), 42 deletions(-)
diff --git a/hw/9pfs/virtio-9p-coth.h b/hw/9pfs/virtio-9p-coth.h
index fdc44f6..2ec1401 100644
--- a/hw/9pfs/virtio-9p-coth.h
+++ b/hw/9pfs/virtio-9p-coth.h
@@ -17,6 +17,7 @@
#include qemu-thread.h
#include qemu-coroutine.h
+#include virtio-9p.h
#include glib.h
typedef struct V9fsRequest {
@@ -34,6 +35,12 @@ typedef struct V9fsThPool {
int wfd;
} V9fsThPool;
+typedef struct V9fsCoPdu {
+V9fsPDU *pdu;
+V9fsState *s;
+Coroutine *coroutine;
+} V9fsCoPdu;
+
/* v9fs glib thread pool */
extern V9fsThPool v9fs_pool;
diff --git a/hw/9pfs/virtio-9p.c b/hw/9pfs/virtio-9p.c
index ec97b10..e308e9b 100644
--- a/hw/9pfs/virtio-9p.c
+++ b/hw/9pfs/virtio-9p.c
@@ -19,6 +19,7 @@
#include fsdev/qemu-fsdev.h
#include virtio-9p-debug.h
#include virtio-9p-xattr.h
+#include virtio-9p-coth.h
int debug_9p_pdu;
@@ -1192,8 +1193,11 @@ static void v9fs_fix_path(V9fsString *dst, V9fsString
*src, int len)
v9fs_string_free(str);
}
-static void v9fs_version(V9fsState *s, V9fsPDU *pdu)
+static void v9fs_version(void *opaque)
{
+V9fsCoPdu *copdu = opaque;
+V9fsState *s = copdu-s;
+V9fsPDU *pdu = copdu-pdu;
V9fsString version;
size_t offset = 7;
@@ -1211,10 +1215,15 @@ static void v9fs_version(V9fsState *s, V9fsPDU *pdu)
complete_pdu(s, pdu, offset);
v9fs_string_free(version);
+qemu_free(copdu);
+return;
}
-static void v9fs_attach(V9fsState *s, V9fsPDU *pdu)
+static void v9fs_attach(void *opaque)
{
+V9fsCoPdu *copdu = opaque;
+V9fsState *s = copdu-s;
+V9fsPDU *pdu = copdu-pdu;
int32_t fid, afid, n_uname;
V9fsString uname, aname;
V9fsFidState *fidp;
@@ -1247,6 +1256,7 @@ out:
complete_pdu(s, pdu, err);
v9fs_string_free(uname);
v9fs_string_free(aname);
+qemu_free(copdu);
}
static void v9fs_stat_post_lstat(V9fsState *s, V9fsStatState *vs, int err)
@@ -1269,8 +1279,11 @@ out:
qemu_free(vs);
}
-static void v9fs_stat(V9fsState *s, V9fsPDU *pdu)
+static void v9fs_stat(void *opaque)
{
+V9fsCoPdu *copdu = opaque;
+V9fsState *s = copdu-s;
+V9fsPDU *pdu = copdu-pdu;
int32_t fid;
V9fsStatState *vs;
ssize_t err = 0;
@@ -1297,6 +1310,7 @@ out:
complete_pdu(s, vs-pdu, err);
v9fs_stat_free(vs-v9stat);
qemu_free(vs);
+qemu_free(copdu);
}
static void v9fs_getattr_post_lstat(V9fsState *s, V9fsStatStateDotl *vs,
@@ -1316,8 +1330,11 @@ out:
qemu_free(vs);
}
-static void v9fs_getattr(V9fsState *s, V9fsPDU *pdu)
+static void v9fs_getattr(void *opaque)
{
+V9fsCoPdu *copdu = opaque;
+V9fsState *s = copdu-s;
+V9fsPDU *pdu = copdu-pdu;
int32_t fid;
V9fsStatStateDotl *vs;
ssize_t err = 0;
@@ -1348,6 +1365,7 @@ static void v9fs_getattr(V9fsState *s, V9fsPDU *pdu)
out:
complete_pdu(s, vs-pdu, err);
qemu_free(vs);
+qemu_free(copdu);
}
/* From Linux kernel code */
@@ -1465,8 +1483,11 @@ out:
qemu_free(vs);
}
-static void v9fs_setattr(V9fsState *s, V9fsPDU *pdu)
+static void v9fs_setattr(void *opaque)
{
+V9fsCoPdu *copdu = opaque;
+V9fsState *s = copdu-s;
+V9fsPDU *pdu = copdu-pdu;
int32_t fid;
V9fsSetattrState *vs;
int err = 0;
@@ -1493,6 +1514,7 @@ static void v9fs_setattr(V9fsState *s, V9fsPDU *pdu)
out:
complete_pdu(s, vs-pdu, err);
qemu_free(vs);
+qemu_free(copdu);
}
static void v9fs_walk_complete(V9fsState *s, V9fsWalkState *vs, int err)
@@ -1579,8 +1601,11 @@ out:
v9fs_walk_complete(s, vs, err);
}
-static void v9fs_walk(V9fsState *s, V9fsPDU *pdu)
+static void v9fs_walk(void *opaque)
{
+V9fsCoPdu *copdu = opaque;
+V9fsState *s = copdu-s;
+V9fsPDU *pdu = copdu-pdu;
int32_t fid, newfid;
V9fsWalkState *vs;
int err = 0;
@@ -1658,6 +1683,7 @@ static void v9fs_walk(V9fsState *s, V9fsPDU *pdu)
err = vs-offset;
out:
v9fs_walk_complete(s, vs, err);
+qemu_free(copdu);
}
static int32_t get_iounit(V9fsState *s, V9fsString *name)
@@ -1751,8 +1777,11 @@ out:
qemu_free(vs);
}
-static void v9fs_open(V9fsState *s, V9fsPDU *pdu)
+static void v9fs_open(void *opaque)
{
+V9fsCoPdu *copdu = opaque;
+V9fsState *s = copdu-s;
+V9fsPDU *pdu = copdu-pdu;
int32_t fid;
V9fsOpenState *vs;
ssize_t err = 0;
@@ -1783,6 +1812,7 @@ static void v9fs_open(V9fsState *s, V9fsPDU *pdu)
out:
complete_pdu(s, pdu, err);
qemu_free(vs);
+qemu_free(copdu);
}
static void
[Qemu-devel] [0/25] Async threading for VirtFS using glib threads coroutines.
VirtFS (fileserver base on 9P) performs many blocking system calls in the vCPU context. This effort is to move the blocking calls out of vCPU/IO thread context, into asynchronous threads. Anthony's Add hard build dependency on glib patch and Kevin/Stefan's coroutine effort is a prerequisite. This patch set contains: - Converting all 9pfs calls into coroutines. - Each 9P operation will be modified for: - Remove post* functions. These are our call back functions which makes the code very hard to read. Now with coroutines, we can achieve the same state machine model with nice sequential code flow. - Move errno access near to the local_syscall() - Introduce asynchronous threading This series has the basic infrastructure and few routines like mkdir,monod,unlink,readdir,xattr,lstat, etc converted. Currently we are working on converting and testing other 9P operations also into this model and those patches will follow shortly. Removing callback functions made some of the patches little lengthy. Here is the git tree for the reviewer convenience. http://repo.or.cz/w/qemu/aliguori/jvrao.git/shortlog/refs/heads/9p-coroutines-round1 -Signed-off-by: Venkateswararao Jujjuri jv...@linux.vnet.ibm.com
[Qemu-devel] [PATCH 14/25] hw/9pfs: Update v9fs_getattr to use coroutines
From: Aneesh Kumar K.V aneesh.ku...@linux.vnet.ibm.com
Signed-off-by: Aneesh Kumar K.V aneesh.ku...@linux.vnet.ibm.com
Signed-off-by: Venkateswararao Jujjuri jv...@linux.vnet.ibm.com
---
hw/9pfs/virtio-9p.c | 59 +-
hw/9pfs/virtio-9p.h |8 ---
2 files changed, 20 insertions(+), 47 deletions(-)
diff --git a/hw/9pfs/virtio-9p.c b/hw/9pfs/virtio-9p.c
index 86e9482..fa2bb1f 100644
--- a/hw/9pfs/virtio-9p.c
+++ b/hw/9pfs/virtio-9p.c
@@ -1088,7 +1088,7 @@ static int stat_to_v9stat(V9fsState *s, V9fsString *name,
static void stat_to_v9stat_dotl(V9fsState *s, const struct stat *stbuf,
-V9fsStatDotl *v9lstat)
+V9fsStatDotl *v9lstat)
{
memset(v9lstat, 0, sizeof(*v9lstat));
@@ -1291,58 +1291,39 @@ out:
qemu_free(copdu);
}
-static void v9fs_getattr_post_lstat(V9fsState *s, V9fsStatStateDotl *vs,
-int err)
-{
-if (err == -1) {
-err = -errno;
-goto out;
-}
-
-stat_to_v9stat_dotl(s, vs-stbuf, vs-v9stat_dotl);
-vs-offset += pdu_marshal(vs-pdu, vs-offset, A, vs-v9stat_dotl);
-err = vs-offset;
-
-out:
-complete_pdu(s, vs-pdu, err);
-qemu_free(vs);
-}
-
static void v9fs_getattr(void *opaque)
{
-V9fsCoPdu *copdu = opaque;
-V9fsState *s = copdu-s;
-V9fsPDU *pdu = copdu-pdu;
int32_t fid;
-V9fsStatStateDotl *vs;
-ssize_t err = 0;
+size_t offset = 7;
+ssize_t retval = 0;
+struct stat stbuf;
V9fsFidState *fidp;
uint64_t request_mask;
+V9fsStatDotl v9stat_dotl;
+V9fsCoPdu *copdu = opaque;
+V9fsState *s = copdu-s;
+V9fsPDU *pdu = copdu-pdu;
-vs = qemu_malloc(sizeof(*vs));
-vs-pdu = pdu;
-vs-offset = 7;
-
-memset(vs-v9stat_dotl, 0, sizeof(vs-v9stat_dotl));
-
-pdu_unmarshal(vs-pdu, vs-offset, dq, fid, request_mask);
+pdu_unmarshal(pdu, offset, dq, fid, request_mask);
fidp = lookup_fid(s, fid);
if (fidp == NULL) {
-err = -ENOENT;
+retval = -ENOENT;
goto out;
}
-
-/* Currently we only support BASIC fields in stat, so there is no
+/*
+ * Currently we only support BASIC fields in stat, so there is no
* need to look at request_mask.
*/
-err = v9fs_do_lstat(s, fidp-path, vs-stbuf);
-v9fs_getattr_post_lstat(s, vs, err);
-return;
-
+retval = v9fs_co_lstat(s, fidp-path, stbuf);
+if (retval 0) {
+goto out;
+}
+stat_to_v9stat_dotl(s, stbuf, v9stat_dotl);
+retval = offset;
+retval += pdu_marshal(pdu, offset, A, v9stat_dotl);
out:
-complete_pdu(s, vs-pdu, err);
-qemu_free(vs);
+complete_pdu(s, pdu, retval);
qemu_free(copdu);
}
diff --git a/hw/9pfs/virtio-9p.h b/hw/9pfs/virtio-9p.h
index ee8cb79..7e4bea0 100644
--- a/hw/9pfs/virtio-9p.h
+++ b/hw/9pfs/virtio-9p.h
@@ -276,14 +276,6 @@ typedef struct V9fsStatDotl {
uint64_t st_data_version;
} V9fsStatDotl;
-typedef struct V9fsStatStateDotl {
-V9fsPDU *pdu;
-size_t offset;
-V9fsStatDotl v9stat_dotl;
-struct stat stbuf;
-} V9fsStatStateDotl;
-
-
typedef struct V9fsWalkState {
V9fsPDU *pdu;
size_t offset;
--
1.7.1
[Qemu-devel] [PATCH 07/25] [virtio-9p] Remove post functions for v9fs_mkdir.
Signed-off-by: Venkateswararao Jujjuri jv...@linux.vnet.ibm.com
---
hw/9pfs/virtio-9p.c | 52 --
1 files changed, 13 insertions(+), 39 deletions(-)
diff --git a/hw/9pfs/virtio-9p.c b/hw/9pfs/virtio-9p.c
index 7ef6ad8..af0143d 100644
--- a/hw/9pfs/virtio-9p.c
+++ b/hw/9pfs/virtio-9p.c
@@ -3357,40 +3357,6 @@ out:
qemu_free(copdu);
}
-static void v9fs_mkdir_post_lstat(V9fsState *s, V9fsMkState *vs, int err)
-{
-if (err == -1) {
-err = -errno;
-goto out;
-}
-
-stat_to_qid(vs-stbuf, vs-qid);
-vs-offset += pdu_marshal(vs-pdu, vs-offset, Q, vs-qid);
-err = vs-offset;
-out:
-complete_pdu(s, vs-pdu, err);
-v9fs_string_free(vs-fullname);
-v9fs_string_free(vs-name);
-qemu_free(vs);
-}
-
-static void v9fs_mkdir_post_mkdir(V9fsState *s, V9fsMkState *vs, int err)
-{
-if (err == -1) {
-err = -errno;
-goto out;
-}
-
-err = v9fs_do_lstat(s, vs-fullname, vs-stbuf);
-v9fs_mkdir_post_lstat(s, vs, err);
-return;
-out:
-complete_pdu(s, vs-pdu, err);
-v9fs_string_free(vs-fullname);
-v9fs_string_free(vs-name);
-qemu_free(vs);
-}
-
static void v9fs_mkdir(void *opaque)
{
V9fsCoPdu *copdu = opaque;
@@ -3409,19 +3375,27 @@ static void v9fs_mkdir(void *opaque)
v9fs_string_init(vs-fullname);
pdu_unmarshal(vs-pdu, vs-offset, dsdd, fid, vs-name, mode,
-gid);
+ gid);
fidp = lookup_fid(s, fid);
if (fidp == NULL) {
err = -ENOENT;
goto out;
}
-
v9fs_string_sprintf(vs-fullname, %s/%s, fidp-path.data,
vs-name.data);
err = v9fs_do_mkdir(s, vs-fullname.data, mode, fidp-uid, gid);
-v9fs_mkdir_post_mkdir(s, vs, err);
-return;
-
+if (err == -1) {
+err = -errno;
+goto out;
+}
+err = v9fs_do_lstat(s, vs-fullname, vs-stbuf);
+if (err == -1) {
+err = -errno;
+goto out;
+}
+stat_to_qid(vs-stbuf, vs-qid);
+vs-offset += pdu_marshal(vs-pdu, vs-offset, Q, vs-qid);
+err = vs-offset;
out:
complete_pdu(s, vs-pdu, err);
v9fs_string_free(vs-fullname);
--
1.7.1
[Qemu-devel] [PATCH 16/25] hw/9pfs: Update v9fs_setattr to use coroutines
From: Aneesh Kumar K.V aneesh.ku...@linux.vnet.ibm.com
Signed-off-by: Aneesh Kumar K.V aneesh.ku...@linux.vnet.ibm.com
Signed-off-by: Venkateswararao Jujjuri jv...@linux.vnet.ibm.com
---
hw/9pfs/virtio-9p.c | 165 +--
hw/9pfs/virtio-9p.h |8 ---
2 files changed, 55 insertions(+), 118 deletions(-)
diff --git a/hw/9pfs/virtio-9p.c b/hw/9pfs/virtio-9p.c
index fa2bb1f..8723039 100644
--- a/hw/9pfs/virtio-9p.c
+++ b/hw/9pfs/virtio-9p.c
@@ -1339,140 +1339,85 @@ out:
#define ATTR_ATIME_SET (1 7)
#define ATTR_MTIME_SET (1 8)
-static void v9fs_setattr_post_truncate(V9fsState *s, V9fsSetattrState *vs,
- int err)
-{
-if (err == -1) {
-err = -errno;
-goto out;
-}
-err = vs-offset;
-
-out:
-complete_pdu(s, vs-pdu, err);
-qemu_free(vs);
-}
-
-static void v9fs_setattr_post_chown(V9fsState *s, V9fsSetattrState *vs, int
err)
+static void v9fs_setattr(void *opaque)
{
-if (err == -1) {
-err = -errno;
-goto out;
-}
-
-if (vs-v9iattr.valid (ATTR_SIZE)) {
-err = v9fs_do_truncate(s, vs-fidp-path, vs-v9iattr.size);
-}
-v9fs_setattr_post_truncate(s, vs, err);
-return;
+int err = 0;
+int32_t fid;
+V9fsFidState *fidp;
+size_t offset = 7;
+V9fsIattr v9iattr;
+V9fsCoPdu *copdu = opaque;
+V9fsState *s = copdu-s;
+V9fsPDU *pdu = copdu-pdu;
-out:
-complete_pdu(s, vs-pdu, err);
-qemu_free(vs);
-}
+pdu_unmarshal(pdu, offset, dI, fid, v9iattr);
-static void v9fs_setattr_post_utimensat(V9fsState *s, V9fsSetattrState *vs,
- int err)
-{
-if (err == -1) {
-err = -errno;
+fidp = lookup_fid(s, fid);
+if (fidp == NULL) {
+err = -EINVAL;
goto out;
}
-
-/* If the only valid entry in iattr is ctime we can call
- * chown(-1,-1) to update the ctime of the file
- */
-if ((vs-v9iattr.valid (ATTR_UID | ATTR_GID)) ||
-((vs-v9iattr.valid ATTR_CTIME)
- !((vs-v9iattr.valid ATTR_MASK) ~ATTR_CTIME))) {
-if (!(vs-v9iattr.valid ATTR_UID)) {
-vs-v9iattr.uid = -1;
-}
-if (!(vs-v9iattr.valid ATTR_GID)) {
-vs-v9iattr.gid = -1;
+if (v9iattr.valid ATTR_MODE) {
+err = v9fs_co_chmod(s, fidp-path, v9iattr.mode);
+if (err 0) {
+goto out;
}
-err = v9fs_do_chown(s, vs-fidp-path, vs-v9iattr.uid,
-vs-v9iattr.gid);
}
-v9fs_setattr_post_chown(s, vs, err);
-return;
-
-out:
-complete_pdu(s, vs-pdu, err);
-qemu_free(vs);
-}
-
-static void v9fs_setattr_post_chmod(V9fsState *s, V9fsSetattrState *vs, int
err)
-{
-if (err == -1) {
-err = -errno;
-goto out;
-}
-
-if (vs-v9iattr.valid (ATTR_ATIME | ATTR_MTIME)) {
+if (v9iattr.valid (ATTR_ATIME | ATTR_MTIME)) {
struct timespec times[2];
-if (vs-v9iattr.valid ATTR_ATIME) {
-if (vs-v9iattr.valid ATTR_ATIME_SET) {
-times[0].tv_sec = vs-v9iattr.atime_sec;
-times[0].tv_nsec = vs-v9iattr.atime_nsec;
+if (v9iattr.valid ATTR_ATIME) {
+if (v9iattr.valid ATTR_ATIME_SET) {
+times[0].tv_sec = v9iattr.atime_sec;
+times[0].tv_nsec = v9iattr.atime_nsec;
} else {
times[0].tv_nsec = UTIME_NOW;
}
} else {
times[0].tv_nsec = UTIME_OMIT;
}
-
-if (vs-v9iattr.valid ATTR_MTIME) {
-if (vs-v9iattr.valid ATTR_MTIME_SET) {
-times[1].tv_sec = vs-v9iattr.mtime_sec;
-times[1].tv_nsec = vs-v9iattr.mtime_nsec;
+if (v9iattr.valid ATTR_MTIME) {
+if (v9iattr.valid ATTR_MTIME_SET) {
+times[1].tv_sec = v9iattr.mtime_sec;
+times[1].tv_nsec = v9iattr.mtime_nsec;
} else {
times[1].tv_nsec = UTIME_NOW;
}
} else {
times[1].tv_nsec = UTIME_OMIT;
}
-err = v9fs_do_utimensat(s, vs-fidp-path, times);
+err = v9fs_co_utimensat(s, fidp-path, times);
+if (err 0) {
+goto out;
+}
}
-v9fs_setattr_post_utimensat(s, vs, err);
-return;
-
-out:
-complete_pdu(s, vs-pdu, err);
-qemu_free(vs);
-}
-
-static void v9fs_setattr(void *opaque)
-{
-V9fsCoPdu *copdu = opaque;
-V9fsState *s = copdu-s;
-V9fsPDU *pdu = copdu-pdu;
-int32_t fid;
-V9fsSetattrState *vs;
-int err = 0;
-
-vs = qemu_malloc(sizeof(*vs));
-vs-pdu = pdu;
-vs-offset = 7;
-
-pdu_unmarshal(pdu, vs-offset, dI, fid, vs-v9iattr);
-
-vs-fidp = lookup_fid(s, fid);
-if (vs-fidp == NULL) {
-err = -EINVAL;
-goto out;
+/*
+ * If the
[Qemu-devel] [PATCH 12/25] hw/9pfs: Update v9fs_statfs to use coroutines
From: Aneesh Kumar K.V aneesh.ku...@linux.vnet.ibm.com
Signed-off-by: Aneesh Kumar K.V aneesh.ku...@linux.vnet.ibm.com
Signed-off-by: Venkateswararao Jujjuri jv...@linux.vnet.ibm.com
---
hw/9pfs/virtio-9p.c | 92 --
hw/9pfs/virtio-9p.h | 22
2 files changed, 44 insertions(+), 70 deletions(-)
diff --git a/hw/9pfs/virtio-9p.c b/hw/9pfs/virtio-9p.c
index 148382d..86e9482 100644
--- a/hw/9pfs/virtio-9p.c
+++ b/hw/9pfs/virtio-9p.c
@@ -3075,80 +3075,76 @@ out:
qemu_free(copdu);
}
-static void v9fs_statfs_post_statfs(V9fsState *s, V9fsStatfsState *vs, int err)
-{
+static int v9fs_fill_statfs(V9fsState *s, V9fsPDU *pdu, struct statfs *stbuf)
+{
+uint32_t f_type;
+uint32_t f_bsize;
+uint64_t f_blocks;
+uint64_t f_bfree;
+uint64_t f_bavail;
+uint64_t f_files;
+uint64_t f_ffree;
+uint64_t fsid_val;
+uint32_t f_namelen;
+size_t offset = 7;
int32_t bsize_factor;
-if (err) {
-err = -errno;
-goto out;
-}
-
/*
* compute bsize factor based on host file system block size
* and client msize
*/
-bsize_factor = (s-msize - P9_IOHDRSZ)/vs-stbuf.f_bsize;
+bsize_factor = (s-msize - P9_IOHDRSZ)/stbuf-f_bsize;
if (!bsize_factor) {
bsize_factor = 1;
}
-vs-v9statfs.f_type = vs-stbuf.f_type;
-vs-v9statfs.f_bsize = vs-stbuf.f_bsize;
-vs-v9statfs.f_bsize *= bsize_factor;
+f_type = stbuf-f_type;
+f_bsize = stbuf-f_bsize;
+f_bsize *= bsize_factor;
/*
* f_bsize is adjusted(multiplied) by bsize factor, so we need to
* adjust(divide) the number of blocks, free blocks and available
* blocks by bsize factor
*/
-vs-v9statfs.f_blocks = vs-stbuf.f_blocks/bsize_factor;
-vs-v9statfs.f_bfree = vs-stbuf.f_bfree/bsize_factor;
-vs-v9statfs.f_bavail = vs-stbuf.f_bavail/bsize_factor;
-vs-v9statfs.f_files = vs-stbuf.f_files;
-vs-v9statfs.f_ffree = vs-stbuf.f_ffree;
-vs-v9statfs.fsid_val = (unsigned int) vs-stbuf.f_fsid.__val[0] |
- (unsigned long long)vs-stbuf.f_fsid.__val[1] 32;
-vs-v9statfs.f_namelen = vs-stbuf.f_namelen;
-
-vs-offset += pdu_marshal(vs-pdu, vs-offset, ddqqd,
- vs-v9statfs.f_type, vs-v9statfs.f_bsize, vs-v9statfs.f_blocks,
- vs-v9statfs.f_bfree, vs-v9statfs.f_bavail, vs-v9statfs.f_files,
- vs-v9statfs.f_ffree, vs-v9statfs.fsid_val,
- vs-v9statfs.f_namelen);
+f_blocks = stbuf-f_blocks/bsize_factor;
+f_bfree = stbuf-f_bfree/bsize_factor;
+f_bavail = stbuf-f_bavail/bsize_factor;
+f_files = stbuf-f_files;
+f_ffree = stbuf-f_ffree;
+fsid_val = (unsigned int) stbuf-f_fsid.__val[0] |
+ (unsigned long long)stbuf-f_fsid.__val[1] 32;
+f_namelen = stbuf-f_namelen;
-out:
-complete_pdu(s, vs-pdu, vs-offset);
-qemu_free(vs);
+return pdu_marshal(pdu, offset, ddqqd,
+ f_type, f_bsize, f_blocks, f_bfree,
+ f_bavail, f_files, f_ffree,
+ fsid_val, f_namelen);
}
static void v9fs_statfs(void *opaque)
{
+int32_t fid;
+ssize_t retval = 0;
+size_t offset = 7;
+V9fsFidState *fidp;
+struct statfs stbuf;
V9fsCoPdu *copdu = opaque;
V9fsState *s = copdu-s;
V9fsPDU *pdu = copdu-pdu;
-V9fsStatfsState *vs;
-ssize_t err = 0;
-vs = qemu_malloc(sizeof(*vs));
-vs-pdu = pdu;
-vs-offset = 7;
-
-memset(vs-v9statfs, 0, sizeof(vs-v9statfs));
-
-pdu_unmarshal(vs-pdu, vs-offset, d, vs-fid);
-
-vs-fidp = lookup_fid(s, vs-fid);
-if (vs-fidp == NULL) {
-err = -ENOENT;
+pdu_unmarshal(pdu, offset, d, fid);
+fidp = lookup_fid(s, fid);
+if (fidp == NULL) {
+retval = -ENOENT;
goto out;
}
-
-err = v9fs_do_statfs(s, vs-fidp-path, vs-stbuf);
-v9fs_statfs_post_statfs(s, vs, err);
-return;
-
+retval = v9fs_co_statfs(s, fidp-path, stbuf);
+if (retval 0) {
+goto out;
+}
+retval = offset;
+retval += v9fs_fill_statfs(s, pdu, stbuf);
out:
-complete_pdu(s, vs-pdu, err);
-qemu_free(vs);
+complete_pdu(s, pdu, retval);
qemu_free(copdu);
return;
}
diff --git a/hw/9pfs/virtio-9p.h b/hw/9pfs/virtio-9p.h
index bf8f64b..ee8cb79 100644
--- a/hw/9pfs/virtio-9p.h
+++ b/hw/9pfs/virtio-9p.h
@@ -396,28 +396,6 @@ struct virtio_9p_config
uint8_t tag[0];
} __attribute__((packed));
-typedef struct V9fsStatfs
-{
-uint32_t f_type;
-uint32_t f_bsize;
-uint64_t f_blocks;
-uint64_t f_bfree;
-uint64_t f_bavail;
-uint64_t f_files;
-uint64_t f_ffree;
-uint64_t fsid_val;
-uint32_t f_namelen;
-} V9fsStatfs;
-
-typedef struct V9fsStatfsState {
-V9fsPDU *pdu;
-size_t offset;
-int32_t fid;
-V9fsStatfs v9statfs;
-V9fsFidState *fidp;
-struct statfs stbuf;
-} V9fsStatfsState;
-
typedef struct V9fsMkState {
V9fsPDU
[Qemu-devel] [PATCH 01/25] [virtio-9p] Add infrastructure to support glib threads and coroutines.
This patch is originally made by Arun Bharadwaj for glib support.
Later Harsh Prateek Bora added coroutines support.
Signed-off-by: Arun R Bharadwaj a...@linux.vnet.ibm.com
Signed-off-by: Harsh Prateek Bora ha...@linux.vnet.ibm.com
Signed-off-by: Venkateswararao Jujjuri jv...@linux.vnet.ibm.com
---
Makefile.objs |2 +
hw/9pfs/virtio-9p-coth.c | 68
hw/9pfs/virtio-9p-coth.h | 45 +
hw/9pfs/virtio-9p-device.c | 26 +++-
hw/9pfs/virtio-9p.h|1 -
5 files changed, 139 insertions(+), 3 deletions(-)
create mode 100644 hw/9pfs/virtio-9p-coth.c
create mode 100644 hw/9pfs/virtio-9p-coth.h
diff --git a/Makefile.objs b/Makefile.objs
index 3873f10..96f6a24 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -297,8 +297,10 @@ hw-obj-$(CONFIG_SOUND) += $(sound-obj-y)
9pfs-nested-$(CONFIG_VIRTFS) = virtio-9p.o virtio-9p-debug.o
9pfs-nested-$(CONFIG_VIRTFS) += virtio-9p-local.o virtio-9p-xattr.o
9pfs-nested-$(CONFIG_VIRTFS) += virtio-9p-xattr-user.o virtio-9p-posix-acl.o
+9pfs-nested-$(CONFIG_VIRTFS) += virtio-9p-coth.o
hw-obj-$(CONFIG_REALLY_VIRTFS) += $(addprefix 9pfs/, $(9pfs-nested-y))
+$(addprefix 9pfs/, $(9pfs-nested-y)): QEMU_CFLAGS+=$(GLIB_CFLAGS)
##
diff --git a/hw/9pfs/virtio-9p-coth.c b/hw/9pfs/virtio-9p-coth.c
new file mode 100644
index 000..edd3cde
--- /dev/null
+++ b/hw/9pfs/virtio-9p-coth.c
@@ -0,0 +1,68 @@
+/*
+ * Virtio 9p backend
+ *
+ * Copyright IBM, Corp. 2010
+ *
+ * Authors:
+ * Harsh Prateek Bora ha...@linux.vnet.ibm.com
+ * Venkateswararao Jujjuri(JV) jv...@linux.vnet.ibm.com
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2. See
+ * the COPYING file in the top-level directory.
+ *
+ */
+
+#include fsdev/qemu-fsdev.h
+#include qemu-thread.h
+#include qemu-coroutine.h
+#include virtio-9p-coth.h
+
+/* v9fs glib thread pool */
+V9fsThPool v9fs_pool;
+
+void v9fs_qemu_submit_request(V9fsRequest *req)
+{
+V9fsThPool *p = v9fs_pool;
+
+req-done = 0;
+p-requests = g_list_append(p-requests, req);
+g_thread_pool_push(v9fs_pool.pool, req, NULL);
+}
+
+void v9fs_qemu_process_req_done(void *arg)
+{
+struct V9fsThPool *p = v9fs_pool;
+char byte;
+ssize_t len;
+GList *cur_req, *next_req;
+
+do {
+len = read(p-rfd, byte, sizeof(byte));
+} while (len == -1 errno == EINTR);
+
+for (cur_req = p-requests; cur_req != NULL; cur_req = next_req) {
+V9fsRequest *req = cur_req-data;
+next_req = g_list_next(cur_req);
+
+if (!req-done) {
+continue;
+}
+
+Coroutine *entry = req-coroutine;
+qemu_coroutine_enter(entry, NULL);
+
+p-requests = g_list_delete_link(p-requests, cur_req);
+}
+}
+
+void v9fs_thread_routine(gpointer data, gpointer user_data)
+{
+V9fsRequest *req = data;
+char byte = 0;
+ssize_t len;
+req-func(req);
+req-done = 1;
+do {
+len = write(v9fs_pool.wfd, byte, sizeof(byte));
+} while (len == -1 errno == EINTR);
+}
diff --git a/hw/9pfs/virtio-9p-coth.h b/hw/9pfs/virtio-9p-coth.h
new file mode 100644
index 000..fdc44f6
--- /dev/null
+++ b/hw/9pfs/virtio-9p-coth.h
@@ -0,0 +1,45 @@
+/*
+ * Virtio 9p backend
+ *
+ * Copyright IBM, Corp. 2010
+ *
+ * Authors:
+ * Harsh Prateek Bora ha...@linux.vnet.ibm.com
+ * Venkateswararao Jujjuri(JV) jv...@linux.vnet.ibm.com
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2. See
+ * the COPYING file in the top-level directory.
+ *
+ */
+
+#ifndef _QEMU_VIRTIO_9P_COTH_H
+#define _QEMU_VIRTIO_9P_COTH_H
+
+#include qemu-thread.h
+#include qemu-coroutine.h
+#include glib.h
+
+typedef struct V9fsRequest {
+void (*func)(struct V9fsRequest *req);
+
+/* Flag to indicate that request is satisfied, ready for post-processing */
+int done;
+Coroutine *coroutine;
+} V9fsRequest;
+
+typedef struct V9fsThPool {
+GThreadPool *pool;
+GList *requests;
+int rfd;
+int wfd;
+} V9fsThPool;
+
+/* v9fs glib thread pool */
+extern V9fsThPool v9fs_pool;
+
+extern void v9fs_thread_routine(gpointer data, gpointer user_data);
+extern void v9fs_qemu_process_req_done(void *arg);
+extern void v9fs_qemu_submit_request(V9fsRequest *req);
+
+
+#endif
diff --git a/hw/9pfs/virtio-9p-device.c b/hw/9pfs/virtio-9p-device.c
index a2b6acc..21fb310 100644
--- a/hw/9pfs/virtio-9p-device.c
+++ b/hw/9pfs/virtio-9p-device.c
@@ -18,6 +18,9 @@
#include virtio-9p.h
#include fsdev/qemu-fsdev.h
#include virtio-9p-xattr.h
+#include virtio-9p-coth.h
+
+static int notifier_fds[2];
static uint32_t virtio_9p_get_features(VirtIODevice *vdev, uint32_t features)
{
@@ -49,14 +52,13 @@ VirtIODevice *virtio_9p_init(DeviceState *dev, V9fsConf
*conf)
int i, len;
struct stat stat;
FsTypeEntry *fse;
-
+V9fsThPool *p = v9fs_pool;
s = (V9fsState
[Qemu-devel] [PATCH 18/25] hw/9pfs: Update v9fs_xattrwalk to coroutines
From: Aneesh Kumar K.V aneesh.ku...@linux.vnet.ibm.com
Signed-off-by: Aneesh Kumar K.V aneesh.ku...@linux.vnet.ibm.com
Signed-off-by: Venkateswararao Jujjuri jv...@linux.vnet.ibm.com
---
hw/9pfs/virtio-9p.c | 198 ---
1 files changed, 63 insertions(+), 135 deletions(-)
diff --git a/hw/9pfs/virtio-9p.c b/hw/9pfs/virtio-9p.c
index 8723039..b50ac3c 100644
--- a/hw/9pfs/virtio-9p.c
+++ b/hw/9pfs/virtio-9p.c
@@ -240,21 +240,6 @@ static int v9fs_do_statfs(V9fsState *s, V9fsString *path,
struct statfs *stbuf)
return s-ops-statfs(s-ctx, path-data, stbuf);
}
-static ssize_t v9fs_do_lgetxattr(V9fsState *s, V9fsString *path,
- V9fsString *xattr_name,
- void *value, size_t size)
-{
-return s-ops-lgetxattr(s-ctx, path-data,
- xattr_name-data, value, size);
-}
-
-static ssize_t v9fs_do_llistxattr(V9fsState *s, V9fsString *path,
- void *value, size_t size)
-{
-return s-ops-llistxattr(s-ctx, path-data,
- value, size);
-}
-
static int v9fs_do_lsetxattr(V9fsState *s, V9fsString *path,
V9fsString *xattr_name,
void *value, size_t size, int flags)
@@ -3289,150 +3274,93 @@ out:
qemu_free(copdu);
}
-static void v9fs_post_xattr_getvalue(V9fsState *s, V9fsXattrState *vs, int err)
-{
-
-if (err 0) {
-err = -errno;
-free_fid(s, vs-xattr_fidp-fid);
-goto out;
-}
-vs-offset += pdu_marshal(vs-pdu, vs-offset, q, vs-size);
-err = vs-offset;
-out:
-complete_pdu(s, vs-pdu, err);
-v9fs_string_free(vs-name);
-qemu_free(vs);
-return;
-}
-
-static void v9fs_post_xattr_check(V9fsState *s, V9fsXattrState *vs, ssize_t
err)
-{
-if (err 0) {
-err = -errno;
-free_fid(s, vs-xattr_fidp-fid);
-goto out;
-}
-/*
- * Read the xattr value
- */
-vs-xattr_fidp-fs.xattr.len = vs-size;
-vs-xattr_fidp-fid_type = P9_FID_XATTR;
-vs-xattr_fidp-fs.xattr.copied_len = -1;
-if (vs-size) {
-vs-xattr_fidp-fs.xattr.value = qemu_malloc(vs-size);
-err = v9fs_do_lgetxattr(s, vs-xattr_fidp-path,
-vs-name, vs-xattr_fidp-fs.xattr.value,
-vs-xattr_fidp-fs.xattr.len);
-}
-v9fs_post_xattr_getvalue(s, vs, err);
-return;
-out:
-complete_pdu(s, vs-pdu, err);
-v9fs_string_free(vs-name);
-qemu_free(vs);
-}
-
-static void v9fs_post_lxattr_getvalue(V9fsState *s,
- V9fsXattrState *vs, int err)
-{
-if (err 0) {
-err = -errno;
-free_fid(s, vs-xattr_fidp-fid);
-goto out;
-}
-vs-offset += pdu_marshal(vs-pdu, vs-offset, q, vs-size);
-err = vs-offset;
-out:
-complete_pdu(s, vs-pdu, err);
-v9fs_string_free(vs-name);
-qemu_free(vs);
-return;
-}
-
-static void v9fs_post_lxattr_check(V9fsState *s,
- V9fsXattrState *vs, ssize_t err)
-{
-if (err 0) {
-err = -errno;
-free_fid(s, vs-xattr_fidp-fid);
-goto out;
-}
-/*
- * Read the xattr value
- */
-vs-xattr_fidp-fs.xattr.len = vs-size;
-vs-xattr_fidp-fid_type = P9_FID_XATTR;
-vs-xattr_fidp-fs.xattr.copied_len = -1;
-if (vs-size) {
-vs-xattr_fidp-fs.xattr.value = qemu_malloc(vs-size);
-err = v9fs_do_llistxattr(s, vs-xattr_fidp-path,
- vs-xattr_fidp-fs.xattr.value,
- vs-xattr_fidp-fs.xattr.len);
-}
-v9fs_post_lxattr_getvalue(s, vs, err);
-return;
-out:
-complete_pdu(s, vs-pdu, err);
-v9fs_string_free(vs-name);
-qemu_free(vs);
-}
-
static void v9fs_xattrwalk(void *opaque)
{
+int64_t size;
+V9fsString name;
+ssize_t err = 0;
+size_t offset = 7;
+int32_t fid, newfid;
+V9fsFidState *file_fidp;
+V9fsFidState *xattr_fidp;
V9fsCoPdu *copdu = opaque;
V9fsState *s = copdu-s;
V9fsPDU *pdu = copdu-pdu;
-ssize_t err = 0;
-V9fsXattrState *vs;
-int32_t fid, newfid;
-vs = qemu_malloc(sizeof(*vs));
-vs-pdu = pdu;
-vs-offset = 7;
-
-pdu_unmarshal(vs-pdu, vs-offset, dds, fid, newfid, vs-name);
-vs-file_fidp = lookup_fid(s, fid);
-if (vs-file_fidp == NULL) {
+pdu_unmarshal(pdu, offset, dds, fid, newfid, name);
+file_fidp = lookup_fid(s, fid);
+if (file_fidp == NULL) {
err = -ENOENT;
goto out;
}
-
-vs-xattr_fidp = alloc_fid(s, newfid);
-if (vs-xattr_fidp == NULL) {
+xattr_fidp = alloc_fid(s, newfid);
+if (xattr_fidp == NULL) {
err = -EINVAL;
goto out;
}
-
-v9fs_string_copy(vs-xattr_fidp-path, vs-file_fidp-path);
-if (vs-name.data[0] == 0) {
+v9fs_string_copy(xattr_fidp-path, file_fidp-path);
+if (name.data[0] == 0) {
[Qemu-devel] [PATCH 10/25] hw/9pfs: Update v9fs_readdir to use coroutines
From: Aneesh Kumar K.V aneesh.ku...@linux.vnet.ibm.com
Signed-off-by: Aneesh Kumar K.V aneesh.ku...@linux.vnet.ibm.com
Signed-off-by: Venkateswararao Jujjuri jv...@linux.vnet.ibm.com
---
hw/9pfs/virtio-9p.c | 170 +--
1 files changed, 69 insertions(+), 101 deletions(-)
diff --git a/hw/9pfs/virtio-9p.c b/hw/9pfs/virtio-9p.c
index 883eced..148382d 100644
--- a/hw/9pfs/virtio-9p.c
+++ b/hw/9pfs/virtio-9p.c
@@ -112,11 +112,6 @@ static off_t v9fs_do_telldir(V9fsState *s, DIR *dir)
return s-ops-telldir(s-ctx, dir);
}
-static struct dirent *v9fs_do_readdir(V9fsState *s, DIR *dir)
-{
-return s-ops-readdir(s-ctx, dir);
-}
-
static void v9fs_do_seekdir(V9fsState *s, DIR *dir, off_t off)
{
return s-ops-seekdir(s-ctx, dir, off);
@@ -1988,7 +1983,7 @@ static void v9fs_read_post_dir_lstat(V9fsState *s,
V9fsReadState *vs,
v9fs_stat_free(vs-v9stat);
v9fs_string_free(vs-name);
vs-dir_pos = vs-dent-d_off;
-vs-dent = v9fs_do_readdir(s, vs-fidp-fs.dir);
+v9fs_co_readdir(s, vs-fidp, vs-dent);
v9fs_read_post_readdir(s, vs, err);
return;
out:
@@ -2020,7 +2015,7 @@ static void v9fs_read_post_readdir(V9fsState *s,
V9fsReadState *vs, ssize_t err)
static void v9fs_read_post_telldir(V9fsState *s, V9fsReadState *vs, ssize_t
err)
{
-vs-dent = v9fs_do_readdir(s, vs-fidp-fs.dir);
+v9fs_co_readdir(s, vs-fidp, vs-dent);
v9fs_read_post_readdir(s, vs, err);
return;
}
@@ -2151,127 +2146,100 @@ out:
qemu_free(copdu);
}
-typedef struct V9fsReadDirState {
-V9fsPDU *pdu;
-V9fsFidState *fidp;
-V9fsQID qid;
-off_t saved_dir_pos;
-struct dirent *dent;
-int32_t count;
-int32_t max_count;
-size_t offset;
-int64_t initial_offset;
-V9fsString name;
-} V9fsReadDirState;
-
-static void v9fs_readdir_post_seekdir(V9fsState *s, V9fsReadDirState *vs)
-{
-vs-offset += pdu_marshal(vs-pdu, vs-offset, d, vs-count);
-vs-offset += vs-count;
-complete_pdu(s, vs-pdu, vs-offset);
-qemu_free(vs);
-return;
-}
-
-/* Size of each dirent on the wire: size of qid (13) + size of offset (8)
+/*
+ * Size of each dirent on the wire: size of qid (13) + size of offset (8)
* size of type (1) + size of name.size (2) + strlen(name.data)
*/
-#define V9_READDIR_DATA_SZ (24 + strlen(vs-name.data))
+#define V9_READDIR_DATA_SZ (24 + strlen(name.data))
-static void v9fs_readdir_post_readdir(V9fsState *s, V9fsReadDirState *vs)
+static int v9fs_do_readdir(V9fsState *s, V9fsPDU *pdu,
+ V9fsFidState *fidp, int32_t max_count)
{
-int len;
size_t size;
+V9fsQID qid;
+V9fsString name;
+int len, err = 0;
+int32_t count = 0;
+off_t saved_dir_pos;
+struct dirent *dent;
-if (vs-dent) {
-v9fs_string_init(vs-name);
-v9fs_string_sprintf(vs-name, %s, vs-dent-d_name);
-
-if ((vs-count + V9_READDIR_DATA_SZ) vs-max_count) {
+/* save the directory position */
+saved_dir_pos = v9fs_co_telldir(s, fidp);
+if (saved_dir_pos 0) {
+return saved_dir_pos;
+}
+while (1) {
+err = v9fs_co_readdir(s, fidp, dent);
+if (err || !dent) {
+break;
+}
+v9fs_string_init(name);
+v9fs_string_sprintf(name, %s, dent-d_name);
+if ((count + V9_READDIR_DATA_SZ) max_count) {
/* Ran out of buffer. Set dir back to old position and return */
-v9fs_do_seekdir(s, vs-fidp-fs.dir, vs-saved_dir_pos);
-v9fs_readdir_post_seekdir(s, vs);
-return;
+v9fs_co_seekdir(s, fidp, saved_dir_pos);
+v9fs_string_free(name);
+return count;
}
-
-/* Fill up just the path field of qid because the client uses
+/*
+ * Fill up just the path field of qid because the client uses
* only that. To fill the entire qid structure we will have
* to stat each dirent found, which is expensive
*/
-size = MIN(sizeof(vs-dent-d_ino), sizeof(vs-qid.path));
-memcpy(vs-qid.path, vs-dent-d_ino, size);
+size = MIN(sizeof(dent-d_ino), sizeof(qid.path));
+memcpy(qid.path, dent-d_ino, size);
/* Fill the other fields with dummy values */
-vs-qid.type = 0;
-vs-qid.version = 0;
-
-len = pdu_marshal(vs-pdu, vs-offset+4+vs-count, Qqbs,
- vs-qid, vs-dent-d_off,
- vs-dent-d_type, vs-name);
-vs-count += len;
-v9fs_string_free(vs-name);
-vs-saved_dir_pos = vs-dent-d_off;
-vs-dent = v9fs_do_readdir(s, vs-fidp-fs.dir);
-v9fs_readdir_post_readdir(s, vs);
-return;
-}
-
-vs-offset += pdu_marshal(vs-pdu, vs-offset, d, vs-count);
-vs-offset += vs-count;
-complete_pdu(s, vs-pdu, vs-offset);
-qemu_free(vs);
-return;
-}
-
-static void v9fs_readdir_post_telldir(V9fsState *s, V9fsReadDirState
[Qemu-devel] [PATCH 20/25] hw/9pfs: Add yield support to mknod coroutine
From: Aneesh Kumar K.V aneesh.ku...@linux.vnet.ibm.com
Signed-off-by: Aneesh Kumar K.V aneesh.ku...@linux.vnet.ibm.com
Signed-off-by: Venkateswararao Jujjuri jv...@linux.vnet.ibm.com
---
hw/9pfs/cofs.c | 45 +
hw/9pfs/virtio-9p-coth.h |2 ++
2 files changed, 47 insertions(+), 0 deletions(-)
diff --git a/hw/9pfs/cofs.c b/hw/9pfs/cofs.c
index 02eb0ba..06127f7 100644
--- a/hw/9pfs/cofs.c
+++ b/hw/9pfs/cofs.c
@@ -224,3 +224,48 @@ int v9fs_co_truncate(V9fsState *s, V9fsString *path, off_t
size)
qemu_coroutine_yield();
return vs.err;
}
+
+typedef struct V9fsThMknodState {
+int err;
+uid_t uid;
+gid_t gid;
+mode_t mode;
+dev_t dev;
+V9fsState *s;
+V9fsString *path;
+struct stat *stbuf;
+V9fsRequest request;
+} V9fsThMknodState;
+
+static void v9fs_th_do_mknod(V9fsRequest *request)
+{
+FsCred cred;
+V9fsThMknodState *vsp = container_of(request, V9fsThMknodState,
+ request);
+cred_init(cred);
+cred.fc_uid = vsp-uid;
+cred.fc_gid = vsp-gid;
+cred.fc_mode = vsp-mode;
+cred.fc_rdev = vsp-dev;
+vsp-err = vsp-s-ops-mknod(vsp-s-ctx, vsp-path-data, cred);
+if (vsp-err 0) {
+vsp-err = -errno;
+}
+}
+
+int v9fs_co_mknod(V9fsState *s, V9fsString *path, uid_t uid,
+ gid_t gid, dev_t dev, mode_t mode)
+{
+V9fsThMknodState vs;
+vs.s = s;
+vs.path = path;
+vs.uid = uid;
+vs.gid = gid;
+vs.dev = dev;
+vs.mode = mode;
+vs.request.func = v9fs_th_do_mknod;
+vs.request.coroutine = qemu_coroutine_self();
+v9fs_qemu_submit_request(vs.request);
+qemu_coroutine_yield();
+return vs.err;
+}
diff --git a/hw/9pfs/virtio-9p-coth.h b/hw/9pfs/virtio-9p-coth.h
index c8d37dd..bd410cb 100644
--- a/hw/9pfs/virtio-9p-coth.h
+++ b/hw/9pfs/virtio-9p-coth.h
@@ -64,4 +64,6 @@ extern int v9fs_co_truncate(V9fsState *, V9fsString *, off_t);
extern int v9fs_co_llistxattr(V9fsState *, V9fsString *, void *, size_t);
extern int v9fs_co_lgetxattr(V9fsState *, V9fsString *,
V9fsString *, void *, size_t);
+extern int v9fs_co_mknod(V9fsState *, V9fsString *, uid_t,
+ gid_t, dev_t, mode_t);
#endif
--
1.7.1
[Qemu-devel] [PATCH 22/25] [virtio-9p] coroutine and threading for mkdir
Signed-off-by: Venkateswararao Jujjuri jv...@linux.vnet.ibm.com
---
hw/9pfs/codir.c | 34 ++
hw/9pfs/virtio-9p-coth.h |1 +
hw/9pfs/virtio-9p.c | 28 ++--
3 files changed, 41 insertions(+), 22 deletions(-)
diff --git a/hw/9pfs/codir.c b/hw/9pfs/codir.c
index adca50c..fcd204d 100644
--- a/hw/9pfs/codir.c
+++ b/hw/9pfs/codir.c
@@ -133,3 +133,37 @@ void v9fs_co_rewinddir(V9fsState *s, V9fsFidState *fidp)
qemu_coroutine_yield();
return;
}
+
+typedef struct V9fsThMkDirState {
+V9fsState *s;
+FsCred cred;
+char *name;
+int err;
+V9fsRequest request;
+} V9fsThMkDirState;
+
+static void v9fs_th_do_mkdir(V9fsRequest *request)
+{
+V9fsThMkDirState *vsp = container_of(request, V9fsThMkDirState, request);
+
+vsp-err = vsp-s-ops-mkdir(vsp-s-ctx, vsp-name, vsp-cred);
+if (vsp-err 0) {
+vsp-err = -errno;
+}
+}
+
+int v9fs_co_mkdir(V9fsState *s, char *name, mode_t mode, uid_t uid, gid_t gid)
+{
+V9fsThMkDirState vs;
+vs.s = s;
+vs.name = name;
+cred_init(vs.cred);
+vs.cred.fc_mode = mode;
+vs.cred.fc_uid = uid;
+vs.cred.fc_gid = gid;
+vs.request.func = v9fs_th_do_mkdir;
+vs.request.coroutine = qemu_coroutine_self();
+v9fs_qemu_submit_request(vs.request);
+qemu_coroutine_yield();
+return vs.err;
+}
diff --git a/hw/9pfs/virtio-9p-coth.h b/hw/9pfs/virtio-9p-coth.h
index bd410cb..3edab4e 100644
--- a/hw/9pfs/virtio-9p-coth.h
+++ b/hw/9pfs/virtio-9p-coth.h
@@ -66,4 +66,5 @@ extern int v9fs_co_lgetxattr(V9fsState *, V9fsString *,
V9fsString *, void *, size_t);
extern int v9fs_co_mknod(V9fsState *, V9fsString *, uid_t,
gid_t, dev_t, mode_t);
+extern int v9fs_co_mkdir(V9fsState *, char *, mode_t, uid_t, gid_t);
#endif
diff --git a/hw/9pfs/virtio-9p.c b/hw/9pfs/virtio-9p.c
index 4858f37..e5112fe 100644
--- a/hw/9pfs/virtio-9p.c
+++ b/hw/9pfs/virtio-9p.c
@@ -149,19 +149,6 @@ static int v9fs_do_mknod(V9fsState *s, char *name,
return s-ops-mknod(s-ctx, name, cred);
}
-static int v9fs_do_mkdir(V9fsState *s, char *name, mode_t mode,
-uid_t uid, gid_t gid)
-{
-FsCred cred;
-
-cred_init(cred);
-cred.fc_uid = uid;
-cred.fc_gid = gid;
-cred.fc_mode = mode;
-
-return s-ops-mkdir(s-ctx, name, cred);
-}
-
static int v9fs_do_fstat(V9fsState *s, int fd, struct stat *stbuf)
{
return s-ops-fstat(s-ctx, fd, stbuf);
@@ -2354,8 +2341,7 @@ out:
static void v9fs_create_post_mkdir(V9fsState *s, V9fsCreateState *vs, int err)
{
-if (err) {
-err = -errno;
+if (err 0) {
goto out;
}
@@ -2404,7 +2390,7 @@ static void v9fs_create_post_lstat(V9fsState *s,
V9fsCreateState *vs, int err)
}
if (vs-perm P9_STAT_MODE_DIR) {
-err = v9fs_do_mkdir(s, vs-fullname.data, vs-perm 0777,
+err = v9fs_co_mkdir(s, vs-fullname.data, vs-perm 0777,
vs-fidp-uid, -1);
v9fs_create_post_mkdir(s, vs, err);
} else if (vs-perm P9_STAT_MODE_SYMLINK) {
@@ -3225,14 +3211,12 @@ static void v9fs_mkdir(void *opaque)
goto out;
}
v9fs_string_sprintf(fullname, %s/%s, fidp-path.data, name.data);
-err = v9fs_do_mkdir(copdu-s, fullname.data, mode, fidp-uid, gid);
-if (err == -1) {
-err = -errno;
+err = v9fs_co_mkdir(copdu-s, fullname.data, mode, fidp-uid, gid);
+if (err 0) {
goto out;
}
-err = v9fs_do_lstat(copdu-s, fullname, stbuf);
-if (err == -1) {
-err = -errno;
+err = v9fs_co_lstat(copdu-s, fullname, stbuf);
+if (err 0) {
goto out;
}
stat_to_qid(stbuf, qid);
--
1.7.1
