5
\$\begingroup\$

The following script closes a virtual machine (which is run in a screen session), waits for the session to close, creates a backup of the VM, and restarts the VM. The shutdown and bootup scripts speak for themselves, but I can post them if necessary. Is there any way to clean up the sockets_found function? It seems like there should be an easier way to detect whether or not screen has any open sessions.

#!/bin/bash
now=`date '+%Y%m%d'`
# No Sockets found
# There is a screen on
function sockets_found {
 screen -ls | grep "There is a screen on"
 if [ $? -eq 1 ]; then
 return 1
 else
 return 0
 fi
}
function wait_for_sockets_to_close {
 while sockets_found; do
 echo "Waiting for screen to close..."
 sleep 1
 done;
}
echo "Shutdown VM..."
/bin/bash ~/shutdown.sh
wait_for_sockets_to_close
# ensure that the backup directory exists
mkdir -p ~/backup
echo "Copying VM to backup directory..."
cp -Rf ~/VirtualBox\ VMs/ ~/backup/VirtualBox\ VMs${now}/
echo "Booting VM..."
/bin/bash ~/bootup.sh
asked Nov 5, 2012 at 13:23
\$\endgroup\$

3 Answers 3

1
\$\begingroup\$

This can be improved and simplified:

function sockets_found {
 screen -ls | grep "There is a screen on"
 if [ $? -eq 1 ]; then
 return 1
 else
 return 0
 fi
}

Like this:

function sockets_found {
 screen -ls | grep -q "There is a screen on"
}

That is:

  • You can remove the if statement, as the exit code of grep naturally becomes the exit code of the function
  • If the exit code of grep is greater than 1, the original function exits with 0. That's inappropriate. Such exit codes indicate errors in grep, and do not imply that screen sessions exist
  • I added the -q flag to suppress the output of grep in case of success

Other minor things:

  • No need for the ; in done;
  • It would be better to double-quote ${now} in the backup target directory to prevent accidental globbing or word splitting, in case you might make a mistake in the syntax of the date command
  • It would be good to add some blank lines in the last chunk of commands for better readability

Like this:

echo "Shutdown VM..."
/bin/bash ~/shutdown.sh
wait_for_sockets_to_close
# ensure that the backup directory exists
mkdir -p ~/backup
echo "Copying VM to backup directory..."
now=$(date '+%Y%m%d')
cp -Rf ~/VirtualBox\ VMs/ ~/backup/VirtualBox\ VMs"${now}"/
echo "Booting VM..."
/bin/bash ~/bootup.sh
answered May 14, 2016 at 6:27
\$\endgroup\$
2
\$\begingroup\$

You also can find if the socket file exists on /var/run/screen/S-yourname/pid.screenname.

palacsint
30.4k9 gold badges82 silver badges157 bronze badges
answered Nov 15, 2012 at 10:05
\$\endgroup\$
1
\$\begingroup\$

There is no need to grep here:

screen -ls | grep "There is a screen on"
if [ $? -eq 1 ]; then

Screen -ls will return with exit code 1 in the case no sockets are found in /var/run.

I would run the date command just before the copy is run, in case the sockets close during a date roll and you get the wrong date on your backup.

echo "Copying VM to backup directory..."
now=$(date '+%Y%m%d')
cp -Rf ~/VirtualBox\ VMs/ ~/backup/VirtualBox\ VMs${now}/

As above use $(cmd) notation instead of backtick notation. They achieve the same thing, but $() is visually clearer and can be nested.

answered Nov 21, 2012 at 15:06
\$\endgroup\$

You must log in to answer this question.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.