all lists on lists.proxmox.com
 help / color / mirror / Atom feed
From: Fabian Ebner <f.ebner@proxmox.com>
To: pve-devel@lists.proxmox.com
Subject: [pve-devel] [PATCH storage 01/10] api: disks: create: re-check disk after fork/lock
Date: Tue, 28 Sep 2021 13:39:41 +0200	[thread overview]
Message-ID: <20210928114001.164081-2-f.ebner@proxmox.com> (raw)
In-Reply-To: <20210928114001.164081-1-f.ebner@proxmox.com>

Because then it might not be unused anymore. If there really is a
race, this prevents e.g. sgdisk creating a partition on a device
already in use by LVM or LVM destroying a partitioned device.

For ZFS, also get the latest udev info once inside the worker.

Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
---
 PVE/API2/Disks/Directory.pm |  2 ++
 PVE/API2/Disks/LVM.pm       |  2 ++
 PVE/API2/Disks/LVMThin.pm   |  2 ++
 PVE/API2/Disks/ZFS.pm       | 10 +++++++---
 4 files changed, 13 insertions(+), 3 deletions(-)

diff --git a/PVE/API2/Disks/Directory.pm b/PVE/API2/Disks/Directory.pm
index 2decb89..0068db6 100644
--- a/PVE/API2/Disks/Directory.pm
+++ b/PVE/API2/Disks/Directory.pm
@@ -212,6 +212,8 @@ __PACKAGE__->register_method ({
 	    my $mountunitpath = "/etc/systemd/system/$mountunitname";
 
 	    PVE::Diskmanage::locked_disk_action(sub {
+		PVE::Diskmanage::assert_disk_unused($dev);
+
 		# create partition
 		my $cmd = [$SGDISK, '-n1', '-t1:8300', $dev];
 		print "# ", join(' ', @$cmd), "\n";
diff --git a/PVE/API2/Disks/LVM.pm b/PVE/API2/Disks/LVM.pm
index 569e9d1..2c216c0 100644
--- a/PVE/API2/Disks/LVM.pm
+++ b/PVE/API2/Disks/LVM.pm
@@ -154,6 +154,8 @@ __PACKAGE__->register_method ({
 
 	my $worker = sub {
 	    PVE::Diskmanage::locked_disk_action(sub {
+		PVE::Diskmanage::assert_disk_unused($dev);
+
 		PVE::Storage::LVMPlugin::lvm_create_volume_group($dev, $name);
 
 		if ($param->{add_storage}) {
diff --git a/PVE/API2/Disks/LVMThin.pm b/PVE/API2/Disks/LVMThin.pm
index 4d303f8..81d91a6 100644
--- a/PVE/API2/Disks/LVMThin.pm
+++ b/PVE/API2/Disks/LVMThin.pm
@@ -108,6 +108,8 @@ __PACKAGE__->register_method ({
 
 	my $worker = sub {
 	    PVE::Diskmanage::locked_disk_action(sub {
+		PVE::Diskmanage::assert_disk_unused($dev);
+
 		PVE::Storage::LVMPlugin::lvm_create_volume_group($dev, $name);
 		my $pv = PVE::Storage::LVMPlugin::lvm_pv_info($dev);
 		# keep some free space just in case
diff --git a/PVE/API2/Disks/ZFS.pm b/PVE/API2/Disks/ZFS.pm
index 60077c4..885b93c 100644
--- a/PVE/API2/Disks/ZFS.pm
+++ b/PVE/API2/Disks/ZFS.pm
@@ -345,9 +345,6 @@ __PACKAGE__->register_method ({
 	foreach my $dev (@$devs) {
 	    $dev = PVE::Diskmanage::verify_blockdev_path($dev);
 	    PVE::Diskmanage::assert_disk_unused($dev);
-	    my $sysfsdev = $dev =~ s!^/dev/!/sys/block/!r;
-	    my $udevinfo = PVE::Diskmanage::get_udev_info($sysfsdev);
-	    $dev = $udevinfo->{by_id_link} if defined($udevinfo->{by_id_link});
 	}
 
 	PVE::Storage::assert_sid_unused($name) if $param->{add_storage};
@@ -374,6 +371,13 @@ __PACKAGE__->register_method ({
 
 	my $worker = sub {
 	    PVE::Diskmanage::locked_disk_action(sub {
+		for my $dev (@$devs) {
+		    PVE::Diskmanage::assert_disk_unused($dev);
+		    my $sysfsdev = $dev =~ s!^/dev/!/sys/block/!r;
+		    my $udevinfo = PVE::Diskmanage::get_udev_info($sysfsdev);
+		    $dev = $udevinfo->{by_id_link} if defined($udevinfo->{by_id_link});
+		}
+
 		# create zpool with desired raidlevel
 
 		my $cmd = [$ZPOOL, 'create', '-o', "ashift=$ashift", $name];
-- 
2.30.2





  reply	other threads:[~2021-09-28 11:40 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-09-28 11:39 [pve-devel] [PATCH-SERIES storage/widget-toolkit/manager] disk creation and wiping improvements Fabian Ebner
2021-09-28 11:39 ` Fabian Ebner [this message]
2021-09-30 16:06   ` [pve-devel] applied: [PATCH storage 01/10] api: disks: create: re-check disk after fork/lock Thomas Lamprecht
2021-09-28 11:39 ` [pve-devel] [PATCH storage 02/10] api: disk: work around udev bug to ensure its database is updated Fabian Ebner
2021-09-30 16:08   ` [pve-devel] applied: " Thomas Lamprecht
2021-09-28 11:39 ` [pve-devel] [PATCH storage 03/10] diskmanage: add change_parttype helper Fabian Ebner
2021-09-28 11:39 ` [pve-devel] [PATCH storage 04/10] diskmanage: wipe blockdev: also change partition type Fabian Ebner
2021-09-28 11:39 ` [pve-devel] [PATCH storage 05/10] diskmanage: don't set usage for unused partitions Fabian Ebner
2021-09-28 11:39 ` [pve-devel] [PATCH storage 06/10] api: disks: initgpt: explicitly abort for partitions Fabian Ebner
2021-09-30 16:02   ` Thomas Lamprecht
2021-10-06  7:11     ` Fabian Ebner
2021-09-28 11:39 ` [pve-devel] [PATCH storage 07/10] diskmanage: allow partitions for get_udev_info Fabian Ebner
2021-09-30 16:10   ` [pve-devel] applied: " Thomas Lamprecht
2021-09-28 11:39 ` [pve-devel] [PATCH storage 08/10] diskmanage: allow passing partitions to get_disks Fabian Ebner
2021-09-30 16:10   ` [pve-devel] applied: " Thomas Lamprecht
2021-09-28 11:39 ` [pve-devel] [PATCH storage 09/10] partially fix #2285: api: disks: allow partitions for creation paths Fabian Ebner
2021-09-28 11:39 ` [pve-devel] [PATCH storage 10/10] api: disks: create: set correct partition type Fabian Ebner
2021-09-28 11:39 ` [pve-devel] [PATCH widget-toolkit 1/2] (multi) disk selector: allow requesting partitions too Fabian Ebner
2021-09-30 16:13   ` [pve-devel] applied: " Thomas Lamprecht
2021-09-28 11:39 ` [pve-devel] [PATCH widget-toolkit 2/2] disk list: allow wiping individual partitions Fabian Ebner
2021-09-28 11:39 ` [pve-devel] [PATCH manager 1/9] api: ceph: create osd: re-check disk requirements after fork/lock Fabian Ebner
2021-09-30 16:12   ` [pve-devel] applied: " Thomas Lamprecht
2021-09-28 11:39 ` [pve-devel] [PATCH manager 2/9] api: check: create osd: use wipe_blockdev from the Diskmanage package Fabian Ebner
2021-09-30 16:12   ` [pve-devel] applied: " Thomas Lamprecht
2021-09-28 11:39 ` [pve-devel] [PATCH manager 3/9] api: ceph: create osd: work around udev bug Fabian Ebner
2021-09-30 16:12   ` [pve-devel] applied: " Thomas Lamprecht
2021-09-28 11:39 ` [pve-devel] [PATCH manager 4/9] api: ceph: create osd: set correct parttype for DB/WAL Fabian Ebner
2021-09-28 11:39 ` [pve-devel] [RFC manager 5/9] partially fix #2285: api: ceph: create osd: allow using partitions Fabian Ebner
2021-09-28 11:39 ` [pve-devel] [RFC manager 6/9] api: ceph: create osd: set correct partition type Fabian Ebner
2021-09-28 11:39 ` [pve-devel] [RFC manager 7/9] partially fix #2285: ui: ceph: allow selecting partitions Fabian Ebner
2021-09-28 11:40 ` [pve-devel] [PATCH manager 8/9] ui: zfs create: switch to using widget-toolkit's multiDiskSelector Fabian Ebner
2021-09-28 11:40 ` [pve-devel] [PATCH manager 9/9] partially fix #2285: ui: disk create: allow selecting partitions Fabian Ebner

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=20210928114001.164081-2-f.ebner@proxmox.com \
    --to=f.ebner@proxmox.com \
    --cc=pve-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