Re: [Qemu-devel] [RFC PATCH 16/41] block: Add error parameter to blk_insert_bs()

[Max Reitz]

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.

Max

>  
>              alloc_aio_bitmap(bmds);
>              error_setg(&bmds->blocker, "block device is in use by 
> migration");

signature.asc
Description: OpenPGP digital signature