Re: [Qemu-devel] [PATCH 3/6] convert pci-host to QOM
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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()
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
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
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
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
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
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
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()
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
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
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
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
Hi Please send in any agenda items you are interested in covering. Cheers, Juan.
Re: [Qemu-devel] Thoughts around dtrace linking...
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
>>> 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
>>> 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
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
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
> 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
> +#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
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
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
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
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
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
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
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
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
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
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
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
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()
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
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
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
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
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
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
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"
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
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
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
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
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