Re: [Qemu-devel] how could I analysis the trace log?

2012-02-12 Thread 陳韋任
> I just guess the format of input events file of the simpletrace.py.
> For so many available events, how could I specify the format of all
> those events?

  
http://repo.or.cz/w/qemu/stefanha.git/blob_plain/refs/heads/tracing:/docs/tracing.txt

  Reading "Trace events" might be help.

Regards,
chenwj

-- 
Wei-Ren Chen (陳韋任)
Computer Systems Lab, Institute of Information Science,
Academia Sinica, Taiwan (R.O.C.)
Tel:886-2-2788-3799 #1667
Homepage: http://people.cs.nctu.edu.tw/~chenwj



Re: [Qemu-devel] [PATCH v2 22/27] qdev: access properties via QOM

2012-02-12 Thread Paolo Bonzini

On 02/11/2012 04:46 PM, Blue Swirl wrote:

Program received signal SIGSEGV, Segmentation fault.
qdev_prop_set_chr (dev=0x1365580, name=0x6ed811 "chrB", value=0x0)
at /src/qemu/hw/qdev-properties.c:1142
1142assert(value->label);
(gdb) bt
#0  qdev_prop_set_chr (dev=0x1365580, name=0x6ed811 "chrB", value=0x0)
at /src/qemu/hw/qdev-properties.c:1142
#1  0x0048b484 in escc_init (base=0, irqA=0x12deb48, irqB=0x12deb60,
chrA=0x128ff60, chrB=0x0, clock=, it_shift=4)
at /src/qemu/hw/escc.c:698
#2  0x005ab173 in ppc_heathrow_init (ram_size=134217728,
boot_device=, kernel_filename=0x0,
kernel_cmdline=,
initrd_filename=, cpu_model=)
at /src/qemu/hw/ppc_oldworld.c:246
#3  0x004d9a8e in main (argc=,
argv=, envp=)
at /src/qemu/vl.c:3391

This is running qemu-system-sparc and qemu-system-ppc with no arguments.


Will fix, thanks for reporting.

Paolo



Re: [Qemu-devel] [PATCH] oslib: make error handling more reasonable

2012-02-12 Thread Stefan Weil

Am 13.02.2012 03:37, schrieb Zhi Yong Wu:

On Fri, Feb 10, 2012 at 11:53 PM, Stefan Weil  wrote:

Am 10.02.2012 16:13, schrieb Zhi Yong Wu:


On Fri, Feb 10, 2012 at 10:41 PM, Daniel P. Berrange
 wrote:


On Fri, Feb 10, 2012 at 10:34:13PM +0800, Zhi Yong Wu wrote:


From: Zhi Yong Wu 

Signed-off-by: Zhi Yong Wu 
---
 oslib-posix.c |4 ++--
 oslib-win32.c |4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/oslib-posix.c b/oslib-posix.c
index b6a3c7f..f978d56 100644
--- a/oslib-posix.c
+++ b/oslib-posix.c
@@ -80,7 +80,7 @@ void *qemu_oom_check(void *ptr)
 {
if (ptr == NULL) {
fprintf(stderr, "Failed to allocate memory: %s\n",
strerror(errno));
-abort();
+exit(EXIT_FAILURE);



exit() will call any atexit()/on_exit() handlers, as well as trying
to flush I/O streams. Any of these actions may require further
memory allocations, which will likely fail, or worse cause this
code to re-enter itself if an atexit() handler calls qemu_malloc


Nice, very reasonable.



The only option other than abort(), is to use  _Exit() which
doesn't try to run cleanup handlers.


I will try to send out v2



Could you please explain why calling exit, _Exit or _exit is more
reasonable than calling abort?

abort can create core dumps or start a debugger which is
useful for me and maybe other developers, too.
pls refer to 
http://lists.gnu.org/archive/html/qemu-devel/2012-02/msg01270.html.

In the scenario, the user should not see core dump, and he perhaps
think that one bug exists in qemu code.
So we hope to use _Exit() instead of abort() here.


So you say that you don't want a core dump just because the
user called QEMU with -m 4000 or some other large value.

Allocating RAM for the emulated machine is perhaps the only
scenario where a core dump is indeed not reasonable. In most
other cases, out-of-memory is an indication of a QEMU internal
problem, so a core dump should be written.

I therefore suggest to restrict any modification to the handling
of -m. In that case you could even improve the error message by
telling the user how much memory would be possible.
Simply call the allocating function with decreasing values until
it no longer fails.

Regards,
Stefan Weil




Re: [Qemu-devel] [PATCH] network: Added option to disable NIC option roms

2012-02-12 Thread Gerhard Wiesinger

On Fri, 27 Jan 2012, Gerhard Wiesinger wrote:


On Thu, 26 Jan 2012, Markus Armbruster wrote:


Gerd Hoffmann  writes:


On 01/26/12 08:45, Markus Armbruster wrote:

Gerhard Wiesinger  writes:

Option ROM for network interface cards (NICs) can now explicitly 
disabled

with romfile=disabled (or romfile=no or romfile=none) parameter.
With hotplugable NICs (currently NE2000, PCNET) romfile=(empty) didn't 
work.

This patch disables Option ROMs for iPXE for alls supported NICs
(hotplugable and non hotplugable).


And now filenames "disabled", "no" and "none" don't work.

Any way to fix "romfile="?


Sure.


This patch looks much better.  Gerhard does it solve your problem?
Gerd, you might want to repost it in its own thread, as maintainers can
easily miss patches buried deep in replies.


Can confirm this patch works well in all tried combinations.

3 Comments:
1.) NOK?: 2 NICs installed, no bootindex specified: Tries to boot only from 
one NIC, then from C: (one NIC has index first, second one has last index)

-boot order=nca,menu=on
-device rtl8139,mac=1a:46:0b:ca:bc:7c,vlan=0 -net 
tap,ifname=tap0,script=no,downscript=no,vlan=0
-device pcnet,mac=1a:46:0b:ca:bc:7e,vlan=1 -net 
tap,ifname=tap1,script=no,downscript=no,vlan=1

I would expect to try to boot from both NICs
1. iPXE (PCI 00:04:0)
...
8. iPXE (PCI 00:05:0)

2.) OK: 2 NICs installed, bootindex specified: Tries to boot from first and 
second NIC

-boot order=nca,menu=on
-device rtl8139,mac=1a:46:0b:ca:bc:7c,vlan=0,bootindex=1 -net 
tap,ifname=tap0,script=no,downscript=no,vlan=0
-device pcnet,mac=1a:46:0b:ca:bc:7e,vlan=1,bootindex=2 -net 
tap,ifname=tap1,script=no,downscript=no,vlan=1

1. iPXE (PCI 00:04:0)
2. iPXE (PCI 00:05:0)
...

3.) NOK: 2 NICs installed, bootindex specified in reverse order: Tries to 
boot from 7e NIC and reboots ...

-boot order=nca,menu=on
-device rtl8139,mac=1a:46:0b:ca:bc:7c,vlan=0,bootindex=2 -net 
tap,ifname=tap0,script=no,downscript=no,vlan=0
-device pcnet,mac=1a:46:0b:ca:bc:7e,vlan=1,bootindex=1 -net 
tap,ifname=tap1,script=no,downscript=no,vlan=1

1. iPXE (PCI 00:05:0)
2. iPXE (PCI 00:04:0)
...


Still waiting for commit ...

Ciao,
Gerhard

--
http://www.wiesinger.com/



[Qemu-devel] Missing patch in QEMU which is in QEMU-KVM

2012-02-12 Thread Gerhard Wiesinger

Hello,

I miss the following patch in QEMU which is in QEMU-KVM:
http://article.gmane.org/gmane.comp.emulators.kvm.devel/13557

commit a7fe0297840908a4fd65a1cf742481ccd45960eb
Author: Andreas Winkelbauer 
Date:   Sun Feb 24 10:33:27 2008 +0200

Extend vram size to 16MB

this is useful for high resolutions.

Signed-off-by: Avi Kivity 

In general: Which patches are missing in QEMU which are in QEMU-KVM and 
vice versa?


Thnx.

Ciao,
Gerhard

--
http://www.wiesinger.com/



Re: [Qemu-devel] weird qdev error

2012-02-12 Thread Michael S. Tsirkin
On Mon, Feb 13, 2012 at 03:18:13AM +0200, Michael S. Tsirkin wrote:
> > I also see this:
> > 
> > device_add virtio-net-pci,netdev=foo,mac=52:54:00:12:34:56,id=bla
> > device_del bla
> > *** glibc detected *** /home/mst/qemu-test/bin/qemu-system-x86_64:
> > corrupted double-linked list: 0x7fae434565a0 ***
> > 
> > Am I alone?

Doesn't solve this issue, but shouldn't we use _SAFE
in object_property_del_child? Like this:

--->
qemu: use safe list macro

As we might remove an element from list, use the safe macro
to walk it.

Signed-off-by: Michael S. Tsirkin 

---

diff --git a/qom/object.c b/qom/object.c
index 5e5b261..8b64fb6 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -299,9 +299,9 @@ static void object_property_del_all(Object *obj)
 
 static void object_property_del_child(Object *obj, Object *child, Error **errp)
 {
-ObjectProperty *prop;
+ObjectProperty *prop, *next;
 
-QTAILQ_FOREACH(prop, &obj->properties, node) {
+QTAILQ_FOREACH_SAFE(prop, &obj->properties, node, next) {
 if (!strstart(prop->type, "child<", NULL)) {
 continue;
 }



Re: [Qemu-devel] how could I analysis the trace log?

2012-02-12 Thread Wei Yang
Hi, this kind of trace is not popular?

2012/2/12 Wei Yang :
> All
>
> I enable the trace function with --enable-trace-backend=simple and I
> create the event file like this
> g_realloc
> g_malloc
>
> Then I start the qemu with following command.
> ./i386-softmmu/qemu-system-i386 -enable-kvm -drive
> file=../../kvm/ubuntu.img -boot dc -m 512 -usb
>  -monitor stdio -trace events=qemu_trace_events,file=qemu_trace.log
>
> After some run time, I run the script like:
> ./scripts/simpletrace.py qemu_trace_events_parse qemu_trace.log
>
> The qemu_trace_events_parse is :
> g_realloc(addr)
> g_malloc(addr)
>
> The output looks like:
> g_malloc 1.831 addr=0xb945d1f0
> g_malloc 2.498 addr=0xb945d1f0
> g_realloc 4.715 addr=0x10
> g_realloc 1.520 addr=0xc
> g_realloc 1.505 addr=0xc
>
> The steps I used is correct?
> I just guess the format of input events file of the simpletrace.py.
> For so many available events, how could I specify the format of all
> those events?
>
> --
> Richard Yang
> Help You, Help Me



-- 
Richard Yang
Help You, Help Me



Re: [Qemu-devel] [PATCH] oslib: make error handling more reasonable

2012-02-12 Thread Zhi Yong Wu
On Sat, Feb 11, 2012 at 2:35 AM, Eric Blake  wrote:
> On 02/10/2012 07:41 AM, Daniel P. Berrange wrote:
>
>>> @@ -80,7 +80,7 @@ void *qemu_oom_check(void *ptr)
>>>  {
>>>      if (ptr == NULL) {
>>>          fprintf(stderr, "Failed to allocate memory: %s\n", 
>>> strerror(errno));
>>> -        abort();
>>> +        exit(EXIT_FAILURE);
>>
>> exit() will call any atexit()/on_exit() handlers, as well as trying
>> to flush I/O streams. Any of these actions may require further
>> memory allocations, which will likely fail, or worse cause this
>> code to re-enter itself if an atexit() handler calls qemu_malloc
>>
>> The only option other than abort(), is to use  _Exit() which
>> doesn't try to run cleanup handlers.
>
> Correct, but in that case, then you need to fflush(stderr) prior to
> _Exit(), or else use write() rather than fprintf(), since otherwise your
> attempt at a nice oom error message is lost.
Great, pls see next version.
>
> --
> Eric Blake   ebl...@redhat.com    +1-919-301-3266
> Libvirt virtualization library http://libvirt.org
>



-- 
Regards,

Zhi Yong Wu



Re: [Qemu-devel] [PATCH] oslib: make error handling more reasonable

2012-02-12 Thread Zhi Yong Wu
On Fri, Feb 10, 2012 at 11:53 PM, Stefan Weil  wrote:
> Am 10.02.2012 16:13, schrieb Zhi Yong Wu:
>
>> On Fri, Feb 10, 2012 at 10:41 PM, Daniel P. Berrange
>>  wrote:
>>>
>>> On Fri, Feb 10, 2012 at 10:34:13PM +0800, Zhi Yong Wu wrote:

 From: Zhi Yong Wu 

 Signed-off-by: Zhi Yong Wu 
 ---
  oslib-posix.c |    4 ++--
  oslib-win32.c |    4 ++--
  2 files changed, 4 insertions(+), 4 deletions(-)

 diff --git a/oslib-posix.c b/oslib-posix.c
 index b6a3c7f..f978d56 100644
 --- a/oslib-posix.c
 +++ b/oslib-posix.c
 @@ -80,7 +80,7 @@ void *qemu_oom_check(void *ptr)
  {
     if (ptr == NULL) {
         fprintf(stderr, "Failed to allocate memory: %s\n",
 strerror(errno));
 -        abort();
 +        exit(EXIT_FAILURE);
>>>
>>>
>>> exit() will call any atexit()/on_exit() handlers, as well as trying
>>> to flush I/O streams. Any of these actions may require further
>>> memory allocations, which will likely fail, or worse cause this
>>> code to re-enter itself if an atexit() handler calls qemu_malloc
>>
>> Nice, very reasonable.
>>>
>>>
>>> The only option other than abort(), is to use  _Exit() which
>>> doesn't try to run cleanup handlers.
>>
>> I will try to send out v2
>
>
> Could you please explain why calling exit, _Exit or _exit is more
> reasonable than calling abort?
>
> abort can create core dumps or start a debugger which is
> useful for me and maybe other developers, too.
pls refer to http://lists.gnu.org/archive/html/qemu-devel/2012-02/msg01270.html.
In the scenario, the user should not see core dump, and he perhaps
think that one bug exists in qemu code.
So we hope to use _Exit() instead of abort() here.

>



-- 
Regards,

Zhi Yong Wu



Re: [Qemu-devel] [PATCH v2 2/8] qemu-ga: move channel/transport functionalit

2012-02-12 Thread MATSUDA, Daiki

(2012/02/10 13:51), Michael Roth wrote:

On Fri, Feb 10, 2012 at 11:26:32AM +0900, MATSUDA, Daiki wrote:

(2012/02/04 2:07), Michael Roth wrote:

On 02/02/2012 10:25 PM, MATSUDA, Daiki wrote:

Hi, Michael!
Thank you for your working.

And I have a question the process id written in pid file.
If qemu-ga is ran as daemon, the parent process id not child is written
in pid file. So, id gotten by 'ps' command is different. Is it correct
work? Many other daemon writes child process id.

Regards
MATSUDA Daiki



Hi Matsuda,

Thank you for testing!

In the become_daemon() function, the parent exits immediately after the
fork(), so only the child has the opportunity to write to the pid file.
It calls getpid() to get the pid to write, which should be it's own
lwpid. So I'm not seeing where there's an opportunity for the parent pid
to be written.

Can you confirm? It seems to behave as expected for me:

[root@vm ~]# /home/mdroth/w/qemu-build/qemu-ga -d
** (process:7441): DEBUG: starting daemon
[root@vm ~]# ps aux | grep qemu-ga
root 7442 0.0 0.0 13792 348 ? Ss 10:56 0:00
/home/mdroth/w/qemu-build/qemu-ga -d
root 7471 0.0 0.1 109108 816 pts/2 R+ 11:00 0:00 grep --color=auto qemu-ga
[root@vm ~]# cat /var/run/qemu-ga.pid
7442



Hi, Michael.

Sorry, it will be my mistake. The child process id is written in pid file.

And in this week, I tried your Windows Guest Agent patches. It is a
little hard way.

At the first I wrote my work.
0. my node OS is RHEL 6.1.
1. build some mingw packages (mingw32-gcc, mingw-32-glib2...)
2. apply your patches to qemu-kvm HEAD source. There is no error.
3. boot WinXP SP3 and Win Server 2008 R2 with VirtIO Console and
Guest Agent options.
4. install VirtIO Serial driver for Windows in
http://alt.fedoraproject.org/pub/alt/virtio-win/latest/images/bin/virtio-win-0.1-15.iso
5. communicate to node OS with viosel-test.exe included upper iso file is OK
6. copy qemu-ga.exe with some dlls included in mingw32 packages
7. 'qemu-ga --service install' does not work well. but 'qemu-ga -s
install' works well.


I see the problem, small bug in the getopt_long() usage that wasn't
triggering on POSIX. I'll fix that up shortly.


8. qemu-ga.exe works. And get good response for guest-info command.
9. But after 30 seconds, qemu-ga.exe displays following and not work
correctly.
1328840605.265625: critical: error retrieving overlapped result: 995
1328840605.296875: critical: channel error, removing source

If possible, could you point my work?


Can you reproduce with the -v option added and post the the output? I
haven't encountered this error code and the documentation for it seems
strange. Does this occur with both WinXP and 2008?


Yes, it occurs with both WinXP and 2008.
And pastes the log with '-v'
C:\qemu-ga>qemuga.exe -v
1329094617.593750: debug: prepare
1329094618.109375: debug: check
 repeat sometimes ...
1329094623.171875: debug: prepare
1329094623.625000: debug: check
1329094623.625000: critical: error retrieving overlapped result: 995
1329094623.640625: debug: dispatch
1329094623.640625: warninng: error reading channel
1329094623.640625: critical: channel error, removing source


If it's not too much trouble, can you try to reproduce with qemu.git
(for WinXP SP3)? Wondering if qemu vs. qemu-kvm is affecting the
virtio-serial driver.


Yes. I am using the qemu.git from
http://repo.or.cz/w/qemu.git.

In addition, now I change the hypervisor from RHEL 6.1 default to qemu 
git tree. But same problem occurs.





Re: [Qemu-devel] [RFC][PATCH 00/16 v6] introducing a new, dedicated memory dump mechanism

2012-02-12 Thread Wen Congyang
At 02/09/2012 11:16 AM, Wen Congyang Wrote:
> Hi, all
> 
> 'virsh dump' can not work when host pci device is used by guest. We have
> discussed this issue here:
> http://lists.nongnu.org/archive/html/qemu-devel/2011-10/msg00736.html
> 
> We have determined to introduce a new command dump to dump memory. The core
> file's format can be elf.

Hi, Jan Kiszka

At 01/10/2012 09:30 PM, Luiz Capitulino Wrote:
> Btw, I'd like to have an ack from Jan for the general approach of this
> command.
> 

Do you agree with the general approach of this command?

Thanks
Wen Congyang

> 
> Note:
> 1. The guest should be x86 or x86_64. The other arch is not supported.
> 2. If you use old gdb, gdb may crash. I use gdb-7.3.1, and it does not crash.
> 3. If the OS is in the second kernel, gdb may not work well, and crash can
>work by specifying '--machdep phys_addr=xxx' in the command line. The
>reason is that the second kernel will update the page table, and we can
>not get the page table for the first kernel.
> 4. If the guest OS is 32 bit and the memory size is larger than 4G, the vmcore
>is elf64 format. You should use the gdb which is built with 
> --enable-64-bit-bfd.
> 5. This patchset is based on the upstream tree, and apply one patch that is 
> still
>in Luiz Capitulino's tree, because I use the API qemu_get_fd() in this 
> patchset.
> 
> Changes from v5 to v6:
> 1. allow user to dump a fraction of the memory
> 2. fix some bugs
> 
> Changes from v4 to v5:
> 1. convert the new command dump to QAPI 
> 
> Changes from v3 to v4:
> 1. support it to run asynchronously
> 2. add API to cancel dumping and query dumping progress
> 3. add API to control dumping speed
> 4. auto cancel dumping when the user resumes vm, and the status is failed.
> 
> Changes from v2 to v3:
> 1. address Jan Kiszka's comment
> 
> Changes from v1 to v2:
> 1. fix virt addr in the vmcore.
> 
> Wen Congyang (16):
>   monitor: introduce qemu_suspend_monitor()/qemu_resume_monitor()
>   Add API to create memory mapping list
>   Add API to check whether a physical address is I/O address
>   target-i386: implement cpu_get_memory_mapping()
>   Add API to get memory mapping
>   target-i386: Add API to write elf notes to core file
>   target-i386: Add API to add extra memory mapping
>   target-i386: add API to get dump info
>   introduce a new monitor command 'dump' to dump guest's memory
>   run dump at the background
>   support detached dump
>   support to cancel the current dumping
>   support to set dumping speed
>   support to query dumping status
>   auto cancel dumping after vm state is changed to run
>   allow user to dump a fraction of the memory
> 
>  Makefile.target |   11 +-
>  cpu-all.h   |   18 +
>  cpu-common.h|2 +
>  dump.c  |  885 
> +++
>  dump.h  |   13 +
>  exec.c  |   16 +
>  hmp-commands.hx |   49 +++
>  hmp.c   |   49 +++
>  hmp.h   |4 +
>  memory_mapping.c|  222 
>  memory_mapping.h|   41 +++
>  monitor.c   |   37 ++
>  monitor.h   |2 +
>  qapi-schema.json|   72 
>  qmp-commands.hx |  119 +++
>  target-i386/arch-dump.c |  574 ++
>  vl.c|5 +-
>  17 files changed, 2112 insertions(+), 7 deletions(-)
>  create mode 100644 dump.c
>  create mode 100644 dump.h
>  create mode 100644 memory_mapping.c
>  create mode 100644 memory_mapping.h
>  create mode 100644 target-i386/arch-dump.c
> 
> 




Re: [Qemu-devel] weird qdev error

2012-02-12 Thread Michael S. Tsirkin
On Mon, Feb 13, 2012 at 02:17:35AM +0200, Michael S. Tsirkin wrote:
> On Sun, Feb 12, 2012 at 02:19:19PM -0600, Anthony Liguori wrote:
> > On 02/12/2012 02:15 PM, Michael S. Tsirkin wrote:
> > >On Sun, Feb 12, 2012 at 02:04:29PM -0600, Anthony Liguori wrote:
> > >>On 02/12/2012 11:57 AM, Michael S. Tsirkin wrote:
> > >>>On Sun, Feb 12, 2012 at 11:38:24AM -0600, Anthony Liguori wrote:
> > From: Anthony Liguori
> > Date: Sun, 12 Feb 2012 11:36:24 -0600
> > Subject: [PATCH] device_add: don't add a /peripheral link until init is 
> > complete
> > 
> > Otherwise we end up with a dangling reference which causes qdev_free() 
> > to fail.
> > 
> > Reported-by: Michael Tsirkin
> > Signed-off-by: Anthony Liguori
> > >>>
> > >>>This handles the option parsing but what about hotplug
> > >>>failures (when bus->hotplug returns an error)?
> > >>
> > >>Sorry, I don't follow.
> > >>
> > >>The assert you reported was that object_free() noted a reference
> > >>count of !0 which indicates something else was holding the reference
> > >>to the object.  In this case, it was the child link in /peripheral.
> > >>
> > >>By delaying creating the link in /peripheral, we eliminate the problem 
> > >>completely.
> > >
> > >Th other problem was internal in pci which calls ->hostplug
> > >during initialization. This doesn't seem affected?
> > >But I didn't try, maybe I misundertand.
> > 
> > Yeah, from qdev's perspective it's all just init failing.  hotplug
> > is entirely a PCI concept.
> > 
> > >
> > >>BTW, the explicit calls to do_pci_unregister are redundant.
> > >>finalize() will be called during cleanup which means exit() will be
> > >>invoked (which already calls do_pci_unregister).  I'm not sure why
> > >>this isn't failing more aggressively but it looks clearly wrong to
> > >>me.
> > >>
> > >>Regards,
> > >>
> > >>Anthony Liguori
> > >
> > >Me too. Want to try to drop them?
> > 
> > Yeah, I'll make this a two patch series.
> > 
> > Regards,
> > 
> > Anthony Liguori
> 
> 
> I also see this:
> 
> device_add virtio-net-pci,netdev=foo,mac=52:54:00:12:34:56,id=bla
> device_del bla
> *** glibc detected *** /home/mst/qemu-test/bin/qemu-system-x86_64:
> corrupted double-linked list: 0x7fae434565a0 ***
> 
> Am I alone?
> 
> 


Tried some tracing:  I set breakpoint at g_free
and gave the device_del command.

(gdb) b g_free
Breakpoint 3 at 0x773f55b0
(gdb) c
Continuing.

Program received signal SIGTSTP, Stopped (user).
[Switching to Thread 0x7fffaf1a3700 (LWP 22749)]
0x76ae074b in pthread_cond_timedwait@@GLIBC_2.3.2 () from 
/lib64/libpthread.so.0
(gdb) c
Continuing.

Program received signal SIGTSTP, Stopped (user).
[Switching to Thread 0x74b28700 (LWP 22727)]
0x76ae03cc in pthread_cond_wait@@GLIBC_2.3.2 () from 
/lib64/libpthread.so.0
(gdb) continue
Continuing.

Program received signal SIGTSTP, Stopped (user).
[Switching to Thread 0x77cb3700 (LWP 22712)]
0x7604c383 in select () from /lib64/libc.so.6
(gdb) continue
Continuing.
[Thread 0x7fffaf1a3700 (LWP 22749) exited]


Breakpoint 3, 0x773f55b0 in g_free () from /lib64/libglib-2.0.so.0
(gdb) continue
Continuing.
(qemu) device_del bla

Breakpoint 3, 0x773f55b0 in g_free () from /lib64/libglib-2.0.so.0
(gdb) where
#0  0x773f55b0 in g_free () from /lib64/libglib-2.0.so.0
#1  0x77ec1d60 in monitor_parse_command (mon=0x78b207a0, 
cmdline=, qdict=0x79161660)
at /home/mst/scm/qemu/monitor.c:3724
#2  0x77ec2684 in handle_user_command (mon=0x78b207a0, cmdline=
0x78c86a30 "device_del bla") at /home/mst/scm/qemu/monitor.c:3803
#3  0x77ec2b6e in monitor_command_cb (mon=0x78b207a0, 
cmdline=, opaque=)
at /home/mst/scm/qemu/monitor.c:4436
#4  0x77e463ed in readline_handle_byte (rs=0x78c86a30, 
ch=) at readline.c:370
#5  0x77ec28b8 in monitor_read (opaque=, buf=
0x7fffce20 "\r\023\f\366\377\177", size=1) at 
/home/mst/scm/qemu/monitor.c:4422
#6  0x77e313bb in qemu_chr_be_write (opaque=0x78b0ca00) at 
qemu-char.c:163
#7  fd_chr_read (opaque=0x78b0ca00) at qemu-char.c:587
#8  0x77d7c967 in qemu_iohandler_poll (readfds=0x7fffdfb0, writefds=
0x7fffdf30, xfds=, ret=)
at iohandler.c:121
#9  0x77e1085f in main_loop_wait (nonblocking=)
at main-loop.c:464
#10 0x77e09284 in main_loop (argc=, 
argv=, envp=)
at /home/mst/scm/qemu/vl.c:1482
#11 main (argc=, argv=, 
envp=) at /home/mst/scm/qemu/vl.c:3525
(gdb) frame 1
#1  0x77ec1d60 in monitor_parse_command (mon=0x78b207a0, 
cmdline=, qdict=0x79161660)
at /home/mst/scm/qemu/monitor.c:3724
3724g_free(key);
(gdb) p key
$1 = 0x79166820 "id"
(gdb) continue
Continuing.

Breakpoint 3, 0x773f55b0 in g_free () from /lib64/libglib-2.0.so.0
(gdb) where
#0  0x773f55b0 in g_free () from /lib64/libglib-2.0.so.0
#1  0x77e42fd4 in object_property_del (

Re: [Qemu-devel] weird qdev error

2012-02-12 Thread Michael S. Tsirkin
On Sun, Feb 12, 2012 at 02:19:19PM -0600, Anthony Liguori wrote:
> On 02/12/2012 02:15 PM, Michael S. Tsirkin wrote:
> >On Sun, Feb 12, 2012 at 02:04:29PM -0600, Anthony Liguori wrote:
> >>On 02/12/2012 11:57 AM, Michael S. Tsirkin wrote:
> >>>On Sun, Feb 12, 2012 at 11:38:24AM -0600, Anthony Liguori wrote:
> From: Anthony Liguori
> Date: Sun, 12 Feb 2012 11:36:24 -0600
> Subject: [PATCH] device_add: don't add a /peripheral link until init is 
> complete
> 
> Otherwise we end up with a dangling reference which causes qdev_free() to 
> fail.
> 
> Reported-by: Michael Tsirkin
> Signed-off-by: Anthony Liguori
> >>>
> >>>This handles the option parsing but what about hotplug
> >>>failures (when bus->hotplug returns an error)?
> >>
> >>Sorry, I don't follow.
> >>
> >>The assert you reported was that object_free() noted a reference
> >>count of !0 which indicates something else was holding the reference
> >>to the object.  In this case, it was the child link in /peripheral.
> >>
> >>By delaying creating the link in /peripheral, we eliminate the problem 
> >>completely.
> >
> >Th other problem was internal in pci which calls ->hostplug
> >during initialization. This doesn't seem affected?
> >But I didn't try, maybe I misundertand.
> 
> Yeah, from qdev's perspective it's all just init failing.  hotplug
> is entirely a PCI concept.
> 
> >
> >>BTW, the explicit calls to do_pci_unregister are redundant.
> >>finalize() will be called during cleanup which means exit() will be
> >>invoked (which already calls do_pci_unregister).  I'm not sure why
> >>this isn't failing more aggressively but it looks clearly wrong to
> >>me.
> >>
> >>Regards,
> >>
> >>Anthony Liguori
> >
> >Me too. Want to try to drop them?
> 
> Yeah, I'll make this a two patch series.
> 
> Regards,
> 
> Anthony Liguori


I also see this:

device_add virtio-net-pci,netdev=foo,mac=52:54:00:12:34:56,id=bla
device_del bla
*** glibc detected *** /home/mst/qemu-test/bin/qemu-system-x86_64:
corrupted double-linked list: 0x7fae434565a0 ***

Am I alone?





Re: [Qemu-devel] virtio-blk performance regression and qemu-kvm

2012-02-12 Thread Rusty Russell
On Fri, 10 Feb 2012 15:36:39 +0100, Dongsu Park  
wrote:
> Hi,
> 
> Recently I observed performance regression regarding virtio-blk,
> especially different IO bandwidths between qemu-kvm 0.14.1 and 1.0.
> So I want to share the benchmark results, and ask you what the reason
> would be.

Interesting.  There are two obvious possibilities here.  One is that
qemu has regressed, the other is that virtio_blk has regressed; the new
qemu may negotiate new features.  Please do the following in the guest
with old and new qemus:

cat /sys/class/block/vdb/device/features

(eg, here that gives: 001010110110100e0).

Thanks,
Rusty.



Re: [Qemu-devel] slirp-related crash

2012-02-12 Thread Jan Kiszka
On 2012-02-12 19:34, Michael S. Tsirkin wrote:
> It seems somewhat easy to crash qemu with slirp if we queue multiple packets.
> I didn't investigate further yet so I don't know if this
> is a regression. Anyone knowledgeable about slirp wants to take a look?
> 
> /home/mst/qemu-test/bin/qemu-system-x86_64  -enable-kvm -m 1G -drive
> file=/home/mst/rhel6.qcow2 -netdev user,id=bar -net
> nic,netdev=bar,model=e1000,macaddr=52:54:00:12:34:57  -redir
> tcp:8022::22  -vnc :1 -monitor stdio
> 
> While guest is booting, quickly do this
> 
> ssh localhost -p 8022
> CTRL-C
> ssh localhost -p 8022
> CTRL-C
> ssh localhost -p 8022
> CTRL-C
> ssh localhost -p 8022
> CTRL-C

Confirmed. A single canceled connection prior the interface setup is
enough. Possibly something is not properly removed / cleaned up here.
Will see if I find some time to debug, can't promise.

Jan

> 
> When guest triest to bring up link,
> qemu crashes:
> 
> Program received signal SIGSEGV, Segmentation fault.
> 0x77e4f8a7 in slirp_insque (a=0x0, b=0x791681f0) at
> slirp/misc.c:27
> 27  element->qh_link = head->qh_link;
> (gdb) where
> #0  0x77e4f8a7 in slirp_insque (a=0x0, b=0x791681f0) at
> slirp/misc.c:27
> #1  0x77e4ddd8 in if_start (slirp=0x78b0e4f0) at
> slirp/if.c:194
> #2  0x77e51290 in slirp_select_poll (readfds=0x7fffdfe0,
> writefds=
> 0x7fffdf60, xfds=0x7fffdee0, select_error=0) at
> slirp/slirp.c:588
> #3  0x77e114c3 in main_loop_wait (nonblocking= out>)
> at main-loop.c:466
> #4  0x77e09ed4 in main_loop (argc=, 
> argv=, envp=)
> at /home/mst/scm/qemu/vl.c:1482
> #5  main (argc=, argv=, 
> envp=) at /home/mst/scm/qemu/vl.c:3525
> (gdb) p element
> $1 = (struct quehead *) 0x0
> 
> 




signature.asc
Description: OpenPGP digital signature


Re: [Qemu-devel] weird qdev error

2012-02-12 Thread Anthony Liguori

On 02/12/2012 02:15 PM, Michael S. Tsirkin wrote:

On Sun, Feb 12, 2012 at 02:04:29PM -0600, Anthony Liguori wrote:

On 02/12/2012 11:57 AM, Michael S. Tsirkin wrote:

On Sun, Feb 12, 2012 at 11:38:24AM -0600, Anthony Liguori wrote:

From: Anthony Liguori
Date: Sun, 12 Feb 2012 11:36:24 -0600
Subject: [PATCH] device_add: don't add a /peripheral link until init is complete

Otherwise we end up with a dangling reference which causes qdev_free() to fail.

Reported-by: Michael Tsirkin
Signed-off-by: Anthony Liguori


This handles the option parsing but what about hotplug
failures (when bus->hotplug returns an error)?


Sorry, I don't follow.

The assert you reported was that object_free() noted a reference
count of !0 which indicates something else was holding the reference
to the object.  In this case, it was the child link in /peripheral.

By delaying creating the link in /peripheral, we eliminate the problem 
completely.


Th other problem was internal in pci which calls ->hostplug
during initialization. This doesn't seem affected?
But I didn't try, maybe I misundertand.


Yeah, from qdev's perspective it's all just init failing.  hotplug is entirely a 
PCI concept.





BTW, the explicit calls to do_pci_unregister are redundant.
finalize() will be called during cleanup which means exit() will be
invoked (which already calls do_pci_unregister).  I'm not sure why
this isn't failing more aggressively but it looks clearly wrong to
me.

Regards,

Anthony Liguori


Me too. Want to try to drop them?


Yeah, I'll make this a two patch series.

Regards,

Anthony Liguori








Re: [Qemu-devel] weird qdev error

2012-02-12 Thread Michael S. Tsirkin
On Sun, Feb 12, 2012 at 02:04:29PM -0600, Anthony Liguori wrote:
> On 02/12/2012 11:57 AM, Michael S. Tsirkin wrote:
> >On Sun, Feb 12, 2012 at 11:38:24AM -0600, Anthony Liguori wrote:
> >>From: Anthony Liguori
> >>Date: Sun, 12 Feb 2012 11:36:24 -0600
> >>Subject: [PATCH] device_add: don't add a /peripheral link until init is 
> >>complete
> >>
> >>Otherwise we end up with a dangling reference which causes qdev_free() to 
> >>fail.
> >>
> >>Reported-by: Michael Tsirkin
> >>Signed-off-by: Anthony Liguori
> >
> >This handles the option parsing but what about hotplug
> >failures (when bus->hotplug returns an error)?
> 
> Sorry, I don't follow.
> 
> The assert you reported was that object_free() noted a reference
> count of !0 which indicates something else was holding the reference
> to the object.  In this case, it was the child link in /peripheral.
> 
> By delaying creating the link in /peripheral, we eliminate the problem 
> completely.

Th other problem was internal in pci which calls ->hostplug
during initialization. This doesn't seem affected?
But I didn't try, maybe I misundertand.

> BTW, the explicit calls to do_pci_unregister are redundant.
> finalize() will be called during cleanup which means exit() will be
> invoked (which already calls do_pci_unregister).  I'm not sure why
> this isn't failing more aggressively but it looks clearly wrong to
> me.
> 
> Regards,
> 
> Anthony Liguori

Me too. Want to try to drop them?




Re: [Qemu-devel] weird qdev error

2012-02-12 Thread Anthony Liguori

On 02/12/2012 11:57 AM, Michael S. Tsirkin wrote:

On Sun, Feb 12, 2012 at 11:38:24AM -0600, Anthony Liguori wrote:

From: Anthony Liguori
Date: Sun, 12 Feb 2012 11:36:24 -0600
Subject: [PATCH] device_add: don't add a /peripheral link until init is complete

Otherwise we end up with a dangling reference which causes qdev_free() to fail.

Reported-by: Michael Tsirkin
Signed-off-by: Anthony Liguori


This handles the option parsing but what about hotplug
failures (when bus->hotplug returns an error)?


Sorry, I don't follow.

The assert you reported was that object_free() noted a reference count of !0 
which indicates something else was holding the reference to the object.  In this 
case, it was the child link in /peripheral.


By delaying creating the link in /peripheral, we eliminate the problem 
completely.

BTW, the explicit calls to do_pci_unregister are redundant.  finalize() will be 
called during cleanup which means exit() will be invoked (which already calls 
do_pci_unregister).  I'm not sure why this isn't failing more aggressively but 
it looks clearly wrong to me.


Regards,

Anthony Liguori



[Qemu-devel] slirp-related crash

2012-02-12 Thread Michael S. Tsirkin
It seems somewhat easy to crash qemu with slirp if we queue multiple packets.
I didn't investigate further yet so I don't know if this
is a regression. Anyone knowledgeable about slirp wants to take a look?

/home/mst/qemu-test/bin/qemu-system-x86_64  -enable-kvm -m 1G -drive
file=/home/mst/rhel6.qcow2 -netdev user,id=bar -net
nic,netdev=bar,model=e1000,macaddr=52:54:00:12:34:57  -redir
tcp:8022::22  -vnc :1 -monitor stdio

While guest is booting, quickly do this

ssh localhost -p 8022
CTRL-C
ssh localhost -p 8022
CTRL-C
ssh localhost -p 8022
CTRL-C
ssh localhost -p 8022
CTRL-C

When guest triest to bring up link,
qemu crashes:

Program received signal SIGSEGV, Segmentation fault.
0x77e4f8a7 in slirp_insque (a=0x0, b=0x791681f0) at
slirp/misc.c:27
27  element->qh_link = head->qh_link;
(gdb) where
#0  0x77e4f8a7 in slirp_insque (a=0x0, b=0x791681f0) at
slirp/misc.c:27
#1  0x77e4ddd8 in if_start (slirp=0x78b0e4f0) at
slirp/if.c:194
#2  0x77e51290 in slirp_select_poll (readfds=0x7fffdfe0,
writefds=
0x7fffdf60, xfds=0x7fffdee0, select_error=0) at
slirp/slirp.c:588
#3  0x77e114c3 in main_loop_wait (nonblocking=)
at main-loop.c:466
#4  0x77e09ed4 in main_loop (argc=, 
argv=, envp=)
at /home/mst/scm/qemu/vl.c:1482
#5  main (argc=, argv=, 
envp=) at /home/mst/scm/qemu/vl.c:3525
(gdb) p element
$1 = (struct quehead *) 0x0


-- 
MST



Re: [Qemu-devel] [PATCH] docs: memory.txt document the endian field

2012-02-12 Thread Michael S. Tsirkin
On Sun, Feb 12, 2012 at 07:20:07PM +0100, Andreas Färber wrote:
> Am 12.02.2012 16:06, schrieb Michael S. Tsirkin:
> > So I think the following is right?
> > 
> > 
> > commit 02aa79aac9bec1c8c17d1b7b5405b59b649dfdb9
> > Author: Michael S. Tsirkin 
> > Date:   Wed Feb 8 17:16:35 2012 +0200
> > 
> > docs: memory.txt document the endian field
> > 
> > This is an attempt to document the endian
> > field in memory API. As this is a confusing topic,
> > add some examples.
> > 
> > Signed-off-by: Michael S. Tsirkin 
> > 
> > diff --git a/docs/memory.txt b/docs/memory.txt
> > index 5bbee8e..9132c86 100644
> > --- a/docs/memory.txt
> > +++ b/docs/memory.txt
> > @@ -170,3 +170,48 @@ various constraints can be supplied to control how 
> > these callbacks are called:
> >   - .old_portio and .old_mmio can be used to ease porting from code using
> > cpu_register_io_memory() and register_ioport().  They should not be used
> > in new code.
> > +- .endianness; specifies the device endian-ness, which affects
> > +   the handling of the value parameter passed from guest to write
> > +   and returned to guest from read callbacks, as follows:
> > +void write(void *opaque, target_phys_addr_t addr,
> > +   uint64_t value, unsigned size)
> > +uint64_t read(void *opaque, target_phys_addr_t addr,
> > +   unsigned size)
> > +   value is always passed in the natural host format,
> > +   low size bytes in value are set, the rest are zero padded
> > +   on input and ignored on output.
> 
> Looks good so far.
> 
> > +   Legal values for endian-ness are:
> > +   DEVICE_NATIVE_ENDIAN - The value is left in the format used by guest.
> > +   Note that although this is typically a fixed format as
> > +   guest drivers take care of endian conversions,
> 
> > +   if host endian-ness does not match the device this will
> > +   result in "mixed endian" since the data is always
> > +   stored in low bits of value.
> 
> Why "mixed" endian? The host always uses host endianness, and with
> "native" we use the (nominal) endianness of the target.
> Note that the endianness of the guest might be different from the
> target's if the CPU is bi-endian.
> 
> > +
> > +   To handle this data, on write, you typically need to first
> > +   convert to the appropriate type, removing the
> > +   padding. On read, handle the data in the appropriate
> > +   type and then convert to uint64_t, padding with leading zeroes.
> 
> That applies to all three endiannesses, doesn't it?
> 
> Andreas
> > +
> > +   DEVICE_LITTLE_ENDIAN - The value is assumed to be
> > +   endian, and is converted to host endian.
> > +   DEVICE_BIG_ENDIAN - The value is assumed to be
> > +big endian, and is converted to host endian.
> > +
> > +As an example, consider a little endian guest writing a 32 bit
> > +value 0x12345678 into an MMIO register, on a big endian host.
> > +The value passed to the write callback is documented below:
> > +
> > +   DEVICE_NATIVE_ENDIAN - value = 0x87654321
> > +Explanation: write callback will get the high bits
> > +in value set to 0, and low bits set to data left
> > +as is, that is in little endian format.
> > +   DEVICE_LITTLE_ENDIAN - value = 0x12345678
> > +Explanation: the write callback will get the high bits
> > +in value set to 0, and low bits set to data in big endian
> > +format.
> > +   DEVICE_BIG_ENDIAN - value = 0x87654321
> > +Explanation: the write callback will get the high bits
> > +in value set to 0, and low bits set to data in little endian
> > +format.
> > +
> 


It looks like the text is wrong anyway.
I give up for now, maybe Avi can document it
properly.


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



Re: [Qemu-devel] [PATCH] docs: memory.txt document the endian field

2012-02-12 Thread Andreas Färber
Am 12.02.2012 16:06, schrieb Michael S. Tsirkin:
> So I think the following is right?
> 
> 
> commit 02aa79aac9bec1c8c17d1b7b5405b59b649dfdb9
> Author: Michael S. Tsirkin 
> Date:   Wed Feb 8 17:16:35 2012 +0200
> 
> docs: memory.txt document the endian field
> 
> This is an attempt to document the endian
> field in memory API. As this is a confusing topic,
> add some examples.
> 
> Signed-off-by: Michael S. Tsirkin 
> 
> diff --git a/docs/memory.txt b/docs/memory.txt
> index 5bbee8e..9132c86 100644
> --- a/docs/memory.txt
> +++ b/docs/memory.txt
> @@ -170,3 +170,48 @@ various constraints can be supplied to control how these 
> callbacks are called:
>   - .old_portio and .old_mmio can be used to ease porting from code using
> cpu_register_io_memory() and register_ioport().  They should not be used
> in new code.
> +- .endianness; specifies the device endian-ness, which affects
> +   the handling of the value parameter passed from guest to write
> +   and returned to guest from read callbacks, as follows:
> +void write(void *opaque, target_phys_addr_t addr,
> +   uint64_t value, unsigned size)
> +uint64_t read(void *opaque, target_phys_addr_t addr,
> +   unsigned size)
> +   value is always passed in the natural host format,
> +   low size bytes in value are set, the rest are zero padded
> +   on input and ignored on output.

Looks good so far.

> +   Legal values for endian-ness are:
> +   DEVICE_NATIVE_ENDIAN - The value is left in the format used by guest.
> +   Note that although this is typically a fixed format as
> +   guest drivers take care of endian conversions,

> +   if host endian-ness does not match the device this will
> +   result in "mixed endian" since the data is always
> +   stored in low bits of value.

Why "mixed" endian? The host always uses host endianness, and with
"native" we use the (nominal) endianness of the target.

Note that the endianness of the guest might be different from the
target's if the CPU is bi-endian.

> +
> +   To handle this data, on write, you typically need to first
> +   convert to the appropriate type, removing the
> +   padding. On read, handle the data in the appropriate
> +   type and then convert to uint64_t, padding with leading zeroes.

That applies to all three endiannesses, doesn't it?

Andreas

> +
> +   DEVICE_LITTLE_ENDIAN - The value is assumed to be
> +   endian, and is converted to host endian.
> +   DEVICE_BIG_ENDIAN - The value is assumed to be
> +big endian, and is converted to host endian.
> +
> +As an example, consider a little endian guest writing a 32 bit
> +value 0x12345678 into an MMIO register, on a big endian host.
> +The value passed to the write callback is documented below:
> +
> +   DEVICE_NATIVE_ENDIAN - value = 0x87654321
> +Explanation: write callback will get the high bits
> +in value set to 0, and low bits set to data left
> +as is, that is in little endian format.
> +   DEVICE_LITTLE_ENDIAN - value = 0x12345678
> +Explanation: the write callback will get the high bits
> +in value set to 0, and low bits set to data in big endian
> +format.
> +   DEVICE_BIG_ENDIAN - value = 0x87654321
> +Explanation: the write callback will get the high bits
> +in value set to 0, and low bits set to data in little endian
> +format.
> +

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



Re: [Qemu-devel] weird qdev error

2012-02-12 Thread Michael S. Tsirkin
On Sun, Feb 12, 2012 at 11:38:24AM -0600, Anthony Liguori wrote:
> On 02/12/2012 11:31 AM, Michael S. Tsirkin wrote:
> >On Sun, Feb 12, 2012 at 07:07:43PM +0200, Michael S. Tsirkin wrote:
> >>I got this assert when working on qemu: pci hotplug
> >>callback failed so qdev_free was called.
> >>
> >>(gdb) where
> >>#0  0x75fa1905 in raise () from /lib64/libc.so.6
> >>#1  0x75fa30e5 in abort () from /lib64/libc.so.6
> >>#2  0x77413a7f in g_assertion_message () from
> >>/lib64/libglib-2.0.so.0
> >>#3  0x77414020 in g_assertion_message_expr () from
> >>/lib64/libglib-2.0.so.0
> >>#4  0x77e452a9 in object_delete (obj=0x79124e60) at
> >>qom/object.c:375
> >>#5  0x77e2f5d4 in qdev_free (dev=0x79124e60)
> >> at /home/mst/scm/qemu/hw/qdev.c:250
> >>#6  qdev_init (dev=0x79124e60) at /home/mst/scm/qemu/hw/qdev.c:149
> >>#7  0x77e2a7fe in qdev_device_add (opts=0x78b0d3a0)
> >> at /home/mst/scm/qemu/hw/qdev-monitor.c:473
> >>#8  0x77e06da9 in device_init_func (opts=,
> >> opaque=) at /home/mst/scm/qemu/vl.c:1754
> >>#9  0x77e3737a in qemu_opts_foreach (list=,
> >>func=
> >> 0x77e06d90, opaque=0x0,
> >> abort_on_failure=) at qemu-option.c:1048
> >>#10 0x77e09cdb in main (argc=, argv= >>optimized out>,
> >> envp=) at /home/mst/scm/qemu/vl.c:3407
> >>(gdb) frame 6
> >>#6  qdev_init (dev=0x79124e60) at /home/mst/scm/qemu/hw/qdev.c:149
> >>149 qdev_free(dev);
> >>
> >>The problems seems to be that
> >>pci_qdev_init calls do_pci_unregister_device on
> >>hotplug  error which will free the device twice?
> >
> >Here's a reproducer to a similar error in property parsing:
> >
> >qemu-system-x86_64  -enable-kvm -m 1G -drive file=/home/mst/rhel6.qcow2
> >-netdev user,id=bar -net
> >nic,netdev=bar,model=e1000,macaddr=52:54:00:12:34:57  -redir
> >tcp:8022::22 -device virtio-net-pci,netdev=foo,mac=5854:00:12:34:56
> >-netdev
> >tap,id=foo,ifname=msttap0,script=/home/mst/ifup,downscript=no,vhost=on
> >-vnc :1 -monitor stdio
> 
> Here's the fix.  I need to do some regression testing and then I'll
> post as a proper top-level patch.
> 
> Thanks for the report.
> 
> Regards,
> 
> Anthony Liguori
> 
> >
> >
> >
> >>--
> >>MST
> 

> >From b7fc6f1eb7c5e041eac7d610061a1be950707e5b Mon Sep 17 00:00:00 2001
> From: Anthony Liguori 
> Date: Sun, 12 Feb 2012 11:36:24 -0600
> Subject: [PATCH] device_add: don't add a /peripheral link until init is 
> complete
> 
> Otherwise we end up with a dangling reference which causes qdev_free() to 
> fail.
> 
> Reported-by: Michael Tsirkin 
> Signed-off-by: Anthony Liguori 

This handles the option parsing but what about hotplug
failures (when bus->hotplug returns an error)?

> ---
>  hw/qdev-monitor.c |   18 ++
>  1 files changed, 10 insertions(+), 8 deletions(-)
> 
> diff --git a/hw/qdev-monitor.c b/hw/qdev-monitor.c
> index 49f13ca..a310cc7 100644
> --- a/hw/qdev-monitor.c
> +++ b/hw/qdev-monitor.c
> @@ -457,6 +457,16 @@ DeviceState *qdev_device_add(QemuOpts *opts)
>  id = qemu_opts_id(opts);
>  if (id) {
>  qdev->id = id;
> +}
> +if (qemu_opt_foreach(opts, set_property, qdev, 1) != 0) {
> +qdev_free(qdev);
> +return NULL;
> +}
> +if (qdev_init(qdev) < 0) {
> +qerror_report(QERR_DEVICE_INIT_FAILED, driver);
> +return NULL;
> +}
> +if (qdev->id) {
>  object_property_add_child(qdev_get_peripheral(), qdev->id,
>OBJECT(qdev), NULL);
>  } else {
> @@ -466,14 +476,6 @@ DeviceState *qdev_device_add(QemuOpts *opts)
>OBJECT(qdev), NULL);
>  g_free(name);
>  }
> -if (qemu_opt_foreach(opts, set_property, qdev, 1) != 0) {
> -qdev_free(qdev);
> -return NULL;
> -}
> -if (qdev_init(qdev) < 0) {
> -qerror_report(QERR_DEVICE_INIT_FAILED, driver);
> -return NULL;
> -}
>  qdev->opts = opts;
>  return qdev;
>  }
> -- 
> 1.7.4.1
> 




Re: [Qemu-devel] weird qdev error

2012-02-12 Thread Anthony Liguori

On 02/12/2012 11:31 AM, Michael S. Tsirkin wrote:

On Sun, Feb 12, 2012 at 07:07:43PM +0200, Michael S. Tsirkin wrote:

I got this assert when working on qemu: pci hotplug
callback failed so qdev_free was called.

(gdb) where
#0  0x75fa1905 in raise () from /lib64/libc.so.6
#1  0x75fa30e5 in abort () from /lib64/libc.so.6
#2  0x77413a7f in g_assertion_message () from
/lib64/libglib-2.0.so.0
#3  0x77414020 in g_assertion_message_expr () from
/lib64/libglib-2.0.so.0
#4  0x77e452a9 in object_delete (obj=0x79124e60) at
qom/object.c:375
#5  0x77e2f5d4 in qdev_free (dev=0x79124e60)
 at /home/mst/scm/qemu/hw/qdev.c:250
#6  qdev_init (dev=0x79124e60) at /home/mst/scm/qemu/hw/qdev.c:149
#7  0x77e2a7fe in qdev_device_add (opts=0x78b0d3a0)
 at /home/mst/scm/qemu/hw/qdev-monitor.c:473
#8  0x77e06da9 in device_init_func (opts=,
 opaque=) at /home/mst/scm/qemu/vl.c:1754
#9  0x77e3737a in qemu_opts_foreach (list=,
func=
 0x77e06d90, opaque=0x0,
 abort_on_failure=) at qemu-option.c:1048
#10 0x77e09cdb in main (argc=, argv=,
 envp=) at /home/mst/scm/qemu/vl.c:3407
(gdb) frame 6
#6  qdev_init (dev=0x79124e60) at /home/mst/scm/qemu/hw/qdev.c:149
149 qdev_free(dev);

The problems seems to be that
pci_qdev_init calls do_pci_unregister_device on
hotplug  error which will free the device twice?


Here's a reproducer to a similar error in property parsing:

qemu-system-x86_64  -enable-kvm -m 1G -drive file=/home/mst/rhel6.qcow2
-netdev user,id=bar -net
nic,netdev=bar,model=e1000,macaddr=52:54:00:12:34:57  -redir
tcp:8022::22 -device virtio-net-pci,netdev=foo,mac=5854:00:12:34:56
-netdev
tap,id=foo,ifname=msttap0,script=/home/mst/ifup,downscript=no,vhost=on
-vnc :1 -monitor stdio


Here's the fix.  I need to do some regression testing and then I'll post as a 
proper top-level patch.


Thanks for the report.

Regards,

Anthony Liguori






--
MST


>From b7fc6f1eb7c5e041eac7d610061a1be950707e5b Mon Sep 17 00:00:00 2001
From: Anthony Liguori 
Date: Sun, 12 Feb 2012 11:36:24 -0600
Subject: [PATCH] device_add: don't add a /peripheral link until init is complete

Otherwise we end up with a dangling reference which causes qdev_free() to fail.

Reported-by: Michael Tsirkin 
Signed-off-by: Anthony Liguori 
---
 hw/qdev-monitor.c |   18 ++
 1 files changed, 10 insertions(+), 8 deletions(-)

diff --git a/hw/qdev-monitor.c b/hw/qdev-monitor.c
index 49f13ca..a310cc7 100644
--- a/hw/qdev-monitor.c
+++ b/hw/qdev-monitor.c
@@ -457,6 +457,16 @@ DeviceState *qdev_device_add(QemuOpts *opts)
 id = qemu_opts_id(opts);
 if (id) {
 qdev->id = id;
+}
+if (qemu_opt_foreach(opts, set_property, qdev, 1) != 0) {
+qdev_free(qdev);
+return NULL;
+}
+if (qdev_init(qdev) < 0) {
+qerror_report(QERR_DEVICE_INIT_FAILED, driver);
+return NULL;
+}
+if (qdev->id) {
 object_property_add_child(qdev_get_peripheral(), qdev->id,
   OBJECT(qdev), NULL);
 } else {
@@ -466,14 +476,6 @@ DeviceState *qdev_device_add(QemuOpts *opts)
   OBJECT(qdev), NULL);
 g_free(name);
 }
-if (qemu_opt_foreach(opts, set_property, qdev, 1) != 0) {
-qdev_free(qdev);
-return NULL;
-}
-if (qdev_init(qdev) < 0) {
-qerror_report(QERR_DEVICE_INIT_FAILED, driver);
-return NULL;
-}
 qdev->opts = opts;
 return qdev;
 }
-- 
1.7.4.1



Re: [Qemu-devel] weird qdev error

2012-02-12 Thread Michael S. Tsirkin
On Sun, Feb 12, 2012 at 07:07:43PM +0200, Michael S. Tsirkin wrote:
> I got this assert when working on qemu: pci hotplug
> callback failed so qdev_free was called.
> 
> (gdb) where
> #0  0x75fa1905 in raise () from /lib64/libc.so.6
> #1  0x75fa30e5 in abort () from /lib64/libc.so.6
> #2  0x77413a7f in g_assertion_message () from
> /lib64/libglib-2.0.so.0
> #3  0x77414020 in g_assertion_message_expr () from
> /lib64/libglib-2.0.so.0
> #4  0x77e452a9 in object_delete (obj=0x79124e60) at
> qom/object.c:375
> #5  0x77e2f5d4 in qdev_free (dev=0x79124e60)
> at /home/mst/scm/qemu/hw/qdev.c:250
> #6  qdev_init (dev=0x79124e60) at /home/mst/scm/qemu/hw/qdev.c:149
> #7  0x77e2a7fe in qdev_device_add (opts=0x78b0d3a0)
> at /home/mst/scm/qemu/hw/qdev-monitor.c:473
> #8  0x77e06da9 in device_init_func (opts=, 
> opaque=) at /home/mst/scm/qemu/vl.c:1754
> #9  0x77e3737a in qemu_opts_foreach (list=,
> func=
> 0x77e06d90 , opaque=0x0, 
> abort_on_failure=) at qemu-option.c:1048
> #10 0x77e09cdb in main (argc=, argv= optimized out>, 
> envp=) at /home/mst/scm/qemu/vl.c:3407
> (gdb) frame 6
> #6  qdev_init (dev=0x79124e60) at /home/mst/scm/qemu/hw/qdev.c:149
> 149 qdev_free(dev);
> 
> The problems seems to be that
> pci_qdev_init calls do_pci_unregister_device on
> hotplug  error which will free the device twice?

Here's a reproducer to a similar error in property parsing:

qemu-system-x86_64  -enable-kvm -m 1G -drive file=/home/mst/rhel6.qcow2
-netdev user,id=bar -net
nic,netdev=bar,model=e1000,macaddr=52:54:00:12:34:57  -redir
tcp:8022::22 -device virtio-net-pci,netdev=foo,mac=5854:00:12:34:56
-netdev
tap,id=foo,ifname=msttap0,script=/home/mst/ifup,downscript=no,vhost=on
-vnc :1 -monitor stdio



> -- 
> MST



[Qemu-devel] weird qdev error

2012-02-12 Thread Michael S. Tsirkin
I got this assert when working on qemu: pci hotplug
callback failed so qdev_free was called.

(gdb) where
#0  0x75fa1905 in raise () from /lib64/libc.so.6
#1  0x75fa30e5 in abort () from /lib64/libc.so.6
#2  0x77413a7f in g_assertion_message () from
/lib64/libglib-2.0.so.0
#3  0x77414020 in g_assertion_message_expr () from
/lib64/libglib-2.0.so.0
#4  0x77e452a9 in object_delete (obj=0x79124e60) at
qom/object.c:375
#5  0x77e2f5d4 in qdev_free (dev=0x79124e60)
at /home/mst/scm/qemu/hw/qdev.c:250
#6  qdev_init (dev=0x79124e60) at /home/mst/scm/qemu/hw/qdev.c:149
#7  0x77e2a7fe in qdev_device_add (opts=0x78b0d3a0)
at /home/mst/scm/qemu/hw/qdev-monitor.c:473
#8  0x77e06da9 in device_init_func (opts=, 
opaque=) at /home/mst/scm/qemu/vl.c:1754
#9  0x77e3737a in qemu_opts_foreach (list=,
func=
0x77e06d90 , opaque=0x0, 
abort_on_failure=) at qemu-option.c:1048
#10 0x77e09cdb in main (argc=, argv=, 
envp=) at /home/mst/scm/qemu/vl.c:3407
(gdb) frame 6
#6  qdev_init (dev=0x79124e60) at /home/mst/scm/qemu/hw/qdev.c:149
149 qdev_free(dev);

The problems seems to be that
pci_qdev_init calls do_pci_unregister_device on
hotplug  error which will free the device twice?

-- 
MST



Re: [Qemu-devel] How to follow a child process created in the guest OS?

2012-02-12 Thread Wei Yang
2012/2/11 malc :
> On Sat, 11 Feb 2012, Andreas F?rber wrote:
>
>> Am 10.02.2012 11:26, schrieb ???:
>> > On Fri, Feb 10, 2012 at 08:14:41AM +, Stefan Hajnoczi wrote:
>> >> On Thu, Feb 09, 2012 at 06:33:16PM +0800, ??? wrote:
>> >>> I am running a tiny OS on QEMU and debugging it with gdbstub. The tiny 
>> >>> OS will
>> >>> fork process 1, 2, ... and so on. I want to follow the child process, 
>> >>> [...]
>> >>>
>> >>>   Is there a way to do what I'm trying to do? Thanks!
>>
>> > - Tiny OS code -
>> > void main(void)   /* This really IS void, no error here. */
>> > {
>> >   /* initialize enviroment */
>> >
>> >   sti();
>> >   move_to_user_mode();
>> >   if (!fork()) {    /* we count on this going ok */
>> >     init();         // task 1
>> >   }
>> >
>> >   for(;;) pause();  // task 0
>> > }
>> > 
>> >
>> >   I am running this tiny OS on QEMU then using GDB to connect it.
>> > I want to follow task 1 after the forking, [...]
>>

Could the Qemu gdbstub debug a user space process?

-- 
Richard Yang
Help You, Help Me



Re: [Qemu-devel] How to follow a child process created in the guest OS?

2012-02-12 Thread 陳韋任
On Fri, Feb 10, 2012 at 11:48:05PM +, Paul Brook wrote:
> >   I am running this tiny OS on QEMU then using GDB to connect it.
> > 
> > I want to follow task 1 after the forking, but it seems that GDB
> > stick with task 0 and cannot follow task 1 even I do `set follow-fork-mode
> > child`.
> 
> You have exactly one CPU. That's what the qemu GDB stub exposes.  Multiple 
> processes are an illusion created by your operating system.  It is not 
> something qemu knows or cares about.
> 
> In most cases if you want to do debugging within that OS created illusion 
> (aka 
> a userspace process) then you probably don't want to be using a hardware 
> debug 
> probe (i.e. the qemu gdb stub) at all. Instead you want to be using the debug 
> facilities provided by your operating system.  On linux this would be ptrace, 
> probably via gdbserver.

  I see. Thanks.

Regards,
chenwj

-- 
Wei-Ren Chen (陳韋任)
Computer Systems Lab, Institute of Information Science,
Academia Sinica, Taiwan (R.O.C.)
Tel:886-2-2788-3799 #1667
Homepage: http://people.cs.nctu.edu.tw/~chenwj



Re: [Qemu-devel] [PATCH] docs: memory.txt document the endian field

2012-02-12 Thread Avi Kivity
On 02/12/2012 05:06 PM, Michael S. Tsirkin wrote:
> > It's really really simple:
> > 
> > If the device spec says "big endian, specify DEVICE_BIG_ENDIAN, and
> > treat the data naturally in the callback.
> > If the device spec says "little endian, specify DEVICE_LITTLE_ENDIAN,
> > and treat the data naturally in the callback.
> > 
> > That's it.
>
> OKay, but I'm sure your API does not go read the spec, so
> we should not base the description on that :)
> Right?
>
> So I think the following is right?
>
>
> commit 02aa79aac9bec1c8c17d1b7b5405b59b649dfdb9
> Author: Michael S. Tsirkin 
> Date:   Wed Feb 8 17:16:35 2012 +0200
>
> docs: memory.txt document the endian field
> 
> This is an attempt to document the endian
> field in memory API. As this is a confusing topic,
> add some examples.
> 
> Signed-off-by: Michael S. Tsirkin 
>
> diff --git a/docs/memory.txt b/docs/memory.txt
> index 5bbee8e..9132c86 100644
> --- a/docs/memory.txt
> +++ b/docs/memory.txt
> @@ -170,3 +170,48 @@ various constraints can be supplied to control how these 
> callbacks are called:
>   - .old_portio and .old_mmio can be used to ease porting from code using
> cpu_register_io_memory() and register_ioport().  They should not be used
> in new code.
> +- .endianness; specifies the device endian-ness, which affects
> +   the handling of the value parameter passed from guest to write
> +   and returned to guest from read callbacks, as follows:
> +void write(void *opaque, target_phys_addr_t addr,
> +   uint64_t value, unsigned size)
> +uint64_t read(void *opaque, target_phys_addr_t addr,
> +   unsigned size)
> +   value is always passed in the natural host format,
> +   low size bytes in value are set, the rest are zero padded
> +   on input and ignored on output.
> +   Legal values for endian-ness are:
> +   DEVICE_NATIVE_ENDIAN - The value is left in the format used by guest.
> +   Note that although this is typically a fixed format as
> +   guest drivers take care of endian conversions,
> +   if host endian-ness does not match the device this will
> +   result in "mixed endian" since the data is always
> +   stored in low bits of value.
> +
> +   To handle this data, on write, you typically need to first
> +   convert to the appropriate type, removing the
> +   padding. On read, handle the data in the appropriate
> +   type and then convert to uint64_t, padding with leading zeroes.

No.  Data is converted from guest endian to host endian on write (vice
versa on read).  This works if the device endianness matches the guest
endianness.

> +
> +   DEVICE_LITTLE_ENDIAN - The value is assumed to be
> +   endian, and is converted to host endian.
> +   DEVICE_BIG_ENDIAN - The value is assumed to be
> +big endian, and is converted to host endian.

Yes.

> +
> +As an example, consider a little endian guest writing a 32 bit
> +value 0x12345678 into an MMIO register, on a big endian host.
> +The value passed to the write callback is documented below:
> +
> +   DEVICE_NATIVE_ENDIAN - value = 0x87654321
> +Explanation: write callback will get the high bits
> +in value set to 0, and low bits set to data left
> +as is, that is in little endian format.

No, you'll see 0x12345678, same as DEVICE_LITTLE_ENDIAN.

> +   DEVICE_LITTLE_ENDIAN - value = 0x12345678
> +Explanation: the write callback will get the high bits
> +in value set to 0, and low bits set to data in big endian
> +format.
> +   DEVICE_BIG_ENDIAN - value = 0x87654321
> +Explanation: the write callback will get the high bits
> +in value set to 0, and low bits set to data in little endian
> +format.
> +

Right value, wrong explanation.  The value is still in big endian format.


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




[Qemu-devel] how could I analysis the trace log?

2012-02-12 Thread Wei Yang
All

I enable the trace function with --enable-trace-backend=simple and I
create the event file like this
g_realloc
g_malloc

Then I start the qemu with following command.
./i386-softmmu/qemu-system-i386 -enable-kvm -drive
file=../../kvm/ubuntu.img -boot dc -m 512 -usb
 -monitor stdio -trace events=qemu_trace_events,file=qemu_trace.log

After some run time, I run the script like:
./scripts/simpletrace.py qemu_trace_events_parse qemu_trace.log

The qemu_trace_events_parse is :
g_realloc(addr)
g_malloc(addr)

The output looks like:
g_malloc 1.831 addr=0xb945d1f0
g_malloc 2.498 addr=0xb945d1f0
g_realloc 4.715 addr=0x10
g_realloc 1.520 addr=0xc
g_realloc 1.505 addr=0xc

The steps I used is correct?
I just guess the format of input events file of the simpletrace.py.
For so many available events, how could I specify the format of all
those events?

-- 
Richard Yang
Help You, Help Me



Re: [Qemu-devel] [PATCH] docs: memory.txt document the endian field

2012-02-12 Thread Michael S. Tsirkin
On Sun, Feb 12, 2012 at 03:55:20PM +0200, Avi Kivity wrote:
> On 02/12/2012 03:47 PM, Michael S. Tsirkin wrote:
> > On Sun, Feb 12, 2012 at 03:02:11PM +0200, Avi Kivity wrote:
> > > On 02/12/2012 02:52 PM, Michael S. Tsirkin wrote:
> > > > This is an attempt to document the endian
> > > > field in memory API. As this is a confusing topic,
> > > > it's best to make the text as explicit as possible.
> > > >
> > > > Signed-off-by: Michael S. Tsirkin 
> > > > ---
> > > >  docs/memory.txt |   28 
> > > >  1 files changed, 28 insertions(+), 0 deletions(-)
> > > >
> > > > diff --git a/docs/memory.txt b/docs/memory.txt
> > > > index 5bbee8e..ff92b52 100644
> > > > --- a/docs/memory.txt
> > > > +++ b/docs/memory.txt
> > > > @@ -170,3 +170,31 @@ various constraints can be supplied to control how 
> > > > these callbacks are called:
> > > >   - .old_portio and .old_mmio can be used to ease porting from code 
> > > > using
> > > > cpu_register_io_memory() and register_ioport().  They should not be 
> > > > used
> > > > in new code.
> > > > +- .endianness; specifies the device endian-ness, which affects
> > > > +   the value parameter passed from guest to write and returned
> > > > +   to guest from read callbacks, as follows:
> > > > +void write(void *opaque, target_phys_addr_t addr,
> > > > +   uint64_t value, unsigned size)
> > > > +uint64_t read(void *opaque, target_phys_addr_t addr,
> > > > +   unsigned size)
> > > > +   Legal values are:
> > > > +   DEVICE_NATIVE_ENDIAN - Callbacks accept and return value in
> > > > +host endian format. This makes it possible to do
> > > > +math on values without type conversions.
> > > > +Low size bytes in value are set, the rest are zero padded
> > > > +on input and ignored on output.
> > > > +   DEVICE_LITTLE_ENDIAN - Callbacks accept and return value
> > > > +in little endian format. This is appropriate
> > > > +if you need to directly copy the data into device memory,
> > > > +and the device programming interface is little endian
> > > > +(true for most pci devices).
> > > > +First size bytes in value are set, the rest are zero padded
> > > > +on input and ignored on output.
> > > > +   DEVICE_BIG_ENDIAN - Callbacks accept and return value
> > > > +in big endian format.
> > > > +in little endian format. This is appropriate
> > > > +if you need to directly copy the data into device memory,
> > > > +and the device programming interface is big endian
> > > > +(true e.g. for some system devices on big endian 
> > > > architectures).
> > > > +Last size bytes in value are set, the rest are zero padded
> > > > +on input and ignored on output.
> > > 
> > > This is wrong.  Callback data is always in host endianness.  Device
> > > endianness is about the device.
> > > 
> > > For example, DEVICE_BIG_ENDIAN means that the device expects data in big
> > > endian format.  Qemu assumes the guest OS writes big endian data to the
> > > device, so it swaps from big endian to host endian before calling the
> > > callback.  Similarly it will swap from host endian to big endian on read.
> > > 
> > > DEVICE_NATIVE_ENDIAN means:
> > > 
> > >   defined(TARGET_WORDS_BIGENDIAN) ? DEVICE_BIG_ENDIAN : 
> > > DEVICE_NATIVE_ENDIAN
> > > 
> > > i.e. the device has the same endianness as the guest cpu.
> >
> > I think this boils down to the same thing in the end, no?
> 
> Maybe.
> 
> > However, it's a bad way to describe the setup
> > for device writers: it documents the
> > internal workings of qemu with multiple
> > swaps. We need to document the end result.
> >
> > And, it is IMO confusing to say that 'a device expects data'
> > this adds a speculative element where you
> > are asked to think about what you would want to
> > do and promised that this will be somehow
> > satisfied.
> >
> > Instead, please specify what the API does, users
> > can make their own decisions on when to use it.
> 
> But "callbacks accept data in little endian format" implies that you
> have to add a swap in the handler,
> since you usually want data in host endian.
> It's really really simple:
> 
> If the device spec says "big endian, specify DEVICE_BIG_ENDIAN, and
> treat the data naturally in the callback.
> If the device spec says "little endian, specify DEVICE_LITTLE_ENDIAN,
> and treat the data naturally in the callback.
> 
> That's it.

OKay, but I'm sure your API does not go read the spec, so
we should not base the description on that :)
Right?

So I think the following is right?


commit 02aa79aac9bec1c8c17d1b7b5405b59b649dfdb9
Author: Michael S. Tsirkin 
Date:   Wed Feb 8 17:16:35 2012 +0200

docs: memory.txt document the endian field

This is an attempt to document the endian
field in memory API. As this is a confusing topic,
add some examples.

Signed-off-by: Michael S

Re: [Qemu-devel] [PATCH] docs: memory.txt document the endian field

2012-02-12 Thread Avi Kivity
On 02/12/2012 03:47 PM, Michael S. Tsirkin wrote:
> On Sun, Feb 12, 2012 at 03:02:11PM +0200, Avi Kivity wrote:
> > On 02/12/2012 02:52 PM, Michael S. Tsirkin wrote:
> > > This is an attempt to document the endian
> > > field in memory API. As this is a confusing topic,
> > > it's best to make the text as explicit as possible.
> > >
> > > Signed-off-by: Michael S. Tsirkin 
> > > ---
> > >  docs/memory.txt |   28 
> > >  1 files changed, 28 insertions(+), 0 deletions(-)
> > >
> > > diff --git a/docs/memory.txt b/docs/memory.txt
> > > index 5bbee8e..ff92b52 100644
> > > --- a/docs/memory.txt
> > > +++ b/docs/memory.txt
> > > @@ -170,3 +170,31 @@ various constraints can be supplied to control how 
> > > these callbacks are called:
> > >   - .old_portio and .old_mmio can be used to ease porting from code using
> > > cpu_register_io_memory() and register_ioport().  They should not be 
> > > used
> > > in new code.
> > > +- .endianness; specifies the device endian-ness, which affects
> > > +   the value parameter passed from guest to write and returned
> > > +   to guest from read callbacks, as follows:
> > > +void write(void *opaque, target_phys_addr_t addr,
> > > +   uint64_t value, unsigned size)
> > > +uint64_t read(void *opaque, target_phys_addr_t addr,
> > > +   unsigned size)
> > > +   Legal values are:
> > > +   DEVICE_NATIVE_ENDIAN - Callbacks accept and return value in
> > > +host endian format. This makes it possible to do
> > > +math on values without type conversions.
> > > +Low size bytes in value are set, the rest are zero padded
> > > +on input and ignored on output.
> > > +   DEVICE_LITTLE_ENDIAN - Callbacks accept and return value
> > > +in little endian format. This is appropriate
> > > +if you need to directly copy the data into device memory,
> > > +and the device programming interface is little endian
> > > +(true for most pci devices).
> > > +First size bytes in value are set, the rest are zero padded
> > > +on input and ignored on output.
> > > +   DEVICE_BIG_ENDIAN - Callbacks accept and return value
> > > +in big endian format.
> > > +in little endian format. This is appropriate
> > > +if you need to directly copy the data into device memory,
> > > +and the device programming interface is big endian
> > > +(true e.g. for some system devices on big endian architectures).
> > > +Last size bytes in value are set, the rest are zero padded
> > > +on input and ignored on output.
> > 
> > This is wrong.  Callback data is always in host endianness.  Device
> > endianness is about the device.
> > 
> > For example, DEVICE_BIG_ENDIAN means that the device expects data in big
> > endian format.  Qemu assumes the guest OS writes big endian data to the
> > device, so it swaps from big endian to host endian before calling the
> > callback.  Similarly it will swap from host endian to big endian on read.
> > 
> > DEVICE_NATIVE_ENDIAN means:
> > 
> >   defined(TARGET_WORDS_BIGENDIAN) ? DEVICE_BIG_ENDIAN : DEVICE_NATIVE_ENDIAN
> > 
> > i.e. the device has the same endianness as the guest cpu.
>
> I think this boils down to the same thing in the end, no?

Maybe.

> However, it's a bad way to describe the setup
> for device writers: it documents the
> internal workings of qemu with multiple
> swaps. We need to document the end result.
>
> And, it is IMO confusing to say that 'a device expects data'
> this adds a speculative element where you
> are asked to think about what you would want to
> do and promised that this will be somehow
> satisfied.
>
> Instead, please specify what the API does, users
> can make their own decisions on when to use it.

But "callbacks accept data in little endian format" implies that you
have to add a swap in the handler, since you usually want data in host
endian.

It's really really simple:

If the device spec says "big endian, specify DEVICE_BIG_ENDIAN, and
treat the data naturally in the callback.
If the device spec says "little endian, specify DEVICE_LITTLE_ENDIAN,
and treat the data naturally in the callback.

That's it.

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




Re: [Qemu-devel] [PATCH] docs: memory.txt document the endian field

2012-02-12 Thread Michael S. Tsirkin
On Sun, Feb 12, 2012 at 03:02:11PM +0200, Avi Kivity wrote:
> On 02/12/2012 02:52 PM, Michael S. Tsirkin wrote:
> > This is an attempt to document the endian
> > field in memory API. As this is a confusing topic,
> > it's best to make the text as explicit as possible.
> >
> > Signed-off-by: Michael S. Tsirkin 
> > ---
> >  docs/memory.txt |   28 
> >  1 files changed, 28 insertions(+), 0 deletions(-)
> >
> > diff --git a/docs/memory.txt b/docs/memory.txt
> > index 5bbee8e..ff92b52 100644
> > --- a/docs/memory.txt
> > +++ b/docs/memory.txt
> > @@ -170,3 +170,31 @@ various constraints can be supplied to control how 
> > these callbacks are called:
> >   - .old_portio and .old_mmio can be used to ease porting from code using
> > cpu_register_io_memory() and register_ioport().  They should not be used
> > in new code.
> > +- .endianness; specifies the device endian-ness, which affects
> > +   the value parameter passed from guest to write and returned
> > +   to guest from read callbacks, as follows:
> > +void write(void *opaque, target_phys_addr_t addr,
> > +   uint64_t value, unsigned size)
> > +uint64_t read(void *opaque, target_phys_addr_t addr,
> > +   unsigned size)
> > +   Legal values are:
> > +   DEVICE_NATIVE_ENDIAN - Callbacks accept and return value in
> > +host endian format. This makes it possible to do
> > +math on values without type conversions.
> > +Low size bytes in value are set, the rest are zero padded
> > +on input and ignored on output.
> > +   DEVICE_LITTLE_ENDIAN - Callbacks accept and return value
> > +in little endian format. This is appropriate
> > +if you need to directly copy the data into device memory,
> > +and the device programming interface is little endian
> > +(true for most pci devices).
> > +First size bytes in value are set, the rest are zero padded
> > +on input and ignored on output.
> > +   DEVICE_BIG_ENDIAN - Callbacks accept and return value
> > +in big endian format.
> > +in little endian format. This is appropriate
> > +if you need to directly copy the data into device memory,
> > +and the device programming interface is big endian
> > +(true e.g. for some system devices on big endian architectures).
> > +Last size bytes in value are set, the rest are zero padded
> > +on input and ignored on output.
> 
> This is wrong.  Callback data is always in host endianness.  Device
> endianness is about the device.
> 
> For example, DEVICE_BIG_ENDIAN means that the device expects data in big
> endian format.  Qemu assumes the guest OS writes big endian data to the
> device, so it swaps from big endian to host endian before calling the
> callback.  Similarly it will swap from host endian to big endian on read.
> 
> DEVICE_NATIVE_ENDIAN means:
> 
>   defined(TARGET_WORDS_BIGENDIAN) ? DEVICE_BIG_ENDIAN : DEVICE_NATIVE_ENDIAN
> 
> i.e. the device has the same endianness as the guest cpu.

I think this boils down to the same thing in the end, no?

However, it's a bad way to describe the setup
for device writers: it documents the
internal workings of qemu with multiple
swaps. We need to document the end result.

And, it is IMO confusing to say that 'a device expects data'
this adds a speculative element where you
are asked to think about what you would want to
do and promised that this will be somehow
satisfied.

Instead, please specify what the API does, users
can make their own decisions on when to use it.

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



Re: [Qemu-devel] [PATCH] docs: memory.txt document the endian field

2012-02-12 Thread Avi Kivity
On 02/12/2012 02:52 PM, Michael S. Tsirkin wrote:
> This is an attempt to document the endian
> field in memory API. As this is a confusing topic,
> it's best to make the text as explicit as possible.
>
> Signed-off-by: Michael S. Tsirkin 
> ---
>  docs/memory.txt |   28 
>  1 files changed, 28 insertions(+), 0 deletions(-)
>
> diff --git a/docs/memory.txt b/docs/memory.txt
> index 5bbee8e..ff92b52 100644
> --- a/docs/memory.txt
> +++ b/docs/memory.txt
> @@ -170,3 +170,31 @@ various constraints can be supplied to control how these 
> callbacks are called:
>   - .old_portio and .old_mmio can be used to ease porting from code using
> cpu_register_io_memory() and register_ioport().  They should not be used
> in new code.
> +- .endianness; specifies the device endian-ness, which affects
> +   the value parameter passed from guest to write and returned
> +   to guest from read callbacks, as follows:
> +void write(void *opaque, target_phys_addr_t addr,
> +   uint64_t value, unsigned size)
> +uint64_t read(void *opaque, target_phys_addr_t addr,
> +   unsigned size)
> +   Legal values are:
> +   DEVICE_NATIVE_ENDIAN - Callbacks accept and return value in
> +host endian format. This makes it possible to do
> +math on values without type conversions.
> +Low size bytes in value are set, the rest are zero padded
> +on input and ignored on output.
> +   DEVICE_LITTLE_ENDIAN - Callbacks accept and return value
> +in little endian format. This is appropriate
> +if you need to directly copy the data into device memory,
> +and the device programming interface is little endian
> +(true for most pci devices).
> +First size bytes in value are set, the rest are zero padded
> +on input and ignored on output.
> +   DEVICE_BIG_ENDIAN - Callbacks accept and return value
> +in big endian format.
> +in little endian format. This is appropriate
> +if you need to directly copy the data into device memory,
> +and the device programming interface is big endian
> +(true e.g. for some system devices on big endian architectures).
> +Last size bytes in value are set, the rest are zero padded
> +on input and ignored on output.

This is wrong.  Callback data is always in host endianness.  Device
endianness is about the device.

For example, DEVICE_BIG_ENDIAN means that the device expects data in big
endian format.  Qemu assumes the guest OS writes big endian data to the
device, so it swaps from big endian to host endian before calling the
callback.  Similarly it will swap from host endian to big endian on read.

DEVICE_NATIVE_ENDIAN means:

  defined(TARGET_WORDS_BIGENDIAN) ? DEVICE_BIG_ENDIAN : DEVICE_NATIVE_ENDIAN

i.e. the device has the same endianness as the guest cpu.

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




[Qemu-devel] [PATCH] docs: memory.txt document the endian field

2012-02-12 Thread Michael S. Tsirkin
This is an attempt to document the endian
field in memory API. As this is a confusing topic,
it's best to make the text as explicit as possible.

Signed-off-by: Michael S. Tsirkin 
---
 docs/memory.txt |   28 
 1 files changed, 28 insertions(+), 0 deletions(-)

diff --git a/docs/memory.txt b/docs/memory.txt
index 5bbee8e..ff92b52 100644
--- a/docs/memory.txt
+++ b/docs/memory.txt
@@ -170,3 +170,31 @@ various constraints can be supplied to control how these 
callbacks are called:
  - .old_portio and .old_mmio can be used to ease porting from code using
cpu_register_io_memory() and register_ioport().  They should not be used
in new code.
+- .endianness; specifies the device endian-ness, which affects
+   the value parameter passed from guest to write and returned
+   to guest from read callbacks, as follows:
+void write(void *opaque, target_phys_addr_t addr,
+   uint64_t value, unsigned size)
+uint64_t read(void *opaque, target_phys_addr_t addr,
+   unsigned size)
+   Legal values are:
+   DEVICE_NATIVE_ENDIAN - Callbacks accept and return value in
+host endian format. This makes it possible to do
+math on values without type conversions.
+Low size bytes in value are set, the rest are zero padded
+on input and ignored on output.
+   DEVICE_LITTLE_ENDIAN - Callbacks accept and return value
+in little endian format. This is appropriate
+if you need to directly copy the data into device memory,
+and the device programming interface is little endian
+(true for most pci devices).
+First size bytes in value are set, the rest are zero padded
+on input and ignored on output.
+   DEVICE_BIG_ENDIAN - Callbacks accept and return value
+in big endian format.
+in little endian format. This is appropriate
+if you need to directly copy the data into device memory,
+and the device programming interface is big endian
+(true e.g. for some system devices on big endian architectures).
+Last size bytes in value are set, the rest are zero padded
+on input and ignored on output.
-- 
1.7.9.111.gf3fb0



Re: [Qemu-devel] [PATCH] libcacard configure fixes

2012-02-12 Thread Alon Levy
On Thu, Feb 09, 2012 at 07:05:29PM +, Paul Brook wrote:
> libcacard is only used by system emulation.
> Only define libcacard_libs/cflags once.
> 

ACK.

Anthony, do you want a single patch pull request in general or is it ok
to ask that you pick this directly?

Alon

> Signed-off-by: Paul Brook 
> ---
>  configure |   12 ++--
>  1 files changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/configure b/configure
> index 763db24..faa65a8 100755
> --- a/configure
> +++ b/configure
> @@ -2569,7 +2569,7 @@ EOF
>compile_prog "$smartcard_cflags $libcacard_cflags" 
> "$libcacard_libs"; then
>  smartcard_nss="yes"
>  QEMU_CFLAGS="$QEMU_CFLAGS $smartcard_cflags $libcacard_cflags"
> -LIBS="$libcacard_libs $LIBS"
> +libs_softmmu="$libcacard_libs $libs_softmmu"
>  else
>  if test "$smartcard_nss" = "yes"; then
>  feature_not_found "nss"
> @@ -3209,6 +3209,8 @@ fi
>  
>  if test "$smartcard_nss" = "yes" ; then
>echo "CONFIG_SMARTCARD_NSS=y" >> $config_host_mak
> +  echo "libcacard_libs=$libcacard_libs" >> $config_host_mak
> +  echo "libcacard_cflags=$libcacard_cflags" >> $config_host_mak
>  fi
>  
>  if test "$usb_redir" = "yes" ; then
> @@ -3628,6 +3630,9 @@ if test "$target_softmmu" = "yes" ; then
>echo "LIBS+=$libs_softmmu $target_libs_softmmu" >> $config_target_mak
>echo "HWDIR=../libhw$target_phys_bits" >> $config_target_mak
>echo "subdir-$target: subdir-libhw$target_phys_bits" >> $config_host_mak
> +  if test "$smartcard_nss" = "yes" ; then
> +echo "subdir-$target: subdir-libcacard" >> $config_host_mak
> +  fi
>  fi
>  if test "$target_user_only" = "yes" ; then
>echo "CONFIG_USER_ONLY=y" >> $config_target_mak
> @@ -3639,11 +3644,6 @@ fi
>  if test "$target_darwin_user" = "yes" ; then
>echo "CONFIG_DARWIN_USER=y" >> $config_target_mak
>  fi
> -if test "$smartcard_nss" = "yes" ; then
> -  echo "subdir-$target: subdir-libcacard" >> $config_host_mak
> -  echo "libcacard_libs=$libcacard_libs" >> $config_host_mak
> -  echo "libcacard_cflags=$libcacard_cflags" >> $config_host_mak
> -fi
>  list=""
>  if test ! -z "$gdb_xml_files" ; then
>for x in $gdb_xml_files; do
> -- 
> 1.7.8.3
> 
> 



Re: [Qemu-devel] [Spice-devel] [PATCH] server: support IPV6 addresses in channel events sent to qemu

2012-02-12 Thread Alon Levy
On Wed, Feb 08, 2012 at 03:39:35PM +0200, Yonit Halperin wrote:
> RHBZ #788444
769512

ACK, just a few comment fixes (take it or leave it).

> 
> CC: Gerd Hoffmann 
> 
> Signed-off-by: Gerd Hoffmann 
> Signed-off-by: Yonit Halperin 
> ---
>  server/reds.c  |   21 +
>  server/spice.h |6 ++
>  2 files changed, 23 insertions(+), 4 deletions(-)
> 
> diff --git a/server/reds.c b/server/reds.c
> index 2492a89..828ba65 100644
> --- a/server/reds.c
> +++ b/server/reds.c
> @@ -2005,7 +2005,7 @@ static void reds_get_spice_ticket(RedLinkInfo *link)
>  
>  #if HAVE_SASL
>  static char *addr_to_string(const char *format,
> -struct sockaddr *sa,
> +struct sockaddr_storage *sa,
>  socklen_t salen) {
>  char *addr;
>  char host[NI_MAXHOST];
> @@ -2013,7 +2013,7 @@ static char *addr_to_string(const char *format,
>  int err;
>  size_t addrlen;
>  
> -if ((err = getnameinfo(sa, salen,
> +if ((err = getnameinfo((struct sockaddr *)sa, salen,
> host, sizeof(host),
> serv, sizeof(serv),
> NI_NUMERICHOST | NI_NUMERICSERV)) != 0) {
> @@ -2402,11 +2402,13 @@ static void reds_start_auth_sasl(RedLinkInfo *link)
>  RedsSASL *sasl = &link->stream->sasl;
>  
>  /* Get local & remote client addresses in form  IPADDR;PORT */
> -if (!(localAddr = addr_to_string("%s;%s", &link->stream->info.laddr, 
> link->stream->info.llen))) {
> +if (!(localAddr = addr_to_string("%s;%s", &link->stream->info.laddr_ext,
> +  link->stream->info.llen_ext))) 
> {
>  goto error;
>  }
>  
> -if (!(remoteAddr = addr_to_string("%s;%s", &link->stream->info.paddr, 
> link->stream->info.plen))) {
> +if (!(remoteAddr = addr_to_string("%s;%s", &link->stream->info.paddr_ext,
> +   
> link->stream->info.plen_ext))) {
>  free(localAddr);
>  goto error;
>  }
> @@ -2717,10 +2719,21 @@ static RedLinkInfo *reds_init_client_connection(int 
> socket)
>  
>  stream->socket = socket;
>  /* gather info + send event */
> +
> +/* deprecated fields. Filling them for backward compatibility */
/* deprecated fields. Filling them for backward compatibility, will
 * truncate for ipv6 addresses (RHBZ #769512) */

>  stream->info.llen = sizeof(stream->info.laddr);
>  stream->info.plen = sizeof(stream->info.paddr);
>  getsockname(stream->socket, (struct sockaddr*)(&stream->info.laddr), 
> &stream->info.llen);
>  getpeername(stream->socket, (struct sockaddr*)(&stream->info.paddr), 
> &stream->info.plen);
> +
> +stream->info.flags |= SPICE_CHANNEL_EVENT_FLAG_ADDR_EXT;
> +stream->info.llen_ext = sizeof(stream->info.laddr_ext);
> +stream->info.plen_ext = sizeof(stream->info.paddr_ext);
> +getsockname(stream->socket, (struct sockaddr*)(&stream->info.laddr_ext),
> +&stream->info.llen_ext);
> +getpeername(stream->socket, (struct sockaddr*)(&stream->info.paddr_ext),
> +&stream->info.plen_ext);
> +
>  reds_stream_channel_event(stream, SPICE_CHANNEL_EVENT_CONNECTED);
>  
>  openssl_init(link);
> diff --git a/server/spice.h b/server/spice.h
> index c582e6c..7397655 100644
> --- a/server/spice.h
> +++ b/server/spice.h
> @@ -54,6 +54,7 @@ typedef struct SpiceCoreInterface SpiceCoreInterface;
>  #define SPICE_CHANNEL_EVENT_DISCONNECTED  3
>  
>  #define SPICE_CHANNEL_EVENT_FLAG_TLS  (1 << 0)
> +#define SPICE_CHANNEL_EVENT_FLAG_ADDR_EXT (1 << 1)
>  
>  typedef struct SpiceWatch SpiceWatch;
>  typedef void (*SpiceWatchFunc)(int fd, int event, void *opaque);
> @@ -66,9 +67,14 @@ typedef struct SpiceChannelEventInfo {
>  int type;
>  int id;
>  int flags;
> +/* deprecated, can't hold ipv6 addresses, kept for backward 
> compatibility */
>  struct sockaddr laddr;
>  struct sockaddr paddr;
>  socklen_t llen, plen;
> +/* should be used if (flags & SPICE_CHANNEL_EVENT_FLAG_ADDR_EXT) */
valid if and only if (flags & SPICE_CHANNEL_EVENT_FLAG_ADDR_EXT)
> +struct sockaddr_storage laddr_ext;
> +struct sockaddr_storage paddr_ext;
> +socklen_t llen_ext, plen_ext;
>  } SpiceChannelEventInfo;
>  
>  struct SpiceCoreInterface {
> -- 
> 1.7.7.6
> 
> ___
> Spice-devel mailing list
> spice-de...@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/spice-devel