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 <kw...@redhat.com>
> > ---
> > 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
pgpX6pgBSaq35.pgp
Description: PGP signature
