[pve-devel] [PATCH storage 3/5] Diskmanage: add append_partition sub

[Thomas Lamprecht]

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.csapak at 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?

> +
> +    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..

> +
> +	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

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