On 16.07.2012 14:23, Andriy Gapon wrote:
> on 26/06/2012 15:50 Andrey V. Elsukov said the following:
>> 3. ZFS code now uses new API and probing on the systems with many disks
>> should be greatly increased:
>>         zfs/zfs.c
>>         i386/loader/main.c
> 
> First of all, it's hard to parse the above sentence. "probing ... should be
> greatly increased".  Probing what? :-)  If probing time, then we don't want 
> that ;-)
> 
> I looked through the ZFS-related part and here are a few comments:

Thanks for that.

> 1. I think that the predominant indentation style of i386/loader/main.c 
> should be
> preserved for consistency.
> 
> 2. I am not sure if I like the approach of moving partition tasting code into
> common ZFS code (zfs.c).  On one hand, it now makes sense because the new
> partition iteration code is machine-independent.  On the other hand, the 
> reason
> that I added arch_zfs_probe method was to give platforms full control over 
> which
> partitions and in what order are probed.  It seems to be important for some 
> of them.
> So, I like how your new partition interface makes it much easier to ZFS-probe
> partitions, but I would prefer to have that code in arch_zfs_probe 
> implementations
> rather than in zfs_probe_dev.

From the other point of view, ZFS is not a just file system and it works
directly with disks and partitions. And it seems to me this code will be common
for other architectures.

> 3.  Related to the above.  In what shape is sparc64 ZFS support in your 
> branch?
> Have you tried to adapt it to the new model too?
> It's the platform that has special requirements for disk/partition probing 
> order.
> Marius can help with additional information and testing here.

Currently i have not received any feedback reports from the users who can test
patches on the other architectures. I added VTOC8 support to the part.c, but it
seems it is not needed and ofw can work without this.


-- 
WBR, Andrey V. Elsukov


Attachment: signature.asc
Description: OpenPGP digital signature

Reply via email to