[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [RFC PATCH 16/41] block: Add error parameter to blk_ins
From: |
Kevin Wolf |
Subject: |
Re: [Qemu-devel] [RFC PATCH 16/41] block: Add error parameter to blk_insert_bs() |
Date: |
Mon, 20 Feb 2017 12:22:01 +0100 |
User-agent: |
Mutt/1.5.21 (2010-09-15) |
Am 20.02.2017 um 12:04 hat Max Reitz geschrieben:
> On 13.02.2017 18:22, Kevin Wolf wrote:
> > Mow that blk_insert_bs() requests the BlockBackend permissions for the
> > node it attaches to, it can fail. Instead of aborting, pass the errors
> > to the callers.
>
> So it does qualify as a FIXME, but just for a single patch. Good. :-)
>
> > Signed-off-by: Kevin Wolf <address@hidden>
> > ---
> > block/backup.c | 5 ++++-
> > block/block-backend.c | 12 ++++++++----
> > block/commit.c | 38
> > ++++++++++++++++++++++++++++++--------
> > block/mirror.c | 15 ++++++++++++---
> > blockdev.c | 6 +++++-
> > blockjob.c | 7 ++++++-
> > hmp.c | 6 +++++-
> > hw/core/qdev-properties-system.c | 7 ++++++-
> > include/sysemu/block-backend.h | 2 +-
> > migration/block.c | 2 +-
> > nbd/server.c | 6 +++++-
> > tests/test-blockjob.c | 2 +-
> > 12 files changed, 84 insertions(+), 24 deletions(-)
>
> [...]
>
> > diff --git a/migration/block.c b/migration/block.c
> > index 6b7ffd4..d259936 100644
> > --- a/migration/block.c
> > +++ b/migration/block.c
> > @@ -446,7 +446,7 @@ static void init_blk_migration(QEMUFile *f)
> > BlockDriverState *bs = bmds_bs[i].bs;
> >
> > if (bmds) {
> > - blk_insert_bs(bmds->blk, bs);
> > + blk_insert_bs(bmds->blk, bs, &error_abort);
>
> I don't think it's obvious why this is correct. I assume it is because
> this was a legal configuration on the source, so it must be a legal
> configuration on the destination.
>
> But I'd certainly appreciate a comment to make that explicitly clear,
> especially considering that it isn't obvious that blk_insert_bs() can
> fail only because of op blockers and thus will always work if the
> configuration is known to be legal.
Actually, it's just because the requested permissions for bmds->blk are
still 0, BLK_PERM_ALL here. Once the FIXME is addressed (patch 37) and
the real permissions are requested, we get some error handling here.
Kevin
pgpzQWw1kRUgq.pgp
Description: PGP signature
- Re: [Qemu-devel] [RFC PATCH 14/41] block: Add permissions to BlockBackend, (continued)
- [Qemu-devel] [RFC PATCH 18/41] block: Allow error return in BlockDevOps.change_media_cb(), Kevin Wolf, 2017/02/13
- [Qemu-devel] [RFC PATCH 19/41] hw/block: Request permissions, Kevin Wolf, 2017/02/13
- [Qemu-devel] [RFC PATCH 21/41] blockjob: Add permissions to block_job_create(), Kevin Wolf, 2017/02/13
- [Qemu-devel] [RFC PATCH 22/41] block: Add BdrvChildRole.get_link_name(), Kevin Wolf, 2017/02/13
- [Qemu-devel] [RFC PATCH 20/41] hw/block: Introduce share-rw qdev property, Kevin Wolf, 2017/02/13