On Jun 25, 2015, at 11:32 AM, Programmingkid wrote: > > On Jun 25, 2015, at 11:14 AM, Programmingkid wrote: > >> >> On Jun 25, 2015, at 2:53 AM, Markus Armbruster wrote: >> >>> Programmingkid <programmingk...@gmail.com> 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 <programmingk...@gmail.com >>>>>> <mailto:programmingk...@gmail.com>> >>>>>> >>>>>> 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? > > Thinking about your question some more, I see what you mean. On Linux > /dev/sr0 refers to the cdrom drive. Also on Linux, the /dev/cdrom link refers > to the /dev/sr0 device file. So if you just use /dev/cdrom, you are good.
Just had another idea. I could change the patch so that if it detects any device file being used, it handles it as raw. That way if you want to use /dev/sr0, you can. /* 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 + || strncmp("/dev/", filename, strlen("/dev/")) == 0) { *pdrv = &bdrv_raw; return ret; }