| Message ID | 3B3C7431-1003-4AAA-90AF-0E1A154DFBE2@gmail.com |
|---|---|
| State | New |
| Headers | show |
On 06/23/2015 01:56 PM, Programmingkid wrote: > Fix real cdrom detection so that a real cdrom can actually be used. > > signed-off-by: John Arbuckle <programmingkidx@gmail.com > <mailto:programmingkidx@gmail.com>> > > This patch has been tested on Mac OS X host and guest. > Command used: qemu-system-ppc -cdrom /dev/cdrom > > Note: I was able to view the files using OpenBIOS, but not on > Mac OS X. The size of the disc is reported correctly but some > error happens that prevents it from mounting in Mac OS X. This > is probably another bug with QEMU. > > --- > block.c | 3 ++- > 1 files changed, 2 insertions(+), 1 deletions(-) > > diff --git a/block.c b/block.c > index dd4f58d..75ccfad 100644 > --- a/block.c > +++ b/block.c > @@ -583,7 +583,8 @@ static int find_image_format(BlockDriverState *bs, > const char *filename, > int ret = 0; > > > > /* Return the raw BlockDriver * to scsi-generic devices or empty > drives */ > - if (bs->sg || !bdrv_is_inserted(bs) || bdrv_getlength(bs) == 0) { > + if (bs->sg || !bdrv_is_inserted(bs) || bdrv_getlength(bs) == 0 > + || strcmp("/dev/cdrom", filename) == 0) { > *pdrv = &bdrv_raw; > return ret; > } > -- > 1.7.5.4 > 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. --js
On Jun 23, 2015, at 2:06 PM, John Snow wrote: > > > On 06/23/2015 01:56 PM, Programmingkid wrote: >> Fix real cdrom detection so that a real cdrom can actually be used. >> >> signed-off-by: John Arbuckle <programmingkidx@gmail.com >> <mailto:programmingkidx@gmail.com>> >> >> This patch has been tested on Mac OS X host and guest. >> Command used: qemu-system-ppc -cdrom /dev/cdrom >> >> Note: I was able to view the files using OpenBIOS, but not on >> Mac OS X. The size of the disc is reported correctly but some >> error happens that prevents it from mounting in Mac OS X. This >> is probably another bug with QEMU. >> >> --- >> block.c | 3 ++- >> 1 files changed, 2 insertions(+), 1 deletions(-) >> >> diff --git a/block.c b/block.c >> index dd4f58d..75ccfad 100644 >> --- a/block.c >> +++ b/block.c >> @@ -583,7 +583,8 @@ static int find_image_format(BlockDriverState *bs, >> const char *filename, >> int ret = 0; >> >> >> >> /* Return the raw BlockDriver * to scsi-generic devices or empty >> drives */ >> - if (bs->sg || !bdrv_is_inserted(bs) || bdrv_getlength(bs) == 0) { >> + if (bs->sg || !bdrv_is_inserted(bs) || bdrv_getlength(bs) == 0 >> + || strcmp("/dev/cdrom", filename) == 0) { >> *pdrv = &bdrv_raw; >> return ret; >> } >> -- >> 1.7.5.4 >> > > 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. Before the patch, the bdrv_open_inherit() function would be incorrectly called. Its documentation says "Opens a disk image (raw, qcow2, vmdk, ...)" meaning only for disk image files (not for real media). This patch prevents the bdrv_open_inherit() function from ever being called. It sets the pdrv variable to the raw format. This made sense to me since a real cdrom is read in the raw format. A quick test does show the patch works. A real cdrom is successfully opened on qemu-system-i386 using a Windows XP guest.
Programmingkid <programmingkidx@gmail.com> writes: > On Jun 23, 2015, at 2:06 PM, John Snow wrote: > >> >> >> On 06/23/2015 01:56 PM, Programmingkid wrote: >>> Fix real cdrom detection so that a real cdrom can actually be used. >>> >>> signed-off-by: John Arbuckle <programmingkidx@gmail.com >>> <mailto:programmingkidx@gmail.com>> >>> >>> This patch has been tested on Mac OS X host and guest. >>> Command used: qemu-system-ppc -cdrom /dev/cdrom >>> >>> Note: I was able to view the files using OpenBIOS, but not on >>> Mac OS X. The size of the disc is reported correctly but some >>> error happens that prevents it from mounting in Mac OS X. This >>> is probably another bug with QEMU. >>> >>> --- >>> block.c | 3 ++- >>> 1 files changed, 2 insertions(+), 1 deletions(-) >>> >>> diff --git a/block.c b/block.c >>> index dd4f58d..75ccfad 100644 >>> --- a/block.c >>> +++ b/block.c >>> @@ -583,7 +583,8 @@ static int find_image_format(BlockDriverState *bs, >>> const char *filename, >>> int ret = 0; >>> >>> >>> >>> /* Return the raw BlockDriver * to scsi-generic devices or empty >>> drives */ >>> - if (bs->sg || !bdrv_is_inserted(bs) || bdrv_getlength(bs) == 0) { >>> + if (bs->sg || !bdrv_is_inserted(bs) || bdrv_getlength(bs) == 0 >>> + || strcmp("/dev/cdrom", filename) == 0) { >>> *pdrv = &bdrv_raw; >>> return ret; >>> } >>> -- >>> 1.7.5.4 >>> >> >> 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. > > Before the patch, the bdrv_open_inherit() function would be > incorrectly called. Its documentation says "Opens a disk image (raw, > qcow2, vmdk, ...)" meaning only for disk image files (not for real > media). This patch prevents the bdrv_open_inherit() function from ever > being called. It sets the pdrv variable to the raw format. This made > sense to me since a real cdrom is read in the raw format. > > A quick test does show the patch works. A real cdrom is successfully > opened on qemu-system-i386 using a Windows XP guest. What about /dev/sr0, /dev/sr1, and whatever other names could refer to a block device without a medium? Comparing filenames isn't a good way to test "is a block device without a medium".
On Tue, Jun 23, 2015 at 02:26:51PM -0400, Programmingkid wrote: > > On Jun 23, 2015, at 2:06 PM, John Snow wrote: > > > > > > > On 06/23/2015 01:56 PM, Programmingkid wrote: > >> Fix real cdrom detection so that a real cdrom can actually be used. > >> > >> signed-off-by: John Arbuckle <programmingkidx@gmail.com > >> <mailto:programmingkidx@gmail.com>> > >> > >> This patch has been tested on Mac OS X host and guest. > >> Command used: qemu-system-ppc -cdrom /dev/cdrom > >> > >> Note: I was able to view the files using OpenBIOS, but not on > >> Mac OS X. The size of the disc is reported correctly but some > >> error happens that prevents it from mounting in Mac OS X. This > >> is probably another bug with QEMU. > >> > >> --- > >> block.c | 3 ++- > >> 1 files changed, 2 insertions(+), 1 deletions(-) > >> > >> diff --git a/block.c b/block.c > >> index dd4f58d..75ccfad 100644 > >> --- a/block.c > >> +++ b/block.c > >> @@ -583,7 +583,8 @@ static int find_image_format(BlockDriverState *bs, > >> const char *filename, > >> int ret = 0; > >> > >> > >> > >> /* Return the raw BlockDriver * to scsi-generic devices or empty > >> drives */ > >> - if (bs->sg || !bdrv_is_inserted(bs) || bdrv_getlength(bs) == 0) { > >> + if (bs->sg || !bdrv_is_inserted(bs) || bdrv_getlength(bs) == 0 > >> + || strcmp("/dev/cdrom", filename) == 0) { > >> *pdrv = &bdrv_raw; > >> return ret; > >> } > >> -- > >> 1.7.5.4 > >> > > > > 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?
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: >> >>> >>> >>> On 06/23/2015 01:56 PM, Programmingkid wrote: >>>> Fix real cdrom detection so that a real cdrom can actually be used. >>>> >>>> signed-off-by: John Arbuckle <programmingkidx@gmail.com >>>> <mailto:programmingkidx@gmail.com>> >>>> >>>> This patch has been tested on Mac OS X host and guest. >>>> Command used: qemu-system-ppc -cdrom /dev/cdrom >>>> >>>> Note: I was able to view the files using OpenBIOS, but not on >>>> Mac OS X. The size of the disc is reported correctly but some >>>> error happens that prevents it from mounting in Mac OS X. This >>>> is probably another bug with QEMU. >>>> >>>> --- >>>> block.c | 3 ++- >>>> 1 files changed, 2 insertions(+), 1 deletions(-) >>>> >>>> diff --git a/block.c b/block.c >>>> index dd4f58d..75ccfad 100644 >>>> --- a/block.c >>>> +++ b/block.c >>>> @@ -583,7 +583,8 @@ static int find_image_format(BlockDriverState *bs, >>>> const char *filename, >>>> int ret = 0; >>>> >>>> >>>> >>>> /* Return the raw BlockDriver * to scsi-generic devices or empty >>>> drives */ >>>> - if (bs->sg || !bdrv_is_inserted(bs) || bdrv_getlength(bs) == 0) { >>>> + if (bs->sg || !bdrv_is_inserted(bs) || bdrv_getlength(bs) == 0 >>>> + || strcmp("/dev/cdrom", filename) == 0) { >>>> *pdrv = &bdrv_raw; >>>> return ret; >>>> } >>>> -- >>>> 1.7.5.4 >>>> >>> >>> 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". This message seems to indicate that QEMU thinks the real cdrom drive is an image file.
On Jun 25, 2015, at 2:53 AM, Markus Armbruster wrote: > Programmingkid <programmingkidx@gmail.com> writes: > >> On Jun 23, 2015, at 2:06 PM, John Snow wrote: >> >>> >>> >>> On 06/23/2015 01:56 PM, Programmingkid wrote: >>>> Fix real cdrom detection so that a real cdrom can actually be used. >>>> >>>> signed-off-by: John Arbuckle <programmingkidx@gmail.com >>>> <mailto:programmingkidx@gmail.com>> >>>> >>>> This patch has been tested on Mac OS X host and guest. >>>> Command used: qemu-system-ppc -cdrom /dev/cdrom >>>> >>>> Note: I was able to view the files using OpenBIOS, but not on >>>> Mac OS X. The size of the disc is reported correctly but some >>>> error happens that prevents it from mounting in Mac OS X. This >>>> is probably another bug with QEMU. >>>> >>>> --- >>>> block.c | 3 ++- >>>> 1 files changed, 2 insertions(+), 1 deletions(-) >>>> >>>> diff --git a/block.c b/block.c >>>> index dd4f58d..75ccfad 100644 >>>> --- a/block.c >>>> +++ b/block.c >>>> @@ -583,7 +583,8 @@ static int find_image_format(BlockDriverState *bs, >>>> const char *filename, >>>> int ret = 0; >>>> >>>> >>>> >>>> /* Return the raw BlockDriver * to scsi-generic devices or empty >>>> drives */ >>>> - if (bs->sg || !bdrv_is_inserted(bs) || bdrv_getlength(bs) == 0) { >>>> + if (bs->sg || !bdrv_is_inserted(bs) || bdrv_getlength(bs) == 0 >>>> + || strcmp("/dev/cdrom", filename) == 0) { >>>> *pdrv = &bdrv_raw; >>>> return ret; >>>> } >>>> -- >>>> 1.7.5.4 >>>> >>> >>> 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. >> >> Before the patch, the bdrv_open_inherit() function would be >> incorrectly called. Its documentation says "Opens a disk image (raw, >> qcow2, vmdk, ...)" meaning only for disk image files (not for real >> media). This patch prevents the bdrv_open_inherit() function from ever >> being called. It sets the pdrv variable to the raw format. This made >> sense to me since a real cdrom is read in the raw format. >> >> A quick test does show the patch works. A real cdrom is successfully >> opened on qemu-system-i386 using a Windows XP guest. > > What about /dev/sr0, /dev/sr1, and whatever other names could refer to a > block device without a medium? > > Comparing filenames isn't a good way to test "is a block device without > a medium". I think we are going to have to agree to disagree. I have never used the /dev/sr(0 | 1) devices and don't see how they would be effected by this patch. Are you trying to say the /dev/sr(0 | 1) devices *should* be handled by this patch?
On Jun 25, 2015, at 11:14 AM, Programmingkid wrote: > > On Jun 25, 2015, at 2:53 AM, Markus Armbruster wrote: > >> Programmingkid <programmingkidx@gmail.com> writes: >> >>> On Jun 23, 2015, at 2:06 PM, John Snow wrote: >>> >>>> >>>> >>>> On 06/23/2015 01:56 PM, Programmingkid wrote: >>>>> Fix real cdrom detection so that a real cdrom can actually be used. >>>>> >>>>> signed-off-by: John Arbuckle <programmingkidx@gmail.com >>>>> <mailto:programmingkidx@gmail.com>> >>>>> >>>>> This patch has been tested on Mac OS X host and guest. >>>>> Command used: qemu-system-ppc -cdrom /dev/cdrom >>>>> >>>>> Note: I was able to view the files using OpenBIOS, but not on >>>>> Mac OS X. The size of the disc is reported correctly but some >>>>> error happens that prevents it from mounting in Mac OS X. This >>>>> is probably another bug with QEMU. >>>>> >>>>> --- >>>>> block.c | 3 ++- >>>>> 1 files changed, 2 insertions(+), 1 deletions(-) >>>>> >>>>> diff --git a/block.c b/block.c >>>>> index dd4f58d..75ccfad 100644 >>>>> --- a/block.c >>>>> +++ b/block.c >>>>> @@ -583,7 +583,8 @@ static int find_image_format(BlockDriverState *bs, >>>>> const char *filename, >>>>> int ret = 0; >>>>> >>>>> >>>>> >>>>> /* Return the raw BlockDriver * to scsi-generic devices or empty >>>>> drives */ >>>>> - if (bs->sg || !bdrv_is_inserted(bs) || bdrv_getlength(bs) == 0) { >>>>> + if (bs->sg || !bdrv_is_inserted(bs) || bdrv_getlength(bs) == 0 >>>>> + || strcmp("/dev/cdrom", filename) == 0) { >>>>> *pdrv = &bdrv_raw; >>>>> return ret; >>>>> } >>>>> -- >>>>> 1.7.5.4 >>>>> >>>> >>>> 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. >>> >>> Before the patch, the bdrv_open_inherit() function would be >>> incorrectly called. Its documentation says "Opens a disk image (raw, >>> qcow2, vmdk, ...)" meaning only for disk image files (not for real >>> media). This patch prevents the bdrv_open_inherit() function from ever >>> being called. It sets the pdrv variable to the raw format. This made >>> sense to me since a real cdrom is read in the raw format. >>> >>> A quick test does show the patch works. A real cdrom is successfully >>> opened on qemu-system-i386 using a Windows XP guest. >> >> What about /dev/sr0, /dev/sr1, and whatever other names could refer to a >> block device without a medium? >> >> Comparing filenames isn't a good way to test "is a block device without >> a medium". > > I think we are going to have to agree to disagree. I have never used the /dev/sr(0 | 1) devices and don't see how they would be effected by this patch. Are you trying to say the /dev/sr(0 | 1) devices *should* be handled by this patch? Thinking about your question some more, I see what you mean. On Linux /dev/sr0 refers to the cdrom drive. Also on Linux, the /dev/cdrom link refers to the /dev/sr0 device file. So if you just use /dev/cdrom, you are good.
On Jun 25, 2015, at 11:32 AM, Programmingkid wrote: > > On Jun 25, 2015, at 11:14 AM, Programmingkid wrote: > >> >> On Jun 25, 2015, at 2:53 AM, Markus Armbruster wrote: >> >>> Programmingkid <programmingkidx@gmail.com> writes: >>> >>>> On Jun 23, 2015, at 2:06 PM, John Snow wrote: >>>> >>>>> >>>>> >>>>> On 06/23/2015 01:56 PM, Programmingkid wrote: >>>>>> Fix real cdrom detection so that a real cdrom can actually be used. >>>>>> >>>>>> signed-off-by: John Arbuckle <programmingkidx@gmail.com >>>>>> <mailto:programmingkidx@gmail.com>> >>>>>> >>>>>> This patch has been tested on Mac OS X host and guest. >>>>>> Command used: qemu-system-ppc -cdrom /dev/cdrom >>>>>> >>>>>> Note: I was able to view the files using OpenBIOS, but not on >>>>>> Mac OS X. The size of the disc is reported correctly but some >>>>>> error happens that prevents it from mounting in Mac OS X. This >>>>>> is probably another bug with QEMU. >>>>>> >>>>>> --- >>>>>> block.c | 3 ++- >>>>>> 1 files changed, 2 insertions(+), 1 deletions(-) >>>>>> >>>>>> diff --git a/block.c b/block.c >>>>>> index dd4f58d..75ccfad 100644 >>>>>> --- a/block.c >>>>>> +++ b/block.c >>>>>> @@ -583,7 +583,8 @@ static int find_image_format(BlockDriverState *bs, >>>>>> const char *filename, >>>>>> int ret = 0; >>>>>> >>>>>> >>>>>> >>>>>> /* Return the raw BlockDriver * to scsi-generic devices or empty >>>>>> drives */ >>>>>> - if (bs->sg || !bdrv_is_inserted(bs) || bdrv_getlength(bs) == 0) { >>>>>> + if (bs->sg || !bdrv_is_inserted(bs) || bdrv_getlength(bs) == 0 >>>>>> + || strcmp("/dev/cdrom", filename) == 0) { >>>>>> *pdrv = &bdrv_raw; >>>>>> return ret; >>>>>> } >>>>>> -- >>>>>> 1.7.5.4 >>>>>> >>>>> >>>>> 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. >>>> >>>> Before the patch, the bdrv_open_inherit() function would be >>>> incorrectly called. Its documentation says "Opens a disk image (raw, >>>> qcow2, vmdk, ...)" meaning only for disk image files (not for real >>>> media). This patch prevents the bdrv_open_inherit() function from ever >>>> being called. It sets the pdrv variable to the raw format. This made >>>> sense to me since a real cdrom is read in the raw format. >>>> >>>> A quick test does show the patch works. A real cdrom is successfully >>>> opened on qemu-system-i386 using a Windows XP guest. >>> >>> What about /dev/sr0, /dev/sr1, and whatever other names could refer to a >>> block device without a medium? >>> >>> Comparing filenames isn't a good way to test "is a block device without >>> a medium". >> >> I think we are going to have to agree to disagree. I have never used the /dev/sr(0 | 1) devices and don't see how they would be effected by this patch. Are you trying to say the /dev/sr(0 | 1) devices *should* be handled by this patch? > > Thinking about your question some more, I see what you mean. On Linux /dev/sr0 refers to the cdrom drive. Also on Linux, the /dev/cdrom link refers to the /dev/sr0 device file. So if you just use /dev/cdrom, you are good. Just had another idea. I could change the patch so that if it detects any device file being used, it handles it as raw. That way if you want to use /dev/sr0, you can. /* Return the raw BlockDriver * to scsi-generic devices or empty drives */ - if (bs->sg || !bdrv_is_inserted(bs) || bdrv_getlength(bs) == 0) { + if (bs->sg || !bdrv_is_inserted(bs) || bdrv_getlength(bs) == 0 + || strncmp("/dev/", filename, strlen("/dev/")) == 0) { *pdrv = &bdrv_raw; return ret; }
On 25/06/2015 17:32, Programmingkid wrote: >> I think we are going to have to agree to disagree. I have never >> used the /dev/sr(0 | 1) devices and don't see how they would be >> effected by this patch. Are you trying to say the /dev/sr(0 | 1) >> devices *should* be handled by this patch? > > Thinking about your question some more, I see what you mean. On Linux > /dev/sr0 refers to the cdrom drive. Also on Linux, the /dev/cdrom > link refers to the /dev/sr0 device file. So if you just use > /dev/cdrom, you are good. Well, that's not how things work. If you do things like that, you end up with a bunch of hacks, not with a decent piece of software. There is support for CD-ROM passthrough on Linux and FreeBSD in block/raw-posix.c. Perhaps the FreeBSD support can be extended to OS X as well. Paolo
On 25/06/2015 17:48, Paolo Bonzini wrote: > > On 25/06/2015 17:32, Programmingkid wrote: >>> I think we are going to have to agree to disagree. I have never >>> used the /dev/sr(0 | 1) devices and don't see how they would be >>> effected by this patch. Are you trying to say the /dev/sr(0 | 1) >>> devices *should* be handled by this patch? >> >> Thinking about your question some more, I see what you mean. On Linux >> /dev/sr0 refers to the cdrom drive. Also on Linux, the /dev/cdrom >> link refers to the /dev/sr0 device file. So if you just use >> /dev/cdrom, you are good. > > Well, that's not how things work. > > If you do things like that, you end up with a bunch of hacks, not with a > decent piece of software. > > There is support for CD-ROM passthrough on Linux and FreeBSD in > block/raw-posix.c. Perhaps the FreeBSD support can be extended to OS X > as well. > In fact, programmingkid, you should fix it in hdev_open() where there is already a #if __APPLE__ . Paolo, I agree with you but : hdev_open() #if defined(__linux__) { char resolved_path[ MAXPATHLEN ], *temp; temp = realpath(filename, resolved_path); if (temp && strstart(temp, "/dev/sg", NULL)) { bs->sg = 1; } #endif I'm wondering who had this strange idea... :) Laurent
On 25/06/2015 18:12, Laurent Vivier wrote: > > > On 25/06/2015 17:48, Paolo Bonzini wrote: >> >> On 25/06/2015 17:32, Programmingkid wrote: >>>> I think we are going to have to agree to disagree. I have never >>>> used the /dev/sr(0 | 1) devices and don't see how they would be >>>> effected by this patch. Are you trying to say the /dev/sr(0 | 1) >>>> devices *should* be handled by this patch? >>> >>> Thinking about your question some more, I see what you mean. On Linux >>> /dev/sr0 refers to the cdrom drive. Also on Linux, the /dev/cdrom >>> link refers to the /dev/sr0 device file. So if you just use >>> /dev/cdrom, you are good. >> >> Well, that's not how things work. >> >> If you do things like that, you end up with a bunch of hacks, not with a >> decent piece of software. >> >> There is support for CD-ROM passthrough on Linux and FreeBSD in >> block/raw-posix.c. Perhaps the FreeBSD support can be extended to OS X >> as well. >> > > In fact, programmingkid, you should fix it in hdev_open() where there is > already a #if __APPLE__ . > > Paolo, I agree with you but : > > hdev_open() > > #if defined(__linux__) > { > char resolved_path[ MAXPATHLEN ], *temp; > > temp = realpath(filename, resolved_path); > if (temp && strstart(temp, "/dev/sg", NULL)) { > bs->sg = 1; > } > #endif > > I'm wondering who had this strange idea... :) I was very scared to type "git blame" here. :) But the question is also where to put the checks. Putting it at a random place in block.c is not a good idea. But yes, this is also bad. It should use stat and check the major/minor numbers. Paolo
On 25/06/2015 18:16, Paolo Bonzini wrote: > > > On 25/06/2015 18:12, Laurent Vivier wrote: >> >> >> On 25/06/2015 17:48, Paolo Bonzini wrote: >>> >>> On 25/06/2015 17:32, Programmingkid wrote: >>>>> I think we are going to have to agree to disagree. I have never >>>>> used the /dev/sr(0 | 1) devices and don't see how they would be >>>>> effected by this patch. Are you trying to say the /dev/sr(0 | 1) >>>>> devices *should* be handled by this patch? >>>> >>>> Thinking about your question some more, I see what you mean. On Linux >>>> /dev/sr0 refers to the cdrom drive. Also on Linux, the /dev/cdrom >>>> link refers to the /dev/sr0 device file. So if you just use >>>> /dev/cdrom, you are good. >>> >>> Well, that's not how things work. >>> >>> If you do things like that, you end up with a bunch of hacks, not with a >>> decent piece of software. >>> >>> There is support for CD-ROM passthrough on Linux and FreeBSD in >>> block/raw-posix.c. Perhaps the FreeBSD support can be extended to OS X >>> as well. >>> >> >> In fact, programmingkid, you should fix it in hdev_open() where there is >> already a #if __APPLE__ . >> >> Paolo, I agree with you but : >> >> hdev_open() >> >> #if defined(__linux__) >> { >> char resolved_path[ MAXPATHLEN ], *temp; >> >> temp = realpath(filename, resolved_path); >> if (temp && strstart(temp, "/dev/sg", NULL)) { >> bs->sg = 1; >> } >> #endif >> >> I'm wondering who had this strange idea... :) > > I was very scared to type "git blame" here. :) But the question is also http://geek-and-poke.com/2013/11/24/simply-explained BTW, it is a legacy from 2006: 19cb373 better support of host drives coming from MacOS X (again!): 3b0d4f6 OS X: support for the built in CD-ROM drive (Mike Kronenberg) > where to put the checks. Putting it at a random place in block.c is not > a good idea. > > But yes, this is also bad. It should use stat and check the major/minor > numbers. Yes, we should check if major is SCSI_GENERIC_MAJOR (21) (on linux). We can also try to send an SG command like in cdrom_probe_device(). Something like in scsi_generic_realize(): rc = blk_ioctl(s->conf.blk, SG_GET_VERSION_NUM, &sg_version); if (rc < 0) { error_setg(errp, "cannot get SG_IO version number: %s. " "Is this a SCSI device?", strerror(-rc)); return; } Laurent
On Jun 25, 2015, at 12:12 PM, Laurent Vivier wrote: > > > On 25/06/2015 17:48, Paolo Bonzini wrote: >> >> On 25/06/2015 17:32, Programmingkid wrote: >>>> I think we are going to have to agree to disagree. I have never >>>> used the /dev/sr(0 | 1) devices and don't see how they would be >>>> effected by this patch. Are you trying to say the /dev/sr(0 | 1) >>>> devices *should* be handled by this patch? >>> >>> Thinking about your question some more, I see what you mean. On Linux >>> /dev/sr0 refers to the cdrom drive. Also on Linux, the /dev/cdrom >>> link refers to the /dev/sr0 device file. So if you just use >>> /dev/cdrom, you are good. >> >> Well, that's not how things work. >> >> If you do things like that, you end up with a bunch of hacks, not with a >> decent piece of software. >> >> There is support for CD-ROM passthrough on Linux and FreeBSD in >> block/raw-posix.c. Perhaps the FreeBSD support can be extended to OS X >> as well. >> > > In fact, programmingkid, you should fix it in hdev_open() where there is > already a #if __APPLE__ . Nice to hear from you again Laurent. The only way a solution in hdev_open() would work is if it could prevent find_image_format() from executing. Otherwise find_image_format() would just quit QEMU with an error.
On Jun 25, 2015, at 11:48 AM, Paolo Bonzini wrote: > > On 25/06/2015 17:32, Programmingkid wrote: >>> I think we are going to have to agree to disagree. I have never >>> used the /dev/sr(0 | 1) devices and don't see how they would be >>> effected by this patch. Are you trying to say the /dev/sr(0 | 1) >>> devices *should* be handled by this patch? >> >> Thinking about your question some more, I see what you mean. On Linux >> /dev/sr0 refers to the cdrom drive. Also on Linux, the /dev/cdrom >> link refers to the /dev/sr0 device file. So if you just use >> /dev/cdrom, you are good. > > Well, that's not how things work. > > If you do things like that, you end up with a bunch of hacks, not with a > decent piece of software. > > There is support for CD-ROM passthrough on Linux and FreeBSD in > block/raw-posix.c. Perhaps the FreeBSD support can be extended to OS X > as well. Could you be more specific? A name of a function would be great.
On 25/06/2015 19:56, Programmingkid wrote: >> In fact, programmingkid, you should fix it in hdev_open() where >> there is already a #if __APPLE__ . > > Nice to hear from you again Laurent. The only way a solution in > hdev_open() would work is if it could prevent find_image_format() > from executing. Otherwise find_image_format() would just quit QEMU > with an error. You have to implement an is_inserted function like this one that is already there for FreeBSD: static int cdrom_is_inserted(BlockDriverState *bs) { return raw_getlength(bs) > 0; } The point you're changing is _already_ testing bdrv_is_inserted. Look at the block starting like this: #if defined (__FreeBSD__) || defined(__FreeBSD_kernel__) static int cdrom_open(BlockDriverState *bs, QDict *options, int flags, Error **errp) and create a similar one that is #ifdef __APPLE__. Feel free to remove pieces that you don't know how to do in OS X / CoreFoundation. Paolo
On 25 June 2015 at 18:56, Programmingkid <programmingkidx@gmail.com> wrote: > Nice to hear from you again Laurent. The only way a solution in > hdev_open() would work is if it could prevent find_image_format() > from executing. Otherwise find_image_format() would just quit QEMU > with an error. The question you should be asking is "what is Linux doing for raw CDROM devices that is different, such that it works there but doesn't work on OSX?". It would also be helpful to know which is the case that doesn't work. Does QEMU fail in all cases, or only if the cdrom drive is empty, or only if there's a disk in the drive? My initial suspicion is that we need OSX support in raw-posix.c for handling the host CDROM specially -- note that Linux and FreeBSD register a bdrv_host_cdrom with an is_inserted function. thanks -- PMM
On Jun 25, 2015, at 12:16 PM, Paolo Bonzini wrote: > > > On 25/06/2015 18:12, Laurent Vivier wrote: >> >> >> On 25/06/2015 17:48, Paolo Bonzini wrote: >>> >>> On 25/06/2015 17:32, Programmingkid wrote: >>>>> I think we are going to have to agree to disagree. I have never >>>>> used the /dev/sr(0 | 1) devices and don't see how they would be >>>>> effected by this patch. Are you trying to say the /dev/sr(0 | 1) >>>>> devices *should* be handled by this patch? >>>> >>>> Thinking about your question some more, I see what you mean. On Linux >>>> /dev/sr0 refers to the cdrom drive. Also on Linux, the /dev/cdrom >>>> link refers to the /dev/sr0 device file. So if you just use >>>> /dev/cdrom, you are good. >>> >>> Well, that's not how things work. >>> >>> If you do things like that, you end up with a bunch of hacks, not with a >>> decent piece of software. >>> >>> There is support for CD-ROM passthrough on Linux and FreeBSD in >>> block/raw-posix.c. Perhaps the FreeBSD support can be extended to OS X >>> as well. >>> >> >> In fact, programmingkid, you should fix it in hdev_open() where there is >> already a #if __APPLE__ . >> >> Paolo, I agree with you but : >> >> hdev_open() >> >> #if defined(__linux__) >> { >> char resolved_path[ MAXPATHLEN ], *temp; >> >> temp = realpath(filename, resolved_path); >> if (temp && strstart(temp, "/dev/sg", NULL)) { >> bs->sg = 1; >> } >> #endif >> >> I'm wondering who had this strange idea... :) > > I was very scared to type "git blame" here. :) But the question is also > where to put the checks. Putting it at a random place in block.c is not > a good idea. I honestly think it is in the right place. The function find_image_format() is doing just that - trying to find the format. The image part of the function's name does bother me. But we could ignore it. Since we know it is a real cdrom drive, it would receive the format of raw. It seems as simple as that.
On 25/06/2015 20:07, Programmingkid wrote: > I honestly think it is in the right place. The function find_image_format() > is doing just that - trying to find the format. The image part of the function's name > does bother me. But we could ignore it. Since we know it is a real cdrom drive, > it would receive the format of raw. It seems as simple as that. But matching against the name in generic block.c code is just wrong. It _is_ as simple as that. Paolo
On 25/06/2015 19:19, Laurent Vivier wrote: > > > On 25/06/2015 18:16, Paolo Bonzini wrote: >> >> >> On 25/06/2015 18:12, Laurent Vivier wrote: >>> >>> >>> On 25/06/2015 17:48, Paolo Bonzini wrote: >>>> >>>> On 25/06/2015 17:32, Programmingkid wrote: >>>>>> I think we are going to have to agree to disagree. I have never >>>>>> used the /dev/sr(0 | 1) devices and don't see how they would be >>>>>> effected by this patch. Are you trying to say the /dev/sr(0 | 1) >>>>>> devices *should* be handled by this patch? >>>>> >>>>> Thinking about your question some more, I see what you mean. On Linux >>>>> /dev/sr0 refers to the cdrom drive. Also on Linux, the /dev/cdrom >>>>> link refers to the /dev/sr0 device file. So if you just use >>>>> /dev/cdrom, you are good. >>>> >>>> Well, that's not how things work. >>>> >>>> If you do things like that, you end up with a bunch of hacks, not with a >>>> decent piece of software. >>>> >>>> There is support for CD-ROM passthrough on Linux and FreeBSD in >>>> block/raw-posix.c. Perhaps the FreeBSD support can be extended to OS X >>>> as well. >>>> >>> >>> In fact, programmingkid, you should fix it in hdev_open() where there is >>> already a #if __APPLE__ . >>> >>> Paolo, I agree with you but : >>> >>> hdev_open() >>> >>> #if defined(__linux__) >>> { >>> char resolved_path[ MAXPATHLEN ], *temp; >>> >>> temp = realpath(filename, resolved_path); >>> if (temp && strstart(temp, "/dev/sg", NULL)) { >>> bs->sg = 1; >>> } >>> #endif >>> >>> I'm wondering who had this strange idea... :) >> >> I was very scared to type "git blame" here. :) But the question is also > > http://geek-and-poke.com/2013/11/24/simply-explained > > BTW, it is a legacy from 2006: > > 19cb373 better support of host drives > > coming from MacOS X (again!): > > 3b0d4f6 OS X: support for the built in CD-ROM drive (Mike Kronenberg) > >> where to put the checks. Putting it at a random place in block.c is not >> a good idea. >> >> But yes, this is also bad. It should use stat and check the major/minor >> numbers. > > Yes, we should check if major is SCSI_GENERIC_MAJOR (21) (on linux). > > We can also try to send an SG command like in cdrom_probe_device(). > Something like in scsi_generic_realize(): > > rc = blk_ioctl(s->conf.blk, SG_GET_VERSION_NUM, &sg_version); > if (rc < 0) { > error_setg(errp, "cannot get SG_IO version number: %s. " > "Is this a SCSI device?", > strerror(-rc)); > return; > } > BTW, this solution is already queued in Stefan's block tree: https://github.com/stefanha/qemu/commit/3307ed7b3fac5ba99eb3b84904b0b7cdc3592a61 Laurent
On Thu, Jun 25, 2015 at 07:19:14PM +0200, Laurent Vivier wrote: > > > On 25/06/2015 18:16, Paolo Bonzini wrote: > > > > > > On 25/06/2015 18:12, Laurent Vivier wrote: > >> > >> > >> On 25/06/2015 17:48, Paolo Bonzini wrote: > >>> > >>> On 25/06/2015 17:32, Programmingkid wrote: > >>>>> I think we are going to have to agree to disagree. I have never > >>>>> used the /dev/sr(0 | 1) devices and don't see how they would be > >>>>> effected by this patch. Are you trying to say the /dev/sr(0 | 1) > >>>>> devices *should* be handled by this patch? > >>>> > >>>> Thinking about your question some more, I see what you mean. On Linux > >>>> /dev/sr0 refers to the cdrom drive. Also on Linux, the /dev/cdrom > >>>> link refers to the /dev/sr0 device file. So if you just use > >>>> /dev/cdrom, you are good. > >>> > >>> Well, that's not how things work. > >>> > >>> If you do things like that, you end up with a bunch of hacks, not with a > >>> decent piece of software. > >>> > >>> There is support for CD-ROM passthrough on Linux and FreeBSD in > >>> block/raw-posix.c. Perhaps the FreeBSD support can be extended to OS X > >>> as well. > >>> > >> > >> In fact, programmingkid, you should fix it in hdev_open() where there is > >> already a #if __APPLE__ . > >> > >> Paolo, I agree with you but : > >> > >> hdev_open() > >> > >> #if defined(__linux__) > >> { > >> char resolved_path[ MAXPATHLEN ], *temp; > >> > >> temp = realpath(filename, resolved_path); > >> if (temp && strstart(temp, "/dev/sg", NULL)) { > >> bs->sg = 1; > >> } > >> #endif > >> > >> I'm wondering who had this strange idea... :) > > > > I was very scared to type "git blame" here. :) But the question is also > > http://geek-and-poke.com/2013/11/24/simply-explained > > BTW, it is a legacy from 2006: > > 19cb373 better support of host drives > > coming from MacOS X (again!): > > 3b0d4f6 OS X: support for the built in CD-ROM drive (Mike Kronenberg) > > > where to put the checks. Putting it at a random place in block.c is not > > a good idea. > > > > But yes, this is also bad. It should use stat and check the major/minor > > numbers. > > Yes, we should check if major is SCSI_GENERIC_MAJOR (21) (on linux). That would be too specific since there are other drivers that support SG ioctls, like block/bsg.c. > We can also try to send an SG command like in cdrom_probe_device(). > Something like in scsi_generic_realize(): > > rc = blk_ioctl(s->conf.blk, SG_GET_VERSION_NUM, &sg_version); > if (rc < 0) { > error_setg(errp, "cannot get SG_IO version number: %s. " > "Is this a SCSI device?", > strerror(-rc)); > return; > } That was recently done in: commit 3307ed7b3fac5ba99eb3b84904b0b7cdc3592a61 Author: Dimitris Aragiorgis <dimara@arrikto.com> Date: Tue Jun 23 13:45:00 2015 +0300 raw-posix: Introduce hdev_is_sg()
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).
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?
On Fri, Jun 26, 2015 at 4:50 PM, Programmingkid <programmingkidx@gmail.com> 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.
On Jun 25, 2015, at 2:01 PM, Peter Maydell wrote: > On 25 June 2015 at 18:56, Programmingkid <programmingkidx@gmail.com> wrote: >> Nice to hear from you again Laurent. The only way a solution in >> hdev_open() would work is if it could prevent find_image_format() >> from executing. Otherwise find_image_format() would just quit QEMU >> with an error. > > The question you should be asking is "what is Linux doing for > raw CDROM devices that is different, such that it works there > but doesn't work on OSX?". > > It would also be helpful to know which is the case that doesn't > work. Does QEMU fail in all cases, or only if the cdrom drive is > empty, or only if there's a disk in the drive? QEMU fails if the cdrom is specified "-cdrom /dev/cdrom", and there is no cd in the drive. QEMU also fails with a real cdrom in the drive. > > My initial suspicion is that we need OSX support in raw-posix.c > for handling the host CDROM specially -- note that Linux and > FreeBSD register a bdrv_host_cdrom with an is_inserted > function. The is_inserted function wouldn't make a difference. I did follow the execution of QEMU from find_image_format(). When bdrv_co_io_em() is called, it returns -22. This is where things appear to start to go wrong.
Hi, On 29/06/2015 01:43, Programmingkid wrote: > > On Jun 25, 2015, at 2:01 PM, Peter Maydell wrote: > >> On 25 June 2015 at 18:56, Programmingkid >> <programmingkidx@gmail.com> wrote: >>> Nice to hear from you again Laurent. The only way a solution in >>> hdev_open() would work is if it could prevent >>> find_image_format() from executing. Otherwise find_image_format() >>> would just quit QEMU with an error. >> >> The question you should be asking is "what is Linux doing for raw >> CDROM devices that is different, such that it works there but >> doesn't work on OSX?". >> >> It would also be helpful to know which is the case that doesn't >> work. Does QEMU fail in all cases, or only if the cdrom drive is >> empty, or only if there's a disk in the drive? > > QEMU fails if the cdrom is specified "-cdrom /dev/cdrom", and there > is no cd in the drive. > > QEMU also fails with a real cdrom in the drive. > >> >> My initial suspicion is that we need OSX support in raw-posix.c for >> handling the host CDROM specially -- note that Linux and FreeBSD >> register a bdrv_host_cdrom with an is_inserted function. > > The is_inserted function wouldn't make a difference. In fact, if your patch fixes the problem, the is_inserted with no cdrom should too: with your " strcmp("/dev/cdrom", filename) == 0 ", you force the selection of bdrv_raw (which is what to do). without your patch, if "bdrv_is_inserted()" was implemented and no cdrom in the drive " !bdrv_is_inserted(bs) " should also select bdrv_raw. It appears also that bdrv_host_cdrom is not registered in bdrv_file_init(). I think this is the missing part to have a host cdrom support on MacOS X. Laurent
On Jun 28, 2015, at 8:29 PM, Laurent Vivier wrote: > Hi, > > On 29/06/2015 01:43, Programmingkid wrote: >> >> On Jun 25, 2015, at 2:01 PM, Peter Maydell wrote: >> >>> On 25 June 2015 at 18:56, Programmingkid >>> <programmingkidx@gmail.com> wrote: >>>> Nice to hear from you again Laurent. The only way a solution in >>>> hdev_open() would work is if it could prevent >>>> find_image_format() from executing. Otherwise find_image_format() >>>> would just quit QEMU with an error. >>> >>> The question you should be asking is "what is Linux doing for raw >>> CDROM devices that is different, such that it works there but >>> doesn't work on OSX?". >>> >>> It would also be helpful to know which is the case that doesn't >>> work. Does QEMU fail in all cases, or only if the cdrom drive is >>> empty, or only if there's a disk in the drive? >> >> QEMU fails if the cdrom is specified "-cdrom /dev/cdrom", and there >> is no cd in the drive. >> >> QEMU also fails with a real cdrom in the drive. >> >>> >>> My initial suspicion is that we need OSX support in raw-posix.c for >>> handling the host CDROM specially -- note that Linux and FreeBSD >>> register a bdrv_host_cdrom with an is_inserted function. >> >> The is_inserted function wouldn't make a difference. > > In fact, if your patch fixes the problem, the is_inserted with no > cdrom should too: > > with your " strcmp("/dev/cdrom", filename) == 0 ", you force the > selection of bdrv_raw (which is what to do). > > without your patch, if "bdrv_is_inserted()" was implemented and no cdrom > in the drive " !bdrv_is_inserted(bs) " should also select bdrv_raw. The thing is a cdrom would be in the drive, and !bdrv_is_inserted() would return false. > > It appears also that bdrv_host_cdrom is not registered in > bdrv_file_init(). I think this is the missing part to have a host cdrom > support on MacOS X. I don't think we need a bdrv_host_cdrom registered. If one tiny change could make the real cdrom drive work, why completely implement this structure?
diff --git a/block.c b/block.c index dd4f58d..75ccfad 100644 --- a/block.c +++ b/block.c @@ -583,7 +583,8 @@ static int find_image_format(BlockDriverState *bs, const char *filename, int ret = 0; /* Return the raw BlockDriver * to scsi-generic devices or empty drives */ - if (bs->sg || !bdrv_is_inserted(bs) || bdrv_getlength(bs) == 0) { + if (bs->sg || !bdrv_is_inserted(bs) || bdrv_getlength(bs) == 0 + || strcmp("/dev/cdrom", filename) == 0) { *pdrv = &bdrv_raw; return ret; }
Fix real cdrom detection so that a real cdrom can actually be used. signed-off-by: John Arbuckle <programmingkidx@gmail.com> This patch has been tested on Mac OS X host and guest. Command used: qemu-system-ppc -cdrom /dev/cdrom Note: I was able to view the files using OpenBIOS, but not on Mac OS X. The size of the disc is reported correctly but some error happens that prevents it from mounting in Mac OS X. This is probably another bug with QEMU. --- block.c | 3 ++- 1 files changed, 2 insertions(+), 1 deletions(-)