Re: [Qemu-devel] [PATCH] spapr: select default vty

[David Gibson <david@gibson.dropbear.id.au>]

[-- Attachment #1: Type: text/plain, Size: 2887 bytes --]

On Tue, Sep 26, 2017 at 02:17:50PM +0200, Laurent Vivier wrote:
> SLOF uses the "/chosen/linux,stdout-path" variable to
> choose its console. This variable is provided by QEMU.
> QEMU selects the spapr-vty using the "reg" property:
> it takes the vty with the lowest reg number.
> This patch allows the user to define "linux,stdout-path"
> from the command line by adding a keyword 'default' to
> the spapr-vty device.
> 
> Signed-off-by: Laurent Vivier <lvivier@redhat.com>

I'm a bit dubious about the worth of this.  With your SLOF fixes it
should still be possible to redirect output correctly using -prom-env,
yes?  Using a secondary vty is a pretty niche case, so I'm not
convinced we need extra properties just to make it simpler.
> ---
>  hw/char/spapr_vty.c | 13 ++++++++++++-
>  1 file changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/char/spapr_vty.c b/hw/char/spapr_vty.c
> index 0fa416ca6b..aa56a9a6cb 100644
> --- a/hw/char/spapr_vty.c
> +++ b/hw/char/spapr_vty.c
> @@ -15,6 +15,7 @@ typedef struct VIOsPAPRVTYDevice {
>      CharBackend chardev;
>      uint32_t in, out;
>      uint8_t buf[VTERM_BUFSIZE];
> +    bool is_default;
>  } VIOsPAPRVTYDevice;
>  
>  #define TYPE_VIO_SPAPR_VTY_DEVICE "spapr-vty"
> @@ -153,6 +154,7 @@ void spapr_vty_create(VIOsPAPRBus *bus, Chardev *chardev)
>  static Property spapr_vty_properties[] = {
>      DEFINE_SPAPR_PROPERTIES(VIOsPAPRVTYDevice, sdev),
>      DEFINE_PROP_CHR("chardev", VIOsPAPRVTYDevice, chardev),
> +    DEFINE_PROP_BOOL("default", VIOsPAPRVTYDevice, is_default, false),
>      DEFINE_PROP_END_OF_LIST(),
>  };
>  
> @@ -194,11 +196,13 @@ static const TypeInfo spapr_vty_info = {
>  VIOsPAPRDevice *spapr_vty_get_default(VIOsPAPRBus *bus)
>  {
>      VIOsPAPRDevice *sdev, *selected;
> +    VIOsPAPRVTYDevice *dev;
>      BusChild *kid;
>  
>      /*
>       * To avoid the console bouncing around we want one VTY to be
> -     * the "default". We haven't really got anything to go on, so
> +     * the "default". If the user doesn't provide the information
> +     * we haven't really got anything to go on, so
>       * arbitrarily choose the one with the lowest reg value.
>       */
>  
> @@ -213,6 +217,13 @@ VIOsPAPRDevice *spapr_vty_get_default(VIOsPAPRBus *bus)
>  
>          sdev = VIO_SPAPR_DEVICE(iter);
>  
> +        /* The user can provide the default console to use */
> +
> +        dev = VIO_SPAPR_VTY_DEVICE(sdev);
> +        if (dev->is_default) {
> +            return sdev;
> +        }
> +
>          /* First VTY we've found, so it is selected for now */
>          if (!selected) {
>              selected = sdev;

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]