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 ?
>
> If user launches MacOS guest without supplying valid or any key
> they will get upset MacOS (and legality of specifying valid key
> on CLI is also questionable). This patch favors QEMU/tests vs
> MacOS guest.
>
> A patch from Apple needed to change default to the valid key,
> until then prefer QEMU maintainers sanity/cleaner test logs
> vs any inconvenience users running MacOS guest might suffer.
>
> 1) After some googling, it seems 'secret' key is the same for
> all Mac hardware and is widely known but whether it's
> permited to use that is up to lawyers/Apple to decide.
>
> Signed-off-by: Igor Mammedov <imamm...@redhat.com>
> ---
> hw/misc/applesmc.c | 8 +++++---
> 1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/hw/misc/applesmc.c b/hw/misc/applesmc.c
> index 5f9c742e50..bce27b0950 100644
> --- a/hw/misc/applesmc.c
> +++ b/hw/misc/applesmc.c
> @@ -84,7 +84,7 @@ enum {
> #define smc_debug(...) do { } while (0)
> #endif
>
> -static char default_osk[64] = "This is a dummy key. Enter the real key "
> +static char default_osk[65] = "This is a dummy key. Enter the real key "
> "using the -osk parameter";
This is a distinct bug fix from the problem described. We should
take this as a standalone fix no matter what.
> struct AppleSMCData {
> @@ -335,10 +335,12 @@ static void applesmc_isa_realize(DeviceState *dev,
> Error **errp)
> isa_register_ioport(&s->parent_obj, &s->io_err,
> s->iobase + APPLESMC_ERR_PORT);
>
> - if (!s->osk || (strlen(s->osk) != 64)) {
> - warn_report("Using AppleSMC with invalid key");
> + if (!s->osk) {
> s->osk = default_osk;
> }
> + if (strlen(s->osk) != 64) {
> + warn_report("Using AppleSMC with invalid key");
> + }
This means users who invoke QEMU won't see the warning any more. This
feels like a dubious change, as it degrades the user experiance for
sake of tests - IMHO users takes priority over any testing.
With 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 :|
