Open Stack

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