Re: [Qemu-devel] [Qemu-block] [PATCH] block/null: Remove 'filename' option

[Jeff Cody <jcody@redhat.com>]

On Fri, Aug 04, 2017 at 04:43:54PM +0200, Kevin Wolf wrote:
> This option was only added to allow 'null-co://' and 'null-aio://' as
> filenames, its value never served any actual purpose and was ignored.
> Nevertheless it was accepted as '-drive driver=null,filename=foo'.
> 
> The correct way to enable the protocol prefixes (and that without adding
> a useless -drive option) is implementing .bdrv_parse_filename. This is
> what this patch does.
> 
> Technically, this is an incompatible change, but the null block driver
> is only used for benchmarking, testing and debugging, and an option
> without effect isn't likely to be used by anyone anyway, so no bad
> effects are to be expected.
> 
> Reported-by: Markus Armbruster <armbru@redhat.com>
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
>  block/null.c | 31 ++++++++++++++++++++++++++-----
>  1 file changed, 26 insertions(+), 5 deletions(-)
> 
> diff --git a/block/null.c b/block/null.c
> index 876f90965b..dd9c13f9ba 100644
> --- a/block/null.c
> +++ b/block/null.c
> @@ -30,11 +30,6 @@ static QemuOptsList runtime_opts = {
>      .head = QTAILQ_HEAD_INITIALIZER(runtime_opts.head),
>      .desc = {
>          {
> -            .name = "filename",
> -            .type = QEMU_OPT_STRING,
> -            .help = "",
> -        },
> -        {
>              .name = BLOCK_OPT_SIZE,
>              .type = QEMU_OPT_SIZE,
>              .help = "size of the null block",
> @@ -54,6 +49,30 @@ static QemuOptsList runtime_opts = {
>      },
>  };
>  
> +static void null_co_parse_filename(const char *filename, QDict *options,
> +                                   Error **errp)
> +{
> +    /* This functions only exists so that a null-co:// filename is accepted
> +     * with the null-co driver. */
> +    if (strcmp(filename, "null-co://")) {
> +        error_setg(errp, "The only allowed filename for this driver is "
> +                         "'null-co://'");
> +        return;
> +    }
> +}
> +
> +static void null_aio_parse_filename(const char *filename, QDict *options,
> +                                    Error **errp)
> +{
> +    /* This functions only exists so that a null-aio:// filename is accepted
> +     * with the null-aio driver. */
> +    if (strcmp(filename, "null-aio://")) {
> +        error_setg(errp, "The only allowed filename for this driver is "
> +                         "'null-aio://'");
> +        return;
> +    }
> +}
> +
>  static int null_file_open(BlockDriverState *bs, QDict *options, int flags,
>                            Error **errp)
>  {
> @@ -242,6 +261,7 @@ static BlockDriver bdrv_null_co = {
>      .instance_size          = sizeof(BDRVNullState),
>  
>      .bdrv_file_open         = null_file_open,
> +    .bdrv_parse_filename    = null_co_parse_filename,
>      .bdrv_close             = null_close,
>      .bdrv_getlength         = null_getlength,
>  
> @@ -261,6 +281,7 @@ static BlockDriver bdrv_null_aio = {
>      .instance_size          = sizeof(BDRVNullState),
>  
>      .bdrv_file_open         = null_file_open,
> +    .bdrv_parse_filename    = null_aio_parse_filename,
>      .bdrv_close             = null_close,
>      .bdrv_getlength         = null_getlength,
>  
> -- 
> 2.13.3
> 
> 

Reviewed-by: Jeff Cody <jcody@redhat.com>