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

[Max Reitz]

On 06.03.2014 23:37, Benoît Canet wrote:
> 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 <address@hidden>
> > ---
> >   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__)
> ?

I didn't want to since the #ifdef linux began before the floppy driver 
and ended after the Linux CD-ROM driver, so I'd have to split it up. But 
since you're suggesting it now, I'll do it. ;-)

Max

> >   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