[pve-devel] [PATCH v7 pve-storage 04/10] Basic storage operations.
Fabian Grünbichler
f.gruenbichler at proxmox.com
Mon Aug 7 14:32:55 CEST 2017
On Tue, Jun 20, 2017 at 10:39:56PM +0200, mir at datanom.net wrote:
> From: Michael Rasmussen <mir at datanom.net>
>
> Signed-off-by: Michael Rasmussen <mir at datanom.net>
> ---
> PVE/Storage/FreeNASPlugin.pm | 597 ++++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 589 insertions(+), 8 deletions(-)
>
> diff --git a/PVE/Storage/FreeNASPlugin.pm b/PVE/Storage/FreeNASPlugin.pm
> index e930cea..3a18a31 100644
> --- a/PVE/Storage/FreeNASPlugin.pm
> +++ b/PVE/Storage/FreeNASPlugin.pm
> @@ -17,11 +17,15 @@ use base qw(PVE::Storage::Plugin);
>
> my $api = '/api/v1.0';
> my $api_timeout = 20; # seconds
> +my $max_luns = 20; # maximum supported luns per target group in freenas is 25
> + # but reserve 5 for temporary LUNs (snapshots with RAM and
> + # snapshot backup)
> my $rows_per_request = 50; # limit for get requests
> # be aware. Setting limit very low (default setting
> # in FreeNAS API is 20) can cause race conditions
> # on the FreeNAS host (seems like an unstable
> # pagination algorithm implemented in FreeNAS)
> +my $active_snaps = 4;
whitespace
> my $version;
> my $target_prefix = 'iqn.2005-10.org.freenas.ctl'; # automatically prepended
> # in FreeNAS
> @@ -245,6 +249,389 @@ my $freenas_list_zvol = sub {
> return $list;
> };
>
whitespace
> +my $freenas_no_more_extents = sub {
isn't this misnamed? you are checking whether AN extent already exists
for a specific target, not whether there are "(no) more extents"?
> + my ($scfg, $target) = @_;
> +
whitespace
> + my $extents = $freenas_request->($scfg, 'GET', "services/iscsi/targettoextent/");
> + foreach my $extent (@$extents) {
> + return 0 if $extent->{iscsi_target} == $target;
> + }
> +
whitespace
> + return 1;
> +};
> +
> +my $freenas_get_target = sub {
> + my ($scfg, $vmid) = @_;
> + my $target = undef;
not needed, see below
> +
whitespace
> + my $targets = $freenas_request->($scfg, 'GET', "services/iscsi/target/");
> + foreach my $t (@$targets) {
> + if ($t->{iscsi_target_name} eq "vm-$vmid") {
> + $target = $t->{id};
> + last;
return $t->{id} if $t->{iscsi_target_name} eq "vm-$vmid";
> + }
> + }
> +
whitespace
> + return $target;
return undef (although I think this is technically not needed)
> +};
> +
> +my $freenas_create_target = sub {
> + my ($scfg, $vmid) = @_;
> + my ($data, $target);
> +
whitespace
> + $freenas_get_version->($scfg);
> +
> + if ($version < 110000) {
> + $data = {
> + iscsi_target_alias => "vm-$vmid",
> + iscsi_target_name => "vm-$vmid",
> + };
> + } elsif ($version < 110100) {
> + $data = {
> + iscsi_target_alias => "vm-$vmid",
> + iscsi_target_name => "vm-$vmid",
> + };
> + } else {
> + die "FreeNAS-$version: Unsupported!\n";
> + }
I don't get why you have this if here, but not in get_target..
> +
> + eval {
> + $target = $freenas_request->(
> + $scfg, 'POST', "services/iscsi/target/", encode_json($data));
> + };
> + if ($@) {
> + if ($@ =~ /^(\d+)\s*$/) {
> + # Fetch existing target if code is 409 (conflict)
> + die HTTP::Status::status_message($1) unless $1 == 409;
> + return $freenas_get_target->($scfg, $vmid);
> + }
IMHO this is ugly:
- unless
- make freenas_request have a list of 'allowed' error status codes, and
use the array variant here to get code and data.
> + die "Creating target for 'vm-$vmid' failed: $@\n";
> + }
> +
whitespace
> + return $target->{id};
> +};
> +
> +my $freenas_delete_target = sub {
> + my ($scfg, $target) = @_;
> +
> + $freenas_request->($scfg, 'DELETE', "services/iscsi/target/$target/");
> +};
> +
> +my $freenas_get_target_name = sub {
this does not make a freenas_request, so IMHO it should be named
differently. also, it seems this is included in patches #3 and #4?
> + my ($scfg, $volname) = @_;
> + my $name = undef;
> +
whitespace
> + if ($volname =~ /^(vm|base)-(\d+)-/) {
> + $name = "vm-$2";
> + return "$target_prefix\:$name";
> + }
> +
> + return undef;
> +};
> +
> +my $freenas_get_target_group = sub {
> + my ($scfg, $target) = @_;
> + my $targetgroup = undef;
> +
whitespace
> + my $targetgroups = $freenas_request->($scfg, 'GET', "services/iscsi/targetgroup/");
> +
> + foreach my $tgroup (@$targetgroups) {
> + if ($tgroup->{iscsi_target} == $target &&
> + $tgroup->{iscsi_target_portalgroup} == $scfg->{portal_group} &&
> + $tgroup->{iscsi_target_initiatorgroup} == $scfg->{initiator_group}) {
> + $targetgroup = $tgroup->{id};
> + last;
see above, drop the last, return from within the foreach, drop the
useless $targetgroup
> + }
> + }
> +
whitespace
> + return $targetgroup;
> +};
> +
> +my $freenas_create_target_group = sub {
> + my ($scfg, $target) = @_;
> + my $data;
> +
whitespace
> + $freenas_get_version->($scfg);
> +
> + # Trying to create a target group which already exists will cause and internal
> + # server error so if creating an existing target group should be allowed (return
> + # existing target group number we must search prior to create
> + my $tg = $freenas_get_target_group->($scfg, $target);
> + return $tg if $tg;
why do you have this here, but not for create_target, which has the same
semantics?
also, you overload $tg with two meanings:
- get_target_group returns the ID
- POST to services/iscsi/targetgroup returns the whole JSON response
I wonder whether explicitly marking the subs and variables only
containing IDs as such would prevent such confusions?
> +
whitespace
> + if ($version < 110000) {
> + $data = {
> + iscsi_target => $target,
> + iscsi_target_authgroup => $scfg->{auth_group} ? $scfg->{auth_group} : undef,
> + iscsi_target_portalgroup => $scfg->{portal_group},
> + iscsi_target_initiatorgroup => $scfg->{initiator_group},
> + iscsi_target_authtype => $scfg->{auth_type} ? $scfg->{auth_type} : 'None',
> + iscsi_target_initialdigest => "Auto",
> + };
> + } elsif ($version < 110100) {
> + $data = {
> + iscsi_target => $target,
> + iscsi_target_authgroup => $scfg->{auth_group} ? $scfg->{auth_group} : undef,
> + iscsi_target_portalgroup => $scfg->{portal_group},
> + iscsi_target_initiatorgroup => $scfg->{initiator_group},
> + iscsi_target_authtype => $scfg->{auth_type} ? $scfg->{auth_type} : 'None',
> + iscsi_target_initialdigest => "Auto",
> + };
> + } else {
> + die "FreeNAS-$version: Unsupported!\n";
> + }
same comment like for the if in create_target
> +
> + eval {
> + $tg = $freenas_request->(
> + $scfg, 'POST', "services/iscsi/targetgroup/", encode_json($data));
> + };
> + if ($@) {
> + if ($@ =~ /^(\d+)\s*$/) {
> + # Fetch existing target group if code is 409 (conflict)
> + die HTTP::Status::status_message($1)."\n" unless $1 == 409;
> + return $freenas_get_target_group->($scfg, $target);
> + }
and again, this is really convoluted - see create_target
> + die "Creating target group for target '$target' failed: $@\n";
> + }
> +
whitespace
> + return $tg->{id};
> +};
> +
> +my $freenas_delete_target_group = sub {
> + my ($scfg, $tg) = @_;
> +
> + $freenas_request->($scfg, 'DELETE', "services/iscsi/targetgroup/$tg");
> +};
> +
> +my $freenas_create_extent = sub {
> + my ($scfg, $zvol) = @_;
> + my $data;
> +
whitespace
> + $freenas_get_version->($scfg);
> +
whitespace
> + if ($version < 110000) {
> + $data = {
> + iscsi_target_extent_type => 'Disk',
> + iscsi_target_extent_name => $zvol,
> + iscsi_target_extent_disk => "zvol/$scfg->{pool}/$zvol",
> + };
> + } elsif ($version < 110100) {
> + $data = {
> + iscsi_target_extent_type => 'Disk',
> + iscsi_target_extent_name => $zvol,
> + iscsi_target_extent_disk => "zvol/$scfg->{pool}/$zvol",
> + };
> + } else {
> + die "FreeNAS-$version: Unsupported!\n";
> + }
again - no such if in the associated GET sub?
> +
> + my $extent = $freenas_request->(
> + $scfg, 'POST', "services/iscsi/extent/", encode_json($data));
> +
whitespace
> + return $extent->{id};
> +};
> +
> +my $freenas_delete_extent = sub {
> + my ($scfg, $extent) = @_;
> +
> + $freenas_request->($scfg, 'DELETE', "services/iscsi/extent/$extent/");
> +};
> +
> +my $freenas_get_extent = sub {
> + my ($scfg, $volname) = @_;
> + my $extent = undef;
> +
whitespace
> + my $extents = $freenas_request->($scfg, 'GET', "services/iscsi/extent/");
> + foreach my $ext (@$extents) {
> + if ($ext->{iscsi_target_extent_path} =~ /$scfg->{pool}\/$volname$/) {
> + $extent = $ext->{id};
> + last;
> + }
and the same comment again, drop the last, return here, ...
> + }
> +
whitespace
> + return $extent;
> +};
> +
> +my $freenas_create_target_to_exent = sub {
> + my ($scfg, $target, $extent, $lunid) = @_;
> + my $data;
> +
whitespace
> + $freenas_get_version->($scfg);
> +
whitespace
> + if ($version < 110000) {
> + $data = {
> + iscsi_target => $target,
> + iscsi_extent => $extent,
> + iscsi_lunid => $lunid,
> + };
> + } elsif ($version < 110100) {
> + $data = {
> + iscsi_target => $target,
> + iscsi_extent => $extent,
> + iscsi_lunid => $lunid,
> + };
> + } else {
> + die "FreeNAS-$version: Unsupported!\n";
> + }
and again, no such if in the GET sub?
> +
> + my $tg2extent = $freenas_request->(
> + $scfg, 'POST', "services/iscsi/targettoextent/", encode_json($data));
does this not have the same semantics as create_target and
create_target_group?
> +
whitespace
> + return $tg2extent->{id};
> +};
> +
> +my $freenas_delete_target_to_exent = sub {
> + my ($scfg, $tg2exent) = @_;
> +
> + $freenas_request->($scfg, 'DELETE', "services/iscsi/targettoextent/$tg2exent/");
> +};
> +
> +my $freenas_get_target_to_exent = sub {
> + my ($scfg, $extent, $target) = @_;
> + my $t2extent = undef;
> +
whitespace
> + my $t2extents = $freenas_request->($scfg, 'GET', "services/iscsi/targettoextent/");
> + foreach my $t2ext (@$t2extents) {
> + if ($t2ext->{iscsi_target} == $target && $t2ext->{iscsi_extent} == $extent) {
> + $t2extent = $t2ext->{id};
> + last;
> + }
again, drop the last, return here, ...
> + }
> +
whitespace
> + return $t2extent;
> +};
> +
> +my $freenas_find_free_diskname = sub {
> + my ($storeid, $scfg, $vmid, $format) = @_;
> +
> + my $name = undef;
> + my $volumes = $freenas_list_zvol->($scfg);
> +
> + my $disk_ids = {};
> + my $dat = $volumes->{$scfg->{pool}};
> +
> + foreach my $image (keys %$dat) {
> + my $volname = $dat->{$image}->{name};
> + if ($volname =~ m/vm-$vmid-disk-(\d+)/){
> + $disk_ids->{$1} = 1;
> + }
> + }
> +
> + for (my $i = 1; $i < $max_luns + 1; $i++) {
> + if (!$disk_ids->{$i}) {
> + return "vm-$vmid-disk-$i";
> + }
> + }
again, here you have the assumption that only numbers from 1 to $max_lun
are valid - I would really like to decouple the LUN numbering from the
disk index.
> +
whitespace
> + die "Maximum number of LUNs($max_luns) for this VM $vmid in storage '$storeid' is reached.\n";
> +};
> +
> +my $freenas_get_lun_number = sub {
> + my ($scfg, $volname) = @_;
> + my $lunid = undef;
> +
whitespace
> + if ($volname =~ /^(vm|base)-\d+-disk-(\d+)$/) {
> + $lunid = $2 - 1;
is there no other way? this seriously limits stuff that works with other
storage plugins, like custom disk names..
> + } elsif ($volname =~ /^vm-(\d+)-state/) {
> + # Find id for temporary LUN
> + my $target = $freenas_get_target->($scfg, $1);
> + my $id = $max_luns;
> + my $t2extents = $freenas_request->($scfg, 'GET', "services/iscsi/targettoextent/");
> +
> + foreach my $t2extent (@$t2extents) {
> + next unless $t2extent->{iscsi_target} == $target &&
> + $t2extent->{iscsi_lunid} + 1 > $max_luns &&
> + $t2extent->{iscsi_lunid} < $max_luns + $active_snaps;
> + my $eid = $freenas_get_extent->($scfg, $volname);
> + if ($eid) {
> + my $extent = $freenas_request->($scfg, 'GET', "services/iscsi/extent/$eid/");
> + # Request to get lunid for an existing lun
> + last if $t2extent->{iscsi_extent} eq $eid;
> + }
> + $id++;
> + }
> + die "Max snapshots ($active_snaps) is reached\n" unless ($id - $max_luns) < $active_snaps;
> + $lunid = $id;
> + } elsif ($volname =~ /^(vm|base)-\d+-disk-\d+\@vzdump$/) {
> + # Required to be able to exposed read-only LUNs for snapshot backup CT
> + $lunid = $max_luns + $active_snaps;
> + }
> +
whitespace
> + return $lunid;
> +};
> +
> +my $freenas_create_lun = sub {
> + my ($scfg, $vmid, $zvol) = @_;
> + my ($target, $tg, $extent, $tg2exent) = (undef, undef, undef, undef);
> +
whitespace
> + eval {
> + $target = $freenas_create_target->($scfg, $vmid);
> + die "create_lun-> Could not create target for VM '$vmid'\n" unless $target;
> + $tg = $freenas_create_target_group->($scfg, $target);
> + die "create_lun-> Could not create target group for VM '$vmid'\n" unless $tg;
> + $extent = $freenas_create_extent->($scfg, $zvol);
> + die "create_lun-> Could not create extent for VM '$vmid'\n" unless $extent;
> + my $lunid = $freenas_get_lun_number->($scfg, $zvol);
> + die "create_lun-> $zvol: Bad name format for VM '$vmid'\n" unless defined $lunid;
> + $tg2exent = $freenas_create_target_to_exent->($scfg, $target, $extent, $lunid);
> + die "create_lun-> Could not create target to extent for VM '$vmid'\n" unless defined $tg2exent;
> + };
the feedback I gave on this is also not incorporated (yet?)
> + if ($@) {
> + my $err = $@;
> + my $no_more_extents = 0;
> + if ($tg2exent) {
> + eval {
> + $freenas_delete_target_to_exent->($scfg, $tg2exent);
> + };
> + warn "Could not delete target to extent: $@\n" if $@;
> + }
> + if ($extent) {
> + eval {
> + $freenas_delete_extent->($scfg, $extent);
> + };
> + warn "Could not delete extent: $@\n" if $@;
> + }
> + eval {
> + $no_more_extents = $freenas_no_more_extents->($scfg, $target);
> + };
> + warn "Could not decide whether more extents exists: $@\n" if $@;
> + if ($target && $no_more_extents) {
> + if ($tg) {
> + eval {
> + $freenas_delete_target_group->($scfg, $tg);
> + };
> + warn "Could not delete target group: $@\n" if $@;
> + }
> + eval {
> + $freenas_delete_target->($scfg, $target);
> + };
> + warn "Could not delete target: $@\n" if $@;
> + }
> + die "create_lun: $err\n";
> + }
> +};
> +
> +my $freenas_create_zvol = sub {
> + my ($scfg, $volname, $size) = @_;
> +
whitespace
> + my $data = {
> + name => $volname,
> + volsize => $size,
> + };
> + my $zvol = $freenas_request->(
> + $scfg, 'POST', "storage/volume/$scfg->{pool}/zvols/", encode_json($data));
> +
> + die "$volname: Failed creating volume\n" unless $zvol && $zvol->{name};
> +
whitespace
> + return $zvol->{name};
> +};
> +
> +my $freenas_delete_zvol = sub {
> + my ($scfg, $volname) = @_;
> +
> + $freenas_request->($scfg, 'DELETE', "storage/volume/$scfg->{pool}/zvols/$volname/");
> +};
> +
> my $os_request = sub {
> my ($cmd, $noerr, $timeout) = @_;
>
> @@ -263,16 +650,49 @@ my $os_request = sub {
> return wantarray ? ($exit_code, $text) : $exit_code;
> };
>
> -my $freenas_get_target_name = sub {
see way above - I guess this was a mishap when using 'git add -p'?
> +my $disk_by_path = sub {
> my ($scfg, $volname) = @_;
> - my $name = undef;
> -
whitespace
> - if ($volname =~ /^(vm|base)-(\d+)-/) {
> - $name = "vm-$2";
> - return "$target_prefix\:$name";
> +
> + my $target = $freenas_get_target_name->($scfg, $volname);
> + my $lun = $freenas_get_lun_number->($scfg, $volname);
> + my $path = "/dev/disk/by-path/ip-$scfg->{portal}\:3260-iscsi-$target-lun-$lun";
> +
> + return $path;
> +};
> +
> +my $build_lun_list = sub {
> + my ($scfg, $sid, $lun) = @_;
> +
> + my $luns = {};
> + my $text = '';
> + my $exit = 0;
> +
> + eval {
> + ($exit, $text) = $os_request->(
> + ['iscsiadm', '-m', 'session', '-r', $sid, '-P3'], 1, 60);
> + };
> + if ($@) {
> + # An exist code of 22 means no active session otherwise an error
> + if ($exit != 22) {
> + die "$@\n";
> + }
again, this is not how run_command with $noerr works.
> + }
> + if ($text =~ /.*Host Number:\s*(\d+)\s+State:\s+running(.*)/s) {
> + my $host = $1;
> + my $found = 0;
> + for (split /^/, $2) {
> + if ($_ =~ /Channel\s+(\d+)\s+Id\s+(\d+)\s+Lun:\s+(\d+)/) {
> + if (defined $lun && $lun == $3) {
> + $luns = {};
> + $found = 1;
no need for $found and reseting $luns - simply return here?
> + }
> + $luns->{$3} = "$host:".int($1).":$2:$3";
> + last if $found;
> + }
> + }
why not iterate line by line and match each line? would make the code
easier to read. some comment about expected output would also be nice.
> }
>
whitespace
> - return undef;
> + return $luns;
> };
>
whitespace
> my $get_sid = sub {
> @@ -360,6 +780,59 @@ my $remove_local_lun = sub {
> }
> };
>
whitespace
> +my $deactivate_luns = sub {
> + # $luns contains a hash of luns to keep
> + my ($scfg, $volname, $luns) = @_;
> +
> + $luns = {} unless $luns;
unless
> + my $sid;
> + my $list = {};
> +
> + $sid = $get_sid->($scfg, $volname);
> +
> + $list = $build_lun_list->($scfg, $sid);
> +
> + foreach my $key (keys %$list) {
> + next if exists($luns->{$key});
s/exists/defined/
> + eval {
> + $remove_local_lun->($list->{$key});
> + };
> + warn "Remove local LUN '$list->{$key}' failed: $@\n" if $@;
$remove_local_lun already warns?
> + }
> +};
> +
> +my $get_active_luns = sub {
> + my ($class, $storeid, $scfg, $volname) = @_;
> +
> + my $sid = 0;
in other places, you use -1 to denote an uninitialized sid?
> + my $luns = {};
> +
> + $sid = $get_sid->($scfg, $volname);
> +
> + if ($sid < 0) {
> + # We have no active sessions so make one
> + $sid = $create_session->($scfg, $volname);
> + # Since no session existed prior to this call deactivate all LUN's found
> + $deactivate_luns->($scfg, $volname);
the sub is called get_active_luns, but if there is no active session
there are no active LUNs, so why create a session and deactivate all
LUNs here? this seems like a strange side-effect that belongs somewhere
else (or should be made explicit)
> + } else {
> + $luns = $build_lun_list->($scfg, $sid);
> + }
> +
> + return $luns;
> +};
> +
> +my $rescan_session = sub {
> + my ($class, $storeid, $scfg, $volname, $exclude_lun) = @_;
> +
> + my $luns_to_keep = $get_active_luns->($class, $storeid, $scfg, $volname);
> + delete $luns_to_keep->{$exclude_lun} if defined $exclude_lun;
> + my $sid = $get_sid->($scfg, $volname);
> + die "Missing session\n" if $sid < 0;
> + $os_request->(['iscsiadm', '-m', 'session', '-r', $sid, '-R'], 0, 60);
> + $deactivate_luns->($scfg, $volname, $luns_to_keep);
> + $delete_session->($scfg, $sid) unless %$luns_to_keep;
unless
> +};
> +
> # Storage implementation
>
> sub volume_size_info {
> @@ -453,6 +926,12 @@ sub path {
>
> my ($vtype, $vname, $vmid) = $class->parse_volname($volname);
>
whitespace
> + $vname = "$vname\@$snapname" if $snapname;
> +
> + my $luns = $get_active_luns->($class, $storeid, $scfg, $vname);
not used anywhere (or is this only because of the side-effect above!?)?
> + my $path = $disk_by_path->($scfg, $vname);
> +
whitespace
> + return ($path, $vmid, $vtype);
> }
>
> sub create_base {
> @@ -475,6 +954,28 @@ sub alloc_image {
> my ($class, $storeid, $scfg, $vmid, $fmt, $name, $size) = @_;
> die "unsupported format '$fmt'\n" if $fmt ne 'raw';
>
> + die "illegal name '$name' - sould be 'vm-$vmid-*'\n"
s/sould/should
> + if $name && $name !~ m/^vm-$vmid-/;
> +
> + $name = $freenas_find_free_diskname->($storeid, $scfg, $vmid, $fmt) unless $name;
unless
> +
whitespace
> + # Size is in KB but Freenas wants in bytes
> + $size *= 1024;
> + my $zvol = $freenas_create_zvol->($scfg, $name, $size);
> +
> + eval {
> + $freenas_create_lun->($scfg, $vmid, $zvol);
> + };
> + if ($@) {
> + my $err = $@;
> + eval {
> + $freenas_delete_zvol->($scfg, $name);
> + };
> + warn "Cleanup failed: $@\n" if $@;
> + die "$err\n";
> + }
> +
> + return $name;
> }
>
> sub free_image {
> @@ -482,6 +983,47 @@ sub free_image {
>
> my ($vtype, $name, $vmid, $basename) = $class->parse_volname($volname);
>
> + my $target = $freenas_get_target->($scfg, $vmid);
> + die "free_image-> missing target\n" unless $target;
> + my $extent = $freenas_get_extent->($scfg, $name);
> + die "free_image-> missing extent\n" unless $extent;
> + my $tg2exent = $freenas_get_target_to_exent->($scfg, $extent, $target);
> + die "free_image-> missing target to extent\n" unless $tg2exent;
> + my $target_group = $freenas_get_target_group->($scfg, $target);
> + die "free_image-> missing target group\n" unless $target_group;
> + my $lun = $freenas_get_lun_number->($scfg, $name);
> + die "free_image-> missing LUN\n" unless defined $lun;
> +
whitespace
> + eval {
> + my $res = $class->deactivate_volume($storeid, $scfg, $volname);
> + warn "Could not deactivate volume '$volname'\n" unless $res;
> + $freenas_delete_target_to_exent->($scfg, $tg2exent);
> + $freenas_delete_extent->($scfg, $extent);
> + if ($target && $freenas_no_more_extents->($scfg, $target)) {
> + if ($target_group) {
> + $freenas_delete_target_group->($scfg, $target_group);
> + }
> + $freenas_delete_target->($scfg, $target);
> + }
> + $freenas_delete_zvol->($scfg, $name);
> + $class->volume_snapshot_delete($scfg, $storeid, $basename, "__base__$vmid") if $basename;
> + if ($isBase) {
> + $basename = $name;
> + $basename =~ s/^base-/vm-/;
> + $class->volume_snapshot_delete($scfg, $storeid, $basename, '__base__') if $basename;
> + $freenas_delete_zvol->($scfg, $basename);
> + }
> + };
> + if ($@) {
> + my $err = $@;
> + eval {
> + $freenas_create_lun->($scfg, $vmid, $name) unless $isBase;
> + };
> + warn "Recreate LUN failed: $@\n" if $@;
> + die "$err\n";
> + }
> +
> + return undef;
> }
>
> sub volume_resize {
> @@ -579,6 +1121,35 @@ sub deactivate_storage {
> # deactivate all luns except our luns
> sub activate_volume {
> my ($class, $storeid, $scfg, $volname, $snapname, $cache) = @_;
> + my $lun;
> +
whitespace
> + my (undef, $name, $vmid) = $class->parse_volname($volname);
> +
whitespace
> + my $luns_to_keep = $get_active_luns->($class, $storeid, $scfg, $name);
> +
> + if ($snapname) {
> + eval {
> + $freenas_create_lun->($scfg, $vmid, "$name\@$snapname");
> + $lun = $freenas_get_lun_number->($scfg, "$name\@$snapname");
> + $luns_to_keep->{$lun} = "0:0:0:$lun";
> + };
> + if ($@) {
> + die "$@ - unable to activate snapshot from remote FreeNAS storage\n";
> + }
> + }
> +
whitespace
> + $lun = $freenas_get_lun_number->($scfg, $name);
> + $luns_to_keep->{$lun} = "0:0:0:$lun";
> +
> + my $sid = $get_sid->($scfg, $name);
> + die "activate_volume-> Missing session\n" if $sid < 0;
> + # Add new LUN's to session
> + $os_request->(['iscsiadm', '-m', 'session', '-r', $sid, '-R'], 0, 60);
> + $os_request->(
> + ['udevadm', 'trigger', '--type=devices', '--subsystem-match=scsi_disk'], 0, 60);
> + $os_request->(['udevadm', 'settle', '-t', $api_timeout], 0, 60);
> + # Remove all LUN's from session which is not currently active
> + $deactivate_luns->($scfg, $name, $luns_to_keep);
>
> return 1;
> }
> @@ -593,7 +1164,17 @@ sub deactivate_volume {
>
> my (undef, $name) = $class->parse_volname($volname);
>
> - return 1;
> + my $luns_to_keep = $get_active_luns->($class, $storeid, $scfg, $name);
> +
> + my $lun = $freenas_get_lun_number->($scfg, $name);
> + delete $luns_to_keep->{$lun};
> +
> + my $sid = $get_sid->($scfg, $name);
> + die "deactivate_volume-> Missing session\n" if $sid < 0;
> + $deactivate_luns->($scfg, $name, $luns_to_keep);
> + $delete_session->($scfg, $sid) unless %$luns_to_keep;
> +
> + return 1;
> }
>
> 1;
> --
> 2.11.0
>
>
> ----
>
> This mail was virus scanned and spam checked before delivery.
> This mail is also DKIM signed. See header dkim-signature.
>
> _______________________________________________
> pve-devel mailing list
> pve-devel at pve.proxmox.com
> https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
More information about the pve-devel
mailing list