Re: [Qemu-block] [Qemu-devel] [PATCH v9] block/raw-posix.c: Make physical devices usable in QEMU under Mac OS X host

[Eric Blake]

On 11/25/2015 09:10 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 <address@hidden>
> 
> ---
> Added DVD support - real DVD media can now be used in QEMU!
> Changed name of FindEjectableCDMedia to FindEjectableOpticalMedia.
> Added mediaType parameter to FindEjectableOpticalMedia() for media 
> indentification.
> Removed unneeded FindEjectableCDMedia() prototype.
> FindEjectableOpticalMedia() now searches for both DVD's and CD's.
> 

> +++ 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 don't know if my review of v8 crossed your posting of this patch, but
I suggested that this hunk belongs in its own patch.

>  #ifdef __sun__
>  #define _POSIX_PTHREAD_SEMANTICS 1
> @@ -1975,32 +1975,44 @@ 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)

Unusual indentation; more typical is:

| static kern_return_t FindEjectableOpticalMedia(io_iterator_t
*mediaIterator,
| char *mediatType)


> +    int index;
> +    for (index = 0; index < ARRAY_SIZE(matching_array); index++) {
> +        classesToMatch = IOServiceMatching(matching_array[index]);
> +        if (classesToMatch == NULL) {
> +            printf("IOServiceMatching returned a NULL dictionary.\n");

Leftover debugging?

> +        } else {
> +        CFDictionarySetValue(classesToMatch, CFSTR(kIOMediaEjectableKey),

Missing indentation.

> +                                                                
> kCFBooleanTrue);
> +        }
> +        kernResult = IOServiceGetMatchingServices(masterPort, classesToMatch,
> +                                                                 
> mediaIterator);

More unusual indentation.

> +        if (KERN_SUCCESS != kernResult) {
> +            printf("IOServiceGetMatchingServices returned %d\n", kernResult);
> +        }
>  
> +        /* If you found a match, leave the loop */
> +        if (*mediaIterator != 0) {
> +            DPRINTF("Matching using %s\n", matching_array[index]);
> +            snprintf(mediaType, strlen(matching_array[index])+1, "%s",

Spaces around binary '+'.

> +    /* 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");

and I already pointed out on v8 that this is not the correct usage of
error_setg().  So, here's hoping v10 addresses the comments here and
elsewhere.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

signature.asc
Description: OpenPGP digital signature