On 6/4/19 10:25 AM, Thomas Lamprecht wrote:
On 6/4/19 9:21 AM, Dominik Csapak wrote:
we will use this for adding a partition to a disk when using a device
for ceph osd db/wal which already has partitions on it

first we search for the highest partition number, then add the partition
and search for the resulting device (we cannot assume to simply
append the number, e.g. from /dev/nvme0n1 we get /dev/nvme0n1pX)

Signed-off-by: Dominik Csapak <d.csa...@proxmox.com>
---
  PVE/Diskmanage.pm | 36 ++++++++++++++++++++++++++++++++++++
  1 file changed, 36 insertions(+)

diff --git a/PVE/Diskmanage.pm b/PVE/Diskmanage.pm
index daad6df..645d67b 100644
--- a/PVE/Diskmanage.pm
+++ b/PVE/Diskmanage.pm
@@ -683,4 +683,40 @@ sub assert_disk_unused {
      return undef;
  }
+sub append_partition {
+    my ($dev, $size) = @_;
+
+    my $devname;
+    if ($dev =~ m|^/dev/(.*)$|) {
+       $devname = $1;
+    }

else ?? $devname can be undef and is used in string and regex below..
either fallback or die?

oops, yes this should be handled...


+
+    my $newpartid = 1;
+    dir_glob_foreach("/sys/block/$devname", qr/\Q$devname\E.+/, sub {
+       my ($part) = @_;
+
+       my ($partid) = $part =~ m/(\d+)$/;
+       $partid = int($partid);

could be undef, if no parition is present, not sure if you then can hit
this code path at all, but maybe still worth dealing with that..
And yes, I know that int(undef) == 0, but it produces also a warning
and is a bit to subtle for my taste..

mhmm, right, better would be to include the (\d+) in the glob regex directly, so that we have it already extracted as parameter
e.g. qr/\Q$devname\E.*?(\d+)/

?


+
+       if ($partid >= $newpartid) {
+           $newpartid = $partid + 1;
+       }
+    });
+
+    $size = PVE::Tools::convert_size($size, 'b' => 'mb');
+
+    run_command([ $SGDISK, '-n', "$newpartid:0:+${size}M", $dev ],
+               errmsg => "error creating partition '$newpartid' on '$dev'");
+
+    my $partition;
+
+    dir_glob_foreach("/sys/block/$devname", qr/\Q$devname\E.*$newpartid/, sub {
+       my ($part) = @_;
+
+       $partition = "/dev/$part";

this is a, in a certain way, elegant solution, maybe adding a small
comment could be good, something like:
# new part name is dependent of underlying device type, "autocomplete" it

sure will do


+    });
+
+    return $partition;
+}
+
  1;




_______________________________________________
pve-devel mailing list
pve-devel@pve.proxmox.com
https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

Reply via email to