Bug 772639

Summary: xen-network-common.sh scripting typo
Product: Red Hat Enterprise Linux 5 Reporter: guybrush <gthreepwood>
Component: xenAssignee: Xen Maintainance List <xen-maint>
Status: CLOSED ERRATA QA Contact: Virtualization Bugs <virt-bugs>
Severity: high Docs Contact:
Priority: high    
Version: 5.7CC: jpallich, leiwang, lersek, lsoft-lapl-pj, mrezanin, pbonzini, qguan, qwan, vsd-xen-all, wshi, xen-maint
Target Milestone: rcKeywords: Reopened, ZStream
Target Release: ---   
Hardware: Unspecified   
OS: Unspecified   
Whiteboard:
Fixed In Version: xen-3.0.3-136.el5 Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2013-01-08 04:07:45 UTC Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Bug Depends On:    
Bug Blocks: 743405, 708415, 766364, 797191    

Description guybrush 2012-01-09 13:47:46 UTC
line 130:
mtu=$(ip link show ${dev-$bridge} | sed -n 's/.* mtu \([0-9]\+\).*/\1/p')

Obviously, it should be 
${dev:-$bridge}

xen-3.0.3-132.el5_7.2.i386

Comment 1 Paolo Bonzini 2012-01-10 09:32:28 UTC
Hi,

you are correct in that $dev is always set and so the command will not pass $bridge when $2 is missing.

However, the wrong path should never trigger in RHEL5.  Can you attach your /etc/xen/xend-config.sxp file?

Thanks.

Comment 2 Laszlo Ersek 2012-01-10 09:57:18 UTC
BTW I checked the form in comment 0, ie. ${UNSET_VAR-default}, and to my surprise, it works too. (RHEL-5, of course.)

$ bash --version
GNU bash, version 3.2.25(1)-release (x86_64-redhat-linux-gnu)
Copyright (C) 2005 Free Software Foundation, Inc.

$ echo ${UNSET_VAR-default}
default

$echo ${UNSET_VAR-$TERM}
xterm

So, a typo it is, but not a bug.

Comment 3 Laszlo Ersek 2012-01-10 10:13:18 UTC
Checking bash-3.2-32.el5, parameter_brace_expand() in "subst.c":

  5992    /* Find out what character ended the variable name.  Then
  5993       do the appropriate thing. */
  5994    if (c = string[sindex])
  5995      sindex++;
  5996  
  5997    /* If c is followed by one of the valid parameter expansion
  5998       characters, move past it as normal.  If not, assume that
  5999       a substring specification is being given, and do not move
  6000       past it. */
  6001    if (c == ':' && VALID_PARAM_EXPAND_CHAR (string[sindex]))
  6002      {
  6003        check_nullness++;
  6004        if (c = string[sindex])
  6005          sindex++;
  6006      }
  6007    else if (c == ':' && string[sindex] != RBRACE)
  6008      want_substring = 1;
  6009    else if (c == '/' && string[sindex] != RBRACE)
  6010      want_patsub = 1;

  6232    /* Do the right thing based on which character ended the variable
             name. */
  6233    switch (c)
  6234      {

  6269      case '-':

It seems that the colon (:) is not required for this kind of expansion; if it's there, it's skipped, if not, no problem.

Comment 4 guybrush 2012-01-10 10:49:15 UTC
(In reply to comment #1)
> Hi,
> 
> you are correct in that $dev is always set and so the command will not pass
> $bridge when $2 is missing.
> 
> However, the wrong path should never trigger in RHEL5.  Can you attach your
> /etc/xen/xend-config.sxp file?
> 
> Thanks.

Yes, I'm using custom network-bridge script for one of the bridges which calls create_bridge with only one argument (${bridge}). It caused breakage in my case.

(In reply to comment #2)
> BTW I checked the form in comment 0, ie. ${UNSET_VAR-default}, and to my
> surprise, it works too. (RHEL-5, of course.)
> 
> $ bash --version
> GNU bash, version 3.2.25(1)-release (x86_64-redhat-linux-gnu)
> Copyright (C) 2005 Free Software Foundation, Inc.
> 
> $ echo ${UNSET_VAR-default}
> default
> 
> $echo ${UNSET_VAR-$TERM}
> xterm
> 
> So, a typo it is, but not a bug.

Try this:
a=
b="b"
echo ${a-$b}
echo ${a:-$b}

Comment 5 Laszlo Ersek 2012-01-10 11:06:17 UTC
(In reply to comment #4)

> Try this:
> a=
> b="b"
> echo ${a-$b}
> echo ${a:-$b}

The variable "a" is not unset in this case, it's null. The ":-" expansion handles those two cases alike, while "-" seems not to. I have to agree that they are not interchangeable.

The code in question was added for bug 733417.

Comment 6 Miroslav Rezanina 2012-01-10 13:30:22 UTC
(In reply to comment #4)
> (In reply to comment #1)
> > Hi,
> > 
> > you are correct in that $dev is always set and so the command will not pass
> > $bridge when $2 is missing.
> > 
> > However, the wrong path should never trigger in RHEL5.  Can you attach your
> > /etc/xen/xend-config.sxp file?
> > 
> > Thanks.
> 
> Yes, I'm using custom network-bridge script for one of the bridges which calls
> create_bridge with only one argument (${bridge}). It caused breakage in my
> case.
> 

Yeah, that's the reason problem is triggered. So the proper construct has to be used. We will fix this issue. However, it will be part of 5.9 release. Until that manual fix has to be applied. This will be documented only for 5.8 release.

Comment 8 Miroslav Rezanina 2012-02-22 08:21:21 UTC
*** Bug 796067 has been marked as a duplicate of this bug. ***

Comment 9 Miroslav Rezanina 2012-02-22 08:22:58 UTC
As this problem is hit by customer (BZ 796067), requesting z-streaming for 5.8.

Comment 11 Miroslav Rezanina 2012-02-24 06:09:43 UTC
Fix built into xen-3.0.3-136.el5.

Comment 13 Petr Pisar 2012-03-29 13:04:24 UTC
*** Bug 794657 has been marked as a duplicate of this bug. ***

Comment 15 Wei Shi 2012-09-28 10:02:48 UTC
Reproduced on xen-3.0.3-135.el5.x86_64.rpm
#  sed -n '130,1p' /etc/xen/scripts/xen-network-common.sh
    mtu=$(ip link show ${dev-$bridge} | sed -n 's/.* mtu \([0-9]\+\).*/\1/p')

Verified on xen-3.0.3-141.el5
#  sed -n '130,1p' /etc/xen/scripts/xen-network-common.sh                       
    mtu=$(ip link show ${dev:-$bridge} | sed -n 's/.* mtu \([0-9]\+\).*/\1/p')

Comment 17 errata-xmlrpc 2013-01-08 04:07:45 UTC
Since the problem described in this bug report should be
resolved in a recent advisory, it has been closed with a
resolution of ERRATA.

For information on the advisory, and where to find the updated
files, follow the link below.

If the solution does not work for you, open a new bug report.

http://rhn.redhat.com/errata/RHBA-2013-0119.html