| Message ID | 1457612036-8953-1-git-send-email-berto@igalia.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 03/10/2016 08:13 PM, 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. If we use FIFO mode, we don't call quorum_report_bad() in quorum_aio_cb(). But it is OK to iniialize sacb->aiocb for it. > > Signed-off-by: Alberto Garcia <berto@igalia.com> > Reviewed-by: Max Reitz <mreitz@redhat.com> Reviewed-by: Wen Congyang <wency@cn.fujitsu.com> > --- > block/quorum.c | 12 +++++++----- > 1 file changed, 7 insertions(+), 5 deletions(-) > > diff --git a/block/quorum.c b/block/quorum.c > index b9ba028..e640688 100644 > --- a/block/quorum.c > +++ b/block/quorum.c > @@ -646,8 +646,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; > @@ -662,9 +663,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; > } >
On Fri 11 Mar 2016 02:31:31 AM CET, Wen Congyang wrote: > On 03/10/2016 08:13 PM, 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. > > If we use FIFO mode, we don't call quorum_report_bad() in > quorum_aio_cb(). But it is OK to iniialize sacb->aiocb for it. You're right. I still think it's a good idea to leave it initialized in case we change that in the future. And now that we're at it, shouldn't we call quorum_report_bad() in FIFO mode as well? Or is there any reason not to do it? Berto
On 03/11/2016 04:25 PM, Alberto Garcia wrote: > On Fri 11 Mar 2016 02:31:31 AM CET, Wen Congyang wrote: >> On 03/10/2016 08:13 PM, 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. >> >> If we use FIFO mode, we don't call quorum_report_bad() in >> quorum_aio_cb(). But it is OK to iniialize sacb->aiocb for it. > Hi betro > You're right. I still think it's a good idea to leave it initialized in > case we change that in the future. Yes. > > And now that we're at it, shouldn't we call quorum_report_bad() in FIFO > mode as well? Or is there any reason not to do it? IMO, no reason not to do it. Thanks -Xie > > Berto > > > >
On Mon 14 Mar 2016 02:57:31 AM CET, Changlong Xie wrote: >> And now that we're at it, shouldn't we call quorum_report_bad() in >> FIFO mode as well? Or is there any reason not to do it? > > IMO, no reason not to do it. I'll send a patch to fix that. Thanks, Berto
diff --git a/block/quorum.c b/block/quorum.c index b9ba028..e640688 100644 --- a/block/quorum.c +++ b/block/quorum.c @@ -646,8 +646,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; @@ -662,9 +663,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; }