Kevin Wolf <kw...@redhat.com> writes: > Am 01.06.2011 17:32, schrieb Markus Armbruster: >> Kevin Wolf <kw...@redhat.com> writes: >> >>> Am 01.06.2011 15:44, schrieb Luiz Capitulino: >>>> On Thu, 26 May 2011 18:12:08 -0300 >>>> Luiz Capitulino <lcapitul...@redhat.com> wrote: >>>> >>>>> On Thu, 19 May 2011 14:33:19 +0200 >>>>> Kevin Wolf <kw...@redhat.com> wrote: >>>>> >>>>>> These printfs aren't really debug messages, but clearly indicate a bug >>>>>> if they >>>>>> ever become effective. >>>>> >>>>> Then we have a bug somewhere, starting a VM with: >>>>> >>>>> # qemu -hda disks/test.img -enable-kvm -m 1G -cdrom /dev/sr0 >>>>> >>>>> Where the host's CDROM is empty, triggers one of these asserts: >>>>> >>>>> qmp-unstable/hw/ide/pci.c:299: bmdma_cmd_writeb: Assertion >>>>> `bm->bus->dma->aiocb == ((void *)0)' >>>> >>>> I found out why this is happening. I'm passing '-snapshot' to the >>>> command-line, >>>> sorry for not mentioning it (I forgot I was using my devel alias). >>> >>> And suddenly it's reproducible. :-) >>> >>> I'll have a look. >>> >>>> I also found out that /usr/bin/eject in the guest won't work when >>>> -snapshot is used. Shouldn't qemu ignore this flag when using cdrom >>>> passthrough? >>> >>> "Won't work" means that it works like with a CD-ROM image? That would be >>> what I expect, as you end up having a qcow2 image with -snapshot. >> >> With a qcow2 stacked onto the host_cdrom. The block layer forwards the >> guest's eject only if the top driver has a bdrv_eject() method. Which >> qcow2 doesn't have. I guess bdrv_eject() fails with -ENOTSUP, and >> cmd_start_stop_unit() reports a funny error to the guest. > > bdrv_eject ignores -ENOTSUP and only changes the virtual tray in this > case. Just the very same thing as with normal ISO images.
Missed that. Thanks. >>> Not sure what's the best way of fixing this. Maybe just ignoring >>> -snapshot for read-only block devices? >> >> Why not, the combination is pointless. > > It could start making a difference in some obscure combinations. Imagine > a read-only image with a backing file, -snapshot and the 'commit' > monitor command. > > Sounds pretty insane, but I wouldn't bet that people aren't using it... People try all kinds of insane things. The question is whether we can change it anyway. >>> Or we could try and forward the >>> eject request to the backing file if the format driver doesn't handle it. >> >> For what it's worth, the "raw" driver forwards manually. > > Yeah, but copying that into each driver probably isn't the right thing > to do. I didn't mean to hold up the "raw" driver as a shining example of how to do stuff. > For a generic implementation we could probably first try the > driver itself, if it returns -ENOTSUP we try the protocol, and if that > returns -ENOTSUP, too, we ask the backing file driver. I don't want to start another "format vs. protocol" semantic war, just point out that the general case is a tree of block drivers, and a general rule for passing eject up the tree better covers that. The current block.c provides for binary trees (bs->file and bs->backing_hd). When we discussed blockdev_add, we came up with much wilder trees. > Can't wait to see the requests that with a qcow2 formatted CD with a > backing file in another CD drive the other one (or both) should be > ejected. :-) Requests like that can make you wish for a way to eject users ;)