From fdc99e30bd2e154623fb71134413a11d695d3420 Mon Sep 17 00:00:00 2001
From: David Steele <david@crunchydata.com>
Date: Wed, 23 Dec 2015 10:48:33 -0500
Subject: [PATCH] Allow restores to be run against a read-only repository.

Two things needed to be changed:
1) Don't open a log file when log-level-file=off
2) New --no-lock option to suppress lock file creation for restores.
---
 doc/xml/change-log.xml                |   2 +-
 doc/xml/reference.xml                 |   9 ++
 lib/BackRest/Common/Lock.pm           | 117 ++++++++++++++------------
 lib/BackRest/Common/Log.pm            |  46 +++++-----
 lib/BackRest/Config/Config.pm         |  15 ++++
 lib/BackRest/Config/ConfigHelpData.pm |  12 +++
 6 files changed, 125 insertions(+), 76 deletions(-)

diff --git a/doc/xml/change-log.xml b/doc/xml/change-log.xml
index 9be22508c..e773b7fbf 100644
--- a/doc/xml/change-log.xml
+++ b/doc/xml/change-log.xml
@@ -9,7 +9,7 @@
         <changelog-release date="XXXX-XX-XX" version="0.90" title="UNDER DEVELOPMENT">
             <release-feature-bullet-list>
                 <release-feature>
-                    <text></text>
+                    <text>Allow restores to be performed on a read-only repository by using <setting>--no-lock</setting> and <setting>--log-level-file=off</setting>.  The <setting>--no-lock</setting> option can only be used with restores.</text>
                 </release-feature>
             </release-feature-bullet-list>
         </changelog-release>
diff --git a/doc/xml/reference.xml b/doc/xml/reference.xml
index 42e987070..16c87c3aa 100644
--- a/doc/xml/reference.xml
+++ b/doc/xml/reference.xml
@@ -594,6 +594,15 @@
                         <example>y</example>
                     </option>
 
+                    <!-- OPERATION - RESTORE COMMAND - LOCK OPTION -->
+                    <option id="lock" name="Lock">
+                        <summary>Create a lock so restores on a stanza cannot run simultaneously.</summary>
+
+                        <text>Locking during restores is enabled by default but can be disabled using --no-lock.  Be <i>very</i> careful when disabling this option because simultaneous restores to a single path might result in a corrupt cluster.</text>
+
+                        <example>--no-lock</example>
+                    </option>
+
                     <!-- OPERATION - RESTORE COMMAND - TYPE OPTION -->
                     <option id="type" name="Type">
                         <summary>Recovery type.</summary>
diff --git a/lib/BackRest/Common/Lock.pm b/lib/BackRest/Common/Lock.pm
index 626bebe5b..2996bb36c 100644
--- a/lib/BackRest/Common/Lock.pm
+++ b/lib/BackRest/Common/Lock.pm
@@ -105,49 +105,54 @@ sub lockAcquire
             {name => 'iProcessIdx', required => false}
         );
 
-    my $strRepoPath = optionGet(OPTION_REPO_PATH);
+    my $bResult = true;
 
-    # Cannot proceed if a lock is currently held
-    if (defined($strCurrentLockType))
+    # Acquire if locking is enabled
+    if (!optionTest(OPTION_LOCK) || optionGet(OPTION_LOCK))
     {
-        confess &lock(ASSERT, "${strCurrentLockType} lock is already held");
-    }
+        $bResult = false;
+        my $strRepoPath = optionGet(OPTION_REPO_PATH);
 
-    # Check if processes are currently stopped
-    lockStopTest($strRepoPath);
-
-    # Create the lock path
-    lockPathCreate($strRepoPath);
-
-    # Attempt to open the lock file
-    $strCurrentLockFile = lockFileName($strLockType, optionGet(OPTION_STANZA, false), $strRepoPath, $bRemote, $iProcessIdx);
-
-    sysopen($hCurrentLockHandle, $strCurrentLockFile, O_WRONLY | O_CREAT, 0640)
-        or confess &log(ERROR, "unable to open lock file ${strCurrentLockFile}", ERROR_FILE_OPEN);
-
-    # Attempt to lock the lock file
-    my $bResult = false;
-
-    if (!flock($hCurrentLockHandle, LOCK_EX | LOCK_NB))
-    {
-        close($hCurrentLockHandle);
-
-        if (!defined($bFailOnNoLock) || $bFailOnNoLock)
+        # Cannot proceed if a lock is currently held
+        if (defined($strCurrentLockType))
         {
-            confess &log(ERROR, "unable to acquire ${strLockType} lock", ERROR_LOCK_ACQUIRE);
+            confess &lock(ASSERT, "${strCurrentLockType} lock is already held");
         }
-    }
-    else
-    {
-        # Write pid into the lock file.  This is used stop terminate processes on a stop --force.
-        syswrite($hCurrentLockHandle, "$$\n")
-            or confess(ERROR, "unable to write process id into lock file ${strCurrentLockFile}", ERROR_FILE_WRITE);
 
-        # Set current lock type so we know we have a lock
-        $strCurrentLockType = $strLockType;
+        # Check if processes are currently stopped
+        lockStopTest($strRepoPath);
 
-        # Lock was successful
-        $bResult = true;
+        # Create the lock path
+        lockPathCreate($strRepoPath);
+
+        # Attempt to open the lock file
+        $strCurrentLockFile = lockFileName($strLockType, optionGet(OPTION_STANZA, false), $strRepoPath, $bRemote, $iProcessIdx);
+
+        sysopen($hCurrentLockHandle, $strCurrentLockFile, O_WRONLY | O_CREAT, 0640)
+            or confess &log(ERROR, "unable to open lock file ${strCurrentLockFile}", ERROR_FILE_OPEN);
+
+        # Attempt to lock the lock file
+        if (!flock($hCurrentLockHandle, LOCK_EX | LOCK_NB))
+        {
+            close($hCurrentLockHandle);
+
+            if (!defined($bFailOnNoLock) || $bFailOnNoLock)
+            {
+                confess &log(ERROR, "unable to acquire ${strLockType} lock", ERROR_LOCK_ACQUIRE);
+            }
+        }
+        else
+        {
+            # Write pid into the lock file.  This is used stop terminate processes on a stop --force.
+            syswrite($hCurrentLockHandle, "$$\n")
+                or confess(ERROR, "unable to write process id into lock file ${strCurrentLockFile}", ERROR_FILE_WRITE);
+
+            # Set current lock type so we know we have a lock
+            $strCurrentLockType = $strLockType;
+
+            # Lock was successful
+            $bResult = true;
+        }
     }
 
     # Return from function and log return values if any
@@ -177,29 +182,33 @@ sub lockRelease
             {name => 'bFailOnNoLock', default => true}
         );
 
-    # Fail if there is no lock
-    if (!defined($strCurrentLockType))
+    # Release if locking is enabled
+    if (!optionTest(OPTION_LOCK) || optionGet(OPTION_LOCK))
     {
-        if ($bFailOnNoLock)
+        # Fail if there is no lock
+        if (!defined($strCurrentLockType))
         {
-            confess &log(ASSERT, 'no lock is currently held');
+            if ($bFailOnNoLock)
+            {
+                confess &log(ASSERT, 'no lock is currently held');
+            }
         }
-    }
-    else
-    {
-        # # Fail if the lock being released is not the one held
-        # if ($strLockType ne $strCurrentLockType)
-        # {
-        #     confess &log(ASSERT, "cannot remove lock ${strLockType} since ${strCurrentLockType} is currently held");
-        # }
+        else
+        {
+            # # Fail if the lock being released is not the one held
+            # if ($strLockType ne $strCurrentLockType)
+            # {
+            #     confess &log(ASSERT, "cannot remove lock ${strLockType} since ${strCurrentLockType} is currently held");
+            # }
 
-        # Remove the file
-        unlink($strCurrentLockFile);
-        close($hCurrentLockHandle);
+            # Remove the file
+            unlink($strCurrentLockFile);
+            close($hCurrentLockHandle);
 
-        # Undef lock variables
-        undef($strCurrentLockType);
-        undef($hCurrentLockHandle);
+            # Undef lock variables
+            undef($strCurrentLockType);
+            undef($hCurrentLockHandle);
+        }
     }
 
     # Return from function and log return values if any
diff --git a/lib/BackRest/Common/Log.pm b/lib/BackRest/Common/Log.pm
index 6552c72f9..5fda080b4 100644
--- a/lib/BackRest/Common/Log.pm
+++ b/lib/BackRest/Common/Log.pm
@@ -99,29 +99,33 @@ sub logFileSet
 {
     my $strFile = shift;
 
-    unless (-e dirname($strFile))
+    # Only open the log file if file logging is enabled
+    if ($strLogLevelFile ne OFF)
     {
-        mkdir(dirname($strFile), oct('0770'))
-            or die "unable to create directory " . dirname($strFile) . " for log file ${strFile}";
+        unless (-e dirname($strFile))
+        {
+            mkdir(dirname($strFile), oct('0770'))
+                or die "unable to create directory " . dirname($strFile) . " for log file ${strFile}";
+        }
+
+        $strFile .= '-' . timestampFormat('%4d%02d%02d') . '.log';
+        my $bExists = false;
+
+        if (-e $strFile)
+        {
+            $bExists = true;
+        }
+
+        sysopen($hLogFile, $strFile, O_WRONLY | O_CREAT | O_APPEND, 0660)
+            or confess &log(ERROR, "unable to open log file ${strFile}", ERROR_FILE_OPEN);
+
+        if ($bExists)
+        {
+            syswrite($hLogFile, "\n");
+        }
+
+        syswrite($hLogFile, "-------------------PROCESS START-------------------\n");
     }
-
-    $strFile .= '-' . timestampFormat('%4d%02d%02d') . '.log';
-    my $bExists = false;
-
-    if (-e $strFile)
-    {
-        $bExists = true;
-    }
-
-    sysopen($hLogFile, $strFile, O_WRONLY | O_CREAT | O_APPEND, 0660)
-        or confess &log(ERROR, "unable to open log file ${strFile}", ERROR_FILE_OPEN);
-
-    if ($bExists)
-    {
-        syswrite($hLogFile, "\n");
-    }
-
-    syswrite($hLogFile, "-------------------PROCESS START-------------------\n");
 }
 
 push @EXPORT, qw(logFileSet);
diff --git a/lib/BackRest/Config/Config.pm b/lib/BackRest/Config/Config.pm
index 0ecf90dde..9e56d8d0e 100644
--- a/lib/BackRest/Config/Config.pm
+++ b/lib/BackRest/Config/Config.pm
@@ -218,6 +218,8 @@ use constant OPTION_TYPE                                            => 'type';
     push @EXPORT, qw(OPTION_TYPE);
 use constant OPTION_OUTPUT                                          => 'output';
     push @EXPORT, qw(OPTION_OUTPUT);
+use constant OPTION_LOCK                                            => 'lock';
+    push @EXPORT, qw(OPTION_LOCK);
 
 # Command-line only help/version
 #-----------------------------------------------------------------------------------------------------------------------------------
@@ -363,6 +365,8 @@ use constant OPTION_DEFAULT_RESTORE_TARGET_RESUME                   => false;
     push @EXPORT, qw(OPTION_DEFAULT_RESTORE_TARGET_RESUME);
 use constant OPTION_DEFAULT_RESTORE_TYPE                            => RECOVERY_TYPE_DEFAULT;
     push @EXPORT, qw(OPTION_DEFAULT_RESTORE_TYPE);
+use constant OPTION_DEFAULT_LOCK                                    => true;
+    push @EXPORT, qw(OPTION_DEFAULT_LOCK);
 
 # Command-line only test
 #-----------------------------------------------------------------------------------------------------------------------------------
@@ -545,6 +549,17 @@ my %oOptionRule =
         }
     },
 
+    &OPTION_LOCK =>
+    {
+        &OPTION_RULE_TYPE => OPTION_TYPE_BOOLEAN,
+        &OPTION_RULE_DEFAULT => OPTION_DEFAULT_LOCK,
+        &OPTION_RULE_NEGATE => true,
+        &OPTION_RULE_COMMAND =>
+        {
+            &CMD_RESTORE => true
+        }
+    },
+
     &OPTION_NO_START_STOP =>
     {
         &OPTION_RULE_TYPE => OPTION_TYPE_BOOLEAN,
diff --git a/lib/BackRest/Config/ConfigHelpData.pm b/lib/BackRest/Config/ConfigHelpData.pm
index ca2f65cf8..95a62ed83 100644
--- a/lib/BackRest/Config/ConfigHelpData.pm
+++ b/lib/BackRest/Config/ConfigHelpData.pm
@@ -853,6 +853,18 @@ my $oConfigHelpData =
                             "combination with --delta a timestamp/size delta will be performed instead of using checksums."
                 },
 
+                # LOCK Option Help
+                #-------------------------------------------------------------------------------------------------------------------
+                'lock' =>
+                {
+                    summary =>
+                        "Create a lock so restores on a stanza cannot run simultaneously.",
+                    description =>
+                        "Locking during restores is enabled by default but can be disabled using --no-lock. Be very careful " .
+                            "when disabling this option because simultaneous restores to a single path might result in a " .
+                            "corrupt cluster."
+                },
+
                 'log-level-console' => 'section',
                 'log-level-file' => 'section',
                 'neutral-umask' => 'section',