Re: [Qemu-devel] [PATCH 3/3] chardev: add hotplug support.

[Eric Blake <eblake@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 2281 bytes --]

On 12/14/2012 05:17 AM, Paolo Bonzini wrote:
> Il 14/12/2012 10:38, Gerd Hoffmann ha scritto:
>> This patch adds chardev_add_file, chardev_add_tty and chardev_remove
>> monitor commands.
>>
>> chardev_add_file and chardev_add_tty expect an id and a path, they
>> create a file/tty chardev.
> 
> I'd rather avoid introducing this interface.  Using multiple commands is
> different from all previous examples, both HMP and QMP (including recent
> ones such as the NBD server).  It is also hard to extend, for example
> file descriptor passing is hard to retrofit.

File descriptor passing via magic /dev/fdset/nnn should probably already
work.  That said, a single command that uses a QAPI union, rather than
one command per source type, would be nicer from the UI perspective, and
it is the QMP UI perspective that libvirt is concerned about.

> 
> Perhaps you can define a QAPI union and slowly build it up?  Something
> that ultimately can become this:
> 
> { 'enum': 'ChardevFileMode', 'data':
>   # pty = console under Windows
>   # serial = tty under POSIX
>   [ 'file', 'pipe', 'parport', 'pty', 'serial' ] }
> 
> { 'enum: 'ChardevFileSource', 'data':
>   [ 'path', 'fd' ] }
> 
> { 'type': 'ChardevFile',
>   'data': {'source': 'string', 'source-type': 'ChardevFileSource',
>            'mode': 'ChardevFileMode'}}
> 
> { 'type': 'ChardevVC',
>   'data': {'width': 'int', 'height': 'int', '*characters': 'bool'}}
> 
> { 'type': 'ChardevSocket',
>   'data': {'addr': 'SocketAddress', '*server': 'bool',
>            '*wait': 'bool', '*nodelay': 'bool', '*telnet': 'bool'} }
> 
> # For future extensibility...
> { 'ChardevDummy', 'data': {} }
> 
> { 'union': 'ChardevBackend', 'data': {
>   'socket': 'ChardevSocket',
>   'udp': 'UDPSocketAddress',
>   'file': 'ChardevFile',
>   'null': 'ChardevDummy',
>   'msmouse': 'ChardevDummy',
>   'braille': 'ChardevDummy',
>   'stdio': 'ChardevDummy',
>   'vc': 'ChardevVC',
> 
>   # Solely for HMP usage.
>   'legacy': 'str'
> }
> 
> { 'command': 'chardev-add', 'data': {
>   'backend': 'ChardevBackend', 'id': 'str', '*mux': 'bool' } }

Yes, this looks nicer.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 619 bytes --]