All of lore.kernel.org
 help / color / mirror / Atom feed
From: Eric Blake <eblake@redhat.com>
To: Michal Privoznik <mprivozn@redhat.com>, qemu-devel@nongnu.org
Cc: Chunyan Liu <cyliu@suse.com>, Stefan Hajnoczi <stefanha@redhat.com>
Subject: Re: [Qemu-devel] [PATCH for 2.1] qemu_opts_append: Play nicely with QemuOptsList's head
Date: Wed, 25 Jun 2014 07:46:32 -0600	[thread overview]
Message-ID: <53AAD2B8.6010706@redhat.com> (raw)
In-Reply-To: <9ca16cc7ed58cd133ea2c8d86c29707b54005e1d.1403685480.git.mprivozn@redhat.com>

[-- Attachment #1: Type: text/plain, Size: 1525 bytes --]

On 06/25/2014 02:38 AM, Michal Privoznik wrote:
> When running a libvirt test suite I've noticed the qemu-img is
> crashing occasionally. Tracing the problem down led me to the
> following valgrind output:

Thanks for tracking this! It has been reported in other threads, but
yours is the first patch.

> The problem is apparently in the qemu_opts_append(). Well, if it
> gets called twice or more. On the first call, when @dst is NULL
> some initialization is done during which @dst->head list gets
> initialized. The list is initialized in a way, so that the list
> tail points at the list head. However, the next time
> qemu_opts_append() is called for new options to be added,
> g_realloc() may move @dst at new address making the old list tail

s/at new/to a new/

> point at invalid address. If that's the case we must update the
> list pointers.
> 
> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
> ---
>  util/qemu-option.c | 11 ++++++++++-
>  1 file changed, 10 insertions(+), 1 deletion(-)

Reviewed-by: Eric Blake <eblake@redhat.com>


> +        need_head_update = true;
> +    } else {
> +        /* Moreover, even if dst is not NULL, the realloc may move it at a

s/at/to/

> +         * different address in which case we may get a stale tail pointer
> +         * in dst->head. */
> +        need_head_update = QTAILQ_EMPTY(&dst->head);
>      }
>  

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

  reply	other threads:[~2014-06-25 13:46 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-06-25  8:38 [Qemu-devel] [PATCH] qemu_opts_append: Play nicely with QemuOptsList's head Michal Privoznik
2014-06-25 13:46 ` Eric Blake [this message]
2014-06-26 13:56   ` [Qemu-devel] [PATCH for 2.1] " Kevin Wolf
2014-06-26  4:56 ` [Qemu-devel] [PATCH] " Chun Yan Liu
2014-06-26  6:33   ` Michal Privoznik

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=53AAD2B8.6010706@redhat.com \
    --to=eblake@redhat.com \
    --cc=cyliu@suse.com \
    --cc=mprivozn@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=stefanha@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.