Re: Functionality issues in UDF support

[Paulo Alcantara <pcacjr@zytor.com>]

Hi,

On September 15, 2017 2:08:28 AM GMT-03:00, "Ni, Ruiyu" <ruiyu.ni@intel.com> wrote:
>Hi Paulo,
>The responsibility of PartitionDxe driver is to partition the physical
>BLK (the entire CDROM)
>into logical BLKs (one BLK for each volume).
>It doesn't make sense to create a child BLK handle which still covers
>the entire CDROM.
>So as I suggested in below, PartitionInstallUdfChildHandles() should
>contain a for-loop to iterate
>all the volumes in the CDROM, and create child BLK handle for each
>volumes, but skipping Eltorito volume.

Ah, makes sense to me now. Thanks for clarifying it. So I agree with you.

In whatever you guys decide to do with it, count on me to give some help.

Thanks!
Paulo

>
>Thanks/Ray
>
>> -----Original Message-----
>> From: Paulo Alcantara [mailto:pcacjr@zytor.com]
>> Sent: Friday, September 15, 2017 12:58 PM
>> To: Ni, Ruiyu <ruiyu.ni@intel.com>; Laszlo Ersek <lersek@redhat.com>
>> Cc: edk2-devel@lists.01.org; Wu, Hao A <hao.a.wu@intel.com>; Zeng,
>Star
>> <star.zeng@intel.com>
>> Subject: Re: Functionality issues in UDF support
>> 
>> 
>> 
>> Hi Ray,
>> 
>> On September 15, 2017 12:33:04 AM GMT-03:00, "Ni, Ruiyu"
>> <ruiyu.ni@intel.com> wrote:
>> >Paulo,
>> >Before raising my questions, I'd like to confirm that for a single
>> >CD/DVD in UDF format, there might be multiple volumes.
>> >One of the volume could be Eltorito type.
>> >If my understanding is correct, please continue reading below.
>> 
>> Right.
>> 
>> >
>> >We found below mapping table using "map -r" shell command in a
>platform
>> >with only PartitionDxe change and without UdfDxe driver.
>> >It's a bug that <BLK6> and <BLK7/FS2> are created. Actually they are
>> >identical to <BLK3> and <BLK4/FS1>.
>> >
>> >--- Mapping table---
>> >     BLK2: Alias(s):
>> >          PciRoot(0x0)/Pci(0x1F,0x2)/Sata(0x0,0xFFFF,0x0)
>> >
>> >     BLK3: Alias(s):
>> >          PciRoot(0x0)/Pci(0x1F,0x2)/Sata(0x0,0xFFFF,0x0)/CDROM(0x0)
>> >      FS1: Alias(s):CD1a65535a1:;BLK4:
>> >          PciRoot(0x0)/Pci(0x1F,0x2)/Sata(0x0,0xFFFF,0x0)/CDROM(0x1)
>> >
>> >     BLK5: Alias(s):
>> >PciRoot(0x0)/Pci(0x1F,0x2)/Sata(0x0,0xFFFF,0x0)/VenMedia(C5BD4D42-
>> 1A76-4996-8956-73CDA326CD0A)
>> >     BLK6: Alias(s):
>> >PciRoot(0x0)/Pci(0x1F,0x2)/Sata(0x0,0xFFFF,0x0)/VenMedia(C5BD4D42-
>> 1A76-4996-8956-73CDA326CD0A)/CDROM(0x0)
>> >      FS2: Alias(s):CD1a65535ab:;BLK7:
>> >PciRoot(0x0)/Pci(0x1F,0x2)/Sata(0x0,0xFFFF,0x0)/VenMedia(C5BD4D42-
>> 1A76-
>> >4996-8956-73CDA326CD0A)/CDROM(0x1)
>> >--- End of mapping table ---
>> >
>> >After investigation, I found the UDF logic in Partition driver
>doesn't
>> >truly skip the Eltorito volume.
>> >The code flow is like below:
>> >
>> >  1.  <BLK2> is created by ScsiDiskDxe driver.
>> >  2.  By passing <BLK2> to PartitionDxe Start()
>> >*   <BLK3> and <BLK4/FS1> are created by PartitionDxe driver, by
>> >PartitionInstallElToritoChildHandles().
>> >*   <BLK5> is created by PartitionDxe driver, by
>> >PartitionInstallUdfChildHandles().
>> >  3.  By passing <BLK5> to PartitionDxe Start()
>> >*   <BLK6> and <BLK7/FS2> are created by PartitionDxe driver, by
>> >PartitionInstallElToritoChildHandles().
>> >
>> >I think step 2.a is not correct if my understanding to UDF is
>correct.
>> >The PartitionInstallUdfChildHandles() should iterate all volumes in
>the
>> >media and creates the child BLK handle for each volume, but skipping
>> >Eltorito volume.
>> >
>> >Instead, the current implementation just creates one child BLK
>handle
>> >for the entire media.
>> >To avoid reclusively creating  child BLK handle, the
>> >PartitionInstallUdfChildHandles() contains a logic to do nothing
>when
>> >the handle is created by ParititonDxe driver. The logic is not
>needed
>> >when the implementation follows my suggestion above.
>> >Due to this, step 3.a creates the additional but shouldn't-exist BLK
>> >handles <BLK6> and <BLK7/FS2>.
>> >
>> >UdfDxe driver is supposed to Start() on each volume and produce
>> >SimpleFileSystem protocol.
>> 
>> It seems that PartitionInstallUdfChildHandles() indeed skips the
>ElTorito
>> partitions otherwise you'd see the ".../CDROM(0x1)/VenMedia()" and
>> ".../CDROM(0x0)/VenMedia()" device paths *also* in the mapping
>output.
>> 
>> If I understand correctly, the problem is that when we create a child
>handle
>> for an UDF volume, Partition driver will execute again, parse the
>newly
>> created UDF handle, find again an ElTorito partition and then install
>a new
>> child handle (VenMedia()/CDROM())
>> 
>> So, the logic of skipping of ElTorito partitions in
>> PartitionInstallUdfChildHandles() is not enough. Unfortunately we
>can't
>> handle UDF bridge disks (ElTorito + UDF) entirely in
>> PartitionInstallUdfChildHandles() -- we should probably also skip UDF
>device
>> paths in PartitionInstallElToritoChildHandles().
>> 
>> Does it make sense to you, Ray? Thanks for raising this up.
>> 
>> Paulo
>> 
>> >
>> >Do you agree with my above suggestions?
>> >
>> >Laszlo,
>> >I understood your needs of this UDF support. But as you can see
>there
>> >are many build failures and even functionality issues due to this
>> >support.
>> >I am not sure how the other open source project handles such cases.
>> >But I am thinking maybe we could move the whole UDF support to
>> >edk2-staging firstly and move it back after all the issues are
>> >resolved.
>> >What's your suggestion?
>> >
>> >Thanks/Ray
>> 
>> --
>> Sent from my Android device with K-9 Mail. Please excuse my brevity.

-- 
Sent from my Android device with K-9 Mail. Please excuse my brevity.