public inbox for pbs-devel@lists.proxmox.com
 help / color / mirror / Atom feed
From: Thomas Lamprecht <t.lamprecht@proxmox.com>
To: pbs-devel@lists.proxmox.com
Subject: [pbs-devel] [PATCH backup] backup: index readers: drop useless shared lock
Date: Fri,  9 Oct 2020 12:45:36 +0200	[thread overview]
Message-ID: <20201009104536.16876-1-t.lamprecht@proxmox.com> (raw)

This is only acquired in those two methods, both as shared. So it has
no use.

It seems, that it was planned in the past that the index deletion
should take the exclusive, while read and write takes the shared
flock on the index, as one can guess from the lock comments in commit
046521895307aa8bde8bab7ea3ef9e437d5ab5e5

But then later, in commit c8ec450e379f54e7ac648b3a3ff701b37e9a6620)
the documented semantics where changed to use a temp file and do an
atomic rename instead for atomicity.

The reader shared flock on the index file was done inbetween,
probably as preparatory step, but was not removed again when strategy
was changed to using the file rename instead.

Do so now, to avoid confusion of readers and a useless flock.

Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
---
 src/backup/dynamic_index.rs | 6 ------
 src/backup/fixed_index.rs   | 6 ------
 2 files changed, 12 deletions(-)

diff --git a/src/backup/dynamic_index.rs b/src/backup/dynamic_index.rs
index 1cc4e53b..8731a418 100644
--- a/src/backup/dynamic_index.rs
+++ b/src/backup/dynamic_index.rs
@@ -90,12 +90,6 @@ impl DynamicIndexReader {
     }
 
     pub fn new(mut file: std::fs::File) -> Result<Self, Error> {
-        if let Err(err) =
-            nix::fcntl::flock(file.as_raw_fd(), nix::fcntl::FlockArg::LockSharedNonblock)
-        {
-            bail!("unable to get shared lock - {}", err);
-        }
-
         // FIXME: This is NOT OUR job! Check the callers of this method and remove this!
         file.seek(SeekFrom::Start(0))?;
 
diff --git a/src/backup/fixed_index.rs b/src/backup/fixed_index.rs
index a2317f0b..eff50055 100644
--- a/src/backup/fixed_index.rs
+++ b/src/backup/fixed_index.rs
@@ -65,12 +65,6 @@ impl FixedIndexReader {
     }
 
     pub fn new(mut file: std::fs::File) -> Result<Self, Error> {
-        if let Err(err) =
-            nix::fcntl::flock(file.as_raw_fd(), nix::fcntl::FlockArg::LockSharedNonblock)
-        {
-            bail!("unable to get shared lock - {}", err);
-        }
-
         file.seek(SeekFrom::Start(0))?;
 
         let header_size = std::mem::size_of::<FixedIndexHeader>();
-- 
2.27.0





             reply	other threads:[~2020-10-09 10:46 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-10-09 10:45 Thomas Lamprecht [this message]
2020-10-09 10:59 ` [pbs-devel] applied: " Dietmar Maurer

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20201009104536.16876-1-t.lamprecht@proxmox.com \
    --to=t.lamprecht@proxmox.com \
    --cc=pbs-devel@lists.proxmox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal