Re: [Qemu-devel] [PATCH v3] ui/cocoa.m: Machine menu patch for Mac OS X

[Programmingkid]

On Feb 16, 2015, at 11:22 AM, Kevin Wolf wrote:

> Am 16.02.2015 um 17:12 hat Programmingkid geschrieben:
>> 
>> On Feb 16, 2015, at 10:42 AM, Kevin Wolf wrote:
>> 
>>> Am 16.02.2015 um 16:31 hat Programmingkid geschrieben:
>>>> 
>>>> On Feb 16, 2015, at 5:08 AM, Kevin Wolf wrote:
>>>> 
>>>>> Am 14.02.2015 um 03:28 hat Peter Maydell geschrieben:
>>>>>> On 14 February 2015 at 01:43, Programmingkid <address@hidden> wrote:
>>>>>>> Added 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>
>>>>>> 
>>>>>> Stefan, Kevin -- could you review the bits of this patch
>>>>>> which determine whether the machine has a floppy/cdrom
>>>>>> drive and if so let the user insert/inject it, please?
>>>>>> (that's the emulatorHasDevice and ejectFloppy/changeFloppy
>>>>>> functions, mostly). I don't know the block layer APIs so
>>>>>> I can't really say if this patch is doing it in the best/
>>>>>> non-deprecated/etc way or not...
>>>>> 
>>>>> Well, it's trying to detect the floppy/cdrom device by comparing string
>>>>> with default IDs that can be overridden by the user, so no, that's
>>>>> probably far from the best way to do it. The code also doesn't consider
>>>>> that you could have more than one floppy or cdrom drive.
>>>>> 
>>>>> The correct way is probably to just display any removable block device,
>>>>> and ideally also to implement some notifiers to deal with hotplug.
>>>> 
>>>> Could you provide examples?
>>> 
>>> You already use qmp_query_block(), so you get all existing devices.
>>> Currently you filter for everything that has a name that starts with
>>> either 'floppy' or 'cdrom'. You could filter for info->removable == true
>>> instead.
>> 
>> 
>>> 
>>> Of course, you'd have to do this while building up the menu, so that the
>>> menu will contain dynamically generated entries for every device.
>>> 
>>> Hotplug is a bit trickier, I guess. If you can make sure that qemu
>>> doesn't crash if the device for a menu entry has gone away, that would
>>> probably be acceptable for the start.
>> 
>> So what you want me to do is loop thru each entry in the BlockInfoList 
>> (returned by qmp_query_block() ) and see if it is removable. Then just add a 
>> menu item for the device name. If I did that we would have menu items like 
>> "ide1-cd0" and "floppy0". The menu items would not have intuitive names that 
>> the user would be able to understand easily. Sorry but your idea is not user 
>> friendly. I did look at the type field of the BlockInfoList structure and it 
>> is only set to "unknown". Maybe a compromise would be the solution. We set 
>> the type field to the common name of the device. "ide1-cd0" would have a 
>> type field set to "cdrom". Then set the menu item to this type field's 
>> value. 
> 
> You could still apply some translation table to the menu entry string,
> like:
> 
> floppy0     => Floppy drive A
> floppy1     => Floppy drive B
> ide0-cd0    => IDE CD-ROM (Primary Master)
> ide0-cd1    => IDE CD-ROM (Primary Slave)
> ide1-cd0    => IDE CD-ROM (Secondary Master)
> ide1-cd1    => IDE CD-ROM (Secondary Slave)
> 
> And everything else just gets the block device ID in the menu name. Then
> you get user friendly menu entry names where we have an idea what the
> device might be, but still let the device show up with an identifiable
> name when we don't.
> 
> Because having a CD-ROM drive not show up at all is definitely even less
> user friendly than having a cryptic name for it.

This is a good start, but still needs more work. Is it safe to assume all cdrom 
drives in QEMU will have "cd" in its name? scsi0-cd0, ide0-cd0,...

I will still give every drive that has its removable property set to true a 
menu item. Is it ok to translate anything that has "-cd#" in its name as cdrom 
instead of using a translation table? A search algorithm I had in mind is look 
for anything that has a "cd" after a hyphen. If it is found, translate it to 
"cdrom". If there are more than one of these, then append a number to cdrom 
like "cdrom 1". Does that sound good?