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

[Max Reitz <mreitz@redhat.com>]

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 <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__)
> ?

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