all lists on lists.proxmox.com
 help / color / mirror / Atom feed
From: Gabriel Goller <g.goller@proxmox.com>
To: "Fabian Grünbichler" <f.gruenbichler@proxmox.com>,
	"Proxmox Backup Server development discussion"
	<pbs-devel@lists.proxmox.com>
Subject: Re: [pbs-devel] [PATCH v2 proxmox{, -backup} 0/2] Move ProcessLocker to tmpfs
Date: Wed, 6 Dec 2023 14:56:25 +0100	[thread overview]
Message-ID: <2507e464-7b0a-4814-b089-dc5b1d8d2904@proxmox.com> (raw)
In-Reply-To: <1764237283.1899.1701870086441@webmail.proxmox.com>

On 12/6/23 14:41, Fabian Grünbichler wrote:
>>
>> Gabriel Goller <g.goller@proxmox.com> hat am 06.12.2023 14:28 CET 
>> geschrieben: This moves the `ProcessLocker`'s `.lock` file to 
>> `/run/proxmox-backup/locks` (tmpfs). The first patch only converts 
>> all the `F_SETLK` flags to `F_OFD_SETLK` flags. This changes normal 
>> locks, which are based on the process, to locks based on an open file 
>> descriptor. This actually doesn't change anything, because we use 
>> mutexes, so the lock is already thread-safe. It would be cleaner 
>> though and would safe us from weird quirks like closing the lock-file 
>> which would drop all the locks when using the POSIX `F_SETLK`. See 
>> more here [0].
>>
> this might be moot, since most likely both patches go in at the same 
> time, is this change reload/upgrade-compatible? i.e., if an old 
> proxmox-backup(-proxy) process is (still) running that has the lock 
> open with F_SETLK, and the new one obtains it using F_OFD_SETLK, is 
> the behaviour still correct? (the other direction might be interesting 
> too, but can only happen on an unsupported downgrade)
>
Just spoke with Stefan Sterz about this and we will probably 
apply/release this with a major version bump (kernel update), so that 
the user
is forced to reboot the system (same as with his tmpfs locking series).
I don't think there is another way, because the lockfiles get moved to 
another dir. Although F_SETLK and F_OFD_SETLK should be compatible,
so having one process use F_SETLK and another F_OFD_SETLK *should* still 
work (don't take my word for it though).





  reply	other threads:[~2023-12-06 13:56 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-12-06 13:28 Gabriel Goller
2023-12-06 13:28 ` [pbs-devel] [PATCH v2 proxmox 1/2] process_locker: use ofd locking Gabriel Goller
2023-12-06 13:28 ` [pbs-devel] [PATCH v2 proxmox-backup 2/2] datastore: store datastore lock on tmpfs Gabriel Goller
2023-12-06 13:41 ` [pbs-devel] [PATCH v2 proxmox{, -backup} 0/2] Move ProcessLocker to tmpfs Fabian Grünbichler
2023-12-06 13:56   ` Gabriel Goller [this message]
2023-12-06 14:14     ` Fabian Grünbichler
2023-12-06 14:21       ` Gabriel Goller
2023-12-06 14:33         ` Fabian Grünbichler
2023-12-06 14:36   ` Thomas Lamprecht
2023-12-06 14:46     ` Gabriel Goller
2023-12-06 14:58       ` Thomas Lamprecht

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=2507e464-7b0a-4814-b089-dc5b1d8d2904@proxmox.com \
    --to=g.goller@proxmox.com \
    --cc=f.gruenbichler@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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal