Re: [Qemu-devel] [PATCH 3/6] convert pci-host to QOM

2012-03-26 Thread Stefan Hajnoczi
On Mon, Mar 26, 2012 at 10:06:45AM +0800, Wanpeng Li wrote:
> diff --git a/hw/pci_host.c b/hw/pci_host.c
> index 44c6c20..44d7e55 100644
> --- a/hw/pci_host.c
> +++ b/hw/pci_host.c
> @@ -162,4 +162,30 @@ const MemoryRegionOps pci_host_data_be_ops = {
>  .endianness = DEVICE_BIG_ENDIAN,
>  };
>  
> +void pci_host_set_mmio(PCIHostState *s, MemoryRegion *value)
> +{
> + object_property_set_link(OBJECT(s), OBJECT(value), "mmio", NULL);
> +}
> +
> +static void pci_host_initfn(Object *obj)
> +{
> +PCIHostState *s = PCI_HOST(obj);
> +
> + object_property_add_link(obj, "mmio", TYPE_MEMORY_REGION,
> +  (Object **)&s->address_space, 
> NULL);

This patch has tabs instead of 4-space indent.



[Qemu-devel] [PATCH 3/6] convert pci-host to QOM

2012-03-26 Thread y
From: Wanpeng Li 


From: Anthony Liguori 


Signed-off-by: Anthony Liguori 
Signed-off-by: Wanpeng Li 

---
 hw/pci_host.c |   26 ++
 hw/pci_host.h |5 +
 2 files changed, 31 insertions(+), 0 deletions(-)

diff --git a/hw/pci_host.c b/hw/pci_host.c
index 44c6c20..44d7e55 100644
--- a/hw/pci_host.c
+++ b/hw/pci_host.c
@@ -162,4 +162,30 @@ const MemoryRegionOps pci_host_data_be_ops = {
 .endianness = DEVICE_BIG_ENDIAN,
 };
 
+void pci_host_set_mmio(PCIHostState *s, MemoryRegion *value)
+{
+object_property_set_link(OBJECT(s), OBJECT(value), "mmio", NULL);
+}
+
+static void pci_host_initfn(Object *obj)
+{
+PCIHostState *s = PCI_HOST(obj);
+
+object_property_add_link(obj, "mmio", TYPE_MEMORY_REGION,
+(Object **)&s->address_space, 
NULL);
+}
+
+static TypeInfo pci_host_type = {
+.name = TYPE_PCI_HOST,
+.parent = TYPE_SYS_BUS_DEVICE,
+.instance_size = sizeof(PCIHostState),
+.instance_init = pci_host_initfn,
+};
+
+static void register_devices(void)
+{
+type_register_static(&pci_host_type);
+}
+
+type_init(register_devices);
 
diff --git a/hw/pci_host.h b/hw/pci_host.h
index 359e38f..084e15c 100644
--- a/hw/pci_host.h
+++ b/hw/pci_host.h
@@ -30,6 +30,9 @@
 
 #include "sysbus.h"
 
+#define TYPE_PCI_HOST "pci-host"
+#define PCI_HOST(obj) OBJECT_CHECK(PCIHostState, (obj), TYPE_PCI_HOST)
+
 struct PCIHostState {
 SysBusDevice busdev;
 MemoryRegion conf_mem;
@@ -49,6 +52,8 @@ uint32_t pci_host_config_read_common(PCIDevice *pci_dev, 
uint32_t addr,
 void pci_data_write(PCIBus *s, uint32_t addr, uint32_t val, int len);
 uint32_t pci_data_read(PCIBus *s, uint32_t addr, int len);
 
+void pci_host_set_mmio(PCIHostState *s, MemoryRegion *value);
+
 extern const MemoryRegionOps pci_host_conf_le_ops;
 extern const MemoryRegionOps pci_host_conf_be_ops;
 extern const MemoryRegionOps pci_host_data_le_ops;
-- 
1.7.5.4




[Qemu-devel] Ignoring errno makes QMP errors suck

2012-03-26 Thread Kevin Wolf
Hi,

I keep getting reports of problems, with nice error descriptions that
usually look very similar to what I produced here:

{"execute":"blockdev-snapshot-sync","arguments":{"device":"ide0-hd0","snapshot-file":"/tmp/backing.qcow2"}}
{"error": {"class": "OpenFileFailed", "desc": "Could not open
'/tmp/backing.qcow2'", "data": {"filename": "/tmp/backing.qcow2"}}}

Who can tell me what has happened here? Oh, yes, the command failed, I
would have guessed that from the "error" key. But the actual error
description is as useless as it gets. It doesn't tell me anything about
_why_ the snapshot couldn't be created. ("Permission denied" would have
been the helpful additional information in this case)

How should management tools ever be able to provide a helpful error
message to their users if all they get is this useless "something went
wrong" error?

Kevin



Re: [Qemu-devel] Windows Virtio Issue

2012-03-26 Thread Vadim Rozenfeld
On Sunday, March 25, 2012 07:01:54 PM Yan Vugenfirer wrote:
Hi Paul,
Could you try reproducing this problem on "-smp 2" guest, with
small memory dump option turned on, instead of kernel memory dump.

Thanks,
Vadim.

> Hello Paul,
> 
> Vadim is the owner of virtio-block Windows driver. He will try to help you.
> 
> Best regards,
> Yan.
> 
> On Mar 23, 2012, at 7:32 PM, Paul Fisher wrote:
> > Dear Yan,
> > 
> > We seem to be having some trouble with virtio disk on Windows Server 2008
> > R2 running on qemu-kvm. Essentially, when disk IO is stressed, it seems
> > to blue screen.
> > 
> > 
> > These are potentially contended disks, since it's public cloud with
> > multiple customers on the host - the issue could be connected to
> > contention making disk response slow?
> > 
> > Pertinent facts:
> > 
> > * qemu-kvm 1.0
> > 
> > * Hosts have linux kernel 3.2.2, 64 bit, AMD Opteron 6128
> > 
> > * VM is Windows Server 2008 R2, 64 bit - all patches downloaded from MS
> > 
> > * Virtio disk drivers are the February 2012 release .2200. Problem is
> > apparent with the older .2000 driver as well.
> > 
> > * Blue screen shows failure in viostor.sys - screen cap and minidump are
> > at http://dl.dropbox.com/u/12332019/2200BSOD.zip
> > 
> > 
> > * Replication: Yes. Easily. In this case I stressed the disk by running
> > Crystal Disk mark (a free IO measuring tool). When it got to the 4 KB
> > random seek tests - I think it was on write - it would consistently blue
> > screen.
> > 
> > * If it would help, I can provide remote access to this virtual machine
> > if you wish - it's in our cloud system.
> > 
> > * qemu-kvm command line:
> > 
> > 
> > 
> > qemu-kvm -m MEMORY -smp SMP -cpu host -nodefaults -vga cirrus -vnc :1 
> > -drive if=none,id=block.0,format=raw, cache=writeback,file=drive.img
> > -device virtio-blk-pci,bootindex=1, drive=block.0 -monitor stdio
> > 
> > Any help would be greatly appreciated.
> > 
> > Regards,
> > Paul Fisher
> > Operations Manager
> > ElasticHosts Ltd



[Qemu-devel] [PATCH 3/6] convert pci-host to QOM

2012-03-26 Thread Wanpeng Li

From: Anthony Liguori 


Signed-off-by: Anthony Liguori 
Signed-off-by: Wanpeng Li 

---
 hw/pci_host.c |   26 ++
 hw/pci_host.h |5 +
 2 files changed, 31 insertions(+), 0 deletions(-)

diff --git a/hw/pci_host.c b/hw/pci_host.c
index 44c6c20..44d7e55 100644
--- a/hw/pci_host.c
+++ b/hw/pci_host.c
@@ -162,4 +162,30 @@ const MemoryRegionOps pci_host_data_be_ops = {
 .endianness = DEVICE_BIG_ENDIAN,
 };
 
+void pci_host_set_mmio(PCIHostState *s, MemoryRegion *value)
+{
+object_property_set_link(OBJECT(s), OBJECT(value), "mmio", NULL);
+}
+
+static void pci_host_initfn(Object *obj)
+{
+PCIHostState *s = PCI_HOST(obj);
+
+object_property_add_link(obj, "mmio", TYPE_MEMORY_REGION,
+(Object **)&s->address_space, 
NULL);
+}
+
+static TypeInfo pci_host_type = {
+.name = TYPE_PCI_HOST,
+.parent = TYPE_SYS_BUS_DEVICE,
+.instance_size = sizeof(PCIHostState),
+.instance_init = pci_host_initfn,
+};
+
+static void register_devices(void)
+{
+type_register_static(&pci_host_type);
+}
+
+type_init(register_devices);
 
diff --git a/hw/pci_host.h b/hw/pci_host.h
index 359e38f..084e15c 100644
--- a/hw/pci_host.h
+++ b/hw/pci_host.h
@@ -30,6 +30,9 @@
 
 #include "sysbus.h"
 
+#define TYPE_PCI_HOST "pci-host"
+#define PCI_HOST(obj) OBJECT_CHECK(PCIHostState, (obj), TYPE_PCI_HOST)
+
 struct PCIHostState {
 SysBusDevice busdev;
 MemoryRegion conf_mem;
@@ -49,6 +52,8 @@ uint32_t pci_host_config_read_common(PCIDevice *pci_dev, 
uint32_t addr,
 void pci_data_write(PCIBus *s, uint32_t addr, uint32_t val, int len);
 uint32_t pci_data_read(PCIBus *s, uint32_t addr, int len);
 
+void pci_host_set_mmio(PCIHostState *s, MemoryRegion *value);
+
 extern const MemoryRegionOps pci_host_conf_le_ops;
 extern const MemoryRegionOps pci_host_conf_be_ops;
 extern const MemoryRegionOps pci_host_data_le_ops;
-- 
1.7.5.4




Re: [Qemu-devel] [libvirt] Modern CPU models cannot be used with libvirt

2012-03-26 Thread Avi Kivity
On 03/25/2012 08:11 PM, Anthony Liguori wrote:
>
>> I don't think -nodefconfig (as defined) is usable, since there is no way
>> for the user to tell what it means short of reading those files.
>
> *if the user doesn't know specifics about this QEMU version.
>
> You make the assumption that all users are going to throw arbitrary
> options at arbitrary QEMU versions.  That's certainly an important
> use-case but it's not the only one.

If a Fedora user is using qemu, then their qemu version will change
every six months.  Their options are to update their scripts/management
tool in step, or not have their management tool use -nodefconfig.

The same holds for anyone using qemu from upstream, since that's
approximately the qemu release cycle.

>
>> -no-user-config is usable, I think it needs also to mean that qemu
>> without -M/-cpu/-m options will error out?
>
> You're confusing -nodefaults (or something stronger than -nodefaults)
> with -no-user-config.
>

Right.

> Yes, the distinctions are confusing.  It's not all fixable tomorrow. 
> If we take my config refactoring series, we can get 90% of the way
> there soon but Paolo has a more thorough refactoring..
>
 "#define westmere blah" is not configuration, otherwise the meaning of
 configuration will drift over time.

 -cpu blah is, of course.
>>>
>>> It's the same mechanism, but the above would create two classes of
>>> default configuration files and then it becomes a question of how
>>> they're used.
>>
>> Confused.
>
> We don't have a formal concept of -read-definition-config and
> -read-configuration-config
>
> There's no easy or obvious way to create such a concept either nor do
> I think the distinction is meaningful to users.

Definition files should be invisible to users.  They're part of the
implementation.  If we have a file that says

  pc-1.1 = piix + cirrus + memory(128) + ...

then it's nobody's business if it's in a text file or a .c file.

Of course it's  nice to allow users to load their own definition files,
but that's strictly a convenience.

>> Exactly.  The types are no different, so there's no reason to
>> discriminate against types that happen to live in qemu-provided data
>> files vs. qemu code.  They aren't instantiated, so we lose nothing by
>> creating the factories (just so long as the factories aren't
>> mass-producing objects).
>
>
> At some point, I'd like to have type modules that are shared objects. 
> I'd like QEMU to start with almost no builtin types and allow the user
> to configure which modules get loaded.
>
> In the long term, I'd like QEMU to be a small, robust core with the
> vast majority of code relegated to modules with the user ultimately in
> control of module loading.
>
> Yes, I'd want some module autoloading system but there should always
> be a way to launch QEMU without loading any modules and then load a
> very specific set of modules (as defined by the user).
>
> You can imagine this being useful for something like Common Criteria
> certifications.

Okay.

> It's obviously defined for a given release, just not defined long term.
>
>> If I see something like -nodefconfig, I assume it will create a bare
>> bones guest that will not depend on any qemu defaults and will be stable
>> across releases.
>
> That's not even close to what -nodefconfig is.  That's pretty much
> what -nodefaults is but -nodefaults has also had a fluid definition
> historically.

Okay.  Let's just make sure to document -nodefconfig as version specific
and -nodefaults as the stable way to create a bare bones guest (and
define exactly what that means).

-- 
error compiling committee.c: too many arguments to function




Re: [Qemu-devel] [PATCH 3/6] convert pci-host to QOM

2012-03-26 Thread Wanpeng Li
On Mon, Mar 26, 2012 at 10:06:45AM +0800, Wanpeng Li wrote:
>
>From: Anthony Liguori 
>
>
>Signed-off-by: Anthony Liguori 
>Signed-off-by: Wanpeng Li 
>
>---
> hw/pci_host.c |   26 ++
> hw/pci_host.h |5 +
> 2 files changed, 31 insertions(+), 0 deletions(-)
>
>diff --git a/hw/pci_host.c b/hw/pci_host.c
>index 44c6c20..44d7e55 100644
>--- a/hw/pci_host.c
>+++ b/hw/pci_host.c
>@@ -162,4 +162,30 @@ const MemoryRegionOps pci_host_data_be_ops = {
> .endianness = DEVICE_BIG_ENDIAN,
> };
>
>+void pci_host_set_mmio(PCIHostState *s, MemoryRegion *value)
>+{
>+  object_property_set_link(OBJECT(s), OBJECT(value), "mmio", NULL);
>+}
>+
>+static void pci_host_initfn(Object *obj)
>+{
>+PCIHostState *s = PCI_HOST(obj);
>+
>+  object_property_add_link(obj, "mmio", TYPE_MEMORY_REGION,
>+   (Object **)&s->address_space, 
>NULL);
>+}
>+
>+static TypeInfo pci_host_type = {
>+.name = TYPE_PCI_HOST,
>+.parent = TYPE_SYS_BUS_DEVICE,
>+  .instance_size = sizeof(PCIHostState),
>+  .instance_init = pci_host_initfn,
>+};
>+
>+static void register_devices(void)
>+{
>+  type_register_static(&pci_host_type);
>+}
>+
>+type_init(register_devices);
>
>diff --git a/hw/pci_host.h b/hw/pci_host.h
>index 359e38f..084e15c 100644
>--- a/hw/pci_host.h
>+++ b/hw/pci_host.h
>@@ -30,6 +30,9 @@
>
> #include "sysbus.h"
>
>+#define TYPE_PCI_HOST "pci-host"
>+#define PCI_HOST(obj) OBJECT_CHECK(PCIHostState, (obj), TYPE_PCI_HOST)
>+
> struct PCIHostState {
> SysBusDevice busdev;
> MemoryRegion conf_mem;
>@@ -49,6 +52,8 @@ uint32_t pci_host_config_read_common(PCIDevice *pci_dev, 
>uint32_t addr,
> void pci_data_write(PCIBus *s, uint32_t addr, uint32_t val, int len);
> uint32_t pci_data_read(PCIBus *s, uint32_t addr, int len);
>
>+void pci_host_set_mmio(PCIHostState *s, MemoryRegion *value);
>+
> extern const MemoryRegionOps pci_host_conf_le_ops;
> extern const MemoryRegionOps pci_host_conf_be_ops;
> extern const MemoryRegionOps pci_host_data_le_ops;
>-- 
>1.7.5.4
>


>From 72bc193e6e25cb393437317843a701b82a9b9233 Mon Sep 17 00:00:00 2001
From: Wanpeng Li 
Date: Thu, 22 Mar 2012 17:57:30 +0800
Subject: [PATCH 3/6] convert pci-host to QOM


From: Anthony Liguori 


Signed-off-by: Anthony Liguori 
Signed-off-by: Wanpeng Li 

---
 hw/pci_host.c |   26 ++
 hw/pci_host.h |5 +
 2 files changed, 31 insertions(+), 0 deletions(-)

diff --git a/hw/pci_host.c b/hw/pci_host.c
index 44c6c20..44d7e55 100644
--- a/hw/pci_host.c
+++ b/hw/pci_host.c
@@ -162,4 +162,30 @@ const MemoryRegionOps pci_host_data_be_ops = {
 .endianness = DEVICE_BIG_ENDIAN,
 };
 
+void pci_host_set_mmio(PCIHostState *s, MemoryRegion *value)
+{
+object_property_set_link(OBJECT(s), OBJECT(value), "mmio", NULL);
+}
+
+static void pci_host_initfn(Object *obj)
+{
+PCIHostState *s = PCI_HOST(obj);
+
+object_property_add_link(obj, "mmio", TYPE_MEMORY_REGION,
+	 (Object **)&s->address_space, NULL);
+}
+
+static TypeInfo pci_host_type = {
+.name = TYPE_PCI_HOST,
+.parent = TYPE_SYS_BUS_DEVICE,
+.instance_size = sizeof(PCIHostState),
+.instance_init = pci_host_initfn,
+};
+
+static void register_devices(void)
+{
+type_register_static(&pci_host_type);
+}
+
+type_init(register_devices);
 
diff --git a/hw/pci_host.h b/hw/pci_host.h
index 359e38f..084e15c 100644
--- a/hw/pci_host.h
+++ b/hw/pci_host.h
@@ -30,6 +30,9 @@
 
 #include "sysbus.h"
 
+#define TYPE_PCI_HOST "pci-host"
+#define PCI_HOST(obj) OBJECT_CHECK(PCIHostState, (obj), TYPE_PCI_HOST)
+
 struct PCIHostState {
 SysBusDevice busdev;
 MemoryRegion conf_mem;
@@ -49,6 +52,8 @@ uint32_t pci_host_config_read_common(PCIDevice *pci_dev, uint32_t addr,
 void pci_data_write(PCIBus *s, uint32_t addr, uint32_t val, int len);
 uint32_t pci_data_read(PCIBus *s, uint32_t addr, int len);
 
+void pci_host_set_mmio(PCIHostState *s, MemoryRegion *value);
+
 extern const MemoryRegionOps pci_host_conf_le_ops;
 extern const MemoryRegionOps pci_host_conf_be_ops;
 extern const MemoryRegionOps pci_host_data_le_ops;
-- 
1.7.5.4



Re: [Qemu-devel] Build broken -- qemu-ga: add guest-network-get-interfaces command

2012-03-26 Thread Michal Privoznik
On 25.03.2012 03:19, Brad Smith wrote:
> On 20/03/12 9:28 AM, Brad Smith wrote:
>> On 20/03/12 6:14 AM, Michal Privoznik wrote:
>>> On 18.03.2012 02:09, Brad Smith wrote:
 Michal,

 http://git.qemu.org/?p=qemu.git;a=commit;h=3424fc9f16a1e7d1c48eb6d605eb0ca63e199ec2




 This broke the build. Un-break the tree.

>>>
>>>
>>> Can you please be more specific? It works for me so I don't have a clue
>>> what are you referring to. I mean, what compiler do you use, what errors
>>> are thrown, etc.
>>>
>>> Michal
>>
>> The patch commited is full of Linux specific code.
> 
> This is *STILL BROKEN*. If you can't get a fix in in a timely manner
> then REVERT
> the broken commit.
> 

Brad,

that's because the fix hasn't been merged yet. If you need fix very
quickly you can apply this patch:

http://lists.nongnu.org/archive/html/qemu-devel/2012-03/msg04054.html

Michal



[Qemu-devel] [PATCH] pc: reduce duplication in compat machine types

2012-03-26 Thread Michael S. Tsirkin
Make it easier to add compat properties, by
adding macros for properties duplicated across
machine types.

Note: there could be bugs in compat properties,
this patch does not attempt to address them,
the code is bug for bug identical to the original.

Tested by: generated a preprocessed file, sorted and
compared to sorted original.
Lightly tested on x86_64.

Signed-off-by: Michael S. Tsirkin 
---

I've put the above on my branch as it's needed for
fixing balloon pci class. Let me know if there are
any objections.

 hw/pc_piix.c |  288 +++---
 1 files changed, 73 insertions(+), 215 deletions(-)

diff --git a/hw/pc_piix.c b/hw/pc_piix.c
index 3f99f9a..e39ef69 100644
--- a/hw/pc_piix.c
+++ b/hw/pc_piix.c
@@ -372,50 +372,69 @@ static QEMUMachine pc_machine_v1_1 = {
 .is_default = 1,
 };
 
+#define PC_COMPAT_1_0 \
+{\
+.driver   = "pc-sysfw",\
+.property = "rom_only",\
+.value= stringify(1),\
+}, {\
+.driver   = "isa-fdc",\
+.property = "check_media_rate",\
+.value= "off",\
+}
+
 static QEMUMachine pc_machine_v1_0 = {
 .name = "pc-1.0",
 .desc = "Standard PC",
 .init = pc_init_pci,
 .max_cpus = 255,
 .compat_props = (GlobalProperty[]) {
-{
-.driver   = "pc-sysfw",
-.property = "rom_only",
-.value= stringify(1),
-}, {
-.driver   = "isa-fdc",
-.property = "check_media_rate",
-.value= "off",
-},
+PC_COMPAT_1_0,
 { /* end of list */ }
 },
 };
 
+#define PC_COMPAT_0_15 \
+PC_COMPAT_1_0
+
 static QEMUMachine pc_machine_v0_15 = {
 .name = "pc-0.15",
 .desc = "Standard PC",
 .init = pc_init_pci,
 .max_cpus = 255,
 .compat_props = (GlobalProperty[]) {
-{
-.driver   = "pc-sysfw",
-.property = "rom_only",
-.value= stringify(1),
-}, {
-.driver   = "isa-fdc",
-.property = "check_media_rate",
-.value= "off",
-},
+PC_COMPAT_0_15,
 { /* end of list */ }
 },
 };
 
+#define PC_COMPAT_0_14 \
+PC_COMPAT_0_15,\
+{\
+.driver   = "virtio-blk-pci",\
+.property = "event_idx",\
+.value= "off",\
+},{\
+.driver   = "virtio-serial-pci",\
+.property = "event_idx",\
+.value= "off",\
+},{\
+.driver   = "virtio-net-pci",\
+.property = "event_idx",\
+.value= "off",\
+},{\
+.driver   = "virtio-balloon-pci",\
+.property = "event_idx",\
+.value= "off",\
+}
+
 static QEMUMachine pc_machine_v0_14 = {
 .name = "pc-0.14",
 .desc = "Standard PC",
 .init = pc_init_pci,
 .max_cpus = 255,
 .compat_props = (GlobalProperty[]) {
+PC_COMPAT_0_14, 
 {
 .driver   = "qxl",
 .property = "revision",
@@ -424,42 +443,30 @@ static QEMUMachine pc_machine_v0_14 = {
 .driver   = "qxl-vga",
 .property = "revision",
 .value= stringify(2),
-},{
-.driver   = "virtio-blk-pci",
-.property = "event_idx",
-.value= "off",
-},{
-.driver   = "virtio-serial-pci",
-.property = "event_idx",
-.value= "off",
-},{
-.driver   = "virtio-net-pci",
-.property = "event_idx",
-.value= "off",
-},{
-.driver   = "virtio-balloon-pci",
-.property = "event_idx",
-.value= "off",
-},{
-.driver   = "isa-fdc",
-.property = "check_media_rate",
-.value= "off",
-},
-{
-.driver   = "pc-sysfw",
-.property = "rom_only",
-.value= stringify(1),
 },
 { /* end of list */ }
 },
 };
 
+#define PC_COMPAT_0_13 \
+PC_COMPAT_0_14,\
+{\
+.driver   = "PCI",\
+.property = "command_serr_enable",\
+.value= "off",\
+},{\
+.driver   = "AC97",\
+.property = "use_broken_id",\
+.value= stringify(1),\
+}
+
 static QEMUMachine pc_machine_v0_13 = {
 .name = "pc-0.13",
 .desc = "Standard PC",
 .init = pc_init_pci_no_kvmclock,
 .max_cpus = 255,
 .compat_props = (GlobalProperty[]) {
+PC_COMPAT_0_13,
 {
 .driver   = "virtio-9p-pci",
 .property = "vectors",
@@ -472,59 +479,31 @@ static QEMUMachine pc_machine_v0_13 = {
 .driver   = "vmware-svga",
 .property = "rombar",
 .value= stringify(0),
-},{
-.driver   = "PCI",
-.property = "command_

Re: [Qemu-devel] [RFC PATCH 06/17] block: use bdrv_{co, aio}_discard for write_zeroes operations

2012-03-26 Thread Daniel P. Berrange
On Sat, Mar 24, 2012 at 04:29:26PM +0100, Christoph Hellwig wrote:
> On Wed, Mar 14, 2012 at 01:01:35PM +0100, Kevin Wolf wrote:
> > Paolo mentioned a use case as a fast way for guests to write zeros, but
> > is it really faster than a normal write when we have to emulate it by a
> > bdrv_write with a temporary buffer of zeros? On the other hand we have
> > the cases where discard really means "I don't care about the data any
> > more" and emulating it by writing zeros is just a waste of resources there.
> 
> On raw a real discard also will destroy the preallocation, which might
> absolutely kill your performance on filesystems.

This suggests that there be a new command line param to '-drive' to turn
discard support on/off, since QEMU can't reliably know if the raw file
it is given is intended to be fully pre-allocated by the mgmt app.

Regards,
Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|



[Qemu-devel] console class in kvm

2012-03-26 Thread Michael S. Tsirkin
kvm used to carry this commit:

commit 4667e6ec0df770867095d8093562d93c94d96ca2
Author: Avi Kivity 
Date:   Thu Feb 12 11:43:17 2009 +0200

Change virtio-console to PCI_CLASS_OTHERS

As a PCI_CLASS_DISPLAY_OTHER, it reduces primary display somehow on Windows 
XP
(possibly Windows disables acceleration since it fails to find a driver).

Signed-off-by: Avi Kivity 

This seems to have been dropped. Is the issue gone?
Could relevant parties speak up please?
Do we want to merge this commit into qemu.git?

-- 
MST



Re: [Qemu-devel] [libvirt] Modern CPU models cannot be used with libvirt

2012-03-26 Thread Gleb Natapov
On Mon, Mar 26, 2012 at 11:08:16AM +0200, Avi Kivity wrote:
> >> Exactly.  The types are no different, so there's no reason to
> >> discriminate against types that happen to live in qemu-provided data
> >> files vs. qemu code.  They aren't instantiated, so we lose nothing by
> >> creating the factories (just so long as the factories aren't
> >> mass-producing objects).
> >
> >
> > At some point, I'd like to have type modules that are shared objects. 
> > I'd like QEMU to start with almost no builtin types and allow the user
> > to configure which modules get loaded.
> >
> > In the long term, I'd like QEMU to be a small, robust core with the
> > vast majority of code relegated to modules with the user ultimately in
> > control of module loading.
> >
> > Yes, I'd want some module autoloading system but there should always
> > be a way to launch QEMU without loading any modules and then load a
> > very specific set of modules (as defined by the user).
> >
> > You can imagine this being useful for something like Common Criteria
> > certifications.
> 
> Okay.
> 
Modularised minimal QEMU may be a good thing, but how -nodefconfig helps
here? Won't you have the same effect if QEMU will load modules on demand,
only when they are actually needed (regardless of -nodefconfig). i.e
virtio-blk is loaded only if there is -device virtio-blk somewhere in
configuration.

> > It's obviously defined for a given release, just not defined long term.
> >
> >> If I see something like -nodefconfig, I assume it will create a bare
> >> bones guest that will not depend on any qemu defaults and will be stable
> >> across releases.
> >
> > That's not even close to what -nodefconfig is.  That's pretty much
> > what -nodefaults is but -nodefaults has also had a fluid definition
> > historically.
> 
> Okay.  Let's just make sure to document -nodefconfig as version specific
> and -nodefaults as the stable way to create a bare bones guest (and
> define exactly what that means).
> 
What is the reason libvirt uses -nodefconfig instead of -nodefaults now?
What the former does for them that the later doesn't?

--
Gleb.



[Qemu-devel] [PATCH 00/12 v11] introducing a new, dedicated guest memory dump mechanism

2012-03-26 Thread Wen Congyang
Hi, all

'virsh dump' can not work when host pci device is used by guest. We have
discussed this issue here:
http://lists.nongnu.org/archive/html/qemu-devel/2011-10/msg00736.html

The last version is here:
http://lists.nongnu.org/archive/html/qemu-devel/2012-03/msg03866.html

We have determined to introduce a new command dump-guest-memory to dump
guest's memory. The core file's format is elf32 or elf64.

Note:
1. The guest should be x86 or x86_64. The other arch is not supported now.
2. If you use old gdb, gdb may crash. I use gdb-7.3.1, and it does not crash.
3. If the OS is in the second kernel, gdb may not work well, and crash can
   work by specifying '--machdep phys_addr=xxx' in the command line. The
   reason is that the second kernel will update the page table, and we can
   not get the page table for the first kernel.
4. The cpu's state is stored in QEMU note. You neet to modify crash to use
   it to calculate phys_base.
5. If the guest OS is 32 bit and the memory size is larger than 4G, the vmcore
   is elf64 format. You should use the gdb which is built with 
--enable-64-bit-bfd.
6. This patchset is based on the upstream tree, and apply one patch that is 
still
   in Luiz Capitulino's tree, because I use the API qemu_get_fd() in this 
patchset.

Changes from v10 to v11:
1. addressed Luiz's and Hatayam's comment
2. fix a bug about filtering feature

Changes from v9 to v10:
1. fix some bug
2. addressed Luiz's and Hatayam's comment
3. remove cancel and query command

Changes from v8 to v9:
1. remove async support(it will be reimplemented after QAPI async commands 
support
   is finished)
2. fix some typo error

Changes from v7 to v8:
1. addressed Hatayama's comments

Changes from v6 to v7:
1. addressed Jan's comments
2. fix some bugs
3. store cpu's state into the vmcore

Changes from v5 to v6:
1. allow user to dump a fraction of the memory
2. fix some bugs

Changes from v4 to v5:
1. convert the new command dump to QAPI 

Changes from v3 to v4:
1. support it to run asynchronously
2. add API to cancel dumping and query dumping progress
3. add API to control dumping speed
4. auto cancel dumping when the user resumes vm, and the status is failed.

Changes from v2 to v3:
1. address Jan Kiszka's comment

Changes from v1 to v2:
1. fix virt addr in the vmcore.

Wen Congyang (12):
  Add API to create memory mapping list
  Add API to check whether a physical address is I/O address
  implement cpu_get_memory_mapping()
  Add API to check whether paging mode is enabled
  Add API to get memory mapping
  Add API to get memory mapping without do paging
  target-i386: Add API to write elf notes to core file
  target-i386: Add API to write cpu status to core file
  target-i386: add API to get dump info
  make gdb_id() generally avialable and rename it to cpu_index()
  QError: Introduce new error for the dump-guest-memory command
  introduce a new monitor command 'dump-guest-memory' to dump guest's
memory

 Makefile.target   |3 +
 configure |8 +
 cpu-all.h |   67 +++
 cpu-common.h  |2 +
 dump.c|  827 +
 dump.h|   23 +
 elf.h |5 +
 exec.c|9 +
 gdbstub.c |   19 +-
 gdbstub.h |9 +
 hmp-commands.hx   |   28 ++
 hmp.c |   22 +
 hmp.h |1 +
 memory_mapping.c  |  236 +++
 memory_mapping.h  |   68 +++
 qapi-schema.json  |   34 ++
 qerror.c  |4 +
 qerror.h  |3 +
 qmp-commands.hx   |   38 ++
 target-i386/arch_dump.c   |  426 +++
 target-i386/arch_memory_mapping.c |  271 
 21 files changed, 2089 insertions(+), 14 deletions(-)
 create mode 100644 dump.c
 create mode 100644 dump.h
 create mode 100644 memory_mapping.c
 create mode 100644 memory_mapping.h
 create mode 100644 target-i386/arch_dump.c
 create mode 100644 target-i386/arch_memory_mapping.c




Re: [Qemu-devel] [RFC PATCH 06/17] block: use bdrv_{co, aio}_discard for write_zeroes operations

2012-03-26 Thread Christoph Hellwig
On Mon, Mar 26, 2012 at 10:44:07AM +0100, Daniel P. Berrange wrote:
> This suggests that there be a new command line param to '-drive' to turn
> discard support on/off, since QEMU can't reliably know if the raw file
> it is given is intended to be fully pre-allocated by the mgmt app.

Yes.




[Qemu-devel] [PATCH 01/12 v11] Add API to create memory mapping list

2012-03-26 Thread Wen Congyang
The memory mapping list stores virtual address and physical address mapping.
The virtual address and physical address are contiguous in the mapping.
The folloing patch will use this information to create PT_LOAD in the vmcore.

Signed-off-by: Wen Congyang 
---
 Makefile.target  |1 +
 memory_mapping.c |  166 ++
 memory_mapping.h |   47 +++
 3 files changed, 214 insertions(+), 0 deletions(-)
 create mode 100644 memory_mapping.c
 create mode 100644 memory_mapping.h

diff --git a/Makefile.target b/Makefile.target
index 44b2e83..33680ae 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -220,6 +220,7 @@ obj-$(CONFIG_KVM) += kvm.o kvm-all.o
 obj-$(CONFIG_NO_KVM) += kvm-stub.o
 obj-$(CONFIG_VGA) += vga.o
 obj-y += memory.o savevm.o
+obj-y += memory_mapping.o
 LIBS+=-lz
 
 obj-i386-$(CONFIG_KVM) += hyperv.o
diff --git a/memory_mapping.c b/memory_mapping.c
new file mode 100644
index 000..718f271
--- /dev/null
+++ b/memory_mapping.c
@@ -0,0 +1,166 @@
+/*
+ * QEMU memory mapping
+ *
+ * Copyright Fujitsu, Corp. 2011, 2012
+ *
+ * Authors:
+ * Wen Congyang 
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2. See
+ * the COPYING file in the top-level directory.
+ *
+ */
+
+#include "cpu.h"
+#include "cpu-all.h"
+#include "memory_mapping.h"
+
+static void memory_mapping_list_add_mapping_sorted(MemoryMappingList *list,
+   MemoryMapping *mapping)
+{
+MemoryMapping *p;
+
+QTAILQ_FOREACH(p, &list->head, next) {
+if (p->phys_addr >= mapping->phys_addr) {
+QTAILQ_INSERT_BEFORE(p, mapping, next);
+return;
+}
+}
+QTAILQ_INSERT_TAIL(&list->head, mapping, next);
+}
+
+static void create_new_memory_mapping(MemoryMappingList *list,
+  target_phys_addr_t phys_addr,
+  target_phys_addr_t virt_addr,
+  ram_addr_t length)
+{
+MemoryMapping *memory_mapping;
+
+memory_mapping = g_malloc(sizeof(MemoryMapping));
+memory_mapping->phys_addr = phys_addr;
+memory_mapping->virt_addr = virt_addr;
+memory_mapping->length = length;
+list->last_mapping = memory_mapping;
+list->num++;
+memory_mapping_list_add_mapping_sorted(list, memory_mapping);
+}
+
+static inline bool mapping_contiguous(MemoryMapping *map,
+  target_phys_addr_t phys_addr,
+  target_phys_addr_t virt_addr)
+{
+return phys_addr == map->phys_addr + map->length &&
+   virt_addr == map->virt_addr + map->length;
+}
+
+/*
+ * [map->phys_addr, map->phys_addr + map->length) and
+ * [phys_addr, phys_addr + length) have intersection?
+ */
+static inline bool mapping_have_same_region(MemoryMapping *map,
+target_phys_addr_t phys_addr,
+ram_addr_t length)
+{
+return !(phys_addr + length < map->phys_addr ||
+ phys_addr >= map->phys_addr + map->length);
+}
+
+/*
+ * [map->phys_addr, map->phys_addr + map->length) and
+ * [phys_addr, phys_addr + length) have intersection. The virtual address in 
the
+ * intersection are the same?
+ */
+static inline bool mapping_conflict(MemoryMapping *map,
+target_phys_addr_t phys_addr,
+target_phys_addr_t virt_addr)
+{
+return virt_addr - map->virt_addr != phys_addr - map->phys_addr;
+}
+
+/*
+ * [map->virt_addr, map->virt_addr + map->length) and
+ * [virt_addr, virt_addr + length) have intersection. And the physical address
+ * in the intersection are the same.
+ */
+static inline void mapping_merge(MemoryMapping *map,
+ target_phys_addr_t virt_addr,
+ ram_addr_t length)
+{
+if (virt_addr < map->virt_addr) {
+map->length += map->virt_addr - virt_addr;
+map->virt_addr = virt_addr;
+}
+
+if ((virt_addr + length) >
+(map->virt_addr + map->length)) {
+map->length = virt_addr + length - map->virt_addr;
+}
+}
+
+void memory_mapping_list_add_merge_sorted(MemoryMappingList *list,
+  target_phys_addr_t phys_addr,
+  target_phys_addr_t virt_addr,
+  ram_addr_t length)
+{
+MemoryMapping *memory_mapping, *last_mapping;
+
+if (QTAILQ_EMPTY(&list->head)) {
+create_new_memory_mapping(list, phys_addr, virt_addr, length);
+return;
+}
+
+last_mapping = list->last_mapping;
+if (last_mapping) {
+if (mapping_contiguous(last_mapping, phys_addr, virt_addr)) {
+last_mapping->length += length;
+return;
+}
+}
+
+QTAILQ_FOREACH(memory_mapping, &list->head, next) {
+if (mapp

[Qemu-devel] [PATCH 02/12 v12] Add API to check whether a physical address is I/O address

2012-03-26 Thread Wen Congyang
This API will be used in the following patch.

Signed-off-by: Wen Congyang 
---
 cpu-common.h |2 ++
 exec.c   |9 +
 2 files changed, 11 insertions(+), 0 deletions(-)

diff --git a/cpu-common.h b/cpu-common.h
index dca5175..fcd50dc 100644
--- a/cpu-common.h
+++ b/cpu-common.h
@@ -71,6 +71,8 @@ void cpu_physical_memory_unmap(void *buffer, 
target_phys_addr_t len,
 void *cpu_register_map_client(void *opaque, void (*callback)(void *opaque));
 void cpu_unregister_map_client(void *cookie);
 
+bool cpu_physical_memory_is_io(target_phys_addr_t phys_addr);
+
 /* Coalesced MMIO regions are areas where write operations can be reordered.
  * This usually implies that write operations are side-effect free.  This 
allows
  * batching which can make a major impact on performance when using
diff --git a/exec.c b/exec.c
index 6731ab8..b3fd8cb 100644
--- a/exec.c
+++ b/exec.c
@@ -4657,3 +4657,12 @@ bool virtio_is_big_endian(void)
 #undef env
 
 #endif
+
+bool cpu_physical_memory_is_io(target_phys_addr_t phys_addr)
+{
+MemoryRegionSection *section;
+
+section = phys_page_find(phys_addr >> TARGET_PAGE_BITS);
+
+return !is_ram_rom_romd(section);
+}
-- 
1.7.1




[Qemu-devel] [PATCH 03/12 v11] implement cpu_get_memory_mapping()

2012-03-26 Thread Wen Congyang
Walk cpu's page table and collect all virtual address and physical address 
mapping.
Then, add these mapping into memory mapping list. If the guest does not use 
paging,
it will do nothing. Note: the I/O memory will be skipped.

Signed-off-by: Wen Congyang 
---
 Makefile.target   |1 +
 configure |4 +
 cpu-all.h |   11 ++
 target-i386/arch_memory_mapping.c |  266 +
 4 files changed, 282 insertions(+), 0 deletions(-)
 create mode 100644 target-i386/arch_memory_mapping.c

diff --git a/Makefile.target b/Makefile.target
index 33680ae..e88bfcf 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -89,6 +89,7 @@ libobj-y += helper.o
 ifeq ($(TARGET_BASE_ARCH), i386)
 libobj-y += cpuid.o
 endif
+libobj-$(CONFIG_HAVE_GET_MEMORY_MAPPING) += arch_memory_mapping.o
 libobj-$(TARGET_SPARC64) += vis_helper.o
 libobj-$(CONFIG_NEED_MMU) += mmu.o
 libobj-$(TARGET_ARM) += neon_helper.o iwmmxt_helper.o
diff --git a/configure b/configure
index 14ef738..0e05b84 100755
--- a/configure
+++ b/configure
@@ -3659,6 +3659,10 @@ case "$target_arch2" in
   fi
 fi
 esac
+case "$target_arch2" in
+  i386|x86_64)
+echo "CONFIG_HAVE_GET_MEMORY_MAPPING=y" >> $config_target_mak
+esac
 if test "$target_arch2" = "ppc64" -a "$fdt" = "yes"; then
   echo "CONFIG_PSERIES=y" >> $config_target_mak
 fi
diff --git a/cpu-all.h b/cpu-all.h
index 9621c3c..390b55b 100644
--- a/cpu-all.h
+++ b/cpu-all.h
@@ -22,6 +22,7 @@
 #include "qemu-common.h"
 #include "qemu-tls.h"
 #include "cpu-common.h"
+#include "memory_mapping.h"
 
 /* some important defines:
  *
@@ -525,4 +526,14 @@ void dump_exec_info(FILE *f, fprintf_function cpu_fprintf);
 int cpu_memory_rw_debug(CPUArchState *env, target_ulong addr,
 uint8_t *buf, int len, int is_write);
 
+#if defined(CONFIG_HAVE_GET_MEMORY_MAPPING)
+int cpu_get_memory_mapping(MemoryMappingList *list, CPUArchState *env);
+#else
+static inline int cpu_get_memory_mapping(MemoryMappingList *list,
+ CPUArchState *env)
+{
+return -1;
+}
+#endif
+
 #endif /* CPU_ALL_H */
diff --git a/target-i386/arch_memory_mapping.c 
b/target-i386/arch_memory_mapping.c
new file mode 100644
index 000..dd64bec
--- /dev/null
+++ b/target-i386/arch_memory_mapping.c
@@ -0,0 +1,266 @@
+/*
+ * i386 memory mapping
+ *
+ * Copyright Fujitsu, Corp. 2011, 2012
+ *
+ * Authors:
+ * Wen Congyang 
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2.  See
+ * the COPYING file in the top-level directory.
+ *
+ */
+
+#include "cpu.h"
+#include "cpu-all.h"
+
+/* PAE Paging or IA-32e Paging */
+static void walk_pte(MemoryMappingList *list, target_phys_addr_t 
pte_start_addr,
+ int32_t a20_mask, target_ulong start_line_addr)
+{
+target_phys_addr_t pte_addr, start_paddr;
+uint64_t pte;
+target_ulong start_vaddr;
+int i;
+
+for (i = 0; i < 512; i++) {
+pte_addr = (pte_start_addr + i * 8) & a20_mask;
+pte = ldq_phys(pte_addr);
+if (!(pte & PG_PRESENT_MASK)) {
+/* not present */
+continue;
+}
+
+start_paddr = (pte & ~0xfff) & ~(0x1ULL << 63);
+if (cpu_physical_memory_is_io(start_paddr)) {
+/* I/O region */
+continue;
+}
+
+start_vaddr = start_line_addr | ((i & 0x1fff) << 12);
+memory_mapping_list_add_merge_sorted(list, start_paddr,
+ start_vaddr, 1 << 12);
+}
+}
+
+/* 32-bit Paging */
+static void walk_pte2(MemoryMappingList *list,
+  target_phys_addr_t pte_start_addr, int32_t a20_mask,
+  target_ulong start_line_addr)
+{
+target_phys_addr_t pte_addr, start_paddr;
+uint32_t pte;
+target_ulong start_vaddr;
+int i;
+
+for (i = 0; i < 1024; i++) {
+pte_addr = (pte_start_addr + i * 4) & a20_mask;
+pte = ldl_phys(pte_addr);
+if (!(pte & PG_PRESENT_MASK)) {
+/* not present */
+continue;
+}
+
+start_paddr = pte & ~0xfff;
+if (cpu_physical_memory_is_io(start_paddr)) {
+/* I/O region */
+continue;
+}
+
+start_vaddr = start_line_addr | ((i & 0x3ff) << 12);
+memory_mapping_list_add_merge_sorted(list, start_paddr,
+ start_vaddr, 1 << 12);
+}
+}
+
+/* PAE Paging or IA-32e Paging */
+static void walk_pde(MemoryMappingList *list, target_phys_addr_t 
pde_start_addr,
+ int32_t a20_mask, target_ulong start_line_addr)
+{
+target_phys_addr_t pde_addr, pte_start_addr, start_paddr;
+uint64_t pde;
+target_ulong line_addr, start_vaddr;
+int i;
+
+for (i = 0; i < 512; i++) {
+pde_addr = (pde_start_addr + i * 8) & a20_mask;
+pde = ldq_phys(pde_addr);
+if (!(pde & PG_PRESENT_MASK)) {
+  

[Qemu-devel] [PATCH 04/12 v11] Add API to check whether paging mode is enabled

2012-03-26 Thread Wen Congyang
This API will be used in the following patch.

Signed-off-by: Wen Congyang 
---
 cpu-all.h |6 ++
 target-i386/arch_memory_mapping.c |7 ++-
 2 files changed, 12 insertions(+), 1 deletions(-)

diff --git a/cpu-all.h b/cpu-all.h
index 390b55b..24c7f77 100644
--- a/cpu-all.h
+++ b/cpu-all.h
@@ -528,12 +528,18 @@ int cpu_memory_rw_debug(CPUArchState *env, target_ulong 
addr,
 
 #if defined(CONFIG_HAVE_GET_MEMORY_MAPPING)
 int cpu_get_memory_mapping(MemoryMappingList *list, CPUArchState *env);
+bool cpu_paging_enabled(CPUArchState *env);
 #else
 static inline int cpu_get_memory_mapping(MemoryMappingList *list,
  CPUArchState *env)
 {
 return -1;
 }
+
+static inline bool cpu_paging_enabled(CPUArchState *env)
+{
+return true;
+}
 #endif
 
 #endif /* CPU_ALL_H */
diff --git a/target-i386/arch_memory_mapping.c 
b/target-i386/arch_memory_mapping.c
index dd64bec..bd50e11 100644
--- a/target-i386/arch_memory_mapping.c
+++ b/target-i386/arch_memory_mapping.c
@@ -233,7 +233,7 @@ static void walk_pml4e(MemoryMappingList *list,
 
 int cpu_get_memory_mapping(MemoryMappingList *list, CPUArchState *env)
 {
-if (!(env->cr[0] & CR0_PG_MASK)) {
+if (!cpu_paging_enabled(env)) {
 /* paging is disabled */
 return 0;
 }
@@ -264,3 +264,8 @@ int cpu_get_memory_mapping(MemoryMappingList *list, 
CPUArchState *env)
 
 return 0;
 }
+
+bool cpu_paging_enabled(CPUArchState *env)
+{
+return env->cr[0] & CR0_PG_MASK;
+}
-- 
1.7.1




[Qemu-devel] [PATCH 05/12 v11] Add API to get memory mapping

2012-03-26 Thread Wen Congyang
Add API to get all virtual address and physical address mapping.
If the guest doesn't use paging, the virtual address is equal to the phyical
address. The virtual address and physical address mapping is for gdb's user, and
it does not include the memory that is not referenced by the page table. So if
you want to use crash to anaylze the vmcore, please do not specify -p option.
the reason why the -p option is not default explicitly: guest machine in a
catastrophic state can have corrupted memory, which we cannot trust.

Signed-off-by: Wen Congyang 
---
 memory_mapping.c |   34 ++
 memory_mapping.h |   15 +++
 2 files changed, 49 insertions(+), 0 deletions(-)

diff --git a/memory_mapping.c b/memory_mapping.c
index 718f271..b92e2f6 100644
--- a/memory_mapping.c
+++ b/memory_mapping.c
@@ -164,3 +164,37 @@ void memory_mapping_list_init(MemoryMappingList *list)
 list->last_mapping = NULL;
 QTAILQ_INIT(&list->head);
 }
+
+#if defined(CONFIG_HAVE_GET_MEMORY_MAPPING)
+int qemu_get_guest_memory_mapping(MemoryMappingList *list)
+{
+CPUArchState *env;
+RAMBlock *block;
+ram_addr_t offset, length;
+int ret;
+bool paging_mode;
+
+paging_mode = cpu_paging_enabled(first_cpu);
+if (paging_mode) {
+for (env = first_cpu; env != NULL; env = env->next_cpu) {
+ret = cpu_get_memory_mapping(list, env);
+if (ret < 0) {
+return -1;
+}
+}
+return 0;
+}
+
+/*
+ * If the guest doesn't use paging, the virtual address is equal to 
physical
+ * address.
+ */
+QLIST_FOREACH(block, &ram_list.blocks, next) {
+offset = block->offset;
+length = block->length;
+create_new_memory_mapping(list, offset, offset, length);
+}
+
+return 0;
+}
+#endif
diff --git a/memory_mapping.h b/memory_mapping.h
index 836b047..4d44641 100644
--- a/memory_mapping.h
+++ b/memory_mapping.h
@@ -44,4 +44,19 @@ void memory_mapping_list_free(MemoryMappingList *list);
 
 void memory_mapping_list_init(MemoryMappingList *list);
 
+/*
+ * Return value:
+ *0: success
+ *   -1: failed
+ *   -2: unsupported
+ */
+#if defined(CONFIG_HAVE_GET_MEMORY_MAPPING)
+int qemu_get_guest_memory_mapping(MemoryMappingList *list);
+#else
+static inline int qemu_get_guest_memory_mapping(MemoryMappingList *list)
+{
+return -2;
+}
+#endif
+
 #endif
-- 
1.7.1




[Qemu-devel] [PATCH 06/12 v11] Add API to get memory mapping without do paging

2012-03-26 Thread Wen Congyang
crash does not need the virtual address and physical address mapping, and the
mapping does not include the memory that is not referenced by the page table.
crash does not use the virtual address, so we can create the mapping for all
physical memory(virtual address is always 0). This patch provides a API to do
this thing, and it will be used in the following patch.

Signed-off-by: Wen Congyang 
---
 memory_mapping.c |9 +
 memory_mapping.h |3 +++
 2 files changed, 12 insertions(+), 0 deletions(-)

diff --git a/memory_mapping.c b/memory_mapping.c
index b92e2f6..9a2ffe6 100644
--- a/memory_mapping.c
+++ b/memory_mapping.c
@@ -198,3 +198,12 @@ int qemu_get_guest_memory_mapping(MemoryMappingList *list)
 return 0;
 }
 #endif
+
+void qemu_get_guest_simple_memory_mapping(MemoryMappingList *list)
+{
+RAMBlock *block;
+
+QLIST_FOREACH(block, &ram_list.blocks, next) {
+create_new_memory_mapping(list, block->offset, 0, block->length);
+}
+}
diff --git a/memory_mapping.h b/memory_mapping.h
index 4d44641..a583e44 100644
--- a/memory_mapping.h
+++ b/memory_mapping.h
@@ -59,4 +59,7 @@ static inline int 
qemu_get_guest_memory_mapping(MemoryMappingList *list)
 }
 #endif
 
+/* get guest's memory mapping without do paging(virtual address is 0). */
+void qemu_get_guest_simple_memory_mapping(MemoryMappingList *list);
+
 #endif
-- 
1.7.1




[Qemu-devel] [PATCH 07/12 v11] target-i386: Add API to write elf notes to core file

2012-03-26 Thread Wen Congyang
The core file contains register's value. These APIs write registers to
core file, and them will be called in the following patch.

Signed-off-by: Wen Congyang 
---
 Makefile.target |1 +
 configure   |4 +
 cpu-all.h   |   23 +
 target-i386/arch_dump.c |  240 +++
 4 files changed, 268 insertions(+), 0 deletions(-)
 create mode 100644 target-i386/arch_dump.c

diff --git a/Makefile.target b/Makefile.target
index e88bfcf..2e20ed2 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -222,6 +222,7 @@ obj-$(CONFIG_NO_KVM) += kvm-stub.o
 obj-$(CONFIG_VGA) += vga.o
 obj-y += memory.o savevm.o
 obj-y += memory_mapping.o
+obj-$(CONFIG_HAVE_CORE_DUMP) += arch_dump.o
 LIBS+=-lz
 
 obj-i386-$(CONFIG_KVM) += hyperv.o
diff --git a/configure b/configure
index 0e05b84..afb294c 100755
--- a/configure
+++ b/configure
@@ -3678,6 +3678,10 @@ if test "$target_softmmu" = "yes" ; then
   if test "$smartcard_nss" = "yes" ; then
 echo "subdir-$target: subdir-libcacard" >> $config_host_mak
   fi
+  case "$target_arch2" in
+i386|x86_64)
+  echo "CONFIG_HAVE_CORE_DUMP=y" >> $config_target_mak
+  esac
 fi
 if test "$target_user_only" = "yes" ; then
   echo "CONFIG_USER_ONLY=y" >> $config_target_mak
diff --git a/cpu-all.h b/cpu-all.h
index 24c7f77..bc29b43 100644
--- a/cpu-all.h
+++ b/cpu-all.h
@@ -542,4 +542,27 @@ static inline bool cpu_paging_enabled(CPUArchState *env)
 }
 #endif
 
+typedef int (*write_core_dump_function)
+(target_phys_addr_t offset, void *buf, size_t size, void *opaque);
+#if defined(CONFIG_HAVE_CORE_DUMP)
+int cpu_write_elf64_note(write_core_dump_function f, CPUArchState *env,
+ int cpuid, target_phys_addr_t *offset, void *opaque);
+int cpu_write_elf32_note(write_core_dump_function f, CPUArchState *env,
+ int cpuid, target_phys_addr_t *offset, void *opaque);
+#else
+static inline int cpu_write_elf64_note(write_core_dump_function f,
+   CPUArchState *env, int cpuid,
+   target_phys_addr_t *offset, void 
*opaque)
+{
+return -1;
+}
+
+static inline int cpu_write_elf32_note(write_core_dump_function f,
+   CPUArchState *env, int cpuid,
+   target_phys_addr_t *offset, void 
*opaque)
+{
+return -1;
+}
+#endif
+
 #endif /* CPU_ALL_H */
diff --git a/target-i386/arch_dump.c b/target-i386/arch_dump.c
new file mode 100644
index 000..285c6e1
--- /dev/null
+++ b/target-i386/arch_dump.c
@@ -0,0 +1,240 @@
+/*
+ * i386 memory mapping
+ *
+ * Copyright Fujitsu, Corp. 2011, 2012
+ *
+ * Authors:
+ * Wen Congyang 
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2.  See
+ * the COPYING file in the top-level directory.
+ *
+ */
+
+#include "cpu.h"
+#include "cpu-all.h"
+#include "elf.h"
+
+#ifdef TARGET_X86_64
+typedef struct {
+target_ulong r15, r14, r13, r12, rbp, rbx, r11, r10;
+target_ulong r9, r8, rax, rcx, rdx, rsi, rdi, orig_rax;
+target_ulong rip, cs, eflags;
+target_ulong rsp, ss;
+target_ulong fs_base, gs_base;
+target_ulong ds, es, fs, gs;
+} x86_64_user_regs_struct;
+
+typedef struct {
+char pad1[32];
+uint32_t pid;
+char pad2[76];
+x86_64_user_regs_struct regs;
+char pad3[8];
+} x86_64_elf_prstatus;
+
+static int x86_64_write_elf64_note(write_core_dump_function f,
+   CPUArchState *env, int id,
+   target_phys_addr_t *offset, void *opaque)
+{
+x86_64_user_regs_struct regs;
+Elf64_Nhdr *note;
+char *buf;
+int descsz, note_size, name_size = 5;
+const char *name = "CORE";
+int ret;
+
+regs.r15 = env->regs[15];
+regs.r14 = env->regs[14];
+regs.r13 = env->regs[13];
+regs.r12 = env->regs[12];
+regs.r11 = env->regs[11];
+regs.r10 = env->regs[10];
+regs.r9  = env->regs[9];
+regs.r8  = env->regs[8];
+regs.rbp = env->regs[R_EBP];
+regs.rsp = env->regs[R_ESP];
+regs.rdi = env->regs[R_EDI];
+regs.rsi = env->regs[R_ESI];
+regs.rdx = env->regs[R_EDX];
+regs.rcx = env->regs[R_ECX];
+regs.rbx = env->regs[R_EBX];
+regs.rax = env->regs[R_EAX];
+regs.rip = env->eip;
+regs.eflags = env->eflags;
+
+regs.orig_rax = 0; /* FIXME */
+regs.cs = env->segs[R_CS].selector;
+regs.ss = env->segs[R_SS].selector;
+regs.fs_base = env->segs[R_FS].base;
+regs.gs_base = env->segs[R_GS].base;
+regs.ds = env->segs[R_DS].selector;
+regs.es = env->segs[R_ES].selector;
+regs.fs = env->segs[R_FS].selector;
+regs.gs = env->segs[R_GS].selector;
+
+descsz = sizeof(x86_64_elf_prstatus);
+note_size = ((sizeof(Elf64_Nhdr) + 3) / 4 + (name_size + 3) / 4 +
+(descsz + 3) / 4) * 4;
+note = g_malloc(note_size);
+
+memset(note, 0, note_size);
+note->n_namesz = cpu_to_le32(name_size

[Qemu-devel] [PATCH 08/12 v11] target-i386: Add API to write cpu status to core file

2012-03-26 Thread Wen Congyang
The core file has register's value. But it does not include all registers value.
Store the cpu status into QEMU note, and the user can get more information
from vmcore. If you change QEMUCPUState, please count up QEMUCPUSTATE_VERSION.

Signed-off-by: Wen Congyang 
---
 cpu-all.h   |   20 ++
 target-i386/arch_dump.c |  152 +++
 2 files changed, 172 insertions(+), 0 deletions(-)

diff --git a/cpu-all.h b/cpu-all.h
index bc29b43..cd9b013 100644
--- a/cpu-all.h
+++ b/cpu-all.h
@@ -549,6 +549,10 @@ int cpu_write_elf64_note(write_core_dump_function f, 
CPUArchState *env,
  int cpuid, target_phys_addr_t *offset, void *opaque);
 int cpu_write_elf32_note(write_core_dump_function f, CPUArchState *env,
  int cpuid, target_phys_addr_t *offset, void *opaque);
+int cpu_write_elf64_qemunote(write_core_dump_function f, CPUArchState *env,
+ target_phys_addr_t *offset, void *opaque);
+int cpu_write_elf32_qemunote(write_core_dump_function f, CPUArchState *env,
+ target_phys_addr_t *offset, void *opaque);
 #else
 static inline int cpu_write_elf64_note(write_core_dump_function f,
CPUArchState *env, int cpuid,
@@ -563,6 +567,22 @@ static inline int 
cpu_write_elf32_note(write_core_dump_function f,
 {
 return -1;
 }
+
+static inline int cpu_write_elf64_qemunote(write_core_dump_function f,
+   CPUArchState *env,
+   target_phys_addr_t *offset,
+   void *opaque);
+{
+return -1;
+}
+
+static inline int cpu_write_elf32_qemunote(write_core_dump_function f,
+   CPUArchState *env,
+   target_phys_addr_t *offset,
+   void *opaque)
+{
+return -1;
+}
 #endif
 
 #endif /* CPU_ALL_H */
diff --git a/target-i386/arch_dump.c b/target-i386/arch_dump.c
index 285c6e1..0f61dae 100644
--- a/target-i386/arch_dump.c
+++ b/target-i386/arch_dump.c
@@ -238,3 +238,155 @@ int cpu_write_elf32_note(write_core_dump_function f, 
CPUArchState *env,
 
 return 0;
 }
+
+/*
+ * please count up QEMUCPUSTATE_VERSION if you have changed definition of
+ * QEMUCPUState, and modify the tools using this information accordingly.
+ */
+#define QEMUCPUSTATE_VERSION (1)
+
+struct QEMUCPUSegment {
+uint32_t selector;
+uint32_t limit;
+uint32_t flags;
+uint32_t pad;
+uint64_t base;
+};
+
+typedef struct QEMUCPUSegment QEMUCPUSegment;
+
+struct QEMUCPUState {
+uint32_t version;
+uint32_t size;
+uint64_t rax, rbx, rcx, rdx, rsi, rdi, rsp, rbp;
+uint64_t r8, r9, r10, r11, r12, r13, r14, r15;
+uint64_t rip, rflags;
+QEMUCPUSegment cs, ds, es, fs, gs, ss;
+QEMUCPUSegment ldt, tr, gdt, idt;
+uint64_t cr[5];
+};
+
+typedef struct QEMUCPUState QEMUCPUState;
+
+static void copy_segment(QEMUCPUSegment *d, SegmentCache *s)
+{
+d->pad = 0;
+d->selector = s->selector;
+d->limit = s->limit;
+d->flags = s->flags;
+d->base = s->base;
+}
+
+static void qemu_get_cpustate(QEMUCPUState *s, CPUArchState *env)
+{
+memset(s, 0, sizeof(QEMUCPUState));
+
+s->version = QEMUCPUSTATE_VERSION;
+s->size = sizeof(QEMUCPUState);
+
+s->rax = env->regs[R_EAX];
+s->rbx = env->regs[R_EBX];
+s->rcx = env->regs[R_ECX];
+s->rdx = env->regs[R_EDX];
+s->rsi = env->regs[R_ESI];
+s->rdi = env->regs[R_EDI];
+s->rsp = env->regs[R_ESP];
+s->rbp = env->regs[R_EBP];
+#ifdef TARGET_X86_64
+s->r8  = env->regs[8];
+s->r9  = env->regs[9];
+s->r10 = env->regs[10];
+s->r11 = env->regs[11];
+s->r12 = env->regs[12];
+s->r13 = env->regs[13];
+s->r14 = env->regs[14];
+s->r15 = env->regs[15];
+#endif
+s->rip = env->eip;
+s->rflags = env->eflags;
+
+copy_segment(&s->cs, &env->segs[R_CS]);
+copy_segment(&s->ds, &env->segs[R_DS]);
+copy_segment(&s->es, &env->segs[R_ES]);
+copy_segment(&s->fs, &env->segs[R_FS]);
+copy_segment(&s->gs, &env->segs[R_GS]);
+copy_segment(&s->ss, &env->segs[R_SS]);
+copy_segment(&s->ldt, &env->ldt);
+copy_segment(&s->tr, &env->tr);
+copy_segment(&s->gdt, &env->gdt);
+copy_segment(&s->idt, &env->idt);
+
+s->cr[0] = env->cr[0];
+s->cr[1] = env->cr[1];
+s->cr[2] = env->cr[2];
+s->cr[3] = env->cr[3];
+s->cr[4] = env->cr[4];
+}
+
+static inline int cpu_write_qemu_note(write_core_dump_function f,
+  CPUArchState *env,
+  target_phys_addr_t *offset,
+  void *opaque,
+  int type)
+{
+QEMUCPUState state;
+Elf64_Nhdr *note64;
+Elf32_Nhdr *note32;
+void *note;
+char *buf;
+int descsz, note_size

[Qemu-devel] [PATCH 09/12 v11] target-i386: add API to get dump info

2012-03-26 Thread Wen Congyang
Dump info contains: endian, class and architecture. The next
patch will use these information to create vmcore. Note: on
x86 box, the  class is ELFCLASS64 if the memory is larger than 4G.

Signed-off-by: Wen Congyang 
---
 cpu-all.h   |7 +++
 dump.h  |   23 +++
 target-i386/arch_dump.c |   34 ++
 3 files changed, 64 insertions(+), 0 deletions(-)
 create mode 100644 dump.h

diff --git a/cpu-all.h b/cpu-all.h
index cd9b013..d7c5a00 100644
--- a/cpu-all.h
+++ b/cpu-all.h
@@ -23,6 +23,7 @@
 #include "qemu-tls.h"
 #include "cpu-common.h"
 #include "memory_mapping.h"
+#include "dump.h"
 
 /* some important defines:
  *
@@ -553,6 +554,7 @@ int cpu_write_elf64_qemunote(write_core_dump_function f, 
CPUArchState *env,
  target_phys_addr_t *offset, void *opaque);
 int cpu_write_elf32_qemunote(write_core_dump_function f, CPUArchState *env,
  target_phys_addr_t *offset, void *opaque);
+int cpu_get_dump_info(ArchDumpInfo *info);
 #else
 static inline int cpu_write_elf64_note(write_core_dump_function f,
CPUArchState *env, int cpuid,
@@ -583,6 +585,11 @@ static inline int 
cpu_write_elf32_qemunote(write_core_dump_function f,
 {
 return -1;
 }
+
+static inline int cpu_get_dump_info(ArchDumpInfo *info)
+{
+return -1;
+}
 #endif
 
 #endif /* CPU_ALL_H */
diff --git a/dump.h b/dump.h
new file mode 100644
index 000..28340cf
--- /dev/null
+++ b/dump.h
@@ -0,0 +1,23 @@
+/*
+ * QEMU dump
+ *
+ * Copyright Fujitsu, Corp. 2011, 2012
+ *
+ * Authors:
+ * Wen Congyang 
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2. See
+ * the COPYING file in the top-level directory.
+ *
+ */
+
+#ifndef DUMP_H
+#define DUMP_H
+
+typedef struct ArchDumpInfo {
+int d_machine;  /* Architecture */
+int d_endian;   /* ELFDATA2LSB or ELFDATA2MSB */
+int d_class;/* ELFCLASS32 or ELFCLASS64 */
+} ArchDumpInfo;
+
+#endif
diff --git a/target-i386/arch_dump.c b/target-i386/arch_dump.c
index 0f61dae..1a75dea 100644
--- a/target-i386/arch_dump.c
+++ b/target-i386/arch_dump.c
@@ -13,6 +13,7 @@
 
 #include "cpu.h"
 #include "cpu-all.h"
+#include "dump.h"
 #include "elf.h"
 
 #ifdef TARGET_X86_64
@@ -390,3 +391,36 @@ int cpu_write_elf32_qemunote(write_core_dump_function f, 
CPUArchState *env,
 {
 return cpu_write_qemu_note(f, env, offset, opaque, 0);
 }
+
+int cpu_get_dump_info(ArchDumpInfo *info)
+{
+bool lma = false;
+RAMBlock *block;
+
+#ifdef TARGET_X86_64
+lma = !!(first_cpu->hflags & HF_LMA_MASK);
+#endif
+
+if (lma) {
+info->d_machine = EM_X86_64;
+} else {
+info->d_machine = EM_386;
+}
+info->d_endian = ELFDATA2LSB;
+
+if (lma) {
+info->d_class = ELFCLASS64;
+} else {
+info->d_class = ELFCLASS32;
+
+QLIST_FOREACH(block, &ram_list.blocks, next) {
+if (block->offset + block->length > UINT_MAX) {
+/* The memory size is greater than 4G */
+info->d_class = ELFCLASS64;
+break;
+}
+}
+}
+
+return 0;
+}
-- 
1.7.1




[Qemu-devel] [PATCH 10/12 v11] make gdb_id() generally avialable and rename it to cpu_index()

2012-03-26 Thread Wen Congyang
The following patch also needs this API, so make it generally avialable.
The function gdb_id() will not be used in gdbstub.c now, so its name is
not suitable, and rename it to cpu_index()

Signed-off-by: Wen Congyang 
---
 gdbstub.c |   19 +--
 gdbstub.h |9 +
 2 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/gdbstub.c b/gdbstub.c
index 6a7e2c4..423ffec 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -1938,21 +1938,12 @@ static void gdb_set_cpu_pc(GDBState *s, target_ulong pc)
 #endif
 }
 
-static inline int gdb_id(CPUArchState *env)
-{
-#if defined(CONFIG_USER_ONLY) && defined(CONFIG_USE_NPTL)
-return env->host_tid;
-#else
-return env->cpu_index + 1;
-#endif
-}
-
 static CPUArchState *find_cpu(uint32_t thread_id)
 {
 CPUArchState *env;
 
 for (env = first_cpu; env != NULL; env = env->next_cpu) {
-if (gdb_id(env) == thread_id) {
+if (cpu_index(env) == thread_id) {
 return env;
 }
 }
@@ -1980,7 +1971,7 @@ static int gdb_handle_packet(GDBState *s, const char 
*line_buf)
 case '?':
 /* TODO: Make this return the correct value for user-mode.  */
 snprintf(buf, sizeof(buf), "T%02xthread:%02x;", GDB_SIGNAL_TRAP,
- gdb_id(s->c_cpu));
+ cpu_index(s->c_cpu));
 put_packet(s, buf);
 /* Remove all the breakpoints when this query is issued,
  * because gdb is doing and initial connect and the state
@@ -2275,7 +2266,7 @@ static int gdb_handle_packet(GDBState *s, const char 
*line_buf)
 } else if (strcmp(p,"sThreadInfo") == 0) {
 report_cpuinfo:
 if (s->query_cpu) {
-snprintf(buf, sizeof(buf), "m%x", gdb_id(s->query_cpu));
+snprintf(buf, sizeof(buf), "m%x", cpu_index(s->query_cpu));
 put_packet(s, buf);
 s->query_cpu = s->query_cpu->next_cpu;
 } else
@@ -2423,7 +2414,7 @@ static void gdb_vm_state_change(void *opaque, int 
running, RunState state)
 }
 snprintf(buf, sizeof(buf),
  "T%02xthread:%02x;%swatch:" TARGET_FMT_lx ";",
- GDB_SIGNAL_TRAP, gdb_id(env), type,
+ GDB_SIGNAL_TRAP, cpu_index(env), type,
  env->watchpoint_hit->vaddr);
 env->watchpoint_hit = NULL;
 goto send_packet;
@@ -2456,7 +2447,7 @@ static void gdb_vm_state_change(void *opaque, int 
running, RunState state)
 ret = GDB_SIGNAL_UNKNOWN;
 break;
 }
-snprintf(buf, sizeof(buf), "T%02xthread:%02x;", ret, gdb_id(env));
+snprintf(buf, sizeof(buf), "T%02xthread:%02x;", ret, cpu_index(env));
 
 send_packet:
 put_packet(s, buf);
diff --git a/gdbstub.h b/gdbstub.h
index b44e275..668de66 100644
--- a/gdbstub.h
+++ b/gdbstub.h
@@ -30,6 +30,15 @@ void gdb_register_coprocessor(CPUArchState *env,
   gdb_reg_cb get_reg, gdb_reg_cb set_reg,
   int num_regs, const char *xml, int g_pos);
 
+static inline int cpu_index(CPUArchState *env)
+{
+#if defined(CONFIG_USER_ONLY) && defined(CONFIG_USE_NPTL)
+return env->host_tid;
+#else
+return env->cpu_index + 1;
+#endif
+}
+
 #endif
 
 #ifdef CONFIG_USER_ONLY
-- 
1.7.1




[Qemu-devel] [PATCH 11/12 v11] QError: Introduce new error for the dump-guest-memory command

2012-03-26 Thread Wen Congyang
The new error is QERR_PIPE_OR_SOCKET_FD, which is going to be
used by the QAPI dump-guest-memory command.

Signed-off-by: Wen Congyang 
---
 qerror.c |4 
 qerror.h |3 +++
 2 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/qerror.c b/qerror.c
index 41c729a..d5b3a8c 100644
--- a/qerror.c
+++ b/qerror.c
@@ -299,6 +299,10 @@ static const QErrorStringTable qerror_table[] = {
 .error_fmt = QERR_VNC_SERVER_FAILED,
 .desc  = "Could not start VNC server on %(target)",
 },
+{
+.error_fmt = QERR_PIPE_OR_SOCKET_FD,
+.desc  = "lseek() failed: the fd is associated with a pipe, 
socket",
+},
 {}
 };
 
diff --git a/qerror.h b/qerror.h
index e16f9c2..6b5e8f7 100644
--- a/qerror.h
+++ b/qerror.h
@@ -244,4 +244,7 @@ QError *qobject_to_qerror(const QObject *obj);
 #define QERR_VNC_SERVER_FAILED \
 "{ 'class': 'VNCServerFailed', 'data': { 'target': %s } }"
 
+#define QERR_PIPE_OR_SOCKET_FD \
+"{ 'class': 'PipeOrSocketFD', 'data': {} }"
+
 #endif /* QERROR_H */
-- 
1.7.1




Re: [Qemu-devel] buildbot failure in qemu on default_i386_debian_6_0

2012-03-26 Thread Stefan Hajnoczi
On Sun, Mar 25, 2012 at 11:04 AM, Daniel Gollub  wrote:
> I finally added "make check" to the "default" factory.
>
> Note, if the "make check" fails the mail will state:
>
> "BUILD FAILED: failed test"
>
>
> It's failing on yuzuki due to missing "bc".
> Could you install "bc" on your buildslave?

Thanks for pointing this out.  I have updated yuzuki and installed
bc(1).  make check now works.

Stefan



[Qemu-devel] [PATCH 12/12 v11] introduce a new monitor command 'dump-guest-memory' to dump guest's memory

2012-03-26 Thread Wen Congyang
The command's usage:
   dump [-p] protocol [begin] [length]
The supported protocol can be file or fd:
1. file: the protocol starts with "file:", and the following string is
   the file's path.
2. fd: the protocol starts with "fd:", and the following string is the
   fd's name.

Note:
  1. If you want to use gdb to process the core, please specify -p option.
 The reason why the -p option is not default is:
   a. guest machine in a catastrophic state can have corrupted memory,
  which we cannot trust.
   b. The guest machine can be in read-mode even if paging is enabled.
  For example: the guest machine uses ACPI to sleep, and ACPI sleep
  state goes in real-mode.
  2. This command doesn't support the fd that is is associated with a pipe,
 socket, or FIFO(lseek will fail with such fd).
  3. If you don't want to dump all guest's memory, please specify the start
 physical address and the length.

Signed-off-by: Wen Congyang 
---
 Makefile.target  |2 +-
 dump.c   |  827 ++
 elf.h|5 +
 hmp-commands.hx  |   28 ++
 hmp.c|   22 ++
 hmp.h|1 +
 memory_mapping.c |   27 ++
 memory_mapping.h |3 +
 qapi-schema.json |   34 +++
 qmp-commands.hx  |   38 +++
 10 files changed, 986 insertions(+), 1 deletions(-)
 create mode 100644 dump.c

diff --git a/Makefile.target b/Makefile.target
index 2e20ed2..43004bc 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -222,7 +222,7 @@ obj-$(CONFIG_NO_KVM) += kvm-stub.o
 obj-$(CONFIG_VGA) += vga.o
 obj-y += memory.o savevm.o
 obj-y += memory_mapping.o
-obj-$(CONFIG_HAVE_CORE_DUMP) += arch_dump.o
+obj-$(CONFIG_HAVE_CORE_DUMP) += arch_dump.o dump.o
 LIBS+=-lz
 
 obj-i386-$(CONFIG_KVM) += hyperv.o
diff --git a/dump.c b/dump.c
new file mode 100644
index 000..d1b3933
--- /dev/null
+++ b/dump.c
@@ -0,0 +1,827 @@
+/*
+ * QEMU dump
+ *
+ * Copyright Fujitsu, Corp. 2011, 2012
+ *
+ * Authors:
+ * Wen Congyang 
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2.  See
+ * the COPYING file in the top-level directory.
+ *
+ */
+
+#include "qemu-common.h"
+#include 
+#include "elf.h"
+#include 
+#include 
+#include "cpu.h"
+#include "cpu-all.h"
+#include "targphys.h"
+#include "monitor.h"
+#include "kvm.h"
+#include "dump.h"
+#include "sysemu.h"
+#include "bswap.h"
+#include "memory_mapping.h"
+#include "error.h"
+#include "qmp-commands.h"
+#include "gdbstub.h"
+
+static uint16_t cpu_convert_to_target16(uint16_t val, int endian)
+{
+if (endian == ELFDATA2LSB) {
+val = cpu_to_le16(val);
+} else {
+val = cpu_to_be16(val);
+}
+
+return val;
+}
+
+static uint32_t cpu_convert_to_target32(uint32_t val, int endian)
+{
+if (endian == ELFDATA2LSB) {
+val = cpu_to_le32(val);
+} else {
+val = cpu_to_be32(val);
+}
+
+return val;
+}
+
+static uint64_t cpu_convert_to_target64(uint64_t val, int endian)
+{
+if (endian == ELFDATA2LSB) {
+val = cpu_to_le64(val);
+} else {
+val = cpu_to_be64(val);
+}
+
+return val;
+}
+
+typedef struct DumpState {
+ArchDumpInfo dump_info;
+MemoryMappingList list;
+uint16_t phdr_num;
+uint32_t sh_info;
+bool have_section;
+bool resume;
+target_phys_addr_t memory_offset;
+int fd;
+
+RAMBlock *block;
+ram_addr_t start;
+bool has_filter;
+int64_t begin;
+int64_t length;
+Error **errp;
+} DumpState;
+
+static int dump_cleanup(DumpState *s)
+{
+int ret = 0;
+
+memory_mapping_list_free(&s->list);
+if (s->fd != -1) {
+close(s->fd);
+}
+if (s->resume) {
+vm_start();
+}
+
+return ret;
+}
+
+static void dump_error(DumpState *s, const char *reason)
+{
+dump_cleanup(s);
+}
+
+static int fd_write_vmcore(target_phys_addr_t offset, void *buf, size_t size,
+   void *opaque)
+{
+DumpState *s = opaque;
+int fd = s->fd;
+off_t ret;
+size_t writen_size;
+
+while (1) {
+ret = lseek(fd, offset, SEEK_SET);
+if (ret < 0) {
+if (errno == ESPIPE) {
+error_set(s->errp, QERR_PIPE_OR_SOCKET_FD);
+return -1;
+}
+
+if (errno != EINTR && errno != EAGAIN) {
+return -1;
+}
+continue;
+}
+break;
+}
+
+/* The fd may be passed from user, and it can be non-blocked */
+while (size) {
+writen_size = qemu_write_full(fd, buf, size);
+if (writen_size != size && errno != EAGAIN) {
+return -1;
+}
+
+buf += writen_size;
+size -= writen_size;
+}
+
+return 0;
+}
+
+static int write_elf64_header(DumpState *s)
+{
+Elf64_Ehdr elf_header;
+int ret;
+int endian = s->dump_info.d_endian;
+
+memset(&elf_header, 0, sizeof(Elf64_Ehdr));
+memcpy(&elf_header, ELFMAG, SELFMAG);
+elf_header.e_

[Qemu-devel] KVM call agenda for Tuesday 27

2012-03-26 Thread Juan Quintela

Hi

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

Cheers,

Juan.



Re: [Qemu-devel] Thoughts around dtrace linking...

2012-03-26 Thread Stefan Hajnoczi
On Fri, Mar 23, 2012 at 2:11 PM, Lee Essen  wrote:
>
> On 23 Mar 2012, at 08:08, Stefan Hajnoczi wrote:
>
>> On Thu, Mar 22, 2012 at 05:00:53PM +, Lee Essen wrote:
>>> On 22/03/2012 16:28, Stefan Hajnoczi wrote:
 On Wed, Mar 21, 2012 at 1:01 PM, Andreas Färber  wrote:
> Hi,
>
> Am 21.03.2012 11:45, schrieb Lee Essen:
>> I've been trying to find a sensible way to solve the Solaris/Illumos
>> dtrace requirement to pass all the objs to the dtrace command so that
>> the resultant object file contains all the symbols needed to properly
>> link the relevant binary.
>>
>>
>> If you're able to try out the dependency-based approach that would be a
>> good starting point.  You may hit a point where it turns out to be too
>> ugly or complicated - in that case, please post your progress and maybe
>> someone can help find a way to structure it.  I'm not a make guru but I
>> can try to give feedback on your patches.
>>
>> Stefan
>>
>
> Ok, here's an attempt … it works for me, but I'm not sure how it would work 
> on a different dtrace platform so it will probably need some different 
> guarding … but it shows what it will look like.

The code makes sense to me.  There's a feeling that there must be a
way to remove the duplication in this approach, but my make foo isn't
good enough to see how.

As you mentioned, we need a way to distinguish between Solaris DTrace,
which requires the global .o linking approach, and other
implementations.  You could use CONFIG_SOLARIS and CONFIG_TRACE_DTRACE
together to enable the global .o linking.

Stefan



Re: [Qemu-devel] console class in kvm

2012-03-26 Thread Avi Kivity
On 03/26/2012 11:48 AM, Michael S. Tsirkin wrote:
> kvm used to carry this commit:

Used to?  Which commit reverts this?

> commit 4667e6ec0df770867095d8093562d93c94d96ca2
> Author: Avi Kivity 
> Date:   Thu Feb 12 11:43:17 2009 +0200
>
> Change virtio-console to PCI_CLASS_OTHERS
> 
> As a PCI_CLASS_DISPLAY_OTHER, it reduces primary display somehow on 
> Windows XP
> (possibly Windows disables acceleration since it fails to find a driver).
> 
> Signed-off-by: Avi Kivity 
>
> This seems to have been dropped. Is the issue gone?
> Could relevant parties speak up please?
> Do we want to merge this commit into qemu.git?

It's an impossible compatibility problem now.  I have this sinking
feeling that we need to create yet another driver property.

-- 
error compiling committee.c: too many arguments to function




Re: [Qemu-devel] [QEMU][RFC PATCH 3/6] memory: Add xen memory hook

2012-03-26 Thread Stefano Stabellini
On Sun, 25 Mar 2012, Avi Kivity wrote:
> On 03/23/2012 06:37 PM, Jan Kiszka wrote:
> > On 2012-03-23 16:08, Julien Grall wrote:
> > > On 03/22/2012 05:44 PM, Jan Kiszka wrote:
> > >>>
> > >>>   static void core_region_nop(MemoryListener *listener,
> > >>> diff --git a/ioport.c b/ioport.c
> > >>> index 78a3b89..073ed75 100644
> > >>> --- a/ioport.c
> > >>> +++ b/ioport.c
> > >>> @@ -28,6 +28,7 @@
> > >>>   #include "ioport.h"
> > >>>   #include "trace.h"
> > >>>   #include "memory.h"
> > >>> +#include "hw/xen.h"
> > >>>
> > >>>   /***/
> > >>>   /* IO Port */
> > >>> @@ -155,6 +156,11 @@ int register_ioport_read(pio_addr_t start, int
> > >>> length, int size,
> > >>>i);
> > >>>   ioport_opaque[i] = opaque;
> > >>>   }
> > >>> +
> > >>> +if (xen_enabled()) {
> > >>> +xen_map_iorange(start, length, 0);
> > >>> +}
> > >>> +
> > >>>   return 0;
> > >>>   }
> > >>>
> > >>> @@ -175,7 +181,13 @@ int register_ioport_write(pio_addr_t start, int
> > >>> length, int size,
> > >>>i);
> > >>>   ioport_opaque[i] = opaque;
> > >>>   }
> > >>> +
> > >>> +if (xen_enabled()) {
> > >>> +xen_map_iorange(start, length, 0);
> > >>> +}
> > >>> +
> > >>>   return 0;
> > >>> +
> > >>>   }
> > >>>
> > >>>   static uint32_t ioport_readb_thunk(void *opaque, uint32_t addr)
> > >>> @@ -260,6 +272,11 @@ void isa_unassign_ioport(pio_addr_t start, int
> > >>> length)
> > >>>   ioport_destructor_table[start](ioport_opaque[start]);
> > >>>   ioport_destructor_table[start] = NULL;
> > >>>   }
> > >>> +
> > >>> +if (xen_enabled()) {
> > >>> +xen_unmap_iorange(start, length, 0);
> > >>> +}
> > >>> +
> > >>>   for(i = start; i<  start + length; i++) {
> > >>>   ioport_read_table[0][i] = NULL;
> > >>>   ioport_read_table[1][i] = NULL;
> > >>>  
> > >> memory_listener_register(xen_hooks, system_io)?
> > >>
> > > QEMU doesn't seem to call region_add/region_del for ioport.
> > > Moreover, some of ioport are directly register without
> > > using memory hook (for example cirrus vga).
> > > 
> > > What is the best way to do it ?
> >
> > I haven't looked at details. Maybe it is just a combination of "use case
> > not yet considered, but can easily be added" and "need to switch legacy
> > code to new scheme". Then this still remains the better option than this
> > hook. Avi?
> 
> Just the second - region_add/del will be called, but only for ioports
> registered via the MemoryRegion APIs.

It looks like there are quite a few register_ioport_read/write left
around, especially in the following files:

hw/acpi_piix4.c
hw/cirrus_vga.c
hw/serial.c
hw/pckbd.c
hw/pc.c

I guess they should all be converted to memory_region_init_io, right?



Re: [Qemu-devel] [QEMU][RFC PATCH 3/6] memory: Add xen memory hook

2012-03-26 Thread Avi Kivity
On 03/26/2012 01:01 PM, Stefano Stabellini wrote:
> On Sun, 25 Mar 2012, Avi Kivity wrote:
> > On 03/23/2012 06:37 PM, Jan Kiszka wrote:
> > > On 2012-03-23 16:08, Julien Grall wrote:
> > > > On 03/22/2012 05:44 PM, Jan Kiszka wrote:
> > > >>>
> > > >>>   static void core_region_nop(MemoryListener *listener,
> > > >>> diff --git a/ioport.c b/ioport.c
> > > >>> index 78a3b89..073ed75 100644
> > > >>> --- a/ioport.c
> > > >>> +++ b/ioport.c
> > > >>> @@ -28,6 +28,7 @@
> > > >>>   #include "ioport.h"
> > > >>>   #include "trace.h"
> > > >>>   #include "memory.h"
> > > >>> +#include "hw/xen.h"
> > > >>>
> > > >>>   /***/
> > > >>>   /* IO Port */
> > > >>> @@ -155,6 +156,11 @@ int register_ioport_read(pio_addr_t start, int
> > > >>> length, int size,
> > > >>>i);
> > > >>>   ioport_opaque[i] = opaque;
> > > >>>   }
> > > >>> +
> > > >>> +if (xen_enabled()) {
> > > >>> +xen_map_iorange(start, length, 0);
> > > >>> +}
> > > >>> +
> > > >>>   return 0;
> > > >>>   }
> > > >>>
> > > >>> @@ -175,7 +181,13 @@ int register_ioport_write(pio_addr_t start, int
> > > >>> length, int size,
> > > >>>i);
> > > >>>   ioport_opaque[i] = opaque;
> > > >>>   }
> > > >>> +
> > > >>> +if (xen_enabled()) {
> > > >>> +xen_map_iorange(start, length, 0);
> > > >>> +}
> > > >>> +
> > > >>>   return 0;
> > > >>> +
> > > >>>   }
> > > >>>
> > > >>>   static uint32_t ioport_readb_thunk(void *opaque, uint32_t addr)
> > > >>> @@ -260,6 +272,11 @@ void isa_unassign_ioport(pio_addr_t start, int
> > > >>> length)
> > > >>>   ioport_destructor_table[start](ioport_opaque[start]);
> > > >>>   ioport_destructor_table[start] = NULL;
> > > >>>   }
> > > >>> +
> > > >>> +if (xen_enabled()) {
> > > >>> +xen_unmap_iorange(start, length, 0);
> > > >>> +}
> > > >>> +
> > > >>>   for(i = start; i<  start + length; i++) {
> > > >>>   ioport_read_table[0][i] = NULL;
> > > >>>   ioport_read_table[1][i] = NULL;
> > > >>>  
> > > >> memory_listener_register(xen_hooks, system_io)?
> > > >>
> > > > QEMU doesn't seem to call region_add/region_del for ioport.
> > > > Moreover, some of ioport are directly register without
> > > > using memory hook (for example cirrus vga).
> > > > 
> > > > What is the best way to do it ?
> > >
> > > I haven't looked at details. Maybe it is just a combination of "use case
> > > not yet considered, but can easily be added" and "need to switch legacy
> > > code to new scheme". Then this still remains the better option than this
> > > hook. Avi?
> > 
> > Just the second - region_add/del will be called, but only for ioports
> > registered via the MemoryRegion APIs.
>
> It looks like there are quite a few register_ioport_read/write left
> around, especially in the following files:
>
> hw/acpi_piix4.c
> hw/cirrus_vga.c
> hw/serial.c
> hw/pckbd.c
> hw/pc.c
>
> I guess they should all be converted to memory_region_init_io, right?

Right.

-- 
error compiling committee.c: too many arguments to function




Re: [Qemu-devel] console class in kvm

2012-03-26 Thread Michael S. Tsirkin
On Mon, Mar 26, 2012 at 12:18:54PM +0200, Avi Kivity wrote:
> On 03/26/2012 11:48 AM, Michael S. Tsirkin wrote:
> > kvm used to carry this commit:
> 
> Used to?  Which commit reverts this?

A merge from qemu.git I would guess.
git log does not seem to show the culprit, I don't know
how to find it.

> > commit 4667e6ec0df770867095d8093562d93c94d96ca2
> > Author: Avi Kivity 
> > Date:   Thu Feb 12 11:43:17 2009 +0200
> >
> > Change virtio-console to PCI_CLASS_OTHERS
> > 
> > As a PCI_CLASS_DISPLAY_OTHER, it reduces primary display somehow on 
> > Windows XP
> > (possibly Windows disables acceleration since it fails to find a 
> > driver).
> > 
> > Signed-off-by: Avi Kivity 
> >
> > This seems to have been dropped. Is the issue gone?
> > Could relevant parties speak up please?
> > Do we want to merge this commit into qemu.git?
> 
> It's an impossible compatibility problem now.  I have this sinking
> feeling that we need to create yet another driver property.

That's easy, we have a class property for this already.

> -- 
> error compiling committee.c: too many arguments to function



Re: [Qemu-devel] [libvirt] Modern CPU models cannot be used with libvirt

2012-03-26 Thread Jiri Denemark
On Sun, Mar 25, 2012 at 10:26:57 -0500, Anthony Liguori wrote:
> On 03/25/2012 10:16 AM, Avi Kivity wrote:
> > On 03/25/2012 04:59 PM, Anthony Liguori wrote:
> So how about:
> 
> 1) Load ['@SYSCONFDIR@/qemu/qemu.cfg', '@SYSCONFDIR@/qemu/target-@ARCH@.cfg',
>   '@DATADIR@/system.cfg', '@DATADIR@/system-@ARCH@.cfg']
> 
> 2) system-@ARCH@.cfg will contain:
> 
> [system]
> readconfig=@DATADIR@/target-@a...@-cpus.cfg
> readconfig=@DATADIR@/target-@a...@-machine.cfg
> 
> 3) -nodefconfig will not load any configuration files from DATADIR or 
> SYSCONFDIR.  -no-user-config will not load any configuration files from 
> SYSCONFDIR.
> 
...
> > The command line becomes unstable if you use -nodefconfig.
> 
> -no-user-config solves this but I fully expect libvirt would continue to use 
> -nodefconfig.

Libvirt uses -nodefaults -nodefconfig because it wants to fully control how
the virtual machine will look like (mainly in terms of devices). In other
words, we don't want any devices to just magically appear without libvirt
knowing about them. -nodefaults gets rid of default devices that are built
directly in qemu. Since users can set any devices or command line options
(such as enable-kvm) into qemu configuration files in @SYSCONFDIR@, we need to
avoid reading those files as well. Hence we use -nodefconfig. However, we
would still like qemu to read CPU definitions, machine types, etc. once they
become externally loaded configuration (or however we decide to call it). That
said, when CPU definitions are moved into @DATADIR@, and -no-user-config is
introduced, I don't see any reason for libvirt to keep using -nodefconfig.

I actually like
-no-user-config
more than
-nodefconfig -readconfig @DATADIR@/...
since it would avoid additional magic to detect what files libvirt should
explicitly pass to -readconfig but basically any approach that would allow us
to do read files only from @DATADIR@ is much better than what we have with
-nodefconfig now.

Jirka



Re: [Qemu-devel] [QEMU][RFC PATCH 3/6] memory: Add xen memory hook

2012-03-26 Thread Julien Grall

On 03/26/2012 12:02 PM, Avi Kivity wrote:

On 03/26/2012 01:01 PM, Stefano Stabellini wrote:
   

On Sun, 25 Mar 2012, Avi Kivity wrote:
 

On 03/23/2012 06:37 PM, Jan Kiszka wrote:
   

On 2012-03-23 16:08, Julien Grall wrote:
 

On 03/22/2012 05:44 PM, Jan Kiszka wrote:
   

   static void core_region_nop(MemoryListener *listener,
diff --git a/ioport.c b/ioport.c
index 78a3b89..073ed75 100644
--- a/ioport.c
+++ b/ioport.c
@@ -28,6 +28,7 @@
   #include "ioport.h"
   #include "trace.h"
   #include "memory.h"
+#include "hw/xen.h"

   /***/
   /* IO Port */
@@ -155,6 +156,11 @@ int register_ioport_read(pio_addr_t start, int
length, int size,
i);
   ioport_opaque[i] = opaque;
   }
+
+if (xen_enabled()) {
+xen_map_iorange(start, length, 0);
+}
+
   return 0;
   }

@@ -175,7 +181,13 @@ int register_ioport_write(pio_addr_t start, int
length, int size,
i);
   ioport_opaque[i] = opaque;
   }
+
+if (xen_enabled()) {
+xen_map_iorange(start, length, 0);
+}
+
   return 0;
+
   }

   static uint32_t ioport_readb_thunk(void *opaque, uint32_t addr)
@@ -260,6 +272,11 @@ void isa_unassign_ioport(pio_addr_t start, int
length)
   ioport_destructor_table[start](ioport_opaque[start]);
   ioport_destructor_table[start] = NULL;
   }
+
+if (xen_enabled()) {
+xen_unmap_iorange(start, length, 0);
+}
+
   for(i = start; i<   start + length; i++) {
   ioport_read_table[0][i] = NULL;
   ioport_read_table[1][i] = NULL;

   

memory_listener_register(xen_hooks, system_io)?

 

QEMU doesn't seem to call region_add/region_del for ioport.
Moreover, some of ioport are directly register without
using memory hook (for example cirrus vga).

What is the best way to do it ?
   

I haven't looked at details. Maybe it is just a combination of "use case
not yet considered, but can easily be added" and "need to switch legacy
code to new scheme". Then this still remains the better option than this
hook. Avi?
 

Just the second - region_add/del will be called, but only for ioports
registered via the MemoryRegion APIs.
   

It looks like there are quite a few register_ioport_read/write left
around, especially in the following files:

hw/acpi_piix4.c
hw/cirrus_vga.c
hw/serial.c
hw/pckbd.c
hw/pc.c

I guess they should all be converted to memory_region_init_io, right?
 

Right

I will modify theses files and send a different patch series.



Re: [Qemu-devel] [Qemu-trivial] [PATCH] trace-events: Fix broken build caused by wrong format specifier

2012-03-26 Thread Stefan Hajnoczi
On Sat, Mar 24, 2012 at 08:13:34AM +0100, Stefan Weil wrote:
> mem is an uint64_t value, so %lx was wrong.
> 
> Signed-off-by: Stefan Weil 
> ---
>  trace-events |2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)

Normally build fixes are sent directly to qemu.git/master committers for
quick merging.  But this fix is only needed for 32-bit hosts that use
stderr tracing or another backend that uses the format string.

Thanks, applied to the trivial patches tree:
https://github.com/stefanha/qemu/commits/trivial-patches

Stefan



Re: [Qemu-devel] [QEMU][RFC PATCH 4/6] xen-pci: Register PCI in Xen

2012-03-26 Thread Stefano Stabellini
On Sun, 25 Mar 2012, Avi Kivity wrote:
> On 03/23/2012 01:02 PM, Stefano Stabellini wrote:
> > Maybe the best thing to do is to have a set of machine specific options
> > to select what devices need to be built in the machine.
> > Most devices already can be dynamically selected: NICs, usb, acpi,
> > cirrus, etc.
> > We already have a Xen machine (xenfv_machine) that uses pc_init1 to
> > initialize it.
> > We could have a simple bitmask to determine what devices need to be
> > enabled, then in pc_init1 we could have something like:
> >
> > if (devices & VGA_ENABLE) {
> > pc_vga_init();
> > }
> >
> > Given the number of enable variable already present in the code
> > (pci_enabled, acpi_enabled, usb_enabled, xen_enabled,
> >  cirrus_vga_enabled, ...), it might end up actually making the code more
> > readable. The flexibility could end up being useful in the generic case
> > as well, for testing if nothing else.
> >
> > We would still need to call xc_hvm_register_pcidev to register PCI
> > devices in Xen, but we wouldn't expect to fail unless there was a
> > misconfiguration somewhere. In that case we could just exit.
> >
> >
> >
> > Now the problem is: there isn't a simple way to specify the BDF where
> > you want to create the device; pci_create_simple takes a devfn but most
> > of the higher level functions (pc_vga_init, pci_nic_init_nofail, ...)
> > don't export the parameter at the moment.
> > We would need to be able to tell pc_vga_init where to create the card,
> > so we would have to export the devfn as a parameter.
> >
> 
> You already have total flexibility with the -device foo parameter.  It
> allows you to create any device, anywhere, with whatever configuration
> you want.  Use in conjunction with -nodefconfig.

Thanks, -device looks exactly like what we need!
However I think that the option to suppress the defaults is -nodefaults.


> You may want your own host/pci bridge that lacks the device 0
> configuration space.

In order not to disrupt the emulated machine in QEMU too much, I was
thinking to let QEMU create the default device 0 and device 1:

00:00.0 Host bridge: Intel Corporation 440FX - 82441FX PMC [Natoma] (rev 02)
00:01.0 ISA bridge: Intel Corporation 82371SB PIIX3 ISA [Natoma/Triton II]
00:01.1 IDE interface: Intel Corporation 82371SB PIIX3 IDE [Natoma/Triton II]
00:01.3 Bridge: Intel Corporation 82371AB/EB/MB PIIX4 ACPI (rev 03)

and then have only the first QEMU register itself for IO events in Xen
related to these devices. That means that only the first QEMU would
actually receive any events to handle while the other QEMUs would never
receive any events for these devices.

Then everything else would go through -device: a device is created
only if the command line option is passed and in that case QEMU
also registers itself as the handler of this specific device in Xen.

There is supposed to be no overlaps in the configuration, so if two
QEMUs both register for the same device Xen would return error and QEMU
would exit.


The reason for doing this is that I am not sure that all OSes would be
able to cope with the ISA bridge being at a location different than
00:01.0 or the IDE controller being on a different device from the ISA
bridge, considering that they are supposed to be two functions of the
same device (Intel PIIX southbridge).
So at that point we might as well leave them as they are and try to
disrupt the basic config at little as possible.



Re: [Qemu-devel] [PATCH v2 0/6] ARM: AREG0 conversion

2012-03-26 Thread Peter Maydell
On 24 March 2012 18:58, Blue Swirl  wrote:
> v2: fix patch 1, tweak patch 2 and rebase to master.
>
> URL     git://repo.or.cz/qemu/blueswirl.git
>        http://repo.or.cz/r/qemu/blueswirl.git
>
> Blue Swirl (6):
>  arm: move neon_tbl to neon_helper.c
>  arm: move saturating arithmetic to helper.c
>  arm: move other arithmetic to helper.c
>  arm: move cpsr and banked register access to helper.c
>  arm: move exception and wfi helpers to helper.c
>  arm: move load and store helpers, switch to AREG0 free mode

The patches themselves look OK, but do we really want to take
a 5% performance hit for this cleanup?

-- PMM



Re: [Qemu-devel] [RFC 0/9] QOM: qomify -netdev

2012-03-26 Thread Stefan Hajnoczi
On Mon, Mar 26, 2012 at 6:40 AM,   wrote:
> From: Zhi Yong Wu 
>
> Sending the patchset is mainly intended to get some comments and void the 
> wrong development direction.
>
> The patchset is used to qomify -netdev, but it introduce one infrastructure 
> for host devices based on raw Class and Object, not qdev. So they are not 
> related with DeviceClass and DeviceState.
>
> patch #1 introduce one new class and object for host devices.

A common infrastructure for host devices is useful.  The Property
mechanism in hw/qdev-properties.c is especially good.  I think host
devices will also need to 2-step creation process (init and realize).

I think the qdev-properties.c code can mostly be shared.  It currently
uses the qdev DeviceState class but I think it only really access the
Property array.

Therefore it should be possible to really share a single copy of the
code.  We don't need to duplicate Property for host devices, instead
we can extract it out of hw/.

Stefan



Re: [Qemu-devel] [QEMU][RFC PATCH 4/6] xen-pci: Register PCI in Xen

2012-03-26 Thread Avi Kivity
On 03/26/2012 01:45 PM, Stefano Stabellini wrote:
> > >
> > >
> > > Now the problem is: there isn't a simple way to specify the BDF where
> > > you want to create the device; pci_create_simple takes a devfn but most
> > > of the higher level functions (pc_vga_init, pci_nic_init_nofail, ...)
> > > don't export the parameter at the moment.
> > > We would need to be able to tell pc_vga_init where to create the card,
> > > so we would have to export the devfn as a parameter.
> > >
> > 
> > You already have total flexibility with the -device foo parameter.  It
> > allows you to create any device, anywhere, with whatever configuration
> > you want.  Use in conjunction with -nodefconfig.
>
> Thanks, -device looks exactly like what we need!
> However I think that the option to suppress the defaults is -nodefaults.

Correct, as I was just informed in another thread.

>
>
> > You may want your own host/pci bridge that lacks the device 0
> > configuration space.
>
> In order not to disrupt the emulated machine in QEMU too much, I was
> thinking to let QEMU create the default device 0 and device 1:
>
> 00:00.0 Host bridge: Intel Corporation 440FX - 82441FX PMC [Natoma] (rev 02)
> 00:01.0 ISA bridge: Intel Corporation 82371SB PIIX3 ISA [Natoma/Triton II]
> 00:01.1 IDE interface: Intel Corporation 82371SB PIIX3 IDE [Natoma/Triton II]
> 00:01.3 Bridge: Intel Corporation 82371AB/EB/MB PIIX4 ACPI (rev 03)
>
> and then have only the first QEMU register itself for IO events in Xen
> related to these devices. That means that only the first QEMU would
> actually receive any events to handle while the other QEMUs would never
> receive any events for these devices.
>
> Then everything else would go through -device: a device is created
> only if the command line option is passed and in that case QEMU
> also registers itself as the handler of this specific device in Xen.
>
> There is supposed to be no overlaps in the configuration, so if two
> QEMUs both register for the same device Xen would return error and QEMU
> would exit.
>
>
> The reason for doing this is that I am not sure that all OSes would be
> able to cope with the ISA bridge being at a location different than
> 00:01.0 or the IDE controller being on a different device from the ISA
> bridge, considering that they are supposed to be two functions of the
> same device (Intel PIIX southbridge).
> So at that point we might as well leave them as they are and try to
> disrupt the basic config at little as possible.

Yes, but won't all qemus have those 00:01.0 devices and try to register
for them?

What about if two BARs (from different devices) are configured for the
same address ranges?

-- 
error compiling committee.c: too many arguments to function




Re: [Qemu-devel] [libvirt] Modern CPU models cannot be used with libvirt

2012-03-26 Thread Avi Kivity
On 03/26/2012 01:24 PM, Jiri Denemark wrote:
> ...
> > > The command line becomes unstable if you use -nodefconfig.
> > 
> > -no-user-config solves this but I fully expect libvirt would continue to 
> > use 
> > -nodefconfig.
>
> Libvirt uses -nodefaults -nodefconfig because it wants to fully control how
> the virtual machine will look like (mainly in terms of devices). In other
> words, we don't want any devices to just magically appear without libvirt
> knowing about them. -nodefaults gets rid of default devices that are built
> directly in qemu. Since users can set any devices or command line options
> (such as enable-kvm) into qemu configuration files in @SYSCONFDIR@, we need to
> avoid reading those files as well. Hence we use -nodefconfig. However, we
> would still like qemu to read CPU definitions, machine types, etc. once they
> become externally loaded configuration (or however we decide to call it). That
> said, when CPU definitions are moved into @DATADIR@, and -no-user-config is
> introduced, I don't see any reason for libvirt to keep using -nodefconfig.
>
> I actually like
> -no-user-config
> more than
> -nodefconfig -readconfig @DATADIR@/...
> since it would avoid additional magic to detect what files libvirt should
> explicitly pass to -readconfig but basically any approach that would allow us
> to do read files only from @DATADIR@ is much better than what we have with
> -nodefconfig now.

That's how I see it as well.

-- 
error compiling committee.c: too many arguments to function




Re: [Qemu-devel] [libvirt] Modern CPU models cannot be used with libvirt

2012-03-26 Thread Gleb Natapov
On Mon, Mar 26, 2012 at 01:59:05PM +0200, Avi Kivity wrote:
> On 03/26/2012 01:24 PM, Jiri Denemark wrote:
> > ...
> > > > The command line becomes unstable if you use -nodefconfig.
> > > 
> > > -no-user-config solves this but I fully expect libvirt would continue to 
> > > use 
> > > -nodefconfig.
> >
> > Libvirt uses -nodefaults -nodefconfig because it wants to fully control how
> > the virtual machine will look like (mainly in terms of devices). In other
> > words, we don't want any devices to just magically appear without libvirt
> > knowing about them. -nodefaults gets rid of default devices that are built
> > directly in qemu. Since users can set any devices or command line options
> > (such as enable-kvm) into qemu configuration files in @SYSCONFDIR@, we need 
> > to
> > avoid reading those files as well. Hence we use -nodefconfig. However, we
> > would still like qemu to read CPU definitions, machine types, etc. once they
> > become externally loaded configuration (or however we decide to call it). 
> > That
> > said, when CPU definitions are moved into @DATADIR@, and -no-user-config is
> > introduced, I don't see any reason for libvirt to keep using -nodefconfig.
> >
> > I actually like
> > -no-user-config
> > more than
> > -nodefconfig -readconfig @DATADIR@/...
> > since it would avoid additional magic to detect what files libvirt should
> > explicitly pass to -readconfig but basically any approach that would allow 
> > us
> > to do read files only from @DATADIR@ is much better than what we have with
> > -nodefconfig now.
> 
> That's how I see it as well.
> 
+1

except that instead of -no-user-config we can do what most other
programs do. If config file is specified during invocation default one
is not used. After implementing -no-user-config (or similar) we can drop
-nodefconfig entirely since its only user will be gone it its semantics
is not clear.

--
Gleb.



[Qemu-devel] [PATCH 1/5] vl.c: fix '-cpu ?' segfault

2012-03-26 Thread Stefan Hajnoczi
From: Eduardo Habkost 

Fix stupid copy&paste mistake at commit
ecf40beae7dcbb057d4f115207f9d8276832a774: I moved code around but kept
"optarg" on the cpu_list() call.

Reported-by: Jiri Denemark 
Signed-off-by: Eduardo Habkost 
Signed-off-by: Stefan Hajnoczi 
---
 vl.c |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/vl.c b/vl.c
index 112b0e0..0fccf50 100644
--- a/vl.c
+++ b/vl.c
@@ -3196,7 +3196,7 @@ int main(int argc, char **argv, char **envp)
 cpudef_init();
 
 if (cpu_model && *cpu_model == '?') {
-list_cpus(stdout, &fprintf, optarg);
+list_cpus(stdout, &fprintf, cpu_model);
 exit(0);
 }
 
-- 
1.7.9.1




[Qemu-devel] [PATCH 3/5] qapi: remove print statements from test-qmp-commands

2012-03-26 Thread Stefan Hajnoczi
From: Michael Roth 

This is necessary for nicer make check integration.

Signed-off-by: Michael Roth 
Signed-off-by: Stefan Hajnoczi 
---
 test-qmp-commands.c |3 ---
 1 files changed, 0 insertions(+), 3 deletions(-)

diff --git a/test-qmp-commands.c b/test-qmp-commands.c
index fa5a7bd..60cbf01 100644
--- a/test-qmp-commands.c
+++ b/test-qmp-commands.c
@@ -46,7 +46,6 @@ static void test_dispatch_cmd(void)
 resp = qmp_dispatch(QOBJECT(req));
 assert(resp != NULL);
 assert(!qdict_haskey(qobject_to_qdict(resp), "error"));
-g_print("\nresp: %s\n", qstring_get_str(qobject_to_json(resp)));
 
 qobject_decref(resp);
 QDECREF(req);
@@ -63,7 +62,6 @@ static void test_dispatch_cmd_error(void)
 resp = qmp_dispatch(QOBJECT(req));
 assert(resp != NULL);
 assert(qdict_haskey(qobject_to_qdict(resp), "error"));
-g_print("\nresp: %s\n", qstring_get_str(qobject_to_json_pretty(resp)));
 
 qobject_decref(resp);
 QDECREF(req);
@@ -92,7 +90,6 @@ static void test_dispatch_cmd_io(void)
 resp = qmp_dispatch(QOBJECT(req));
 assert(resp != NULL);
 assert(!qdict_haskey(qobject_to_qdict(resp), "error"));
-g_print("\nresp: %s\n", qstring_get_str(qobject_to_json_pretty(resp)));
 
 qobject_decref(resp);
 QDECREF(req);
-- 
1.7.9.1




[Qemu-devel] [PULL 0/5] Trivial patches for 20 to 26 March 2012

2012-03-26 Thread Stefan Hajnoczi
The following changes since commit cb1977d308f6e1d6bf398d42e6148187b82456c1:

  tcg-sparc: Add debug_frame support. (2012-03-24 19:57:58 +)

are available in the git repository at:

  git://github.com/stefanha/qemu.git trivial-patches

for you to fetch changes up to 95b752bc32ccabe48430c0d0062b7c6947d864d0:

  trace-events: Fix broken build caused by wrong format specifier (2012-03-26 
12:34:20 +0100)


Eduardo Habkost (1):
  vl.c: fix '-cpu ?' segfault

Michael Roth (3):
  test: remove qemu-ga reference
  qapi: remove print statements from test-qmp-commands
  test: add test-qmp-commands to make check

Stefan Weil (1):
  trace-events: Fix broken build caused by wrong format specifier

 Makefile|1 +
 test-qmp-commands.c |3 ---
 tests/Makefile  |3 ++-
 trace-events|2 +-
 vl.c|2 +-
 5 files changed, 5 insertions(+), 6 deletions(-)

-- 
1.7.9.1




[Qemu-devel] [PATCH 2/5] test: remove qemu-ga reference

2012-03-26 Thread Stefan Hajnoczi
From: Michael Roth 

This was added by mistake a while back.

Signed-off-by: Michael Roth 
Signed-off-by: Stefan Hajnoczi 
---
 Makefile   |1 +
 tests/Makefile |2 +-
 2 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/Makefile b/Makefile
index 8d6b558..d8e1f36 100644
--- a/Makefile
+++ b/Makefile
@@ -173,6 +173,7 @@ qemu-img-cmds.h: $(SRC_PATH)/qemu-img-cmds.hx
 $(qapi-obj-y): $(GENERATED_HEADERS)
 qapi-dir := $(BUILD_DIR)/qapi-generated
 qemu-ga$(EXESUF): LIBS = $(LIBS_QGA)
+qemu-ga$(EXESUF): QEMU_CFLAGS += -I $(qapi-dir)
 
 gen-out-type = $(subst .,-,$(suffix $@))
 
diff --git a/tests/Makefile b/tests/Makefile
index c78ade1..354fdbb 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -17,7 +17,7 @@ test-coroutine: test-coroutine.o qemu-timer-common.o async.o 
$(coroutine-obj-y)
 
 test-qmp-input-visitor.o test-qmp-output-visitor.o \
 test-string-input-visitor.o test-string-output-visitor.o \
-   test-qmp-commands.o qemu-ga$(EXESUF): QEMU_CFLAGS += -I $(qapi-dir)
+   test-qmp-commands.o: QEMU_CFLAGS += -I $(qapi-dir)
 
 $(qapi-dir)/test-qapi-types.c $(qapi-dir)/test-qapi-types.h :\
 $(SRC_PATH)/qapi-schema-test.json $(SRC_PATH)/scripts/qapi-types.py
-- 
1.7.9.1




[Qemu-devel] [PATCH 4/5] test: add test-qmp-commands to make check

2012-03-26 Thread Stefan Hajnoczi
From: Michael Roth 

All the deps are here but the test was never added to the list of tests
for make check

Signed-off-by: Michael Roth 
Signed-off-by: Stefan Hajnoczi 
---
 tests/Makefile |1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/tests/Makefile b/tests/Makefile
index 354fdbb..94ea342 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -3,6 +3,7 @@ export SRC_PATH
 CHECKS = check-qdict check-qfloat check-qint check-qstring check-qlist
 CHECKS += check-qjson test-qmp-output-visitor test-qmp-input-visitor
 CHECKS += test-string-input-visitor test-string-output-visitor test-coroutine
+CHECKS += test-qmp-commands
 CHECKS += $(SRC_PATH)/tests/qemu-iotests-quick.sh
 
 check-qint.o check-qstring.o check-qdict.o check-qlist.o check-qfloat.o 
check-qjson.o test-coroutine.o: $(GENERATED_HEADERS)
-- 
1.7.9.1




Re: [Qemu-devel] [QEMU][RFC PATCH 4/6] xen-pci: Register PCI in Xen

2012-03-26 Thread Stefano Stabellini
On Mon, 26 Mar 2012, Avi Kivity wrote:
> > > You may want your own host/pci bridge that lacks the device 0
> > > configuration space.
> >
> > In order not to disrupt the emulated machine in QEMU too much, I was
> > thinking to let QEMU create the default device 0 and device 1:
> >
> > 00:00.0 Host bridge: Intel Corporation 440FX - 82441FX PMC [Natoma] (rev 02)
> > 00:01.0 ISA bridge: Intel Corporation 82371SB PIIX3 ISA [Natoma/Triton II]
> > 00:01.1 IDE interface: Intel Corporation 82371SB PIIX3 IDE [Natoma/Triton 
> > II]
> > 00:01.3 Bridge: Intel Corporation 82371AB/EB/MB PIIX4 ACPI (rev 03)
> >
> > and then have only the first QEMU register itself for IO events in Xen
> > related to these devices. That means that only the first QEMU would
> > actually receive any events to handle while the other QEMUs would never
> > receive any events for these devices.
> >
> > Then everything else would go through -device: a device is created
> > only if the command line option is passed and in that case QEMU
> > also registers itself as the handler of this specific device in Xen.
> >
> > There is supposed to be no overlaps in the configuration, so if two
> > QEMUs both register for the same device Xen would return error and QEMU
> > would exit.
> >
> >
> > The reason for doing this is that I am not sure that all OSes would be
> > able to cope with the ISA bridge being at a location different than
> > 00:01.0 or the IDE controller being on a different device from the ISA
> > bridge, considering that they are supposed to be two functions of the
> > same device (Intel PIIX southbridge).
> > So at that point we might as well leave them as they are and try to
> > disrupt the basic config at little as possible.
> 
> Yes, but won't all qemus have those 00:01.0 devices and try to register
> for them?

Yes, this is where it becomes ugly again.

One possibility would be to have a special option, maybe on xenstore, to
tell QEMU "(do not)register North and South Bridge".
In the register_device callback that we'll have in xen-all.c, we'll have
a brief list of "special devices" that we might want to ignore even if
they are being emulated.
If this approach turns out to be too ugly from the code point of view,
we might have to make 00:00.0 and 00:01.X configurable too.


> What about if two BARs (from different devices) are configured for the
> same address ranges?

I think that it should have the same chance of happening as if there was
just one QEMU, because from the guest OS and firmware POV the emulated
hardware is the same. In any case Xen would return an error and QEMU can
either exit or try to cope with it.



Re: [Qemu-devel] [Xen-devel] [XEN][RFC PATCH 03/15] hvm-pci: Handle PCI config space in Xen

2012-03-26 Thread Julien Grall

On 03/23/2012 08:29 AM, Jan Beulich wrote:

Is there a reasonably low enforced boundary on the number
of devices? Otherwise, a linear lookup would seem overly
simple to me.
   

The maximum of bdf is 2^16 => 65536.
Which kind of structure do you advice ? Array ? Hash Table ?


Further, with how PCI_CMP_BDF() is defined, you're doing the
wrong thing here anyway - bit 31 is required to be set for the
port CFC access to be a config space one. Plus there's an AMD
extension to this interface, so I think other than shifting out
the low 8 bits and checking that the high bit is set, you shouldn't
do any other masking here.
   

Actually in config address register the 24-30 bits are reserved.
So, do I need to mask it ?

Moreover what is the AMD extension ?


Jan

   

+
+/* We just fill the ioreq, hvm_send_assist_req will send the request */
+if (unlikely(pci == NULL))
+{
+*val = ~0;
+rc = X86EMUL_OKAY;
+goto end_handle;
+}
+
+p->type = IOREQ_TYPE_PCI_CONFIG;
+p->addr = (pci_cf8&  ~3) + (p->addr&  3);
+
+set_ioreq(v,&pci->server->ioreq, p);
+
+end_handle:
+spin_unlock(&v->domain->arch.hvm_domain.pci_root.pci_lock);
+return rc;
+}
 


   





Re: [Qemu-devel] [PATCH 0/6] refactor PC machine, i440fx and piix3 to take advantage of QOM

2012-03-26 Thread Jan Kiszka
On 2012-03-26 04:06, Wanpeng Li wrote:
> From: Anthony Liguori 
> 
> 
> This series aggressively refactors the PC machine initialization to be more
> modelled and less ad-hoc.  The highlights of this series are:
> 
>  1) Things like -m and -bios-name are now device model properties
> 
>  2) The i440fx and piix3 are now modelled in a thorough fashion
> 
>  3) Most of the chipset features of the piix3 are modelled through composition
> 
>  4) i440fx_init is trivialized to creating devices and setting properties
> 
>  5) convert MemoryRegion to QOM
> 
>  6) convert PCI host bridge to QOM
> 
> The point (4) is the most important one.  As we refactor in this fashion,
> we should quickly get to the point where machine->init disappears completely 
> in
> favor of just creating a handful of devices.
> 
> The two stage initialization of QOM is important here.  instance_init() is 
> when
> composed devices are created which means that after you've created a device, 
> all
> of its children are visible in the device model.  This lets you set properties
> of the parent and its children.
> 
> realize() (which is still called DeviceState::init today) will be called right
> before the guest starts up for the first time.

While I see the value of the overall direction, I still disagree on
making internal data structures of HPET, RTC and 8254 publicly
available. That's a wrong step back. I'm sure there are smarter
solutions, alse as there were some proposals back then in the original
thread.

I'm also sure we will have to refactor the merge significantly again for
the introduction of additional chipsets and PC boards. But unless those
requirements can already be specified (Isaku?), that might be unavoidable.

Jan

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



[Qemu-devel] [PATCH v2] pcnet: Properly handle TX requests during Link Fail

2012-03-26 Thread Jan Kiszka
As long as we have no link and we aren't in internal loopback mode, no
packet must be sent. Instead, LCAR needs to be set in any active TX
descriptor and also CERR in CSR0.

Signed-off-by: Jan Kiszka 
---

Change in v2:
 - We were lacking a reset of xmit_pos in case of a link-down error.
   That caused bogus underflow errors.

 hw/pcnet.c |   11 +++
 hw/pcnet.h |1 +
 2 files changed, 12 insertions(+), 0 deletions(-)

diff --git a/hw/pcnet.c b/hw/pcnet.c
index 7413409..d769b08 100644
--- a/hw/pcnet.c
+++ b/hw/pcnet.c
@@ -77,6 +77,7 @@ struct qemu_ether_header {
 #define CSR_DTX(S)   !!(((S)->csr[15])&0x0002)
 #define CSR_LOOP(S)  !!(((S)->csr[15])&0x0004)
 #define CSR_DXMTFCS(S)   !!(((S)->csr[15])&0x0008)
+#define CSR_INTL(S)  !!(((S)->csr[15])&0x0040)
 #define CSR_DRCVPA(S)!!(((S)->csr[15])&0x2000)
 #define CSR_DRCVBC(S)!!(((S)->csr[15])&0x4000)
 #define CSR_PROM(S)  !!(((S)->csr[15])&0x8000)
@@ -1234,6 +1235,15 @@ static void pcnet_transmit(PCNetState *s)
 if (BCR_SWSTYLE(s) != 1)
 add_crc = GET_FIELD(tmd.status, TMDS, ADDFCS);
 }
+if (s->lnkst == 0 &&
+(!CSR_LOOP(s) || (!CSR_INTL(s) && !BCR_TMAULOOP(s {
+SET_FIELD(&tmd.misc, TMDM, LCAR, 1);
+SET_FIELD(&tmd.status, TMDS, ERR, 1);
+SET_FIELD(&tmd.status, TMDS, OWN, 0);
+s->csr[0] |= 0xa000; /* ERR | CERR */
+s->xmit_pos = -1;
+goto txdone;
+}
 if (!GET_FIELD(tmd.status, TMDS, ENP)) {
 int bcnt = 4096 - GET_FIELD(tmd.length, TMDL, BCNT);
 s->phys_mem_read(s->dma_opaque, PHYSADDR(s, tmd.tbadr),
@@ -1262,6 +1272,7 @@ static void pcnet_transmit(PCNetState *s)
 s->xmit_pos = -1;
 }

+txdone:
 SET_FIELD(&tmd.status, TMDS, OWN, 0);
 TMDSTORE(&tmd, PHYSADDR(s,CSR_CXDA(s)));
 if (!CSR_TOKINTD(s) || (CSR_LTINTEN(s) && GET_FIELD(tmd.status,
TMDS, LTINT)))
diff --git a/hw/pcnet.h b/hw/pcnet.h
index edc81c9..803a2cc 100644
--- a/hw/pcnet.h
+++ b/hw/pcnet.h
@@ -20,6 +20,7 @@
 #define BCR_SWS  20
 #define BCR_PLAT 22

+#define BCR_TMAULOOP(S)  !!((S)->bcr[BCR_MC  ] & 0x4000)
 #define BCR_APROMWE(S)   !!((S)->bcr[BCR_MC  ] & 0x0100)
 #define BCR_DWIO(S)  !!((S)->bcr[BCR_BSBC] & 0x0080)
 #define BCR_SSIZE32(S)   !!((S)->bcr[BCR_SWS ] & 0x0100)




[Qemu-devel] [PATCH 5/5] trace-events: Fix broken build caused by wrong format specifier

2012-03-26 Thread Stefan Hajnoczi
From: Stefan Weil 

mem is an uint64_t value, so %lx was wrong.

Signed-off-by: Stefan Weil 
Signed-off-by: Stefan Hajnoczi 
---
 trace-events |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/trace-events b/trace-events
index 70f059d..db2cd39 100644
--- a/trace-events
+++ b/trace-events
@@ -726,7 +726,7 @@ ppm_save(const char *filename, void *display_surface) "%s 
surface=%p"
 # hw/qxl.c
 disable qxl_interface_set_mm_time(int qid, uint32_t mm_time) "%d %d"
 disable qxl_io_write_vga(int qid, const char *mode, uint32_t addr, uint32_t 
val) "%d %s addr=%u val=%u"
-qxl_create_guest_primary(int qid, uint32_t width, uint32_t height, uint64_t 
mem, uint32_t format, uint32_t position) "%d %dx%d mem=%lx %d,%d"
+qxl_create_guest_primary(int qid, uint32_t width, uint32_t height, uint64_t 
mem, uint32_t format, uint32_t position) "%d %ux%u mem=%" PRIx64 " %u,%u"
 qxl_create_guest_primary_rest(int qid, int32_t stride, uint32_t type, uint32_t 
flags) "%d %d,%d,%d"
 qxl_destroy_primary(int qid) "%d"
 qxl_enter_vga_mode(int qid) "%d"
-- 
1.7.9.1




Re: [Qemu-devel] console class in kvm

2012-03-26 Thread Avi Kivity
On 03/26/2012 01:19 PM, Michael S. Tsirkin wrote:
> On Mon, Mar 26, 2012 at 12:18:54PM +0200, Avi Kivity wrote:
> > On 03/26/2012 11:48 AM, Michael S. Tsirkin wrote:
> > > kvm used to carry this commit:
> > 
> > Used to?  Which commit reverts this?
>
> A merge from qemu.git I would guess.
> git log does not seem to show the culprit, I don't know
> how to find it.

It was e2478f504fff20ad (git log  -SPCI_CLASS_OTHERS master -m -p).


> > > commit 4667e6ec0df770867095d8093562d93c94d96ca2
> > > Author: Avi Kivity 
> > > Date:   Thu Feb 12 11:43:17 2009 +0200
> > >
> > > Change virtio-console to PCI_CLASS_OTHERS
> > > 
> > > As a PCI_CLASS_DISPLAY_OTHER, it reduces primary display somehow on 
> > > Windows XP
> > > (possibly Windows disables acceleration since it fails to find a 
> > > driver).
> > > 
> > > Signed-off-by: Avi Kivity 
> > >
> > > This seems to have been dropped. Is the issue gone?
> > > Could relevant parties speak up please?
> > > Do we want to merge this commit into qemu.git?
> > 
> > It's an impossible compatibility problem now.  I have this sinking
> > feeling that we need to create yet another driver property.
>
> That's easy, we have a class property for this already.

Yes, d6beee9938.


-- 
error compiling committee.c: too many arguments to function




[Qemu-devel] [PATCH] ehci: fix ehci_child_detach

2012-03-26 Thread Gerd Hoffmann
Looks like a cut+paste bug from ehci_detach.  When the device itself is
detached from a ehci port (ehci_detach op) we have to clear the
device pointer for the companion port too.  When a device gets removed
from a downstream port of a usb hub (ehci_child_detach op) the ehci port
where the usb hub is plugged in is not affected.

Signed-off-by: Gerd Hoffmann 
---
 hw/usb/hcd-ehci.c |1 -
 1 files changed, 0 insertions(+), 1 deletions(-)

diff --git a/hw/usb/hcd-ehci.c b/hw/usb/hcd-ehci.c
index 60f9f5b..34f7538 100644
--- a/hw/usb/hcd-ehci.c
+++ b/hw/usb/hcd-ehci.c
@@ -797,7 +797,6 @@ static void ehci_child_detach(USBPort *port, USBDevice 
*child)
 if (portsc & PORTSC_POWNER) {
 USBPort *companion = s->companion_ports[port->index];
 companion->ops->child_detach(companion, child);
-companion->dev = NULL;
 return;
 }
 
-- 
1.7.1




Re: [Qemu-devel] [Xen-devel] [XEN][RFC PATCH 01/15] hvm: Modify interface to support multiple ioreq server

2012-03-26 Thread Julien Grall

On 03/23/2012 08:18 AM, Jan Beulich wrote:

+#define HVMOP_register_pcidev 24
+struct xen_hvm_register_pcidev {
+domid_t domid;/* IN - domain to be serviced */
+servid_t id;  /* IN - handle from HVMOP_register_ioreq_server */
+uint16_t bdf; /* IN - pci */
 

Can we please avoid the mistake of again not surfacing the PCI
segment in interface definitions, even if it may be required to be
zero for the immediate needs?
   


What do you hear by surfacing the PCI segment ?
Do I need to add the PCI domain ?




Re: [Qemu-devel] [QEMU][RFC PATCH 4/6] xen-pci: Register PCI in Xen

2012-03-26 Thread Avi Kivity
On 03/26/2012 02:20 PM, Stefano Stabellini wrote:
> On Mon, 26 Mar 2012, Avi Kivity wrote:
> > > > You may want your own host/pci bridge that lacks the device 0
> > > > configuration space.
> > >
> > > In order not to disrupt the emulated machine in QEMU too much, I was
> > > thinking to let QEMU create the default device 0 and device 1:
> > >
> > > 00:00.0 Host bridge: Intel Corporation 440FX - 82441FX PMC [Natoma] (rev 
> > > 02)
> > > 00:01.0 ISA bridge: Intel Corporation 82371SB PIIX3 ISA [Natoma/Triton II]
> > > 00:01.1 IDE interface: Intel Corporation 82371SB PIIX3 IDE [Natoma/Triton 
> > > II]
> > > 00:01.3 Bridge: Intel Corporation 82371AB/EB/MB PIIX4 ACPI (rev 03)
> > >
> > > and then have only the first QEMU register itself for IO events in Xen
> > > related to these devices. That means that only the first QEMU would
> > > actually receive any events to handle while the other QEMUs would never
> > > receive any events for these devices.
> > >
> > > Then everything else would go through -device: a device is created
> > > only if the command line option is passed and in that case QEMU
> > > also registers itself as the handler of this specific device in Xen.
> > >
> > > There is supposed to be no overlaps in the configuration, so if two
> > > QEMUs both register for the same device Xen would return error and QEMU
> > > would exit.
> > >
> > >
> > > The reason for doing this is that I am not sure that all OSes would be
> > > able to cope with the ISA bridge being at a location different than
> > > 00:01.0 or the IDE controller being on a different device from the ISA
> > > bridge, considering that they are supposed to be two functions of the
> > > same device (Intel PIIX southbridge).
> > > So at that point we might as well leave them as they are and try to
> > > disrupt the basic config at little as possible.
> > 
> > Yes, but won't all qemus have those 00:01.0 devices and try to register
> > for them?
>
> Yes, this is where it becomes ugly again.
>
> One possibility would be to have a special option, maybe on xenstore, to
> tell QEMU "(do not)register North and South Bridge".
> In the register_device callback that we'll have in xen-all.c, we'll have
> a brief list of "special devices" that we might want to ignore even if
> they are being emulated.
> If this approach turns out to be too ugly from the code point of view,
> we might have to make 00:00.0 and 00:01.X configurable too.
>
>
> > What about if two BARs (from different devices) are configured for the
> > same address ranges?
>
> I think that it should have the same chance of happening as if there was
> just one QEMU, because from the guest OS and firmware POV the emulated
> hardware is the same. In any case Xen would return an error and QEMU can
> either exit or try to cope with it.

How can qemu cope?  In a normal situation it's aware of all the devices,
here it's not aware of any device (except the one it is managing).

You're trying to convert a hierarchical problem into a flat one with no
communication.  What happens if one of the devices is a PCI-PCI bridge
and it turns off its PCI window?  The devices behind it should no longer
respond, yet they know nothing about it.

I think you need to preserve the hierarchy.  The host-pci bridge needs
to talk to devices behind it, (as does a pci-pci bridge).

-- 
error compiling committee.c: too many arguments to function




Re: [Qemu-devel] [PATCH 5/5] trace-events: Fix broken build caused by wrong format specifier

2012-03-26 Thread Andreas Färber
Am 26.03.2012 14:07, schrieb Stefan Hajnoczi:
> From: Stefan Weil 
> 
> mem is an uint64_t value, so %lx was wrong.
> 
> Signed-off-by: Stefan Weil 

Alon, you replied with an "ACK". Should an Acked-by be inserted here?

Andreas

> Signed-off-by: Stefan Hajnoczi 
> ---
>  trace-events |2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/trace-events b/trace-events
> index 70f059d..db2cd39 100644
> --- a/trace-events
> +++ b/trace-events
> @@ -726,7 +726,7 @@ ppm_save(const char *filename, void *display_surface) "%s 
> surface=%p"
>  # hw/qxl.c
>  disable qxl_interface_set_mm_time(int qid, uint32_t mm_time) "%d %d"
>  disable qxl_io_write_vga(int qid, const char *mode, uint32_t addr, uint32_t 
> val) "%d %s addr=%u val=%u"
> -qxl_create_guest_primary(int qid, uint32_t width, uint32_t height, uint64_t 
> mem, uint32_t format, uint32_t position) "%d %dx%d mem=%lx %d,%d"
> +qxl_create_guest_primary(int qid, uint32_t width, uint32_t height, uint64_t 
> mem, uint32_t format, uint32_t position) "%d %ux%u mem=%" PRIx64 " %u,%u"
>  qxl_create_guest_primary_rest(int qid, int32_t stride, uint32_t type, 
> uint32_t flags) "%d %d,%d,%d"
>  qxl_destroy_primary(int qid) "%d"
>  qxl_enter_vga_mode(int qid) "%d"

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg



Re: [Qemu-devel] [PATCH 5/6] merge pc_piix.c to pc.c

2012-03-26 Thread Avi Kivity
On 03/26/2012 04:06 AM, Wanpeng Li wrote:
> From: Anthony Liguori 
>
> @@ -889,7 +900,7 @@ static DeviceState *apic_init(void *env, uint8_t apic_id)
>  DeviceState *dev;
>  static int apic_mapped;
>  
> -if (kvm_irqchip_in_kernel()) {
> +if (kvm_enabled() && kvm_irqchip_in_kernel()) {
>  dev = qdev_create(NULL, "kvm-apic");
>  } else {
>  dev = qdev_create(NULL, "apic");
> @@ -908,7 +919,7 @@ static DeviceState *apic_init(void *env, uint8_t apic_id)
>  }
>  
>  /* KVM does not support MSI yet. */
> -if (!kvm_irqchip_in_kernel()) {
> +if (!kvm_enabled() || !kvm_irqchip_in_kernel()) {
>  msi_supported = true;

Why these changes?

-- 
error compiling committee.c: too many arguments to function




Re: [Qemu-devel] [PATCH] ehci: fix ehci_child_detach

2012-03-26 Thread Hans de Goede

Hi,

Oh, good one:

Acked-by: Hans de Goede 

Regards,

Hans



On 03/26/2012 02:32 PM, Gerd Hoffmann wrote:

Looks like a cut+paste bug from ehci_detach.  When the device itself is
detached from a ehci port (ehci_detach op) we have to clear the
device pointer for the companion port too.  When a device gets removed
from a downstream port of a usb hub (ehci_child_detach op) the ehci port
where the usb hub is plugged in is not affected.

Signed-off-by: Gerd Hoffmann
---
  hw/usb/hcd-ehci.c |1 -
  1 files changed, 0 insertions(+), 1 deletions(-)

diff --git a/hw/usb/hcd-ehci.c b/hw/usb/hcd-ehci.c
index 60f9f5b..34f7538 100644
--- a/hw/usb/hcd-ehci.c
+++ b/hw/usb/hcd-ehci.c
@@ -797,7 +797,6 @@ static void ehci_child_detach(USBPort *port, USBDevice 
*child)
  if (portsc&  PORTSC_POWNER) {
  USBPort *companion = s->companion_ports[port->index];
  companion->ops->child_detach(companion, child);
-companion->dev = NULL;
  return;
  }





Re: [Qemu-devel] Ignoring errno makes QMP errors suck

2012-03-26 Thread Luiz Capitulino
On Mon, 26 Mar 2012 10:39:50 +0200
Kevin Wolf  wrote:

> Hi,
> 
> I keep getting reports of problems, with nice error descriptions that
> usually look very similar to what I produced here:
> 
> {"execute":"blockdev-snapshot-sync","arguments":{"device":"ide0-hd0","snapshot-file":"/tmp/backing.qcow2"}}
> {"error": {"class": "OpenFileFailed", "desc": "Could not open
> '/tmp/backing.qcow2'", "data": {"filename": "/tmp/backing.qcow2"}}}
> 
> Who can tell me what has happened here? Oh, yes, the command failed, I
> would have guessed that from the "error" key. But the actual error
> description is as useless as it gets. It doesn't tell me anything about
> _why_ the snapshot couldn't be created. ("Permission denied" would have
> been the helpful additional information in this case)

There's a function called qemu_fopen_err() in the screendump conversion series
that return more specific errors. It will be trivial to add qemu_open_err()
as soon as qemu_fopen_err() is merged.

We're adding a bunch of more precise errors (some map directly to errno). That's
the easy part. The hard part is to convert everything to use them.

Note that while it's true that this shouldn't have leaked to QMP, good error
reporting is a general problem in QEMU.



Re: [Qemu-devel] Windows Virtio Issue

2012-03-26 Thread Paul Fisher
On 26 March 2012 09:50, Vadim Rozenfeld  wrote:

> On Sunday, March 25, 2012 07:01:54 PM Yan Vugenfirer wrote:
> Hi Paul,
> Could you try reproducing this problem on "-smp 2" guest, with
> small memory dump option turned on, instead of kernel memory dump.
>
> Thanks,
> Vadim.
>
>

Hi Vadim.

Thank you kindly for getting back to me.

I can certainly do that:

Dump and BSOD are in a zip at
http://dl.dropbox.com/u/12332019/2200-VirtioBSOD-2.zip

This time running with 2 cores:

qemu-kvm -m 1024 -smp sockets=1,cores=2 -cpu host -nodefaults -vga cirrus
-vnc :1  -drive if=none,id=block.0,format=raw,
cache=writeback,file=drive.img -device
virtio-blk-pci,bootindex=1,drive=block.0 -monitor stdio

Same procedure to get it to break - run crystal disk mark. Passed the
stream read/write and 512k block read/write - broke at the seek intensive
4k level.

Regards,
Paul


Re: [Qemu-devel] [PATCH v2 0/6] ARM: AREG0 conversion

2012-03-26 Thread Lluís Vilanova
Peter Maydell writes:

> On 24 March 2012 18:58, Blue Swirl  wrote:
>> v2: fix patch 1, tweak patch 2 and rebase to master.
>> 
>> URL     git://repo.or.cz/qemu/blueswirl.git
>>        http://repo.or.cz/r/qemu/blueswirl.git
>> 
>> Blue Swirl (6):
>>  arm: move neon_tbl to neon_helper.c
>>  arm: move saturating arithmetic to helper.c
>>  arm: move other arithmetic to helper.c
>>  arm: move cpsr and banked register access to helper.c
>>  arm: move exception and wfi helpers to helper.c
>>  arm: move load and store helpers, switch to AREG0 free mode

> The patches themselves look OK, but do we really want to take
> a 5% performance hit for this cleanup?

I was also wondering this. Is there any plan for recovering that 5% afterwards?


Thanks,
Lluis

-- 
 "And it's much the same thing with knowledge, for whenever you learn
 something new, the whole world becomes that much richer."
 -- The Princess of Pure Reason, as told by Norton Juster in The Phantom
 Tollbooth



Re: [Qemu-devel] [PATCH 0/6] refactor PC machine, i440fx and piix3 to take advantage of QOM

2012-03-26 Thread Andreas Färber
Am 26.03.2012 04:06, schrieb Wanpeng Li:
> From: Anthony Liguori 

Resending an old cover letter is not a good idea. This looks like a v2,
so please mark it as such in the subjects; it's missing a Change Log
against Anthony's version. I take it, some patches were dropped?

> This series aggressively refactors the PC machine initialization to be more
> modelled and less ad-hoc.  The highlights of this series are:
> 
>  1) Things like -m and -bios-name are now device model properties
> 
>  2) The i440fx and piix3 are now modelled in a thorough fashion
> 
>  3) Most of the chipset features of the piix3 are modelled through composition
> 
>  4) i440fx_init is trivialized to creating devices and setting properties
> 
>  5) convert MemoryRegion to QOM
> 
>  6) convert PCI host bridge to QOM
> 
> The point (4) is the most important one.  As we refactor in this fashion,
> we should quickly get to the point where machine->init disappears completely 
> in
> favor of just creating a handful of devices.

I agree that machine->init needs to be refactored, however I don't think
it'll disappear, just be moved into initfn/realize functions.

Andreas

> 
> The two stage initialization of QOM is important here.  instance_init() is 
> when
> composed devices are created which means that after you've created a device, 
> all
> of its children are visible in the device model.  This lets you set properties
> of the parent and its children.
> 
> realize() (which is still called DeviceState::init today) will be called right
> before the guest starts up for the first time.
> 
> Signed-off-by: Anthony Liguori 
> Signed-off-by: Wanpeng Li 
> 
> ---
>  Makefile.target |3 +-
>  hw/hpet.c   |   39 +---
>  hw/hpet_emul.h  |   41 +++
>  hw/i440fx.c |  431 ++
>  hw/i440fx.h |   78 +
>  hw/i8254_internal.h |2 +-
>  hw/mc146818rtc.c|   26 --
>  hw/mc146818rtc.h|   29 ++
>  hw/pc.c |  838 
> +--
>  hw/pc.h |   46 +---
>  hw/pc_piix.c|  762 --
>  hw/pci_host.c   |   26 ++
>  hw/pci_host.h   |5 +
>  hw/piix3.c  |  274 +
>  hw/piix3.h  |   79 +
>  hw/piix_pci.c   |  600 
>  memory.c|   94 +--
>  memory.h|8 +
>  18 files changed, 1795 insertions(+), 1586 deletions(-)
>  create mode 100644 hw/i440fx.c
>  create mode 100644 hw/i440fx.h
>  delete mode 100644 hw/pc_piix.c
>  create mode 100644 hw/piix3.c
>  create mode 100644 hw/piix3.h
>  delete mode 100644 hw/piix_pci.c
> --

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg



[Qemu-devel] [PATCH qemu+spice] expose server mouse status

2012-03-26 Thread Alon Levy
Below are the combined summaries. This lets the current mouse mode the server
is using be shown to qemu users:

(qemu) info spice
Server:
 address: 0.0.0.0:10005
auth: none
compiled: 0.10.2
  mouse-mode: server

qemu:

 Alon Levy (1):
   spice_info: add mouse_mode
 
  hmp.c|2 ++
  qapi-schema.json |7 ++-
  ui/spice-core.c  |5 +
  3 files changed, 13 insertions(+), 1 deletion(-)

spice:

 Alon Levy (1):
   server: export spice_server_is_server_mouse predicate
 
  server/reds.c|6 ++
  server/spice-server.syms |4 
  server/spice.h   |4 +++-
  3 files changed, 13 insertions(+), 1 deletion(-)

-- 
1.7.9.3




Re: [Qemu-devel] [PATCH 5/6] merge pc_piix.c to pc.c

2012-03-26 Thread Jan Kiszka
On 2012-03-26 14:42, Avi Kivity wrote:
> On 03/26/2012 04:06 AM, Wanpeng Li wrote:
>> From: Anthony Liguori 
>>
>> @@ -889,7 +900,7 @@ static DeviceState *apic_init(void *env, uint8_t apic_id)
>>  DeviceState *dev;
>>  static int apic_mapped;
>>  
>> -if (kvm_irqchip_in_kernel()) {
>> +if (kvm_enabled() && kvm_irqchip_in_kernel()) {
>>  dev = qdev_create(NULL, "kvm-apic");
>>  } else {
>>  dev = qdev_create(NULL, "apic");
>> @@ -908,7 +919,7 @@ static DeviceState *apic_init(void *env, uint8_t apic_id)
>>  }
>>  
>>  /* KVM does not support MSI yet. */
>> -if (!kvm_irqchip_in_kernel()) {
>> +if (!kvm_enabled() || !kvm_irqchip_in_kernel()) {
>>  msi_supported = true;
> 
> Why these changes?
> 

Yep, they are obsolete, likely related to the rebase of the original
patch. A lot of code is moved around here, and I bet there are more
artifacts...

Jan

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



[Qemu-devel] [PATCH] server: export spice_server_is_server_mouse predicate

2012-03-26 Thread Alon Levy
To be used by qemu query-spice / info spice commands.
---
 server/reds.c|6 ++
 server/spice-server.syms |4 
 server/spice.h   |4 +++-
 3 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/server/reds.c b/server/reds.c
index 27e7ea2..bf26864 100644
--- a/server/reds.c
+++ b/server/reds.c
@@ -4006,6 +4006,12 @@ SPICE_GNUC_VISIBLE int 
spice_server_get_peer_info(SpiceServer *s, struct sockadd
 return 0;
 }
 
+SPICE_GNUC_VISIBLE int spice_server_is_server_mouse(SpiceServer *s)
+{
+spice_assert(reds == s);
+return reds->mouse_mode == SPICE_MOUSE_MODE_SERVER;
+}
+
 SPICE_GNUC_VISIBLE int spice_server_add_renderer(SpiceServer *s, const char 
*name)
 {
 spice_assert(reds == s);
diff --git a/server/spice-server.syms b/server/spice-server.syms
index 4b842a3..99a7271 100644
--- a/server/spice-server.syms
+++ b/server/spice-server.syms
@@ -108,3 +108,7 @@ global:
 spice_server_set_uuid;
 spice_server_set_listen_socket_fd;
 } SPICE_SERVER_0.10.1;
+
+SPICE_SERVER_0.10.3 {
+spice_server_is_server_mouse;
+} SPICE_SERVER_0.10.2;
diff --git a/server/spice.h b/server/spice.h
index 8dd1c3d..77aec92 100644
--- a/server/spice.h
+++ b/server/spice.h
@@ -22,7 +22,7 @@
 #include 
 #include 
 
-#define SPICE_SERVER_VERSION 0x000a02 /* release 0.10.2 */
+#define SPICE_SERVER_VERSION 0x000a03 /* release 0.10.3 */
 
 /* interface base type */
 
@@ -485,6 +485,8 @@ int spice_server_set_agent_copypaste(SpiceServer *s, int 
enable);
 int spice_server_get_sock_info(SpiceServer *s, struct sockaddr *sa, socklen_t 
*salen);
 int spice_server_get_peer_info(SpiceServer *s, struct sockaddr *sa, socklen_t 
*salen);
 
+int spice_server_is_server_mouse(SpiceServer *s);
+
 /* migration interface */
 #define SPICE_INTERFACE_MIGRATION "migration"
 #define SPICE_INTERFACE_MIGRATION_MAJOR 1
-- 
1.7.9.3




[Qemu-devel] [PATCH] spice_info: add mouse_mode

2012-03-26 Thread Alon Levy
Add mouse_mode, either server or mouse, to qmp and hmp commands, based
on spice_server_is_server_mouse added in spice-server 0.10.3.

Signed-off-by: Alon Levy 
---
 hmp.c|2 ++
 qapi-schema.json |7 ++-
 ui/spice-core.c  |5 +
 3 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/hmp.c b/hmp.c
index 9cf2d13..c1224fb 100644
--- a/hmp.c
+++ b/hmp.c
@@ -350,6 +350,8 @@ void hmp_info_spice(Monitor *mon)
 }
 monitor_printf(mon, "auth: %s\n", info->auth);
 monitor_printf(mon, "compiled: %s\n", info->compiled_version);
+monitor_printf(mon, "  mouse-mode: %s\n",
+   info->has_mouse_mode ? info->mouse_mode : "unknown");
 
 if (!info->has_channels || info->channels == NULL) {
 monitor_printf(mon, "Channels: none\n");
diff --git a/qapi-schema.json b/qapi-schema.json
index 0d11d6e..72c0080 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -654,6 +654,11 @@
 #'spice' uses SASL or direct TLS authentication, depending on command
 #line options
 #
+# @mouse-mode: #optional current server mouse mode if spice server is new
+#enough and exposes this information.
+#'client' if client side
+#'server' if server side
+#
 # @channels: a list of @SpiceChannel for each active spice channel
 #
 # Since: 0.14.0
@@ -661,7 +666,7 @@
 { 'type': 'SpiceInfo',
   'data': {'enabled': 'bool', '*host': 'str', '*port': 'int',
'*tls-port': 'int', '*auth': 'str', '*compiled-version': 'str',
-   '*channels': ['SpiceChannel']} }
+   '*mouse-mode': 'str', '*channels': ['SpiceChannel']} }
 
 ##
 # @query-spice
diff --git a/ui/spice-core.c b/ui/spice-core.c
index a468524..0155dba 100644
--- a/ui/spice-core.c
+++ b/ui/spice-core.c
@@ -462,6 +462,11 @@ SpiceInfo *qmp_query_spice(Error **errp)
 info->tls_port = tls_port;
 }
 
+#if SPICE_SERVER_VERSION >= 0x000a03 /* 0.10.3 */
+info->has_mouse_mode = true;
+info->mouse_mode = g_strdup(spice_server_is_server_mouse(spice_server) ?
+"server" : "client");
+#endif
 /* for compatibility with the original command */
 info->has_channels = true;
 info->channels = qmp_query_spice_channels();
-- 
1.7.9.3




Re: [Qemu-devel] [PATCH v2 0/6] ARM: AREG0 conversion

2012-03-26 Thread Andreas Färber
Am 26.03.2012 14:46, schrieb Lluís Vilanova:
> Peter Maydell writes:
> 
>> On 24 March 2012 18:58, Blue Swirl  wrote:
>>> v2: fix patch 1, tweak patch 2 and rebase to master.
>>>
>>> URL git://repo.or.cz/qemu/blueswirl.git
>>>http://repo.or.cz/r/qemu/blueswirl.git
>>>
>>> Blue Swirl (6):
>>>  arm: move neon_tbl to neon_helper.c
>>>  arm: move saturating arithmetic to helper.c
>>>  arm: move other arithmetic to helper.c
>>>  arm: move cpsr and banked register access to helper.c
>>>  arm: move exception and wfi helpers to helper.c
>>>  arm: move load and store helpers, switch to AREG0 free mode
> 
>> The patches themselves look OK, but do we really want to take
>> a 5% performance hit for this cleanup?
> 
> I was also wondering this. Is there any plan for recovering that 5% 
> afterwards?

Maybe by switching to clang? ;)

Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg



Re: [Qemu-devel] [PATCH 5/5] trace-events: Fix broken build caused by wrong format specifier

2012-03-26 Thread Alon Levy
On Mon, Mar 26, 2012 at 02:41:23PM +0200, Andreas Färber wrote:
> Am 26.03.2012 14:07, schrieb Stefan Hajnoczi:
> > From: Stefan Weil 
> > 
> > mem is an uint64_t value, so %lx was wrong.
> > 
> > Signed-off-by: Stefan Weil 
> 
> Alon, you replied with an "ACK". Should an Acked-by be inserted here?

Yes, should have been verbose, sorry. But I don't think it matters much
for this kind of small patch.

> 
> Andreas
> 
> > Signed-off-by: Stefan Hajnoczi 
> > ---
> >  trace-events |2 +-
> >  1 files changed, 1 insertions(+), 1 deletions(-)
> > 
> > diff --git a/trace-events b/trace-events
> > index 70f059d..db2cd39 100644
> > --- a/trace-events
> > +++ b/trace-events
> > @@ -726,7 +726,7 @@ ppm_save(const char *filename, void *display_surface) 
> > "%s surface=%p"
> >  # hw/qxl.c
> >  disable qxl_interface_set_mm_time(int qid, uint32_t mm_time) "%d %d"
> >  disable qxl_io_write_vga(int qid, const char *mode, uint32_t addr, 
> > uint32_t val) "%d %s addr=%u val=%u"
> > -qxl_create_guest_primary(int qid, uint32_t width, uint32_t height, 
> > uint64_t mem, uint32_t format, uint32_t position) "%d %dx%d mem=%lx %d,%d"
> > +qxl_create_guest_primary(int qid, uint32_t width, uint32_t height, 
> > uint64_t mem, uint32_t format, uint32_t position) "%d %ux%u mem=%" PRIx64 " 
> > %u,%u"
> >  qxl_create_guest_primary_rest(int qid, int32_t stride, uint32_t type, 
> > uint32_t flags) "%d %d,%d,%d"
> >  qxl_destroy_primary(int qid) "%d"
> >  qxl_enter_vga_mode(int qid) "%d"
> 
> -- 
> SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
> GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg



Re: [Qemu-devel] [PATCH] pc: reduce duplication in compat machine types

2012-03-26 Thread Avi Kivity
On 03/26/2012 11:40 AM, Michael S. Tsirkin wrote:
> Make it easier to add compat properties, by
> adding macros for properties duplicated across
> machine types.
>
> Note: there could be bugs in compat properties,
> this patch does not attempt to address them,
> the code is bug for bug identical to the original.
>
> Tested by: generated a preprocessed file, sorted and
> compared to sorted original.
> Lightly tested on x86_64.
>
>  
> +#define PC_COMPAT_1_0 \
> +{\
> +.driver   = "pc-sysfw",\
> +.property = "rom_only",\
> +.value= stringify(1),\
> +}, {\
> +.driver   = "isa-fdc",\
> +.property = "check_media_rate",\
> +.value= "off",\
> +}
> +

Hmm.  how about

>  static QEMUMachine pc_machine_v1_0 = {
>  .name = "pc-1.0",
>  .desc = "Standard PC",
>  .init = pc_init_pci,
>  .max_cpus = 255,
>  .compat_props = (GlobalProperty[]) {
> -{
> -.driver   = "pc-sysfw",
> -.property = "rom_only",
> -.value= stringify(1),
> -}, {
> -.driver   = "isa-fdc",
> -.property = "check_media_rate",
> -.value= "off",
> -},
> +PC_COMPAT_1_0,

+.base_machine =  &pc_machine_v1_1;

Then it would be easier to define machines differentially.


>  { /* end of list */ }
>  },


-- 
error compiling committee.c: too many arguments to function




Re: [Qemu-devel] [Xen-devel] [XEN][RFC PATCH 03/15] hvm-pci: Handle PCI config space in Xen

2012-03-26 Thread Jan Beulich
>>> On 26.03.12 at 14:20, Julien Grall  wrote:
> On 03/23/2012 08:29 AM, Jan Beulich wrote:
>> Is there a reasonably low enforced boundary on the number
>> of devices? Otherwise, a linear lookup would seem overly
>> simple to me.
>>
> The maximum of bdf is 2^16 => 65536.
> Which kind of structure do you advice ? Array ? Hash Table ?

Radix tree, especially if you fold in the segment number.

>> Further, with how PCI_CMP_BDF() is defined, you're doing the
>> wrong thing here anyway - bit 31 is required to be set for the
>> port CFC access to be a config space one. Plus there's an AMD
>> extension to this interface, so I think other than shifting out
>> the low 8 bits and checking that the high bit is set, you shouldn't
>> do any other masking here.
>>
> Actually in config address register the 24-30 bits are reserved.
> So, do I need to mask it ?

Not necessarily - I'd suggest considering the part of the address
(which should generally result in a mismatch on any comparison. This
so that this ...

> Moreover what is the AMD extension ?

... can work without additional code. For an implementation, please
have a look at current Linux'es arch/x86/pci/direct.c - bits 24...27
are used for extended config space register accesses (which will be
needed for advanced PCIe or PCI-X functionality, and which may
be particularly important as long as we don't emulate MMCFG - at
least I don't think we do).

Jan




Re: [Qemu-devel] [Xen-devel] [XEN][RFC PATCH 01/15] hvm: Modify interface to support multiple ioreq server

2012-03-26 Thread Jan Beulich
>>> On 26.03.12 at 14:32, Julien Grall  wrote:
> On 03/23/2012 08:18 AM, Jan Beulich wrote:
>>> +#define HVMOP_register_pcidev 24
>>> +struct xen_hvm_register_pcidev {
>>> +domid_t domid;/* IN - domain to be serviced */
>>> +servid_t id;  /* IN - handle from HVMOP_register_ioreq_server */
>>> +uint16_t bdf; /* IN - pci */
>>>  
>> Can we please avoid the mistake of again not surfacing the PCI
>> segment in interface definitions, even if it may be required to be
>> zero for the immediate needs?
>>
> 
> What do you hear by surfacing the PCI segment ?
> Do I need to add the PCI domain ?

"domain" and "segment" appear to be used interchangeably.

Jan




Re: [Qemu-devel] [Spice-devel] [PATCH qemu+spice] expose server mouse status

2012-03-26 Thread Arnon Gilboa

ACK series
Acked-by: Arnon Gilboa 

Alon Levy wrote:

Below are the combined summaries. This lets the current mouse mode the server
is using be shown to qemu users:

(qemu) info spice
Server:
 address: 0.0.0.0:10005
auth: none
compiled: 0.10.2
  mouse-mode: server

qemu:

 Alon Levy (1):
   spice_info: add mouse_mode
 
  hmp.c|2 ++

  qapi-schema.json |7 ++-
  ui/spice-core.c  |5 +
  3 files changed, 13 insertions(+), 1 deletion(-)

spice:

 Alon Levy (1):
   server: export spice_server_is_server_mouse predicate
 
  server/reds.c|6 ++

  server/spice-server.syms |4 
  server/spice.h   |4 +++-
  3 files changed, 13 insertions(+), 1 deletion(-)

  





Re: [Qemu-devel] [PATCH 0/6] refactor PC machine, i440fx and piix3 to take advantage of QOM

2012-03-26 Thread Wanpeng Li
On Mon, Mar 26, 2012 at 02:47:19PM +0200, Andreas Färber wrote:
>Am 26.03.2012 04:06, schrieb Wanpeng Li:
>> From: Anthony Liguori 
>
>Resending an old cover letter is not a good idea. This looks like a v2,
>so please mark it as such in the subjects; it's missing a Change Log
>against Anthony's version. I take it, some patches were dropped?
>
No, I just help him rebase his patches.

>> This series aggressively refactors the PC machine initialization to be more
>> modelled and less ad-hoc.  The highlights of this series are:
>> 
>>  1) Things like -m and -bios-name are now device model properties
>> 
>>  2) The i440fx and piix3 are now modelled in a thorough fashion
>> 
>>  3) Most of the chipset features of the piix3 are modelled through 
>> composition
>> 
>>  4) i440fx_init is trivialized to creating devices and setting properties
>> 
>>  5) convert MemoryRegion to QOM
>> 
>>  6) convert PCI host bridge to QOM
>> 
>> The point (4) is the most important one.  As we refactor in this fashion,
>> we should quickly get to the point where machine->init disappears completely 
>> in
>> favor of just creating a handful of devices.
>
>I agree that machine->init needs to be refactored, however I don't think
>it'll disappear, just be moved into initfn/realize functions.
>
>Andreas
>
>> 
>> The two stage initialization of QOM is important here.  instance_init() is 
>> when
>> composed devices are created which means that after you've created a device, 
>> all
>> of its children are visible in the device model.  This lets you set 
>> properties
>> of the parent and its children.
>> 
>> realize() (which is still called DeviceState::init today) will be called 
>> right
>> before the guest starts up for the first time.
>> 
>> Signed-off-by: Anthony Liguori 
>> Signed-off-by: Wanpeng Li 
>> 
>> ---
>>  Makefile.target |3 +-
>>  hw/hpet.c   |   39 +---
>>  hw/hpet_emul.h  |   41 +++
>>  hw/i440fx.c |  431 ++
>>  hw/i440fx.h |   78 +
>>  hw/i8254_internal.h |2 +-
>>  hw/mc146818rtc.c|   26 --
>>  hw/mc146818rtc.h|   29 ++
>>  hw/pc.c |  838 
>> +--
>>  hw/pc.h |   46 +---
>>  hw/pc_piix.c|  762 --
>>  hw/pci_host.c   |   26 ++
>>  hw/pci_host.h   |5 +
>>  hw/piix3.c  |  274 +
>>  hw/piix3.h  |   79 +
>>  hw/piix_pci.c   |  600 
>>  memory.c|   94 +--
>>  memory.h|8 +
>>  18 files changed, 1795 insertions(+), 1586 deletions(-)
>>  create mode 100644 hw/i440fx.c
>>  create mode 100644 hw/i440fx.h
>>  delete mode 100644 hw/pc_piix.c
>>  create mode 100644 hw/piix3.c
>>  create mode 100644 hw/piix3.h
>>  delete mode 100644 hw/piix_pci.c
>> --
>
>-- 
>SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
>GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
>

-- 
LTC China, IBM, Shanghai




Re: [Qemu-devel] [PATCH v2 0/6] ARM: AREG0 conversion

2012-03-26 Thread Paul Brook
> On 24 March 2012 18:58, Blue Swirl  wrote:
> > v2: fix patch 1, tweak patch 2 and rebase to master.
> > 
> > URL git://repo.or.cz/qemu/blueswirl.git
> >http://repo.or.cz/r/qemu/blueswirl.git
> > 
> > Blue Swirl (6):
> >  arm: move neon_tbl to neon_helper.c
> >  arm: move saturating arithmetic to helper.c
> >  arm: move other arithmetic to helper.c
> >  arm: move cpsr and banked register access to helper.c
> >  arm: move exception and wfi helpers to helper.c
> >  arm: move load and store helpers, switch to AREG0 free mode
> 
> The patches themselves look OK, but do we really want to take
> a 5% performance hit for this cleanup?

I have a similar concern.  I'd like to at least have some idea where this 
slowdown is coming from.

Paul



Re: [Qemu-devel] [PATCH] spice_info: add mouse_mode

2012-03-26 Thread Gerd Hoffmann
> +#if SPICE_SERVER_VERSION >= 0x000a03 /* 0.10.3 */
> +info->has_mouse_mode = true;
> +info->mouse_mode = g_strdup(spice_server_is_server_mouse(spice_server) ?
> +"server" : "client");

#else
  info->mouse_mode = "unknown";
#endif

cheers,
  Gerd



Re: [Qemu-devel] Ignoring errno makes QMP errors suck

2012-03-26 Thread Kevin Wolf
Am 26.03.2012 14:46, schrieb Luiz Capitulino:
> On Mon, 26 Mar 2012 10:39:50 +0200
> Kevin Wolf  wrote:
> 
>> Hi,
>>
>> I keep getting reports of problems, with nice error descriptions that
>> usually look very similar to what I produced here:
>>
>> {"execute":"blockdev-snapshot-sync","arguments":{"device":"ide0-hd0","snapshot-file":"/tmp/backing.qcow2"}}
>> {"error": {"class": "OpenFileFailed", "desc": "Could not open
>> '/tmp/backing.qcow2'", "data": {"filename": "/tmp/backing.qcow2"}}}
>>
>> Who can tell me what has happened here? Oh, yes, the command failed, I
>> would have guessed that from the "error" key. But the actual error
>> description is as useless as it gets. It doesn't tell me anything about
>> _why_ the snapshot couldn't be created. ("Permission denied" would have
>> been the helpful additional information in this case)
> 
> There's a function called qemu_fopen_err() in the screendump conversion series
> that return more specific errors. It will be trivial to add qemu_open_err()
> as soon as qemu_fopen_err() is merged.
> 
> We're adding a bunch of more precise errors (some map directly to errno). 
> That's
> the easy part. The hard part is to convert everything to use them.
> 
> Note that while it's true that this shouldn't have leaked to QMP, good error
> reporting is a general problem in QEMU.

I guess my point is that we're actually moving backwards here. In HMP
things like this do print the right error message (using error_report).
And the return code is passed all the way down to where the QMP error is
generated, it's just ignored there.

The last time I checked there was no easy way to handle it there because
errno and strerror(-errno) aren't things allowed in QMP messages. This
is the problem for which I wanted to get some attention.

Does the patch that you mentioned add a generic way for adding an
(converted) errno to QMP errors? Or does it split up existing errors
into more and finer grained errors?

Kevin



Re: [Qemu-devel] [PATCH] pc: reduce duplication in compat machine types

2012-03-26 Thread Andreas Färber
Am 26.03.2012 11:40, schrieb Michael S. Tsirkin:
> Make it easier to add compat properties, by
> adding macros for properties duplicated across
> machine types.
> 
> Note: there could be bugs in compat properties,
> this patch does not attempt to address them,
> the code is bug for bug identical to the original.
> 
> Tested by: generated a preprocessed file, sorted and
> compared to sorted original.
> Lightly tested on x86_64.
> 
> Signed-off-by: Michael S. Tsirkin 
> ---
> 
> I've put the above on my branch as it's needed for
> fixing balloon pci class. Let me know if there are
> any objections.

Better reusing code is certainly a good idea. I wonder though if we
might ever get into a situation where things change back and forth. I'd
hope that properties are processed top to bottom so that, e.g.,
PC_COMPAT_1_0, { ... } can overwrite 0_15 compat properties inherited
through 1_0.

Andreas

> 
>  hw/pc_piix.c |  288 
> +++---
>  1 files changed, 73 insertions(+), 215 deletions(-)
> 
> diff --git a/hw/pc_piix.c b/hw/pc_piix.c
> index 3f99f9a..e39ef69 100644
> --- a/hw/pc_piix.c
> +++ b/hw/pc_piix.c
> @@ -372,50 +372,69 @@ static QEMUMachine pc_machine_v1_1 = {
>  .is_default = 1,
>  };
>  
> +#define PC_COMPAT_1_0 \
> +{\
> +.driver   = "pc-sysfw",\
> +.property = "rom_only",\
> +.value= stringify(1),\
> +}, {\
> +.driver   = "isa-fdc",\
> +.property = "check_media_rate",\
> +.value= "off",\
> +}
> +
>  static QEMUMachine pc_machine_v1_0 = {
>  .name = "pc-1.0",
>  .desc = "Standard PC",
>  .init = pc_init_pci,
>  .max_cpus = 255,
>  .compat_props = (GlobalProperty[]) {
> -{
> -.driver   = "pc-sysfw",
> -.property = "rom_only",
> -.value= stringify(1),
> -}, {
> -.driver   = "isa-fdc",
> -.property = "check_media_rate",
> -.value= "off",
> -},
> +PC_COMPAT_1_0,
>  { /* end of list */ }
>  },
>  };
[snip]

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg



Re: [Qemu-devel] [QEMU][RFC PATCH 3/6] memory: Add xen memory hook

2012-03-26 Thread Avi Kivity
On 03/26/2012 01:24 PM, Julien Grall wrote:
>>> It looks like there are quite a few register_ioport_read/write left
>>> around, especially in the following files:
>>>
>>> hw/acpi_piix4.c
>>> hw/cirrus_vga.c
>>> hw/serial.c
>>> hw/pckbd.c
>>> hw/pc.c
>>>
>>> I guess they should all be converted to memory_region_init_io, right?
>>>  
>> Right
>I will modify theses files and send a different patch series.
>

Great.  Please post them as a separate series, they can go in relatively
quickly since they should be mostly straighforward.

-- 
error compiling committee.c: too many arguments to function




Re: [Qemu-devel] [PATCH] pc: reduce duplication in compat machine types

2012-03-26 Thread Michael S. Tsirkin
On Mon, Mar 26, 2012 at 03:12:37PM +0200, Andreas Färber wrote:
> Am 26.03.2012 11:40, schrieb Michael S. Tsirkin:
> > Make it easier to add compat properties, by
> > adding macros for properties duplicated across
> > machine types.
> > 
> > Note: there could be bugs in compat properties,
> > this patch does not attempt to address them,
> > the code is bug for bug identical to the original.
> > 
> > Tested by: generated a preprocessed file, sorted and
> > compared to sorted original.
> > Lightly tested on x86_64.
> > 
> > Signed-off-by: Michael S. Tsirkin 
> > ---
> > 
> > I've put the above on my branch as it's needed for
> > fixing balloon pci class. Let me know if there are
> > any objections.
> 
> Better reusing code is certainly a good idea. I wonder though if we
> might ever get into a situation where things change back and forth. I'd
> hope that properties are processed top to bottom so that, e.g.,
> PC_COMPAT_1_0, { ... } can overwrite 0_15 compat properties inherited
> through 1_0.
> 
> Andreas

There are some properties like that currently.
For example, see ide-drive "ver" property or
VGA rombar property.  I'm not sure that's intentional but I kept
it as is.

In that case we can simply keep it out of
the common macros, that's what I did. There's no rule that says
we must have everything in there.

> > 
> >  hw/pc_piix.c |  288 
> > +++---
> >  1 files changed, 73 insertions(+), 215 deletions(-)
> > 
> > diff --git a/hw/pc_piix.c b/hw/pc_piix.c
> > index 3f99f9a..e39ef69 100644
> > --- a/hw/pc_piix.c
> > +++ b/hw/pc_piix.c
> > @@ -372,50 +372,69 @@ static QEMUMachine pc_machine_v1_1 = {
> >  .is_default = 1,
> >  };
> >  
> > +#define PC_COMPAT_1_0 \
> > +{\
> > +.driver   = "pc-sysfw",\
> > +.property = "rom_only",\
> > +.value= stringify(1),\
> > +}, {\
> > +.driver   = "isa-fdc",\
> > +.property = "check_media_rate",\
> > +.value= "off",\
> > +}
> > +
> >  static QEMUMachine pc_machine_v1_0 = {
> >  .name = "pc-1.0",
> >  .desc = "Standard PC",
> >  .init = pc_init_pci,
> >  .max_cpus = 255,
> >  .compat_props = (GlobalProperty[]) {
> > -{
> > -.driver   = "pc-sysfw",
> > -.property = "rom_only",
> > -.value= stringify(1),
> > -}, {
> > -.driver   = "isa-fdc",
> > -.property = "check_media_rate",
> > -.value= "off",
> > -},
> > +PC_COMPAT_1_0,
> >  { /* end of list */ }
> >  },
> >  };
> [snip]
> 
> -- 
> SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
> GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg



Re: [Qemu-devel] Ignoring errno makes QMP errors suck

2012-03-26 Thread Luiz Capitulino
On Mon, 26 Mar 2012 15:13:36 +0200
Kevin Wolf  wrote:

> Am 26.03.2012 14:46, schrieb Luiz Capitulino:
> > On Mon, 26 Mar 2012 10:39:50 +0200
> > Kevin Wolf  wrote:
> > 
> >> Hi,
> >>
> >> I keep getting reports of problems, with nice error descriptions that
> >> usually look very similar to what I produced here:
> >>
> >> {"execute":"blockdev-snapshot-sync","arguments":{"device":"ide0-hd0","snapshot-file":"/tmp/backing.qcow2"}}
> >> {"error": {"class": "OpenFileFailed", "desc": "Could not open
> >> '/tmp/backing.qcow2'", "data": {"filename": "/tmp/backing.qcow2"}}}
> >>
> >> Who can tell me what has happened here? Oh, yes, the command failed, I
> >> would have guessed that from the "error" key. But the actual error
> >> description is as useless as it gets. It doesn't tell me anything about
> >> _why_ the snapshot couldn't be created. ("Permission denied" would have
> >> been the helpful additional information in this case)
> > 
> > There's a function called qemu_fopen_err() in the screendump conversion 
> > series
> > that return more specific errors. It will be trivial to add qemu_open_err()
> > as soon as qemu_fopen_err() is merged.
> > 
> > We're adding a bunch of more precise errors (some map directly to errno). 
> > That's
> > the easy part. The hard part is to convert everything to use them.
> > 
> > Note that while it's true that this shouldn't have leaked to QMP, good error
> > reporting is a general problem in QEMU.
> 
> I guess my point is that we're actually moving backwards here. In HMP
> things like this do print the right error message (using error_report).
> And the return code is passed all the way down to where the QMP error is
> generated, it's just ignored there.
> 
> The last time I checked there was no easy way to handle it there because
> errno and strerror(-errno) aren't things allowed in QMP messages. This
> is the problem for which I wanted to get some attention.

What we're doing now is to add QErrors that map to errno. So, for example,
we have PermissionDenied for EPERM.

I think this is exactly the same thing, except:

 1. We don't use the errno number, because this may differ among OSs
 2. We don't use the sterrror() string to allow for internationalization,
but we have our own string that should have the same end result

> Does the patch that you mentioned add a generic way for adding an
> (converted) errno to QMP errors? Or does it split up existing errors
> into more and finer grained errors?

The latter. The QMP errors have to be added manually. But it's just a matter
of time to get the most used errnos added.



Re: [Qemu-devel] [PATCH] spice_info: add mouse_mode

2012-03-26 Thread Alon Levy
On Mon, Mar 26, 2012 at 03:06:22PM +0200, Gerd Hoffmann wrote:
> > +#if SPICE_SERVER_VERSION >= 0x000a03 /* 0.10.3 */
> > +info->has_mouse_mode = true;
> > +info->mouse_mode = g_strdup(spice_server_is_server_mouse(spice_server) 
> > ?
> > +"server" : 
> > "client");
> 
> #else
>   info->mouse_mode = "unknown";
> #endif

Why? I don't set has_mouse_mode in this case, which defaults to 0 ==
false because of the malloc0, and then I check has_mouse_mode in the hmp
command, and the qmp just won't send the field because has == false.

> 
> cheers,
>   Gerd



Re: [Qemu-devel] [PATCH] pc: reduce duplication in compat machine types

2012-03-26 Thread Michael S. Tsirkin
On Mon, Mar 26, 2012 at 02:51:38PM +0200, Avi Kivity wrote:
> On 03/26/2012 11:40 AM, Michael S. Tsirkin wrote:
> > Make it easier to add compat properties, by
> > adding macros for properties duplicated across
> > machine types.
> >
> > Note: there could be bugs in compat properties,
> > this patch does not attempt to address them,
> > the code is bug for bug identical to the original.
> >
> > Tested by: generated a preprocessed file, sorted and
> > compared to sorted original.
> > Lightly tested on x86_64.
> >
> >  
> > +#define PC_COMPAT_1_0 \
> > +{\
> > +.driver   = "pc-sysfw",\
> > +.property = "rom_only",\
> > +.value= stringify(1),\
> > +}, {\
> > +.driver   = "isa-fdc",\
> > +.property = "check_media_rate",\
> > +.value= "off",\
> > +}
> > +
> 
> Hmm.  how about
> 
> >  static QEMUMachine pc_machine_v1_0 = {
> >  .name = "pc-1.0",
> >  .desc = "Standard PC",
> >  .init = pc_init_pci,
> >  .max_cpus = 255,
> >  .compat_props = (GlobalProperty[]) {
> > -{
> > -.driver   = "pc-sysfw",
> > -.property = "rom_only",
> > -.value= stringify(1),
> > -}, {
> > -.driver   = "isa-fdc",
> > -.property = "check_media_rate",
> > -.value= "off",
> > -},
> > +PC_COMPAT_1_0,
> 
> +.base_machine =  &pc_machine_v1_1;
> 

It's a lot of work, and the result won't be easier to
modify or debug than it already is, IMO.

> Then it would be easier to define machines differentially.

We add new 1 machine each release, so I'm not worried about
adding them easily, adding properies easily happens
between releases so I want to make is simpler.

> >  { /* end of list */ }
> >  },

> 
> -- 
> error compiling committee.c: too many arguments to function



Re: [Qemu-devel] Ignoring errno makes QMP errors suck

2012-03-26 Thread Anthony Liguori

On 03/26/2012 03:39 AM, Kevin Wolf wrote:

Hi,

I keep getting reports of problems, with nice error descriptions that
usually look very similar to what I produced here:

{"execute":"blockdev-snapshot-sync","arguments":{"device":"ide0-hd0","snapshot-file":"/tmp/backing.qcow2"}}
{"error": {"class": "OpenFileFailed", "desc": "Could not open
'/tmp/backing.qcow2'", "data": {"filename": "/tmp/backing.qcow2"}}}


This is not QMP's fault.  This is the block layers.  Specifically, you're 
missing:

diff --git a/blockdev.c b/blockdev.c
index 1a500b8..04c3a39 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -777,7 +777,11 @@ void qmp_transaction(BlockdevActionList *dev_list, Error **
   states->old_bs->drv->format_name,
   NULL, -1, flags);
 if (ret) {
-error_set(errp, QERR_OPEN_FILE_FAILED, new_image_file);
+if (ret == -EPERM) {
+error_set(errp, QERR_PERMISSION_DENIED);
+} else {
+error_set(errp, QERR_OPEN_FILE_FAILED, new_image_file);
+}
 goto delete_and_fail;
 }
 }

Which is handling:

ret = bdrv_img_create(new_image_file, format,
  states->old_bs->filename,
  states->old_bs->drv->format_name,
  NULL, -1, flags);

But it would be even better to push Error ** into bdrv_img_create().  There's 
only two callers so it would be trivial to do that.  Then you could do:


diff --git a/block.c b/block.c
index b88ee90..a7bf8a9 100644
--- a/block.c
+++ b/block.c
@@ -3881,7 +3881,8 @@ bdrv_acct_done(BlockDriverState *bs, BlockAcctCookie *cook

 int bdrv_img_create(const char *filename, const char *fmt,
 const char *base_filename, const char *base_fmt,
-char *options, uint64_t img_size, int flags)
+char *options, uint64_t img_size, int flags,
+Error **errp)
 {
 QEMUOptionParameter *param = NULL, *create_options = NULL;
 QEMUOptionParameter *backing_fmt, *backing_file, *size;
@@ -3893,14 +3894,14 @@ int bdrv_img_create(const char *filename, const char *fm
 /* Find driver and parse its options */
 drv = bdrv_find_format(fmt);
 if (!drv) {
-error_report("Unknown file format '%s'", fmt);
+error_set(errp, QERR_INVALID_BLOCK_FORMAT, fmt);
 ret = -EINVAL;
 goto out;
 }

Etc.


Who can tell me what has happened here? Oh, yes, the command failed, I
would have guessed that from the "error" key. But the actual error
description is as useless as it gets. It doesn't tell me anything about
_why_ the snapshot couldn't be created. ("Permission denied" would have
been the helpful additional information in this case)

How should management tools ever be able to provide a helpful error
message to their users if all they get is this useless "something went
wrong" error?


You need to kill off error_report in the block layer and replace it with 
error_set.  The problem with error_report is that while you can understand what 
"Unknown file format 'qcow2'" means, management tools can't.  Responding that 
"the tool can just present that error to the user" implies that the management 
tool only provides an English-language interface which is not terribly friendly.


QMP provides all the infrastructure you need.   You just have to use it.

Regards,

Anthony Liguori


Kevin





Re: [Qemu-devel] Ignoring errno makes QMP errors suck

2012-03-26 Thread Anthony Liguori

On 03/26/2012 08:28 AM, Luiz Capitulino wrote:

On Mon, 26 Mar 2012 15:13:36 +0200
Kevin Wolf  wrote:


Am 26.03.2012 14:46, schrieb Luiz Capitulino:

On Mon, 26 Mar 2012 10:39:50 +0200
Kevin Wolf  wrote:


Hi,

I keep getting reports of problems, with nice error descriptions that
usually look very similar to what I produced here:

{"execute":"blockdev-snapshot-sync","arguments":{"device":"ide0-hd0","snapshot-file":"/tmp/backing.qcow2"}}
{"error": {"class": "OpenFileFailed", "desc": "Could not open
'/tmp/backing.qcow2'", "data": {"filename": "/tmp/backing.qcow2"}}}

Who can tell me what has happened here? Oh, yes, the command failed, I
would have guessed that from the "error" key. But the actual error
description is as useless as it gets. It doesn't tell me anything about
_why_ the snapshot couldn't be created. ("Permission denied" would have
been the helpful additional information in this case)


There's a function called qemu_fopen_err() in the screendump conversion series
that return more specific errors. It will be trivial to add qemu_open_err()
as soon as qemu_fopen_err() is merged.

We're adding a bunch of more precise errors (some map directly to errno). That's
the easy part. The hard part is to convert everything to use them.

Note that while it's true that this shouldn't have leaked to QMP, good error
reporting is a general problem in QEMU.


I guess my point is that we're actually moving backwards here. In HMP
things like this do print the right error message (using error_report).
And the return code is passed all the way down to where the QMP error is
generated, it's just ignored there.

The last time I checked there was no easy way to handle it there because
errno and strerror(-errno) aren't things allowed in QMP messages. This
is the problem for which I wanted to get some attention.


What we're doing now is to add QErrors that map to errno. So, for example,
we have PermissionDenied for EPERM.

I think this is exactly the same thing, except:

  1. We don't use the errno number, because this may differ among OSs
  2. We don't use the sterrror() string to allow for internationalization,
 but we have our own string that should have the same end result


Does the patch that you mentioned add a generic way for adding an
(converted) errno to QMP errors? Or does it split up existing errors
into more and finer grained errors?


The latter. The QMP errors have to be added manually. But it's just a matter
of time to get the most used errnos added.


Note that this whole discussion is somewhat irrelevant.  The error gets 
"converted" in the block code, not in generic QMP code.


If you want a error_set_from_errno() function, that's fairly trivial to add 
although you're missing the opportunity to add more useful information (like the 
name of the file).


Regards,

Anthony Liguori



Re: [Qemu-devel] console class in kvm

2012-03-26 Thread Avi Kivity
On 03/26/2012 03:37 PM, Michael S. Tsirkin wrote:
> Exactly. qemu-kvm used to set the class to CLASS_OTHER while
> the current code sets it to PCI_CLASS_COMMUNICATION_OTHER.
> Do we want support for CLASS_OTHER or is it ok to drop it?
>
>

Looks like starting with qemu-kvm-0.11, qemu-kvm matches qemu.  I think
we can let the compatibility problem with qemu-kvm-0.10 remain.

-- 
error compiling committee.c: too many arguments to function




Re: [Qemu-devel] [PATCH] spice_info: add mouse_mode

2012-03-26 Thread Gerd Hoffmann
On 03/26/12 15:30, Alon Levy wrote:
> On Mon, Mar 26, 2012 at 03:06:22PM +0200, Gerd Hoffmann wrote:
>>> +#if SPICE_SERVER_VERSION >= 0x000a03 /* 0.10.3 */
>>> +info->has_mouse_mode = true;
>>> +info->mouse_mode = g_strdup(spice_server_is_server_mouse(spice_server) 
>>> ?
>>> +"server" : 
>>> "client");
>>
>> #else
>>   info->mouse_mode = "unknown";
>> #endif
> 
> Why?

has_mouse_mode looks superfluous and makes the code a bit more
complicated than needed.

> I don't set has_mouse_mode in this case, which defaults to 0 ==
> false because of the malloc0, and then I check has_mouse_mode in the hmp
> command, and the qmp just won't send the field because has == false.

Ah, ok, didn't see qmp does something different, then I'd suggest to
just leave mouse_mode zero-initialized (aka NULL) when mouse mode info
isn't available.

cheers,
  Gerd




[Qemu-devel] [PATCH RFC 3/3] qdev: Hook up DeviceClass::init to ObjectClass::realize

2012-03-26 Thread Andreas Färber
On realize, call the qdev init function.
If that returns an error, raise QERR_DEVICE_INIT_FAILED.

Signed-off-by: Andreas Färber 
Cc: Anthony Liguori 
Cc: Paolo Bonzini 
---
 hw/qdev.c |   13 +
 1 files changed, 13 insertions(+), 0 deletions(-)

diff --git a/hw/qdev.c b/hw/qdev.c
index ee21d90..c84b656 100644
--- a/hw/qdev.c
+++ b/hw/qdev.c
@@ -649,6 +649,13 @@ static void device_finalize(Object *obj)
 QTAILQ_REMOVE(&dev->parent_bus->children, dev, sibling);
 }
 
+static int device_realize(Object *obj)
+{
+DeviceState *dev = DEVICE(obj);
+
+return qdev_init(dev);
+}
+
 void device_reset(DeviceState *dev)
 {
 DeviceClass *klass = DEVICE_GET_CLASS(dev);
@@ -658,6 +665,11 @@ void device_reset(DeviceState *dev)
 }
 }
 
+static void device_class_init(ObjectClass *class, void *data)
+{
+class->realize = device_realize;
+}
+
 static TypeInfo device_type_info = {
 .name = TYPE_DEVICE,
 .parent = TYPE_OBJECT,
@@ -666,6 +678,7 @@ static TypeInfo device_type_info = {
 .instance_finalize = device_finalize,
 .abstract = true,
 .class_size = sizeof(DeviceClass),
+.class_init = device_class_init,
 };
 
 static void qdev_register_types(void)
-- 
1.7.7




[Qemu-devel] [PATCH RFC 2/3] qom: Introduce object_realize()

2012-03-26 Thread Andreas Färber
Wrap setting of Object::realized property, error reporting and exit(1)
into a helper function.

Signed-off-by: Andreas Färber 
Cc: Anthony Liguori 
Cc: Paolo Bonzini 
---
 include/qemu/object.h |9 +
 qom/object.c  |   11 +++
 2 files changed, 20 insertions(+), 0 deletions(-)

diff --git a/include/qemu/object.h b/include/qemu/object.h
index 742b5b6..dc8ae0f 100644
--- a/include/qemu/object.h
+++ b/include/qemu/object.h
@@ -467,6 +467,15 @@ void object_initialize_with_type(void *data, Type type);
 void object_initialize(void *obj, const char *typename);
 
 /**
+ * object_realize:
+ * @obj: The object to realize.
+ *
+ * This function will complete the initialization of an object based on
+ * properties set by setting the "realized" property to true.
+ */
+void object_realize(Object *obj);
+
+/**
  * object_finalize:
  * @obj: The object to finalize.
  *
diff --git a/qom/object.c b/qom/object.c
index ec143ad..33f6efc 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -327,6 +327,17 @@ void object_initialize(void *data, const char *typename)
 object_initialize_with_type(data, type);
 }
 
+void object_realize(Object *obj)
+{
+Error *err = NULL;
+
+object_property_set_bool(obj, true, "realized", &err);
+if (error_is_set(&err)) {
+qerror_report_err(err);
+exit(1);
+}
+}
+
 static void object_property_del_all(Object *obj)
 {
 while (!QTAILQ_EMPTY(&obj->properties)) {
-- 
1.7.7




Re: [Qemu-devel] [PATCH RFC 3/3] qdev: Hook up DeviceClass::init to ObjectClass::realize

2012-03-26 Thread Andreas Färber
Am 26.03.2012 15:46, schrieb Andreas Färber:
> On realize, call the qdev init function.

> If that returns an error, raise QERR_DEVICE_INIT_FAILED.

Sorry, that sentence is outdated - the error is set in qom/object.c
(patch 1/3) for non-zero return values. It is merely being passed
through here.

Andreas

> 
> Signed-off-by: Andreas Färber 
> Cc: Anthony Liguori 
> Cc: Paolo Bonzini 
> ---
>  hw/qdev.c |   13 +
>  1 files changed, 13 insertions(+), 0 deletions(-)
> 
> diff --git a/hw/qdev.c b/hw/qdev.c
> index ee21d90..c84b656 100644
> --- a/hw/qdev.c
> +++ b/hw/qdev.c
> @@ -649,6 +649,13 @@ static void device_finalize(Object *obj)
>  QTAILQ_REMOVE(&dev->parent_bus->children, dev, sibling);
>  }
>  
> +static int device_realize(Object *obj)
> +{
> +DeviceState *dev = DEVICE(obj);
> +
> +return qdev_init(dev);
> +}
> +
>  void device_reset(DeviceState *dev)
>  {
>  DeviceClass *klass = DEVICE_GET_CLASS(dev);
[snip]

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg



Re: [Qemu-devel] [QEMU][RFC PATCH 4/6] xen-pci: Register PCI in Xen

2012-03-26 Thread Stefano Stabellini
On Mon, 26 Mar 2012, Avi Kivity wrote:
> > > What about if two BARs (from different devices) are configured for the
> > > same address ranges?
> >
> > I think that it should have the same chance of happening as if there was
> > just one QEMU, because from the guest OS and firmware POV the emulated
> > hardware is the same. In any case Xen would return an error and QEMU can
> > either exit or try to cope with it.
> 
> How can qemu cope?  In a normal situation it's aware of all the devices,
> here it's not aware of any device (except the one it is managing).
> 
> You're trying to convert a hierarchical problem into a flat one with no
> communication.  What happens if one of the devices is a PCI-PCI bridge
> and it turns off its PCI window?  The devices behind it should no longer
> respond, yet they know nothing about it.
> 
> I think you need to preserve the hierarchy.  The host-pci bridge needs
> to talk to devices behind it, (as does a pci-pci bridge).

PCI bridges are a problem, in fact I was thinking to just assume that
there are no PCI Bridges with PCI devices behind them other than the
Host PCI Bridge for now.

There is going to be a need for some kind of communication between the
multiple QEMU instances, for example for ACPI power state changes and
PCI Bridges, like you wrote. So I think that at some point we'll need to
introduce a type of message that originates from one of the QEMU
instances (the first one especially) and goes to all the others.
It should probably be relayed by the hypervisor.



Re: [Qemu-devel] Ignoring errno makes QMP errors suck

2012-03-26 Thread Kevin Wolf
Am 26.03.2012 15:28, schrieb Luiz Capitulino:
> On Mon, 26 Mar 2012 15:13:36 +0200
> Kevin Wolf  wrote:
> 
>> Am 26.03.2012 14:46, schrieb Luiz Capitulino:
>>> On Mon, 26 Mar 2012 10:39:50 +0200
>>> Kevin Wolf  wrote:
>>>
 Hi,

 I keep getting reports of problems, with nice error descriptions that
 usually look very similar to what I produced here:

 {"execute":"blockdev-snapshot-sync","arguments":{"device":"ide0-hd0","snapshot-file":"/tmp/backing.qcow2"}}
 {"error": {"class": "OpenFileFailed", "desc": "Could not open
 '/tmp/backing.qcow2'", "data": {"filename": "/tmp/backing.qcow2"}}}

 Who can tell me what has happened here? Oh, yes, the command failed, I
 would have guessed that from the "error" key. But the actual error
 description is as useless as it gets. It doesn't tell me anything about
 _why_ the snapshot couldn't be created. ("Permission denied" would have
 been the helpful additional information in this case)
>>>
>>> There's a function called qemu_fopen_err() in the screendump conversion 
>>> series
>>> that return more specific errors. It will be trivial to add qemu_open_err()
>>> as soon as qemu_fopen_err() is merged.
>>>
>>> We're adding a bunch of more precise errors (some map directly to errno). 
>>> That's
>>> the easy part. The hard part is to convert everything to use them.
>>>
>>> Note that while it's true that this shouldn't have leaked to QMP, good error
>>> reporting is a general problem in QEMU.
>>
>> I guess my point is that we're actually moving backwards here. In HMP
>> things like this do print the right error message (using error_report).
>> And the return code is passed all the way down to where the QMP error is
>> generated, it's just ignored there.
>>
>> The last time I checked there was no easy way to handle it there because
>> errno and strerror(-errno) aren't things allowed in QMP messages. This
>> is the problem for which I wanted to get some attention.
> 
> What we're doing now is to add QErrors that map to errno. So, for example,
> we have PermissionDenied for EPERM.

EACCES in this case, but yes.

> I think this is exactly the same thing, except:
> 
>  1. We don't use the errno number, because this may differ among OSs
>  2. We don't use the sterrror() string to allow for internationalization,
> but we have our own string that should have the same end result

Yes, I know the reasons why we can't use these options and they are good
reasons. If we get something to replace it (so that we can have a
errno_to_qmp()), that part should be solved.

>> Does the patch that you mentioned add a generic way for adding an
>> (converted) errno to QMP errors? Or does it split up existing errors
>> into more and finer grained errors?
> 
> The latter. The QMP errors have to be added manually. But it's just a matter
> of time to get the most used errnos added.

Your PermissionDenied example doesn't really do this. It is a generic
error message that may occur in multiple contexts, right? So in one
context you may need a file name as additional information, in another
context permission for something completely different may be missing
(especially if you include EPERM in the same error).

I think 'OpenFileFailed' is not too bad, it's just missing a field that
gives the detail 'PermissionDenied'. I'm not sure if having
'PermissionDenied' as the top-level error object is the best idea.

Kevin



Re: [Qemu-devel] console class in kvm

2012-03-26 Thread Michael S. Tsirkin
On Mon, Mar 26, 2012 at 02:29:58PM +0200, Avi Kivity wrote:
> On 03/26/2012 01:19 PM, Michael S. Tsirkin wrote:
> > On Mon, Mar 26, 2012 at 12:18:54PM +0200, Avi Kivity wrote:
> > > On 03/26/2012 11:48 AM, Michael S. Tsirkin wrote:
> > > > kvm used to carry this commit:
> > > 
> > > Used to?  Which commit reverts this?
> >
> > A merge from qemu.git I would guess.
> > git log does not seem to show the culprit, I don't know
> > how to find it.
> 
> It was e2478f504fff20ad (git log  -SPCI_CLASS_OTHERS master -m -p).
> 
> 
> > > > commit 4667e6ec0df770867095d8093562d93c94d96ca2
> > > > Author: Avi Kivity 
> > > > Date:   Thu Feb 12 11:43:17 2009 +0200
> > > >
> > > > Change virtio-console to PCI_CLASS_OTHERS
> > > > 
> > > > As a PCI_CLASS_DISPLAY_OTHER, it reduces primary display somehow on 
> > > > Windows XP
> > > > (possibly Windows disables acceleration since it fails to find a 
> > > > driver).
> > > > 
> > > > Signed-off-by: Avi Kivity 
> > > >
> > > > This seems to have been dropped. Is the issue gone?
> > > > Could relevant parties speak up please?
> > > > Do we want to merge this commit into qemu.git?
> > > 
> > > It's an impossible compatibility problem now.  I have this sinking
> > > feeling that we need to create yet another driver property.
> >
> > That's easy, we have a class property for this already.
> 
> Yes, d6beee9938.

Exactly. qemu-kvm used to set the class to CLASS_OTHER while
the current code sets it to PCI_CLASS_COMMUNICATION_OTHER.
Do we want support for CLASS_OTHER or is it ok to drop it?

> 
> -- 
> error compiling committee.c: too many arguments to function



Re: [Qemu-devel] [RFC 0/9] QOM: qomify -netdev

2012-03-26 Thread Zhi Yong Wu
On Mon, Mar 26, 2012 at 7:54 PM, Stefan Hajnoczi  wrote:
> On Mon, Mar 26, 2012 at 6:40 AM,   wrote:
>> From: Zhi Yong Wu 
>>
>> Sending the patchset is mainly intended to get some comments and void the 
>> wrong development direction.
>>
>> The patchset is used to qomify -netdev, but it introduce one infrastructure 
>> for host devices based on raw Class and Object, not qdev. So they are not 
>> related with DeviceClass and DeviceState.
>>
>> patch #1 introduce one new class and object for host devices.
>
> A common infrastructure for host devices is useful.  The Property
> mechanism in hw/qdev-properties.c is especially good.  I think host
> devices will also need to 2-step creation process (init and realize).
Yeah.
>
> I think the qdev-properties.c code can mostly be shared.  It currently
> uses the qdev DeviceState class but I think it only really access the
> Property array.
struct Property {
const char   *name;
PropertyInfo *info;
int  offset;
uint8_t  bitnr;
uint8_t  qtype;
int64_t  defval;
};

struct PropertyInfo {
const char *name;
const char *legacy_name;
const char **enum_table;
int64_t min;
int64_t max;
int (*parse)(DeviceState *dev, Property *prop, const char *str);
int (*print)(DeviceState *dev, Property *prop, char *dest, size_t len);
ObjectPropertyAccessor *get;
ObjectPropertyAccessor *set;
ObjectPropertyRelease *release;
};

Like above, its member functions pointers use DeviceState, so we will
have to consider how to make this PropertyInfo more generic and be
used by other Class and Object which are not based on DeviceState.

>
> Therefore it should be possible to really share a single copy of the
> code.  We don't need to duplicate Property for host devices, instead
> we can extract it out of hw/.
Yeah, but we need to make some effort to reach this goal.
>
> Stefan



-- 
Regards,

Zhi Yong Wu



Re: [Qemu-devel] [PATCH] block: add the support to drain throttled requests

2012-03-26 Thread Stefan Hajnoczi
On Tue, Mar 20, 2012 at 11:44 AM, Stefan Hajnoczi
 wrote:
> On Tue, Mar 20, 2012 at 10:58:10AM +0100, Kevin Wolf wrote:
>> Am 20.03.2012 10:47, schrieb Paolo Bonzini:
>> > Il 20/03/2012 10:40, Zhi Yong Wu ha scritto:
>> >> HI, Kevin,
>> >>
>> >> We hope that I/O throttling can be shipped without known issue in QEMU
>> >> 1.1, so if you are available, can you give this patch some love?
>> >
>> > I'm sorry to say this, but I think I/O throttling is impossible to save.
>> >  As it is implemented now, it just cannot work in the presence of
>> > synchronous I/O, except at the cost of busy waiting with the global
>> > mutex taken.  See the message from Stefan yesterday.
>>
>> qemu_aio_flush() is busy waiting with the global mutex taken anyway, so
>> it doesn't change that much.
>
> Yesterday I only posted an analysis of the bug but here are some
> thoughts on how to move forward.  Throttling itself is not the problem.
> We've known that synchronous operations in the vcpu thread are a problem
> long before throttling.  This is just another reason to convert device
> emulation to use asynchronous interfaces.
>
> Here is the list of device models that perform synchronous block I/O:
> hw/fdc.c
> hw/ide/atapi.c
> hw/ide/core.c
> hw/nand.c
> hw/onenand.c
> hw/pflash_cfi01.c
> hw/pflash_cfi02.c
> hw/sd.c
>
> Zhi Hui Li is working on hw/fdc.c and recently sent a patch.
>
> I think it's too close to QEMU 1.1 to convert all the remaining devices
> and test them properly before the soft-freeze.  But it's probably
> possible to convert IDE before the soft-freeze.
>
> In the meantime we could add this to bdrv_rw_co():
>
> if (bs->io_limits_enabled) {
>    fprintf(stderr, "Disabling I/O throttling on '%s' due "
>            "to synchronous I/O\n", bdrv_get_device_name(bs));
>    bdrv_io_limits_disable(bs);
> }
>
> It's not pretty but tells the user there is an issue and avoids
> deadlocking.

No one has commented on this suggestion.  I think leaving a known hang
in QEMU 1.1 is undesirable.  Better to have this warning and disable
throttling in the case we cannot support right now.

Kevin: Would you accept a patch like this?  Or do you have another
solution in mind?

Stefan



[Qemu-devel] [PATCH RFC 0/3] qom: Generalize qdev init to "realize"

2012-03-26 Thread Andreas Färber
Hello Anthony,

Here's a mini series introducing ObjectClass::realize(Object *) and
forwarding it to existing DeviceClass::init(DeviceState *).

I've also added a convenience wrapper for setting the "realized" property
and doing error checking and exit, tested with the SH7750 SoC.

Regards,
Andreas

Cc: Anthony Liguori 
Cc: Paolo Bonzini 
Cc: Wanpeng Li 

Andreas Färber (3):
  qom: Add "realized" property to Object
  qom: Introduce object_realize()
  qdev: Hook up DeviceClass::init to ObjectClass::realize

 hw/qdev.c |   13 +
 include/qemu/object.h |   11 +++
 qom/object.c  |   42 ++
 3 files changed, 66 insertions(+), 0 deletions(-)

-- 
1.7.7




Re: [Qemu-devel] [PATCH 3/6] convert pci-host to QOM

2012-03-26 Thread Andreas Färber
Am 26.03.2012 04:06, schrieb Wanpeng Li:
> From: Anthony Liguori 
> 
> 
> Signed-off-by: Anthony Liguori 
> Signed-off-by: Wanpeng Li 

Some minor formal comments inline, otherwise looks okay.

> ---
>  hw/pci_host.c |   26 ++
>  hw/pci_host.h |5 +
>  2 files changed, 31 insertions(+), 0 deletions(-)
> 
> diff --git a/hw/pci_host.c b/hw/pci_host.c
> index 44c6c20..44d7e55 100644
> --- a/hw/pci_host.c
> +++ b/hw/pci_host.c
> @@ -162,4 +162,30 @@ const MemoryRegionOps pci_host_data_be_ops = {
>  .endianness = DEVICE_BIG_ENDIAN,
>  };
>  
> +void pci_host_set_mmio(PCIHostState *s, MemoryRegion *value)
> +{
> + object_property_set_link(OBJECT(s), OBJECT(value), "mmio", NULL);
> +}
> +
> +static void pci_host_initfn(Object *obj)
> +{
> +PCIHostState *s = PCI_HOST(obj);
> +
> + object_property_add_link(obj, "mmio", TYPE_MEMORY_REGION,
> +  (Object **)&s->address_space, 
> NULL);
> +}
> +
> +static TypeInfo pci_host_type = {

I thought the convention was ..._type_info (in case we ever need to do a
mass conversion again). And please make it static const.

> +.name = TYPE_PCI_HOST,
> +.parent = TYPE_SYS_BUS_DEVICE,
> + .instance_size = sizeof(PCIHostState),
> + .instance_init = pci_host_initfn,
> +};
> +
> +static void register_devices(void)

pci_host_register_types please.

> +{
> + type_register_static(&pci_host_type);
> +}
> +
> +type_init(register_devices);

No semicolon please, it's not a statement.

There's still some tabs left in the revised version, please run
script/checkpatch.pl and repost a v3 inline so that we can comment on it.

Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg



Re: [Qemu-devel] [PATCH] spice_info: add mouse_mode

2012-03-26 Thread Alon Levy
On Mon, Mar 26, 2012 at 03:06:22PM +0200, Gerd Hoffmann wrote:
> > +#if SPICE_SERVER_VERSION >= 0x000a03 /* 0.10.3 */
> > +info->has_mouse_mode = true;
> > +info->mouse_mode = g_strdup(spice_server_is_server_mouse(spice_server) 
> > ?
> > +"server" : 
> > "client");
> 
> #else
>   info->mouse_mode = "unknown";
> #endif
> 

ok, it's probably better to have this consistent across hmp and qmp,
will resend this patch.

> cheers,
>   Gerd



Re: [Qemu-devel] [PATCH 02/10] qapi: fail hard on stack imbalance

2012-03-26 Thread Luiz Capitulino
On Thu, 22 Mar 2012 12:51:04 +0100
Paolo Bonzini  wrote:

> QmpOutputVisitor will segfault if an imbalanced end function is
> called.  So we can abort in QmpInputVisitor too.
> 
> Signed-off-by: Paolo Bonzini 
> ---
>  qapi/qmp-input-visitor.c |5 +
>  1 files changed, 1 insertions(+), 4 deletions(-)
> 
> diff --git a/qapi/qmp-input-visitor.c b/qapi/qmp-input-visitor.c
> index e6b6152..b4013cc 100644
> --- a/qapi/qmp-input-visitor.c
> +++ b/qapi/qmp-input-visitor.c
> @@ -77,11 +77,8 @@ static void qmp_input_push(QmpInputVisitor *qiv, const 
> QObject *obj, Error **err
>  
>  static void qmp_input_pop(QmpInputVisitor *qiv, Error **errp)
>  {
> +assert(qiv->nb_stack > 0);
>  qiv->nb_stack--;
> -if (qiv->nb_stack < 0) {
> -error_set(errp, QERR_BUFFER_OVERRUN);
> -return;
> -}
>  }

Just to confirm: this can't be triggered by malicious clients, right?

The original series submitted by Michael had this, but I asked him to
change because I thought clients could trigger it. But by reading the code
now it seems to me that the end_struct() function is only generated by types
we know about.

>  
>  static void qmp_input_start_struct(Visitor *v, void **obj, const char *kind,




Re: [Qemu-devel] [PATCH] block: add the support to drain throttled requests

2012-03-26 Thread Kevin Wolf
Am 26.03.2012 16:21, schrieb Stefan Hajnoczi:
> On Tue, Mar 20, 2012 at 11:44 AM, Stefan Hajnoczi
>  wrote:
>> On Tue, Mar 20, 2012 at 10:58:10AM +0100, Kevin Wolf wrote:
>>> Am 20.03.2012 10:47, schrieb Paolo Bonzini:
 Il 20/03/2012 10:40, Zhi Yong Wu ha scritto:
> HI, Kevin,
>
> We hope that I/O throttling can be shipped without known issue in QEMU
> 1.1, so if you are available, can you give this patch some love?

 I'm sorry to say this, but I think I/O throttling is impossible to save.
  As it is implemented now, it just cannot work in the presence of
 synchronous I/O, except at the cost of busy waiting with the global
 mutex taken.  See the message from Stefan yesterday.
>>>
>>> qemu_aio_flush() is busy waiting with the global mutex taken anyway, so
>>> it doesn't change that much.
>>
>> Yesterday I only posted an analysis of the bug but here are some
>> thoughts on how to move forward.  Throttling itself is not the problem.
>> We've known that synchronous operations in the vcpu thread are a problem
>> long before throttling.  This is just another reason to convert device
>> emulation to use asynchronous interfaces.
>>
>> Here is the list of device models that perform synchronous block I/O:
>> hw/fdc.c
>> hw/ide/atapi.c
>> hw/ide/core.c
>> hw/nand.c
>> hw/onenand.c
>> hw/pflash_cfi01.c
>> hw/pflash_cfi02.c
>> hw/sd.c
>>
>> Zhi Hui Li is working on hw/fdc.c and recently sent a patch.
>>
>> I think it's too close to QEMU 1.1 to convert all the remaining devices
>> and test them properly before the soft-freeze.  But it's probably
>> possible to convert IDE before the soft-freeze.
>>
>> In the meantime we could add this to bdrv_rw_co():
>>
>> if (bs->io_limits_enabled) {
>>fprintf(stderr, "Disabling I/O throttling on '%s' due "
>>"to synchronous I/O\n", bdrv_get_device_name(bs));
>>bdrv_io_limits_disable(bs);
>> }
>>
>> It's not pretty but tells the user there is an issue and avoids
>> deadlocking.
> 
> No one has commented on this suggestion.  I think leaving a known hang
> in QEMU 1.1 is undesirable.  Better to have this warning and disable
> throttling in the case we cannot support right now.
> 
> Kevin: Would you accept a patch like this?  Or do you have another
> solution in mind?

No, I don't have any clever short-term solution to offer. Put in
whatever hack you think works best for 1.1.

Kevin



  1   2   3   >