When a VM has multiple disks on the same per-VM target, migrating one
disk while the VM is running leaves the target in use for the other
disks. TrueNAS refuses to delete an extent associated with an in-use
target even with force=true.
Fix: delete the targetextent (LUN mapping) first, then delete the
extent. Removing the mapping severs this disk's association without
touching the active session or other LUNs on the same target.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Removed _vm_is_running() check from free_image. When a disk is detached
from a running VM (unused0), QEMU immediately closes its libiscsi
connection — there is no active session to block the delete. The
force=true flag on the TrueNAS extent DELETE is the correct guard for
any residual session. The VM-running check was overly broad and blocked
legitimate deletes of already-detached disks.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Each VM now gets its own TrueNAS iSCSI target (proxmox-vm-<vmid>)
instead of sharing a single storage-wide target. path() returns
iscsi://portal/iqn:proxmox-vm-<vmid>/lun — QEMU connects via libiscsi,
the same pattern ZFSPlugin.pm uses.
When a VM stops, QEMU closes its libiscsi connection. TrueNAS sees
no active session on that VM's target, so extent DELETE with force=true
succeeds without stopping the iSCSI service. Deleting one VM's disk
while other VMs are running on the same storage now works correctly.
Changes:
- Remove all iscsiadm session management (_iscsi_ensure_session,
_iscsi_session_exists, _wait_for_device, _dev_path,
_running_vms_on_storage, _resolve_target)
- Add _resolve_vm_target: find-or-create proxmox-vm-<vmid> target,
inherit portal/initiator groups from existing targets
- Add _maybe_cleanup_vm_target: delete empty VM target after last disk
- Add _vm_is_running: check owning VM before deletion (uses PID file)
- Add _api_global: cached iSCSI global config (basename)
- path(): returns iscsi://portal/per-vm-iqn/lun (target looked up by
target_id from extent, so legacy shared-target disks still work)
- activate_storage: API reachability check only
- activate_volume: verify extent exists, QEMU handles the connection
- deactivate_storage/volume: clear cache, return 1
- free_image: check owning VM stopped, force-DELETE extent, zvol,
cleanup empty target — no service-stop fallback needed
Closes#255
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
PVE runs pvedaemon with -T (taint mode). Data read from files is tainted
and cannot be passed to kill() without explicit untainting. Extract PID
via regex capture which Perl treats as safe.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Stopping the TrueNAS iSCSI service to clear session state (required on
CORE 13 when force=true is ignored) disrupts all LUNs on the target,
causing io-error on any other running VM using this storage.
Before triggering the service-stop path, scan /etc/pve/qemu-server/*.conf
for running VMs (PID file exists + process alive) that reference this
storage ID. If any are found, fail with a clear message naming the
blocking VMs rather than silently disrupting them.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
force=true on extent/targetextent DELETE is unconditionally ignored by
CORE 13.0 — it always checks for active sessions. POST /service/restart
is async and iscsid reconnects in milliseconds, so the restart window
was never clear enough for the DELETE to succeed.
New fallback sequence (only triggered when force=true fails):
1. Set iscsiadm node to manual startup — prevents auto-reconnect
2. iscsiadm --logout — drops initiator session
3. POST /service/stop + poll until service is stopped (up to 15s)
4. DELETE targetextent — succeeds with service fully stopped
5. DELETE extent
6. POST /service/start — restore service
7. Restore automatic startup + discovery + login + rescan
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
force=true on extent DELETE is not sufficient when CORE 13 has an active
or recovering session holding the targetextent (422 persists through retries).
Fallback path (triggered only when force fails and a targetextent exists):
1. POST /service/restart — purges all server-side session state
2. DELETE /iscsi/targetextent/id/{id} — now succeeds with service down
3. DELETE /iscsi/extent/id/{id} — clean delete with no association
Fast path (force=true) is tried first and handles the normal case without
any service disruption.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Session management was only needed to prevent 422 when explicitly
deleting the targetextent. Now that we delete the extent first with
force=true (which cascades targetextent removal server-side), the
initiator session can stay up throughout — same as the original
LunCmd/FreeNAS.pm delete path which never touched iscsiadm.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The original LunCmd/FreeNAS.pm (v2.0 path) deleted the extent with
{ force: true } and skipped the explicit targetextent DELETE entirely,
relying on TrueNAS to cascade it. Our v3.0 had the order backwards:
deleting the targetextent first always hit 422 "target in use" when
any iSCSI session was active.
New approach mirrors the original:
1. Log out initiator session
2. DELETE extent with { force: true } — TrueNAS cascades targetextent
3. If that fails (CORE 13 strict session enforcement), restart iSCSI
service and retry the extent DELETE (up to 5 times)
4. Restore session
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
volume_size_info: base class routes through filesystem_path() → get_subdir()
which dies on block storage. Override to query TrueNAS API directly for
volsize.parsed so create_efidisk and other callers get the correct zvol size.
free_image retry loop: TrueNAS CORE 13 refuses targetextent deletion while any
iSCSI session is active. Single logout+restart was racy — pvedaemon workers
reconnect between the restart and DELETE when multiple disks are deleted
concurrently (e.g. VM destroy). Retry up to 5 times with fresh logout+service
restart each cycle and exponential backoff.
Remove rootdir from plugindata content types: rootdir signals LXC
container/directory storage and caused PVE to route TPM state allocation to
TrueNAS, which immediately failed since TrueNAS has no filesystem path.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
parse_volname: implement for vm-/base- volume names so PVE can resolve
device paths and hotplug disks. Without this the base class falls through
to directory-volume parsing and fails with a 400 hotplug error.
free_image: TrueNAS CORE 13 holds iSCSI sessions in recovery state after
TCP disconnect, blocking targetextent deletion with 422 even seconds after
logout. Fix: log out the initiator session by SID, then restart the TrueNAS
iSCSI service to immediately clear server-side session state. Delete the
targetextent and extent, then restore the initiator session so other LUNs
on the same target remain accessible.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
After TrueNAS reloads its iSCSI service, the PVE host's existing session
does not automatically pick up new LUNs. Adding iscsiadm --rescan every 5s
during the device wait loop lets the kernel discover newly exported LUNs
without requiring a full session logout/login.
Fixes the 30s hotplug timeout seen when adding a disk to a running VM.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
TrueNAS CORE 13.0 /pool API does not expose top-level size/free/allocated
fields (they are nested in topology.data[].stats). Switch status() to query
/pool/dataset?id=<pool> which has available.parsed + used.parsed on both
CORE and SCALE.
Add shared=1 as the default in the UI panel — iSCSI is a network block
device accessible from all cluster nodes, so it should be shared storage
by default.
Verified on pve01-hq against Tank01 (CORE 13.0-U6.7):
TrueNAS01-Tank01 active 1804599296 165936464 1638662832 9.20%
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
status() was comparing the full dataset path (e.g. tank/proxmox/vdisks)
against TrueNAS /pool names which are top-level only (e.g. tank).
Extract the first path component so pool stats resolve correctly.
Rename Pool field label to 'Pool / Dataset Path' and update its hint
to make clear it accepts the full ZFS path (matching v2.x 'pool' field).
Rename Dataset to 'Sub-dataset' with a hint that discourages filling it
unless you genuinely need an extra sub-level beyond what's in Pool.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
TrueNAS CORE 13.0 does not expose /iscsi/targetgroup as a REST endpoint
(returns 404). Both CORE and SCALE include the portal group associations
inline in each target's 'groups' array from GET /iscsi/target, which is
all we need to filter targets by reachable portal IP.
Remove the /iscsi/targetgroup call entirely — use target.groups[].portal
cross-referenced against /iscsi/portal results instead.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
PVE's sensitive-properties mechanism extracts listed keys from $param
before check_config and passes them only to on_add_hook/on_update_hook.
activate_storage reads from $cfg which never receives those values, so
the API key was always missing at runtime.
The API key now lives in storage.cfg (root-readable, mode 0640, same as
the v2.x truenas_secret field). Proper on_add_hook private-file storage
is tracked in issue #247.
Restore truenas_api_key => {} (required on create). PVE's update flow
passes $create=0 to check_config which skips absent keys, so
edit-without-changing-key still works.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
truenas_api_key must be optional in options() because PVE extracts
sensitive-properties from the POST body before calling check_config,
causing a spurious 'missing required option' 500 on storage create.
Add a guard in _api() so a missing key produces a clear error.
UI: add autocomplete="url" on host and "new-password" on API key to
prevent Firefox/Chrome from filling in PVE login credentials.
Add a reveal trigger button to show/hide the API key field.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Remove the validate-patches job (no more patch files in v3.0).
Update lint to target TrueNAS.pm instead of FreeNAS.pm.
Rebuild staging assembly: TrueNAS.pm + truenas-storage.js are the
only payload — no patch dirs, no REST-Client.pm, no triggers file.
Add $VERSION = '3.0.0' to TrueNAS.pm as the single source of truth.
Add release/3.x as a testing-channel branch alongside master.
Pin actions to @v4 (upload/download-artifact, checkout).
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Package must match filename (TrueNAS.pm → PVE::Storage::Custom::TrueNAS)
so PVE's module loader finds the class when checking ISA PVE::Storage::Plugin.
api() returns 11 to match APIVER in PVE::Storage on PVE 8.4.x.
Returning 10 loaded successfully but triggered a deprecation warning.
Verified: pvedaemon loads plugin cleanly with no errors or warnings.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Full PVE::Storage::Custom implementation — zero patches to PVE system files.
Discovered automatically by Proxmox VE at runtime via Module::Load.
Implements:
- alloc_image: create zvol via TrueNAS API + wire iSCSI extent/targetextent
- free_image: tear down iSCSI association + delete zvol
- list_images: enumerate zvols under configured pool/dataset
- status: pool total/used/free via TrueNAS API (replaces SSH)
- path: resolve /dev/disk/by-path from LUN ID via API
- activate_storage: resolve target IQN + iscsiadm login
- deactivate_storage: iscsiadm logout + clear state cache
- activate_volume: ensure session + wait for block device
- volume_has_feature: declare copy and snapshot support
iSCSI target resolution supports both explicit IQN (truenas_target) and
auto-discovery from TrueNAS portal config — the field being set or blank
acts as the gate between modes.
No SSH keys required. Bearer token auth only. REST API v2.0.
Transport: CORE 13.x + SCALE <= 24.10. WebSocket (SCALE 25.04+) in v3.1.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Remove everything that existed solely to wedge into PVE system files:
- stable-5 through stable-8 versioned patch archives
- pve-manager/js patch files (no more pvemanagerlib.js patching)
- pve-docs/api-viewer patch files (no more apidoc.js patching)
- perl5/PVE/Storage/LunCmd/ (replaced by PVE::Storage::Custom plugin)
- perl5/PVE/Storage/ZFSPlugin-*.patch (no more ZFSPlugin.pm patching)
Add perl5/PVE/Storage/Custom/FreeNAS.pm as the v3.0 starting point.
v3.0 ships one .pm file; PVE discovers it automatically — zero patches.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Basic auth (username/password) has been supported since the plugin's
origin but TrueNAS API keys have been available since TrueNAS 12 (2020).
Sending credentials on every API call is a security risk and TrueNAS
may stop accepting it in future releases.
Log a syslog(warn) on every connection attempt that uses Basic Auth so
operators see the deprecation message in their Proxmox logs. The warning
links to issue #244 which tracks removal in v3.0.0.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* 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.
Kevin Adams
Kevin Scott Adams
Hans