Re: [Qemu-devel] [PATCH] vl.c: Report unknown machines correctly

["Philippe Mathieu-Daudé" <philmd@redhat.com>]

On 9/15/19 10:20 PM, Palmer Dabbelt wrote:
> I was recently typing in a QEMU command line, and ended up with
> something like
> 
>     qemu-system-riscv64 -machine virt ... -M 8G
> 
> which is, in retrospect, obviously incorrect: there is no "8G" machine.
> I should have typed something like
> 
>     qemu-system-riscv64 -machine virt ... -m 8G
> 
> but since QEMU was giving me the excessively unhelpful error message
> 
>     qemu-system-riscv64: -machine virt: unsupported machine type
>     Use -machine help to list supported machines
> 
> I had to spend a few minutes scratching my head to figure out what was
> going on.  For some reason I felt like I'd done that before, so I
> figured I'd take a whack at fixing the problem this time.  It turns out
> the error reporting for non-existant machines is just wrong: the invalid
> machine is detected after we've lost the argument pointer, so it just
> prints out the first instance of "-machine" instead of the one we're
> actually looking for.
> 
> I've fixed this by just printing out "-machine $NAME" directly, but I
> feel like there's a better way to do this.  Specifically, my issue is
> that it always prints out "-machine" instead of "-M" -- that's actually
> a regression for users just passing a single invalid machine via "-M",
> which I assume is the more common case.
> 
> I'm not sure how to do this right, though, and my flight is boarding so
> I figured I'd send this out as a way to ask the question.  I didn't have
> time to run the test suite or figure out how to add a test for this, as
> I'm assuming there's a better way to do it.
> 
> Signed-off-by: Palmer Dabbelt <palmer@sifive.com>
> ---
>  vl.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/vl.c b/vl.c
> index 630f5c5e9c..821a5d91c8 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -2487,7 +2487,7 @@ static MachineClass *machine_parse(const char *name, GSList *machines)
>  
>      mc = find_machine(name, machines);
>      if (!mc) {
> -        error_report("unsupported machine type");
> +        error_printf("-machine %s: unsupported machine type\n", name);
>          error_printf("Use -machine help to list supported machines\n");
>          exit(1);
>      }
> 

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>