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

[Peter Maydell]

On 14 June 2015 at 18:48, Programmingkid <address@hidden> wrote:
>
> On Jun 14, 2015, at 1:11 PM, Peter Maydell wrote:
>
>> On 18 May 2015 at 17:23, 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>
>>
>> I'm afraid this one still needs a bit more work on the
>> detail, though I think it's OK in general approach.
>>
>>> ---
>>> Replace NSRunAlertPanel() with QEMU_Alert().
>>> Free currentDevice variable after finished using it.
>>> Add "Removable Media" text to the top of devices menu items for easier
>>> identification.
>>> Replace depreciated code.
>>>
>>> ui/cocoa.m |  140
>>> ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>> 1 files changed, 140 insertions(+), 0 deletions(-)
>>>
>>> diff --git a/ui/cocoa.m b/ui/cocoa.m
>>> index 559058b..3acde50 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_5
>>> #define MAC_OS_X_VERSION_10_5 1050
>>> @@ -242,7 +243,27 @@ static int cocoa_keycode_to_qemu(int keycode)
>>>     return keymap[keycode];
>>> }
>>>
>>>
>>>
>>> +/* Displays an alert dialog box with the specified message */
>>> +static void QEMU_Alert(NSString *message)
>>> +{
>>> +    NSAlert *alert;
>>> +    alert = [NSAlert alertWithMessageText:message
>>> +                    defaultButton:@"OK"
>>> +                  alternateButton:nil
>>> +                      otherButton:nil
>>> +        informativeTextWithFormat:@""];
>>
>> This gives a deprecation warning on 10.10:
>>
>> /Users/pm215/src/qemu/ui/cocoa.m:250:22: warning:
>>      
>> 'alertWithMessageText:defaultButton:alternateButton:otherButton:informativeTextWithFormat:'
>> is deprecated: first
>>      deprecated in OS X 10.10 - Use -init instead [-Wdeprecated-declarations]
>>    alert = [NSAlert alertWithMessageText:message
>>                     ^
>> /System/Library/Frameworks/AppKit.framework/Headers/NSAlert.h:70:1: note:
>>      
>> 'alertWithMessageText:defaultButton:alternateButton:otherButton:informativeTextWithFormat:'
>> has been explicitly marked
>>      deprecated here
>> + (NSAlert *)alertWithMessageText:(NSString *)message
>> defaultButton:(NSString *)defaultButton alternateButton:(NSStri...
>> ^
>
> Apple is becoming depreciation crazy. Ok, will fix this problem.
>
>
>>> +    [alert runModal];
>>> +}
>>>
>>>
>>
>>> +/* 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();
>>> +        QEMU_Alert(@"Could not find drive!");
>>> +        return;
>>> +    }
>>> +
>>> +    /* Display the file open dialog */
>>> +    NSOpenPanel * openPanel;
>>> +    openPanel = [NSOpenPanel openPanel];
>>> +    [openPanel setCanChooseFiles: YES];
>>> +    [openPanel setAllowsMultipleSelection: NO];
>>> +    [openPanel setAllowedFileTypes: [NSArray arrayWithObjects: @"iso",
>>> @"cdr", @"img", @"dmg", nil]];
>>
>> This list is too short (no qcow2, among other things) and doesn't match
>> the one we use for the initial image. Ideally we should use the same
>> code for "let user pick initial disk image" and "let user pick new
>> image for this drive".
>
> Is this the complete list you want: iso, cdr, img, dmg, cow, qcow, qcow2,
> vmdk, cloop, vpc, vdi

We want the same list as we have already for the other file-open.
>>
>> currentDevice will usually be NULL here because you've used
>> it as your iterator variable. In any case this
>> isn't the right way to free the pointer you get back from
>> qmp_query_block() -- you need qapi_free_BlockInfoList().
>
> qapi_free_BlockInfoList(currentDevice);
> This is what you want?

Yes, as long as you fix the iterator variable not to be
currentDevice...

-- PMM