Integrating patches in SLES10 e2fsprogs

Integrating patches in SLES10 e2fsprogs

[Andreas Dilger]

I was looking through the SLES10 e2fsprogs patch set, and I wonder if some
of them could be integrated upstream, and if any effort had been made in
that direction in the past?  In particular, the addition of et_list_lock()
and et_list_unlock() to libcom_err cause failures if e2fsprogs is updated
to a non-SLES10 derived RPM.


A list of patches and (my) descriptions are below:
libcom_err-no-static-buffer.patch - avoids static buffer returned to caller
				    by error_message() function
libcom_err-no-init_error_table.patch - removes init_error_table() function
				       (maybe because it isn't thread safe?),
				       but I think this could be made thread
				       safe by adding locking around use of
				       _et_dynamic_list, or maybe it is
				       obsoleted by add_error_table()?
libcom_err-no-e2fsck.static.patch - can't build e2fsck.static because of
				    -lpthread in libcom_err-mutex.patch, but
				    nothing uses e2fsck.static anymore?
libcom_err-mutex.patch - add et_list_{un,}lock() via pthread mutex


e2fsprogs-blkid.diff - Adds documentation of BLKID_FILE environment variable.
                       This is actually implemented directly in libblkid in
		       e2fsprogs-1.40.2 but no mention of it in the man pages.
e2fsprogs-mdraid.patch - allows skip of mdraid probing, not sure why?
e2fsprogs-probe_reiserfs-fpe.patch - fixes a legitimate bug in probe_reiserfs,
				     though it might be better to just return
				     an error if the blocksize is bad?


In addition to this, the SLES10 .spec file is completely different than that
shipped with upstream e2fsprogs, and I'd like to reconcile that if possible.
In particular it has libcom_err and libss in a separate RPM (libcom_err).
I understand that FC8 (not sure about RHEl5) has also split out some of the
libraries, but in a different way (e2fsprogs-libs) and that is a bit of a
headache.  It might be possible to reconcile with suitable rpm-fu, but it
would be desirable that SLES pick up these changes in the future...

I don't want to spam the list with all of the patches yet, but if there is
interest in merging these upstream then I can provide versions of these
patches against the current e2fsprogs instead of 1.38 that is in SLES10.

Eric, I haven't looked at the FC8/9 e2fsprogs yet, but do they also have
a ton of patches (possibly in the -pu branch), or do they track upstream
more closely?

Cheers, Andreas
--
Andreas Dilger
Sr. Staff Engineer, Lustre Group
Sun Microsystems of Canada, Inc.

Re: Integrating patches in SLES10 e2fsprogs

[Eric Sandeen]

Andreas Dilger wrote:

> Eric, I haven't looked at the FC8/9 e2fsprogs yet, but do they also have
> a ton of patches (possibly in the -pu branch), or do they track upstream
> more closely?

I try to track upstream as closely as I can; F8 had quite a lot of
patches for a while, and I got it whittled down to just a couple.  I
think I'm back up to 7 though :)

Patch1: e2fsprogs-1.39-blkid-devmapper.patch
Patch2: e2fsprogs-1.38-etcblkid.patch
Patch3: e2fsprogs-1.39-mkinstalldirs.patch
Patch4: e2fsprogs-1.40.4-uuidd-tidy.patch
Patch5: e2fsprogs-1.40.4-sb_feature_check_ignore.patch
Patch6: e2fsprogs-1.40.4-blkid-ext4dev.patch
Patch7: e2fsprogs-1.40.4-no-static-e2fsck.patch

...

# look at device mapper devices -
# autoconf-foo to find ldevmapper in /lib
%patch1 -p1 -b .dm
# put blkid.tab in /etc/blkid/ - fedora-ism I think
%patch2 -p1 -b .etcblkid
# Fix for newer autoconf (#220715) - only 'cause we must
# re-run autoconf due to patch 1 IIRC
%patch3 -p1 -b .mkinstalldirs
# uuidd manpage tidyup <--- sent upstream already
%patch4 -p1 -b .uuidd-tidy
# ignore some flag differences on primary/backup sb feature checks
# sent (bad... ) patch upstream, will resend
# ignores flags set on the fly by the kernel
%patch5 -p1 -b .featurecheck
# teach blkid about ext4dev, for now
%patch6 -p1 -b .ext4-blkid
# completely clobber e2fsck.static build,
# fedora is missing some .a's, and don't need it
%patch7 -p1 -b .e2fsck-static




> Cheers, Andreas
> --
> Andreas Dilger
> Sr. Staff Engineer, Lustre Group
> Sun Microsystems of Canada, Inc.
> 

Re: Integrating patches in SLES10 e2fsprogs

[Matthias Koenig]

Andreas Dilger <adilger@sun.com> writes:

> I was looking through the SLES10 e2fsprogs patch set, and I wonder if some
> of them could be integrated upstream, and if any effort had been made in
> that direction in the past?  In particular, the addition of et_list_lock()
> and et_list_unlock() to libcom_err cause failures if e2fsprogs is updated
> to a non-SLES10 derived RPM.
>
>
> A list of patches and (my) descriptions are below:
> libcom_err-no-static-buffer.patch - avoids static buffer returned to caller
> 				    by error_message() function
> libcom_err-no-init_error_table.patch - removes init_error_table() function
> 				       (maybe because it isn't thread safe?),
> 				       but I think this could be made thread
> 				       safe by adding locking around use of
> 				       _et_dynamic_list, or maybe it is
> 				       obsoleted by add_error_table()?
> libcom_err-no-e2fsck.static.patch - can't build e2fsck.static because of
> 				    -lpthread in libcom_err-mutex.patch, but
> 				    nothing uses e2fsck.static anymore?
> libcom_err-mutex.patch - add et_list_{un,}lock() via pthread mutex

This adresses
https://bugzilla.novell.com/show_bug.cgi?id=66534

> e2fsprogs-blkid.diff - Adds documentation of BLKID_FILE environment variable.
>                        This is actually implemented directly in libblkid in
> 		       e2fsprogs-1.40.2 but no mention of it in the man pages.

https://bugzilla.novell.com/show_bug.cgi?id=50156

> e2fsprogs-mdraid.patch - allows skip of mdraid probing, not sure why?

https://bugzilla.novell.com/show_bug.cgi?id=100530

> e2fsprogs-probe_reiserfs-fpe.patch - fixes a legitimate bug in probe_reiserfs,
> 				     though it might be better to just return
> 				     an error if the blocksize is bad?

https://bugzilla.novell.com/show_bug.cgi?id=115827

> In addition to this, the SLES10 .spec file is completely different than that
> shipped with upstream e2fsprogs, and I'd like to reconcile that if possible.
> In particular it has libcom_err and libss in a separate RPM (libcom_err).
> I understand that FC8 (not sure about RHEl5) has also split out some of the
> libraries, but in a different way (e2fsprogs-libs) and that is a bit of a
> headache.  It might be possible to reconcile with suitable rpm-fu, but it
> would be desirable that SLES pick up these changes in the future...

We have now at SuSE a clear policy about packaging shared libraries:
http://en.opensuse.org/Packaging/Shared_Library_Packaging_Policy
This is pretty much similar to what debian does since ages.
It might be possible to do this in one spec, so that it works for
FC and SuSE, but I don't see this being worth the effort.

> I don't want to spam the list with all of the patches yet, but if there is
> interest in merging these upstream then I can provide versions of these
> patches against the current e2fsprogs instead of 1.38 that is in SLES10.

Since the SLES10 patches are against e2fsprogs 1.38 a better base for
upstream work is Opensuse Factory, which just has been updated to 1.40.4.
Yes, there are still patches in there which I need to check for upstream
inclusion, and this is something I wanted to do since some time. I just
didn't get the time recently. But your mail is a good oppertunity to
start working on this.

Matthias

Re: Integrating patches in SLES10 e2fsprogs

[Theodore Tso]

Wow.  You have a lot of patches in the SLES 10 e2fsprogs.  I'm not
sure why of them are there, though.  For example:

Patch0:         elf.diff

I'm not sure what this one is for.

Patch1:		e2fsprogs-1.35-libdir.diff

This one does two different things.  One is include AC_HEADER_TIME in
configure.in, and the other is to use $lib instead of "lib" when
defining root_libdir.  This seems to force root_libdir to /, which
makes no sense to me.


Patch4:         e2fsprogs-blkid.diff

This patch causes fsck to check the BLKID_FILE environment variable
and passes it to the blkid library.  But the blkid library *already*
checks the BLKID_FILE environtment variable already.  So I'm not sure
why this is necessary at all.


Patch6:         e2fsprogs-mdraid.patch

This apparently adds a new environment variable,
BLKID_SKIP_CHECK_MDRAID, which forces blkid to not detect mdraid
devices.  I'm not sure why.


Patch9:         e2fsprogs-libvolume_id-support.patch

This one is going to cause problems for ext4 support, because
libvolume_id isn't going to have the right logic for ext4.  There are
also issues with volume_id library where I haven't been happy with the
choices Kay has made in terms of correctness.


Patch10:        close.patch

I don't understand what this patch is trying to do.


Patch12:        e2fsprogs-mkinstalldirs.patch

Why?


Patch22:        e2fsprogs-1.40.4-uuidd_pid_path.patch

The problem with this patch is that /var/run is cleared via rm -rf, so
it is highly problamtic to put the scratch directory for uuidd in
/var/run.


Patch32:        libcom_err-no-e2fsck.static.patch

This patch does two completely unrelated things.  One is to disable
the libcom_err regression test suite (probably because some of the
other changes made) and the other is to disable building the
e2fsck.static file.  Why these two are bundled into a single patch I'm
not sure.


Patch34:        libcom_err-compile_et_permissions.patch

Why?

							- Ted

Re: Integrating patches in SLES10 e2fsprogs

[Eric Sandeen]

Theodore Tso wrote:

> Patch10:        close.patch
> 
> I don't understand what this patch is trying to do.

* Tue Nov 15 2005 jblunck@suse.de
- added close.patch provided by Ted Tso (IBM) to fix bug #132708

*grin* Maybe obsolete by now?  haven't looked closely.

> Patch12:        e2fsprogs-mkinstalldirs.patch
> 
> Why?
> 

Probably same as why we have something similar; for one reason or other
need to rerun autoconf, and e2fsprogs isn't compatible with latest
autoconf.  (This is a patch I inherited, and haven't yet investigated
all the details)

> Patch22:        e2fsprogs-1.40.4-uuidd_pid_path.patch
> 
> The problem with this patch is that /var/run is cleared via rm -rf, so
> it is highly problamtic to put the scratch directory for uuidd in
> /var/run.

Hm, I had similar issues with uuidd too - common theme here?

> 
> Patch32:        libcom_err-no-e2fsck.static.patch
> 
> This patch does two completely unrelated things.  One is to disable
> the libcom_err regression test suite (probably because some of the
> other changes made) and the other is to disable building the
> e2fsck.static file.  Why these two are bundled into a single patch I'm
> not sure.

And I have a patch to do the latter as well.  Interesting how we've
arrived at similar needed changes, independently.  :)

and Patch99:        e2fsprogs-no_cmd_hiding.patch

honestly I like that; I should whip up a nice patch to emulate kbuild,
with V=1 or something, unless there is some other easy way to show full
build commands already?

-Eric

Re: Integrating patches in SLES10 e2fsprogs

[Theodore Tso]

On Sun, Jan 27, 2008 at 09:06:59AM -0600, Eric Sandeen wrote:
> 
> * Tue Nov 15 2005 jblunck@suse.de
> - added close.patch provided by Ted Tso (IBM) to fix bug #132708

Hmm, yes, I had forgotten about the changelog in the spec file.  I'm
used to the kernel patch convention of *detailed* comments in the
patch file itself.


> *grin* Maybe obsolete by now?  haven't looked closely.

I can't tell.  Looks like my SuSE bugzilla account was flushed,
probably when it was merged with some central Novell authentication
system.  I created a new account, but "You are not authorized to view
bug #132708".  I tried searching the IBM LTC bugzilla, and turned up
nothing there, as well as the SCM logs for e2fsprogs --- and it's my
general practice that when I fix bug reported through either Red Hat
or Novell, that I include the Red Hat/SuSE/LTC bugzilla numbers in the
changelog.

The patch makes no sense, in any case, since super_shadow gets set a
few line laters, so the patch as it stands is either a no-op (assuming
a smart enough GCC), or wastes a few bytes of text segment space.

> > Patch12:        e2fsprogs-mkinstalldirs.patch
> > 
> > Why?
> > 
> 
> Probably same as why we have something similar; for one reason or other
> need to rerun autoconf, and e2fsprogs isn't compatible with latest
> autoconf.  (This is a patch I inherited, and haven't yet investigated
> all the details)

Define "latest autoconf"?  I'm using autoconf 2.61, which is
reasonably up-to-date.  Can you send me the output of config.status,
so I can see what it's setting @MKINSTALLDIRS@ to?

> > Patch22:        e2fsprogs-1.40.4-uuidd_pid_path.patch
> > 
> > The problem with this patch is that /var/run is cleared via rm -rf, so
> > it is highly problamtic to put the scratch directory for uuidd in
> > /var/run.
> 
> Hm, I had similar issues with uuidd too - common theme here?

What issues?  I thought you agreed that using /var/lib was the best
approach for now.  The Novell patch moves it back to /var/run, which
will cause significant problems if uuidd is run setuid to a non-root
user.

> > Patch32:        libcom_err-no-e2fsck.static.patch
> > 
> > This patch does two completely unrelated things.  One is to disable
> > the libcom_err regression test suite (probably because some of the
> > other changes made) and the other is to disable building the
> > e2fsck.static file.  Why these two are bundled into a single patch I'm
> > not sure.
> 
> And I have a patch to do the latter as well.  Interesting how we've
> arrived at similar needed changes, independently.  :)

Yeah, I'll check in a change so that e2fsck is built dynamically by
default, and e2fsck.static is only built if it is explicitly
requiested via --enable-static-e2fsck.

> and Patch99:        e2fsprogs-no_cmd_hiding.patch
> 
> honestly I like that; I should whip up a nice patch to emulate kbuild,
> with V=1 or something, unless there is some other easy way to show full
> build commands already?

Yes, a way to do kbuild with V=1 would be nice.  The main thing that
makes this difficult is that I've tried to make e2fsprogs not rely on
any GNU make'isms, since it builds on a number of non-Linux platforms,
including *BSD, MacOS, Solaris, etc. 

Personally, it's not a big deal; whenever I need to see what is going
on, I just edit the makefile and quickly remove the '@' signs.  It's
really not that hard, and it's rare that I need to look at things.  Of
course, that could be because I'm more familiar with e2fsprog's build
system.  :-)

					- Ted

Re: Integrating patches in SLES10 e2fsprogs

[Eric Sandeen]

Theodore Tso wrote:
> On Sun, Jan 27, 2008 at 09:06:59AM -0600, Eric Sandeen wrote:

>>> Patch12:        e2fsprogs-mkinstalldirs.patch
>>>
>>> Why?
>>>
>> Probably same as why we have something similar; for one reason or other
>> need to rerun autoconf, and e2fsprogs isn't compatible with latest
>> autoconf.  (This is a patch I inherited, and haven't yet investigated
>> all the details)
> 
> Define "latest autoconf"?  I'm using autoconf 2.61, which is
> reasonably up-to-date.  Can you send me the output of config.status,
> so I can see what it's setting @MKINSTALLDIRS@ to?

[esandeen@neon devel]$ rpm -q autoconf automake
autoconf-2.61-9.fc8
automake-1.10-6

in our spec file we do:

%build
aclocal
autoconf
%configure --enable-elf-shlibs --enable-nls --disable-e2initrd-helper
--enable-blkid-devmapper --enable-blkid-selinux --enable-dynamic-e2fsck
make %{?_smp_mflags}

and if I fire off the rpm w/o our changes, it terminates with:

making install in e2fsck
make[1]: Entering directory `/usr/src/redhat/BUILD/e2fsprogs-1.40.4/e2fsck'
	MKINSTALLDIRS /sbin /usr/share/man/man8
make[1]: MKINSTALLDIRS@: Command not found
make[1]: *** [installdirs] Error 127

the rpm-generated config.status has no reference to MKINSTALLDIRS.

The e2fsck looks like:

MKINSTALLDIRS = @MKINSTALLDIRS@
...
installdirs:
        @echo " MKINSTALLDIRS $(root_sbindir) $(man8dir)"
        @$(MKINSTALLDIRS) $(DESTDIR)$(root_sbindir) \
                $(DESTDIR)$(man8dir) $(DESTDIR)$(man5dir)

sooo... dunno.  I've never been proficient at autotools.  There's a RH
bug, btw, https://bugzilla.redhat.com/show_bug.cgi?id=220715, which was
"resolved" with the patch we're carrying.

But if you have to chose, I'd rather have extents support in e2fsprogs
than have this problem fixed. ;)

>>> Patch22:        e2fsprogs-1.40.4-uuidd_pid_path.patch
>>>
>>> The problem with this patch is that /var/run is cleared via rm -rf, so
>>> it is highly problamtic to put the scratch directory for uuidd in
>>> /var/run.
>> Hm, I had similar issues with uuidd too - common theme here?
> 
> What issues?  I thought you agreed that using /var/lib was the best
> approach for now.  The Novell patch moves it back to /var/run, which
> will cause significant problems if uuidd is run setuid to a non-root
> user.

Yeah, I ended up leaving it as is - I just mean I went around a bit on
this one too.

>>> Patch32:        libcom_err-no-e2fsck.static.patch
>>>
>>> This patch does two completely unrelated things.  One is to disable
>>> the libcom_err regression test suite (probably because some of the
>>> other changes made) and the other is to disable building the
>>> e2fsck.static file.  Why these two are bundled into a single patch I'm
>>> not sure.
>> And I have a patch to do the latter as well.  Interesting how we've
>> arrived at similar needed changes, independently.  :)
> 
> Yeah, I'll check in a change so that e2fsck is built dynamically by
> default, and e2fsck.static is only built if it is explicitly
> requiested via --enable-static-e2fsck.

That'd be great, thanks, tho again I have a small patch to disable it
too (which I sent, tho I expect you'll do something different)

>> and Patch99:        e2fsprogs-no_cmd_hiding.patch
>>
>> honestly I like that; I should whip up a nice patch to emulate kbuild,
>> with V=1 or something, unless there is some other easy way to show full
>> build commands already?
> 
> Yes, a way to do kbuild with V=1 would be nice.  The main thing that
> makes this difficult is that I've tried to make e2fsprogs not rely on
> any GNU make'isms, since it builds on a number of non-Linux platforms,
> including *BSD, MacOS, Solaris, etc. 
> 
> Personally, it's not a big deal; whenever I need to see what is going
> on, I just edit the makefile and quickly remove the '@' signs.  It's
> really not that hard, and it's rare that I need to look at things.  Of
> course, that could be because I'm more familiar with e2fsprog's build
> system.  :-)

Yes, but when you're sending it off to some multi-arch-build mothership,
and something fails, it's nice to be able to just look at the generated
logs and see exactly what was going on.... ah well.  Again, not the
biggest issue.

-Eric

Re: Integrating patches in SLES10 e2fsprogs

[Theodore Tso]

On Sun, Jan 27, 2008 at 10:40:25PM -0600, Eric Sandeen wrote:
> 
> %build
> aclocal
> autoconf
> %configure --enable-elf-shlibs --enable-nls --disable-e2initrd-helper
> --enable-blkid-devmapper --enable-blkid-selinux --enable-dynamic-e2fsck
> make %{?_smp_mflags}
> 

It should be fine if you eliminate the "aclocal" call.  And I've never
understood why people like to regenerate configure scripts from
configure.in in the first place....

In any case, I've just released e2fsprogs 1.40.6.  It contains some of
obviously correct patches from the Novell and Red Hat packages.  More
importantly, it includes the "mke2fs -E test_fs" feature which will be
needed once we push test_fs ext4 patches to mainline, which I plan to
push to Linus in the next day or two.

						- Ted

Re: Integrating patches in SLES10 e2fsprogs

[Eric Sandeen]

Theodore Tso wrote:
> On Sun, Jan 27, 2008 at 10:40:25PM -0600, Eric Sandeen wrote:
>> %build
>> aclocal
>> autoconf
>> %configure --enable-elf-shlibs --enable-nls --disable-e2initrd-helper
>> --enable-blkid-devmapper --enable-blkid-selinux --enable-dynamic-e2fsck
>> make %{?_smp_mflags}
>>
> 
> It should be fine if you eliminate the "aclocal" call.  And I've never
> understood why people like to regenerate configure scripts from
> configure.in in the first place....

Well, we patched configure.in too, for other reasons...  that at least
explains the autoconf call.  And I'll plead inheritance on some of this ;)

> In any case, I've just released e2fsprogs 1.40.6.  It contains some of
> obviously correct patches from the Novell and Red Hat packages.  More
> importantly, it includes the "mke2fs -E test_fs" feature which will be
> needed once we push test_fs ext4 patches to mainline, which I plan to
> push to Linus in the next day or two.

Good deal.  I'll push that to Rawhide tomorrow.

Thanks,
-Eric

Re: Integrating patches in SLES10 e2fsprogs

[Thierry Vignaud]

Theodore Tso <tytso@mit.edu> writes:

> In any case, I've just released e2fsprogs 1.40.6.

which prompts me to note that it's neither on
ftp://ftp.kernel.org/pub/linux/utils/util-linux-ng/v2.13 nor on
http://e2fsprogs.sourceforge.net/

What is supposed to be the current primary source?

> It contains some of obviously correct patches from the Novell and
> Red Hat packages.  More importantly, it includes the "mke2fs -E
> test_fs" feature which will be needed once we push test_fs ext4
> patches to mainline, which I plan to push to Linus in the next day
> or two.

Re: Integrating patches in SLES10 e2fsprogs

[Thierry Vignaud]

Thierry Vignaud <tvignaud@mandriva.com> writes:

> > In any case, I've just released e2fsprogs 1.40.6.
> 
> which prompts me to note that it's neither on
> ftp://ftp.kernel.org/pub/linux/utils/util-linux-ng/v2.13 nor on
> http://e2fsprogs.sourceforge.net/
> 
> What is supposed to be the current primary source?

Worse, http://git.kernel.org/?p=fs/ext2/e2fsprogs.git;a=summary hasn't
any tag for neither 1.40.5 nor 1.40.6 in primary branch (only maint
shows 1.40.5)

You could be more user-friendly to use, distro packagers &
maintainers :-(

Re: Integrating patches in SLES10 e2fsprogs

[Theodore Tso]

On Mon, Jan 28, 2008 at 05:06:47PM +0100, Thierry Vignaud wrote:
> Worse, http://git.kernel.org/?p=fs/ext2/e2fsprogs.git;a=summary hasn't
> any tag for neither 1.40.5 nor 1.40.6 in primary branch (only maint
> shows 1.40.5)

Sorry, I forget to use the --tags option when I pushed the patches to
the git tree.  This was fixed about an hour ago.

    	       	    	      	       	    - Ted

Re: Integrating patches in SLES10 e2fsprogs

[Theodore Tso]

On Mon, Jan 28, 2008 at 05:01:36PM +0100, Thierry Vignaud wrote:
> Theodore Tso <tytso@mit.edu> writes:
> 
> > In any case, I've just released e2fsprogs 1.40.6.

Oops, typo, that should have read 1.40.5.

> 
> which prompts me to note that it's neither on
> ftp://ftp.kernel.org/pub/linux/utils/util-linux-ng/v2.13 nor on
> http://e2fsprogs.sourceforge.net/

I don't know why people would think it's at
/pub/linux/utils/util-linux-ng, but it is at e2fsprogs.sf.net, and
I've just uploaded 1.40.5 to /pub/linux/kernel/people/tytso/e2fsprogs
on master.kernel.org, so it should be at ftp.kernel.org very shortly.
(Sorry, didn't get to that last night.)

> What is supposed to be the current primary source?

Well, for the absolutely *latest*, the git trees are at:

	git://git.kernel.org/pub/scm/fs/ext2/e2fsprogs.git
	http://git.kernel.org/?p=fs/ext2/e2fsprogs.git;a=summary
or
	git://repo.or.cz/e2fsprogs.git
	http://repo.or.cz/w/e2fsprogs.git

For the release tarballs, they can be downloaded at e2fsprogs's
SourceForge page, or at

	ftp.kernel.org:/pub/linux/kernel/people/tytso/e2fsprogs

Regards,

					- Ted

Re: Integrating patches in SLES10 e2fsprogs

[Matthias Koenig]

Theodore Tso <tytso@mit.edu> writes:

> Wow.  You have a lot of patches in the SLES 10 e2fsprogs.  I'm not
> sure why of them are there, though.  For example:
>
> Patch0:         elf.diff
>
> I'm not sure what this one is for.
>
> Patch1:		e2fsprogs-1.35-libdir.diff
>
> This one does two different things.  One is include AC_HEADER_TIME in
> configure.in, and the other is to use $lib instead of "lib" when
> defining root_libdir.  This seems to force root_libdir to /, which
> makes no sense to me.

We want to have the shared libs in /lib{,64}, but the devel so links
have to remain in /usr/lib{,64}. 
But looking closer at this, it seems that these patches are not needed,
since the result can be obtained by defining ELF_INSTALL_DIR.
I will drop these.

> Patch4:         e2fsprogs-blkid.diff
>
> This patch causes fsck to check the BLKID_FILE environment variable
> and passes it to the blkid library.  But the blkid library *already*
> checks the BLKID_FILE environtment variable already.  So I'm not sure
> why this is necessary at all.

Ok, the patch is obsolete indeed.

> Patch6:         e2fsprogs-mdraid.patch
>
> This apparently adds a new environment variable,
> BLKID_SKIP_CHECK_MDRAID, which forces blkid to not detect mdraid
> devices.  I'm not sure why.

Workaround for people having stale RAID signature on their disk:
https://bugzilla.novell.com/show_bug.cgi?id=100530

> Patch10:        close.patch
>
> I don't understand what this patch is trying to do.

This patch is obsolete, as the issue is fixed by
git commit 0d961040fe9ad927254b5a0e1a4de7bedadd8579

The original patch posted in Novell bugzilla #132708
contained this additional hunk, which is likely obsolete:
@@ -217,6 +217,7 @@
 	
 	EXT2_CHECK_MAGIC(fs, EXT2_ET_MAGIC_EXT2FS_FILSYS);
 
+	super_shadow = fs->super;
 	fs_state = fs->super->s_state;
 
 	fs->super->s_wtime = time(NULL);

> Patch12:        e2fsprogs-mkinstalldirs.patch
>
> Why?

Is needed since we recreate the auto* files.
But I agree that this patch should better set
MKINSTALLDIRS = @MKDIR_P@
not to literal "mkdir -p". The @MKINSTALLDIRS@ seems to be
obsolete in newer gettext (which seems to pull this in).

> Patch22:        e2fsprogs-1.40.4-uuidd_pid_path.patch
>
> The problem with this patch is that /var/run is cleared via rm -rf, so
> it is highly problamtic to put the scratch directory for uuidd in
> /var/run.

Are you really sure? My interpretation of FHS is, that files under
/var/run/ have to be cleared or truncated, but the subdirectories do not
get deleted.

> Patch34:        libcom_err-compile_et_permissions.patch
>
> Why?

This is just a workaround and is not intended to stay there forever,
it is also not intended for upstream inclusion.
I have been asked to add this to avoid build problems of some other
package (I think it was kerberos). I will have to check if this is
still needed.

Thanks,
Matthias

Re: Integrating patches in SLES10 e2fsprogs

[Theodore Tso]

On Mon, Jan 28, 2008 at 04:26:53PM +0100, Matthias Koenig wrote:
> > Patch6:         e2fsprogs-mdraid.patch
> >
> > This apparently adds a new environment variable,
> > BLKID_SKIP_CHECK_MDRAID, which forces blkid to not detect mdraid
> > devices.  I'm not sure why.
> 
> Workaround for people having stale RAID signature on their disk:
> https://bugzilla.novell.com/show_bug.cgi?id=100530

Hmm... there's got to be a better way around this.

I'm not authorized to look at that BZ entry.  Is there any way you can
make my "tytso" account have access to any e2fsprogs related BZ
entries?  Thanks!

> > Patch12:        e2fsprogs-mkinstalldirs.patch
> >
> > Why?
> 
> Is needed since we recreate the auto* files.
> But I agree that this patch should better set
> MKINSTALLDIRS = @MKDIR_P@
> not to literal "mkdir -p". The @MKINSTALLDIRS@ seems to be
> obsolete in newer gettext (which seems to pull this in).

Are you running aclocal?  That may be causing the issues since I think
that may be causing it to ignore the autoconf macros I've established
in the top-level aclocal.m4 file.

> > Patch22:        e2fsprogs-1.40.4-uuidd_pid_path.patch
> >
> > The problem with this patch is that /var/run is cleared via rm -rf, so
> > it is highly problamtic to put the scratch directory for uuidd in
> > /var/run.
> 
> Are you really sure? My interpretation of FHS is, that files under
> /var/run/ have to be cleared or truncated, but the subdirectories do not
> get deleted.

The problem is that the FHS does not guarantee that the subdirectories
stick around, and a number of distro's are using mounting tmpfs for
/var/run.  Makes sense, it can significantly speed up operations ---
but upon reboot, everything in /var/run is *gone*.


Please let me know when you have a newer OpenSUSE factory RPM ready
for use, and I'll take a look at the remaining patches and see which
ones are ready for merging upstream.  Markus managed to convince me it
was worthwhile to install the latest OpenSUSE on a laptop so I could
see how things are shaping up, and I have to say I'm pretty impressed.
I haven't updated it to try tracking the latest OpenSUSE factory
RPM's, and I still haven't figured out how to easily build my own
custom kernel RPM's on OpenSUSE, but it's quite nice.

On my "one of these days" list is to get another cheap/used laptop so
I can try out the latest Fedora Core Rawhide without having to fire up
a huge (noisy) x86_64 box....

						- Ted

Re: Integrating patches in SLES10 e2fsprogs

[Eric Sandeen]

Theodore Tso wrote:
> On Mon, Jan 28, 2008 at 04:26:53PM +0100, Matthias Koenig wrote:
>>> Patch6:         e2fsprogs-mdraid.patch
>>>
>>> This apparently adds a new environment variable,
>>> BLKID_SKIP_CHECK_MDRAID, which forces blkid to not detect mdraid
>>> devices.  I'm not sure why.
>> Workaround for people having stale RAID signature on their disk:
>> https://bugzilla.novell.com/show_bug.cgi?id=100530
> 
> Hmm... there's got to be a better way around this.

Won't help existing block devices, but it'd be nice to have a common
library which could be called @ mkfs time to wipe out all known
signatures...

mkfs.xfs tries to do this, but it'd be silly to duplicate in every mkfs.

> On my "one of these days" list is to get another cheap/used laptop so
> I can try out the latest Fedora Core Rawhide without having to fire up
> a huge (noisy) x86_64 box....

Just partition... ;)

-Eric

Re: Integrating patches in SLES10 e2fsprogs

[Andreas Dilger]

On Jan 28, 2008  10:54 -0600, Eric Sandeen wrote:
> Theodore Tso wrote:
> > On Mon, Jan 28, 2008 at 04:26:53PM +0100, Matthias Koenig wrote:
> >>> Patch6:         e2fsprogs-mdraid.patch
> >>>
> >>> This apparently adds a new environment variable,
> >>> BLKID_SKIP_CHECK_MDRAID, which forces blkid to not detect mdraid
> >>> devices.  I'm not sure why.
> >> Workaround for people having stale RAID signature on their disk:
> >> https://bugzilla.novell.com/show_bug.cgi?id=100530
> > 
> > Hmm... there's got to be a better way around this.
> 
> Won't help existing block devices, but it'd be nice to have a common
> library which could be called @ mkfs time to wipe out all known
> signatures...
> 
> mkfs.xfs tries to do this, but it'd be silly to duplicate in every mkfs.

Well, blkid already has a way to _detect_ a lot of filesystem signatures,
so it might be relatively easy to exploit the type_array[] entries to have
it zap out all of these blocks.  That said, the majority of them are in
the first 68kB of the filesystem (mdraid excluded) so it shouldn't be too
hard to zero them out.  Let's hope nobody ever uses "0x00000000" as magic.

mke2fs already tries to do this, though I notice:
- the zap_sector() call will skip the entire write if there is a BSD bootblock,
  instead of skipping only the first sector(s) and overwriting the rest...
  Since I don't know much about BSD bootblocks, I don't know what the right
  behaviour is, but I can guess we still want to zero out 4-68kB (or whatever).
- it only overwrites up to sector 8 (4kB) and not further into the disk to
  catch e.g. reiserfs superblocks.  Usually it will overwrite this anyways
  (GDT, bitmaps, inode table), but in some rare cases it might not.

Cheers, Andreas
--
Andreas Dilger
Sr. Staff Engineer, Lustre Group
Sun Microsystems of Canada, Inc.

Re: Integrating patches in SLES10 e2fsprogs

[Matthias Koenig]

Theodore Tso <tytso@mit.edu> writes:

> On Mon, Jan 28, 2008 at 04:26:53PM +0100, Matthias Koenig wrote:
>> > Patch6:         e2fsprogs-mdraid.patch
>> >
>> > This apparently adds a new environment variable,
>> > BLKID_SKIP_CHECK_MDRAID, which forces blkid to not detect mdraid
>> > devices.  I'm not sure why.
>> 
>> Workaround for people having stale RAID signature on their disk:
>> https://bugzilla.novell.com/show_bug.cgi?id=100530
>
> Hmm... there's got to be a better way around this.
>
> I'm not authorized to look at that BZ entry.  Is there any way you can
> make my "tytso" account have access to any e2fsprogs related BZ
> entries?  Thanks!

Still waiting for the person with privileges to do this.
In the meanwhile I will add you to the CC list of this bug, then you
should also be able to access the bug.

>
>> > Patch12:        e2fsprogs-mkinstalldirs.patch
>> >
>> > Why?
>> 
>> Is needed since we recreate the auto* files.
>> But I agree that this patch should better set
>> MKINSTALLDIRS = @MKDIR_P@
>> not to literal "mkdir -p". The @MKINSTALLDIRS@ seems to be
>> obsolete in newer gettext (which seems to pull this in).
>
> Are you running aclocal?  That may be causing the issues since I think
> that may be causing it to ignore the autoconf macros I've established
> in the top-level aclocal.m4 file.

Yes, someone here fixed the german po file (I already sent it to
translation project). So we have to rerun the
gettext related part, which results in the requirement to rerun aclocal.

>
>> > Patch22:        e2fsprogs-1.40.4-uuidd_pid_path.patch
>> >
>> > The problem with this patch is that /var/run is cleared via rm -rf, so
>> > it is highly problamtic to put the scratch directory for uuidd in
>> > /var/run.
>> 
>> Are you really sure? My interpretation of FHS is, that files under
>> /var/run/ have to be cleared or truncated, but the subdirectories do not
>> get deleted.
>
> The problem is that the FHS does not guarantee that the subdirectories
> stick around, and a number of distro's are using mounting tmpfs for
> /var/run.  Makes sense, it can significantly speed up operations ---
> but upon reboot, everything in /var/run is *gone*.

I see, then there is a problem of course. This is not the way it is
done in Suse, so I did not see this problem.
But, shouldn't the daemon then have an rc init script which checks for
the needed subdirectory in /var/run and creates this if necessary?  If I
look into /var/run I see a lot of daemons running with their own UID/GID
and having their own directory. In this case there must be a mechanism
to guarantee the existence of this directory, which would obviously be
done in the init script (not sure if any init scripts currently do this
in Suse). The only problem with uuidd is that it is designed to run upon
demand. But on the other side it is also not strictly necessary to run
uuidd to create UUIDs, only if you have special requirements on the
UUIDs. So I don't see a problem if uuidd could not be started for the
creation of UUIDs *generally* and if people wanting really unique
time-based UUIDs are required to run the init script first upon boot, 
which guarantees the existence of the /var/run/uuidd directory.
Any problems with this approach?

> Please let me know when you have a newer OpenSUSE factory RPM ready
> for use, and I'll take a look at the remaining patches and see which
> ones are ready for merging upstream. 

Yes, will do so. I am currently still looking through all the patches
where they come from and why they are needed. Will probably submit
a new package today.

> and I still haven't figured out how to easily build my own
> custom kernel RPM's on OpenSUSE, but it's quite nice.

unfortunately we don't have make-kpkg.

Matthias

Re: Integrating patches in SLES10 e2fsprogs

[Theodore Tso]

On Tue, Jan 29, 2008 at 02:52:10PM +0100, Matthias Koenig wrote:
> > Are you running aclocal?  That may be causing the issues since I think
> > that may be causing it to ignore the autoconf macros I've established
> > in the top-level aclocal.m4 file.
> 
> Yes, someone here fixed the german po file (I already sent it to
> translation project). So we have to rerun the
> gettext related part, which results in the requirement to rerun aclocal.

Hmm?  You should be able to rebuild the .gmo file without needing to
re-run aclocal.  I do it all the time.  :-)

> > The problem is that the FHS does not guarantee that the subdirectories
> > stick around, and a number of distro's are using mounting tmpfs for
> > /var/run.  Makes sense, it can significantly speed up operations ---
> > but upon reboot, everything in /var/run is *gone*.
> 
> I see, then there is a problem of course. This is not the way it is
> done in Suse, so I did not see this problem.
> But, shouldn't the daemon then have an rc init script which checks for
> the needed subdirectory in /var/run and creates this if necessary?  If I
> look into /var/run I see a lot of daemons running with their own UID/GID
> and having their own directory. In this case there must be a mechanism
> to guarantee the existence of this directory, which would obviously be
> done in the init script (not sure if any init scripts currently do this
> in Suse).

Well, it either needs to be done in the init script or the daemons run
with root privs, create the directory, and then drop root privs.  So
yes, there are alternatives.  (a) Yet Another init script, (b) setuid
root, with a hard-coded user name that has to be looked up via
getpwent() and configured into /etc/passwd or the LDAP server, (c)
just put the darned directory in /var/lib.

It's all about tradeoffs, and for me, (c) was the easist, especially
since /var/lib/libuuid/clock.txt needs to be persistent across boots
anyway, and so needs to be in /var/lib instead of /var/run.  So yes, I
could have created some machinery to make sure /var/run/uuidd is
always present, but why not just reuse the already existing
/var/lib/libuuid directory, which has the correct ownership and which
has to be in /var/lib anyway?

> > and I still haven't figured out how to easily build my own
> > custom kernel RPM's on OpenSUSE, but it's quite nice.
> 
> unfortunately we don't have make-kpkg.

Yeah, and the RPM macros for generating kernel are incredibly twisted.
At least they were for RHEL4, when I was figuring out how to generate
a real-time kernel rpm.  Serious temptations to gouge out my eyeballs
with a spoon by the time I was done....

       	     					- Ted