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; } You have introduced yourself the support for raw_getlength() for MacOS X: commit 728dacbda817b2ca259e9d337fab06bcf14e94a6 Author: Programmingkid <programmingk...@gmail.com> Date: Mon Jan 19 17:12:55 2015 -0500 block/raw-posix.c: Fix raw_getlength() on Mac OS X block devices This patch replaces the dummy code in raw_getlength() for block devices on OS X, which always returned LLONG_MAX, with a real implementation that returns the actual block device size. Then, just "#ifdef CONFIG_BSD" around the existing bdrv_host_cdrom of FreeBSD (minus cdrom_eject and cdrom_lock_medium) and bdrv_register(). Laurent