All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH] ui/cocoa.m: Add Mount image file menu item
@ 2015-09-02  0:56 Programmingkid
  2015-09-02  9:05 ` Markus Armbruster
  2015-09-08 16:17 ` Peter Maydell
  0 siblings, 2 replies; 20+ messages in thread
From: Programmingkid @ 2015-09-02  0:56 UTC (permalink / raw)
  To: Peter Maydell, qemu-devel qemu-devel

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

Add "Mount Image File..." and a "Eject Image File" menu items to
cocoa interface. This patch makes sharing files between the
host and the guest user-friendly.

The "Mount Image File..." menu item displays a dialog box having the
user pick an image file to use in QEMU. The image file is setup as
a USB flash drive. The user can do the equivalent of removing the
flash drive by selecting the file in the "Eject Image File" submenu.

Signed-off-by: John Arbuckle <programmingkidx@gmail.com>

---
 ui/cocoa.m |  212 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 files changed, 210 insertions(+), 1 deletions(-)

diff --git a/ui/cocoa.m b/ui/cocoa.m
index 334e6f6..6c0ec18 100644
--- a/ui/cocoa.m
+++ b/ui/cocoa.m
@@ -52,6 +52,9 @@
 #endif
 
 #define cgrect(nsrect) (*(CGRect *)&(nsrect))
+#define USB_DISK_ID "USB_DISK"
+#define EJECT_IMAGE_FILE_TAG 2099
 
 typedef struct {
     int width;
@@ -263,6 +266,43 @@ static void handleAnyDeviceErrors(Error * err)
     }
 }
 
+/* Sends a command to the monitor console */
+static void sendMonitorCommand(const char * commandString)
+{
+    int index;
+    char * consoleName;
+    static QemuConsole *monitor;
+
+    /* If the monitor console hasn't been found yet */
+    if(!monitor) {
+        index = 0;
+        /* Find the monitor console */
+        while (qemu_console_lookup_by_index(index) != NULL) {
+            consoleName = qemu_console_get_label(qemu_console_lookup_by_index(index));
+            if(strstr(consoleName, "monitor")) {
+                monitor = qemu_console_lookup_by_index(index);
+                break;
+            }
+            index++;
+        }
+    }
+
+    /* If the monitor console was not found */
+    if(!monitor) {
+        NSBeep();
+        QEMU_Alert(@"Failed to find the monitor console!");
+        return;
+    }
+
+    /* send each letter in the commandString to the monitor */
+    for (index = 0; index < strlen(commandString); index++) {
+        kbd_put_keysym_console(monitor, commandString[index]);
+    }
+
+    /* simulate the user pushing the return key */
+    kbd_put_keysym_console(monitor, '\n');
+}
+
 /*
  ------------------------------------------------------
     QemuCocoaView
@@ -585,6 +625,7 @@ QemuCocoaView *cocoaView;
             // forward command key combos to the host UI unless the mouse is grabbed
             if (!isMouseGrabbed && ([event modifierFlags] & NSCommandKeyMask)) {
                 [NSApp sendEvent:event];
+                printf("Returning\n");
                 return;
             }
 
@@ -829,6 +870,9 @@ QemuCocoaView *cocoaView;
 - (void)powerDownQEMU:(id)sender;
 - (void)ejectDeviceMedia:(id)sender;
 - (void)changeDeviceMedia:(id)sender;
+- (void)mountImageFile:(id)sender;
+- (void)ejectImageFile:(id)sender;
+- (void)updateEjectImageMenuItems;
 @end
 
 @implementation QemuCocoaAppController
@@ -1125,6 +1169,150 @@ QemuCocoaView *cocoaView;
     }
 }
 
+/* Displays a dialog box asking the user for an image file to mount */
+- (void)mountImageFile:(id)sender
+{
+    /* Display the file open dialog */
+    NSOpenPanel * openPanel;
+    openPanel = [NSOpenPanel openPanel];
+    [openPanel setCanChooseFiles: YES];
+    [openPanel setAllowsMultipleSelection: NO];
+    [openPanel setAllowedFileTypes: supportedImageFileTypes];
+    if([openPanel runModal] == NSFileHandlingPanelOKButton) {
+        NSString * file = [[[openPanel URLs] objectAtIndex: 0] path];
+        if(file == nil) {
+            NSBeep();
+            QEMU_Alert(@"Failed to convert URL to file path!");
+            return;
+        }
+
+        static int usbDiskCount;  // used for the ID
+        char *commandBuffer, *fileName, *idString, *fileNameHint;
+        NSString *buffer;
+        const int fileNameHintSize = 10;
+
+        fileName = g_strdup_printf("%s",
+                            [file cStringUsingEncoding: NSASCIIStringEncoding]);
+        buffer = [file lastPathComponent];
+        buffer = [buffer stringByDeletingPathExtension];
+        if([buffer length] > fileNameHintSize) {
+            buffer = [buffer substringToIndex: fileNameHintSize];
+        }
+        fileNameHint = g_strdup_printf("%s",
+                        [buffer cStringUsingEncoding: NSASCIIStringEncoding]);
+        idString = g_strdup_printf("%s_%s_%d", USB_DISK_ID, fileNameHint, usbDiskCount);
+        commandBuffer = g_strdup_printf("drive_add 0 if=none,id=%s,file=%s",
+                                                            idString, fileName);
+        sendMonitorCommand(commandBuffer);
+        commandBuffer = g_strdup_printf("device_add usb-storage,"
+                                         "id=%s,drive=%s", idString, idString);
+        sendMonitorCommand(commandBuffer);
+        [self updateEjectImageMenuItems];
+        usbDiskCount++;
+        g_free(fileName);
+        g_free(fileNameHint);
+        g_free(idString);
+        g_free(commandBuffer);
+    }
+}
+
+/* Removes an image file from QEMU */
+- (void)ejectImageFile:(id) sender
+{
+    char *commandBuffer;
+    NSString *imageFileID;
+
+    imageFileID = [sender representedObject];
+    if (imageFileID == nil) {
+        NSBeep();
+        QEMU_Alert(@"Could not find image file's ID!");
+        return;
+    }
+
+    commandBuffer = g_strdup_printf("drive_del %s",
+                    [imageFileID cStringUsingEncoding: NSASCIIStringEncoding]);
+    sendMonitorCommand(commandBuffer);
+    g_free(commandBuffer);
+
+    commandBuffer = g_strdup_printf("device_del %s",
+                        [imageFileID cStringUsingEncoding: NSASCIIStringEncoding]);
+    sendMonitorCommand(commandBuffer);
+    g_free(commandBuffer);
+
+    [self updateEjectImageMenuItems];
+}
+
+/* Gives each mounted image file an eject menu item */
+- (void) updateEjectImageMenuItems
+{
+    NSMenu *machineMenu;
+    machineMenu = [[[NSApp mainMenu] itemWithTitle:@"Machine"] submenu];
+
+    /* Remove old menu items*/
+    NSMenu * ejectSubmenu;
+    ejectSubmenu = [[machineMenu itemWithTag: EJECT_IMAGE_FILE_TAG] submenu];
+    if(!ejectSubmenu) {
+        NSBeep();
+        QEMU_Alert(@"Failed to find eject submenu!");
+        return;
+    }
+    int index;
+    for (index = 0; index < [ejectSubmenu numberOfItems]; index++) {
+        [ejectSubmenu removeItemAtIndex: 0];
+    }
+     /* Needed probably because of a bug with cocoa */
+    if ([ejectSubmenu numberOfItems] > 0) {
+        [ejectSubmenu removeItemAtIndex: 0];
+    }
+
+    BlockInfoList *currentDevice;
+    currentDevice = qmp_query_block(NULL);
+
+    NSString *fileName, *deviceName;
+    NSMenuItem *ejectFileMenuItem;  /* Used with each mounted image file */
+
+    /* Look for mounted image files */
+    while(currentDevice) {
+        if (!currentDevice->value || !currentDevice->value->inserted
+                                  || !currentDevice->value->inserted->file) {
+            currentDevice = currentDevice->next;
+            continue;
+        }
+
+        /* if the device's name is the generated ID */
+        if (!strstr(currentDevice->value->device, USB_DISK_ID)) {
+            currentDevice = currentDevice->next;
+            continue;
+        }
+
+        fileName = [NSString stringWithFormat: @"%s", currentDevice->value->inserted->file];
+        fileName = [fileName lastPathComponent]; /* To obtain only the file name */
+
+        ejectFileMenuItem = [[NSMenuItem alloc] initWithTitle: [NSString stringWithFormat: @"Eject %@", fileName]
+                                                  action: @selector(ejectImageFile:)
+                                           keyEquivalent: @""];
+        [ejectSubmenu addItem: ejectFileMenuItem];
+        deviceName = [NSString stringWithFormat: @"%s", currentDevice->value->device];
+        [ejectFileMenuItem setRepresentedObject: deviceName];
+        [ejectFileMenuItem autorelease];
+        currentDevice = currentDevice->next;
+    }
+
+    /* Add default menu item if submenu is empty */
+    if([ejectSubmenu numberOfItems] == 0) {
+
+        /* Create the default menu item */
+        NSMenuItem *emptyMenuItem;
+        emptyMenuItem = [NSMenuItem new];
+        [emptyMenuItem setTitle: @"No items available"];
+        [emptyMenuItem setEnabled: NO];
+
+        /* Add the default menu item to the submenu */
+        [ejectSubmenu addItem: emptyMenuItem];
+        [emptyMenuItem release];
+    }
+}
+
 @end
 
 
@@ -1383,7 +1571,6 @@ static void addRemovableDevicesMenuItems()
     /* Loop thru all the block devices in the emulator */
     while (currentDevice) {
         deviceName = [[NSString stringWithFormat: @"%s", currentDevice->value->device] retain];
-
         if(currentDevice->value->removable) {
             menuItem = [[NSMenuItem alloc] initWithTitle: [NSString stringWithFormat: @"Change %s...", currentDevice->value->device]
                                                   action: @selector(changeDeviceMedia:)
@@ -1402,6 +1589,29 @@ static void addRemovableDevicesMenuItems()
         currentDevice = currentDevice->next;
     }
     qapi_free_BlockInfoList(pointerToFree);
+    [menu addItem: [[[NSMenuItem alloc] initWithTitle: @"Mount Image File..." action: @selector(mountImageFile:) keyEquivalent: @""] autorelease]];
+
+    /* Create the eject menu item */
+    NSMenuItem *ejectMenuItem;
+    ejectMenuItem = [NSMenuItem new];
+    [ejectMenuItem setTitle: @"Eject Image File"];
+    [ejectMenuItem setTag: EJECT_IMAGE_FILE_TAG];
+    [menu addItem: ejectMenuItem];
+    [ejectMenuItem autorelease];
+
+    /* Create the default menu item for the eject menu item's submenu*/
+    NSMenuItem *emptyMenuItem;
+    emptyMenuItem = [NSMenuItem new];
+    [emptyMenuItem setTitle: @"No items available"];
+    [emptyMenuItem setEnabled: NO];
+    [emptyMenuItem autorelease];
+
+    /* Add the default menu item to the submenu */
+    NSMenu *submenu;
+    submenu = [NSMenu new];
+    [ejectMenuItem setSubmenu: submenu];
+    [submenu addItem: emptyMenuItem];
+    [submenu autorelease];
 }
 
 void cocoa_display_init(DisplayState *ds, int full_screen)
-- 
1.7.5.4


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

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* Re: [Qemu-devel] [PATCH] ui/cocoa.m: Add Mount image file menu item
  2015-09-02  0:56 [Qemu-devel] [PATCH] ui/cocoa.m: Add Mount image file menu item Programmingkid
@ 2015-09-02  9:05 ` Markus Armbruster
  2015-09-08 16:17 ` Peter Maydell
  1 sibling, 0 replies; 20+ messages in thread
From: Markus Armbruster @ 2015-09-02  9:05 UTC (permalink / raw)
  To: Programmingkid
  Cc: Peter Maydell, Andreas Färber, qemu-devel qemu-devel, Gerd Hoffmann

Cc'ing the maintainers you forgot.  scripts/get_maintainer.pl is your
friend.

Programmingkid <programmingkidx@gmail.com> writes:

> Add "Mount Image File..." and a "Eject Image File" menu items to
> cocoa interface. This patch makes sharing files between the
> host and the guest user-friendly.
>
> The "Mount Image File..." menu item displays a dialog box having the
> user pick an image file to use in QEMU. The image file is setup as
> a USB flash drive. The user can do the equivalent of removing the
> flash drive by selecting the file in the "Eject Image File" submenu.
>
> Signed-off-by: John Arbuckle <programmingkidx@gmail.com>
>
> ---
>  ui/cocoa.m |  212 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
>  1 files changed, 210 insertions(+), 1 deletions(-)
>
> diff --git a/ui/cocoa.m b/ui/cocoa.m
> index 334e6f6..6c0ec18 100644
> --- a/ui/cocoa.m
> +++ b/ui/cocoa.m
> @@ -52,6 +52,9 @@
>  #endif
>  
>  #define cgrect(nsrect) (*(CGRect *)&(nsrect))
> +#define USB_DISK_ID "USB_DISK"
> +#define EJECT_IMAGE_FILE_TAG 2099
>  
>  typedef struct {
>      int width;
> @@ -263,6 +266,43 @@ static void handleAnyDeviceErrors(Error * err)
>      }
>  }
>  
> +/* Sends a command to the monitor console */
> +static void sendMonitorCommand(const char * commandString)
> +{
> +    int index;
> +    char * consoleName;
> +    static QemuConsole *monitor;
> +
> +    /* If the monitor console hasn't been found yet */
> +    if(!monitor) {
> +        index = 0;
> +        /* Find the monitor console */
> +        while (qemu_console_lookup_by_index(index) != NULL) {
> +            consoleName = qemu_console_get_label(qemu_console_lookup_by_index(index));
> +            if(strstr(consoleName, "monitor")) {
> +                monitor = qemu_console_lookup_by_index(index);
> +                break;
> +            }
> +            index++;
> +        }
> +    }
> +
> +    /* If the monitor console was not found */
> +    if(!monitor) {
> +        NSBeep();
> +        QEMU_Alert(@"Failed to find the monitor console!");
> +        return;
> +    }
> +
> +    /* send each letter in the commandString to the monitor */
> +    for (index = 0; index < strlen(commandString); index++) {
> +        kbd_put_keysym_console(monitor, commandString[index]);
> +    }
> +
> +    /* simulate the user pushing the return key */
> +    kbd_put_keysym_console(monitor, '\n');
> +}
> +
>  /*
>   ------------------------------------------------------
>      QemuCocoaView
> @@ -585,6 +625,7 @@ QemuCocoaView *cocoaView;
>              // forward command key combos to the host UI unless the mouse is grabbed
>              if (!isMouseGrabbed && ([event modifierFlags] & NSCommandKeyMask)) {
>                  [NSApp sendEvent:event];
> +                printf("Returning\n");
>                  return;
>              }
>  
> @@ -829,6 +870,9 @@ QemuCocoaView *cocoaView;
>  - (void)powerDownQEMU:(id)sender;
>  - (void)ejectDeviceMedia:(id)sender;
>  - (void)changeDeviceMedia:(id)sender;
> +- (void)mountImageFile:(id)sender;
> +- (void)ejectImageFile:(id)sender;
> +- (void)updateEjectImageMenuItems;
>  @end
>  
>  @implementation QemuCocoaAppController
> @@ -1125,6 +1169,150 @@ QemuCocoaView *cocoaView;
>      }
>  }
>  
> +/* Displays a dialog box asking the user for an image file to mount */
> +- (void)mountImageFile:(id)sender
> +{
> +    /* Display the file open dialog */
> +    NSOpenPanel * openPanel;
> +    openPanel = [NSOpenPanel openPanel];
> +    [openPanel setCanChooseFiles: YES];
> +    [openPanel setAllowsMultipleSelection: NO];
> +    [openPanel setAllowedFileTypes: supportedImageFileTypes];
> +    if([openPanel runModal] == NSFileHandlingPanelOKButton) {
> +        NSString * file = [[[openPanel URLs] objectAtIndex: 0] path];
> +        if(file == nil) {
> +            NSBeep();
> +            QEMU_Alert(@"Failed to convert URL to file path!");
> +            return;
> +        }
> +
> +        static int usbDiskCount;  // used for the ID
> +        char *commandBuffer, *fileName, *idString, *fileNameHint;
> +        NSString *buffer;
> +        const int fileNameHintSize = 10;
> +
> +        fileName = g_strdup_printf("%s",
> +                            [file cStringUsingEncoding: NSASCIIStringEncoding]);
> +        buffer = [file lastPathComponent];
> +        buffer = [buffer stringByDeletingPathExtension];
> +        if([buffer length] > fileNameHintSize) {
> +            buffer = [buffer substringToIndex: fileNameHintSize];
> +        }
> +        fileNameHint = g_strdup_printf("%s",
> +                        [buffer cStringUsingEncoding: NSASCIIStringEncoding]);
> +        idString = g_strdup_printf("%s_%s_%d", USB_DISK_ID, fileNameHint, usbDiskCount);
> +        commandBuffer = g_strdup_printf("drive_add 0 if=none,id=%s,file=%s",
> +                                                            idString, fileName);
> +        sendMonitorCommand(commandBuffer);
> +        commandBuffer = g_strdup_printf("device_add usb-storage,"
> +                                         "id=%s,drive=%s", idString, idString);
> +        sendMonitorCommand(commandBuffer);
> +        [self updateEjectImageMenuItems];
> +        usbDiskCount++;
> +        g_free(fileName);
> +        g_free(fileNameHint);
> +        g_free(idString);
> +        g_free(commandBuffer);
> +    }
> +}
> +
> +/* Removes an image file from QEMU */
> +- (void)ejectImageFile:(id) sender
> +{
> +    char *commandBuffer;
> +    NSString *imageFileID;
> +
> +    imageFileID = [sender representedObject];
> +    if (imageFileID == nil) {
> +        NSBeep();
> +        QEMU_Alert(@"Could not find image file's ID!");
> +        return;
> +    }
> +
> +    commandBuffer = g_strdup_printf("drive_del %s",
> +                    [imageFileID cStringUsingEncoding: NSASCIIStringEncoding]);
> +    sendMonitorCommand(commandBuffer);
> +    g_free(commandBuffer);
> +
> +    commandBuffer = g_strdup_printf("device_del %s",
> +                        [imageFileID cStringUsingEncoding: NSASCIIStringEncoding]);
> +    sendMonitorCommand(commandBuffer);
> +    g_free(commandBuffer);
> +
> +    [self updateEjectImageMenuItems];
> +}
> +
> +/* Gives each mounted image file an eject menu item */
> +- (void) updateEjectImageMenuItems
> +{
> +    NSMenu *machineMenu;
> +    machineMenu = [[[NSApp mainMenu] itemWithTitle:@"Machine"] submenu];
> +
> +    /* Remove old menu items*/
> +    NSMenu * ejectSubmenu;
> +    ejectSubmenu = [[machineMenu itemWithTag: EJECT_IMAGE_FILE_TAG] submenu];
> +    if(!ejectSubmenu) {
> +        NSBeep();
> +        QEMU_Alert(@"Failed to find eject submenu!");
> +        return;
> +    }
> +    int index;
> +    for (index = 0; index < [ejectSubmenu numberOfItems]; index++) {
> +        [ejectSubmenu removeItemAtIndex: 0];
> +    }
> +     /* Needed probably because of a bug with cocoa */
> +    if ([ejectSubmenu numberOfItems] > 0) {
> +        [ejectSubmenu removeItemAtIndex: 0];
> +    }
> +
> +    BlockInfoList *currentDevice;
> +    currentDevice = qmp_query_block(NULL);
> +
> +    NSString *fileName, *deviceName;
> +    NSMenuItem *ejectFileMenuItem;  /* Used with each mounted image file */
> +
> +    /* Look for mounted image files */
> +    while(currentDevice) {
> +        if (!currentDevice->value || !currentDevice->value->inserted
> +                                  || !currentDevice->value->inserted->file) {
> +            currentDevice = currentDevice->next;
> +            continue;
> +        }
> +
> +        /* if the device's name is the generated ID */
> +        if (!strstr(currentDevice->value->device, USB_DISK_ID)) {
> +            currentDevice = currentDevice->next;
> +            continue;
> +        }
> +
> +        fileName = [NSString stringWithFormat: @"%s", currentDevice->value->inserted->file];
> +        fileName = [fileName lastPathComponent]; /* To obtain only the file name */
> +
> +        ejectFileMenuItem = [[NSMenuItem alloc] initWithTitle: [NSString stringWithFormat: @"Eject %@", fileName]
> +                                                  action: @selector(ejectImageFile:)
> +                                           keyEquivalent: @""];
> +        [ejectSubmenu addItem: ejectFileMenuItem];
> +        deviceName = [NSString stringWithFormat: @"%s", currentDevice->value->device];
> +        [ejectFileMenuItem setRepresentedObject: deviceName];
> +        [ejectFileMenuItem autorelease];
> +        currentDevice = currentDevice->next;
> +    }
> +
> +    /* Add default menu item if submenu is empty */
> +    if([ejectSubmenu numberOfItems] == 0) {
> +
> +        /* Create the default menu item */
> +        NSMenuItem *emptyMenuItem;
> +        emptyMenuItem = [NSMenuItem new];
> +        [emptyMenuItem setTitle: @"No items available"];
> +        [emptyMenuItem setEnabled: NO];
> +
> +        /* Add the default menu item to the submenu */
> +        [ejectSubmenu addItem: emptyMenuItem];
> +        [emptyMenuItem release];
> +    }
> +}
> +
>  @end
>  
>  
> @@ -1383,7 +1571,6 @@ static void addRemovableDevicesMenuItems()
>      /* Loop thru all the block devices in the emulator */
>      while (currentDevice) {
>          deviceName = [[NSString stringWithFormat: @"%s", currentDevice->value->device] retain];
> -
>          if(currentDevice->value->removable) {
>              menuItem = [[NSMenuItem alloc] initWithTitle: [NSString stringWithFormat: @"Change %s...", currentDevice->value->device]
>                                                    action: @selector(changeDeviceMedia:)
> @@ -1402,6 +1589,29 @@ static void addRemovableDevicesMenuItems()
>          currentDevice = currentDevice->next;
>      }
>      qapi_free_BlockInfoList(pointerToFree);
> +    [menu addItem: [[[NSMenuItem alloc] initWithTitle: @"Mount Image File..." action: @selector(mountImageFile:) keyEquivalent: @""] autorelease]];
> +
> +    /* Create the eject menu item */
> +    NSMenuItem *ejectMenuItem;
> +    ejectMenuItem = [NSMenuItem new];
> +    [ejectMenuItem setTitle: @"Eject Image File"];
> +    [ejectMenuItem setTag: EJECT_IMAGE_FILE_TAG];
> +    [menu addItem: ejectMenuItem];
> +    [ejectMenuItem autorelease];
> +
> +    /* Create the default menu item for the eject menu item's submenu*/
> +    NSMenuItem *emptyMenuItem;
> +    emptyMenuItem = [NSMenuItem new];
> +    [emptyMenuItem setTitle: @"No items available"];
> +    [emptyMenuItem setEnabled: NO];
> +    [emptyMenuItem autorelease];
> +
> +    /* Add the default menu item to the submenu */
> +    NSMenu *submenu;
> +    submenu = [NSMenu new];
> +    [ejectMenuItem setSubmenu: submenu];
> +    [submenu addItem: emptyMenuItem];
> +    [submenu autorelease];
>  }
>  
>  void cocoa_display_init(DisplayState *ds, int full_screen)

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [Qemu-devel] [PATCH] ui/cocoa.m: Add Mount image file menu item
  2015-09-02  0:56 [Qemu-devel] [PATCH] ui/cocoa.m: Add Mount image file menu item Programmingkid
  2015-09-02  9:05 ` Markus Armbruster
@ 2015-09-08 16:17 ` Peter Maydell
  2015-09-08 16:39   ` Programmingkid
  1 sibling, 1 reply; 20+ messages in thread
From: Peter Maydell @ 2015-09-08 16:17 UTC (permalink / raw)
  To: Programmingkid; +Cc: qemu-devel qemu-devel

On 2 September 2015 at 01:56, Programmingkid <programmingkidx@gmail.com> wrote:
> Add "Mount Image File..." and a "Eject Image File" menu items to
> cocoa interface. This patch makes sharing files between the
> host and the guest user-friendly.
>
> The "Mount Image File..." menu item displays a dialog box having the
> user pick an image file to use in QEMU. The image file is setup as
> a USB flash drive. The user can do the equivalent of removing the
> flash drive by selecting the file in the "Eject Image File" submenu.
>
> Signed-off-by: John Arbuckle <programmingkidx@gmail.com>
>
> ---
>  ui/cocoa.m |  212
> +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
>  1 files changed, 210 insertions(+), 1 deletions(-)
>
> diff --git a/ui/cocoa.m b/ui/cocoa.m
> index 334e6f6..6c0ec18 100644
> --- a/ui/cocoa.m
> +++ b/ui/cocoa.m
> @@ -52,6 +52,9 @@
>  #endif
>
>
>
>  #define cgrect(nsrect) (*(CGRect *)&(nsrect))
> +#define USB_DISK_ID "USB_DISK"
> +#define EJECT_IMAGE_FILE_TAG 2099
>
>
>
>  typedef struct {
>      int width;
> @@ -263,6 +266,43 @@ static void handleAnyDeviceErrors(Error * err)
>      }
>  }
>
>
>
> +/* Sends a command to the monitor console */
> +static void sendMonitorCommand(const char * commandString)
> +{
> +    int index;
> +    char * consoleName;
> +    static QemuConsole *monitor;
> +
> +    /* If the monitor console hasn't been found yet */
> +    if(!monitor) {
> +        index = 0;
> +        /* Find the monitor console */
> +        while (qemu_console_lookup_by_index(index) != NULL) {
> +            consoleName =
> qemu_console_get_label(qemu_console_lookup_by_index(index));
> +            if(strstr(consoleName, "monitor")) {
> +                monitor = qemu_console_lookup_by_index(index);
> +                break;
> +            }
> +            index++;
> +        }
> +    }
> +
> +    /* If the monitor console was not found */
> +    if(!monitor) {
> +        NSBeep();
> +        QEMU_Alert(@"Failed to find the monitor console!");
> +        return;
> +    }
> +
> +    /* send each letter in the commandString to the monitor */
> +    for (index = 0; index < strlen(commandString); index++) {
> +        kbd_put_keysym_console(monitor, commandString[index]);
> +    }

We're doing this by sending a string to the human monitor?
That definitely doesn't seem like the right way to do this
(and there might not even be a human monitor to talk to)...

What happens on machines with no USB bus?

thanks
-- PMM

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [Qemu-devel] [PATCH] ui/cocoa.m: Add Mount image file menu item
  2015-09-08 16:17 ` Peter Maydell
@ 2015-09-08 16:39   ` Programmingkid
  2015-09-08 16:54     ` Peter Maydell
  2015-09-08 18:46     ` Markus Armbruster
  0 siblings, 2 replies; 20+ messages in thread
From: Programmingkid @ 2015-09-08 16:39 UTC (permalink / raw)
  To: Peter Maydell; +Cc: qemu-devel qemu-devel


On Sep 8, 2015, at 12:17 PM, Peter Maydell wrote:

> On 2 September 2015 at 01:56, Programmingkid <programmingkidx@gmail.com> wrote:
>> Add "Mount Image File..." and a "Eject Image File" menu items to
>> cocoa interface. This patch makes sharing files between the
>> host and the guest user-friendly.
>> 
>> The "Mount Image File..." menu item displays a dialog box having the
>> user pick an image file to use in QEMU. The image file is setup as
>> a USB flash drive. The user can do the equivalent of removing the
>> flash drive by selecting the file in the "Eject Image File" submenu.
>> 
>> Signed-off-by: John Arbuckle <programmingkidx@gmail.com>
>> 
>> ---
>> ui/cocoa.m |  212
>> +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
>> 1 files changed, 210 insertions(+), 1 deletions(-)
>> 
>> diff --git a/ui/cocoa.m b/ui/cocoa.m
>> index 334e6f6..6c0ec18 100644
>> --- a/ui/cocoa.m
>> +++ b/ui/cocoa.m
>> @@ -52,6 +52,9 @@
>> #endif
>> 
>> 
>> 
>> #define cgrect(nsrect) (*(CGRect *)&(nsrect))
>> +#define USB_DISK_ID "USB_DISK"
>> +#define EJECT_IMAGE_FILE_TAG 2099
>> 
>> 
>> 
>> typedef struct {
>>     int width;
>> @@ -263,6 +266,43 @@ static void handleAnyDeviceErrors(Error * err)
>>     }
>> }
>> 
>> 
>> 
>> +/* Sends a command to the monitor console */
>> +static void sendMonitorCommand(const char * commandString)
>> +{
>> +    int index;
>> +    char * consoleName;
>> +    static QemuConsole *monitor;
>> +
>> +    /* If the monitor console hasn't been found yet */
>> +    if(!monitor) {
>> +        index = 0;
>> +        /* Find the monitor console */
>> +        while (qemu_console_lookup_by_index(index) != NULL) {
>> +            consoleName =
>> qemu_console_get_label(qemu_console_lookup_by_index(index));
>> +            if(strstr(consoleName, "monitor")) {
>> +                monitor = qemu_console_lookup_by_index(index);
>> +                break;
>> +            }
>> +            index++;
>> +        }
>> +    }
>> +
>> +    /* If the monitor console was not found */
>> +    if(!monitor) {
>> +        NSBeep();
>> +        QEMU_Alert(@"Failed to find the monitor console!");
>> +        return;
>> +    }
>> +
>> +    /* send each letter in the commandString to the monitor */
>> +    for (index = 0; index < strlen(commandString); index++) {
>> +        kbd_put_keysym_console(monitor, commandString[index]);
>> +    }
> 
> We're doing this by sending a string to the human monitor?
> That definitely doesn't seem like the right way to do this
> (and there might not even be a human monitor to talk to)...

Under what situation is the human monitor not available? 

Would you know what function I should use in place of the commands the patch uses?

> 
> What happens on machines with no USB bus?

I will add code that checks for USB support before using this feature. 

Thanks for reviewing my patch.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [Qemu-devel] [PATCH] ui/cocoa.m: Add Mount image file menu item
  2015-09-08 16:39   ` Programmingkid
@ 2015-09-08 16:54     ` Peter Maydell
  2015-09-08 18:46     ` Markus Armbruster
  1 sibling, 0 replies; 20+ messages in thread
From: Peter Maydell @ 2015-09-08 16:54 UTC (permalink / raw)
  To: Programmingkid; +Cc: qemu-devel qemu-devel

On 8 September 2015 at 17:39, Programmingkid <programmingkidx@gmail.com> wrote:
> Under what situation is the human monitor not available?

It might be connected to a TCP port, or to stdio, rather than to
a QEMU console, for instance. If you're running a -nodefaults
command line then there won't be a monitor at all unless you've
specifically asked for it.

thanks
-- PMM

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [Qemu-devel] [PATCH] ui/cocoa.m: Add Mount image file menu item
  2015-09-08 16:39   ` Programmingkid
  2015-09-08 16:54     ` Peter Maydell
@ 2015-09-08 18:46     ` Markus Armbruster
  2015-09-09  3:02       ` Programmingkid
  1 sibling, 1 reply; 20+ messages in thread
From: Markus Armbruster @ 2015-09-08 18:46 UTC (permalink / raw)
  To: Programmingkid; +Cc: Peter Maydell, qemu-devel qemu-devel

Programmingkid <programmingkidx@gmail.com> writes:

> On Sep 8, 2015, at 12:17 PM, Peter Maydell wrote:
>
>> On 2 September 2015 at 01:56, Programmingkid
>> <programmingkidx@gmail.com> wrote:
>>> Add "Mount Image File..." and a "Eject Image File" menu items to
>>> cocoa interface. This patch makes sharing files between the
>>> host and the guest user-friendly.
>>> 
>>> The "Mount Image File..." menu item displays a dialog box having the
>>> user pick an image file to use in QEMU. The image file is setup as
>>> a USB flash drive. The user can do the equivalent of removing the
>>> flash drive by selecting the file in the "Eject Image File" submenu.
>>> 
>>> Signed-off-by: John Arbuckle <programmingkidx@gmail.com>
>>> 
>>> ---
>>> ui/cocoa.m |  212
>>> +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
>>> 1 files changed, 210 insertions(+), 1 deletions(-)
>>> 
>>> diff --git a/ui/cocoa.m b/ui/cocoa.m
>>> index 334e6f6..6c0ec18 100644
>>> --- a/ui/cocoa.m
>>> +++ b/ui/cocoa.m
>>> @@ -52,6 +52,9 @@
>>> #endif
>>> 
>>> 
>>> 
>>> #define cgrect(nsrect) (*(CGRect *)&(nsrect))
>>> +#define USB_DISK_ID "USB_DISK"
>>> +#define EJECT_IMAGE_FILE_TAG 2099
>>> 
>>> 
>>> 
>>> typedef struct {
>>>     int width;
>>> @@ -263,6 +266,43 @@ static void handleAnyDeviceErrors(Error * err)
>>>     }
>>> }
>>> 
>>> 
>>> 
>>> +/* Sends a command to the monitor console */
>>> +static void sendMonitorCommand(const char * commandString)
>>> +{
>>> +    int index;
>>> +    char * consoleName;
>>> +    static QemuConsole *monitor;
>>> +
>>> +    /* If the monitor console hasn't been found yet */
>>> +    if(!monitor) {
>>> +        index = 0;
>>> +        /* Find the monitor console */
>>> +        while (qemu_console_lookup_by_index(index) != NULL) {
>>> +            consoleName =
>>> qemu_console_get_label(qemu_console_lookup_by_index(index));
>>> +            if(strstr(consoleName, "monitor")) {
>>> +                monitor = qemu_console_lookup_by_index(index);
>>> +                break;
>>> +            }
>>> +            index++;
>>> +        }
>>> +    }
>>> +
>>> +    /* If the monitor console was not found */
>>> +    if(!monitor) {
>>> +        NSBeep();
>>> +        QEMU_Alert(@"Failed to find the monitor console!");
>>> +        return;
>>> +    }
>>> +
>>> +    /* send each letter in the commandString to the monitor */
>>> +    for (index = 0; index < strlen(commandString); index++) {
>>> +        kbd_put_keysym_console(monitor, commandString[index]);
>>> +    }
>> 
>> We're doing this by sending a string to the human monitor?

No way :)

You should not send a string to a monitor (QMP or HMP) just because you
can't be bothered to look up the proper C interfaces.

>> That definitely doesn't seem like the right way to do this
>> (and there might not even be a human monitor to talk to)...
>
> Under what situation is the human monitor not available? 
>
> Would you know what function I should use in place of the commands the
> patch uses?

I explained that already for QMP:
http://lists.gnu.org/archive/html/qemu-devel/2015-09/msg00008.html

The mapping from HMP to the C interfaces can be more complex.  Going
from QMP to C is easier.

[...]

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [Qemu-devel] [PATCH] ui/cocoa.m: Add Mount image file menu item
  2015-09-08 18:46     ` Markus Armbruster
@ 2015-09-09  3:02       ` Programmingkid
  2015-09-09  7:29         ` Markus Armbruster
  0 siblings, 1 reply; 20+ messages in thread
From: Programmingkid @ 2015-09-09  3:02 UTC (permalink / raw)
  To: Markus Armbruster; +Cc: Peter Maydell, qemu-devel qemu-devel


On Sep 8, 2015, at 2:46 PM, Markus Armbruster wrote:

> Programmingkid <programmingkidx@gmail.com> writes:
> 
>> On Sep 8, 2015, at 12:17 PM, Peter Maydell wrote:
>> 
>>> On 2 September 2015 at 01:56, Programmingkid
>>> <programmingkidx@gmail.com> wrote:
>>>> Add "Mount Image File..." and a "Eject Image File" menu items to
>>>> cocoa interface. This patch makes sharing files between the
>>>> host and the guest user-friendly.
>>>> 
>>>> The "Mount Image File..." menu item displays a dialog box having the
>>>> user pick an image file to use in QEMU. The image file is setup as
>>>> a USB flash drive. The user can do the equivalent of removing the
>>>> flash drive by selecting the file in the "Eject Image File" submenu.
>>>> 
>>>> Signed-off-by: John Arbuckle <programmingkidx@gmail.com>
>>>> 
>>>> ---
>>>> ui/cocoa.m |  212
>>>> +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
>>>> 1 files changed, 210 insertions(+), 1 deletions(-)
>>>> 
>>>> diff --git a/ui/cocoa.m b/ui/cocoa.m
>>>> index 334e6f6..6c0ec18 100644
>>>> --- a/ui/cocoa.m
>>>> +++ b/ui/cocoa.m
>>>> @@ -52,6 +52,9 @@
>>>> #endif
>>>> 
>>>> 
>>>> 
>>>> #define cgrect(nsrect) (*(CGRect *)&(nsrect))
>>>> +#define USB_DISK_ID "USB_DISK"
>>>> +#define EJECT_IMAGE_FILE_TAG 2099
>>>> 
>>>> 
>>>> 
>>>> typedef struct {
>>>>    int width;
>>>> @@ -263,6 +266,43 @@ static void handleAnyDeviceErrors(Error * err)
>>>>    }
>>>> }
>>>> 
>>>> 
>>>> 
>>>> +/* Sends a command to the monitor console */
>>>> +static void sendMonitorCommand(const char * commandString)
>>>> +{
>>>> +    int index;
>>>> +    char * consoleName;
>>>> +    static QemuConsole *monitor;
>>>> +
>>>> +    /* If the monitor console hasn't been found yet */
>>>> +    if(!monitor) {
>>>> +        index = 0;
>>>> +        /* Find the monitor console */
>>>> +        while (qemu_console_lookup_by_index(index) != NULL) {
>>>> +            consoleName =
>>>> qemu_console_get_label(qemu_console_lookup_by_index(index));
>>>> +            if(strstr(consoleName, "monitor")) {
>>>> +                monitor = qemu_console_lookup_by_index(index);
>>>> +                break;
>>>> +            }
>>>> +            index++;
>>>> +        }
>>>> +    }
>>>> +
>>>> +    /* If the monitor console was not found */
>>>> +    if(!monitor) {
>>>> +        NSBeep();
>>>> +        QEMU_Alert(@"Failed to find the monitor console!");
>>>> +        return;
>>>> +    }
>>>> +
>>>> +    /* send each letter in the commandString to the monitor */
>>>> +    for (index = 0; index < strlen(commandString); index++) {
>>>> +        kbd_put_keysym_console(monitor, commandString[index]);
>>>> +    }
>>> 
>>> We're doing this by sending a string to the human monitor?
> 
> No way :)
> 
> You should not send a string to a monitor (QMP or HMP) just because you
> can't be bothered to look up the proper C interfaces.

I wouldn't say it like that. I did try to find the functions I needed, but didn't succeed. 
Didn't you say yourself you don't want to see gui patches that change other code?
My research indicated there would have to be changes to other files if I did try to use
the C interface functions.  

For example, the function add_init_drive() in device-hotplug.c looked really good. The
problem was that it is a static function. Since you don't want changes made to other
files, I decided not to use it. 

> 
>>> That definitely doesn't seem like the right way to do this
>>> (and there might not even be a human monitor to talk to)...
>> 
>> Under what situation is the human monitor not available? 
>> 
>> Would you know what function I should use in place of the commands the
>> patch uses?
> 
> I explained that already for QMP:
> http://lists.gnu.org/archive/html/qemu-devel/2015-09/msg00008.html
> 
> The mapping from HMP to the C interfaces can be more complex.  Going
> from QMP to C is easier.

The problem with QMP is that it is so difficult to use. It could be made to be more
user-friendly. I will try to use it anyways in my next patch. Thank you.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [Qemu-devel] [PATCH] ui/cocoa.m: Add Mount image file menu item
  2015-09-09  3:02       ` Programmingkid
@ 2015-09-09  7:29         ` Markus Armbruster
  2015-09-09 21:37           ` Programmingkid
  2015-09-10  3:28           ` Programmingkid
  0 siblings, 2 replies; 20+ messages in thread
From: Markus Armbruster @ 2015-09-09  7:29 UTC (permalink / raw)
  To: Programmingkid; +Cc: Peter Maydell, qemu-devel qemu-devel

Programmingkid <programmingkidx@gmail.com> writes:

> On Sep 8, 2015, at 2:46 PM, Markus Armbruster wrote:
>
>> Programmingkid <programmingkidx@gmail.com> writes:
>> 
>>> On Sep 8, 2015, at 12:17 PM, Peter Maydell wrote:
>>> 
>>>> On 2 September 2015 at 01:56, Programmingkid
>>>> <programmingkidx@gmail.com> wrote:
>>>>> Add "Mount Image File..." and a "Eject Image File" menu items to
>>>>> cocoa interface. This patch makes sharing files between the
>>>>> host and the guest user-friendly.
>>>>> 
>>>>> The "Mount Image File..." menu item displays a dialog box having the
>>>>> user pick an image file to use in QEMU. The image file is setup as
>>>>> a USB flash drive. The user can do the equivalent of removing the
>>>>> flash drive by selecting the file in the "Eject Image File" submenu.
>>>>> 
>>>>> Signed-off-by: John Arbuckle <programmingkidx@gmail.com>
>>>>> 
>>>>> ---
>>>>> ui/cocoa.m |  212
>>>>> +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
>>>>> 1 files changed, 210 insertions(+), 1 deletions(-)
>>>>> 
>>>>> diff --git a/ui/cocoa.m b/ui/cocoa.m
>>>>> index 334e6f6..6c0ec18 100644
>>>>> --- a/ui/cocoa.m
>>>>> +++ b/ui/cocoa.m
>>>>> @@ -52,6 +52,9 @@
>>>>> #endif
>>>>> 
>>>>> 
>>>>> 
>>>>> #define cgrect(nsrect) (*(CGRect *)&(nsrect))
>>>>> +#define USB_DISK_ID "USB_DISK"
>>>>> +#define EJECT_IMAGE_FILE_TAG 2099
>>>>> 
>>>>> 
>>>>> 
>>>>> typedef struct {
>>>>>    int width;
>>>>> @@ -263,6 +266,43 @@ static void handleAnyDeviceErrors(Error * err)
>>>>>    }
>>>>> }
>>>>> 
>>>>> 
>>>>> 
>>>>> +/* Sends a command to the monitor console */
>>>>> +static void sendMonitorCommand(const char * commandString)
>>>>> +{
>>>>> +    int index;
>>>>> +    char * consoleName;
>>>>> +    static QemuConsole *monitor;
>>>>> +
>>>>> +    /* If the monitor console hasn't been found yet */
>>>>> +    if(!monitor) {
>>>>> +        index = 0;
>>>>> +        /* Find the monitor console */
>>>>> +        while (qemu_console_lookup_by_index(index) != NULL) {
>>>>> +            consoleName =
>>>>> qemu_console_get_label(qemu_console_lookup_by_index(index));
>>>>> +            if(strstr(consoleName, "monitor")) {
>>>>> +                monitor = qemu_console_lookup_by_index(index);
>>>>> +                break;
>>>>> +            }
>>>>> +            index++;
>>>>> +        }
>>>>> +    }
>>>>> +
>>>>> +    /* If the monitor console was not found */
>>>>> +    if(!monitor) {
>>>>> +        NSBeep();
>>>>> +        QEMU_Alert(@"Failed to find the monitor console!");
>>>>> +        return;
>>>>> +    }
>>>>> +
>>>>> +    /* send each letter in the commandString to the monitor */
>>>>> +    for (index = 0; index < strlen(commandString); index++) {
>>>>> +        kbd_put_keysym_console(monitor, commandString[index]);
>>>>> +    }
>>>> 
>>>> We're doing this by sending a string to the human monitor?
>> 
>> No way :)
>> 
>> You should not send a string to a monitor (QMP or HMP) just because you
>> can't be bothered to look up the proper C interfaces.
>
> I wouldn't say it like that. I did try to find the functions I needed,
> but didn't succeed.

I shouldn't have said "just because you can't be bothered", since I
don't know whether you tried.  My apologies.

What I needed to tell you was that this method is not going to be
accepted, preferrably before you sink more of your time into it.

My first try[1]: "Sure you want to do that, and not call the C interface
instead?"  This is the exceedingly polite version (for a German) of "bad
idea, drop it".  Followed by an explanation how to find the C interface
to use.  No reply.

Second try[2]: "On the flip side, you'll never get a patch abusing
handle_hmp_command() or handle_qmp_command() as internal interface
committed."  This was in context of you discussing the bad idea further,
with Max.  No reply to that part.  Okay, still not blunt enough.

Third try[3]: "No way :)".  Blunt enough to get the point across?

> Didn't you say yourself you don't want to see gui patches that change
> other code?
> My research indicated there would have to be changes to other files if
> I did try to use
> the C interface functions.  
>
> For example, the function add_init_drive() in device-hotplug.c looked
> really good. The
> problem was that it is a static function. Since you don't want changes
> made to other
> files, I decided not to use it. 

Let me clarify my attitude to integrated GUIs once more:

1. We shouldn't do it (my opinion, not the project's, obviously).

2. As long as we do it anyway, it shouldn't interfere with our two core
   missions: user space component for hypervisors, machine emulator.

2a. It shouldn't mess up core code.

2b. It shouldn't waste time of core contributors.

Now, the fact that I've been wasting my time (and my employer's money)
by trying to teach you finding the proper interfaces should show that 2b
isn't absolute.  More like guidelines, anyway.  More like "shouldn't
waste too much time".  Likewise 2a: simple changes can be okay.  If not,
review will lead you to how it needs to be done instead.

See below for advice on add_init_drive().

>>>> That definitely doesn't seem like the right way to do this
>>>> (and there might not even be a human monitor to talk to)...
>>> 
>>> Under what situation is the human monitor not available? 
>>> 
>>> Would you know what function I should use in place of the commands the
>>> patch uses?
>> 
>> I explained that already for QMP:
>> http://lists.gnu.org/archive/html/qemu-devel/2015-09/msg00008.html
>> 
>> The mapping from HMP to the C interfaces can be more complex.  Going
>> from QMP to C is easier.
>
> The problem with QMP is that it is so difficult to use. It could be
> made to be more
> user-friendly. I will try to use it anyways in my next patch. Thank you.

Going from QMP to C is easier, because QMP commands should always be
straightforward wrappers around an internal C interface you can use.

Of course, you can try to find the proper C interfaces from HMP, too.
Find the HMP command handler in hmp-commands.hx.  Drill down until you
hit the internal interface that is also used by QMP.  In simple cases,
it's obvious, e.g. hmp_drive_backup() calls qmp_drive_backup().
However, there are cases where the HMP command does non-trivial
HMP-specific stuff, and you have to find your way through it to the
proper internal interface.  Worse, there are a few hairy cases left
where HMP still isn't what it should be, namely a wrapper around the
internal C interface for QMP.

Creating block backends is one of these.  At this time, add_init_drive()
is just a hotplug helper function, not an internal interface.  The
internal interfaces are further down: blockdev_init().  This is one
layer below the internal C interface for QMP qmp_blockdev_add(), because
the HMP commands behave differently for historical reasons (read:
they're messed up, but we won't change them until the ongoing work on
the QMP commands is complete).


[1] Subject: Re: [Qemu-devel] function to execute qmp commands
Date: Tue, 01 Sep 2015 08:19:02 +0200
Message-ID: <87lhcqbr1l.fsf@blackfin.pond.sub.org>

[2] Subject: Re: [Qemu-devel] Mount image file feature
Date: Thu, 03 Sep 2015 11:46:29 +0200
Message-ID: <878u8nygwa.fsf@blackfin.pond.sub.org>

[3] Subject: Re: [Qemu-devel] [PATCH] ui/cocoa.m: Add Mount image file
menu item
Date: Tue, 08 Sep 2015 20:46:02 +0200
Message-ID: <87fv2og36d.fsf@blackfin.pond.sub.org>

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [Qemu-devel] [PATCH] ui/cocoa.m: Add Mount image file menu item
  2015-09-09  7:29         ` Markus Armbruster
@ 2015-09-09 21:37           ` Programmingkid
  2015-09-09 22:25             ` Eric Blake
  2015-09-10  3:28           ` Programmingkid
  1 sibling, 1 reply; 20+ messages in thread
From: Programmingkid @ 2015-09-09 21:37 UTC (permalink / raw)
  To: Markus Armbruster; +Cc: Peter Maydell, qemu-devel qemu-devel


On Sep 9, 2015, at 3:29 AM, Markus Armbruster wrote:

> Programmingkid <programmingkidx@gmail.com> writes:
> 
>> On Sep 8, 2015, at 2:46 PM, Markus Armbruster wrote:
>> 
>>> Programmingkid <programmingkidx@gmail.com> writes:
>>> 
>>>> On Sep 8, 2015, at 12:17 PM, Peter Maydell wrote:
>>>> 
>>>>> On 2 September 2015 at 01:56, Programmingkid
>>>>> <programmingkidx@gmail.com> wrote:
>>>>>> Add "Mount Image File..." and a "Eject Image File" menu items to
>>>>>> cocoa interface. This patch makes sharing files between the
>>>>>> host and the guest user-friendly.
>>>>>> 
>>>>>> The "Mount Image File..." menu item displays a dialog box having the
>>>>>> user pick an image file to use in QEMU. The image file is setup as
>>>>>> a USB flash drive. The user can do the equivalent of removing the
>>>>>> flash drive by selecting the file in the "Eject Image File" submenu.
>>>>>> 
>>>>>> Signed-off-by: John Arbuckle <programmingkidx@gmail.com>
>>>>>> 
>>>>>> ---
>>>>>> ui/cocoa.m |  212
>>>>>> +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
>>>>>> 1 files changed, 210 insertions(+), 1 deletions(-)
>>>>>> 
>>>>>> diff --git a/ui/cocoa.m b/ui/cocoa.m
>>>>>> index 334e6f6..6c0ec18 100644
>>>>>> --- a/ui/cocoa.m
>>>>>> +++ b/ui/cocoa.m
>>>>>> @@ -52,6 +52,9 @@
>>>>>> #endif
>>>>>> 
>>>>>> 
>>>>>> 
>>>>>> #define cgrect(nsrect) (*(CGRect *)&(nsrect))
>>>>>> +#define USB_DISK_ID "USB_DISK"
>>>>>> +#define EJECT_IMAGE_FILE_TAG 2099
>>>>>> 
>>>>>> 
>>>>>> 
>>>>>> typedef struct {
>>>>>>   int width;
>>>>>> @@ -263,6 +266,43 @@ static void handleAnyDeviceErrors(Error * err)
>>>>>>   }
>>>>>> }
>>>>>> 
>>>>>> 
>>>>>> 
>>>>>> +/* Sends a command to the monitor console */
>>>>>> +static void sendMonitorCommand(const char * commandString)
>>>>>> +{
>>>>>> +    int index;
>>>>>> +    char * consoleName;
>>>>>> +    static QemuConsole *monitor;
>>>>>> +
>>>>>> +    /* If the monitor console hasn't been found yet */
>>>>>> +    if(!monitor) {
>>>>>> +        index = 0;
>>>>>> +        /* Find the monitor console */
>>>>>> +        while (qemu_console_lookup_by_index(index) != NULL) {
>>>>>> +            consoleName =
>>>>>> qemu_console_get_label(qemu_console_lookup_by_index(index));
>>>>>> +            if(strstr(consoleName, "monitor")) {
>>>>>> +                monitor = qemu_console_lookup_by_index(index);
>>>>>> +                break;
>>>>>> +            }
>>>>>> +            index++;
>>>>>> +        }
>>>>>> +    }
>>>>>> +
>>>>>> +    /* If the monitor console was not found */
>>>>>> +    if(!monitor) {
>>>>>> +        NSBeep();
>>>>>> +        QEMU_Alert(@"Failed to find the monitor console!");
>>>>>> +        return;
>>>>>> +    }
>>>>>> +
>>>>>> +    /* send each letter in the commandString to the monitor */
>>>>>> +    for (index = 0; index < strlen(commandString); index++) {
>>>>>> +        kbd_put_keysym_console(monitor, commandString[index]);
>>>>>> +    }
>>>>> 
>>>>> We're doing this by sending a string to the human monitor?
>>> 
>>> No way :)
>>> 
>>> You should not send a string to a monitor (QMP or HMP) just because you
>>> can't be bothered to look up the proper C interfaces.
>> 
>> I wouldn't say it like that. I did try to find the functions I needed,
>> but didn't succeed.
> 
> I shouldn't have said "just because you can't be bothered", since I
> don't know whether you tried.  My apologies.
> 
> What I needed to tell you was that this method is not going to be
> accepted, preferrably before you sink more of your time into it.
> 
> My first try[1]: "Sure you want to do that, and not call the C interface
> instead?"  This is the exceedingly polite version (for a German) of "bad
> idea, drop it".  Followed by an explanation how to find the C interface
> to use.  No reply.
> 
> Second try[2]: "On the flip side, you'll never get a patch abusing
> handle_hmp_command() or handle_qmp_command() as internal interface
> committed."  This was in context of you discussing the bad idea further,
> with Max.  No reply to that part.  Okay, still not blunt enough.
> 
> Third try[3]: "No way :)".  Blunt enough to get the point across?
> 
>> Didn't you say yourself you don't want to see gui patches that change
>> other code?
>> My research indicated there would have to be changes to other files if
>> I did try to use
>> the C interface functions.  
>> 
>> For example, the function add_init_drive() in device-hotplug.c looked
>> really good. The
>> problem was that it is a static function. Since you don't want changes
>> made to other
>> files, I decided not to use it. 
> 
> Let me clarify my attitude to integrated GUIs once more:
> 
> 1. We shouldn't do it (my opinion, not the project's, obviously).
> 
> 2. As long as we do it anyway, it shouldn't interfere with our two core
>   missions: user space component for hypervisors, machine emulator.
> 
> 2a. It shouldn't mess up core code.
> 
> 2b. It shouldn't waste time of core contributors.
> 
> Now, the fact that I've been wasting my time (and my employer's money)
> by trying to teach you finding the proper interfaces should show that 2b
> isn't absolute.  More like guidelines, anyway.  More like "shouldn't
> waste too much time".  Likewise 2a: simple changes can be okay.  If not,
> review will lead you to how it needs to be done instead.
> 
> See below for advice on add_init_drive().
> 
>>>>> That definitely doesn't seem like the right way to do this
>>>>> (and there might not even be a human monitor to talk to)...
>>>> 
>>>> Under what situation is the human monitor not available? 
>>>> 
>>>> Would you know what function I should use in place of the commands the
>>>> patch uses?
>>> 
>>> I explained that already for QMP:
>>> http://lists.gnu.org/archive/html/qemu-devel/2015-09/msg00008.html
>>> 
>>> The mapping from HMP to the C interfaces can be more complex.  Going
>>> from QMP to C is easier.
>> 
>> The problem with QMP is that it is so difficult to use. It could be
>> made to be more
>> user-friendly. I will try to use it anyways in my next patch. Thank you.
> 
> Going from QMP to C is easier, because QMP commands should always be
> straightforward wrappers around an internal C interface you can use.
> 
> Of course, you can try to find the proper C interfaces from HMP, too.
> Find the HMP command handler in hmp-commands.hx.  Drill down until you
> hit the internal interface that is also used by QMP.  In simple cases,
> it's obvious, e.g. hmp_drive_backup() calls qmp_drive_backup().
> However, there are cases where the HMP command does non-trivial
> HMP-specific stuff, and you have to find your way through it to the
> proper internal interface.  Worse, there are a few hairy cases left
> where HMP still isn't what it should be, namely a wrapper around the
> internal C interface for QMP.
> 
> Creating block backends is one of these.  At this time, add_init_drive()
> is just a hotplug helper function, not an internal interface.  The
> internal interfaces are further down: blockdev_init().  This is one
> layer below the internal C interface for QMP qmp_blockdev_add(), because
> the HMP commands behave differently for historical reasons (read:
> they're messed up, but we won't change them until the ongoing work on
> the QMP commands is complete).
> 
> 
> [1] Subject: Re: [Qemu-devel] function to execute qmp commands
> Date: Tue, 01 Sep 2015 08:19:02 +0200
> Message-ID: <87lhcqbr1l.fsf@blackfin.pond.sub.org>
> 
> [2] Subject: Re: [Qemu-devel] Mount image file feature
> Date: Thu, 03 Sep 2015 11:46:29 +0200
> Message-ID: <878u8nygwa.fsf@blackfin.pond.sub.org>
> 
> [3] Subject: Re: [Qemu-devel] [PATCH] ui/cocoa.m: Add Mount image file
> menu item
> Date: Tue, 08 Sep 2015 20:46:02 +0200
> Message-ID: <87fv2og36d.fsf@blackfin.pond.sub.org>

Thank you very much for caring. I appreciate all the help I can receive. I so like my idea of
sending a command to QEMU as if the user typed it himself. It is so easy to maintain. So 
easy to use. So expandable. But given that two maintainers have told me that I can't do this,
the idea has to be abandoned. 

The C interface idea sounds good, but trying to figure out how to make any of the handler
functions work is very difficult. Just trying to make the QDict and QObject variables is just
too much. It needs to be a lot easier than this.

That leaves QMP. I am trying to figure it out. This is my attempt so far:

Error **errp;
char *commandBuffer;
commandBuffer = g_strdup_printf("{ \"execute\": \"quit\" }");
qmp_query_command_line_options(false, commandBuffer, errp);
printf("Program should quit now\n");

For some reason it didn't work. What am I doing wrong? Examples would really help.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [Qemu-devel] [PATCH] ui/cocoa.m: Add Mount image file menu item
  2015-09-09 21:37           ` Programmingkid
@ 2015-09-09 22:25             ` Eric Blake
  2015-09-09 22:31               ` Eric Blake
  2015-09-09 23:34               ` Programmingkid
  0 siblings, 2 replies; 20+ messages in thread
From: Eric Blake @ 2015-09-09 22:25 UTC (permalink / raw)
  To: Programmingkid, Markus Armbruster; +Cc: Peter Maydell, qemu-devel qemu-devel

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

On 09/09/2015 03:37 PM, Programmingkid wrote:

>>>>>> On 2 September 2015 at 01:56, Programmingkid
>>>>>> <programmingkidx@gmail.com> wrote:

It's okay to trim your reply, to make it easier for others to quickly
skip to the relevant part of your emails.


> 
> Thank you very much for caring. I appreciate all the help I can receive. I so like my idea of
> sending a command to QEMU as if the user typed it himself. It is so easy to maintain. So 
> easy to use. So expandable. But given that two maintainers have told me that I can't do this,
> the idea has to be abandoned. 

Even emulating typing into the QMP interface is a non-starter.  Just
call the same functions that QMP would eventually call.

> 
> The C interface idea sounds good, but trying to figure out how to make any of the handler
> functions work is very difficult. Just trying to make the QDict and QObject variables is just
> too much. It needs to be a lot easier than this.
> 
> That leaves QMP. I am trying to figure it out. This is my attempt so far:
> 
> Error **errp;
> char *commandBuffer;
> commandBuffer = g_strdup_printf("{ \"execute\": \"quit\" }");
> qmp_query_command_line_options(false, commandBuffer, errp);
> printf("Program should quit now\n");
> 

If you type the QMP command:

{ "execute": "query-command-line-options", "arguments": { "option":
"quit" } }

then that will result in calling:

Error *err;
qmp_query_command_line_options(true, "quit", &err);

which will set err (because '-quit' is not a command line option).

But if you are worried about constructing a QDict (which is a subset of
QObject), you are probably trying to call the wrong interface.

In qmp-commands.hx, the command 'query-command-line-options' is tied to
qmp_marshal_query_command_line_options (well, after Markus' latest
patches are applied; in current qemu.git it is still named
qmp_marshal_input_query_command_line_options).  That is the generated
function that takes QDict, and then calls into the much nicer interface
of qmp_query_command_line_options() that has already broken out all the
arguments into straight-forward parameters.

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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [Qemu-devel] [PATCH] ui/cocoa.m: Add Mount image file menu item
  2015-09-09 22:25             ` Eric Blake
@ 2015-09-09 22:31               ` Eric Blake
  2015-09-09 23:35                 ` Programmingkid
  2015-09-09 23:34               ` Programmingkid
  1 sibling, 1 reply; 20+ messages in thread
From: Eric Blake @ 2015-09-09 22:31 UTC (permalink / raw)
  To: Programmingkid, Markus Armbruster; +Cc: Peter Maydell, qemu-devel qemu-devel

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

On 09/09/2015 04:25 PM, Eric Blake wrote:

>> That leaves QMP. I am trying to figure it out. This is my attempt so far:
>>
>> Error **errp;
>> char *commandBuffer;
>> commandBuffer = g_strdup_printf("{ \"execute\": \"quit\" }");
>> qmp_query_command_line_options(false, commandBuffer, errp);
>> printf("Program should quit now\n");
>>

And if you're trying quit, then query-command-line-options is not how
you do it.  In QMP, quitting is done by:

{ "execute":"quit" }

which per qmp-commands.hx is serviced by qmp_marshal_quit() [was
qmp_marshal_input_quit()], which calls into qmp_quit().  So if you're
trying to quit qemu, you would do:

Error *err;
qmp_quit(&err);

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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [Qemu-devel] [PATCH] ui/cocoa.m: Add Mount image file menu item
  2015-09-09 22:25             ` Eric Blake
  2015-09-09 22:31               ` Eric Blake
@ 2015-09-09 23:34               ` Programmingkid
  2015-09-10  7:17                 ` Markus Armbruster
  1 sibling, 1 reply; 20+ messages in thread
From: Programmingkid @ 2015-09-09 23:34 UTC (permalink / raw)
  To: Eric Blake; +Cc: Peter Maydell, Markus Armbruster, qemu-devel qemu-devel


On Sep 9, 2015, at 6:25 PM, Eric Blake wrote:

> On 09/09/2015 03:37 PM, Programmingkid wrote:
> 
>> 
>> Thank you very much for caring. I appreciate all the help I can receive. I so like my idea of
>> sending a command to QEMU as if the user typed it himself. It is so easy to maintain. So 
>> easy to use. So expandable. But given that two maintainers have told me that I can't do this,
>> the idea has to be abandoned. 
> 
> Even emulating typing into the QMP interface is a non-starter.  Just
> call the same functions that QMP would eventually call.

That is a lot easier said than done. 

>> The C interface idea sounds good, but trying to figure out how to make any of the handler
>> functions work is very difficult. Just trying to make the QDict and QObject variables is just
>> too much. It needs to be a lot easier than this.
>> 
>> That leaves QMP. I am trying to figure it out. This is my attempt so far:
>> 
>> Error **errp;
>> char *commandBuffer;
>> commandBuffer = g_strdup_printf("{ \"execute\": \"quit\" }");
>> qmp_query_command_line_options(false, commandBuffer, errp);
>> printf("Program should quit now\n");
>> 
> 
> If you type the QMP command:
> 
> { "execute": "query-command-line-options", "arguments": { "option":
> "quit" } }
> 
> then that will result in calling:
> 
> Error *err;
> qmp_query_command_line_options(true, "quit", &err);
> 
> which will set err (because '-quit' is not a command line option).
> 
> But if you are worried about constructing a QDict (which is a subset of
> QObject), you are probably trying to call the wrong interface.

That may be so. I need to do the equivalent of "device_add" and "device_del". 
You have any ideas how to do it in C? 

> In qmp-commands.hx, the command 'query-command-line-options' is tied to
> qmp_marshal_query_command_line_options (well, after Markus' latest
> patches are applied; in current qemu.git it is still named
> qmp_marshal_input_query_command_line_options).  That is the generated
> function that takes QDict, and then calls into the much nicer interface
> of qmp_query_command_line_options() that has already broken out all the
> arguments into straight-forward parameters.

I just wish it was as easy as qmp("{ \"execute\" : \"<some command> \"}"); 

If you know a way to execute a qmp command by using a function that takes a string, 
please let me know.

Thanks for the help.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [Qemu-devel] [PATCH] ui/cocoa.m: Add Mount image file menu item
  2015-09-09 22:31               ` Eric Blake
@ 2015-09-09 23:35                 ` Programmingkid
  0 siblings, 0 replies; 20+ messages in thread
From: Programmingkid @ 2015-09-09 23:35 UTC (permalink / raw)
  To: Eric Blake; +Cc: Peter Maydell, Markus Armbruster, qemu-devel qemu-devel


On Sep 9, 2015, at 6:31 PM, Eric Blake wrote:

> On 09/09/2015 04:25 PM, Eric Blake wrote:
> 
>>> That leaves QMP. I am trying to figure it out. This is my attempt so far:
>>> 
>>> Error **errp;
>>> char *commandBuffer;
>>> commandBuffer = g_strdup_printf("{ \"execute\": \"quit\" }");
>>> qmp_query_command_line_options(false, commandBuffer, errp);
>>> printf("Program should quit now\n");
>>> 
> 
> And if you're trying quit, then query-command-line-options is not how
> you do it.  In QMP, quitting is done by:
> 
> { "execute":"quit" }
> 
> which per qmp-commands.hx is serviced by qmp_marshal_quit() [was
> qmp_marshal_input_quit()], which calls into qmp_quit().  So if you're
> trying to quit qemu, you would do:
> 
> Error *err;
> qmp_quit(&err);
> 

I only chose the quit command because it looked very short and simple to try. 

Would you know how to create a QDict, QObject, and QemuOpts objects?

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [Qemu-devel] [PATCH] ui/cocoa.m: Add Mount image file menu item
  2015-09-09  7:29         ` Markus Armbruster
  2015-09-09 21:37           ` Programmingkid
@ 2015-09-10  3:28           ` Programmingkid
  2015-09-10  7:21             ` Markus Armbruster
  1 sibling, 1 reply; 20+ messages in thread
From: Programmingkid @ 2015-09-10  3:28 UTC (permalink / raw)
  To: Markus Armbruster; +Cc: Peter Maydell, qemu-devel qemu-devel

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

Does this look about right?

    QDict *qdict;
    Error *errp;
    QObject **ret_data;
    static int counter;
    char *idString, *fileName;

	// The file variable is objective-c, left that code out

        fileName = g_strdup_printf("%s",
                            [file cStringUsingEncoding: NSASCIIStringEncoding]);
        
        /* Create an unique id */
        idString = g_strdup_printf("USB%d", counter++);

        /* Create the QDICT object */
        qdict = qdict_new();
        qdict_put_obj(qdict, "id", qstring_from_str(idString));
        qdict_put_obj(qdict, "device", qstring_from_str(idString));
        qdict_put_obj(qdict, "if", qstring_from_str("none"));
        qdict_put_obj(qdict, "file", qstring_from_str(fileName));
        qdict_put_obj(qdict, "driver", qstring_from_str("usb-storage"));
        drive_add(IF_DEFAULT, 0, fileName, "none");
        qmp_device_add(qdict, ret_data, &errp);
        handleAnyDeviceErrors(errp);
        g_free(fileName);
        g_free(idString);

This is a sample of what I am working on. For some reason, it crashes QEMU. Any clues why? I think it might be because of qdict_put_obj(). 

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [Qemu-devel] [PATCH] ui/cocoa.m: Add Mount image file menu item
  2015-09-09 23:34               ` Programmingkid
@ 2015-09-10  7:17                 ` Markus Armbruster
  0 siblings, 0 replies; 20+ messages in thread
From: Markus Armbruster @ 2015-09-10  7:17 UTC (permalink / raw)
  To: Programmingkid; +Cc: Peter Maydell, qemu-devel qemu-devel

Programmingkid <programmingkidx@gmail.com> writes:

> On Sep 9, 2015, at 6:25 PM, Eric Blake wrote:
>
>> On 09/09/2015 03:37 PM, Programmingkid wrote:
>> 
>>> 
>>> Thank you very much for caring. I appreciate all the help I can
>>> receive. I so like my idea of
>>> sending a command to QEMU as if the user typed it himself. It is so
>>> easy to maintain. So
>>> easy to use. So expandable. But given that two maintainers have
>>> told me that I can't do this,
>>> the idea has to be abandoned. 
>> 
>> Even emulating typing into the QMP interface is a non-starter.  Just
>> call the same functions that QMP would eventually call.
>
> That is a lot easier said than done. 

Most of the time, it's actually a lot easier than creating a monitor,
constructing a command safely (mind the quoting rules), feeding it to
the monitor, receiving the reply, parsing the reply, and having fun with
manual memory management along the way.

For an exception, see below.

>>> The C interface idea sounds good, but trying to figure out how to
>>> make any of the handler
>>> functions work is very difficult. Just trying to make the QDict and
>>> QObject variables is just
>>> too much. It needs to be a lot easier than this.
>>> 
>>> That leaves QMP. I am trying to figure it out. This is my attempt so far:
>>> 
>>> Error **errp;
>>> char *commandBuffer;
>>> commandBuffer = g_strdup_printf("{ \"execute\": \"quit\" }");
>>> qmp_query_command_line_options(false, commandBuffer, errp);
>>> printf("Program should quit now\n");
>>> 
>> 
>> If you type the QMP command:
>> 
>> { "execute": "query-command-line-options", "arguments": { "option":
>> "quit" } }
>> 
>> then that will result in calling:
>> 
>> Error *err;
>> qmp_query_command_line_options(true, "quit", &err);
>> 
>> which will set err (because '-quit' is not a command line option).
>> 
>> But if you are worried about constructing a QDict (which is a subset of
>> QObject), you are probably trying to call the wrong interface.
>
> That may be so. I need to do the equivalent of "device_add" and "device_del". 
> You have any ideas how to do it in C? 

That's the exception.  See further below.

>> In qmp-commands.hx, the command 'query-command-line-options' is tied to
>> qmp_marshal_query_command_line_options (well, after Markus' latest
>> patches are applied; in current qemu.git it is still named
>> qmp_marshal_input_query_command_line_options).  That is the generated
>> function that takes QDict, and then calls into the much nicer interface
>> of qmp_query_command_line_options() that has already broken out all the
>> arguments into straight-forward parameters.
>
> I just wish it was as easy as qmp("{ \"execute\" : \"<some command> \"}"); 

What's difficult about any of the following?

    qmp_system_reset(&err);

    qmp_query_command_line_options(false, NULL, &err);

    qmp_device_del("my-dev-id", &err);

Each of them is about as simple as it gets.

The sad exceptions are the QMP commands defined with 'gen': false.  For
these, you have to use a much less convenient interface, of the form

    void qmp_FOO(QDict *qdict, QObject **ret_data, Error **errp);

Matches the QMP wire format closely.  qdict corrsponds to the
"arguments" JSON object, ret_data corresponds to the success response's
"return" JSON value, and errp corresponds do the error response.

There are only a few such commands, and we're working on reducing their
number further.  The hardest nut, however, is device_add.

> If you know a way to execute a qmp command by using a function that
> takes a string,
> please let me know.

If there was one, it wouldn't be the C interface you should use.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [Qemu-devel] [PATCH] ui/cocoa.m: Add Mount image file menu item
  2015-09-10  3:28           ` Programmingkid
@ 2015-09-10  7:21             ` Markus Armbruster
  2015-09-10 16:22               ` Programmingkid
  0 siblings, 1 reply; 20+ messages in thread
From: Markus Armbruster @ 2015-09-10  7:21 UTC (permalink / raw)
  To: Programmingkid; +Cc: Peter Maydell, qemu-devel qemu-devel

Programmingkid <programmingkidx@gmail.com> writes:

> Does this look about right?
>
>     QDict *qdict;
>     Error *errp;
>     QObject **ret_data;
>     static int counter;
>     char *idString, *fileName;
>
> 	// The file variable is objective-c, left that code out
>
>         fileName = g_strdup_printf("%s",
>                             [file cStringUsingEncoding: NSASCIIStringEncoding]);
>         
>         /* Create an unique id */
>         idString = g_strdup_printf("USB%d", counter++);
>
>         /* Create the QDICT object */
>         qdict = qdict_new();
>         qdict_put_obj(qdict, "id", qstring_from_str(idString));
>         qdict_put_obj(qdict, "device", qstring_from_str(idString));
>         qdict_put_obj(qdict, "if", qstring_from_str("none"));
>         qdict_put_obj(qdict, "file", qstring_from_str(fileName));
>         qdict_put_obj(qdict, "driver", qstring_from_str("usb-storage"));
>         drive_add(IF_DEFAULT, 0, fileName, "none");
>         qmp_device_add(qdict, ret_data, &errp);
>         handleAnyDeviceErrors(errp);
>         g_free(fileName);
>         g_free(idString);
>
> This is a sample of what I am working on. For some reason, it crashes QEMU. Any clues why? I think it might be because of qdict_put_obj(). 

My crystal ball is down for maintenance today, so you'll have to gives
us the clues yourself: a stack backtrace, for starters :)

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [Qemu-devel] [PATCH] ui/cocoa.m: Add Mount image file menu item
  2015-09-10  7:21             ` Markus Armbruster
@ 2015-09-10 16:22               ` Programmingkid
  2015-09-10 17:15                 ` Markus Armbruster
  0 siblings, 1 reply; 20+ messages in thread
From: Programmingkid @ 2015-09-10 16:22 UTC (permalink / raw)
  To: Markus Armbruster; +Cc: Peter Maydell, qemu-devel qemu-devel


On Sep 10, 2015, at 3:21 AM, Markus Armbruster wrote:

> Programmingkid <programmingkidx@gmail.com> writes:
> 
>> Does this look about right?
>> 
>>    QDict *qdict;
>>    Error *errp;
>>    QObject **ret_data;
>>    static int counter;
>>    char *idString, *fileName;
>> 
>> 	// The file variable is objective-c, left that code out
>> 
>>        fileName = g_strdup_printf("%s",
>>                            [file cStringUsingEncoding: NSASCIIStringEncoding]);
>> 
>>        /* Create an unique id */
>>        idString = g_strdup_printf("USB%d", counter++);
>> 
>>        /* Create the QDICT object */
>>        qdict = qdict_new();
>>        qdict_put_obj(qdict, "id", qstring_from_str(idString));
>>        qdict_put_obj(qdict, "device", qstring_from_str(idString));
>>        qdict_put_obj(qdict, "if", qstring_from_str("none"));
>>        qdict_put_obj(qdict, "file", qstring_from_str(fileName));
>>        qdict_put_obj(qdict, "driver", qstring_from_str("usb-storage"));
>>        drive_add(IF_DEFAULT, 0, fileName, "none");
>>        qmp_device_add(qdict, ret_data, &errp);
>>        handleAnyDeviceErrors(errp);
>>        g_free(fileName);
>>        g_free(idString);
>> 
>> This is a sample of what I am working on. For some reason, it crashes QEMU. Any clues why? I think it might be because of qdict_put_obj(). 
> 
> My crystal ball is down for maintenance today, so you'll have to gives
> us the clues yourself: a stack backtrace, for starters :)

Here is the error:

2015-09-10 12:21:12.355 qemu-system-ppc[17603:903] HIToolbox: ignoring exception 'Uncaught system exception: signal 11' that raised inside Carbon event dispatch
(
	0   CoreFoundation                      0x00007fff83ad37b4 __exceptionPreprocess + 180
	1   libobjc.A.dylib                     0x00007fff83567f03 objc_exception_throw + 45
	2   CoreFoundation                      0x00007fff83b2b969 -[NSException raise] + 9
	3   ExceptionHandling                   0x00007fff845082d3 NSExceptionHandlerUncaughtSignalHandler + 37
	4   libSystem.B.dylib                   0x00007fff825431ba _sigtramp + 26
	5   ???                                 0x00007fff5fc12dc0 0x0 + 140734799883712
	6   qemu-system-ppc                     0x00000001003c4109 qdict_get_try_str + 58
	7   qemu-system-ppc                     0x00000001003dba04 qemu_opts_from_qdict + 63
	8   qemu-system-ppc                     0x0000000100169388 qmp_device_add + 78

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [Qemu-devel] [PATCH] ui/cocoa.m: Add Mount image file menu item
  2015-09-10 16:22               ` Programmingkid
@ 2015-09-10 17:15                 ` Markus Armbruster
  2015-09-10 17:40                   ` Programmingkid
  0 siblings, 1 reply; 20+ messages in thread
From: Markus Armbruster @ 2015-09-10 17:15 UTC (permalink / raw)
  To: Programmingkid; +Cc: Peter Maydell, qemu-devel qemu-devel

Programmingkid <programmingkidx@gmail.com> writes:

> On Sep 10, 2015, at 3:21 AM, Markus Armbruster wrote:
>
>> Programmingkid <programmingkidx@gmail.com> writes:
>> 
>>> Does this look about right?
>>> 
>>>    QDict *qdict;
>>>    Error *errp;
>>>    QObject **ret_data;
>>>    static int counter;
>>>    char *idString, *fileName;
>>> 
>>> 	// The file variable is objective-c, left that code out
>>> 
>>>        fileName = g_strdup_printf("%s",
>>>                            [file cStringUsingEncoding: NSASCIIStringEncoding]);
>>> 
>>>        /* Create an unique id */
>>>        idString = g_strdup_printf("USB%d", counter++);
>>> 
>>>        /* Create the QDICT object */
>>>        qdict = qdict_new();
>>>        qdict_put_obj(qdict, "id", qstring_from_str(idString));
>>>        qdict_put_obj(qdict, "device", qstring_from_str(idString));
>>>        qdict_put_obj(qdict, "if", qstring_from_str("none"));
>>>        qdict_put_obj(qdict, "file", qstring_from_str(fileName));
>>>        qdict_put_obj(qdict, "driver", qstring_from_str("usb-storage"));
>>>        drive_add(IF_DEFAULT, 0, fileName, "none");
>>>        qmp_device_add(qdict, ret_data, &errp);
>>>        handleAnyDeviceErrors(errp);
>>>        g_free(fileName);
>>>        g_free(idString);
>>> 
>>> This is a sample of what I am working on. For some reason, it crashes QEMU. Any clues why? I think it might be because of qdict_put_obj(). 
>> 
>> My crystal ball is down for maintenance today, so you'll have to gives
>> us the clues yourself: a stack backtrace, for starters :)
>
> Here is the error:
>
> 2015-09-10 12:21:12.355 qemu-system-ppc[17603:903] HIToolbox: ignoring
> exception 'Uncaught system exception: signal 11' that raised inside
> Carbon event dispatch
> (
> 	0 CoreFoundation 0x00007fff83ad37b4 __exceptionPreprocess +
> 180
> 	1 libobjc.A.dylib 0x00007fff83567f03 objc_exception_throw + 45
> 	2 CoreFoundation 0x00007fff83b2b969 -[NSException raise] + 9
> 	3 ExceptionHandling 0x00007fff845082d3
> NSExceptionHandlerUncaughtSignalHandler + 37
> 	4 libSystem.B.dylib 0x00007fff825431ba _sigtramp + 26
> 	5 ???  0x00007fff5fc12dc0 0x0 + 140734799883712
> 	6 qemu-system-ppc 0x00000001003c4109 qdict_get_try_str + 58
> 	7 qemu-system-ppc 0x00000001003dba04 qemu_opts_from_qdict + 63
> 	8 qemu-system-ppc 0x0000000100169388 qmp_device_add + 78

Crashes in qdict_get_try_str().  Use a debugger to find out what goes
wrong there.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [Qemu-devel] [PATCH] ui/cocoa.m: Add Mount image file menu item
  2015-09-10 17:15                 ` Markus Armbruster
@ 2015-09-10 17:40                   ` Programmingkid
  2015-09-11  7:09                     ` Markus Armbruster
  0 siblings, 1 reply; 20+ messages in thread
From: Programmingkid @ 2015-09-10 17:40 UTC (permalink / raw)
  To: Markus Armbruster; +Cc: Peter Maydell, qemu-devel qemu-devel


On Sep 10, 2015, at 1:15 PM, Markus Armbruster wrote:

> Programmingkid <programmingkidx@gmail.com> writes:
> 
>> On Sep 10, 2015, at 3:21 AM, Markus Armbruster wrote:
>> 
>>> Programmingkid <programmingkidx@gmail.com> writes:
>>> 
>>>> Does this look about right?
>>>> 
>>>>   QDict *qdict;
>>>>   Error *errp;
>>>>   QObject **ret_data;
>>>>   static int counter;
>>>>   char *idString, *fileName;
>>>> 
>>>> 	// The file variable is objective-c, left that code out
>>>> 
>>>>       fileName = g_strdup_printf("%s",
>>>>                           [file cStringUsingEncoding: NSASCIIStringEncoding]);
>>>> 
>>>>       /* Create an unique id */
>>>>       idString = g_strdup_printf("USB%d", counter++);
>>>> 
>>>>       /* Create the QDICT object */
>>>>       qdict = qdict_new();
>>>>       qdict_put_obj(qdict, "id", qstring_from_str(idString));
>>>>       qdict_put_obj(qdict, "device", qstring_from_str(idString));
>>>>       qdict_put_obj(qdict, "if", qstring_from_str("none"));
>>>>       qdict_put_obj(qdict, "file", qstring_from_str(fileName));
>>>>       qdict_put_obj(qdict, "driver", qstring_from_str("usb-storage"));
>>>>       drive_add(IF_DEFAULT, 0, fileName, "none");
>>>>       qmp_device_add(qdict, ret_data, &errp);
>>>>       handleAnyDeviceErrors(errp);
>>>>       g_free(fileName);
>>>>       g_free(idString);
>>>> 
>>>> This is a sample of what I am working on. For some reason, it crashes QEMU. Any clues why? I think it might be because of qdict_put_obj(). 
>>> 
>>> My crystal ball is down for maintenance today, so you'll have to gives
>>> us the clues yourself: a stack backtrace, for starters :)
>> 
>> Here is the error:
>> 
>> 2015-09-10 12:21:12.355 qemu-system-ppc[17603:903] HIToolbox: ignoring
>> exception 'Uncaught system exception: signal 11' that raised inside
>> Carbon event dispatch
>> (
>> 	0 CoreFoundation 0x00007fff83ad37b4 __exceptionPreprocess +
>> 180
>> 	1 libobjc.A.dylib 0x00007fff83567f03 objc_exception_throw + 45
>> 	2 CoreFoundation 0x00007fff83b2b969 -[NSException raise] + 9
>> 	3 ExceptionHandling 0x00007fff845082d3
>> NSExceptionHandlerUncaughtSignalHandler + 37
>> 	4 libSystem.B.dylib 0x00007fff825431ba _sigtramp + 26
>> 	5 ???  0x00007fff5fc12dc0 0x0 + 140734799883712
>> 	6 qemu-system-ppc 0x00000001003c4109 qdict_get_try_str + 58
>> 	7 qemu-system-ppc 0x00000001003dba04 qemu_opts_from_qdict + 63
>> 	8 qemu-system-ppc 0x0000000100169388 qmp_device_add + 78
> 
> Crashes in qdict_get_try_str().  Use a debugger to find out what goes
> wrong there.

This is what it said:

Program received signal EXC_BAD_ACCESS, Could not access memory.
Reason: KERN_INVALID_ADDRESS at address: 0x0000000001a7e130
0x00000001003c39e9 in qobject_type (obj=0x1a7e130) at qobject.h:109
109	    assert(obj->type != NULL);
(gdb) bt
#0  0x00000001003c39e9 in qobject_type (obj=0x1a7e130) at qobject.h:109
#1  0x00000001003c4145 in qdict_get_try_str (qdict=0x102890a00, key=0x1003e8308 "id") at qobject/qdict.c:341
#2  0x00000001003dba44 in qemu_opts_from_qdict (list=0x1005a2f40, qdict=0x102890a00, errp=0x7fff5fbfcfe0) at util/qemu-option.c:968
#3  0x00000001001693b4 in qmp_device_add (qdict=0x102890a00, ret_data=0x7fff5fbfd038, errp=0x7fff5fbfd030) at qdev-monitor.c:767

I do not know much about the QDict type. Did I use it right by using qstring_from_str() to set a key's value to another string?

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [Qemu-devel] [PATCH] ui/cocoa.m: Add Mount image file menu item
  2015-09-10 17:40                   ` Programmingkid
@ 2015-09-11  7:09                     ` Markus Armbruster
  0 siblings, 0 replies; 20+ messages in thread
From: Markus Armbruster @ 2015-09-11  7:09 UTC (permalink / raw)
  To: Programmingkid; +Cc: Peter Maydell, qemu-devel qemu-devel

Programmingkid <programmingkidx@gmail.com> writes:

> On Sep 10, 2015, at 1:15 PM, Markus Armbruster wrote:
>
>> Programmingkid <programmingkidx@gmail.com> writes:
>> 
>>> On Sep 10, 2015, at 3:21 AM, Markus Armbruster wrote:
>>> 
>>>> Programmingkid <programmingkidx@gmail.com> writes:
>>>> 
>>>>> Does this look about right?
>>>>> 
>>>>>   QDict *qdict;
>>>>>   Error *errp;
>>>>>   QObject **ret_data;
>>>>>   static int counter;
>>>>>   char *idString, *fileName;
>>>>> 
>>>>> 	// The file variable is objective-c, left that code out
>>>>> 
>>>>>       fileName = g_strdup_printf("%s",
>>>>>                           [file cStringUsingEncoding: NSASCIIStringEncoding]);
>>>>> 
>>>>>       /* Create an unique id */
>>>>>       idString = g_strdup_printf("USB%d", counter++);
>>>>> 
>>>>>       /* Create the QDICT object */
>>>>>       qdict = qdict_new();
>>>>>       qdict_put_obj(qdict, "id", qstring_from_str(idString));
>>>>>       qdict_put_obj(qdict, "device", qstring_from_str(idString));
>>>>>       qdict_put_obj(qdict, "if", qstring_from_str("none"));
>>>>>       qdict_put_obj(qdict, "file", qstring_from_str(fileName));
>>>>>       qdict_put_obj(qdict, "driver", qstring_from_str("usb-storage"));
>>>>>       drive_add(IF_DEFAULT, 0, fileName, "none");
>>>>>       qmp_device_add(qdict, ret_data, &errp);
>>>>>       handleAnyDeviceErrors(errp);
>>>>>       g_free(fileName);
>>>>>       g_free(idString);
>>>>> 
>>>>> This is a sample of what I am working on. For some reason, it crashes QEMU. Any clues why? I think it might be because of qdict_put_obj(). 
>>>> 
>>>> My crystal ball is down for maintenance today, so you'll have to gives
>>>> us the clues yourself: a stack backtrace, for starters :)
>>> 
>>> Here is the error:
>>> 
>>> 2015-09-10 12:21:12.355 qemu-system-ppc[17603:903] HIToolbox: ignoring
>>> exception 'Uncaught system exception: signal 11' that raised inside
>>> Carbon event dispatch
>>> (
>>> 	0 CoreFoundation 0x00007fff83ad37b4 __exceptionPreprocess +
>>> 180
>>> 	1 libobjc.A.dylib 0x00007fff83567f03 objc_exception_throw + 45
>>> 	2 CoreFoundation 0x00007fff83b2b969 -[NSException raise] + 9
>>> 	3 ExceptionHandling 0x00007fff845082d3
>>> NSExceptionHandlerUncaughtSignalHandler + 37
>>> 	4 libSystem.B.dylib 0x00007fff825431ba _sigtramp + 26
>>> 	5 ???  0x00007fff5fc12dc0 0x0 + 140734799883712
>>> 	6 qemu-system-ppc 0x00000001003c4109 qdict_get_try_str + 58
>>> 	7 qemu-system-ppc 0x00000001003dba04 qemu_opts_from_qdict + 63
>>> 	8 qemu-system-ppc 0x0000000100169388 qmp_device_add + 78
>> 
>> Crashes in qdict_get_try_str().  Use a debugger to find out what goes
>> wrong there.
>
> This is what it said:
>
> Program received signal EXC_BAD_ACCESS, Could not access memory.
> Reason: KERN_INVALID_ADDRESS at address: 0x0000000001a7e130
> 0x00000001003c39e9 in qobject_type (obj=0x1a7e130) at qobject.h:109
> 109	    assert(obj->type != NULL);
> (gdb) bt
> #0  0x00000001003c39e9 in qobject_type (obj=0x1a7e130) at qobject.h:109
> #1 0x00000001003c4145 in qdict_get_try_str (qdict=0x102890a00,
> key=0x1003e8308 "id") at qobject/qdict.c:341
> #2 0x00000001003dba44 in qemu_opts_from_qdict (list=0x1005a2f40,
> qdict=0x102890a00, errp=0x7fff5fbfcfe0) at util/qemu-option.c:968
> #3 0x00000001001693b4 in qmp_device_add (qdict=0x102890a00,
> ret_data=0x7fff5fbfd038, errp=0x7fff5fbfd030) at qdev-monitor.c:767
>
> I do not know much about the QDict type. Did I use it right by using
> qstring_from_str() to set a key's value to another string?

I can't see your bug.

I'm happy to explain how things work and provide advice, I review
working patches touching my areas of expertise, but I can't do your
debugging for you, sorry.

For example code using QDict, check out tests/check-qdict.c.

^ permalink raw reply	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2015-09-11  7:09 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-09-02  0:56 [Qemu-devel] [PATCH] ui/cocoa.m: Add Mount image file menu item Programmingkid
2015-09-02  9:05 ` Markus Armbruster
2015-09-08 16:17 ` Peter Maydell
2015-09-08 16:39   ` Programmingkid
2015-09-08 16:54     ` Peter Maydell
2015-09-08 18:46     ` Markus Armbruster
2015-09-09  3:02       ` Programmingkid
2015-09-09  7:29         ` Markus Armbruster
2015-09-09 21:37           ` Programmingkid
2015-09-09 22:25             ` Eric Blake
2015-09-09 22:31               ` Eric Blake
2015-09-09 23:35                 ` Programmingkid
2015-09-09 23:34               ` Programmingkid
2015-09-10  7:17                 ` Markus Armbruster
2015-09-10  3:28           ` Programmingkid
2015-09-10  7:21             ` Markus Armbruster
2015-09-10 16:22               ` Programmingkid
2015-09-10 17:15                 ` Markus Armbruster
2015-09-10 17:40                   ` Programmingkid
2015-09-11  7:09                     ` Markus Armbruster

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.