Re: [Qemu-devel] [RFC PATCH 1/6] machine: add a deprecated_reason property

["Daniel P. Berrange" <berrange@redhat.com>]

On Wed, Nov 08, 2017 at 11:23:53AM +0100, Thomas Huth wrote:
> On 08.11.2017 03:28, Philippe Mathieu-Daudé wrote:
> > If a machine has the deprecated_reason property set:
> > - it won't get listed anymore by the "-M help" command,
> > - a warning will be displayed if the machine is used.
> > 
> > Example:
> >   $ ppc-softmmu/qemu-system-ppc -M prep
> >   qemu-system-ppc: -M prep: warning: The PowerPC PREP platform machine is deprecated (Obsoleted by the "40p" machine)
> >   [machine starting ...]
> > 
> > Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> > ---
> >  include/hw/boards.h | 1 +
> >  vl.c                | 7 +++++++
> >  2 files changed, 8 insertions(+)
> > 
> > diff --git a/include/hw/boards.h b/include/hw/boards.h
> > index 191a5b3cd8..4d5c180968 100644
> > --- a/include/hw/boards.h
> > +++ b/include/hw/boards.h
> > @@ -182,6 +182,7 @@ struct MachineClass {
> >      const char *default_boot_order;
> >      const char *default_display;
> >      GArray *compat_props;
> > +    const char *deprecated_reason;
> >      const char *hw_version;
> >      ram_addr_t default_ram_size;
> >      const char *default_cpu_type;
> > diff --git a/vl.c b/vl.c
> > index ec299099ff..9ed4648af2 100644
> > --- a/vl.c
> > +++ b/vl.c
> > @@ -2742,6 +2742,10 @@ static gint machine_class_cmp(gconstpointer a, gconstpointer b)
> >      }
> >      if (mc) {
> >          g_slist_free(machines);
> > +        if (mc->deprecated_reason) {
> > +            warn_report("The %s machine is deprecated (%s)",
> > +                        mc->desc, mc->deprecated_reason);
> > +        }
> 
> Just cosmetic, but maybe rather put that into select_machine() instead?
> 
> >          return mc;
> >      }
> >      if (name && !is_help_option(name)) {
> > @@ -2752,6 +2756,9 @@ static gint machine_class_cmp(gconstpointer a, gconstpointer b)
> >          machines = g_slist_sort(machines, machine_class_cmp);
> >          for (el = machines; el; el = el->next) {
> >              MachineClass *mc = el->data;
> > +            if (mc->deprecated_reason) {
> > +                continue;
> > +            }
> 
> Not sure, but maybe it would be more user-friendly to still list the
> machine, but add a "(deprecated)" string at the end?

Yeah, as long as it still exists, it should be visible.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|