On Jul 17, 2015, at 9:41 AM, Stefan Hajnoczi wrote: > On Thu, Jul 16, 2015 at 04:46:07PM -0400, Programmingkid wrote: >> @@ -2014,7 +2015,9 @@ kern_return_t GetBSDPath( io_iterator_t mediaIterator, >> char *bsdPath, CFIndex ma >> if ( bsdPathAsCFString ) { >> size_t devPathLength; >> strcpy( bsdPath, _PATH_DEV ); >> - strcat( bsdPath, "r" ); >> + if (flags & BDRV_O_NOCACHE) { >> + strcat(bsdPath, "r"); >> + } >> devPathLength = strlen( bsdPath ); >> if ( CFStringGetCString( bsdPathAsCFString, bsdPath + >> devPathLength, maxPathSize - devPathLength, kCFStringEncodingASCII ) ) { >> kernResult = KERN_SUCCESS; > > Is this the fix that makes CD-ROM passthrough work for you? > > Does the guest boot successfully when you do: > > -drive if=ide,media=cdrom,cache=none,file=/dev/cdrom
The guest fails during the boot process with the above command line. >> @@ -2027,7 +2030,67 @@ kern_return_t GetBSDPath( io_iterator_t >> mediaIterator, char *bsdPath, CFIndex ma >> return kernResult; >> } >> >> -#endif >> +/* Sets up a physical device for use in QEMU */ >> +static void setupDevice(const char *bsdPath) >> +{ >> + /* >> + * Mac OS X does not like allowing QEMU to use physical devices that are >> + * mounted. Attempts to do so result in 'Resource busy' errors. >> + */ >> + >> + int fd; >> + fd = qemu_open(bsdPath, O_RDONLY | O_BINARY | O_LARGEFILE); >> + >> + /* if the device fails to open */ >> + if (fd < 0) { >> + printf("Error: failed to open %s\n", bsdPath); >> + printf("If device %s is mounted on the desktop, unmount it" >> + " first before using it in QEMU.\n", bsdPath); >> + printf("\nCommand to unmount device: diskutil unmountDisk %s", >> bsdPath); >> + printf("\nCommand to mount device: diskutil mountDisk %s\n\n", >> bsdPath); >> + } > > This error message is printed regardless of the errno value. What is > the specific errno value when open(2) fails because the device is > mounted? I could change the patch to take into account the errno value. errno = 2 when it fails. strerror(errno) = "No such file or directory". > > Also, can you move this after the raw_open_common() call to avoid the > setupCDROM()/setupDevice() changes made in this patch and duplicating > the error message. It doesn't seem necessary to open the files ahead of > raw_open_common() since the code continues in the error case anyway: > > ret = raw_open_common(bs, options, flags, 0, &local_err); > if (ret < 0) { > if (local_err) { > error_propagate(errp, local_err); > } > #if defined(__APPLE__) && defined(__MACH__) > if (strstart(filename, "/dev/") == 0 && ret == -EBUSY) { /* or > whatever */ > error_report("If device %s is mounted on the desktop, unmount it" > " first before using it in QEMU.", bsdPath); > error_report("Command to unmount device: diskutil unmountDisk %s", > bsdPath); > error_report("Command to mount device: diskutil mountDisk %s", > bsdPath); > } > #endif /* defined(__APPLE__) && defined(__MACH__) */ > return ret; > } > > That's a much smaller change. I will see what I can do. > >> + >> + /* if the device opens */ >> + else { >> + qemu_close(fd); >> + } >> +} >> + >> +/* Sets up a real cdrom for use in QEMU */ >> +static void setupCDROM(char *bsdPath) >> +{ >> + int index, numOfTestPartitions = 2, fd; >> + char testPartition[MAXPATHLEN]; >> + bool partitionFound = false; >> + >> + /* look for a working partition */ >> + for (index = 0; index < numOfTestPartitions; index++) { >> + strncpy(testPartition, bsdPath, MAXPATHLEN); > > The safe way to use strncpy() is: > > strncpy(testPartition, bsdPath, MAXPATHLEN - 1); > testPartition[MAXPATHLEN - 1] = '\0'; > > but pstrcpy() is a easier to use correctly. Please use that instead. Is pstrcpy() ansi c? I'm having trouble finding documentation for it. > >> + snprintf(testPartition, MAXPATHLEN, "%ss%d", testPartition, index); >> + fd = qemu_open(testPartition, O_RDONLY | O_BINARY | O_LARGEFILE); >> + if (fd > 0) { > > Should be fd >= 0 since fd = 0 is valid too, although unlikely. > >> + partitionFound = true; >> + qemu_close(fd); >> + break; >> + } >> + } >> + >> + /* if a working partition on the device was not found */ >> + if (partitionFound == false) { >> + printf("Error: Failed to find a working partition on disc!\n"); >> + printf("If your disc is mounted on the desktop, trying unmounting >> it" >> + " first before using it in QEMU.\n"); >> + printf("\nCommand to unmount disc: " >> + "diskutil unmountDisk %s\n", bsdPath); >> + printf("Command to mount disc: " >> + "diskutil mountDisk %s\n\n", bsdPath); >> + } >> + >> + DPRINTF("Using %s as CDROM\n", testPartition); >> + strncpy(bsdPath, testPartition, MAXPATHLEN); > > Please use pstrcpy(). > >> +} >> + >> +#endif /* defined(__APPLE__) && defined(__MACH__) */ >> >> static int hdev_probe_device(const char *filename) >> { >> @@ -2119,30 +2182,28 @@ static int hdev_open(BlockDriverState *bs, QDict >> *options, int flags, >> #if defined(__APPLE__) && defined(__MACH__) >> const char *filename = qdict_get_str(options, "filename"); >> >> - if (strstart(filename, "/dev/cdrom", NULL)) { >> - kern_return_t kernResult; >> - io_iterator_t mediaIterator; >> - char bsdPath[ MAXPATHLEN ]; >> - int fd; >> - >> - kernResult = FindEjectableCDMedia( &mediaIterator ); >> - kernResult = GetBSDPath( mediaIterator, bsdPath, sizeof( bsdPath ) >> ); >> - >> - if ( bsdPath[ 0 ] != '\0' ) { >> - strcat(bsdPath,"s0"); >> - /* some CDs don't have a partition 0 */ >> - fd = qemu_open(bsdPath, O_RDONLY | O_BINARY | O_LARGEFILE); >> - if (fd < 0) { >> - bsdPath[strlen(bsdPath)-1] = '1'; >> - } else { >> - qemu_close(fd); >> + /* If using a physical device */ >> + if (strstart(filename, "/dev/", NULL)) { >> + >> + /* If the physical device is a cdrom */ >> + if (strcmp(filename, "/dev/cdrom") == 0) { >> + char bsdPath[MAXPATHLEN]; >> + io_iterator_t mediaIterator; >> + FindEjectableCDMedia(&mediaIterator); >> + GetBSDPath(mediaIterator, bsdPath, sizeof(bsdPath), flags); > > Why did you remove the if (bsdPath[0] != [0]) check? Now the code > ignored GetBSDPath() failures. Ok, I will put that back in. > >> + if (mediaIterator) { >> + IOObjectRelease(mediaIterator); >> } >> + setupCDROM(bsdPath); >> filename = bsdPath; >> - qdict_put(options, "filename", qstring_from_str(filename)); >> } > > bsdPath is out of scope here so filename is a dangling pointer in the > /dev/cdrom case. Good catch. Will fix that.