[Qemu-devel] [questions] savevm|loadvm

2010-03-29 Thread Wenhao Xu
Hi, all,
   I am working with switching QEMU from running in KVM mode to QEMU
emulatoin mode dynamically.
   Intuitively, if the snapshot created using savevm in kvm mode can be used
by the loadvm command in QEMU emulator mode, the switchment could makes use
of this.  I tried to do so. However, it does not work.  Any idea how to fix
it?
Thanks for the help.

regards,
Wenhao

-- 
~_~


[Qemu-devel] Re: [PATCH 4/4] Add virtio disk identification support

2010-03-29 Thread john cooper
Rusty Russell wrote:
> On Thu, 25 Mar 2010 04:04:02 pm john cooper wrote:
>> Return serial string to the guest application via
>> ioctl driver call.
> 
> This is quite nice.  Minor nits:
> 
>> +if (cmd == 'VBID') {
>> +void *usr_data = (void __user *)data;
> 
> void __user *usr_data;
> 
>> +char *id_str;
>> +int err;
>> +
>> +if (!(id_str = kmalloc(VIRTIO_BLK_ID_BYTES, GFP_KERNEL)))
>> +err = -ENOMEM;
>> +else if ((err = virtblk_get_id(disk, id_str)))
>> +;
>> +else if (copy_to_user(usr_data, id_str, VIRTIO_BLK_ID_BYTES))
>> +err = -EFAULT;
>> +if (id_str)
>> +kfree(id_str);
>> +return err;
>> +}
> 
> We can't put the id_str on the stack?  Makes it even simpler :)

At a VIRTIO_BLK_ID_BYTES of the current 20 bytes it seems
safe but there was discussion of extending it so I thought
to locate it in safer storage.

Note the above was intended as more of an example to
illustrate the mechanism.  Marc had proposed a /sys
style interface to retrieve a virtio id string which
is what motivated revisiting this issue.

Marc, if you don't foresee tying off that work relatively
soon where a /sys interface would be made available, I'll
rework the above to be a little more general.  The first
version of the S/N patch pulled a user buffer size from
the caller and limited the copy out to that length.

-john

-- 
john.coo...@redhat.com




[Qemu-devel] KVM call agenda for Mar 30

2010-03-29 Thread Chris Wright
- vhost-blk

Please send in any additional agenda items you are interested in covering.

thanks,
-chris




Re: [Qemu-devel] GSoC projects about AHCI and S3 Trio

2010-03-29 Thread Natalia Portillo

Hi,

El 30/03/2010, a las 01:55, Alexander Graf escribió:


Hi Roland,


On 30.03.2010, at 01:52, Roland Elek wrote:


Dear Qemu developers,

I am a university student from Hungary interested in contributing  
to Qemu through Google Summer of Code.
I am interested in emulation, and two projects from the ideas page  
in particular. One of them is AHCI emulation.
Can I kindly ask you what were the hardest points that made the  
project get a high difficulty rating, so that I
could determine whether to apply for it or not? At a first glance,  
I think that AHCI code from VirtualBox OSE

would be a good place to start. What do you think?


I looked at the AHCI code from vbox some time ago and deemed it  
unreadable. It's probably easier to go with the spec and implement  
it from there.


Nevertheless, if you're good at reading C++ that code might come in  
handy. Just don't expect to be able to take any of it over to  
Qemu :-). Coding styles between the two projects differ a lot.


The hard part is that it's a reasonably complex piece of hardware  
that needs to be emulated. The "high" ranking was basically to show  
people that we need someone with serious skills and eagerness to  
work on it :-). If you're a quick learner the skills part shouldn't  
be too hard. The eagerness and ambition part is the really important  
one.


A quick search through the mailing list archives showed that the  
other topic, the S3 Trio, has been around for
some time now. DOSBox has support for it, and is open source, so I  
think we could build upon that.


Yep. On IRC there's also someone called "jai" who wanted to look  
into that as well. Mind to coordinate with him?




I'm the one that proposed the S3 Trio/Virge idea.
The greatest thing about this card is that almost every operating  
system has or had a driver for it, and has also been used in non-x86  
machines extensively.


If you need testing or getting information from a real working card (I  
have a couple of them, all for x86 unfortuneately) just mail me.


Regards,
Natalia Portillo



[Qemu-devel] Re: [PATCH 4/4] Add virtio disk identification support

2010-03-29 Thread Rusty Russell
On Thu, 25 Mar 2010 04:04:02 pm john cooper wrote:
> Return serial string to the guest application via
> ioctl driver call.

This is quite nice.  Minor nits:

> + if (cmd == 'VBID') {
> + void *usr_data = (void __user *)data;

void __user *usr_data;

> + char *id_str;
> + int err;
> +
> + if (!(id_str = kmalloc(VIRTIO_BLK_ID_BYTES, GFP_KERNEL)))
> + err = -ENOMEM;
> + else if ((err = virtblk_get_id(disk, id_str)))
> + ;
> + else if (copy_to_user(usr_data, id_str, VIRTIO_BLK_ID_BYTES))
> + err = -EFAULT;
> + if (id_str)
> + kfree(id_str);
> + return err;
> + }

We can't put the id_str on the stack?  Makes it even simpler :)

Cheers,
Rusty.




Re: [Qemu-devel] Re: [PATCH 1/5] linux-user/ia64: workaround ia64 strangenesses

2010-03-29 Thread Aurelien Jarno
On Tue, Mar 30, 2010 at 01:00:39AM +0100, Jamie Lokier wrote:
> Aurelien Jarno wrote:
> > On Mon, Mar 29, 2010 at 11:36:50AM +0200, Paolo Bonzini wrote:
> > >
> > >> +#ifdef __ia64
> > >> +sigprocmask(SIG_SETMASK, (sigset_t *)&uc->uc_sigmask, NULL);
> > >> +#else
> > >>   sigprocmask(SIG_SETMASK,&uc->uc_sigmask, NULL);
> > >> +#endif
> > >
> > > Any reason for the ifdef?
> > >
> > 
> > It is not strictly needed, as all architectures can cope with the ia64
> > version. I have added it to make sure that a new architecture triggers a
> > warning if uc->uc_sigmask is not of type sigset_t, so that a human can
> > verify the cast is correct.
> 
> What type is the ia64 uc_sigmask, if not sigset_t?

It is defined as a unsigned long int. From :

|  /* sc_mask is actually an sigset_t but we don't want to
|   * include the kernel headers here. */
|  unsigned long int sc_mask;/* signal mask to restore after handler 
returns */


-- 
Aurelien Jarno  GPG: 1024D/F1BCDB73
aurel...@aurel32.net http://www.aurel32.net




[Qemu-devel] about porting a new device

2010-03-29 Thread daniel tian
Hi, guys:
 I wanna port framebuffer LCD device on mips machine. And I wrote
the code based on malta machine in qemu-0.11.1.
 The lcd device is simple, which has a fixed Framebuffer address
and always 32bit bpp. Of course this is my imaginary graphic card. and
qemu will deplay the frambebuffer data on SDL window with calling the
function: framebuffer_update_display.
 I have already wrote the kernel framebuffer LCD driver(still need
to verify on qemu.)
 Now I wrote this virtual lcd controller in qemu, and the malta
init function will call this lcd init function.
 But I have a question which have puzzled me in past two weeks.
 Here is the code structure:
 static
static
void malta_mips_init (ram_addr_t ram_size,
  const char *boot_device,
  const char *kernel_filename, const char *kernel_cmdline,
  const char *initrd_filename, const char *cpu_model)
{
..
/* Sound card */
#ifdef HAS_AUDIO
audio_init(pci_bus);
#endif

/* Network card */
network_init();
//The code above is exactly the same as malta, I just replace the
cirrus vga display part with my lcd
VirtualLcd_init(0x4000, 0x6000);
}

LCD device source code:
int VirtualLcd_init(target_phys_addr_t vram_base,
   target_phys_addr_t ctrl_base)
{
VirtualLcdState*s;
int io_ctrl;

s = qemu_mallocz(sizeof(VirtualLcdState));

s->vram_size = 4 * 640 * 480;
s->vram_offset = qemu_ram_alloc(s->vram_size);
s->vram = qemu_get_ram_ptr(s->vram_offset);

qemu_register_reset(virtual_fb_reset, s);
virtual_fb_reset(s);

s->ds = graphic_console_init(virtual_fb_update_display,
 virtual_fb_invalidate_display,
 virtual_fb_screen_dump, NULL, s);

cpu_register_physical_memory(vram_base, s->vram_size, s->vram_offset);

io_ctrl = cpu_register_io_memory(virtual_fb_ctrl_read,
virtual_fb_ctrl_write, s);
cpu_register_physical_memory(ctrl_base, 0x20, io_ctrl);

return 0;
}

There are many devices in malta machine  including my virtual lcd
controller. But I don't understand how to integrate those devices
together as a machine. I mean how those devices communicating each
other, e.g: mips cpu with virtual LCD. and How the kernel device
drivers detect corresponding device?
Do I have to simulator PCI lcd controller? PCI device seems
complicate, I just wanna be simply in both driver and simulator device
code.

Can you guys give me some advices?

Thanks very much.
Any suggestion is appreciated!



  daniel.tian




Re: [Qemu-devel] [PATCH] usb-bus: fix no params

2010-03-29 Thread TeLeMan
On Mon, Mar 29, 2010 at 20:16, Kevin Wolf  wrote:
> Am 27.03.2010 13:47, schrieb Aurelien Jarno:
>> On Fri, Mar 19, 2010 at 12:59:24PM +0800, TeLeMan wrote:
>>> The "params" is never NULL and the usb hid devices have no params.
>>
>> This looks plainly wrong. With your patch, usb devices which don't
>> accept parameters, will accept and ignore them.
>>
>> What are you trying to fix here?
>
> It looks like it's fixing -usbdevice tablet (and keyboard/mouse) which
> currently fails like this:
>
> qemu-system-x86_64: usbdevice tablet accepts no params
> qemu: could not add USB device 'tablet'
>
> He's correct in that params is never NULL (if it was NULL it's set to an
> empty string some lines earlier, introduced by 702f3e0f), so
> usb_create_simple is never called. Maybe the right fix is to check for
> *params instead of params now?
Yes, you are right. I did a new patch for it.




[Qemu-devel] [PATCHv2] usb-bus: fix no params

2010-03-29 Thread TeLeMan
After commit 702f3e0fb52c124c07f215426eeadb70a716643f, the params is
nerver NULL. It should check *params instead of params to determine
whether the params is empty.

Signed-off-by: TeLeMan 
---
 hw/usb-bus.c |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/hw/usb-bus.c b/hw/usb-bus.c
index ce8a694..ee0e9e3 100644
--- a/hw/usb-bus.c
+++ b/hw/usb-bus.c
@@ -299,7 +299,7 @@ USBDevice *usbdevice_create(const char *cmdline)
 }

 if (!usb->usbdevice_init) {
-if (params) {
+if (*params) {
 error_report("usbdevice %s accepts no params", driver);
 return NULL;
 }
-- 
1.6.5.1.1367.gcd48

--
SUN OF A BEACH




Re: [Qemu-devel] GSoC projects about AHCI and S3 Trio

2010-03-29 Thread Alexander Graf
Hi Roland,


On 30.03.2010, at 01:52, Roland Elek wrote:

> Dear Qemu developers,
> 
> I am a university student from Hungary interested in contributing to Qemu 
> through Google Summer of Code.
> I am interested in emulation, and two projects from the ideas page in 
> particular. One of them is AHCI emulation.
> Can I kindly ask you what were the hardest points that made the project get a 
> high difficulty rating, so that I
> could determine whether to apply for it or not? At a first glance, I think 
> that AHCI code from VirtualBox OSE
> would be a good place to start. What do you think?

I looked at the AHCI code from vbox some time ago and deemed it unreadable. 
It's probably easier to go with the spec and implement it from there.

Nevertheless, if you're good at reading C++ that code might come in handy. Just 
don't expect to be able to take any of it over to Qemu :-). Coding styles 
between the two projects differ a lot.

The hard part is that it's a reasonably complex piece of hardware that needs to 
be emulated. The "high" ranking was basically to show people that we need 
someone with serious skills and eagerness to work on it :-). If you're a quick 
learner the skills part shouldn't be too hard. The eagerness and ambition part 
is the really important one.

> A quick search through the mailing list archives showed that the other topic, 
> the S3 Trio, has been around for
> some time now. DOSBox has support for it, and is open source, so I think we 
> could build upon that.

Yep. On IRC there's also someone called "jai" who wanted to look into that as 
well. Mind to coordinate with him?


Alex





Re: [Qemu-devel] Re: [PATCH 1/5] linux-user/ia64: workaround ia64 strangenesses

2010-03-29 Thread Jamie Lokier
Aurelien Jarno wrote:
> On Mon, Mar 29, 2010 at 11:36:50AM +0200, Paolo Bonzini wrote:
> >
> >> +#ifdef __ia64
> >> +sigprocmask(SIG_SETMASK, (sigset_t *)&uc->uc_sigmask, NULL);
> >> +#else
> >>   sigprocmask(SIG_SETMASK,&uc->uc_sigmask, NULL);
> >> +#endif
> >
> > Any reason for the ifdef?
> >
> 
> It is not strictly needed, as all architectures can cope with the ia64
> version. I have added it to make sure that a new architecture triggers a
> warning if uc->uc_sigmask is not of type sigset_t, so that a human can
> verify the cast is correct.

What type is the ia64 uc_sigmask, if not sigset_t?
A git grep of the kernel found only:

  arch/ia64/include/asm/ucontext.h:#define uc_sigmask uc_mcontext.sc_sigmask

with sc_sigmask not defined anywhere.  The uc_mcontext.sc_mask field,
though, is a sigset_t.

-- Jamie




[Qemu-devel] GSoC projects about AHCI and S3 Trio

2010-03-29 Thread Roland Elek
Dear Qemu developers,

I am a university student from Hungary interested in contributing to Qemu
through Google Summer of Code.
I am interested in emulation, and two projects from the ideas page in
particular. One of them is AHCI emulation.
Can I kindly ask you what were the hardest points that made the project get
a high difficulty rating, so that I
could determine whether to apply for it or not? At a first glance, I think
that AHCI code from VirtualBox OSE
would be a good place to start. What do you think?

A quick search through the mailing list archives showed that the other
topic, the S3 Trio, has been around for
some time now. DOSBox has support for it, and is open source, so I think we
could build upon that.

Best regards,
Roland Elek


Re: [Qemu-devel] [PATCH -V3 09/32] virtio-9p: Implement P9_TWRITE/ Thread model in QEMU

2010-03-29 Thread Anthony Liguori

On 03/29/2010 03:54 PM, Avi Kivity wrote:

On 03/29/2010 11:42 PM, Anthony Liguori wrote:
For individual device models or host services, I think (3) is 
probably the worst model overall.  I personally think that (1) is 
better in the long run but ultimately would need an existence proof 
to compare against (2).  (2) looks appealing until you actually try 
to have the device handle multiple requests at a time.


Sooner or later nature and the ever more complicated code will force 
us towards (3).  As an example, we've observed live migration to 
throttle vcpus when sending a large guest's zeroed memory over; the 
bandwidth control doesn't kick in since zero pages are compressed, 
so the iothread spends large amounts of time reading memory.



Making things re-entrant is different than (3) in my mind.

There's no reason that VCPU threads should run in lock-step with live 
migration during the live phase.  Making device models re-entrant and 
making live migration depend not depend on the big global lock is a 
good thing to do.


It's not sufficient.  If you have a single thread that runs both live 
migrations and timers, then timers will be backlogged behind live 
migration, or you'll have to yield often.  This is regardless of the 
locking model (and of course having threads without fixing the locking 
is insufficient as well, live migration accesses guest memory so it 
needs the big qemu lock).


But what's the solution?  Sending every timer in a separate thread?  
We'll hit the same problem if we implement an arbitrary limit to number 
of threads.


What I'm skeptical of, is whether converting virtio-9p or qcow2 to 
handle each request in a separate thread is really going to improve 
things. 


Currently qcow2 isn't even fullly asynchronous, so it can't fail to 
improve things.


Unless it introduces more data corruptions which is my concern with any 
significant change to qcow2.


The VNC server is another area that I think multithreading would be a 
bad idea.


If the vnc server is stuffing a few megabytes of screen into a socket, 
then timers will be delayed behind it, unless you litter the code with 
calls to bottom halves.  Even worse if it does complicated compression 
and encryption.


Sticking the VNC server in it's own thread would be fine.  Trying to 
make the VNC server multithreaded though would be problematic.


Basically, sticking isolated components in a single thread should be 
pretty reasonable.




But if those system calls are blocking, you need a thread?


You can dispatch just the system call to a thread pool.  The 
advantage of doing that is that you don't need to worry about locking 
since the system calls are not (usually) handling shared state.


There is always implied shared state.  If you're doing direct guest 
memory access, you need to lock memory against hotunplug, or the 
syscall will end up writing into freed memory.  If the device can be 
hotunplugged, you need to make sure all threads have returned before 
unplugging it.


There are other ways to handle hot unplug (like reference counting) that 
avoid this problem.


Ultimately, this comes down to a question of lock granularity and thread 
granularity.  I don't think it's a good idea to start with the 
assumption that we want extremely fine granularity.  There's certainly 
very low hanging fruit with respect to threading.


On a philosophical note, threads may be easier to model complex 
hardware that includes a processor, for example our scsi card (and 
how about using tcg as a jit to boost it :)


Yeah, it's hard to argue that script evaluation shouldn't be done in 
a thread.  But that doesn't prevent me from being very cautious about 
how and where we use threading :-)


Caution where threads are involved is a good thing.  They are 
inevitable however, IMO.


We already are using threads so they aren't just inevitable, they're 
reality.  I still don't think using threads would significantly simplify 
virtio-9p.


Regards,

Anthony Liguori





Re: [Qemu-devel] [PATCH -V3 09/32] virtio-9p: Implement P9_TWRITE/ Thread model in QEMU

2010-03-29 Thread jvrao
Anthony Liguori wrote:
> On 03/29/2010 03:31 PM, Avi Kivity wrote:
>> On 03/29/2010 06:00 PM, Anthony Liguori wrote:
>>>
>>> In qemu, we tend to prefer state-machine based code.
>>>
>>> There are a few different models that we have discussed at different
>>> points:
>>>
>>> 1) state machines with a thread pool to make blocking functions
>>> asynchronous (what we have today)
>>>
>>> 2) co-operative threading
>>>
>>> 3) pre-emptive threading
>>>
>>> All three models are independent of making device models re-entrant. 
>>> In order to allow VCPU threads to run in qemu simultaneously, we need
>>> each device to carry a lock and for that lock to be acquired upon any
>>> of the public functions for the device model.
>>>
>>> For individual device models or host services, I think (3) is
>>> probably the worst model overall.  I personally think that (1) is
>>> better in the long run but ultimately would need an existence proof
>>> to compare against (2).  (2) looks appealing until you actually try
>>> to have the device handle multiple requests at a time.
>>
>> Sooner or later nature and the ever more complicated code will force
>> us towards (3).  As an example, we've observed live migration to
>> throttle vcpus when sending a large guest's zeroed memory over; the
>> bandwidth control doesn't kick in since zero pages are compressed, so
>> the iothread spends large amounts of time reading memory.
> 
> Making things re-entrant is different than (3) in my mind.
> 
> There's no reason that VCPU threads should run in lock-step with live
> migration during the live phase.  Making device models re-entrant and
> making live migration depend not depend on the big global lock is a good
> thing to do.
> 
> What I'm skeptical of, is whether converting virtio-9p or qcow2 to
> handle each request in a separate thread is really going to improve
> things.  The VNC server is another area that I think multithreading
> would be a bad idea.

Excuse me for some basic questions..still trying to understand QEMU concepts..

How does IO thread is accounted for? 

If I start a 2 CPU QEMU, we will be occupying two physical CPUs with two VCPU 
threads
and IO thread runs on other CPUs owned by the host right? If the answer is yes, 
then
we are good as the fileserver load has to be on the host CPUs. 

> 
>> We could fix this by yielding every so often (and a patch will be
>> posted soon), but it remains an issue.  We have too much work in the
>> I/O thread and that defers I/O completion and timers.
>>
>>> For virtio-9p, I don't think (1) is much of a burden to be honest. 
>>> In particular, when the 9p2000.L dialect is used, there should be a
>>> 1-1 correlation between protocol operations and system calls which
>>> means that for the most part, there's really no advantage to
>>> threading from a complexity point of view.
>>
>> But if those system calls are blocking, you need a thread?
> 
> You can dispatch just the system call to a thread pool.  The advantage
> of doing that is that you don't need to worry about locking since the
> system calls are not (usually) handling shared state.

How is locking avoided in the IO thread model? do we just have 1 IO thread 
irrespective of #VCPU threads? Is is that how the locking is avoided?

I think IO thread is used to do disk/network IO right? 
Putting 9P also on that will have any performance/scalability issues?

Thanks,
JV

> 
>> On a philosophical note, threads may be easier to model complex
>> hardware that includes a processor, for example our scsi card (and how
>> about using tcg as a jit to boost it :)
> 
> Yeah, it's hard to argue that script evaluation shouldn't be done in a
> thread.  But that doesn't prevent me from being very cautious about how
> and where we use threading :-)
> 
> Regards,
> 
> Anthony Liguori
> 






[Qemu-devel] Re: [PATCH v3 1/1] Shared memory uio_pci driver

2010-03-29 Thread Avi Kivity

On 03/28/2010 10:48 PM, Cam Macdonell wrote:

On Sat, Mar 27, 2010 at 11:48 AM, Avi Kivity  wrote:
   

On 03/26/2010 07:14 PM, Cam Macdonell wrote:
 
   

I'm not familiar with the uio internals, but for the interface, an
ioctl()
on the fd to assign an eventfd to an MSI vector.  Similar to ioeventfd,
but
instead of mapping a doorbell to an eventfd, it maps a real MSI to an
eventfd.

 

uio will never support ioctls.
   

Why not?
 

Perhaps I spoke too strongly, but it was rejected before

http://thread.gmane.org/gmane.linux.kernel/756481

With a compelling case perhaps it could be added.
   


Ah, the usual "ioctls are ugly, go away".

It could be done via sysfs:

  $ cat /sys/.../msix/max-interrupts
  256
  $ echo 4 > /sys/.../msix/allocate
  $ # subdirectories 0 1 2 3 magically appear
  $ # bind fd 13 to msix
  $ echo 13 > /sys/.../msix/2/bind-fd
  $ # from now on, msix interrupt 2 will call eventfd_signal() on fd 13

Call me old fashioned, but I prefer ioctls.

--
Do not meddle in the internals of kernels, for they are subtle and quick to 
panic.





Re: [Qemu-devel] Qemu-mips64

2010-03-29 Thread Stefan Weil
EHSAN UL HAQ schrieb:
> Hi,
> I have an executable file for mips64,how can i run this file using Qemu??
> Thanks,

There is currently no user mode emulation for mips64,
so you cannot run it directly.

If your executable is a Linux executable, you might run it
in a malta mips64 system emulation running (debian) linux.

Regards,
Stefan Weil





[Qemu-devel] accessing acpi (mcfg) table from qemu

2010-03-29 Thread sanjay chandra
Hi,
I need to access mcfg table from qemu, please provide some pointers how I
can access the table.

Thanks,
Sanjay


[Qemu-devel] Qemu-mips64

2010-03-29 Thread EHSAN UL HAQ

Hi,
I have an executable file for mips64,how can i run this file using Qemu??
Thanks,

  
_
Hotmail: Trusted email with Microsoft’s powerful SPAM protection.
https://signup.live.com/signup.aspx?id=60969

Re: [Qemu-devel] [PATCH -V3 09/32] virtio-9p: Implement P9_TWRITE/ Thread model in QEMU

2010-03-29 Thread Avi Kivity

On 03/29/2010 11:42 PM, Anthony Liguori wrote:
For individual device models or host services, I think (3) is 
probably the worst model overall.  I personally think that (1) is 
better in the long run but ultimately would need an existence proof 
to compare against (2).  (2) looks appealing until you actually try 
to have the device handle multiple requests at a time.


Sooner or later nature and the ever more complicated code will force 
us towards (3).  As an example, we've observed live migration to 
throttle vcpus when sending a large guest's zeroed memory over; the 
bandwidth control doesn't kick in since zero pages are compressed, so 
the iothread spends large amounts of time reading memory.



Making things re-entrant is different than (3) in my mind.

There's no reason that VCPU threads should run in lock-step with live 
migration during the live phase.  Making device models re-entrant and 
making live migration depend not depend on the big global lock is a 
good thing to do.


It's not sufficient.  If you have a single thread that runs both live 
migrations and timers, then timers will be backlogged behind live 
migration, or you'll have to yield often.  This is regardless of the 
locking model (and of course having threads without fixing the locking 
is insufficient as well, live migration accesses guest memory so it 
needs the big qemu lock).


What I'm skeptical of, is whether converting virtio-9p or qcow2 to 
handle each request in a separate thread is really going to improve 
things. 


Currently qcow2 isn't even fullly asynchronous, so it can't fail to 
improve things.


The VNC server is another area that I think multithreading would be a 
bad idea.


If the vnc server is stuffing a few megabytes of screen into a socket, 
then timers will be delayed behind it, unless you litter the code with 
calls to bottom halves.  Even worse if it does complicated compression 
and encryption.




But if those system calls are blocking, you need a thread?


You can dispatch just the system call to a thread pool.  The advantage 
of doing that is that you don't need to worry about locking since the 
system calls are not (usually) handling shared state.


There is always implied shared state.  If you're doing direct guest 
memory access, you need to lock memory against hotunplug, or the syscall 
will end up writing into freed memory.  If the device can be 
hotunplugged, you need to make sure all threads have returned before 
unplugging it.


On a philosophical note, threads may be easier to model complex 
hardware that includes a processor, for example our scsi card (and 
how about using tcg as a jit to boost it :)


Yeah, it's hard to argue that script evaluation shouldn't be done in a 
thread.  But that doesn't prevent me from being very cautious about 
how and where we use threading :-)


Caution where threads are involved is a good thing.  They are inevitable 
however, IMO.


--
Do not meddle in the internals of kernels, for they are subtle and quick to 
panic.





Re: [Qemu-devel] [PATCH -V3 09/32] virtio-9p: Implement P9_TWRITE/ Thread model in QEMU

2010-03-29 Thread Anthony Liguori

On 03/29/2010 03:31 PM, Avi Kivity wrote:

On 03/29/2010 06:00 PM, Anthony Liguori wrote:


In qemu, we tend to prefer state-machine based code.

There are a few different models that we have discussed at different 
points:


1) state machines with a thread pool to make blocking functions 
asynchronous (what we have today)


2) co-operative threading

3) pre-emptive threading

All three models are independent of making device models re-entrant.  
In order to allow VCPU threads to run in qemu simultaneously, we need 
each device to carry a lock and for that lock to be acquired upon any 
of the public functions for the device model.


For individual device models or host services, I think (3) is 
probably the worst model overall.  I personally think that (1) is 
better in the long run but ultimately would need an existence proof 
to compare against (2).  (2) looks appealing until you actually try 
to have the device handle multiple requests at a time.


Sooner or later nature and the ever more complicated code will force 
us towards (3).  As an example, we've observed live migration to 
throttle vcpus when sending a large guest's zeroed memory over; the 
bandwidth control doesn't kick in since zero pages are compressed, so 
the iothread spends large amounts of time reading memory.


Making things re-entrant is different than (3) in my mind.

There's no reason that VCPU threads should run in lock-step with live 
migration during the live phase.  Making device models re-entrant and 
making live migration depend not depend on the big global lock is a good 
thing to do.


What I'm skeptical of, is whether converting virtio-9p or qcow2 to 
handle each request in a separate thread is really going to improve 
things.  The VNC server is another area that I think multithreading 
would be a bad idea.


We could fix this by yielding every so often (and a patch will be 
posted soon), but it remains an issue.  We have too much work in the 
I/O thread and that defers I/O completion and timers.


For virtio-9p, I don't think (1) is much of a burden to be honest.  
In particular, when the 9p2000.L dialect is used, there should be a 
1-1 correlation between protocol operations and system calls which 
means that for the most part, there's really no advantage to 
threading from a complexity point of view.


But if those system calls are blocking, you need a thread?


You can dispatch just the system call to a thread pool.  The advantage 
of doing that is that you don't need to worry about locking since the 
system calls are not (usually) handling shared state.


On a philosophical note, threads may be easier to model complex 
hardware that includes a processor, for example our scsi card (and how 
about using tcg as a jit to boost it :)


Yeah, it's hard to argue that script evaluation shouldn't be done in a 
thread.  But that doesn't prevent me from being very cautious about how 
and where we use threading :-)


Regards,

Anthony Liguori





Re: [Qemu-devel] [PATCH -V3 09/32] virtio-9p: Implement P9_TWRITE/ Thread model in QEMU

2010-03-29 Thread Avi Kivity

On 03/29/2010 06:00 PM, Anthony Liguori wrote:


In qemu, we tend to prefer state-machine based code.

There are a few different models that we have discussed at different 
points:


1) state machines with a thread pool to make blocking functions 
asynchronous (what we have today)


2) co-operative threading

3) pre-emptive threading

All three models are independent of making device models re-entrant.  
In order to allow VCPU threads to run in qemu simultaneously, we need 
each device to carry a lock and for that lock to be acquired upon any 
of the public functions for the device model.


For individual device models or host services, I think (3) is probably 
the worst model overall.  I personally think that (1) is better in the 
long run but ultimately would need an existence proof to compare 
against (2).  (2) looks appealing until you actually try to have the 
device handle multiple requests at a time.


Sooner or later nature and the ever more complicated code will force us 
towards (3).  As an example, we've observed live migration to throttle 
vcpus when sending a large guest's zeroed memory over; the bandwidth 
control doesn't kick in since zero pages are compressed, so the iothread 
spends large amounts of time reading memory.


We could fix this by yielding every so often (and a patch will be posted 
soon), but it remains an issue.  We have too much work in the I/O thread 
and that defers I/O completion and timers.


For virtio-9p, I don't think (1) is much of a burden to be honest.  In 
particular, when the 9p2000.L dialect is used, there should be a 1-1 
correlation between protocol operations and system calls which means 
that for the most part, there's really no advantage to threading from 
a complexity point of view.


But if those system calls are blocking, you need a thread?

On a philosophical note, threads may be easier to model complex hardware 
that includes a processor, for example our scsi card (and how about 
using tcg as a jit to boost it :)


--
Do not meddle in the internals of kernels, for they are subtle and quick to 
panic.





[Qemu-devel] Re: [PATCH v2 0/4] monitor: Convert do_set_link() to QObject, QError

2010-03-29 Thread Luiz Capitulino
On Fri, 26 Mar 2010 09:07:07 +0100
Markus Armbruster  wrote:

> PATCH 3/4 changes syntax of set_link's second argument from up|down to
> on|off.  I feel that the argument needs to be boolean in QMP, and this
> is the simplest way to get it.  Luiz likes this approach.  The change
> doesn't affect libvirt, because it doesn't use set_link, yet.

 Looks good to me.




Re: [Qemu-devel] Check format specifiers for fprintf like function pointers

2010-03-29 Thread Stefan Weil
Stefan Weil schrieb:
> fprintf like function pointers are used for log output, especially
> for cpu / fpu register dumps.
>
> When I examined wrong values for an x86_64 target on a i386 host,
> I noticed that it was caused by wrong format specifiers and that
> there are more errors of this kind.
>
> I could fix some (see list below), but for target-mips some errors
> remain to be fixed.
>
> Regards,
> Stefan Weil
>
>
>

The list of patches was missing. Here it is:

 [PATCH 01/14] Add new data type for fprintf like function pointers
 [PATCH 02/14] Use fprint_function and fix wrong format specifiers
 [PATCH 03/14] target-alpha: Use fprintf_function
 [PATCH 04/14] target-arm: Use fprintf_function
 [PATCH 05/14] target-cris: Use fprintf_function
 [PATCH 06/14] target-i386: Use fprintf_function and fix dump of DR
registers
 [PATCH 07/14] target-m68k: Use fprintf_function
 [PATCH 08/14] target-microblaze: Use fprintf_function
 [PATCH 09/14] target-mips: Use fprintf_function and fix wrong format
specifiers
 [PATCH 10/14] target-ppc: Use fprintf_function and fix wrong format
specifiers
 [PATCH 11/14] target-sh4: Use fprintf_function
 [PATCH 12/14] target-sparc: Use fprintf_function
 [PATCH 13/14] target-s390: Use fprintf_function
 [PATCH 14/14] tcg: Use fprintf_function





[Qemu-devel] [PATCH 14/14] tcg: Use fprintf_function

2010-03-29 Thread Stefan Weil
Signed-off-by: Stefan Weil 
---
 tcg/tcg.c |6 ++
 tcg/tcg.h |3 +--
 2 files changed, 3 insertions(+), 6 deletions(-)

diff --git a/tcg/tcg.c b/tcg/tcg.c
index 3294743..a40b797 100644
--- a/tcg/tcg.c
+++ b/tcg/tcg.c
@@ -2087,8 +2087,7 @@ int tcg_gen_code_search_pc(TCGContext *s, uint8_t 
*gen_code_buf, long offset)
 }
 
 #ifdef CONFIG_PROFILER
-void tcg_dump_info(FILE *f,
-   int (*cpu_fprintf)(FILE *f, const char *fmt, ...))
+void tcg_dump_info(FILE *f, fprintf_function cpu_fprintf)
 {
 TCGContext *s = &tcg_ctx;
 int64_t tot;
@@ -2132,8 +2131,7 @@ void tcg_dump_info(FILE *f,
 dump_op_count();
 }
 #else
-void tcg_dump_info(FILE *f,
-   int (*cpu_fprintf)(FILE *f, const char *fmt, ...))
+void tcg_dump_info(FILE *f, fprintf_function cpu_fprintf)
 {
 cpu_fprintf(f, "[TCG profiler not compiled]\n");
 }
diff --git a/tcg/tcg.h b/tcg/tcg.h
index 166c889..5bf57c6 100644
--- a/tcg/tcg.h
+++ b/tcg/tcg.h
@@ -384,8 +384,7 @@ static inline TCGv_i64 tcg_temp_local_new_i64(void)
 void tcg_temp_free_i64(TCGv_i64 arg);
 char *tcg_get_arg_str_i64(TCGContext *s, char *buf, int buf_size, TCGv_i64 
arg);
 
-void tcg_dump_info(FILE *f,
-   int (*cpu_fprintf)(FILE *f, const char *fmt, ...));
+void tcg_dump_info(FILE *f, fprintf_function cpu_fprintf);
 
 #define TCG_CT_ALIAS  0x80
 #define TCG_CT_IALIAS 0x40
-- 
1.7.0





[Qemu-devel] [PATCH 13/14] target-s390: Use fprintf_function

2010-03-29 Thread Stefan Weil
Signed-off-by: Stefan Weil 
---
 target-s390x/translate.c |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/target-s390x/translate.c b/target-s390x/translate.c
index 44dfa65..b4b5537 100644
--- a/target-s390x/translate.c
+++ b/target-s390x/translate.c
@@ -24,7 +24,7 @@
 #include "qemu-log.h"
 
 void cpu_dump_state(CPUState *env, FILE *f,
-int (*cpu_fprintf)(FILE *f, const char *fmt, ...),
+fprintf_function cpu_fprintf,
 int flags)
 {
 int i;
-- 
1.7.0





[Qemu-devel] [PATCH 12/14] target-sparc: Use fprintf_function

2010-03-29 Thread Stefan Weil
Signed-off-by: Stefan Weil 
---
 target-sparc/helper.c |8 
 1 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/target-sparc/helper.c b/target-sparc/helper.c
index 1f0f7d4..d1914d9 100644
--- a/target-sparc/helper.c
+++ b/target-sparc/helper.c
@@ -1261,7 +1261,7 @@ static const char * const feature_name[] = {
 };
 
 static void print_features(FILE *f,
-   int (*cpu_fprintf)(FILE *f, const char *fmt, ...),
+   fprintf_function cpu_fprintf,
uint32_t features, const char *prefix)
 {
 unsigned int i;
@@ -1389,7 +1389,7 @@ static int cpu_sparc_find_by_name(sparc_def_t *cpu_def, 
const char *cpu_model)
 return -1;
 }
 
-void sparc_cpu_list(FILE *f, int (*cpu_fprintf)(FILE *f, const char *fmt, ...))
+void sparc_cpu_list(FILE *f, fprintf_function cpu_fprintf)
 {
 unsigned int i;
 
@@ -1417,7 +1417,7 @@ void sparc_cpu_list(FILE *f, int (*cpu_fprintf)(FILE *f, 
const char *fmt, ...))
 }
 
 static void cpu_print_cc(FILE *f,
- int (*cpu_fprintf)(FILE *f, const char *fmt, ...),
+ fprintf_function cpu_fprintf,
  uint32_t cc)
 {
 cpu_fprintf(f, "%c%c%c%c", cc & PSR_NEG? 'N' : '-',
@@ -1432,7 +1432,7 @@ static void cpu_print_cc(FILE *f,
 #endif
 
 void cpu_dump_state(CPUState *env, FILE *f,
-int (*cpu_fprintf)(FILE *f, const char *fmt, ...),
+fprintf_function cpu_fprintf,
 int flags)
 {
 int i, x;
-- 
1.7.0





[Qemu-devel] [PATCH 11/14] target-sh4: Use fprintf_function

2010-03-29 Thread Stefan Weil
Signed-off-by: Stefan Weil 
---
 target-sh4/cpu.h   |2 +-
 target-sh4/translate.c |2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/target-sh4/cpu.h b/target-sh4/cpu.h
index f8b1680..78602af 100644
--- a/target-sh4/cpu.h
+++ b/target-sh4/cpu.h
@@ -168,7 +168,7 @@ int cpu_sh4_handle_mmu_fault(CPUSH4State * env, 
target_ulong address, int rw,
 #define cpu_handle_mmu_fault cpu_sh4_handle_mmu_fault
 void do_interrupt(CPUSH4State * env);
 
-void sh4_cpu_list(FILE *f, int (*cpu_fprintf)(FILE *f, const char *fmt, ...));
+void sh4_cpu_list(FILE *f, fprintf_function cpu_fprintf);
 #if !defined(CONFIG_USER_ONLY)
 void cpu_sh4_invalidate_tlb(CPUSH4State *s);
 void cpu_sh4_write_mmaped_utlb_addr(CPUSH4State *s, target_phys_addr_t addr,
diff --git a/target-sh4/translate.c b/target-sh4/translate.c
index bff3188..bb6a10a 100644
--- a/target-sh4/translate.c
+++ b/target-sh4/translate.c
@@ -255,7 +255,7 @@ static const sh4_def_t *cpu_sh4_find_by_name(const char 
*name)
 return NULL;
 }
 
-void sh4_cpu_list(FILE *f, int (*cpu_fprintf)(FILE *f, const char *fmt, ...))
+void sh4_cpu_list(FILE *f, fprintf_function cpu_fprintf)
 {
 int i;
 
-- 
1.7.0





[Qemu-devel] [PATCH 10/14] target-ppc: Use fprintf_function and fix wrong format specifiers

2010-03-29 Thread Stefan Weil
* The register dump was wrong for XER register.
* cpu_ppc_load_tbl returns uint64_t, so use PRIx64.
* Print space before DECR, but not at end of line.

Signed-off-by: Stefan Weil 
---
 target-ppc/cpu.h|2 +-
 target-ppc/translate.c  |   12 ++--
 target-ppc/translate_init.c |2 +-
 3 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/target-ppc/cpu.h b/target-ppc/cpu.h
index 2ad4486..23e6a3c 100644
--- a/target-ppc/cpu.h
+++ b/target-ppc/cpu.h
@@ -757,7 +757,7 @@ void ppc_store_sr (CPUPPCState *env, int srnum, 
target_ulong value);
 #endif /* !defined(CONFIG_USER_ONLY) */
 void ppc_store_msr (CPUPPCState *env, target_ulong value);
 
-void ppc_cpu_list (FILE *f, int (*cpu_fprintf)(FILE *f, const char *fmt, ...));
+void ppc_cpu_list (FILE *f, fprintf_function cpu_fprintf);
 
 const ppc_def_t *cpu_ppc_find_by_name (const char *name);
 int cpu_ppc_register_internal (CPUPPCState *env, const ppc_def_t *def);
diff --git a/target-ppc/translate.c b/target-ppc/translate.c
index 0af7e4f..63ffd60 100644
--- a/target-ppc/translate.c
+++ b/target-ppc/translate.c
@@ -8831,7 +8831,7 @@ GEN_SPEOP_LDST(evstwwo, 0x1E, 2),
 /*/
 /* Misc PowerPC helpers */
 void cpu_dump_state (CPUState *env, FILE *f,
- int (*cpu_fprintf)(FILE *f, const char *fmt, ...),
+ fprintf_function cpu_fprintf,
  int flags)
 {
 #define RGPL  4
@@ -8840,15 +8840,15 @@ void cpu_dump_state (CPUState *env, FILE *f,
 int i;
 
 cpu_fprintf(f, "NIP " TARGET_FMT_lx "   LR " TARGET_FMT_lx " CTR "
-TARGET_FMT_lx " XER %08x\n", env->nip, env->lr, env->ctr,
-env->xer);
+TARGET_FMT_lx " XER " TARGET_FMT_lx "\n",
+env->nip, env->lr, env->ctr, env->xer);
 cpu_fprintf(f, "MSR " TARGET_FMT_lx " HID0 " TARGET_FMT_lx "  HF "
 TARGET_FMT_lx " idx %d\n", env->msr, env->spr[SPR_HID0],
 env->hflags, env->mmu_idx);
 #if !defined(NO_TIMER_DUMP)
-cpu_fprintf(f, "TB %08x %08x "
+cpu_fprintf(f, "TB %08x %08" PRIx64
 #if !defined(CONFIG_USER_ONLY)
-"DECR %08x"
+" DECR %08x"
 #endif
 "\n",
 cpu_ppc_load_tbu(env), cpu_ppc_load_tbl(env)
@@ -8899,7 +8899,7 @@ void cpu_dump_state (CPUState *env, FILE *f,
 }
 
 void cpu_dump_statistics (CPUState *env, FILE*f,
-  int (*cpu_fprintf)(FILE *f, const char *fmt, ...),
+  fprintf_function cpu_fprintf,
   int flags)
 {
 #if defined(DO_PPC_STATISTICS)
diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
index e8eadf4..413a343 100644
--- a/target-ppc/translate_init.c
+++ b/target-ppc/translate_init.c
@@ -9754,7 +9754,7 @@ const ppc_def_t *cpu_ppc_find_by_name (const char *name)
 return ret;
 }
 
-void ppc_cpu_list (FILE *f, int (*cpu_fprintf)(FILE *f, const char *fmt, ...))
+void ppc_cpu_list (FILE *f, fprintf_function cpu_fprintf)
 {
 int i, max;
 
-- 
1.7.0





[Qemu-devel] [PATCH 09/14] target-mips: Use fprintf_function and fix wrong format specifiers

2010-03-29 Thread Stefan Weil
Removed unused declaration of fpu_dump_state.

These compiler warnings still have to be fixed:

cc1: warnings being treated as errors
/x/target-mips/translate.c: In function 'fpu_dump_state':
/x/target-mips/translate.c:9585: error: format '%08x' expects type 'unsigned 
int', but argument 6 has type 'float_status'
/x/target-mips/translate.c:9588: error: format '%13g' expects type 'double', 
but argument 5 has type 'float64'
/x/target-mips/translate.c:9588: error: format '%13g' expects type 'double', 
but argument 6 has type 'float32'
/x/target-mips/translate.c:9588: error: format '%13g' expects type 'double', 
but argument 7 has type 'float32'
/x/target-mips/translate.c:9588: error: format '%13g' expects type 'double', 
but argument 5 has type 'float64'
/x/target-mips/translate.c:9588: error: format '%13g' expects type 'double', 
but argument 6 has type 'float32'
/x/target-mips/translate.c:9588: error: format '%13g' expects type 'double', 
but argument 7 has type 'float32'

Signed-off-by: Stefan Weil 
---
 target-mips/cpu.h|2 +-
 target-mips/exec.h   |3 ---
 target-mips/translate.c  |6 +++---
 target-mips/translate_init.c |2 +-
 4 files changed, 5 insertions(+), 8 deletions(-)

diff --git a/target-mips/cpu.h b/target-mips/cpu.h
index 7285636..d2d038a 100644
--- a/target-mips/cpu.h
+++ b/target-mips/cpu.h
@@ -495,7 +495,7 @@ void do_unassigned_access(target_phys_addr_t addr, int 
is_write, int is_exec,
   int unused, int size);
 #endif
 
-void mips_cpu_list (FILE *f, int (*cpu_fprintf)(FILE *f, const char *fmt, 
...));
+void mips_cpu_list (FILE *f, fprintf_function cpu_fprintf);
 
 #define cpu_init cpu_mips_init
 #define cpu_exec cpu_mips_exec
diff --git a/target-mips/exec.h b/target-mips/exec.h
index 01e9c4d..abd37fd 100644
--- a/target-mips/exec.h
+++ b/target-mips/exec.h
@@ -18,9 +18,6 @@ register struct CPUMIPSState *env asm(AREG0);
 #endif /* !defined(CONFIG_USER_ONLY) */
 
 void dump_fpu(CPUState *env);
-void fpu_dump_state(CPUState *env, FILE *f,
-int (*fpu_fprintf)(FILE *f, const char *fmt, ...),
-int flags);
 
 void cpu_mips_clock_init (CPUState *env);
 void cpu_mips_tlb_flush (CPUState *env, int flush_global);
diff --git a/target-mips/translate.c b/target-mips/translate.c
index 0ade3bd..c0a4b87 100644
--- a/target-mips/translate.c
+++ b/target-mips/translate.c
@@ -9557,7 +9557,7 @@ void gen_intermediate_code_pc (CPUState *env, struct 
TranslationBlock *tb)
 }
 
 static void fpu_dump_state(CPUState *env, FILE *f,
-   int (*fpu_fprintf)(FILE *f, const char *fmt, ...),
+   fprintf_function fpu_fprintf,
int flags)
 {
 int i;
@@ -9599,7 +9599,7 @@ static void fpu_dump_state(CPUState *env, FILE *f,
 
 static void
 cpu_mips_check_sign_extensions (CPUState *env, FILE *f,
-int (*cpu_fprintf)(FILE *f, const char *fmt, 
...),
+fprintf_function cpu_fprintf,
 int flags)
 {
 int i;
@@ -9626,7 +9626,7 @@ cpu_mips_check_sign_extensions (CPUState *env, FILE *f,
 #endif
 
 void cpu_dump_state (CPUState *env, FILE *f,
- int (*cpu_fprintf)(FILE *f, const char *fmt, ...),
+ fprintf_function cpu_fprintf,
  int flags)
 {
 int i;
diff --git a/target-mips/translate_init.c b/target-mips/translate_init.c
index b79ed56..11385ba 100644
--- a/target-mips/translate_init.c
+++ b/target-mips/translate_init.c
@@ -469,7 +469,7 @@ static const mips_def_t *cpu_mips_find_by_name (const char 
*name)
 return NULL;
 }
 
-void mips_cpu_list (FILE *f, int (*cpu_fprintf)(FILE *f, const char *fmt, ...))
+void mips_cpu_list (FILE *f, fprintf_function cpu_fprintf)
 {
 int i;
 
-- 
1.7.0





[Qemu-devel] [PATCH 08/14] target-microblaze: Use fprintf_function

2010-03-29 Thread Stefan Weil
Signed-off-by: Stefan Weil 
---
 target-microblaze/translate.c |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/target-microblaze/translate.c b/target-microblaze/translate.c
index ca54e2c..8b6a184 100644
--- a/target-microblaze/translate.c
+++ b/target-microblaze/translate.c
@@ -1447,7 +1447,7 @@ void gen_intermediate_code_pc (CPUState *env, struct 
TranslationBlock *tb)
 }
 
 void cpu_dump_state (CPUState *env, FILE *f,
- int (*cpu_fprintf)(FILE *f, const char *fmt, ...),
+ fprintf_function cpu_fprintf,
  int flags)
 {
 int i;
-- 
1.7.0





[Qemu-devel] [PATCH 07/14] target-m68k: Use fprintf_function

2010-03-29 Thread Stefan Weil
Signed-off-by: Stefan Weil 
---
 target-m68k/cpu.h   |2 +-
 target-m68k/helper.c|2 +-
 target-m68k/translate.c |2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/target-m68k/cpu.h b/target-m68k/cpu.h
index b2f37ec..27d4e8f 100644
--- a/target-m68k/cpu.h
+++ b/target-m68k/cpu.h
@@ -198,7 +198,7 @@ static inline int m68k_feature(CPUM68KState *env, int 
feature)
 return (env->features & (1u << feature)) != 0;
 }
 
-void m68k_cpu_list(FILE *f, int (*cpu_fprintf)(FILE *f, const char *fmt, ...));
+void m68k_cpu_list(FILE *f, fprintf_function cpu_fprintf);
 
 void register_m68k_insns (CPUM68KState *env);
 
diff --git a/target-m68k/helper.c b/target-m68k/helper.c
index 2dfd48f..23d7adb 100644
--- a/target-m68k/helper.c
+++ b/target-m68k/helper.c
@@ -53,7 +53,7 @@ static m68k_def_t m68k_cpu_defs[] = {
 {NULL, 0},
 };
 
-void m68k_cpu_list(FILE *f, int (*cpu_fprintf)(FILE *f, const char *fmt, ...))
+void m68k_cpu_list(FILE *f, fprintf_function cpu_fprintf)
 {
 unsigned int i;
 
diff --git a/target-m68k/translate.c b/target-m68k/translate.c
index 99cf6dd..8a92c57 100644
--- a/target-m68k/translate.c
+++ b/target-m68k/translate.c
@@ -3096,7 +3096,7 @@ void gen_intermediate_code_pc(CPUState *env, 
TranslationBlock *tb)
 }
 
 void cpu_dump_state(CPUState *env, FILE *f,
-int (*cpu_fprintf)(FILE *f, const char *fmt, ...),
+fprintf_function cpu_fprintf,
 int flags)
 {
 int i;
-- 
1.7.0





[Qemu-devel] [PATCH 06/14] target-i386: Use fprintf_function and fix dump of DR registers

2010-03-29 Thread Stefan Weil
The dump was wrong for 32 bit hosts with 64 bit target.

Signed-off-by: Stefan Weil 
---
 target-i386/cpu.h|3 +--
 target-i386/cpuid.c  |3 +--
 target-i386/helper.c |   12 +++-
 3 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/target-i386/cpu.h b/target-i386/cpu.h
index 548ab80..dabf084 100644
--- a/target-i386/cpu.h
+++ b/target-i386/cpu.h
@@ -723,8 +723,7 @@ typedef struct CPUX86State {
 CPUX86State *cpu_x86_init(const char *cpu_model);
 int cpu_x86_exec(CPUX86State *s);
 void cpu_x86_close(CPUX86State *s);
-void x86_cpu_list (FILE *f, int (*cpu_fprintf)(FILE *f, const char *fmt, ...),
-   const char *optarg);
+void x86_cpu_list (FILE *f, fprintf_function cpu_fprintf, const char *optarg);
 void x86_cpudef_setup(void);
 
 int cpu_get_pic_interrupt(CPUX86State *s);
diff --git a/target-i386/cpuid.c b/target-i386/cpuid.c
index 56938e2..9229665 100644
--- a/target-i386/cpuid.c
+++ b/target-i386/cpuid.c
@@ -709,8 +709,7 @@ static void listflags(char *buf, int bufsize, uint32_t 
fbits,
  * -?dumpoutput all model (x86_def_t) data
  * -?cpuid   list all recognized cpuid flag names
  */
-void x86_cpu_list (FILE *f, int (*cpu_fprintf)(FILE *f, const char *fmt, ...),
-  const char *optarg)
+void x86_cpu_list(FILE *f, fprintf_function cpu_fprintf, const char *optarg)
 {
 unsigned char model = !strcmp("?model", optarg);
 unsigned char dump = !strcmp("?dump", optarg);
diff --git a/target-i386/helper.c b/target-i386/helper.c
index 35ab720..136ca8d 100644
--- a/target-i386/helper.c
+++ b/target-i386/helper.c
@@ -169,7 +169,7 @@ static const char *cc_op_str[] = {
 
 static void
 cpu_x86_dump_seg_cache(CPUState *env, FILE *f,
-   int (*cpu_fprintf)(FILE *f, const char *fmt, ...),
+   fprintf_function cpu_fprintf,
const char *name, struct SegmentCache *sc)
 {
 #ifdef TARGET_X86_64
@@ -223,7 +223,7 @@ done:
 }
 
 void cpu_dump_state(CPUState *env, FILE *f,
-int (*cpu_fprintf)(FILE *f, const char *fmt, ...),
+fprintf_function cpu_fprintf,
 int flags)
 {
 int eflags, i, nb;
@@ -333,9 +333,11 @@ void cpu_dump_state(CPUState *env, FILE *f,
 (uint32_t)env->cr[2],
 (uint32_t)env->cr[3],
 (uint32_t)env->cr[4]);
-for(i = 0; i < 4; i++)
-cpu_fprintf(f, "DR%d=%08x ", i, env->dr[i]);
-cpu_fprintf(f, "\nDR6=%08x DR7=%08x\n", env->dr[6], env->dr[7]);
+for(i = 0; i < 4; i++) {
+cpu_fprintf(f, "DR%d=%08x ", i, (uint32_t)env->dr[i]);
+}
+cpu_fprintf(f, "\nDR6=%08x DR7=%08x\n",
+(uint32_t)env->dr[6], (uint32_t)env->dr[7]);
 }
 if (flags & X86_DUMP_CCOP) {
 if ((unsigned)env->cc_op < CC_OP_NB)
-- 
1.7.0





[Qemu-devel] [PATCH 05/14] target-cris: Use fprintf_function

2010-03-29 Thread Stefan Weil
Signed-off-by: Stefan Weil 
---
 target-cris/cpu.h   |2 +-
 target-cris/translate.c |4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/target-cris/cpu.h b/target-cris/cpu.h
index 063a240..a45870b 100644
--- a/target-cris/cpu.h
+++ b/target-cris/cpu.h
@@ -266,6 +266,6 @@ static inline void cpu_get_tb_cpu_state(CPUState *env, 
target_ulong *pc,
 }
 
 #define cpu_list cris_cpu_list
-void cris_cpu_list(FILE *f, int (*cpu_fprintf)(FILE *f, const char *fmt, ...));
+void cris_cpu_list(FILE *f, fprintf_function cpu_fprintf);
 
 #endif
diff --git a/target-cris/translate.c b/target-cris/translate.c
index f8baa88..14627d3 100644
--- a/target-cris/translate.c
+++ b/target-cris/translate.c
@@ -3408,7 +3408,7 @@ void gen_intermediate_code_pc (CPUState *env, struct 
TranslationBlock *tb)
 }
 
 void cpu_dump_state (CPUState *env, FILE *f,
- int (*cpu_fprintf)(FILE *f, const char *fmt, ...),
+ fprintf_function cpu_fprintf,
  int flags)
 {
int i;
@@ -3461,7 +3461,7 @@ struct
{32, "crisv32"},
 };
 
-void cris_cpu_list(FILE *f, int (*cpu_fprintf)(FILE *f, const char *fmt, ...))
+void cris_cpu_list(FILE *f, fprintf_function cpu_fprintf)
 {
 unsigned int i;
 
-- 
1.7.0





[Qemu-devel] [PATCH 04/14] target-arm: Use fprintf_function

2010-03-29 Thread Stefan Weil
Signed-off-by: Stefan Weil 
---
 target-arm/cpu.h   |2 +-
 target-arm/helper.c|2 +-
 target-arm/translate.c |2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/target-arm/cpu.h b/target-arm/cpu.h
index 3892db4..7c9d5ca 100644
--- a/target-arm/cpu.h
+++ b/target-arm/cpu.h
@@ -353,7 +353,7 @@ static inline int arm_feature(CPUARMState *env, int feature)
 return (env->features & (1u << feature)) != 0;
 }
 
-void arm_cpu_list(FILE *f, int (*cpu_fprintf)(FILE *f, const char *fmt, ...));
+void arm_cpu_list(FILE *f, fprintf_function cpu_fprintf);
 
 /* Interface between CPU and Interrupt controller.  */
 void armv7m_nvic_set_pending(void *opaque, int irq);
diff --git a/target-arm/helper.c b/target-arm/helper.c
index e092b21..dabea4a 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -332,7 +332,7 @@ static const struct arm_cpu_t arm_cpu_names[] = {
 { 0, NULL}
 };
 
-void arm_cpu_list(FILE *f, int (*cpu_fprintf)(FILE *f, const char *fmt, ...))
+void arm_cpu_list(FILE *f, fprintf_function cpu_fprintf)
 {
 int i;
 
diff --git a/target-arm/translate.c b/target-arm/translate.c
index 3b84c1d..f9898e9 100644
--- a/target-arm/translate.c
+++ b/target-arm/translate.c
@@ -9260,7 +9260,7 @@ static const char *cpu_mode_names[16] = {
 };
 
 void cpu_dump_state(CPUState *env, FILE *f,
-int (*cpu_fprintf)(FILE *f, const char *fmt, ...),
+fprintf_function cpu_fprintf,
 int flags)
 {
 int i;
-- 
1.7.0





[Qemu-devel] [PATCH 03/14] target-alpha: Use fprintf_function

2010-03-29 Thread Stefan Weil
Signed-off-by: Stefan Weil 
---
 target-alpha/helper.c |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/target-alpha/helper.c b/target-alpha/helper.c
index 46335cd..0476066 100644
--- a/target-alpha/helper.c
+++ b/target-alpha/helper.c
@@ -537,7 +537,7 @@ void do_interrupt (CPUState *env)
 #endif
 
 void cpu_dump_state (CPUState *env, FILE *f,
- int (*cpu_fprintf)(FILE *f, const char *fmt, ...),
+ fprintf_function cpu_fprintf,
  int flags)
 {
 static const char *linux_reg_names[] = {
-- 
1.7.0





[Qemu-devel] [PATCH 02/14] Use fprint_function and fix wrong format specifiers

2010-03-29 Thread Stefan Weil
Signed-off-by: Stefan Weil 
---
 exec.c |   10 +-
 1 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/exec.c b/exec.c
index 0916208..6be15c4 100644
--- a/exec.c
+++ b/exec.c
@@ -3964,8 +3964,7 @@ void cpu_io_recompile(CPUState *env, void *retaddr)
 
 #if !defined(CONFIG_USER_ONLY)
 
-void dump_exec_info(FILE *f,
-int (*cpu_fprintf)(FILE *f, const char *fmt, ...))
+void dump_exec_info(FILE *f, fprintf_function cpu_fprintf)
 {
 int i, target_code_size, max_target_code_size;
 int direct_jmp_count, direct_jmp2_count, cross_page;
@@ -3992,15 +3991,16 @@ void dump_exec_info(FILE *f,
 }
 /* XXX: avoid using doubles ? */
 cpu_fprintf(f, "Translation buffer state:\n");
-cpu_fprintf(f, "gen code size   %ld/%ld\n",
-code_gen_ptr - code_gen_buffer, code_gen_buffer_max_size);
+cpu_fprintf(f, "gen code size   %d/%ld\n",
+(int)(code_gen_ptr - code_gen_buffer),
+code_gen_buffer_max_size);
 cpu_fprintf(f, "TB count%d/%d\n", 
 nb_tbs, code_gen_max_blocks);
 cpu_fprintf(f, "TB avg target size  %d max=%d bytes\n",
 nb_tbs ? target_code_size / nb_tbs : 0,
 max_target_code_size);
 cpu_fprintf(f, "TB avg host size%d bytes (expansion ratio: %0.1f)\n",
-nb_tbs ? (code_gen_ptr - code_gen_buffer) / nb_tbs : 0,
+(int)(nb_tbs ? (code_gen_ptr - code_gen_buffer) / nb_tbs : 0),
 target_code_size ? (double) (code_gen_ptr - code_gen_buffer) / 
target_code_size : 0);
 cpu_fprintf(f, "cross page TB count %d (%d%%)\n",
 cross_page,
-- 
1.7.0





[Qemu-devel] [PATCH 01/14] Add new data type for fprintf like function pointers

2010-03-29 Thread Stefan Weil
The compiler should check the arguments for these functions.

gcc can do this, but only if the function pointer's prototype
includes the __attribute__ flag.

As the necessary declaration is a bit lengthy, we use a new
data type 'fprintf_function'.

It is not easy to find a single header file which is included
everywhere, so fprint_function had to be declared in several
header files.

Signed-off-by: Stefan Weil 
---
 cpu-all.h |   13 +
 cpu-defs.h|6 ++
 qemu-common.h |6 ++
 3 files changed, 21 insertions(+), 4 deletions(-)

diff --git a/cpu-all.h b/cpu-all.h
index f281a91..d5c1380 100644
--- a/cpu-all.h
+++ b/cpu-all.h
@@ -760,11 +760,17 @@ void cpu_exec_init_all(unsigned long tb_size);
 CPUState *cpu_copy(CPUState *env);
 CPUState *qemu_get_cpu(int cpu);
 
+#if !defined(FPRINTF_FUNCTION_DEFINED)
+#define FPRINTF_FUNCTION_DEFINED
+typedef int (*fprintf_function)(FILE *f, const char *fmt, ...)
+__attribute__ ((format(printf, 2, 3)));
+#endif
+
 void cpu_dump_state(CPUState *env, FILE *f,
-int (*cpu_fprintf)(FILE *f, const char *fmt, ...),
+fprintf_function cpu_fprintf,
 int flags);
 void cpu_dump_statistics (CPUState *env, FILE *f,
-  int (*cpu_fprintf)(FILE *f, const char *fmt, ...),
+  fprintf_function cpu_fprintf,
   int flags);
 
 void QEMU_NORETURN cpu_abort(CPUState *env, const char *fmt, ...)
@@ -915,8 +921,7 @@ int cpu_physical_memory_get_dirty_tracking(void);
 int cpu_physical_sync_dirty_bitmap(target_phys_addr_t start_addr,
target_phys_addr_t end_addr);
 
-void dump_exec_info(FILE *f,
-int (*cpu_fprintf)(FILE *f, const char *fmt, ...));
+void dump_exec_info(FILE *f, fprintf_function cpu_fprintf);
 #endif /* !CONFIG_USER_ONLY */
 
 int cpu_memory_rw_debug(CPUState *env, target_ulong addr,
diff --git a/cpu-defs.h b/cpu-defs.h
index 2e94585..81edf87 100644
--- a/cpu-defs.h
+++ b/cpu-defs.h
@@ -72,6 +72,12 @@ typedef uint64_t target_ulong;
 #define TB_JMP_ADDR_MASK (TB_JMP_PAGE_SIZE - 1)
 #define TB_JMP_PAGE_MASK (TB_JMP_CACHE_SIZE - TB_JMP_PAGE_SIZE)
 
+#if !defined(FPRINTF_FUNCTION_DEFINED)
+#define FPRINTF_FUNCTION_DEFINED
+typedef int (*fprintf_function)(FILE *f, const char *fmt, ...)
+__attribute__ ((format(printf, 2, 3)));
+#endif
+
 #if !defined(CONFIG_USER_ONLY)
 #define CPU_TLB_BITS 8
 #define CPU_TLB_SIZE (1 << CPU_TLB_BITS)
diff --git a/qemu-common.h b/qemu-common.h
index 087c034..3658bfe 100644
--- a/qemu-common.h
+++ b/qemu-common.h
@@ -91,6 +91,12 @@ static inline char *realpath(const char *path, char 
*resolved_path)
 
 #else
 
+#if !defined(FPRINTF_FUNCTION_DEFINED)
+#define FPRINTF_FUNCTION_DEFINED
+typedef int (*fprintf_function)(FILE *f, const char *fmt, ...)
+__attribute__ ((format(printf, 2, 3)));
+#endif
+
 #include "cpu.h"
 
 #endif /* !defined(NEED_CPU_H) */
-- 
1.7.0





[Qemu-devel] Check format specifiers for fprintf like function pointers

2010-03-29 Thread Stefan Weil
fprintf like function pointers are used for log output, especially
for cpu / fpu register dumps.

When I examined wrong values for an x86_64 target on a i386 host,
I noticed that it was caused by wrong format specifiers and that
there are more errors of this kind.

I could fix some (see list below), but for target-mips some errors
remain to be fixed.

Regards,
Stefan Weil





Re: [Qemu-devel] [PATCH] Elo touchpad 10 bytes emulator

2010-03-29 Thread Ricardo Ribalda Delgado
Hello Paul

Hmmm, you are right, it should be part of the calibration (userland).
As Dmtry says, the output from the chip should be from 96-4000. I have
fixed the max values to that.

The previous code was done to emulate an old privative app that uses
an elo touchscreen. I managed to reverse engineer the code and found
the 10 bytes protocol it was using. It also uses a privative library
to talk with the touchscreen.

Digging in the kernel, I found more of the protocol specification, and
digging a little bit more in the internet I have found even more
information.

Please take a look to the last patch and tell me what do you think



   Best regards and thanks for your comments.

On Mon, Mar 29, 2010 at 17:57, Paul Brook  wrote:
>> New char device emulating an Elo serial touchpad.
>>
>> TODO: The output of the touchpad should be in the range of the
>> resolution. But I don't know a clean way to get the screen resolution.
>> Any help will be very wellcomed
>
> Are you sure? I don't see how real hardware would be able to do that.
>
> Paul
>



-- 
Ricardo Ribalda
http://www.eps.uam.es/~rribalda/




[Qemu-devel] [PATCH] Elo touchpad 10 bytes emulator v2

2010-03-29 Thread Ricardo Ribalda Delgado
New char device emulating an Elo serial touchpad.

-Emulate id and touch packets
-Absolute Output limited to 96-4000
---
 Makefile.objs   |2 +-
 hw/elo.c|  153 +++
 hw/elo.h|2 +
 hw/serial.c |2 +-
 qemu-char.c |3 +
 qemu-options.hx |5 ++-
 6 files changed, 164 insertions(+), 3 deletions(-)
 create mode 100644 hw/elo.c
 create mode 100644 hw/elo.h

diff --git a/Makefile.objs b/Makefile.objs
index b73e2cb..07c2e68 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -75,7 +75,7 @@ common-obj-y += bt.o bt-host.o bt-vhci.o bt-l2cap.o bt-sdp.o 
bt-hci.o bt-hid.o u
 common-obj-y += bt-hci-csr.o
 common-obj-y += buffered_file.o migration.o migration-tcp.o qemu-sockets.o
 common-obj-y += qemu-char.o savevm.o #aio.o
-common-obj-y += msmouse.o ps2.o
+common-obj-y += msmouse.o ps2.o elo.o
 common-obj-y += qdev.o qdev-properties.o
 common-obj-y += qemu-config.o block-migration.o
 
diff --git a/hw/elo.c b/hw/elo.c
new file mode 100644
index 000..359333d
--- /dev/null
+++ b/hw/elo.c
@@ -0,0 +1,153 @@
+/*
+ * QEMU ELO Touchpad via serial port
+ *
+ * Copyright (c) 2010 Ricardo Ribalda: QTechnology http://qtec.com
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to 
deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING 
FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+#include 
+#include "../qemu-common.h"
+#include "../qemu-char.h"
+#include "../console.h"
+#include "elo.h"
+
+static void elo_event(void *opaque,
+  int ax, int ay, int az, int buttons_state)
+{
+CharDriverState *chr = (CharDriverState *)opaque;
+
+unsigned char bytes[10];
+static int is_down=0;
+int old_ax=0,old_ay=0;
+int i;
+
+/*A touchpad cannot capture flight events*/
+if ((!is_down)&&(!buttons_state))
+   return;
+
+ax=(ax*(4000-96+1))/0x7fff;
+ax+=96;
+ay=(ay*(4000-96+1))/0x7fff;
+ay+=96;
+
+/*Move event*/
+if (is_down&&buttons_state){
+   bytes[2]=0x2;
+   is_down++;
+   if ((old_ay==ay)&&(old_ax==ax))
+   return;
+   old_ay=ay;
+   old_ax=ax;
+}
+
+/*Click*/
+if ((!is_down)&&buttons_state){
+   bytes[2]=0x1;
+   is_down=1;
+   old_ay=ay;
+   old_ax=ax;
+}
+/*Release*/
+if (is_down&&(!buttons_state)){
+   bytes[2]=0x4;
+   is_down=0;
+}
+
+bytes[0]='U';
+bytes[1]='T';
+bytes[3]=ax&0xff;
+bytes[4]=(ax>>8)&0xff;
+bytes[5]=ay&0xff;
+bytes[6]=(ay>>8)&0xff;
+bytes[7]=0x0;/*No presure capabilities*/
+bytes[8]=0x0;
+bytes[9]=0xaa;
+for(i=0;i<9;i++)
+   bytes[9]+=bytes[i];
+
+qemu_chr_read(chr, bytes, 10);
+}
+
+static int elo_chr_write (struct CharDriverState *s, const uint8_t *buf, int 
len)
+{
+unsigned char bytes[10];
+static int in_cmd=0,i;
+
+if (buf[0]==0x55)
+   in_cmd=0;
+
+/*Only response to ID*/
+in_cmd+=len;
+if (in_cmd<10)
+   return len;
+
+/* Only respond cmd ID This should be enough at least for linux*/
+/*Full ref at 
http://tge.cegep-baie-comeau.qc.ca/fichestech/References/Elo%20Touch%20Screen/smartset/pages/chapter_6.htm#command_descriptions*/
+/*ID*/
+in_cmd=0;
+bytes[0]='U';
+bytes[1]='I';
+bytes[2]='0';/*Accu*/
+bytes[3]='0';/*Serial*/
+bytes[4]=0;/*No features*/
+bytes[5]=1;
+bytes[6]=2;
+bytes[7]=1;/*1 response*/
+bytes[8]=0xe;
+bytes[9]=0xaa;
+for(i=0;i<9;i++)
+   bytes[9]+=bytes[i];
+qemu_chr_read(s, bytes, 10);
+
+/*ACK no err*/
+in_cmd=0;
+bytes[0]='U';
+bytes[1]='A';
+bytes[2]='0';
+bytes[3]='0';
+bytes[4]='0';
+bytes[5]=0;
+bytes[6]=0;
+bytes[7]=0;
+bytes[8]=0;
+bytes[9]=0xaa;
+for(i=0;i<9;i++)
+   bytes[9]+=bytes[i];
+qemu_chr_read(s, bytes, 10);
+in_cmd=0;
+return len;
+}
+
+static void elo_chr_close (struct CharDriverState *chr)
+{
+qemu_free (chr);
+}
+
+

Re: [Qemu-devel] [PATCH] Fix busted driftfix option

2010-03-29 Thread Blue Swirl
On 3/30/10, Zachary Amsden  wrote:
> On 03/26/10 23:14, Blue Swirl wrote:
>  > On 3/26/10, Zachary Amsden  wrote:
>  >
>  >> For some reason, this uses CONFIG_TARGET_I386 instead of TARGET_I386, so
>  >>  the code is dead.
>  >>
>  > The code is also broken: it references undefined variable 'buf'
>  > instead of 'value'.
>  >
>
>
> Sorry, that wasn't the case on the branch I ported from.  Can you apply
>  a trivial fix or should I send along a patch?

I've already committed the fix.




Re: [Qemu-devel] [PATCH] Fix busted driftfix option

2010-03-29 Thread Zachary Amsden
On 03/26/10 23:14, Blue Swirl wrote:
> On 3/26/10, Zachary Amsden  wrote:
>   
>> For some reason, this uses CONFIG_TARGET_I386 instead of TARGET_I386, so
>>  the code is dead.
>> 
> The code is also broken: it references undefined variable 'buf'
> instead of 'value'.
>   

Sorry, that wasn't the case on the branch I ported from.  Can you apply
a trivial fix or should I send along a patch?

Thanks,

Zach




Re: [Qemu-devel] [PATCH] Elo touchpad 10 bytes emulator

2010-03-29 Thread Dmitry Zhurikhin

Ricardo Ribalda Delgado wrote:

TODO: The output of the touchpad should be in the range of the
resolution. But I don't know a clean way to get the screen resolution.
Any help will be very wellcomed
Hello.  Looking at the Linux kernel driver it seems to consider reported 
coordinates to be in the range of 96-4000.  So you might try to convert 
QEMU absolute coordinates 0-0x7FFF to these ones and check if it works.  
Hope this helps you.



   Regards,
   Dmitry




Re: [Qemu-devel] Re: [PATCH 1/5] linux-user/ia64: workaround ia64 strangenesses

2010-03-29 Thread Aurelien Jarno
On Mon, Mar 29, 2010 at 11:36:50AM +0200, Paolo Bonzini wrote:
>
>> +#ifdef __ia64
>> +sigprocmask(SIG_SETMASK, (sigset_t *)&uc->uc_sigmask, NULL);
>> +#else
>>   sigprocmask(SIG_SETMASK,&uc->uc_sigmask, NULL);
>> +#endif
>
> Any reason for the ifdef?
>

It is not strictly needed, as all architectures can cope with the ia64
version. I have added it to make sure that a new architecture triggers a
warning if uc->uc_sigmask is not of type sigset_t, so that a human can
verify the cast is correct.

That said, I am fine removing the #ifdef if we consider this is unlikely
to happen.

-- 
Aurelien Jarno  GPG: 1024D/F1BCDB73
aurel...@aurel32.net http://www.aurel32.net




[Qemu-devel] [PATCH 01/10] Elo touchpad 10 bytes emulator

2010-03-29 Thread Ricardo Ribalda Delgado
From: Ricardo Ribalda Delgado 

New char device emulating an Elo serial touchpad.

TODO: The output of the touchpad should be in the range of the
resolution. But I don't know a clean way to get the screen resolution.
Any help will be very wellcomed
---


Please, be nice it is my first patch to the list :)

 Makefile.objs   |2 +-
 hw/elo.c|   95 +++
 hw/elo.h|2 +
 qemu-char.c |3 ++
 qemu-options.hx |5 ++-
 5 files changed, 105 insertions(+), 2 deletions(-)
 create mode 100644 hw/elo.c
 create mode 100644 hw/elo.h

diff --git a/Makefile.objs b/Makefile.objs
index b73e2cb..07c2e68 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -75,7 +75,7 @@ common-obj-y += bt.o bt-host.o bt-vhci.o bt-l2cap.o bt-sdp.o 
bt-hci.o bt-hid.o u
 common-obj-y += bt-hci-csr.o
 common-obj-y += buffered_file.o migration.o migration-tcp.o qemu-sockets.o
 common-obj-y += qemu-char.o savevm.o #aio.o
-common-obj-y += msmouse.o ps2.o
+common-obj-y += msmouse.o ps2.o elo.o
 common-obj-y += qdev.o qdev-properties.o
 common-obj-y += qemu-config.o block-migration.o
 
diff --git a/hw/elo.c b/hw/elo.c
new file mode 100644
index 000..c79a8db
--- /dev/null
+++ b/hw/elo.c
@@ -0,0 +1,95 @@
+/*
+ * QEMU ELO Touchpad via serial port
+ *
+ * Copyright (c) 2010 Ricardo Ribalda: QTechnology http://qtec.com
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to 
deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING 
FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+#include 
+#include "../qemu-common.h"
+#include "../qemu-char.h"
+#include "../console.h"
+#include "elo.h"
+
+static void elo_event(void *opaque,
+  int ax, int ay, int az, int buttons_state)
+{
+CharDriverState *chr = (CharDriverState *)opaque;
+
+unsigned char bytes[10];
+static int is_down=0;
+/*Only 1 button is supported*/
+buttons_state&=1;
+
+/*A touchpad cannot capture flight events*/
+if ((!is_down)&&(!buttons_state))
+   return;
+
+/*Move event*/
+if (is_down&&buttons_state){
+   bytes[2]=0x2;
+}
+
+/*Click*/
+if ((!is_down)&&buttons_state){
+   bytes[2]=0x1;
+   is_down=1;
+}
+/*Release*/
+if (is_down&&(!buttons_state)){
+   bytes[2]=0x4;
+   is_down=0;
+}
+
+bytes[0]=0x55;
+bytes[1]=0x54;
+bytes[3]=ax&0xff;
+bytes[4]=(ax>>8)&0xff;
+bytes[5]=ay&0xff;
+bytes[6]=(ay>>8)&0xff;
+bytes[7]=0x0;
+bytes[8]=0x0;
+bytes[9]=0x0;
+
+qemu_chr_read(chr, bytes, 10);
+}
+
+static int elo_chr_write (struct CharDriverState *s, const uint8_t *buf, int 
len)
+{
+/* Ignore writes to mouse port */
+return len;
+}
+
+static void elo_chr_close (struct CharDriverState *chr)
+{
+qemu_free (chr);
+}
+
+CharDriverState *qemu_chr_open_elo(QemuOpts *opts)
+{
+CharDriverState *chr;
+
+chr = qemu_mallocz(sizeof(CharDriverState));
+chr->chr_write = elo_chr_write;
+chr->chr_close = elo_chr_close;
+
+qemu_add_mouse_event_handler(elo_event, chr, 1, "QEMU Elo Touchpad");
+
+return chr;
+}
diff --git a/hw/elo.h b/hw/elo.h
new file mode 100644
index 000..4b4e09a
--- /dev/null
+++ b/hw/elo.h
@@ -0,0 +1,2 @@
+/* elo.c */
+CharDriverState *qemu_chr_open_elo(QemuOpts *opts);
diff --git a/qemu-char.c b/qemu-char.c
index 40cfefa..2f95767 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -32,6 +32,7 @@
 #include "hw/usb.h"
 #include "hw/baum.h"
 #include "hw/msmouse.h"
+#include "hw/elo.h"
 #include "qemu-objects.h"
 
 #include 
@@ -2278,6 +2279,7 @@ QemuOpts *qemu_chr_parse_compat(const char *label, const 
char *filename)
 if (strcmp(filename, "null")== 0 ||
 strcmp(filename, "pty") == 0 ||
 strcmp(filename, "msmouse") == 0 ||
+strcmp(filename, "elo") == 0 ||
 strcmp(filename, "braille") == 0 ||
 strcmp(filename, "stdio")   == 0) {
 qemu_opt_set(opts, "backend", filename);
@@ -2391,6 +2393,7 @@ static const struct {
 { .name = "socket",.o

Re: [Qemu-devel] [PATCH] Elo touchpad 10 bytes emulator

2010-03-29 Thread Paul Brook
> New char device emulating an Elo serial touchpad.
> 
> TODO: The output of the touchpad should be in the range of the
> resolution. But I don't know a clean way to get the screen resolution.
> Any help will be very wellcomed

Are you sure? I don't see how real hardware would be able to do that.

Paul




[Qemu-devel] [PATCH] Elo touchpad 10 bytes emulator

2010-03-29 Thread Ricardo Ribalda Delgado
New char device emulating an Elo serial touchpad.

TODO: The output of the touchpad should be in the range of the
resolution. But I don't know a clean way to get the screen resolution.
Any help will be very wellcomed
---

Please. Ignore the old 01/10


 Makefile.objs   |2 +-
 hw/elo.c|   95 +++
 hw/elo.h|2 +
 qemu-char.c |3 ++
 qemu-options.hx |5 ++-
 5 files changed, 105 insertions(+), 2 deletions(-)
 create mode 100644 hw/elo.c
 create mode 100644 hw/elo.h

diff --git a/Makefile.objs b/Makefile.objs
index b73e2cb..07c2e68 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -75,7 +75,7 @@ common-obj-y += bt.o bt-host.o bt-vhci.o bt-l2cap.o bt-sdp.o 
bt-hci.o bt-hid.o u
 common-obj-y += bt-hci-csr.o
 common-obj-y += buffered_file.o migration.o migration-tcp.o qemu-sockets.o
 common-obj-y += qemu-char.o savevm.o #aio.o
-common-obj-y += msmouse.o ps2.o
+common-obj-y += msmouse.o ps2.o elo.o
 common-obj-y += qdev.o qdev-properties.o
 common-obj-y += qemu-config.o block-migration.o
 
diff --git a/hw/elo.c b/hw/elo.c
new file mode 100644
index 000..c79a8db
--- /dev/null
+++ b/hw/elo.c
@@ -0,0 +1,95 @@
+/*
+ * QEMU ELO Touchpad via serial port
+ *
+ * Copyright (c) 2010 Ricardo Ribalda: QTechnology http://qtec.com
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to 
deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING 
FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+#include 
+#include "../qemu-common.h"
+#include "../qemu-char.h"
+#include "../console.h"
+#include "elo.h"
+
+static void elo_event(void *opaque,
+  int ax, int ay, int az, int buttons_state)
+{
+CharDriverState *chr = (CharDriverState *)opaque;
+
+unsigned char bytes[10];
+static int is_down=0;
+/*Only 1 button is supported*/
+buttons_state&=1;
+
+/*A touchpad cannot capture flight events*/
+if ((!is_down)&&(!buttons_state))
+   return;
+
+/*Move event*/
+if (is_down&&buttons_state){
+   bytes[2]=0x2;
+}
+
+/*Click*/
+if ((!is_down)&&buttons_state){
+   bytes[2]=0x1;
+   is_down=1;
+}
+/*Release*/
+if (is_down&&(!buttons_state)){
+   bytes[2]=0x4;
+   is_down=0;
+}
+
+bytes[0]=0x55;
+bytes[1]=0x54;
+bytes[3]=ax&0xff;
+bytes[4]=(ax>>8)&0xff;
+bytes[5]=ay&0xff;
+bytes[6]=(ay>>8)&0xff;
+bytes[7]=0x0;
+bytes[8]=0x0;
+bytes[9]=0x0;
+
+qemu_chr_read(chr, bytes, 10);
+}
+
+static int elo_chr_write (struct CharDriverState *s, const uint8_t *buf, int 
len)
+{
+/* Ignore writes to mouse port */
+return len;
+}
+
+static void elo_chr_close (struct CharDriverState *chr)
+{
+qemu_free (chr);
+}
+
+CharDriverState *qemu_chr_open_elo(QemuOpts *opts)
+{
+CharDriverState *chr;
+
+chr = qemu_mallocz(sizeof(CharDriverState));
+chr->chr_write = elo_chr_write;
+chr->chr_close = elo_chr_close;
+
+qemu_add_mouse_event_handler(elo_event, chr, 1, "QEMU Elo Touchpad");
+
+return chr;
+}
diff --git a/hw/elo.h b/hw/elo.h
new file mode 100644
index 000..4b4e09a
--- /dev/null
+++ b/hw/elo.h
@@ -0,0 +1,2 @@
+/* elo.c */
+CharDriverState *qemu_chr_open_elo(QemuOpts *opts);
diff --git a/qemu-char.c b/qemu-char.c
index 40cfefa..2f95767 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -32,6 +32,7 @@
 #include "hw/usb.h"
 #include "hw/baum.h"
 #include "hw/msmouse.h"
+#include "hw/elo.h"
 #include "qemu-objects.h"
 
 #include 
@@ -2278,6 +2279,7 @@ QemuOpts *qemu_chr_parse_compat(const char *label, const 
char *filename)
 if (strcmp(filename, "null")== 0 ||
 strcmp(filename, "pty") == 0 ||
 strcmp(filename, "msmouse") == 0 ||
+strcmp(filename, "elo") == 0 ||
 strcmp(filename, "braille") == 0 ||
 strcmp(filename, "stdio")   == 0) {
 qemu_opt_set(opts, "backend", filename);
@@ -2391,6 +2393,7 @@ static const struct {
 { .name = "socket",.open = qemu_chr_open_socket },
 { .name = "udp",

Re: [Qemu-devel] [PATCH -V3 09/32] virtio-9p: Implement P9_TWRITE/ Thread model in QEMU

2010-03-29 Thread Anthony Liguori

On 03/29/2010 01:36 AM, jvrao wrote:

Aneesh Kumar K.V wrote:
   

From: Anthony Liguori
 

We have implemented all the vfs calls in state machine model so that we are 
prepared
for the model where the VCPU thread(s) does the initial work until it needs to 
block then it
submits that work (via a function pointer) to a thread pool.  A thread in that 
thread pool
picks up the work, and completes the blocking call, when blocking call returns 
a callback is
invoked in the IO thread.  Then the IO thread runs until the next blocking 
function, and goto start.

Basically the VCPU/IO threads does all the non-blocking work, and let the 
threads in the
thread pool work on the blocking calls like mkdir() stat() etc.

My question is, why not let the whole work done by the thread in the thread 
pool?
VCPU thread receives the PDU and hands over the entire job to worker thread.
When all work is completed, either the worker thread or the IO thread(we can 
switch back at this point if needed) marks the request as completed in the 
virtqueue and injects an
interrupt to notify the guest.

We can still keep the same number of threads in the thread pool.
This way, we are not increasing #of threads employed by QEMU...also it makes 
code lot
more easy to read/maintain.

I may be missing something..but would like to know more on the advantages of 
this model.
   


In qemu, we tend to prefer state-machine based code.

There are a few different models that we have discussed at different points:

1) state machines with a thread pool to make blocking functions 
asynchronous (what we have today)


2) co-operative threading

3) pre-emptive threading

All three models are independent of making device models re-entrant.  In 
order to allow VCPU threads to run in qemu simultaneously, we need each 
device to carry a lock and for that lock to be acquired upon any of the 
public functions for the device model.


For individual device models or host services, I think (3) is probably 
the worst model overall.  I personally think that (1) is better in the 
long run but ultimately would need an existence proof to compare against 
(2).  (2) looks appealing until you actually try to have the device 
handle multiple requests at a time.


For virtio-9p, I don't think (1) is much of a burden to be honest.  In 
particular, when the 9p2000.L dialect is used, there should be a 1-1 
correlation between protocol operations and system calls which means 
that for the most part, there's really no advantage to threading from a 
complexity point of view.


Regards,

Anthony Liguori


Thanks,
JV
   






[Qemu-devel] [PATCH v2 1/7] qemu-config: qemu_read_config_file() reads the normal config file

2010-03-29 Thread Kevin Wolf
Introduce a new function qemu_read_config_file which reads the VM configuration
from a config file. Unlike qemu_config_parse it doesn't take a open file but a
filename and reduces code duplication as a side effect.

Signed-off-by: Kevin Wolf 
---
 qemu-config.c |   15 +++
 qemu-config.h |2 ++
 vl.c  |   38 +-
 3 files changed, 30 insertions(+), 25 deletions(-)

diff --git a/qemu-config.c b/qemu-config.c
index 150157c..57dc9a5 100644
--- a/qemu-config.c
+++ b/qemu-config.c
@@ -489,3 +489,18 @@ out:
 loc_pop(&loc);
 return res;
 }
+
+int qemu_read_config_file(const char *filename)
+{
+FILE *f = fopen(filename, "r");
+if (f == NULL) {
+return -errno;
+}
+
+if (qemu_config_parse(f, filename) != 0) {
+return -EINVAL;
+}
+fclose(f);
+
+return 0;
+}
diff --git a/qemu-config.h b/qemu-config.h
index f217c58..07a7a27 100644
--- a/qemu-config.h
+++ b/qemu-config.h
@@ -19,4 +19,6 @@ void qemu_add_globals(void);
 void qemu_config_write(FILE *fp);
 int qemu_config_parse(FILE *fp, const char *fname);
 
+int qemu_read_config_file(const char *filename);
+
 #endif /* QEMU_CONFIG_H */
diff --git a/vl.c b/vl.c
index 8a73235..6c022bf 100644
--- a/vl.c
+++ b/vl.c
@@ -3831,25 +3831,17 @@ int main(int argc, char **argv, char **envp)
 }
 
 if (defconfig) {
-const char *fname;
-FILE *fp;
+int ret;
 
-fname = CONFIG_QEMU_CONFDIR "/qemu.conf";
-fp = fopen(fname, "r");
-if (fp) {
-if (qemu_config_parse(fp, fname) != 0) {
-exit(1);
-}
-fclose(fp);
+ret = qemu_read_config_file(CONFIG_QEMU_CONFDIR "/qemu.conf");
+if (ret == -EINVAL) {
+exit(1);
 }
 
-fname = CONFIG_QEMU_CONFDIR "/target-" TARGET_ARCH ".conf";
-fp = fopen(fname, "r");
-if (fp) {
-if (qemu_config_parse(fp, fname) != 0) {
-exit(1);
-}
-fclose(fp);
+ret = qemu_read_config_file(CONFIG_QEMU_CONFDIR
+"/target-" TARGET_ARCH ".conf");
+if (ret == -EINVAL) {
+exit(1);
 }
 }
 #if defined(cpudef_setup)
@@ -4525,16 +4517,12 @@ int main(int argc, char **argv, char **envp)
 #endif
 case QEMU_OPTION_readconfig:
 {
-FILE *fp;
-fp = fopen(optarg, "r");
-if (fp == NULL) {
-fprintf(stderr, "open %s: %s\n", optarg, 
strerror(errno));
+int ret = qemu_read_config_file(optarg);
+if (ret < 0) {
+fprintf(stderr, "read config %s: %s\n", optarg,
+strerror(-ret));
 exit(1);
 }
-if (qemu_config_parse(fp, optarg) != 0) {
-exit(1);
-}
-fclose(fp);
 break;
 }
 case QEMU_OPTION_writeconfig:
-- 
1.6.6.1





[Qemu-devel] [PATCH v2 4/7] blkdebug: Inject errors

2010-03-29 Thread Kevin Wolf
Add a mechanism to inject errors instead of passing requests on. With no
further patches applied, you can use it by setting inject_errno in gdb.

Signed-off-by: Kevin Wolf 
---
 block/blkdebug.c |   81 ++
 1 files changed, 81 insertions(+), 0 deletions(-)

diff --git a/block/blkdebug.c b/block/blkdebug.c
index 2c7e0dd..dad3ac6 100644
--- a/block/blkdebug.c
+++ b/block/blkdebug.c
@@ -26,10 +26,41 @@
 #include "block_int.h"
 #include "module.h"
 
+#include 
+
+typedef struct BlkdebugVars {
+int state;
+
+/* If inject_errno != 0, an error is injected for requests */
+int inject_errno;
+
+/* Decides if all future requests fail (false) or only the next one and
+ * after the next request inject_errno is reset to 0 (true) */
+bool inject_once;
+
+/* Decides if aio_readv/writev fails right away (true) or returns an error
+ * return value only in the callback (false) */
+bool inject_immediately;
+} BlkdebugVars;
+
 typedef struct BDRVBlkdebugState {
 BlockDriverState *hd;
+BlkdebugVars vars;
 } BDRVBlkdebugState;
 
+typedef struct BlkdebugAIOCB {
+BlockDriverAIOCB common;
+QEMUBH *bh;
+int ret;
+} BlkdebugAIOCB;
+
+static void blkdebug_aio_cancel(BlockDriverAIOCB *blockacb);
+
+static AIOPool blkdebug_aio_pool = {
+.aiocb_size = sizeof(BlkdebugAIOCB),
+.cancel = blkdebug_aio_cancel,
+};
+
 static int blkdebug_open(BlockDriverState *bs, const char *filename, int flags)
 {
 BDRVBlkdebugState *s = bs->opaque;
@@ -42,11 +73,56 @@ static int blkdebug_open(BlockDriverState *bs, const char 
*filename, int flags)
 return bdrv_file_open(&s->hd, filename, flags);
 }
 
+static void error_callback_bh(void *opaque)
+{
+struct BlkdebugAIOCB *acb = opaque;
+qemu_bh_delete(acb->bh);
+acb->common.cb(acb->common.opaque, acb->ret);
+qemu_aio_release(acb);
+}
+
+static void blkdebug_aio_cancel(BlockDriverAIOCB *blockacb)
+{
+BlkdebugAIOCB *acb = (BlkdebugAIOCB*) blockacb;
+qemu_aio_release(acb);
+}
+
+static BlockDriverAIOCB *inject_error(BlockDriverState *bs,
+BlockDriverCompletionFunc *cb, void *opaque)
+{
+BDRVBlkdebugState *s = bs->opaque;
+int error = s->vars.inject_errno;
+struct BlkdebugAIOCB *acb;
+QEMUBH *bh;
+
+if (s->vars.inject_once) {
+s->vars.inject_errno = 0;
+}
+
+if (s->vars.inject_immediately) {
+return NULL;
+}
+
+acb = qemu_aio_get(&blkdebug_aio_pool, bs, cb, opaque);
+acb->ret = -error;
+
+bh = qemu_bh_new(error_callback_bh, acb);
+acb->bh = bh;
+qemu_bh_schedule(bh);
+
+return (BlockDriverAIOCB*) acb;
+}
+
 static BlockDriverAIOCB *blkdebug_aio_readv(BlockDriverState *bs,
 int64_t sector_num, QEMUIOVector *qiov, int nb_sectors,
 BlockDriverCompletionFunc *cb, void *opaque)
 {
 BDRVBlkdebugState *s = bs->opaque;
+
+if (s->vars.inject_errno) {
+return inject_error(bs, cb, opaque);
+}
+
 BlockDriverAIOCB *acb =
 bdrv_aio_readv(s->hd, sector_num, qiov, nb_sectors, cb, opaque);
 return acb;
@@ -57,6 +133,11 @@ static BlockDriverAIOCB 
*blkdebug_aio_writev(BlockDriverState *bs,
 BlockDriverCompletionFunc *cb, void *opaque)
 {
 BDRVBlkdebugState *s = bs->opaque;
+
+if (s->vars.inject_errno) {
+return inject_error(bs, cb, opaque);
+}
+
 BlockDriverAIOCB *acb =
 bdrv_aio_writev(s->hd, sector_num, qiov, nb_sectors, cb, opaque);
 return acb;
-- 
1.6.6.1





[Qemu-devel] [PATCH v2 2/7] qemu-config: Make qemu_config_parse more generic

2010-03-29 Thread Kevin Wolf
qemu_config_parse gets the option groups as a parameter now instead of
hardcoding the VM configuration groups. This way it can be used for other
configurations, too.

Signed-off-by: Kevin Wolf 
---
 qemu-config.c |   18 --
 qemu-config.h |2 +-
 2 files changed, 13 insertions(+), 7 deletions(-)

diff --git a/qemu-config.c b/qemu-config.c
index 57dc9a5..dedf177 100644
--- a/qemu-config.c
+++ b/qemu-config.c
@@ -296,7 +296,7 @@ QemuOptsList qemu_cpudef_opts = {
 },
 };
 
-static QemuOptsList *lists[] = {
+static QemuOptsList *vm_config_groups[] = {
 &qemu_drive_opts,
 &qemu_chardev_opts,
 &qemu_device_opts,
@@ -309,7 +309,7 @@ static QemuOptsList *lists[] = {
 NULL,
 };
 
-QemuOptsList *qemu_find_opts(const char *group)
+static QemuOptsList *find_list(QemuOptsList **lists, const char *group)
 {
 int i;
 
@@ -323,6 +323,11 @@ QemuOptsList *qemu_find_opts(const char *group)
 return lists[i];
 }
 
+QemuOptsList *qemu_find_opts(const char *group)
+{
+return find_list(vm_config_groups, group);
+}
+
 int qemu_set_option(const char *str)
 {
 char group[64], id[64], arg[64];
@@ -421,6 +426,7 @@ static int config_write_opts(QemuOpts *opts, void *opaque)
 void qemu_config_write(FILE *fp)
 {
 struct ConfigWriteData data = { .fp = fp };
+QemuOptsList **lists = vm_config_groups;
 int i;
 
 fprintf(fp, "# qemu config file\n\n");
@@ -430,7 +436,7 @@ void qemu_config_write(FILE *fp)
 }
 }
 
-int qemu_config_parse(FILE *fp, const char *fname)
+int qemu_config_parse(FILE *fp, QemuOptsList **lists, const char *fname)
 {
 char line[1024], group[64], id[64], arg[64], value[1024];
 Location loc;
@@ -451,7 +457,7 @@ int qemu_config_parse(FILE *fp, const char *fname)
 }
 if (sscanf(line, "[%63s \"%63[^\"]\"]", group, id) == 2) {
 /* group with id */
-list = qemu_find_opts(group);
+list = find_list(lists, group);
 if (list == NULL)
 goto out;
 opts = qemu_opts_create(list, id, 1);
@@ -459,7 +465,7 @@ int qemu_config_parse(FILE *fp, const char *fname)
 }
 if (sscanf(line, "[%63[^]]]", group) == 1) {
 /* group without id */
-list = qemu_find_opts(group);
+list = find_list(lists, group);
 if (list == NULL)
 goto out;
 opts = qemu_opts_create(list, NULL, 0);
@@ -497,7 +503,7 @@ int qemu_read_config_file(const char *filename)
 return -errno;
 }
 
-if (qemu_config_parse(f, filename) != 0) {
+if (qemu_config_parse(f, vm_config_groups, filename) != 0) {
 return -EINVAL;
 }
 fclose(f);
diff --git a/qemu-config.h b/qemu-config.h
index 07a7a27..dd299c2 100644
--- a/qemu-config.h
+++ b/qemu-config.h
@@ -17,7 +17,7 @@ int qemu_global_option(const char *str);
 void qemu_add_globals(void);
 
 void qemu_config_write(FILE *fp);
-int qemu_config_parse(FILE *fp, const char *fname);
+int qemu_config_parse(FILE *fp, QemuOptsList **lists, const char *fname);
 
 int qemu_read_config_file(const char *filename);
 
-- 
1.6.6.1





[Qemu-devel] [PATCH v2 3/7] blkdebug: Basic request passthrough

2010-03-29 Thread Kevin Wolf
This isn't doing anything interesting. It creates the blkdebug block driver as
a protocol which just passes everything through to raw.

Signed-off-by: Kevin Wolf 
---
 Makefile.objs|2 +-
 block/blkdebug.c |  104 ++
 2 files changed, 105 insertions(+), 1 deletions(-)
 create mode 100644 block/blkdebug.c

diff --git a/Makefile.objs b/Makefile.objs
index 50ae890..8c8f79b 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -14,7 +14,7 @@ block-obj-$(CONFIG_LINUX_AIO) += linux-aio.o
 
 block-nested-y += cow.o qcow.o vdi.o vmdk.o cloop.o dmg.o bochs.o vpc.o vvfat.o
 block-nested-y += qcow2.o qcow2-refcount.o qcow2-cluster.o qcow2-snapshot.o
-block-nested-y += parallels.o nbd.o
+block-nested-y += parallels.o nbd.o blkdebug.o
 block-nested-$(CONFIG_WIN32) += raw-win32.o
 block-nested-$(CONFIG_POSIX) += raw-posix.o
 block-nested-$(CONFIG_CURL) += curl.o
diff --git a/block/blkdebug.c b/block/blkdebug.c
new file mode 100644
index 000..2c7e0dd
--- /dev/null
+++ b/block/blkdebug.c
@@ -0,0 +1,104 @@
+/*
+ * Block protocol for I/O error injection
+ *
+ * Copyright (c) 2010 Kevin Wolf 
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to 
deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING 
FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#include "qemu-common.h"
+#include "block_int.h"
+#include "module.h"
+
+typedef struct BDRVBlkdebugState {
+BlockDriverState *hd;
+} BDRVBlkdebugState;
+
+static int blkdebug_open(BlockDriverState *bs, const char *filename, int flags)
+{
+BDRVBlkdebugState *s = bs->opaque;
+
+if (strncmp(filename, "blkdebug:", strlen("blkdebug:"))) {
+return -EINVAL;
+}
+filename += strlen("blkdebug:");
+
+return bdrv_file_open(&s->hd, filename, flags);
+}
+
+static BlockDriverAIOCB *blkdebug_aio_readv(BlockDriverState *bs,
+int64_t sector_num, QEMUIOVector *qiov, int nb_sectors,
+BlockDriverCompletionFunc *cb, void *opaque)
+{
+BDRVBlkdebugState *s = bs->opaque;
+BlockDriverAIOCB *acb =
+bdrv_aio_readv(s->hd, sector_num, qiov, nb_sectors, cb, opaque);
+return acb;
+}
+
+static BlockDriverAIOCB *blkdebug_aio_writev(BlockDriverState *bs,
+int64_t sector_num, QEMUIOVector *qiov, int nb_sectors,
+BlockDriverCompletionFunc *cb, void *opaque)
+{
+BDRVBlkdebugState *s = bs->opaque;
+BlockDriverAIOCB *acb =
+bdrv_aio_writev(s->hd, sector_num, qiov, nb_sectors, cb, opaque);
+return acb;
+}
+
+static void blkdebug_close(BlockDriverState *bs)
+{
+BDRVBlkdebugState *s = bs->opaque;
+bdrv_delete(s->hd);
+}
+
+static void blkdebug_flush(BlockDriverState *bs)
+{
+BDRVBlkdebugState *s = bs->opaque;
+bdrv_flush(s->hd);
+}
+
+static BlockDriverAIOCB *blkdebug_aio_flush(BlockDriverState *bs,
+BlockDriverCompletionFunc *cb, void *opaque)
+{
+BDRVBlkdebugState *s = bs->opaque;
+return bdrv_aio_flush(s->hd, cb, opaque);
+}
+
+static BlockDriver bdrv_blkdebug = {
+.format_name= "blkdebug",
+.protocol_name  = "blkdebug",
+
+.instance_size  = sizeof(BDRVBlkdebugState),
+
+.bdrv_open  = blkdebug_open,
+.bdrv_close = blkdebug_close,
+.bdrv_flush = blkdebug_flush,
+
+.bdrv_aio_readv = blkdebug_aio_readv,
+.bdrv_aio_writev= blkdebug_aio_writev,
+.bdrv_aio_flush = blkdebug_aio_flush,
+};
+
+static void bdrv_blkdebug_init(void)
+{
+bdrv_register(&bdrv_blkdebug);
+}
+
+block_init(bdrv_blkdebug_init);
-- 
1.6.6.1





[Qemu-devel] [PATCH v2 0/7] blkdebug

2010-03-29 Thread Kevin Wolf
This patch series introduces a new block driver which acts as a protocol and
whose purpose it is to fail requests. To be more precise, I want it to fail in
configurable places, so that qemu-iotests can be extended with tests for the
error paths (for example for the case when something with metadata writes goes
wrong deep in qcow2).

It works like this (I think this is self-explanatory):

$ cat /tmp/blkdebug.cfg
[inject-error]
event = "l1_update"
errno = "5"
immediately = "on"
$ qemu-io blkdebug:/tmp/blkdebug.cfg:/tmp/empty.qcow2
qemu-io> read 0 4k
read 4096/4096 bytes at offset 0
4 KiB, 1 ops; 0. sec (195.312 MiB/sec and 5. ops/sec)
qemu-io> write 0 4k
write failed: Input/output error

Changes in comparison to the RFC:
- Had to rebase qemu-config changes
- Code cleanup (including resolving TODOs and FIXMEs)
- More events all over qcow2
- No debug messages on stderr any more

v2:
- Another rebase
- Fixed memleak in blkdebug_open

Kevin Wolf (7):
  qemu-config: qemu_read_config_file() reads the normal config file
  qemu-config: Make qemu_config_parse more generic
  blkdebug: Basic request passthrough
  blkdebug: Inject errors
  Make qemu-config available for tools
  blkdebug: Add events and rules
  qcow2: Trigger blkdebug events

 Makefile.objs  |6 +-
 block.c|   12 ++
 block.h|   53 ++
 block/blkdebug.c   |  475 
 block/qcow2-cluster.c  |   15 ++
 block/qcow2-refcount.c |   18 ++
 block/qcow2.c  |6 +
 block_int.h|2 +
 hw/qdev-properties.c   |   19 ++-
 hw/qdev.h  |1 -
 qemu-config.c  |   48 +++---
 qemu-config.h  |4 +-
 vl.c   |   38 ++---
 13 files changed, 644 insertions(+), 53 deletions(-)
 create mode 100644 block/blkdebug.c





[Qemu-devel] [PATCH v2 5/7] Make qemu-config available for tools

2010-03-29 Thread Kevin Wolf
To be able to use config files for blkdebug, we need to make these functions
available in the tools. This involves moving two functions that can only be
built in the context of the emulator.

Signed-off-by: Kevin Wolf 
---
 Makefile.objs|4 ++--
 hw/qdev-properties.c |   19 ++-
 hw/qdev.h|1 -
 qemu-config.c|   17 -
 4 files changed, 20 insertions(+), 21 deletions(-)

diff --git a/Makefile.objs b/Makefile.objs
index 8c8f79b..03e49ec 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -8,7 +8,7 @@ qobject-obj-y += qerror.o
 # block-obj-y is code used by both qemu system emulation and qemu-img
 
 block-obj-y = cutils.o cache-utils.o qemu-malloc.o qemu-option.o module.o
-block-obj-y += nbd.o block.o aio.o aes.o osdep.o
+block-obj-y += nbd.o block.o aio.o aes.o osdep.o qemu-config.o
 block-obj-$(CONFIG_POSIX) += posix-aio-compat.o
 block-obj-$(CONFIG_LINUX_AIO) += linux-aio.o
 
@@ -76,7 +76,7 @@ common-obj-y += buffered_file.o migration.o migration-tcp.o 
qemu-sockets.o
 common-obj-y += qemu-char.o savevm.o #aio.o
 common-obj-y += msmouse.o ps2.o
 common-obj-y += qdev.o qdev-properties.o
-common-obj-y += qemu-config.o block-migration.o
+common-obj-y += block-migration.o
 
 common-obj-$(CONFIG_BRLAPI) += baum.o
 common-obj-$(CONFIG_POSIX) += migration-exec.o migration-unix.o migration-fd.o
diff --git a/hw/qdev-properties.c b/hw/qdev-properties.c
index 157a111..9ffdba7 100644
--- a/hw/qdev-properties.c
+++ b/hw/qdev-properties.c
@@ -661,7 +661,7 @@ void qdev_prop_set_defaults(DeviceState *dev, Property 
*props)
 
 static QTAILQ_HEAD(, GlobalProperty) global_props = 
QTAILQ_HEAD_INITIALIZER(global_props);
 
-void qdev_prop_register_global(GlobalProperty *prop)
+static void qdev_prop_register_global(GlobalProperty *prop)
 {
 QTAILQ_INSERT_TAIL(&global_props, prop, next);
 }
@@ -689,3 +689,20 @@ void qdev_prop_set_globals(DeviceState *dev)
 }
 }
 }
+
+static int qdev_add_one_global(QemuOpts *opts, void *opaque)
+{
+GlobalProperty *g;
+
+g = qemu_mallocz(sizeof(*g));
+g->driver   = qemu_opt_get(opts, "driver");
+g->property = qemu_opt_get(opts, "property");
+g->value= qemu_opt_get(opts, "value");
+qdev_prop_register_global(g);
+return 0;
+}
+
+void qemu_add_globals(void)
+{
+qemu_opts_foreach(&qemu_global_opts, qdev_add_one_global, NULL, 0);
+}
diff --git a/hw/qdev.h b/hw/qdev.h
index 9475705..3a8e801 100644
--- a/hw/qdev.h
+++ b/hw/qdev.h
@@ -273,7 +273,6 @@ void qdev_prop_set_macaddr(DeviceState *dev, const char 
*name, uint8_t *value);
 void qdev_prop_set_ptr(DeviceState *dev, const char *name, void *value);
 void qdev_prop_set_defaults(DeviceState *dev, Property *props);
 
-void qdev_prop_register_global(GlobalProperty *prop);
 void qdev_prop_register_global_list(GlobalProperty *props);
 void qdev_prop_set_globals(DeviceState *dev);
 
diff --git a/qemu-config.c b/qemu-config.c
index dedf177..ad91133 100644
--- a/qemu-config.c
+++ b/qemu-config.c
@@ -378,23 +378,6 @@ int qemu_global_option(const char *str)
 return 0;
 }
 
-static int qemu_add_one_global(QemuOpts *opts, void *opaque)
-{
-GlobalProperty *g;
-
-g = qemu_mallocz(sizeof(*g));
-g->driver   = qemu_opt_get(opts, "driver");
-g->property = qemu_opt_get(opts, "property");
-g->value= qemu_opt_get(opts, "value");
-qdev_prop_register_global(g);
-return 0;
-}
-
-void qemu_add_globals(void)
-{
-qemu_opts_foreach(&qemu_global_opts, qemu_add_one_global, NULL, 0);
-}
-
 struct ConfigWriteData {
 QemuOptsList *list;
 FILE *fp;
-- 
1.6.6.1





[Qemu-devel] [PATCH v2 6/7] blkdebug: Add events and rules

2010-03-29 Thread Kevin Wolf
Block drivers can trigger a blkdebug event whenever they reach a place where it
could be useful to inject an error for testing/debugging purposes.

Rules are read from a blkdebug config file and describe which action is taken
when an event is triggered. For now this is only injecting an error (with a few
options) or changing the state (which is an integer). Rules can be declared to
be active only in a specific state; this way later rules can distiguish on
which path we came to trigger their event.

Signed-off-by: Kevin Wolf 
---
 block.c  |   12 +++
 block.h  |9 ++
 block/blkdebug.c |  250 +-
 block_int.h  |2 +
 4 files changed, 272 insertions(+), 1 deletions(-)

diff --git a/block.c b/block.c
index e891544..122a620 100644
--- a/block.c
+++ b/block.c
@@ -1535,6 +1535,18 @@ int bdrv_load_vmstate(BlockDriverState *bs, uint8_t *buf,
 return drv->bdrv_load_vmstate(bs, buf, pos, size);
 }
 
+void bdrv_debug_event(BlockDriverState *bs, BlkDebugEvent event)
+{
+BlockDriver *drv = bs->drv;
+
+if (!drv || !drv->bdrv_debug_event) {
+return;
+}
+
+return drv->bdrv_debug_event(bs, event);
+
+}
+
 /**/
 /* handling of snapshots */
 
diff --git a/block.h b/block.h
index edf5704..2fd8361 100644
--- a/block.h
+++ b/block.h
@@ -207,4 +207,13 @@ int bdrv_get_dirty(BlockDriverState *bs, int64_t sector);
 void bdrv_reset_dirty(BlockDriverState *bs, int64_t cur_sector,
   int nr_sectors);
 int64_t bdrv_get_dirty_count(BlockDriverState *bs);
+
+
+typedef enum {
+BLKDBG_EVENT_MAX,
+} BlkDebugEvent;
+
+#define BLKDBG_EVENT(bs, evt) bdrv_debug_event(bs, evt)
+void bdrv_debug_event(BlockDriverState *bs, BlkDebugEvent event);
+
 #endif
diff --git a/block/blkdebug.c b/block/blkdebug.c
index dad3ac6..b813bfa 100644
--- a/block/blkdebug.c
+++ b/block/blkdebug.c
@@ -46,6 +46,7 @@ typedef struct BlkdebugVars {
 typedef struct BDRVBlkdebugState {
 BlockDriverState *hd;
 BlkdebugVars vars;
+QLIST_HEAD(list, BlkdebugRule) rules[BLKDBG_EVENT_MAX];
 } BDRVBlkdebugState;
 
 typedef struct BlkdebugAIOCB {
@@ -61,16 +62,211 @@ static AIOPool blkdebug_aio_pool = {
 .cancel = blkdebug_aio_cancel,
 };
 
+enum {
+ACTION_INJECT_ERROR,
+ACTION_SET_STATE,
+};
+
+typedef struct BlkdebugRule {
+BlkDebugEvent event;
+int action;
+int state;
+union {
+struct {
+int error;
+int immediately;
+int once;
+} inject;
+struct {
+int new_state;
+} set_state;
+} options;
+QLIST_ENTRY(BlkdebugRule) next;
+} BlkdebugRule;
+
+static QemuOptsList inject_error_opts = {
+.name = "inject-error",
+.head = QTAILQ_HEAD_INITIALIZER(inject_error_opts.head),
+.desc = {
+{
+.name = "event",
+.type = QEMU_OPT_STRING,
+},
+{
+.name = "state",
+.type = QEMU_OPT_NUMBER,
+},
+{
+.name = "errno",
+.type = QEMU_OPT_NUMBER,
+},
+{
+.name = "once",
+.type = QEMU_OPT_BOOL,
+},
+{
+.name = "immediately",
+.type = QEMU_OPT_BOOL,
+},
+{ /* end of list */ }
+},
+};
+
+static QemuOptsList set_state_opts = {
+.name = "set-state",
+.head = QTAILQ_HEAD_INITIALIZER(inject_error_opts.head),
+.desc = {
+{
+.name = "event",
+.type = QEMU_OPT_STRING,
+},
+{
+.name = "state",
+.type = QEMU_OPT_NUMBER,
+},
+{
+.name = "new_state",
+.type = QEMU_OPT_NUMBER,
+},
+{ /* end of list */ }
+},
+};
+
+static QemuOptsList *config_groups[] = {
+&inject_error_opts,
+&set_state_opts,
+NULL
+};
+
+static const char *event_names[BLKDBG_EVENT_MAX] = {
+};
+
+static int get_event_by_name(const char *name, BlkDebugEvent *event)
+{
+int i;
+
+for (i = 0; i < BLKDBG_EVENT_MAX; i++) {
+if (!strcmp(event_names[i], name)) {
+*event = i;
+return 0;
+}
+}
+
+return -1;
+}
+
+struct add_rule_data {
+BDRVBlkdebugState *s;
+int action;
+};
+
+static int add_rule(QemuOpts *opts, void *opaque)
+{
+struct add_rule_data *d = opaque;
+BDRVBlkdebugState *s = d->s;
+const char* event_name;
+BlkDebugEvent event;
+struct BlkdebugRule *rule;
+
+/* Find the right event for the rule */
+event_name = qemu_opt_get(opts, "event");
+if (!event_name || get_event_by_name(event_name, &event) < 0) {
+return -1;
+}
+
+/* Set attributes common for all actions */
+rule = qemu_mallocz(sizeof(*rule));
+*rule = (struct BlkdebugRule) {
+.event  = event,
+.action = d->action,
+.state  = qemu_opt_get_number(opts, "state", 0),

[Qemu-devel] [PATCH v2 7/7] qcow2: Trigger blkdebug events

2010-03-29 Thread Kevin Wolf
This adds blkdebug events to qcow2 to allow injecting I/O errors in specific
places.

Signed-off-by: Kevin Wolf 
---
 block.h|   44 
 block/blkdebug.c   |   42 ++
 block/qcow2-cluster.c  |   15 +++
 block/qcow2-refcount.c |   18 ++
 block/qcow2.c  |6 ++
 5 files changed, 125 insertions(+), 0 deletions(-)

diff --git a/block.h b/block.h
index 2fd8361..14443f1 100644
--- a/block.h
+++ b/block.h
@@ -210,6 +210,50 @@ int64_t bdrv_get_dirty_count(BlockDriverState *bs);
 
 
 typedef enum {
+BLKDBG_L1_UPDATE,
+
+BLKDBG_L1_GROW_ALLOC_TABLE,
+BLKDBG_L1_GROW_WRITE_TABLE,
+BLKDBG_L1_GROW_ACTIVATE_TABLE,
+
+BLKDBG_L2_LOAD,
+BLKDBG_L2_UPDATE,
+BLKDBG_L2_UPDATE_COMPRESSED,
+BLKDBG_L2_ALLOC_COW_READ,
+BLKDBG_L2_ALLOC_WRITE,
+
+BLKDBG_READ,
+BLKDBG_READ_AIO,
+BLKDBG_READ_BACKING,
+BLKDBG_READ_BACKING_AIO,
+BLKDBG_READ_COMPRESSED,
+
+BLKDBG_WRITE_AIO,
+BLKDBG_WRITE_COMPRESSED,
+
+BLKDBG_VMSTATE_LOAD,
+BLKDBG_VMSTATE_SAVE,
+
+BLKDBG_COW_READ,
+BLKDBG_COW_WRITE,
+
+BLKDBG_REFTABLE_LOAD,
+BLKDBG_REFTABLE_GROW,
+
+BLKDBG_REFBLOCK_LOAD,
+BLKDBG_REFBLOCK_UPDATE,
+BLKDBG_REFBLOCK_UPDATE_PART,
+BLKDBG_REFBLOCK_ALLOC,
+BLKDBG_REFBLOCK_ALLOC_HOOKUP,
+BLKDBG_REFBLOCK_ALLOC_WRITE,
+BLKDBG_REFBLOCK_ALLOC_WRITE_BLOCKS,
+BLKDBG_REFBLOCK_ALLOC_WRITE_TABLE,
+BLKDBG_REFBLOCK_ALLOC_SWITCH_TABLE,
+
+BLKDBG_CLUSTER_ALLOC,
+BLKDBG_CLUSTER_ALLOC_BYTES,
+BLKDBG_CLUSTER_FREE,
+
 BLKDBG_EVENT_MAX,
 } BlkDebugEvent;
 
diff --git a/block/blkdebug.c b/block/blkdebug.c
index b813bfa..643c397 100644
--- a/block/blkdebug.c
+++ b/block/blkdebug.c
@@ -139,6 +139,48 @@ static QemuOptsList *config_groups[] = {
 };
 
 static const char *event_names[BLKDBG_EVENT_MAX] = {
+[BLKDBG_L1_UPDATE]  = "l1_update",
+[BLKDBG_L1_GROW_ALLOC_TABLE]= "l1_grow.alloc_table",
+[BLKDBG_L1_GROW_WRITE_TABLE]= "l1_grow.write_table",
+[BLKDBG_L1_GROW_ACTIVATE_TABLE] = "l1_grow.activate_table",
+
+[BLKDBG_L2_LOAD]= "l2_load",
+[BLKDBG_L2_UPDATE]  = "l2_update",
+[BLKDBG_L2_UPDATE_COMPRESSED]   = "l2_update_compressed",
+[BLKDBG_L2_ALLOC_COW_READ]  = "l2_alloc.cow_read",
+[BLKDBG_L2_ALLOC_WRITE] = "l2_alloc.write",
+
+[BLKDBG_READ]   = "read",
+[BLKDBG_READ_AIO]   = "read_aio",
+[BLKDBG_READ_BACKING]   = "read_backing",
+[BLKDBG_READ_BACKING_AIO]   = "read_backing_aio",
+[BLKDBG_READ_COMPRESSED]= "read_compressed",
+
+[BLKDBG_WRITE_AIO]  = "write_aio",
+[BLKDBG_WRITE_COMPRESSED]   = "write_compressed",
+
+[BLKDBG_VMSTATE_LOAD]   = "vmstate_load",
+[BLKDBG_VMSTATE_SAVE]   = "vmstate_save",
+
+[BLKDBG_COW_READ]   = "cow_read",
+[BLKDBG_COW_WRITE]  = "cow_write",
+
+[BLKDBG_REFTABLE_LOAD]  = "reftable_load",
+[BLKDBG_REFTABLE_GROW]  = "reftable_grow",
+
+[BLKDBG_REFBLOCK_LOAD]  = "refblock_load",
+[BLKDBG_REFBLOCK_UPDATE]= "refblock_update",
+[BLKDBG_REFBLOCK_UPDATE_PART]   = "refblock_update_part",
+[BLKDBG_REFBLOCK_ALLOC] = "refblock_alloc",
+[BLKDBG_REFBLOCK_ALLOC_HOOKUP]  = "refblock_alloc.hookup",
+[BLKDBG_REFBLOCK_ALLOC_WRITE]   = "refblock_alloc.write",
+[BLKDBG_REFBLOCK_ALLOC_WRITE_BLOCKS]= "refblock_alloc.write_blocks",
+[BLKDBG_REFBLOCK_ALLOC_WRITE_TABLE] = "refblock_alloc.write_table",
+[BLKDBG_REFBLOCK_ALLOC_SWITCH_TABLE]= "refblock_alloc.switch_table",
+
+[BLKDBG_CLUSTER_ALLOC]  = "cluster_alloc",
+[BLKDBG_CLUSTER_ALLOC_BYTES]= "cluster_alloc_bytes",
+[BLKDBG_CLUSTER_FREE]   = "cluster_free",
 };
 
 static int get_event_by_name(const char *name, BlkDebugEvent *event)
diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
index b13b693..9b3d686 100644
--- a/block/qcow2-cluster.c
+++ b/block/qcow2-cluster.c
@@ -54,12 +54,14 @@ int qcow2_grow_l1_table(BlockDriverState *bs, int min_size)
 memcpy(new_l1_table, s->l1_table, s->l1_size * sizeof(uint64_t));
 
 /* write new table (align to cluster) */
+BLKDBG_EVENT(s->hd, BLKDBG_L1_GROW_ALLOC_TABLE);
 new_l1_table_offset = qcow2_alloc_clusters(bs, new_l1_size2);
 if (new_l1_table_offset < 0) {
 qemu_free(new_l1_table);
 return new_l1_table_offset;
 }
 
+BLKDBG_EVENT(s->hd, BLKDBG_L1_GROW_WRITE_TABLE);
 for(i = 0; i < s->l1_size; i++)
 new_l1_table[i] = cpu_to_be64(new_l1_table[i]);
 

Re: [Qemu-devel] [PATCH -v2 02/22] vrtio-9p: Implement P9_TVERSION for 9P

2010-03-29 Thread Anthony Liguori

On 03/29/2010 02:01 AM, Aneesh Kumar K. V wrote:

On Fri, 26 Mar 2010 11:15:47 -0500, Anthony 
Liguori  wrote:
   

On 03/16/2010 04:15 AM, Aneesh Kumar K.V wrote:
 

From: Anthony Liguori

[ki...@linux.vnet.ibm.com: malloc to qemu_malloc coversion]

Signed-off-by: Anthony Liguori
Signed-off-by: Aneesh Kumar K.V
---
   hw/virtio-9p.c |  263 
+++-
   1 files changed, 261 insertions(+), 2 deletions(-)

diff --git a/hw/virtio-9p.c b/hw/virtio-9p.c
index 115c93b..53b3d78 100644
--- a/hw/virtio-9p.c
+++ b/hw/virtio-9p.c
@@ -111,10 +111,269 @@ static void free_pdu(V9fsState *s, V9fsPDU *pdu)
   }
   }

+static void v9fs_string_free(V9fsString *str)
+{
+free(str->data);

   

qemu_free.
 

+str->data = NULL;
+str->size = 0;
+}
+
+static size_t pdu_unpack(void *dst, V9fsPDU *pdu, size_t offset, size_t size)
+{
+struct iovec *sg = pdu->elem.out_sg;
+BUG_ON((offset + size)>   sg[0].iov_len);
+memcpy(dst, sg[0].iov_base + offset, size);
+return size;
+}
+
+/* FIXME i can do this with less variables */
+static size_t pdu_pack(V9fsPDU *pdu, size_t offset, const void *src, size_t 
size)
+{
+struct iovec *sg = pdu->elem.in_sg;
+size_t off = 0;
+size_t copied = 0;
+int i = 0;
+
+for (i = 0; size&&   i<   pdu->elem.in_num; i++) {
+size_t len;
+
+if (offset>= off&&   offset<   (off + sg[i].iov_len)) {
+len = MIN(sg[i].iov_len - (offset - off), size);
+memcpy(sg[i].iov_base + (offset - off), src, len);
+size -= len;
+offset += len;
+off = offset;
+copied += len;
+src += len;
+} else {
+off += sg[i].iov_len;
+}
+}
+
+return copied;
+}
+
+static int pdu_copy_sg(V9fsPDU *pdu, size_t offset, int rx, struct iovec *sg)
+{
+size_t pos = 0;
+int i, j;
+struct iovec *src_sg;
+unsigned int num;
+
+if (rx) {
+src_sg = pdu->elem.in_sg;
+num = pdu->elem.in_num;
+} else {
+src_sg = pdu->elem.out_sg;
+num = pdu->elem.out_num;
+}
+
+j = 0;
+for (i = 0; i<   num; i++) {
+if (offset<= pos) {
+sg[j].iov_base = src_sg[i].iov_base;
+sg[j].iov_len = src_sg[i].iov_len;
+j++;
+} else if (offset<   (src_sg[i].iov_len + pos)) {
+sg[j].iov_base = src_sg[i].iov_base;
+sg[j].iov_len = src_sg[i].iov_len;
+sg[j].iov_base += (offset - pos);
+sg[j].iov_len -= (offset - pos);
+j++;
+}
+pos += src_sg[i].iov_len;
+}
+
+return j;
+}
+
+static size_t pdu_unmarshal(V9fsPDU *pdu, size_t offset, const char *fmt, ...)
+{
+size_t old_offset = offset;
+va_list ap;
+int i;
+
+va_start(ap, fmt);
+for (i = 0; fmt[i]; i++) {
+   switch (fmt[i]) {
+   case 'b': {
+   int8_t *valp = va_arg(ap, int8_t *);
+   offset += pdu_unpack(valp, pdu, offset, sizeof(*valp));
+   break;
+   }
+   case 'w': {
+   int16_t *valp = va_arg(ap, int16_t *);
+   offset += pdu_unpack(valp, pdu, offset, sizeof(*valp));
+   break;
+   }
+   case 'd': {
+   int32_t *valp = va_arg(ap, int32_t *);
+   offset += pdu_unpack(valp, pdu, offset, sizeof(*valp));
+   break;
+   }
+   case 'q': {
+   int64_t *valp = va_arg(ap, int64_t *);
+   offset += pdu_unpack(valp, pdu, offset, sizeof(*valp));
+   break;
+   }
+   case 'v': {
+   struct iovec *iov = va_arg(ap, struct iovec *);
+   int *iovcnt = va_arg(ap, int *);
+   *iovcnt = pdu_copy_sg(pdu, offset, 0, iov);
+   break;
+   }
+   case 's': {
+   V9fsString *str = va_arg(ap, V9fsString *);
+   offset += pdu_unmarshal(pdu, offset, "w",&str->size);
+   /* FIXME: sanity check str->size */
+   str->data = qemu_malloc(str->size + 1);
+   offset += pdu_unpack(str->data, pdu, offset, str->size);
+   str->data[str->size] = 0;
+   break;
+   }
+   case 'Q': {
+   V9fsQID *qidp = va_arg(ap, V9fsQID *);
+   offset += pdu_unmarshal(pdu, offset, "bdq",
+   &qidp->type,&qidp->version,&qidp->path);
+   break;
+   }
+   case 'S': {
+   V9fsStat *statp = va_arg(ap, V9fsStat *);
+   offset += pdu_unmarshal(pdu, offset, "wwdQdddqsddd",
+   &statp->size,&statp->type,&statp->dev,
+   &statp->qid,&statp->mode,&statp->atime,
+   &statp->mtime,&statp->length,
+   &statp->name,&statp->uid,&statp->gid,
+   &statp->muid,&statp->extension,
+   &statp->n_uid,&statp->n_gid,
+   &statp->n_muid);
+   bre

Re: [Qemu-devel] QEMU 0.12.3 and SCSI boot

2010-03-29 Thread Gerd Hoffmann

On 03/29/10 15:51, Kevin Wolf wrote:

It actually searches the queue in case tag != s->current->tag, and it
should most likely do the same for s->current == NULL ...

Attached patch makes the rom boot for me.


Yes, works for me. And it seems to work reliably, unlike the 0.12.x
version.


Oh.  The lsi cleanup patches where supposed to be a no-op.
Looks like I fixed bugs by accident ;)

Seriously:  Could be that stable code silently does something wong when 
reaching the point where master segfaults due to the NULL pointer 
dereference.



Maybe we should include the lsi patches in stable-0.12?


Probably much easier than brewing a different version of the fix for 0.12.

cheers
  Gerd




Re: [Qemu-devel] QEMU 0.12.3 and SCSI boot

2010-03-29 Thread Kevin Wolf
Am 29.03.2010 15:41, schrieb Gerd Hoffmann:
> 
>> Tried the same with current git master and it segfaults. This segfault
>> was introduced in af12ac98 (lsi: have lsi_request for the whole life
>> time of the request):
>>
>> #0  0x0052e2d3 in lsi_command_complete (bus=0xca22f8, reason=1,
>> tag=0, arg=512) at /home/kwolf/source/qemu/hw/lsi53c895a.c:690
>> #1  0x004416e7 in qcow_aio_read_cb (opaque=0xc813f0, ret=0) at
>> block/qcow2.c:480
>> #2  0x00433028 in posix_aio_process_queue (opaque=> optimized out>) at posix-aio-compat.c:459
>> #3  0x004330cc in posix_aio_read (opaque=0xc4bb60) at
>> posix-aio-compat.c:489
>> #4  0x0040ac60 in main_loop_wait (timeout=0) at
>> /home/kwolf/source/qemu/vl.c:3949
>> #5  0x0040ce85 in main_loop (argc=,
>> argv=, envp=)
>>  at /home/kwolf/source/qemu/vl.c:4172
>> #6  main (argc=, argv=,
>> envp=) at /home/kwolf/source/qemu/vl.c:6147
>>
>> s->current is set to NULL by lsi_queue_command. I don't know the code
>> well enough to say if lsi_queue_command is wrong in setting it to NULL
>> or if lsi_command_complete shouldn't even try to access it (maybe it
>> should search in the queue for the right tag?)
> 
> It actually searches the queue in case tag != s->current->tag, and it 
> should most likely do the same for s->current == NULL ...
> 
> Attached patch makes the rom boot for me.

Yes, works for me. And it seems to work reliably, unlike the 0.12.x
version. Maybe we should include the lsi patches in stable-0.12?

Kevin




[Qemu-devel] [PATCH] lsi: fix segfault in lsi_command_complete

2010-03-29 Thread Gerd Hoffmann

Signed-off-by: Gerd Hoffmann 
---
 hw/lsi53c895a.c |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/hw/lsi53c895a.c b/hw/lsi53c895a.c
index a332401..525f3ca 100644
--- a/hw/lsi53c895a.c
+++ b/hw/lsi53c895a.c
@@ -679,7 +679,7 @@ static void lsi_command_complete(SCSIBus *bus, int reason, 
uint32_t tag,
 return;
 }
 
-if (s->waiting == 1 || tag != s->current->tag ||
+if (s->waiting == 1 || !s->current || tag != s->current->tag ||
 (lsi_irq_on_rsl(s) && !(s->scntl1 & LSI_SCNTL1_CON))) {
 if (lsi_queue_tag(s, tag, arg))
 return;
-- 
1.6.6.1





Re: [Qemu-devel] QEMU 0.12.3 and SCSI boot

2010-03-29 Thread Gerd Hoffmann



Tried the same with current git master and it segfaults. This segfault
was introduced in af12ac98 (lsi: have lsi_request for the whole life
time of the request):

#0  0x0052e2d3 in lsi_command_complete (bus=0xca22f8, reason=1,
tag=0, arg=512) at /home/kwolf/source/qemu/hw/lsi53c895a.c:690
#1  0x004416e7 in qcow_aio_read_cb (opaque=0xc813f0, ret=0) at
block/qcow2.c:480
#2  0x00433028 in posix_aio_process_queue (opaque=) at posix-aio-compat.c:459
#3  0x004330cc in posix_aio_read (opaque=0xc4bb60) at
posix-aio-compat.c:489
#4  0x0040ac60 in main_loop_wait (timeout=0) at
/home/kwolf/source/qemu/vl.c:3949
#5  0x0040ce85 in main_loop (argc=,
argv=, envp=)
 at /home/kwolf/source/qemu/vl.c:4172
#6  main (argc=, argv=,
envp=) at /home/kwolf/source/qemu/vl.c:6147

s->current is set to NULL by lsi_queue_command. I don't know the code
well enough to say if lsi_queue_command is wrong in setting it to NULL
or if lsi_command_complete shouldn't even try to access it (maybe it
should search in the queue for the right tag?)


It actually searches the queue in case tag != s->current->tag, and it 
should most likely do the same for s->current == NULL ...


Attached patch makes the rom boot for me.

cheers,
  Gerd

>From 4b385e8b5c617f2e14261a609898afdb13c12062 Mon Sep 17 00:00:00 2001
From: Gerd Hoffmann 
Date: Mon, 29 Mar 2010 15:31:03 +0200
Subject: [PATCH] lsi: fix segfault in lsi_command_complete


Signed-off-by: Gerd Hoffmann 
---
 hw/lsi53c895a.c |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/hw/lsi53c895a.c b/hw/lsi53c895a.c
index a332401..525f3ca 100644
--- a/hw/lsi53c895a.c
+++ b/hw/lsi53c895a.c
@@ -679,7 +679,7 @@ static void lsi_command_complete(SCSIBus *bus, int reason, 
uint32_t tag,
 return;
 }
 
-if (s->waiting == 1 || tag != s->current->tag ||
+if (s->waiting == 1 || !s->current || tag != s->current->tag ||
 (lsi_irq_on_rsl(s) && !(s->scntl1 & LSI_SCNTL1_CON))) {
 if (lsi_queue_tag(s, tag, arg))
 return;
-- 
1.6.6.1



Re: [Qemu-devel] Re: [PATCH 10/15] virtio-serial: Add QMP events for failed port/device add

2010-03-29 Thread Luiz Capitulino
On Sat, 27 Mar 2010 13:33:22 +0530
Amit Shah  wrote:

> On (Fri) Mar 26 2010 [14:52:36], Luiz Capitulino wrote:
> > > >  My suggestion for the immediate term is to do what we have been doing 
> > > > so
> > > > far, ie. call it VIRTIO_SERIAL_ADD. Worst case here is: we add a new way
> > > > to group events which requires a new VIRTIO_SERIAL event, in this case 
> > > > we
> > > > could emit both, the new VIRTIO_SERIAL and the old VIRTIO_SERIAL_ADD. 
> > > > The
> > > > latter would be deprecated too.
> > > 
> > > I've no problem doing it either way - whatever you prefer is fine.
> > > 
> > > BTW these are two distinct events already - failure in initialising a
> > > device and failure in initialising a port. Do you think these should be
> > > separate as well?
> > 
> >  That depends on what you want clients to know/do about it.
> > 
> >  Can ports and devices be added and work independently of each other?
> > Why is it relevant for a client to know that a _device_ has failed to
> > initialize?
> 
> I'm not sure what you mean by a client, but let's say libvirt handles
> the qemu session.

 Client is any application talking to QEMU through QMP.

> A single device can have multiple ports. If a device
> fails to register *in the guest*, all the ports associated with that
> device could be hot-unplugged on the host to reduce host resource usage.
> 
> If just a single port fails to register *in the guest*, libvirt may just
> want to hot-unplug it to free up resources.
> 
> So yes, I think both are necessary.
> 
> Anyway, I guess the answer is to split both these events.

 Yes, that makes sense.

[...]

> > > >  Or, if you can wait I can _try_ to solve this problem next week, 
> > > > although
> > > > I have no idea how hard this is going to be.
> > > 
> > > I think it's cleaner to club everything; but basically I'll go with
> > > whatever you say. I've no problem waiting.
> > 
> >  It's definitely needed to group events some way, we just have to
> > find a good way to do it. Having each subsystem doing it its own way
> > is not what we want because of protocol consistency and related
> > problems.
> 
> Yes, that's exactly why I think waiting till you iron it out would help.

 Ok.




[Qemu-devel] Re: [PATCH 3/3] monitor: Convert do_migrate() to QError

2010-03-29 Thread Luiz Capitulino
On Mon, 29 Mar 2010 14:38:35 +0200
Markus Armbruster  wrote:

> Luiz Capitulino  writes:
> 
> > On Thu, 25 Mar 2010 20:30:33 +0100
> > Markus Armbruster  wrote:
> >
> >> Luiz Capitulino  writes:
> >> 
> >> > On Thu, 25 Mar 2010 18:37:25 +0100
> >> > Markus Armbruster  wrote:
> >> >
> >> > [...]
> >> >
> >> >> >> @@ -86,12 +86,13 @@ int do_migrate(Monitor *mon, const QDict *qdict, 
> >> >> >> QObject **ret_data)
> >> >> >>  (int)qdict_get_int(qdict, 
> >> >> >> "inc"));
> >> >> >>  #endif
> >> >> >>  } else {
> >> >> >> -monitor_printf(mon, "unknown migration protocol: %s\n", 
> >> >> >> uri);
> >> >> >> +qerror_report(QERR_INVALID_PARAMETER, "uri");
> >> >> >>  return -1;
> >> >> >>  }
> >> >> >>  
> >> >> >>  if (s == NULL) {
> >> >> >> -monitor_printf(mon, "migration failed\n");
> >> >> >> +/* TODO push error reporting into the 
> >> >> >> FOO_start_outgoing_migration() */
> >> >> >> +qerror_report(QERR_MIGRATION_FAILED);
> >> >> >>  return -1;
> >> >> >>  }
> >> >> >
> >> >> >  I think this one is no better than the automatic UndefinedError
> >> >> > which is going to be triggered. I would only touch this when/if
> >> >> > we get the migration functions converted.
> >> >> 
> >> >> I feel it is a bit better, because:
> >> >> 
> >> >> * It doesn't dilute the nice "this is a bug, and I should report it"
> >> >>   property of UndefinedError.
> >> >> 
> >> >> * It avoids the "returned failure but did not pass an error" message.
> >> >>   Minor, because it's under CONFIG_DEBUG_MONITOR.
> >> >
> >> >  But this is exactly what we want because having QERR_MIGRATION_FAILED
> >> > there doesn't fix the problems those warnings are making us aware of.
> >> 
> >> Except we are already aware of the problem, and additional warnings are
> >> just noise.
> >
> >  Our memory is not the best place for it, not only because we can forget,
> > but also because it's limited on you and me.
> >
> >  Anyone who enables QMP's debugging support should be able to know what's
> > there to be done.
> 
> That's what TODO comments are for.

 It's case by case and in this very case I'd only add QERR_MIGRATION_FAILED
if it makes difference for clients, because besides silencing the warning
w/o the proper fix, this error is probably going to be dropped when the
*_outgoing_migration() functions are converted.




[Qemu-devel] [PATCH] smc91c111: allow access to reserved register

2010-03-29 Thread Lars Munch
Some drivers seems to access the reserved register in bank 0 so allow and
ignore these accesses.

Signed-off-by: Lars Munch 
---
 hw/smc91c111.c |4 
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/hw/smc91c111.c b/hw/smc91c111.c
index a2ef299..e4a2447 100644
--- a/hw/smc91c111.c
+++ b/hw/smc91c111.c
@@ -277,6 +277,8 @@ static void smc91c111_writeb(void *opaque, 
target_phys_addr_t offset,
 case 10: case 11: /* RPCR */
 /* Ignored */
 return;
+case 12: case 13: /* Reserved */
+return;
 }
 break;
 
@@ -463,6 +465,8 @@ static uint32_t smc91c111_readb(void *opaque, 
target_phys_addr_t offset)
 case 10: case 11: /* RPCR */
 /* Not implemented.  */
 return 0;
+case 12: case 13: /* Reserved */
+return 0;
 }
 break;
 
-- 
1.7.0.3





[Qemu-devel] [PATCH] smc91c111: mask register offset

2010-03-29 Thread Lars Munch
this fixes the smc91c111 emulation which has been broken for gumstix and
mainstone and maybe others since the "MMIO callback interface changes"
8da3ff180974732fc4272cb4433fef85c1822961 where commited, see:

http://thread.gmane.org/gmane.comp.emulators.qemu/33607/focus=35194

Signed-off-by: Lars Munch 
---
 hw/smc91c111.c |2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/hw/smc91c111.c b/hw/smc91c111.c
index c1a88c9..a2ef299 100644
--- a/hw/smc91c111.c
+++ b/hw/smc91c111.c
@@ -250,6 +250,7 @@ static void smc91c111_writeb(void *opaque, 
target_phys_addr_t offset,
 {
 smc91c111_state *s = (smc91c111_state *)opaque;
 
+offset = offset & 0xf;
 if (offset == 14) {
 s->bank = value;
 return;
@@ -421,6 +422,7 @@ static uint32_t smc91c111_readb(void *opaque, 
target_phys_addr_t offset)
 {
 smc91c111_state *s = (smc91c111_state *)opaque;
 
+offset = offset & 0xf;
 if (offset == 14) {
 return s->bank;
 }
-- 
1.7.0.3





Re: [Qemu-devel] QEMU 0.12.3 and SCSI boot

2010-03-29 Thread Kevin Wolf
Am 27.03.2010 10:38, schrieb Gerhard Wiesinger:
> Hello,
> 
> I'm having trouble booting from SCSI adapter 53C895a and e.g. INT13h OS 
> like MS-DOS 6.22.
> 
> I downloaded and installed the option ROM with -option-rom 8xx_64.rom:
> http://www.lsi.com/DistributionSystem/AssetDocument/files/support/ssp/sdms/Bios/lsi_bios.zip
> 
> I'm seeing that Harddisks are installed well and that also "PCI boot ROM 
> succesfully installed" message appears. So that part looks good as DDIM 
> (Device Driver Initialization Model) has been implemented.
> 
> Also booting (sometimes) and sometimes access works until nearly 
> immediatly the following problems occour (repeated messages with different 
> Tags):
> lsi_scsi: error: Reselect with pending DMA
> scsi-disk: Tag 0x0 already in use
> paio_remove: aio request not found!
> 
> So it seems to me that there is some incompatibility with the ROM and the 
> SCSI emulation (busmaster DMA?) and INT 13h.
> 
> BTW: Booting Knoppix 6.2 Live CD without any option ROM and even with 
> option ROM works well with SCSI disks (at least reading from them without 
> any errors on the console, i guess because of own drivers and not INT13h 
> access).
> 
> Any ideas to fix that issue and to bugfix it?

Tried the same with current git master and it segfaults. This segfault
was introduced in af12ac98 (lsi: have lsi_request for the whole life
time of the request):

#0  0x0052e2d3 in lsi_command_complete (bus=0xca22f8, reason=1,
tag=0, arg=512) at /home/kwolf/source/qemu/hw/lsi53c895a.c:690
#1  0x004416e7 in qcow_aio_read_cb (opaque=0xc813f0, ret=0) at
block/qcow2.c:480
#2  0x00433028 in posix_aio_process_queue (opaque=) at posix-aio-compat.c:459
#3  0x004330cc in posix_aio_read (opaque=0xc4bb60) at
posix-aio-compat.c:489
#4  0x0040ac60 in main_loop_wait (timeout=0) at
/home/kwolf/source/qemu/vl.c:3949
#5  0x0040ce85 in main_loop (argc=,
argv=, envp=)
at /home/kwolf/source/qemu/vl.c:4172
#6  main (argc=, argv=,
envp=) at /home/kwolf/source/qemu/vl.c:6147

s->current is set to NULL by lsi_queue_command. I don't know the code
well enough to say if lsi_queue_command is wrong in setting it to NULL
or if lsi_command_complete shouldn't even try to access it (maybe it
should search in the queue for the right tag?)

Gerd, do you remember how it's supposed to work?

Kevin




[Qemu-devel] Re: [PATCH 3/3] monitor: Convert do_migrate() to QError

2010-03-29 Thread Markus Armbruster
Luiz Capitulino  writes:

> On Thu, 25 Mar 2010 20:30:33 +0100
> Markus Armbruster  wrote:
>
>> Luiz Capitulino  writes:
>> 
>> > On Thu, 25 Mar 2010 18:37:25 +0100
>> > Markus Armbruster  wrote:
>> >
>> > [...]
>> >
>> >> >> @@ -86,12 +86,13 @@ int do_migrate(Monitor *mon, const QDict *qdict, 
>> >> >> QObject **ret_data)
>> >> >>  (int)qdict_get_int(qdict, 
>> >> >> "inc"));
>> >> >>  #endif
>> >> >>  } else {
>> >> >> -monitor_printf(mon, "unknown migration protocol: %s\n", uri);
>> >> >> +qerror_report(QERR_INVALID_PARAMETER, "uri");
>> >> >>  return -1;
>> >> >>  }
>> >> >>  
>> >> >>  if (s == NULL) {
>> >> >> -monitor_printf(mon, "migration failed\n");
>> >> >> +/* TODO push error reporting into the 
>> >> >> FOO_start_outgoing_migration() */
>> >> >> +qerror_report(QERR_MIGRATION_FAILED);
>> >> >>  return -1;
>> >> >>  }
>> >> >
>> >> >  I think this one is no better than the automatic UndefinedError
>> >> > which is going to be triggered. I would only touch this when/if
>> >> > we get the migration functions converted.
>> >> 
>> >> I feel it is a bit better, because:
>> >> 
>> >> * It doesn't dilute the nice "this is a bug, and I should report it"
>> >>   property of UndefinedError.
>> >> 
>> >> * It avoids the "returned failure but did not pass an error" message.
>> >>   Minor, because it's under CONFIG_DEBUG_MONITOR.
>> >
>> >  But this is exactly what we want because having QERR_MIGRATION_FAILED
>> > there doesn't fix the problems those warnings are making us aware of.
>> 
>> Except we are already aware of the problem, and additional warnings are
>> just noise.
>
>  Our memory is not the best place for it, not only because we can forget,
> but also because it's limited on you and me.
>
>  Anyone who enables QMP's debugging support should be able to know what's
> there to be done.

That's what TODO comments are for.




Re: [Qemu-devel] [PATCH] usb-bus: fix no params

2010-03-29 Thread Kevin Wolf
Am 27.03.2010 13:47, schrieb Aurelien Jarno:
> On Fri, Mar 19, 2010 at 12:59:24PM +0800, TeLeMan wrote:
>> The "params" is never NULL and the usb hid devices have no params.
> 
> This looks plainly wrong. With your patch, usb devices which don't
> accept parameters, will accept and ignore them.
> 
> What are you trying to fix here?

It looks like it's fixing -usbdevice tablet (and keyboard/mouse) which
currently fails like this:

qemu-system-x86_64: usbdevice tablet accepts no params
qemu: could not add USB device 'tablet'

He's correct in that params is never NULL (if it was NULL it's set to an
empty string some lines earlier, introduced by 702f3e0f), so
usb_create_simple is never called. Maybe the right fix is to check for
*params instead of params now?

Kevin

> 
>> Signed-off-by: TeLeMan 
>> ---
>>  hw/usb-bus.c |4 
>>  1 files changed, 0 insertions(+), 4 deletions(-)
>>
>> diff --git a/hw/usb-bus.c b/hw/usb-bus.c
>> index ce8a694..f3f1ed6 100644
>> --- a/hw/usb-bus.c
>> +++ b/hw/usb-bus.c
>> @@ -299,10 +299,6 @@ USBDevice *usbdevice_create(const char *cmdline)
>>  }
>>
>>  if (!usb->usbdevice_init) {
>> -if (params) {
>> -error_report("usbdevice %s accepts no params", driver);
>> -return NULL;
>> -}
>>  return usb_create_simple(bus, usb->qdev.name);
>>  }
>>  return usb->usbdevice_init(params);
>> -- 
>> 1.6.5.1.1367.gcd48
>> --
>> SUN OF A BEACH
>>
>>
>>
> 





Re: [Qemu-devel] [PATCH 3/3] qemu-nbd: Improve error reporting

2010-03-29 Thread Kevin Wolf
Am 28.03.2010 19:07, schrieb Ryota Ozaki:
> - use err(3) instead of errx(3) if errno is available
>   to report why failed
> - let fail prior to daemon(3) if opening a nbd file
>   is likely to fail after daemonizing to avoid silent
>   failure exit
> - add missing 'ret = 1' when unix_socket_outgoing failed
> 
> Signed-off-by: Ryota Ozaki 

Acked-by: Kevin Wolf 

> @@ -343,25 +343,31 @@ int main(int argc, char **argv)
>  return 1;
>  }
>  
> -if (bdrv_open(bs, argv[optind], flags) < 0) {
> -return 1;
> +if ((ret = bdrv_open(bs, argv[optind], flags)) < 0) {
> +errno = -ret;
> +err(EXIT_FAILURE, "Failed to bdrv_open '%s'", argv[optind]);
>  }

If you do it like this, you could do the change for even more errors,
like the ones returned by bdrv_read. But that doesn't make this patch
less correct, of course.

Kevin




Re: [Qemu-devel] [PATCH 2/3] qemu-nbd: Extend read-only option to nbd device file

2010-03-29 Thread Kevin Wolf
Am 28.03.2010 19:07, schrieb Ryota Ozaki:
> This patch allows to operate on nbd device file
> without write permission for the file if read-only
> option is specified.
> 
> Signed-off-by: Ryota Ozaki 

The help for -r should be changed, too. Currently it says:

-r, --read-only  export read-only

> ---
>  qemu-nbd.c |   10 +-
>  1 files changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/qemu-nbd.c b/qemu-nbd.c
> index 00b8896..7ef409f 100644
> --- a/qemu-nbd.c
> +++ b/qemu-nbd.c
> @@ -162,7 +162,7 @@ static int find_partition(BlockDriverState *bs, int 
> partition,
>  return -1;
>  }
>  
> -static void show_parts(const char *device)
> +static void show_parts(const char *device, bool readonly)
>  {
>  if (fork() == 0) {
>  int nbd;
> @@ -172,7 +172,7 @@ static void show_parts(const char *device)
>   * but remember to load the module with max_part != 0 :
>   * modprobe nbd max_part=63
>   */
> -nbd = open(device, O_RDWR);
> +nbd = open(device, readonly ? O_RDONLY : O_RDWR);
>  if (nbd != -1) {
>close(nbd);
>  }

Can't we always use O_RDONLY here? Assuming that this is enough to
trigger a partition table update, I haven't tested it. But if it's not
enough, wouldn't be enough for readonly either.

Kevin




Re: [Qemu-devel] [PATCH 1/3] qemu-nbd: Fix coding style

2010-03-29 Thread Kevin Wolf
Am 28.03.2010 19:07, schrieb Ryota Ozaki:
> Follow "Every indented statement is braced; even if the block
> contains just one statement." described in CODING_STYLE.
> 
> Signed-off-by: Ryota Ozaki 

Usually we fix the coding style in places that need to be changed
anyway, but avoid touching things only for style cleanup (it makes git
blame less meaningful, for example).

Kevin




Re: [Qemu-devel] [PATCH] qemu-img: add FUSE-based image access

2010-03-29 Thread Alexander Graf

On 29.03.2010, at 11:37, Jan Kiszka wrote:

> Alexander Graf wrote:
>> On 29.03.2010, at 09:46, Jan Kiszka wrote:
>> 
>>> Christoph Hellwig wrote:
 On Thu, Mar 25, 2010 at 06:52:59PM +0100, Jan Kiszka wrote:
> This adds the "map" subcommand to qemu-img. It is able to expose the raw
> content of a disk image via a FUSE filesystem. Both the whole disk can
> be accessed, e.g. to run partitioning tools against it, as well as
> individual partitions. This allows to create new filesystems in the
> image or loop-back mount exiting ones. Using the great mountlo tool
> from the FUSE collection [1][2], the latter can even be done by non-root
> users (the former anyway).
 Is there a good reason to throw this into qemu-img instead of making
 a separate qemu-fuse or similar tool?  It's doing something quite
 different than the rest of qemu-img.
 
>>> qemu-img is the swiss knife for QEMU disk image manipulation (like git
>>> is for everything around a git repository). So, IHMO, mapping the image
>>> content into the host filesystem for further manipulation with standard
>>> tools belongs to this.
>>> 
>>> If the "map" thing works out for most users, I could even imagine some
>>> helper sub-command "mount" that encapsulates map and mountlo (or some
>>> other unprivileged mounting mechanism). This should make it easier for
>>> users to explore all possibilities they have when working with disk images.
>> 
>> We also have a tool called "qemu-ext2" lying around that allows you to 
>> explore ext2 based file system contents in any qemu block layer supported 
>> backend.
> 
> "we" == SUSE?

"we" == "SUSE Studio" (in fact, Nat wrote it). It is GPL'ed, just not released 
yet. As soon as there will be a separate project with a broader scope than just 
qemu for the block layer, I'll happily invest the time to clean it up for 
upstream submission.


Alex



Re: [Qemu-devel] [PATCH] qemu-img: add FUSE-based image access

2010-03-29 Thread Jan Kiszka
Alexander Graf wrote:
> On 29.03.2010, at 09:46, Jan Kiszka wrote:
> 
>> Christoph Hellwig wrote:
>>> On Thu, Mar 25, 2010 at 06:52:59PM +0100, Jan Kiszka wrote:
 This adds the "map" subcommand to qemu-img. It is able to expose the raw
 content of a disk image via a FUSE filesystem. Both the whole disk can
 be accessed, e.g. to run partitioning tools against it, as well as
 individual partitions. This allows to create new filesystems in the
 image or loop-back mount exiting ones. Using the great mountlo tool
 from the FUSE collection [1][2], the latter can even be done by non-root
 users (the former anyway).
>>> Is there a good reason to throw this into qemu-img instead of making
>>> a separate qemu-fuse or similar tool?  It's doing something quite
>>> different than the rest of qemu-img.
>>>
>> qemu-img is the swiss knife for QEMU disk image manipulation (like git
>> is for everything around a git repository). So, IHMO, mapping the image
>> content into the host filesystem for further manipulation with standard
>> tools belongs to this.
>>
>> If the "map" thing works out for most users, I could even imagine some
>> helper sub-command "mount" that encapsulates map and mountlo (or some
>> other unprivileged mounting mechanism). This should make it easier for
>> users to explore all possibilities they have when working with disk images.
> 
> We also have a tool called "qemu-ext2" lying around that allows you to 
> explore ext2 based file system contents in any qemu block layer supported 
> backend.

"we" == SUSE?

[ Wow - just typed "qemu-ext2" into Big Brother's search bar and found
the very same mail I'm just replying to. That's fast. ]

> 
> IMHO the best move to do here (Anthony's idea) is to somehow get the full 
> block layer into a library, move it out of qemu into a separate project and 
> allow other tools in there too.
> 
> That move would vastly improve the situation of distributions too. I don't 
> want to have a qemu-img each coming from the Xen, KVM and Qemu packages. One 
> is enough :-). And it could enable block layer experienced people to be the 
> project maintainers, making that more valuable.
> 

Full ack.

Jan



signature.asc
Description: OpenPGP digital signature


[Qemu-devel] Re: [PATCH 1/5] linux-user/ia64: workaround ia64 strangenesses

2010-03-29 Thread Paolo Bonzini



+#ifdef __ia64
+sigprocmask(SIG_SETMASK, (sigset_t *)&uc->uc_sigmask, NULL);
+#else
  sigprocmask(SIG_SETMASK,&uc->uc_sigmask, NULL);
+#endif


Any reason for the ifdef?

Paolo




Re: [Qemu-devel] [PATCH] qemu-img: add FUSE-based image access

2010-03-29 Thread Alexander Graf

On 29.03.2010, at 09:46, Jan Kiszka wrote:

> Christoph Hellwig wrote:
>> On Thu, Mar 25, 2010 at 06:52:59PM +0100, Jan Kiszka wrote:
>>> This adds the "map" subcommand to qemu-img. It is able to expose the raw
>>> content of a disk image via a FUSE filesystem. Both the whole disk can
>>> be accessed, e.g. to run partitioning tools against it, as well as
>>> individual partitions. This allows to create new filesystems in the
>>> image or loop-back mount exiting ones. Using the great mountlo tool
>>> from the FUSE collection [1][2], the latter can even be done by non-root
>>> users (the former anyway).
>> 
>> Is there a good reason to throw this into qemu-img instead of making
>> a separate qemu-fuse or similar tool?  It's doing something quite
>> different than the rest of qemu-img.
>> 
> 
> qemu-img is the swiss knife for QEMU disk image manipulation (like git
> is for everything around a git repository). So, IHMO, mapping the image
> content into the host filesystem for further manipulation with standard
> tools belongs to this.
> 
> If the "map" thing works out for most users, I could even imagine some
> helper sub-command "mount" that encapsulates map and mountlo (or some
> other unprivileged mounting mechanism). This should make it easier for
> users to explore all possibilities they have when working with disk images.

We also have a tool called "qemu-ext2" lying around that allows you to explore 
ext2 based file system contents in any qemu block layer supported backend.

IMHO the best move to do here (Anthony's idea) is to somehow get the full block 
layer into a library, move it out of qemu into a separate project and allow 
other tools in there too.

That move would vastly improve the situation of distributions too. I don't want 
to have a qemu-img each coming from the Xen, KVM and Qemu packages. One is 
enough :-). And it could enable block layer experienced people to be the 
project maintainers, making that more valuable.


Alex



Re: [Qemu-devel] [RFC][PATCH 6/7] blkdebug: Add events and rules

2010-03-29 Thread Kevin Wolf
Am 28.03.2010 15:12, schrieb Christoph Hellwig:
> On Mon, Mar 15, 2010 at 06:08:34PM +0100, Kevin Wolf wrote:
>> +fprintf(stderr, "bdrv_debug_event: %d\n", event);
> 
> Is this supposed to be in the final version or a leftover debugging aid?

It's not there in the final version (which I have already sent out, btw,
so you have reviewed an old version)

>> +#define BLKDBG_EVENT(bs, evt) bdrv_debug_event(bs, evt)
> 
> Why not call bdrv_debug_event directly?

Originally I had intended to add a flag to ./configure to enable
blkdebug and #ifdef this out if it's not compiled in. Maybe we still
want to add that, but then it's not really much that you save.

>> +config = strdup(filename);
>> +config[c - filename] = '\0';
>> +ret = read_config(s, config);
>> +if (ret < 0) {
>> +return ret;
>> +}
>> +filename = c + 1;
>> +
>> +/* Open the backing file */
>> +ret = bdrv_file_open(&s->hd, filename, flags);
>> +if (ret < 0) {
>> +return ret;
>> +}
>> +
>> +return 0;
> 
> Don't we need to free config somewhere?

Oops, this one is still there in the final version. I'll send another
version of the series.

Kevin




Re: [Qemu-devel] [PATCH] qemu-img: add FUSE-based image access

2010-03-29 Thread Jan Kiszka
Christoph Hellwig wrote:
> On Thu, Mar 25, 2010 at 06:52:59PM +0100, Jan Kiszka wrote:
>> This adds the "map" subcommand to qemu-img. It is able to expose the raw
>> content of a disk image via a FUSE filesystem. Both the whole disk can
>> be accessed, e.g. to run partitioning tools against it, as well as
>> individual partitions. This allows to create new filesystems in the
>> image or loop-back mount exiting ones. Using the great mountlo tool
>> from the FUSE collection [1][2], the latter can even be done by non-root
>> users (the former anyway).
> 
> Is there a good reason to throw this into qemu-img instead of making
> a separate qemu-fuse or similar tool?  It's doing something quite
> different than the rest of qemu-img.
> 

qemu-img is the swiss knife for QEMU disk image manipulation (like git
is for everything around a git repository). So, IHMO, mapping the image
content into the host filesystem for further manipulation with standard
tools belongs to this.

If the "map" thing works out for most users, I could even imagine some
helper sub-command "mount" that encapsulates map and mountlo (or some
other unprivileged mounting mechanism). This should make it easier for
users to explore all possibilities they have when working with disk images.

Jan



signature.asc
Description: OpenPGP digital signature


Re: [Qemu-devel] Question about memory micro operations in Qemu 0.12.x

2010-03-29 Thread Alexander

Thank you for your answer.

I still have some questions.

27.03.2010 12:49, Stuart Brady пишет:

On Fri, Mar 26, 2010 at 11:23:30PM +0300, coo...@gmail.com wrote:
   

Hello.
in qemu 0.9.x there was a special file with micro-operations, which
implemented access to memory. For example for arm architecture it
was op_mem.h file. I was able to add some printfs to this functions
and get information about memory accesses.

My question is : how memory access microoperations are now
implemented in qemu 0.12.x ?

Thanks for you answers.
 

To generate load and store operations, tcg_gen_qemu_{ld,st}*() are now
used.  See tcg/README for more information on TCG ops.

In tcg/*/tcg-target.c, you'll find tcg_out_qemu_{ld,st}().  The easiest
way to do this would probably be to place your printfs in __ld_mmu() and
__st_mmu() (which are defined via softmmu_template.h), and remove the
TLB lookups from tcg_out_qemu_{ld,st}() so that your tracing code is
always called.
   


1. How can I remove TLB lookups from tcg_out_qemu_{ld,st} ?


Instead of modifying tcg_out_qemu_{ld,st}(), you might also be able to
bypass it entirely, by using having tcg_gen_qemu_{ld,st}*() generate
calls to a helper function.
   

2. Can you give me some examples of it ?

Cheers,
   


Thanks for your help.




Re: [Qemu-devel] [PATCH -v2 02/22] vrtio-9p: Implement P9_TVERSION for 9P

2010-03-29 Thread Aneesh Kumar K. V
On Fri, 26 Mar 2010 11:15:47 -0500, Anthony Liguori 
 wrote:
> On 03/16/2010 04:15 AM, Aneesh Kumar K.V wrote:
> > From: Anthony Liguori
> >
> > [ki...@linux.vnet.ibm.com: malloc to qemu_malloc coversion]
> >
> > Signed-off-by: Anthony Liguori
> > Signed-off-by: Aneesh Kumar K.V
> > ---
> >   hw/virtio-9p.c |  263 
> > +++-
> >   1 files changed, 261 insertions(+), 2 deletions(-)
> >
> > diff --git a/hw/virtio-9p.c b/hw/virtio-9p.c
> > index 115c93b..53b3d78 100644
> > --- a/hw/virtio-9p.c
> > +++ b/hw/virtio-9p.c
> > @@ -111,10 +111,269 @@ static void free_pdu(V9fsState *s, V9fsPDU *pdu)
> >   }
> >   }
> >
> > +static void v9fs_string_free(V9fsString *str)
> > +{
> > +free(str->data);
> >
> 
> qemu_free.
> > +str->data = NULL;
> > +str->size = 0;
> > +}
> > +
> > +static size_t pdu_unpack(void *dst, V9fsPDU *pdu, size_t offset, size_t 
> > size)
> > +{
> > +struct iovec *sg = pdu->elem.out_sg;
> > +BUG_ON((offset + size)>  sg[0].iov_len);
> > +memcpy(dst, sg[0].iov_base + offset, size);
> > +return size;
> > +}
> > +
> > +/* FIXME i can do this with less variables */
> > +static size_t pdu_pack(V9fsPDU *pdu, size_t offset, const void *src, 
> > size_t size)
> > +{
> > +struct iovec *sg = pdu->elem.in_sg;
> > +size_t off = 0;
> > +size_t copied = 0;
> > +int i = 0;
> > +
> > +for (i = 0; size&&  i<  pdu->elem.in_num; i++) {
> > +size_t len;
> > +
> > +if (offset>= off&&  offset<  (off + sg[i].iov_len)) {
> > +len = MIN(sg[i].iov_len - (offset - off), size);
> > +memcpy(sg[i].iov_base + (offset - off), src, len);
> > +size -= len;
> > +offset += len;
> > +off = offset;
> > +copied += len;
> > +src += len;
> > +} else {
> > +off += sg[i].iov_len;
> > +}
> > +}
> > +
> > +return copied;
> > +}
> > +
> > +static int pdu_copy_sg(V9fsPDU *pdu, size_t offset, int rx, struct iovec 
> > *sg)
> > +{
> > +size_t pos = 0;
> > +int i, j;
> > +struct iovec *src_sg;
> > +unsigned int num;
> > +
> > +if (rx) {
> > +src_sg = pdu->elem.in_sg;
> > +num = pdu->elem.in_num;
> > +} else {
> > +src_sg = pdu->elem.out_sg;
> > +num = pdu->elem.out_num;
> > +}
> > +
> > +j = 0;
> > +for (i = 0; i<  num; i++) {
> > +if (offset<= pos) {
> > +sg[j].iov_base = src_sg[i].iov_base;
> > +sg[j].iov_len = src_sg[i].iov_len;
> > +j++;
> > +} else if (offset<  (src_sg[i].iov_len + pos)) {
> > +sg[j].iov_base = src_sg[i].iov_base;
> > +sg[j].iov_len = src_sg[i].iov_len;
> > +sg[j].iov_base += (offset - pos);
> > +sg[j].iov_len -= (offset - pos);
> > +j++;
> > +}
> > +pos += src_sg[i].iov_len;
> > +}
> > +
> > +return j;
> > +}
> > +
> > +static size_t pdu_unmarshal(V9fsPDU *pdu, size_t offset, const char *fmt, 
> > ...)
> > +{
> > +size_t old_offset = offset;
> > +va_list ap;
> > +int i;
> > +
> > +va_start(ap, fmt);
> > +for (i = 0; fmt[i]; i++) {
> > +   switch (fmt[i]) {
> > +   case 'b': {
> > +   int8_t *valp = va_arg(ap, int8_t *);
> > +   offset += pdu_unpack(valp, pdu, offset, sizeof(*valp));
> > +   break;
> > +   }
> > +   case 'w': {
> > +   int16_t *valp = va_arg(ap, int16_t *);
> > +   offset += pdu_unpack(valp, pdu, offset, sizeof(*valp));
> > +   break;
> > +   }
> > +   case 'd': {
> > +   int32_t *valp = va_arg(ap, int32_t *);
> > +   offset += pdu_unpack(valp, pdu, offset, sizeof(*valp));
> > +   break;
> > +   }
> > +   case 'q': {
> > +   int64_t *valp = va_arg(ap, int64_t *);
> > +   offset += pdu_unpack(valp, pdu, offset, sizeof(*valp));
> > +   break;
> > +   }
> > +   case 'v': {
> > +   struct iovec *iov = va_arg(ap, struct iovec *);
> > +   int *iovcnt = va_arg(ap, int *);
> > +   *iovcnt = pdu_copy_sg(pdu, offset, 0, iov);
> > +   break;
> > +   }
> > +   case 's': {
> > +   V9fsString *str = va_arg(ap, V9fsString *);
> > +   offset += pdu_unmarshal(pdu, offset, "w",&str->size);
> > +   /* FIXME: sanity check str->size */
> > +   str->data = qemu_malloc(str->size + 1);
> > +   offset += pdu_unpack(str->data, pdu, offset, str->size);
> > +   str->data[str->size] = 0;
> > +   break;
> > +   }
> > +   case 'Q': {
> > +   V9fsQID *qidp = va_arg(ap, V9fsQID *);
> > +   offset += pdu_unmarshal(pdu, offset, "bdq",
> > +   &qidp->type,&qidp->version,&qidp->path);
> > +   break;
> > +   }
> > +   case 'S': {
> > +   V9fsStat *statp = va_arg(ap, V9fsStat *);
> > +   offset += pdu_unmarshal(pdu, offset, "wwdQdddqsddd",
> > +   &statp->size,&statp->type,&statp->dev,
> > +   &statp->qi