[Qemu-devel] [questions] savevm|loadvm
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
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
- 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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
* 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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
> 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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
+#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
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
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
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
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
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