Re: [Qemu-devel] [PATCH] ui/cocoa.m: Add Mount image file menu item

[Programmingkid]

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

> Programmingkid <address@hidden> writes:
> 
>> On Sep 8, 2015, at 12:17 PM, Peter Maydell wrote:
>> 
>>> On 2 September 2015 at 01:56, Programmingkid
>>> <address@hidden> 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 <address@hidden>
>>>> 
>>>> ---
>>>> 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.