On Jun 29, 2015, at 4:43 PM, Laurent Vivier wrote: > > > On 29/06/2015 20:37, Programmingkid wrote: >> >> On Jun 29, 2015, at 2:16 PM, Peter Maydell wrote: >> >>> On 29 June 2015 at 19:04, Programmingkid <programmingk...@gmail.com >>> <mailto:programmingk...@gmail.com>> wrote: >>>> >>>> On Jun 29, 2015, at 1:11 PM, Peter Maydell wrote: >>>> >>>>> On 29 June 2015 at 17:54, Programmingkid <programmingk...@gmail.com >>>>> <mailto:programmingk...@gmail.com>> wrote: >>>>>> @@ -2365,6 +2384,10 @@ static BlockDriver bdrv_host_device = { >>>>>> .bdrv_ioctl = hdev_ioctl, >>>>>> .bdrv_aio_ioctl = hdev_aio_ioctl, >>>>>> #endif >>>>>> + >>>>>> +#ifdef __APPLE__ >>>>>> + .bdrv_is_inserted = cdrom_is_inserted, >>>>>> +#endif >>>>> >>>>> Why isn't this handled by having a bdrv_host_cdrom, >>>>> like Linux and FreeBSD do for their CDROM support? >>>> >>>> That would involve a lot of unnecessary work and modifications. This >>>> small change is all that is needed. >>> >>> Yes, but it's obviously wrong, because this: >>> >>> + if (count == 0) { >>> + count++; >>> + returnValue = 0; /* get around find_image_format() issue */ >>> + } >>> >>> makes no sense at all -- this means that we'll always report "drive >>> empty" the first time this function is called. We should always >>> report the correct answer, regardless of who's calling us. >>> >>> If you find yourself writing this kind of weird workaround, it >>> generally suggests that the change is a "this happens to make it >>> work" patch, not the correct fix for the problem. We need clean >>> fixes in QEMU, because if we allow "happens to make it work" >>> patches to pile up then the whole system becomes unmaintainable. >>> Yes, this often means that the amount of work required to >>> fix a bug is more than a handful of lines. That doesn't mean >>> that the work is unnecessary. >>> >>> (For instance, what happens if somebody changes some other >>> part of QEMU so that it happens that find_image_format() is not >>> the first thing to call this function?) >>> >>> We know the correct way to support host cdrom drives, because >>> we're already doing that on Linux. We should consistently >>> support host cdrom drives the same way for all hosts. >> >> I have really tried to find out what was wrong. It is a asynchronous, >> multi-threaded mess. Trying to follow where QEMU messes up >> was hard. The closest I came to was to a function called >> bdrv_co_io_em(). It was returning a value of -22. >> >> If some change does happen to make this patch to >> not work anymore, I can easily fix it. > > Frankly, I don't understand you. > > The only thing you have to do is to write: > > static int cdrom_is_inserted(BlockDriverState *bs) > { > return raw_getlength(bs) > 0; > }
Yes, this is probably the correct implementation for cdrom_is_inserted(), but what I'm trying to do is to make a real cdrom work in QEMU. This implementation of cdrom_is_inserted() doesn't solve the problem with find_image_format(). The problem still causes QEMU to quit when using the option "-cdrom /dev/cdrom". My patch I sent you before does fix things, but it is viewed as a hack. I was hoping the patch might be temporarily accepted until better solution was made. That is not going to happen :(