Message ID | 1403809762-4858-2-git-send-email-mreitz@redhat.com |
---|---|
State | New |
Headers | show |
On 06/26/2014 01:09 PM, Max Reitz wrote: > If "filename" is removed from the options QDict before entering > bdrv_open_common(), it cannot be stored in the BDS. Therefore, wait > until it has been copied there and remove it from the options only > afterwards. > > This fixes "filename" in the BDS being empty for block drivers which do > not need the filename because they have parsed it already (e.g. NBD). > > Signed-off-by: Max Reitz <mreitz@redhat.com> > --- > block.c | 24 +++++++++++++++++------- > 1 file changed, 17 insertions(+), 7 deletions(-) > How does this play with Kevin's bdrv_open cleanups? https://lists.gnu.org/archive/html/qemu-devel/2014-06/msg06173.html
On 26.06.2014 21:46, Eric Blake wrote: > On 06/26/2014 01:09 PM, Max Reitz wrote: >> If "filename" is removed from the options QDict before entering >> bdrv_open_common(), it cannot be stored in the BDS. Therefore, wait >> until it has been copied there and remove it from the options only >> afterwards. >> >> This fixes "filename" in the BDS being empty for block drivers which do >> not need the filename because they have parsed it already (e.g. NBD). >> >> Signed-off-by: Max Reitz <mreitz@redhat.com> >> --- >> block.c | 24 +++++++++++++++++------- >> 1 file changed, 17 insertions(+), 7 deletions(-) >> > How does this play with Kevin's bdrv_open cleanups? > https://lists.gnu.org/archive/html/qemu-devel/2014-06/msg06173.html To be honest, I hoped to get this series merged before Kevin's, because I guessed adapting his series to this would be easier than the other way round. ;-) I guess there will be some conflicts but nothing unfixable (and the concept will stay the same), as he still leaves bdrv_open_common() intact. Max
On 26.06.2014 22:01, Max Reitz wrote: > On 26.06.2014 21:46, Eric Blake wrote: >> On 06/26/2014 01:09 PM, Max Reitz wrote: >>> If "filename" is removed from the options QDict before entering >>> bdrv_open_common(), it cannot be stored in the BDS. Therefore, wait >>> until it has been copied there and remove it from the options only >>> afterwards. >>> >>> This fixes "filename" in the BDS being empty for block drivers which do >>> not need the filename because they have parsed it already (e.g. NBD). >>> >>> Signed-off-by: Max Reitz <mreitz@redhat.com> >>> --- >>> block.c | 24 +++++++++++++++++------- >>> 1 file changed, 17 insertions(+), 7 deletions(-) >>> >> How does this play with Kevin's bdrv_open cleanups? >> https://lists.gnu.org/archive/html/qemu-devel/2014-06/msg06173.html > > To be honest, I hoped to get this series merged before Kevin's, > because I guessed adapting his series to this would be easier than the > other way round. ;-) > > I guess there will be some conflicts but nothing unfixable (and the > concept will stay the same), as he still leaves bdrv_open_common() intact. Hm, now that I'm seeing Kevin merged his series already, I'll see to fix eventual conflicts myself. Max
diff --git a/block.c b/block.c index ff44e76..850c5d7 100644 --- a/block.c +++ b/block.c @@ -881,7 +881,8 @@ static void bdrv_assign_node_name(BlockDriverState *bs, * Removes all processed options from *options. */ static int bdrv_open_common(BlockDriverState *bs, BlockDriverState *file, - QDict *options, int flags, BlockDriver *drv, Error **errp) + QDict *options, int flags, BlockDriver *drv, bool parsed_filename, + Error **errp) { int ret, open_flags; const char *filename; @@ -954,6 +955,14 @@ static int bdrv_open_common(BlockDriverState *bs, BlockDriverState *file, bs->filename[0] = '\0'; } + if (parsed_filename && !drv->bdrv_needs_filename) { + qdict_del(options, "filename"); + /* The pointer "filename" may reference the just deleted QDict entry; in + * any case, it is no longer needed, so indicate that by setting it to + * NULL. */ + filename = NULL; + } + bs->drv = drv; bs->opaque = g_malloc0(drv->instance_size); @@ -1020,7 +1029,7 @@ static int bdrv_file_open(BlockDriverState *bs, const char *filename, { BlockDriver *drv; const char *drvname; - bool parse_filename = false; + bool parse_filename = false, parsed_filename = false; Error *local_err = NULL; int ret; @@ -1070,18 +1079,19 @@ static int bdrv_file_open(BlockDriverState *bs, const char *filename, goto fail; } - if (!drv->bdrv_needs_filename) { - qdict_del(*options, "filename"); - } else { + if (drv->bdrv_needs_filename) { filename = qdict_get_str(*options, "filename"); } + + parsed_filename = true; } if (!drv->bdrv_file_open) { ret = bdrv_open(&bs, filename, NULL, *options, flags, drv, &local_err); *options = NULL; } else { - ret = bdrv_open_common(bs, NULL, *options, flags, drv, &local_err); + ret = bdrv_open_common(bs, NULL, *options, flags, drv, parsed_filename, + &local_err); } if (ret < 0) { error_propagate(errp, local_err); @@ -1471,7 +1481,7 @@ int bdrv_open(BlockDriverState **pbs, const char *filename, } /* Open the image */ - ret = bdrv_open_common(bs, file, options, flags, drv, &local_err); + ret = bdrv_open_common(bs, file, options, flags, drv, false, &local_err); if (ret < 0) { goto fail; }
If "filename" is removed from the options QDict before entering bdrv_open_common(), it cannot be stored in the BDS. Therefore, wait until it has been copied there and remove it from the options only afterwards. This fixes "filename" in the BDS being empty for block drivers which do not need the filename because they have parsed it already (e.g. NBD). Signed-off-by: Max Reitz <mreitz@redhat.com> --- block.c | 24 +++++++++++++++++------- 1 file changed, 17 insertions(+), 7 deletions(-)