From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from gate001.proxmox.com (gate001.proxmox.com [45.144.208.40]) by lore.proxmox.com (Postfix) with ESMTPS id AA5581FF09B for ; Mon, 26 Oct 2026 08:56:12 +0100 (CET) Received: from gate001.proxmox.com (localhost.localdomain [127.0.0.1]) by gate001.proxmox.com (Proxmox) with ESMTP id 057EA211F3; Mon, 26 Oct 2026 08:56:12 +0100 (CET) From: Kefu Chai To: pve-devel@lists.proxmox.com Subject: [PATCH manager] fix #8195: api: ceph: mds: lock ceph.conf when changing MDS sections Date: Mon, 26 Oct 2026 15:56:04 +0800 Message-ID: <20261026075604.1101150-1-k.chai@proxmox.com> X-Mailer: git-send-email 2.47.3 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1793001367772 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.292 Adjusted score from AWL reputation of From: address DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment (newer systems) RCVD_IN_DNSWL_MED -2.3 Sender listed at https://www.dnswl.org/, medium trust SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Message-ID-Hash: UGUSFA6C54QQIACBW67QDMEXJLCVIWV6 X-Message-ID-Hash: UGUSFA6C54QQIACBW67QDMEXJLCVIWV6 X-MailFrom: k.chai@proxmox.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; loop; banned-address; emergency; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header X-Mailman-Version: 3.3.10 Precedence: list List-Id: Proxmox VE development discussion List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: createmds and destroymds read, modify and write ceph.conf without the cfs lock, so concurrent tasks can lose each other's changes. E.g. destroying two MDS in parallel can leave one section behind, blocking that ID from being reused. Take the lock for each update, as the monitor code does. The createmds error path runs after the lock was released, so it only removes the section if it still names this node. Signed-off-by: Kefu Chai --- PVE/API2/Ceph/MDS.pm | 66 ++++++++++++++++++++++++++++++-------------- 1 file changed, 46 insertions(+), 20 deletions(-) diff --git a/PVE/API2/Ceph/MDS.pm b/PVE/API2/Ceph/MDS.pm index 1a579115..25110a98 100644 --- a/PVE/API2/Ceph/MDS.pm +++ b/PVE/API2/Ceph/MDS.pm @@ -179,24 +179,32 @@ __PACKAGE__->register_method({ my $timeout = PVE::Ceph::Tools::get_config('long_rados_timeout'); my $rados = PVE::RADOS->new(timeout => $timeout); - my $cfg = cfs_read_file('ceph.conf'); - my $section = "mds.$mds_id"; - if (defined($cfg->{$section})) { - die "MDS '$mds_id' already referenced in ceph config, abort!\n"; - } + # lock only the config update, creating the MDS does not touch ceph.conf + PVE::Cluster::cfs_lock_file( + 'ceph.conf', + undef, + sub { + my $cfg = cfs_read_file('ceph.conf'); - if (!defined($cfg->{mds}->{keyring})) { - # $id isn't a perl variable but a ceph metavariable - my $keyring = '/var/lib/ceph/mds/ceph-$id/keyring'; + if (defined($cfg->{$section})) { + die "MDS '$mds_id' already referenced in ceph config, abort!\n"; + } - $cfg->{mds}->{keyring} = $keyring; - } + if (!defined($cfg->{mds}->{keyring})) { + # $id isn't a perl variable but a ceph metavariable + my $keyring = '/var/lib/ceph/mds/ceph-$id/keyring'; - $cfg->{$section}->{host} = $nodename; + $cfg->{mds}->{keyring} = $keyring; + } - cfs_write_file('ceph.conf', $cfg); + $cfg->{$section}->{host} = $nodename; + + cfs_write_file('ceph.conf', $cfg); + }, + ); + die $@ if $@; eval { PVE::Ceph::Services::create_mds($mds_id, $rados) }; if (my $err = $@) { @@ -204,9 +212,20 @@ __PACKAGE__->register_method({ # wrote it at this point. Do not auto remove the service, could # do real harm for previously manual setup MDS warn "Encountered error, remove '$section' from ceph.conf\n"; - my $cfg = cfs_read_file('ceph.conf'); - delete $cfg->{$section}; - cfs_write_file('ceph.conf', $cfg); + PVE::Cluster::cfs_lock_file( + 'ceph.conf', + undef, + sub { + my $cfg = cfs_read_file('ceph.conf'); + # the lock was released in between, the section may be another node's now + my $host = $cfg->{$section} ? $cfg->{$section}->{host} : undef; + return if !defined($host) || $host ne $nodename; + delete $cfg->{$section}; + cfs_write_file('ceph.conf', $cfg); + }, + ); + warn "failed to remove '$section' from ceph.conf, remove it manually: $@" + if $@; die "$err\n"; } @@ -253,12 +272,19 @@ __PACKAGE__->register_method({ my $timeout = PVE::Ceph::Tools::get_config('long_rados_timeout'); my $rados = PVE::RADOS->new(timeout => $timeout); - my $cfg = cfs_read_file('ceph.conf'); + PVE::Cluster::cfs_lock_file( + 'ceph.conf', + undef, + sub { + my $cfg = cfs_read_file('ceph.conf'); - if (defined($cfg->{"mds.$mds_id"})) { - delete $cfg->{"mds.$mds_id"}; - cfs_write_file('ceph.conf', $cfg); - } + if (defined($cfg->{"mds.$mds_id"})) { + delete $cfg->{"mds.$mds_id"}; + cfs_write_file('ceph.conf', $cfg); + } + }, + ); + die $@ if $@; PVE::Ceph::Services::destroy_mds($mds_id, $rados); }; -- 2.47.3