On 11/12/20 11:03, Stephen Finucane wrote:
Design discussion for interested parties. As discussed during the nova team meeting today, we're looking at providing support for virtio-based input devices in nova, given benefits w.r.t. performance and security (no emulated USB bus!). The current proposal I have for this is to introduce a new image metadata property, 'hw_input_bus', and extend the existing 'hw_pointer_model' image metadata property, which currently only accepts a 'usbtablet' value, to accept 'mouse' or 'tablet' values. This means you'd end up with a matrix something like so:
+------------------+--------------+------------------------+ | hw_pointer_model | hw_input_bus | Result | +------------------|--------------+------------------------+ | - | - | PS2-based mouse [*] | | usbtablet | - | USB-based tablet | | usbtablet | usb | USB-based tablet | | usbtablet | virtio | **ERROR** | | mouse | - | USB-based mouse | | mouse | usb | USB-based mouse | | mouse | virtio | virtio-based mouse | | tablet | - | USB-based tablet | | tablet | usb | USB-based tablet | | tablet | virtio | virtio-based tablet | +------------------+--------------+------------------------+
[*] libvirt adds these by default on x86 hosts
dansmith noted that the only reason to select the 'mouse' pointer model nowadays is for compatibility, and something like a virtio-based mouse didn't really make sense. That being the case, I agree that this change is likely more complex that it needs to be.
+1, agree that having several user choices for pointer + bus that result in invalid or nonsensical combinations would not be a positive UX.
We do, however, disagree on the remedy. dansmith's idea is to drop the 'hw_input_bus' image metadata property entirely and simply add a new 'virtiotablet' value to 'hw_pointer_model' instead. This still leaves the question of what bus we should use for keyboards, and his suggestion there is to extrapolate out and use virtio for keyboards if 'hw_pointer_model=virtiotablet' is specified and presumably USB if 'hw_pointer_model=usbtablet'.
Needless to say, I don't like this idea and prefer we took another tack and kept 'hw_input_bus' but didn't build on 'hw_pointer_model' and instead "deprecated" it. We can't really ever remove the an image metadata property, since that would be a breaking upgrade, which means we'll eventually be left with effectively dead code to maintain forever. However, I don't think that's a big deal. 'hw_pointer_model=usbtablet' is already on a path to obsolescence as the Q35 machine type slowly becomes the default on x86 hosts and the use of non-x86 hosts grows since neither support PS2 and must use a USB-based input device. In addition, I think the extrapolation of 'virtiotablet' to mean also virtio-based keyboard is unclear and leaves a gaping hole w.r.t. requesting USB-based keyboards on non-AArch64 hosts (where it's currently added by default), since we don't currently do this extrapolation and introducing it would be breaking change on x86 hosts (instances would suddenly switch from PS2-based keyboards to USB-based ones).
If I understand correctly, we do choose the "best fit" keyboard based on architecture, independent of the requested hw_pointer_model, today. It feels like it would be simpler to continue doing that and add a virtio choice to hw_pointer_model and let the driver pick the "best" keyboard to go along with that. Is it your view that having the driver choose a virtio keyboard if hw_pointer_model=virtiotablet is inconsistent with the existing "best fit" keyboard selection behavior? From what I understand, the pointer is the only user selection we can guarantee and the keyboard is architecture dependent. If we "deprecated" hw_pointer_model and introduced hw_input_bus, how does the user know which thing (pointer or keyboard) they are specifying? If users can't actually choose the keyboard and can only choose the pointer, wouldn't presenting a selection of a generic "hw_input_bus" be more confusing than hw_pointer_model? Best, -melanie
We need to decide what approach to go for before I rework this. If anyone has input, particularly operators that think they'd use this feature, I'd love to hear it so that I can t̶e̶l̶l̶ ̶d̶a̶n̶s̶m̶i̶t̶h̶ ̶t̶o̶ ̶s̶h̶o̶v̶e̶ ̶i̶t̶ come to the best possible solution ;-) Feel free to either reply here or on the review [1].
Cheers, Stephen