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

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

[Programmingkid]

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

Mac OS X can be picky when it comes to allowing the user to use physical devices
in QEMU. This patch fixes that issue by testing each physical device first
before using it in 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 volume unmounting code.
Removed automatic remounting code.
Displays helpful error message in place of remounting code.

 block/raw-posix.c |  115 ++++++++++++++++++++++++++++++++++++++++------------
 1 files changed, 88 insertions(+), 27 deletions(-)

diff --git a/block/raw-posix.c b/block/raw-posix.c
index cbe6574..9de37ea 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,8 +1971,9 @@ 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 );
+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 )
 {
@@ -2001,7 +2001,8 @@ kern_return_t FindEjectableCDMedia( io_iterator_t *mediaIterator )
     return kernResult;
 }
 
-kern_return_t GetBSDPath( io_iterator_t mediaIterator, char *bsdPath, CFIndex maxPathSize )
+kern_return_t GetBSDPath(io_iterator_t mediaIterator, char *bsdPath,
+                         CFIndex maxPathSize, int flags)
 {
     io_object_t     nextMedia;
     kern_return_t   kernResult = KERN_FAILURE;
@@ -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;
@@ -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);
+    }
+
+    /* 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);
+        snprintf(testPartition, MAXPATHLEN, "%ss%d", testPartition, 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");
+        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);
+}
+
+#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);
+            if (mediaIterator) {
+                IOObjectRelease(mediaIterator);
             }
+            setupCDROM(bsdPath);
             filename = bsdPath;
-            qdict_put(options, "filename", qstring_from_str(filename));
         }
 
-        if ( mediaIterator )
-            IOObjectRelease( mediaIterator );
+        /* Setup any other physical device e.g. USB flash drive */
+        else {
+            setupDevice(filename);
+        }
+
+        qdict_put(options, "filename", qstring_from_str(filename));
     }
 #endif
 
-- 
1.7.5.4


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

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

[Stefan Hajnoczi]

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

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

?

> @@ -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?

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.

> +
> +    /* 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.

> +        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.

> +            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.

>  
> -        if ( mediaIterator )
> -            IOObjectRelease( mediaIterator );
> +        /* Setup any other physical device e.g. USB flash drive */
> +        else {
> +            setupDevice(filename);
> +        }
> +
> +        qdict_put(options, "filename", qstring_from_str(filename));
>      }
>  #endif
>  
> -- 
> 1.7.5.4
> 

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

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

[Programmingkid]

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. 

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

[Peter Maydell]

On 17 July 2015 at 20:24, Programmingkid <programmingkidx@gmail.com> wrote:
> Is pstrcpy() ansi c? I'm having trouble finding documentation for it.

No, it's something we provide in util/cutils.c. We recommend it
in HACKING, but we don't actually document the semantics, which
is a bit unhelpful. I've just sent a patch which adds doc comments
for pstrcpy and a handful of other string utility functions to
qemu-common.h:
  http://patchwork.ozlabs.org/patch/497526/

thanks
-- PMM

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

[Stefan Hajnoczi]

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

On Fri, Jul 17, 2015 at 03:24:34PM -0400, Programmingkid wrote:
> 
> 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. 

That means the issue you originally hit hasn't been solved yet.

Take a look at s->needs_alignment and raw_probe_alignment().  In the -drive
cache=none case raw-posix needs to detect the correct alignment (probably 2 KB
for CD-ROMs).

Stefan

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

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

[Laurent Vivier]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1



On 20/07/2015 12:48, Stefan Hajnoczi wrote:
> On Fri, Jul 17, 2015 at 03:24:34PM -0400, Programmingkid wrote:
>> 
>> 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.
> 
> That means the issue you originally hit hasn't been solved yet.
> 
> Take a look at s->needs_alignment and raw_probe_alignment().  In
> the -drive cache=none case raw-posix needs to detect the correct
> alignment (probably 2 KB for CD-ROMs).

As raw_open_common() sets needs_alignment to true on BDRV_O_NOCACHE
(cache="none") and raw_probe_alignment() detects alignment if
needs_alignment is true, I don't understand why it doesn't work.

Could you explain ?

Laurent
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2

iEYEARECAAYFAlWs7bcACgkQNKT2yavzbFMxIwCcCPYXvcSZTnjp7UVQBUVLAj6K
iY0An2l1ttpVEb9bZP+VEakuU75X/Zd7
=S83F
-----END PGP SIGNATURE-----

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

[Programmingkid]

On Jul 20, 2015, at 8:46 AM, Laurent Vivier wrote:

> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
> 
> 
> 
> On 20/07/2015 12:48, Stefan Hajnoczi wrote:
>> On Fri, Jul 17, 2015 at 03:24:34PM -0400, Programmingkid wrote:
>>> 
>>> 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.
>> 
>> That means the issue you originally hit hasn't been solved yet.
>> 
>> Take a look at s->needs_alignment and raw_probe_alignment().  In
>> the -drive cache=none case raw-posix needs to detect the correct
>> alignment (probably 2 KB for CD-ROMs).
> 
> As raw_open_common() sets needs_alignment to true on BDRV_O_NOCACHE
> (cache="none") and raw_probe_alignment() detects alignment if
> needs_alignment is true, I don't understand why it doesn't work.
> 
> Could you explain ?


I just did several tests with real CD-ROM discs and it does work. I first booted up Mac OS 10.2 with Stefan's command options using a professionally made CD, and it worked. I then did the same test again using a burned CD-R disc and it also worked. The last test I did was just listing the files from OpenBIOS using this: qemu-system-ppc -drive if=ide,media=cdrom,cache=none,file=/dev/cdrom. All tests were a success. 

Mac OS 10.2 panicked while booting in the original test using Stefan's command. I remember the panic happened about a minute into the boot process, so it could have been a guest issue rather than a QEMU issue. Either way everything is working now. 

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

[Stefan Hajnoczi]

On Mon, Jul 20, 2015 at 5:17 PM, Programmingkid
<programmingkidx@gmail.com> wrote:
>
> On Jul 20, 2015, at 8:46 AM, Laurent Vivier wrote:
>
>> -----BEGIN PGP SIGNED MESSAGE-----
>> Hash: SHA1
>>
>>
>>
>> On 20/07/2015 12:48, Stefan Hajnoczi wrote:
>>> On Fri, Jul 17, 2015 at 03:24:34PM -0400, Programmingkid wrote:
>>>>
>>>> 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.
>>>
>>> That means the issue you originally hit hasn't been solved yet.
>>>
>>> Take a look at s->needs_alignment and raw_probe_alignment().  In
>>> the -drive cache=none case raw-posix needs to detect the correct
>>> alignment (probably 2 KB for CD-ROMs).
>>
>> As raw_open_common() sets needs_alignment to true on BDRV_O_NOCACHE
>> (cache="none") and raw_probe_alignment() detects alignment if
>> needs_alignment is true, I don't understand why it doesn't work.
>>
>> Could you explain ?
>
>
> I just did several tests with real CD-ROM discs and it does work. I first booted up Mac OS 10.2 with Stefan's command options using a professionally made CD, and it worked. I then did the same test again using a burned CD-R disc and it also worked. The last test I did was just listing the files from OpenBIOS using this: qemu-system-ppc -drive if=ide,media=cdrom,cache=none,file=/dev/cdrom. All tests were a success.
>
> Mac OS 10.2 panicked while booting in the original test using Stefan's command. I remember the panic happened about a minute into the boot process, so it could have been a guest issue rather than a QEMU issue. Either way everything is working now.

I don't see what your patch changed to make -drive
if=ide,media=cdrom,cache=none,file=/dev/cdrom work?

Stefan

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

[Stefan Hajnoczi]

On Fri, Jul 24, 2015 at 3:22 PM, Stefan Hajnoczi <stefanha@gmail.com> wrote:
> On Mon, Jul 20, 2015 at 5:17 PM, Programmingkid
> <programmingkidx@gmail.com> wrote:
>>
>> On Jul 20, 2015, at 8:46 AM, Laurent Vivier wrote:
>>
>>> -----BEGIN PGP SIGNED MESSAGE-----
>>> Hash: SHA1
>>>
>>>
>>>
>>> On 20/07/2015 12:48, Stefan Hajnoczi wrote:
>>>> On Fri, Jul 17, 2015 at 03:24:34PM -0400, Programmingkid wrote:
>>>>>
>>>>> 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.
>>>>
>>>> That means the issue you originally hit hasn't been solved yet.
>>>>
>>>> Take a look at s->needs_alignment and raw_probe_alignment().  In
>>>> the -drive cache=none case raw-posix needs to detect the correct
>>>> alignment (probably 2 KB for CD-ROMs).
>>>
>>> As raw_open_common() sets needs_alignment to true on BDRV_O_NOCACHE
>>> (cache="none") and raw_probe_alignment() detects alignment if
>>> needs_alignment is true, I don't understand why it doesn't work.
>>>
>>> Could you explain ?
>>
>>
>> I just did several tests with real CD-ROM discs and it does work. I first booted up Mac OS 10.2 with Stefan's command options using a professionally made CD, and it worked. I then did the same test again using a burned CD-R disc and it also worked. The last test I did was just listing the files from OpenBIOS using this: qemu-system-ppc -drive if=ide,media=cdrom,cache=none,file=/dev/cdrom. All tests were a success.
>>
>> Mac OS 10.2 panicked while booting in the original test using Stefan's command. I remember the panic happened about a minute into the boot process, so it could have been a guest issue rather than a QEMU issue. Either way everything is working now.
>
> I don't see what your patch changed to make -drive
> if=ide,media=cdrom,cache=none,file=/dev/cdrom work?

Sorry for the confusion, I understand now.

When you said the guest fails, I thought you meant that bdrv_read() is
still failing and the guest won't boot.  But you meant that the guest
kernel paniced (which is fine, not necessarily a CD-ROM problem).

Laurent's comment also clarifies that the BDRV_O_NOCACHE enables
probing, and therefore bdrv_read() will work correctly!

So I'm satisfied with why cache=none works now.

Thanks,
Stefan