Message ID | 153FBDBF-2650-4D9E-BFC1-3D838BCC30F3@gmail.com |
---|---|
State | New |
Headers | show |
Only reviewing around qmp_device_add() and such, ignoring the GUI part. 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> > > --- > Detects if the user actually specified the -usb option. > Removed the sendMonitorCommand() function. > Replaced a lot of code with C interface equivalent of monitor commands > drive_add and device_add. > > ui/cocoa.m | 228 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++- > 1 files changed, 227 insertions(+), 1 deletions(-) > > diff --git a/ui/cocoa.m b/ui/cocoa.m > index 334e6f6..df1faea 100644 > --- a/ui/cocoa.m > +++ b/ui/cocoa.m > @@ -31,6 +31,8 @@ > #include "sysemu/sysemu.h" > #include "qmp-commands.h" > #include "sysemu/blockdev.h" > +#include "include/monitor/qdev.h" > +#include "hw/boards.h" > > #ifndef MAC_OS_X_VERSION_10_5 > #define MAC_OS_X_VERSION_10_5 1050 > @@ -52,6 +54,8 @@ > #endif > > #define cgrect(nsrect) (*(CGRect *)&(nsrect)) > +#define USB_DISK_ID "USB_DISK" > +#define EJECT_IMAGE_FILE_TAG 2099 > > typedef struct { > int width; > @@ -829,6 +833,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 +1132,179 @@ 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; > + char *idString, *fileName, *driveOptions, *fileNameHint; > + > + /* Create the file name hint */ > + NSString *stringBuffer; > + const int fileNameHintSize = 10; > + stringBuffer = [file lastPathComponent]; > + stringBuffer = [stringBuffer stringByDeletingPathExtension]; > + if([stringBuffer length] > fileNameHintSize) { > + stringBuffer = [stringBuffer substringToIndex: fileNameHintSize]; > + } > + fileNameHint = g_strdup_printf("%s", > + [stringBuffer cStringUsingEncoding: NSASCIIStringEncoding]); > + > + idString = g_strdup_printf("%s_%s_%d", USB_DISK_ID, fileNameHint, > + usbDiskCount++); Indentation is off, long line. We don't use CamelCase for variable names in QEMU. See CODING_STYLE, section 3. Naming. More of the same elsewhere. Your system-generated ID can theoretically clash with a user-specified ID. We should reserve a name space for system-generated IDs. Related: our discussion Re: Should we auto-generate IDs? Anyway, wider issue, not something this patch can solve. > + > + fileName = g_strdup_printf("%s", > + [file cStringUsingEncoding: NSASCIIStringEncoding]); > + > + /* drive_add equivalent code */ > + driveOptions = g_strdup_printf("if=none,id=%s,file=%s", idString, > + fileName); > + DriveInfo *dinfo; > + QemuOpts *opts; > + MachineClass *mc; No declarations in the middle of blocks, please. See CODING_STYLE section 5. Declarations. More of the same elsewhere. > + > + opts = drive_def(driveOptions); Breaks when fileName contains a comma. Fine example of why formatting stuff only to parse it apart again is a bad idea. Use drive_add() instead. Roughly like opts = drive_add(IF_NONE, -1, file_name, ""); You could've found this yourself easily had you slowed down a bit to examine how we create drive QemuOpts elsewhere. > + if (!opts) { > + QEMU_Alert(@"Error: could not create QemuOpts object!"); > + return; > + } > + > + mc = MACHINE_GET_CLASS(current_machine); > + dinfo = drive_new(opts, mc->block_default_type); > + if (!dinfo) { > + qemu_opts_del(opts); > + QEMU_Alert(@"Error: could not create DriveInfo object!"); > + return; > + } The real error messages will end up on stderr. The proper interface to use will be blockdev_init(), or maybe qmp_blockdev_add(). "Will be" because they're still being developed, and using them in this state might be too much to ask of you. > + > + /* device_add equivalent code */ > + > + /* Create the QDict object */ > + QDict * qdict; > + qdict = qdict_new(); > + qdict_set_default_str(qdict, "driver", "usb-storage"); > + qdict_set_default_str(qdict, "id", idString); > + qdict_set_default_str(qdict, "drive", idString); Using the same ID for two different things happens to work at this time, but it's confusing, and best avoided. > + > + QObject *ret_data; > + ret_data = g_malloc(1); /* This is just to silence a warning */ Must be ret_data = NULL. > + > + Error *errp = NULL; > + qmp_device_add(qdict, &ret_data, &errp); > + handleAnyDeviceErrors(errp); > + [self updateEjectImageMenuItems]; > + > + g_free(qdict); > + g_free(fileName); > + g_free(idString); > + g_free(driveOptions); > + g_free(ret_data); You leak most of these on your error returns. Didn't check whether you leak anything here. > + } > +} > + > +/* Removes an image file from QEMU */ > +- (void)ejectImageFile:(id) sender > +{ > + NSString *imageFileID; > + > + imageFileID = [sender representedObject]; > + if (imageFileID == nil) { > + NSBeep(); > + QEMU_Alert(@"Could not find image file's ID!"); > + return; > + } > + > + Error *errp = NULL; > + qmp_device_del([imageFileID cStringUsingEncoding: NSASCIIStringEncoding], > + &errp); > + handleAnyDeviceErrors(errp); > + [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 > > > @@ -1338,10 +1518,52 @@ static void add_console_menu_entries(void) > } > } > > +/* Determines if USB is available */ > +static bool usbAvailable(void) > +{ > + /* Look thru all the arguments sent to QEMU for '-usb' */ > + int index; > + for (index = 0; index < gArgc; index++) { > + if(strcmp(gArgv[index], "-usb") == 0) { > + return true; > + } > + } > + return false; > +} -usb is just one way to enable USB. There's also --usb, and variations on --machine usb=on. USB may even be enabled by default. You need to search QOM for USB sockets. > + > +/* Adds menu items that can mount an image file like a usb flash drive */ > +static void addImageMountingMenus(NSMenu *menu) > +{ > + [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]; Okay, one drive-by GUI remark: We eject media, we unplug devices. Ejecting image files makes no sense to me :) > + > + /* 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, the "No items" menu item */ > + NSMenu *submenu; > + submenu = [NSMenu new]; > + [ejectMenuItem setSubmenu: submenu]; > + [submenu addItem: emptyMenuItem]; > + [submenu autorelease]; > +} > + > /* Make menu items for all removable devices. > * Each device is given an 'Eject' and 'Change' menu item. > */ > -static void addRemovableDevicesMenuItems() > +static void addRemovableDevicesMenuItems(void) > { > NSMenu *menu; > NSMenuItem *menuItem; > @@ -1402,6 +1624,10 @@ static void addRemovableDevicesMenuItems() > currentDevice = currentDevice->next; > } > qapi_free_BlockInfoList(pointerToFree); > + > + if(usbAvailable() == true){ > + addImageMountingMenus(menu); > + } > } > > void cocoa_display_init(DisplayState *ds, int full_screen)
On Sep 11, 2015, at 3:40 AM, Markus Armbruster wrote: > Only reviewing around qmp_device_add() and such, ignoring the GUI part. > > 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> >> >> --- >> Detects if the user actually specified the -usb option. >> Removed the sendMonitorCommand() function. >> Replaced a lot of code with C interface equivalent of monitor commands >> drive_add and device_add. >> >> ui/cocoa.m | 228 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++- >> 1 files changed, 227 insertions(+), 1 deletions(-) >> >> diff --git a/ui/cocoa.m b/ui/cocoa.m >> index 334e6f6..df1faea 100644 >> --- a/ui/cocoa.m >> +++ b/ui/cocoa.m >> @@ -31,6 +31,8 @@ >> #include "sysemu/sysemu.h" >> #include "qmp-commands.h" >> #include "sysemu/blockdev.h" >> +#include "include/monitor/qdev.h" >> +#include "hw/boards.h" >> >> #ifndef MAC_OS_X_VERSION_10_5 >> #define MAC_OS_X_VERSION_10_5 1050 >> @@ -52,6 +54,8 @@ >> #endif >> >> #define cgrect(nsrect) (*(CGRect *)&(nsrect)) >> +#define USB_DISK_ID "USB_DISK" >> +#define EJECT_IMAGE_FILE_TAG 2099 >> >> typedef struct { >> int width; >> @@ -829,6 +833,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 +1132,179 @@ 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; >> + char *idString, *fileName, *driveOptions, *fileNameHint; >> + >> + /* Create the file name hint */ >> + NSString *stringBuffer; >> + const int fileNameHintSize = 10; >> + stringBuffer = [file lastPathComponent]; >> + stringBuffer = [stringBuffer stringByDeletingPathExtension]; >> + if([stringBuffer length] > fileNameHintSize) { >> + stringBuffer = [stringBuffer substringToIndex: fileNameHintSize]; >> + } >> + fileNameHint = g_strdup_printf("%s", >> + [stringBuffer cStringUsingEncoding: NSASCIIStringEncoding]); >> + >> + idString = g_strdup_printf("%s_%s_%d", USB_DISK_ID, fileNameHint, >> + usbDiskCount++); > > Indentation is off, long line. I assume you are talking about the "usbDiskCount++);" part. Did you want it more right justified? > We don't use CamelCase for variable names in QEMU. See CODING_STYLE, > section 3. Naming. Good catch. > > More of the same elsewhere. > > Your system-generated ID can theoretically clash with a user-specified > ID. The gui code does run at the start of QEMU. But the user could have specified an ID at the command line. Given how long the auto-generated ID is, I think there is little probability that would happen. I can make the auto-generated ID impossible for the user to use by using a '#' character at the start. > We should reserve a name space for system-generated IDs. Related: > our discussion Re: Should we auto-generate IDs? Anyway, wider issue, > not something this patch can solve. I don't like mixing topics in emails but what was the outcome of the fifth great discussion on auto-generated ID's? Are you or another maintainer going to use someone's patch? > >> + >> + fileName = g_strdup_printf("%s", >> + [file cStringUsingEncoding: NSASCIIStringEncoding]); >> + >> + /* drive_add equivalent code */ >> + driveOptions = g_strdup_printf("if=none,id=%s,file=%s", idString, >> + fileName); >> + DriveInfo *dinfo; >> + QemuOpts *opts; >> + MachineClass *mc; > > No declarations in the middle of blocks, please. See CODING_STYLE > section 5. Declarations. Another good catch. > > More of the same elsewhere. > >> + >> + opts = drive_def(driveOptions); > > Breaks when fileName contains a comma. Thanks for catching it. > Fine example of why formatting > stuff only to parse it apart again is a bad idea. Use drive_add() > instead. Roughly like > > opts = drive_add(IF_NONE, -1, file_name, ""); I tired using drive_add() so many times. It didn't work. Using 'info block' in the monitor is how I checked. > You could've found this yourself easily had you slowed down a bit to > examine how we create drive QemuOpts elsewhere. > >> + if (!opts) { >> + QEMU_Alert(@"Error: could not create QemuOpts object!"); >> + return; >> + } >> + >> + mc = MACHINE_GET_CLASS(current_machine); >> + dinfo = drive_new(opts, mc->block_default_type); >> + if (!dinfo) { >> + qemu_opts_del(opts); >> + QEMU_Alert(@"Error: could not create DriveInfo object!"); >> + return; >> + } > > The real error messages will end up on stderr. The proper interface to > use will be blockdev_init(), or maybe qmp_blockdev_add(). "Will be" > because they're still being developed, and using them in this state > might be too much to ask of you. qmp_blockdev_add() has that BlockdevOptions type that doesn't look easy to use. blockdev_init() looks usable. The only thing to find out is what does it expect to be in its QDict parameter. There doesn't seem to be a problem with drive_new(). There is no indication that the function is depreciated. > >> + >> + /* device_add equivalent code */ >> + >> + /* Create the QDict object */ >> + QDict * qdict; >> + qdict = qdict_new(); >> + qdict_set_default_str(qdict, "driver", "usb-storage"); >> + qdict_set_default_str(qdict, "id", idString); >> + qdict_set_default_str(qdict, "drive", idString); > > Using the same ID for two different things happens to work at this time, > but it's confusing, and best avoided. Didn't know this was a problem. I just followed the example from here: https://wiki.ubuntu.com/QemuDiskHotplug#Hotplug_USB_Disk] What do you suggest I use for "drive"? > >> + >> + QObject *ret_data; >> + ret_data = g_malloc(1); /* This is just to silence a warning */ > > Must be ret_data = NULL. Excellent idea. > >> + >> + Error *errp = NULL; >> + qmp_device_add(qdict, &ret_data, &errp); >> + handleAnyDeviceErrors(errp); >> + [self updateEjectImageMenuItems]; >> + >> + g_free(qdict); >> + g_free(fileName); >> + g_free(idString); >> + g_free(driveOptions); >> + g_free(ret_data); > > You leak most of these on your error returns. A goto statement will fix this. > Didn't check whether you leak anything here. > >> + } >> +} >> + >> +/* Removes an image file from QEMU */ >> +- (void)ejectImageFile:(id) sender >> +{ >> + NSString *imageFileID; >> + >> + imageFileID = [sender representedObject]; >> + if (imageFileID == nil) { >> + NSBeep(); >> + QEMU_Alert(@"Could not find image file's ID!"); >> + return; >> + } >> + >> + Error *errp = NULL; >> + qmp_device_del([imageFileID cStringUsingEncoding: NSASCIIStringEncoding], >> + &errp); >> + handleAnyDeviceErrors(errp); >> + [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 >> >> >> @@ -1338,10 +1518,52 @@ static void add_console_menu_entries(void) >> } >> } >> >> +/* Determines if USB is available */ >> +static bool usbAvailable(void) >> +{ >> + /* Look thru all the arguments sent to QEMU for '-usb' */ >> + int index; >> + for (index = 0; index < gArgc; index++) { >> + if(strcmp(gArgv[index], "-usb") == 0) { >> + return true; >> + } >> + } >> + return false; >> +} > > -usb is just one way to enable USB. There's also --usb, and variations > on --machine usb=on. USB may even be enabled by default. > > You need to search QOM for USB sockets. Do you know of example code I could study that does this? > >> + >> +/* Adds menu items that can mount an image file like a usb flash drive */ >> +static void addImageMountingMenus(NSMenu *menu) >> +{ >> + [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]; > > Okay, one drive-by GUI remark: We eject media, we unplug devices. > Ejecting image files makes no sense to me :) I think eject conveys the message of removing something from the system. Just thinking about a menu item called "unplug" makes me cringe. > >> + >> + /* 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, the "No items" menu item */ >> + NSMenu *submenu; >> + submenu = [NSMenu new]; >> + [ejectMenuItem setSubmenu: submenu]; >> + [submenu addItem: emptyMenuItem]; >> + [submenu autorelease]; >> +} >> + >> /* Make menu items for all removable devices. >> * Each device is given an 'Eject' and 'Change' menu item. >> */ >> -static void addRemovableDevicesMenuItems() >> +static void addRemovableDevicesMenuItems(void) >> { >> NSMenu *menu; >> NSMenuItem *menuItem; >> @@ -1402,6 +1624,10 @@ static void addRemovableDevicesMenuItems() >> currentDevice = currentDevice->next; >> } >> qapi_free_BlockInfoList(pointerToFree); >> + >> + if(usbAvailable() == true){ >> + addImageMountingMenus(menu); >> + } >> } >> >> void cocoa_display_init(DisplayState *ds, int full_screen) Thank you very much for helping.
diff --git a/ui/cocoa.m b/ui/cocoa.m index 334e6f6..df1faea 100644 --- a/ui/cocoa.m +++ b/ui/cocoa.m @@ -31,6 +31,8 @@ #include "sysemu/sysemu.h" #include "qmp-commands.h" #include "sysemu/blockdev.h" +#include "include/monitor/qdev.h" +#include "hw/boards.h" #ifndef MAC_OS_X_VERSION_10_5 #define MAC_OS_X_VERSION_10_5 1050 @@ -52,6 +54,8 @@ #endif #define cgrect(nsrect) (*(CGRect *)&(nsrect)) +#define USB_DISK_ID "USB_DISK" +#define EJECT_IMAGE_FILE_TAG 2099 typedef struct { int width; @@ -829,6 +833,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 +1132,179 @@ 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; + char *idString, *fileName, *driveOptions, *fileNameHint; + + /* Create the file name hint */ + NSString *stringBuffer; + const int fileNameHintSize = 10; + stringBuffer = [file lastPathComponent]; + stringBuffer = [stringBuffer stringByDeletingPathExtension]; + if([stringBuffer length] > fileNameHintSize) { + stringBuffer = [stringBuffer substringToIndex: fileNameHintSize]; + } + fileNameHint = g_strdup_printf("%s", + [stringBuffer cStringUsingEncoding: NSASCIIStringEncoding]); + + idString = g_strdup_printf("%s_%s_%d", USB_DISK_ID, fileNameHint, + usbDiskCount++); + + fileName = g_strdup_printf("%s", + [file cStringUsingEncoding: NSASCIIStringEncoding]); + + /* drive_add equivalent code */ + driveOptions = g_strdup_printf("if=none,id=%s,file=%s", idString, + fileName); + DriveInfo *dinfo; + QemuOpts *opts; + MachineClass *mc; + + opts = drive_def(driveOptions); + if (!opts) { + QEMU_Alert(@"Error: could not create QemuOpts object!"); + return; + } + + mc = MACHINE_GET_CLASS(current_machine); + dinfo = drive_new(opts, mc->block_default_type); + if (!dinfo) { + qemu_opts_del(opts); + QEMU_Alert(@"Error: could not create DriveInfo object!"); + return; + } + + /* device_add equivalent code */ + + /* Create the QDict object */ + QDict * qdict; + qdict = qdict_new(); + qdict_set_default_str(qdict, "driver", "usb-storage"); + qdict_set_default_str(qdict, "id", idString); + qdict_set_default_str(qdict, "drive", idString); + + QObject *ret_data; + ret_data = g_malloc(1); /* This is just to silence a warning */ + + Error *errp = NULL; + qmp_device_add(qdict, &ret_data, &errp); + handleAnyDeviceErrors(errp); + [self updateEjectImageMenuItems]; + + g_free(qdict); + g_free(fileName); + g_free(idString); + g_free(driveOptions); + g_free(ret_data); + } +} + +/* Removes an image file from QEMU */ +- (void)ejectImageFile:(id) sender +{ + NSString *imageFileID; + + imageFileID = [sender representedObject]; + if (imageFileID == nil) { + NSBeep(); + QEMU_Alert(@"Could not find image file's ID!"); + return; + } + + Error *errp = NULL; + qmp_device_del([imageFileID cStringUsingEncoding: NSASCIIStringEncoding], + &errp); + handleAnyDeviceErrors(errp); + [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 @@ -1338,10 +1518,52 @@ static void add_console_menu_entries(void) } } +/* Determines if USB is available */ +static bool usbAvailable(void) +{ + /* Look thru all the arguments sent to QEMU for '-usb' */ + int index; + for (index = 0; index < gArgc; index++) { + if(strcmp(gArgv[index], "-usb") == 0) { + return true; + } + } + return false; +} + +/* Adds menu items that can mount an image file like a usb flash drive */ +static void addImageMountingMenus(NSMenu *menu) +{ + [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, the "No items" menu item */ + NSMenu *submenu; + submenu = [NSMenu new]; + [ejectMenuItem setSubmenu: submenu]; + [submenu addItem: emptyMenuItem]; + [submenu autorelease]; +} + /* Make menu items for all removable devices. * Each device is given an 'Eject' and 'Change' menu item. */ -static void addRemovableDevicesMenuItems() +static void addRemovableDevicesMenuItems(void) { NSMenu *menu; NSMenuItem *menuItem; @@ -1402,6 +1624,10 @@ static void addRemovableDevicesMenuItems() currentDevice = currentDevice->next; } qapi_free_BlockInfoList(pointerToFree); + + if(usbAvailable() == true){ + addImageMountingMenus(menu); + } } void cocoa_display_init(DisplayState *ds, int full_screen)
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> --- Detects if the user actually specified the -usb option. Removed the sendMonitorCommand() function. Replaced a lot of code with C interface equivalent of monitor commands drive_add and device_add. ui/cocoa.m | 228 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++- 1 files changed, 227 insertions(+), 1 deletions(-)