Re: [PATCH v4 5/7] ext2fs: Add helper functions to access inode numbers

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hello Theodore,

I need your suggestions. There are many e2fsprogs libs interfaces which use ext2_ino_t type for parameters. 
Obviously, we need save previous 32-bit versions of this interfaces for already compiled code and add new 64-bit inodes  somehow. 
I tried Andreas’ idea. Changed ext2_ino_t to 64bit and added ext2_ino32_t, but things become too complicated.
ext2_ino_t is quite popular type. Adding two versions of all this code is quite large patch. I am not sure you will be agree to accept it.

So I want to discuss some alternatives:

1) We could compile and link two lib versions: one with 32bit ext2_ino_t and another with 64bit.
Pass some macross that says “ext2_ino_t” is *bit now. User can link both libraries
(functions have same names, but different prototypes). I believe some extra cleanup is needed.
 There are some local variables and functions parameters which have type “bitness” hardcoded. 
But probably this less work then make both interface versions.

2) we could use LD Version Scripts
https://www.gnu.org/software/gnulib/manual/html_node/LD-Version-Scripts.html
This approach looks elegant, but still need more work to be done

Any other ideas? 

Thanks,

Artem Blagodarenko.

> On 4 May 2018, at 12:34, Andreas Dilger <adilger@xxxxxxxxx> wrote:
> 
> On May 4, 2018, at 1:09 AM, c17828 <artem.blagodarenko@xxxxxxxxx> wrote:
>> 
>> From: Artem Blagodarenko <artem.blagodarenko@xxxxxxxxx>
>> 
>> 64-bit inodes counter uses extra fields to store hight part.
>> Let's incapsulate inode number reading and writing to extend
>> counter in next commits.
>> 
>> Lustre-bug: https://jira.hpdd.intel.com/browse/LU-9309
>> Signed-off-by: Artem Blagodarenko <artem.blagodarenko@xxxxxxxxx>
>> ---
> 
> So this patch appears fine by itself, but I don't see in the 6/7 patch
> where all of the interfaces in lib/ext2fs/ext2fs.h that use ext2fs_ino_t
> are updated to add a new "version 2" interface that takes ext2fs_ino64_t
> instead?
> 
> This would otherwise break ABI compatibility, and likely also apps
> that are using the old interfaces with a 32-bit inode number.
> 
> It seems to me that you have fixed the places where there can be a
> 64-bit *count* of inodes, but you haven't fixed all of the places that
> have a 64-bit inode *number* (i.e. passed as a parameter to a function).
> 
> I think one good option would be to introduce a new "ext2fs_ino32_t"
> and *not* add "ext2fs_ino64_t".  Then change the ext2fs_ino_t to be
> 64-bit and make the old interfaces use ext2fs_ino32_t.  That way newly
> compiled applications using libext2fs will use the new 64-bit interfaces
> and the old ones would just need the old interfaces for compatibility.
> I'm not sure what Ted thinks about that, however.
> 
> I believe there is also a "library ABI checker" tool somewhere in the
> build system, but I don't know how to activate it.
> 
> Cheers, Andreas
> 
> 
> 
> 
> 





[Index of Archives]     [Reiser Filesystem Development]     [Ceph FS]     [Kernel Newbies]     [Security]     [Netfilter]     [Bugtraq]     [Linux FS]     [Yosemite National Park]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Device Mapper]     [Linux Media]

  Powered by Linux