Improvement of backup iso script
Message ID | 1483367788-911-1-git-send-email-jonatan.schlag@ipfire.org |
---|---|
State | Superseded |
Headers |
Return-Path: <development-bounces@lists.ipfire.org> Received: from mail01.ipfire.org (unknown [172.28.1.200]) by web02.ipfire.org (Postfix) with ESMTP id B0C9361FEA for <patchwork@ipfire.org>; Mon, 2 Jan 2017 15:36:34 +0100 (CET) Received: from mail01.ipfire.org (localhost [IPv6:::1]) by mail01.ipfire.org (Postfix) with ESMTP id 3126C933; Mon, 2 Jan 2017 15:36:34 +0100 (CET) Received: from fangorn.local.familyschlag (dslb-088-073-210-219.088.073.pools.vodafone-ip.de [88.73.210.219]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-SHA256 (128/128 bits)) (No client certificate requested) by mail01.ipfire.org (Postfix) with ESMTPSA id AC407933; Mon, 2 Jan 2017 15:36:31 +0100 (CET) From: Jonatan Schlag <jonatan.schlag@ipfire.org> To: development@lists.ipfire.org Subject: [PATCH] Improvement of backup iso script Date: Mon, 2 Jan 2017 15:36:28 +0100 Message-Id: <1483367788-911-1-git-send-email-jonatan.schlag@ipfire.org> X-Mailer: git-send-email 2.1.4 X-BeenThere: development@lists.ipfire.org X-Mailman-Version: 2.1.20 Precedence: list List-Id: IPFire development talk <development.lists.ipfire.org> List-Unsubscribe: <http://lists.ipfire.org/mailman/options/development>, <mailto:development-request@lists.ipfire.org?subject=unsubscribe> List-Archive: <http://lists.ipfire.org/pipermail/development/> List-Post: <mailto:development@lists.ipfire.org> List-Help: <mailto:development-request@lists.ipfire.org?subject=help> List-Subscribe: <http://lists.ipfire.org/mailman/listinfo/development>, <mailto:development-request@lists.ipfire.org?subject=subscribe> Errors-To: development-bounces@lists.ipfire.org Sender: "Development" <development-bounces@lists.ipfire.org> |
Message
Jonatan Schlag
Jan. 3, 2017, 1:36 a.m. UTC
The backup iso script did not check the arch of the host. On x86_64 host
the wrong iso was downloaded.
Furthermore, there were some if clauses which could cause trouble which
I also tried to improve.
(For example: -e is valid if we have a directory or a file, but we want
to check for a file only )
Fixes: 11258
Signed-off-by: Jonatan Schlag <jonatan.schlag@ipfire.org>
---
src/scripts/backupiso | 31 ++++++++++++++++++++++++++-----
1 file changed, 26 insertions(+), 5 deletions(-)
Comments
Hi, thanks for having a look at this :) On Mon, 2017-01-02 at 15:36 +0100, Jonatan Schlag wrote: > The backup iso script did not check the arch of the host. On x86_64 host > the wrong iso was downloaded. > > Furthermore, there were some if clauses which could cause trouble which > I also tried to improve. > (For example: -e is valid if we have a directory or a file, but we want > to check for a file only ) > > Fixes: 11258 > > Signed-off-by: Jonatan Schlag <jonatan.schlag@ipfire.org> > --- > src/scripts/backupiso | 31 ++++++++++++++++++++++++++----- > 1 file changed, 26 insertions(+), 5 deletions(-) > > diff --git a/src/scripts/backupiso b/src/scripts/backupiso > index 014e8e9..e92f385 100644 > --- a/src/scripts/backupiso > +++ b/src/scripts/backupiso > @@ -1,9 +1,29 @@ > #!/bin/sh > +arch=$(uname -m) > + > +case $arch in > + "i586") > + arch="i586" > + echo "Your arch is $arch" > + ;; > + "i686") > + arch="i586" > + echo "Your arch is $arch" > + ;; > + "x86_64") > + arch="x86_64" > + echo "Your arch is $arch" > + ;; > + *) > + echo "Arch is not supported" > + exit 1 > + ;; > +esac You could make this a bit shorter by using i?86 for i586 and i686. I also would recommend to not use quotes for the matches. I also indent it like this: case x in a) some code ;; b) ... ;; esac > COREVER=$(cat /opt/pakfire/db/core/mine) > # FIXME: edit this lines before release > URL="http://download.ipfire.org/releases/ipfire-2.x/2.19-core$COREVER/" > -ISO="ipfire-2.19.i586-full-core$COREVER.iso" > +ISO="ipfire-2.19.$arch-full-core$COREVER.iso" > > if [ -z $1 ]; then > echo usage: $0 backup-file > @@ -13,9 +33,9 @@ fi > TS=$1 > > mkdir -p /var/tmp/backupiso > -cd /var/tmp/backupiso > +(cd /var/tmp/backupiso || exit > > -if [ ! -e ${ISO} ] > +if [ ! -f ${ISO} ] > then > echo "Fetching ${URL}${ISO}" > wget --quiet -c ${URL}${ISO} > @@ -26,7 +46,7 @@ wget --quiet -O ${ISO}.md5 ${URL}${ISO}.md5 > > echo "Checking md5 of ${ISO}" > md5sum --status -c ${ISO}.md5 > -if [ $? -eq 0 -o $? -eq 24 ] > +if [ $? -eq 0 ] || [ $? -eq 24 ] > then > echo "md5 is OK" > else It would be nice to know what 24 is for. > @@ -35,7 +55,7 @@ else > wget --quiet -O ${ISO} ${URL}${ISO} > echo "Checking again md5 of ${ISO}" > md5sum --status -c ${ISO}.md5 > - if [ $? -eq 0 -o $? -eq 24 ] > + if [ $? -eq 0 ] || [ $? -eq 24 ] > then > echo "md5 is OK" > else > @@ -64,3 +84,4 @@ isohybrid $(basename ${ISO} .iso)-${TS}.iso > > echo "Cleaning up" > rm -rf backupiso.${TS} > +) Why is this block executed in a sub-shell? -Michael
On Mon, 2017-01-02 at 16:04 +0100, Jonatan Schlag wrote: > > Am Mo, 2. Jan, 2017 um 3:47 schrieb Michael Tremer > <michael.tremer@ipfire.org>: > > > > Hi, > > > > thanks for having a look at this :) > > > > On Mon, 2017-01-02 at 15:36 +0100, Jonatan Schlag wrote: > > > > > > The backup iso script did not check the arch of the host. On x86_64 > > > host > > > the wrong iso was downloaded. > > > > > > Furthermore, there were some if clauses which could cause trouble > > > which > > > I also tried to improve. > > > (For example: -e is valid if we have a directory or a file, but we > > > want > > > to check for a file only ) > > > > > > Fixes: 11258 > > > > > > Signed-off-by: Jonatan Schlag <jonatan.schlag@ipfire.org> > > > --- > > > src/scripts/backupiso | 31 ++++++++++++++++++++++++++----- > > > 1 file changed, 26 insertions(+), 5 deletions(-) > > > > > > diff --git a/src/scripts/backupiso b/src/scripts/backupiso > > > index 014e8e9..e92f385 100644 > > > --- a/src/scripts/backupiso > > > +++ b/src/scripts/backupiso > > > @@ -1,9 +1,29 @@ > > > #!/bin/sh > > > +arch=$(uname -m) > > > + > > > +case $arch in > > > + "i586") > > > + arch="i586" > > > + echo "Your arch is $arch" > > > + ;; > > > + "i686") > > > + arch="i586" > > > + echo "Your arch is $arch" > > > + ;; > > > + "x86_64") > > > + arch="x86_64" > > > + echo "Your arch is $arch" > > > + ;; > > > + *) > > > + echo "Arch is not supported" > > > + exit 1 > > > + ;; > > > +esac > > > > You could make this a bit shorter by using i?86 for i586 and i686. I > > also would > > recommend to not use quotes for the matches. > > > > I also indent it like this: > > > > case x in > > a) > > some code > > ;; > > b) > > ... > > ;; > > esac > > I will send a new patch with better indention. > > > > > > > > > > COREVER=$(cat /opt/pakfire/db/core/mine) > > > # FIXME: edit this lines before release > > > > > > URL="http://download.ipfire.org/releases/ipfire-2.x/2.19-core$COREVER/" > > > -ISO="ipfire-2.19.i586-full-core$COREVER.iso" > > > +ISO="ipfire-2.19.$arch-full-core$COREVER.iso" > > > > > > if [ -z $1 ]; then > > > echo usage: $0 backup-file > > > @@ -13,9 +33,9 @@ fi > > > TS=$1 > > > > > > mkdir -p /var/tmp/backupiso > > > -cd /var/tmp/backupiso > > > +(cd /var/tmp/backupiso || exit > > > > Here 1 > > > > > > > > > > > > -if [ ! -e ${ISO} ] > > > +if [ ! -f ${ISO} ] > > > then > > > echo "Fetching ${URL}${ISO}" > > > wget --quiet -c ${URL}${ISO} > > > @@ -26,7 +46,7 @@ wget --quiet -O ${ISO}.md5 ${URL}${ISO}.md5 > > > > > > echo "Checking md5 of ${ISO}" > > > md5sum --status -c ${ISO}.md5 > > > -if [ $? -eq 0 -o $? -eq 24 ] > > > +if [ $? -eq 0 ] || [ $? -eq 24 ] > > > then > > > echo "md5 is OK" > > > else > > > > It would be nice to know what 24 is for. > > Yes, but I do not know it and after some search in the web I did not > found a explanation. > I would keep the 24 till we can sure why it was added and If we could > remove it without problems. > > > > > > > > > > > > @@ -35,7 +55,7 @@ else > > > wget --quiet -O ${ISO} ${URL}${ISO} > > > echo "Checking again md5 of ${ISO}" > > > md5sum --status -c ${ISO}.md5 > > > - if [ $? -eq 0 -o $? -eq 24 ] > > > + if [ $? -eq 0 ] || [ $? -eq 24 ] > > > then > > > echo "md5 is OK" > > > else > > > @@ -64,3 +84,4 @@ isohybrid $(basename ${ISO} .iso)-${TS}.iso > > > > > > echo "Cleaning up" > > > rm -rf backupiso.${TS} > > > +) > > > > Why is this block executed in a sub-shell? > > To fail gracefully (Here1) when /var/tmp/backupiso not exists. That should never exist. Please use mktemp to create a temporary file. -Michael > > > > > > > > > > > -Michael > > Jonatan >