From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from list by lists.gnu.org with archive (Exim 4.90_1) id 1oQrO6-0006bI-IN for mharc-grub-devel@gnu.org; Wed, 24 Aug 2022 10:29:46 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:58678) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1oQrO4-0006aS-8i for grub-devel@gnu.org; Wed, 24 Aug 2022 10:29:44 -0400 Received: from us-smtp-delivery-124.mimecast.com ([170.10.133.124]:41088) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1oQrO0-0004xl-8s for grub-devel@gnu.org; Wed, 24 Aug 2022 10:29:42 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1661351378; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=TEyWN+DBbkJBpzKl+dD9a9HxuCqt7PxNMgHFv3XXQoM=; b=OC1BYlk0Dr+FlxXtAbq+dKXYlHNA99fVaA1tf6ZeEB5vEWUnsTZSQKrzgjur4GOAFf4oK8 46LYhvV7IV1HfQuzr6Tc/eN00UcrCRfSS5UoiZ9qM69D6XucHRlSxxymppGiuMDFqazcK5 jeMASy/9Dsg+YOhk/nRa9cEm3oBXRE8= Received: from mail-qv1-f72.google.com (mail-qv1-f72.google.com [209.85.219.72]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_128_GCM_SHA256) id us-mta-492-k54m38ZCM0aSUdpRNtnHKA-1; Wed, 24 Aug 2022 10:29:35 -0400 X-MC-Unique: k54m38ZCM0aSUdpRNtnHKA-1 Received: by mail-qv1-f72.google.com with SMTP id dc16-20020a056214175000b0049717d038b5so618135qvb.11 for ; Wed, 24 Aug 2022 07:29:35 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=mime-version:message-id:date:references:in-reply-to:subject:cc:to :from:x-gm-message-state:from:to:cc; bh=TEyWN+DBbkJBpzKl+dD9a9HxuCqt7PxNMgHFv3XXQoM=; b=u83WcTjkDqdhfXt1mj7usOHk81cVB7wMIGVAmdcbg280/YlO+1FQD8QGac88mwBzZt 6T/opnsMGSh3mbzRt09dcsTwfVC8Lpyrc9NnMvC25rxiQzIeX3TC3/7+DXlhqXBmBo+b yAxuEKyrNPKgdxdmK34l0BFrXVeeyCXjV6amXadd8V++pDh2nPaVJXst6vQe/fgM0FiF Ism6Avo6ez24a95tN6p0OXiodQo+jbA4HNXnYISmek28eBXhoKw7xV6n1Pjc3WLMBwr8 39kamEEitXC6mWEZOkx7+YcxhftumeZQ+5OjHfm3fLmYUwuZ4UR9hH/8UASomQpydnZh sDlg== X-Gm-Message-State: ACgBeo1lCJ/IpO//SsHF1bXnOvuuokua3CC/Ntdgw1V2y4ZPst9TOmfD Vzriomni8fGzrS34Asru7Xa1Xx0okwJmiIu+lFnaxNLL/27Lmfkkv47Q7rDdVehO35QprX543+G eetg9VlB3dJ0= X-Received: by 2002:a05:622a:289:b0:344:9844:ba5b with SMTP id z9-20020a05622a028900b003449844ba5bmr23256498qtw.234.1661351374643; Wed, 24 Aug 2022 07:29:34 -0700 (PDT) X-Google-Smtp-Source: AA6agR5VmdaIKMlqfToVmKpFAof1IgN9QxQ71FNuRBp6dt06+6ANUaIp5yl1OaXWbKLrBJjFaavKOQ== X-Received: by 2002:a05:622a:289:b0:344:9844:ba5b with SMTP id z9-20020a05622a028900b003449844ba5bmr23256466qtw.234.1661351374224; Wed, 24 Aug 2022 07:29:34 -0700 (PDT) Received: from localhost ([2601:184:4181:74c0:862e:5809:ed9e:e10e]) by smtp.gmail.com with ESMTPSA id w4-20020a05620a424400b006b6757a11fcsm16084273qko.36.2022.08.24.07.29.33 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 24 Aug 2022 07:29:33 -0700 (PDT) From: Robbie Harwood To: Raymund Will Cc: grub-devel@gnu.org, dkiper@net-space.pl, Javier Martinez Canillas Subject: Re: [PATCH v3 1/1] Add support for grub-emu to kexec Linux menu entries In-Reply-To: <20220824103522.GG7668@suse.de> References: <20220823211542.167373-1-rharwood@redhat.com> <20220823211542.167373-2-rharwood@redhat.com> <20220824103522.GG7668@suse.de> Date: Wed, 24 Aug 2022 10:29:30 -0400 Message-ID: MIME-Version: 1.0 Content-Type: multipart/signed; boundary="=-=-="; micalg=pgp-sha512; protocol="application/pgp-signature" Received-SPF: pass client-ip=170.10.133.124; envelope-from=rharwood@redhat.com; helo=us-smtp-delivery-124.mimecast.com X-Spam_score_int: -20 X-Spam_score: -2.1 X-Spam_bar: -- X-Spam_report: (-2.1 / 5.0 requ) BAYES_00=-1.9, DKIMWL_WL_HIGH=-0.001, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, RCVD_IN_DNSWL_NONE=-0.0001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001, T_SCC_BODY_TEXT_LINE=-0.01 autolearn=ham autolearn_force=no X-Spam_action: no action X-BeenThere: grub-devel@gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: The development of GNU GRUB List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 24 Aug 2022 14:29:44 -0000 --=-=-= Content-Type: text/plain Raymund Will writes: > Robbie Harwood wrote on 2022-08-23T17:15:42 -0400: >> From: Raymund Will > [...] >> By default the systemctl kexec option is used so systemd can shutdown >> all of the running services before doing a reboot using kexec. But if >> this is not present, it fallbacks to executing the kexec user-space >> tool directly. > > The last sentence should probably read more like: > > The provision to force a kexec-reboot, in case systemctl kexec > fails, must only be used in controlled environments to avoid > possible file-system corruption and data-loss. I can append something to this effect, sure. > [...] >> --- /dev/null >> +++ b/grub-core/loader/emu/linux.c >> @@ -0,0 +1,180 @@ > [...] >> +static grub_err_t >> +grub_linux_boot (void) >> +{ >> + grub_err_t rc = GRUB_ERR_NONE; >> + char *initrd_param; >> + const char *kexec[] = { "kexec", "-la", kernel_path, boot_cmdline, NULL, NULL }; > > You might have noticed the change in the first parameter to kexec, > which makes a perfect argument for Daniel's request to have that > configurable! But implementation would be quite expensive, maybe > unless it's strictly restricted to non-whitespace- bearing parameters. > Would that be sufficient and viable? Well, just because configuration changed doesn't mean it should be configurable... my understanding is that -a causes KEXEC_FILE_LOAD to be tried first, and that without -a it isn't tried at all, so I don't see the use case for not having -a. I mentioned in my other email that restricting to parameters that don't contain whiespace breaks things like --append and --comand-line. Maybe that's okay? I'm just not immediately seeing the use case for it being configurable, but I could be convinced if someone has one. >> + const char *systemctl[] = { "systemctl", "kexec", NULL }; >> + int kexecute = grub_util_get_kexecute (); >> + >> + if (initrd_path) >> + { >> + initrd_param = grub_xasprintf ("--initrd=%s", initrd_path); >> + kexec[3] = initrd_param; >> + kexec[4] = boot_cmdline; >> + } >> + else >> + { >> + initrd_param = grub_xasprintf ("%s", ""); >> + } >> + >> + grub_dprintf ("linux", "%serforming 'kexec -la %s %s %s'\n", >> + (kexecute) ? "P" : "Not p", >> + kernel_path, initrd_param, boot_cmdline); > > Well, the original grub_printf() in this case was very helpful to > "create" a kexec-load command for cut-n-paste. Is it really necessary > to bury it in a ton of debug messages? I'll defer to Daniel on this, but feedback we've received in the past has requested that all printing go through the debug infrastructure. >> + if (kexecute) >> + rc = grub_util_exec (kexec); >> + >> + grub_free(initrd_param); >> + >> + if (rc != GRUB_ERR_NONE) >> + { >> + grub_error (rc, N_("Error trying to perform kexec load operation.")); >> + grub_sleep (3); >> + return rc; >> + } >> + >> + if (kexecute < 1) >> + grub_fatal (N_("Use '"PACKAGE"-emu --kexec' to force a system restart.")); >> + >> + grub_dprintf ("linux", "Performing 'systemctl kexec' (%s) ", >> + (kexecute==1) ? "do-or-die" : "just-in-case"); >> + rc = grub_util_exec (systemctl); >> + >> + if (kexecute == 1) >> + grub_error (rc, N_("Error trying to perform 'systemctl kexec'")); > > This grub_error() needs to be a grub_fatal() to achieve the intended > behavior, right? > > If kexecute is 1 it should bail out here. Only if it's bigger the > forced kexec should be tried! (Note, that "< 1" is already covered > above!) Correct, will fix. (Peeking behind the curtain, I'm manually merging your patch with ours, so this kind of checking is appreciated.) >> + /* need to check read-only root before resetting hard!? */ > > This comment probably needs to be replaced with a strict one > (reflected in GRUB's docs) explaining, that the user takes full > responsiblity in "force" being exclusively used in read-only > environments, as grub-emu itself simply can't guarantee this. (Any > check here would hardly scratch the surface.) Okay. For what it's worth, the openSUSE patch also has the same comment. Be well, --Robbie --=-=-= Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQJIBAEBCgAyFiEEA5qc6hnelQjDaHWqJTL5F2qVpEIFAmMGNcsUHHJoYXJ3b29k QHJlZGhhdC5jb20ACgkQJTL5F2qVpEJiCxAArsl4iQNN5vFrGwl9JqTYBQf+vSVi cACVfJKnML4uBpwroBOyNlghPKE7XajrCGgjZkvAmowMsGrR6bQBaL9Ak696Lxxq NJKsTGugBcIh6GkLqBFyj4QKStJdqn3yRd2FGZvoFwTfL4kOdA84ozQYspQ+L6Sl /vgyxziCgvRk9t2FfDVkPiR1tb+DiNHKBFA+wvffZ5OAE73IEx6YrYSJwuwJDJTS iN027Emi5aTWkhqvw95W8TXQWstdxh4SF3Qe7jQS+JIbD36nxX7DSlezw1La1sjc 5nbXirEPf/U7pyDrUCzsYev7qdb4x5CprzoAlUUZ7VuM/63ItxFQq6yHlevn3YvP LpKsJO1TVInvF6eO4SI2rZTsG7+xuJkGTZ3vRHxwwsH+sTbKwf0PRKNzCHFj202D fGLAer1mazJp/qH3aX+PFDVh7A6u/97vswv4RYd0hLOEkj8I1MRGHTzpazw0sWLd KIY2ZRPM5bkzIlSOwcXn/7VOGUGT7qz0BnpNVLC+HbH+67rrFzhdSOxbKRpQ6gbI l25Kjf+4AM9sfu8CVaRFwtyIIskrM3iEQd3OE3QfO6izWEy1YTpjmt59KEuQD9n+ O+S3ZFZ6nZfhTOvylpEhlmP9UY8PcXtNYW/XvORho1RRXAAnFORqhSqWgsRHZII3 dbaqzKkp8kpIBhU= =icaj -----END PGP SIGNATURE----- --=-=-=--