Re: [Qemu-devel] [PATCH 2/2] LAPIC: make lapic support cpu hotplug
On 2011-10-06 03:13, liu ping fan wrote: > On Wed, Oct 5, 2011 at 7:01 PM, Jan Kiszka wrote: >> On 2011-10-05 12:26, liu ping fan wrote: > And make the creation of apic as part of cpu initialization, so > apic's state has been ready, before setting kvm_apic. There is no kvm-apic upstream yet, so it's hard to judge why we need this here. If we do, this has to be a separate patch. But I seriously doubt we need it (my hack worked without it, and that was not because of its hack nature). Sorry, I did not explain it clearly. What I mean is that “env->apic_state” >>> must be prepared >>> before qemu_kvm_cpu_thread_fn() -> ... -> kvm_put_sregs(), where we get >>> apic_base by >>> “ sregs.apic_base = cpu_get_apic_base(env->apic_state);” >>> and then call “kvm_vcpu_ioctl(env, KVM_SET_SREGS, &sregs);” which will >>> finally affect the >>> kvm_apic structure in kernel. >>> >>> But as current code, in pc_new_cpu(), we call apic_init() to initialize >>> apic_state, after cpu_init(), >>> so we can not guarantee the order of apic_state initializaion and the >>> setting to kernel. >>> >>> Because LAPIC is part of x86 chip, I want to move it into cpu_x86_init(), >>> and ensure apic_init() >>> called before thread “qemu_kvm_cpu_thread_fn()” creation. >> >> The LAPIC is part of the CPU, the classic APIC was a dedicated chip. > Sorry, a little puzzle. I think x86 interrupt system consists of two > parts: IOAPIC/LAPIC. > For we have "hw/ioapic.c" to simulate IOAPIC, I think "hw/apic.c" > takes the role as LAPIC, > especially that we create an APICState instance for each CPUX86State, > just like each LAPIC > for x86 CPU in real machine. > So we can consider apic_init() to create a LAPIC instance, rather than > create a "classic APIC"? > > I guess If there is lack of something in IOAPIC/LAPIC bus topology, > that will be the arbitrator of ICC bus, right? > So "the classic APIC was a dedicated chip" what you said, play this > role, right? > Could you tell me a sample chipset of APIC, and I can increase my > knowledge about it, thanks. The 82489DX was used as a discrete APIC on 486 SMP systems. > >> >> For various reasons, a safer approach for creating a new CPU is to stop >> the machine, add the new device models, run cpu_synchronize_post_init on >> that new cpu (looks like you missed that) and then resume everything. >> See >> http://git.kiszka.org/?p=qemu-kvm.git;a=commitdiff;h=be8f21c6b54eac82f7add7ee9d4ecf9cb8ebb320 >> > Great job. And I am interesting about it. Could you give some sample > reason about why we need to stop > the machine, or list all of the reasons, so we can resolve it one by > one. I can not figure out such scenes by myself. > From my view, especially for KVM, the creation of vcpu are protected > well by lock mechanism from other > vcpu threads in kernel, so we need not to stop all of the threads. Maybe I was seeing ghosts: I thought that there is a race window between VCPU_CREATE and the last initialization IOCTL when we allow other VCPUs to interact with the new one already. However, I do not find the scenario again ATM. But if you want to move the VCPU resource initialization completely over the VCPU thread, you also have to handle env->halted in that context. See [1] for this topic and associated todos. And don't forget the cpu_synchronize_post_init. Running this after each VCPU creation directly should also obsolete cpu_synchronize_all_post_init. Jan [1] http://thread.gmane.org/gmane.comp.emulators.qemu/100806 signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] balloon drivers missing in virtio-win-1.1.16.vfd
- Original Message - > From: "Onkar N Mahajan" > To: k...@vger.kernel.org, qemu-devel@nongnu.org > Sent: Thursday, September 29, 2011 6:03:26 AM > Subject: balloon drivers missing in virtio-win-1.1.16.vfd > > virtio_balloon drivers are missing in the virtio-win floppy disk > image > found at > http://alt.fedoraproject.org/pub/alt/virtio-win/latest/images/bin/ > whereas they are present in the ISO image , any specific reason for > this ? Shouldn't they be ideally present ? You probably want to be asking this on the Fedora virt list rather than the kvm & qemu developer list. > > Regards, > Onkar > > > -- > To unsubscribe from this list: send the line "unsubscribe kvm" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html >
[Qemu-devel] buildbot failure in qemu on block_x86_64_debian_6_0
The Buildbot has detected a new failure on builder block_x86_64_debian_6_0 while building qemu. Full details are available at: http://buildbot.b1-systems.de/qemu/builders/block_x86_64_debian_6_0/builds/52 Buildbot URL: http://buildbot.b1-systems.de/qemu/ Buildslave for this Build: yuzuki Build Reason: The Nightly scheduler named 'nightly_block' triggered this build Build Source Stamp: [branch block] HEAD Blamelist: BUILD FAILED: failed git sincerely, -The Buildbot
[Qemu-devel] buildbot failure in qemu on block_i386_debian_6_0
The Buildbot has detected a new failure on builder block_i386_debian_6_0 while building qemu. Full details are available at: http://buildbot.b1-systems.de/qemu/builders/block_i386_debian_6_0/builds/52 Buildbot URL: http://buildbot.b1-systems.de/qemu/ Buildslave for this Build: yuzuki Build Reason: The Nightly scheduler named 'nightly_block' triggered this build Build Source Stamp: [branch block] HEAD Blamelist: BUILD FAILED: failed git sincerely, -The Buildbot
Re: [Qemu-devel] [PATCH 2/2] LAPIC: make lapic support cpu hotplug
On 10/05/2011 08:13 PM, liu ping fan wrote:
On Wed, Oct 5, 2011 at 7:01 PM, Jan Kiszka wrote:
On 2011-10-05 12:26, liu ping fan wrote:
> And make the creation of apic as part of cpu initialization, so
apic's state has been ready, before setting kvm_apic.
There is no kvm-apic upstream yet, so it's hard to judge why we need
this here. If we do, this has to be a separate patch. But I seriously
doubt we need it (my hack worked without it, and that was not because of
its hack nature).
Sorry, I did not explain it clearly. What I mean is that “env->apic_state”
must be prepared
before qemu_kvm_cpu_thread_fn() -> ... -> kvm_put_sregs(), where we get
apic_base by
“ sregs.apic_base = cpu_get_apic_base(env->apic_state);”
and then call “kvm_vcpu_ioctl(env, KVM_SET_SREGS,&sregs);” which will
finally affect the
kvm_apic structure in kernel.
But as current code, in pc_new_cpu(), we call apic_init() to initialize
apic_state, after cpu_init(),
so we can not guarantee the order of apic_state initializaion and the
setting to kernel.
Because LAPIC is part of x86 chip, I want to move it into cpu_x86_init(),
and ensure apic_init()
called before thread “qemu_kvm_cpu_thread_fn()” creation.
The LAPIC is part of the CPU, the classic APIC was a dedicated chip.
Sorry, a little puzzle. I think x86 interrupt system consists of two
parts: IOAPIC/LAPIC.
For we have "hw/ioapic.c" to simulate IOAPIC, I think "hw/apic.c"
takes the role as LAPIC,
especially that we create an APICState instance for each CPUX86State,
just like each LAPIC
for x86 CPU in real machine.
So we can consider apic_init() to create a LAPIC instance, rather than
create a "classic APIC"?
I guess If there is lack of something in IOAPIC/LAPIC bus topology,
that will be the arbitrator of ICC bus, right?
So "the classic APIC was a dedicated chip" what you said, play this
role, right?
Could you tell me a sample chipset of APIC, and I can increase my
knowledge about it, thanks.
I think Jan meant the PIC was a dedicated chip. hw/apic.c implements an LAPIC,
hw/i8259.c implements an I8259A otherwise known as the PIC. hw/ioapic.c
implements an I/O APIC.
Together, the I/O APIC and the LAPIC form an 'APIC Architecture'. Usually, the
legacy PIC can hang off of the BSP LAPIC.
Regards,
Anthony Liguori
For various reasons, a safer approach for creating a new CPU is to stop
the machine, add the new device models, run cpu_synchronize_post_init on
that new cpu (looks like you missed that) and then resume everything.
See
http://git.kiszka.org/?p=qemu-kvm.git;a=commitdiff;h=be8f21c6b54eac82f7add7ee9d4ecf9cb8ebb320
Great job. And I am interesting about it. Could you give some sample
reason about why we need to stop
the machine, or list all of the reasons, so we can resolve it one by
one. I can not figure out such scenes by myself.
From my view, especially for KVM, the creation of vcpu are protected
well by lock mechanism from other
vcpu threads in kernel, so we need not to stop all of the threads.
Hope for suggestion and direct from you, and make a further step for
CPU hot-plug feature,
Thanks and regards,
ping fan
...
diff --git a/hw/icc_bus.c b/hw/icc_bus.c
new file mode 100644
index 000..360ca2a
--- /dev/null
+++ b/hw/icc_bus.c
@@ -0,0 +1,62 @@
+/*
+*/
+#define ICC_BUS_PLUG
+#ifdef ICC_BUS_PLUG
+#include "icc_bus.h"
+
+
+
+struct icc_bus_info icc_info = {
+.qinfo.name = "icc",
+.qinfo.size = sizeof(struct icc_bus),
+.qinfo.props = (Property[]) {
+DEFINE_PROP_END_OF_LIST(),
+}
+
+};
+
+
+static const VMStateDescription vmstate_icc_bus = {
+.name = "icc_bus",
+.version_id = 1,
+.minimum_version_id = 1,
+.minimum_version_id_old = 1,
+.pre_save = NULL,
+.post_load = NULL,
+};
+
+struct icc_bus *g_iccbus;
+
+struct icc_bus *icc_init_bus(DeviceState *parent, const char *name)
+{
+struct icc_bus *bus;
+
+bus = FROM_QBUS(icc_bus, qbus_create(&icc_info.qinfo, parent,
name));
+bus->qbus.allow_hotplug = 1; /* Yes, we can */
+bus->qbus.name = "icc";
+vmstate_register(NULL, -1,&vmstate_icc_bus, bus);
The chipset is the owner of this bus and instantiates it. So it also
provides a vmstate. You can drop this unneeded one here (it's created
via an obsolete API anyway).
No familiar with Qemu bus emulation, keep on learning :) . But what I
thought is,
the x86-ICC bus is not the same as bus like PCI.
For a PCI bus, it lies behind a host bridge, but ICC is shared by all x86
processors in SMP system,
so there is not a outstanding owner. And I right?
ICC is also attached to the chipset (due to the IOAPIC). So it looks
reasonable to me to let the chipset do the lifecycle management as well.
It is the fixed point, CPUs may come and go.
Jan
Re: [Qemu-devel] [PATCH 2/2] LAPIC: make lapic support cpu hotplug
On Wed, Oct 5, 2011 at 7:01 PM, Jan Kiszka wrote:
> On 2011-10-05 12:26, liu ping fan wrote:
>>> > And make the creation of apic as part of cpu initialization, so
apic's state has been ready, before setting kvm_apic.
>>>
>>> There is no kvm-apic upstream yet, so it's hard to judge why we need
>>> this here. If we do, this has to be a separate patch. But I seriously
>>> doubt we need it (my hack worked without it, and that was not because of
>>> its hack nature).
>>>
>>> Sorry, I did not explain it clearly. What I mean is that “env->apic_state”
>> must be prepared
>> before qemu_kvm_cpu_thread_fn() -> ... -> kvm_put_sregs(), where we get
>> apic_base by
>> “ sregs.apic_base = cpu_get_apic_base(env->apic_state);”
>> and then call “kvm_vcpu_ioctl(env, KVM_SET_SREGS, &sregs);” which will
>> finally affect the
>> kvm_apic structure in kernel.
>>
>> But as current code, in pc_new_cpu(), we call apic_init() to initialize
>> apic_state, after cpu_init(),
>> so we can not guarantee the order of apic_state initializaion and the
>> setting to kernel.
>>
>> Because LAPIC is part of x86 chip, I want to move it into cpu_x86_init(),
>> and ensure apic_init()
>> called before thread “qemu_kvm_cpu_thread_fn()” creation.
>
> The LAPIC is part of the CPU, the classic APIC was a dedicated chip.
Sorry, a little puzzle. I think x86 interrupt system consists of two
parts: IOAPIC/LAPIC.
For we have "hw/ioapic.c" to simulate IOAPIC, I think "hw/apic.c"
takes the role as LAPIC,
especially that we create an APICState instance for each CPUX86State,
just like each LAPIC
for x86 CPU in real machine.
So we can consider apic_init() to create a LAPIC instance, rather than
create a "classic APIC"?
I guess If there is lack of something in IOAPIC/LAPIC bus topology,
that will be the arbitrator of ICC bus, right?
So "the classic APIC was a dedicated chip" what you said, play this
role, right?
Could you tell me a sample chipset of APIC, and I can increase my
knowledge about it, thanks.
>
> For various reasons, a safer approach for creating a new CPU is to stop
> the machine, add the new device models, run cpu_synchronize_post_init on
> that new cpu (looks like you missed that) and then resume everything.
> See
> http://git.kiszka.org/?p=qemu-kvm.git;a=commitdiff;h=be8f21c6b54eac82f7add7ee9d4ecf9cb8ebb320
>
Great job. And I am interesting about it. Could you give some sample
reason about why we need to stop
the machine, or list all of the reasons, so we can resolve it one by
one. I can not figure out such scenes by myself.
>From my view, especially for KVM, the creation of vcpu are protected
well by lock mechanism from other
vcpu threads in kernel, so we need not to stop all of the threads.
Hope for suggestion and direct from you, and make a further step for
CPU hot-plug feature,
Thanks and regards,
ping fan
> ...
diff --git a/hw/icc_bus.c b/hw/icc_bus.c
new file mode 100644
index 000..360ca2a
--- /dev/null
+++ b/hw/icc_bus.c
@@ -0,0 +1,62 @@
+/*
+*/
+#define ICC_BUS_PLUG
+#ifdef ICC_BUS_PLUG
+#include "icc_bus.h"
+
+
+
+struct icc_bus_info icc_info = {
+ .qinfo.name = "icc",
+ .qinfo.size = sizeof(struct icc_bus),
+ .qinfo.props = (Property[]) {
+ DEFINE_PROP_END_OF_LIST(),
+ }
+
+};
+
+
+static const VMStateDescription vmstate_icc_bus = {
+ .name = "icc_bus",
+ .version_id = 1,
+ .minimum_version_id = 1,
+ .minimum_version_id_old = 1,
+ .pre_save = NULL,
+ .post_load = NULL,
+};
+
+struct icc_bus *g_iccbus;
+
+struct icc_bus *icc_init_bus(DeviceState *parent, const char *name)
+{
+ struct icc_bus *bus;
+
+ bus = FROM_QBUS(icc_bus, qbus_create(&icc_info.qinfo, parent,
>>> name));
+ bus->qbus.allow_hotplug = 1; /* Yes, we can */
+ bus->qbus.name = "icc";
+ vmstate_register(NULL, -1, &vmstate_icc_bus, bus);
>>>
>>> The chipset is the owner of this bus and instantiates it. So it also
>>> provides a vmstate. You can drop this unneeded one here (it's created
>>> via an obsolete API anyway).
>>>
>>
>> No familiar with Qemu bus emulation, keep on learning :) . But what I
>> thought is,
>> the x86-ICC bus is not the same as bus like PCI.
>> For a PCI bus, it lies behind a host bridge, but ICC is shared by all x86
>> processors in SMP system,
>> so there is not a outstanding owner. And I right?
>
> ICC is also attached to the chipset (due to the IOAPIC). So it looks
> reasonable to me to let the chipset do the lifecycle management as well.
> It is the fixed point, CPUs may come and go.
>
> Jan
>
>
[Qemu-devel] Info
Buongiorno sono Viviana Vorrei semplicemente invitarvi ad iscrivervi al nostro gruppo facebook per condividere pareri su musica e artisti, stringere nuove amicizie tramite la nostra Hyde Radio Il Gruppo e’ accessibile dal sito Hyde Radio . com Grazie per la cortese attenzione, un cordiale saluto Viviana ;-)
Re: [Qemu-devel] [PATCH] ppc64: Fix linker script
On 04.10.2011, at 17:35, Richard Henderson wrote: > On 10/04/2011 08:14 AM, Andreas Färber wrote: >> Since commit 8733f609 (Fix linker scripts) linking on Linux/ppc64 fails: > > It occurs to me to wonder if we ought to simply auto-detect the presence > of the -Ttext-segment ADDR option. If that's present, don't override the > linker script and all the changes that might have been introduced therein. I'm pretty sure that is unrelated to fixing the compile breakage :) Alex
Re: [Qemu-devel] [PATCH 17/23] migration: make sure we always have a migration state
Anthony Liguori wrote: > On 10/04/2011 09:49 AM, Juan Quintela wrote: >> Anthony Liguori wrote: >>> On 09/23/2011 07:57 AM, Juan Quintela wrote: [ much more stuff ] >> It avoids s==NULL checks, > > In favor of s->state == MIG_STATE_NONE. > >> and it also avoids having to have new >> variables (max_throotle) just because we don't have a migration state >> handy. > > The intention of the global variable is to set a default for new > sessions. I can imagine a world where if you had parallel migrations, > you could either toggle the default (the global variable) or the > specific parameter within a single migration session. > > But it's not about features. It's a general reluctances to heavily > modify the code to rely on a global instead of passing the state > around. Here's a pretty good short write up of why this is a Bad > Thing. > > http://stackoverflow.com/questions/1392315/problems-with-singleton-pattern > > Ultimately, MIG_STATE_NONE shouldn't be needed because we should not > be relying on a singleton accessor to get the MigrationState. A > better cleanup would be to further pass MigrationState between > functions and eliminate the need for a global at all. I understand the singleton problem, but the reason to put STATE_NONE is not for that O:-) (it just happens that we only have a migration now). Why I want it? Just now, the only thing that we are "setting" for a migration before it starts is the "bandwidth". I see the future when migration becomes something like: migration_set_speed migration_set_target migration_set_ migrate as you can see, we are "preparing" migration, and we don't have a "STATE" now that describes this state, migration not started, but we want to prepare it. Perhaps a better name that STATE_NONE is in order. I got NONE in the sense that "migration" has not been attemted yet. Later, Juan.
Re: [Qemu-devel] [PATCH 04/11] migration: return real error code
Anthony Liguori wrote: > On 10/04/2011 01:35 PM, Juan Quintela wrote: >> Anthony Liguori wrote: >>> On 09/23/2011 07:50 AM, Juan Quintela wrote: make functions propaget errno, instead of just using -EIO. Signed-off-by: Juan Quintela >>> >>> qemu_file_has_error() implies a boolean response. Wouldn't >>> qemu_file_get_error() make more sense if you're going to rely on the >>> return value? >> >> I just didn't want to change more things on the same patch. >> I can add a patch on top of this series? > > It's terribly odd to make a function that looks like a bool return a > non-boolean value. It can't be that much of a change to do it in this > patch, it's just a matter of running sed. That is already fixed. Problem was not doing that change, was to fix the rejects after that on the following patches. Later, Juan.
Re: [Qemu-devel] [PATCH 17/23] migration: make sure we always have a migration state
On 10/04/2011 09:49 AM, Juan Quintela wrote:
Anthony Liguori wrote:
On 09/23/2011 07:57 AM, Juan Quintela wrote:
This cleans up a lot the code as we don't have to check anymore if
the variable is NULL or not.
We don't make it static, because when we integrate fault tolerance, we
can have several migrations at once.
Signed-off-by: Juan Quintela
---
migration.c | 126 +-
1 files changed, 54 insertions(+), 72 deletions(-)
diff --git a/migration.c b/migration.c
index 1cc21f0..c382383 100644
--- a/migration.c
+++ b/migration.c
@@ -34,7 +34,13 @@
/* Migration speed throttling */
static int64_t max_throttle = (32<< 20);
-static MigrationState *current_migration;
+/* When we add fault tolerance, we could have several
+ migrations at once. For now we don't need to add
+ dynamic creation of migration */
+static MigrationState current_migration_static = {
+.state = MIG_STATE_NONE,
+};
+static MigrationState *current_migration =¤t_migration_static;
This doesn't make sense to me. We can have two migration states that
are both in the MIG_STATE_NONE? What does that actually mean?
I don't see the point in making migration state nullable via the state
member other than avoiding the if (s == NULL) check.
It avoids s==NULL checks,
In favor of s->state == MIG_STATE_NONE.
and it also avoids having to have new
variables (max_throotle) just because we don't have a migration state
handy.
The intention of the global variable is to set a default for new sessions. I
can imagine a world where if you had parallel migrations, you could either
toggle the default (the global variable) or the specific parameter within a
single migration session.
But it's not about features. It's a general reluctances to heavily modify the
code to rely on a global instead of passing the state around. Here's a pretty
good short write up of why this is a Bad Thing.
http://stackoverflow.com/questions/1392315/problems-with-singleton-pattern
Ultimately, MIG_STATE_NONE shouldn't be needed because we should not be relying
on a singleton accessor to get the MigrationState. A better cleanup would be to
further pass MigrationState between functions and eliminate the need for a
global at all.
Regards,
Anthony Liguori
Obviously the problem can't be the size (the variable is smaller that
all the code tests for NULL).
For me, it is easier to understand that we have an state (migration
never attempted) with the same states that the other migrations states.
Later, Juan.
Re: [Qemu-devel] [PATCH 04/11] migration: return real error code
On 10/04/2011 01:35 PM, Juan Quintela wrote: Anthony Liguori wrote: On 09/23/2011 07:50 AM, Juan Quintela wrote: make functions propaget errno, instead of just using -EIO. Signed-off-by: Juan Quintela qemu_file_has_error() implies a boolean response. Wouldn't qemu_file_get_error() make more sense if you're going to rely on the return value? I just didn't want to change more things on the same patch. I can add a patch on top of this series? It's terribly odd to make a function that looks like a bool return a non-boolean value. It can't be that much of a change to do it in this patch, it's just a matter of running sed. Regards, Anthony Liguori Later, Juan.
Re: [Qemu-devel] [PATCH 4/4] Revert "savevm: fix corruption in vmstate_subsection_load()."
On 10/04/2011 09:38 AM, Juan Quintela wrote:
This reverts commit eb60260de0b050a5e8ab725e84d377d0b44c43ae.
Conflicts:
savevm.c
We changed qemu_peek_byte() prototype, just fixed the rejects.
Signed-off-by: Juan Quintela
Reviewed-by: Anthony Liguori
Regards,
Anthony Liguori
---
savevm.c | 10 +-
1 files changed, 1 insertions(+), 9 deletions(-)
diff --git a/savevm.c b/savevm.c
index db6ea12..aaa303e 100644
--- a/savevm.c
+++ b/savevm.c
@@ -1715,12 +1715,6 @@ static const VMStateDescription
*vmstate_get_subsection(const VMStateSubsection
static int vmstate_subsection_load(QEMUFile *f, const VMStateDescription
*vmsd,
void *opaque)
{
-const VMStateSubsection *sub = vmsd->subsections;
-
-if (!sub || !sub->needed) {
-return 0;
-}
-
while (qemu_peek_byte(f, 0) == QEMU_VM_SUBSECTION) {
char idstr[256];
int ret;
@@ -1742,7 +1736,7 @@ static int vmstate_subsection_load(QEMUFile *f, const
VMStateDescription *vmsd,
/* it don't have a valid subsection name */
return 0;
}
-sub_vmsd = vmstate_get_subsection(sub, idstr);
+sub_vmsd = vmstate_get_subsection(vmsd->subsections, idstr);
if (sub_vmsd == NULL) {
return -ENOENT;
}
@@ -1752,7 +1746,6 @@ static int vmstate_subsection_load(QEMUFile *f, const
VMStateDescription *vmsd,
idstr[len] = 0;
version_id = qemu_get_be32(f);
-assert(!sub_vmsd->subsections);
ret = vmstate_load_state(f, sub_vmsd, opaque, version_id);
if (ret) {
return ret;
@@ -1776,7 +1769,6 @@ static void vmstate_subsection_save(QEMUFile *f, const
VMStateDescription *vmsd,
qemu_put_byte(f, len);
qemu_put_buffer(f, (uint8_t *)vmsd->name, len);
qemu_put_be32(f, vmsd->version_id);
-assert(!vmsd->subsections);
vmstate_save_state(f, vmsd, opaque);
}
sub++;
Re: [Qemu-devel] [PATCH 3/4] savevm: improve subsections detection on load
On 10/04/2011 09:38 AM, Juan Quintela wrote:
We add qemu_peek_buffer, that is identical to qemu_get_buffer, just
that it don't update f->buf_index.
We add a paramenter to qemu_peek_byte() to be able to peek more than
one byte.
Once this is done, to see if we have a subsection we look:
- 1st byte is QEMU_VM_SUBSECTION
- 2nd byte is a length, and is bigger than section name
- 3rd element is a string that starts with section_name
So, we shouldn't have false positives (yes, content could still get us
wrong but probabilities are really low).
Signed-off-by: Juan Quintela
---
savevm.c | 71 -
1 files changed, 60 insertions(+), 11 deletions(-)
diff --git a/savevm.c b/savevm.c
index 5fee4e2..db6ea12 100644
--- a/savevm.c
+++ b/savevm.c
@@ -532,6 +532,37 @@ void qemu_put_byte(QEMUFile *f, int v)
qemu_fflush(f);
}
+static int qemu_peek_buffer(QEMUFile *f, uint8_t *buf, int size1, int offset)
+{
+int size, l;
+int index = f->buf_index + offset;
+
+if (f->is_write) {
+abort();
+}
+
+size = size1;
+while (size> 0) {
+l = f->buf_size - index;
+if (l == 0) {
+qemu_fill_buffer(f);
+index = f->buf_index + offset;
+l = f->buf_size - index;
+if (l == 0) {
+break;
+}
+}
+if (l> size) {
+l = size;
+}
+memcpy(buf, f->buf + index, l);
+index += l;
+buf += l;
+size -= l;
+}
+return size1 - size;
+}
+
int qemu_get_buffer(QEMUFile *f, uint8_t *buf, int size1)
{
int size, l;
Can we implement get_buffer in terms of peek_buffer and just increment
f->buf_index in get_buffer?
@@ -561,19 +592,22 @@ int qemu_get_buffer(QEMUFile *f, uint8_t *buf, int size1)
return size1 - size;
}
-static int qemu_peek_byte(QEMUFile *f)
+static int qemu_peek_byte(QEMUFile *f, int offset)
{
+int index = f->buf_index + offset;
+
if (f->is_write) {
abort();
}
-if (f->buf_index>= f->buf_size) {
+if (index>= f->buf_size) {
qemu_fill_buffer(f);
-if (f->buf_index>= f->buf_size) {
+index = f->buf_index + offset;
+if (index>= f->buf_size) {
return 0;
}
}
-return f->buf[f->buf_index];
+return f->buf[index];
}
int qemu_get_byte(QEMUFile *f)
@@ -1687,22 +1721,37 @@ static int vmstate_subsection_load(QEMUFile *f, const
VMStateDescription *vmsd,
return 0;
}
-while (qemu_peek_byte(f) == QEMU_VM_SUBSECTION) {
+while (qemu_peek_byte(f, 0) == QEMU_VM_SUBSECTION) {
char idstr[256];
int ret;
-uint8_t version_id, len;
+uint8_t version_id, len, size;
const VMStateDescription *sub_vmsd;
-qemu_get_byte(f); /* subsection */
-len = qemu_get_byte(f);
-qemu_get_buffer(f, (uint8_t *)idstr, len);
-idstr[len] = 0;
-version_id = qemu_get_be32(f);
+len = qemu_peek_byte(f, 1);
+if (len< strlen(vmsd->name) + 1) {
+/* subsection name has be be "section_name/a" */
+return 0;
+}
+size = qemu_peek_buffer(f, (uint8_t *)idstr, len, 2);
+if (size != len) {
+return 0;
+}
+idstr[size] = 0;
Regards,
Anthony Liguori
Re: [Qemu-devel] [PATCH 2/4] savevm: some coding style cleanups
On 10/04/2011 09:38 AM, Juan Quintela wrote:
This patch will make moving code on next patches and having checkpatch
happy easier.
Signed-off-by: Juan Quintela
Reviewed-by: Anthony Liguori
Regards,
Anthony Liguori
---
savevm.c | 21 ++---
1 files changed, 14 insertions(+), 7 deletions(-)
diff --git a/savevm.c b/savevm.c
index 31131df..5fee4e2 100644
--- a/savevm.c
+++ b/savevm.c
@@ -536,8 +536,9 @@ int qemu_get_buffer(QEMUFile *f, uint8_t *buf, int size1)
{
int size, l;
-if (f->is_write)
+if (f->is_write) {
abort();
+}
size = size1;
while (size> 0) {
@@ -545,11 +546,13 @@ int qemu_get_buffer(QEMUFile *f, uint8_t *buf, int size1)
if (l == 0) {
qemu_fill_buffer(f);
l = f->buf_size - f->buf_index;
-if (l == 0)
+if (l == 0) {
break;
+}
}
-if (l> size)
+if (l> size) {
l = size;
+}
memcpy(buf, f->buf + f->buf_index, l);
f->buf_index += l;
buf += l;
@@ -560,26 +563,30 @@ int qemu_get_buffer(QEMUFile *f, uint8_t *buf, int size1)
static int qemu_peek_byte(QEMUFile *f)
{
-if (f->is_write)
+if (f->is_write) {
abort();
+}
if (f->buf_index>= f->buf_size) {
qemu_fill_buffer(f);
-if (f->buf_index>= f->buf_size)
+if (f->buf_index>= f->buf_size) {
return 0;
+}
}
return f->buf[f->buf_index];
}
int qemu_get_byte(QEMUFile *f)
{
-if (f->is_write)
+if (f->is_write) {
abort();
+}
if (f->buf_index>= f->buf_size) {
qemu_fill_buffer(f);
-if (f->buf_index>= f->buf_size)
+if (f->buf_index>= f->buf_size) {
return 0;
+}
}
return f->buf[f->buf_index++];
}
Re: [Qemu-devel] [PATCH 1/4] savevm: teach qemu_fill_buffer to do partial refills
On 10/04/2011 09:38 AM, Juan Quintela wrote:
We will need on next patch to be able to lookahead on next patch
Signed-off-by: Juan Quintela
Reviewed-by: Anthony Liguori
Regards,
Anthony Liguori
---
savevm.c | 14 +++---
1 files changed, 11 insertions(+), 3 deletions(-)
diff --git a/savevm.c b/savevm.c
index 46f2447..31131df 100644
--- a/savevm.c
+++ b/savevm.c
@@ -455,6 +455,7 @@ void qemu_fflush(QEMUFile *f)
static void qemu_fill_buffer(QEMUFile *f)
{
int len;
+int used;
if (!f->get_buffer)
return;
@@ -462,10 +463,17 @@ static void qemu_fill_buffer(QEMUFile *f)
if (f->is_write)
abort();
-len = f->get_buffer(f->opaque, f->buf, f->buf_offset, IO_BUF_SIZE);
+used = f->buf_size - f->buf_index;
+if (used> 0) {
+memmove(f->buf, f->buf + f->buf_index, used);
+}
+f->buf_index = 0;
+f->buf_size = used;
+
+len = f->get_buffer(f->opaque, f->buf + used, f->buf_offset,
+IO_BUF_SIZE - used);
if (len> 0) {
-f->buf_index = 0;
-f->buf_size = len;
+f->buf_size += len;
f->buf_offset += len;
} else if (len != -EAGAIN)
f->has_error = 1;
Re: [Qemu-devel] [PATCH] runstate: do not discard runstate changes when paused
On Wed, 5 Oct 2011 15:49:42 -0300 Luiz Capitulino wrote: > On Wed, 05 Oct 2011 20:02:04 +0200 > Avi Kivity wrote: > > > On 10/05/2011 07:39 PM, Luiz Capitulino wrote: > > > On Wed, 05 Oct 2011 19:23:21 +0200 > > > Avi Kivity wrote: > > > > > > > On 10/05/2011 07:02 PM, Luiz Capitulino wrote: > > > > > On Wed, 05 Oct 2011 18:37:51 +0200 > > > > > Avi Kivity wrote: > > > > > > > > > > > On 10/05/2011 06:31 PM, Jan Kiszka wrote: > > > > > > > >> > > > > > > > > > > > > > > > >vm_start() should be symmetric with vm_stop(). That > > > > is, if a piece of > > > > > > > >code wants to execute with vcpus stopped, it should > > > > just run inside a > > > > > > > >stop/start pair. > > > > > > > > > > > > > > > >The only confusion can come from the user, if he sees > > > > multiple stop > > > > > > > >events and expects that just one cont will continue > > > > the vm. For the > > > > > > > >machine monitor, we should just document that the you > > > > have to issue one > > > > > > > >cont for every stop event you see (plus any stops you > > > > issue). It's not > > > > > > > >unnatural - the code that handles a > > > > stop_due_to_enospace can work to fix > > > > > > > >the error and issue a cont, disregarding any other > > > > stops in progress > > > > > > > >(due to a user pressing the stop button, or migration, > > > > or cpu hotplug, > > > > > > > >or whatever). For the human monitor, it's not so > > > > intuitive, but the > > > > > > > >situation is so rare we can just rely on the user to > > > > issue cont again. > > > > > > > > > > > > > > Making this kind of user-visible change would be a bad idea. > > > > > > > > > > > > The current situation is a bad idea. > > > > > > > > > > Let's take the migration use-case as an example (ie. the user stops > > > > the VM > > > > > before performing the migration). Today, if migration fails, > > > > > migrate_fd_put_ready() will call vm_start() which will put the VM to > > > > > run again. > > > > > > > > > > But if we implement the ref count idea, then vm_start() will just > > > > "unlock" > > > > > migrate_fd_put_ready()'s own call to vm_stop(), that's, the user > > > > stop will > > > > > remain and the user is required to do a 'cont'. > > > > > > > > > > I'd probably agree that that's the ideal semantics, but chances are > > > > we're > > > > > going to break qmp clients here. > > > > > > > > There are two questions here. Is this autostart desirable? (IMO no, > > > > but haven't given it much thought). > > > > > > It should be configurable at migration time I think. > > > > Why? autostart can easily be emulated by non-autostart, but not vice versa. > > What I meant is to give the user the choice to use. > > > > > If yes, we should provide it > > > > somehow. If not, we should default to providing it, but switch to > > > > non-autostart if a newer client indicates it understands the new > > > > semantics. > > > > > > That's only one example. You mention another one above: if qemu stops > > > itself twice, will the user be required to run 'cont' twice? > > > > Yes. That's how the user tells qemu it saw the two events and handled > > both of them. Otherwise there is a race condition and an event goes > > unhandled (or rather, it is handled while the guest is running instead > > of stopped). > > Breaks compatibility. > > > > I'm not exactly against the semantics you're proposing, but they don't > > > seem to fit today's qemu. > > > > Today's qemu is broken here. > > For me it's broken because it will abort() if you migrate a paused vm, for > you it seems to be broken at the semantic level. > > We can fix the semantics without breaking compatibility. s/We can/ We can't
Re: [Qemu-devel] [PATCH] runstate: do not discard runstate changes when paused
On Wed, 05 Oct 2011 20:02:04 +0200 Avi Kivity wrote: > On 10/05/2011 07:39 PM, Luiz Capitulino wrote: > > On Wed, 05 Oct 2011 19:23:21 +0200 > > Avi Kivity wrote: > > > > > On 10/05/2011 07:02 PM, Luiz Capitulino wrote: > > > > On Wed, 05 Oct 2011 18:37:51 +0200 > > > > Avi Kivity wrote: > > > > > > > > > On 10/05/2011 06:31 PM, Jan Kiszka wrote: > > > > > > >> > > > > > > > > > > > > > >vm_start() should be symmetric with vm_stop(). That is, > > > if a piece of > > > > > > >code wants to execute with vcpus stopped, it should just > > > run inside a > > > > > > >stop/start pair. > > > > > > > > > > > > > >The only confusion can come from the user, if he sees > > > multiple stop > > > > > > >events and expects that just one cont will continue the > > > vm. For the > > > > > > >machine monitor, we should just document that the you > > > have to issue one > > > > > > >cont for every stop event you see (plus any stops you > > > issue). It's not > > > > > > >unnatural - the code that handles a stop_due_to_enospace > > > can work to fix > > > > > > >the error and issue a cont, disregarding any other stops > > > in progress > > > > > > >(due to a user pressing the stop button, or migration, > > > or cpu hotplug, > > > > > > >or whatever). For the human monitor, it's not so > > > intuitive, but the > > > > > > >situation is so rare we can just rely on the user to > > > issue cont again. > > > > > > > > > > > > Making this kind of user-visible change would be a bad idea. > > > > > > > > > > The current situation is a bad idea. > > > > > > > > Let's take the migration use-case as an example (ie. the user stops > > > the VM > > > > before performing the migration). Today, if migration fails, > > > > migrate_fd_put_ready() will call vm_start() which will put the VM to > > > > run again. > > > > > > > > But if we implement the ref count idea, then vm_start() will just > > > "unlock" > > > > migrate_fd_put_ready()'s own call to vm_stop(), that's, the user stop > > > will > > > > remain and the user is required to do a 'cont'. > > > > > > > > I'd probably agree that that's the ideal semantics, but chances are > > > we're > > > > going to break qmp clients here. > > > > > > There are two questions here. Is this autostart desirable? (IMO no, > > > but haven't given it much thought). > > > > It should be configurable at migration time I think. > > Why? autostart can easily be emulated by non-autostart, but not vice versa. What I meant is to give the user the choice to use. > > > If yes, we should provide it > > > somehow. If not, we should default to providing it, but switch to > > > non-autostart if a newer client indicates it understands the new > > > semantics. > > > > That's only one example. You mention another one above: if qemu stops > > itself twice, will the user be required to run 'cont' twice? > > Yes. That's how the user tells qemu it saw the two events and handled > both of them. Otherwise there is a race condition and an event goes > unhandled (or rather, it is handled while the guest is running instead > of stopped). Breaks compatibility. > > I'm not exactly against the semantics you're proposing, but they don't > > seem to fit today's qemu. > > Today's qemu is broken here. For me it's broken because it will abort() if you migrate a paused vm, for you it seems to be broken at the semantic level. We can fix the semantics without breaking compatibility.
Re: [Qemu-devel] [PATCH] runstate: do not discard runstate changes when paused
On 2011-10-05 19:12, Avi Kivity wrote: > On 10/05/2011 06:49 PM, Jan Kiszka wrote: >> On 2011-10-05 18:37, Avi Kivity wrote: >> > On 10/05/2011 06:31 PM, Jan Kiszka wrote: >> >> >> >> >> > >> >> > vm_start() should be symmetric with vm_stop(). That is, if a >> piece of >> >> > code wants to execute with vcpus stopped, it should just run >> inside a >> >> > stop/start pair. >> >> > >> >> > The only confusion can come from the user, if he sees multiple >> stop >> >> > events and expects that just one cont will continue the vm. >> For the >> >> > machine monitor, we should just document that the you have to >> issue >> >> one >> >> > cont for every stop event you see (plus any stops you issue). >> It's >> >> not >> >> > unnatural - the code that handles a stop_due_to_enospace can work >> >> to fix >> >> > the error and issue a cont, disregarding any other stops in >> progress >> >> > (due to a user pressing the stop button, or migration, or cpu >> hotplug, >> >> > or whatever). For the human monitor, it's not so intuitive, >> but the >> >> > situation is so rare we can just rely on the user to issue >> cont again. >> >> >> >> Making this kind of user-visible change would be a bad idea. >> > >> > The current situation is a bad idea. >> > >> > Consider a user-initiated or qemu-initiated stop; the user starts to >> > deal with it, types 'cont', and as the Enter key is being depressed >> > another qemu-initiated stop comes along. The 'cont' restarts the >> guest >> > even though the second event was not dealt with. >> >> You always have this kind of problems when you attach two keyboards to >> the same console. A counting stop/cont will just create different >> effects of the same problem but not solve it. > > Let's examine a concrete example: a user is debugging a guest, which > stops at a breakpoint. Meanwhile a live migration is going on, > involving internal stops. When the guest does manage to run for a bit, > it runs out of disk space, generating a stop, which the management agent > resolves by allocating more space and issuing a cont. > > With a counting cont, no matter in what order these events happen, > things work out fine. How do they work out with your proposal? We can enforce stop for temporal reasons (migration/savevm), something that overrules user/management initiated stops. BTW, does stop due to migration actually have a window where it accepts other commands? I thought that phase is synchronous. Then we would just have to implement proper state saving/restoring. Anyway, there is no point in lock counting for stop reasons that require external synchronization anyway. gdb vs. management stack vs. human monitor - nothing is solved by counting the stops, they all can step on each other's shoes. Even worse, exposing a counting stop via the user interface requires additional interfaces to recover lost or forgotten locks. We've discussed this in the past IIRC. Jan signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH] runstate: do not discard runstate changes when paused
On 10/05/2011 07:39 PM, Luiz Capitulino wrote: On Wed, 05 Oct 2011 19:23:21 +0200 Avi Kivity wrote: > On 10/05/2011 07:02 PM, Luiz Capitulino wrote: > > On Wed, 05 Oct 2011 18:37:51 +0200 > > Avi Kivity wrote: > > > > > On 10/05/2011 06:31 PM, Jan Kiszka wrote: > > > > >> > > > > > > > > > >vm_start() should be symmetric with vm_stop(). That is, if a piece of > > > > >code wants to execute with vcpus stopped, it should just run inside a > > > > >stop/start pair. > > > > > > > > > >The only confusion can come from the user, if he sees multiple stop > > > > >events and expects that just one cont will continue the vm. For the > > > > >machine monitor, we should just document that the you have to issue one > > > > >cont for every stop event you see (plus any stops you issue). It's not > > > > >unnatural - the code that handles a stop_due_to_enospace can work to fix > > > > >the error and issue a cont, disregarding any other stops in progress > > > > >(due to a user pressing the stop button, or migration, or cpu hotplug, > > > > >or whatever). For the human monitor, it's not so intuitive, but the > > > > >situation is so rare we can just rely on the user to issue cont again. > > > > > > > > Making this kind of user-visible change would be a bad idea. > > > > > > The current situation is a bad idea. > > > > Let's take the migration use-case as an example (ie. the user stops the VM > > before performing the migration). Today, if migration fails, > > migrate_fd_put_ready() will call vm_start() which will put the VM to > > run again. > > > > But if we implement the ref count idea, then vm_start() will just "unlock" > > migrate_fd_put_ready()'s own call to vm_stop(), that's, the user stop will > > remain and the user is required to do a 'cont'. > > > > I'd probably agree that that's the ideal semantics, but chances are we're > > going to break qmp clients here. > > There are two questions here. Is this autostart desirable? (IMO no, > but haven't given it much thought). It should be configurable at migration time I think. Why? autostart can easily be emulated by non-autostart, but not vice versa. > If yes, we should provide it > somehow. If not, we should default to providing it, but switch to > non-autostart if a newer client indicates it understands the new semantics. That's only one example. You mention another one above: if qemu stops itself twice, will the user be required to run 'cont' twice? Yes. That's how the user tells qemu it saw the two events and handled both of them. Otherwise there is a race condition and an event goes unhandled (or rather, it is handled while the guest is running instead of stopped). I'm not exactly against the semantics you're proposing, but they don't seem to fit today's qemu. Today's qemu is broken here. -- error compiling committee.c: too many arguments to function
Re: [Qemu-devel] [PATCH] runstate: do not discard runstate changes when paused
On Wed, 05 Oct 2011 19:23:21 +0200 Avi Kivity wrote: > On 10/05/2011 07:02 PM, Luiz Capitulino wrote: > > On Wed, 05 Oct 2011 18:37:51 +0200 > > Avi Kivity wrote: > > > > > On 10/05/2011 06:31 PM, Jan Kiszka wrote: > > > > >> > > > > > > > > > > vm_start() should be symmetric with vm_stop(). That is, if a > > > piece of > > > > > code wants to execute with vcpus stopped, it should just run > > > inside a > > > > > stop/start pair. > > > > > > > > > > The only confusion can come from the user, if he sees multiple > > > stop > > > > > events and expects that just one cont will continue the vm. For > > > the > > > > > machine monitor, we should just document that the you have to > > > issue one > > > > > cont for every stop event you see (plus any stops you issue). > > > It's not > > > > > unnatural - the code that handles a stop_due_to_enospace can work > > > to fix > > > > > the error and issue a cont, disregarding any other stops in > > > progress > > > > > (due to a user pressing the stop button, or migration, or cpu > > > hotplug, > > > > > or whatever). For the human monitor, it's not so intuitive, but > > > the > > > > > situation is so rare we can just rely on the user to issue cont > > > again. > > > > > > > > Making this kind of user-visible change would be a bad idea. > > > > > > The current situation is a bad idea. > > > > Let's take the migration use-case as an example (ie. the user stops the VM > > before performing the migration). Today, if migration fails, > > migrate_fd_put_ready() will call vm_start() which will put the VM to > > run again. > > > > But if we implement the ref count idea, then vm_start() will just "unlock" > > migrate_fd_put_ready()'s own call to vm_stop(), that's, the user stop will > > remain and the user is required to do a 'cont'. > > > > I'd probably agree that that's the ideal semantics, but chances are we're > > going to break qmp clients here. > > There are two questions here. Is this autostart desirable? (IMO no, > but haven't given it much thought). It should be configurable at migration time I think. > If yes, we should provide it > somehow. If not, we should default to providing it, but switch to > non-autostart if a newer client indicates it understands the new semantics. That's only one example. You mention another one above: if qemu stops itself twice, will the user be required to run 'cont' twice? I'm not exactly against the semantics you're proposing, but they don't seem to fit today's qemu.
Re: [Qemu-devel] [PATCH] runstate: do not discard runstate changes when paused
On 10/05/2011 07:02 PM, Luiz Capitulino wrote: On Wed, 05 Oct 2011 18:37:51 +0200 Avi Kivity wrote: > On 10/05/2011 06:31 PM, Jan Kiszka wrote: > > >> > > > > > > vm_start() should be symmetric with vm_stop(). That is, if a piece of > > > code wants to execute with vcpus stopped, it should just run inside a > > > stop/start pair. > > > > > > The only confusion can come from the user, if he sees multiple stop > > > events and expects that just one cont will continue the vm. For the > > > machine monitor, we should just document that the you have to issue one > > > cont for every stop event you see (plus any stops you issue). It's not > > > unnatural - the code that handles a stop_due_to_enospace can work to fix > > > the error and issue a cont, disregarding any other stops in progress > > > (due to a user pressing the stop button, or migration, or cpu hotplug, > > > or whatever). For the human monitor, it's not so intuitive, but the > > > situation is so rare we can just rely on the user to issue cont again. > > > > Making this kind of user-visible change would be a bad idea. > > The current situation is a bad idea. Let's take the migration use-case as an example (ie. the user stops the VM before performing the migration). Today, if migration fails, migrate_fd_put_ready() will call vm_start() which will put the VM to run again. But if we implement the ref count idea, then vm_start() will just "unlock" migrate_fd_put_ready()'s own call to vm_stop(), that's, the user stop will remain and the user is required to do a 'cont'. I'd probably agree that that's the ideal semantics, but chances are we're going to break qmp clients here. There are two questions here. Is this autostart desirable? (IMO no, but haven't given it much thought). If yes, we should provide it somehow. If not, we should default to providing it, but switch to non-autostart if a newer client indicates it understands the new semantics. -- error compiling committee.c: too many arguments to function
Re: [Qemu-devel] [PATCH] KVM: emulate lapic tsc deadline timer for guest
Marcelo Tosatti wrote: > On Sun, Sep 25, 2011 at 10:47:46PM +0800, Liu, Jinsong wrote: >> Marcelo Tosatti wrote: >>> On Fri, Sep 23, 2011 at 04:25:51PM +0800, Liu, Jinsong wrote: Marcelo Tosatti wrote: > On Thu, Sep 22, 2011 at 04:55:52PM +0800, Liu, Jinsong wrote: >>> From 4d5b83aba40ce0d421add9a41a6c591a8590a32e Mon Sep 17 >>> 00:00:00 2001 >> From: Liu, Jinsong >> Date: Thu, 22 Sep 2011 14:00:08 +0800 >> Subject: [PATCH 2/2] KVM: emulate lapic tsc deadline timer for >> guest >> >> This patch emulate lapic tsc deadline timer for guest: >> Enumerate tsc deadline timer capability by CPUID; >> Enable tsc deadline timer mode by lapic MMIO; >> Start tsc deadline timer by WRMSR; >> >> Signed-off-by: Liu, Jinsong --- >> arch/x86/include/asm/kvm_host.h |2 + >> arch/x86/kvm/kvm_timer.h|2 + >> arch/x86/kvm/lapic.c| 123 >> --- arch/x86/kvm/lapic.h >> |3 + arch/x86/kvm/x86.c | 16 +- >> 5 files changed, 123 insertions(+), 23 deletions(-) > > Looks good, please rebase against branch master of > > git://github.com/avikivity/kvm.git Rebased as attached. Thanks, Jinsong >>> >>> Please write a simple test case to arm a lapic timer via wrmsr (see >>> https://github.com/avikivity/kvm-unit-tests). >>> >>> Kernel patches have been applied, thanks. >> >> Marcelo, >> >> I'm not quite clear the purpose and usage of test case of the >> kvm-unit-tests. Can you give me some hint? > > The purpose is to add unit tests for new features (such as lapic > deadline timer). There are examples that make it relatively easy to > construct new test case (or modify existing ones to accomodate new > tests). > > Please add a new test case for lapic deadline timer, thanks. Thanks Marcelo. I will add the test case. Sorry for slow email reply because of holiday. Regards, Jinsong
Re: [Qemu-devel] [PATCH] runstate: do not discard runstate changes when paused
On 10/05/2011 06:49 PM, Jan Kiszka wrote: On 2011-10-05 18:37, Avi Kivity wrote: > On 10/05/2011 06:31 PM, Jan Kiszka wrote: >> >> >> > >> > vm_start() should be symmetric with vm_stop(). That is, if a piece of >> > code wants to execute with vcpus stopped, it should just run inside a >> > stop/start pair. >> > >> > The only confusion can come from the user, if he sees multiple stop >> > events and expects that just one cont will continue the vm. For the >> > machine monitor, we should just document that the you have to issue >> one >> > cont for every stop event you see (plus any stops you issue). It's >> not >> > unnatural - the code that handles a stop_due_to_enospace can work >> to fix >> > the error and issue a cont, disregarding any other stops in progress >> > (due to a user pressing the stop button, or migration, or cpu hotplug, >> > or whatever). For the human monitor, it's not so intuitive, but the >> > situation is so rare we can just rely on the user to issue cont again. >> >> Making this kind of user-visible change would be a bad idea. > > The current situation is a bad idea. > > Consider a user-initiated or qemu-initiated stop; the user starts to > deal with it, types 'cont', and as the Enter key is being depressed > another qemu-initiated stop comes along. The 'cont' restarts the guest > even though the second event was not dealt with. You always have this kind of problems when you attach two keyboards to the same console. A counting stop/cont will just create different effects of the same problem but not solve it. Let's examine a concrete example: a user is debugging a guest, which stops at a breakpoint. Meanwhile a live migration is going on, involving internal stops. When the guest does manage to run for a bit, it runs out of disk space, generating a stop, which the management agent resolves by allocating more space and issuing a cont. With a counting cont, no matter in what order these events happen, things work out fine. How do they work out with your proposal? > >> We are talking about multiple stop states here, but only a single >> function (vm_stop) to enter them - maybe that's not optimal. But the >> point is that we were missing one stop-to-stop transition. And that >> needs to be fixed, either inside vm_stop or when it is called. > > Those stops are orthogonal. There is no relationship between a > migration stop, a user stop, an ENOSPC stop, a hotplug stop, and a > debugger stop. There is no reason to start inventing stop-to-stop > transitions between them. A 'cont' intended for one should not undo > another. No, they aren't. They are all different states, and we need to model transitions between them. If there are redundant states, lets collapse them. What's the state caused by 'stopped-due-to-migration-phase-3' and 'stopped-due-to-breakpoint'? 'stopped-due-to-migration-phase-3-and-breakpoint'? >> > > Which stop states would these be? When would you want one cont to undo > two stops? No, cont would remain cont: the resume-after-stop command. Locking would never be user-exposed. It would be a QEMU-internal thing, used when there must be no resume for a certain finite while (e.g. during migration or savevm). I think one problem with the current state machine is that it does not differentiate between these two types of "stop". My second proposal (a set of stop reasons) differentiates between all kinds of cont. Maybe we should extend the monitor command to cont [reason]. -- error compiling committee.c: too many arguments to function
Re: [Qemu-devel] [PATCH] runstate: do not discard runstate changes when paused
On Wed, 05 Oct 2011 18:37:51 +0200 Avi Kivity wrote: > On 10/05/2011 06:31 PM, Jan Kiszka wrote: > > >> > > > > > > vm_start() should be symmetric with vm_stop(). That is, if a piece of > > > code wants to execute with vcpus stopped, it should just run inside a > > > stop/start pair. > > > > > > The only confusion can come from the user, if he sees multiple stop > > > events and expects that just one cont will continue the vm. For the > > > machine monitor, we should just document that the you have to issue one > > > cont for every stop event you see (plus any stops you issue). It's not > > > unnatural - the code that handles a stop_due_to_enospace can work to fix > > > the error and issue a cont, disregarding any other stops in progress > > > (due to a user pressing the stop button, or migration, or cpu hotplug, > > > or whatever). For the human monitor, it's not so intuitive, but the > > > situation is so rare we can just rely on the user to issue cont again. > > > > Making this kind of user-visible change would be a bad idea. > > The current situation is a bad idea. Let's take the migration use-case as an example (ie. the user stops the VM before performing the migration). Today, if migration fails, migrate_fd_put_ready() will call vm_start() which will put the VM to run again. But if we implement the ref count idea, then vm_start() will just "unlock" migrate_fd_put_ready()'s own call to vm_stop(), that's, the user stop will remain and the user is required to do a 'cont'. I'd probably agree that that's the ideal semantics, but chances are we're going to break qmp clients here.
Re: [Qemu-devel] [PATCH] runstate: do not discard runstate changes when paused
On 2011-10-05 18:37, Avi Kivity wrote: > On 10/05/2011 06:31 PM, Jan Kiszka wrote: >> >> >> > >> > vm_start() should be symmetric with vm_stop(). That is, if a piece of >> > code wants to execute with vcpus stopped, it should just run inside a >> > stop/start pair. >> > >> > The only confusion can come from the user, if he sees multiple stop >> > events and expects that just one cont will continue the vm. For the >> > machine monitor, we should just document that the you have to issue >> one >> > cont for every stop event you see (plus any stops you issue). It's >> not >> > unnatural - the code that handles a stop_due_to_enospace can work >> to fix >> > the error and issue a cont, disregarding any other stops in progress >> > (due to a user pressing the stop button, or migration, or cpu hotplug, >> > or whatever). For the human monitor, it's not so intuitive, but the >> > situation is so rare we can just rely on the user to issue cont again. >> >> Making this kind of user-visible change would be a bad idea. > > The current situation is a bad idea. > > Consider a user-initiated or qemu-initiated stop; the user starts to > deal with it, types 'cont', and as the Enter key is being depressed > another qemu-initiated stop comes along. The 'cont' restarts the guest > even though the second event was not dealt with. You always have this kind of problems when you attach two keyboards to the same console. A counting stop/cont will just create different effects of the same problem but not solve it. > >> We are talking about multiple stop states here, but only a single >> function (vm_stop) to enter them - maybe that's not optimal. But the >> point is that we were missing one stop-to-stop transition. And that >> needs to be fixed, either inside vm_stop or when it is called. > > Those stops are orthogonal. There is no relationship between a > migration stop, a user stop, an ENOSPC stop, a hotplug stop, and a > debugger stop. There is no reason to start inventing stop-to-stop > transitions between them. A 'cont' intended for one should not undo > another. No, they aren't. They are all different states, and we need to model transitions between them. If there are redundant states, lets collapse them. > > There are two ways to do this, one is to store a set of stop reasons and > let both 'stop' and 'cont' specify the reason, the other, which is > simpler but less safe, is to use a reference counting approach. > >> >> If you want to lock the VM into paused state, add a new symmetric API >> that does precisely this. That API would send the VM into RSTATE_LOCKED >> if it is not yet stopped on lock or is still locked on resume. That >> would avoid redefining stop states that have no use for lock-counting >> semantics. >> > > Which stop states would these be? When would you want one cont to undo > two stops? No, cont would remain cont: the resume-after-stop command. Locking would never be user-exposed. It would be a QEMU-internal thing, used when there must be no resume for a certain finite while (e.g. during migration or savevm). I think one problem with the current state machine is that it does not differentiate between these two types of "stop". Jan signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH] runstate: do not discard runstate changes when paused
On 10/05/2011 06:31 PM, Jan Kiszka wrote: >> > > vm_start() should be symmetric with vm_stop(). That is, if a piece of > code wants to execute with vcpus stopped, it should just run inside a > stop/start pair. > > The only confusion can come from the user, if he sees multiple stop > events and expects that just one cont will continue the vm. For the > machine monitor, we should just document that the you have to issue one > cont for every stop event you see (plus any stops you issue). It's not > unnatural - the code that handles a stop_due_to_enospace can work to fix > the error and issue a cont, disregarding any other stops in progress > (due to a user pressing the stop button, or migration, or cpu hotplug, > or whatever). For the human monitor, it's not so intuitive, but the > situation is so rare we can just rely on the user to issue cont again. Making this kind of user-visible change would be a bad idea. The current situation is a bad idea. Consider a user-initiated or qemu-initiated stop; the user starts to deal with it, types 'cont', and as the Enter key is being depressed another qemu-initiated stop comes along. The 'cont' restarts the guest even though the second event was not dealt with. We are talking about multiple stop states here, but only a single function (vm_stop) to enter them - maybe that's not optimal. But the point is that we were missing one stop-to-stop transition. And that needs to be fixed, either inside vm_stop or when it is called. Those stops are orthogonal. There is no relationship between a migration stop, a user stop, an ENOSPC stop, a hotplug stop, and a debugger stop. There is no reason to start inventing stop-to-stop transitions between them. A 'cont' intended for one should not undo another. There are two ways to do this, one is to store a set of stop reasons and let both 'stop' and 'cont' specify the reason, the other, which is simpler but less safe, is to use a reference counting approach. If you want to lock the VM into paused state, add a new symmetric API that does precisely this. That API would send the VM into RSTATE_LOCKED if it is not yet stopped on lock or is still locked on resume. That would avoid redefining stop states that have no use for lock-counting semantics. Which stop states would these be? When would you want one cont to undo two stops? -- error compiling committee.c: too many arguments to function
Re: [Qemu-devel] [PATCH] runstate: do not discard runstate changes when paused
On 2011-10-05 17:43, Avi Kivity wrote: > On 10/05/2011 04:37 PM, Luiz Capitulino wrote: >> Now, we have three options to fix this but I don't know which one to >> choose: >> >> 1. We could just add the transition RSTATE_PAUSED -> >> RSTATE_POST_MIGRATE >> as valid. Not sure this is a good thing to do though, as it seems >> a silly >> workaround for the fact that the transition to RSTATE_PRE_MIGRATE >> has >> never occurred >> >> 2. This patch makes vm_stop() do the state transition even if the VM >> is already stopped. Seems good enough, except that I fear two >> things. >> First, today we know that vm_stop() is a no-op if the VM is already >> stopped, so there's a semantic change that could turn out to be >> trap. >> Second, I also fear people using vm_stop() as a way to change the >> VM status, just like runstate_set() (which can also become an >> horrible >> trap) >> >> 3. Avi suggested we should keep a reference count, so that states are >> not discarded: >> >> http://lists.gnu.org/archive/html/qemu-devel/2011-08/msg00595.html >> >> That solution seemed to be the perfect one, except for one important >> detail: how should we implement vm_start() (and thus 'cont')? >> >> In order to maintain how we behave with the external world, the only >> option is that vm_start() will set the stop count to 0. Ie, doesn't >> matter if we have stopped the VM 500 times at some point, a >> vm_start() >> call will discard all stored states. >> >> Not sure if that's what you expected, but the first time I read >> Avi's >> idea I had the impression that it would be a good idea that >> vm_start() >> decremented the ref count only once, ie. vm_stop() and vm_start() >> calls >> have to match. >> > > vm_start() should be symmetric with vm_stop(). That is, if a piece of > code wants to execute with vcpus stopped, it should just run inside a > stop/start pair. > > The only confusion can come from the user, if he sees multiple stop > events and expects that just one cont will continue the vm. For the > machine monitor, we should just document that the you have to issue one > cont for every stop event you see (plus any stops you issue). It's not > unnatural - the code that handles a stop_due_to_enospace can work to fix > the error and issue a cont, disregarding any other stops in progress > (due to a user pressing the stop button, or migration, or cpu hotplug, > or whatever). For the human monitor, it's not so intuitive, but the > situation is so rare we can just rely on the user to issue cont again. Making this kind of user-visible change would be a bad idea. We are talking about multiple stop states here, but only a single function (vm_stop) to enter them - maybe that's not optimal. But the point is that we were missing one stop-to-stop transition. And that needs to be fixed, either inside vm_stop or when it is called. If you want to lock the VM into paused state, add a new symmetric API that does precisely this. That API would send the VM into RSTATE_LOCKED if it is not yet stopped on lock or is still locked on resume. That would avoid redefining stop states that have no use for lock-counting semantics. Jan signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH] qed: fix use-after-free during l2 cache commit
On 09/30/2011 05:39 AM, Stefan Hajnoczi wrote:
QED's metadata caching strategy allows two parallel requests to race for
metadata lookup. The first one to complete will populate the metadata
cache and the second one will drop the data it just read in favor of the
cached data.
There is a use-after-free in qed_read_l2_table_cb() and
qed_commit_l2_update() where l2_table->offset was used after the
l2_table may have been freed due to a metadata lookup race. Fix this by
keeping the l2_offset in a local variable and not reaching into the
possibly freed l2_table.
Reported-by: Amit Shah
Signed-off-by: Stefan Hajnoczi
Applied. Thanks.
Regards,
Anthony Liguori
---
Hi Amit,
Thanks for reporting the assertion failure you saw at http://fpaste.org/CDuv/.
Does this patch fix the problem?
If not, please send details on your setup and how to reproduce the issue.
Thanks,
Stefan
block/qed-table.c |6 +++---
block/qed.c |4 ++--
2 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/block/qed-table.c b/block/qed-table.c
index d96afa8..f31f9ff 100644
--- a/block/qed-table.c
+++ b/block/qed-table.c
@@ -222,21 +222,21 @@ static void qed_read_l2_table_cb(void *opaque, int ret)
QEDRequest *request = read_l2_table_cb->request;
BDRVQEDState *s = read_l2_table_cb->s;
CachedL2Table *l2_table = request->l2_table;
+uint64_t l2_offset = read_l2_table_cb->l2_offset;
if (ret) {
/* can't trust loaded L2 table anymore */
qed_unref_l2_cache_entry(l2_table);
request->l2_table = NULL;
} else {
-l2_table->offset = read_l2_table_cb->l2_offset;
+l2_table->offset = l2_offset;
qed_commit_l2_cache_entry(&s->l2_cache, l2_table);
/* This is guaranteed to succeed because we just committed the entry
* to the cache.
*/
-request->l2_table = qed_find_l2_cache_entry(&s->l2_cache,
-l2_table->offset);
+request->l2_table = qed_find_l2_cache_entry(&s->l2_cache, l2_offset);
assert(request->l2_table != NULL);
}
diff --git a/block/qed.c b/block/qed.c
index 624e261..e87dc4d 100644
--- a/block/qed.c
+++ b/block/qed.c
@@ -911,14 +911,14 @@ static void qed_commit_l2_update(void *opaque, int ret)
QEDAIOCB *acb = opaque;
BDRVQEDState *s = acb_to_s(acb);
CachedL2Table *l2_table = acb->request.l2_table;
+uint64_t l2_offset = l2_table->offset;
qed_commit_l2_cache_entry(&s->l2_cache, l2_table);
/* This is guaranteed to succeed because we just committed the entry to
the
* cache.
*/
-acb->request.l2_table = qed_find_l2_cache_entry(&s->l2_cache,
-l2_table->offset);
+acb->request.l2_table = qed_find_l2_cache_entry(&s->l2_cache, l2_offset);
assert(acb->request.l2_table != NULL);
qed_aio_next_io(opaque, ret);
[Qemu-devel] [PATCH 0/6] block: do request processing in a coroutine
Block layer features like dirty block tracing, I/O throttling, and live block copy are forced to duplicate code due to the three different interfaces: synchronous, asynchronous, and coroutines. Since there are bdrv_read(), bdrv_aio_readv(), and bdrv_co_readv() interfaces for read (and similar for write), per-request processing needs to be duplicated for each of these execution contexts. For example, dirty block tracking code is duplicated across these three interfaces. This patch series unifies request processing so that there is only one code path. I see this as a prerequisite to the live block copy (image streaming) code I am working on, so I'm pushing it now. The short-term win from this series is that it becomes easy to add live block copy and other features. We now have a single code path where the perf-request processing is done. The longer-term win will be dropping the BlockDriver .bdrv_read(), .bdrv_write(), .bdrv_aio_readv(), and .bdrv_aio_writev() interfaces. By doing that we can bring all BlockDrivers onto a common interface, namely .bdrv_co_readv() and .bdrv_co_writev(). It will also allow us to drop most of the sync and aio emulation code. A consequence of this patch series is that every I/O request goes through at least one coroutine. There is no longer a direct .bdrv_read(), .bdrv_write(), .bdrv_aio_readv(), or .bdrv_aio_writev() call - we're trying to phase out those interfaces. I have not noticed performance degradation in correctness tests but we need to confirm that there has not been a performance regression. Stefan Hajnoczi (6): block: directly invoke .bdrv_aio_*() in bdrv_co_io_em() block: split out bdrv_co_do_readv() and bdrv_co_do_writev() block: switch bdrv_read()/bdrv_write() to coroutines block: switch bdrv_aio_readv() to coroutines block: mark blocks dirty on coroutine write completion block: switch bdrv_aio_writev() to coroutines block.c | 273 +++ 1 files changed, 134 insertions(+), 139 deletions(-) -- 1.7.6.3
[Qemu-devel] [PATCH 6/6] block: switch bdrv_aio_writev() to coroutines
More sync, aio, and coroutine unification. Make bdrv_aio_writev() go
through coroutine request processing.
Remove the dirty block callback mechanism which was needed only for aio
processing and can be done more naturally in coroutine context.
Signed-off-by: Stefan Hajnoczi
---
block.c | 66 +-
1 files changed, 2 insertions(+), 64 deletions(-)
diff --git a/block.c b/block.c
index 5d6e17f..e80121b 100644
--- a/block.c
+++ b/block.c
@@ -2364,76 +2364,14 @@ BlockDriverAIOCB *bdrv_aio_readv(BlockDriverState *bs,
int64_t sector_num,
cb, opaque, false, bdrv_co_do_rw);
}
-typedef struct BlockCompleteData {
-BlockDriverCompletionFunc *cb;
-void *opaque;
-BlockDriverState *bs;
-int64_t sector_num;
-int nb_sectors;
-} BlockCompleteData;
-
-static void block_complete_cb(void *opaque, int ret)
-{
-BlockCompleteData *b = opaque;
-
-if (b->bs->dirty_bitmap) {
-set_dirty_bitmap(b->bs, b->sector_num, b->nb_sectors, 1);
-}
-b->cb(b->opaque, ret);
-g_free(b);
-}
-
-static BlockCompleteData *blk_dirty_cb_alloc(BlockDriverState *bs,
- int64_t sector_num,
- int nb_sectors,
- BlockDriverCompletionFunc *cb,
- void *opaque)
-{
-BlockCompleteData *blkdata = g_malloc0(sizeof(BlockCompleteData));
-
-blkdata->bs = bs;
-blkdata->cb = cb;
-blkdata->opaque = opaque;
-blkdata->sector_num = sector_num;
-blkdata->nb_sectors = nb_sectors;
-
-return blkdata;
-}
-
BlockDriverAIOCB *bdrv_aio_writev(BlockDriverState *bs, int64_t sector_num,
QEMUIOVector *qiov, int nb_sectors,
BlockDriverCompletionFunc *cb, void *opaque)
{
-BlockDriver *drv = bs->drv;
-BlockDriverAIOCB *ret;
-BlockCompleteData *blk_cb_data;
-
trace_bdrv_aio_writev(bs, sector_num, nb_sectors, opaque);
-if (!drv)
-return NULL;
-if (bs->read_only)
-return NULL;
-if (bdrv_check_request(bs, sector_num, nb_sectors))
-return NULL;
-
-if (bs->dirty_bitmap) {
-blk_cb_data = blk_dirty_cb_alloc(bs, sector_num, nb_sectors, cb,
- opaque);
-cb = &block_complete_cb;
-opaque = blk_cb_data;
-}
-
-ret = drv->bdrv_aio_writev(bs, sector_num, qiov, nb_sectors,
- cb, opaque);
-
-if (ret) {
-if (bs->wr_highest_sector < sector_num + nb_sectors - 1) {
-bs->wr_highest_sector = sector_num + nb_sectors - 1;
-}
-}
-
-return ret;
+return bdrv_co_aio_rw_vector(bs, sector_num, qiov, nb_sectors,
+ cb, opaque, true, bdrv_co_do_rw);
}
--
1.7.6.3
[Qemu-devel] [PATCH 5/6] block: mark blocks dirty on coroutine write completion
The aio write operation marks blocks dirty when the write operation
completes. The coroutine write operation marks blocks dirty before
issuing the write operation.
It seems safest to mark the block dirty when the operation completes so
that anything tracking dirty blocks will not act before the change has
been made to the image file.
Make the coroutine write operation dirty blocks on write completion.
Signed-off-by: Stefan Hajnoczi
---
block.c |5 -
1 files changed, 4 insertions(+), 1 deletions(-)
diff --git a/block.c b/block.c
index b83e911..5d6e17f 100644
--- a/block.c
+++ b/block.c
@@ -1307,6 +1307,7 @@ static int coroutine_fn
bdrv_co_do_writev(BlockDriverState *bs,
int64_t sector_num, int nb_sectors, QEMUIOVector *qiov)
{
BlockDriver *drv = bs->drv;
+int ret;
if (!bs->drv) {
return -ENOMEDIUM;
@@ -1318,6 +1319,8 @@ static int coroutine_fn
bdrv_co_do_writev(BlockDriverState *bs,
return -EIO;
}
+ret = drv->bdrv_co_writev(bs, sector_num, nb_sectors, qiov);
+
if (bs->dirty_bitmap) {
set_dirty_bitmap(bs, sector_num, nb_sectors, 1);
}
@@ -1326,7 +1329,7 @@ static int coroutine_fn
bdrv_co_do_writev(BlockDriverState *bs,
bs->wr_highest_sector = sector_num + nb_sectors - 1;
}
-return drv->bdrv_co_writev(bs, sector_num, nb_sectors, qiov);
+return ret;
}
int coroutine_fn bdrv_co_writev(BlockDriverState *bs, int64_t sector_num,
--
1.7.6.3
[Qemu-devel] [PATCH 3/6] block: switch bdrv_read()/bdrv_write() to coroutines
The bdrv_read()/bdrv_write() functions call .bdrv_read()/.bdrv_write().
They should go through bdrv_co_do_readv() and bdrv_co_do_writev()
instead in order to unify request processing code across sync, aio, and
coroutine interfaces. This is also an important step towards removing
BlockDriverState .bdrv_read()/.bdrv_write() in the future.
Signed-off-by: Stefan Hajnoczi
---
block.c | 112 +++
1 files changed, 62 insertions(+), 50 deletions(-)
diff --git a/block.c b/block.c
index d15784e..90c29db 100644
--- a/block.c
+++ b/block.c
@@ -44,6 +44,8 @@
#include
#endif
+#define NOT_DONE 0x7fff /* used while emulated sync operation in progress
*/
+
static void bdrv_dev_change_media_cb(BlockDriverState *bs, bool load);
static BlockDriverAIOCB *bdrv_aio_readv_em(BlockDriverState *bs,
int64_t sector_num, QEMUIOVector *qiov, int nb_sectors,
@@ -74,6 +76,8 @@ static int coroutine_fn bdrv_co_writev_em(BlockDriverState
*bs,
static int coroutine_fn bdrv_co_flush_em(BlockDriverState *bs);
static int coroutine_fn bdrv_co_do_readv(BlockDriverState *bs,
int64_t sector_num, int nb_sectors, QEMUIOVector *qiov);
+static int coroutine_fn bdrv_co_do_writev(BlockDriverState *bs,
+int64_t sector_num, int nb_sectors, QEMUIOVector *qiov);
static QTAILQ_HEAD(, BlockDriverState) bdrv_states =
QTAILQ_HEAD_INITIALIZER(bdrv_states);
@@ -1038,30 +1042,69 @@ static inline bool bdrv_has_async_flush(BlockDriver
*drv)
return drv->bdrv_aio_flush != bdrv_aio_flush_em;
}
-/* return < 0 if error. See bdrv_write() for the return codes */
-int bdrv_read(BlockDriverState *bs, int64_t sector_num,
- uint8_t *buf, int nb_sectors)
+typedef struct RwCo {
+BlockDriverState *bs;
+int64_t sector_num;
+int nb_sectors;
+QEMUIOVector *qiov;
+bool is_write;
+int ret;
+} RwCo;
+
+static void coroutine_fn bdrv_rw_co_entry(void *opaque)
{
-BlockDriver *drv = bs->drv;
+RwCo *rwco = opaque;
-if (!drv)
-return -ENOMEDIUM;
+if (!rwco->is_write) {
+rwco->ret = bdrv_co_do_readv(rwco->bs, rwco->sector_num,
+ rwco->nb_sectors, rwco->qiov);
+} else {
+rwco->ret = bdrv_co_do_writev(rwco->bs, rwco->sector_num,
+ rwco->nb_sectors, rwco->qiov);
+}
+}
-if (bdrv_has_async_rw(drv) && qemu_in_coroutine()) {
-QEMUIOVector qiov;
-struct iovec iov = {
-.iov_base = (void *)buf,
-.iov_len = nb_sectors * BDRV_SECTOR_SIZE,
-};
+/*
+ * Process a synchronous request using coroutines
+ */
+static int bdrv_rw_co(BlockDriverState *bs, int64_t sector_num, uint8_t *buf,
+ int nb_sectors, bool is_write)
+{
+QEMUIOVector qiov;
+struct iovec iov = {
+.iov_base = (void *)buf,
+.iov_len = nb_sectors * BDRV_SECTOR_SIZE,
+};
+Coroutine *co;
+RwCo rwco = {
+.bs = bs,
+.sector_num = sector_num,
+.nb_sectors = nb_sectors,
+.qiov = &qiov,
+.is_write = is_write,
+.ret = NOT_DONE,
+};
-qemu_iovec_init_external(&qiov, &iov, 1);
-return bdrv_co_readv(bs, sector_num, nb_sectors, &qiov);
-}
+qemu_iovec_init_external(&qiov, &iov, 1);
-if (bdrv_check_request(bs, sector_num, nb_sectors))
-return -EIO;
+if (qemu_in_coroutine()) {
+/* Fast-path if already in coroutine context */
+bdrv_rw_co_entry(&rwco);
+} else {
+co = qemu_coroutine_create(bdrv_rw_co_entry);
+qemu_coroutine_enter(co, &rwco);
+while (rwco.ret == NOT_DONE) {
+qemu_aio_wait();
+}
+}
+return rwco.ret;
+}
-return drv->bdrv_read(bs, sector_num, buf, nb_sectors);
+/* return < 0 if error. See bdrv_write() for the return codes */
+int bdrv_read(BlockDriverState *bs, int64_t sector_num,
+ uint8_t *buf, int nb_sectors)
+{
+return bdrv_rw_co(bs, sector_num, buf, nb_sectors, false);
}
static void set_dirty_bitmap(BlockDriverState *bs, int64_t sector_num,
@@ -1101,36 +1144,7 @@ static void set_dirty_bitmap(BlockDriverState *bs,
int64_t sector_num,
int bdrv_write(BlockDriverState *bs, int64_t sector_num,
const uint8_t *buf, int nb_sectors)
{
-BlockDriver *drv = bs->drv;
-
-if (!bs->drv)
-return -ENOMEDIUM;
-
-if (bdrv_has_async_rw(drv) && qemu_in_coroutine()) {
-QEMUIOVector qiov;
-struct iovec iov = {
-.iov_base = (void *)buf,
-.iov_len = nb_sectors * BDRV_SECTOR_SIZE,
-};
-
-qemu_iovec_init_external(&qiov, &iov, 1);
-return bdrv_co_writev(bs, sector_num, nb_sectors, &qiov);
-}
-
-if (bs->read_only)
-return -EACCES;
-if (bdrv_check_request(bs, sector_num, nb_sectors))
-return -EIO;
-
-if (bs->dirty_bitmap) {
-set_dirty_bitmap(bs, sector_num, nb_sectors,
[Qemu-devel] [PATCH 1/6] block: directly invoke .bdrv_aio_*() in bdrv_co_io_em()
We will unify block layer request processing across sync, aio, and
coroutines and this means a .bdrv_co_*() emulation function should not
call back into the public interface. There's no need here, just call
.bdrv_aio_*() directly.
The gory details: bdrv_co_io_em() cannot call back into the public
bdrv_aio_*() interface since that will be handled using coroutines,
which causes us to call into bdrv_co_io_em() again in an infinite loop
:).
Signed-off-by: Stefan Hajnoczi
---
block.c |8
1 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/block.c b/block.c
index e3fe97f..dc36b2b 100644
--- a/block.c
+++ b/block.c
@@ -2990,11 +2990,11 @@ static int coroutine_fn bdrv_co_io_em(BlockDriverState
*bs, int64_t sector_num,
BlockDriverAIOCB *acb;
if (is_write) {
-acb = bdrv_aio_writev(bs, sector_num, iov, nb_sectors,
- bdrv_co_io_em_complete, &co);
+acb = bs->drv->bdrv_aio_writev(bs, sector_num, iov, nb_sectors,
+ bdrv_co_io_em_complete, &co);
} else {
-acb = bdrv_aio_readv(bs, sector_num, iov, nb_sectors,
- bdrv_co_io_em_complete, &co);
+acb = bs->drv->bdrv_aio_readv(bs, sector_num, iov, nb_sectors,
+ bdrv_co_io_em_complete, &co);
}
trace_bdrv_co_io(is_write, acb);
--
1.7.6.3
[Qemu-devel] [PATCH 2/6] block: split out bdrv_co_do_readv() and bdrv_co_do_writev()
The public interface for I/O in coroutine context is bdrv_co_readv() and
bdrv_co_writev(). Split out the request processing code into
bdrv_co_do_readv() and bdrv_co_writev() so that it can be called
internally when we refactor all request processing to use coroutines.
Signed-off-by: Stefan Hajnoczi
---
block.c | 34 +++---
1 files changed, 27 insertions(+), 7 deletions(-)
diff --git a/block.c b/block.c
index dc36b2b..d15784e 100644
--- a/block.c
+++ b/block.c
@@ -72,6 +72,8 @@ static int coroutine_fn bdrv_co_writev_em(BlockDriverState
*bs,
int64_t sector_num, int nb_sectors,
QEMUIOVector *iov);
static int coroutine_fn bdrv_co_flush_em(BlockDriverState *bs);
+static int coroutine_fn bdrv_co_do_readv(BlockDriverState *bs,
+int64_t sector_num, int nb_sectors, QEMUIOVector *qiov);
static QTAILQ_HEAD(, BlockDriverState) bdrv_states =
QTAILQ_HEAD_INITIALIZER(bdrv_states);
@@ -1249,13 +1251,14 @@ int bdrv_pwrite_sync(BlockDriverState *bs, int64_t
offset,
return 0;
}
-int coroutine_fn bdrv_co_readv(BlockDriverState *bs, int64_t sector_num,
-int nb_sectors, QEMUIOVector *qiov)
+/*
+ * 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)
{
BlockDriver *drv = bs->drv;
-trace_bdrv_co_readv(bs, sector_num, nb_sectors);
-
if (!drv) {
return -ENOMEDIUM;
}
@@ -1266,12 +1269,21 @@ int coroutine_fn bdrv_co_readv(BlockDriverState *bs,
int64_t sector_num,
return drv->bdrv_co_readv(bs, sector_num, nb_sectors, qiov);
}
-int coroutine_fn bdrv_co_writev(BlockDriverState *bs, int64_t sector_num,
+int coroutine_fn bdrv_co_readv(BlockDriverState *bs, int64_t sector_num,
int nb_sectors, QEMUIOVector *qiov)
{
-BlockDriver *drv = bs->drv;
+trace_bdrv_co_readv(bs, sector_num, nb_sectors);
-trace_bdrv_co_writev(bs, sector_num, nb_sectors);
+return bdrv_co_do_readv(bs, sector_num, nb_sectors, qiov);
+}
+
+/*
+ * Handle a write request in coroutine context
+ */
+static int coroutine_fn bdrv_co_do_writev(BlockDriverState *bs,
+int64_t sector_num, int nb_sectors, QEMUIOVector *qiov)
+{
+BlockDriver *drv = bs->drv;
if (!bs->drv) {
return -ENOMEDIUM;
@@ -1294,6 +1306,14 @@ int coroutine_fn bdrv_co_writev(BlockDriverState *bs,
int64_t sector_num,
return drv->bdrv_co_writev(bs, sector_num, nb_sectors, qiov);
}
+int coroutine_fn bdrv_co_writev(BlockDriverState *bs, int64_t sector_num,
+int nb_sectors, QEMUIOVector *qiov)
+{
+trace_bdrv_co_writev(bs, sector_num, nb_sectors);
+
+return bdrv_co_do_writev(bs, sector_num, nb_sectors, qiov);
+}
+
/**
* Truncate file to 'offset' bytes (needed only for file protocols)
*/
--
1.7.6.3
[Qemu-devel] [PATCH 4/6] block: switch bdrv_aio_readv() to coroutines
More sync, aio, and coroutine unification. Make bdrv_aio_readv() go
through coroutine request processing.
Signed-off-by: Stefan Hajnoczi
---
block.c | 48 +++-
1 files changed, 35 insertions(+), 13 deletions(-)
diff --git a/block.c b/block.c
index 90c29db..b83e911 100644
--- a/block.c
+++ b/block.c
@@ -78,6 +78,15 @@ static int coroutine_fn bdrv_co_do_readv(BlockDriverState
*bs,
int64_t sector_num, int nb_sectors, QEMUIOVector *qiov);
static int coroutine_fn bdrv_co_do_writev(BlockDriverState *bs,
int64_t sector_num, int nb_sectors, QEMUIOVector *qiov);
+static BlockDriverAIOCB *bdrv_co_aio_rw_vector(BlockDriverState *bs,
+ int64_t sector_num,
+ QEMUIOVector *qiov,
+ int nb_sectors,
+ BlockDriverCompletionFunc *cb,
+ void *opaque,
+ bool is_write,
+ CoroutineEntry *entry);
+static void coroutine_fn bdrv_co_do_rw(void *opaque);
static QTAILQ_HEAD(, BlockDriverState) bdrv_states =
QTAILQ_HEAD_INITIALIZER(bdrv_states);
@@ -2346,17 +2355,10 @@ BlockDriverAIOCB *bdrv_aio_readv(BlockDriverState *bs,
int64_t sector_num,
QEMUIOVector *qiov, int nb_sectors,
BlockDriverCompletionFunc *cb, void *opaque)
{
-BlockDriver *drv = bs->drv;
-
trace_bdrv_aio_readv(bs, sector_num, nb_sectors, opaque);
-if (!drv)
-return NULL;
-if (bdrv_check_request(bs, sector_num, nb_sectors))
-return NULL;
-
-return drv->bdrv_aio_readv(bs, sector_num, qiov, nb_sectors,
- cb, opaque);
+return bdrv_co_aio_rw_vector(bs, sector_num, qiov, nb_sectors,
+ cb, opaque, false, bdrv_co_do_rw);
}
typedef struct BlockCompleteData {
@@ -2803,6 +2805,7 @@ static void bdrv_co_rw_bh(void *opaque)
qemu_aio_release(acb);
}
+/* Invoke .bdrv_co_readv/.bdrv_co_writev */
static void coroutine_fn bdrv_co_rw(void *opaque)
{
BlockDriverAIOCBCoroutine *acb = opaque;
@@ -2820,13 +2823,32 @@ static void coroutine_fn bdrv_co_rw(void *opaque)
qemu_bh_schedule(acb->bh);
}
+/* Invoke bdrv_co_do_readv/bdrv_co_do_writev */
+static void coroutine_fn bdrv_co_do_rw(void *opaque)
+{
+BlockDriverAIOCBCoroutine *acb = opaque;
+BlockDriverState *bs = acb->common.bs;
+
+if (!acb->is_write) {
+acb->req.error = bdrv_co_do_readv(bs, acb->req.sector,
+acb->req.nb_sectors, acb->req.qiov);
+} else {
+acb->req.error = bdrv_co_do_writev(bs, acb->req.sector,
+acb->req.nb_sectors, acb->req.qiov);
+}
+
+acb->bh = qemu_bh_new(bdrv_co_rw_bh, acb);
+qemu_bh_schedule(acb->bh);
+}
+
static BlockDriverAIOCB *bdrv_co_aio_rw_vector(BlockDriverState *bs,
int64_t sector_num,
QEMUIOVector *qiov,
int nb_sectors,
BlockDriverCompletionFunc *cb,
void *opaque,
- bool is_write)
+ bool is_write,
+ CoroutineEntry *entry)
{
Coroutine *co;
BlockDriverAIOCBCoroutine *acb;
@@ -2837,7 +2859,7 @@ static BlockDriverAIOCB
*bdrv_co_aio_rw_vector(BlockDriverState *bs,
acb->req.qiov = qiov;
acb->is_write = is_write;
-co = qemu_coroutine_create(bdrv_co_rw);
+co = qemu_coroutine_create(entry);
qemu_coroutine_enter(co, acb);
return &acb->common;
@@ -2848,7 +2870,7 @@ static BlockDriverAIOCB
*bdrv_co_aio_readv_em(BlockDriverState *bs,
BlockDriverCompletionFunc *cb, void *opaque)
{
return bdrv_co_aio_rw_vector(bs, sector_num, qiov, nb_sectors, cb, opaque,
- false);
+ false, bdrv_co_rw);
}
static BlockDriverAIOCB *bdrv_co_aio_writev_em(BlockDriverState *bs,
@@ -2856,7 +2878,7 @@ static BlockDriverAIOCB
*bdrv_co_aio_writev_em(BlockDriverState *bs,
BlockDriverCompletionFunc *cb, void *opaque)
{
return bdrv_co_aio_rw_vector(bs, sector_num, qiov, nb_sectors, cb, opaque,
- true);
+ true, bdrv_co_rw);
}
static BlockDriverAIOCB *bdrv_aio_flush_em(BlockDriverState *bs,
--
1.7.6.3
Re: [Qemu-devel] [PATCH] runstate: do not discard runstate changes when paused
On 10/05/2011 05:43 PM, Avi Kivity wrote: On 10/05/2011 04:37 PM, Luiz Capitulino wrote: Now, we have three options to fix this but I don't know which one to choose: 1. We could just add the transition RSTATE_PAUSED -> RSTATE_POST_MIGRATE as valid. Not sure this is a good thing to do though, as it seems a silly workaround for the fact that the transition to RSTATE_PRE_MIGRATE has never occurred 2. This patch makes vm_stop() do the state transition even if the VM is already stopped. Seems good enough, except that I fear two things. First, today we know that vm_stop() is a no-op if the VM is already stopped, so there's a semantic change that could turn out to be trap. Second, I also fear people using vm_stop() as a way to change the VM status, just like runstate_set() (which can also become an horrible trap) 3. Avi suggested we should keep a reference count, so that states are not discarded: http://lists.gnu.org/archive/html/qemu-devel/2011-08/msg00595.html That solution seemed to be the perfect one, except for one important detail: how should we implement vm_start() (and thus 'cont')? In order to maintain how we behave with the external world, the only option is that vm_start() will set the stop count to 0. Ie, doesn't matter if we have stopped the VM 500 times at some point, a vm_start() call will discard all stored states. Not sure if that's what you expected, but the first time I read Avi's idea I had the impression that it would be a good idea that vm_start() decremented the ref count only once, ie. vm_stop() and vm_start() calls have to match. vm_start() should be symmetric with vm_stop(). That is, if a piece of code wants to execute with vcpus stopped, it should just run inside a stop/start pair. The only confusion can come from the user, if he sees multiple stop events and expects that just one cont will continue the vm. For the machine monitor, we should just document that the you have to issue one cont for every stop event you see (plus any stops you issue). It's not unnatural - the code that handles a stop_due_to_enospace can work to fix the error and issue a cont, disregarding any other stops in progress (due to a user pressing the stop button, or migration, or cpu hotplug, or whatever). For the human monitor, it's not so intuitive, but the situation is so rare we can just rely on the user to issue cont again. btw, another way to look at it is like a lock: vm_lock() / vm_unlock(). It's obvious that vm_unlock() can't release all the locks. -- error compiling committee.c: too many arguments to function
Re: [Qemu-devel] [PATCH] runstate: do not discard runstate changes when paused
On 10/05/2011 04:37 PM, Luiz Capitulino wrote: Now, we have three options to fix this but I don't know which one to choose: 1. We could just add the transition RSTATE_PAUSED -> RSTATE_POST_MIGRATE as valid. Not sure this is a good thing to do though, as it seems a silly workaround for the fact that the transition to RSTATE_PRE_MIGRATE has never occurred 2. This patch makes vm_stop() do the state transition even if the VM is already stopped. Seems good enough, except that I fear two things. First, today we know that vm_stop() is a no-op if the VM is already stopped, so there's a semantic change that could turn out to be trap. Second, I also fear people using vm_stop() as a way to change the VM status, just like runstate_set() (which can also become an horrible trap) 3. Avi suggested we should keep a reference count, so that states are not discarded: http://lists.gnu.org/archive/html/qemu-devel/2011-08/msg00595.html That solution seemed to be the perfect one, except for one important detail: how should we implement vm_start() (and thus 'cont')? In order to maintain how we behave with the external world, the only option is that vm_start() will set the stop count to 0. Ie, doesn't matter if we have stopped the VM 500 times at some point, a vm_start() call will discard all stored states. Not sure if that's what you expected, but the first time I read Avi's idea I had the impression that it would be a good idea that vm_start() decremented the ref count only once, ie. vm_stop() and vm_start() calls have to match. vm_start() should be symmetric with vm_stop(). That is, if a piece of code wants to execute with vcpus stopped, it should just run inside a stop/start pair. The only confusion can come from the user, if he sees multiple stop events and expects that just one cont will continue the vm. For the machine monitor, we should just document that the you have to issue one cont for every stop event you see (plus any stops you issue). It's not unnatural - the code that handles a stop_due_to_enospace can work to fix the error and issue a cont, disregarding any other stops in progress (due to a user pressing the stop button, or migration, or cpu hotplug, or whatever). For the human monitor, it's not so intuitive, but the situation is so rare we can just rely on the user to issue cont again. -- error compiling committee.c: too many arguments to function
Re: [Qemu-devel] [RFC] New Migration Protocol using Visitor Interface
On Wed, Oct 05, 2011 at 07:54:21AM -0500, Anthony Liguori wrote: > On 10/04/2011 09:05 PM, Stefan Berger wrote: > >On 10/03/2011 09:43 AM, Anthony Liguori wrote: > >>On 10/03/2011 08:24 AM, Michael S. Tsirkin wrote: > >>>On Mon, Oct 03, 2011 at 07:51:00AM -0500, Anthony Liguori wrote: > >Here are some suggestions: > > > >- Let's make the protocol be BER directly. > >As a first step, use a single octet string for > >the whole of data. Next, start splitting this up. > > This can't be done without breaking the old style migration > protocol. I don't have a problem with that but I do have a problem > with repeatedly breaking migration protocol. > >>> > >>>As long as this is within a release cycle, is this a real problem? > >> > >>I think if we try to fit it within a release we'll either end up with a 2 > >>year > >>long release or a half-broken conversion. > >> > >>I'd rather buy ourselves time by supporting both formats. That way we can > >>remove the old format when we're satisfied with the ASN.1 encoding. > >Hm, if backwards compatibility is what we want to achieve (migrating from > >Qemu > >1.1 to Qemu 1.0) then at least the ASN.1 decoder / encoder should be all > >done in > >1.0, no? Otherwise what would it mean to if 1.0 just skipped types 1.1 sends > >and > >doesn't understand? > > Before we introduce ASN1, we ought to introduce migration > capabilities. FWIW we introduce it for tpm as first step. > Migration capabilities would be used to negotation > ASN.1 over the wire. I'm not yet really convinced we need capabilities at all. Would be sad to make asn depend on that. > That means that 1.1 would use the existing > protocol to talk to 1.0. We basically never had qemu that can talk across versions 100%. The compability we carry around is so much dead code. And no wonder: there's no way to parse the protocol without being bug for bug compatible. IMO let's just switch to a sane protocol first. Being compatible with that will be much easier. > >>There are multiple things to consider with compatibility: > >> > >>1) Creating compatible device models. This is a qdev problem and can't be > >>solved in the protocol. > >> > >>2) Ensuring we are sending all the data we need to. I think we solve this > >>problem by autogenerating Visitors from the C definitions of the device > >>structures. > >> > >I would have thought that we would write a function that takes the > >VMStateDescription as an argument and write ASN.1 BER or CER comprising: > >- a header containing the version of the device data > >- the minimum version required to read the device data > >- walk the array of VMStateFields and encode the the device data > > Sort of. You modify VMStateInfo to accept a visitor and name > parameter in load and put. Then you write an ASN.1 BER Visitor and > pass that visitor to VMStateInfo->load/put. > > Regards, > > Anthony Liguori > > > > >and similarly a function for walking the fields for decoding of each device > >state. > > > >So at least I am surprised to hear 'autogeneration' for this particular > >case... > > > >Stefan
Re: [Qemu-devel] [PATCH 19/23] migration: Export a function that tells if the migration has finished correctly
On 10/05/2011 07:24 AM, Juan Quintela wrote:
Anthony Liguori wrote:
On 09/23/2011 07:57 AM, Juan Quintela wrote:
This will allows us to hide the status values.
--- a/ui/spice-core.c
+++ b/ui/spice-core.c
@@ -447,9 +447,7 @@ void do_info_spice(Monitor *mon, QObject **ret_data)
static void migration_state_notifier(Notifier *notifier, void *data)
{
-int state = get_migration_state();
-
-if (state == MIG_STATE_COMPLETED) {
+if (migration_has_finished()) {
#if SPICE_SERVER_VERSION>= 0x000701 /* 0.7.1 */
spice_server_migrate_switch(spice_server);
#endif
I think the bug here is migration_state_notifier. It should take an
additional argument of MigrationState. Otherwise, how does this code
work with FT?
Thinking about it, we need to pass MigrationState and export the
function that see if migration has finished (otherwise we also need to
export all STATE definitions, or worse, the whole MigrationState
definition.
Moving to have a function
bool migration_has_finished(MIgrationState *s);
That does the obvious thing.
What do you think?
Yeah, that was what I was advocating.
Regards,
Anthony Liguori
Later, Juan.
[Qemu-devel] [PULL 00/26]: QMP queue
Anthony, This pull request contains only the first round of QAPI conversions series. I had to rebase it but the changes are rather simple. The changes (since d11cf8cc80d946dfc9a23597cd9a0bb1c487cfa7) are available in the following repository: git://repo.or.cz/qemu/qmp-unstable.git queue/qmp Anthony Liguori (6): error: let error_is_type take a NULL error qerror: add qerror_report_err() qapi: add code generation support for middle mode qapi: use middle mode in QMP server qapi: fixup command generation for functions that return list types qapi: convert query-name Luiz Capitulino (15): qapi: Don't use c_var() on enum strings qapi: Automatically generate a _MAX value for enums qapi: Convert query-version qapi: Convert query-kvm vl: Change qemu_vmstop_requested() to return a bool RunState: Drop the RSTATE_NO_STATE value RunState: Rename enum values as generated by the QAPI qapi: Convert query-status qapi: Convert query-uuid qapi: Convert query-chardev qapi: Convert query-commands qapi: Convert quit qapi: Convert stop qapi: Convert system_reset qapi: Convert system_powerdown Michael Roth (5): qapi: dealloc visitor, fix premature free and iteration logic qapi: generate qapi_free_* functions for *List types qapi: add test cases for generated free functions qapi: dealloc visitor, support freeing of nested lists qapi: modify visitor code generation for list iteration Makefile| 12 ++ Makefile.objs |3 + Makefile.target |6 +- error.c |4 + gdbstub.c | 26 ++-- hmp-commands.hx | 11 +- hmp.c | 116 ++ hmp.h | 31 + hw/ide/core.c |2 +- hw/scsi-disk.c |2 +- hw/virtio-blk.c |2 +- hw/watchdog.c |2 +- kvm-all.c |2 +- migration.c |6 +- monitor.c | 281 +-- qapi-schema.json| 273 + qapi/qapi-dealloc-visitor.c | 34 +- qapi/qapi-types-core.h |3 + qapi/qmp-input-visitor.c|4 +- qapi/qmp-output-visitor.c | 20 +++- qemu-char.c | 35 ++ qerror.c| 33 + qerror.h|2 + qmp-commands.hx | 57 +++-- qmp.c | 92 ++ savevm.c|4 +- scripts/qapi-commands.py| 98 --- scripts/qapi-types.py | 12 ++- scripts/qapi-visit.py |4 +- scripts/qapi.py |4 +- sysemu.h| 20 +--- test-qmp-commands.c | 29 + test-visitor.c | 48 ++-- vl.c| 133 ++--- 34 files changed, 959 insertions(+), 452 deletions(-)
[Qemu-devel] [PATCH 14/26] qapi: Convert query-version
Signed-off-by: Anthony Liguori
Signed-off-by: Luiz Capitulino
---
hmp.c| 13 +
hmp.h|1 +
monitor.c| 46 +++---
qapi-schema.json | 37 +
qmp-commands.hx |6 ++
qmp.c| 16
6 files changed, 76 insertions(+), 43 deletions(-)
diff --git a/hmp.c b/hmp.c
index 47e1ff7..bb6c86f 100644
--- a/hmp.c
+++ b/hmp.c
@@ -24,3 +24,16 @@ void hmp_info_name(Monitor *mon)
}
qapi_free_NameInfo(info);
}
+
+void hmp_info_version(Monitor *mon)
+{
+VersionInfo *info;
+
+info = qmp_query_version(NULL);
+
+monitor_printf(mon, "%" PRId64 ".%" PRId64 ".%" PRId64 "%s\n",
+ info->qemu.major, info->qemu.minor, info->qemu.micro,
+ info->package);
+
+qapi_free_VersionInfo(info);
+}
diff --git a/hmp.h b/hmp.h
index 5fe73f1..2aa75a2 100644
--- a/hmp.h
+++ b/hmp.h
@@ -18,5 +18,6 @@
#include "qapi-types.h"
void hmp_info_name(Monitor *mon);
+void hmp_info_version(Monitor *mon);
#endif
diff --git a/monitor.c b/monitor.c
index 83b4fa7..483ea71 100644
--- a/monitor.c
+++ b/monitor.c
@@ -731,37 +731,6 @@ help:
help_cmd(mon, "info");
}
-static void do_info_version_print(Monitor *mon, const QObject *data)
-{
-QDict *qdict;
-QDict *qemu;
-
-qdict = qobject_to_qdict(data);
-qemu = qdict_get_qdict(qdict, "qemu");
-
-monitor_printf(mon, "%" PRId64 ".%" PRId64 ".%" PRId64 "%s\n",
- qdict_get_int(qemu, "major"),
- qdict_get_int(qemu, "minor"),
- qdict_get_int(qemu, "micro"),
- qdict_get_str(qdict, "package"));
-}
-
-static void do_info_version(Monitor *mon, QObject **ret_data)
-{
-const char *version = QEMU_VERSION;
-int major = 0, minor = 0, micro = 0;
-char *tmp;
-
-major = strtol(version, &tmp, 10);
-tmp++;
-minor = strtol(tmp, &tmp, 10);
-tmp++;
-micro = strtol(tmp, &tmp, 10);
-
-*ret_data = qobject_from_jsonf("{ 'qemu': { 'major': %d, 'minor': %d, \
-'micro': %d }, 'package': %s }", major, minor, micro, QEMU_PKGVERSION);
-}
-
static QObject *get_cmd_dict(const char *name)
{
const char *p;
@@ -2872,8 +2841,7 @@ static const mon_cmd_t info_cmds[] = {
.args_type = "",
.params = "",
.help = "show the version of QEMU",
-.user_print = do_info_version_print,
-.mhandler.info_new = do_info_version,
+.mhandler.info = hmp_info_version,
},
{
.name = "network",
@@ -3172,14 +3140,6 @@ static const mon_cmd_t qmp_cmds[] = {
static const mon_cmd_t qmp_query_cmds[] = {
{
-.name = "version",
-.args_type = "",
-.params = "",
-.help = "show the version of QEMU",
-.user_print = do_info_version_print,
-.mhandler.info_new = do_info_version,
-},
-{
.name = "commands",
.args_type = "",
.params = "",
@@ -5185,9 +5145,9 @@ void monitor_resume(Monitor *mon)
static QObject *get_qmp_greeting(void)
{
-QObject *ver;
+QObject *ver = NULL;
-do_info_version(NULL, &ver);
+qmp_marshal_input_query_version(NULL, NULL, &ver);
return qobject_from_jsonf("{'QMP':{'version': %p,'capabilities':
[]}}",ver);
}
diff --git a/qapi-schema.json b/qapi-schema.json
index 3585324..3c0ac4e 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -23,3 +23,40 @@
# Since 0.14.0
##
{ 'command': 'query-name', 'returns': 'NameInfo' }
+
+##
+# @VersionInfo:
+#
+# A description of QEMU's version.
+#
+# @qemu.major: The major version of QEMU
+#
+# @qemu.minor: The minor version of QEMU
+#
+# @qemu.micro: The micro version of QEMU. By current convention, a micro
+# version of 50 signifies a development branch. A micro version
+# greater than or equal to 90 signifies a release candidate for
+# the next minor version. A micro version of less than 50
+# signifies a stable release.
+#
+# @package: QEMU will always set this field to an empty string. Downstream
+# versions of QEMU should set this to a non-empty string. The
+# exact format depends on the downstream however it highly
+# recommended that a unique name is used.
+#
+# Since: 0.14.0
+##
+{ 'type': 'VersionInfo',
+ 'data': {'qemu': {'major': 'int', 'minor': 'int', 'micro': 'int'},
+ 'package': 'str'} }
+
+##
+# @query-version:
+#
+# Returns the current version of QEMU.
+#
+# Returns: A @VersionInfo object describing the current version of QEMU.
+#
+# Since: 0.14.0
+##
+{ 'command': 'query-version', 'returns': 'VersionInfo' }
diff --git a/qmp-commands.hx b/qmp-commands.hx
index 7b3839e..9f067ea 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -1053,6 +1053,12 @@ Example:
EQMP
+{
+.name = "query-ver
[Qemu-devel] [PATCH 06/26] qapi: dealloc visitor, fix premature free and iteration logic
From: Michael Roth
Currently we do 3 things wrong:
1) The list iterator, in practice, is used in a manner where the pointer
we pass in is the same as the pointer we assign the output to from
visit_next_list(). This causes an infinite loop where we keep freeing
the same structures.
2) We attempt to free list->value rather than list. visit_type_
handles this. We should only be concerned with the containing list.
3) We free prematurely: iterator function will continue accessing values
we've already freed.
This patch should fix all of these issues. QmpOutputVisitor also suffers
from 1).
Reviewed-by: Anthony Liguori
Signed-off-by: Michael Roth
Signed-off-by: Luiz Capitulino
---
qapi/qapi-dealloc-visitor.c | 20 +++-
1 files changed, 15 insertions(+), 5 deletions(-)
diff --git a/qapi/qapi-dealloc-visitor.c b/qapi/qapi-dealloc-visitor.c
index f629061..6b586ad 100644
--- a/qapi/qapi-dealloc-visitor.c
+++ b/qapi/qapi-dealloc-visitor.c
@@ -26,6 +26,7 @@ struct QapiDeallocVisitor
{
Visitor visitor;
QTAILQ_HEAD(, StackEntry) stack;
+bool is_list_head;
};
static QapiDeallocVisitor *to_qov(Visitor *v)
@@ -70,15 +71,24 @@ static void qapi_dealloc_end_struct(Visitor *v, Error
**errp)
static void qapi_dealloc_start_list(Visitor *v, const char *name, Error **errp)
{
+QapiDeallocVisitor *qov = to_qov(v);
+qov->is_list_head = true;
}
-static GenericList *qapi_dealloc_next_list(Visitor *v, GenericList **list,
+static GenericList *qapi_dealloc_next_list(Visitor *v, GenericList **listp,
Error **errp)
{
-GenericList *retval = *list;
-g_free(retval->value);
-*list = retval->next;
-return retval;
+GenericList *list = *listp;
+QapiDeallocVisitor *qov = to_qov(v);
+
+if (!qov->is_list_head) {
+*listp = list->next;
+g_free(list);
+return *listp;
+}
+
+qov->is_list_head = false;
+return list;
}
static void qapi_dealloc_end_list(Visitor *v, Error **errp)
--
1.7.7.rc0.72.g4b5ea
[Qemu-devel] [PATCH 23/26] qapi: Convert quit
Signed-off-by: Anthony Liguori
Signed-off-by: Luiz Capitulino
---
hmp-commands.hx |2 +-
hmp.c|6 ++
hmp.h|1 +
monitor.c| 12
qapi-schema.json | 11 +++
qmp-commands.hx |5 +
qmp.c|6 ++
7 files changed, 26 insertions(+), 17 deletions(-)
diff --git a/hmp-commands.hx b/hmp-commands.hx
index 9e1cca8..2163e6f 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -43,7 +43,7 @@ ETEXI
.params = "",
.help = "quit the emulator",
.user_print = monitor_user_noop,
-.mhandler.cmd_new = do_quit,
+.mhandler.cmd = hmp_quit,
},
STEXI
diff --git a/hmp.c b/hmp.c
index 30c6cc9..7695dfc 100644
--- a/hmp.c
+++ b/hmp.c
@@ -93,3 +93,9 @@ void hmp_info_chardev(Monitor *mon)
qapi_free_ChardevInfoList(char_info);
}
+
+void hmp_quit(Monitor *mon, const QDict *qdict)
+{
+monitor_suspend(mon);
+qmp_quit(NULL);
+}
diff --git a/hmp.h b/hmp.h
index 7388f9a..a3dfafd 100644
--- a/hmp.h
+++ b/hmp.h
@@ -23,5 +23,6 @@ void hmp_info_kvm(Monitor *mon);
void hmp_info_status(Monitor *mon);
void hmp_info_uuid(Monitor *mon);
void hmp_info_chardev(Monitor *mon);
+void hmp_quit(Monitor *mon, const QDict *qdict);
#endif
diff --git a/monitor.c b/monitor.c
index 0c90506..6e13ef6 100644
--- a/monitor.c
+++ b/monitor.c
@@ -946,18 +946,6 @@ static void do_trace_print_events(Monitor *mon)
trace_print_events((FILE *)mon, &monitor_fprintf);
}
-/**
- * do_quit(): Quit QEMU execution
- */
-static int do_quit(Monitor *mon, const QDict *qdict, QObject **ret_data)
-{
-monitor_suspend(mon);
-no_shutdown = 0;
-qemu_system_shutdown_request();
-
-return 0;
-}
-
#ifdef CONFIG_VNC
static int change_vnc_password(const char *password)
{
diff --git a/qapi-schema.json b/qapi-schema.json
index 35434c1..3810463 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -225,3 +225,14 @@
##
{ 'command': 'query-commands', 'returns': ['CommandInfo'] }
+##
+# @quit:
+#
+# This command will cause the QEMU process to exit gracefully. While every
+# attempt is made to send the QMP response before terminating, this is not
+# guaranteed. When using this interface, a premature EOF would not be
+# unexpected.
+#
+# Since: 0.14.0
+##
+{ 'command': 'quit' }
diff --git a/qmp-commands.hx b/qmp-commands.hx
index 0fda5c3..151a5fa 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -63,10 +63,7 @@ EQMP
{
.name = "quit",
.args_type = "",
-.params = "",
-.help = "quit the emulator",
-.user_print = monitor_user_noop,
-.mhandler.cmd_new = do_quit,
+.mhandler.cmd_new = qmp_marshal_input_quit,
},
SQMP
diff --git a/qmp.c b/qmp.c
index 58337c7..1d380b6 100644
--- a/qmp.c
+++ b/qmp.c
@@ -70,3 +70,9 @@ UuidInfo *qmp_query_uuid(Error **errp)
return info;
}
+void qmp_quit(Error **err)
+{
+no_shutdown = 0;
+qemu_system_shutdown_request();
+}
+
--
1.7.7.rc0.72.g4b5ea
[Qemu-devel] [PATCH 22/26] qapi: Convert query-commands
Signed-off-by: Luiz Capitulino
---
monitor.c| 40 +++-
qapi-schema.json | 23 +++
qmp-commands.hx |6 ++
3 files changed, 44 insertions(+), 25 deletions(-)
diff --git a/monitor.c b/monitor.c
index 86a0dad..0c90506 100644
--- a/monitor.c
+++ b/monitor.c
@@ -731,39 +731,37 @@ help:
help_cmd(mon, "info");
}
-static QObject *get_cmd_dict(const char *name)
+static CommandInfoList *alloc_cmd_entry(const char *cmd_name)
{
-const char *p;
+CommandInfoList *info;
-/* Remove '|' from some commands */
-p = strchr(name, '|');
-if (p) {
-p++;
-} else {
-p = name;
-}
+info = g_malloc0(sizeof(*info));
+info->value = g_malloc0(sizeof(*info->value));
+info->value->name = g_strdup(cmd_name);
-return qobject_from_jsonf("{ 'name': %s }", p);
+return info;
}
-static void do_info_commands(Monitor *mon, QObject **ret_data)
+CommandInfoList *qmp_query_commands(Error **errp)
{
-QList *cmd_list;
+CommandInfoList *info, *cmd_list = NULL;
const mon_cmd_t *cmd;
-cmd_list = qlist_new();
-
for (cmd = qmp_cmds; cmd->name != NULL; cmd++) {
-qlist_append_obj(cmd_list, get_cmd_dict(cmd->name));
+info = alloc_cmd_entry(cmd->name);
+info->next = cmd_list;
+cmd_list = info;
}
for (cmd = qmp_query_cmds; cmd->name != NULL; cmd++) {
char buf[128];
snprintf(buf, sizeof(buf), "query-%s", cmd->name);
-qlist_append_obj(cmd_list, get_cmd_dict(buf));
+info = alloc_cmd_entry(buf);
+info->next = cmd_list;
+cmd_list = info;
}
-*ret_data = QOBJECT(cmd_list);
+return cmd_list;
}
/* get the current CPU defined by the user */
@@ -3064,14 +3062,6 @@ static const mon_cmd_t qmp_cmds[] = {
static const mon_cmd_t qmp_query_cmds[] = {
{
-.name = "commands",
-.args_type = "",
-.params = "",
-.help = "list QMP available commands",
-.user_print = monitor_user_noop,
-.mhandler.info_new = do_info_commands,
-},
-{
.name = "block",
.args_type = "",
.params = "",
diff --git a/qapi-schema.json b/qapi-schema.json
index cf55504..35434c1 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -202,3 +202,26 @@
# Since: 0.14.0
##
{ 'command': 'query-chardev', 'returns': ['ChardevInfo'] }
+
+##
+# @CommandInfo:
+#
+# Information about a QMP command
+#
+# @name: The command name
+#
+# Since: 0.14.0
+##
+{ 'type': 'CommandInfo', 'data': {'name': 'str'} }
+
+##
+# @query-commands:
+#
+# Return a list of supported QMP commands by this server
+#
+# Returns: A list of @CommandInfo for all supported commands
+#
+# Since: 0.14.0
+##
+{ 'command': 'query-commands', 'returns': ['CommandInfo'] }
+
diff --git a/qmp-commands.hx b/qmp-commands.hx
index dfc02af..0fda5c3 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -1090,6 +1090,12 @@ Note: This example has been shortened as the real
response is too long.
EQMP
+{
+.name = "query-commands",
+.args_type = "",
+.mhandler.cmd_new = qmp_marshal_input_query_commands,
+},
+
SQMP
query-chardev
-
--
1.7.7.rc0.72.g4b5ea
[Qemu-devel] [PATCH 04/26] qapi: use middle mode in QMP server
From: Anthony Liguori
Use the new middle mode within the existing QMP server.
Signed-off-by: Anthony Liguori
Signed-off-by: Luiz Capitulino
---
Makefile | 12
Makefile.objs|2 ++
Makefile.target |6 +++---
monitor.c|9 -
qapi-schema.json |3 +++
5 files changed, 24 insertions(+), 8 deletions(-)
create mode 100644 qapi-schema.json
diff --git a/Makefile b/Makefile
index 6ed3194..55ee79c 100644
--- a/Makefile
+++ b/Makefile
@@ -7,6 +7,7 @@ GENERATED_HEADERS = config-host.h trace.h qemu-options.def
ifeq ($(TRACE_BACKEND),dtrace)
GENERATED_HEADERS += trace-dtrace.h
endif
+GENERATED_HEADERS += qmp-commands.h qapi-types.h qapi-visit.h
ifneq ($(wildcard config-host.mak),)
# Put the all: rule here so that config-host.mak can contain dependencies.
@@ -187,9 +188,20 @@ $(qapi-dir)/qga-qapi-types.h:
$(SRC_PATH)/qapi-schema-guest.json $(SRC_PATH)/scr
$(qapi-dir)/qga-qapi-visit.c: $(qapi-dir)/qga-qapi-visit.h
$(qapi-dir)/qga-qapi-visit.h: $(SRC_PATH)/qapi-schema-guest.json
$(SRC_PATH)/scripts/qapi-visit.py
$(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-visit.py -o
"$(qapi-dir)" -p "qga-" < $<, " GEN $@")
+$(qapi-dir)/qga-qmp-commands.h: $(qapi-dir)/qga-qmp-marshal.c
$(qapi-dir)/qga-qmp-marshal.c: $(SRC_PATH)/qapi-schema-guest.json
$(SRC_PATH)/scripts/qapi-commands.py
$(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-commands.py -o
"$(qapi-dir)" -p "qga-" < $<, " GEN $@")
+qapi-types.c: qapi-types.h
+qapi-types.h: $(SRC_PATH)/qapi-schema.json $(SRC_PATH)/scripts/qapi-types.py
+ $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-types.py -o "."
< $<, " GEN $@")
+qapi-visit.c: qapi-visit.h
+qapi-visit.h: $(SRC_PATH)/qapi-schema.json $(SRC_PATH)/scripts/qapi-visit.py
+ $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-visit.py -o "."
< $<, " GEN $@")
+qmp-commands.h: qmp-marshal.c
+qmp-marshal.c: $(SRC_PATH)/qapi-schema.json
$(SRC_PATH)/scripts/qapi-commands.py
+ $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-commands.py -m
-o "." < $<, " GEN $@")
+
test-visitor.o: $(addprefix $(qapi-dir)/, test-qapi-types.c test-qapi-types.h
test-qapi-visit.c test-qapi-visit.h) $(qapi-obj-y)
test-visitor: test-visitor.o qfloat.o qint.o qdict.o qstring.o qlist.o qbool.o
$(qapi-obj-y) error.o osdep.o $(oslib-obj-y) qjson.o json-streamer.o
json-lexer.o json-parser.o qerror.o qemu-error.o qemu-tool.o
$(qapi-dir)/test-qapi-visit.o $(qapi-dir)/test-qapi-types.o
diff --git a/Makefile.objs b/Makefile.objs
index 8d23fbb..587ec36 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -406,6 +406,8 @@ qapi-nested-y = qapi-visit-core.o qmp-input-visitor.o
qmp-output-visitor.o qapi-
qapi-nested-y += qmp-registry.o qmp-dispatch.o
qapi-obj-y = $(addprefix qapi/, $(qapi-nested-y))
+common-obj-y += qmp-marshal.o qapi-visit.o qapi-types.o $(qapi-obj-y)
+
##
# guest agent
diff --git a/Makefile.target b/Makefile.target
index 88d2f1f..b5ed5df 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -375,7 +375,7 @@ obj-xtensa-y += xtensa-semi.o
main.o: QEMU_CFLAGS+=$(GPROF_CFLAGS)
-monitor.o: hmp-commands.h qmp-commands.h
+monitor.o: hmp-commands.h qmp-commands-old.h
$(obj-y) $(obj-$(TARGET_BASE_ARCH)-y): $(GENERATED_HEADERS)
@@ -407,13 +407,13 @@ gdbstub-xml.c: $(TARGET_XML_FILES)
$(SRC_PATH)/scripts/feature_to_c.sh
hmp-commands.h: $(SRC_PATH)/hmp-commands.hx
$(call quiet-command,sh $(SRC_PATH)/scripts/hxtool -h < $< > $@," GEN
$(TARGET_DIR)$@")
-qmp-commands.h: $(SRC_PATH)/qmp-commands.hx
+qmp-commands-old.h: $(SRC_PATH)/qmp-commands.hx
$(call quiet-command,sh $(SRC_PATH)/scripts/hxtool -h < $< > $@," GEN
$(TARGET_DIR)$@")
clean:
rm -f *.o *.a *~ $(PROGS) nwfpe/*.o fpu/*.o
rm -f *.d */*.d tcg/*.o ide/*.o 9pfs/*.o
- rm -f hmp-commands.h qmp-commands.h gdbstub-xml.c
+ rm -f hmp-commands.h qmp-commands-old.h gdbstub-xml.c
ifdef CONFIG_TRACE_SYSTEMTAP
rm -f *.stp
endif
diff --git a/monitor.c b/monitor.c
index d323ea5..e267d58 100644
--- a/monitor.c
+++ b/monitor.c
@@ -122,6 +122,7 @@ typedef struct mon_cmd_t {
int (*cmd_async)(Monitor *mon, const QDict *params,
MonitorCompletion *cb, void *opaque);
} mhandler;
+bool qapi;
int flags;
} mon_cmd_t;
@@ -3182,7 +3183,7 @@ static const mon_cmd_t info_cmds[] = {
};
static const mon_cmd_t qmp_cmds[] = {
-#include "qmp-commands.h"
+#include "qmp-commands-old.h"
{ /* NULL */ },
};
@@ -5107,12 +5108,10 @@ static void handle_qmp_command(JSONMessageParser
*parser, QList *tokens)
goto err_out;
}
-if (strstart(cmd_name, "query-", &query_cmd)) {
+cmd = qmp_find_cmd(cmd_name);
+if (!cmd && strstart(cmd_name, "query-", &query_cmd)) {
cmd = qmp_find_query_cmd(query_cmd);
-} else {
-cmd = qmp_f
[Qemu-devel] [PATCH 15/26] qapi: Convert query-kvm
Signed-off-by: Anthony Liguori
Signed-off-by: Luiz Capitulino
---
hmp.c| 16
hmp.h|1 +
monitor.c| 36 +---
qapi-schema.json | 25 +
qmp-commands.hx |6 ++
qmp.c| 13 +
6 files changed, 62 insertions(+), 35 deletions(-)
diff --git a/hmp.c b/hmp.c
index bb6c86f..94a7f74 100644
--- a/hmp.c
+++ b/hmp.c
@@ -37,3 +37,19 @@ void hmp_info_version(Monitor *mon)
qapi_free_VersionInfo(info);
}
+
+void hmp_info_kvm(Monitor *mon)
+{
+KvmInfo *info;
+
+info = qmp_query_kvm(NULL);
+monitor_printf(mon, "kvm support: ");
+if (info->present) {
+monitor_printf(mon, "%s\n", info->enabled ? "enabled" : "disabled");
+} else {
+monitor_printf(mon, "not compiled\n");
+}
+
+qapi_free_KvmInfo(info);
+}
+
diff --git a/hmp.h b/hmp.h
index 2aa75a2..a93ac1f 100644
--- a/hmp.h
+++ b/hmp.h
@@ -19,5 +19,6 @@
void hmp_info_name(Monitor *mon);
void hmp_info_version(Monitor *mon);
+void hmp_info_kvm(Monitor *mon);
#endif
diff --git a/monitor.c b/monitor.c
index 483ea71..dad4221 100644
--- a/monitor.c
+++ b/monitor.c
@@ -2430,31 +2430,6 @@ static void do_info_mtree(Monitor *mon)
mtree_info((fprintf_function)monitor_printf, mon);
}
-static void do_info_kvm_print(Monitor *mon, const QObject *data)
-{
-QDict *qdict;
-
-qdict = qobject_to_qdict(data);
-
-monitor_printf(mon, "kvm support: ");
-if (qdict_get_bool(qdict, "present")) {
-monitor_printf(mon, "%s\n", qdict_get_bool(qdict, "enabled") ?
-"enabled" : "disabled");
-} else {
-monitor_printf(mon, "not compiled\n");
-}
-}
-
-static void do_info_kvm(Monitor *mon, QObject **ret_data)
-{
-#ifdef CONFIG_KVM
-*ret_data = qobject_from_jsonf("{ 'enabled': %i, 'present': true }",
- kvm_enabled());
-#else
-*ret_data = qobject_from_jsonf("{ 'enabled': false, 'present': false }");
-#endif
-}
-
static void do_info_numa(Monitor *mon)
{
int i;
@@ -2955,8 +2930,7 @@ static const mon_cmd_t info_cmds[] = {
.args_type = "",
.params = "",
.help = "show KVM information",
-.user_print = do_info_kvm_print,
-.mhandler.info_new = do_info_kvm,
+.mhandler.info = hmp_info_kvm,
},
{
.name = "numa",
@@ -3188,14 +3162,6 @@ static const mon_cmd_t qmp_query_cmds[] = {
.mhandler.info_new = do_pci_info,
},
{
-.name = "kvm",
-.args_type = "",
-.params = "",
-.help = "show KVM information",
-.user_print = do_info_kvm_print,
-.mhandler.info_new = do_info_kvm,
-},
-{
.name = "status",
.args_type = "",
.params = "",
diff --git a/qapi-schema.json b/qapi-schema.json
index 3c0ac4e..641f12d 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -60,3 +60,28 @@
# Since: 0.14.0
##
{ 'command': 'query-version', 'returns': 'VersionInfo' }
+
+##
+# @KvmInfo:
+#
+# Information about support for KVM acceleration
+#
+# @enabled: true if KVM acceleration is active
+#
+# @present: true if KVM acceleration is built into this executable
+#
+# Since: 0.14.0
+##
+{ 'type': 'KvmInfo', 'data': {'enabled': 'bool', 'present': 'bool'} }
+
+##
+# @query-kvm:
+#
+# Returns information about KVM acceleration
+#
+# Returns: @KvmInfo
+#
+# Since: 0.14.0
+##
+{ 'command': 'query-kvm', 'returns': 'KvmInfo' }
+
diff --git a/qmp-commands.hx b/qmp-commands.hx
index 9f067ea..9f9751d 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -1570,6 +1570,12 @@ Example:
EQMP
+{
+.name = "query-kvm",
+.args_type = "",
+.mhandler.cmd_new = qmp_marshal_input_query_kvm,
+},
+
SQMP
query-status
diff --git a/qmp.c b/qmp.c
index f978ea4..8f7f666 100644
--- a/qmp.c
+++ b/qmp.c
@@ -14,6 +14,8 @@
#include "qemu-common.h"
#include "sysemu.h"
#include "qmp-commands.h"
+#include "kvm.h"
+#include "arch_init.h"
NameInfo *qmp_query_name(Error **errp)
{
@@ -42,3 +44,14 @@ VersionInfo *qmp_query_version(Error **err)
return info;
}
+
+KvmInfo *qmp_query_kvm(Error **errp)
+{
+KvmInfo *info = g_malloc0(sizeof(*info));
+
+info->enabled = kvm_enabled();
+info->present = kvm_available();
+
+return info;
+}
+
--
1.7.7.rc0.72.g4b5ea
[Qemu-devel] [PATCH 20/26] qapi: Convert query-uuid
Signed-off-by: Anthony Liguori
Signed-off-by: Luiz Capitulino
---
hmp.c|8
hmp.h|1 +
monitor.c| 28 +---
qapi-schema.json | 24
qmp-commands.hx |6 ++
qmp.c| 15 +++
6 files changed, 55 insertions(+), 27 deletions(-)
diff --git a/hmp.c b/hmp.c
index 30925b0..25302f5 100644
--- a/hmp.c
+++ b/hmp.c
@@ -72,3 +72,11 @@ void hmp_info_status(Monitor *mon)
qapi_free_StatusInfo(info);
}
+void hmp_info_uuid(Monitor *mon)
+{
+UuidInfo *info;
+
+info = qmp_query_uuid(NULL);
+monitor_printf(mon, "%s\n", info->UUID);
+qapi_free_UuidInfo(info);
+}
diff --git a/hmp.h b/hmp.h
index df4242f..49a5971 100644
--- a/hmp.h
+++ b/hmp.h
@@ -21,5 +21,6 @@ void hmp_info_name(Monitor *mon);
void hmp_info_version(Monitor *mon);
void hmp_info_kvm(Monitor *mon);
void hmp_info_status(Monitor *mon);
+void hmp_info_uuid(Monitor *mon);
#endif
diff --git a/monitor.c b/monitor.c
index e68f2e2..4e9ecdf 100644
--- a/monitor.c
+++ b/monitor.c
@@ -766,23 +766,6 @@ static void do_info_commands(Monitor *mon, QObject
**ret_data)
*ret_data = QOBJECT(cmd_list);
}
-static void do_info_uuid_print(Monitor *mon, const QObject *data)
-{
-monitor_printf(mon, "%s\n", qdict_get_str(qobject_to_qdict(data), "UUID"));
-}
-
-static void do_info_uuid(Monitor *mon, QObject **ret_data)
-{
-char uuid[64];
-
-snprintf(uuid, sizeof(uuid), UUID_FMT, qemu_uuid[0], qemu_uuid[1],
- qemu_uuid[2], qemu_uuid[3], qemu_uuid[4], qemu_uuid[5],
- qemu_uuid[6], qemu_uuid[7], qemu_uuid[8], qemu_uuid[9],
- qemu_uuid[10], qemu_uuid[11], qemu_uuid[12], qemu_uuid[13],
- qemu_uuid[14], qemu_uuid[15]);
-*ret_data = qobject_from_jsonf("{ 'UUID': %s }", uuid);
-}
-
/* get the current CPU defined by the user */
static int mon_set_cpu(int cpu_index)
{
@@ -2996,8 +2979,7 @@ static const mon_cmd_t info_cmds[] = {
.args_type = "",
.params = "",
.help = "show the current VM UUID",
-.user_print = do_info_uuid_print,
-.mhandler.info_new = do_info_uuid,
+.mhandler.info = hmp_info_uuid,
},
#if defined(TARGET_PPC)
{
@@ -3157,14 +3139,6 @@ static const mon_cmd_t qmp_query_cmds[] = {
},
#endif
{
-.name = "uuid",
-.args_type = "",
-.params = "",
-.help = "show the current VM UUID",
-.user_print = do_info_uuid_print,
-.mhandler.info_new = do_info_uuid,
-},
-{
.name = "migrate",
.args_type = "",
.params = "",
diff --git a/qapi-schema.json b/qapi-schema.json
index 0e59c71..b717491 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -152,3 +152,27 @@
##
{ 'command': 'query-status', 'returns': 'StatusInfo' }
+##
+# @UuidInfo:
+#
+# Guest UUID information.
+#
+# @UUID: the UUID of the guest
+#
+# Since: 0.14.0
+#
+# Notes: If no UUID was specified for the guest, a null UUID is returned.
+##
+{ 'type': 'UuidInfo', 'data': {'UUID': 'str'} }
+
+##
+# @query-uuid:
+#
+# Query the guest UUID information.
+#
+# Returns: The @UuidInfo for the guest
+#
+# Since 0.14.0
+##
+{ 'command': 'query-uuid', 'returns': 'UuidInfo' }
+
diff --git a/qmp-commands.hx b/qmp-commands.hx
index afa95bd..4fef25f 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -1821,6 +1821,12 @@ Example:
EQMP
+{
+.name = "query-uuid",
+.args_type = "",
+.mhandler.cmd_new = qmp_marshal_input_query_uuid,
+},
+
SQMP
query-migrate
-
diff --git a/qmp.c b/qmp.c
index 8f7f666..58337c7 100644
--- a/qmp.c
+++ b/qmp.c
@@ -55,3 +55,18 @@ KvmInfo *qmp_query_kvm(Error **errp)
return info;
}
+UuidInfo *qmp_query_uuid(Error **errp)
+{
+UuidInfo *info = g_malloc0(sizeof(*info));
+char uuid[64];
+
+snprintf(uuid, sizeof(uuid), UUID_FMT, qemu_uuid[0], qemu_uuid[1],
+ qemu_uuid[2], qemu_uuid[3], qemu_uuid[4], qemu_uuid[5],
+ qemu_uuid[6], qemu_uuid[7], qemu_uuid[8], qemu_uuid[9],
+ qemu_uuid[10], qemu_uuid[11], qemu_uuid[12], qemu_uuid[13],
+ qemu_uuid[14], qemu_uuid[15]);
+
+info->UUID = g_strdup(uuid);
+return info;
+}
+
--
1.7.7.rc0.72.g4b5ea
[Qemu-devel] [PATCH 26/26] qapi: Convert system_powerdown
Signed-off-by: Anthony Liguori
Signed-off-by: Luiz Capitulino
---
hmp-commands.hx |3 +--
hmp.c|5 +
hmp.h|1 +
qapi-schema.json | 14 ++
qmp.c|5 +
5 files changed, 26 insertions(+), 2 deletions(-)
diff --git a/hmp-commands.hx b/hmp-commands.hx
index b2f5cd1..07b493c 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -492,8 +492,7 @@ ETEXI
.args_type = "",
.params = "",
.help = "send system power down event",
-.user_print = monitor_user_noop,
-.mhandler.cmd_new = do_system_powerdown,
+.mhandler.cmd = hmp_system_powerdown,
},
STEXI
diff --git a/hmp.c b/hmp.c
index 24f30bc..34416fc 100644
--- a/hmp.c
+++ b/hmp.c
@@ -109,3 +109,8 @@ void hmp_system_reset(Monitor *mon, const QDict *qdict)
{
qmp_system_reset(NULL);
}
+
+void hmp_system_powerdown(Monitor *mon, const QDict *qdict)
+{
+qmp_system_powerdown(NULL);
+}
diff --git a/hmp.h b/hmp.h
index a49a6e6..92433cf 100644
--- a/hmp.h
+++ b/hmp.h
@@ -26,5 +26,6 @@ void hmp_info_chardev(Monitor *mon);
void hmp_quit(Monitor *mon, const QDict *qdict);
void hmp_stop(Monitor *mon, const QDict *qdict);
void hmp_system_reset(Monitor *mon, const QDict *qdict);
+void hmp_system_powerdown(Monitor *mon, const QDict *qdict);
#endif
diff --git a/qapi-schema.json b/qapi-schema.json
index 02de4b5..5922c4a 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -257,3 +257,17 @@
# Since: 0.14.0
##
{ 'command': 'system_reset' }
+
+##
+# @system_powerdown:
+#
+# Requests that a guest perform a powerdown operation.
+#
+# Since: 0.14.0
+#
+# Notes: A guest may or may not respond to this command. This command
+#returning does not indicate that a guest has accepted the request or
+#that it has shut down. Many guests will respond to this command by
+#prompting the user in some way.
+##
+{ 'command': 'system_powerdown' }
diff --git a/qmp.c b/qmp.c
index 51d9383..bf58b05 100644
--- a/qmp.c
+++ b/qmp.c
@@ -85,3 +85,8 @@ void qmp_system_reset(Error **errp)
{
qemu_system_reset_request();
}
+
+void qmp_system_powerdown(Error **erp)
+{
+qemu_system_powerdown_request();
+}
--
1.7.7.rc0.72.g4b5ea
[Qemu-devel] [PATCH 13/26] qapi: convert query-name
From: Anthony Liguori
A simple example conversion 'info name'. This also adds the new files for
QMP and HMP.
Signed-off-by: Anthony Liguori
Signed-off-by: Luiz Capitulino
---
Makefile.objs|1 +
hmp.c| 26 ++
hmp.h| 22 ++
monitor.c| 31 +++
qapi-schema.json | 22 ++
qmp-commands.hx |6 ++
qmp.c| 28
7 files changed, 108 insertions(+), 28 deletions(-)
create mode 100644 hmp.c
create mode 100644 hmp.h
create mode 100644 qmp.c
diff --git a/Makefile.objs b/Makefile.objs
index 587ec36..a669c62 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -407,6 +407,7 @@ qapi-nested-y += qmp-registry.o qmp-dispatch.o
qapi-obj-y = $(addprefix qapi/, $(qapi-nested-y))
common-obj-y += qmp-marshal.o qapi-visit.o qapi-types.o $(qapi-obj-y)
+common-obj-y += qmp.o hmp.o
##
# guest agent
diff --git a/hmp.c b/hmp.c
new file mode 100644
index 000..47e1ff7
--- /dev/null
+++ b/hmp.c
@@ -0,0 +1,26 @@
+/*
+ * Human Monitor Interface
+ *
+ * Copyright IBM, Corp. 2011
+ *
+ * Authors:
+ * Anthony Liguori
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2. See
+ * the COPYING file in the top-level directory.
+ *
+ */
+
+#include "hmp.h"
+#include "qmp-commands.h"
+
+void hmp_info_name(Monitor *mon)
+{
+NameInfo *info;
+
+info = qmp_query_name(NULL);
+if (info->has_name) {
+monitor_printf(mon, "%s\n", info->name);
+}
+qapi_free_NameInfo(info);
+}
diff --git a/hmp.h b/hmp.h
new file mode 100644
index 000..5fe73f1
--- /dev/null
+++ b/hmp.h
@@ -0,0 +1,22 @@
+/*
+ * Human Monitor Interface
+ *
+ * Copyright IBM, Corp. 2011
+ *
+ * Authors:
+ * Anthony Liguori
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2. See
+ * the COPYING file in the top-level directory.
+ *
+ */
+
+#ifndef HMP_H
+#define HMP_H
+
+#include "qemu-common.h"
+#include "qapi-types.h"
+
+void hmp_info_name(Monitor *mon);
+
+#endif
diff --git a/monitor.c b/monitor.c
index e267d58..83b4fa7 100644
--- a/monitor.c
+++ b/monitor.c
@@ -64,6 +64,8 @@
#include "trace/control.h"
#include "ui/qemu-spice.h"
#include "memory.h"
+#include "qmp-commands.h"
+#include "hmp.h"
//#define DEBUG
//#define DEBUG_COMPLETION
@@ -760,24 +762,6 @@ static void do_info_version(Monitor *mon, QObject
**ret_data)
'micro': %d }, 'package': %s }", major, minor, micro, QEMU_PKGVERSION);
}
-static void do_info_name_print(Monitor *mon, const QObject *data)
-{
-QDict *qdict;
-
-qdict = qobject_to_qdict(data);
-if (qdict_size(qdict) == 0) {
-return;
-}
-
-monitor_printf(mon, "%s\n", qdict_get_str(qdict, "name"));
-}
-
-static void do_info_name(Monitor *mon, QObject **ret_data)
-{
-*ret_data = qemu_name ? qobject_from_jsonf("{'name': %s }", qemu_name) :
-qobject_from_jsonf("{}");
-}
-
static QObject *get_cmd_dict(const char *name)
{
const char *p;
@@ -3094,8 +3078,7 @@ static const mon_cmd_t info_cmds[] = {
.args_type = "",
.params = "",
.help = "show the current VM name",
-.user_print = do_info_name_print,
-.mhandler.info_new = do_info_name,
+.mhandler.info = hmp_info_name,
},
{
.name = "uuid",
@@ -3287,14 +3270,6 @@ static const mon_cmd_t qmp_query_cmds[] = {
},
#endif
{
-.name = "name",
-.args_type = "",
-.params = "",
-.help = "show the current VM name",
-.user_print = do_info_name_print,
-.mhandler.info_new = do_info_name,
-},
-{
.name = "uuid",
.args_type = "",
.params = "",
diff --git a/qapi-schema.json b/qapi-schema.json
index 7fcefdb..3585324 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -1,3 +1,25 @@
# -*- Mode: Python -*-
#
# QAPI Schema
+
+##
+# @NameInfo:
+#
+# Guest name information.
+#
+# @name: #optional The name of the guest
+#
+# Since 0.14.0
+##
+{ 'type': 'NameInfo', 'data': {'*name': 'str'} }
+
+##
+# @query-name:
+#
+# Return the name information of a guest.
+#
+# Returns: @NameInfo of the guest
+#
+# Since 0.14.0
+##
+{ 'command': 'query-name', 'returns': 'NameInfo' }
diff --git a/qmp-commands.hx b/qmp-commands.hx
index d83bce5..7b3839e 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -1780,6 +1780,12 @@ Example:
EQMP
+{
+.name = "query-name",
+.args_type = "",
+.mhandler.cmd_new = qmp_marshal_input_query_name,
+},
+
SQMP
query-uuid
--
diff --git a/qmp.c b/qmp.c
new file mode 100644
index 000..8aa9c66
--- /dev/null
+++ b/qmp.c
@@ -0,0 +1,28 @@
+/*
+ * QEMU Management Protocol
+ *
+ * Copyright IBM, Corp. 2011
+ *
+ * Authors:
+ * Anthony Liguori
+ *
+ * This w
[Qemu-devel] [PATCH 11/26] qapi: Don't use c_var() on enum strings
Otherwise if we have something like 'foo-bar' in the schema,
it will be generated as 'foo_bar' in the string lookup table.
c_var() is good for C variables, but not for enum strings.
Signed-off-by: Luiz Capitulino
---
scripts/qapi-types.py |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py
index 4797a70..3bacc0c 100644
--- a/scripts/qapi-types.py
+++ b/scripts/qapi-types.py
@@ -70,7 +70,7 @@ const char *%(name)s_lookup[] = {
ret += mcgen('''
"%(value)s",
''',
- value=c_var(value).lower())
+ value=value.lower())
ret += mcgen('''
NULL,
--
1.7.7.rc0.72.g4b5ea
Re: [Qemu-devel] [PATCH] runstate: do not discard runstate changes when paused
On Tue, 4 Oct 2011 14:04:45 +0200
Paolo Bonzini wrote:
> Trying to migrate a paused machine fails. The reason is that
> the RSTATE_PRE_MIGRATE is reached with vm_stop, and this
> transition is eaten when the vm is already paused. This patch
> fixes the problem by always going through runstate_set and
> always notifying the new state.
Let's start over, this time CC'ing Jan, Anthony and Avi.
Basically, what Paolo is describing above is this:
1. The user issues the stop command. vm_stop() will set the state to
RSTATE_PAUSED
2. The user starts a migration. migrate_fd_put_ready() will call
vm_stop(RSTATE_PRE_MIGRATE). However, the VM is already stopped
so vm_stop() just returns (IOW, the state is still RSTATE_PAUSED)
3. The migration process completes. migrate_fd_put_ready() will now
call runstate_set(RSTATE_POST_MIGRATE), which in turn causes the
transition RSTATE_PAUSED -> RSTATE_POST_MIGRATE, which is invalid
and the world of qemu ends
Now, we have three options to fix this but I don't know which one to choose:
1. We could just add the transition RSTATE_PAUSED -> RSTATE_POST_MIGRATE
as valid. Not sure this is a good thing to do though, as it seems a silly
workaround for the fact that the transition to RSTATE_PRE_MIGRATE has
never occurred
2. This patch makes vm_stop() do the state transition even if the VM
is already stopped. Seems good enough, except that I fear two things.
First, today we know that vm_stop() is a no-op if the VM is already
stopped, so there's a semantic change that could turn out to be trap.
Second, I also fear people using vm_stop() as a way to change the
VM status, just like runstate_set() (which can also become an horrible
trap)
3. Avi suggested we should keep a reference count, so that states are
not discarded:
http://lists.gnu.org/archive/html/qemu-devel/2011-08/msg00595.html
That solution seemed to be the perfect one, except for one important
detail: how should we implement vm_start() (and thus 'cont')?
In order to maintain how we behave with the external world, the only
option is that vm_start() will set the stop count to 0. Ie, doesn't
matter if we have stopped the VM 500 times at some point, a vm_start()
call will discard all stored states.
Not sure if that's what you expected, but the first time I read Avi's
idea I had the impression that it would be a good idea that vm_start()
decremented the ref count only once, ie. vm_stop() and vm_start() calls
have to match.
>
> Signed-off-by: Paolo Bonzini
> ---
> cpus.c |2 ++
> 1 files changed, 2 insertions(+), 0 deletions(-)
>
> diff --git a/cpus.c b/cpus.c
> index 8978779..eab8ff6 100644
> --- a/cpus.c
> +++ b/cpus.c
> @@ -128,6 +128,8 @@ static void do_vm_stop(RunState state)
> qemu_aio_flush();
> bdrv_flush_all();
> monitor_protocol_event(QEVENT_STOP, NULL);
> +} else {
> +runstate_set(state);
> }
> }
>
[Qemu-devel] [PATCH 21/26] qapi: Convert query-chardev
Reviewed-by: Michael Roth
Tested-by: Michael Roth
Signed-off-by: Anthony Liguori
Signed-off-by: Luiz Capitulino
---
hmp.c| 13 +
hmp.h|1 +
monitor.c| 11 +--
qapi-schema.json | 26 ++
qemu-char.c | 35 +++
qmp-commands.hx |6 ++
6 files changed, 58 insertions(+), 34 deletions(-)
diff --git a/hmp.c b/hmp.c
index 25302f5..30c6cc9 100644
--- a/hmp.c
+++ b/hmp.c
@@ -80,3 +80,16 @@ void hmp_info_uuid(Monitor *mon)
monitor_printf(mon, "%s\n", info->UUID);
qapi_free_UuidInfo(info);
}
+
+void hmp_info_chardev(Monitor *mon)
+{
+ChardevInfoList *char_info, *info;
+
+char_info = qmp_query_chardev(NULL);
+for (info = char_info; info; info = info->next) {
+monitor_printf(mon, "%s: filename=%s\n", info->value->label,
+ info->value->filename);
+}
+
+qapi_free_ChardevInfoList(char_info);
+}
diff --git a/hmp.h b/hmp.h
index 49a5971..7388f9a 100644
--- a/hmp.h
+++ b/hmp.h
@@ -22,5 +22,6 @@ void hmp_info_version(Monitor *mon);
void hmp_info_kvm(Monitor *mon);
void hmp_info_status(Monitor *mon);
void hmp_info_uuid(Monitor *mon);
+void hmp_info_chardev(Monitor *mon);
#endif
diff --git a/monitor.c b/monitor.c
index 4e9ecdf..86a0dad 100644
--- a/monitor.c
+++ b/monitor.c
@@ -2783,8 +2783,7 @@ static const mon_cmd_t info_cmds[] = {
.args_type = "",
.params = "",
.help = "show the character devices",
-.user_print = qemu_chr_info_print,
-.mhandler.info_new = qemu_chr_info,
+.mhandler.info = hmp_info_chardev,
},
{
.name = "block",
@@ -3073,14 +3072,6 @@ static const mon_cmd_t qmp_query_cmds[] = {
.mhandler.info_new = do_info_commands,
},
{
-.name = "chardev",
-.args_type = "",
-.params = "",
-.help = "show the character devices",
-.user_print = qemu_chr_info_print,
-.mhandler.info_new = qemu_chr_info,
-},
-{
.name = "block",
.args_type = "",
.params = "",
diff --git a/qapi-schema.json b/qapi-schema.json
index b717491..cf55504 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -176,3 +176,29 @@
##
{ 'command': 'query-uuid', 'returns': 'UuidInfo' }
+##
+# @ChardevInfo:
+#
+# Information about a character device.
+#
+# @label: the label of the character device
+#
+# @filename: the filename of the character device
+#
+# Notes: @filename is encoded using the QEMU command line character device
+#encoding. See the QEMU man page for details.
+#
+# Since: 0.14.0
+##
+{ 'type': 'ChardevInfo', 'data': {'label': 'str', 'filename': 'str'} }
+
+##
+# @query-chardev:
+#
+# Returns information about current character devices.
+#
+# Returns: a list of @ChardevInfo
+#
+# Since: 0.14.0
+##
+{ 'command': 'query-chardev', 'returns': ['ChardevInfo'] }
diff --git a/qemu-char.c b/qemu-char.c
index 09d2309..8bdbcfd 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -31,7 +31,7 @@
#include "hw/usb.h"
#include "hw/baum.h"
#include "hw/msmouse.h"
-#include "qemu-objects.h"
+#include "qmp-commands.h"
#include
#include
@@ -2650,35 +2650,22 @@ void qemu_chr_delete(CharDriverState *chr)
g_free(chr);
}
-static void qemu_chr_qlist_iter(QObject *obj, void *opaque)
+ChardevInfoList *qmp_query_chardev(Error **errp)
{
-QDict *chr_dict;
-Monitor *mon = opaque;
-
-chr_dict = qobject_to_qdict(obj);
-monitor_printf(mon, "%s: filename=%s\n", qdict_get_str(chr_dict, "label"),
- qdict_get_str(chr_dict, "filename"));
-}
-
-void qemu_chr_info_print(Monitor *mon, const QObject *ret_data)
-{
-qlist_iter(qobject_to_qlist(ret_data), qemu_chr_qlist_iter, mon);
-}
-
-void qemu_chr_info(Monitor *mon, QObject **ret_data)
-{
-QList *chr_list;
+ChardevInfoList *chr_list = NULL;
CharDriverState *chr;
-chr_list = qlist_new();
-
QTAILQ_FOREACH(chr, &chardevs, next) {
-QObject *obj = qobject_from_jsonf("{ 'label': %s, 'filename': %s }",
- chr->label, chr->filename);
-qlist_append_obj(chr_list, obj);
+ChardevInfoList *info = g_malloc0(sizeof(*info));
+info->value = g_malloc0(sizeof(*info->value));
+info->value->label = g_strdup(chr->label);
+info->value->filename = g_strdup(chr->filename);
+
+info->next = chr_list;
+chr_list = info;
}
-*ret_data = QOBJECT(chr_list);
+return chr_list;
}
CharDriverState *qemu_chr_find(const char *name)
diff --git a/qmp-commands.hx b/qmp-commands.hx
index 4fef25f..dfc02af 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -1120,6 +1120,12 @@ Example:
EQMP
+{
+.name = "query-chardev",
+.args_type = "",
+.mhandler.cmd_new = qmp_marshal_input_quer
[Qemu-devel] [PATCH 10/26] qapi: modify visitor code generation for list iteration
From: Michael Roth
Modify logic such that we never assign values to the list head argument
to progress through the list on subsequent iterations, instead rely only
on having our return value passed back in as an argument on the next
call. Also update QMP I/O visitors and test cases accordingly, and add a
missing test case for QmpOutputVisitor.
Reviewed-by: Anthony Liguori
Signed-off-by: Michael Roth
Signed-off-by: Luiz Capitulino
---
qapi/qmp-input-visitor.c |4 +-
qapi/qmp-output-visitor.c | 20 +++---
scripts/qapi-visit.py |4 +-
test-visitor.c| 48 +---
4 files changed, 60 insertions(+), 16 deletions(-)
diff --git a/qapi/qmp-input-visitor.c b/qapi/qmp-input-visitor.c
index fcf8bf9..8cbc0ab 100644
--- a/qapi/qmp-input-visitor.c
+++ b/qapi/qmp-input-visitor.c
@@ -144,8 +144,6 @@ static GenericList *qmp_input_next_list(Visitor *v,
GenericList **list,
}
(*list)->next = entry;
}
-*list = entry;
-
return entry;
}
@@ -240,9 +238,11 @@ static void qmp_input_type_enum(Visitor *v, int *obj,
const char *strings[],
if (strings[value] == NULL) {
error_set(errp, QERR_INVALID_PARAMETER, name ? name : "null");
+g_free(enum_str);
return;
}
+g_free(enum_str);
*obj = value;
}
diff --git a/qapi/qmp-output-visitor.c b/qapi/qmp-output-visitor.c
index 4419a31..d67724e 100644
--- a/qapi/qmp-output-visitor.c
+++ b/qapi/qmp-output-visitor.c
@@ -20,6 +20,7 @@
typedef struct QStackEntry
{
QObject *value;
+bool is_list_head;
QTAILQ_ENTRY(QStackEntry) node;
} QStackEntry;
@@ -45,6 +46,9 @@ static void qmp_output_push_obj(QmpOutputVisitor *qov,
QObject *value)
QStackEntry *e = g_malloc0(sizeof(*e));
e->value = value;
+if (qobject_type(e->value) == QTYPE_QLIST) {
+e->is_list_head = true;
+}
QTAILQ_INSERT_HEAD(&qov->stack, e, node);
}
@@ -122,12 +126,20 @@ static void qmp_output_start_list(Visitor *v, const char
*name, Error **errp)
qmp_output_push(qov, list);
}
-static GenericList *qmp_output_next_list(Visitor *v, GenericList **list,
+static GenericList *qmp_output_next_list(Visitor *v, GenericList **listp,
Error **errp)
{
-GenericList *retval = *list;
-*list = retval->next;
-return retval;
+GenericList *list = *listp;
+QmpOutputVisitor *qov = to_qov(v);
+QStackEntry *e = QTAILQ_FIRST(&qov->stack);
+
+assert(e);
+if (e->is_list_head) {
+e->is_list_head = false;
+return list;
+}
+
+return list ? list->next : NULL;
}
static void qmp_output_end_list(Visitor *v, Error **errp)
diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py
index 252230e..62de83d 100644
--- a/scripts/qapi-visit.py
+++ b/scripts/qapi-visit.py
@@ -79,11 +79,11 @@ def generate_visit_list(name, members):
void visit_type_%(name)sList(Visitor *m, %(name)sList ** obj, const char
*name, Error **errp)
{
-GenericList *i;
+GenericList *i, **head = (GenericList **)obj;
visit_start_list(m, name, errp);
-for (i = visit_next_list(m, (GenericList **)obj, errp); i; i =
visit_next_list(m, &i, errp)) {
+for (*head = i = visit_next_list(m, head, errp); i; i = visit_next_list(m,
&i, errp)) {
%(name)sList *native_i = (%(name)sList *)i;
visit_type_%(name)s(m, &native_i->value, NULL, errp);
}
diff --git a/test-visitor.c b/test-visitor.c
index b7717de..847ce14 100644
--- a/test-visitor.c
+++ b/test-visitor.c
@@ -27,11 +27,11 @@ static void visit_type_TestStruct(Visitor *v, TestStruct
**obj, const char *name
static void visit_type_TestStructList(Visitor *m, TestStructList ** obj, const
char *name, Error **errp)
{
-GenericList *i;
+GenericList *i, **head = (GenericList **)obj;
visit_start_list(m, name, errp);
-for (i = visit_next_list(m, (GenericList **)obj, errp); i; i =
visit_next_list(m, &i, errp)) {
+for (*head = i = visit_next_list(m, head, errp); i; i = visit_next_list(m,
&i, errp)) {
TestStructList *native_i = (TestStructList *)i;
visit_type_TestStruct(m, &native_i->value, NULL, errp);
}
@@ -50,6 +50,8 @@ static void test_visitor_core(void)
TestStructList *lts = NULL;
Error *err = NULL;
QObject *obj;
+QList *qlist;
+QDict *qdict;
QString *str;
int64_t value = 0;
@@ -96,7 +98,9 @@ static void test_visitor_core(void)
g_assert(pts->y == 84);
qobject_decref(obj);
+g_free(pts);
+/* test list input visitor */
obj = qobject_from_json("[{'x': 42, 'y': 84}, {'x': 12, 'y': 24}]");
mi = qmp_input_visitor_new(obj);
v = qmp_input_get_visitor(mi);
@@ -110,14 +114,41 @@ static void test_visitor_core(void)
g_assert(lts->value->x == 42);
g_assert(lts->value->y == 84);
-lts = lts->next;
-g_assert(lts != NULL);
-g_assert(lts->value->x == 12);
-g_assert(l
[Qemu-devel] [PATCH 05/26] qapi: fixup command generation for functions that return list types
From: Anthony Liguori
Signed-off-by: Anthony Liguori
Signed-off-by: Luiz Capitulino
---
scripts/qapi-commands.py | 23 +++
1 files changed, 15 insertions(+), 8 deletions(-)
diff --git a/scripts/qapi-commands.py b/scripts/qapi-commands.py
index 2776804..c947ba4 100644
--- a/scripts/qapi-commands.py
+++ b/scripts/qapi-commands.py
@@ -17,12 +17,18 @@ import os
import getopt
import errno
+def type_visitor(name):
+if type(name) == list:
+return 'visit_type_%sList' % name[0]
+else:
+return 'visit_type_%s' % name
+
def generate_decl_enum(name, members, genlist=True):
return mcgen('''
-void visit_type_%(name)s(Visitor *m, %(name)s * obj, const char *name, Error
**errp);
+void %(visitor)s(Visitor *m, %(name)s * obj, const char *name, Error **errp);
''',
-name=name)
+ visitor=type_visitor(name))
def generate_command_decl(name, args, ret_type):
arglist=""
@@ -146,9 +152,10 @@ if (has_%(c_name)s) {
c_name=c_var(argname), name=argname)
push_indent()
ret += mcgen('''
-visit_type_%(argtype)s(v, &%(c_name)s, "%(name)s", errp);
+%(visitor)s(v, &%(c_name)s, "%(name)s", errp);
''',
- c_name=c_var(argname), name=argname, argtype=argtype)
+ c_name=c_var(argname), name=argname, argtype=argtype,
+ visitor=type_visitor(argtype))
if optional:
pop_indent()
ret += mcgen('''
@@ -179,18 +186,18 @@ static void qmp_marshal_output_%(c_name)s(%(c_ret_type)s
ret_in, QObject **ret_o
Visitor *v;
v = qmp_output_get_visitor(mo);
-visit_type_%(ret_type)s(v, &ret_in, "unused", errp);
+%(visitor)s(v, &ret_in, "unused", errp);
if (!error_is_set(errp)) {
*ret_out = qmp_output_get_qobject(mo);
}
qmp_output_visitor_cleanup(mo);
v = qapi_dealloc_get_visitor(md);
-visit_type_%(ret_type)s(v, &ret_in, "unused", errp);
+%(visitor)s(v, &ret_in, "unused", errp);
qapi_dealloc_visitor_cleanup(md);
}
''',
- c_ret_type=c_type(ret_type), c_name=c_var(name),
- ret_type=ret_type)
+c_ret_type=c_type(ret_type), c_name=c_var(name),
+visitor=type_visitor(ret_type))
return ret
--
1.7.7.rc0.72.g4b5ea
[Qemu-devel] [PATCH 08/26] qapi: add test cases for generated free functions
From: Michael Roth
Signed-off-by: Michael Roth
Signed-off-by: Luiz Capitulino
---
test-qmp-commands.c | 29 +
1 files changed, 29 insertions(+), 0 deletions(-)
diff --git a/test-qmp-commands.c b/test-qmp-commands.c
index f142cc6..fa5a7bd 100644
--- a/test-qmp-commands.c
+++ b/test-qmp-commands.c
@@ -98,6 +98,34 @@ static void test_dispatch_cmd_io(void)
QDECREF(req);
}
+/* test generated dealloc functions for generated types */
+static void test_dealloc_types(void)
+{
+UserDefOne *ud1test, *ud1a, *ud1b;
+UserDefOneList *ud1list;
+
+ud1test = g_malloc0(sizeof(UserDefOne));
+ud1test->integer = 42;
+ud1test->string = g_strdup("hi there 42");
+
+qapi_free_UserDefOne(ud1test);
+
+ud1a = g_malloc0(sizeof(UserDefOne));
+ud1a->integer = 43;
+ud1a->string = g_strdup("hi there 43");
+
+ud1b = g_malloc0(sizeof(UserDefOne));
+ud1b->integer = 44;
+ud1b->string = g_strdup("hi there 44");
+
+ud1list = g_malloc0(sizeof(UserDefOneList));
+ud1list->value = ud1a;
+ud1list->next = g_malloc0(sizeof(UserDefOneList));
+ud1list->next->value = ud1b;
+
+qapi_free_UserDefOneList(ud1list);
+}
+
int main(int argc, char **argv)
{
g_test_init(&argc, &argv, NULL);
@@ -105,6 +133,7 @@ int main(int argc, char **argv)
g_test_add_func("/0.15/dispatch_cmd", test_dispatch_cmd);
g_test_add_func("/0.15/dispatch_cmd_error", test_dispatch_cmd_error);
g_test_add_func("/0.15/dispatch_cmd_io", test_dispatch_cmd_io);
+g_test_add_func("/0.15/dealloc_types", test_dealloc_types);
module_call_init(MODULE_INIT_QAPI);
g_test_run();
--
1.7.7.rc0.72.g4b5ea
Re: [Qemu-devel] segfault on current HEAD, qemu-system-arm
On 2 October 2011 19:44, Blue Swirl wrote:
> Bah, bug in bccd9ec5f098668576342c83d90d6d6833d61d33,
> target-arm/op_helper.c missed this change unlike all other targets:
> diff --git a/target-arm/op_helper.c b/target-arm/op_helper.c
> index ab9c923..1892b35 100644
> --- a/target-arm/op_helper.c
> +++ b/target-arm/op_helper.c
> @@ -84,6 +84,7 @@ void tlb_fill(CPUState *env1, target_ulong addr, int
> is_write, int mmu_idx,
> int ret;
>
> saved_env = env;
> + env = env1;
> ret = cpu_arm_handle_mmu_fault(env, addr, is_write, mmu_idx);
> if (unlikely(ret)) {
> if (retaddr) {
>
This fixes the segfault for me.
Reviewed-by: Peter Maydell
-- PMM
[Qemu-devel] [PATCH 12/26] qapi: Automatically generate a _MAX value for enums
It's the last value in the enum and is very useful for the C
implementation.
Signed-off-by: Luiz Capitulino
---
scripts/qapi-types.py |5 -
1 files changed, 4 insertions(+), 1 deletions(-)
diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py
index 3bacc0c..f64d84c 100644
--- a/scripts/qapi-types.py
+++ b/scripts/qapi-types.py
@@ -91,8 +91,11 @@ typedef enum %(name)s
''',
name=name)
+# append automatically generated _MAX value
+enum_values = values + [ 'MAX' ]
+
i = 0
-for value in values:
+for value in enum_values:
enum_decl += mcgen('''
%(abbrev)s_%(value)s = %(i)d,
''',
--
1.7.7.rc0.72.g4b5ea
[Qemu-devel] [PATCH 09/26] qapi: dealloc visitor, support freeing of nested lists
From: Michael Roth
Previously our logic for keeping track of when we're visiting the head
of a list was done via a global bool. This can be overwritten if dealing
with nested lists, so use stack entries to track this instead.
Reviewed-by: Anthony Liguori
Signed-off-by: Michael Roth
Signed-off-by: Luiz Capitulino
---
qapi/qapi-dealloc-visitor.c | 28 +---
1 files changed, 21 insertions(+), 7 deletions(-)
diff --git a/qapi/qapi-dealloc-visitor.c b/qapi/qapi-dealloc-visitor.c
index 6b586ad..a154523 100644
--- a/qapi/qapi-dealloc-visitor.c
+++ b/qapi/qapi-dealloc-visitor.c
@@ -19,6 +19,7 @@
typedef struct StackEntry
{
void *value;
+bool is_list_head;
QTAILQ_ENTRY(StackEntry) node;
} StackEntry;
@@ -39,6 +40,11 @@ static void qapi_dealloc_push(QapiDeallocVisitor *qov, void
*value)
StackEntry *e = g_malloc0(sizeof(*e));
e->value = value;
+
+/* see if we're just pushing a list head tracker */
+if (value == NULL) {
+e->is_list_head = true;
+}
QTAILQ_INSERT_HEAD(&qov->stack, e, node);
}
@@ -72,7 +78,7 @@ static void qapi_dealloc_end_struct(Visitor *v, Error **errp)
static void qapi_dealloc_start_list(Visitor *v, const char *name, Error **errp)
{
QapiDeallocVisitor *qov = to_qov(v);
-qov->is_list_head = true;
+qapi_dealloc_push(qov, NULL);
}
static GenericList *qapi_dealloc_next_list(Visitor *v, GenericList **listp,
@@ -80,19 +86,27 @@ static GenericList *qapi_dealloc_next_list(Visitor *v,
GenericList **listp,
{
GenericList *list = *listp;
QapiDeallocVisitor *qov = to_qov(v);
+StackEntry *e = QTAILQ_FIRST(&qov->stack);
-if (!qov->is_list_head) {
-*listp = list->next;
-g_free(list);
-return *listp;
+if (e && e->is_list_head) {
+e->is_list_head = false;
+return list;
}
-qov->is_list_head = false;
-return list;
+if (list) {
+list = list->next;
+g_free(*listp);
+return list;
+}
+
+return NULL;
}
static void qapi_dealloc_end_list(Visitor *v, Error **errp)
{
+QapiDeallocVisitor *qov = to_qov(v);
+void *obj = qapi_dealloc_pop(qov);
+assert(obj == NULL); /* should've been list head tracker with no payload */
}
static void qapi_dealloc_type_str(Visitor *v, char **obj, const char *name,
--
1.7.7.rc0.72.g4b5ea
[Qemu-devel] [PATCH 24/26] qapi: Convert stop
Signed-off-by: Anthony Liguori
Signed-off-by: Luiz Capitulino
---
hmp-commands.hx |3 +--
hmp.c|5 +
hmp.h|1 +
monitor.c|9 -
qapi-schema.json | 12
qmp-commands.hx |5 +
qmp.c|5 +
7 files changed, 25 insertions(+), 15 deletions(-)
diff --git a/hmp-commands.hx b/hmp-commands.hx
index 2163e6f..3ad1ce7 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -290,8 +290,7 @@ ETEXI
.args_type = "",
.params = "",
.help = "stop emulation",
-.user_print = monitor_user_noop,
-.mhandler.cmd_new = do_stop,
+.mhandler.cmd = hmp_stop,
},
STEXI
diff --git a/hmp.c b/hmp.c
index 7695dfc..efcb744 100644
--- a/hmp.c
+++ b/hmp.c
@@ -99,3 +99,8 @@ void hmp_quit(Monitor *mon, const QDict *qdict)
monitor_suspend(mon);
qmp_quit(NULL);
}
+
+void hmp_stop(Monitor *mon, const QDict *qdict)
+{
+qmp_stop(NULL);
+}
diff --git a/hmp.h b/hmp.h
index a3dfafd..cb21cce 100644
--- a/hmp.h
+++ b/hmp.h
@@ -24,5 +24,6 @@ void hmp_info_status(Monitor *mon);
void hmp_info_uuid(Monitor *mon);
void hmp_info_chardev(Monitor *mon);
void hmp_quit(Monitor *mon, const QDict *qdict);
+void hmp_stop(Monitor *mon, const QDict *qdict);
#endif
diff --git a/monitor.c b/monitor.c
index 6e13ef6..2a5230c 100644
--- a/monitor.c
+++ b/monitor.c
@@ -1212,15 +1212,6 @@ static void do_singlestep(Monitor *mon, const QDict
*qdict)
}
}
-/**
- * do_stop(): Stop VM execution
- */
-static int do_stop(Monitor *mon, const QDict *qdict, QObject **ret_data)
-{
-vm_stop(RUN_STATE_PAUSED);
-return 0;
-}
-
static void encrypted_bdrv_it(void *opaque, BlockDriverState *bs);
struct bdrv_iterate_context {
diff --git a/qapi-schema.json b/qapi-schema.json
index 3810463..cd05034 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -236,3 +236,15 @@
# Since: 0.14.0
##
{ 'command': 'quit' }
+
+##
+# @stop:
+#
+# Stop all guest VCPU execution.
+#
+# Since: 0.14.0
+#
+# Notes: This function will succeed even if the guest is already in the
stopped
+# state
+##
+{ 'command': 'stop' }
diff --git a/qmp-commands.hx b/qmp-commands.hx
index 151a5fa..2ccddee 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -178,10 +178,7 @@ EQMP
{
.name = "stop",
.args_type = "",
-.params = "",
-.help = "stop emulation",
-.user_print = monitor_user_noop,
-.mhandler.cmd_new = do_stop,
+.mhandler.cmd_new = qmp_marshal_input_stop,
},
SQMP
diff --git a/qmp.c b/qmp.c
index 1d380b6..6c46479 100644
--- a/qmp.c
+++ b/qmp.c
@@ -76,3 +76,8 @@ void qmp_quit(Error **err)
qemu_system_shutdown_request();
}
+void qmp_stop(Error **errp)
+{
+vm_stop(RUN_STATE_PAUSED);
+}
+
--
1.7.7.rc0.72.g4b5ea
[Qemu-devel] [PATCH 19/26] qapi: Convert query-status
Please, note that the RunState type as defined in sysemu.h and its
runstate_as_string() function are being dropped in favor of the
RunState type generated by the QAPI.
Signed-off-by: Anthony Liguori
Signed-off-by: Luiz Capitulino
---
hmp.c| 19 +++
hmp.h|1 +
monitor.c| 41 +
qapi-schema.json | 67 ++
qmp-commands.hx |6 +
sysemu.h | 19 +--
vl.c | 32 +-
7 files changed, 106 insertions(+), 79 deletions(-)
diff --git a/hmp.c b/hmp.c
index 94a7f74..30925b0 100644
--- a/hmp.c
+++ b/hmp.c
@@ -53,3 +53,22 @@ void hmp_info_kvm(Monitor *mon)
qapi_free_KvmInfo(info);
}
+void hmp_info_status(Monitor *mon)
+{
+StatusInfo *info;
+
+info = qmp_query_status(NULL);
+
+monitor_printf(mon, "VM status: %s%s",
+ info->running ? "running" : "paused",
+ info->singlestep ? " (single step mode)" : "");
+
+if (!info->running && info->status != RUN_STATE_PAUSED) {
+monitor_printf(mon, " (%s)", RunState_lookup[info->status]);
+}
+
+monitor_printf(mon, "\n");
+
+qapi_free_StatusInfo(info);
+}
+
diff --git a/hmp.h b/hmp.h
index a93ac1f..df4242f 100644
--- a/hmp.h
+++ b/hmp.h
@@ -20,5 +20,6 @@
void hmp_info_name(Monitor *mon);
void hmp_info_version(Monitor *mon);
void hmp_info_kvm(Monitor *mon);
+void hmp_info_status(Monitor *mon);
#endif
diff --git a/monitor.c b/monitor.c
index da729ce..e68f2e2 100644
--- a/monitor.c
+++ b/monitor.c
@@ -2550,36 +2550,6 @@ static int do_inject_nmi(Monitor *mon, const QDict
*qdict, QObject **ret_data)
}
#endif
-static void do_info_status_print(Monitor *mon, const QObject *data)
-{
-QDict *qdict;
-const char *status;
-
-qdict = qobject_to_qdict(data);
-
-monitor_printf(mon, "VM status: ");
-if (qdict_get_bool(qdict, "running")) {
-monitor_printf(mon, "running");
-if (qdict_get_bool(qdict, "singlestep")) {
-monitor_printf(mon, " (single step mode)");
-}
-} else {
-monitor_printf(mon, "paused");
-}
-
-status = qdict_get_str(qdict, "status");
-if (strcmp(status, "paused") && strcmp(status, "running")) {
-monitor_printf(mon, " (%s)", status);
-}
-
-monitor_printf(mon, "\n");
-}
-
-static void do_info_status(Monitor *mon, QObject **ret_data)
-{
-*ret_data = qobject_from_jsonf("{ 'running': %i, 'singlestep': %i,
'status': %s }", runstate_is_running(), singlestep, runstate_as_string());
-}
-
static qemu_acl *find_acl(Monitor *mon, const char *name)
{
qemu_acl *acl = qemu_acl_find(name);
@@ -2979,8 +2949,7 @@ static const mon_cmd_t info_cmds[] = {
.args_type = "",
.params = "",
.help = "show the current VM status (running|paused)",
-.user_print = do_info_status_print,
-.mhandler.info_new = do_info_status,
+.mhandler.info = hmp_info_status,
},
{
.name = "pcmcia",
@@ -3162,14 +3131,6 @@ static const mon_cmd_t qmp_query_cmds[] = {
.mhandler.info_new = do_pci_info,
},
{
-.name = "status",
-.args_type = "",
-.params = "",
-.help = "show the current VM status (running|paused)",
-.user_print = do_info_status_print,
-.mhandler.info_new = do_info_status,
-},
-{
.name = "mice",
.args_type = "",
.params = "",
diff --git a/qapi-schema.json b/qapi-schema.json
index 641f12d..0e59c71 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -85,3 +85,70 @@
##
{ 'command': 'query-kvm', 'returns': 'KvmInfo' }
+##
+# @RunState
+#
+# An enumation of VM run states.
+#
+# @debug: QEMU is running on a debugger
+#
+# @inmigrate: guest is paused waiting for an incoming migration
+#
+# @internal-error: An internal error that prevents further guest execution
+# has occurred
+#
+# @io-error: the last IOP has failed and the device is configured to pause
+# on I/O errors
+#
+# @paused: guest has been paused via the 'stop' command
+#
+# @postmigrate: guest is paused following a successful 'migrate'
+#
+# @prelaunch: QEMU was started with -S and guest has not started
+#
+# @finish-migrate: guest is paused to finish the migration process
+#
+# @restore-vm: guest is paused to restore VM state
+#
+# @running: guest is actively running
+#
+# @save-vm: guest is paused to save the VM state
+#
+# @shutdown: guest is shut down (and -no-shutdown is in use)
+#
+# @watchdog: the watchdog action is configured to pause and has been triggered
+##
+{ 'enum': 'RunState',
+ 'data': [ 'debug', 'inmigrate', 'internal-error', 'io-error', 'paused',
+'postmigrate', 'prelaunch', 'finish-migrate', 'restore-vm',
+'running', 'save-vm', 'shutdown', 'watchdog' ] }
+
+##
+# @StatusInfo:
+#
+# Information about VCPU run state
[Qemu-devel] [PATCH 07/26] qapi: generate qapi_free_* functions for *List types
From: Michael Roth
Reviewed-by: Anthony Liguori
Signed-off-by: Michael Roth
Signed-off-by: Luiz Capitulino
---
scripts/qapi-types.py |2 ++
1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py
index fc0f7af..4797a70 100644
--- a/scripts/qapi-types.py
+++ b/scripts/qapi-types.py
@@ -254,6 +254,8 @@ for expr in exprs:
ret = "\n"
if expr.has_key('type'):
ret += generate_struct(expr['type'], "", expr['data']) + "\n"
+ret += generate_type_cleanup_decl(expr['type'] + "List")
+fdef.write(generate_type_cleanup(expr['type'] + "List") + "\n")
ret += generate_type_cleanup_decl(expr['type'])
fdef.write(generate_type_cleanup(expr['type']) + "\n")
elif expr.has_key('union'):
--
1.7.7.rc0.72.g4b5ea
[Qemu-devel] [PATCH 03/26] qapi: add code generation support for middle mode
From: Anthony Liguori
To get the ball rolling merging QAPI, this patch introduces a "middle mode" to
the code generator. In middle mode, the code generator generates marshalling
functions that are compatible with the current QMP server. We absolutely need
to replace the current QMP server in order to support proper asynchronous
commands but using a middle mode provides a middle-ground that lets us start
converting commands in tree.
Note that all of the commands have been converted already in my glib branch.
Middle mode only exists until we finish merging them from my branch into the
main tree.
Signed-off-by: Anthony Liguori
Signed-off-by: Luiz Capitulino
---
qapi/qapi-types-core.h |3 ++
scripts/qapi-commands.py | 79 +
scripts/qapi-types.py|3 ++
scripts/qapi.py |4 ++-
4 files changed, 74 insertions(+), 15 deletions(-)
diff --git a/qapi/qapi-types-core.h b/qapi/qapi-types-core.h
index a79bc2b..27e6be0 100644
--- a/qapi/qapi-types-core.h
+++ b/qapi/qapi-types-core.h
@@ -17,4 +17,7 @@
#include "qemu-common.h"
#include "error.h"
+/* FIXME this is temporary until we remove middle mode */
+#include "monitor.h"
+
#endif
diff --git a/scripts/qapi-commands.py b/scripts/qapi-commands.py
index bf61740..2776804 100644
--- a/scripts/qapi-commands.py
+++ b/scripts/qapi-commands.py
@@ -167,9 +167,10 @@ qmp_input_visitor_cleanup(mi);
pop_indent()
return ret.rstrip()
-def gen_marshal_output(name, args, ret_type):
+def gen_marshal_output(name, args, ret_type, middle_mode):
if not ret_type:
return ""
+
ret = mcgen('''
static void qmp_marshal_output_%(c_name)s(%(c_ret_type)s ret_in, QObject
**ret_out, Error **errp)
{
@@ -188,16 +189,34 @@ static void qmp_marshal_output_%(c_name)s(%(c_ret_type)s
ret_in, QObject **ret_o
qapi_dealloc_visitor_cleanup(md);
}
''',
-c_ret_type=c_type(ret_type), c_name=c_var(name), ret_type=ret_type)
+ c_ret_type=c_type(ret_type), c_name=c_var(name),
+ ret_type=ret_type)
return ret
-def gen_marshal_input(name, args, ret_type):
+def gen_marshal_input_decl(name, args, ret_type, middle_mode):
+if middle_mode:
+return 'int qmp_marshal_input_%s(Monitor *mon, const QDict *qdict,
QObject **ret)' % c_var(name)
+else:
+return 'static void qmp_marshal_input_%s(QDict *args, QObject **ret,
Error **errp)' % c_var(name)
+
+
+
+def gen_marshal_input(name, args, ret_type, middle_mode):
+hdr = gen_marshal_input_decl(name, args, ret_type, middle_mode)
+
ret = mcgen('''
-static void qmp_marshal_input_%(c_name)s(QDict *args, QObject **ret, Error
**errp)
+%(header)s
{
''',
-c_name=c_var(name))
+header=hdr)
+
+if middle_mode:
+ret += mcgen('''
+Error *local_err = NULL;
+Error **errp = &local_err;
+QDict *args = (QDict *)qdict;
+''')
if ret_type:
if c_type(ret_type).endswith("*"):
@@ -220,6 +239,10 @@ static void qmp_marshal_input_%(c_name)s(QDict *args,
QObject **ret, Error **err
visitor_input_containers_decl=gen_visitor_input_containers_decl(args),
visitor_input_vars_decl=gen_visitor_input_vars_decl(args),
visitor_input_block=gen_visitor_input_block(args,
"QOBJECT(args)"))
+else:
+ret += mcgen('''
+(void)args;
+''')
ret += mcgen('''
if (error_is_set(errp)) {
@@ -234,10 +257,29 @@ out:
''')
ret += mcgen('''
%(visitor_input_block_cleanup)s
+''',
+ visitor_input_block_cleanup=gen_visitor_input_block(args,
None,
+
dealloc=True))
+
+if middle_mode:
+ret += mcgen('''
+
+if (local_err) {
+qerror_report_err(local_err);
+error_free(local_err);
+return -1;
+}
+return 0;
+''')
+else:
+ret += mcgen('''
return;
+''')
+
+ret += mcgen('''
}
-''',
- visitor_input_block_cleanup=gen_visitor_input_block(args,
None, dealloc=True))
+''')
+
return ret
def gen_registry(commands):
@@ -284,7 +326,7 @@ def gen_command_decl_prologue(header, guard, prefix=""):
#include "error.h"
''',
- header=basename(h_file), guard=guardname(h_file),
prefix=prefix)
+ header=basename(header), guard=guardname(header),
prefix=prefix)
return ret
def gen_command_def_prologue(prefix="", proxy=False):
@@ -317,11 +359,11 @@ def gen_command_def_prologue(prefix="", proxy=False):
prefix=prefix)
if not proxy:
ret += '#include "%sqmp-commands.h"' % prefix
-return ret + "\n"
+return ret + "\n\n"
try:
-opts, args = getopt.gnu_getopt(sys.argv[1:], "p:o:", ["prefix=",
"output-dir=", "type="])
+opts, args = getopt.gnu_getopt(sys.argv[1:], "p:o:m", ["prefix=",
"output-dir=", "type=", "middle"])
except getopt.Getop
[Qemu-devel] [PATCH 25/26] qapi: Convert system_reset
Signed-off-by: Anthony Liguori
Signed-off-by: Luiz Capitulino
---
hmp-commands.hx |3 +--
hmp.c|5 +
hmp.h|1 +
monitor.c| 10 --
qapi-schema.json |9 +
qmp-commands.hx |5 +
qmp.c|4
7 files changed, 21 insertions(+), 16 deletions(-)
diff --git a/hmp-commands.hx b/hmp-commands.hx
index 3ad1ce7..b2f5cd1 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -477,8 +477,7 @@ ETEXI
.args_type = "",
.params = "",
.help = "reset the system",
-.user_print = monitor_user_noop,
-.mhandler.cmd_new = do_system_reset,
+.mhandler.cmd = hmp_system_reset,
},
STEXI
diff --git a/hmp.c b/hmp.c
index efcb744..24f30bc 100644
--- a/hmp.c
+++ b/hmp.c
@@ -104,3 +104,8 @@ void hmp_stop(Monitor *mon, const QDict *qdict)
{
qmp_stop(NULL);
}
+
+void hmp_system_reset(Monitor *mon, const QDict *qdict)
+{
+qmp_system_reset(NULL);
+}
diff --git a/hmp.h b/hmp.h
index cb21cce..a49a6e6 100644
--- a/hmp.h
+++ b/hmp.h
@@ -25,5 +25,6 @@ void hmp_info_uuid(Monitor *mon);
void hmp_info_chardev(Monitor *mon);
void hmp_quit(Monitor *mon, const QDict *qdict);
void hmp_stop(Monitor *mon, const QDict *qdict);
+void hmp_system_reset(Monitor *mon, const QDict *qdict);
#endif
diff --git a/monitor.c b/monitor.c
index 2a5230c..c226879 100644
--- a/monitor.c
+++ b/monitor.c
@@ -1930,16 +1930,6 @@ static void do_boot_set(Monitor *mon, const QDict *qdict)
}
/**
- * do_system_reset(): Issue a machine reset
- */
-static int do_system_reset(Monitor *mon, const QDict *qdict,
- QObject **ret_data)
-{
-qemu_system_reset_request();
-return 0;
-}
-
-/**
* do_system_powerdown(): Issue a machine powerdown
*/
static int do_system_powerdown(Monitor *mon, const QDict *qdict,
diff --git a/qapi-schema.json b/qapi-schema.json
index cd05034..02de4b5 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -248,3 +248,12 @@
# state
##
{ 'command': 'stop' }
+
+##
+# @system_reset:
+#
+# Performs a hard reset of a guest.
+#
+# Since: 0.14.0
+##
+{ 'command': 'system_reset' }
diff --git a/qmp-commands.hx b/qmp-commands.hx
index 2ccddee..ea96191 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -223,10 +223,7 @@ EQMP
{
.name = "system_reset",
.args_type = "",
-.params = "",
-.help = "reset the system",
-.user_print = monitor_user_noop,
-.mhandler.cmd_new = do_system_reset,
+.mhandler.cmd_new = qmp_marshal_input_system_reset,
},
SQMP
diff --git a/qmp.c b/qmp.c
index 6c46479..51d9383 100644
--- a/qmp.c
+++ b/qmp.c
@@ -81,3 +81,7 @@ void qmp_stop(Error **errp)
vm_stop(RUN_STATE_PAUSED);
}
+void qmp_system_reset(Error **errp)
+{
+qemu_system_reset_request();
+}
--
1.7.7.rc0.72.g4b5ea
[Qemu-devel] [PATCH 17/26] RunState: Drop the RSTATE_NO_STATE value
The QAPI framework won't generate it, so we need to get rid of it.
In order to do that, this commit makes RSTATE_PRE_LAUNCH the initial
state and change qemu_vmstop_requested() to use RSTATE_MAX.
Signed-off-by: Luiz Capitulino
---
sysemu.h |1 -
vl.c | 19 +++
2 files changed, 7 insertions(+), 13 deletions(-)
diff --git a/sysemu.h b/sysemu.h
index 43ff546..41ccc02 100644
--- a/sysemu.h
+++ b/sysemu.h
@@ -11,7 +11,6 @@
/* vl.c */
typedef enum {
-RSTATE_NO_STATE,
RSTATE_DEBUG, /* qemu is running under gdb */
RSTATE_IN_MIGRATE, /* paused waiting for an incoming migration */
RSTATE_PANICKED, /* paused due to an internal error */
diff --git a/vl.c b/vl.c
index 4db58bd..bc61494 100644
--- a/vl.c
+++ b/vl.c
@@ -323,7 +323,7 @@ static int default_driver_check(QemuOpts *opts, void
*opaque)
/***/
/* QEMU state */
-static RunState current_run_state = RSTATE_NO_STATE;
+static RunState current_run_state = RSTATE_PRE_LAUNCH;
typedef struct {
RunState from;
@@ -332,10 +332,6 @@ typedef struct {
static const RunStateTransition runstate_transitions_def[] = {
/* from -> to */
-{ RSTATE_NO_STATE, RSTATE_RUNNING },
-{ RSTATE_NO_STATE, RSTATE_IN_MIGRATE },
-{ RSTATE_NO_STATE, RSTATE_PRE_LAUNCH },
-
{ RSTATE_DEBUG, RSTATE_RUNNING },
{ RSTATE_IN_MIGRATE, RSTATE_RUNNING },
@@ -350,6 +346,7 @@ static const RunStateTransition runstate_transitions_def[]
= {
{ RSTATE_POST_MIGRATE, RSTATE_RUNNING },
{ RSTATE_PRE_LAUNCH, RSTATE_RUNNING },
+{ RSTATE_PRE_LAUNCH, RSTATE_IN_MIGRATE },
{ RSTATE_PRE_LAUNCH, RSTATE_POST_MIGRATE },
{ RSTATE_PRE_MIGRATE, RSTATE_RUNNING },
@@ -424,8 +421,7 @@ void runstate_set(RunState new_state)
const char *runstate_as_string(void)
{
-assert(current_run_state > RSTATE_NO_STATE &&
- current_run_state < RSTATE_MAX);
+assert(current_run_state < RSTATE_MAX);
return runstate_name_tbl[current_run_state];
}
@@ -1294,7 +1290,7 @@ static int shutdown_requested, shutdown_signal = -1;
static pid_t shutdown_pid;
static int powerdown_requested;
static int debug_requested;
-static RunState vmstop_requested = RSTATE_NO_STATE;
+static RunState vmstop_requested = RSTATE_MAX;
int qemu_shutdown_requested_get(void)
{
@@ -1350,11 +1346,12 @@ static int qemu_debug_requested(void)
return r;
}
+/* We use RSTATE_MAX but any invalid value will do */
static bool qemu_vmstop_requested(RunState *r)
{
-if (vmstop_requested != RSTATE_NO_STATE) {
+if (vmstop_requested < RSTATE_MAX) {
*r = vmstop_requested;
-vmstop_requested = RSTATE_NO_STATE;
+vmstop_requested = RSTATE_MAX;
return true;
}
@@ -3569,8 +3566,6 @@ int main(int argc, char **argv, char **envp)
}
} else if (autostart) {
vm_start();
-} else {
-runstate_set(RSTATE_PRE_LAUNCH);
}
os_setup_post();
--
1.7.7.rc0.72.g4b5ea
[Qemu-devel] [PATCH 18/26] RunState: Rename enum values as generated by the QAPI
Next commit will convert the query-status command to use the
RunState type as generated by the QAPI.
In order to "transparently" replace the current enum by the QAPI
one, we have to make some changes to some enum values.
As the changes are simple renames, I'll do them in one shot. The
changes are:
- Rename the prefix from RSTATE_ to RUN_STATE_
- RUN_STATE_SAVEVM to RUN_STATE_SAVE_VM
- RUN_STATE_IN_MIGRATE to RUN_STATE_INMIGRATE
- RUN_STATE_PANICKED to RUN_STATE_INTERNAL_ERROR
- RUN_STATE_POST_MIGRATE to RUN_STATE_POSTMIGRATE
- RUN_STATE_PRE_LAUNCH to RUN_STATE_PRELAUNCH
- RUN_STATE_PRE_MIGRATE to RUN_STATE_PREMIGRATE
- RUN_STATE_RESTORE to RUN_STATE_RESTORE_VM
- RUN_STATE_PRE_MIGRATE to RUN_STATE_FINISH_MIGRATE
Signed-off-by: Luiz Capitulino
---
gdbstub.c | 26 ++--
hw/ide/core.c |2 +-
hw/scsi-disk.c |2 +-
hw/virtio-blk.c |2 +-
hw/watchdog.c |2 +-
kvm-all.c |2 +-
migration.c |6 +-
monitor.c | 10 ++--
savevm.c|4 +-
sysemu.h| 28 +++---
vl.c| 118 +++---
11 files changed, 101 insertions(+), 101 deletions(-)
diff --git a/gdbstub.c b/gdbstub.c
index 12dd100..31ecad2 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -2385,7 +2385,7 @@ static void gdb_vm_state_change(void *opaque, int
running, RunState state)
return;
}
switch (state) {
-case RSTATE_DEBUG:
+case RUN_STATE_DEBUG:
if (env->watchpoint_hit) {
switch (env->watchpoint_hit->flags & BP_MEM_ACCESS) {
case BP_MEM_READ:
@@ -2408,25 +2408,25 @@ static void gdb_vm_state_change(void *opaque, int
running, RunState state)
tb_flush(env);
ret = GDB_SIGNAL_TRAP;
break;
-case RSTATE_PAUSED:
+case RUN_STATE_PAUSED:
ret = GDB_SIGNAL_INT;
break;
-case RSTATE_SHUTDOWN:
+case RUN_STATE_SHUTDOWN:
ret = GDB_SIGNAL_QUIT;
break;
-case RSTATE_IO_ERROR:
+case RUN_STATE_IO_ERROR:
ret = GDB_SIGNAL_IO;
break;
-case RSTATE_WATCHDOG:
+case RUN_STATE_WATCHDOG:
ret = GDB_SIGNAL_ALRM;
break;
-case RSTATE_PANICKED:
+case RUN_STATE_INTERNAL_ERROR:
ret = GDB_SIGNAL_ABRT;
break;
-case RSTATE_SAVEVM:
-case RSTATE_RESTORE:
+case RUN_STATE_SAVE_VM:
+case RUN_STATE_RESTORE_VM:
return;
-case RSTATE_PRE_MIGRATE:
+case RUN_STATE_FINISH_MIGRATE:
ret = GDB_SIGNAL_XCPU;
break;
default:
@@ -2463,7 +2463,7 @@ void gdb_do_syscall(gdb_syscall_complete_cb cb, const
char *fmt, ...)
gdb_current_syscall_cb = cb;
s->state = RS_SYSCALL;
#ifndef CONFIG_USER_ONLY
-vm_stop(RSTATE_DEBUG);
+vm_stop(RUN_STATE_DEBUG);
#endif
s->state = RS_IDLE;
va_start(va, fmt);
@@ -2537,7 +2537,7 @@ static void gdb_read_byte(GDBState *s, int ch)
if (runstate_is_running()) {
/* when the CPU is running, we cannot do anything except stop
it when receiving a char */
-vm_stop(RSTATE_PAUSED);
+vm_stop(RUN_STATE_PAUSED);
} else
#endif
{
@@ -2799,7 +2799,7 @@ static void gdb_chr_event(void *opaque, int event)
{
switch (event) {
case CHR_EVENT_OPENED:
-vm_stop(RSTATE_PAUSED);
+vm_stop(RUN_STATE_PAUSED);
gdb_has_xml = 0;
break;
default:
@@ -2840,7 +2840,7 @@ static int gdb_monitor_write(CharDriverState *chr, const
uint8_t *buf, int len)
static void gdb_sigterm_handler(int signal)
{
if (runstate_is_running()) {
-vm_stop(RSTATE_PAUSED);
+vm_stop(RUN_STATE_PAUSED);
}
}
#endif
diff --git a/hw/ide/core.c b/hw/ide/core.c
index 4e76fc7..b71a356 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -527,7 +527,7 @@ static int ide_handle_rw_error(IDEState *s, int error, int
op)
s->bus->dma->ops->set_unit(s->bus->dma, s->unit);
s->bus->error_status = op;
bdrv_mon_event(s->bs, BDRV_ACTION_STOP, is_read);
-vm_stop(RSTATE_IO_ERROR);
+vm_stop(RUN_STATE_IO_ERROR);
} else {
if (op & BM_STATUS_DMA_RETRY) {
dma_buf_commit(s, 0);
diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
index e843f71..4f681ef 100644
--- a/hw/scsi-disk.c
+++ b/hw/scsi-disk.c
@@ -227,7 +227,7 @@ static int scsi_handle_rw_error(SCSIDiskReq *r, int error,
int type)
r->status |= SCSI_REQ_STATUS_RETRY | type;
bdrv_mon_event(s->bs, BDRV_ACTION_STOP, is_read);
-vm_stop(RSTATE_IO_ERROR);
+vm_stop(RUN_STATE_IO_ERROR);
} else {
switch (error) {
case ENOMEM:
diff --git a/hw/virtio-blk.c b/hw/virtio-blk.c
index daa8e42..03878bf 100644
--- a/hw/virtio-blk.c
+++ b/hw/virtio-blk.c
@@ -77,7 +77,7 @@ static int virtio_blk_handle_rw_error(VirtIOBlockReq *req,
int error,
req->next = s->rq;
s->rq = req;
bdrv_mon_event(s->bs, BDRV_ACTION_STOP, is_read);
-
Re: [Qemu-devel] QEMU + ARMMP11Core combination does not work
On 4 October 2011 06:07, TusharK wrote: > I tried executing QEMU (realview-smp ARM11MPCore) with Linux kernel 2.6.39.3, > but it failed. Kernel itself is not getting decompressed. > I am executing below command. > > qemu-system-arm -M realview-eb-mpcore -cpu arm11mpcore -kernel > arch/arm/boot/zImage -m 256M -append "root=/dev/sda console=ttyAMA0" -serial > stdio -hda ../flash/flash.bin > > Nothing is shown on terminal and QEMU window. > > I am using linux kernel 2.6.39.3 version. I also tried with Linux kernel > 2.6.35 + kernel_src_patch-2.6.35-arm1_bak version. Both are failing i.e. > nothing is shown on terminal. Kernel itself does not boot. 11MPCore isn't in the set of models I usually run. However "just sits there" is often a kernel configuration bug rather than a QEMU bug. (1) Does your kernel boot on the real hardware? (2) If you add "earlyprintk" to your -append string (and ensure early printk is selected as a config option when your kernel was built) do you get any kernel output? -- PMM
[Qemu-devel] [PATCH 16/26] vl: Change qemu_vmstop_requested() to return a bool
The stop reason is returned in the RunState argument. This is a
preparation for a future commit which will convert the query-status
command to the QAPI.
Signed-off-by: Luiz Capitulino
---
vl.c | 16 ++--
1 files changed, 10 insertions(+), 6 deletions(-)
diff --git a/vl.c b/vl.c
index bd4a5ce..4db58bd 100644
--- a/vl.c
+++ b/vl.c
@@ -1350,11 +1350,15 @@ static int qemu_debug_requested(void)
return r;
}
-static RunState qemu_vmstop_requested(void)
+static bool qemu_vmstop_requested(RunState *r)
{
-RunState s = vmstop_requested;
-vmstop_requested = RSTATE_NO_STATE;
-return s;
+if (vmstop_requested != RSTATE_NO_STATE) {
+*r = vmstop_requested;
+vmstop_requested = RSTATE_NO_STATE;
+return true;
+}
+
+return false;
}
void qemu_register_reset(QEMUResetHandler *func, void *opaque)
@@ -1567,7 +1571,7 @@ static void main_loop(void)
#ifdef CONFIG_PROFILER
int64_t ti;
#endif
-int r;
+RunState r;
qemu_main_loop_start();
@@ -1606,7 +1610,7 @@ static void main_loop(void)
monitor_protocol_event(QEVENT_POWERDOWN, NULL);
qemu_irq_raise(qemu_system_powerdown);
}
-if ((r = qemu_vmstop_requested())) {
+if (qemu_vmstop_requested(&r)) {
vm_stop(r);
}
}
--
1.7.7.rc0.72.g4b5ea
[Qemu-devel] [PATCH 02/26] qerror: add qerror_report_err()
From: Anthony Liguori
This provides a bridge between Error (new error mechanism) and QError (old error
mechanism). Errors can be propagated whereas QError cannot.
The minor evilness avoids layering violations. Since QError should go away RSN,
it seems like a reasonable hack.
Signed-off-by: Anthony Liguori
Signed-off-by: Luiz Capitulino
---
qerror.c | 33 +
qerror.h |2 ++
2 files changed, 35 insertions(+), 0 deletions(-)
diff --git a/qerror.c b/qerror.c
index c591a54..68998d4 100644
--- a/qerror.c
+++ b/qerror.c
@@ -482,6 +482,39 @@ void qerror_report_internal(const char *file, int linenr,
const char *func,
}
}
+/* Evil... */
+struct Error
+{
+QDict *obj;
+const char *fmt;
+char *msg;
+};
+
+void qerror_report_err(Error *err)
+{
+QError *qerr;
+int i;
+
+qerr = qerror_new();
+loc_save(&qerr->loc);
+QINCREF(err->obj);
+qerr->error = err->obj;
+
+for (i = 0; qerror_table[i].error_fmt; i++) {
+if (strcmp(qerror_table[i].error_fmt, err->fmt) == 0) {
+qerr->entry = &qerror_table[i];
+break;
+}
+}
+
+if (monitor_cur_is_qmp()) {
+monitor_set_error(cur_mon, qerr);
+} else {
+qerror_print(qerr);
+QDECREF(qerr);
+}
+}
+
/**
* qobject_to_qerror(): Convert a QObject into a QError
*/
diff --git a/qerror.h b/qerror.h
index d407001..d4bfcfd 100644
--- a/qerror.h
+++ b/qerror.h
@@ -15,6 +15,7 @@
#include "qdict.h"
#include "qstring.h"
#include "qemu-error.h"
+#include "error.h"
#include
typedef struct QErrorStringTable {
@@ -39,6 +40,7 @@ QString *qerror_human(const QError *qerror);
void qerror_print(QError *qerror);
void qerror_report_internal(const char *file, int linenr, const char *func,
const char *fmt, ...) GCC_FMT_ATTR(4, 5);
+void qerror_report_err(Error *err);
QString *qerror_format(const char *fmt, QDict *error);
#define qerror_report(fmt, ...) \
qerror_report_internal(__FILE__, __LINE__, __func__, fmt, ## __VA_ARGS__)
--
1.7.7.rc0.72.g4b5ea
[Qemu-devel] [PATCH 01/26] error: let error_is_type take a NULL error
From: Anthony Liguori
Reported-by: Luiz Capitulino
Signed-off-by: Anthony Liguori
Signed-off-by: Luiz Capitulino
---
error.c |4
1 files changed, 4 insertions(+), 0 deletions(-)
diff --git a/error.c b/error.c
index b802752..68c0039 100644
--- a/error.c
+++ b/error.c
@@ -97,6 +97,10 @@ bool error_is_type(Error *err, const char *fmt)
char *ptr;
char *end;
+if (!err) {
+return false;
+}
+
ptr = strstr(fmt, "'class': '");
assert(ptr != NULL);
ptr += strlen("'class': '");
--
1.7.7.rc0.72.g4b5ea
Re: [Qemu-devel] [RFC] New Migration Protocol using Visitor Interface
On 10/04/2011 09:05 PM, Stefan Berger wrote: On 10/03/2011 09:43 AM, Anthony Liguori wrote: On 10/03/2011 08:24 AM, Michael S. Tsirkin wrote: On Mon, Oct 03, 2011 at 07:51:00AM -0500, Anthony Liguori wrote: Here are some suggestions: - Let's make the protocol be BER directly. As a first step, use a single octet string for the whole of data. Next, start splitting this up. This can't be done without breaking the old style migration protocol. I don't have a problem with that but I do have a problem with repeatedly breaking migration protocol. As long as this is within a release cycle, is this a real problem? I think if we try to fit it within a release we'll either end up with a 2 year long release or a half-broken conversion. I'd rather buy ourselves time by supporting both formats. That way we can remove the old format when we're satisfied with the ASN.1 encoding. Hm, if backwards compatibility is what we want to achieve (migrating from Qemu 1.1 to Qemu 1.0) then at least the ASN.1 decoder / encoder should be all done in 1.0, no? Otherwise what would it mean to if 1.0 just skipped types 1.1 sends and doesn't understand? Before we introduce ASN1, we ought to introduce migration capabilities. Migration capabilities would be used to negotation ASN.1 over the wire. That means that 1.1 would use the existing protocol to talk to 1.0. There are multiple things to consider with compatibility: 1) Creating compatible device models. This is a qdev problem and can't be solved in the protocol. 2) Ensuring we are sending all the data we need to. I think we solve this problem by autogenerating Visitors from the C definitions of the device structures. I would have thought that we would write a function that takes the VMStateDescription as an argument and write ASN.1 BER or CER comprising: - a header containing the version of the device data - the minimum version required to read the device data - walk the array of VMStateFields and encode the the device data Sort of. You modify VMStateInfo to accept a visitor and name parameter in load and put. Then you write an ASN.1 BER Visitor and pass that visitor to VMStateInfo->load/put. Regards, Anthony Liguori and similarly a function for walking the fields for decoding of each device state. So at least I am surprised to hear 'autogeneration' for this particular case... Stefan
Re: [Qemu-devel] [RFC] New Migration Protocol using Visitor Interface
On 10/05/2011 06:28 AM, Michael S. Tsirkin wrote: On Mon, Oct 03, 2011 at 10:00:48AM -0500, Anthony Liguori wrote: Yes, it's easy to quantify. I think the following gives us the offset before and after, so the difference is the size we seek, right? OK, Orit (Cc'd) did some research - this is a booting while still in grub, size probably does not get much less than that. windows is said to be much more aggressive in allocating memory. start offset: 9600673 end offset: 9614933 So we get 15K out of 9M. So, let's do some napkin math. Assume that it works out that most of that 15k are 4 byte integers. If we assume an average name length of 6 characters, a string should be encoded in 8 bytes. That means for every 4 bytes, we add 8 bytes which means we're increasing by 200%. That means 45k. A 1gbit link can xmit at max, 128k in 1ms. So that extra 30k is going to cost ~250us in transmit time if we can get 1gbit. A the default rate limit, it should cost us right around 1ms. I guess it's liveable although with 30 network cards, I suspect it gets a heck of a lot worse. Regards, Anthony Liguori By the way, most of the memory here is pretty much all uniform I guess, because compressing it gets us: gzip: 1934169 bzip2 -9: 1462551 So even with aggressive compression, we probably won't be able to get below 1.5M for memory, two orders of magnitude above device state. Sounds convincing?
Re: [Qemu-devel] [PATCH 19/23] migration: Export a function that tells if the migration has finished correctly
Anthony Liguori wrote:
> On 09/23/2011 07:57 AM, Juan Quintela wrote:
>> This will allows us to hide the status values.
>>
>> --- a/ui/spice-core.c
>> +++ b/ui/spice-core.c
>> @@ -447,9 +447,7 @@ void do_info_spice(Monitor *mon, QObject **ret_data)
>>
>> static void migration_state_notifier(Notifier *notifier, void *data)
>> {
>> -int state = get_migration_state();
>> -
>> -if (state == MIG_STATE_COMPLETED) {
>> +if (migration_has_finished()) {
>> #if SPICE_SERVER_VERSION>= 0x000701 /* 0.7.1 */
>> spice_server_migrate_switch(spice_server);
>> #endif
>
> I think the bug here is migration_state_notifier. It should take an
> additional argument of MigrationState. Otherwise, how does this code
> work with FT?
Thinking about it, we need to pass MigrationState and export the
function that see if migration has finished (otherwise we also need to
export all STATE definitions, or worse, the whole MigrationState
definition.
Moving to have a function
bool migration_has_finished(MIgrationState *s);
That does the obvious thing.
What do you think?
Later, Juan.
Re: [Qemu-devel] QEMU + ARMMP11Core combination does not work
Hello, I am not getting any reply on this query. I do not know how should I use this combination and really stuck because of this. Can somebody from QEMU devel quickly provide me help in this regard? I do not know what is taking time to answer this query. Let me know if anything is needed from my side. I hope to get reply ASAP. Thanks & Regards, Tushar On Tue, 04 Oct 2011 10:37:23 +0530 wrote >Hi, > > I tried executing QEMU (realview-smp ARM11MPCore) with Linux kernel 2.6.39.3, but it failed. Kernel itself is not getting decompressed. > I am executing below command. > > qemu-system-arm -M realview-eb-mpcore -cpu arm11mpcore -kernel arch/arm/boot/zImage -m 256M -append "root=/dev/sda console=ttyAMA0" -serial stdio -hda ../flash/flash.bin > > Nothing is shown on terminal and QEMU window. > > I am using linux kernel 2.6.39.3 version. I also tried with Linux kernel 2.6.35 + kernel_src_patch-2.6.35-arm1_bak version. Both are failing i.e. nothing is shown on terminal. Kernel itself does not boot. > > Can you please help me out in this regards? > > Thanks & Regards, > TK Treat yourself at a restaurant, spa, resort and much more with Rediff Deal ho jaye!
Re: [Qemu-devel] [PATCH RFC V1 11/11] config/make: Introduce --enable-xen-pci-passthrough, built it.
On Tue, 4 Oct 2011, Anthony PERARD wrote: > Signed-off-by: Anthony PERARD > --- > Makefile.target |7 +++ > configure | 21 + > 2 files changed, 28 insertions(+), 0 deletions(-) > > diff --git a/Makefile.target b/Makefile.target > index f708453..b5fbc18 100644 > --- a/Makefile.target > +++ b/Makefile.target > @@ -208,6 +208,13 @@ obj-$(CONFIG_NO_XEN) += xen-stub.o > > obj-i386-$(CONFIG_XEN) += xen_platform.o > > +# Xen PCI Passthrough > +obj-i386-$(CONFIG_XEN_PCI_PASSTHROUGH) += host-pci-device.o > +obj-i386-$(CONFIG_XEN_PCI_PASSTHROUGH) += xen_pci_passthrough_helpers.o > +obj-i386-$(CONFIG_XEN_PCI_PASSTHROUGH) += xen_pci_passthrough.o > +obj-i386-$(CONFIG_XEN_PCI_PASSTHROUGH) += xen_pci_passthrough_config_init.o > +obj-i386-$(CONFIG_XEN_PCI_PASSTHROUGH) += xen_pci_passthrough_msi.o > + > # Inter-VM PCI shared memory > CONFIG_IVSHMEM = > ifeq ($(CONFIG_KVM), y) > diff --git a/configure b/configure > index 0875f95..b90cfe1 100755 > --- a/configure > +++ b/configure > @@ -127,6 +127,7 @@ vnc_png="" > vnc_thread="no" > xen="" > xen_ctrl_version="" > +xen_pci_passthrough="" > linux_aio="" > attr="" > xfs="" > @@ -635,6 +636,10 @@ for opt do >;; >--enable-xen) xen="yes" >;; > + --disable-xen-pci-passthrough) xen_pci_passthrough="no" > + ;; > + --enable-xen-pci-passthrough) xen_pci_passthrough="yes" > + ;; >--disable-brlapi) brlapi="no" >;; >--enable-brlapi) brlapi="yes" > @@ -972,6 +977,8 @@ echo " (affects only QEMU, not > qemu-img)" > echo " --enable-mixemu enable mixer emulation" > echo " --disable-xendisable xen backend driver support" > echo " --enable-xen enable xen backend driver support" > +echo " --disable-xen-pci-passthrough" > +echo " --enable-xen-pci-passthrough" > echo " --disable-brlapi disable BrlAPI" > echo " --enable-brlapi enable BrlAPI" > echo " --disable-vnc-tlsdisable TLS encryption for VNC server" > @@ -1335,6 +1342,17 @@ EOF >fi > fi > > +if test "$xen_pci_passthrough" != "no"; then > + if test "$xen" = "yes"; then > +xen_pci_passthrough=yes > + else > +if test "$xen_pci_passthrough" = "yes"; then > + feature_not_found "Xen PCI Passthrough without Xen" > +fi > +xen_pci_passthrough=no > + fi > +fi > + Xen works on many OSes that are not Linux, however host-pci-device only works on Linux. For the moment we can turn off xen_pci_passthrough if we are not running on Linux, but one day not too far I hope will have a host-pci-device implementation that works on BSDs and Solaris. > ## > # pkg-config probe > > @@ -3378,6 +3396,9 @@ case "$target_arch2" in > if test "$xen" = "yes" -a "$target_softmmu" = "yes" ; then >target_phys_bits=64 >echo "CONFIG_XEN=y" >> $config_target_mak > + if test "$xen_pci_passthrough" = yes; then > +echo "CONFIG_XEN_PCI_PASSTHROUGH=y" >> "$config_target_mak" > + fi > else >echo "CONFIG_NO_XEN=y" >> $config_target_mak > fi > -- > Anthony PERARD >
Re: [Qemu-devel] [PATCH RFC V1 10/11] Introduce Xen PCI Passthrough, MSI (3/3)
On Tue, 4 Oct 2011, Anthony PERARD wrote:
> Signed-off-by: Anthony PERARD
You should set the original author of this patch correctly and add his
signed-off-by.
Remember to run the patch through checkpatch.pl.
> ---
> hw/xen_pci_passthrough_msi.c | 674
> ++
> 1 files changed, 674 insertions(+), 0 deletions(-)
> create mode 100644 hw/xen_pci_passthrough_msi.c
>
> diff --git a/hw/xen_pci_passthrough_msi.c b/hw/xen_pci_passthrough_msi.c
> new file mode 100644
> index 000..be18ff1
> --- /dev/null
> +++ b/hw/xen_pci_passthrough_msi.c
> @@ -0,0 +1,674 @@
> +/*
> + * Copyright (c) 2007, Intel Corporation.
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms and conditions of the GNU General Public License,
> + * version 2, as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope it will be useful, but WITHOUT
> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
> + * more details.
> + *
> + * You should have received a copy of the GNU General Public License along
> with
> + * this program; if not, write to the Free Software Foundation, Inc., 59
> Temple
> + * Place - Suite 330, Boston, MA 02111-1307 USA.
> + *
> + * Jiang Yunhong
> + *
> + * This file implements direct PCI assignment to a HVM guest
> + */
> +
> +#include
> +
> +#include "xen_backend.h"
> +#include "xen_pci_passthrough.h"
> +
> +void msi_set_enable(XenPCIPassthroughState *dev, int en)
> +{
> +uint16_t val = 0;
> +uint32_t address = 0;
> +PT_LOG("enable: %i\n", en);
> +
> +if (!dev->msi) {
> +return;
> +}
> +
> +address = dev->msi->ctrl_offset;
> +if (!address) {
> +return;
> +}
> +
> +val = host_pci_read_word(dev->real_device, address);
> +val &= ~PCI_MSI_FLAGS_ENABLE;
> +val |= en & PCI_MSI_FLAGS_ENABLE;
> +host_pci_write_word(dev->real_device, address, val);
> +
> +PT_LOG("done, address: %#x, val: %#x\n", address, val);
> +}
> +
> +static void msix_set_enable(XenPCIPassthroughState *dev, int en)
> +{
> +uint16_t val = 0;
> +uint32_t address = 0;
> +
> +if (!dev->msix) {
> +return;
> +}
> +
> +address = dev->msix->ctrl_offset;
> +if (!address) {
> +return;
> +}
> +
> +val = host_pci_read_word(dev->real_device, address);
> +val &= ~PCI_MSIX_FLAGS_ENABLE;
> +if (en) {
> +val |= PCI_MSIX_FLAGS_ENABLE;
> +}
> +host_pci_write_word(dev->real_device, address, val);
> +}
> +
> +/*/
> +/* MSI virtuailization functions */
> +
> +/*
> + * setup physical msi, but didn't enable it
> + */
> +int pt_msi_setup(XenPCIPassthroughState *dev)
> +{
> +int pirq = -1;
> +uint8_t gvec = 0;
> +
> +if (!(dev->msi->flags & MSI_FLAG_UNINIT)) {
> +PT_LOG("Error: setup physical after initialized?? \n");
> +return -1;
> +}
> +
> +gvec = dev->msi->data & 0xFF;
> +if (!gvec) {
> +/* if gvec is 0, the guest is asking for a particular pirq that
> + * is passed as dest_id */
> +pirq = (dev->msi->addr_hi & 0xff00) |
> + ((dev->msi->addr_lo >> MSI_TARGET_CPU_SHIFT) & 0xff);
> +if (!pirq) {
> +/* this probably identifies an misconfiguration of the guest,
> + * try the emulated path */
> +pirq = -1;
> +} else {
> +PT_LOG("pt_msi_setup requested pirq = %d\n", pirq);
> +}
> +}
> +
> +if (xc_physdev_map_pirq_msi(xen_xc, xen_domid, AUTO_ASSIGN, &pirq,
> +PCI_DEVFN(dev->real_device->dev,
> + dev->real_device->func),
> +dev->real_device->bus, 0, 0)) {
> +PT_LOG("Error: Mapping of MSI failed.\n");
> +return -1;
> +}
> +
> +if (pirq < 0) {
> +PT_LOG("Error: Invalid pirq number\n");
> +return -1;
> +}
> +
> +dev->msi->pirq = pirq;
> +PT_LOG("msi mapped with pirq %x\n", pirq);
> +
> +return 0;
> +}
> +
> +static uint32_t __get_msi_gflags(uint32_t data, uint64_t addr)
> +{
> +uint32_t result = 0;
> +int rh, dm, dest_id, deliv_mode, trig_mode;
> +
> +rh = (addr >> MSI_ADDR_REDIRECTION_SHIFT) & 0x1;
> +dm = (addr >> MSI_ADDR_DESTMODE_SHIFT) & 0x1;
> +dest_id = (addr >> MSI_TARGET_CPU_SHIFT) & 0xff;
> +deliv_mode = (data >> MSI_DATA_DELIVERY_SHIFT) & 0x7;
> +trig_mode = (data >> MSI_DATA_TRIGGER_SHIFT) & 0x1;
> +
> +result |= dest_id | (rh << GFLAGS_SHIFT_RH) | (dm << GFLAGS_SHIFT_DM) | \
> + (deliv_mode << GLFAGS_SHIFT_DELIV_MODE) |
> + (trig_mode << GLFAGS_SHIFT_TRG_MODE);
> +
> +return result;
> +}
> +
> +int pt_msi_update(XenPCIPassthroughState *d)
> +{
> +uint8_t gvec = 0;
> +uint32_t gflags
Re: [Qemu-devel] [PATCH RFC V1 08/11] Introduce Xen PCI Passthrough, qdevice (1/3)
On Tue, 4 Oct 2011, Anthony PERARD wrote:
> Signed-off-by: Anthony PERARD
You should set the original author correctly and add his signed-off-by.
Remeber to run the patch through checkpatch.pl.
> ---
> hw/xen_pci_passthrough.c | 763
> ++
> hw/xen_pci_passthrough.h | 335 +
> hw/xen_pci_passthrough_helpers.c | 46 +++
> 3 files changed, 1144 insertions(+), 0 deletions(-)
> create mode 100644 hw/xen_pci_passthrough.c
> create mode 100644 hw/xen_pci_passthrough.h
> create mode 100644 hw/xen_pci_passthrough_helpers.c
>
> diff --git a/hw/xen_pci_passthrough.c b/hw/xen_pci_passthrough.c
> new file mode 100644
> index 000..bfbe042
> --- /dev/null
> +++ b/hw/xen_pci_passthrough.c
> @@ -0,0 +1,763 @@
> +#include
> +
> +#include "pci.h"
> +#include "xen.h"
> +#include "xen_backend.h"
> +#include "xen_pci_passthrough.h"
it would be nice to keep the comment about PCI-MSI translation we have
in the corresponding source file in qemu-xen
> +#define PCI_BAR_ENTRIES (6)
> +
> +typedef struct PlugDevice {
> +uint8_t r_bus;
> +uint8_t r_slot;
> +uint8_t r_func;
> +int bus;
> +int slot;
> +int func;
> +} PlugDevice;
> +
> +QLIST_HEAD(php_dev_list, PlugDevice) php_dev_list =
> +QLIST_HEAD_INITIALIZER(php_dev_list);
> +
What is the difference between r_bus,r_slot and r_func and bus, slot and
func? It should be well documented with a comment.
Also do we need both PlugDevice and PHPDev? What is the difference?
> +#define PT_BAR_ALLF 0x /* BAR ALLF value */
already defined in hw/xen_pci_passthrough.h
> +
> +/* #define PT_NR_IRQS (256) */
> +/* char mapped_machine_irq[PT_NR_IRQS] = {0}; */
> +
> +/* Config Space */
> +static int pt_pci_config_access_check(PCIDevice *d, uint32_t address, int
> len)
> +{
> +/* check offset range */
> +if (address >= 0xFF) {
> +PT_LOG("Error: Failed to access register with offset exceeding FFh. "
> + "[%02x:%02x.%x][Offset:%02xh][Length:%d]\n",
> + pci_bus_num(d->bus), PCI_SLOT(d->devfn), PCI_FUNC(d->devfn),
> + address, len);
> +return -1;
> +}
> +
> +/* check read size */
> +if ((len != 1) && (len != 2) && (len != 4)) {
> +PT_LOG("Error: Failed to access register with invalid access length.
> "
> + "[%02x:%02x.%x][Offset:%02xh][Length:%d]\n",
> + pci_bus_num(d->bus), PCI_SLOT(d->devfn), PCI_FUNC(d->devfn),
> + address, len);
> +return -1;
> +}
> +
> +/* check offset alignment */
> +if (address & (len - 1)) {
> +PT_LOG("Error: Failed to access register with invalid access size "
> +"alignment. [%02x:%02x.%x][Offset:%02xh][Length:%d]\n",
> +pci_bus_num(d->bus), PCI_SLOT(d->devfn), PCI_FUNC(d->devfn),
> +address, len);
> +return -1;
> +}
> +
> +return 0;
> +}
> +
> +int pt_bar_offset_to_index(uint32_t offset)
> +{
> +int index = 0;
> +
> +/* check Exp ROM BAR */
> +if (offset == PCI_ROM_ADDRESS) {
> +return PCI_ROM_SLOT;
> +}
> +
> +/* calculate BAR index */
> +index = (offset - PCI_BASE_ADDRESS_0) >> 2;
> +if (index >= PCI_NUM_REGIONS) {
> +return -1;
> +}
> +
> +return index;
> +}
> +
> +static uint32_t pt_pci_read_config(PCIDevice *d, uint32_t address, int len)
> +{
> +XenPCIPassthroughState *s = DO_UPCAST(XenPCIPassthroughState, dev, d);
> +uint32_t val = 0;
> +XenPTRegGroup *reg_grp_entry = NULL;
> +XenPTReg *reg_entry = NULL;
> +int rc = 0;
> +int emul_len = 0;
> +uint32_t find_addr = address;
> +
> +if (pt_pci_config_access_check(d, address, len)) {
> +goto exit;
> +}
> +
> +/* check power state transition flags */
> +if (s->pm_state != NULL && s->pm_state->flags & PT_FLAG_TRANSITING) {
> +/* can't accept untill previous power state transition is completed.
> + * so finished previous request here.
> + */
> +qemu_run_one_timer(s->pm_state->pm_timer);
> +}
the timer is not acceptable, we should probably remove it and return an
error instead, unless somebody has any better ideas.
> +/* find register group entry */
> +reg_grp_entry = pt_find_reg_grp(s, address);
> +if (reg_grp_entry) {
> +/* check 0 Hardwired register group */
> +if (reg_grp_entry->reg_grp->grp_type == GRP_TYPE_HARDWIRED) {
> +/* no need to emulate, just return 0 */
> +val = 0;
> +goto exit;
> +}
> +}
> +
> +/* read I/O device register value */
> +rc = host_pci_read_block(s->real_device, address, (uint8_t *)&val, len);
> +if (!rc) {
> +PT_LOG("Error: pci_read_block failed. return value[%d].\n", rc);
> +memset(&val, 0xff, len);
> +}
> +
> +/* just return the I/O device register value for
> + * passthrough type register
Re: [Qemu-devel] [PATCH RFC V1 07/11] host-pci-device: Add host_pci_find_ext_cap_offset
On Tue, 4 Oct 2011, Anthony PERARD wrote:
> Signed-off-by: Anthony PERARD
any reason why we shouldn't merge this patch with patch #1?
> ---
> hw/host-pci-device.c | 31 +++
> hw/host-pci-device.h |2 ++
> 2 files changed, 33 insertions(+), 0 deletions(-)
>
> diff --git a/hw/host-pci-device.c b/hw/host-pci-device.c
> index b3f2899..2a889d5 100644
> --- a/hw/host-pci-device.c
> +++ b/hw/host-pci-device.c
> @@ -162,6 +162,37 @@ int host_pci_write_block(HostPCIDevice *d, int pos,
> uint8_t *buf, int len)
>return host_pci_config_write(d, pos, buf, len);
> }
>
> +uint32_t host_pci_find_ext_cap_offset(HostPCIDevice *d, uint32_t cap)
> +{
> +uint32_t header = 0;
> +int max_cap = 480;
> +int pos = 0x100;
> +
> +do {
> +header = host_pci_read_long(d, pos);
> +/*
> + * If we have no capabilities, this is indicated by cap ID,
> + * cap version and next pointer all being 0.
> + */
> +if (header == 0) {
> +break;
> +}
> +
> +if (PCI_EXT_CAP_ID(header) == cap) {
> +return pos;
> +}
> +
> +pos = PCI_EXT_CAP_NEXT(header);
> +if (pos < 0x100) {
> +break;
> +}
> +
> +max_cap--;
> +} while (max_cap > 0);
> +
> +return 0;
> +}
> +
> HostPCIDevice *host_pci_device_get(uint8_t bus, uint8_t dev, uint8_t func)
> {
> HostPCIDevice *d = NULL;
> diff --git a/hw/host-pci-device.h b/hw/host-pci-device.h
> index 0137507..2734eb3 100644
> --- a/hw/host-pci-device.h
> +++ b/hw/host-pci-device.h
> @@ -33,4 +33,6 @@ int host_pci_write_word(HostPCIDevice *d, int pos, uint16_t
> data);
> int host_pci_write_long(HostPCIDevice *d, int pos, uint32_t data);
> int host_pci_write_block(HostPCIDevice *d, int pos, uint8_t *buf, int len);
>
> +uint32_t host_pci_find_ext_cap_offset(HostPCIDevice *s, uint32_t cap);
> +
> #endif /* !HW_HOST_PCI_DEVICE */
> --
> Anthony PERARD
>
Re: [Qemu-devel] [PATCH 2/2] LAPIC: make lapic support cpu hotplug
On 2011-10-05 12:26, liu ping fan wrote:
>> > And make the creation of apic as part of cpu initialization, so
>>> apic's state has been ready, before setting kvm_apic.
>>
>> There is no kvm-apic upstream yet, so it's hard to judge why we need
>> this here. If we do, this has to be a separate patch. But I seriously
>> doubt we need it (my hack worked without it, and that was not because of
>> its hack nature).
>>
>> Sorry, I did not explain it clearly. What I mean is that “env->apic_state”
> must be prepared
> before qemu_kvm_cpu_thread_fn() -> ... -> kvm_put_sregs(), where we get
> apic_base by
> “ sregs.apic_base = cpu_get_apic_base(env->apic_state);”
> and then call “kvm_vcpu_ioctl(env, KVM_SET_SREGS, &sregs);” which will
> finally affect the
> kvm_apic structure in kernel.
>
> But as current code, in pc_new_cpu(), we call apic_init() to initialize
> apic_state, after cpu_init(),
> so we can not guarantee the order of apic_state initializaion and the
> setting to kernel.
>
> Because LAPIC is part of x86 chip, I want to move it into cpu_x86_init(),
> and ensure apic_init()
> called before thread “qemu_kvm_cpu_thread_fn()” creation.
The LAPIC is part of the CPU, the classic APIC was a dedicated chip.
For various reasons, a safer approach for creating a new CPU is to stop
the machine, add the new device models, run cpu_synchronize_post_init on
that new cpu (looks like you missed that) and then resume everything.
See
http://git.kiszka.org/?p=qemu-kvm.git;a=commitdiff;h=be8f21c6b54eac82f7add7ee9d4ecf9cb8ebb320
...
>>> diff --git a/hw/icc_bus.c b/hw/icc_bus.c
>>> new file mode 100644
>>> index 000..360ca2a
>>> --- /dev/null
>>> +++ b/hw/icc_bus.c
>>> @@ -0,0 +1,62 @@
>>> +/*
>>> +*/
>>> +#define ICC_BUS_PLUG
>>> +#ifdef ICC_BUS_PLUG
>>> +#include "icc_bus.h"
>>> +
>>> +
>>> +
>>> +struct icc_bus_info icc_info = {
>>> +.qinfo.name = "icc",
>>> +.qinfo.size = sizeof(struct icc_bus),
>>> +.qinfo.props = (Property[]) {
>>> +DEFINE_PROP_END_OF_LIST(),
>>> +}
>>> +
>>> +};
>>> +
>>> +
>>> +static const VMStateDescription vmstate_icc_bus = {
>>> +.name = "icc_bus",
>>> +.version_id = 1,
>>> +.minimum_version_id = 1,
>>> +.minimum_version_id_old = 1,
>>> +.pre_save = NULL,
>>> +.post_load = NULL,
>>> +};
>>> +
>>> +struct icc_bus *g_iccbus;
>>> +
>>> +struct icc_bus *icc_init_bus(DeviceState *parent, const char *name)
>>> +{
>>> +struct icc_bus *bus;
>>> +
>>> +bus = FROM_QBUS(icc_bus, qbus_create(&icc_info.qinfo, parent,
>> name));
>>> +bus->qbus.allow_hotplug = 1; /* Yes, we can */
>>> +bus->qbus.name = "icc";
>>> +vmstate_register(NULL, -1, &vmstate_icc_bus, bus);
>>
>> The chipset is the owner of this bus and instantiates it. So it also
>> provides a vmstate. You can drop this unneeded one here (it's created
>> via an obsolete API anyway).
>>
>
> No familiar with Qemu bus emulation, keep on learning :) . But what I
> thought is,
> the x86-ICC bus is not the same as bus like PCI.
> For a PCI bus, it lies behind a host bridge, but ICC is shared by all x86
> processors in SMP system,
> so there is not a outstanding owner. And I right?
ICC is also attached to the chipset (due to the IOAPIC). So it looks
reasonable to me to let the chipset do the lifecycle management as well.
It is the fixed point, CPUs may come and go.
Jan
signature.asc
Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH 2/2] LAPIC: make lapic support cpu hotplug
On Wed, Oct 5, 2011 at 12:43 AM, Jan Kiszka wrote:
> On 2011-10-04 13:13, pingf...@linux.vnet.com wrote:
> > From: Liu Ping Fan
> >
> > Separate apic from qbus to icc bus which supports hotplug feature.
>
> Modeling the ICC bus looks like a step in the right direction. The
> IOAPIC could be attached to it as well to get rid of
> "ioapics[MAX_IOAPICS]".
Yes, but it will be the next step. But I introduce it because with current
code,
the hotplug creation of apic instance by apic_init() will hit the following
assert
"qdev_create_from_info: Assertion `bus->allow_hotplug' failed."
So I want to separate apic and create ICC-bus which connects LAPICs
together.
Most of all, it can support hotplug just like we can hotplug x86 physical
CPU in real machine.
> > And make the creation of apic as part of cpu initialization, so
> > apic's state has been ready, before setting kvm_apic.
>
> There is no kvm-apic upstream yet, so it's hard to judge why we need
> this here. If we do, this has to be a separate patch. But I seriously
> doubt we need it (my hack worked without it, and that was not because of
> its hack nature).
>
> Sorry, I did not explain it clearly. What I mean is that “env->apic_state”
must be prepared
before qemu_kvm_cpu_thread_fn() -> ... -> kvm_put_sregs(), where we get
apic_base by
“ sregs.apic_base = cpu_get_apic_base(env->apic_state);”
and then call “kvm_vcpu_ioctl(env, KVM_SET_SREGS, &sregs);” which will
finally affect the
kvm_apic structure in kernel.
But as current code, in pc_new_cpu(), we call apic_init() to initialize
apic_state, after cpu_init(),
so we can not guarantee the order of apic_state initializaion and the
setting to kernel.
Because LAPIC is part of x86 chip, I want to move it into cpu_x86_init(),
and ensure apic_init()
called before thread “qemu_kvm_cpu_thread_fn()” creation.
>
> > Signed-off-by: Liu Ping Fan
> > ---
> > Makefile.target |1 +
> > hw/apic.c|7 -
> > hw/apic.h|1 +
> > hw/icc_bus.c | 62
> ++
> > hw/icc_bus.h | 24 +++
> > hw/pc.c | 11 -
> > target-i386/cpu.h|1 +
> > target-i386/helper.c |7 +-
> > target-i386/kvm.c|1 -
> > 9 files changed, 105 insertions(+), 10 deletions(-)
> > create mode 100644 hw/icc_bus.c
> > create mode 100644 hw/icc_bus.h
> >
> > diff --git a/Makefile.target b/Makefile.target
> > index 9011f28..5607c6d 100644
> > --- a/Makefile.target
> > +++ b/Makefile.target
> > @@ -241,6 +241,7 @@ obj-i386-$(CONFIG_KVM) += kvmclock.o
> > obj-i386-$(CONFIG_SPICE) += qxl.o qxl-logger.o qxl-render.o
> > obj-i386-y += testdev.o
> > obj-i386-y += acpi.o acpi_piix4.o
> > +obj-i386-y += icc_bus.o
> >
> > obj-i386-y += pcspk.o i8254.o
> > obj-i386-$(CONFIG_KVM_PIT) += i8254-kvm.o
> > diff --git a/hw/apic.c b/hw/apic.c
> > index 69d6ac5..95a1664 100644
> > --- a/hw/apic.c
> > +++ b/hw/apic.c
> > @@ -24,6 +24,7 @@
> > #include "sysbus.h"
> > #include "trace.h"
> > #include "kvm.h"
> > +#include "icc_bus.h"
> >
> > /* APIC Local Vector Table */
> > #define APIC_LVT_TIMER 0
> > @@ -304,11 +305,13 @@ void cpu_set_apic_base(DeviceState *d, uint64_t
> val)
> >
> > if (!s)
> > return;
> > +
> > if (kvm_enabled() && kvm_irqchip_in_kernel())
> > s->apicbase = val;
> > else
> > s->apicbase = (val & 0xf000) |
> > (s->apicbase & (MSR_IA32_APICBASE_BSP |
> MSR_IA32_APICBASE_ENABLE));
> > +
> > /* if disabled, cannot be enabled again */
> > if (!(val & MSR_IA32_APICBASE_ENABLE)) {
> > s->apicbase &= ~MSR_IA32_APICBASE_ENABLE;
> > @@ -1075,7 +1078,7 @@ static const VMStateDescription vmstate_apic = {
> > }
> > };
> >
> > -static void apic_reset(DeviceState *d)
> > +void apic_reset(DeviceState *d)
>
> Still unused outside of this file, so keep it private.
>
> > {
> > APICState *s = DO_UPCAST(APICState, busdev.qdev, d);
> > int bsp;
> > @@ -1138,7 +1141,7 @@ static SysBusDeviceInfo apic_info = {
> >
> > static void apic_register_devices(void)
> > {
> > -sysbus_register_withprop(&apic_info);
> > +iccbus_register(&apic_info);
> > }
> >
> > device_init(apic_register_devices)
> > diff --git a/hw/apic.h b/hw/apic.h
> > index c857d52..e258efa 100644
> > --- a/hw/apic.h
> > +++ b/hw/apic.h
> > @@ -24,5 +24,6 @@ void apic_sipi(DeviceState *s);
> > /* pc.c */
> > int cpu_is_bsp(CPUState *env);
> > DeviceState *cpu_get_current_apic(void);
> > +void apic_reset(DeviceState *d);
> >
> > #endif
> > diff --git a/hw/icc_bus.c b/hw/icc_bus.c
> > new file mode 100644
> > index 000..360ca2a
> > --- /dev/null
> > +++ b/hw/icc_bus.c
> > @@ -0,0 +1,62 @@
> > +/*
> > +*/
> > +#define ICC_BUS_PLUG
> > +#ifdef ICC_BUS_PLUG
> > +#include "icc_bus.h"
> > +
> > +
> > +
> > +struct icc_bus_info icc_info = {
> > +.qinfo.name = "icc",
> > +.qinfo.size = sizeof(struct icc_bus),
> > +.qinfo
Re: [Qemu-devel] [PATCH 2/2] LAPIC: make lapic support cpu hotplug
On Tue, Oct 4, 2011 at 8:58 PM, Anthony Liguori wrote:
> On 10/04/2011 06:13 AM, pingf...@linux.vnet.com wrote:
>
>> From: Liu Ping Fan
>> >
>>
>> Separate apic from qbus to icc bus which supports hotplug feature.
>> And make the creation of apic as part of cpu initialization, so
>> apic's state has been ready, before setting kvm_apic.
>>
>> Signed-off-by: Liu Ping
>> Fan
>> >
>> ---
>> Makefile.target |1 +
>> hw/apic.c|7 -
>> hw/apic.h|1 +
>> hw/icc_bus.c | 62 ++**
>>
>> hw/icc_bus.h | 24 +++
>> hw/pc.c | 11 -
>> target-i386/cpu.h|1 +
>> target-i386/helper.c |7 +-
>> target-i386/kvm.c|1 -
>> 9 files changed, 105 insertions(+), 10 deletions(-)
>> create mode 100644 hw/icc_bus.c
>> create mode 100644 hw/icc_bus.h
>>
>> diff --git a/Makefile.target b/Makefile.target
>> index 9011f28..5607c6d 100644
>> --- a/Makefile.target
>> +++ b/Makefile.target
>> @@ -241,6 +241,7 @@ obj-i386-$(CONFIG_KVM) += kvmclock.o
>> obj-i386-$(CONFIG_SPICE) += qxl.o qxl-logger.o qxl-render.o
>> obj-i386-y += testdev.o
>> obj-i386-y += acpi.o acpi_piix4.o
>> +obj-i386-y += icc_bus.o
>>
>> obj-i386-y += pcspk.o i8254.o
>> obj-i386-$(CONFIG_KVM_PIT) += i8254-kvm.o
>> diff --git a/hw/apic.c b/hw/apic.c
>> index 69d6ac5..95a1664 100644
>> --- a/hw/apic.c
>> +++ b/hw/apic.c
>> @@ -24,6 +24,7 @@
>> #include "sysbus.h"
>> #include "trace.h"
>> #include "kvm.h"
>> +#include "icc_bus.h"
>>
>> /* APIC Local Vector Table */
>> #define APIC_LVT_TIMER 0
>> @@ -304,11 +305,13 @@ void cpu_set_apic_base(DeviceState *d, uint64_t val)
>>
>> if (!s)
>> return;
>> +
>> if (kvm_enabled()&& kvm_irqchip_in_kernel())
>>
>> s->apicbase = val;
>> else
>> s->apicbase = (val& 0xf000) |
>> (s->apicbase& (MSR_IA32_APICBASE_BSP |
>> MSR_IA32_APICBASE_ENABLE));
>>
>> +
>> /* if disabled, cannot be enabled again */
>> if (!(val& MSR_IA32_APICBASE_ENABLE)) {
>> s->apicbase&= ~MSR_IA32_APICBASE_ENABLE;
>>
>> @@ -1075,7 +1078,7 @@ static const VMStateDescription vmstate_apic = {
>> }
>> };
>>
>
> Please don't introduce extra whitespace in unrelated code.
>
> Adopt,
>
>
>> -static void apic_reset(DeviceState *d)
>> +void apic_reset(DeviceState *d)
>> {
>> APICState *s = DO_UPCAST(APICState, busdev.qdev, d);
>> int bsp;
>> @@ -1138,7 +1141,7 @@ static SysBusDeviceInfo apic_info = {
>>
>> static void apic_register_devices(void)
>> {
>> -sysbus_register_withprop(&**apic_info);
>> +iccbus_register(&apic_info);
>> }
>>
>> device_init(apic_register_**devices)
>> diff --git a/hw/apic.h b/hw/apic.h
>> index c857d52..e258efa 100644
>> --- a/hw/apic.h
>> +++ b/hw/apic.h
>> @@ -24,5 +24,6 @@ void apic_sipi(DeviceState *s);
>> /* pc.c */
>> int cpu_is_bsp(CPUState *env);
>> DeviceState *cpu_get_current_apic(void);
>> +void apic_reset(DeviceState *d);
>>
>> #endif
>> diff --git a/hw/icc_bus.c b/hw/icc_bus.c
>> new file mode 100644
>> index 000..360ca2a
>> --- /dev/null
>> +++ b/hw/icc_bus.c
>> @@ -0,0 +1,62 @@
>> +/*
>> +*/
>>
>
> New files need to include copyright/licenses.
>
> Adopt,
+#define ICC_BUS_PLUG
>> +#ifdef ICC_BUS_PLUG
>>
>
> Drop these guards here and at the end of the file. We conditionally build
> files by having an:
>
> obj-$(CONFIG_ICC_BUS) += icc_bus.o
>
> In the Makefile.
>
> Adopt,
> +#include "icc_bus.h"
>>
>
> +
>> +
>> +
>> +struct icc_bus_info icc_info = {
>> +.qinfo.name = "icc",
>> +.qinfo.size = sizeof(struct icc_bus),
>> +.qinfo.props = (Property[]) {
>> +DEFINE_PROP_END_OF_LIST(),
>> +}
>> +
>> +};
>>
>
> Structure name is not following Coding Style.
>
I will fix it.
+
>> +static const VMStateDescription vmstate_icc_bus = {
>> +.name = "icc_bus",
>> +.version_id = 1,
>> +.minimum_version_id = 1,
>> +.minimum_version_id_old = 1,
>> +.pre_save = NULL,
>> +.post_load = NULL,
>> +};
>> +
>> +struct icc_bus *g_iccbus;
>> +
>> +struct icc_bus *icc_init_bus(DeviceState *parent, const char *name)
>> +{
>> +struct icc_bus *bus;
>> +
>> +bus = FROM_QBUS(icc_bus, qbus_create(&icc_info.qinfo, parent, name));
>> +bus->qbus.allow_hotplug = 1; /* Yes, we can */
>> +bus->qbus.name = "icc";
>> +vmstate_register(NULL, -1,&vmstate_icc_bus, bus);
>>
>> +g_iccbus = bus;
>> +return bus;
>> +}
>> +
>> +
>> +
>> +
>> +
>> +static int iccbus_device_init(DeviceState *dev, DeviceInfo *base)
>> +{
>> +SysBusDeviceInfo *info = container_of(base, SysBusDeviceInfo, qdev);
>> +
>> +return info->init(sysbus_from_qdev(**dev));
>> +}
>> +
>> +void iccbus_register(**SysBusDeviceInfo *info)
>> +{
>> +info->qdev.init = iccbus_device_init;
>> +info->qdev.bus_info =&icc_info.qinfo;
>> +
>> +assert(info->qdev.size>= sizeof(SysBusDevice));
>> +qdev_register(&info->qdev)
Re: [Qemu-devel] [PATCH] Make cpu_single_env thread local (Linux only for now)
On 10/05/2011 09:52 AM, Jan Kiszka wrote:
Yeah, it probably makes sense to build the abstractions around __thread
so that - one day - we can drop the legacy wrappers.
Just do not prepend "tls__" in the gcc model
Actually I did that on purpose so that people would not forget get_tls. :)
(there is also some inconsistency with prefixes in patch 3).
Yep, the attached v2 actually builds. I also needed a small change to
avoid errors with -Wredundant-decls, and I changed it to support arrays with
DECLARE_TLS(int[10], array);
And avoid leading "_" unless
they are dictated by the platform.
Ok, I replaced tls_init_thread with tls_init_main_thread and
_tls_init_thread with tls_init_thread.
And patch 3 needs to update darwin-user/main.c as well.
I think the declaration can just be removed.
What is the default priority of constructors BTW? You picked the
highest, will others that do not specify one have the same?
Looks like the prioritized constructors always run _before_ the others,
which is good.
$ cat f.c
int f(void) __attribute__((constructor(101)));
int f(void) { write (1, "101\n", 4); }
int h(void) __attribute__((constructor));
int h(void) { write (1, "default\n", 8); }
int g(void) __attribute__((constructor(102)));
int g(void) { write (1, "102\n", 4); }
int main() { write(1, "main\n", 5); }
$ gcc f.c
$ ./a.out
101
102
default
main
If interested people can test the patches more and submit them more
formally, I'd be very glad. I wrote it for RCU, but of course that one
is not really going to be 1.0 material (even for 9p).
Paolo
>From 497ed0672f7fe08d9654a0e5c11b682bea43a59e Mon Sep 17 00:00:00 2001
From: Paolo Bonzini
Date: Wed, 5 Oct 2011 08:29:39 +0200
Subject: [PATCH 0/3] *** SUBJECT HERE ***
*** BLURB HERE ***
Paolo Bonzini (3):
qemu-threads: add TLS wrappers
Prepare Windows port for thread-local cpu_single_env
Make cpu_single_env thread-local
configure | 20 +
coroutine-win32.c |7 -
cpu-all.h |4 ++-
cpus.c | 13 +++---
exec.c |2 +-
qemu-thread-posix.c | 42 ---
qemu-thread-win32.c | 16 +
qemu-tls-gcc.h | 25 +
qemu-tls-pthread.h | 58 ++
qemu-tls-win32.h| 59 +++
10 files changed, 234 insertions(+), 12 deletions(-)
create mode 100644 qemu-tls-gcc.h
create mode 100644 qemu-tls-pthread.h
create mode 100644 qemu-tls-win32.h
--
1.7.6
>From d8c3c4e789f9b86a66042a9181333e1a096b6b93 Mon Sep 17 00:00:00 2001
From: Paolo Bonzini
Date: Tue, 16 Aug 2011 10:37:44 -0700
Subject: [PATCH 1/3] qemu-threads: add TLS wrappers
Win32 emulated TLS is slow and is not available on all versions of GCC;
some versions of Unix only have pthread_getspecific as a means to access
TLS.
Actually, Win32 does have support for decent TLS, and GCC does not map
__thread to it. But kind of unlike ELF TLS, it's perfectly possible
to declare TLS variables with simple C code! For pthread_getspecific
we similarly allocate a memory block; we have to compute all the offsets
at load time, which is also cheaper than doing a pthread_key_create for
each variable. Not optimal, but it works.
This patch adds wrappers to qemu-thread that will use __thread or
pthread_getspecific on POSIX systems, and the .tls segment on Windows.
It does kinda uglify the declarations, but not too much.
Signed-off-by: Paolo Bonzini
---
configure | 20 +
coroutine-win32.c |7 -
qemu-thread-posix.c | 42 ---
qemu-thread-win32.c | 16 +
qemu-tls-gcc.h | 25 +
qemu-tls-pthread.h | 58 ++
qemu-tls-win32.h| 59 +++
7 files changed, 221 insertions(+), 6 deletions(-)
create mode 100644 qemu-tls-gcc.h
create mode 100644 qemu-tls-pthread.h
create mode 100644 qemu-tls-win32.h
diff --git a/configure b/configure
index 59b1494..50d7b54 100755
--- a/configure
+++ b/configure
@@ -1215,6 +1215,23 @@ EOF
fi
##
+# __thread check
+
+if test "$mingw32" = "yes" ; then
+tls_model=win32
+else
+cat > $TMPC << EOF
+__thread int x;
+int main() { return x; }
+EOF
+if compile_prog "" "" ; then
+tls_model=gcc
+else
+tls_model=pthread
+fi
+fi
+
+##
# zlib check
if test "$zlib" != "no" ; then
@@ -2697,6 +2714,7 @@ echo "Documentation $docs"
[ ! -z "$uname_release" ] && \
echo "uname -r $uname_release"
echo "NPTL support $nptl"
+echo "TLS support $tls_model"
echo "GUEST_BASE$guest_base"
echo "PIE user targets $user_pie"
echo "vde support $vde"
@@ -3580,6 +3598,8 @@ if test "$target_linux_user" = "ye
[Qemu-devel] [PATCH 7/7] linux-user: Remove unused code
From: Stefan Weil The code is unused since 8 years, so remove it. Signed-off-by: Stefan Weil Signed-off-by: Stefan Hajnoczi --- linux-user/signal.c |5 + 1 files changed, 1 insertions(+), 4 deletions(-) diff --git a/linux-user/signal.c b/linux-user/signal.c index 89276eb..40c5eb18 100644 --- a/linux-user/signal.c +++ b/linux-user/signal.c @@ -1274,10 +1274,7 @@ setup_return(CPUState *env, struct target_sigaction *ka, if (__put_user(retcodes[idx], rc)) return 1; -#if 0 - flush_icache_range((abi_ulong)rc, - (abi_ulong)(rc + 1)); -#endif + retcode = rc_addr + thumb; } -- 1.7.6.3
[Qemu-devel] [PATCH 4/7] lsi: Fix tag reference in debug print
From: Jan Kiszka
Signed-off-by: Jan Kiszka
Signed-off-by: Stefan Hajnoczi
---
hw/lsi53c895a.c |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/hw/lsi53c895a.c b/hw/lsi53c895a.c
index 75a03a7..e077ec0 100644
--- a/hw/lsi53c895a.c
+++ b/hw/lsi53c895a.c
@@ -697,7 +697,7 @@ static int lsi_queue_req(LSIState *s, SCSIRequest *req,
uint32_t len)
lsi_reselect(s, p);
return 0;
} else {
-DPRINTF("Queueing IO tag=0x%x\n", tag);
+DPRINTF("Queueing IO tag=0x%x\n", p->tag);
p->pending = len;
return 1;
}
--
1.7.6.3
[Qemu-devel] [PATCH 3/7] gt64xxx.c: remove reference to non-existing ISD_handle field
From: Antony Pavlov
The commit fc2bf44972349b078d8310466c3866615500e67f
removed ISD_handle field from struct GT64120State,
so remove the field from DPRINTF too.
Signed-off-by: Antony Pavlov
Signed-off-by: Stefan Hajnoczi
---
hw/gt64xxx.c |5 +++--
1 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/hw/gt64xxx.c b/hw/gt64xxx.c
index 73454ff..432683a 100644
--- a/hw/gt64xxx.c
+++ b/hw/gt64xxx.c
@@ -275,8 +275,9 @@ static void gt64120_isd_mapping(GT64120State *s)
check_reserved_space(&start, &length);
length = 0x1000;
/* Map new address */
-DPRINTF("ISD: "TARGET_FMT_plx"@"TARGET_FMT_plx" ->
"TARGET_FMT_plx"@"TARGET_FMT_plx", %x\n", s->ISD_length, s->ISD_start,
-length, start, s->ISD_handle);
+DPRINTF("ISD: "TARGET_FMT_plx"@"TARGET_FMT_plx
+" -> "TARGET_FMT_plx"@"TARGET_FMT_plx"\n",
+s->ISD_length, s->ISD_start, length, start);
s->ISD_start = start;
s->ISD_length = length;
memory_region_add_subregion(get_system_memory(), s->ISD_start,
&s->ISD_mem);
--
1.7.6.3
[Qemu-devel] [PATCH 6/7] Fix mismatching allocation and deallocation
From: Stefan Weil
This error was reported by cppcheck.
Signed-off-by: Stefan Weil
Signed-off-by: Stefan Hajnoczi
---
console.c |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/console.c b/console.c
index 6dfcc47..e43de92 100644
--- a/console.c
+++ b/console.c
@@ -1538,7 +1538,7 @@ int text_console_init(QemuOpts *opts, CharDriverState
**_chr)
}
if (!s) {
-free(chr);
+g_free(chr);
return -EBUSY;
}
--
1.7.6.3
[Qemu-devel] [PATCH 5/7] target-arm: Fix typo
From: Andreas Färber
The command line option is called -kernel, not -kenrel.
Cc: Paul Brook
Reviewed-by: Peter Maydell
Signed-off-by: Andreas Färber
Signed-off-by: Stefan Hajnoczi
---
target-arm/helper.c |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/target-arm/helper.c b/target-arm/helper.c
index d3a3ba2..e2428eb 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -298,7 +298,7 @@ void cpu_reset(CPUARMState *env)
if (rom) {
/* We should really use ldl_phys here, in case the guest
modified flash and reset itself. However images
- loaded via -kenrel have not been copied yet, so load the
+ loaded via -kernel have not been copied yet, so load the
values directly from there. */
env->regs[13] = ldl_p(rom);
pc = ldl_p(rom + 4);
--
1.7.6.3
[Qemu-devel] [PATCH 2/7] gt64xxx.c: fix length modifier in DPRINTF format string
From: Antony Pavlov
The commit fc2bf44972349b078d8310466c3866615500e67f
changed the type of val argument of the function gt64120_writel()
from uint32_t to uint64_t, so we need to change the corresponding
length modifier from "%x" to "%" PRIx64.
Signed-off-by: Antony Pavlov
Signed-off-by: Stefan Hajnoczi
---
hw/gt64xxx.c |8
1 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/hw/gt64xxx.c b/hw/gt64xxx.c
index 1c34253..73454ff 100644
--- a/hw/gt64xxx.c
+++ b/hw/gt64xxx.c
@@ -543,19 +543,19 @@ static void gt64120_writel (void *opaque,
target_phys_addr_t addr,
/* not really implemented */
s->regs[saddr] = ~(~(s->regs[saddr]) | ~(val & 0xfffe));
s->regs[saddr] |= !!(s->regs[saddr] & 0xfffe);
-DPRINTF("INTRCAUSE %x\n", val);
+DPRINTF("INTRCAUSE %" PRIx64 "\n", val);
break;
case GT_INTRMASK:
s->regs[saddr] = val & 0x3c3e;
-DPRINTF("INTRMASK %x\n", val);
+DPRINTF("INTRMASK %" PRIx64 "\n", val);
break;
case GT_PCI0_ICMASK:
s->regs[saddr] = val & 0x03fe;
-DPRINTF("ICMASK %x\n", val);
+DPRINTF("ICMASK %" PRIx64 "\n", val);
break;
case GT_PCI0_SERR0MASK:
s->regs[saddr] = val & 0x003f;
-DPRINTF("SERR0MASK %x\n", val);
+DPRINTF("SERR0MASK %" PRIx64 "\n", val);
break;
/* Reserved when only PCI_0 is configured. */
--
1.7.6.3
[Qemu-devel] [PATCH 1/7] makefile: extract tools-obj-y
From: Paolo Bonzini Signed-off-by: Paolo Bonzini Signed-off-by: Stefan Hajnoczi --- Makefile |9 + 1 files changed, 5 insertions(+), 4 deletions(-) diff --git a/Makefile b/Makefile index 6ed3194..5723108 100644 --- a/Makefile +++ b/Makefile @@ -145,11 +145,12 @@ endif qemu-img.o: qemu-img-cmds.h qemu-img.o qemu-tool.o qemu-nbd.o qemu-io.o cmd.o qemu-ga.o: $(GENERATED_HEADERS) -qemu-img$(EXESUF): qemu-img.o qemu-tool.o qemu-error.o $(oslib-obj-y) $(trace-obj-y) $(block-obj-y) $(qobject-obj-y) $(version-obj-y) qemu-timer-common.o +tools-obj-y = qemu-tool.o qemu-error.o $(oslib-obj-y) $(trace-obj-y) \ +$(block-obj-y) $(qobject-obj-y) $(version-obj-y) qemu-timer-common.o -qemu-nbd$(EXESUF): qemu-nbd.o qemu-tool.o qemu-error.o $(oslib-obj-y) $(trace-obj-y) $(block-obj-y) $(qobject-obj-y) $(version-obj-y) qemu-timer-common.o - -qemu-io$(EXESUF): qemu-io.o cmd.o qemu-tool.o qemu-error.o $(oslib-obj-y) $(trace-obj-y) $(block-obj-y) $(qobject-obj-y) $(version-obj-y) qemu-timer-common.o +qemu-img$(EXESUF): qemu-img.o $(tools-obj-y) +qemu-nbd$(EXESUF): qemu-nbd.o $(tools-obj-y) +qemu-io$(EXESUF): qemu-io.o cmd.o $(tools-obj-y) qemu-img-cmds.h: $(SRC_PATH)/qemu-img-cmds.hx $(call quiet-command,sh $(SRC_PATH)/scripts/hxtool -h < $< > $@," GEN $@") -- 1.7.6.3
[Qemu-devel] [PULL 0/7] Trivial patches for September 22 to October 5 2011
The following changes since commit d11cf8cc80d946dfc9a23597cd9a0bb1c487cfa7: etrax-dma: Remove bogus if statement (2011-10-03 10:20:13 +0200) are available in the git repository at: ssh://repo.or.cz/srv/git/qemu/stefanha.git trivial-patches Andreas Färber (1): target-arm: Fix typo Antony Pavlov (2): gt64xxx.c: fix length modifier in DPRINTF format string gt64xxx.c: remove reference to non-existing ISD_handle field Jan Kiszka (1): lsi: Fix tag reference in debug print Paolo Bonzini (1): makefile: extract tools-obj-y Stefan Weil (2): Fix mismatching allocation and deallocation linux-user: Remove unused code Makefile|9 + console.c |2 +- hw/gt64xxx.c| 13 +++-- hw/lsi53c895a.c |2 +- linux-user/signal.c |5 + target-arm/helper.c |2 +- 6 files changed, 16 insertions(+), 17 deletions(-) -- 1.7.6.3
[Qemu-devel] [PATCH v3 08/15] block: add bdrv_co_discard and bdrv_aio_discard support
Signed-off-by: Paolo Bonzini
---
block.c | 140 -
block.h |3 +
block_int.h |9 +++-
trace-events |1 +
4 files changed, 148 insertions(+), 5 deletions(-)
diff --git a/block.c b/block.c
index f4b9089..7853982 100644
--- a/block.c
+++ b/block.c
@@ -53,6 +53,9 @@ static BlockDriverAIOCB *bdrv_aio_writev_em(BlockDriverState
*bs,
BlockDriverCompletionFunc *cb, void *opaque);
static BlockDriverAIOCB *bdrv_aio_flush_em(BlockDriverState *bs,
BlockDriverCompletionFunc *cb, void *opaque);
+static BlockDriverAIOCB *bdrv_aio_discard_em(BlockDriverState *bs,
+int64_t sector_num, int nb_sectors,
+BlockDriverCompletionFunc *cb, void *opaque);
static BlockDriverAIOCB *bdrv_aio_noop_em(BlockDriverState *bs,
BlockDriverCompletionFunc *cb, void *opaque);
static int bdrv_read_em(BlockDriverState *bs, int64_t sector_num,
@@ -60,6 +63,8 @@ static int bdrv_read_em(BlockDriverState *bs, int64_t
sector_num,
static int bdrv_write_em(BlockDriverState *bs, int64_t sector_num,
const uint8_t *buf, int nb_sectors);
static int bdrv_flush_em(BlockDriverState *bs);
+static int bdrv_discard_em(BlockDriverState *bs, int64_t sector_num,
+ int nb_sectors);
static BlockDriverAIOCB *bdrv_co_aio_readv_em(BlockDriverState *bs,
int64_t sector_num, QEMUIOVector *qiov, int nb_sectors,
BlockDriverCompletionFunc *cb, void *opaque);
@@ -68,6 +73,9 @@ static BlockDriverAIOCB
*bdrv_co_aio_writev_em(BlockDriverState *bs,
BlockDriverCompletionFunc *cb, void *opaque);
static BlockDriverAIOCB *bdrv_co_aio_flush_em(BlockDriverState *bs,
BlockDriverCompletionFunc *cb, void *opaque);
+static BlockDriverAIOCB *bdrv_co_aio_discard_em(BlockDriverState *bs,
+int64_t sector_num, int nb_sectors,
+BlockDriverCompletionFunc *cb, void *opaque);
static int coroutine_fn bdrv_co_readv_em(BlockDriverState *bs,
int64_t sector_num, int nb_sectors,
QEMUIOVector *iov);
@@ -75,6 +83,8 @@ static int coroutine_fn bdrv_co_writev_em(BlockDriverState
*bs,
int64_t sector_num, int nb_sectors,
QEMUIOVector *iov);
static int coroutine_fn bdrv_co_flush_em(BlockDriverState *bs);
+static int coroutine_fn bdrv_co_discard_em(BlockDriverState *bs,
+int64_t sector_num, int nb_sectors);
static QTAILQ_HEAD(, BlockDriverState) bdrv_states =
QTAILQ_HEAD_INITIALIZER(bdrv_states);
@@ -209,6 +219,14 @@ void bdrv_register(BlockDriver *bdrv)
bdrv->bdrv_aio_flush = bdrv_aio_flush_em;
}
+if (bdrv->bdrv_co_discard) {
+bdrv->bdrv_aio_discard = bdrv_co_aio_discard_em;
+} else if (bdrv->bdrv_aio_discard) {
+bdrv->bdrv_co_discard = bdrv_co_discard_em;
+} else {
+bdrv->bdrv_aio_discard = bdrv_aio_discard_em;
+}
+
/* add synchronous IO emulation layer */
if (!bdrv->bdrv_read) {
bdrv->bdrv_read = bdrv_read_em;
@@ -217,6 +235,9 @@ void bdrv_register(BlockDriver *bdrv)
if (!bdrv->bdrv_flush) {
bdrv->bdrv_flush = bdrv_flush_em;
}
+if (!bdrv->bdrv_discard) {
+bdrv->bdrv_discard = bdrv_discard_em;
+}
QLIST_INSERT_HEAD(&bdrv_drivers, bdrv, list);
}
@@ -1791,10 +1812,18 @@ int bdrv_discard(BlockDriverState *bs, int64_t
sector_num, int nb_sectors)
if (!bs->drv) {
return -ENOMEDIUM;
}
-if (!bs->drv->bdrv_discard) {
-return 0;
+if (bdrv_check_request(bs, sector_num, nb_sectors)) {
+return -EIO;
+}
+if (bs->drv->bdrv_co_discard && qemu_in_coroutine()) {
+return bs->drv->bdrv_co_discard(bs, sector_num, nb_sectors);
}
-return bs->drv->bdrv_discard(bs, sector_num, nb_sectors);
+
+if (bs->drv->bdrv_discard) {
+return bs->drv->bdrv_discard(bs, sector_num, nb_sectors);
+}
+
+return 0;
}
/*
@@ -2656,6 +2685,24 @@ BlockDriverAIOCB *bdrv_aio_flush(BlockDriverState *bs,
return drv->bdrv_aio_flush(bs, cb, opaque);
}
+BlockDriverAIOCB *bdrv_aio_discard(BlockDriverState *bs,
+int64_t sector_num, int nb_sectors,
+BlockDriverCompletionFunc *cb, void *opaque)
+{
+trace_bdrv_aio_discard(bs, sector_num, nb_sectors, opaque);
+
+if (!bs->drv) {
+return NULL;
+}
+if (bs->read_only) {
+return NULL;
+}
+if (bdrv_check_request(bs, sector_num, nb_sectors)) {
+return NULL;
+}
+return bs->drv->bdrv_aio_discard(bs, sector_num, nb_sectors, cb, opaque);
+}
+
void bdrv_aio_cancel(BlockDriverAIOCB *acb)
{
acb->pool->cancel(acb);
@@ -2873,6 +2920,52 @@ static BlockDriverAIOCB
*bdrv_aio_flush_em(BlockDriverState *bs,
return &acb->common;
}
+static void coroutine_fn bdrv_co_discard(void *opaque)
+{
+BlockDriverAIOCBCoroutine *acb = op
[Qemu-devel] [PATCH v3 14/15] nbd: split requests
qemu-nbd has a limit of slightly less than 1M per request. Work
around this in the nbd block driver.
Signed-off-by: Paolo Bonzini
---
block/nbd.c | 52 ++--
1 files changed, 46 insertions(+), 6 deletions(-)
diff --git a/block/nbd.c b/block/nbd.c
index 90c4f2f..a52b6f2 100644
--- a/block/nbd.c
+++ b/block/nbd.c
@@ -277,8 +277,9 @@ static int nbd_open(BlockDriverState *bs, const char*
filename, int flags)
return result;
}
-static int nbd_co_readv(BlockDriverState *bs, int64_t sector_num,
-int nb_sectors, QEMUIOVector *qiov)
+static int nbd_co_readv_1(BlockDriverState *bs, int64_t sector_num,
+ int nb_sectors, QEMUIOVector *qiov,
+ int offset)
{
BDRVNBDState *s = bs->opaque;
struct nbd_request request;
@@ -292,15 +293,16 @@ static int nbd_co_readv(BlockDriverState *bs, int64_t
sector_num,
if (nbd_co_send_request(s, &request, NULL, 0) == -1) {
reply.error = errno;
} else {
-nbd_co_receive_reply(s, &request, &reply, qiov->iov, 0);
+nbd_co_receive_reply(s, &request, &reply, qiov->iov, offset);
}
nbd_coroutine_end(s, &request);
return -reply.error;
}
-static int nbd_co_writev(BlockDriverState *bs, int64_t sector_num,
- int nb_sectors, QEMUIOVector *qiov)
+static int nbd_co_writev_1(BlockDriverState *bs, int64_t sector_num,
+ int nb_sectors, QEMUIOVector *qiov,
+ int offset)
{
BDRVNBDState *s = bs->opaque;
struct nbd_request request;
@@ -315,7 +317,7 @@ static int nbd_co_writev(BlockDriverState *bs, int64_t
sector_num,
request.len = nb_sectors * 512;
nbd_coroutine_start(s, &request);
-if (nbd_co_send_request(s, &request, qiov->iov, 0) == -1) {
+if (nbd_co_send_request(s, &request, qiov->iov, offset) == -1) {
reply.error = errno;
} else {
nbd_co_receive_reply(s, &request, &reply, NULL, 0);
@@ -324,6 +326,44 @@ static int nbd_co_writev(BlockDriverState *bs, int64_t
sector_num,
return -reply.error;
}
+/* qemu-nbd has a limit of slightly less than 1M per request. Try to
+ * remain aligned to 4K. */
+#define NBD_MAX_SECTORS 2040
+
+static int nbd_co_readv(BlockDriverState *bs, int64_t sector_num,
+int nb_sectors, QEMUIOVector *qiov)
+{
+int offset = 0;
+int ret;
+while (nb_sectors > NBD_MAX_SECTORS) {
+ret = nbd_co_readv_1(bs, sector_num, NBD_MAX_SECTORS, qiov, offset);
+if (ret < 0) {
+return ret;
+}
+offset += NBD_MAX_SECTORS * 512;
+sector_num += NBD_MAX_SECTORS;
+nb_sectors -= NBD_MAX_SECTORS;
+}
+return nbd_co_readv_1(bs, sector_num, nb_sectors, qiov, offset);
+}
+
+static int nbd_co_writev(BlockDriverState *bs, int64_t sector_num,
+ int nb_sectors, QEMUIOVector *qiov)
+{
+int offset = 0;
+int ret;
+while (nb_sectors > NBD_MAX_SECTORS) {
+ret = nbd_co_writev_1(bs, sector_num, NBD_MAX_SECTORS, qiov, offset);
+if (ret < 0) {
+return ret;
+}
+offset += NBD_MAX_SECTORS * 512;
+sector_num += NBD_MAX_SECTORS;
+nb_sectors -= NBD_MAX_SECTORS;
+}
+return nbd_co_writev_1(bs, sector_num, nb_sectors, qiov, offset);
+}
+
static int nbd_co_flush(BlockDriverState *bs)
{
BDRVNBDState *s = bs->opaque;
--
1.7.6
[Qemu-devel] [PATCH v3 02/15] add socket_set_block
Cc: MORITA Kazutaka
Signed-off-by: Paolo Bonzini
---
oslib-posix.c |7 +++
oslib-win32.c |6 ++
qemu_socket.h |1 +
3 files changed, 14 insertions(+), 0 deletions(-)
diff --git a/oslib-posix.c b/oslib-posix.c
index a304fb0..dbc8ee8 100644
--- a/oslib-posix.c
+++ b/oslib-posix.c
@@ -103,6 +103,13 @@ void qemu_vfree(void *ptr)
free(ptr);
}
+void socket_set_block(int fd)
+{
+int f;
+f = fcntl(fd, F_GETFL);
+fcntl(fd, F_SETFL, f & ~O_NONBLOCK);
+}
+
void socket_set_nonblock(int fd)
{
int f;
diff --git a/oslib-win32.c b/oslib-win32.c
index 5f0759f..5e3de7d 100644
--- a/oslib-win32.c
+++ b/oslib-win32.c
@@ -73,6 +73,12 @@ void qemu_vfree(void *ptr)
VirtualFree(ptr, 0, MEM_RELEASE);
}
+void socket_set_block(int fd)
+{
+unsigned long opt = 0;
+ioctlsocket(fd, FIONBIO, &opt);
+}
+
void socket_set_nonblock(int fd)
{
unsigned long opt = 1;
diff --git a/qemu_socket.h b/qemu_socket.h
index 180e4db..9e32fac 100644
--- a/qemu_socket.h
+++ b/qemu_socket.h
@@ -35,6 +35,7 @@ int inet_aton(const char *cp, struct in_addr *ia);
/* misc helpers */
int qemu_socket(int domain, int type, int protocol);
int qemu_accept(int s, struct sockaddr *addr, socklen_t *addrlen);
+void socket_set_block(int fd);
void socket_set_nonblock(int fd);
int send_all(int fd, const void *buf, int len1);
--
1.7.6
[Qemu-devel] [PATCH v3 03/15] add qemu_send_full and qemu_recv_full
Signed-off-by: Paolo Bonzini
---
osdep.c | 67 +
qemu-common.h |4 +++
2 files changed, 71 insertions(+), 0 deletions(-)
diff --git a/osdep.c b/osdep.c
index 56e6963..718a25d 100644
--- a/osdep.c
+++ b/osdep.c
@@ -166,3 +166,70 @@ int qemu_accept(int s, struct sockaddr *addr, socklen_t
*addrlen)
return ret;
}
+
+/*
+ * A variant of send(2) which handles partial write.
+ *
+ * Return the number of bytes transferred, which is only
+ * smaller than `count' if there is an error.
+ *
+ * This function won't work with non-blocking fd's.
+ * Any of the possibilities with non-bloking fd's is bad:
+ * - return a short write (then name is wrong)
+ * - busy wait adding (errno == EAGAIN) to the loop
+ */
+ssize_t qemu_send_full(int fd, const void *buf, size_t count, int flags)
+{
+ssize_t ret = 0;
+ssize_t total = 0;
+
+while (count) {
+ret = send(fd, buf, count, flags);
+if (ret < 0) {
+if (errno == EINTR) {
+continue;
+}
+break;
+}
+
+count -= ret;
+buf += ret;
+total += ret;
+}
+
+return total;
+}
+
+/*
+ * A variant of recv(2) which handles partial write.
+ *
+ * Return the number of bytes transferred, which is only
+ * smaller than `count' if there is an error.
+ *
+ * This function won't work with non-blocking fd's.
+ * Any of the possibilities with non-bloking fd's is bad:
+ * - return a short write (then name is wrong)
+ * - busy wait adding (errno == EAGAIN) to the loop
+ */
+ssize_t qemu_recv_full(int fd, const void *buf, size_t count, int flags)
+{
+ssize_t ret = 0;
+ssize_t total = 0;
+
+while (count) {
+ret = recv(fd, buf, count, flags);
+if (ret <= 0) {
+if (ret < 0 && errno == EINTR) {
+continue;
+}
+break;
+}
+
+count -= ret;
+buf += ret;
+total += ret;
+}
+
+return total;
+}
+
diff --git a/qemu-common.h b/qemu-common.h
index 5e87bdf..ed9112a 100644
--- a/qemu-common.h
+++ b/qemu-common.h
@@ -189,6 +189,10 @@ void qemu_mutex_unlock_iothread(void);
int qemu_open(const char *name, int flags, ...);
ssize_t qemu_write_full(int fd, const void *buf, size_t count)
QEMU_WARN_UNUSED_RESULT;
+ssize_t qemu_send_full(int fd, const void *buf, size_t count, int flags)
+QEMU_WARN_UNUSED_RESULT;
+ssize_t qemu_recv_full(int fd, const void *buf, size_t count, int flags)
+QEMU_WARN_UNUSED_RESULT;
void qemu_set_cloexec(int fd);
#ifndef _WIN32
--
1.7.6
[Qemu-devel] [PATCH 1/6] vvfat: fix out of bounds array_get usage
When reading the address of the first free entry, you cannot use array_get without first marking all entries as occupied. This is visible if you change the sectors per cluster on a floppy from 2 to 1. Signed-off-by: Paolo Bonzini --- block/vvfat.c |1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/block/vvfat.c b/block/vvfat.c index f567c9a..cee3971 100644 --- a/block/vvfat.c +++ b/block/vvfat.c @@ -799,6 +799,7 @@ static int read_directory(BDRVVVFATState* s, int mapping_index) /* root directory */ int cur = s->directory.next; array_ensure_allocated(&(s->directory), ROOT_ENTRIES - 1); + s->directory.next = ROOT_ENTRIES; memset(array_get(&(s->directory), cur), 0, (ROOT_ENTRIES - cur) * sizeof(direntry_t)); } -- 1.7.6
[Qemu-devel] [PATCH v3 13/15] nbd: switch to asynchronous operation
Signed-off-by: Paolo Bonzini
---
block/nbd.c | 218 +++
nbd.c |8 ++
2 files changed, 152 insertions(+), 74 deletions(-)
diff --git a/block/nbd.c b/block/nbd.c
index 35c15c8..90c4f2f 100644
--- a/block/nbd.c
+++ b/block/nbd.c
@@ -53,6 +53,11 @@ typedef struct BDRVNBDState {
size_t blocksize;
char *export_name; /* An NBD server may export several devices */
+CoMutex mutex;
+Coroutine *coroutine;
+
+struct nbd_reply reply;
+
/* If it begins with '/', this is a UNIX domain socket. Otherwise,
* it's a string of the form :port
*/
@@ -105,6 +110,95 @@ out:
return err;
}
+static void nbd_coroutine_start(BDRVNBDState *s, struct nbd_request *request)
+{
+qemu_co_mutex_lock(&s->mutex);
+s->coroutine = qemu_coroutine_self();
+request->handle = (uint64_t)(intptr_t)s;
+}
+
+static int nbd_have_request(void *opaque)
+{
+BDRVNBDState *s = opaque;
+
+return !!s->coroutine;
+}
+
+static void nbd_reply_ready(void *opaque)
+{
+BDRVNBDState *s = opaque;
+
+if (s->reply.handle == 0) {
+/* No reply already in flight. Fetch a header. */
+if (nbd_receive_reply(s->sock, &s->reply) < 0) {
+s->reply.handle = 0;
+}
+}
+
+/* There's no need for a mutex on the receive side, because the
+ * handler acts as a synchronization point and ensures that only
+ * one coroutine is called until the reply finishes. */
+if (s->coroutine) {
+qemu_coroutine_enter(s->coroutine, NULL);
+}
+}
+
+static void nbd_restart_write(void *opaque)
+{
+BDRVNBDState *s = opaque;
+qemu_coroutine_enter(s->coroutine, NULL);
+}
+
+static int nbd_co_send_request(BDRVNBDState *s, struct nbd_request *request,
+ struct iovec *iov, int offset)
+{
+int rc, ret;
+
+qemu_aio_set_fd_handler(s->sock, nbd_reply_ready, nbd_restart_write,
+nbd_have_request, NULL, s);
+rc = nbd_send_request(s->sock, request);
+if (rc != -1 && iov) {
+ret = qemu_co_sendv(s->sock, iov, request->len, offset);
+if (ret != request->len) {
+errno = -EIO;
+rc = -1;
+}
+}
+qemu_aio_set_fd_handler(s->sock, nbd_reply_ready, NULL,
+nbd_have_request, NULL, s);
+return rc;
+}
+
+static void nbd_co_receive_reply(BDRVNBDState *s, struct nbd_request *request,
+ struct nbd_reply *reply,
+ struct iovec *iov, int offset)
+{
+int ret;
+
+/* Wait until we're woken up by the read handler. */
+qemu_coroutine_yield();
+*reply = s->reply;
+if (reply->handle != request->handle) {
+reply->error = EIO;
+} else {
+if (iov && reply->error == 0) {
+ret = qemu_co_recvv(s->sock, iov, request->len, offset);
+if (ret != request->len) {
+reply->error = EIO;
+}
+}
+
+/* Tell the read handler to read another header. */
+s->reply.handle = 0;
+}
+}
+
+static void nbd_coroutine_end(BDRVNBDState *s, struct nbd_request *request)
+{
+s->coroutine = NULL;
+qemu_co_mutex_unlock(&s->mutex);
+}
+
static int nbd_establish_connection(BlockDriverState *bs)
{
BDRVNBDState *s = bs->opaque;
@@ -134,8 +228,11 @@ static int nbd_establish_connection(BlockDriverState *bs)
return -errno;
}
-/* Now that we're connected, set the socket to be non-blocking */
+/* Now that we're connected, set the socket to be non-blocking and
+ * kick the reply mechanism. */
socket_set_nonblock(sock);
+qemu_aio_set_fd_handler(s->sock, nbd_reply_ready, NULL,
+nbd_have_request, NULL, s);
s->sock = sock;
s->size = size;
@@ -151,11 +248,11 @@ static void nbd_teardown_connection(BlockDriverState *bs)
struct nbd_request request;
request.type = NBD_CMD_DISC;
-request.handle = (uint64_t)(intptr_t)bs;
request.from = 0;
request.len = 0;
nbd_send_request(s->sock, &request);
+qemu_aio_set_fd_handler(s->sock, NULL, NULL, NULL, NULL, NULL);
closesocket(s->sock);
}
@@ -164,6 +261,8 @@ static int nbd_open(BlockDriverState *bs, const char*
filename, int flags)
BDRVNBDState *s = bs->opaque;
int result;
+qemu_co_mutex_init(&s->mutex);
+
/* Pop the config into our state object. Exit if invalid. */
result = nbd_config(s, filename, flags);
if (result != 0) {
@@ -178,38 +277,30 @@ static int nbd_open(BlockDriverState *bs, const char*
filename, int flags)
return result;
}
-static int nbd_read(BlockDriverState *bs, int64_t sector_num,
-uint8_t *buf, int nb_sectors)
+static int nbd_co_readv(BlockDriverState *bs, int64_t sector_num,
+int nb_sectors, QEMUIOVector *qiov)
{
BDRVNBDState *s = bs->opaque;
struct nb
[Qemu-devel] [PATCH 2/6] vvfat: do not fail if the disk has spare sectors
If the number of "faked sectors" + the number of sectors that are
part of a cluster does not sum up to the total number of sectors,
qemu-img convert fails. Read these spare sectors as all zeros.
Signed-off-by: Paolo Bonzini
---
block/vvfat.c |4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/block/vvfat.c b/block/vvfat.c
index cee3971..bc2dd4c 100644
--- a/block/vvfat.c
+++ b/block/vvfat.c
@@ -1294,7 +1294,7 @@ static int vvfat_read(BlockDriverState *bs, int64_t
sector_num,
int i;
for(i=0;i= s->sector_count)
+ if (sector_num >= bs->total_sectors)
return -1;
if (s->qcow) {
int n;
@@ -1320,7 +1320,7 @@ DLOG(fprintf(stderr, "sector %d not allocated\n",
(int)sector_num));
uint32_t sector=sector_num-s->faked_sectors,
sector_offset_in_cluster=(sector%s->sectors_per_cluster),
cluster_num=sector/s->sectors_per_cluster;
- if(read_cluster(s, cluster_num) != 0) {
+ if(cluster_num > s->cluster_count || read_cluster(s, cluster_num)
!= 0) {
/* LATER TODO: strict: return -1; */
memset(buf+i*0x200,0,0x200);
continue;
--
1.7.6
[Qemu-devel] [PATCH v3 04/15] sheepdog: move coroutine send/recv function to generic code
Outside coroutines, avoid busy waiting on EAGAIN by temporarily
making the socket blocking.
The API of qemu_recvv/qemu_sendv is slightly different from
do_readv/do_writev because they do not handle coroutines. It
returns the number of bytes written before encountering an
EAGAIN. The specificity of yielding on EAGAIN is entirely in
qemu-coroutine.c.
Reviewed-by: MORITA Kazutaka
Signed-off-by: Paolo Bonzini
---
Makefile.objs |2 +-
block/sheepdog.c| 230 +--
cutils.c| 111 +
osdep.c |4 +-
qemu-common.h | 32 +++-
qemu-coroutine-io.c | 96 +
6 files changed, 262 insertions(+), 213 deletions(-)
create mode 100644 qemu-coroutine-io.c
diff --git a/Makefile.objs b/Makefile.objs
index 8d23fbb..55fd634 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -12,7 +12,7 @@ oslib-obj-$(CONFIG_POSIX) += oslib-posix.o qemu-thread-posix.o
###
# coroutines
-coroutine-obj-y = qemu-coroutine.o qemu-coroutine-lock.o
+coroutine-obj-y = qemu-coroutine.o qemu-coroutine-lock.o qemu-coroutine-io.o
ifeq ($(CONFIG_UCONTEXT_COROUTINE),y)
coroutine-obj-$(CONFIG_POSIX) += coroutine-ucontext.o
else
diff --git a/block/sheepdog.c b/block/sheepdog.c
index af696a5..d168681 100644
--- a/block/sheepdog.c
+++ b/block/sheepdog.c
@@ -443,129 +443,6 @@ static SheepdogAIOCB *sd_aio_setup(BlockDriverState *bs,
QEMUIOVector *qiov,
return acb;
}
-#ifdef _WIN32
-
-struct msghdr {
-struct iovec *msg_iov;
-size_tmsg_iovlen;
-};
-
-static ssize_t sendmsg(int s, const struct msghdr *msg, int flags)
-{
-size_t size = 0;
-char *buf, *p;
-int i, ret;
-
-/* count the msg size */
-for (i = 0; i < msg->msg_iovlen; i++) {
-size += msg->msg_iov[i].iov_len;
-}
-buf = g_malloc(size);
-
-p = buf;
-for (i = 0; i < msg->msg_iovlen; i++) {
-memcpy(p, msg->msg_iov[i].iov_base, msg->msg_iov[i].iov_len);
-p += msg->msg_iov[i].iov_len;
-}
-
-ret = send(s, buf, size, flags);
-
-g_free(buf);
-return ret;
-}
-
-static ssize_t recvmsg(int s, struct msghdr *msg, int flags)
-{
-size_t size = 0;
-char *buf, *p;
-int i, ret;
-
-/* count the msg size */
-for (i = 0; i < msg->msg_iovlen; i++) {
-size += msg->msg_iov[i].iov_len;
-}
-buf = g_malloc(size);
-
-ret = qemu_recv(s, buf, size, flags);
-if (ret < 0) {
-goto out;
-}
-
-p = buf;
-for (i = 0; i < msg->msg_iovlen; i++) {
-memcpy(msg->msg_iov[i].iov_base, p, msg->msg_iov[i].iov_len);
-p += msg->msg_iov[i].iov_len;
-}
-out:
-g_free(buf);
-return ret;
-}
-
-#endif
-
-/*
- * Send/recv data with iovec buffers
- *
- * This function send/recv data from/to the iovec buffer directly.
- * The first `offset' bytes in the iovec buffer are skipped and next
- * `len' bytes are used.
- *
- * For example,
- *
- * do_send_recv(sockfd, iov, len, offset, 1);
- *
- * is equals to
- *
- * char *buf = malloc(size);
- * iov_to_buf(iov, iovcnt, buf, offset, size);
- * send(sockfd, buf, size, 0);
- * free(buf);
- */
-static int do_send_recv(int sockfd, struct iovec *iov, int len, int offset,
-int write)
-{
-struct msghdr msg;
-int ret, diff;
-
-memset(&msg, 0, sizeof(msg));
-msg.msg_iov = iov;
-msg.msg_iovlen = 1;
-
-len += offset;
-
-while (iov->iov_len < len) {
-len -= iov->iov_len;
-
-iov++;
-msg.msg_iovlen++;
-}
-
-diff = iov->iov_len - len;
-iov->iov_len -= diff;
-
-while (msg.msg_iov->iov_len <= offset) {
-offset -= msg.msg_iov->iov_len;
-
-msg.msg_iov++;
-msg.msg_iovlen--;
-}
-
-msg.msg_iov->iov_base = (char *) msg.msg_iov->iov_base + offset;
-msg.msg_iov->iov_len -= offset;
-
-if (write) {
-ret = sendmsg(sockfd, &msg, 0);
-} else {
-ret = recvmsg(sockfd, &msg, 0);
-}
-
-msg.msg_iov->iov_base = (char *) msg.msg_iov->iov_base - offset;
-msg.msg_iov->iov_len += offset;
-
-iov->iov_len += diff;
-return ret;
-}
-
static int connect_to_sdog(const char *addr, const char *port)
{
char hbuf[NI_MAXHOST], sbuf[NI_MAXSERV];
@@ -618,83 +495,19 @@ success:
return fd;
}
-static int do_readv_writev(int sockfd, struct iovec *iov, int len,
- int iov_offset, int write)
-{
-int ret;
-again:
-ret = do_send_recv(sockfd, iov, len, iov_offset, write);
-if (ret < 0) {
-if (errno == EINTR) {
-goto again;
-}
-if (errno == EAGAIN) {
-if (qemu_in_coroutine()) {
-qemu_coroutine_yield();
-}
-goto again;
-}
-error_report("failed to recv a rsp, %s", strerror(errno));
-return 1;
-}
-
-iov_offset += ret;
-len -=
Re: [Qemu-devel] [PATCH 1/2] linux-user: Remove unused code
On Mon, Oct 03, 2011 at 10:43:19PM +0200, Stefan Weil wrote: > The code is unused since 8 years, so remove it. > > Signed-off-by: Stefan Weil > --- > linux-user/signal.c |5 + > 1 files changed, 1 insertions(+), 4 deletions(-) Thanks, applied to the trivial patches tree: http://repo.or.cz/w/qemu/stefanha.git/shortlog/refs/heads/trivial-patches Stefan
[Qemu-devel] [PATCH 3/6] vvfat: need to use first_sectors_number to distinguish fdd/hdd
This is consistent with what "real" floppies have, so file(1) now actually recognizes the VVFAT image as a 1.44 MB floppy. Signed-off-by: Paolo Bonzini --- block/vvfat.c |4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/block/vvfat.c b/block/vvfat.c index bc2dd4c..eb91642 100644 --- a/block/vvfat.c +++ b/block/vvfat.c @@ -968,7 +968,7 @@ static int init_directories(BDRVVVFATState* s, bootsector->number_of_fats=0x2; /* number of FATs */ bootsector->root_entries=cpu_to_le16(s->sectors_of_root_directory*0x10); bootsector->total_sectors16=s->sector_count>0x?0:cpu_to_le16(s->sector_count); - bootsector->media_type=(s->fat_type!=12?0xf8:s->sector_count==5760?0xf9:0xf8); /* media descriptor */ +bootsector->media_type=(s->first_sectors_number>1?0xf8:0xf0); /* media descriptor (f8=hd, f0=3.5 fd)*/ s->fat.pointer[0] = bootsector->media_type; bootsector->sectors_per_fat=cpu_to_le16(s->sectors_per_fat); bootsector->sectors_per_track=cpu_to_le16(s->bs->secs); @@ -977,7 +977,7 @@ static int init_directories(BDRVVVFATState* s, bootsector->total_sectors=cpu_to_le32(s->sector_count>0x?s->sector_count:0); /* LATER TODO: if FAT32, this is wrong */ -bootsector->u.fat16.drive_number=s->fat_type==12?0:0x80; /* assume this is hda (TODO) */ +bootsector->u.fat16.drive_number=s->first_sectors_number==1?0:0x80; /* fda=0, hda=0x80 */ bootsector->u.fat16.current_head=0; bootsector->u.fat16.signature=0x29; bootsector->u.fat16.id=cpu_to_le32(0xfabe1afd); -- 1.7.6
