From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail.zytor.com (terminus.zytor.com [65.50.211.136]) (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 8BC5521E0C2FF for ; Wed, 9 Aug 2017 07:01:20 -0700 (PDT) Received: from [10.26.0.110] (corporativo.static.gvt.net.br [177.135.97.54] (may be forged)) (authenticated bits=0) by mail.zytor.com (8.15.2/8.15.2) with ESMTPSA id v79E1KB7030130 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128 verify=NO); Wed, 9 Aug 2017 07:01:22 -0700 From: Paulo Alcantara To: "Ni, Ruiyu" , "Zeng, Star" , "edk2-devel@lists.01.org" Cc: "Dong, Eric" , "Wu, Hao A" , "Justen, Jordan L" , Andrew Fish , "Gao, Liming" , "Kinney, Michael D" , Laszlo Ersek References: <20170808193143.18128-1-pcacjr@zytor.com> <0C09AFA07DD0434D9E2A0C6AEB0483103B9132AF@shsmsx102.ccr.corp.intel.com> <734D49CCEBEEF84792F5B80ED585239D5B9DED45@SHSMSX104.ccr.corp.intel.com> <50a39f82-576f-f0bb-c133-4b448c332ca5@zytor.com> Message-ID: <7e6edff5-2d39-80bb-91dc-c47e332b28ad@zytor.com> Date: Wed, 9 Aug 2017 11:01:17 -0300 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:52.0) Gecko/20100101 Thunderbird/52.2.1 MIME-Version: 1.0 In-Reply-To: <50a39f82-576f-f0bb-c133-4b448c332ca5@zytor.com> Subject: Re: [PATCH 0/4] read-only UDF file system support X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 09 Aug 2017 14:01:20 -0000 Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 8bit (heh - forgot to answer your question about the GUID :-) ) On 8/9/2017 10:26 AM, Paulo Alcantara wrote: > Hi Ray, > > Thanks for the review. My comments below. > > On 8/9/2017 3:05 AM, Ni, Ruiyu wrote: >> Paulo, >> Thanks for enabling the UDF support into EDKII. >> Below are several comments: >> 1. Could you please separate the patch to modify MdePkg and MdeModulePkg? > > Sure. > >> 2. UDF_CDROM_VOLUME_IDENTIFIER is also defined in Eltorito.h as CDVOL_ID. >> Maybe we could redefine CDVOL_ID to UDF_CDROM_VOLUME_IDENTIFIER? > > The Volume Identifier structure is defined in ECMA-167 specification > (not UDF specific), so perhaps it would be better if we create a > separate header file (e.g., Ecma-167.h) and define > ECMA167_VOLUME_IDENTIFIER structure. That would be used in both ElTorito > and UDF codes. What do you think? > >> >> 2. Why do you need a PCD to control the UDF support? I prefer no. More >> choices >> is not always good😊 > > The original idea of including this PCD was to avoid adding unnecessary > overhead in PartitionDxe driver to something (UDF) that wasn't actually > defined in UEFI specification -- leaving it as an optional feature. > > But yes, I agree with you that just complicates things and would be > better to remove it. > >> >> 3. Is gUdfVolumeSignatureGuid only used in Partition driver to >> produce the device >> path? Can we just use HARDDRIVE_DEVICE_PATH? Or at least >> gUdfVolumeSignatureGuid >> can be removed, the GUID macro is enough? The GUID is used in both Partition and UdfDxe drivers. Do you mean that we should use HARDDRIVE_DEVICE_PATH and then appending the UDF-specific GUID to it? Thanks, Paulo