Re: [Qemu-devel] [PATCH 3/5] block/raw-posix: bdrv_parse_filename() for cdrom

["Benoît Canet" <benoit.canet@irqsave.net>]

The Thursday 06 Mar 2014 à 23:25:37 (+0100), Max Reitz wrote :
> The "host_cdrom" protocol drivers should strip the "host_cdrom:" prefix
> from filenames if present.
> 
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
>  block/raw-posix.c | 20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)
> 
> diff --git a/block/raw-posix.c b/block/raw-posix.c
> index 4b8c183..6c9b8f2 100644
> --- a/block/raw-posix.c
> +++ b/block/raw-posix.c
> @@ -1984,6 +1984,15 @@ static BlockDriver bdrv_host_floppy = {
>      .bdrv_eject         = floppy_eject,
>  };
>  
> +static void cdrom_parse_filename(const char *filename, QDict *options,
> +                                 Error **errp)
> +{
> +    /* The prefix is optional, just as for "file". */
> +    strstart(filename, "host_cdrom:", &filename);
> +
> +    qdict_put_obj(options, "filename", QOBJECT(qstring_from_str(filename)));
> +}
> +
>  static int cdrom_open(BlockDriverState *bs, QDict *options, int flags,
>                        Error **errp)
>  {
> @@ -2070,6 +2079,7 @@ static BlockDriver bdrv_host_cdrom = {
>      .instance_size      = sizeof(BDRVRawState),
>      .bdrv_needs_filename = true,
>      .bdrv_probe_device	= cdrom_probe_device,
> +    .bdrv_parse_filename = cdrom_parse_filename,
>      .bdrv_file_open     = cdrom_open,
>      .bdrv_close         = raw_close,
>      .bdrv_reopen_prepare = raw_reopen_prepare,
> @@ -2101,6 +2111,15 @@ static BlockDriver bdrv_host_cdrom = {
>  #endif /* __linux__ */
>  
>  #if defined (__FreeBSD__) || defined(__FreeBSD_kernel__)
> +static void cdrom_parse_filename(const char *filename, QDict *options,
> +                                 Error **errp)
> +{
> +    /* The prefix is optional, just as for "file". */
> +    strstart(filename, "host_cdrom:", &filename);
> +
> +    qdict_put_obj(options, "filename", QOBJECT(qstring_from_str(filename)));
> +}
> +

The cdrom_probe_device and cdrom_open function actually differs and are not
duplicated whereas these two instances of cdrom_parse_filename are pretty much
the same.
Why not putting a merged version on the top of the file and guarded by
 #if defined(__linux__) || defined (__FreeBSD__) || defined(__FreeBSD_kernel__)
?


>  static int cdrom_open(BlockDriverState *bs, QDict *options, int flags,
>                        Error **errp)
>  {
> @@ -2200,6 +2219,7 @@ static BlockDriver bdrv_host_cdrom = {
>      .instance_size      = sizeof(BDRVRawState),
>      .bdrv_needs_filename = true,
>      .bdrv_probe_device	= cdrom_probe_device,
> +    .bdrv_parse_filename = cdrom_parse_filename,
>      .bdrv_file_open     = cdrom_open,
>      .bdrv_close         = raw_close,
>      .bdrv_reopen_prepare = raw_reopen_prepare,
> -- 
> 1.9.0
>