btrfs-progs: Restrict e2fsprogs version for new convert
diff mbox

Message ID 20160502123321.GH29353@twin.jikos.cz
State New
Headers show

Commit Message

David Sterba May 2, 2016, 12:33 p.m. UTC
On Mon, Apr 25, 2016 at 04:55:32PM +0200, David Sterba wrote:
> On Mon, Apr 18, 2016 at 09:20:18AM +0800, Qu Wenruo wrote:
> > 
> > 
> > David Sterba wrote on 2016/04/15 13:17 +0200:
> > > On Thu, Apr 14, 2016 at 02:24:34PM +0800, Qu Wenruo wrote:
> > >> New btrfs-convert is using a lot of new macro in e2fsprogs 1.42.
> > >> Unfortunately the new compatible layer for older e2fsprogs is still
> > >> under development.
> > >
> > > It hasn't been released yet so it's not really a big problem, although
> > > it makes testing on my side a bit harder. The configure-time check
> > > should be 1.41 and until it's fixed we can print a warning.
> > >
> > >
> > Did I missed something?
> > 
> > I checkout 1.41.14 and it shows no cluster support in ext2fs.h.
> > 
> > Also git describe shows it's v1.41.14-36-g1da5ef7, after the last v1.41 
> > version.
> > 
> > So I think the check should be 1.42, just as the patch.
> 
> The idea is to keep lowest supported version 1.41, because this version
> can be commonly found on enterprise distros. The lack of cluster is
> expected and needs to be dealt with both build- and run-time.

I've committed the following patch, as it was your idea to add the
version check:

btrfs-progs: Restrict e2fsprogs version for convert

We want to support version 1.41 due to longterm and enterprise distros,
make the check explicit.

Signed-off-by: Qu Wenruo <quwenruo@cn.fujitsu.com>
Signed-off-by: David Sterba <dsterba@suse.com>

the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Qu Wenruo May 3, 2016, 12:59 a.m. UTC | #1
David Sterba wrote on 2016/05/02 14:33 +0200:
> On Mon, Apr 25, 2016 at 04:55:32PM +0200, David Sterba wrote:
>> On Mon, Apr 18, 2016 at 09:20:18AM +0800, Qu Wenruo wrote:
>>>
>>>
>>> David Sterba wrote on 2016/04/15 13:17 +0200:
>>>> On Thu, Apr 14, 2016 at 02:24:34PM +0800, Qu Wenruo wrote:
>>>>> New btrfs-convert is using a lot of new macro in e2fsprogs 1.42.
>>>>> Unfortunately the new compatible layer for older e2fsprogs is still
>>>>> under development.
>>>>
>>>> It hasn't been released yet so it's not really a big problem, although
>>>> it makes testing on my side a bit harder. The configure-time check
>>>> should be 1.41 and until it's fixed we can print a warning.
>>>>
>>>>
>>> Did I missed something?
>>>
>>> I checkout 1.41.14 and it shows no cluster support in ext2fs.h.
>>>
>>> Also git describe shows it's v1.41.14-36-g1da5ef7, after the last v1.41
>>> version.
>>>
>>> So I think the check should be 1.42, just as the patch.
>>
>> The idea is to keep lowest supported version 1.41, because this version
>> can be commonly found on enterprise distros. The lack of cluster is
>> expected and needs to be dealt with both build- and run-time.
>
> I've committed the following patch, as it was your idea to add the
> version check:

I'm fine to 1.41 for now, but I'm afraid that the compatible layer won't 
be any time soon.
So it may still broke 1.41 e2fsprogs.

(My fault, I still didn't quite know how e2fsprogs are handling the new 
RO compact feature)

Thanks,
Qu

>
> btrfs-progs: Restrict e2fsprogs version for convert
>
> We want to support version 1.41 due to longterm and enterprise distros,
> make the check explicit.
>
> Signed-off-by: Qu Wenruo <quwenruo@cn.fujitsu.com>
> Signed-off-by: David Sterba <dsterba@suse.com>
>
> diff --git a/configure.ac b/configure.ac
> index fc343ea1aa5d..797eb7903520 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -105,7 +105,7 @@ AS_IF([test "x$enable_convert" = xyes], [DISABLE_BTRFSCONVERT=0], [DISABLE_BTRFS
>  AC_SUBST([DISABLE_BTRFSCONVERT])
>
>  if test "x$enable_convert" = xyes; then
> -       PKG_CHECK_MODULES(EXT2FS, [ext2fs])
> +       PKG_CHECK_MODULES(EXT2FS, [ext2fs >= 1.41])
>         PKG_CHECK_MODULES(COM_ERR, [com_err])
>  fi
> --
> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
>


--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
David Sterba May 6, 2016, 3:03 p.m. UTC | #2
On Tue, May 03, 2016 at 08:59:58AM +0800, Qu Wenruo wrote:
> 
> 
> David Sterba wrote on 2016/05/02 14:33 +0200:
> > On Mon, Apr 25, 2016 at 04:55:32PM +0200, David Sterba wrote:
> >> On Mon, Apr 18, 2016 at 09:20:18AM +0800, Qu Wenruo wrote:
> >>>
> >>>
> >>> David Sterba wrote on 2016/04/15 13:17 +0200:
> >>>> On Thu, Apr 14, 2016 at 02:24:34PM +0800, Qu Wenruo wrote:
> >>>>> New btrfs-convert is using a lot of new macro in e2fsprogs 1.42.
> >>>>> Unfortunately the new compatible layer for older e2fsprogs is still
> >>>>> under development.
> >>>>
> >>>> It hasn't been released yet so it's not really a big problem, although
> >>>> it makes testing on my side a bit harder. The configure-time check
> >>>> should be 1.41 and until it's fixed we can print a warning.
> >>>>
> >>>>
> >>> Did I missed something?
> >>>
> >>> I checkout 1.41.14 and it shows no cluster support in ext2fs.h.
> >>>
> >>> Also git describe shows it's v1.41.14-36-g1da5ef7, after the last v1.41
> >>> version.
> >>>
> >>> So I think the check should be 1.42, just as the patch.
> >>
> >> The idea is to keep lowest supported version 1.41, because this version
> >> can be commonly found on enterprise distros. The lack of cluster is
> >> expected and needs to be dealt with both build- and run-time.
> >
> > I've committed the following patch, as it was your idea to add the
> > version check:
> 
> I'm fine to 1.41 for now, but I'm afraid that the compatible layer won't 
> be any time soon.
> So it may still broke 1.41 e2fsprogs.

This means the convert rework has to be postponed until the
compatibility with 1.41 is implemented, otherwise it would be regression.
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Qu Wenruo May 9, 2016, 1:36 a.m. UTC | #3
David Sterba wrote on 2016/05/06 17:03 +0200:
> On Tue, May 03, 2016 at 08:59:58AM +0800, Qu Wenruo wrote:
>>
>>
>> David Sterba wrote on 2016/05/02 14:33 +0200:
>>> On Mon, Apr 25, 2016 at 04:55:32PM +0200, David Sterba wrote:
>>>> On Mon, Apr 18, 2016 at 09:20:18AM +0800, Qu Wenruo wrote:
>>>>>
>>>>>
>>>>> David Sterba wrote on 2016/04/15 13:17 +0200:
>>>>>> On Thu, Apr 14, 2016 at 02:24:34PM +0800, Qu Wenruo wrote:
>>>>>>> New btrfs-convert is using a lot of new macro in e2fsprogs 1.42.
>>>>>>> Unfortunately the new compatible layer for older e2fsprogs is still
>>>>>>> under development.
>>>>>>
>>>>>> It hasn't been released yet so it's not really a big problem, although
>>>>>> it makes testing on my side a bit harder. The configure-time check
>>>>>> should be 1.41 and until it's fixed we can print a warning.
>>>>>>
>>>>>>
>>>>> Did I missed something?
>>>>>
>>>>> I checkout 1.41.14 and it shows no cluster support in ext2fs.h.
>>>>>
>>>>> Also git describe shows it's v1.41.14-36-g1da5ef7, after the last v1.41
>>>>> version.
>>>>>
>>>>> So I think the check should be 1.42, just as the patch.
>>>>
>>>> The idea is to keep lowest supported version 1.41, because this version
>>>> can be commonly found on enterprise distros. The lack of cluster is
>>>> expected and needs to be dealt with both build- and run-time.
>>>
>>> I've committed the following patch, as it was your idea to add the
>>> version check:
>>
>> I'm fine to 1.41 for now, but I'm afraid that the compatible layer won't
>> be any time soon.
>> So it may still broke 1.41 e2fsprogs.
>
> This means the convert rework has to be postponed until the
> compatibility with 1.41 is implemented, otherwise it would be regression.
>
>
I'll try my best to implement it asap.

As the BIGALLOC ro compat feature is not really ro compat.
That changed how we get used space.
(Even we are only getting used space, it much like space allocation though)

So for 1.41 e2fsprogs, we must implement(copy from dump2fs though) a new 
method for get ext2 used space.

And for 1.41 case, we won't even support any unknown RO compat flags, 
like BIGALLOC.

If that's OK for you, I think it can be done in this week.

Thanks,
Qu


--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch
diff mbox

diff --git a/configure.ac b/configure.ac
index fc343ea1aa5d..797eb7903520 100644
--- a/configure.ac
+++ b/configure.ac
@@ -105,7 +105,7 @@  AS_IF([test "x$enable_convert" = xyes], [DISABLE_BTRFSCONVERT=0], [DISABLE_BTRFS
 AC_SUBST([DISABLE_BTRFSCONVERT])

 if test "x$enable_convert" = xyes; then
-       PKG_CHECK_MODULES(EXT2FS, [ext2fs])
+       PKG_CHECK_MODULES(EXT2FS, [ext2fs >= 1.41])
        PKG_CHECK_MODULES(COM_ERR, [com_err])
 fi
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in