Re: [Xen-devel] block-iscsi with Xen 4.5 / 4.6

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Wed, May 04, 2016 at 06:48:13PM +1000, Steven Haigh wrote:
> On 4/05/2016 6:25 PM, George Dunlap wrote:
> > On Wed, May 4, 2016 at 8:34 AM, Roger Pau Monné <roger.pau@xxxxxxxxxx> 
> > wrote:
> >>> 1) -e is set on the script - and maybe elsewhere - so any time something
> >>> returns non-zero, you can't clean up. For example, if you can't get a
> >>> lock, you should make sure all locks are removed from the host in
> >>> question and then detach the iSCSI target.
> >>
> >> You can avoid this by adding something like:
> >>
> >> sg_persist ... || true
> >>
> >> Of course you can replace the "true" command with something else, like a
> >> fatal message or some cleanup code. You can also place the command inside 
> >> of
> >> a conditional if you know it might fail:
> >>
> >> if ! sg_persist ...; then
> >>         fatal ...
> >> fi
> >>
> >> It is important for us to use the '-e' in order to make sure all the 
> >> failure
> >> points are correctly handled, without the '-e' some command might fail and
> >> the script wouldn't realize.
> > 
> > I realize I'm a bit in the minority here, but I've always thought this
> > was rather a strange habit of bash scripts.  In every other language,
> > you check the error codes of things that can fail and you handle them
> > appropriately.  If you're just hacking something together, then "set
> > -e" is probably OK, but wouldn't it make more sense in an
> > infrastructure script like this to actually go through and handle all
> > the errors?   Worst case you could just if ! [command] ; then exit 1 ;
> > fi.
> 
> Then I'm in the minority too ;)
> 
> > Regarding the "maybe elsewhere" -- AFAIK the block-scsi script itself
> > is run directly from libxl, so nothing "above" it should be setting
> > -e; and it only includes block-common, which does not seem to be
> > setting it.
> 
> Right now, even removing -e from line 1 causes something like this to
> exit the script:
> 
> if [ $? != 0 ]; then
> 
> So, if I run the following, the script will always fail:
> 
> key=$(gethostip -x $(uname -n))
> sg_persist -d ${dev} -o -G -S ${key} > /dev/null
> if [ $? != 0 ]; then
>       iscsiadm -m node -T ${iqn} --logout > /dev/null
>       fatal "Could not obtain lock on $iqn"
> fi

I cannot explain this behaviour, removing the -e from the shebang should 
remove the 'exit on error' behaviour. IMHO, I would rather prefer that you 
keep the '-e' at the top and use:

set +e
... code which might fail ...
set -e

> man 3 sg3_utils (http://linux.die.net/man/8/sg3_utils) shows a possible
> list of exit codes - which it would be useful to consult at least *some*
> of them with useful errors - such as:
> 
> Exit status 3:
> the DEVICE reports that it is not ready for the operation requested. The
> device may be in the process of becoming ready (e.g. spinning up but not
> at speed) so the utility may work after a wait.
> 
> Also possibly a case for locking not being supported.

It seems like you want to use a case statement then in order to handle the 
error codes.
 
Roger.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel