| Message ID | 56307511-53E2-4B28-BCF5-423DCDABD0FC@gmail.com |
|---|---|
| State | New |
| Headers | show |
https://patchwork.ozlabs.org/patch/555945/ On Dec 11, 2015, at 10:27 PM, Programmingkid wrote: > Mac OS X can be picky when it comes to allowing the user > to use physical devices in QEMU. Most mounted volumes > appear to be off limits to QEMU. If an issue is detected, > a message is displayed showing the user how to unmount a > volume. Now QEMU uses both CD and DVD media. > > Signed-off-by: John Arbuckle <programmingkidx@gmail.com> > > --- > Removed mediaType parameter from FindEjectableOpticalMedia(). > Added goto statements to hdev_open. > Replaced snprintf() with g_strdup() in FindEjectableOpticalMedia(). > Put back return statement in hdev_open for Linux compatibility. > > block/raw-posix.c | 163 ++++++++++++++++++++++++++++++++++++++++------------- > 1 files changed, 124 insertions(+), 39 deletions(-) > > diff --git a/block/raw-posix.c b/block/raw-posix.c > index d9162fd..82e8e62 100644 > --- a/block/raw-posix.c > +++ b/block/raw-posix.c > @@ -43,6 +43,7 @@ > #include <IOKit/storage/IOMedia.h> > #include <IOKit/storage/IOCDMedia.h> > //#include <IOKit/storage/IOCDTypes.h> > +#include <IOKit/storage/IODVDMedia.h> > #include <CoreFoundation/CoreFoundation.h> > #endif > > @@ -1975,33 +1976,46 @@ BlockDriver bdrv_file = { > /* host device */ > > #if defined(__APPLE__) && defined(__MACH__) > -static kern_return_t FindEjectableCDMedia( io_iterator_t *mediaIterator ); > static kern_return_t GetBSDPath(io_iterator_t mediaIterator, char *bsdPath, > CFIndex maxPathSize, int flags); > -kern_return_t FindEjectableCDMedia( io_iterator_t *mediaIterator ) > +static char *FindEjectableOpticalMedia(io_iterator_t *mediaIterator) > { > - kern_return_t kernResult; > + kern_return_t kernResult = KERN_FAILURE; > mach_port_t masterPort; > CFMutableDictionaryRef classesToMatch; > + const char *matching_array[] = {kIODVDMediaClass, kIOCDMediaClass}; > + char *mediaType = NULL; > > kernResult = IOMasterPort( MACH_PORT_NULL, &masterPort ); > if ( KERN_SUCCESS != kernResult ) { > printf( "IOMasterPort returned %d\n", kernResult ); > } > > - classesToMatch = IOServiceMatching( kIOCDMediaClass ); > - if ( classesToMatch == NULL ) { > - printf( "IOServiceMatching returned a NULL dictionary.\n" ); > - } else { > - CFDictionarySetValue( classesToMatch, CFSTR( kIOMediaEjectableKey ), kCFBooleanTrue ); > - } > - kernResult = IOServiceGetMatchingServices( masterPort, classesToMatch, mediaIterator ); > - if ( KERN_SUCCESS != kernResult ) > - { > - printf( "IOServiceGetMatchingServices returned %d\n", kernResult ); > - } > + int index; > + for (index = 0; index < ARRAY_SIZE(matching_array); index++) { > + classesToMatch = IOServiceMatching(matching_array[index]); > + if (classesToMatch == NULL) { > + error_report("IOServiceMatching returned NULL for %s", > + matching_array[index]); > + continue; > + } > + CFDictionarySetValue(classesToMatch, CFSTR(kIOMediaEjectableKey), > + kCFBooleanTrue); > + kernResult = IOServiceGetMatchingServices(masterPort, classesToMatch, > + mediaIterator); > + if (kernResult != KERN_SUCCESS) { > + error_report("Note: IOServiceGetMatchingServices returned %d", > + kernResult); > + } > > - return kernResult; > + /* If a match was found, leave the loop */ > + if (*mediaIterator != 0) { > + DPRINTF("Matching using %s\n", matching_array[index]); > + mediaType = g_strdup(matching_array[index]); > + break; > + } > + } > + return mediaType; > } > > kern_return_t GetBSDPath(io_iterator_t mediaIterator, char *bsdPath, > @@ -2033,7 +2047,35 @@ kern_return_t GetBSDPath(io_iterator_t mediaIterator, char *bsdPath, > return kernResult; > } > > -#endif > +/* Sets up a real cdrom for use in QEMU */ > +static bool setup_cdrom(char *bsd_path, Error **errp) > +{ > + int index, num_of_test_partitions = 2, fd; > + char test_partition[MAXPATHLEN]; > + bool partition_found = false; > + > + /* look for a working partition */ > + for (index = 0; index < num_of_test_partitions; index++) { > + snprintf(test_partition, sizeof(test_partition), "%ss%d", bsd_path, > + index); > + fd = qemu_open(test_partition, O_RDONLY | O_BINARY | O_LARGEFILE); > + if (fd >= 0) { > + partition_found = true; > + qemu_close(fd); > + break; > + } > + } > + > + /* if a working partition on the device was not found */ > + if (partition_found == false) { > + error_setg(errp, "Failed to find a working partition on disc"); > + } else { > + DPRINTF("Using %s as optical disc\n", test_partition); > + pstrcpy(bsd_path, MAXPATHLEN, test_partition); > + } > + return partition_found; > +} > +#endif /* defined(__APPLE__) && defined(__MACH__) */ > > static int hdev_probe_device(const char *filename) > { > @@ -2115,6 +2157,16 @@ static bool hdev_is_sg(BlockDriverState *bs) > return false; > } > > +/* Prints directions on mounting and unmounting a device */ > +static void print_unmounting_directions(const char *file_name) > +{ > + error_report("If device %s is mounted on the desktop, unmount" > + " it first before using it in QEMU", file_name); > + error_report("Command to unmount device: diskutil unmountDisk %s", > + file_name); > + error_report("Command to mount device: diskutil mountDisk %s", file_name); > +} > + > static int hdev_open(BlockDriverState *bs, QDict *options, int flags, > Error **errp) > { > @@ -2125,32 +2177,55 @@ 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), > - flags); > - 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); > - } > - filename = bsdPath; > - qdict_put(options, "filename", qstring_from_str(filename)); > + /* If using a real cdrom */ > + if (strcmp(filename, "/dev/cdrom") == 0) { > + char bsd_path[MAXPATHLEN]; > + char *mediaType = NULL; > + kern_return_t ret_val; > + io_iterator_t mediaIterator = 0; > + > + mediaType = FindEjectableOpticalMedia(&mediaIterator); > + if (mediaType == NULL) { > + error_setg(errp, "Please make sure your CD/DVD is in the optical" > + " drive"); > + goto hdev_open_Mac_error; > + } > + > + ret_val = GetBSDPath(mediaIterator, bsd_path, sizeof(bsd_path), flags); > + if (ret_val != KERN_SUCCESS) { > + error_setg(errp, "Could not get BSD path for optical drive"); > + goto hdev_open_Mac_error; > + } > + > + /* If a real optical drive was not found */ > + if (bsd_path[0] == '\0') { > + error_setg(errp, "Failed to obtain bsd path for optical drive"); > + goto hdev_open_Mac_error; > + } > + > + /* If using a cdrom disc and finding a partition on the disc failed */ > + if (strncmp(mediaType, "IOCDMedia", 9) == 0 && > + setup_cdrom(bsd_path, errp) == false) { > + print_unmounting_directions(bsd_path); > + goto hdev_open_Mac_error; > } > > - if ( mediaIterator ) > - IOObjectRelease( mediaIterator ); > + g_free(mediaType); > + filename = bsd_path; > + qdict_put(options, "filename", qstring_from_str(filename)); > + goto continue_as_normal; /* skip over error handling code */ > + > + /* If an error occurred above */ > + hdev_open_Mac_error: > + if (mediaIterator) { > + IOObjectRelease(mediaIterator); > + } > + g_free(mediaType); > + return -1; > + > + continue_as_normal: ; > } > -#endif > +#endif /* defined(__APPLE__) && defined(__MACH__) */ > > s->type = FTYPE_FILE; > > @@ -2159,8 +2234,18 @@ static int hdev_open(BlockDriverState *bs, QDict *options, int flags, > if (local_err) { > error_propagate(errp, local_err); > } > + #if !defined(__APPLE__) && !defined(__MACH__) > return ret; > + #endif /* !defined(__APPLE__) && !defined(__MACH__) */ > + } > + > +#if defined(__APPLE__) && defined(__MACH__) > + /* if a physical device experienced an error while being opened */ > + if (strncmp(filename, "/dev/", 5) == 0 && ret != 0) { > + print_unmounting_directions(filename); > + return -1; > } > +#endif /* defined(__APPLE__) && defined(__MACH__) */ > > /* Since this does ioctl the device must be already opened */ > bs->sg = hdev_is_sg(bs); > -- > 1.7.5.4 > >
I do realize you are busy Kevin, but I would appreciate knowing my patch is in line for review. https://patchwork.ozlabs.org/patch/555945/ On Dec 11, 2015, at 10:27 PM, Programmingkid wrote: > Mac OS X can be picky when it comes to allowing the user > to use physical devices in QEMU. Most mounted volumes > appear to be off limits to QEMU. If an issue is detected, > a message is displayed showing the user how to unmount a > volume. Now QEMU uses both CD and DVD media. > > Signed-off-by: John Arbuckle <programmingkidx@gmail.com> > > --- > Removed mediaType parameter from FindEjectableOpticalMedia(). > Added goto statements to hdev_open. > Replaced snprintf() with g_strdup() in FindEjectableOpticalMedia(). > Put back return statement in hdev_open for Linux compatibility. > > block/raw-posix.c | 163 ++++++++++++++++++++++++++++++++++++++++------------- > 1 files changed, 124 insertions(+), 39 deletions(-) > > diff --git a/block/raw-posix.c b/block/raw-posix.c > index d9162fd..82e8e62 100644 > --- a/block/raw-posix.c > +++ b/block/raw-posix.c > @@ -43,6 +43,7 @@ > #include <IOKit/storage/IOMedia.h> > #include <IOKit/storage/IOCDMedia.h> > //#include <IOKit/storage/IOCDTypes.h> > +#include <IOKit/storage/IODVDMedia.h> > #include <CoreFoundation/CoreFoundation.h> > #endif > > @@ -1975,33 +1976,46 @@ BlockDriver bdrv_file = { > /* host device */ > > #if defined(__APPLE__) && defined(__MACH__) > -static kern_return_t FindEjectableCDMedia( io_iterator_t *mediaIterator ); > static kern_return_t GetBSDPath(io_iterator_t mediaIterator, char *bsdPath, > CFIndex maxPathSize, int flags); > -kern_return_t FindEjectableCDMedia( io_iterator_t *mediaIterator ) > +static char *FindEjectableOpticalMedia(io_iterator_t *mediaIterator) > { > - kern_return_t kernResult; > + kern_return_t kernResult = KERN_FAILURE; > mach_port_t masterPort; > CFMutableDictionaryRef classesToMatch; > + const char *matching_array[] = {kIODVDMediaClass, kIOCDMediaClass}; > + char *mediaType = NULL; > > kernResult = IOMasterPort( MACH_PORT_NULL, &masterPort ); > if ( KERN_SUCCESS != kernResult ) { > printf( "IOMasterPort returned %d\n", kernResult ); > } > > - classesToMatch = IOServiceMatching( kIOCDMediaClass ); > - if ( classesToMatch == NULL ) { > - printf( "IOServiceMatching returned a NULL dictionary.\n" ); > - } else { > - CFDictionarySetValue( classesToMatch, CFSTR( kIOMediaEjectableKey ), kCFBooleanTrue ); > - } > - kernResult = IOServiceGetMatchingServices( masterPort, classesToMatch, mediaIterator ); > - if ( KERN_SUCCESS != kernResult ) > - { > - printf( "IOServiceGetMatchingServices returned %d\n", kernResult ); > - } > + int index; > + for (index = 0; index < ARRAY_SIZE(matching_array); index++) { > + classesToMatch = IOServiceMatching(matching_array[index]); > + if (classesToMatch == NULL) { > + error_report("IOServiceMatching returned NULL for %s", > + matching_array[index]); > + continue; > + } > + CFDictionarySetValue(classesToMatch, CFSTR(kIOMediaEjectableKey), > + kCFBooleanTrue); > + kernResult = IOServiceGetMatchingServices(masterPort, classesToMatch, > + mediaIterator); > + if (kernResult != KERN_SUCCESS) { > + error_report("Note: IOServiceGetMatchingServices returned %d", > + kernResult); > + } > > - return kernResult; > + /* If a match was found, leave the loop */ > + if (*mediaIterator != 0) { > + DPRINTF("Matching using %s\n", matching_array[index]); > + mediaType = g_strdup(matching_array[index]); > + break; > + } > + } > + return mediaType; > } > > kern_return_t GetBSDPath(io_iterator_t mediaIterator, char *bsdPath, > @@ -2033,7 +2047,35 @@ kern_return_t GetBSDPath(io_iterator_t mediaIterator, char *bsdPath, > return kernResult; > } > > -#endif > +/* Sets up a real cdrom for use in QEMU */ > +static bool setup_cdrom(char *bsd_path, Error **errp) > +{ > + int index, num_of_test_partitions = 2, fd; > + char test_partition[MAXPATHLEN]; > + bool partition_found = false; > + > + /* look for a working partition */ > + for (index = 0; index < num_of_test_partitions; index++) { > + snprintf(test_partition, sizeof(test_partition), "%ss%d", bsd_path, > + index); > + fd = qemu_open(test_partition, O_RDONLY | O_BINARY | O_LARGEFILE); > + if (fd >= 0) { > + partition_found = true; > + qemu_close(fd); > + break; > + } > + } > + > + /* if a working partition on the device was not found */ > + if (partition_found == false) { > + error_setg(errp, "Failed to find a working partition on disc"); > + } else { > + DPRINTF("Using %s as optical disc\n", test_partition); > + pstrcpy(bsd_path, MAXPATHLEN, test_partition); > + } > + return partition_found; > +} > +#endif /* defined(__APPLE__) && defined(__MACH__) */ > > static int hdev_probe_device(const char *filename) > { > @@ -2115,6 +2157,16 @@ static bool hdev_is_sg(BlockDriverState *bs) > return false; > } > > +/* Prints directions on mounting and unmounting a device */ > +static void print_unmounting_directions(const char *file_name) > +{ > + error_report("If device %s is mounted on the desktop, unmount" > + " it first before using it in QEMU", file_name); > + error_report("Command to unmount device: diskutil unmountDisk %s", > + file_name); > + error_report("Command to mount device: diskutil mountDisk %s", file_name); > +} > + > static int hdev_open(BlockDriverState *bs, QDict *options, int flags, > Error **errp) > { > @@ -2125,32 +2177,55 @@ 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), > - flags); > - 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); > - } > - filename = bsdPath; > - qdict_put(options, "filename", qstring_from_str(filename)); > + /* If using a real cdrom */ > + if (strcmp(filename, "/dev/cdrom") == 0) { > + char bsd_path[MAXPATHLEN]; > + char *mediaType = NULL; > + kern_return_t ret_val; > + io_iterator_t mediaIterator = 0; > + > + mediaType = FindEjectableOpticalMedia(&mediaIterator); > + if (mediaType == NULL) { > + error_setg(errp, "Please make sure your CD/DVD is in the optical" > + " drive"); > + goto hdev_open_Mac_error; > + } > + > + ret_val = GetBSDPath(mediaIterator, bsd_path, sizeof(bsd_path), flags); > + if (ret_val != KERN_SUCCESS) { > + error_setg(errp, "Could not get BSD path for optical drive"); > + goto hdev_open_Mac_error; > + } > + > + /* If a real optical drive was not found */ > + if (bsd_path[0] == '\0') { > + error_setg(errp, "Failed to obtain bsd path for optical drive"); > + goto hdev_open_Mac_error; > + } > + > + /* If using a cdrom disc and finding a partition on the disc failed */ > + if (strncmp(mediaType, "IOCDMedia", 9) == 0 && > + setup_cdrom(bsd_path, errp) == false) { > + print_unmounting_directions(bsd_path); > + goto hdev_open_Mac_error; > } > > - if ( mediaIterator ) > - IOObjectRelease( mediaIterator ); > + g_free(mediaType); > + filename = bsd_path; > + qdict_put(options, "filename", qstring_from_str(filename)); > + goto continue_as_normal; /* skip over error handling code */ > + > + /* If an error occurred above */ > + hdev_open_Mac_error: > + if (mediaIterator) { > + IOObjectRelease(mediaIterator); > + } > + g_free(mediaType); > + return -1; > + > + continue_as_normal: ; > } > -#endif > +#endif /* defined(__APPLE__) && defined(__MACH__) */ > > s->type = FTYPE_FILE; > > @@ -2159,8 +2234,18 @@ static int hdev_open(BlockDriverState *bs, QDict *options, int flags, > if (local_err) { > error_propagate(errp, local_err); > } > + #if !defined(__APPLE__) && !defined(__MACH__) > return ret; > + #endif /* !defined(__APPLE__) && !defined(__MACH__) */ > + } > + > +#if defined(__APPLE__) && defined(__MACH__) > + /* if a physical device experienced an error while being opened */ > + if (strncmp(filename, "/dev/", 5) == 0 && ret != 0) { > + print_unmounting_directions(filename); > + return -1; > } > +#endif /* defined(__APPLE__) && defined(__MACH__) */ > > /* Since this does ioctl the device must be already opened */ > bs->sg = hdev_is_sg(bs); > -- > 1.7.5.4 > >
On 29.12.2015 01:27, Programmingkid wrote: > I do realize you are busy Kevin, but I would > appreciate knowing my patch is in line > for review. Primarily, he's been on holiday since before christmas until next week. (I'm telling you so you don't wonder why nothing happens.) Max
On Jan 4, 2016, at 11:35 AM, Max Reitz wrote: > On 29.12.2015 01:27, Programmingkid wrote: >> I do realize you are busy Kevin, but I would >> appreciate knowing my patch is in line >> for review. > > Primarily, he's been on holiday since before christmas until next week. > > (I'm telling you so you don't wonder why nothing happens.) > > Max > Thank you very much. I guess I have been a little frustrated with this patch. I have been trying to have it submitted into QEMU since August 2015!
Am 12.12.2015 um 04:27 hat Programmingkid geschrieben: > Mac OS X can be picky when it comes to allowing the user > to use physical devices in QEMU. Most mounted volumes > appear to be off limits to QEMU. If an issue is detected, > a message is displayed showing the user how to unmount a > volume. Now QEMU uses both CD and DVD media. > > Signed-off-by: John Arbuckle <programmingkidx@gmail.com> Please post patches at the top level instead as answers deeply nested in the email thread of another version of the same series. > diff --git a/block/raw-posix.c b/block/raw-posix.c > index d9162fd..82e8e62 100644 > --- a/block/raw-posix.c > +++ b/block/raw-posix.c > @@ -43,6 +43,7 @@ > #include <IOKit/storage/IOMedia.h> > #include <IOKit/storage/IOCDMedia.h> > //#include <IOKit/storage/IOCDTypes.h> > +#include <IOKit/storage/IODVDMedia.h> > #include <CoreFoundation/CoreFoundation.h> > #endif > > @@ -1975,33 +1976,46 @@ BlockDriver bdrv_file = { > /* host device */ > > #if defined(__APPLE__) && defined(__MACH__) > -static kern_return_t FindEjectableCDMedia( io_iterator_t *mediaIterator ); > static kern_return_t GetBSDPath(io_iterator_t mediaIterator, char *bsdPath, > CFIndex maxPathSize, int flags); > -kern_return_t FindEjectableCDMedia( io_iterator_t *mediaIterator ) > +static char *FindEjectableOpticalMedia(io_iterator_t *mediaIterator) > { > - kern_return_t kernResult; > + kern_return_t kernResult = KERN_FAILURE; > mach_port_t masterPort; > CFMutableDictionaryRef classesToMatch; > + const char *matching_array[] = {kIODVDMediaClass, kIOCDMediaClass}; > + char *mediaType = NULL; > > kernResult = IOMasterPort( MACH_PORT_NULL, &masterPort ); > if ( KERN_SUCCESS != kernResult ) { > printf( "IOMasterPort returned %d\n", kernResult ); > } > > - classesToMatch = IOServiceMatching( kIOCDMediaClass ); > - if ( classesToMatch == NULL ) { > - printf( "IOServiceMatching returned a NULL dictionary.\n" ); > - } else { > - CFDictionarySetValue( classesToMatch, CFSTR( kIOMediaEjectableKey ), kCFBooleanTrue ); > - } > - kernResult = IOServiceGetMatchingServices( masterPort, classesToMatch, mediaIterator ); > - if ( KERN_SUCCESS != kernResult ) > - { > - printf( "IOServiceGetMatchingServices returned %d\n", kernResult ); > - } > + int index; > + for (index = 0; index < ARRAY_SIZE(matching_array); index++) { > + classesToMatch = IOServiceMatching(matching_array[index]); > + if (classesToMatch == NULL) { > + error_report("IOServiceMatching returned NULL for %s", > + matching_array[index]); > + continue; > + } > + CFDictionarySetValue(classesToMatch, CFSTR(kIOMediaEjectableKey), > + kCFBooleanTrue); > + kernResult = IOServiceGetMatchingServices(masterPort, classesToMatch, > + mediaIterator); > + if (kernResult != KERN_SUCCESS) { > + error_report("Note: IOServiceGetMatchingServices returned %d", > + kernResult); > + } > > - return kernResult; > + /* If a match was found, leave the loop */ > + if (*mediaIterator != 0) { > + DPRINTF("Matching using %s\n", matching_array[index]); > + mediaType = g_strdup(matching_array[index]); > + break; > + } It's unclear to me whether *mediaIterator is valid in error cases. The documentation says "An iterator handle is returned on success, and should be released by the caller when the iteration is finished", but it doesn't say anything about error cases. It feels safer to 'continue;' in the != KERN_SUCCESS case. > + } > + return mediaType; > } > > kern_return_t GetBSDPath(io_iterator_t mediaIterator, char *bsdPath, > @@ -2033,7 +2047,35 @@ kern_return_t GetBSDPath(io_iterator_t mediaIterator, char *bsdPath, > return kernResult; > } > > -#endif > +/* Sets up a real cdrom for use in QEMU */ > +static bool setup_cdrom(char *bsd_path, Error **errp) > +{ > + int index, num_of_test_partitions = 2, fd; > + char test_partition[MAXPATHLEN]; > + bool partition_found = false; > + > + /* look for a working partition */ > + for (index = 0; index < num_of_test_partitions; index++) { > + snprintf(test_partition, sizeof(test_partition), "%ss%d", bsd_path, > + index); > + fd = qemu_open(test_partition, O_RDONLY | O_BINARY | O_LARGEFILE); > + if (fd >= 0) { > + partition_found = true; > + qemu_close(fd); > + break; > + } > + } > + > + /* if a working partition on the device was not found */ > + if (partition_found == false) { > + error_setg(errp, "Failed to find a working partition on disc"); > + } else { > + DPRINTF("Using %s as optical disc\n", test_partition); > + pstrcpy(bsd_path, MAXPATHLEN, test_partition); > + } > + return partition_found; > +} > +#endif /* defined(__APPLE__) && defined(__MACH__) */ > > static int hdev_probe_device(const char *filename) > { > @@ -2115,6 +2157,16 @@ static bool hdev_is_sg(BlockDriverState *bs) > return false; > } > > +/* Prints directions on mounting and unmounting a device */ > +static void print_unmounting_directions(const char *file_name) > +{ > + error_report("If device %s is mounted on the desktop, unmount" > + " it first before using it in QEMU", file_name); > + error_report("Command to unmount device: diskutil unmountDisk %s", > + file_name); > + error_report("Command to mount device: diskutil mountDisk %s", file_name); > +} On Linux: block/raw-posix.c:2150:13: error: 'print_unmounting_directions' defined but not used [-Werror=unused-function] static void print_unmounting_directions(const char *file_name) > static int hdev_open(BlockDriverState *bs, QDict *options, int flags, > Error **errp) > { > @@ -2125,32 +2177,55 @@ 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), > - flags); > - 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); > - } > - filename = bsdPath; > - qdict_put(options, "filename", qstring_from_str(filename)); > + /* If using a real cdrom */ > + if (strcmp(filename, "/dev/cdrom") == 0) { > + char bsd_path[MAXPATHLEN]; > + char *mediaType = NULL; > + kern_return_t ret_val; > + io_iterator_t mediaIterator = 0; > + > + mediaType = FindEjectableOpticalMedia(&mediaIterator); > + if (mediaType == NULL) { > + error_setg(errp, "Please make sure your CD/DVD is in the optical" > + " drive"); > + goto hdev_open_Mac_error; > + } > + > + ret_val = GetBSDPath(mediaIterator, bsd_path, sizeof(bsd_path), flags); > + if (ret_val != KERN_SUCCESS) { > + error_setg(errp, "Could not get BSD path for optical drive"); > + goto hdev_open_Mac_error; > + } > + > + /* If a real optical drive was not found */ > + if (bsd_path[0] == '\0') { > + error_setg(errp, "Failed to obtain bsd path for optical drive"); > + goto hdev_open_Mac_error; > + } > + > + /* If using a cdrom disc and finding a partition on the disc failed */ > + if (strncmp(mediaType, "IOCDMedia", 9) == 0 && > + setup_cdrom(bsd_path, errp) == false) { Indentation is off. I guess kIOCDMediaClass, as returned by FindEjectableOpticalMedia(), is the same as "IOCDMedia". You should use the same constant here, then. > + print_unmounting_directions(bsd_path); > + goto hdev_open_Mac_error; > } > > - if ( mediaIterator ) > - IOObjectRelease( mediaIterator ); > + g_free(mediaType); > + filename = bsd_path; filename is still used after bsd_path goes out of scope. > + qdict_put(options, "filename", qstring_from_str(filename)); > + goto continue_as_normal; /* skip over error handling code */ > + > + /* If an error occurred above */ > + hdev_open_Mac_error: > + if (mediaIterator) { > + IOObjectRelease(mediaIterator); > + } > + g_free(mediaType); > + return -1; > + > + continue_as_normal: ; What is this? Seriously, you don't expect me to merge _that_, do you? There is no way I'll accept jumping across code with goto like this. Error handling belongs at the end of the function. If you need many variables from a local block, so that pulling them up to function level would be awkward and doing local error handling is preferable, it may be an indication that your block should become a function of its own. > } > -#endif > +#endif /* defined(__APPLE__) && defined(__MACH__) */ > > s->type = FTYPE_FILE; > > @@ -2159,8 +2234,18 @@ static int hdev_open(BlockDriverState *bs, QDict *options, int flags, > if (local_err) { > error_propagate(errp, local_err); > } > + #if !defined(__APPLE__) && !defined(__MACH__) > return ret; > + #endif /* !defined(__APPLE__) && !defined(__MACH__) */ *sigh* > + } > + > +#if defined(__APPLE__) && defined(__MACH__) > + /* if a physical device experienced an error while being opened */ > + if (strncmp(filename, "/dev/", 5) == 0 && ret != 0) { > + print_unmounting_directions(filename); > + return -1; > } > +#endif /* defined(__APPLE__) && defined(__MACH__) */ How many times did I tell you that you should move this check inside the if (ret < 0) block? Finally some meta comments: In your ping email you wrote about how frustrating this series has become for you. Do you realise that it has become frustrating for reviewers as well, and that that's the reason why you don't get quick reviews any more? Everyone assumes that, like every time, you still haven't addressed the things that were commented on five versions ago (and hey, look above, they were right!), and that reviewing it for the twelfth time would simply be a waste of reviewers' time. Reviewers getting tired of reviewing the same thing again is a problem that any patch series that goes beyond v3 has. However, if you ignore review comments repeatedly and/or you get close to v10, people will stop thinking a more or less neutral "oh, another version of that series" and start thinking a clearly negative "oh no, not again this crap". That's where this series is, and not getting timely review (because the maintainer is the only one who still _has_ to look at it and you've lost everyone else) is a consequence of it. Kevin
diff --git a/block/raw-posix.c b/block/raw-posix.c index d9162fd..82e8e62 100644 --- a/block/raw-posix.c +++ b/block/raw-posix.c @@ -43,6 +43,7 @@ #include <IOKit/storage/IOMedia.h> #include <IOKit/storage/IOCDMedia.h> //#include <IOKit/storage/IOCDTypes.h> +#include <IOKit/storage/IODVDMedia.h> #include <CoreFoundation/CoreFoundation.h> #endif @@ -1975,33 +1976,46 @@ BlockDriver bdrv_file = { /* host device */ #if defined(__APPLE__) && defined(__MACH__) -static kern_return_t FindEjectableCDMedia( io_iterator_t *mediaIterator ); static kern_return_t GetBSDPath(io_iterator_t mediaIterator, char *bsdPath, CFIndex maxPathSize, int flags); -kern_return_t FindEjectableCDMedia( io_iterator_t *mediaIterator ) +static char *FindEjectableOpticalMedia(io_iterator_t *mediaIterator) { - kern_return_t kernResult; + kern_return_t kernResult = KERN_FAILURE; mach_port_t masterPort; CFMutableDictionaryRef classesToMatch; + const char *matching_array[] = {kIODVDMediaClass, kIOCDMediaClass}; + char *mediaType = NULL; kernResult = IOMasterPort( MACH_PORT_NULL, &masterPort ); if ( KERN_SUCCESS != kernResult ) { printf( "IOMasterPort returned %d\n", kernResult ); } - classesToMatch = IOServiceMatching( kIOCDMediaClass ); - if ( classesToMatch == NULL ) { - printf( "IOServiceMatching returned a NULL dictionary.\n" ); - } else { - CFDictionarySetValue( classesToMatch, CFSTR( kIOMediaEjectableKey ), kCFBooleanTrue ); - } - kernResult = IOServiceGetMatchingServices( masterPort, classesToMatch, mediaIterator ); - if ( KERN_SUCCESS != kernResult ) - { - printf( "IOServiceGetMatchingServices returned %d\n", kernResult ); - } + int index; + for (index = 0; index < ARRAY_SIZE(matching_array); index++) { + classesToMatch = IOServiceMatching(matching_array[index]); + if (classesToMatch == NULL) { + error_report("IOServiceMatching returned NULL for %s", + matching_array[index]); + continue; + } + CFDictionarySetValue(classesToMatch, CFSTR(kIOMediaEjectableKey), + kCFBooleanTrue); + kernResult = IOServiceGetMatchingServices(masterPort, classesToMatch, + mediaIterator); + if (kernResult != KERN_SUCCESS) { + error_report("Note: IOServiceGetMatchingServices returned %d", + kernResult); + } - return kernResult; + /* If a match was found, leave the loop */ + if (*mediaIterator != 0) { + DPRINTF("Matching using %s\n", matching_array[index]); + mediaType = g_strdup(matching_array[index]); + break; + } + } + return mediaType; } kern_return_t GetBSDPath(io_iterator_t mediaIterator, char *bsdPath, @@ -2033,7 +2047,35 @@ kern_return_t GetBSDPath(io_iterator_t mediaIterator, char *bsdPath, return kernResult; } -#endif +/* Sets up a real cdrom for use in QEMU */ +static bool setup_cdrom(char *bsd_path, Error **errp) +{ + int index, num_of_test_partitions = 2, fd; + char test_partition[MAXPATHLEN]; + bool partition_found = false; + + /* look for a working partition */ + for (index = 0; index < num_of_test_partitions; index++) { + snprintf(test_partition, sizeof(test_partition), "%ss%d", bsd_path, + index); + fd = qemu_open(test_partition, O_RDONLY | O_BINARY | O_LARGEFILE); + if (fd >= 0) { + partition_found = true; + qemu_close(fd); + break; + } + } + + /* if a working partition on the device was not found */ + if (partition_found == false) { + error_setg(errp, "Failed to find a working partition on disc"); + } else { + DPRINTF("Using %s as optical disc\n", test_partition); + pstrcpy(bsd_path, MAXPATHLEN, test_partition); + } + return partition_found; +} +#endif /* defined(__APPLE__) && defined(__MACH__) */ static int hdev_probe_device(const char *filename) { @@ -2115,6 +2157,16 @@ static bool hdev_is_sg(BlockDriverState *bs) return false; } +/* Prints directions on mounting and unmounting a device */ +static void print_unmounting_directions(const char *file_name) +{ + error_report("If device %s is mounted on the desktop, unmount" + " it first before using it in QEMU", file_name); + error_report("Command to unmount device: diskutil unmountDisk %s", + file_name); + error_report("Command to mount device: diskutil mountDisk %s", file_name); +} + static int hdev_open(BlockDriverState *bs, QDict *options, int flags, Error **errp) { @@ -2125,32 +2177,55 @@ 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), - flags); - 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); - } - filename = bsdPath; - qdict_put(options, "filename", qstring_from_str(filename)); + /* If using a real cdrom */ + if (strcmp(filename, "/dev/cdrom") == 0) { + char bsd_path[MAXPATHLEN]; + char *mediaType = NULL; + kern_return_t ret_val; + io_iterator_t mediaIterator = 0; + + mediaType = FindEjectableOpticalMedia(&mediaIterator); + if (mediaType == NULL) { + error_setg(errp, "Please make sure your CD/DVD is in the optical" + " drive"); + goto hdev_open_Mac_error; + } + + ret_val = GetBSDPath(mediaIterator, bsd_path, sizeof(bsd_path), flags); + if (ret_val != KERN_SUCCESS) { + error_setg(errp, "Could not get BSD path for optical drive"); + goto hdev_open_Mac_error; + } + + /* If a real optical drive was not found */ + if (bsd_path[0] == '\0') { + error_setg(errp, "Failed to obtain bsd path for optical drive"); + goto hdev_open_Mac_error; + } + + /* If using a cdrom disc and finding a partition on the disc failed */ + if (strncmp(mediaType, "IOCDMedia", 9) == 0 && + setup_cdrom(bsd_path, errp) == false) { + print_unmounting_directions(bsd_path); + goto hdev_open_Mac_error; } - if ( mediaIterator ) - IOObjectRelease( mediaIterator ); + g_free(mediaType); + filename = bsd_path; + qdict_put(options, "filename", qstring_from_str(filename)); + goto continue_as_normal; /* skip over error handling code */ + + /* If an error occurred above */ + hdev_open_Mac_error: + if (mediaIterator) { + IOObjectRelease(mediaIterator); + } + g_free(mediaType); + return -1; + + continue_as_normal: ; } -#endif +#endif /* defined(__APPLE__) && defined(__MACH__) */ s->type = FTYPE_FILE; @@ -2159,8 +2234,18 @@ static int hdev_open(BlockDriverState *bs, QDict *options, int flags, if (local_err) { error_propagate(errp, local_err); } + #if !defined(__APPLE__) && !defined(__MACH__) return ret; + #endif /* !defined(__APPLE__) && !defined(__MACH__) */ + } + +#if defined(__APPLE__) && defined(__MACH__) + /* if a physical device experienced an error while being opened */ + if (strncmp(filename, "/dev/", 5) == 0 && ret != 0) { + print_unmounting_directions(filename); + return -1; } +#endif /* defined(__APPLE__) && defined(__MACH__) */ /* Since this does ioctl the device must be already opened */ bs->sg = hdev_is_sg(bs);
Mac OS X can be picky when it comes to allowing the user to use physical devices in QEMU. Most mounted volumes appear to be off limits to QEMU. If an issue is detected, a message is displayed showing the user how to unmount a volume. Now QEMU uses both CD and DVD media. Signed-off-by: John Arbuckle <programmingkidx@gmail.com> --- Removed mediaType parameter from FindEjectableOpticalMedia(). Added goto statements to hdev_open. Replaced snprintf() with g_strdup() in FindEjectableOpticalMedia(). Put back return statement in hdev_open for Linux compatibility. block/raw-posix.c | 163 ++++++++++++++++++++++++++++++++++++++++------------- 1 files changed, 124 insertions(+), 39 deletions(-)