[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v3 3/5] block/null: Implement bdrv_refresh_filen
From: |
Max Reitz |
Subject: |
Re: [Qemu-devel] [PATCH v3 3/5] block/null: Implement bdrv_refresh_filename() |
Date: |
Tue, 14 Jun 2016 14:59:35 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.1.1 |
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
- [Qemu-devel] [PATCH v3 0/5] block/mirror: Fix (?) target backing BDS, Max Reitz, 2016/06/10
- [Qemu-devel] [PATCH v3 1/5] block: Allow replacement of a BDS by its overlay, Max Reitz, 2016/06/10
- [Qemu-devel] [PATCH v3 4/5] iotests: Add test for post-mirror backing chains, Max Reitz, 2016/06/10
- [Qemu-devel] [PATCH v3 3/5] block/null: Implement bdrv_refresh_filename(), Max Reitz, 2016/06/10
- [Qemu-devel] [PATCH v3 5/5] iotests: Add test for oVirt-like storage migration, Max Reitz, 2016/06/10
- [Qemu-devel] [PATCH v3 2/5] block/mirror: Fix target backing BDS, Max Reitz, 2016/06/10
- Re: [Qemu-devel] [PATCH v3 0/5] block/mirror: Fix (?) target backing BDS, Fam Zheng, 2016/06/12
- Re: [Qemu-devel] [PATCH v3 0/5] block/mirror: Fix (?) target backing BDS, Kevin Wolf, 2016/06/13
- Re: [Qemu-devel] [PATCH v3 0/5] block/mirror: Fix (?) target backing BDS, Max Reitz, 2016/06/14