[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 08/23] block: Eliminate BlockDriverState member
From: |
Eric Blake |
Subject: |
Re: [Qemu-devel] [PATCH 08/23] block: Eliminate BlockDriverState member device_name[] |
Date: |
Thu, 11 Sep 2014 07:00:41 -0600 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.7.0 |
On 09/11/2014 05:34 AM, Benoît Canet wrote:
> The Wednesday 10 Sep 2014 à 10:13:37 (+0200), Markus Armbruster wrote :
>> device_name[] is can become non-empty only in bdrv_new_named() and
>> bdrv_move_feature_fields(). The latter is used only to undo damage
>> done by bdrv_swap(). The former is called only by blk_new_with_bs().
>> Therefore, when a BlockDriverState's device_name[] is non-empty, then
>> it's owned by a BlockBackend.
[lots of lines trimmed - it's not only okay, but desirable to trim out
portions of a patch that you are okay with, in order to call attention
to the problem spots that you are commenting on without making the
reader have to scroll through pages of quoted context]
>>
>> -const char *bdrv_get_device_name(BlockDriverState *bs)
>> +const char *bdrv_get_device_name(const BlockDriverState *bs)
>> {
>> - return bs->device_name;
>> + const char *name = bs->blk ? blk_name(bs->blk) : NULL;
>> + return name ?: "";
>> }
>
> Why not ?
>
> return bs->blk ? blk_name(bs->blk) : "";
If I understand right, it was because blk_name(bs->blk) may return NULL,
but this function is guaranteed to return non-NULL.
>
> or
>
> if (bs->blk) {
> return blk_name(bs->blk);
> }
>
> return "";
>
> Would it fail to do the job ?
>
> Also we could have made sure that bdrv_get_device_name return either
> a non empty string or NULL.
>
> (We know blk_name will return a non empty string it's asserted)
>
> This would need to change a few test all around the code but the final
> code logic would be less convoluted:
> -convert NULL to "" then test for not ""
> would turn into
> -return NULL test for not NULL
Indeed, the logic of NULL vs. string is slightly easier than the logic
of "" vs non-empty string; if we can guarantee that a non-NULL string
will be non-empty. I'm not sure how much churn it would cause, though.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
- Re: [Qemu-devel] [PATCH 10/23] block: Eliminate DriveInfo member bdrv, use blk_by_legacy_dinfo(), (continued)
[Qemu-devel] [PATCH 15/23] ide: Complete conversion from BlockDriverState to BlockBackend, Markus Armbruster, 2014/09/10
[Qemu-devel] [PATCH 19/23] blockdev: Drop DriveInfo member enable_auto_del, Markus Armbruster, 2014/09/10
[Qemu-devel] [PATCH 13/23] virtio-blk: Rename VirtIOBlkConf variables to conf, Markus Armbruster, 2014/09/10
[Qemu-devel] [PATCH 20/23] block/qapi: Convert qmp_query_block() to BlockBackend, Markus Armbruster, 2014/09/10
[Qemu-devel] [PATCH 17/23] blockdev: Drop superfluous DriveInfo member id, Markus Armbruster, 2014/09/10
[Qemu-devel] [PATCH 18/23] blockdev: Fix blockdev-add not to create IDE drive (0, 0), Markus Armbruster, 2014/09/10
[Qemu-devel] [PATCH 23/23] block: Make device model's references to BlockBackend strong, Markus Armbruster, 2014/09/10