Re: [PATCH for-7.1] applesmc: silence invalid key warning in case default one is used

[Igor Mammedov]

On Thu, 28 Jul 2022 11:23:00 +0100
Daniel P. Berrangé <berrange@redhat.com> wrote:

> On Thu, Jul 28, 2022 at 11:05:13AM +0100, Peter Maydell wrote:
> > On Thu, 28 Jul 2022 at 10:48, Daniel P. Berrangé <berrange@redhat.com> 
> > wrote:  
> > >
> > > On Thu, Jul 28, 2022 at 05:35:58AM -0400, Igor Mammedov wrote:  
> > > > QEMU probably can't carry OSK key[1] for legal reasons so it
> > > > can't supply the valid default key. However when tests are run
> > > > applesmc will pollute test log with distracting warning,
> > > > silence that warning so it won't distract maintainers/CI.  
> > >
> > > What test is causing this problem ?  
> > 
> > bios-tables-test -- see here for the relevant bit of the log:
> > 
> > https://lore.kernel.org/qemu-devel/CAFEAcA8u8jm7b+JD_t0qMNMy+WSJPOw=qxqptZpwTp=TkcXrhg@mail.gmail.com/
> >   
> 
> The right fix is for bios-tables-tests to pass an explicit 'osk' value
> then. As its a test it doesn't have to be a genuine OSK, jsut any old
> 64-byte string
> 
> diff --git a/tests/qtest/bios-tables-test.c b/tests/qtest/bios-tables-test.c
> index 359916c228..f6b5adf200 100644
> --- a/tests/qtest/bios-tables-test.c
> +++ b/tests/qtest/bios-tables-test.c
> @@ -1632,7 +1632,7 @@ static void test_acpi_q35_applesmc(void)
>          .variant = ".applesmc",
>      };
>  
> -    test_acpi_one("-device isa-applesmc", &data);
> +    test_acpi_one("-device 
> isa-applesmc,osk=iamalsonottherealoskimjustheretostopbiostablestestspammingstderr",
>  &data);
>      free_test_data(&data);
>  }

that will work, care tho send a formal patch or should I take over?

However we still have bogus default_osk, yes it will cause warning which
typically nobody will see and end user will still end up with upset guest.
Right thing would be to require osk explicitly and drop default completely.
Users who actually run MacOS guest must be providing OSK explicitly already
so they won't be affected and anyone else using default is broken anyways
(whether QEMU started directly or through mgmt layer)

> 
> 
> With regards,
> Daniel