[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 01/23] block: Split bdrv_new_named() off bdrv_ne
From: |
Markus Armbruster |
Subject: |
Re: [Qemu-devel] [PATCH 01/23] block: Split bdrv_new_named() off bdrv_new() |
Date: |
Thu, 11 Sep 2014 10:29:50 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/24.3 (gnu/linux) |
Benoît Canet <address@hidden> writes:
> The Wednesday 10 Sep 2014 à 10:13:30 (+0200), Markus Armbruster wrote :
>> Creating an anonymous BDS can't fail. Make that obvious.
>>
>> Signed-off-by: Markus Armbruster <address@hidden>
>> ---
>> block.c | 26 +++++++++++++++++++-------
>> block/iscsi.c | 2 +-
>> block/vvfat.c | 2 +-
>> blockdev.c | 2 +-
>> hw/block/xen_disk.c | 2 +-
>> include/block/block.h | 3 ++-
>> qemu-img.c | 6 +++---
>> qemu-io.c | 2 +-
>> qemu-nbd.c | 2 +-
>> 9 files changed, 30 insertions(+), 17 deletions(-)
>>
>> diff --git a/block.c b/block.c
>> index d06dd51..4b3bcd4 100644
>> --- a/block.c
>> +++ b/block.c
>> @@ -335,10 +335,11 @@ void bdrv_register(BlockDriver *bdrv)
>> }
>>
>> /* create a new block device (by default it is empty) */
>> -BlockDriverState *bdrv_new(const char *device_name, Error **errp)
>> +BlockDriverState *bdrv_new_named(const char *device_name, Error **errp)
>> {
>> BlockDriverState *bs;
>> - int i;
>> +
>> + assert(*device_name);
>
> This assert that device_name is not a null pointer.
> But here we are pretty sure that the BDS should be named given the
> name of the function.
> Should we bake an assert on device_name[0] != '\0' to avoid
> bdrv_new_named being called
> with "" as device_name ?
>
>>
>> if (bdrv_find(device_name)) {
>> error_setg(errp, "Device with id '%s' already exists",
>> @@ -351,12 +352,23 @@ BlockDriverState *bdrv_new(const char *device_name,
>> Error **errp)
>> return NULL;
>> }
>>
>> - bs = g_new0(BlockDriverState, 1);
>> - QLIST_INIT(&bs->dirty_bitmaps);
>> + bs = bdrv_new();
>> +
>> pstrcpy(bs->device_name, sizeof(bs->device_name), device_name);
>
>> if (device_name[0] != '\0') {
> Then we could remove this test.
>
>> QTAILQ_INSERT_TAIL(&bdrv_states, bs, device_list);
> Because here would be too late anyway: an unammed BDS would have been
> created if device_name == "".
Short answer: not worth it, the function goes away in PATCH 08, and its
replacement works like you suggest.
Slightly longer answer: PATCH 02 adds BlockBackend, which also checks
device names. blk_new() does it the way you suggest. PATCH 08 removes
the now redundant BlockDriverState device name, including this function.
[...]
- [Qemu-devel] [PATCH 09/23] block: Merge BlockBackend and BlockDriverState name spaces, (continued)
- [Qemu-devel] [PATCH 09/23] block: Merge BlockBackend and BlockDriverState name spaces, Markus Armbruster, 2014/09/10
- [Qemu-devel] [PATCH 06/23] block: Eliminate bdrv_states, use block_next() instead, Markus Armbruster, 2014/09/10
- [Qemu-devel] [PATCH 07/23] block: Eliminate bdrv_iterate(), use bdrv_next(), Markus Armbruster, 2014/09/10
- [Qemu-devel] [PATCH 01/23] block: Split bdrv_new_named() off bdrv_new(), Markus Armbruster, 2014/09/10
- Re: [Qemu-devel] [PATCH 01/23] block: Split bdrv_new_named() off bdrv_new(), Eric Blake, 2014/09/10
- Re: [Qemu-devel] [PATCH 01/23] block: Split bdrv_new_named() off bdrv_new(), Benoît Canet, 2014/09/10
- Re: [Qemu-devel] [PATCH 01/23] block: Split bdrv_new_named() off bdrv_new(), Benoît Canet, 2014/09/10
- Re: [Qemu-devel] [PATCH 01/23] block: Split bdrv_new_named() off bdrv_new(), Fam Zheng, 2014/09/11
- [Qemu-devel] [PATCH 04/23] block: Connect BlockBackend and DriveInfo, Markus Armbruster, 2014/09/10
- Re: [Qemu-devel] [PATCH 04/23] block: Connect BlockBackend and DriveInfo, Kevin Wolf, 2014/09/10