Re: [Qemu-devel] [Qemu-block] [PATCH] block.c: fix real cdrom detection

[Stefan Hajnoczi]

On Wed, Jul 1, 2015 at 4:35 PM, Programmingkid
<address@hidden> wrote:
> On Jun 26, 2015, at 4:01 PM, Stefan Hajnoczi wrote:
>> On Fri, Jun 26, 2015 at 4:50 PM, Programmingkid
>> <address@hidden> wrote:
>>> On Jun 26, 2015, at 5:34 AM, Stefan Hajnoczi wrote:
>>>> On Thu, Jun 25, 2015 at 11:11:24AM -0400, Programmingkid wrote:
>>>>> On Jun 25, 2015, at 9:31 AM, Stefan Hajnoczi wrote:
>>>>>> On Tue, Jun 23, 2015 at 02:26:51PM -0400, Programmingkid wrote:
>>>>>>> On Jun 23, 2015, at 2:06 PM, John Snow wrote:
>>>>>>>> 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.
>>>>>>
>>>>>> Why does it quit?
>>>>>
>>>>> Because of a bug. This is what it prints: "Could not read image for 
>>>>> determining its format: Invalid argument".
>>>>
>>>> The bdrv_pread() failure is what need you need to investigate.  In the
>>>> other sub-thread there have been hints about adding CD-ROM passthrough
>>>> support on Mac OS X by filling in the missing parts in
>>>> block/raw-posix.c.  That should help you get to the bottom of the
>>>> problem.
>>>>
>>>>> This message seems to indicate that QEMU thinks the real cdrom drive is 
>>>>> an image file.
>>>>
>>>> If the -drive format= option is not given, QEMU will try to detect the
>>>> image format.  That's the purpose of the find_image_format() function.
>>>>
>>>> QEMU does not make a distinction between image files and block devices
>>>> because there are valid use cases where a block device uses an image
>>>> format.  For example, a disk or partition can contain qcow2 data (there
>>>> is no partition table or file system, just qcow2).
>>>
>>> So what you are saying is if the user enters "-cdrom /dev/cdrom", QEMU is 
>>> suppose to call find_image_format. If everything goes right, the first if 
>>> statement should be skipped. Then the bdrv_pread() function should succeed 
>>> and so should the bdrv_probe_all() function. Is that how it is suppose to 
>>> work?
>>
>> Exactly.  bdrv_pread() will grab some data from the start of the
>> CD-ROM.  No qcow2, vmdk, etc header will be found (there is probably
>> an ISO file system there), so QEMU will default to the raw format and
>> everything will work as expected.  This is also how CD-ROM passthrough
>> works on Linux and FreeBSD.
>
>
> I need some help. In the function bdrv_co_io_em(), the co argument's ret 
> field is changed to -22. Would you know where exactly it is set? That 
> question again is where is the co.ret variable set? I have tried stepping 
> thru the function, but the change always escapes me.

Please keep address@hidden CCed so others can participate in
the discussion.

The return value comes from:

        acb = bs->drv->bdrv_aio_readv(bs, sector_num, iov, nb_sectors,
                                      bdrv_co_io_em_complete, &co);

When the read request completes it invokes the callback.  In this case
it's bdrv_co_io_em_complete().  That function fills in co.ret.

You'll probably find that ->bdrv_aio_readv() points to
block/raw-posix.c:raw_aio_readv().  This is the block driver for raw
image files on POSIX systems (Mac OS, Linux, etc).

The function doing the actual preadv(2) system call is
handle_aiocb_rw() and its children.

Hope this helps.

Stefan