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

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

[Programmingkid]

[-- Attachment #1: Type: text/plain, Size: 5795 bytes --]

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 <programmingkidx@gmail.com>

---
Removed changes to GetBSDPath() to a separate patch.
This patch now depends on the GetBSDPath patch.

 block/raw-posix.c |   90 +++++++++++++++++++++++++++++++++++++---------------
 1 files changed, 64 insertions(+), 26 deletions(-)

diff --git a/block/raw-posix.c b/block/raw-posix.c
index 67d1d48..a41006f 100644
--- a/block/raw-posix.c
+++ b/block/raw-posix.c
@@ -42,9 +42,8 @@
 #include <IOKit/storage/IOMediaBSDClient.h>
 #include <IOKit/storage/IOMedia.h>
 #include <IOKit/storage/IOCDMedia.h>
-//#include <IOKit/storage/IOCDTypes.h>
 #include <CoreFoundation/CoreFoundation.h>
-#endif
+#endif /* (__APPLE__) && (__MACH__) */
 
 #ifdef __sun__
 #define _POSIX_PTHREAD_SEMANTICS 1
@@ -1972,7 +1971,7 @@ BlockDriver bdrv_file = {
 /* host device */
 
 #if defined(__APPLE__) && defined(__MACH__)
-static kern_return_t FindEjectableCDMedia( io_iterator_t *mediaIterator );
+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 )
@@ -2030,7 +2029,34 @@ kern_return_t GetBSDPath(io_iterator_t mediaIterator, char *bsdPath,
     return kernResult;
 }
 
-#endif
+/* Sets up a real cdrom for use in QEMU */
+static bool 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++) {
+        snprintf(testPartition, sizeof(testPartition), "%ss%d", bsdPath, index);
+        fd = qemu_open(testPartition, O_RDONLY | O_BINARY | O_LARGEFILE);
+        if (fd >= 0) {
+            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");
+    } else {
+        DPRINTF("Using %s as optical disc\n", testPartition);
+        pstrcpy(bsdPath, MAXPATHLEN, testPartition);
+    }
+    return partitionFound;
+}
+#endif /* defined(__APPLE__) && defined(__MACH__) */
 
 static int hdev_probe_device(const char *filename)
 {
@@ -2118,34 +2144,32 @@ static int hdev_open(BlockDriverState *bs, QDict *options, int flags,
     BDRVRawState *s = bs->opaque;
     Error *local_err = NULL;
     int ret;
+    bool cdromOK = true;
 
 #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';
+    /* If using a physical device */
+    if (strstart(filename, "/dev/", NULL)) {
+        char bsdPath[MAXPATHLEN];
+
+        /* If the physical device is a cdrom */
+        if (strcmp(filename, "/dev/cdrom") == 0) {
+            io_iterator_t mediaIterator;
+            FindEjectableCDMedia(&mediaIterator);
+            GetBSDPath(mediaIterator, bsdPath, sizeof(bsdPath), flags);
+            if (bsdPath[0] == '\0') {
+                printf("Error: failed to obtain bsd path for optical drive!\n");
             } else {
-                qemu_close(fd);
+                cdromOK = setupCDROM(bsdPath);
+                filename = bsdPath;
             }
-            filename = bsdPath;
-            qdict_put(options, "filename", qstring_from_str(filename));
-        }
 
-        if ( mediaIterator )
-            IOObjectRelease( mediaIterator );
+            if (mediaIterator) {
+                IOObjectRelease(mediaIterator);
+            }
+        }
+        qdict_put(options, "filename", qstring_from_str(filename));
     }
 #endif
 
@@ -2156,7 +2180,21 @@ static int hdev_open(BlockDriverState *bs, QDict *options, int flags,
         if (local_err) {
             error_propagate(errp, local_err);
         }
-        return ret;
+    }
+
+#if defined(__APPLE__) && defined(__MACH__)
+    /* if a physical device experienced an error while being opened */
+    if (strncmp(filename, "/dev/", 5) == 0 && (cdromOK == false || ret != 0)) {
+        printf("If device %s is mounted on the desktop, unmount it"
+                        " first before using it in QEMU.\n", filename);
+        printf("Command to unmount device: diskutil unmountDisk %s\n",
+                                                                    filename);
+        printf("Command to mount device: diskutil mountDisk %s\n", filename);
+    }
+#endif /* defined(__APPLE__) && defined(__MACH__) */
+
+    if (ret < 0) {
+       return ret;
     }
 
     /* Since this does ioctl the device must be already opened */
-- 
1.7.5.4


[-- Attachment #2: Type: text/html, Size: 29523 bytes --]

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

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 668 bytes --]

On Mon, Jul 27, 2015 at 01:05:15PM -0400, 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 <programmingkidx@gmail.com>
> 
> ---
> Removed changes to GetBSDPath() to a separate patch.
> This patch now depends on the GetBSDPath patch.
> 
>  block/raw-posix.c |   90 +++++++++++++++++++++++++++++++++++++---------------
>  1 files changed, 64 insertions(+), 26 deletions(-)

Kevin: Please review for QEMU 2.5

[-- Attachment #2: Type: application/pgp-signature, Size: 473 bytes --]

[Qemu-devel] ping: [PATCH v5] raw-posix.c: Make physical devices usable in QEMU under Mac OS X host

[Programmingkid]

[-- Attachment #1: Type: text/plain, Size: 6210 bytes --]

Is there a reason why this patch hasn't been reviewed yet?

http://patchwork.ozlabs.org/patch/500498/

> 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 <programmingkidx@gmail.com>
> 
> ---
> Removed changes to GetBSDPath() to a separate patch.
> This patch now depends on the GetBSDPath patch.
> 
>  block/raw-posix.c |   90 +++++++++++++++++++++++++++++++++++++---------------
>  1 files changed, 64 insertions(+), 26 deletions(-)
> 
> diff --git a/block/raw-posix.c b/block/raw-posix.c
> index 67d1d48..a41006f 100644
> --- a/block/raw-posix.c
> +++ b/block/raw-posix.c
> @@ -42,9 +42,8 @@
>  #include <IOKit/storage/IOMediaBSDClient.h>
>  #include <IOKit/storage/IOMedia.h>
>  #include <IOKit/storage/IOCDMedia.h>
> -//#include <IOKit/storage/IOCDTypes.h>
>  #include <CoreFoundation/CoreFoundation.h>
> -#endif
> +#endif /* (__APPLE__) && (__MACH__) */
>  
>  #ifdef __sun__
>  #define _POSIX_PTHREAD_SEMANTICS 1
> @@ -1972,7 +1971,7 @@ BlockDriver bdrv_file = {
>  /* host device */
>  
>  #if defined(__APPLE__) && defined(__MACH__)
> -static kern_return_t FindEjectableCDMedia( io_iterator_t *mediaIterator );
> +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 )
> @@ -2030,7 +2029,34 @@ kern_return_t GetBSDPath(io_iterator_t mediaIterator, char *bsdPath,
>      return kernResult;
>  }
>  
> -#endif
> +/* Sets up a real cdrom for use in QEMU */
> +static bool 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++) {
> +        snprintf(testPartition, sizeof(testPartition), "%ss%d", bsdPath, index);
> +        fd = qemu_open(testPartition, O_RDONLY | O_BINARY | O_LARGEFILE);
> +        if (fd >= 0) {
> +            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");
> +    } else {
> +        DPRINTF("Using %s as optical disc\n", testPartition);
> +        pstrcpy(bsdPath, MAXPATHLEN, testPartition);
> +    }
> +    return partitionFound;
> +}
> +#endif /* defined(__APPLE__) && defined(__MACH__) */
>  
>  static int hdev_probe_device(const char *filename)
>  {
> @@ -2118,34 +2144,32 @@ static int hdev_open(BlockDriverState *bs, QDict *options, int flags,
>      BDRVRawState *s = bs->opaque;
>      Error *local_err = NULL;
>      int ret;
> +    bool cdromOK = true;
>  
>  #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';
> +    /* If using a physical device */
> +    if (strstart(filename, "/dev/", NULL)) {
> +        char bsdPath[MAXPATHLEN];
> +
> +        /* If the physical device is a cdrom */
> +        if (strcmp(filename, "/dev/cdrom") == 0) {
> +            io_iterator_t mediaIterator;
> +            FindEjectableCDMedia(&mediaIterator);
> +            GetBSDPath(mediaIterator, bsdPath, sizeof(bsdPath), flags);
> +            if (bsdPath[0] == '\0') {
> +                printf("Error: failed to obtain bsd path for optical drive!\n");
>              } else {
> -                qemu_close(fd);
> +                cdromOK = setupCDROM(bsdPath);
> +                filename = bsdPath;
>              }
> -            filename = bsdPath;
> -            qdict_put(options, "filename", qstring_from_str(filename));
> -        }
>  
> -        if ( mediaIterator )
> -            IOObjectRelease( mediaIterator );
> +            if (mediaIterator) {
> +                IOObjectRelease(mediaIterator);
> +            }
> +        }
> +        qdict_put(options, "filename", qstring_from_str(filename));
>      }
>  #endif
>  
> @@ -2156,7 +2180,21 @@ static int hdev_open(BlockDriverState *bs, QDict *options, int flags,
>          if (local_err) {
>              error_propagate(errp, local_err);
>          }
> -        return ret;
> +    }
> +
> +#if defined(__APPLE__) && defined(__MACH__)
> +    /* if a physical device experienced an error while being opened */
> +    if (strncmp(filename, "/dev/", 5) == 0 && (cdromOK == false || ret != 0)) {
> +        printf("If device %s is mounted on the desktop, unmount it"
> +                        " first before using it in QEMU.\n", filename);
> +        printf("Command to unmount device: diskutil unmountDisk %s\n",
> +                                                                    filename);
> +        printf("Command to mount device: diskutil mountDisk %s\n", filename);
> +    }
> +#endif /* defined(__APPLE__) && defined(__MACH__) */
> +
> +    if (ret < 0) {
> +       return ret;
>      }
>  
>      /* Since this does ioctl the device must be already opened */
> -- 
> 1.7.5.4
> 


[-- Attachment #2: Type: text/html, Size: 30511 bytes --]

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

[Kevin Wolf]

Am 27.07.2015 um 19:05 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.
> 
> Signed-off-by: John Arbuckle <programmingkidx@gmail.com>
> 
> ---
> Removed changes to GetBSDPath() to a separate patch.
> This patch now depends on the GetBSDPath patch.

Unfortunately, this patch was sent as HTML, so git am doesn't accept it.
I tried to manually get something working out of it, but I failed.
Possibly there are actual merge conflicts, too (even going back to
master@{2015-07-27}), but in any case I couldn't apply this.

Can you please rebase and send as a plain text patch that applies to
current master?

> @@ -2156,7 +2180,21 @@ static int hdev_open(BlockDriverState *bs, QDict
> *options, int flags,
>          if (local_err) {
>              error_propagate(errp, local_err);
>          }
> -        return ret;
> +    }
> +
> +#if defined(__APPLE__) && defined(__MACH__)
> +    /* if a physical device experienced an error while being opened */
> +    if (strncmp(filename, "/dev/", 5) == 0 && (cdromOK == false || ret != 0))
> {
> +        printf("If device %s is mounted on the desktop, unmount it"
> +                        " first before using it in QEMU.\n", filename);
> +        printf("Command to unmount device: diskutil unmountDisk %s\n",
> +                                                                    filename);
> +        printf("Command to mount device: diskutil mountDisk %s\n", filename);
> +    }
> +#endif /* defined(__APPLE__) && defined(__MACH__) */
> +
> +    if (ret < 0) {
> +       return ret;
>      }

Why don't you simply include the #ifdef block in the first if (ret < 0)?
Or does ret > 0 happen and the message must be displayed for it?

Kevin

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

[Programmingkid]

On Nov 20, 2015, at 11:26 AM, Kevin Wolf wrote:

> Am 27.07.2015 um 19:05 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.
>> 
>> Signed-off-by: John Arbuckle <programmingkidx@gmail.com>
>> 
>> ---
>> Removed changes to GetBSDPath() to a separate patch.
>> This patch now depends on the GetBSDPath patch.
> 
> Unfortunately, this patch was sent as HTML, so git am doesn't accept it.
> I tried to manually get something working out of it, but I failed.
> Possibly there are actual merge conflicts, too (even going back to
> master@{2015-07-27}), but in any case I couldn't apply this.
> 
> Can you please rebase and send as a plain text patch that applies to
> current master?

Sorry for the HTML. This patch has been fixed and sent in.

> 
>> @@ -2156,7 +2180,21 @@ static int hdev_open(BlockDriverState *bs, QDict
>> *options, int flags,
>>         if (local_err) {
>>             error_propagate(errp, local_err);
>>         }
>> -        return ret;
>> +    }
>> +
>> +#if defined(__APPLE__) && defined(__MACH__)
>> +    /* if a physical device experienced an error while being opened */
>> +    if (strncmp(filename, "/dev/", 5) == 0 && (cdromOK == false || ret != 0))
>> {
>> +        printf("If device %s is mounted on the desktop, unmount it"
>> +                        " first before using it in QEMU.\n", filename);
>> +        printf("Command to unmount device: diskutil unmountDisk %s\n",
>> +                                                                    filename);
>> +        printf("Command to mount device: diskutil mountDisk %s\n", filename);
>> +    }
>> +#endif /* defined(__APPLE__) && defined(__MACH__) */
>> +
>> +    if (ret < 0) {
>> +       return ret;
>>     }
> 
> Why don't you simply include the #ifdef block in the first if (ret < 0)?
> Or does ret > 0 happen and the message must be displayed for it?

ret > 0 could happen and a message should be displayed for it.