Alberto Garcia <be...@igalia.com> writes:
> On Fri, Mar 20, 2015 at 08:40:32AM +0100, Markus Armbruster wrote:
>
>> > +const char *bdrv_get_device_or_node_name(const BlockDriverState *bs)
>> > +{
>> > + return bs->blk ? blk_name(bs->blk) : bs->node_name;
>> > +}
>> > +
>>
>> Does this have uses beyond identifying @bs to the user?
>
> None that I can think of, although apart from error messages it can
> also be used in data types, like the cases of BLOCK_IMAGE_CORRUPTED
> and BlockJobInfo that are being discussed:
>
> https://lists.gnu.org/archive/html/qemu-devel/2015-03/msg03651.html
Workable because BB and BDS names share a common name space. But is it
a good idea? No need to discuss this here if it's already discussed
elsewhere.
>> > static void quorum_report_failure(QuorumAIOCB *acb)
>> > {
>> > - const char *reference = bdrv_get_device_name(acb->common.bs)[0] ?
>> > - bdrv_get_device_name(acb->common.bs) :
>> > - acb->common.bs->node_name;
>> > -
>> > + const char *reference = bdrv_get_device_or_node_name(acb->common.bs);
>> > qapi_event_send_quorum_failure(reference, acb->sector_num,
>> > acb->nb_sectors, &error_abort);
>> > }
>>
>> Preexisting: what if reference is null?
>
> It can't happen, both the device and the node name strings are
> guaranteed to be non-null. The latter is actually a static string so
> there's no chance bs->node_name returns a null pointer.
You're right, I missed the fact that BDS member node_name is an array.
Suggest to add a function comment to bdrv_get_device_or_node_name()
explaining intended use, and that it never returns null.
