Re: [Qemu-devel] [PATCH] Machine menu patch for Mac OS X

[Peter Maydell]

On 13 January 2015 at 01:49, Programmingkid <address@hidden> wrote:
> This patch adds a Machine menu to QEMU. This menu gives the user the ability 
> to easily work with floppy and CD image files.
>
> Features:
> Menu items to switch floppy and CD image files.
> Menu items to eject floppy and CD image files.
> Menu item to use /dev/cdrom.
> Verifies with the user before quitting QEMU by displaying a dialog box.
>
> Signed-off-by: John Arbuckle <address@hidden>

Hi. I'm afraid I couldn't get this patch to apply -- is it dependent
on one of your other patches?

Some comments below, anyway.

> +/* Pause the guest */
> +- (void)pauseQemu:(id)sender
> +{
> +    qmp_stop(NULL);
> +    [sender setEnabled: NO];
> +    [[[sender menu] itemWithTitle: @"Resume"] setEnabled: YES];
> +    [normalWindow setTitle: @"*** Paused ***"];
> +}
> +
> +/* Resume running the guest operating system */
> +- (void)resumeQemu: (id) sender
> +{
> +    qmp_cont(NULL);
> +    [sender setEnabled: NO];
> +    [[[sender menu] itemWithTitle: @"Pause"] setEnabled: YES];
> +    [normalWindow setTitle: @"QEMU"];

Isn't this changing of the title string going to conflict with the
changes that we make for mouse grab/ungrab?

> +}
> +
> +/* Eject the floppy0 disk */
> +- (void)ejectFloppy:(id)sender
> +{
> +    Error *err = NULL;
> +    qmp_eject("floppy0", false, false, &err);
> +    handleAnyDeviceErrors(err);
> +}
> +
> +/* Displays a dialog box asking the user to select a floppy image to load */
> +- (void)changeFloppy:(id)sender
> +{
> +    NSOpenPanel * open_panel;
> +    open_panel = [NSOpenPanel openPanel];
> +    [open_panel setCanChooseFiles: YES];
> +    [open_panel setAllowsMultipleSelection: NO];
> +    if([open_panel runModalForDirectory: nil file: nil] == NSOKButton) {
> +        Error *err = NULL;
> +        NSString * file = [[open_panel filenames] objectAtIndex: 0];
> +        qmp_change_blockdev("floppy0", [file cString], "raw", &err);
> +        handleAnyDeviceErrors(err);
> +    }
> +}

You don't know that the machine being emulated has a floppy drive
at all, or that it's called "floppy0"...

> +
> +// Ejects the cdrom
> +- (void)ejectCdrom:(id)sender
> +{
> +    Error *err = NULL;
> +    qmp_eject("ide1-cd0", false, false, &err);
> +    handleAnyDeviceErrors(err);
> +}
> +
> +/* Displays a dialog box asking the user to select a CD image to load */
> +- (void)changeCdrom:(id)sender
> +{
> +    NSOpenPanel * open_panel;
> +    open_panel = [NSOpenPanel openPanel];
> +    [open_panel setCanChooseFiles: YES];
> +    [open_panel setAllowsMultipleSelection: NO];
> +    if([open_panel runModalForDirectory: nil file: nil] == NSOKButton) {
> +        NSString * file = [[open_panel filenames] objectAtIndex: 0];
> +        Error *err = NULL;
> +        qmp_change_blockdev("ide1-cd0", [file cString], "raw", &err);
> +        handleAnyDeviceErrors(err);
> +    }
> +}

Similarly, the CD may not exist in the guest machine or may
not be called "ide1-cd0".

> +
> +/* Restarts QEMU */
> +- (void)restartQemu: (id) sender
> +{
> +    qemu_system_reset_request();
> +}
> +
> +/* Switches QEMU to use the real cdrom drive */
> +- (void)useRealCdrom: (id) sender
> +{
> +    Error *err = NULL;
> +    qmp_change_blockdev("ide1-cd0", "/dev/cdrom", "raw", &err);
> +    handleAnyDeviceErrors(err);
> +}
> +
> +/* Verifies if the user really wants to quit */
> +- (void)verifyQuit: (id) sender
> +{
> +    NSInteger response;
> +    response = NSRunAlertPanel(@"Quit?", @"Are you sure you want to quit?", 
> @"Cancel", @"Quit", nil);

We don't have an are-you-sure prompt for closing the QEMU window via
the red button, and we don't for the Quit menu option in the GTK UI either...

> +    if(response == NSAlertAlternateReturn)
> +        exit(EXIT_SUCCESS);

You should use qmp_quit(NULL) rather than just exit().

> +}
> +
>  @end
>
>
> @@ -1046,7 +1153,7 @@ int main (int argc, const char * argv[]) {
>      [menuItem 
> setKeyEquivalentModifierMask:(NSAlternateKeyMask|NSCommandKeyMask)];
>      [menu addItemWithTitle:@"Show All" 
> action:@selector(unhideAllApplications:) keyEquivalent:@""]; // Show All
>      [menu addItem:[NSMenuItem separatorItem]]; //Separator
> -    [menu addItemWithTitle:@"Quit QEMU" action:@selector(terminate:) 
> keyEquivalent:@"q"];
> +    [menu addItemWithTitle:@"Quit QEMU" action:@selector(verifyQuit:) 
> keyEquivalent:@"q"];
>      menuItem = [[NSMenuItem alloc] initWithTitle:@"Apple" action:nil 
> keyEquivalent:@""];
>      [menuItem setSubmenu:menu];
>      [[NSApp mainMenu] addItem:menuItem];
> @@ -1059,6 +1166,24 @@ int main (int argc, const char * argv[]) {
>      [menuItem setSubmenu:menu];
>      [[NSApp mainMenu] addItem:menuItem];
>
> +    /* Machine menu */
> +     menu = [[NSMenu alloc] initWithTitle: @"Machine"];
> +     [menu setAutoenablesItems: NO];
> +     [menu addItem: [[[NSMenuItem alloc] initWithTitle: @"Pause" action: 
> @selector(pauseQemu:) keyEquivalent: @""] autorelease]];
> +     [menu addItem: [[[NSMenuItem alloc] initWithTitle: @"Resume" action: 
> @selector(resumeQemu:) keyEquivalent: @""] autorelease]];
> +     [menu addItem: [NSMenuItem separatorItem]];
> +     [menu addItem: [[[NSMenuItem alloc] initWithTitle: @"Eject Floppy" 
> action: @selector(ejectFloppy:) keyEquivalent: @""] autorelease]];
> +     [menu addItem: [[[NSMenuItem alloc] initWithTitle: @"Change Floppy..." 
> action: @selector(changeFloppy:) keyEquivalent: @""] autorelease]];
> +     [menu addItem: [[[NSMenuItem alloc] initWithTitle: @"Eject cdrom" 
> action: @selector(ejectCdrom:) keyEquivalent: @""] autorelease]];
> +     [menu addItem: [[[NSMenuItem alloc] initWithTitle: @"Use cdrom 
> image..." action: @selector(changeCdrom:) keyEquivalent: @""] autorelease]];
> +     [menu addItem: [[[NSMenuItem alloc] initWithTitle: @"Use real cdrom 
> drive" action: @selector(useRealCdrom:) keyEquivalent: @""] autorelease]];
> +     [menu addItem: [NSMenuItem separatorItem]];
> +     [menu addItem: [[[NSMenuItem alloc] initWithTitle: @"Restart" action: 
> @selector(restartQemu:) keyEquivalent: @""] autorelease]];

In the GTK UI we call this "Reset". We also have a "Power Down" which would
probably be nice for consistency.

> +     menuItem = [[[NSMenuItem alloc] initWithTitle: @"Machine" action:nil 
> keyEquivalent:@""] autorelease];
> +    [menuItem setSubmenu:menu];
> +    [[NSApp mainMenu] addItem:menuItem];
> +        [[menu itemWithTitle: @"Resume"] setEnabled: NO];
> +
>      // Window menu
>      menu = [[NSMenu alloc] initWithTitle:@"Window"];
>      [menu addItem: [[[NSMenuItem alloc] initWithTitle:@"Minimize" 
> action:@selector(performMiniaturize:) keyEquivalent:@"m"] autorelease]]; // 
> Miniaturize
> @@ -1168,7 +1293,7 @@ void cocoa_display_init(DisplayState *ds, int 
> full_screen)
>          [NSApp activateIgnoringOtherApps: YES];
>          [[[NSApplication sharedApplication] delegate] toggleFullScreen: nil];
>      }
> -
> +
>      dcl = g_malloc0(sizeof(DisplayChangeListener));
>
>      // register vga output callbacks

Stray whitespace change.

thanks
-- PMM