On Thu, 28 Jul 2022 10:48:27 +0100 Daniel P. Berrangé <berra...@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 ? > > > > > 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. thought about it (until I started to write commit message for that) but it becomes bug only if default_osk is treated as NULL terminated string, which happens only if following hunk is applied. Without that, it's treated as 64 bytes. (we can null-terminate it anyways just to be on a safe side) > > 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