Re: [Qemu-devel] [PATCH] block.c: fix real cdrom detection

[Programmingkid]

On Jun 25, 2015, at 2:53 AM, Markus Armbruster wrote:

> Programmingkid <address@hidden> writes:
> 
>> On Jun 23, 2015, at 2:06 PM, John Snow wrote:
>> 
>>> 
>>> 
>>> On 06/23/2015 01:56 PM, Programmingkid wrote:
>>>> Fix real cdrom detection so that a real cdrom can actually be used.
>>>> 
>>>> signed-off-by: John Arbuckle <address@hidden
>>>> <mailto:address@hidden>>
>>>> 
>>>> This patch has been tested on Mac OS X host and guest. 
>>>> Command used: qemu-system-ppc -cdrom /dev/cdrom
>>>> 
>>>> Note: I was able to view the files using OpenBIOS, but not on 
>>>> Mac OS X. The size of the disc is reported correctly but some
>>>> error happens that prevents it from mounting in Mac OS X. This
>>>> is probably another bug with QEMU.
>>>> 
>>>> ---
>>>> block.c |    3 ++-
>>>> 1 files changed, 2 insertions(+), 1 deletions(-)
>>>> 
>>>> diff --git a/block.c b/block.c
>>>> index dd4f58d..75ccfad 100644
>>>> --- a/block.c
>>>> +++ b/block.c
>>>> @@ -583,7 +583,8 @@ static int find_image_format(BlockDriverState *bs,
>>>> const char *filename,
>>>>    int ret = 0;
>>>> 
>>>> 
>>>> 
>>>>    /* Return the raw BlockDriver * to scsi-generic devices or empty
>>>> drives */
>>>> -    if (bs->sg || !bdrv_is_inserted(bs) || bdrv_getlength(bs) == 0) {
>>>> +    if (bs->sg || !bdrv_is_inserted(bs) || bdrv_getlength(bs) == 0 
>>>> +               || strcmp("/dev/cdrom", filename) == 0) {
>>>>        *pdrv = &bdrv_raw;
>>>>        return ret;
>>>>    }
>>>> -- 
>>>> 1.7.5.4
>>>> 
>>> 
>>> So what's the issue that this patch attempts to fix and how did you
>>> determine that the fix was needed here? It doesn't look like it respects
>>> proper abstraction at a glance.
>> 
>> Without the patch, QEMU would just quit when the "-cdrom /dev/cdrom"
>> option is given.
>> 
>> Before the patch, the bdrv_open_inherit() function would be
>> incorrectly called. Its documentation says "Opens a disk image (raw,
>> qcow2, vmdk, ...)" meaning only for disk image files (not for real
>> media). This patch prevents the bdrv_open_inherit() function from ever
>> being called. It sets the pdrv variable to the raw format. This made
>> sense to me since a real cdrom is read in the raw format.
>> 
>> A quick test does show the patch works. A real cdrom is successfully
>> opened on qemu-system-i386 using a Windows XP guest.
> 
> What about /dev/sr0, /dev/sr1, and whatever other names could refer to a
> block device without a medium?
> 
> Comparing filenames isn't a good way to test "is a block device without
> a medium".

I think we are going to have to agree to disagree. I have never used the 
/dev/sr(0 | 1) devices and don't see how they would be effected by this patch. 
Are you trying to say the /dev/sr(0 | 1) devices *should* be handled by this 
patch?