Re: [Qemu-devel] [PATCH, RFC] Replace assert(0) with abort() or cpu_abort()
Blue Swirl blauwir...@gmail.com writes: When building with -DNDEBUG, assert(0) will not stop execution so it must not be used for abnormal termination. For each case: are you sure the code does not recover after assert(0)? Not saying it does, just asking whether you checked. Use cpu_abort() when in CPU context, abort() otherwise. I sympathize with the general idea, but I don't like dead code after abort(). What about cleaning that up?
Re: [Qemu-devel] [PATCH 0/6] Multi-level page tables and userland mapping fixes, v3
On Sat, Mar 13, 2010 at 03:00:52PM +0100, Martin Mohring wrote: I am sorry to bring us all down to earth, we all had wished that this stuff gets finally fixed, but it seems that those patches applied to QEMU master have killed QEMU user mode. sid and maemo userlands work for me like before (64bit and 32bit hosts). I am referring to the user mode fixes after commit 0aef4261ac0ec9089ade0e3a92f986cb4ba7317e I had QEMU working on PowerPC and ARM in chroots from the Linux versions: - Fedora 11 / 12 - Ubuntu 9.04, 9.10, 10.04 - Debian 4, 5 and Sid for ARM and PowerPC. My host OS is openSUSE 11.2 using a 2.6.31 kernel, or openSUSE 11.1. All machines are 64 Bit machines. Now I have the situation that all the listed target chroots fail, except: - Fedora 11 / 12 @ ARM Please try to be more specific. What is killed? what do you mean by chroots fail. qemu segfault? some binary doesn't work that before did? some syscall doesn't work anymore?
Re: [Qemu-devel] -fda fat:dir -snapshot
Am 13.03.2010 20:18, schrieb Michael Tokarev: Apparently this does not work, and for a lng time: $ kvm -fda fat:dir [ it opens the sdl window ] $ kvm -fda fat:dir -snapshot qemu: could not open disk image fat:dir: No such file or directory Is it supposed to work? Wow, that's a crazy case. I guess nobody has ever tested this, and indeed it looks like it never has worked. As you might know, -snapshot internally creates a temporary qcow2 image which takes the image you originally asked for as a backing file. So we have a qcow2 file with a backing file tmp:dir. Now, backing file paths are always resolved relative to the COW file, so we get /tmp/fat:dir. Oops. We could just disable this for protocols as a quick fix, but I think in fact you do want to have this behaviour when using protocols as a backing file for a persistent COW image. I guess this needs some more thought, especially in respect to the discussions of making file/host_device/... protocols. If you really have a use case for this, you can use an absolute path after fat: as a workaround, it won't touch the path then. Kevin
[Qemu-devel] [PATCH] linux-user: use arm features based on cpu model
Use arm features based on cpu model. The hardcoded feature list gave problems in the setjmp/longjmp functions of glibc since it tried to use VFP instructions even though I specified a pxa270 as cpu model. Signed-off-by: Lars Munch l...@segv.dk --- linux-user/elfload.c | 10 ++ 1 files changed, 6 insertions(+), 4 deletions(-) diff --git a/linux-user/elfload.c b/linux-user/elfload.c index 91eea62..79af51d 100644 --- a/linux-user/elfload.c +++ b/linux-user/elfload.c @@ -333,10 +333,12 @@ enum ARM_HWCAP_ARM_VFPv3D16 = 1 13, }; -#define ELF_HWCAP (ARM_HWCAP_ARM_SWP | ARM_HWCAP_ARM_HALF \ -| ARM_HWCAP_ARM_THUMB | ARM_HWCAP_ARM_FAST_MULT \ -| ARM_HWCAP_ARM_FPA | ARM_HWCAP_ARM_VFP \ -| ARM_HWCAP_ARM_NEON | ARM_HWCAP_ARM_VFPv3 ) +#define ELF_HWCAP get_elf_hwcap() + +static uint32_t get_elf_hwcap(void) +{ +return thread_env-features; +} #endif -- 1.7.0
Re: [Qemu-devel] Ideas wiki for GSoC 2010
On 03/10/2010 11:30 PM, Luiz Capitulino wrote: Hi there, Our wiki page for the Summer of Code 2010 is doing quite well: http://wiki.qemu.org/Google_Summer_of_Code_2010 I will add another project - iommu emulation. Could be very useful for doing device assignment to nested guests, which could make testing a lot easier. -- error compiling committee.c: too many arguments to function
Re: [Qemu-devel] [PATCH] linux-user: use arm features based on cpu model
+static uint32_t get_elf_hwcap(void) +{ +return thread_env-features; +} No. These values are not the same. Paul
[Qemu-devel] Re: [PATCH v2 1/2] read-only: minor cleanup
On 03/14/2010 02:19 PM, Naphtali Sprei wrote: Really use read-only flags for opening the file when asked for read-only Signed-off-by: Naphtali Spreinsp...@redhat.com --- qemu-nbd.c |1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/qemu-nbd.c b/qemu-nbd.c index eac0c21..a393583 100644 --- a/qemu-nbd.c +++ b/qemu-nbd.c @@ -258,6 +258,7 @@ int main(int argc, char **argv) break; case 'r': readonly = true; +flags= ~BDRV_O_RDWR; break; case 'P': partition = strtol(optarg,end, 0); Acked-by: Paolo Bonzini pbonz...@redhat.com Paolo
[Qemu-devel] [PATCH] linux-user: fix running programs with iwmmxt instructions
When using linux-user for emulating an pxa270 we cannot generate an illegal instruction trap to the kernel to save/load the iwmmxt registers. Signed-off-by: Lars Munch l...@segv.dk --- target-arm/translate.c |2 ++ 1 files changed, 2 insertions(+), 0 deletions(-) diff --git a/target-arm/translate.c b/target-arm/translate.c index cdfe946..6b621b1 100644 --- a/target-arm/translate.c +++ b/target-arm/translate.c @@ -5802,9 +5802,11 @@ static int disas_coproc_insn(CPUState * env, DisasContext *s, uint32_t insn) int cpnum; cpnum = (insn 8) 0xf; +#if !defined(CONFIG_USER_ONLY) if (arm_feature(env, ARM_FEATURE_XSCALE) ((env-cp15.c15_cpar ^ 0x3fff) (1 cpnum))) return 1; +#endif switch (cpnum) { case 0: -- 1.7.0
Re: [Qemu-devel] Ideas wiki for GSoC 2010
On 03/15/2010 02:38 PM, Joerg Roedel wrote: On Mon, Mar 15, 2010 at 02:25:41PM +0200, Avi Kivity wrote: On 03/10/2010 11:30 PM, Luiz Capitulino wrote: Hi there, Our wiki page for the Summer of Code 2010 is doing quite well: http://wiki.qemu.org/Google_Summer_of_Code_2010 I will add another project - iommu emulation. Could be very useful for doing device assignment to nested guests, which could make testing a lot easier. Good idea. If there is interest I could help to mentor this project. Thanks. I volunteered Anthony, but he may be a little overcommitted. -- error compiling committee.c: too many arguments to function
Re: [Qemu-devel] [PATCH 0/6] Multi-level page tables and userland mapping fixes, v3
I had QEMU working on PowerPC and ARM in chroots from the Linux versions: - Fedora 11 / 12 - Ubuntu 9.04, 9.10, 10.04 - Debian 4, 5 and Sid for ARM and PowerPC. My host OS is openSUSE 11.2 using a 2.6.31 kernel, or openSUSE 11.1. All machines are 64 Bit machines. Now I have the situation that all the listed target chroots fail, except: - Fedora 11 / 12 @ ARM Please try to be more specific. What is killed? what do you mean by chroots fail. qemu segfault? some binary doesn't work that before did? some syscall doesn't work anymore? We're still investigating the topic. So far, we tracked one failure down to ldconfig.real which is a static arm binary executed after the chroot got the basic packages installed. If we setup a chroot with an older qemu-arm and exchange it afterwards with the new qemu and rerun just the build step, it works. Thus it seems to be an issue with static arm binaries atm. Strace of a call of ldconfig.real with the qemu-arm failing: http://pastie.org/870189 Sort version: r...@frodo:/# qemu-arm -strace /sbin/ldconfig.real 16359 uname(0x403fef78) = 0 16359 brk(NULL) = 0x000a9000 16359 brk(0x000a9d08) = 0x000a9d08 16359 open(/dev/urandom,O_RDONLY) = 3 16359 read(3,0x403ff27d,3) = 3 16359 close(3) = 0 [...] 16359 stat64(/usr/lib/libgettextlib.so,0x403fdf28) = 0 16359 stat64(/usr/lib/libgettextpo.so.0,0x403fdec0) = 0 16359 stat64(/usr/lib/libgettextpo.so.0.4.0,0x403fdf28) = 0 16359 stat64(/usr/lib/libpython2.6.so.1.0,0x403fdec0) = 0 16359 stat64(/usr/lib/libpython2.6.so.1.0,0x403fdf28) = 0 16359 open(/etc/ld.so.cache~,O_WRONLY|O_CREAT|O_NOFOLLOW|O_TRUNC,0600) = 3 16359 write(3,0xb03d0,1288) = 1288 16359 write(3,0x403ff0a0,0) = -1 errno=14 (Bad address) 16359 write(2,0x403fca08,21)/sbin/ldconfig.real: = 21 16359 write(2,0x403fc9e8,28)Writing of cache data failed = 28 16359 write(2,0x403fc5b8,13): Bad address = 13 16359 write(2,0x403fc9c0,1) = 1 16359 exit_group(1) Best, Jan-Simon
Re: [Qemu-devel] [PATCH] linux-user: fix running programs with iwmmxt instructions
+#if !defined(CONFIG_USER_ONLY) if (arm_feature(env, ARM_FEATURE_XSCALE) ((env-cp15.c15_cpar ^ 0x3fff) (1 cpnum))) return 1; +#endif This is almost certainly the wrong way to fix this. Paul
Re: [Qemu-devel] Ideas wiki for GSoC 2010
On 03/15/2010 03:03 PM, Joerg Roedel wrote: I will add another project - iommu emulation. Could be very useful for doing device assignment to nested guests, which could make testing a lot easier. Our experiments show that nested device assignment is pretty much required for I/O performance in nested scenarios. Really? I did a small test with virtio-blk in a nested guest (disk read with dd, so not a real benchmark) and got a reasonable read-performance of around 25MB/s from the disk in the l2-guest. Your guest wasn't doing a zillion VMREADs and VMWRITEs every exit. I plan to reduce VMREAD/VMWRITE overhead for kvm, but not much we can do for other guests. -- error compiling committee.c: too many arguments to function
Re: [Qemu-devel] Ideas wiki for GSoC 2010
On 03/15/2010 07:42 AM, Avi Kivity wrote: On 03/15/2010 02:38 PM, Joerg Roedel wrote: On Mon, Mar 15, 2010 at 02:25:41PM +0200, Avi Kivity wrote: On 03/10/2010 11:30 PM, Luiz Capitulino wrote: Hi there, Our wiki page for the Summer of Code 2010 is doing quite well: http://wiki.qemu.org/Google_Summer_of_Code_2010 I will add another project - iommu emulation. Could be very useful for doing device assignment to nested guests, which could make testing a lot easier. Good idea. If there is interest I could help to mentor this project. Thanks. I volunteered Anthony, but he may be a little overcommitted. Joerg, feel free to put your name against too. Regards, Anthony Liguori
Re: [Qemu-devel] Ideas wiki for GSoC 2010
On 03/15/2010 08:11 AM, Avi Kivity wrote: On 03/15/2010 03:03 PM, Joerg Roedel wrote: I will add another project - iommu emulation. Could be very useful for doing device assignment to nested guests, which could make testing a lot easier. Our experiments show that nested device assignment is pretty much required for I/O performance in nested scenarios. Really? I did a small test with virtio-blk in a nested guest (disk read with dd, so not a real benchmark) and got a reasonable read-performance of around 25MB/s from the disk in the l2-guest. Your guest wasn't doing a zillion VMREADs and VMWRITEs every exit. I plan to reduce VMREAD/VMWRITE overhead for kvm, but not much we can do for other guests. VMREAD/VMWRITEs are generally optimized by hypervisors as they tend to be costly. KVM is a bit unusual in terms of how many times the instructions are executed per exit. Regards, Anthony Liguori
[Qemu-devel] [PATCH] pcnet: make subsystem vendor id match hardware
Real pcnet device (AT2450) apparently has subsystem device and vendor id set to 0, this is out of spec (which requires that vendor id is obtained from PCI SIG) but windows xp driver seems to need this in order to associate. qemu sets pci subsystem id to qumranet/qemu since d350d97d196a632b6c7493acf07a061017fc6f7d, debian does not yet have this patch. https://bugzilla.redhat.com/show_bug.cgi?id=521247 Signed-off-by: Michael S. Tsirkin m...@redhat.com Cc: Gerd Hoffmann kra...@redhat.com Cc: Anthony Liguori aligu...@us.ibm.com --- hw/pcnet.c |3 +++ 1 files changed, 3 insertions(+), 0 deletions(-) diff --git a/hw/pcnet.c b/hw/pcnet.c index 44b5b31..12260be 100644 --- a/hw/pcnet.c +++ b/hw/pcnet.c @@ -1997,6 +1997,9 @@ static int pci_pcnet_init(PCIDevice *pci_dev) pci_set_long(pci_conf + PCI_BASE_ADDRESS_0 + 4, PCI_BASE_ADDRESS_SPACE_MEMORY); +pci_set_word(pci_conf + PCI_SUBSYSTEM_VENDOR_ID, 0x0); +pci_set_word(pci_conf + PCI_SUBSYSTEM_ID, 0x0); + /* TODO: value must be 0 at RST# */ pci_conf[PCI_INTERRUPT_PIN] = 1; // interrupt pin 0 pci_conf[PCI_MIN_GNT] = 0x06; -- 1.7.0.18.g0d53a5
Re: [Qemu-devel] Ideas wiki for GSoC 2010
On 03/15/2010 08:24 AM, Joerg Roedel wrote: On Mon, Mar 15, 2010 at 03:11:42PM +0200, Avi Kivity wrote: On 03/15/2010 03:03 PM, Joerg Roedel wrote: I will add another project - iommu emulation. Could be very useful for doing device assignment to nested guests, which could make testing a lot easier. Our experiments show that nested device assignment is pretty much required for I/O performance in nested scenarios. Really? I did a small test with virtio-blk in a nested guest (disk read with dd, so not a real benchmark) and got a reasonable read-performance of around 25MB/s from the disk in the l2-guest. Your guest wasn't doing a zillion VMREADs and VMWRITEs every exit. I plan to reduce VMREAD/VMWRITE overhead for kvm, but not much we can do for other guests. Does it matter for the ept-on-ept case? The initial patchset of nested-vmx implemented it and they reported a performance drop of around 12% between levels which is reasonable. So I expected the loss of io-performance for l2 also reasonable in this case. My small measurement was also done using npt-on-npt. But that was something like kernbench IIRC which is actually exit light once ept is enabled. Network IO is typically exit heavy and becomes something more of a pathological work load (both for nested ept and nested npt). Regards, Anthony Liguori Joerg -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] -fda fat:dir -snapshot
Kevin Wolf wrote: Am 13.03.2010 20:18, schrieb Michael Tokarev: Apparently this does not work, and for a lng time: $ kvm -fda fat:dir [ it opens the sdl window ] $ kvm -fda fat:dir -snapshot qemu: could not open disk image fat:dir: No such file or directory Is it supposed to work? Wow, that's a crazy case. I guess nobody has ever tested this, and indeed it looks like it never has worked. As you might know, -snapshot internally creates a temporary qcow2 image which takes the image you originally asked for as a backing file. Yeah, I looked at strace output at least. [] We could just disable this for protocols as a quick fix, but I think in fact you do want to have this behaviour when using protocols as a backing file for a persistent COW image. I guess this needs some more thought, especially in respect to the discussions of making file/host_device/... protocols. If you really have a use case for this, you can use an absolute path after fat: as a workaround, it won't touch the path then. Initially it was a bugreport against kvm in Debian - see http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=504049 Virtual FAT drive doesn't work with -snapshot, I'm not really sure what use case there is. But for me, I already had a more-or-less valid use case - trying a new driver in windows, supplying it in a virtual floppy (hence fat:dir) and checking if it works (hence -snapshot). I actually tried to do something in the past, but didn't know the fat: thing (apparently it is not mentioned in the manpage). But the whole thing indeed looks somewhat.. messy. ;) Thanks! /mjt
Re: [Qemu-devel] [PATCH 0/6] Multi-level page tables and userland mapping fixes, v3
On Mon, Mar 15, 2010 at 01:46:10PM +0100, Jan-Simon Möller wrote: We're still investigating the topic. So far, we tracked one failure down to ldconfig.real which is a static arm binary executed after the chroot got the basic packages installed. If we setup a chroot with an older qemu-arm and exchange it afterwards with the new qemu and rerun just the build step, it works. Thus it seems to be an issue with static arm binaries atm. Strace of a call of ldconfig.real with the qemu-arm failing: http://pastie.org/870189 Sort version: r...@frodo:/# qemu-arm -strace /sbin/ldconfig.real 16359 uname(0x403fef78) = 0 16359 brk(NULL) = 0x000a9000 16359 brk(0x000a9d08) = 0x000a9d08 16359 open(/dev/urandom,O_RDONLY) = 3 16359 read(3,0x403ff27d,3) = 3 16359 close(3) = 0 [...] 16359 stat64(/usr/lib/libgettextlib.so,0x403fdf28) = 0 16359 stat64(/usr/lib/libgettextpo.so.0,0x403fdec0) = 0 16359 stat64(/usr/lib/libgettextpo.so.0.4.0,0x403fdf28) = 0 16359 stat64(/usr/lib/libpython2.6.so.1.0,0x403fdec0) = 0 16359 stat64(/usr/lib/libpython2.6.so.1.0,0x403fdf28) = 0 16359 open(/etc/ld.so.cache~,O_WRONLY|O_CREAT|O_NOFOLLOW|O_TRUNC,0600) = 3 16359 write(3,0xb03d0,1288) = 1288 16359 write(3,0x403ff0a0,0) = -1 errno=14 (Bad address) A zero sized write. According to manpage ok. In qemu we do a lock_user to to get the string to write. Richards change changes the access checks the get called by lock_user: page_check_range: -if (start + len start) -/* we've wrapped around */ ... +if (start + len - 1 start) { +/* We've wrapped around. */ This now blows up with len = 0; 16359 write(2,0x403fca08,21)/sbin/ldconfig.real: = 21 16359 write(2,0x403fc9e8,28)Writing of cache data failed = 28 16359 write(2,0x403fc5b8,13): Bad address = 13 16359 write(2,0x403fc9c0,1) = 1 16359 exit_group(1) Best, Jan-Simon
[Qemu-devel] wake-on-lan IPMI implementation; real power-off and -no-shutdown
Hello, while working on a demonstrator for a green-IT project, to show scheduled machine shutdown and powering depending on various conditions, I wondered if I could use QEMU with wake-on-lan transparently, but it seems it's not implemented at all. I though I could try to add support for it, and with -S it theorically should be doable at least for the first boot, but the network packets do not go much further until the NIC is actually initialized, as most network layers use qemu_can_send_packet() which returns 0 if the machine is stopped. Hacking this function to return 1 seems to push the packet upward, but I couldn't find a single point where I could check for WOL packets, different -net subsystems using different code paths. Also, it seems -no-shutdown doesn't actually stop the emulation as said in the manual, it actually keeps the vm running (and using cpu), despite the OS trying to shutdown via ACPI. At least I tested so with Haiku (and acpi=true in kernel config), which properly exits QEMU without -no-shutdown. Ideally this would evolve into supporting IPMI, which would allow managing VMs exactly like physical servers without concern, appart launching the actual process first. cf. http://en.wikipedia.org/wiki/Intelligent_Platform_Management_Interface http://openipmi.sourceforge.net/ Anyone interested in this ? Anyone tried already ? François.
Re: [Qemu-devel] Ideas wiki for GSoC 2010
On 03/15/2010 03:23 PM, Anthony Liguori wrote: On 03/15/2010 08:11 AM, Avi Kivity wrote: On 03/15/2010 03:03 PM, Joerg Roedel wrote: I will add another project - iommu emulation. Could be very useful for doing device assignment to nested guests, which could make testing a lot easier. Our experiments show that nested device assignment is pretty much required for I/O performance in nested scenarios. Really? I did a small test with virtio-blk in a nested guest (disk read with dd, so not a real benchmark) and got a reasonable read-performance of around 25MB/s from the disk in the l2-guest. Your guest wasn't doing a zillion VMREADs and VMWRITEs every exit. I plan to reduce VMREAD/VMWRITE overhead for kvm, but not much we can do for other guests. VMREAD/VMWRITEs are generally optimized by hypervisors as they tend to be costly. KVM is a bit unusual in terms of how many times the instructions are executed per exit. Do you know offhand of any unnecessary read/writes? There's update_cr8_intercept(), but on normal exits, I don't see what else we can remove. -- error compiling committee.c: too many arguments to function
Re: [Qemu-devel] [PATCH 0/6] Multi-level page tables and userland mapping fixes, v3
Am Montag, 15. März 2010 15:48:03 schrieb Riku Voipio: On Mon, Mar 15, 2010 at 01:46:10PM +0100, Jan-Simon Möller wrote: We're still investigating the topic. So far, we tracked one failure down to ldconfig.real which is a static arm binary executed after the chroot got the basic packages installed. If we setup a chroot with an older qemu-arm and exchange it afterwards with the new qemu and rerun just the build step, it works. Thus it seems to be an issue with static arm binaries atm. Strace of a call of ldconfig.real with the qemu-arm failing: http://pastie.org/870189 Sort version: r...@frodo:/# qemu-arm -strace /sbin/ldconfig.real 16359 uname(0x403fef78) = 0 16359 brk(NULL) = 0x000a9000 16359 brk(0x000a9d08) = 0x000a9d08 16359 open(/dev/urandom,O_RDONLY) = 3 16359 read(3,0x403ff27d,3) = 3 16359 close(3) = 0 [...] 16359 stat64(/usr/lib/libgettextlib.so,0x403fdf28) = 0 16359 stat64(/usr/lib/libgettextpo.so.0,0x403fdec0) = 0 16359 stat64(/usr/lib/libgettextpo.so.0.4.0,0x403fdf28) = 0 16359 stat64(/usr/lib/libpython2.6.so.1.0,0x403fdec0) = 0 16359 stat64(/usr/lib/libpython2.6.so.1.0,0x403fdf28) = 0 16359 open(/etc/ld.so.cache~,O_WRONLY|O_CREAT|O_NOFOLLOW|O_TRUNC,0600) = 3 16359 write(3,0xb03d0,1288) = 1288 16359 write(3,0x403ff0a0,0) = -1 errno=14 (Bad address) A zero sized write. According to manpage ok. In qemu we do a lock_user to to get the string to write. Richards change changes the access checks the get called by lock_user: page_check_range: -if (start + len start) -/* we've wrapped around */ ... +if (start + len - 1 start) { +/* We've wrapped around. */ This now blows up with len = 0; Confirmed. A quick test with if (len 0) around and ldconfig.real runs. Best, Jan-Simon
Re: [Qemu-devel] wake-on-lan IPMI implementation; real power-off and -no-shutdown
Ideally this would evolve into supporting IPMI, which would allow managing VMs exactly like physical servers without concern, appart launching the actual process first. cf. http://en.wikipedia.org/wiki/Intelligent_Platform_Management_Interface http://openipmi.sourceforge.net/ Anyone interested in this ? Anyone tried already ? TBH I don't really see the point. This definitely seems like something that should be handled by your your mangement app (via libvirt, or whatever). i.e. have that implement whatever remote protocol you want, and just talk to qemu over the normal monitor interface. Paul
Re: [Qemu-devel] wake-on-lan IPMI implementation; real power-off and -no-shutdown
Le Mon, 15 Mar 2010 15:26:24 +, Paul Brook a écrit : Ideally this would evolve into supporting IPMI, which would allow managing VMs exactly like physical servers without concern, appart launching the actual process first. cf. http://en.wikipedia.org/wiki/Intelligent_Platform_Management_Interface http://openipmi.sourceforge.net/ Anyone interested in this ? Anyone tried already ? TBH I don't really see the point. This definitely seems like something that should be handled by your your mangement app (via libvirt, or whatever). i.e. have that implement whatever remote protocol you want, and just talk to qemu over the normal monitor interface. Hmm for IPMI that might probably indeed be a better way, moreover since they use a parallel subnet on the primary ethernet card... Though it would require the implementation to listen to the same virtual card, which are mapped differently to the host depending on the VM (-net user, ...). As for WOL, it would still be handy to have I think... btw, do we support suspending the emulation via ACPI ? VirtualBox has something called Pause mode, which I'm not sure actually if it's reflected to ACPI, which allows to avoid wasting cpu when not usign the guest, though it requires support from the Guest Additions to avoid clock drifts. François.
Re: [Qemu-devel] wake-on-lan IPMI implementation; real power-off and -no-shutdown
On 03/15/2010 10:37 AM, François Revol wrote: Le Mon, 15 Mar 2010 15:26:24 +, Paul Brook a écrit : Ideally this would evolve into supporting IPMI, which would allow managing VMs exactly like physical servers without concern, appart launching the actual process first. cf. http://en.wikipedia.org/wiki/Intelligent_Platform_Management_Interface http://openipmi.sourceforge.net/ Anyone interested in this ? Anyone tried already ? TBH I don't really see the point. This definitely seems like something that should be handled by your your mangement app (via libvirt, or whatever). i.e. have that implement whatever remote protocol you want, and just talk to qemu over the normal monitor interface. Hmm for IPMI that might probably indeed be a better way, moreover since they use a parallel subnet on the primary ethernet card... Though it would require the implementation to listen to the same virtual card, which are mapped differently to the host depending on the VM (-net user, ...). As for WOL, it would still be handy to have I think... btw, do we support suspending the emulation via ACPI ? VirtualBox has something called Pause mode, which I'm not sure actually if it's reflected to ACPI, which allows to avoid wasting cpu when not usign the guest, though it requires support from the Guest Additions to avoid clock drifts. It's just stop. It won't cause time drift in qemu when using pvclock but you will get soft lockups in Linux. I presume you get that with Virtual Box too. Regards, Anthony Liguori François.
Re: [Qemu-devel] wake-on-lan IPMI implementation; real power-off and -no-shutdown
As for WOL, it would still be handy to have I think... btw, do we support suspending the emulation via ACPI ? VirtualBox has something called Pause mode, which I'm not sure actually if it's reflected to ACPI, which allows to avoid wasting cpu when not usign the guest, though it requires support from the Guest Additions to avoid clock drifts. It's just stop. It won't cause time drift in qemu when using pvclock but you will get soft lockups in Linux. I presume you get that with Virtual Box too. Hmm yeah I didn't try playing with the various clock options, since Haiku doesn't support them anyway, only TSC, and HPET but it's in progress. François.
Re: [Qemu-devel] wake-on-lan IPMI implementation; real power-off and -no-shutdown
On Mon, Mar 15, 2010 at 04:01:27PM +0100, Fran?ois Revol wrote: Hello, while working on a demonstrator for a green-IT project, to show scheduled machine shutdown and powering depending on various conditions, I wondered if I could use QEMU with wake-on-lan transparently, but it seems it's not implemented at all. I though I could try to add support for it, and with -S it theorically should be doable at least for the first boot, but the network packets do not go much further until the NIC is actually initialized, as most network layers use qemu_can_send_packet() which returns 0 if the machine is stopped. Hacking this function to return 1 seems to push the packet upward, but I couldn't find a single point where I could check for WOL packets, different -net subsystems using different code paths. Also, it seems -no-shutdown doesn't actually stop the emulation as said in the manual, it actually keeps the vm running (and using cpu), despite the OS trying to shutdown via ACPI. At least I tested so with Haiku (and acpi=true in kernel config), which properly exits QEMU without -no-shutdown. Hmm, I think -no-shutdown should at least stop the CPUs executing. It is not really useful on its own though. The app managing QEMU would want to use the new JSON based monitor to listen for the SHUTDOWN event to be emitted, so it can detect the shutdown completing then take action it wants either reset the guest, or kill QEMU, etc Daniel -- |: Red Hat, Engineering, London-o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :| |: http://autobuild.org-o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
Re: [Qemu-devel] [PATCH] linux-user: fix running programs with iwmmxt instructions
On Mon, Mar 15, 2010 at 12:53:25PM +, Paul Brook wrote: +#if !defined(CONFIG_USER_ONLY) if (arm_feature(env, ARM_FEATURE_XSCALE) ((env-cp15.c15_cpar ^ 0x3fff) (1 cpnum))) return 1; +#endif This is almost certainly the wrong way to fix this. Paul Here is a different fix. Hopefully I am on the right track this time. This patch inits the cpu to have CP0 and/or CP1 accessible from the beginning if in linux-user mode. Signed-off-by: Lars Munch l...@segv.dk --- target-arm/helper.c |6 ++ 1 files changed, 6 insertions(+), 0 deletions(-) diff --git a/target-arm/helper.c b/target-arm/helper.c index 1a181ac..3a55da2 100644 --- a/target-arm/helper.c +++ b/target-arm/helper.c @@ -165,6 +165,9 @@ static void cpu_reset_model_id(CPUARMState *env, uint32_t id) /* JTAG_ID is ((id 28) | 0x09265013) */ env-cp15.c0_cachetype = 0xd172172; env-cp15.c1_sys = 0x0078; +#if defined (CONFIG_USER_ONLY) +env-cp15.c15_cpar = 0x0001; +#endif break; case ARM_CPUID_PXA270_A0: case ARM_CPUID_PXA270_A1: @@ -178,6 +181,9 @@ static void cpu_reset_model_id(CPUARMState *env, uint32_t id) env-iwmmxt.cregs[ARM_IWMMXT_wCID] = 0x69051000 | 'Q'; env-cp15.c0_cachetype = 0xd172172; env-cp15.c1_sys = 0x0078; +#if defined (CONFIG_USER_ONLY) +env-cp15.c15_cpar = 0x0003; +#endif break; default: cpu_abort(env, Bad CPU ID: %x\n, id); -- 1.7.0
Re: [Qemu-devel] [PATCH] linux-user: use arm features based on cpu model
On Mon, Mar 15, 2010 at 12:26:27PM +, Paul Brook wrote: +static uint32_t get_elf_hwcap(void) +{ +return thread_env-features; +} No. These values are not the same. Paul Yes, these values are indeed not the same. Below is an updated patch with a function similar to the PPC get_elf_hwcap function. I am unsure if I have too many or too few features as I do not know the details on all the capability flags, so comments are more than welcome. Signed-off-by: Lars Munch l...@segv.dk --- linux-user/elfload.c | 24 1 files changed, 20 insertions(+), 4 deletions(-) diff --git a/linux-user/elfload.c b/linux-user/elfload.c index 91eea62..6364176 100644 --- a/linux-user/elfload.c +++ b/linux-user/elfload.c @@ -333,10 +333,26 @@ enum ARM_HWCAP_ARM_VFPv3D16 = 1 13, }; -#define ELF_HWCAP (ARM_HWCAP_ARM_SWP | ARM_HWCAP_ARM_HALF \ -| ARM_HWCAP_ARM_THUMB | ARM_HWCAP_ARM_FAST_MULT \ -| ARM_HWCAP_ARM_FPA | ARM_HWCAP_ARM_VFP \ -| ARM_HWCAP_ARM_NEON | ARM_HWCAP_ARM_VFPv3 ) +#define ELF_HWCAP get_elf_hwcap() + +static uint32_t get_elf_hwcap(void) +{ +CPUARMState *e = thread_env; +uint32_t features = ARM_HWCAP_ARM_SWP | ARM_HWCAP_ARM_HALF + | ARM_HWCAP_ARM_THUMB | ARM_HWCAP_ARM_FAST_MULT + | ARM_HWCAP_ARM_FPA; + +#define GET_FEATURE(flag, feature) \ +do {if (e-features flag) features |= feature; } while(0) +GET_FEATURE(ARM_FEATURE_VFP, ARM_HWCAP_ARM_VFP); +GET_FEATURE(ARM_FEATURE_IWMMXT, ARM_HWCAP_ARM_IWMMXT); +GET_FEATURE(ARM_FEATURE_NEON, ARM_HWCAP_ARM_NEON); +GET_FEATURE(ARM_FEATURE_VFP3, ARM_HWCAP_ARM_VFPv3); +GET_FEATURE(ARM_FEATURE_VFP_FP16, ARM_HWCAP_ARM_VFPv3D16); +#undef GET_FEATURE + +return features; +} #endif -- 1.7.0
Re: [Qemu-devel] wake-on-lan IPMI implementation; real power-off and -no-shutdown
On 03/15/2010 10:55 AM, Daniel P. Berrange wrote: On Mon, Mar 15, 2010 at 04:01:27PM +0100, Fran?ois Revol wrote: Hello, while working on a demonstrator for a green-IT project, to show scheduled machine shutdown and powering depending on various conditions, I wondered if I could use QEMU with wake-on-lan transparently, but it seems it's not implemented at all. I though I could try to add support for it, and with -S it theorically should be doable at least for the first boot, but the network packets do not go much further until the NIC is actually initialized, as most network layers use qemu_can_send_packet() which returns 0 if the machine is stopped. Hacking this function to return 1 seems to push the packet upward, but I couldn't find a single point where I could check for WOL packets, different -net subsystems using different code paths. Also, it seems -no-shutdown doesn't actually stop the emulation as said in the manual, it actually keeps the vm running (and using cpu), despite the OS trying to shutdown via ACPI. At least I tested so with Haiku (and acpi=true in kernel config), which properly exits QEMU without -no-shutdown. Hmm, I think -no-shutdown should at least stop the CPUs executing. It is not really useful on its own though. The app managing QEMU would want to use the new JSON based monitor to listen for the SHUTDOWN event to be emitted, so it can detect the shutdown completing then take action it wants either reset the guest, or kill QEMU, etc The semantics of -no-shutdown are awful. I'd personally prefer to see the option deprecated and a new set of options introduced with clearer semantics. Currently, -no-shutdown does too many things. It affects reboot behaviour, shutdown behaviour, the behavior of the SDL close button. Each of these things should be individual tunables. Regards, Anthony Liguori Daniel
Re: [Qemu-devel] wake-on-lan IPMI implementation; real power-off and -no-shutdown
Also, it seems -no-shutdown doesn't actually stop the emulation as said in the manual, it actually keeps the vm running (and using cpu), despite the OS trying to shutdown via ACPI. At least I tested so with Haiku (and acpi=true in kernel config), which properly exits QEMU without -no-shutdown. Hmm, I think -no-shutdown should at least stop the CPUs executing. It is not really useful on its own though. The app managing QEMU would want to use the new JSON based monitor to listen for the SHUTDOWN event to be emitted, so it can detect the shutdown completing then take action it wants either reset the guest, or kill QEMU, etc The semantics of -no-shutdown are awful. I'd personally prefer to see the option deprecated and a new set of options introduced with clearer semantics. Currently, -no-shutdown does too many things. It affects reboot behaviour, shutdown behaviour, the behavior of the SDL close button. Each of these things should be individual tunables. Indeed, it both means do not allow user to close the vm and do not allow the guest to stop (or reboot ?) it seems... François.
[Qemu-devel] [RFC][PATCH 0/7] blkdebug
This patch series introduces a new block driver which acts as a protocol and whose purpose it is to fail requests. To be more precise, I want it to fail in configurable places, so that qemu-iotests can be extended with tests for the error paths (for example for the case when something with metadata writes goes wrong deep in qcow2). It works like this (I think this is self-explanatory): $ cat /tmp/blkdebug.cfg [inject-error] event = l1_update errno = 5 immediately = on $ qemu-io blkdebug:/tmp/blkdebug.cfg:/tmp/empty.qcow2 qemu-io read 0 4k read 4096/4096 bytes at offset 0 4 KiB, 1 ops; 0. sec (195.312 MiB/sec and 5. ops/sec) qemu-io write 0 4k bdrv_debug_event: 4 blkdebug_debug_event: 4 write failed: Input/output error Basically what I think is left to do is: - Resolve TODOs and FIXMEs left in the code - Require blkdebug to be explicitly enabled before compiling in event generation (not sure if it really matters) - Add more events, they are only added for qcow2-clusters so far - Address your comments (if any) Kevin Wolf (7): qemu-config: qemu_read_config_file() reads the normal config file qemu-config: Make qemu_config_parse more generic blkdebug: Basic request passthrough blkdebug: Inject errors Make qemu-config available for tools blkdebug: Add events and rules qcow2: Trigger blkdebug events Makefile.objs |6 +- block.c | 13 ++ block.h | 28 block/blkdebug.c | 423 + block/qcow2-cluster.c | 15 ++ block_int.h |2 + hw/qdev-properties.c | 19 ++- hw/qdev.h |1 - qemu-config.c | 45 +++--- qemu-config.h |4 +- vl.c | 34 ++--- 11 files changed, 540 insertions(+), 50 deletions(-) create mode 100644 block/blkdebug.c
[Qemu-devel] [RFC][PATCH 2/7] qemu-config: Make qemu_config_parse more generic
qemu_config_parse gets the option groups as a parameter now instead of hardcoding the VM configuration groups. This way it can be used for other configurations, too. Signed-off-by: Kevin Wolf kw...@redhat.com --- qemu-config.c | 15 --- qemu-config.h |2 +- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/qemu-config.c b/qemu-config.c index 8166b5e..e121f2a 100644 --- a/qemu-config.c +++ b/qemu-config.c @@ -290,7 +290,7 @@ QemuOptsList qemu_cpudef_opts = { }, }; -static QemuOptsList *lists[] = { +static QemuOptsList *vm_config_groups[] = { qemu_drive_opts, qemu_chardev_opts, qemu_device_opts, @@ -303,7 +303,7 @@ static QemuOptsList *lists[] = { NULL, }; -static QemuOptsList *find_list(const char *group) +static QemuOptsList *find_list(QemuOptsList **lists, const char *group) { int i; @@ -330,7 +330,7 @@ int qemu_set_option(const char *str) return -1; } -list = find_list(group); +list = find_list(vm_config_groups, group); if (list == NULL) { return -1; } @@ -415,6 +415,7 @@ static int config_write_opts(QemuOpts *opts, void *opaque) void qemu_config_write(FILE *fp) { struct ConfigWriteData data = { .fp = fp }; +QemuOptsList **lists = vm_config_groups; int i; fprintf(fp, # qemu config file\n\n); @@ -424,7 +425,7 @@ void qemu_config_write(FILE *fp) } } -int qemu_config_parse(FILE *fp) +int qemu_config_parse(FILE *fp, QemuOptsList **lists) { char line[1024], group[64], id[64], arg[64], value[1024]; QemuOptsList *list = NULL; @@ -441,7 +442,7 @@ int qemu_config_parse(FILE *fp) } if (sscanf(line, [%63s \%63[^\]\], group, id) == 2) { /* group with id */ -list = find_list(group); +list = find_list(lists, group); if (list == NULL) return -1; opts = qemu_opts_create(list, id, 1); @@ -449,7 +450,7 @@ int qemu_config_parse(FILE *fp) } if (sscanf(line, [%63[^]]], group) == 1) { /* group without id */ -list = find_list(group); +list = find_list(lists, group); if (list == NULL) return -1; opts = qemu_opts_create(list, NULL, 0); @@ -481,7 +482,7 @@ int qemu_read_config_file(const char *filename) return -errno; } -if (qemu_config_parse(f) != 0) { +if (qemu_config_parse(f, vm_config_groups) != 0) { return -EINVAL; } fclose(f); diff --git a/qemu-config.h b/qemu-config.h index bbe9d41..086aa2a 100644 --- a/qemu-config.h +++ b/qemu-config.h @@ -16,7 +16,7 @@ int qemu_global_option(const char *str); void qemu_add_globals(void); void qemu_config_write(FILE *fp); -int qemu_config_parse(FILE *fp); +int qemu_config_parse(FILE *fp, QemuOptsList **lists); int qemu_read_config_file(const char *filename); -- 1.6.6.1
[Qemu-devel] [RFC][PATCH 1/7] qemu-config: qemu_read_config_file() reads the normal config file
Introduce a new function qemu_read_config_file which reads the VM configuration from a config file. Unlike qemu_config_parse it doesn't take a open file but a filename and reduces code duplication as a side effect. Signed-off-by: Kevin Wolf kw...@redhat.com --- qemu-config.c | 15 +++ qemu-config.h |2 ++ vl.c | 34 +- 3 files changed, 30 insertions(+), 21 deletions(-) diff --git a/qemu-config.c b/qemu-config.c index 246fae6..8166b5e 100644 --- a/qemu-config.c +++ b/qemu-config.c @@ -473,3 +473,18 @@ int qemu_config_parse(FILE *fp) } return 0; } + +int qemu_read_config_file(const char *filename) +{ +FILE *f = fopen(filename, r); +if (f == NULL) { +return -errno; +} + +if (qemu_config_parse(f) != 0) { +return -EINVAL; +} +fclose(f); + +return 0; +} diff --git a/qemu-config.h b/qemu-config.h index b335c42..bbe9d41 100644 --- a/qemu-config.h +++ b/qemu-config.h @@ -18,4 +18,6 @@ void qemu_add_globals(void); void qemu_config_write(FILE *fp); int qemu_config_parse(FILE *fp); +int qemu_read_config_file(const char *filename); + #endif /* QEMU_CONFIG_H */ diff --git a/vl.c b/vl.c index a3e43ad..078e3f9 100644 --- a/vl.c +++ b/vl.c @@ -4940,21 +4940,17 @@ int main(int argc, char **argv, char **envp) } if (defconfig) { -FILE *fp; -fp = fopen(CONFIG_QEMU_CONFDIR /qemu.conf, r); -if (fp) { -if (qemu_config_parse(fp) != 0) { -exit(1); -} -fclose(fp); +int ret; + +ret = qemu_read_config_file(CONFIG_QEMU_CONFDIR /qemu.conf); +if (ret == -EINVAL) { +exit(1); } -fp = fopen(CONFIG_QEMU_CONFDIR /target- TARGET_ARCH .conf, r); -if (fp) { -if (qemu_config_parse(fp) != 0) { -exit(1); -} -fclose(fp); +ret = qemu_read_config_file(CONFIG_QEMU_CONFDIR +/target- TARGET_ARCH .conf); +if (ret == -EINVAL) { +exit(1); } } #if defined(cpudef_setup) @@ -5635,16 +5631,12 @@ int main(int argc, char **argv, char **envp) #endif case QEMU_OPTION_readconfig: { -FILE *fp; -fp = fopen(optarg, r); -if (fp == NULL) { -fprintf(stderr, open %s: %s\n, optarg, strerror(errno)); +int ret = qemu_read_config_file(optarg); +if (ret 0) { +fprintf(stderr, read config %s: %s\n, optarg, +strerror(-ret)); exit(1); } -if (qemu_config_parse(fp) != 0) { -exit(1); -} -fclose(fp); break; } case QEMU_OPTION_writeconfig: -- 1.6.6.1
[Qemu-devel] [RFC][PATCH 3/7] blkdebug: Basic request passthrough
This isn't doing anything interesting. It creates the blkdebug block driver as a protocol which just passes everything through to raw. Signed-off-by: Kevin Wolf kw...@redhat.com --- Makefile.objs|2 +- block/blkdebug.c | 104 ++ 2 files changed, 105 insertions(+), 1 deletions(-) create mode 100644 block/blkdebug.c diff --git a/Makefile.objs b/Makefile.objs index e791dd5..ece02d5 100644 --- a/Makefile.objs +++ b/Makefile.objs @@ -14,7 +14,7 @@ block-obj-$(CONFIG_LINUX_AIO) += linux-aio.o block-nested-y += cow.o qcow.o vdi.o vmdk.o cloop.o dmg.o bochs.o vpc.o vvfat.o block-nested-y += qcow2.o qcow2-refcount.o qcow2-cluster.o qcow2-snapshot.o -block-nested-y += parallels.o nbd.o +block-nested-y += parallels.o nbd.o blkdebug.o block-nested-$(CONFIG_WIN32) += raw-win32.o block-nested-$(CONFIG_POSIX) += raw-posix.o block-nested-$(CONFIG_CURL) += curl.o diff --git a/block/blkdebug.c b/block/blkdebug.c new file mode 100644 index 000..2c7e0dd --- /dev/null +++ b/block/blkdebug.c @@ -0,0 +1,104 @@ +/* + * Block protocol for I/O error injection + * + * Copyright (c) 2010 Kevin Wolf kw...@redhat.com + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the Software), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED AS IS, WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN + * THE SOFTWARE. + */ + +#include qemu-common.h +#include block_int.h +#include module.h + +typedef struct BDRVBlkdebugState { +BlockDriverState *hd; +} BDRVBlkdebugState; + +static int blkdebug_open(BlockDriverState *bs, const char *filename, int flags) +{ +BDRVBlkdebugState *s = bs-opaque; + +if (strncmp(filename, blkdebug:, strlen(blkdebug:))) { +return -EINVAL; +} +filename += strlen(blkdebug:); + +return bdrv_file_open(s-hd, filename, flags); +} + +static BlockDriverAIOCB *blkdebug_aio_readv(BlockDriverState *bs, +int64_t sector_num, QEMUIOVector *qiov, int nb_sectors, +BlockDriverCompletionFunc *cb, void *opaque) +{ +BDRVBlkdebugState *s = bs-opaque; +BlockDriverAIOCB *acb = +bdrv_aio_readv(s-hd, sector_num, qiov, nb_sectors, cb, opaque); +return acb; +} + +static BlockDriverAIOCB *blkdebug_aio_writev(BlockDriverState *bs, +int64_t sector_num, QEMUIOVector *qiov, int nb_sectors, +BlockDriverCompletionFunc *cb, void *opaque) +{ +BDRVBlkdebugState *s = bs-opaque; +BlockDriverAIOCB *acb = +bdrv_aio_writev(s-hd, sector_num, qiov, nb_sectors, cb, opaque); +return acb; +} + +static void blkdebug_close(BlockDriverState *bs) +{ +BDRVBlkdebugState *s = bs-opaque; +bdrv_delete(s-hd); +} + +static void blkdebug_flush(BlockDriverState *bs) +{ +BDRVBlkdebugState *s = bs-opaque; +bdrv_flush(s-hd); +} + +static BlockDriverAIOCB *blkdebug_aio_flush(BlockDriverState *bs, +BlockDriverCompletionFunc *cb, void *opaque) +{ +BDRVBlkdebugState *s = bs-opaque; +return bdrv_aio_flush(s-hd, cb, opaque); +} + +static BlockDriver bdrv_blkdebug = { +.format_name= blkdebug, +.protocol_name = blkdebug, + +.instance_size = sizeof(BDRVBlkdebugState), + +.bdrv_open = blkdebug_open, +.bdrv_close = blkdebug_close, +.bdrv_flush = blkdebug_flush, + +.bdrv_aio_readv = blkdebug_aio_readv, +.bdrv_aio_writev= blkdebug_aio_writev, +.bdrv_aio_flush = blkdebug_aio_flush, +}; + +static void bdrv_blkdebug_init(void) +{ +bdrv_register(bdrv_blkdebug); +} + +block_init(bdrv_blkdebug_init); -- 1.6.6.1
[Qemu-devel] [RFC][PATCH 4/7] blkdebug: Inject errors
Add a mechanism to inject errors instead of passing requests on. With no further patches applied, you can use it by setting inject_errno in gdb. Signed-off-by: Kevin Wolf kw...@redhat.com --- block/blkdebug.c | 63 ++ 1 files changed, 63 insertions(+), 0 deletions(-) diff --git a/block/blkdebug.c b/block/blkdebug.c index 2c7e0dd..f8ccd3c 100644 --- a/block/blkdebug.c +++ b/block/blkdebug.c @@ -26,10 +26,26 @@ #include block_int.h #include module.h +#include stdbool.h + typedef struct BDRVBlkdebugState { BlockDriverState *hd; + +int inject_errno; +bool inject_once; + +/* Decides if aio_readv/writev fails right away (true) or returns an error + * return value only in the callback (false) */ +bool inject_immediately; } BDRVBlkdebugState; +struct callback_data { +QEMUBH *bh; +BlockDriverCompletionFunc *cb; +int ret; +void *opaque; +}; + static int blkdebug_open(BlockDriverState *bs, const char *filename, int flags) { BDRVBlkdebugState *s = bs-opaque; @@ -42,11 +58,53 @@ static int blkdebug_open(BlockDriverState *bs, const char *filename, int flags) return bdrv_file_open(s-hd, filename, flags); } +static void error_callback_bh(void *opaque) +{ +struct callback_data *d = opaque; +qemu_bh_delete(d-bh); +d-cb(d-opaque, d-ret); +qemu_free(d); +} + +static BlockDriverAIOCB *inject_error(BlockDriverState *bs, +BlockDriverCompletionFunc *cb, void *opaque) +{ +BDRVBlkdebugState *s = bs-opaque; +int error = s-inject_errno; +struct callback_data *d; +QEMUBH *bh; + +if (s-inject_once) { +s-inject_errno = 0; +} + +if (s-inject_immediately) { +return NULL; +} + +d = qemu_mallocz(sizeof(*d)); +d-ret = error; +d-opaque = opaque; +d-cb = cb; + +bh = qemu_bh_new(error_callback_bh, d); +d-bh = bh; +qemu_bh_schedule(bh); + +// TODO Use a real AIOCB +return (BlockDriverAIOCB*) 42; +} + static BlockDriverAIOCB *blkdebug_aio_readv(BlockDriverState *bs, int64_t sector_num, QEMUIOVector *qiov, int nb_sectors, BlockDriverCompletionFunc *cb, void *opaque) { BDRVBlkdebugState *s = bs-opaque; + +if (s-inject_errno) { +return inject_error(bs, cb, opaque); +} + BlockDriverAIOCB *acb = bdrv_aio_readv(s-hd, sector_num, qiov, nb_sectors, cb, opaque); return acb; @@ -57,6 +115,11 @@ static BlockDriverAIOCB *blkdebug_aio_writev(BlockDriverState *bs, BlockDriverCompletionFunc *cb, void *opaque) { BDRVBlkdebugState *s = bs-opaque; + +if (s-inject_errno) { +return inject_error(bs, cb, opaque); +} + BlockDriverAIOCB *acb = bdrv_aio_writev(s-hd, sector_num, qiov, nb_sectors, cb, opaque); return acb; -- 1.6.6.1
[Qemu-devel] [RFC][PATCH 5/7] Make qemu-config available for tools
To be able to use config files for blkdebug, we need to make these functions available in the tools. This involves moving two functions that can only be built in the context of the emulator. Signed-off-by: Kevin Wolf kw...@redhat.com --- Makefile.objs|4 ++-- hw/qdev-properties.c | 19 ++- hw/qdev.h|1 - qemu-config.c| 17 - 4 files changed, 20 insertions(+), 21 deletions(-) diff --git a/Makefile.objs b/Makefile.objs index ece02d5..930408b 100644 --- a/Makefile.objs +++ b/Makefile.objs @@ -8,7 +8,7 @@ qobject-obj-y += qerror.o # block-obj-y is code used by both qemu system emulation and qemu-img block-obj-y = cutils.o cache-utils.o qemu-malloc.o qemu-option.o module.o -block-obj-y += nbd.o block.o aio.o aes.o osdep.o +block-obj-y += nbd.o block.o aio.o aes.o osdep.o qemu-config.o block-obj-$(CONFIG_POSIX) += posix-aio-compat.o block-obj-$(CONFIG_LINUX_AIO) += linux-aio.o @@ -76,7 +76,7 @@ common-obj-y += buffered_file.o migration.o migration-tcp.o qemu-sockets.o common-obj-y += qemu-char.o savevm.o #aio.o common-obj-y += msmouse.o ps2.o common-obj-y += qdev.o qdev-properties.o -common-obj-y += qemu-config.o block-migration.o +common-obj-y += block-migration.o common-obj-$(CONFIG_BRLAPI) += baum.o common-obj-$(CONFIG_POSIX) += migration-exec.o migration-unix.o migration-fd.o diff --git a/hw/qdev-properties.c b/hw/qdev-properties.c index 24671af..35d284e 100644 --- a/hw/qdev-properties.c +++ b/hw/qdev-properties.c @@ -666,7 +666,7 @@ void qdev_prop_set_defaults(DeviceState *dev, Property *props) static QTAILQ_HEAD(, GlobalProperty) global_props = QTAILQ_HEAD_INITIALIZER(global_props); -void qdev_prop_register_global(GlobalProperty *prop) +static void qdev_prop_register_global(GlobalProperty *prop) { QTAILQ_INSERT_TAIL(global_props, prop, next); } @@ -694,3 +694,20 @@ void qdev_prop_set_globals(DeviceState *dev) } } } + +static int qdev_add_one_global(QemuOpts *opts, void *opaque) +{ +GlobalProperty *g; + +g = qemu_mallocz(sizeof(*g)); +g-driver = qemu_opt_get(opts, driver); +g-property = qemu_opt_get(opts, property); +g-value= qemu_opt_get(opts, value); +qdev_prop_register_global(g); +return 0; +} + +void qemu_add_globals(void) +{ +qemu_opts_foreach(qemu_global_opts, qdev_add_one_global, NULL, 0); +} diff --git a/hw/qdev.h b/hw/qdev.h index adfcf79..30f31a1 100644 --- a/hw/qdev.h +++ b/hw/qdev.h @@ -273,7 +273,6 @@ void qdev_prop_set_macaddr(DeviceState *dev, const char *name, uint8_t *value); void qdev_prop_set_ptr(DeviceState *dev, const char *name, void *value); void qdev_prop_set_defaults(DeviceState *dev, Property *props); -void qdev_prop_register_global(GlobalProperty *prop); void qdev_prop_register_global_list(GlobalProperty *props); void qdev_prop_set_globals(DeviceState *dev); diff --git a/qemu-config.c b/qemu-config.c index e121f2a..9070729 100644 --- a/qemu-config.c +++ b/qemu-config.c @@ -367,23 +367,6 @@ int qemu_global_option(const char *str) return 0; } -static int qemu_add_one_global(QemuOpts *opts, void *opaque) -{ -GlobalProperty *g; - -g = qemu_mallocz(sizeof(*g)); -g-driver = qemu_opt_get(opts, driver); -g-property = qemu_opt_get(opts, property); -g-value= qemu_opt_get(opts, value); -qdev_prop_register_global(g); -return 0; -} - -void qemu_add_globals(void) -{ -qemu_opts_foreach(qemu_global_opts, qemu_add_one_global, NULL, 0); -} - struct ConfigWriteData { QemuOptsList *list; FILE *fp; -- 1.6.6.1
[Qemu-devel] [RFC][PATCH 6/7] blkdebug: Add events and rules
Block drivers can trigger a blkdebug event whenever they reach a place where it could be useful to inject an error for testing/debugging purposes. Rules are read from a blkdebug config file and describe which action is taken when an event is triggered. For now this is only injecting an error (with a few options) or changing the state (which is an integer). Rules can be declared to be active only in a specific state; this way later rules can distiguish on which path we came to trigger their event. Signed-off-by: Kevin Wolf kw...@redhat.com --- block.c | 13 +++ block.h |9 ++ block/blkdebug.c | 241 +- block_int.h |2 + 4 files changed, 264 insertions(+), 1 deletions(-) diff --git a/block.c b/block.c index 31d1ba4..d4adbc3 100644 --- a/block.c +++ b/block.c @@ -1532,6 +1532,19 @@ int bdrv_load_vmstate(BlockDriverState *bs, uint8_t *buf, return drv-bdrv_load_vmstate(bs, buf, pos, size); } +void bdrv_debug_event(BlockDriverState *bs, BlkDebugEvent event) +{ +BlockDriver *drv = bs-drv; + +fprintf(stderr, bdrv_debug_event: %d\n, event); +if (!drv || !drv-bdrv_debug_event) { +return; +} + +return drv-bdrv_debug_event(bs, event); + +} + /**/ /* handling of snapshots */ diff --git a/block.h b/block.h index edf5704..2fd8361 100644 --- a/block.h +++ b/block.h @@ -207,4 +207,13 @@ int bdrv_get_dirty(BlockDriverState *bs, int64_t sector); void bdrv_reset_dirty(BlockDriverState *bs, int64_t cur_sector, int nr_sectors); int64_t bdrv_get_dirty_count(BlockDriverState *bs); + + +typedef enum { +BLKDBG_EVENT_MAX, +} BlkDebugEvent; + +#define BLKDBG_EVENT(bs, evt) bdrv_debug_event(bs, evt) +void bdrv_debug_event(BlockDriverState *bs, BlkDebugEvent event); + #endif diff --git a/block/blkdebug.c b/block/blkdebug.c index f8ccd3c..22b1768 100644 --- a/block/blkdebug.c +++ b/block/blkdebug.c @@ -31,12 +31,15 @@ typedef struct BDRVBlkdebugState { BlockDriverState *hd; +int state; int inject_errno; bool inject_once; /* Decides if aio_readv/writev fails right away (true) or returns an error * return value only in the callback (false) */ bool inject_immediately; + +QLIST_HEAD(list, blkdebug_rule) rules[BLKDBG_EVENT_MAX]; } BDRVBlkdebugState; struct callback_data { @@ -46,16 +49,210 @@ struct callback_data { void *opaque; }; +enum { +ACTION_INJECT_ERROR, +ACTION_SET_STATE, +}; + +struct blkdebug_rule { +BlkDebugEvent event; +int action; +int state; +union { +struct { +int error; +int immediately; +int once; +} inject; +struct { +int new_state; +} set_state; +} options; +QLIST_ENTRY(blkdebug_rule) next; +}; + +static QemuOptsList inject_error_opts = { +.name = inject-error, +.head = QTAILQ_HEAD_INITIALIZER(inject_error_opts.head), +.desc = { +{ +.name = event, +.type = QEMU_OPT_STRING, +}, +{ +.name = state, +.type = QEMU_OPT_NUMBER, +}, +{ +.name = errno, +.type = QEMU_OPT_NUMBER, +}, +{ +.name = once, +.type = QEMU_OPT_BOOL, +}, +{ +.name = immediately, +.type = QEMU_OPT_BOOL, +}, +{ /* end of list */ } +}, +}; + +static QemuOptsList set_state_opts = { +.name = set-state, +.head = QTAILQ_HEAD_INITIALIZER(inject_error_opts.head), +.desc = { +{ +.name = event, +.type = QEMU_OPT_STRING, +}, +{ +.name = state, +.type = QEMU_OPT_NUMBER, +}, +{ +.name = new_state, +.type = QEMU_OPT_NUMBER, +}, +{ /* end of list */ } +}, +}; + +static QemuOptsList *config_groups[] = { +inject_error_opts, +set_state_opts, +NULL +}; + +static const char *event_names[BLKDBG_EVENT_MAX] = { +}; + +static int get_event_by_name(const char *name, BlkDebugEvent *event) +{ +int i; + +for (i = 0; i BLKDBG_EVENT_MAX; i++) { +if (!strcmp(event_names[i], name)) { +*event = i; +return 0; +} +} + +return -1; +} + +struct add_rule_data { +BDRVBlkdebugState *s; +int action; +}; + +static int add_rule(QemuOpts *opts, void *opaque) +{ +struct add_rule_data *d = opaque; +BDRVBlkdebugState *s = d-s; +const char* event_name; +BlkDebugEvent event; +struct blkdebug_rule *rule; + +/* Find the right event for the rule */ +event_name = qemu_opt_get(opts, event); +if (!event_name || get_event_by_name(event_name, event) 0) { +return -1; +} + +/* Set attributes common for all actions */ +rule =
[Qemu-devel] [RFC][PATCH 7/7] qcow2: Trigger blkdebug events
This adds blkdebug events to qcow2 to allow injecting I/O errors in specific places. Signed-off-by: Kevin Wolf kw...@redhat.com --- block.h | 19 +++ block/blkdebug.c | 17 + block/qcow2-cluster.c | 15 +++ 3 files changed, 51 insertions(+), 0 deletions(-) diff --git a/block.h b/block.h index 2fd8361..c0f17aa 100644 --- a/block.h +++ b/block.h @@ -210,6 +210,25 @@ int64_t bdrv_get_dirty_count(BlockDriverState *bs); typedef enum { +BLKDBG_L1_UPDATE, + +BLKDBG_L1_GROW_ALLOC_TABLE, +BLKDBG_L1_GROW_WRITE_TABLE, +BLKDBG_L1_GROW_ACTIVATE_TABLE, + +BLKDBG_L2_LOAD, +BLKDBG_L2_UPDATE, +BLKDBG_L2_UPDATE_COMPRESSED, +BLKDBG_L2_ALLOC_COW_READ, +BLKDBG_L2_ALLOC_WRITE, + +BLKDBG_READ, +BLKDBG_READ_BACKING, +BLKDBG_READ_COMPRESSED, + +BLKDBG_COW_READ, +BLKDBG_COW_WRITE, + BLKDBG_EVENT_MAX, } BlkDebugEvent; diff --git a/block/blkdebug.c b/block/blkdebug.c index 22b1768..2398975 100644 --- a/block/blkdebug.c +++ b/block/blkdebug.c @@ -126,6 +126,23 @@ static QemuOptsList *config_groups[] = { }; static const char *event_names[BLKDBG_EVENT_MAX] = { +[BLKDBG_L1_UPDATE] = l1_update, +[BLKDBG_L1_GROW_ALLOC_TABLE]= l1_grow.alloc_table, +[BLKDBG_L1_GROW_WRITE_TABLE]= l1_grow.write_table, +[BLKDBG_L1_GROW_ACTIVATE_TABLE] = l1_grow.activate_table, + +[BLKDBG_L2_LOAD]= l2_load, +[BLKDBG_L2_UPDATE] = l2_update, +[BLKDBG_L2_UPDATE_COMPRESSED] = l2_update_compressed, +[BLKDBG_L2_ALLOC_COW_READ] = l2_alloc.cow_read, +[BLKDBG_L2_ALLOC_WRITE] = l2_alloc.write, + +[BLKDBG_READ] = read, +[BLKDBG_READ_BACKING] = read_backing, +[BLKDBG_READ_COMPRESSED]= read_compressed, + +[BLKDBG_COW_READ] = cow_read, +[BLKDBG_COW_WRITE] = cow_write, }; static int get_event_by_name(const char *name, BlkDebugEvent *event) diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c index b13b693..9b3d686 100644 --- a/block/qcow2-cluster.c +++ b/block/qcow2-cluster.c @@ -54,12 +54,14 @@ int qcow2_grow_l1_table(BlockDriverState *bs, int min_size) memcpy(new_l1_table, s-l1_table, s-l1_size * sizeof(uint64_t)); /* write new table (align to cluster) */ +BLKDBG_EVENT(s-hd, BLKDBG_L1_GROW_ALLOC_TABLE); new_l1_table_offset = qcow2_alloc_clusters(bs, new_l1_size2); if (new_l1_table_offset 0) { qemu_free(new_l1_table); return new_l1_table_offset; } +BLKDBG_EVENT(s-hd, BLKDBG_L1_GROW_WRITE_TABLE); for(i = 0; i s-l1_size; i++) new_l1_table[i] = cpu_to_be64(new_l1_table[i]); ret = bdrv_pwrite(s-hd, new_l1_table_offset, new_l1_table, new_l1_size2); @@ -69,6 +71,7 @@ int qcow2_grow_l1_table(BlockDriverState *bs, int min_size) new_l1_table[i] = be64_to_cpu(new_l1_table[i]); /* set new table */ +BLKDBG_EVENT(s-hd, BLKDBG_L1_GROW_ACTIVATE_TABLE); cpu_to_be32w((uint32_t*)data, new_l1_size); cpu_to_be64w((uint64_t*)(data + 4), new_l1_table_offset); ret = bdrv_pwrite(s-hd, offsetof(QCowHeader, l1_size), data,sizeof(data)); @@ -170,6 +173,8 @@ static uint64_t *l2_load(BlockDriverState *bs, uint64_t l2_offset) min_index = l2_cache_new_entry(bs); l2_table = s-l2_cache + (min_index s-l2_bits); + +BLKDBG_EVENT(s-hd, BLKDBG_L2_LOAD); if (bdrv_pread(s-hd, l2_offset, l2_table, s-l2_size * sizeof(uint64_t)) != s-l2_size * sizeof(uint64_t)) return NULL; @@ -195,6 +200,7 @@ static int write_l1_entry(BDRVQcowState *s, int l1_index) buf[i] = cpu_to_be64(s-l1_table[l1_start_index + i]); } +BLKDBG_EVENT(s-hd, BLKDBG_L1_UPDATE); if (bdrv_pwrite(s-hd, s-l1_table_offset + 8 * l1_start_index, buf, sizeof(buf)) != sizeof(buf)) { @@ -248,12 +254,14 @@ static uint64_t *l2_allocate(BlockDriverState *bs, int l1_index) memset(l2_table, 0, s-l2_size * sizeof(uint64_t)); } else { /* if there was an old l2 table, read it from the disk */ +BLKDBG_EVENT(s-hd, BLKDBG_L2_ALLOC_COW_READ); if (bdrv_pread(s-hd, old_l2_offset, l2_table, s-l2_size * sizeof(uint64_t)) != s-l2_size * sizeof(uint64_t)) return NULL; } /* write the l2 table to the file */ +BLKDBG_EVENT(s-hd, BLKDBG_L2_ALLOC_WRITE); if (bdrv_pwrite(s-hd, l2_offset, l2_table, s-l2_size * sizeof(uint64_t)) != s-l2_size * sizeof(uint64_t)) @@ -335,6 +343,7 @@ static int qcow_read(BlockDriverState *bs, int64_t sector_num, /* read from the base image */ n1 = qcow2_backing_read1(bs-backing_hd, sector_num, buf, n); if (n1 0) { +BLKDBG_EVENT(s-hd, BLKDBG_READ_BACKING); ret = bdrv_read(bs-backing_hd, sector_num, buf,
[Qemu-devel] [PATCH] resource leak fixes for iwmmxt disassemble
This patch fixes few resource leaks in the iwmmxt disassemble. Signed-off-by: Lars Munch l...@segv.dk --- target-arm/translate.c |2 ++ 1 files changed, 2 insertions(+), 0 deletions(-) diff --git a/target-arm/translate.c b/target-arm/translate.c index cdfe946..2ab7881 100644 --- a/target-arm/translate.c +++ b/target-arm/translate.c @@ -1131,6 +1131,7 @@ static inline TCGv iwmmxt_load_creg(int reg) static inline void iwmmxt_store_creg(int reg, TCGv var) { tcg_gen_st_i32(var, cpu_env, offsetof(CPUState, iwmmxt.cregs[reg])); +dead_tmp(var); } static inline void gen_op_iwmmxt_movq_wRn_M0(int rn) @@ -1415,6 +1416,7 @@ static int disas_iwmmxt_insn(CPUState *env, DisasContext *s, uint32_t insn) } } } +dead_tmp(addr); return 0; } -- 1.7.0
Re: [Qemu-devel] [PATCH, RFC] Replace assert(0) with abort() or cpu_abort()
On 3/15/10, Markus Armbruster arm...@redhat.com wrote: Blue Swirl blauwir...@gmail.com writes: When building with -DNDEBUG, assert(0) will not stop execution so it must not be used for abnormal termination. For each case: are you sure the code does not recover after assert(0)? Not saying it does, just asking whether you checked. I had not checked but did just now, QEMU system emulators do not handle SIGABRT. The situation is different for user emulator, but then assert(0) and abort() would be equally correct or incorrect. Use cpu_abort() when in CPU context, abort() otherwise. I sympathize with the general idea, but I don't like dead code after abort(). What about cleaning that up? Good idea, but it should be a separate patch. This patch is safe, whereas the cleanup patch could cause problems if it's not done carefully.
Re: [Qemu-devel] [RFC][PATCH 0/7] blkdebug
On 3/15/10, Kevin Wolf kw...@redhat.com wrote: This patch series introduces a new block driver which acts as a protocol and whose purpose it is to fail requests. To be more precise, I want it to fail in configurable places, so that qemu-iotests can be extended with tests for the error paths (for example for the case when something with metadata writes goes wrong deep in qcow2). Nice. Do you think this could be extended (later) to inject errors also to guest from QEMU? Then it would be nice to be able to inject errors when reading/writing to a specific guest block range and for other formats (raw etc.) too.
Re: [Qemu-devel] [RFC][PATCH 0/7] blkdebug
Am 15.03.2010 18:40, schrieb Blue Swirl: On 3/15/10, Kevin Wolf kw...@redhat.com wrote: This patch series introduces a new block driver which acts as a protocol and whose purpose it is to fail requests. To be more precise, I want it to fail in configurable places, so that qemu-iotests can be extended with tests for the error paths (for example for the case when something with metadata writes goes wrong deep in qcow2). Nice. Do you think this could be extended (later) to inject errors also to guest from QEMU? Not sure what you mean by from QEMU? To allow injecting errors from the monitor instead of based on rules? Sounds certainly doable. Or do you just mean to make it work in qemu as opposed to qemu-io? This really doesn't make a difference to the block layer. If it works in one, it works in the other one, too. Then it would be nice to be able to inject errors when reading/writing to a specific guest block range and for other formats (raw etc.) too. Should work with all formats, it's implemented as a protocol (but it's untested with everything but qcow2). So you basically stick it between the format driver and the image file. The only thing that could be needed for other formats is some additional events. For the block range thing, this would need to implement some more conditions instead of just checking the state, but in general this is exactly the kind of things that I'd like to see supported eventually. So I completely agree that this is the right direction for future improvements, however I can't tell yet if (or when) I'll get to implement it myself. My focus is really the qcow2 error paths for now. Kevin
[Qemu-devel] git clone --recursive fails on git://git.qemu.org/qemu.git
I tried to grab a recursive clone of qemu.git in order to get the head of the seabios and vgabios trees to match the head of qemu.git, but the operation failed with $ git clone --recursive git://git.qemu.org/qemu.git qemu-r Initialized empty Git repository in /home/chris/git/qemu-r/.git/ remote: Counting objects: 59824, done. remote: Compressing objects: 100% (15786/15786), done. remote: Total 59824 (delta 47532), reused 55283 (delta 43936) Receiving objects: 100% (59824/59824), 24.67 MiB | 292 KiB/s, done. Resolving deltas: 100% (47532/47532), done. Submodule 'roms/seabios' (git://git.qemu.org/seabios.git/) registered for path 'roms/seabios' Submodule 'roms/vgabios' (git://git.qemu.org/vgabios.git/) registered for path 'roms/vgabios' Initialized empty Git repository in /home/chris/git/qemu-r/roms/seabios/.git/ remote: Counting objects: 4711, done. remote: Compressing objects: 100% (1259/1259), done. remote: Total 4711 (delta 3758), reused 4360 (delta 3450) Receiving objects: 100% (4711/4711), 1.04 MiB | 266 KiB/s, done. Resolving deltas: 100% (3758/3758), done. fatal: reference is not a tree: 8f469b9676127ba6bb52609d89ec774e61db0ee1 Unable to checkout '8f469b9676127ba6bb52609d89ec774e61db0ee1' in submodule path 'roms/seabios' Is this a problem with qemu.git, or am I just failing to drive git properly? Cheers, Chris.
Re: [Qemu-devel] [RFC][PATCH 0/7] blkdebug
On 3/15/10, Kevin Wolf kw...@redhat.com wrote: Am 15.03.2010 18:40, schrieb Blue Swirl: On 3/15/10, Kevin Wolf kw...@redhat.com wrote: This patch series introduces a new block driver which acts as a protocol and whose purpose it is to fail requests. To be more precise, I want it to fail in configurable places, so that qemu-iotests can be extended with tests for the error paths (for example for the case when something with metadata writes goes wrong deep in qcow2). Nice. Do you think this could be extended (later) to inject errors also to guest from QEMU? Not sure what you mean by from QEMU? To allow injecting errors from the monitor instead of based on rules? Sounds certainly doable. I was thinking rule file, but monitor would be more flexible. Even better: allow switching rule files from monitor :-). Or do you just mean to make it work in qemu as opposed to qemu-io? This really doesn't make a difference to the block layer. If it works in one, it works in the other one, too. Then it would be nice to be able to inject errors when reading/writing to a specific guest block range and for other formats (raw etc.) too. Should work with all formats, it's implemented as a protocol (but it's untested with everything but qcow2). So you basically stick it between the format driver and the image file. The only thing that could be needed for other formats is some additional events. For the block range thing, this would need to implement some more conditions instead of just checking the state, but in general this is exactly the kind of things that I'd like to see supported eventually. So I completely agree that this is the right direction for future improvements, however I can't tell yet if (or when) I'll get to implement it myself. My focus is really the qcow2 error paths for now. Ok, it's enough to know at this stage that the architecture is flexible for this.
[Qemu-devel] Re: [PATCH, RFC] Replace assert(0) with abort() or cpu_abort()
I sympathize with the general idea, but I don't like dead code after abort(). What about cleaning that up? Good idea, but it should be a separate patch. This patch is safe, whereas the cleanup patch could cause problems if it's not done carefully. This patch is safe, however I'd consider not changing assert(0)-abort() if there is code after the assert that looks like an attempt at recovering. Example: if (!p) { printf (the impossible has happened!); assert (0); } return p-q; should be changed to abort, while if (!p) { printf (the impossible has happened!); assert (0); return 0; } return p-q; should not. Paolo
[Qemu-devel] Re: [PATCH, RFC] Replace assert(0) with abort() or cpu_abort()
On 3/15/10, Paolo Bonzini pbonz...@redhat.com wrote: I sympathize with the general idea, but I don't like dead code after abort(). What about cleaning that up? Good idea, but it should be a separate patch. This patch is safe, whereas the cleanup patch could cause problems if it's not done carefully. This patch is safe, however I'd consider not changing assert(0)-abort() if there is code after the assert that looks like an attempt at recovering. Example: if (!p) { printf (the impossible has happened!); assert (0); } return p-q; should be changed to abort, while if (!p) { printf (the impossible has happened!); assert (0); return 0; } return p-q; should not. Why not? According to manual page, assert(x) is equal to if (!x) abort(). As I mentioned earlier, system emulators don't handle SIGABRT and for user emulators the level of bugginess remains constant (or rather, it no longer depends on NDEBUG). Therefore the recovery code will never be executed.
[Qemu-devel] Re: [PATCH, RFC] Replace assert(0) with abort() or cpu_abort()
Paolo Bonzini pbonz...@redhat.com writes: I sympathize with the general idea, but I don't like dead code after abort(). What about cleaning that up? Good idea, but it should be a separate patch. This patch is safe, whereas the cleanup patch could cause problems if it's not done carefully. This patch is safe, however I'd consider not changing assert(0)-abort() if there is code after the assert that looks like an attempt at recovering. Example: if (!p) { printf (the impossible has happened!); assert (0); } return p-q; should be changed to abort, while if (!p) { printf (the impossible has happened!); assert (0); return 0; } return p-q; should not. Except when you find that the recovery attempt is insufficient, of course. Requires closer inspection.
[Qemu-devel] Re: [PATCH, RFC] Replace assert(0) with abort() or cpu_abort()
I'd consider not changing assert(0)-abort() if there is code after the assert that looks like an attempt at recovering. Example: if (!p) { printf (the impossible has happened!); assert (0); } return p-q; should be changed to abort, while if (!p) { printf (the impossible has happened!); assert (0); return 0; } return p-q; should not. Why not? According to manual page, assert(x) is equal to if (!x) abort(). As I mentioned earlier, system emulators don't handle SIGABRT ... which won't be generated if !NDEBUG. Only if the recovery code makes sense, of course. However, my point was that those cases where there is recovery code are not no-brainers. Paolo
Re: [Qemu-devel] [PATCH, RFC] Replace assert(0) with abort() or cpu_abort()
Blue Swirl blauwir...@gmail.com writes: On 3/15/10, Markus Armbruster arm...@redhat.com wrote: Blue Swirl blauwir...@gmail.com writes: When building with -DNDEBUG, assert(0) will not stop execution so it must not be used for abnormal termination. For each case: are you sure the code does not recover after assert(0)? Not saying it does, just asking whether you checked. I had not checked but did just now, QEMU system emulators do not handle SIGABRT. The situation is different for user emulator, but then assert(0) and abort() would be equally correct or incorrect. Please don't tell me that user emulators make abort() return. abort() is declared __noreturn__, and the optimizer may well rely on that. Use cpu_abort() when in CPU context, abort() otherwise. I sympathize with the general idea, but I don't like dead code after abort(). What about cleaning that up? Good idea, but it should be a separate patch. This patch is safe, whereas the cleanup patch could cause problems if it's not done carefully. I support keeping separate things separate. However, separating something like if (mapping-first_mapping_index != first_mapping_index mapping-info.file.offset 0) { - assert(0); +abort(); copy_it = 1; } from if (mapping-first_mapping_index != first_mapping_index mapping-info.file.offset 0) { abort(); - copy_it = 1; } doesn't seem worth it.
Re: [Qemu-devel] git clone --recursive fails on git://git.qemu.org/qemu.git
On 03/15/2010 01:16 PM, Chris Webb wrote: I tried to grab a recursive clone of qemu.git in order to get the head of the seabios and vgabios trees to match the head of qemu.git, but the operation failed with $ git clone --recursive git://git.qemu.org/qemu.git qemu-r Initialized empty Git repository in /home/chris/git/qemu-r/.git/ remote: Counting objects: 59824, done. remote: Compressing objects: 100% (15786/15786), done. remote: Total 59824 (delta 47532), reused 55283 (delta 43936) Receiving objects: 100% (59824/59824), 24.67 MiB | 292 KiB/s, done. Resolving deltas: 100% (47532/47532), done. Submodule 'roms/seabios' (git://git.qemu.org/seabios.git/) registered for path 'roms/seabios' Submodule 'roms/vgabios' (git://git.qemu.org/vgabios.git/) registered for path 'roms/vgabios' Initialized empty Git repository in /home/chris/git/qemu-r/roms/seabios/.git/ remote: Counting objects: 4711, done. remote: Compressing objects: 100% (1259/1259), done. remote: Total 4711 (delta 3758), reused 4360 (delta 3450) Receiving objects: 100% (4711/4711), 1.04 MiB | 266 KiB/s, done. Resolving deltas: 100% (3758/3758), done. fatal: reference is not a tree: 8f469b9676127ba6bb52609d89ec774e61db0ee1 Unable to checkout '8f469b9676127ba6bb52609d89ec774e61db0ee1' in submodule path 'roms/seabios' Is this a problem with qemu.git, or am I just failing to drive git properly? Should work now. Sorry about that. Regards, Anthony Liguori Cheers, Chris.
[Qemu-devel] Re: [PATCH, RFC] Replace assert(0) with abort() or cpu_abort()
On 3/15/10, Paolo Bonzini pbonz...@redhat.com wrote: I'd consider not changing assert(0)-abort() if there is code after the assert that looks like an attempt at recovering. Example: if (!p) { printf (the impossible has happened!); assert (0); } return p-q; should be changed to abort, while if (!p) { printf (the impossible has happened!); assert (0); return 0; } return p-q; should not. Why not? According to manual page, assert(x) is equal to if (!x) abort(). As I mentioned earlier, system emulators don't handle SIGABRT ... which won't be generated if !NDEBUG. Only if the recovery code makes sense, of course. However, my point was that those cases where there is recovery code are not no-brainers. Except that compiling with -DNDEBUG was broken and fixed only recently with a6c6f76ceb95a0986fd1a36cc30f8241734d20c3. Thus I suspect nobody uses -DNDEBUG for production builds and the code paths after assert(0) are untested.
Re: [Qemu-devel] [PATCH 1/7] Add support for generic notifier lists
On 03/11/2010 08:19 AM, Paul Brook wrote: On 03/11/2010 06:57 AM, Paul Brook wrote: +struct QEMUNotifier +{ +void (*notify)(QEMUNotifier *notifier); +}; I suggest combining this with QEMUBH. I take it your not opposed to converting QEMUBH to be a QEMUNotifier? If so, I'm happy to do it. It's unclear to me why you've invented a new thing in the first place. It's a better approximation of the command pattern because the command is a single object as opposed to a tuple. Because the command is an object, you can also do things like binding. For instance: Which now gives you a notifier that has an fd bound to it's second argument (which is pretty useful for IO dispatch). You can do this with a tuple representation, but it gets awkward. One could argue for formalizing the tuple as a struct but extending by nesting becomes more complicated. Also, for the most part, you already have a state for the command and embedding the object means less dynamic memory allocation and less code to handle that. Regards, Anthony Liguori
[Qemu-devel] [PATCH 1/7] Add support for generic notifier lists (v2)
Notifiers are data-less callbacks and a notifier list is a list of registered notifiers that all are interested in a particular event. We'll use this in a few patches to implement mouse change notification. Signed-off-by: Anthony Liguori aligu...@us.ibm.com --- v1 - v2 - Do not do memory allocations by placing list nodes in notifier --- Makefile.objs |1 + notify.c | 39 +++ notify.h | 43 +++ 3 files changed, 83 insertions(+), 0 deletions(-) create mode 100644 notify.c create mode 100644 notify.h diff --git a/Makefile.objs b/Makefile.objs index e791dd5..dcb5a92 100644 --- a/Makefile.objs +++ b/Makefile.objs @@ -104,6 +104,7 @@ common-obj-$(CONFIG_VNC_TLS) += vnc-tls.o vnc-auth-vencrypt.o common-obj-$(CONFIG_VNC_SASL) += vnc-auth-sasl.o common-obj-$(CONFIG_COCOA) += cocoa.o common-obj-$(CONFIG_IOTHREAD) += qemu-thread.o +common-obj-y += notify.o slirp-obj-y = cksum.o if.o ip_icmp.o ip_input.o ip_output.o slirp-obj-y += slirp.o mbuf.o misc.o sbuf.o socket.o tcp_input.o tcp_output.o diff --git a/notify.c b/notify.c new file mode 100644 index 000..bcd3fc5 --- /dev/null +++ b/notify.c @@ -0,0 +1,39 @@ +/* + * Notifier lists + * + * Copyright IBM, Corp. 2010 + * + * Authors: + * Anthony Liguori aligu...@us.ibm.com + * + * This work is licensed under the terms of the GNU GPL, version 2. See + * the COPYING file in the top-level directory. + * + */ + +#include qemu-common.h +#include notify.h + +void notifier_list_init(NotifierList *list) +{ +QTAILQ_INIT(list-notifiers); +} + +void notifier_list_add(NotifierList *list, Notifier *notifier) +{ +QTAILQ_INSERT_HEAD(list-notifiers, notifier, node); +} + +void notifier_list_remove(NotifierList *list, Notifier *notifier) +{ +QTAILQ_REMOVE(list-notifiers, notifier, node); +} + +void notifier_list_notify(NotifierList *list) +{ +Notifier *notifier, *next; + +QTAILQ_FOREACH_SAFE(notifier, list-notifiers, node, next) { +notifier-notify(notifier); +} +} diff --git a/notify.h b/notify.h new file mode 100644 index 000..b40522f --- /dev/null +++ b/notify.h @@ -0,0 +1,43 @@ +/* + * Notifier lists + * + * Copyright IBM, Corp. 2010 + * + * Authors: + * Anthony Liguori aligu...@us.ibm.com + * + * This work is licensed under the terms of the GNU GPL, version 2. See + * the COPYING file in the top-level directory. + * + */ + +#ifndef QEMU_NOTIFY_H +#define QEMU_NOTIFY_H + +#include qemu-queue.h + +typedef struct Notifier Notifier; + +struct Notifier +{ +void (*notify)(Notifier *notifier); +QTAILQ_ENTRY(Notifier) node; +}; + +typedef struct NotifierList +{ +QTAILQ_HEAD(, Notifier) notifiers; +} NotifierList; + +#define NOTIFIER_LIST_INITIALIZER(head) \ +{ QTAILQ_HEAD_INITIALIZER((head).notifiers) } + +void notifier_list_init(NotifierList *list); + +void notifier_list_add(NotifierList *list, Notifier *notifier); + +void notifier_list_remove(NotifierList *list, Notifier *notifier); + +void notifier_list_notify(NotifierList *list); + +#endif -- 1.6.5.2
[Qemu-devel] [PATCH 3/7] Add kbd_mouse_has_absolute()
kbd_mouse_is_absolute tells us whether the current mouse handler is an absolute device. kbd_mouse_has_absolute tells us whether we have any device that is capable of absolute input. This lets us tell a user that they have configured an absolute device but that the guest is not currently using it. Signed-off-by: Anthony Liguori aligu...@us.ibm.com --- console.h |5 + input.c | 13 + 2 files changed, 18 insertions(+), 0 deletions(-) diff --git a/console.h b/console.h index 94d9cae..f3c619f 100644 --- a/console.h +++ b/console.h @@ -53,8 +53,13 @@ void qemu_remove_led_event_handler(QEMUPutLEDEntry *entry); void kbd_put_keycode(int keycode); void kbd_put_ledstate(int ledstate); void kbd_mouse_event(int dx, int dy, int dz, int buttons_state); + +/* Does the current mouse generate absolute events */ int kbd_mouse_is_absolute(void); +/* Of all the mice, is there one that generates absolute events */ +int kbd_mouse_has_absolute(void); + struct MouseTransformInfo { /* Touchscreen resolution */ int x; diff --git a/input.c b/input.c index 397bfc5..8d5a14d 100644 --- a/input.c +++ b/input.c @@ -153,6 +153,19 @@ int kbd_mouse_is_absolute(void) return QTAILQ_FIRST(mouse_handlers)-qemu_put_mouse_event_absolute; } +int kbd_mouse_has_absolute(void) +{ +QEMUPutMouseEntry *entry; + +QTAILQ_FOREACH(entry, mouse_handlers, node) { +if (entry-qemu_put_mouse_event_absolute) { +return 1; +} +} + +return 0; +} + static void info_mice_iter(QObject *data, void *opaque) { QDict *mouse; -- 1.6.5.2
[Qemu-devel] [PATCH 2/7] Rewrite mouse handlers to use QTAILQ and to have an activation function
And convert usb-hid to use it (to avoid regression with bisection) Right now, when we do info mice and we've added a usb tablet, we don't see it until the guest starts using the tablet. We implement this behavior in order to provide a means to delay registration of a mouse handler since we treat the last registered handler as the current handler. This is a usability problem though as we would like to give the user feedback that they've either 1) not added an absolute device 2) there is an absolute device but the guest isn't using it 3) we have an absolute device and it's active. By using QTAILQ and having an explicit activation function that moves the handler to the front of the queue, we can implement the same semantics as before with respect to automatically switching to usb tablet while providing the user with a whole lot more information. Signed-off-by: Anthony Liguori aligu...@us.ibm.com --- console.h|6 +++- hw/usb-hid.c | 15 ++-- input.c | 109 +++--- 3 files changed, 59 insertions(+), 71 deletions(-) diff --git a/console.h b/console.h index 71e8ff2..94d9cae 100644 --- a/console.h +++ b/console.h @@ -28,8 +28,10 @@ typedef struct QEMUPutMouseEntry { int qemu_put_mouse_event_absolute; char *qemu_put_mouse_event_name; +int index; + /* used internally by qemu for handling mice */ -struct QEMUPutMouseEntry *next; +QTAILQ_ENTRY(QEMUPutMouseEntry) node; } QEMUPutMouseEntry; typedef struct QEMUPutLEDEntry { @@ -43,6 +45,8 @@ QEMUPutMouseEntry *qemu_add_mouse_event_handler(QEMUPutMouseEvent *func, void *opaque, int absolute, const char *name); void qemu_remove_mouse_event_handler(QEMUPutMouseEntry *entry); +void qemu_activate_mouse_event_handler(QEMUPutMouseEntry *entry); + QEMUPutLEDEntry *qemu_add_led_event_handler(QEMUPutLEDEvent *func, void *opaque); void qemu_remove_led_event_handler(QEMUPutLEDEntry *entry); diff --git a/hw/usb-hid.c b/hw/usb-hid.c index 2e4e647..8e6c6e0 100644 --- a/hw/usb-hid.c +++ b/hw/usb-hid.c @@ -511,8 +511,7 @@ static int usb_mouse_poll(USBHIDState *hs, uint8_t *buf, int len) USBMouseState *s = hs-ptr; if (!s-mouse_grabbed) { - s-eh_entry = qemu_add_mouse_event_handler(usb_mouse_event, hs, - 0, QEMU USB Mouse); +qemu_activate_mouse_event_handler(s-eh_entry); s-mouse_grabbed = 1; } @@ -553,8 +552,7 @@ static int usb_tablet_poll(USBHIDState *hs, uint8_t *buf, int len) USBMouseState *s = hs-ptr; if (!s-mouse_grabbed) { - s-eh_entry = qemu_add_mouse_event_handler(usb_tablet_event, hs, - 1, QEMU USB Tablet); +qemu_activate_mouse_event_handler(s-eh_entry); s-mouse_grabbed = 1; } @@ -866,6 +864,15 @@ static int usb_hid_initfn(USBDevice *dev, int kind) USBHIDState *s = DO_UPCAST(USBHIDState, dev, dev); s-dev.speed = USB_SPEED_FULL; s-kind = kind; + +if (s-kind == USB_MOUSE) { +s-ptr.eh_entry = qemu_add_mouse_event_handler(usb_mouse_event, s, + 0, QEMU USB Mouse); +} else if (s-kind == USB_TABLET) { +s-ptr.eh_entry = qemu_add_mouse_event_handler(usb_tablet_event, s, + 1, QEMU USB Tablet); +} + /* Force poll routine to be run and grab input the first time. */ s-changed = 1; return 0; diff --git a/input.c b/input.c index baaa4c6..397bfc5 100644 --- a/input.c +++ b/input.c @@ -31,9 +31,9 @@ static QEMUPutKBDEvent *qemu_put_kbd_event; static void *qemu_put_kbd_event_opaque; -static QEMUPutMouseEntry *qemu_put_mouse_event_head; -static QEMUPutMouseEntry *qemu_put_mouse_event_current; static QTAILQ_HEAD(, QEMUPutLEDEntry) led_handlers = QTAILQ_HEAD_INITIALIZER(led_handlers); +static QTAILQ_HEAD(, QEMUPutMouseEntry) mouse_handlers = +QTAILQ_HEAD_INITIALIZER(mouse_handlers); void qemu_add_kbd_event_handler(QEMUPutKBDEvent *func, void *opaque) { @@ -45,7 +45,8 @@ QEMUPutMouseEntry *qemu_add_mouse_event_handler(QEMUPutMouseEvent *func, void *opaque, int absolute, const char *name) { -QEMUPutMouseEntry *s, *cursor; +QEMUPutMouseEntry *s; +static int mouse_index = 0; s = qemu_mallocz(sizeof(QEMUPutMouseEntry)); @@ -53,51 +54,24 @@ QEMUPutMouseEntry *qemu_add_mouse_event_handler(QEMUPutMouseEvent *func, s-qemu_put_mouse_event_opaque = opaque; s-qemu_put_mouse_event_absolute = absolute; s-qemu_put_mouse_event_name = qemu_strdup(name); -s-next = NULL; +s-index = mouse_index++; -if (!qemu_put_mouse_event_head) { -qemu_put_mouse_event_head = qemu_put_mouse_event_current
[Qemu-devel] [PATCH 7/7] sdl: use mouse mode notifier
Today we poll the mouse mode whenever there is a mouse movement. There is a subtle usability problem with this though. If we're in relative mode and grab is enabled, when we change to absolute mode, we break grab. This gives a user a seamless transition when the new pointer is enabled. But because we poll for mouse change, this grab break won't occur until the user attempts to move the mouse. By using notifiers, the grab break happens as soon as possible. Signed-off-by: Anthony Liguori aligu...@us.ibm.com --- sdl.c | 31 --- 1 files changed, 20 insertions(+), 11 deletions(-) diff --git a/sdl.c b/sdl.c index 1d1e100..e9edbcc 100644 --- a/sdl.c +++ b/sdl.c @@ -57,6 +57,7 @@ static SDL_Cursor *guest_sprite = NULL; static uint8_t allocator; static SDL_PixelFormat host_format; static int scaling_active = 0; +static Notifier mouse_mode_notifier; static void sdl_update(DisplayState *ds, int x, int y, int w, int h) { @@ -485,6 +486,22 @@ static void sdl_grab_end(void) sdl_update_caption(); } +static void sdl_mouse_mode_change(Notifier *notify) +{ +if (kbd_mouse_is_absolute()) { +if (!absolute_enabled) { +sdl_hide_cursor(); +if (gui_grab) { +sdl_grab_end(); +} +absolute_enabled = 1; +} +} else if (absolute_enabled) { + sdl_show_cursor(); + absolute_enabled = 0; +} +} + static void sdl_send_mouse_event(int dx, int dy, int dz, int x, int y, int state) { int buttons; @@ -497,19 +514,8 @@ static void sdl_send_mouse_event(int dx, int dy, int dz, int x, int y, int state buttons |= MOUSE_EVENT_MBUTTON; if (kbd_mouse_is_absolute()) { - if (!absolute_enabled) { - sdl_hide_cursor(); - if (gui_grab) { - sdl_grab_end(); - } - absolute_enabled = 1; - } - dx = x * 0x7FFF / (width - 1); dy = y * 0x7FFF / (height - 1); -} else if (absolute_enabled) { - sdl_show_cursor(); - absolute_enabled = 0; } else if (guest_cursor) { x -= guest_x; y -= guest_y; @@ -875,6 +881,9 @@ void sdl_display_init(DisplayState *ds, int full_screen, int no_frame) dpy_resize(ds); } +mouse_mode_notifier.notify = sdl_mouse_mode_change; +qemu_add_mouse_mode_change_notifier(mouse_mode_notifier); + sdl_update_caption(); SDL_EnableKeyRepeat(250, 50); gui_grab = 0; -- 1.6.5.2
[Qemu-devel] [PATCH 6/7] input: make vnc use mouse mode notifiers
When we switch to absolute mode, we send out a notification (if the client supports it). Today, we only send this notification when the client sends us a mouse event and we're in the wrong mode. Signed-off-by: Anthony Liguori aligu...@us.ibm.com --- vnc.c | 13 - vnc.h |2 ++ 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/vnc.c b/vnc.c index 7ce73dc..14ded6c 100644 --- a/vnc.c +++ b/vnc.c @@ -1110,6 +1110,7 @@ static void vnc_disconnect_finish(VncState *vs) dcl-idle = 1; } +qemu_remove_mouse_mode_change_notifier(vs-mouse_mode_notifier); vnc_remove_timer(vs-vd); qemu_remove_led_event_handler(vs-led); qemu_free(vs); @@ -1427,8 +1428,11 @@ static void client_cut_text(VncState *vs, size_t len, uint8_t *text) { } -static void check_pointer_type_change(VncState *vs, int absolute) +static void check_pointer_type_change(Notifier *notifier) { +VncState *vs = container_of(notifier, VncState, mouse_mode_notifier); +int absolute = kbd_mouse_is_absolute(); + if (vnc_has_feature(vs, VNC_FEATURE_POINTER_TYPE_CHANGE) vs-absolute != absolute) { vnc_write_u8(vs, 0); vnc_write_u8(vs, 0); @@ -1474,8 +1478,6 @@ static void pointer_event(VncState *vs, int button_mask, int x, int y) vs-last_x = x; vs-last_y = y; } - -check_pointer_type_change(vs, kbd_mouse_is_absolute()); } static void reset_keys(VncState *vs) @@ -1814,8 +1816,6 @@ static void set_encodings(VncState *vs, int32_t *encodings, size_t n_encodings) break; } } - -check_pointer_type_change(vs, kbd_mouse_is_absolute()); } static void set_pixel_conversion(VncState *vs) @@ -2431,6 +2431,9 @@ static void vnc_connect(VncDisplay *vd, int csock) reset_keys(vs); vs-led = qemu_add_led_event_handler(kbd_leds, vs); +vs-mouse_mode_notifier.notify = check_pointer_type_change; +qemu_add_mouse_mode_change_notifier(vs-mouse_mode_notifier); + vnc_init_timer(vd); /* vs might be free()ed here */ diff --git a/vnc.h b/vnc.h index 0fc89bd..8052000 100644 --- a/vnc.h +++ b/vnc.h @@ -167,6 +167,8 @@ struct VncState Buffer zlib_tmp; z_stream zlib_stream[4]; +Notifier mouse_mode_notifier; + QTAILQ_ENTRY(VncState) next; }; -- 1.6.5.2
[Qemu-devel] [PATCH 4/7] Add notifier for mouse mode changes
Right now, DisplayState clients rely on polling the mouse mode to determine when the device is changed to an absolute device. Use a notification list to add an explicit notification. Signed-off-by: Anthony Liguori aligu...@us.ibm.com --- console.h |3 +++ input.c | 37 - 2 files changed, 39 insertions(+), 1 deletions(-) diff --git a/console.h b/console.h index f3c619f..6def115 100644 --- a/console.h +++ b/console.h @@ -3,6 +3,7 @@ #include qemu-char.h #include qdict.h +#include notify.h /* keyboard/mouse support */ @@ -56,6 +57,8 @@ void kbd_mouse_event(int dx, int dy, int dz, int buttons_state); /* Does the current mouse generate absolute events */ int kbd_mouse_is_absolute(void); +void qemu_add_mouse_mode_change_notifier(Notifier *notify); +void qemu_remove_mouse_mode_change_notifier(Notifier *notify); /* Of all the mice, is there one that generates absolute events */ int kbd_mouse_has_absolute(void); diff --git a/input.c b/input.c index 8d5a14d..c956e06 100644 --- a/input.c +++ b/input.c @@ -28,12 +28,13 @@ #include console.h #include qjson.h - static QEMUPutKBDEvent *qemu_put_kbd_event; static void *qemu_put_kbd_event_opaque; static QTAILQ_HEAD(, QEMUPutLEDEntry) led_handlers = QTAILQ_HEAD_INITIALIZER(led_handlers); static QTAILQ_HEAD(, QEMUPutMouseEntry) mouse_handlers = QTAILQ_HEAD_INITIALIZER(mouse_handlers); +static NotifierList mouse_mode_notifiers = +NOTIFIER_LIST_INITIALIZER(mouse_mode_notifiers); void qemu_add_kbd_event_handler(QEMUPutKBDEvent *func, void *opaque) { @@ -41,6 +42,24 @@ void qemu_add_kbd_event_handler(QEMUPutKBDEvent *func, void *opaque) qemu_put_kbd_event = func; } +static void check_mode_change(void) +{ +static int current_is_absolute, current_has_absolute; +int is_absolute; +int has_absolute; + +is_absolute = kbd_mouse_is_absolute(); +has_absolute = kbd_mouse_has_absolute(); + +if (is_absolute != current_is_absolute || +has_absolute != current_has_absolute) { +notifier_list_notify(mouse_mode_notifiers); +} + +current_is_absolute = is_absolute; +current_has_absolute = has_absolute; +} + QEMUPutMouseEntry *qemu_add_mouse_event_handler(QEMUPutMouseEvent *func, void *opaque, int absolute, const char *name) @@ -58,6 +77,8 @@ QEMUPutMouseEntry *qemu_add_mouse_event_handler(QEMUPutMouseEvent *func, QTAILQ_INSERT_TAIL(mouse_handlers, s, node); +check_mode_change(); + return s; } @@ -75,6 +96,8 @@ void qemu_remove_mouse_event_handler(QEMUPutMouseEntry *entry) qemu_free(entry-qemu_put_mouse_event_name); qemu_free(entry); + +check_mode_change(); } QEMUPutLEDEntry *qemu_add_led_event_handler(QEMUPutLEDEvent *func, @@ -256,4 +279,16 @@ void do_mouse_set(Monitor *mon, const QDict *qdict) if (!found) { monitor_printf(mon, Mouse at given index not found\n); } + +check_mode_change(); +} + +void qemu_add_mouse_mode_change_notifier(Notifier *notify) +{ +notifier_list_add(mouse_mode_notifiers, notify); +} + +void qemu_remove_mouse_mode_change_notifier(Notifier *notify) +{ +notifier_list_remove(mouse_mode_notifiers, notify); } -- 1.6.5.2
[Qemu-devel] [PATCH 5/7] Expose whether a mouse is an absolute device via QMP and the human monitor.
For QMP, we just add an attribute which is backwards compatible. For the human monitor, we add (absolute) to the end of the line. Signed-off-by: Anthony Liguori aligu...@us.ibm.com --- input.c | 18 -- 1 files changed, 12 insertions(+), 6 deletions(-) diff --git a/input.c b/input.c index c956e06..8f0941e 100644 --- a/input.c +++ b/input.c @@ -195,9 +195,10 @@ static void info_mice_iter(QObject *data, void *opaque) Monitor *mon = opaque; mouse = qobject_to_qdict(data); -monitor_printf(mon, %c Mouse #% PRId64 : %s\n, +monitor_printf(mon, %c Mouse #% PRId64 : %s%s\n, (qdict_get_bool(mouse, current) ? '*' : ' '), - qdict_get_int(mouse, index), qdict_get_str(mouse, name)); + qdict_get_int(mouse, index), qdict_get_str(mouse, name), + qdict_get_bool(mouse, absolute) ? (absolute) : ); } void do_info_mice_print(Monitor *mon, const QObject *data) @@ -224,11 +225,12 @@ void do_info_mice_print(Monitor *mon, const QObject *data) * - name: mouse's name * - index: mouse's index * - current: true if this mouse is receiving events, false otherwise + * - absolute: true if the mouse generates absolute input events * * Example: * - * [ { name: QEMU Microsoft Mouse, index: 0, current: false }, - * { name: QEMU PS/2 Mouse, index: 1, current: true } ] + * [ { name: QEMU Microsoft Mouse, index: 0, current: false, absolute: false }, + * { name: QEMU PS/2 Mouse, index: 1, current: true, absolute: true } ] */ void do_info_mice(Monitor *mon, QObject **ret_data) { @@ -246,10 +248,14 @@ void do_info_mice(Monitor *mon, QObject **ret_data) QTAILQ_FOREACH(cursor, mouse_handlers, node) { QObject *obj; -obj = qobject_from_jsonf({ 'name': %s, 'index': %d, 'current': %i }, +obj = qobject_from_jsonf({ 'name': %s, + 'index': %d, + 'current': %i, + 'absolute': %i }, cursor-qemu_put_mouse_event_name, cursor-index, - cursor-index == current); + cursor-index == current, + !!cursor-qemu_put_mouse_event_absolute); qlist_append_obj(mice_list, obj); } -- 1.6.5.2
Re: [Qemu-devel] [PATCH 1/7] Add support for generic notifier lists
On 03/15/2010 03:31 PM, Anthony Liguori wrote: On 03/11/2010 08:19 AM, Paul Brook wrote: On 03/11/2010 06:57 AM, Paul Brook wrote: +struct QEMUNotifier +{ +void (*notify)(QEMUNotifier *notifier); +}; I suggest combining this with QEMUBH. I take it your not opposed to converting QEMUBH to be a QEMUNotifier? If so, I'm happy to do it. It's unclear to me why you've invented a new thing in the first place. It's a better approximation of the command pattern because the command is a single object as opposed to a tuple. Because the command is an object, you can also do things like binding. For instance: typedef struct IONotifier { Notifier parent; void (*notify)(IONotifier *notifier, int fd); int fd; } IONotifier; It's been a long day... Which now gives you a notifier that has an fd bound to it's second argument (which is pretty useful for IO dispatch). You can do this with a tuple representation, but it gets awkward. One could argue for formalizing the tuple as a struct but extending by nesting becomes more complicated. Also, for the most part, you already have a state for the command and embedding the object means less dynamic memory allocation and less code to handle that. Regards, Anthony Liguori Regards, Anthony Liguori
Re: [Qemu-devel] Ideas wiki for GSoC 2010
On Mon, Mar 15, 2010 at 02:25:41PM +0200, Avi Kivity wrote: On 03/10/2010 11:30 PM, Luiz Capitulino wrote: Hi there, Our wiki page for the Summer of Code 2010 is doing quite well: http://wiki.qemu.org/Google_Summer_of_Code_2010 I will add another project - iommu emulation. Could be very useful for doing device assignment to nested guests, which could make testing a lot easier. Good idea. If there is interest I could help to mentor this project. Joerg
Re: [Qemu-devel] Ideas wiki for GSoC 2010
On Mon, Mar 15, 2010 at 02:25:41PM +0200, Avi Kivity wrote: On 03/10/2010 11:30 PM, Luiz Capitulino wrote: Hi there, Our wiki page for the Summer of Code 2010 is doing quite well: http://wiki.qemu.org/Google_Summer_of_Code_2010 I will add another project - iommu emulation. Could be very useful for doing device assignment to nested guests, which could make testing a lot easier. Our experiments show that nested device assignment is pretty much required for I/O performance in nested scenarios. Cheers, Muli
Re: [Qemu-devel] Ideas wiki for GSoC 2010
On Mon, Mar 15, 2010 at 05:53:13AM -0700, Muli Ben-Yehuda wrote: On Mon, Mar 15, 2010 at 02:25:41PM +0200, Avi Kivity wrote: On 03/10/2010 11:30 PM, Luiz Capitulino wrote: Hi there, Our wiki page for the Summer of Code 2010 is doing quite well: http://wiki.qemu.org/Google_Summer_of_Code_2010 I will add another project - iommu emulation. Could be very useful for doing device assignment to nested guests, which could make testing a lot easier. Our experiments show that nested device assignment is pretty much required for I/O performance in nested scenarios. Really? I did a small test with virtio-blk in a nested guest (disk read with dd, so not a real benchmark) and got a reasonable read-performance of around 25MB/s from the disk in the l2-guest. Joerg
Re: [Qemu-devel] Ideas wiki for GSoC 2010
On Mon, Mar 15, 2010 at 03:11:42PM +0200, Avi Kivity wrote: On 03/15/2010 03:03 PM, Joerg Roedel wrote: I will add another project - iommu emulation. Could be very useful for doing device assignment to nested guests, which could make testing a lot easier. Our experiments show that nested device assignment is pretty much required for I/O performance in nested scenarios. Really? I did a small test with virtio-blk in a nested guest (disk read with dd, so not a real benchmark) and got a reasonable read-performance of around 25MB/s from the disk in the l2-guest. Your guest wasn't doing a zillion VMREADs and VMWRITEs every exit. I plan to reduce VMREAD/VMWRITE overhead for kvm, but not much we can do for other guests. Does it matter for the ept-on-ept case? The initial patchset of nested-vmx implemented it and they reported a performance drop of around 12% between levels which is reasonable. So I expected the loss of io-performance for l2 also reasonable in this case. My small measurement was also done using npt-on-npt. Joerg
Re: [Qemu-devel] Ideas wiki for GSoC 2010
On Mon, Mar 15, 2010 at 02:03:11PM +0100, Joerg Roedel wrote: On Mon, Mar 15, 2010 at 05:53:13AM -0700, Muli Ben-Yehuda wrote: On Mon, Mar 15, 2010 at 02:25:41PM +0200, Avi Kivity wrote: On 03/10/2010 11:30 PM, Luiz Capitulino wrote: Hi there, Our wiki page for the Summer of Code 2010 is doing quite well: http://wiki.qemu.org/Google_Summer_of_Code_2010 I will add another project - iommu emulation. Could be very useful for doing device assignment to nested guests, which could make testing a lot easier. Our experiments show that nested device assignment is pretty much required for I/O performance in nested scenarios. Really? I did a small test with virtio-blk in a nested guest (disk read with dd, so not a real benchmark) and got a reasonable read-performance of around 25MB/s from the disk in the l2-guest. Netperf running in L1 with direct access: ~950 Mbps throughput with 25% CPU utilization. Netperf running in L2 with virtio between L2 and L1 and direct assignment between L1 and L0: roughly the same throughput, but over 90% CPU utilization! Now extrapolate to 10GbE. Cheers, Muli
Re: [Qemu-devel] Ideas wiki for GSoC 2010
On Mon, Mar 15, 2010 at 08:14:29AM -0500, Anthony Liguori wrote: On 03/15/2010 07:42 AM, Avi Kivity wrote: On 03/15/2010 02:38 PM, Joerg Roedel wrote: On Mon, Mar 15, 2010 at 02:25:41PM +0200, Avi Kivity wrote: On 03/10/2010 11:30 PM, Luiz Capitulino wrote: Hi there, Our wiki page for the Summer of Code 2010 is doing quite well: http://wiki.qemu.org/Google_Summer_of_Code_2010 I will add another project - iommu emulation. Could be very useful for doing device assignment to nested guests, which could make testing a lot easier. Good idea. If there is interest I could help to mentor this project. Thanks. I volunteered Anthony, but he may be a little overcommitted. Joerg, feel free to put your name against too. [x] Done. Thanks, Joerg
Re: [Qemu-devel] Ideas wiki for GSoC 2010
On 03/15/2010 10:06 AM, Avi Kivity wrote: On 03/15/2010 03:23 PM, Anthony Liguori wrote: On 03/15/2010 08:11 AM, Avi Kivity wrote: On 03/15/2010 03:03 PM, Joerg Roedel wrote: I will add another project - iommu emulation. Could be very useful for doing device assignment to nested guests, which could make testing a lot easier. Our experiments show that nested device assignment is pretty much required for I/O performance in nested scenarios. Really? I did a small test with virtio-blk in a nested guest (disk read with dd, so not a real benchmark) and got a reasonable read-performance of around 25MB/s from the disk in the l2-guest. Your guest wasn't doing a zillion VMREADs and VMWRITEs every exit. I plan to reduce VMREAD/VMWRITE overhead for kvm, but not much we can do for other guests. VMREAD/VMWRITEs are generally optimized by hypervisors as they tend to be costly. KVM is a bit unusual in terms of how many times the instructions are executed per exit. Do you know offhand of any unnecessary read/writes? There's update_cr8_intercept(), but on normal exits, I don't see what else we can remove. Yeah, there are a number of examples. vmcs_clear_bits() and vmcs_set_bits() read a field of the VMCS and then immediately writes it. This is unnecessary as the same information could be kept in a shadow variable. In vmx_fpu_activate, we call vmcs_clear_bits() followed immediately by vmcs_set_bits(). which means we're reading GUEST_CR0 twice and writing it twice. vmx_get_rflags() reads from the VMCS and we frequently call get_rflags() followed by a set_rflags() to update a bit. We also don't cache the value between calls and there's a few spots in the code that make multiple calls. Regards, Anthony Liguori
Re: [Qemu-devel] Ideas wiki for GSoC 2010
On 03/16/2010 03:21 AM, Anthony Liguori wrote: On 03/15/2010 10:06 AM, Avi Kivity wrote: On 03/15/2010 03:23 PM, Anthony Liguori wrote: On 03/15/2010 08:11 AM, Avi Kivity wrote: On 03/15/2010 03:03 PM, Joerg Roedel wrote: I will add another project - iommu emulation. Could be very useful for doing device assignment to nested guests, which could make testing a lot easier. Our experiments show that nested device assignment is pretty much required for I/O performance in nested scenarios. Really? I did a small test with virtio-blk in a nested guest (disk read with dd, so not a real benchmark) and got a reasonable read-performance of around 25MB/s from the disk in the l2-guest. Your guest wasn't doing a zillion VMREADs and VMWRITEs every exit. I plan to reduce VMREAD/VMWRITE overhead for kvm, but not much we can do for other guests. VMREAD/VMWRITEs are generally optimized by hypervisors as they tend to be costly. KVM is a bit unusual in terms of how many times the instructions are executed per exit. Do you know offhand of any unnecessary read/writes? There's update_cr8_intercept(), but on normal exits, I don't see what else we can remove. Yeah, there are a number of examples. vmcs_clear_bits() and vmcs_set_bits() read a field of the VMCS and then immediately writes it. This is unnecessary as the same information could be kept in a shadow variable. In vmx_fpu_activate, we call vmcs_clear_bits() followed immediately by vmcs_set_bits(). which means we're reading GUEST_CR0 twice and writing it twice. This should be much better these days (2.6.34-rc1) as vmx_fpu_activate() is called at most once per heavyweight exit (and I have evil plans to reduce it even further). Still, that code should be optimized. vmx_get_rflags() reads from the VMCS and we frequently call get_rflags() followed by a set_rflags() to update a bit. We also don't cache the value between calls and there's a few spots in the code that make multiple calls. We definitely should cache that (and segment access from the emulator as well). But I'd have thought this to be relatively infrequent. At least with Linux, using x2apic and virtio allows you to eliminate most emulator access, if you have npt or ept. -- Do not meddle in the internals of kernels, for they are subtle and quick to panic.