Re: [PATCH] GPT Shell Application/Library

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

OK,
Thank you for advice.

> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
> Shah, Tapan
> Sent: October-18-16 11:04 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
>
> 1. Create this new shared library in MdeModulePkg.
> 2. For parameters: Other than standard parameters defined in Shell
> specification I would suggest to follow single character flag where it
> makes
> sense (example: -d to delete, -l to list, -c to create etc.) and if more
> than one
> operation can be tied with one flag, then add individual value as a flag
> value.
> There are plenty of ShellPkg commands you can use as a reference.
>
> -----Original Message-----
> From: Carsey, Jaben [mailto:jaben.carsey@intel.com]
> Sent: Tuesday, October 18, 2016 12:04 PM
> To: Vladimir Olovyannikov <vladimir.olovyannikov@broadcom.com>; Shah,
> Tapan <tapandshah@hpe.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>; Laszlo Ersek
> <lersek@redhat.com>; Carsey, Jaben <jaben.carsey@intel.com>
> Subject: RE: [edk2] [PATCH] GPT Shell Application/Library
>
> For the standards, I would try to use the UEFI Shell Spec for some
> parameters.  For Example:
> -v for verbose
> -h for help
> -sfo for standard format output
>
> > -----Original Message-----
> > From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
> > Vladimir Olovyannikov
> > Sent: Tuesday, October 18, 2016 9:58 AM
> > To: Shah, Tapan <tapandshah@hpe.com>; Carsey, Jaben
> > <jaben.carsey@intel.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>; Laszlo Ersek <lersek@redhat.com>
> > Subject: Re: [edk2] [PATCH] GPT Shell Application/Library
> > Importance: High
> >
> > 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.
> > 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
> > dcom.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
> > _______________________________________________
> > 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