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

[Markus Armbruster]

Kevin Wolf <address@hidden> writes:

> Am 16.02.2015 um 18:00 hat Programmingkid geschrieben:
>> 
>> 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,...
>
> The ID is user-defined, so no, no assumption about it is safe.  If you
> like, you can name your floppy drive 'ide0-cd1', your virtio harddisk
> 'floppy1' and your ATAPI CD-ROM drive 'virtio0'. But if you do that,
> I think it's reasonable to argue that it's your own fault that you get
> misleading menu entries. The default would yield the right results
> anyway.
>
> The only other option would be to start at the device tree, look for
> all known block devices with removable media, and then get the
> associated block backend from there. That would end up a lot more
> complicated, though.

The backend has a pointer to the frontend.  We could include the
frontend's canonical QOM path in some backend query- output.  Backends
that still haven't qdevified after all these years would make it warty,
however.

With the QOM path, you can use qom-get to find the device model name and
other frontend properties.  Example:

    { "execute": "qom-get", "arguments": { "path": "/machine/peripheral/ide2",  
"property": "type" } }
    {"return": "ide-cd"}
    { "execute": "qom-get", "arguments": { "path": "/machine/peripheral/ide2",  
"property": "parent_bus" } }
    {"return": "/machine/unattached/device[16]/ide.1"}
    { "execute": "qom-get", "arguments": { "path": "/machine/peripheral/ide2",  
"property": "unit" } }
    {"return": 0}

>From within QEMU, using the interfaces directly could be easier than
going through the QMP functions.  blk_get_attached_dev() returns the
backend.  Unfortunately, it returns void * rather than DeviceState *,
because block device qdevification is still incomplete (and has seen no
further progress in years).

[...]