Re: [Qemu-devel] [PATCH V2] qemu-img: use QemuOpts instead of QEMUOptionParameter in resize function
On Mon, Aug 6, 2012 at 3:18 AM, Dong Xu Wang wrote: > > Signed-off-by: Dong Xu Wang > --- > v1->v2: fix param leak. > > qemu-img.c | 28 +--- > 1 files changed, 17 insertions(+), 11 deletions(-) Reviewed-by: Stefan Hajnoczi
Re: [Qemu-devel] [untested PATCH] virtio: fix vhost handling
On Fri, Aug 3, 2012 at 5:16 PM, Paolo Bonzini wrote: > Commit b1f416aa8d870fab71030abc9401cfc77b948e8e breaks vhost_net > because it always registers the virtio_pci_host_notifier_read() handler > function on the ioeventfd, even when vhost_net.ko is using the ioeventfd. > The result is both QEMU and vhost_net.ko polling on the same eventfd > and the virtio_net.ko guest driver seeing inconsistent results: > > # ifconfig eth0 192.168.0.1 netmask 255.255.255.0 > virtio_net virtio0: output:id 0 is not a head! > > To fix this, proceed the same as we do for irqfd: add a parameter to > virtio_queue_set_host_notifier_fd_handler and in that case only set > the notifier, not the handler > > Cc: Stefan Hajnoczi > Signed-off-by: Paolo Bonzini > --- > Interesting, I tested vhost (or thought so). Can you try this > patch instead? Does this really make the code better than just reverting the patch? Tested-by: Stefan Hajnoczi > > hw/virtio-pci.c | 14 +++--- > hw/virtio.c |7 +-- > hw/virtio.h |3 ++- > 3 files changed, 14 insertions(+), 10 deletions(-) > > diff --git a/hw/virtio-pci.c b/hw/virtio-pci.c > index 3ab9747..6133626 100644 > --- a/hw/virtio-pci.c > +++ b/hw/virtio-pci.c > @@ -160,7 +160,7 @@ static int virtio_pci_load_queue(void * opaque, int n, > QEMUFile *f) > } > > static int virtio_pci_set_host_notifier_internal(VirtIOPCIProxy *proxy, > - int n, bool assign) > + int n, bool assign, bool > with_vhost) I don't like this name because virtio-blk-data-plane also wants to use the ioeventfd. I suggest we call it use_handler (note logic is reversed from with_vhost). Stefan
Re: [Qemu-devel] [PATCH master/stable] virtio-mlk: fix use-after-free while handling scsi commands
On Mon, Aug 6, 2012 at 1:49 PM, Avi Kivity wrote: > The scsi passthrough handler falls through after completing a > request into the failure path, resulting in a use after free. > > Reprducible by running a guest with aio=native on a block device. > > Reported-by: Stefan Priebe > Signed-off-by: Avi Kivity > --- > hw/virtio-blk.c | 1 + > 1 file changed, 1 insertion(+) It would be nice to fix up the commit message: s/virtio-mlk/virtio-blk/ Reviewed-by: Stefan Hajnoczi
Re: [Qemu-devel] [PATCH] virtio: fix vhost handling
On Mon, Aug 6, 2012 at 2:26 PM, Paolo Bonzini wrote: > Commit b1f416aa8d870fab71030abc9401cfc77b948e8e breaks vhost_net > because it always registers the virtio_pci_host_notifier_read() handler > function on the ioeventfd, even when vhost_net.ko is using the ioeventfd. > The result is both QEMU and vhost_net.ko polling on the same eventfd > and the virtio_net.ko guest driver seeing inconsistent results: > > # ifconfig eth0 192.168.0.1 netmask 255.255.255.0 > virtio_net virtio0: output:id 0 is not a head! > > To fix this, proceed the same as we do for irqfd: add a parameter to > virtio_queue_set_host_notifier_fd_handler and in that case only set > the notifier, not the handler. > > Cc: Stefan Hajnoczi > Signed-off-by: Paolo Bonzini > --- > hw/virtio-pci.c | 14 +++--- > hw/virtio.c | 7 +-- > hw/virtio.h | 3 ++- > 3 file modificati, 14 inserzioni(+), 10 rimozioni(-) Internationalized diff stat, I never noticed... :) Tested-by: Stefan Hajnoczi Reviewed-by: Stefan Hajnoczi
Re: [Qemu-devel] qemu-kvm-1.0.1 - unable to exit if vcpu is in infinite loop
On Thu, Jun 28, 2012 at 2:05 PM, Peter Lieven wrote: > i debugged my initial problem further and found out that the problem happens > to be that > the main thread is stuck in pause_all_vcpus() on reset or quit commands in > the monitor > if one cpu is stuck in the do-while loop kvm_cpu_exec. If I modify the > condition from while (ret == 0) > to while ((ret == 0) && !env->stop); it works, but is this the right fix? > "Quit" command seems to work, but on "Reset" the VM enterns pause state. I think I'm hitting something similar. I installed a F17 amd64 guest (3.5 kernel) but before booting entered the GRUB boot menu edit mode. The guest seemed unresponsive so I switched to the monitor, which also froze shortly afterwards. The VNC screen ended up being all black. qemu-kvm.git/master 3e4305694fd891b69e4450e59ec4c65420907ede Linux 3.2.0-3-amd64 from Debian testing $ qemu-system-x86_64 -enable-kvm -m 1024 -smp 2 -drive if=virtio,cache=none,file=f17.img,aio=native -serial stdio (gdb) thread apply all bt Thread 3 (Thread 0x7f8008e23700 (LWP 367)): #0 0x7f800f891727 in ioctl () at ../sysdeps/unix/syscall-template.S:82 #1 0x7f80137b92c9 in kvm_vcpu_ioctl (env=env@entry=0x7f8015b49640, type=type@entry=44672) at /home/stefanha/qemu-kvm/kvm-all.c:1619 #2 0x7f80137b93fe in kvm_cpu_exec (env=env@entry=0x7f8015b49640) at /home/stefanha/qemu-kvm/kvm-all.c:1506 #3 0x7f8013766f31 in qemu_kvm_cpu_thread_fn (arg=0x7f8015b49640) at /home/stefanha/qemu-kvm/cpus.c:756 #4 0x7f800fb4db50 in start_thread (arg=) at pthread_create.c:304 #5 0x7f800f8986dd in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:112 #6 0x in ?? () This vcpu is still executing guest code and I've seen it successfully dispatching I/O. The problem is it's missing the exit_request... Thread 2 (Thread 0x7f8008622700 (LWP 368)): #0 pthread_cond_wait@@GLIBC_2.3.2 () at ../nptl/sysdeps/unix/sysv/linux/x86_64/pthread_cond_wait.S:162 #1 0x7f801372b229 in qemu_cond_wait (cond=, mutex=mutex@entry=0x7f80144367c0) at qemu-thread-posix.c:113 #2 0x7f8013766eff in qemu_kvm_wait_io_event (env=) at /home/stefanha/qemu-kvm/cpus.c:724 #3 qemu_kvm_cpu_thread_fn (arg=0x7f8015b67450) at /home/stefanha/qemu-kvm/cpus.c:761 #4 0x7f800fb4db50 in start_thread (arg=) at pthread_create.c:304 #5 0x7f800f8986dd in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:112 #6 0x in ?? () No problems here. Thread 1 (Thread 0x7f801347b8c0 (LWP 365)): #0 pthread_cond_wait@@GLIBC_2.3.2 () at ../nptl/sysdeps/unix/sysv/linux/x86_64/pthread_cond_wait.S:162 #1 0x7f801372b229 in qemu_cond_wait (cond=cond@entry=0x7f801402fd80, mutex=mutex@entry=0x7f80144367c0) at qemu-thread-posix.c:113 #2 0x7f8013768949 in pause_all_vcpus () at /home/stefanha/qemu-kvm/cpus.c:962 #3 0x7f80136028c8 in main (argc=, argv=, envp=) at /home/stefanha/qemu-kvm/vl.c:3695 We're deadlocked in pause_all_vcpus(), waiting for vcpu #0 to pause. Unfortunately vcpu #0 has ->exit_request=0 although ->stop=1. Here are the vcpus: (gdb) p first_cpu $6 = (struct CPUX86State *) 0x7f8015b49640 (gdb) p first_cpu->next_cpu $7 = (struct CPUX86State *) 0x7f8015b67450 (gdb) p first_cpu->next_cpu->next_cpu $8 = (struct CPUX86State *) 0x0 (gdb) p first_cpu->stop $9 = 1 (gdb) p first_cpu->stopped $10 = 0 (gdb) p first_cpu->exit_request $11 = 0 :( This isn't easy to reproduce. I tried entering the GRUB boot menu again and there was no deadlock. Stefan
Re: [Qemu-devel] [PATCH] virtio: fix vhost handling
On Mon, Aug 6, 2012 at 3:56 PM, Paolo Bonzini wrote: > Commit b1f416aa8d870fab71030abc9401cfc77b948e8e breaks vhost_net > because it always registers the virtio_pci_host_notifier_read() handler > function on the ioeventfd, even when vhost_net.ko is using the ioeventfd. > The result is both QEMU and vhost_net.ko polling on the same eventfd > and the virtio_net.ko guest driver seeing inconsistent results: > > # ifconfig eth0 192.168.0.1 netmask 255.255.255.0 > virtio_net virtio0: output:id 0 is not a head! > > To fix this, proceed the same as we do for irqfd: add a parameter to > virtio_queue_set_host_notifier_fd_handler and in that case only set > the notifier, not the handler. > > Cc: Stefan Hajnoczi > Signed-off-by: Paolo Bonzini > --- > The patch is a bit different from what I posted before, > so I didn't add Stefan's *-by tags. > > hw/virtio-pci.c | 14 +++--- > hw/virtio.c | 7 +-- > hw/virtio.h | 3 ++- > 3 file modificati, 14 inserzioni(+), 10 rimozioni(-) Tested-by: Stefan Hajnoczi Reviewed-by: Stefan Hajnoczi
Re: [Qemu-devel] [PATCH v6 2/6] qapi: Introduce add-fd, remove-fd, query-fdsets
On Fri, Aug 3, 2012 at 6:28 PM, Corey Bryant wrote:
> diff --git a/monitor.c b/monitor.c
> index 49dccfe..9aa9f7e 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -140,6 +140,24 @@ struct mon_fd_t {
> QLIST_ENTRY(mon_fd_t) next;
> };
>
> +/* file descriptor associated with a file descriptor set */
> +typedef struct mon_fdset_fd_t mon_fdset_fd_t;
> +struct mon_fdset_fd_t {
QEMU coding style is:
typedef struct MonFdsetFd MonFdsetFd;
struct MonFdsetFd {
See ./CODING_STYLE for more info.
> +int fd;
> +bool removed;
> +QLIST_ENTRY(mon_fdset_fd_t) next;
> +};
> +
> +/* file descriptor set containing fds passed via SCM_RIGHTS */
> +typedef struct mon_fdset_t mon_fdset_t;
> +struct mon_fdset_t {
> +int64_t id;
> +int refcount;
> +bool in_use;
> +QLIST_HEAD(, mon_fdset_fd_t) fds;
> +QLIST_ENTRY(mon_fdset_t) next;
> +};
At this point in the patch series it's not clear to me whether the
removed and refcount/in_use fields are a clean and correct solution.
Exposing these fields via QMP is also something I'm going to carefully
review because they seem like internals.
> +
> typedef struct MonitorControl {
> QObject *id;
> JSONMessageParser parser;
> @@ -176,7 +194,8 @@ struct Monitor {
> int print_calls_nr;
> #endif
> QError *error;
> -QLIST_HEAD(,mon_fd_t) fds;
> +QLIST_HEAD(, mon_fd_t) fds;
> +QLIST_HEAD(, mon_fdset_t) fdsets;
> QLIST_ENTRY(Monitor) entry;
> };
>
> @@ -2389,6 +2408,157 @@ int monitor_get_fd(Monitor *mon, const char *fdname)
> return -1;
> }
>
> +static void monitor_fdset_cleanup(mon_fdset_t *mon_fdset)
> +{
> +mon_fdset_fd_t *mon_fdset_fd;
> +mon_fdset_fd_t *mon_fdset_fd_next;
> +
> +if (mon_fdset->refcount != 0) {
> +return;
> +}
> +
> +QLIST_FOREACH_SAFE(mon_fdset_fd, &mon_fdset->fds, next,
> mon_fdset_fd_next) {
> +if (!mon_fdset->in_use || mon_fdset_fd->removed) {
> +close(mon_fdset_fd->fd);
> +QLIST_REMOVE(mon_fdset_fd, next);
> +g_free(mon_fdset_fd);
> +}
> +}
> +
> +if (QLIST_EMPTY(&mon_fdset->fds)) {
> +QLIST_REMOVE(mon_fdset, next);
> +g_free(mon_fdset);
> +}
> +}
> +
> +AddfdInfo *qmp_add_fd(bool has_fdset_id, int64_t fdset_id, Error **errp)
> +{
> +int fd;
> +Monitor *mon = cur_mon;
> +mon_fdset_t *mon_fdset;
> +mon_fdset_fd_t *mon_fdset_fd;
> +AddfdInfo *fdinfo;
> +
> +fd = qemu_chr_fe_get_msgfd(mon->chr);
> +if (fd == -1) {
> +qerror_report(QERR_FD_NOT_SUPPLIED);
> +return NULL;
> +}
> +
> +if (has_fdset_id) {
> +QLIST_FOREACH(mon_fdset, &mon->fdsets, next) {
> +if (mon_fdset->id == fdset_id) {
> +break;
> +}
> +}
> +if (mon_fdset == NULL) {
> +qerror_report(QERR_FDSET_NOT_FOUND, fdset_id);
> +return NULL;
fd is leaked?
> +}
> +} else {
> +int64_t fdset_id_prev = -1;
> +mon_fdset_t *mon_fdset_cur = QLIST_FIRST(&mon->fdsets);
> +
> +/* Use first available fdset ID */
> +QLIST_FOREACH(mon_fdset, &mon->fdsets, next) {
> +mon_fdset_cur = mon_fdset;
> +if (fdset_id_prev == mon_fdset_cur->id - 1) {
> +fdset_id_prev = mon_fdset_cur->id;
> +continue;
> +}
> +break;
> +}
> +
> +mon_fdset = g_malloc0(sizeof(*mon_fdset));
> +mon_fdset->id = fdset_id_prev + 1;
> +mon_fdset->refcount = 0;
> +mon_fdset->in_use = true;
> +
> +/* The fdset list is ordered by fdset ID */
> +if (mon_fdset->id == 0) {
> +QLIST_INSERT_HEAD(&mon->fdsets, mon_fdset, next);
> +} else if (mon_fdset->id < mon_fdset_cur->id) {
> +QLIST_INSERT_BEFORE(mon_fdset_cur, mon_fdset, next);
> +} else {
> +QLIST_INSERT_AFTER(mon_fdset_cur, mon_fdset, next);
> +}
> +}
> +
> +mon_fdset_fd = g_malloc0(sizeof(*mon_fdset_fd));
> +mon_fdset_fd->fd = fd;
> +mon_fdset_fd->removed = false;
> +QLIST_INSERT_HEAD(&mon_fdset->fds, mon_fdset_fd, next);
> +
> +fdinfo = g_malloc0(sizeof(*fdinfo));
> +fdinfo->fdset_id = mon_fdset->id;
> +fdinfo->fd = mon_fdset_fd->fd;
> +
> +return fdinfo;
> +}
> +
> +void qmp_remove_fd(int64_t fdset_id, bool has_fd, int64_t fd, Error **errp)
> +{
> +Monitor *mon = cur_mon;
> +mon_fdset_t *mon_fdset;
> +mon_fdset_fd_t *mon_fdset_fd;
> +char fd_str[20];
> +
> +QLIST_FOREACH(mon_fdset, &mon->fdsets, next) {
> +if (mon_fdset->id != fdset_id) {
> +continue;
> +}
> +QLIST_FOREACH(mon_fdset_fd, &mon_fdset->fds, next) {
> +if (has_fd && mon_fdset_fd->fd != fd) {
> +continue;
> +}
> +mon_fdset_fd->removed = true;
> +if (has_fd) {
> +break;
> +}
> +}
> +monitor_fdset_
Re: [Qemu-devel] [PATCH v6 3/6] monitor: Clean up fd sets on monitor disconnect
On Fri, Aug 03, 2012 at 01:28:06PM -0400, Corey Bryant wrote: > Each fd set has a boolean that keeps track of whether or not the > fd set is in use by a monitor connection. When a monitor > disconnects, all fds that are members of an fd set with refcount > of zero are closed. This prevents any fd leakage associated with > a client disconnect prior to using a passed fd. > > v5: > -This patch is new in v5. > -This support addresses concerns from v4 regarding fd leakage > if the client disconnects unexpectedly. (ebl...@redhat.com, > kw...@redhat.com, dberra...@redhat.com) > > v6: > -No changes > > Signed-off-by: Corey Bryant > --- > monitor.c | 15 +++ > 1 file changed, 15 insertions(+) The lifecycle of fdsets and fds isn't clear to me. It seems like just a refcount in fdset should handle this without extra fields like in_use. Stefan
Re: [Qemu-devel] KVM segfaults with 3.5 while installing ubuntu 12.04
On Wed, Aug 08, 2012 at 07:51:07AM +0200, Stefan Priebe wrote: > Any news? Was this applied upstream? Kevin is ill. He has asked me to review and test patches in his absence. When he gets back later this week this will get picked up (and included in QEMU 1.2). Here is the tree, it includes this patch: https://github.com/stefanha/qemu/commits/for-kevin Stefan
Re: [Qemu-devel] [PATCH v6 2/6] qapi: Introduce add-fd, remove-fd, query-fdsets
On Tue, Aug 7, 2012 at 8:59 PM, Corey Bryant wrote: > > > On 08/07/2012 02:16 PM, Stefan Hajnoczi wrote: >> >> On Fri, Aug 3, 2012 at 6:28 PM, Corey Bryant >> wrote: >>> +snprintf(fd_str, sizeof(fd_str), "%ld", fd); >>> +qerror_report(QERR_FD_NOT_FOUND, fd_str); >> >> >> Why use an fd_str instead of passing an int64_t into the error >> message? This also assumed sizeof(long) == 8, which isn't true on >> 32-bit hosts, so %ld should be %"PRId64". > > > Can I pass an int64_t into the message if it takes a string? > > I thought int64_t was a long long in 32-bit mode, but perhaps that's not > always the case? The PRId64 format specifier macro from the C standard hides this so you can pass int64_t values to printf()-style functions in a portable way. Stefan
Re: [Qemu-devel] [PATCH v7 6/6] block: Enable qemu_open/close to work with fd sets
On Tue, Aug 07, 2012 at 11:58:28AM -0400, Corey Bryant wrote:
> @@ -2566,6 +2567,92 @@ FdsetInfoList *qmp_query_fdsets(Error **errp)
> return fdset_list;
> }
>
> +int monitor_fdset_get_fd(int64_t fdset_id, int flags)
> +{
> +mon_fdset_t *mon_fdset;
> +mon_fdset_fd_t *mon_fdset_fd;
> +int mon_fd_flags;
> +
> +QLIST_FOREACH(mon_fdset, &mon_fdsets, next) {
> +if (mon_fdset->id != fdset_id) {
> +continue;
> +}
> +QLIST_FOREACH(mon_fdset_fd, &mon_fdset->fds, next) {
> +if (mon_fdset_fd->removed) {
> +continue;
> +}
This makes me wonder about immediately closing in remove-fd and drop the
removed field. This way, code using mon_fdset->fds does not need to
worry about removed=true fds.
Re: [Qemu-devel] [PATCH v7 0/6] file descriptor passing using fd sets
On Tue, Aug 7, 2012 at 4:58 PM, Corey Bryant wrote: > libvirt's sVirt security driver provides SELinux MAC isolation for > Qemu guest processes and their corresponding image files. In other > words, sVirt uses SELinux to prevent a QEMU process from opening > files that do not belong to it. > > sVirt provides this support by labeling guests and resources with > security labels that are stored in file system extended attributes. > Some file systems, such as NFS, do not support the extended > attribute security namespace, and therefore cannot support sVirt > isolation. > > A solution to this problem is to provide fd passing support, where > libvirt opens files and passes file descriptors to QEMU. This, > along with SELinux policy to prevent QEMU from opening files, can > provide image file isolation for NFS files stored on the same NFS > mount. > > This patch series adds the add-fd, remove-fd, and query-fdsets > QMP monitor commands, which allow file descriptors to be passed > via SCM_RIGHTS, and assigned to specified fd sets. This allows > fd sets to be created per file with fds having, for example, > different access rights. When QEMU needs to reopen a file with > different access rights, it can search for a matching fd in the > fd set. Fd sets also allow for easy tracking of fds per file, > helping to prevent fd leaks. > > Support is also added to the block layer to allow QEMU to dup an > fd from an fdset when the filename is of the /dev/fdset/nnn format, > where nnn is the fd set ID. > > No new SELinux policy is required to prevent open of NFS files > (files with type nfs_t). The virt_use_nfs boolean type simply > needs to be set to false, and open will be prevented (and dup will > be allowed). For example: > > # setsebool virt_use_nfs 0 > # getsebool virt_use_nfs > virt_use_nfs --> off > > Corey Bryant (6): > qemu-char: Add MSG_CMSG_CLOEXEC flag to recvmsg > qapi: Introduce add-fd, remove-fd, query-fdsets > monitor: Clean up fd sets on monitor disconnect > block: Convert open calls to qemu_open > block: Convert close calls to qemu_close > block: Enable qemu_open/close to work with fd sets > > block/raw-posix.c | 42 - > block/raw-win32.c |6 +- > block/vdi.c |5 +- > block/vmdk.c | 25 +++-- > block/vpc.c |4 +- > block/vvfat.c | 16 ++-- > cutils.c |5 + > monitor.c | 273 > + > monitor.h |5 + > osdep.c | 117 +++ > qapi-schema.json | 110 + > qemu-char.c | 12 ++- > qemu-common.h |2 + > qemu-tool.c | 20 > qerror.c |4 + > qerror.h |3 + > qmp-commands.hx | 131 + > savevm.c |4 +- > 18 files changed, 730 insertions(+), 54 deletions(-) Are there tests for this feature? Do you have test scripts used during development? Here's what I've gathered: Applications use add-fd to add file descriptors to fd sets. An fd set contains one or more file descriptors, each with different access modes (O_RDONLY, O_RDWR, O_WRONLY). File descriptors can be retrieved from the fd set and are matched by their access modes. This allows QEMU to reopen files with different access modes. File descriptors stay in their fd set until explicitly removed by the remove-fd command or when all monitor clients have disconnected. This ensures that file descriptors are not leaked after a monitor client crashes. Automatic removal on monitor close is postponed until all duped fds have been fd - this means QEMU can still reopen an in-use fd after a client disconnects. Does this sound right? Please do the QEMU coding style naming of MonFdset/MonFdsetFd mentioned in v6. Stefan
Re: [Qemu-devel] [PATCH 01/15] atomic: introduce atomic operations
On Wed, Aug 8, 2012 at 10:21 AM, Peter Maydell wrote:
> On 8 August 2012 07:25, Liu Ping Fan wrote:
>> +static inline void atomic_sub(int i, Atomic *v)
>> +{
>> +asm volatile("lock; subl %1,%0"
>> + : "+m" (v->counter)
>> + : "ir" (i));
>> +}
>
> NAK. We don't want random inline assembly implementations of locking
> primitives in QEMU, they are way too hard to keep working with all the
> possible host architectures we support. I spent some time a while back
> getting rid of the (variously busted) versions we had previously.
>
> If you absolutely must use atomic ops, use the gcc builtins. For
> preference, stick to higher level and less error-prone abstractions.
We're spoilt for choice here:
1. Atomic built-ins from gcc
2. glib atomics
No need to roll our own or copy the implementation from the kernel.
Stefan
Re: [Qemu-devel] [PATCH v5 2/2] block: Support GlusterFS as a QEMU block backend
On Tue, Aug 7, 2012 at 9:01 AM, Bharata B Rao
wrote:
> block: Support GlusterFS as a QEMU block backend.
>
> From: Bharata B Rao
>
> This patch adds gluster as the new block backend in QEMU. This gives
> QEMU the ability to boot VM images from gluster volumes. Its already
> possible to boot from VM images on gluster volumes using FUSE mount, but
> this patchset provides the ability to boot VM images from gluster volumes
> by by-passing the FUSE layer in gluster. This is made possible by
> using libgfapi routines to perform IO on gluster volumes directly.
>
> VM Image on gluster volume is specified like this:
>
> file=gluster://server:[port]/volname/image[?transport=socket]
>
> 'gluster' is the protocol.
>
> 'server' specifies the server where the volume file specification for
> the given volume resides. This can be either hostname or ipv4 address
> or ipv6 address. ipv6 address needs to be with in square brackets [ ].
>
> port' is the port number on which gluster management daemon (glusterd) is
> listening. This is optional and if not specified, QEMU will send 0 which
> will make libgfapi to use the default port.
>
> 'volname' is the name of the gluster volume which contains the VM image.
>
> 'image' is the path to the actual VM image in the gluster volume.
>
> 'transport' specifies the transport used to connect to glusterd. This is
> optional and if not specified, socket transport is used.
>
> Examples:
>
> file=gluster://1.2.3.4/testvol/a.img
> file=gluster://1.2.3.4:5000/testvol/dir/a.img?transport=socket
> file=gluster://[1:2:3:4:5:6:7:8]/testvol/dir/a.img
> file=gluster://[1:2:3:4:5:6:7:8]:5000/testvol/dir/a.img?transport=socket
> file=gluster://server.domain.com:5000/testvol/dir/a.img
>
> Signed-off-by: Bharata B Rao
> Reviewed-by: Stefan Hajnoczi
> ---
>
> block/Makefile.objs |1
> block/gluster.c | 623
> +++
> 2 files changed, 624 insertions(+), 0 deletions(-)
> create mode 100644 block/gluster.c
I have left a few small comments. Perhaps you can resend with your
fixes to Patch 1?
> diff --git a/block/Makefile.objs b/block/Makefile.objs
> index b5754d3..a1ae67f 100644
> --- a/block/Makefile.objs
> +++ b/block/Makefile.objs
> @@ -9,3 +9,4 @@ block-obj-$(CONFIG_POSIX) += raw-posix.o
> block-obj-$(CONFIG_LIBISCSI) += iscsi.o
> block-obj-$(CONFIG_CURL) += curl.o
> block-obj-$(CONFIG_RBD) += rbd.o
> +block-obj-$(CONFIG_GLUSTERFS) += gluster.o
> diff --git a/block/gluster.c b/block/gluster.c
> new file mode 100644
> index 000..39c55fe
> --- /dev/null
> +++ b/block/gluster.c
> @@ -0,0 +1,623 @@
> +/*
> + * GlusterFS backend for QEMU
> + *
> + * (AIO implementation is derived from block/rbd.c)
> + *
> + * Copyright (C) 2012 Bharata B Rao
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or
> + * (at your option) any later version. See the COPYING file in the top-level
> + * directory.
> + */
> +#include "block_int.h"
> +#include
System headers followed by user headers is a good order to prevent
application-specific macros from interfering with system headers:
#include
#include "block_int.h"
> +
> +typedef struct GlusterAIOCB {
> +BlockDriverAIOCB common;
> +bool canceled;
> +int64_t size;
> +int ret;
> +} GlusterAIOCB;
> +
> +typedef struct BDRVGlusterState {
> +struct glfs *glfs;
> +int fds[2];
> +struct glfs_fd *fd;
> +int qemu_aio_count;
> +} BDRVGlusterState;
> +
> +#define GLUSTER_FD_READ 0
> +#define GLUSTER_FD_WRITE 1
> +
> +typedef struct GlusterURI {
> +char *server;
> +int port;
> +char *volname;
> +char *image;
> +char *transport;
> +} GlusterURI;
> +
> +static void qemu_gluster_uri_free(GlusterURI *uri)
> +{
> +g_free(uri->server);
> +g_free(uri->volname);
> +g_free(uri->image);
> +g_free(uri->transport);
> +g_free(uri);
> +}
> +
> +/*
> + * We don't validate the transport option obtained here but
> + * instead depend on gluster to flag an error.
> + */
> +static int parse_transport(GlusterURI *uri, char *transport)
> +{
> +char *token, *saveptr;
> +int ret = -EINVAL;
> +
> +if (!transport) {
> +uri->transport = g_strdup("socket");
> +ret = 0;
> +goto out;
> +}
> +
> +token = strtok_r(transport, "=", &saveptr);
> +if (!token) {
> +goto out;
> +}
> +if (strcmp(token, "transport")) {
> +goto out;
> +}
> +token = strto
Re: [Qemu-devel] [PATCH v7 6/6] block: Enable qemu_open/close to work with fd sets
On Wed, Aug 8, 2012 at 2:54 PM, Corey Bryant wrote:
>
>
> On 08/08/2012 09:02 AM, Stefan Hajnoczi wrote:
>>
>> On Tue, Aug 07, 2012 at 11:58:28AM -0400, Corey Bryant wrote:
>>>
>>> @@ -2566,6 +2567,92 @@ FdsetInfoList *qmp_query_fdsets(Error **errp)
>>> return fdset_list;
>>> }
>>>
>>> +int monitor_fdset_get_fd(int64_t fdset_id, int flags)
>>> +{
>>> +mon_fdset_t *mon_fdset;
>>> +mon_fdset_fd_t *mon_fdset_fd;
>>> +int mon_fd_flags;
>>> +
>>> +QLIST_FOREACH(mon_fdset, &mon_fdsets, next) {
>>> +if (mon_fdset->id != fdset_id) {
>>> +continue;
>>> +}
>>> +QLIST_FOREACH(mon_fdset_fd, &mon_fdset->fds, next) {
>>> +if (mon_fdset_fd->removed) {
>>> +continue;
>>> +}
>>
>>
>> This makes me wonder about immediately closing in remove-fd and drop the
>> removed field. This way, code using mon_fdset->fds does not need to
>> worry about removed=true fds.
>>
>>
>
> The reason we don't immediately close in remove-fd is so that the client
> doesn't have to keep track of what fd's are in use by QEMU.
>
> Let's say libvirt uses add-fd to add fd=4 (O_RDONLY) and fd=5 (O_RDWR) to fd
> set 2 and they both refer to /mnt/nfs/data.img. libvirt can then issue a
> command that uses the fd (e.g. drive_add file=/dev/fdsets/2). QEMU then
> opens and closes that file as it desires by dup'ing the fd in the fd set
> that has matching access mode (O_RDONLY or O_RDWR) and closing the dup'd fd.
>
> By not closing the fd immediately in remove-fd, we allow the client to issue
> a command like drive_open and immediately follow it with a remove-fd and not
> have to worry about determining when QEMU is done using the fd.
I see. So as long as the fd set's refcount > 0 the fd will not get closed.
Stefan
Re: [Qemu-devel] [PATCH 00/10] Quorum disk image corruption resiliency
On Tue, Aug 7, 2012 at 2:44 PM, Benoît Canet wrote: > This patchset create a block driver implementing a quorum using three qemu > disk > images. Writes are mirrored on the three files. > For the reading part the three files are read at the same time and a vote is > done to determine which is the majority qiov version. It then return this > majority version to the upper layers. > When three differents versions of the data are returned by the lower layer the > quorum is broken and the read return -EIO. If you make the quorum setting configurable, then this can replace blkverify. n=2 is blkverify, n=3 is your current patch series, n/m is also possible where n=number of mirrors and m=threshold needed to achieve quorum. Stefan
Re: [Qemu-devel] [RFC V2 03/10] quorum: Add quorum_open().
On Tue, Aug 7, 2012 at 2:44 PM, Benoît Canet wrote:
> Signed-off-by: Benoit Canet
> ---
> block/quorum.c | 62
>
> 1 file changed, 62 insertions(+)
>
> diff --git a/block/quorum.c b/block/quorum.c
> index e0405b6..de58ab8 100644
> --- a/block/quorum.c
> +++ b/block/quorum.c
> @@ -47,11 +47,73 @@ struct QuorumAIOCB {
> int vote_ret;
> };
>
> +/* Valid quorum filenames look like
> + * quorum:path/to/a_image:path/to/b_image:path/to/c_image
> + */
> +static int quorum_open(BlockDriverState *bs, const char *filename, int flags)
> +{
> +BDRVQuorumState *s = bs->opaque;
> +int ret, i;
> +char *a, *b, *c, *filenames[3];
> +
> +/* Parse the quorum: prefix */
> +if (strncmp(filename, "quorum:", strlen("quorum:"))) {
> +return -EINVAL;
> +}
> +a = g_strdup(filename + strlen("quorum:"));
> +
> +/* Find separators */
> +b = strchr(a, ':');
> +if (b == NULL) {
> +return -EINVAL;
Leaks a. Same for returns below.
> static BlockDriver bdrv_quorum = {
> .format_name= "quorum",
> .protocol_name = "quorum",
>
> .instance_size = sizeof(BDRVQuorumState),
> +
> +.bdrv_file_open = quorum_open,
Please squash quorum_close() into this commit so the g_strdup()
filesnames[] aren't leaked on close.
Re: [Qemu-devel] [RFC V2 04/10] quorum: Add quorum_close().
On Tue, Aug 7, 2012 at 9:30 PM, Blue Swirl wrote:
> On Tue, Aug 7, 2012 at 1:44 PM, Benoît Canet wrote:
>> +static void quorum_close(BlockDriverState *bs)
>> +{
>> +BDRVQuorumState *s = bs->opaque;
>> +int i;
>> +
>> +/* Ensure writes reach stable storage */
>> +for (i = 0; i <= 2; i++) {
>> +bdrv_flush(s->bs[i]);
>
> bdrv_close()
bdrv_delete() instead of close since we did bdrv_new() to allocate them.
We also need to g_free() the filenames.
Re: [Qemu-devel] [RFC V2 05/10] quorum: Add quorum_getlength().
On Tue, Aug 7, 2012 at 2:44 PM, Benoît Canet wrote:
> Signed-off-by: Benoit Canet
> ---
> block/quorum.c | 17 +
> 1 file changed, 17 insertions(+)
>
> diff --git a/block/quorum.c b/block/quorum.c
> index 9da0432..5cd7083 100644
> --- a/block/quorum.c
> +++ b/block/quorum.c
> @@ -118,12 +118,29 @@ static void quorum_close(BlockDriverState *bs)
> }
> }
>
> +static int64_t quorum_getlength(BlockDriverState *bs)
> +{
> +BDRVQuorumState *s = bs->opaque;
> +int i;
> +int64_t ret;
> +
> +/* return the length of the first available quorum file */
> +for (i = 0, ret = bdrv_getlength(s->bs[i]);
> + ret == -ENOMEDIUM && i <= 2;
> + i++, ret = bdrv_getlength(s->bs[i])) {
> +}
Why is -ENOMEDIUM an expected error value?
IMO a for loop with a body or a do while loop would make this easier to read.
Re: [Qemu-devel] [RFC V2 06/10] quorum: Add quorum_aio_writev and its dependencies.
On Tue, Aug 7, 2012 at 2:44 PM, Benoît Canet wrote:
> +static int quorum_check_ret(QuorumAIOCB *acb)
> +{
> +int i, j;
> +
> +for (i = 0, j = 0; i <= 2; i++) {
> +if (acb->aios[0].ret) {
> +j++;
> +}
> +}
> +
> +if (j > 1) {
> +return -EIO;
> +}
> +
> +return 0;
> +}
Simpler version just scans the return values (also I think
acb->aios[0].ret should be acb->aios[i].ret):
static int quorum_check_ret(QuorumAIOCB *acb)
{
int i;
for (i = 0; i <= 2; i++) {
if (acb->aios[i].ret) {
return -EIO; /* or acb->aios[i].ret */
}
}
return 0;
}
> +
> +static void quorum_aio_bh(void *opaque)
> +{
> +QuorumAIOCB *acb = opaque;
> +
> +qemu_bh_delete(acb->bh);
> +acb->common.cb(acb->common.opaque, quorum_check_ret(acb));
> +if (acb->finished) {
> +*acb->finished = true;
> +}
> +qemu_aio_release(acb);
> +}
> +
> +static QuorumAIOCB *quorum_aio_get(BlockDriverState *bs,
> + QEMUIOVector *qiov,
> + int64_t sector_num,
> + int nb_sectors,
> + BlockDriverCompletionFunc *cb,
> + void *opaque)
> +{
> +QuorumAIOCB *acb = qemu_aio_get(&quorum_aio_pool, bs, cb, opaque);
> +int i;
> +
> +acb->qiov = qiov;
> +acb->bh = NULL;
> +acb->count = 0;
> +acb->sector_num = sector_num;
> +acb->nb_sectors = nb_sectors;
> +acb->vote = NULL;
> +acb->vote_ret = 0;
acb->finished = NULL;
Re: [Qemu-devel] [RFC V2 07/10] blkverify: Make blkverify_iovec_clone() and blkverify_iovec_compare() public
On Tue, Aug 7, 2012 at 2:44 PM, Benoît Canet wrote: > Signed-off-by: Benoit Canet > --- > block/blkverify.c |8 ++-- > block/quorum.c|4 > 2 files changed, 10 insertions(+), 2 deletions(-) Perhaps these should be in cutils.c with the other iovec functions.
Re: [Qemu-devel] [RFC V2 09/10] quorum: Add quorum_aio_readv.
On Tue, Aug 7, 2012 at 2:44 PM, Benoît Canet wrote:
> Signed-off-by: Benoit Canet
> ---
> block/quorum.c | 35 +++
> 1 file changed, 35 insertions(+)
>
> diff --git a/block/quorum.c b/block/quorum.c
> index 2df3ae6..13804c1 100644
> --- a/block/quorum.c
> +++ b/block/quorum.c
> @@ -174,6 +174,14 @@ static int quorum_check_ret(QuorumAIOCB *acb)
> static void quorum_aio_bh(void *opaque)
> {
> QuorumAIOCB *acb = opaque;
> +int i;
> +
> +for (i = 0; i <= 2; i++) {
> +if (acb->aios[i].buf) {
> +g_free(acb->aios[i].buf);
> +acb->aios[i].buf = NULL;
> +}
qemu_blockalign() buffers must be freed using qemu_vfree(). Also
qemu_vfree(NULL) is a nop so there no need for if (acb->aios[i].buf),
it can be done unconditionally.
> +}
>
> qemu_bh_delete(acb->bh);
> acb->common.cb(acb->common.opaque, quorum_check_ret(acb));
> @@ -224,6 +232,32 @@ static void quorum_aio_cb(void *opaque, int ret)
> }
> }
>
> +static BlockDriverAIOCB *quorum_aio_readv(BlockDriverState *bs,
> + int64_t sector_num,
> + QEMUIOVector *qiov,
> + int nb_sectors,
> + BlockDriverCompletionFunc *cb,
> + void *opaque)
> +{
> +BDRVQuorumState *s = bs->opaque;
> +QuorumAIOCB *acb = quorum_aio_get(bs, qiov, sector_num,
> + nb_sectors, cb, opaque);
> +int i;
> +
> +for (i = 0; i <= 2; i++) {
> +acb->aios[i].buf = qemu_blockalign(bs->file, qiov->size);
> +qemu_iovec_init(&acb->qiovs[i], qiov->niov);
> +blkverify_iovec_clone(&acb->qiovs[i], qiov, acb->aios[i].buf);
> +}
> +
> +for (i = 0; i <= 2; i++) {
> +bdrv_aio_readv(s->bs[i], sector_num, qiov, nb_sectors,
> + quorum_aio_cb, &acb->aios[i]);
acb->qiovs[i] instead of qiov.
Re: [Qemu-devel] [RFC V2 09/10] quorum: Add quorum_aio_readv.
On Wed, Aug 8, 2012 at 4:44 PM, Stefan Hajnoczi wrote:
> On Tue, Aug 7, 2012 at 2:44 PM, Benoît Canet wrote:
>> @@ -224,6 +232,32 @@ static void quorum_aio_cb(void *opaque, int ret)
>> }
>> }
>>
>> +static BlockDriverAIOCB *quorum_aio_readv(BlockDriverState *bs,
>> + int64_t sector_num,
>> + QEMUIOVector *qiov,
>> + int nb_sectors,
>> + BlockDriverCompletionFunc *cb,
>> + void *opaque)
>> +{
>> +BDRVQuorumState *s = bs->opaque;
>> +QuorumAIOCB *acb = quorum_aio_get(bs, qiov, sector_num,
>> + nb_sectors, cb, opaque);
>> +int i;
>> +
>> +for (i = 0; i <= 2; i++) {
>> +acb->aios[i].buf = qemu_blockalign(bs->file, qiov->size);
>> +qemu_iovec_init(&acb->qiovs[i], qiov->niov);
>> +blkverify_iovec_clone(&acb->qiovs[i], qiov, acb->aios[i].buf);
>> +}
>> +
>> +for (i = 0; i <= 2; i++) {
>> +bdrv_aio_readv(s->bs[i], sector_num, qiov, nb_sectors,
>> + quorum_aio_cb, &acb->aios[i]);
>
> acb->qiovs[i] instead of qiov.
Ah, I now see this was intentional because voting hasn't been added yet.
Re: [Qemu-devel] [RFC V2 10/10] quorum: Add quorum mechanism.
On Tue, Aug 7, 2012 at 2:44 PM, Benoît Canet wrote:
> +static void quorum_copy_qiov(QEMUIOVector *dest, QEMUIOVector *source)
> +{
> +int i;
> +for (i = 0; i < source->niov; i++) {
> +memcpy(dest->iov[i].iov_base,
> + source->iov[i].iov_base,
> + source->iov[i].iov_len);
> +dest->iov[i].iov_len = source->iov[i].iov_len;
> +}
> +dest->niov = source->niov;
> +dest->nalloc = source->nalloc;
> +dest->size = source->size;
dest and source must be compatible. Their element lengths must be identical.
Therefore I suggest dropping the assignments and replacing them with
assert(3) calls that remind us that we know they are compatible.
> +}
> +
> +static void quorum_vote(QuorumAIOCB *acb)
> +{
> +ssize_t a_b, b_c, a_c;
> +a_b = blkverify_iovec_compare(&acb->qiovs[0], &acb->qiovs[1]);
> +b_c = blkverify_iovec_compare(&acb->qiovs[1], &acb->qiovs[2]);
> +
> +/* Three vector identical -> quorum */
> +if (a_b == b_c && a_b == -1) {
> +quorum_copy_qiov(acb->qiov, &acb->qiovs[0]); /*clone a */
> +return;
> +}
> +if (a_b == -1) {
> +quorum_print_bad(acb, "C");
> +quorum_copy_qiov(acb->qiov, &acb->qiovs[0]); /*clone a */
> +return;
> +}
> +if (b_c == -1) {
> +quorum_print_bad(acb, "A");
> +quorum_copy_qiov(acb->qiov, &acb->qiovs[1]); /*clone b */
> +return;
> +}
> +a_c = blkverify_iovec_compare(&acb->qiovs[0], &acb->qiovs[2]);
> +if (a_c == -1) {
> +quorum_print_bad(acb, "B");
> +quorum_copy_qiov(acb->qiov, &acb->qiovs[0]); /*clone a */
> +return;
> +}
> +quorum_print_failure(acb);
> +acb->vote_ret = -EIO;
> }
In the common case comparison will succeed so we could use acb->qiov
as acb->qiovs[0] (a's qiov). In that case we wouldn't need to copy
the data. If you feel this will complicate things you could leave a
comment so someone can add it in the future, if necessary.
Re: [Qemu-devel] [PATCH v7 0/6] file descriptor passing using fd sets
On Wed, Aug 8, 2012 at 3:54 PM, Corey Bryant wrote: > > > On 08/08/2012 09:04 AM, Stefan Hajnoczi wrote: >> >> On Tue, Aug 7, 2012 at 4:58 PM, Corey Bryant >> wrote: >>> >>> libvirt's sVirt security driver provides SELinux MAC isolation for >>> Qemu guest processes and their corresponding image files. In other >>> words, sVirt uses SELinux to prevent a QEMU process from opening >>> files that do not belong to it. >>> >>> sVirt provides this support by labeling guests and resources with >>> security labels that are stored in file system extended attributes. >>> Some file systems, such as NFS, do not support the extended >>> attribute security namespace, and therefore cannot support sVirt >>> isolation. >>> >>> A solution to this problem is to provide fd passing support, where >>> libvirt opens files and passes file descriptors to QEMU. This, >>> along with SELinux policy to prevent QEMU from opening files, can >>> provide image file isolation for NFS files stored on the same NFS >>> mount. >>> >>> This patch series adds the add-fd, remove-fd, and query-fdsets >>> QMP monitor commands, which allow file descriptors to be passed >>> via SCM_RIGHTS, and assigned to specified fd sets. This allows >>> fd sets to be created per file with fds having, for example, >>> different access rights. When QEMU needs to reopen a file with >>> different access rights, it can search for a matching fd in the >>> fd set. Fd sets also allow for easy tracking of fds per file, >>> helping to prevent fd leaks. >>> >>> Support is also added to the block layer to allow QEMU to dup an >>> fd from an fdset when the filename is of the /dev/fdset/nnn format, >>> where nnn is the fd set ID. >>> >>> No new SELinux policy is required to prevent open of NFS files >>> (files with type nfs_t). The virt_use_nfs boolean type simply >>> needs to be set to false, and open will be prevented (and dup will >>> be allowed). For example: >>> >>> # setsebool virt_use_nfs 0 >>> # getsebool virt_use_nfs >>> virt_use_nfs --> off >>> >>> Corey Bryant (6): >>>qemu-char: Add MSG_CMSG_CLOEXEC flag to recvmsg >>>qapi: Introduce add-fd, remove-fd, query-fdsets >>>monitor: Clean up fd sets on monitor disconnect >>>block: Convert open calls to qemu_open >>>block: Convert close calls to qemu_close >>>block: Enable qemu_open/close to work with fd sets >>> >>> block/raw-posix.c | 42 - >>> block/raw-win32.c |6 +- >>> block/vdi.c |5 +- >>> block/vmdk.c | 25 +++-- >>> block/vpc.c |4 +- >>> block/vvfat.c | 16 ++-- >>> cutils.c |5 + >>> monitor.c | 273 >>> + >>> monitor.h |5 + >>> osdep.c | 117 +++ >>> qapi-schema.json | 110 + >>> qemu-char.c | 12 ++- >>> qemu-common.h |2 + >>> qemu-tool.c | 20 >>> qerror.c |4 + >>> qerror.h |3 + >>> qmp-commands.hx | 131 + >>> savevm.c |4 +- >>> 18 files changed, 730 insertions(+), 54 deletions(-) >> >> >> Are there tests for this feature? Do you have test scripts used >> during development? > > > Yes I have some C code that I've been using for testing. I can clean it up > and provide it if you'd like. That would be very useful. tests/ has test cases. For the block layer tests/qemu-iotests/ is especially relevant, that's where a lot of the test cases go. If you look at test case 030 you'll see how a Python script interacts with QMP to test image streaming - unfortunately I think Python doesn't natively support SCM_RIGHTS. But a test script would be very useful so it can be used as a regression test in the future. >> >> Here's what I've gathered: >> >> Applications use add-fd to add file descriptors to fd sets. An fd set >> contains one or more file descriptors, each with different access >> modes (O_RDONLY, O_RDWR, O_WRONLY). File descriptors can be retrieved >> from the fd set and are matched by their access modes. This allows >> QEMU to reopen files with different access modes. >> >> File descriptor
Re: [Qemu-devel] [PATCH] vl.c: Exit QEMU early if no machine is found
On Tue, Jul 24, 2012 at 12:42:20AM +0800, riegama...@gmail.com wrote: > From: Dunrong Huang > > We check whether the variable machine is NULL or not before accessing > it. If machine is NULL, exit QEMU with an error, this can avoids a > segfault error. > > Signed-off-by: Dunrong Huang > --- > vl.c | 10 +- > 1 files changed, 5 insertions(+), 5 deletions(-) Thanks, applied to the trivial patches tree: https://github.com/stefanha/qemu/commits/trivial-patches Stefan
Re: [Qemu-devel] [PATCH v7 0/6] file descriptor passing using fd sets
On Wed, Aug 8, 2012 at 7:51 PM, Corey Bryant wrote: > > > On 08/08/2012 11:58 AM, Stefan Hajnoczi wrote: >> >> On Wed, Aug 8, 2012 at 3:54 PM, Corey Bryant >> wrote: >>> >>> >>> >>> On 08/08/2012 09:04 AM, Stefan Hajnoczi wrote: >>>> >>>> >>>> On Tue, Aug 7, 2012 at 4:58 PM, Corey Bryant >>>> wrote: >>>>> >>>>> >>>>> libvirt's sVirt security driver provides SELinux MAC isolation for >>>>> Qemu guest processes and their corresponding image files. In other >>>>> words, sVirt uses SELinux to prevent a QEMU process from opening >>>>> files that do not belong to it. >>>>> >>>>> sVirt provides this support by labeling guests and resources with >>>>> security labels that are stored in file system extended attributes. >>>>> Some file systems, such as NFS, do not support the extended >>>>> attribute security namespace, and therefore cannot support sVirt >>>>> isolation. >>>>> >>>>> A solution to this problem is to provide fd passing support, where >>>>> libvirt opens files and passes file descriptors to QEMU. This, >>>>> along with SELinux policy to prevent QEMU from opening files, can >>>>> provide image file isolation for NFS files stored on the same NFS >>>>> mount. >>>>> >>>>> This patch series adds the add-fd, remove-fd, and query-fdsets >>>>> QMP monitor commands, which allow file descriptors to be passed >>>>> via SCM_RIGHTS, and assigned to specified fd sets. This allows >>>>> fd sets to be created per file with fds having, for example, >>>>> different access rights. When QEMU needs to reopen a file with >>>>> different access rights, it can search for a matching fd in the >>>>> fd set. Fd sets also allow for easy tracking of fds per file, >>>>> helping to prevent fd leaks. >>>>> >>>>> Support is also added to the block layer to allow QEMU to dup an >>>>> fd from an fdset when the filename is of the /dev/fdset/nnn format, >>>>> where nnn is the fd set ID. >>>>> >>>>> No new SELinux policy is required to prevent open of NFS files >>>>> (files with type nfs_t). The virt_use_nfs boolean type simply >>>>> needs to be set to false, and open will be prevented (and dup will >>>>> be allowed). For example: >>>>> >>>>> # setsebool virt_use_nfs 0 >>>>> # getsebool virt_use_nfs >>>>> virt_use_nfs --> off >>>>> >>>>> Corey Bryant (6): >>>>> qemu-char: Add MSG_CMSG_CLOEXEC flag to recvmsg >>>>> qapi: Introduce add-fd, remove-fd, query-fdsets >>>>> monitor: Clean up fd sets on monitor disconnect >>>>> block: Convert open calls to qemu_open >>>>> block: Convert close calls to qemu_close >>>>> block: Enable qemu_open/close to work with fd sets >>>>> >>>>>block/raw-posix.c | 42 - >>>>>block/raw-win32.c |6 +- >>>>>block/vdi.c |5 +- >>>>>block/vmdk.c | 25 +++-- >>>>>block/vpc.c |4 +- >>>>>block/vvfat.c | 16 ++-- >>>>>cutils.c |5 + >>>>>monitor.c | 273 >>>>> + >>>>>monitor.h |5 + >>>>>osdep.c | 117 +++ >>>>>qapi-schema.json | 110 + >>>>>qemu-char.c | 12 ++- >>>>>qemu-common.h |2 + >>>>>qemu-tool.c | 20 >>>>>qerror.c |4 + >>>>>qerror.h |3 + >>>>>qmp-commands.hx | 131 + >>>>>savevm.c |4 +- >>>>>18 files changed, 730 insertions(+), 54 deletions(-) >>>> >>>> >>>> >>>> Are there tests for this feature? Do you have test scripts used >>>> during development? >>> >>> >>> >>> Yes I have some C code that I've been using for testing. I can clean it >>> up >>> and provide it if you'd like. >> >> >> That would be very useful. tests/ has test cases. For the block >> layer tests/qemu-iotests/ is especially relevant, that's where a lot >> of the test cases go. If you look at test case 030 you'll see how a >> Python script interacts with QMP to test image streaming - >> unfortunately I think Python doesn't natively support SCM_RIGHTS. But >> a test script would be very useful so it can be used as a regression >> test in the future. >> > > Sure I'll take a look. Hopefully a C test is ok if I can't use SCM_RIGHTS > in Python. Great, thanks. Stefan
Re: [Qemu-devel] [libvirt] [PATCH v7 2/6] qapi: Introduce add-fd, remove-fd, query-fdsets
On Tue, Aug 7, 2012 at 4:58 PM, Corey Bryant wrote:
> +##
> +# @FdsetFdInfo:
> +#
> +# Information about a file descriptor that belongs to an fd set.
> +#
> +# @fd: The file descriptor value.
> +#
> +# @removed: If true, the remove-fd command has been issued for this fd.
> +#
> +# Since: 1.2.0
> +##
> +{ 'type': 'FdsetFdInfo', 'data': {'fd': 'int', 'removed': 'bool'} }
I'm not sure if the removed field is useful, especially since
remove-fd is idempotent (you can keep querying fds and then marking
them removed again until they finally close). The reason I suggest
minimizing the schema is so that we can change implementation details
later without having to synthesize this state.
> +##
> +# @FdsetInfo:
> +#
> +# Information about an fd set.
> +#
> +# @fdset_id: The ID of the fd set.
> +#
> +# @refcount: A count of the number of outstanding dup() references to
> +#this fd set.
> +#
> +# @in_use: If true, a monitor is connected and has access to this fd set.
> +#
> +# @fds: A list of file descriptors that belong to this fd set.
> +#
> +# Since: 1.2.0
> +##
> +{ 'type': 'FdsetInfo',
> + 'data': {'fdset_id': 'int', 'refcount': 'int', 'in_use': 'bool',
> + 'fds': ['FdsetFdInfo']} }
Why are refcount and in_use exposed? How will applications use them?
This seems like internal state to me.
Should add-fd allow the application to associate an opaque string with
the fdset? This could be used to recover after crash. Otherwise the
application needs to store the fdset_id -> filename mapping in a file
on disk and hope it was safely stored before crash. It seems like the
best place to keep this info is with the fdset.
Stefan
Re: [Qemu-devel] [RFC V2 05/10] quorum: Add quorum_getlength().
On Thu, Aug 9, 2012 at 10:07 AM, Benoît Canet wrote:
>> > +static int64_t quorum_getlength(BlockDriverState *bs)
>> > +{
>> > +BDRVQuorumState *s = bs->opaque;
>> > +int i;
>> > +int64_t ret;
>> > +
>> > +/* return the length of the first available quorum file */
>> > +for (i = 0, ret = bdrv_getlength(s->bs[i]);
>> > + ret == -ENOMEDIUM && i <= 2;
>> > + i++, ret = bdrv_getlength(s->bs[i])) {
>> > +}
>>
>> Why is -ENOMEDIUM an expected error value?
>
> I put the -ENOMEDIUM test because of the following piece of code.
> /**
> * Length of a file in bytes. Return < 0 if error or unknown.
> */
> int64_t bdrv_getlength(BlockDriverState *bs)
> {
> BlockDriver *drv = bs->drv;
> if (!drv)
> return -ENOMEDIUM;
>
> Still I am not sure it's needed. What is your stance on this ?
A BlockDriverState has more than just block filter or image format
state, it also has some guest-visible state unfortunately. The
BlockDriverState attached to the guest's storage controller (e.g. IDE
CD-ROM) can be closed by blockdev.c:eject_device() and left as an
empty BlockDriverState with ->drv == NULL.
I think we don't need to worry about this in a block filter like
quorum because all child BlockDriverStates will not be ejected.
Stefan
Re: [Qemu-devel] [Qemu-trivial] [RESEND PATCH] vl.c: Exit QEMU early if no machine is found
On Thu, Aug 9, 2012 at 7:57 AM, wrote: > From: Dunrong Huang > > We check whether the variable machine is NULL or not before accessing > it. If machine is NULL, exit QEMU with an error, this can avoids a > segfault error. > > Signed-off-by: Dunrong Huang > --- > vl.c | 10 +- > 1 files changed, 5 insertions(+), 5 deletions(-) Applied to the trivial-patches tree in the other email thread. Stefan
Re: [Qemu-devel] [RFC V2 06/10] quorum: Add quorum_aio_writev and its dependencies.
On Thu, Aug 9, 2012 at 10:24 AM, Benoît Canet wrote:
> Le Wednesday 08 Aug 2012 ŕ 16:37:13 (+0100), Stefan Hajnoczi a écrit :
>> On Tue, Aug 7, 2012 at 2:44 PM, Benoît Canet wrote:
>> > +static int quorum_check_ret(QuorumAIOCB *acb)
>> > +{
>> > +int i, j;
>> > +
>> > +for (i = 0, j = 0; i <= 2; i++) {
>> > +if (acb->aios[0].ret) {
>> > +j++;
>> > +}
>> > +}
>> > +
>> > +if (j > 1) {
>> > +return -EIO;
>> > +}
>> > +
>> > +return 0;
>> > +}
>>
>> Simpler version just scans the return values (also I think
>> acb->aios[0].ret should be acb->aios[i].ret):
>>
>> static int quorum_check_ret(QuorumAIOCB *acb)
>> {
>> int i;
>> for (i = 0; i <= 2; i++) {
>> if (acb->aios[i].ret) {
>> return -EIO; /* or acb->aios[i].ret */
>> }
>> }
>> return 0;
>> }
>
> I am wondering what is the best code to return.
> There is some potential case like a filer containing a particular
> image (or a image on the network) going down where the user probably
> don't want to get an -EIO.
>
> The
> if (j > 1) {
> return -EIO;
> }
> part was about detecting an error count greater dans the threshold (2).
Yeah, this starts to get into policy and depends on the user. The
best we can do is to make it configurable and choose a sane default.
Stefan
Re: [Qemu-devel] virtio-scsi vs. virtio-blk
On Thu, Aug 9, 2012 at 8:41 AM, Stefan Priebe wrote: > starting line: > /usr/bin/qemu-x86_64 -chardev > socket,id=qmp,path=/var/run/qemu-server/103.qmp,server,nowait -mon > chardev=qmp,mode=control -pidfile /var/run/qemu-server/103.pid -daemonize > -usbdevice tablet -name kvmcrash -smp sockets=1,cores=8 -nodefaults -boot > menu=on -vga cirrus -k de -device > virtio-scsi-pci,id=scsihw0,bus=pci.0,addr=0x5 -drive > file=iscsi://10.0.255.100/iqn.1986-03.com.sun:02:8a9019a4-4aa3-cd8a-f723-f05db9085ef9/0,if=none,id=drive-scsi1,cache=writethrough,aio=native > -device > scsi-hd,bus=scsihw0.0,channel=0,scsi-id=0,lun=1,drive=drive-scsi1,id=scsi1 > -drive > file=iscsi://10.0.255.100/iqn.1986-03.com.sun:02:8a9019a4-4aa3-cd8a-f723-f05db9085ef9/2,if=none,id=drive-virtio0,cache=writethrough,aio=native > -device virtio-blk-pci,drive=drive-virtio0,id=virtio0,bus=pci.0,addr=0xa > -drive > file=iscsi://10.0.255.100/iqn.1986-03.com.sun:02:8a9019a4-4aa3-cd8a-f723-f05db9085ef9/1,if=none,id=drive-scsi0,cache=writethrough,aio=native > -device > scsi-hd,bus=scsihw0.0,channel=0,scsi-id=0,lun=0,drive=drive-scsi0,id=scsi0,bootindex=102 > -m 4096 -netdev > type=tap,id=net0,ifname=tap103i0,script=/var/lib/qemu-server/pve-bridge,vhost=on > -device > virtio-net-pci,mac=BA:5B:86:AD:14:3A,netdev=net0,bus=pci.0,addr=0x12,id=net0,bootindex=300 You're running qemu.git? In that case make sure to use -enable-kvm, otherwise you get TCG (dynamic binary translator) instead of using the CPU's hardware virtualization extensions. qemu-kvm.git enables KVM by default so this could explain slowness if you've run distro qemu-kvm binaries or qemu-kvm.git in the past.
Re: [Qemu-devel] virtio-scsi vs. virtio-blk
On Thu, Aug 9, 2012 at 11:17 AM, Stefan Priebe - Profihost AG
wrote:
> That looks better - thanks for the hint. But now network isn't working at
> all ;-(
You need to have commit 26b9b5fe17cc1b6be2e8bf8b9d16094f420bb8ad
("virtio: fix vhost handling"). Pull the latest qemu.git/master
changes if you don't have it.
Besides that recent vhost-net fix I'm not sure what the problem could
be. Your command-line looks sane. Can you share errors messages or
details on the failure?
With tap the most common problem is permissions since the QEMU process
needs to have access to the /dev/tap* device.
Stefan
Re: [Qemu-devel] [PATCH 3/5] s390: Add new channel I/O based virtio transport.
On Tue, Aug 7, 2012 at 3:52 PM, Cornelia Huck wrote: > Add a new virtio transport that uses channel commands to perform > virtio operations. > > Add a new machine type s390-ccw that uses this virtio-ccw transport > and make it the default machine for s390. > > Signed-off-by: Cornelia Huck > --- > hw/qdev-monitor.c | 5 + > hw/s390-virtio.c | 268 ++ > hw/s390x/Makefile.objs | 1 + > hw/s390x/virtio-ccw.c | 962 > + > hw/s390x/virtio-ccw.h | 77 > vl.c | 1 + > 6 files changed, 1243 insertions(+), 71 deletions(-) > create mode 100644 hw/s390x/virtio-ccw.c > create mode 100644 hw/s390x/virtio-ccw.h Is the virtqueue still using vring and assuming the hypervisor reaches into guest memory? Can existing ccw device types access memory directly (for some reason I assumed ccw always copies or send messages)? Stefan
[Qemu-devel] [PATCH v2 4/4] qemu-iotests: skip 039 with ./check -nocache
When the qemu-io --nocache option is used the 039 test case cannot abort
QEMU at a point where the image is dirty. Skip the test case.
Signed-off-by: Stefan Hajnoczi
---
tests/qemu-iotests/039 |1 +
tests/qemu-iotests/common.rc | 14 ++
2 files changed, 15 insertions(+)
diff --git a/tests/qemu-iotests/039 b/tests/qemu-iotests/039
index a749fcf..c5ae806 100755
--- a/tests/qemu-iotests/039
+++ b/tests/qemu-iotests/039
@@ -44,6 +44,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
_supported_fmt qcow2
_supported_proto generic
_supported_os Linux
+_unsupported_qemu_io_options --nocache
size=128M
diff --git a/tests/qemu-iotests/common.rc b/tests/qemu-iotests/common.rc
index 7782808..c5129f4 100644
--- a/tests/qemu-iotests/common.rc
+++ b/tests/qemu-iotests/common.rc
@@ -297,6 +297,20 @@ _supported_os()
_notrun "not suitable for this OS: $HOSTOS"
}
+_unsupported_qemu_io_options()
+{
+for bad_opt
+do
+for opt in $QEMU_IO_OPTIONS
+do
+if [ "$bad_opt" = "$opt" ]
+then
+_notrun "not suitable for qemu-io option: $bad_opt"
+fi
+done
+done
+}
+
# this test requires that a specified command (executable) exists
#
_require_command()
--
1.7.10.4
[Qemu-devel] [PATCH v2 1/4] qed: mark image clean after repair succeeds
The dirty bit is cleared after image repair succeeds in qed_open().
Move this into qed_check() so that all callers benefit from this
behavior when fix=true.
This is necessary so qemu-img check can call .bdrv_check() and mark the
image clean.
Signed-off-by: Stefan Hajnoczi
---
block/qed-check.c | 26 ++
block/qed.c |9 +
block/qed.h |5 +
3 files changed, 32 insertions(+), 8 deletions(-)
diff --git a/block/qed-check.c b/block/qed-check.c
index 5edf607..b473dcd 100644
--- a/block/qed-check.c
+++ b/block/qed-check.c
@@ -194,6 +194,28 @@ static void qed_check_for_leaks(QEDCheck *check)
}
}
+/**
+ * Mark an image clean once it passes check or has been repaired
+ */
+static void qed_check_mark_clean(BDRVQEDState *s, BdrvCheckResult *result)
+{
+/* Skip if there were unfixable corruptions or I/O errors */
+if (result->corruptions > 0 || result->check_errors > 0) {
+return;
+}
+
+/* Skip if image is already marked clean */
+if (!(s->header.features & QED_F_NEED_CHECK)) {
+return;
+}
+
+/* Ensure fixes reach storage before clearing check bit */
+bdrv_flush(s->bs);
+
+s->header.features &= ~QED_F_NEED_CHECK;
+qed_write_header_sync(s);
+}
+
int qed_check(BDRVQEDState *s, BdrvCheckResult *result, bool fix)
{
QEDCheck check = {
@@ -215,6 +237,10 @@ int qed_check(BDRVQEDState *s, BdrvCheckResult *result,
bool fix)
if (ret == 0) {
/* Only check for leaks if entire image was scanned successfully */
qed_check_for_leaks(&check);
+
+if (fix) {
+qed_check_mark_clean(s, result);
+}
}
g_free(check.used_clusters);
diff --git a/block/qed.c b/block/qed.c
index 5f3eefa..226545d 100644
--- a/block/qed.c
+++ b/block/qed.c
@@ -89,7 +89,7 @@ static void qed_header_cpu_to_le(const QEDHeader *cpu,
QEDHeader *le)
le->backing_filename_size = cpu_to_le32(cpu->backing_filename_size);
}
-static int qed_write_header_sync(BDRVQEDState *s)
+int qed_write_header_sync(BDRVQEDState *s)
{
QEDHeader le;
int ret;
@@ -491,13 +491,6 @@ static int bdrv_qed_open(BlockDriverState *bs, int flags)
if (ret) {
goto out;
}
-if (!result.corruptions && !result.check_errors) {
-/* Ensure fixes reach storage before clearing check bit */
-bdrv_flush(s->bs);
-
-s->header.features &= ~QED_F_NEED_CHECK;
-qed_write_header_sync(s);
-}
}
}
diff --git a/block/qed.h b/block/qed.h
index c716772..a063bf7 100644
--- a/block/qed.h
+++ b/block/qed.h
@@ -211,6 +211,11 @@ void *gencb_alloc(size_t len, BlockDriverCompletionFunc
*cb, void *opaque);
void gencb_complete(void *opaque, int ret);
/**
+ * Header functions
+ */
+int qed_write_header_sync(BDRVQEDState *s);
+
+/**
* L2 cache functions
*/
void qed_init_l2_cache(L2TableCache *l2_cache);
--
1.7.10.4
[Qemu-devel] [PATCH v2 0/4] qcow2: clean-ups around lazy refcounts
The qcow2 lazy refcount feature was merged with two pending clean-ups suggested by Kevin: 1. Ensure that qemu-iotests/check -nocache 039 does not fail. 2. Fix qemu-img check output when a dirty image file is opened. It currently omits the error summary because automatic repair cleans the image during bdrv_open() before qemu-img.c can call bdrv_check(). v2: * Fail when qcow2 check finds corruptions [Kevin] Stefan Hajnoczi (4): qed: mark image clean after repair succeeds qcow2: mark image clean after repair succeeds block: add BLOCK_O_CHECK for qemu-img check qemu-iotests: skip 039 with ./check -nocache block.h |1 + block/qcow2.c| 32 +--- block/qed-check.c| 26 ++ block/qed.c | 11 ++- block/qed.h |5 + qemu-img.c |2 +- tests/qemu-iotests/039 |1 + tests/qemu-iotests/039.out |6 ++ tests/qemu-iotests/common.rc | 14 ++ 9 files changed, 73 insertions(+), 25 deletions(-) -- 1.7.10.4
[Qemu-devel] [PATCH v2 3/4] block: add BLOCK_O_CHECK for qemu-img check
Image formats with a dirty bit, like qed and qcow2, repair dirty image
files upon open with BDRV_O_RDWR. Performing automatic repair when
qemu-img check runs is not ideal because the bdrv_open() call repairs
the image before the actual bdrv_check() call from qemu-img.c.
Fix this "double repair" since it leads to confusing output from
qemu-img check. Tell the block driver that this image is being opened
just for bdrv_check(). This skips automatic repair and qemu-img.c can
invoke it manually with bdrv_check().
Update the golden output for qemu-iotests 039 to reflect the new
qemu-img check output.
Signed-off-by: Stefan Hajnoczi
---
block.h|1 +
block/qcow2.c |4 ++--
block/qed.c|2 +-
qemu-img.c |2 +-
tests/qemu-iotests/039.out |6 ++
5 files changed, 11 insertions(+), 4 deletions(-)
diff --git a/block.h b/block.h
index 650d872..2e2be11 100644
--- a/block.h
+++ b/block.h
@@ -79,6 +79,7 @@ typedef struct BlockDevOps {
#define BDRV_O_NO_FLUSH0x0200 /* disable flushing on this disk */
#define BDRV_O_COPY_ON_READ 0x0400 /* copy read backing sectors into image */
#define BDRV_O_INCOMING0x0800 /* consistency hint for incoming migration
*/
+#define BDRV_O_CHECK 0x1000 /* open solely for consistency check */
#define BDRV_O_CACHE_MASK (BDRV_O_NOCACHE | BDRV_O_CACHE_WB | BDRV_O_NO_FLUSH)
diff --git a/block/qcow2.c b/block/qcow2.c
index 5896fd6..8f183f1 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -484,8 +484,8 @@ static int qcow2_open(BlockDriverState *bs, int flags)
qemu_co_mutex_init(&s->lock);
/* Repair image if dirty */
-if ((s->incompatible_features & QCOW2_INCOMPAT_DIRTY) &&
-!bs->read_only) {
+if (!(flags & BDRV_O_CHECK) && !bs->read_only &&
+(s->incompatible_features & QCOW2_INCOMPAT_DIRTY)) {
BdrvCheckResult result = {0};
ret = qcow2_check(bs, &result, BDRV_FIX_ERRORS);
diff --git a/block/qed.c b/block/qed.c
index 226545d..a02dbfd 100644
--- a/block/qed.c
+++ b/block/qed.c
@@ -477,7 +477,7 @@ static int bdrv_qed_open(BlockDriverState *bs, int flags)
}
/* If image was not closed cleanly, check consistency */
-if (s->header.features & QED_F_NEED_CHECK) {
+if (!(flags & BDRV_O_CHECK) && (s->header.features & QED_F_NEED_CHECK)) {
/* Read-only images cannot be fixed. There is no risk of corruption
* since write operations are not possible. Therefore, allow
* potentially inconsistent images to be opened read-only. This can
diff --git a/qemu-img.c b/qemu-img.c
index 94a31ad..b41e670 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -379,7 +379,7 @@ static int img_check(int argc, char **argv)
BlockDriverState *bs;
BdrvCheckResult result;
int fix = 0;
-int flags = BDRV_O_FLAGS;
+int flags = BDRV_O_FLAGS | BDRV_O_CHECK;
fmt = NULL;
for(;;) {
diff --git a/tests/qemu-iotests/039.out b/tests/qemu-iotests/039.out
index 155a05e..cb510d6 100644
--- a/tests/qemu-iotests/039.out
+++ b/tests/qemu-iotests/039.out
@@ -26,6 +26,12 @@ incompatible_features 0x1
== Repairing the image file must succeed ==
ERROR OFLAG_COPIED: offset=8005 refcount=0
Repairing cluster 5 refcount=0 reference=1
+The following inconsistencies were found and repaired:
+
+0 leaked clusters
+1 corruptions
+
+Double checking the fixed image now...
No errors were found on the image.
incompatible_features 0x0
--
1.7.10.4
[Qemu-devel] [PATCH v2 2/4] qcow2: mark image clean after repair succeeds
The dirty bit is cleared after image repair succeeds in qcow2_open().
Move this into qcow2_check() so that all callers benefit from this
behavior when fix mode is enabled.
This is necessary so qemu-img check can call .bdrv_check() and mark the
image clean.
Signed-off-by: Stefan Hajnoczi
---
block/qcow2.c | 28 +++-
1 file changed, 15 insertions(+), 13 deletions(-)
diff --git a/block/qcow2.c b/block/qcow2.c
index fd5e214..5896fd6 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -270,6 +270,20 @@ static int qcow2_mark_clean(BlockDriverState *bs)
return 0;
}
+static int qcow2_check(BlockDriverState *bs, BdrvCheckResult *result,
+ BdrvCheckMode fix)
+{
+int ret = qcow2_check_refcounts(bs, result, fix);
+if (ret < 0) {
+return ret;
+}
+
+if (fix && result->check_errors == 0 && result->corruptions == 0) {
+return qcow2_mark_clean(bs);
+}
+return ret;
+}
+
static int qcow2_open(BlockDriverState *bs, int flags)
{
BDRVQcowState *s = bs->opaque;
@@ -474,12 +488,7 @@ static int qcow2_open(BlockDriverState *bs, int flags)
!bs->read_only) {
BdrvCheckResult result = {0};
-ret = qcow2_check_refcounts(bs, &result, BDRV_FIX_ERRORS);
-if (ret < 0) {
-goto fail;
-}
-
-ret = qcow2_mark_clean(bs);
+ret = qcow2_check(bs, &result, BDRV_FIX_ERRORS);
if (ret < 0) {
goto fail;
}
@@ -1568,13 +1577,6 @@ static int qcow2_get_info(BlockDriverState *bs,
BlockDriverInfo *bdi)
return 0;
}
-
-static int qcow2_check(BlockDriverState *bs, BdrvCheckResult *result,
- BdrvCheckMode fix)
-{
-return qcow2_check_refcounts(bs, result, fix);
-}
-
#if 0
static void dump_refcounts(BlockDriverState *bs)
{
--
1.7.10.4
Re: [Qemu-devel] [PATCH 3/5] s390: Add new channel I/O based virtio transport.
On Thu, Aug 9, 2012 at 1:12 PM, Cornelia Huck wrote: > On Thu, 9 Aug 2012 12:34:04 +0100 > Stefan Hajnoczi wrote: > >> On Tue, Aug 7, 2012 at 3:52 PM, Cornelia Huck >> wrote: >> > Add a new virtio transport that uses channel commands to perform >> > virtio operations. >> > >> > Add a new machine type s390-ccw that uses this virtio-ccw transport >> > and make it the default machine for s390. >> > >> > Signed-off-by: Cornelia Huck >> > --- >> > hw/qdev-monitor.c | 5 + >> > hw/s390-virtio.c | 268 ++ >> > hw/s390x/Makefile.objs | 1 + >> > hw/s390x/virtio-ccw.c | 962 >> > + >> > hw/s390x/virtio-ccw.h | 77 >> > vl.c | 1 + >> > 6 files changed, 1243 insertions(+), 71 deletions(-) >> > create mode 100644 hw/s390x/virtio-ccw.c >> > create mode 100644 hw/s390x/virtio-ccw.h >> >> Is the virtqueue still using vring and assuming the hypervisor reaches >> into guest memory? > > The virtqueues are guest-allocated and their location is transmitted > via a control-type ccw to the host, which can then use it until > notified otherwise. > >> >> Can existing ccw device types access memory directly (for some reason >> I assumed ccw always copies or send messages)? > > Not sure if I understand your question correctly, but read or write > type ccws specify a memory area where the hardware/hypervisor may write > to or read from. These accesses happen while the channel program is > running (any time between the ssch/rsch and ending status present at > the subchannel). The "specify an area that can be used by hardware and > os" approach exists as well; the closed thing to the virtio-ccw > approach is probably qdio. Okay, thanks! Stefan
Re: [Qemu-devel] [PATCH 0/3] net: add missing queue flush for e1000 and xen
On Thu, Aug 09, 2012 at 04:45:54PM +0200, Paolo Bonzini wrote: > Luigi reminded me of these patches... > > When the guests replenish the receive ring buffer, the network device > should flush its queue of pending packets. This is done with > qemu_flush_queued_packets, and patches 2+3 add the missing call to > two drivers, e1000 and xen. More may come later---no time to test > them now. > > However, the device should not just retry delivery of packets that were > already read from the tap device, it should also try to read more packets > from the tap device. The latter requires a qemu_notify_event to force > recomputation of the fd_sets. virtio already does this, but it is a > layering violation; patch 1 moves the call from virtio to the network > subsystem, so that e1000 and xen will then get it for free. > > Paolo Bonzini (3): > net: notify iothread after flushing queue > e1000: flush queue whenever can_receive can go from false to true > xen: flush queue when getting an event > > hw/e1000.c | 4 > hw/virtio-net.c | 4 > hw/xen_nic.c| 1 + > net.c | 7 ++- > net/queue.c | 5 +++-- > net/queue.h | 2 +- > 6 file modificati, 15 inserzioni(+), 8 rimozioni(-) Looks good for QEMU 1.2. I'll give Jan and Stefano time to respond, if they want. Stefan
Re: [Qemu-devel] [PATCH v8 2/7] qapi: Introduce add-fd, remove-fd, query-fdsets
On Thu, Aug 09, 2012 at 10:10:44PM -0400, Corey Bryant wrote:
> +void qmp_remove_fd(int64_t fdset_id, bool has_fd, int64_t fd, Error **errp)
> +{
> +MonFdset *mon_fdset;
> +MonFdsetFd *mon_fdset_fd;
> +char fd_str[20];
> +
> +QLIST_FOREACH(mon_fdset, &mon_fdsets, next) {
> +if (mon_fdset->id != fdset_id) {
> +continue;
> +}
> +QLIST_FOREACH(mon_fdset_fd, &mon_fdset->fds, next) {
> +if (has_fd && mon_fdset_fd->fd != fd) {
> +continue;
> +}
> +mon_fdset_fd->removed = true;
> +if (has_fd) {
> +break;
> +}
> +}
> +monitor_fdset_cleanup(mon_fdset);
> +return;
> +}
> +snprintf(fd_str, sizeof(fd_str), "%" PRId64, fd);
> +error_set(errp, QERR_FD_NOT_FOUND, fd_str);
fd is optional and may be uninitialized. I think the human-readable
string should be:
if has_fd:
fd_str = '%s:%s' % (fdset_id, fd)
else:
fd_str = '%s' % fdset_id
Otherwise, looks good.
Re: [Qemu-devel] [PATCH 1/3] net: notify iothread after flushing queue
On Thu, Aug 09, 2012 at 05:55:50PM +, Blue Swirl wrote:
> On Thu, Aug 9, 2012 at 2:45 PM, Paolo Bonzini wrote:
> > @@ -244,7 +244,7 @@ void qemu_net_queue_flush(NetQueue *queue)
> > packet->size);
> > if (ret == 0) {
> > QTAILQ_INSERT_HEAD(&queue->packets, packet, entry);
> > -break;
> > +return 0;
>
> return false;
>
> > }
> >
> > if (packet->sent_cb) {
> > @@ -253,4 +253,5 @@ void qemu_net_queue_flush(NetQueue *queue)
> >
> > g_free(packet);
> > }
> > +return 1;
>
> return true;
Will fix these up when merging. I think it's clearer to use bool too.
Stefan
[Qemu-devel] [PATCH] Makefile: add qapi.py dependencies
Commit 427a1a2cb1d35b83b6302886f46289f6d617134d ("qapi: avoid reserved
keywords") modifies qapi.py, which is used by qapi-types.py and other
Python scripts. Because Makefile has no dependencies for qapi.py the
qapi code generator will not be rerun and the following build error is
produced:
net/slirp.c: In function ‘net_init_slirp’:
net/slirp.c:721:50: error: ‘NetdevUserOptions’ has no member named
‘q_restrict’
Fix this issue by adding the missing qapi.py dependencies.
Signed-off-by: Stefan Hajnoczi
---
Makefile | 14 --
1 file changed, 8 insertions(+), 6 deletions(-)
diff --git a/Makefile b/Makefile
index 000b46c..d736ea5 100644
--- a/Makefile
+++ b/Makefile
@@ -181,24 +181,26 @@ ifneq ($(wildcard config-host.mak),)
include $(SRC_PATH)/tests/Makefile
endif
+qapi-py = $(SRC_PATH)/scripts/qapi.py $(SRC_PATH)/scripts/ordereddict.py
+
qga/qapi-generated/qga-qapi-types.c qga/qapi-generated/qga-qapi-types.h :\
-$(SRC_PATH)/qapi-schema-guest.json $(SRC_PATH)/scripts/qapi-types.py
+$(SRC_PATH)/qapi-schema-guest.json $(SRC_PATH)/scripts/qapi-types.py $(qapi-py)
$(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-types.py
$(gen-out-type) -o qga/qapi-generated -p "qga-" < $<, " GEN $@")
qga/qapi-generated/qga-qapi-visit.c qga/qapi-generated/qga-qapi-visit.h :\
-$(SRC_PATH)/qapi-schema-guest.json $(SRC_PATH)/scripts/qapi-visit.py
+$(SRC_PATH)/qapi-schema-guest.json $(SRC_PATH)/scripts/qapi-visit.py $(qapi-py)
$(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-visit.py
$(gen-out-type) -o qga/qapi-generated -p "qga-" < $<, " GEN $@")
qga/qapi-generated/qga-qmp-commands.h qga/qapi-generated/qga-qmp-marshal.c :\
-$(SRC_PATH)/qapi-schema-guest.json $(SRC_PATH)/scripts/qapi-commands.py
+$(SRC_PATH)/qapi-schema-guest.json $(SRC_PATH)/scripts/qapi-commands.py
$(qapi-py)
$(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-commands.py
$(gen-out-type) -o qga/qapi-generated -p "qga-" < $<, " GEN $@")
qapi-types.c qapi-types.h :\
-$(SRC_PATH)/qapi-schema.json $(SRC_PATH)/scripts/qapi-types.py
+$(SRC_PATH)/qapi-schema.json $(SRC_PATH)/scripts/qapi-types.py $(qapi-py)
$(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-types.py
$(gen-out-type) -o "." < $<, " GEN $@")
qapi-visit.c qapi-visit.h :\
-$(SRC_PATH)/qapi-schema.json $(SRC_PATH)/scripts/qapi-visit.py
+$(SRC_PATH)/qapi-schema.json $(SRC_PATH)/scripts/qapi-visit.py $(qapi-py)
$(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-visit.py
$(gen-out-type) -o "." < $<, " GEN $@")
qmp-commands.h qmp-marshal.c :\
-$(SRC_PATH)/qapi-schema.json $(SRC_PATH)/scripts/qapi-commands.py
+$(SRC_PATH)/qapi-schema.json $(SRC_PATH)/scripts/qapi-commands.py $(qapi-py)
$(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-commands.py
$(gen-out-type) -m -o "." < $<, " GEN $@")
QGALIB_GEN=$(addprefix qga/qapi-generated/, qga-qapi-types.h qga-qapi-visit.h
qga-qmp-commands.h)
--
1.7.10.4
Re: [Qemu-devel] [Qemu-trivial] [PATCH] arm: translate: comment typo - s/middel/middle/
On Mon, Aug 06, 2012 at 05:05:56PM +1000, Peter A. G. Crosthwaite wrote: > Signed-off-by: Peter A. G. Crosthwaite > --- > target-arm/translate.c |2 +- > 1 files changed, 1 insertions(+), 1 deletions(-) Thanks, applied to the trivial patches tree: https://github.com/stefanha/qemu/commits/trivial-patches Also congrats on getting two Reviewed-bys. That's a rate of 1 Reviewed-by per changed character and the most thorough review a QEMU patch has ever had! :) Stefan
[Qemu-devel] [PULL 0/3] Net patches
Paolo's fixes to resume rx when buffers have been replenished on e1000 and xen_nic. virtio-net does not have this bug. The following changes since commit 3d1d9652978ac5a32a0beb4bdf6065ca39440d89: handle device help before accelerator set up (2012-08-09 19:53:01 +) are available in the git repository at: git://github.com/stefanha/qemu.git net for you to fetch changes up to 4247561bbeacb5ece8b90d0d2439efefc0e7283a: xen: flush queue when getting an event (2012-08-10 11:43:54 +0100) Paolo Bonzini (3): net: notify iothread after flushing queue e1000: flush queue whenever can_receive can go from false to true xen: flush queue when getting an event hw/e1000.c |4 hw/virtio-net.c |4 hw/xen_nic.c|1 + net.c |7 ++- net/queue.c |5 +++-- net/queue.h |2 +- 6 files changed, 15 insertions(+), 8 deletions(-) -- 1.7.10.4
Re: [Qemu-devel] [Qemu-trivial] [PATCH v2] target-arm: Fix typos in comments
On Mon, Aug 06, 2012 at 05:42:18PM +0100, Peter Maydell wrote: > Fix a variety of typos in comments in target-arm files. > > Signed-off-by: Peter Maydell > --- > Changes v1->v2: s/inputs values/input values/ > > target-arm/arm-semi.c|2 +- > target-arm/cpu.h |2 +- > target-arm/helper.c |6 +++--- > target-arm/neon_helper.c | 26 +- > target-arm/op_helper.c |2 +- > target-arm/translate.c | 10 +- > 6 files changed, 24 insertions(+), 24 deletions(-) Thanks, applied to the trivial patches tree: https://github.com/stefanha/qemu/commits/trivial-patches Stefan
[Qemu-devel] [PATCH 1/3] net: notify iothread after flushing queue
From: Paolo Bonzini
virtio-net has code to flush the queue and notify the iothread
whenever new receive buffers are added by the guest. That is
fine, and indeed we need to do the same in all other drivers.
However, notifying the iothread should be work for the network
subsystem. And since we are at it we can add a little smartness:
if some of the queued packets already could not be delivered,
there is no need to notify the iothread.
Reported-by: Luigi Rizzo
Cc: Stefan Hajnoczi
Cc: Jan Kiszka
Signed-off-by: Paolo Bonzini
Signed-off-by: Stefan Hajnoczi
---
hw/virtio-net.c |4
net.c |7 ++-
net/queue.c |5 +++--
net/queue.h |2 +-
4 files changed, 10 insertions(+), 8 deletions(-)
diff --git a/hw/virtio-net.c b/hw/virtio-net.c
index b1998b2..6490743 100644
--- a/hw/virtio-net.c
+++ b/hw/virtio-net.c
@@ -447,10 +447,6 @@ static void virtio_net_handle_rx(VirtIODevice *vdev,
VirtQueue *vq)
VirtIONet *n = to_virtio_net(vdev);
qemu_flush_queued_packets(&n->nic->nc);
-
-/* We now have RX buffers, signal to the IO thread to break out of the
- * select to re-poll the tap file descriptor */
-qemu_notify_event();
}
static int virtio_net_can_receive(NetClientState *nc)
diff --git a/net.c b/net.c
index 60043dd..76a8336 100644
--- a/net.c
+++ b/net.c
@@ -357,7 +357,12 @@ void qemu_flush_queued_packets(NetClientState *nc)
{
nc->receive_disabled = 0;
-qemu_net_queue_flush(nc->send_queue);
+if (qemu_net_queue_flush(nc->send_queue)) {
+/* We emptied the queue successfully, signal to the IO thread to repoll
+ * the file descriptor (for tap, for example).
+ */
+qemu_notify_event();
+}
}
static ssize_t qemu_send_packet_async_with_flags(NetClientState *sender,
diff --git a/net/queue.c b/net/queue.c
index e8030aa..6e64091 100644
--- a/net/queue.c
+++ b/net/queue.c
@@ -228,7 +228,7 @@ void qemu_net_queue_purge(NetQueue *queue, NetClientState
*from)
}
}
-void qemu_net_queue_flush(NetQueue *queue)
+bool qemu_net_queue_flush(NetQueue *queue)
{
while (!QTAILQ_EMPTY(&queue->packets)) {
NetPacket *packet;
@@ -244,7 +244,7 @@ void qemu_net_queue_flush(NetQueue *queue)
packet->size);
if (ret == 0) {
QTAILQ_INSERT_HEAD(&queue->packets, packet, entry);
-break;
+return false;
}
if (packet->sent_cb) {
@@ -253,4 +253,5 @@ void qemu_net_queue_flush(NetQueue *queue)
g_free(packet);
}
+return true;
}
diff --git a/net/queue.h b/net/queue.h
index 9d44a9b..fc02b33 100644
--- a/net/queue.h
+++ b/net/queue.h
@@ -53,6 +53,6 @@ ssize_t qemu_net_queue_send_iov(NetQueue *queue,
NetPacketSent *sent_cb);
void qemu_net_queue_purge(NetQueue *queue, NetClientState *from);
-void qemu_net_queue_flush(NetQueue *queue);
+bool qemu_net_queue_flush(NetQueue *queue);
#endif /* QEMU_NET_QUEUE_H */
--
1.7.10.4
Re: [Qemu-devel] [PATCH] device_tree: load_device_tree(): Allow NULL sizep
On Fri, Aug 10, 2012 at 01:54:26PM +1000, Peter A. G. Crosthwaite wrote:
> The sizep arg is populated with the size of the loaded device tree. Since this
> is one of those informational "please populate" type arguments it should be
> optional. Guarded writes to *sizep against NULL accordingly.
>
> Signed-off-by: Peter A. G. Crosthwaite
> Acked-by: Alexander Graf
> ---
> device_tree.c |8 ++--
> 1 files changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/device_tree.c b/device_tree.c
> index d7a9b6b..641a48a 100644
> --- a/device_tree.c
> +++ b/device_tree.c
> @@ -71,7 +71,9 @@ void *load_device_tree(const char *filename_path, int
> *sizep)
> int ret;
> void *fdt = NULL;
>
> -*sizep = 0;
> +if (sizep) {
> +*sizep = 0;
> +}
> dt_size = get_image_size(filename_path);
> if (dt_size < 0) {
> printf("Unable to get size of device tree file '%s'\n",
> @@ -104,7 +106,9 @@ void *load_device_tree(const char *filename_path, int
> *sizep)
> filename_path);
> goto fail;
> }
> -*sizep = dt_size;
> +if (sizep) {
> +*sizep = dt_size;
> +}
What can the caller do with this void* buffer without knowing its size?
They cannot hand the buffer to the guest unless they duplicate the
get_image_size() call, which is pointless, or we're assuming a fixed
size, which is unsafe. I'm asking what the legitimate use case for
sizep == NULL is?
Stefan
[Qemu-devel] [PATCH 3/3] xen: flush queue when getting an event
From: Paolo Bonzini
xen does not have a register that, when written, will cause can_receive
to go from false to true. However, flushing the queue can be attempted
whenever the front-end raises its side of the Xen event channel. There
is a single event channel for tx and rx.
Cc: Stefano Stabellini
Cc: Stefan Hajnoczi
Signed-off-by: Paolo Bonzini
Signed-off-by: Stefan Hajnoczi
---
hw/xen_nic.c |1 +
1 file changed, 1 insertion(+)
diff --git a/hw/xen_nic.c b/hw/xen_nic.c
index 8b79bfb..cf7d559 100644
--- a/hw/xen_nic.c
+++ b/hw/xen_nic.c
@@ -415,6 +415,7 @@ static void net_event(struct XenDevice *xendev)
{
struct XenNetDev *netdev = container_of(xendev, struct XenNetDev, xendev);
net_tx_packets(netdev);
+qemu_flush_queued_packets(&netdev->nic->nc);
}
static int net_free(struct XenDevice *xendev)
--
1.7.10.4
[Qemu-devel] [PATCH 2/3] e1000: flush queue whenever can_receive can go from false to true
From: Paolo Bonzini
When the guests replenish the receive ring buffer, the network device
should flush its queue of pending packets. This is done with
qemu_flush_queued_packets.
e1000's can_receive can go from false to true when RCTL or RDT are
modified.
Reported-by: Luigi Rizzo
Cc: Stefan Hajnoczi
Cc: Jan Kiszka
Signed-off-by: Paolo Bonzini
Signed-off-by: Stefan Hajnoczi
---
hw/e1000.c |4
1 file changed, 4 insertions(+)
diff --git a/hw/e1000.c b/hw/e1000.c
index ae8a6c5..ec3a7c4 100644
--- a/hw/e1000.c
+++ b/hw/e1000.c
@@ -295,6 +295,7 @@ set_rx_control(E1000State *s, int index, uint32_t val)
s->rxbuf_min_shift = ((val / E1000_RCTL_RDMTS_QUAT) & 3) + 1;
DBGOUT(RX, "RCTL: %d, mac_reg[RCTL] = 0x%x\n", s->mac_reg[RDT],
s->mac_reg[RCTL]);
+qemu_flush_queued_packets(&s->nic->nc);
}
static void
@@ -926,6 +927,9 @@ set_rdt(E1000State *s, int index, uint32_t val)
{
s->check_rxov = 0;
s->mac_reg[index] = val & 0x;
+if (e1000_has_rxbufs(s, 1)) {
+qemu_flush_queued_packets(&s->nic->nc);
+}
}
static void
--
1.7.10.4
[Qemu-devel] [PULL 0/3] Trivial patches for 3 to 10 August 2012
The following changes since commit 3d1d9652978ac5a32a0beb4bdf6065ca39440d89: handle device help before accelerator set up (2012-08-09 19:53:01 +) are available in the git repository at: git://github.com/stefanha/qemu.git trivial-patches for you to fetch changes up to b90372ad2a69a9cdad2a40766eb46f0a89d98535: target-arm: Fix typos in comments (2012-08-10 14:37:28 +0100) Dunrong Huang (1): vl.c: Exit QEMU early if no machine is found Peter A. G. Crosthwaite (1): arm: translate: comment typo - s/middel/middle/ Peter Maydell (1): target-arm: Fix typos in comments target-arm/arm-semi.c|2 +- target-arm/cpu.h |2 +- target-arm/helper.c |6 +++--- target-arm/neon_helper.c | 26 +- target-arm/op_helper.c |2 +- target-arm/translate.c | 12 ++-- vl.c | 10 +- 7 files changed, 30 insertions(+), 30 deletions(-) -- 1.7.10.4
[Qemu-devel] [PATCH 2/3] arm: translate: comment typo - s/middel/middle/
From: "Peter A. G. Crosthwaite"
Signed-off-by: Peter A. G. Crosthwaite
Reviewed-by: Andreas Färber
Reviewed-by: Peter Maydell
Signed-off-by: Stefan Hajnoczi
---
target-arm/translate.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/target-arm/translate.c b/target-arm/translate.c
index 29008a4..494c682 100644
--- a/target-arm/translate.c
+++ b/target-arm/translate.c
@@ -9892,7 +9892,7 @@ static inline void
gen_intermediate_code_internal(CPUARMState *env,
} else {
/* While branches must always occur at the end of an IT block,
there are a few other things that can cause us to terminate
- the TB in the middel of an IT block:
+ the TB in the middle of an IT block:
- Exception generating instructions (bkpt, swi, undefined).
- Page boundaries.
- Hardware watchpoints.
--
1.7.10.4
[Qemu-devel] [PATCH 1/3] vl.c: Exit QEMU early if no machine is found
From: Dunrong Huang
We check whether the variable machine is NULL or not before accessing
it. If machine is NULL, exit QEMU with an error, this can avoids a
segfault error.
Markus Armbruster adds that the segfault can be
reproduced as follows:
$ qemu-system-xtensa -cpu help
Signed-off-by: Dunrong Huang
Reviewed-by: Markus Armbruster
Signed-off-by: Stefan Hajnoczi
---
vl.c | 10 +-
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/vl.c b/vl.c
index a4a520f..4871428 100644
--- a/vl.c
+++ b/vl.c
@@ -3204,6 +3204,11 @@ int main(int argc, char **argv, char **envp)
}
loc_set_none();
+if (machine == NULL) {
+fprintf(stderr, "No machine found.\n");
+exit(1);
+}
+
if (machine->hw_version) {
qemu_set_version(machine->hw_version);
}
@@ -3246,11 +3251,6 @@ int main(int argc, char **argv, char **envp)
data_dir = CONFIG_QEMU_DATADIR;
}
-if (machine == NULL) {
-fprintf(stderr, "No machine found.\n");
-exit(1);
-}
-
/*
* Default to max_cpus = smp_cpus, in case the user doesn't
* specify a max_cpus value.
--
1.7.10.4
[Qemu-devel] [PATCH 3/3] target-arm: Fix typos in comments
From: Peter Maydell
Fix a variety of typos in comments in target-arm files.
Signed-off-by: Peter Maydell
Reviewed-by: Peter Crosthwaite
Signed-off-by: Stefan Hajnoczi
---
target-arm/arm-semi.c|2 +-
target-arm/cpu.h |2 +-
target-arm/helper.c |6 +++---
target-arm/neon_helper.c | 26 +-
target-arm/op_helper.c |2 +-
target-arm/translate.c | 10 +-
6 files changed, 24 insertions(+), 24 deletions(-)
diff --git a/target-arm/arm-semi.c b/target-arm/arm-semi.c
index 88ca9bb..2495206 100644
--- a/target-arm/arm-semi.c
+++ b/target-arm/arm-semi.c
@@ -281,7 +281,7 @@ uint32_t do_arm_semihosting(CPUARMState *env)
return len - ret;
}
case TARGET_SYS_READC:
- /* XXX: Read from debug cosole. Not implemented. */
+ /* XXX: Read from debug console. Not implemented. */
return 0;
case TARGET_SYS_ISTTY:
if (use_gdb_syscalls()) {
diff --git a/target-arm/cpu.h b/target-arm/cpu.h
index 191895c..d7f93d9 100644
--- a/target-arm/cpu.h
+++ b/target-arm/cpu.h
@@ -79,7 +79,7 @@ struct arm_boot_info;
typedef struct CPUARMState {
/* Regs for current mode. */
uint32_t regs[16];
-/* Frequently accessed CPSR bits are stored separately for efficiently.
+/* Frequently accessed CPSR bits are stored separately for efficiency.
This contains all the other bits. Use cpsr_{read,write} to access
the whole CPSR. */
uint32_t uncached_cpsr;
diff --git a/target-arm/helper.c b/target-arm/helper.c
index 5727da2..dceaa95 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -988,7 +988,7 @@ static void ttbr164_reset(CPUARMState *env, const
ARMCPRegInfo *ri)
}
static const ARMCPRegInfo lpae_cp_reginfo[] = {
-/* NOP AMAIR0/1: the override is because these clash with tha rather
+/* NOP AMAIR0/1: the override is because these clash with the rather
* broadly specified TLB_LOCKDOWN entry in the generic cp_reginfo.
*/
{ .name = "AMAIR0", .cp = 15, .crn = 10, .crm = 3, .opc1 = 0, .opc2 = 0,
@@ -2899,8 +2899,8 @@ uint32_t HELPER(logicq_cc)(uint64_t val)
return (val >> 32) | (val != 0);
}
-/* VFP support. We follow the convention used for VFP instrunctions:
- Single precition routines have a "s" suffix, double precision a
+/* VFP support. We follow the convention used for VFP instructions:
+ Single precision routines have a "s" suffix, double precision a
"d" suffix. */
/* Convert host exception flags to vfp form. */
diff --git a/target-arm/neon_helper.c b/target-arm/neon_helper.c
index e0b9dbf..8bb5129 100644
--- a/target-arm/neon_helper.c
+++ b/target-arm/neon_helper.c
@@ -530,7 +530,7 @@ NEON_VOP(rshl_s16, neon_s16, 2)
#undef NEON_FN
/* The addition of the rounding constant may overflow, so we use an
- * intermediate 64 bits accumulator. */
+ * intermediate 64 bit accumulator. */
uint32_t HELPER(neon_rshl_s32)(uint32_t valop, uint32_t shiftop)
{
int32_t dest;
@@ -547,8 +547,8 @@ uint32_t HELPER(neon_rshl_s32)(uint32_t valop, uint32_t
shiftop)
return dest;
}
-/* Handling addition overflow with 64 bits inputs values is more
- * tricky than with 32 bits values. */
+/* Handling addition overflow with 64 bit input values is more
+ * tricky than with 32 bit values. */
uint64_t HELPER(neon_rshl_s64)(uint64_t valop, uint64_t shiftop)
{
int8_t shift = (int8_t)shiftop;
@@ -590,7 +590,7 @@ NEON_VOP(rshl_u16, neon_u16, 2)
#undef NEON_FN
/* The addition of the rounding constant may overflow, so we use an
- * intermediate 64 bits accumulator. */
+ * intermediate 64 bit accumulator. */
uint32_t HELPER(neon_rshl_u32)(uint32_t val, uint32_t shiftop)
{
uint32_t dest;
@@ -608,8 +608,8 @@ uint32_t HELPER(neon_rshl_u32)(uint32_t val, uint32_t
shiftop)
return dest;
}
-/* Handling addition overflow with 64 bits inputs values is more
- * tricky than with 32 bits values. */
+/* Handling addition overflow with 64 bit input values is more
+ * tricky than with 32 bit values. */
uint64_t HELPER(neon_rshl_u64)(uint64_t val, uint64_t shiftop)
{
int8_t shift = (uint8_t)shiftop;
@@ -817,7 +817,7 @@ NEON_VOP_ENV(qrshl_u16, neon_u16, 2)
#undef NEON_FN
/* The addition of the rounding constant may overflow, so we use an
- * intermediate 64 bits accumulator. */
+ * intermediate 64 bit accumulator. */
uint32_t HELPER(neon_qrshl_u32)(CPUARMState *env, uint32_t val, uint32_t
shiftop)
{
uint32_t dest;
@@ -846,8 +846,8 @@ uint32_t HELPER(neon_qrshl_u32)(CPUARMState *env, uint32_t
val, uint32_t shiftop
return dest;
}
-/* Handling addition overflow with 64 bits inputs values is more
- * tricky than with 32 bits values. */
+/* Handling addition overflow with 64 bit input values is more
+ * tricky than with 32 bit values. */
uint64_t HELPER(neon_qrshl_u64)(CPUARMState *env, uint64_t val, uint64_t
shiftop)
{
in
Re: [Qemu-devel] KVM segfaults with 3.5 while installing ubuntu 12.04
On Wed, Aug 8, 2012 at 9:29 AM, Stefan Priebe wrote: > ah OK - thanks. Will there be a fixed 1.1.2 as well? mdroth: Kevin has the fix in his block branch, which means qemu.git will get it soon. Here's the commit: http://repo.or.cz/w/qemu/kevin.git/commit/730a9c53b4e52681fcfe31cf38854cbf91e132c7 > > Am 08.08.2012 10:06, schrieb Stefan Hajnoczi: > >> On Wed, Aug 08, 2012 at 07:51:07AM +0200, Stefan Priebe wrote: >>> >>> Any news? Was this applied upstream? >> >> >> Kevin is ill. He has asked me to review and test patches in his >> absence. When he gets back later this week this will get picked up (and >> included in QEMU 1.2). >> >> Here is the tree, it includes this patch: >> >> https://github.com/stefanha/qemu/commits/for-kevin >> >> Stefan >> > -- > To unsubscribe from this list: send the line "unsubscribe kvm" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] [PATCH] device_tree: load_device_tree(): Allow NULL sizep
On Sat, Aug 11, 2012 at 12:11 AM, Peter Crosthwaite
wrote:
> On Fri, Aug 10, 2012 at 11:42 PM, Stefan Hajnoczi wrote:
>> On Fri, Aug 10, 2012 at 01:54:26PM +1000, Peter A. G. Crosthwaite wrote:
>>> The sizep arg is populated with the size of the loaded device tree. Since
>>> this
>>> is one of those informational "please populate" type arguments it should be
>>> optional. Guarded writes to *sizep against NULL accordingly.
>>>
>>> Signed-off-by: Peter A. G. Crosthwaite
>>> Acked-by: Alexander Graf
>>> ---
>>> device_tree.c |8 ++--
>>> 1 files changed, 6 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/device_tree.c b/device_tree.c
>>> index d7a9b6b..641a48a 100644
>>> --- a/device_tree.c
>>> +++ b/device_tree.c
>>> @@ -71,7 +71,9 @@ void *load_device_tree(const char *filename_path, int
>>> *sizep)
>>> int ret;
>>> void *fdt = NULL;
>>>
>>> -*sizep = 0;
>>> +if (sizep) {
>>> +*sizep = 0;
>>> +}
>>> dt_size = get_image_size(filename_path);
>>> if (dt_size < 0) {
>>> printf("Unable to get size of device tree file '%s'\n",
>>> @@ -104,7 +106,9 @@ void *load_device_tree(const char *filename_path, int
>>> *sizep)
>>> filename_path);
>>> goto fail;
>>> }
>>> -*sizep = dt_size;
>>> +if (sizep) {
>>> +*sizep = dt_size;
>>> +}
>>
>> What can the caller do with this void* buffer without knowing its size?
>>
>
> Sanity check the machine:
>
> dtb = load_device_tree( ... ); //dont care how big it is
> foo = fdt_gep_prop( dtb, ... );
> if (foo != object_get_prop(foo_device, foo_prop, ... )) {
> hw_error("your dtb is bad because ... !\n", ... );
> }
What happens if the fdt is corrupt or malicious? I guess we'll access
memory beyond the end of blob.
This seems to be libfdt's fault. I didn't see an API to validate the
blob's size.
I'm "happy" with this patch but if fdt's can ever come from untrusted
sources then we're in trouble.
Stefan
Re: [Qemu-devel] virtio-scsi <-> vhost multi lun/adapter performance results with 3.6-rc0
On Sat, Aug 11, 2012 at 12:23 AM, Nicholas A. Bellinger wrote: > Using a KVM guest with 32x vCPUs and 4G memory, the results for 4x > random I/O now look like: > > workload | jobs | 25% write / 75% read | 75% write / 25% read > -|--|--|- > 1x rd_mcp LUN| 8 | ~155K IOPs | ~145K IOPs > 16x rd_mcp LUNs | 16 | ~315K IOPs | ~305K IOPs > 32x rd_mcp LUNs | 16 | ~425K IOPs | ~410K IOPs > > The full fio randrw results for the six test cases are attached below. > Also, using a workload of fio numjobs > 16 currently makes performance > start to fall off pretty sharply regardless of the number of vCPUs.. > > So running a similar workload with loopback SCSI ports on bare-metal > produces ~1M random IOPs with 12x LUNs + numjobs=32. At numjobs=16 here > with vhost the 16x LUN configuration ends up being in the range of ~310K > IOPs for the current sweet spot.. This makes me wonder what a comparison against baremetal looks like and the perf top, mpstat, and kvm_stat results on the host. Stefan
Re: [Qemu-devel] TRIM, UNMAP and QCOW2 release of block information - Thin provisioning
On Sun, Aug 12, 2012 at 8:00 PM, Gerhard Wiesinger wrote: > As far as I saw QEMU/KVM supports the trim command on IDE/SATA devices and > the UNMAP command on SCSI devices/disks (thanks Paolo Bonzini). Will the > qcow2 format (or other formats) use this information and also release the > blocks for thin provisioning to save disk space on filesystems? > > VirtualBox also has such a feature in the new beta version: > http://www.fb-developers.info/blog/2012/virtualbox-v4-2-is-coming/ This has recently been discussed, please search the list for more info from Kevin Wolf or Paolo Bonzini. qcow2 marks the discarded blocks as free and will reuse them in future allocations. It does *not* discard at the OS level. For raw files you can get discard to work on an xfs host file system. In the future ext4 support can also be added. Stefan
Re: [Qemu-devel] [PATCH 2/3] vfio: vfio-pci device assignment driver
On Tue, Jul 31, 2012 at 11:18:15PM -0600, Alex Williamson wrote: > This adds the core of the QEMU VFIO-based PCI device assignment driver. > To make use of this driver, enable CONFIG_VFIO, CONFIG_VFIO_IOMMU_TYPE1, > and CONFIG_VFIO_PCI in your host Linux kernel config. Load the vfio-pci > module. To assign device :05:00.0 to a guest, do the following: > > for dev in $(ls /sys/bus/pci/devices/:05:00.0/iommu_group/devices); do > vendor=$(cat /sys/bus/pci/devices/$dev/vendor) > device=$(cat /sys/bus/pci/devices/$dev/device) > if [ -e /sys/bus/pci/devices/$dev/driver ]; then > echo $dev > /sys/bus/pci/devices/$dev/driver/unbind > fi > echo $vendor $device > /sys/bus/pci/drivers/vfio-pci/new_id > done Both vfio-pci and the old driver successfully match the $vendor:$device. What happens when another $vendor:$device PCI adapter is hotplugged into the host? Is there a way to bind vfio-pci on a per-adapter basis instead of a per-$vendor:$device? Stefan
[Qemu-devel] [PATCH] MAINTAINERS: Update email address for Stefan Hajnoczi
Switch to my personal email address. Signed-off-by: Stefan Hajnoczi --- Access to the IBM email address ceases from September 2012. Keeping qemu-trivial moving during September should be no problem. I'll be back 100% and continuing to contribute to QEMU in October again. MAINTAINERS |4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/MAINTAINERS b/MAINTAINERS index 708ad54..6d864c1 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -568,7 +568,7 @@ F: monitor.c Network device layer M: Anthony Liguori -M: Stefan Hajnoczi +M: Stefan Hajnoczi S: Maintained F: net/ T: git git://github.com/stefanha/qemu.git net @@ -588,7 +588,7 @@ F: slirp/ T: git git://git.kiszka.org/qemu.git queues/slirp Tracing -M: Stefan Hajnoczi +M: Stefan Hajnoczi S: Maintained F: trace/ F: scripts/tracetool.py -- 1.7.10.4
Re: [Qemu-devel] [PATCH] iscsi: fix race between task completition and task abortion
On Tue, Aug 14, 2012 at 08:44:46AM +0200, Stefan Priebe wrote:
> From: spriebe
CCing Ronnie, iSCSI block driver author.
>
> ---
> block/iscsi.c | 36
> 1 files changed, 20 insertions(+), 16 deletions(-)
>
> diff --git a/block/iscsi.c b/block/iscsi.c
> index 12ca76d..257f97f 100644
> --- a/block/iscsi.c
> +++ b/block/iscsi.c
> @@ -76,6 +76,10 @@ static void
> iscsi_abort_task_cb(struct iscsi_context *iscsi, int status, void
> *command_data,
> void *private_data)
> {
> + IscsiAIOCB *acb = (IscsiAIOCB *)private_data;
> +
> + scsi_free_scsi_task(acb->task);
> + acb->task = NULL;
> }
>
> static void
> @@ -85,14 +89,19 @@ iscsi_aio_cancel(BlockDriverAIOCB *blockacb)
> IscsiLun *iscsilun = acb->iscsilun;
>
> acb->common.cb(acb->common.opaque, -ECANCELED);
> -acb->canceled = 1;
>
> -/* send a task mgmt call to the target to cancel the task on the target
> */
> -iscsi_task_mgmt_abort_task_async(iscsilun->iscsi, acb->task,
> - iscsi_abort_task_cb, NULL);
> +if (acb->canceled != 0)
> +return;
> +
> +acb->canceled = 1;
>
> -/* then also cancel the task locally in libiscsi */
> -iscsi_scsi_task_cancel(iscsilun->iscsi, acb->task);
> +/* send a task mgmt call to the target to cancel the task on the target
> + * this also cancels the task in libiscsi
> + */
> +if (acb->task) {
> +iscsi_task_mgmt_abort_task_async(iscsilun->iscsi, acb->task,
> + iscsi_abort_task_cb, &acb);
> +}
> }
>
> static AIOPool iscsi_aio_pool = {
> @@ -184,6 +193,11 @@ iscsi_readv_writev_bh_cb(void *p)
> }
>
> qemu_aio_release(acb);
> +
> +if (acb->task) {
> + scsi_free_scsi_task(acb->task);
> + acb->task = NULL;
> +}
> }
>
>
> @@ -212,8 +226,6 @@ iscsi_aio_write16_cb(struct iscsi_context *iscsi, int
> status,
> }
>
> iscsi_schedule_bh(iscsi_readv_writev_bh_cb, acb);
> -scsi_free_scsi_task(acb->task);
> -acb->task = NULL;
> }
>
> static int64_t sector_qemu2lun(int64_t sector, IscsiLun *iscsilun)
> @@ -313,8 +325,6 @@ iscsi_aio_read16_cb(struct iscsi_context *iscsi, int
> status,
> }
>
> iscsi_schedule_bh(iscsi_readv_writev_bh_cb, acb);
> -scsi_free_scsi_task(acb->task);
> -acb->task = NULL;
> }
>
> static BlockDriverAIOCB *
> @@ -429,8 +439,6 @@ iscsi_synccache10_cb(struct iscsi_context *iscsi, int
> status,
> }
>
> iscsi_schedule_bh(iscsi_readv_writev_bh_cb, acb);
> -scsi_free_scsi_task(acb->task);
> -acb->task = NULL;
> }
>
> static BlockDriverAIOCB *
> @@ -483,8 +491,6 @@ iscsi_unmap_cb(struct iscsi_context *iscsi, int status,
> }
>
> iscsi_schedule_bh(iscsi_readv_writev_bh_cb, acb);
> -scsi_free_scsi_task(acb->task);
> -acb->task = NULL;
> }
>
> static BlockDriverAIOCB *
> @@ -560,8 +566,6 @@ iscsi_aio_ioctl_cb(struct iscsi_context *iscsi, int
> status,
> }
>
> iscsi_schedule_bh(iscsi_readv_writev_bh_cb, acb);
> -scsi_free_scsi_task(acb->task);
> -acb->task = NULL;
> }
>
> static BlockDriverAIOCB *iscsi_aio_ioctl(BlockDriverState *bs,
> --
> 1.7.2.5
>
>
Re: [Qemu-devel] [PATCH] iscsi: fix race between task completition and task abortion
On Tue, Aug 14, 2012 at 1:09 PM, ronnie sahlberg wrote: > Is a reply with the text > > Acked-by: Ronnie Sahlberg > > sufficient ? Yes
Re: [Qemu-devel] [PATCH] trace/simple: Fix compiler warning for 32 bit hosts
On Mon, Aug 13, 2012 at 09:50:56PM +0200, Stefan Weil wrote: > gcc complains when a 32 bit pointer is casted to a 64 bit integer. > > Cc: Stefan Hajnoczi > Signed-off-by: Stefan Weil > --- > scripts/tracetool/backend/simple.py |2 +- > 1 files changed, 1 insertions(+), 1 deletions(-) Thanks, applied to the tracing tree: https://github.com/stefanha/qemu/commits/tracing Stefan
Re: [Qemu-devel] [PATCH] trace/simple: Replace asprintf by g_strdup_printf
On Mon, Aug 13, 2012 at 09:51:16PM +0200, Stefan Weil wrote: > asprintf is not available for all hosts. g_strdup_printf is > more portable and simplifies the code because if does not > need error handling. > > The static variable does not need an explicit assignment to be NULL. > > Cc: Stefan Hajnoczi > Signed-off-by: Stefan Weil > --- > trace/simple.c | 14 -- > 1 files changed, 4 insertions(+), 10 deletions(-) Thanks, applied to the tracing tree: https://github.com/stefanha/qemu/commits/tracing Stefan
[Qemu-devel] [PATCH 1/6] trace: rename TraceRecordHeader to TraceLogHeader
From: Harsh Prateek Bora
The TraceRecordHeader is really the header for the entire trace log
file. It's not per-record header so make this obvious by renaming it.
Signed-off-by: Harsh Prateek Bora
Signed-off-by: Stefan Hajnoczi
---
trace/simple.c |4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/trace/simple.c b/trace/simple.c
index b700ea3..5d92939 100644
--- a/trace/simple.c
+++ b/trace/simple.c
@@ -70,7 +70,7 @@ typedef struct {
uint64_t header_event_id; /* HEADER_EVENT_ID */
uint64_t header_magic;/* HEADER_MAGIC*/
uint64_t header_version; /* HEADER_VERSION */
-} TraceRecordHeader;
+} TraceLogHeader;
static void read_from_buffer(unsigned int idx, void *dataptr, size_t size);
@@ -295,7 +295,7 @@ void st_set_trace_file_enabled(bool enable)
flush_trace_file(true);
if (enable) {
-static const TraceRecordHeader header = {
+static const TraceLogHeader header = {
.header_event_id = HEADER_EVENT_ID,
.header_magic = HEADER_MAGIC,
/* Older log readers will check for version at next location */
--
1.7.10.4
[Qemu-devel] [PATCH 2/6] trace: remove unnecessary write_to_buffer() typecasting
From: Harsh Prateek Bora The buffer argument is void* so it is not necessary to cast. Signed-off-by: Harsh Prateek Bora Signed-off-by: Stefan Hajnoczi --- trace/simple.c |6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/trace/simple.c b/trace/simple.c index 5d92939..a0e0f05 100644 --- a/trace/simple.c +++ b/trace/simple.c @@ -235,9 +235,9 @@ int trace_record_start(TraceBufferRecord *rec, TraceEventID event, size_t datasi rec->next_tbuf_idx = new_idx % TRACE_BUF_LEN; rec_off = idx; -rec_off = write_to_buffer(rec_off, (uint8_t*)&event, sizeof(event)); -rec_off = write_to_buffer(rec_off, (uint8_t*)×tamp_ns, sizeof(timestamp_ns)); -rec_off = write_to_buffer(rec_off, (uint8_t*)&rec_len, sizeof(rec_len)); +rec_off = write_to_buffer(rec_off, &event, sizeof(event)); +rec_off = write_to_buffer(rec_off, ×tamp_ns, sizeof(timestamp_ns)); +rec_off = write_to_buffer(rec_off, &rec_len, sizeof(rec_len)); rec->tbuf_idx = idx; rec->rec_off = (idx + sizeof(TraceRecord)) % TRACE_BUF_LEN; -- 1.7.10.4
[Qemu-devel] [PATCH 5/6] trace/simple: Fix compiler warning for 32 bit hosts
From: Stefan Weil
gcc complains when a 32 bit pointer is casted to a 64 bit integer.
Cc: Stefan Hajnoczi
Signed-off-by: Stefan Weil
Signed-off-by: Stefan Hajnoczi
---
scripts/tracetool/backend/simple.py |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/scripts/tracetool/backend/simple.py
b/scripts/tracetool/backend/simple.py
index c7e47d6..e4b4a7f 100644
--- a/scripts/tracetool/backend/simple.py
+++ b/scripts/tracetool/backend/simple.py
@@ -79,7 +79,7 @@ def c(events):
)
# pointer var (not string)
elif type_.endswith('*'):
-out('trace_record_write_u64(&rec, (uint64_t)(uint64_t
*)%(name)s);',
+out('trace_record_write_u64(&rec, (uintptr_t)(uint64_t
*)%(name)s);',
name = name,
)
# primitive data type
--
1.7.10.4
[Qemu-devel] [PATCH 3/6] trace: drop unused TraceBufferRecord->next_tbuf_idx field
From: Harsh Prateek Bora
Signed-off-by: Harsh Prateek Bora
Signed-off-by: Stefan Hajnoczi
---
trace/simple.c |2 --
trace/simple.h |1 -
2 files changed, 3 deletions(-)
diff --git a/trace/simple.c b/trace/simple.c
index a0e0f05..4fed07f 100644
--- a/trace/simple.c
+++ b/trace/simple.c
@@ -231,8 +231,6 @@ int trace_record_start(TraceBufferRecord *rec, TraceEventID
event, size_t datasi
}
idx = old_idx % TRACE_BUF_LEN;
-/* To check later if threshold crossed */
-rec->next_tbuf_idx = new_idx % TRACE_BUF_LEN;
rec_off = idx;
rec_off = write_to_buffer(rec_off, &event, sizeof(event));
diff --git a/trace/simple.h b/trace/simple.h
index 7e521c1..2ab96a8 100644
--- a/trace/simple.h
+++ b/trace/simple.h
@@ -29,7 +29,6 @@ void st_flush_trace_buffer(void);
typedef struct {
unsigned int tbuf_idx;
-unsigned int next_tbuf_idx;
unsigned int rec_off;
} TraceBufferRecord;
--
1.7.10.4
[Qemu-devel] [PATCH 6/6] trace/simple: Replace asprintf by g_strdup_printf
From: Stefan Weil
asprintf is not available for all hosts. g_strdup_printf is
more portable and simplifies the code because if does not
need error handling.
The static variable does not need an explicit assignment to be NULL.
Cc: Stefan Hajnoczi
Signed-off-by: Stefan Weil
Signed-off-by: Stefan Hajnoczi
---
trace/simple.c | 14 --
1 file changed, 4 insertions(+), 10 deletions(-)
diff --git a/trace/simple.c b/trace/simple.c
index 8e175ec..d83681b 100644
--- a/trace/simple.c
+++ b/trace/simple.c
@@ -55,7 +55,7 @@ static unsigned int trace_idx;
static unsigned int writeout_idx;
static uint64_t dropped_events;
static FILE *trace_fp;
-static char *trace_file_name = NULL;
+static char *trace_file_name;
/* * Trace buffer entry */
typedef struct {
@@ -329,18 +329,12 @@ bool st_set_trace_file(const char *file)
{
st_set_trace_file_enabled(false);
-free(trace_file_name);
+g_free(trace_file_name);
if (!file) {
-if (asprintf(&trace_file_name, CONFIG_TRACE_FILE, getpid()) < 0) {
-trace_file_name = NULL;
-return false;
-}
+trace_file_name = g_strdup_printf(CONFIG_TRACE_FILE, getpid());
} else {
-if (asprintf(&trace_file_name, "%s", file) < 0) {
-trace_file_name = NULL;
-return false;
-}
+trace_file_name = g_strdup_printf("%s", file);
}
st_set_trace_file_enabled(true);
--
1.7.10.4
[Qemu-devel] [PATCH 4/6] trace: avoid pointer aliasing in trace_record_finish()
From: Harsh Prateek Bora
Declaring a TraceRecord on the stack works fine. No need for a
uint8_t array and pointer aliasing.
Signed-off-by: Harsh Prateek Bora
Signed-off-by: Stefan Hajnoczi
---
trace/simple.c |9 -
1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/trace/simple.c b/trace/simple.c
index 4fed07f..8e175ec 100644
--- a/trace/simple.c
+++ b/trace/simple.c
@@ -269,12 +269,11 @@ static unsigned int write_to_buffer(unsigned int idx,
void *dataptr, size_t size
void trace_record_finish(TraceBufferRecord *rec)
{
-uint8_t temp_rec[sizeof(TraceRecord)];
-TraceRecord *record = (TraceRecord *) temp_rec;
-read_from_buffer(rec->tbuf_idx, temp_rec, sizeof(TraceRecord));
+TraceRecord record;
+read_from_buffer(rec->tbuf_idx, &record, sizeof(TraceRecord));
smp_wmb(); /* write barrier before marking as valid */
-record->event |= TRACE_RECORD_VALID;
-write_to_buffer(rec->tbuf_idx, temp_rec, sizeof(TraceRecord));
+record.event |= TRACE_RECORD_VALID;
+write_to_buffer(rec->tbuf_idx, &record, sizeof(TraceRecord));
if ((trace_idx - writeout_idx) > TRACE_BUF_FLUSH_THRESHOLD) {
flush_trace_file(false);
--
1.7.10.4
[Qemu-devel] [PULL 0/6 1.2] Tracing patches for QEMU 1.2
The last pull request from 19 July was not merged. Here it is rebased on qemu.git/master with two additional fixes from Stefan Weil. The following changes since commit 633decd71119a4293e5e53e6059026c517a8bef0: Merge remote-tracking branch 'qmp/queue/qmp' into staging (2012-08-13 16:12:35 -0500) are available in the git repository at: git://github.com/stefanha/qemu.git tracing for you to fetch changes up to 4552e41025af4694c55854448c3ae4d95e72c7f6: trace/simple: Replace asprintf by g_strdup_printf (2012-08-14 13:19:57 +0100) Harsh Prateek Bora (4): trace: rename TraceRecordHeader to TraceLogHeader trace: remove unnecessary write_to_buffer() typecasting trace: drop unused TraceBufferRecord->next_tbuf_idx field trace: avoid pointer aliasing in trace_record_finish() Stefan Weil (2): trace/simple: Fix compiler warning for 32 bit hosts trace/simple: Replace asprintf by g_strdup_printf scripts/tracetool/backend/simple.py |2 +- trace/simple.c | 35 +-- trace/simple.h |1 - 3 files changed, 14 insertions(+), 24 deletions(-) -- 1.7.10.4
[Qemu-devel] [PATCH] net: add -netdev options to man page
Document the -netdev syntax which supercedes the older -net syntax.
This patch is a first step to making -netdev prominent in the QEMU
manual.
Reported-by: Anatoly Techtonik
Signed-off-by: Stefan Hajnoczi
---
qemu-options.hx |7 +++
1 file changed, 7 insertions(+)
diff --git a/qemu-options.hx b/qemu-options.hx
index 47cb5bd..ea9b824 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -1335,6 +1335,7 @@ Valid values for @var{type} are
Not all devices are supported on all targets. Use -net nic,model=?
for a list of available devices for your target.
+@item -netdev user,id=@var{id}[,@var{option}][,@var{option}][,...]
@item -net user[,@var{option}][,@var{option}][,...]
Use the user mode network stack which requires no administrator
privilege to run. Valid options are:
@@ -1343,6 +1344,7 @@ privilege to run. Valid options are:
@item vlan=@var{n}
Connect user mode stack to VLAN @var{n} (@var{n} = 0 is the default).
+@item id=@var{id}
@item name=@var{name}
Assign symbolic name for use in monitor commands.
@@ -1468,6 +1470,7 @@ processed and applied to -net user. Mixing them with the
new configuration
syntax gives undefined results. Their use for new applications is discouraged
as they will be removed from future versions.
+@item -netdev
tap,id=@var{id}[,fd=@var{h}][,ifname=@var{name}][,script=@var{file}][,downscript=@var{dfile}][,helper=@var{helper}]
@item -net
tap[,vlan=@var{n}][,name=@var{name}][,fd=@var{h}][,ifname=@var{name}][,script=@var{file}][,downscript=@var{dfile}][,helper=@var{helper}]
Connect the host TAP network interface @var{name} to VLAN @var{n}.
@@ -1507,6 +1510,7 @@ qemu-system-i386 linux.img \
-net nic -net
tap,"helper=/usr/local/libexec/qemu-bridge-helper"
@end example
+@item -netdev bridge,id=@var{id}[,br=@var{bridge}][,helper=@var{helper}]
@item -net
bridge[,vlan=@var{n}][,name=@var{name}][,br=@var{bridge}][,helper=@var{helper}]
Connect a host TAP network interface to a host bridge device.
@@ -1529,6 +1533,7 @@ qemu-system-i386 linux.img -net bridge -net
nic,model=virtio
qemu-system-i386 linux.img -net bridge,br=qemubr0 -net nic,model=virtio
@end example
+@item -netdev
socket,id=@var{id}[,fd=@var{h}][,listen=[@var{host}]:@var{port}][,connect=@var{host}:@var{port}]
@item -net socket[,vlan=@var{n}][,name=@var{name}][,fd=@var{h}]
[,listen=[@var{host}]:@var{port}][,connect=@var{host}:@var{port}]
Connect the VLAN @var{n} to a remote VLAN in another QEMU virtual
@@ -1551,6 +1556,7 @@ qemu-system-i386 linux.img \
-net socket,connect=127.0.0.1:1234
@end example
+@item -netdev
socket,id=@var{id}[,fd=@var{h}][,mcast=@var{maddr}:@var{port}[,localaddr=@var{addr}]]
@item -net
socket[,vlan=@var{n}][,name=@var{name}][,fd=@var{h}][,mcast=@var{maddr}:@var{port}[,localaddr=@var{addr}]]
Create a VLAN @var{n} shared with another QEMU virtual
@@ -1602,6 +1608,7 @@ qemu-system-i386 linux.img \
-net socket,mcast=239.192.168.1:1102,localaddr=1.2.3.4
@end example
+@item -netdev
vde,id=@var{id}[,sock=@var{socketpath}][,port=@var{n}][,group=@var{groupname}][,mode=@var{octalmode}]
@item -net vde[,vlan=@var{n}][,name=@var{name}][,sock=@var{socketpath}]
[,port=@var{n}][,group=@var{groupname}][,mode=@var{octalmode}]
Connect VLAN @var{n} to PORT @var{n} of a vde switch running on host and
listening for incoming connections on @var{socketpath}. Use GROUP
@var{groupname}
--
1.7.10.4
Re: [Qemu-devel] QEMU 1.2 Test Day - August 16 2012
On Thu, Aug 2, 2012 at 1:22 PM, Stefan Hajnoczi wrote: > I have set up the QEMU 1.2 Testing wiki page and suggest August 16 as > the Test Day: > > http://wiki.qemu.org/Planning/1.2/Testing QEMU 1.2 Test Day is only 2 days away! Please help test QEMU 1.2-rc and add yourself to the wiki: http://wiki.qemu.org/Planning/1.2/Testing More about QEMU 1.2 Test Day from my original email: > Test Day is an event for QEMU contributors and users to try out the > release candidate. QEMU has a large feature matrix that is hard to > test by a single entity. On Test Day everyone is encouraged to test > their favorite features, host OSes, and guest OSes to make sure that > the release candidate is stable. > > Please add yourself to the http://wiki.qemu.org/Planning/1.2/Testing wiki > page! > > The Test Day is August 16 2012, one day after the planned -rc0 > release. On the day, use #qemu IRC on irc.oftc.net to chat about bugs > and update the wiki page with your pass/fail results. > > There are usually -rc1, -rc2, ... follow-up release candidates. > Please retest those if you have time, especially if bugs you found are > supposed to be resolved. > > Stefan
Re: [Qemu-devel] [PATCH] block-migration: deprecate block migration for the 1.2 release
On Tue, Aug 14, 2012 at 08:32:31AM -0500, Anthony Liguori wrote:
> To be replaced with live block copy.
>
> Signed-off-by: Anthony Liguori
> ---
> migration.c |9 +
> 1 files changed, 9 insertions(+), 0 deletions(-)
>
> diff --git a/migration.c b/migration.c
> index 653a3c1..babccf4 100644
> --- a/migration.c
> +++ b/migration.c
> @@ -482,10 +482,19 @@ void qmp_migrate(const char *uri, bool has_blk, bool
> blk,
> MigrationParams params;
> const char *p;
> int ret;
> +static bool suppress_deprecation_message;
>
> params.blk = blk;
> params.shared = inc;
>
> +if (blk && !suppress_deprecation_message) {
> +qerror_report(ERROR_CLASS_GENERIC_ERROR,
qerror_report_once() would be nice :).
> + "Block migration is deprecated. "
> + "See http://wiki.qemu.org/Features/LiveBlockCopy "
The page doesn't exist, I think it should be:
http://wiki.qemu.org/Features/LiveBlockMigration
Re: [Qemu-devel] [PULL 0/6 1.2] Tracing patches for QEMU 1.2
On Tue, Aug 14, 2012 at 7:44 PM, Anthony Liguori wrote: > Stefan Hajnoczi writes: > >> The last pull request from 19 July was not merged. Here it is rebased on >> qemu.git/master with two additional fixes from Stefan Weil. > > I'm not sure what you mean: > > commit 903f650b0c77827f8d92b35f61419401d648df1e > Merge: 61dc008 90a147a > Author: Anthony Liguori > Date: Mon Jul 23 13:15:34 2012 -0500 > > Merge remote-tracking branch 'stefanha/tracing' into staging > > * stefanha/tracing: > Update simpletrace.py for new log format > Simpletrace v2: Support multiple arguments, strings. > monitor: remove unused do_info_trace > trace: added ability to comment out events in the list > > > Your pull request was for 90a147a275da3a432bdf00238ebf438eff1d2c1b which > is what is being merged here. > > What your pull request inaccurate? You are right. I was confused and thought I previously sent a pull request with Harsh's cleanup patches when I didn't. The pull request I raised yesterday (4552e41025af4694c55854448c3ae4d95e72c7f6) is good though. Sorry, Stefan
Re: [Qemu-devel] [RFC V3 9/9] quorum: Add quorum mechanism.
On Tue, Aug 14, 2012 at 04:14:11PM +0200, Benoît Canet wrote:
> +#define QUORUM_FREE_QIOV_ITEMS(qlist) do { \
> +QLIST_FOREACH_SAFE(item, qlist, next, next_item) { \
> +QLIST_REMOVE(item, next); \
> +g_free(item); \
> +} } while (0)
This is only used once, please open code it and don't use a macro.
> +
> +static void quorum_free_vote_list(QuorumAIOCB *acb)
> +{
> +QuorumIOVectorVersion *version, *next_version;
> +QuorumIOVectorItem *item, *next_item;
> +
> +QLIST_FOREACH_SAFE(version, &acb->vote_list, next, next_version) {
> +QLIST_REMOVE(version, next);
> +QUORUM_FREE_QIOV_ITEMS(&version->qiov_items);
> +g_free(version);
> +}
> +}
> +
> +#undef QUORUM_FREE_QIOV_ITEMS
Re: [Qemu-devel] [RFC V3 8/9] quorum: Add quorum_aio_readv.
On Tue, Aug 14, 2012 at 04:14:10PM +0200, Benoît Canet wrote:
> Signed-off-by: Benoit Canet
> ---
> block/quorum.c | 35 ++-
> 1 file changed, 34 insertions(+), 1 deletion(-)
>
> diff --git a/block/quorum.c b/block/quorum.c
> index 86962b4..8b449fb 100644
> --- a/block/quorum.c
> +++ b/block/quorum.c
> @@ -190,10 +190,16 @@ static void quorum_aio_bh(void *opaque)
> {
> QuorumAIOCB *acb = opaque;
> BDRVQuorumState *s = acb->bqs;
> -int ret;
> +int i, ret;
>
> ret = s->n <= acb->success_count ? 0 : -EIO;
>
> +for (i = 0; i < s->m; i++) {
> +qemu_vfree(acb->aios[i].buf);
> +acb->aios[i].buf = NULL;
> +acb->aios[i].ret = 0;
> +}
> +
> qemu_bh_delete(acb->bh);
> acb->common.cb(acb->common.opaque, ret);
> if (acb->finished) {
> @@ -258,6 +264,32 @@ static void quorum_aio_cb(void *opaque, int ret)
> qemu_bh_schedule(acb->bh);
> }
>
> +static BlockDriverAIOCB *quorum_aio_readv(BlockDriverState *bs,
> + int64_t sector_num,
> + QEMUIOVector *qiov,
> + int nb_sectors,
> + BlockDriverCompletionFunc *cb,
> + void *opaque)
> +{
> +BDRVQuorumState *s = bs->opaque;
> +QuorumAIOCB *acb = quorum_aio_get(s, bs, qiov, sector_num,
> + nb_sectors, cb, opaque);
> +int i;
> +
> +for (i = 0; i < s->m; i++) {
> +acb->aios[i].buf = qemu_blockalign(bs->file, qiov->size);
> +qemu_iovec_init(&acb->qiovs[i], qiov->niov);
> +qemu_iovec_clone(&acb->qiovs[i], qiov, acb->aios[i].buf);
> +}
Need to call qemu_iovec_destroy() to free &acb->qiovs[i] iovecs.
Stefan
Re: [Qemu-devel] [PATCH] block-migration: deprecate block migration for the 1.2 release
On Wed, Aug 15, 2012 at 10:12:42AM +0200, Ruben Kerkhof wrote:
> On Tue, Aug 14, 2012 at 4:42 PM, Stefan Hajnoczi
> wrote:
> > On Tue, Aug 14, 2012 at 08:32:31AM -0500, Anthony Liguori wrote:
> >> To be replaced with live block copy.
> >>
> >> Signed-off-by: Anthony Liguori
> >> ---
> >> migration.c |9 +
> >> 1 files changed, 9 insertions(+), 0 deletions(-)
> >>
> >> diff --git a/migration.c b/migration.c
> >> index 653a3c1..babccf4 100644
> >> --- a/migration.c
> >> +++ b/migration.c
> >> @@ -482,10 +482,19 @@ void qmp_migrate(const char *uri, bool has_blk, bool
> >> blk,
> >> MigrationParams params;
> >> const char *p;
> >> int ret;
> >> +static bool suppress_deprecation_message;
> >>
> >> params.blk = blk;
> >> params.shared = inc;
> >>
> >> +if (blk && !suppress_deprecation_message) {
> >> +qerror_report(ERROR_CLASS_GENERIC_ERROR,
> >
> > qerror_report_once() would be nice :).
> >
> >> + "Block migration is deprecated. "
> >> + "See http://wiki.qemu.org/Features/LiveBlockCopy "
> >
> > The page doesn't exist, I think it should be:
> > http://wiki.qemu.org/Features/LiveBlockMigration
>
> Can the new live block copy method still use tcp just like the current
> block migration? The wiki page only mentions iscsi.
> I make extensive use of block migration over tcp, which works fine and
> is handled by libvirt. I'd rather not introduce iscsi in my
> environment.
The new live block copy approach is different and that's why classic
block migration is only deprecated but not dropped:
Live block copy doesn't transfer data in-band during live migration.
Instead it currently requires storage access from both hosts, for
example:
1. NFS or CIFS
2. iSCSI or NBD
I think when classic block migration is removed for good it should be
just as easy to use through libvirt using TCP because that's still a
valid use case and probably the simplest one to get started. (Libvirt
could orchestrate an NBD connection behind the scenes, for example.)
Stefan
Re: [Qemu-devel] [PATCH] qemu-iotests: Fix 030 after switch to GenericError
On Wed, Aug 15, 2012 at 1:11 PM, Kevin Wolf wrote: > Signed-off-by: Kevin Wolf > --- > tests/qemu-iotests/030 |6 ++ > 1 files changed, 2 insertions(+), 4 deletions(-) Reviewed-by: Stefan Hajnoczi
Re: [Qemu-devel] [PATCH] block: Flush parent to OS with cache=unsafe
On Wed, Aug 15, 2012 at 11:57 AM, Kevin Wolf wrote: > Commit 29cdb251 already added a comment that no unnecessary flushes to > disk will occur, this patch makes the code even get to the point of the > comment. This is mostly theoretical because in practice we only stack > one format on top of one protocol, the former implementing flush_to_os > and the latter only flush_to_disk. It starts to matter when drivers that > are not on top implement flush_to_os. > > Signed-off-by: Kevin Wolf > --- > block.c |3 ++- > 1 files changed, 2 insertions(+), 1 deletions(-) Reviewed-by: Stefan Hajnoczi
Re: [Qemu-devel] [RFC V3 0/9] Quorum disk image corruption resiliency
On Tue, Aug 14, 2012 at 3:14 PM, Benoît Canet wrote: > This patchset create a block driver implementing a quorum using m qemu disk > images. Writes are mirrored on the m files. > For the reading part the m files are read at the same time and a vote is > done to determine if a qiov version is present n or more times. It then return > this majority version to the upper layers. > When i < n versions of the data are returned by the lower layer the > quorum is broken and the read return -EIO. > > The goal of this patchset is to be turned in a QEMU block filter living just > above raw-*.c and below qcow2/qed when the required infrastructure will be > done. > > Main use of this feature will be people using NFS appliances which can be > subjected to bitflip errors. > > This patchset can be used to replace blkverify and the out of tree blkmirror. > > usage: -drive file=quorum:n/m:image_1.raw:...:image_m.raw,if=virtio,cache=none > > in v2: > > eblake: fix typos > squash two first commits > > afärber: Modify the Makefile on first commit > > bcanet: move function prototype of quorum.c one patch down > > in v3: > > Blue Swirl: change char * to uint8_t * in QuorumSingleAIOCB > > Eric Blake: Add escaping of the : separator > Allow to specify the n/m ratio parameters of the Quorum > > Stefan Hajnoczi: Squash quorum_close and quorum_open patch to avoid leak > Add missing bdrv_delete() in quorum_close > simpler quorum_getlength > make the quorum_check_ret threshold a user setting (bind it > to n) > move blkverify_iovec_clone() and blkverify_iovec_compare() > to cutils.c > free unconditionally qemu_blockalign() with qemu_vfree() > turn assignement into assert in quorum_copy_qiov() > > Benoît Canet (9): > quorum: Create quorum.c, add QuorumSingleAIOCB and QuorumAIOCB. > quorum: Create BDRVQuorumState and BlkDriver and do init. > quorum: Add quorum_open() and quorum_close(). > quorum: Add quorum_getlength(). > quorum: Add quorum_aio_writev and its dependencies. > blkverify: Extract qemu_iovec_clone() and qemu_iovec_compare() from > blkverify. > quorum: Add quorum_co_flush(). > quorum: Add quorum_aio_readv. > quorum: Add quorum mechanism. > > block/Makefile.objs |1 + > block/blkverify.c | 108 +- > block/quorum.c | 559 > +++ > cutils.c| 103 ++ > qemu-common.h |2 + > 5 files changed, 667 insertions(+), 106 deletions(-) > create mode 100644 block/quorum.c BTW once this feature is merged we could drop blkverify since quorum is more generic and provides blkverify behavior in the n=2/m=2 configuration. Stefan
Re: [Qemu-devel] [PATCH v2] Makefile: Avoid explicit list of directories in clean target
On Wed, Aug 15, 2012 at 12:29:24PM +0100, Peter Maydell wrote: > Avoid having an explicit list of directories in the 'clean' > target by using 'find' to remove all .o and .d files instead. > > Signed-off-by: Peter Maydell > --- > Changes v1->v2: use portable 'find -exec' rather than unsafe 'find | xargs' > or unportable 'find -print0 | xargs -0'. > > Makefile |7 ++- > 1 file changed, 2 insertions(+), 5 deletions(-) Thanks, applied to the trivial patches tree: https://github.com/stefanha/qemu/commits/trivial-patches Stefan
Re: [Qemu-devel] [Qemu-trivial] [PATCH] ehci: fix assertion typo
On Tue, Aug 14, 2012 at 04:13:02PM +0200, Alejandro Martinez Ruiz wrote: > Signed-off-by: Alejandro Martinez Ruiz > --- > hw/usb/hcd-ehci.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) Looks good but would be good to hear from Gerd just to be sure. Thanks, applied to the trivial patches tree: https://github.com/stefanha/qemu/commits/trivial-patches Stefan
Re: [Qemu-devel] [PATCH] device_tree: load_device_tree(): Allow NULL sizep
On Sat, Aug 11, 2012 at 01:33:42PM +0100, Stefan Hajnoczi wrote:
> On Sat, Aug 11, 2012 at 12:11 AM, Peter Crosthwaite
> wrote:
> > On Fri, Aug 10, 2012 at 11:42 PM, Stefan Hajnoczi
> > wrote:
> >> On Fri, Aug 10, 2012 at 01:54:26PM +1000, Peter A. G. Crosthwaite wrote:
> >>> The sizep arg is populated with the size of the loaded device tree. Since
> >>> this
> >>> is one of those informational "please populate" type arguments it should
> >>> be
> >>> optional. Guarded writes to *sizep against NULL accordingly.
> >>>
> >>> Signed-off-by: Peter A. G. Crosthwaite
> >>> Acked-by: Alexander Graf
> >>> ---
> >>> device_tree.c |8 ++--
> >>> 1 files changed, 6 insertions(+), 2 deletions(-)
> >>>
> >>> diff --git a/device_tree.c b/device_tree.c
> >>> index d7a9b6b..641a48a 100644
> >>> --- a/device_tree.c
> >>> +++ b/device_tree.c
> >>> @@ -71,7 +71,9 @@ void *load_device_tree(const char *filename_path, int
> >>> *sizep)
> >>> int ret;
> >>> void *fdt = NULL;
> >>>
> >>> -*sizep = 0;
> >>> +if (sizep) {
> >>> +*sizep = 0;
> >>> +}
> >>> dt_size = get_image_size(filename_path);
> >>> if (dt_size < 0) {
> >>> printf("Unable to get size of device tree file '%s'\n",
> >>> @@ -104,7 +106,9 @@ void *load_device_tree(const char *filename_path, int
> >>> *sizep)
> >>> filename_path);
> >>> goto fail;
> >>> }
> >>> -*sizep = dt_size;
> >>> +if (sizep) {
> >>> +*sizep = dt_size;
> >>> +}
> >>
> >> What can the caller do with this void* buffer without knowing its size?
> >>
> >
> > Sanity check the machine:
> >
> > dtb = load_device_tree( ... ); //dont care how big it is
> > foo = fdt_gep_prop( dtb, ... );
> > if (foo != object_get_prop(foo_device, foo_prop, ... )) {
> > hw_error("your dtb is bad because ... !\n", ... );
> > }
>
> What happens if the fdt is corrupt or malicious? I guess we'll access
> memory beyond the end of blob.
>
> This seems to be libfdt's fault. I didn't see an API to validate the
> blob's size.
>
> I'm "happy" with this patch but if fdt's can ever come from untrusted
> sources then we're in trouble.
Jon/David, can you confirm that libfdt has no way of check the size of
the fdt blob?
For example, if I pass a corrupt or malicious blob to libfdt, is there a
way to detect that or will it access memory beyond the end of the blob
as we query the device tree?
Stefan
Re: [Qemu-devel] [PATCH] cputlb.c: Fix out of date comment
On Fri, Aug 10, 2012 at 05:14:05PM +0100, Peter Maydell wrote: > The comment about the return address from get_page_addr_code() was > well out of date as phys_ram_base has not existed for some time. > > Signed-off-by: Peter Maydell > --- > cputlb.c |4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) Thanks, applied to the trivial patches tree: https://github.com/stefanha/qemu/commits/trivial-patches Stefan
Re: [Qemu-devel] [Qemu-trivial] [PATCH] Fix QEMU
On Fri, Aug 10, 2012 at 09:48:07PM +0200, Stefan Weil wrote: > One more... > > Signed-off-by: Stefan Weil > --- > scripts/simpletrace.py |2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) Updated the commit description to be more specific :). Thanks, applied to the trivial patches tree: https://github.com/stefanha/qemu/commits/trivial-patches Stefan
Re: [Qemu-devel] [Qemu-trivial] [PATCH] vnc: Improve comment
On Fri, Aug 10, 2012 at 09:51:33PM +0200, Stefan Weil wrote: > It's still not good, but hopefully better than before. > > Signed-off-by: Stefan Weil > --- > ui/vnc-enc-zywrle.h |4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/ui/vnc-enc-zywrle.h b/ui/vnc-enc-zywrle.h > index 188e247..addee55 100644 > --- a/ui/vnc-enc-zywrle.h > +++ b/ui/vnc-enc-zywrle.h > @@ -305,8 +305,8 @@ static inline void harr(int8_t *px0, int8_t *px1) > |L1H0H1H0|L1H0H1H0|L1H0H1H0|L1H0H1H0| : level 1 > > In this method, H/L and X0/X1 is always same position. > - This lead us to more speed and less memory. > - Of cause, the result of both methods is quite same > + This leads us to more speed and less memory. > + Of course, the result of both methods is quite same > because its only difference is that coefficient position. > */ > static inline void wavelet_level(int *data, int size, int l, int skip_pixel) I would drop this patch since this file seems to be copied from an external project and not customized for QEMU: "THIS FILE IS PART OF THE 'ZYWRLE' VNC CODEC SOURCE CODE." If we need to sync to a newer version in the future, the spelling fixes just add diff noise. Stefan
Re: [Qemu-devel] [Qemu-trivial] [PATCH] docs: Fix spelling (propery -> property)
On Fri, Aug 10, 2012 at 09:53:02PM +0200, Stefan Weil wrote: > Signed-off-by: Stefan Weil > --- > docs/bootindex.txt |2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) Thanks, applied to the trivial patches tree: https://github.com/stefanha/qemu/commits/trivial-patches Stefan
Re: [Qemu-devel] [PATCH] Spelling fix in comment (peripherans -> peripherals)
On Fri, Aug 10, 2012 at 09:56:46PM +0200, Stefan Weil wrote: > Signed-off-by: Stefan Weil > --- > hw/versatilepb.c |2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) Thanks, applied to the trivial patches tree: https://github.com/stefanha/qemu/commits/trivial-patches Stefan
Re: [Qemu-devel] [Qemu-trivial] [PATCH] srp: Don't use QEMU_PACKED for single elements of a structured type
On Fri, Aug 10, 2012 at 10:03:27PM +0200, Stefan Weil wrote:
> QEMU_PACKED results in a MinGW compiler warning when it is
> used for single structure elements:
>
> warning: 'gcc_struct' attribute ignored
>
> Using QEMU_PACKED for the whole structure avoids the compiler warning
> without changing the memory layout.
Quick link for other reviewers:
http://gcc.gnu.org/onlinedocs/gcc-4.7.1/gcc/Type-Attributes.html#Type-Attributes
>
> Signed-off-by: Stefan Weil
> ---
> hw/srp.h |8
> 1 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/hw/srp.h b/hw/srp.h
> index 3009bd5..5e0cad5 100644
> --- a/hw/srp.h
> +++ b/hw/srp.h
> @@ -177,13 +177,13 @@ struct srp_tsk_mgmt {
> uint8_treserved1[6];
> uint64_t tag;
> uint8_treserved2[4];
> -uint64_t lun QEMU_PACKED;
> +uint64_t lun;
> uint8_treserved3[2];
> uint8_ttsk_mgmt_func;
> uint8_treserved4;
> uint64_t task_tag;
> uint8_treserved5[8];
> -};
> +} QEMU_PACKED;
Here I actually see a difference for the uint64_t task_tag field.
Previously it was not packed, now it is packed and because it has 4 *
uint8_t before it there will be a difference in layout.
Looking at how QEMU accesses srp_tsk_mgmt, I think we're safe because we
never actually access task_tag?
Ben: Any thoughts on this patch?
Stefan
Re: [Qemu-devel] [PATCH] framebuffer: Fix spelling in comment (leight -> height)
On Sat, Aug 11, 2012 at 09:32:02PM +0200, Stefan Weil wrote: > Signed-off-by: Stefan Weil > --- > hw/framebuffer.c |2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) Let there be leight! Thanks, applied to the trivial patches tree: https://github.com/stefanha/qemu/commits/trivial-patches Stefan
Re: [Qemu-devel] [Qemu-trivial] [PATCH] iov_send_recv(): Handle zero bytes case even if OS does not
On Sat, Aug 11, 2012 at 10:24:35PM +0100, Peter Maydell wrote: > POSIX allows sendmsg() and recvmsg() to fail EMSGSIZE if passed a zero > msg.msg_iovlen (in particular the MacOS X implementation will do this). > Handle the case where iov_send_recv() is passed a zero byte count > explicitly, to avoid accidentally depending on the OS to treat zero > msg_iovlen as a no-op. > > Signed-off-by: Peter Maydell > --- > This is what was causing 'make check' to fail on MacOS X. > The other option was to declare that a zero bytecount was illegal, I guess. > > iov.c | 7 +++ > 1 file changed, 7 insertions(+) Thanks, applied to the trivial patches tree: https://github.com/stefanha/qemu/commits/trivial-patches Stefan
[Qemu-devel] [PULL 0/9 for-1.2] Trivial patches for 11 to 15 August 2012
The last pull request for QEMU 1.2! The following changes since commit 03834e22abafbc8dc4052d46a5ccd6dd135a54a3: Merge remote-tracking branch 'origin/master' into staging (2012-08-14 15:19:50 -0500) are available in the git repository at: git://github.com/stefanha/qemu.git trivial-patches for you to fetch changes up to c3594ed73e0a7e7feae309be79f0eb6bafcc02ab: ivshmem, qdev-monitor: fix order of qerror parameters (2012-08-15 15:37:08 +0100) Alberto Garcia (1): ivshmem, qdev-monitor: fix order of qerror parameters Alejandro Martinez Ruiz (1): ehci: fix assertion typo Peter Maydell (3): Makefile: Avoid explicit list of directories in clean target cputlb.c: Fix out of date comment iov_send_recv(): Handle zero bytes case even if OS does not Stefan Weil (4): trace: Fix "Qemu" -> "QEMU" docs: Fix spelling (propery -> property) Spelling fix in comment (peripherans -> peripherals) framebuffer: Fix spelling in comment (leight -> height) Makefile |7 ++- cputlb.c |4 +++- docs/bootindex.txt |2 +- hw/framebuffer.c |2 +- hw/ivshmem.c |3 ++- hw/qdev-monitor.c |2 +- hw/usb/hcd-ehci.c |2 +- hw/versatilepb.c |2 +- iov.c |7 +++ scripts/simpletrace.py |2 +- 10 files changed, 20 insertions(+), 13 deletions(-) -- 1.7.10.4
Re: [Qemu-devel] Hard freeze for 1.2 today
On Wed, Aug 15, 2012 at 3:22 PM, wrote: > > Hi, > > Today is the hard freeze for 1.2. If you have any pull requests and/or > patches targetted for the hard freeze, please send them by 3pm > US/Central time today and clearly mark them "for-1.2". > > If there are existing patches and/or pull requests on the mailing list > that you think should be in 1.2, please respond to the email with a > pointer to the mail. Trivial patches pull request: http://thread.gmane.org/gmane.comp.emulators.qemu/165704 Tracing pull request: http://comments.gmane.org/gmane.comp.emulators.qemu/165311 Stefan
[Qemu-devel] [PATCH 8/9] iov_send_recv(): Handle zero bytes case even if OS does not
From: Peter Maydell
POSIX allows sendmsg() and recvmsg() to fail EMSGSIZE if passed a zero
msg.msg_iovlen (in particular the MacOS X implementation will do this).
Handle the case where iov_send_recv() is passed a zero byte count
explicitly, to avoid accidentally depending on the OS to treat zero
msg_iovlen as a no-op.
Signed-off-by: Peter Maydell
Acked-by: Michael Tokarev
Signed-off-by: Stefan Hajnoczi
---
iov.c |7 +++
1 file changed, 7 insertions(+)
diff --git a/iov.c b/iov.c
index b333061..60705c7 100644
--- a/iov.c
+++ b/iov.c
@@ -146,6 +146,13 @@ ssize_t iov_send_recv(int sockfd, struct iovec *iov,
unsigned iov_cnt,
{
ssize_t ret;
unsigned si, ei;/* start and end indexes */
+if (bytes == 0) {
+/* Catch the do-nothing case early, as otherwise we will pass an
+ * empty iovec to sendmsg/recvmsg(), and not all implementations
+ * accept this.
+ */
+return 0;
+}
/* Find the start position, skipping `offset' bytes:
* first, skip all full-sized vector elements, */
--
1.7.10.4
[Qemu-devel] [PATCH 1/9] Makefile: Avoid explicit list of directories in clean target
From: Peter Maydell
Avoid having an explicit list of directories in the 'clean'
target by using 'find' to remove all .o and .d files instead.
Signed-off-by: Peter Maydell
Reviewed-by: Eric Blake
Signed-off-by: Stefan Hajnoczi
---
Makefile |7 ++-
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/Makefile b/Makefile
index d736ea5..2964d5c 100644
--- a/Makefile
+++ b/Makefile
@@ -214,13 +214,10 @@ clean:
# avoid old build problems by removing potentially incorrect old files
rm -f config.mak op-i386.h opc-i386.h gen-op-i386.h op-arm.h opc-arm.h
gen-op-arm.h
rm -f qemu-options.def
- rm -f *.o *.d *.a *.lo $(TOOLS) $(HELPERS-y) qemu-ga TAGS cscope.*
*.pod *~ */*~
+ find . -name '*.[od]' -exec rm -f {} +
+ rm -f *.a *.lo $(TOOLS) $(HELPERS-y) qemu-ga TAGS cscope.* *.pod *~ */*~
rm -Rf .libs
- rm -f slirp/*.o slirp/*.d audio/*.o audio/*.d block/*.o block/*.d
net/*.o net/*.d fsdev/*.o fsdev/*.d ui/*.o ui/*.d qapi/*.o qapi/*.d qga/*.o
qga/*.d
- rm -f qom/*.o qom/*.d libuser/qom/*.o libuser/qom/*.d
- rm -f hw/usb/*.o hw/usb/*.d hw/*.o hw/*.d
rm -f qemu-img-cmds.h
- rm -f trace/*.o trace/*.d
rm -f trace-dtrace.dtrace trace-dtrace.dtrace-timestamp
@# May not be present in GENERATED_HEADERS
rm -f trace-dtrace.h trace-dtrace.h-timestamp
--
1.7.10.4
[Qemu-devel] [PATCH 2/9] ehci: fix assertion typo
From: Alejandro Martinez Ruiz Signed-off-by: Alejandro Martinez Ruiz Reviewed-by: Andreas Färber Signed-off-by: Stefan Hajnoczi --- hw/usb/hcd-ehci.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/usb/hcd-ehci.c b/hw/usb/hcd-ehci.c index b043e7c..104c21d 100644 --- a/hw/usb/hcd-ehci.c +++ b/hw/usb/hcd-ehci.c @@ -2010,7 +2010,7 @@ static void ehci_fill_queue(EHCIPacket *p) p->qtdaddr = qtdaddr; p->qtd = qtd; p->usb_status = ehci_execute(p, "queue"); -assert(p->usb_status = USB_RET_ASYNC); +assert(p->usb_status == USB_RET_ASYNC); p->async = EHCI_ASYNC_INFLIGHT; } } -- 1.7.10.4
[Qemu-devel] [PATCH 9/9] ivshmem, qdev-monitor: fix order of qerror parameters
From: Alberto Garcia
Now that the QERR_ macros no longer contain a json dictionary,
the order of some parameters needs to be fixed for them to appear
correctly.
Signed-off-by: Alberto Garcia
Reviewed-by: Luiz Capitulino
Signed-off-by: Stefan Hajnoczi
---
hw/ivshmem.c |3 ++-
hw/qdev-monitor.c |2 +-
2 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/hw/ivshmem.c b/hw/ivshmem.c
index 0c58161..b4d65a6 100644
--- a/hw/ivshmem.c
+++ b/hw/ivshmem.c
@@ -677,7 +677,8 @@ static int pci_ivshmem_init(PCIDevice *dev)
}
if (s->role_val == IVSHMEM_PEER) {
-error_set(&s->migration_blocker, QERR_DEVICE_FEATURE_BLOCKS_MIGRATION,
"ivshmem", "peer mode");
+error_set(&s->migration_blocker, QERR_DEVICE_FEATURE_BLOCKS_MIGRATION,
+ "peer mode", "ivshmem");
migrate_add_blocker(s->migration_blocker);
}
diff --git a/hw/qdev-monitor.c b/hw/qdev-monitor.c
index b22a37a..018b386 100644
--- a/hw/qdev-monitor.c
+++ b/hw/qdev-monitor.c
@@ -443,7 +443,7 @@ DeviceState *qdev_device_add(QemuOpts *opts)
bus = qbus_find_recursive(sysbus_get_default(), NULL, k->bus_type);
if (!bus) {
qerror_report(QERR_NO_BUS_FOR_DEVICE,
- driver, k->bus_type);
+ k->bus_type, driver);
return NULL;
}
}
--
1.7.10.4
[Qemu-devel] [PATCH 4/9] trace: Fix "Qemu" -> "QEMU"
From: Stefan Weil
Signed-off-by: Stefan Weil
Signed-off-by: Stefan Hajnoczi
---
scripts/simpletrace.py |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/scripts/simpletrace.py b/scripts/simpletrace.py
index 9b4419f..8bbcb42 100755
--- a/scripts/simpletrace.py
+++ b/scripts/simpletrace.py
@@ -71,7 +71,7 @@ def read_trace_file(edict, fobj):
log_version = header[2]
if log_version == 0:
-raise ValueError('Older log format, not supported with this Qemu
release!')
+raise ValueError('Older log format, not supported with this QEMU
release!')
while True:
rec = read_record(edict, fobj)
--
1.7.10.4
