From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from list by lists.gnu.org with archive (Exim 4.71) id 1ZtDyQ-0007ge-TN for mharc-grub-devel@gnu.org; Mon, 02 Nov 2015 07:12:30 -0500 Received: from eggs.gnu.org ([2001:4830:134:3::10]:34617) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZtDyN-0007eu-EF for grub-devel@gnu.org; Mon, 02 Nov 2015 07:12:28 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZtDyK-0003ET-3z for grub-devel@gnu.org; Mon, 02 Nov 2015 07:12:27 -0500 Received: from smtp.citrix.com ([66.165.176.89]:45624) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZtDyJ-0003EN-VE for grub-devel@gnu.org; Mon, 02 Nov 2015 07:12:24 -0500 X-IronPort-AV: E=Sophos;i="5.20,234,1444694400"; d="scan'208";a="309720158" Message-ID: <1446466341.3088.26.camel@citrix.com> Subject: Re: [PATCH grub-core/kern/xen/init.c] pvgrub2 xen cmdline xenstore var to grubenv From: Ian Campbell To: Mark Pryor , Date: Mon, 2 Nov 2015 12:12:21 +0000 In-Reply-To: <1445645493-24798-1-git-send-email-pryorm09@gmail.com> References: <1445645493-24798-1-git-send-email-pryorm09@gmail.com> Content-Type: text/plain; charset="UTF-8" X-Mailer: Evolution 3.16.5-1 MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-DLP: MIA2 X-detected-operating-system: by eggs.gnu.org: Genre and OS details not recognized. X-Received-From: 66.165.176.89 X-BeenThere: grub-devel@gnu.org X-Mailman-Version: 2.1.14 Precedence: list Reply-To: The development of GNU GRUB List-Id: The development of GNU GRUB List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 02 Nov 2015 12:12:28 -0000 On Fri, 2015-10-23 at 17:11 -0700, Mark Pryor wrote: > When entering the grub2 shell during a pvgrub2 boot, there is no info about the current > domU in the grubenv (set). Starting with a patch submitted by Olaf Herring I exported > the xenstore cmdline only. xenstore? The command line comes from the start info. I'd just drop the mention of xenstore rather than trying to change it. > > The env var, xen_cmdline, can then be used in the top level script used to make > the pvgrub2 kernel blob. > > Signed-off-by: Mark Pryor > --- > grub-core/kern/xen/init.c | 25 +++++++++++++++++++++++++ > 1 file changed, 25 insertions(+) > > diff --git a/grub-core/kern/xen/init.c b/grub-core/kern/xen/init.c > index 0559c03..2a3112d 100644 > --- a/grub-core/kern/xen/init.c > +++ b/grub-core/kern/xen/init.c > @@ -524,6 +524,29 @@ map_all_pages (void) > grub_mm_init_region ((void *) heap_start, heap_end - heap_start); > } > > +/* > + * Find all name=val pairs in the provided cmd_line and export them > + * so that scripts can evaluate the variables for their own purpose. I don't think that is what this function does, it seems to simply provide a single env var containing the complete command line. I think that is a fine behaviour to be starting with. > + */ > +static void > +export_cmdline (void) > +{ > + char *p; > + const char *name="xen_cmdline"; > + > + p = grub_malloc (MAX_GUEST_CMDLINE + 1); > + if (!p) > + return; > + > + grub_memcpy (p, grub_xen_start_page_addr->cmd_line, MAX_GUEST_CMDLINE); > + p[MAX_GUEST_CMDLINE] = '\0'; > + > + grub_env_set (name, p); > + grub_env_export (name); > + > + grub_free (p); I'm not sure what grub coding style is, but either this last line or all the others seem to be incorrectly indented (judging from the inconsistency). Also, I might be wrong but I think grub_xen_start_page_addr->cmd_line is guaranteed to be NULL terminated, if you assume that then you can get away with just calling grub_env_set direct without the copying, I think. Lastly, if you also cc xen-devel then you might get feedback from people more knowledgable about such things as whether si->cmd_line is NULL terminated than I am. Ian.