On 11/27/2015 02:49 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. > > Signed-off-by: John Arbuckle <programmingk...@gmail.com> > > --- > Fixed some spacing issues. > Removed else condition in FindEjectableOpticalMedia. > Added continue statement to FindEjectableOpticalMedia. > Replaced printf() with error_report() in FindEjectableOpticalMedia. > Altered comment in FindEjectableOpticalMedia. > If the spacing in this patch looks off, try changing the font to something > that is mono-spaced.
Patches are best read in monospaced fonts, anyways; it's better to make that part of your workflow, and assume that everyone else has already done likewise, than to advertise that you are only making life harder for yourself. > > block/raw-posix.c | 140 ++++++++++++++++++++++++++++++++++++++-------------- > 1 files changed, 102 insertions(+), 38 deletions(-) > > diff --git a/block/raw-posix.c b/block/raw-posix.c > index ccfec1c..9e7de11 100644 > --- a/block/raw-posix.c > +++ b/block/raw-posix.c > @@ -42,9 +42,9 @@ > #include <IOKit/storage/IOMediaBSDClient.h> > #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 > +#endif /* (__APPLE__) && (__MACH__) */ > I have now mentioned in both v8 and v9 that this hunk should be its own patch (and is simple enough to cc qemu-trivial). Disregarding reviewers suggestions is not a good idea - it only serves to waste time (both yours and reviewers) and earn you black marks, such that it will be even less likely that anyone wants to review your patches in the first place. I'm trying to help you be a better contributor, but it feels like you are ignoring advice, and so my natural reaction is to ignore you. > #ifdef __sun__ > #define _POSIX_PTHREAD_SEMANTICS 1 > @@ -1975,32 +1975,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 kern_return_t FindEjectableOpticalMedia(io_iterator_t *mediaIterator, > + char > *mediaType) No, your indentation is still wrong. I tried to point out on your v8 that we don't right-justify to 80 columns, but rather left-justify to the point just after the (. > + 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.\n", No. Don't use trailing '.' or trailing '\n' in error_report() calls. I've already mentioned this, and feel like I'm becoming a broken record. When you disregard my review comments, I become disinclined to review your patches any further. > + > matching_array[index]); Indentation is still wrong. > + continue; > + } > + CFDictionarySetValue(classesToMatch, CFSTR(kIOMediaEjectableKey), > + > kCFBooleanTrue); > + kernResult = IOServiceGetMatchingServices(masterPort, classesToMatch, > + > mediaIterator); > + if (kernResult != KERN_SUCCESS) { > + error_report("Note: IOServiceGetMatchingServices returned %d\n", > + > kernResult); No trailing \n in error_report(), indentation is wrong. > + } > > + /* If a match was found, leave the loop */ > + if (*mediaIterator != 0) { > + DPRINTF("Matching using %s\n", matching_array[index]); > + snprintf(mediaType, strlen(matching_array[index])+1, "%s", > + > matching_array[index]); Spaces around binary '+', and indentation is wrong. > + /* 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); Indentation is wrong. > + /* if a working partition on the device was not found */ > + if (partition_found == false) { > + error_setg(errp, "Error: Failed to find a working partition on " > + > "disc!\n"); Indentation is wrong, no trailing '!' or '\n' in error_setg(). I'm so bothered by the fact that I already pointed this out in v9 and you still didn't fix it for v10 that I am not even paying attention to actual code, and just looking at style violations. You have effectively lost me as a valid reviewer on this patch. I don't like feeling like this, as I try hard to be welcoming to new contributors, but in the open source world, you have to return the favor by learning from the advice you are given, rather than repeating the same mistakes. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature