* feat: auto-detect and correct ZFS blocksize for TrueNAS SCALE/CORE (#241)
SCALE requires >= 16k volblocksize; CORE works with 8k. Without correction,
creating a disk on SCALE with blocksize=8k triggers a ZFS warning and wastes
space on SCALE's minimum-4k-block pool layout.
Three changes:
- freenas_get_recommended_blocksize: fixes freenas_api_connect → freenas_api_check
so product_name is actually populated before the SCALE check (was always returning
8192 before this fix)
- freenas_parse_blocksize: new helper to compare "8k"/"16k"/integer blocksize strings
- alloc_image (both 8.x and 8.4.x patches): always detect recommended blocksize for
freenas provider; if configured < recommended, override for this call AND persist
the correction back to storage.cfg via lock_storage_config
- on_add_hook (8.4.x patch only): detect at storage creation time and correct $scfg
before write_config saves it — no extra write needed
Tested on pve01-hq (PVE 8.4.19) against Tank02 (SCALE 24.10.2.1):
- Disk created with blocksize=8k → task log shows correction message, storage.cfg
updated to 16384, no volblocksize warning from TrueNAS
- Disk created with blocksize=16384 → no-op, clean TASK OK
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* ci: add validate-patches step for ZFSPlugin PVE 8.4.x patch
ZFSPlugin-8.4.14_1.pm.patch has been in the build since v2.3.0 but was
never dry-run validated in CI. Now that we have the 8.4 orig committed
(ZFSPlugin-8.4.14_1.pm.orig from libpve-storage-perl 8.3.8), wire it up.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* fix: replace explicit return undef with bare return (perlcritic)
Subroutines::ProhibitExplicitReturnUndef violation in
freenas_get_recommended_blocksize. Bare return in list context returns
an empty list rather than a list containing undef, which is the
correct Perl idiom.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
---------
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
ZFSPlugin's free_image deletes the zvol via SSH after calling our
run_delete_lu. Our earlier addition of freenas_delete_zvol in
run_delete_lu caused a double-delete: SSH would fail, free_image's
error recovery would call run_create_lu on the now-missing zvol,
producing a spurious "Unable to create lun (rolled back)" in the task
log even though the migration had already succeeded.
freenas_delete_zvol remains in run_create_lu's rollback path where it
correctly cleans up zvols orphaned by a failed LUN creation (#239).
Also documents ZFS blocksize requirements for TrueNAS SCALE (16k) vs
CORE (8k) in README configuration and troubleshooting sections (#241).
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
remove:true in the iSCSI extent DELETE body is unreliable on TrueNAS
SCALE 24.10+ — the extent config is removed but the underlying zvol is
left behind as an orphaned dataset. Add freenas_delete_zvol() which calls
DELETE /api/v2.0/pool/dataset/id/{path}/ explicitly after every extent
removal, covering both the rollback path in run_create_lu and normal
disk deletion in run_delete_lu. Guarded to v2.0 API only using the
per-host cache to avoid stale package-global state when two storage
hosts with different API versions are active simultaneously.
Verified on TrueNAS SCALE 24.10.2.1: zvol is fully removed from
Datasets after a forced rollback test (FREENAS_TEST_ROLLBACK=1).
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
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 <noreply@anthropic.com>
Two bugs caused clone/migrate to fail with 'setHost on undefined value'
followed by 'Loop recursion prevention':
1. freenas_api_check reads $freenas_server_list->{$apihost}{client} to
check if initialized. Perl autovivifies {$apihost} as an empty {} on
that read. freenas_api_connect then saw a defined (but empty) hash and
skipped initialization, leaving client undef → setHost crash.
Fix: guard also checks !defined ...{client}.
2. $ping was a local variable reset to v1.0 on each recursive call.
TrueNAS 25.04+ removed the v1.0 endpoint (returns 404), so the
v1.0→v2.0 upgrade branch looped until the runaway counter tripped.
Fix: accept $ping as optional second parameter (defaults to v1.0)
and pass it through on both recursive call sites.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
RequireUseStrict and RequireUseWarnings require that strict and warnings
are the first statements after the package declaration. Move our \$VERSION
to after the two use pragmas.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
our \$VERSION = '2.3.0' is now the single source of truth for the release
series. The build.yml extracts it with a perl one-liner (no module load,
no stubs needed) and uses it as BASE_VERSION for alpha/beta channel builds.
Git tags remain the release trigger: pushing an exact vX.Y.Z tag publishes
to the stable Cloudsmith channel. A mismatch warning fires if the tag
version and \$VERSION disagree, catching forgotten version bumps.
The v2.3.0-pre anchor tag approach is superseded by this; the anchor tag
can stay in history as a breadcrumb but \$VERSION is now what drives builds.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Two problems in run_create_lu:
1. If freenas_iscsi_create_extent returned undef, the next line accessed
$extent->{'id'} unconditionally, causing a crash rather than a clean error.
2. If freenas_iscsi_create_target_to_extent failed after a successful extent
creation, the code just died with "Unable to create lun", leaving the
newly created extent orphaned on TrueNAS with no target association.
Subsequent delete_lu calls could not find or clean up this extent because
it was never in the LUN list, requiring manual TrueNAS cleanup.
Fix: check extent creation result before dereferencing; on target-to-extent
failure, call freenas_iscsi_remove_extent to roll back the extent before
dying. The rollback is best-effort (failure is logged but not re-thrown)
so a secondary API error does not mask the primary failure message.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
All of these were module-level scalars that the last freenas_api_check call
would overwrite. With two TrueNAS hosts in a Proxmox config (e.g., one CORE
and one SCALE), the second host's API check would clobber the first host's
product_name, dev_prefix, and api_version, causing wrong API paths and wrong
pool-name slash-vs-dash conversion on subsequent calls to the first host.
Changes:
- $freenas_server_list->{$host} now stores a hashref {client, product_name,
api_version, dev_prefix, runaway_count} instead of a bare REST::Client
- freenas_api_connect: recursion counter is now per-host (runaway_count);
$apiping made local so v1->v2 detection for one host does not affect others
- freenas_api_check: fixed broken init-guard (was testing $freenas_rest_connection->{$host}
which always evaluates undef on a REST::Client object); saves detected
product_name/api_version/dev_prefix back into the per-host hash after detection
- freenas_api_call: restores all module-level pointers (product_name, dev_prefix,
api_methods, api_variables) from per-host storage on every call, so the
correct host context is active regardless of which host was accessed last
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
'! $method =~ /pattern/' is parsed as '(! $method) =~ /pattern/' due to
precedence, so the regex ran against "" and always returned false. The
guard never fired and any method string passed through to the API call.
Changed to '$method !~ /^(?:GET|DELETE|POST)$/' which correctly tests the
method string. Also replaced atomic group (?>...) with non-capturing (?:...)
since there is no backtracking concern here.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
TrueNAS 25.04 removed the v1.0 REST API entirely. Previously it redirected
(302) to v2.0; now it returns 404. The plugin treated 404 as a fatal
connection failure, preventing any operation on 25.04 hosts.
Fix: extend the v1.0→v2.0 upgrade condition in freenas_api_connect to also
trigger on HTTP 404, but only while probing the v1.0 endpoint (guarded by
`$apiping =~ /v1\.0/`). A 404 on the v2.0 endpoint still fails cleanly.
Also add a syslog warn when Bearer Token auth is attempted over plain HTTP:
TrueNAS 25.04+ revokes API keys sent without SSL, causing auth failures that
are otherwise invisible in the plugin log. The warning points admins to the
"Use SSL" storage config option.
Closes#205
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
freenas_iscsi_create_extent and freenas_iscsi_create_target_to_extent
both used `eval $value` to expand template variable names like '$name'
into their runtime values. Replace with a local %vars hash and an
`exists` lookup — same behavior, no stringy eval, no perlcritic warning.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
ProhibitExplicitReturnUndef (severity 5): returning undef explicitly is
wrong because it forces scalar context on callers. 'return;' returns
undef in scalar context and an empty list in list context, which is the
correct idiom. Fixed all 7 occurrences in FreeNAS.pm.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- When SSL redirect is enabled on TrueNAS and a change from V1 to V2
api's needs to happen then we overrun the limit of 1 on the recursion
loop. Set it to 2.
- Added some logging for TrueNAS-Scale conversion from slash (/) to dash
(-).
- When changing from slashes (/) to dashes (-), I did not provide the
global parameter so a Storage/Tank/Disks would turn out to be
Storage-Tank/Disks causing a iSCSI: Failed to connect to LUN :
iscsi_service failed with : iscsi_service_reconnect_if_loggedin. Can not
reconnect right now.
- Created conditional to parse new TrueNAS 13 version.
- Created conditional to check for a 200 and that the Content-Type is
text/html so it will change to v2.0 of the API's. This is new behavior
in TrueNAS-Core 13.
- Add the true path to the debug statement when comparing targets.
- Add patch set for ZFSPlugin.pm for Storage lib 7.1-1
- Add patch set for pvemanagerlib.js for pve-manager version 7.1-11
- Add patch set for apidoc.js for pve-docs version 7.1-2.
- Create stable-7 directory for future installs based on Proxmox major
version each modules version.
- The eval{} block was originally returning a value that would allow
execution in the $@ conditional. This changed between Proxmox 6 and 7.
Why, no idea at the moment. Also, the original HASH was no longer valid
as a variable. I don't like the code block at this moment, I need to do
some regression testing to Proxmox 5 and FreeNAS/TrueNAS 11 and 12 to
see if I can clean up the conditional block. It was there for a reason I
just don't know at this moment.
- Added a '}' at line 515.
- Fixes issues reported in #98 abd #75.
- freenas-proxmox-packer also fixed. The stable build directory somehow
was moved to the development building directory.
- Adjusted Regex to parse the current TrueNAS-SCALE product and
versioning. Also changed the FreeNAS and TrueNAS so releases will be a
variable (e.g. - U, BETA, RC, MASTER)
- Used the $product_name to change use either a slash (/) for
FreeNAS/TrueNAS-CORE and a dash (-) for FreeNAS-SCALE.
- Under a certain condition, the variable $freenas_rest_connection would
not be defined resulting in a perl ‘blessed’ error. Added logic to see
if a variable is being passed in the @_ and use it, else use
$freenas_rest_connection.
- Use caller (1)[3] instead of passing the function name through the @_.
- Just addign a 'last' statement once an extent is found. No since
looking for more after the extent is found because there is no more.
Should make plugin run a bit faster.
- When cleaning up code from last commit, I removed a line from the
run_lun_command that initialize the code thinking that the way it would
work would be throught the freenas_api_check when calling the
freenas_api_call. Well certain procedures from the Proxmox ZFSPlugin
call procedures that do not have the $freenas_api_methods variable set
yet because freenas_api_check has NOT been called yet. Inserted the
freenas_api_check() in the run_lun_command() to get the variable
initialized. Fixes#62 and other tests that I found to have this issue.
freenas-proxmox-2.0.0-0beta4 will also have the fix.
- When optimizing the code to remove the redundant calls to create a
'new' REST::Client this had a single variable to hold the instance. If
you were moving between two different servers then the variables would
not match the destination parameters of the [Free|True]NAS server.
Implemented a hashref of the construct of REST:Client with the hashref
being the IP address of the portal or API IP. Fixes#61.
- Remove some white spacing and superfluous code.
- Added some comments so I know what I was changing.
other changes.
- Add logic to freenas_api_connect to handle a '302 Found' which seems
to mean it does not handle v1.0 API's. Fixes#60.
- Add logic to freenas_api_connect to handle a '307 Temporary Redirect'
which is when the redirect http to https box is checked. Fixes#59.
- Redesign the RegEx that validates and pulls the version information
from the API system/version call. This is due to a format like
TrueNAS-12.0-BETA instead of [FreeNAS|TrueNAS]-12.0-U1.1. Also make sure
empty RegEx return variables are zero (0) so the version number that is
formulated is correct (e.g. 12000000, 11030302). Fixes#60.
- During testing needed to add a check due to the
api/v2.0/system/version returning a quoted string where
api/v1.0/system/version returned a JSON-format data.
- Tested against baremetal FreeNAS 11.3-U3.2 and virtual TrueNAS
12.0-BETA machines.
- Cleaned up the API calling routine so it will only create one instance
of the REST::Client module. Typically was 10 to 30+ instances per VM
disk CRUD.
- Due to this some variables needed to be “global” to the plugin.
- Removed some superfluous hashes and variables.
- Repetitive code reduced to only on call.
- Created one “Frankenstein” hash that retains all API methods,
variables and json body structures for each API version used by the
plugin.
- One call to the global configuration in the version check to reduce
execution time.
- Cleanup some code and formatting.
- Added some comments.
- Allows the use of FreeNAS API v2.0 so we can finally remove a VM drive
from the Proxmox VE GUI.
- Created a Frankenstein of out of FreeNAS.pm to allow 'one module to
rule them all' for v1.0 and v2.0 API's to FreeNAS.
- librest-client-perl changes so the call can have a <body/> to it on
GET and DELETE methods.
- Some spelling mistakes.
- Better syslog entries.
- More debbugging for testing purposes to the syslog.
- Removed code in FreeNAS.PM that used the blocksize field from the GUI
for the use of iSCSI block size causing migration and large extent
sizes. Fixes issues #9 and #33.
- Prefixed 'ZFS' to the 'Block Size' label on the GUI.
- Updated for 5.3-5
- This change allows the API traffic to be on a different interface then
the iSCSI Provider interface. This allows the selection of the FreeNAS
WebGUI address for HTTP/HTTPS to be on a specific interface instead of
"ALL" (0.0.0.0) interfaces. If the API Host is not defined then the API
traffic will default to the iSCSI Provider interface.
- Add a simple API test using the FreeNAS API call to get its version
number, which could prove useful in future versions. Closes#17
- Whitespace cleanup
- Whitespace cleanup
- Removed the 'hardcoded' username and password. Will not throw an error
if it is not defined or blank in /etc/pve/storage.cfg for each ZFS via
ISCSI definition.
- Used a Ternary (Elvis - ?:) Operator for the scheme variable.
- The perl code was creating a new variable in the ‘if’ code block by
using 'my' at the beginning of the assignment at line 452. Once the code
exited this block that variable was released and now defaulted to the
main block variable at line 441 which is still set to 'undef'. Removing
the 'my' from the assignment line allowed the code to use the main block
$iscsi_lunid variable.
Fixes issue #7
- So we do not have to change the /usr/bin/pvedaemon a small fix to
check the $item->{'iscsi_target'} varaible from the API call is needed.
This closes issue #3.
- In FreeNAS 11.1 it seems like a Lun of 0 becomes 'Auto'. When the API
returns the targent_to_extent data any Lun of "Auto" become 'undef' and
causes issues and errors in the syslog. This change corrects this
behavior. Have tested with adding, detaching, removing and "Moving Disk"
functions of Proxmox version 5.1-51.
Kevin Scott Adams
Kevin Adams
Hans
Hans
KevinAdams
Tristan Lostroh
Krzysztof Raczkowski