OpenE2K/qemu-e2k
13
4
Fork 0
Code Issues 4 Pull Requests Projects 2 Releases Activity

blockdev: Split monitor reference from BB creation

Browse Source 
Before this patch, blk_new() automatically assigned a name to the new
BlockBackend and considered it referenced by the monitor. This patch
removes the implicit monitor_add_blk() call from blk_new() (and
consequently the monitor_remove_blk() call from blk_delete(), too) and
thus blk_new() (and related functions) no longer take a BB name
argument.

In fact, there is only a single point where blk_new()/blk_new_open() is
called and the new BB is monitor-owned, and that is in blockdev_init().
Besides thus relieving us from having to invent names for all of the BBs
we use in qemu-img, this fixes a bug where qemu cannot create a new
image if there already is a monitor-owned BB named "image".

If a BB and its BDS tree are created in a single operation, as of this
patch the BDS tree will be created before the BB is given a name
(whereas it was the other way around before). This results in minor
change to the output of iotest 087, whose reference output is amended
accordingly.

Signed-off-by: Max Reitz <mreitz@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
This commit is contained in:
Max Reitz 2016-03-16 19:54:38 +01:00 committed by Kevin Wolf
parent e5e785500b
commit efaa7c4eeb
18 changed files with 68 additions and 76 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

26
block/block-backend.c
View File

@ -80,13 +80,11 @@ static QTAILQ_HEAD(, BlockBackend) monitor_block_backends =
QTAILQ_HEAD_INITIALIZER(monitor_block_backends);
/*
* Create a new BlockBackend with @name, with a reference count of one.
* @name must not be null or empty.
* Fail if a BlockBackend with this name already exists.
* Create a new BlockBackend with a reference count of one.
* Store an error through @errp on failure, unless it's null.
* Return the new BlockBackend on success, null on failure.
*/
BlockBackend *blk_new(const char *name, Error **errp)
BlockBackend *blk_new(Error **errp)
{
BlockBackend *blk;
@ -94,14 +92,7 @@ BlockBackend *blk_new(const char *name, Error **errp)
blk->refcnt = 1;
notifier_list_init(&blk->remove_bs_notifiers);
notifier_list_init(&blk->insert_bs_notifiers);
QTAILQ_INSERT_TAIL(&block_backends, blk, link);
if (!monitor_add_blk(blk, name, errp)) {
blk_unref(blk);
return NULL;
}
return blk;
}
@ -109,12 +100,12 @@ BlockBackend *blk_new(const char *name, Error **errp)
* Create a new BlockBackend with a new BlockDriverState attached.
* Otherwise just like blk_new(), which see.
*/
BlockBackend *blk_new_with_bs(const char *name, Error **errp)
BlockBackend *blk_new_with_bs(Error **errp)
{
BlockBackend *blk;
BlockDriverState *bs;
blk = blk_new(name, errp);
blk = blk_new(errp);
if (!blk) {
return NULL;
}
@ -137,14 +128,13 @@ BlockBackend *blk_new_with_bs(const char *name, Error **errp)
* though, so callers of this function have to be able to specify @filename and
* @flags.
*/
BlockBackend *blk_new_open(const char *name, const char *filename,
const char *reference, QDict *options, int flags,
Error **errp)
BlockBackend *blk_new_open(const char *filename, const char *reference,
QDict *options, int flags, Error **errp)
{
BlockBackend *blk;
int ret;
blk = blk_new_with_bs(name, errp);
blk = blk_new_with_bs(errp);
if (!blk) {
QDECREF(options);
return NULL;
@ -161,8 +151,6 @@ BlockBackend *blk_new_open(const char *name, const char *filename,
static void blk_delete(BlockBackend *blk)
{
monitor_remove_blk(blk);
assert(!blk->refcnt);
assert(!blk->name);
assert(!blk->dev);

2
block/parallels.c
View File

@ -478,7 +478,7 @@ static int parallels_create(const char *filename, QemuOpts *opts, Error **errp)
return ret;
}
file = blk_new_open("image", filename, NULL, NULL,
file = blk_new_open(filename, NULL, NULL,
BDRV_O_RDWR | BDRV_O_CACHE_WB | BDRV_O_PROTOCOL,
&local_err);
if (file == NULL) {

2
block/qcow.c
View File

@ -793,7 +793,7 @@ static int qcow_create(const char *filename, QemuOpts *opts, Error **errp)
goto cleanup;
}
qcow_blk = blk_new_open("image", filename, NULL, NULL,
qcow_blk = blk_new_open(filename, NULL, NULL,
BDRV_O_RDWR | BDRV_O_CACHE_WB | BDRV_O_PROTOCOL,
&local_err);
if (qcow_blk == NULL) {

6
block/qcow2.c
View File

@ -2159,7 +2159,7 @@ static int qcow2_create2(const char *filename, int64_t total_size,
return ret;
}
blk = blk_new_open("image", filename, NULL, NULL,
blk = blk_new_open(filename, NULL, NULL,
BDRV_O_RDWR | BDRV_O_CACHE_WB | BDRV_O_PROTOCOL,
&local_err);
if (blk == NULL) {
@ -2224,7 +2224,7 @@ static int qcow2_create2(const char *filename, int64_t total_size,
*/
options = qdict_new();
qdict_put(options, "driver", qstring_from_str("qcow2"));
blk = blk_new_open("image-qcow2", filename, NULL, options,
blk = blk_new_open(filename, NULL, options,
BDRV_O_RDWR | BDRV_O_CACHE_WB | BDRV_O_NO_FLUSH,
&local_err);
if (blk == NULL) {
@ -2286,7 +2286,7 @@ static int qcow2_create2(const char *filename, int64_t total_size,
/* Reopen the image without BDRV_O_NO_FLUSH to flush it before returning */
options = qdict_new();
qdict_put(options, "driver", qstring_from_str("qcow2"));
blk = blk_new_open("image-flush", filename, NULL, options,
blk = blk_new_open(filename, NULL, options,
BDRV_O_RDWR | BDRV_O_CACHE_WB | BDRV_O_NO_BACKING,
&local_err);
if (blk == NULL) {

2
block/qed.c
View File

@ -574,7 +574,7 @@ static int qed_create(const char *filename, uint32_t cluster_size,
return ret;
}
blk = blk_new_open("image", filename, NULL, NULL,
blk = blk_new_open(filename, NULL, NULL,
BDRV_O_RDWR | BDRV_O_CACHE_WB | BDRV_O_PROTOCOL,
&local_err);
if (blk == NULL) {

4
block/sheepdog.c
View File

@ -1646,7 +1646,7 @@ static int sd_prealloc(const char *filename, Error **errp)
void *buf = NULL;
int ret;
blk = blk_new_open("image-prealloc", filename, NULL, NULL,
blk = blk_new_open(filename, NULL, NULL,
BDRV_O_RDWR | BDRV_O_CACHE_WB | BDRV_O_PROTOCOL,
errp);
if (blk == NULL) {
@ -1843,7 +1843,7 @@ static int sd_create(const char *filename, QemuOpts *opts,
goto out;
}
blk = blk_new_open("backing", backing_file, NULL, NULL,
blk = blk_new_open(backing_file, NULL, NULL,
BDRV_O_PROTOCOL | BDRV_O_CACHE_WB, errp);
if (blk == NULL) {
ret = -EIO;

2
block/vdi.c
View File

@ -768,7 +768,7 @@ static int vdi_create(const char *filename, QemuOpts *opts, Error **errp)
goto exit;
}
blk = blk_new_open("image", filename, NULL, NULL,
blk = blk_new_open(filename, NULL, NULL,
BDRV_O_RDWR | BDRV_O_CACHE_WB | BDRV_O_PROTOCOL,
&local_err);
if (blk == NULL) {

2
block/vhdx.c
View File

@ -1838,7 +1838,7 @@ static int vhdx_create(const char *filename, QemuOpts *opts, Error **errp)
goto exit;
}
blk = blk_new_open("image", filename, NULL, NULL,
blk = blk_new_open(filename, NULL, NULL,
BDRV_O_RDWR | BDRV_O_CACHE_WB | BDRV_O_PROTOCOL,
&local_err);
if (blk == NULL) {

6
block/vmdk.c
View File

@ -1661,7 +1661,7 @@ static int vmdk_create_extent(const char *filename, int64_t filesize,
goto exit;
}
blk = blk_new_open("extent", filename, NULL, NULL,
blk = blk_new_open(filename, NULL, NULL,
BDRV_O_RDWR | BDRV_O_CACHE_WB | BDRV_O_PROTOCOL,
&local_err);
if (blk == NULL) {
@ -1946,7 +1946,7 @@ static int vmdk_create(const char *filename, QemuOpts *opts, Error **errp)
goto exit;
}
blk = blk_new_open("backing", full_backing, NULL, NULL,
blk = blk_new_open(full_backing, NULL, NULL,
BDRV_O_NO_BACKING | BDRV_O_CACHE_WB, errp);
g_free(full_backing);
if (blk == NULL) {
@ -2018,7 +2018,7 @@ static int vmdk_create(const char *filename, QemuOpts *opts, Error **errp)
}
}
new_blk = blk_new_open("descriptor", filename, NULL, NULL,
new_blk = blk_new_open(filename, NULL, NULL,
BDRV_O_RDWR | BDRV_O_CACHE_WB | BDRV_O_PROTOCOL,
&local_err);
if (new_blk == NULL) {

2
block/vpc.c
View File

@ -888,7 +888,7 @@ static int vpc_create(const char *filename, QemuOpts *opts, Error **errp)
goto out;
}
blk = blk_new_open("image", filename, NULL, NULL,
blk = blk_new_open(filename, NULL, NULL,
BDRV_O_RDWR | BDRV_O_CACHE_WB | BDRV_O_PROTOCOL,
&local_err);
if (blk == NULL) {

17
blockdev.c
View File

@ -147,6 +147,7 @@ void blockdev_auto_del(BlockBackend *blk)
DriveInfo *dinfo = blk_legacy_dinfo(blk);
if (dinfo && dinfo->auto_del) {
monitor_remove_blk(blk);
blk_unref(blk);
}
}
@ -561,7 +562,7 @@ static BlockBackend *blockdev_init(const char *file, QDict *bs_opts,
if ((!file || !*file) && !qdict_size(bs_opts)) {
BlockBackendRootState *blk_rs;
blk = blk_new(qemu_opts_id(opts), errp);
blk = blk_new(errp);
if (!blk) {
goto early_err;
}
@ -597,8 +598,7 @@ static BlockBackend *blockdev_init(const char *file, QDict *bs_opts,
bdrv_flags |= BDRV_O_INACTIVE;
}
blk = blk_new_open(qemu_opts_id(opts), file, NULL, bs_opts, bdrv_flags,
errp);
blk = blk_new_open(file, NULL, bs_opts, bdrv_flags, errp);
if (!blk) {
goto err_no_bs_opts;
}
@ -630,6 +630,12 @@ static BlockBackend *blockdev_init(const char *file, QDict *bs_opts,
blk_set_on_error(blk, on_read_error, on_write_error);
if (!monitor_add_blk(blk, qemu_opts_id(opts), errp)) {
blk_unref(blk);
blk = NULL;
goto err_no_bs_opts;
}
err_no_bs_opts:
qemu_opts_del(opts);
QDECREF(interval_dict);
@ -2859,6 +2865,8 @@ void hmp_drive_del(Monitor *mon, const QDict *qdict)
blk_remove_bs(blk);
}
monitor_remove_blk(blk);
/* if we have a device attached to this BlockDriverState
* then we need to make the drive anonymous until the device
* can be removed. If this is a drive with no device backing
@ -3976,6 +3984,7 @@ void qmp_blockdev_add(BlockdevOptions *options, Error **errp)
if (bs && bdrv_key_required(bs)) {
if (blk) {
monitor_remove_blk(blk);
blk_unref(blk);
} else {
QTAILQ_REMOVE(&monitor_bdrv_states, bs, monitor_list);
@ -4005,6 +4014,7 @@ void qmp_x_blockdev_del(bool has_id, const char *id,
}
if (has_id) {
/* blk_by_name() never returns a BB that is not owned by the monitor */
blk = blk_by_name(id);
if (!blk) {
error_setg(errp, "Cannot find block backend %s", id);
@ -4052,6 +4062,7 @@ void qmp_x_blockdev_del(bool has_id, const char *id,
}
if (blk) {
monitor_remove_blk(blk);
blk_unref(blk);
} else {
QTAILQ_REMOVE(&monitor_bdrv_states, bs, monitor_list);

4
device-hotplug.c
View File

@ -84,6 +84,8 @@ void hmp_drive_add(Monitor *mon, const QDict *qdict)
err:
if (dinfo) {
blk_unref(blk_by_legacy_dinfo(dinfo));
BlockBackend *blk = blk_by_legacy_dinfo(dinfo);
monitor_remove_blk(blk);
blk_unref(blk);
}
}

2
hw/block/xen_disk.c
View File

@ -917,7 +917,7 @@ static int blk_connect(struct XenDevice *xendev)
/* setup via xenbus -> create new block driver instance */
xen_be_printf(&blkdev->xendev, 2, "create new bdrv (xenbus setup)\n");
blkdev->blk = blk_new_open(blkdev->dev, blkdev->filename, NULL, options,
blkdev->blk = blk_new_open(blkdev->filename, NULL, options,
qflags, &local_err);
if (!blkdev->blk) {
xen_be_printf(&blkdev->xendev, 0, "error: %s\n",

9
include/sysemu/block-backend.h
View File

@ -59,11 +59,10 @@ typedef struct BlockDevOps {
void (*resize_cb)(void *opaque);
} BlockDevOps;
BlockBackend *blk_new(const char *name, Error **errp);
BlockBackend *blk_new_with_bs(const char *name, Error **errp);
BlockBackend *blk_new_open(const char *name, const char *filename,
const char *reference, QDict *options, int flags,
Error **errp);
BlockBackend *blk_new(Error **errp);
BlockBackend *blk_new_with_bs(Error **errp);
BlockBackend *blk_new_open(const char *filename, const char *reference,
QDict *options, int flags, Error **errp);
int blk_get_refcnt(BlockBackend *blk);
void blk_ref(BlockBackend *blk);
void blk_unref(BlockBackend *blk);

50
qemu-img.c
View File

@ -245,8 +245,7 @@ static int img_open_password(BlockBackend *blk, const char *filename,
}
static BlockBackend *img_open_opts(const char *id,
const char *optstr,
static BlockBackend *img_open_opts(const char *optstr,
QemuOpts *opts, int flags,
bool require_io, bool quiet)
{
@ -254,7 +253,7 @@ static BlockBackend *img_open_opts(const char *id,
Error *local_err = NULL;
BlockBackend *blk;
options = qemu_opts_to_qdict(opts, NULL);
blk = blk_new_open(id, NULL, NULL, options, flags, &local_err);
blk = blk_new_open(NULL, NULL, options, flags, &local_err);
if (!blk) {
error_reportf_err(local_err, "Could not open '%s'", optstr);
return NULL;
@ -267,7 +266,7 @@ static BlockBackend *img_open_opts(const char *id,
return blk;
}
static BlockBackend *img_open_file(const char *id, const char *filename,
static BlockBackend *img_open_file(const char *filename,
const char *fmt, int flags,
bool require_io, bool quiet)
{
@ -280,7 +279,7 @@ static BlockBackend *img_open_file(const char *id, const char *filename,
qdict_put(options, "driver", qstring_from_str(fmt));
}
blk = blk_new_open(id, filename, NULL, options, flags, &local_err);
blk = blk_new_open(filename, NULL, options, flags, &local_err);
if (!blk) {
error_reportf_err(local_err, "Could not open '%s': ", filename);
return NULL;
@ -294,8 +293,7 @@ static BlockBackend *img_open_file(const char *id, const char *filename,
}
static BlockBackend *img_open(const char *id,
bool image_opts,
static BlockBackend *img_open(bool image_opts,
const char *filename,
const char *fmt, int flags,
bool require_io, bool quiet)
@ -312,9 +310,9 @@ static BlockBackend *img_open(const char *id,
if (!opts) {
return NULL;
}
blk = img_open_opts(id, filename, opts, flags, true, quiet);
blk = img_open_opts(filename, opts, flags, true, quiet);
} else {
blk = img_open_file(id, filename, fmt, flags, true, quiet);
blk = img_open_file(filename, fmt, flags, true, quiet);
}
return blk;
}
@ -686,7 +684,7 @@ static int img_check(int argc, char **argv)
return 1;
}
blk = img_open("image", image_opts, filename, fmt, flags, true, quiet);
blk = img_open(image_opts, filename, fmt, flags, true, quiet);
if (!blk) {
return 1;
}
@ -878,7 +876,7 @@ static int img_commit(int argc, char **argv)
return 1;
}
blk = img_open("image", image_opts, filename, fmt, flags, true, quiet);
blk = img_open(image_opts, filename, fmt, flags, true, quiet);
if (!blk) {
return 1;
}
@ -1212,13 +1210,13 @@ static int img_compare(int argc, char **argv)
goto out3;
}
blk1 = img_open("image_1", image_opts, filename1, fmt1, flags, true, quiet);
blk1 = img_open(image_opts, filename1, fmt1, flags, true, quiet);
if (!blk1) {
ret = 2;
goto out3;
}
blk2 = img_open("image_2", image_opts, filename2, fmt2, flags, true, quiet);
blk2 = img_open(image_opts, filename2, fmt2, flags, true, quiet);
if (!blk2) {
ret = 2;
goto out2;
@ -1899,11 +1897,8 @@ static int img_convert(int argc, char **argv)
total_sectors = 0;
for (bs_i = 0; bs_i < bs_n; bs_i++) {
char *id = bs_n > 1 ? g_strdup_printf("source_%d", bs_i)
: g_strdup("source");
blk[bs_i] = img_open(id, image_opts, argv[optind + bs_i],
blk[bs_i] = img_open(image_opts, argv[optind + bs_i],
fmt, src_flags, true, quiet);
g_free(id);
if (!blk[bs_i]) {
ret = -1;
goto out;
@ -2048,8 +2043,7 @@ static int img_convert(int argc, char **argv)
* the bdrv_create() call which takes different params.
* Not critical right now, so fix can wait...
*/
out_blk = img_open_file("target", out_filename,
out_fmt, flags, true, quiet);
out_blk = img_open_file(out_filename, out_fmt, flags, true, quiet);
if (!out_blk) {
ret = -1;
goto out;
@ -2240,7 +2234,7 @@ static ImageInfoList *collect_image_info_list(bool image_opts,
}
g_hash_table_insert(filenames, (gpointer)filename, NULL);
blk = img_open("image", image_opts, filename, fmt,
blk = img_open(image_opts, filename, fmt,
BDRV_O_FLAGS | BDRV_O_NO_BACKING,
false, false);
if (!blk) {
@ -2572,8 +2566,7 @@ static int img_map(int argc, char **argv)
return 1;
}
blk = img_open("image", image_opts, filename, fmt,
BDRV_O_FLAGS, true, false);
blk = img_open(image_opts, filename, fmt, BDRV_O_FLAGS, true, false);
if (!blk) {
return 1;
}
@ -2718,8 +2711,7 @@ static int img_snapshot(int argc, char **argv)
}
/* Open the image */
blk = img_open("image", image_opts, filename, NULL,
bdrv_oflags, true, quiet);
blk = img_open(image_opts, filename, NULL, bdrv_oflags, true, quiet);
if (!blk) {
return 1;
}
@ -2890,7 +2882,7 @@ static int img_rebase(int argc, char **argv)
* Ignore the old backing file for unsafe rebase in case we want to correct
* the reference to a renamed or moved backing file.
*/
blk = img_open("image", image_opts, filename, fmt, flags, true, quiet);
blk = img_open(image_opts, filename, fmt, flags, true, quiet);
if (!blk) {
ret = -1;
goto out;
@ -2916,7 +2908,7 @@ static int img_rebase(int argc, char **argv)
}
bdrv_get_backing_filename(bs, backing_name, sizeof(backing_name));
blk_old_backing = blk_new_open("old_backing", backing_name, NULL,
blk_old_backing = blk_new_open(backing_name, NULL,
options, src_flags, &local_err);
if (!blk_old_backing) {
error_reportf_err(local_err,
@ -2933,7 +2925,7 @@ static int img_rebase(int argc, char **argv)
options = NULL;
}
blk_new_backing = blk_new_open("new_backing", out_baseimg, NULL,
blk_new_backing = blk_new_open(out_baseimg, NULL,
options, src_flags, &local_err);
if (!blk_new_backing) {
error_reportf_err(local_err,
@ -3227,7 +3219,7 @@ static int img_resize(int argc, char **argv)
n = qemu_opt_get_size(param, BLOCK_OPT_SIZE, 0);
qemu_opts_del(param);
blk = img_open("image", image_opts, filename, fmt,
blk = img_open(image_opts, filename, fmt,
BDRV_O_FLAGS | BDRV_O_RDWR, true, quiet);
if (!blk) {
ret = -1;
@ -3387,7 +3379,7 @@ static int img_amend(int argc, char **argv)
goto out;
}
blk = img_open("image", image_opts, filename, fmt, flags, true, quiet);
blk = img_open(image_opts, filename, fmt, flags, true, quiet);
if (!blk) {
ret = -1;
goto out;

2
qemu-io.c
View File

@ -61,7 +61,7 @@ static int openfile(char *name, int flags, QDict *opts)
return 1;
}
qemuio_blk = blk_new_open("hda", name, NULL, opts, flags, &local_err);
qemuio_blk = blk_new_open(name, NULL, opts, flags, &local_err);
if (!qemuio_blk) {
error_reportf_err(local_err, "can't open%s%s: ",
name ? " device " : "", name ?: "");

4
qemu-nbd.c
View File

@ -831,13 +831,13 @@ int main(int argc, char **argv)
}
options = qemu_opts_to_qdict(opts, NULL);
qemu_opts_reset(&file_opts);
blk = blk_new_open("hda", NULL, NULL, options, flags, &local_err);
blk = blk_new_open(NULL, NULL, options, flags, &local_err);
} else {
if (fmt) {
options = qdict_new();
qdict_put(options, "driver", qstring_from_str(fmt));
}
blk = blk_new_open("hda", srcpath, NULL, options, flags, &local_err);
blk = blk_new_open(srcpath, NULL, options, flags, &local_err);
}
if (!blk) {

2
tests/qemu-iotests/087.out
View File

@ -21,7 +21,7 @@ QMP_VERSION
{"error": {"class": "GenericError", "desc": "Device name 'test-node' conflicts with an existing node name"}}
{"error": {"class": "GenericError", "desc": "node-name=disk is conflicting with a device id"}}
{"error": {"class": "GenericError", "desc": "Duplicate node name"}}
{"error": {"class": "GenericError", "desc": "node-name=disk3 is conflicting with a device id"}}
{"error": {"class": "GenericError", "desc": "Device name 'disk3' conflicts with an existing node name"}}
{"return": {}}
{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "SHUTDOWN"}