Skip to content

Commit efaa7c4

Browse files
XanClickevmw
XanClic
authored and
kevmw
committed
blockdev: Split monitor reference from BB creation
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 <[email protected]> Signed-off-by: Kevin Wolf <[email protected]>
1 parent e5e7855 commit efaa7c4

18 files changed

+68
-76
lines changed

‎block/block-backend.c

Lines changed: 7 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -80,41 +80,32 @@ static QTAILQ_HEAD(, BlockBackend) monitor_block_backends =
8080
QTAILQ_HEAD_INITIALIZER(monitor_block_backends);
8181

8282
/*
83-
* Create a new BlockBackend with @name, with a reference count of one.
84-
* @name must not be null or empty.
85-
* Fail if a BlockBackend with this name already exists.
83+
* Create a new BlockBackend with a reference count of one.
8684
* Store an error through @errp on failure, unless it's null.
8785
* Return the new BlockBackend on success, null on failure.
8886
*/
89-
BlockBackend *blk_new(const char *name, Error **errp)
87+
BlockBackend *blk_new(Error **errp)
9088
{
9189
BlockBackend *blk;
9290

9391
blk = g_new0(BlockBackend, 1);
9492
blk->refcnt = 1;
9593
notifier_list_init(&blk->remove_bs_notifiers);
9694
notifier_list_init(&blk->insert_bs_notifiers);
97-
9895
QTAILQ_INSERT_TAIL(&block_backends, blk, link);
99-
100-
if (!monitor_add_blk(blk, name, errp)) {
101-
blk_unref(blk);
102-
return NULL;
103-
}
104-
10596
return blk;
10697
}
10798

10899
/*
109100
* Create a new BlockBackend with a new BlockDriverState attached.
110101
* Otherwise just like blk_new(), which see.
111102
*/
112-
BlockBackend *blk_new_with_bs(const char *name, Error **errp)
103+
BlockBackend *blk_new_with_bs(Error **errp)
113104
{
114105
BlockBackend *blk;
115106
BlockDriverState *bs;
116107

117-
blk = blk_new(name, errp);
108+
blk = blk_new(errp);
118109
if (!blk) {
119110
return NULL;
120111
}
@@ -137,14 +128,13 @@ BlockBackend *blk_new_with_bs(const char *name, Error **errp)
137128
* though, so callers of this function have to be able to specify @filename and
138129
* @flags.
139130
*/
140-
BlockBackend *blk_new_open(const char *name, const char *filename,
141-
const char *reference, QDict *options, int flags,
142-
Error **errp)
131+
BlockBackend *blk_new_open(const char *filename, const char *reference,
132+
QDict *options, int flags, Error **errp)
143133
{
144134
BlockBackend *blk;
145135
int ret;
146136

147-
blk = blk_new_with_bs(name, errp);
137+
blk = blk_new_with_bs(errp);
148138
if (!blk) {
149139
QDECREF(options);
150140
return NULL;
@@ -161,8 +151,6 @@ BlockBackend *blk_new_open(const char *name, const char *filename,
161151

162152
static void blk_delete(BlockBackend *blk)
163153
{
164-
monitor_remove_blk(blk);
165-
166154
assert(!blk->refcnt);
167155
assert(!blk->name);
168156
assert(!blk->dev);

‎block/parallels.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -478,7 +478,7 @@ static int parallels_create(const char *filename, QemuOpts *opts, Error **errp)
478478
return ret;
479479
}
480480

481-
file = blk_new_open("image", filename, NULL, NULL,
481+
file = blk_new_open(filename, NULL, NULL,
482482
BDRV_O_RDWR | BDRV_O_CACHE_WB | BDRV_O_PROTOCOL,
483483
&local_err);
484484
if (file == NULL) {

‎block/qcow.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -793,7 +793,7 @@ static int qcow_create(const char *filename, QemuOpts *opts, Error **errp)
793793
goto cleanup;
794794
}
795795

796-
qcow_blk = blk_new_open("image", filename, NULL, NULL,
796+
qcow_blk = blk_new_open(filename, NULL, NULL,
797797
BDRV_O_RDWR | BDRV_O_CACHE_WB | BDRV_O_PROTOCOL,
798798
&local_err);
799799
if (qcow_blk == NULL) {

‎block/qcow2.c

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2159,7 +2159,7 @@ static int qcow2_create2(const char *filename, int64_t total_size,
21592159
return ret;
21602160
}
21612161

2162-
blk = blk_new_open("image", filename, NULL, NULL,
2162+
blk = blk_new_open(filename, NULL, NULL,
21632163
BDRV_O_RDWR | BDRV_O_CACHE_WB | BDRV_O_PROTOCOL,
21642164
&local_err);
21652165
if (blk == NULL) {
@@ -2224,7 +2224,7 @@ static int qcow2_create2(const char *filename, int64_t total_size,
22242224
*/
22252225
options = qdict_new();
22262226
qdict_put(options, "driver", qstring_from_str("qcow2"));
2227-
blk = blk_new_open("image-qcow2", filename, NULL, options,
2227+
blk = blk_new_open(filename, NULL, options,
22282228
BDRV_O_RDWR | BDRV_O_CACHE_WB | BDRV_O_NO_FLUSH,
22292229
&local_err);
22302230
if (blk == NULL) {
@@ -2286,7 +2286,7 @@ static int qcow2_create2(const char *filename, int64_t total_size,
22862286
/* Reopen the image without BDRV_O_NO_FLUSH to flush it before returning */
22872287
options = qdict_new();
22882288
qdict_put(options, "driver", qstring_from_str("qcow2"));
2289-
blk = blk_new_open("image-flush", filename, NULL, options,
2289+
blk = blk_new_open(filename, NULL, options,
22902290
BDRV_O_RDWR | BDRV_O_CACHE_WB | BDRV_O_NO_BACKING,
22912291
&local_err);
22922292
if (blk == NULL) {

‎block/qed.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -574,7 +574,7 @@ static int qed_create(const char *filename, uint32_t cluster_size,
574574
return ret;
575575
}
576576

577-
blk = blk_new_open("image", filename, NULL, NULL,
577+
blk = blk_new_open(filename, NULL, NULL,
578578
BDRV_O_RDWR | BDRV_O_CACHE_WB | BDRV_O_PROTOCOL,
579579
&local_err);
580580
if (blk == NULL) {

‎block/sheepdog.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1646,7 +1646,7 @@ static int sd_prealloc(const char *filename, Error **errp)
16461646
void *buf = NULL;
16471647
int ret;
16481648

1649-
blk = blk_new_open("image-prealloc", filename, NULL, NULL,
1649+
blk = blk_new_open(filename, NULL, NULL,
16501650
BDRV_O_RDWR | BDRV_O_CACHE_WB | BDRV_O_PROTOCOL,
16511651
errp);
16521652
if (blk == NULL) {
@@ -1843,7 +1843,7 @@ static int sd_create(const char *filename, QemuOpts *opts,
18431843
goto out;
18441844
}
18451845

1846-
blk = blk_new_open("backing", backing_file, NULL, NULL,
1846+
blk = blk_new_open(backing_file, NULL, NULL,
18471847
BDRV_O_PROTOCOL | BDRV_O_CACHE_WB, errp);
18481848
if (blk == NULL) {
18491849
ret = -EIO;

‎block/vdi.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -768,7 +768,7 @@ static int vdi_create(const char *filename, QemuOpts *opts, Error **errp)
768768
goto exit;
769769
}
770770

771-
blk = blk_new_open("image", filename, NULL, NULL,
771+
blk = blk_new_open(filename, NULL, NULL,
772772
BDRV_O_RDWR | BDRV_O_CACHE_WB | BDRV_O_PROTOCOL,
773773
&local_err);
774774
if (blk == NULL) {

‎block/vhdx.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1838,7 +1838,7 @@ static int vhdx_create(const char *filename, QemuOpts *opts, Error **errp)
18381838
goto exit;
18391839
}
18401840

1841-
blk = blk_new_open("image", filename, NULL, NULL,
1841+
blk = blk_new_open(filename, NULL, NULL,
18421842
BDRV_O_RDWR | BDRV_O_CACHE_WB | BDRV_O_PROTOCOL,
18431843
&local_err);
18441844
if (blk == NULL) {

‎block/vmdk.c

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1661,7 +1661,7 @@ static int vmdk_create_extent(const char *filename, int64_t filesize,
16611661
goto exit;
16621662
}
16631663

1664-
blk = blk_new_open("extent", filename, NULL, NULL,
1664+
blk = blk_new_open(filename, NULL, NULL,
16651665
BDRV_O_RDWR | BDRV_O_CACHE_WB | BDRV_O_PROTOCOL,
16661666
&local_err);
16671667
if (blk == NULL) {
@@ -1946,7 +1946,7 @@ static int vmdk_create(const char *filename, QemuOpts *opts, Error **errp)
19461946
goto exit;
19471947
}
19481948

1949-
blk = blk_new_open("backing", full_backing, NULL, NULL,
1949+
blk = blk_new_open(full_backing, NULL, NULL,
19501950
BDRV_O_NO_BACKING | BDRV_O_CACHE_WB, errp);
19511951
g_free(full_backing);
19521952
if (blk == NULL) {
@@ -2018,7 +2018,7 @@ static int vmdk_create(const char *filename, QemuOpts *opts, Error **errp)
20182018
}
20192019
}
20202020

2021-
new_blk = blk_new_open("descriptor", filename, NULL, NULL,
2021+
new_blk = blk_new_open(filename, NULL, NULL,
20222022
BDRV_O_RDWR | BDRV_O_CACHE_WB | BDRV_O_PROTOCOL,
20232023
&local_err);
20242024
if (new_blk == NULL) {

‎block/vpc.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -888,7 +888,7 @@ static int vpc_create(const char *filename, QemuOpts *opts, Error **errp)
888888
goto out;
889889
}
890890

891-
blk = blk_new_open("image", filename, NULL, NULL,
891+
blk = blk_new_open(filename, NULL, NULL,
892892
BDRV_O_RDWR | BDRV_O_CACHE_WB | BDRV_O_PROTOCOL,
893893
&local_err);
894894
if (blk == NULL) {

‎blockdev.c

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -147,6 +147,7 @@ void blockdev_auto_del(BlockBackend *blk)
147147
DriveInfo *dinfo = blk_legacy_dinfo(blk);
148148

149149
if (dinfo && dinfo->auto_del) {
150+
monitor_remove_blk(blk);
150151
blk_unref(blk);
151152
}
152153
}
@@ -561,7 +562,7 @@ static BlockBackend *blockdev_init(const char *file, QDict *bs_opts,
561562
if ((!file || !*file) && !qdict_size(bs_opts)) {
562563
BlockBackendRootState *blk_rs;
563564

564-
blk = blk_new(qemu_opts_id(opts), errp);
565+
blk = blk_new(errp);
565566
if (!blk) {
566567
goto early_err;
567568
}
@@ -597,8 +598,7 @@ static BlockBackend *blockdev_init(const char *file, QDict *bs_opts,
597598
bdrv_flags |= BDRV_O_INACTIVE;
598599
}
599600

600-
blk = blk_new_open(qemu_opts_id(opts), file, NULL, bs_opts, bdrv_flags,
601-
errp);
601+
blk = blk_new_open(file, NULL, bs_opts, bdrv_flags, errp);
602602
if (!blk) {
603603
goto err_no_bs_opts;
604604
}
@@ -630,6 +630,12 @@ static BlockBackend *blockdev_init(const char *file, QDict *bs_opts,
630630

631631
blk_set_on_error(blk, on_read_error, on_write_error);
632632

633+
if (!monitor_add_blk(blk, qemu_opts_id(opts), errp)) {
634+
blk_unref(blk);
635+
blk = NULL;
636+
goto err_no_bs_opts;
637+
}
638+
633639
err_no_bs_opts:
634640
qemu_opts_del(opts);
635641
QDECREF(interval_dict);
@@ -2859,6 +2865,8 @@ void hmp_drive_del(Monitor *mon, const QDict *qdict)
28592865
blk_remove_bs(blk);
28602866
}
28612867

2868+
monitor_remove_blk(blk);
2869+
28622870
/* if we have a device attached to this BlockDriverState
28632871
* then we need to make the drive anonymous until the device
28642872
* can be removed. If this is a drive with no device backing
@@ -3976,6 +3984,7 @@ void qmp_blockdev_add(BlockdevOptions *options, Error **errp)
39763984

39773985
if (bs && bdrv_key_required(bs)) {
39783986
if (blk) {
3987+
monitor_remove_blk(blk);
39793988
blk_unref(blk);
39803989
} else {
39813990
QTAILQ_REMOVE(&monitor_bdrv_states, bs, monitor_list);
@@ -4005,6 +4014,7 @@ void qmp_x_blockdev_del(bool has_id, const char *id,
40054014
}
40064015

40074016
if (has_id) {
4017+
/* blk_by_name() never returns a BB that is not owned by the monitor */
40084018
blk = blk_by_name(id);
40094019
if (!blk) {
40104020
error_setg(errp, "Cannot find block backend %s", id);
@@ -4052,6 +4062,7 @@ void qmp_x_blockdev_del(bool has_id, const char *id,
40524062
}
40534063

40544064
if (blk) {
4065+
monitor_remove_blk(blk);
40554066
blk_unref(blk);
40564067
} else {
40574068
QTAILQ_REMOVE(&monitor_bdrv_states, bs, monitor_list);

‎device-hotplug.c

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,8 @@ void hmp_drive_add(Monitor *mon, const QDict *qdict)
8484

8585
err:
8686
if (dinfo) {
87-
blk_unref(blk_by_legacy_dinfo(dinfo));
87+
BlockBackend *blk = blk_by_legacy_dinfo(dinfo);
88+
monitor_remove_blk(blk);
89+
blk_unref(blk);
8890
}
8991
}

‎hw/block/xen_disk.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -917,7 +917,7 @@ static int blk_connect(struct XenDevice *xendev)
917917

918918
/* setup via xenbus -> create new block driver instance */
919919
xen_be_printf(&blkdev->xendev, 2, "create new bdrv (xenbus setup)\n");
920-
blkdev->blk = blk_new_open(blkdev->dev, blkdev->filename, NULL, options,
920+
blkdev->blk = blk_new_open(blkdev->filename, NULL, options,
921921
qflags, &local_err);
922922
if (!blkdev->blk) {
923923
xen_be_printf(&blkdev->xendev, 0, "error: %s\n",

‎include/sysemu/block-backend.h

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -59,11 +59,10 @@ typedef struct BlockDevOps {
5959
void (*resize_cb)(void *opaque);
6060
} BlockDevOps;
6161

62-
BlockBackend *blk_new(const char *name, Error **errp);
63-
BlockBackend *blk_new_with_bs(const char *name, Error **errp);
64-
BlockBackend *blk_new_open(const char *name, const char *filename,
65-
const char *reference, QDict *options, int flags,
66-
Error **errp);
62+
BlockBackend *blk_new(Error **errp);
63+
BlockBackend *blk_new_with_bs(Error **errp);
64+
BlockBackend *blk_new_open(const char *filename, const char *reference,
65+
QDict *options, int flags, Error **errp);
6766
int blk_get_refcnt(BlockBackend *blk);
6867
void blk_ref(BlockBackend *blk);
6968
void blk_unref(BlockBackend *blk);

0 commit comments

Comments
 (0)