diff options
35 files changed, 185 insertions, 302 deletions
diff --git a/block.c b/block.c index 39f373e035..6161dbe3eb 100644 --- a/block.c +++ b/block.c @@ -2584,6 +2584,7 @@ static BlockDriverState *bdrv_open_inherit(const char *filename, BlockBackend *file = NULL; BlockDriverState *bs; BlockDriver *drv = NULL; + BdrvChild *child; const char *drvname; const char *backing; Error *local_err = NULL; @@ -2767,6 +2768,15 @@ static BlockDriverState *bdrv_open_inherit(const char *filename, } } + /* Remove all children options from bs->options and bs->explicit_options */ + QLIST_FOREACH(child, &bs->children, next) { + char *child_key_dot; + child_key_dot = g_strdup_printf("%s.", child->name); + qdict_extract_subqdict(bs->explicit_options, NULL, child_key_dot); + qdict_extract_subqdict(bs->options, NULL, child_key_dot); + g_free(child_key_dot); + } + bdrv_refresh_filename(bs); /* Check if any unknown options were used */ @@ -2976,6 +2986,7 @@ static BlockReopenQueue *bdrv_reopen_queue_child(BlockReopenQueue *bs_queue, } child_key_dot = g_strdup_printf("%s.", child->name); + qdict_extract_subqdict(explicit_options, NULL, child_key_dot); qdict_extract_subqdict(options, &new_child_options, child_key_dot); g_free(child_key_dot); @@ -3039,12 +3050,13 @@ int bdrv_reopen_multiple(AioContext *ctx, BlockReopenQueue *bs_queue, Error **er cleanup: QSIMPLEQ_FOREACH_SAFE(bs_entry, bs_queue, entry, next) { - if (ret && bs_entry->prepared) { - bdrv_reopen_abort(&bs_entry->state); - } else if (ret) { + if (ret) { + if (bs_entry->prepared) { + bdrv_reopen_abort(&bs_entry->state); + } qobject_unref(bs_entry->state.explicit_options); + qobject_unref(bs_entry->state.options); } - qobject_unref(bs_entry->state.options); g_free(bs_entry); } g_free(bs_queue); @@ -3144,6 +3156,7 @@ int bdrv_reopen_prepare(BDRVReopenState *reopen_state, BlockReopenQueue *queue, Error *local_err = NULL; BlockDriver *drv; QemuOpts *opts; + QDict *orig_reopen_opts; const char *value; bool read_only; @@ -3151,6 +3164,11 @@ int bdrv_reopen_prepare(BDRVReopenState *reopen_state, BlockReopenQueue *queue, assert(reopen_state->bs->drv != NULL); drv = reopen_state->bs->drv; + /* This function and each driver's bdrv_reopen_prepare() remove + * entries from reopen_state->options as they are processed, so + * we need to make a copy of the original QDict. */ + orig_reopen_opts = qdict_clone_shallow(reopen_state->options); + /* Process generic block layer options */ opts = qemu_opts_create(&bdrv_runtime_opts, NULL, 0, &error_abort); qemu_opts_absorb_qdict(opts, reopen_state->options, &local_err); @@ -3257,8 +3275,13 @@ int bdrv_reopen_prepare(BDRVReopenState *reopen_state, BlockReopenQueue *queue, ret = 0; + /* Restore the original reopen_state->options QDict */ + qobject_unref(reopen_state->options); + reopen_state->options = qobject_ref(orig_reopen_opts); + error: qemu_opts_del(opts); + qobject_unref(orig_reopen_opts); return ret; } @@ -3288,8 +3311,10 @@ void bdrv_reopen_commit(BDRVReopenState *reopen_state) /* set BDS specific flags now */ qobject_unref(bs->explicit_options); + qobject_unref(bs->options); bs->explicit_options = reopen_state->explicit_options; + bs->options = reopen_state->options; bs->open_flags = reopen_state->flags; bs->read_only = !(reopen_state->flags & BDRV_O_RDWR); @@ -3330,8 +3355,6 @@ void bdrv_reopen_abort(BDRVReopenState *reopen_state) drv->bdrv_reopen_abort(reopen_state); } - qobject_unref(reopen_state->explicit_options); - bdrv_abort_perm_update(reopen_state->bs); } @@ -3349,7 +3372,9 @@ static void bdrv_close(BlockDriverState *bs) bdrv_drain(bs); /* in case flush left pending I/O */ if (bs->drv) { - bs->drv->bdrv_close(bs); + if (bs->drv->bdrv_close) { + bs->drv->bdrv_close(bs); + } bs->drv = NULL; } @@ -5125,16 +5150,13 @@ static bool append_open_options(QDict *d, BlockDriverState *bs) QemuOptDesc *desc; BdrvChild *child; bool found_any = false; - const char *p; for (entry = qdict_first(bs->options); entry; entry = qdict_next(bs->options, entry)) { - /* Exclude options for children */ + /* Exclude node-name references to children */ QLIST_FOREACH(child, &bs->children, next) { - if (strstart(qdict_entry_key(entry), child->name, &p) - && (!*p || *p == '.')) - { + if (!strcmp(entry->key, child->name)) { break; } } diff --git a/block/blkreplay.c b/block/blkreplay.c index 766150ade6..b5d9efdeca 100755 --- a/block/blkreplay.c +++ b/block/blkreplay.c @@ -43,10 +43,6 @@ fail: return ret; } -static void blkreplay_close(BlockDriverState *bs) -{ -} - static int64_t blkreplay_getlength(BlockDriverState *bs) { return bdrv_getlength(bs->file->bs); @@ -135,7 +131,6 @@ static BlockDriver bdrv_blkreplay = { .instance_size = 0, .bdrv_open = blkreplay_open, - .bdrv_close = blkreplay_close, .bdrv_child_perm = bdrv_filter_default_perms, .bdrv_getlength = blkreplay_getlength, diff --git a/block/block-backend.c b/block/block-backend.c index f2f75a977d..fa120630be 100644 --- a/block/block-backend.c +++ b/block/block-backend.c @@ -419,7 +419,6 @@ static void drive_info_del(DriveInfo *dinfo) return; } qemu_opts_del(dinfo->opts); - g_free(dinfo->serial); g_free(dinfo); } diff --git a/block/commit.c b/block/commit.c index e1814d9693..eb414579bd 100644 --- a/block/commit.c +++ b/block/commit.c @@ -239,10 +239,6 @@ static void bdrv_commit_top_refresh_filename(BlockDriverState *bs, QDict *opts) bs->backing->bs->filename); } -static void bdrv_commit_top_close(BlockDriverState *bs) -{ -} - static void bdrv_commit_top_child_perm(BlockDriverState *bs, BdrvChild *c, const BdrvChildRole *role, BlockReopenQueue *reopen_queue, @@ -260,7 +256,6 @@ static BlockDriver bdrv_commit_top = { .bdrv_co_preadv = bdrv_commit_top_preadv, .bdrv_co_block_status = bdrv_co_block_status_from_backing, .bdrv_refresh_filename = bdrv_commit_top_refresh_filename, - .bdrv_close = bdrv_commit_top_close, .bdrv_child_perm = bdrv_commit_top_child_perm, }; diff --git a/block/copy-on-read.c b/block/copy-on-read.c index a19164f9eb..64dcc424b5 100644 --- a/block/copy-on-read.c +++ b/block/copy-on-read.c @@ -45,11 +45,6 @@ static int cor_open(BlockDriverState *bs, QDict *options, int flags, } -static void cor_close(BlockDriverState *bs) -{ -} - - #define PERM_PASSTHROUGH (BLK_PERM_CONSISTENT_READ \ | BLK_PERM_WRITE \ | BLK_PERM_RESIZE) @@ -143,7 +138,6 @@ BlockDriver bdrv_copy_on_read = { .format_name = "copy-on-read", .bdrv_open = cor_open, - .bdrv_close = cor_close, .bdrv_child_perm = cor_child_perm, .bdrv_getlength = cor_getlength, diff --git a/block/crypto.c b/block/crypto.c index 146d81c90a..33ee01bebd 100644 --- a/block/crypto.c +++ b/block/crypto.c @@ -627,7 +627,9 @@ BlockDriver bdrv_crypto_luks = { .bdrv_probe = block_crypto_probe_luks, .bdrv_open = block_crypto_open_luks, .bdrv_close = block_crypto_close, - .bdrv_child_perm = bdrv_format_default_perms, + /* This driver doesn't modify LUKS metadata except when creating image. + * Allow share-rw=on as a special case. */ + .bdrv_child_perm = bdrv_filter_default_perms, .bdrv_co_create = block_crypto_co_create_luks, .bdrv_co_create_opts = block_crypto_co_create_opts_luks, .bdrv_co_truncate = block_crypto_co_truncate, diff --git a/block/mirror.c b/block/mirror.c index b48c3f8cf5..6cc10df5c9 100644 --- a/block/mirror.c +++ b/block/mirror.c @@ -1426,10 +1426,6 @@ static void bdrv_mirror_top_refresh_filename(BlockDriverState *bs, QDict *opts) bs->backing->bs->filename); } -static void bdrv_mirror_top_close(BlockDriverState *bs) -{ -} - static void bdrv_mirror_top_child_perm(BlockDriverState *bs, BdrvChild *c, const BdrvChildRole *role, BlockReopenQueue *reopen_queue, @@ -1456,7 +1452,6 @@ static BlockDriver bdrv_mirror_top = { .bdrv_co_flush = bdrv_mirror_top_flush, .bdrv_co_block_status = bdrv_co_block_status_from_backing, .bdrv_refresh_filename = bdrv_mirror_top_refresh_filename, - .bdrv_close = bdrv_mirror_top_close, .bdrv_child_perm = bdrv_mirror_top_child_perm, }; @@ -1499,6 +1494,11 @@ static void mirror_start_job(const char *job_id, BlockDriverState *bs, buf_size = DEFAULT_MIRROR_BUF_SIZE; } + if (bs == target) { + error_setg(errp, "Can't mirror node into itself"); + return; + } + /* In the case of active commit, add dummy driver to provide consistent * reads on the top, while disabling it in the intermediate nodes, and make * the backing chain writable. */ diff --git a/block/null.c b/block/null.c index 5d610fdfba..d442d3e901 100644 --- a/block/null.c +++ b/block/null.c @@ -97,10 +97,6 @@ static int null_file_open(BlockDriverState *bs, QDict *options, int flags, return ret; } -static void null_close(BlockDriverState *bs) -{ -} - static int64_t null_getlength(BlockDriverState *bs) { BDRVNullState *s = bs->opaque; @@ -263,7 +259,6 @@ static BlockDriver bdrv_null_co = { .bdrv_file_open = null_file_open, .bdrv_parse_filename = null_co_parse_filename, - .bdrv_close = null_close, .bdrv_getlength = null_getlength, .bdrv_co_preadv = null_co_preadv, @@ -283,7 +278,6 @@ static BlockDriver bdrv_null_aio = { .bdrv_file_open = null_file_open, .bdrv_parse_filename = null_aio_parse_filename, - .bdrv_close = null_close, .bdrv_getlength = null_getlength, .bdrv_aio_preadv = null_aio_preadv, diff --git a/block/qapi.c b/block/qapi.c index 339727f0f4..c66f949db8 100644 --- a/block/qapi.c +++ b/block/qapi.c @@ -594,7 +594,7 @@ BlockStatsList *qmp_query_blockstats(bool has_query_nodes, } } else { for (blk = blk_all_next(NULL); blk; blk = blk_all_next(blk)) { - BlockStatsList *info = g_malloc0(sizeof(*info)); + BlockStatsList *info; AioContext *ctx = blk_get_aio_context(blk); BlockStats *s; char *qdev; @@ -619,6 +619,7 @@ BlockStatsList *qmp_query_blockstats(bool has_query_nodes, bdrv_query_blk_stats(s->stats, blk); aio_context_release(ctx); + info = g_malloc0(sizeof(*info)); info->value = s; *p_next = info; p_next = &info->next; diff --git a/block/raw-format.c b/block/raw-format.c index 2fd69cdb08..6f6dc99b2c 100644 --- a/block/raw-format.c +++ b/block/raw-format.c @@ -459,10 +459,6 @@ static int raw_open(BlockDriverState *bs, QDict *options, int flags, return 0; } -static void raw_close(BlockDriverState *bs) -{ -} - static int raw_probe(const uint8_t *buf, int buf_size, const char *filename) { /* smallest possible positive score so that raw is used if and only if no @@ -543,7 +539,6 @@ BlockDriver bdrv_raw = { .bdrv_reopen_commit = &raw_reopen_commit, .bdrv_reopen_abort = &raw_reopen_abort, .bdrv_open = &raw_open, - .bdrv_close = &raw_close, .bdrv_child_perm = bdrv_filter_default_perms, .bdrv_co_create_opts = &raw_co_create_opts, .bdrv_co_preadv = &raw_co_preadv, diff --git a/block/snapshot.c b/block/snapshot.c index f9903bc94e..3218a542df 100644 --- a/block/snapshot.c +++ b/block/snapshot.c @@ -218,7 +218,9 @@ int bdrv_snapshot_goto(BlockDriverState *bs, qobject_unref(file_options); qdict_put_str(options, "file", bdrv_get_node_name(file)); - drv->bdrv_close(bs); + if (drv->bdrv_close) { + drv->bdrv_close(bs); + } bdrv_unref_child(bs, bs->file); bs->file = NULL; diff --git a/block/throttle-groups.c b/block/throttle-groups.c index e297b04e17..5d8213a443 100644 --- a/block/throttle-groups.c +++ b/block/throttle-groups.c @@ -36,6 +36,7 @@ static void throttle_group_obj_init(Object *obj); static void throttle_group_obj_complete(UserCreatable *obj, Error **errp); +static void timer_cb(ThrottleGroupMember *tgm, bool is_write); /* The ThrottleGroup structure (with its ThrottleState) is shared * among different ThrottleGroupMembers and it's independent from @@ -221,6 +222,15 @@ static ThrottleGroupMember *next_throttle_token(ThrottleGroupMember *tgm, ThrottleGroup *tg = container_of(ts, ThrottleGroup, ts); ThrottleGroupMember *token, *start; + /* If this member has its I/O limits disabled then it means that + * it's being drained. Skip the round-robin search and return tgm + * immediately if it has pending requests. Otherwise we could be + * forcing it to wait for other member's throttled requests. */ + if (tgm_has_pending_reqs(tgm, is_write) && + atomic_read(&tgm->io_limits_disabled)) { + return tgm; + } + start = token = tg->tokens[is_write]; /* get next bs round in round robin style */ @@ -415,15 +425,31 @@ static void throttle_group_restart_queue(ThrottleGroupMember *tgm, bool is_write rd->tgm = tgm; rd->is_write = is_write; + /* This function is called when a timer is fired or when + * throttle_group_restart_tgm() is called. Either way, there can + * be no timer pending on this tgm at this point */ + assert(!timer_pending(tgm->throttle_timers.timers[is_write])); + co = qemu_coroutine_create(throttle_group_restart_queue_entry, rd); aio_co_enter(tgm->aio_context, co); } void throttle_group_restart_tgm(ThrottleGroupMember *tgm) { + int i; + if (tgm->throttle_state) { - throttle_group_restart_queue(tgm, 0); - throttle_group_restart_queue(tgm, 1); + for (i = 0; i < 2; i++) { + QEMUTimer *t = tgm->throttle_timers.timers[i]; + if (timer_pending(t)) { + /* If there's a pending timer on this tgm, fire it now */ + timer_del(t); + timer_cb(tgm, i); + } else { + /* Else run the next request from the queue manually */ + throttle_group_restart_queue(tgm, i); + } + } } } @@ -558,16 +584,11 @@ void throttle_group_unregister_tgm(ThrottleGroupMember *tgm) return; } - assert(tgm->pending_reqs[0] == 0 && tgm->pending_reqs[1] == 0); - assert(qemu_co_queue_empty(&tgm->throttled_reqs[0])); - assert(qemu_co_queue_empty(&tgm->throttled_reqs[1])); - qemu_mutex_lock(&tg->lock); for (i = 0; i < 2; i++) { - if (timer_pending(tgm->throttle_timers.timers[i])) { - tg->any_timer_armed[i] = false; - schedule_next_request(tgm, i); - } + assert(tgm->pending_reqs[i] == 0); + assert(qemu_co_queue_empty(&tgm->throttled_reqs[i])); + assert(!timer_pending(tgm->throttle_timers.timers[i])); if (tg->tokens[i] == tgm) { token = throttle_group_next_tgm(tgm); /* Take care of the case where this is the last tgm in the group */ diff --git a/blockdev.c b/blockdev.c index dcf8c8d2ab..72f5347df5 100644 --- a/blockdev.c +++ b/blockdev.c @@ -731,30 +731,6 @@ QemuOptsList qemu_legacy_drive_opts = { .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)", - },{ - .name = "addr", - .type = QEMU_OPT_STRING, - .help = "pci address (virtio only)", - },{ - .name = "serial", - .type = QEMU_OPT_STRING, - .help = "disk serial number", - },{ .name = "file", .type = QEMU_OPT_STRING, .help = "file name", @@ -792,19 +768,13 @@ DriveInfo *drive_new(QemuOpts *all_opts, BlockInterfaceType block_default_type) QemuOpts *legacy_opts; DriveMediaType media = MEDIA_DISK; BlockInterfaceType type; - int cyls, heads, secs, translation; int max_devs, bus_id, unit_id, index; - const char *devaddr; const char *werror, *rerror; bool read_only = false; bool copy_on_read; - const char *serial; const char *filename; Error *local_err = NULL; int i; - const char *deprecated[] = { - "serial", "trans", "secs", "heads", "cyls", "addr" - }; /* Change legacy command line options into QMP ones */ static const struct { @@ -881,16 +851,6 @@ DriveInfo *drive_new(QemuOpts *all_opts, BlockInterfaceType block_default_type) goto fail; } - /* Other deprecated options */ - if (!qtest_enabled()) { - for (i = 0; i < ARRAY_SIZE(deprecated); i++) { - if (qemu_opt_get(legacy_opts, deprecated[i]) != NULL) { - error_report("'%s' is deprecated, please use the corresponding " - "option of '-device' instead", deprecated[i]); - } - } - } - /* Media type */ value = qemu_opt_get(legacy_opts, "media"); if (value) { @@ -932,57 +892,6 @@ DriveInfo *drive_new(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, "large")) { - translation = BIOS_ATA_TRANSLATION_LARGE; - } else if (!strcmp(value, "rechs")) { - translation = BIOS_ATA_TRANSLATION_RECHS; - } 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; - } - } - /* 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); @@ -1022,9 +931,6 @@ DriveInfo *drive_new(QemuOpts *all_opts, BlockInterfaceType block_default_type) goto fail; } - /* Serial number */ - serial = qemu_opt_get(legacy_opts, "serial"); - /* no id supplied -> create one */ if (qemu_opts_id(all_opts) == NULL) { char *new_id; @@ -1044,12 +950,6 @@ DriveInfo *drive_new(QemuOpts *all_opts, BlockInterfaceType block_default_type) } /* 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(qemu_find_opts("device"), NULL, 0, @@ -1061,9 +961,6 @@ DriveInfo *drive_new(QemuOpts *all_opts, BlockInterfaceType block_default_type) } qemu_opt_set(devopts, "drive", qdict_get_str(bs_opts, "id"), &error_abort); - if (devaddr) { - qemu_opt_set(devopts, "addr", devaddr, &error_abort); - } } filename = qemu_opt_get(legacy_opts, "file"); @@ -1105,16 +1002,9 @@ DriveInfo *drive_new(QemuOpts *all_opts, BlockInterfaceType block_default_type) dinfo = g_malloc0(sizeof(*dinfo)); dinfo->opts = all_opts; - dinfo->cyls = cyls; - dinfo->heads = heads; - dinfo->secs = secs; - dinfo->trans = translation; - dinfo->type = type; dinfo->bus = bus_id; dinfo->unit = unit_id; - dinfo->devaddr = devaddr; - dinfo->serial = g_strdup(serial); blk_set_legacy_dinfo(blk, dinfo); diff --git a/device-hotplug.c b/device-hotplug.c index 23fd6656f1..cd427e2c76 100644 --- a/device-hotplug.c +++ b/device-hotplug.c @@ -69,10 +69,6 @@ void hmp_drive_add(Monitor *mon, const QDict *qdict) if (!dinfo) { goto err; } - if (dinfo->devaddr) { - monitor_printf(mon, "Parameter addr not supported\n"); - goto err; - } switch (dinfo->type) { case IF_NONE: diff --git a/hmp-commands.hx b/hmp-commands.hx index 91dfe51c37..c1fc747403 100644 --- a/hmp-commands.hx +++ b/hmp-commands.hx @@ -1306,7 +1306,6 @@ ETEXI .params = "[-n] [[<domain>:]<bus>:]<slot>\n" "[file=file][,if=type][,bus=n]\n" "[,unit=m][,media=d][,index=i]\n" - "[,cyls=c,heads=h,secs=s[,trans=t]]\n" "[,snapshot=on|off][,cache=on|off]\n" "[,readonly=on|off][,copy-on-read=on|off]", .help = "add drive to PCI storage controller", diff --git a/hw/block/block.c b/hw/block/block.c index b91e2b6d7e..cf0eb826f1 100644 --- a/hw/block/block.c +++ b/hw/block/block.c @@ -15,19 +15,6 @@ #include "qapi/qapi-types-block.h" #include "qemu/error-report.h" -void blkconf_serial(BlockConf *conf, char **serial) -{ - DriveInfo *dinfo; - - if (!*serial) { - /* try to fall back to value set with legacy -drive serial=... */ - dinfo = blk_legacy_dinfo(conf->blk); - if (dinfo) { - *serial = g_strdup(dinfo->serial); - } - } -} - void blkconf_blocksizes(BlockConf *conf) { BlockBackend *blk = conf->blk; @@ -108,20 +95,6 @@ bool blkconf_geometry(BlockConf *conf, int *ptrans, unsigned cyls_max, unsigned heads_max, unsigned secs_max, Error **errp) { - DriveInfo *dinfo; - - if (!conf->cyls && !conf->heads && !conf->secs) { - /* try to fall back to value set with legacy -drive cyls=... */ - dinfo = blk_legacy_dinfo(conf->blk); - if (dinfo) { - conf->cyls = dinfo->cyls; - conf->heads = dinfo->heads; - conf->secs = dinfo->secs; - if (ptrans) { - *ptrans = dinfo->trans; - } - } - } if (!conf->cyls && !conf->heads && !conf->secs) { hd_geometry_guess(conf->blk, &conf->cyls, &conf->heads, &conf->secs, diff --git a/hw/block/nvme.c b/hw/block/nvme.c index 5e508ab1b3..fc7dacb816 100644 --- a/hw/block/nvme.c +++ b/hw/block/nvme.c @@ -1217,7 +1217,6 @@ static void nvme_realize(PCIDevice *pci_dev, Error **errp) return; } - blkconf_serial(&n->conf, &n->serial); if (!n->serial) { error_setg(errp, "serial property not set"); return; diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c index 50b5c869e3..225fe44b7a 100644 --- a/hw/block/virtio-blk.c +++ b/hw/block/virtio-blk.c @@ -935,7 +935,6 @@ static void virtio_blk_device_realize(DeviceState *dev, Error **errp) return; } - blkconf_serial(&conf->conf, &conf->serial); if (!blkconf_apply_backend_options(&conf->conf, blk_is_read_only(conf->conf.blk), true, errp)) { diff --git a/hw/ide/qdev.c b/hw/ide/qdev.c index f395d24592..573b022e1e 100644 --- a/hw/ide/qdev.c +++ b/hw/ide/qdev.c @@ -188,7 +188,6 @@ static void ide_dev_initfn(IDEDevice *dev, IDEDriveKind kind, Error **errp) return; } - blkconf_serial(&dev->conf, &dev->serial); if (kind != IDE_CD) { if (!blkconf_geometry(&dev->conf, &dev->chs_trans, 65535, 16, 255, errp)) { diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c index 5bb390773b..5ae7baa082 100644 --- a/hw/scsi/scsi-disk.c +++ b/hw/scsi/scsi-disk.c @@ -2379,7 +2379,6 @@ static void scsi_realize(SCSIDevice *dev, Error **errp) return; } - blkconf_serial(&s->qdev.conf, &s->serial); blkconf_blocksizes(&s->qdev.conf); if (s->qdev.conf.logical_block_size > diff --git a/hw/usb/dev-storage.c b/hw/usb/dev-storage.c index 45a9487cdb..cd5551d94f 100644 --- a/hw/usb/dev-storage.c +++ b/hw/usb/dev-storage.c @@ -599,7 +599,6 @@ static void usb_msd_storage_realize(USBDevice *dev, Error **errp) return; } - blkconf_serial(&s->conf, &dev->serial); blkconf_blocksizes(&s->conf); if (!blkconf_apply_backend_options(&s->conf, blk_is_read_only(blk), true, errp)) { diff --git a/include/hw/block/block.h b/include/hw/block/block.h index d4f4dfffab..e9f9e2223f 100644 --- a/include/hw/block/block.h +++ b/include/hw/block/block.h @@ -72,7 +72,6 @@ static inline unsigned int get_physical_block_exp(BlockConf *conf) /* Configuration helpers */ -void blkconf_serial(BlockConf *conf, char **serial); bool blkconf_geometry(BlockConf *conf, int *trans, unsigned cyls_max, unsigned heads_max, unsigned secs_max, Error **errp); diff --git a/include/sysemu/blockdev.h b/include/sysemu/blockdev.h index ac22f2ae1f..24954b94e0 100644 --- a/include/sysemu/blockdev.h +++ b/include/sysemu/blockdev.h @@ -28,16 +28,13 @@ typedef enum { } BlockInterfaceType; struct DriveInfo { - const char *devaddr; BlockInterfaceType type; int bus; int unit; int auto_del; /* see blockdev_mark_auto_del() */ bool is_default; /* Added by default_drive() ? */ int media_cd; - int cyls, heads, secs, trans; QemuOpts *opts; - char *serial; QTAILQ_ENTRY(DriveInfo) next; }; diff --git a/qapi/block-core.json b/qapi/block-core.json index 5b9084a394..4c7a37afdc 100644 --- a/qapi/block-core.json +++ b/qapi/block-core.json @@ -1499,11 +1499,8 @@ # autogenerated. (Since: 2.9) # # Returns: Nothing on success -# If commit or stream is already active on this device, DeviceInUse # If @device does not exist, DeviceNotFound -# If image commit is not supported by this device, NotSupported -# If @base or @top is invalid, a generic error is returned -# If @speed is invalid, InvalidParameter +# Any other error returns a GenericError. # # Since: 1.3 # @@ -3572,6 +3569,9 @@ # @driver: block driver name # @node-name: the node name of the new node (Since 2.0). # This option is required on the top level of blockdev-add. +# Valid node names start with an alphabetic character and may +# contain only alphanumeric characters, '-', '.' and '_'. Their +# maximum length is 31 characters. # @discard: discard-related options (default: ignore) # @cache: cache-related options # @read-only: whether the block device should be read-only (default: false). diff --git a/qemu-deprecated.texi b/qemu-deprecated.texi index 9920a85adc..df319cfd82 100644 --- a/qemu-deprecated.texi +++ b/qemu-deprecated.texi @@ -94,21 +94,6 @@ with ``-device ...,netdev=x''), or ``-nic user,smb=/some/dir'' (for embedded NICs). The new syntax allows different settings to be provided per NIC. -@subsection -drive cyls=...,heads=...,secs=...,trans=... (since 2.10.0) - -The drive geometry arguments are replaced by the the geometry arguments -that can be specified with the ``-device'' parameter. - -@subsection -drive serial=... (since 2.10.0) - -The drive serial argument is replaced by the the serial argument -that can be specified with the ``-device'' parameter. - -@subsection -drive addr=... (since 2.10.0) - -The drive addr argument is replaced by the the addr argument -that can be specified with the ``-device'' parameter. - @subsection -usbdevice (since 2.10.0) The ``-usbdevice DEV'' argument is now a synonym for setting diff --git a/qemu-img.c b/qemu-img.c index 1acddf693c..b12f4cd19b 100644 --- a/qemu-img.c +++ b/qemu-img.c @@ -345,21 +345,6 @@ static int img_add_key_secrets(void *opaque, return 0; } -static BlockBackend *img_open_new_file(const char *filename, - QemuOpts *create_opts, - const char *fmt, int flags, - bool writethrough, bool quiet, - bool force_share) -{ - QDict *options = NULL; - - options = qdict_new(); - qemu_opt_foreach(create_opts, img_add_key_secrets, options, &error_abort); - - return img_open_file(filename, options, fmt, flags, writethrough, quiet, - force_share); -} - static BlockBackend *img_open(bool image_opts, const char *filename, @@ -2018,6 +2003,7 @@ static int img_convert(int argc, char **argv) BlockDriverState *out_bs; QemuOpts *opts = NULL, *sn_opts = NULL; QemuOptsList *create_opts = NULL; + QDict *open_opts = NULL; char *options = NULL; Error *local_err = NULL; bool writethrough, src_writethrough, quiet = false, image_opts = false, @@ -2362,6 +2348,16 @@ static int img_convert(int argc, char **argv) } } + /* + * The later open call will need any decryption secrets, and + * bdrv_create() will purge "opts", so extract them now before + * they are lost. + */ + if (!skip_create) { + open_opts = qdict_new(); + qemu_opt_foreach(opts, img_add_key_secrets, open_opts, &error_abort); + } + if (!skip_create) { /* Create the new image */ ret = bdrv_create(drv, out_filename, opts, &local_err); @@ -2388,8 +2384,9 @@ static int img_convert(int argc, char **argv) * That has to wait for bdrv_create to be improved * to allow filenames in option syntax */ - s.target = img_open_new_file(out_filename, opts, out_fmt, - flags, writethrough, quiet, false); + s.target = img_open_file(out_filename, open_opts, out_fmt, + flags, writethrough, quiet, false); + open_opts = NULL; /* blk_new_open will have freed it */ } if (!s.target) { ret = -1; @@ -2464,6 +2461,7 @@ out: qemu_opts_del(opts); qemu_opts_free(create_opts); qemu_opts_del(sn_opts); + qobject_unref(open_opts); blk_unref(s.target); if (s.src) { for (bs_i = 0; bs_i < s.src_num; bs_i++) { diff --git a/qemu-options.hx b/qemu-options.hx index b1bf0f485f..ea4edb4938 100644 --- a/qemu-options.hx +++ b/qemu-options.hx @@ -804,9 +804,8 @@ ETEXI DEF("drive", HAS_ARG, QEMU_OPTION_drive, "-drive [file=file][,if=type][,bus=n][,unit=m][,media=d][,index=i]\n" - " [,cyls=c,heads=h,secs=s[,trans=t]][,snapshot=on|off]\n" " [,cache=writethrough|writeback|none|directsync|unsafe][,format=f]\n" - " [,serial=s][,addr=A][,rerror=ignore|stop|report]\n" + " [,snapshot=on|off][,rerror=ignore|stop|report]\n" " [,werror=ignore|stop|report|enospc][,id=name][,aio=threads|native]\n" " [,readonly=on|off][,copy-on-read=on|off]\n" " [,discard=ignore|unmap][,detect-zeroes=on|off|unmap]\n" @@ -847,10 +846,6 @@ This option defines where is connected the drive by using an index in the list of available connectors of a given interface type. @item media=@var{media} This option defines the type of the media: disk or cdrom. -@item cyls=@var{c},heads=@var{h},secs=@var{s}[,trans=@var{t}] -Force disk physical geometry and the optional BIOS translation (trans=none or -lba). These parameters are deprecated, use the corresponding parameters -of @code{-device} instead. @item snapshot=@var{snapshot} @var{snapshot} is "on" or "off" and controls snapshot mode for the given drive (see @option{-snapshot}). @@ -884,13 +879,6 @@ The default mode is @option{cache=writeback}. Specify which disk @var{format} will be used rather than detecting the format. Can be used to specify format=raw to avoid interpreting an untrusted format header. -@item serial=@var{serial} -This option specifies the serial number to assign to the device. This -parameter is deprecated, use the corresponding parameter of @code{-device} -instead. -@item addr=@var{addr} -Specify the controller's PCI address (if=virtio only). This parameter is -deprecated, use the corresponding parameter of @code{-device} instead. @item werror=@var{action},rerror=@var{action} Specify which @var{action} to take on write and read errors. Valid actions are: "ignore" (ignore the error and try to continue), "stop" (pause QEMU), diff --git a/qobject/block-qdict.c b/qobject/block-qdict.c index 80c653013f..42054cc274 100644 --- a/qobject/block-qdict.c +++ b/qobject/block-qdict.c @@ -158,20 +158,25 @@ void qdict_flatten(QDict *qdict) qdict_flatten_qdict(qdict, qdict, NULL); } -/* extract all the src QDict entries starting by start into dst */ +/* extract all the src QDict entries starting by start into dst. + * If dst is NULL then the entries are simply removed from src. */ void qdict_extract_subqdict(QDict *src, QDict **dst, const char *start) { const QDictEntry *entry, *next; const char *p; - *dst = qdict_new(); + if (dst) { + *dst = qdict_new(); + } entry = qdict_first(src); while (entry != NULL) { next = qdict_next(src, entry); if (strstart(entry->key, start, &p)) { - qdict_put_obj(*dst, p, qobject_ref(entry->value)); + if (dst) { + qdict_put_obj(*dst, p, qobject_ref(entry->value)); + } qdict_del(src, entry->key); } entry = next; diff --git a/tests/ahci-test.c b/tests/ahci-test.c index 60ba32f4f2..5dd380e5ea 100644 --- a/tests/ahci-test.c +++ b/tests/ahci-test.c @@ -183,12 +183,12 @@ static AHCIQState *ahci_boot(const char *cli, ...) s = ahci_vboot(cli, ap); va_end(ap); } else { - cli = "-drive if=none,id=drive0,file=%s,cache=writeback,serial=%s" - ",format=%s" + cli = "-drive if=none,id=drive0,file=%s,cache=writeback,format=%s" " -M q35 " "-device ide-hd,drive=drive0 " + "-global ide-hd.serial=%s " "-global ide-hd.ver=%s"; - s = ahci_boot(cli, tmp_path, "testdisk", imgfmt, "version"); + s = ahci_boot(cli, tmp_path, imgfmt, "testdisk", "version"); } return s; diff --git a/tests/hd-geo-test.c b/tests/hd-geo-test.c index 24870b38f4..ce665f1f83 100644 --- a/tests/hd-geo-test.c +++ b/tests/hd-geo-test.c @@ -201,7 +201,7 @@ static void setup_mbr(int img_idx, MBRcontents mbr) static int setup_ide(int argc, char *argv[], int argv_sz, int ide_idx, const char *dev, int img_idx, - MBRcontents mbr, const char *opts) + MBRcontents mbr) { char *s1, *s2, *s3; @@ -216,7 +216,7 @@ static int setup_ide(int argc, char *argv[], int argv_sz, s3 = g_strdup(",media=cdrom"); } argc = append_arg(argc, argv, argv_sz, - g_strdup_printf("%s%s%s%s", s1, s2, s3, opts)); + g_strdup_printf("%s%s%s", s1, s2, s3)); g_free(s1); g_free(s2); g_free(s3); @@ -260,7 +260,7 @@ static void test_ide_mbr(bool use_device, MBRcontents mbr) for (i = 0; i < backend_last; i++) { cur_ide[i] = &hd_chst[i][mbr]; dev = use_device ? (is_hd(cur_ide[i]) ? "ide-hd" : "ide-cd") : NULL; - argc = setup_ide(argc, argv, ARGV_SIZE, i, dev, i, mbr, ""); + argc = setup_ide(argc, argv, ARGV_SIZE, i, dev, i, mbr); } args = g_strjoinv(" ", argv); qtest_start(args); @@ -327,16 +327,12 @@ static void test_ide_drive_user(const char *dev, bool trans) const CHST expected_chst = { secs / (4 * 32) , 4, 32, trans }; argc = setup_common(argv, ARGV_SIZE); - opts = g_strdup_printf("%s,%s%scyls=%d,heads=%d,secs=%d", - dev ?: "", - trans && dev ? "bios-chs-" : "", - trans ? "trans=lba," : "", + opts = g_strdup_printf("%s,%scyls=%d,heads=%d,secs=%d", + dev, trans ? "bios-chs-trans=lba," : "", expected_chst.cyls, expected_chst.heads, expected_chst.secs); cur_ide[0] = &expected_chst; - argc = setup_ide(argc, argv, ARGV_SIZE, - 0, dev ? opts : NULL, backend_small, mbr_chs, - dev ? "" : opts); + argc = setup_ide(argc, argv, ARGV_SIZE, 0, opts, backend_small, mbr_chs); g_free(opts); args = g_strjoinv(" ", argv); qtest_start(args); @@ -347,22 +343,6 @@ static void test_ide_drive_user(const char *dev, bool trans) } /* - * Test case: IDE device (if=ide) with explicit CHS - */ -static void test_ide_drive_user_chs(void) -{ - test_ide_drive_user(NULL, false); -} - -/* - * Test case: IDE device (if=ide) with explicit CHS and translation - */ -static void test_ide_drive_user_chst(void) -{ - test_ide_drive_user(NULL, true); -} - -/* * Test case: IDE device (if=none) with explicit CHS */ static void test_ide_device_user_chs(void) @@ -392,8 +372,7 @@ static void test_ide_drive_cd_0(void) for (i = 0; i <= backend_empty; i++) { ide_idx = backend_empty - i; cur_ide[ide_idx] = &hd_chst[i][mbr_blank]; - argc = setup_ide(argc, argv, ARGV_SIZE, - ide_idx, NULL, i, mbr_blank, ""); + argc = setup_ide(argc, argv, ARGV_SIZE, ide_idx, NULL, i, mbr_blank); } args = g_strjoinv(" ", argv); qtest_start(args); @@ -422,8 +401,6 @@ int main(int argc, char **argv) qtest_add_func("hd-geo/ide/drive/mbr/blank", test_ide_drive_mbr_blank); qtest_add_func("hd-geo/ide/drive/mbr/lba", test_ide_drive_mbr_lba); qtest_add_func("hd-geo/ide/drive/mbr/chs", test_ide_drive_mbr_chs); - qtest_add_func("hd-geo/ide/drive/user/chs", test_ide_drive_user_chs); - qtest_add_func("hd-geo/ide/drive/user/chst", test_ide_drive_user_chst); qtest_add_func("hd-geo/ide/drive/cd_0", test_ide_drive_cd_0); qtest_add_func("hd-geo/ide/device/mbr/blank", test_ide_device_mbr_blank); qtest_add_func("hd-geo/ide/device/mbr/lba", test_ide_device_mbr_lba); diff --git a/tests/ide-test.c b/tests/ide-test.c index 89c4468c83..33cef61e1f 100644 --- a/tests/ide-test.c +++ b/tests/ide-test.c @@ -532,8 +532,8 @@ static void test_bmdma_no_busmaster(void) static void test_bmdma_setup(void) { ide_test_start( - "-drive file=%s,if=ide,serial=%s,cache=writeback,format=raw " - "-global ide-hd.ver=%s", + "-drive file=%s,if=ide,cache=writeback,format=raw " + "-global ide-hd.serial=%s -global ide-hd.ver=%s", tmp_path, "testdisk", "version"); qtest_irq_intercept_in(global_qtest, "ioapic"); } @@ -564,8 +564,8 @@ static void test_identify(void) int ret; ide_test_start( - "-drive file=%s,if=ide,serial=%s,cache=writeback,format=raw " - "-global ide-hd.ver=%s", + "-drive file=%s,if=ide,cache=writeback,format=raw " + "-global ide-hd.serial=%s -global ide-hd.ver=%s", tmp_path, "testdisk", "version"); dev = get_pci_device(&bmdma_bar, &ide_bar); diff --git a/tests/qemu-iotests/041 b/tests/qemu-iotests/041 index c20ac7da87..9336ab6ff5 100755 --- a/tests/qemu-iotests/041 +++ b/tests/qemu-iotests/041 @@ -234,6 +234,12 @@ class TestSingleBlockdev(TestSingleDrive): result = self.vm.qmp("blockdev-add", **args) self.assert_qmp(result, 'return', {}) + def test_mirror_to_self(self): + result = self.vm.qmp(self.qmp_cmd, job_id='job0', + device=self.qmp_target, sync='full', + target=self.qmp_target) + self.assert_qmp(result, 'error/class', 'GenericError') + test_large_cluster = None test_image_not_found = None test_small_buffer2 = None diff --git a/tests/qemu-iotests/041.out b/tests/qemu-iotests/041.out index c28b392b87..e071d0b261 100644 --- a/tests/qemu-iotests/041.out +++ b/tests/qemu-iotests/041.out @@ -1,5 +1,5 @@ -..................................................................................... +........................................................................................ ---------------------------------------------------------------------- -Ran 85 tests +Ran 88 tests OK diff --git a/tests/qemu-iotests/093 b/tests/qemu-iotests/093 index 68e344f8c1..9d1971a56c 100755 --- a/tests/qemu-iotests/093 +++ b/tests/qemu-iotests/093 @@ -208,6 +208,61 @@ class ThrottleTestCase(iotests.QMPTestCase): limits[tk] = rate self.do_test_throttle(ndrives, 5, limits) + # Test that removing a drive from a throttle group should not + # affect the remaining members of the group. + # https://bugzilla.redhat.com/show_bug.cgi?id=1535914 + def test_remove_group_member(self): + # Create a throttle group with two drives + # and set a 4 KB/s read limit. + params = {"bps": 0, + "bps_rd": 4096, + "bps_wr": 0, + "iops": 0, + "iops_rd": 0, + "iops_wr": 0 } + self.configure_throttle(2, params) + + # Read 4KB from drive0. This is performed immediately. + self.vm.hmp_qemu_io("drive0", "aio_read 0 4096") + + # Read 2KB. The I/O limit has been exceeded so this + # request is throttled and a timer is set to wake it up. + self.vm.hmp_qemu_io("drive0", "aio_read 0 2048") + + # Read 2KB again. We're still over the I/O limit so this is + # request is also throttled, but no new timer is set since + # there's already one. + self.vm.hmp_qemu_io("drive0", "aio_read 0 2048") + + # Read from drive1. This request is also throttled, and no + # timer is set in drive1 because there's already one in + # drive0. + self.vm.hmp_qemu_io("drive1", "aio_read 0 4096") + + # At this point only the first 4KB have been read from drive0. + # The other requests are throttled. + self.assertEqual(self.blockstats('drive0')[0], 4096) + self.assertEqual(self.blockstats('drive1')[0], 0) + + # Remove drive0 from the throttle group and disable its I/O limits. + # drive1 remains in the group with a throttled request. + params['bps_rd'] = 0 + params['device'] = 'drive0' + result = self.vm.qmp("block_set_io_throttle", conv_keys=False, **params) + self.assert_qmp(result, 'return', {}) + + # Removing the I/O limits from drive0 drains its two pending requests. + # The read request in drive1 is still throttled. + self.assertEqual(self.blockstats('drive0')[0], 8192) + self.assertEqual(self.blockstats('drive1')[0], 0) + + # Advance the clock 5 seconds. This completes the request in drive1 + self.vm.qtest("clock_step %d" % (5 * nsec_per_sec)) + + # Now all requests have been processed. + self.assertEqual(self.blockstats('drive0')[0], 8192) + self.assertEqual(self.blockstats('drive1')[0], 4096) + class ThrottleTestCoroutine(ThrottleTestCase): test_img = "null-co://" diff --git a/tests/qemu-iotests/093.out b/tests/qemu-iotests/093.out index 594c16f49f..36376bed87 100644 --- a/tests/qemu-iotests/093.out +++ b/tests/qemu-iotests/093.out @@ -1,5 +1,5 @@ -........ +.......... ---------------------------------------------------------------------- -Ran 8 tests +Ran 10 tests OK |