[pve-devel] [PATCH storage 1/1] implement map_volume and unmap_volume

[Thomas Lamprecht]

On 11/8/18 2:05 PM, Alwin Antreich wrote:
> From: Dietmar Maurer <dietmar at proxmox.com>
> 
> This allows to request a mapped device/path explicitly, regardles of the
> storage option, eg. krbd option in the RBDplugin. Bump of the storage API => 2
> 
> Co-authored-by: Alwin Antreich <a.antreich at proxmox.com>
> Signed-off-by: Dietmar Maurer <dietmar at proxmox.com>

with slightly reformatted commit message: applied, thanks!

> ---
>  PVE/Storage.pm           | 26 +++++++++++++++++-
>  PVE/Storage/Plugin.pm    | 12 +++++++++
>  PVE/Storage/RBDPlugin.pm | 69 ++++++++++++++++++++++++++++++------------------
>  3 files changed, 80 insertions(+), 27 deletions(-)
> 
> diff --git a/PVE/Storage.pm b/PVE/Storage.pm
> index 3b92ee3..a5f7fdb 100755
> --- a/PVE/Storage.pm
> +++ b/PVE/Storage.pm
> @@ -37,7 +37,7 @@ use PVE::Storage::ZFSPlugin;
>  use PVE::Storage::DRBDPlugin;
>  
>  # Storage API version. Icrement it on changes in storage API interface.
> -use constant APIVER => 1;
> +use constant APIVER => 2;
>  
>  # load standard plugins
>  PVE::Storage::DirPlugin->register();
> @@ -671,6 +671,30 @@ sub vdisk_create_base {
>      });
>  }
>  
> +sub map_volume {
> +    my ($cfg, $volid, $snapname) = @_;
> +
> +    my ($storeid, $volname) = parse_volume_id($volid);
> +
> +    my $scfg = storage_config($cfg, $storeid);
> +
> +    my $plugin = PVE::Storage::Plugin->lookup($scfg->{type});
> +
> +    return $plugin->map_volume($storeid, $scfg, $volname, $snapname);
> +}
> +
> +sub unmap_volume {
> +    my ($cfg, $volid, $snapname) = @_;
> +
> +    my ($storeid, $volname) = parse_volume_id($volid);
> +
> +    my $scfg = storage_config($cfg, $storeid);
> +
> +    my $plugin = PVE::Storage::Plugin->lookup($scfg->{type});
> +
> +    return $plugin->unmap_volume($storeid, $scfg, $volname, $snapname);
> +}
> +
>  sub vdisk_alloc {
>      my ($cfg, $storeid, $vmid, $fmt, $name, $size) = @_;
>  
> diff --git a/PVE/Storage/Plugin.pm b/PVE/Storage/Plugin.pm
> index 8ae78e9..e0c2a4e 100644
> --- a/PVE/Storage/Plugin.pm
> +++ b/PVE/Storage/Plugin.pm
> @@ -949,6 +949,18 @@ sub deactivate_storage {
>      # do nothing by default
>  }
>  
> +sub map_volume {
> +    my ($class, $storeid, $scfg, $volname, $snapname) = @_;
> +
> +    return undef;
> +}
> +
> +sub unmap_volume {
> +    my ($class, $storeid, $scfg, $volname, $snapname) = @_;
> +
> +    return 1;
> +}
> +
>  sub activate_volume {
>      my ($class, $storeid, $scfg, $volname, $snapname, $cache) = @_;
>  
> diff --git a/PVE/Storage/RBDPlugin.pm b/PVE/Storage/RBDPlugin.pm
> index 41d89b5..c93071c 100644
> --- a/PVE/Storage/RBDPlugin.pm
> +++ b/PVE/Storage/RBDPlugin.pm
> @@ -74,8 +74,6 @@ my $librados_connect = sub {
>  my $krbd_feature_disable = sub {
>      my ($scfg, $storeid, $name) = @_;
>  
> -    return 1 if !$scfg->{krbd};
> -
>      my ($major, undef, undef, undef) = ceph_version();
>      return 1 if $major < 10;
>  
> @@ -259,7 +257,7 @@ sub properties {
>  	    type => 'string',
>  	},
>  	krbd => {
> -	    description => "Access rbd through krbd kernel module.",
> +	    description => "Always access rbd through krbd kernel module.",
>  	    type => 'boolean',
>  	},
>      };
> @@ -428,8 +426,6 @@ sub clone_image {
>  
>      run_rbd_command($cmd, errmsg => "rbd clone '$basename' error");
>  
> -    &$krbd_feature_disable($scfg, $storeid, $name);
> -
>      return $newvol;
>  }
>  
> @@ -445,8 +441,6 @@ sub alloc_image {
>      my $cmd = &$rbd_cmd($scfg, $storeid, 'create', '--image-format' , 2, '--size', int(($size+1023)/1024), $name);
>      run_rbd_command($cmd, errmsg => "rbd create $name' error");
>  
> -    &$krbd_feature_disable($scfg, $storeid, $name);
> -
>      return $name;
>  }
>  
> @@ -544,39 +538,62 @@ sub deactivate_storage {
>      return 1;
>  }
>  
> -sub activate_volume {
> -    my ($class, $storeid, $scfg, $volname, $snapname, $cache) = @_;
> +my $get_kernel_device_name = sub {
> +    my ($pool, $name) = @_;
> +
> +    return "/dev/rbd/$pool/$name";
> +};
>  
> -    return 1 if !$scfg->{krbd};
> +sub map_volume {
> +    my ($class, $storeid, $scfg, $volname, $snapname) = @_;
>  
>      my ($vtype, $name, $vmid) = $class->parse_volname($volname);
> +    $name .= '@'.$snapname if $snapname;
> +
>      my $pool =  $scfg->{pool} ? $scfg->{pool} : 'rbd';
>  
> -    my $path = "/dev/rbd/$pool/$name";
> -    $path .= '@'.$snapname if $snapname;
> -    return if -b $path;
> +    my $kerneldev = $get_kernel_device_name->($pool, $name);
> +
> +    return $kerneldev if -b $kerneldev; # already mapped
> +
> +    &$krbd_feature_disable($scfg, $storeid, $name);
>  
> -    $name .= '@'.$snapname if $snapname;
>      my $cmd = &$rbd_cmd($scfg, $storeid, 'map', $name);
> -    run_rbd_command($cmd, errmsg => "can't mount rbd volume $name");
> +    run_rbd_command($cmd, errmsg => "can't map rbd volume $name");
> +
> +    return $kerneldev;
> +}
> +
> +sub unmap_volume {
> +    my ($class, $storeid, $scfg, $volname, $snapname) = @_;
> +
> +    my ($vtype, $name, $vmid) = $class->parse_volname($volname);
> +    $name .= '@'.$snapname if $snapname;
> +
> +    my $pool =  $scfg->{pool} ? $scfg->{pool} : 'rbd';
> +
> +    my $kerneldev = $get_kernel_device_name->($pool, $name);
> +
> +    if (-b $kerneldev) {
> +	my $cmd = &$rbd_cmd($scfg, $storeid, 'unmap', $kerneldev);
> +	run_rbd_command($cmd, errmsg => "can't unmap rbd device $kerneldev");
> +    }
>  
>      return 1;
>  }
>  
> -sub deactivate_volume {
> +sub activate_volume {
>      my ($class, $storeid, $scfg, $volname, $snapname, $cache) = @_;
>  
> -    return 1 if !$scfg->{krbd};
> +    $class->map_volume($storeid, $scfg, $volname, $snapname) if $scfg->{krbd};
>  
> -    my ($vtype, $name, $vmid) = $class->parse_volname($volname);
> -    my $pool =  $scfg->{pool} ? $scfg->{pool} : 'rbd';
> +    return 1;
> +}
>  
> -    my $path = "/dev/rbd/$pool/$name";
> -    $path .= '@'.$snapname if $snapname;
> -    return if ! -b $path;
> +sub deactivate_volume {
> +    my ($class, $storeid, $scfg, $volname, $snapname, $cache) = @_;
>  
> -    my $cmd = &$rbd_cmd($scfg, $storeid, 'unmap', $path);
> -    run_rbd_command($cmd, errmsg => "can't unmap rbd volume $name");
> +    $class->unmap_volume($storeid, $scfg, $volname, $snapname);
>  
>      return 1;
>  }
> @@ -592,7 +609,7 @@ sub volume_size_info {
>  sub volume_resize {
>      my ($class, $scfg, $storeid, $volname, $size, $running) = @_;
>  
> -    return 1 if $running && !$scfg->{krbd};
> +    return 1 if $running && !$scfg->{krbd}; # FIXME???
>  
>      my ($vtype, $name, $vmid) = $class->parse_volname($volname);
>  
> @@ -623,7 +640,7 @@ sub volume_snapshot_rollback {
>  sub volume_snapshot_delete {
>      my ($class, $scfg, $storeid, $volname, $snap, $running) = @_;
>  
> -    return 1 if $running && !$scfg->{krbd};
> +    return 1 if $running && !$scfg->{krbd}; # FIXME: ????
>  
>      $class->deactivate_volume($storeid, $scfg, $volname, $snap, {});
>  
>