Re: [Qemu-devel] [PATCH v3 3/5] block/null: Implement bdrv_refresh_filename()

[Max Reitz]

On 12.06.2016 06:08, Fam Zheng wrote:
> On Fri, 06/10 20:57, Max Reitz wrote:
>> Signed-off-by: Max Reitz <address@hidden>
> 
> The commit message could go a little more informative. Seems nothing special 
> in
> the added callback, aren't things supposed to just work without this patch?
> What is missing?

If you pass a filename, it works without this patch. If you don't, it
doesn't.

Compare (before this patch):

$ x86_64-softmmu/qemu-system-x86_64 \
  -drive if=none,file=null-co://,driver=raw -nodefaults -qmp stdio
[...]
{'execute':'query-block'}
[...] "filename": "null-co://", [...]

With:

$ x86_64-softmmu/qemu-system-x86_64 \
  -drive if=none,driver=raw,file.driver=null-co -nodefaults -qmp stdio
[...]
{'execute':'query-block'}
[...] "filename": "json:{\"driver\": \"raw\", \"file\": {\"driver\":
\"null-co\"}}", [...]


So the issue is that you can use the null-co/aio drivers without giving
a filename.

I wouldn't mind including this information in a v4, but I'm not sure it
alone warrants a v4.

> 
>> ---
>>  block/null.c | 20 ++++++++++++++++++++
>>  1 file changed, 20 insertions(+)
>>
>> diff --git a/block/null.c b/block/null.c
>> index 396500b..b511010 100644
>> --- a/block/null.c
>> +++ b/block/null.c
>> @@ -12,6 +12,8 @@
>>  
>>  #include "qemu/osdep.h"
>>  #include "qapi/error.h"
>> +#include "qapi/qmp/qdict.h"
>> +#include "qapi/qmp/qstring.h"
>>  #include "block/block_int.h"
>>  
>>  #define NULL_OPT_LATENCY "latency-ns"
>> @@ -223,6 +225,20 @@ static int64_t coroutine_fn 
>> null_co_get_block_status(BlockDriverState *bs,
>>      }
>>  }
>>  
>> +static void null_refresh_filename(BlockDriverState *bs, QDict *opts)
>> +{
>> +    QINCREF(opts);
>> +    qdict_del(opts, "filename");
> 
> Why is this qdict_del necessary?

It's not strictly necessary. But the null drivers ignore the filename,
so there's no harm in dropping it from the JSON filename (if we need to
construct one).

Max

>> +
>> +    if (!qdict_size(opts)) {
>> +        snprintf(bs->exact_filename, sizeof(bs->exact_filename), "%s://",
>> +                 bs->drv->format_name);
>> +    }
>> +
>> +    qdict_put(opts, "driver", qstring_from_str(bs->drv->format_name));
>> +    bs->full_open_options = opts;
>> +}
>> +
>>  static BlockDriver bdrv_null_co = {
>>      .format_name            = "null-co",
>>      .protocol_name          = "null-co",
>> @@ -238,6 +254,8 @@ static BlockDriver bdrv_null_co = {
>>      .bdrv_reopen_prepare    = null_reopen_prepare,
>>  
>>      .bdrv_co_get_block_status   = null_co_get_block_status,
>> +
>> +    .bdrv_refresh_filename  = null_refresh_filename,
>>  };
>>  
>>  static BlockDriver bdrv_null_aio = {
>> @@ -255,6 +273,8 @@ static BlockDriver bdrv_null_aio = {
>>      .bdrv_reopen_prepare    = null_reopen_prepare,
>>  
>>      .bdrv_co_get_block_status   = null_co_get_block_status,
>> +
>> +    .bdrv_refresh_filename  = null_refresh_filename,
>>  };
>>  
>>  static void bdrv_null_init(void)
>> -- 
>> 2.8.3
>>

signature.asc
Description: OpenPGP digital signature