| Message ID | 20160610185750.30956-4-mreitz@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, 06/10 20:57, Max Reitz wrote: > Signed-off-by: Max Reitz <mreitz@redhat.com> 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? > --- > 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? > + > + 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 >
On 12.06.2016 06:08, Fam Zheng wrote: > On Fri, 06/10 20:57, Max Reitz wrote: >> Signed-off-by: Max Reitz <mreitz@redhat.com> > > 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 >>
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"); + + 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)
Signed-off-by: Max Reitz <mreitz@redhat.com> --- block/null.c | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+)