Page 1 of 8 123 ... LastLast
Results 1 to 10 of 73

Thread: First bash script - critique & comments?

  1. #1
    Join Date
    Mar 2014
    Location
    US
    Posts
    115

    Default First bash script - critique & comments?

    Hi all
    I am just learning bash wrote my first script. It's purpose is to perform a daily backup of hard drive to a usb stick. I appears to be working ok, but would appreciate any insight, comments or suggestions as to how I might improve it. If possible, please don't post actual code, just point me in the right direction or else I'll be to lazy to learn . Thanks all

    Code:
    # Test if usb drive is mounted
    #
    TESTDIR="/media/sparkz"
    if
      [ -d "$TESTDIR" ]
    then
    #  echo "already mounted"
    #  ls -al /media
      rsync -a -v -i --progress -H -s \
      --delete \
      --delete-excluded \
      --exclude-from=/home/sparkz/Backup/exclude_list.txt \
      --log-file=/var/log/rsync.log / /media/
      umount /dev/sdb1
    else
      mount /dev/sdb1 /media/
      rsync -a -v -i --progress -H -s \
      --delete \
      --delete-excluded \
      --exclude-from=/home/sparkz/Backup/exclude_list.txt \
      --log-file=/var/log/rsync.log / /media/
    #  echo "now mounted"
    #  ls -al /media
      umount /dev/sdb1
    fi
    
    
    #
    # rsync exclude list used by full system backup
    #
    dev
    lost+found
    media
    mnt
    proc
    run
    sys
    tmp
    var
    .readahead
    */lost+found/*
    */.readahead/*
    */Trash/*
    *~*
    *.backup
    *.bak
    *.cache
    *.swp
    *.tmp
    *.temp

  2. #2
    Join Date
    Jun 2008
    Location
    Netherlands
    Posts
    29,834

    Default Re: First bash script - critique & comments?

    As there is no extensive explanation about what you want to achieve, it is a bit difficult to comment.

    In any case, therr should be a shebang to make your statements her into a real executable program.

    I am not clea if your testing if a directory exists is the same astesting if something is mounted. As I sid, background information missing.
    Henk van Velden

  3. #3
    Join Date
    Jun 2008
    Location
    Netherlands
    Posts
    29,834

    Default Re: First bash script - critique & comments?

    As an addition, you should not use /media yourself. /media is for the use of the desktop mounting mechanism. And as you now use /media as mountpoint, you are in the way of any mounts there.
    Use e.g. something inside /mnt (and not /mnt itself) or any other place you like.
    Henk van Velden

  4. #4
    Join Date
    Mar 2014
    Location
    US
    Posts
    115

    Default Re: First bash script - critique & comments?

    Quote Originally Posted by hcvv View Post
    As there is no extensive explanation about what you want to achieve, it is a bit difficult to comment.

    In any case, therr should be a shebang to make your statements her into a real executable program.

    I am not clea if your testing if a directory exists is the same astesting if something is mounted. As I sid, background information missing.
    I appreciate the feedback, forgot to copy the whole file for some reason. Yes, I'm testing for a directory to see if the usb is mounted. If there is a better way, then I'm open to suggestions. Right now I'm reading up on 'exit' codes to see if I can use them in this script. Thanks again for the reply and here (hopefully) is the full text:

    Code:
    #! /bin/bash
    #
    # Shell program to run rsyn backup of system
    #
    #
    # Test if usb drive is mounted
    #
    TESTDIR="/media/sparkz"
    if
      [ -d "$TESTDIR" ]
    then
    #  echo "already mounted"
    #  ls -al /media
      rsync -a -v -i --progress -H -s \
      --delete \
      --delete-excluded \
      --exclude-from=/home/sparkz/Backup/exclude_list.txt \
      --log-file=/var/log/rsync.log / /media/
      umount /dev/sdb1
    else
      mount /dev/sdb1 /media/
      rsync -a -v -i --progress -H -s \
      --delete \
      --delete-excluded \
      --exclude-from=/home/sparkz/Backup/exclude_list.txt \
      --log-file=/var/log/rsync.log / /media/
    #  echo "now mounted"
    #  ls -al /media
      umount /dev/sdb1
    fi
    
    
    #
    # rsync exclude list used by full system backup
    #
    dev
    lost+found
    media
    mnt
    proc
    run
    sys
    tmp
    var
    .readahead
    */lost+found/*
    */.readahead/*
    */Trash/*
    *~*
    *.backup
    *.bak
    *.cache
    *.swp
    *.tmp
    *.temp

  5. #5
    Join Date
    Mar 2014
    Location
    US
    Posts
    115

    Default Re: First bash script - critique & comments?

    Quote Originally Posted by hcvv View Post
    As an addition, you should not use /media yourself. /media is for the use of the desktop mounting mechanism. And as you now use /media as mountpoint, you are in the way of any mounts there.
    Use e.g. something inside /mnt (and not /mnt itself) or any other place you like.
    Thanks,
    I also notice it shows up in /var/run/media. would this be a better location to check?

  6. #6
    Join Date
    Jun 2008
    Location
    Netherlands
    Posts
    29,834

    Default Re: First bash script - critique & comments?

    And I can not see the difference between the two rsync statements you have in the two cases. To me they look the same and thus should be outside if the if-the-else contruct.

    And that list at the end should not be in the script. But maybe you do not show one file, but two.

    Please copy/paste complete: the prompt, the command, theoutput and the next promt. Thus we can see waht you did. So the following is "wrong"
    Code:
    inhoud
    nieuwe inhoud
    But this is providing the information needed:
    Code:
    henk@boven:~/test> cat file
    inhoud
    nieuwe inhoud
    henk@boven:~/test>
    Henk van Velden

  7. #7
    Join Date
    Jun 2008
    Location
    Netherlands
    Posts
    29,834

    Default Re: First bash script - critique & comments?

    You did not say so, but as I see it, this is to be a script to be run by the superuser (root).

    You should not depend on that device mounted somewhere (maybe tomorrow on a different place) when an ennduser (thus not root) connects it to the system. To begin with the ownership of the mountpoint will also be wrong.

    As this is a root action,, you should mount as root at a place defined by root (create the mountpoint in advance as root and not in /media or the /var places, that is all for end user media mounts, media meaning CDs DVDs, Stickies with movies, music, etc.) and best is to have an fstab entry for it (with noauto).

    BTW there is no real need to test if it is mounted. When you do the mount, it will mount when it is not mounted and do nothing (maybe a message) when it is already mounted.
    Henk van Velden

  8. #8

    Default AW: Re: First bash script - critique & comments?

    Quote Originally Posted by hcvv View Post
    You did not say so, but as I see it, this is to be a script to be run by the superuser (root).

    You should not depend on that device mounted somewhere (maybe tomorrow on a different place) when an ennduser (thus not root) connects it to the system. To begin with the ownership of the mountpoint will also be wrong.

    As this is a root action,, you should mount as root at a place defined by root (create the mountpoint in advance as root and not in /media or the /var places, that is all for end user media mounts, media meaning CDs DVDs, Stickies with movies, music, etc.) and best is to have an fstab entry for it (with noauto).
    .
    Or use "udisksctl" to mount it. This will mount it the same way as KDE or GNOME do (and to the same mount point of course), and will even be possible if run as a user.
    You could also check whether it's mounted already, but it wouldn't harm if you try to mount it again in that case (it won't be mounted again), as already mentioned.

  9. #9
    Join Date
    Mar 2014
    Location
    US
    Posts
    115

    Default Re: First bash script - critique & comments?

    Quote Originally Posted by hcvv View Post
    And I can not see the difference between the two rsync statements you have in the two cases. To me they look the same and thus should be outside if the if-the-else contruct.

    And that list at the end should not be in the script. But maybe you do not show one file, but two.

    Please copy/paste complete: the prompt, the command, theoutput and the next promt. Thus we can see waht you did. So the following is "wrong"
    Code:
    inhoud
    nieuwe inhoud
    But this is providing the information needed:
    Code:
    henk@boven:~/test> cat file
    inhoud
    nieuwe inhoud
    henk@boven:~/test>
    Thanks, here is the output without the usb mounted (with majority of files deleted to conserve space:

    Code:
    lenovo:/home/sparkz/Backup # ./rsync_script.sh
    sending incremental file list
    .d..t...... etc/webmin/system-status/
    >f.st...... etc/webmin/system-status/info
              1,203 100%    0.00kB/s    0:00:00 (xfr#1, ir-chk=1092/4385)
    .
    .d..t...... root/.kde4/share/config/
    >f..t...... root/.kde4/share/config/kwriterc
              3,578 100%    1.71MB/s    0:00:00 (xfr#34, ir-chk=1014/9661)
    
    sent 34,760,741 bytes  received 13,304 bytes  1,178,781.19 bytes/sec
    total size is 4,332,203,120  speedup is 124.58
    now mounted
    total 76
    drwxr-xr-x  15 root     root   4096 May 11 08:50 .
    drwxr-xr-x  24 root     root   4096 May 11 08:50 ..
    drwxr-xr-x   2 root     root   4096 May 10 12:59 bin
    drwxr-xr-x   4 root     root   4096 May 10 13:24 boot
    drwxr-xr-x 132 root     root  12288 May 10 18:00 etc
    drwxr--r--   2 sambashr users  4096 May  2 15:05 export
    drwxr-xr-x   5 root     root   4096 Sep 27  2013 home
    drwxr-xr-x  14 root     root   4096 May 10 13:01 lib
    drwxr-xr-x   7 root     root   4096 May 10 12:58 lib64
    drwxr-xr-x   2 root     root   4096 Sep 27  2013 opt
    drwx------  18 root     root   4096 May 11 12:27 root
    drwxr-xr-x   2 root     root  12288 May 10 13:02 sbin
    drwxr-xr-x   2 root     root   4096 Sep 27  2013 selinux
    drwxr-xr-x   5 root     root   4096 Sep 27  2013 srv
    drwxr-xr-x  14 root     root   4096 May 10 12:46 usr
    and here it is with the usb already mounted:

    Code:
    lenovo:/home/sparkz/Backup # ./rsync_script.sh
    sending incremental file list
    .d..t...... etc/webmin/system-status/
    >f.st...... etc/webmin/system-status/info
              1,202 100%    0.00kB/s    0:00:00 (xfr#1, ir-chk=1092/4385)
    
    .d..t...... root/.kde4/share/apps/kfileplaces/
    >f.st...... root/.kde4/share/apps/kfileplaces/bookmarks.xml
              2,412 100%    2.40kB/s    0:00:00 (xfr#26, ir-chk=1083/9661)
    >f..t...... root/.kde4/share/apps/kfileplaces/bookmarks.xml.tbcache
                  0 100%    0.00kB/s    0:00:00 (xfr#27, ir-chk=1082/9661)
    
    sent 34,455,877 bytes  received 13,176 bytes  1,767,643.74 bytes/sec
    total size is 4,332,238,099  speedup is 125.68
    now mounted
    total 76
    drwxr-xr-x  15 root     root   4096 May 11 08:50 .
    drwxr-xr-x  24 root     root   4096 May 11 08:50 ..
    drwxr-xr-x   2 root     root   4096 May 10 12:59 bin
    drwxr-xr-x   4 root     root   4096 May 10 13:24 boot
    drwxr-xr-x 132 root     root  12288 May 10 18:00 etc
    drwxr--r--   2 sambashr users  4096 May  2 15:05 export
    drwxr-xr-x   5 root     root   4096 Sep 27  2013 home
    drwxr-xr-x  14 root     root   4096 May 10 13:01 lib
    drwxr-xr-x   7 root     root   4096 May 10 12:58 lib64
    drwxr-xr-x   2 root     root   4096 Sep 27  2013 opt
    drwx------  18 root     root   4096 May 11 12:27 root
    drwxr-xr-x   2 root     root  12288 May 10 13:02 sbin
    drwxr-xr-x   2 root     root   4096 Sep 27  2013 selinux
    drwxr-xr-x   5 root     root   4096 Sep 27  2013 srv
    drwxr-xr-x  14 root     root   4096 May 10 12:46 usr
    I see what you mean, it seems to run the 'else' section regardless..

  10. #10
    Join Date
    Jun 2008
    Location
    Netherlands
    Posts
    29,834

    Default Re: First bash script - critique & comments?

    Sorry, but I did not ask you to run the script. I did ask you to post complete when you show the script. In post #4 above you showed output (looked like a script and some data after it, but shown in one long list), but not how you got that output.

    Another thing I said is that the following lines:
    Code:
    rsync -a -v -i --progress -H -s \
          --delete \
          --delete-excluded \
          --exclude-from=/home/sparkz/Backup/exclude_list.txt \
          --log-file=/var/log/rsync.log / /media/
       umount /dev/sdb1
    appear twice in the script, exactly the same. Those should then of copurse not be inside the if-then-else, but outside. Thus, where you have (I changed the layout to make it better readable for me):
    Code:
    if [ -d "$TESTDIR" ]
    then
            rsync -a -v -i --progress -H -s \
                    --delete \
                    --delete-excluded \
                    --exclude-from=/home/sparkz/Backup/exclude_list.txt \
                    --log-file=/var/log/rsync.log / /media/
            umount /dev/sdb1
    else
            mount /dev/sdb1 /media/
            rsync -a -v -i --progress -H -s \
                    --delete \
                    --delete-excluded \
                    --exclude-from=/home/sparkz/Backup/exclude_list.txt \
                    --log-file=/var/log/rsync.log / /media/
            umount /dev/sdb1
    fi
    You can take the common part out:
    Code:
    if [ -d "$TESTDIR" ]
    then
    else
            mount /dev/sdb1 /media/
    fi
    rsync -a -v -i --progress -H -s \
            --delete \
            --delete-excluded \
            --exclude-from=/home/sparkz/Backup/exclude_list.txt \
            --log-file=/var/log/rsync.log / /media/
    umount /dev/sdb1
    fi
    Which isn't of course correct coding, so change to something like:
    Code:
    if [[ -d ${TESTDIR} ]] || mount /dev/sdb1 /media/
    rsync -a -v -i --progress -H -s \
            --delete \
            --delete-excluded \
            --exclude-from=/home/sparkz/Backup/exclude_list.txt \
            --log-file=/var/log/rsync.log / /media/
    umount /dev/sdb1
    After all, the logic is that you check if it is mounted, when not mount it.
    And then do the rsync, regardless of what happened earlier.
    Last edited by hcvv; 11-May-2014 at 10:55.
    Henk van Velden

Page 1 of 8 123 ... LastLast

Posting Permissions

  • You may not post new threads
  • You may not post replies
  • You may not post attachments
  • You may not edit your posts
  •