Re: [edk2-devel] [RFC PATCH 1/1] OvmfPkg: add 'initrd' shell command to expose Linux initrd via device path

["Ard Biesheuvel" <ard.biesheuvel@linaro.org>]

On Fri, 14 Feb 2020 at 15:17, Laszlo Ersek <lersek@redhat.com> wrote:
>
> On 02/14/20 01:55, Ni, Ray wrote:
> >
> >
> >> -----Original Message-----
> >> From: Laszlo Ersek <lersek@redhat.com>
> >> Sent: Friday, February 14, 2020 7:15 AM
> >> To: Ni, Ray <ray.ni@intel.com>; devel@edk2.groups.io; Ard Biesheuvel
> >> <ard.biesheuvel@arm.com>
> >> Cc: leif@nuviainc.com; philmd@redhat.com; Gao, Zhichao
> >> <zhichao.gao@intel.com>
> >> Subject: Re: [edk2-devel] [RFC PATCH 1/1] OvmfPkg: add 'initrd' shell
> >> command to expose Linux initrd via device path
> >>
> >> On 02/12/20 15:21, Ni, Ray wrote:
> >>>> (3) However: I think this should be added as a Dynamic Command instead.
> >>>> I'm basing this on the message of commit 0961002352e9 ("ShellPkg/tftp:
> >>>> Convert from NULL class library to Dynamic Command", 2017-11-28),
> >> which
> >>>> is the first commit in edk2 ever to introduce a Dynamic Command.
> >>>>
> >>>> And the commit message there says:
> >>>>
> >>>>     The guideline is:
> >>>>     1. Only use NULL class library for Shell spec defined commands.
> >>>>     2. New commands can be provided as not only a standalone application
> >>>>        but also a dynamic command. So it can be used either as an
> >>>>        internal command, but also as a standalone application.
> >>>>
> >>>> I'm not asking for the command to be usable as a separate application,
> >>>> but I think we might want to follow the first guideline.
> >>>>
> >>>> (I've checked the UEFI Shell 2.2 spec. While it talks about dynamic
> >>>> commands, it does not seem to spell out guideline#1. So I think it's
> >>>> rather an edk2-specific guideline than a standard one. Nonetheless we
> >>>> might want to adhere to it.)
> >>>
> >>> Laszlo, thanks for the comments.
> >>> I didn't remember that I said these guideline publicly.
> >>> The reason behind that is we can have the same shell binary everywhere
> >>> and new non-spec commands can be added through dynamic command
> >> without
> >>> impacting the shell binary.
> >>
> >> Thanks for the explanation -- this means that the NULL class lib
> >> approach is good for OvmfPkg after all. I'm putting the remaining parts
> >> of this patch back on my review queue (it will take a while).
> >
> > Please don't misunderstand my points.
>
> OK. From your response, I thought that the guidelines you captured in
> the commit message in question were only for internal shell builds.
>
> > I still prefer to use dynamic commands
> > for all non-spec defined shell internal commands.
> > Sorry for the confusion caused by my previous mail.
>
> It's OK, I understand better now. So I guess I'll de-queue the review of
> the rest of this patch once again, and wait for the next version (with
> the dynamic command implementation).
>

Thanks for the review and the clarification. I will change this into a
dynamic command for v2, but it may be a while before I get back to it,
since this feature is still under discussion on the Linux side as
well.