| Message ID | 1410439779-20402-1-git-send-email-stefanha@redhat.com |
|---|---|
| State | New |
| Headers | show |
On 09/11/2014 06:49 AM, Stefan Hajnoczi wrote: > Commit 3718d8ab65f68de2acccbe6a315907805f54e3cc ("block: Replace in_use > with operation blocker") broke the error path because it consumed > local_err instead of propagating it. > > The caller has no way to know that the function failed. This caused > virtio-blk to start "successfully" even though there was a fatal > dataplane error. > > --- > hw/block/dataplane/virtio-blk.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) Reviewed-by: Eric Blake <eblake@redhat.com>
Am 11.09.2014 um 14:49 hat Stefan Hajnoczi geschrieben: > Commit 3718d8ab65f68de2acccbe6a315907805f54e3cc ("block: Replace in_use > with operation blocker") broke the error path because it consumed > local_err instead of propagating it. > > The caller has no way to know that the function failed. This caused > virtio-blk to start "successfully" even though there was a fatal > dataplane error. > > Steps to reproduce: > > $ qemu-system-x86_64 -enable-kvm -object iothread,id=iothread0 \ > -drive if=none,id=drive0,file=a.img \ > (qemu) drive_mirror drive0 /tmp/foo.img > (qemu) device_add virtio-blk-pci,iothread=iothread0,drive=drive0 > > Expected result: > > Since the mirror block job is using drive0 it is not possible to start > virtio-blk data-plane. > > device_add fails and the PCI adapter is not added. > > Actual result: > > device_add completes and the PCI adapter is added. > > Cc: Fam Zheng <famz@redhat.com> > Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> Thanks, applied to the block branch. Kevin
diff --git a/hw/block/dataplane/virtio-blk.c b/hw/block/dataplane/virtio-blk.c index b55188c..5458f9d 100644 --- a/hw/block/dataplane/virtio-blk.c +++ b/hw/block/dataplane/virtio-blk.c @@ -164,8 +164,8 @@ void virtio_blk_data_plane_create(VirtIODevice *vdev, VirtIOBlkConf *blk, * block jobs that can conflict. */ if (bdrv_op_is_blocked(blk->conf.bs, BLOCK_OP_TYPE_DATAPLANE, &local_err)) { - error_report("cannot start dataplane thread: %s", - error_get_pretty(local_err)); + error_setg(errp, "cannot start dataplane thread: %s", + error_get_pretty(local_err)); error_free(local_err); return; }
Commit 3718d8ab65f68de2acccbe6a315907805f54e3cc ("block: Replace in_use with operation blocker") broke the error path because it consumed local_err instead of propagating it. The caller has no way to know that the function failed. This caused virtio-blk to start "successfully" even though there was a fatal dataplane error. Steps to reproduce: $ qemu-system-x86_64 -enable-kvm -object iothread,id=iothread0 \ -drive if=none,id=drive0,file=a.img \ (qemu) drive_mirror drive0 /tmp/foo.img (qemu) device_add virtio-blk-pci,iothread=iothread0,drive=drive0 Expected result: Since the mirror block job is using drive0 it is not possible to start virtio-blk data-plane. device_add fails and the PCI adapter is not added. Actual result: device_add completes and the PCI adapter is added. Cc: Fam Zheng <famz@redhat.com> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> --- hw/block/dataplane/virtio-blk.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)