[nova] How best to provide virtio-based input devices?
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. 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). 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 [1] https://review.opendev.org/#/c/756552/
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 | +------------------+--------------+------------------------+
Yeah, my complaint here is that there are too many ways to describe things that can't possibly work, in addition to the things you might ask for that aren't supported by your virt driver (which are unavoidable).
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. 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.
Just for clarification, I want to drop the _new_ parameter from the _proposal_, as in not add it in the first place.
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'.
Indeed. That leaves you with no new image metadata keys, no new object structural changes, deprecation implications thereof, and just a single new value for the existing key of "virtiotablet" (or even just "virtio"). I don't really care whether or not we change the usb case from how it works today. I think most people that care a lot about how the graphical console works are probably looking for pointer support more than anything else.
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.
Right, so this is what I don't like. Users will come along trying to figure out how to get either a USB tablet or a virtio pointer, and be presented with a bunch of options. For years, google will return them results that point to the existing key that omit the new option. If they google for updated docs for that key, they won't find a new virtio-based value that they can use, but will hopefully find the explanation and truth table of how to get what they want, depending on how new or old their cloud is, what virt driver it's running, how new that virt driver is, and what platform they're using. And of course, we have to decide what to do if they specify both keys if they're confused by the forumla for deciding which to use. I'd much prefer to retain compatibility with images from the last ten years (or however long we've allowed this to be specified) and just add another value for the new model. Obviously "hw_pointer_model" isn't the nicest name, given that we're using it to determine both input devices, but I don't think that wrecking google results, docs, and people's images is really worth it. It's important to consider that people do have images and code that work on multiple clouds running various versions. It seems a lot nicer to me to just keep using the key that works everywhere, extended for the new value where available. --Dan
On Thu, 2020-11-12 at 11:50 -0800, Dan Smith 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 | +------------------+--------------+------------------------+
Yeah, my complaint here is that there are too many ways to describe things that can't possibly work, in addition to the things you might ask for that aren't supported by your virt driver (which are unavoidable).
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. 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.
Just for clarification, I want to drop the _new_ parameter from the _proposal_, as in not add it in the first place.
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'.
Indeed. That leaves you with no new image metadata keys, no new object structural changes, deprecation implications thereof, and just a single new value for the existing key of "virtiotablet" (or even just "virtio"). I don't really care whether or not we change the usb case from how it works today. I think most people that care a lot about how the graphical console works are probably looking for pointer support more than anything else.
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.
Right, so this is what I don't like. Users will come along trying to figure out how to get either a USB tablet or a virtio pointer, and be presented with a bunch of options. For years, google will return them results that point to the existing key that omit the new option. If they google for updated docs for that key, they won't find a new virtio-based value that they can use, but will hopefully find the explanation and truth table of how to get what they want, depending on how new or old their cloud is, what virt driver it's running, how new that virt driver is, and what platform they're using. And of course, we have to decide what to do if they specify both keys if they're confused by the forumla for deciding which to use. I'd much prefer to retain compatibility with images from the last ten years (or however long we've allowed this to be specified) and just add another value for the new model. Obviously "hw_pointer_model" isn't the nicest name, given that we're using it to determine both input devices, but I don't think that wrecking google results, docs, and people's images is really worth it. It's important to consider that people do have images and code that work on multiple clouds running various versions. It seems a lot nicer to me to just keep using the key that works everywhere, extended for the new value where available. This is all very misleading. Deprecation does not equal deprecation for removal. We can and will continue to support the old 'hw_pointer_model' option until such a time as we provide a way to automatically migrate images (and flavours) from an older format to a newer one, or forever if we never do this. Old Google results, docs, and people's images will all remain valid. Nothing is wrecked and nothing actually changes until you decide to use the new, easily understood image metadata property. There's also a suggestion here that 'hw_pointer_model' is a widely used, well understood image metadata property. The three pages of results I see on Google, most of which are from clones of the nova git repo, suggest otherwise. I'd wager that few if any users are actually using this, given this is also configurable at a host level via the '[DEFAULT] pointer_model' option, which defaults to a USB-based tablet. There isn't a lot to save here. Conversely, there something to lose by building upon rotten foundations. Ultimately, 'hw_input_bus' is self-explanatory and aligns very well with existing properties like 'hw_disk_bus'. 'hw_pointer_model' with the "spooky behaviour at a distance" behaviour that's been proposed is neither. I think the 5 lines of code necessary to add a 'hw_input_bus' field to the 'ImageMetadataProps' object and handle potential conflicts with the legacy 'hw_pointer_model' are not worth worrying about, and would be a net positive once the UX improvements are taken into account. Can we just do this? Stephen --Dan
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
On Thu, 2020-11-12 at 17:09 -0800, melanie witt wrote: 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. Not quite. libvirt on x86 will automatically add a PS2 mouse and keyboard (even if you request something else) because that's what QEMU does. On all other platforms (to the best of my knowledge, based on Kashyap quizzing a handful of multi-arch libvirt devs), this must be done manually. We currently only do this for AArch64, through the non- configurable addition of a USB keyboard [1]. There is currently no way to request e.g. a USB or virtio keyboard on any architecture. [1] https://github.com/openstack/nova/commit/b92e3bc95fd 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?
My concerns are twofold. Firstly, having the value of 'hw_pointer_model' influence the keyboard bus is poor UX, since this "feature" is not at all apparent from the property name. Secondly, 'hw_pointer_model' is a poorly designed property, given it configured the bus as well as the model, munging the two. To be clear, I agree that having different buses for keyboard and pointer makes no sense. There's no reason they shouldn't be the same. I'm just saying that configuring the bus for input devices via an appropriately named 'hw_input_bus' image metadata property makes much more sense that maybe getting one configured magically based on the "pointer model" in use.
From what I understand, the pointer is the only user selection we can guarantee and the keyboard is architecture dependent.
Again, not really. There isn't an analog for pointer vs. mouse when it comes to keyboards. The keyboard isn't architecture dependent. What is architecture dependent is whether you get a keyboard and mouse by default (yes on x86, in the form of a PS2 mouse and keyboard, and no for everyone else).
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?
They can choose the _bus_ that the keyboard uses. There's an assumption here that if you request a graphics device (VNC, SPICE, ...) then you will get a keyboard and tablet input devices, because that graphics device is unusable otherwise. There is also an assumption (based on dansmith's comments about mouse being only used for legacy compat reasons) that the user will never want to explicitly request a 'mouse' pointer model and should therefore get 'tablet' by default. Based on these two assumptions, the only thing remaining is to decide how the keyboard and tablet devices will be attached to the guest...achieved via a sensibly named 'hw_input_bus' image metadata property. That seems reasonable to me. Stephen 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
On 11/16/20 02:41, Stephen Finucane wrote:
On Thu, 2020-11-12 at 17:09 -0800, melanie witt wrote:
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).
For some reason your email didn't quote my inline replies when you replied back, so I'm going to label them for clarity. melwitt:
If I understand correctly, we do choose the "best fit" keyboard based on architecture, independent of the requested hw_pointer_model, today.
stephenfin:
Not quite. libvirt on x86 will automatically add a PS2 mouse and keyboard (even if you request something else) because that's what QEMU does. On all other platforms (to the best of my knowledge, based on Kashyap quizzing a handful of multi-arch libvirt devs), this must be done manually. We currently only do this for AArch64, through the non- configurable addition of a USB keyboard [1]. There is currently no way to request e.g. a USB or virtio keyboard on any architecture.
Hm, OK, apologies for my lack of familiarity here. So there are really three things here: tablet, mouse, and keyboard. And the user today can choose hw_pointer_model as None (leave it to default behavior) or usbtablet which sets the input.type=tablet and the input.bus=usb. And then the mouse and keyboard are set to whatever they have to be given the architecture. melwitt:
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?
stephenfin:
My concerns are twofold. Firstly, having the value of 'hw_pointer_model' influence the keyboard bus is poor UX, since this "feature" is not at all apparent from the property name. Secondly, 'hw_pointer_model' is a poorly designed property, given it configured the bus as well as the model, munging the two.
To be clear, I agree that having different buses for keyboard and pointer makes no sense. There's no reason they shouldn't be the same. I'm just saying that configuring the bus for input devices via an appropriately named 'hw_input_bus' image metadata property makes much more sense that maybe getting one configured magically based on the "pointer model" in use.
OK, so the introduction of virtio would be the first time that we would be able to set all of tablet, mouse, and keyboard to the same bus. What if they don't want all three input devices, could it cause any undesirable behavior for them? And then if they set hw_input_bus=usb for libvirt on x86 they'd get a USB tablet, a PS2 mouse, and a PS2 keyboard and on AArch64 they'd get a USB tablet, ? mouse, and USB keyboard. I dunno, it's all pretty confusing IMHO. My point in my last reply was that for the libvirt on x86 case, if the user sets hw_input_bus=usb, how would they know they're really only specifying the tablet? Today they use hw_pointer_model=usbtablet which makes it clear what they have control over. I acknowledge that for hw_input_bus=virtio, things all line up nicely, but for everything else, it doesn't necessarily. This makes me think it would be clearest and least confusing (IMHO) to add a new option like dansmith mentioned, hw_pointer_model=virtio. That way it's not talking only about a tablet but generically just 'virtio' and then all of tablet, mouse, and keyboard get added with bus=virtio. melwitt:
From what I understand, the pointer is the only user selection we can guarantee and the keyboard is architecture dependent.
stephenfin:
Again, not really. There isn't an analog for pointer vs. mouse when it comes to keyboards. The keyboard isn't architecture dependent. What is architecture dependent is whether you get a keyboard and mouse by default (yes on x86, in the form of a PS2 mouse and keyboard, and no for everyone else).
melwitt:
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?
stephenfin:
They can choose the _bus_ that the keyboard uses. There's an assumption here that if you request a graphics device (VNC, SPICE, ...) then you will get a keyboard and tablet input devices, because that graphics device is unusable otherwise. There is also an assumption (based on dansmith's comments about mouse being only used for legacy compat reasons) that the user will never want to explicitly request a 'mouse' pointer model and should therefore get 'tablet' by default. Based on these two assumptions, the only thing remaining is to decide how the keyboard and tablet devices will be attached to the guest...achieved via a sensibly named 'hw_input_bus' image metadata property. That seems reasonable to me.
I hope I'm not woefully wrong here again, but IIUC they can't choose the bus the mouse and keyboard uses for libvirt on x86, they get PS2 regardless. This is why I feel like hw_input_bus as a replacement for hw_pointer_model is not clear. It's only guaranteed to be used for the tablet bus. I'm sorry I'm not seeing how hw_pointer_model=virtio isn't the clearest, least confusing way to add virtio. (Or maybe hw_pointer_model=virtiotabletmousekeyboard ?!) Thanks for taking the time to explain the technical details around this area of the code. I think I now understand your point of view since the name of hw_pointer_model doesn't nicely convey the tablet+mouse+keyboard setting when it comes to virtio. On the flip side of that, I feel like hw_input_bus=usb when it's only going to be honored for the tablet for libvirt on x86 seems even less nice. This is all just MHO. If most other people think that hw_input_bus is the clearer and more user-friendly way to present this config, then I will of course accept the consensus. Cheers, -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
On Wed, 2020-11-18 at 16:12 -0800, melanie witt wrote:
On 11/16/20 02:41, Stephen Finucane wrote:
On Thu, 2020-11-12 at 17:09 -0800, melanie witt wrote:
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).
For some reason your email didn't quote my inline replies when you replied back, so I'm going to label them for clarity.
Looks like Evolution has broken plain text editing :( Apologies.
melwitt:
If I understand correctly, we do choose the "best fit" keyboard based on architecture, independent of the requested hw_pointer_model, today. stephenfin: Not quite. libvirt on x86 will automatically add a PS2 mouse and keyboard (even if you request something else) because that's what QEMU does. On all other platforms (to the best of my knowledge, based on Kashyap quizzing a handful of multi-arch libvirt devs), this must be done manually. We currently only do this for AArch64, through the non- configurable addition of a USB keyboard [1]. There is currently no way to request e.g. a USB or virtio keyboard on any architecture.
Hm, OK, apologies for my lack of familiarity here. So there are really three things here: tablet, mouse, and keyboard. And the user today can choose hw_pointer_model as None (leave it to default behavior) or usbtablet which sets the input.type=tablet and the input.bus=usb. And then the mouse and keyboard are set to whatever they have to be given the architecture.
Sort of. As above, you'll get the PS2 keyboard for free on x86. You'll get a USB keyboard on AArch64. You won't get anything on any other arch.
melwitt:
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?
stephenfin:
My concerns are twofold. Firstly, having the value of 'hw_pointer_model' influence the keyboard bus is poor UX, since this "feature" is not at all apparent from the property name. Secondly, 'hw_pointer_model' is a poorly designed property, given it configured the bus as well as the model, munging the two.
To be clear, I agree that having different buses for keyboard and pointer makes no sense. There's no reason they shouldn't be the same. I'm just saying that configuring the bus for input devices via an appropriately named 'hw_input_bus' image metadata property makes much more sense that maybe getting one configured magically based on the "pointer model" in use.
OK, so the introduction of virtio would be the first time that we would be able to set all of tablet, mouse, and keyboard to the same bus. What if they don't want all three input devices, could it cause any undesirable behavior for them?
You don't have any choice: if you have a graphical console, you need a pointer (mouse or tablet) and you need a keyboard for it to be usable. Conversely, if you don't have a graphical console, then we've no need to add either. If a user didn't want a mouse and keyboard then they can and should disable graphical consoles.
And then if they set hw_input_bus=usb for libvirt on x86 they'd get a USB tablet, a PS2 mouse, and a PS2 keyboard and on AArch64 they'd get a USB tablet, ? mouse, and USB keyboard.
No, on x86 you'd get a USB tablet and keyboard, and a PS2 mouse PS2 keyboard. This is because there isn't currently a way to disable the PS2 devices in libvirt/QEMU. However, this is not an issue since the guest OS will default to using the USB-based devices (or so danpb tells me). Also note that this is very similar to how things currently work with 'hw_pointer_model=usbtablet': setting this means you'll get a USB tablet (but not keyboard), and a PS2 mouse and keyboard. On AArch64 and any other non-x86 host, they'd get simply a USB tablet and a USB keyboard because libvirt doesn't have that QEMU quirk to work with.
I dunno, it's all pretty confusing IMHO. My point in my last reply was that for the libvirt on x86 case, if the user sets hw_input_bus=usb, how would they know they're really only specifying the tablet? Today they use hw_pointer_model=usbtablet which makes it clear what they have control over. I acknowledge that for hw_input_bus=virtio, things all line up nicely, but for everything else, it doesn't necessarily.
As above, they're not. hw_input_bus means USB keyboard and tablet on any host. The only arch that will do something weird is x86, where we have those pesky PS2 remnants that we can't get rid of.
This makes me think it would be clearest and least confusing (IMHO) to add a new option like dansmith mentioned, hw_pointer_model=virtio. That way it's not talking only about a tablet but generically just 'virtio' and then all of tablet, mouse, and keyboard get added with bus=virtio. melwitt:
From what I understand, the pointer is the only user selection we can guarantee and the keyboard is architecture dependent.
stephenfin:
Again, not really. There isn't an analog for pointer vs. mouse when it comes to keyboards. The keyboard isn't architecture dependent. What is architecture dependent is whether you get a keyboard and mouse by default (yes on x86, in the form of a PS2 mouse and keyboard, and no for everyone else).
melwitt:
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?
stephenfin:
They can choose the _bus_ that the keyboard uses. There's an assumption here that if you request a graphics device (VNC, SPICE, ...) then you will get a keyboard and tablet input devices, because that graphics device is unusable otherwise. There is also an assumption (based on dansmith's comments about mouse being only used for legacy compat reasons) that the user will never want to explicitly request a 'mouse' pointer model and should therefore get 'tablet' by default. Based on these two assumptions, the only thing remaining is to decide how the keyboard and tablet devices will be attached to the guest...achieved via a sensibly named 'hw_input_bus' image metadata property. That seems reasonable to me.
I hope I'm not woefully wrong here again, but IIUC they can't choose the bus the mouse and keyboard uses for libvirt on x86, they get PS2 regardless. This is why I feel like hw_input_bus as a replacement for hw_pointer_model is not clear. It's only guaranteed to be used for the tablet bus.
I'm sorry I'm not seeing how hw_pointer_model=virtio isn't the clearest, least confusing way to add virtio. (Or maybe hw_pointer_model=virtiotabletmousekeyboard ?!)
Thanks for taking the time to explain the technical details around this area of the code. I think I now understand your point of view since the name of hw_pointer_model doesn't nicely convey the tablet+mouse+keyboard setting when it comes to virtio. On the flip side of that, I feel like hw_input_bus=usb when it's only going to be honored for the tablet for libvirt on x86 seems even less nice.
Given the above, do you still have the same concerns around using hw_input_bus? Cheers, Stephen
This is all just MHO. If most other people think that hw_input_bus is the clearer and more user-friendly way to present this config, then I will of course accept the consensus.
Cheers, -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
On 11/20/20 10:14, Stephen Finucane wrote:
On Wed, 2020-11-18 at 16:12 -0800, melanie witt wrote:
On 11/16/20 02:41, Stephen Finucane wrote:
On Thu, 2020-11-12 at 17:09 -0800, melanie witt wrote: OK, so the introduction of virtio would be the first time that we would be able to set all of tablet, mouse, and keyboard to the same bus. What if they don't want all three input devices, could it cause any undesirable behavior for them?
You don't have any choice: if you have a graphical console, you need a pointer (mouse or tablet) and you need a keyboard for it to be usable. Conversely, if you don't have a graphical console, then we've no need to add either. If a user didn't want a mouse and keyboard then they can and should disable graphical consoles.
Ack, understood.
And then if they set hw_input_bus=usb for libvirt on x86 they'd get a USB tablet, a PS2 mouse, and a PS2 keyboard and on AArch64 they'd get a USB tablet, ? mouse, and USB keyboard.
No, on x86 you'd get a USB tablet and keyboard, and a PS2 mouse PS2 keyboard. This is because there isn't currently a way to disable the PS2 devices in libvirt/QEMU. However, this is not an issue since the guest OS will default to using the USB-based devices (or so danpb tells me). Also note that this is very similar to how things currently work with 'hw_pointer_model=usbtablet': setting this means you'll get a USB tablet (but not keyboard), and a PS2 mouse and keyboard.
On AArch64 and any other non-x86 host, they'd get simply a USB tablet and a USB keyboard because libvirt doesn't have that QEMU quirk to work with.
OK, this was something I didn't previously get. So, your proposal is that if you were to add hw_input_bus=usb you would also add the USB keyboard and USB mouse for x86. So you end up with USB tablet and keyboard and mouse on x86. You just also happen to have PS2 keyboard and mouse as well and they will not be defaulted. And then AArch64 would get USB tablet and keyboard and mouse with hw_input_bus=usb. I had been previously thinking the hw_input_bus addition would be a change in name only and not a change (additive change) of behavior as well.
I'm sorry I'm not seeing how hw_pointer_model=virtio isn't the clearest, least confusing way to add virtio. (Or maybe hw_pointer_model=virtiotabletmousekeyboard ?!)
Thanks for taking the time to explain the technical details around this area of the code. I think I now understand your point of view since the name of hw_pointer_model doesn't nicely convey the tablet+mouse+keyboard setting when it comes to virtio. On the flip side of that, I feel like hw_input_bus=usb when it's only going to be honored for the tablet for libvirt on x86 seems even less nice.
Given the above, do you still have the same concerns around using hw_input_bus?
Now that I understand that the user's bus choice via hw_input_bus would indeed be honored for all of tablet, keyboard, and mouse for all architectures, I can acknowledge that hw_input_bus could be an improved name over hw_pointer_model. I still think the main, important user selectable bit is the pointer, so I don't think the hw_pointer_model name is so terrible. The remaining concern would be around the work users and operators would have to do to adopt the new name. Are you thinking that existing images just stay as-is and if users want virtio or all USB they can only get it via hw_input_bus=virtio or hw_input_bus=usb and that will facilitate use of the new image property? So the summary would be: * hw_pointer_model=usbtablet you still get a USB tablet and the rest is unselectable * hw_input_bus=usb you get USB everything * hw_input_bus=virtio you get virtio everything If this is the case, then I would say I'm no longer actively opposed to adding hw_input_bus image property to get virtio, but I'm still not sure whether it's worth the amount of change and new image property users need to learn, given that pointer is the main thing users care about and we already have the pointer property. I'll defer to others to decide whether it's worth it, as I'm ambivalent :) Cheers, -melanie
This is all just MHO. If most other people think that hw_input_bus is the clearer and more user-friendly way to present this config, then I will of course accept the consensus.
Cheers, -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
[1] https://review.opendev.org/#/c/756552/ <https://review.opendev.org/#/c/756552/>
participants (3)
-
Dan Smith
-
melanie witt
-
Stephen Finucane