From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id C19A21A1E4F for ; Tue, 18 Oct 2016 11:12:23 -0700 (PDT) Received: from int-mx13.intmail.prod.int.phx2.redhat.com (int-mx13.intmail.prod.int.phx2.redhat.com [10.5.11.26]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 323124E335; Tue, 18 Oct 2016 18:12:23 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-116-72.phx2.redhat.com [10.3.116.72]) by int-mx13.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id u9IICKjb026739; Tue, 18 Oct 2016 14:12:21 -0400 To: Vladimir Olovyannikov , "Shah, Tapan" , "Carsey, Jaben" , Michael Zimmermann References: <1476595420-12566-1-git-send-email-vladimir.olovyannikov@broadcom.com> <40b80588-1bb4-e5f2-439c-97a405c873d3@redhat.com> <1e599793ddedca50db54fb22b429cf53@mail.gmail.com> <38cdf2c64b531db8250f900f4f4193cb@mail.gmail.com> Cc: "Ni, Ruiyu" , "Arshi, Shala" , "edk2-devel@lists.01.org" From: Laszlo Ersek Message-ID: <2fe18e8f-3e9d-e365-42d1-fd107b65f590@redhat.com> Date: Tue, 18 Oct 2016 20:12:20 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.4.0 MIME-Version: 1.0 In-Reply-To: X-Scanned-By: MIMEDefang 2.68 on 10.5.11.26 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.38]); Tue, 18 Oct 2016 18:12:23 +0000 (UTC) Subject: Re: [PATCH] GPT Shell Application/Library X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 18 Oct 2016 18:12:24 -0000 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit On 10/18/16 20:03, Vladimir Olovyannikov wrote: >> -----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. 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-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 ; >>> Michael Zimmermann >>> Cc: Ni, Ruiyu ; Arshi, Shala >>> ; edk2-devel@lists.01.org >>> ; Carsey, Jaben ; >>> Laszlo Ersek >>> 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 >>> Cc: Laszlo Ersek ; Carsey, Jaben >>> ; Ni, Ruiyu ; >>> edk2-devel@lists.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] >>> Sent: October-17-16 12:25 AM >>> To: Vladimir Olovyannikov >>> Cc: Laszlo Ersek; Jaben Carsey; Ni, Ruiyu; >>> 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 >>> >> > 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" >>> > 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 [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: >>>> ) >>>> , 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 >>> 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 >>>