Re: [PATCH] GPT Shell Application/Library

[Vladimir Olovyannikov <vladimir.olovyannikov@broadcom.com>]

> -----Original Message-----
> From: Laszlo Ersek [mailto:lersek@redhat.com]
> Sent: October-18-16 10:24 AM
> To: Vladimir Olovyannikov; Shah, Tapan; Carsey, Jaben; Michael Zimmermann
> Cc: Ni, Ruiyu; Arshi, Shala; edk2-devel@lists.01.org
> Subject: Re: [edk2] [PATCH] GPT Shell Application/Library
>
> On 10/18/16 18:58, Vladimir Olovyannikov wrote:
> > Thank you all for comments,
> >
> > So to summarize the discussion:
> >
> > 1. I will create a Shell library which would perform all GPT operations.
> >      Part of PartitionDxe will also be in that library so PartitionDxe
> > will be using it.
> >      The gpt Shell tool will also be using it.
>
> I think you might want to place the library class header file, and the
> library
> instance, under MdeModulePkg. It's okay for the shell application (or
> shell
> libraries) to depend on library classes / library instances from
> MdeModulePkg, but -- I think -- it's not okay for a driver in
> MdeModulePkg,
> such as PartitionDxe, to depend on a class + instance from under ShellPkg.
Laszlo,
I think PartitionDxe will depend on the GPT shared library (which will have
nothing to do to the ShellPkg),
as well as gpt tool (Shell library/application) will depend on that shared
library as well to get information regarding partitions and
to perform GPT management.

Thank you,
Vladimir
>
> Thanks!
> Laszlo
>
> > 2.  Refactor the parameters of the gpt utility to make it similar to
> > other existing Shell commands.
> >       BTW Is there any document describing Shell utility parameters'
> > standards?
> >
> > Please let me know if you have other suggestions.
> >
> > Thank you,
> > Vladimir
> >
> > -----Original Message-----
> > From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
> > Shah, Tapan
> > Sent: October-18-16 6:59 AM
> > To: Carsey, Jaben; Vladimir Olovyannikov; Michael Zimmermann
> > Cc: Ni, Ruiyu; Arshi, Shala; edk2-devel@lists.01.org; Laszlo Ersek
> > Subject: Re: [edk2] [PATCH] GPT Shell Application/Library
> >
> > Thanks for the contribution Vladimir!
> >
> > Few comments:
> > 1. It's better to refactor the code now before commit and move GPT
> > related code outside ShellPkg and create a shared library.
> > 2. CLI parameters of this utility are too complex and need to be
> > refactored to make it similar to other existing Shell commands.
> >
> > Thanks,
> > Tapan
> >
> > -----Original Message-----
> > From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
> > Carsey, Jaben
> > Sent: Monday, October 17, 2016 12:56 PM
> > To: Vladimir Olovyannikov <vladimir.olovyannikov@broadcom.com>;
> > Michael Zimmermann <sigmaepsilon92@gmail.com>
> > Cc: Ni, Ruiyu <ruiyu.ni@intel.com>; Arshi, Shala
> > <shala.arshi@intel.com>; edk2-devel@lists.01.org
> > <edk2-devel@ml01.01.org>; Carsey, Jaben <jaben.carsey@intel.com>;
> > Laszlo Ersek <lersek@redhat.com>
> > Subject: Re: [edk2] [PATCH] GPT Shell Application/Library
> >
> > To the old question about license: I asked our people to check and was
> > told that the license is compatible with our BSD and ok by Intel.
> >
> > To the technical content – I really like this idea of a shared library.
> > That would be a great way to share code and not have as much duplicate.
> >
> > -Jaben
> >
> > From: Vladimir Olovyannikov
> > [mailto:vladimir.olovyannikov@broadcom.com]
> > Sent: Monday, October 17, 2016 10:52 AM
> > To: Michael Zimmermann <sigmaepsilon92@gmail.com>
> > Cc: Laszlo Ersek <lersek@redhat.com>; Carsey, Jaben
> > <jaben.carsey@intel.com>; Ni, Ruiyu <ruiyu.ni@intel.com>;
> > edk2-devel@lists.01.org <edk2-devel@ml01.01.org>
> > Subject: RE: [edk2] [PATCH] GPT Shell Application/Library
> > Importance: High
> >
> > Hi Michael,
> > I am absolutely agree with your proposal.
> >
> > In the gpt Shell library/application I had to “borrow” some stuff from
> > PartitionDxe.c to not reinvent a  wheel.
> > If the PartitionDxe maintainer agrees to have a separate library
> > available for everybody, I would move all the GPT-related stuff from
> > the GptWorker (and partially from the PartitionDxe itself) to that
> independent library.
> > This could be a longer-term task.
> > Right now I just wanted to share the tool which could be useful for
> > anybody who would wish to manage GPT partitions (and/or do a FAT32
> > format of either a disk or a GPT partition) from within the Shell. What
> > do
> you think?
> >
> > Thank you,
> > Vladimir
> > From: Michael Zimmermann
> > [mailto:sigmaepsilon92@gmail.com<mailto:sigmaepsilon92@gmail.com>]
> > Sent: October-17-16 12:25 AM
> > To: Vladimir Olovyannikov
> > Cc: Laszlo Ersek; Jaben Carsey; Ni, Ruiyu;
> > edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>
> > Subject: Re: [edk2] [PATCH] GPT Shell Application/Library
> >
> > Hi,
> >
> > wouldn't it be better to make a generic gpt parsing library which is
> > independent of the shell so both the shell and PartitionDxe can use it?
> > It may also be useful for other applications which need additional
> > information like the gpt partition names.
> >
> > Thanks
> > Michael
> >
> > On Mon, Oct 17, 2016 at 8:49 AM, Vladimir Olovyannikov
> >
> <vladimir.olovyannikov@broadcom.com<mailto:vladimir.olovyannikov@broa
> d
> > com.com>>
> > wrote:
> > Thank you Laszlo,
> >
> > Sorry, I missed the fields; it is my first contribution, I will add
> > the required lines, review the source according to your comments and
> > will resubmit the patch.
> > So do you think the command should be _gpt instead of gpt? I was
> > following TFTP and SF commands as a template.
> >
> > Thank you,
> > Vladimir
> >
> > On Oct 16, 2016 1:05 PM, "Laszlo Ersek"
> > <lersek@redhat.com<mailto:lersek@redhat.com>> wrote:
> >>
> >> On 10/16/16 07:23, Vladimir Olovyannikov wrote:
> >>> This allows managing (create, delete, modify, fat format) of GPT
> >>> partitions from within UEFI Shell.
> >>> Syntax:
> >>> gpt <command> [device_mapped_name] [parameters...] See usage
> >>> examples in the .uni file
> >>> ---
> >>>  .../Library/UefiShellGptCommandLib/FatFormat.c     |  611 +++++++
> >>>  .../Library/UefiShellGptCommandLib/FatFormat.h     |  111 ++
> >>>  .../Library/UefiShellGptCommandLib/GptWorker.c     | 1902
> > ++++++++++++++++++++
> >>>  .../Library/UefiShellGptCommandLib/GptWorker.h     |  186 ++
> >>>  .../UefiShellGptCommandLib.c                       | 1135
> >>> ++++++++++++
> >>>  .../UefiShellGptCommandLib.inf                     |   79 +
> >>>  .../UefiShellGptCommandLib.uni                     |  117 ++
> >>>  ShellPkg/ShellPkg.dec                              |    1 +
> >>>  ShellPkg/ShellPkg.dsc                              |    4 +
> >>>  9 files changed, 4146 insertions(+)  create mode 100644
> >>> ShellPkg/Library/UefiShellGptCommandLib/FatFormat.c
> >>>  create mode 100644
> >>> ShellPkg/Library/UefiShellGptCommandLib/FatFormat.h
> >>>  create mode 100644
> >>> ShellPkg/Library/UefiShellGptCommandLib/GptWorker.c
> >>>  create mode 100644
> >>> ShellPkg/Library/UefiShellGptCommandLib/GptWorker.h
> >>>  create mode 100644
> > ShellPkg/Library/UefiShellGptCommandLib/UefiShellGptCommandLib.c
> >>>  create mode 100644
> > ShellPkg/Library/UefiShellGptCommandLib/UefiShellGptCommandLib.inf
> >>>  create mode 100644
> > ShellPkg/Library/UefiShellGptCommandLib/UefiShellGptCommandLib.uni
> >>
> >> This looks like a supremely welcome, long-awaited addition (latest
> >> request:
> >> <https://lists.01.org/pipermail/edk2-devel/2016-October/002667.html>)
> >> , but it really needs your Signed-off-by, and the Contributed-under
> >> line above it:
> >>
> >> ShellPkg/Contributions.txt
> >>
> >> I would also suggest (simply based on what I've seen elsewhere in
> >> edk2) to keep the copyright notices tightly collected in the file
> >> headings.
> >>
> >> Someone will have to go over all the licenses too -- does the
> >> "Marvell BSD License Option" for example correspond to the 3-clause
> BSDL?
> >>
> >> On the technical side, I believe that as long as a shell command (or
> >> a command option) is not standardized (in the shell spec), it usually
> >> starts with an underscore (_), so as to prevent future name collisions.
> >> (I could be wrong about this -- I now recall the TFTP command, which
> >> is also not in the 2.2 spec.)
> >>
> >> Just my two cents.
> >>
> >> Thanks
> >> Laszlo
> > _______________________________________________
> > edk2-devel mailing list
> > edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>
> > https://lists.01.org/mailman/listinfo/edk2-devel
> >
> > _______________________________________________
> > edk2-devel mailing list
> > edk2-devel@lists.01.org
> > https://lists.01.org/mailman/listinfo/edk2-devel
> > _______________________________________________
> > edk2-devel mailing list
> > edk2-devel@lists.01.org
> > https://lists.01.org/mailman/listinfo/edk2-devel
> >