Re: Functionality issues in UDF support

["Ni, Ruiyu" <ruiyu.ni@intel.com>]

Star,
Eltorito file system access should work.
But UDF support may not work as expected if there are multiple
volumes in the CDROM.

Thanks/Ray

> -----Original Message-----
> From: Zeng, Star
> Sent: Friday, September 15, 2017 3:01 PM
> To: Paulo Alcantara <pcacjr@zytor.com>; 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
> 
> How serious is this issue?
> Only causing more BLKs are shown, but not impacting Eltorito? Or impacting
> Eltorito definitely?
> 
> 
> 
> Thanks,
> Star
> -----Original Message-----
> From: Paulo Alcantara [mailto:pcacjr@zytor.com]
> Sent: Friday, September 15, 2017 2:26 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
> 
> 
> 
> Ray,
> 
> On September 15, 2017 3:02:11 AM GMT-03:00, "Ni, Ruiyu"
> <ruiyu.ni@intel.com> wrote:
> >Paulo,
> >When will you change the implementation to this way?
> 
> Unfortunately I can't promise you when I'd be doing this because, you know,
> I can only work on this in my free time (very short, ATM). Perhaps this
> weekend - but real life also happens and I may get preempted by something
> else.
> 
> Paulo
> 
> >
> >
> >Thanks/Ray
> >
> >> -----Original Message-----
> >> From: Paulo Alcantara [mailto:pcacjr@zytor.com]
> >> Sent: Friday, September 15, 2017 1:38 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,
> >>
> >> 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.
> 
> --
> Sent from my Android device with K-9 Mail. Please excuse my brevity.