Bug 124442

Additional Info:

After I have created the missed files with no content, no error
messages was displayed.

Best Regards:

Jochen Schmitt

these files should not be in /etc
use the attached patch (which also corrects a type) and add to the
specfile:
%install
touch $RPM_BUILD_ROOT/var/lib/devlabel/ignore_list
touch $RPM_BUILD_ROOT/var/lib/devlabel/proc_partitions

%files
%dir /var/lib/devlabel
%verify(not md5 size mtime) %config(noreplace)
/var/lib/devlabel/ignore_list
%verify(not md5 size mtime) %config(noreplace)
/var/lib/devlabel/proc_partitions

Created attachment 100657 [details]
typo corrected and path

*** Bug 124696 has been marked as a duplicate of this bug. ***

Added in -3, thanks!

Hmm, I had originally myself put ignore_list and proc_partition 
in /var/lib/devlabel.  Then we ran into problems in the Dell factory 
with this because in some scenarios /var wasn't on the / partition 
and there were many error messages as a result.  Have you considered 
this scenario?

Continuing...

The error messages were a result of devlabel getting started 
before /var was mounted.

I took a look at the "typo" patch:

-    if [ `grep -c "$1$" $IGNORE_LIST` -gt 0 ]; then
+    if [ `grep -c "$1" $IGNORE_LIST` -gt 0 ]; then

Was the extra '$' causing some sort of problem on your box that 
prompted you to make this change?  The reason I put it in there is in 
the event that /dev/sda has been added to your ignore_list, this 
keeps /dev/sdaa or /dev/sdab from also being ignored.  As you may 
know, the '$' tells it to match at the end of the string.

Also, I'd like clarification on the implications of going back and 
using /var/lib.

Reopening this bugzilla.  

devlabel uses tr, which is installed in /usr/bin. If /usr is on a
seperate partition this will result in errors since devlabel is
executed before /usr is mounted.

Created attachment 100731 [details]
A patch to clean  'tr' out of  'devlabel'

Attached patch removes all instances of /usr/bin/tr from devlabel script
by replacing them with equivalent constructs which do not bother /usr/bin.
This patch also makes checks for root to run only if 'id' is available.
It is for devlabel-0.48.01-3.

Still devlabel has multiple calls to /usr/bin/logger and I do not know
now how to kill that.  The same problem - /usr/bin may be not mounted
yet when devlabel is invoked in a startup sequence.  Would replacing
'logger' with a function which just eats arguments if logger is not
available, and runs the later otherwise, be acceptable?

BTW, the whole script starts with '#!/bin/bash' so there is really no good
reason in using `...` constructs instead of much more readable, and nestable,
$(...); but I left things in the same style all over.

if you want 'grep -c "$1$" '  then you better quote the second '$'

I missed the one from comment #11 although I do not see right
away a context where this would be harmful.  Seems to be safe
enough as a trailing "$" will be not expanded.  There could be
some other dragons be hidden in the script.  The main purpose
of a patch from comment #10 was to prevent flooding of my startup
sequence with a zillion of error messages. :-)  Obviously I still
see some leftover whining about 'logger'.

If I would be writing something like that from scratch a quick glance
seem to suggest that I would likely do that in a single, per case,
invocation of awk with some rather thin shell "wrapper".  Most likely
would end up easier on eyes then the current variant.

/sbin/devlabel: line 167: [: /dev/loop0: integer expression expected
/sbin/devlabel: line 167: [: /dev/md0: integer expression expected

Are the linx 167 errors occuring with the tr patch or from the 
original devlabel?

Created attachment 100757 [details]
Devlabel 0.48.02

48.02 includes:

* removal of usage of tr from patch (note that one part of the patch 
just seemed to change my usage of grep and sed to fancy awk, but 
since seeing comment #13, i did not apply this as it did not have to 
do with tr removal)

* [ -n "`type -p logger`" ] before any calls to logger so that it 
shouldn't complain if /usr isn't mounted

* log dir is still /etc/sysconfig/devlabel.d/ and 
not /var/lib/devlabel since /var may not be mounted

* grep -c "$1" references left unchanged from my previous release 
(does this need to be changed?)

* Though I did include that part of tr removal patch, will someone 
please confirm that

line_arg=`echo "$1" | tr -d '\r'`

&

line_arg="${1//
/}"

are equivalent?
 

Comments to #16
> ... change my usage of grep and sed to fancy awk

"Fancy awk"??? This is as nearly elementary awk as it can be.
Indeed that part was not 'tr' removal but if you are using
grep to pipe something into awk then this is nearly as good
as "dead cat".  I could not restrain myself. :-)

A replacement of this construct:

line_arg=`echo "$1" | tr -d '\r'`

in my patch had a literal character '^M'.  That means that
it looked

line_arg="${1//^M/}"

where instead of ^M was a literal '\012'. If this got changed
in two lines then this is likely a bugzilla web interface doing.
I do not see anything else which would work here.  You can do  

    echo "$1" | sed 's/^M//'

but you need a literal character too.  Unfortunately sed does
not have an escape sequence for it so we may simply ask bash
to do the work.

Well, no.  I am lying.  You can do "fancy awk" here:

   line_arg=`echo "$1" | awk '{gsub(/\r/, ""); print}'`

If something mangled the substitution you qoute into two lines then
results are _not_ equivalent.  You have '\015' there instead
of intended '\012'.  So that awk may be more robust in face of
possible mangling by programs which "know better".

BTW - if you are using awk only to pick up the third field then
something like that instead will do:

   grep ... | ( read a b c d ; echo $c ) | sed ....

Look, Ma!  No awk.

lol, I only call it fancy awk because `awk '{print $#}'` is the only 
thing I know how to do with it. I'm not opposed to awk in any 
respect, just ignorance combined with Comment #13 temper my 
acceptance of that code.

I'll be serving up 48.03 with the line_arg=`echo "$1" | awk '{gsub(/
\r/, ""); print}'` shortly.

Created attachment 100763 [details]
Devlabel 0.48.03

line_arg fixed

err, please correct the mime types of your attachements

Referring to comment #17 I realized that there is a way to remove
all carriage returns using only bash and without putting in a literal
'\015'.  Like that:

    eval "$(echo -ne 'line_arg=${line_arg//'\\r'/}')"

This will not mess up other white-space in $line_arg.

If you want to have that instead of a call to awk that is a different
question.

Like I said, I don't mind awk, I'm just not so fluent with it.  I'll 
keep what I have in 48.03.  

Bill, do you plan to move to this?  I don't think we ever settled the 
debate over /var/lib/devlabel vs. /etc/sysconfig/devlabel.d as per 
what is available during boot.  48.03 uses /etc.

It is not a question of minding or not minding.  Simply it is a good
idea for programs used in a startup sequence to keep a number of
outside tools to a reasonable minimum and then, with a right set,
we would not have this bug listing.

It seems that all occurences of awk in devlabel, as it is used right
now, can be replaced by other simple means; so this looks like
a good opportunity.  This makes sense only if you will not leave
_any_ awk invocations. If you want to do that is another story
(if you prefer I can patch that easily enough if you plan to
apply results).

Another option would be to use consistently awk as it is supposed
to be used and dump other things instead.  For example, currently
any invocation of 'head' is really superfluous.

Agreed.  If you send me an anti-awk patch, I'll apply it.

Well....

In the grand scheme of things, devlabel configs really should be
migrated to be using udev. So, long term, /etc vs /var doesn't matter.

Short term, I think we can probably go with /etc.

Actually...

In what cases would devlabel be called before /var is mounted?

0.48.03-1 built (using /var) in the meantime.

It was seen in our factory.  Generally, this is possible for any 
partition that isn't root, as devlabel should get called in 
rc.sysinit before others are mounted such that it is possible to 
mount them by devlabel symlink.

Agreed on the udev future statement.  At first it was nice to have 
your name attached to a piece of software.  Later on, it becomes a 
bit of a headache.  :) ... more time to spend on DKMS headaches 
instead.

Created attachment 100957 [details]
simplify devlabel a bit

This patch to devlabel
 - replaces all invocations of awk and head with various other constructs
 - leaves /usr/bin/diff only in remove_entry() which checks that
   diff is already available before trying to execute
 - it fixes asignment to start_sector (there are no guarantees
   that in a partx output there will be a blank after '-') and
   considerably simplifies the whole thing in the process
 - removes instances of "dead cat"
 - does some other cleanups and checks

It applies to the script from devlabel-0.48.03

There are still problems.  After 'devlabel status' it prints for me:
Cannot open master raw device '/dev/rawctl' (No such device or address)

Current rawhide still has this problem with /var on a separate
partition: loads of

grep: /var/lib/devlabel/ignore_list: No such file or directory
/sbin/devlabel: line 144: [: -gt: unary operator expected
/sbin/devlabel: line 192: /var/lib/devlabel/ignore_list: No such file
or directory

spouted during boot.  In response to

"In what cases would devlabel be called before /var is mounted?"

rc.sysinit does a

if [ -x /sbin/devlabel ]; then
        /sbin/devlabel restart
fi
 
before even fscking, never mind mounting, local filesystems.

I realize /etc/sysconfig/devlabel.d might not be the most elegant 
place to put devlabel scripts, but it takes them out of /var and this 
is how it is currently handled in latest at linux.dell.com/devlabel.  
RH just needs to make the switch.

I'm still wondering whether, with udev, devlabel still needs to be
included in later releases.

Also, the latest at linux.dell.com/devlabel is 0.48.01, which is two
revs lower than things posted in this bug. :)

Per #33, Doesn't udev rely on outside applications to determine 
identifiers that it uses?  I'm no udev expert, but the partition_uuid 
program within devlabel has got to be useful for udev given that 
mount still does not make these externally available and has some 
uuid issues with JFS filesystems and does not read reiser uuids.  

Per #34, yes, I forgot.  I made a couple spins for this bugzilla to 
create 48.03 and haven't externally posted them yet.  Still, the 
48.03 I sent you uses /etc and not /var.  You must be patching it.  
Just yank those patches.

udev does support naming of partitions via UUID, although I have not
specifically looked as to the implementation thereof.

Bill, please just remove the devlabel-0.48.01-typo.patch which is 
being applied in Red Hat's RPM.  The patch has two parts.  The bottom 
part is what is changing the /etc to /var (coincidentally, I just was 
assigned an internal bug regarding this yesterday).  If you remove 
this patch, you can fix Stephen's issue.

The top part of the patch fixes a "typo" which isn't exactly right.  
See comment #8 and comment #11 above.  The second $ should be there.  
I'm not sure what Harold means by being sure quote the second $ since 
its surrounded by quotes.

Fixed in -4.