Re: [Qemu-devel] [PATCH 2/3] ui/cocoa.m: Adds device menu items to Machine menu

[Peter Maydell]

On 11 May 2015 at 23:16, Programmingkid <address@hidden> wrote:
> Adds all removable devices to the Machine menu as a Change and Eject menu
> item pair.
> ide-cd0 would have a "Change ide-cd0..." and "Eject ide-cd0" menu items.
>
> Signed-off-by: John Arbuckle <address@hidden>

Kevin -- is there a block layer API for a UI to be informed
of hotplugged/unplugged block devices so we can update our menu
entries as removable devices appear or disappear?

> ---
>  ui/cocoa.m |  113
> ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++--
>  1 files changed, 110 insertions(+), 3 deletions(-)
>
> diff --git a/ui/cocoa.m b/ui/cocoa.m
> index 7a9c194..5e558ea 100644
> --- a/ui/cocoa.m
> +++ b/ui/cocoa.m
> @@ -30,6 +30,7 @@
>  #include "ui/input.h"
>  #include "sysemu/sysemu.h"
>  #include "qmp-commands.h"
> +#include "sysemu/blockdev.h"
>
>
>
>  #ifndef MAC_OS_X_VERSION_10_4
>  #define MAC_OS_X_VERSION_10_4 1040
> @@ -241,7 +242,16 @@ static int cocoa_keycode_to_qemu(int keycode)
>      return keymap[keycode];
>  }
>
>
>
> -
> +/* Handles any errors that happen with a device transaction */
> +static void handleAnyDeviceErrors(Error * err)
> +{
> +    if (err) {
> +        NSRunAlertPanel(@"Alert", [NSString stringWithCString:
> error_get_pretty(err)
> +                                                     encoding:
> NSASCIIStringEncoding],
> +                                                     @"OK", nil, nil);

NSRunAlertPanel() is deprecated in OSX 10.10.

> +        error_free(err);
> +    }
> +}
>
>
>
>  /*
>   ------------------------------------------------------
> @@ -301,6 +311,17 @@ QemuCocoaView *cocoaView;
>          screen.width = frameRect.size.width;
>          screen.height = frameRect.size.height;
>
>
>
> +        /* Used for displaying pause on the screen */
> +        pauseLabel = [NSTextField new];
> +        [pauseLabel setBezeled:YES];
> +        [pauseLabel setDrawsBackground:YES];
> +        [pauseLabel setBackgroundColor: [NSColor whiteColor]];
> +        [pauseLabel setEditable:NO];
> +        [pauseLabel setSelectable:NO];
> +        [pauseLabel setStringValue: @"Paused"];
> +        [pauseLabel setFont: [NSFont fontWithName: @"Helvetica" size: 90]];
> +        [pauseLabel setTextColor: [NSColor blackColor]];
> +        [pauseLabel sizeToFit];

This hunk has snuck in from the pause/resume patchset.

>      }
>      return self;
>  }
> @@ -788,8 +809,6 @@ QemuCocoaView *cocoaView;
>  - (QEMUScreen) gscreen {return screen;}
>  @end
>
>
>
> -
> -
>  /*
>   ------------------------------------------------------
>      QemuCocoaAppController
> @@ -807,6 +826,8 @@ QemuCocoaView *cocoaView;
>  - (void)resumeQemu: (id) sender;
>  - (void)displayPause;
>  - (void)removePause;
> +- (void)ejectDeviceMedia:(id)sender;
> +- (void)changeDeviceMedia:(id)sender;
>  @end
>
>
>
>  @implementation QemuCocoaAppController
> @@ -998,6 +1019,51 @@ QemuCocoaView *cocoaView;
>      [pauseLabel removeFromSuperview];
>  }
>
>
>
> +// Ejects the media
> +// Uses sender's tag to figure out device to eject
> +- (void)ejectDeviceMedia:(id)sender
> +{
> +    NSString * drive;
> +    drive = [sender representedObject];
> +    if(drive == nil) {
> +        NSBeep();
> +        NSRunAlertPanel(@"Alert", @"Failed to find drive to eject!", @"OK",
> nil, nil);
> +        NSLog(@"Error: Failed to find drive to eject!\n");
> +        return;
> +    }
> +
> +    Error *err = NULL;
> +    qmp_eject([drive cStringUsingEncoding: NSASCIIStringEncoding], false,
> false, &err);
> +    handleAnyDeviceErrors(err);
> +}
> +
> +/* Displays a dialog box asking the user to select an image file to load.
> +   Uses sender's tag value to figure out which drive to use. */
> +- (void)changeDeviceMedia:(id)sender
> +{
> +    /* Find the drive name */
> +    NSString * drive;
> +    drive = [sender representedObject];
> +    if(drive == nil) {
> +        NSBeep();
> +        NSRunAlertPanel(@"Alert", @"Could not find drive!", @"OK", nil,
> nil);
> +        NSLog(@"Error: Could not find drive!\n");
> +        return;
> +    }
> +
> +    /* Display the file open dialog */
> +    NSOpenPanel * open_panel;
> +    open_panel = [NSOpenPanel openPanel];
> +    [open_panel setCanChooseFiles: YES];
> +    [open_panel setAllowsMultipleSelection: NO];
> +    if([open_panel runModalForDirectory: nil file: nil] == NSOKButton) {

Can't we share the dialogbox handling code with the initial
"you didn't pass any arguments" code somehow? For instance this
doesn't have the "restrict filetypes to image files" handling.

runModalForDirectory is deprecated in 10.6.
NSOKButton is deprecated in 10.10.

> +        NSString * file = [[open_panel filenames] objectAtIndex: 0];
> +        Error *err = NULL;
> +        qmp_change_blockdev([drive cStringUsingEncoding:
> NSASCIIStringEncoding], [file cStringUsingEncoding: NSASCIIStringEncoding],
> "raw", &err);
> +        handleAnyDeviceErrors(err);
> +    }
> +}
> +
>  @end
>
>
>
>
>
> @@ -1096,6 +1162,46 @@ int main (int argc, const char * argv[]) {
>
>
>
>  #pragma mark machine menu code
>
>
>
> +// Make menu items for all removable devices
> +static void addDeviceMenuItems(NSMenu * menu)
> +{
> +    NSMenuItem * menuItem;
> +    BlockInfoList * currentDevice;
> +    NSString * deviceName;
> +
> +    currentDevice = qmp_query_block(false);

qmp_query_block's argument is an Error**, not a bool!

> +    if(currentDevice == NULL) {
> +        NSBeep();
> +        NSRunAlertPanel(@"Alert", @"Failed to query for block devices!",
> @"OK", nil, nil);
> +        return;
> +    }
> +
> +    // Add a separator between related groups of menu items
> +    [menu addItem:[NSMenuItem separatorItem]];
> +
> +    /* Loop thru all the block devices in the emulator */
> +    while (currentDevice) {
> +        deviceName = [[NSString stringWithFormat: @"%s",
> currentDevice->value->device] retain];
> +
> +        if(currentDevice->value->removable == true) {

Not much point in comparing against true.

> +            menuItem = [[NSMenuItem alloc] initWithTitle: [NSString
> stringWithFormat: @"Change %s...", currentDevice->value->device]
> +                                                  action:
> @selector(changeDeviceMedia:)
> +                                           keyEquivalent: @""];
> +            [menu addItem: menuItem];
> +            [menuItem setRepresentedObject: deviceName];
> +            [menuItem autorelease];
> +
> +            menuItem = [[NSMenuItem alloc] initWithTitle: [NSString
> stringWithFormat: @"Eject %s", currentDevice->value->device]
> +                                                  action:
> @selector(ejectDeviceMedia:)
> +                                           keyEquivalent: @""];
> +            [menu addItem: menuItem];
> +            [menuItem setRepresentedObject: deviceName];
> +            [menuItem autorelease];
> +        }
> +        currentDevice = currentDevice->next;

This doesn't look like it's going to cope very gracefully
with device hotplugging...

> +    }

Doesn't this leak the BlockInfoList* that qmp_query_block() returns?

> +}
> +
>  /*
>     Adds the Machine menu to the menu bar.
>     Has to be added separately because QEMU needs
> @@ -1115,6 +1221,7 @@ static void createMachineMenu()
>      [menuItem setSubmenu:menu];
>      [[NSApp mainMenu] insertItem: menuItem atIndex: 2]; // Insert after
> View menu
>      [[menu itemWithTitle: @"Resume"] setEnabled: NO];  // Disables the
> Resume menu item because it isn't needed right now.
> +    addDeviceMenuItems(menu);
>  }
>

thanks
-- PMM