block: Add error parameter to blk_insert_bs()

Now 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.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Acked-by: Fam Zheng <famz@redhat.com>

5 files changed, 62 insertions, 19 deletions

diff --git a/block/backup.c b/block/backup.c
index 4b3c94c812..f38d1d030e 100644
--- a/block/backup.c
+++ b/block/backup.c
@@ -626,7 +626,10 @@ BlockJob *backup_job_create(const char *job_id, BlockDriverState *bs,
 
     /* FIXME Use real permissions */
     job->target = blk_new(0, BLK_PERM_ALL);
-    blk_insert_bs(job->target, target);
+    ret = blk_insert_bs(job->target, target, errp);
+    if (ret < 0) {
+        goto error;
+    }
 
     job->on_source_error = on_source_error;
     job->on_target_error = on_target_error;
diff --git a/block/block-backend.c b/block/block-backend.c
index 0319220a78..299948f96b 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -508,19 +508,22 @@ void blk_remove_bs(BlockBackend *blk)
 /*
  * Associates a new BlockDriverState with @blk.
  */
-void blk_insert_bs(BlockBackend *blk, BlockDriverState *bs)
+int blk_insert_bs(BlockBackend *blk, BlockDriverState *bs, Error **errp)
 {
-    bdrv_ref(bs);
-    /* FIXME Error handling */
     blk->root = bdrv_root_attach_child(bs, "root", &child_root,
-                                       blk->perm, blk->shared_perm, blk,
-                                       &error_abort);
+                                       blk->perm, blk->shared_perm, blk, errp);
+    if (blk->root == NULL) {
+        return -EPERM;
+    }
+    bdrv_ref(bs);
 
     notifier_list_notify(&blk->insert_bs_notifiers, blk);
     if (blk->public.throttle_state) {
         throttle_timers_attach_aio_context(
             &blk->public.throttle_timers, bdrv_get_aio_context(bs));
     }
+
+    return 0;
 }
 
 /*
diff --git a/block/commit.c b/block/commit.c
index 1897e982c5..2ad8138aac 100644
--- a/block/commit.c
+++ b/block/commit.c
@@ -220,6 +220,7 @@ void commit_start(const char *job_id, BlockDriverState *bs,
     BlockDriverState *iter;
     BlockDriverState *overlay_bs;
     Error *local_err = NULL;
+    int ret;
 
     assert(top != bs);
     if (top == base) {
@@ -256,8 +257,7 @@ void commit_start(const char *job_id, BlockDriverState *bs,
         bdrv_reopen_multiple(bdrv_get_aio_context(bs), reopen_queue, &local_err);
         if (local_err != NULL) {
             error_propagate(errp, local_err);
-            block_job_unref(&s->common);
-            return;
+            goto fail;
         }
     }
 
@@ -277,11 +277,17 @@ void commit_start(const char *job_id, BlockDriverState *bs,
 
     /* FIXME Use real permissions */
     s->base = blk_new(0, BLK_PERM_ALL);
-    blk_insert_bs(s->base, base);
+    ret = blk_insert_bs(s->base, base, errp);
+    if (ret < 0) {
+        goto fail;
+    }
 
     /* FIXME Use real permissions */
     s->top = blk_new(0, BLK_PERM_ALL);
-    blk_insert_bs(s->top, top);
+    ret = blk_insert_bs(s->top, top, errp);
+    if (ret < 0) {
+        goto fail;
+    }
 
     s->active = bs;
 
@@ -294,6 +300,16 @@ void commit_start(const char *job_id, BlockDriverState *bs,
 
     trace_commit_start(bs, base, top, s);
     block_job_start(&s->common);
+    return;
+
+fail:
+    if (s->base) {
+        blk_unref(s->base);
+    }
+    if (s->top) {
+        blk_unref(s->top);
+    }
+    block_job_unref(&s->common);
 }
 
 
@@ -332,11 +348,17 @@ int bdrv_commit(BlockDriverState *bs)
 
     /* FIXME Use real permissions */
     src = blk_new(0, BLK_PERM_ALL);
-    blk_insert_bs(src, bs);
-
-    /* FIXME Use real permissions */
     backing = blk_new(0, BLK_PERM_ALL);
-    blk_insert_bs(backing, bs->backing->bs);
+
+    ret = blk_insert_bs(src, bs, NULL);
+    if (ret < 0) {
+        goto ro_cleanup;
+    }
+
+    ret = blk_insert_bs(backing, bs->backing->bs, NULL);
+    if (ret < 0) {
+        goto ro_cleanup;
+    }
 
     length = blk_getlength(src);
     if (length < 0) {
diff --git a/block/mirror.c b/block/mirror.c
index 30398fb857..063925a1f0 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -525,9 +525,12 @@ static void mirror_exit(BlockJob *job, void *opaque)
         bdrv_replace_in_backing_chain(to_replace, target_bs);
         bdrv_drained_end(target_bs);
 
-        /* We just changed the BDS the job BB refers to */
+        /* We just changed the BDS the job BB refers to, so switch the BB back
+         * so the cleanup does the right thing. We don't need any permissions
+         * any more now. */
         blk_remove_bs(job->blk);
-        blk_insert_bs(job->blk, src);
+        blk_set_perm(job->blk, 0, BLK_PERM_ALL, &error_abort);
+        blk_insert_bs(job->blk, src, &error_abort);
     }
     if (s->to_replace) {
         bdrv_op_unblock_all(s->to_replace, s->replace_blocker);
@@ -995,6 +998,7 @@ static void mirror_start_job(const char *job_id, BlockDriverState *bs,
                              bool auto_complete)
 {
     MirrorBlockJob *s;
+    int ret;
 
     if (granularity == 0) {
         granularity = bdrv_get_default_bitmap_granularity(target);
@@ -1019,7 +1023,12 @@ static void mirror_start_job(const char *job_id, BlockDriverState *bs,
 
     /* FIXME Use real permissions */
     s->target = blk_new(0, BLK_PERM_ALL);
-    blk_insert_bs(s->target, target);
+    ret = blk_insert_bs(s->target, target, errp);
+    if (ret < 0) {
+        blk_unref(s->target);
+        block_job_unref(&s->common);
+        return;
+    }
 
     s->replaces = g_strdup(replaces);
     s->on_source_error = on_source_error;
diff --git a/block/qcow2.c b/block/qcow2.c
index 0356e69e4e..6f79df8517 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -3113,6 +3113,7 @@ static int qcow2_amend_options(BlockDriverState *bs, QemuOpts *opts,
     uint64_t cluster_size = s->cluster_size;
     bool encrypt;
     int refcount_bits = s->refcount_bits;
+    Error *local_err = NULL;
     int ret;
     QemuOptDesc *desc = opts->list->desc;
     Qcow2AmendHelperCBInfo helper_cb_info;
@@ -3263,10 +3264,15 @@ static int qcow2_amend_options(BlockDriverState *bs, QemuOpts *opts,
 
     if (new_size) {
         BlockBackend *blk = blk_new(BLK_PERM_RESIZE, BLK_PERM_ALL);
-        blk_insert_bs(blk, bs);
+        ret = blk_insert_bs(blk, bs, &local_err);
+        if (ret < 0) {
+            error_report_err(local_err);
+            blk_unref(blk);
+            return ret;
+        }
+
         ret = blk_truncate(blk, new_size);
         blk_unref(blk);
-
         if (ret < 0) {
             return ret;
         }