| Message ID | 8ad83e37ec80ac89ee7a30426c4b0f2c02e033eb.1457539274.git.berto@igalia.com |
|---|---|
| State | New |
| Headers | show |
On 09.03.2016 17:11, Alberto Garcia wrote: > quorum_aio_cb() emits the QUORUM_REPORT_BAD event if there's > an I/O error in a Quorum child. However sacb->aiocb must be > correctly initialized for this to happen. read_quorum_children() and > read_fifo_child() are not doing this, which results in a QEMU crash. > > Signed-off-by: Alberto Garcia <berto@igalia.com> > --- > block/quorum.c | 12 +++++++----- > 1 file changed, 7 insertions(+), 5 deletions(-) Reviewed-by: Max Reitz <mreitz@redhat.com> Although I'm wondering whether we could have just used acb->common.bs instead of sacb->aiocb->bs in quorum_aio_cb(). I guess that sacb->aiocb is supposed to be equal to &acb->common. Max
On Wed 09 Mar 2016 05:54:55 PM CET, Max Reitz <mreitz@redhat.com> wrote: > On 09.03.2016 17:11, Alberto Garcia wrote: >> quorum_aio_cb() emits the QUORUM_REPORT_BAD event if there's >> an I/O error in a Quorum child. However sacb->aiocb must be >> correctly initialized for this to happen. read_quorum_children() and >> read_fifo_child() are not doing this, which results in a QEMU crash. >> >> Signed-off-by: Alberto Garcia <berto@igalia.com> >> --- >> block/quorum.c | 12 +++++++----- >> 1 file changed, 7 insertions(+), 5 deletions(-) > > Reviewed-by: Max Reitz <mreitz@redhat.com> > > Although I'm wondering whether we could have just used acb->common.bs > instead of sacb->aiocb->bs in quorum_aio_cb(). I guess that > sacb->aiocb is supposed to be equal to &acb->common. acb->common.bs is the Quorum BDS, sacb->aiocb->bs is the child BDS. Berto
On 10.03.2016 11:10, Alberto Garcia wrote: > On Wed 09 Mar 2016 05:54:55 PM CET, Max Reitz <mreitz@redhat.com> wrote: >> On 09.03.2016 17:11, Alberto Garcia wrote: >>> quorum_aio_cb() emits the QUORUM_REPORT_BAD event if there's >>> an I/O error in a Quorum child. However sacb->aiocb must be >>> correctly initialized for this to happen. read_quorum_children() and >>> read_fifo_child() are not doing this, which results in a QEMU crash. >>> >>> Signed-off-by: Alberto Garcia <berto@igalia.com> >>> --- >>> block/quorum.c | 12 +++++++----- >>> 1 file changed, 7 insertions(+), 5 deletions(-) >> >> Reviewed-by: Max Reitz <mreitz@redhat.com> >> >> Although I'm wondering whether we could have just used acb->common.bs >> instead of sacb->aiocb->bs in quorum_aio_cb(). I guess that >> sacb->aiocb is supposed to be equal to &acb->common. > > acb->common.bs is the Quorum BDS, sacb->aiocb->bs is the child BDS. You're right, thanks for explaining. Max
diff --git a/block/quorum.c b/block/quorum.c index 11cc60b..73ff309 100644 --- a/block/quorum.c +++ b/block/quorum.c @@ -648,8 +648,9 @@ static BlockAIOCB *read_quorum_children(QuorumAIOCB *acb) } for (i = 0; i < s->num_children; i++) { - bdrv_aio_readv(s->children[i]->bs, acb->sector_num, &acb->qcrs[i].qiov, - acb->nb_sectors, quorum_aio_cb, &acb->qcrs[i]); + acb->qcrs[i].aiocb = bdrv_aio_readv(s->children[i]->bs, acb->sector_num, + &acb->qcrs[i].qiov, acb->nb_sectors, + quorum_aio_cb, &acb->qcrs[i]); } return &acb->common; @@ -664,9 +665,10 @@ static BlockAIOCB *read_fifo_child(QuorumAIOCB *acb) qemu_iovec_init(&acb->qcrs[acb->child_iter].qiov, acb->qiov->niov); qemu_iovec_clone(&acb->qcrs[acb->child_iter].qiov, acb->qiov, acb->qcrs[acb->child_iter].buf); - bdrv_aio_readv(s->children[acb->child_iter]->bs, acb->sector_num, - &acb->qcrs[acb->child_iter].qiov, acb->nb_sectors, - quorum_aio_cb, &acb->qcrs[acb->child_iter]); + acb->qcrs[acb->child_iter].aiocb = + bdrv_aio_readv(s->children[acb->child_iter]->bs, acb->sector_num, + &acb->qcrs[acb->child_iter].qiov, acb->nb_sectors, + quorum_aio_cb, &acb->qcrs[acb->child_iter]); return &acb->common; }
quorum_aio_cb() emits the QUORUM_REPORT_BAD event if there's an I/O error in a Quorum child. However sacb->aiocb must be correctly initialized for this to happen. read_quorum_children() and read_fifo_child() are not doing this, which results in a QEMU crash. Signed-off-by: Alberto Garcia <berto@igalia.com> --- block/quorum.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-)