Re: [Qemu-devel] [PATCH 05/18] ide: Turn debug messages into assertions

[Kevin Wolf]

Am 01.06.2011 17:32, schrieb Markus Armbruster:
> Kevin Wolf <address@hidden> writes:
> 
>> Am 01.06.2011 15:44, schrieb Luiz Capitulino:
>>> On Thu, 26 May 2011 18:12:08 -0300
>>> Luiz Capitulino <address@hidden> wrote:
>>>
>>>> On Thu, 19 May 2011 14:33:19 +0200
>>>> Kevin Wolf <address@hidden> 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.

>> 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...

>>                                        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. 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.

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. :-)

Kevin