Re: [Qemu-devel] [PATCH v9 05/11] block: allow block jobs in any arbitrary node

[Alberto Garcia]

On Wed 27 Apr 2016 02:30:55 PM CEST, Max Reitz wrote:
>> @@ -3694,18 +3696,18 @@ static BlockJob *find_block_job(const char *device, 
>> AioContext **aio_context,
>>  
>>      *aio_context = NULL;
>>  
>> -    blk = blk_by_name(device);
>> -    if (!blk) {
>> +    bs = bdrv_lookup_bs(device_or_node, device_or_node, errp);
>> +    if (!bs) {
>
> If we get here, *errp is already set... [1]

Good catch, thanks.

>> -    if (!blk_is_available(blk)) {
>> +    blk = blk_by_name(device_or_node);
>> +    if (blk && !blk_is_available(blk)) {
>
> I'd just drop this. The reason blk_is_available() was added here
> because it became possible for BBs not to have a BDS.
>
> Now that you get the BDS directly through bdrv_lookup_bs(), it's no
> longer necessary.

Ok.

>> @@ -78,7 +79,7 @@ void *block_job_create(const BlockJobDriver *driver, 
>> BlockDriverState *bs,
>>      bdrv_op_unblock(bs, BLOCK_OP_TYPE_DATAPLANE, job->blocker);
>>  
>>      job->driver        = driver;
>> -    job->id            = g_strdup(bdrv_get_device_name(bs));
>> +    job->id            = g_strdup(bdrv_get_device_or_node_name(bs));
>
> Hm, since this is only used for events, it's not too bad that a block
> job will have a different name if it was created on a root BDS by
> specifying its node name. But it still is kind of strange.

The idea is that if you create a block job on a root BDS (which is still
the most common case) you get the same id that you used to get before
this series.

Berto