diff mbox series

[v3,2/4] usb/hub: mark as deprecated

Message ID 20240606143010.1318226-3-kraxel@redhat.com (mailing list archive)
State New, archived
Headers show
Series allow to deprecate objects and devices | expand

Commit Message

Gerd Hoffmann June 6, 2024, 2:30 p.m. UTC
The hub supports only USB 1.1.  When running out of usb ports it is in
almost all cases the much better choice to add another usb host adapter
(or increase the number of root ports when using xhci) instead of using
the usb hub.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/usb/dev-hub.c | 1 +
 1 file changed, 1 insertion(+)

Comments

Daniel P. Berrangé June 6, 2024, 2:41 p.m. UTC | #1
On Thu, Jun 06, 2024 at 04:30:08PM +0200, Gerd Hoffmann wrote:
> The hub supports only USB 1.1.  When running out of usb ports it is in
> almost all cases the much better choice to add another usb host adapter
> (or increase the number of root ports when using xhci) instead of using
> the usb hub.

Is that actually a strong enough reason to delete this device though ?
This reads like its merely something we don't expect to be commonly
used, rather than something we would actively want to delete.

> 
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  hw/usb/dev-hub.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/hw/usb/dev-hub.c b/hw/usb/dev-hub.c
> index 06e9537d0356..bc8d0ba4cfcf 100644
> --- a/hw/usb/dev-hub.c
> +++ b/hw/usb/dev-hub.c
> @@ -686,6 +686,7 @@ static void usb_hub_class_initfn(ObjectClass *klass, void *data)
>      set_bit(DEVICE_CATEGORY_BRIDGE, dc->categories);
>      dc->fw_name = "hub";
>      dc->vmsd = &vmstate_usb_hub;
> +    klass->deprecated = true;
>      device_class_set_props(dc, usb_hub_properties);
>  }

Deprecations should also have an entry in docs/about/deprecated.rst to
warn users about the intent to delete the code in future.

With regards,
Daniel
Alex Bennée June 12, 2024, 3:52 p.m. UTC | #2
Daniel P. Berrangé <berrange@redhat.com> writes:

> On Thu, Jun 06, 2024 at 04:30:08PM +0200, Gerd Hoffmann wrote:
>> The hub supports only USB 1.1.  When running out of usb ports it is in
>> almost all cases the much better choice to add another usb host adapter
>> (or increase the number of root ports when using xhci) instead of using
>> the usb hub.
>
> Is that actually a strong enough reason to delete this device though ?
> This reads like its merely something we don't expect to be commonly
> used, rather than something we would actively want to delete.

This does seem quite aggressive because there may be cases when users
explicitly want to use old devices. Maybe there is need for a third
state (better_alternatives?) so we can steer users away from old command
lines they may have picked up from the web to the modern alternative?
Markus Armbruster June 13, 2024, 8:31 a.m. UTC | #3
Alex Bennée <alex.bennee@linaro.org> writes:

> Daniel P. Berrangé <berrange@redhat.com> writes:
>
>> On Thu, Jun 06, 2024 at 04:30:08PM +0200, Gerd Hoffmann wrote:
>>> The hub supports only USB 1.1.  When running out of usb ports it is in
>>> almost all cases the much better choice to add another usb host adapter
>>> (or increase the number of root ports when using xhci) instead of using
>>> the usb hub.
>>
>> Is that actually a strong enough reason to delete this device though ?
>> This reads like its merely something we don't expect to be commonly
>> used, rather than something we would actively want to delete.
>
> This does seem quite aggressive because there may be cases when users
> explicitly want to use old devices. Maybe there is need for a third
> state (better_alternatives?) so we can steer users away from old command
> lines they may have picked up from the web to the modern alternative?

What exactly do we mean when we call something deprecated?

For me, it means "you should not normally use this".

Important special case: "because we intend to remove it."

But there can be other cases, say "because <alternative> will serve you
better".
Daniel P. Berrangé June 13, 2024, 8:34 a.m. UTC | #4
On Thu, Jun 13, 2024 at 10:31:56AM +0200, Markus Armbruster wrote:
> Alex Bennée <alex.bennee@linaro.org> writes:
> 
> > Daniel P. Berrangé <berrange@redhat.com> writes:
> >
> >> On Thu, Jun 06, 2024 at 04:30:08PM +0200, Gerd Hoffmann wrote:
> >>> The hub supports only USB 1.1.  When running out of usb ports it is in
> >>> almost all cases the much better choice to add another usb host adapter
> >>> (or increase the number of root ports when using xhci) instead of using
> >>> the usb hub.
> >>
> >> Is that actually a strong enough reason to delete this device though ?
> >> This reads like its merely something we don't expect to be commonly
> >> used, rather than something we would actively want to delete.
> >
> > This does seem quite aggressive because there may be cases when users
> > explicitly want to use old devices. Maybe there is need for a third
> > state (better_alternatives?) so we can steer users away from old command
> > lines they may have picked up from the web to the modern alternative?
> 
> What exactly do we mean when we call something deprecated?
> 
> For me, it means "you should not normally use this".
> 
> Important special case: "because we intend to remove it."

That's not the special case, it is the regular case - the documented
meaning of 'deprecated' in QEMU. When we deprecate something, it is
a warning that we intend to delete it in 2 releases time.

With regards,
Daniel
Daniel P. Berrangé June 13, 2024, 8:44 a.m. UTC | #5
On Wed, Jun 12, 2024 at 04:52:50PM +0100, Alex Bennée wrote:
> Daniel P. Berrangé <berrange@redhat.com> writes:
> 
> > On Thu, Jun 06, 2024 at 04:30:08PM +0200, Gerd Hoffmann wrote:
> >> The hub supports only USB 1.1.  When running out of usb ports it is in
> >> almost all cases the much better choice to add another usb host adapter
> >> (or increase the number of root ports when using xhci) instead of using
> >> the usb hub.
> >
> > Is that actually a strong enough reason to delete this device though ?
> > This reads like its merely something we don't expect to be commonly
> > used, rather than something we would actively want to delete.
> 
> This does seem quite aggressive because there may be cases when users
> explicitly want to use old devices. Maybe there is need for a third
> state (better_alternatives?) so we can steer users away from old command
> lines they may have picked up from the web to the modern alternative?

We've got 2 flags proposed in patch 1 - "deprecated" and "not_secure" -
which we formally expose to mgmt apps/users. Both of these are actionable
in an unambiguous way. An application can query this info, and can also
tell QEMU to enforce policy on this. Both are good.

The "better alternatives" conceptable, however, is an inherantly fuzzy
concept, because "better" is very much a use-case depedent / eye of the
beholder concept. This would makes it difficult/impractical for an
application to take action based on such a "better-alternatives' schema
marker. It would imply the application has to understands the better
alternatives ahead of time, as well as understanding the end users'
intent and that's not really viable.



This is a long winded way of saying that while "better alternatives"
is certainly a concept that is relevant, I'm not convinced it belongs
in the schema, as opposed to being a documentation task.

We haven't consistently provided documentation in the manual for every
device, so in many cases '-device help' is all that exists, but in the
case of USB we do actually have docs:

  https://www.qemu.org/docs/master/system/devices/usb.html

and those docs give little guidance to users about 'usb-hub', so IMHO
that's where we should be documenting the tradeoffs of the different
USB config scenarios.

With regards,
Daniel
Markus Armbruster June 13, 2024, 10:38 a.m. UTC | #6
Daniel P. Berrangé <berrange@redhat.com> writes:

> On Thu, Jun 13, 2024 at 10:31:56AM +0200, Markus Armbruster wrote:
>> Alex Bennée <alex.bennee@linaro.org> writes:
>> 
>> > Daniel P. Berrangé <berrange@redhat.com> writes:
>> >
>> >> On Thu, Jun 06, 2024 at 04:30:08PM +0200, Gerd Hoffmann wrote:
>> >>> The hub supports only USB 1.1.  When running out of usb ports it is in
>> >>> almost all cases the much better choice to add another usb host adapter
>> >>> (or increase the number of root ports when using xhci) instead of using
>> >>> the usb hub.
>> >>
>> >> Is that actually a strong enough reason to delete this device though ?
>> >> This reads like its merely something we don't expect to be commonly
>> >> used, rather than something we would actively want to delete.
>> >
>> > This does seem quite aggressive because there may be cases when users
>> > explicitly want to use old devices. Maybe there is need for a third
>> > state (better_alternatives?) so we can steer users away from old command
>> > lines they may have picked up from the web to the modern alternative?
>> 
>> What exactly do we mean when we call something deprecated?
>> 
>> For me, it means "you should not normally use this".
>> 
>> Important special case: "because we intend to remove it."
>
> That's not the special case, it is the regular case - the documented
> meaning of 'deprecated' in QEMU. When we deprecate something, it is
> a warning that we intend to delete it in 2 releases time.

It's the regular case in QEMU today because we made it so there, by
electing to limit deprecation to "because we intend to remove it."
Daniel P. Berrangé June 13, 2024, 10:48 a.m. UTC | #7
On Thu, Jun 13, 2024 at 12:38:31PM +0200, Markus Armbruster wrote:
> Daniel P. Berrangé <berrange@redhat.com> writes:
> 
> > On Thu, Jun 13, 2024 at 10:31:56AM +0200, Markus Armbruster wrote:
> >> Alex Bennée <alex.bennee@linaro.org> writes:
> >> 
> >> > Daniel P. Berrangé <berrange@redhat.com> writes:
> >> >
> >> >> On Thu, Jun 06, 2024 at 04:30:08PM +0200, Gerd Hoffmann wrote:
> >> >>> The hub supports only USB 1.1.  When running out of usb ports it is in
> >> >>> almost all cases the much better choice to add another usb host adapter
> >> >>> (or increase the number of root ports when using xhci) instead of using
> >> >>> the usb hub.
> >> >>
> >> >> Is that actually a strong enough reason to delete this device though ?
> >> >> This reads like its merely something we don't expect to be commonly
> >> >> used, rather than something we would actively want to delete.
> >> >
> >> > This does seem quite aggressive because there may be cases when users
> >> > explicitly want to use old devices. Maybe there is need for a third
> >> > state (better_alternatives?) so we can steer users away from old command
> >> > lines they may have picked up from the web to the modern alternative?
> >> 
> >> What exactly do we mean when we call something deprecated?
> >> 
> >> For me, it means "you should not normally use this".
> >> 
> >> Important special case: "because we intend to remove it."
> >
> > That's not the special case, it is the regular case - the documented
> > meaning of 'deprecated' in QEMU. When we deprecate something, it is
> > a warning that we intend to delete it in 2 releases time.
> 
> It's the regular case in QEMU today because we made it so there, by
> electing to limit deprecation to "because we intend to remove it."

Fair point, but assigning additional meanings to the existing 'deprecation'
concept will undermine the value QEMU & its consumers obtain from current
usage.

Consumers know if they see the "deprecated" marker, it has started a clock
ticking and they must immediately plan work to stop using the feature.

QEMU maintainers know if they see the 'deprecated' marker and it has been
2 releases, then we can delete it.

I don't want to loose that clear & easily understood meaning, by overloading
"deprecated" for scenarios like "it is sometimes better to use a different
device instead of this one, depending on factors X, Y & Z".

With regards,
Daniel
Alex Bennée June 13, 2024, 2:49 p.m. UTC | #8
Daniel P. Berrangé <berrange@redhat.com> writes:

> On Thu, Jun 13, 2024 at 12:38:31PM +0200, Markus Armbruster wrote:
>> Daniel P. Berrangé <berrange@redhat.com> writes:
>> 
>> > On Thu, Jun 13, 2024 at 10:31:56AM +0200, Markus Armbruster wrote:
>> >> Alex Bennée <alex.bennee@linaro.org> writes:
>> >> 
>> >> > Daniel P. Berrangé <berrange@redhat.com> writes:
>> >> >
>> >> >> On Thu, Jun 06, 2024 at 04:30:08PM +0200, Gerd Hoffmann wrote:
>> >> >>> The hub supports only USB 1.1.  When running out of usb ports it is in
>> >> >>> almost all cases the much better choice to add another usb host adapter
>> >> >>> (or increase the number of root ports when using xhci) instead of using
>> >> >>> the usb hub.
>> >> >>
>> >> >> Is that actually a strong enough reason to delete this device though ?
>> >> >> This reads like its merely something we don't expect to be commonly
>> >> >> used, rather than something we would actively want to delete.
>> >> >
>> >> > This does seem quite aggressive because there may be cases when users
>> >> > explicitly want to use old devices. Maybe there is need for a third
>> >> > state (better_alternatives?) so we can steer users away from old command
>> >> > lines they may have picked up from the web to the modern alternative?
>> >> 
>> >> What exactly do we mean when we call something deprecated?
>> >> 
>> >> For me, it means "you should not normally use this".
>> >> 
>> >> Important special case: "because we intend to remove it."
>> >
>> > That's not the special case, it is the regular case - the documented
>> > meaning of 'deprecated' in QEMU. When we deprecate something, it is
>> > a warning that we intend to delete it in 2 releases time.
>> 
>> It's the regular case in QEMU today because we made it so there, by
>> electing to limit deprecation to "because we intend to remove it."
>
> Fair point, but assigning additional meanings to the existing 'deprecation'
> concept will undermine the value QEMU & its consumers obtain from current
> usage.
>
> Consumers know if they see the "deprecated" marker, it has started a clock
> ticking and they must immediately plan work to stop using the feature.
>
> QEMU maintainers know if they see the 'deprecated' marker and it has been
> 2 releases, then we can delete it.
>
> I don't want to loose that clear & easily understood meaning, by overloading
> "deprecated" for scenarios like "it is sometimes better to use a different
> device instead of this one, depending on factors X, Y & Z".

I agree we shouldn't overload the meaning if deprecated.

So to this case in point. Is there anything you can do with usb/hub that
you can't do with the newer xhci based one? Is it backward compatible
enough that an old kernel would work? Or are we talking really old
kernels at this point?

Is there non-PC hardware that utilises these hubs?
Gerd Hoffmann June 14, 2024, 7:03 a.m. UTC | #9
On Thu, Jun 13, 2024 at 03:49:23PM GMT, Alex Bennée wrote:
> Daniel P. Berrangé <berrange@redhat.com> writes:
> 
> > I don't want to loose that clear & easily understood meaning, by overloading
> > "deprecated" for scenarios like "it is sometimes better to use a different
> > device instead of this one, depending on factors X, Y & Z".
> 
> I agree we shouldn't overload the meaning if deprecated.
> 
> So to this case in point. Is there anything you can do with usb/hub that
> you can't do with the newer xhci based one?

Well, it's a hub.  You can (virtually) plug it into a usb root port of a
ohci/uhci/ehci/xhci host adapter and get 8 more usb ports.  Those new
ports are usb 1.1 only though, so not very useful in a modern world,
except maybe for HID devices which don't need much bandwidth.

> Is it backward compatible
> enough that an old kernel would work? Or are we talking really old
> kernels at this point?

Pretty much everything should be able to drive the usb hub.

take care,
  Gerd
Gerd Hoffmann June 14, 2024, 8:40 a.m. UTC | #10
Hi,

> > This does seem quite aggressive because there may be cases when users
> > explicitly want to use old devices. Maybe there is need for a third
> > state (better_alternatives?) so we can steer users away from old command
> > lines they may have picked up from the web to the modern alternative?
> 
> We've got 2 flags proposed in patch 1 - "deprecated" and "not_secure" -
> which we formally expose to mgmt apps/users. Both of these are actionable
> in an unambiguous way. An application can query this info, and can also
> tell QEMU to enforce policy on this. Both are good.

Well, given that people apparently don't want actually delete stuff
I'm not sure the 'deprecated' flag actually makes sense.

> The "better alternatives" conceptable, however, is an inherantly fuzzy
> concept, because "better" is very much a use-case depedent / eye of the
> beholder concept.

Well.  I think the use cases can be grouped into two cases:

  (1) Operating system museum.  People using qemu qemu to keep their
      beloved (historical) OS alive on modern hardware.

  (2) Virtualization.  Run your production workload in VMs.

For (1) "better alternatives" doesn't make sense because you have to
consider what your (old) guest OS supports.

For (2) I think it does make sense.  I'd be conservative here and would
define possible production workloads as "OS which is still supported
with security updates".  Hardware old enough to be supported by all
production workloads (which should roughly being 10-15 years in the
market) can go into the "better alternative" bucket.  Examples:

 - xhci is old enough and can replace ohci/uhci/ehci.
 - intel-hda is old enough and can replace ac97/es1370/sb/...
 - sata is old enough and can replace ide.
 - nvme isn't old enough yet I'd say (also no cdrom support so it
   can't fully replace ide/sata).
 - q35 is old enough and can replace pc.

> This would makes it difficult/impractical for an application to take
> action based on such a "better-alternatives' schema marker.  It would
> imply the application has to understands the better alternatives ahead
> of time, as well as understanding the end users' intent and that's not
> really viable.

Well, with the conservative classification it should be possible to
simply apply it universally.  Throw warnings or errors in case a device
with a "better-alternative" tag is used.  Or build qemu without these
devices.  Or move these devices to a sub-rpm (if modularized), so they
are not present by default but can be installed for the (1) use cases.

take care,
  Gerd
diff mbox series

Patch

diff --git a/hw/usb/dev-hub.c b/hw/usb/dev-hub.c
index 06e9537d0356..bc8d0ba4cfcf 100644
--- a/hw/usb/dev-hub.c
+++ b/hw/usb/dev-hub.c
@@ -686,6 +686,7 @@  static void usb_hub_class_initfn(ObjectClass *klass, void *data)
     set_bit(DEVICE_CATEGORY_BRIDGE, dc->categories);
     dc->fw_name = "hub";
     dc->vmsd = &vmstate_usb_hub;
+    klass->deprecated = true;
     device_class_set_props(dc, usb_hub_properties);
 }