Re: [Qemu-devel] [PATCH RFC] Implement GIC-500 from GICv3 family for arm64
On 2015年03月10日 17:47, Shlomo Pongratz wrote: On 10 آذار, 2015 ص 09:06, Pei XiaoYong wrote: 于 2015/3/9 22:41, shlomo.pongr...@toganetworks.com 写道: From: Shlomo Pongratz This patch is a first step toward 128 cores support for arm64. At first only 64 cores are supported for two reasons: First the largest integer type has the size of 64 bits and modifying essential data structures in order to support 128 cores will require the usage of bitops. Second currently the Linux (kernel) can be configured to support up to 64 cores thus there is no urgency with 128 cores support. Things left to do: Currently the booting Linux may got stuck. The probability of getting stuck increases with the number of cores. I'll appreciate core review. There is a need to support flexible clusters size. The GIC-500 can support up to 128 cores, up to 32 clusters and up to 8 cores is a cluster. So for example, if one wishes to have 16 cores, the options are: 2 clusters of 8 cores each, 4 clusters with 4 cores each Currently only the first option is supported. There is an issue of passing clock affinity to via the dtb. In the dtb interrupt section there are only 24 bit left to affinity since the variable is a 32 bit entity and 8 bits are reserved for flags. See Documentation/devicetree/bindings/arm/arch_timer.txt. Note that this issue is not seems to be critical as when checking /proc/irq/3/smp_affinity with 32 cores all 32 bits are one. The last issue is to add support for 128 cores. This requires the usage of bitops and currently can be tested up to 64 cores. Signed-off-by: Shlomo Pongratz --- hw/arm/Makefile.objs |2 +- hw/arm/virtv2.c| 774 + hw/intc/Makefile.objs |2 + hw/intc/arm_gic_common.c |2 + hw/intc/arm_gicv3.c| 1596 hw/intc/arm_gicv3_common.c | 188 + hw/intc/gicv3_internal.h | 153 include/hw/intc/arm_gicv3.h| 44 + include/hw/intc/arm_gicv3_common.h | 136 +++ target-arm/cpu.c |1 + target-arm/cpu.h |6 + target-arm/cpu64.c | 92 +++ target-arm/helper.c| 12 +- target-arm/psci.c | 18 +- target-arm/translate-a64.c | 14 + 15 files changed, 3034 insertions(+), 6 deletions(-) create mode 100644 hw/arm/virtv2.c create mode 100644 hw/intc/arm_gicv3.c create mode 100644 hw/intc/arm_gicv3_common.c create mode 100644 hw/intc/gicv3_internal.h create mode 100644 include/hw/intc/arm_gicv3.h create mode 100644 include/hw/intc/arm_gicv3_common.h as far as we know , there are many components in gic-v3 implementation , like distributor , redistributor , its , lpi . Offsets of them is not defined in the gic-v3 specification , i think wo should implement these components independently , not like v2&v1 implementation in qemu. Hi Peixiaoyong, My immediate goal is running more than 8 cores, so currently "its" and "ipi" are not supported. I've used the offsets' rules from GIC-500 which is an implementation of GICv3. When and if "its" and "ipi" will be implemented then I think a new virt machine will need to be created as this is like a new HW BSP with different architecture. Best regards, Hi : I think we should focus on the scalable of the code . On the other hand , we need remove the redundant code .
[Qemu-devel] [Bug 1569988] [NEW] [2.6] user network broken reaching foreign servers on Win64
Public bug reported: Both on master and, starting with 2016-03-22 builds from http://qemu.weilnetz.de/w64/, user mode network can't reach foreign servers. For example, wget http://mifritscher resolves the DNS, but then the message "network target couldn't be reached" occures. 2016-03-03 works fine. I suspect the IPv6 changes. My connection is IPv4 only. I tested via knoppix 7.6. The command line is qemu-system-x86_64.exe -m 512 -k de --cdrom c:\Users\michaelfritscher\Downloads\linux\KNOPPIX_V7.4.2DVD-2014-09-28-DE.iso -netdev user,id=mynet0,restrict=n -device e1000,netdev=mynet0 ** Affects: qemu Importance: Undecided Status: New ** Tags: dev network windows -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1569988 Title: [2.6] user network broken reaching foreign servers on Win64 Status in QEMU: New Bug description: Both on master and, starting with 2016-03-22 builds from http://qemu.weilnetz.de/w64/, user mode network can't reach foreign servers. For example, wget http://mifritscher resolves the DNS, but then the message "network target couldn't be reached" occures. 2016-03-03 works fine. I suspect the IPv6 changes. My connection is IPv4 only. I tested via knoppix 7.6. The command line is qemu-system-x86_64.exe -m 512 -k de --cdrom c:\Users\michaelfritscher\Downloads\linux\KNOPPIX_V7.4.2DVD-2014-09-28-DE.iso -netdev user,id=mynet0,restrict=n -device e1000,netdev=mynet0 To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1569988/+subscriptions
[Qemu-devel] [Bug 1569988] Re: [2.6] user network broken reaching foreign servers on Win64
Under Linux, it is working fine. -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1569988 Title: [2.6] user network broken reaching foreign servers on Win64 Status in QEMU: New Bug description: Both on master and, starting with 2016-03-22 builds from http://qemu.weilnetz.de/w64/, user mode network can't reach foreign servers. For example, wget http://mifritscher resolves the DNS, but then the message "network target couldn't be reached" occures. 2016-03-03 works fine. I suspect the IPv6 changes. My connection is IPv4 only. I tested via knoppix 7.6. The command line is qemu-system-x86_64.exe -m 512 -k de --cdrom c:\Users\michaelfritscher\Downloads\linux\KNOPPIX_V7.4.2DVD-2014-09-28-DE.iso -netdev user,id=mynet0,restrict=n -device e1000,netdev=mynet0 To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1569988/+subscriptions
[Qemu-devel] [Bug 1569988] Re: [2.6] user network broken reaching foreign servers on Win64
Another thing: if I disable ipv6, ipv4 runs still in sort of a timeout before writing Network is unreachable (ENETUNREACH), while ipv6 says this without delay (which is ok/good) ** Description changed: Both on master and, starting with 2016-03-22 builds from http://qemu.weilnetz.de/w64/, user mode network can't reach foreign servers. For example, wget http://mifritscher resolves the DNS, but then - the message "network target couldn't be reached" occures. 2016-03-03 - works fine. I suspect the IPv6 changes. My connection is IPv4 only. + the message "Network is unreachable" occures. 2016-03-03 works fine. I + suspect the IPv6 changes. My connection is IPv4 only. I tested via knoppix 7.6. The command line is qemu-system-x86_64.exe -m 512 -k de --cdrom c:\Users\michaelfritscher\Downloads\linux\KNOPPIX_V7.4.2DVD-2014-09-28-DE.iso -netdev user,id=mynet0,restrict=n -device e1000,netdev=mynet0 -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1569988 Title: [2.6] user network broken reaching foreign servers on Win64 Status in QEMU: New Bug description: Both on master and, starting with 2016-03-22 builds from http://qemu.weilnetz.de/w64/, user mode network can't reach foreign servers. For example, wget http://mifritscher resolves the DNS, but then the message "Network is unreachable" occures. 2016-03-03 works fine. I suspect the IPv6 changes. My connection is IPv4 only. I tested via knoppix 7.6. The command line is qemu-system-x86_64.exe -m 512 -k de --cdrom c:\Users\michaelfritscher\Downloads\linux\KNOPPIX_V7.4.2DVD-2014-09-28-DE.iso -netdev user,id=mynet0,restrict=n -device e1000,netdev=mynet0 To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1569988/+subscriptions
Re: [Qemu-devel] [PATCH 0/5] QOM'ify hw/display devices
Am 2016-05-04 16:26, schrieb Peter Maydell: On 24 March 2016 at 10:29, xiaoqiang zhao wrote: This patch set trys to QOM'ify hw/display files, see commit messages for more details xiaoqiang zhao (5): hw/display: QOM'ify exynos4210_fimd.c hw/display: QOM'ify jazz_led.c hw/display: QOM'ify milkymist-tmu2.c hw/display: QOM'ify milkymist-vgafb.c hw/display: QOM'ify pl110.c Hi; I was going to review this series (apologies for taking so long!) but looking at my email archive and the patchwork server the patches in it seem a bit confused. I see seven patches, not five, with rather odd patch number indications: 1/5 2/5 3/6 4/5 5/6 5/5 6/6 (and 5/5 and 6/6 seem to be the same). Could you resend the series with the correct patches in it, please? Oh my. I wanted to test the milkymist stuff last week, but didn't find time to complete it. I'll try it this week(end). If 5/5 and 6/6 is the milkymist-vgafb.c, then I noticed the duplication, too. -michael
Re: [Qemu-devel] [PATCH v2 4/5] hw/display: QOM'ify milkymist-vgafb.c
Am 2016-05-06 12:59, schrieb xiaoqiang zhao: * Drop the old SysBus init function and use instance_init * Move graphic_console_init into realize stage Signed-off-by: xiaoqiang zhao Acked-by: Michael Walle Tested-by: Michael Walle -michael
Re: [Qemu-devel] [PATCH v2 3/5] hw/display: QOM'ify milkymist-tmu2.c
Am 2016-05-06 12:59, schrieb xiaoqiang zhao: * Drop the old SysBus init function and use instance_init * Move tmu2_glx_init into realize stage Signed-off-by: xiaoqiang zhao Acked-by: Michael Walle Tested-by: Michael Walle -michael
Re: [Qemu-devel] [PATCH RESEND 3/5] hw/display: QOM'ify milkymist-tmu2.c
Am 2016-05-06 09:32, schrieb Markus Armbruster:
Peter Maydell writes:
On 5 May 2016 at 04:04, xiaoqiang zhao wrote:
* Drop the old SysBus init function and use instance_init
* Move tmu2_glx_init into realize stage
Signed-off-by: xiaoqiang zhao
Reviewed-by: Peter Maydell
+static void milkymist_tmu2_realize(DeviceState *dev, Error **errp)
+{
+MilkymistTMU2State *s = MILKYMIST_TMU2(dev);
+
+if (tmu2_glx_init(s)) {
+error_setg(errp, "tmu2_glx_init failed.");
+}
}
The milkymist maintainer might have a suggestion for a
more informative error message here.
Also, error_setg() doesn't want the period:
* The resulting message should be a single phrase, with no newline or
* trailing punctuation.
Sorry, I can't think of a better description. The TMU2 code isn't my
code. I had a look, but basically, it's because some glX functions might
fail. But glx being not available isn't one of them because that is
already checked in milkymist_tmu2_create().
-michael
Re: [Qemu-devel] [PATCH 5/9] hw/intc: QOM'ify lm32_pic.c
Am 2016-03-30 12:09, schrieb xiaoqiang zhao: Drop the old SysBus init function and use instance_init Signed-off-by: xiaoqiang zhao Acked-by: Michael Walle Tested-by: Michael Walle -michael
Re: [Qemu-devel] [RESEND PATCH v2 4/4] hw/audio: QOM'ify milkymist-ac97.c
Am 2016-03-17 10:06, schrieb xiaoqiang zhao: * Drop the old SysBus init function and use instance_init * Move AUD_open_in / AUD_open_out function into realize stage Acked-by: Michael Walle Tested-by: Michael Walle -michael
Re: [Qemu-devel] [PATCH v2 4/6] hw/char: QOM'ify lm32_uart.c
Am 2016-03-29 09:47, schrieb xiaoqiang zhao: Drop the old SysBus init function and use instance_init Signed-off-by: xiaoqiang zhao Acked-by: Michael Walle Tested-by: Michael Walle -michael
Re: [Qemu-devel] [PATCH v2 3/6] hw/char: QOM'ify lm32_juart.c
Am 2016-03-29 09:47, schrieb xiaoqiang zhao: Drop the old SysBus init function and use instance_init Signed-off-by: xiaoqiang zhao Acked-by: Michael Walle Tested-by: Michael Walle -michael
Re: [Qemu-devel] [PATCH 0/9] QOM'ify hw/intc files
Hi Peter, Am 2016-05-04 16:56, schrieb Peter Maydell: SPARC, lm32, CRIS maintainers: do you want to take your patches or shall I just take 1-8 through the target-arm.next tree? There are other ones (milkymist-ac97, lm32_uart, lm32_juart, milkymist-vgafb, milkymist-tmu2, lm32_timer, milkymist-sysctl). So unless you'll take these too, I'll pick them. -michael
Re: [Qemu-devel] [PATCH v4 6/9] hw/timer: QOM'ify milkymist_sysctl
[I had some problems with my mailserver and the v5 version didn't make it through. I know there is a v5 version of this patch series and I've tested with the v5 version] Am 2016-02-22 04:15, schrieb xiaoqiang zhao: * split the old SysBus init function into an instance_init and a Device realize function * use DeviceClass::realize instead of SysBusDeviceClass::init Reviewed-by: Peter Maydell Signed-off-by: xiaoqiang zhao Acked-by: Michael Walle Tested-by: Michael Walle -michael
Re: [Qemu-devel] [PATCH v4 3/9] hw/timer: QOM'ify lm32_timer
[I had some problems with my mailserver and the v5 version didn't make it through. I know there is a v5 version of this patch series and I've tested with the v5 version] Am 2016-02-22 04:15, schrieb xiaoqiang zhao: * split the old SysBus init function into an instance_init and a Device realize function * use DeviceClass::realize instead of SysBusDeviceClass::init Reviewed-by: Peter Maydell Signed-off-by: xiaoqiang zhao Signed-off-by: Michael Walle -michael
Re: [Qemu-devel] [PATCH v4 6/9] hw/timer: QOM'ify milkymist_sysctl
[I had some problems with my mailserver and the v5 version didn't make it through. I know there is a v5 version of this patch series and I've tested with the v5 version] Am 2016-02-22 04:15, schrieb xiaoqiang zhao: * split the old SysBus init function into an instance_init and a Device realize function * use DeviceClass::realize instead of SysBusDeviceClass::init Reviewed-by: Peter Maydell Signed-off-by: xiaoqiang zhao Signed-off-by: Michael Walle -michael
Re: [Qemu-devel] [PATCH v4 3/9] hw/timer: QOM'ify lm32_timer
Am 2016-03-17 16:00, schrieb Peter Maydell: On 17 March 2016 at 14:59, wrote: [I had some problems with my mailserver and the v5 version didn't make it through. I know there is a v5 version of this patch series and I've tested with the v5 version] Am 2016-02-22 04:15, schrieb xiaoqiang zhao: * split the old SysBus init function into an instance_init and a Device realize function * use DeviceClass::realize instead of SysBusDeviceClass::init Reviewed-by: Peter Maydell Signed-off-by: xiaoqiang zhao Signed-off-by: Michael Walle Signed-off-by: doesn't make much sense in this situation -- did you want Acked-by, Reviewed-by, Tested-by, or some combination of those ? ahh sorry, need more sleep :( Acked-by: Michael Walle Tested-by: Michael Walle -michael
Re: [Qemu-devel] [PATCH v4 0/4] QOM'ify hw/char devices
Am 2016-05-19 13:32, schrieb Paolo Bonzini: On 19/05/2016 05:43, xiaoqiang zhao wrote: This patch set trys to QOM'ify hw/char files, see commit messages for more details Thanks Paolo for your suggestions. Note: * CRIS axis_dev88 broad related test is passed and looks ok. * lm32 test is needed. Michael, can you test patches 3 and 4? Yes, already on my todo list. -michael
Re: [Qemu-devel] [PATCH v4 0/4] QOM'ify hw/char devices
Am 2016-05-19 13:32, schrieb Paolo Bonzini: Michael, can you test patches 3 and 4? Doesn't work for me: $ qemu-system-lm32 -kernel serial.bin -serial vc -serial vc Unexpected error in parse_chr() at /home/mwalle/repos/qemu/hw/core/qdev-properties-system.c:149: qemu-system-lm32: Property 'lm32-uart.chardev' can't take value 'serial0', it's in use serial0 seems already be claimed. Please note that even $ qemu-system-lm32 -kernel serial.bin and $ qemu-system-lm32 does not work. "-serial .. -serial --" should still work, shouldn't it? I've uploaded a small test binary to http://milkymist.walle.cc/tests/ which should prints 'A' and 'U' characters on the UARTs. -michael
[Qemu-devel] several guests with static ip-address
hi qemu list, is it possible, that several (for example two) guests communicate to one qemu host, which has only one network interface and every os has it's own static ip-address? For me, only one guest and a qemu host with static ip-address configuration communicate with each other (and with the router, all are in the same subnet over a tun/bridge interface), but when i would bring up a second guest os that should communicate with the qemu host (all with static ip-addresses) it doesn't work. With two network interfaces (for two guests) on the qemu host it should work i think, but with only one? thanks for any help! Michael ___ Qemu-devel mailing list Qemu-devel@nongnu.org http://lists.nongnu.org/mailman/listinfo/qemu-devel
Re: [Qemu-devel] Config error message wrong.
Rob Landley schrieb: >>ERROR: QEMU requires SDL or Cocoa for graphical output >>To build QEMU with graphical output configure with --disable-gfx-check >>Note that this will disable all output from the virtual graphics card. >> >> > >Possibly that "with" should be a "without"? > > > on debian do: $> apt-get install libsdl1.2-dev regards, michael >Rob > > >_ > ___ Qemu-devel mailing list Qemu-devel@nongnu.org http://lists.nongnu.org/mailman/listinfo/qemu-devel
[Qemu-devel] qemu-ppc install problems
Hi, the following error occurs (at boot time), when i want to install osx 10.3: System Failure: cpu=0 code=0007 (Unaligned stack) Latest crash info for cpu 0: Exception state (sv=0x00877000) PC=0x0008CE84; MSR=0x1000; DAR=0x0030A398; DSISR=0x4000; LR=0x00088c64; R1=0x05413A90; XCP=0x0098 (System Failure) Backtrace: 0x0005ED6C 0x00084EC8 0x0005A400 0x0005FF3C 0x000604D4 0x0005B8B0 0x0008676C 0x0002CE68 0x0002CDA8 Proceeding back via exception chain: Exception state (sv=0x00B77000) PC=0x; MSR=0xD030; DAR=0x; DSISR=0x; LR=0x; R1=0x; XCP=0x (Unknown) Kernel version: Darwin kernel Version 7.0.0: ...the system hangs... I have used qemu-0.72 (src). Any help? Thanks! Regards, Michael ___ Qemu-devel mailing list Qemu-devel@nongnu.org http://lists.nongnu.org/mailman/listinfo/qemu-devel
[Qemu-devel] new network options in qemu cvs version
Hi qemu-list, in qemu-0.7.2 i have used the following network options for tun/bridge device setup: qemu -macaddr aa:bb:cc:dd:ee:ff -n /etc/qemu/qemu.boot -hda /opt/xyz.img -boot c -nics 2 -localtime with the following (tun) network startscript (qemu.boot): ifconfig $1 0.0.0.0 promisc up brctl addif br0 $1 How can i do this with the new qemu (cvs) network options? Thanks for any help! Best Regards, Michael ___ Qemu-devel mailing list Qemu-devel@nongnu.org http://lists.nongnu.org/mailman/listinfo/qemu-devel
[Qemu-devel] [Bug 1810343] [NEW] qemu-nbd -l and -s options don't work together
Public bug reported: When using qemu-nbd with -l to load a snapshot along with -s to create new active layer the tool fails to find the snapshot specified on the command line: For example the following does not work: sudo qemu-nbd -s --load-snapshot=files --connect /dev/nbd0 rootfs.qcow2 Failed to load snapshot: Can't find snapshot However, the following option works sudo qemu-nbd -s --connect /dev/nbd0 rootfs.qcow2 and so does sudo qemu-nbd --load-snapshot=files --connect /dev/nbd0 rootfs.qcow2 ** Affects: qemu Importance: Undecided Status: New -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1810343 Title: qemu-nbd -l and -s options don't work together Status in QEMU: New Bug description: When using qemu-nbd with -l to load a snapshot along with -s to create new active layer the tool fails to find the snapshot specified on the command line: For example the following does not work: sudo qemu-nbd -s --load-snapshot=files --connect /dev/nbd0 rootfs.qcow2 Failed to load snapshot: Can't find snapshot However, the following option works sudo qemu-nbd -s --connect /dev/nbd0 rootfs.qcow2 and so does sudo qemu-nbd --load-snapshot=files --connect /dev/nbd0 rootfs.qcow2 To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1810343/+subscriptions
Re: [RFC PATCH v2] hw/display/tcx: Allow 64-bit accesses to framebuffer stippler and blitter
Hello, since I wrote the NetBSD code in question, here are my 2 cent: On Sat, 29 Aug 2020 08:41:43 -0700 Richard Henderson wrote: > On 8/22/20 7:21 AM, Philippe Mathieu-Daudé wrote: > > The S24/TCX datasheet is listed as "Unable to locate" on [1]. I don't have it either, but someone did a lot of reverse engineering and gave me his notes. The hardware isn't that complicated, but quite weird. > > However the NetBSD revision 1.32 of the driver introduced > > 64-bit accesses to the stippler and blitter [2]. It is safe > > to assume these memory regions are 64-bit accessible. > > QEMU implementation is 32-bit, so fill the 'impl' fields. IIRC the real hardware *requires* 64bit accesses for stipple and blitter operations to work. For stipples you write a 64bit word into STIP space, the address defines where in the framebuffer you want to draw, the data contain a 32bit bitmask, foreground colour and a ROP. BLIT space works similarly, the 64bit word contains an offset were to read pixels from, and how many you want to copy. have fun Michael
Re: [RFC PATCH v2] hw/display/tcx: Allow 64-bit accesses to framebuffer stippler and blitter
Hello, On Sat, 29 Aug 2020 18:45:06 +0200 Philippe Mathieu-Daudé wrote: > > > > However the NetBSD revision 1.32 of the driver introduced > > > > 64-bit accesses to the stippler and blitter [2]. It is safe > > > > to assume these memory regions are 64-bit accessible. > > > > QEMU implementation is 32-bit, so fill the 'impl' fields. > > > > IIRC the real hardware *requires* 64bit accesses for stipple and > > blitter operations to work. For stipples you write a 64bit word into > > STIP space, the address defines where in the framebuffer you want to > > draw, the data contain a 32bit bitmask, foreground colour and a ROP. > > BLIT space works similarly, the 64bit word contains an offset were to > > read pixels from, and how many you want to copy. > > > > Thanks Michael for this information! > If you don't mind I'll amend it to the commit description so there is a > reference for posterity. One more thing since there seems to be some confusion - 64bit accesses on the framebuffer are fine as well. TCX/S24 is *not* an SBus device, even though its node says it is. S24 is a card that plugs into a special slot on the SS5 mainboard, which is shared with an SBus slot and looks a lot like a horizontal UPA slot. Both S24 and TCX are accessed through the Micro/TurboSPARC's AFX bus which is 64bit wide and intended for graphics. Early FFB docs even mentioned connecting to both AFX and UPA, no idea if that was ever realized in hardware though. have fun Michael
[Qemu-devel] [Bug 1033727] Re: USB passthrough doesn't work anymore with qemu-kvm 1.1.1
I think similar bug has been filed against qemu-kvm debian package (http://bugs.debian.org/683983). Will try to reproduce/bisect as time permits. Note the debian bugreport also mentions segfault on usb_del in monitor. ** Bug watch added: Debian Bug tracker #683983 http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=683983 ** Also affects: qemu-kvm (Debian) via http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=683983 Importance: Unknown Status: Unknown -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1033727 Title: USB passthrough doesn't work anymore with qemu-kvm 1.1.1 Status in QEMU: New Status in “qemu-kvm” package in Debian: Unknown Bug description: Hi, I have a "Bus 006 Device 002: ID 0d46:3003 Kobil Systems GmbH mIDentity Light / KAAN SIM III" (kind of smart card) in an USB port which I make available to a Windows XP guest. This worked fine with every older qemu-kvm version I've used so far. But since 1.1.0 it doesn't work anymore. The device shows up in the guest, but the software can't access it anymore (and the guest is pretty unresponsive). On the host I get every 2 seconds this message: [ 7719.239528] usb 6-1: reset full-speed USB device number 2 using uhci_hcd Command line options are: /usr/bin/kvm ... -device usb-host,vendorid=0x0d46,productid=0x3003,bus=usb.0,port=3 ... When I switch back to qemu-kvm 1.0.1 everything works fine again. Any idea what the problem could be? Thanks Klaus To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1033727/+subscriptions
[Qemu-devel] [Bug 1033727] Re: USB passthrough doesn't work anymore with qemu-kvm 1.1.1
Ok. I tried to bisect this, but it appears to be not so easy. The problem is that between 1.0 and 1.1, there's a lot of usb breakage, and bisection leads to segfaults or assertion failures. (qemu) usb_add host:003.002 usb_create: no bus specified, using "usb.0" for "usb-host" (qemu) Segmentation fault This is fixed in 8db36e9dddb1b6fab3554a8c00d92268b33a487b. (qemu) usb_add host:003.002 (qemu) qemu-system-x86_64: /build/kvm/git/hw/usb.c:358: usb_packet_complete: Assertion `p->state == USB_PACKET_QUEUED' failed. I skipped this commit, which lead to: (qemu) usb_add host:003.002 (qemu) qemu-system-x86_64: /build/kvm/git/hw/usb.c:410: usb_packet_complete: Assertion `((&ep->queue)->tqh_first) == p' failed. I'm continuing, but I've no much hope this will lead to anything useful at this rate. -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1033727 Title: USB passthrough doesn't work anymore with qemu-kvm 1.1.1 Status in QEMU: New Status in “qemu-kvm” package in Debian: Unknown Bug description: Hi, I have a "Bus 006 Device 002: ID 0d46:3003 Kobil Systems GmbH mIDentity Light / KAAN SIM III" (kind of smart card) in an USB port which I make available to a Windows XP guest. This worked fine with every older qemu-kvm version I've used so far. But since 1.1.0 it doesn't work anymore. The device shows up in the guest, but the software can't access it anymore (and the guest is pretty unresponsive). On the host I get every 2 seconds this message: [ 7719.239528] usb 6-1: reset full-speed USB device number 2 using uhci_hcd Command line options are: /usr/bin/kvm ... -device usb-host,vendorid=0x0d46,productid=0x3003,bus=usb.0,port=3 ... When I switch back to qemu-kvm 1.0.1 everything works fine again. Any idea what the problem could be? Thanks Klaus To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1033727/+subscriptions
[Qemu-devel] commit da57febfed "qdev: give all devices a canonical path" broke usb_del
While dealing with USB issues today I noticed that usb_del monitor command is broken, attempting to delete any usb device immediately results in assertion failure: (qemu) usb_del 0.1 ERROR:qom/object.c:408:object_delete: assertion failed: (obj->ref == 0) Aborted I bisected this issue to commit: commit da57febfed7bad11be79f047b59719c38abd0712 Author: Paolo Bonzini Date: Tue Mar 27 18:38:47 2012 +0200 qdev: give all devices a canonical path A strong limitation of QOM right now is that unconverted ports (e.g. all...) do not give a canonical path to devices that are part of the board. This in turn makes it impossible to replace PROP_PTR with a QOM link for example. Reviewed-by: Anthony Liguori Signed-off-by: Paolo Bonzini Signed-off-by: Anthony Liguori The problem is still present in current qemu/master git. I'm not sure what to do with this. See also http://lists.gnu.org/archive/html/qemu-devel/2012-07/msg00199.html -- Peter Maydell reoprted this very issue a while back but no one replied. Thanks, /mjt
Re: [Qemu-devel] commit da57febfed "qdev: give all devices a canonical path" broke usb_del
On 08.08.2012 16:18, Michael Tokarev wrote:
> While dealing with USB issues today I noticed that
> usb_del monitor command is broken, attempting to
> delete any usb device immediately results in assertion
> failure:
>
> (qemu) usb_del 0.1
> ERROR:qom/object.c:408:object_delete: assertion failed: (obj->ref == 0)
> Aborted
>
>
> I bisected this issue to commit:
>
> commit da57febfed7bad11be79f047b59719c38abd0712
> Author: Paolo Bonzini
> Date: Tue Mar 27 18:38:47 2012 +0200
>
> qdev: give all devices a canonical path
>
> A strong limitation of QOM right now is that unconverted ports
> (e.g. all...) do not give a canonical path to devices that are
> part of the board. This in turn makes it impossible to replace
> PROP_PTR with a QOM link for example.
>
> Reviewed-by: Anthony Liguori
> Signed-off-by: Paolo Bonzini
> Signed-off-by: Anthony Liguori
>
>
> The problem is still present in current qemu/master git.
>
> I'm not sure what to do with this.
Well. This commit adds this code:
@@ -152,6 +152,16 @@ int qdev_init(DeviceState *dev)
+
+if (!OBJECT(dev)->parent) {
+static int unattached_count = 0;
+gchar *name = g_strdup_printf("device[%d]", unattached_count++);
+
+object_property_add_child(container_get("/unattached"), name,
+ OBJECT(dev), NULL);
+g_free(name);
+}
ie, there's now extra call to object_property_add_child(),
which does object_ref(). So no doubt there's no a new
assertion failure: (obj->ref == 0).
Once again, patch description does not reflect what is
actually done by the patch. Can we please stop this
practice of accepting patches with desrciption not
reflecting reality?
Back to the point: should there be a call to object_unref()
somewhere?
Thanks,
/mjt
Re: [Qemu-devel] commit da57febfed "qdev: give all devices a canonical path" broke usb_del
On 08.08.2012 16:39, Paolo Bonzini wrote:
> Il 08/08/2012 14:22, Michael Tokarev ha scritto:
>> @@ -152,6 +152,16 @@ int qdev_init(DeviceState *dev)
>> +
>> +if (!OBJECT(dev)->parent) {
>> +static int unattached_count = 0;
>> +gchar *name = g_strdup_printf("device[%d]", unattached_count++);
>> +
>> +object_property_add_child(container_get("/unattached"), name,
>> + OBJECT(dev), NULL);
>> +g_free(name);
>> +}
>>
>> ie, there's now extra call to object_property_add_child(),
>> which does object_ref(). So no doubt there's no a new
>> assertion failure: (obj->ref == 0).
>>
>> Once again, patch description does not reflect what is
>> actually done by the patch.
>
> Huh? The add_child ensures that there is a canonical path.
>
>> Can we please stop this
>> practice of accepting patches with desrciption not
>> reflecting reality?
I must clarify this.
I'm not trying to blame Paolo for the wrong patch which
"broke" things (it exposed an old bug in other codepath).
I'm just saying that the patch description appears to be
"too innocent" -- yes, now each device has a path, or,
in other words, a NAME, but this patch ALSO changes
refcounting, and THIS is what I'm referring to above --
it isn't only gives a name, but also links "unowned"
objects to a bus. To me it is a wrong (too innocent)
description. I browsed comits trying to find which
change might have caused it, and provided ther was
a mention of "references" or "connecting" in this patch
description somewhere, I'd found this change much faster,
without a painful (*) bisection.
Maybe it is just me who does not know this code area
well enough, so things aren't as obvious for me as for
others, I dunno, but to me the description -- the only
thing I'm trying to "blame" Paolo for here -- might be
quite a bit better.
(*) painful because this bisection come in the process
of another bisection dealing with another usb issue,
which come to yet another bug in usb handling somewhere
(giving another assertion).
>> Back to the point: should there be a call to object_unref()
>> somewhere?
>
> There should be a call to object_unparent() somewhere actually.
> We've been peppering the code with them for a few months now while
> waiting for "the" solution, but I fail to see what the solution
> could be other than the patch below (something similar has probably
> been proposed already). BTW there are probably a lot of other similar
> bugs somewhere.
This sounds ecouraging -- "alot of other similar bugs".. :(
Something similar should be applied to 1.1-stable. FWIW, some
changes are not needed there, like this:
> --- a/hw/qdev.c
> +++ b/hw/qdev.c
> @@ -159,7 +159,6 @@ int qdev_init(DeviceState *dev)
>
> rc = dc->init(dev);
> if (rc < 0) {
> -object_unparent(OBJECT(dev));
> qdev_free(dev);
> return rc;
> }
and this:
> --- a/hw/shpc.c
> +++ b/hw/shpc.c
> @@ -253,7 +253,6 @@ static void shpc_free_devices_in_slot(SHPCDevice *shpc,
> int slot)
> ++devfn) {
> PCIDevice *affected_dev = shpc->sec_bus->devices[devfn];
> if (affected_dev) {
> -object_unparent(OBJECT(affected_dev));
> qdev_free(&affected_dev->qdev);
> }
> }
the lines being removed does not exist in 1.1-stable.
I can guess this is due to previous attempts to fix
similar issues in other places.
Thank you!
/mjt
Re: [Qemu-devel] commit da57febfed "qdev: give all devices a canonical path" broke usb_del
On 08.08.2012 17:09, Michael Tokarev wrote:
[]
> Something similar should be applied to 1.1-stable. FWIW, some
> changes are not needed there.
Cherry-pick to stable-1.1 removes the two unneeded hunks.
This is what I plan to include into debian package. It
fixes the original usb_del issue, and I didn't find new
regressions so far - tried a few device_del and similar.
Should it go to qemu/stable-1.1 as well?
Thank you!
/mjt
Author: Paolo Bonzini
Date: Wed Aug 8 14:39:11 2012 +0200
Bug-Debian: http://bugs.debian.org/684282
Comment: cherry-picked from qemu/master to stable-1.1 (mjt)
qom: object_delete should unparent the object first
object_deinit is only called when the reference count goes to zero,
and yet tries to do an object_unparent. Now, object_unparent
either does nothing or it will decrease the reference count.
Because we know the reference count is zero, the object_unparent
call in object_deinit is useless.
Instead, we need to disconnect the object from its parent just
before we remove the last reference apart from the parent's. This
happens in object_delete. Once we do this, all calls to
object_unparent peppered through QEMU can go away.
Signed-off-by: Paolo Bonzini
Signed-off-by: Michael Tokarev
diff --git a/hw/acpi_piix4.c b/hw/acpi_piix4.c
index 0345490..585da4e 100644
--- a/hw/acpi_piix4.c
+++ b/hw/acpi_piix4.c
@@ -299,7 +299,6 @@ static void acpi_piix_eject_slot(PIIX4PMState *s, unsigned slots)
if (pc->no_hotplug) {
slot_free = false;
} else {
-object_unparent(OBJECT(dev));
qdev_free(qdev);
}
}
diff --git a/hw/qdev.c b/hw/qdev.c
index 6a8f6bd..9bb1c6b 100644
--- a/hw/qdev.c
+++ b/hw/qdev.c
@@ -240,7 +240,6 @@ void qbus_reset_all_fn(void *opaque)
int qdev_simple_unplug_cb(DeviceState *dev)
{
/* just zap it */
-object_unparent(OBJECT(dev));
qdev_free(dev);
return 0;
}
diff --git a/hw/xen_platform.c b/hw/xen_platform.c
index 0214f37..84221df 100644
--- a/hw/xen_platform.c
+++ b/hw/xen_platform.c
@@ -87,9 +87,6 @@ static void unplug_nic(PCIBus *b, PCIDevice *d)
{
if (pci_get_word(d->config + PCI_CLASS_DEVICE) ==
PCI_CLASS_NETWORK_ETHERNET) {
-/* Until qdev_free includes a call to object_unparent, we call it here
- */
-object_unparent(&d->qdev.parent_obj);
qdev_free(&d->qdev);
}
}
diff --git a/qom/object.c b/qom/object.c
index 6f839ad..58dd886 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -347,8 +347,6 @@ static void object_deinit(Object *obj, TypeImpl *type)
if (type_has_parent(type)) {
object_deinit(obj, type_get_parent(type));
}
-
-object_unparent(obj);
}
void object_finalize(void *data)
@@ -385,8 +383,9 @@ Object *object_new(const char *typename)
void object_delete(Object *obj)
{
+object_unparent(obj);
+g_assert(obj->ref == 1);
object_unref(obj);
-g_assert(obj->ref == 0);
g_free(obj);
}
Re: [Qemu-devel] commit da57febfed "qdev: give all devices a canonical path" broke usb_del
On 08.08.2012 18:42, Michael Tokarev wrote: > Should it go to qemu/stable-1.1 as well? qemu/stable-1.1 also includes f63e60327b8e239ae97fa71060940ca20a8bf38e. FWIW.
[Qemu-devel] [Bug 1013888] Re: windows xp sp3 setup blank screen on boot
Which debian package do you mean? The fix is included is current debian qemu-kvm 1.1.0+dfsg-3 release. qemu package in debian does not have this fix however. -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1013888 Title: windows xp sp3 setup blank screen on boot Status in QEMU: New Bug description: When attempting to run Windows XP SP3 setup in qemu on a Lubuntu host with the following kernel: Linux michael-XPS-M1530 3.2.0-23-generic #36-Ubuntu SMP Tue Apr 10 20:39:51 UTC 2012 x86_64 x86_64 x86_64 GNU/Linux Qemu does not get past a blank screen after "Setup is inspecting your computer's hardware configuration" To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1013888/+subscriptions
Re: [Qemu-devel] [PATCH 5/7] scsi-disk: more assertions and resets for aiocb
On 09.08.2012 17:38, Paolo Bonzini wrote: > Leaving the aiocb to a non-NULL value leads to an assertion failure when > rerror/werror are set to stop or enospc, and the operation is retried. > scsi-disk checks that the aiocb member is NULL before filling it. > > This patch correctly resets the aiocb to NULL values everywhere, > and adds the dual assertion that the aiocb was non-NULL before > calling bdrv_acct_done. Stable matherial? Thanks, /mjt
[Qemu-devel] TSC in qem[-kvm] 1.1+ and in-kernel irqchip
As a follow-up to the patch "tsc: use kvmclock for
calibration".
There's another problem reported by several users.
The sympthom is that grub does not show boot menu,
it boots default entry right away without any pause.
After quite some debugging it turned out to be
TSC issue. Grub uses tsc for its timeout handling.
When setting timeout to some very large value
(1), I can see the counter is ticking backwards
at a very high speed, ticking from 1 to 0 in
about 5 seconds.
Running kvm -cpu host,-tsc forces grub to use
rtc clocksource, and the problem goes away.
The most interesting thing is that this is a
problem new for qemu-kvm 1.1 (and is still
present in current git), 1.0 version had no
such issue. And it only happens when in-kernel
irqchip is enabled -- running with -no-kvm-irqchip
also fixes the grub problem, so that tsc starts
counting "correctly" for grub again.
Gerd mentioned mis-calibration of bios timer
when host is heavily loaded. I tested grub on
my workstation today which was completely idle,
no other processes running.
It smells like a bug in kvm somewhere. And it
happens when I explicitly pin kvm to a single
core, so tsc should tick correctly even if its
syncronization is broken between cores.
Current qemu also has this issue (since 1.1),
since it also has in-kernel irqchip support now.
FWIW, here's the TSC calibration routine from
grub:
/* Calibrate the TSC based on the RTC. */
static void
calibrate_tsc (void)
{
/* First calibrate the TSC rate (relative, not absolute time). */
grub_uint64_t start_tsc;
grub_uint64_t end_tsc;
start_tsc = grub_get_tsc ();
grub_pit_wait (0x);
end_tsc = grub_get_tsc ();
tsc_ticks_per_ms = grub_divmod64 (end_tsc - start_tsc, 55, 0);
}
Thanks,
/mjt
Re: [Qemu-devel] [PATCH 13/23] lm32: Suppress unused default drives
Am Donnerstag 09 August 2012, 15:31:14 schrieb Markus Armbruster:
> Cc: Michael Walle
>
> Suppress default floppy and CD-ROM drives for machines lm32-evr,
> lm32-uclinux, milkymist.
>
> Suppress default SD card drive for machines lm32-evr, lm32-uclinux.
>
> Signed-off-by: Markus Armbruster
> ---
> hw/lm32_boards.c | 6 ++
> hw/milkymist.c | 2 ++
> 2 files changed, 8 insertions(+)
>
> diff --git a/hw/lm32_boards.c b/hw/lm32_boards.c
> index b76d800..973f89e 100644
> --- a/hw/lm32_boards.c
> +++ b/hw/lm32_boards.c
> @@ -290,6 +290,9 @@ static QEMUMachine lm32_evr_machine = {
> .name = "lm32-evr",
> .desc = "LatticeMico32 EVR32 eval system",
> .init = lm32_evr_init,
> +.no_floppy = 1,
> +.no_cdrom = 1,
> +.no_sdcard = 1,
> .is_default = 1
> };
>
> @@ -297,6 +300,9 @@ static QEMUMachine lm32_uclinux_machine = {
> .name = "lm32-uclinux",
> .desc = "lm32 platform for uClinux and u-boot by Theobroma Systems",
> .init = lm32_uclinux_init,
> +.no_floppy = 1,
> +.no_cdrom = 1,
> +.no_sdcard = 1,
> .is_default = 0
> };
>
> diff --git a/hw/milkymist.c b/hw/milkymist.c
> index 2e7235b..d58afe4 100644
> --- a/hw/milkymist.c
> +++ b/hw/milkymist.c
> @@ -207,6 +207,8 @@ static QEMUMachine milkymist_machine = {
> .name = "milkymist",
> .desc = "Milkymist One",
> .init = milkymist_init,
> +.no_floppy = 1,
> +.no_cdrom = 1,
> .is_default = 0
> };
Acked-by: Michael Walle
--
Michael
Re: [Qemu-devel] TSC in qem[-kvm] 1.1+ and in-kernel irqchip
On 10.08.2012 00:47, Marcelo Tosatti wrote:
[]
>>> calibrate_tsc (void)
>>> {
>>> /* First calibrate the TSC rate (relative, not absolute time). */
>>> grub_uint64_t start_tsc;
>>> grub_uint64_t end_tsc;
>>>
>>> start_tsc = grub_get_tsc ();
>>> grub_pit_wait (0x);
>>> end_tsc = grub_get_tsc ();
>>>
>>> tsc_ticks_per_ms = grub_divmod64 (end_tsc - start_tsc, 55, 0);
>>> }
>>
>> Emulation of grub_pit_wait sequence by in-kernel PIT is probably broken.
This is grub_pit_wait():
#define TIMER2_REG_CONTROL 0x42
#define TIMER_REG_COMMAND 0x43
#define TIMER2_REG_LATCH0x61
#define TIMER2_SELECT 0x80
#define TIMER_ENABLE_LSB0x20
#define TIMER_ENABLE_MSB0x10
#define TIMER2_LATCH0x20
#define TIMER2_SPEAKER 0x02
#define TIMER2_GATE 0x01
void
grub_pit_wait (grub_uint16_t tics)
{
/* Disable timer2 gate and speaker. */
grub_outb (grub_inb (TIMER2_REG_LATCH) & ~ (TIMER2_SPEAKER | TIMER2_GATE),
TIMER2_REG_LATCH);
/* Set tics. */
grub_outb (TIMER2_SELECT | TIMER_ENABLE_LSB | TIMER_ENABLE_MSB,
TIMER_REG_COMMAND);
grub_outb (tics & 0xff, TIMER2_REG_CONTROL);
grub_outb (tics >> 8, TIMER2_REG_CONTROL);
/* Enable timer2 gate, keep speaker disabled. */
grub_outb ((grub_inb (TIMER2_REG_LATCH) & ~ TIMER2_SPEAKER) | TIMER2_GATE,
TIMER2_REG_LATCH);
/* Wait. */
while ((grub_inb (TIMER2_REG_LATCH) & TIMER2_LATCH) == 0x00);
/* Disable timer2 gate and speaker. */
grub_outb (grub_inb (TIMER2_REG_LATCH) & ~ (TIMER2_SPEAKER | TIMER2_GATE),
TIMER2_REG_LATCH);
}
>> QEMU PIT emulation is also affected by miscalibration.
>>
>> Please provide steps to reproduce.
>
> I mean verbose on the steps (does it happen always when setting timeout=1,
> how to set timeout=1, etc).
untested:
mkdir /tmp/grub
cd /tmp/grub
mkdir boot boot/grub
cat > boot/grub/grub.cfg <
Re: [Qemu-devel] TSC in qem[-kvm] 1.1+ and in-kernel irqchip
On 10.08.2012 11:33, Gleb Natapov wrote: > On Thu, Aug 09, 2012 at 10:27:43PM +0400, Michael Tokarev wrote: >> As a follow-up to the patch "tsc: use kvmclock for >> calibration". >> >> There's another problem reported by several users. >> The sympthom is that grub does not show boot menu, >> it boots default entry right away without any pause. >> >> After quite some debugging it turned out to be >> TSC issue. Grub uses tsc for its timeout handling. >> When setting timeout to some very large value >> (1), I can see the counter is ticking backwards >> at a very high speed, ticking from 1 to 0 in >> about 5 seconds. >> >> Running kvm -cpu host,-tsc forces grub to use >> rtc clocksource, and the problem goes away. >> > Can you try -no-kvm-pit-reinjection please. It does not help. With -no-kvm-pit-reinjection, the time in grub is ticking about 1000 times faster still, just like without. >> The most interesting thing is that this is a >> problem new for qemu-kvm 1.1 (and is still >> present in current git), 1.0 version had no >> such issue. And it only happens when in-kernel >> irqchip is enabled -- running with -no-kvm-irqchip >> also fixes the grub problem, so that tsc starts >> counting "correctly" for grub again. >> > 1.0 work on the same kernel 1.1 doesn't? Yes. This issue does not look like kernel-dependent - it happens the same way on 3.0, 3.2 and 3.5 kernels. Note that upstream qemu 1.1+ is also affected, when used with -machine pc,kernel_irqchip=on. /mjt
Re: [Qemu-devel] [PATCH 2/2] usb-storage: fix SYNCHRONIZE_CACHE
On 07.08.2012 12:59, Gerd Hoffmann wrote:
> Commit 59310659073d85745854f2f10c4292555c5a1c51 is incomplete,
> we'll arrive in the scsi command complete callback in CSW state
> and must handle that case correctly.
It appears to be 1.1-stable material, rigt?
What's the outcome of the issue -- guest-triggerable qemu crashing?
Thanks,
/mjt
> Signed-off-by: Gerd Hoffmann
> ---
> hw/usb/dev-storage.c |3 +++
> 1 files changed, 3 insertions(+), 0 deletions(-)
>
> diff --git a/hw/usb/dev-storage.c b/hw/usb/dev-storage.c
> index 976fe1a..ff48d91 100644
> --- a/hw/usb/dev-storage.c
> +++ b/hw/usb/dev-storage.c
> @@ -247,6 +247,9 @@ static void usb_msd_command_complete(SCSIRequest *req,
> uint32_t status, size_t r
> the status read packet. */
> usb_msd_send_status(s, p);
> s->mode = USB_MSDM_CBW;
> +} else if (s->mode == USB_MSDM_CSW) {
> +usb_msd_send_status(s, p);
> +s->mode = USB_MSDM_CBW;
> } else {
> if (s->data_len) {
> int len = (p->iov.size - p->result);
[Qemu-devel] [Bug 1033494] Re: qemu-system-x86_64 segfaults with kernel 3.5.0
this thread and this fix http://thread.gmane.org/gmane.comp.emulators.kvm.devel/95314/focus=1338226 are about the same issue, apparently. Please try it and see if it fixes you issue too. -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1033494 Title: qemu-system-x86_64 segfaults with kernel 3.5.0 Status in QEMU: New Bug description: qemu-kvm 1.1.1 stable is running fine for me with RHEL 6 2.6.32 based kernel. But with 3.5.0 kernel qemu-system-x86_64 segfaults while i'm trying to install ubuntu 12.04 server reproducable. You find three backtraces here: http://pastebin.com/raw.php?i=xCy2pEcP Stefan To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1033494/+subscriptions
[Qemu-devel] [PATCH] json-parser: don't replicate tokens at each level of recursion
Currently, when parsing a stream of tokens we make a copy of the token
list at the beginning of each level of recursion so that we do not
modify the original list in cases where we need to fall back to an
earlier state.
In the worst case, we will only read 1 or 2 tokens off the list before
recursing again, which means an upper bound of roughly N^2 token allocations.
For a "reasonably" sized QMP request (in this a QMP representation of
cirrus_vga's device state, generated via QIDL, being passed in via
qom-set), this caused my 16GB's of memory to be exhausted before any
noticeable progress was made by the parser. The command is here for
reference, and can be issued against upstream QMP to reproduce (failure
occurs before any qmp command routing/execution):
http://pastebin.com/mJrZ3Ctg
This patch works around the issue by using single copy of the token list
in the form of an indexable array so that we can save/restore state by
manipulating indices. With this patch applied the above QMP/JSON request
can be parsed in under a second.
Tested with valgrind, make check, and QMP.
Signed-off-by: Michael Roth
---
json-parser.c | 234 +++--
1 file changed, 146 insertions(+), 88 deletions(-)
diff --git a/json-parser.c b/json-parser.c
index 849e215..b4e6a60 100644
--- a/json-parser.c
+++ b/json-parser.c
@@ -27,6 +27,11 @@
typedef struct JSONParserContext
{
Error *err;
+struct {
+QObject **buf;
+size_t pos;
+size_t count;
+} tokens;
} JSONParserContext;
#define BUG_ON(cond) assert(!(cond))
@@ -40,7 +45,7 @@ typedef struct JSONParserContext
* 4) deal with premature EOI
*/
-static QObject *parse_value(JSONParserContext *ctxt, QList **tokens, va_list
*ap);
+static QObject *parse_value(JSONParserContext *ctxt, va_list *ap);
/**
* Token manipulators
@@ -270,27 +275,115 @@ out:
return NULL;
}
+static QObject *parser_context_pop_token(JSONParserContext *ctxt)
+{
+QObject *token;
+g_assert(ctxt->tokens.pos < ctxt->tokens.count);
+token = ctxt->tokens.buf[ctxt->tokens.pos];
+ctxt->tokens.pos++;
+return token;
+}
+
+/* Note: parser_context_{peek|pop}_token do not increment the
+ * token object's refcount. In both cases the references will continue
+ * to be * tracked and cleanup in parser_context_free()
+ */
+static QObject *parser_context_peek_token(JSONParserContext *ctxt)
+{
+QObject *token;
+g_assert(ctxt->tokens.pos < ctxt->tokens.count);
+token = ctxt->tokens.buf[ctxt->tokens.pos];
+return token;
+}
+
+static JSONParserContext parser_context_save(JSONParserContext *ctxt)
+{
+JSONParserContext saved_ctxt;
+saved_ctxt.tokens.pos = ctxt->tokens.pos;
+saved_ctxt.tokens.count = ctxt->tokens.count;
+saved_ctxt.tokens.buf = ctxt->tokens.buf;
+return saved_ctxt;
+}
+
+static void parser_context_restore(JSONParserContext *ctxt,
+ JSONParserContext saved_ctxt)
+{
+ctxt->tokens.pos = saved_ctxt.tokens.pos;
+ctxt->tokens.count = saved_ctxt.tokens.count;
+ctxt->tokens.buf = saved_ctxt.tokens.buf;
+}
+
+static void tokens_count_from_iter(QObject *obj, void *opaque)
+{
+size_t *count = opaque;
+(*count)++;
+}
+
+static void tokens_append_from_iter(QObject *obj, void *opaque)
+{
+JSONParserContext *ctxt = opaque;
+g_assert(ctxt->tokens.pos < ctxt->tokens.count);
+ctxt->tokens.buf[ctxt->tokens.pos++] = obj;
+qobject_incref(obj);
+}
+
+static JSONParserContext *parser_context_new(QList *tokens)
+{
+JSONParserContext *ctxt;
+size_t count = 0;
+
+if (!tokens) {
+return NULL;
+}
+
+qlist_iter(tokens, tokens_count_from_iter, &count);
+if (count == 0) {
+return NULL;
+}
+
+ctxt = g_malloc0(sizeof(JSONParserContext));
+ctxt->tokens.pos = 0;
+ctxt->tokens.count = count;
+ctxt->tokens.buf = g_malloc(count * sizeof(QObject *));
+qlist_iter(tokens, tokens_append_from_iter, ctxt);
+ctxt->tokens.pos = 0;
+
+return ctxt;
+}
+
+static void parser_context_free(JSONParserContext *ctxt)
+{
+int i;
+if (ctxt) {
+for (i = 0; i < ctxt->tokens.count; i++) {
+qobject_decref(ctxt->tokens.buf[i]);
+}
+g_free(ctxt->tokens.buf);
+g_free(ctxt);
+}
+}
+
/**
* Parsing rules
*/
-static int parse_pair(JSONParserContext *ctxt, QDict *dict, QList **tokens,
va_list *ap)
+static int parse_pair(JSONParserContext *ctxt, QDict *dict, va_list *ap)
{
QObject *key = NULL, *token = NULL, *value, *peek;
-QList *working = qlist_copy(*tokens);
+JSONParserContext saved_ctxt = parser_context_save(ctxt);
-peek = qlist_peek(working);
+peek = parser_context_peek_token(ctxt);
if (peek == NULL) {
parse_error(ctxt, NULL, "premature EOI");
g
[Qemu-devel] [Bug 1033494] Re: qemu-system-x86_64 segfaults with kernel 3.5.0
** Changed in: qemu Status: New => Incomplete -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1033494 Title: qemu-system-x86_64 segfaults with kernel 3.5.0 Status in QEMU: Incomplete Bug description: qemu-kvm 1.1.1 stable is running fine for me with RHEL 6 2.6.32 based kernel. But with 3.5.0 kernel qemu-system-x86_64 segfaults while i'm trying to install ubuntu 12.04 server reproducable. You find three backtraces here: http://pastebin.com/raw.php?i=xCy2pEcP Stefan To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1033494/+subscriptions
Re: [Qemu-devel] [PATCH] iov_send_recv(): Handle zero bytes case even if OS does not
On 12.08.2012 01:24, Peter Maydell wrote:
> POSIX allows sendmsg() and recvmsg() to fail EMSGSIZE if passed a zero
> msg.msg_iovlen (in particular the MacOS X implementation will do this).
> Handle the case where iov_send_recv() is passed a zero byte count
> explicitly, to avoid accidentally depending on the OS to treat zero
> msg_iovlen as a no-op.
> Signed-off-by: Peter Maydell
> ---
> This is what was causing 'make check' to fail on MacOS X.
> The other option was to declare that a zero bytecount was illegal, I guess.
I don't think sending/receiving zero bytes is a good idea in the
first place. Which test were failed on MacOS? Was it failing at
test-iov "random I/O"?
I thought I ensured that the test does not call any i/o function
with zero "count" argument. Might be I was wrong, and in that
case THAT place should be fixed instead.
Can you provide a bit more details please?
The whole thing is actually interesting: this is indeed a system-
dependent corner case which should be handled in the code to make
the routine consistent. But how to fix this is an open question
I think. Your approach seems to be best, but we as well may
print a warning there...
Thank you!
/mjt
> iov.c | 7 +++
> 1 file changed, 7 insertions(+)
>
> diff --git a/iov.c b/iov.c
> index b333061..60705c7 100644
> --- a/iov.c
> +++ b/iov.c
> @@ -146,6 +146,13 @@ ssize_t iov_send_recv(int sockfd, struct iovec *iov,
> unsigned iov_cnt,
> {
> ssize_t ret;
> unsigned si, ei;/* start and end indexes */
> +if (bytes == 0) {
> +/* Catch the do-nothing case early, as otherwise we will pass an
> + * empty iovec to sendmsg/recvmsg(), and not all implementations
> + * accept this.
> + */
> +return 0;
> +}
>
> /* Find the start position, skipping `offset' bytes:
> * first, skip all full-sized vector elements, */
Re: [Qemu-devel] TSC in qem[-kvm] 1.1+ and in-kernel irqchip
On 12.08.2012 12:10, Gleb Natapov wrote:
[]
> Any chance to bisect it?
The bisecion leads to this commit:
commit 17ee47418e65b1593defb30edbab33ccd47fc1f8
Merge: 13b0496 5d17c0d
Author: Jan Kiszka
Date: Tue Apr 10 16:26:23 2012 +0200
Merge commit '5d17c0d2df4998598e6002b27b8e47e792899a0f' into
queues/qemu-merge
Conflicts:
hw/pc.c
diff --cc Makefile.target
index 33a7255,1bd25a8..32c8e42
--- a/Makefile.target
+++ b/Makefile.target
@@@ -245,13 -244,8 +245,13 @@@ obj-i386-y += pci-hotplug.o smbios.o wd
obj-i386-y += debugcon.o multiboot.o
obj-i386-y += pc_piix.o
obj-i386-y += pc_sysfw.o
- obj-i386-$(CONFIG_KVM) += kvm/clock.o kvm/apic.o kvm/i8259.o kvm/ioapic.o
+ obj-i386-$(CONFIG_KVM) += kvm/clock.o kvm/apic.o kvm/i8259.o kvm/ioapic.o
kvm/i8254.o
obj-i386-$(CONFIG_SPICE) += qxl.o qxl-logger.o qxl-render.o
+obj-i386-y += testdev.o
+obj-i386-y += acpi.o acpi_piix4.o
+
+obj-i386-y += i8254_common.o i8254.o
+obj-i386-$(CONFIG_KVM_DEVICE_ASSIGNMENT) += device-assignment.o
# shared objects
obj-ppc-y = ppc.o ppc_booke.o
diff --cc hw/pc.c
index 74c19b9,bb9867b..feb6ef3
--- a/hw/pc.c
+++ b/hw/pc.c
@@@ -1116,8 -1118,12 +1122,12 @@@ void pc_basic_device_init(ISABus *isa_b
qemu_register_boot_set(pc_boot_set, *rtc_state);
- pit = pit_init(isa_bus, 0x40, pit_isa_irq, pit_alt_irq);
+ if (kvm_irqchip_in_kernel()) {
+ pit = kvm_pit_init(isa_bus, 0x40);
+ } else {
+ pit = pit_init(isa_bus, 0x40, pit_isa_irq, pit_alt_irq);
+ }
-if (hpet) {
+if (hpet && !(kvm_enabled() && kvm_irqchip_in_kernel())) {
/* connect PIT to output control line of the HPET */
qdev_connect_gpio_out(hpet, 0, qdev_get_gpio_in(&pit->qdev, 0));
}
Note this commit itself talks about pit and irqchip.
But I don't know what does it mean.
Cc'ing Jan for help. The short story: tsc timer calibration
broke in 1.1+ with in-kernel irqchip (only) for several
apps (seabios and grub are two examples), the time is ticking
about 100 times faster. In grub the timer is calibrated
using pit. The above commit is the result of bisection.
Thanks,
/mjt
Re: [Qemu-devel] [PATCH] virtio: fix vhost handling
On 06.08.2012 18:56, Paolo Bonzini wrote:
> Commit b1f416aa8d870fab71030abc9401cfc77b948e8e breaks vhost_net
> because it always registers the virtio_pci_host_notifier_read() handler
> function on the ioeventfd, even when vhost_net.ko is using the ioeventfd.
> The result is both QEMU and vhost_net.ko polling on the same eventfd
> and the virtio_net.ko guest driver seeing inconsistent results:
>
> # ifconfig eth0 192.168.0.1 netmask 255.255.255.0
> virtio_net virtio0: output:id 0 is not a head!
>
> To fix this, proceed the same as we do for irqfd: add a parameter to
> virtio_queue_set_host_notifier_fd_handler and in that case only set
> the notifier, not the handler.
Stable-1.1 material? The mentioned commit is included into 1.1 release.
Thanks,
/mjt
> Cc: Stefan Hajnoczi
> Signed-off-by: Paolo Bonzini
> ---
> The patch is a bit different from what I posted before,
> so I didn't add Stefan's *-by tags.
>
> hw/virtio-pci.c | 14 +++---
> hw/virtio.c | 7 +--
> hw/virtio.h | 3 ++-
> 3 file modificati, 14 inserzioni(+), 10 rimozioni(-)
>
> diff --git a/hw/virtio-pci.c b/hw/virtio-pci.c
> index 3ab9747..6133626 100644
> --- a/hw/virtio-pci.c
> +++ b/hw/virtio-pci.c
> @@ -160,7 +160,7 @@ static int virtio_pci_load_queue(void * opaque, int n,
> QEMUFile *f)
> }
>
> static int virtio_pci_set_host_notifier_internal(VirtIOPCIProxy *proxy,
> - int n, bool assign)
> + int n, bool assign, bool
> set_handler)
> {
> VirtQueue *vq = virtio_get_queue(proxy->vdev, n);
> EventNotifier *notifier = virtio_queue_get_host_notifier(vq);
> @@ -173,13 +173,13 @@ static int
> virtio_pci_set_host_notifier_internal(VirtIOPCIProxy *proxy,
> __func__, r);
> return r;
> }
> -virtio_queue_set_host_notifier_fd_handler(vq, true);
> +virtio_queue_set_host_notifier_fd_handler(vq, true, set_handler);
> memory_region_add_eventfd(&proxy->bar, VIRTIO_PCI_QUEUE_NOTIFY, 2,
>true, n, notifier);
> } else {
> memory_region_del_eventfd(&proxy->bar, VIRTIO_PCI_QUEUE_NOTIFY, 2,
>true, n, notifier);
> -virtio_queue_set_host_notifier_fd_handler(vq, false);
> +virtio_queue_set_host_notifier_fd_handler(vq, false, false);
> event_notifier_cleanup(notifier);
> }
> return r;
> @@ -200,7 +200,7 @@ static void virtio_pci_start_ioeventfd(VirtIOPCIProxy
> *proxy)
> continue;
> }
>
> -r = virtio_pci_set_host_notifier_internal(proxy, n, true);
> +r = virtio_pci_set_host_notifier_internal(proxy, n, true, true);
> if (r < 0) {
> goto assign_error;
> }
> @@ -214,7 +214,7 @@ assign_error:
> continue;
> }
>
> -r = virtio_pci_set_host_notifier_internal(proxy, n, false);
> +r = virtio_pci_set_host_notifier_internal(proxy, n, false, false);
> assert(r >= 0);
> }
> proxy->ioeventfd_started = false;
> @@ -235,7 +235,7 @@ static void virtio_pci_stop_ioeventfd(VirtIOPCIProxy
> *proxy)
> continue;
> }
>
> -r = virtio_pci_set_host_notifier_internal(proxy, n, false);
> +r = virtio_pci_set_host_notifier_internal(proxy, n, false, false);
> assert(r >= 0);
> }
> proxy->ioeventfd_started = false;
> @@ -683,7 +683,7 @@ static int virtio_pci_set_host_notifier(void *opaque, int
> n, bool assign)
> * currently only stops on status change away from ok,
> * reset, vmstop and such. If we do add code to start here,
> * need to check vmstate, device state etc. */
> -return virtio_pci_set_host_notifier_internal(proxy, n, assign);
> +return virtio_pci_set_host_notifier_internal(proxy, n, assign, false);
> }
>
> static void virtio_pci_vmstate_change(void *opaque, bool running)
> diff --git a/hw/virtio.c b/hw/virtio.c
> index d146f86..89e6d6f 100644
> --- a/hw/virtio.c
> +++ b/hw/virtio.c
> @@ -1021,13 +1021,16 @@ static void
> virtio_queue_host_notifier_read(EventNotifier *n)
> }
> }
>
> -void virtio_queue_set_host_notifier_fd_handler(VirtQueue *vq, bool assign)
> +void virtio_queue_set_host_notifier_fd_handler(VirtQueue *vq, bool assign,
> + bool set_handler)
> {
> -if (assign) {
> +if (assign && set_handler) {
> event_notifier_set_handler(&vq->host_notifier,
> virtio_queue_host_notifier_read);
> } else {
> event_notifier_set_handler(&vq->host_notifier, NULL);
> +}
> +if (!assign) {
> /* Test and clear notifier before after disabling event,
> * in case poll callback didn't have time to run. */
> virtio_queue_host_notifier_read(&vq->host_notifier);
> diff --git a/hw/v
Re: [Qemu-devel] [PATCH] iov_send_recv(): Handle zero bytes case even if OS does not
On 12.08.2012 01:24, Peter Maydell wrote:
> POSIX allows sendmsg() and recvmsg() to fail EMSGSIZE if passed a zero
> msg.msg_iovlen (in particular the MacOS X implementation will do this).
> Handle the case where iov_send_recv() is passed a zero byte count
> explicitly, to avoid accidentally depending on the OS to treat zero
> msg_iovlen as a no-op.
>
> Signed-off-by: Peter Maydell
> ---
> This is what was causing 'make check' to fail on MacOS X.
> The other option was to declare that a zero bytecount was illegal, I guess.
Acked-by: Michael Tokarev
Kevin, does this fix the test-iov failure you're seeing on one of
the build bots?
Thank you!
/mjt
> iov.c | 7 +++
> 1 file changed, 7 insertions(+)
>
> diff --git a/iov.c b/iov.c
> index b333061..60705c7 100644
> --- a/iov.c
> +++ b/iov.c
> @@ -146,6 +146,13 @@ ssize_t iov_send_recv(int sockfd, struct iovec *iov,
> unsigned iov_cnt,
> {
> ssize_t ret;
> unsigned si, ei;/* start and end indexes */
> +if (bytes == 0) {
> +/* Catch the do-nothing case early, as otherwise we will pass an
> + * empty iovec to sendmsg/recvmsg(), and not all implementations
> + * accept this.
> + */
> +return 0;
> +}
>
> /* Find the start position, skipping `offset' bytes:
> * first, skip all full-sized vector elements, */
Re: [Qemu-devel] TSC in qem[-kvm] 1.1+ and in-kernel irqchip
On 13.08.2012 17:07, Jan Kiszka wrote: [] >> The bisecion leads to this commit: >> >> commit 17ee47418e65b1593defb30edbab33ccd47fc1f8 >> Merge: 13b0496 5d17c0d >> Author: Jan Kiszka >> Date: Tue Apr 10 16:26:23 2012 +0200 >> >> Merge commit '5d17c0d2df4998598e6002b27b8e47e792899a0f' into >> queues/qemu-merge [] >> Cc'ing Jan for help. The short story: tsc timer calibration >> broke in 1.1+ with in-kernel irqchip (only) for several >> apps (seabios and grub are two examples), the time is ticking >> about 100 times faster. In grub the timer is calibrated >> using pit. The above commit is the result of bisection. > > Did the versions you tested include the commit 0cdd3d1444 (kvm: i8254: > Fix conversion of in-kernel to userspace state)? While bisecting I didn't have this commit applied, since it were applied past (qemu)-1.1. It is included into qemu-kvm 1.1.0 (as 960d355dc60d9), and that version behaves _exactly_ the same - the time in grub is ticking 100 times faster. I mentioned in this thread that the problem persists in current qemu (and qemu-kvm) git too. I can repeat the bisection with this commit applied after the above "bad" commit. Should I? Thanks, /mjt
Re: [Qemu-devel] [PATCH uq/master] kvm: i8254: Finish time conversion fix
On 13.08.2012 22:18, Jan Kiszka wrote: > 0cdd3d1444 fixed reading back the counter load time from the kernel > while assuming the kernel would always update its load time on writing > the state. That is only true for channel 1, and so pit_get_channel_info > returned wrong output pin states for high counter values. > > Fix this by applying the offset also on kvm_pit_put. For this purpose, > we cache the clock offset in KVMPITState, only updating it on VM state > changes or when we write the state while the VM is stopped. Wug. The fix (consisting of two halves) appears to be quite messy. Is it a (temporary) workaround or a real solution? And yes, this second half fixes the reported issue with grub timekeeping, and should fix the seabios problem as well (so it shouldn't be necessary to mess with timekeeping in seabios anymore). Thank you Jan! /mjt
Re: [Qemu-devel] [PATCH] json-parser: don't replicate tokens at each level of recursion
On Mon, Aug 13, 2012 at 03:01:56PM -0300, Luiz Capitulino wrote:
> On Fri, 10 Aug 2012 18:24:10 -0500
> Michael Roth wrote:
>
> > Currently, when parsing a stream of tokens we make a copy of the token
> > list at the beginning of each level of recursion so that we do not
> > modify the original list in cases where we need to fall back to an
> > earlier state.
> >
> > In the worst case, we will only read 1 or 2 tokens off the list before
> > recursing again, which means an upper bound of roughly N^2 token
> > allocations.
> >
> > For a "reasonably" sized QMP request (in this a QMP representation of
> > cirrus_vga's device state, generated via QIDL, being passed in via
> > qom-set), this caused my 16GB's of memory to be exhausted before any
> > noticeable progress was made by the parser. The command is here for
> > reference, and can be issued against upstream QMP to reproduce (failure
> > occurs before any qmp command routing/execution):
> >
> > http://pastebin.com/mJrZ3Ctg
> >
> > This patch works around the issue by using single copy of the token list
> > in the form of an indexable array so that we can save/restore state by
> > manipulating indices. With this patch applied the above QMP/JSON request
> > can be parsed in under a second.
>
> Approach looks sane to me, a few review comments below.
>
> Anthony, could you provide your reviewed-by please?
>
> >
> > Tested with valgrind, make check, and QMP.
> >
> > Signed-off-by: Michael Roth
> > ---
> > json-parser.c | 234
> > +++--
> > 1 file changed, 146 insertions(+), 88 deletions(-)
> >
> > diff --git a/json-parser.c b/json-parser.c
> > index 849e215..b4e6a60 100644
> > --- a/json-parser.c
> > +++ b/json-parser.c
> > @@ -27,6 +27,11 @@
> > typedef struct JSONParserContext
> > {
> > Error *err;
> > +struct {
> > +QObject **buf;
> > +size_t pos;
> > +size_t count;
> > +} tokens;
> > } JSONParserContext;
> >
> > #define BUG_ON(cond) assert(!(cond))
> > @@ -40,7 +45,7 @@ typedef struct JSONParserContext
> > * 4) deal with premature EOI
> > */
> >
> > -static QObject *parse_value(JSONParserContext *ctxt, QList **tokens,
> > va_list *ap);
> > +static QObject *parse_value(JSONParserContext *ctxt, va_list *ap);
> >
> > /**
> > * Token manipulators
> > @@ -270,27 +275,115 @@ out:
> > return NULL;
> > }
> >
> > +static QObject *parser_context_pop_token(JSONParserContext *ctxt)
> > +{
> > +QObject *token;
> > +g_assert(ctxt->tokens.pos < ctxt->tokens.count);
> > +token = ctxt->tokens.buf[ctxt->tokens.pos];
> > +ctxt->tokens.pos++;
> > +return token;
> > +}
> > +
> > +/* Note: parser_context_{peek|pop}_token do not increment the
> > + * token object's refcount. In both cases the references will continue
> > + * to be * tracked and cleanup in parser_context_free()
> > + */
> > +static QObject *parser_context_peek_token(JSONParserContext *ctxt)
> > +{
> > +QObject *token;
> > +g_assert(ctxt->tokens.pos < ctxt->tokens.count);
> > +token = ctxt->tokens.buf[ctxt->tokens.pos];
> > +return token;
> > +}
> > +
> > +static JSONParserContext parser_context_save(JSONParserContext *ctxt)
> > +{
> > +JSONParserContext saved_ctxt;
> > +saved_ctxt.tokens.pos = ctxt->tokens.pos;
> > +saved_ctxt.tokens.count = ctxt->tokens.count;
> > +saved_ctxt.tokens.buf = ctxt->tokens.buf;
>
> Shouldn't saved_ctxt.err be initialized to NULL?
>
Yes.
> > +return saved_ctxt;
>
> I was going to say that saved_ctxt had to be static, but then I realized
> this will end up copying the struct to the caller.
>
Yah, every caller needs to manage it's own context, so I went with
passing by copy since allocating/deallocating these "half-contexts" (we
only care about token state with these) seems unecessary.
seemed
> > +}
> > +
> > +static void parser_context_restore(JSONParserContext *ctxt,
> > + JSONParserContext saved_ctxt)
> > +{
> > +ctxt->tokens.pos = saved_ctxt.tokens.pos;
> > +ctxt->tokens.count = saved_ctxt.tokens.count;
> > +ctxt->tokens.buf = saved_ctxt.tokens.buf;
> > +}
> > +
> > +static void tokens_count_from_iter(QObject *obj,
Re: [Qemu-devel] [PATCH] json-parser: don't replicate tokens at each level of recursion
On Mon, Aug 13, 2012 at 08:49:26PM +0200, Markus Armbruster wrote: > Michael Roth writes: > > > Currently, when parsing a stream of tokens we make a copy of the token > > list at the beginning of each level of recursion so that we do not > > modify the original list in cases where we need to fall back to an > > earlier state. > > > > In the worst case, we will only read 1 or 2 tokens off the list before > > recursing again, which means an upper bound of roughly N^2 token > > allocations. > > > > For a "reasonably" sized QMP request (in this a QMP representation of > > cirrus_vga's device state, generated via QIDL, being passed in via > > qom-set), this caused my 16GB's of memory to be exhausted before any > > noticeable progress was made by the parser. The command is here for > > reference, and can be issued against upstream QMP to reproduce (failure > > occurs before any qmp command routing/execution): > > > > http://pastebin.com/mJrZ3Ctg > > Commit messages are forever, pastebins aren't. > > What about preserving your test case for eternity under tests/? We might be able to generate some json objects that cause the behavior and add them to check-qjson. > > [...] >
[Qemu-devel] Add infrastructure for supporting QIDL annotations
These patches are based are origin/master, and can also be obtained from: git://github.com/mdroth/qemu.git qidl-base This is a cleanup of the infrastructure bits from "[RFC v2] Use QEMU IDL for device serialization/introspection". I know this is pretty late into the release cycle, but the patches in this series have gotten decent review, and I have a seperate branch (qidl-conv1 in my github repo) that uses these bits to serialize i440fx, piix3/piix ide, usb, cirrus_vga, and a few others via QIDL. So there shouldn't be too much churn going forward. Changes since rfc v2: - Parser/Codegen fix-ups for cases encountered converting piix ide and usb. - Fixed license headers. - Stricter arg-checking for QIDL macros when passing to codegen. - Added asserts to detect mis-use of QIDL property/visitor interfaces. - Renamed QAPI visit_*_array interfaces to visit_*_carray to clarify that these are serialization routines for single-dimension C arrays.
[Qemu-devel] [PATCH 11/20] qapi: qapi.py, make json parser more robust
Currently the QAPI JSON parser expects a very particular style of code
indentation, the major one being that terminating curly/square brackets are
not on placed on a seperate line. This is incompatible with most
pretty-print formats, so make it a little more robust by supporting
these cases.
Signed-off-by: Michael Roth
---
scripts/qapi.py |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/scripts/qapi.py b/scripts/qapi.py
index a347203..47cd672 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -81,7 +81,7 @@ def parse_schema(fp):
if line.startswith('#') or line == '\n':
continue
-if line.startswith(' '):
+if line[0] in ['}', ']', ' ', '\t']:
expr += line
elif expr:
expr_eval = evaluate(expr)
--
1.7.9.5
[Qemu-devel] [PATCH 13/20] qom-fuse: workaround for truncated properties > 4096
We currently hard-code property size at 4096 for the purposes of
getattr()/stat()/etc. For 'state' properties we can exceed this easily,
leading to truncated responses.
Instead, for a particular property, make it
max(4096, most_recent_property_size * 2). This allows some
head-room for properties that change size periodically (numbers,
strings, state properties containing arrays, etc)
Also, implement a simple property cache to avoid spinning on qom-get
if an application reads beyond the actual size. This also allows us
to use a snapshot of a single qom-get that persists across read()'s.
Old Cache entries are evicted as soon as we attempt to read() from
offset 0 again.
Reviewed-by: Anthony Liguori
Signed-off-by: Michael Roth
---
QMP/qom-fuse | 24 +---
1 file changed, 17 insertions(+), 7 deletions(-)
diff --git a/QMP/qom-fuse b/QMP/qom-fuse
index 5c6754a..bd43f29 100755
--- a/QMP/qom-fuse
+++ b/QMP/qom-fuse
@@ -26,6 +26,7 @@ class QOMFS(Fuse):
self.qmp.connect()
self.ino_map = {}
self.ino_count = 1
+self.prop_cache = {}
def get_ino(self, path):
if self.ino_map.has_key(path):
@@ -67,12 +68,16 @@ class QOMFS(Fuse):
if not self.is_property(path):
return -ENOENT
-path, prop = path.rsplit('/', 1)
-try:
-data = str(self.qmp.command('qom-get', path=path, property=prop))
-data += '\n' # make values shell friendly
-except:
-return -EPERM
+# avoid extra calls to qom-get by using cached value when offset > 0
+if offset == 0 or not self.prop_cache.has_key(path):
+directory, prop = path.rsplit('/', 1)
+try:
+resp = str(self.qmp.command('qom-get', path=directory,
property=prop))
+self.prop_cache[path] = resp + '\n' # make values shell
friendly
+except:
+return -EPERM
+
+data = self.prop_cache[path]
if offset > len(data):
return ''
@@ -111,13 +116,18 @@ class QOMFS(Fuse):
0,
0))
elif self.is_property(path):
+directory, prop = path.rsplit('/', 1)
+try:
+resp = str(self.qmp.command('qom-get', path=directory,
property=prop))
+except:
+return -ENOENT
value = posix.stat_result((0644 | stat.S_IFREG,
self.get_ino(path),
0,
1,
1000,
1000,
- 4096,
+ max(len(resp) * 2, 4096),
0,
0,
0))
--
1.7.9.5
[Qemu-devel] [PATCH 05/20] qapi: qapi_visit.py, support arrays and complex qapi definitions
Add support for arrays in the code generators.
Complex field descriptions can now be used to provide additional
information to the visitor generators, such as the max size of an array,
or the field within a struct to use to determine how many elements are
present in the array to avoid serializing uninitialized elements.
Add handling for these in the code generators as well.
Reviewed-by: Anthony Liguori
Signed-off-by: Michael Roth
---
scripts/qapi.py |8 --
scripts/qapi_commands.py |8 +++---
scripts/qapi_types.py|2 +-
scripts/qapi_visit.py| 64 +-
4 files changed, 68 insertions(+), 14 deletions(-)
diff --git a/scripts/qapi.py b/scripts/qapi.py
index 122b4cb..a347203 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -110,12 +110,16 @@ def parse_args(typeinfo):
argentry = typeinfo[member]
optional = False
structured = False
+annotated = False
if member.startswith('*'):
argname = member[1:]
optional = True
if isinstance(argentry, OrderedDict):
-structured = True
-yield (argname, argentry, optional, structured)
+if argentry.has_key(''):
+annotated = True
+else:
+structured = True
+yield (argname, argentry, optional, structured, annotated)
def de_camel_case(name):
new_name = ''
diff --git a/scripts/qapi_commands.py b/scripts/qapi_commands.py
index 3c4678d..a2e0c23 100644
--- a/scripts/qapi_commands.py
+++ b/scripts/qapi_commands.py
@@ -32,7 +32,7 @@ void %(visitor)s(Visitor *m, %(name)s * obj, const char
*name, Error **errp);
def generate_command_decl(name, args, ret_type):
arglist=""
-for argname, argtype, optional, structured in parse_args(args):
+for argname, argtype, optional, structured, annotated in parse_args(args):
argtype = c_type(argtype)
if argtype == "char *":
argtype = "const char *"
@@ -50,7 +50,7 @@ def gen_sync_call(name, args, ret_type, indent=0):
retval=""
if ret_type:
retval = "retval = "
-for argname, argtype, optional, structured in parse_args(args):
+for argname, argtype, optional, structured, annotated in parse_args(args):
if optional:
arglist += "has_%s, " % c_var(argname)
arglist += "%s, " % (c_var(argname))
@@ -106,7 +106,7 @@ Visitor *v;
def gen_visitor_input_vars_decl(args):
ret = ""
push_indent()
-for argname, argtype, optional, structured in parse_args(args):
+for argname, argtype, optional, structured, annotated in parse_args(args):
if optional:
ret += mcgen('''
bool has_%(argname)s = false;
@@ -145,7 +145,7 @@ v = qmp_input_get_visitor(mi);
''',
obj=obj)
-for argname, argtype, optional, structured in parse_args(args):
+for argname, argtype, optional, structured, annotated in parse_args(args):
if optional:
ret += mcgen('''
visit_start_optional(v, &has_%(c_name)s, "%(name)s", errp);
diff --git a/scripts/qapi_types.py b/scripts/qapi_types.py
index cf601ae..8babfe1 100644
--- a/scripts/qapi_types.py
+++ b/scripts/qapi_types.py
@@ -35,7 +35,7 @@ struct %(name)s
''',
name=structname)
-for argname, argentry, optional, structured in parse_args(members):
+for argname, argentry, optional, structured, annotated in
parse_args(members):
if optional:
ret += mcgen('''
bool has_%(c_name)s;
diff --git a/scripts/qapi_visit.py b/scripts/qapi_visit.py
index 25707f5..aac2172 100644
--- a/scripts/qapi_visit.py
+++ b/scripts/qapi_visit.py
@@ -16,6 +16,52 @@ import sys
import os
import getopt
import errno
+import types
+
+def generate_visit_carray_body(name, info):
+if info['array_size'][0].isdigit():
+array_size = info['array_size']
+elif info['array_size'][0] == '(' and info['array_size'][-1] == ')':
+array_size = info['array_size']
+else:
+array_size = "(*obj)->%s" % info['array_size']
+
+if info.has_key('array_capacity'):
+array_capacity = info['array_capacity']
+else:
+array_capacity = array_size
+
+if camel_case(de_camel_case(info['type'][0])) == info['type'][0]:
+ret = mcgen('''
+visit_start_carray(m, (void **)obj, "%(name)s", %(array_capacity)s,
sizeof(%(type)s), errp);
+int %(name)s_i;
+for (%(name)s_i = 0; %(name)s_i < %(array_size)s; %(name)s_i++) {
+%(type)s %(name)s_ptr = &(*obj)->%(name)s[%(name)s_i];
+visit_type_%(type_short)s(m, &%(na
[Qemu-devel] [PATCH 04/20] qapi: qapi_visit.py, make code useable as module
Reviewed-by: Anthony Liguori
Signed-off-by: Michael Roth
---
scripts/qapi_visit.py | 143 +
1 file changed, 74 insertions(+), 69 deletions(-)
diff --git a/scripts/qapi_visit.py b/scripts/qapi_visit.py
index 04ef7c4..25707f5 100644
--- a/scripts/qapi_visit.py
+++ b/scripts/qapi_visit.py
@@ -224,55 +224,57 @@ void visit_type_%(name)s(Visitor *m, %(name)s * obj,
const char *name, Error **e
''',
name=name)
-try:
-opts, args = getopt.gnu_getopt(sys.argv[1:], "chp:o:",
- ["source", "header", "prefix=",
"output-dir="])
-except getopt.GetoptError, err:
-print str(err)
-sys.exit(1)
-
-output_dir = ""
-prefix = ""
-c_file = 'qapi-visit.c'
-h_file = 'qapi-visit.h'
-
-do_c = False
-do_h = False
-
-for o, a in opts:
-if o in ("-p", "--prefix"):
-prefix = a
-elif o in ("-o", "--output-dir"):
-output_dir = a + "/"
-elif o in ("-c", "--source"):
+def main(argv=[]):
+try:
+opts, args = getopt.gnu_getopt(sys.argv[1:], "chp:o:",
+ ["source", "header", "prefix=",
+"output-dir="])
+except getopt.GetoptError, err:
+print str(err)
+sys.exit(1)
+
+output_dir = ""
+prefix = ""
+c_file = 'qapi-visit.c'
+h_file = 'qapi-visit.h'
+
+do_c = False
+do_h = False
+
+for o, a in opts:
+if o in ("-p", "--prefix"):
+prefix = a
+elif o in ("-o", "--output-dir"):
+output_dir = a + "/"
+elif o in ("-c", "--source"):
+do_c = True
+elif o in ("-h", "--header"):
+do_h = True
+
+if not do_c and not do_h:
do_c = True
-elif o in ("-h", "--header"):
do_h = True
-if not do_c and not do_h:
-do_c = True
-do_h = True
+c_file = output_dir + prefix + c_file
+h_file = output_dir + prefix + h_file
-c_file = output_dir + prefix + c_file
-h_file = output_dir + prefix + h_file
+try:
+os.makedirs(output_dir)
+except os.error, e:
+if e.errno != errno.EEXIST:
+raise
-try:
-os.makedirs(output_dir)
-except os.error, e:
-if e.errno != errno.EEXIST:
-raise
-
-def maybe_open(really, name, opt):
-if really:
-return open(name, opt)
-else:
-import StringIO
-return StringIO.StringIO()
+def maybe_open(really, name, opt):
+if really:
+return open(name, opt)
+else:
+import StringIO
+return StringIO.StringIO()
-fdef = maybe_open(do_c, c_file, 'w')
-fdecl = maybe_open(do_h, h_file, 'w')
+fdef = maybe_open(do_c, c_file, 'w')
+fdecl = maybe_open(do_h, h_file, 'w')
-fdef.write(mcgen('''
+fdef.write(mcgen('''
/* THIS FILE IS AUTOMATICALLY GENERATED, DO NOT MODIFY */
/*
@@ -292,7 +294,7 @@ fdef.write(mcgen('''
''',
header=basename(h_file)))
-fdecl.write(mcgen('''
+fdecl.write(mcgen('''
/* THIS FILE IS AUTOMATICALLY GENERATED, DO NOT MODIFY */
/*
@@ -316,37 +318,40 @@ fdecl.write(mcgen('''
''',
prefix=prefix, guard=guardname(h_file)))
-exprs = parse_schema(sys.stdin)
+exprs = parse_schema(sys.stdin)
-for expr in exprs:
-if expr.has_key('type'):
-ret = generate_visit_struct(expr['type'], expr['data'])
-ret += generate_visit_list(expr['type'], expr['data'])
-fdef.write(ret)
+for expr in exprs:
+if expr.has_key('type'):
+ret = generate_visit_struct(expr['type'], expr['data'])
+ret += generate_visit_list(expr['type'], expr['data'])
+fdef.write(ret)
-ret = generate_declaration(expr['type'], expr['data'])
-fdecl.write(ret)
-elif expr.has_key('union'):
-ret = generate_visit_union(expr['union'], expr['data'])
-ret += generate_visit_list(expr['union'], expr['data'])
-fdef.write(ret)
+ret = generate_declaration(expr['type'], expr['data'])
+fdecl.write(ret)
+elif expr.has_key('union'):
+ret = generate_visit_union(expr['union'], expr['data'])
+ret
[Qemu-devel] [PATCH 17/20] qidl: parser, initial import from qc.git
Signed-off-by: Michael Roth
---
scripts/qidl_parser.py | 512
1 file changed, 512 insertions(+)
create mode 100644 scripts/qidl_parser.py
diff --git a/scripts/qidl_parser.py b/scripts/qidl_parser.py
new file mode 100644
index 000..831b3f5
--- /dev/null
+++ b/scripts/qidl_parser.py
@@ -0,0 +1,512 @@
+#
+# QEMU IDL Parser
+#
+# Copyright IBM, Corp. 2012
+#
+# Authors:
+# Anthony Liguori
+# Michael Roth
+#
+# This work is licensed under the terms of the GNU GPLv2.
+# See the COPYING file in the top-level directory.
+#
+# The lexer code is based off of:
+# http://www.lysator.liu.se/c/ANSI-C-grammar-l.html
+
+import sys, json
+
+class Input(object):
+def __init__(self, text):
+self.fp = text
+self.buf = text
+self.eof = False
+
+def pop(self):
+if len(self.buf) == 0:
+self.eof = True
+return ''
+ch = self.buf[0]
+self.buf = self.buf[1:]
+return ch
+
+def in_range(ch, start, end):
+if ch >= start and ch <= end:
+return True
+return False
+
+# D[0-9]
+# L[a-zA-Z_]
+# H[a-fA-F0-9]
+# E[Ee][+-]?{D}+
+# FS (f|F|l|L)
+# IS (u|U|l|L)*
+
+def is_D(ch):
+return in_range(ch, '0', '9')
+
+def is_L(ch):
+return in_range(ch, 'a', 'z') or in_range(ch, 'A', 'Z') or ch == '_'
+
+def is_H(ch):
+return in_range(ch, 'a', 'f') or in_range(ch, 'A', 'F') or is_D(ch)
+
+def is_FS(ch):
+return ch in 'fFlL'
+
+def is_IS(ch):
+return ch in 'uUlL'
+
+def lexer(fp):
+ch = fp.pop()
+
+while not fp.eof:
+token = ''
+
+if is_L(ch):
+token += ch
+
+ch = fp.pop()
+while is_L(ch) or is_D(ch):
+token += ch
+ch = fp.pop()
+if token in [ 'auto', 'break', 'case', 'const', 'continue',
+ 'default', 'do', 'else', 'enum', 'extern',
+ 'for', 'goto', 'if', 'register', 'return', 'signed',
+ 'sizeof',
+ 'static', 'struct', 'typedef', 'union', 'unsigned',
+ 'volatile', 'while' ]:
+yield (token, token)
+else:
+yield ('symbol', token)
+elif ch == "'":
+token += ch
+
+ch = fp.pop()
+if ch == '\\':
+token += ch
+token += fp.pop()
+else:
+token += ch
+token += fp.pop()
+ch = fp.pop()
+yield ('literal', token)
+elif ch == '"':
+token += ch
+
+ch = fp.pop()
+while ch not in ['', '"']:
+token += ch
+if ch == '\\':
+token += fp.pop()
+ch = fp.pop()
+token += ch
+yield ('literal', token)
+ch = fp.pop()
+elif ch in '.><+-*/%&^|!;{},:=()[]~?':
+token += ch
+ch = fp.pop()
+tmp_token = token + ch
+if tmp_token in ['<:']:
+yield ('operator', '[')
+ch = fp.pop()
+elif tmp_token in [':>']:
+yield ('operator', ']')
+ch = fp.pop()
+elif tmp_token in ['<%']:
+yield ('operator', '{')
+ch = fp.pop()
+elif tmp_token in ['%>']:
+yield ('operator', '}')
+ch = fp.pop()
+elif tmp_token == '//':
+token = tmp_token
+ch = fp.pop()
+while ch != '\n' and ch != '':
+token += ch
+ch = fp.pop()
+yield ('comment', token)
+elif tmp_token == '/*':
+token = tmp_token
+
+ch = fp.pop()
+while True:
+while ch != '*':
+token += ch
+ch = fp.pop()
+token += ch
+ch = fp.pop()
+if ch == '/':
+to
[Qemu-devel] [PATCH 19/20] qidl: qidl.h, definitions for qidl annotations
Signed-off-by: Michael Roth
---
qidl.h | 63 +++
1 file changed, 63 insertions(+)
create mode 100644 qidl.h
diff --git a/qidl.h b/qidl.h
new file mode 100644
index 000..210f4c6
--- /dev/null
+++ b/qidl.h
@@ -0,0 +1,63 @@
+/*
+ * QEMU IDL Macros/stubs
+ *
+ * See docs/qidl.txt for usage information.
+ *
+ * Copyright IBM, Corp. 2012
+ *
+ * Authors:
+ * Michael Roth
+ *
+ * This work is licensed under the terms of the GNU GPLv2 or later.
+ * See the COPYING.LIB file in the top-level directory.
+ *
+ */
+
+#ifndef QIDL_H
+#define QIDL_H
+
+#include
+#include "qapi/qapi-visit-core.h"
+#include "qemu/object.h"
+#include "hw/qdev-properties.h"
+
+#ifdef QIDL_GEN
+
+/* we pass the code through the preprocessor with QIDL_GEN defined to parse
+ * structures as they'd appear after preprocessing, and use the following
+ * definitions mostly to re-insert the initial macros/annotations so they
+ * stick around for the parser to process
+ */
+#define QIDL(...) QIDL(__VA_ARGS__)
+#define QIDL_START(name, ...) QIDL_START(name, ##__VA_ARGS__)
+#define QIDL_END(name) QIDL_END(name)
+
+#define QIDL_VISIT_TYPE(name, v, s, f, e)
+#define QIDL_SCHEMA_ADD_LINK(name, obj, path, errp)
+#define QIDL_PROPERTIES(name)
+
+#else /* !QIDL_GEN */
+
+#define QIDL(...)
+#define QIDL_START(name, ...)
+#define QIDL_END(name) \
+static struct { \
+void (*visitor)(Visitor *, struct name **, const char *, Error **); \
+const char *schema_json_text; \
+Object *schema_obj; \
+Property *properties; \
+} qidl_data_##name;
+
+#define QIDL_VISIT_TYPE(name, v, s, f, e) \
+g_assert(qidl_data_##name.visitor); \
+qidl_data_##name.visitor(v, s, f, e)
+#define QIDL_SCHEMA_ADD_LINK(name, obj, path, errp) \
+g_assert(qidl_data_##name.schema_obj); \
+object_property_add_link(obj, path, "container", \
+ &qidl_data_##name.schema_obj, errp)
+#define QIDL_PROPERTIES(name) \
+qidl_data_##name.properties
+
+#endif /* QIDL_GEN */
+
+#endif
--
1.7.9.5
[Qemu-devel] [PATCH 12/20] qapi: add open-coded visitor for struct tm types
Reviewed-by: Anthony Liguori
Signed-off-by: Michael Roth
---
qapi/Makefile.objs |1 +
qapi/misc-qapi-visit.c | 14 ++
qapi/qapi-visit-core.h |3 +++
3 files changed, 18 insertions(+)
create mode 100644 qapi/misc-qapi-visit.c
diff --git a/qapi/Makefile.objs b/qapi/Makefile.objs
index 5f5846e..7604b52 100644
--- a/qapi/Makefile.objs
+++ b/qapi/Makefile.objs
@@ -1,3 +1,4 @@
qapi-obj-y = qapi-visit-core.o qapi-dealloc-visitor.o qmp-input-visitor.o
qapi-obj-y += qmp-output-visitor.o qmp-registry.o qmp-dispatch.o
qapi-obj-y += string-input-visitor.o string-output-visitor.o opts-visitor.o
+qapi-obj-y += misc-qapi-visit.o
diff --git a/qapi/misc-qapi-visit.c b/qapi/misc-qapi-visit.c
new file mode 100644
index 000..a44773d
--- /dev/null
+++ b/qapi/misc-qapi-visit.c
@@ -0,0 +1,14 @@
+#include
+#include "qidl.h"
+
+void visit_type_tm(Visitor *v, struct tm *obj, const char *name, Error **errp)
+{
+visit_start_struct(v, NULL, "struct tm", name, 0, errp);
+visit_type_int32(v, &obj->tm_year, "tm_year", errp);
+visit_type_int32(v, &obj->tm_mon, "tm_mon", errp);
+visit_type_int32(v, &obj->tm_mday, "tm_mday", errp);
+visit_type_int32(v, &obj->tm_hour, "tm_hour", errp);
+visit_type_int32(v, &obj->tm_min, "tm_min", errp);
+visit_type_int32(v, &obj->tm_sec, "tm_sec", errp);
+visit_end_struct(v, errp);
+}
diff --git a/qapi/qapi-visit-core.h b/qapi/qapi-visit-core.h
index 5eb1616..10ec044 100644
--- a/qapi/qapi-visit-core.h
+++ b/qapi/qapi-visit-core.h
@@ -100,4 +100,7 @@ void visit_start_carray(Visitor *v, void **obj, const char
*name,
void visit_next_carray(Visitor *v, Error **errp);
void visit_end_carray(Visitor *v, Error **errp);
+/* misc. visitors */
+void visit_type_tm(Visitor *m, struct tm *obj, const char *name, Error **errp);
+
#endif
--
1.7.9.5
[Qemu-devel] [PATCH 18/20] qidl: codegen, initial commit
Signed-off-by: Michael Roth
---
scripts/qidl.py | 282 +++
1 file changed, 282 insertions(+)
create mode 100644 scripts/qidl.py
diff --git a/scripts/qidl.py b/scripts/qidl.py
new file mode 100644
index 000..71c89bc
--- /dev/null
+++ b/scripts/qidl.py
@@ -0,0 +1,282 @@
+#
+# QIDL Code Generator
+#
+# Copyright IBM, Corp. 2012
+#
+# Authors:
+# Anthony Liguori
+# Michael Roth
+#
+# This work is licensed under the terms of the GNU GPLv2.
+# See the COPYING file in the top-level directory.
+
+from ordereddict import OrderedDict
+from qidl_parser import parse_file
+from qapi import parse_schema, mcgen, camel_case, de_camel_case
+from qapi_visit import generate_visit_struct, push_indent, pop_indent
+import sys
+import json
+import getopt
+import os
+import errno
+
+def qapi_schema(node):
+schema = OrderedDict()
+data = OrderedDict()
+fields = None
+if node.has_key('typedef'):
+schema['type'] = node['typedef']
+fields = node['type']['fields']
+elif node.has_key('struct'):
+schema['type'] = node['struct']
+fields = node['fields']
+else:
+raise Exception("top-level neither typedef nor struct")
+
+for field in fields:
+if filter(lambda x: field.has_key(x), \
+['is_derived', 'is_immutable', 'is_broken', 'is_function', \
+ 'is_nested_decl', 'is_elsewhere']):
+continue
+
+description = {}
+
+if field['type'].endswith('_t'):
+typename = field['type'][:-2]
+elif field['type'].startswith('struct '):
+typename = field['type'].split(" ")[1]
+elif field['type'].startswith('enum '):
+typename = 'int'
+elif field['type'] == "_Bool":
+typename = 'bool'
+elif field['type'].endswith("char") and field.has_key('is_pointer'):
+typename = 'str';
+elif field['type'] == 'int':
+typename = 'int32'
+elif field['type'] == 'unsigned int':
+typename = 'uint32'
+elif field['type'] == 'char':
+typename = 'uint8'
+else:
+typename = field['type']
+
+if field.has_key('is_array') and field['is_array']:
+description['type'] = [typename]
+description[''] = 'true'
+if field.has_key('array_size'):
+description['array_size'] = field['array_size']
+if field.has_key('array_capacity'):
+description['array_capacity'] = field['array_capacity']
+elif camel_case(de_camel_case(typename)) == typename and \
+(not field.has_key('is_pointer') or not field['is_pointer']):
+description[''] = 'true'
+description[''] = 'true'
+description['type'] = typename
+else:
+description = typename
+
+if field.has_key('is_optional') and field['is_optional']:
+data["*" + field['variable']] = description
+else:
+data[field['variable']] = description
+
+schema['data'] = data
+return schema
+
+
+def parse_schema_file(filename):
+return parse_schema(open(filename, 'r'))
+
+def write_file(output, filename):
+if filename:
+output_file = open(filename, 'w')
+else:
+output_file = sys.stdout
+output_file.write(output)
+if filename:
+output_file.close()
+
+def property_list(node):
+prop_list = []
+fields = None
+if node.has_key('typedef'):
+state = node['typedef']
+fields = node['type']['fields']
+elif node.has_key('struct'):
+state = node['struct']
+fields = node['fields']
+else:
+raise Exception("top-level neither typedef nor struct")
+
+for field in fields:
+if not field.has_key('is_property'):
+continue
+
+for arglist in field['property_fields']:
+if field['variable'] == 'devfn':
+typename = 'pci_devfn'
+elif field['type'].endswith('_t'):
+typename = field['type'][:-2]
+
[Qemu-devel] [PATCH 07/20] qapi: qapi_visit.py, support generating static functions
qidl embeds visitor code into object files rather than linking against
seperate files, so allow for static declarations when we're using
qapi_visit.py as a library as we do with qidl.py
Reviewed-by: Anthony Liguori
Signed-off-by: Michael Roth
---
scripts/qapi_visit.py | 51 -
1 file changed, 33 insertions(+), 18 deletions(-)
diff --git a/scripts/qapi_visit.py b/scripts/qapi_visit.py
index aac2172..d146013 100644
--- a/scripts/qapi_visit.py
+++ b/scripts/qapi_visit.py
@@ -141,13 +141,16 @@ visit_end_optional(m, &err);
''')
return ret
-def generate_visit_struct(name, members):
+def generate_visit_struct(name, members, static=False):
+ret_type = "void"
+if static:
+ret_type = "static " + ret_type
ret = mcgen('''
-void visit_type_%(name)s(Visitor *m, %(name)s ** obj, const char *name, Error
**errp)
+%(ret_type)s visit_type_%(name)s(Visitor *m, %(name)s ** obj, const char
*name, Error **errp)
{
''',
-name=name)
+name=name, ret_type=ret_type)
push_indent()
ret += generate_visit_struct_body("", name, members)
@@ -158,10 +161,13 @@ void visit_type_%(name)s(Visitor *m, %(name)s ** obj,
const char *name, Error **
''')
return ret
-def generate_visit_list(name, members):
+def generate_visit_list(name, members, static=False):
+ret_type = "void"
+if static:
+ret_type = "static " + ret_type
return mcgen('''
-void visit_type_%(name)sList(Visitor *m, %(name)sList ** obj, const char
*name, Error **errp)
+%(ret_type)s visit_type_%(name)sList(Visitor *m, %(name)sList ** obj, const
char *name, Error **errp)
{
GenericList *i, **prev = (GenericList **)obj;
Error *err = NULL;
@@ -183,19 +189,22 @@ void visit_type_%(name)sList(Visitor *m, %(name)sList **
obj, const char *name,
}
}
''',
-name=name)
+name=name, ret_type=ret_type)
-def generate_visit_enum(name, members):
+def generate_visit_enum(name, members, static=False):
+ret_type = "void"
+if static:
+ret_type = "static " + ret_type
return mcgen('''
-void visit_type_%(name)s(Visitor *m, %(name)s * obj, const char *name, Error
**errp)
+%(ret_type)s visit_type_%(name)s(Visitor *m, %(name)s * obj, const char *name,
Error **errp)
{
visit_type_enum(m, (int *)obj, %(name)s_lookup, "%(name)s", name, errp);
}
''',
- name=name)
+ name=name, ret_type=ret_type)
-def generate_visit_union(name, members):
+def generate_visit_union(name, members, static=False):
ret = generate_visit_enum('%sKind' % name, members.keys())
ret += mcgen('''
@@ -252,27 +261,33 @@ void visit_type_%(name)s(Visitor *m, %(name)s ** obj,
const char *name, Error **
return ret
-def generate_declaration(name, members, genlist=True):
+def generate_declaration(name, members, genlist=True, static=False):
+ret_type = "void"
+if static:
+ret_type = "static " + ret_type
ret = mcgen('''
-void visit_type_%(name)s(Visitor *m, %(name)s ** obj, const char *name, Error
**errp);
+%(ret_type)s visit_type_%(name)s(Visitor *m, %(name)s ** obj, const char
*name, Error **errp);
''',
-name=name)
+name=name, ret_type=ret_type)
if genlist:
ret += mcgen('''
-void visit_type_%(name)sList(Visitor *m, %(name)sList ** obj, const char
*name, Error **errp);
+%(ret_type)s visit_type_%(name)sList(Visitor *m, %(name)sList ** obj, const
char *name, Error **errp);
''',
- name=name)
+ name=name, ret_type=ret_type)
return ret
-def generate_decl_enum(name, members, genlist=True):
+def generate_decl_enum(name, members, genlist=True, static=False):
+ret_type = "void"
+if static:
+ret_type = "static " + ret_type
return mcgen('''
-void visit_type_%(name)s(Visitor *m, %(name)s * obj, const char *name, Error
**errp);
+%(ret_type)s visit_type_%(name)s(Visitor *m, %(name)s * obj, const char *name,
Error **errp);
''',
-name=name)
+name=name, ret_type=ret_type)
def main(argv=[]):
try:
--
1.7.9.5
[Qemu-devel] [PATCH 20/20] qidl: unit tests
Signed-off-by: Michael Roth
---
Makefile |2 +
rules.mak | 15 +++-
tests/Makefile |8 +-
tests/test-qidl-included.h | 31
tests/test-qidl-linked.c | 91 +++
tests/test-qidl-linked.h | 18 +
tests/test-qidl.c | 177
7 files changed, 339 insertions(+), 3 deletions(-)
create mode 100644 tests/test-qidl-included.h
create mode 100644 tests/test-qidl-linked.c
create mode 100644 tests/test-qidl-linked.h
create mode 100644 tests/test-qidl.c
diff --git a/Makefile b/Makefile
index 7733e4c..f02251f 100644
--- a/Makefile
+++ b/Makefile
@@ -233,6 +233,7 @@ clean:
if test -d $$d; then $(MAKE) -C $$d $@ || exit 1; fi; \
rm -f $$d/qemu-options.def; \
done
+ find -depth -name qidl-generated -type d -exec rm -rf {} \;
VERSION ?= $(shell cat VERSION)
@@ -403,6 +404,7 @@ qemu-doc.dvi qemu-doc.html qemu-doc.info qemu-doc.pdf: \
# rebuilt before other object files
Makefile: $(GENERATED_HEADERS)
+
# Include automatically generated dependency files
# Dependencies in Makefile.objs files come from our recursive subdir rules
-include $(wildcard *.d tests/*.d)
diff --git a/rules.mak b/rules.mak
index a284946..48e7a95 100644
--- a/rules.mak
+++ b/rules.mak
@@ -15,7 +15,20 @@ MAKEFLAGS += -rR
QEMU_DGFLAGS += -MMD -MP -MT $@ -MF $(*D)/$(*F).d
%.o: %.c
- $(call quiet-command,$(CC) $(QEMU_INCLUDES) $(QEMU_CFLAGS)
$(QEMU_DGFLAGS) $(CFLAGS) -c -o $@ $<," CC$(TARGET_DIR)$@")
+
+%.qidl.c: %.c $(SRC_PATH)/qidl.h $(addprefix $(SRC_PATH)/scripts/,qidl.py
qidl_parser.py qapi.py qapi_visit.py)
+ $(call rm -f $(*D)/qidl-generated/$(*F).qidl.c)
+ $(call quiet-command, $(CC) $(QEMU_INCLUDES) $(QEMU_CFLAGS) $(CFLAGS)
-E -c -DQIDL_GEN $< | \
+ $(PYTHON) $(SRC_PATH)/scripts/qidl.py
--output-filepath=$(*D)/qidl-generated/$(*F).qidl.c || [ "$$?" -eq 2 ])
+
+%.o: %.c %.qidl.c
+ $(if $(strip $(shell test -f $(*D)/qidl-generated/$(*F).qidl.c && echo
"true")), \
+ $(call quiet-command, \
+ $(CC) $(QEMU_INCLUDES) $(QEMU_CFLAGS) $(QEMU_DGFLAGS) $(CFLAGS) -c \
+ -include $< -o $@ $(*D)/qidl-generated/$(*F).qidl.c,"qidl CC
$@"), \
+ $(call quiet-command, \
+ $(CC) $(QEMU_INCLUDES) $(QEMU_CFLAGS) $(QEMU_DGFLAGS) $(CFLAGS) -c \
+ -o $@ $<," CC$@"))
ifeq ($(LIBTOOL),)
%.lo: %.c
diff --git a/tests/Makefile b/tests/Makefile
index ec6a050..a387643 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -15,6 +15,7 @@ check-unit-y += tests/test-string-output-visitor$(EXESUF)
check-unit-y += tests/test-coroutine$(EXESUF)
check-unit-y += tests/test-visitor-serialization$(EXESUF)
check-unit-y += tests/test-iov$(EXESUF)
+check-unit-y += tests/test-qidl$(EXESUF)
check-block-$(CONFIG_POSIX) += tests/qemu-iotests-quick.sh
@@ -34,11 +35,12 @@ test-obj-y = tests/check-qint.o tests/check-qstring.o
tests/check-qdict.o \
tests/test-coroutine.o tests/test-string-output-visitor.o \
tests/test-string-input-visitor.o tests/test-qmp-output-visitor.o \
tests/test-qmp-input-visitor.o tests/test-qmp-input-strict.o \
- tests/test-qmp-commands.o tests/test-visitor-serialization.o
+ tests/test-qmp-commands.o tests/test-visitor-serialization.o \
+ tests/test-qidl.o
test-qapi-obj-y = $(qobject-obj-y) $(qapi-obj-y) $(tools-obj-y)
test-qapi-obj-y += tests/test-qapi-visit.o tests/test-qapi-types.o
-test-qapi-obj-y += module.o
+test-qapi-obj-y += module.o $(qom-obj-y)
$(test-obj-y): QEMU_INCLUDES += -Itests
@@ -84,6 +86,8 @@ check-qtest-$(CONFIG_POSIX)=$(foreach TARGET,$(TARGETS),
$(check-qtest-$(TARGET)
qtest-obj-y = tests/libqtest.o $(oslib-obj-y)
$(check-qtest-y): $(qtest-obj-y)
+tests/test-qidl$(EXESUF): tests/test-qidl.o tests/test-qidl-linked.o
$(test-qapi-obj-y) qapi/misc-qapi-visit.o
+
.PHONY: check-help
check-help:
@echo "Regression testing targets:"
diff --git a/tests/test-qidl-included.h b/tests/test-qidl-included.h
new file mode 100644
index 000..2aae51e
--- /dev/null
+++ b/tests/test-qidl-included.h
@@ -0,0 +1,31 @@
+/*
+ * Unit-tests for QIDL-generated visitors/code
+ *
+ * Copyright IBM, Corp. 2012
+ *
+ * Authors:
+ * Michael Roth
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#ifndef TEST_QIDL_INCLUDED_H
+#define TEST_QIDL_INCLUDED_H
+
+#include "qidl.h"
+
+QIDL_START(TestStructIncluded, state, properties)
+typedef struct TestStructIncluded {
+int32_t a QIDL(immutable);
+int32_t b;
+uint32_t c QIDL(immutable);
+uint32_t d;
+uint64_t e QIDL(immutable);
+uint64_t f QIDL(property, "f", 42);
+char *g QIDL(property, "g");
+char *h QIDL(immutable) QIDL(proper
[Qemu-devel] [PATCH 10/20] qapi: QmpInputVisitor, implement array handling
Signed-off-by: Michael Roth
---
qapi/qmp-input-visitor.c | 32 +++-
1 file changed, 31 insertions(+), 1 deletion(-)
diff --git a/qapi/qmp-input-visitor.c b/qapi/qmp-input-visitor.c
index 107d8d3..c4388f3 100644
--- a/qapi/qmp-input-visitor.c
+++ b/qapi/qmp-input-visitor.c
@@ -132,7 +132,7 @@ static void qmp_input_start_struct(Visitor *v, void **obj,
const char *kind,
return;
}
-if (obj) {
+if (obj && *obj == NULL) {
*obj = g_malloc0(size);
}
}
@@ -274,6 +274,33 @@ static void qmp_input_start_optional(Visitor *v, bool
*present,
*present = true;
}
+static void qmp_input_start_carray(Visitor *v, void **obj, const char *name,
+ size_t elem_count, size_t elem_size,
+ Error **errp)
+{
+if (obj && *obj == NULL) {
+*obj = g_malloc0(elem_count * elem_size);
+}
+qmp_input_start_list(v, name, errp);
+}
+
+static void qmp_input_next_carray(Visitor *v, Error **errp)
+{
+QmpInputVisitor *qiv = to_qiv(v);
+StackObject *so = &qiv->stack[qiv->nb_stack - 1];
+
+if (so->entry == NULL) {
+so->entry = qlist_first(qobject_to_qlist(so->obj));
+} else {
+so->entry = qlist_next(so->entry);
+}
+}
+
+static void qmp_input_end_carray(Visitor *v, Error **errp)
+{
+qmp_input_end_list(v, errp);
+}
+
Visitor *qmp_input_get_visitor(QmpInputVisitor *v)
{
return &v->visitor;
@@ -302,6 +329,9 @@ QmpInputVisitor *qmp_input_visitor_new(QObject *obj)
v->visitor.type_str = qmp_input_type_str;
v->visitor.type_number = qmp_input_type_number;
v->visitor.start_optional = qmp_input_start_optional;
+v->visitor.start_carray = qmp_input_start_carray;
+v->visitor.next_carray = qmp_input_next_carray;
+v->visitor.end_carray = qmp_input_end_carray;
qmp_input_push(v, obj, NULL);
qobject_incref(obj);
--
1.7.9.5
[Qemu-devel] [PATCH 16/20] qidl: Add documentation
Signed-off-by: Michael Roth
---
docs/qidl.txt | 343 +
1 file changed, 343 insertions(+)
create mode 100644 docs/qidl.txt
diff --git a/docs/qidl.txt b/docs/qidl.txt
new file mode 100644
index 000..19976d9
--- /dev/null
+++ b/docs/qidl.txt
@@ -0,0 +1,343 @@
+How to Serialize Device State with QIDL
+==
+
+This document describes how to implement save/restore of a device in QEMU using
+the QIDL compiler. The QIDL compiler makes it easier to support live
+migration in devices by converging the serialization description with the
+device type declaration. It has the following features:
+
+ 1. Single description of device state and how to serialize
+
+ 2. Fully inclusive serialization description--fields that aren't serialized
+are explicitly marked as such including the reason why.
+
+ 3. Optimized for the common case. Even without any special annotations,
+many devices will Just Work out of the box.
+
+ 4. Build time schema definition. Since QIDL runs at build time, we have full
+access to the schema during the build which means we can fail the build if
+the schema breaks.
+
+For the rest, of the document, the following simple device will be used as an
+example.
+
+typedef struct SerialDevice {
+SysBusDevice parent;
+
+uint8_t thr;// transmit holding register
+uint8_t lsr;// line status register
+uint8_t ier;// interrupt enable register
+
+int int_pending;// whether we have a pending queued interrupt
+CharDriverState *chr; // backend
+} SerialDevice;
+
+Getting Started
+---
+
+The first step is to move your device struct definition to a header file. This
+header file should only contain the struct definition and any preprocessor
+declarations you need to define the structure. This header file will act as
+the source for the QIDL compiler.
+
+Do not include any function declarations in this header file as QIDL does not
+understand function declarations.
+
+Determining What State Gets Saved
+-
+
+By default, QIDL saves every field in a structure it sees. This provides
maximum
+correctness by default. However, device structures generally contain state
+that reflects state that is in someway duplicated or not guest visible. This
+more often that not reflects design implementation details.
+
+Since design implementation details change over time, saving this state makes
+compatibility hard to maintain since it would effectively lock down a device's
+implementation.
+
+QIDL allows a device author to suppress certain fields from being saved
although
+there are very strict rules about when this is allowed and what needs to be
done
+to ensure that this does not impact correctness.
+
+There are three cases where state can be suppressed: when it is **immutable**,
+**derived**, or **broken**. In addition, QIDL can decide at run time whether
to
+suppress a field by assigning it a **default** value.
+
+## Immutable Fields
+
+If a field is only set during device construction, based on parameters passed
to
+the device's constructor, then there is no need to send save and restore this
+value. We call these fields immutable and we tell QIDL about this fact by
using
+a **immutable** marker.
+
+In our *SerialDevice* example, the *CharDriverState* pointer reflects the host
+backend that we use to send serial output to the user. This is only assigned
+during device construction and never changes. This means we can add an
+**immutable** marker to it:
+
+QIDL_START(SerialDevice, state)
+typedef struct SerialDevice {
+SysBusDevice parent;
+
+uint8_t thr;// transmit holding register
+uint8_t lsr;// line status register
+uint8_t ier;// interrupt enable register
+
+int int_pending;// whether we have a pending queued interrupt
+CharDriverState *chr QIDL(immutable);
+} SerialDevice;
+QIDL_END(SerialDevice)
+
+When reviewing patches that make use of the **immutable** marker, the following
+guidelines should be followed to determine if the marker is being used
+correctly.
+
+ 1. Check to see if the field is assigned anywhere other than the device
+initialization function.
+
+ 2. Check to see if any function is being called that modifies the state of the
+field outside of the initialization function.
+
+It can be subtle whether a field is truly immutable. A good example is a
+*QEMUTimer*. Timer's will usually have their timeout modified with a call to
+*qemu_mod_timer()* even though they are only assigned in the device
+initialization function.
+
+If the timer is always modified with a fixed value that is not dependent on
+guest state, then the timer is immutable since it's unaffected by the state of
+the gu
[Qemu-devel] [PATCH 01/20] qapi: qapi-visit.py -> qapi_visit.py so we can import
Python doesn't allow "-" in module names, so we need to rename the file
so we can re-use bits of the codegen
Reviewed-by: Anthony Liguori
Signed-off-by: Michael Roth
---
Makefile |8
scripts/{qapi-visit.py => qapi_visit.py} |0
tests/Makefile |4 ++--
3 files changed, 6 insertions(+), 6 deletions(-)
rename scripts/{qapi-visit.py => qapi_visit.py} (100%)
diff --git a/Makefile b/Makefile
index d736ea5..08c7d0e 100644
--- a/Makefile
+++ b/Makefile
@@ -187,8 +187,8 @@ qga/qapi-generated/qga-qapi-types.c
qga/qapi-generated/qga-qapi-types.h :\
$(SRC_PATH)/qapi-schema-guest.json $(SRC_PATH)/scripts/qapi-types.py $(qapi-py)
$(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-types.py
$(gen-out-type) -o qga/qapi-generated -p "qga-" < $<, " GEN $@")
qga/qapi-generated/qga-qapi-visit.c qga/qapi-generated/qga-qapi-visit.h :\
-$(SRC_PATH)/qapi-schema-guest.json $(SRC_PATH)/scripts/qapi-visit.py $(qapi-py)
- $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-visit.py
$(gen-out-type) -o qga/qapi-generated -p "qga-" < $<, " GEN $@")
+$(SRC_PATH)/qapi-schema-guest.json $(SRC_PATH)/scripts/qapi_visit.py $(qapi-py)
+ $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi_visit.py
$(gen-out-type) -o qga/qapi-generated -p "qga-" < $<, " GEN $@")
qga/qapi-generated/qga-qmp-commands.h qga/qapi-generated/qga-qmp-marshal.c :\
$(SRC_PATH)/qapi-schema-guest.json $(SRC_PATH)/scripts/qapi-commands.py
$(qapi-py)
$(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-commands.py
$(gen-out-type) -o qga/qapi-generated -p "qga-" < $<, " GEN $@")
@@ -197,8 +197,8 @@ qapi-types.c qapi-types.h :\
$(SRC_PATH)/qapi-schema.json $(SRC_PATH)/scripts/qapi-types.py $(qapi-py)
$(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-types.py
$(gen-out-type) -o "." < $<, " GEN $@")
qapi-visit.c qapi-visit.h :\
-$(SRC_PATH)/qapi-schema.json $(SRC_PATH)/scripts/qapi-visit.py $(qapi-py)
- $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-visit.py
$(gen-out-type) -o "." < $<, " GEN $@")
+$(SRC_PATH)/qapi-schema.json $(SRC_PATH)/scripts/qapi_visit.py $(qapi-py)
+ $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi_visit.py
$(gen-out-type) -o "." < $<, " GEN $@")
qmp-commands.h qmp-marshal.c :\
$(SRC_PATH)/qapi-schema.json $(SRC_PATH)/scripts/qapi-commands.py $(qapi-py)
$(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-commands.py
$(gen-out-type) -m -o "." < $<, " GEN $@")
diff --git a/scripts/qapi-visit.py b/scripts/qapi_visit.py
similarity index 100%
rename from scripts/qapi-visit.py
rename to scripts/qapi_visit.py
diff --git a/tests/Makefile b/tests/Makefile
index f3f4159..3c05bdf 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -55,8 +55,8 @@ tests/test-qapi-types.c tests/test-qapi-types.h :\
$(SRC_PATH)/qapi-schema-test.json $(SRC_PATH)/scripts/qapi-types.py
$(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-types.py
$(gen-out-type) -o tests -p "test-" < $<, " GEN $@")
tests/test-qapi-visit.c tests/test-qapi-visit.h :\
-$(SRC_PATH)/qapi-schema-test.json $(SRC_PATH)/scripts/qapi-visit.py
- $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-visit.py
$(gen-out-type) -o tests -p "test-" < $<, " GEN $@")
+$(SRC_PATH)/qapi-schema-test.json $(SRC_PATH)/scripts/qapi_visit.py
+ $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi_visit.py
$(gen-out-type) -o tests -p "test-" < $<, " GEN $@")
tests/test-qmp-commands.h tests/test-qmp-marshal.c :\
$(SRC_PATH)/qapi-schema-test.json $(SRC_PATH)/scripts/qapi-commands.py
$(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-commands.py
$(gen-out-type) -o tests -p "test-" < $<, " GEN $@")
--
1.7.9.5
[Qemu-devel] [PATCH 08/20] qapi: qapi_visit.py, support for visiting non-pointer/embedded structs
Reviewed-by: Anthony Liguori
Signed-off-by: Michael Roth
---
scripts/qapi_visit.py |9 +
1 file changed, 9 insertions(+)
diff --git a/scripts/qapi_visit.py b/scripts/qapi_visit.py
index d146013..ab44f11 100644
--- a/scripts/qapi_visit.py
+++ b/scripts/qapi_visit.py
@@ -107,6 +107,15 @@ if (obj && (*obj)->%(prefix)shas_%(c_name)s) {
if annotated:
if isinstance(argentry['type'], types.ListType):
ret += generate_visit_carray_body(argname, argentry)
+elif argentry.has_key(''):
+tmp_ptr_name = "%s_%s_ptr" %
(c_var(field_prefix).replace(".", ""), c_var(argname))
+ret += mcgen('''
+%(type)s *%(tmp_ptr)s = &(*obj)->%(c_prefix)s%(c_name)s;
+visit_type_%(type)s(m, (obj && *obj) ? &%(tmp_ptr)s : NULL, "%(name)s", errp);
+''',
+ c_prefix=c_var(field_prefix),
prefix=field_prefix,
+ type=type_name(argentry['type']),
c_name=c_var(argname),
+ name=argname, tmp_ptr=tmp_ptr_name)
else:
ret += mcgen('''
visit_type_%(type)s(m, obj ? &(*obj)->%(c_prefix)s%(c_name)s : NULL,
"%(name)s", errp);
--
1.7.9.5
[Qemu-devel] [PATCH 02/20] qapi: qapi-types.py -> qapi_types.py
Python doesn't allow "-" in module names, so we need to rename the file
so we can re-use bits of the codegen
Signed-off-by: Michael Roth
---
Makefile |8
scripts/{qapi-types.py => qapi_types.py} |0
tests/Makefile |4 ++--
3 files changed, 6 insertions(+), 6 deletions(-)
rename scripts/{qapi-types.py => qapi_types.py} (100%)
diff --git a/Makefile b/Makefile
index 08c7d0e..e92ea6b 100644
--- a/Makefile
+++ b/Makefile
@@ -184,8 +184,8 @@ endif
qapi-py = $(SRC_PATH)/scripts/qapi.py $(SRC_PATH)/scripts/ordereddict.py
qga/qapi-generated/qga-qapi-types.c qga/qapi-generated/qga-qapi-types.h :\
-$(SRC_PATH)/qapi-schema-guest.json $(SRC_PATH)/scripts/qapi-types.py $(qapi-py)
- $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-types.py
$(gen-out-type) -o qga/qapi-generated -p "qga-" < $<, " GEN $@")
+$(SRC_PATH)/qapi-schema-guest.json $(SRC_PATH)/scripts/qapi_types.py $(qapi-py)
+ $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi_types.py
$(gen-out-type) -o qga/qapi-generated -p "qga-" < $<, " GEN $@")
qga/qapi-generated/qga-qapi-visit.c qga/qapi-generated/qga-qapi-visit.h :\
$(SRC_PATH)/qapi-schema-guest.json $(SRC_PATH)/scripts/qapi_visit.py $(qapi-py)
$(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi_visit.py
$(gen-out-type) -o qga/qapi-generated -p "qga-" < $<, " GEN $@")
@@ -194,8 +194,8 @@ $(SRC_PATH)/qapi-schema-guest.json
$(SRC_PATH)/scripts/qapi-commands.py $(qapi-p
$(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-commands.py
$(gen-out-type) -o qga/qapi-generated -p "qga-" < $<, " GEN $@")
qapi-types.c qapi-types.h :\
-$(SRC_PATH)/qapi-schema.json $(SRC_PATH)/scripts/qapi-types.py $(qapi-py)
- $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-types.py
$(gen-out-type) -o "." < $<, " GEN $@")
+$(SRC_PATH)/qapi-schema.json $(SRC_PATH)/scripts/qapi_types.py $(qapi-py)
+ $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi_types.py
$(gen-out-type) -o "." < $<, " GEN $@")
qapi-visit.c qapi-visit.h :\
$(SRC_PATH)/qapi-schema.json $(SRC_PATH)/scripts/qapi_visit.py $(qapi-py)
$(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi_visit.py
$(gen-out-type) -o "." < $<, " GEN $@")
diff --git a/scripts/qapi-types.py b/scripts/qapi_types.py
similarity index 100%
rename from scripts/qapi-types.py
rename to scripts/qapi_types.py
diff --git a/tests/Makefile b/tests/Makefile
index 3c05bdf..156105c 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -52,8 +52,8 @@ tests/test-coroutine$(EXESUF): tests/test-coroutine.o
$(coroutine-obj-y) $(tools
tests/test-iov$(EXESUF): tests/test-iov.o iov.o
tests/test-qapi-types.c tests/test-qapi-types.h :\
-$(SRC_PATH)/qapi-schema-test.json $(SRC_PATH)/scripts/qapi-types.py
- $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-types.py
$(gen-out-type) -o tests -p "test-" < $<, " GEN $@")
+$(SRC_PATH)/qapi-schema-test.json $(SRC_PATH)/scripts/qapi_types.py
+ $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi_types.py
$(gen-out-type) -o tests -p "test-" < $<, " GEN $@")
tests/test-qapi-visit.c tests/test-qapi-visit.h :\
$(SRC_PATH)/qapi-schema-test.json $(SRC_PATH)/scripts/qapi_visit.py
$(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi_visit.py
$(gen-out-type) -o tests -p "test-" < $<, " GEN $@")
--
1.7.9.5
[Qemu-devel] [PATCH 03/20] qapi: qapi-commands.py -> qapi_commands.py
Python doesn't allow "-" in module names, so we need to rename the file
so we can re-use bits of the codegen.
Signed-off-by: Michael Roth
---
Makefile |8
scripts/{qapi-commands.py => qapi_commands.py} |0
tests/Makefile |4 ++--
3 files changed, 6 insertions(+), 6 deletions(-)
rename scripts/{qapi-commands.py => qapi_commands.py} (100%)
diff --git a/Makefile b/Makefile
index e92ea6b..7733e4c 100644
--- a/Makefile
+++ b/Makefile
@@ -190,8 +190,8 @@ qga/qapi-generated/qga-qapi-visit.c
qga/qapi-generated/qga-qapi-visit.h :\
$(SRC_PATH)/qapi-schema-guest.json $(SRC_PATH)/scripts/qapi_visit.py $(qapi-py)
$(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi_visit.py
$(gen-out-type) -o qga/qapi-generated -p "qga-" < $<, " GEN $@")
qga/qapi-generated/qga-qmp-commands.h qga/qapi-generated/qga-qmp-marshal.c :\
-$(SRC_PATH)/qapi-schema-guest.json $(SRC_PATH)/scripts/qapi-commands.py
$(qapi-py)
- $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-commands.py
$(gen-out-type) -o qga/qapi-generated -p "qga-" < $<, " GEN $@")
+$(SRC_PATH)/qapi-schema-guest.json $(SRC_PATH)/scripts/qapi_commands.py
$(qapi-py)
+ $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi_commands.py
$(gen-out-type) -o qga/qapi-generated -p "qga-" < $<, " GEN $@")
qapi-types.c qapi-types.h :\
$(SRC_PATH)/qapi-schema.json $(SRC_PATH)/scripts/qapi_types.py $(qapi-py)
@@ -200,8 +200,8 @@ qapi-visit.c qapi-visit.h :\
$(SRC_PATH)/qapi-schema.json $(SRC_PATH)/scripts/qapi_visit.py $(qapi-py)
$(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi_visit.py
$(gen-out-type) -o "." < $<, " GEN $@")
qmp-commands.h qmp-marshal.c :\
-$(SRC_PATH)/qapi-schema.json $(SRC_PATH)/scripts/qapi-commands.py $(qapi-py)
- $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-commands.py
$(gen-out-type) -m -o "." < $<, " GEN $@")
+$(SRC_PATH)/qapi-schema.json $(SRC_PATH)/scripts/qapi_commands.py $(qapi-py)
+ $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi_commands.py
$(gen-out-type) -m -o "." < $<, " GEN $@")
QGALIB_GEN=$(addprefix qga/qapi-generated/, qga-qapi-types.h qga-qapi-visit.h
qga-qmp-commands.h)
$(qga-obj-y) qemu-ga.o: $(QGALIB_GEN)
diff --git a/scripts/qapi-commands.py b/scripts/qapi_commands.py
similarity index 100%
rename from scripts/qapi-commands.py
rename to scripts/qapi_commands.py
diff --git a/tests/Makefile b/tests/Makefile
index 156105c..ec6a050 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -58,8 +58,8 @@ tests/test-qapi-visit.c tests/test-qapi-visit.h :\
$(SRC_PATH)/qapi-schema-test.json $(SRC_PATH)/scripts/qapi_visit.py
$(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi_visit.py
$(gen-out-type) -o tests -p "test-" < $<, " GEN $@")
tests/test-qmp-commands.h tests/test-qmp-marshal.c :\
-$(SRC_PATH)/qapi-schema-test.json $(SRC_PATH)/scripts/qapi-commands.py
- $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-commands.py
$(gen-out-type) -o tests -p "test-" < $<, " GEN $@")
+$(SRC_PATH)/qapi-schema-test.json $(SRC_PATH)/scripts/qapi_commands.py
+ $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi_commands.py
$(gen-out-type) -o tests -p "test-" < $<, " GEN $@")
tests/test-string-output-visitor$(EXESUF): tests/test-string-output-visitor.o
$(test-qapi-obj-y)
--
1.7.9.5
[Qemu-devel] [PATCH 14/20] module additions for schema registration
Reviewed-by: Anthony Liguori
Signed-off-by: Michael Roth
---
module.h |2 ++
vl.c |1 +
2 files changed, 3 insertions(+)
diff --git a/module.h b/module.h
index c4ccd57..cb81aa2 100644
--- a/module.h
+++ b/module.h
@@ -25,6 +25,7 @@ typedef enum {
MODULE_INIT_MACHINE,
MODULE_INIT_QAPI,
MODULE_INIT_QOM,
+MODULE_INIT_QIDL,
MODULE_INIT_MAX
} module_init_type;
@@ -32,6 +33,7 @@ typedef enum {
#define machine_init(function) module_init(function, MODULE_INIT_MACHINE)
#define qapi_init(function) module_init(function, MODULE_INIT_QAPI)
#define type_init(function) module_init(function, MODULE_INIT_QOM)
+#define qidl_init(function) module_init(function, MODULE_INIT_QIDL)
void register_module_init(void (*fn)(void), module_init_type type);
diff --git a/vl.c b/vl.c
index d01256a..7895e9d 100644
--- a/vl.c
+++ b/vl.c
@@ -2358,6 +2358,7 @@ int main(int argc, char **argv, char **envp)
}
module_call_init(MODULE_INIT_QOM);
+module_call_init(MODULE_INIT_QIDL);
runstate_init();
--
1.7.9.5
[Qemu-devel] [PATCH 15/20] qdev: move Property-related declarations to qdev-properties.h
Signed-off-by: Michael Roth
---
hw/qdev-properties.h | 151 ++
hw/qdev.h| 126 +
2 files changed, 152 insertions(+), 125 deletions(-)
create mode 100644 hw/qdev-properties.h
diff --git a/hw/qdev-properties.h b/hw/qdev-properties.h
new file mode 100644
index 000..ce79a79
--- /dev/null
+++ b/hw/qdev-properties.h
@@ -0,0 +1,151 @@
+/*
+ * Property definitions for qdev
+ *
+ * Copyright (c) 2009 CodeSourcery
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see <http://www.gnu.org/licenses/>.
+ */
+
+#ifndef QDEV_PROPERTIES_H
+#define QDEV_PROPERTIES_H
+
+#include "qemu/object.h"
+#include "qemu-queue.h"
+
+typedef struct Property Property;
+
+typedef struct PropertyInfo PropertyInfo;
+
+typedef struct CompatProperty CompatProperty;
+
+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;
+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;
+};
+
+typedef struct GlobalProperty {
+const char *driver;
+const char *property;
+const char *value;
+QTAILQ_ENTRY(GlobalProperty) next;
+} GlobalProperty;
+
+extern PropertyInfo qdev_prop_bit;
+extern PropertyInfo qdev_prop_uint8;
+extern PropertyInfo qdev_prop_uint16;
+extern PropertyInfo qdev_prop_uint32;
+extern PropertyInfo qdev_prop_int32;
+extern PropertyInfo qdev_prop_uint64;
+extern PropertyInfo qdev_prop_hex8;
+extern PropertyInfo qdev_prop_hex32;
+extern PropertyInfo qdev_prop_hex64;
+extern PropertyInfo qdev_prop_string;
+extern PropertyInfo qdev_prop_chr;
+extern PropertyInfo qdev_prop_ptr;
+extern PropertyInfo qdev_prop_macaddr;
+extern PropertyInfo qdev_prop_losttickpolicy;
+extern PropertyInfo qdev_prop_bios_chs_trans;
+extern PropertyInfo qdev_prop_drive;
+extern PropertyInfo qdev_prop_netdev;
+extern PropertyInfo qdev_prop_vlan;
+extern PropertyInfo qdev_prop_pci_devfn;
+extern PropertyInfo qdev_prop_blocksize;
+extern PropertyInfo qdev_prop_pci_host_devaddr;
+
+#define DEFINE_PROP(_name, _state, _field, _prop, _type) { \
+.name = (_name),\
+.info = &(_prop), \
+.offset= offsetof(_state, _field)\
++ type_check(_type,typeof_field(_state, _field)),\
+}
+#define DEFINE_PROP_DEFAULT(_name, _state, _field, _defval, _prop, _type) { \
+.name = (_name), \
+.info = &(_prop), \
+.offset= offsetof(_state, _field) \
++ type_check(_type,typeof_field(_state, _field)), \
+.qtype = QTYPE_QINT,\
+.defval= (_type)_defval,\
+}
+#define DEFINE_PROP_BIT(_name, _state, _field, _bit, _defval) { \
+.name = (_name),\
+.info = &(qdev_prop_bit), \
+.bitnr= (_bit), \
+.offset= offsetof(_state, _field)\
++ type_check(uint32_t,typeof_field(_state, _field)), \
+.qtype = QTYPE_QBOOL,\
+.defval= (bool)_defval, \
+}
+
+#define DEFINE_PROP_UINT8(_n, _s, _f, _d) \
+DEFINE_PROP_DEFAULT(_n, _s, _f, _d, qdev_prop_uint8, uint8_t)
+#define DEFINE_PROP_UINT16(_n, _s, _f, _d) \
+DEFINE_PROP_DEFAULT(_n, _s, _f, _d, qdev_prop_uint16, uint16_t)
+#define DEFINE_PROP_UINT32(_n, _s, _f, _d) \
+DEFINE_PROP_DEFAULT(_n, _s, _f, _d, qdev_prop_uint32, uint32_t)
+#define DEFINE_PROP_INT32(_n, _s, _f, _d) \
+DEFINE_PROP_DEFAULT(_n, _s, _f,
[Qemu-devel] [PATCH 06/20] qapi: add visitor interfaces for C arrays
Generally these will be serialized into lists, but the
representation can be of any form so long as it can
be deserialized into a single-dimension C array.
Signed-off-by: Michael Roth
---
qapi/qapi-visit-core.c | 25 +
qapi/qapi-visit-core.h |8
2 files changed, 33 insertions(+)
diff --git a/qapi/qapi-visit-core.c b/qapi/qapi-visit-core.c
index 7a82b63..9a74ed0 100644
--- a/qapi/qapi-visit-core.c
+++ b/qapi/qapi-visit-core.c
@@ -311,3 +311,28 @@ void input_type_enum(Visitor *v, int *obj, const char
*strings[],
g_free(enum_str);
*obj = value;
}
+
+void visit_start_carray(Visitor *v, void **obj, const char *name,
+size_t elem_count, size_t elem_size, Error **errp)
+{
+g_assert(v->start_carray);
+if (!error_is_set(errp)) {
+v->start_carray(v, obj, name, elem_count, elem_size, errp);
+}
+}
+
+void visit_next_carray(Visitor *v, Error **errp)
+{
+g_assert(v->next_carray);
+if (!error_is_set(errp)) {
+v->next_carray(v, errp);
+}
+}
+
+void visit_end_carray(Visitor *v, Error **errp)
+{
+g_assert(v->end_carray);
+if (!error_is_set(errp)) {
+v->end_carray(v, errp);
+}
+}
diff --git a/qapi/qapi-visit-core.h b/qapi/qapi-visit-core.h
index 60aceda..5eb1616 100644
--- a/qapi/qapi-visit-core.h
+++ b/qapi/qapi-visit-core.h
@@ -43,6 +43,10 @@ struct Visitor
void (*type_str)(Visitor *v, char **obj, const char *name, Error **errp);
void (*type_number)(Visitor *v, double *obj, const char *name,
Error **errp);
+void (*start_carray)(Visitor *v, void **obj, const char *name,
+size_t elem_count, size_t elem_size, Error **errp);
+void (*next_carray)(Visitor *v, Error **errp);
+void (*end_carray)(Visitor *v, Error **errp);
/* May be NULL */
void (*start_optional)(Visitor *v, bool *present, const char *name,
@@ -91,5 +95,9 @@ void visit_type_size(Visitor *v, uint64_t *obj, const char
*name, Error **errp);
void visit_type_bool(Visitor *v, bool *obj, const char *name, Error **errp);
void visit_type_str(Visitor *v, char **obj, const char *name, Error **errp);
void visit_type_number(Visitor *v, double *obj, const char *name, Error
**errp);
+void visit_start_carray(Visitor *v, void **obj, const char *name,
+ size_t elem_count, size_t elem_size, Error **errp);
+void visit_next_carray(Visitor *v, Error **errp);
+void visit_end_carray(Visitor *v, Error **errp);
#endif
--
1.7.9.5
[Qemu-devel] [PATCH 09/20] qapi: QmpOutputVisitor, implement array handling
Signed-off-by: Michael Roth
---
qapi/qmp-output-visitor.c | 20
1 file changed, 20 insertions(+)
diff --git a/qapi/qmp-output-visitor.c b/qapi/qmp-output-visitor.c
index 2bce9d5..ea08795 100644
--- a/qapi/qmp-output-visitor.c
+++ b/qapi/qmp-output-visitor.c
@@ -181,6 +181,23 @@ static void qmp_output_type_number(Visitor *v, double
*obj, const char *name,
qmp_output_add(qov, name, qfloat_from_double(*obj));
}
+static void qmp_output_start_carray(Visitor *v, void **obj, const char *name,
+size_t elem_count, size_t elem_size,
+Error **errp)
+{
+qmp_output_start_list(v, name, errp);
+}
+
+static void qmp_output_next_carray(Visitor *v, Error **errp)
+{
+}
+
+static void qmp_output_end_carray(Visitor *v, Error **errp)
+{
+QmpOutputVisitor *qov = to_qov(v);
+qmp_output_pop(qov);
+}
+
QObject *qmp_output_get_qobject(QmpOutputVisitor *qov)
{
QObject *obj = qmp_output_first(qov);
@@ -228,6 +245,9 @@ QmpOutputVisitor *qmp_output_visitor_new(void)
v->visitor.type_bool = qmp_output_type_bool;
v->visitor.type_str = qmp_output_type_str;
v->visitor.type_number = qmp_output_type_number;
+v->visitor.start_carray = qmp_output_start_carray;
+v->visitor.next_carray = qmp_output_next_carray;
+v->visitor.end_carray = qmp_output_end_carray;
QTAILQ_INIT(&v->stack);
--
1.7.9.5
Re: [Qemu-devel] [PATCH 16/20] qidl: Add documentation
On Tue, Aug 14, 2012 at 08:41:56PM +0100, Peter Maydell wrote:
> On 14 August 2012 17:27, Michael Roth wrote:
> >
> > Signed-off-by: Michael Roth
> > ---
> > docs/qidl.txt | 343
> > +
> > 1 file changed, 343 insertions(+)
> > create mode 100644 docs/qidl.txt
> >
> > diff --git a/docs/qidl.txt b/docs/qidl.txt
> > new file mode 100644
> > index 000..19976d9
> > --- /dev/null
> > +++ b/docs/qidl.txt
> > @@ -0,0 +1,343 @@
> > +How to Serialize Device State with QIDL
> > +==
> > +
> > +This document describes how to implement save/restore of a device in QEMU
> > using
> > +the QIDL compiler. The QIDL compiler makes it easier to support live
> > +migration in devices by converging the serialization description with the
> > +device type declaration. It has the following features:
> > +
> > + 1. Single description of device state and how to serialize
> > +
> > + 2. Fully inclusive serialization description--fields that aren't
> > serialized
> > +are explicitly marked as such including the reason why.
> > +
> > + 3. Optimized for the common case. Even without any special annotations,
> > +many devices will Just Work out of the box.
> > +
> > + 4. Build time schema definition. Since QIDL runs at build time, we have
> > full
> > +access to the schema during the build which means we can fail the
> > build if
> > +the schema breaks.
> > +
> > +For the rest, of the document, the following simple device will be used as
> > an
> > +example.
> > +
> > +typedef struct SerialDevice {
> > +SysBusDevice parent;
> > +
> > +uint8_t thr;// transmit holding register
> > +uint8_t lsr;// line status register
> > +uint8_t ier;// interrupt enable register
> > +
> > +int int_pending;// whether we have a pending queued
> > interrupt
> > +CharDriverState *chr; // backend
> > +} SerialDevice;
>
> "//" style comments are a breach of the QEMU coding style so we shouldn't
> be using them in documentation examples...
>
> > +
> > +In our *SerialDevice* example, the *CharDriverState* pointer reflects the
> > host
> > +backend that we use to send serial output to the user. This is only
> > assigned
> > +during device construction and never changes. This means we can add an
> > +**immutable** marker to it:
> > +
> > +QIDL_START(SerialDevice, state)
> > +typedef struct SerialDevice {
> > +SysBusDevice parent;
> > +
> > +uint8_t thr;// transmit holding register
> > +uint8_t lsr;// line status register
> > +uint8_t ier;// interrupt enable register
> > +
> > +int int_pending;// whether we have a pending queued
> > interrupt
> > +CharDriverState *chr QIDL(immutable);
> > +} SerialDevice;
> > +QIDL_END(SerialDevice)
>
> I think it would be nicer to have a QIDL input format from which the structure
> is generated as one of the outputs; that would avoid having to have some of
> this ugly QIDL() markup.
Some kind of inline/embedded input format, or external (like QAPI
schemas)?
In terms of the ugliness of things, Anthony suggested we re-introduce
the simpler form of the annotations by doing something like:
#define _immutable QIDL(immutable)
as syntactic sugar for the more commonly used annotations/macros. This
can all be done in qidl.h, transparently to the parser (thanks to the
fact that we pass the code through the preprocessor prior to parsing).
There's also a few things we can do to replace the QIDL_START/QIDL_END
pairs with a single indicator.
>
> We could also use this to autogenerate a lot of the QOM required boilerplate,
> qdev Property arrays, etc etc.
QIDL handles properties, and could possibly be used to generate some QOM
boilerplate. We can get as creative as need be really, it's just that
properties/serialization were the primary/initial use-cases.
>
> -- PMM
>
Re: [Qemu-devel] [PATCH] savevm.c: Fix compilation error on 32bit host.
On 15.08.2012 13:10, Evgeny Voevodin wrote:
> Casting of 0x0101010101010101ULL to long will truncate it to 32
> bits on 32bit hosts, and won't truncate on 64bit hosts.
>
> Signed-off-by: Evgeny Voevodin
> ---
> savevm.c |2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/savevm.c b/savevm.c
> index 0ea10c9..9ab4d83 100644
> --- a/savevm.c
> +++ b/savevm.c
> @@ -2473,7 +2473,7 @@ int xbzrle_encode_buffer(uint8_t *old_buf, uint8_t
> *new_buf, int slen,
> /* word at a time for speed, use of 32-bit long okay */
> if (!res) {
> /* truncation to 32-bit long okay */
> -long mask = 0x0101010101010101ULL;
> +long mask = (long)0x0101010101010101ULL;
> while (i < slen) {
> xor = *(long *)(old_buf + i) ^ *(long *)(new_buf + i);
> if ((xor - mask) & ~xor & (mask << 7)) {
Um, how it is ugly... Can't we use unsigned types for all this stuff?
Including slen too - the function parameter...
Thanks,
/mjt
Re: [Qemu-devel] [PATCH v2 0/2] Fix two bugs related to ram_size
On 15.08.2012 13:31, Markus Armbruster wrote: > There are more, but let's start with these two. Yes, there are more of the same theme. In particular, why transparent huge pages support has never been applied? IIRC it went to nowhere after a question about memory sizing and alignment popped up -- see for example this thread: http://lists.gnu.org/archive/html/qemu-devel/2010-03/msg01250.html It has nothing to do with the patchset, but... Thanks, /mjt
Re: [Qemu-devel] [PATCH v2 0/2] Fix two bugs related to ram_size
On 15.08.2012 15:46, Avi Kivity wrote: > On 08/15/2012 02:44 PM, Michael Tokarev wrote: >> Yes, there are more of the same theme. In particular, >> why transparent huge pages support has never been >> applied? IIRC it went to nowhere after a question >> about memory sizing and alignment popped up -- see >> for example this thread: >> >> http://lists.gnu.org/archive/html/qemu-devel/2010-03/msg01250.html > > See 36b586284e678d. Oh. I missed that one. Thank you Avi! Now I wonder why there's so many questions about THP not being used with qemu-kvm. I just fired up a random guest with 1Gb memory -- AnonHugePages in /proc/meminfo is still 0. Hum... But this is a different topic now. Thanks! /mjt
[Qemu-devel] qemu and transparent huge pages
Quite some time ago there was a thread on qemu-devel,
started by Andrea, about modifying qemu to better
use transparent huge pages:
http://lists.gnu.org/archive/html/qemu-devel/2010-03/msg01250.html
That thread hasn't reached any conclusion, but some time
after that Avi implemented a similar change:
commit 36b586284e678da28df3af9fd0907d2b16f9311c
Author: Avi Kivity
Date: Mon Sep 5 11:07:05 2011 +0300
qemu_vmalloc: align properly for transparent hugepages and KVM
To make good use of transparent hugepages, KVM requires that guest-physical
and host-virtual addresses share the low 21 bits (as opposed to just the low
12 bits normally required).
Adjust qemu_vmalloc() to honor that requirement. Ignore it for small region
to avoid fragmentation.
Signed-off-by: Avi Kivity
Signed-off-by: Anthony Liguori
diff --git a/oslib-posix.c b/oslib-posix.c
index 196099c..a304fb0 100644
--- a/oslib-posix.c
+++ b/oslib-posix.c
@@ -35,6 +35,13 @@
extern int daemon(int, int);
#endif
+#if defined(__linux__) && defined(__x86_64__)
+ /* Use 2MB alignment so transparent hugepages can be used by KVM */
+# define QEMU_VMALLOC_ALIGN (512 * 4096)
+#else
+# define QEMU_VMALLOC_ALIGN getpagesize()
+#endif
+
#include "config-host.h"
#include "sysemu.h"
#include "trace.h"
@@ -80,7 +87,12 @@ void *qemu_memalign(size_t alignment, size_t size)
void *qemu_vmalloc(size_t size)
{
void *ptr;
-ptr = qemu_memalign(getpagesize(), size);
+size_t align = QEMU_VMALLOC_ALIGN;
+
+if (size < align) {
+align = getpagesize();
+}
+ptr = qemu_memalign(align, size);
trace_qemu_vmalloc(size, ptr);
return ptr;
}
(why it is 64bit-only is a different, unrelated question).
But apparently, THP does not work still, even with 2Mb
alignment: when running a guest, AnonHugePages in
/proc/meminfo stays at 0 - either in kvm mode or in tcg
mode. Any idea why? What else is needed for THP to work?
This is quite a frequent question in #kvm IRC channel,
and I always suggested using -mem-path for this, but
I'm curios why it doesn't work automatically when it
probably should?
Thanks,
/mjt
[Qemu-devel] qemu and transparent huge pages
[Reposting with the right email address of Andrea]
Quite some time ago there was a thread on qemu-devel,
started by Andrea, about modifying qemu to better
use transparent huge pages:
http://lists.gnu.org/archive/html/qemu-devel/2010-03/msg01250.html
That thread hasn't reached any conclusion, but some time
after that Avi implemented a similar change:
commit 36b586284e678da28df3af9fd0907d2b16f9311c
Author: Avi Kivity
Date: Mon Sep 5 11:07:05 2011 +0300
qemu_vmalloc: align properly for transparent hugepages and KVM
To make good use of transparent hugepages, KVM requires that guest-physical
and host-virtual addresses share the low 21 bits (as opposed to just the low
12 bits normally required).
Adjust qemu_vmalloc() to honor that requirement. Ignore it for small region
to avoid fragmentation.
Signed-off-by: Avi Kivity
Signed-off-by: Anthony Liguori
diff --git a/oslib-posix.c b/oslib-posix.c
index 196099c..a304fb0 100644
--- a/oslib-posix.c
+++ b/oslib-posix.c
@@ -35,6 +35,13 @@
extern int daemon(int, int);
#endif
+#if defined(__linux__) && defined(__x86_64__)
+ /* Use 2MB alignment so transparent hugepages can be used by KVM */
+# define QEMU_VMALLOC_ALIGN (512 * 4096)
+#else
+# define QEMU_VMALLOC_ALIGN getpagesize()
+#endif
+
#include "config-host.h"
#include "sysemu.h"
#include "trace.h"
@@ -80,7 +87,12 @@ void *qemu_memalign(size_t alignment, size_t size)
void *qemu_vmalloc(size_t size)
{
void *ptr;
-ptr = qemu_memalign(getpagesize(), size);
+size_t align = QEMU_VMALLOC_ALIGN;
+
+if (size < align) {
+align = getpagesize();
+}
+ptr = qemu_memalign(align, size);
trace_qemu_vmalloc(size, ptr);
return ptr;
}
(why it is 64bit-only is a different, unrelated question).
But apparently, THP does not work still, even with 2Mb
alignment: when running a guest, AnonHugePages in
/proc/meminfo stays at 0 - either in kvm mode or in tcg
mode. Any idea why? What else is needed for THP to work?
This is quite a frequent question in #kvm IRC channel,
and I always suggested using -mem-path for this, but
I'm curios why it doesn't work automatically when it
probably should?
Thanks,
/mjt
Re: [Qemu-devel] qemu and transparent huge pages
On 15.08.2012 16:51, Avi Kivity wrote: > On 08/15/2012 03:45 PM, Michael Tokarev wrote: >> >> But apparently, THP does not work still, even with 2Mb >> alignment: when running a guest, AnonHugePages in >> /proc/meminfo stays at 0 - either in kvm mode or in tcg >> mode. Any idea why? What else is needed for THP to work? > > It does for me: > > AnonHugePages:368640 kB > > Note the patch you reference doesn't impact thp, just kvm's ability to > propagate them to the shadow page table. > >> >> This is quite a frequent question in #kvm IRC channel, >> and I always suggested using -mem-path for this, but >> I'm curios why it doesn't work automatically when it >> probably should? >> > > Please provide extra info, like the setting of > /sys/kernel/mm/transparent_hugepage/enabled. That was it - sort of. Default value here is enabled=madvise. When setting it to always the effect finally started appearing, so it is actually working. But can't qemu set MADV_HUGEPAGE flag too, so it works automatically? Thanks, /mjt
Re: [Qemu-devel] qemu and transparent huge pages
On 15.08.2012 18:26, Avi Kivity wrote:
> On 08/15/2012 05:22 PM, Michael Tokarev wrote:
>
>>>
>>> Please provide extra info, like the setting of
>>> /sys/kernel/mm/transparent_hugepage/enabled.
>>
>> That was it - sort of. Default value here is enabled=madvise.
>> When setting it to always the effect finally started appearing,
>> so it is actually working.
>>
>> But can't qemu set MADV_HUGEPAGE flag too, so it works automatically?
>
> It can and should.
Something like the attached patch?
Thanks,
/mjt
>From 705b3efb8c0cf06cbf087204fc61863c2bbb9e27 Mon Sep 17 00:00:00 2001
From: Michael Tokarev
Date: Wed, 15 Aug 2012 18:55:16 +0400
Subject: [PATCH] mark large vmalloc areas as MADV_HUGEPAGE and allow
hugepages on i386
A followup to commit 36b586284e678d.
On linux only (which supports transparent hugepages), explicitly mark
large vmalloced areas with madvise(MADV_HUGEPAGES). The patch changes
previous logic a bit to allow inserting the call to madvise(), but keeps
the code the same (and saves one call to getpagesize() per allocation).
The code also adds #include to the linux-specific part,
to get MADV_HUGEPAGES definition.
While at it, enable transparent hugepages (alignment and the new
explicit marking with madvise()) for 32bit x86 too - it makes good
sense for, say, 32bit userspace on 64bit kernel.
Signed-off-by: Michael Tokarev
---
oslib-posix.c | 35 +--
1 file changed, 25 insertions(+), 10 deletions(-)
diff --git a/oslib-posix.c b/oslib-posix.c
index dbeb627..ab32d6d 100644
--- a/oslib-posix.c
+++ b/oslib-posix.c
@@ -35,19 +35,23 @@
extern int daemon(int, int);
#endif
-#if defined(__linux__) && defined(__x86_64__)
+#ifdef __linux__
+# include
+
+# if defined(__x86_64__) || defined(__i386__)
/* Use 2 MiB alignment so transparent hugepages can be used by KVM.
Valgrind does not support alignments larger than 1 MiB,
therefore we need special code which handles running on Valgrind. */
-# define QEMU_VMALLOC_ALIGN (512 * 4096)
+# define QEMU_VMALLOC_ALIGN_HUGE (512 * 4096)
# define CONFIG_VALGRIND
-#elif defined(__linux__) && defined(__s390x__)
+# elif defined(__s390x__)
/* Use 1 MiB (segment size) alignment so gmap can be used by KVM. */
-# define QEMU_VMALLOC_ALIGN (256 * 4096)
-#else
-# define QEMU_VMALLOC_ALIGN getpagesize()
+# define QEMU_VMALLOC_ALIGN_HUGE (256 * 4096)
+# endif
#endif
+#define QEMU_VMALLOC_ALIGN getpagesize()
+
#include "config-host.h"
#include "sysemu.h"
#include "trace.h"
@@ -114,7 +118,6 @@ void *qemu_memalign(size_t alignment, size_t size)
void *qemu_vmalloc(size_t size)
{
void *ptr;
-size_t align = QEMU_VMALLOC_ALIGN;
#if defined(CONFIG_VALGRIND)
if (running_on_valgrind < 0) {
@@ -125,10 +128,22 @@ void *qemu_vmalloc(size_t size)
}
#endif
-if (size < align || running_on_valgrind) {
-align = getpagesize();
+#ifdef QEMU_VMALLOC_ALIGN_HUGE
+/* try to allocate as huge pages if supported and large enough */
+if (size >= QEMU_VMALLOC_ALIGN_HUGE && !running_on_valgrind) {
+ptr = qemu_memalign(QEMU_VMALLOC_ALIGN_HUGE, size);
+#ifdef MADV_HUGEPAGE
+#error
+qemu_madvise(ptr, size, MADV_HUGEPAGE);
+#endif
}
-ptr = qemu_memalign(align, size);
+else
+#endif
+{
+/* if unsupported or small, allocate pagesize-aligned */
+ptr = qemu_memalign(QEMU_VMALLOC_ALIGN, size);
+}
+
trace_qemu_vmalloc(size, ptr);
return ptr;
}
--
1.7.10.4
Re: [Qemu-devel] qemu and transparent huge pages
On 15.08.2012 19:03, Michael Tokarev wrote: > +#ifdef MADV_HUGEPAGE > +#error Heh. This #error shouldn't be here ofcourse, I were checking if we really getting there. > +qemu_madvise(ptr, size, MADV_HUGEPAGE); > +#endif
Re: [Qemu-devel] [PATCH] json-parser: don't replicate tokens at each level of recursion
On Mon, Aug 13, 2012 at 09:25:44PM -0500, Michael Roth wrote:
> On Mon, Aug 13, 2012 at 03:01:56PM -0300, Luiz Capitulino wrote:
> > On Fri, 10 Aug 2012 18:24:10 -0500
> > Michael Roth wrote:
> >
> > > +qlist_iter(tokens, tokens_count_from_iter, &count);
> > > +if (count == 0) {
> > > +return NULL;
> > > +}
> >
> > Please, add qlist_size() instead.
> >
>
> Gladly :) I spent a good amount for a function that did this, not sure
> how I missed it.
>
Heh, read that as "please, use qlist_size() instead". I've added it
after further searching for it :)
> > > +
> > > +ctxt = g_malloc0(sizeof(JSONParserContext));
> > > +ctxt->tokens.pos = 0;
> > > +ctxt->tokens.count = count;
> > > +ctxt->tokens.buf = g_malloc(count * sizeof(QObject *));
> > > +qlist_iter(tokens, tokens_append_from_iter, ctxt);
> > > +ctxt->tokens.pos = 0;
> > > +
> > > +return ctxt;
> > > +}
> > > +
> > > +static void parser_context_free(JSONParserContext *ctxt)
> > > +{
> > > +int i;
> > > +if (ctxt) {
> > > +for (i = 0; i < ctxt->tokens.count; i++) {
> > > +qobject_decref(ctxt->tokens.buf[i]);
> > > +}
> > > +g_free(ctxt->tokens.buf);
> > > +g_free(ctxt);
> >
> > Isn't this leaking ctxt->err?
> >
>
> Indeed. Looks like we were leaking this previously as well. I'll get it
> in V2.
>
Apparently not, actually. It looks like error_propagate() handles
whether to free the error or pass it up the stack, so we can't mess with
it in the free function. I've added a comment to note that ctxt->err
needs to be freed seperately.
V2 is incoming with comments addressed.
[Qemu-devel] [PATCH for-1.2 v2 1/3] qlist: add qlist_size()
Signed-off-by: Michael Roth
---
qlist.c | 13 +
qlist.h |1 +
2 files changed, 14 insertions(+)
diff --git a/qlist.c b/qlist.c
index 88498b1..b48ec5b 100644
--- a/qlist.c
+++ b/qlist.c
@@ -124,6 +124,19 @@ int qlist_empty(const QList *qlist)
return QTAILQ_EMPTY(&qlist->head);
}
+static void qlist_size_iter(QObject *obj, void *opaque)
+{
+size_t *count = opaque;
+(*count)++;
+}
+
+size_t qlist_size(const QList *qlist)
+{
+size_t count = 0;
+qlist_iter(qlist, qlist_size_iter, &count);
+return count;
+}
+
/**
* qobject_to_qlist(): Convert a QObject into a QList
*/
diff --git a/qlist.h b/qlist.h
index d426bd4..ae776f9 100644
--- a/qlist.h
+++ b/qlist.h
@@ -49,6 +49,7 @@ void qlist_iter(const QList *qlist,
QObject *qlist_pop(QList *qlist);
QObject *qlist_peek(QList *qlist);
int qlist_empty(const QList *qlist);
+size_t qlist_size(const QList *qlist);
QList *qobject_to_qlist(const QObject *obj);
static inline const QListEntry *qlist_first(const QList *qlist)
--
1.7.9.5
[Qemu-devel] [PATCH for-1.2 v2 2/3] json-parser: don't replicate tokens at each level of recursion
Currently, when parsing a stream of tokens we make a copy of the token
list at the beginning of each level of recursion so that we do not
modify the original list in cases where we need to fall back to an
earlier state.
In the worst case, we will only read 1 or 2 tokens off the list before
recursing again, which means an upper bound of roughly N^2 token allocations.
For a "reasonably" sized QMP request (in this a QMP representation of
cirrus_vga's device state, generated via QIDL, being passed in via
qom-set), this caused my 16GB's of memory to be exhausted before any
noticeable progress was made by the parser.
This patch works around the issue by using single copy of the token list
in the form of an indexable array so that we can save/restore state by
manipulating indices.
A subsequent commit adds a "large_dict" test case which exhibits the
same behavior as above. With this patch applied the test case successfully
completes in under a second.
Tested with valgrind, make check, and QMP.
Signed-off-by: Michael Roth
---
json-parser.c | 229 +++--
1 file changed, 141 insertions(+), 88 deletions(-)
diff --git a/json-parser.c b/json-parser.c
index 849e215..65a87c7 100644
--- a/json-parser.c
+++ b/json-parser.c
@@ -27,6 +27,11 @@
typedef struct JSONParserContext
{
Error *err;
+struct {
+QObject **buf;
+size_t pos;
+size_t count;
+} tokens;
} JSONParserContext;
#define BUG_ON(cond) assert(!(cond))
@@ -40,7 +45,7 @@ typedef struct JSONParserContext
* 4) deal with premature EOI
*/
-static QObject *parse_value(JSONParserContext *ctxt, QList **tokens, va_list
*ap);
+static QObject *parse_value(JSONParserContext *ctxt, va_list *ap);
/**
* Token manipulators
@@ -270,27 +275,110 @@ out:
return NULL;
}
+static QObject *parser_context_pop_token(JSONParserContext *ctxt)
+{
+QObject *token;
+g_assert(ctxt->tokens.pos < ctxt->tokens.count);
+token = ctxt->tokens.buf[ctxt->tokens.pos];
+ctxt->tokens.pos++;
+return token;
+}
+
+/* Note: parser_context_{peek|pop}_token do not increment the
+ * token object's refcount. In both cases the references will continue
+ * to be * tracked and cleanup in parser_context_free()
+ */
+static QObject *parser_context_peek_token(JSONParserContext *ctxt)
+{
+QObject *token;
+g_assert(ctxt->tokens.pos < ctxt->tokens.count);
+token = ctxt->tokens.buf[ctxt->tokens.pos];
+return token;
+}
+
+static JSONParserContext parser_context_save(JSONParserContext *ctxt)
+{
+JSONParserContext saved_ctxt = {0};
+saved_ctxt.tokens.pos = ctxt->tokens.pos;
+saved_ctxt.tokens.count = ctxt->tokens.count;
+saved_ctxt.tokens.buf = ctxt->tokens.buf;
+return saved_ctxt;
+}
+
+static void parser_context_restore(JSONParserContext *ctxt,
+ JSONParserContext saved_ctxt)
+{
+ctxt->tokens.pos = saved_ctxt.tokens.pos;
+ctxt->tokens.count = saved_ctxt.tokens.count;
+ctxt->tokens.buf = saved_ctxt.tokens.buf;
+}
+
+static void tokens_append_from_iter(QObject *obj, void *opaque)
+{
+JSONParserContext *ctxt = opaque;
+g_assert(ctxt->tokens.pos < ctxt->tokens.count);
+ctxt->tokens.buf[ctxt->tokens.pos++] = obj;
+qobject_incref(obj);
+}
+
+static JSONParserContext *parser_context_new(QList *tokens)
+{
+JSONParserContext *ctxt;
+size_t count;
+
+if (!tokens) {
+return NULL;
+}
+
+count = qlist_size(tokens);
+if (count == 0) {
+return NULL;
+}
+
+ctxt = g_malloc0(sizeof(JSONParserContext));
+ctxt->tokens.pos = 0;
+ctxt->tokens.count = count;
+ctxt->tokens.buf = g_malloc(count * sizeof(QObject *));
+qlist_iter(tokens, tokens_append_from_iter, ctxt);
+ctxt->tokens.pos = 0;
+
+return ctxt;
+}
+
+/* to support error propagation, ctxt->err must be freed seperately */
+static void parser_context_free(JSONParserContext *ctxt)
+{
+int i;
+if (ctxt) {
+for (i = 0; i < ctxt->tokens.count; i++) {
+qobject_decref(ctxt->tokens.buf[i]);
+}
+g_free(ctxt->tokens.buf);
+g_free(ctxt);
+}
+}
+
/**
* Parsing rules
*/
-static int parse_pair(JSONParserContext *ctxt, QDict *dict, QList **tokens,
va_list *ap)
+static int parse_pair(JSONParserContext *ctxt, QDict *dict, va_list *ap)
{
QObject *key = NULL, *token = NULL, *value, *peek;
-QList *working = qlist_copy(*tokens);
+JSONParserContext saved_ctxt = parser_context_save(ctxt);
-peek = qlist_peek(working);
+peek = parser_context_peek_token(ctxt);
if (peek == NULL) {
parse_error(ctxt, NULL, "premature EOI");
goto out;
}
-key = parse_value(ctxt, &working, ap);
+key = parse_value(ctxt, ap);
if (!key || qobject_type(key) !=
[Qemu-devel] [PATCH for-1.2 v2 3/3] check-qjson: add test for large JSON objects
Signed-off-by: Michael Roth
---
tests/check-qjson.c | 53 +++
1 file changed, 53 insertions(+)
diff --git a/tests/check-qjson.c b/tests/check-qjson.c
index 526e25e..ef9b529 100644
--- a/tests/check-qjson.c
+++ b/tests/check-qjson.c
@@ -690,6 +690,58 @@ static void unterminated_literal(void)
g_assert(obj == NULL);
}
+/*
+ * this generates json of the form:
+ * a(0,m) = [0, 1, ..., m-1]
+ * a(n,m) = {
+ *'key0': a(0,m),
+ *'key1': a(1,m),
+ *...
+ *'key(n-1)': a(n-1,m)
+ * }
+ */
+static void gen_test_json(GString *gstr, int nest_level_max,
+ int elem_count)
+{
+int i;
+
+g_assert(gstr);
+if (nest_level_max == 0) {
+g_string_append(gstr, "[");
+for (i = 0; i < elem_count; i++) {
+g_string_append_printf(gstr, "%d", i);
+if (i < elem_count - 1) {
+g_string_append_printf(gstr, ", ");
+}
+}
+g_string_append(gstr, "]");
+return;
+}
+
+g_string_append(gstr, "{");
+for (i = 0; i < nest_level_max; i++) {
+g_string_append_printf(gstr, "'key%d': ", i);
+gen_test_json(gstr, i, elem_count);
+if (i < nest_level_max - 1) {
+g_string_append(gstr, ",");
+}
+}
+g_string_append(gstr, "}");
+}
+
+static void large_dict(void)
+{
+GString *gstr = g_string_new("");
+QObject *obj;
+
+gen_test_json(gstr, 10, 100);
+obj = qobject_from_json(gstr->str);
+g_assert(obj != NULL);
+
+qobject_decref(obj);
+g_string_free(gstr, true);
+}
+
int main(int argc, char **argv)
{
g_test_init(&argc, &argv, NULL);
@@ -706,6 +758,7 @@ int main(int argc, char **argv)
g_test_add_func("/literals/keyword", keyword_literal);
g_test_add_func("/dicts/simple_dict", simple_dict);
+g_test_add_func("/dicts/large_dict", large_dict);
g_test_add_func("/lists/simple_list", simple_list);
g_test_add_func("/whitespace/simple_whitespace", simple_whitespace);
--
1.7.9.5
Re: [Qemu-devel] [PATCH for-1.2 v2 2/3] json-parser: don't replicate tokens at each level of recursion
On Wed, Aug 15, 2012 at 12:09:09PM -0600, Eric Blake wrote:
> On 08/15/2012 11:56 AM, Michael Roth wrote:
> > Currently, when parsing a stream of tokens we make a copy of the token
> > list at the beginning of each level of recursion so that we do not
> > modify the original list in cases where we need to fall back to an
> > earlier state.
> >
> > +
> > +/* Note: parser_context_{peek|pop}_token do not increment the
> > + * token object's refcount. In both cases the references will continue
> > + * to be * tracked and cleanup in parser_context_free()
>
> Bad comment reflow? s/be * tracked/be tracked/
>
> > +
> > +/* to support error propagation, ctxt->err must be freed seperately */
>
> s/seperately/separately/
>
Thanks, fixed in incoming v3
> --
> Eric Blake ebl...@redhat.com+1-919-301-3266
> Libvirt virtualization library http://libvirt.org
>
[Qemu-devel] [PATCH for-1.2 v3 1/3] qlist: add qlist_size()
Signed-off-by: Michael Roth
---
qlist.c | 13 +
qlist.h |1 +
2 files changed, 14 insertions(+)
diff --git a/qlist.c b/qlist.c
index 88498b1..b48ec5b 100644
--- a/qlist.c
+++ b/qlist.c
@@ -124,6 +124,19 @@ int qlist_empty(const QList *qlist)
return QTAILQ_EMPTY(&qlist->head);
}
+static void qlist_size_iter(QObject *obj, void *opaque)
+{
+size_t *count = opaque;
+(*count)++;
+}
+
+size_t qlist_size(const QList *qlist)
+{
+size_t count = 0;
+qlist_iter(qlist, qlist_size_iter, &count);
+return count;
+}
+
/**
* qobject_to_qlist(): Convert a QObject into a QList
*/
diff --git a/qlist.h b/qlist.h
index d426bd4..ae776f9 100644
--- a/qlist.h
+++ b/qlist.h
@@ -49,6 +49,7 @@ void qlist_iter(const QList *qlist,
QObject *qlist_pop(QList *qlist);
QObject *qlist_peek(QList *qlist);
int qlist_empty(const QList *qlist);
+size_t qlist_size(const QList *qlist);
QList *qobject_to_qlist(const QObject *obj);
static inline const QListEntry *qlist_first(const QList *qlist)
--
1.7.9.5
[Qemu-devel] [PATCH for-1.2 v3 3/3] check-qjson: add test for large JSON objects
Signed-off-by: Michael Roth
---
tests/check-qjson.c | 53 +++
1 file changed, 53 insertions(+)
diff --git a/tests/check-qjson.c b/tests/check-qjson.c
index 526e25e..3b896f5 100644
--- a/tests/check-qjson.c
+++ b/tests/check-qjson.c
@@ -466,6 +466,58 @@ static void simple_dict(void)
}
}
+/*
+ * this generates json of the form:
+ * a(0,m) = [0, 1, ..., m-1]
+ * a(n,m) = {
+ *'key0': a(0,m),
+ *'key1': a(1,m),
+ *...
+ *'key(n-1)': a(n-1,m)
+ * }
+ */
+static void gen_test_json(GString *gstr, int nest_level_max,
+ int elem_count)
+{
+int i;
+
+g_assert(gstr);
+if (nest_level_max == 0) {
+g_string_append(gstr, "[");
+for (i = 0; i < elem_count; i++) {
+g_string_append_printf(gstr, "%d", i);
+if (i < elem_count - 1) {
+g_string_append_printf(gstr, ", ");
+}
+}
+g_string_append(gstr, "]");
+return;
+}
+
+g_string_append(gstr, "{");
+for (i = 0; i < nest_level_max; i++) {
+g_string_append_printf(gstr, "'key%d': ", i);
+gen_test_json(gstr, i, elem_count);
+if (i < nest_level_max - 1) {
+g_string_append(gstr, ",");
+}
+}
+g_string_append(gstr, "}");
+}
+
+static void large_dict(void)
+{
+GString *gstr = g_string_new("");
+QObject *obj;
+
+gen_test_json(gstr, 10, 100);
+obj = qobject_from_json(gstr->str);
+g_assert(obj != NULL);
+
+qobject_decref(obj);
+g_string_free(gstr, true);
+}
+
static void simple_list(void)
{
int i;
@@ -706,6 +758,7 @@ int main(int argc, char **argv)
g_test_add_func("/literals/keyword", keyword_literal);
g_test_add_func("/dicts/simple_dict", simple_dict);
+g_test_add_func("/dicts/large_dict", large_dict);
g_test_add_func("/lists/simple_list", simple_list);
g_test_add_func("/whitespace/simple_whitespace", simple_whitespace);
--
1.7.9.5
[Qemu-devel] [PATCH for-1.2 v3 2/3] json-parser: don't replicate tokens at each level of recursion
Currently, when parsing a stream of tokens we make a copy of the token
list at the beginning of each level of recursion so that we do not
modify the original list in cases where we need to fall back to an
earlier state.
In the worst case, we will only read 1 or 2 tokens off the list before
recursing again, which means an upper bound of roughly N^2 token allocations.
For a "reasonably" sized QMP request (in this a QMP representation of
cirrus_vga's device state, generated via QIDL, being passed in via
qom-set), this caused my 16GB's of memory to be exhausted before any
noticeable progress was made by the parser.
This patch works around the issue by using single copy of the token list
in the form of an indexable array so that we can save/restore state by
manipulating indices.
A subsequent commit adds a "large_dict" test case which exhibits the
same behavior as above. With this patch applied the test case successfully
completes in under a second.
Tested with valgrind, make check, and QMP.
Signed-off-by: Michael Roth
---
json-parser.c | 230 +++--
1 file changed, 142 insertions(+), 88 deletions(-)
diff --git a/json-parser.c b/json-parser.c
index 849e215..457291b 100644
--- a/json-parser.c
+++ b/json-parser.c
@@ -27,6 +27,11 @@
typedef struct JSONParserContext
{
Error *err;
+struct {
+QObject **buf;
+size_t pos;
+size_t count;
+} tokens;
} JSONParserContext;
#define BUG_ON(cond) assert(!(cond))
@@ -40,7 +45,7 @@ typedef struct JSONParserContext
* 4) deal with premature EOI
*/
-static QObject *parse_value(JSONParserContext *ctxt, QList **tokens, va_list
*ap);
+static QObject *parse_value(JSONParserContext *ctxt, va_list *ap);
/**
* Token manipulators
@@ -270,27 +275,111 @@ out:
return NULL;
}
+static QObject *parser_context_pop_token(JSONParserContext *ctxt)
+{
+QObject *token;
+g_assert(ctxt->tokens.pos < ctxt->tokens.count);
+token = ctxt->tokens.buf[ctxt->tokens.pos];
+ctxt->tokens.pos++;
+return token;
+}
+
+/* Note: parser_context_{peek|pop}_token do not increment the
+ * token object's refcount. In both cases the references will continue
+ * to be tracked and cleaned up in parser_context_free(), so do not
+ * attempt to free the token object.
+ */
+static QObject *parser_context_peek_token(JSONParserContext *ctxt)
+{
+QObject *token;
+g_assert(ctxt->tokens.pos < ctxt->tokens.count);
+token = ctxt->tokens.buf[ctxt->tokens.pos];
+return token;
+}
+
+static JSONParserContext parser_context_save(JSONParserContext *ctxt)
+{
+JSONParserContext saved_ctxt = {0};
+saved_ctxt.tokens.pos = ctxt->tokens.pos;
+saved_ctxt.tokens.count = ctxt->tokens.count;
+saved_ctxt.tokens.buf = ctxt->tokens.buf;
+return saved_ctxt;
+}
+
+static void parser_context_restore(JSONParserContext *ctxt,
+ JSONParserContext saved_ctxt)
+{
+ctxt->tokens.pos = saved_ctxt.tokens.pos;
+ctxt->tokens.count = saved_ctxt.tokens.count;
+ctxt->tokens.buf = saved_ctxt.tokens.buf;
+}
+
+static void tokens_append_from_iter(QObject *obj, void *opaque)
+{
+JSONParserContext *ctxt = opaque;
+g_assert(ctxt->tokens.pos < ctxt->tokens.count);
+ctxt->tokens.buf[ctxt->tokens.pos++] = obj;
+qobject_incref(obj);
+}
+
+static JSONParserContext *parser_context_new(QList *tokens)
+{
+JSONParserContext *ctxt;
+size_t count;
+
+if (!tokens) {
+return NULL;
+}
+
+count = qlist_size(tokens);
+if (count == 0) {
+return NULL;
+}
+
+ctxt = g_malloc0(sizeof(JSONParserContext));
+ctxt->tokens.pos = 0;
+ctxt->tokens.count = count;
+ctxt->tokens.buf = g_malloc(count * sizeof(QObject *));
+qlist_iter(tokens, tokens_append_from_iter, ctxt);
+ctxt->tokens.pos = 0;
+
+return ctxt;
+}
+
+/* to support error propagation, ctxt->err must be freed separately */
+static void parser_context_free(JSONParserContext *ctxt)
+{
+int i;
+if (ctxt) {
+for (i = 0; i < ctxt->tokens.count; i++) {
+qobject_decref(ctxt->tokens.buf[i]);
+}
+g_free(ctxt->tokens.buf);
+g_free(ctxt);
+}
+}
+
/**
* Parsing rules
*/
-static int parse_pair(JSONParserContext *ctxt, QDict *dict, QList **tokens,
va_list *ap)
+static int parse_pair(JSONParserContext *ctxt, QDict *dict, va_list *ap)
{
QObject *key = NULL, *token = NULL, *value, *peek;
-QList *working = qlist_copy(*tokens);
+JSONParserContext saved_ctxt = parser_context_save(ctxt);
-peek = qlist_peek(working);
+peek = parser_context_peek_token(ctxt);
if (peek == NULL) {
parse_error(ctxt, NULL, "premature EOI");
goto out;
}
-key = parse_value(ctxt, &working, ap);
+key = parse_value(ctxt, ap
Re: [Qemu-devel] [PATCH for-1.2 v3 1/3] qlist: add qlist_size()
On Wed, Aug 15, 2012 at 01:52:33PM -0600, Eric Blake wrote: > On 08/15/2012 12:45 PM, Michael Roth wrote: > > Signed-off-by: Michael Roth > > --- > > qlist.c | 13 + > > qlist.h |1 + > > 2 files changed, 14 insertions(+) > > No cover-letter? Started off as 1 patch with the explanation in the commit > > Reviewed-by: Eric Blake > > -- > Eric Blake ebl...@redhat.com+1-919-301-3266 > Libvirt virtualization library http://libvirt.org >
Re: [Qemu-devel] [PATCH for-1.2 v3 2/3] json-parser: don't replicate tokens at each level of recursion
On Wed, Aug 15, 2012 at 02:04:52PM -0600, Eric Blake wrote:
> On 08/15/2012 12:45 PM, Michael Roth wrote:
> > Currently, when parsing a stream of tokens we make a copy of the token
> > list at the beginning of each level of recursion so that we do not
> > modify the original list in cases where we need to fall back to an
> > earlier state.
> >
> > In the worst case, we will only read 1 or 2 tokens off the list before
> > recursing again, which means an upper bound of roughly N^2 token
> > allocations.
> >
> > For a "reasonably" sized QMP request (in this a QMP representation of
> > cirrus_vga's device state, generated via QIDL, being passed in via
> > qom-set), this caused my 16GB's of memory to be exhausted before any
> > noticeable progress was made by the parser.
> >
> > This patch works around the issue by using single copy of the token list
> > in the form of an indexable array so that we can save/restore state by
> > manipulating indices.
> >
> > A subsequent commit adds a "large_dict" test case which exhibits the
> > same behavior as above. With this patch applied the test case successfully
> > completes in under a second.
> >
> > Tested with valgrind, make check, and QMP.
> >
> > Signed-off-by: Michael Roth
> > ---
> > json-parser.c | 230
> > +++--
> > 1 file changed, 142 insertions(+), 88 deletions(-)
>
> I'm not the most familiar with this code, so take my review with a grain
> of salt, but I read through it and the transformation looks sane (and my
> non-code findings from v2 were fixed).
>
> Reviewed-by: Eric Blake
>
> > +static JSONParserContext parser_context_save(JSONParserContext *ctxt)
> > +{
> > +JSONParserContext saved_ctxt = {0};
> > +saved_ctxt.tokens.pos = ctxt->tokens.pos;
> > +saved_ctxt.tokens.count = ctxt->tokens.count;
> > +saved_ctxt.tokens.buf = ctxt->tokens.buf;
>
> Is it any simpler to condense 3 lines to 1:
>
> saved_cts.tokens = ctxt->tokens;
>
> > +return saved_ctxt;
> > +}
> > +
> > +static void parser_context_restore(JSONParserContext *ctxt,
> > + JSONParserContext saved_ctxt)
> > +{
> > +ctxt->tokens.pos = saved_ctxt.tokens.pos;
> > +ctxt->tokens.count = saved_ctxt.tokens.count;
> > +ctxt->tokens.buf = saved_ctxt.tokens.buf;
>
> and again, ctxt->tokens = saved_ctxt.tokens;
Poor function naming: save/restore apply to the token state, the other
fields in ctxt are unused, so I opted to set the fields explicitly.
Can probably make this read a little better by breaking token state off
into it's own struct, but I think we can clean that up later.
Thanks for the review!
>
> --
> Eric Blake ebl...@redhat.com+1-919-301-3266
> Libvirt virtualization library http://libvirt.org
>
[Qemu-devel] Apparent USB assignment issue on 1.1+
Hello. This issue has been reported several times already, but I can't reproduce it locally. Gerd, can you please take a look, maybe you may ask better question to the OP(s) about what to try. https://bugs.launchpad.net/qemu/+bug/1033727 http://bugs.debian.org/683983 Gerd, I tried to catch you on irc for several days, but you appears to be a non-frequent guest there these days... :) Thanks, /mjt
Re: [Qemu-devel] Apparent USB assignment issue on 1.1+
On 16.08.2012 12:14, Gerd Hoffmann wrote: > On 08/16/12 09:45, Michael Tokarev wrote: >> Hello. >> >> This issue has been reported several times already, >> but I can't reproduce it locally. Gerd, can you >> please take a look, maybe you may ask better >> question to the OP(s) about what to try. >> >> https://bugs.launchpad.net/qemu/+bug/1033727 > > http://patchwork.ozlabs.org/patch/176030/ could fix this. Interesting. > If not: enable all usb_host_* tracepoints & send log. Ok, will try to ping the OP... ;) >> http://bugs.debian.org/683983 > > No idea, must be some QOM thingy. There are two issues in there -- QOM (assertion failure) is unrelated. The main issue is that win BSODs when trying to use the USB device, but it worked fine in 1.0. I guess the only possible solution here is to try to bisect it. But it wont be easy due to other issues between 1.0 and 1.1. Overall, somehow I thought the two issues are related. Now when I re-read it again, I don't see why I thought so -- that's two different problems apparently. Thank you! /mjt
Re: [Qemu-devel] Apparent USB assignment issue on 1.1+
On 16.08.2012 12:23, Peter Maydell wrote: > On 16 August 2012 09:14, Gerd Hoffmann wrote: [] >>> http://bugs.debian.org/683983 >> >> No idea, must be some QOM thingy. This was http://bugs.debian.org/684282, unrelated to usb. > The "crash on usb_del" problem has been discussed on the list > before, Paolo posted a sketch of a patch to fix this: > http://lists.gnu.org/archive/html/qemu-devel/2012-08/msg01357.html > > I'd forgotten about the issue but I think we should definitely > fix for 1.2 if we can. Anthony -- does Paolo's patch look like > the right direction? Yes, this definitely needs fixing for 1.2. I too almost forgot about that after applying Paolo's patch to debian package... Thanks, /mjt
Re: [Qemu-devel] [PATCH for-1.2 v3 2/3] json-parser: don't replicate tokens at each level of recursion
On Thu, Aug 16, 2012 at 11:11:26AM -0300, Luiz Capitulino wrote:
> On Wed, 15 Aug 2012 13:45:43 -0500
> Michael Roth wrote:
>
> > Currently, when parsing a stream of tokens we make a copy of the token
> > list at the beginning of each level of recursion so that we do not
> > modify the original list in cases where we need to fall back to an
> > earlier state.
> >
> > In the worst case, we will only read 1 or 2 tokens off the list before
> > recursing again, which means an upper bound of roughly N^2 token
> > allocations.
> >
> > For a "reasonably" sized QMP request (in this a QMP representation of
> > cirrus_vga's device state, generated via QIDL, being passed in via
> > qom-set), this caused my 16GB's of memory to be exhausted before any
> > noticeable progress was made by the parser.
> >
> > This patch works around the issue by using single copy of the token list
> > in the form of an indexable array so that we can save/restore state by
> > manipulating indices.
> >
> > A subsequent commit adds a "large_dict" test case which exhibits the
> > same behavior as above. With this patch applied the test case successfully
> > completes in under a second.
> >
> > Tested with valgrind, make check, and QMP.
> >
> > Signed-off-by: Michael Roth
> > ---
> > json-parser.c | 230
> > +++--
> > 1 file changed, 142 insertions(+), 88 deletions(-)
> >
> > diff --git a/json-parser.c b/json-parser.c
> > index 849e215..457291b 100644
> > --- a/json-parser.c
> > +++ b/json-parser.c
> > @@ -27,6 +27,11 @@
> > typedef struct JSONParserContext
> > {
> > Error *err;
> > +struct {
> > +QObject **buf;
> > +size_t pos;
> > +size_t count;
> > +} tokens;
> > } JSONParserContext;
> >
> > #define BUG_ON(cond) assert(!(cond))
> > @@ -40,7 +45,7 @@ typedef struct JSONParserContext
> > * 4) deal with premature EOI
> > */
> >
> > -static QObject *parse_value(JSONParserContext *ctxt, QList **tokens,
> > va_list *ap);
> > +static QObject *parse_value(JSONParserContext *ctxt, va_list *ap);
> >
> > /**
> > * Token manipulators
> > @@ -270,27 +275,111 @@ out:
> > return NULL;
> > }
> >
> > +static QObject *parser_context_pop_token(JSONParserContext *ctxt)
> > +{
> > +QObject *token;
> > +g_assert(ctxt->tokens.pos < ctxt->tokens.count);
> > +token = ctxt->tokens.buf[ctxt->tokens.pos];
> > +ctxt->tokens.pos++;
>
> Shouldn't pos be decremented instead? Haven't tried it yet to confirm, but
> if I'm right I can fix it myself (unless this requires fixes in other areas).
>
No that's intentional, since qlist_pop() (which this replaces) actually "pops"
from the beginning of the list. So in this case we make the next element the
new "head" by simply incrementing the starting position (so it's easy to roll
back later).
> > +return token;
> > +}
> > +
> > +/* Note: parser_context_{peek|pop}_token do not increment the
> > + * token object's refcount. In both cases the references will continue
> > + * to be tracked and cleaned up in parser_context_free(), so do not
> > + * attempt to free the token object.
> > + */
> > +static QObject *parser_context_peek_token(JSONParserContext *ctxt)
> > +{
> > +QObject *token;
> > +g_assert(ctxt->tokens.pos < ctxt->tokens.count);
> > +token = ctxt->tokens.buf[ctxt->tokens.pos];
> > +return token;
> > +}
> > +
> > +static JSONParserContext parser_context_save(JSONParserContext *ctxt)
> > +{
> > +JSONParserContext saved_ctxt = {0};
> > +saved_ctxt.tokens.pos = ctxt->tokens.pos;
> > +saved_ctxt.tokens.count = ctxt->tokens.count;
> > +saved_ctxt.tokens.buf = ctxt->tokens.buf;
> > +return saved_ctxt;
> > +}
> > +
> > +static void parser_context_restore(JSONParserContext *ctxt,
> > + JSONParserContext saved_ctxt)
> > +{
> > +ctxt->tokens.pos = saved_ctxt.tokens.pos;
> > +ctxt->tokens.count = saved_ctxt.tokens.count;
> > +ctxt->tokens.buf = saved_ctxt.tokens.buf;
> > +}
> > +
> > +static void tokens_append_from_iter(QObject *obj, void *opaque)
> > +{
> > +JSONParserContext *ctxt = opaque;
> > +g_assert(ctxt->tokens.pos < ctxt-
Re: [Qemu-devel] [PATCH for-1.2 v3 1/3] qlist: add qlist_size()
On Thu, Aug 16, 2012 at 10:40:05AM -0300, Luiz Capitulino wrote:
> On Wed, 15 Aug 2012 13:45:42 -0500
> Michael Roth wrote:
>
> >
> > Signed-off-by: Michael Roth
>
> I've applied this series to the qmp branch for 1.2. I'll run some tests and if
> all goes alright will send a pull request shortly.
>
I just noticed via our fancy #qemu github bot that Anthony
applied these already, but I asked about and if you want to
test/submit through your queue it should all work out. So either
way.
Thanks!
> > ---
> > qlist.c | 13 +
> > qlist.h |1 +
> > 2 files changed, 14 insertions(+)
> >
> > diff --git a/qlist.c b/qlist.c
> > index 88498b1..b48ec5b 100644
> > --- a/qlist.c
> > +++ b/qlist.c
> > @@ -124,6 +124,19 @@ int qlist_empty(const QList *qlist)
> > return QTAILQ_EMPTY(&qlist->head);
> > }
> >
> > +static void qlist_size_iter(QObject *obj, void *opaque)
> > +{
> > +size_t *count = opaque;
> > +(*count)++;
> > +}
> > +
> > +size_t qlist_size(const QList *qlist)
> > +{
> > +size_t count = 0;
> > +qlist_iter(qlist, qlist_size_iter, &count);
> > +return count;
> > +}
> > +
> > /**
> > * qobject_to_qlist(): Convert a QObject into a QList
> > */
> > diff --git a/qlist.h b/qlist.h
> > index d426bd4..ae776f9 100644
> > --- a/qlist.h
> > +++ b/qlist.h
> > @@ -49,6 +49,7 @@ void qlist_iter(const QList *qlist,
> > QObject *qlist_pop(QList *qlist);
> > QObject *qlist_peek(QList *qlist);
> > int qlist_empty(const QList *qlist);
> > +size_t qlist_size(const QList *qlist);
> > QList *qobject_to_qlist(const QObject *obj);
> >
> > static inline const QListEntry *qlist_first(const QList *qlist)
>
Re: [Qemu-devel] [PATCH] block: handle filenames with colons better
On 16.08.2012 18:58, Iustin Pop wrote: > Commit 947995c (block: protect path_has_protocol from filenames with > colons) introduced a way to handle filenames with colons based on > whether the path contains a slash or not. IMHO this is not optimal, > since we shouldn't rely on the contents of the path but rather on > whether the given path exists as a file or not. > > As such, this patch tries to handle both files with and without > slashes by falling back to opening them as files if no drivers > supporting the protocol has been identified. I for one dislike this idea entirely: I think there should be a way to stop qemu from trying to open something as a file. It opens a security hole after all, "what if" such a file will actually exist? If I can vote, I'm voting against this with both hands. Thanks, /mjt
[Qemu-devel] [Bug 1037675] Re: Guest Kernel Panic if using "-cpu host" in qemu-kvm 1.1.1
First of all, your kernel panic screenshot is incomplete: it lacks the most important information which were scrolled off the (virtual) screen. Please enable serial console and capture whole OOPs in a text form. Second, it isn't clear whenever this is HOST kernel panic or GUEST kernel panic. I assume it is guest. Third, you nevrer said which guest (kernel) it is. And forth, gentoo is very well known for breaking qemu-kvm by their "hardened" patches. Disable the hardening and retry. /mjt -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1037675 Title: Guest Kernel Panic if using "-cpu host" in qemu-kvm 1.1.1 Status in QEMU: New Bug description: After Upgrading to qemu-kvm-1.1.1-r1 from version 1.0.1-r1 my virtual machines (running gentoo linux) panic at intel_pmu_init. (detailed information including stacktrace are in the uploaded screenshot). When i remove the "-cpu host" option, the system starts normally. the command line from whicht the system is bootet: qemu-kvm -vnc :7 -usbdevice tablet -daemonize -m 256 -drive file=/data/virtual_machines/wgs-l08.img,if=virtio -boot c -k de -net nic,model=virtio,macaddr=12:12:00:12:34:63,vlan=0 -net tap,ifname=qtap6,script=no,downscript=no,vlan=0 -smp 2 -enable-kvm -cpu host -monitor unix:/var/run/qemu- kvm/wgs-l08.monitor,server,nowait also reported on gentoo bug tracker (with some more details of the host): https://bugs.gentoo.org/show_bug.cgi?id=431640 To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1037675/+subscriptions
Re: [Qemu-devel] [RESEND][PATCH for 1.2] i82378: Remove bogus MMIO coalescing
On 17.08.2012 14:56, Jan Kiszka wrote: > This MMIO area is an entry gate to legacy PC ISA devices, addressed via > PIO over there. Quite a few of the PIO ports have side effects on access > like starting/stopping timers that must be executed properly ordered > /wrt the CPU. So we have to remove the coalescing mark. This appears to be 1.1-stable material right? Thanks, /mjt
Re: [Qemu-devel] [PATCH] vmware_vga: Redraw only visible area
On 17.08.2012 06:55, Marek Vasut wrote:
> Disallow negative value boundaries of the redraw rectangle.
> This fixes a segfault when using -vga vmware.
>
> Signed-off-by: Marek Vasut
> ---
> hw/vmware_vga.c |4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> NOTE: I tested this by emulating some recent version of ubuntu. The rect->x
> value was set to -65 for some reason at one point, which caused the
> kvm to crash. Trimming the rectangle fixed the issue.
>
> diff --git a/hw/vmware_vga.c b/hw/vmware_vga.c
> index f5e4f44..62e5887 100644
> --- a/hw/vmware_vga.c
> +++ b/hw/vmware_vga.c
> @@ -337,8 +337,8 @@ static inline void vmsvga_update_rect_delayed(struct
> vmsvga_state_s *s,
> {
> struct vmsvga_rect_s *rect = &s->redraw_fifo[s->redraw_fifo_last ++];
> s->redraw_fifo_last &= REDRAW_FIFO_LEN - 1;
> -rect->x = x;
> -rect->y = y;
> +rect->x = (x < 0) ? 0 : x;
> +rect->y = (y < 0) ? 0 : y;
> rect->w = w;
> rect->h = h;
> }
Is it the same as https://bugs.launchpad.net/bugs/918791 ?
At least it appears to be the same theme... But there,
the patch (https://launchpadlibrarian.net/94916786/qemu-vmware.debdiff)
also updates width/height. My comment:
https://bugs.launchpad.net/ubuntu/+source/qemu-kvm/+bug/918791/comments/21
"So indeed, some (upstream) verification is needed here -- where these
negative values are coming from, whenever it is EVER okay to have them,
what to do with these, and where to check (I guess the check should be
done somewhere in the upper layer)."
Especially the last part about the layer.
Thanks,
/mjt
