From ec4085b2449ff5135e37b028d1487bf910f34cb7 Mon Sep 17 00:00:00 2001 From: Kevin Adams Date: Wed, 20 May 2026 00:00:55 -0400 Subject: [PATCH] fix: full eval-based rollback on LUN creation failure, add test trigger (#239) Replace the ad-hoc if/rollback in run_create_lu with an eval block that tracks both extent_id and link_id so either resource is cleaned up if anything fails at any point during creation (#214 only covered the link-creation step). Also adds a FREENAS_TEST_ROLLBACK env-var trigger: set it in pvedaemon's environment to force a rollback after a successful create, allowing verification that orphaned extents and links are removed without needing a real failure condition. Co-Authored-By: Claude Sonnet 4.6 --- perl5/PVE/Storage/LunCmd/FreeNAS.pm | 28 ++++++++++++++++++---------- 1 file changed, 18 insertions(+), 10 deletions(-) diff --git a/perl5/PVE/Storage/LunCmd/FreeNAS.pm b/perl5/PVE/Storage/LunCmd/FreeNAS.pm index c440061..0eb1a32 100644 --- a/perl5/PVE/Storage/LunCmd/FreeNAS.pm +++ b/perl5/PVE/Storage/LunCmd/FreeNAS.pm @@ -261,17 +261,25 @@ sub run_create_lu { my $target_id = freenas_get_targetid($scfg); die "Unable to find the target id for $scfg->{target}" if !defined($target_id); - # Create the extent - my $extent = freenas_iscsi_create_extent($scfg, $lun_path); - die "Unable to create extent for $lun_path" if !defined($extent); + my ($extent_id, $link_id); + eval { + my $extent = freenas_iscsi_create_extent($scfg, $lun_path); + die "Unable to create extent for $lun_path" if !defined($extent); + $extent_id = $extent->{'id'}; - # Associate the new extent to the target; roll back the extent if this fails - # to avoid leaving a dangling extent on TrueNAS (issue #214) - my $link = freenas_iscsi_create_target_to_extent($scfg, $target_id, $extent->{'id'}, $lun_id); - if (!defined($link)) { - syslog("err", (caller(0))[3] . " : target-to-extent failed for $lun_path -- rolling back extent $extent->{'id'}"); - freenas_iscsi_remove_extent($scfg, $extent->{'id'}); - die "Unable to create lun $lun_path (extent rolled back)"; + my $link = freenas_iscsi_create_target_to_extent($scfg, $target_id, $extent_id, $lun_id); + die "Unable to link extent to target for $lun_path" if !defined($link); + $link_id = $link->{'id'}; + + # Set FREENAS_TEST_ROLLBACK=1 in pvedaemon environment to exercise + # rollback without a real failure (#239) + die "TEST: forced rollback for #239 verification" if $ENV{FREENAS_TEST_ROLLBACK}; + }; + if (my $err = $@) { + syslog("err", (caller(0))[3] . " : rolling back after error for $lun_path: $err"); + freenas_iscsi_remove_target_to_extent($scfg, $link_id) if defined $link_id; + freenas_iscsi_remove_extent($scfg, $extent_id) if defined $extent_id; + die "Unable to create lun $lun_path (rolled back): $err"; } syslog("info", (caller(0))[3] . "(lun_path=$lun_path, lun_id=$lun_id) : successful");