From 3fc4b10af09b75a1cb811b61abc9d8c90771dfb2 Mon Sep 17 00:00:00 2001 From: Fam Zheng Date: Tue, 8 Oct 2013 17:29:38 +0800 Subject: [PATCH 01/61] blockjob: rename BlockJobType to BlockJobDriver We will use BlockJobType as the enum type name of block jobs in QAPI, rename current BlockJobType to BlockJobDriver, which will eventually become a set of operations, similar to block drivers. Signed-off-by: Fam Zheng Reviewed-by: Eric Blake Signed-off-by: Kevin Wolf --- block/backup.c | 4 ++-- block/commit.c | 4 ++-- block/mirror.c | 4 ++-- block/stream.c | 4 ++-- blockjob.c | 22 +++++++++++----------- include/block/blockjob.h | 12 ++++++------ 6 files changed, 25 insertions(+), 25 deletions(-) diff --git a/block/backup.c b/block/backup.c index 04c4b5c263..d374472da8 100644 --- a/block/backup.c +++ b/block/backup.c @@ -202,7 +202,7 @@ static void backup_iostatus_reset(BlockJob *job) bdrv_iostatus_reset(s->target); } -static const BlockJobType backup_job_type = { +static const BlockJobDriver backup_job_driver = { .instance_size = sizeof(BackupBlockJob), .job_type = "backup", .set_speed = backup_set_speed, @@ -370,7 +370,7 @@ void backup_start(BlockDriverState *bs, BlockDriverState *target, return; } - BackupBlockJob *job = block_job_create(&backup_job_type, bs, speed, + BackupBlockJob *job = block_job_create(&backup_job_driver, bs, speed, cb, opaque, errp); if (!job) { return; diff --git a/block/commit.c b/block/commit.c index ac4b7ccbc9..5146138c67 100644 --- a/block/commit.c +++ b/block/commit.c @@ -173,7 +173,7 @@ static void commit_set_speed(BlockJob *job, int64_t speed, Error **errp) ratelimit_set_speed(&s->limit, speed / BDRV_SECTOR_SIZE, SLICE_TIME); } -static const BlockJobType commit_job_type = { +static const BlockJobDriver commit_job_driver = { .instance_size = sizeof(CommitBlockJob), .job_type = "commit", .set_speed = commit_set_speed, @@ -238,7 +238,7 @@ void commit_start(BlockDriverState *bs, BlockDriverState *base, } - s = block_job_create(&commit_job_type, bs, speed, cb, opaque, errp); + s = block_job_create(&commit_job_driver, bs, speed, cb, opaque, errp); if (!s) { return; } diff --git a/block/mirror.c b/block/mirror.c index 6e7a274e43..991cc243bc 100644 --- a/block/mirror.c +++ b/block/mirror.c @@ -525,7 +525,7 @@ static void mirror_complete(BlockJob *job, Error **errp) block_job_resume(job); } -static const BlockJobType mirror_job_type = { +static const BlockJobDriver mirror_job_driver = { .instance_size = sizeof(MirrorBlockJob), .job_type = "mirror", .set_speed = mirror_set_speed, @@ -563,7 +563,7 @@ void mirror_start(BlockDriverState *bs, BlockDriverState *target, return; } - s = block_job_create(&mirror_job_type, bs, speed, cb, opaque, errp); + s = block_job_create(&mirror_job_driver, bs, speed, cb, opaque, errp); if (!s) { return; } diff --git a/block/stream.c b/block/stream.c index 45837f4476..7f412bda59 100644 --- a/block/stream.c +++ b/block/stream.c @@ -203,7 +203,7 @@ static void stream_set_speed(BlockJob *job, int64_t speed, Error **errp) ratelimit_set_speed(&s->limit, speed / BDRV_SECTOR_SIZE, SLICE_TIME); } -static const BlockJobType stream_job_type = { +static const BlockJobDriver stream_job_driver = { .instance_size = sizeof(StreamBlockJob), .job_type = "stream", .set_speed = stream_set_speed, @@ -224,7 +224,7 @@ void stream_start(BlockDriverState *bs, BlockDriverState *base, return; } - s = block_job_create(&stream_job_type, bs, speed, cb, opaque, errp); + s = block_job_create(&stream_job_driver, bs, speed, cb, opaque, errp); if (!s) { return; } diff --git a/blockjob.c b/blockjob.c index e7d49b7169..6814e699fc 100644 --- a/blockjob.c +++ b/blockjob.c @@ -35,7 +35,7 @@ #include "qmp-commands.h" #include "qemu/timer.h" -void *block_job_create(const BlockJobType *job_type, BlockDriverState *bs, +void *block_job_create(const BlockJobDriver *driver, BlockDriverState *bs, int64_t speed, BlockDriverCompletionFunc *cb, void *opaque, Error **errp) { @@ -48,8 +48,8 @@ void *block_job_create(const BlockJobType *job_type, BlockDriverState *bs, bdrv_ref(bs); bdrv_set_in_use(bs, 1); - job = g_malloc0(job_type->instance_size); - job->job_type = job_type; + job = g_malloc0(driver->instance_size); + job->driver = driver; job->bs = bs; job->cb = cb; job->opaque = opaque; @@ -87,11 +87,11 @@ void block_job_set_speed(BlockJob *job, int64_t speed, Error **errp) { Error *local_err = NULL; - if (!job->job_type->set_speed) { + if (!job->driver->set_speed) { error_set(errp, QERR_NOT_SUPPORTED); return; } - job->job_type->set_speed(job, speed, &local_err); + job->driver->set_speed(job, speed, &local_err); if (error_is_set(&local_err)) { error_propagate(errp, local_err); return; @@ -102,12 +102,12 @@ void block_job_set_speed(BlockJob *job, int64_t speed, Error **errp) void block_job_complete(BlockJob *job, Error **errp) { - if (job->paused || job->cancelled || !job->job_type->complete) { + if (job->paused || job->cancelled || !job->driver->complete) { error_set(errp, QERR_BLOCK_JOB_NOT_READY, job->bs->device_name); return; } - job->job_type->complete(job, errp); + job->driver->complete(job, errp); } void block_job_pause(BlockJob *job) @@ -143,8 +143,8 @@ bool block_job_is_cancelled(BlockJob *job) void block_job_iostatus_reset(BlockJob *job) { job->iostatus = BLOCK_DEVICE_IO_STATUS_OK; - if (job->job_type->iostatus_reset) { - job->job_type->iostatus_reset(job); + if (job->driver->iostatus_reset) { + job->driver->iostatus_reset(job); } } @@ -209,7 +209,7 @@ void block_job_sleep_ns(BlockJob *job, QEMUClockType type, int64_t ns) BlockJobInfo *block_job_query(BlockJob *job) { BlockJobInfo *info = g_new0(BlockJobInfo, 1); - info->type = g_strdup(job->job_type->job_type); + info->type = g_strdup(job->driver->job_type); info->device = g_strdup(bdrv_get_device_name(job->bs)); info->len = job->len; info->busy = job->busy; @@ -236,7 +236,7 @@ QObject *qobject_from_block_job(BlockJob *job) "'len': %" PRId64 "," "'offset': %" PRId64 "," "'speed': %" PRId64 " }", - job->job_type->job_type, + job->driver->job_type, bdrv_get_device_name(job->bs), job->len, job->offset, diff --git a/include/block/blockjob.h b/include/block/blockjob.h index d530409ff5..99359b5502 100644 --- a/include/block/blockjob.h +++ b/include/block/blockjob.h @@ -28,11 +28,11 @@ #include "block/block.h" /** - * BlockJobType: + * BlockJobDriver: * - * A class type for block job objects. + * A class type for block job driver. */ -typedef struct BlockJobType { +typedef struct BlockJobDriver { /** Derived BlockJob struct size */ size_t instance_size; @@ -50,7 +50,7 @@ typedef struct BlockJobType { * manually. */ void (*complete)(BlockJob *job, Error **errp); -} BlockJobType; +} BlockJobDriver; /** * BlockJob: @@ -59,7 +59,7 @@ typedef struct BlockJobType { */ struct BlockJob { /** The job type, including the job vtable. */ - const BlockJobType *job_type; + const BlockJobDriver *driver; /** The block device on which the job is operating. */ BlockDriverState *bs; @@ -128,7 +128,7 @@ struct BlockJob { * This function is not part of the public job interface; it should be * called from a wrapper that is specific to the job type. */ -void *block_job_create(const BlockJobType *job_type, BlockDriverState *bs, +void *block_job_create(const BlockJobDriver *driver, BlockDriverState *bs, int64_t speed, BlockDriverCompletionFunc *cb, void *opaque, Error **errp); From 2cb5b22286a7546226d9e9363aaee543fcba6b61 Mon Sep 17 00:00:00 2001 From: Fam Zheng Date: Tue, 8 Oct 2013 17:29:39 +0800 Subject: [PATCH 02/61] qapi: Introduce enum BlockJobType This will replace the open coded block job type string for mirror, commit and backup. Signed-off-by: Fam Zheng Reviewed-by: Eric Blake Signed-off-by: Kevin Wolf --- qapi-schema.json | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/qapi-schema.json b/qapi-schema.json index 145eca8855..381ffbf932 100644 --- a/qapi-schema.json +++ b/qapi-schema.json @@ -1365,6 +1365,24 @@ { 'enum': 'MirrorSyncMode', 'data': ['top', 'full', 'none'] } +## +# @BlockJobType: +# +# Type of a block job. +# +# @commit: block commit job type, see "block-commit" +# +# @stream: block stream job type, see "block-stream" +# +# @mirror: drive mirror job type, see "drive-mirror" +# +# @backup: drive backup job type, see "drive-backup" +# +# Since: 1.7 +## +{ 'enum': 'BlockJobType', + 'data': ['commit', 'stream', 'mirror', 'backup'] } + ## # @BlockJobInfo: # From 79e14bf7782d861d3d773a67680de07a8f354f4e Mon Sep 17 00:00:00 2001 From: Fam Zheng Date: Tue, 8 Oct 2013 17:29:40 +0800 Subject: [PATCH 03/61] qapi: make use of new BlockJobType Switch the string to enum type BlockJobType in BlockJobDriver. Signed-off-by: Fam Zheng Reviewed-by: Eric Blake Signed-off-by: Kevin Wolf --- block/backup.c | 2 +- block/commit.c | 2 +- block/mirror.c | 2 +- block/stream.c | 2 +- blockjob.c | 4 ++-- include/block/blockjob.h | 2 +- 6 files changed, 7 insertions(+), 7 deletions(-) diff --git a/block/backup.c b/block/backup.c index d374472da8..cad14c90b2 100644 --- a/block/backup.c +++ b/block/backup.c @@ -204,7 +204,7 @@ static void backup_iostatus_reset(BlockJob *job) static const BlockJobDriver backup_job_driver = { .instance_size = sizeof(BackupBlockJob), - .job_type = "backup", + .job_type = BLOCK_JOB_TYPE_BACKUP, .set_speed = backup_set_speed, .iostatus_reset = backup_iostatus_reset, }; diff --git a/block/commit.c b/block/commit.c index 5146138c67..d4090cbf7d 100644 --- a/block/commit.c +++ b/block/commit.c @@ -175,7 +175,7 @@ static void commit_set_speed(BlockJob *job, int64_t speed, Error **errp) static const BlockJobDriver commit_job_driver = { .instance_size = sizeof(CommitBlockJob), - .job_type = "commit", + .job_type = BLOCK_JOB_TYPE_COMMIT, .set_speed = commit_set_speed, }; diff --git a/block/mirror.c b/block/mirror.c index 991cc243bc..7b95acf88c 100644 --- a/block/mirror.c +++ b/block/mirror.c @@ -527,7 +527,7 @@ static void mirror_complete(BlockJob *job, Error **errp) static const BlockJobDriver mirror_job_driver = { .instance_size = sizeof(MirrorBlockJob), - .job_type = "mirror", + .job_type = BLOCK_JOB_TYPE_MIRROR, .set_speed = mirror_set_speed, .iostatus_reset= mirror_iostatus_reset, .complete = mirror_complete, diff --git a/block/stream.c b/block/stream.c index 7f412bda59..694fd42e41 100644 --- a/block/stream.c +++ b/block/stream.c @@ -205,7 +205,7 @@ static void stream_set_speed(BlockJob *job, int64_t speed, Error **errp) static const BlockJobDriver stream_job_driver = { .instance_size = sizeof(StreamBlockJob), - .job_type = "stream", + .job_type = BLOCK_JOB_TYPE_STREAM, .set_speed = stream_set_speed, }; diff --git a/blockjob.c b/blockjob.c index 6814e699fc..9e5fd5c162 100644 --- a/blockjob.c +++ b/blockjob.c @@ -209,7 +209,7 @@ void block_job_sleep_ns(BlockJob *job, QEMUClockType type, int64_t ns) BlockJobInfo *block_job_query(BlockJob *job) { BlockJobInfo *info = g_new0(BlockJobInfo, 1); - info->type = g_strdup(job->driver->job_type); + info->type = g_strdup(BlockJobType_lookup[job->driver->job_type]); info->device = g_strdup(bdrv_get_device_name(job->bs)); info->len = job->len; info->busy = job->busy; @@ -236,7 +236,7 @@ QObject *qobject_from_block_job(BlockJob *job) "'len': %" PRId64 "," "'offset': %" PRId64 "," "'speed': %" PRId64 " }", - job->driver->job_type, + BlockJobType_lookup[job->driver->job_type], bdrv_get_device_name(job->bs), job->len, job->offset, diff --git a/include/block/blockjob.h b/include/block/blockjob.h index 99359b5502..d76de62a46 100644 --- a/include/block/blockjob.h +++ b/include/block/blockjob.h @@ -37,7 +37,7 @@ typedef struct BlockJobDriver { size_t instance_size; /** String describing the operation, part of query-block-jobs QMP API */ - const char *job_type; + BlockJobType job_type; /** Optional callback for job types that support setting a speed limit */ void (*set_speed)(BlockJob *job, int64_t speed, Error **errp); From f2bb8a8a47597634b74c161c44b9980c7f4e50ac Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Wed, 9 Oct 2013 10:46:15 +0200 Subject: [PATCH 04/61] qapi: Add ImageInfoSpecific type Add a new type ImageInfoSpecific as a union for image format specific information in ImageInfo. Signed-off-by: Max Reitz Reviewed-by: Eric Blake Signed-off-by: Kevin Wolf --- qapi-schema.json | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/qapi-schema.json b/qapi-schema.json index 381ffbf932..246789b378 100644 --- a/qapi-schema.json +++ b/qapi-schema.json @@ -209,6 +209,18 @@ 'date-sec': 'int', 'date-nsec': 'int', 'vm-clock-sec': 'int', 'vm-clock-nsec': 'int' } } +## +# @ImageInfoSpecific: +# +# A discriminated record of image format specific information structures. +# +# Since: 1.7 +## + +{ 'union': 'ImageInfoSpecific', + 'data': { + } } + ## # @ImageInfo: # @@ -238,6 +250,9 @@ # # @backing-image: #optional info of the backing image (since 1.6) # +# @format-specific: #optional structure supplying additional format-specific +# information (since 1.7) +# # Since: 1.3 # ## @@ -248,7 +263,8 @@ '*cluster-size': 'int', '*encrypted': 'bool', '*backing-filename': 'str', '*full-backing-filename': 'str', '*backing-filename-format': 'str', '*snapshots': ['SnapshotInfo'], - '*backing-image': 'ImageInfo' } } + '*backing-image': 'ImageInfo', + '*format-specific': 'ImageInfoSpecific' } } ## # @ImageCheck: From eae041fe6f4314962e873615134eefb32cf8ba61 Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Wed, 9 Oct 2013 10:46:16 +0200 Subject: [PATCH 05/61] block: Add bdrv_get_specific_info Add a function for retrieving an ImageInfoSpecific object from a block driver. Signed-off-by: Max Reitz Reviewed-by: Eric Blake Signed-off-by: Kevin Wolf --- block.c | 9 +++++++++ block/qapi.c | 3 +++ include/block/block.h | 1 + include/block/block_int.h | 1 + 4 files changed, 14 insertions(+) diff --git a/block.c b/block.c index d7ca37e6ad..43acaadbce 100644 --- a/block.c +++ b/block.c @@ -3322,6 +3322,15 @@ int bdrv_get_info(BlockDriverState *bs, BlockDriverInfo *bdi) return drv->bdrv_get_info(bs, bdi); } +ImageInfoSpecific *bdrv_get_specific_info(BlockDriverState *bs) +{ + BlockDriver *drv = bs->drv; + if (drv && drv->bdrv_get_specific_info) { + return drv->bdrv_get_specific_info(bs); + } + return NULL; +} + int bdrv_save_vmstate(BlockDriverState *bs, const uint8_t *buf, int64_t pos, int size) { diff --git a/block/qapi.c b/block/qapi.c index 782051c65d..ab1dd24b76 100644 --- a/block/qapi.c +++ b/block/qapi.c @@ -134,6 +134,9 @@ void bdrv_query_image_info(BlockDriverState *bs, info->dirty_flag = bdi.is_dirty; info->has_dirty_flag = true; } + info->format_specific = bdrv_get_specific_info(bs); + info->has_format_specific = info->format_specific != NULL; + backing_filename = bs->backing_file; if (backing_filename[0] != '\0') { info->backing_filename = g_strdup(backing_filename); diff --git a/include/block/block.h b/include/block/block.h index f808550959..e26512472d 100644 --- a/include/block/block.h +++ b/include/block/block.h @@ -335,6 +335,7 @@ int bdrv_get_flags(BlockDriverState *bs); int bdrv_write_compressed(BlockDriverState *bs, int64_t sector_num, const uint8_t *buf, int nb_sectors); int bdrv_get_info(BlockDriverState *bs, BlockDriverInfo *bdi); +ImageInfoSpecific *bdrv_get_specific_info(BlockDriverState *bs); void bdrv_round_to_clusters(BlockDriverState *bs, int64_t sector_num, int nb_sectors, int64_t *cluster_sector_num, diff --git a/include/block/block_int.h b/include/block/block_int.h index 211087aa91..17b26b2a9f 100644 --- a/include/block/block_int.h +++ b/include/block/block_int.h @@ -168,6 +168,7 @@ struct BlockDriver { int (*bdrv_snapshot_load_tmp)(BlockDriverState *bs, const char *snapshot_name); int (*bdrv_get_info)(BlockDriverState *bs, BlockDriverInfo *bdi); + ImageInfoSpecific *(*bdrv_get_specific_info)(BlockDriverState *bs); int (*bdrv_save_vmstate)(BlockDriverState *bs, QEMUIOVector *qiov, int64_t pos); From a8d8ecb77fc16da49ea2c1edae267dc9d0c01dfd Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Wed, 9 Oct 2013 10:46:17 +0200 Subject: [PATCH 06/61] block/qapi: Human-readable ImageInfoSpecific dump Add a function for generically dumping the ImageInfoSpecific information in a human-readable format to block/qapi.c. Use this function in bdrv_image_info_dump and qemu-io-cmds.c:info_f to allow qemu-img info resp. qemu-io -c info to print that format specific information. Signed-off-by: Max Reitz Reviewed-by: Eric Blake Signed-off-by: Kevin Wolf --- block/qapi.c | 121 +++++++++++++++++++++++++++++++++++++++++++ include/block/qapi.h | 2 + qemu-io-cmds.c | 9 ++++ 3 files changed, 132 insertions(+) diff --git a/block/qapi.c b/block/qapi.c index ab1dd24b76..5880b3e42b 100644 --- a/block/qapi.c +++ b/block/qapi.c @@ -25,6 +25,9 @@ #include "block/qapi.h" #include "block/block_int.h" #include "qmp-commands.h" +#include "qapi-visit.h" +#include "qapi/qmp-output-visitor.h" +#include "qapi/qmp/types.h" /* * Returns 0 on success, with *p_list either set to describe snapshot @@ -426,6 +429,119 @@ void bdrv_snapshot_dump(fprintf_function func_fprintf, void *f, } } +static void dump_qdict(fprintf_function func_fprintf, void *f, int indentation, + QDict *dict); +static void dump_qlist(fprintf_function func_fprintf, void *f, int indentation, + QList *list); + +static void dump_qobject(fprintf_function func_fprintf, void *f, + int comp_indent, QObject *obj) +{ + switch (qobject_type(obj)) { + case QTYPE_QINT: { + QInt *value = qobject_to_qint(obj); + func_fprintf(f, "%" PRId64, qint_get_int(value)); + break; + } + case QTYPE_QSTRING: { + QString *value = qobject_to_qstring(obj); + func_fprintf(f, "%s", qstring_get_str(value)); + break; + } + case QTYPE_QDICT: { + QDict *value = qobject_to_qdict(obj); + dump_qdict(func_fprintf, f, comp_indent, value); + break; + } + case QTYPE_QLIST: { + QList *value = qobject_to_qlist(obj); + dump_qlist(func_fprintf, f, comp_indent, value); + break; + } + case QTYPE_QFLOAT: { + QFloat *value = qobject_to_qfloat(obj); + func_fprintf(f, "%g", qfloat_get_double(value)); + break; + } + case QTYPE_QBOOL: { + QBool *value = qobject_to_qbool(obj); + func_fprintf(f, "%s", qbool_get_int(value) ? "true" : "false"); + break; + } + case QTYPE_QERROR: { + QString *value = qerror_human((QError *)obj); + func_fprintf(f, "%s", qstring_get_str(value)); + break; + } + case QTYPE_NONE: + break; + case QTYPE_MAX: + default: + abort(); + } +} + +static void dump_qlist(fprintf_function func_fprintf, void *f, int indentation, + QList *list) +{ + const QListEntry *entry; + int i = 0; + + for (entry = qlist_first(list); entry; entry = qlist_next(entry), i++) { + qtype_code type = qobject_type(entry->value); + bool composite = (type == QTYPE_QDICT || type == QTYPE_QLIST); + const char *format = composite ? "%*s[%i]:\n" : "%*s[%i]: "; + + func_fprintf(f, format, indentation * 4, "", i); + dump_qobject(func_fprintf, f, indentation + 1, entry->value); + if (!composite) { + func_fprintf(f, "\n"); + } + } +} + +static void dump_qdict(fprintf_function func_fprintf, void *f, int indentation, + QDict *dict) +{ + const QDictEntry *entry; + + for (entry = qdict_first(dict); entry; entry = qdict_next(dict, entry)) { + qtype_code type = qobject_type(entry->value); + bool composite = (type == QTYPE_QDICT || type == QTYPE_QLIST); + const char *format = composite ? "%*s%s:\n" : "%*s%s: "; + char key[strlen(entry->key) + 1]; + int i; + + /* replace dashes with spaces in key (variable) names */ + for (i = 0; entry->key[i]; i++) { + key[i] = entry->key[i] == '-' ? ' ' : entry->key[i]; + } + key[i] = 0; + + func_fprintf(f, format, indentation * 4, "", key); + dump_qobject(func_fprintf, f, indentation + 1, entry->value); + if (!composite) { + func_fprintf(f, "\n"); + } + } +} + +void bdrv_image_info_specific_dump(fprintf_function func_fprintf, void *f, + ImageInfoSpecific *info_spec) +{ + Error *local_err = NULL; + QmpOutputVisitor *ov = qmp_output_visitor_new(); + QObject *obj, *data; + + visit_type_ImageInfoSpecific(qmp_output_get_visitor(ov), &info_spec, NULL, + &local_err); + obj = qmp_output_get_qobject(ov); + assert(qobject_type(obj) == QTYPE_QDICT); + data = qdict_get(qobject_to_qdict(obj), "data"); + dump_qobject(func_fprintf, f, 1, data); + qmp_output_visitor_cleanup(ov); +} + void bdrv_image_info_dump(fprintf_function func_fprintf, void *f, ImageInfo *info) { @@ -496,4 +612,9 @@ void bdrv_image_info_dump(fprintf_function func_fprintf, void *f, func_fprintf(f, "\n"); } } + + if (info->has_format_specific) { + func_fprintf(f, "Format specific information:\n"); + bdrv_image_info_specific_dump(func_fprintf, f, info->format_specific); + } } diff --git a/include/block/qapi.h b/include/block/qapi.h index 0496cc9282..9518ee4001 100644 --- a/include/block/qapi.h +++ b/include/block/qapi.h @@ -42,6 +42,8 @@ BlockStats *bdrv_query_stats(const BlockDriverState *bs); void bdrv_snapshot_dump(fprintf_function func_fprintf, void *f, QEMUSnapshotInfo *sn); +void bdrv_image_info_specific_dump(fprintf_function func_fprintf, void *f, + ImageInfoSpecific *info_spec); void bdrv_image_info_dump(fprintf_function func_fprintf, void *f, ImageInfo *info); #endif diff --git a/qemu-io-cmds.c b/qemu-io-cmds.c index 8565d49336..667f4e4f3a 100644 --- a/qemu-io-cmds.c +++ b/qemu-io-cmds.c @@ -10,6 +10,7 @@ #include "qemu-io.h" #include "block/block_int.h" +#include "block/qapi.h" #include "qemu/main-loop.h" #define CMD_NOFILE_OK 0x01 @@ -1678,6 +1679,7 @@ static const cmdinfo_t length_cmd = { static int info_f(BlockDriverState *bs, int argc, char **argv) { BlockDriverInfo bdi; + ImageInfoSpecific *spec_info; char s1[64], s2[64]; int ret; @@ -1699,6 +1701,13 @@ static int info_f(BlockDriverState *bs, int argc, char **argv) printf("cluster size: %s\n", s1); printf("vm state offset: %s\n", s2); + spec_info = bdrv_get_specific_info(bs); + if (spec_info) { + printf("Format specific information:\n"); + bdrv_image_info_specific_dump(fprintf, stdout, spec_info); + qapi_free_ImageInfoSpecific(spec_info); + } + return 0; } From 37764dfb71c4d0d058b71ba33340c6beab7d5a66 Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Wed, 9 Oct 2013 10:46:18 +0200 Subject: [PATCH 07/61] qcow2: Add support for ImageInfoSpecific Add a new ImageInfoSpecificQCow2 type as a subtype of ImageInfoSpecific. This contains the compatibility level as a string and an optional lazy_refcounts boolean (optional means mandatory for compat >= 1.1 and not available for compat == 0.10). Also, add qcow2_get_specific_info, which returns this information. Signed-off-by: Max Reitz Signed-off-by: Kevin Wolf --- block/qcow2.c | 28 ++++++++++++++++++++++++++++ qapi-schema.json | 16 ++++++++++++++++ 2 files changed, 44 insertions(+) diff --git a/block/qcow2.c b/block/qcow2.c index 4a9888cc7f..e8d2735be7 100644 --- a/block/qcow2.c +++ b/block/qcow2.c @@ -1810,6 +1810,33 @@ static int qcow2_get_info(BlockDriverState *bs, BlockDriverInfo *bdi) return 0; } +static ImageInfoSpecific *qcow2_get_specific_info(BlockDriverState *bs) +{ + BDRVQcowState *s = bs->opaque; + ImageInfoSpecific *spec_info = g_new(ImageInfoSpecific, 1); + + *spec_info = (ImageInfoSpecific){ + .kind = IMAGE_INFO_SPECIFIC_KIND_QCOW2, + { + .qcow2 = g_new(ImageInfoSpecificQCow2, 1), + }, + }; + if (s->qcow_version == 2) { + *spec_info->qcow2 = (ImageInfoSpecificQCow2){ + .compat = g_strdup("0.10"), + }; + } else if (s->qcow_version == 3) { + *spec_info->qcow2 = (ImageInfoSpecificQCow2){ + .compat = g_strdup("1.1"), + .lazy_refcounts = s->compatible_features & + QCOW2_COMPAT_LAZY_REFCOUNTS, + .has_lazy_refcounts = true, + }; + } + + return spec_info; +} + #if 0 static void dump_refcounts(BlockDriverState *bs) { @@ -2130,6 +2157,7 @@ static BlockDriver bdrv_qcow2 = { .bdrv_snapshot_list = qcow2_snapshot_list, .bdrv_snapshot_load_tmp = qcow2_snapshot_load_tmp, .bdrv_get_info = qcow2_get_info, + .bdrv_get_specific_info = qcow2_get_specific_info, .bdrv_save_vmstate = qcow2_save_vmstate, .bdrv_load_vmstate = qcow2_load_vmstate, diff --git a/qapi-schema.json b/qapi-schema.json index 246789b378..a1a81a4481 100644 --- a/qapi-schema.json +++ b/qapi-schema.json @@ -209,6 +209,21 @@ 'date-sec': 'int', 'date-nsec': 'int', 'vm-clock-sec': 'int', 'vm-clock-nsec': 'int' } } +## +# @ImageInfoSpecificQCow2: +# +# @compat: compatibility level +# +# @lazy-refcounts: #optional on or off; only valid for compat >= 1.1 +# +# Since: 1.7 +## +{ 'type': 'ImageInfoSpecificQCow2', + 'data': { + 'compat': 'str', + '*lazy-refcounts': 'bool' + } } + ## # @ImageInfoSpecific: # @@ -219,6 +234,7 @@ { 'union': 'ImageInfoSpecific', 'data': { + 'qcow2': 'ImageInfoSpecificQCow2' } } ## From 4c2e946500c45685fdec61b3d929311dc26a2ad5 Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Wed, 9 Oct 2013 10:46:19 +0200 Subject: [PATCH 08/61] qemu-iotests: Discard specific info in _img_info In _img_info, filter out additional information specific to the image format provided by qemu-img info, since tests designed for multiple image formats would produce different outputs for every image format otherwise. In a human-readable dump, that new information will always be last for each "image information block" (multiple blocks are emitted when inspecting the backing file chain). Every block is separated by an empty line. Therefore, in this case, everything starting with the line "Format specific information:" up to that empty line (or EOF, if it is the last block) has to be stripped. The JSON dump will always emit pretty JSON data. Therefore, the opening and closing braces of every object will be on lines which are indented by exactly the same amount, and all lines in between will have more indentation. Thus, in this case, everything starting with a line matching the regular expression /^ *"format-specific": {/ until /^ *},?/ has to be stripped, where the number of spaces at the beginning of the respective lines is equal. Signed-off-by: Max Reitz Reviewed-by: Eric Blake Signed-off-by: Kevin Wolf --- tests/qemu-iotests/common.rc | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/tests/qemu-iotests/common.rc b/tests/qemu-iotests/common.rc index 1b22db04f4..227c003538 100644 --- a/tests/qemu-iotests/common.rc +++ b/tests/qemu-iotests/common.rc @@ -197,12 +197,30 @@ _check_test_img() _img_info() { + discard=0 + regex_json_spec_start='^ *"format-specific": \{' $QEMU_IMG info "$@" "$TEST_IMG" 2>&1 | \ sed -e "s#$IMGPROTO:$TEST_DIR#TEST_DIR#g" \ -e "s#$TEST_DIR#TEST_DIR#g" \ -e "s#$IMGFMT#IMGFMT#g" \ -e "/^disk size:/ D" \ - -e "/actual-size/ D" + -e "/actual-size/ D" | \ + while IFS='' read line; do + if [[ $line == "Format specific information:" ]]; then + discard=1 + elif [[ $line =~ $regex_json_spec_start ]]; then + discard=2 + regex_json_spec_end="^${line%%[^ ]*}\\},? *$" + fi + if [[ $discard == 0 ]]; then + echo "$line" + elif [[ $discard == 1 && ! $line ]]; then + echo + discard=0 + elif [[ $discard == 2 && $line =~ $regex_json_spec_end ]]; then + discard=0 + fi + done } _get_pids_by_name() From 3677e6f6252542cbab85674d97d051d95e91693b Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Wed, 9 Oct 2013 10:46:20 +0200 Subject: [PATCH 09/61] qemu-iotests: Additional info from qemu-img info Add a test for the additional information now provided by qemu-img info when used on qcow2 images. It also tests the qemu QMP output from the query-block command when running qemu with different runtime options than specified in the image (ImageInfoSpecific should always refer to the image). Signed-off-by: Max Reitz Signed-off-by: Kevin Wolf --- tests/qemu-iotests/065 | 125 ++++++++++++++++++++++++++++++++++ tests/qemu-iotests/065.out | 5 ++ tests/qemu-iotests/group | 1 + tests/qemu-iotests/iotests.py | 4 ++ 4 files changed, 135 insertions(+) create mode 100755 tests/qemu-iotests/065 create mode 100644 tests/qemu-iotests/065.out diff --git a/tests/qemu-iotests/065 b/tests/qemu-iotests/065 new file mode 100755 index 0000000000..ab5445f62d --- /dev/null +++ b/tests/qemu-iotests/065 @@ -0,0 +1,125 @@ +#!/usr/bin/env python2 +# +# Test for additional information emitted by qemu-img info on qcow2 +# images +# +# Copyright (C) 2013 Red Hat, Inc. +# +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 2 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see . +# + +import os +import re +import json +import iotests +from iotests import qemu_img, qemu_img_pipe +import unittest + +test_img = os.path.join(iotests.test_dir, 'test.img') + +class TestImageInfoSpecific(iotests.QMPTestCase): + '''Abstract base class for ImageInfoSpecific tests''' + + def setUp(self): + if self.img_options is None: + self.skipTest('Skipping abstract test class') + qemu_img('create', '-f', iotests.imgfmt, '-o', self.img_options, + test_img, '128K') + + def tearDown(self): + os.remove(test_img) + +class TestQemuImgInfo(TestImageInfoSpecific): + '''Abstract base class for qemu-img info tests''' + + img_options = None + json_compare = None + human_compare = None + + def test_json(self): + data = json.loads(qemu_img_pipe('info', '--output=json', test_img)) + data = data['format-specific'] + self.assertEqual(data['type'], iotests.imgfmt) + self.assertEqual(data['data'], self.json_compare) + + def test_human(self): + data = qemu_img_pipe('info', '--output=human', test_img).split('\n') + data = data[(data.index('Format specific information:') + 1) + :data.index('')] + for field in data: + self.assertTrue(re.match('^ {4}[^ ]', field) is not None) + data = map(lambda line: line.strip(), data) + self.assertEqual(data, self.human_compare) + +class TestQMP(TestImageInfoSpecific): + '''Abstract base class for qemu QMP tests''' + + img_options = None + qemu_options = '' + TestImageInfoSpecific = TestImageInfoSpecific + + def setUp(self): + self.TestImageInfoSpecific.setUp(self) + self.vm = iotests.VM().add_drive(test_img, self.qemu_options) + self.vm.launch() + + def tearDown(self): + self.vm.shutdown() + self.TestImageInfoSpecific.tearDown(self) + + def test_qmp(self): + result = self.vm.qmp('query-block')['return'] + drive = filter(lambda drive: drive['device'] == 'drive0', result)[0] + data = drive['inserted']['image']['format-specific'] + self.assertEqual(data['type'], iotests.imgfmt) + self.assertEqual(data['data'], self.compare) + +class TestQCow2(TestQemuImgInfo): + '''Testing a qcow2 version 2 image''' + img_options = 'compat=0.10' + json_compare = { 'compat': '0.10' } + human_compare = [ 'compat: 0.10' ] + +class TestQCow3NotLazy(TestQemuImgInfo): + '''Testing a qcow2 version 3 image with lazy refcounts disabled''' + img_options = 'compat=1.1,lazy_refcounts=off' + json_compare = { 'compat': '1.1', 'lazy-refcounts': False } + human_compare = [ 'compat: 1.1', 'lazy refcounts: false' ] + +class TestQCow3Lazy(TestQemuImgInfo): + '''Testing a qcow2 version 3 image with lazy refcounts enabled''' + img_options = 'compat=1.1,lazy_refcounts=on' + json_compare = { 'compat': '1.1', 'lazy-refcounts': True } + human_compare = [ 'compat: 1.1', 'lazy refcounts: true' ] + +class TestQCow3NotLazyQMP(TestQMP): + '''Testing a qcow2 version 3 image with lazy refcounts disabled, opening + with lazy refcounts enabled''' + img_options = 'compat=1.1,lazy_refcounts=off' + qemu_options = 'lazy-refcounts=on' + compare = { 'compat': '1.1', 'lazy-refcounts': False } + +class TestQCow3LazyQMP(TestQMP): + '''Testing a qcow2 version 3 image with lazy refcounts enabled, opening + with lazy refcounts disabled''' + img_options = 'compat=1.1,lazy_refcounts=on' + qemu_options = 'lazy-refcounts=off' + compare = { 'compat': '1.1', 'lazy-refcounts': True } + +TestImageInfoSpecific = None +TestQemuImgInfo = None +TestQMP = None + +if __name__ == '__main__': + iotests.main(supported_fmts=['qcow2']) diff --git a/tests/qemu-iotests/065.out b/tests/qemu-iotests/065.out new file mode 100644 index 0000000000..594c16f49f --- /dev/null +++ b/tests/qemu-iotests/065.out @@ -0,0 +1,5 @@ +........ +---------------------------------------------------------------------- +Ran 8 tests + +OK diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group index 1ad02e5a2c..f1a68b068f 100644 --- a/tests/qemu-iotests/group +++ b/tests/qemu-iotests/group @@ -69,3 +69,4 @@ 061 rw auto 062 rw auto 063 rw auto +065 rw auto diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py index 376d6e8ffe..fb10ff43a7 100644 --- a/tests/qemu-iotests/iotests.py +++ b/tests/qemu-iotests/iotests.py @@ -49,6 +49,10 @@ def qemu_img_verbose(*args): '''Run qemu-img without suppressing its output and return the exit code''' return subprocess.call(qemu_img_args + list(args)) +def qemu_img_pipe(*args): + '''Run qemu-img and return its output''' + return subprocess.Popen(qemu_img_args + list(args), stdout=subprocess.PIPE).communicate()[0] + def qemu_io(*args): '''Run qemu-io and return the stdout data''' args = qemu_io_args + list(args) From f252080453ec081ba653bba4e0c1ca86c52cf19f Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Wed, 9 Oct 2013 10:34:10 +0200 Subject: [PATCH 10/61] qcow2: Alignment of snapshot table entries The qcow2 specification does not explicitly state so far that every snapshot table entry is aligned to 8 bytes. QEMU, in contrast, does this alignment, thus it should be properly documented (which this patch does). Signed-off-by: Max Reitz Signed-off-by: Kevin Wolf --- docs/specs/qcow2.txt | 3 +++ 1 file changed, 3 insertions(+) diff --git a/docs/specs/qcow2.txt b/docs/specs/qcow2.txt index 33eca360cc..f19536a46f 100644 --- a/docs/specs/qcow2.txt +++ b/docs/specs/qcow2.txt @@ -355,3 +355,6 @@ Snapshot table entry: variable: Unique ID string for the snapshot (not null terminated) variable: Name of the snapshot (not null terminated) + + variable: Padding to round up the snapshot table entry size to the + next multiple of 8. From 998b959c1e59044f5d5f64c482f4ce8facc8e0bc Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Wed, 9 Oct 2013 10:42:56 +0200 Subject: [PATCH 11/61] qcow2: Use pread for inactive L1 in overlap check Currently, qcow2_check_metadata_overlap uses bdrv_read to read inactive L1 tables from disk. The number of sectors to read is calculated through a truncating integer division, therefore, if the L1 table size is not a multiple of the sector size, the final entries will not be read and their entries in memory remain undefined (from the g_malloc). Using bdrv_pread fixes this. Signed-off-by: Max Reitz Signed-off-by: Kevin Wolf --- block/qcow2-refcount.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c index 2d67885850..4cb9c2332a 100644 --- a/block/qcow2-refcount.c +++ b/block/qcow2-refcount.c @@ -1719,12 +1719,11 @@ int qcow2_check_metadata_overlap(BlockDriverState *bs, int chk, int64_t offset, for (i = 0; i < s->nb_snapshots; i++) { uint64_t l1_ofs = s->snapshots[i].l1_table_offset; uint32_t l1_sz = s->snapshots[i].l1_size; - uint64_t *l1 = g_malloc(l1_sz * sizeof(uint64_t)); + uint64_t l1_sz2 = l1_sz * sizeof(uint64_t); + uint64_t *l1 = g_malloc(l1_sz2); int ret; - ret = bdrv_read(bs->file, l1_ofs / BDRV_SECTOR_SIZE, (uint8_t *)l1, - l1_sz * sizeof(uint64_t) / BDRV_SECTOR_SIZE); - + ret = bdrv_pread(bs->file, l1_ofs, l1, l1_sz2); if (ret < 0) { g_free(l1); return ret; From 8f730dd24edd2576ecbd596de7ea4361296b129c Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Wed, 9 Oct 2013 10:44:28 +0200 Subject: [PATCH 12/61] qcow2: Free preallocated zero clusters In qcow2_free_any_clusters, preallocated zero clusters should be freed just as normal clusters are. Signed-off-by: Max Reitz Signed-off-by: Kevin Wolf --- block/qcow2-refcount.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c index 4cb9c2332a..4ef689996f 100644 --- a/block/qcow2-refcount.c +++ b/block/qcow2-refcount.c @@ -796,11 +796,13 @@ void qcow2_free_any_clusters(BlockDriverState *bs, uint64_t l2_entry, } break; case QCOW2_CLUSTER_NORMAL: - qcow2_free_clusters(bs, l2_entry & L2E_OFFSET_MASK, - nb_clusters << s->cluster_bits, type); + case QCOW2_CLUSTER_ZERO: + if (l2_entry & L2E_OFFSET_MASK) { + qcow2_free_clusters(bs, l2_entry & L2E_OFFSET_MASK, + nb_clusters << s->cluster_bits, type); + } break; case QCOW2_CLUSTER_UNALLOCATED: - case QCOW2_CLUSTER_ZERO: break; default: abort(); From 37d41f0a04e5017d37906728a806d7944e867a2a Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Wed, 9 Oct 2013 10:51:04 +0200 Subject: [PATCH 13/61] qcow2: Always use error path on writing snapshots qcow2_write_snapshots does contain a fail label and there is no reason not to use it on some errors; therefore, we should always jump there on error. Signed-off-by: Max Reitz Signed-off-by: Kevin Wolf --- block/qcow2-snapshot.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/block/qcow2-snapshot.c b/block/qcow2-snapshot.c index 5e8a7794f4..33379749c1 100644 --- a/block/qcow2-snapshot.c +++ b/block/qcow2-snapshot.c @@ -182,11 +182,12 @@ static int qcow2_write_snapshots(BlockDriverState *bs) snapshots_offset = qcow2_alloc_clusters(bs, snapshots_size); offset = snapshots_offset; if (offset < 0) { - return offset; + ret = offset; + goto fail; } ret = bdrv_flush(bs); if (ret < 0) { - return ret; + goto fail; } /* The snapshot list position has not yet been updated, so these clusters @@ -194,7 +195,7 @@ static int qcow2_write_snapshots(BlockDriverState *bs) ret = qcow2_pre_write_overlap_check(bs, QCOW2_OL_DEFAULT, offset, snapshots_size); if (ret < 0) { - return ret; + goto fail; } From 9186ad9658cc597937fbc03ad66bceb3a0515d99 Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Wed, 9 Oct 2013 10:51:05 +0200 Subject: [PATCH 14/61] qcow2: Free allocated snapshot table on error If an error occurs during qcow2_write_snapshots, the newly allocated snapshot table clusters are leaked and should thus be freed. Signed-off-by: Max Reitz Signed-off-by: Kevin Wolf --- block/qcow2-snapshot.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/block/qcow2-snapshot.c b/block/qcow2-snapshot.c index 33379749c1..f6f3e6423d 100644 --- a/block/qcow2-snapshot.c +++ b/block/qcow2-snapshot.c @@ -279,6 +279,10 @@ static int qcow2_write_snapshots(BlockDriverState *bs) return 0; fail: + if (snapshots_offset > 0) { + qcow2_free_clusters(bs, snapshots_offset, snapshots_size, + QCOW2_DISCARD_ALWAYS); + } return ret; } From 88fb15351284868b70fa1d5b101e809057fcc5aa Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Wed, 9 Oct 2013 10:51:06 +0200 Subject: [PATCH 15/61] qcow2: Assert against snapshot name/ID overflow qcow2_write_snapshots relies on the length of every snapshot ID and name fitting into an unsigned 16 bit integer. This is currently ensured by QEMU through generally only allowing 128 byte IDs and 256 byte names. However, if this should change in the future, the length written to the image file should not be silently truncated (though the name itself would be written completely). Since this is currently not an issue but might require attention due to internal QEMU changes in the future, an assert ensuring sanity is enough for now. Signed-off-by: Max Reitz Signed-off-by: Kevin Wolf --- block/qcow2-snapshot.c | 1 + 1 file changed, 1 insertion(+) diff --git a/block/qcow2-snapshot.c b/block/qcow2-snapshot.c index f6f3e6423d..812dab2aa5 100644 --- a/block/qcow2-snapshot.c +++ b/block/qcow2-snapshot.c @@ -221,6 +221,7 @@ static int qcow2_write_snapshots(BlockDriverState *bs) id_str_size = strlen(sn->id_str); name_size = strlen(sn->name); + assert(id_str_size <= UINT16_MAX && name_size <= UINT16_MAX); h.id_str_size = cpu_to_be16(id_str_size); h.name_size = cpu_to_be16(name_size); offset = align_offset(offset, 8); From 92bc50a5ad7fbc9a0bd17240eaea5027a100ca79 Mon Sep 17 00:00:00 2001 From: Peter Lieven Date: Tue, 8 Oct 2013 14:43:14 +0200 Subject: [PATCH 16/61] block/get_block_status: avoid redundant callouts on raw devices if a raw device like an iscsi target or host device is used the current implementation makes a second call out to get the block status of bs->file. Signed-off-by: Peter Lieven Reviewed-by: Eric Blake Signed-off-by: Kevin Wolf --- block.c | 6 ++++++ block/raw_bsd.c | 4 +++- include/block/block.h | 4 ++++ 3 files changed, 13 insertions(+), 1 deletion(-) diff --git a/block.c b/block.c index 43acaadbce..d86efad867 100644 --- a/block.c +++ b/block.c @@ -3147,6 +3147,12 @@ static int64_t coroutine_fn bdrv_co_get_block_status(BlockDriverState *bs, return ret; } + if (ret & BDRV_BLOCK_RAW) { + assert(ret & BDRV_BLOCK_OFFSET_VALID); + return bdrv_get_block_status(bs->file, ret >> BDRV_SECTOR_BITS, + *pnum, pnum); + } + if (!(ret & BDRV_BLOCK_DATA)) { if (bdrv_has_zero_init(bs)) { ret |= BDRV_BLOCK_ZERO; diff --git a/block/raw_bsd.c b/block/raw_bsd.c index d4ace6020b..d61906bcc2 100644 --- a/block/raw_bsd.c +++ b/block/raw_bsd.c @@ -62,7 +62,9 @@ static int64_t coroutine_fn raw_co_get_block_status(BlockDriverState *bs, int64_t sector_num, int nb_sectors, int *pnum) { - return bdrv_get_block_status(bs->file, sector_num, nb_sectors, pnum); + *pnum = nb_sectors; + return BDRV_BLOCK_RAW | BDRV_BLOCK_OFFSET_VALID | BDRV_BLOCK_DATA | + (sector_num << BDRV_SECTOR_BITS); } static int coroutine_fn raw_co_write_zeroes(BlockDriverState *bs, diff --git a/include/block/block.h b/include/block/block.h index e26512472d..0d4d5c336c 100644 --- a/include/block/block.h +++ b/include/block/block.h @@ -84,6 +84,9 @@ typedef struct BlockDevOps { /* BDRV_BLOCK_DATA: data is read from bs->file or another file * BDRV_BLOCK_ZERO: sectors read as zero * BDRV_BLOCK_OFFSET_VALID: sector stored in bs->file as raw data + * BDRV_BLOCK_RAW: used internally to indicate that the request + * was answered by the raw driver and that one + * should look in bs->file directly. * * If BDRV_BLOCK_OFFSET_VALID is set, bits 9-62 represent the offset in * bs->file where sector data can be read from as raw data. @@ -105,6 +108,7 @@ typedef struct BlockDevOps { #define BDRV_BLOCK_DATA 1 #define BDRV_BLOCK_ZERO 2 #define BDRV_BLOCK_OFFSET_VALID 4 +#define BDRV_BLOCK_RAW 8 #define BDRV_BLOCK_OFFSET_MASK BDRV_SECTOR_MASK typedef enum { From f6186f49e2c98d91f22027d8c62996df4fcf3f92 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beno=C3=AEt=20Canet?= Date: Wed, 2 Oct 2013 14:33:48 +0200 Subject: [PATCH 17/61] block: Add BlockDriver.bdrv_check_ext_snapshot. This field is used by blkverify to disable external snapshots creation. It will also be used by block filters like quorum to disable external snapshot creation. Signed-off-by: Benoit Canet Reviewed-by: Max Reitz Signed-off-by: Kevin Wolf --- block.c | 19 +++++++++++++++++++ block/blkverify.c | 2 ++ blockdev.c | 5 +++++ include/block/block.h | 14 ++++++++++++++ include/block/block_int.h | 6 ++++++ 5 files changed, 46 insertions(+) diff --git a/block.c b/block.c index d86efad867..beea0274f8 100644 --- a/block.c +++ b/block.c @@ -4647,3 +4647,22 @@ int bdrv_amend_options(BlockDriverState *bs, QEMUOptionParameter *options) } return bs->drv->bdrv_amend_options(bs, options); } + +ExtSnapshotPerm bdrv_check_ext_snapshot(BlockDriverState *bs) +{ + if (bs->drv->bdrv_check_ext_snapshot) { + return bs->drv->bdrv_check_ext_snapshot(bs); + } + + if (bs->file && bs->file->drv && bs->file->drv->bdrv_check_ext_snapshot) { + return bs->file->drv->bdrv_check_ext_snapshot(bs); + } + + /* external snapshots are allowed by default */ + return EXT_SNAPSHOT_ALLOWED; +} + +ExtSnapshotPerm bdrv_check_ext_snapshot_forbidden(BlockDriverState *bs) +{ + return EXT_SNAPSHOT_FORBIDDEN; +} diff --git a/block/blkverify.c b/block/blkverify.c index bff95d2a45..1e8e6d9961 100644 --- a/block/blkverify.c +++ b/block/blkverify.c @@ -417,6 +417,8 @@ static BlockDriver bdrv_blkverify = { .bdrv_aio_readv = blkverify_aio_readv, .bdrv_aio_writev = blkverify_aio_writev, .bdrv_aio_flush = blkverify_aio_flush, + + .bdrv_check_ext_snapshot = bdrv_check_ext_snapshot_forbidden, }; static void bdrv_blkverify_init(void) diff --git a/blockdev.c b/blockdev.c index 8c83f6f1ca..a91d5a87df 100644 --- a/blockdev.c +++ b/blockdev.c @@ -1131,6 +1131,11 @@ static void external_snapshot_prepare(BlkTransactionState *common, } } + if (bdrv_check_ext_snapshot(state->old_bs) != EXT_SNAPSHOT_ALLOWED) { + error_set(errp, QERR_FEATURE_DISABLED, "snapshot"); + return; + } + flags = state->old_bs->open_flags; /* create new image w/backing file */ diff --git a/include/block/block.h b/include/block/block.h index 0d4d5c336c..3560deb883 100644 --- a/include/block/block.h +++ b/include/block/block.h @@ -248,6 +248,20 @@ int bdrv_check(BlockDriverState *bs, BdrvCheckResult *res, BdrvCheckMode fix); int bdrv_amend_options(BlockDriverState *bs_new, QEMUOptionParameter *options); +/* external snapshots */ + +typedef enum { + EXT_SNAPSHOT_ALLOWED, + EXT_SNAPSHOT_FORBIDDEN, +} ExtSnapshotPerm; + +/* return EXT_SNAPSHOT_ALLOWED if external snapshot is allowed + * return EXT_SNAPSHOT_FORBIDDEN if external snapshot is forbidden + */ +ExtSnapshotPerm bdrv_check_ext_snapshot(BlockDriverState *bs); +/* helper used to forbid external snapshots like in blkverify */ +ExtSnapshotPerm bdrv_check_ext_snapshot_forbidden(BlockDriverState *bs); + /* async block I/O */ typedef void BlockDriverDirtyHandler(BlockDriverState *bs, int64_t sector, int sector_num); diff --git a/include/block/block_int.h b/include/block/block_int.h index 17b26b2a9f..a48731d539 100644 --- a/include/block/block_int.h +++ b/include/block/block_int.h @@ -67,6 +67,12 @@ typedef struct BdrvTrackedRequest { struct BlockDriver { const char *format_name; int instance_size; + + /* if not defined external snapshots are allowed + * future block filters will query their children to build the response + */ + ExtSnapshotPerm (*bdrv_check_ext_snapshot)(BlockDriverState *bs); + int (*bdrv_probe)(const uint8_t *buf, int buf_size, const char *filename); int (*bdrv_probe_device)(const char *filename); From 975a93c082452db9aa1397a797ca8f13ba367393 Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Wed, 9 Oct 2013 12:07:33 +0200 Subject: [PATCH 18/61] qemu-iotests: Discard preallocated zero clusters Add a new test case for discarding preallocated zero clusters; doing this should not result in any leaks. Signed-off-by: Max Reitz Signed-off-by: Kevin Wolf --- tests/qemu-iotests/066 | 63 ++++++++++++++++++++++++++++++++++++++ tests/qemu-iotests/066.out | 13 ++++++++ tests/qemu-iotests/group | 1 + 3 files changed, 77 insertions(+) create mode 100755 tests/qemu-iotests/066 create mode 100644 tests/qemu-iotests/066.out diff --git a/tests/qemu-iotests/066 b/tests/qemu-iotests/066 new file mode 100755 index 0000000000..1c2452b0c5 --- /dev/null +++ b/tests/qemu-iotests/066 @@ -0,0 +1,63 @@ +#!/bin/bash +# +# Test case for discarding preallocated zero clusters in qcow2 +# +# Copyright (C) 2013 Red Hat, Inc. +# +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 2 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see . +# + +# creator +owner=mreitz@redhat.com + +seq="$(basename $0)" +echo "QA output created by $seq" + +here="$PWD" +tmp=/tmp/$$ +status=1 # failure is the default! + +_cleanup() +{ + _cleanup_test_img +} +trap "_cleanup; exit \$status" 0 1 2 3 15 + +# get standard environment, filters and checks +. ./common.rc +. ./common.filter + +# This tests qocw2-specific low-level functionality +_supported_fmt qcow2 +_supported_proto generic +_supported_os Linux + +IMGOPTS="compat=1.1" +IMG_SIZE=64M + +echo +echo "=== Testing snapshotting an image with zero clusters ===" +echo +_make_test_img $IMG_SIZE +# Write some normal clusters, zero them (creating preallocated zero clusters) +# and discard those +$QEMU_IO -c "write 0 256k" -c "write -z 0 256k" -c "discard 0 256k" "$TEST_IMG" \ + | _filter_qemu_io +# Check the image (there shouldn't be any leaks) +_check_test_img + +# success, all done +echo "*** done" +rm -f $seq.full +status=0 diff --git a/tests/qemu-iotests/066.out b/tests/qemu-iotests/066.out new file mode 100644 index 0000000000..9139780f49 --- /dev/null +++ b/tests/qemu-iotests/066.out @@ -0,0 +1,13 @@ +QA output created by 066 + +=== Testing snapshotting an image with zero clusters === + +Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864 +wrote 262144/262144 bytes at offset 0 +256 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) +wrote 262144/262144 bytes at offset 0 +256 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) +discard 262144/262144 bytes at offset 0 +256 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) +No errors were found on the image. +*** done diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group index f1a68b068f..9c94d14632 100644 --- a/tests/qemu-iotests/group +++ b/tests/qemu-iotests/group @@ -70,3 +70,4 @@ 062 rw auto 063 rw auto 065 rw auto +066 rw auto From 13164591f30ad95ae24f9892cf2caf779271a29b Mon Sep 17 00:00:00 2001 From: "Michael S. Tsirkin" Date: Sun, 29 Sep 2013 00:09:35 +0300 Subject: [PATCH 19/61] ahci: set ahci mode on reset ATM we set AHCI mode on 1st GHC write. Spec says we should set it on reset. Signed-off-by: Michael S. Tsirkin Signed-off-by: Kevin Wolf --- hw/ide/ahci.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c index a71a4ca47c..a8be62cf99 100644 --- a/hw/ide/ahci.c +++ b/hw/ide/ahci.c @@ -1198,7 +1198,15 @@ void ahci_reset(AHCIState *s) int i; s->control_regs.irqstatus = 0; - s->control_regs.ghc = 0; + /* AHCI Enable (AE) + * The implementation of this bit is dependent upon the value of the + * CAP.SAM bit. If CAP.SAM is '0', then GHC.AE shall be read-write and + * shall have a reset value of '0'. If CAP.SAM is '1', then AE shall be + * read-only and shall have a reset value of '1'. + * + * We set HOST_CAP_AHCI so we must enable AHCI at reset. + */ + s->control_regs.ghc = HOST_CTL_AHCI_EN; for (i = 0; i < s->ports; i++) { pr = &s->dev[i].port_regs; From 89e911816a1d5cdbc9480d5464c571d216cf5ea8 Mon Sep 17 00:00:00 2001 From: Jeff Cody Date: Fri, 27 Sep 2013 08:48:15 -0400 Subject: [PATCH 20/61] block: qemu-iotests for vhdx, read sample dynamic image This adds the VHDX format to the qemu-iotests format, and adds a read test. The test reads from an existing sample image, that was created with Hyper-V under Windwos Server 2012. The image file is a 1GB dynamic image, with 32MB blocks. The pattern 0xa5 exists from 0MB-33MB (past a block size boundary) The pattern 0x96 exists from 33MB-66MB (past another block boundary, and leaving a partial blank block) From 66MB-1024MB, all reads should return 0. Although 1GB dynamic image with 66MB of data, the bzip2'ed image file size is only 874 bytes. This also adds in the IMGFMT_GENERIC flag, so r/o images can be tested (e.g. ./check -vhdx) without failing tests that assume r/w support. Signed-off-by: Jeff Cody Signed-off-by: Kevin Wolf --- tests/qemu-iotests/064 | 62 ++++++++++++++++++ tests/qemu-iotests/064.out | 14 ++++ tests/qemu-iotests/common | 8 +++ tests/qemu-iotests/common.rc | 2 +- tests/qemu-iotests/group | 1 + .../sample_images/iotest-dynamic-1G.vhdx.bz2 | Bin 0 -> 874 bytes 6 files changed, 86 insertions(+), 1 deletion(-) create mode 100755 tests/qemu-iotests/064 create mode 100644 tests/qemu-iotests/064.out create mode 100644 tests/qemu-iotests/sample_images/iotest-dynamic-1G.vhdx.bz2 diff --git a/tests/qemu-iotests/064 b/tests/qemu-iotests/064 new file mode 100755 index 0000000000..6789aa6ee4 --- /dev/null +++ b/tests/qemu-iotests/064 @@ -0,0 +1,62 @@ +#!/bin/bash +# +# Test VHDX read/write from a sample image created with Hyper-V +# +# Copyright (C) 2013 Red Hat, Inc. +# +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 2 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see . +# + +# creator +owner=jcody@redhat.com + +seq=`basename $0` +echo "QA output created by $seq" + +here=`pwd` +tmp=/tmp/$$ +status=1 # failure is the default! + +_cleanup() +{ + _cleanup_test_img +} +trap "_cleanup; exit \$status" 0 1 2 3 15 + +# get standard environment, filters and checks +. ./common.rc +. ./common.filter + +_supported_fmt vhdx +_supported_proto generic +_supported_os Linux + +_use_sample_img iotest-dynamic-1G.vhdx.bz2 + +echo +echo "=== Verify pattern 0xa5, 0 - 33MB ===" +$QEMU_IO -r -c "read -pP 0xa5 0 33M" "$TEST_IMG" | _filter_qemu_io + +echo +echo "=== Verify pattern 0x96, 33M - 66M ===" +$QEMU_IO -r -c "read -pP 0x96 33M 33M" "$TEST_IMG" | _filter_qemu_io + +echo +echo "=== Verify pattern 0x00, 66M - 1024M ===" +$QEMU_IO -r -c "read -pP 0x00 66M 958M" "$TEST_IMG" | _filter_qemu_io + +# success, all done +echo "*** done" +rm -f $seq.full +status=0 diff --git a/tests/qemu-iotests/064.out b/tests/qemu-iotests/064.out new file mode 100644 index 0000000000..b9e8e4a873 --- /dev/null +++ b/tests/qemu-iotests/064.out @@ -0,0 +1,14 @@ +QA output created by 064 + +=== Verify pattern 0xa5, 0 - 33MB === +read 34603008/34603008 bytes at offset 0 +33 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) + +=== Verify pattern 0x96, 33M - 66M === +read 34603008/34603008 bytes at offset 34603008 +33 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) + +=== Verify pattern 0x00, 66M - 1024M === +read 1004535808/1004535808 bytes at offset 69206016 +958 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) +*** done diff --git a/tests/qemu-iotests/common b/tests/qemu-iotests/common index fecaf85074..2932e14e73 100644 --- a/tests/qemu-iotests/common +++ b/tests/qemu-iotests/common @@ -45,6 +45,7 @@ valgrind=false rm -f $tmp.list $tmp.tmp $tmp.sed export IMGFMT=raw +export IMGFMT_GENERIC=true export IMGPROTO=file export IMGOPTS="" export QEMU_IO_OPTIONS="" @@ -133,6 +134,7 @@ check options -qed test qed -vdi test vdi -vpc test vpc + -vhdx test vhdx -vmdk test vmdk -rbd test rbd -sheepdog test sheepdog @@ -195,6 +197,12 @@ testlist options xpand=false ;; + -vhdx) + IMGFMT=vhdx + xpand=false + IMGFMT_GENERIC=false + ;; + -rbd) IMGPROTO=rbd xpand=false diff --git a/tests/qemu-iotests/common.rc b/tests/qemu-iotests/common.rc index 227c003538..4e826040d4 100644 --- a/tests/qemu-iotests/common.rc +++ b/tests/qemu-iotests/common.rc @@ -339,7 +339,7 @@ _fail() _supported_fmt() { for f; do - if [ "$f" = "$IMGFMT" -o "$f" = "generic" ]; then + if [ "$f" = "$IMGFMT" -o "$f" = "generic" -a "$IMGFMT_GENERIC" = "true" ]; then return fi done diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group index 9c94d14632..514bd87628 100644 --- a/tests/qemu-iotests/group +++ b/tests/qemu-iotests/group @@ -69,5 +69,6 @@ 061 rw auto 062 rw auto 063 rw auto +064 rw auto 065 rw auto 066 rw auto diff --git a/tests/qemu-iotests/sample_images/iotest-dynamic-1G.vhdx.bz2 b/tests/qemu-iotests/sample_images/iotest-dynamic-1G.vhdx.bz2 new file mode 100644 index 0000000000000000000000000000000000000000..77d97a0bae2e45b84ad489a77788084cc09331e0 GIT binary patch literal 874 zcmV-w1C{(jT4*^jL0KkKS)@Vdf&hZIfB*mY_ijz2|NMXYGlTE%z2+coecwBIyV?7? zRYF<0^&db2mkJrM&`P9@s2TtO0000000001paz-%27#ae0017K&}e8HN-(1kVrigg z13&{nXwU!#fB~QY0000000001pg_pcpwKcJX_G+7gwSGQGHB2;7>oqbhyZB84Gbd_ zCQULjF*ML*U`z^kK$s^jUBKR?z~CteOe$Qc!=`MK zpppgLEo&~>A_R+6h=@Ddl1ZgN7+7*hWL09_s{bL}swE6xrU?On8VxK`u45pSB}p6^9?4@FBo_q60jNQ8)7nwfo~p{X z21&Hu-QCr>IEs=a0NK%&p{HE(oeD(TehV85d6fuURBs<`t5K&|m^Maoo0fjdi=f}{ z>E$yWvA4r~wZiNcLm<7lTF>T|a9Q1JFM7G$o)?hwxePeOZq}`A`za=Z)W^WEd#*Qp z#kZ!X0!i<$>Ta+f8L@u+=r$sPWWhg>cfbk7O*y(bhdvqNE8G<%ccZNIDKSnW9b)lr z?+BB(@#_SGMX@o$bqc@{OqyW;7?h|wi9+N-4D|@7-dVn6g4ihCM|pt2A}N5VGk<5? z{FoRKTsV)8P`B0_=}2A^fU(eP{;OfF)7siztdO#@z6*HnA0O+ly16Hl#BjL02H~Lm zU#o1s*F84YWNZGN7x>n4q>?0voA1&{kdPzmvi42q4UHtuzRJ>PzbPk0x?j$@OjIC3 z291dl2@(l)mGJeNMy5-7djC|iYdFpGgRa2BeV-I!Lj_~=H)^bLy>S7`E*Kbz7L6m% z!pI?M^X|&xFCh=b`Ky@r+f7T@9xGSDdF)KJ&1d`VHFb7cxH1v|W<-)Ch@CnRAp<+? z+_Yc$iVDdxn#!(cW8fGi)&T|p2nYZGK>!1RnrIY~H8%;$rRy2Ky^V+T-`Z@ A&j0`b literal 0 HcmV?d00001 From 9e3f08923a14ba0655c6797edd9ffef44bb8cbf2 Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Wed, 9 Oct 2013 14:40:48 +0200 Subject: [PATCH 21/61] qcow2: Add missing space in error message The error message in qcow2_downgrade about an unsupported refcount order is missing a space. This patch adds it. Signed-off-by: Max Reitz Signed-off-by: Kevin Wolf --- block/qcow2.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/block/qcow2.c b/block/qcow2.c index e8d2735be7..9095f7ca6e 100644 --- a/block/qcow2.c +++ b/block/qcow2.c @@ -1915,7 +1915,7 @@ static int qcow2_downgrade(BlockDriverState *bs, int target_version) * support anything different than 4 anyway, there is no point in doing * so right now; however, we should error out (if qemu supports this in * the future and this code has not been adapted) */ - error_report("qcow2_downgrade: Image refcount orders other than 4 are" + error_report("qcow2_downgrade: Image refcount orders other than 4 are " "currently not supported."); return -ENOTSUP; } From f9bff971436b5924ca3c3203c6a3dcd6437bd430 Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Wed, 9 Oct 2013 14:41:28 +0200 Subject: [PATCH 22/61] qcow2: Remove wrong metadata overlap check In qcow2_write_compressed, if the compression fails, a normal cluster is written to disk. This is done through bdrv_write on the qcow2 BDS itself (using the guest offset), thus it is wrong to do a metadata overlap check before. Signed-off-by: Max Reitz Signed-off-by: Kevin Wolf --- block/qcow2.c | 8 -------- 1 file changed, 8 deletions(-) diff --git a/block/qcow2.c b/block/qcow2.c index 9095f7ca6e..3d1e74d38d 100644 --- a/block/qcow2.c +++ b/block/qcow2.c @@ -1738,14 +1738,6 @@ static int qcow2_write_compressed(BlockDriverState *bs, int64_t sector_num, if (ret != Z_STREAM_END || out_len >= s->cluster_size) { /* could not compress: write normal cluster */ - - ret = qcow2_pre_write_overlap_check(bs, QCOW2_OL_DEFAULT, - sector_num * BDRV_SECTOR_SIZE, - s->cluster_sectors * BDRV_SECTOR_SIZE); - if (ret < 0) { - goto fail; - } - ret = bdrv_write(bs, sector_num, buf, s->cluster_sectors); if (ret < 0) { goto fail; From 84757f7e67cda3df8b04e06fbdeecc266415d2f3 Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Wed, 9 Oct 2013 14:42:00 +0200 Subject: [PATCH 23/61] qcow2: Fix snapshot restoration in snapshot_create If the new snapshot table could not be written in qcow2_snapshot_create, the old snapshot table has to be restored in memory and the new one released. This should include restoration of the old snapshot count as well, which is added by this patch. Signed-off-by: Max Reitz Signed-off-by: Kevin Wolf --- block/qcow2-snapshot.c | 1 + 1 file changed, 1 insertion(+) diff --git a/block/qcow2-snapshot.c b/block/qcow2-snapshot.c index 812dab2aa5..fe7e14cc89 100644 --- a/block/qcow2-snapshot.c +++ b/block/qcow2-snapshot.c @@ -433,6 +433,7 @@ int qcow2_snapshot_create(BlockDriverState *bs, QEMUSnapshotInfo *sn_info) if (ret < 0) { g_free(s->snapshots); s->snapshots = old_snapshot_list; + s->nb_snapshots--; goto fail; } From 00c49b21e7af1dd8d2167c1b019619ac186dad14 Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Wed, 9 Oct 2013 14:42:47 +0200 Subject: [PATCH 24/61] qcow2: Use better type for numerical snapshot ID When trying to find a new snapshot ID, the existing ones are converted to integers using strtoul. This function returns an unsigned long, therefore its result should be saved in an unsigned long as well. Signed-off-by: Max Reitz Signed-off-by: Kevin Wolf --- block/qcow2-snapshot.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/block/qcow2-snapshot.c b/block/qcow2-snapshot.c index fe7e14cc89..884b06d9fe 100644 --- a/block/qcow2-snapshot.c +++ b/block/qcow2-snapshot.c @@ -292,7 +292,8 @@ static void find_new_snapshot_id(BlockDriverState *bs, { BDRVQcowState *s = bs->opaque; QCowSnapshot *sn; - int i, id, id_max = 0; + int i; + unsigned long id, id_max = 0; for(i = 0; i < s->nb_snapshots; i++) { sn = s->snapshots + i; @@ -300,7 +301,7 @@ static void find_new_snapshot_id(BlockDriverState *bs, if (id > id_max) id_max = id; } - snprintf(id_str, id_str_size, "%d", id_max + 1); + snprintf(id_str, id_str_size, "%lu", id_max + 1); } static int find_snapshot_by_id_and_name(BlockDriverState *bs, From 8f94a6e40e46cbc8e8014da825d25824b1803b34 Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Thu, 10 Oct 2013 11:45:55 +0200 Subject: [PATCH 25/61] block: Improve driver whitelist checks The main intent of this patch is to consolidate the whitelist checks to a single point in the code instead of spreading it everywhere. This adds a nicer error message for read-only whitelisting, too, in places where it was still missing. The patch also contains a bonus bug fix: By finding the format first in bdrv_open() and then independently checking against the whitelist only later, we avoid the case that use of a non-whitelisted format results in probing rather than an error message. Previously, this could happen when using the driver=... option. Signed-off-by: Kevin Wolf Reviewed-by: Fam Zheng --- block.c | 10 +++++++--- blockdev.c | 8 ++------ 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/block.c b/block.c index beea0274f8..84c0eac671 100644 --- a/block.c +++ b/block.c @@ -769,7 +769,11 @@ static int bdrv_open_common(BlockDriverState *bs, BlockDriverState *file, bs->read_only = !(open_flags & BDRV_O_RDWR); if (use_bdrv_whitelist && !bdrv_is_whitelisted(drv, bs->read_only)) { - error_setg(errp, "Driver '%s' is not whitelisted", drv->format_name); + error_setg(errp, + !bs->read_only && bdrv_is_whitelisted(drv, true) + ? "Driver '%s' can only be used for read-only devices" + : "Driver '%s' is not whitelisted", + drv->format_name); return -ENOTSUP; } @@ -881,7 +885,7 @@ int bdrv_file_open(BlockDriverState **pbs, const char *filename, /* Find the right block driver */ drvname = qdict_get_try_str(options, "driver"); if (drvname) { - drv = bdrv_find_whitelisted_format(drvname, !(flags & BDRV_O_RDWR)); + drv = bdrv_find_format(drvname); if (!drv) { error_setg(errp, "Unknown driver '%s'", drvname); } @@ -1123,7 +1127,7 @@ int bdrv_open(BlockDriverState *bs, const char *filename, QDict *options, /* Find the right image format driver */ drvname = qdict_get_try_str(options, "driver"); if (drvname) { - drv = bdrv_find_whitelisted_format(drvname, !(flags & BDRV_O_RDWR)); + drv = bdrv_find_format(drvname); qdict_del(options, "driver"); } diff --git a/blockdev.c b/blockdev.c index a91d5a87df..ab79df7bff 100644 --- a/blockdev.c +++ b/blockdev.c @@ -468,13 +468,9 @@ static DriveInfo *blockdev_init(QemuOpts *all_opts, return NULL; } - drv = bdrv_find_whitelisted_format(buf, ro); + drv = bdrv_find_format(buf); if (!drv) { - if (!ro && bdrv_find_whitelisted_format(buf, !ro)) { - error_report("'%s' can be only used as read-only device.", buf); - } else { - error_report("'%s' invalid format", buf); - } + error_report("'%s' invalid format", buf); return NULL; } } From 231bb267644ee3a9ebfd9c7f42d5d41610194b45 Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Thu, 10 Oct 2013 11:09:23 +0200 Subject: [PATCH 26/61] qcow2: Use negated overflow check mask In qcow2_check_metadata_overlap and qcow2_pre_write_overlap_check, change the parameter signifying the checks to perform from its current positive form to a negative one, i.e., it will no longer explicitly specify every check to perform but rather a mask of checks not to perform. Signed-off-by: Max Reitz Signed-off-by: Kevin Wolf --- block/qcow2-cache.c | 8 +++----- block/qcow2-cluster.c | 16 +++++++--------- block/qcow2-refcount.c | 22 ++++++++++------------ block/qcow2-snapshot.c | 12 +++++------- block/qcow2.c | 5 ++--- block/qcow2.h | 4 ++-- 6 files changed, 29 insertions(+), 38 deletions(-) diff --git a/block/qcow2-cache.c b/block/qcow2-cache.c index 40a5a3fc39..8ecbb5bc00 100644 --- a/block/qcow2-cache.c +++ b/block/qcow2-cache.c @@ -115,15 +115,13 @@ static int qcow2_cache_entry_flush(BlockDriverState *bs, Qcow2Cache *c, int i) } if (c == s->refcount_block_cache) { - ret = qcow2_pre_write_overlap_check(bs, - QCOW2_OL_DEFAULT & ~QCOW2_OL_REFCOUNT_BLOCK, + ret = qcow2_pre_write_overlap_check(bs, QCOW2_OL_REFCOUNT_BLOCK, c->entries[i].offset, s->cluster_size); } else if (c == s->l2_table_cache) { - ret = qcow2_pre_write_overlap_check(bs, - QCOW2_OL_DEFAULT & ~QCOW2_OL_ACTIVE_L2, + ret = qcow2_pre_write_overlap_check(bs, QCOW2_OL_ACTIVE_L2, c->entries[i].offset, s->cluster_size); } else { - ret = qcow2_pre_write_overlap_check(bs, QCOW2_OL_DEFAULT, + ret = qcow2_pre_write_overlap_check(bs, 0, c->entries[i].offset, s->cluster_size); } diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c index 0fd26bb4cc..0348b971b1 100644 --- a/block/qcow2-cluster.c +++ b/block/qcow2-cluster.c @@ -83,8 +83,8 @@ int qcow2_grow_l1_table(BlockDriverState *bs, uint64_t min_size, /* the L1 position has not yet been updated, so these clusters must * indeed be completely free */ - ret = qcow2_pre_write_overlap_check(bs, QCOW2_OL_DEFAULT, - new_l1_table_offset, new_l1_size2); + ret = qcow2_pre_write_overlap_check(bs, 0, new_l1_table_offset, + new_l1_size2); if (ret < 0) { goto fail; } @@ -160,8 +160,7 @@ int qcow2_write_l1_entry(BlockDriverState *bs, int l1_index) buf[i] = cpu_to_be64(s->l1_table[l1_start_index + i]); } - ret = qcow2_pre_write_overlap_check(bs, - QCOW2_OL_DEFAULT & ~QCOW2_OL_ACTIVE_L1, + ret = qcow2_pre_write_overlap_check(bs, QCOW2_OL_ACTIVE_L1, s->l1_table_offset + 8 * l1_start_index, sizeof(buf)); if (ret < 0) { return ret; @@ -396,7 +395,7 @@ static int coroutine_fn copy_sectors(BlockDriverState *bs, &s->aes_encrypt_key); } - ret = qcow2_pre_write_overlap_check(bs, QCOW2_OL_DEFAULT, + ret = qcow2_pre_write_overlap_check(bs, 0, cluster_offset + n_start * BDRV_SECTOR_SIZE, n * BDRV_SECTOR_SIZE); if (ret < 0) { goto out; @@ -1604,8 +1603,7 @@ static int expand_zero_clusters_in_l1(BlockDriverState *bs, uint64_t *l1_table, } } - ret = qcow2_pre_write_overlap_check(bs, QCOW2_OL_DEFAULT, - offset, s->cluster_size); + ret = qcow2_pre_write_overlap_check(bs, 0, offset, s->cluster_size); if (ret < 0) { if (!preallocated) { qcow2_free_clusters(bs, offset, s->cluster_size, @@ -1661,8 +1659,8 @@ static int expand_zero_clusters_in_l1(BlockDriverState *bs, uint64_t *l1_table, } } else { if (l2_dirty) { - ret = qcow2_pre_write_overlap_check(bs, QCOW2_OL_DEFAULT & - ~(QCOW2_OL_INACTIVE_L2 | QCOW2_OL_ACTIVE_L2), l2_offset, + ret = qcow2_pre_write_overlap_check(bs, + QCOW2_OL_INACTIVE_L2 | QCOW2_OL_ACTIVE_L2, l2_offset, s->cluster_size); if (ret < 0) { goto fail; diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c index 4ef689996f..988644a281 100644 --- a/block/qcow2-refcount.c +++ b/block/qcow2-refcount.c @@ -1311,9 +1311,8 @@ static int check_oflag_copied(BlockDriverState *bs, BdrvCheckResult *res, } if (l2_dirty) { - ret = qcow2_pre_write_overlap_check(bs, - QCOW2_OL_DEFAULT & ~QCOW2_OL_ACTIVE_L2, l2_offset, - s->cluster_size); + ret = qcow2_pre_write_overlap_check(bs, QCOW2_OL_ACTIVE_L2, + l2_offset, s->cluster_size); if (ret < 0) { fprintf(stderr, "ERROR: Could not write L2 table; metadata " "overlap check failed: %s\n", strerror(-ret)); @@ -1354,8 +1353,7 @@ static int write_reftable_entry(BlockDriverState *bs, int rt_index) buf[i] = cpu_to_be64(s->refcount_table[rt_start_index + i]); } - ret = qcow2_pre_write_overlap_check(bs, - QCOW2_OL_DEFAULT & ~QCOW2_OL_REFCOUNT_TABLE, + ret = qcow2_pre_write_overlap_check(bs, QCOW2_OL_REFCOUNT_TABLE, s->refcount_table_offset + rt_start_index * sizeof(uint64_t), sizeof(buf)); if (ret < 0) { @@ -1406,8 +1404,7 @@ static int64_t realloc_refcount_block(BlockDriverState *bs, int reftable_index, /* new block has not yet been entered into refcount table, therefore it is * no refcount block yet (regarding this check) */ - ret = qcow2_pre_write_overlap_check(bs, QCOW2_OL_DEFAULT, new_offset, - s->cluster_size); + ret = qcow2_pre_write_overlap_check(bs, 0, new_offset, s->cluster_size); if (ret < 0) { fprintf(stderr, "Could not write refcount block; metadata overlap " "check failed: %s\n", strerror(-ret)); @@ -1639,8 +1636,8 @@ fail: * looking for overlaps with important metadata sections (L1/L2 tables etc.), * i.e. a sanity check without relying on the refcount tables. * - * The chk parameter specifies exactly what checks to perform (being a bitmask - * of QCow2MetadataOverlap values). + * The ign parameter specifies what checks not to perform (being a bitmask of + * QCow2MetadataOverlap values), i.e., what sections to ignore. * * Returns: * - 0 if writing to this offset will not affect the mentioned metadata @@ -1648,10 +1645,11 @@ fail: * - a negative value (-errno) indicating an error while performing a check, * e.g. when bdrv_read failed on QCOW2_OL_INACTIVE_L2 */ -int qcow2_check_metadata_overlap(BlockDriverState *bs, int chk, int64_t offset, +int qcow2_check_metadata_overlap(BlockDriverState *bs, int ign, int64_t offset, int64_t size) { BDRVQcowState *s = bs->opaque; + int chk = QCOW2_OL_DEFAULT & ~ign; int i, j; if (!size) { @@ -1767,10 +1765,10 @@ static const char *metadata_ol_names[] = { * Returns 0 if there were neither overlaps nor errors while checking for * overlaps; or a negative value (-errno) on error. */ -int qcow2_pre_write_overlap_check(BlockDriverState *bs, int chk, int64_t offset, +int qcow2_pre_write_overlap_check(BlockDriverState *bs, int ign, int64_t offset, int64_t size) { - int ret = qcow2_check_metadata_overlap(bs, chk, offset, size); + int ret = qcow2_check_metadata_overlap(bs, ign, offset, size); if (ret < 0) { return ret; diff --git a/block/qcow2-snapshot.c b/block/qcow2-snapshot.c index 884b06d9fe..3529c683c6 100644 --- a/block/qcow2-snapshot.c +++ b/block/qcow2-snapshot.c @@ -192,8 +192,7 @@ static int qcow2_write_snapshots(BlockDriverState *bs) /* The snapshot list position has not yet been updated, so these clusters * must indeed be completely free */ - ret = qcow2_pre_write_overlap_check(bs, QCOW2_OL_DEFAULT, offset, - snapshots_size); + ret = qcow2_pre_write_overlap_check(bs, 0, offset, snapshots_size); if (ret < 0) { goto fail; } @@ -395,8 +394,8 @@ int qcow2_snapshot_create(BlockDriverState *bs, QEMUSnapshotInfo *sn_info) l1_table[i] = cpu_to_be64(s->l1_table[i]); } - ret = qcow2_pre_write_overlap_check(bs, QCOW2_OL_DEFAULT, - sn->l1_table_offset, s->l1_size * sizeof(uint64_t)); + ret = qcow2_pre_write_overlap_check(bs, 0, sn->l1_table_offset, + s->l1_size * sizeof(uint64_t)); if (ret < 0) { goto fail; } @@ -521,9 +520,8 @@ int qcow2_snapshot_goto(BlockDriverState *bs, const char *snapshot_id) goto fail; } - ret = qcow2_pre_write_overlap_check(bs, - QCOW2_OL_DEFAULT & ~QCOW2_OL_ACTIVE_L1, - s->l1_table_offset, cur_l1_bytes); + ret = qcow2_pre_write_overlap_check(bs, QCOW2_OL_ACTIVE_L1, + s->l1_table_offset, cur_l1_bytes); if (ret < 0) { goto fail; } diff --git a/block/qcow2.c b/block/qcow2.c index 3d1e74d38d..c461471ade 100644 --- a/block/qcow2.c +++ b/block/qcow2.c @@ -965,7 +965,7 @@ static coroutine_fn int qcow2_co_writev(BlockDriverState *bs, cur_nr_sectors * 512); } - ret = qcow2_pre_write_overlap_check(bs, QCOW2_OL_DEFAULT, + ret = qcow2_pre_write_overlap_check(bs, 0, cluster_offset + index_in_cluster * BDRV_SECTOR_SIZE, cur_nr_sectors * BDRV_SECTOR_SIZE); if (ret < 0) { @@ -1751,8 +1751,7 @@ static int qcow2_write_compressed(BlockDriverState *bs, int64_t sector_num, } cluster_offset &= s->cluster_offset_mask; - ret = qcow2_pre_write_overlap_check(bs, QCOW2_OL_DEFAULT, - cluster_offset, out_len); + ret = qcow2_pre_write_overlap_check(bs, 0, cluster_offset, out_len); if (ret < 0) { goto fail; } diff --git a/block/qcow2.h b/block/qcow2.h index 455e38de64..8692011347 100644 --- a/block/qcow2.h +++ b/block/qcow2.h @@ -433,9 +433,9 @@ int qcow2_check_refcounts(BlockDriverState *bs, BdrvCheckResult *res, void qcow2_process_discards(BlockDriverState *bs, int ret); -int qcow2_check_metadata_overlap(BlockDriverState *bs, int chk, int64_t offset, +int qcow2_check_metadata_overlap(BlockDriverState *bs, int ign, int64_t offset, int64_t size); -int qcow2_pre_write_overlap_check(BlockDriverState *bs, int chk, int64_t offset, +int qcow2_pre_write_overlap_check(BlockDriverState *bs, int ign, int64_t offset, int64_t size); /* qcow2-cluster.c functions */ From 3e3553905cfc814d59de6d1a634c3a991b2a9257 Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Thu, 10 Oct 2013 11:09:24 +0200 Subject: [PATCH 27/61] qcow2: Make overlap check mask variable Replace the QCOW2_OL_DEFAULT macro by a variable overlap_check in BDRVQcowState. Signed-off-by: Max Reitz Signed-off-by: Kevin Wolf --- block/qcow2-refcount.c | 2 +- block/qcow2.c | 2 ++ block/qcow2.h | 5 ++--- 3 files changed, 5 insertions(+), 4 deletions(-) diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c index 988644a281..1ff43d0906 100644 --- a/block/qcow2-refcount.c +++ b/block/qcow2-refcount.c @@ -1649,7 +1649,7 @@ int qcow2_check_metadata_overlap(BlockDriverState *bs, int ign, int64_t offset, int64_t size) { BDRVQcowState *s = bs->opaque; - int chk = QCOW2_OL_DEFAULT & ~ign; + int chk = s->overlap_check & ~ign; int i, j; if (!size) { diff --git a/block/qcow2.c b/block/qcow2.c index c461471ade..46acca75f0 100644 --- a/block/qcow2.c +++ b/block/qcow2.c @@ -631,6 +631,8 @@ static int qcow2_open(BlockDriverState *bs, QDict *options, int flags, s->discard_passthrough[QCOW2_DISCARD_OTHER] = qemu_opt_get_bool(opts, QCOW2_OPT_DISCARD_OTHER, false); + s->overlap_check = QCOW2_OL_CACHED; + qemu_opts_del(opts); if (s->use_lazy_refcounts && s->qcow_version < 3) { diff --git a/block/qcow2.h b/block/qcow2.h index 8692011347..6c85bb9395 100644 --- a/block/qcow2.h +++ b/block/qcow2.h @@ -203,6 +203,8 @@ typedef struct BDRVQcowState { bool discard_passthrough[QCOW2_DISCARD_MAX]; + int overlap_check; /* bitmask of Qcow2MetadataOverlap values */ + uint64_t incompatible_features; uint64_t compatible_features; uint64_t autoclear_features; @@ -321,9 +323,6 @@ typedef enum QCow2MetadataOverlap { QCOW2_OL_REFCOUNT_TABLE | QCOW2_OL_REFCOUNT_BLOCK | \ QCOW2_OL_SNAPSHOT_TABLE | QCOW2_OL_INACTIVE_L1) -/* The default checks to perform */ -#define QCOW2_OL_DEFAULT QCOW2_OL_CACHED - #define L1E_OFFSET_MASK 0x00ffffffffffff00ULL #define L2E_OFFSET_MASK 0x00ffffffffffff00ULL #define L2E_COMPRESSED_OFFSET_SIZE_MASK 0x3fffffffffffffffULL From 05de7e86cab3ed3830de38b38b39bbc711bc1158 Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Thu, 10 Oct 2013 11:09:25 +0200 Subject: [PATCH 28/61] qcow2: Add overlap-check options Add runtime options to tune the overlap checks to be performed before write accesses. Signed-off-by: Max Reitz Signed-off-by: Kevin Wolf --- block/qcow2.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++ block/qcow2.h | 9 +++++++++ 2 files changed, 55 insertions(+) diff --git a/block/qcow2.c b/block/qcow2.c index 46acca75f0..a517e3dad9 100644 --- a/block/qcow2.c +++ b/block/qcow2.c @@ -354,6 +354,52 @@ static QemuOptsList qcow2_runtime_opts = { .type = QEMU_OPT_BOOL, .help = "Generate discard requests when other clusters are freed", }, + { + .name = QCOW2_OPT_OVERLAP, + .type = QEMU_OPT_STRING, + .help = "Selects which overlap checks to perform from a range of " + "templates (none, constant, cached, all)", + }, + { + .name = QCOW2_OPT_OVERLAP_MAIN_HEADER, + .type = QEMU_OPT_BOOL, + .help = "Check for unintended writes into the main qcow2 header", + }, + { + .name = QCOW2_OPT_OVERLAP_ACTIVE_L1, + .type = QEMU_OPT_BOOL, + .help = "Check for unintended writes into the active L1 table", + }, + { + .name = QCOW2_OPT_OVERLAP_ACTIVE_L2, + .type = QEMU_OPT_BOOL, + .help = "Check for unintended writes into an active L2 table", + }, + { + .name = QCOW2_OPT_OVERLAP_REFCOUNT_TABLE, + .type = QEMU_OPT_BOOL, + .help = "Check for unintended writes into the refcount table", + }, + { + .name = QCOW2_OPT_OVERLAP_REFCOUNT_BLOCK, + .type = QEMU_OPT_BOOL, + .help = "Check for unintended writes into a refcount block", + }, + { + .name = QCOW2_OPT_OVERLAP_SNAPSHOT_TABLE, + .type = QEMU_OPT_BOOL, + .help = "Check for unintended writes into the snapshot table", + }, + { + .name = QCOW2_OPT_OVERLAP_INACTIVE_L1, + .type = QEMU_OPT_BOOL, + .help = "Check for unintended writes into an inactive L1 table", + }, + { + .name = QCOW2_OPT_OVERLAP_INACTIVE_L2, + .type = QEMU_OPT_BOOL, + .help = "Check for unintended writes into an inactive L2 table", + }, { /* end of list */ } }, }; diff --git a/block/qcow2.h b/block/qcow2.h index 6c85bb9395..28ccc4aa5f 100644 --- a/block/qcow2.h +++ b/block/qcow2.h @@ -63,6 +63,15 @@ #define QCOW2_OPT_DISCARD_REQUEST "pass-discard-request" #define QCOW2_OPT_DISCARD_SNAPSHOT "pass-discard-snapshot" #define QCOW2_OPT_DISCARD_OTHER "pass-discard-other" +#define QCOW2_OPT_OVERLAP "overlap-check" +#define QCOW2_OPT_OVERLAP_MAIN_HEADER "overlap-check.main-header" +#define QCOW2_OPT_OVERLAP_ACTIVE_L1 "overlap-check.active-l1" +#define QCOW2_OPT_OVERLAP_ACTIVE_L2 "overlap-check.active-l2" +#define QCOW2_OPT_OVERLAP_REFCOUNT_TABLE "overlap-check.refcount-table" +#define QCOW2_OPT_OVERLAP_REFCOUNT_BLOCK "overlap-check.refcount-block" +#define QCOW2_OPT_OVERLAP_SNAPSHOT_TABLE "overlap-check.snapshot-table" +#define QCOW2_OPT_OVERLAP_INACTIVE_L1 "overlap-check.inactive-l1" +#define QCOW2_OPT_OVERLAP_INACTIVE_L2 "overlap-check.inactive-l2" typedef struct QCowHeader { uint32_t magic; From 4092e99d935fe26fd53631cc9e170f9a19e3ee4a Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Thu, 10 Oct 2013 11:09:26 +0200 Subject: [PATCH 29/61] qcow2: Array assigning options to OL check bits Add an array which assigns the option string to its corresponding overlap check bit. Signed-off-by: Max Reitz Signed-off-by: Kevin Wolf --- block/qcow2.c | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/block/qcow2.c b/block/qcow2.c index a517e3dad9..eee7eaf7b3 100644 --- a/block/qcow2.c +++ b/block/qcow2.c @@ -404,6 +404,17 @@ static QemuOptsList qcow2_runtime_opts = { }, }; +static const char *overlap_bool_option_names[QCOW2_OL_MAX_BITNR] = { + [QCOW2_OL_MAIN_HEADER_BITNR] = QCOW2_OPT_OVERLAP_MAIN_HEADER, + [QCOW2_OL_ACTIVE_L1_BITNR] = QCOW2_OPT_OVERLAP_ACTIVE_L1, + [QCOW2_OL_ACTIVE_L2_BITNR] = QCOW2_OPT_OVERLAP_ACTIVE_L2, + [QCOW2_OL_REFCOUNT_TABLE_BITNR] = QCOW2_OPT_OVERLAP_REFCOUNT_TABLE, + [QCOW2_OL_REFCOUNT_BLOCK_BITNR] = QCOW2_OPT_OVERLAP_REFCOUNT_BLOCK, + [QCOW2_OL_SNAPSHOT_TABLE_BITNR] = QCOW2_OPT_OVERLAP_SNAPSHOT_TABLE, + [QCOW2_OL_INACTIVE_L1_BITNR] = QCOW2_OPT_OVERLAP_INACTIVE_L1, + [QCOW2_OL_INACTIVE_L2_BITNR] = QCOW2_OPT_OVERLAP_INACTIVE_L2, +}; + static int qcow2_open(BlockDriverState *bs, QDict *options, int flags, Error **errp) { From 4a273c398b0c96985d56fed8156e19876b2e3c9e Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Thu, 10 Oct 2013 11:09:27 +0200 Subject: [PATCH 30/61] qcow2: Add more overlap check bitmask macros Introduces the macros QCOW2_OL_CONSTANT and QCOW2_OL_ALL in addition to the already existing QCOW2_OL_CACHED, signifying all metadata overlap checks that can be performed in constant time (regardless of image size etc.) and truly all available overlap checks, respectively. Signed-off-by: Max Reitz Signed-off-by: Kevin Wolf --- block/qcow2.h | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/block/qcow2.h b/block/qcow2.h index 28ccc4aa5f..922e19062a 100644 --- a/block/qcow2.h +++ b/block/qcow2.h @@ -326,11 +326,19 @@ typedef enum QCow2MetadataOverlap { QCOW2_OL_INACTIVE_L2 = (1 << QCOW2_OL_INACTIVE_L2_BITNR), } QCow2MetadataOverlap; +/* Perform all overlap checks which can be done in constant time */ +#define QCOW2_OL_CONSTANT \ + (QCOW2_OL_MAIN_HEADER | QCOW2_OL_ACTIVE_L1 | QCOW2_OL_REFCOUNT_TABLE | \ + QCOW2_OL_SNAPSHOT_TABLE) + /* Perform all overlap checks which don't require disk access */ #define QCOW2_OL_CACHED \ - (QCOW2_OL_MAIN_HEADER | QCOW2_OL_ACTIVE_L1 | QCOW2_OL_ACTIVE_L2 | \ - QCOW2_OL_REFCOUNT_TABLE | QCOW2_OL_REFCOUNT_BLOCK | \ - QCOW2_OL_SNAPSHOT_TABLE | QCOW2_OL_INACTIVE_L1) + (QCOW2_OL_CONSTANT | QCOW2_OL_ACTIVE_L2 | QCOW2_OL_REFCOUNT_BLOCK | \ + QCOW2_OL_INACTIVE_L1) + +/* Perform all overlap checks */ +#define QCOW2_OL_ALL \ + (QCOW2_OL_CACHED | QCOW2_OL_INACTIVE_L2) #define L1E_OFFSET_MASK 0x00ffffffffffff00ULL #define L2E_OFFSET_MASK 0x00ffffffffffff00ULL From 1fa5cc839aa6068c9182ad8d611f844c58f95f42 Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Thu, 10 Oct 2013 11:09:28 +0200 Subject: [PATCH 31/61] qcow2: Evaluate overlap check options Evaluate the runtime overlap check options and set BDRVQcowState.overlap_check appropriately. Signed-off-by: Max Reitz Signed-off-by: Kevin Wolf --- block/qcow2.c | 29 ++++++++++++++++++++++++++++- 1 file changed, 28 insertions(+), 1 deletion(-) diff --git a/block/qcow2.c b/block/qcow2.c index eee7eaf7b3..c1abaffa19 100644 --- a/block/qcow2.c +++ b/block/qcow2.c @@ -425,6 +425,8 @@ static int qcow2_open(BlockDriverState *bs, QDict *options, int flags, Error *local_err = NULL; uint64_t ext_end; uint64_t l1_vm_state_index; + const char *opt_overlap_check; + int overlap_check_template = 0; ret = bdrv_pread(bs->file, 0, &header, sizeof(header)); if (ret < 0) { @@ -688,7 +690,32 @@ static int qcow2_open(BlockDriverState *bs, QDict *options, int flags, s->discard_passthrough[QCOW2_DISCARD_OTHER] = qemu_opt_get_bool(opts, QCOW2_OPT_DISCARD_OTHER, false); - s->overlap_check = QCOW2_OL_CACHED; + opt_overlap_check = qemu_opt_get(opts, "overlap-check") ?: "cached"; + if (!strcmp(opt_overlap_check, "none")) { + overlap_check_template = 0; + } else if (!strcmp(opt_overlap_check, "constant")) { + overlap_check_template = QCOW2_OL_CONSTANT; + } else if (!strcmp(opt_overlap_check, "cached")) { + overlap_check_template = QCOW2_OL_CACHED; + } else if (!strcmp(opt_overlap_check, "all")) { + overlap_check_template = QCOW2_OL_ALL; + } else { + error_setg(errp, "Unsupported value '%s' for qcow2 option " + "'overlap-check'. Allowed are either of the following: " + "none, constant, cached, all", opt_overlap_check); + qemu_opts_del(opts); + ret = -EINVAL; + goto fail; + } + + s->overlap_check = 0; + for (i = 0; i < QCOW2_OL_MAX_BITNR; i++) { + /* overlap-check defines a template bitmask, but every flag may be + * overwritten through the associated boolean option */ + s->overlap_check |= + qemu_opt_get_bool(opts, overlap_bool_option_names[i], + overlap_check_template & (1 << i)) << i; + } qemu_opts_del(opts); From 92f1deec317230575726a8e0ab5c110781d30ec0 Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Thu, 10 Oct 2013 15:44:00 +0200 Subject: [PATCH 32/61] block/raw_bsd: Employ error parameter Propagate errors in raw_create rather than directly reporting and afterwards discarding them. Signed-off-by: Max Reitz Signed-off-by: Kevin Wolf --- block/raw_bsd.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/block/raw_bsd.c b/block/raw_bsd.c index d61906bcc2..0078c1baeb 100644 --- a/block/raw_bsd.c +++ b/block/raw_bsd.c @@ -140,8 +140,7 @@ static int raw_create(const char *filename, QEMUOptionParameter *options, ret = bdrv_create_file(filename, options, &local_err); if (error_is_set(&local_err)) { - qerror_report_err(local_err); - error_free(local_err); + error_propagate(errp, local_err); } return ret; } From c6252b7cea0dfa893cf1f49de3a58f222e910783 Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Thu, 10 Oct 2013 15:44:02 +0200 Subject: [PATCH 33/61] block/raw-win32: Employ error parameter Make use of the error parameter in the opening and creating functions in block/raw-win32.c. Signed-off-by: Max Reitz Signed-off-by: Kevin Wolf --- block/raw-win32.c | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/block/raw-win32.c b/block/raw-win32.c index 6ef320f16a..c3e4c62d53 100644 --- a/block/raw-win32.c +++ b/block/raw-win32.c @@ -251,8 +251,7 @@ static int raw_open(BlockDriverState *bs, QDict *options, int flags, opts = qemu_opts_create_nofail(&raw_runtime_opts); qemu_opts_absorb_qdict(opts, options, &local_err); if (error_is_set(&local_err)) { - qerror_report_err(local_err); - error_free(local_err); + error_propagate(errp, local_err); ret = -EINVAL; goto fail; } @@ -264,6 +263,7 @@ static int raw_open(BlockDriverState *bs, QDict *options, int flags, if ((flags & BDRV_O_NATIVE_AIO) && aio == NULL) { aio = win32_aio_init(); if (aio == NULL) { + error_setg(errp, "Could not initialize AIO"); ret = -EINVAL; goto fail; } @@ -280,6 +280,7 @@ static int raw_open(BlockDriverState *bs, QDict *options, int flags, } else { ret = -EINVAL; } + error_setg_errno(errp, -ret, "Could not open file"); goto fail; } @@ -287,6 +288,7 @@ static int raw_open(BlockDriverState *bs, QDict *options, int flags, ret = win32_aio_attach(aio, s->hfile); if (ret < 0) { CloseHandle(s->hfile); + error_setg_errno(errp, -ret, "Could not enable AIO"); goto fail; } s->aio = aio; @@ -438,8 +440,10 @@ static int raw_create(const char *filename, QEMUOptionParameter *options, fd = qemu_open(filename, O_WRONLY | O_CREAT | O_TRUNC | O_BINARY, 0644); - if (fd < 0) + if (fd < 0) { + error_setg_errno(errp, errno, "Could not create file"); return -EIO; + } set_sparse(fd); ftruncate(fd, total_size * 512); qemu_close(fd); @@ -550,8 +554,7 @@ static int hdev_open(BlockDriverState *bs, QDict *options, int flags, QemuOpts *opts = qemu_opts_create_nofail(&raw_runtime_opts); qemu_opts_absorb_qdict(opts, options, &local_err); if (error_is_set(&local_err)) { - qerror_report_err(local_err); - error_free(local_err); + error_propagate(errp, local_err); ret = -EINVAL; goto done; } @@ -560,6 +563,7 @@ static int hdev_open(BlockDriverState *bs, QDict *options, int flags, if (strstart(filename, "/dev/cdrom", NULL)) { if (find_cdrom(device_name, sizeof(device_name)) < 0) { + error_setg(errp, "Could not open CD-ROM drive"); ret = -ENOENT; goto done; } @@ -586,8 +590,10 @@ static int hdev_open(BlockDriverState *bs, QDict *options, int flags, int err = GetLastError(); if (err == ERROR_ACCESS_DENIED) { + error_setg_errno(errp, EACCES, "Could not open device"); ret = -EACCES; } else { + error_setg(errp, "Could not open device"); ret = -1; } goto done; From 10ffa72faed7e02805d7911d58d429efe6f95f93 Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Thu, 10 Oct 2013 15:44:03 +0200 Subject: [PATCH 34/61] blkdebug: Employ error parameter Make use of the error parameter in blkdebug_open. Signed-off-by: Max Reitz Signed-off-by: Kevin Wolf --- block/blkdebug.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/block/blkdebug.c b/block/blkdebug.c index be948b2fdd..16d2b91ac9 100644 --- a/block/blkdebug.c +++ b/block/blkdebug.c @@ -362,8 +362,7 @@ static int blkdebug_open(BlockDriverState *bs, QDict *options, int flags, opts = qemu_opts_create_nofail(&runtime_opts); qemu_opts_absorb_qdict(opts, options, &local_err); if (error_is_set(&local_err)) { - qerror_report_err(local_err); - error_free(local_err); + error_propagate(errp, local_err); ret = -EINVAL; goto fail; } @@ -373,6 +372,7 @@ static int blkdebug_open(BlockDriverState *bs, QDict *options, int flags, if (config) { ret = read_config(s, config); if (ret < 0) { + error_setg_errno(errp, -ret, "Could not read blkdebug config file"); goto fail; } } @@ -383,14 +383,14 @@ static int blkdebug_open(BlockDriverState *bs, QDict *options, int flags, /* Open the backing file */ filename = qemu_opt_get(opts, "x-image"); if (filename == NULL) { + error_setg(errp, "Could not retrieve image file name"); ret = -EINVAL; goto fail; } ret = bdrv_file_open(&bs->file, filename, NULL, flags, &local_err); if (ret < 0) { - qerror_report_err(local_err); - error_free(local_err); + error_propagate(errp, local_err); goto fail; } From ca2884087a36c60d592aa0e8e327bf1579972077 Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Thu, 10 Oct 2013 15:44:04 +0200 Subject: [PATCH 35/61] blkverify: Employ error parameter Make use of the error parameter in blkverify_open. Signed-off-by: Max Reitz Signed-off-by: Kevin Wolf --- block/blkverify.c | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/block/blkverify.c b/block/blkverify.c index 1e8e6d9961..3c6352898f 100644 --- a/block/blkverify.c +++ b/block/blkverify.c @@ -128,8 +128,7 @@ static int blkverify_open(BlockDriverState *bs, QDict *options, int flags, opts = qemu_opts_create_nofail(&runtime_opts); qemu_opts_absorb_qdict(opts, options, &local_err); if (error_is_set(&local_err)) { - qerror_report_err(local_err); - error_free(local_err); + error_propagate(errp, local_err); ret = -EINVAL; goto fail; } @@ -137,20 +136,21 @@ static int blkverify_open(BlockDriverState *bs, QDict *options, int flags, /* Parse the raw image filename */ raw = qemu_opt_get(opts, "x-raw"); if (raw == NULL) { + error_setg(errp, "Could not retrieve raw image filename"); ret = -EINVAL; goto fail; } ret = bdrv_file_open(&bs->file, raw, NULL, flags, &local_err); if (ret < 0) { - qerror_report_err(local_err); - error_free(local_err); + error_propagate(errp, local_err); goto fail; } /* Open the test file */ filename = qemu_opt_get(opts, "x-image"); if (filename == NULL) { + error_setg(errp, "Could not retrieve test image filename"); ret = -EINVAL; goto fail; } @@ -158,8 +158,7 @@ static int blkverify_open(BlockDriverState *bs, QDict *options, int flags, s->test_file = bdrv_new(""); ret = bdrv_open(s->test_file, filename, NULL, flags, NULL, &local_err); if (ret < 0) { - qerror_report_err(local_err); - error_free(local_err); + error_propagate(errp, local_err); bdrv_unref(s->test_file); s->test_file = NULL; goto fail; From 5dd75f9afbea2e4e370c96676d34676e6f6b95b7 Mon Sep 17 00:00:00 2001 From: Fam Zheng Date: Fri, 11 Oct 2013 14:59:49 +0800 Subject: [PATCH 36/61] qemu-iotests: move blank lines of output in case 059 Move the blank line to above the test step banner, so it looks clearer in blocks. Signed-off-by: Fam Zheng Signed-off-by: Kevin Wolf --- tests/qemu-iotests/059 | 8 ++++---- tests/qemu-iotests/059.out | 8 ++++---- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/tests/qemu-iotests/059 b/tests/qemu-iotests/059 index dd6addfaab..18cdad1341 100755 --- a/tests/qemu-iotests/059 +++ b/tests/qemu-iotests/059 @@ -47,27 +47,27 @@ capacity_offset=16 granularity_offset=20 grain_table_size_offset=44 -echo "=== Testing invalid granularity ===" echo +echo "=== Testing invalid granularity ===" _make_test_img 64M poke_file "$TEST_IMG" "$granularity_offset" "\xff\xff\xff\xff\xff\xff\xff\xff" { $QEMU_IO -c "read 0 512" "$TEST_IMG"; } 2>&1 | _filter_qemu_io | _filter_testdir -echo "=== Testing too big L2 table size ===" echo +echo "=== Testing too big L2 table size ===" _make_test_img 64M poke_file "$TEST_IMG" "$grain_table_size_offset" "\xff\xff\xff\xff" { $QEMU_IO -c "read 0 512" "$TEST_IMG"; } 2>&1 | _filter_qemu_io | _filter_testdir -echo "=== Testing too big L1 table size ===" echo +echo "=== Testing too big L1 table size ===" _make_test_img 64M poke_file "$TEST_IMG" "$capacity_offset" "\xff\xff\xff\xff" poke_file "$TEST_IMG" "$grain_table_size_offset" "\x01\x00\x00\x00" { $QEMU_IO -c "read 0 512" "$TEST_IMG"; } 2>&1 | _filter_qemu_io | _filter_testdir -echo "=== Testing monolithicFlat creation and opening ===" echo +echo "=== Testing monolithicFlat creation and opening ===" IMGOPTS="subformat=monolithicFlat" _make_test_img 2G $QEMU_IMG info $TEST_IMG | _filter_testdir diff --git a/tests/qemu-iotests/059.out b/tests/qemu-iotests/059.out index 9159dbec60..21de6e7322 100644 --- a/tests/qemu-iotests/059.out +++ b/tests/qemu-iotests/059.out @@ -1,24 +1,24 @@ QA output created by 059 -=== Testing invalid granularity === +=== Testing invalid granularity === Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864 invalid granularity, image may be corrupt qemu-io: can't open device TEST_DIR/t.vmdk: Could not open 'TEST_DIR/t.vmdk': Wrong medium type no file open, try 'help open' -=== Testing too big L2 table size === +=== Testing too big L2 table size === Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864 L2 table size too big qemu-io: can't open device TEST_DIR/t.vmdk: Could not open 'TEST_DIR/t.vmdk': Wrong medium type no file open, try 'help open' -=== Testing too big L1 table size === +=== Testing too big L1 table size === Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864 L1 size too big qemu-io: can't open device TEST_DIR/t.vmdk: Could not open 'TEST_DIR/t.vmdk': Wrong medium type no file open, try 'help open' -=== Testing monolithicFlat creation and opening === +=== Testing monolithicFlat creation and opening === Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=2147483648 image: TEST_DIR/t.vmdk file format: vmdk From e428e439df4d92ac42cb913a1dd19b86155eae86 Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Fri, 11 Oct 2013 11:37:01 +0200 Subject: [PATCH 37/61] block/raw-posix: Employ error parameter Make use of the error parameter in the opening and creating functions in block/raw-posix.c. Signed-off-by: Max Reitz Signed-off-by: Kevin Wolf --- block/raw-posix.c | 74 +++++++++++++++++++++++++++++--------- tests/qemu-iotests/051.out | 2 +- 2 files changed, 59 insertions(+), 17 deletions(-) diff --git a/block/raw-posix.c b/block/raw-posix.c index f7f102d2e2..6f03fbf793 100644 --- a/block/raw-posix.c +++ b/block/raw-posix.c @@ -276,7 +276,7 @@ static QemuOptsList raw_runtime_opts = { }; static int raw_open_common(BlockDriverState *bs, QDict *options, - int bdrv_flags, int open_flags) + int bdrv_flags, int open_flags, Error **errp) { BDRVRawState *s = bs->opaque; QemuOpts *opts; @@ -287,8 +287,7 @@ static int raw_open_common(BlockDriverState *bs, QDict *options, opts = qemu_opts_create_nofail(&raw_runtime_opts); qemu_opts_absorb_qdict(opts, options, &local_err); if (error_is_set(&local_err)) { - qerror_report_err(local_err); - error_free(local_err); + error_propagate(errp, local_err); ret = -EINVAL; goto fail; } @@ -297,6 +296,7 @@ static int raw_open_common(BlockDriverState *bs, QDict *options, ret = raw_normalize_devicepath(&filename); if (ret != 0) { + error_setg_errno(errp, -ret, "Could not normalize device path"); goto fail; } @@ -310,6 +310,7 @@ static int raw_open_common(BlockDriverState *bs, QDict *options, if (ret == -EROFS) { ret = -EACCES; } + error_setg_errno(errp, -ret, "Could not open file"); goto fail; } s->fd = fd; @@ -318,6 +319,7 @@ static int raw_open_common(BlockDriverState *bs, QDict *options, if (raw_set_aio(&s->aio_ctx, &s->use_aio, bdrv_flags)) { qemu_close(fd); ret = -errno; + error_setg_errno(errp, -ret, "Could not set AIO state"); goto fail; } #endif @@ -339,9 +341,15 @@ static int raw_open(BlockDriverState *bs, QDict *options, int flags, Error **errp) { BDRVRawState *s = bs->opaque; + Error *local_err = NULL; + int ret; s->type = FTYPE_FILE; - return raw_open_common(bs, options, flags, 0); + ret = raw_open_common(bs, options, flags, 0, &local_err); + if (error_is_set(&local_err)) { + error_propagate(errp, local_err); + } + return ret; } static int raw_reopen_prepare(BDRVReopenState *state, @@ -366,6 +374,7 @@ static int raw_reopen_prepare(BDRVReopenState *state, * valid in the 'false' condition even if aio_ctx is set, and raw_set_aio() * won't override aio_ctx if aio_ctx is non-NULL */ if (raw_set_aio(&s->aio_ctx, &raw_s->use_aio, state->flags)) { + error_setg(errp, "Could not set AIO state"); return -1; } #endif @@ -417,6 +426,7 @@ static int raw_reopen_prepare(BDRVReopenState *state, assert(!(raw_s->open_flags & O_CREAT)); raw_s->fd = qemu_open(state->bs->filename, raw_s->open_flags); if (raw_s->fd == -1) { + error_setg_errno(errp, errno, "Could not reopen file"); ret = -1; } } @@ -1060,12 +1070,15 @@ static int raw_create(const char *filename, QEMUOptionParameter *options, 0644); if (fd < 0) { result = -errno; + error_setg_errno(errp, -result, "Could not create file"); } else { if (ftruncate(fd, total_size * BDRV_SECTOR_SIZE) != 0) { result = -errno; + error_setg_errno(errp, -result, "Could not resize file"); } if (qemu_close(fd) != 0) { result = -errno; + error_setg_errno(errp, -result, "Could not close the new file"); } } return result; @@ -1338,6 +1351,7 @@ static int hdev_open(BlockDriverState *bs, QDict *options, int flags, Error **errp) { BDRVRawState *s = bs->opaque; + Error *local_err = NULL; int ret; const char *filename = qdict_get_str(options, "filename"); @@ -1381,8 +1395,11 @@ static int hdev_open(BlockDriverState *bs, QDict *options, int flags, } #endif - ret = raw_open_common(bs, options, flags, 0); + ret = raw_open_common(bs, options, flags, 0, &local_err); if (ret < 0) { + if (error_is_set(&local_err)) { + error_propagate(errp, local_err); + } return ret; } @@ -1390,6 +1407,7 @@ static int hdev_open(BlockDriverState *bs, QDict *options, int flags, ret = check_hdev_writable(s); if (ret < 0) { raw_close(bs); + error_setg_errno(errp, -ret, "The device is not writable"); return ret; } } @@ -1525,15 +1543,23 @@ static int hdev_create(const char *filename, QEMUOptionParameter *options, } fd = qemu_open(filename, O_WRONLY | O_BINARY); - if (fd < 0) - return -errno; - - if (fstat(fd, &stat_buf) < 0) + if (fd < 0) { ret = -errno; - else if (!S_ISBLK(stat_buf.st_mode) && !S_ISCHR(stat_buf.st_mode)) + error_setg_errno(errp, -ret, "Could not open device"); + return ret; + } + + if (fstat(fd, &stat_buf) < 0) { + ret = -errno; + error_setg_errno(errp, -ret, "Could not stat device"); + } else if (!S_ISBLK(stat_buf.st_mode) && !S_ISCHR(stat_buf.st_mode)) { + error_setg(errp, + "The given file is neither a block nor a character device"); ret = -ENODEV; - else if (lseek(fd, 0, SEEK_END) < total_size * BDRV_SECTOR_SIZE) + } else if (lseek(fd, 0, SEEK_END) < total_size * BDRV_SECTOR_SIZE) { + error_setg(errp, "Device is too small"); ret = -ENOSPC; + } qemu_close(fd); return ret; @@ -1575,14 +1601,19 @@ static int floppy_open(BlockDriverState *bs, QDict *options, int flags, Error **errp) { BDRVRawState *s = bs->opaque; + Error *local_err = NULL; int ret; s->type = FTYPE_FD; /* open will not fail even if no floppy is inserted, so add O_NONBLOCK */ - ret = raw_open_common(bs, options, flags, O_NONBLOCK); - if (ret) + ret = raw_open_common(bs, options, flags, O_NONBLOCK, &local_err); + if (ret) { + if (error_is_set(&local_err)) { + error_propagate(errp, local_err); + } return ret; + } /* close fd so that we can reopen it as needed */ qemu_close(s->fd); @@ -1698,11 +1729,17 @@ static int cdrom_open(BlockDriverState *bs, QDict *options, int flags, Error **errp) { BDRVRawState *s = bs->opaque; + Error *local_err = NULL; + int ret; s->type = FTYPE_CD; /* open will not fail even if no CD is inserted, so add O_NONBLOCK */ - return raw_open_common(bs, options, flags, O_NONBLOCK); + ret = raw_open_common(bs, options, flags, O_NONBLOCK, &local_err); + if (error_is_set(&local_err)) { + error_propagate(errp, local_err); + } + return ret; } static int cdrom_probe_device(const char *filename) @@ -1806,13 +1843,18 @@ static BlockDriver bdrv_host_cdrom = { static int cdrom_open(BlockDriverState *bs, QDict *options, int flags) { BDRVRawState *s = bs->opaque; + Error *local_err = NULL; int ret; s->type = FTYPE_CD; - ret = raw_open_common(bs, options, flags, 0); - if (ret) + ret = raw_open_common(bs, options, flags, 0, &local_err); + if (ret) { + if (error_is_set(&local_err)) { + error_propagate(errp, local_err); + } return ret; + } /* make sure the door isn't locked at this time */ ioctl(s->fd, CDIOCALLOW); diff --git a/tests/qemu-iotests/051.out b/tests/qemu-iotests/051.out index 04bb236655..e58776a046 100644 --- a/tests/qemu-iotests/051.out +++ b/tests/qemu-iotests/051.out @@ -223,6 +223,6 @@ Testing: -drive file=foo:bar QEMU_PROG: -drive file=foo:bar: could not open disk image foo:bar: Unknown protocol Testing: -drive file.filename=foo:bar -QEMU_PROG: -drive file.filename=foo:bar: could not open disk image ide0-hd0: Could not open 'foo:bar': No such file or directory +QEMU_PROG: -drive file.filename=foo:bar: could not open disk image ide0-hd0: Could not open file: No such file or directory *** done From 22ee5a557acc820109a9948620a26f66e4fa3a8f Mon Sep 17 00:00:00 2001 From: Wenchao Xia Date: Thu, 26 Sep 2013 08:42:55 +0800 Subject: [PATCH 38/61] tests: build the helper program by default Usually we may configure and make, then goto ./tests/qemu-iotest, check. In this case an error will happen since helper program was not built. This patch simply build it by default. A better way may be introducing Makefile in ./tests/qemu-iotest, but it is more complicate to handle out of tree case, and a bit overkill for a single file now, we can do that when more files come. Signed-off-by: Wenchao Xia Reviewed-by: Fam Zheng Signed-off-by: Kevin Wolf --- tests/Makefile | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/tests/Makefile b/tests/Makefile index 915ae5e2d1..6d67fdf900 100644 --- a/tests/Makefile +++ b/tests/Makefile @@ -252,8 +252,10 @@ check-report.html: check-report.xml # Other tests +QEMU_IOTESTS_HELPERS-$(CONFIG_LINUX) = tests/qemu-iotests/socket_scm_helper$(EXESUF) + .PHONY: check-tests/qemu-iotests-quick.sh -check-tests/qemu-iotests-quick.sh: tests/qemu-iotests-quick.sh qemu-img$(EXESUF) qemu-io$(EXESUF) tests/qemu-iotests/socket_scm_helper$(EXESUF) +check-tests/qemu-iotests-quick.sh: tests/qemu-iotests-quick.sh qemu-img$(EXESUF) qemu-io$(EXESUF) $(QEMU_IOTESTS_HELPERS-y) $< .PHONY: check-tests/test-qapi.py @@ -275,5 +277,9 @@ check-unit: $(patsubst %,check-%, $(check-unit-y)) check-block: $(patsubst %,check-%, $(check-block-y)) check: check-qapi-schema check-unit check-qtest +# Build the help program automatically + +all: $(QEMU_IOTESTS_HELPERS-y) + -include $(wildcard tests/*.d) -include $(wildcard tests/libqos/*.d) From 2cdfb12332e885c8ce36f520d2a2a9200101e183 Mon Sep 17 00:00:00 2001 From: Wenchao Xia Date: Thu, 26 Sep 2013 08:42:56 +0800 Subject: [PATCH 39/61] build: add command check-clean This command will package the clean operations in tests. Now root Makefile simply calls the command and do not care the details of it any more. Original the built binaries for test will not be removed, now they will be deleted in clean operation. Signed-off-by: Wenchao Xia Reviewed-by: Fam Zheng Signed-off-by: Kevin Wolf --- Makefile | 1 - tests/Makefile | 8 +++++++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/Makefile b/Makefile index 60fb87e2dd..b15003f9d2 100644 --- a/Makefile +++ b/Makefile @@ -246,7 +246,6 @@ clean: rm -f $(foreach f,$(GENERATED_SOURCES),$(f) $(f)-timestamp) rm -rf qapi-generated rm -rf qga/qapi-generated - $(MAKE) -C tests/tcg clean for d in $(ALL_SUBDIRS); do \ if test -d $$d; then $(MAKE) -C $$d $@ || exit 1; fi; \ rm -f $$d/qemu-options.def; \ diff --git a/tests/Makefile b/tests/Makefile index 6d67fdf900..fa4c9f0cbb 100644 --- a/tests/Makefile +++ b/tests/Makefile @@ -196,6 +196,7 @@ check-help: @echo " make check-qapi-schema Run QAPI schema tests" @echo " make check-block Run block tests" @echo " make check-report.html Generates an HTML test report" + @echo " make check-clean Clean the tests" @echo @echo "Please note that HTML reports do not regenerate if the unit tests" @echo "has not changed." @@ -270,12 +271,17 @@ $(patsubst %, check-%, $(check-qapi-schema-y)): check-%.json: $(SRC_PATH)/%.json # Consolidated targets -.PHONY: check-qapi-schema check-qtest check-unit check +.PHONY: check-qapi-schema check-qtest check-unit check check-clean check-qapi-schema: $(patsubst %,check-%, $(check-qapi-schema-y)) check-qtest: $(patsubst %,check-qtest-%, $(QTEST_TARGETS)) check-unit: $(patsubst %,check-%, $(check-unit-y)) check-block: $(patsubst %,check-%, $(check-block-y)) check: check-qapi-schema check-unit check-qtest +check-clean: + $(MAKE) -C tests/tcg clean + rm -rf $(check-unit-y) $(check-qtest-i386-y) $(check-qtest-x86_64-y) $(check-qtest-sparc64-y) $(check-qtest-sparc-y) tests/*.o $(QEMU_IOTESTS_HELPERS-y) + +clean: check-clean # Build the help program automatically From 4823970bcb882cd5b7e9c9a21fa6573190035050 Mon Sep 17 00:00:00 2001 From: Fam Zheng Date: Fri, 11 Oct 2013 15:43:22 +0800 Subject: [PATCH 40/61] vmdk: convert error code to use errp Convert "fprintf(stderr,..." and standardize error messages: Remove a few local_error's and use errp. Remove "VMDK:" or "Vmdk:" prefixes in error message and fix to upper case. Signed-off-by: Fam Zheng Signed-off-by: Kevin Wolf --- block/vmdk.c | 116 +++++++++++++++++++------------------ tests/qemu-iotests/059.out | 6 +- 2 files changed, 63 insertions(+), 59 deletions(-) diff --git a/block/vmdk.c b/block/vmdk.c index 5d56e316f1..a1aaea77e9 100644 --- a/block/vmdk.c +++ b/block/vmdk.c @@ -331,8 +331,7 @@ static int vmdk_reopen_prepare(BDRVReopenState *state, assert(state->bs != NULL); if (queue == NULL) { - error_set(errp, ERROR_CLASS_GENERIC_ERROR, - "No reopen queue for VMDK extents"); + error_setg(errp, "No reopen queue for VMDK extents"); goto exit; } @@ -391,22 +390,23 @@ static int vmdk_add_extent(BlockDriverState *bs, int64_t l1_offset, int64_t l1_backup_offset, uint32_t l1_size, int l2_size, uint64_t cluster_sectors, - VmdkExtent **new_extent) + VmdkExtent **new_extent, + Error **errp) { VmdkExtent *extent; BDRVVmdkState *s = bs->opaque; if (cluster_sectors > 0x200000) { /* 0x200000 * 512Bytes = 1GB for one cluster is unrealistic */ - error_report("invalid granularity, image may be corrupt"); - return -EINVAL; + error_setg(errp, "Invalid granularity, image may be corrupt"); + return -EFBIG; } if (l1_size > 512 * 1024 * 1024) { /* Although with big capacity and small l1_entry_sectors, we can get a * big l1_size, we don't want unbounded value to allocate the table. * Limit it to 512M, which is 16PB for default cluster and L2 table * size */ - error_report("L1 size too big"); + error_setg(errp, "L1 size too big"); return -EFBIG; } @@ -438,7 +438,8 @@ static int vmdk_add_extent(BlockDriverState *bs, return 0; } -static int vmdk_init_tables(BlockDriverState *bs, VmdkExtent *extent) +static int vmdk_init_tables(BlockDriverState *bs, VmdkExtent *extent, + Error **errp) { int ret; int l1_size, i; @@ -447,10 +448,13 @@ static int vmdk_init_tables(BlockDriverState *bs, VmdkExtent *extent) l1_size = extent->l1_size * sizeof(uint32_t); extent->l1_table = g_malloc(l1_size); ret = bdrv_pread(extent->file, - extent->l1_table_offset, - extent->l1_table, - l1_size); + extent->l1_table_offset, + extent->l1_table, + l1_size); if (ret < 0) { + error_setg_errno(errp, -ret, + "Could not read l1 table from extent '%s'", + extent->file->filename); goto fail_l1; } for (i = 0; i < extent->l1_size; i++) { @@ -460,10 +464,13 @@ static int vmdk_init_tables(BlockDriverState *bs, VmdkExtent *extent) if (extent->l1_backup_table_offset) { extent->l1_backup_table = g_malloc(l1_size); ret = bdrv_pread(extent->file, - extent->l1_backup_table_offset, - extent->l1_backup_table, - l1_size); + extent->l1_backup_table_offset, + extent->l1_backup_table, + l1_size); if (ret < 0) { + error_setg_errno(errp, -ret, + "Could not read l1 backup table from extent '%s'", + extent->file->filename); goto fail_l1b; } for (i = 0; i < extent->l1_size; i++) { @@ -483,7 +490,7 @@ static int vmdk_init_tables(BlockDriverState *bs, VmdkExtent *extent) static int vmdk_open_vmfs_sparse(BlockDriverState *bs, BlockDriverState *file, - int flags) + int flags, Error **errp) { int ret; uint32_t magic; @@ -492,6 +499,9 @@ static int vmdk_open_vmfs_sparse(BlockDriverState *bs, ret = bdrv_pread(file, sizeof(magic), &header, sizeof(header)); if (ret < 0) { + error_setg_errno(errp, -ret, + "Could not read header from file '%s'", + file->filename); return ret; } ret = vmdk_add_extent(bs, file, false, @@ -501,11 +511,12 @@ static int vmdk_open_vmfs_sparse(BlockDriverState *bs, le32_to_cpu(header.l1dir_size), 4096, le32_to_cpu(header.granularity), - &extent); + &extent, + errp); if (ret < 0) { return ret; } - ret = vmdk_init_tables(bs, extent); + ret = vmdk_init_tables(bs, extent, errp); if (ret) { /* free extent allocated by vmdk_add_extent */ vmdk_free_last_extent(bs); @@ -514,11 +525,11 @@ static int vmdk_open_vmfs_sparse(BlockDriverState *bs, } static int vmdk_open_desc_file(BlockDriverState *bs, int flags, - uint64_t desc_offset); + uint64_t desc_offset, Error **errp); static int vmdk_open_vmdk4(BlockDriverState *bs, BlockDriverState *file, - int flags) + int flags, Error **errp) { int ret; uint32_t magic; @@ -529,12 +540,14 @@ static int vmdk_open_vmdk4(BlockDriverState *bs, ret = bdrv_pread(file, sizeof(magic), &header, sizeof(header)); if (ret < 0) { - return ret; + error_setg_errno(errp, -ret, + "Could not read header from file '%s'", + file->filename); } if (header.capacity == 0) { uint64_t desc_offset = le64_to_cpu(header.desc_offset); if (desc_offset) { - return vmdk_open_desc_file(bs, flags, desc_offset << 9); + return vmdk_open_desc_file(bs, flags, desc_offset << 9, errp); } } @@ -616,7 +629,8 @@ static int vmdk_open_vmdk4(BlockDriverState *bs, l1_size, le32_to_cpu(header.num_gtes_per_gt), le64_to_cpu(header.granularity), - &extent); + &extent, + errp); if (ret < 0) { return ret; } @@ -625,7 +639,7 @@ static int vmdk_open_vmdk4(BlockDriverState *bs, extent->has_marker = le32_to_cpu(header.flags) & VMDK4_FLAG_MARKER; extent->version = le32_to_cpu(header.version); extent->has_zero_grain = le32_to_cpu(header.flags) & VMDK4_FLAG_ZERO_GRAIN; - ret = vmdk_init_tables(bs, extent); + ret = vmdk_init_tables(bs, extent, errp); if (ret) { /* free extent allocated by vmdk_add_extent */ vmdk_free_last_extent(bs); @@ -663,7 +677,7 @@ static int vmdk_parse_description(const char *desc, const char *opt_name, /* Open an extent file and append to bs array */ static int vmdk_open_sparse(BlockDriverState *bs, BlockDriverState *file, - int flags) + int flags, Error **errp) { uint32_t magic; @@ -674,10 +688,10 @@ static int vmdk_open_sparse(BlockDriverState *bs, magic = be32_to_cpu(magic); switch (magic) { case VMDK3_MAGIC: - return vmdk_open_vmfs_sparse(bs, file, flags); + return vmdk_open_vmfs_sparse(bs, file, flags, errp); break; case VMDK4_MAGIC: - return vmdk_open_vmdk4(bs, file, flags); + return vmdk_open_vmdk4(bs, file, flags, errp); break; default: return -EMEDIUMTYPE; @@ -686,7 +700,7 @@ static int vmdk_open_sparse(BlockDriverState *bs, } static int vmdk_parse_extents(const char *desc, BlockDriverState *bs, - const char *desc_file_path) + const char *desc_file_path, Error **errp) { int ret; char access[11]; @@ -697,7 +711,6 @@ static int vmdk_parse_extents(const char *desc, BlockDriverState *bs, int64_t flat_offset; char extent_path[PATH_MAX]; BlockDriverState *extent_file; - Error *local_err = NULL; while (*p) { /* parse extent line: @@ -712,9 +725,11 @@ static int vmdk_parse_extents(const char *desc, BlockDriverState *bs, goto next_line; } else if (!strcmp(type, "FLAT")) { if (ret != 5 || flat_offset < 0) { + error_setg(errp, "Invalid extent lines: \n%s", p); return -EINVAL; } } else if (ret != 4) { + error_setg(errp, "Invalid extent lines: \n%s", p); return -EINVAL; } @@ -728,10 +743,8 @@ static int vmdk_parse_extents(const char *desc, BlockDriverState *bs, path_combine(extent_path, sizeof(extent_path), desc_file_path, fname); ret = bdrv_file_open(&extent_file, extent_path, NULL, bs->open_flags, - &local_err); + errp); if (ret) { - qerror_report_err(local_err); - error_free(local_err); return ret; } @@ -741,21 +754,20 @@ static int vmdk_parse_extents(const char *desc, BlockDriverState *bs, VmdkExtent *extent; ret = vmdk_add_extent(bs, extent_file, true, sectors, - 0, 0, 0, 0, 0, &extent); + 0, 0, 0, 0, 0, &extent, errp); if (ret < 0) { return ret; } extent->flat_start_offset = flat_offset << 9; } else if (!strcmp(type, "SPARSE") || !strcmp(type, "VMFSSPARSE")) { /* SPARSE extent and VMFSSPARSE extent are both "COWD" sparse file*/ - ret = vmdk_open_sparse(bs, extent_file, bs->open_flags); + ret = vmdk_open_sparse(bs, extent_file, bs->open_flags, errp); if (ret) { bdrv_unref(extent_file); return ret; } } else { - fprintf(stderr, - "VMDK: Not supported extent type \"%s\""".\n", type); + error_setg(errp, "Unsupported extent type '%s'", type); return -ENOTSUP; } next_line: @@ -769,7 +781,7 @@ next_line: } static int vmdk_open_desc_file(BlockDriverState *bs, int flags, - uint64_t desc_offset) + uint64_t desc_offset, Error **errp) { int ret; char *buf = NULL; @@ -798,13 +810,12 @@ static int vmdk_open_desc_file(BlockDriverState *bs, int flags, strcmp(ct, "vmfsSparse") && strcmp(ct, "twoGbMaxExtentSparse") && strcmp(ct, "twoGbMaxExtentFlat")) { - fprintf(stderr, - "VMDK: Not supported image type \"%s\""".\n", ct); + error_setg(errp, "Unsupported image type '%s'", ct); ret = -ENOTSUP; goto exit; } s->desc_offset = 0; - ret = vmdk_parse_extents(buf, bs, bs->file->filename); + ret = vmdk_parse_extents(buf, bs, bs->file->filename, errp); exit: g_free(buf); return ret; @@ -816,10 +827,10 @@ static int vmdk_open(BlockDriverState *bs, QDict *options, int flags, int ret; BDRVVmdkState *s = bs->opaque; - if (vmdk_open_sparse(bs, bs->file, flags) == 0) { + if (vmdk_open_sparse(bs, bs->file, flags, errp) == 0) { s->desc_offset = 0x200; } else { - ret = vmdk_open_desc_file(bs, flags, 0); + ret = vmdk_open_desc_file(bs, flags, 0, errp); if (ret) { goto fail; } @@ -1286,8 +1297,7 @@ static int vmdk_write(BlockDriverState *bs, int64_t sector_num, VmdkMetaData m_data; if (sector_num > bs->total_sectors) { - fprintf(stderr, - "(VMDK) Wrong offset: sector_num=0x%" PRIx64 + error_report("Wrong offset: sector_num=0x%" PRIx64 " total_sectors=0x%" PRIx64 "\n", sector_num, bs->total_sectors); return -EIO; @@ -1307,9 +1317,8 @@ static int vmdk_write(BlockDriverState *bs, int64_t sector_num, if (extent->compressed) { if (ret == VMDK_OK) { /* Refuse write to allocated cluster for streamOptimized */ - fprintf(stderr, - "VMDK: can't write to allocated cluster" - " for streamOptimized\n"); + error_report("Could not write to allocated cluster" + " for streamOptimized"); return -EIO; } else { /* allocate */ @@ -1517,12 +1526,12 @@ static int vmdk_create_extent(const char *filename, int64_t filesize, } static int filename_decompose(const char *filename, char *path, char *prefix, - char *postfix, size_t buf_len) + char *postfix, size_t buf_len, Error **errp) { const char *p, *q; if (filename == NULL || !strlen(filename)) { - fprintf(stderr, "Vmdk: no filename provided.\n"); + error_setg(errp, "No filename provided"); return VMDK_ERROR; } p = strrchr(filename, '/'); @@ -1595,9 +1604,8 @@ static int vmdk_create(const char *filename, QEMUOptionParameter *options, "ddb.geometry.heads = \"%d\"\n" "ddb.geometry.sectors = \"63\"\n" "ddb.adapterType = \"%s\"\n"; - Error *local_err = NULL; - if (filename_decompose(filename, path, prefix, postfix, PATH_MAX)) { + if (filename_decompose(filename, path, prefix, postfix, PATH_MAX, errp)) { return -EINVAL; } /* Read out options */ @@ -1623,7 +1631,7 @@ static int vmdk_create(const char *filename, QEMUOptionParameter *options, strcmp(adapter_type, "buslogic") && strcmp(adapter_type, "lsilogic") && strcmp(adapter_type, "legacyESX")) { - fprintf(stderr, "VMDK: Unknown adapter type: '%s'.\n", adapter_type); + error_setg(errp, "Unknown adapter type: '%s'", adapter_type); return -EINVAL; } if (strcmp(adapter_type, "ide") != 0) { @@ -1639,7 +1647,7 @@ static int vmdk_create(const char *filename, QEMUOptionParameter *options, strcmp(fmt, "twoGbMaxExtentSparse") && strcmp(fmt, "twoGbMaxExtentFlat") && strcmp(fmt, "streamOptimized")) { - fprintf(stderr, "VMDK: Unknown subformat: %s\n", fmt); + error_setg(errp, "Unknown subformat: '%s'", fmt); return -EINVAL; } split = !(strcmp(fmt, "twoGbMaxExtentFlat") && @@ -1653,15 +1661,13 @@ static int vmdk_create(const char *filename, QEMUOptionParameter *options, desc_extent_line = "RW %lld SPARSE \"%s\"\n"; } if (flat && backing_file) { - /* not supporting backing file for flat image */ + error_setg(errp, "Flat image can't have backing file"); return -ENOTSUP; } if (backing_file) { BlockDriverState *bs = bdrv_new(""); - ret = bdrv_open(bs, backing_file, NULL, 0, NULL, &local_err); + ret = bdrv_open(bs, backing_file, NULL, 0, NULL, errp); if (ret != 0) { - qerror_report_err(local_err); - error_free(local_err); bdrv_unref(bs); return ret; } diff --git a/tests/qemu-iotests/059.out b/tests/qemu-iotests/059.out index 21de6e7322..265cd76169 100644 --- a/tests/qemu-iotests/059.out +++ b/tests/qemu-iotests/059.out @@ -2,8 +2,7 @@ QA output created by 059 === Testing invalid granularity === Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864 -invalid granularity, image may be corrupt -qemu-io: can't open device TEST_DIR/t.vmdk: Could not open 'TEST_DIR/t.vmdk': Wrong medium type +qemu-io: can't open device TEST_DIR/t.vmdk: Invalid granularity, image may be corrupt no file open, try 'help open' === Testing too big L2 table size === @@ -14,8 +13,7 @@ no file open, try 'help open' === Testing too big L1 table size === Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864 -L1 size too big -qemu-io: can't open device TEST_DIR/t.vmdk: Could not open 'TEST_DIR/t.vmdk': Wrong medium type +qemu-io: can't open device TEST_DIR/t.vmdk: L1 size too big no file open, try 'help open' === Testing monolithicFlat creation and opening === From 52c8d629cac27ad16dd51507b4733d46fa4efc55 Mon Sep 17 00:00:00 2001 From: Fam Zheng Date: Fri, 11 Oct 2013 15:43:23 +0800 Subject: [PATCH 41/61] vmdk: refuse enabling zeroed grain with flat images This is a header flag and we needs sparse for the header. Signed-off-by: Fam Zheng Signed-off-by: Kevin Wolf --- block/vmdk.c | 4 ++++ tests/qemu-iotests/059 | 4 ++++ tests/qemu-iotests/059.out | 4 ++++ 3 files changed, 12 insertions(+) diff --git a/block/vmdk.c b/block/vmdk.c index a1aaea77e9..709aa3deb0 100644 --- a/block/vmdk.c +++ b/block/vmdk.c @@ -1664,6 +1664,10 @@ static int vmdk_create(const char *filename, QEMUOptionParameter *options, error_setg(errp, "Flat image can't have backing file"); return -ENOTSUP; } + if (flat && zeroed_grain) { + error_setg(errp, "Flat image can't enable zeroed grain"); + return -ENOTSUP; + } if (backing_file) { BlockDriverState *bs = bdrv_new(""); ret = bdrv_open(bs, backing_file, NULL, 0, NULL, errp); diff --git a/tests/qemu-iotests/059 b/tests/qemu-iotests/059 index 18cdad1341..b81c575d94 100755 --- a/tests/qemu-iotests/059 +++ b/tests/qemu-iotests/059 @@ -71,6 +71,10 @@ echo "=== Testing monolithicFlat creation and opening ===" IMGOPTS="subformat=monolithicFlat" _make_test_img 2G $QEMU_IMG info $TEST_IMG | _filter_testdir +echo +echo "=== Testing monolithicFlat with zeroed_grain ===" +IMGOPTS="subformat=monolithicFlat,zeroed_grain=on" _make_test_img 2G + # success, all done echo "*** done" rm -f $seq.full diff --git a/tests/qemu-iotests/059.out b/tests/qemu-iotests/059.out index 265cd76169..9b12efb466 100644 --- a/tests/qemu-iotests/059.out +++ b/tests/qemu-iotests/059.out @@ -22,4 +22,8 @@ image: TEST_DIR/t.vmdk file format: vmdk virtual size: 2.0G (2147483648 bytes) disk size: 4.0K + +=== Testing monolithicFlat with zeroed_grain === +qemu-img: TEST_DIR/t.IMGFMT: Flat image can't enable zeroed grain +Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=2147483648 *** done From 14d36307ffdf949df9c1dd7f435e138b36f63bb0 Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Wed, 18 Sep 2013 17:22:02 +0200 Subject: [PATCH 42/61] qapi-types/visit.py: Pass whole expr dict for structs Signed-off-by: Kevin Wolf Reviewed-by: Eric Blake --- scripts/qapi-types.py | 11 ++++++++--- scripts/qapi-visit.py | 8 ++++++-- 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py index 5222463893..566fe5e4d9 100644 --- a/scripts/qapi-types.py +++ b/scripts/qapi-types.py @@ -71,7 +71,7 @@ def generate_struct_fields(members): c_name=c_var(argname)) if structured: push_indent() - ret += generate_struct("", argname, argentry) + ret += generate_struct({ "field": argname, "data": argentry}) pop_indent() else: ret += mcgen(''' @@ -81,7 +81,12 @@ def generate_struct_fields(members): return ret -def generate_struct(structname, fieldname, members): +def generate_struct(expr): + + structname = expr.get('type', "") + fieldname = expr.get('field', "") + members = expr['data'] + ret = mcgen(''' struct %(name)s { @@ -417,7 +422,7 @@ def maybe_open(really, name, opt): for expr in exprs: ret = "\n" if expr.has_key('type'): - ret += generate_struct(expr['type'], "", expr['data']) + "\n" + ret += generate_struct(expr) + "\n" ret += generate_type_cleanup_decl(expr['type'] + "List") fdef.write(generate_type_cleanup(expr['type'] + "List") + "\n") ret += generate_type_cleanup_decl(expr['type']) diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py index 597cca4b66..1e44004a1e 100644 --- a/scripts/qapi-visit.py +++ b/scripts/qapi-visit.py @@ -120,7 +120,11 @@ def generate_visit_struct_body(field_prefix, name, members): ''') return ret -def generate_visit_struct(name, members): +def generate_visit_struct(expr): + + name = expr['type'] + members = expr['data'] + ret = generate_visit_struct_fields(name, "", "", members) ret += mcgen(''' @@ -472,7 +476,7 @@ def maybe_open(really, name, opt): for expr in exprs: if expr.has_key('type'): - ret = generate_visit_struct(expr['type'], expr['data']) + ret = generate_visit_struct(expr) ret += generate_visit_list(expr['type'], expr['data']) fdef.write(ret) From 622f557f5aaea1326c94ca4cddfa4eafeade3723 Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Thu, 19 Sep 2013 11:56:36 +0200 Subject: [PATCH 43/61] qapi-types/visit.py: Inheritance for structs This introduces a new 'base' key for struct definitions that refers to another struct type. On the JSON level, the fields of the base type are included directly into the same namespace as the fields of the defined type, like with unions. On the C level, a pointer to a struct of the base type is included. Signed-off-by: Kevin Wolf Reviewed-by: Eric Blake --- docs/qapi-code-gen.txt | 17 +++++++++++++++++ scripts/qapi-types.py | 4 ++++ scripts/qapi-visit.py | 18 ++++++++++++++++-- 3 files changed, 37 insertions(+), 2 deletions(-) diff --git a/docs/qapi-code-gen.txt b/docs/qapi-code-gen.txt index 0ce045c0b3..91f44d01b9 100644 --- a/docs/qapi-code-gen.txt +++ b/docs/qapi-code-gen.txt @@ -53,6 +53,23 @@ The use of '*' as a prefix to the name means the member is optional. Optional members should always be added to the end of the dictionary to preserve backwards compatibility. + +A complex type definition can specify another complex type as its base. +In this case, the fields of the base type are included as top-level fields +of the new complex type's dictionary in the QMP wire format. An example +definition is: + + { 'type': 'BlockdevOptionsGenericFormat', 'data': { 'file': 'str' } } + { 'type': 'BlockdevOptionsGenericCOWFormat', + 'base': 'BlockdevOptionsGenericFormat', + 'data': { '*backing': 'str' } } + +An example BlockdevOptionsGenericCOWFormat object on the wire could use +both fields like this: + + { "file": "/some/place/my-image", + "backing": "/some/place/my-backing-file" } + === Enumeration types === An enumeration type is a dictionary containing a single key whose value is a diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py index 566fe5e4d9..4a1652b56f 100644 --- a/scripts/qapi-types.py +++ b/scripts/qapi-types.py @@ -86,6 +86,7 @@ def generate_struct(expr): structname = expr.get('type', "") fieldname = expr.get('field', "") members = expr['data'] + base = expr.get('base') ret = mcgen(''' struct %(name)s @@ -93,6 +94,9 @@ def generate_struct(expr): ''', name=structname) + if base: + ret += generate_struct_fields({'base': base}) + ret += generate_struct_fields(members) if len(fieldname): diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py index 1e44004a1e..c39e6284b8 100644 --- a/scripts/qapi-visit.py +++ b/scripts/qapi-visit.py @@ -17,7 +17,7 @@ import getopt import errno -def generate_visit_struct_fields(name, field_prefix, fn_prefix, members): +def generate_visit_struct_fields(name, field_prefix, fn_prefix, members, base = None): substructs = [] ret = '' full_name = name if not fn_prefix else "%s_%s" % (name, fn_prefix) @@ -42,6 +42,19 @@ def generate_visit_struct_fields(name, field_prefix, fn_prefix, members): name=name, full_name=full_name) push_indent() + if base: + ret += mcgen(''' +visit_start_implicit_struct(m, obj ? (void**) &(*obj)->%(c_name)s : NULL, sizeof(%(type)s), &err); +if (!err) { + visit_type_%(type)s_fields(m, obj ? &(*obj)->%(c_prefix)s%(c_name)s : NULL, &err); + error_propagate(errp, err); + err = NULL; + visit_end_implicit_struct(m, &err); +} +''', + c_prefix=c_var(field_prefix), + type=type_name(base), c_name=c_var('base')) + for argname, argentry, optional, structured in parse_args(members): if optional: ret += mcgen(''' @@ -124,8 +137,9 @@ def generate_visit_struct(expr): name = expr['type'] members = expr['data'] + base = expr.get('base') - ret = generate_visit_struct_fields(name, "", "", members) + ret = generate_visit_struct_fields(name, "", "", members, base) ret += mcgen(''' From 2d246f01d374c1a10c48c45aa931aa18f0a56634 Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Wed, 18 Sep 2013 15:14:47 +0200 Subject: [PATCH 44/61] blockdev: Introduce DriveInfo.enable_auto_del BlockDriverStates shouldn't be affected by an unplugged guest device, except if created with the legacy -drive command line option or the drive_add HMP command. Make the automatic deletion as well as cancelling of jobs conditional on an enable_auto_del boolean that is only set in drive_init(). Signed-off-by: Kevin Wolf Reviewed-by: Eric Blake Reviewed-by: Wenchao Xia --- blockdev.c | 17 ++++++++++++++++- include/sysemu/blockdev.h | 1 + 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/blockdev.c b/blockdev.c index ab79df7bff..52996eada3 100644 --- a/blockdev.c +++ b/blockdev.c @@ -89,6 +89,10 @@ void blockdev_mark_auto_del(BlockDriverState *bs) { DriveInfo *dinfo = drive_get_by_blockdev(bs); + if (dinfo && !dinfo->enable_auto_del) { + return; + } + if (bs->job) { block_job_cancel(bs->job); } @@ -746,6 +750,7 @@ static void qemu_opt_rename(QemuOpts *opts, const char *from, const char *to) DriveInfo *drive_init(QemuOpts *all_opts, BlockInterfaceType block_default_type) { const char *value; + DriveInfo *dinfo; /* Change legacy command line options into QMP ones */ qemu_opt_rename(all_opts, "iops", "throttling.iops-total"); @@ -794,7 +799,17 @@ DriveInfo *drive_init(QemuOpts *all_opts, BlockInterfaceType block_default_type) qemu_opt_unset(all_opts, "cache"); } - return blockdev_init(all_opts, block_default_type); + /* Actual block device init: Functionality shared with blockdev-add */ + dinfo = blockdev_init(all_opts, block_default_type); + if (dinfo == NULL) { + goto fail; + } + + /* Set legacy DriveInfo fields */ + dinfo->enable_auto_del = true; + +fail: + return dinfo; } void do_commit(Monitor *mon, const QDict *qdict) diff --git a/include/sysemu/blockdev.h b/include/sysemu/blockdev.h index 804ec8839b..10820910d7 100644 --- a/include/sysemu/blockdev.h +++ b/include/sysemu/blockdev.h @@ -37,6 +37,7 @@ struct DriveInfo { int bus; int unit; int auto_del; /* see blockdev_mark_auto_del() */ + bool enable_auto_del; /* Only for legacy drive_init() */ int media_cd; int cyls, heads, secs, trans; QemuOpts *opts; From d26c9a15738147a8dccc451c6f6d1ddc2305713d Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Mon, 23 Sep 2013 15:26:03 +0200 Subject: [PATCH 45/61] blockdev: 'blockdev-add' QMP command For examples see the changes to qmp-commands.hx. Signed-off-by: Kevin Wolf --- blockdev.c | 57 ++++++++++++ qapi-schema.json | 236 +++++++++++++++++++++++++++++++++++++++++++++++ qmp-commands.hx | 55 +++++++++++ 3 files changed, 348 insertions(+) diff --git a/blockdev.c b/blockdev.c index 52996eada3..9929d781a5 100644 --- a/blockdev.c +++ b/blockdev.c @@ -38,6 +38,8 @@ #include "qemu/option.h" #include "qemu/config-file.h" #include "qapi/qmp/types.h" +#include "qapi-visit.h" +#include "qapi/qmp-output-visitor.h" #include "sysemu/sysemu.h" #include "block/block_int.h" #include "qmp-commands.h" @@ -2066,6 +2068,61 @@ void qmp_block_job_complete(const char *device, Error **errp) block_job_complete(job, errp); } +void qmp_blockdev_add(BlockdevOptions *options, Error **errp) +{ + QmpOutputVisitor *ov = qmp_output_visitor_new(); + QObject *obj; + QDict *qdict; + DriveInfo *dinfo; + Error *local_err = NULL; + + /* Require an ID in the top level */ + if (!options->has_id) { + error_setg(errp, "Block device needs an ID"); + goto fail; + } + + /* TODO Sort it out in raw-posix and drive_init: Reject aio=native with + * cache.direct=false instead of silently switching to aio=threads, except + * if called from drive_init. + * + * For now, simply forbidding the combination for all drivers will do. */ + if (options->has_aio && options->aio == BLOCKDEV_AIO_OPTIONS_NATIVE) { + bool direct = options->cache->has_direct && options->cache->direct; + if (!options->has_cache && !direct) { + error_setg(errp, "aio=native requires cache.direct=true"); + goto fail; + } + } + + visit_type_BlockdevOptions(qmp_output_get_visitor(ov), + &options, NULL, &local_err); + if (error_is_set(&local_err)) { + error_propagate(errp, local_err); + goto fail; + } + + obj = qmp_output_get_qobject(ov); + qdict = qobject_to_qdict(obj); + + qdict_flatten(qdict); + + QemuOpts *opts = qemu_opts_from_qdict(&qemu_drive_opts, qdict, &local_err); + if (error_is_set(&local_err)) { + error_propagate(errp, local_err); + goto fail; + } + + dinfo = blockdev_init(opts, IF_NONE); + if (!dinfo) { + error_setg(errp, "Could not open image"); + goto fail; + } + +fail: + qmp_output_visitor_cleanup(ov); +} + static void do_qmp_query_block_jobs_one(void *opaque, BlockDriverState *bs) { BlockJobInfoList **prev = opaque; diff --git a/qapi-schema.json b/qapi-schema.json index a1a81a4481..60f3fd1db6 100644 --- a/qapi-schema.json +++ b/qapi-schema.json @@ -3952,3 +3952,239 @@ ## { 'command': 'query-rx-filter', 'data': { '*name': 'str' }, 'returns': ['RxFilterInfo'] } + + +## +# @BlockdevDiscardOptions +# +# Determines how to handle discard requests. +# +# @ignore: Ignore the request +# @unmap: Forward as an unmap request +# +# Since: 1.7 +## +{ 'enum': 'BlockdevDiscardOptions', + 'data': [ 'ignore', 'unmap' ] } + +## +# @BlockdevAioOptions +# +# Selects the AIO backend to handle I/O requests +# +# @threads: Use qemu's thread pool +# @native: Use native AIO backend (only Linux and Windows) +# +# Since: 1.7 +## +{ 'enum': 'BlockdevAioOptions', + 'data': [ 'threads', 'native' ] } + +## +# @BlockdevCacheOptions +# +# Includes cache-related options for block devices +# +# @writeback: #optional enables writeback mode for any caches (default: true) +# @direct: #optional enables use of O_DIRECT (bypass the host page cache; +# default: false) +# @no-flush: #optional ignore any flush requests for the device (default: +# false) +# +# Since: 1.7 +## +{ 'type': 'BlockdevCacheOptions', + 'data': { '*writeback': 'bool', + '*direct': 'bool', + '*no-flush': 'bool' } } + +## +# @BlockdevOptionsBase +# +# Options that are available for all block devices, independent of the block +# driver. +# +# @driver: block driver name +# @id: #optional id by which the new block device can be referred to. +# This is a required option on the top level of blockdev-add, and +# currently not allowed on any other level. +# @discard: #optional discard-related options (default: ignore) +# @cache: #optional cache-related options +# @aio: #optional AIO backend (default: threads) +# @rerror: #optional how to handle read errors on the device +# (default: report) +# @werror: #optional how to handle write errors on the device +# (default: enospc) +# @read-only: #optional whether the block device should be read-only +# (default: false) +# +# Since: 1.7 +## +{ 'type': 'BlockdevOptionsBase', + 'data': { 'driver': 'str', + '*id': 'str', + '*discard': 'BlockdevDiscardOptions', + '*cache': 'BlockdevCacheOptions', + '*aio': 'BlockdevAioOptions', + '*rerror': 'BlockdevOnError', + '*werror': 'BlockdevOnError', + '*read-only': 'bool' } } + +## +# @BlockdevOptionsFile +# +# Driver specific block device options for the file backend and similar +# protocols. +# +# @filename: path to the image file +# +# Since: 1.7 +## +{ 'type': 'BlockdevOptionsFile', + 'data': { 'filename': 'str' } } + +## +# @BlockdevOptionsVVFAT +# +# Driver specific block device options for the vvfat protocol. +# +# @dir: directory to be exported as FAT image +# @fat-type: #optional FAT type: 12, 16 or 32 +# @floppy: #optional whether to export a floppy image (true) or +# partitioned hard disk (false; default) +# @rw: #optional whether to allow write operations (default: false) +# +# Since: 1.7 +## +{ 'type': 'BlockdevOptionsVVFAT', + 'data': { 'dir': 'str', '*fat-type': 'int', '*floppy': 'bool', + '*rw': 'bool' } } + +## +# @BlockdevOptionsGenericFormat +# +# Driver specific block device options for image format that have no option +# besides their data source. +# +# @file: reference to or definition of the data source block device +# +# Since: 1.7 +## +{ 'type': 'BlockdevOptionsGenericFormat', + 'data': { 'file': 'BlockdevRef' } } + +## +# @BlockdevOptionsGenericCOWFormat +# +# Driver specific block device options for image format that have no option +# besides their data source and an optional backing file. +# +# @backing: #optional reference to or definition of the backing file block +# device (if missing, taken from the image file content). It is +# allowed to pass an empty string here in order to disable the +# default backing file. +# +# Since: 1.7 +## +{ 'type': 'BlockdevOptionsGenericCOWFormat', + 'base': 'BlockdevOptionsGenericFormat', + 'data': { '*backing': 'BlockdevRef' } } + +## +# @BlockdevOptionsQcow2 +# +# Driver specific block device options for qcow2. +# +# @lazy-refcounts: #optional whether to enable the lazy refcounts +# feature (default is taken from the image file) +# +# @pass-discard-request: #optional whether discard requests to the qcow2 +# device should be forwarded to the data source +# +# @pass-discard-snapshot: #optional whether discard requests for the data source +# should be issued when a snapshot operation (e.g. +# deleting a snapshot) frees clusters in the qcow2 file +# +# @pass-discard-other: #optional whether discard requests for the data source +# should be issued on other occasions where a cluster +# gets freed +# +# Since: 1.7 +## +{ 'type': 'BlockdevOptionsQcow2', + 'base': 'BlockdevOptionsGenericCOWFormat', + 'data': { '*lazy-refcounts': 'bool', + '*pass-discard-request': 'bool', + '*pass-discard-snapshot': 'bool', + '*pass-discard-other': 'bool' } } + +## +# @BlockdevOptions +# +# Options for creating a block device. +# +# Since: 1.7 +## +{ 'union': 'BlockdevOptions', + 'base': 'BlockdevOptionsBase', + 'discriminator': 'driver', + 'data': { + 'file': 'BlockdevOptionsFile', + 'http': 'BlockdevOptionsFile', + 'https': 'BlockdevOptionsFile', + 'ftp': 'BlockdevOptionsFile', + 'ftps': 'BlockdevOptionsFile', + 'tftp': 'BlockdevOptionsFile', +# TODO gluster: Wait for structured options +# TODO iscsi: Wait for structured options +# TODO nbd: Should take InetSocketAddress for 'host'? +# TODO rbd: Wait for structured options +# TODO sheepdog: Wait for structured options +# TODO ssh: Should take InetSocketAddress for 'host'? + 'vvfat': 'BlockdevOptionsVVFAT', + +# TODO blkdebug: Wait for structured options +# TODO blkverify: Wait for structured options + + 'bochs': 'BlockdevOptionsGenericFormat', + 'cloop': 'BlockdevOptionsGenericFormat', + 'cow': 'BlockdevOptionsGenericCOWFormat', + 'dmg': 'BlockdevOptionsGenericFormat', + 'parallels': 'BlockdevOptionsGenericFormat', + 'qcow': 'BlockdevOptionsGenericCOWFormat', + 'qcow2': 'BlockdevOptionsQcow2', + 'qed': 'BlockdevOptionsGenericCOWFormat', + 'raw': 'BlockdevOptionsGenericFormat', + 'vdi': 'BlockdevOptionsGenericFormat', + 'vhdx': 'BlockdevOptionsGenericFormat', + 'vmdk': 'BlockdevOptionsGenericCOWFormat', + 'vpc': 'BlockdevOptionsGenericFormat' + } } + +## +# @BlockdevRef +# +# Reference to a block device. +# +# @definition: defines a new block device inline +# @reference: references the ID of an existing block device. An +# empty string means that no block device should be +# referenced. +# +# Since: 1.7 +## +{ 'union': 'BlockdevRef', + 'discriminator': {}, + 'data': { 'definition': 'BlockdevOptions', + 'reference': 'str' } } + +## +# @blockdev-add: +# +# Creates a new block device. +# +# @options: block device options for the new device +# +# Since: 1.7 +## +{ 'command': 'blockdev-add', 'data': { 'options': 'BlockdevOptions' } } diff --git a/qmp-commands.hx b/qmp-commands.hx index b17c46e0b1..fba15cdc3b 100644 --- a/qmp-commands.hx +++ b/qmp-commands.hx @@ -3239,4 +3239,59 @@ Example: ] } +EQMP + + { + .name = "blockdev-add", + .args_type = "options:q", + .mhandler.cmd_new = qmp_marshal_input_blockdev_add, + }, + +SQMP +blockdev-add +------------ + +Add a block device. + +Arguments: + +- "options": block driver options + +Example (1): + +-> { "execute": "blockdev-add", + "arguments": { "options" : { "driver": "qcow2", + "file": { "driver": "file", + "filename": "test.qcow2" } } } } +<- { "return": {} } + +Example (2): + +-> { "execute": "blockdev-add", + "arguments": { + "options": { + "driver": "qcow2", + "id": "my_disk", + "discard": "unmap", + "cache": { + "direct": true, + "writeback": true + }, + "file": { + "driver": "file", + "filename": "/tmp/test.qcow2" + }, + "backing": { + "driver": "raw", + "file": { + "driver": "file", + "filename": "/dev/fdset/4" + } + } + } + } + } + +<- { "return": {} } + EQMP From 326642bc7f0ff95a0c08db527861a9a114a109da Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Thu, 11 Jul 2013 12:52:34 +0200 Subject: [PATCH 46/61] blockdev: Separate ID generation from DriveInfo creation blockdev-add shouldn't automatically generate IDs, but will keep most of the DriveInfo creation code. Signed-off-by: Kevin Wolf Reviewed-by: Max Reitz Reviewed-by: Wenchao Xia Reviewed-by: Eric Blake --- blockdev.c | 32 +++++++++++++++++--------------- include/qemu/option.h | 1 + util/qemu-option.c | 6 ++++++ 3 files changed, 24 insertions(+), 15 deletions(-) diff --git a/blockdev.c b/blockdev.c index 9929d781a5..c1fcd3c8d3 100644 --- a/blockdev.c +++ b/blockdev.c @@ -600,23 +600,25 @@ static DriveInfo *blockdev_init(QemuOpts *all_opts, return NULL; } - /* init */ - - dinfo = g_malloc0(sizeof(*dinfo)); - if ((buf = qemu_opts_id(opts)) != NULL) { - dinfo->id = g_strdup(buf); - } else { - /* no id supplied -> create one */ - dinfo->id = g_malloc0(32); - if (type == IF_IDE || type == IF_SCSI) + /* no id supplied -> create one */ + if (qemu_opts_id(opts) == NULL) { + char *new_id; + if (type == IF_IDE || type == IF_SCSI) { mediastr = (media == MEDIA_CDROM) ? "-cd" : "-hd"; - if (max_devs) - snprintf(dinfo->id, 32, "%s%i%s%i", - if_name[type], bus_id, mediastr, unit_id); - else - snprintf(dinfo->id, 32, "%s%s%i", - if_name[type], mediastr, unit_id); + } + if (max_devs) { + new_id = g_strdup_printf("%s%i%s%i", if_name[type], bus_id, + mediastr, unit_id); + } else { + new_id = g_strdup_printf("%s%s%i", if_name[type], + mediastr, unit_id); + } + qemu_opts_set_id(opts, new_id); } + + /* init */ + dinfo = g_malloc0(sizeof(*dinfo)); + dinfo->id = g_strdup(qemu_opts_id(opts)); dinfo->bdrv = bdrv_new(dinfo->id); dinfo->bdrv->open_flags = snapshot ? BDRV_O_SNAPSHOT : 0; dinfo->bdrv->read_only = ro; diff --git a/include/qemu/option.h b/include/qemu/option.h index 63db4ccb9a..5c0c6dd294 100644 --- a/include/qemu/option.h +++ b/include/qemu/option.h @@ -142,6 +142,7 @@ void qemu_opts_loc_restore(QemuOpts *opts); int qemu_opts_set(QemuOptsList *list, const char *id, const char *name, const char *value); const char *qemu_opts_id(QemuOpts *opts); +void qemu_opts_set_id(QemuOpts *opts, char *id); void qemu_opts_del(QemuOpts *opts); void qemu_opts_validate(QemuOpts *opts, const QemuOptDesc *desc, Error **errp); int qemu_opts_do_parse(QemuOpts *opts, const char *params, const char *firstname); diff --git a/util/qemu-option.c b/util/qemu-option.c index e0844a966c..efcb5dcfcb 100644 --- a/util/qemu-option.c +++ b/util/qemu-option.c @@ -834,6 +834,12 @@ const char *qemu_opts_id(QemuOpts *opts) return opts->id; } +/* The id string will be g_free()d by qemu_opts_del */ +void qemu_opts_set_id(QemuOpts *opts, char *id) +{ + opts->id = id; +} + void qemu_opts_del(QemuOpts *opts) { QemuOpt *opt; From f298d071662af6cf5dc221ee3e3bd0154035e570 Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Tue, 10 Sep 2013 12:01:20 +0200 Subject: [PATCH 47/61] blockdev: Pass QDict to blockdev_init() Working on a QDict instead of a QemuOpts that accepts anything is more in line with bdrv_open(). A QDict is what qmp_blockdev_add() already has anyway, so this saves additional conversions. And last, but not least, it allows later patches to easily extract legacy options into a separate, typed QemuOpts for drive_init() (the untyped QemuOpts that drive_init already has doesn't allow access to numbers, only strings, and is therefore useless without conversion). Signed-off-by: Kevin Wolf Reviewed-by: Benoit Canet Reviewed-by: Eric Blake --- blockdev.c | 34 +++++++++++++++++----------------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/blockdev.c b/blockdev.c index c1fcd3c8d3..617fad9b8d 100644 --- a/blockdev.c +++ b/blockdev.c @@ -217,7 +217,10 @@ static void bdrv_format_print(void *opaque, const char *name) static void drive_uninit(DriveInfo *dinfo) { - qemu_opts_del(dinfo->opts); + if (dinfo->opts) { + qemu_opts_del(dinfo->opts); + } + bdrv_unref(dinfo->bdrv); g_free(dinfo->id); QTAILQ_REMOVE(&drives, dinfo, next); @@ -302,7 +305,8 @@ static bool check_throttle_config(ThrottleConfig *cfg, Error **errp) return true; } -static DriveInfo *blockdev_init(QemuOpts *all_opts, +/* Takes the ownership of bs_opts */ +static DriveInfo *blockdev_init(QDict *bs_opts, BlockInterfaceType block_default_type) { const char *buf; @@ -326,7 +330,6 @@ static DriveInfo *blockdev_init(QemuOpts *all_opts, int ret; Error *error = NULL; QemuOpts *opts; - QDict *bs_opts; const char *id; bool has_driver_specific_opts; BlockDriver *drv = NULL; @@ -334,9 +337,9 @@ static DriveInfo *blockdev_init(QemuOpts *all_opts, translation = BIOS_ATA_TRANSLATION_AUTO; media = MEDIA_DISK; - /* Check common options by copying from all_opts to opts, all other options - * are stored in bs_opts. */ - id = qemu_opts_id(all_opts); + /* Check common options by copying from bs_opts to opts, all other options + * stay in bs_opts for processing by bdrv_open(). */ + id = qdict_get_try_str(bs_opts, "id"); opts = qemu_opts_create(&qemu_common_drive_opts, id, 1, &error); if (error_is_set(&error)) { qerror_report_err(error); @@ -344,8 +347,6 @@ static DriveInfo *blockdev_init(QemuOpts *all_opts, return NULL; } - bs_opts = qdict_new(); - qemu_opts_to_qdict(all_opts, bs_opts); qemu_opts_absorb_qdict(opts, bs_opts, &error); if (error_is_set(&error)) { qerror_report_err(error); @@ -630,7 +631,6 @@ static DriveInfo *blockdev_init(QemuOpts *all_opts, dinfo->heads = heads; dinfo->secs = secs; dinfo->trans = translation; - dinfo->opts = all_opts; dinfo->refcount = 1; if (serial != NULL) { dinfo->serial = g_strdup(serial); @@ -755,6 +755,7 @@ DriveInfo *drive_init(QemuOpts *all_opts, BlockInterfaceType block_default_type) { const char *value; DriveInfo *dinfo; + QDict *bs_opts; /* Change legacy command line options into QMP ones */ qemu_opt_rename(all_opts, "iops", "throttling.iops-total"); @@ -803,14 +804,19 @@ DriveInfo *drive_init(QemuOpts *all_opts, BlockInterfaceType block_default_type) qemu_opt_unset(all_opts, "cache"); } + /* Get a QDict for processing the options */ + bs_opts = qdict_new(); + qemu_opts_to_qdict(all_opts, bs_opts); + /* Actual block device init: Functionality shared with blockdev-add */ - dinfo = blockdev_init(all_opts, block_default_type); + dinfo = blockdev_init(bs_opts, block_default_type); if (dinfo == NULL) { goto fail; } /* Set legacy DriveInfo fields */ dinfo->enable_auto_del = true; + dinfo->opts = all_opts; fail: return dinfo; @@ -2109,13 +2115,7 @@ void qmp_blockdev_add(BlockdevOptions *options, Error **errp) qdict_flatten(qdict); - QemuOpts *opts = qemu_opts_from_qdict(&qemu_drive_opts, qdict, &local_err); - if (error_is_set(&local_err)) { - error_propagate(errp, local_err); - goto fail; - } - - dinfo = blockdev_init(opts, IF_NONE); + dinfo = blockdev_init(qdict, IF_NONE); if (!dinfo) { error_setg(errp, "Could not open image"); goto fail; From 33cb7dc8b7a26ccdff2f054056d3f2e487cbb4cd Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Wed, 28 Aug 2013 17:00:13 +0200 Subject: [PATCH 48/61] blockdev: Move parsing of 'media' option to drive_init This moves as much as possible of the processing of the 'media' option to drive_init so that it can only be accessed using legacy functions, but never with anything blockdev-add related. Signed-off-by: Kevin Wolf Reviewed-by: Benoit Canet Reviewed-by: Eric Blake --- blockdev.c | 73 +++++++++++++++++++++++++++++++++++++----------------- 1 file changed, 50 insertions(+), 23 deletions(-) diff --git a/blockdev.c b/blockdev.c index 617fad9b8d..149ddc1b3e 100644 --- a/blockdev.c +++ b/blockdev.c @@ -305,16 +305,18 @@ static bool check_throttle_config(ThrottleConfig *cfg, Error **errp) return true; } +typedef enum { MEDIA_DISK, MEDIA_CDROM } DriveMediaType; + /* Takes the ownership of bs_opts */ static DriveInfo *blockdev_init(QDict *bs_opts, - BlockInterfaceType block_default_type) + BlockInterfaceType block_default_type, + DriveMediaType media) { const char *buf; const char *file = NULL; const char *serial; const char *mediastr = ""; BlockInterfaceType type; - enum { MEDIA_DISK, MEDIA_CDROM } media; int bus_id, unit_id; int cyls, heads, secs, translation; int max_devs; @@ -335,7 +337,6 @@ static DriveInfo *blockdev_init(QDict *bs_opts, BlockDriver *drv = NULL; translation = BIOS_ATA_TRANSLATION_AUTO; - media = MEDIA_DISK; /* Check common options by copying from bs_opts to opts, all other options * stay in bs_opts for processing by bdrv_open(). */ @@ -422,19 +423,11 @@ static DriveInfo *blockdev_init(QDict *bs_opts, } } - if ((buf = qemu_opt_get(opts, "media")) != NULL) { - if (!strcmp(buf, "disk")) { - media = MEDIA_DISK; - } else if (!strcmp(buf, "cdrom")) { - if (cyls || secs || heads) { - error_report("CHS can't be set with media=%s", buf); - return NULL; - } - media = MEDIA_CDROM; - } else { - error_report("'%s' invalid media", buf); - return NULL; - } + if (media == MEDIA_CDROM) { + if (cyls || secs || heads) { + error_report("CHS can't be set with media=cdrom"); + return NULL; + } } if ((buf = qemu_opt_get(opts, "discard")) != NULL) { @@ -751,11 +744,27 @@ static void qemu_opt_rename(QemuOpts *opts, const char *from, const char *to) } } +QemuOptsList qemu_legacy_drive_opts = { + .name = "drive", + .head = QTAILQ_HEAD_INITIALIZER(qemu_legacy_drive_opts.head), + .desc = { + { + .name = "media", + .type = QEMU_OPT_STRING, + .help = "media type (disk, cdrom)", + }, + { /* end of list */ } + }, +}; + DriveInfo *drive_init(QemuOpts *all_opts, BlockInterfaceType block_default_type) { const char *value; - DriveInfo *dinfo; + DriveInfo *dinfo = NULL; QDict *bs_opts; + QemuOpts *legacy_opts; + DriveMediaType media = MEDIA_DISK; + Error *local_err = NULL; /* Change legacy command line options into QMP ones */ qemu_opt_rename(all_opts, "iops", "throttling.iops-total"); @@ -808,8 +817,29 @@ DriveInfo *drive_init(QemuOpts *all_opts, BlockInterfaceType block_default_type) bs_opts = qdict_new(); qemu_opts_to_qdict(all_opts, bs_opts); + legacy_opts = qemu_opts_create_nofail(&qemu_legacy_drive_opts); + qemu_opts_absorb_qdict(legacy_opts, bs_opts, &local_err); + if (error_is_set(&local_err)) { + qerror_report_err(local_err); + error_free(local_err); + goto fail; + } + + /* Media type */ + value = qemu_opt_get(legacy_opts, "media"); + if (value) { + if (!strcmp(value, "disk")) { + media = MEDIA_DISK; + } else if (!strcmp(value, "cdrom")) { + media = MEDIA_CDROM; + } else { + error_report("'%s' invalid media", value); + goto fail; + } + } + /* Actual block device init: Functionality shared with blockdev-add */ - dinfo = blockdev_init(bs_opts, block_default_type); + dinfo = blockdev_init(bs_opts, block_default_type, media); if (dinfo == NULL) { goto fail; } @@ -819,6 +849,7 @@ DriveInfo *drive_init(QemuOpts *all_opts, BlockInterfaceType block_default_type) dinfo->opts = all_opts; fail: + qemu_opts_del(legacy_opts); return dinfo; } @@ -2115,7 +2146,7 @@ void qmp_blockdev_add(BlockdevOptions *options, Error **errp) qdict_flatten(qdict); - dinfo = blockdev_init(qdict, IF_NONE); + dinfo = blockdev_init(qdict, IF_NONE, MEDIA_DISK); if (!dinfo) { error_setg(errp, "Could not open image"); goto fail; @@ -2183,10 +2214,6 @@ QemuOptsList qemu_common_drive_opts = { .name = "trans", .type = QEMU_OPT_STRING, .help = "chs translation (auto, lba. none)", - },{ - .name = "media", - .type = QEMU_OPT_STRING, - .help = "media type (disk, cdrom)", },{ .name = "snapshot", .type = QEMU_OPT_BOOL, From 593d464bd43900c2a0c8800b76212f6a93e99a0d Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Wed, 28 Aug 2013 17:24:51 +0200 Subject: [PATCH 49/61] blockdev: Move parsing of 'if' option to drive_init It's always IF_NONE for blockdev-add. Signed-off-by: Kevin Wolf Reviewed-by: Benoit Canet Reviewed-by: Eric Blake --- blockdev.c | 40 ++++++++++++++++++++++------------------ 1 file changed, 22 insertions(+), 18 deletions(-) diff --git a/blockdev.c b/blockdev.c index 149ddc1b3e..e25f0b8d2c 100644 --- a/blockdev.c +++ b/blockdev.c @@ -309,14 +309,13 @@ typedef enum { MEDIA_DISK, MEDIA_CDROM } DriveMediaType; /* Takes the ownership of bs_opts */ static DriveInfo *blockdev_init(QDict *bs_opts, - BlockInterfaceType block_default_type, + BlockInterfaceType type, DriveMediaType media) { const char *buf; const char *file = NULL; const char *serial; const char *mediastr = ""; - BlockInterfaceType type; int bus_id, unit_id; int cyls, heads, secs, translation; int max_devs; @@ -377,17 +376,6 @@ static DriveInfo *blockdev_init(QDict *bs_opts, file = qemu_opt_get(opts, "file"); serial = qemu_opt_get(opts, "serial"); - if ((buf = qemu_opt_get(opts, "if")) != NULL) { - for (type = 0; type < IF_COUNT && strcmp(buf, if_name[type]); type++) - ; - if (type == IF_COUNT) { - error_report("unsupported bus type '%s'", buf); - return NULL; - } - } else { - type = block_default_type; - } - max_devs = if_max_devs[type]; if (cyls || heads || secs) { @@ -752,6 +740,10 @@ QemuOptsList qemu_legacy_drive_opts = { .name = "media", .type = QEMU_OPT_STRING, .help = "media type (disk, cdrom)", + },{ + .name = "if", + .type = QEMU_OPT_STRING, + .help = "interface (ide, scsi, sd, mtd, floppy, pflash, virtio)", }, { /* end of list */ } }, @@ -764,6 +756,7 @@ DriveInfo *drive_init(QemuOpts *all_opts, BlockInterfaceType block_default_type) QDict *bs_opts; QemuOpts *legacy_opts; DriveMediaType media = MEDIA_DISK; + BlockInterfaceType type; Error *local_err = NULL; /* Change legacy command line options into QMP ones */ @@ -838,8 +831,23 @@ DriveInfo *drive_init(QemuOpts *all_opts, BlockInterfaceType block_default_type) } } + /* Controller type */ + value = qemu_opt_get(legacy_opts, "if"); + if (value) { + for (type = 0; + type < IF_COUNT && strcmp(value, if_name[type]); + type++) { + } + if (type == IF_COUNT) { + error_report("unsupported bus type '%s'", value); + goto fail; + } + } else { + type = block_default_type; + } + /* Actual block device init: Functionality shared with blockdev-add */ - dinfo = blockdev_init(bs_opts, block_default_type, media); + dinfo = blockdev_init(bs_opts, type, media); if (dinfo == NULL) { goto fail; } @@ -2190,10 +2198,6 @@ QemuOptsList qemu_common_drive_opts = { .name = "unit", .type = QEMU_OPT_NUMBER, .help = "unit number (i.e. lun for scsi)", - },{ - .name = "if", - .type = QEMU_OPT_STRING, - .help = "interface (ide, scsi, sd, mtd, floppy, pflash, virtio)", },{ .name = "index", .type = QEMU_OPT_NUMBER, From b41a7338cfdeeb913ee4846d79a3f7e221350aed Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Mon, 9 Sep 2013 16:49:49 +0200 Subject: [PATCH 50/61] blockdev: Moving parsing of geometry options to drive_init This moves all of the geometry options (cyls/heads/secs/trans) to drive_init so that they can only be accessed using legacy functions, but never with anything blockdev-add related. Signed-off-by: Kevin Wolf Reviewed-by: Eric Blake --- blockdev.c | 136 +++++++++++++++++++++++++++-------------------------- 1 file changed, 69 insertions(+), 67 deletions(-) diff --git a/blockdev.c b/blockdev.c index e25f0b8d2c..8fa9510850 100644 --- a/blockdev.c +++ b/blockdev.c @@ -317,7 +317,6 @@ static DriveInfo *blockdev_init(QDict *bs_opts, const char *serial; const char *mediastr = ""; int bus_id, unit_id; - int cyls, heads, secs, translation; int max_devs; int index; int ro = 0; @@ -335,8 +334,6 @@ static DriveInfo *blockdev_init(QDict *bs_opts, bool has_driver_specific_opts; BlockDriver *drv = NULL; - translation = BIOS_ATA_TRANSLATION_AUTO; - /* Check common options by copying from bs_opts to opts, all other options * stay in bs_opts for processing by bdrv_open(). */ id = qdict_get_try_str(bs_opts, "id"); @@ -365,10 +362,6 @@ static DriveInfo *blockdev_init(QDict *bs_opts, unit_id = qemu_opt_get_number(opts, "unit", -1); index = qemu_opt_get_number(opts, "index", -1); - cyls = qemu_opt_get_number(opts, "cyls", 0); - heads = qemu_opt_get_number(opts, "heads", 0); - secs = qemu_opt_get_number(opts, "secs", 0); - snapshot = qemu_opt_get_bool(opts, "snapshot", 0); ro = qemu_opt_get_bool(opts, "read-only", 0); copy_on_read = qemu_opt_get_bool(opts, "copy-on-read", false); @@ -378,46 +371,6 @@ static DriveInfo *blockdev_init(QDict *bs_opts, max_devs = if_max_devs[type]; - if (cyls || heads || secs) { - if (cyls < 1) { - error_report("invalid physical cyls number"); - return NULL; - } - if (heads < 1) { - error_report("invalid physical heads number"); - return NULL; - } - if (secs < 1) { - error_report("invalid physical secs number"); - return NULL; - } - } - - if ((buf = qemu_opt_get(opts, "trans")) != NULL) { - if (!cyls) { - error_report("'%s' trans must be used with cyls, heads and secs", - buf); - return NULL; - } - if (!strcmp(buf, "none")) - translation = BIOS_ATA_TRANSLATION_NONE; - else if (!strcmp(buf, "lba")) - translation = BIOS_ATA_TRANSLATION_LBA; - else if (!strcmp(buf, "auto")) - translation = BIOS_ATA_TRANSLATION_AUTO; - else { - error_report("'%s' invalid translation type", buf); - return NULL; - } - } - - if (media == MEDIA_CDROM) { - if (cyls || secs || heads) { - error_report("CHS can't be set with media=cdrom"); - return NULL; - } - } - if ((buf = qemu_opt_get(opts, "discard")) != NULL) { if (bdrv_parse_discard_flags(buf, &bdrv_flags) != 0) { error_report("invalid discard option"); @@ -608,10 +561,6 @@ static DriveInfo *blockdev_init(QDict *bs_opts, dinfo->type = type; dinfo->bus = bus_id; dinfo->unit = unit_id; - dinfo->cyls = cyls; - dinfo->heads = heads; - dinfo->secs = secs; - dinfo->trans = translation; dinfo->refcount = 1; if (serial != NULL) { dinfo->serial = g_strdup(serial); @@ -744,6 +693,22 @@ QemuOptsList qemu_legacy_drive_opts = { .name = "if", .type = QEMU_OPT_STRING, .help = "interface (ide, scsi, sd, mtd, floppy, pflash, virtio)", + },{ + .name = "cyls", + .type = QEMU_OPT_NUMBER, + .help = "number of cylinders (ide disk geometry)", + },{ + .name = "heads", + .type = QEMU_OPT_NUMBER, + .help = "number of heads (ide disk geometry)", + },{ + .name = "secs", + .type = QEMU_OPT_NUMBER, + .help = "number of sectors (ide disk geometry)", + },{ + .name = "trans", + .type = QEMU_OPT_STRING, + .help = "chs translation (auto, lba, none)", }, { /* end of list */ } }, @@ -757,6 +722,7 @@ DriveInfo *drive_init(QemuOpts *all_opts, BlockInterfaceType block_default_type) QemuOpts *legacy_opts; DriveMediaType media = MEDIA_DISK; BlockInterfaceType type; + int cyls, heads, secs, translation; Error *local_err = NULL; /* Change legacy command line options into QMP ones */ @@ -846,6 +812,53 @@ DriveInfo *drive_init(QemuOpts *all_opts, BlockInterfaceType block_default_type) type = block_default_type; } + /* Geometry */ + cyls = qemu_opt_get_number(legacy_opts, "cyls", 0); + heads = qemu_opt_get_number(legacy_opts, "heads", 0); + secs = qemu_opt_get_number(legacy_opts, "secs", 0); + + if (cyls || heads || secs) { + if (cyls < 1) { + error_report("invalid physical cyls number"); + goto fail; + } + if (heads < 1) { + error_report("invalid physical heads number"); + goto fail; + } + if (secs < 1) { + error_report("invalid physical secs number"); + goto fail; + } + } + + translation = BIOS_ATA_TRANSLATION_AUTO; + value = qemu_opt_get(legacy_opts, "trans"); + if (value != NULL) { + if (!cyls) { + error_report("'%s' trans must be used with cyls, heads and secs", + value); + goto fail; + } + if (!strcmp(value, "none")) { + translation = BIOS_ATA_TRANSLATION_NONE; + } else if (!strcmp(value, "lba")) { + translation = BIOS_ATA_TRANSLATION_LBA; + } else if (!strcmp(value, "auto")) { + translation = BIOS_ATA_TRANSLATION_AUTO; + } else { + error_report("'%s' invalid translation type", value); + goto fail; + } + } + + if (media == MEDIA_CDROM) { + if (cyls || secs || heads) { + error_report("CHS can't be set with media=cdrom"); + goto fail; + } + } + /* Actual block device init: Functionality shared with blockdev-add */ dinfo = blockdev_init(bs_opts, type, media); if (dinfo == NULL) { @@ -856,6 +869,11 @@ DriveInfo *drive_init(QemuOpts *all_opts, BlockInterfaceType block_default_type) dinfo->enable_auto_del = true; dinfo->opts = all_opts; + dinfo->cyls = cyls; + dinfo->heads = heads; + dinfo->secs = secs; + dinfo->trans = translation; + fail: qemu_opts_del(legacy_opts); return dinfo; @@ -2202,22 +2220,6 @@ QemuOptsList qemu_common_drive_opts = { .name = "index", .type = QEMU_OPT_NUMBER, .help = "index number", - },{ - .name = "cyls", - .type = QEMU_OPT_NUMBER, - .help = "number of cylinders (ide disk geometry)", - },{ - .name = "heads", - .type = QEMU_OPT_NUMBER, - .help = "number of heads (ide disk geometry)", - },{ - .name = "secs", - .type = QEMU_OPT_NUMBER, - .help = "number of sectors (ide disk geometry)", - },{ - .name = "trans", - .type = QEMU_OPT_STRING, - .help = "chs translation (auto, lba. none)", },{ .name = "snapshot", .type = QEMU_OPT_BOOL, From 26929298023b0592dc6ac8bc15163b5a24341670 Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Mon, 9 Sep 2013 17:01:03 +0200 Subject: [PATCH 51/61] blockdev: Move parsing of 'boot' option to drive_init It's already ignored and only prints a deprecation message. No use in making it available in new interfaces. Signed-off-by: Kevin Wolf Reviewed-by: Eric Blake --- blockdev.c | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/blockdev.c b/blockdev.c index 8fa9510850..d633f6ea68 100644 --- a/blockdev.c +++ b/blockdev.c @@ -452,12 +452,6 @@ static DriveInfo *blockdev_init(QDict *bs_opts, return NULL; } - if (qemu_opt_get(opts, "boot") != NULL) { - fprintf(stderr, "qemu-kvm: boot=on|off is deprecated and will be " - "ignored. Future versions will reject this parameter. Please " - "update your scripts.\n"); - } - on_write_error = BLOCKDEV_ON_ERROR_ENOSPC; if ((buf = qemu_opt_get(opts, "werror")) != NULL) { if (type != IF_IDE && type != IF_SCSI && type != IF_VIRTIO && type != IF_NONE) { @@ -709,6 +703,10 @@ QemuOptsList qemu_legacy_drive_opts = { .name = "trans", .type = QEMU_OPT_STRING, .help = "chs translation (auto, lba, none)", + },{ + .name = "boot", + .type = QEMU_OPT_BOOL, + .help = "(deprecated, ignored)", }, { /* end of list */ } }, @@ -784,6 +782,13 @@ DriveInfo *drive_init(QemuOpts *all_opts, BlockInterfaceType block_default_type) goto fail; } + /* Deprecated option boot=[on|off] */ + if (qemu_opt_get(legacy_opts, "boot") != NULL) { + fprintf(stderr, "qemu-kvm: boot=on|off is deprecated and will be " + "ignored. Future versions will reject this parameter. Please " + "update your scripts.\n"); + } + /* Media type */ value = qemu_opt_get(legacy_opts, "media"); if (value) { @@ -2328,10 +2333,6 @@ QemuOptsList qemu_common_drive_opts = { .name = "copy-on-read", .type = QEMU_OPT_BOOL, .help = "copy read data from backing file into image file", - },{ - .name = "boot", - .type = QEMU_OPT_BOOL, - .help = "(deprecated, ignored)", }, { /* end of list */ } }, From 87a899c5090c7864fc7dcff3ea0ac34153ea621b Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Tue, 10 Sep 2013 15:48:13 +0200 Subject: [PATCH 52/61] blockdev: Move bus/unit/index processing to drive_init This requires moving the automatic ID generation at the same time, so let's do that as well. Signed-off-by: Kevin Wolf Reviewed-by: Max Reitz Reviewed-by: Eric Blake --- blockdev.c | 157 +++++++++++++++++++++++++---------------------------- 1 file changed, 73 insertions(+), 84 deletions(-) diff --git a/blockdev.c b/blockdev.c index d633f6ea68..b543f6d0e9 100644 --- a/blockdev.c +++ b/blockdev.c @@ -315,10 +315,6 @@ static DriveInfo *blockdev_init(QDict *bs_opts, const char *buf; const char *file = NULL; const char *serial; - const char *mediastr = ""; - int bus_id, unit_id; - int max_devs; - int index; int ro = 0; int bdrv_flags = 0; int on_read_error, on_write_error; @@ -358,10 +354,6 @@ static DriveInfo *blockdev_init(QDict *bs_opts, has_driver_specific_opts = !!qdict_size(bs_opts); /* extract parameters */ - bus_id = qemu_opt_get_number(opts, "bus", 0); - unit_id = qemu_opt_get_number(opts, "unit", -1); - index = qemu_opt_get_number(opts, "index", -1); - snapshot = qemu_opt_get_bool(opts, "snapshot", 0); ro = qemu_opt_get_bool(opts, "read-only", 0); copy_on_read = qemu_opt_get_bool(opts, "copy-on-read", false); @@ -369,8 +361,6 @@ static DriveInfo *blockdev_init(QDict *bs_opts, file = qemu_opt_get(opts, "file"); serial = qemu_opt_get(opts, "serial"); - max_devs = if_max_devs[type]; - if ((buf = qemu_opt_get(opts, "discard")) != NULL) { if (bdrv_parse_discard_flags(buf, &bdrv_flags) != 0) { error_report("invalid discard option"); @@ -485,66 +475,6 @@ static DriveInfo *blockdev_init(QDict *bs_opts, } } - /* compute bus and unit according index */ - - if (index != -1) { - if (bus_id != 0 || unit_id != -1) { - error_report("index cannot be used with bus and unit"); - return NULL; - } - bus_id = drive_index_to_bus_id(type, index); - unit_id = drive_index_to_unit_id(type, index); - } - - /* if user doesn't specify a unit_id, - * try to find the first free - */ - - if (unit_id == -1) { - unit_id = 0; - while (drive_get(type, bus_id, unit_id) != NULL) { - unit_id++; - if (max_devs && unit_id >= max_devs) { - unit_id -= max_devs; - bus_id++; - } - } - } - - /* check unit id */ - - if (max_devs && unit_id >= max_devs) { - error_report("unit %d too big (max is %d)", - unit_id, max_devs - 1); - return NULL; - } - - /* - * catch multiple definitions - */ - - if (drive_get(type, bus_id, unit_id) != NULL) { - error_report("drive with bus=%d, unit=%d (index=%d) exists", - bus_id, unit_id, index); - return NULL; - } - - /* no id supplied -> create one */ - if (qemu_opts_id(opts) == NULL) { - char *new_id; - if (type == IF_IDE || type == IF_SCSI) { - mediastr = (media == MEDIA_CDROM) ? "-cd" : "-hd"; - } - if (max_devs) { - new_id = g_strdup_printf("%s%i%s%i", if_name[type], bus_id, - mediastr, unit_id); - } else { - new_id = g_strdup_printf("%s%s%i", if_name[type], - mediastr, unit_id); - } - qemu_opts_set_id(opts, new_id); - } - /* init */ dinfo = g_malloc0(sizeof(*dinfo)); dinfo->id = g_strdup(qemu_opts_id(opts)); @@ -553,8 +483,6 @@ static DriveInfo *blockdev_init(QDict *bs_opts, dinfo->bdrv->read_only = ro; dinfo->devaddr = devaddr; dinfo->type = type; - dinfo->bus = bus_id; - dinfo->unit = unit_id; dinfo->refcount = 1; if (serial != NULL) { dinfo->serial = g_strdup(serial); @@ -680,6 +608,18 @@ QemuOptsList qemu_legacy_drive_opts = { .head = QTAILQ_HEAD_INITIALIZER(qemu_legacy_drive_opts.head), .desc = { { + .name = "bus", + .type = QEMU_OPT_NUMBER, + .help = "bus number", + },{ + .name = "unit", + .type = QEMU_OPT_NUMBER, + .help = "unit number (i.e. lun for scsi)", + },{ + .name = "index", + .type = QEMU_OPT_NUMBER, + .help = "index number", + },{ .name = "media", .type = QEMU_OPT_STRING, .help = "media type (disk, cdrom)", @@ -721,6 +661,7 @@ DriveInfo *drive_init(QemuOpts *all_opts, BlockInterfaceType block_default_type) DriveMediaType media = MEDIA_DISK; BlockInterfaceType type; int cyls, heads, secs, translation; + int max_devs, bus_id, unit_id, index; Error *local_err = NULL; /* Change legacy command line options into QMP ones */ @@ -864,6 +805,63 @@ DriveInfo *drive_init(QemuOpts *all_opts, BlockInterfaceType block_default_type) } } + /* Device address specified by bus/unit or index. + * If none was specified, try to find the first free one. */ + bus_id = qemu_opt_get_number(legacy_opts, "bus", 0); + unit_id = qemu_opt_get_number(legacy_opts, "unit", -1); + index = qemu_opt_get_number(legacy_opts, "index", -1); + + max_devs = if_max_devs[type]; + + if (index != -1) { + if (bus_id != 0 || unit_id != -1) { + error_report("index cannot be used with bus and unit"); + goto fail; + } + bus_id = drive_index_to_bus_id(type, index); + unit_id = drive_index_to_unit_id(type, index); + } + + if (unit_id == -1) { + unit_id = 0; + while (drive_get(type, bus_id, unit_id) != NULL) { + unit_id++; + if (max_devs && unit_id >= max_devs) { + unit_id -= max_devs; + bus_id++; + } + } + } + + if (max_devs && unit_id >= max_devs) { + error_report("unit %d too big (max is %d)", unit_id, max_devs - 1); + goto fail; + } + + if (drive_get(type, bus_id, unit_id) != NULL) { + error_report("drive with bus=%d, unit=%d (index=%d) exists", + bus_id, unit_id, index); + goto fail; + } + + /* no id supplied -> create one */ + if (qemu_opts_id(all_opts) == NULL) { + char *new_id; + const char *mediastr = ""; + if (type == IF_IDE || type == IF_SCSI) { + mediastr = (media == MEDIA_CDROM) ? "-cd" : "-hd"; + } + if (max_devs) { + new_id = g_strdup_printf("%s%i%s%i", if_name[type], bus_id, + mediastr, unit_id); + } else { + new_id = g_strdup_printf("%s%s%i", if_name[type], + mediastr, unit_id); + } + qdict_put(bs_opts, "id", qstring_from_str(new_id)); + g_free(new_id); + } + /* Actual block device init: Functionality shared with blockdev-add */ dinfo = blockdev_init(bs_opts, type, media); if (dinfo == NULL) { @@ -879,6 +877,9 @@ DriveInfo *drive_init(QemuOpts *all_opts, BlockInterfaceType block_default_type) dinfo->secs = secs; dinfo->trans = translation; + dinfo->bus = bus_id; + dinfo->unit = unit_id; + fail: qemu_opts_del(legacy_opts); return dinfo; @@ -2214,18 +2215,6 @@ QemuOptsList qemu_common_drive_opts = { .head = QTAILQ_HEAD_INITIALIZER(qemu_common_drive_opts.head), .desc = { { - .name = "bus", - .type = QEMU_OPT_NUMBER, - .help = "bus number", - },{ - .name = "unit", - .type = QEMU_OPT_NUMBER, - .help = "unit number (i.e. lun for scsi)", - },{ - .name = "index", - .type = QEMU_OPT_NUMBER, - .help = "index number", - },{ .name = "snapshot", .type = QEMU_OPT_BOOL, .help = "enable/disable snapshot mode", From 394c7d4d6bd06386308e2fef0cf1c613a10e0d23 Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Fri, 13 Sep 2013 14:09:17 +0200 Subject: [PATCH 53/61] blockdev: Move virtio-blk device creation to drive_init Signed-off-by: Kevin Wolf Reviewed-by: Max Reitz Reviewed-by: Eric Blake --- blockdev.c | 54 +++++++++++++++++++++++++++--------------------------- 1 file changed, 27 insertions(+), 27 deletions(-) diff --git a/blockdev.c b/blockdev.c index b543f6d0e9..b11155caef 100644 --- a/blockdev.c +++ b/blockdev.c @@ -318,7 +318,6 @@ static DriveInfo *blockdev_init(QDict *bs_opts, int ro = 0; int bdrv_flags = 0; int on_read_error, on_write_error; - const char *devaddr; DriveInfo *dinfo; ThrottleConfig cfg; int snapshot = 0; @@ -468,20 +467,12 @@ static DriveInfo *blockdev_init(QDict *bs_opts, } } - if ((devaddr = qemu_opt_get(opts, "addr")) != NULL) { - if (type != IF_VIRTIO) { - error_report("addr is not supported by this bus type"); - return NULL; - } - } - /* init */ dinfo = g_malloc0(sizeof(*dinfo)); dinfo->id = g_strdup(qemu_opts_id(opts)); dinfo->bdrv = bdrv_new(dinfo->id); dinfo->bdrv->open_flags = snapshot ? BDRV_O_SNAPSHOT : 0; dinfo->bdrv->read_only = ro; - dinfo->devaddr = devaddr; dinfo->type = type; dinfo->refcount = 1; if (serial != NULL) { @@ -508,22 +499,8 @@ static DriveInfo *blockdev_init(QDict *bs_opts, case IF_FLOPPY: case IF_PFLASH: case IF_MTD: - break; case IF_VIRTIO: - { - /* add virtio block device */ - QemuOpts *devopts; - devopts = qemu_opts_create_nofail(qemu_find_opts("device")); - if (arch_type == QEMU_ARCH_S390X) { - qemu_opt_set(devopts, "driver", "virtio-blk-s390"); - } else { - qemu_opt_set(devopts, "driver", "virtio-blk-pci"); - } - qemu_opt_set(devopts, "drive", dinfo->id); - if (devaddr) - qemu_opt_set(devopts, "addr", devaddr); break; - } default: abort(); } @@ -647,6 +624,10 @@ QemuOptsList qemu_legacy_drive_opts = { .name = "boot", .type = QEMU_OPT_BOOL, .help = "(deprecated, ignored)", + },{ + .name = "addr", + .type = QEMU_OPT_STRING, + .help = "pci address (virtio only)", }, { /* end of list */ } }, @@ -662,6 +643,7 @@ DriveInfo *drive_init(QemuOpts *all_opts, BlockInterfaceType block_default_type) BlockInterfaceType type; int cyls, heads, secs, translation; int max_devs, bus_id, unit_id, index; + const char *devaddr; Error *local_err = NULL; /* Change legacy command line options into QMP ones */ @@ -862,6 +844,27 @@ DriveInfo *drive_init(QemuOpts *all_opts, BlockInterfaceType block_default_type) g_free(new_id); } + /* Add virtio block device */ + devaddr = qemu_opt_get(legacy_opts, "addr"); + if (devaddr && type != IF_VIRTIO) { + error_report("addr is not supported by this bus type"); + goto fail; + } + + if (type == IF_VIRTIO) { + QemuOpts *devopts; + devopts = qemu_opts_create_nofail(qemu_find_opts("device")); + if (arch_type == QEMU_ARCH_S390X) { + qemu_opt_set(devopts, "driver", "virtio-blk-s390"); + } else { + qemu_opt_set(devopts, "driver", "virtio-blk-pci"); + } + qemu_opt_set(devopts, "drive", qdict_get_str(bs_opts, "id")); + if (devaddr) { + qemu_opt_set(devopts, "addr", devaddr); + } + } + /* Actual block device init: Functionality shared with blockdev-add */ dinfo = blockdev_init(bs_opts, type, media); if (dinfo == NULL) { @@ -879,6 +882,7 @@ DriveInfo *drive_init(QemuOpts *all_opts, BlockInterfaceType block_default_type) dinfo->bus = bus_id; dinfo->unit = unit_id; + dinfo->devaddr = devaddr; fail: qemu_opts_del(legacy_opts); @@ -2258,10 +2262,6 @@ QemuOptsList qemu_common_drive_opts = { .name = "werror", .type = QEMU_OPT_STRING, .help = "write error action", - },{ - .name = "addr", - .type = QEMU_OPT_STRING, - .help = "pci address (virtio only)", },{ .name = "read-only", .type = QEMU_OPT_BOOL, From 4f8a066b5fc254eeaabbbde56ba4f5b29cc68fdf Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Fri, 13 Sep 2013 15:51:47 +0200 Subject: [PATCH 54/61] blockdev: Remove IF_* check for read-only blockdev_init IF_NONE allows read-only, which makes forbidding it in this place for other types pretty much pointless. Instead, make sure that all devices for which the check would have errored out check in their init function that they don't get a read-only BlockDriverState. This catches even cases where IF_NONE and -device is used. Signed-off-by: Kevin Wolf Reviewed-by: Eric Blake --- blockdev.c | 6 ------ hw/block/m25p80.c | 5 +++++ hw/block/xen_disk.c | 5 +++++ hw/sd/milkymist-memcard.c | 4 ++++ hw/sd/omap_mmc.c | 6 ++++++ hw/sd/pl181.c | 4 ++++ hw/sd/pxa2xx_mmci.c | 3 +++ hw/sd/sd.c | 5 +++++ hw/sd/sdhci.c | 3 +++ hw/sd/ssi-sd.c | 3 +++ tests/qemu-iotests/051.out | 5 ++++- 11 files changed, 42 insertions(+), 7 deletions(-) diff --git a/blockdev.c b/blockdev.c index b11155caef..401ee25e02 100644 --- a/blockdev.c +++ b/blockdev.c @@ -528,12 +528,6 @@ static DriveInfo *blockdev_init(QDict *bs_opts, if (media == MEDIA_CDROM) { /* CDROM is fine for any interface, don't check. */ ro = 1; - } else if (ro == 1) { - if (type != IF_SCSI && type != IF_VIRTIO && type != IF_FLOPPY && - type != IF_NONE && type != IF_PFLASH) { - error_report("read-only not supported by this bus type"); - goto err; - } } bdrv_flags |= ro ? 0 : BDRV_O_RDWR; diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c index 8c3b7f0d3b..02a15441fa 100644 --- a/hw/block/m25p80.c +++ b/hw/block/m25p80.c @@ -624,6 +624,11 @@ static int m25p80_init(SSISlave *ss) if (dinfo && dinfo->bdrv) { DB_PRINT_L(0, "Binding to IF_MTD drive\n"); s->bdrv = dinfo->bdrv; + if (bdrv_is_read_only(s->bdrv)) { + fprintf(stderr, "Can't use a read-only drive"); + return 1; + } + /* FIXME: Move to late init */ if (bdrv_read(s->bdrv, 0, s->storage, DIV_ROUND_UP(s->size, BDRV_SECTOR_SIZE))) { diff --git a/hw/block/xen_disk.c b/hw/block/xen_disk.c index 8742294dfb..098f6c62c7 100644 --- a/hw/block/xen_disk.c +++ b/hw/block/xen_disk.c @@ -830,6 +830,11 @@ static int blk_connect(struct XenDevice *xendev) /* setup via qemu cmdline -> already setup for us */ xen_be_printf(&blkdev->xendev, 2, "get configured bdrv (cmdline setup)\n"); blkdev->bs = blkdev->dinfo->bdrv; + if (bdrv_is_read_only(blkdev->bs) && !readonly) { + xen_be_printf(&blkdev->xendev, 0, "Unexpected read-only drive"); + blkdev->bs = NULL; + return -1; + } /* blkdev->bs is not create by us, we get a reference * so we can bdrv_unref() unconditionally */ bdrv_ref(blkdev->bs); diff --git a/hw/sd/milkymist-memcard.c b/hw/sd/milkymist-memcard.c index 42613b3aff..d1168c9e04 100644 --- a/hw/sd/milkymist-memcard.c +++ b/hw/sd/milkymist-memcard.c @@ -255,6 +255,10 @@ static int milkymist_memcard_init(SysBusDevice *dev) dinfo = drive_get_next(IF_SD); s->card = sd_init(dinfo ? dinfo->bdrv : NULL, false); + if (s->card == NULL) { + return -1; + } + s->enabled = dinfo ? bdrv_is_inserted(dinfo->bdrv) : 0; memory_region_init_io(&s->regs_region, OBJECT(s), &memcard_mmio_ops, s, diff --git a/hw/sd/omap_mmc.c b/hw/sd/omap_mmc.c index bf5d1fbf6d..937a47869a 100644 --- a/hw/sd/omap_mmc.c +++ b/hw/sd/omap_mmc.c @@ -593,6 +593,9 @@ struct omap_mmc_s *omap_mmc_init(hwaddr base, /* Instantiate the storage */ s->card = sd_init(bd, false); + if (s->card == NULL) { + exit(1); + } return s; } @@ -618,6 +621,9 @@ struct omap_mmc_s *omap2_mmc_init(struct omap_target_agent_s *ta, /* Instantiate the storage */ s->card = sd_init(bd, false); + if (s->card == NULL) { + exit(1); + } s->cdet = qemu_allocate_irqs(omap_mmc_cover_cb, s, 1)[0]; sd_set_cb(s->card, NULL, s->cdet); diff --git a/hw/sd/pl181.c b/hw/sd/pl181.c index 03875bf6ca..c35896d28c 100644 --- a/hw/sd/pl181.c +++ b/hw/sd/pl181.c @@ -491,6 +491,10 @@ static int pl181_init(SysBusDevice *sbd) qdev_init_gpio_out(dev, s->cardstatus, 2); dinfo = drive_get_next(IF_SD); s->card = sd_init(dinfo ? dinfo->bdrv : NULL, false); + if (s->card == NULL) { + return -1; + } + return 0; } diff --git a/hw/sd/pxa2xx_mmci.c b/hw/sd/pxa2xx_mmci.c index 90c955fe62..b9d8b1a3e1 100644 --- a/hw/sd/pxa2xx_mmci.c +++ b/hw/sd/pxa2xx_mmci.c @@ -539,6 +539,9 @@ PXA2xxMMCIState *pxa2xx_mmci_init(MemoryRegion *sysmem, /* Instantiate the actual storage */ s->card = sd_init(bd, false); + if (s->card == NULL) { + exit(1); + } register_savevm(NULL, "pxa2xx_mmci", 0, 0, pxa2xx_mmci_save, pxa2xx_mmci_load, s); diff --git a/hw/sd/sd.c b/hw/sd/sd.c index 346d86f69c..7380f063f7 100644 --- a/hw/sd/sd.c +++ b/hw/sd/sd.c @@ -494,6 +494,11 @@ SDState *sd_init(BlockDriverState *bs, bool is_spi) { SDState *sd; + if (bdrv_is_read_only(bs)) { + fprintf(stderr, "sd_init: Cannot use read-only drive\n"); + return NULL; + } + sd = (SDState *) g_malloc0(sizeof(SDState)); sd->buf = qemu_blockalign(bs, 512); sd->spi = is_spi; diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c index 1483e196cd..0906a1d62b 100644 --- a/hw/sd/sdhci.c +++ b/hw/sd/sdhci.c @@ -1166,6 +1166,9 @@ static void sdhci_initfn(Object *obj) di = drive_get_next(IF_SD); s->card = sd_init(di ? di->bdrv : NULL, false); + if (s->card == NULL) { + exit(1); + } s->eject_cb = qemu_allocate_irqs(sdhci_insert_eject_cb, s, 1)[0]; s->ro_cb = qemu_allocate_irqs(sdhci_card_readonly_cb, s, 1)[0]; sd_set_cb(s->card, s->ro_cb, s->eject_cb); diff --git a/hw/sd/ssi-sd.c b/hw/sd/ssi-sd.c index d47e2377f9..1bb56c4d54 100644 --- a/hw/sd/ssi-sd.c +++ b/hw/sd/ssi-sd.c @@ -246,6 +246,9 @@ static int ssi_sd_init(SSISlave *dev) s->mode = SSI_SD_CMD; dinfo = drive_get_next(IF_SD); s->sd = sd_init(dinfo ? dinfo->bdrv : NULL, true); + if (s->sd == NULL) { + return -1; + } register_savevm(&dev->qdev, "ssi_sd", -1, 1, ssi_sd_save, ssi_sd_load, s); return 0; } diff --git a/tests/qemu-iotests/051.out b/tests/qemu-iotests/051.out index e58776a046..2839e32807 100644 --- a/tests/qemu-iotests/051.out +++ b/tests/qemu-iotests/051.out @@ -139,7 +139,10 @@ QEMU X.Y.Z monitor - type 'help' for more information (qemu) qququiquit Testing: -drive file=TEST_DIR/t.qcow2,if=ide,readonly=on -QEMU_PROG: -drive file=TEST_DIR/t.qcow2,if=ide,readonly=on: read-only not supported by this bus type +QEMU X.Y.Z monitor - type 'help' for more information +(qemu) QEMU_PROG: Can't use a read-only drive +QEMU_PROG: Device initialization failed. +QEMU_PROG: Initialization of device ide-hd failed Testing: -drive file=TEST_DIR/t.qcow2,if=virtio,readonly=on QEMU X.Y.Z monitor - type 'help' for more information From a9b43397a9782d028f45b63fb4affee164f85948 Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Wed, 18 Sep 2013 16:47:41 +0200 Subject: [PATCH 55/61] qemu-iotests: Check autodel behaviour for device_del Block devices creates with -drive and drive_add should automatically disappear if the guest device is unplugged. blockdev-add ones shouldn't. Signed-off-by: Kevin Wolf Reviewed-by: Max Reitz Reviewed-by: Eric Blake --- tests/qemu-iotests/067 | 133 +++++++++++++++++++++++++++++++ tests/qemu-iotests/067.out | 80 +++++++++++++++++++ tests/qemu-iotests/common.filter | 8 ++ tests/qemu-iotests/group | 1 + 4 files changed, 222 insertions(+) create mode 100755 tests/qemu-iotests/067 create mode 100644 tests/qemu-iotests/067.out diff --git a/tests/qemu-iotests/067 b/tests/qemu-iotests/067 new file mode 100755 index 0000000000..79dc38bc04 --- /dev/null +++ b/tests/qemu-iotests/067 @@ -0,0 +1,133 @@ +#!/bin/bash +# +# Test automatic deletion of BDSes created by -drive/drive_add +# +# Copyright (C) 2013 Red Hat, Inc. +# +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 2 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see . +# + +# creator +owner=kwolf@redhat.com + +seq=`basename $0` +echo "QA output created by $seq" + +here=`pwd` +tmp=/tmp/$$ +status=1 # failure is the default! + +# get standard environment, filters and checks +. ./common.rc +. ./common.filter + +_supported_fmt qcow2 +_supported_proto file +_supported_os Linux + +function do_run_qemu() +{ + echo Testing: "$@" + $QEMU -nographic -qmp stdio -serial none "$@" + echo +} + +function run_qemu() +{ + do_run_qemu "$@" 2>&1 | _filter_testdir | _filter_qmp +} + +size=128M + +_make_test_img $size + +echo +echo === -drive/-device and device_del === +echo + +run_qemu -drive file=$TEST_IMG,format=$IMGFMT,if=none,id=disk -device virtio-blk-pci,drive=disk,id=virtio0 < Date: Thu, 19 Sep 2013 14:24:10 +0200 Subject: [PATCH 56/61] blockdev: Remove 'media' parameter from blockdev_init() The remaining users shouldn't be there with blockdev-add and are easy to move to drive_init(). Bonus bug fix: As a side effect, CD-ROM drives can now use block drivers on the read-only whitelist without explicitly specifying read-only=on, even if a format is explicitly specified. Signed-off-by: Kevin Wolf Reviewed-by: Max Reitz Reviewed-by: Eric Blake --- blockdev.c | 40 +++++++++++++++------------------------- 1 file changed, 15 insertions(+), 25 deletions(-) diff --git a/blockdev.c b/blockdev.c index 401ee25e02..e1ad319281 100644 --- a/blockdev.c +++ b/blockdev.c @@ -309,8 +309,7 @@ typedef enum { MEDIA_DISK, MEDIA_CDROM } DriveMediaType; /* Takes the ownership of bs_opts */ static DriveInfo *blockdev_init(QDict *bs_opts, - BlockInterfaceType type, - DriveMediaType media) + BlockInterfaceType type) { const char *buf; const char *file = NULL; @@ -488,22 +487,6 @@ static DriveInfo *blockdev_init(QDict *bs_opts, bdrv_set_io_limits(dinfo->bdrv, &cfg); } - switch(type) { - case IF_IDE: - case IF_SCSI: - case IF_XEN: - case IF_NONE: - dinfo->media_cd = media == MEDIA_CDROM; - break; - case IF_SD: - case IF_FLOPPY: - case IF_PFLASH: - case IF_MTD: - case IF_VIRTIO: - break; - default: - abort(); - } if (!file || !*file) { if (has_driver_specific_opts) { file = NULL; @@ -525,11 +508,6 @@ static DriveInfo *blockdev_init(QDict *bs_opts, bdrv_flags |= BDRV_O_INCOMING; } - if (media == MEDIA_CDROM) { - /* CDROM is fine for any interface, don't check. */ - ro = 1; - } - bdrv_flags |= ro ? 0 : BDRV_O_RDWR; if (ro && copy_on_read) { @@ -713,6 +691,7 @@ DriveInfo *drive_init(QemuOpts *all_opts, BlockInterfaceType block_default_type) media = MEDIA_DISK; } else if (!strcmp(value, "cdrom")) { media = MEDIA_CDROM; + qdict_put(bs_opts, "read-only", qstring_from_str("on")); } else { error_report("'%s' invalid media", value); goto fail; @@ -860,7 +839,7 @@ DriveInfo *drive_init(QemuOpts *all_opts, BlockInterfaceType block_default_type) } /* Actual block device init: Functionality shared with blockdev-add */ - dinfo = blockdev_init(bs_opts, type, media); + dinfo = blockdev_init(bs_opts, type); if (dinfo == NULL) { goto fail; } @@ -878,6 +857,17 @@ DriveInfo *drive_init(QemuOpts *all_opts, BlockInterfaceType block_default_type) dinfo->unit = unit_id; dinfo->devaddr = devaddr; + switch(type) { + case IF_IDE: + case IF_SCSI: + case IF_XEN: + case IF_NONE: + dinfo->media_cd = media == MEDIA_CDROM; + break; + default: + break; + } + fail: qemu_opts_del(legacy_opts); return dinfo; @@ -2176,7 +2166,7 @@ void qmp_blockdev_add(BlockdevOptions *options, Error **errp) qdict_flatten(qdict); - dinfo = blockdev_init(qdict, IF_NONE, MEDIA_DISK); + dinfo = blockdev_init(qdict, IF_NONE); if (!dinfo) { error_setg(errp, "Could not open image"); goto fail; From 0ebd24e0a203cf2852c310b59fbe050190dc6c8c Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Thu, 19 Sep 2013 15:12:18 +0200 Subject: [PATCH 57/61] blockdev: Don't disable COR automatically with blockdev-add If a read-only device is configured with copy-on-read=on, the old code only prints a warning and automatically disables copy on read. Make it a real error for blockdev-add. Signed-off-by: Kevin Wolf Reviewed-by: Max Reitz Reviewed-by: Eric Blake --- block.c | 9 +++++++-- blockdev.c | 31 +++++++++++++++++++++++++++---- 2 files changed, 34 insertions(+), 6 deletions(-) diff --git a/block.c b/block.c index 84c0eac671..fd05a8008a 100644 --- a/block.c +++ b/block.c @@ -778,8 +778,13 @@ static int bdrv_open_common(BlockDriverState *bs, BlockDriverState *file, } assert(bs->copy_on_read == 0); /* bdrv_new() and bdrv_close() make it so */ - if (!bs->read_only && (flags & BDRV_O_COPY_ON_READ)) { - bdrv_enable_copy_on_read(bs); + if (flags & BDRV_O_COPY_ON_READ) { + if (!bs->read_only) { + bdrv_enable_copy_on_read(bs); + } else { + error_setg(errp, "Can't use copy-on-read on read-only device"); + return -EINVAL; + } } if (filename != NULL) { diff --git a/blockdev.c b/blockdev.c index e1ad319281..1f14514bea 100644 --- a/blockdev.c +++ b/blockdev.c @@ -510,10 +510,6 @@ static DriveInfo *blockdev_init(QDict *bs_opts, bdrv_flags |= ro ? 0 : BDRV_O_RDWR; - if (ro && copy_on_read) { - error_report("warning: disabling copy_on_read on read-only drive"); - } - QINCREF(bs_opts); ret = bdrv_open(dinfo->bdrv, file, bs_opts, bdrv_flags, drv, &error); @@ -601,6 +597,18 @@ QemuOptsList qemu_legacy_drive_opts = { .type = QEMU_OPT_STRING, .help = "pci address (virtio only)", }, + + /* Options that are passed on, but have special semantics with -drive */ + { + .name = "read-only", + .type = QEMU_OPT_BOOL, + .help = "open drive file as read-only", + },{ + .name = "copy-on-read", + .type = QEMU_OPT_BOOL, + .help = "copy read data from backing file into image file", + }, + { /* end of list */ } }, }; @@ -616,6 +624,7 @@ DriveInfo *drive_init(QemuOpts *all_opts, BlockInterfaceType block_default_type) int cyls, heads, secs, translation; int max_devs, bus_id, unit_id, index; const char *devaddr; + bool read_only, copy_on_read; Error *local_err = NULL; /* Change legacy command line options into QMP ones */ @@ -698,6 +707,20 @@ DriveInfo *drive_init(QemuOpts *all_opts, BlockInterfaceType block_default_type) } } + /* copy-on-read is disabled with a warning for read-only devices */ + read_only = qemu_opt_get_bool(legacy_opts, "read-only", false); + copy_on_read = qemu_opt_get_bool(legacy_opts, "copy-on-read", false); + + if (read_only && copy_on_read) { + error_report("warning: disabling copy-on-read on read-only drive"); + copy_on_read = false; + } + + qdict_put(bs_opts, "read-only", + qstring_from_str(read_only ? "on" : "off")); + qdict_put(bs_opts, "copy-on-read", + qstring_from_str(copy_on_read ? "on" :"off")); + /* Controller type */ value = qemu_opt_get(legacy_opts, "if"); if (value) { From b681072d2005911b79835d2a6af208eba3983a48 Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Fri, 20 Sep 2013 11:33:11 +0200 Subject: [PATCH 58/61] blockdev: blockdev_init() error conversion This gives us meaningful error messages for the blockdev-add QMP command. Signed-off-by: Kevin Wolf Reviewed-by: Max Reitz Reviewed-by: Eric Blake --- blockdev.c | 56 ++++++++++++++++++++++++++++++------------------------ 1 file changed, 31 insertions(+), 25 deletions(-) diff --git a/blockdev.c b/blockdev.c index 1f14514bea..4f76e28164 100644 --- a/blockdev.c +++ b/blockdev.c @@ -272,7 +272,7 @@ static void bdrv_put_ref_bh_schedule(BlockDriverState *bs) qemu_bh_schedule(s->bh); } -static int parse_block_error_action(const char *buf, bool is_read) +static int parse_block_error_action(const char *buf, bool is_read, Error **errp) { if (!strcmp(buf, "ignore")) { return BLOCKDEV_ON_ERROR_IGNORE; @@ -283,8 +283,8 @@ static int parse_block_error_action(const char *buf, bool is_read) } else if (!strcmp(buf, "report")) { return BLOCKDEV_ON_ERROR_REPORT; } else { - error_report("'%s' invalid %s error action", - buf, is_read ? "read" : "write"); + error_setg(errp, "'%s' invalid %s error action", + buf, is_read ? "read" : "write"); return -1; } } @@ -309,7 +309,8 @@ typedef enum { MEDIA_DISK, MEDIA_CDROM } DriveMediaType; /* Takes the ownership of bs_opts */ static DriveInfo *blockdev_init(QDict *bs_opts, - BlockInterfaceType type) + BlockInterfaceType type, + Error **errp) { const char *buf; const char *file = NULL; @@ -333,15 +334,13 @@ static DriveInfo *blockdev_init(QDict *bs_opts, id = qdict_get_try_str(bs_opts, "id"); opts = qemu_opts_create(&qemu_common_drive_opts, id, 1, &error); if (error_is_set(&error)) { - qerror_report_err(error); - error_free(error); + error_propagate(errp, error); return NULL; } qemu_opts_absorb_qdict(opts, bs_opts, &error); if (error_is_set(&error)) { - qerror_report_err(error); - error_free(error); + error_propagate(errp, error); return NULL; } @@ -361,7 +360,7 @@ static DriveInfo *blockdev_init(QDict *bs_opts, if ((buf = qemu_opt_get(opts, "discard")) != NULL) { if (bdrv_parse_discard_flags(buf, &bdrv_flags) != 0) { - error_report("invalid discard option"); + error_setg(errp, "invalid discard option"); return NULL; } } @@ -383,7 +382,7 @@ static DriveInfo *blockdev_init(QDict *bs_opts, } else if (!strcmp(buf, "threads")) { /* this is the default */ } else { - error_report("invalid aio option"); + error_setg(errp, "invalid aio option"); return NULL; } } @@ -399,7 +398,7 @@ static DriveInfo *blockdev_init(QDict *bs_opts, drv = bdrv_find_format(buf); if (!drv) { - error_report("'%s' invalid format", buf); + error_setg(errp, "'%s' invalid format", buf); return NULL; } } @@ -435,20 +434,20 @@ static DriveInfo *blockdev_init(QDict *bs_opts, cfg.op_size = qemu_opt_get_number(opts, "throttling.iops-size", 0); if (!check_throttle_config(&cfg, &error)) { - error_report("%s", error_get_pretty(error)); - error_free(error); + error_propagate(errp, error); return NULL; } on_write_error = BLOCKDEV_ON_ERROR_ENOSPC; if ((buf = qemu_opt_get(opts, "werror")) != NULL) { if (type != IF_IDE && type != IF_SCSI && type != IF_VIRTIO && type != IF_NONE) { - error_report("werror is not supported by this bus type"); + error_setg(errp, "werror is not supported by this bus type"); return NULL; } - on_write_error = parse_block_error_action(buf, 0); - if (on_write_error < 0) { + on_write_error = parse_block_error_action(buf, 0, &error); + if (error_is_set(&error)) { + error_propagate(errp, error); return NULL; } } @@ -460,8 +459,9 @@ static DriveInfo *blockdev_init(QDict *bs_opts, return NULL; } - on_read_error = parse_block_error_action(buf, 1); - if (on_read_error < 0) { + on_read_error = parse_block_error_action(buf, 1, &error); + if (error_is_set(&error)) { + error_propagate(errp, error); return NULL; } } @@ -514,8 +514,9 @@ static DriveInfo *blockdev_init(QDict *bs_opts, ret = bdrv_open(dinfo->bdrv, file, bs_opts, bdrv_flags, drv, &error); if (ret < 0) { - error_report("could not open disk image %s: %s", - file ?: dinfo->id, error_get_pretty(error)); + error_setg(errp, "could not open disk image %s: %s", + file ?: dinfo->id, error_get_pretty(error)); + error_free(error); goto err; } @@ -862,9 +863,15 @@ DriveInfo *drive_init(QemuOpts *all_opts, BlockInterfaceType block_default_type) } /* Actual block device init: Functionality shared with blockdev-add */ - dinfo = blockdev_init(bs_opts, type); + dinfo = blockdev_init(bs_opts, type, &local_err); if (dinfo == NULL) { + if (error_is_set(&local_err)) { + qerror_report_err(local_err); + error_free(local_err); + } goto fail; + } else { + assert(!error_is_set(&local_err)); } /* Set legacy DriveInfo fields */ @@ -2155,7 +2162,6 @@ void qmp_blockdev_add(BlockdevOptions *options, Error **errp) QmpOutputVisitor *ov = qmp_output_visitor_new(); QObject *obj; QDict *qdict; - DriveInfo *dinfo; Error *local_err = NULL; /* Require an ID in the top level */ @@ -2189,9 +2195,9 @@ void qmp_blockdev_add(BlockdevOptions *options, Error **errp) qdict_flatten(qdict); - dinfo = blockdev_init(qdict, IF_NONE); - if (!dinfo) { - error_setg(errp, "Could not open image"); + blockdev_init(qdict, IF_NONE, &local_err); + if (error_is_set(&local_err)) { + error_propagate(errp, local_err); goto fail; } From 899f1ae219d5eaa96a53c996026cb0178d62a86d Mon Sep 17 00:00:00 2001 From: Fam Zheng Date: Fri, 11 Oct 2013 19:48:29 +0800 Subject: [PATCH 59/61] vmdk: Fix vmdk_parse_extents An extra 'p++' after while loop when *p == '\n' will move p to unknown data position, risking parsing junk data or memory access violation. Cc: qemu-stable@nongnu.org Signed-off-by: Fam Zheng Signed-off-by: Kevin Wolf --- block/vmdk.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/block/vmdk.c b/block/vmdk.c index 709aa3deb0..5a9f2787f8 100644 --- a/block/vmdk.c +++ b/block/vmdk.c @@ -772,10 +772,13 @@ static int vmdk_parse_extents(const char *desc, BlockDriverState *bs, } next_line: /* move to next line */ - while (*p && *p != '\n') { + while (*p) { + if (*p == '\n') { + p++; + break; + } p++; } - p++; } return 0; } From b543c5cdcb818ffed90cfc97aa8e297214650d84 Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Fri, 11 Oct 2013 14:02:10 +0200 Subject: [PATCH 60/61] qemu-io: Let "open" pass options to block driver Add an option to the open command to specify runtime options for the block driver used. Signed-off-by: Max Reitz Signed-off-by: Kevin Wolf --- qemu-io.c | 39 +++++++++++++++++++++++++++++++-------- 1 file changed, 31 insertions(+), 8 deletions(-) diff --git a/qemu-io.c b/qemu-io.c index f4b8efcceb..3b3340ab1b 100644 --- a/qemu-io.c +++ b/qemu-io.c @@ -16,6 +16,8 @@ #include "qemu-io.h" #include "qemu/main-loop.h" +#include "qemu/option.h" +#include "qemu/config-file.h" #include "block/block_int.h" #include "trace/control.h" @@ -44,7 +46,7 @@ static const cmdinfo_t close_cmd = { .oneline = "close the current open file", }; -static int openfile(char *name, int flags, int growable) +static int openfile(char *name, int flags, int growable, QDict *opts) { Error *local_err = NULL; @@ -54,7 +56,7 @@ static int openfile(char *name, int flags, int growable) } if (growable) { - if (bdrv_file_open(&qemuio_bs, name, NULL, flags, &local_err)) { + if (bdrv_file_open(&qemuio_bs, name, opts, flags, &local_err)) { fprintf(stderr, "%s: can't open device %s: %s\n", progname, name, error_get_pretty(local_err)); error_free(local_err); @@ -63,7 +65,7 @@ static int openfile(char *name, int flags, int growable) } else { qemuio_bs = bdrv_new("hda"); - if (bdrv_open(qemuio_bs, name, NULL, flags, NULL, &local_err) < 0) { + if (bdrv_open(qemuio_bs, name, opts, flags, NULL, &local_err) < 0) { fprintf(stderr, "%s: can't open device %s: %s\n", progname, name, error_get_pretty(local_err)); error_free(local_err); @@ -89,7 +91,8 @@ static void open_help(void) " -r, -- open file read-only\n" " -s, -- use snapshot file\n" " -n, -- disable host cache\n" -" -g, -- allow file to grow (only applies to protocols)" +" -g, -- allow file to grow (only applies to protocols)\n" +" -o, -- options to be given to the block driver" "\n"); } @@ -102,19 +105,30 @@ static const cmdinfo_t open_cmd = { .argmin = 1, .argmax = -1, .flags = CMD_NOFILE_OK, - .args = "[-Crsn] [path]", + .args = "[-Crsn] [-o options] [path]", .oneline = "open the file specified by path", .help = open_help, }; +static QemuOptsList empty_opts = { + .name = "drive", + .head = QTAILQ_HEAD_INITIALIZER(empty_opts.head), + .desc = { + /* no elements => accept any params */ + { /* end of list */ } + }, +}; + static int open_f(BlockDriverState *bs, int argc, char **argv) { int flags = 0; int readonly = 0; int growable = 0; int c; + QemuOpts *qopts; + QDict *opts = NULL; - while ((c = getopt(argc, argv, "snrg")) != EOF) { + while ((c = getopt(argc, argv, "snrgo:")) != EOF) { switch (c) { case 's': flags |= BDRV_O_SNAPSHOT; @@ -128,6 +142,15 @@ static int open_f(BlockDriverState *bs, int argc, char **argv) case 'g': growable = 1; break; + case 'o': + qopts = qemu_opts_parse(&empty_opts, optarg, 0); + if (qopts == NULL) { + printf("could not parse option list -- %s\n", optarg); + return 0; + } + opts = qemu_opts_to_qdict(qopts, opts); + qemu_opts_del(qopts); + break; default: return qemuio_command_usage(&open_cmd); } @@ -141,7 +164,7 @@ static int open_f(BlockDriverState *bs, int argc, char **argv) return qemuio_command_usage(&open_cmd); } - return openfile(argv[optind], flags, growable); + return openfile(argv[optind], flags, growable, opts); } static int quit_f(BlockDriverState *bs, int argc, char **argv) @@ -418,7 +441,7 @@ int main(int argc, char **argv) } if ((argc - optind) == 1) { - openfile(argv[optind], flags, growable); + openfile(argv[optind], flags, growable, NULL); } command_loop(); From 34eeb82de65ce9f83081a3357b0afe80a6a1d86a Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Fri, 11 Oct 2013 14:02:11 +0200 Subject: [PATCH 61/61] qemu-iotests: Add test for inactive L2 overlap Extend 060 by a test which creates a corrupted image with an active L2 entry pointing to an inactive L2 table and writes to the corresponding guest offset. Also, use overlap-check=all for all tests in 060. Signed-off-by: Max Reitz Signed-off-by: Kevin Wolf --- tests/qemu-iotests/060 | 47 ++++++++++++++++++++++++++++++++------ tests/qemu-iotests/060.out | 40 +++++++++++++++++++++++++++++++- 2 files changed, 79 insertions(+), 8 deletions(-) diff --git a/tests/qemu-iotests/060 b/tests/qemu-iotests/060 index 9bbc43b706..bbb19090a1 100755 --- a/tests/qemu-iotests/060 +++ b/tests/qemu-iotests/060 @@ -21,10 +21,10 @@ # creator owner=mreitz@redhat.com -seq=`basename $0` +seq="$(basename $0)" echo "QA output created by $seq" -here=`pwd` +here="$PWD" tmp=/tmp/$$ status=1 # failure is the default! @@ -47,9 +47,15 @@ rt_offset=65536 # 0x10000 (XXX: just an assumption) rb_offset=131072 # 0x20000 (XXX: just an assumption) l1_offset=196608 # 0x30000 (XXX: just an assumption) l2_offset=262144 # 0x40000 (XXX: just an assumption) +l2_offset_after_snapshot=524288 # 0x80000 (XXX: just an assumption) IMGOPTS="compat=1.1" +OPEN_RW="open -o overlap-check=all $TEST_IMG" +# Overlap checks are done before write operations only, therefore opening an +# image read-only makes the overlap-check option irrelevant +OPEN_RO="open -r $TEST_IMG" + echo echo "=== Testing L2 reference into L1 ===" echo @@ -65,16 +71,18 @@ _check_test_img ./qcow2.py "$TEST_IMG" dump-header | grep incompatible_features # Try to write something, thereby forcing the corrupt bit to be set -$QEMU_IO -c "write -P 0x2a 0 512" "$TEST_IMG" | _filter_qemu_io +$QEMU_IO -c "$OPEN_RW" -c "write -P 0x2a 0 512" | _filter_qemu_io # The corrupt bit must now be set ./qcow2.py "$TEST_IMG" dump-header | grep incompatible_features # Try to open the image R/W (which should fail) -$QEMU_IO -c "read 0 512" "$TEST_IMG" 2>&1 | _filter_qemu_io | _filter_testdir | _filter_imgfmt +$QEMU_IO -c "$OPEN_RW" -c "read 0 512" 2>&1 | _filter_qemu_io \ + | _filter_testdir \ + | _filter_imgfmt # Try to open it RO (which should succeed) -$QEMU_IO -c "read 0 512" -r "$TEST_IMG" | _filter_qemu_io +$QEMU_IO -c "$OPEN_RO" -c "read 0 512" | _filter_qemu_io # We could now try to fix the image, but this would probably fail (how should an # L2 table linked onto the L1 table be fixed?) @@ -92,7 +100,7 @@ poke_file "$TEST_IMG" "$(($rb_offset+8))" "\x00\x01" poke_file "$TEST_IMG" "$l2_offset" "\x80\x00\x00\x00\x00\x02\x00\x00" _check_test_img ./qcow2.py "$TEST_IMG" dump-header | grep incompatible_features -$QEMU_IO -c "write -P 0x2a 0 512" "$TEST_IMG" | _filter_qemu_io +$QEMU_IO -c "$OPEN_RW" -c "write -P 0x2a 0 512" | _filter_qemu_io ./qcow2.py "$TEST_IMG" dump-header | grep incompatible_features # Try to fix it @@ -102,9 +110,34 @@ _check_test_img -r all ./qcow2.py "$TEST_IMG" dump-header | grep incompatible_features # Look if it's really really fixed -$QEMU_IO -c "write -P 0x2a 0 512" "$TEST_IMG" | _filter_qemu_io +$QEMU_IO -c "$OPEN_RW" -c "write -P 0x2a 0 512" | _filter_qemu_io ./qcow2.py "$TEST_IMG" dump-header | grep incompatible_features +echo +echo "=== Testing cluster data reference into inactive L2 table ===" +echo +_make_test_img 64M +$QEMU_IO -c "$OPEN_RW" -c "write -P 1 0 512" | _filter_qemu_io +$QEMU_IMG snapshot -c foo "$TEST_IMG" +$QEMU_IO -c "$OPEN_RW" -c "write -P 2 0 512" | _filter_qemu_io +# The inactive L2 table remains at its old offset +poke_file "$TEST_IMG" "$l2_offset_after_snapshot" \ + "\x80\x00\x00\x00\x00\x04\x00\x00" +_check_test_img +./qcow2.py "$TEST_IMG" dump-header | grep incompatible_features +$QEMU_IO -c "$OPEN_RW" -c "write -P 3 0 512" | _filter_qemu_io +./qcow2.py "$TEST_IMG" dump-header | grep incompatible_features +_check_test_img -r all +./qcow2.py "$TEST_IMG" dump-header | grep incompatible_features +$QEMU_IO -c "$OPEN_RW" -c "write -P 4 0 512" | _filter_qemu_io +./qcow2.py "$TEST_IMG" dump-header | grep incompatible_features + +# Check data +$QEMU_IO -c "$OPEN_RO" -c "read -P 4 0 512" | _filter_qemu_io +$QEMU_IMG snapshot -a foo "$TEST_IMG" +_check_test_img +$QEMU_IO -c "$OPEN_RO" -c "read -P 1 0 512" | _filter_qemu_io + # success, all done echo "*** done" rm -f $seq.full diff --git a/tests/qemu-iotests/060.out b/tests/qemu-iotests/060.out index 648f7437a2..6c7bdbb2f2 100644 --- a/tests/qemu-iotests/060.out +++ b/tests/qemu-iotests/060.out @@ -12,7 +12,6 @@ qcow2: Preventing invalid write on metadata (overlaps with active L1 table); ima write failed: Input/output error incompatible_features 0x2 qemu-io: can't open device TEST_DIR/t.IMGFMT: IMGFMT: Image is corrupt; cannot be opened read/write -no file open, try 'help open' read 512/512 bytes at offset 0 512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) @@ -40,4 +39,43 @@ incompatible_features 0x0 wrote 512/512 bytes at offset 0 512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) incompatible_features 0x0 + +=== Testing cluster data reference into inactive L2 table === + +Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864 +wrote 512/512 bytes at offset 0 +512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) +wrote 512/512 bytes at offset 0 +512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) +ERROR cluster 4 refcount=1 reference=2 +Leaked cluster 9 refcount=1 reference=0 + +1 errors were found on the image. +Data may be corrupted, or further writes to the image may corrupt it. + +1 leaked clusters were found on the image. +This means waste of disk space, but no harm to data. +incompatible_features 0x0 +qcow2: Preventing invalid write on metadata (overlaps with inactive L2 table); image marked as corrupt. +write failed: Input/output error +incompatible_features 0x2 +Repairing cluster 4 refcount=1 reference=2 +Repairing cluster 9 refcount=1 reference=0 +Repairing OFLAG_COPIED data cluster: l2_entry=8000000000040000 refcount=2 +The following inconsistencies were found and repaired: + + 1 leaked clusters + 2 corruptions + +Double checking the fixed image now... +No errors were found on the image. +incompatible_features 0x0 +wrote 512/512 bytes at offset 0 +512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) +incompatible_features 0x0 +read 512/512 bytes at offset 0 +512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) +No errors were found on the image. +read 512/512 bytes at offset 0 +512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) *** done