I have found one bug and some enhancement ideas.
I think in the new script this line:
for VM_NAME in `cat ${VM_INPUT} | sed '/^$/d' | sed -e 's/^[ \t]*//;s/[ \t]*$//'`;
Should be:
for VM_NAME in `cat ${VM_INPUT} | sed '/^$/d' | sed -e 's/^[:blank:]*//;s/[:blank:]*$//'`;
Else you are going to just remove any "t","\" or spaces at beginning or end.
While at that, maybe this lines:
VMX_DIR=`echo ${VMX_CONF%.vmx}`
VMX_DIR=`echo ${VMX_DIR%/*}`
Could be combined to:
VMX_DIR=`echo ${VMX_CONF%/*.vmx}`
At least in my tests it produced the same output that way ...
Then all
grep -E "${VM_NAME}" /tmp/vms_list | ...
should be changed to:
grep -E \"${VM_NAME}\" /tmp/vms_list | ...
So that the " is part of the search pattern. Else if input file contains 'test', it would also match for VMs that are called 'test1' or 'test vm' or anything else containing that word. Now that we have the " in the vms_list we should take benefit from it
Regarding the crontab:
I think it should be enough to do amkdir /etc/rc.early.d
and then create a script file in it:
echo 'echo "01 05 * * 1-5 /opt/Backup/ghettoVCBni.sh /opt/Backup/VMs2Backup" >>/var/spool/cron/crontabs/root' > /etc/rc.early.d/AddBackup2Crontab.sh
chown root:root /etc/rc.early.d/AddBackup2Crontab.sh
chmod 774 /etc/rc.early.d/AddBackup2Crontab.sh
I still need to test this, but I would think this should work.
-JoeSt
Hi William
Is there any special reason why there needs to be an input file that contains the display names of the VM's you want to back up? I tried your first version of the script and I'm sure it didn't do this (could have dreamt it
however the new one seems to require this, is it becuase of the spaces in display names fix?Good work on the script though...
kpc,
Both version 1 and 2 of the script has always worked in that fashion. You would feed in a list of the Virtual Machines you would like backed up and they're queried and backed up. How else would you do it
The reason for the display name, which I think most user's have both the display name and the actual name of their directory/etc all match up, is that VMware allows you to have a different display name than the actual directory and possibly some of the other attributes. The queries that are return by using either "vmware-cmd -l" or "vmware-vim-cmd vmsvc/getallvms" show only display name
Hopefully this answered your question.
Both version 1 and 2 of the script has always worked in that fashion. You would feed in a list of the Virtual Machines you would like backed up and they're queried and backed up. How else would you do it
The reason for the display name, which I think most user's have both the display name and the actual name of their directory/etc all match up, is that VMware allows you to have a different display name than the actual directory and possibly some of the other attributes. The queries that are return by using either "vmware-cmd -l" or "vmware-vim-cmd vmsvc/getallvms" show only display name
Hopefully this answered your question.
JoSte wrote:
I think in the new script this line:
Should be:
Else you are going to just remove any "t","\" or spaces at beginning or end.
"\t" is the tab character, so it'd remove either spaces or tabs. Also, your sed command wouldn't work; you'd need to recode it as:I think in the new script this line:
for VM_NAME in `cat ${VM_INPUT} | sed '/^$/d' | sed -e 's/^[ \t]*//;s/[ \t]*$//'`;
Should be:
for VM_NAME in `cat ${VM_INPUT} | sed '/^$/d' | sed -e 's/^[:blank:]*//;s/[:blank:]*$//'`;
Else you are going to just remove any "t","\" or spaces at beginning or end.
for VM_NAME in `cat "${VM_INPUT}" | sed '/^$/d' | sed -e 's/^[[:blank:]]*//;s/[[:blank:]]*$//'`
Or:
cat "${VM_INPUT}" | sed '/^$/d' | sed -e 's/^[[:blank:]]*//;s/[[:blank:]]*$//' | while read VM_NAME;
Note the quotes around ${VM_INPUT}; without them, any spaces in the variable would break the script (the shell would see them as argument separators).
While at that, maybe this lines:
Could be combined to:
At least in my tests it produced the same output that way ...
'dirname' was intended for that specific purpose:VMX_DIR=`echo ${VMX_CONF%.vmx}`
VMX_DIR=`echo ${VMX_DIR%/*}`
Could be combined to:
VMX_DIR=`echo ${VMX_CONF%/*.vmx}`
At least in my tests it produced the same output that way ...
VMX_DIR=`dirname "${VMX_CONF}"`
The quotes ensure that spaces in the path are passed to the command correctly.
Then all
should be changed to:
So that the " is part of the search pattern. Else if input file contains 'test', it would also match for VMs that are called 'test1' or 'test vm' or anything else containing that word. Now that we have the " in the vms_list we should take benefit from it
Preferably you'd use word boundaries in the regular expression so that you're not forced to have quotes in the VM names:grep -E "${VM_NAME}" /tmp/vms_list | ...
should be changed to:
grep -E \"${VM_NAME}\" /tmp/vms_list | ...
So that the " is part of the search pattern. Else if input file contains 'test', it would also match for VMs that are called 'test1' or 'test vm' or anything else containing that word. Now that we have the " in the vms_list we should take benefit from it
grep -E "\<${VM_NAME}\>" /tmp/vms_list | ...
That way, if $VM_NAME == 'test', it won't match 'tests', 'test1', 'newtest', etc.