| Message ID | AB4CFA84-0BE1-41D1-A825-E879FD3EF2CB@gmail.com |
|---|---|
| State | New |
| Headers | show |
On Jul 1, 2015, at 6:13 PM, Programmingkid 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. Just wanted to note that qemu-system-ppc does work. I was able to read a file from a real cd from the Mac OS 10.2 guest. It was OpenBIOS that has the problem.
On 02/07/2015 00:13, Programmingkid 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 > <mailto: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); > + } I think you can just remove the strcat in here: if ( bsdPathAsCFString ) { size_t devPathLength; strcpy( bsdPath, _PATH_DEV ); strcat( bsdPath, "r" ); devPathLength = strlen( bsdPath ); if ( CFStringGetCString( bsdPathAsCFString, bsdPath + devPathLength, maxPathSize - devPathLength, kCFStringEncodingASCII ) ) { kernResult = KERN_SUCCESS; } CFRelease( bsdPathAsCFString ); So it's still a one-line change. What are the other consequences of removing the "r"? Paolo > if ( bsdPath[ 0 ] != '\0' ) { > strcat(bsdPath,"s0"); > /* some CDs don't have a partition 0 */ > -- > 1.7.5.4 >
On 02/07/2015 09:18, Paolo Bonzini wrote: > > > On 02/07/2015 00:13, Programmingkid 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 >> <mailto: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); >> + } > > I think you can just remove the strcat in here: > > if ( bsdPathAsCFString ) { > size_t devPathLength; > strcpy( bsdPath, _PATH_DEV ); > strcat( bsdPath, "r" ); > devPathLength = strlen( bsdPath ); > if ( CFStringGetCString( bsdPathAsCFString, bsdPath + > devPathLength, maxPathSize - devPathLength, kCFStringEncodingASCII ) ) { > kernResult = KERN_SUCCESS; > } > CFRelease( bsdPathAsCFString ); > > So it's still a one-line change. What are the other consequences of > removing the "r"? This code seems to be a cut'n'paste of an Apple example (bad...): https://developer.apple.com/library/mac/samplecode/CDROMSample/Listings/CDROMSample_CDROMSample_c.html Without this interesting comment: Add "r" before the BSD node name from the I/O Registry to specify the raw disknode. The raw disk nodes receive I/O requests directly and do not go through the buffer cache. Laurent
On 02/07/2015 09:39, Laurent Vivier wrote: > This code seems to be a cut'n'paste of an Apple example (bad...): > > https://developer.apple.com/library/mac/samplecode/CDROMSample/Listings/CDROMSample_CDROMSample_c.html > > Without this interesting comment: > > Add "r" before the BSD node name from the I/O Registry to specify > the raw disknode. The raw disk nodes receive I/O requests directly > and do not go through the buffer cache. Ok, so that's not too bad. Paolo
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(). Stefan
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
On 02/07/2015 14:24, Laurent Vivier wrote: > > #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 So on FreeBSD and Apple /dev/r* is the equivalent of BDRV_O_NO_CACHE? Paolo
On 02/07/2015 15:47, Paolo Bonzini wrote: > > > On 02/07/2015 14:24, Laurent Vivier wrote: >> >> #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 > > So on FreeBSD and Apple /dev/r* is the equivalent of BDRV_O_NO_CACHE? This is what I understand (MacOS is a derivative from FreeBSD) https://developer.apple.com/library/mac/documentation/Darwin/Reference/ManPages/man1/hdiutil.1.html DEVICE SPECIAL FILES Since any /dev entry can be treated as a raw disk image, it is worth noting which devices can be accessed when and how. /dev/rdisk nodes are character-special devices, but are "raw" in the BSD sense and force block-aligned I/O. They are closer to the physical disk than the buffer cache. /dev/disk nodes, on the other hand, are buffered block-special devices and are used primarily by the kernel's filesystem code. Laurent
On 02/07/2015 15:58, Laurent Vivier wrote: > Since any /dev entry can be treated as a raw disk image, it is worth > noting which devices can be accessed when and how. /dev/rdisk nodes are > character-special devices, but are "raw" in the BSD sense and force > block-aligned I/O. They are closer to the physical disk than the buffer > cache. /dev/disk nodes, on the other hand, are buffered block-special > devices and are used primarily by the kernel's filesystem code. So the right thing to do would not be just to set need_alignment, but to probe it like we do on Linux for BDRV_O_NO_CACHE. I'm okay with doing the simple thing, but it needs a comment for non-BSDers. Paolo
On 02/07/2015 16:03, Paolo Bonzini wrote: > > > On 02/07/2015 15:58, Laurent Vivier wrote: >> Since any /dev entry can be treated as a raw disk image, it is worth >> noting which devices can be accessed when and how. /dev/rdisk nodes are >> character-special devices, but are "raw" in the BSD sense and force >> block-aligned I/O. They are closer to the physical disk than the buffer >> cache. /dev/disk nodes, on the other hand, are buffered block-special >> devices and are used primarily by the kernel's filesystem code. > > So the right thing to do would not be just to set need_alignment, but to > probe it like we do on Linux for BDRV_O_NO_CACHE. > > I'm okay with doing the simple thing, but it needs a comment for non-BSDers. So, what we have to do, in our case, for MacOS X cdrom, is something like: ... GetBSDPath ... ... if (flags & BDRV_O_NOCACHE) { strcat(bsdPath, "r"); } ... ? Laurent
On 02/07/2015 16:18, Laurent Vivier wrote: >> > I'm okay with doing the simple thing, but it needs a comment for non-BSDers. > So, what we have to do, in our case, for MacOS X cdrom, is something like: > > ... GetBSDPath ... > ... > if (flags & BDRV_O_NOCACHE) { > strcat(bsdPath, "r"); > } > ... > > ? Well, what to do with Mac OS X CD-ROM is another story... Raw access "seems not do work well" according to John, so we may have a comment there explaining why we're not adding the "r". A FIXME comment saying "we should probe for alignment here" would be placed where you check S_ISCHR and set need_alignment to true. Paolo
On 02/07/2015 16:20, Paolo Bonzini wrote: > > > On 02/07/2015 16:18, Laurent Vivier wrote: >>>> I'm okay with doing the simple thing, but it needs a comment for non-BSDers. >> So, what we have to do, in our case, for MacOS X cdrom, is something like: >> >> ... GetBSDPath ... >> ... >> if (flags & BDRV_O_NOCACHE) { >> strcat(bsdPath, "r"); >> } >> ... >> >> ? > > Well, what to do with Mac OS X CD-ROM is another story... Raw access > "seems not do work well" according to John, so we may have a comment > there explaining why we're not adding the "r". I think it doesn't work well because they need to be aligned, and NOCACHE implies that (with BDRV_O_NOCACHE code will be self explicit :) ) raw_open_common() if ((bs->open_flags & BDRV_O_NOCACHE) != 0) { s->needs_alignment = true; } and needs_alignment allows to probe alignment (raw_probe_alignment()) > A FIXME comment saying "we should probe for alignment here" would be > placed where you check S_ISCHR and set need_alignment to true. It is another case, in the previous case (MacOS cdrom), user provides "-cdrom /dev/cdrom" and QEMU extracts /dev/rdiskX (or now /dev/diskX) from the system DB. In the FreeBSD case, user provides /dev/diskX or /dev/rdiskX, and QEMU must know if it needs alignment or not. I don't think we need more comment here. Laurent
On Jul 2, 2015, at 10:33 AM, Laurent Vivier wrote: > > > On 02/07/2015 16:20, Paolo Bonzini wrote: >> >> >> On 02/07/2015 16:18, Laurent Vivier wrote: >>>>> I'm okay with doing the simple thing, but it needs a comment for non-BSDers. >>> So, what we have to do, in our case, for MacOS X cdrom, is something like: >>> >>> ... GetBSDPath ... >>> ... >>> if (flags & BDRV_O_NOCACHE) { >>> strcat(bsdPath, "r"); >>> } >>> ... >>> >>> ? >> >> Well, what to do with Mac OS X CD-ROM is another story... Raw access >> "seems not do work well" according to John, so we may have a comment >> there explaining why we're not adding the "r". > > I think it doesn't work well because they need to be aligned, and > NOCACHE implies that (with BDRV_O_NOCACHE code will be self explicit :) ) > > raw_open_common() > > if ((bs->open_flags & BDRV_O_NOCACHE) != 0) { > s->needs_alignment = true; > } > > and needs_alignment allows to probe alignment (raw_probe_alignment()) Thank you very much for this. Raw cdrom access now works with this code. I just had to modify it so it just sets s->needs_alignment to true without checking the if condition. The if condition didn't work.
On Mon, Jul 6, 2015 at 4:58 PM, Programmingkid
<programmingkidx@gmail.com> wrote:
> Quick question, In order to use a real cdrom in buffered mode (/dev/disk1s0), QEMU would have to unmount the cdrom from the desktop. Is unmounting the cdrom in the hdev_open() function ok? . I am making a version 3 of the cdrom patch, so please disregard the last patch.
Please keep qemu-devel@nongnu.org CCed so discussion stays on the
mailing list and others can participate.
Does the user need to manually mount the CD-ROM again after QEMU has
terminated? If so, then maybe the user should manually unmount before
running QEMU.
Stefan
Am 02.07.2015 um 16:18 hat Laurent Vivier geschrieben: > > > On 02/07/2015 16:03, Paolo Bonzini wrote: > > > > > > On 02/07/2015 15:58, Laurent Vivier wrote: > >> Since any /dev entry can be treated as a raw disk image, it is worth > >> noting which devices can be accessed when and how. /dev/rdisk nodes are > >> character-special devices, but are "raw" in the BSD sense and force > >> block-aligned I/O. They are closer to the physical disk than the buffer > >> cache. /dev/disk nodes, on the other hand, are buffered block-special > >> devices and are used primarily by the kernel's filesystem code. > > > > So the right thing to do would not be just to set need_alignment, but to > > probe it like we do on Linux for BDRV_O_NO_CACHE. > > > > I'm okay with doing the simple thing, but it needs a comment for non-BSDers. > > So, what we have to do, in our case, for MacOS X cdrom, is something like: > > ... GetBSDPath ... > ... > if (flags & BDRV_O_NOCACHE) { > strcat(bsdPath, "r"); > } > ... I would avoid such magic. What we could do is rejecting /dev/rdisk nodes without BDRV_O_NOCACHE. Kevin
On 08/07/2015 12:31, Kevin Wolf wrote: > Am 02.07.2015 um 16:18 hat Laurent Vivier geschrieben: >> >> >> On 02/07/2015 16:03, Paolo Bonzini wrote: >>> >>> >>> On 02/07/2015 15:58, Laurent Vivier wrote: >>>> Since any /dev entry can be treated as a raw disk image, it is worth >>>> noting which devices can be accessed when and how. /dev/rdisk nodes are >>>> character-special devices, but are "raw" in the BSD sense and force >>>> block-aligned I/O. They are closer to the physical disk than the buffer >>>> cache. /dev/disk nodes, on the other hand, are buffered block-special >>>> devices and are used primarily by the kernel's filesystem code. >>> >>> So the right thing to do would not be just to set need_alignment, but to >>> probe it like we do on Linux for BDRV_O_NO_CACHE. >>> >>> I'm okay with doing the simple thing, but it needs a comment for non-BSDers. >> >> So, what we have to do, in our case, for MacOS X cdrom, is something like: >> >> ... GetBSDPath ... >> ... >> if (flags & BDRV_O_NOCACHE) { >> strcat(bsdPath, "r"); >> } >> ... > > I would avoid such magic. What we could do is rejecting /dev/rdisk nodes > without BDRV_O_NOCACHE. It's not how it works... Look in hdev_open(). If user provides /dev/cdrom on the command line, in the case of MacOS X, QEMU searches for a cdrom drive in the system and set filename to /dev/rdiskX according to the result. Perhaps this part should be removed. But if we just want to correct the bug, we must not set filename to /dev/rdiskX if NOCACHE is not set but to /dev/diskX It's the aim of this change. Laurent
Am 08.07.2015 um 12:47 hat Laurent Vivier geschrieben: > > > On 08/07/2015 12:31, Kevin Wolf wrote: > > Am 02.07.2015 um 16:18 hat Laurent Vivier geschrieben: > >> > >> > >> On 02/07/2015 16:03, Paolo Bonzini wrote: > >>> > >>> > >>> On 02/07/2015 15:58, Laurent Vivier wrote: > >>>> Since any /dev entry can be treated as a raw disk image, it is worth > >>>> noting which devices can be accessed when and how. /dev/rdisk nodes are > >>>> character-special devices, but are "raw" in the BSD sense and force > >>>> block-aligned I/O. They are closer to the physical disk than the buffer > >>>> cache. /dev/disk nodes, on the other hand, are buffered block-special > >>>> devices and are used primarily by the kernel's filesystem code. > >>> > >>> So the right thing to do would not be just to set need_alignment, but to > >>> probe it like we do on Linux for BDRV_O_NO_CACHE. > >>> > >>> I'm okay with doing the simple thing, but it needs a comment for non-BSDers. > >> > >> So, what we have to do, in our case, for MacOS X cdrom, is something like: > >> > >> ... GetBSDPath ... > >> ... > >> if (flags & BDRV_O_NOCACHE) { > >> strcat(bsdPath, "r"); > >> } > >> ... > > > > I would avoid such magic. What we could do is rejecting /dev/rdisk nodes > > without BDRV_O_NOCACHE. > > It's not how it works... > > Look in hdev_open(). > > If user provides /dev/cdrom on the command line, in the case of MacOS X, > QEMU searches for a cdrom drive in the system and set filename to > /dev/rdiskX according to the result. Oh, we're already playing such games... I guess you're right then. It even seems to be not only for '/dev/cdrom', but for everything starting with this string. Does anyone know what's the reason for that? Also, I guess before doing strcat() on bsdPath, we should check the buffer length... > Perhaps this part should be removed. > > But if we just want to correct the bug, we must not set filename to > /dev/rdiskX if NOCACHE is not set but to /dev/diskX > > It's the aim of this change. Yes, that looks right. Kevin
On Jul 8, 2015, at 7:01 AM, Kevin Wolf wrote: > Am 08.07.2015 um 12:47 hat Laurent Vivier geschrieben: >> >> >> On 08/07/2015 12:31, Kevin Wolf wrote: >>> Am 02.07.2015 um 16:18 hat Laurent Vivier geschrieben: >>>> >>>> >>>> On 02/07/2015 16:03, Paolo Bonzini wrote: >>>>> >>>>> >>>>> On 02/07/2015 15:58, Laurent Vivier wrote: >>>>>> Since any /dev entry can be treated as a raw disk image, it is worth >>>>>> noting which devices can be accessed when and how. /dev/rdisk nodes are >>>>>> character-special devices, but are "raw" in the BSD sense and force >>>>>> block-aligned I/O. They are closer to the physical disk than the buffer >>>>>> cache. /dev/disk nodes, on the other hand, are buffered block-special >>>>>> devices and are used primarily by the kernel's filesystem code. >>>>> >>>>> So the right thing to do would not be just to set need_alignment, but to >>>>> probe it like we do on Linux for BDRV_O_NO_CACHE. >>>>> >>>>> I'm okay with doing the simple thing, but it needs a comment for non-BSDers. >>>> >>>> So, what we have to do, in our case, for MacOS X cdrom, is something like: >>>> >>>> ... GetBSDPath ... >>>> ... >>>> if (flags & BDRV_O_NOCACHE) { >>>> strcat(bsdPath, "r"); >>>> } >>>> ... >>> >>> I would avoid such magic. What we could do is rejecting /dev/rdisk nodes >>> without BDRV_O_NOCACHE. >> >> It's not how it works... >> >> Look in hdev_open(). >> >> If user provides /dev/cdrom on the command line, in the case of MacOS X, >> QEMU searches for a cdrom drive in the system and set filename to >> /dev/rdiskX according to the result. > > Oh, we're already playing such games... I guess you're right then. > > It even seems to be not only for '/dev/cdrom', but for everything > starting with this string. Does anyone know what's the reason for that? > > Also, I guess before doing strcat() on bsdPath, we should check the > buffer length... By buffer, do you mean the bsdPath variable?
On 08/07/2015 13:01, Kevin Wolf wrote: > Am 08.07.2015 um 12:47 hat Laurent Vivier geschrieben: >> >> >> On 08/07/2015 12:31, Kevin Wolf wrote: >>> Am 02.07.2015 um 16:18 hat Laurent Vivier geschrieben: >>>> >>>> >>>> On 02/07/2015 16:03, Paolo Bonzini wrote: >>>>> >>>>> >>>>> On 02/07/2015 15:58, Laurent Vivier wrote: >>>>>> Since any /dev entry can be treated as a raw disk image, it is worth >>>>>> noting which devices can be accessed when and how. /dev/rdisk nodes are >>>>>> character-special devices, but are "raw" in the BSD sense and force >>>>>> block-aligned I/O. They are closer to the physical disk than the buffer >>>>>> cache. /dev/disk nodes, on the other hand, are buffered block-special >>>>>> devices and are used primarily by the kernel's filesystem code. >>>>> >>>>> So the right thing to do would not be just to set need_alignment, but to >>>>> probe it like we do on Linux for BDRV_O_NO_CACHE. >>>>> >>>>> I'm okay with doing the simple thing, but it needs a comment for non-BSDers. >>>> >>>> So, what we have to do, in our case, for MacOS X cdrom, is something like: >>>> >>>> ... GetBSDPath ... >>>> ... >>>> if (flags & BDRV_O_NOCACHE) { >>>> strcat(bsdPath, "r"); >>>> } >>>> ... >>> >>> I would avoid such magic. What we could do is rejecting /dev/rdisk nodes >>> without BDRV_O_NOCACHE. >> >> It's not how it works... >> >> Look in hdev_open(). >> >> If user provides /dev/cdrom on the command line, in the case of MacOS X, >> QEMU searches for a cdrom drive in the system and set filename to >> /dev/rdiskX according to the result. > > Oh, we're already playing such games... I guess you're right then. > > It even seems to be not only for '/dev/cdrom', but for everything > starting with this string. Does anyone know what's the reason for that? At least 10 years old reason: commit 3b0d4f61c917c4612b561d75b33a11f4da00738b Author: bellard <bellard@c046a42c-6fe2-441c-8c8c-71466251a162> Date: Sun Oct 30 18:30:10 2005 +0000 OS X: support for the built in CD-ROM drive (Mike Kronenberg) > > Also, I guess before doing strcat() on bsdPath, we should check the > buffer length... > >> Perhaps this part should be removed. >> >> But if we just want to correct the bug, we must not set filename to >> /dev/rdiskX if NOCACHE is not set but to /dev/diskX >> >> It's the aim of this change. > > Yes, that looks right. > > Kevin >
Am 08.07.2015 um 14:56 hat Programmingkid geschrieben: > > On Jul 8, 2015, at 7:01 AM, Kevin Wolf wrote: > > > Am 08.07.2015 um 12:47 hat Laurent Vivier geschrieben: > >> > >> > >> On 08/07/2015 12:31, Kevin Wolf wrote: > >>> Am 02.07.2015 um 16:18 hat Laurent Vivier geschrieben: > >>>> > >>>> > >>>> On 02/07/2015 16:03, Paolo Bonzini wrote: > >>>>> > >>>>> > >>>>> On 02/07/2015 15:58, Laurent Vivier wrote: > >>>>>> Since any /dev entry can be treated as a raw disk image, it is worth > >>>>>> noting which devices can be accessed when and how. /dev/rdisk nodes are > >>>>>> character-special devices, but are "raw" in the BSD sense and force > >>>>>> block-aligned I/O. They are closer to the physical disk than the buffer > >>>>>> cache. /dev/disk nodes, on the other hand, are buffered block-special > >>>>>> devices and are used primarily by the kernel's filesystem code. > >>>>> > >>>>> So the right thing to do would not be just to set need_alignment, but to > >>>>> probe it like we do on Linux for BDRV_O_NO_CACHE. > >>>>> > >>>>> I'm okay with doing the simple thing, but it needs a comment for non-BSDers. > >>>> > >>>> So, what we have to do, in our case, for MacOS X cdrom, is something like: > >>>> > >>>> ... GetBSDPath ... > >>>> ... > >>>> if (flags & BDRV_O_NOCACHE) { > >>>> strcat(bsdPath, "r"); > >>>> } > >>>> ... > >>> > >>> I would avoid such magic. What we could do is rejecting /dev/rdisk nodes > >>> without BDRV_O_NOCACHE. > >> > >> It's not how it works... > >> > >> Look in hdev_open(). > >> > >> If user provides /dev/cdrom on the command line, in the case of MacOS X, > >> QEMU searches for a cdrom drive in the system and set filename to > >> /dev/rdiskX according to the result. > > > > Oh, we're already playing such games... I guess you're right then. > > > > It even seems to be not only for '/dev/cdrom', but for everything > > starting with this string. Does anyone know what's the reason for that? > > > > Also, I guess before doing strcat() on bsdPath, we should check the > > buffer length... > > By buffer, do you mean the bsdPath variable? Yes. In theory, bsdPath could be completely filled with the path returned by GetBSDPath() because we pass sizeof(bsdPath) as maxPathSize. Appending "s0" would then overflow the buffer. I'll admit that this is rather unlikely to happen, but being careful when dealing with strings can never hurt. Kevin
On Jul 8, 2015, at 9:11 AM, Kevin Wolf wrote: > Am 08.07.2015 um 14:56 hat Programmingkid geschrieben: >> >> On Jul 8, 2015, at 7:01 AM, Kevin Wolf wrote: >> >>> Am 08.07.2015 um 12:47 hat Laurent Vivier geschrieben: >>>> >>>> >>>> On 08/07/2015 12:31, Kevin Wolf wrote: >>>>> Am 02.07.2015 um 16:18 hat Laurent Vivier geschrieben: >>>>>> >>>>>> >>>>>> On 02/07/2015 16:03, Paolo Bonzini wrote: >>>>>>> >>>>>>> >>>>>>> On 02/07/2015 15:58, Laurent Vivier wrote: >>>>>>>> Since any /dev entry can be treated as a raw disk image, it is worth >>>>>>>> noting which devices can be accessed when and how. /dev/rdisk nodes are >>>>>>>> character-special devices, but are "raw" in the BSD sense and force >>>>>>>> block-aligned I/O. They are closer to the physical disk than the buffer >>>>>>>> cache. /dev/disk nodes, on the other hand, are buffered block-special >>>>>>>> devices and are used primarily by the kernel's filesystem code. >>>>>>> >>>>>>> So the right thing to do would not be just to set need_alignment, but to >>>>>>> probe it like we do on Linux for BDRV_O_NO_CACHE. >>>>>>> >>>>>>> I'm okay with doing the simple thing, but it needs a comment for non-BSDers. >>>>>> >>>>>> So, what we have to do, in our case, for MacOS X cdrom, is something like: >>>>>> >>>>>> ... GetBSDPath ... >>>>>> ... >>>>>> if (flags & BDRV_O_NOCACHE) { >>>>>> strcat(bsdPath, "r"); >>>>>> } >>>>>> ... >>>>> >>>>> I would avoid such magic. What we could do is rejecting /dev/rdisk nodes >>>>> without BDRV_O_NOCACHE. >>>> >>>> It's not how it works... >>>> >>>> Look in hdev_open(). >>>> >>>> If user provides /dev/cdrom on the command line, in the case of MacOS X, >>>> QEMU searches for a cdrom drive in the system and set filename to >>>> /dev/rdiskX according to the result. >>> >>> Oh, we're already playing such games... I guess you're right then. >>> >>> It even seems to be not only for '/dev/cdrom', but for everything >>> starting with this string. Does anyone know what's the reason for that? >>> >>> Also, I guess before doing strcat() on bsdPath, we should check the >>> buffer length... >> >> By buffer, do you mean the bsdPath variable? > > Yes. In theory, bsdPath could be completely filled with the path > returned by GetBSDPath() because we pass sizeof(bsdPath) as maxPathSize. > Appending "s0" would then overflow the buffer. > > I'll admit that this is rather unlikely to happen, but being careful > when dealing with strings can never hurt. > > Kevin Ok, after my newest patch has been reviewed, I will add the size checking code.
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); + } + if ( bsdPath[ 0 ] != '\0' ) { strcat(bsdPath,"s0"); /* some CDs don't have a partition 0 */
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(-)