[Qemu-devel] [PATCH v4 21/38] block: Prepare for NULL BDS

[Max Reitz <mreitz@redhat.com>]

blk_bs() will not necessarily return a non-NULL value any more (unless
blk_is_available() is true or it can be assumed to otherwise, e.g.
because it is called immediately after a successful blk_new_with_bs() or
blk_new_open()).

Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 block.c             |   5 ++
 block/qapi.c        |   4 +-
 blockdev.c          | 203 ++++++++++++++++++++++++++++++++++------------------
 hw/block/xen_disk.c |   4 +-
 migration/block.c   |   5 ++
 monitor.c           |   4 ++
 6 files changed, 154 insertions(+), 71 deletions(-)

diff --git a/block.c b/block.c
index 5e05c72..f51005d 100644
--- a/block.c
+++ b/block.c
@@ -2771,6 +2771,11 @@ BlockDriverState *bdrv_lookup_bs(const char *device,
         blk = blk_by_name(device);
 
         if (blk) {
+            if (!blk_bs(blk)) {
+                error_setg(errp, "Device '%s' has no medium", device);
+                return NULL;
+            }
+
             return blk_bs(blk);
         }
     }
diff --git a/block/qapi.c b/block/qapi.c
index f295692..e936ba7 100644
--- a/block/qapi.c
+++ b/block/qapi.c
@@ -306,12 +306,12 @@ static void bdrv_query_info(BlockBackend *blk, BlockInfo **p_info,
         info->io_status = blk_iostatus(blk);
     }
 
-    if (!QLIST_EMPTY(&bs->dirty_bitmaps)) {
+    if (bs && !QLIST_EMPTY(&bs->dirty_bitmaps)) {
         info->has_dirty_bitmaps = true;
         info->dirty_bitmaps = bdrv_query_dirty_bitmaps(bs);
     }
 
-    if (bs->drv) {
+    if (bs && bs->drv) {
         info->has_inserted = true;
         info->inserted = bdrv_block_device_info(bs, errp);
         if (info->inserted == NULL) {
diff --git a/blockdev.c b/blockdev.c
index 7da8c45..7bb6ad0 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -124,14 +124,16 @@ void blockdev_mark_auto_del(BlockBackend *blk)
         return;
     }
 
-    aio_context = bdrv_get_aio_context(bs);
-    aio_context_acquire(aio_context);
+    if (bs) {
+        aio_context = bdrv_get_aio_context(bs);
+        aio_context_acquire(aio_context);
 
-    if (bs->job) {
-        block_job_cancel(bs->job);
-    }
+        if (bs->job) {
+            block_job_cancel(bs->job);
+        }
 
-    aio_context_release(aio_context);
+        aio_context_release(aio_context);
+    }
 
     dinfo->auto_del = 1;
 }
@@ -229,8 +231,8 @@ bool drive_check_orphaned(void)
             dinfo->type != IF_NONE) {
             fprintf(stderr, "Warning: Orphaned drive without device: "
                     "id=%s,file=%s,if=%s,bus=%d,unit=%d\n",
-                    blk_name(blk), blk_bs(blk)->filename, if_name[dinfo->type],
-                    dinfo->bus, dinfo->unit);
+                    blk_name(blk), blk_bs(blk) ? blk_bs(blk)->filename : "",
+                    if_name[dinfo->type], dinfo->bus, dinfo->unit);
             rs = true;
         }
     }
@@ -1038,6 +1040,10 @@ void hmp_commit(Monitor *mon, const QDict *qdict)
             monitor_printf(mon, "Device '%s' not found\n", device);
             return;
         }
+        if (!blk_is_available(blk)) {
+            monitor_printf(mon, "Device '%s' has no medium\n", device);
+            return;
+        }
         ret = bdrv_commit(blk_bs(blk));
     }
     if (ret < 0) {
@@ -1117,7 +1123,9 @@ SnapshotInfo *qmp_blockdev_snapshot_delete_internal_sync(const char *device,
                   "Device '%s' not found", device);
         return NULL;
     }
-    bs = blk_bs(blk);
+
+    aio_context = blk_get_aio_context(blk);
+    aio_context_acquire(aio_context);
 
     if (!has_id) {
         id = NULL;
@@ -1129,11 +1137,14 @@ SnapshotInfo *qmp_blockdev_snapshot_delete_internal_sync(const char *device,
 
     if (!id && !name) {
         error_setg(errp, "Name or id must be provided");
-        return NULL;
+        goto out_aio_context;
     }
 
-    aio_context = bdrv_get_aio_context(bs);
-    aio_context_acquire(aio_context);
+    if (!blk_is_available(blk)) {
+        error_setg(errp, "Device '%s' has no medium", device);
+        goto out_aio_context;
+    }
+    bs = blk_bs(blk);
 
     if (bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_INTERNAL_SNAPSHOT_DELETE, errp)) {
         goto out_aio_context;
@@ -1307,16 +1318,16 @@ static void internal_snapshot_prepare(BlkTransactionState *common,
                   "Device '%s' not found", device);
         return;
     }
-    bs = blk_bs(blk);
 
     /* AioContext is released in .clean() */
-    state->aio_context = bdrv_get_aio_context(bs);
+    state->aio_context = blk_get_aio_context(blk);
     aio_context_acquire(state->aio_context);
 
-    if (!bdrv_is_inserted(bs)) {
+    if (!blk_is_available(blk)) {
         error_setg(errp, QERR_DEVICE_HAS_NO_MEDIUM, device);
         return;
     }
+    bs = blk_bs(blk);
 
     if (bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_INTERNAL_SNAPSHOT, errp)) {
         return;
@@ -1574,7 +1585,6 @@ typedef struct DriveBackupState {
 static void drive_backup_prepare(BlkTransactionState *common, Error **errp)
 {
     DriveBackupState *state = DO_UPCAST(DriveBackupState, common, common);
-    BlockDriverState *bs;
     BlockBackend *blk;
     DriveBackup *backup;
     Error *local_err = NULL;
@@ -1588,12 +1598,16 @@ static void drive_backup_prepare(BlkTransactionState *common, Error **errp)
                   "Device '%s' not found", backup->device);
         return;
     }
-    bs = blk_bs(blk);
 
     /* AioContext is released in .clean() */
-    state->aio_context = bdrv_get_aio_context(bs);
+    state->aio_context = blk_get_aio_context(blk);
     aio_context_acquire(state->aio_context);
 
+    if (!blk_is_available(blk)) {
+        error_setg(errp, "Device '%s' has no medium", backup->device);
+        return;
+    }
+
     qmp_drive_backup(backup->device, backup->target,
                      backup->has_format, backup->format,
                      backup->sync,
@@ -1608,7 +1622,7 @@ static void drive_backup_prepare(BlkTransactionState *common, Error **errp)
         return;
     }
 
-    state->bs = bs;
+    state->bs = blk_bs(blk);
     state->job = state->bs->job;
 }
 
@@ -1643,8 +1657,7 @@ static void blockdev_backup_prepare(BlkTransactionState *common, Error **errp)
 {
     BlockdevBackupState *state = DO_UPCAST(BlockdevBackupState, common, common);
     BlockdevBackup *backup;
-    BlockDriverState *bs, *target;
-    BlockBackend *blk;
+    BlockBackend *blk, *target;
     Error *local_err = NULL;
 
     assert(common->action->kind == TRANSACTION_ACTION_KIND_BLOCKDEV_BACKUP);
@@ -1655,18 +1668,16 @@ static void blockdev_backup_prepare(BlkTransactionState *common, Error **errp)
         error_setg(errp, "Device '%s' not found", backup->device);
         return;
     }
-    bs = blk_bs(blk);
 
-    blk = blk_by_name(backup->target);
-    if (!blk) {
+    target = blk_by_name(backup->target);
+    if (!target) {
         error_setg(errp, "Device '%s' not found", backup->target);
         return;
     }
-    target = blk_bs(blk);
 
     /* AioContext is released in .clean() */
-    state->aio_context = bdrv_get_aio_context(bs);
-    if (state->aio_context != bdrv_get_aio_context(target)) {
+    state->aio_context = blk_get_aio_context(blk);
+    if (state->aio_context != blk_get_aio_context(target)) {
         state->aio_context = NULL;
         error_setg(errp, "Backup between two IO threads is not implemented");
         return;
@@ -1684,7 +1695,7 @@ static void blockdev_backup_prepare(BlkTransactionState *common, Error **errp)
         return;
     }
 
-    state->bs = bs;
+    state->bs = blk_bs(blk);
     state->job = state->bs->job;
 }
 
@@ -1822,10 +1833,10 @@ static void eject_device(BlockBackend *blk, int force, Error **errp)
     BlockDriverState *bs = blk_bs(blk);
     AioContext *aio_context;
 
-    aio_context = bdrv_get_aio_context(bs);
+    aio_context = blk_get_aio_context(blk);
     aio_context_acquire(aio_context);
 
-    if (bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_EJECT, errp)) {
+    if (bs && bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_EJECT, errp)) {
         goto out;
     }
     if (!blk_dev_has_removable_media(blk)) {
@@ -1843,7 +1854,9 @@ static void eject_device(BlockBackend *blk, int force, Error **errp)
         }
     }
 
-    bdrv_close(bs);
+    if (bs) {
+        bdrv_close(bs);
+    }
 
 out:
     aio_context_release(aio_context);
@@ -1888,18 +1901,21 @@ void qmp_block_passwd(bool has_device, const char *device,
 }
 
 /* Assumes AioContext is held */
-static void qmp_bdrv_open_encrypted(BlockDriverState *bs, const char *filename,
+static void qmp_bdrv_open_encrypted(BlockDriverState **pbs,
+                                    const char *filename,
                                     int bdrv_flags, BlockDriver *drv,
                                     const char *password, Error **errp)
 {
+    BlockDriverState *bs;
     Error *local_err = NULL;
     int ret;
 
-    ret = bdrv_open(&bs, filename, NULL, NULL, bdrv_flags, drv, &local_err);
+    ret = bdrv_open(pbs, filename, NULL, NULL, bdrv_flags, drv, &local_err);
     if (ret < 0) {
         error_propagate(errp, local_err);
         return;
     }
+    bs = *pbs;
 
     bdrv_add_key(bs, password, errp);
 }
@@ -1912,6 +1928,7 @@ void qmp_change_blockdev(const char *device, const char *filename,
     AioContext *aio_context;
     BlockDriver *drv = NULL;
     int bdrv_flags;
+    bool new_bs;
     Error *err = NULL;
 
     blk = blk_by_name(device);
@@ -1921,12 +1938,13 @@ void qmp_change_blockdev(const char *device, const char *filename,
         return;
     }
     bs = blk_bs(blk);
+    new_bs = !bs;
 
-    aio_context = bdrv_get_aio_context(bs);
+    aio_context = blk_get_aio_context(blk);
     aio_context_acquire(aio_context);
 
     if (format) {
-        drv = bdrv_find_whitelisted_format(format, bs->read_only);
+        drv = bdrv_find_whitelisted_format(format, blk_is_read_only(blk));
         if (!drv) {
             error_setg(errp, QERR_INVALID_BLOCK_FORMAT, format);
             goto out;
@@ -1939,10 +1957,18 @@ void qmp_change_blockdev(const char *device, const char *filename,
         goto out;
     }
 
-    bdrv_flags = bdrv_is_read_only(bs) ? 0 : BDRV_O_RDWR;
-    bdrv_flags |= bdrv_is_snapshot(bs) ? BDRV_O_SNAPSHOT : 0;
+    bdrv_flags = blk_is_read_only(blk) ? 0 : BDRV_O_RDWR;
+    bdrv_flags |= blk_get_root_state(blk)->open_flags & ~BDRV_O_RDWR;
 
-    qmp_bdrv_open_encrypted(bs, filename, bdrv_flags, drv, NULL, errp);
+    qmp_bdrv_open_encrypted(&bs, filename, bdrv_flags, drv, NULL, &err);
+    if (err) {
+        error_propagate(errp, err);
+    } else if (new_bs) {
+        blk_insert_bs(blk, bs);
+        /* Has been sent automatically by bdrv_open() if blk_bs(blk) was not
+         * NULL */
+        blk_dev_change_media_cb(blk, true);
+    }
 
 out:
     aio_context_release(aio_context);
@@ -1982,7 +2008,15 @@ void qmp_block_set_io_throttle(const char *device, int64_t bps, int64_t bps_rd,
                   "Device '%s' not found", device);
         return;
     }
+
+    aio_context = blk_get_aio_context(blk);
+    aio_context_acquire(aio_context);
+
     bs = blk_bs(blk);
+    if (!bs) {
+        error_setg(errp, "Device '%s' has no medium", device);
+        goto out;
+    }
 
     memset(&cfg, 0, sizeof(cfg));
     cfg.buckets[THROTTLE_BPS_TOTAL].avg = bps;
@@ -2017,12 +2051,9 @@ void qmp_block_set_io_throttle(const char *device, int64_t bps, int64_t bps_rd,
     }
 
     if (!check_throttle_config(&cfg, errp)) {
-        return;
+        goto out;
     }
 
-    aio_context = bdrv_get_aio_context(bs);
-    aio_context_acquire(aio_context);
-
     if (throttle_enabled(&cfg)) {
         /* Enable I/O limits if they're not enabled yet, otherwise
          * just update the throttling group. */
@@ -2038,6 +2069,7 @@ void qmp_block_set_io_throttle(const char *device, int64_t bps, int64_t bps_rd,
         bdrv_io_limits_disable(bs);
     }
 
+out:
     aio_context_release(aio_context);
 }
 
@@ -2150,7 +2182,6 @@ void hmp_drive_del(Monitor *mon, const QDict *qdict)
         error_report("Device '%s' not found", id);
         return;
     }
-    bs = blk_bs(blk);
 
     if (!blk_legacy_dinfo(blk)) {
         error_report("Deleting device added with blockdev-add"
@@ -2158,16 +2189,19 @@ void hmp_drive_del(Monitor *mon, const QDict *qdict)
         return;
     }
 
-    aio_context = bdrv_get_aio_context(bs);
+    aio_context = blk_get_aio_context(blk);
     aio_context_acquire(aio_context);
 
-    if (bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_DRIVE_DEL, &local_err)) {
+    bs = blk_bs(blk);
+    if (bs && bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_DRIVE_DEL, &local_err)) {
         error_report_err(local_err);
         aio_context_release(aio_context);
         return;
     }
 
-    bdrv_close(bs);
+    if (bs) {
+        bdrv_close(bs);
+    }
 
     /* if we have a device attached to this BlockDriverState
      * then we need to make the drive anonymous until the device
@@ -2300,11 +2334,16 @@ void qmp_block_stream(const char *device,
                   "Device '%s' not found", device);
         return;
     }
-    bs = blk_bs(blk);
 
-    aio_context = bdrv_get_aio_context(bs);
+    aio_context = blk_get_aio_context(blk);
     aio_context_acquire(aio_context);
 
+    if (!blk_is_available(blk)) {
+        error_setg(errp, "Device '%s' has no medium", device);
+        goto out;
+    }
+    bs = blk_bs(blk);
+
     if (bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_STREAM, errp)) {
         goto out;
     }
@@ -2375,11 +2414,16 @@ void qmp_block_commit(const char *device,
                   "Device '%s' not found", device);
         return;
     }
-    bs = blk_bs(blk);
 
-    aio_context = bdrv_get_aio_context(bs);
+    aio_context = blk_get_aio_context(blk);
     aio_context_acquire(aio_context);
 
+    if (!blk_is_available(blk)) {
+        error_setg(errp, "Device '%s' has no medium", device);
+        goto out;
+    }
+    bs = blk_bs(blk);
+
     if (bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_COMMIT_SOURCE, errp)) {
         goto out;
     }
@@ -2485,17 +2529,17 @@ void qmp_drive_backup(const char *device, const char *target,
                   "Device '%s' not found", device);
         return;
     }
-    bs = blk_bs(blk);
 
-    aio_context = bdrv_get_aio_context(bs);
+    aio_context = blk_get_aio_context(blk);
     aio_context_acquire(aio_context);
 
     /* Although backup_run has this check too, we need to use bs->drv below, so
      * do an early check redundantly. */
-    if (!bdrv_is_inserted(bs)) {
+    if (!blk_is_available(blk)) {
         error_setg(errp, QERR_DEVICE_HAS_NO_MEDIUM, device);
         goto out;
     }
+    bs = blk_bs(blk);
 
     if (!has_format) {
         format = mode == NEW_IMAGE_MODE_EXISTING ? NULL : bs->drv->format_name;
@@ -2594,7 +2638,7 @@ void qmp_blockdev_backup(const char *device, const char *target,
                          BlockdevOnError on_target_error,
                          Error **errp)
 {
-    BlockBackend *blk;
+    BlockBackend *blk, *target_blk;
     BlockDriverState *bs;
     BlockDriverState *target_bs;
     Error *local_err = NULL;
@@ -2615,17 +2659,27 @@ void qmp_blockdev_backup(const char *device, const char *target,
         error_setg(errp, "Device '%s' not found", device);
         return;
     }
-    bs = blk_bs(blk);
 
-    aio_context = bdrv_get_aio_context(bs);
+    aio_context = blk_get_aio_context(blk);
     aio_context_acquire(aio_context);
 
-    blk = blk_by_name(target);
-    if (!blk) {
+    if (!blk_is_available(blk)) {
+        error_setg(errp, "Device '%s' has no medium", device);
+        goto out;
+    }
+    bs = blk_bs(blk);
+
+    target_blk = blk_by_name(target);
+    if (!target_blk) {
         error_setg(errp, "Device '%s' not found", target);
         goto out;
     }
-    target_bs = blk_bs(blk);
+
+    if (!blk_is_available(target_blk)) {
+        error_setg(errp, "Device '%s' has no medium", target);
+        goto out;
+    }
+    target_bs = blk_bs(target_blk);
 
     bdrv_ref(target_bs);
     bdrv_set_aio_context(target_bs, aio_context);
@@ -2703,15 +2757,15 @@ void qmp_drive_mirror(const char *device, const char *target,
                   "Device '%s' not found", device);
         return;
     }
-    bs = blk_bs(blk);
 
-    aio_context = bdrv_get_aio_context(bs);
+    aio_context = blk_get_aio_context(blk);
     aio_context_acquire(aio_context);
 
-    if (!bdrv_is_inserted(bs)) {
+    if (!blk_is_available(blk)) {
         error_setg(errp, QERR_DEVICE_HAS_NO_MEDIUM, device);
         goto out;
     }
+    bs = blk_bs(blk);
 
     if (!has_format) {
         format = mode == NEW_IMAGE_MODE_EXISTING ? NULL : bs->drv->format_name;
@@ -2845,17 +2899,22 @@ static BlockJob *find_block_job(const char *device, AioContext **aio_context,
     BlockBackend *blk;
     BlockDriverState *bs;
 
+    *aio_context = NULL;
+
     blk = blk_by_name(device);
     if (!blk) {
         goto notfound;
     }
-    bs = blk_bs(blk);
 
-    *aio_context = bdrv_get_aio_context(bs);
+    *aio_context = blk_get_aio_context(blk);
     aio_context_acquire(*aio_context);
 
+    if (!blk_is_available(blk)) {
+        goto notfound;
+    }
+    bs = blk_bs(blk);
+
     if (!bs->job) {
-        aio_context_release(*aio_context);
         goto notfound;
     }
 
@@ -2864,7 +2923,10 @@ static BlockJob *find_block_job(const char *device, AioContext **aio_context,
 notfound:
     error_set(errp, ERROR_CLASS_DEVICE_NOT_ACTIVE,
               "No active block job on device '%s'", device);
-    *aio_context = NULL;
+    if (*aio_context) {
+        aio_context_release(*aio_context);
+        *aio_context = NULL;
+    }
     return NULL;
 }
 
@@ -2971,11 +3033,16 @@ void qmp_change_backing_file(const char *device,
                   "Device '%s' not found", device);
         return;
     }
-    bs = blk_bs(blk);
 
-    aio_context = bdrv_get_aio_context(bs);
+    aio_context = blk_get_aio_context(blk);
     aio_context_acquire(aio_context);
 
+    if (!blk_is_available(blk)) {
+        error_setg(errp, "Device '%s' has no medium", device);
+        goto out;
+    }
+    bs = blk_bs(blk);
+
     image_bs = bdrv_lookup_bs(NULL, image_node_name, &local_err);
     if (local_err) {
         error_propagate(errp, local_err);
diff --git a/hw/block/xen_disk.c b/hw/block/xen_disk.c
index 267d8a8..2f59b5b 100644
--- a/hw/block/xen_disk.c
+++ b/hw/block/xen_disk.c
@@ -932,9 +932,11 @@ static int blk_connect(struct XenDevice *xendev)
     blk_attach_dev_nofail(blkdev->blk, blkdev);
     blkdev->file_size = blk_getlength(blkdev->blk);
     if (blkdev->file_size < 0) {
+        BlockDriverState *bs = blk_bs(blkdev->blk);
+        const char *drv_name = bs ? bdrv_get_format_name(bs) : NULL;
         xen_be_printf(&blkdev->xendev, 1, "blk_getlength: %d (%s) | drv %s\n",
                       (int)blkdev->file_size, strerror(-blkdev->file_size),
-                      bdrv_get_format_name(blk_bs(blkdev->blk)) ?: "-");
+                      drv_name ?: "-");
         blkdev->file_size = 0;
     }
 
diff --git a/migration/block.c b/migration/block.c
index ed865ed..f7bb1e0 100644
--- a/migration/block.c
+++ b/migration/block.c
@@ -808,6 +808,11 @@ static int block_load(QEMUFile *f, void *opaque, int version_id)
                 return -EINVAL;
             }
             bs = blk_bs(blk);
+            if (!bs) {
+                fprintf(stderr, "Block device %s has no medium\n",
+                        device_name);
+                return -EINVAL;
+            }
 
             if (bs != bs_prev) {
                 bs_prev = bs;
diff --git a/monitor.c b/monitor.c
index aeea2b5..e18372d 100644
--- a/monitor.c
+++ b/monitor.c
@@ -5320,6 +5320,10 @@ int monitor_read_block_device_key(Monitor *mon, const char *device,
         monitor_printf(mon, "Device not found %s\n", device);
         return -1;
     }
+    if (!blk_bs(blk)) {
+        monitor_printf(mon, "Device '%s' has no medium\n", device);
+        return -1;
+    }
 
     bdrv_add_key(blk_bs(blk), NULL, &err);
     if (err) {
-- 
2.4.6