| Message ID | 1397749715-15701-4-git-send-email-kwolf@redhat.com |
|---|---|
| State | New |
| Headers | show |
On Thu, 04/17 17:48, Kevin Wolf wrote: > Since commit f298d071, block devices added with blockdev-add don't have > a QemuOpts around in dinfo->opts. Consequently, we can't rely any more > on QemuOpts catching duplicate IDs for block devices. > > This patch add a new check for duplicate IDs to bdrv_new(), and moves the s/add/adds/ > existing check that the ID isn't already taken for a node-name there as > well. > > Signed-off-by: Kevin Wolf <kwolf@redhat.com> > --- > block.c | 11 +++++++++++ > blockdev.c | 6 ------ > tests/qemu-iotests/087 | 33 +++++++++++++++++++++++++++++++++ > tests/qemu-iotests/087.out | 13 +++++++++++++ > 4 files changed, 57 insertions(+), 6 deletions(-) > > diff --git a/block.c b/block.c > index f3b93c9..fc2edd3 100644 > --- a/block.c > +++ b/block.c > @@ -336,6 +336,17 @@ BlockDriverState *bdrv_new(const char *device_name, Error **errp) > { > BlockDriverState *bs; > > + if (bdrv_find(device_name)) { > + error_setg(errp, "Device with id '%s' already exists", > + device_name); > + return NULL; > + } > + if (bdrv_find_node(device_name)) { > + error_setg(errp, "Device with node-name '%s' already exists", > + device_name); > + return NULL; > + } > + > bs = g_malloc0(sizeof(BlockDriverState)); > QLIST_INIT(&bs->dirty_bitmaps); > pstrcpy(bs->device_name, sizeof(bs->device_name), device_name); > diff --git a/blockdev.c b/blockdev.c > index 3a11a62..09826f1 100644 > --- a/blockdev.c > +++ b/blockdev.c > @@ -452,12 +452,6 @@ static DriveInfo *blockdev_init(const char *file, QDict *bs_opts, > } > } > > - if (bdrv_find_node(qemu_opts_id(opts))) { > - error_setg(errp, "device id=%s is conflicting with a node-name", > - qemu_opts_id(opts)); > - goto early_err; > - } > - > /* init */ > dinfo = g_malloc0(sizeof(*dinfo)); > dinfo->id = g_strdup(qemu_opts_id(opts)); > diff --git a/tests/qemu-iotests/087 b/tests/qemu-iotests/087 > index a38bb70..37d82fc 100755 > --- a/tests/qemu-iotests/087 > +++ b/tests/qemu-iotests/087 > @@ -73,6 +73,39 @@ run_qemu <<EOF > EOF > > echo > +echo === Duplicate ID === > +echo > + > +run_qemu <<EOF > +{ "execute": "qmp_capabilities" } > +{ "execute": "blockdev-add", > + "arguments": { > + "options": { > + "driver": "$IMGFMT", > + "id": "disk", > + "file": { > + "driver": "file", > + "filename": "$TEST_IMG" > + } > + } > + } > + } > +{ "execute": "blockdev-add", > + "arguments": { > + "options": { > + "driver": "$IMGFMT", > + "id": "disk", > + "file": { > + "driver": "file", > + "filename": "$TEST_IMG" > + } > + } > + } > + } > +{ "execute": "quit" } > +EOF > + > +echo > echo === aio=native without O_DIRECT === > echo > > diff --git a/tests/qemu-iotests/087.out b/tests/qemu-iotests/087.out > index e65dcdf..479bf86 100644 > --- a/tests/qemu-iotests/087.out > +++ b/tests/qemu-iotests/087.out > @@ -13,6 +13,19 @@ QMP_VERSION > {"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "DEVICE_TRAY_MOVED", "data": {"device": "floppy0", "tray-open": true}} > > > +=== Duplicate ID === > + > +Testing: > +QMP_VERSION > +{"return": {}} > +{"return": {}} > +{"error": {"class": "GenericError", "desc": "Device with id 'disk' already exists"}} > +{"return": {}} > +{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "SHUTDOWN"} > +{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "DEVICE_TRAY_MOVED", "data": {"device": "ide1-cd0", "tray-open": true}} > +{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "DEVICE_TRAY_MOVED", "data": {"device": "floppy0", "tray-open": true}} > + > + > === aio=native without O_DIRECT === > > Testing: > -- > 1.8.3.1 > >
On 04/17/2014 09:48 AM, Kevin Wolf wrote: > Since commit f298d071, block devices added with blockdev-add don't have > a QemuOpts around in dinfo->opts. Consequently, we can't rely any more > on QemuOpts catching duplicate IDs for block devices. > > This patch add a new check for duplicate IDs to bdrv_new(), and moves the > existing check that the ID isn't already taken for a node-name there as > well. > > Signed-off-by: Kevin Wolf <kwolf@redhat.com> > --- > block.c | 11 +++++++++++ > blockdev.c | 6 ------ > tests/qemu-iotests/087 | 33 +++++++++++++++++++++++++++++++++ > tests/qemu-iotests/087.out | 13 +++++++++++++ > 4 files changed, 57 insertions(+), 6 deletions(-) Modulo the commit message Fam pointed out... Reviewed-by: Eric Blake <eblake@redhat.com>
Am 21.04.2014 um 17:01 hat Eric Blake geschrieben: > On 04/17/2014 09:48 AM, Kevin Wolf wrote: > > Since commit f298d071, block devices added with blockdev-add don't have > > a QemuOpts around in dinfo->opts. Consequently, we can't rely any more > > on QemuOpts catching duplicate IDs for block devices. > > > > This patch add a new check for duplicate IDs to bdrv_new(), and moves the > > existing check that the ID isn't already taken for a node-name there as > > well. > > > > Signed-off-by: Kevin Wolf <kwolf@redhat.com> > > --- > > block.c | 11 +++++++++++ > > blockdev.c | 6 ------ > > tests/qemu-iotests/087 | 33 +++++++++++++++++++++++++++++++++ > > tests/qemu-iotests/087.out | 13 +++++++++++++ > > 4 files changed, 57 insertions(+), 6 deletions(-) > > Modulo the commit message Fam pointed out... > Reviewed-by: Eric Blake <eblake@redhat.com> Thanks, I fixed that typo locally before I added your Reviewed-by. Kevin
diff --git a/block.c b/block.c index f3b93c9..fc2edd3 100644 --- a/block.c +++ b/block.c @@ -336,6 +336,17 @@ BlockDriverState *bdrv_new(const char *device_name, Error **errp) { BlockDriverState *bs; + if (bdrv_find(device_name)) { + error_setg(errp, "Device with id '%s' already exists", + device_name); + return NULL; + } + if (bdrv_find_node(device_name)) { + error_setg(errp, "Device with node-name '%s' already exists", + device_name); + return NULL; + } + bs = g_malloc0(sizeof(BlockDriverState)); QLIST_INIT(&bs->dirty_bitmaps); pstrcpy(bs->device_name, sizeof(bs->device_name), device_name); diff --git a/blockdev.c b/blockdev.c index 3a11a62..09826f1 100644 --- a/blockdev.c +++ b/blockdev.c @@ -452,12 +452,6 @@ static DriveInfo *blockdev_init(const char *file, QDict *bs_opts, } } - if (bdrv_find_node(qemu_opts_id(opts))) { - error_setg(errp, "device id=%s is conflicting with a node-name", - qemu_opts_id(opts)); - goto early_err; - } - /* init */ dinfo = g_malloc0(sizeof(*dinfo)); dinfo->id = g_strdup(qemu_opts_id(opts)); diff --git a/tests/qemu-iotests/087 b/tests/qemu-iotests/087 index a38bb70..37d82fc 100755 --- a/tests/qemu-iotests/087 +++ b/tests/qemu-iotests/087 @@ -73,6 +73,39 @@ run_qemu <<EOF EOF echo +echo === Duplicate ID === +echo + +run_qemu <<EOF +{ "execute": "qmp_capabilities" } +{ "execute": "blockdev-add", + "arguments": { + "options": { + "driver": "$IMGFMT", + "id": "disk", + "file": { + "driver": "file", + "filename": "$TEST_IMG" + } + } + } + } +{ "execute": "blockdev-add", + "arguments": { + "options": { + "driver": "$IMGFMT", + "id": "disk", + "file": { + "driver": "file", + "filename": "$TEST_IMG" + } + } + } + } +{ "execute": "quit" } +EOF + +echo echo === aio=native without O_DIRECT === echo diff --git a/tests/qemu-iotests/087.out b/tests/qemu-iotests/087.out index e65dcdf..479bf86 100644 --- a/tests/qemu-iotests/087.out +++ b/tests/qemu-iotests/087.out @@ -13,6 +13,19 @@ QMP_VERSION {"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "DEVICE_TRAY_MOVED", "data": {"device": "floppy0", "tray-open": true}} +=== Duplicate ID === + +Testing: +QMP_VERSION +{"return": {}} +{"return": {}} +{"error": {"class": "GenericError", "desc": "Device with id 'disk' already exists"}} +{"return": {}} +{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "SHUTDOWN"} +{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "DEVICE_TRAY_MOVED", "data": {"device": "ide1-cd0", "tray-open": true}} +{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "DEVICE_TRAY_MOVED", "data": {"device": "floppy0", "tray-open": true}} + + === aio=native without O_DIRECT === Testing:
Since commit f298d071, block devices added with blockdev-add don't have a QemuOpts around in dinfo->opts. Consequently, we can't rely any more on QemuOpts catching duplicate IDs for block devices. This patch add a new check for duplicate IDs to bdrv_new(), and moves the existing check that the ID isn't already taken for a node-name there as well. Signed-off-by: Kevin Wolf <kwolf@redhat.com> --- block.c | 11 +++++++++++ blockdev.c | 6 ------ tests/qemu-iotests/087 | 33 +++++++++++++++++++++++++++++++++ tests/qemu-iotests/087.out | 13 +++++++++++++ 4 files changed, 57 insertions(+), 6 deletions(-)