Re: [Qemu-block] [PATCH] raw-posix.c: Make physical devices usable in QEMU
On Jul 16, 2015, at 9:19 AM, Stefan Hajnoczi wrote: On Thu, Jul 09, 2015 at 10:02:26AM -0400, Programmingkid wrote: On Jul 9, 2015, at 6:52 AM, Stefan Hajnoczi wrote: On Tue, Jul 07, 2015 at 01:33:23PM -0400, Programmingkid wrote: Make physical devices like a USB flash drive or a CDROM drive work in QEMU. With this patch I can use a USB flash drive like a hard drive. Before this patch, QEMU would just quit with a message like resource busy. The commit message and the description are missing on Mac OS X. It should be clear right away that this applies to Mac only. This works fine on Linux and probably other host OSes. Yeah, that should have been done. Did you see any issues with the code? QEMU shouldn't silently open a different file than the one given by the user. The user should give the exact device file they want. If there is magic behavior it needs to be documented, but I don't see a reason why that's necessary in the case of device files. I think you are reviewing an older patch. The newest one doesn't do that. QEMU shouldn't mount/unmount the CD-ROM. atexit(3) doesn't handle crashes or abort(). Users may be confused to find their CD-ROM unmounted in those cases and would see this as a bug. Instead we should refuse mounted CD-ROMs so the user understands that block-level access requires them to unmount first. That can be done. It just wouldn't be as user friendly as having QEMU do it for the user :( The strcpy/sprintf usage in this patch is unsafe and can lead to buffer overflow, for example in the case of generating command-lines. The command-line buffer is only MAXPATHLEN so prepending the command to the filename could exceed the buffer size. There is also a buffer overflow in the array of devices that need to be mounted. What happens if there are more than 7 devices? Ok. Will correct this issue.
[Qemu-block] [PATCH v2] raw-posix.c: Make physical devices usable in QEMU under Mac OS X host
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 programmingk...@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
Re: [Qemu-block] [PATCH] raw-posix.c: Make physical devices usable in QEMU
On Thu, Jul 16, 2015 at 6:26 PM, Programmingkid programmingk...@gmail.com wrote: On Jul 16, 2015, at 9:19 AM, Stefan Hajnoczi wrote: On Thu, Jul 09, 2015 at 10:02:26AM -0400, Programmingkid wrote: On Jul 9, 2015, at 6:52 AM, Stefan Hajnoczi wrote: On Tue, Jul 07, 2015 at 01:33:23PM -0400, Programmingkid wrote: Make physical devices like a USB flash drive or a CDROM drive work in QEMU. With this patch I can use a USB flash drive like a hard drive. Before this patch, QEMU would just quit with a message like resource busy. The commit message and the description are missing on Mac OS X. It should be clear right away that this applies to Mac only. This works fine on Linux and probably other host OSes. Yeah, that should have been done. Did you see any issues with the code? QEMU shouldn't silently open a different file than the one given by the user. The user should give the exact device file they want. If there is magic behavior it needs to be documented, but I don't see a reason why that's necessary in the case of device files. I think you are reviewing an older patch. The newest one doesn't do that. I don't see a more recent patch on the mailing list. What is the Message-Id of your latest patch email?
Re: [Qemu-block] [PATCH] raw-posix.c: Make physical devices usable in QEMU
On Jul 16, 2015, at 3:43 PM, Stefan Hajnoczi wrote: On Thu, Jul 16, 2015 at 6:26 PM, Programmingkid programmingk...@gmail.com wrote: On Jul 16, 2015, at 9:19 AM, Stefan Hajnoczi wrote: On Thu, Jul 09, 2015 at 10:02:26AM -0400, Programmingkid wrote: On Jul 9, 2015, at 6:52 AM, Stefan Hajnoczi wrote: On Tue, Jul 07, 2015 at 01:33:23PM -0400, Programmingkid wrote: Make physical devices like a USB flash drive or a CDROM drive work in QEMU. With this patch I can use a USB flash drive like a hard drive. Before this patch, QEMU would just quit with a message like resource busy. The commit message and the description are missing on Mac OS X. It should be clear right away that this applies to Mac only. This works fine on Linux and probably other host OSes. Yeah, that should have been done. Did you see any issues with the code? QEMU shouldn't silently open a different file than the one given by the user. The user should give the exact device file they want. If there is magic behavior it needs to be documented, but I don't see a reason why that's necessary in the case of device files. I think you are reviewing an older patch. The newest one doesn't do that. I don't see a more recent patch on the mailing list. What is the Message-Id of your latest patch email? Since I will be uploading a new patch shortly, it doesn't matter anymore.
[Qemu-block] [PATCH] ahci: Force ICC bits in PxCMD to zero
The AHCI spec requires that the HBA sets the ICC bits to zero after the ICC change is done. Since we don't do any ICC change, force the bits to zero all the time. This fixes delays with some OSs (e.g. OpenBSD) waiting for the ICC bits to change to 0. Signed-off-by: Stefan Fritsch s...@sfritsch.de --- hw/ide/ahci.c | 9 +++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c index bb6a92f..48749c1 100644 --- a/hw/ide/ahci.c +++ b/hw/ide/ahci.c @@ -279,8 +279,13 @@ static void ahci_port_write(AHCIState *s, int port, int offset, uint32_t val) break; case PORT_CMD: /* Block any Read-only fields from being set; - * including LIST_ON and FIS_ON. */ -pr-cmd = (pr-cmd PORT_CMD_RO_MASK) | (val ~PORT_CMD_RO_MASK); + * including LIST_ON and FIS_ON. + * The spec requires to set ICC bits to zero after the ICC change + * is done. We don't support ICC state changes, therefore always + * force the ICC bits to zero. + */ +pr-cmd = (pr-cmd PORT_CMD_RO_MASK) | + (val ~(PORT_CMD_RO_MASK|PORT_CMD_ICC_MASK)); /* Check FIS RX and CLB engines, allow transition to false: */ ahci_cond_start_engines(s-dev[port], true); -- 2.1.4
