Paulo, Supporting multiple LVDs can be considered as a long-term task, or can be added per request. But I think a urgent fix in PartitionDxe driver is to: 1. create child BLK only for the portion covered by the LVD 2. Do not create child BLK for LVD that's actually an Eltorito LVD
I submitted a Bugzilla to record this: https://bugzilla.tianocore.org/show_bug.cgi?id=707 Thanks/Ray > -----Original Message----- > From: Paulo Alcantara [mailto:[email protected]] > Sent: Friday, September 15, 2017 3:22 PM > To: Ni, Ruiyu <[email protected]>; Laszlo Ersek <[email protected]>; Yao, > Jiewen <[email protected]> > Cc: [email protected]; Wu, Hao A <[email protected]>; Zeng, Star > <[email protected]> > Subject: RE: Functionality issues in UDF support > > > > On September 15, 2017 4:10:10 AM GMT-03:00, "Ni, Ruiyu" > <[email protected]> wrote: > >Paulo, > >Can current code support multiple volumes in CDROM? > > No. It can handle only a single UDF logical volume. I think I never found an > UDF disk or image with more than a LVD (Logical Volume Descriptior). > Although I think I also added some comment in the code that it currently > handles only one logical volume per disk (no access to the code right now, > sorry). Other than that, there should be also a DEBUG() or perhaps an > ASSERT() in case more than one is found. > > For supporting multiple logical volumes, we would have to follow your > suggestion and change Partition driver to install a child handle for each LVD > found in an UDF fs. > > Thanks! > Paulo > > > > >Thanks/Ray > > > >> -----Original Message----- > >> From: Paulo Alcantara [mailto:[email protected]] > >> Sent: Friday, September 15, 2017 2:26 PM > >> To: Ni, Ruiyu <[email protected]>; Laszlo Ersek <[email protected]> > >> Cc: [email protected]; Wu, Hao A <[email protected]>; Zeng, > >Star > >> <[email protected]> > >> Subject: RE: Functionality issues in UDF support > >> > >> > >> > >> Ray, > >> > >> On September 15, 2017 3:02:11 AM GMT-03:00, "Ni, Ruiyu" > >> <[email protected]> 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:[email protected]] > >> >> Sent: Friday, September 15, 2017 1:38 PM > >> >> To: Ni, Ruiyu <[email protected]>; Laszlo Ersek > ><[email protected]> > >> >> Cc: [email protected]; Wu, Hao A <[email protected]>; Zeng, > >> >Star > >> >> <[email protected]> > >> >> Subject: RE: Functionality issues in UDF support > >> >> > >> >> > >> >> > >> >> Hi, > >> >> > >> >> On September 15, 2017 2:08:28 AM GMT-03:00, "Ni, Ruiyu" > >> >> <[email protected]> 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:[email protected]] > >> >> >> Sent: Friday, September 15, 2017 12:58 PM > >> >> >> To: Ni, Ruiyu <[email protected]>; Laszlo Ersek > >> ><[email protected]> > >> >> >> Cc: [email protected]; Wu, Hao A <[email protected]>; > >Zeng, > >> >> >Star > >> >> >> <[email protected]> > >> >> >> Subject: Re: Functionality issues in UDF support > >> >> >> > >> >> >> > >> >> >> > >> >> >> Hi Ray, > >> >> >> > >> >> >> On September 15, 2017 12:33:04 AM GMT-03:00, "Ni, Ruiyu" > >> >> >> <[email protected]> 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. > > -- > Sent from my Android device with K-9 Mail. Please excuse my brevity. _______________________________________________ edk2-devel mailing list [email protected] https://lists.01.org/mailman/listinfo/edk2-devel
