Re: [Qemu-devel] [PATCH RFT 2/5] virtio: Convert VirtioDevice to QOM realize/unrealize
On Sun, Jun 09, 2013 at 09:08:15PM -0500, Anthony Liguori wrote:
Peter Crosthwaite peter.crosthwa...@xilinx.com writes:
Hi Andreas,
On Sat, Jun 8, 2013 at 7:55 PM, Andreas Färber afaer...@suse.de wrote:
Hi,
Am 08.06.2013 04:22, schrieb Peter Crosthwaite:
On Sat, Jun 8, 2013 at 4:18 AM, Andreas Färber afaer...@suse.de wrote:
diff --git a/hw/9pfs/virtio-9p-device.c b/hw/9pfs/virtio-9p-device.c
index dc6f4e4..409d315 100644
--- a/hw/9pfs/virtio-9p-device.c
+++ b/hw/9pfs/virtio-9p-device.c
[...]
@@ -136,12 +138,16 @@ static Property virtio_9p_properties[] = {
DEFINE_PROP_END_OF_LIST(),
};
-static void virtio_9p_class_init(ObjectClass *klass, void *data)
+static void virtio_9p_class_init(ObjectClass *oc, void *data)
{
-DeviceClass *dc = DEVICE_CLASS(klass);
-VirtioDeviceClass *vdc = VIRTIO_DEVICE_CLASS(klass);
+DeviceClass *dc = DEVICE_CLASS(oc);
+VirtioDeviceClass *vdc = VIRTIO_DEVICE_CLASS(oc);
+V9fsClass *v9c = VIRTIO_9P_CLASS(oc);
+
+v9c-parent_realize = dc-realize;
+dc-realize = virtio_9p_device_realize;
+
dc-props = virtio_9p_properties;
-vdc-init = virtio_9p_device_init;
vdc-get_features = virtio_9p_get_features;
vdc-get_config = virtio_9p_get_config;
}
@@ -151,6 +157,7 @@ static const TypeInfo virtio_device_info = {
.parent = TYPE_VIRTIO_DEVICE,
.instance_size = sizeof(V9fsState),
.class_init = virtio_9p_class_init,
+.class_size = sizeof(V9fsClass),
};
static void virtio_9p_register_types(void)
diff --git a/hw/9pfs/virtio-9p.h b/hw/9pfs/virtio-9p.h
index 1d6eedb..85699a7 100644
--- a/hw/9pfs/virtio-9p.h
+++ b/hw/9pfs/virtio-9p.h
@@ -227,6 +227,15 @@ typedef struct V9fsState
V9fsConf fsconf;
} V9fsState;
+typedef struct V9fsClass {
+/* private */
+VirtioDeviceClass parent_class;
+/* public */
+
+DeviceRealize parent_realize;
+} V9fsClass;
+
+
If applied tree-wide this change pattern is going to add a lot of
boiler-plate to all devices. There is capability in QOM to access the
overridden parent class functions already, so it can be made to work
without every class having to do this save-and-call trick with
overridden realize (and friends). How about this:
diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index 9190a7e..696702a 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -37,6 +37,18 @@ int qdev_hotplug = 0;
static bool qdev_hot_added = false;
static bool qdev_hot_removed = false;
+void device_parent_realize(DeviceState *dev, Error **errp)
+{
+ObjectClass *class = object_get_class(dev);
+DeviceClass *dc;
+
+class = object_class_get_parent(dc);
+assert(class);
+dc = DEVICE_CLASS(class);
+
+dc-realize(dev, errp);
+}
What's weird about this is that you aren't necessarily calling
Device::realize() here, you're really calling super()::realize().
I really don't know whether it's a better approach or not.
Another option is to have a VirtioDevice::realize() that virtio devices
overload such that you don't have to explicitly call the super()
version. The advantage of this approach is that you don't have to
explicitly call the super version.
Regards,
Anthony Liguori
Nod. Since all realize calls get same parameters not calling
parent's constructor automatically seems strange.
+
And child class realize fns can call this to realize themselves as the
parent would. Ditto for reset and unrealize. Then you would only need
to define struct FooClass when creating new abstractions (or virtual
functions if your C++).
Indeed, if you check the archives you will find that I suggested the
same in the context of ISA i8254/i8259 or CPUState. ;) Yet Anthony
specifically instructed me to do it this way, referring to GObject.
I then documented the expected process in qdev-core.h and object.h.
Thanks for the history. I found this:
Jan 18 2013 Anthony Liguori wrote:
It's idiomatic from GObject.
I'm not sure I can come up with a concrete example but in the absense of
a compelling reason to shift from the idiom, I'd strongly suggest not.
Regards,
Anthony Liguori
The additive diff on this series would take a massive nosedive - is
that a compelling reason? It is very unfortunate that pretty much
every class inheriting from device is going to have to define a class
struct just for the sake of multi-level realization.
There is roughly 15 or so lines of boiler plate added to every class,
and in just the cleanup you have done this week you have about 10 or
so instances of this change pattern. Under the
child-access-to-parent-version proposal those 15 lines become 1.
I don't see the fundamental reason why child classes shouldnt be able
to access parent implementations of virtualised functions. Many OO
oriented
Re: [Qemu-devel] broken incoming migration
On 06/09/2013 05:27 PM, Peter Lieven wrote:
Am 09.06.2013 um 05:09 schrieb Alexey Kardashevskiy a...@ozlabs.ru:
On 06/09/2013 01:01 PM, Wenchao Xia wrote:
于 2013-6-9 10:34, Alexey Kardashevskiy 写道:
On 06/09/2013 12:16 PM, Wenchao Xia wrote:
于 2013-6-8 16:30, Alexey Kardashevskiy 写道:
On 06/08/2013 06:27 PM, Wenchao Xia wrote:
On 04.06.2013 16:40, Paolo Bonzini wrote:
Il 04/06/2013 16:38, Peter Lieven ha scritto:
On 04.06.2013 16:14, Paolo Bonzini wrote:
Il 04/06/2013 15:52, Peter Lieven ha scritto:
On 30.05.2013 16:41, Paolo Bonzini wrote:
Il 30/05/2013 16:38, Peter Lieven ha scritto:
You could also scan the page for nonzero
values before writing it.
i had this in mind, but then choosed the other
approach turned out to be a bad idea.
alexey: i will prepare a patch later today,
could you then please verify it fixes your
problem.
paolo: would we still need the madvise or is
it enough to not write the zeroes?
It should be enough to not write them.
Problem: checking the pages for zero allocates
them. even at the source.
It doesn't look like. I tried this program and top
doesn't show an increasing amount of reserved
memory:
#include stdio.h #include stdlib.h int main() {
char *x = malloc(500 20); int i, j; for (i = 0; i
500; i += 10) { for (j = 0; j 10 20; j +=
4096) { *(volatile char*) (x + (i 20) + j); }
getchar(); } }
strange. we are talking about RSS size, right?
None of the three top values change, and only VIRT is
500 MB.
is the malloc above using mmapped memory?
Yes.
which kernel version do you use?
3.9.
what avoids allocating the memory for me is the
following (with whatever side effects it has ;-))
This would also fail to migrate any page that is swapped
out, breaking overcommit in a more subtle way. :)
Paolo
the following does also not allocate memory, but qemu
does...
Hi, Peter As the patch writes
not sending zero pages breaks migration if a page is zero
at the source but not at the destination.
I don't understand why it would be trouble, shouldn't all
page not received in dest be treated as zero pages?
How would the destination guest know if some page must be
cleared? The previous patch (which Peter reverted) did not
send anything for the pages which were zero on the source
side.
If an page was not received and destination knows that page
should exist according to total size, fill it with zero at
destination, would it solve the problem?
It is _live_ migration, the source sends changes, same pages can
change and be sent several times. So we would need to turn
tracking on on the destination to know if some page was received
from the source or changed by the destination itself (by writing
there bios/firmware images, etc) and then clear pages which were
touched by the destination and were not sent by the source.
OK, I can understand the problem is, for example: Destination boots
up with 0x-0x filled with bios image. Source forgot to send
zero pages in 0x-0x.
The source did not forget, instead it zeroed these pages during its
life and thought that they must be zeroed at the destination already
(as the destination did not start and did not have a chance to write
something there).
After migration destination got 0x-0x dirty(different with
source)
Yep. And those pages were empty on the source what made debugging very
easy :)
Thanks for explain.
This seems refer to the migration protocol: how should the guest
treat unsent pages. The patch causing the problem, actually treat
zero pages as not to sent at source, but another half is missing:
treat not received as zero pages at destination. I guess if second
half is added, problem is gone: after page transfer completed,
before destination resume, fill zero in not received pages.
Make a working patch, we'll discuss it :) I do not see much
acceleration coming from there.
I would also not spent much time with this. I would either look to find
an easy way to fix the initialization code to not unneccessarily load
data into RAM or i will sent a v2 of my patch following Eric's
concerns.
There is no easy way to implement the flag and keep your original patch as
we have to implement this flag in all architectures which got broken by
your patch and I personally can fix only PPC64-pseries but not the others.
Furthermore your revert + new patches perfectly solve the problem, why
would we want to bother now with this new flag which nobody really needs
right now?
Please, please, revert the original patch or I'll try to do it :)
--
Alexey
Re: [Qemu-devel] broken incoming migration
On 10.06.2013 08:39, Alexey Kardashevskiy wrote:
On 06/09/2013 05:27 PM, Peter Lieven wrote:
Am 09.06.2013 um 05:09 schrieb Alexey Kardashevskiy a...@ozlabs.ru:
On 06/09/2013 01:01 PM, Wenchao Xia wrote:
于 2013-6-9 10:34, Alexey Kardashevskiy 写道:
On 06/09/2013 12:16 PM, Wenchao Xia wrote:
于 2013-6-8 16:30, Alexey Kardashevskiy 写道:
On 06/08/2013 06:27 PM, Wenchao Xia wrote:
On 04.06.2013 16:40, Paolo Bonzini wrote:
Il 04/06/2013 16:38, Peter Lieven ha scritto:
On 04.06.2013 16:14, Paolo Bonzini wrote:
Il 04/06/2013 15:52, Peter Lieven ha scritto:
On 30.05.2013 16:41, Paolo Bonzini wrote:
Il 30/05/2013 16:38, Peter Lieven ha scritto:
You could also scan the page for nonzero
values before writing it.
i had this in mind, but then choosed the other
approach turned out to be a bad idea.
alexey: i will prepare a patch later today,
could you then please verify it fixes your
problem.
paolo: would we still need the madvise or is
it enough to not write the zeroes?
It should be enough to not write them.
Problem: checking the pages for zero allocates
them. even at the source.
It doesn't look like. I tried this program and top
doesn't show an increasing amount of reserved
memory:
#include stdio.h #include stdlib.h int main() {
char *x = malloc(500 20); int i, j; for (i = 0; i
500; i += 10) { for (j = 0; j 10 20; j +=
4096) { *(volatile char*) (x + (i 20) + j); }
getchar(); } }
strange. we are talking about RSS size, right?
None of the three top values change, and only VIRT is
500 MB.
is the malloc above using mmapped memory?
Yes.
which kernel version do you use?
3.9.
what avoids allocating the memory for me is the
following (with whatever side effects it has ;-))
This would also fail to migrate any page that is swapped
out, breaking overcommit in a more subtle way. :)
Paolo
the following does also not allocate memory, but qemu
does...
Hi, Peter As the patch writes
not sending zero pages breaks migration if a page is zero
at the source but not at the destination.
I don't understand why it would be trouble, shouldn't all
page not received in dest be treated as zero pages?
How would the destination guest know if some page must be
cleared? The previous patch (which Peter reverted) did not
send anything for the pages which were zero on the source
side.
If an page was not received and destination knows that page
should exist according to total size, fill it with zero at
destination, would it solve the problem?
It is _live_ migration, the source sends changes, same pages can
change and be sent several times. So we would need to turn
tracking on on the destination to know if some page was received
from the source or changed by the destination itself (by writing
there bios/firmware images, etc) and then clear pages which were
touched by the destination and were not sent by the source.
OK, I can understand the problem is, for example: Destination boots
up with 0x-0x filled with bios image. Source forgot to send
zero pages in 0x-0x.
The source did not forget, instead it zeroed these pages during its
life and thought that they must be zeroed at the destination already
(as the destination did not start and did not have a chance to write
something there).
After migration destination got 0x-0x dirty(different with
source)
Yep. And those pages were empty on the source what made debugging very
easy :)
Thanks for explain.
This seems refer to the migration protocol: how should the guest
treat unsent pages. The patch causing the problem, actually treat
zero pages as not to sent at source, but another half is missing:
treat not received as zero pages at destination. I guess if second
half is added, problem is gone: after page transfer completed,
before destination resume, fill zero in not received pages.
Make a working patch, we'll discuss it :) I do not see much
acceleration coming from there.
I would also not spent much time with this. I would either look to find
an easy way to fix the initialization code to not unneccessarily load
data into RAM or i will sent a v2 of my patch following Eric's
concerns.
There is no easy way to implement the flag and keep your original patch as
we have to implement this flag in all architectures which got broken by
your patch and I personally can fix only PPC64-pseries but not the others.
Furthermore your revert + new patches perfectly solve the problem, why
would we want to bother now with this new flag which nobody really needs
right now?
Please, please, revert the original patch or I'll try to do it :)
I tried, but there where concerns by the community. Alternativly I found
the following alternate solution. Please drop the 2 patches and try the
following:
diff --git a/arch_init.c b/arch_init.c
index 5d32ecf..458bf8c 100644
--- a/arch_init.c
+++ b/arch_init.c
@@ -799,6 +799,8 @@ static int ram_load(QEMUFile *f, void *opaque, int
version_id)
while (total_ram_bytes) {
Re: [Qemu-devel] broken incoming migration
On 06/10/2013 04:50 PM, Peter Lieven wrote:
On 10.06.2013 08:39, Alexey Kardashevskiy wrote:
On 06/09/2013 05:27 PM, Peter Lieven wrote:
Am 09.06.2013 um 05:09 schrieb Alexey Kardashevskiy a...@ozlabs.ru:
On 06/09/2013 01:01 PM, Wenchao Xia wrote:
于 2013-6-9 10:34, Alexey Kardashevskiy 写道:
On 06/09/2013 12:16 PM, Wenchao Xia wrote:
于 2013-6-8 16:30, Alexey Kardashevskiy 写道:
On 06/08/2013 06:27 PM, Wenchao Xia wrote:
On 04.06.2013 16:40, Paolo Bonzini wrote:
Il 04/06/2013 16:38, Peter Lieven ha scritto:
On 04.06.2013 16:14, Paolo Bonzini wrote:
Il 04/06/2013 15:52, Peter Lieven ha scritto:
On 30.05.2013 16:41, Paolo Bonzini wrote:
Il 30/05/2013 16:38, Peter Lieven ha scritto:
You could also scan the page for nonzero
values before writing it.
i had this in mind, but then choosed the other
approach turned out to be a bad idea.
alexey: i will prepare a patch later today,
could you then please verify it fixes your
problem.
paolo: would we still need the madvise or is
it enough to not write the zeroes?
It should be enough to not write them.
Problem: checking the pages for zero allocates
them. even at the source.
It doesn't look like. I tried this program and top
doesn't show an increasing amount of reserved
memory:
#include stdio.h #include stdlib.h int main() {
char *x = malloc(500 20); int i, j; for (i = 0; i
500; i += 10) { for (j = 0; j 10 20; j +=
4096) { *(volatile char*) (x + (i 20) + j); }
getchar(); } }
strange. we are talking about RSS size, right?
None of the three top values change, and only VIRT is
500 MB.
is the malloc above using mmapped memory?
Yes.
which kernel version do you use?
3.9.
what avoids allocating the memory for me is the
following (with whatever side effects it has ;-))
This would also fail to migrate any page that is swapped
out, breaking overcommit in a more subtle way. :)
Paolo
the following does also not allocate memory, but qemu
does...
Hi, Peter As the patch writes
not sending zero pages breaks migration if a page is zero
at the source but not at the destination.
I don't understand why it would be trouble, shouldn't all
page not received in dest be treated as zero pages?
How would the destination guest know if some page must be
cleared? The previous patch (which Peter reverted) did not
send anything for the pages which were zero on the source
side.
If an page was not received and destination knows that page
should exist according to total size, fill it with zero at
destination, would it solve the problem?
It is _live_ migration, the source sends changes, same pages can
change and be sent several times. So we would need to turn
tracking on on the destination to know if some page was received
from the source or changed by the destination itself (by writing
there bios/firmware images, etc) and then clear pages which were
touched by the destination and were not sent by the source.
OK, I can understand the problem is, for example: Destination boots
up with 0x-0x filled with bios image. Source forgot to send
zero pages in 0x-0x.
The source did not forget, instead it zeroed these pages during its
life and thought that they must be zeroed at the destination already
(as the destination did not start and did not have a chance to write
something there).
After migration destination got 0x-0x dirty(different with
source)
Yep. And those pages were empty on the source what made debugging very
easy :)
Thanks for explain.
This seems refer to the migration protocol: how should the guest
treat unsent pages. The patch causing the problem, actually treat
zero pages as not to sent at source, but another half is missing:
treat not received as zero pages at destination. I guess if second
half is added, problem is gone: after page transfer completed,
before destination resume, fill zero in not received pages.
Make a working patch, we'll discuss it :) I do not see much
acceleration coming from there.
I would also not spent much time with this. I would either look to find
an easy way to fix the initialization code to not unneccessarily load
data into RAM or i will sent a v2 of my patch following Eric's
concerns.
There is no easy way to implement the flag and keep your original patch as
we have to implement this flag in all architectures which got broken by
your patch and I personally can fix only PPC64-pseries but not the others.
Furthermore your revert + new patches perfectly solve the problem, why
would we want to bother now with this new flag which nobody really needs
right now?
Please, please, revert the original patch or I'll try to do it :)
I tried, but there where concerns by the community.
Was here anybody who did not want to revert the patch (besides you)?
I did not notice.
Alternativly I found
the following alternate solution. Please drop the 2 patches and try the
following:
How is it going to work if upstream QEMU doesn't send
[Qemu-devel] [PATCH] migration: ensure memory is zeroized at the destination
migration relies on the target memory to be zeroed out since commit
f1c72795 (migration: do not sent zero pages in bulk stage). however,
there is a subtle case where this breaks migration. if for some reason
a page is zero at the source but not at the destination the destination
memory is corrupted.
this was reported to break migration on pseries and also other platforms
might be affected. to ultimatively make sure the destination memory is zero
at the destination check for it on negotiation of ram blocks.
note: the better fix for this would be to pass a flag to the machine
init functions of all architectures to indicate that the machine is a
migration target and then avoid copying ram images etc. to physical
ram in this case. but this would require a lot of code to be changed
and reviewed.
Signed-off-by: Peter Lieven p...@kamp.de
---
arch_init.c | 10 ++
1 file changed, 10 insertions(+)
diff --git a/arch_init.c b/arch_init.c
index 5d32ecf..458bf8c 100644
--- a/arch_init.c
+++ b/arch_init.c
@@ -799,6 +799,8 @@ static int ram_load(QEMUFile *f, void *opaque, int
version_id)
while (total_ram_bytes) {
RAMBlock *block;
uint8_t len;
+void *base;
+ram_addr_t offset;
len = qemu_get_byte(f);
qemu_get_buffer(f, (uint8_t *)id, len);
@@ -822,6 +824,14 @@ static int ram_load(QEMUFile *f, void *opaque, int
version_id)
goto done;
}
+base = memory_region_get_ram_ptr(block-mr);
+for (offset = 0; offset block-length;
+ offset += TARGET_PAGE_SIZE) {
+if (!is_zero_page(base + offset)) {
+memset(base + offset, 0x00, TARGET_PAGE_SIZE);
+}
+}
+
total_ram_bytes -= length;
}
}
--
1.7.9.5
Re: [Qemu-devel] [PATCH qom-cpu 31/59] monitor: Simplify do_info_numa()
Andreas Färber afaer...@suse.de writes:
Use new qemu_for_each_cpu().
Signed-off-by: Andreas Färber afaer...@suse.de
---
monitor.c | 27 +++
1 file changed, 19 insertions(+), 8 deletions(-)
diff --git a/monitor.c b/monitor.c
index 9be515c..f37bf3d 100644
--- a/monitor.c
+++ b/monitor.c
@@ -1803,21 +1803,32 @@ static void do_info_mtree(Monitor *mon, const QDict
*qdict)
mtree_info((fprintf_function)monitor_printf, mon);
}
+typedef struct DoInfoNUMAData {
+Monitor *mon;
+int numa_node;
+} DoInfoNUMAData;
+
+static void do_info_numa_one(CPUState *cpu, void *data)
+{
+DoInfoNUMAData *s = data;
+
+if (cpu-numa_node == s-numa_node) {
+monitor_printf(s-mon, %d, cpu-cpu_index);
+}
+}
+
static void do_info_numa(Monitor *mon, const QDict *qdict)
{
int i;
-CPUArchState *env;
-CPUState *cpu;
+DoInfoNUMAData s = {
+.mon = mon,
+};
monitor_printf(mon, %d nodes\n, nb_numa_nodes);
for (i = 0; i nb_numa_nodes; i++) {
monitor_printf(mon, node %d cpus:, i);
-for (env = first_cpu; env != NULL; env = env-next_cpu) {
-cpu = ENV_GET_CPU(env);
-if (cpu-numa_node == i) {
-monitor_printf(mon, %d, cpu-cpu_index);
-}
-}
+s.numa_node = i;
+qemu_for_each_cpu(do_info_numa_one, s);
monitor_printf(mon, \n);
monitor_printf(mon, node %d size: % PRId64 MB\n, i,
node_mem[i] 20);
This again demonstrates the relative clunkiness of higher order
functions in C. Control flow jumps back and forth in the source
(lambda, how I miss you), you need an extra type, and you need to go
around the type system.
In my experience, loops are a much more natural fit for C.
Would it be possible to have a function cpu_next(CPUState *)? So you
can simply do:
for (cpu = cpu_next(NULL); cpu; cpu = cpu_next(cpu) {
if (cpu-numa_node == i) {
monitor_printf(mon, %d, cpu-cpu_index);
}
}
Simple and type safe.
Precedence: bdrv_next().
[Qemu-devel] [PATCH] Deactivate timer for target_bit above 61
QEMU timer supports a maximum timer of INT64_MAX. So starting timer only for time which is calculated using target_bit 62 and deactivate/stop timer if the target bit is above 61. This patch also fix the time calculation from target_bit. The code was doing (1 (target_bit + 1)) while this should be (1ULL (target_bit + 1)). Signed-off-by: Bharat Bhushan bharat.bhus...@freescale.com --- hw/ppc/ppc_booke.c |8 +++- 1 files changed, 7 insertions(+), 1 deletions(-) diff --git a/hw/ppc/ppc_booke.c b/hw/ppc/ppc_booke.c index e41b036..f4eda15 100644 --- a/hw/ppc/ppc_booke.c +++ b/hw/ppc/ppc_booke.c @@ -133,9 +133,15 @@ static void booke_update_fixed_timer(CPUPPCState *env, ppc_tb_t *tb_env = env-tb_env; uint64_t lapse; uint64_t tb; -uint64_t period = 1 (target_bit + 1); +uint64_t period; uint64_t now; +/* Deactivate timer for target_bit 61 */ +if (target_bit 61) +return; + +period = 1ULL (target_bit + 1); + now = qemu_get_clock_ns(vm_clock); tb = cpu_ppc_get_tb(tb_env, now, tb_env-tb_offset); -- 1.7.0.4
[Qemu-devel] [PATCH v2]booke: timer: Deactivate timer for target_bit above 61
QEMU timer supports a maximum timer of INT64_MAX. So starting timer only for time which is calculated using target_bit 62 and deactivate/stop timer if the target bit is above 61. This patch also fix the time calculation from target_bit. The code was doing (1 (target_bit + 1)) while this should be (1ULL (target_bit + 1)). Signed-off-by: Bharat Bhushan bharat.bhus...@freescale.com --- v1-v2 - Added booke: timer: in patch subject hw/ppc/ppc_booke.c |8 +++- 1 files changed, 7 insertions(+), 1 deletions(-) diff --git a/hw/ppc/ppc_booke.c b/hw/ppc/ppc_booke.c index e41b036..f4eda15 100644 --- a/hw/ppc/ppc_booke.c +++ b/hw/ppc/ppc_booke.c @@ -133,9 +133,15 @@ static void booke_update_fixed_timer(CPUPPCState *env, ppc_tb_t *tb_env = env-tb_env; uint64_t lapse; uint64_t tb; -uint64_t period = 1 (target_bit + 1); +uint64_t period; uint64_t now; +/* Deactivate timer for target_bit 61 */ +if (target_bit 61) +return; + +period = 1ULL (target_bit + 1); + now = qemu_get_clock_ns(vm_clock); tb = cpu_ppc_get_tb(tb_env, now, tb_env-tb_offset); -- 1.7.0.4
Re: [Qemu-devel] Qemu crashed while unpluging IDE disk
On Fri, Jun 07, 2013 at 02:31:00PM +, Gonglei (Arei) wrote:
While starting a Fedora_14 guest, we came across a segfault of qemu:
the logs in /var/log/messages are:
Jun 1 02:38:56 NC587 kernel: [403549.565754] show_signal_msg: 136 callbacks
suppressed
Jun 1 02:38:56 NC587 kernel: [403549.565758] qemu-system-i38[25840]:
segfault at 28 ip 00418d91 sp 7fe02aef4f00 error 4 in
qemu-system-i386[40+35]
the very segfault refers to the code:
/*
* Handle a read request in coroutine context
*/
static int coroutine_fn bdrv_co_do_readv(BlockDriverState *bs,
int64_t sector_num, int nb_sectors, QEMUIOVector *qiov,
BdrvRequestFlags flags)
{
BlockDriver *drv = bs-drv;//The segfault occurs when bs equals to
NULL.
BdrvTrackedRequest req;
int ret;
NOTE: we are running on a XEN hypervisor with qemu 1.2.0
Can you try qemu.git/master?
Stefan
Re: [Qemu-devel] [PATCH 3/9] block: bdrv_reopen_prepare(): use error_setg_file_open()
On Fri, Jun 07, 2013 at 03:52:29PM -0400, Luiz Capitulino wrote:
Signed-off-by: Luiz Capitulino lcapitul...@redhat.com
---
block.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/block.c b/block.c
index 79ad33d..c78f152 100644
--- a/block.c
+++ b/block.c
@@ -1291,8 +1291,7 @@ int bdrv_reopen_prepare(BDRVReopenState *reopen_state,
BlockReopenQueue *queue,
if (local_err != NULL) {
error_propagate(errp, local_err);
} else {
-error_set(errp, QERR_OPEN_FILE_FAILED,
- reopen_state-bs-filename);
+error_setg_file_open(errp, errno,
reopen_state-bs-filename);
s/errno/-ret/
Re: [Qemu-devel] [PATCH] chardev: add baud parameter for serial host device
On Monday 10 June 2013 07:56:01 Gerd Hoffmann wrote: On 06/08/13 23:49, Peter Wu wrote: When QEMU starts, it always changes the serial port parameters including baud rate. This confused my guest which thought it was outputting at 9600 baud while it was in fact changed to 115200. After this patch, I can use `-serial /dev/ttyS0,baud=9600` to override the default baud rate of 115200. I think we should just flip the default to 9600. IIRC this is the power-on default baud rate of the 8250 uart family, so this should be the qemu default too. If a guest wants to use a higher baudrate it has to reprogram the uart anyway (and qemu will apply the guest changes to the host uart). FWIW, when I tried MODE.COM in ms-dos to change the baud rate, `stty -F /dev/ttyS0 -a` still reported 115200 baud. This is on Linux 3.9 if that matters. Besides this comment, any other feedback on the patch itself? Regards, Peter
Re: [Qemu-devel] [PATCH 3/9] block: bdrv_reopen_prepare(): use error_setg_file_open()
On Fri, Jun 07, 2013 at 03:52:29PM -0400, Luiz Capitulino wrote:
Signed-off-by: Luiz Capitulino lcapitul...@redhat.com
---
block.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/block.c b/block.c
index 79ad33d..c78f152 100644
--- a/block.c
+++ b/block.c
@@ -1291,8 +1291,7 @@ int bdrv_reopen_prepare(BDRVReopenState *reopen_state,
BlockReopenQueue *queue,
if (local_err != NULL) {
error_propagate(errp, local_err);
} else {
-error_set(errp, QERR_OPEN_FILE_FAILED,
- reopen_state-bs-filename);
+error_setg_file_open(errp, errno,
reopen_state-bs-filename);
Looking closer, my suggestion was wrong too.
I think QERR_OPEN_FILE_FAILED is simply the wrong error here. We don't
know that the error occurred when trying to open a file.
errno does not necessarily contain the error value!
Stefan
Re: [Qemu-devel] broken incoming migration
On 10.06.2013 08:55, Alexey Kardashevskiy wrote:
On 06/10/2013 04:50 PM, Peter Lieven wrote:
On 10.06.2013 08:39, Alexey Kardashevskiy wrote:
On 06/09/2013 05:27 PM, Peter Lieven wrote:
Am 09.06.2013 um 05:09 schrieb Alexey Kardashevskiy a...@ozlabs.ru:
On 06/09/2013 01:01 PM, Wenchao Xia wrote:
于 2013-6-9 10:34, Alexey Kardashevskiy 写道:
On 06/09/2013 12:16 PM, Wenchao Xia wrote:
于 2013-6-8 16:30, Alexey Kardashevskiy 写道:
On 06/08/2013 06:27 PM, Wenchao Xia wrote:
On 04.06.2013 16:40, Paolo Bonzini wrote:
Il 04/06/2013 16:38, Peter Lieven ha scritto:
On 04.06.2013 16:14, Paolo Bonzini wrote:
Il 04/06/2013 15:52, Peter Lieven ha scritto:
On 30.05.2013 16:41, Paolo Bonzini wrote:
Il 30/05/2013 16:38, Peter Lieven ha scritto:
You could also scan the page for nonzero
values before writing it.
i had this in mind, but then choosed the other
approach turned out to be a bad idea.
alexey: i will prepare a patch later today,
could you then please verify it fixes your
problem.
paolo: would we still need the madvise or is
it enough to not write the zeroes?
It should be enough to not write them.
Problem: checking the pages for zero allocates
them. even at the source.
It doesn't look like. I tried this program and top
doesn't show an increasing amount of reserved
memory:
#include stdio.h #include stdlib.h int main() {
char *x = malloc(500 20); int i, j; for (i = 0; i
500; i += 10) { for (j = 0; j 10 20; j +=
4096) { *(volatile char*) (x + (i 20) + j); }
getchar(); } }
strange. we are talking about RSS size, right?
None of the three top values change, and only VIRT is
500 MB.
is the malloc above using mmapped memory?
Yes.
which kernel version do you use?
3.9.
what avoids allocating the memory for me is the
following (with whatever side effects it has ;-))
This would also fail to migrate any page that is swapped
out, breaking overcommit in a more subtle way. :)
Paolo
the following does also not allocate memory, but qemu
does...
Hi, Peter As the patch writes
not sending zero pages breaks migration if a page is zero
at the source but not at the destination.
I don't understand why it would be trouble, shouldn't all
page not received in dest be treated as zero pages?
How would the destination guest know if some page must be
cleared? The previous patch (which Peter reverted) did not
send anything for the pages which were zero on the source
side.
If an page was not received and destination knows that page
should exist according to total size, fill it with zero at
destination, would it solve the problem?
It is _live_ migration, the source sends changes, same pages can
change and be sent several times. So we would need to turn
tracking on on the destination to know if some page was received
from the source or changed by the destination itself (by writing
there bios/firmware images, etc) and then clear pages which were
touched by the destination and were not sent by the source.
OK, I can understand the problem is, for example: Destination boots
up with 0x-0x filled with bios image. Source forgot to send
zero pages in 0x-0x.
The source did not forget, instead it zeroed these pages during its
life and thought that they must be zeroed at the destination already
(as the destination did not start and did not have a chance to write
something there).
After migration destination got 0x-0x dirty(different with
source)
Yep. And those pages were empty on the source what made debugging very
easy :)
Thanks for explain.
This seems refer to the migration protocol: how should the guest
treat unsent pages. The patch causing the problem, actually treat
zero pages as not to sent at source, but another half is missing:
treat not received as zero pages at destination. I guess if second
half is added, problem is gone: after page transfer completed,
before destination resume, fill zero in not received pages.
Make a working patch, we'll discuss it :) I do not see much
acceleration coming from there.
I would also not spent much time with this. I would either look to find
an easy way to fix the initialization code to not unneccessarily load
data into RAM or i will sent a v2 of my patch following Eric's
concerns.
There is no easy way to implement the flag and keep your original patch as
we have to implement this flag in all architectures which got broken by
your patch and I personally can fix only PPC64-pseries but not the others.
Furthermore your revert + new patches perfectly solve the problem, why
would we want to bother now with this new flag which nobody really needs
right now?
Please, please, revert the original patch or I'll try to do it :)
I tried, but there where concerns by the community.
Was here anybody who did not want to revert the patch (besides you)?
I did not notice.
Eric said I should not drop the skipped_pages stuff in the monitor.
Alternativly I found
the following alternate solution. Please drop the 2 patches and try the
following:
Re: [Qemu-devel] [PATCH 4/9] block: mirror_complete(): use error_setg_file_open()
On Fri, Jun 07, 2013 at 03:52:30PM -0400, Luiz Capitulino wrote: Signed-off-by: Luiz Capitulino lcapitul...@redhat.com --- block/mirror.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/block/mirror.c b/block/mirror.c index 8b07dec..89d531d 100644 --- a/block/mirror.c +++ b/block/mirror.c @@ -512,7 +512,7 @@ static void mirror_complete(BlockJob *job, Error **errp) char backing_filename[PATH_MAX]; bdrv_get_full_backing_filename(s-target, backing_filename, sizeof(backing_filename)); -error_set(errp, QERR_OPEN_FILE_FAILED, backing_filename); +error_setg_file_open(errp, errno, backing_filename); s/errno/-ret/
Re: [Qemu-devel] [PATCH 5/9] blockdev: use error_setg_file_open()
On Fri, Jun 07, 2013 at 03:52:31PM -0400, Luiz Capitulino wrote: Signed-off-by: Luiz Capitulino lcapitul...@redhat.com --- blockdev.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) s/errno/-ret/g
Re: [Qemu-devel] how to know qemu devices topology
On Sun, Jun 09, 2013 at 01:26:21PM +0800, yue-kvm wrote: 1. how could i know how many devices qemu support and hardware topology of all of them? 2.the quantitativerestriction of guest's devices. vcpu,disk,ethernet,memory? Some resources are limited by the bus (e.g. PCI slots). The details depend on the machine type because the emulated busses are different. Other resources may be limited on guest factors like firmware tables or device tree. Others may be limited only by host hardware resources. Your question is very broad and you need to look into specifics to find the answers. If you want a quick solution, I suggest looking at vendor documentation - for example, look what distros shipping KVM say about x86 guest limits. Stefan
Re: [Qemu-devel] [PATCH v4 0/2] fix 'qemu-img snapshot -a' operation for sheepdog
Am 07.06.2013 um 19:54 hat Liu Yuan geschrieben: v4: - fix savevm, pass current vdi_id instead of parent_vdi_id v3: - fix sheepdog's loadvm handling, don't rely on the write to create branch v2: - add the comment to make things more clear - call sd_create_branch() after s-is_snapshot = true because after calling sd_create_branch, it is not snapshot anymore. Nothing big, just two simple patches to enable this commind for sheepdog. Cc: qemu-devel@nongnu.org Cc: MORITA Kazutaka morita.kazut...@lab.ntt.co.jp Cc: Kevin Wolf kw...@redhat.com Cc: Stefan Hajnoczi stefa...@redhat.com Liu Yuan (2): sheepdog: fix snapshot tag initialization sheepdog: support 'qemu-img snapshot -a' block/sheepdog.c | 10 ++ 1 file changed, 6 insertions(+), 4 deletions(-) Thanks, applied to the block branch. Kevin
Re: [Qemu-devel] [PATCH 8/8] [PATCH RFC v2] s390-qemu: cpu hotplug - Treat S390 cpus as devices
On Sun, 09 Jun 2013 03:11:35 +0200
Andreas Färber afaer...@suse.de wrote:
Am 07.06.2013 19:28, schrieb Jason J. Herne:
From: Jason J. Herne jjhe...@us.ibm.com
Modify cpu initialization and QOM routines associated with s390-cpu such
that
all cpus on S390 are now created via the QOM device creation code path.
Signed-off-by: Jason J. Herne jjhe...@us.ibm.com
---
hw/s390x/s390-virtio-ccw.c | 15 ++-
hw/s390x/s390-virtio.c | 25 +
hw/s390x/s390-virtio.h |2 +-
include/qapi/qmp/qerror.h |3 +++
qdev-monitor.c | 17 +
target-s390x/cpu.c | 24 ++--
6 files changed, 58 insertions(+), 28 deletions(-)
diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
index 70bd858..141adce 100644
--- a/hw/s390x/s390-virtio-ccw.c
+++ b/hw/s390x/s390-virtio-ccw.c
@@ -95,12 +95,8 @@ static void ccw_init(QEMUMachineInitArgs *args)
/* allocate storage keys */
s390_set_storage_keys_p(g_malloc0(my_ram_size / TARGET_PAGE_SIZE));
-/* init CPUs */
-s390_init_cpus(args-cpu_model);
+s390_init_ipi_states();
-if (kvm_enabled()) {
-kvm_s390_enable_css_support(s390_cpu_addr2state(0));
-}
/*
* Create virtual css and set it as default so that non mcss-e
* enabled guests only see virtio devices.
@@ -112,11 +108,20 @@ static void ccw_init(QEMUMachineInitArgs *args)
s390_create_virtio_net(BUS(css_bus), virtio-net-ccw);
}
+static void ccw_post_cpu_init(void)
+{
+if (kvm_enabled()) {
+kvm_s390_enable_css_support(s390_cpu_addr2state(0));
+}
+}
Am I understanding correctly that all this is about differentiating one
call between the ccw and legacy machines?
Isn't there a machine-init-done Notifier that the ccw machine init could
register for?
I wasn't aware of that, but it looks worth a try.
What if CPU 0 were hot-unplugged? Would the capability need to be
re-enabled or will this remain a one-time task?
KVM_ENABLE_CAP is a vcpu ioctl, but we use it to enable a machine-wide
capability (which will stay enabled during the lifetime of the
machine). (It probably should be any cpu instead of cpu 0, but
that's probably not the only place doing that.)
[Qemu-devel] [PATCH] vmdk: byteswap VMDK4Header.desc_offset field
Remember to byteswap VMDK4Header.desc_offset on big-endian machines.
Cc: qemu-sta...@nongnu.org
Signed-off-by: Stefan Hajnoczi stefa...@redhat.com
---
block/vmdk.c | 7 +--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/block/vmdk.c b/block/vmdk.c
index 608daaf..ee50a73 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -507,8 +507,11 @@ static int vmdk_open_vmdk4(BlockDriverState *bs,
if (ret 0) {
return ret;
}
-if (header.capacity == 0 header.desc_offset) {
-return vmdk_open_desc_file(bs, flags, header.desc_offset 9);
+if (header.capacity == 0) {
+int64_t desc_offset = le64_to_cpu(header.desc_offset);
+if (desc_offset) {
+return vmdk_open_desc_file(bs, flags, desc_offset 9);
+}
}
if (le64_to_cpu(header.gd_offset) == VMDK4_GD_AT_END) {
--
1.8.1.4
Re: [Qemu-devel] [PATCH] vmdk: refuse to open higher version than supported
On Sun, Jun 09, 2013 at 09:44:15AM +0800, Fam Zheng wrote:
Although we try to be compatible with published VMDK spec, VMware has
newer version from ESXi 5.1 exported OVF/OVA, which we have no knowledge
what's changed in it. And it is very likely to have more new versions in
the future, so it's not safe to open them blindly.
The best I could find was this high-level overview:
http://myvirtualcloud.net/?p=3829
Signed-off-by: Fam Zheng f...@redhat.com
---
block/vmdk.c | 4
1 file changed, 4 insertions(+)
diff --git a/block/vmdk.c b/block/vmdk.c
index 608daaf..d9c2368 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -558,6 +558,10 @@ static int vmdk_open_vmdk4(BlockDriverState *bs,
header = footer.header;
}
+if (le32_to_cpu(header.version) = 3) {
+return -EINVAL;
+}
+
Looks fine, the VMDK 5.0 spec says header.version may be 1 or 2.
Reviewed-by: Stefan Hajnoczi stefa...@redhat.com
Re: [Qemu-devel] [PATCH] vmdk: refuse to open higher version than supported
Am 09.06.2013 um 03:44 hat Fam Zheng geschrieben:
Refuse to open higher version for safety.
Although we try to be compatible with published VMDK spec, VMware has
newer version from ESXi 5.1 exported OVF/OVA, which we have no knowledge
what's changed in it. And it is very likely to have more new versions in
the future, so it's not safe to open them blindly.
Signed-off-by: Fam Zheng f...@redhat.com
Yes, it's definitely a good idea to add a check.
@@ -558,6 +558,10 @@ static int vmdk_open_vmdk4(BlockDriverState *bs,
header = footer.header;
}
+if (le32_to_cpu(header.version) = 3) {
+return -EINVAL;
+}
+
Other block drivers return -ENOTSUP for this case, and also call
qerror_report(QERR_UNKNOWN_BLOCK_FORMAT_FEATURE, ...) so that you get a
meaningful error message. Can you model the VMDK code after them?
Kevin
Re: [Qemu-devel] broken incoming migration
On 06/10/2013 06:44 PM, Peter Lieven wrote:
On 10.06.2013 08:55, Alexey Kardashevskiy wrote:
On 06/10/2013 04:50 PM, Peter Lieven wrote:
On 10.06.2013 08:39, Alexey Kardashevskiy wrote:
On 06/09/2013 05:27 PM, Peter Lieven wrote:
Am 09.06.2013 um 05:09 schrieb Alexey Kardashevskiy a...@ozlabs.ru:
On 06/09/2013 01:01 PM, Wenchao Xia wrote:
于 2013-6-9 10:34, Alexey Kardashevskiy 写道:
On 06/09/2013 12:16 PM, Wenchao Xia wrote:
于 2013-6-8 16:30, Alexey Kardashevskiy 写道:
On 06/08/2013 06:27 PM, Wenchao Xia wrote:
On 04.06.2013 16:40, Paolo Bonzini wrote:
Il 04/06/2013 16:38, Peter Lieven ha scritto:
On 04.06.2013 16:14, Paolo Bonzini wrote:
Il 04/06/2013 15:52, Peter Lieven ha scritto:
On 30.05.2013 16:41, Paolo Bonzini wrote:
Il 30/05/2013 16:38, Peter Lieven ha scritto:
You could also scan the page for nonzero
values before writing it.
i had this in mind, but then choosed the other
approach turned out to be a bad idea.
alexey: i will prepare a patch later today,
could you then please verify it fixes your
problem.
paolo: would we still need the madvise or is
it enough to not write the zeroes?
It should be enough to not write them.
Problem: checking the pages for zero allocates
them. even at the source.
It doesn't look like. I tried this program and top
doesn't show an increasing amount of reserved
memory:
#include stdio.h #include stdlib.h int main() {
char *x = malloc(500 20); int i, j; for (i = 0; i
500; i += 10) { for (j = 0; j 10 20; j +=
4096) { *(volatile char*) (x + (i 20) + j); }
getchar(); } }
strange. we are talking about RSS size, right?
None of the three top values change, and only VIRT is
500 MB.
is the malloc above using mmapped memory?
Yes.
which kernel version do you use?
3.9.
what avoids allocating the memory for me is the
following (with whatever side effects it has ;-))
This would also fail to migrate any page that is swapped
out, breaking overcommit in a more subtle way. :)
Paolo
the following does also not allocate memory, but qemu
does...
Hi, Peter As the patch writes
not sending zero pages breaks migration if a page is zero
at the source but not at the destination.
I don't understand why it would be trouble, shouldn't all
page not received in dest be treated as zero pages?
How would the destination guest know if some page must be
cleared? The previous patch (which Peter reverted) did not
send anything for the pages which were zero on the source
side.
If an page was not received and destination knows that page
should exist according to total size, fill it with zero at
destination, would it solve the problem?
It is _live_ migration, the source sends changes, same pages can
change and be sent several times. So we would need to turn
tracking on on the destination to know if some page was received
from the source or changed by the destination itself (by writing
there bios/firmware images, etc) and then clear pages which were
touched by the destination and were not sent by the source.
OK, I can understand the problem is, for example: Destination boots
up with 0x-0x filled with bios image. Source forgot to send
zero pages in 0x-0x.
The source did not forget, instead it zeroed these pages during its
life and thought that they must be zeroed at the destination already
(as the destination did not start and did not have a chance to write
something there).
After migration destination got 0x-0x dirty(different with
source)
Yep. And those pages were empty on the source what made debugging very
easy :)
Thanks for explain.
This seems refer to the migration protocol: how should the guest
treat unsent pages. The patch causing the problem, actually treat
zero pages as not to sent at source, but another half is missing:
treat not received as zero pages at destination. I guess if second
half is added, problem is gone: after page transfer completed,
before destination resume, fill zero in not received pages.
Make a working patch, we'll discuss it :) I do not see much
acceleration coming from there.
I would also not spent much time with this. I would either look to find
an easy way to fix the initialization code to not unneccessarily load
data into RAM or i will sent a v2 of my patch following Eric's
concerns.
There is no easy way to implement the flag and keep your original patch as
we have to implement this flag in all architectures which got broken by
your patch and I personally can fix only PPC64-pseries but not the others.
Furthermore your revert + new patches perfectly solve the problem, why
would we want to bother now with this new flag which nobody really needs
right now?
Please, please, revert the original patch or I'll try to do it :)
I tried, but there where concerns by the community.
Was here anybody who did not want to revert the patch (besides you)?
I did not notice.
Eric said I should not drop the skipped_pages stuff in the monitor.
Re: [Qemu-devel] [Qemu-trivial] [PATCH] ioport/memory: check that both .read and .write callbacks are defined
Hi, On Mon, Jun 10, 2013 at 3:27 PM, Gerd Hoffmann kra...@redhat.com wrote: Hi, Maybe instead (or in addition to), we should provide a dummy read or write functions -- instead of fixing each such occurence to use its own dummy function Makes sense, especially for write where we can just ignore what the guest attempts to write. Not sure we can have a generic handler for reads. Maybe two, one which returns 0xff and one which returns 0x00. FWIW, I have one in my tree that qemu_log(LOG_GUEST_ERROR's such accesses that I use for unimplemented devices. It's worthwhile to trap such accesses and speaking for the Xilinx LQSPI case, my preference is for some form of failure rather than silent write-ignore. And can we have an option where a invalid writes have consistent behavior with unassigned accesses? Regards, Peter cheers, Gerd
Re: [Qemu-devel] [PATCH] curl: refuse to open URL from HTTP server without range support
On Sun, Jun 09, 2013 at 10:34:54AM +0800, Fam Zheng wrote:
@@ -110,14 +111,14 @@ static int curl_sock_cb(CURL *curl, curl_socket_t fd,
int action,
return 0;
}
-static size_t curl_size_cb(void *ptr, size_t size, size_t nmemb, void
*opaque)
+static size_t curl_header_cb(void *ptr, size_t size, size_t nmemb, void
*opaque)
{
-CURLState *s = ((CURLState*)opaque);
+BDRVCURLState *s = opaque;
size_t realsize = size * nmemb;
-size_t fsize;
+const char *accept_line = Accept-Ranges: bytes;
-if(sscanf(ptr, Content-Length: %zd, fsize) == 1) {
-s-s-len = fsize;
+if (strncmp((char *)ptr, accept_line, strlen(accept_line)) == 0) {
+s-accept_range = true;
}
This still assumes ptr is NUL-terminated. You need to pass size * nmemb
instead of strlen(accept_line).
Re: [Qemu-devel] [PATCH v4 00/10] qemu-ga: fsfreeze on Windows using VSS
On Thu, Jun 06, 2013 at 11:06:19AM -0400, Tomoki Sekiyama wrote: changes from v3: -[01/10] Use c++ instead of g++ in configureing C++ compiler if neither $cross_prefix nor $CXX is specified. -[02/10] Add alternative representations as a reserved keywords in qapi.py. -[03/10 (newly added)] modify check-patch.pl to check C++ source code -[04/10] Improve compatibility (POSIX-compliant). -[04/10] Added some error checks. -[05/10] Added --with-win-sdk option to configure path to Windows SDK. -[06/10] Patch v3 09/11 (adding binary .tlb file) is squashed into this patch in order to avoid git-bisect failure. -[06/10 07/10] Fix coding style. -Dropped Patch v3 11/11 (encode error desc of exception with current locale) changes from v2: -[06/11] Fix errors in Windows 7, reported by Li Baiqing changes from v1: - Fix out-tree build by stop using recursive Makefile - Added script to extract VSS SDK headers on POSIX systems using msitools (thanks Paolo) - Remove some unnecessary header files Cool, thanks for the checkpatch.pl and ./configure changes! Stefan
Re: [Qemu-devel] [PATCH v2]booke: timer: Deactivate timer for target_bit above 61
On 10 June 2013 08:55, Bharat Bhushan r65...@freescale.com wrote: QEMU timer supports a maximum timer of INT64_MAX. So starting timer only for time which is calculated using target_bit 62 and deactivate/stop timer if the target bit is above 61. Is this really what the hardware does? Or do we need to set a timer for the maximum QEMU allows and then reset it for the residual time when the first timer expires? thanks -- PMM
Re: [Qemu-devel] [Qemu-ppc] broken incoming migration
On Mon, 2013-06-10 at 19:10 +1000, Alexey Kardashevskiy wrote: I would prefer not to completely drop the patch since it saves bandwidth and resources. I would like migration to do what it should do - send pages no matter what, this is exactly what migration is for. If there any many, many empty pages (which I doubt to be a very often real life case), they could all merged in big consecutive chunks and sent at the end of migration. I tend to agree. The problem of sending empty pages is purely a problem of compression. If the current mechanism is deemed not efficient enough for in the case of having lots of zero-pages, then by all means invent a better packet format for more tightly representing them on the wire, but don't break things by not sending them at all. Cheers, Ben.
Re: [Qemu-devel] [PATCH] script: git script to compile every commit in a range of commits
Hi, On Sat, Jun 8, 2013 at 6:30 AM, Jeff Cody jc...@redhat.com wrote: On Fri, Jun 07, 2013 at 11:51:36AM -0500, Anthony Liguori wrote: Laszlo Ersek ler...@redhat.com writes: On 06/07/13 16:44, Jeff Cody wrote: Thanks. I can either do the above changes for a v2, or as follow on patches. Whichever is easier for you, certainly! I'm fine with the script going-in as is. A suggestion I'll make is to split the script into two parts. Hi Anthony, I'm sorry, but I'm a bit confused by your suggestion. I think I know what you are asking (see below), but I'm not positive. git-bisect run is a terribly useful command and I use a bisect script that looks like this: #!/bin/sh set -e pushd ~/build/qemu make -j1 || exit 1 popd # Add right seed here if test $1; then $@ fi I'm sure we all have bisect scripts like this. What you're proposing is very similar to bisect but instead of doing a binary search, it does a linear search starting from the oldest commit. Basically: I agree that git bisect is useful, but solves a slightly different problem than what I am looking to solve. For instance, in my working branches I have a whole stack of commits that I will rebase and squash into a set of sane patches before submitting. To make sure I don't do something silly in that process, and create a patch X that does not build without patch X+1, I want to explicitly compile each patch, without skipping over any patches. #!/bin/sh refspec=$1 shift git rev-list $refspec | while read commit; do git checkout $commit $@ || exit $? done And indeed, I have a local script called git-foreach to do exactly this. I suspect a nicer version would make a very good addition to the git project. So to bisect for a make check failure, I do: git bisect run bisect.sh make check Or to bisect for a qemu-test failure: git bisect run bisect.sh qemu-test-regress.sh With git-foreach, you can do: git-foreach bisect.sh To do a simple build test. Or you can do: git-foreach git show checkpatch-head.sh etc. Ah! So if I understand correctly, what you are asking is to split the script up into two different scripts: 1.) A 'foreach' framework script to run an arbitrary command over a range of commits, against each commit (i.e. in the place where I run 'make clean, git checkout, configure, and make [lines 188-191], simply do the git checkout and execute a passed script / command). 2.) A second script to perform the complication check, intended to be called by script 1). We could then add additional scripts to be called by the 'foreach' framework patch as desired. Heck, if we wanted to, we could then even create a menu-drive meta-script to interactively run any number of tests (checkpatch, compilation, etc..) using that framework. Make sense to me. I have a little script that does this stuff for me, but my for-each mechanism runs using git am rather than commit ranges and git checkout. Verifies that the series as-about-to-be-sent applies cleanly to the master without build breakage or checkpatch fail. If you make this two stage split developers can choose between either a commit or am based foreach iterator and the second script as your call it is common to both. Regards, Peter Regards, Anthony Liguori Thanks, Jeff
Re: [Qemu-devel] [Qemu-ppc] broken incoming migration
On 10.06.2013 11:33, Benjamin Herrenschmidt wrote: On Mon, 2013-06-10 at 19:10 +1000, Alexey Kardashevskiy wrote: I would prefer not to completely drop the patch since it saves bandwidth and resources. I would like migration to do what it should do - send pages no matter what, this is exactly what migration is for. If there any many, many empty pages (which I doubt to be a very often real life case), they could all merged in big consecutive chunks and sent at the end of migration. I tend to agree. The problem of sending empty pages is purely a problem of compression. If the current mechanism is deemed not efficient enough for in the case of having lots of zero-pages, then by all means invent a better packet format for more tightly representing them on the wire, but don't break things by not sending them at all. Ok, I see the point. I think the paradigm to say that the destination should decide if it needs a page or not is a sound one. Zero pages are quite often depending on the lifetime and the operating system used. But a consecutive range of zero pages is only likely in the bulk stage. I don't know if its reasonable to add a special encoding for that. I will sent a v2 of my previous revert patch addressing Erics concerns shortly. Peter
Re: [Qemu-devel] [PATCH v2]booke: timer: Deactivate timer for target_bit above 61
-Original Message- From: Peter Maydell [mailto:peter.mayd...@linaro.org] Sent: Monday, June 10, 2013 2:56 PM To: Bhushan Bharat-R65777 Cc: qemu-...@nongnu.org; qemu-devel@nongnu.org; ag...@suse.de; Wood Scott- B07421; Bhushan Bharat-R65777 Subject: Re: [Qemu-devel] [PATCH v2]booke: timer: Deactivate timer for target_bit above 61 On 10 June 2013 08:55, Bharat Bhushan r65...@freescale.com wrote: QEMU timer supports a maximum timer of INT64_MAX. So starting timer only for time which is calculated using target_bit 62 and deactivate/stop timer if the target bit is above 61. Is this really what the hardware does? Or do we need to set a timer for the maximum QEMU allows and then reset it for the residual time when the first timer expires? No, this is not how hardware works. But the time with the maximum target bit of 61 (with current range of CPU frequency PowerPC supports) will be many many years. So I think it is pretty safe to stop the timer. Thanks -Bharat thanks -- PMM
[Qemu-devel] [PATCH] virtio-scsi: forward scsibus for virtio-scsi-pci.
From: KONRAD Frederic fred.kon...@greensocs.com
This fix a bug with scsi hotplug on virtio-scsi-pci:
As virtio-scsi-pci doesn't have any scsi bus, we need to forward scsi-hot-add
to the virtio-scsi-device plugged on the virtio-bus.
Reported-by: Alexey Kardashevskiy a...@ozlabs.ru
Signed-off-by: KONRAD Frederic fred.kon...@greensocs.com
---
hw/pci/pci-hotplug.c | 19 +--
1 file changed, 17 insertions(+), 2 deletions(-)
diff --git a/hw/pci/pci-hotplug.c b/hw/pci/pci-hotplug.c
index 12287d1..c708752 100644
--- a/hw/pci/pci-hotplug.c
+++ b/hw/pci/pci-hotplug.c
@@ -30,6 +30,8 @@
#include monitor/monitor.h
#include hw/scsi/scsi.h
#include hw/virtio/virtio-blk.h
+#include hw/virtio/virtio-scsi.h
+#include hw/virtio/virtio-pci.h
#include qemu/config-file.h
#include sysemu/blockdev.h
#include qapi/error.h
@@ -79,13 +81,26 @@ static int scsi_hot_add(Monitor *mon, DeviceState *adapter,
{
SCSIBus *scsibus;
SCSIDevice *scsidev;
+VirtIOPCIProxy *virtio_proxy;
scsibus = (SCSIBus *)
object_dynamic_cast(OBJECT(QLIST_FIRST(adapter-child_bus)),
TYPE_SCSI_BUS);
if (!scsibus) {
- error_report(Device is not a SCSI adapter);
- return -1;
+/*
+ * Check if the adapter is a virtio-scsi-pci, and forward scsi_hot_add
+ * to the virtio-scsi-device.
+ */
+if (!object_dynamic_cast(OBJECT(adapter), TYPE_VIRTIO_SCSI_PCI)) {
+error_report(Device is not a SCSI adapter);
+return -1;
+}
+virtio_proxy = VIRTIO_PCI(adapter);
+adapter = DEVICE(virtio_proxy-bus.vdev);
+scsibus = (SCSIBus *)
+ object_dynamic_cast(OBJECT(QLIST_FIRST(adapter-child_bus)),
+TYPE_SCSI_BUS);
+assert(scsibus);
}
/*
--
1.8.1.4
Re: [Qemu-devel] [PATCH] migration: ensure memory is zeroized at the destination
please ignore this one.
Peter
On 10.06.2013 09:03, Peter Lieven wrote:
migration relies on the target memory to be zeroed out since commit
f1c72795 (migration: do not sent zero pages in bulk stage). however,
there is a subtle case where this breaks migration. if for some reason
a page is zero at the source but not at the destination the destination
memory is corrupted.
this was reported to break migration on pseries and also other platforms
might be affected. to ultimatively make sure the destination memory is zero
at the destination check for it on negotiation of ram blocks.
note: the better fix for this would be to pass a flag to the machine
init functions of all architectures to indicate that the machine is a
migration target and then avoid copying ram images etc. to physical
ram in this case. but this would require a lot of code to be changed
and reviewed.
Signed-off-by: Peter Lieven p...@kamp.de
---
arch_init.c | 10 ++
1 file changed, 10 insertions(+)
diff --git a/arch_init.c b/arch_init.c
index 5d32ecf..458bf8c 100644
--- a/arch_init.c
+++ b/arch_init.c
@@ -799,6 +799,8 @@ static int ram_load(QEMUFile *f, void *opaque, int
version_id)
while (total_ram_bytes) {
RAMBlock *block;
uint8_t len;
+void *base;
+ram_addr_t offset;
len = qemu_get_byte(f);
qemu_get_buffer(f, (uint8_t *)id, len);
@@ -822,6 +824,14 @@ static int ram_load(QEMUFile *f, void *opaque, int
version_id)
goto done;
}
+base = memory_region_get_ram_ptr(block-mr);
+for (offset = 0; offset block-length;
+ offset += TARGET_PAGE_SIZE) {
+if (!is_zero_page(base + offset)) {
+memset(base + offset, 0x00, TARGET_PAGE_SIZE);
+}
+}
+
total_ram_bytes -= length;
}
}
--
Mit freundlichen Grüßen
Peter Lieven
...
KAMP Netzwerkdienste GmbH
Vestische Str. 89-91 | 46117 Oberhausen
Tel: +49 (0) 208.89 402-50 | Fax: +49 (0) 208.89 402-40
p...@kamp.de | http://www.kamp.de
Geschäftsführer: Heiner Lante | Michael Lante
Amtsgericht Duisburg | HRB Nr. 12154
USt-Id-Nr.: DE 120607556
...
[Qemu-devel] [PATCH] block/curl.c: Refuse to open the handle for writes.
This simple patch avoids a segfault in qemu if the user tries to open a curl disk for writing. This was previously part of Fam Zheng's curl patch series, but it stands alone and we shouldn't forget about it. Rich.
[Qemu-devel] [PATCH] block/curl.c: Refuse to open the handle for writes.
From: Richard W.M. Jones rjo...@redhat.com
Signed-off-by: Richard W.M. Jones rjo...@redhat.com
Signed-off-by: Fam Zheng f...@redhat.com
---
block/curl.c | 4
1 file changed, 4 insertions(+)
diff --git a/block/curl.c b/block/curl.c
index b8935fd..f1e302b 100644
--- a/block/curl.c
+++ b/block/curl.c
@@ -406,6 +406,10 @@ static int curl_open(BlockDriverState *bs, QDict *options,
int flags)
static int inited = 0;
+if (flags BDRV_O_RDWR) {
+return -ENOTSUP;
+}
+
opts = qemu_opts_create_nofail(runtime_opts);
qemu_opts_absorb_qdict(opts, options, local_err);
if (error_is_set(local_err)) {
--
1.8.2.1
[Qemu-devel] [PATCHv2 2/2] migration: do not overwrite zero pages
on incoming migration do not memset pages to zero if they already read as zero.
this will allocate a new zero page and consume memory unnecessarily. even
if we madvise a MADV_DONTNEED later this will only deallocate the memory
asynchronously.
Signed-off-by: Peter Lieven p...@kamp.de
---
arch_init.c | 14 --
1 file changed, 8 insertions(+), 6 deletions(-)
diff --git a/arch_init.c b/arch_init.c
index 08fccf6..cf4e1d5 100644
--- a/arch_init.c
+++ b/arch_init.c
@@ -832,14 +832,16 @@ static int ram_load(QEMUFile *f, void *opaque, int
version_id)
}
ch = qemu_get_byte(f);
-memset(host, ch, TARGET_PAGE_SIZE);
+if (ch != 0 || !is_zero_page(host)) {
+memset(host, ch, TARGET_PAGE_SIZE);
#ifndef _WIN32
-if (ch == 0
-(!kvm_enabled() || kvm_has_sync_mmu())
-getpagesize() = TARGET_PAGE_SIZE) {
-qemu_madvise(host, TARGET_PAGE_SIZE, QEMU_MADV_DONTNEED);
-}
+if (ch == 0
+(!kvm_enabled() || kvm_has_sync_mmu())
+getpagesize() = TARGET_PAGE_SIZE) {
+qemu_madvise(host, TARGET_PAGE_SIZE, QEMU_MADV_DONTNEED);
+}
#endif
+}
} else if (flags RAM_SAVE_FLAG_PAGE) {
void *host;
--
1.7.9.5
[Qemu-devel] [PATCHv2 0/2] fix migration of zero pages
There have been reports that migration is broken on pseries by Alexey Kardashevskiy. It turned out that migration will fail in general as soon as a page is zero on the source, but not on the destination. I thus reverted the skipping of zero pages in bulk transfer phase and added a patch that does not (over)write zero pages at the destination. v2: - keep skipped pages fields in monitoring and qmp intact to avoid compatiblity issues. Peter Lieven (2): Revert migration: do not sent zero pages in bulk stage migration: do not overwrite zero pages arch_init.c | 27 --- 1 file changed, 12 insertions(+), 15 deletions(-) -- 1.7.9.5
[Qemu-devel] [PATCHv2 1/2] Revert migration: do not sent zero pages in bulk stage
Not sending zero pages breaks migration if a page is zero
at the source but not at the destination. This can e.g. happen
if different BIOS versions are used at source and destination.
It has also been reported that migration on pseries is completely
broken with this patch.
This effectively reverts commit f1c72795af573b24a7da5eb52375c9aba8a37972.
Conflicts:
arch_init.c
Signed-off-by: Peter Lieven p...@kamp.de
---
arch_init.c | 13 -
1 file changed, 4 insertions(+), 9 deletions(-)
diff --git a/arch_init.c b/arch_init.c
index 5d32ecf..08fccf6 100644
--- a/arch_init.c
+++ b/arch_init.c
@@ -457,15 +457,10 @@ static int ram_save_block(QEMUFile *f, bool last_stage)
bytes_sent = -1;
if (is_zero_page(p)) {
acct_info.dup_pages++;
-if (!ram_bulk_stage) {
-bytes_sent = save_block_hdr(f, block, offset, cont,
-RAM_SAVE_FLAG_COMPRESS);
-qemu_put_byte(f, 0);
-bytes_sent++;
-} else {
-acct_info.skipped_pages++;
-bytes_sent = 0;
-}
+bytes_sent = save_block_hdr(f, block, offset, cont,
+RAM_SAVE_FLAG_COMPRESS);
+qemu_put_byte(f, 0);
+bytes_sent++;
} else if (!ram_bulk_stage migrate_use_xbzrle()) {
current_addr = block-offset + offset;
bytes_sent = save_xbzrle_page(f, p, current_addr, block,
--
1.7.9.5
Re: [Qemu-devel] [PATCH v1 0/3] Serial cleanup
Ping! Any objections to this one going in? perhaps even via trivial queue? Regards, Peter On Mon, Jun 3, 2013 at 3:11 PM, peter.crosthwa...@xilinx.com wrote: From: Peter Crosthwaite peter.crosthwa...@xilinx.com Some cosmetics, refactored to use util/fifo8 for the FIFO8, then factored out some common code. Tested as working on petalogix-ml605 machine model + Linux (has coverage of serial fifo usage). Peter Crosthwaite (3): char/serial: cosmetic fixes. char/serial: Use generic Fifo8 char/serial: serial_ioport_write: Factor out common code hw/char/serial.c | 128 +++ include/hw/char/serial.h | 15 ++ 2 files changed, 56 insertions(+), 87 deletions(-) -- 1.8.3.rc1.44.gb387c77.dirty
Re: [Qemu-devel] [PATCH] xilinx_axienet: Fix bit mask code
On Sun, Jun 09, 2013 at 10:56:20PM +0200, Stefan Weil wrote:
Obviously the code wanted to mask the lower bits but failed to do so
because of a missing .
cppcheck detected a conditional expression which was always true (1 7).
Applied, thanks Stefan
Signed-off-by: Stefan Weil s...@weilnetz.de
---
Please review - I did not look for a Xilinx manual to see whether
the code was correct at all.
Regards,
Stefan Weil
hw/net/xilinx_axienet.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/net/xilinx_axienet.c b/hw/net/xilinx_axienet.c
index 8989e95..2ca1511 100644
--- a/hw/net/xilinx_axienet.c
+++ b/hw/net/xilinx_axienet.c
@@ -575,7 +575,7 @@ static void enet_write(void *opaque, hwaddr addr,
break;
case R_MC:
- value = ((1 7) - 1);
+ value = ((1 7) - 1);
/* Enable the MII. */
if (value MC_EN) {
--
1.7.10.4
Re: [Qemu-devel] [PATCH v1 1/1] xilinx_axidma: Do not set DMA .notify to NULL after notify
On Fri, Jun 07, 2013 at 01:05:38PM +1000, peter.crosthwa...@xilinx.com wrote:
From: Wendy Liang jli...@xilinx.com
If a stream notify function is not ready, it may re-populate the notify call-
back to indicate it should be re-polled later. This break in this usage, as
immediately following the notify() call, .notify is set to NULL. reverse the
ordering of the notify call and NULL assignment accordingly.
[PC: Reworked commit message]
Signed-off-by: Wendy Liang jli...@xilinx.com
Signed-off-by: Peter Crosthwaite peter.crosthwa...@xilinx.com
Applied master, thanks Wendy and Peter.
ACK for stable.
Acked-by: Edgar E. Iglesias edgar.igles...@gmail.com
---
hw/dma/xilinx_axidma.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/hw/dma/xilinx_axidma.c b/hw/dma/xilinx_axidma.c
index 3a3ef8a..50054cf 100644
--- a/hw/dma/xilinx_axidma.c
+++ b/hw/dma/xilinx_axidma.c
@@ -514,8 +514,9 @@ static void axidma_write(void *opaque, hwaddr addr,
break;
}
if (sid == 1 d-notify) {
-d-notify(d-notify_opaque);
+StreamCanPushNotifyFn notifytmp = d-notify;
d-notify = NULL;
+notifytmp(d-notify_opaque);
}
stream_update_irq(s);
}
--
1.8.3.rc1.44.gb387c77.dirty
Re: [Qemu-devel] [PATCH] block/curl.c: Refuse to open the handle for writes.
Am 10.06.2013 um 12:06 hat Richard W.M. Jones geschrieben:
From: Richard W.M. Jones rjo...@redhat.com
Signed-off-by: Richard W.M. Jones rjo...@redhat.com
Signed-off-by: Fam Zheng f...@redhat.com
---
block/curl.c | 4
1 file changed, 4 insertions(+)
diff --git a/block/curl.c b/block/curl.c
index b8935fd..f1e302b 100644
--- a/block/curl.c
+++ b/block/curl.c
@@ -406,6 +406,10 @@ static int curl_open(BlockDriverState *bs, QDict
*options, int flags)
static int inited = 0;
+if (flags BDRV_O_RDWR) {
+return -ENOTSUP;
+}
+
Thanks, I applied this patch to the block branch for now, but I think a
v2 with some better error reporting wouldn't hurt. We can either add a
qerror_report() call or at least change the error code to -EROFS or
something like that.
Kevin
Re: [Qemu-devel] [PATCH 0/5] Trace fixes
On Fri, Jun 07, 2013 at 12:59:16PM +0200, Markus Armbruster wrote: Markus Armbruster (5): cleanup-trace-events.pl: New slavio_misc: Fix slavio_led_mem_readw/_writew tracepoints milkymist-minimac2: Fix minimac2_read/_write tracepoints trace-events: Drop unused events trace-events: Fix up source file comments hw/misc/slavio_misc.c | 2 +- hw/net/milkymist-minimac2.c | 2 +- scripts/cleanup-trace-events.pl | 51 trace-events| 176 4 files changed, 140 insertions(+), 91 deletions(-) create mode 100755 scripts/cleanup-trace-events.pl -- 1.7.11.7 Thanks, applied to my block tree: https://github.com/stefanha/qemu/commits/block Stefan
Re: [Qemu-devel] [PATCH 0/5] Trace fixes
On Fri, Jun 07, 2013 at 12:59:16PM +0200, Markus Armbruster wrote: Markus Armbruster (5): cleanup-trace-events.pl: New slavio_misc: Fix slavio_led_mem_readw/_writew tracepoints milkymist-minimac2: Fix minimac2_read/_write tracepoints trace-events: Drop unused events trace-events: Fix up source file comments hw/misc/slavio_misc.c | 2 +- hw/net/milkymist-minimac2.c | 2 +- scripts/cleanup-trace-events.pl | 51 trace-events| 176 4 files changed, 140 insertions(+), 91 deletions(-) create mode 100755 scripts/cleanup-trace-events.pl -- 1.7.11.7 Err...I meant tracing tree :-) Thanks, applied to my tracing tree: https://github.com/stefanha/qemu/commits/tracing Stefan
[Qemu-devel] [PATCH v2] block/curl.c: Refuse to open the handle for writes.
From: Richard W.M. Jones rjo...@redhat.com
Signed-off-by: Richard W.M. Jones rjo...@redhat.com
Signed-off-by: Fam Zheng f...@redhat.com
---
block/curl.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/block/curl.c b/block/curl.c
index b8935fd..b634ccf 100644
--- a/block/curl.c
+++ b/block/curl.c
@@ -406,6 +406,12 @@ static int curl_open(BlockDriverState *bs, QDict *options,
int flags)
static int inited = 0;
+if (flags BDRV_O_RDWR) {
+qerror_report(ERROR_CLASS_GENERIC_ERROR,
+ curl block device does not support writes);
+return -EROFS;
+}
+
opts = qemu_opts_create_nofail(runtime_opts);
qemu_opts_absorb_qdict(opts, options, local_err);
if (error_is_set(local_err)) {
--
1.8.2.1
[Qemu-devel] [PATCH v2] block/curl.c: Refuse to open the handle for writes.
v2: - Use qerror_report to report an error. - Return -EROFS instead of -ENOTSUP.
Re: [Qemu-devel] [PATCH RFT 1/5] virtio-blk-dataplane: Improve error reporting
On Fri, Jun 07, 2013 at 08:18:56PM +0200, Andreas Färber wrote: Return an Error so that it can be propagated later. Signed-off-by: Andreas Färber afaer...@suse.de --- hw/block/dataplane/virtio-blk.c | 25 + hw/block/dataplane/virtio-blk.h | 5 +++-- hw/block/virtio-blk.c | 8 +++- 3 files changed, 23 insertions(+), 15 deletions(-) Tested-by: Stefan Hajnoczi stefa...@redhat.com Acked-by: Stefan Hajnoczi stefa...@redhat.com
Re: [Qemu-devel] [PATCH v1 0/3] Serial cleanup
Am 10.06.2013 12:23, schrieb Peter Crosthwaite: Ping! Any objections to this one going in? perhaps even via trivial queue? No strong objection, but you are using an unusual 12-char indentation in some places that you may want to check. Otherwise the cosmetic cleanup looks fine to me. Cheers, Andreas On Mon, Jun 3, 2013 at 3:11 PM, peter.crosthwa...@xilinx.com wrote: From: Peter Crosthwaite peter.crosthwa...@xilinx.com Some cosmetics, refactored to use util/fifo8 for the FIFO8, then factored out some common code. Tested as working on petalogix-ml605 machine model + Linux (has coverage of serial fifo usage). Peter Crosthwaite (3): char/serial: cosmetic fixes. char/serial: Use generic Fifo8 char/serial: serial_ioport_write: Factor out common code hw/char/serial.c | 128 +++ include/hw/char/serial.h | 15 ++ 2 files changed, 56 insertions(+), 87 deletions(-) -- 1.8.3.rc1.44.gb387c77.dirty -- SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
Re: [Qemu-devel] [PATCH v2] block/curl.c: Refuse to open the handle for writes.
Am 10.06.2013 um 13:38 hat Richard W.M. Jones geschrieben: From: Richard W.M. Jones rjo...@redhat.com Signed-off-by: Richard W.M. Jones rjo...@redhat.com Signed-off-by: Fam Zheng f...@redhat.com Thanks, applied to the block branch. Kevin
Re: [Qemu-devel] [PATCH] virtio-scsi: forward scsibus for virtio-scsi-pci.
Am 10.06.2013 11:53, schrieb fred.kon...@greensocs.com: From: KONRAD Frederic fred.kon...@greensocs.com This fix a bug with scsi hotplug on virtio-scsi-pci: As virtio-scsi-pci doesn't have any scsi bus, we need to forward scsi-hot-add to the virtio-scsi-device plugged on the virtio-bus. Reported-by: Alexey Kardashevskiy a...@ozlabs.ru Missing Cc: qemu-sta...@nongnu.org Signed-off-by: KONRAD Frederic fred.kon...@greensocs.com Otherwise, Reviewed-by: Andreas Färber afaer...@suse.de Can you add a simple qtest for this hot-add scenario? Andreas -- SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
Re: [Qemu-devel] [PATCH] virtio-scsi: forward scsibus for virtio-scsi-pci.
On 10/06/2013 13:58, Andreas Färber wrote: Am 10.06.2013 11:53, schrieb fred.kon...@greensocs.com: From: KONRAD Frederic fred.kon...@greensocs.com This fix a bug with scsi hotplug on virtio-scsi-pci: As virtio-scsi-pci doesn't have any scsi bus, we need to forward scsi-hot-add to the virtio-scsi-device plugged on the virtio-bus. Reported-by: Alexey Kardashevskiy a...@ozlabs.ru Missing Cc: qemu-sta...@nongnu.org oops, I CC'ed it by hand. Signed-off-by: KONRAD Frederic fred.kon...@greensocs.com Otherwise, Reviewed-by: Andreas Färber afaer...@suse.de Can you add a simple qtest for this hot-add scenario? Yes but I'm not familiar with this, can you point me at this qtest mechanism? Thanks, Fred Andreas
Re: [Qemu-devel] [PATCH v1 0/3] Serial cleanup
Hi Andreas, On Mon, Jun 10, 2013 at 9:49 PM, Andreas Färber afaer...@suse.de wrote: Am 10.06.2013 12:23, schrieb Peter Crosthwaite: Ping! Any objections to this one going in? perhaps even via trivial queue? No strong objection, but you are using an unusual 12-char indentation in some places that you may want to check. I use that indentation when a ? : operator continues to the next line. What indentation scheme should be used in this instance? Regards, Peter Otherwise the cosmetic cleanup looks fine to me. Cheers, Andreas On Mon, Jun 3, 2013 at 3:11 PM, peter.crosthwa...@xilinx.com wrote: From: Peter Crosthwaite peter.crosthwa...@xilinx.com Some cosmetics, refactored to use util/fifo8 for the FIFO8, then factored out some common code. Tested as working on petalogix-ml605 machine model + Linux (has coverage of serial fifo usage). Peter Crosthwaite (3): char/serial: cosmetic fixes. char/serial: Use generic Fifo8 char/serial: serial_ioport_write: Factor out common code hw/char/serial.c | 128 +++ include/hw/char/serial.h | 15 ++ 2 files changed, 56 insertions(+), 87 deletions(-) -- 1.8.3.rc1.44.gb387c77.dirty -- SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
Re: [Qemu-devel] [PATCH v2]booke: timer: Deactivate timer for target_bit above 61
Am 10.06.2013 09:55, schrieb Bharat Bhushan: QEMU timer supports a maximum timer of INT64_MAX. So starting timer only for time which is calculated using target_bit 62 and deactivate/stop timer if the target bit is above 61. This patch also fix the time calculation from target_bit. The code was doing (1 (target_bit + 1)) while this should be (1ULL (target_bit + 1)). Signed-off-by: Bharat Bhushan bharat.bhus...@freescale.com --- v1-v2 - Added booke: timer: in patch subject hw/ppc/ppc_booke.c |8 +++- 1 files changed, 7 insertions(+), 1 deletions(-) diff --git a/hw/ppc/ppc_booke.c b/hw/ppc/ppc_booke.c index e41b036..f4eda15 100644 --- a/hw/ppc/ppc_booke.c +++ b/hw/ppc/ppc_booke.c @@ -133,9 +133,15 @@ static void booke_update_fixed_timer(CPUPPCState *env, ppc_tb_t *tb_env = env-tb_env; uint64_t lapse; uint64_t tb; -uint64_t period = 1 (target_bit + 1); +uint64_t period; uint64_t now; +/* Deactivate timer for target_bit 61 */ +if (target_bit 61) +return; Braces missing and trailing whitespace after return. So IIUC we can only allow 63 bits due to signedness, thus a maximum of (1 62), thus target_bit = 61. Any chance at least the comment can be worded to explain that any better? Maybe also use (target-bit + 1 = 63) or period INT64_MAX as condition? Best regards, Andreas + +period = 1ULL (target_bit + 1); + now = qemu_get_clock_ns(vm_clock); tb = cpu_ppc_get_tb(tb_env, now, tb_env-tb_offset); -- SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
[Qemu-devel] [PATCH v3 06/17] block/curl: drop curl_aio_flush()
.io_flush() is no longer called so drop curl_aio_flush(). The acb[]
array that the function checks is still used in other parts of
block/curl.c. Therefore we cannot remove acb[], it is needed.
Signed-off-by: Stefan Hajnoczi stefa...@redhat.com
---
block/curl.c | 22 +++---
1 file changed, 3 insertions(+), 19 deletions(-)
diff --git a/block/curl.c b/block/curl.c
index b8935fd..2147076 100644
--- a/block/curl.c
+++ b/block/curl.c
@@ -85,7 +85,6 @@ typedef struct BDRVCURLState {
static void curl_clean_state(CURLState *s);
static void curl_multi_do(void *arg);
-static int curl_aio_flush(void *opaque);
static int curl_sock_cb(CURL *curl, curl_socket_t fd, int action,
void *s, void *sp)
@@ -93,14 +92,14 @@ static int curl_sock_cb(CURL *curl, curl_socket_t fd, int
action,
DPRINTF(CURL (AIO): Sock action %d on fd %d\n, action, fd);
switch (action) {
case CURL_POLL_IN:
-qemu_aio_set_fd_handler(fd, curl_multi_do, NULL, curl_aio_flush,
s);
+qemu_aio_set_fd_handler(fd, curl_multi_do, NULL, NULL, s);
break;
case CURL_POLL_OUT:
-qemu_aio_set_fd_handler(fd, NULL, curl_multi_do, curl_aio_flush,
s);
+qemu_aio_set_fd_handler(fd, NULL, curl_multi_do, NULL, s);
break;
case CURL_POLL_INOUT:
qemu_aio_set_fd_handler(fd, curl_multi_do, curl_multi_do,
-curl_aio_flush, s);
+NULL, s);
break;
case CURL_POLL_REMOVE:
qemu_aio_set_fd_handler(fd, NULL, NULL, NULL, NULL);
@@ -479,21 +478,6 @@ out_noclean:
return -EINVAL;
}
-static int curl_aio_flush(void *opaque)
-{
-BDRVCURLState *s = opaque;
-int i, j;
-
-for (i=0; i CURL_NUM_STATES; i++) {
-for(j=0; j CURL_NUM_ACB; j++) {
-if (s-states[i].acb[j]) {
-return 1;
-}
-}
-}
-return 0;
-}
-
static void curl_aio_cancel(BlockDriverAIOCB *blockacb)
{
// Do we have to implement canceling? Seems to work without...
--
1.8.1.4
[Qemu-devel] [PATCH v3 03/17] tests: adjust test-aio to new aio_poll() semantics
aio_poll(ctx, true) will soon block if any fd handlers have been set.
Previously it would only block when .io_flush() returned true.
This means that callers must check their wait condition *before*
aio_poll() to avoid deadlock.
Signed-off-by: Stefan Hajnoczi stefa...@redhat.com
---
tests/test-aio.c | 26 +-
1 file changed, 17 insertions(+), 9 deletions(-)
diff --git a/tests/test-aio.c b/tests/test-aio.c
index c173870..20bf5e6 100644
--- a/tests/test-aio.c
+++ b/tests/test-aio.c
@@ -15,6 +15,13 @@
AioContext *ctx;
+typedef struct {
+EventNotifier e;
+int n;
+int active;
+bool auto_set;
+} EventNotifierTestData;
+
/* Wait until there are no more BHs or AIO requests */
static void wait_for_aio(void)
{
@@ -23,6 +30,14 @@ static void wait_for_aio(void)
}
}
+/* Wait until event notifier becomes inactive */
+static void wait_until_inactive(EventNotifierTestData *data)
+{
+while (data-active 0) {
+aio_poll(ctx, true);
+}
+}
+
/* Simple callbacks for testing. */
typedef struct {
@@ -50,13 +65,6 @@ static void bh_delete_cb(void *opaque)
}
}
-typedef struct {
-EventNotifier e;
-int n;
-int active;
-bool auto_set;
-} EventNotifierTestData;
-
static int event_active_cb(EventNotifier *e)
{
EventNotifierTestData *data = container_of(e, EventNotifierTestData, e);
@@ -281,7 +289,7 @@ static void test_flush_event_notifier(void)
g_assert_cmpint(data.active, ==, 9);
g_assert(aio_poll(ctx, false));
-wait_for_aio();
+wait_until_inactive(data);
g_assert_cmpint(data.n, ==, 10);
g_assert_cmpint(data.active, ==, 0);
g_assert(!aio_poll(ctx, false));
@@ -325,7 +333,7 @@ static void test_wait_event_notifier_noflush(void)
g_assert_cmpint(data.n, ==, 2);
event_notifier_set(dummy.e);
-wait_for_aio();
+wait_until_inactive(dummy);
g_assert_cmpint(data.n, ==, 2);
g_assert_cmpint(dummy.n, ==, 1);
g_assert_cmpint(dummy.active, ==, 0);
--
1.8.1.4
[Qemu-devel] [PATCH v3 00/17] aio: drop io_flush()
v3: * I forgot about this series, time to push it again! * Rebase onto qemu.git/master * Drop now-unused AioFlushHandler typedef [bonzini] This series gets rid of aio's .io_flush() callback. It's based on Paolo's insight that bdrv_drain_all() can be implemented using the bs-tracked_requests list. io_flush() is redundant since the block layer already knows if requests are pending. The point of this effort is to simplify our event loop(s). If we can reduce custom features like io_flush() it becomes easier to unify event loops and reuse glib or other options. This is also important to me for dataplane, since bdrv_drain_all() is one of the synchronization points between threads. QEMU monitor commands invoke bdrv_drain_all() while the block device is accessed from a dataplane thread. Background on io_flush() semantics: The io_flush() handler must return 1 if this aio fd handler is active. That is, requests are pending and we'd like to make progress by monitoring the fd. If io_flush() returns 0, the aio event loop skips monitoring this fd. This is critical for block drivers like iscsi where we have an idle TCP socket which we want to block on *only* when there are pending requests. The series works as follows: Part 1 - stop relying on .io_flush() The first patches change aio_poll() callers to check their termination condition themselves instead of relying on .io_flush(): ceca767 block: stop relying on io_flush() in bdrv_drain_all() 79bda11 dataplane/virtio-blk: check exit conditions before aio_poll() 6f43f3a tests: adjust test-aio to new aio_poll() semantics 59ff663 tests: adjust test-thread-pool to new aio_poll() semantics Part 2 - stop calling .io_flush() from aio_poll() The semantic change to aio_poll() is made here: 7d16e9a aio: stop using .io_flush() Part 3 - drop io_flush() handlers, just pass NULL for the io_flush argument Remove the now dead code from all .io_flush() users: 1b83f95 block/curl: drop curl_aio_flush() 2ecf2ad block/gluster: drop qemu_gluster_aio_flush_cb() 31d9a26 block/iscsi: drop iscsi_process_flush() 93cfb3a block/linux-aio: drop qemu_laio_completion_cb() e7acfe6 block/nbd: drop nbd_have_request() 5fd9bdf block/rbd: drop qemu_rbd_aio_flush_cb() d41d926 block/sheepdog: drop have_co_req() and aio_flush_request() 03cf446 block/ssh: drop return_true() 9373ab6 dataplane/virtio-blk: drop flush_true() and flush_io() 7465ade thread-pool: drop thread_pool_active() 6963b93 tests: drop event_active_cb() Part 4 - remove io_flush arguments from aio functions The big, mechanical patch that drops the io_flush argument: b765f00 aio: drop io_flush argument Note that I split Part 3 from Part 4 so it's easy to review individual block drivers. Stefan Hajnoczi (17): block: stop relying on io_flush() in bdrv_drain_all() dataplane/virtio-blk: check exit conditions before aio_poll() tests: adjust test-aio to new aio_poll() semantics tests: adjust test-thread-pool to new aio_poll() semantics aio: stop using .io_flush() block/curl: drop curl_aio_flush() block/gluster: drop qemu_gluster_aio_flush_cb() block/iscsi: drop iscsi_process_flush() block/linux-aio: drop qemu_laio_completion_cb() block/nbd: drop nbd_have_request() block/rbd: drop qemu_rbd_aio_flush_cb() block/sheepdog: drop have_co_req() and aio_flush_request() block/ssh: drop return_true() dataplane/virtio-blk: drop flush_true() and flush_io() thread-pool: drop thread_pool_active() tests: drop event_active_cb() aio: drop io_flush argument aio-posix.c | 36 ++ aio-win32.c | 37 --- async.c | 4 +- block.c | 47 --- block/curl.c| 25 ++--- block/gluster.c | 21 ++- block/iscsi.c | 10 + block/linux-aio.c | 18 + block/nbd.c | 18 ++--- block/rbd.c | 16 +--- block/sheepdog.c| 33 - block/ssh.c | 12 +- hw/block/dataplane/virtio-blk.c | 25 +++-- include/block/aio.h | 14 +-- main-loop.c | 9 ++--- tests/test-aio.c| 82 + tests/test-thread-pool.c| 24 ++-- thread-pool.c | 11 +- 18 files changed, 155 insertions(+), 287 deletions(-) -- 1.8.1.4
[Qemu-devel] [PATCH v3 01/17] block: stop relying on io_flush() in bdrv_drain_all()
If a block driver has no file descriptors to monitor but there are still
active requests, it can return 1 from .io_flush(). This is used to spin
during synchronous I/O.
Stop relying on .io_flush() and instead check
QLIST_EMPTY(bs-tracked_requests) to decide whether there are active
requests.
This is the first step in removing .io_flush() so that event loops no
longer need to have the concept of synchronous I/O. Eventually we may
be able to kill synchronous I/O completely by running everything in a
coroutine, but that is future work.
Note this patch moves bs-throttled_reqs initialization to bdrv_new() so
that bdrv_requests_pending(bs) can safely access it. In practice bs is
g_malloc0() so the memory is already zeroed but it's safer to initialize
the queue properly.
In bdrv_delete() make sure to call bdrv_make_anon() *after* bdrv_close()
so that the device is still seen by bdrv_drain_all() when iterating
bdrv_states.
Signed-off-by: Stefan Hajnoczi stefa...@redhat.com
---
block.c | 47 ++-
1 file changed, 34 insertions(+), 13 deletions(-)
diff --git a/block.c b/block.c
index 79ad33d..31f7231 100644
--- a/block.c
+++ b/block.c
@@ -148,7 +148,6 @@ static void bdrv_block_timer(void *opaque)
void bdrv_io_limits_enable(BlockDriverState *bs)
{
-qemu_co_queue_init(bs-throttled_reqs);
bs-block_timer = qemu_new_timer_ns(vm_clock, bdrv_block_timer, bs);
bs-io_limits_enabled = true;
}
@@ -305,6 +304,7 @@ BlockDriverState *bdrv_new(const char *device_name)
}
bdrv_iostatus_disable(bs);
notifier_list_init(bs-close_notifiers);
+qemu_co_queue_init(bs-throttled_reqs);
return bs;
}
@@ -1412,6 +1412,35 @@ void bdrv_close_all(void)
}
}
+/* Check if any requests are in-flight (including throttled requests) */
+static bool bdrv_requests_pending(BlockDriverState *bs)
+{
+if (!QLIST_EMPTY(bs-tracked_requests)) {
+return true;
+}
+if (!qemu_co_queue_empty(bs-throttled_reqs)) {
+return true;
+}
+if (bs-file bdrv_requests_pending(bs-file)) {
+return true;
+}
+if (bs-backing_hd bdrv_requests_pending(bs-backing_hd)) {
+return true;
+}
+return false;
+}
+
+static bool bdrv_requests_pending_all(void)
+{
+BlockDriverState *bs;
+QTAILQ_FOREACH(bs, bdrv_states, list) {
+if (bdrv_requests_pending(bs)) {
+return true;
+}
+}
+return false;
+}
+
/*
* Wait for pending requests to complete across all BlockDriverStates
*
@@ -1427,26 +1456,18 @@ void bdrv_close_all(void)
void bdrv_drain_all(void)
{
BlockDriverState *bs;
-bool busy;
-
-do {
-busy = qemu_aio_wait();
+while (bdrv_requests_pending_all()) {
/* FIXME: We do not have timer support here, so this is effectively
* a busy wait.
*/
QTAILQ_FOREACH(bs, bdrv_states, list) {
if (!qemu_co_queue_empty(bs-throttled_reqs)) {
qemu_co_queue_restart_all(bs-throttled_reqs);
-busy = true;
}
}
-} while (busy);
-/* If requests are still pending there is a bug somewhere */
-QTAILQ_FOREACH(bs, bdrv_states, list) {
-assert(QLIST_EMPTY(bs-tracked_requests));
-assert(qemu_co_queue_empty(bs-throttled_reqs));
+qemu_aio_wait();
}
}
@@ -1591,11 +1612,11 @@ void bdrv_delete(BlockDriverState *bs)
assert(!bs-job);
assert(!bs-in_use);
+bdrv_close(bs);
+
/* remove from list, if necessary */
bdrv_make_anon(bs);
-bdrv_close(bs);
-
g_free(bs);
}
--
1.8.1.4
[Qemu-devel] [PATCH v3 07/17] block/gluster: drop qemu_gluster_aio_flush_cb()
Since .io_flush() is no longer called we do not need
qemu_gluster_aio_flush_cb() anymore. It turns out that qemu_aio_count
is unused now and can be dropped.
Signed-off-by: Stefan Hajnoczi stefa...@redhat.com
---
block/gluster.c | 16 +---
1 file changed, 1 insertion(+), 15 deletions(-)
diff --git a/block/gluster.c b/block/gluster.c
index 91acde2..7a69a12 100644
--- a/block/gluster.c
+++ b/block/gluster.c
@@ -32,7 +32,6 @@ typedef struct BDRVGlusterState {
struct glfs *glfs;
int fds[2];
struct glfs_fd *fd;
-int qemu_aio_count;
int event_reader_pos;
GlusterAIOCB *event_acb;
} BDRVGlusterState;
@@ -247,7 +246,6 @@ static void qemu_gluster_complete_aio(GlusterAIOCB *acb,
BDRVGlusterState *s)
ret = -EIO; /* Partial read/write - fail it */
}
-s-qemu_aio_count--;
qemu_aio_release(acb);
cb(opaque, ret);
if (finished) {
@@ -275,13 +273,6 @@ static void qemu_gluster_aio_event_reader(void *opaque)
} while (ret 0 errno == EINTR);
}
-static int qemu_gluster_aio_flush_cb(void *opaque)
-{
-BDRVGlusterState *s = opaque;
-
-return (s-qemu_aio_count 0);
-}
-
/* TODO Convert to fine grained options */
static QemuOptsList runtime_opts = {
.name = gluster,
@@ -348,7 +339,7 @@ static int qemu_gluster_open(BlockDriverState *bs, QDict
*options,
}
fcntl(s-fds[GLUSTER_FD_READ], F_SETFL, O_NONBLOCK);
qemu_aio_set_fd_handler(s-fds[GLUSTER_FD_READ],
-qemu_gluster_aio_event_reader, NULL, qemu_gluster_aio_flush_cb, s);
+qemu_gluster_aio_event_reader, NULL, NULL, s);
out:
qemu_opts_del(opts);
@@ -445,7 +436,6 @@ static void gluster_finish_aiocb(struct glfs_fd *fd,
ssize_t ret, void *arg)
qemu_mutex_lock_iothread(); /* We are in gluster thread context */
acb-common.cb(acb-common.opaque, -EIO);
qemu_aio_release(acb);
-s-qemu_aio_count--;
close(s-fds[GLUSTER_FD_READ]);
close(s-fds[GLUSTER_FD_WRITE]);
qemu_aio_set_fd_handler(s-fds[GLUSTER_FD_READ], NULL, NULL, NULL,
@@ -467,7 +457,6 @@ static BlockDriverAIOCB
*qemu_gluster_aio_rw(BlockDriverState *bs,
offset = sector_num * BDRV_SECTOR_SIZE;
size = nb_sectors * BDRV_SECTOR_SIZE;
-s-qemu_aio_count++;
acb = qemu_aio_get(gluster_aiocb_info, bs, cb, opaque);
acb-size = size;
@@ -488,7 +477,6 @@ static BlockDriverAIOCB
*qemu_gluster_aio_rw(BlockDriverState *bs,
return acb-common;
out:
-s-qemu_aio_count--;
qemu_aio_release(acb);
return NULL;
}
@@ -518,7 +506,6 @@ static BlockDriverAIOCB
*qemu_gluster_aio_flush(BlockDriverState *bs,
acb-size = 0;
acb-ret = 0;
acb-finished = NULL;
-s-qemu_aio_count++;
ret = glfs_fsync_async(s-fd, gluster_finish_aiocb, acb);
if (ret 0) {
@@ -527,7 +514,6 @@ static BlockDriverAIOCB
*qemu_gluster_aio_flush(BlockDriverState *bs,
return acb-common;
out:
-s-qemu_aio_count--;
qemu_aio_release(acb);
return NULL;
}
--
1.8.1.4
[Qemu-devel] [PATCH v3 12/17] block/sheepdog: drop have_co_req() and aio_flush_request()
.io_flush() is no longer called so drop have_co_req() and
aio_flush_request().
Signed-off-by: Stefan Hajnoczi stefa...@redhat.com
---
block/sheepdog.c | 25 +
1 file changed, 5 insertions(+), 20 deletions(-)
diff --git a/block/sheepdog.c b/block/sheepdog.c
index 21a4edf..66918c6 100644
--- a/block/sheepdog.c
+++ b/block/sheepdog.c
@@ -509,13 +509,6 @@ static void restart_co_req(void *opaque)
qemu_coroutine_enter(co, NULL);
}
-static int have_co_req(void *opaque)
-{
-/* this handler is set only when there is a pending request, so
- * always returns 1. */
-return 1;
-}
-
typedef struct SheepdogReqCo {
int sockfd;
SheepdogReq *hdr;
@@ -538,14 +531,14 @@ static coroutine_fn void do_co_req(void *opaque)
unsigned int *rlen = srco-rlen;
co = qemu_coroutine_self();
-qemu_aio_set_fd_handler(sockfd, NULL, restart_co_req, have_co_req, co);
+qemu_aio_set_fd_handler(sockfd, NULL, restart_co_req, NULL, co);
ret = send_co_req(sockfd, hdr, data, wlen);
if (ret 0) {
goto out;
}
-qemu_aio_set_fd_handler(sockfd, restart_co_req, NULL, have_co_req, co);
+qemu_aio_set_fd_handler(sockfd, restart_co_req, NULL, NULL, co);
ret = qemu_co_recv(sockfd, hdr, sizeof(*hdr));
if (ret sizeof(*hdr)) {
@@ -796,14 +789,6 @@ static void co_write_request(void *opaque)
qemu_coroutine_enter(s-co_send, NULL);
}
-static int aio_flush_request(void *opaque)
-{
-BDRVSheepdogState *s = opaque;
-
-return !QLIST_EMPTY(s-inflight_aio_head) ||
-!QLIST_EMPTY(s-pending_aio_head);
-}
-
/*
* Return a socket discriptor to read/write objects.
*
@@ -819,7 +804,7 @@ static int get_sheep_fd(BDRVSheepdogState *s)
return fd;
}
-qemu_aio_set_fd_handler(fd, co_read_response, NULL, aio_flush_request, s);
+qemu_aio_set_fd_handler(fd, co_read_response, NULL, NULL, s);
return fd;
}
@@ -1070,7 +1055,7 @@ static int coroutine_fn add_aio_request(BDRVSheepdogState
*s, AIOReq *aio_req,
qemu_co_mutex_lock(s-lock);
s-co_send = qemu_coroutine_self();
qemu_aio_set_fd_handler(s-fd, co_read_response, co_write_request,
-aio_flush_request, s);
+NULL, s);
socket_set_cork(s-fd, 1);
/* send a header */
@@ -1092,7 +1077,7 @@ static int coroutine_fn add_aio_request(BDRVSheepdogState
*s, AIOReq *aio_req,
socket_set_cork(s-fd, 0);
qemu_aio_set_fd_handler(s-fd, co_read_response, NULL,
-aio_flush_request, s);
+NULL, s);
qemu_co_mutex_unlock(s-lock);
return 0;
--
1.8.1.4
[Qemu-devel] [PATCH v3 02/17] dataplane/virtio-blk: check exit conditions before aio_poll()
Check exit conditions before entering blocking aio_poll(). This is
mainly for consistency since it's unlikely that we are stopping in the
first event loop iteration.
Signed-off-by: Stefan Hajnoczi stefa...@redhat.com
---
hw/block/dataplane/virtio-blk.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/hw/block/dataplane/virtio-blk.c b/hw/block/dataplane/virtio-blk.c
index 0356665..0509d3f 100644
--- a/hw/block/dataplane/virtio-blk.c
+++ b/hw/block/dataplane/virtio-blk.c
@@ -379,9 +379,9 @@ static void *data_plane_thread(void *opaque)
{
VirtIOBlockDataPlane *s = opaque;
-do {
+while (!s-stopping || s-num_reqs 0) {
aio_poll(s-ctx, true);
-} while (!s-stopping || s-num_reqs 0);
+}
return NULL;
}
--
1.8.1.4
[Qemu-devel] [PATCH v3 08/17] block/iscsi: drop iscsi_process_flush()
.io_flush() is no longer called so drop iscsi_process_flush().
Signed-off-by: Stefan Hajnoczi stefa...@redhat.com
---
block/iscsi.c | 9 +
1 file changed, 1 insertion(+), 8 deletions(-)
diff --git a/block/iscsi.c b/block/iscsi.c
index f7199c1..e2041ca 100644
--- a/block/iscsi.c
+++ b/block/iscsi.c
@@ -147,13 +147,6 @@ static const AIOCBInfo iscsi_aiocb_info = {
static void iscsi_process_read(void *arg);
static void iscsi_process_write(void *arg);
-static int iscsi_process_flush(void *arg)
-{
-IscsiLun *iscsilun = arg;
-
-return iscsi_queue_length(iscsilun-iscsi) 0;
-}
-
static void
iscsi_set_events(IscsiLun *iscsilun)
{
@@ -167,7 +160,7 @@ iscsi_set_events(IscsiLun *iscsilun)
qemu_aio_set_fd_handler(iscsi_get_fd(iscsi),
iscsi_process_read,
(ev POLLOUT) ? iscsi_process_write : NULL,
- iscsi_process_flush,
+ NULL,
iscsilun);
}
--
1.8.1.4
[Qemu-devel] [PATCH v3 11/17] block/rbd: drop qemu_rbd_aio_flush_cb()
.io_flush() is no longer called so drop qemu_rbd_aio_flush_cb().
qemu_aio_count is unused now so drop it too.
Signed-off-by: Stefan Hajnoczi stefa...@redhat.com
---
block/rbd.c | 14 +-
1 file changed, 1 insertion(+), 13 deletions(-)
diff --git a/block/rbd.c b/block/rbd.c
index 0f2608b..40e5d55 100644
--- a/block/rbd.c
+++ b/block/rbd.c
@@ -100,7 +100,6 @@ typedef struct BDRVRBDState {
rados_ioctx_t io_ctx;
rbd_image_t image;
char name[RBD_MAX_IMAGE_NAME_SIZE];
-int qemu_aio_count;
char *snap;
int event_reader_pos;
RADOSCB *event_rcb;
@@ -428,19 +427,11 @@ static void qemu_rbd_aio_event_reader(void *opaque)
if (s-event_reader_pos == sizeof(s-event_rcb)) {
s-event_reader_pos = 0;
qemu_rbd_complete_aio(s-event_rcb);
-s-qemu_aio_count--;
}
}
} while (ret 0 errno == EINTR);
}
-static int qemu_rbd_aio_flush_cb(void *opaque)
-{
-BDRVRBDState *s = opaque;
-
-return (s-qemu_aio_count 0);
-}
-
/* TODO Convert to fine grained options */
static QemuOptsList runtime_opts = {
.name = rbd,
@@ -554,7 +545,7 @@ static int qemu_rbd_open(BlockDriverState *bs, QDict
*options, int flags)
fcntl(s-fds[0], F_SETFL, O_NONBLOCK);
fcntl(s-fds[1], F_SETFL, O_NONBLOCK);
qemu_aio_set_fd_handler(s-fds[RBD_FD_READ], qemu_rbd_aio_event_reader,
-NULL, qemu_rbd_aio_flush_cb, s);
+NULL, NULL, s);
qemu_opts_del(opts);
@@ -741,8 +732,6 @@ static BlockDriverAIOCB *rbd_start_aio(BlockDriverState *bs,
off = sector_num * BDRV_SECTOR_SIZE;
size = nb_sectors * BDRV_SECTOR_SIZE;
-s-qemu_aio_count++; /* All the RADOSCB */
-
rcb = g_malloc(sizeof(RADOSCB));
rcb-done = 0;
rcb-acb = acb;
@@ -779,7 +768,6 @@ static BlockDriverAIOCB *rbd_start_aio(BlockDriverState *bs,
failed:
g_free(rcb);
-s-qemu_aio_count--;
qemu_aio_release(acb);
return NULL;
}
--
1.8.1.4
[Qemu-devel] [PATCH v3 05/17] aio: stop using .io_flush()
Now that aio_poll() users check their termination condition themselves,
it is no longer necessary to call .io_flush() handlers.
The behavior of aio_poll() changes as follows:
1. .io_flush() is no longer invoked and file descriptors are *always*
monitored. Previously returning 0 from .io_flush() would skip this file
descriptor.
Due to this change it is essential to check that requests are pending
before calling qemu_aio_wait(). Failure to do so means we block, for
example, waiting for an idle iSCSI socket to become readable when there
are no requests. Currently all qemu_aio_wait()/aio_poll() callers check
before calling.
2. aio_poll() now returns true if progress was made (BH or fd handlers
executed) and false otherwise. Previously it would return true whenever
'busy', which means that .io_flush() returned true. The 'busy' concept
no longer exists so just progress is returned.
Due to this change we need to update tests/test-aio.c which asserts
aio_poll() return values. Note that QEMU doesn't actually rely on these
return values so only tests/test-aio.c cares.
Note that ctx-notifier, the EventNotifier fd used for aio_notify(), is
now handled as a special case. This is a little ugly but maintains
aio_poll() semantics, i.e. aio_notify() does not count as 'progress' and
aio_poll() avoids blocking when the user has not set any fd handlers yet.
Patches after this remove .io_flush() handler code until we can finally
drop the io_flush arguments to aio_set_fd_handler() and friends.
Signed-off-by: Stefan Hajnoczi stefa...@redhat.com
---
aio-posix.c | 29 +
aio-win32.c | 34 ++
tests/test-aio.c | 10 +-
3 files changed, 28 insertions(+), 45 deletions(-)
diff --git a/aio-posix.c b/aio-posix.c
index b68eccd..7d66048 100644
--- a/aio-posix.c
+++ b/aio-posix.c
@@ -23,7 +23,6 @@ struct AioHandler
GPollFD pfd;
IOHandler *io_read;
IOHandler *io_write;
-AioFlushHandler *io_flush;
int deleted;
int pollfds_idx;
void *opaque;
@@ -84,7 +83,6 @@ void aio_set_fd_handler(AioContext *ctx,
/* Update handler with latest information */
node-io_read = io_read;
node-io_write = io_write;
-node-io_flush = io_flush;
node-opaque = opaque;
node-pollfds_idx = -1;
@@ -147,7 +145,11 @@ static bool aio_dispatch(AioContext *ctx)
(revents (G_IO_IN | G_IO_HUP | G_IO_ERR))
node-io_read) {
node-io_read(node-opaque);
-progress = true;
+
+/* aio_notify() does not count as progress */
+if (node-opaque != ctx-notifier) {
+progress = true;
+}
}
if (!node-deleted
(revents (G_IO_OUT | G_IO_ERR))
@@ -173,7 +175,7 @@ bool aio_poll(AioContext *ctx, bool blocking)
{
AioHandler *node;
int ret;
-bool busy, progress;
+bool progress;
progress = false;
@@ -200,20 +202,8 @@ bool aio_poll(AioContext *ctx, bool blocking)
g_array_set_size(ctx-pollfds, 0);
/* fill pollfds */
-busy = false;
QLIST_FOREACH(node, ctx-aio_handlers, node) {
node-pollfds_idx = -1;
-
-/* If there aren't pending AIO operations, don't invoke callbacks.
- * Otherwise, if there are no AIO requests, qemu_aio_wait() would
- * wait indefinitely.
- */
-if (!node-deleted node-io_flush) {
-if (node-io_flush(node-opaque) == 0) {
-continue;
-}
-busy = true;
-}
if (!node-deleted node-pfd.events) {
GPollFD pfd = {
.fd = node-pfd.fd,
@@ -226,8 +216,8 @@ bool aio_poll(AioContext *ctx, bool blocking)
ctx-walking_handlers--;
-/* No AIO operations? Get us out of here */
-if (!busy) {
+/* early return if we only have the aio_notify() fd */
+if (ctx-pollfds-len == 1) {
return progress;
}
@@ -250,6 +240,5 @@ bool aio_poll(AioContext *ctx, bool blocking)
}
}
-assert(progress || busy);
-return true;
+return progress;
}
diff --git a/aio-win32.c b/aio-win32.c
index 38723bf..4309c16 100644
--- a/aio-win32.c
+++ b/aio-win32.c
@@ -23,7 +23,6 @@
struct AioHandler {
EventNotifier *e;
EventNotifierHandler *io_notify;
-AioFlushEventNotifierHandler *io_flush;
GPollFD pfd;
int deleted;
QLIST_ENTRY(AioHandler) node;
@@ -73,7 +72,6 @@ void aio_set_event_notifier(AioContext *ctx,
}
/* Update handler with latest information */
node-io_notify = io_notify;
-node-io_flush = io_flush;
}
aio_notify(ctx);
@@ -96,7 +94,7 @@ bool aio_poll(AioContext *ctx, bool blocking)
{
AioHandler *node;
HANDLE events[MAXIMUM_WAIT_OBJECTS + 1];
-bool busy, progress;
+bool progress;
int count;
progress = false;
@@ -126,7 +124,11 @@ bool aio_poll(AioContext *ctx, bool
[Qemu-devel] [PATCH v3 10/17] block/nbd: drop nbd_have_request()
.io_flush() is no longer called so drop nbd_have_request(). We cannot
drop in_flight since it is still used by other block/nbd.c code.
Signed-off-by: Stefan Hajnoczi stefa...@redhat.com
---
block/nbd.c | 13 +++--
1 file changed, 3 insertions(+), 10 deletions(-)
diff --git a/block/nbd.c b/block/nbd.c
index 30e3b78..80d2b31 100644
--- a/block/nbd.c
+++ b/block/nbd.c
@@ -270,13 +270,6 @@ static void nbd_coroutine_start(BDRVNBDState *s, struct
nbd_request *request)
request-handle = INDEX_TO_HANDLE(s, i);
}
-static int nbd_have_request(void *opaque)
-{
-BDRVNBDState *s = opaque;
-
-return s-in_flight 0;
-}
-
static void nbd_reply_ready(void *opaque)
{
BDRVNBDState *s = opaque;
@@ -333,7 +326,7 @@ static int nbd_co_send_request(BDRVNBDState *s, struct
nbd_request *request,
qemu_co_mutex_lock(s-send_mutex);
s-send_coroutine = qemu_coroutine_self();
qemu_aio_set_fd_handler(s-sock, nbd_reply_ready, nbd_restart_write,
-nbd_have_request, s);
+NULL, s);
if (qiov) {
if (!s-is_unix) {
socket_set_cork(s-sock, 1);
@@ -353,7 +346,7 @@ static int nbd_co_send_request(BDRVNBDState *s, struct
nbd_request *request,
rc = nbd_send_request(s-sock, request);
}
qemu_aio_set_fd_handler(s-sock, nbd_reply_ready, NULL,
-nbd_have_request, s);
+NULL, s);
s-send_coroutine = NULL;
qemu_co_mutex_unlock(s-send_mutex);
return rc;
@@ -430,7 +423,7 @@ static int nbd_establish_connection(BlockDriverState *bs)
* kick the reply mechanism. */
qemu_set_nonblock(sock);
qemu_aio_set_fd_handler(sock, nbd_reply_ready, NULL,
-nbd_have_request, s);
+NULL, s);
s-sock = sock;
s-size = size;
--
1.8.1.4
[Qemu-devel] [PATCH v3 09/17] block/linux-aio: drop qemu_laio_completion_cb()
.io_flush() is no longer called so drop qemu_laio_completion_cb(). It
turns out that count is now unused so drop that too.
Signed-off-by: Stefan Hajnoczi stefa...@redhat.com
---
block/linux-aio.c | 17 ++---
1 file changed, 2 insertions(+), 15 deletions(-)
diff --git a/block/linux-aio.c b/block/linux-aio.c
index ee0f8d1..d9128f3 100644
--- a/block/linux-aio.c
+++ b/block/linux-aio.c
@@ -39,7 +39,6 @@ struct qemu_laiocb {
struct qemu_laio_state {
io_context_t ctx;
EventNotifier e;
-int count;
};
static inline ssize_t io_event_ret(struct io_event *ev)
@@ -55,8 +54,6 @@ static void qemu_laio_process_completion(struct
qemu_laio_state *s,
{
int ret;
-s-count--;
-
ret = laiocb-ret;
if (ret != -ECANCELED) {
if (ret == laiocb-nbytes) {
@@ -101,13 +98,6 @@ static void qemu_laio_completion_cb(EventNotifier *e)
}
}
-static int qemu_laio_flush_cb(EventNotifier *e)
-{
-struct qemu_laio_state *s = container_of(e, struct qemu_laio_state, e);
-
-return (s-count 0) ? 1 : 0;
-}
-
static void laio_cancel(BlockDriverAIOCB *blockacb)
{
struct qemu_laiocb *laiocb = (struct qemu_laiocb *)blockacb;
@@ -177,14 +167,11 @@ BlockDriverAIOCB *laio_submit(BlockDriverState *bs, void
*aio_ctx, int fd,
goto out_free_aiocb;
}
io_set_eventfd(laiocb-iocb, event_notifier_get_fd(s-e));
-s-count++;
if (io_submit(s-ctx, 1, iocbs) 0)
-goto out_dec_count;
+goto out_free_aiocb;
return laiocb-common;
-out_dec_count:
-s-count--;
out_free_aiocb:
qemu_aio_release(laiocb);
return NULL;
@@ -204,7 +191,7 @@ void *laio_init(void)
}
qemu_aio_set_event_notifier(s-e, qemu_laio_completion_cb,
-qemu_laio_flush_cb);
+NULL);
return s;
--
1.8.1.4
[Qemu-devel] [PATCH v3 15/17] thread-pool: drop thread_pool_active()
.io_flush() is no longer called so drop thread_pool_active(). The block
layer is the only thread-pool.c user and it already tracks in-flight
requests, therefore we do not need thread_pool_active().
Signed-off-by: Stefan Hajnoczi stefa...@redhat.com
---
thread-pool.c | 8 +---
1 file changed, 1 insertion(+), 7 deletions(-)
diff --git a/thread-pool.c b/thread-pool.c
index 0ebd4c2..096f007 100644
--- a/thread-pool.c
+++ b/thread-pool.c
@@ -197,12 +197,6 @@ restart:
}
}
-static int thread_pool_active(EventNotifier *notifier)
-{
-ThreadPool *pool = container_of(notifier, ThreadPool, notifier);
-return !QLIST_EMPTY(pool-head);
-}
-
static void thread_pool_cancel(BlockDriverAIOCB *acb)
{
ThreadPoolElement *elem = (ThreadPoolElement *)acb;
@@ -310,7 +304,7 @@ static void thread_pool_init_one(ThreadPool *pool,
AioContext *ctx)
QTAILQ_INIT(pool-request_list);
aio_set_event_notifier(ctx, pool-notifier, event_notifier_ready,
- thread_pool_active);
+ NULL);
}
ThreadPool *thread_pool_new(AioContext *ctx)
--
1.8.1.4
[Qemu-devel] [PATCH v3 04/17] tests: adjust test-thread-pool to new aio_poll() semantics
aio_poll(ctx, true) will soon block when fd handlers have been set.
Previously aio_poll() would return early if all .io_flush() returned
false. This means we need to check the equivalent of the .io_flush()
condition *before* calling aio_poll(ctx, true) to avoid deadlock.
Signed-off-by: Stefan Hajnoczi stefa...@redhat.com
---
tests/test-thread-pool.c | 24
1 file changed, 12 insertions(+), 12 deletions(-)
diff --git a/tests/test-thread-pool.c b/tests/test-thread-pool.c
index 22915aa..f0b2ef1 100644
--- a/tests/test-thread-pool.c
+++ b/tests/test-thread-pool.c
@@ -40,19 +40,13 @@ static void done_cb(void *opaque, int ret)
active--;
}
-/* Wait until all aio and bh activity has finished */
-static void qemu_aio_wait_all(void)
-{
-while (aio_poll(ctx, true)) {
-/* Do nothing */
-}
-}
-
static void test_submit(void)
{
WorkerTestData data = { .n = 0 };
thread_pool_submit(pool, worker_cb, data);
-qemu_aio_wait_all();
+while (data.n == 0) {
+aio_poll(ctx, true);
+}
g_assert_cmpint(data.n, ==, 1);
}
@@ -65,7 +59,9 @@ static void test_submit_aio(void)
/* The callbacks are not called until after the first wait. */
active = 1;
g_assert_cmpint(data.ret, ==, -EINPROGRESS);
-qemu_aio_wait_all();
+while (data.ret == -EINPROGRESS) {
+aio_poll(ctx, true);
+}
g_assert_cmpint(active, ==, 0);
g_assert_cmpint(data.n, ==, 1);
g_assert_cmpint(data.ret, ==, 0);
@@ -103,7 +99,9 @@ static void test_submit_co(void)
/* qemu_aio_wait_all will execute the rest of the coroutine. */
-qemu_aio_wait_all();
+while (data.ret == -EINPROGRESS) {
+aio_poll(ctx, true);
+}
/* Back here after the coroutine has finished. */
@@ -187,7 +185,9 @@ static void test_cancel(void)
}
/* Finish execution and execute any remaining callbacks. */
-qemu_aio_wait_all();
+while (active 0) {
+aio_poll(ctx, true);
+}
g_assert_cmpint(active, ==, 0);
for (i = 0; i 100; i++) {
if (data[i].n == 3) {
--
1.8.1.4
[Qemu-devel] [PATCH v3 16/17] tests: drop event_active_cb()
The .io_flush() handler no longer exists and has no users. Drop the
io_flush argument to aio_set_fd_handler() and related functions.
Signed-off-by: Stefan Hajnoczi stefa...@redhat.com
---
tests/test-aio.c | 22 --
1 file changed, 8 insertions(+), 14 deletions(-)
diff --git a/tests/test-aio.c b/tests/test-aio.c
index 1251952..7b2892a 100644
--- a/tests/test-aio.c
+++ b/tests/test-aio.c
@@ -65,12 +65,6 @@ static void bh_delete_cb(void *opaque)
}
}
-static int event_active_cb(EventNotifier *e)
-{
-EventNotifierTestData *data = container_of(e, EventNotifierTestData, e);
-return data-active 0;
-}
-
static void event_ready_cb(EventNotifier *e)
{
EventNotifierTestData *data = container_of(e, EventNotifierTestData, e);
@@ -239,7 +233,7 @@ static void test_set_event_notifier(void)
{
EventNotifierTestData data = { .n = 0, .active = 0 };
event_notifier_init(data.e, false);
-aio_set_event_notifier(ctx, data.e, event_ready_cb, event_active_cb);
+aio_set_event_notifier(ctx, data.e, event_ready_cb, NULL);
g_assert(!aio_poll(ctx, false));
g_assert_cmpint(data.n, ==, 0);
@@ -253,7 +247,7 @@ static void test_wait_event_notifier(void)
{
EventNotifierTestData data = { .n = 0, .active = 1 };
event_notifier_init(data.e, false);
-aio_set_event_notifier(ctx, data.e, event_ready_cb, event_active_cb);
+aio_set_event_notifier(ctx, data.e, event_ready_cb, NULL);
g_assert(!aio_poll(ctx, false));
g_assert_cmpint(data.n, ==, 0);
g_assert_cmpint(data.active, ==, 1);
@@ -278,7 +272,7 @@ static void test_flush_event_notifier(void)
{
EventNotifierTestData data = { .n = 0, .active = 10, .auto_set = true };
event_notifier_init(data.e, false);
-aio_set_event_notifier(ctx, data.e, event_ready_cb, event_active_cb);
+aio_set_event_notifier(ctx, data.e, event_ready_cb, NULL);
g_assert(!aio_poll(ctx, false));
g_assert_cmpint(data.n, ==, 0);
g_assert_cmpint(data.active, ==, 10);
@@ -318,7 +312,7 @@ static void test_wait_event_notifier_noflush(void)
/* An active event notifier forces aio_poll to look at EventNotifiers. */
event_notifier_init(dummy.e, false);
-aio_set_event_notifier(ctx, dummy.e, event_ready_cb, event_active_cb);
+aio_set_event_notifier(ctx, dummy.e, event_ready_cb, NULL);
event_notifier_set(data.e);
g_assert(aio_poll(ctx, false));
@@ -521,7 +515,7 @@ static void test_source_set_event_notifier(void)
{
EventNotifierTestData data = { .n = 0, .active = 0 };
event_notifier_init(data.e, false);
-aio_set_event_notifier(ctx, data.e, event_ready_cb, event_active_cb);
+aio_set_event_notifier(ctx, data.e, event_ready_cb, NULL);
while (g_main_context_iteration(NULL, false));
g_assert_cmpint(data.n, ==, 0);
@@ -535,7 +529,7 @@ static void test_source_wait_event_notifier(void)
{
EventNotifierTestData data = { .n = 0, .active = 1 };
event_notifier_init(data.e, false);
-aio_set_event_notifier(ctx, data.e, event_ready_cb, event_active_cb);
+aio_set_event_notifier(ctx, data.e, event_ready_cb, NULL);
g_assert(g_main_context_iteration(NULL, false));
g_assert_cmpint(data.n, ==, 0);
g_assert_cmpint(data.active, ==, 1);
@@ -560,7 +554,7 @@ static void test_source_flush_event_notifier(void)
{
EventNotifierTestData data = { .n = 0, .active = 10, .auto_set = true };
event_notifier_init(data.e, false);
-aio_set_event_notifier(ctx, data.e, event_ready_cb, event_active_cb);
+aio_set_event_notifier(ctx, data.e, event_ready_cb, NULL);
g_assert(g_main_context_iteration(NULL, false));
g_assert_cmpint(data.n, ==, 0);
g_assert_cmpint(data.active, ==, 10);
@@ -600,7 +594,7 @@ static void test_source_wait_event_notifier_noflush(void)
/* An active event notifier forces aio_poll to look at EventNotifiers. */
event_notifier_init(dummy.e, false);
-aio_set_event_notifier(ctx, dummy.e, event_ready_cb, event_active_cb);
+aio_set_event_notifier(ctx, dummy.e, event_ready_cb, NULL);
event_notifier_set(data.e);
g_assert(g_main_context_iteration(NULL, false));
--
1.8.1.4
[Qemu-devel] [PATCH v3 17/17] aio: drop io_flush argument
The .io_flush() handler no longer exists and has no users. Drop the
io_flush argument to aio_set_fd_handler() and related functions.
The AioFlushEventNotifierHandler and AioFlushHandler typedefs are no
longer used and are dropped too.
Signed-off-by: Stefan Hajnoczi stefa...@redhat.com
---
aio-posix.c | 7 ++-
aio-win32.c | 3 +--
async.c | 4 ++--
block/curl.c| 9 -
block/gluster.c | 7 +++
block/iscsi.c | 3 +--
block/linux-aio.c | 3 +--
block/nbd.c | 11 ---
block/rbd.c | 4 ++--
block/sheepdog.c| 18 --
block/ssh.c | 4 ++--
hw/block/dataplane/virtio-blk.c | 8
include/block/aio.h | 14 ++
main-loop.c | 9 +++--
tests/test-aio.c| 40
thread-pool.c | 5 ++---
16 files changed, 61 insertions(+), 88 deletions(-)
diff --git a/aio-posix.c b/aio-posix.c
index 7d66048..2440eb9 100644
--- a/aio-posix.c
+++ b/aio-posix.c
@@ -46,7 +46,6 @@ void aio_set_fd_handler(AioContext *ctx,
int fd,
IOHandler *io_read,
IOHandler *io_write,
-AioFlushHandler *io_flush,
void *opaque)
{
AioHandler *node;
@@ -95,12 +94,10 @@ void aio_set_fd_handler(AioContext *ctx,
void aio_set_event_notifier(AioContext *ctx,
EventNotifier *notifier,
-EventNotifierHandler *io_read,
-AioFlushEventNotifierHandler *io_flush)
+EventNotifierHandler *io_read)
{
aio_set_fd_handler(ctx, event_notifier_get_fd(notifier),
- (IOHandler *)io_read, NULL,
- (AioFlushHandler *)io_flush, notifier);
+ (IOHandler *)io_read, NULL, notifier);
}
bool aio_pending(AioContext *ctx)
diff --git a/aio-win32.c b/aio-win32.c
index 4309c16..78b2801 100644
--- a/aio-win32.c
+++ b/aio-win32.c
@@ -30,8 +30,7 @@ struct AioHandler {
void aio_set_event_notifier(AioContext *ctx,
EventNotifier *e,
-EventNotifierHandler *io_notify,
-AioFlushEventNotifierHandler *io_flush)
+EventNotifierHandler *io_notify)
{
AioHandler *node;
diff --git a/async.c b/async.c
index 90fe906..fe2c8bf 100644
--- a/async.c
+++ b/async.c
@@ -174,7 +174,7 @@ aio_ctx_finalize(GSource *source)
AioContext *ctx = (AioContext *) source;
thread_pool_free(ctx-thread_pool);
-aio_set_event_notifier(ctx, ctx-notifier, NULL, NULL);
+aio_set_event_notifier(ctx, ctx-notifier, NULL);
event_notifier_cleanup(ctx-notifier);
g_array_free(ctx-pollfds, TRUE);
}
@@ -214,7 +214,7 @@ AioContext *aio_context_new(void)
event_notifier_init(ctx-notifier, false);
aio_set_event_notifier(ctx, ctx-notifier,
(EventNotifierHandler *)
- event_notifier_test_and_clear, NULL);
+ event_notifier_test_and_clear);
return ctx;
}
diff --git a/block/curl.c b/block/curl.c
index 2147076..e88621a 100644
--- a/block/curl.c
+++ b/block/curl.c
@@ -92,17 +92,16 @@ static int curl_sock_cb(CURL *curl, curl_socket_t fd, int
action,
DPRINTF(CURL (AIO): Sock action %d on fd %d\n, action, fd);
switch (action) {
case CURL_POLL_IN:
-qemu_aio_set_fd_handler(fd, curl_multi_do, NULL, NULL, s);
+qemu_aio_set_fd_handler(fd, curl_multi_do, NULL, s);
break;
case CURL_POLL_OUT:
-qemu_aio_set_fd_handler(fd, NULL, curl_multi_do, NULL, s);
+qemu_aio_set_fd_handler(fd, NULL, curl_multi_do, s);
break;
case CURL_POLL_INOUT:
-qemu_aio_set_fd_handler(fd, curl_multi_do, curl_multi_do,
-NULL, s);
+qemu_aio_set_fd_handler(fd, curl_multi_do, curl_multi_do, s);
break;
case CURL_POLL_REMOVE:
-qemu_aio_set_fd_handler(fd, NULL, NULL, NULL, NULL);
+qemu_aio_set_fd_handler(fd, NULL, NULL, NULL);
break;
}
diff --git a/block/gluster.c b/block/gluster.c
index 7a69a12..3cff308 100644
--- a/block/gluster.c
+++ b/block/gluster.c
@@ -339,7 +339,7 @@ static int qemu_gluster_open(BlockDriverState *bs, QDict
*options,
}
fcntl(s-fds[GLUSTER_FD_READ], F_SETFL, O_NONBLOCK);
qemu_aio_set_fd_handler(s-fds[GLUSTER_FD_READ],
-qemu_gluster_aio_event_reader, NULL, NULL, s);
+qemu_gluster_aio_event_reader, NULL, s);
out:
qemu_opts_del(opts);
@@ -438,8 +438,7 @@ static void
Re: [Qemu-devel] [PATCH] gtk: implement -full-screen and -no-frame
Am 09.06.2013 um 12:30 hat Peter Wu geschrieben: Aiming for GTK as replacement for SDL, features like -full-screen and -no-frame should also be implemented. Bringing the window into full-screen mode is done by faking activating the full screen menu item with a NULL menu item (which currently is not used by gd_menu_full_screen). This is done after showing the windows to make the cursor and menu hidden. Signed-off-by: Peter Wu lekenst...@gmail.com --- include/ui/console.h | 2 +- ui/gtk.c | 10 +- vl.c | 2 +- 3 files changed, 11 insertions(+), 3 deletions(-) diff --git a/include/ui/console.h b/include/ui/console.h index 4307b5f..7174ba9 100644 --- a/include/ui/console.h +++ b/include/ui/console.h @@ -339,6 +339,6 @@ int index_from_keycode(int code); /* gtk.c */ void early_gtk_display_init(void); -void gtk_display_init(DisplayState *ds); +void gtk_display_init(DisplayState *ds, int full_screen, int no_frame); Should the new arguments be bool? Kevin
[Qemu-devel] [PATCH v3 13/17] block/ssh: drop return_true()
.io_flush() is no longer called so drop return_true().
Signed-off-by: Stefan Hajnoczi stefa...@redhat.com
---
block/ssh.c | 10 +-
1 file changed, 1 insertion(+), 9 deletions(-)
diff --git a/block/ssh.c b/block/ssh.c
index 246a70d..ed525cc 100644
--- a/block/ssh.c
+++ b/block/ssh.c
@@ -725,14 +725,6 @@ static void restart_coroutine(void *opaque)
qemu_coroutine_enter(co, NULL);
}
-/* Always true because when we have called set_fd_handler there is
- * always a request being processed.
- */
-static int return_true(void *opaque)
-{
-return 1;
-}
-
static coroutine_fn void set_fd_handler(BDRVSSHState *s)
{
int r;
@@ -751,7 +743,7 @@ static coroutine_fn void set_fd_handler(BDRVSSHState *s)
DPRINTF(s-sock=%d rd_handler=%p wr_handler=%p, s-sock,
rd_handler, wr_handler);
-qemu_aio_set_fd_handler(s-sock, rd_handler, wr_handler, return_true, co);
+qemu_aio_set_fd_handler(s-sock, rd_handler, wr_handler, NULL, co);
}
static coroutine_fn void clear_fd_handler(BDRVSSHState *s)
--
1.8.1.4
[Qemu-devel] [PATCH v3 14/17] dataplane/virtio-blk: drop flush_true() and flush_io()
.io_flush() is no longer called so drop flush_true() and flush_io().
Signed-off-by: Stefan Hajnoczi stefa...@redhat.com
---
hw/block/dataplane/virtio-blk.c | 17 ++---
1 file changed, 2 insertions(+), 15 deletions(-)
diff --git a/hw/block/dataplane/virtio-blk.c b/hw/block/dataplane/virtio-blk.c
index 0509d3f..9e6d32b 100644
--- a/hw/block/dataplane/virtio-blk.c
+++ b/hw/block/dataplane/virtio-blk.c
@@ -264,11 +264,6 @@ static int process_request(IOQueue *ioq, struct iovec
iov[],
}
}
-static int flush_true(EventNotifier *e)
-{
-return true;
-}
-
static void handle_notify(EventNotifier *e)
{
VirtIOBlockDataPlane *s = container_of(e, VirtIOBlockDataPlane,
@@ -348,14 +343,6 @@ static void handle_notify(EventNotifier *e)
}
}
-static int flush_io(EventNotifier *e)
-{
-VirtIOBlockDataPlane *s = container_of(e, VirtIOBlockDataPlane,
- io_notifier);
-
-return s-num_reqs 0;
-}
-
static void handle_io(EventNotifier *e)
{
VirtIOBlockDataPlane *s = container_of(e, VirtIOBlockDataPlane,
@@ -486,7 +473,7 @@ void virtio_blk_data_plane_start(VirtIOBlockDataPlane *s)
exit(1);
}
s-host_notifier = *virtio_queue_get_host_notifier(vq);
-aio_set_event_notifier(s-ctx, s-host_notifier, handle_notify,
flush_true);
+aio_set_event_notifier(s-ctx, s-host_notifier, handle_notify, NULL);
/* Set up ioqueue */
ioq_init(s-ioqueue, s-fd, REQ_MAX);
@@ -494,7 +481,7 @@ void virtio_blk_data_plane_start(VirtIOBlockDataPlane *s)
ioq_put_iocb(s-ioqueue, s-requests[i].iocb);
}
s-io_notifier = *ioq_get_notifier(s-ioqueue);
-aio_set_event_notifier(s-ctx, s-io_notifier, handle_io, flush_io);
+aio_set_event_notifier(s-ctx, s-io_notifier, handle_io, NULL);
s-started = true;
trace_virtio_blk_data_plane_start(s);
--
1.8.1.4
Re: [Qemu-devel] [PATCH] gtk: implement -full-screen and -no-frame
On Monday 10 June 2013 14:33:28 Kevin Wolf wrote: Am 09.06.2013 um 12:30 hat Peter Wu geschrieben: Aiming for GTK as replacement for SDL, features like -full-screen and -no-frame should also be implemented. Bringing the window into full-screen mode is done by faking activating the full screen menu item with a NULL menu item (which currently is not used by gd_menu_full_screen). This is done after showing the windows to make the cursor and menu hidden. Signed-off-by: Peter Wu lekenst...@gmail.com --- include/ui/console.h | 2 +- ui/gtk.c | 10 +- vl.c | 2 +- 3 files changed, 11 insertions(+), 3 deletions(-) diff --git a/include/ui/console.h b/include/ui/console.h index 4307b5f..7174ba9 100644 --- a/include/ui/console.h +++ b/include/ui/console.h @@ -339,6 +339,6 @@ int index_from_keycode(int code); /* gtk.c */ void early_gtk_display_init(void); -void gtk_display_init(DisplayState *ds); +void gtk_display_init(DisplayState *ds, int full_screen, int no_frame); Should the new arguments be bool? Probably yes, but for consistency with the existing types I kept it as int. A future patch could change all uses of int to bool where 1 or 0 are used, do you prefer to use bool here anyway? Regards, Peter
Re: [Qemu-devel] [PATCH v2]booke: timer: Deactivate timer for target_bit above 61
-Original Message- From: Andreas Färber [mailto:afaer...@suse.de] Sent: Monday, June 10, 2013 5:43 PM To: Bhushan Bharat-R65777 Cc: qemu-...@nongnu.org; qemu-devel@nongnu.org; ag...@suse.de; Wood Scott- B07421; Bhushan Bharat-R65777 Subject: Re: [Qemu-devel] [PATCH v2]booke: timer: Deactivate timer for target_bit above 61 Am 10.06.2013 09:55, schrieb Bharat Bhushan: QEMU timer supports a maximum timer of INT64_MAX. So starting timer only for time which is calculated using target_bit 62 and deactivate/stop timer if the target bit is above 61. This patch also fix the time calculation from target_bit. The code was doing (1 (target_bit + 1)) while this should be (1ULL (target_bit + 1)). Signed-off-by: Bharat Bhushan bharat.bhus...@freescale.com --- v1-v2 - Added booke: timer: in patch subject hw/ppc/ppc_booke.c |8 +++- 1 files changed, 7 insertions(+), 1 deletions(-) diff --git a/hw/ppc/ppc_booke.c b/hw/ppc/ppc_booke.c index e41b036..f4eda15 100644 --- a/hw/ppc/ppc_booke.c +++ b/hw/ppc/ppc_booke.c @@ -133,9 +133,15 @@ static void booke_update_fixed_timer(CPUPPCState *env, ppc_tb_t *tb_env = env-tb_env; uint64_t lapse; uint64_t tb; -uint64_t period = 1 (target_bit + 1); +uint64_t period; uint64_t now; +/* Deactivate timer for target_bit 61 */ +if (target_bit 61) +return; Braces missing and trailing whitespace after return. Ok, will correct So IIUC we can only allow 63 bits due to signedness, thus a maximum of (1 62), thus target_bit = 61. Any chance at least the comment can be worded to explain that any better? Maybe also use (target-bit + 1 = 63) or period INT64_MAX as condition? How about this: /* QEMU timer supports a maximum timer of INT64_MAX (0x7fff_). * Run booke fit/wdog timer when * ((1ULL target_bit + 1) 0x4000_), i.e target_bit = 61. * Also the time with this maximum target_bit (with current range of * CPU frequency PowerPC supports) will be many many years. So it is * pretty safe to stop the timer above this threshold. */ Best regards, Andreas + +period = 1ULL (target_bit + 1); + now = qemu_get_clock_ns(vm_clock); tb = cpu_ppc_get_tb(tb_env, now, tb_env-tb_offset); -- SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
Re: [Qemu-devel] [PATCH v4 0/9] Make 'dump-guest-memory' dump in kdump-compressed format
On Mon, 10 Jun 2013 10:15:35 +0800 Qiao Nuohan qiaonuo...@cn.fujitsu.com wrote: On 06/05/2013 10:15 AM, Luiz Capitulino wrote: I can review it until the end of this week. If this series is adding a new argument (which I believe is what it does) then there's only two ways to get this merged: either we wait for full introspection or you add this feature as a new command. I'd prefer to wait for full introspection, but it depends how long it's going to take to get it merged and how much time you're willing to wait. Amos, can you give us an update on that work? Hi Luiz, I would like to get these patches reviewed first. If introspection won't take too much time, I will choose to wait. However, I cannot figure out how long it will take, why not get the parts not related to introspection settled first? What do you mean by settled? We can keep the review cycle going, but to merge this we have two options: we wait for the full introspection or we make this a different command. Seems Amos's draft will not change qapi-schema.json. The point of Amos series is discovery, not conflicts. If we merge your series w/o introspection support, then it's impossible for a mngt app like libvirt to know whether or not a given QEMU version supports your new argument.
Re: [Qemu-devel] [PATCH] chardev: add baud parameter for serial host device
On 06/10/13 10:42, Peter Wu wrote: On Monday 10 June 2013 07:56:01 Gerd Hoffmann wrote: On 06/08/13 23:49, Peter Wu wrote: When QEMU starts, it always changes the serial port parameters including baud rate. This confused my guest which thought it was outputting at 9600 baud while it was in fact changed to 115200. After this patch, I can use `-serial /dev/ttyS0,baud=9600` to override the default baud rate of 115200. I think we should just flip the default to 9600. IIRC this is the power-on default baud rate of the 8250 uart family, so this should be the qemu default too. If a guest wants to use a higher baudrate it has to reprogram the uart anyway (and qemu will apply the guest changes to the host uart). FWIW, when I tried MODE.COM in ms-dos to change the baud rate, `stty -F /dev/ttyS0 -a` still reported 115200 baud. This is on Linux 3.9 if that matters. Hmm, with a linux guest changing the baudrate works just fine. Any chance mode.com takes a shortcut in case it thinks the rate didn't change? Does setting the speed first to 4800, then to 9600 work? Besides this comment, any other feedback on the patch itself? Style is fine. But it appears to paper over some bug, and I'd prefer to find+fix the bug instead of allowing/requiring the user to set the baud rate manually to the correct value. cheers, Gerd
Re: [Qemu-devel] [PATCH] gtk: implement -full-screen and -no-frame
Am 10.06.2013 um 14:43 hat Peter Wu geschrieben: On Monday 10 June 2013 14:33:28 Kevin Wolf wrote: Am 09.06.2013 um 12:30 hat Peter Wu geschrieben: --- a/include/ui/console.h +++ b/include/ui/console.h @@ -339,6 +339,6 @@ int index_from_keycode(int code); /* gtk.c */ void early_gtk_display_init(void); -void gtk_display_init(DisplayState *ds); +void gtk_display_init(DisplayState *ds, int full_screen, int no_frame); Should the new arguments be bool? Probably yes, but for consistency with the existing types I kept it as int. A future patch could change all uses of int to bool where 1 or 0 are used, do you prefer to use bool here anyway? Yes, if you have to respin anyway, I would prefer if you changed it to bool. I mean it's not a big deal, but it moves us one step closer to consistent use of bool for boolean values, so it can't be bad. I don't think we'll ever see one big conversion patch, but as the code is touched over time, ints abused as bools will slowly disappear if we don't introduce new instances. Kevin
[Qemu-devel] [PATCH] curl: Don't set curl options on the handle just before it's going to be deleted.
From: Richard W.M. Jones rjo...@redhat.com (Found by Kamil Dudka) Signed-off-by: Richard W.M. Jones rjo...@redhat.com --- block/curl.c | 1 - 1 file changed, 1 deletion(-) diff --git a/block/curl.c b/block/curl.c index b634ccf..bf31efe 100644 --- a/block/curl.c +++ b/block/curl.c @@ -453,7 +453,6 @@ static int curl_open(BlockDriverState *bs, QDict *options, int flags) goto out; curl_easy_getinfo(state-curl, CURLINFO_CONTENT_LENGTH_DOWNLOAD, d); curl_easy_setopt(state-curl, CURLOPT_WRITEFUNCTION, (void *)curl_read_cb); -curl_easy_setopt(state-curl, CURLOPT_NOBODY, 0); if (d) s-len = (size_t)d; else if(!s-len) -- 1.8.2.1
Re: [Qemu-devel] [PATCH 3/9] block: bdrv_reopen_prepare(): use error_setg_file_open()
On Mon, 10 Jun 2013 10:43:47 +0200
Stefan Hajnoczi stefa...@gmail.com wrote:
On Fri, Jun 07, 2013 at 03:52:29PM -0400, Luiz Capitulino wrote:
Signed-off-by: Luiz Capitulino lcapitul...@redhat.com
---
block.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/block.c b/block.c
index 79ad33d..c78f152 100644
--- a/block.c
+++ b/block.c
@@ -1291,8 +1291,7 @@ int bdrv_reopen_prepare(BDRVReopenState
*reopen_state, BlockReopenQueue *queue,
if (local_err != NULL) {
error_propagate(errp, local_err);
} else {
-error_set(errp, QERR_OPEN_FILE_FAILED,
- reopen_state-bs-filename);
+error_setg_file_open(errp, errno,
reopen_state-bs-filename);
Looking closer, my suggestion was wrong too.
I think QERR_OPEN_FILE_FAILED is simply the wrong error here. We don't
know that the error occurred when trying to open a file.
Right.
errno does not necessarily contain the error value!
There are two ways to fix it (and they're not mutually exclusive):
1. We could review all bdrv_reopen_prepare() methods and make sure they
set errno on failure
2. We set errno=0 before calling the bdrv_reopen_prepare() method and if
there's an error and if errno != 0 we use it, otherwise we set a generic
failed to prepare to reopen image error
Option 1 goes a bit beyond the time I'd like to spent on this series.
Option 2 is quite reasonable.
What do you think?
Re: [Qemu-devel] [PATCH] chardev: add baud parameter for serial host device
Am 10.06.2013 14:58, schrieb Gerd Hoffmann: On 06/10/13 10:42, Peter Wu wrote: On Monday 10 June 2013 07:56:01 Gerd Hoffmann wrote: On 06/08/13 23:49, Peter Wu wrote: When QEMU starts, it always changes the serial port parameters including baud rate. This confused my guest which thought it was outputting at 9600 baud while it was in fact changed to 115200. After this patch, I can use `-serial /dev/ttyS0,baud=9600` to override the default baud rate of 115200. I think we should just flip the default to 9600. IIRC this is the power-on default baud rate of the 8250 uart family, so this should be the qemu default too. If a guest wants to use a higher baudrate it has to reprogram the uart anyway (and qemu will apply the guest changes to the host uart). FWIW, when I tried MODE.COM in ms-dos to change the baud rate, `stty -F /dev/ttyS0 -a` still reported 115200 baud. This is on Linux 3.9 if that matters. Hmm, with a linux guest changing the baudrate works just fine. Any chance mode.com takes a shortcut in case it thinks the rate didn't change? Does setting the speed first to 4800, then to 9600 work? Besides this comment, any other feedback on the patch itself? Style is fine. But it appears to paper over some bug, and I'd prefer to find+fix the bug instead of allowing/requiring the user to set the baud rate manually to the correct value. Well, there's two instances of hardcoded 115200 baudrate: in the chardev for the host and in the device for the guest - I don't see the latter changed here: hw/arm/nseries.c:stw_raw(w ++, 115200); /* u32 console_speed */ hw/char/serial-isa.c:s-baudbase = 115200; hw/char/serial-pci.c:s-baudbase = 115200; hw/char/serial-pci.c:s-baudbase = 115200; hw/display/sm501.c: 115200, chr, DEVICE_NATIVE_ENDIAN); hw/lm32/lm32_hwsetup.h:hwsetup_add_u32(hw, 115200); /* baudrate */ hw/microblaze/petalogix_ml605_mmu.c: irq[5], 115200, serial_hds[0], DEVICE_LITTLE_ENDIAN); hw/mips/mips_mipssim.c:serial_init(0x3f8, env-irq[4], 115200, serial_hds[0], hw/openrisc/openrisc_sim.c: 115200, serial_hds[0], DEVICE_NATIVE_ENDIAN); hw/ppc/ppc405_boards.c:bd.bi_baudrate = 115200; hw/ppc/virtex_ml507.c:serial_mm_init(address_space_mem, 0x83e01003ULL, 2, irq[9], 115200, hw/sparc64/sun4u.c: NULL, 115200, serial_hds[i], DEVICE_BIG_ENDIAN); hw/xtensa/xtensa_lx60.c:115200, serial_hds[0], DEVICE_NATIVE_ENDIAN); qemu-char.c:check_speed(115200); qemu-char.c:#ifdef B1152000 qemu-char.c:check_speed(1152000); qemu-char.c:spd = B115200; qemu-char.c:tty_serial_init(fd, 115200, 'N', 8, 1); slirp/slirp.h:#define DEFAULT_BAUD 115200 (The slirp define seems unused.) So yes, MS-DOS will get reported from hardware that it is at 115200, whatever you set for the chardev on the host side with your changes. Thought I pointed you to that fact on IRC already... Regards, Andreas -- SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
Re: [Qemu-devel] [PATCH 5/9] blockdev: use error_setg_file_open()
On Mon, 10 Jun 2013 10:45:23 +0200 Stefan Hajnoczi stefa...@gmail.com wrote: On Fri, Jun 07, 2013 at 03:52:31PM -0400, Luiz Capitulino wrote: Signed-off-by: Luiz Capitulino lcapitul...@redhat.com --- blockdev.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) s/errno/-ret/g I'll fix this and double check the other patches. Thanks for reviewing Stefan!
Re: [Qemu-devel] [PATCH] chardev: add baud parameter for serial host device
On Monday 10 June 2013 14:58:07 Gerd Hoffmann wrote: On 06/10/13 10:42, Peter Wu wrote: On Monday 10 June 2013 07:56:01 Gerd Hoffmann wrote: On 06/08/13 23:49, Peter Wu wrote: When QEMU starts, it always changes the serial port parameters including baud rate. This confused my guest which thought it was outputting at 9600 baud while it was in fact changed to 115200. After this patch, I can use `-serial /dev/ttyS0,baud=9600` to override the default baud rate of 115200. I think we should just flip the default to 9600. IIRC this is the power-on default baud rate of the 8250 uart family, so this should be the qemu default too. If a guest wants to use a higher baudrate it has to reprogram the uart anyway (and qemu will apply the guest changes to the host uart). FWIW, when I tried MODE.COM in ms-dos to change the baud rate, `stty -F /dev/ttyS0 -a` still reported 115200 baud. This is on Linux 3.9 if that matters. Hmm, with a linux guest changing the baudrate works just fine. Any chance mode.com takes a shortcut in case it thinks the rate didn't change? Does setting the speed first to 4800, then to 9600 work? I can confirm that a Linux guest can correctly control the speed. At home I only have a USB serial, but that shouldn't matter. 1. stty -F /dev/ttyUSB0 reports 9600 2. Start QEMU using: qemu-system-x86_64 -enable-kvm -m 1G -serial /dev/ttyUSB0 \ -cdrom ubuntu-12.04.1-desktop-amd64.iso 3. stty -F /dev/ttyUSB0 reports 115200 before booting the live CD 4. After boot, both the guest and host report 9600 again 5. Changing it using stty -F /dev/ttyS0 from the guest is also visible on both sides. On ms-dos I observe with 1.4.1 and 1.5.0 that the baud rate *is* changed correctly with my USB serial converter. I will have to check with the serial printer again whether I made a mistake before. I did actually have to use `stty -F /dev/ttyS0 raw 9600` to avoid characters being eaten which caused the serial printer to spit out empty lines or hang. Regards, Peter Besides this comment, any other feedback on the patch itself? Style is fine. But it appears to paper over some bug, and I'd prefer to find+fix the bug instead of allowing/requiring the user to set the baud rate manually to the correct value.
Re: [Qemu-devel] [PATCH v1 1/3] char/serial: cosmetic fixes.
Hi Peter,
Am 03.06.2013 07:12, schrieb peter.crosthwa...@xilinx.com:
From: Peter Crosthwaite peter.crosthwa...@xilinx.com
Some cosmetic fixes to char/serial fixing some checkpatch errors.
Cc: qemu-triv...@nongnu.org
Signed-off-by: Peter Crosthwaite peter.crosthwa...@xilinx.com
---
Needed for the next patch to pass checkpatch. Done as sep patch to not
obscure that patch.
hw/char/serial.c | 30 +++---
1 file changed, 19 insertions(+), 11 deletions(-)
diff --git a/hw/char/serial.c b/hw/char/serial.c
index 66b6348..bd6813e 100644
--- a/hw/char/serial.c
+++ b/hw/char/serial.c
@@ -263,8 +263,9 @@ static gboolean serial_xmit(GIOChannel *chan,
GIOCondition cond, void *opaque)
if (s-tsr_retry = 0) {
if (s-fcr UART_FCR_FE) {
s-tsr = fifo_get(s,XMIT_FIFO);
-if (!s-xmit_fifo.count)
+if (!s-xmit_fifo.count) {
s-lsr |= UART_LSR_THRE;
+}
} else if ((s-lsr UART_LSR_THRE)) {
return FALSE;
} else {
@@ -461,10 +462,11 @@ static uint64_t serial_ioport_read(void *opaque, hwaddr
addr, unsigned size)
} else {
if(s-fcr UART_FCR_FE) {
ret = fifo_get(s,RECV_FIFO);
-if (s-recv_fifo.count == 0)
+if (s-recv_fifo.count == 0) {
s-lsr = ~(UART_LSR_DR | UART_LSR_BI);
-else
+} else {
qemu_mod_timer(s-fifo_timeout_timer, qemu_get_clock_ns
(vm_clock) + s-char_transmit_time * 4);
Wanna rebreak this one too in case you respin/pull?
+}
s-timeout_ipending = 0;
} else {
ret = s-rbr;
@@ -534,15 +536,21 @@ static uint64_t serial_ioport_read(void *opaque, hwaddr
addr, unsigned size)
static int serial_can_receive(SerialState *s)
{
if(s-fcr UART_FCR_FE) {
-if(s-recv_fifo.count UART_FIFO_LENGTH)
-/* Advertise (fifo.itl - fifo.count) bytes when count ITL, and 1
if above. If UART_FIFO_LENGTH - fifo.count is
-advertised the effect will be to almost always fill the fifo
completely before the guest has a chance to respond,
-effectively overriding the ITL that the guest has set. */
- return (s-recv_fifo.count = s-recv_fifo.itl) ?
s-recv_fifo.itl - s-recv_fifo.count : 1;
-else
- return 0;
+if (s-recv_fifo.count UART_FIFO_LENGTH) {
+/*
+ * Advertise (fifo.itl - fifo.count) bytes when count ITL, and
1
+ * if above. If UART_FIFO_LENGTH - fifo.count is advertised the
+ * effect will be to almost always fill the fifo completely
before
+ * the guest has a chance to respond, effectively overriding the
ITL
+ * that the guest has set.
+ */
+return (s-recv_fifo.count = s-recv_fifo.itl) ?
+s-recv_fifo.itl - s-recv_fifo.count : 1;
Here I stumbled over the indentation being 5 chars from '(' or 4 chars
within, but the latter doesn't make sense since it's terminated before.
I would've expected 4 chars from block or aligned below '(' or
4-char-indented from there. But I'm not sure if there are any clear
recommendations, so since it's apparently not using tabs (my initial
suspicion), no objection.
Cheers,
Andreas
+} else {
+return 0;
+}
} else {
-return !(s-lsr UART_LSR_DR);
+return !(s-lsr UART_LSR_DR);
}
}
--
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
Re: [Qemu-devel] [PATCH v1 3/3] char/serial: serial_ioport_write: Factor out common code
Am 03.06.2013 07:14, schrieb peter.crosthwa...@xilinx.com: From: Peter Crosthwaite peter.crosthwa...@xilinx.com These three lines are common to both FIFO and regular mode. Just factor them out to outside the if rather than replicate the same lines inside both if and else. Cc: qemu-triv...@nongnu.org Signed-off-by: Peter Crosthwaite peter.crosthwa...@xilinx.com Reviewed-by: Andreas Färber afaer...@suse.de Andreas -- SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
Re: [Qemu-devel] [Qemu-ppc] real cdrom access failure
On 06/10/2013 03:39 PM, Programmingkid wrote: On Jun 9, 2013, at 12:34 PM, Alexander Graf wrote: On 09.06.2013, at 18:28, Programmingkid wrote: I am trying to access the cdrom drive in QEMU 1.5.0, but can't. This is the error I see: qemu-system-ppc: -cdrom /dev/cdrom: could not open disk image /dev/cdrom: No such file or directory. I think this is a bug with version 1.5.0 on Mac OS X. Anybody else notice this problem? Mac OS X doesn't provide a /dev/cdrom link. You have to point it directly to the target device. To get a list of available devices, try $ diskutil list Also make sure that all partitions and file systems on top of the CD-ROM are unmounted (diskutil unmount or just umount), as OSX won't allow direct access to /dev/disk1 otherwise. Alex The -cdrom /dev/cdrom option always worked in the past. Just not with version 1.5.0. Hrm. CC'ing Andreas and Peter. They're the best matches to people knowing their way around OSX host support :). Also Changing qemu-ppc@ to qemu-devel@, as this is 100% unrelated to the ppc target. Alex
Re: [Qemu-devel] [PATCH] chardev: add baud parameter for serial host device
On Monday 10 June 2013 15:28:32 Peter Wu wrote: On Monday 10 June 2013 14:58:07 Gerd Hoffmann wrote: On 06/10/13 10:42, Peter Wu wrote: On Monday 10 June 2013 07:56:01 Gerd Hoffmann wrote: On 06/08/13 23:49, Peter Wu wrote: When QEMU starts, it always changes the serial port parameters including baud rate. This confused my guest which thought it was outputting at 9600 baud while it was in fact changed to 115200. After this patch, I can use `-serial /dev/ttyS0,baud=9600` to override the default baud rate of 115200. I think we should just flip the default to 9600. IIRC this is the power-on default baud rate of the 8250 uart family, so this should be the qemu default too. If a guest wants to use a higher baudrate it has to reprogram the uart anyway (and qemu will apply the guest changes to the host uart). FWIW, when I tried MODE.COM in ms-dos to change the baud rate, `stty -F /dev/ttyS0 -a` still reported 115200 baud. This is on Linux 3.9 if that matters. Hmm, with a linux guest changing the baudrate works just fine. Any chance mode.com takes a shortcut in case it thinks the rate didn't change? Does setting the speed first to 4800, then to 9600 work? I can confirm that a Linux guest can correctly control the speed. At home I only have a USB serial, but that shouldn't matter. 1. stty -F /dev/ttyUSB0 reports 9600 2. Start QEMU using: qemu-system-x86_64 -enable-kvm -m 1G -serial /dev/ttyUSB0 \ -cdrom ubuntu-12.04.1-desktop-amd64.iso 3. stty -F /dev/ttyUSB0 reports 115200 before booting the live CD 4. After boot, both the guest and host report 9600 again 5. Changing it using stty -F /dev/ttyS0 from the guest is also visible on both sides. On ms-dos I observe with 1.4.1 and 1.5.0 that the baud rate *is* changed correctly with my USB serial converter. I will have to check with the serial printer again whether I made a mistake before. I did actually have to use `stty -F /dev/ttyS0 raw 9600` to avoid characters being eaten which caused the serial printer to spit out empty lines or hang. Aha, I just checked that machine again and realised something. The serial cable has only four pins connected to a printer (DB9-DB25), namely RX, TX, DTR and RTS. The remaining cables are cut (originally it was a null modem cable, but that did not work with the printer). So, what is the likely issue here? Having a printer instead of a serial console or the hardware (cables) missing some lines? FYI, when I set the speed and options manually after starting QEMU, the printer(s) work(s) as expected, otherwise I get garbage out. Regards, Peter
Re: [Qemu-devel] [PATCH 3/9] block: bdrv_reopen_prepare(): use error_setg_file_open()
Am 10.06.2013 um 15:21 hat Luiz Capitulino geschrieben:
On Mon, 10 Jun 2013 10:43:47 +0200
Stefan Hajnoczi stefa...@gmail.com wrote:
On Fri, Jun 07, 2013 at 03:52:29PM -0400, Luiz Capitulino wrote:
Signed-off-by: Luiz Capitulino lcapitul...@redhat.com
---
block.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/block.c b/block.c
index 79ad33d..c78f152 100644
--- a/block.c
+++ b/block.c
@@ -1291,8 +1291,7 @@ int bdrv_reopen_prepare(BDRVReopenState
*reopen_state, BlockReopenQueue *queue,
if (local_err != NULL) {
error_propagate(errp, local_err);
} else {
-error_set(errp, QERR_OPEN_FILE_FAILED,
- reopen_state-bs-filename);
+error_setg_file_open(errp, errno,
reopen_state-bs-filename);
Looking closer, my suggestion was wrong too.
I think QERR_OPEN_FILE_FAILED is simply the wrong error here. We don't
know that the error occurred when trying to open a file.
Right.
errno does not necessarily contain the error value!
There are two ways to fix it (and they're not mutually exclusive):
1. We could review all bdrv_reopen_prepare() methods and make sure they
set errno on failure
2. We set errno=0 before calling the bdrv_reopen_prepare() method and if
there's an error and if errno != 0 we use it, otherwise we set a generic
failed to prepare to reopen image error
Option 1 goes a bit beyond the time I'd like to spent on this series.
Option 2 is quite reasonable.
What do you think?
errno is definitely not reliable here and won't be. You must use -ret if
you want a meaningful error message.
I think Stefan's point was more that Could not open might not be the
right message for a reopen, so we'd have to use error_setg_errno()
directly with a different message here, like Could not prepare reopen
for '%s'.
The _real_ solution, of course, is to make bdrv_prepare_reopen() a void
function and always set errp when something goes wrong.
Kevin
Re: [Qemu-devel] [PATCH 3/9] block: bdrv_reopen_prepare(): use error_setg_file_open()
On Mon, 10 Jun 2013 15:54:10 +0200
Kevin Wolf kw...@redhat.com wrote:
Am 10.06.2013 um 15:21 hat Luiz Capitulino geschrieben:
On Mon, 10 Jun 2013 10:43:47 +0200
Stefan Hajnoczi stefa...@gmail.com wrote:
On Fri, Jun 07, 2013 at 03:52:29PM -0400, Luiz Capitulino wrote:
Signed-off-by: Luiz Capitulino lcapitul...@redhat.com
---
block.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/block.c b/block.c
index 79ad33d..c78f152 100644
--- a/block.c
+++ b/block.c
@@ -1291,8 +1291,7 @@ int bdrv_reopen_prepare(BDRVReopenState
*reopen_state, BlockReopenQueue *queue,
if (local_err != NULL) {
error_propagate(errp, local_err);
} else {
-error_set(errp, QERR_OPEN_FILE_FAILED,
- reopen_state-bs-filename);
+error_setg_file_open(errp, errno,
reopen_state-bs-filename);
Looking closer, my suggestion was wrong too.
I think QERR_OPEN_FILE_FAILED is simply the wrong error here. We don't
know that the error occurred when trying to open a file.
Right.
errno does not necessarily contain the error value!
There are two ways to fix it (and they're not mutually exclusive):
1. We could review all bdrv_reopen_prepare() methods and make sure they
set errno on failure
2. We set errno=0 before calling the bdrv_reopen_prepare() method and if
there's an error and if errno != 0 we use it, otherwise we set a generic
failed to prepare to reopen image error
Option 1 goes a bit beyond the time I'd like to spent on this series.
Option 2 is quite reasonable.
What do you think?
errno is definitely not reliable here and won't be. You must use -ret if
you want a meaningful error message.
I think Stefan's point was more that Could not open might not be the
right message for a reopen, so we'd have to use error_setg_errno()
directly with a different message here, like Could not prepare reopen
for '%s'.
Ok, but Stefan also said that -ret is not reliable. And after quickly
checking the code I see he's right, as there are methods that return -1.
I think the safest thing to do is to have a generic error message for
for this now and in the future we should propagate errp or return -errno.
Re: [Qemu-devel] [PATCH] vmdk: byteswap VMDK4Header.desc_offset field
Am 10.06.2013 um 11:07 hat Stefan Hajnoczi geschrieben:
Remember to byteswap VMDK4Header.desc_offset on big-endian machines.
Cc: qemu-sta...@nongnu.org
Signed-off-by: Stefan Hajnoczi stefa...@redhat.com
Thanks, applied to the block layer.
@@ -507,8 +507,11 @@ static int vmdk_open_vmdk4(BlockDriverState *bs,
if (ret 0) {
return ret;
}
-if (header.capacity == 0 header.desc_offset) {
-return vmdk_open_desc_file(bs, flags, header.desc_offset 9);
+if (header.capacity == 0) {
+int64_t desc_offset = le64_to_cpu(header.desc_offset);
+if (desc_offset) {
+return vmdk_open_desc_file(bs, flags, desc_offset 9);
+}
}
Splitting up the if condition wouldn't have been necessary, strictly
speaking. But I don't mind too much here.
Kevin
Re: [Qemu-devel] [PATCH 3/9] block: bdrv_reopen_prepare(): use error_setg_file_open()
Am 10.06.2013 um 16:02 hat Luiz Capitulino geschrieben:
On Mon, 10 Jun 2013 15:54:10 +0200
Kevin Wolf kw...@redhat.com wrote:
Am 10.06.2013 um 15:21 hat Luiz Capitulino geschrieben:
On Mon, 10 Jun 2013 10:43:47 +0200
Stefan Hajnoczi stefa...@gmail.com wrote:
On Fri, Jun 07, 2013 at 03:52:29PM -0400, Luiz Capitulino wrote:
Signed-off-by: Luiz Capitulino lcapitul...@redhat.com
---
block.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/block.c b/block.c
index 79ad33d..c78f152 100644
--- a/block.c
+++ b/block.c
@@ -1291,8 +1291,7 @@ int bdrv_reopen_prepare(BDRVReopenState
*reopen_state, BlockReopenQueue *queue,
if (local_err != NULL) {
error_propagate(errp, local_err);
} else {
-error_set(errp, QERR_OPEN_FILE_FAILED,
- reopen_state-bs-filename);
+error_setg_file_open(errp, errno,
reopen_state-bs-filename);
Looking closer, my suggestion was wrong too.
I think QERR_OPEN_FILE_FAILED is simply the wrong error here. We don't
know that the error occurred when trying to open a file.
Right.
errno does not necessarily contain the error value!
There are two ways to fix it (and they're not mutually exclusive):
1. We could review all bdrv_reopen_prepare() methods and make sure they
set errno on failure
2. We set errno=0 before calling the bdrv_reopen_prepare() method and if
there's an error and if errno != 0 we use it, otherwise we set a
generic
failed to prepare to reopen image error
Option 1 goes a bit beyond the time I'd like to spent on this series.
Option 2 is quite reasonable.
What do you think?
errno is definitely not reliable here and won't be. You must use -ret if
you want a meaningful error message.
I think Stefan's point was more that Could not open might not be the
right message for a reopen, so we'd have to use error_setg_errno()
directly with a different message here, like Could not prepare reopen
for '%s'.
Ok, but Stefan also said that -ret is not reliable. And after quickly
checking the code I see he's right, as there are methods that return -1.
I think the safest thing to do is to have a generic error message for
for this now and in the future we should propagate errp or return -errno.
Ah, yes. I agree, let's make it error_setg() without errno for now.
Kevin
Re: [Qemu-devel] [PATCH qom-cpu 00/59] QOM CPUState, part 10: CPU loops
On Sun, 9 Jun 2013, Andreas Färber wrote:
Hello,
Based on my guest-memory-dump cleanup patches, this large series changes
cpu_single_env, first_cpu, next_cpu and thread_env to CPUState.
As a prerequisite, most open-coded CPU loops are replaced by either
qemu_for_each_cpu() or qemu_get_cpu(). Individual review appreciated!
qemu_init_vcpu() is converted to CPUState and moved away from targets.
cpu_unassigned_access(), cpu_dump_state() and cpu_dump_statistics() are turned
into CPUClass methods. exec/hwaddr.h is modified to allows its use in
qom/cpu.h.
Available for testing at:
git://github.com/afaerber/qemu-cpu.git qom-cpu-10.v1
https://github.com/afaerber/qemu-cpu/commits/qom-cpu-10.v1
Regards,
Andreas
Cc: Anthony Liguori anth...@codemonkey.ws
Cc: Blue Swirl blauwir...@gmail.com
Cc: Aurélien Jarno aurel...@aurel32.net
Cc: Paolo Bonzini pbonz...@redhat.com (cpu_unassigned_access)
Cc: Stefano Stabellini stefano.stabell...@eu.citrix.com (dummy CPU thread
changes)
Cc: Peter Maydell peter.mayd...@linaro.org (hwaddr)
Andreas Färber (59):
kvm: Change kvm_cpu_synchronize_state() argument to CPUState
kvm: Change cpu_synchronize_state() argument to CPUState
cpus: Simplify cpu_synchronize_all_post_reset()
cpus: Simplify cpu_synchronize_all_post_init()
cpus: Simplify pause_all_vcpus()
cpus: Simplify resume_all_vcpus()
cpus: Simplify set_numa_modes()
cpus: Simplify qmp_inject_nmi()
cpus: Simplify hw_error()
cpus: Simplify qemu_tcg_wait_io_event() and qemu_tcg_cpu_thread_fn()
monitor: Simplify do_inject_mce()
gdbstub: Simplify find_cpu()
cpu: Change cpu_exit() argument to CPUState
cpus: Change cpu_thread_is_idle() argument to CPUState
cpus: Change qemu_kvm_wait_io_event() argument to CPUState
kvm: Change kvm_set_signal_mask() argument to CPUState
cpus: Change qemu_kvm_init_cpu_signals() argument to CPUState
cpu: Turn cpu_dump_{state,statistics}() into CPUState hooks
kvm: Change kvm_handle_internal_error() argument to CPUState
kvm: Change kvm_cpu_exec() argument to CPUState
gdbstub: Set gdb_set_stop_cpu() argument to CPUState
cpus: Change cpu_handle_guest_debug() argument to CPUState
cpus: Change qemu_kvm_start_vcpu() argument to CPUState
cpus: Change qemu_dummy_start_vcpu() argument to CPUState
cpu: Change qemu_init_vcpu() argument to CPUState
hwaddr: Make hwaddr type usable beyond softmmu
cpu: Turn cpu_unassigned_access() into a CPUState hook
^
git-bisect tells me that this commit breaks Xen HVM support in QEMU
Re: [Qemu-devel] [PATCH] target-mips: 64-bit FPU for user-mode emulation.
On Sun, 9 Jun 2013, Thomas Schwinge wrote:
In my reading of the relevant documents, the latter change is not correct
for o32, and empirically has interesting effects on the glibc math
testsuite, for example. Keeping the FR register unset for o32 I'm
proposing to fix with the following patch:
Correct, unless (until?) the -mfp64 o32 ABI extension is implemented for
Linux, CP0.Status.FR must remain 0 for o32 programs.
diff --git target-mips/translate.c target-mips/translate.c
index 0a53203..51837d4 100644
--- target-mips/translate.c
+++ target-mips/translate.c
@@ -15962,10 +15962,12 @@ void cpu_state_reset(CPUMIPSState *env)
if (env-CP0_Config3 (1 CP0C3_DSPP)) {
env-CP0_Status |= (1 CP0St_MX);
}
-/* Enable 64-bit FPU if the target cpu supports it. */
+# if defined(TARGET_ABI_MIPSN32) || defined(TARGET_ABI_MIPSN64)
+/* Enable 64-bit FPU if the target CPU supports it. */
if (env-active_fpu.fcr0 (1 FCR0_F64)) {
env-CP0_Status |= (1 CP0St_FR);
This is not entirely correct, older 64-bit FPUs, i.e. any before the
MIPS32/MIPS64 rev. 2 ISA (e.g. R4000, R1, 5Kf, etc.) won't have the
CP1.FIR.F64 bit set; it was only defined at that ISA level because for
earlier architecture revisions the type of the FPU could have been
inferred from the type of the CPU. Therefore the condition has to be
changed, perhaps the best way would simply be just checking in the
CP0.Status mask if the FR bit is writable.
Also I suppose there must be an else clause here:
} else {
fprintf(stderr, A 64-bit FPU required for NewABI emulation\n);
exit(1);
or suchlike because the NewABI mandates full 64-bit FPU operation (or the
condtion might be changed to an assertion instead and the emulated
environment checked elsewhere earlier on, because a 64-bit CPU is required
for NewABI operation anyway and a 64-bit CPU can't ever have a 32-bit FPU
-- I don't know QEMU well enough to be sure offhand, please check).
}
+# endif
#else
if (env-hflags MIPS_HFLAG_BMASK) {
/* If the exception was raised from a delay slot,
Maciej
Re: [Qemu-devel] [Qemu-ppc] real cdrom access failure
Am 10.06.2013 15:41, schrieb Alexander Graf: On 06/10/2013 03:39 PM, Programmingkid wrote: On Jun 9, 2013, at 12:34 PM, Alexander Graf wrote: On 09.06.2013, at 18:28, Programmingkid wrote: I am trying to access the cdrom drive in QEMU 1.5.0, but can't. This is the error I see: qemu-system-ppc: -cdrom /dev/cdrom: could not open disk image /dev/cdrom: No such file or directory. I think this is a bug with version 1.5.0 on Mac OS X. Anybody else notice this problem? Mac OS X doesn't provide a /dev/cdrom link. You have to point it directly to the target device. To get a list of available devices, try $ diskutil list Also make sure that all partitions and file systems on top of the CD-ROM are unmounted (diskutil unmount or just umount), as OSX won't allow direct access to /dev/disk1 otherwise. Alex The -cdrom /dev/cdrom option always worked in the past. Just not with version 1.5.0. Hrm. CC'ing Andreas and Peter. They're the best matches to people knowing their way around OSX host support :). The translation of /dev/cdrom happens in block/raw-posix.c:hdev_open(). For v1.5.0 a filename parameter was dropped from the block API, so currently the Mac OS X code is changing the local variable so the modified filename variable never makes it into the QDict *options. :/ Andreas Also Changing qemu-ppc@ to qemu-devel@, as this is 100% unrelated to the ppc target. Alex -- SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
Re: [Qemu-devel] [PATCH v2]booke: timer: Deactivate timer for target_bit above 61
On 06/10/2013 02:47 PM, Bhushan Bharat-R65777 wrote: -Original Message- From: Andreas Färber [mailto:afaer...@suse.de] Sent: Monday, June 10, 2013 5:43 PM To: Bhushan Bharat-R65777 Cc: qemu-...@nongnu.org; qemu-devel@nongnu.org; ag...@suse.de; Wood Scott- B07421; Bhushan Bharat-R65777 Subject: Re: [Qemu-devel] [PATCH v2]booke: timer: Deactivate timer for target_bit above 61 Am 10.06.2013 09:55, schrieb Bharat Bhushan: QEMU timer supports a maximum timer of INT64_MAX. So starting timer only for time which is calculated using target_bit 62 and deactivate/stop timer if the target bit is above 61. This patch also fix the time calculation from target_bit. The code was doing (1 (target_bit + 1)) while this should be (1ULL (target_bit + 1)). Signed-off-by: Bharat Bhushanbharat.bhus...@freescale.com --- v1-v2 - Added booke: timer: in patch subject hw/ppc/ppc_booke.c |8 +++- 1 files changed, 7 insertions(+), 1 deletions(-) diff --git a/hw/ppc/ppc_booke.c b/hw/ppc/ppc_booke.c index e41b036..f4eda15 100644 --- a/hw/ppc/ppc_booke.c +++ b/hw/ppc/ppc_booke.c @@ -133,9 +133,15 @@ static void booke_update_fixed_timer(CPUPPCState *env, ppc_tb_t *tb_env = env-tb_env; uint64_t lapse; uint64_t tb; -uint64_t period = 1 (target_bit + 1); +uint64_t period; uint64_t now; +/* Deactivate timer for target_bit 61 */ +if (target_bit 61) +return; Braces missing and trailing whitespace after return. Ok, will correct So IIUC we can only allow 63 bits due to signedness, thus a maximum of (1 62), thus target_bit= 61. Any chance at least the comment can be worded to explain that any better? Maybe also use (target-bit + 1= 63) or period INT64_MAX as condition? How about this: /* QEMU timer supports a maximum timer of INT64_MAX (0x7fff_). * Run booke fit/wdog timer when * ((1ULL target_bit + 1) 0x4000_), i.e target_bit = 61. * Also the time with this maximum target_bit (with current range of * CPU frequency PowerPC supports) will be many many years. So it is * pretty safe to stop the timer above this threshold. */ How about /* This timeout will take years to trigger. Treat the timer as disabled. */ Alex
Re: [Qemu-devel] [PATCH] vmdk: byteswap VMDK4Header.desc_offset field
Am 10.06.2013 um 16:32 hat Stefan Hajnoczi geschrieben:
On Mon, Jun 10, 2013 at 04:04:55PM +0200, Kevin Wolf wrote:
Am 10.06.2013 um 11:07 hat Stefan Hajnoczi geschrieben:
Remember to byteswap VMDK4Header.desc_offset on big-endian machines.
Cc: qemu-sta...@nongnu.org
Signed-off-by: Stefan Hajnoczi stefa...@redhat.com
Thanks, applied to the block layer.
@@ -507,8 +507,11 @@ static int vmdk_open_vmdk4(BlockDriverState *bs,
if (ret 0) {
return ret;
}
-if (header.capacity == 0 header.desc_offset) {
-return vmdk_open_desc_file(bs, flags, header.desc_offset 9);
+if (header.capacity == 0) {
+int64_t desc_offset = le64_to_cpu(header.desc_offset);
+if (desc_offset) {
+return vmdk_open_desc_file(bs, flags, desc_offset 9);
+}
}
Splitting up the if condition wouldn't have been necessary, strictly
speaking. But I don't mind too much here.
True. The reason I did it is because accessing header.desc_offset
directly is a bad habit. Someone modifying the code might conclude it's
safe to access directly when it actually only works for the limited
cases of zero and non-zero.
It just looks a bit weird because you're still doing the same for
header.capacity and there's no real reason for treating the two fields
differently.
Kevin
Re: [Qemu-devel] [PATCH] vmdk: byteswap VMDK4Header.desc_offset field
On Mon, Jun 10, 2013 at 04:04:55PM +0200, Kevin Wolf wrote:
Am 10.06.2013 um 11:07 hat Stefan Hajnoczi geschrieben:
Remember to byteswap VMDK4Header.desc_offset on big-endian machines.
Cc: qemu-sta...@nongnu.org
Signed-off-by: Stefan Hajnoczi stefa...@redhat.com
Thanks, applied to the block layer.
@@ -507,8 +507,11 @@ static int vmdk_open_vmdk4(BlockDriverState *bs,
if (ret 0) {
return ret;
}
-if (header.capacity == 0 header.desc_offset) {
-return vmdk_open_desc_file(bs, flags, header.desc_offset 9);
+if (header.capacity == 0) {
+int64_t desc_offset = le64_to_cpu(header.desc_offset);
+if (desc_offset) {
+return vmdk_open_desc_file(bs, flags, desc_offset 9);
+}
}
Splitting up the if condition wouldn't have been necessary, strictly
speaking. But I don't mind too much here.
True. The reason I did it is because accessing header.desc_offset
directly is a bad habit. Someone modifying the code might conclude it's
safe to access directly when it actually only works for the limited
cases of zero and non-zero.
Stefan
Re: [Qemu-devel] [PATCH v3 01/17] block: stop relying on io_flush() in bdrv_drain_all()
On Mon, Jun 10, 2013 at 02:25:57PM +0200, Stefan Hajnoczi wrote:
@@ -1427,26 +1456,18 @@ void bdrv_close_all(void)
void bdrv_drain_all(void)
{
BlockDriverState *bs;
-bool busy;
-
-do {
-busy = qemu_aio_wait();
+while (bdrv_requests_pending_all()) {
/* FIXME: We do not have timer support here, so this is effectively
* a busy wait.
*/
QTAILQ_FOREACH(bs, bdrv_states, list) {
if (!qemu_co_queue_empty(bs-throttled_reqs)) {
qemu_co_queue_restart_all(bs-throttled_reqs);
-busy = true;
}
}
-} while (busy);
-/* If requests are still pending there is a bug somewhere */
-QTAILQ_FOREACH(bs, bdrv_states, list) {
-assert(QLIST_EMPTY(bs-tracked_requests));
-assert(qemu_co_queue_empty(bs-throttled_reqs));
+qemu_aio_wait();
}
}
tests/ide-test found an issue here: block.c invokes callbacks from a BH
so we may not yet have completed the request when this loop terminates.
Kevin: can you fold in this patch?
diff --git a/block.c b/block.c
index 31f7231..e176215 100644
--- a/block.c
+++ b/block.c
@@ -1469,6 +1469,9 @@ void bdrv_drain_all(void)
qemu_aio_wait();
}
+
+/* Process pending completion BHs */
+aio_poll(qemu_get_aio_context(), false);
}
/* make a BlockDriverState anonymous by removing from bdrv_state list.
Re: [Qemu-devel] [Qemu-ppc] real cdrom access failure
Am 10.06.2013 um 16:22 hat Andreas Färber geschrieben: Am 10.06.2013 15:41, schrieb Alexander Graf: On 06/10/2013 03:39 PM, Programmingkid wrote: On Jun 9, 2013, at 12:34 PM, Alexander Graf wrote: On 09.06.2013, at 18:28, Programmingkid wrote: I am trying to access the cdrom drive in QEMU 1.5.0, but can't. This is the error I see: qemu-system-ppc: -cdrom /dev/cdrom: could not open disk image /dev/cdrom: No such file or directory. I think this is a bug with version 1.5.0 on Mac OS X. Anybody else notice this problem? Mac OS X doesn't provide a /dev/cdrom link. You have to point it directly to the target device. To get a list of available devices, try $ diskutil list Also make sure that all partitions and file systems on top of the CD-ROM are unmounted (diskutil unmount or just umount), as OSX won't allow direct access to /dev/disk1 otherwise. Alex The -cdrom /dev/cdrom option always worked in the past. Just not with version 1.5.0. Hrm. CC'ing Andreas and Peter. They're the best matches to people knowing their way around OSX host support :). The translation of /dev/cdrom happens in block/raw-posix.c:hdev_open(). For v1.5.0 a filename parameter was dropped from the block API, so currently the Mac OS X code is changing the local variable so the modified filename variable never makes it into the QDict *options. :/ Oh nice, magic filenames. Whoever thought this was a good idea... It's easy enough to fix, just put the string back to the QDict in the end. It feels wrong to do something like this, but if we have been doing it before, I guess we must keep doing so. Kevin
Re: [Qemu-devel] [Qemu-ppc] real cdrom access failure
Am 10.06.2013 16:33, schrieb Kevin Wolf:
Am 10.06.2013 um 16:22 hat Andreas Färber geschrieben:
Am 10.06.2013 15:41, schrieb Alexander Graf:
On 06/10/2013 03:39 PM, Programmingkid wrote:
On Jun 9, 2013, at 12:34 PM, Alexander Graf wrote:
On 09.06.2013, at 18:28, Programmingkid wrote:
I am trying to access the cdrom drive in QEMU 1.5.0, but can't. This
is the error I see: qemu-system-ppc: -cdrom /dev/cdrom: could not
open disk image /dev/cdrom: No such file or directory. I think this
is a bug with version 1.5.0 on Mac OS X. Anybody else notice this
problem?
Mac OS X doesn't provide a /dev/cdrom link. You have to point it
directly to the target device. To get a list of available devices, try
$ diskutil list
Also make sure that all partitions and file systems on top of the
CD-ROM are unmounted (diskutil unmount or just umount), as OSX won't
allow direct access to /dev/disk1 otherwise.
The -cdrom /dev/cdrom option always worked in the past. Just not with
version 1.5.0.
Hrm. CC'ing Andreas and Peter. They're the best matches to people
knowing their way around OSX host support :).
The translation of /dev/cdrom happens in block/raw-posix.c:hdev_open().
For v1.5.0 a filename parameter was dropped from the block API, so
currently the Mac OS X code is changing the local variable so the
modified filename variable never makes it into the QDict *options. :/
Oh nice, magic filenames. Whoever thought this was a good idea...
It's easy enough to fix, just put the string back to the QDict in the
end. It feels wrong to do something like this, but if we have been doing
it before, I guess we must keep doing so.
block.c: * Detect host devices. By convention, /dev/cdrom[N] is always
block/raw-posix.c:if (strstart(filename, /dev/cdrom, NULL))
block/raw-posix.c:if (strstart(filename, /dev/cdrom, NULL)) {
block/raw-win32.c:if (strstart(filename, /dev/cdrom, NULL))
block/raw-win32.c:if (strstart(filename, /dev/cdrom, NULL)) {
I happened to know about this issue because we have similar downstream
block drivers that need to translate the filename and broke with v1.5.
I'll look into fixing this if no one beats me to it. We'll probably need
a g_strdup() since bsdPath[] is on the stack.
Andreas
--
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
Re: [Qemu-devel] [PATCH qom-cpu 00/59] QOM CPUState, part 10: CPU loops
Am 10.06.2013 16:17, schrieb Stefano Stabellini:
On Sun, 9 Jun 2013, Andreas Färber wrote:
Hello,
Based on my guest-memory-dump cleanup patches, this large series changes
cpu_single_env, first_cpu, next_cpu and thread_env to CPUState.
As a prerequisite, most open-coded CPU loops are replaced by either
qemu_for_each_cpu() or qemu_get_cpu(). Individual review appreciated!
qemu_init_vcpu() is converted to CPUState and moved away from targets.
cpu_unassigned_access(), cpu_dump_state() and cpu_dump_statistics() are
turned
into CPUClass methods. exec/hwaddr.h is modified to allows its use in
qom/cpu.h.
Available for testing at:
git://github.com/afaerber/qemu-cpu.git qom-cpu-10.v1
https://github.com/afaerber/qemu-cpu/commits/qom-cpu-10.v1
Regards,
Andreas
Cc: Anthony Liguori anth...@codemonkey.ws
Cc: Blue Swirl blauwir...@gmail.com
Cc: Aurélien Jarno aurel...@aurel32.net
Cc: Paolo Bonzini pbonz...@redhat.com (cpu_unassigned_access)
Cc: Stefano Stabellini stefano.stabell...@eu.citrix.com (dummy CPU thread
changes)
Cc: Peter Maydell peter.mayd...@linaro.org (hwaddr)
Andreas Färber (59):
kvm: Change kvm_cpu_synchronize_state() argument to CPUState
kvm: Change cpu_synchronize_state() argument to CPUState
cpus: Simplify cpu_synchronize_all_post_reset()
cpus: Simplify cpu_synchronize_all_post_init()
cpus: Simplify pause_all_vcpus()
cpus: Simplify resume_all_vcpus()
cpus: Simplify set_numa_modes()
cpus: Simplify qmp_inject_nmi()
cpus: Simplify hw_error()
cpus: Simplify qemu_tcg_wait_io_event() and qemu_tcg_cpu_thread_fn()
monitor: Simplify do_inject_mce()
gdbstub: Simplify find_cpu()
cpu: Change cpu_exit() argument to CPUState
cpus: Change cpu_thread_is_idle() argument to CPUState
cpus: Change qemu_kvm_wait_io_event() argument to CPUState
kvm: Change kvm_set_signal_mask() argument to CPUState
cpus: Change qemu_kvm_init_cpu_signals() argument to CPUState
cpu: Turn cpu_dump_{state,statistics}() into CPUState hooks
kvm: Change kvm_handle_internal_error() argument to CPUState
kvm: Change kvm_cpu_exec() argument to CPUState
gdbstub: Set gdb_set_stop_cpu() argument to CPUState
cpus: Change cpu_handle_guest_debug() argument to CPUState
cpus: Change qemu_kvm_start_vcpu() argument to CPUState
cpus: Change qemu_dummy_start_vcpu() argument to CPUState
cpu: Change qemu_init_vcpu() argument to CPUState
hwaddr: Make hwaddr type usable beyond softmmu
cpu: Turn cpu_unassigned_access() into a CPUState hook
^
git-bisect tells me that this commit breaks Xen HVM support in QEMU
Thanks, I'll look into it - any more detail as to how it breaks?
SIGSEGV, assertion failure, guest misbehavior, ...?
Andreas
--
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
Re: [Qemu-devel] latest qemu with gdb remote not working
Sorry about that, i forgot i am in a mailing list Date: Mon, 3 Jun 2013 06:19:56 -0600 From: ebl...@redhat.com To: mcheun...@hotmail.com CC: qemu-devel@nongnu.org Subject: Re: [Qemu-devel] latest qemu with gdb remote not working On 06/01/2013 02:09 PM, Peter Cheung wrote: i start my qemu by this ~/qemu/bin/qemu-system-x86_64 -hda hd10meg.img -gdb tcp::1234 -k en-us -S -m 256m it was working before, but not for the latest code. When i use GDB to connect to it, it show up a warning message and i was unable to start qemu. Please take a look the attachment. thanks Posting 1 megabyte screenshots is considered poor netiquette - your 1M incoming is multiplied to several hundred megabytes outgoing based on the large number of subscribers to this list, which puts a burden on the mail server. Then, not everyone has cheap fast internet connections, and downloading a megabyte message is not trivial. And several list readers prefer to see only plain text emails by default, where your screenshot requires lots of extra work to even see what you are talking about. Please, don't send images when a copy-and-paste of the terminal contents will do; or at a minimum consider posting the screenshot to a website then posting only the URL to that website in your mail message. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org
