[Qemu-devel] [PATCH 5/6] linux-user: remove duplicate statement
Signed-off-by: Prasad Joshi prasadjoshi.li...@gmail.com --- linux-user/signal.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/linux-user/signal.c b/linux-user/signal.c index e5fb933..7d6246f 100644 --- a/linux-user/signal.c +++ b/linux-user/signal.c @@ -4043,8 +4043,6 @@ static void setup_rt_frame(int sig, struct target_sigaction *ka, struct target_rt_sigframe *frame; abi_ulong info_addr, uc_addr; -frame_addr = get_sigframe(ka, env, sizeof *frame); - frame_addr = get_sigframe(ka, env, sizeof(*frame)); if (!lock_user_struct(VERIFY_WRITE, frame, frame_addr, 0)) { goto give_sigsegv; -- 1.8.1.2
[Qemu-devel] [PATCH 1/6] audio: set top level latch for each slot
CSMKeyControll function is supposed to set the top level latch for each slot. However, at the moment, it incorrectly updates only the first slot. Patch fixes the problem. Signed-off-by: Prasad Joshi prasadjoshi.li...@gmail.com --- hw/audio/fmopl.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/audio/fmopl.c b/hw/audio/fmopl.c index 290a224..eb60c12 100644 --- a/hw/audio/fmopl.c +++ b/hw/audio/fmopl.c @@ -725,7 +725,7 @@ INLINE void CSMKeyControll(OPL_CH *CH) OPL_KEYOFF(slot2); /* total level latch */ slot1-TLL = slot1-TL + (CH-ksl_baseslot1-ksl); - slot1-TLL = slot1-TL + (CH-ksl_baseslot1-ksl); + slot2-TLL = slot2-TL + (CH-ksl_baseslot2-ksl); /* key on */ CH-op1_out[0] = CH-op1_out[1] = 0; OPL_KEYON(slot1); -- 1.8.1.2
[Qemu-devel] [PATCH 2/6] intc/openpic_kvm: fix MemListener delete regiion callback function
Signed-off-by: Prasad Joshi prasadjoshi.li...@gmail.com --- hw/intc/openpic_kvm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/intc/openpic_kvm.c b/hw/intc/openpic_kvm.c index 87fdb12..afa604d 100644 --- a/hw/intc/openpic_kvm.c +++ b/hw/intc/openpic_kvm.c @@ -200,7 +200,7 @@ static void kvm_openpic_realize(DeviceState *dev, Error **errp) qdev_init_gpio_in(dev, kvm_openpic_set_irq, OPENPIC_MAX_IRQ); opp-mem_listener.region_add = kvm_openpic_region_add; -opp-mem_listener.region_add = kvm_openpic_region_del; +opp-mem_listener.region_del = kvm_openpic_region_del; memory_listener_register(opp-mem_listener, address_space_memory); /* indicate pic capabilities */ -- 1.8.1.2
[Qemu-devel] [PATCH 3/6] pcnet: remove duplicate assignment
Signed-off-by: Prasad Joshi prasadjoshi.li...@gmail.com --- hw/net/pcnet.c | 1 - 1 file changed, 1 deletion(-) diff --git a/hw/net/pcnet.c b/hw/net/pcnet.c index 7cb47b3..ebe5057 100644 --- a/hw/net/pcnet.c +++ b/hw/net/pcnet.c @@ -718,7 +718,6 @@ static void pcnet_s_reset(PCNetState *s) s-csr[94] = 0x; s-csr[100] = 0x0200; s-csr[103] = 0x0105; -s-csr[103] = 0x0105; s-csr[112] = 0x; s-csr[114] = 0x; s-csr[122] = 0x; -- 1.8.1.2
[Qemu-devel] [PATCH 6/6] net: netmap_poll must update both read/write poll state
Signed-off-by: Prasad Joshi prasadjoshi.li...@gmail.com --- net/netmap.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/net/netmap.c b/net/netmap.c index 8213304..0c1772b 100644 --- a/net/netmap.c +++ b/net/netmap.c @@ -177,8 +177,8 @@ static void netmap_poll(NetClientState *nc, bool enable) NetmapState *s = DO_UPCAST(NetmapState, nc, nc); if (s-read_poll != enable || s-write_poll != enable) { -s-read_poll = enable; -s-read_poll = enable; +s-write_poll = enable; +s-read_poll = enable; netmap_update_fd_handler(s); } } -- 1.8.1.2
[Qemu-devel] [PATCH 4/6] hw/timer/grlib_gptimer: remove unnecessary assignment
Signed-off-by: Prasad Joshi prasadjoshi.li...@gmail.com --- hw/timer/grlib_gptimer.c | 1 - 1 file changed, 1 deletion(-) diff --git a/hw/timer/grlib_gptimer.c b/hw/timer/grlib_gptimer.c index 7672d3a..d655bb2 100644 --- a/hw/timer/grlib_gptimer.c +++ b/hw/timer/grlib_gptimer.c @@ -328,7 +328,6 @@ static void grlib_gptimer_reset(DeviceState *d) unit-scaler = 0; unit-reload = 0; -unit-config = 0; unit-config = unit-nr_timers; unit-config |= unit-irq_line 3; -- 1.8.1.2
Re: [Qemu-devel] [PATCH for-2.0 V3] tests/acpi-test: do not run iasl on big endian machines
On Fri, Mar 21, 2014 at 12:16:53AM +0100, Paolo Bonzini wrote: Il 20/03/2014 23:33, Marcel Apfelbaum ha scritto: I've seen something like that somewhere, but I didn't quite like it. I was looking for something more elegant as I was *almost* sure this kind of solution will not pass the reviews :) But maybe I'll try this, let's see what happens, If all you're looking for is bigendian (disabling iasl disassembly on bigendian makes sense), your patch v2 is fine. Assembling ASL on bigendian is supported by at least Fedora and Debian (and hence Ubuntu). Paolo At this point I'm confused. If iasl compiler is broken, we should detect and fix that. It might be ok to just detect endian-ness as a quick work-around. BTW configure already has code to detect endian-ness: if test $bigendian = yes ; then echo HOST_WORDS_BIGENDIAN=y $config_host_mak fi If only the dis-assembler is broken, we should only detect that when running tests. Using expected files for this should be fine. -- MST
Re: [Qemu-devel] [PATCH for-2.0 V3] tests/acpi-test: do not run iasl on big endian machines
On Thu, Mar 20, 2014 at 10:17:14PM +, Peter Maydell wrote: On 20 March 2014 22:06, Marcel Apfelbaum marce...@redhat.com wrote: On Thu, 2014-03-20 at 22:57 +0100, Paolo Bonzini wrote: Il 20/03/2014 22:14, Marcel Apfelbaum ha scritto: +# All known versions of iasl on BE machines are broken. +# TODO: add detection code once a non-broken version makes an appearance. +if ($iasl -h /dev/null 21) + (lscpu | grep Byte Order | grep --quiet Little Endian ); then lscpu is not portable. I am open to suggestions... echo trivial iasl source | iasl --compile-options | iasl --disassemble-options | grep error Fill in the handwaving with actual syntax ;-) thanks -- PMM True but whatever test we write, it's possible that a buggy iasl passes it by luck. Let's wait for a working iasl to appear on some BE machines. When that happens, we'll be able to write a robust detection script. -- MST
Re: [Qemu-devel] [PATCH for-2.0 V3] tests/acpi-test: do not run iasl on big endian machines
On Thu, Mar 20, 2014 at 11:03:04PM +, Peter Maydell wrote: On 20 March 2014 22:41, Marcel Apfelbaum marce...@redhat.com wrote: On Thu, 2014-03-20 at 22:17 +, Peter Maydell wrote: echo trivial iasl source | iasl --compile-options | iasl --disassemble-options | grep error Fill in the handwaving with actual syntax ;-) Problem with this solution is that if we start from the source, it will compile into a *wrong* AML (e.g header length will be BE and not LE), then the disassemble will *succeed* (!!!). However, the expected AML file will be in the right format and fail :(. Well, grep for 'some integer that comes out wrong due to the iasl bug', if it doesn't always result in an error like the case you reported to the iasl mailing list. You need to write this code at some point because we do need to be able to configure-time detect whether we have a working iasl or a broken one, once fixed ones get out into the wild. It doesn't seem like it ought to be that hard to just do it right to start with... thanks -- PMM Hard to predict the future: it's possible that some distros will ship partially broken iasl on BE. We really need to see a working one to be sure ... -- MST
Re: [Qemu-devel] [PATCH for-2.0 V3] tests/acpi-test: do not run iasl on big endian machines
On 23 March 2014 09:49, Michael S. Tsirkin m...@redhat.com wrote: At this point I'm confused. If iasl compiler is broken, we should detect and fix that. It might be ok to just detect endian-ness as a quick work-around. BTW configure already has code to detect endian-ness: if test $bigendian = yes ; then echo HOST_WORDS_BIGENDIAN=y $config_host_mak fi That's the endianness of the machine we're compiling QEMU for, not the endianness of the machine we're compiling QEMU on. If for instance you're on x86_64 cross-compiling for PPC then HOST_WORDS_BIGENDIAN is true, but the iasl you use in the build process will be running on a little endian machine. thanks -- PMM
Re: [Qemu-devel] [PATCH for-2.0 V3] tests/acpi-test: do not run iasl on big endian machines
On Fri, 2014-03-21 at 00:16 +0100, Paolo Bonzini wrote: Il 20/03/2014 23:33, Marcel Apfelbaum ha scritto: I've seen something like that somewhere, but I didn't quite like it. I was looking for something more elegant as I was *almost* sure this kind of solution will not pass the reviews :) But maybe I'll try this, let's see what happens, If all you're looking for is bigendian (disabling iasl disassembly on bigendian makes sense), your patch v2 is fine. Thanks Paolo, I thought so too, but Michael NACKED it having a valid point: We should check this at configure time... . Anyway, if only iasl dis-assembler has issues, this can be seen as acpi test's problem and handled inside the test. Thanks, Marcel Assembling ASL on bigendian is supported by at least Fedora and Debian (and hence Ubuntu). Paolo
Re: [Qemu-devel] [PATCH for-2.0 V3] tests/acpi-test: do not run iasl on big endian machines
On 23 March 2014 09:52, Michael S. Tsirkin m...@redhat.com wrote: Hard to predict the future: it's possible that some distros will ship partially broken iasl on BE. We really need to see a working one to be sure ... Well at the moment we have a known broken iasl, and we have a known OK iasl (in the sense that we know what the correct behaviour is, it should match what it does on LE hosts). So we can write a test that distinguishes the two. In the future if somebody fixes half the bugs for some reason we will then be able to tighten up the test if we have to. But I think a test that correctly distinguishes the two cases we have currently is better than one which marks them both as broken. thanks -- PMM
Re: [Qemu-devel] [PATCH for-2.0 V3] tests/acpi-test: do not run iasl on big endian machines
On Sun, 2014-03-23 at 12:14 +, Peter Maydell wrote: On 23 March 2014 09:49, Michael S. Tsirkin m...@redhat.com wrote: At this point I'm confused. If iasl compiler is broken, we should detect and fix that. It might be ok to just detect endian-ness as a quick work-around. BTW configure already has code to detect endian-ness: if test $bigendian = yes ; then echo HOST_WORDS_BIGENDIAN=y $config_host_mak fi That's the endianness of the machine we're compiling QEMU for, not the endianness of the machine we're compiling QEMU on. If for instance you're on x86_64 cross-compiling for PPC then HOST_WORDS_BIGENDIAN is true, but the iasl you use in the build process will be running on a little endian machine. Hi Peter, are you sure about this? I saw the 'target_bigendian' that does what you described above. $bigendian is the result of a little C program that checks *host's* endian-ness. Of course I might have missed something. By the way, considering what Paolo said that the iasl compiler does work for BE machines (the dis-assembler has the problem), leads me to see this issue as the test's problem. So I am going to leave the 'configure' with no modifications and check inside the test itself that the expected files can be disassembled. Thanks, Marcel thanks -- PMM
Re: [Qemu-devel] [PATCH for-2.0 V3] tests/acpi-test: do not run iasl on big endian machines
On 23 March 2014 12:32, Marcel Apfelbaum marce...@redhat.com wrote: On Sun, 2014-03-23 at 12:14 +, Peter Maydell wrote: On 23 March 2014 09:49, Michael S. Tsirkin m...@redhat.com wrote: At this point I'm confused. If iasl compiler is broken, we should detect and fix that. It might be ok to just detect endian-ness as a quick work-around. BTW configure already has code to detect endian-ness: if test $bigendian = yes ; then echo HOST_WORDS_BIGENDIAN=y $config_host_mak fi That's the endianness of the machine we're compiling QEMU for, not the endianness of the machine we're compiling QEMU on. If for instance you're on x86_64 cross-compiling for PPC then HOST_WORDS_BIGENDIAN is true, but the iasl you use in the build process will be running on a little endian machine. Hi Peter, are you sure about this? I saw the 'target_bigendian' that does what you described above. $bigendian is the result of a little C program that checks *host's* endian-ness. Of course I might have missed something. host for QEMU means the machine QEMU will run on (as opposed to target meaning the machine QEMU is emulating). Those can both be different from the machine you're building on. Example: you can be on an x86_64 machine cross-compiling a qemu-system-mips intended to run on PPC hosts: build system: x86_64 (we don't currently try to identify its endianness) host system: PPC ($bigendian is set to endianness) target system: mips ($target_bigendian is set to endianness) bigendian is set by cross-compiling a test object, which produces a PPC object file that we then examine to see which endianness the PPC system we're building for is. target_bigendian is set for the target machine by just hard-coding it -- in this case it would be set because 'mips' is a bigendian target. The iasl we use in the build process is the x86_64 native binary, so neither $bigendian nor $target_bigendian are correct. thanks -- PMM
Re: [Qemu-devel] [PATCH for-2.0 V3] tests/acpi-test: do not run iasl on big endian machines
Il 23/03/2014 13:32, Marcel Apfelbaum ha scritto: That's the endianness of the machine we're compiling QEMU for, not the endianness of the machine we're compiling QEMU on. If for instance you're on x86_64 cross-compiling for PPC then HOST_WORDS_BIGENDIAN is true, but the iasl you use in the build process will be running on a little endian machine. Hi Peter, are you sure about this? I saw the 'target_bigendian' that does what you described above. $bigendian is the result of a little C program that checks *host's* endian-ness. Of course I might have missed something. Peter is right. There are three systems: build (what you run on), host (what you compile for), target (always x86-64 for now for acpi-test). $bigendian and G_BYTE_ORDER check the host system. If you wanted to test the build system's endianness, Laszlo's suggestion would be fine. However, Michael is also right if we assume that iasl on bigendian can assemble, and that the problems are only on disassembling. This is because in this particular case you'll be running an executable compiled for the host and you know that host == build. I think the assumption is sane. As far as I remember, upstream iasl doesn't compile at all on big-endian machines. You need specific patches such as those shared by Fedora and Debian's. Unfortunately, the patches are incomplete, they only fix assembling because that's what was needed so far to cross-compile SeaBIOS. Paolo
Re: [Qemu-devel] [PATCH for-2.0 V3] tests/acpi-test: do not run iasl on big endian machines
On Sun, 2014-03-23 at 12:48 +, Peter Maydell wrote: On 23 March 2014 12:32, Marcel Apfelbaum marce...@redhat.com wrote: On Sun, 2014-03-23 at 12:14 +, Peter Maydell wrote: On 23 March 2014 09:49, Michael S. Tsirkin m...@redhat.com wrote: At this point I'm confused. If iasl compiler is broken, we should detect and fix that. It might be ok to just detect endian-ness as a quick work-around. BTW configure already has code to detect endian-ness: if test $bigendian = yes ; then echo HOST_WORDS_BIGENDIAN=y $config_host_mak fi That's the endianness of the machine we're compiling QEMU for, not the endianness of the machine we're compiling QEMU on. If for instance you're on x86_64 cross-compiling for PPC then HOST_WORDS_BIGENDIAN is true, but the iasl you use in the build process will be running on a little endian machine. Hi Peter, are you sure about this? I saw the 'target_bigendian' that does what you described above. $bigendian is the result of a little C program that checks *host's* endian-ness. Of course I might have missed something. host for QEMU means the machine QEMU will run on (as opposed to target meaning the machine QEMU is emulating). Those can both be different from the machine you're building on. Example: you can be on an x86_64 machine cross-compiling a qemu-system-mips intended to run on PPC hosts: build system: x86_64 (we don't currently try to identify its endianness) host system: PPC ($bigendian is set to endianness) target system: mips ($target_bigendian is set to endianness) bigendian is set by cross-compiling a test object, which produces a PPC object file that we then examine to see which endianness the PPC system we're building for is. target_bigendian is set for the target machine by just hard-coding it -- in this case it would be set because 'mips' is a bigendian target. The iasl we use in the build process is the x86_64 native binary, so neither $bigendian nor $target_bigendian are correct. Thanks for the detailed answer! It really helped! I missed the build machine != host machine != target machine :( Marcel thanks -- PMM
Re: [Qemu-devel] [PATCH for-2.0 V3] tests/acpi-test: do not run iasl on big endian machines
On Sun, 2014-03-23 at 13:51 +0100, Paolo Bonzini wrote: Il 23/03/2014 13:32, Marcel Apfelbaum ha scritto: That's the endianness of the machine we're compiling QEMU for, not the endianness of the machine we're compiling QEMU on. If for instance you're on x86_64 cross-compiling for PPC then HOST_WORDS_BIGENDIAN is true, but the iasl you use in the build process will be running on a little endian machine. Hi Peter, are you sure about this? I saw the 'target_bigendian' that does what you described above. $bigendian is the result of a little C program that checks *host's* endian-ness. Of course I might have missed something. Peter is right. There are three systems: build (what you run on), host (what you compile for), target (always x86-64 for now for acpi-test). $bigendian and G_BYTE_ORDER check the host system. If you wanted to test the build system's endianness, Laszlo's suggestion would be fine. However, Michael is also right if we assume that iasl on bigendian can assemble, and that the problems are only on disassembling. This is because in this particular case you'll be running an executable compiled for the host and you know that host == build. I think the assumption is sane. As far as I remember, upstream iasl doesn't compile at all on big-endian machines. You need specific patches such as those shared by Fedora and Debian's. Unfortunately, the patches are incomplete, they only fix assembling because that's what was needed so far to cross-compile SeaBIOS. Hi Paolo, thanks for the help! I need a decision here :) 1. If iasl *does* work for the scenarios used by Qemu until now, I can conclude that the only part that suffers from it is the test itself, because it dis-assembles AML code. In this case I can handle it in the test itself by checking the endian-ness (V2 already posted), or trying to dis-assemble the expected AML file and not failing the test if this fails. 2. If iasl causes problems to run a x86_64 guest if one or both build/host machines are BE, we need to tweak the configuration file. After this discussion it seems that (2.) is not the case, I think maybe re-sending: [PATCH for-2.0 V2] tests/acpi-test: do not run iasl on big endian machines Is everyone OK with this? Thanks, Marcel Paolo
Re: [Qemu-devel] [PATCH 4/5] hw/9pfs: use g_strdup_printf() instead of PATH_MAX limitation
On 03/16/2014 09:32 PM, Chen Gang wrote: On 03/08/2014 09:58 PM, Chen Gang wrote: OK, thanks. Next, I will/should continue to analyse the performance issue for 9pfs when users drop into a long directory path under bash shell. After have a test, I am sure it is not 9pfs issue, either not Qemu's issue, it's Linux kernel vfs or block sub-systems' issue. The related test environments (originally, our 9pfs is upper on ext4): - for ext4 file system under my Fedora laptop (Qemu does not start). - for ntfs file system under my Fedora laptop (Qemu does not start). - for ext4 file system under my Ubuntu in Qemu. For a very long file name (e.g. 3K long), all of them are very very slow. (and I also tested the ext2 /boot partition under Ubuntu in Qemu, it is not slow, I guess the reson is its partition size is small). Next, I will/shall communicate with upstream kernel for it. :-) Although I am not quite sure, hope I can find the root cause within month (2014-03-31). Welcome any suggestions, discussions, and completions for it. Thanks. Sorry, after give a little more test, for the lower performance issue under a long deep path, 'bash' is the direct cause (may also be root cause). Also sorry, 'bash' is out of my focusing border now, so I provide the related information below, welcome any members (e.g. 'bash', fedora, or ubuntu members) to help check, when they have time, thanks. Environments (e.g. fedora 17): [root@gchen ~]# uname -a Linux gchen 3.14.0-rc7-next-20140321 #2 SMP Sun Mar 23 19:46:37 CST 2014 x86_64 x86_64 x86_64 GNU/Linux [root@gchen ~]# cat /etc/*-release Fedora release 17 (Beefy Miracle) NAME=Fedora VERSION=17 (Beefy Miracle) ID=fedora VERSION_ID=17 PRETTY_NAME=Fedora 17 (Beefy Miracle) ANSI_COLOR=0;34 CPE_NAME=cpe:/o:fedoraproject:fedora:17 Fedora release 17 (Beefy Miracle) Fedora release 17 (Beefy Miracle) 'bash' is very busy under user mode (I am only one cpu): top - 20:12:01 up 23 min, 5 users, load average: 0.60, 0.35, 0.33 Tasks: 137 total, 2 running, 135 sleeping, 0 stopped, 0 zombie Cpu(s): 97.1%us, 2.9%sy, 0.0%ni, 0.0%id, 0.0%wa, 0.0%hi, 0.0%si, 0.0%st Mem: 1942468k total, 820896k used, 1121572k free,40060k buffers Swap:0k total,0k used,0k free, 446328k cached PID USER PR NI VIRT RES SHR S %CPU %MEMTIME+ COMMAND 4010 root 20 0 113m 5236 3096 R 92.3 0.3 3:57.19 -bash 2955 root 20 0 136m 21m 10m S 2.9 1.1 0:16.00 /usr/bin/Xorg :0 -background none -logverbose 7 -auth /var/run/gdm/auth-for-gdm-8 3860 gchen 20 0 582m 28m 19m S 1.9 1.5 0:22.93 gnome-terminal 3348 gchen 20 0 1405m 117m 46m S 1.0 6.2 0:23.34 /usr/bin/gnome-shell 3728 gchen 20 0 442m 8248 6140 S 1.0 0.4 0:12.05 /usr/bin/ibus-daemon -r --xim ... The related 'bash' stack is below (each time, always drop into it): #0 0x0036bb290c1c in strcoll_l () from /lib64/libc.so.6 #1 0x0047ff48 in ?? () #2 0x00480517 in ?? () #3 0x00480fb7 in ?? () #4 0x004806e0 in ?? () #5 0x00480fb7 in ?? () #6 0x004806e0 in ?? () #7 0x00482528 in xstrmatch () #8 0x00456f79 in binary_test () #9 0x0042f308 in ?? () #10 0x0042f449 in ?? () #11 0x00432379 in execute_command_internal () #12 0x0043583e in execute_command () #13 0x0043295e in execute_command_internal () #14 0x0043349f in execute_command_internal () #15 0x0043583e in execute_command () #16 0x00433464 in execute_command_internal () #17 0x0043583e in execute_command () #18 0x00433464 in execute_command_internal () #19 0x0043583e in execute_command () #20 0x00433464 in execute_command_internal () #21 0x0043583e in execute_command () #22 0x00433464 in execute_command_internal () #23 0x0043583e in execute_command () #24 0x00433464 in execute_command_internal () #25 0x0043583e in execute_command () #26 0x00433464 in execute_command_internal () #27 0x00432613 in execute_command_internal () #28 0x0043440e in ?? () #29 0x00431a8c in ?? () #30 0x00432873 in execute_command_internal () #31 0x0043583e in execute_command () #32 0x0043310d in execute_command_internal () #33 0x0043349f in execute_command_internal () #34 0x0043583e in execute_command () #35 0x00433464 in execute_command_internal () #36 0x0043583e in execute_command () #37 0x00433464 in execute_command_internal () #38 0x0043583e in execute_command () #39 0x00433464 in
Re: [Qemu-devel] [PATCH for-2.0 V3] tests/acpi-test: do not run iasl on big endian machines
On Sun, Mar 23, 2014 at 03:17:32PM +0200, Marcel Apfelbaum wrote: On Sun, 2014-03-23 at 13:51 +0100, Paolo Bonzini wrote: Il 23/03/2014 13:32, Marcel Apfelbaum ha scritto: That's the endianness of the machine we're compiling QEMU for, not the endianness of the machine we're compiling QEMU on. If for instance you're on x86_64 cross-compiling for PPC then HOST_WORDS_BIGENDIAN is true, but the iasl you use in the build process will be running on a little endian machine. Hi Peter, are you sure about this? I saw the 'target_bigendian' that does what you described above. $bigendian is the result of a little C program that checks *host's* endian-ness. Of course I might have missed something. Peter is right. There are three systems: build (what you run on), host (what you compile for), target (always x86-64 for now for acpi-test). $bigendian and G_BYTE_ORDER check the host system. If you wanted to test the build system's endianness, Laszlo's suggestion would be fine. However, Michael is also right if we assume that iasl on bigendian can assemble, and that the problems are only on disassembling. This is because in this particular case you'll be running an executable compiled for the host and you know that host == build. I think the assumption is sane. As far as I remember, upstream iasl doesn't compile at all on big-endian machines. You need specific patches such as those shared by Fedora and Debian's. Unfortunately, the patches are incomplete, they only fix assembling because that's what was needed so far to cross-compile SeaBIOS. Hi Paolo, thanks for the help! I need a decision here :) 1. If iasl *does* work for the scenarios used by Qemu until now, I can conclude that the only part that suffers from it is the test itself, because it dis-assembles AML code. In this case I can handle it in the test itself by checking the endian-ness (V2 already posted), or trying to dis-assemble the expected AML file and not failing the test if this fails. The last option seems best to me. It will automatically start working when iasl disassembler is fixed and will handle any other case of breakage, not just BE. 2. If iasl causes problems to run a x86_64 guest if one or both build/host machines are BE, we need to tweak the configuration file. After this discussion it seems that (2.) is not the case, I think maybe re-sending: [PATCH for-2.0 V2] tests/acpi-test: do not run iasl on big endian machines Is everyone OK with this? Thanks, Marcel Paolo
Re: [Qemu-devel] [RFC 3/8] pc: prepare PC for custom machine state
On Thu, 2014-03-20 at 16:01 +0100, Igor Mammedov wrote: Signed-off-by: Igor Mammedov imamm...@redhat.com --- hw/i386/pc.c | 26 ++ hw/i386/pc_piix.c| 34 +- hw/i386/pc_q35.c | 10 +- include/hw/i386/pc.h | 14 ++ 4 files changed, 62 insertions(+), 22 deletions(-) diff --git a/hw/i386/pc.c b/hw/i386/pc.c index e715a33..e0bc3a2 100644 --- a/hw/i386/pc.c +++ b/hw/i386/pc.c @@ -1413,3 +1413,29 @@ void ioapic_init_gsi(GSIState *gsi_state, const char *parent_name) gsi_state-ioapic_irq[i] = qdev_get_gpio_in(dev, i); } } + +void qemu_register_pc_machine(QEMUMachine *m) I am not so comfortable with this approach because every subsystem (e.g pc) will have to duplicate the register machine code until the conversion from QEMUMachine to MachineClass is over. (which I hope it will not take too much time) I propose a patch already in the list which does that automatically by moving this logic into hw/core/machine.c . In this way it will be much less code touched during conversion. Andreas, did you have anything against the usage of 'class_base_init' ? The patch is: [PATCH 1/3] hw/machine: move QEMUMachine assignment into the core machine http://lists.gnu.org/archive/html/qemu-devel/2014-03/msg02151.html +{ +TypeInfo ti = { +.name = g_strconcat(m-name, TYPE_MACHINE_SUFFIX, NULL), This leads to a small memory leak, I missed that myself :( Here is a fixed version: [PATCHv2] vl.c: Fix memory leak in qemu_register_machine https://www.mail-archive.com/qemu-devel@nongnu.org/msg222800.html Thanks, Marcel +.parent = TYPE_PC_MACHINE, +.class_init = machine_class_init, +.class_data = (void *)m, +}; + +type_register(ti); +} + +static const TypeInfo pc_machine_info = { +.name = TYPE_PC_MACHINE, +.parent = TYPE_MACHINE, +.abstract = true, +.instance_size = sizeof(PCMachineState), +}; + +static void pc_machine_register_types(void) +{ +type_register_static(pc_machine_info); +} + +type_init(pc_machine_register_types) diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c index 7930a26..97df43e 100644 --- a/hw/i386/pc_piix.c +++ b/hw/i386/pc_piix.c @@ -817,24 +817,24 @@ static QEMUMachine xenfv_machine = { static void pc_machine_init(void) { -qemu_register_machine(pc_i440fx_machine_v2_0); -qemu_register_machine(pc_i440fx_machine_v1_7); -qemu_register_machine(pc_i440fx_machine_v1_6); -qemu_register_machine(pc_i440fx_machine_v1_5); -qemu_register_machine(pc_i440fx_machine_v1_4); -qemu_register_machine(pc_machine_v1_3); -qemu_register_machine(pc_machine_v1_2); -qemu_register_machine(pc_machine_v1_1); -qemu_register_machine(pc_machine_v1_0); -qemu_register_machine(pc_machine_v0_15); -qemu_register_machine(pc_machine_v0_14); -qemu_register_machine(pc_machine_v0_13); -qemu_register_machine(pc_machine_v0_12); -qemu_register_machine(pc_machine_v0_11); -qemu_register_machine(pc_machine_v0_10); -qemu_register_machine(isapc_machine); +qemu_register_pc_machine(pc_i440fx_machine_v2_0); +qemu_register_pc_machine(pc_i440fx_machine_v1_7); +qemu_register_pc_machine(pc_i440fx_machine_v1_6); +qemu_register_pc_machine(pc_i440fx_machine_v1_5); +qemu_register_pc_machine(pc_i440fx_machine_v1_4); +qemu_register_pc_machine(pc_machine_v1_3); +qemu_register_pc_machine(pc_machine_v1_2); +qemu_register_pc_machine(pc_machine_v1_1); +qemu_register_pc_machine(pc_machine_v1_0); +qemu_register_pc_machine(pc_machine_v0_15); +qemu_register_pc_machine(pc_machine_v0_14); +qemu_register_pc_machine(pc_machine_v0_13); +qemu_register_pc_machine(pc_machine_v0_12); +qemu_register_pc_machine(pc_machine_v0_11); +qemu_register_pc_machine(pc_machine_v0_10); +qemu_register_pc_machine(isapc_machine); #ifdef CONFIG_XEN -qemu_register_machine(xenfv_machine); +qemu_register_pc_machine(xenfv_machine); #endif } diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c index c844dc2..16b4daa 100644 --- a/hw/i386/pc_q35.c +++ b/hw/i386/pc_q35.c @@ -358,11 +358,11 @@ static QEMUMachine pc_q35_machine_v1_4 = { static void pc_q35_machine_init(void) { -qemu_register_machine(pc_q35_machine_v2_0); -qemu_register_machine(pc_q35_machine_v1_7); -qemu_register_machine(pc_q35_machine_v1_6); -qemu_register_machine(pc_q35_machine_v1_5); -qemu_register_machine(pc_q35_machine_v1_4); +qemu_register_pc_machine(pc_q35_machine_v2_0); +qemu_register_pc_machine(pc_q35_machine_v1_7); +qemu_register_pc_machine(pc_q35_machine_v1_6); +qemu_register_pc_machine(pc_q35_machine_v1_5); +qemu_register_pc_machine(pc_q35_machine_v1_4); } machine_init(pc_q35_machine_init); diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h index
Re: [Qemu-devel] [RFC 2/8] make machine_class_init() accessible outside of vl.c
On Thu, 2014-03-20 at 16:01 +0100, Igor Mammedov wrote: Signed-off-by: Igor Mammedov imamm...@redhat.com --- hw/core/machine.c |7 +++ include/hw/boards.h |2 ++ vl.c|7 --- 3 files changed, 9 insertions(+), 7 deletions(-) diff --git a/hw/core/machine.c b/hw/core/machine.c index d3ffef7..ae308f4 100644 --- a/hw/core/machine.c +++ b/hw/core/machine.c @@ -12,6 +12,13 @@ #include hw/boards.h +void machine_class_init(ObjectClass *oc, void *data) As I said in 3/8 review, if we *need* to move the above method, I would hide it in hw/core/machine.c. Please have a look on 3/8 review for details. Thanks, Marcel +{ +MachineClass *mc = MACHINE_CLASS(oc); + +mc-qemu_machine = data; +} + static const TypeInfo machine_info = { .name = TYPE_MACHINE, .parent = TYPE_OBJECT, diff --git a/include/hw/boards.h b/include/hw/boards.h index 22d9496..e1f1938 100644 --- a/include/hw/boards.h +++ b/include/hw/boards.h @@ -81,6 +81,8 @@ struct MachineClass { QEMUMachine *qemu_machine; }; +void machine_class_init(ObjectClass *oc, void *data); + /** * MachineState: */ diff --git a/vl.c b/vl.c index 7bae6fe..05b1158 100644 --- a/vl.c +++ b/vl.c @@ -1588,13 +1588,6 @@ void pcmcia_info(Monitor *mon, const QDict *qdict) MachineState *current_machine; -static void machine_class_init(ObjectClass *oc, void *data) -{ -MachineClass *mc = MACHINE_CLASS(oc); - -mc-qemu_machine = data; -} - int qemu_register_machine(QEMUMachine *m) { TypeInfo ti = {
Re: [Qemu-devel] [PATCH] osdep: initialize glib threads in all QEMU tools
On Tue, Oct 08, 2013 at 11:58:31AM +0200, Stefan Hajnoczi wrote: glib versions prior to 2.31.0 require an explicit g_thread_init() call to enable multi-threading. Failure to initialize threading causes glib to take single-threaded code paths without synchronization. For example, the g_slice allocator will crash due to race conditions. Fix this for all QEMU tool programs (qemu-nbd, qemu-io, qemu-img) by moving the g_thread_init() call from vl.c:main() into a new osdep.c:thread_init() constructor function. thread_init() has __attribute__((constructor)) and is automatically invoked by the runtime during startup. We can now drop the simple trace backend's g_thread_init() call since thread_init() already called it. Note that we must keep coroutine-gthread.c's g_thread_init() call which is located in a constructor function. There is no guarantee for constructor function ordering so thread_init() may only be called later. Reported-by: Mario de Chenno mario.deche...@unina2.it Signed-off-by: Stefan Hajnoczi stefa...@redhat.com --- trace/simple.c | 9 - util/osdep.c | 18 ++ vl.c | 8 3 files changed, 18 insertions(+), 17 deletions(-) Applied to my block tree, we need this for QEMU 2.0: https://github.com/stefanha/qemu/commits/block Stefan
Re: [Qemu-devel] [RFC 2/8] make machine_class_init() accessible outside of vl.c
On Sun, 2014-03-23 at 17:27 +0200, Marcel Apfelbaum wrote: On Thu, 2014-03-20 at 16:01 +0100, Igor Mammedov wrote: Signed-off-by: Igor Mammedov imamm...@redhat.com --- hw/core/machine.c |7 +++ include/hw/boards.h |2 ++ vl.c|7 --- 3 files changed, 9 insertions(+), 7 deletions(-) diff --git a/hw/core/machine.c b/hw/core/machine.c index d3ffef7..ae308f4 100644 --- a/hw/core/machine.c +++ b/hw/core/machine.c @@ -12,6 +12,13 @@ #include hw/boards.h +void machine_class_init(ObjectClass *oc, void *data) As I said in 3/8 review, if we *need* to move the above method, I would hide it in hw/core/machine.c. Just to make it more clear, I was talking about not making it visible in hw/boards.h, because you *did* move it to hw/core/machine.c :) Thanks, Marcel Please have a look on 3/8 review for details. Thanks, Marcel +{ +MachineClass *mc = MACHINE_CLASS(oc); + +mc-qemu_machine = data; +} + static const TypeInfo machine_info = { .name = TYPE_MACHINE, .parent = TYPE_OBJECT, diff --git a/include/hw/boards.h b/include/hw/boards.h index 22d9496..e1f1938 100644 --- a/include/hw/boards.h +++ b/include/hw/boards.h @@ -81,6 +81,8 @@ struct MachineClass { QEMUMachine *qemu_machine; }; +void machine_class_init(ObjectClass *oc, void *data); + /** * MachineState: */ diff --git a/vl.c b/vl.c index 7bae6fe..05b1158 100644 --- a/vl.c +++ b/vl.c @@ -1588,13 +1588,6 @@ void pcmcia_info(Monitor *mon, const QDict *qdict) MachineState *current_machine; -static void machine_class_init(ObjectClass *oc, void *data) -{ -MachineClass *mc = MACHINE_CLASS(oc); - -mc-qemu_machine = data; -} - int qemu_register_machine(QEMUMachine *m) { TypeInfo ti = {
[Qemu-devel] [PATCH] target-ppc: Bug: VSX Convert to Integer Should Truncate
The various VSX Convert to Integer instructions should truncate the mantissa. This fix forces the softfloat rounding mode to round to zero prior to performing the conversion. After the conversion is completed, the internal rounding mode is restored from the PowerPC FPSCR bits. Signed-off-by: Tom Musta tommu...@gmail.com --- This bug was discovered when running wget, which does this: double maxtime; struct timeval tmout; ... tmout.tv_usec = 100 * (maxtime - (long) maxtime); The newest PowerPC 64-bit gcc's are now using xscvdpsxds to perform the cast of the double to long. A timeout of 0.95 was erroneously rounding up to 1 and hence computing a negative timeout value. It would be great if we could still get this into 2.0. target-ppc/fpu_helper.c |4 1 files changed, 4 insertions(+), 0 deletions(-) diff --git a/target-ppc/fpu_helper.c b/target-ppc/fpu_helper.c index e7f3295..ccfc5cc 100644 --- a/target-ppc/fpu_helper.c +++ b/target-ppc/fpu_helper.c @@ -2558,10 +2558,14 @@ void helper_##op(CPUPPCState *env, uint32_t opcode) \ fload_invalid_op_excp(env, POWERPC_EXCP_FP_VXCVI, 0);\ xt.tfld = rnan; \ } else { \ +/* force round to zero mode (truncation) */ \ +set_float_rounding_mode(float_round_to_zero, env-fp_status); \ xt.tfld = stp##_to_##ttp(xb.sfld, env-fp_status); \ if (env-fp_status.float_exception_flags float_flag_invalid) { \ fload_invalid_op_excp(env, POWERPC_EXCP_FP_VXCVI, 0);\ }\ +/* restore rounding mode from FPSCR */ \ +fpscr_set_rounding_mode(env);\ }\ }\ \ -- 1.7.1
Re: [Qemu-devel] [PATCH] target-ppc: Bug: VSX Convert to Integer Should Truncate
On 23 March 2014 18:02, Tom Musta tommu...@gmail.com wrote: diff --git a/target-ppc/fpu_helper.c b/target-ppc/fpu_helper.c index e7f3295..ccfc5cc 100644 --- a/target-ppc/fpu_helper.c +++ b/target-ppc/fpu_helper.c @@ -2558,10 +2558,14 @@ void helper_##op(CPUPPCState *env, uint32_t opcode) \ fload_invalid_op_excp(env, POWERPC_EXCP_FP_VXCVI, 0); \ xt.tfld = rnan; \ } else { \ +/* force round to zero mode (truncation) */ \ +set_float_rounding_mode(float_round_to_zero, env-fp_status); \ xt.tfld = stp##_to_##ttp(xb.sfld, env-fp_status); \ if (env-fp_status.float_exception_flags float_flag_invalid) { \ fload_invalid_op_excp(env, POWERPC_EXCP_FP_VXCVI, 0); \ } \ If we raise a CPU exception here (via helper_raise_exception_err()) we'll longjmp out of here and never restore the rounding mode. So the restoring of the rounding mode needs to happen before we check for exceptions here. +/* restore rounding mode from FPSCR */ \ +fpscr_set_rounding_mode(env); \ } \ } \ \ thanks -- PMM
[Qemu-devel] [V2 PATCH] target-ppc: Bug: VSX Convert to Integer Should Truncate
The various VSX Convert to Integer instructions should truncate the mantissa. This fix forces the softfloat rounding mode to round to zero prior to performing the conversion. After the conversion is completed, the internal rounding mode is restored from the PowerPC FPSCR bits. Signed-off-by: Tom Musta tommu...@gmail.com --- V2: Restored rounding mode prior to checking exceptions per Peter Maydell's review. This bug was discovered when running wget, which does this: double maxtime; struct timeval tmout; ... tmout.tv_usec = 100 * (maxtime - (long) maxtime); The newest PowerPC 64-bit gcc's are now using xscvdpsxds to perform the cast of the double to long. A timeout of 0.95 was erroneously rounding up to 1 and hence computing a negative timeout value. It would be great if we could still get this into 2.0. target-ppc/fpu_helper.c |4 1 files changed, 4 insertions(+), 0 deletions(-) diff --git a/target-ppc/fpu_helper.c b/target-ppc/fpu_helper.c index fd91239..9b3a6f7 100644 --- a/target-ppc/fpu_helper.c +++ b/target-ppc/fpu_helper.c @@ -2568,7 +2568,11 @@ void helper_##op(CPUPPCState *env, uint32_t opcode) \ fload_invalid_op_excp(env, POWERPC_EXCP_FP_VXCVI, 0);\ xt.tfld = rnan; \ } else { \ +/* force round to zero mode (truncation) */ \ +set_float_rounding_mode(float_round_to_zero, env-fp_status); \ xt.tfld = stp##_to_##ttp(xb.sfld, env-fp_status); \ +/* restore rounding mode from FPSCR */ \ +fpscr_set_rounding_mode(env);\ if (env-fp_status.float_exception_flags float_flag_invalid) { \ fload_invalid_op_excp(env, POWERPC_EXCP_FP_VXCVI, 0);\ }\ -- 1.7.1
Re: [Qemu-devel] [RFC PATCH V3 2/5] qapi: add event helper functions
于 2014/3/21 6:53, Eric Blake 写道: On 03/18/2014 11:16 PM, Wenchao Xia wrote: This file hold some functions that do not need to be generated. s/hold/holds/ Signed-off-by: Wenchao Xiawenchaoq...@gmail.com --- include/qapi/qmp-event.h | 25 qapi/Makefile.objs |1 + qapi/qmp-event.c | 71 ++ 3 files changed, 97 insertions(+), 0 deletions(-) create mode 100644 include/qapi/qmp-event.h create mode 100644 qapi/qmp-event.c diff --git a/include/qapi/qmp-event.h b/include/qapi/qmp-event.h new file mode 100644 index 000..fdf1a7f --- /dev/null +++ b/include/qapi/qmp-event.h @@ -0,0 +1,25 @@ +/* + * QMP Event related + * + * Authors: + * Wenchao Xiawenchaoq...@gmail.com + * + * This work is licensed under the terms of the GNU LGPL, version 2.1 or later. + * See the COPYING.LIB file in the top-level directory. For the [L]GPL to work, someone must assert copyright. Will fix. +++ b/qapi/qmp-event.c @@ -0,0 +1,71 @@ +/* + * QMP Event related + * + * Authors: + * Wenchao Xiawenchaoq...@gmail.com + * + * This work is licensed under the terms of the GNU LGPL, version 2.1 or later. + * See the COPYING.LIB file in the top-level directory. Again, missing an actual use of the word Copyright. + +typedef struct QMPEventFunctions { +QMPEventFuncEmit emit; +} QMPEventFunctions; + +QMPEventFunctions qmp_event_functions; + +void qmp_event_set_func_emit(QMPEventFuncEmit emit) +{ +qmp_event_functions.emit = emit; +} + +QMPEventFuncEmit qmp_event_get_func_emit(void) +{ +return qmp_event_functions.emit; +} Is this struct a bit overkill, or do you extend it to include other fields later? No other fields will be added in this series, it allow different emit function hooked. Do you mean remove it and put it into generated qapi-event.c? +err = qemu_gettimeofday(tv); +if (err 0) { +/* Put -1 to indicate failure of getting host time */ +tv.tv_sec = tv.tv_usec = -1; Believe it or not, this is NOT portable. Let's consider what happens when tv_sec is int64_t and tv_usec is uint32_t. Assignments happen right to left, so tv_usec gets the unsigned value 0x, then since all uint32_t values fit in int64_t, integer promotion says that the value is 0-extended (not sign-extended), and tv_sec is NOT assigned -1. Solution: break this into two separate statements: tv.tv_sec = -1; tv.tv_usec = -1; Good catch, thanks! +} + +obj = qobject_from_jsonf({ 'seconds': % PRId64 , +'microseconds': % PRId64 }, +(int64_t) tv.tv_sec, (int64_t) tv.tv_usec); Indentation is odd, but that's cosmetic. Will fix.
Re: [Qemu-devel] [RFC PATCH V2 3/5] qapi script: add event support by qapi-event.py
于 2014/3/21 6:29, Eric Blake 写道: On 03/18/2014 08:38 PM, Wenchao Xia wrote: 于 2014/3/7 2:49, Eric Blake 写道: On 01/02/2014 04:10 PM, Wenchao Xia wrote: qapi-event.py will parse the schema and generate qapi-event.c, then the API in qapi-event.c can be used to handle event in qemu code. All API have prefix qapi_event, all types have prefix QAPIEvent. Examples can be found in following patches. +for o, a in opts: +if o in (-p, --prefix): +prefix = a +elif o in (-o, --output-dir): +output_dir = a + / +elif o in (-c, --source): +do_c = True +elif o in (-h, --header): +do_h = True +elif o in (-b, --builtins): +do_builtins = True You may need to rebase this on top of other patches that refactor the qapi generators to track the input file, for improved error messages. It seems qapi-visit.py and qapi-types.py remains the same as above in upstream, which kind of change are your referring to? Lluís' patch to use an explicit input file via a new -i option: https://lists.gnu.org/archive/html/qemu-devel/2014-02/msg05220.html I see, will adjust it.
Re: [Qemu-devel] [RFC PATCH V3 3/5] qapi script: add event support
于 2014/3/21 7:06, Eric Blake 写道: On 03/18/2014 11:16 PM, Wenchao Xia wrote: qapi-event.py will parse the schema and generate qapi-event.c, then the API in qapi-event.c can be used to handle event in qemu code. All API have prefix qapi_event. The script mainly include two parts: generate API for each event s/include/includes/ define, generate an enum type for all defined events. Since in some case the real emit behavior may change, for example, s/case/cases/ qemu-img would not send a event, a callback layer is used to control the behavior. As a result, the stubs at compile time can be saved, the binding of block layer code and monitor code will become looser. Signed-off-by: Wenchao Xiawenchaoq...@gmail.com --- Makefile |9 +- Makefile.objs |2 +- docs/qapi-code-gen.txt | 18 +++ scripts/qapi-event.py | 373 4 files changed, 398 insertions(+), 4 deletions(-) create mode 100644 scripts/qapi-event.py +++ b/docs/qapi-code-gen.txt @@ -180,6 +180,24 @@ An example command is: 'data': { 'arg1': 'str', '*arg2': 'str' }, 'returns': 'str' } +=== Events === + +Events are defined with key workd 'event'. When 'data' is also specified, s/workd/word/ +additional info will be carried on. Finally there will be C API generated +in qapi-event.h, and when called by QEMU code, message with timestamp will s/message/a message/ +be emit on the wire. If timestamp is -1, it means failure in host time s/emit/emitted/ +retrieving. s/in host time retrieving/to retrieve host time/ +++ b/scripts/qapi-event.py @@ -0,0 +1,373 @@ +# +# QAPI event generator +# +# Authors: +# Wenchao Xiawenchaoq...@gmail.com +# +# This work is licensed under the terms of the GNU GPL, version 2. +# See the COPYING file in the top-level directory. Needs to use Copyright. + +if params: +for argname, argentry, optional, structured in parse_args(params): +if structured: +sys.stderr.write(Nested structure define in event is not + supported now, event '%s', argname '%s'\n % + (event_name, argname)) +sys.exit(1) +continue Isn't this 'continue' dead code? Yes, I missed it. I'd like to respin a rfc v4, with better error check function moved into qapi.py, just like the series which check error for other kind of schema error. + +# Following are the functions that generate an enum type for all defined +# events, similar with qapi-types.py. Here we already have enum name and s/with/to/ +# values which is generated before and recorded in event_enum_*. It also s/is/were/ +# walk around the issue that import qapi-types can't work. s/walk around/works around/ + +fdef.write(mcgen(''' +/* THIS FILE IS AUTOMATICALLY GENERATED, DO NOT MODIFY */ + +/* + * schema-defined QAPI event functions + * + * Authors: + * Wenchao Xiawenchaoq...@gmail.com + * + * This work is licensed under the terms of the GNU LGPL, version 2.1 or later. + * See the COPYING.LIB file in the top-level directory. Also needs Copyright +fdecl.write(mcgen(''' +/* THIS FILE IS AUTOMATICALLY GENERATED, DO NOT MODIFY */ + +/* + * schema-defined QAPI event function s/function/functions/ + * + * Authors: + * Wenchao Xiawenchaoq...@gmail.com + * + * This work is licensed under the terms of the GNU LGPL, version 2.1 or later. + * See the COPYING.LIB file in the top-level directory. Needs Copyright
Re: [Qemu-devel] [RFC PATCH V3 4/5] test: add test cases for qapi event
于 2014/3/21 8:23, Eric Blake 写道: On 03/18/2014 11:16 PM, Wenchao Xia wrote: These cases will verify whether the expected qdict is built. Signed-off-by: Wenchao Xiawenchaoq...@gmail.com --- tests/Makefile | 14 ++- tests/qapi-schema/qapi-schema-test.json | 12 ++ tests/qapi-schema/qapi-schema-test.out | 10 +- tests/test-qmp-event.c | 258 +++ 4 files changed, 289 insertions(+), 5 deletions(-) create mode 100644 tests/test-qmp-event.c +++ b/tests/test-qmp-event.c @@ -0,0 +1,258 @@ +/* + * qapi event unit-tests. + * + * Authors: + * Wenchao Xiawenchaoq...@gmail.com + * + * This work is licensed under the terms of the GNU LGPL, version 2.1 or later. + * See the COPYING.LIB file in the top-level directory. + * Missing Copyright +case QTYPE_QINT: +d-result = (qint_get_int(qobject_to_qint(obj1)) == + qint_get_int(qobject_to_qint(obj2))); +return; +case QTYPE_QSTRING: +if (!g_strcmp0(qstring_get_str(qobject_to_qstring(obj1)), + qstring_get_str(qobject_to_qstring(obj2 { +d-result = true; +} else { +d-result = false; +} Could also be written without 'if': d-result = g_strcmp0(...) == 0; +obj = qdict_get(t, seconds); +g_assert(obj qobject_type(obj) == QTYPE_QINT); +obj = qdict_get(t, microseconds); +g_assert(obj qobject_type(obj) == QTYPE_QINT); Might be worth asserting that microseconds is within the range [0,99] (or -1 if seconds is -1) will add those test.
Re: [Qemu-devel] [PATCH v22 00/25] replace QEMUOptionParameter with QemuOpts
2014-03-21 20:31 GMT+08:00 Leandro Dorileo l...@dorileo.org: On Fri, Mar 21, 2014 at 06:09:22PM +0800, Chunyan Liu wrote: 2014-03-21 8:07 GMT+08:00 Leandro Dorileo l...@dorileo.org: Hi Chunyan, On Mon, Mar 10, 2014 at 03:31:36PM +0800, Chunyan Liu wrote: This patch series is to replace QEMUOptionParameter with QemuOpts, so that only one Qemu Option structure is kept in QEMU code. Last night I took some time do take a deeper look at you series and the required effort to do the QemuOptionParameter - QemuOpts migration. I think you've over complicated the things, I understand you tried to keep your serie's bisectability (?), but the final result was something really hard to review and to integrate as well. The overall approach wasn't even resolving the bisectability problem since it breaks the tree until the last commit. Moreover, in the path of getting things ready you created new problems and their respective fixes, what we really don't need to. In this regards you could have kept things as simple as possible and submitted the patches in a natural way, even if they were breaking the build between each patch, you could get all the required maintainer's Reviewed-by + Tested-by + Signed-off-by and so on for each individual patch and when it was time to integrate get squashed the needed patches. Well, if breaking the build could be allowed between each patch, then it could be much cleaner. Indeed there are lots of code just for build and function unbroken between each patch. I'm inclined to listen to more voice. If all agree to this method, it's OK to me. The thing is the balance between complexity and the change size. Do we really want to avoid a small patch - doing all the change - and increase the whole thing complexity? I don't see a great benefit on that :) I mean, add N patches introducing new required QemuOpts API's, 1 patch migrating the block upper layer (block.c, block.h, etc), one patch for each block driver (i.e ssh.c, qcow.c, qcow2.c, etc), one patch for qemu-img.c and finally a last patch removing the QEMUOptionParamer itself. When time comes to integrate your series the patches changing the block layer + patches changing the block drivers + patches changing qemu-img.c could be squashed adding all the collected Reviewed-by to this single squashed patch. As I said, last night I took a deeper look at the problem and, understood most of changes weren't required to do the job. We don't need an adaptation layer between QemuOptionParameter and QemuOpts, we don't need to add new opts accessors (like those qemu_opt_*_del() functions), all we need is 1) that qemu_opts_append() function so we can merge the protocol and drivers options in a single QemuOptList and 2) the default value support. All we need is already present in the QemuOpts APIs. qemu_opt_*_del functions are needed. Each driver handles options they expected then delete, left options passed to 2nd driver and let it handle. Like qcow2 create, first, qcow2 driver handle, then raw driver handle. Not true, the only place you need to allocate QemuOpts or QemuOptsList is on qemu-img.c and block.c, if they're doing so they should free it, not the lower lavels. The block drivers should just use it, unless they do allocate anything themselves. The reason qemu_opt_get_*_del functions should be used in backend drivers, is to keep same behavior as how previous QEMUOptionParameter handles. At least, in one case: create a qcow2 img. size option is handled by qcow2 driver, then delete; in 2nd raw driver, there is no size option any more, it will create a 0 size file. If qemu_opt_get but not delete, then all options will be passed to 2nd raw driver, it will create a full sized file. That is not expected. But as you point, some changes are not required for this job, I've omitted in my new patch series, like: qemu_opt_set, NULL check in qemu_opt_get and qemu_opt_find, assert() update in qemu_opt_get. Ok. -- Leandro Dorileo With that simpler approach in mind I ended up putting my hands in the source code trying to see how feasible it is, and turns out I came up with a full solution. I'm sending the job's resulting series to the mailing list so I can show you what I mean and have some more room for discussion. It doesn't mean I want to overlap you work, I just needed to have a little more input on that. No matter. I'm OK to follow a more acceptable way :) Regards -- Leandro Dorileo
Re: [Qemu-devel] [PATCH v23 03/32] qapi: output def_value_str when query command line options
I'll update. All patch series could also be available from: https://github.com/chunyanliu/qemu/commits/QemuOpts On 3/22/2014 at 07:27 AM, in message 532ccadc.40...@redhat.com, Eric Blake ebl...@redhat.com wrote: On 03/21/2014 04:12 AM, Chunyan Liu wrote: Change qapi interfaces to output the newly added def_value_str when querying command line options. Reviewed-by: Eric Blake ebl...@redhat.com Signed-off-by: Dong Xu Wang wdon...@linux.vnet.ibm.com Signed-off-by: Chunyan Liu cy...@suse.com --- qapi-schema.json | 6 +- qmp-commands.hx| 2 ++ util/qemu-config.c | 4 3 files changed, 11 insertions(+), 1 deletion(-) diff --git a/qapi-schema.json b/qapi-schema.json index b68cd44..cf9174e 100644 --- a/qapi-schema.json +++ b/qapi-schema.json @@ -4088,12 +4088,16 @@ # # @help: #optional human readable text string, not suitable for parsing. # +# @default: #optional string representation of the default used +# if the option is omitted. (since 2.0) We've missed 2.0; this needs to be 2.1 now. However, I'm okay if you keep my Reviewed-by with just that change. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org
[Qemu-devel] [RFC PATCH] target-ppc: enable migration within the same CPU family
Currently only migration fails if CPU version is different even a bit. For example, migration from POWER7 v2.0 to POWER7 v2.1 fails because of that. Since there is no difference between CPU versions which could affect migration stream, we can safely enable it. This adds a helper to find the closest POWERPC family class (i.e. first abstract class in hierarchy). This replaces VMSTATE_UINTTL_EQUAL statement with a custom handler which checks if the source and destination CPUs belong to the same family and fails if they are not. This adds a PVR reset to the default value as it will be overwritten by VMSTATE_UINTTL_ARRAY(env.spr, PowerPCCPU, 1024). Since the actual migration format is not changed by this patch, @version_id of vmstate_ppc_cpu does not have to be changed either. Signed-off-by: Bharata B Rao bhar...@linux.vnet.ibm.com Signed-off-by: Alexey Kardashevskiy a...@ozlabs.ru --- target-ppc/cpu-qom.h| 1 + target-ppc/machine.c| 40 ++-- target-ppc/translate_init.c | 12 3 files changed, 51 insertions(+), 2 deletions(-) diff --git a/target-ppc/cpu-qom.h b/target-ppc/cpu-qom.h index 47dc8e6..5eb56ea 100644 --- a/target-ppc/cpu-qom.h +++ b/target-ppc/cpu-qom.h @@ -105,6 +105,7 @@ static inline PowerPCCPU *ppc_env_get_cpu(CPUPPCState *env) PowerPCCPUClass *ppc_cpu_class_by_pvr(uint32_t pvr); PowerPCCPUClass *ppc_cpu_class_by_pvr_mask(uint32_t pvr); +PowerPCCPUClass *ppc_cpu_family_class_by_pvr_mask(uint32_t pvr); void ppc_cpu_do_interrupt(CPUState *cpu); void ppc_cpu_dump_state(CPUState *cpu, FILE *f, fprintf_function cpu_fprintf, diff --git a/target-ppc/machine.c b/target-ppc/machine.c index 063b379..834297e 100644 --- a/target-ppc/machine.c +++ b/target-ppc/machine.c @@ -160,6 +160,11 @@ static int cpu_post_load(void *opaque, int version_id) CPUPPCState *env = cpu-env; int i; +/* + * Allow migration between hosts of same processor family + * by restoring the default PVR for this VM on this host. + */ +env-spr[SPR_PVR] = env-spr_cb[SPR_PVR].default_value; env-lr = env-spr[SPR_LR]; env-ctr = env-spr[SPR_CTR]; env-xer = env-spr[SPR_XER]; @@ -462,6 +467,37 @@ static const VMStateDescription vmstate_tlbmas = { } }; +static int get_pvr(QEMUFile *f, void *pv, size_t size) +{ +target_ulong pvrdest = *(target_ulong *)pv; +#if TARGET_LONG_BITS == 64 +target_ulong pvrsrc = qemu_get_be64(f); +#else +target_ulong pvrsrc = qemu_get_be32(f); +#endif +PowerPCCPUClass *pccdest = ppc_cpu_family_class_by_pvr_mask(pvrdest); +PowerPCCPUClass *pccsrc = ppc_cpu_family_class_by_pvr_mask(pvrsrc); + +return (pccdest == pccsrc) ? 0 : -1; +} + +static void put_pvr(QEMUFile *f, void *pv, size_t size) +{ +target_ulong pvr = *(target_ulong *)pv; + +#if TARGET_LONG_BITS == 64 +qemu_put_be64(f, pvr); +#else +qemu_put_be32(f, pvr); +#endif +} + +static const VMStateInfo vmstate_pvr = { +.name = PVR, +.get = get_pvr, +.put = put_pvr, +}; + const VMStateDescription vmstate_ppc_cpu = { .name = cpu, .version_id = 5, @@ -471,8 +507,8 @@ const VMStateDescription vmstate_ppc_cpu = { .pre_save = cpu_pre_save, .post_load = cpu_post_load, .fields = (VMStateField []) { -/* Verify we haven't changed the pvr */ -VMSTATE_UINTTL_EQUAL(env.spr[SPR_PVR], PowerPCCPU), +VMSTATE_SINGLE(env.spr[SPR_PVR], PowerPCCPU, 0, vmstate_pvr, + target_ulong), /* User mode architected state */ VMSTATE_UINTTL_ARRAY(env.gpr, PowerPCCPU, 32), diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c index cdb2d2a..0c5c6a8 100644 --- a/target-ppc/translate_init.c +++ b/target-ppc/translate_init.c @@ -8146,6 +8146,18 @@ PowerPCCPUClass *ppc_cpu_class_by_pvr_mask(uint32_t pvr) return pcc; } +PowerPCCPUClass *ppc_cpu_family_class_by_pvr_mask(uint32_t pvr) +{ +PowerPCCPUClass *pcc = ppc_cpu_class_by_pvr_mask(pvr); +ObjectClass *oc = OBJECT_CLASS(pcc); + +while (oc !object_class_is_abstract(oc)) { +oc = object_class_get_parent(oc); +} + +return POWERPC_CPU_CLASS(oc); +} + static gint ppc_cpu_compare_class_name(gconstpointer a, gconstpointer b) { ObjectClass *oc = (ObjectClass *)a; -- 1.8.4.rc4
Re: [Qemu-devel] [PATCH] usb-ohci: add vmstate descriptor
On 03/23/2014 08:23 AM, Peter Maydell wrote: On 22 March 2014 21:04, Andreas Färber afaer...@suse.de wrote: Am 22.03.2014 21:54, schrieb Peter Maydell: On 22 March 2014 20:18, Andreas Färber afaer...@suse.de wrote: Because AFAIU migration is possible without VMSD, just not with VMSD that sets .unmigratable = 1. Well, the migration won't fail with an error, but on the destination end you'll end up with a device in its reset state but a guest which may think the device is in some other state. If the device was quiescent and doesn't need complex setup it might cope, but more likely is that that guest driver will fall over in a heap next time you try to use it... Well, there is no OHCI state being added, only PCI state. So I'd be curious to know what in there is the problem because a general review of PCI devices might be due then - and ideally before we do the release. Oops, I hadn't noticed that; this patch is incorrect, then, because vmstate_ohci needs to include a line for the OHCIState, and we need a second vmstate struct for the OHCIState. Sorry but what is incorrect in the patch? I can understand that it is incomplete as it is missing OHCI-specific bits from the OHCIState state and I can do that but I need some hints what is really necessary. So far the USB device was able to recover, only PCI bits were really needed. Thanks. -- Alexey