mirror of
https://github.com/TheGrandWazoo/freenas-proxmox.git
synced 2026-10-21 11:16:10 +02:00
fix: return owner VMID and vtype from multipath path() (#291)
qm destroy calls path() and skips any disk whose owner != vmid. The
multipath plugin returned the LUN id (usually 0) as the owner and the
storeid as the vtype, so every truenas-multipath disk was silently left
behind on TrueNAS while the task reported OK.
Also make _naa_to_wwid return undef for a missing NAA: it returned a
truthy "3", which pointed path()/activate_volume at /dev/mapper/3 and
bypassed the #292 placeholder.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
(cherry picked from commit 147b461ea8)
This commit is contained in:
@@ -607,6 +607,21 @@ If you see iSCSI extents in TrueNAS that are not associated with any target, the
|
||||
perl -MPVE::Storage -e 'PVE::Storage::vdisk_free(PVE::Storage::config(), "STORAGE:vm-101-disk-0")'
|
||||
```
|
||||
|
||||
### `qm destroy` left multipath disks behind on TrueNAS
|
||||
|
||||
**Symptom:** you destroy a VM whose disks are on a `truenas-multipath` storage. The task reports `OK`, but the zvols, extents and the `proxmox-vm-<id>` target are still on TrueNAS. The journal has no `free_image` line for those disks.
|
||||
|
||||
**Cause:** in multipath versions up to and including 4.0.1, `path()` returned the LUN id where Proxmox expects the owning VMID. `qm destroy` skips any disk whose owner doesn't match, so it skipped all of them without a warning. **Fixed in v4.0.2** ([#291](https://github.com/TheGrandWazoo/freenas-proxmox/issues/291)). The base `truenas` storage type was never affected.
|
||||
|
||||
**Finding orphans:** list the storage's volumes and look for VMIDs that have no config:
|
||||
|
||||
```bash
|
||||
pvesm list <storage> # every vm-<id>-* volume on the storage
|
||||
ls /etc/pve/nodes/*/qemu-server/<id>.conf 2>/dev/null # no output → VM no longer exists
|
||||
```
|
||||
|
||||
To free an orphan, use `pvesm free <storage>:<vol>`. On 4.0.1 and earlier, use the `vdisk_free` one-liner from the entry above. Freeing a VM's last disk also removes its now-empty per-VM target.
|
||||
|
||||
### Filing a Bug Report
|
||||
|
||||
Please use the [GitHub issue tracker](https://github.com/TheGrandWazoo/truenas-proxmox/issues) and include the [bug report template](.github/ISSUE_TEMPLATE/bug_report.md). Include relevant log lines from `syslog` (search for `FreeNAS::`).
|
||||
|
||||
@@ -57,9 +57,11 @@ sub _portals {
|
||||
}
|
||||
|
||||
# Convert TrueNAS NAA string (e.g. "0x6589cfc...") to the dm-multipath WWID
|
||||
# used in /dev/mapper/ (e.g. "36589cfc...").
|
||||
# used in /dev/mapper/ (e.g. "36589cfc..."). Returns undef for a missing/empty
|
||||
# NAA — a bare "3" would be truthy and send callers to /dev/mapper/3.
|
||||
sub _naa_to_wwid {
|
||||
my ($naa) = @_;
|
||||
return unless defined $naa && length $naa;
|
||||
$naa =~ s/^0x//i;
|
||||
return "3$naa";
|
||||
}
|
||||
@@ -95,6 +97,11 @@ sub path {
|
||||
|
||||
PVE::Storage::Custom::TrueNAS::_resolve_token($storeid, $scfg);
|
||||
|
||||
# PVE's contract is ($path, $owner_vmid, $vtype). qm destroy skips any
|
||||
# disk whose owner != vmid, so returning the LUN id here silently
|
||||
# orphaned every multipath disk on destroy (#291).
|
||||
my ($vtype, undef, $ds_vmid) = $class->parse_volname($volname);
|
||||
|
||||
# Orphaned zvol (no extent / no NAA): return a placeholder with the real
|
||||
# owner and vtype so pvesm free / GUI Remove can still delete it (#292).
|
||||
# activate_volume dies on the same condition, so it is never opened.
|
||||
@@ -103,11 +110,10 @@ sub path {
|
||||
unless ($wwid) {
|
||||
PVE::Storage::Custom::TrueNAS::_log('warning',
|
||||
"multipath path: $volname has no iSCSI extent/WWID — returning placeholder path");
|
||||
my ($vtype, undef, $ds_vmid) = $class->parse_volname($volname);
|
||||
return ("/dev/mapper/truenas-unmapped-$volname", $ds_vmid, $vtype);
|
||||
}
|
||||
|
||||
return ("/dev/mapper/$wwid", $ext->{lun_id}, $storeid);
|
||||
return ("/dev/mapper/$wwid", $ds_vmid, $vtype);
|
||||
}
|
||||
|
||||
# ── qemu_blockdev_options — delegate to Plugin.pm (host_device for /dev/mapper) ─
|
||||
|
||||
@@ -66,6 +66,26 @@ subtest 'TrueNAS: qemu_blockdev_options still dies for extent-less zvol' => sub
|
||||
like($@, qr/has no iSCSI extent/, 'qemu_blockdev_options dies');
|
||||
};
|
||||
|
||||
# #291: qm destroy compares the owner to the VMID; the LUN id here made it skip every disk.
|
||||
subtest 'Multipath: mapped volume returns owner VMID and vtype (#291)' => sub {
|
||||
MockTrueNAS->install(%mapped);
|
||||
my ($path, $owner, $vtype) = $MP->path(MockTrueNAS::scfg(), 'vm-100-disk-0', 'store');
|
||||
is($path, '/dev/mapper/36589cfc000000abc', 'path is the dm-multipath device');
|
||||
is($owner, 100, 'owner is the VMID, not the LUN id');
|
||||
is($vtype, 'images', 'vtype, not the storeid');
|
||||
};
|
||||
|
||||
subtest 'Multipath: base image owner is the template VMID (#291)' => sub {
|
||||
MockTrueNAS->install(
|
||||
extents => [ { id => 4, name => 'base-9000-disk-0', naa => '0x6589cfc000000def' } ],
|
||||
targetextents => [ { id => 6, extent => 4, target => 8, lunid => 0 } ],
|
||||
targets => [ { id => 8, name => 'proxmox-vm-9000' } ],
|
||||
);
|
||||
my (undef, $owner, $vtype) = $MP->path(MockTrueNAS::scfg(), 'base-9000-disk-0', 'store');
|
||||
is($owner, 9000, 'owner');
|
||||
is($vtype, 'images', 'vtype');
|
||||
};
|
||||
|
||||
subtest 'Multipath: extent-less zvol returns placeholder, not die (#292)' => sub {
|
||||
MockTrueNAS->install(extents => []);
|
||||
my ($path, $owner, $vtype) = eval { $MP->path(MockTrueNAS::scfg(), 'vm-990-disk-0', 'store') };
|
||||
@@ -75,6 +95,17 @@ subtest 'Multipath: extent-less zvol returns placeholder, not die (#292)' => sub
|
||||
is($vtype, 'images', 'vtype');
|
||||
};
|
||||
|
||||
subtest 'Multipath: extent with no NAA returns placeholder, not /dev/mapper/3' => sub {
|
||||
MockTrueNAS->install(
|
||||
extents => [ { id => 3, name => 'vm-100-disk-0', naa => undef } ],
|
||||
targetextents => [ { id => 5, extent => 3, target => 7, lunid => 0 } ],
|
||||
targets => [ { id => 7, name => 'proxmox-vm-100' } ],
|
||||
);
|
||||
my ($path, $owner) = $MP->path(MockTrueNAS::scfg(), 'vm-100-disk-0', 'store');
|
||||
is($path, '/dev/mapper/truenas-unmapped-vm-100-disk-0', 'placeholder path');
|
||||
is($owner, 100, 'owner');
|
||||
};
|
||||
|
||||
subtest 'Multipath: activate_volume still dies for extent-less zvol' => sub {
|
||||
MockTrueNAS->install(extents => []);
|
||||
eval { $MP->activate_volume('store', MockTrueNAS::scfg(), 'vm-990-disk-0') };
|
||||
|
||||
Reference in New Issue
Block a user