Re: [PATCH v2 0/6] read-only UDF file system support

[Paulo Alcantara <pcacjr@zytor.com>]

Hi,

On 8/22/2017 2:21 PM, Andrew Fish wrote:
> 
>> On Aug 22, 2017, at 6:14 AM, Paulo Alcantara <pcacjr@zytor.com> wrote:
>>
>> Hi,
>>
>> I do apologize my late replies. At the moment, I'm only able to work on this during my free time. Thank you all for the reviews!
>>
>> FWIW, my comments below.
>>
>> On 8/20/2017 11:29 PM, Ni, Ruiyu wrote:
>>> Paulo,
>>> 1. Could you please run the ECC check tool (BaseTools\Source\Python\Ecc\)
>>>    "CRC" might need to be replaced with "Crc".
>>>    I also noticed some TAB key in file content.
>>
>> Sure.
>>
>>> 2. Your current implementation uses HARD_DRIVE_DEVICE_PATH.
>>>    But with:
>>>       SignatureType = SIGNATURE_TYPE_UDF
>>>       MBRType = MBR_TYPE_PCAT
>>>       Signature = *
>>>    And later UdfDxe driver checks the SignatureType and MBRType.
>>>    I am not sure if it would be better to put the definitions in UEFI Spec,
>>>    since they are referenced by different modules.
>>>    I also noticed you use PARTITION_TYPE_OTHER for PartitionInfo.
>>>    When proposing to UEFI Spec, this also needs to be considered,
>>>    for example, add PARTITION_TYPE_UDF to spec.
>>
>> Yes - I agree with you. My only concern is that UEFI specification doesn't either support UDF or there is any interest in supporting it, so by proposing an additional type for something that shouldn't be supported, might not work out.
>>
>> (Andrew, any thoughts?)
>>
> 
> The enum space is owned by the UEFI spec and should never be extended outside the scope of the spec. Its not good to have an implementation running around that is not in a released spec, as it it is a future compatibility risk.
> 
> Does the UDF actually start with a real MBR? If so is it possible to define a 32-bit MBR signature to indicate UDF. If not it should probably be a device path node like CD-ROM.

No. But it's possible to create an UDF file system inside a MBR/GPT 
partition so it would end up having a valid HARDDRIVE_DEVICE_PATH and no 
fake MBR device path wouldn't be created or needed at all.

The only problem is that when we find an UDF file system which isn't 
part of either a MBR or a GPT partition table, and then there is no 
matching device path for using with it.

> You can all ways use the Vendor-Defined Media Device Path since it has GUID there is no risk of collision.

Right. If you guys agree, I'll start using the vendor-defined device 
path again for all the cases where an UDF file system is found -- 
avoiding to break anything that's unrelated.

Thanks Andrew!

Paulo