Skip to content

Race condition#92

Open
tommymcguiver wants to merge 3 commits intokolesa-team:mainfrom
tommymcguiver:unmap-remove-patch-race
Open

Race condition#92
tommymcguiver wants to merge 3 commits intokolesa-team:mainfrom
tommymcguiver:unmap-remove-patch-race

Conversation

@tommymcguiver
Copy link
Copy Markdown

Summary

Fixes race condition in volume unmap/remove operations (#88) where iscsid rescans re-add device paths after removal but
before deactivation.

Changes

• Split device cleanup into offline and delete phases to prevent udev re-adding devices.
• Added deactivation callback mechanism to unmap_volume for proper sequencing.
• Updated deactivate_volume to disconnect volume after offlining devices.
• Improved error handling in device operations.

Testing Plan

• Verify volume unmap/remove operations complete without device path failures.
• Test with multipath-enabled volumes.
• Confirm no "Logical unit not supported" errors in logs.

Ken Miles added 2 commits February 26, 2026 09:59
Separate device offline and delete operations to prevent udev from re-adding devices during cleanup. Add deactivation
callback mechanism to ensure proper sequencing between volume disconnection and device node removal. Improve error
handling in device operations.
$logger->( P_WARN, "Kernel rejected '$value' for $path (EINVAL). Device may be busy, mounted, or driver doesn't support this transition.", undef );
return;
}
$logger->( P_WARN, "Critical failure writing to $path: $err", undef );
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

close( $ fh ) could be called twice.


sub unmap_volume {
my ( $class, $storeid, $scfg, $volname, $snapname ) = @_;
my ( $class, $storeid, $scfg, $volname, $deactivate_cb ) = @_;
Copy link
Copy Markdown
Collaborator

@amulet1 amulet1 Feb 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unmap_volume (and few other plugin methods) is called by PVE::Storage:

sub unmap_volume {
    my ($cfg, $volid, $snapname) = @_;

    my ($storeid, $volname) = parse_volume_id($volid);

    my $scfg = storage_config($cfg, $storeid);

    my $plugin = PVE::Storage::Plugin->lookup($scfg->{type});

    return $plugin->unmap_volume($storeid, $scfg, $volname, $snapname);
}

You should not change its signature.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants