qemu-block
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-block] [RFC PATCH 16/41] block: Add error parameter to blk_ins


From: Kevin Wolf
Subject: Re: [Qemu-block] [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

Attachment: pgp2csG0260HV.pgp
Description: PGP signature


reply via email to

[Prev in Thread] Current Thread [Next in Thread]