Re: [Qemu-devel] [PATCH] raw-posix.c: remove raw device access for cdrom

[Laurent Vivier <lvivier@redhat.com>]

On 02/07/2015 13:14, Stefan Hajnoczi wrote:
> On Wed, Jul 1, 2015 at 11:13 PM, Programmingkid
> <programmingkidx@gmail.com> wrote:
>> Fix real cdrom access in Mac OS X so it can be used in QEMU.
>> It simply removes the r from a device file's name. This
>> allows for a real cdrom to be accessible to the guest.
>> It has been successfully tested with a Windows XP guest
>> in qemu-system-i386. The qemu-system-ppc emulator doesn't
>> quit anymore, but there is another problem that prevents a
>> real cdrom from working.
>>
>> Signed-off-by: John Arbuckle <programmingkidx@gmail.com>
>>
>> ---
>>  block/raw-posix.c |   10 ++++++++++
>>  1 files changed, 10 insertions(+), 0 deletions(-)
>>
>> diff --git a/block/raw-posix.c b/block/raw-posix.c
>> index a967464..3585ed9 100644
>> --- a/block/raw-posix.c
>> +++ b/block/raw-posix.c
>> @@ -2096,6 +2096,16 @@ static int hdev_open(BlockDriverState *bs, QDict
>> *options, int flags,
>>          kernResult = FindEjectableCDMedia( &mediaIterator );
>>          kernResult = GetBSDPath( mediaIterator, bsdPath, sizeof( bsdPath )
>> );
>>
>>
>>
>> +        /*
>> +         * Remove the r from cdrom block device if needed.
>> +         * /dev/rdisk1 would become /dev/disk1.
>> +         * The r means raw access. It doesn't work well.
>> +         */
>> +        int sizeOfString = strlen("/dev/r");
>> +        if (strncmp("/dev/r", bsdPath, sizeOfString) == 0) {
>> +            sprintf(bsdPath, "/dev/%s", bsdPath + sizeOfString);
>> +        }
> 
> Why doesn't raw access "work well"?
> 
> This patch looks like a workaround but you haven't identified the root cause.
> 
> Perhaps because of request alignment requirements?  Typically CD-ROMs
> have 2 KB block size.  You could test this by changing the following
> in block_int.h:
> #define BLOCK_PROBE_BUF_SIZE        512
> 
> In that case you need to fix raw-posix.c alignment detection code for
> Mac OS X.  Look at raw_probe_alignment().

I agree with that, I think we need "s->needs_alignment = true".

So in raw_open_common() change "#ifdef __FreeBSD__" by "#ifdef
CONFIG_BSD" in:

#ifdef __FreeBSD__
    if (S_ISCHR(st.st_mode)) {
        /*
         * The file is a char device (disk), which on FreeBSD isn't behind
         * a pager, so force all requests to be aligned. This is needed
         * so QEMU makes sure all IO operations on the device are aligned
         * to sector size, or else FreeBSD will reject them with EINVAL.
         */
        s->needs_alignment = true;
    }
#endif

Laurent