On 10/18/16 20:03, Vladimir Olovyannikov wrote: >> -----Original Message----- >> From: Laszlo Ersek [mailto:ler...@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.
I understood that, yes. My point was the location of the new common library (class header and instance both). You wrote "I will create a Shell library ..." which made me think you wanted to place the files under: ShellPkg/Include/Library/... ShellPkg/Library/... which is not right in this instance. I suggested to place those files under MdeModulePkg/Include/Library/... MdeModulePkg/Library/... Thanks! Laszlo > > 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-boun...@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-boun...@lists.01.org] On Behalf Of >>> Carsey, Jaben >>> Sent: Monday, October 17, 2016 12:56 PM >>> To: Vladimir Olovyannikov <vladimir.olovyanni...@broadcom.com>; >>> Michael Zimmermann <sigmaepsilo...@gmail.com> >>> Cc: Ni, Ruiyu <ruiyu...@intel.com>; Arshi, Shala >>> <shala.ar...@intel.com>; edk2-devel@lists.01.org >>> <edk2-de...@ml01.01.org>; Carsey, Jaben <jaben.car...@intel.com>; >>> Laszlo Ersek <ler...@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.olovyanni...@broadcom.com] >>> Sent: Monday, October 17, 2016 10:52 AM >>> To: Michael Zimmermann <sigmaepsilo...@gmail.com> >>> Cc: Laszlo Ersek <ler...@redhat.com>; Carsey, Jaben >>> <jaben.car...@intel.com>; Ni, Ruiyu <ruiyu...@intel.com>; >>> edk2-devel@lists.01.org <edk2-de...@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:sigmaepsilo...@gmail.com<mailto:sigmaepsilo...@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.olovyanni...@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" >>> <ler...@redhat.com<mailto:ler...@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
