[Qemu-devel] qemu audio alsa support
Hi , Can anyone tell me what are the dependencies for building qemu with alsa support ,My build is failing with the below error Error: alsa check failed Make sure to have the alsa libs and headers installed. Can anyone give me the list of packages to be installed ,I am doing a static build of qemu. Thanks and regards wasim The contents of this e-mail and any attachment(s) may contain confidential or privileged information for the intended recipient(s). Unintended recipients are prohibited from taking action on the basis of information in this e-mail and using or disseminating the information, and must notify the sender and delete it from their system. LT Infotech will not accept responsibility or liability for the accuracy or completeness of, or the presence of any virus or disabling code in this e-mail __
Re: [Qemu-devel] [RFC PATCH 0/5] allow arbitrary scaling of timers
On 03/13/2011 12:33 AM, Anthony Liguori wrote: Really nice series. The whole thing Reviewed-by: Anthony Liguori aligu...@us.ibm.com Did you really mean to RFC this? I don't think there's any sort of problem applying this as it's mostly mechanical. Yeah, it's just that the topic had been brought before and other people may have other ideas regarding the API. It's something pretty central so the API change warranted an RFC. If you want to apply it, now or in a few days, I certainly won't complain. Paolo
[Qemu-devel] Re: [PATCH v5] vnc: don't mess up with iohandlers in the vnc thread
On Thu, Mar 10, 2011 at 3:13 PM, Corentin Chary corentin.ch...@gmail.com wrote: The threaded VNC servers messed up with QEMU fd handlers without any kind of locking, and that can cause some nasty race conditions. Using qemu_mutex_lock_iothread() won't work because vnc_dpy_cpy(), which will wait for the current job queue to finish, can be called with the iothread lock held. Instead, we now store the data in a temporary buffer, and use a bottom half to notify the main thread that new data is available. vnc_[un]lock_ouput() is still needed to access VncState members like abort, csock or jobs_buffer. Signed-off-by: Corentin Chary corentin.ch...@gmail.com --- ui/vnc-jobs-async.c | 48 +--- ui/vnc-jobs.h | 1 + ui/vnc.c | 12 ui/vnc.h | 2 ++ 4 files changed, 44 insertions(+), 19 deletions(-) diff --git a/ui/vnc-jobs-async.c b/ui/vnc-jobs-async.c index f596247..4438776 100644 --- a/ui/vnc-jobs-async.c +++ b/ui/vnc-jobs-async.c @@ -28,6 +28,7 @@ #include vnc.h #include vnc-jobs.h +#include qemu_socket.h /* * Locking: @@ -155,6 +156,24 @@ void vnc_jobs_join(VncState *vs) qemu_cond_wait(queue-cond, queue-mutex); } vnc_unlock_queue(queue); + vnc_jobs_consume_buffer(vs); +} + +void vnc_jobs_consume_buffer(VncState *vs) +{ + bool flush; + + vnc_lock_output(vs); + if (vs-jobs_buffer.offset) { + vnc_write(vs, vs-jobs_buffer.buffer, vs-jobs_buffer.offset); + buffer_reset(vs-jobs_buffer); + } + flush = vs-csock != -1 vs-abort != true; + vnc_unlock_output(vs); + + if (flush) { + vnc_flush(vs); + } } /* @@ -197,7 +216,6 @@ static int vnc_worker_thread_loop(VncJobQueue *queue) VncState vs; int n_rectangles; int saved_offset; - bool flush; vnc_lock_queue(queue); while (QTAILQ_EMPTY(queue-jobs) !queue-exit) { @@ -213,6 +231,7 @@ static int vnc_worker_thread_loop(VncJobQueue *queue) vnc_lock_output(job-vs); if (job-vs-csock == -1 || job-vs-abort == true) { + vnc_unlock_output(job-vs); goto disconnected; } vnc_unlock_output(job-vs); @@ -233,10 +252,6 @@ static int vnc_worker_thread_loop(VncJobQueue *queue) if (job-vs-csock == -1) { vnc_unlock_display(job-vs-vd); - /* output mutex must be locked before going to - * disconnected: - */ - vnc_lock_output(job-vs); goto disconnected; } @@ -254,24 +269,19 @@ static int vnc_worker_thread_loop(VncJobQueue *queue) vs.output.buffer[saved_offset] = (n_rectangles 8) 0xFF; vs.output.buffer[saved_offset + 1] = n_rectangles 0xFF; - /* Switch back buffers */ vnc_lock_output(job-vs); - if (job-vs-csock == -1) { - goto disconnected; + if (job-vs-csock != -1) { + buffer_reserve(job-vs-jobs_buffer, vs.output.offset); + buffer_append(job-vs-jobs_buffer, vs.output.buffer, + vs.output.offset); + /* Copy persistent encoding data */ + vnc_async_encoding_end(job-vs, vs); + + qemu_bh_schedule(job-vs-bh); } - - vnc_write(job-vs, vs.output.buffer, vs.output.offset); - -disconnected: - /* Copy persistent encoding data */ - vnc_async_encoding_end(job-vs, vs); - flush = (job-vs-csock != -1 job-vs-abort != true); vnc_unlock_output(job-vs); - if (flush) { - vnc_flush(job-vs); - } - +disconnected: vnc_lock_queue(queue); QTAILQ_REMOVE(queue-jobs, job, next); vnc_unlock_queue(queue); diff --git a/ui/vnc-jobs.h b/ui/vnc-jobs.h index b8dab81..4c661f9 100644 --- a/ui/vnc-jobs.h +++ b/ui/vnc-jobs.h @@ -40,6 +40,7 @@ void vnc_jobs_join(VncState *vs); #ifdef CONFIG_VNC_THREAD +void vnc_jobs_consume_buffer(VncState *vs); void vnc_start_worker_thread(void); bool vnc_worker_thread_running(void); void vnc_stop_worker_thread(void); diff --git a/ui/vnc.c b/ui/vnc.c index 610f884..f28873b 100644 --- a/ui/vnc.c +++ b/ui/vnc.c @@ -1011,7 +1011,10 @@ static void vnc_disconnect_finish(VncState *vs) #ifdef CONFIG_VNC_THREAD qemu_mutex_destroy(vs-output_mutex); + qemu_bh_delete(vs-bh); + buffer_free(vs-jobs_buffer); #endif + for (i = 0; i VNC_STAT_ROWS; ++i) { qemu_free(vs-lossy_rect[i]); } @@ -1226,6 +1229,14 @@ static long vnc_client_read_plain(VncState *vs) return ret; } +#ifdef CONFIG_VNC_THREAD +static void vnc_jobs_bh(void *opaque) +{ + VncState *vs = opaque; + + vnc_jobs_consume_buffer(vs); +} +#endif /* * First function called whenever there is more data to be read from @@ -2525,6 +2536,7 @@ static void vnc_connect(VncDisplay *vd, int csock) #ifdef CONFIG_VNC_THREAD qemu_mutex_init(vs-output_mutex); + vs-bh = qemu_bh_new(vnc_jobs_bh, vs); #endif
[Qemu-devel] [PATCH] vnc: added missing name-keysym pairs for Polish national characters
Hi, There are no { name, keysym } values in name2keysym[] array for Polish national characters so -k pl option has no effect. Regards, Leszek Signed-off-by: Leszek Grzegorek leszek.grzego...@gmail.com --- diff --git a/ui/vnc_keysym.h b/ui/vnc_keysym.h index 55cb87e..df33cfe 100644 --- a/ui/vnc_keysym.h +++ b/ui/vnc_keysym.h @@ -202,6 +202,24 @@ static const name2keysym_t name2keysym[]={ { ydiaeresis, 0x0ff}, {EuroSign, 0x20ac}, /* XK_EuroSign */ +/* latin 2 - Polish national characters */ +{ eogonek, 0x1ea}, +{ Eogonek, 0x1ca}, +{ aogonek, 0x1b1}, +{ Aogonek, 0x1a1}, +{ sacute, 0x1b6}, +{ Sacute, 0x1a6}, +{ lstroke, 0x1b3}, +{ Lstroke, 0x1a3}, +{ zabovedot,0x1bf}, +{ Zabovedot,0x1af}, +{ zacute, 0x1bc}, +{ Zacute, 0x1ac}, +{ cacute, 0x1e6}, +{ Cacute, 0x1c6}, +{ nacute, 0x1f1}, +{ Nacute, 0x1d1}, + /* modifiers */ {ISO_Level3_Shift, 0xfe03}, /* XK_ISO_Level3_Shift */ {Control_L, 0xffe3}, /* XK_Control_L */
[Qemu-devel] Re: [PATCH v2] Make VNC support optional
On 03/11/11 17:35, Jan Kiszka wrote: On 2011-03-11 16:11, Peter Maydell wrote: (Also the -nographic running of serial over stdio doesn't let you kill qemu with ^C, the way -serial stdio does, which is just annoyingly inconsistent...) ^A-x to exit qemu, ^A-c to enable the monitor: -nographic comes with -serial mon:stdio. That's still suboptimal as we have no handy way to reconfigure this (there's -nodefaults, but that drops everything). The problem with this is that it is very non-intuitive if you didn't specify -nographic and expected this behavior. I think Anthony's suggestion of -display none is probably the most reasonable way to go. I'll try and cook up a patch. Cheers, Jes
[Qemu-devel] Re: [PATCH v5] vnc: don't mess up with iohandlers in the vnc thread
Am 14.03.2011 um 10:19 schrieb Corentin Chary: On Thu, Mar 10, 2011 at 3:13 PM, Corentin Chary corentin.ch...@gmail.com wrote: The threaded VNC servers messed up with QEMU fd handlers without any kind of locking, and that can cause some nasty race conditions. Using qemu_mutex_lock_iothread() won't work because vnc_dpy_cpy(), which will wait for the current job queue to finish, can be called with the iothread lock held. Instead, we now store the data in a temporary buffer, and use a bottom half to notify the main thread that new data is available. vnc_[un]lock_ouput() is still needed to access VncState members like abort, csock or jobs_buffer. Signed-off-by: Corentin Chary corentin.ch...@gmail.com --- ui/vnc-jobs-async.c | 48 +--- ui/vnc-jobs.h |1 + ui/vnc.c| 12 ui/vnc.h|2 ++ 4 files changed, 44 insertions(+), 19 deletions(-) diff --git a/ui/vnc-jobs-async.c b/ui/vnc-jobs-async.c index f596247..4438776 100644 --- a/ui/vnc-jobs-async.c +++ b/ui/vnc-jobs-async.c @@ -28,6 +28,7 @@ #include vnc.h #include vnc-jobs.h +#include qemu_socket.h /* * Locking: @@ -155,6 +156,24 @@ void vnc_jobs_join(VncState *vs) qemu_cond_wait(queue-cond, queue-mutex); } vnc_unlock_queue(queue); +vnc_jobs_consume_buffer(vs); +} + +void vnc_jobs_consume_buffer(VncState *vs) +{ +bool flush; + +vnc_lock_output(vs); +if (vs-jobs_buffer.offset) { +vnc_write(vs, vs-jobs_buffer.buffer, vs-jobs_buffer.offset); +buffer_reset(vs-jobs_buffer); +} +flush = vs-csock != -1 vs-abort != true; +vnc_unlock_output(vs); + +if (flush) { + vnc_flush(vs); +} } /* @@ -197,7 +216,6 @@ static int vnc_worker_thread_loop(VncJobQueue *queue) VncState vs; int n_rectangles; int saved_offset; -bool flush; vnc_lock_queue(queue); while (QTAILQ_EMPTY(queue-jobs) !queue-exit) { @@ -213,6 +231,7 @@ static int vnc_worker_thread_loop(VncJobQueue *queue) vnc_lock_output(job-vs); if (job-vs-csock == -1 || job-vs-abort == true) { +vnc_unlock_output(job-vs); goto disconnected; } vnc_unlock_output(job-vs); @@ -233,10 +252,6 @@ static int vnc_worker_thread_loop(VncJobQueue *queue) if (job-vs-csock == -1) { vnc_unlock_display(job-vs-vd); -/* output mutex must be locked before going to - * disconnected: - */ -vnc_lock_output(job-vs); goto disconnected; } @@ -254,24 +269,19 @@ static int vnc_worker_thread_loop(VncJobQueue *queue) vs.output.buffer[saved_offset] = (n_rectangles 8) 0xFF; vs.output.buffer[saved_offset + 1] = n_rectangles 0xFF; -/* Switch back buffers */ vnc_lock_output(job-vs); -if (job-vs-csock == -1) { -goto disconnected; +if (job-vs-csock != -1) { +buffer_reserve(job-vs-jobs_buffer, vs.output.offset); +buffer_append(job-vs-jobs_buffer, vs.output.buffer, + vs.output.offset); +/* Copy persistent encoding data */ +vnc_async_encoding_end(job-vs, vs); + + qemu_bh_schedule(job-vs-bh); } - -vnc_write(job-vs, vs.output.buffer, vs.output.offset); - -disconnected: -/* Copy persistent encoding data */ -vnc_async_encoding_end(job-vs, vs); -flush = (job-vs-csock != -1 job-vs-abort != true); vnc_unlock_output(job-vs); -if (flush) { -vnc_flush(job-vs); -} - +disconnected: vnc_lock_queue(queue); QTAILQ_REMOVE(queue-jobs, job, next); vnc_unlock_queue(queue); diff --git a/ui/vnc-jobs.h b/ui/vnc-jobs.h index b8dab81..4c661f9 100644 --- a/ui/vnc-jobs.h +++ b/ui/vnc-jobs.h @@ -40,6 +40,7 @@ void vnc_jobs_join(VncState *vs); #ifdef CONFIG_VNC_THREAD +void vnc_jobs_consume_buffer(VncState *vs); void vnc_start_worker_thread(void); bool vnc_worker_thread_running(void); void vnc_stop_worker_thread(void); diff --git a/ui/vnc.c b/ui/vnc.c index 610f884..f28873b 100644 --- a/ui/vnc.c +++ b/ui/vnc.c @@ -1011,7 +1011,10 @@ static void vnc_disconnect_finish(VncState *vs) #ifdef CONFIG_VNC_THREAD qemu_mutex_destroy(vs-output_mutex); +qemu_bh_delete(vs-bh); +buffer_free(vs-jobs_buffer); #endif + for (i = 0; i VNC_STAT_ROWS; ++i) { qemu_free(vs-lossy_rect[i]); } @@ -1226,6 +1229,14 @@ static long vnc_client_read_plain(VncState *vs) return ret; } +#ifdef CONFIG_VNC_THREAD +static void vnc_jobs_bh(void *opaque) +{ +VncState *vs = opaque; + +vnc_jobs_consume_buffer(vs); +} +#endif /* * First function called whenever there is more data to be read from @@ -2525,6 +2536,7 @@ static void vnc_connect(VncDisplay *vd, int csock) #ifdef CONFIG_VNC_THREAD qemu_mutex_init(vs-output_mutex); +vs-bh =
Re: [Qemu-devel] [PATCH] Added missing sigbus_reraise() to non-Linux platforms
On Sun, Mar 13, 2011 at 8:16 PM, Palle Lyckegaard pa...@lyckegaard.dk wrote: Hi, File cpus.c seems to be missing sigbus_reraise() on non-Linux platforms. The following patch fixes building qemu on Solaris 11 Express (SPARC). Regards Palle Signed-off-by: Palle Lyckegaard pa...@lyckegaard.dk --- cpus.c | 5 + 1 files changed, 5 insertions(+), 0 deletions(-) Great to see your SPARC Solaris 11 buildslave already catching build breakages. Thanks for the patch! Reviewed-by: Stefan Hajnoczi stefa...@linux.vnet.ibm.com
Re: [Qemu-devel] Re: Strategic decision: COW format
Am 13.03.2011 06:51, schrieb Chunqiang Tang: After the heated debate, I thought more about the right approach of implementing snapshot, and it becomes clear to me that there are major limitations with both VMDK's external snapshot approach (which stores each snapshot as a separate CoW file) and QCOW2's internal snapshot approach (which stores all snapshots in one file and uses a reference count table to keep track of them). I just posted to the mailing list a patch that implements internal snapshot in FVD but does it in a way without the limitations of VMDK and QCOW2. Let's first list the properties of an ideal virtual disk snapshot solution, and then discuss how to achieve them. G1: Do no harm (or avoid being a misfeature), i.e., the added snapshot code should not slow down the runtime performance of an image that has no snapshots. This implies that an image without snapshot should not cache the reference count table in memory and should not update the on-disk reference count table. G2: Even better, an image with 1 snapshot runs as fast as an image without snapshot. G3: Even even better, an image with 1,000 snapshots runs as fast as an image without snapshot. This basically means getting the snapshot feature for free. G4: An image with 1,000 snapshots consumes no more memory than an image without snapshot. This again means getting the snapshot feature for free. G5: Regardless of the number of existing snapshots, creating a new snapshot is fast, e.g., taking no more than 1 second. G6: Regardless of the number of existing snapshots, deleting a snapshot is fast, e.g., taking no more than 1 second. Now let's evaluate VMDK and QCOW2 against these ideal properties. G1: VMDK good; QCOW2 poor G2: VMDK ok; QCOW2 poor G3: VMDK very poor; QCOW2 poor G4: VMDK very poor; QCOW2 poor G5: VMDK good; QCOW2 good G6: VMDK poor; QCOW2 good Okay. I think I don't agree with all of these. I'm not entirely sure how VMDK works, so I take this as random image format that uses backing files (so it also applies to qcow2 with backing files, which I hope isn't too confusing). G1: VMDK good; QCOW2 poor for cache=writethrough, ok otherwise; QCOW3 good G2: VMDK ok; QCOW2 good G3: VMDK poor; QCOW2 good G4: VMDK very poor; QCOW2 ok G5: VMDK good; QCOW2 good G6: VMDK very poor; QCOW2 good Also, let me add another feature which I believe is an important factor in the decision between internal and external snapshots: G7: Loading/Reverting to a snapshot is fast G7: VMDK good; QCOW2 ok On the other hand, QCOW'2 internal snapshot has two major limitations that hurt runtime performance: caching the reference count table in memory and updating the on-disk reference count table. If we can eliminate both, then it is an ideal solution. It's not even necessary to get completely rid of it. What hurts is writing the additional metadata. So if you can delay writing the metadata and only write out a refcount block once you need to load the next one into memory, the overhead is lost in the noise (remember, even with 64k clusters, a refcount block covers 2 GB of virtual disk space). We already do that for qcow2 in all writeback cache modes. We can't do it yet for cache=writethrough, but we were planning to allow using QED's dirty flag approach which would get rid of the writes also in writethrough modes. I think this explains my estimation for G1. For G2 and G3, I'm not sure why you think that having internal snapshots slows down operation. It's basically just data that sits in the image file and is unused. After startup or after deleting a snapshot you probably have to look at all of the refcount table again for cluster allocations, is this what you mean? For G4, the size of snapshots in memory, the only overhead of internal snapshots that I could think of is the snapshot table. I would hardly rate this as poor. For G5 and G6 I basically agree with your estimation, except that I think that the overhead of deleting a snapshot is _really_ bad. This is one of the major problems we have with external snapshots today. In an internal snapshot implementation, the reference count table is used to track used blocks and free blocks. It serves no other purposes. In FVD, its static reference count table only tracks blocks used by (static) snapshots, and it does not track blocks (dynamically) allocated (on a write) or freed (on a trim) for the running VM. This is a simple but fundamental difference w.r.t. to QCOW2, whose reference count table tracks both the static content and the dynamic content. Because data blocks used by snapshots are static and do not change unless a snapshot is created or deleted, there is no need to update FVD's static reference count table when a VM runs, and actually there is even no need to cache it in memory. Data blocks that are dynamically allocated or freed for a running VM are already tracked by FVD's one-level lookup table
Re: [Qemu-devel] [PATCH -V3 2/8] hw/9pfs: Add file descriptor reclaim support
On Sun, Mar 13, 2011 at 6:57 PM, Aneesh Kumar K. V aneesh.ku...@linux.vnet.ibm.com wrote: On Sun, 13 Mar 2011 16:08:29 +, Stefan Hajnoczi stefa...@gmail.com wrote: On Sat, Mar 5, 2011 at 5:52 PM, Aneesh Kumar K.V aneesh.ku...@linux.vnet.ibm.com wrote: @@ -107,7 +108,12 @@ static int v9fs_do_closedir(V9fsState *s, DIR *dir) static int v9fs_do_open(V9fsState *s, V9fsString *path, int flags) { - return s-ops-open(s-ctx, path-data, flags); + int fd; + fd = s-ops-open(s-ctx, path-data, flags); + if (fd P9_FD_RECLAIM_THRES) { + v9fs_reclaim_fd(s); + } I think the threshold should depend on the file descriptor ulimit. The hardcoded constant doesn't work if the ulimit is set to 1000 or less (it would cause other users in QEMU to hit EMFILE errors). Yes. That is suppose to be a follow up patch. I had that set to 100 for all the early testing. Using getrlimit(2) to choose a good threshold at runtime shouldn't be a lot of code. Please add it to this patch so the threshold isn't arbitrary and possibly ineffective due to ulimit. @@ -2719,7 +2806,11 @@ static void v9fs_remove(V9fsState *s, V9fsPDU *pdu) err = -EINVAL; goto out; } - + /* + * IF the file is unlinked, we cannot reopen + * the file later. So don't reclaim fd + */ + v9fs_mark_fids_unreclaim(s, vs-fidp-fsmap.path); This poses a problem for the case where guest and host are both accessing the file system. If the fd is reclaimed and the host deletes the file, then the guest cannot access its open file anymore. The same issue also affects rename and has not been covered by this patch. Currently virtFS don't handle the host rename/unlink. That we walk a name and get the fid and then use the fid to open the file. In between if the file get removed/renamed we will get an EINVAL. All that will go away once we switch to handle based open. Can you explain this more? Will multiple entities be able to safely use the file system (e.g. host and guest)? Stefan
Re: [Qemu-devel] [PATCH -V3 1/8] hw/9pfs: Add V9fsfidmap in preparation for adding fd reclaim
On Sun, Mar 13, 2011 at 8:53 PM, Stefan Hajnoczi stefa...@gmail.com wrote: On Sun, Mar 13, 2011 at 7:06 PM, Aneesh Kumar K. V aneesh.ku...@linux.vnet.ibm.com wrote: On Sun, 13 Mar 2011 15:46:29 +, Stefan Hajnoczi stefa...@gmail.com wrote: On Sat, Mar 5, 2011 at 5:52 PM, Aneesh Kumar K.V aneesh.ku...@linux.vnet.ibm.com wrote: @@ -185,17 +188,22 @@ typedef struct V9fsXattr int flags; } V9fsXattr; +typedef struct V9fsfidmap { V9fsFidMap (naming convention) + union { + int fd; + DIR *dir; + V9fsXattr xattr; + } fs; The name fs is not meaningful. + int fid_type; + V9fsString path; + int flags; +} V9fsFidMap; + struct V9fsFidState { - int fid_type; int32_t fid; - V9fsString path; - union { - int fd; - DIR *dir; - V9fsXattr xattr; - } fs; uid_t uid; + V9fsFidMap fsmap; This name is confusing. A map is usually a container that stores key/value pairs. V9fsFidMapEntry would be clearer. But then I thought that is what V9fsFidState is? I am bad at naming. I wanted to indicate something that can be shared across multiple fids and also indicate the local file system mapping/data. I will take any suggestion. Where does sharing happen, I didn't notice any code that shares fds between fids? I saw your response to a later patch in this series that fd sharing is not implemented. In that case this patch doesn't add value, it just makes the code harder to understand by introducing an unused level of indirection for which we don't have a good name yet. Perhaps drop this patch from the series and send it later if you implement fd sharing. Stefan
[Qemu-devel] Re: qemu-0.14.0 doesn't compile on ppc32
On 21 February 2011 08:10, Paolo Bonzini pbonz...@redhat.com wrote: On 02/20/2011 06:32 PM, Peter Maydell wrote: Some of qemu's code does seem to trigger rather excessive memory use by gcc; for instance we've had problems with memory usage building for ARM with gcc of target-sparc/translate.c wanting gigabytes of RAM with some compiler flags: https://bugs.launchpad.net/gcc-linaro/+bug/714921 I suspect it's all those large switch statements... It's on my todo list to report it to GCC, since this memory-hog behavior is a GCC regression. We've been working on this in Linaro, and the following two patches have been posted upstream: http://gcc.gnu.org/ml/gcc-patches/2011-03/msg00193.html http://gcc.gnu.org/ml/gcc-patches/2011-03/msg00255.html so you can probably cross that item off your todo list :-) -- PMM
Re: [Qemu-devel] [PATCH -V3 7/8] hw/9pfs: Add new virtfs option cache=none to skip host page cache
On Sun, Mar 13, 2011 at 7:04 PM, Aneesh Kumar K. V aneesh.ku...@linux.vnet.ibm.com wrote: On Sun, 13 Mar 2011 17:23:50 +, Stefan Hajnoczi stefa...@gmail.com wrote: On Sat, Mar 5, 2011 at 5:52 PM, Aneesh Kumar K.V aneesh.ku...@linux.vnet.ibm.com wrote: cache=none implies the file are opened in the host with O_SYNC open flag O_SYNC does not bypass the host page cache. It ensures that writes only complete once data has been written to the disk. O_DIRECT is a hint to bypass the host page cache when possible. A boolean on|off option would be nicer than an option that takes the special string none. For example, direct=on|off. It also makes the code nicer by using bools instead of strdup strings that get leaked. What i wanted is the O_SYNC behavior. Well the comment should be updated. I want to make sure that we don't have dirty data in host page cache after a write. It is always good to make read hit the page cache Why silently enforce O_SYNC on the server side? The client does not know whether or not O_SYNC is in effect, cannot take advantage of that knowledge, and cannot control it. I think a more useful solution is a 9p client mount option called sync that caused the client to always add O_SYNC and skip syncfs. The whole stack becomes aware of O_SYNC and clients are in control over whether or not they need O_SYNC semantics. Stefan
Re: [Qemu-devel] Re: qemu-0.14.0 doesn't compile on ppc32
On Mon, Mar 14, 2011 at 10:23 AM, Peter Maydell peter.mayd...@linaro.org wrote: On 21 February 2011 08:10, Paolo Bonzini pbonz...@redhat.com wrote: On 02/20/2011 06:32 PM, Peter Maydell wrote: Some of qemu's code does seem to trigger rather excessive memory use by gcc; for instance we've had problems with memory usage building for ARM with gcc of target-sparc/translate.c wanting gigabytes of RAM with some compiler flags: https://bugs.launchpad.net/gcc-linaro/+bug/714921 I suspect it's all those large switch statements... It's on my todo list to report it to GCC, since this memory-hog behavior is a GCC regression. We've been working on this in Linaro, and the following two patches have been posted upstream: http://gcc.gnu.org/ml/gcc-patches/2011-03/msg00193.html http://gcc.gnu.org/ml/gcc-patches/2011-03/msg00255.html so you can probably cross that item off your todo list :-) Interesting. Alexander Graf and I recently hit an out of memory when building translate.o on ppc64 host. It was solved by adding more RAM to the box but this patch looks nice too ;). Stefan
[Qemu-devel] Re: [PATCH] Added missing sigbus_reraise() to non-Linux platforms
On 2011-03-14 11:03, Stefan Hajnoczi wrote: On Sun, Mar 13, 2011 at 8:16 PM, Palle Lyckegaard pa...@lyckegaard.dk wrote: Hi, File cpus.c seems to be missing sigbus_reraise() on non-Linux platforms. The following patch fixes building qemu on Solaris 11 Express (SPARC). Regards Palle Signed-off-by: Palle Lyckegaard pa...@lyckegaard.dk --- cpus.c |5 + 1 files changed, 5 insertions(+), 0 deletions(-) Great to see your SPARC Solaris 11 buildslave already catching build breakages. Thanks for the patch! Reviewed-by: Stefan Hajnoczi stefa...@linux.vnet.ibm.com There is already an (implicit) fix [1] on the way. Will come via uq/master unless someone applies it earlier directly. Jan [1] http://permalink.gmane.org/gmane.comp.emulators.kvm.devel/68818 signature.asc Description: OpenPGP digital signature
[Qemu-devel] Re: qemu-0.14.0 doesn't compile on ppc32
On 03/14/2011 11:23 AM, Peter Maydell wrote: On 21 February 2011 08:10, Paolo Bonzinipbonz...@redhat.com wrote: On 02/20/2011 06:32 PM, Peter Maydell wrote: Some of qemu's code does seem to trigger rather excessive memory use by gcc; for instance we've had problems with memory usage building for ARM with gcc of target-sparc/translate.c wanting gigabytes of RAM with some compiler flags: https://bugs.launchpad.net/gcc-linaro/+bug/714921 I suspect it's all those large switch statements... It's on my todo list to report it to GCC, since this memory-hog behavior is a GCC regression. We've been working on this in Linaro, and the following two patches have been posted upstream: http://gcc.gnu.org/ml/gcc-patches/2011-03/msg00193.html http://gcc.gnu.org/ml/gcc-patches/2011-03/msg00255.html so you can probably cross that item off your todo list :-) Yes, I'm running with those two patches backported. :) Paolo
[Qemu-devel] KVM call agenda for Mars 14th
Please send any agenda items you are interested in covering. Thanks, Juan. pgpjk4K84MhkE.pgp Description: PGP signature
[Qemu-devel] [PATCH 0/5] Introduce -display and make VNC optional
From: Jes Sorensen jes.soren...@redhat.com Hi, Here is the follow-up patches after the discussion on Friday. I have introduced a new -display argument to consolidate the current -sdl/-curses/-nographic arguments. Patch 2 introduces -display none which was suggested by Anthony. Patches 3+4 adds error warnings if a user tries to call -sdl or -curses when these features are not enabled and patch five finally makes VNC optional in the build. Longer term we should add sub-argument support to -display so we can make -display handle all the sub-arguments for -sdl etc. Comments? Cheers, Jes Jes Sorensen (5): Introduce -display argument Introduce -display none error message if user specifies SDL cmd line option when SDL is disabled error message if user specifies curses on cmd line when curses is disabled Make VNC support optional Makefile.objs | 19 +++--- configure | 37 +++- console.h | 26 ++- monitor.c | 22 +++- qemu-options.hx | 46 ++- qerror.h|3 ++ sysemu.h|1 + ui/vnc.c| 14 +++--- vl.c| 72 ++ 9 files changed, 183 insertions(+), 57 deletions(-) -- 1.7.4
[Qemu-devel] [PATCH 2/5] Introduce -display none
From: Jes Sorensen jes.soren...@redhat.com New option -display none. This option differs from -display nographic by not trying to take control of stdio etc. but instead behaves as if a graphics display is enabled, except that it doesn't show one. Signed-off-by: Jes Sorensen jes.soren...@redhat.com --- qemu-options.hx |6 ++ sysemu.h|1 + vl.c|2 ++ 3 files changed, 9 insertions(+), 0 deletions(-) diff --git a/qemu-options.hx b/qemu-options.hx index f14ff02..21c0d97 100644 --- a/qemu-options.hx +++ b/qemu-options.hx @@ -615,6 +615,12 @@ you can totally disable graphical output so that QEMU is a simple command line application. The emulated serial port is redirected on the console. Therefore, you can still use QEMU to debug a Linux kernel with a serial console. +@item none +Pick the none display option. This option will still run with an +emulated graphics card, but none will be displayed to the QEMU +user. This options differs from the -nographic option in that QEMU +will behave like if one of the display options had been picked, it +will not change the control on the command line. @end table ETEXI diff --git a/sysemu.h b/sysemu.h index 0a83ab9..c43c7af 100644 --- a/sysemu.h +++ b/sysemu.h @@ -110,6 +110,7 @@ typedef enum DisplayType DT_CURSES, DT_SDL, DT_NOGRAPHIC, +DT_NONE, } DisplayType; extern int autostart; diff --git a/vl.c b/vl.c index e797e61..8ed4607 100644 --- a/vl.c +++ b/vl.c @@ -1577,6 +1577,8 @@ static DisplayType select_display(const char *p) #endif } else if (strstart(p, nographic, opts)) { display = DT_NOGRAPHIC; +} else if (strstart(p, none, opts)) { +display = DT_NONE; } else { fprintf(stderr, Unknown display type: %s\n, p); exit(1); -- 1.7.4
[Qemu-devel] [PATCH 4/5] error message if user specifies curses on cmd line when curses is disabled
From: Jes Sorensen jes.soren...@redhat.com Signed-off-by: Jes Sorensen jes.soren...@redhat.com --- qemu-options.hx |2 -- vl.c|7 +-- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/qemu-options.hx b/qemu-options.hx index 960762a..55f0872 100644 --- a/qemu-options.hx +++ b/qemu-options.hx @@ -637,11 +637,9 @@ the console. Therefore, you can still use QEMU to debug a Linux kernel with a serial console. ETEXI -#ifdef CONFIG_CURSES DEF(curses, 0, QEMU_OPTION_curses, -curses use a curses/ncurses interface instead of SDL\n, QEMU_ARCH_ALL) -#endif STEXI @item -curses @findex curses diff --git a/vl.c b/vl.c index ab78e4c..f256f86 100644 --- a/vl.c +++ b/vl.c @@ -2191,11 +2191,14 @@ int main(int argc, char **argv, char **envp) case QEMU_OPTION_nographic: display_type = DT_NOGRAPHIC; break; -#ifdef CONFIG_CURSES case QEMU_OPTION_curses: +#ifdef CONFIG_CURSES display_type = DT_CURSES; -break; +#else +fprintf(stderr, Curses support is disabled\n); +exit(1); #endif +break; case QEMU_OPTION_portrait: graphic_rotate = 1; break; -- 1.7.4
[Qemu-devel] [PATCH 1/5] Introduce -display argument
From: Jes Sorensen jes.soren...@redhat.com This patch introduces a -display argument which consolidates the setting of the display mode. Valid options are: sdl/curses/default/nographic Signed-off-by: Jes Sorensen jes.soren...@redhat.com --- qemu-options.hx | 28 vl.c| 34 ++ 2 files changed, 62 insertions(+), 0 deletions(-) diff --git a/qemu-options.hx b/qemu-options.hx index badb730..f14ff02 100644 --- a/qemu-options.hx +++ b/qemu-options.hx @@ -590,6 +590,34 @@ STEXI @table @option ETEXI +DEF(display, HAS_ARG, QEMU_OPTION_display, +-display [default|sdl|curses|nographic|none]\n +select display type\n, QEMU_ARCH_ALL) +STEXI +@item -display @var{type} +@findex -display +Select type of display to use. This option is a replacement for the +old style -sdl/-curses/... options. Valid values for @var{type} are +@table @option +@item default +Pick the default display option. This will depend on how QEMU was +configured. (This one is the default) +@item sdl +Pick the SDL display option. +@item curses +Pick the curses display option. Normally, QEMU uses SDL to display the +VGA output. With this option, QEMU can display the VGA output when in +text mode using a curses/ncurses interface. Nothing is displayed in +graphical mode. +@item nographic +Normally, QEMU uses SDL to display the VGA output. With this option, +you can totally disable graphical output so that QEMU is a simple +command line application. The emulated serial port is redirected on +the console. Therefore, you can still use QEMU to debug a Linux kernel +with a serial console. +@end table +ETEXI + DEF(nographic, 0, QEMU_OPTION_nographic, -nographic disable graphical output and redirect serial I/Os to console\n, QEMU_ARCH_ALL) diff --git a/vl.c b/vl.c index 5e007a7..e797e61 100644 --- a/vl.c +++ b/vl.c @@ -1554,6 +1554,37 @@ static void select_vgahw (const char *p) } } +static DisplayType select_display(const char *p) +{ +const char *opts; +DisplayType display = DT_DEFAULT; + +if (strstart(p, default, opts)) { +display = DT_DEFAULT; +} else if (strstart(p, sdl, opts)) { +#ifdef CONFIG_SDL +display = DT_SDL; +#else +fprintf(stderr, SDL support is disabled\n); +exit(1); +#endif +} else if (strstart(p, curses, opts)) { +#ifdef CONFIG_CURSES +display = DT_CURSES; +#else +fprintf(stderr, Curses support is disabled\n); +exit(1); +#endif +} else if (strstart(p, nographic, opts)) { +display = DT_NOGRAPHIC; +} else { +fprintf(stderr, Unknown display type: %s\n, p); +exit(1); +} + +return display; +} + static int balloon_parse(const char *arg) { QemuOpts *opts; @@ -2152,6 +2183,9 @@ int main(int argc, char **argv, char **envp) } numa_add(optarg); break; +case QEMU_OPTION_display: +display_type = select_display(optarg); +break; case QEMU_OPTION_nographic: display_type = DT_NOGRAPHIC; break; -- 1.7.4
[Qemu-devel] [PATCH 5/5] Make VNC support optional
From: Jes Sorensen jes.soren...@redhat.com Per default VNC is enabled. Signed-off-by: Jes Sorensen jes.soren...@redhat.com --- Makefile.objs | 19 ++- configure | 37 + console.h | 26 -- monitor.c | 22 ++ qerror.h |3 +++ ui/vnc.c | 14 ++ vl.c | 21 + 7 files changed, 99 insertions(+), 43 deletions(-) diff --git a/Makefile.objs b/Makefile.objs index a52f42f..9796d12 100644 --- a/Makefile.objs +++ b/Makefile.objs @@ -127,19 +127,20 @@ common-obj-y += $(addprefix audio/, $(audio-obj-y)) ui-obj-y += keymaps.o ui-obj-$(CONFIG_SDL) += sdl.o sdl_zoom.o x_keymap.o ui-obj-$(CONFIG_CURSES) += curses.o -ui-obj-y += vnc.o d3des.o -ui-obj-y += vnc-enc-zlib.o vnc-enc-hextile.o -ui-obj-y += vnc-enc-tight.o vnc-palette.o -ui-obj-y += vnc-enc-zrle.o -ui-obj-$(CONFIG_VNC_TLS) += vnc-tls.o vnc-auth-vencrypt.o -ui-obj-$(CONFIG_VNC_SASL) += vnc-auth-sasl.o -ui-obj-$(CONFIG_COCOA) += cocoa.o +vnc-obj-y += vnc.o d3des.o +vnc-obj-y += vnc-enc-zlib.o vnc-enc-hextile.o +vnc-obj-y += vnc-enc-tight.o vnc-palette.o +vnc-obj-y += vnc-enc-zrle.o +vnc-obj-$(CONFIG_VNC_TLS) += vnc-tls.o vnc-auth-vencrypt.o +vnc-obj-$(CONFIG_VNC_SASL) += vnc-auth-sasl.o +vnc-obj-$(CONFIG_COCOA) += cocoa.o ifdef CONFIG_VNC_THREAD -ui-obj-y += vnc-jobs-async.o +vnc-obj-y += vnc-jobs-async.o else -ui-obj-y += vnc-jobs-sync.o +vnc-obj-y += vnc-jobs-sync.o endif common-obj-y += $(addprefix ui/, $(ui-obj-y)) +common-obj-$(CONFIG_VNC) += $(addprefix ui/, $(vnc-obj-y)) common-obj-y += iov.o acl.o common-obj-$(CONFIG_POSIX) += qemu-thread-posix.o compatfd.o diff --git a/configure b/configure index a166de0..abd3317 100755 --- a/configure +++ b/configure @@ -117,6 +117,7 @@ kvm= kvm_para= nptl= sdl= +vnc=yes sparse=no uuid= vde= @@ -539,6 +540,10 @@ for opt do ;; --enable-sdl) sdl=yes ;; + --disable-vnc) vnc=no + ;; + --enable-vnc) vnc=yes + ;; --fmod-lib=*) fmod_lib=$optarg ;; --fmod-inc=*) fmod_inc=$optarg @@ -836,6 +841,8 @@ echo --disable-strip disable stripping binaries echo --disable-werror disable compilation abort on warning echo --disable-sdldisable SDL echo --enable-sdl enable SDL +echo --disable-vncdisable VNC +echo --enable-vnc enable VNC echo --enable-cocoa enable COCOA (Mac OS X only) echo --audio-drv-list=LISTset audio drivers list: echoAvailable drivers: $audio_possible_drivers @@ -1273,7 +1280,7 @@ fi ## # VNC TLS detection -if test $vnc_tls != no ; then +if test $vnc = yes -a $vnc_tls != no ; then cat $TMPC EOF #include gnutls/gnutls.h int main(void) { gnutls_session_t s; gnutls_init(s, GNUTLS_SERVER); return 0; } @@ -1293,7 +1300,7 @@ fi ## # VNC SASL detection -if test $vnc_sasl != no ; then +if test $vnc = yes -a $vnc_sasl != no ; then cat $TMPC EOF #include sasl/sasl.h #include stdio.h @@ -1315,7 +1322,7 @@ fi ## # VNC JPEG detection -if test $vnc_jpeg != no ; then +if test $vnc = yes -a $vnc_jpeg != no ; then cat $TMPC EOF #include stdio.h #include jpeglib.h @@ -1336,7 +1343,7 @@ fi ## # VNC PNG detection -if test $vnc_png != no ; then +if test $vnc = yes -a $vnc_png != no ; then cat $TMPC EOF //#include stdio.h #include png.h @@ -2495,11 +2502,14 @@ echo Audio drivers $audio_drv_list echo Extra audio cards $audio_card_list echo Block whitelist $block_drv_whitelist echo Mixer emulation $mixemu -echo VNC TLS support $vnc_tls -echo VNC SASL support $vnc_sasl -echo VNC JPEG support $vnc_jpeg -echo VNC PNG support $vnc_png -echo VNC thread$vnc_thread +echo VNC support $vnc +if test $vnc = yes ; then +echo VNC TLS support $vnc_tls +echo VNC SASL support $vnc_sasl +echo VNC JPEG support $vnc_jpeg +echo VNC PNG support $vnc_png +echo VNC thread$vnc_thread +fi if test -n $sparc_cpu; then echo Target Sparc Arch $sparc_cpu fi @@ -2649,6 +2659,9 @@ echo CONFIG_BDRV_WHITELIST=$block_drv_whitelist $config_host_mak if test $mixemu = yes ; then echo CONFIG_MIXEMU=y $config_host_mak fi +if test $vnc = yes ; then + echo CONFIG_VNC=y $config_host_mak +fi if test $vnc_tls = yes ; then echo CONFIG_VNC_TLS=y $config_host_mak echo VNC_TLS_CFLAGS=$vnc_tls_cflags $config_host_mak @@ -2657,15 +2670,15 @@ if test $vnc_sasl = yes ; then echo CONFIG_VNC_SASL=y $config_host_mak echo VNC_SASL_CFLAGS=$vnc_sasl_cflags $config_host_mak fi -if test $vnc_jpeg != no ; then +if test $vnc_jpeg = yes ; then echo CONFIG_VNC_JPEG=y $config_host_mak echo VNC_JPEG_CFLAGS=$vnc_jpeg_cflags $config_host_mak fi -if test $vnc_png != no ; then +if
[Qemu-devel] [PATCH 0/2] Fix VRECPS edge cases handling
This patchset fixes the edge case handling of VRECPS. Patch 2/2 is just a bit of cleanup of the neighbouring vrsqrts helper which can then use the float32_two introduced by 1/1. Tested in the usual random-insn-generation way and also with the neon64 test program from the valgrind ARM testsuite. Peter Maydell (2): target-arm: Fix VRECPS edge cases handling target-arm: use make_float32() to make constant floats for VRSQRTS target-arm/helper.c | 22 +- 1 files changed, 13 insertions(+), 9 deletions(-)
[Qemu-devel] [PATCH 3/5] error message if user specifies SDL cmd line option when SDL is disabled
From: Jes Sorensen jes.soren...@redhat.com Signed-off-by: Jes Sorensen jes.soren...@redhat.com --- qemu-options.hx | 10 -- vl.c|8 2 files changed, 8 insertions(+), 10 deletions(-) diff --git a/qemu-options.hx b/qemu-options.hx index 21c0d97..960762a 100644 --- a/qemu-options.hx +++ b/qemu-options.hx @@ -650,11 +650,9 @@ QEMU can display the VGA output when in text mode using a curses/ncurses interface. Nothing is displayed in graphical mode. ETEXI -#ifdef CONFIG_SDL DEF(no-frame, 0, QEMU_OPTION_no_frame, -no-frame open SDL window without a frame and window decorations\n, QEMU_ARCH_ALL) -#endif STEXI @item -no-frame @findex -no-frame @@ -663,42 +661,34 @@ available screen space. This makes the using QEMU in a dedicated desktop workspace more convenient. ETEXI -#ifdef CONFIG_SDL DEF(alt-grab, 0, QEMU_OPTION_alt_grab, -alt-grab use Ctrl-Alt-Shift to grab mouse (instead of Ctrl-Alt)\n, QEMU_ARCH_ALL) -#endif STEXI @item -alt-grab @findex -alt-grab Use Ctrl-Alt-Shift to grab mouse (instead of Ctrl-Alt). ETEXI -#ifdef CONFIG_SDL DEF(ctrl-grab, 0, QEMU_OPTION_ctrl_grab, -ctrl-grab use Right-Ctrl to grab mouse (instead of Ctrl-Alt)\n, QEMU_ARCH_ALL) -#endif STEXI @item -ctrl-grab @findex -ctrl-grab Use Right-Ctrl to grab mouse (instead of Ctrl-Alt). ETEXI -#ifdef CONFIG_SDL DEF(no-quit, 0, QEMU_OPTION_no_quit, -no-quitdisable SDL window close capability\n, QEMU_ARCH_ALL) -#endif STEXI @item -no-quit @findex -no-quit Disable SDL window close capability. ETEXI -#ifdef CONFIG_SDL DEF(sdl, 0, QEMU_OPTION_sdl, -sdlenable SDL\n, QEMU_ARCH_ALL) -#endif STEXI @item -sdl @findex -sdl diff --git a/vl.c b/vl.c index 8ed4607..ab78e4c 100644 --- a/vl.c +++ b/vl.c @@ -2569,6 +2569,14 @@ int main(int argc, char **argv, char **envp) case QEMU_OPTION_sdl: display_type = DT_SDL; break; +#else +case QEMU_OPTION_no_frame: +case QEMU_OPTION_alt_grab: +case QEMU_OPTION_ctrl_grab: +case QEMU_OPTION_no_quit: +case QEMU_OPTION_sdl: +fprintf(stderr, SDL support is disabled\n); +exit(1); #endif case QEMU_OPTION_pidfile: pid_file = optarg; -- 1.7.4
[Qemu-devel] [PATCH 2/2] target-arm: use make_float32() to make constant floats for VRSQRTS
The preferred way to create a constant floating point value is to use make_float32() rather than doing a runtime int32_to_float32(). Convert the code in the VRSQRTS helper to work this way. Signed-off-by: Peter Maydell peter.mayd...@linaro.org --- target-arm/helper.c | 11 +-- 1 files changed, 5 insertions(+), 6 deletions(-) diff --git a/target-arm/helper.c b/target-arm/helper.c index c01a5a2..c3238d3 100644 --- a/target-arm/helper.c +++ b/target-arm/helper.c @@ -2708,6 +2708,8 @@ uint32_t HELPER(vfp_fcvt_f32_to_f16)(float32 a, CPUState *env) } #define float32_two make_float32(0x4000) +#define float32_three make_float32(0x4040) +#define float32_one_point_five makefloat32(0x3fc0) float32 HELPER(recps_f32)(float32 a, float32 b, CPUState *env) { @@ -2722,16 +2724,13 @@ float32 HELPER(recps_f32)(float32 a, float32 b, CPUState *env) float32 HELPER(rsqrts_f32)(float32 a, float32 b, CPUState *env) { float_status *s = env-vfp.standard_fp_status; -float32 two = int32_to_float32(2, s); -float32 three = int32_to_float32(3, s); float32 product; if ((float32_is_infinity(a) float32_is_zero_or_denormal(b)) || (float32_is_infinity(b) float32_is_zero_or_denormal(a))) { -product = float32_zero; -} else { -product = float32_mul(a, b, s); +return float32_one_point_five; } -return float32_div(float32_sub(three, product, s), two, s); +product = float32_mul(a, b, s); +return float32_div(float32_sub(float32_three, product, s), float32_two, s); } /* NEON helpers. */ -- 1.7.1
Re: [Qemu-devel] KVM call agenda for Mars 14th
On 03/14/11 13:14, Juan Quintela wrote: Please send any agenda items you are interested in covering. Thanks, Juan. I presume you mean for March 15? Today is the 14th and it is Monday :)
[Qemu-devel] Re: [PATCH 1/5] Introduce -display argument
On 03/14/11 13:40, Peter Maydell wrote: On 14 March 2011 12:28, jes.soren...@redhat.com wrote: This patch introduces a -display argument which consolidates the setting of the display mode. Valid options are: sdl/curses/default/nographic I wasn't expecting to see nographic as an option here -- my preference would be to retain the existing '-nographic' switch for backwards-compatibility, but '-display foo' should only affect the video type. (ie the equivalent of '-nographic' ought to be a set of options including '-display none', '-serial mon:stdio' and so on, not a single '-display:nographic'). It would be easy to remove it from the -display option, but I think it is fair to keep it in. I was pretty much trying to cover every DT_foo option we have. If nographic is going away as a display option, we should make it in the code that it is for legacy support only. Cheers, Jes
[Qemu-devel] [PATCH 1/2] target-arm: Fix VRECPS edge cases handling
Correct the handling of edge cases for the VRECPS instruction: * this is a Neon instruction so uses the standard FPSCR value * (zero, inf) is a special case which returns 2.0 Signed-off-by: Peter Maydell peter.mayd...@linaro.org --- target-arm/helper.c | 11 --- 1 files changed, 8 insertions(+), 3 deletions(-) diff --git a/target-arm/helper.c b/target-arm/helper.c index d360121..c01a5a2 100644 --- a/target-arm/helper.c +++ b/target-arm/helper.c @@ -2707,11 +2707,16 @@ uint32_t HELPER(vfp_fcvt_f32_to_f16)(float32 a, CPUState *env) return do_fcvt_f32_to_f16(a, env, env-vfp.fp_status); } +#define float32_two make_float32(0x4000) + float32 HELPER(recps_f32)(float32 a, float32 b, CPUState *env) { -float_status *s = env-vfp.fp_status; -float32 two = int32_to_float32(2, s); -return float32_sub(two, float32_mul(a, b, s), s); +float_status *s = env-vfp.standard_fp_status; +if ((float32_is_infinity(a) float32_is_zero_or_denormal(b)) || +(float32_is_infinity(b) float32_is_zero_or_denormal(a))) { +return float32_two; +} +return float32_sub(float32_two, float32_mul(a, b, s), s); } float32 HELPER(rsqrts_f32)(float32 a, float32 b, CPUState *env) -- 1.7.1
[Qemu-devel] Re: [PATCH 1/5] Introduce -display argument
On 14 March 2011 12:28, jes.soren...@redhat.com wrote: This patch introduces a -display argument which consolidates the setting of the display mode. Valid options are: sdl/curses/default/nographic I wasn't expecting to see nographic as an option here -- my preference would be to retain the existing '-nographic' switch for backwards-compatibility, but '-display foo' should only affect the video type. (ie the equivalent of '-nographic' ought to be a set of options including '-display none', '-serial mon:stdio' and so on, not a single '-display:nographic'). -- PMM
[Qemu-devel] [PATCH] report that QEMU process was killed by a signal
Currently when rogue script kills QEMU process (using TERM/INT/HUP signal) it looks indistinguishable from system shutdown. Lets report that QMEU was killed and leave some clues about the killed identity. Signed-off-by: Gleb Natapov g...@redhat.com diff --git a/os-posix.c b/os-posix.c index 38c29d1..7e175e8 100644 --- a/os-posix.c +++ b/os-posix.c @@ -61,8 +61,9 @@ void os_setup_early_signal_handling(void) sigaction(SIGPIPE, act, NULL); } -static void termsig_handler(int signal) +static void termsig_handler(int signal, siginfo_t *info, void *c) { +fprintf(stderr, Got signal %d from pid %d\n, info-si_signo, info-si_pid); qemu_system_shutdown_request(); } @@ -76,7 +77,8 @@ void os_setup_signal_handling(void) struct sigaction act; memset(act, 0, sizeof(act)); -act.sa_handler = termsig_handler; +act.sa_sigaction = termsig_handler; +act.sa_flags = SA_SIGINFO; sigaction(SIGINT, act, NULL); sigaction(SIGHUP, act, NULL); sigaction(SIGTERM, act, NULL); -- Gleb.
Re: [Qemu-devel] [PATCH] report that QEMU process was killed by a signal
On Mon, Mar 14, 2011 at 02:58:55PM +0200, Gleb Natapov wrote: Currently when rogue script kills QEMU process (using TERM/INT/HUP signal) it looks indistinguishable from system shutdown. Lets report that QMEU was killed and leave some clues about the killed identity. Good idea, but Signed-off-by: Gleb Natapov g...@redhat.com diff --git a/os-posix.c b/os-posix.c index 38c29d1..7e175e8 100644 --- a/os-posix.c +++ b/os-posix.c @@ -61,8 +61,9 @@ void os_setup_early_signal_handling(void) sigaction(SIGPIPE, act, NULL); } -static void termsig_handler(int signal) +static void termsig_handler(int signal, siginfo_t *info, void *c) { +fprintf(stderr, Got signal %d from pid %d\n, info-si_signo, info-si_pid); qemu_system_shutdown_request(); ...fprintf() isn't async signal safe. It could deadlock the process if it malloc()s while doing the arg formatting the interrupted thread was already holding a malloc() lock, or indeed if the stdio impl itself has internal locks which are held. So this data needs to be manually output using just write() Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
Re: [Qemu-devel] [PATCH] report that QEMU process was killed by a signal
On Mon, Mar 14, 2011 at 01:07:40PM +, Daniel P. Berrange wrote: On Mon, Mar 14, 2011 at 02:58:55PM +0200, Gleb Natapov wrote: Currently when rogue script kills QEMU process (using TERM/INT/HUP signal) it looks indistinguishable from system shutdown. Lets report that QMEU was killed and leave some clues about the killed identity. Good idea, but Signed-off-by: Gleb Natapov g...@redhat.com diff --git a/os-posix.c b/os-posix.c index 38c29d1..7e175e8 100644 --- a/os-posix.c +++ b/os-posix.c @@ -61,8 +61,9 @@ void os_setup_early_signal_handling(void) sigaction(SIGPIPE, act, NULL); } -static void termsig_handler(int signal) +static void termsig_handler(int signal, siginfo_t *info, void *c) { +fprintf(stderr, Got signal %d from pid %d\n, info-si_signo, info-si_pid); qemu_system_shutdown_request(); ...fprintf() isn't async signal safe. It could deadlock the process if it malloc()s while doing the arg formatting the interrupted thread was already holding a malloc() lock, or indeed if the stdio impl itself has internal locks which are held. So this data needs to be manually output using just write() Or we can save info-si_signo info-si_pid in global variables and print this when shutdown is performed by main loop. Will send updated patch. -- Gleb.
Re: [Qemu-devel] Re: Strategic decision: COW format
On 03/13/2011 09:28 PM, Chunqiang Tang wrote: In short, FVD's internal snapshot achieves the ideal properties of G1-G6, by 1) using the reference count table to only track static snapshots, 2) not keeping the reference count table in memory, 3) not updating the on-disk static reference count table when the VM runs, and 4) efficiently tracking dynamically allocated blocks by piggybacking on FVD's other features, i.e., its journal and small one-level lookup table. Are you assuming snapshots are read-only? It's not clear to me how this would work with writeable snapshots. It's not clear to me that writeable snapshots are really that important, but this is an advantage of having a refcount table. External snapshots are essentially read-only snapshots so I can understand the argument for it. By definition, a snapshot itself must be immutable (read-only), but a writeable image state can be derived from an immutable snapshot by using copy-on-write, which I guess is what you meant by writeable snapshot. No, because the copy-on-write is another layer on top of the snapshot and AFAICT, they don't persist when moving between snapshots. The equivalent for external snapshots would be: base0 - base1 - base2 - image And then if I wanted to move to base1 without destroying base2 and image, I could do: qemu-img create -f qcow2 -b base1 base1-overlay.img The file system can keep a lot of these things around pretty easily but with your proposal, it seems like there can only be one. If you support many of them, I think you'll degenerate to something as complex as a reference count table. On the other hand, I think it's reasonable to just avoid the CoW overlay entirely and say that moving to a previous snapshot destroys any of it's children. I think this ends up being a simplifying assumption that is worth investigating further. From the use-cases that I'm aware of (backup and RAS), I think these semantics are okay. I'm curious what other people think (Kevin/Stefan?). Regards, Anthony Liguori
[Qemu-devel] Re: [PATCH 1/5] Introduce -display argument
On 03/14/2011 07:28 AM, jes.soren...@redhat.com wrote: From: Jes Sorensenjes.soren...@redhat.com This patch introduces a -display argument which consolidates the setting of the display mode. Valid options are: sdl/curses/default/nographic Signed-off-by: Jes Sorensenjes.soren...@redhat.com I'd suggest using 'serial' instead of 'nographic' as that makes it more obvious to the user that they need to configure a serial session. I would drop 'default' for no other reason than we don't really expose default as a choice anywhere else and I'm struggling to justify it to myself. I think we also need to leave room to make -display take per-display specific parameters via QemuOpts. That said, converting -vnc to QemuOpts is hard so what I'd suggest doing is just cheating for the moment. Document the option to take per-display args after the main display, and then do something like: if (strstarts(optarg, vnc,, p)) { vnc_display_init(p); } We have a bunch of little stupid SDL options like -sdl-no-frame and it would be good to eventually be able to express that as -display sdl,frame=off Regards, Anthony Liguori
[Qemu-devel] [PATCHv2] report that QEMU process was killed by a signal
Currently when rogue script kills QEMU process (using TERM/INT/HUP signal) it looks indistinguishable from system shutdown. Lets report that QMEU was killed and leave some clues about the killed identity. Signed-off-by: Gleb Natapov g...@redhat.com --- v1-v2: - print message from a main loop instead of signal handler diff --git a/os-posix.c b/os-posix.c index 38c29d1..36d937c 100644 --- a/os-posix.c +++ b/os-posix.c @@ -61,9 +61,9 @@ void os_setup_early_signal_handling(void) sigaction(SIGPIPE, act, NULL); } -static void termsig_handler(int signal) +static void termsig_handler(int signal, siginfo_t *info, void *c) { -qemu_system_shutdown_request(); +qemu_system_killed(info-si_signo, info-si_pid); } static void sigchld_handler(int signal) @@ -76,7 +76,8 @@ void os_setup_signal_handling(void) struct sigaction act; memset(act, 0, sizeof(act)); -act.sa_handler = termsig_handler; +act.sa_sigaction = termsig_handler; +act.sa_flags = SA_SIGINFO; sigaction(SIGINT, act, NULL); sigaction(SIGHUP, act, NULL); sigaction(SIGTERM, act, NULL); diff --git a/sysemu.h b/sysemu.h index 0a83ab9..fc7048a 100644 --- a/sysemu.h +++ b/sysemu.h @@ -66,6 +66,8 @@ void qemu_system_vmstop_request(int reason); int qemu_shutdown_requested(void); int qemu_reset_requested(void); int qemu_powerdown_requested(void); +void qemu_system_killed(int signal, int pid); +void qemu_kill_report(void); extern qemu_irq qemu_system_powerdown; void qemu_system_reset(void); diff --git a/vl.c b/vl.c index 5e007a7..d14c42d 100644 --- a/vl.c +++ b/vl.c @@ -1213,7 +1213,7 @@ typedef struct QEMUResetEntry { static QTAILQ_HEAD(reset_handlers, QEMUResetEntry) reset_handlers = QTAILQ_HEAD_INITIALIZER(reset_handlers); static int reset_requested; -static int shutdown_requested; +static int shutdown_requested, shutdown_signal = -1, shutdown_pid; static int powerdown_requested; static int debug_requested; static int vmstop_requested; @@ -1225,6 +1225,15 @@ int qemu_shutdown_requested(void) return r; } +void qemu_kill_report(void) +{ +if (shutdown_signal != -1) { +fprintf(stderr, Got signal %d from pid %d\n, + shutdown_signal, shutdown_pid); +shutdown_signal = -1; +} +} + int qemu_reset_requested(void) { int r = reset_requested; @@ -1298,6 +1307,13 @@ void qemu_system_reset_request(void) qemu_notify_event(); } +void qemu_system_killed(int signal, int pid) +{ +shutdown_signal = signal; +shutdown_pid = pid; +qemu_system_shutdown_request(); +} + void qemu_system_shutdown_request(void) { shutdown_requested = 1; @@ -1441,6 +1457,7 @@ static void main_loop(void) vm_stop(VMSTOP_DEBUG); } if (qemu_shutdown_requested()) { +qemu_kill_report(); monitor_protocol_event(QEVENT_SHUTDOWN, NULL); if (no_shutdown) { vm_stop(VMSTOP_SHUTDOWN); -- Gleb.
Re: [Qemu-devel] Re: Strategic decision: COW format
No, because the copy-on-write is another layer on top of the snapshot and AFAICT, they don't persist when moving between snapshots. The equivalent for external snapshots would be: base0 - base1 - base2 - image And then if I wanted to move to base1 without destroying base2 and image, I could do: qemu-img create -f qcow2 -b base1 base1-overlay.img The file system can keep a lot of these things around pretty easily but with your proposal, it seems like there can only be one. If you support many of them, I think you'll degenerate to something as complex as a reference count table. On the other hand, I think it's reasonable to just avoid the CoW overlay entirely and say that moving to a previous snapshot destroys any of it's children. I think this ends up being a simplifying assumption that is worth investigating further. No, both VMware and FVD have the same semantics as QCOW2. Moving to a previous snapshot does not destroy any of its children. In the example I gave (copied below), it goes from Image: s1-s2-s3-s4-(current-state) back to snapshot s2, and now the state is Image: s1-s2-s3-s4 |-(curren-state) where all snapshots s1-s4 are kept. From there, it can take another snapshot s5, and then further go back to snapshot s4, ending up with Image: s1-s2-s3-s4 |-s5 | |- (current-state) FVD does have a reference count table like that in QCOW2, but it avoids the need for updating the reference count table during normal execution of the VM. The reference count table is only updated at the time of creating a snapshot or deleting a snapshot. Therefore, during normal execution of a VM, images with snapshots are as fast as images without snapshot. FVD can do this because of the following: FVD's reference count table only tracks the snapshots (s1, s2, ...), but does not track the current-state. Instead, FVD's default mechanism (one-level lookup table, journal, etc.), which exists even before introducing snapshot, already tracks the current-state. Working together, FVD's reference count table and its default mechanism tracks all the states. In QCOW2, when a new cluster is allocated during handling a running VM's write request, it updates both the lookup table and the reference count table, which is unnecessary because their information is redundant. By contrast, in FVD, when a new chunk is allocated during handling a running VM's write request, it only updates the lookup table without updating the reference count table, because by design the reference count table does not track the current-state and this chunk allocation operation belongs to the current-state. This is the key why FVD can get all the functions of QCOW2's internal snapshot but without its memory overhead to cache the reference count table and its disk I/O overhead to read or write the reference count table during normal execution of VM. Regards, ChunQiang (CQ) Tang, Ph.D. Homepage: http://www.research.ibm.com/people/c/ctang
Re: [Qemu-devel] [PATCH 1/7] usb-ccid: add CCID bus
On 02/23/11 12:20, Alon Levy wrote: diff --git a/configure b/configure index 791b71d..147aab3 100755 --- a/configure +++ b/configure @@ -174,6 +174,7 @@ trace_backend=nop trace_file=trace spice= rbd= +smartcard=yes IMHO smartcard support shouldn't be enabled per default. The userbase is limited. diff --git a/hw/ccid.h b/hw/ccid.h new file mode 100644 index 000..4350bc2 --- /dev/null +++ b/hw/ccid.h @@ -0,0 +1,54 @@ +/* + * CCID Passthru Card Device emulation + * + * Copyright (c) 2011 Red Hat. + * Written by Alon Levy. + * + * This code is licenced under the GNU LGPL, version 2 or later. + */ + [snip] + +/* callbacks to be used by the CCID device (hw/usb-ccid.c) to call + * into the smartcard device (hw/ccid-card-*.c) + */ This is inconsistent with the comment above. Normally multi-line comments in QEMU are like this: /* * foo * bar */ +struct CCIDCardInfo { +DeviceInfo qdev; +void (*print)(Monitor *mon, CCIDCardState *card, int indent); +const uint8_t *(*get_atr)(CCIDCardState *card, uint32_t *len); +void (*apdu_from_guest)(CCIDCardState *card, +const uint8_t *apdu, +uint32_t len); +int (*exitfn)(CCIDCardState *card); +int (*initfn)(CCIDCardState *card); +}; + +/* API for smartcard calling the CCID device (used by hw/ccid-card-*.c) + */ again here +void ccid_card_send_apdu_to_guest(CCIDCardState *card, + uint8_t *apdu, + uint32_t len); +void ccid_card_card_removed(CCIDCardState *card); +void ccid_card_card_inserted(CCIDCardState *card); +void ccid_card_card_error(CCIDCardState *card, uint64_t error); +void ccid_card_qdev_register(CCIDCardInfo *card); + +/* support guest visible insertion/removal of ccid devices based on actual + * devices connected/removed. Called by card implementation (passthru, local) */ and here diff --git a/hw/usb-ccid.c b/hw/usb-ccid.c new file mode 100644 index 000..bf4022a --- /dev/null +++ b/hw/usb-ccid.c +#define CCID_DEV_NAME usb-ccid + +/* The two options for variable sized buffers: + * make them constant size, for large enough constant, + * or handle the migration complexity - VMState doesn't handle this case. + * sizes are expected never to be exceeded, unless guest misbehaves. */ here again [snip] +/* Using Gemplus Vendor and Product id + Effect on various drivers: + * usbccid.sys (winxp, others untested) is a class driver so it doesn't care. + * linux has a number of class drivers, but openct filters based on +vendor/product (/etc/openct.conf under fedora), hence Gemplus. + */ Something went totally boink with the comments there! +/* 6.2.6 RDR_to_PC_SlotStatus definitions */ +enum { +CLOCK_STATUS_RUNNING = 0, +/* 0 - Clock Running, 1 - Clock stopped in State L, 2 - H, + 3 - unkonwn state. rest are RFU + */ +}; + +typedef struct __attribute__ ((__packed__)) CCID_Header { +uint8_t bMessageType; +uint32_tdwLength; +uint8_t bSlot; +uint8_t bSeq; +} CCID_Header; Is this header decided upon by the CCID spec or the code? It seems suboptimal to have a uint8 in front of a uint32 like that. Inefficient structure alignment :( + +/* 6.1.4 PC_to_RDR_XfrBlock */ +typedef struct __attribute__ ((__packed__)) CCID_XferBlock { +CCID_Header hdr; +uint8_t bBWI; /* Block Waiting Timeout */ +uint16_t wLevelParameter; /* XXX currently unused */ +uint8_t abData[0]; +} CCID_XferBlock; + +typedef struct __attribute__ ((__packed__)) CCID_IccPowerOn { +CCID_Header hdr; +uint8_t bPowerSelect; +uint16_tabRFU; +} CCID_IccPowerOn; Same problem with the above two structs +typedef struct __attribute__ ((__packed__)) CCID_IccPowerOff { +CCID_Header hdr; +uint16_tabRFU; +} CCID_IccPowerOff; + +typedef struct __attribute__ ((__packed__)) CCID_SetParameters { +CCID_Header hdr; +uint8_t bProtocolNum; +uint16_t abRFU; +uint8_tabProtocolDataStructure[0]; +} CCID_SetParameters; and again. +/** + * powered - defaults to true, changed by PowerOn/PowerOff messages + */ +struct USBCCIDState { +USBDevice dev; +CCIDBus *bus; +CCIDCardState *card; +CCIDCardInfo *cardinfo; /* caching the info pointer */ +uint8_t debug; +BulkIn bulk_in_pending[BULK_IN_PENDING_NUM]; /* circular */ +uint32_t bulk_in_pending_start; +uint32_t bulk_in_pending_end; /* first free */ +uint32_t bulk_in_pending_num; +BulkIn *current_bulk_in; +uint8_t bulk_out_data[BULK_OUT_DATA_SIZE]; +uint32_t bulk_out_pos; +uint8_t bmSlotICCState; +uint8_t powered; +uint8_t notify_slot_change; +uint64_t last_answer_error; +Answer pending_answers[PENDING_ANSWERS_NUM]; +uint32_t pending_answers_start; +uint32_t pending_answers_end;
Re: [Qemu-devel] [PATCH 2/7] introduce libcacard/vscard_common.h
On 02/23/11 12:20, Alon Levy wrote: diff --git a/libcacard/vscard_common.h b/libcacard/vscard_common.h new file mode 100644 index 000..7449314 --- /dev/null +++ b/libcacard/vscard_common.h @@ -0,0 +1,167 @@ +/* Virtual Smart Card protocol definition + * + * This protocol is between a host using virtual smart card readers, + * and a client providing the smart cards, perhaps by emulating them or by + * access to real cards. + * + * Definitions for this protocol: + * Host - user of the card + * Client - owner of the card + * + * The current implementation passes the raw APDU's from 7816 and additionally + * contains messages to setup and teardown readers, handle insertion and + * removal of cards, negotiate the protocol via capabilities and provide + * for error responses. + * + * Copyright (c) 2011 Red Hat. + * + * This code is licensed under the LGPL. Please be more specific on the license. Other code has the following: * This work is licensed under the terms of the GNU LGPL, version 2.1 or later. * See the COPYING.LIB file in the top-level directory. [snip] +/* VSCMsgInit Client - Host + * Client sends it on connection, with its own capabilities. + * Host replies with VSCMsgInit filling in its capabilities. + * + * It is not meant to be used for negotiation, i.e. sending more then + * once from any side, but could be used for that in the future. + * */ Looks like some automatic mangling of comments - happens in more than one place. Except for the cosmetic stuff, looks ok. Jes
Re: [Qemu-devel] [PATCH 3/7] ccid: add passthru card device
On 02/23/11 12:20, Alon Levy wrote: The passthru ccid card is a device sitting on the usb-ccid bus and using a chardevice to communicate with a remote device using the VSCard protocol defined in libcacard/vscard_common.h Usage docs available in following patch in docs/ccid.txt Signed-off-by: Alon Levy al...@redhat.com Except for the usual formatting bits, this one looks fine to me. Cheers, Jes
Re: [Qemu-devel] Re: Strategic decision: COW format
On 03/14/2011 08:53 AM, Chunqiang Tang wrote: No, because the copy-on-write is another layer on top of the snapshot and AFAICT, they don't persist when moving between snapshots. The equivalent for external snapshots would be: base0- base1- base2- image And then if I wanted to move to base1 without destroying base2 and image, I could do: qemu-img create -f qcow2 -b base1 base1-overlay.img The file system can keep a lot of these things around pretty easily but with your proposal, it seems like there can only be one. If you support many of them, I think you'll degenerate to something as complex as a reference count table. On the other hand, I think it's reasonable to just avoid the CoW overlay entirely and say that moving to a previous snapshot destroys any of it's children. I think this ends up being a simplifying assumption that is worth investigating further. No, both VMware and FVD have the same semantics as QCOW2. Moving to a previous snapshot does not destroy any of its children. In the example I gave (copied below), it goes from Image: s1-s2-s3-s4-(current-state) back to snapshot s2, and now the state is Image: s1-s2-s3-s4 |-(curren-state) where all snapshots s1-s4 are kept. From there, it can take another snapshot s5, and then further go back to snapshot s4, ending up with Image: s1-s2-s3-s4 |-s5 | |- (current-state) Your use of current-state is confusing me because AFAICT, current-state is just semantically another snapshot. It's writable because it has no children. You only keep around one writable snapshot and to make another snapshot writable, you have to discard the former. This is not the semantics of qcow2. Every time you create a snapshot, it's essentially a new image. You can write directly to it. While we don't do this today and I don't think we ever should, it's entirely possible to have two disks served simultaneously out of the same qcow2 file using snapshots. Regards, Anthony Liguori
Re: [Qemu-devel] [PATCH 1/7] usb-ccid: add CCID bus
On Mon, Mar 14, 2011 at 02:54:59PM +0100, Jes Sorensen wrote: On 02/23/11 12:20, Alon Levy wrote: diff --git a/configure b/configure index 791b71d..147aab3 100755 --- a/configure +++ b/configure @@ -174,6 +174,7 @@ trace_backend=nop trace_file=trace spice= rbd= +smartcard=yes IMHO smartcard support shouldn't be enabled per default. The userbase is limited. Deciding based on importance/size of userbase is rather subjective. IMHO all features should be enabled by default, but if they depend on a 3rd party library that isn't installed on the build host, they should automatically disable themselves. eg * [the default] - on, if external library is present, off otherwise * --enable-feature - always on, raise error if external library is missing * --disable-feature - always off, Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
[Qemu-devel] [PATCH 1/3] microblaze: Compile uart 16550 serial driver
From: Michal Simek mon...@monstr.eu Upcomming little endian platform will use 16550 serial driver. Signed-off-by: Michal Simek mon...@monstr.eu Signed-off-by: Edgar E. Iglesias edgar.igles...@petalogix.com --- default-configs/microblaze-softmmu.mak |1 + default-configs/microblazeel-softmmu.mak |1 + 2 files changed, 2 insertions(+), 0 deletions(-) diff --git a/default-configs/microblaze-softmmu.mak b/default-configs/microblaze-softmmu.mak index 4399b8b..613edab 100644 --- a/default-configs/microblaze-softmmu.mak +++ b/default-configs/microblaze-softmmu.mak @@ -2,3 +2,4 @@ CONFIG_PTIMER=y CONFIG_PFLASH_CFI01=y +CONFIG_SERIAL=y diff --git a/default-configs/microblazeel-softmmu.mak b/default-configs/microblazeel-softmmu.mak index ddc6bf4..4b40fb2 100644 --- a/default-configs/microblazeel-softmmu.mak +++ b/default-configs/microblazeel-softmmu.mak @@ -2,3 +2,4 @@ CONFIG_PTIMER=y CONFIG_PFLASH_CFI01=y +CONFIG_SERIAL=y -- 1.7.3.4
[Qemu-devel] [PATCH 0/3] microblaze: Add petalogix-ml605 machine
From: Edgar E. Iglesias edgar.igles...@petalogix.com Adds model of the petalogix-ml605 reference design. Normally this is a little endian machine allthough qemu supports it in both modes. Edgar E. Iglesias (1): xilinx: Add AXIENET DMA models Michal Simek (2): microblaze: Compile uart 16550 serial driver microblaze: Add PetaLogix ml605 MMU little-endian ref design Makefile |2 +- Makefile.target |3 + default-configs/microblaze-softmmu.mak |1 + default-configs/microblazeel-softmmu.mak |1 + hw/petalogix_ml605_mmu.c | 265 + hw/xilinx.h | 39 ++ hw/xilinx_axidma.c | 463 hw/xilinx_axidma.h | 40 ++ hw/xilinx_axienet.c | 857 ++ pc-bios/petalogix-ml605.dtb | Bin 0 - 9982 bytes 10 files changed, 1670 insertions(+), 1 deletions(-) create mode 100644 hw/petalogix_ml605_mmu.c create mode 100644 hw/xilinx_axidma.c create mode 100644 hw/xilinx_axidma.h create mode 100644 hw/xilinx_axienet.c create mode 100644 pc-bios/petalogix-ml605.dtb -- 1.7.3.4
Re: [Qemu-devel] [PATCH 1/7] usb-ccid: add CCID bus
On 03/14/2011 09:07 AM, Daniel P. Berrange wrote: On Mon, Mar 14, 2011 at 02:54:59PM +0100, Jes Sorensen wrote: On 02/23/11 12:20, Alon Levy wrote: diff --git a/configure b/configure index 791b71d..147aab3 100755 --- a/configure +++ b/configure @@ -174,6 +174,7 @@ trace_backend=nop trace_file=trace spice= rbd= +smartcard=yes IMHO smartcard support shouldn't be enabled per default. The userbase is limited. Deciding based on importance/size of userbase is rather subjective. IMHO all features should be enabled by default, but if they depend on a 3rd party library that isn't installed on the build host, they should automatically disable themselves. eg Yes, and this is the policy we've taken pretty consistently in QEMU. Features that aren't enabled by default are subject to bit-rot. Regards, Anthony Liguori * [the default] - on, if external library is present, off otherwise * --enable-feature - always on, raise error if external library is missing * --disable-feature - always off, Regards, Daniel
[Qemu-devel] [PATCH 3/3] microblaze: Add PetaLogix ml605 MMU little-endian ref design
From: Michal Simek mon...@monstr.eu Add the first Microblaze little endian platform. Platform uses uart16550, axi ethernet, timer, intc. Signed-off-by: Michal Simek mon...@monstr.eu Signed-off-by: Edgar E. Iglesias edgar.igles...@petalogix.com --- Makefile|2 +- Makefile.target |1 + hw/petalogix_ml605_mmu.c| 265 +++ hw/xilinx.h | 39 +++ pc-bios/petalogix-ml605.dtb | Bin 0 - 9982 bytes 5 files changed, 306 insertions(+), 1 deletions(-) create mode 100644 hw/petalogix_ml605_mmu.c create mode 100644 pc-bios/petalogix-ml605.dtb diff --git a/Makefile b/Makefile index eca4c76..89e88b4 100644 --- a/Makefile +++ b/Makefile @@ -211,7 +211,7 @@ gpxe-eepro100-80861209.rom \ pxe-e1000.bin \ pxe-ne2k_pci.bin pxe-pcnet.bin \ pxe-rtl8139.bin pxe-virtio.bin \ -bamboo.dtb petalogix-s3adsp1800.dtb \ +bamboo.dtb petalogix-s3adsp1800.dtb petalogix-ml605.dtb \ multiboot.bin linuxboot.bin \ s390-zipl.rom else diff --git a/Makefile.target b/Makefile.target index d11eb4f..62b102a 100644 --- a/Makefile.target +++ b/Makefile.target @@ -266,6 +266,7 @@ obj-mips-y += cirrus_vga.o obj-mips-$(CONFIG_FULONG) += bonito.o vt82c686.o mips_fulong2e.o obj-microblaze-y = petalogix_s3adsp1800_mmu.o +obj-microblaze-y += petalogix_ml605_mmu.o obj-microblaze-y += microblaze_pic_cpu.o obj-microblaze-y += xilinx_intc.o diff --git a/hw/petalogix_ml605_mmu.c b/hw/petalogix_ml605_mmu.c new file mode 100644 index 000..ef24e35 --- /dev/null +++ b/hw/petalogix_ml605_mmu.c @@ -0,0 +1,265 @@ +/* + * Model of Petalogix linux reference design targeting Xilinx Spartan ml605 + * board. + * + * Copyright (c) 2011 Michal Simek mon...@monstr.eu + * Copyright (c) 2011 PetaLogix + * Copyright (c) 2009 Edgar E. Iglesias. + * + * 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 sysbus.h +#include hw.h +#include net.h +#include flash.h +#include sysemu.h +#include devices.h +#include boards.h +#include device_tree.h +#include xilinx.h +#include loader.h +#include elf.h +#include blockdev.h +#include pc.h + +#include xilinx_axidma.h + +#define LMB_BRAM_SIZE (128 * 1024) +#define FLASH_SIZE (32 * 1024 * 1024) + +static struct +{ +uint32_t bootstrap_pc; +uint32_t cmdline; +uint32_t fdt; +} boot_info; + +static void main_cpu_reset(void *opaque) +{ +CPUState *env = opaque; + +cpu_reset(env); +env-regs[5] = boot_info.cmdline; +env-regs[7] = boot_info.fdt; +env-sregs[SR_PC] = boot_info.bootstrap_pc; +env-pvr.regs[10] = 0x0e00; /* virtex 6 */ +/* setup pvr to match kernel setting */ +env-pvr.regs[5] |= PVR5_DCACHE_WRITEBACK_MASK; +env-pvr.regs[0] |= PVR0_USE_FPU_MASK | PVR0_ENDI; +env-pvr.regs[0] = (env-pvr.regs[0] ~PVR0_VERSION_MASK) | (0x14 8); +env-pvr.regs[2] ^= PVR2_USE_FPU2_MASK; +env-pvr.regs[4] = 0xc56b8000; +env-pvr.regs[5] = 0xc56be000; +} + +#define BINARY_DEVICE_TREE_FILE petalogix-ml605.dtb +static int petalogix_load_device_tree(target_phys_addr_t addr, + uint32_t ramsize, + target_phys_addr_t initrd_base, + target_phys_addr_t initrd_size, + const char *kernel_cmdline) +{ +char *path; +int fdt_size; +#ifdef CONFIG_FDT +void *fdt; +int r; + +/* Try the local mb.dtb override. */ +fdt = load_device_tree(mb.dtb, fdt_size); +if (!fdt) { +path = qemu_find_file(QEMU_FILE_TYPE_BIOS, BINARY_DEVICE_TREE_FILE); +if (path) { +fdt = load_device_tree(path, fdt_size); +qemu_free(path); +} +if (!fdt) +return 0; +} + +r = qemu_devtree_setprop_string(fdt, /chosen, bootargs, kernel_cmdline); +if (r 0) +fprintf(stderr, couldn't set /chosen/bootargs\n); +cpu_physical_memory_write
[Qemu-devel] CPU type qemu64 breaks guest code
Hi guys, While I was off on vacation a pretty nasty bug emerged. It's our old friend the non-existent -cpu qemu64 CPU type. To refresh your memories, this is the definition of the default 64-bit CPU type in Qemu: { .name = qemu64, .level = 4, .vendor1 = CPUID_VENDOR_AMD_1, .vendor2 = CPUID_VENDOR_AMD_2, .vendor3 = CPUID_VENDOR_AMD_3, .family = 6, .model = 2, .stepping = 3, .features = PPRO_FEATURES | CPUID_MTRR | CPUID_CLFLUSH | CPUID_MCA | CPUID_PSE36, .ext_features = CPUID_EXT_SSE3 | CPUID_EXT_CX16 | CPUID_EXT_POPCNT, .ext2_features = (PPRO_FEATURES EXT2_FEATURE_MASK) | CPUID_EXT2_LM | CPUID_EXT2_SYSCALL | CPUID_EXT2_NX, .ext3_features = CPUID_EXT3_LAHF_LM | CPUID_EXT3_SVM | CPUID_EXT3_ABM | CPUID_EXT3_SSE4A, .xlevel = 0x800A, .model_id = QEMU Virtual CPU version QEMU_VERSION, }, Before I left, we already had weird build breakages where gcc simply abort()ed when running inside of KVM. This breakage has been tracked down to libgmp, which has this code (http://gmplib.org:8000/gmp-5.0/file/1ebe39104437/mpn/x86_64/fat/fat.c): if (strcmp (vendor_string, GenuineIntel) == 0) { } else if (strcmp (vendor_string, AuthenticAMD) == 0) { switch (family) { case 5: case 6: abort (); break; case 15: /* CPUVEC_SETUP_athlon */ break; } The reasoning behind it is that for AMD, all 64-bit CPUs were family 15. This breakage is obviously pretty bad for guests running qemu and shows once again that deriving from real hardware is a bad idea. I guess the best way to move forward is to change the CPU type to something that actually exists in the real world - even if we have to strip off some features. Any ideas? Comments? Alex
Re: [Qemu-devel] Re: Strategic decision: COW format
Am 14.03.2011 14:22, schrieb Anthony Liguori: On 03/13/2011 09:28 PM, Chunqiang Tang wrote: In short, FVD's internal snapshot achieves the ideal properties of G1-G6, by 1) using the reference count table to only track static snapshots, 2) not keeping the reference count table in memory, 3) not updating the on-disk static reference count table when the VM runs, and 4) efficiently tracking dynamically allocated blocks by piggybacking on FVD's other features, i.e., its journal and small one-level lookup table. Are you assuming snapshots are read-only? It's not clear to me how this would work with writeable snapshots. It's not clear to me that writeable snapshots are really that important, but this is an advantage of having a refcount table. External snapshots are essentially read-only snapshots so I can understand the argument for it. By definition, a snapshot itself must be immutable (read-only), but a writeable image state can be derived from an immutable snapshot by using copy-on-write, which I guess is what you meant by writeable snapshot. No, because the copy-on-write is another layer on top of the snapshot and AFAICT, they don't persist when moving between snapshots. The equivalent for external snapshots would be: base0 - base1 - base2 - image And then if I wanted to move to base1 without destroying base2 and image, I could do: qemu-img create -f qcow2 -b base1 base1-overlay.img The file system can keep a lot of these things around pretty easily but with your proposal, it seems like there can only be one. If you support many of them, I think you'll degenerate to something as complex as a reference count table. IIUC, he already uses a refcount table. Actually, I think that a refcount table is a requirement to provide the interesting properties that internal snapshots have (see my other mail). Refcount tables aren't a very complex thing either. In fact, it makes a format much simpler to have one concept like refcount tables instead of adding another different mechanism for each new feature that would be natural with refcount tables. The only problem with them is that they are metadata that must be updated. However, I think we have discussed enough how to avoid the greatest part of that cost. On the other hand, I think it's reasonable to just avoid the CoW overlay entirely and say that moving to a previous snapshot destroys any of it's children. I think this ends up being a simplifying assumption that is worth investigating further. From the use-cases that I'm aware of (backup and RAS), I think these semantics are okay. I don't think this semantics would be expected. Any anyway, would this really allow simplification of the format? I'm afraid that you would go for complicated solutions with odd semantics just because of an arbitrary dislike of refcounts. Kevin
[Qemu-devel] [PATCH 2/3] xilinx: Add AXIENET DMA models
From: Edgar E. Iglesias edgar.igles...@petalogix.com Signed-off-by: Edgar E. Iglesias edgar.igles...@petalogix.com --- Makefile.target |2 + hw/xilinx_axidma.c | 463 +++ hw/xilinx_axidma.h | 40 +++ hw/xilinx_axienet.c | 857 +++ 4 files changed, 1362 insertions(+), 0 deletions(-) create mode 100644 hw/xilinx_axidma.c create mode 100644 hw/xilinx_axidma.h create mode 100644 hw/xilinx_axienet.c diff --git a/Makefile.target b/Makefile.target index f0df98e..d11eb4f 100644 --- a/Makefile.target +++ b/Makefile.target @@ -272,6 +272,8 @@ obj-microblaze-y += xilinx_intc.o obj-microblaze-y += xilinx_timer.o obj-microblaze-y += xilinx_uartlite.o obj-microblaze-y += xilinx_ethlite.o +obj-microblaze-y += xilinx_axidma.o +obj-microblaze-y += xilinx_axienet.o obj-microblaze-$(CONFIG_FDT) += device_tree.o diff --git a/hw/xilinx_axidma.c b/hw/xilinx_axidma.c new file mode 100644 index 000..ca56625 --- /dev/null +++ b/hw/xilinx_axidma.c @@ -0,0 +1,463 @@ +/* + * QEMU model of Xilinx AXI-DMA block. + * + * Copyright (c) 2011 Edgar E. Iglesias. + * + * 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 sysbus.h +#include qemu-char.h +#include qemu-timer.h +#include qemu-log.h +#include qdev-addr.h + +#include xilinx_axidma.h + +#define D(x) + +#define R_DMACR (0x00 / 4) +#define R_DMASR (0x04 / 4) +#define R_CURDESC (0x08 / 4) +#define R_TAILDESC (0x10 / 4) +#define R_MAX (0x44 / 4) + +struct sdesc +{ +uint64_t nxtdesc; +uint64_t buffer_address; +uint64_t reserved; +uint32_t control; +uint32_t status; +uint32_t app[6]; +}; + +struct axi_stream +{ +QEMUBH *bh; +ptimer_state *ptimer; +qemu_irq irq; + +int nr; + +struct sdesc desc; +int pos; +unsigned int complete_cnt; +uint32_t regs[R_MAX]; +}; + +struct xlx_axidma +{ +SysBusDevice busdev; +uint32_t freqhz; +void *dmach; + +struct axi_stream streams[2]; +}; + +/* + * Helper calls to extract info from desriptors and other trivial + * state from regs. + */ +static inline int stream_desc_sof(struct sdesc *d) +{ +return d-control (1 27); +} + +static inline int stream_desc_eof(struct sdesc *d) +{ +return d-control (1 26); +} + +static inline int stream_resetting(struct axi_stream *s) +{ +return !!(s-regs[R_DMACR] (1 2)); +} + +static inline int stream_running(struct axi_stream *s) +{ +return s-regs[R_DMACR] 1; +} + +static inline int stream_halted(struct axi_stream *s) +{ +return s-regs[R_DMASR] 1; +} + +static inline int stream_idle(struct axi_stream *s) +{ +return !!(s-regs[R_DMASR] 2); +} + +static void stream_reset(struct axi_stream *s) +{ +s-regs[R_DMASR] = 1; /* starts up halted. */ +s-regs[R_DMACR] = 1 16; /* Starts with one in compl threshhold. */ +} + +/* Mapp an offset addr into a channel index. */ +static inline int streamid_from_addr(target_phys_addr_t addr) +{ +int sid; + +sid = addr / (0x30); +sid = 1; +return sid; +} + +#if 0 +static void stream_desc_show(struct sdesc *d) +{ +qemu_log(buffer_addr = %lx\n, d-buffer_address); +qemu_log(nxtdesc = %lx\n, d-nxtdesc); +qemu_log(control = %x\n, d-control); +qemu_log(status = %x\n, d-control); +} +#endif + +static void stream_desc_load(struct axi_stream *s, target_phys_addr_t addr) +{ +cpu_physical_memory_read(addr, (void *) s-desc, sizeof s-desc); +} + +static void stream_desc_store(struct axi_stream *s, target_phys_addr_t addr) +{ +cpu_physical_memory_write(addr, (void *) s-desc, sizeof s-desc); +} + +static void stream_update_irq(struct axi_stream *s) +{ +unsigned int pending, mask, irq; + +pending = (s-regs[R_DMASR] 12) 0x7; +mask = (s-regs[R_DMACR] 12) 0x7; + +irq = pending mask; + +qemu_set_irq(s-irq, !!irq); +} + +static void
Re: [Qemu-devel] Re: Strategic decision: COW format
IIUC, he already uses a refcount table. Actually, I think that a refcount table is a requirement to provide the interesting properties that internal snapshots have (see my other mail). Refcount tables aren't a very complex thing either. In fact, it makes a format much simpler to have one concept like refcount tables instead of adding another different mechanism for each new feature that would be natural with refcount tables. The only problem with them is that they are metadata that must be updated. However, I think we have discussed enough how to avoid the greatest part of that cost. FVD's novel uses of the reference count table reduces the metadata update overhead down to literally zero during normal execution of a VM. This gets the bests of QCOW2's reference count table but without its oeverhead. In FVD, the reference count table is only updated when creating a new snapshot or deleting an existing snapshot. The reference count table is never updated during normal execution of a VM.
Re: [Qemu-devel] Re: Strategic decision: COW format
Am 14.03.2011 15:02, schrieb Anthony Liguori: On 03/14/2011 08:53 AM, Chunqiang Tang wrote: No, because the copy-on-write is another layer on top of the snapshot and AFAICT, they don't persist when moving between snapshots. The equivalent for external snapshots would be: base0- base1- base2- image And then if I wanted to move to base1 without destroying base2 and image, I could do: qemu-img create -f qcow2 -b base1 base1-overlay.img The file system can keep a lot of these things around pretty easily but with your proposal, it seems like there can only be one. If you support many of them, I think you'll degenerate to something as complex as a reference count table. On the other hand, I think it's reasonable to just avoid the CoW overlay entirely and say that moving to a previous snapshot destroys any of it's children. I think this ends up being a simplifying assumption that is worth investigating further. No, both VMware and FVD have the same semantics as QCOW2. Moving to a previous snapshot does not destroy any of its children. In the example I gave (copied below), it goes from Image: s1-s2-s3-s4-(current-state) back to snapshot s2, and now the state is Image: s1-s2-s3-s4 |-(curren-state) where all snapshots s1-s4 are kept. From there, it can take another snapshot s5, and then further go back to snapshot s4, ending up with Image: s1-s2-s3-s4 |-s5 | |- (current-state) Your use of current-state is confusing me because AFAICT, current-state is just semantically another snapshot. It's writable because it has no children. You only keep around one writable snapshot and to make another snapshot writable, you have to discard the former. This is not the semantics of qcow2. Every time you create a snapshot, it's essentially a new image. You can write directly to it. While we don't do this today and I don't think we ever should, it's entirely possible to have two disks served simultaneously out of the same qcow2 file using snapshots. No, CQ is describing the semantics of internal snapshots in qcow2 correctly. You have all the snapshots that are stored in the snapshot table (all read-only) plus one current state described by the image header (read-write). Kevin
Re: [Qemu-devel] Re: Strategic decision: COW format
On Mon, Mar 14, 2011 at 1:53 PM, Chunqiang Tang ct...@us.ibm.com wrote: Therefore, during normal execution of a VM, images with snapshots are as fast as images without snapshot. Hang on, an image with a snapshot still needs to do copy-on-write, just like backing files. The cost of copy-on-write is reading data from the backing file, whereas a non-CoW write doesn't need to do that. So no, snapshots are not free during normal execution. Stefan
[Qemu-devel] Re: RFC: emulation of system flash
On Fri, Mar 11, 2011 at 01:41:17PM -0800, Jordan Justen wrote: On Thu, Mar 10, 2011 at 12:21, Gleb Natapov g...@redhat.com wrote: On Thu, Mar 10, 2011 at 11:50:42AM -0800, Jordan Justen wrote: So, perhaps this feature should build upon the other feature you and Jan are discussing. When will it become available? When somebody will be motivated enough to send patches. If this was the only thing that stands in a way of QEMU having EFI firmware with CSM support it would have happened much faster though. Regarding the read-IO, write-IO, read-RAM, write-RAM slots feature for kvm... Unfortunately, this is not much related to the CSM issue. It sounds to me like this feature would be important for a good implementation of flash support that covers both qemu kvm. How big is this task? How can I find out more to see if I can help? Having BIOS (or executable part of a flash) to be mapped as memory and rest of the flash to be IO does not require any special changes to KVM or QEMU. This is just a matter of implementing command line parsing and flash device emulation. -- Gleb.
Re: [Qemu-devel] Re: Strategic decision: COW format
Your use of current-state is confusing me because AFAICT, current-state is just semantically another snapshot. It's writable because it has no children. You only keep around one writable snapshot and to make another snapshot writable, you have to discard the former. This is not the semantics of qcow2. Every time you create a snapshot, it's essentially a new image. You can write directly to it. While we don't do this today and I don't think we ever should, it's entirely possible to have two disks served simultaneously out of the same qcow2 file using snapshots. No, CQ is describing the semantics of internal snapshots in qcow2 correctly. You have all the snapshots that are stored in the snapshot table (all read-only) plus one current state described by the image header (read-write). That's also the semantics of VMware's external snapshot. So there is no difference in semantics. It is just a difference in implementation and performance. Regards, ChunQiang (CQ) Tang Homepage: http://www.research.ibm.com/people/c/ctang
Re: [Qemu-devel] Re: Strategic decision: COW format
Am 14.03.2011 15:25, schrieb Chunqiang Tang: IIUC, he already uses a refcount table. Actually, I think that a refcount table is a requirement to provide the interesting properties that internal snapshots have (see my other mail). Refcount tables aren't a very complex thing either. In fact, it makes a format much simpler to have one concept like refcount tables instead of adding another different mechanism for each new feature that would be natural with refcount tables. The only problem with them is that they are metadata that must be updated. However, I think we have discussed enough how to avoid the greatest part of that cost. FVD's novel uses of the reference count table reduces the metadata update overhead down to literally zero during normal execution of a VM. This gets the bests of QCOW2's reference count table but without its oeverhead. In FVD, the reference count table is only updated when creating a new snapshot or deleting an existing snapshot. The reference count table is never updated during normal execution of a VM. Yeah, I think that's basically an interesting property. However, I don't think that it makes a big difference compared to qcow2's refcount table when you use a writeback metadata cache. What about the question that I had in my other mail? (How do you determine if a cluster is free without scanning the whole lookup table?) I think this might be the missing piece for me to understand how your approach works. Kevin
Re: [Qemu-devel] Re: Strategic decision: COW format
On 03/14/2011 09:15 AM, Kevin Wolf wrote: The file system can keep a lot of these things around pretty easily but with your proposal, it seems like there can only be one. If you support many of them, I think you'll degenerate to something as complex as a reference count table. IIUC, he already uses a refcount table. Well, he needs a separate mechanism to make trim/discard work, but for the snapshot discussion, a reference count table is avoided. The bitmap only covers whether the guest has accessed a block or not. Then there is a separate table that maps guest offsets to offsets within the file. I haven't thought hard about it, but my guess is that there is an ordering constraint between these two pieces of metadata which is why the journal is necessary. I get worried about the complexity of a journal even more than a reference count table. Actually, I think that a refcount table is a requirement to provide the interesting properties that internal snapshots have (see my other mail). Well the trick here AFAICT is that you're basically storing external snapshots internally. So it's sort of like a bunch of FVD formats embedded into a single image. Refcount tables aren't a very complex thing either. In fact, it makes a format much simpler to have one concept like refcount tables instead of adding another different mechanism for each new feature that would be natural with refcount tables. I think it's a reasonable design goal to minimize any metadata updates in the fast path. If we can write 1 piece of metadata verses writing 2, then it's worth exploring IMHO. The only problem with them is that they are metadata that must be updated. However, I think we have discussed enough how to avoid the greatest part of that cost. Maybe I missed it, but in the WCE=0 mode, is it really possible to avoid the writes for the refcount table? On the other hand, I think it's reasonable to just avoid the CoW overlay entirely and say that moving to a previous snapshot destroys any of it's children. I think this ends up being a simplifying assumption that is worth investigating further. From the use-cases that I'm aware of (backup and RAS), I think these semantics are okay. I don't think this semantics would be expected. Any anyway, would this really allow simplification of the format? I don't know, I'm really just trying to separate out the implementation of the format to the use-cases we're trying to address. Even if we're talking about qcow3, then if we only really care about read-only snapshots, perhaps we can add a feature bit for this and take advantage of this to make the WCE=0 case much faster. But the fundamental question is, does this satisfy the use-cases we care about? Regards, Anthony Liguori I'm afraid that you would go for complicated solutions with odd semantics just because of an arbitrary dislike of refcounts. Kevin
Re: [Qemu-devel] Re: Strategic decision: COW format
On Mon, Mar 14, 2011 at 1:53 PM, Chunqiang Tang ct...@us.ibm.com wrote: Therefore, during normal execution of a VM, images with snapshots are as fast as images without snapshot. Hang on, an image with a snapshot still needs to do copy-on-write, just like backing files. The cost of copy-on-write is reading data from the backing file, whereas a non-CoW write doesn't need to do that. So no, snapshots are not free during normal execution. You are right. For any implementation of snapshot (internal or external), this CoW overhead is unavoidable. What I meant to say was that, other than this mandatory CoW overhead, FVD's internal snapshot does not incur any additional metadata update overhead (unlike that in QCOW2).
Re: [Qemu-devel] Re: Strategic decision: COW format
On 03/14/2011 09:21 AM, Kevin Wolf wrote: Am 14.03.2011 15:02, schrieb Anthony Liguori: On 03/14/2011 08:53 AM, Chunqiang Tang wrote: No, because the copy-on-write is another layer on top of the snapshot and AFAICT, they don't persist when moving between snapshots. The equivalent for external snapshots would be: base0- base1- base2- image And then if I wanted to move to base1 without destroying base2 and image, I could do: qemu-img create -f qcow2 -b base1 base1-overlay.img The file system can keep a lot of these things around pretty easily but with your proposal, it seems like there can only be one. If you support many of them, I think you'll degenerate to something as complex as a reference count table. On the other hand, I think it's reasonable to just avoid the CoW overlay entirely and say that moving to a previous snapshot destroys any of it's children. I think this ends up being a simplifying assumption that is worth investigating further. No, both VMware and FVD have the same semantics as QCOW2. Moving to a previous snapshot does not destroy any of its children. In the example I gave (copied below), it goes from Image: s1-s2-s3-s4-(current-state) back to snapshot s2, and now the state is Image: s1-s2-s3-s4 |-(curren-state) where all snapshots s1-s4 are kept. From there, it can take another snapshot s5, and then further go back to snapshot s4, ending up with Image: s1-s2-s3-s4 |-s5 | |- (current-state) Your use of current-state is confusing me because AFAICT, current-state is just semantically another snapshot. It's writable because it has no children. You only keep around one writable snapshot and to make another snapshot writable, you have to discard the former. This is not the semantics of qcow2. Every time you create a snapshot, it's essentially a new image. You can write directly to it. While we don't do this today and I don't think we ever should, it's entirely possible to have two disks served simultaneously out of the same qcow2 file using snapshots. No, CQ is describing the semantics of internal snapshots in qcow2 correctly. You have all the snapshots that are stored in the snapshot table (all read-only) plus one current state described by the image header (read-write). But is there any problem (in the format) with writing to the non-current state? I can't think of one. Regards, Anthony Liguori Kevin
Re: [Qemu-devel] Re: Strategic decision: COW format
On Mon, Mar 14, 2011 at 2:25 PM, Chunqiang Tang ct...@us.ibm.com wrote: IIUC, he already uses a refcount table. Actually, I think that a refcount table is a requirement to provide the interesting properties that internal snapshots have (see my other mail). Refcount tables aren't a very complex thing either. In fact, it makes a format much simpler to have one concept like refcount tables instead of adding another different mechanism for each new feature that would be natural with refcount tables. The only problem with them is that they are metadata that must be updated. However, I think we have discussed enough how to avoid the greatest part of that cost. FVD's novel uses of the reference count table reduces the metadata update overhead down to literally zero during normal execution of a VM. This gets the bests of QCOW2's reference count table but without its oeverhead. In FVD, the reference count table is only updated when creating a new snapshot or deleting an existing snapshot. The reference count table is never updated during normal execution of a VM. Do you want to send out a break-down of the steps (and cost) involved in doing: 1. Snapshot creation. 2. Snapshot deletion. 3. Opening an image with n snapshots. Stefan
Re: [Qemu-devel] [PATCH 2/7] introduce libcacard/vscard_common.h
On Mon, Mar 14, 2011 at 03:01:19PM +0100, Jes Sorensen wrote: On 02/23/11 12:20, Alon Levy wrote: diff --git a/libcacard/vscard_common.h b/libcacard/vscard_common.h new file mode 100644 index 000..7449314 --- /dev/null +++ b/libcacard/vscard_common.h @@ -0,0 +1,167 @@ +/* Virtual Smart Card protocol definition + * + * This protocol is between a host using virtual smart card readers, + * and a client providing the smart cards, perhaps by emulating them or by + * access to real cards. + * + * Definitions for this protocol: + * Host - user of the card + * Client - owner of the card + * + * The current implementation passes the raw APDU's from 7816 and additionally + * contains messages to setup and teardown readers, handle insertion and + * removal of cards, negotiate the protocol via capabilities and provide + * for error responses. + * + * Copyright (c) 2011 Red Hat. + * + * This code is licensed under the LGPL. Please be more specific on the license. Other code has the following: * This work is licensed under the terms of the GNU LGPL, version 2.1 or later. * See the COPYING.LIB file in the top-level directory. [snip] +/* VSCMsgInit Client - Host + * Client sends it on connection, with its own capabilities. + * Host replies with VSCMsgInit filling in its capabilities. + * + * It is not meant to be used for negotiation, i.e. sending more then + * once from any side, but could be used for that in the future. + * */ Looks like some automatic mangling of comments - happens in more than one place. What, the repeated capabilities word at the end of two consecutive lines? that's on purpose. Except for the cosmetic stuff, looks ok. Jes
Re: [Qemu-devel] [PATCH 3/7] ccid: add passthru card device
On Mon, Mar 14, 2011 at 03:04:04PM +0100, Jes Sorensen wrote: On 02/23/11 12:20, Alon Levy wrote: The passthru ccid card is a device sitting on the usb-ccid bus and using a chardevice to communicate with a remote device using the VSCard protocol defined in libcacard/vscard_common.h Usage docs available in following patch in docs/ccid.txt Signed-off-by: Alon Levy al...@redhat.com Except for the usual formatting bits, this one looks fine to me. What exactly? Cheers, Jes
Re: [Qemu-devel] [PATCH 2/7] introduce libcacard/vscard_common.h
On Mon, Mar 14, 2011 at 03:01:19PM +0100, Jes Sorensen wrote: On 02/23/11 12:20, Alon Levy wrote: diff --git a/libcacard/vscard_common.h b/libcacard/vscard_common.h new file mode 100644 index 000..7449314 --- /dev/null +++ b/libcacard/vscard_common.h @@ -0,0 +1,167 @@ +/* Virtual Smart Card protocol definition + * + * This protocol is between a host using virtual smart card readers, + * and a client providing the smart cards, perhaps by emulating them or by + * access to real cards. + * + * Definitions for this protocol: + * Host - user of the card + * Client - owner of the card + * + * The current implementation passes the raw APDU's from 7816 and additionally + * contains messages to setup and teardown readers, handle insertion and + * removal of cards, negotiate the protocol via capabilities and provide + * for error responses. + * + * Copyright (c) 2011 Red Hat. + * + * This code is licensed under the LGPL. Please be more specific on the license. Other code has the following: * This work is licensed under the terms of the GNU LGPL, version 2.1 or later. * See the COPYING.LIB file in the top-level directory. Yes, I have this ready on the v21 branch.. [snip] +/* VSCMsgInit Client - Host + * Client sends it on connection, with its own capabilities. + * Host replies with VSCMsgInit filling in its capabilities. + * + * It is not meant to be used for negotiation, i.e. sending more then + * once from any side, but could be used for that in the future. + * */ Looks like some automatic mangling of comments - happens in more than one place. Except for the cosmetic stuff, looks ok. Jes
Re: [Qemu-devel] Re: Strategic decision: COW format
The file system can keep a lot of these things around pretty easily but with your proposal, it seems like there can only be one. If you support many of them, I think you'll degenerate to something as complex as a reference count table. IIUC, he already uses a refcount table. Well, he needs a separate mechanism to make trim/discard work, but for the snapshot discussion, a reference count table is avoided. Kevin is right. FVD does have a refcount table. Sorry for causing confusion. I am going to send out a very detailed email which describes the operation steps in FVD, as Stefan requested. The bitmap only covers whether the guest has accessed a block or not. Then there is a separate table that maps guest offsets to offsets within the file. I haven't thought hard about it, but my guess is that there is an ordering constraint between these two pieces of metadata which is why the journal is necessary. I get worried about the complexity of a journal even more than a reference count table. No, the journal is not necessary. Actually, a very old version of FVD worked without journal. Journal was later introduced as a performance enhancement. Maybe I missed it, but in the WCE=0 mode, is it really possible to avoid the writes for the refcount table? Yes, this is indeed achieved in FVD, with zero writes to the refcount table on the fast path. See details in the other email I am going to send out soon. Regards, ChunQiang (CQ) Tang Homepage: http://www.research.ibm.com/people/c/ctang
Re: [Qemu-devel] Re: Strategic decision: COW format
On Mon, Mar 14, 2011 at 2:49 PM, Anthony Liguori anth...@codemonkey.ws wrote: On 03/14/2011 09:21 AM, Kevin Wolf wrote: Am 14.03.2011 15:02, schrieb Anthony Liguori: On 03/14/2011 08:53 AM, Chunqiang Tang wrote: No, because the copy-on-write is another layer on top of the snapshot and AFAICT, they don't persist when moving between snapshots. The equivalent for external snapshots would be: base0- base1- base2- image And then if I wanted to move to base1 without destroying base2 and image, I could do: qemu-img create -f qcow2 -b base1 base1-overlay.img The file system can keep a lot of these things around pretty easily but with your proposal, it seems like there can only be one. If you support many of them, I think you'll degenerate to something as complex as a reference count table. On the other hand, I think it's reasonable to just avoid the CoW overlay entirely and say that moving to a previous snapshot destroys any of it's children. I think this ends up being a simplifying assumption that is worth investigating further. No, both VMware and FVD have the same semantics as QCOW2. Moving to a previous snapshot does not destroy any of its children. In the example I gave (copied below), it goes from Image: s1-s2-s3-s4-(current-state) back to snapshot s2, and now the state is Image: s1-s2-s3-s4 |-(curren-state) where all snapshots s1-s4 are kept. From there, it can take another snapshot s5, and then further go back to snapshot s4, ending up with Image: s1-s2-s3-s4 |-s5 | |- (current-state) Your use of current-state is confusing me because AFAICT, current-state is just semantically another snapshot. It's writable because it has no children. You only keep around one writable snapshot and to make another snapshot writable, you have to discard the former. This is not the semantics of qcow2. Every time you create a snapshot, it's essentially a new image. You can write directly to it. While we don't do this today and I don't think we ever should, it's entirely possible to have two disks served simultaneously out of the same qcow2 file using snapshots. No, CQ is describing the semantics of internal snapshots in qcow2 correctly. You have all the snapshots that are stored in the snapshot table (all read-only) plus one current state described by the image header (read-write). But is there any problem (in the format) with writing to the non-current state? I can't think of one. Here is a problem: there is a single global refcount table in QCOW2. You need to synchronize updates of the refcounts between multiple writers to avoid introducing incorrect refcounts. Stefan
Re: [Qemu-devel] Re: Strategic decision: COW format
Am 14.03.2011 15:47, schrieb Anthony Liguori: On 03/14/2011 09:15 AM, Kevin Wolf wrote: The file system can keep a lot of these things around pretty easily but with your proposal, it seems like there can only be one. If you support many of them, I think you'll degenerate to something as complex as a reference count table. IIUC, he already uses a refcount table. Well, he needs a separate mechanism to make trim/discard work, but for the snapshot discussion, a reference count table is avoided. The bitmap only covers whether the guest has accessed a block or not. Then there is a separate table that maps guest offsets to offsets within the file. I haven't thought hard about it, but my guess is that there is an ordering constraint between these two pieces of metadata which is why the journal is necessary. I get worried about the complexity of a journal even more than a reference count table. Honestly I think that a journal is a good idea that we'll want to implement in the long run. There are people who aren't really happy about the dirty flag + fsck approach, and there are people who are concerned about cluster leaks without fsck. Both problems should be solved with a journal. Compared to other questions in the discussio, I think it's only a nice-to-have addition, though. Actually, I think that a refcount table is a requirement to provide the interesting properties that internal snapshots have (see my other mail). Well the trick here AFAICT is that you're basically storing external snapshots internally. So it's sort of like a bunch of FVD formats embedded into a single image. CQ, can you please clarify? From your description, Anthony seems to understand something completely different than I do. Are its characteristics more like qcow2's internal snapshots (which is what I understand) or more like external snapshots (which is what Anthony seems to understand). Refcount tables aren't a very complex thing either. In fact, it makes a format much simpler to have one concept like refcount tables instead of adding another different mechanism for each new feature that would be natural with refcount tables. I think it's a reasonable design goal to minimize any metadata updates in the fast path. If we can write 1 piece of metadata verses writing 2, then it's worth exploring IMHO. The only problem with them is that they are metadata that must be updated. However, I think we have discussed enough how to avoid the greatest part of that cost. Maybe I missed it, but in the WCE=0 mode, is it really possible to avoid the writes for the refcount table? Protected by a dirty flag (and/or a journal), sure. I mean, wasn't that the whole point of starting the qcow3 discussion? Kevin
Re: [Qemu-devel] Re: Strategic decision: COW format
On Mon, Mar 14, 2011 at 3:04 PM, Chunqiang Tang ct...@us.ibm.com wrote: The file system can keep a lot of these things around pretty easily but with your proposal, it seems like there can only be one. If you support many of them, I think you'll degenerate to something as complex as a reference count table. IIUC, he already uses a refcount table. Well, he needs a separate mechanism to make trim/discard work, but for the snapshot discussion, a reference count table is avoided. Kevin is right. FVD does have a refcount table. Sorry for causing confusion. I am going to send out a very detailed email which describes the operation steps in FVD, as Stefan requested. The bitmap only covers whether the guest has accessed a block or not. Then there is a separate table that maps guest offsets to offsets within the file. I haven't thought hard about it, but my guess is that there is an ordering constraint between these two pieces of metadata which is why the journal is necessary. I get worried about the complexity of a journal even more than a reference count table. No, the journal is not necessary. Actually, a very old version of FVD worked without journal. Journal was later introduced as a performance enhancement. I like the journal because it allows us to isolate metadata updates into one specific area that can be scanned on image recovery. If we take the QED approach with the dirty bit then we have to scan all L1/L2 tables. The journal makes recovery more efficient than a full consistency check. Stefan
Re: [Qemu-devel] Re: Strategic decision: COW format
Am 14.03.2011 15:49, schrieb Anthony Liguori: On 03/14/2011 09:21 AM, Kevin Wolf wrote: Am 14.03.2011 15:02, schrieb Anthony Liguori: On 03/14/2011 08:53 AM, Chunqiang Tang wrote: No, because the copy-on-write is another layer on top of the snapshot and AFAICT, they don't persist when moving between snapshots. The equivalent for external snapshots would be: base0- base1- base2- image And then if I wanted to move to base1 without destroying base2 and image, I could do: qemu-img create -f qcow2 -b base1 base1-overlay.img The file system can keep a lot of these things around pretty easily but with your proposal, it seems like there can only be one. If you support many of them, I think you'll degenerate to something as complex as a reference count table. On the other hand, I think it's reasonable to just avoid the CoW overlay entirely and say that moving to a previous snapshot destroys any of it's children. I think this ends up being a simplifying assumption that is worth investigating further. No, both VMware and FVD have the same semantics as QCOW2. Moving to a previous snapshot does not destroy any of its children. In the example I gave (copied below), it goes from Image: s1-s2-s3-s4-(current-state) back to snapshot s2, and now the state is Image: s1-s2-s3-s4 |-(curren-state) where all snapshots s1-s4 are kept. From there, it can take another snapshot s5, and then further go back to snapshot s4, ending up with Image: s1-s2-s3-s4 |-s5 | |- (current-state) Your use of current-state is confusing me because AFAICT, current-state is just semantically another snapshot. It's writable because it has no children. You only keep around one writable snapshot and to make another snapshot writable, you have to discard the former. This is not the semantics of qcow2. Every time you create a snapshot, it's essentially a new image. You can write directly to it. While we don't do this today and I don't think we ever should, it's entirely possible to have two disks served simultaneously out of the same qcow2 file using snapshots. No, CQ is describing the semantics of internal snapshots in qcow2 correctly. You have all the snapshots that are stored in the snapshot table (all read-only) plus one current state described by the image header (read-write). But is there any problem (in the format) with writing to the non-current state? I can't think of one. You would run into problems with the COW flag in the L2 tables. They are only an optimization, though, so you could probably avoid using them and directly look up the refcount table for each write, at the cost of performance. Anyway, I don't think there's a real use case for something like this. Kevin
Re: [Qemu-devel] Re: Strategic decision: COW format
On 03/14/2011 10:03 AM, Kevin Wolf wrote: The only problem with them is that they are metadata that must be updated. However, I think we have discussed enough how to avoid the greatest part of that cost. Maybe I missed it, but in the WCE=0 mode, is it really possible to avoid the writes for the refcount table? Protected by a dirty flag (and/or a journal), sure. I mean, wasn't that the whole point of starting the qcow3 discussion? Okay, I thought you had something else in mind. Regards, Anthony Liguori Kevin
Re: [Qemu-devel] Re: KVM call agenda for Jan 25
Nice that qemu-img convert isn't that far out by default on raw :). About Google Summer of Code, I have posted my take on applying and want to share that with you and qemu-devel: http://blog.vmsplice.net/2011/03/advice-for-students-applying-to-google.html Thanks for sharing your experiences. After reading about qcow2 and qed and how they organize data (thanks to the newly added qcow2 doc and discussions on the mailing list), this is what I understand. So, the main difference between qed and qcow2 is the absence of reference count structure in qed(means less meta data). It improves performance due to: 1. For write operations, less or no metadata to update. 2. Data write and metadata write can be in any order This also means these features are no longer supported: 1. Internal snapshots, 2. CPU/device state snapshots, 3. Compression, 4. Encryption Now, coming to qed--qcow2 conversion, I want to clarify some things. 1. header_size: variable in qed, equals to cluster size in qcow2: When will it be larger than 1 cluster in qed? So, what will happen to that extra data on qed-qcow2 conversion. 2. L2 table size: equals to L1 table size in qed, equals to cluster size in qcow2: we need to take it into account during conversion. 3. refcount table: qcow2-qed:we do not keep refcount structure qed-qcow2: initialize refcount structure So, a qcow2-qed-qcow2 conversion means if earlier, qcow2 image was using additional features{1-4}, all information related to that will be lost. What do you think? Please correct me if I am making some incorrect statement. Thanks, Dushyant
Re: [Qemu-devel] [PATCH 4/7] libcacard: initial commit
On 02/23/11 12:20, Alon Levy wrote: +/* private data for PKI applets */ +typedef struct CACPKIAppletDataStruct { +unsigned char *cert; +int cert_len; +unsigned char *cert_buffer; +int cert_buffer_len; +unsigned char *sign_buffer; +int sign_buffer_len; +VCardKey *key; +} CACPKIAppletData; Grouping the ints together would allow for better struct padding. +/* + * resest the inter call state between applet selects + */ I presume it meant to say 'resets' ? diff --git a/libcacard/event.c b/libcacard/event.c new file mode 100644 index 000..20276a0 --- /dev/null +++ b/libcacard/event.c @@ -0,0 +1,112 @@ +/* + * + */ This comment is really spot on :) diff --git a/libcacard/mutex.h b/libcacard/mutex.h new file mode 100644 index 000..cb46aa7 --- /dev/null +++ b/libcacard/mutex.h UH OH! +/* + * This header file provides a way of mapping windows and linux thread calls + * to a set of macros. Ideally this would be shared by whatever subsystem we + * link with. + */ + +#ifndef _H_MUTEX +#define _H_MUTEX +#ifdef _WIN32 +#include windows.h +typedef CRITICAL_SECTION mutex_t; +#define MUTEX_INIT(mutex) InitializeCriticalSection(mutex) +#define MUTEX_LOCK(mutex) EnterCriticalSection(mutex) +#define MUTEX_UNLOCK(mutex) LeaveCriticalSection(mutex) +typedef CONDITION_VARIABLE condition_t; +#define CONDITION_INIT(cond) InitializeConditionVariable(cond) +#define CONDITION_WAIT(cond, mutex) \ +SleepConditionVariableCS(cond, mutex, INFINTE) +#define CONDITION_NOTIFY(cond) WakeConditionVariable(cond) +typedef uint32_t thread_t; +typedef HANDLE thread_status_t; +#define THREAD_CREATE(tid, func, arg) \ +CreateThread(NULL, 0, (LPTHREAD_START_ROUTINE)func, arg, 0, tid) +#define THREAD_SUCCESS(status) ((status) != NULL) +#else +#include pthread.h +typedef pthread_mutex_t mutex_t; +#define MUTEX_INIT(mutex) pthread_mutex_init(mutex, NULL) +#define MUTEX_LOCK(mutex) pthread_mutex_lock(mutex) +#define MUTEX_UNLOCK(mutex) pthread_mutex_unlock(mutex) +typedef pthread_cond_t condition_t; +#define CONDITION_INIT(cond) pthread_cond_init(cond, NULL) +#define CONDITION_WAIT(cond, mutex) pthread_cond_wait(cond, mutex) +#define CONDITION_NOTIFY(cond) pthread_cond_signal(cond) +typedef pthread_t thread_t; +typedef int thread_status_t; +#define THREAD_CREATE(tid, func, arg) pthread_create(tid, NULL, func, arg) +#define THREAD_SUCCESS(status) ((status) == 0) +#endif NACK! This is no good, the code needs to be fixed to use QemuMutex from qemu-thread.h In addition, a thing like a mutex feature should be in a separate patch, not part of the code that uses it. However QEMU already has a mutex set so this part needs to go. +static VCardAppletPrivate * +passthru_new_applet_private(VReader *reader) +{ +VCardAppletPrivate *applet_private = NULL; +LONG rv; + +applet_private = (VCardAppletPrivate *)malloc(sizeof(VCardAppletPrivate)); qemu_malloc() +if (applet_private == NULL) { +goto fail; +} and it never fails. +if (new_reader_list_len != reader_list_len) { +/* update the list */ +new_reader_list = (char *)malloc(new_reader_list_len); qemu_malloc() again. Please grep through the full patch set and make sure you do not have any direct calls to malloc() or free(). +/* try resetting the pcsc_lite subsystem */ +SCardReleaseContext(global_context); +global_context = 0; /* should close it */ +printf(* SCard failure %x\n, rv); error_report() diff --git a/libcacard/vcard_emul_nss.c b/libcacard/vcard_emul_nss.c new file mode 100644 index 000..e4f0b73 --- /dev/null +++ b/libcacard/vcard_emul_nss.c [snip] +struct VReaderEmulStruct { +PK11SlotInfo *slot; +VCardEmulType default_type; +char *type_params; +PRBool present; What is PRBool and where does it come from? +void +vcard_emul_reset(VCard *card, VCardPower power) +{ +PK11SlotInfo *slot; + +if (!nss_emul_init) { +return; +} + +/* if we reset the card (either power on or power off), we loose our login + * state */ +/* TODO: we may also need to send insertion/removal events? */ +slot = vcard_emul_card_get_slot(card); +(void)PK11_Logout(slot); We don't (void) cast calls like that in QEMU. +/* + * NSS needs the app to supply a password prompt. In our case the only time + * the password is supplied is as part of the Login APDU. The actual password + * is passed in the pw_arg in that case. In all other cases pw_arg should be + * NULL. + */ +static char * +vcard_emul_get_password(PK11SlotInfo *slot, PRBool retries, void *pw_arg) +{ +/* if it didn't work the first time, don't keep trying */ +if (retries) { +return NULL; +} +/* we are looking up a password when we don't have one in hand */ +if (pw_arg == NULL) { +
[Qemu-devel] [PATCH v2 2/2] target-arm: use make_float32() to make constant floats for VRSQRTS
The preferred way to create a constant floating point value is to use make_float32() rather than doing a runtime int32_to_float32(). Convert the code in the VRSQRTS helper to work this way. Signed-off-by: Peter Maydell peter.mayd...@linaro.org --- target-arm/helper.c | 11 +-- 1 files changed, 5 insertions(+), 6 deletions(-) diff --git a/target-arm/helper.c b/target-arm/helper.c index c01a5a2..0dd2549 100644 --- a/target-arm/helper.c +++ b/target-arm/helper.c @@ -2708,6 +2708,8 @@ uint32_t HELPER(vfp_fcvt_f32_to_f16)(float32 a, CPUState *env) } #define float32_two make_float32(0x4000) +#define float32_three make_float32(0x4040) +#define float32_one_point_five make_float32(0x3fc0) float32 HELPER(recps_f32)(float32 a, float32 b, CPUState *env) { @@ -2722,16 +2724,13 @@ float32 HELPER(recps_f32)(float32 a, float32 b, CPUState *env) float32 HELPER(rsqrts_f32)(float32 a, float32 b, CPUState *env) { float_status *s = env-vfp.standard_fp_status; -float32 two = int32_to_float32(2, s); -float32 three = int32_to_float32(3, s); float32 product; if ((float32_is_infinity(a) float32_is_zero_or_denormal(b)) || (float32_is_infinity(b) float32_is_zero_or_denormal(a))) { -product = float32_zero; -} else { -product = float32_mul(a, b, s); +return float32_one_point_five; } -return float32_div(float32_sub(three, product, s), two, s); +product = float32_mul(a, b, s); +return float32_div(float32_sub(float32_three, product, s), float32_two, s); } /* NEON helpers. */ -- 1.7.1
Re: [Qemu-devel] [PATCH 5/7] ccid: add ccid-card-emulated device
On 02/23/11 12:20, Alon Levy wrote: diff --git a/hw/ccid-card-emulated.c b/hw/ccid-card-emulated.c new file mode 100644 index 000..bd84d45 --- /dev/null +++ b/hw/ccid-card-emulated.c @@ -0,0 +1,599 @@ +/* + * CCID Card Device. Emulated card. + * + * Copyright (c) 2011 Red Hat. + * Written by Alon Levy. + * + * This code is licenced under the GNU LGPL, version 2 or later. + */ + +/* + * It can be used to provide access to the local hardware in a non exclusive + * way, or it can use certificates. It requires the usb-ccid bus. + * + * Usage 1: standard, mirror hardware reader+card: + * qemu .. -usb -device usb-ccid -device ccid-card-emulated + * + * Usage 2: use certificates, no hardware required + * one time: create the certificates: + * for i in 1 2 3; do + * certutil -d /etc/pki/nssdb -x -t CT,CT,CT -S -s CN=user$i -n user$i + * done + * qemu .. -usb -device usb-ccid \ + * -device ccid-card-emulated,cert1=user1,cert2=user2,cert3=user3 + * + * If you use a non default db for the certificates you can specify it using + * the db parameter. + */ + +#include pthread.h qemu-thread.h +static const char *emul_event_to_string(uint32_t emul_event) +{ +switch (emul_event) { +case EMUL_READER_INSERT: return EMUL_READER_INSERT; +case EMUL_READER_REMOVE: return EMUL_READER_REMOVE; +case EMUL_CARD_INSERT: return EMUL_CARD_INSERT; +case EMUL_CARD_REMOVE: return EMUL_CARD_REMOVE; +case EMUL_GUEST_APDU: return EMUL_GUEST_APDU; +case EMUL_RESPONSE_APDU: return EMUL_RESPONSE_APDU; +case EMUL_ERROR: return EMUL_ERROR; YUCK! No multi statements on a single line! +#define MAX_ATR_SIZE 40 +struct EmulatedState { +CCIDCardState base; +uint8_t debug; +char*backend_str; +uint32_t backend; +char*cert1; +char*cert2; +char*cert3; +char*db; +uint8_t atr[MAX_ATR_SIZE]; +uint8_t atr_length; +QSIMPLEQ_HEAD(event_list, EmulEvent) event_list; +pthread_mutex_t event_list_mutex; +VReader *reader; +QSIMPLEQ_HEAD(guest_apdu_list, EmulEvent) guest_apdu_list; +pthread_mutex_t vreader_mutex; /* and guest_apdu_list mutex */ +pthread_mutex_t handle_apdu_mutex; +pthread_cond_t handle_apdu_cond; +int pipe[2]; +int quit_apdu_thread; +pthread_mutex_t apdu_thread_quit_mutex; +pthread_cond_t apdu_thread_quit_cond; +}; Bad struct packing and wrong thread types. +static void emulated_push_type(EmulatedState *card, uint32_t type) +{ +EmulEvent *event = (EmulEvent *)malloc(sizeof(EmulEvent)); qemu_malloc() +assert(event); +event-p.gen.type = type; +emulated_push_event(card, event); +} + +static void emulated_push_error(EmulatedState *card, uint64_t code) +{ +EmulEvent *event = (EmulEvent *)malloc(sizeof(EmulEvent)); qemu_malloc() +assert(event); +event-p.error.type = EMUL_ERROR; +event-p.error.code = code; +emulated_push_event(card, event); +} + +static void emulated_push_data_type(EmulatedState *card, uint32_t type, +const uint8_t *data, uint32_t len) +{ +EmulEvent *event = (EmulEvent *)malloc(sizeof(EmulEvent) + len); qemu_malloc() +static void pipe_read(void *opaque) +{ +EmulatedState *card = opaque; +EmulEvent *event, *next; +char dummy; +int len; + +do { +len = read(card-pipe[0], dummy, sizeof(dummy)); +} while (len == sizeof(dummy)); Shouldn't you check error codes here? Jes
Re: [Qemu-devel] [PATCH 6/7] ccid: add docs
On 02/23/11 12:20, Alon Levy wrote: Add documentation for the usb-ccid device and accompanying two card devices, ccid-card-emulated and ccid-card-passthru. Signed-off-by: Alon Levy al...@redhat.com --- docs/ccid.txt | 135 + 1 files changed, 135 insertions(+), 0 deletions(-) create mode 100644 docs/ccid.txt This one looks fine :) Jes
Re: [Qemu-devel] [PATCH 7/7] ccid: configure: improve --enable-smartcard flags
On 02/23/11 12:20, Alon Levy wrote: * add --enable-smartcard and --disable-smartcard flags * let the nss check only disable building the ccid-card-emulated device * report only if nss is found or not, not smartcard build inclusion * don't link with NSS if --disable-smartcard-nss The --disable-smartcard flag really should go with the initial smartcard changes to configure. That way it is possible to test that each individual patch doesn't break the build when smartcard support is enabled or disabled. If you add it at the end, you figuring out which patch is the problem is much harder. Adding the smartcard-nss flag should go next to the patch that adds the code relying on the flag. Cheers, Jes
[Qemu-devel] [PATCH v2 0/2] Fix VRECPS edge cases handling
This patchset fixes the edge case handling of VRECPS. Patch 2/2 is just a bit of cleanup of the neighbouring vrsqrts helper which can then use the float32_two introduced by 1/1. Tested in the usual random-insn-generation way and also with the neon64 test program from the valgrind ARM testsuite. Changes from v1: fix compile failure in 2/2 (forgot to run stg refresh before sending out patchset, oops...) Peter Maydell (2): target-arm: Fix VRECPS edge cases handling target-arm: use make_float32() to make constant floats for VRSQRTS target-arm/helper.c | 22 +- 1 files changed, 13 insertions(+), 9 deletions(-)
[Qemu-devel] [PATCH v2 1/2] target-arm: Fix VRECPS edge cases handling
Correct the handling of edge cases for the VRECPS instruction: * this is a Neon instruction so uses the standard FPSCR value * (zero, inf) is a special case which returns 2.0 Signed-off-by: Peter Maydell peter.mayd...@linaro.org --- target-arm/helper.c | 11 --- 1 files changed, 8 insertions(+), 3 deletions(-) diff --git a/target-arm/helper.c b/target-arm/helper.c index d360121..c01a5a2 100644 --- a/target-arm/helper.c +++ b/target-arm/helper.c @@ -2707,11 +2707,16 @@ uint32_t HELPER(vfp_fcvt_f32_to_f16)(float32 a, CPUState *env) return do_fcvt_f32_to_f16(a, env, env-vfp.fp_status); } +#define float32_two make_float32(0x4000) + float32 HELPER(recps_f32)(float32 a, float32 b, CPUState *env) { -float_status *s = env-vfp.fp_status; -float32 two = int32_to_float32(2, s); -return float32_sub(two, float32_mul(a, b, s), s); +float_status *s = env-vfp.standard_fp_status; +if ((float32_is_infinity(a) float32_is_zero_or_denormal(b)) || +(float32_is_infinity(b) float32_is_zero_or_denormal(a))) { +return float32_two; +} +return float32_sub(float32_two, float32_mul(a, b, s), s); } float32 HELPER(rsqrts_f32)(float32 a, float32 b, CPUState *env) -- 1.7.1
Re: [Qemu-devel] [PATCH 2/7] introduce libcacard/vscard_common.h
On 03/14/11 15:52, Alon Levy wrote: [snip] +/* VSCMsgInit Client - Host + * Client sends it on connection, with its own capabilities. + * Host replies with VSCMsgInit filling in its capabilities. + * + * It is not meant to be used for negotiation, i.e. sending more then + * once from any side, but could be used for that in the future. + * */ Looks like some automatic mangling of comments - happens in more than one place. What, the repeated capabilities word at the end of two consecutive lines? that's on purpose. Nope the * * * * */ looks odd. Jes
Re: [Qemu-devel] [PATCH 3/7] ccid: add passthru card device
On 03/14/11 15:53, Alon Levy wrote: On Mon, Mar 14, 2011 at 03:04:04PM +0100, Jes Sorensen wrote: On 02/23/11 12:20, Alon Levy wrote: The passthru ccid card is a device sitting on the usb-ccid bus and using a chardevice to communicate with a remote device using the VSCard protocol defined in libcacard/vscard_common.h Usage docs available in following patch in docs/ccid.txt Signed-off-by: Alon Levy al...@redhat.com Except for the usual formatting bits, this one looks fine to me. What exactly? The comments formatting as I mentioned in previous patches. Jes
Re: [Qemu-devel] [PATCH 2/7] introduce libcacard/vscard_common.h
On Mon, Mar 14, 2011 at 04:50:21PM +0100, Jes Sorensen wrote: On 03/14/11 15:52, Alon Levy wrote: [snip] +/* VSCMsgInit Client - Host + * Client sends it on connection, with its own capabilities. + * Host replies with VSCMsgInit filling in its capabilities. + * + * It is not meant to be used for negotiation, i.e. sending more then + * once from any side, but could be used for that in the future. + * */ Looks like some automatic mangling of comments - happens in more than one place. What, the repeated capabilities word at the end of two consecutive lines? that's on purpose. Nope the * * * * */ oh, that I actually left on purpose (well, not really caring that much). looks odd. Jes
Re: [Qemu-devel] Re: Strategic decision: COW format
FVD's novel uses of the reference count table reduces the metadata update overhead down to literally zero during normal execution of a VM. This gets the bests of QCOW2's reference count table but without its oeverhead. In FVD, the reference count table is only updated when creating a new snapshot or deleting an existing snapshot. The reference count table is never updated during normal execution of a VM. Do you want to send out a break-down of the steps (and cost) involved in doing: 1. Snapshot creation. 2. Snapshot deletion. 3. Opening an image with n snapshots. Here is a detailed description. Relevant to the discussion of snapshot, FVD uses a one-level lookup table and a refcount table. FVD’s one-level lookup table is very similar to QCOW2’s two-level lookup table, except that it is much smaller in FVD, and is preallocated and hence contiguous in image. FVD’s refcount table is almost identical to that of QCOW2, but with a key difference. An image consists of an arbitrary number of read-only snapshots, and a single writeable image front, which is the current image state perceived by the VM. Below, I will simply refer to the read-only snapshots as snapshots, and refer to the “writeable image front” as “writeable-front.” QCOW2’s refcount table counts clusters that are used by either read-only snapshots or writeable-front. Because writeable-front changes as the VM runs, QCOW2 needs to update the refcount table on the fast path of normal VM execution. By contrast, FVD’s refcount table only counts chunks that are used by read-only snapshots, and does not count chunks used by write-front. This is the key that allows FVD to entirely avoid updating the refcount table on the fast path of normal VM execution. Below are the detailed steps for different operations. O1: Open an image with n snapshots. Let me introduce some basic concepts first. The storage allocation unit in FVD is called a chunk (like cluster in QCOW2). The default chunk size is 1MB, like that in VDI (VMDK and Microsoft VHD use 2MB chunks). An FVD image file is conceptually divided into chunks, where chunk 0 is the first 1MB of the image file, chunk 1 is the second 1MB, … chunk j, …and so forth. The size of an image file grow as needed, just like that of QCOW2. The refcount table is a linear array “uint16_t refcount[]”. If a chunk j is referenced by s different snapshots, then refcount[j] = s. If a new snapshot is created and this new snapshot also uses chunk j, then refcount[j] is incremented to refcount[j] = s+1. If all snapshots together use 1TB storage spaces, there are 1TB/1MB=1,000,000 chunks, and the size of the refcount table is 2MB. Loading the entire 2MB refcount table from disk into memory takes about 15 milliseconds. If the virtual disk size perceived by the VM is also 1TB, FVD’s one-level lookup table is 4MB. FVD’s one-level lookup table serves the same purpose as QCOW2’s two-level lookup table, but FVD’s one-level table is much smaller and is preallocated and hence continuous in the image. Loading the entire 4MB lookup table from disk into memory takes about 20 milliseconds. These numbers mean that it is quite affordable to scan the entire tables at VM boot time, although the scan can also be avoided in FVD. The optimizations will be described later. When opening an image with n snapshots, an unoptimized version of FVD performs the following steps: O1: Load the entire 2MB reference count table from disk into memory. This step takes about 15ms. O2: Load the entire 4MB lookup table from disk into memory. This step takes about 20ms. O3: Use the two tables to build an in-memory data structure called “free-chunk-bitmap.” This step takes about 2ms. The free-chunk-bitmap identifies free chunks that are not used by either snapshots or writeable front, and hence can be allocated for future writes. The size of the free-chunk-bitmap is only 125KB for a 1TB disk, and hence the memory overhead is negligible. The free-chunk-bitmap also supports trim operations. The free-chunk-bitmap does not have to be persisted on disk as it can always be rebuilt easily, although as an optimization it can be persisted on disk on VM shutdown. O4: Compare the refcount table and the lookup table to identify chunks that are in both tables (i.e., shared) and hence the running VM’s write to those chunks in writeable-front triggers copy-on-write. This step takes about 2ms. One bit in the lookup table’s entry is stolen to mark whether a chunk in writeable-front is shared with snapshots and hence needs copy-on-write upon a write. The whole process above, i.e., opening an image with n (e.g., n=1000) snapshots, takes about 39ms and it is a one-time cost at VM boot. Later, I will describe optimizations that can further reduce this 39ms by saving the 125KB free-chunk-bitmap to disk on VM shutdown, but that optimization is more than likely to an over-engineering effort, given that 39ms
[Qemu-devel] Re: KVM call agenda for Mars 14th
Jes Sorensen jes.soren...@redhat.com wrote: On 03/14/11 13:14, Juan Quintela wrote: Please send any agenda items you are interested in covering. Thanks, Juan. I presume you mean for March 15? Today is the 14th and it is Monday :) Dunno what calendar I looked to :p Yes, you are right. #secure method=pgpmime mode=sign
Re: [Qemu-devel] [PATCH 4/7] libcacard: initial commit
On Mon, Mar 14, 2011 at 04:20:22PM +0100, Jes Sorensen wrote: ok, here is a note where I kinda ignored my own wishes but I want to be very clear on them: libcacard should not be part of qemu. it is here because I once thought it would speed things up. So I'm not taking it out or anything - it's fine with me that it goes into qemu, just as long as it's understood that I'm now maintaining another copy of it for usage outside of qemu, in the spice client (or any other client for that matter - it will be the same when we do vnc support for this). If I start using qemu-ism then I end up having something I have to change outside of qemu. Like using QemuMutex. Another option is for me to check a define, i.e. QEMU_EMBEDDED. I'm fine with that. This goes for malloc too of course. And for assuming it never fails. Actually the later I could change the library to have it's internal malloc that does an assert (it's ok with the client to abort on oom). rest of my comments are inline. On 02/23/11 12:20, Alon Levy wrote: +/* private data for PKI applets */ +typedef struct CACPKIAppletDataStruct { +unsigned char *cert; +int cert_len; +unsigned char *cert_buffer; +int cert_buffer_len; +unsigned char *sign_buffer; +int sign_buffer_len; +VCardKey *key; +} CACPKIAppletData; Grouping the ints together would allow for better struct padding. +/* + * resest the inter call state between applet selects + */ I presume it meant to say 'resets' ? diff --git a/libcacard/event.c b/libcacard/event.c new file mode 100644 index 000..20276a0 --- /dev/null +++ b/libcacard/event.c @@ -0,0 +1,112 @@ +/* + * + */ This comment is really spot on :) diff --git a/libcacard/mutex.h b/libcacard/mutex.h new file mode 100644 index 000..cb46aa7 --- /dev/null +++ b/libcacard/mutex.h UH OH! +/* + * This header file provides a way of mapping windows and linux thread calls + * to a set of macros. Ideally this would be shared by whatever subsystem we + * link with. + */ + +#ifndef _H_MUTEX +#define _H_MUTEX +#ifdef _WIN32 +#include windows.h +typedef CRITICAL_SECTION mutex_t; +#define MUTEX_INIT(mutex) InitializeCriticalSection(mutex) +#define MUTEX_LOCK(mutex) EnterCriticalSection(mutex) +#define MUTEX_UNLOCK(mutex) LeaveCriticalSection(mutex) +typedef CONDITION_VARIABLE condition_t; +#define CONDITION_INIT(cond) InitializeConditionVariable(cond) +#define CONDITION_WAIT(cond, mutex) \ +SleepConditionVariableCS(cond, mutex, INFINTE) +#define CONDITION_NOTIFY(cond) WakeConditionVariable(cond) +typedef uint32_t thread_t; +typedef HANDLE thread_status_t; +#define THREAD_CREATE(tid, func, arg) \ +CreateThread(NULL, 0, (LPTHREAD_START_ROUTINE)func, arg, 0, tid) +#define THREAD_SUCCESS(status) ((status) != NULL) +#else +#include pthread.h +typedef pthread_mutex_t mutex_t; +#define MUTEX_INIT(mutex) pthread_mutex_init(mutex, NULL) +#define MUTEX_LOCK(mutex) pthread_mutex_lock(mutex) +#define MUTEX_UNLOCK(mutex) pthread_mutex_unlock(mutex) +typedef pthread_cond_t condition_t; +#define CONDITION_INIT(cond) pthread_cond_init(cond, NULL) +#define CONDITION_WAIT(cond, mutex) pthread_cond_wait(cond, mutex) +#define CONDITION_NOTIFY(cond) pthread_cond_signal(cond) +typedef pthread_t thread_t; +typedef int thread_status_t; +#define THREAD_CREATE(tid, func, arg) pthread_create(tid, NULL, func, arg) +#define THREAD_SUCCESS(status) ((status) == 0) +#endif NACK! This is no good, the code needs to be fixed to use QemuMutex from qemu-thread.h In addition, a thing like a mutex feature should be in a separate patch, not part of the code that uses it. However QEMU already has a mutex set so this part needs to go. +static VCardAppletPrivate * +passthru_new_applet_private(VReader *reader) +{ +VCardAppletPrivate *applet_private = NULL; +LONG rv; + +applet_private = (VCardAppletPrivate *)malloc(sizeof(VCardAppletPrivate)); qemu_malloc() +if (applet_private == NULL) { +goto fail; +} and it never fails. +if (new_reader_list_len != reader_list_len) { +/* update the list */ +new_reader_list = (char *)malloc(new_reader_list_len); qemu_malloc() again. Please grep through the full patch set and make sure you do not have any direct calls to malloc() or free(). +/* try resetting the pcsc_lite subsystem */ +SCardReleaseContext(global_context); +global_context = 0; /* should close it */ +printf(* SCard failure %x\n, rv); error_report() diff --git a/libcacard/vcard_emul_nss.c b/libcacard/vcard_emul_nss.c new file mode 100644 index 000..e4f0b73 --- /dev/null +++ b/libcacard/vcard_emul_nss.c [snip] +struct VReaderEmulStruct { +PK11SlotInfo *slot; +VCardEmulType
Re: [Qemu-devel] [PATCH 5/7] ccid: add ccid-card-emulated device
On Mon, Mar 14, 2011 at 04:41:02PM +0100, Jes Sorensen wrote: On 02/23/11 12:20, Alon Levy wrote: diff --git a/hw/ccid-card-emulated.c b/hw/ccid-card-emulated.c new file mode 100644 index 000..bd84d45 --- /dev/null +++ b/hw/ccid-card-emulated.c @@ -0,0 +1,599 @@ +/* + * CCID Card Device. Emulated card. + * + * Copyright (c) 2011 Red Hat. + * Written by Alon Levy. + * + * This code is licenced under the GNU LGPL, version 2 or later. + */ + +/* + * It can be used to provide access to the local hardware in a non exclusive + * way, or it can use certificates. It requires the usb-ccid bus. + * + * Usage 1: standard, mirror hardware reader+card: + * qemu .. -usb -device usb-ccid -device ccid-card-emulated + * + * Usage 2: use certificates, no hardware required + * one time: create the certificates: + * for i in 1 2 3; do + * certutil -d /etc/pki/nssdb -x -t CT,CT,CT -S -s CN=user$i -n user$i + * done + * qemu .. -usb -device usb-ccid \ + * -device ccid-card-emulated,cert1=user1,cert2=user2,cert3=user3 + * + * If you use a non default db for the certificates you can specify it using + * the db parameter. + */ + +#include pthread.h qemu-thread.h +static const char *emul_event_to_string(uint32_t emul_event) +{ +switch (emul_event) { +case EMUL_READER_INSERT: return EMUL_READER_INSERT; +case EMUL_READER_REMOVE: return EMUL_READER_REMOVE; +case EMUL_CARD_INSERT: return EMUL_CARD_INSERT; +case EMUL_CARD_REMOVE: return EMUL_CARD_REMOVE; +case EMUL_GUEST_APDU: return EMUL_GUEST_APDU; +case EMUL_RESPONSE_APDU: return EMUL_RESPONSE_APDU; +case EMUL_ERROR: return EMUL_ERROR; YUCK! No multi statements on a single line! passes check-patch :) will fix. +#define MAX_ATR_SIZE 40 +struct EmulatedState { +CCIDCardState base; +uint8_t debug; +char*backend_str; +uint32_t backend; +char*cert1; +char*cert2; +char*cert3; +char*db; +uint8_t atr[MAX_ATR_SIZE]; +uint8_t atr_length; +QSIMPLEQ_HEAD(event_list, EmulEvent) event_list; +pthread_mutex_t event_list_mutex; +VReader *reader; +QSIMPLEQ_HEAD(guest_apdu_list, EmulEvent) guest_apdu_list; +pthread_mutex_t vreader_mutex; /* and guest_apdu_list mutex */ +pthread_mutex_t handle_apdu_mutex; +pthread_cond_t handle_apdu_cond; +int pipe[2]; +int quit_apdu_thread; +pthread_mutex_t apdu_thread_quit_mutex; +pthread_cond_t apdu_thread_quit_cond; +}; Bad struct packing and wrong thread types. Will fix. Aside: Why do we care about packing something that has a single instance per device? isn't logical readable order more important in this case? +static void emulated_push_type(EmulatedState *card, uint32_t type) +{ +EmulEvent *event = (EmulEvent *)malloc(sizeof(EmulEvent)); qemu_malloc() will fix. (and the rest) +assert(event); +event-p.gen.type = type; +emulated_push_event(card, event); +} + +static void emulated_push_error(EmulatedState *card, uint64_t code) +{ +EmulEvent *event = (EmulEvent *)malloc(sizeof(EmulEvent)); qemu_malloc() +assert(event); +event-p.error.type = EMUL_ERROR; +event-p.error.code = code; +emulated_push_event(card, event); +} + +static void emulated_push_data_type(EmulatedState *card, uint32_t type, +const uint8_t *data, uint32_t len) +{ +EmulEvent *event = (EmulEvent *)malloc(sizeof(EmulEvent) + len); qemu_malloc() +static void pipe_read(void *opaque) +{ +EmulatedState *card = opaque; +EmulEvent *event, *next; +char dummy; +int len; + +do { +len = read(card-pipe[0], dummy, sizeof(dummy)); +} while (len == sizeof(dummy)); Shouldn't you check error codes here? I should. Jes
Re: [Qemu-devel] [PATCH] vhost: force vhost off for non-MSI guests
Seeking clarification to the original question I posted: This maybe a novice question - Would appreciate it if you can you provide a pointer to documentation or relevant code that explains what is the *limitation in supporting level irq support in kvm irqfd.* After browsing the KVM kernel code, it does look like direct assignment of PCI devices allows support for level-triggered interrupts to be injected to the guest from the kernel. (*as opposed to not supporting it for vhost irqfd mechanism*) This occurs when the guest device supports INTX. Reference: kvm_assigned_dev_interrupt_work_handler() in assigned-dev.c calls kvm_set_irq() with the guest_irq. This function in turn invokes the assigned set function (either kvm_set_pic_irq or kvm_set_ioapic_irq) which was setup at kvm_irq_chip creation time when kvm_setup_default_irq_routing () called for handling ioctl KVM_CREATE_IRQCHIP. So, it isn't clear why level-triggered interrupt isn't supported for irqfd mechanism. Would greatly appreciate clarification here Thanks -Rukhsana
[Qemu-devel] Write cache enable from guest at runtime
Hi Christoph, I have written up thoughts on write cache enable, including a workaround to change O_SYNC on a file without kernel changes: http://wiki.qemu.org/Features/WriteCacheEnable Guest ability to control write cache enable is useful and I'd like to see it in QEMU soon. Any thoughts about what I've posted? Care to submit your patches? Stefan
Re: [Qemu-devel] [PATCH 5/7] ccid: add ccid-card-emulated device
On 03/14/11 17:44, Alon Levy wrote: On Mon, Mar 14, 2011 at 04:41:02PM +0100, Jes Sorensen wrote: +#define MAX_ATR_SIZE 40 +struct EmulatedState { +CCIDCardState base; +uint8_t debug; +char*backend_str; +uint32_t backend; +char*cert1; +char*cert2; +char*cert3; +char*db; +uint8_t atr[MAX_ATR_SIZE]; +uint8_t atr_length; +QSIMPLEQ_HEAD(event_list, EmulEvent) event_list; +pthread_mutex_t event_list_mutex; +VReader *reader; +QSIMPLEQ_HEAD(guest_apdu_list, EmulEvent) guest_apdu_list; +pthread_mutex_t vreader_mutex; /* and guest_apdu_list mutex */ +pthread_mutex_t handle_apdu_mutex; +pthread_cond_t handle_apdu_cond; +int pipe[2]; +int quit_apdu_thread; +pthread_mutex_t apdu_thread_quit_mutex; +pthread_cond_t apdu_thread_quit_cond; +}; Bad struct packing and wrong thread types. Will fix. Aside: Why do we care about packing something that has a single instance per device? isn't logical readable order more important in this case? We don't care too much - use your own judgement for what makes sense in this case. I am used to spotting those so I mention them, but I didn't actually check how often the struct was used. Cheers, Jes
[Qemu-devel] Re: Write cache enable from guest at runtime
On Mon, Mar 14, 2011 at 05:07:57PM +, Stefan Hajnoczi wrote: Hi Christoph, I have written up thoughts on write cache enable, including a workaround to change O_SYNC on a file without kernel changes: http://wiki.qemu.org/Features/WriteCacheEnable Guest ability to control write cache enable is useful and I'd like to see it in QEMU soon. Any thoughts about what I've posted? Care to submit your patches? I have patches using Prerna's close and reopen method almost ready for posting. Still fighting a bit with the new virtio code to wire it up. Unlike your variant I actually added a generic change config request type, which allows to re-use it for other runtime controllable features later.
[Qemu-devel] [PATCH] target-arm/helper.c: For float-int conversion helpers pass ints as ints
Correct the argument and return types for the float-int conversion helper functions so that integer arguments and return values are declared as uint32_t/uint64_t, not float32/float64. This allows us to remove the hand-rolled functions which were doing bitwise copies between the types via unions. Signed-off-by: Peter Maydell peter.mayd...@linaro.org --- This patch is in some ways doing for helper.c what the patch http://patchwork.ozlabs.org/patch/86441/ did for neon_helper.c, although in this case I opted to fix the prototypes not to pass integer arguments in and out in float32/float64 rather than simply replacing the helper functions with make_float*/float*_val calls. There should be no behaviour change here, it's just a code cleanup. Tested with the usual random-instruction-set stuff. target-arm/helper.c | 155 ++ target-arm/helpers.h | 60 ++-- 2 files changed, 85 insertions(+), 130 deletions(-) diff --git a/target-arm/helper.c b/target-arm/helper.c index d360121..ff9f59b 100644 --- a/target-arm/helper.c +++ b/target-arm/helper.c @@ -2486,135 +2486,90 @@ DO_VFP_cmp(s, float32) DO_VFP_cmp(d, float64) #undef DO_VFP_cmp -/* Helper routines to perform bitwise copies between float and int. */ -static inline float32 vfp_itos(uint32_t i) -{ -union { -uint32_t i; -float32 s; -} v; - -v.i = i; -return v.s; -} - -static inline uint32_t vfp_stoi(float32 s) -{ -union { -uint32_t i; -float32 s; -} v; - -v.s = s; -return v.i; -} - -static inline float64 vfp_itod(uint64_t i) -{ -union { -uint64_t i; -float64 d; -} v; - -v.i = i; -return v.d; -} - -static inline uint64_t vfp_dtoi(float64 d) -{ -union { -uint64_t i; -float64 d; -} v; - -v.d = d; -return v.i; -} - /* Integer to float conversion. */ -float32 VFP_HELPER(uito, s)(float32 x, CPUState *env) +float32 VFP_HELPER(uito, s)(uint32_t x, CPUState *env) { -return uint32_to_float32(vfp_stoi(x), env-vfp.fp_status); +return uint32_to_float32(x, env-vfp.fp_status); } -float64 VFP_HELPER(uito, d)(float32 x, CPUState *env) +float64 VFP_HELPER(uito, d)(uint32_t x, CPUState *env) { -return uint32_to_float64(vfp_stoi(x), env-vfp.fp_status); +return uint32_to_float64(x, env-vfp.fp_status); } -float32 VFP_HELPER(sito, s)(float32 x, CPUState *env) +float32 VFP_HELPER(sito, s)(uint32_t x, CPUState *env) { -return int32_to_float32(vfp_stoi(x), env-vfp.fp_status); +return int32_to_float32(x, env-vfp.fp_status); } -float64 VFP_HELPER(sito, d)(float32 x, CPUState *env) +float64 VFP_HELPER(sito, d)(uint32_t x, CPUState *env) { -return int32_to_float64(vfp_stoi(x), env-vfp.fp_status); +return int32_to_float64(x, env-vfp.fp_status); } /* Float to integer conversion. */ -float32 VFP_HELPER(toui, s)(float32 x, CPUState *env) +uint32_t VFP_HELPER(toui, s)(float32 x, CPUState *env) { if (float32_is_any_nan(x)) { -return float32_zero; +return 0; } -return vfp_itos(float32_to_uint32(x, env-vfp.fp_status)); +return float32_to_uint32(x, env-vfp.fp_status); } -float32 VFP_HELPER(toui, d)(float64 x, CPUState *env) +uint32_t VFP_HELPER(toui, d)(float64 x, CPUState *env) { if (float64_is_any_nan(x)) { -return float32_zero; +return 0; } -return vfp_itos(float64_to_uint32(x, env-vfp.fp_status)); +return float64_to_uint32(x, env-vfp.fp_status); } -float32 VFP_HELPER(tosi, s)(float32 x, CPUState *env) +uint32_t VFP_HELPER(tosi, s)(float32 x, CPUState *env) { if (float32_is_any_nan(x)) { -return float32_zero; +return 0; } -return vfp_itos(float32_to_int32(x, env-vfp.fp_status)); +return float32_to_int32(x, env-vfp.fp_status); } -float32 VFP_HELPER(tosi, d)(float64 x, CPUState *env) +uint32_t VFP_HELPER(tosi, d)(float64 x, CPUState *env) { if (float64_is_any_nan(x)) { -return float32_zero; +return 0; } -return vfp_itos(float64_to_int32(x, env-vfp.fp_status)); +return float64_to_int32(x, env-vfp.fp_status); } -float32 VFP_HELPER(touiz, s)(float32 x, CPUState *env) +uint32_t VFP_HELPER(touiz, s)(float32 x, CPUState *env) { if (float32_is_any_nan(x)) { -return float32_zero; +return 0; } -return vfp_itos(float32_to_uint32_round_to_zero(x, env-vfp.fp_status)); +return float32_to_uint32_round_to_zero(x, env-vfp.fp_status); } -float32 VFP_HELPER(touiz, d)(float64 x, CPUState *env) +uint32_t VFP_HELPER(touiz, d)(float64 x, CPUState *env) { if (float64_is_any_nan(x)) { -return float32_zero; +return 0; } -return vfp_itos(float64_to_uint32_round_to_zero(x, env-vfp.fp_status)); +return float64_to_uint32_round_to_zero(x, env-vfp.fp_status); } -float32 VFP_HELPER(tosiz, s)(float32 x, CPUState *env) +uint32_t VFP_HELPER(tosiz, s)(float32 x,
[Qemu-devel] [RFC] QCFG: a new mechanism to replace QemuOpts and option handling
As I've been waiting for QAPI review, I've been working on the design of a new mechanism to replace our current command line option handling (QemuOpts) with something that reuses the QAPI infrastructure. The 'QemuOpts' syntax is just a way to encode complex data structures. 'nic,model=virtio,macaddress=00:01:02:03:04:05' can be mapped directly to a C data structure. This is exactly what QCFG does using the same JSON schema mechanism that QMP uses. The effect is that you describe a command line argument in JSON like so: { 'type': 'VncConfig', 'data': { 'address': 'str', '*password': 'bool', '*reverse': 'bool', '*no-lock-key-sync': 'bool', '*sasl': 'bool', '*tls': 'bool', '*x509': 'str', '*x509verify': 'str', '*acl': 'bool', '*lossy': 'bool' } } You then just implement a C function that gets called for each -vnc option specified: void qcfg_handle_vnc(VncConfig *option, Error **errp) { } And that's it. You can squirrel away the option such that they all can be processed later, you can perform additional validation and return an error, or you can implement the appropriate logic. The VncConfig structure is a proper C data structure. The advantages of this approach compared to QemuOpts are similar to QAPI: 1) Strong typing means less bugs with lack of command line validation. In many cases, a bad command line results in a SEGV today. 2) Every option is formally specified and documented in a way that is both rigorous and machine readable. This means we can generate high quality documentation in a variety of formats. 3) The command line parameters support full introspection. This should provide the same functionality as Dan's earlier introspection patches. 4) The 'VncConfig' structure also has JSON marshallers and the qcfg_handle_vnc() function can be trivially bridged to QMP. This means command line oriented interfaces (like device_add) are better integrated with QMP. 5) Very complex data types can be implemented. We had some discussion of supporting nested structures with -blockdev. This wouldn't work with QemuOpts but I've already implemented it with QCFG (blockdev syntax is my test case right now). The syntax I'm currently using is -blockdev cache=none,id=foo,format.qcow.protocol.nbd.hostname=localhost where '.' is used to reference sub structures. 6) Unions are supported which means complex options like -net can be specified in the schema and don't require hand validation. I've got a spec written up at http://wiki.qemu.org/Features/QCFG. Initial code is in my QAPI tree. I'm not going to start converting things until we get closer to the end of 0.15 and QAPI is fully merged. My plan is to focus on this for 0.16 and do a full conversion for the 0.16 time frame using the same approach as QAPI. That means that for 0.16, we would be able to set all command line options via QMP in a programmatic fashion with full support for introspection. I'm haven't yet closed on how to bridge this to qdev. qdev is a big consumer of QemuOpts today. I have some general ideas about what I'd like to do but so far, I haven't written anything down. I wanted to share these plans early hoping to get some feedback and also to maybe interest some folks in helping out. Regards, Anthony Liguori
Re: [Qemu-devel] Re: KVM call agenda for Mars 14th
On 03/14/2011 11:36 AM, Juan Quintela wrote: Jes Sorensenjes.soren...@redhat.com wrote: On 03/14/11 13:14, Juan Quintela wrote: Please send any agenda items you are interested in covering. Thanks, Juan. I presume you mean for March 15? Today is the 14th and it is Monday :) Dunno what calendar I looked to :p Yes, you are right. - QCFG, http://wiki.qemu.org/Features/QCFG Regards, Anthony Liguori
Re: [Qemu-devel] Re: Strategic decision: COW format
Am 14.03.2011 17:32, schrieb Chunqiang Tang: FVD's novel uses of the reference count table reduces the metadata update overhead down to literally zero during normal execution of a VM. This gets the bests of QCOW2's reference count table but without its oeverhead. In FVD, the reference count table is only updated when creating a new snapshot or deleting an existing snapshot. The reference count table is never updated during normal execution of a VM. Do you want to send out a break-down of the steps (and cost) involved in doing: 1. Snapshot creation. 2. Snapshot deletion. 3. Opening an image with n snapshots. Here is a detailed description. Relevant to the discussion of snapshot, FVD uses a one-level lookup table and a refcount table. FVD’s one-level lookup table is very similar to QCOW2’s two-level lookup table, except that it is much smaller in FVD, and is preallocated and hence contiguous in image. Does this mean that FVD can't hold VM state of arbitrary size? FVD’s refcount table is almost identical to that of QCOW2, but with a key difference. An image consists of an arbitrary number of read-only snapshots, and a single writeable image front, which is the current image state perceived by the VM. Below, I will simply refer to the read-only snapshots as snapshots, and refer to the “writeable image front” as “writeable-front.” QCOW2’s refcount table counts clusters that are used by either read-only snapshots or writeable-front. Because writeable-front changes as the VM runs, QCOW2 needs to update the refcount table on the fast path of normal VM execution. Needs to update, but not necessarily on the fast path. Updates can be delayed and batched. Kevin
[Qemu-devel] Windows 7 on pure qemu-0.14
Hi to alll, I'm trying to run Windows7 64 on pure qemu-0.14 ( no kvm/xen) I compiled qemu with the following : .configure --disable-kvm --enable-vnc-thread --target-list=x86_64-softmmu Running qemu-system-x86_64 -m 2030 -boot c -cdrom /win7_64.iso -hda win7_64.img gives me a BSOD with this error: Stop: 0x005D (0x78BFBF9,0x,0x,0x Is there a way to solve this issue? Any hint is appreciated, thanks a lot in advance Marco
Re: [Qemu-devel] [PATCH] vhost: force vhost off for non-MSI guests
On Mon, Mar 14, 2011 at 10:35:08PM +0530, rukhsana ansari wrote: Seeking clarification to the original question I posted: This maybe a novice question - Would appreciate it if you can you provide a pointer to documentation or relevant code that explains what is the limitation in supporting level irq support in kvm irqfd. After browsing the KVM kernel code, it does look like direct assignment of PCI devices allows support for level-triggered interrupts to be injected to the guest from the kernel. (as opposed to not supporting it for vhost irqfd mechanism) This occurs when the guest device supports INTX. Reference: kvm_assigned_dev_interrupt_work_handler() in assigned-dev.c calls kvm_set_irq() with the guest_irq. This function in turn invokes the assigned set function (either kvm_set_pic_irq or kvm_set_ioapic_irq) which was setup at kvm_irq_chip creation time when kvm_setup_default_irq_routing () called for handling ioctl KVM_CREATE_IRQCHIP. So, it isn't clear why level-triggered interrupt isn't supported for irqfd mechanism. Would greatly appreciate clarification here Thanks -Rukhsana Mostly, no one came up with an implementation so far. If the point is to use irqfd with vhost-net, there's also a question of adding interfaces to 1. pass IO read transactions directly to another kernel module 2. add an interface to clear the irq level Maybe the right thing is to combine the two somehow: irqfd might get an oiption to set a bit in memory, ioeventfd might get an option to read and clear from memory and clear irqfd line at the same time. -- MST
Re: [Qemu-devel] Windows 7 on pure qemu-0.14
On Mon, Mar 14, 2011 at 07:19:29PM +0100, Marco Cianfriglia wrote: Hi to alll, I'm trying to run Windows7 64 on pure qemu-0.14 ( no kvm/xen) I compiled qemu with the following : .configure --disable-kvm --enable-vnc-thread --target-list=x86_64-softmmu Running qemu-system-x86_64 -m 2030 -boot c -cdrom /win7_64.iso -hda win7_64.img gives me a BSOD with this error: Stop: 0x005D (0x78BFBF9,0x,0x,0x Is there a way to solve this issue? Any hint is appreciated, thanks a lot in advance Marco Try adding -cpu Nehalem. -- Gleb.
[Qemu-devel] Re: Write cache enable from guest at runtime
On Mon, Mar 14, 2011 at 5:16 PM, Christoph Hellwig h...@lst.de wrote: On Mon, Mar 14, 2011 at 05:07:57PM +, Stefan Hajnoczi wrote: Hi Christoph, I have written up thoughts on write cache enable, including a workaround to change O_SYNC on a file without kernel changes: http://wiki.qemu.org/Features/WriteCacheEnable Guest ability to control write cache enable is useful and I'd like to see it in QEMU soon. Any thoughts about what I've posted? Care to submit your patches? I have patches using Prerna's close and reopen method almost ready for posting. Still fighting a bit with the new virtio code to wire it up. Unlike your variant I actually added a generic change config request type, which allows to re-use it for other runtime controllable features later. Sounds like a good idea. Feel free to post the patches RFC and I or someone else can debug and polish them if you don't have time. Stefan
[Qemu-devel] Re: [PATCH 02/11] qerror: expose a function to format an error
On Fri, 11 Mar 2011 15:08:38 -0600 Anthony Liguori aligu...@us.ibm.com wrote: On 03/11/2011 03:00 PM, Anthony Liguori wrote: This will let Error share the QError human formatting. This is only used for HMP. Signed-off-by: Anthony Liguorialigu...@us.ibm.com diff --git a/qerror.c b/qerror.c index 4855604..13d53c9 100644 --- a/qerror.c +++ b/qerror.c @@ -326,12 +326,18 @@ QError *qerror_from_info(const char *file, int linenr, const char *func, return qerr; } -static void parse_error(const QError *qerror, int c) +static void parse_error(const QErrorStringTable *entry, int c) { -qerror_abort(qerror, expected '%c' in '%s', c, qerror-entry-desc); +#if 0 +qerror_abort(qerror, expected '%c' in '%s', c, entry-desc); +#else +fprintf(stderr, expected '%c' in '%s', c, entry-desc); +abort(); +#endif } Err, I shouldn't have left these #if 0's in here. Please ignore them. But you're going to keep qerror_abort() usage, right? Regards, Anthony Liguori -static const char *append_field(QString *outstr, const QError *qerror, +static const char *append_field(QDict *error, QString *outstr, +const QErrorStringTable *entry, const char *start) { QObject *obj; @@ -339,24 +345,31 @@ static const char *append_field(QString *outstr, const QError *qerror, QString *key_qs; const char *end, *key; -if (*start != '%') -parse_error(qerror, '%'); +if (*start != '%') { +parse_error(entry, '%'); +} start++; -if (*start != '(') -parse_error(qerror, '('); +if (*start != '(') { +parse_error(entry, '('); +} start++; end = strchr(start, ')'); -if (!end) -parse_error(qerror, ')'); +if (!end) { +parse_error(entry, ')'); +} key_qs = qstring_from_substr(start, 0, end - start - 1); key = qstring_get_str(key_qs); -qdict = qobject_to_qdict(qdict_get(qerror-error, data)); +qdict = qobject_to_qdict(qdict_get(error, data)); obj = qdict_get(qdict, key); if (!obj) { +#if 0 qerror_abort(qerror, key '%s' not found in QDict, key); +#else +abort(); +#endif } switch (qobject_type(obj)) { @@ -367,41 +380,66 @@ static const char *append_field(QString *outstr, const QError *qerror, qstring_append_int(outstr, qdict_get_int(qdict, key)); break; default: +#if 0 qerror_abort(qerror, invalid type '%c', qobject_type(obj)); +#else +abort(); +#endif } QDECREF(key_qs); return ++end; } -/** - * qerror_human(): Format QError data into human-readable string. - * - * Formats according to member 'desc' of the specified QError object. - */ -QString *qerror_human(const QError *qerror) +static QString *qerror_format_desc(QDict *error, + const QErrorStringTable *entry) { -const char *p; QString *qstring; +const char *p; -assert(qerror-entry != NULL); +assert(entry != NULL); qstring = qstring_new(); -for (p = qerror-entry-desc; *p != '\0';) { +for (p = entry-desc; *p != '\0';) { if (*p != '%') { qstring_append_chr(qstring, *p++); } else if (*(p + 1) == '%') { qstring_append_chr(qstring, '%'); p += 2; } else { -p = append_field(qstring, qerror, p); +p = append_field(error, qstring, entry, p); } } return qstring; } +QString *qerror_format(const char *fmt, QDict *error) +{ +const QErrorStringTable *entry = NULL; +int i; + +for (i = 0; qerror_table[i].error_fmt; i++) { +if (strcmp(qerror_table[i].error_fmt, fmt) == 0) { +entry =qerror_table[i]; +break; +} +} + +return qerror_format_desc(error, entry); +} + +/** + * qerror_human(): Format QError data into human-readable string. + * + * Formats according to member 'desc' of the specified QError object. + */ +QString *qerror_human(const QError *qerror) +{ +return qerror_format_desc(qerror-error, qerror-entry); +} + /** * qerror_print(): Print QError data * diff --git a/qerror.h b/qerror.h index f732d45..fd63ee9 100644 --- a/qerror.h +++ b/qerror.h @@ -42,6 +42,7 @@ void qerror_report_internal(const char *file, int linenr, const char *func, #define qerror_report(fmt, ...) \ qerror_report_internal(__FILE__, __LINE__, __func__, fmt, ## __VA_ARGS__) QError *qobject_to_qerror(const QObject *obj); +QString *qerror_format(const char *fmt, QDict *error); /* * QError class list
Re: [Qemu-devel] Re: qemu-0.14.0 doesn't compile on ppc32
On Mon, Mar 14, 2011 at 12:33 PM, Stefan Hajnoczi stefa...@gmail.com wrote: On Mon, Mar 14, 2011 at 10:23 AM, Peter Maydell peter.mayd...@linaro.org wrote: On 21 February 2011 08:10, Paolo Bonzini pbonz...@redhat.com wrote: On 02/20/2011 06:32 PM, Peter Maydell wrote: Some of qemu's code does seem to trigger rather excessive memory use by gcc; for instance we've had problems with memory usage building for ARM with gcc of target-sparc/translate.c wanting gigabytes of RAM with some compiler flags: https://bugs.launchpad.net/gcc-linaro/+bug/714921 I suspect it's all those large switch statements... It's on my todo list to report it to GCC, since this memory-hog behavior is a GCC regression. We've been working on this in Linaro, and the following two patches have been posted upstream: http://gcc.gnu.org/ml/gcc-patches/2011-03/msg00193.html http://gcc.gnu.org/ml/gcc-patches/2011-03/msg00255.html so you can probably cross that item off your todo list :-) Interesting. Alexander Graf and I recently hit an out of memory when building translate.o on ppc64 host. It was solved by adding more RAM to the box but this patch looks nice too ;). Maybe the switch blocks should be rearranged, or inline attribute removed from the functions. Are the nested switch and if statements optimal from performance point of view or could there be a faster and less memory hungry version?
Re: [Qemu-devel] [PATCHv2] report that QEMU process was killed by a signal
On Mon, Mar 14, 2011 at 3:44 PM, Gleb Natapov g...@redhat.com wrote: Currently when rogue script kills QEMU process (using TERM/INT/HUP signal) it looks indistinguishable from system shutdown. Lets report that QMEU was killed and leave some clues about the killed identity. QEMU ;-) Signed-off-by: Gleb Natapov g...@redhat.com --- v1-v2: - print message from a main loop instead of signal handler diff --git a/os-posix.c b/os-posix.c index 38c29d1..36d937c 100644 --- a/os-posix.c +++ b/os-posix.c @@ -61,9 +61,9 @@ void os_setup_early_signal_handling(void) sigaction(SIGPIPE, act, NULL); } -static void termsig_handler(int signal) +static void termsig_handler(int signal, siginfo_t *info, void *c) { - qemu_system_shutdown_request(); + qemu_system_killed(info-si_signo, info-si_pid); } static void sigchld_handler(int signal) @@ -76,7 +76,8 @@ void os_setup_signal_handling(void) struct sigaction act; memset(act, 0, sizeof(act)); - act.sa_handler = termsig_handler; + act.sa_sigaction = termsig_handler; + act.sa_flags = SA_SIGINFO; sigaction(SIGINT, act, NULL); sigaction(SIGHUP, act, NULL); sigaction(SIGTERM, act, NULL); diff --git a/sysemu.h b/sysemu.h index 0a83ab9..fc7048a 100644 --- a/sysemu.h +++ b/sysemu.h @@ -66,6 +66,8 @@ void qemu_system_vmstop_request(int reason); int qemu_shutdown_requested(void); int qemu_reset_requested(void); int qemu_powerdown_requested(void); +void qemu_system_killed(int signal, int pid); pid_t pid +void qemu_kill_report(void); extern qemu_irq qemu_system_powerdown; void qemu_system_reset(void); diff --git a/vl.c b/vl.c index 5e007a7..d14c42d 100644 --- a/vl.c +++ b/vl.c @@ -1213,7 +1213,7 @@ typedef struct QEMUResetEntry { static QTAILQ_HEAD(reset_handlers, QEMUResetEntry) reset_handlers = QTAILQ_HEAD_INITIALIZER(reset_handlers); static int reset_requested; -static int shutdown_requested; +static int shutdown_requested, shutdown_signal = -1, shutdown_pid; static pid_t shutdown_pid; static int powerdown_requested; static int debug_requested; static int vmstop_requested; @@ -1225,6 +1225,15 @@ int qemu_shutdown_requested(void) return r; } +void qemu_kill_report(void) +{ + if (shutdown_signal != -1) { + fprintf(stderr, Got signal %d from pid %d\n, Are there any systems with 64 bit pid_t? Then a cast may be needed and/or format adjusted. + shutdown_signal, shutdown_pid); + shutdown_signal = -1; + } +} + int qemu_reset_requested(void) { int r = reset_requested; @@ -1298,6 +1307,13 @@ void qemu_system_reset_request(void) qemu_notify_event(); } +void qemu_system_killed(int signal, int pid) pid_t pid
Re: [Qemu-devel] Upgrading Seabios to 0.6.2
On Tue, Mar 08, 2011 at 04:02:32PM +0200, Gleb Natapov wrote: On Tue, Mar 08, 2011 at 07:56:56AM -0600, Anthony Liguori wrote: On 03/08/2011 05:53 AM, Gleb Natapov wrote: Seabios 0.6.2 is finally released. What about upgrading QEMU to it and also applying this http://patchwork.ozlabs.org/patch/81099/? Should we just move to the latest git? I don't mind tracking git during development provided that we land on a stable release by the time we release. Sounds good to me. It just requires someone to do a little smoke testing on the particular git snapshots to make sure things aren't horribly regressing. I often run guests with latest seabios git, not sure it can count as smoke testing. But since we will do it in development tree only, if something breaks we will fix it. That is what development tree is for :) Better to catch breakage earlier than later. Any news about this? -- Gleb.
[Qemu-devel] Re: [PATCH 04/11] qerror: split out the reporting bits of QError
On 03/14/2011 02:18 PM, Luiz Capitulino wrote: On Fri, 11 Mar 2011 15:00:42 -0600 Anthony Liguorialigu...@us.ibm.com wrote: These make it very hard to compile QError outside of QEMU. Why would someone do this? libqmp It's very nice to have client code that convert QMP errors to human readable strings. Regards, Anthony Liguori
[Qemu-devel] Re: [PATCH 09/11] json-lexer: limit the maximum size of a given token
On Fri, 11 Mar 2011 15:00:47 -0600 Anthony Liguori aligu...@us.ibm.com wrote: This is a security consideration. We don't want a client to cause an arbitrary amount of memory to be allocated in QEMU. For now, we use a limit of 64MB which should be large enough for any reasonably sized token. This is important for parsing JSON from untrusted sources. Signed-off-by: Anthony Liguori aligu...@us.ibm.com diff --git a/json-lexer.c b/json-lexer.c index 834d7af..3462c89 100644 --- a/json-lexer.c +++ b/json-lexer.c @@ -18,6 +18,8 @@ #include qemu-common.h #include json-lexer.h +#define MAX_TOKEN_SIZE (64ULL 20) + /* * \([^\\\]|(\'\\/\\b\\f\\n\\r\\t\\u[0-9a-fA-F][0-9a-fA-F][0-9a-fA-F][0-9a-fA-F]))*\ * '([^\\']|(\'\\/\\b\\f\\n\\r\\t\\u[0-9a-fA-F][0-9a-fA-F][0-9a-fA-F][0-9a-fA-F]))*' @@ -312,6 +314,17 @@ static int json_lexer_feed_char(JSONLexer *lexer, char ch) } lexer-state = new_state; } while (!char_consumed); + +/* Do not let a single token grow to an arbitrarily large size, + * this is a security consideration. + */ +if (lexer-token-length MAX_TOKEN_SIZE) { +lexer-emit(lexer, lexer-token, lexer-state, lexer-x, lexer-y); +QDECREF(lexer-token); +lexer-token = qstring_new(); +lexer-state = IN_START; +} Entering an invalid token is an error, we should fail here. Which brings two features: 1. A test code could trigger this condition and check for the specific error code 2. Developers will know when they hit the limit. Although I don't expect expect this to happen, there was talking about adding base64 support to transfer something (I can't remember what, but we never know how the protocol will evolve). Also, by testing this I found that the parser seems to get confused when the limit is reached: it stops responding. + return 0; }
Re: [Qemu-devel] Re: [PATCH 02/11] qerror: expose a function to format an error
On 03/14/2011 02:17 PM, Luiz Capitulino wrote: On Fri, 11 Mar 2011 15:08:38 -0600 Anthony Liguorialigu...@us.ibm.com wrote: On 03/11/2011 03:00 PM, Anthony Liguori wrote: This will let Error share the QError human formatting. This is only used for HMP. Signed-off-by: Anthony Liguorialigu...@us.ibm.com diff --git a/qerror.c b/qerror.c index 4855604..13d53c9 100644 --- a/qerror.c +++ b/qerror.c @@ -326,12 +326,18 @@ QError *qerror_from_info(const char *file, int linenr, const char *func, return qerr; } -static void parse_error(const QError *qerror, int c) +static void parse_error(const QErrorStringTable *entry, int c) { -qerror_abort(qerror, expected '%c' in '%s', c, qerror-entry-desc); +#if 0 +qerror_abort(qerror, expected '%c' in '%s', c, entry-desc); +#else +fprintf(stderr, expected '%c' in '%s', c, entry-desc); +abort(); +#endif } Err, I shouldn't have left these #if 0's in here. Please ignore them. But you're going to keep qerror_abort() usage, right? No, qerror_abort() needs to go away. It's too tied to QError and this patch is making the formatting code work outside of of QEMU. Once this whole series is completely merged, QError goes away entirely and this pretty formatting is replaced with something much simpler. Regards, Anthony Liguori