proxmox-mirrors/qemu-server
1
0
Fork 0
mirror of https://git.proxmox.com/git/qemu-server synced 2025-08-14 15:36:06 +00:00
Code Issues Actions Packages Projects Releases Wiki Activity

pci reservation: rework helpers style and readability wise

Browse Source 
both style and readability are naturally subjective to a certain
degree...

Also, this patch mixes a bit much into one thing, but splitting that
up would mean lots of work I just wanted to avoid, sorry about that.

Among other things:

- avoid a level of indentation in the reserve loop
- rename pciids to reservation_list where it was a better fit
- make reserve set either pid or time to avoid suggesting that we
  save both
- rename parameters to requested/dropped IDs for easier understanding
  what's going on in the code
- avoid old_pid/pid, use running_pid and reserver_pid instead to
  clarify what they actually mean
- drop useless returns to avoid suggesting the return value has any
  use and save some lnes
- use a hash slice to delete all dropped IDs at once, shorter and
  faster
- use 5 second timeout for reservation, this does nothing intensive
  nor does it wait for anything, so the critical section should be
  really short, 5s is really long enough for a wait..

Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
This commit is contained in:
Thomas Lamprecht 2021-10-15 18:08:22 +02:00
parent bda0ebff2d
commit a01593676c
1 changed files with 37 additions and 52 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

87
PVE/QemuServer/PCI.pm
View File

@ -491,7 +491,6 @@ my $PCIID_RESERVATION_LOCK = "${PCIID_RESERVATION_FILE}.lock";
my $parse_pci_reservation_unlocked = sub { my $parse_pci_reservation_unlocked = sub {
my $pciids = {}; my $pciids = {};
if (my $fh = IO::File->new($PCIID_RESERVATION_FILE, "r")) { if (my $fh = IO::File->new($PCIID_RESERVATION_FILE, "r")) {
while (my $line = <$fh>) { while (my $line = <$fh>) {
if ($line =~ m/^($PCIRE)\s(\d+)\s(time|pid)\:(\d+)$/) { if ($line =~ m/^($PCIRE)\s(\d+)\s(time|pid)\:(\d+)$/) {
@ -502,88 +501,74 @@ my $parse_pci_reservation_unlocked = sub {
} }
} }
} }
return $pciids; return $pciids;
}; };
my $write_pci_reservation_unlocked = sub { my $write_pci_reservation_unlocked = sub {
my ($pciids) = @_; my ($reservations) = @_;
my $data = ""; my $data = "";
foreach my $p (keys %$pciids) { for my $pci_id (sort keys $reservations->%*) {
my $entry = $pciids->{$p}; my ($vmid, $pid, $time) = $reservations->{$pci_id}->@{'vmid', 'pid', 'time'};
if (defined($entry->{pid})) { if (defined($pid)) {
$data .= "$p $entry->{vmid} pid:$entry->{pid}\n"; $data .= "$pci_id $vmid pid:$pid\n";
} else { } else {
$data .= "$p $entry->{vmid} time:$entry->{time}\n"; $data .= "$pci_id $vmid time:$time\n";
} }
} }
PVE::Tools::file_set_contents($PCIID_RESERVATION_FILE, $data); PVE::Tools::file_set_contents($PCIID_RESERVATION_FILE, $data);
}; };
sub remove_pci_reservation { sub remove_pci_reservation {
my ($ids) = @_; my ($dropped_ids) = @_;
return if !scalar(@$ids); # do nothing for empty list $dropped_ids = [ $dropped_ids ] if !ref($dropped_ids);
return if !scalar(@$dropped_ids); # do nothing for empty list
my $code = sub { PVE::Tools::lock_file($PCIID_RESERVATION_LOCK, 2, sub {
my $pciids = $parse_pci_reservation_unlocked->(); my $reservation_list = $parse_pci_reservation_unlocked->();
delete $reservation_list->@{$dropped_ids->@*};
for my $id (@$ids) { $write_pci_reservation_unlocked->($reservation_list);
delete $pciids->{$id}; });
}
$write_pci_reservation_unlocked->($pciids);
};
PVE::Tools::lock_file($PCIID_RESERVATION_LOCK, 10, $code);
die $@ if $@; die $@ if $@;
return;
} }
sub reserve_pci_usage { sub reserve_pci_usage {
my ($ids, $vmid, $timeout, $pid) = @_; my ($requested_ids, $vmid, $timeout, $pid) = @_;
return if !scalar(@$ids); # do nothing for empty list $requested_ids = [ $requested_ids ] if !ref($requested_ids);
return if !scalar(@$requested_ids); # do nothing for empty list
PVE::Tools::lock_file($PCIID_RESERVATION_LOCK, 10, sub { PVE::Tools::lock_file($PCIID_RESERVATION_LOCK, 5, sub {
my $reservation_list = $parse_pci_reservation_unlocked->();
my $ctime = time(); my $ctime = time();
my $pciids = $parse_pci_reservation_unlocked->(); for my $id ($requested_ids->@*) {
my $reservation = $reservation_list->{$id};
for my $id (@$ids) { if ($reservation && $reservation->{vmid} != $vmid) {
my $errmsg = "PCI device '$id' already in use\n";
if (my $pciid = $pciids->{$id}) {
if ($pciid->{vmid} != $vmid) {
# check time based reservation # check time based reservation
die $errmsg if defined($pciid->{time}) && $pciid->{time} > $ctime; die "PCI device '$id' is currently reserved for use by VMID '$reservation->{vmid}'\n"
if defined($reservation->{time}) && $reservation->{time} > $ctime;
if (my $reserved_pid = $reservation->{pid}) {
# check running vm # check running vm
my $pid = PVE::QemuServer::Helpers::vm_running_locally($pciid->{vmid}); my $running_pid = PVE::QemuServer::Helpers::vm_running_locally($reservation->{vmid});
if (my $oldpid = $pciid->{pid}) { if (defined($running_pid) && $running_pid == $reserved_pid) {
die $errmsg if defined($pid) && $pid == $oldpid; die "PCI device '$id' already in use by VMID '$reservation->{vmid}'\n";
warn "leftover PCI reservation found for $id, ignoring...\n" } else {
if !defined($pid) || $pid != $oldpid; warn "leftover PCI reservation found for $id, lets take it...\n";
} }
} }
} }
$pciids->{$id} = { $reservation_list->{$id} = { vmid => $vmid };
vmid => $vmid, if (defined($pid)) { # VM started up, we can reserve now with the actual PID
}; $reservation_list->{$id}->{pid} = $pid;
if (defined($timeout)) { } elsif (defined($timeout)) { # tempoaray reserve as we don't now the PID yet
# we save the time when the reservation gets invalid (+5s), $reservation_list->{$id}->{time} = $ctime + $timeout + 5;
# since the start timeout depends on the config
$pciids->{$id}->{time} = $ctime + $timeout + 5;
} }
$pciids->{$id}->{pid} = $pid if defined($pid);
} }
$write_pci_reservation_unlocked->($reservation_list);
$write_pci_reservation_unlocked->($pciids);
return;
}); });
die $@ if $@; die $@ if $@;
} }