diff mbox

[v13,00/10] Block replication for continuous checkpoints

Message ID 56A57867.8000204@cn.fujitsu.com
State New
Headers show

Commit Message

Wen Congyang Jan. 25, 2016, 1:20 a.m. UTC
On 01/22/2016 11:14 PM, Dr. David Alan Gilbert wrote:
> Hi,
>   I can trigger a segfault if I wire in the block replication together with
> a quorum instance; it only triggers with both of them present but,
> it looks like the problem is a disagreement about the number of quorum
> members;  I'm triggering this on the 'colo-v2.4-periodic-mode' branch
> that is posted in the colo-framework set that I think includes this set
> (from https://github.com/coloft/qemu.git).
> 
> To trigger:
> ./git/colo/jan-16/try/x86_64-softmmu/qemu-system-x86_64 -nographic -S
> 
> (qemu) drive_add 0 if=none,id=colo-disk0,file.filename=/home/localvms/bugzilla.raw,driver=raw,node-name=node0
> (qemu) drive_add 1 if=none,id=active-disk0,throttling.bps-total=70000000,driver=replication,mode=secondary,file.driver=qcow2,file.file.filename=/run/colo-active-disk.qcow2,file.backing.driver=qcow2,file.backing.file.filename=/run/colo-hidden-disk.qcow2,file.backing.backing=colo-disk0
> (qemu) drive_add 2 if=none,id=top-quorum,driver=quorum,read-pattern=fifo,vote-threshold=1,children.0=active-disk0
> (qemu) device_add virtio-blk-pci,drive=top-quorum,addr=9
> 
> *** Error in `/root/colo/jan-2016/./try/x86_64-softmmu/qemu-system-x86_64': free(): invalid pointer: 0x0000555555a8fdf0 ***
> ======= Backtrace: =========
> /lib64/libc.so.6(+0x7cfe1)[0x7ffff110ffe1]
> /lib64/libglib-2.0.so.0(g_free+0xf)[0x7ffff1ecc36f]
> /root/colo/jan-2016/./try/x86_64-softmmu/qemu-system-x86_64
> Program received signal SIGABRT, Aborted.
> 0x00007ffff10c85f7 in raise () from /lib64/libc.so.6
> (gdb) where
> #0  0x00007ffff10c85f7 in raise () from /lib64/libc.so.6
> #1  0x00007ffff10c9ce8 in abort () from /lib64/libc.so.6
> #2  0x00007ffff1108317 in __libc_message () from /lib64/libc.so.6
> #3  0x00007ffff110ffe1 in _int_free () from /lib64/libc.so.6
> #4  0x00007ffff1ecc36f in g_free () from /lib64/libglib-2.0.so.0
> #5  0x00005555559dfdd7 in qemu_iovec_destroy (qiov=0x555557815410) at /root/colo/jan-2016/qemu/util/iov.c:378
> #6  0x0000555555989cce in quorum_aio_finalize (acb=0x555557815350) at /root/colo/jan-2016/qemu/block/quorum.c:171
> 171	            qemu_iovec_destroy(&acb->qcrs[i].qiov);
> (gdb) list
> 166	
> 167	    if (acb->is_read) {
> 168	        /* on the quorum case acb->child_iter == s->num_children - 1 */
> 169	        for (i = 0; i <= acb->child_iter; i++) {
> 170	            qemu_vfree(acb->qcrs[i].buf);
> 171	            qemu_iovec_destroy(&acb->qcrs[i].qiov);
> 172	        }
> 173	    }
> 174	
> 175	    g_free(acb->qcrs);
> (gdb) p acb->child_iter
> $1 = 1
> (gdb) p i
> $3 = 1
> 
> #7  0x000055555598afca in quorum_aio_cb (opaque=<optimized out>, ret=-5)
>     at /root/colo/jan-2016/qemu/block/quorum.c:302
> #8  0x00005555559990ee in bdrv_co_complete (acb=0x555557815410) at /root/colo/jan-2016/qemu/block/io.c:2122
> .....
> 
> So I guess acb->child_iter is wrong, since we only have one child on that quorum?
> and we're trying to do a destroy on the second child.

Can you try the following patch:
From 3f2c5ec288cd9a36afb392b4bba24029f3e9345a Mon Sep 17 00:00:00 2001
From: Wen Congyang <wency@cn.fujitsu.com>
Date: Mon, 25 Jan 2016 09:18:09 +0800
Subject: [PATCH] quorum: fix segfault when read fails in fifo mode

Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
---
 block/quorum.c | 3 +++
 1 file changed, 3 insertions(+)

Comments

Dr. David Alan Gilbert Jan. 25, 2016, 11:56 a.m. UTC | #1
* Wen Congyang (wency@cn.fujitsu.com) wrote:
> On 01/22/2016 11:14 PM, Dr. David Alan Gilbert wrote:
> > Hi,
> >   I can trigger a segfault if I wire in the block replication together with
> > a quorum instance; it only triggers with both of them present but,
> > it looks like the problem is a disagreement about the number of quorum
> > members;  I'm triggering this on the 'colo-v2.4-periodic-mode' branch
> > that is posted in the colo-framework set that I think includes this set
> > (from https://github.com/coloft/qemu.git).
> > 
> > To trigger:
> > ./git/colo/jan-16/try/x86_64-softmmu/qemu-system-x86_64 -nographic -S
> > 
> > (qemu) drive_add 0 if=none,id=colo-disk0,file.filename=/home/localvms/bugzilla.raw,driver=raw,node-name=node0
> > (qemu) drive_add 1 if=none,id=active-disk0,throttling.bps-total=70000000,driver=replication,mode=secondary,file.driver=qcow2,file.file.filename=/run/colo-active-disk.qcow2,file.backing.driver=qcow2,file.backing.file.filename=/run/colo-hidden-disk.qcow2,file.backing.backing=colo-disk0
> > (qemu) drive_add 2 if=none,id=top-quorum,driver=quorum,read-pattern=fifo,vote-threshold=1,children.0=active-disk0
> > (qemu) device_add virtio-blk-pci,drive=top-quorum,addr=9
> > 
> > *** Error in `/root/colo/jan-2016/./try/x86_64-softmmu/qemu-system-x86_64': free(): invalid pointer: 0x0000555555a8fdf0 ***
> > ======= Backtrace: =========
> > /lib64/libc.so.6(+0x7cfe1)[0x7ffff110ffe1]
> > /lib64/libglib-2.0.so.0(g_free+0xf)[0x7ffff1ecc36f]
> > /root/colo/jan-2016/./try/x86_64-softmmu/qemu-system-x86_64
> > Program received signal SIGABRT, Aborted.
> > 0x00007ffff10c85f7 in raise () from /lib64/libc.so.6
> > (gdb) where
> > #0  0x00007ffff10c85f7 in raise () from /lib64/libc.so.6
> > #1  0x00007ffff10c9ce8 in abort () from /lib64/libc.so.6
> > #2  0x00007ffff1108317 in __libc_message () from /lib64/libc.so.6
> > #3  0x00007ffff110ffe1 in _int_free () from /lib64/libc.so.6
> > #4  0x00007ffff1ecc36f in g_free () from /lib64/libglib-2.0.so.0
> > #5  0x00005555559dfdd7 in qemu_iovec_destroy (qiov=0x555557815410) at /root/colo/jan-2016/qemu/util/iov.c:378
> > #6  0x0000555555989cce in quorum_aio_finalize (acb=0x555557815350) at /root/colo/jan-2016/qemu/block/quorum.c:171
> > 171	            qemu_iovec_destroy(&acb->qcrs[i].qiov);
> > (gdb) list
> > 166	
> > 167	    if (acb->is_read) {
> > 168	        /* on the quorum case acb->child_iter == s->num_children - 1 */
> > 169	        for (i = 0; i <= acb->child_iter; i++) {
> > 170	            qemu_vfree(acb->qcrs[i].buf);
> > 171	            qemu_iovec_destroy(&acb->qcrs[i].qiov);
> > 172	        }
> > 173	    }
> > 174	
> > 175	    g_free(acb->qcrs);
> > (gdb) p acb->child_iter
> > $1 = 1
> > (gdb) p i
> > $3 = 1
> > 
> > #7  0x000055555598afca in quorum_aio_cb (opaque=<optimized out>, ret=-5)
> >     at /root/colo/jan-2016/qemu/block/quorum.c:302
> > #8  0x00005555559990ee in bdrv_co_complete (acb=0x555557815410) at /root/colo/jan-2016/qemu/block/io.c:2122
> > .....
> > 
> > So I guess acb->child_iter is wrong, since we only have one child on that quorum?
> > and we're trying to do a destroy on the second child.
> 
> Can you try the following patch:
> From 3f2c5ec288cd9a36afb392b4bba24029f3e9345a Mon Sep 17 00:00:00 2001
> From: Wen Congyang <wency@cn.fujitsu.com>
> Date: Mon, 25 Jan 2016 09:18:09 +0800
> Subject: [PATCH] quorum: fix segfault when read fails in fifo mode
> 
> Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
> ---
>  block/quorum.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/block/quorum.c b/block/quorum.c
> index a5ae4b8..0965277 100644
> --- a/block/quorum.c
> +++ b/block/quorum.c
> @@ -295,6 +295,9 @@ static void quorum_aio_cb(void *opaque, int ret)
>              quorum_copy_qiov(acb->qiov, &acb->qcrs[acb->child_iter].qiov);
>          }
>          acb->vote_ret = ret;
> +        if (ret < 0) {
> +            acb->child_iter--;
> +        }
>          quorum_aio_finalize(acb);
>          return;
>      }

Yes, that seems to fix it; thanks.

Dave

> -- 
> 2.5.0
> 
> 
> 
> > 
> > Dave
> > --
> > Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
> > 
> > 
> > .
> > 
> 
> 
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
diff mbox

Patch

diff --git a/block/quorum.c b/block/quorum.c
index a5ae4b8..0965277 100644
--- a/block/quorum.c
+++ b/block/quorum.c
@@ -295,6 +295,9 @@  static void quorum_aio_cb(void *opaque, int ret)
             quorum_copy_qiov(acb->qiov, &acb->qcrs[acb->child_iter].qiov);
         }
         acb->vote_ret = ret;
+        if (ret < 0) {
+            acb->child_iter--;
+        }
         quorum_aio_finalize(acb);
         return;
     }