From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:52552) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZljIY-00004t-Bz for qemu-devel@nongnu.org; Mon, 12 Oct 2015 16:02:20 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZljIW-00010v-62 for qemu-devel@nongnu.org; Mon, 12 Oct 2015 16:02:18 -0400 From: Max Reitz Date: Mon, 12 Oct 2015 22:00:26 +0200 Message-Id: <1444680042-13207-24-git-send-email-mreitz@redhat.com> In-Reply-To: <1444680042-13207-1-git-send-email-mreitz@redhat.com> References: <1444680042-13207-1-git-send-email-mreitz@redhat.com> Subject: [Qemu-devel] [PATCH v6 23/39] block: Prepare for NULL BDS List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: qemu-block@nongnu.org Cc: Kevin Wolf , Alberto Garcia , Markus Armbruster , qemu-devel@nongnu.org, Max Reitz , John Snow , Stefan Hajnoczi 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 --- block.c | 5 ++ block/qapi.c | 4 +- blockdev.c | 201 ++++++++++++++++++++++++++++++++++------------------ hw/block/xen_disk.c | 4 +- migration/block.c | 5 ++ monitor.c | 4 ++ 6 files changed, 153 insertions(+), 70 deletions(-) diff --git a/block.c b/block.c index 48f7067..e5f00e4 100644 --- a/block.c +++ b/block.c @@ -2673,6 +2673,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 3b46f97..ec0f513 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 25959eb..35efe84 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; } } @@ -1040,6 +1042,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) { @@ -1119,7 +1125,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; @@ -1131,11 +1139,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; @@ -1309,16 +1320,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; @@ -1570,7 +1581,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; @@ -1584,12 +1594,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, @@ -1604,7 +1618,7 @@ static void drive_backup_prepare(BlkTransactionState *common, Error **errp) return; } - state->bs = bs; + state->bs = blk_bs(blk); state->job = state->bs->job; } @@ -1639,8 +1653,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); @@ -1651,18 +1664,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; @@ -1680,7 +1691,7 @@ static void blockdev_backup_prepare(BlkTransactionState *common, Error **errp) return; } - state->bs = bs; + state->bs = blk_bs(blk); state->job = state->bs->job; } @@ -1818,10 +1829,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)) { @@ -1839,7 +1850,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); @@ -1884,10 +1897,12 @@ 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, const char *format, const char *password, Error **errp) { + BlockDriverState *bs; Error *local_err = NULL; QDict *options = NULL; int ret; @@ -1897,11 +1912,12 @@ static void qmp_bdrv_open_encrypted(BlockDriverState *bs, const char *filename, qdict_put(options, "driver", qstring_from_str(format)); } - ret = bdrv_open(&bs, filename, NULL, options, bdrv_flags, &local_err); + ret = bdrv_open(pbs, filename, NULL, options, bdrv_flags, &local_err); if (ret < 0) { error_propagate(errp, local_err); return; } + bs = *pbs; bdrv_add_key(bs, password, errp); } @@ -1913,6 +1929,7 @@ void qmp_change_blockdev(const char *device, const char *filename, BlockDriverState *bs; AioContext *aio_context; int bdrv_flags; + bool new_bs; Error *err = NULL; blk = blk_by_name(device); @@ -1922,8 +1939,9 @@ 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); eject_device(blk, 0, &err); @@ -1932,10 +1950,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, format, NULL, errp); + qmp_bdrv_open_encrypted(&bs, filename, bdrv_flags, format, 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); @@ -1975,7 +2001,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; @@ -2010,12 +2044,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. */ @@ -2031,6 +2062,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); } @@ -2143,7 +2175,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" @@ -2151,16 +2182,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 @@ -2293,11 +2327,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; } @@ -2368,11 +2407,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; } @@ -2478,17 +2522,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; @@ -2585,7 +2629,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; @@ -2606,17 +2650,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); @@ -2693,15 +2747,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; @@ -2831,17 +2885,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; } @@ -2850,7 +2909,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; } @@ -2957,11 +3019,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 36d7398..1bbc111 100644 --- a/hw/block/xen_disk.c +++ b/hw/block/xen_disk.c @@ -931,9 +931,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 4f1ba2f..301a143 100644 --- a/monitor.c +++ b/monitor.c @@ -4145,6 +4145,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.6.1