Various improvements.

Signed-off-by: Volker Theile <votdev@gmx.de> (cherry picked from commit 8632a6c)
openmediavault · Aug 28, 2018 · 8327da9 · 8327da9
1 parent 6956ec9
commit 8327da9
Show file tree

Hide file tree

Showing 3 changed files with 24 additions and 21 deletions.
diff --git a/deb/openmediavault-usbbackup/debian/changelog b/deb/openmediavault-usbbackup/debian/changelog
@@ -1,7 +1,7 @@
 openmediavault-usbbackup (4.0.7-1) stable; urgency=low
 
-  * Fix bug in scripts when devicefile like /dev/disk/by-label/...
-    contain blanks.
+  * Fix various bugs, e.g. in shell scripts when the devicefile looks like
+    /dev/disk/by-label/... and contains blanks.
 
  -- Volker Theile <volker.theile@openmediavault.org>  Sun, 26 Aug 2018 22:09:03 +0200
 

diff --git a/deb/openmediavault-usbbackup/usr/share/openmediavault/engined/rpc/usbbackup.inc b/deb/openmediavault-usbbackup/usr/share/openmediavault/engined/rpc/usbbackup.inc
@@ -230,23 +230,25 @@ class OMVRpcServiceUsbBackup extends \OMV\Rpc\ServiceAbstract {
 		// Get the configuration object.
 		$db = \OMV\Config\Database::getInstance();
 		$object = $db->get("conf.service.usbbackup.job", $params['uuid']);
-		// Get the file system and mount it if necessary.
+		// Get the device file of the destination storage device and
+		// generate the mount point path. This is generated using the
+		// MD5 sum of the device file.
 		$deviceFile = $object->get("devicefile");
-		\OMV\System\Filesystem\Filesystem::assertGetImpl($deviceFile);
-		$fs = \OMV\System\Filesystem\Filesystem::getImpl($deviceFile);
-		if (FALSE === $fs->isMounted()) {
-			// Make sure the mount point exists.
-			// Note, the name of the mount point is generated from the MD5
-			// sum of the device file.
-			$dir = \OMV\System\MountPoint::buildPath(md5($deviceFile));
-			$mp = new \OMV\System\MountPoint($dir);
-			$mp->create();
-			// Mount the file system at the specified directory.
+		$dir = \OMV\System\MountPoint::buildPath(md5($deviceFile));
+		// Make sure the mount point exists.
+		$mp = new \OMV\System\MountPoint($dir);
+		$mp->create();
+		// Get the file system and mount it if necessary. Note, the
+		// file system is not listed in the fstab, so we need to
+		// specify the mount point, too.
+		if (FALSE === $mp->isMounted()) {
+			\OMV\System\Filesystem\Filesystem::assertGetImpl($deviceFile);
+			$fs = \OMV\System\Filesystem\Filesystem::getImpl($deviceFile);
 			$fs->mount([], $dir);
 		}
 		// Create a background process.
 		return $this->execBgProc(function($bgStatusFilename, $bgOutputFilename)
-		  use ($object, $fs, $mp) {
+		  use ($object, $mp) {
 			// Execute the rsync script.
 			$cmdArgs = [];
   			$cmdArgs[] = "--shell";
@@ -265,7 +267,7 @@ class OMVRpcServiceUsbBackup extends \OMV\Rpc\ServiceAbstract {
 			// Execute the command.
 			$exitStatus = $this->exec($cmdLine, $output, $bgOutputFilename);
 			// Cleanup.
-			$fs->umount();
+			$mp->umount();
 			$mp->unlink();
 			// Check the exit status.
 			if (0 !== $exitStatus)

diff --git a/deb/openmediavault-usbbackup/usr/share/openmediavault/mkconf/usbbackup b/deb/openmediavault-usbbackup/usr/share/openmediavault/mkconf/usbbackup
@@ -24,6 +24,7 @@
 # http://blog.tjll.net/systemd-for-device-activation-and-media-archiving/
 #
 # systemctl --all --full -t device
+# udevadm info --query=property --name=/dev/sdc1 | grep DEVLINKS
 
 set -e
 
@@ -67,9 +68,9 @@ xmlstarlet sel -t \
 		-o "}" -n \
 		-o "[ -e ${runfile} ] && exit 1" -n \
 		-o "# Make sure the storage device is mounted." -n \
-		-v "concat('id=\$(omv_md5 ',devicefile,')')" -n \
+		-v "concat('id=\$(omv_md5 \"',devicefile,'\")')" -n \
 		-o "mountdir=\$(omv_build_mount_dir \${id})" -n \
-		-o "if ! omv_is_mounted \${mountdir} ; then" -n \
+		-o "if ! omv_is_mounted \"\${mountdir}\" ; then" -n \
 		-v "concat('    omv_error \"External storage device <',devicefile,'> not mounted!\"')" -n \
 		-o "    exit 1" -n \
 		-o "fi" -n \
@@ -115,12 +116,12 @@ xmlstarlet sel -t \
   -b \
   ${OMV_CONFIG_FILE} | xmlstarlet unesc |
   while read -r devicefile; do
-	  id=$(omv_md5 ${devicefile})
+	  id=$(omv_md5 "${devicefile}")
 	  scriptfile="${OMV_USBBACKUP_SCRIPTS_DIR}/${OMV_USBBACKUP_JOBS_SCRIPT_PREFIX}${id}"
 	  runfile="/var/run/${OMV_USBBACKUP_JOBS_SCRIPT_PREFIX}${id}"
 	  mountdir=$(omv_build_mount_dir ${id})
 	  unitfile="${OMV_USBBACKUP_SYSTEMD_SCRIPTS_DIR}/${OMV_USBBACKUP_SYSTEMD_UNITFILE_PREFIX}${id}.service"
-	  device=$(systemd-escape --path ${devicefile})
+	  device=$(systemd-escape --path "${devicefile}")
 
 	  xmlstarlet sel -t \
 		-o "#!/bin/sh" -n \
@@ -135,7 +136,7 @@ xmlstarlet sel -t \
 		-o "    rm -f ${runfile}" -n \
 		-o "    rm -f \${outfile}" -n \
 		-o "    # Umount the storage device." -n \
-		-o "    umount ${devicefile} || true" -n \
+		-o "    umount \"${devicefile}\" || true" -n \
 		-o "    rmdir ${mountdir} || true" -n \
 		-o "    exit" -n \
 		-o "}" -n \
@@ -152,7 +153,7 @@ xmlstarlet sel -t \
 		-o "fi" -n \
 		-o "# Mount the storage device." -n \
 		-o "mkdir -p ${mountdir}" -n \
-		-o "mount ${devicefile} ${mountdir}" -n \
+		-o "mount \"${devicefile}\" \"${mountdir}\"" -n \
 		-o "# Make sure the storage device is mounted." -n \
 		-o "if ! omv_is_mounted ${mountdir} ; then" -n \
 		-o "    omv_error \"External storage device '${devicefile}' not mounted!\"" -n \