Re: [pve-devel] [RFC PATCH storage] pvesm export: convert dd's \r in status=progress output to \n
On January 16, 2025 9:13 am, Dominik Csapak wrote: > On 1/15/25 12:28, Thomas Lamprecht wrote: >> Am 15.01.25 um 10:59 schrieb Dominik Csapak: >>> pvesm export is mostly used for (remote) migrations, where the >>> status progress output lands in a task log. For task logs we want to >>> have line based output (since it's not a terminal), but dd uses \r >>> to overwrite the same line which does not work in every situation, e.g. >>> browsers sometimes simply don't show them, making the dd output a long >>> line instead of separate ones. >>> >>> To fix this, use run_command's `errfunc` to log the lines. run_command >>> will split also on \r, but with warn we print a \n so this does the >>> conversion. >>> >>> This fixes an issue where the remote migration task log on PDM does not >>> display that part in new lines. (ExtJS works because it does things >>> differently and some browser quirks convert \r to \n) >>> >>> Signed-off-by: Dominik Csapak >>> --- >>> Not sure if we want to take this approach because we lose the >>> functionalty of overwriting progress on the terminal. >> >> FWIW, you could test with `-t STDERR` if the std error FD is a terminal >> and differ between replacing \r or not. >> > > not sure if that would work here since we do quite some redirection for > the worker task (to be able to display + putting it in the task log at > the same time), but yes, I'll try that > >>> >>> AFAICS there is no easy way to only do this for the task log, since >>> we simply pipe the output fh of the worker task to the task log fh. >>> >>> Alternatively we could patch the task log api to parse \r as newlines or >>> patch the yew widget toolkit to replace \r with \n. >> >> Wouldn't be one alternative also be to do that in the UI? > > thats what i meant in that sentence. (replacing in yew widget toolkit) one issue with that though would be that the line (in the task log/API response) can get really long? not sure whether/where that might cause problems.. ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [RFC PATCH storage] pvesm export: convert dd's \r in status=progress output to \n
Am 16.01.25 um 09:13 schrieb Dominik Csapak: >> FWIW, you could test with `-t STDERR` if the std error FD is a terminal >> and differ between replacing \r or not. >> > not sure if that would work here since we do quite some redirection for > the worker task (to be able to display + putting it in the task log at > the same time), but yes, I'll try that > >>> AFAICS there is no easy way to only do this for the task log, since >>> we simply pipe the output fh of the worker task to the task log fh. >>> >>> Alternatively we could patch the task log api to parse \r as newlines or >>> patch the yew widget toolkit to replace \r with \n. >> Wouldn't be one alternative also be to do that in the UI? > thats what i meant in that sentence. (replacing in yew widget toolkit) Ah OK, seems I sorta skipped reading after the first half of the sentence, sorry. Well, I see some merit in having this done at backend side, having some consistency and avoiding that other API consumer need to to such transformations, but it then might be worth thinking about doing it more generally. I shortly looked into that (fork_worker and run_command) and I initiially got slightly confused, as in run_command we already do `print STDERR "$laststderr\n" if $laststderr;` for the case there is no errfunc defined, with $laststderr being a single line without any limiter (\r and/or \n or \b, fwiw), but that's only called if there is a errmsg parameter; bleh, run_command should really be reworked (or die). So FWIW, this could be also "fixed" by passing something like errmsg => "failed to write file '$file'" ... <.< Another completely different option, skip DD and do the write ourself, might be even be done relatively easily with the sendfile syscall in a loop of 64K blocks and some output every few seconds, i.e. not totally trivial, but also not _that_ hard, just mentioning for sake of completeness, even if we want to go in that direction we can employ transforming the \r to \n as a stop gap. ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH docs v3 1/2] pvesr: update the chapter and bring it into good condition
On 1/10/25 17:58, Alexander Zeidler wrote: Existing information is preserved in a restructured way and usefully supplemented. * restructure and revise the introduction * add subchapter "Considerations" * remove the subchapter "Schedule Format" with its one line of content and link where appropriate directly to the copy under "25. Appendix D: Calendar Events". The help button at adding/editing a job links now to the subchapter "Managing Jobs". * provide details on job removal and how to enforce it if necessary * add more helpful CLI examples and improve/fix existing ones * restructure and revise the subchapter "Error Handling" Signed-off-by: Alexander Zeidler --- v3: * adapt the introduction and section "Risk of Data Loss" to provide information about using a shared storage together with storage replication * update the CLI example `pvesr update` (*:00 replaces incorrect */00) * implement most suggestions from Daniel Kral ** update commit message ** reword first paragraph of introduction ** rename subchapter "Recommendations" to "Considerations" ** write "every 15 minutes" to be consistent with UI (and additionally mention that examples from the drop-down list can be modified) ** reword the description of the bandwidth limit v2: https://lore.proxmox.com/pve-devel/20241218161948.3-1-a.zeid...@proxmox.com/ * no changes, only add missing pve-manager patch pvecm.adoc | 2 + pvesr.adoc | 411 + 2 files changed, 287 insertions(+), 126 deletions(-) I've read another time through the article and it reads great again - nothing to complain from my side ;). The description for why the anchors were created could've still landed in the commit message, but that's just a tiny nit for me. Otherwise thanks for the great article, consider this: Reviewed-by: Daniel Kral ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH qemu] add fix for crash during live migration in combination with block flush
Am 15.01.25 um 17:28 schrieb Thomas Lamprecht: > Am 08.01.25 um 14:03 schrieb Fiona Ebner: >> Setting blk->root is a graph change operation and thus needs to be >> protected by the block graph write lock in blk_remove_bs(). The >> assignment to blk->root in blk_insert_bs() is already protected by >> the block graph write lock. >> >> In particular, the graph read lock in blk_co_do_flush() could >> previously not ensure that blk_bs(blk) would always return the same >> value during the locked section, which could lead to a segfault [0] in >> combination with migration [1]. >> >> From the user-provided backtraces in the forum thread [1], it seems >> like blk_co_do_flush() managed to get past the >> blk_co_is_available(blk) check, meaning that blk_bs(blk) returned a >> non-NULL value during the check, but then, when calling >> bdrv_co_flush(), blk_bs(blk) returned NULL. >> >> [0]: >> >>> 0 bdrv_primary_child (bs=bs@entry=0x0) at ../block.c:8287 >>> 1 bdrv_co_flush (bs=0x0) at ../block/io.c:2948 >>> 2 bdrv_co_flush_entry (opaque=0x7a610affae90) at block/block-gen.c:901 >> >> [1]: https://forum.proxmox.com/threads/158072 >> >> Signed-off-by: Fiona Ebner >> --- >> >> Upstream submission of the same patch: >> https://lore.kernel.org/qemu-devel/20250108124649.333668-1-f.eb...@proxmox.com/T/ > > I only skimmed the upstream discussion, but seems that there is still some > issue left; so should I wait this version out? Yes, we should at least also put the "root = blk->root;" assignment into the write lock section like the upstream maintainer suggested. That more complete change is in the package provided to the forum user. The change should still be an improvement over the status quo, however, the user reported that it didn't help with the specific crash. I don't see other code paths that would fit the provided backtraces right now :/ I'll ask the user to try again with a more complete GDB script in the hope of discovering something I missed. ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH qemu-server v2 02/18] machine: drop unused parameter from assert_valid_machine_property() helper
Signed-off-by: Fiona Ebner
---
PVE/API2/Qemu.pm | 4 ++--
PVE/QemuServer.pm | 2 +-
PVE/QemuServer/Machine.pm | 2 +-
3 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
index 8392400e..52425ee8 100644
--- a/PVE/API2/Qemu.pm
+++ b/PVE/API2/Qemu.pm
@@ -1250,7 +1250,7 @@ __PACKAGE__->register_method({
$conf->{machine} =
PVE::QemuServer::Machine::print_machine($machine_conf);
}
}
-
PVE::QemuServer::Machine::assert_valid_machine_property($conf, $machine_conf);
+
PVE::QemuServer::Machine::assert_valid_machine_property($machine_conf);
$conf->{lock} = 'import' if $live_import_mapping;
@@ -2113,7 +2113,7 @@ my $update_vm_api = sub {
$conf->{pending}->{$opt} = $param->{$opt};
} elsif ($opt eq 'machine') {
my $machine_conf =
PVE::QemuServer::Machine::parse_machine($param->{$opt});
-
PVE::QemuServer::Machine::assert_valid_machine_property($conf, $machine_conf);
+
PVE::QemuServer::Machine::assert_valid_machine_property($machine_conf);
$conf->{pending}->{$opt} = $param->{$opt};
} elsif ($opt eq 'cipassword') {
if
(!PVE::QemuServer::Helpers::windows_version($conf->{ostype})) {
diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
index 5cde94a1..bc38fb1b 100644
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -4193,7 +4193,7 @@ sub config_to_command {
$machine_type_min .= "+pve$required_pve_version";
push @$machineFlags, "type=${machine_type_min}";
-PVE::QemuServer::Machine::assert_valid_machine_property($conf,
$machine_conf);
+PVE::QemuServer::Machine::assert_valid_machine_property($machine_conf);
if (my $viommu = $machine_conf->{viommu}) {
if ($viommu eq 'intel') {
diff --git a/PVE/QemuServer/Machine.pm b/PVE/QemuServer/Machine.pm
index a072ac29..7f03ef20 100644
--- a/PVE/QemuServer/Machine.pm
+++ b/PVE/QemuServer/Machine.pm
@@ -56,7 +56,7 @@ sub print_machine {
}
sub assert_valid_machine_property {
-my ($conf, $machine_conf) = @_;
+my ($machine_conf) = @_;
my $q35 = $machine_conf->{type} && ($machine_conf->{type} =~ m/q35/) ? 1 :
0;
if ($machine_conf->{viommu} && $machine_conf->{viommu} eq "intel" &&
!$q35) {
die "to use Intel vIOMMU please set the machine type to q35\n";
--
2.39.5
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH qemu/qemu-server/docs v2 00/18] adapt to changes in QEMU machine version deprecation/removal
Changes in v2:
* change deprecation logic into QEMU itself rather than just weakening
the warning there (so no need to change the deprecation logic in
qemu-server anymore)
* get rid of outdated information from "Update to a Newer Machine
Version" section in docs
In QEMU commit a35f8577a0 ("include/hw: add macros for deprecation &
removal of versioned machines"), a new machine version deprecation and
removal policy was introduced. After only 3 years a machine version
will be deprecated while being removed after 6 years.
The deprecation is a bit early considering major PVE releases are
approximately every 2 years. This means that a deprecation warning can
already happen for a machine version that was introduced during the
previous major release. This would scare users for no good reason, so
avoid deprecating machine versions in PVE too early and define a
baseline of machine versions that will be supported throughout a
single major PVE release.
The new policy takes effect only in QEMU 10.1, see QEMU commit
c9fd2d9a48 ("include/hw: temporarily disable deletion of versioned
machine types"). Machine versions <=2.3 were already deprecated for a
while, with PVE also warning about it since commit dec371d9 ("vm
start: add warning about deprecated machine version") in qemu-server
8.0.8. These have been dropped in QEMU 9.1, so the baseline for PVE 8
is 2.4.
This series is intended to allow broader QEMU 9.1 rollout. Still
required in addition to this series before PVE 9:
* wiki article about what to look out for when changing machine
version
* checks in pve8to9 script giving errors/warnings when machine version
is too old
qemu:
Fiona Ebner (1):
adapt machine version deprecation for Proxmox VE
...e-version-deprecation-for-Proxmox-VE.patch | 137 ++
debian/patches/series | 1 +
2 files changed, 138 insertions(+)
create mode 100644
debian/patches/pve/0052-adapt-machine-version-deprecation-for-Proxmox-VE.patch
qemu-server:
Fiona Ebner (16):
machine: drop unused parameter from assert_valid_machine_property()
helper
move get_command_for_arch() helper to helpers module
helpers: improve name for variable for mapping arch to binary
move kvm_user_version() function to helpers module
move get_vm_arch() helper to helpers module
machine: add default_machine_for_arch() helper
move get_installed_machine_version() helper to machine module
move windows_get_pinned_machine_version() function to machine module
move get_vm_machine() function to machine module
move meta information handling to its own module
machine: get vm machine: fallback to creation QEMU version for windows
starting with 9.1
machine: add check_and_pin_machine_string() helper
api: update vm config: pin machine version when switching to windows
os type
machine: log informational line when pinning machine version for
Windows guest
machine: rename machine_version() function to
machine_version_at_least()
machine: code cleanup: avoid superfluous augmented assignment operator
PVE/API2/Qemu.pm | 44 +---
PVE/QemuServer.pm| 184 +++
PVE/QemuServer/Helpers.pm| 49
PVE/QemuServer/Machine.pm| 107 +-
PVE/QemuServer/Makefile | 1 +
PVE/QemuServer/MetaInfo.pm | 47
test/run_config2command_tests.pl | 3 +-
7 files changed, 248 insertions(+), 187 deletions(-)
create mode 100644 PVE/QemuServer/MetaInfo.pm
pve-docs:
Fiona Ebner (1):
qm: machine version: document support in PVE
qm.adoc | 39 +++
1 file changed, 31 insertions(+), 8 deletions(-)
Summary over all repositories:
10 files changed, 417 insertions(+), 195 deletions(-)
--
Generated by git-murpp 0.5.0
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH qemu-server v2 08/18] move get_installed_machine_version() helper to machine module
Signed-off-by: Fiona Ebner
---
PVE/QemuServer.pm | 9 +
PVE/QemuServer/Machine.pm | 7 +++
2 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
index 82795767..ee7bb017 100644
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -3271,13 +3271,6 @@ sub vga_conf_has_spice {
return $1 || 1;
}
-sub get_installed_machine_version {
-my ($kvmversion) = @_;
-$kvmversion = kvm_user_version() if !defined($kvmversion);
-$kvmversion =~ m/^(\d+\.\d+)/;
-return $1;
-}
-
sub windows_get_pinned_machine_version {
my ($machine, $base_version, $kvmversion) = @_;
@@ -3285,7 +3278,7 @@ sub windows_get_pinned_machine_version {
if (!defined($base_version) ||
!PVE::QemuServer::Machine::can_run_pve_machine_version($base_version,
$kvmversion)
) {
- $pin_version = get_installed_machine_version($kvmversion);
+ $pin_version =
PVE::QemuServer::Machine::get_installed_machine_version($kvmversion);
}
if (!$machine || $machine eq 'pc') {
$machine = "pc-i440fx-$pin_version";
diff --git a/PVE/QemuServer/Machine.pm b/PVE/QemuServer/Machine.pm
index 5a039244..075554f4 100644
--- a/PVE/QemuServer/Machine.pm
+++ b/PVE/QemuServer/Machine.pm
@@ -186,4 +186,11 @@ sub qemu_machine_pxe {
return $machine;
}
+sub get_installed_machine_version {
+my ($kvmversion) = @_;
+$kvmversion = PVE::QemuServer::Helpers::kvm_user_version() if
!defined($kvmversion);
+$kvmversion =~ m/^(\d+\.\d+)/;
+return $1;
+}
+
1;
--
2.39.5
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH docs v2 18/18] qm: machine version: document support in PVE
Elaborate on new QEMU machine version removal policy and how PVE will
support machine versions. Make sure to also mention the years, so that
users immediately have a good idea for how long it will be.
Signed-off-by: Fiona Ebner
---
Changes in v2:
* get rid of outdated information from "Update to a Newer Machine
Version" section
* drop sentence about early pre-deprecation warning (doesn't exist
anymore)
qm.adoc | 39 +++
1 file changed, 31 insertions(+), 8 deletions(-)
diff --git a/qm.adoc b/qm.adoc
index 94fdd4e..9d8becf 100644
--- a/qm.adoc
+++ b/qm.adoc
@@ -173,19 +173,42 @@ This means that after a fresh start, the newest machine
version supported by the
QEMU binary is used (e.g. the newest machine version QEMU 8.1 supports is
version 8.1 for each machine type).
+Starting with QEMU 10.1, machine versions will be removed from upstream QEMU
+after 6 years. In {pve}, major releases happen approximately every 2 years, so
a
+major {pve} release will support machine versions from approximately two
+previous major {pve} releases, more details below.
+
+Before upgrading to a new major {pve} release, you should update VM
+configurations to avoid all machine versions that will be dropped during the
+next major {pve} release. This ensures that the guests can still be used
+throughout that release. See the section
+xref:qm_machine_update[Update to a Newer Machine Version].
+
+The following table shows the expected baselines of supported machine versions
+for the current and upcoming major {pve} releases (best guesses):
+
+[width="100%",cols=">s,>,>s,2*>",options="header"]
+|===
+| {pve} | active development | supported baseline | dropped during life cycle
| last QEMU binary
+| 8 | 2023-2025 |2.4 | 2.3 and older
| 9.2
+| 9 | 2025-2027 |6.0 | 5.2 and older
| 11.2
+|10 | 2027-2029 |8.0 | 7.2 and older
| 13.2
+|===
+
+NOTE: Support for {pve} releases is longer than active development, but no new
+QEMU binary versions will be added after active development, just backports and
+fixes for existing binary versions.
+
[[qm_machine_update]]
Update to a Newer Machine Version
+
-Very old machine versions might become deprecated in QEMU. For example, this is
-the case for versions 1.4 to 1.7 for the i440fx machine type. It is expected
-that support for these machine versions will be dropped at some point. If you
-see a deprecation warning, you should change the machine version to a newer
one.
-Be sure to have a working backup first and be prepared for changes to how the
-guest sees hardware. In some scenarios, re-installing certain drivers might be
-required. You should also check for snapshots with RAM that were taken with
-these machine versions (i.e. the `runningmachine` configuration entry).
+If you see a deprecation warning, you should change the machine version to a
+newer one. Be sure to have a working backup first and be prepared for changes
to
+how the guest sees hardware. In some scenarios, re-installing certain drivers
+might be required. You should also check for snapshots with RAM that were taken
+with these machine versions (i.e. the `runningmachine` configuration entry).
Unfortunately, there is no way to change the machine version of a snapshot, so
you'd need to load the snapshot to salvage any data from it.
--
2.39.5
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH qemu-server v2 06/18] move get_vm_arch() helper to helpers module
Signed-off-by: Fiona Ebner
---
PVE/API2/Qemu.pm | 4 ++--
PVE/QemuServer.pm | 13 -
PVE/QemuServer/Helpers.pm | 5 +
3 files changed, 11 insertions(+), 11 deletions(-)
diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
index 868508e7..2840de1b 100644
--- a/PVE/API2/Qemu.pm
+++ b/PVE/API2/Qemu.pm
@@ -1198,7 +1198,7 @@ __PACKAGE__->register_method({
my $realcmd = sub {
my $conf = $param;
- my $arch = PVE::QemuServer::get_vm_arch($conf);
+ my $arch = PVE::QemuServer::Helpers::get_vm_arch($conf);
for my $opt (sort keys $param->%*) {
next if $opt !~ m/^scsi\d+$/;
@@ -2034,7 +2034,7 @@ my $update_vm_api = sub {
$conf = PVE::QemuConfig->load_config($vmid); # update/reload
next if defined($conf->{pending}->{$opt}) && ($param->{$opt} eq
$conf->{pending}->{$opt}); # skip if nothing changed
- my $arch = PVE::QemuServer::get_vm_arch($conf);
+ my $arch = PVE::QemuServer::Helpers::get_vm_arch($conf);
if (PVE::QemuServer::is_valid_drivename($opt)) {
# old drive
diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
index fd1feada..164cb0e8 100644
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -3271,11 +3271,6 @@ sub vga_conf_has_spice {
return $1 || 1;
}
-sub get_vm_arch {
-my ($conf) = @_;
-return $conf->{arch} // get_host_arch();
-}
-
my $default_machines = {
x86_64 => 'pc',
aarch64 => 'virt',
@@ -3590,7 +3585,7 @@ sub config_to_command {
my $machine_conf =
PVE::QemuServer::Machine::parse_machine($conf->{machine});
-my $arch = get_vm_arch($conf);
+my $arch = PVE::QemuServer::Helpers::get_vm_arch($conf);
my $kvm_binary = PVE::QemuServer::Helpers::get_command_for_arch($arch);
my $kvmver = kvm_user_version($kvm_binary);
@@ -4670,7 +4665,7 @@ sub qemu_cpu_hotplug {
if scalar(@{$currentrunningvcpus}) != $currentvcpus;
if (PVE::QemuServer::Machine::machine_version($machine_type, 2, 7)) {
- my $arch = get_vm_arch($conf);
+ my $arch = PVE::QemuServer::Helpers::get_vm_arch($conf);
for (my $i = $currentvcpus+1; $i <= $vcpus; $i++) {
my $cpustr = print_cpu_device($conf, $arch, $i);
@@ -4912,7 +4907,7 @@ sub vmconfig_hotplug_pending {
my ($vmid, $conf, $storecfg, $selection, $errors) = @_;
my $defaults = load_defaults();
-my $arch = get_vm_arch($conf);
+my $arch = PVE::QemuServer::Helpers::get_vm_arch($conf);
my $machine_type = get_vm_machine($conf, undef, $arch);
# commit values which do not have any impact on running VM first
@@ -8493,7 +8488,7 @@ sub qemu_use_old_bios_files {
sub get_efivars_size {
my ($conf, $efidisk) = @_;
-my $arch = get_vm_arch($conf);
+my $arch = PVE::QemuServer::Helpers::get_vm_arch($conf);
$efidisk //= $conf->{efidisk0} ? parse_drive('efidisk0',
$conf->{efidisk0}) : undef;
my $smm = PVE::QemuServer::Machine::machine_type_is_q35($conf);
my (undef, $ovmf_vars) = get_ovmf_files($arch, $efidisk, $smm);
diff --git a/PVE/QemuServer/Helpers.pm b/PVE/QemuServer/Helpers.pm
index 07b2ff6e..8e9f4fc0 100644
--- a/PVE/QemuServer/Helpers.pm
+++ b/PVE/QemuServer/Helpers.pm
@@ -34,6 +34,11 @@ sub get_command_for_arch($) {
return $cmd;
}
+sub get_vm_arch {
+my ($conf) = @_;
+return $conf->{arch} // get_host_arch();
+}
+
my $kvm_user_version = {};
my $kvm_mtime = {};
--
2.39.5
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH qemu-server v2 09/18] move windows_get_pinned_machine_version() function to machine module
Signed-off-by: Fiona Ebner
---
PVE/API2/Qemu.pm | 3 ++-
PVE/QemuServer.pm | 24 +---
PVE/QemuServer/Machine.pm | 20
3 files changed, 23 insertions(+), 24 deletions(-)
diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
index 2840de1b..8d0d788f 100644
--- a/PVE/API2/Qemu.pm
+++ b/PVE/API2/Qemu.pm
@@ -1246,7 +1246,8 @@ __PACKAGE__->register_method({
if (!$machine || $machine =~ m/^(?:pc|q35|virt)$/) {
# always pin Windows' machine version on create, they
get to easily confused
if
(PVE::QemuServer::Helpers::windows_version($conf->{ostype})) {
- $machine_conf->{type} =
PVE::QemuServer::windows_get_pinned_machine_version($machine);
+ $machine_conf->{type} =
+
PVE::QemuServer::Machine::windows_get_pinned_machine_version($machine);
$conf->{machine} =
PVE::QemuServer::Machine::print_machine($machine_conf);
}
}
diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
index ee7bb017..989fafbd 100644
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -3271,28 +3271,6 @@ sub vga_conf_has_spice {
return $1 || 1;
}
-sub windows_get_pinned_machine_version {
-my ($machine, $base_version, $kvmversion) = @_;
-
-my $pin_version = $base_version;
-if (!defined($base_version) ||
- !PVE::QemuServer::Machine::can_run_pve_machine_version($base_version,
$kvmversion)
-) {
- $pin_version =
PVE::QemuServer::Machine::get_installed_machine_version($kvmversion);
-}
-if (!$machine || $machine eq 'pc') {
- $machine = "pc-i440fx-$pin_version";
-} elsif ($machine eq 'q35') {
- $machine = "pc-q35-$pin_version";
-} elsif ($machine eq 'virt') {
- $machine = "virt-$pin_version";
-} else {
- warn "unknown machine type '$machine', not touching that!\n";
-}
-
-return $machine;
-}
-
sub get_vm_machine {
my ($conf, $forcemachine, $arch) = @_;
@@ -3305,7 +3283,7 @@ sub get_vm_machine {
# layout which confuses windows quite a bit and may result in various
regressions..
# see:
https://lists.gnu.org/archive/html/qemu-devel/2021-02/msg08484.html
if (windows_version($conf->{ostype})) {
- $machine = windows_get_pinned_machine_version($machine, '5.1',
$kvmversion);
+ $machine =
PVE::QemuServer::Machine::windows_get_pinned_machine_version($machine, '5.1',
$kvmversion);
}
$arch //= 'x86_64';
$machine ||= PVE::QemuServer::Machine::default_machine_for_arch($arch);
diff --git a/PVE/QemuServer/Machine.pm b/PVE/QemuServer/Machine.pm
index 075554f4..04c77ed5 100644
--- a/PVE/QemuServer/Machine.pm
+++ b/PVE/QemuServer/Machine.pm
@@ -193,4 +193,24 @@ sub get_installed_machine_version {
return $1;
}
+sub windows_get_pinned_machine_version {
+my ($machine, $base_version, $kvmversion) = @_;
+
+my $pin_version = $base_version;
+if (!defined($base_version) || !can_run_pve_machine_version($base_version,
$kvmversion)) {
+ $pin_version = get_installed_machine_version($kvmversion);
+}
+if (!$machine || $machine eq 'pc') {
+ $machine = "pc-i440fx-$pin_version";
+} elsif ($machine eq 'q35') {
+ $machine = "pc-q35-$pin_version";
+} elsif ($machine eq 'virt') {
+ $machine = "virt-$pin_version";
+} else {
+ warn "unknown machine type '$machine', not touching that!\n";
+}
+
+return $machine;
+}
+
1;
--
2.39.5
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH qemu-server v2 15/18] machine: log informational line when pinning machine version for Windows guest
Signed-off-by: Fiona Ebner
---
PVE/QemuServer/Machine.pm | 1 +
1 file changed, 1 insertion(+)
diff --git a/PVE/QemuServer/Machine.pm b/PVE/QemuServer/Machine.pm
index cf00da6d..9b18cf6e 100644
--- a/PVE/QemuServer/Machine.pm
+++ b/PVE/QemuServer/Machine.pm
@@ -269,6 +269,7 @@ sub check_and_pin_machine_string {
# always pin Windows' machine version on create, they get confused too
easily
if (PVE::QemuServer::Helpers::windows_version($ostype)) {
$machine_conf->{type} =
windows_get_pinned_machine_version($machine);
+ print "pinning machine type to '$machine_conf->{type}' for Windows
guest OS\n";
}
}
--
2.39.5
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH qemu-server v2 17/18] machine: code cleanup: avoid superfluous augmented assignment operator
Suggested by perlcritic.
Signed-off-by: Fiona Ebner
---
PVE/QemuServer/Machine.pm | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/PVE/QemuServer/Machine.pm b/PVE/QemuServer/Machine.pm
index 6398e756..7b7479d1 100644
--- a/PVE/QemuServer/Machine.pm
+++ b/PVE/QemuServer/Machine.pm
@@ -221,7 +221,7 @@ sub get_vm_machine {
my $machine = $forcemachine || $machine_conf->{type};
if (!$machine || $machine =~ m/^(?:pc|q35|virt)$/) {
- my $kvmversion //= PVE::QemuServer::Helpers::kvm_user_version();
+ my $kvmversion = PVE::QemuServer::Helpers::kvm_user_version();
# we must pin Windows VMs without a specific version and no meta info
about creation QEMU to
# 5.1, as 5.2 fixed a bug in ACPI layout which confuses windows quite a
bit and may result
# in various regressions..
--
2.39.5
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH qemu-server v2 10/18] move get_vm_machine() function to machine module
Signed-off-by: Fiona Ebner
---
PVE/API2/Qemu.pm | 2 +-
PVE/QemuServer.pm | 38 ++
PVE/QemuServer/Machine.pm | 34 ++
3 files changed, 37 insertions(+), 37 deletions(-)
diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
index 8d0d788f..868049cb 100644
--- a/PVE/API2/Qemu.pm
+++ b/PVE/API2/Qemu.pm
@@ -852,7 +852,7 @@ sub assert_scsi_feature_compatibility {
my $drive = PVE::QemuServer::Drive::parse_drive($opt, $drive_attributes,
1);
-my $machine_type = PVE::QemuServer::get_vm_machine($conf, undef,
$conf->{arch});
+my $machine_type = PVE::QemuServer::Machine::get_vm_machine($conf, undef,
$conf->{arch});
my $machine_version = PVE::QemuServer::Machine::extract_version(
$machine_type, PVE::QemuServer::Helpers::kvm_user_version());
my $drivetype = PVE::QemuServer::Drive::get_scsi_device_type(
diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
index 989fafbd..04d251e0 100644
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -3271,40 +3271,6 @@ sub vga_conf_has_spice {
return $1 || 1;
}
-sub get_vm_machine {
-my ($conf, $forcemachine, $arch) = @_;
-
-my $machine_conf =
PVE::QemuServer::Machine::parse_machine($conf->{machine});
-my $machine = $forcemachine || $machine_conf->{type};
-
-if (!$machine || $machine =~ m/^(?:pc|q35|virt)$/) {
- my $kvmversion //= kvm_user_version();
- # we must pin Windows VMs without a specific version to 5.1, as 5.2
fixed a bug in ACPI
- # layout which confuses windows quite a bit and may result in various
regressions..
- # see:
https://lists.gnu.org/archive/html/qemu-devel/2021-02/msg08484.html
- if (windows_version($conf->{ostype})) {
- $machine =
PVE::QemuServer::Machine::windows_get_pinned_machine_version($machine, '5.1',
$kvmversion);
- }
- $arch //= 'x86_64';
- $machine ||= PVE::QemuServer::Machine::default_machine_for_arch($arch);
- my $pvever = PVE::QemuServer::Machine::get_pve_version($kvmversion);
- $machine .= "+pve$pvever";
-}
-
-if ($machine !~ m/\+pve\d+?(?:\.pxe)?$/) {
- my $is_pxe = $machine =~ m/^(.*?)\.pxe$/;
- $machine = $1 if $is_pxe;
-
- # for version-pinned machines that do not include a pve-version (e.g.
- # pc-q35-4.1), we assume 0 to keep them stable in case we bump
- $machine .= '+pve0';
-
- $machine .= '.pxe' if $is_pxe;
-}
-
-return $machine;
-}
-
sub get_ovmf_files($$$) {
my ($arch, $efidisk, $smm) = @_;
@@ -3560,7 +3526,7 @@ sub config_to_command {
die "Detected old QEMU binary ('$kvmver', at least 5.0 is required)\n";
}
-my $machine_type = get_vm_machine($conf, $forcemachine, $arch);
+my $machine_type = PVE::QemuServer::Machine::get_vm_machine($conf,
$forcemachine, $arch);
my $machine_version = extract_version($machine_type, $kvmver);
$kvm //= 1 if is_native_arch($arch);
@@ -4874,7 +4840,7 @@ sub vmconfig_hotplug_pending {
my $defaults = load_defaults();
my $arch = PVE::QemuServer::Helpers::get_vm_arch($conf);
-my $machine_type = get_vm_machine($conf, undef, $arch);
+my $machine_type = PVE::QemuServer::Machine::get_vm_machine($conf, undef,
$arch);
# commit values which do not have any impact on running VM first
# Note: those option cannot raise errors, we we do not care about
diff --git a/PVE/QemuServer/Machine.pm b/PVE/QemuServer/Machine.pm
index 04c77ed5..915913c0 100644
--- a/PVE/QemuServer/Machine.pm
+++ b/PVE/QemuServer/Machine.pm
@@ -213,4 +213,38 @@ sub windows_get_pinned_machine_version {
return $machine;
}
+sub get_vm_machine {
+my ($conf, $forcemachine, $arch) = @_;
+
+my $machine_conf = parse_machine($conf->{machine});
+my $machine = $forcemachine || $machine_conf->{type};
+
+if (!$machine || $machine =~ m/^(?:pc|q35|virt)$/) {
+ my $kvmversion //= PVE::QemuServer::Helpers::kvm_user_version();
+ # we must pin Windows VMs without a specific version to 5.1, as 5.2
fixed a bug in ACPI
+ # layout which confuses windows quite a bit and may result in various
regressions..
+ # see:
https://lists.gnu.org/archive/html/qemu-devel/2021-02/msg08484.html
+ if (PVE::QemuServer::Helpers::windows_version($conf->{ostype})) {
+ $machine = windows_get_pinned_machine_version($machine, '5.1',
$kvmversion);
+ }
+ $arch //= 'x86_64';
+ $machine ||= default_machine_for_arch($arch);
+ my $pvever = get_pve_version($kvmversion);
+ $machine .= "+pve$pvever";
+}
+
+if ($machine !~ m/\+pve\d+?(?:\.pxe)?$/) {
+ my $is_pxe = $machine =~ m/^(.*?)\.pxe$/;
+ $machine = $1 if $is_pxe;
+
+ # for version-pinned machines that do not include a pve-version (e.g.
+ # pc-q35-4.1), we assume 0 to keep them stable in case we bump
+ $machine .= '+pve0';
+
+ $machine .= '.pxe' if $is_pxe;
+}
+
+r
[pve-devel] [PATCH qemu-server v2 03/18] move get_command_for_arch() helper to helpers module
Cannot use the is_native_arch() helper inside the function anymore,
to avoid a cyclic dependency between the 'CPUConfig' and 'Helpers'
modules, inline it.
Signed-off-by: Fiona Ebner
---
PVE/QemuServer.pm | 19 +++
PVE/QemuServer/Helpers.pm | 13 +
2 files changed, 16 insertions(+), 16 deletions(-)
diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
index bc38fb1b..ce962b7a 100644
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -1199,7 +1199,7 @@ my $kvm_mtime = {};
sub kvm_user_version {
my ($binary) = @_;
-$binary //= get_command_for_arch(get_host_arch()); # get the native arch
by default
+$binary //=
PVE::QemuServer::Helpers::get_command_for_arch(get_host_arch()); # get the
native arch by default
my $st = stat($binary);
my $cachedmtime = $kvm_mtime->{$binary} // -1;
@@ -3397,19 +3397,6 @@ sub get_ovmf_files($$$) {
return ($ovmf_code, $ovmf_vars);
}
-my $Arch2Qemu = {
-aarch64 => '/usr/bin/qemu-system-aarch64',
-x86_64 => '/usr/bin/qemu-system-x86_64',
-};
-sub get_command_for_arch($) {
-my ($arch) = @_;
-return '/usr/bin/kvm' if is_native_arch($arch);
-
-my $cmd = $Arch2Qemu->{$arch}
- or die "don't know how to emulate architecture '$arch'\n";
-return $cmd;
-}
-
# To use query_supported_cpu_flags and query_understood_cpu_flags to get flags
# to use in a QEMU command line (-cpu element), first array_intersect the
result
# of query_supported_ with query_understood_. This is necessary because:
@@ -3444,7 +3431,7 @@ sub query_supported_cpu_flags {
$arch eq "aarch64";
my $kvm_supported = defined(kvm_version());
-my $qemu_cmd = get_command_for_arch($arch);
+my $qemu_cmd = PVE::QemuServer::Helpers::get_command_for_arch($arch);
my $fakevmid = -1;
my $pidfile = PVE::QemuServer::Helpers::pidfile_name($fakevmid);
@@ -3634,7 +3621,7 @@ sub config_to_command {
my $machine_conf =
PVE::QemuServer::Machine::parse_machine($conf->{machine});
my $arch = get_vm_arch($conf);
-my $kvm_binary = get_command_for_arch($arch);
+my $kvm_binary = PVE::QemuServer::Helpers::get_command_for_arch($arch);
my $kvmver = kvm_user_version($kvm_binary);
if (!$kvmver || $kvmver !~ m/^(\d+)\.(\d+)/ || $1 < 5) {
diff --git a/PVE/QemuServer/Helpers.pm b/PVE/QemuServer/Helpers.pm
index 72a46a0a..d58bad2b 100644
--- a/PVE/QemuServer/Helpers.pm
+++ b/PVE/QemuServer/Helpers.pm
@@ -19,6 +19,19 @@ windows_version
my $nodename = PVE::INotify::nodename();
+my $Arch2Qemu = {
+aarch64 => '/usr/bin/qemu-system-aarch64',
+x86_64 => '/usr/bin/qemu-system-x86_64',
+};
+sub get_command_for_arch($) {
+my ($arch) = @_;
+return '/usr/bin/kvm' if get_host_arch() eq $arch; # i.e. native arch
+
+my $cmd = $Arch2Qemu->{$arch}
+ or die "don't know how to emulate architecture '$arch'\n";
+return $cmd;
+}
+
# Paths and directories
our $var_run_tmpdir = "/var/run/qemu-server";
--
2.39.5
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH qemu-server v2 11/18] move meta information handling to its own module
Like this, it can be used by modules that cannot depend on
QemuServer.pm without creating a cyclic dependency.
Signed-off-by: Fiona Ebner
---
PVE/API2/Qemu.pm | 3 ++-
PVE/QemuServer.pm | 42 +++---
PVE/QemuServer/Makefile| 1 +
PVE/QemuServer/MetaInfo.pm | 47 ++
4 files changed, 53 insertions(+), 40 deletions(-)
create mode 100644 PVE/QemuServer/MetaInfo.pm
diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
index 868049cb..60b8a4cc 100644
--- a/PVE/API2/Qemu.pm
+++ b/PVE/API2/Qemu.pm
@@ -35,6 +35,7 @@ use PVE::QemuServer::ImportDisk;
use PVE::QemuServer::Monitor qw(mon_cmd);
use PVE::QemuServer::Machine;
use PVE::QemuServer::Memory qw(get_current_memory);
+use PVE::QemuServer::MetaInfo;
use PVE::QemuServer::PCI;
use PVE::QemuServer::QMPHelpers;
use PVE::QemuServer::USB;
@@ -1205,7 +1206,7 @@ __PACKAGE__->register_method({
assert_scsi_feature_compatibility($opt, $conf, $storecfg,
$param->{$opt});
}
- $conf->{meta} = PVE::QemuServer::new_meta_info_string();
+ $conf->{meta} =
PVE::QemuServer::MetaInfo::new_meta_info_string();
my $vollist = [];
eval {
diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
index 04d251e0..6f2a9aed 100644
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -57,6 +57,7 @@ use PVE::QemuServer::CPUConfig qw(print_cpu_device
get_cpu_options get_cpu_bitne
use PVE::QemuServer::Drive qw(is_valid_drivename checked_volume_format
drive_is_cloudinit drive_is_cdrom drive_is_read_only parse_drive print_drive);
use PVE::QemuServer::Machine;
use PVE::QemuServer::Memory qw(get_current_memory);
+use PVE::QemuServer::MetaInfo;
use PVE::QemuServer::Monitor qw(mon_cmd);
use PVE::QemuServer::PCI qw(print_pci_addr print_pcie_addr
print_pcie_root_port parse_hostpci);
use PVE::QemuServer::QMPHelpers qw(qemu_deviceadd qemu_devicedel
qemu_objectadd qemu_objectdel);
@@ -281,21 +282,6 @@ my $rng_fmt = {
},
};
-my $meta_info_fmt = {
-'ctime' => {
- type => 'integer',
- description => "The guest creation timestamp as UNIX epoch time",
- minimum => 0,
- optional => 1,
-},
-'creation-qemu' => {
- type => 'string',
- description => "The QEMU (machine) version from the time this VM was
created.",
- pattern => '\d+(\.\d+)+',
- optional => 1,
-},
-};
-
my $confdesc = {
onboot => {
optional => 1,
@@ -729,7 +715,7 @@ EODESCR
},
meta => {
type => 'string',
- format => $meta_info_fmt,
+ format => $PVE::QemuServer::MetaInfo::meta_info_fmt,
description => "Some (read-only) meta-information about this guest.",
optional => 1,
},
@@ -2058,32 +2044,10 @@ sub parse_rng {
return $res;
}
-sub parse_meta_info {
-my ($value) = @_;
-
-return if !$value;
-
-my $res = eval { parse_property_string($meta_info_fmt, $value) };
-warn $@ if $@;
-return $res;
-}
-
-sub new_meta_info_string {
-my () = @_; # for now do not allow to override any value
-
-return PVE::JSONSchema::print_property_string(
- {
- 'creation-qemu' => kvm_user_version(),
- ctime => "". int(time()),
- },
- $meta_info_fmt
-);
-}
-
sub qemu_created_version_fixups {
my ($conf, $forcemachine, $kvmver) = @_;
-my $meta = parse_meta_info($conf->{meta}) // {};
+my $meta = PVE::QemuServer::MetaInfo::parse_meta_info($conf->{meta}) // {};
my $forced_vers = PVE::QemuServer::Machine::extract_version($forcemachine);
# check if we need to apply some handling for VMs that always use the
latest machine version but
diff --git a/PVE/QemuServer/Makefile b/PVE/QemuServer/Makefile
index 89d12091..18fd13ea 100644
--- a/PVE/QemuServer/Makefile
+++ b/PVE/QemuServer/Makefile
@@ -7,6 +7,7 @@ SOURCES=PCI.pm \
Helpers.pm \
Monitor.pm \
Machine.pm \
+ MetaInfo.pm \
CPUConfig.pm\
CGroup.pm \
Drive.pm\
diff --git a/PVE/QemuServer/MetaInfo.pm b/PVE/QemuServer/MetaInfo.pm
new file mode 100644
index ..a8cb6c5e
--- /dev/null
+++ b/PVE/QemuServer/MetaInfo.pm
@@ -0,0 +1,47 @@
+package PVE::QemuServer::MetaInfo;
+
+use strict;
+use warnings;
+
+use PVE::JSONSchema;
+
+use PVE::QemuServer::Helpers;
+
+our $meta_info_fmt = {
+'ctime' => {
+ type => 'integer',
+ description => "The guest creation timestamp as UNIX epoch time",
+ minimum => 0,
+ optional => 1,
+},
+'creation-qemu' => {
+ type => 'string',
+ description => "The QEMU (machine) version from the time this VM was
created.",
+ pattern => '\d+(\.\d+)+',
+ optional => 1,
+},
+};
+
+sub parse_meta_info {
+my ($value) = @_;
+
+return if !$value;
+
+my $res = eval { PVE::JSONSchema::parse_property_string($meta_info_fmt,
$value) }
[pve-devel] [PATCH qemu-server v2 16/18] machine: rename machine_version() function to machine_version_at_least()
The old name does not make it clear what exactly the function does.
Signed-off-by: Fiona Ebner
---
PVE/QemuServer.pm | 4 ++--
PVE/QemuServer/Machine.pm | 2 +-
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
index 6f2a9aed..58d23533 100644
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -4532,7 +4532,7 @@ sub qemu_cpu_hotplug {
if ($vcpus < $currentvcpus) {
- if (PVE::QemuServer::Machine::machine_version($machine_type, 2, 7)) {
+ if (PVE::QemuServer::Machine::machine_version_at_least($machine_type,
2, 7)) {
for (my $i = $currentvcpus; $i > $vcpus; $i--) {
qemu_devicedel($vmid, "cpu$i");
@@ -4560,7 +4560,7 @@ sub qemu_cpu_hotplug {
die "vcpus in running vm does not match its configuration\n"
if scalar(@{$currentrunningvcpus}) != $currentvcpus;
-if (PVE::QemuServer::Machine::machine_version($machine_type, 2, 7)) {
+if (PVE::QemuServer::Machine::machine_version_at_least($machine_type, 2,
7)) {
my $arch = PVE::QemuServer::Helpers::get_vm_arch($conf);
for (my $i = $currentvcpus+1; $i <= $vcpus; $i++) {
diff --git a/PVE/QemuServer/Machine.pm b/PVE/QemuServer/Machine.pm
index 9b18cf6e..6398e756 100644
--- a/PVE/QemuServer/Machine.pm
+++ b/PVE/QemuServer/Machine.pm
@@ -136,7 +136,7 @@ sub extract_version {
return;
}
-sub machine_version {
+sub machine_version_at_least {
my ($machine_type, $major, $minor, $pve) = @_;
return PVE::QemuServer::Helpers::min_version(
--
2.39.5
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH qemu-server v2 07/18] machine: add default_machine_for_arch() helper
There are already other places where 'aarch64' and 'x86_64' are
checked to be the only valid architectures, for example, the
get_command_for_arch() helper, so the new error scenario for an
unknown arch should not cause any regressions.
Signed-off-by: Fiona Ebner
---
PVE/QemuServer.pm | 9 ++---
PVE/QemuServer/Machine.pm | 12
2 files changed, 14 insertions(+), 7 deletions(-)
diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
index 164cb0e8..82795767 100644
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -3271,11 +3271,6 @@ sub vga_conf_has_spice {
return $1 || 1;
}
-my $default_machines = {
-x86_64 => 'pc',
-aarch64 => 'virt',
-};
-
sub get_installed_machine_version {
my ($kvmversion) = @_;
$kvmversion = kvm_user_version() if !defined($kvmversion);
@@ -3320,7 +3315,7 @@ sub get_vm_machine {
$machine = windows_get_pinned_machine_version($machine, '5.1',
$kvmversion);
}
$arch //= 'x86_64';
- $machine ||= $default_machines->{$arch};
+ $machine ||= PVE::QemuServer::Machine::default_machine_for_arch($arch);
my $pvever = PVE::QemuServer::Machine::get_pve_version($kvmversion);
$machine .= "+pve$pvever";
}
@@ -3386,7 +3381,7 @@ sub query_supported_cpu_flags {
my ($arch) = @_;
$arch //= get_host_arch();
-my $default_machine = $default_machines->{$arch};
+my $default_machine =
PVE::QemuServer::Machine::default_machine_for_arch($arch);
my $flags = {};
diff --git a/PVE/QemuServer/Machine.pm b/PVE/QemuServer/Machine.pm
index 7f03ef20..5a039244 100644
--- a/PVE/QemuServer/Machine.pm
+++ b/PVE/QemuServer/Machine.pm
@@ -55,6 +55,18 @@ sub print_machine {
return print_property_string($machine_conf, $machine_fmt);
}
+my $default_machines = {
+x86_64 => 'pc',
+aarch64 => 'virt',
+};
+
+sub default_machine_for_arch {
+my ($arch) = @_;
+
+my $machine = $default_machines->{$arch} or die "unsupported architecture
'$arch'\n";
+return $machine;
+}
+
sub assert_valid_machine_property {
my ($machine_conf) = @_;
my $q35 = $machine_conf->{type} && ($machine_conf->{type} =~ m/q35/) ? 1 :
0;
--
2.39.5
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH qemu-server v2 05/18] move kvm_user_version() function to helpers module
Add an export, since the function is rather commonly used (in
particular inlined in function calls, where prefixing with the module
name would hurt readability) and there won't be much potential for
confusion name-wise.
This was the only user of stat(), so remove the File::stat include.
Signed-off-by: Fiona Ebner
---
PVE/API2/Qemu.pm | 2 +-
PVE/QemuServer.pm| 32 +---
PVE/QemuServer/Helpers.pm| 31 +++
test/run_config2command_tests.pl | 3 ++-
4 files changed, 35 insertions(+), 33 deletions(-)
diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
index 52425ee8..868508e7 100644
--- a/PVE/API2/Qemu.pm
+++ b/PVE/API2/Qemu.pm
@@ -854,7 +854,7 @@ sub assert_scsi_feature_compatibility {
my $machine_type = PVE::QemuServer::get_vm_machine($conf, undef,
$conf->{arch});
my $machine_version = PVE::QemuServer::Machine::extract_version(
- $machine_type, PVE::QemuServer::kvm_user_version());
+ $machine_type, PVE::QemuServer::Helpers::kvm_user_version());
my $drivetype = PVE::QemuServer::Drive::get_scsi_device_type(
$drive, $storecfg, $machine_version);
diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
index ce962b7a..fd1feada 100644
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -10,7 +10,6 @@ use Fcntl;
use File::Basename;
use File::Copy qw(copy);
use File::Path;
-use File::stat;
use Getopt::Long;
use IO::Dir;
use IO::File;
@@ -51,7 +50,7 @@ use PVE::Tools qw(run_command file_read_firstline
file_get_contents dir_glob_for
use PVE::QMPClient;
use PVE::QemuConfig;
-use PVE::QemuServer::Helpers qw(config_aware_timeout min_version
windows_version);
+use PVE::QemuServer::Helpers qw(config_aware_timeout min_version
kvm_user_version windows_version);
use PVE::QemuServer::Cloudinit;
use PVE::QemuServer::CGroup;
use PVE::QemuServer::CPUConfig qw(print_cpu_device get_cpu_options
get_cpu_bitness is_native_arch get_amd_sev_object);
@@ -1193,35 +1192,6 @@ sub kvm_version {
return $kvm_api_version;
}
-my $kvm_user_version = {};
-my $kvm_mtime = {};
-
-sub kvm_user_version {
-my ($binary) = @_;
-
-$binary //=
PVE::QemuServer::Helpers::get_command_for_arch(get_host_arch()); # get the
native arch by default
-my $st = stat($binary);
-
-my $cachedmtime = $kvm_mtime->{$binary} // -1;
-return $kvm_user_version->{$binary} if $kvm_user_version->{$binary} &&
- $cachedmtime == $st->mtime;
-
-$kvm_user_version->{$binary} = 'unknown';
-$kvm_mtime->{$binary} = $st->mtime;
-
-my $code = sub {
- my $line = shift;
- if ($line =~ m/^QEMU( PC)? emulator version
(\d+\.\d+(\.\d+)?)(\.\d+)?[,\s]/) {
- $kvm_user_version->{$binary} = $2;
- }
-};
-
-eval { run_command([$binary, '--version'], outfunc => $code); };
-warn $@ if $@;
-
-return $kvm_user_version->{$binary};
-
-}
my sub extract_version {
my ($machine_type, $version) = @_;
$version = kvm_user_version() if !defined($version);
diff --git a/PVE/QemuServer/Helpers.pm b/PVE/QemuServer/Helpers.pm
index be10a92a..07b2ff6e 100644
--- a/PVE/QemuServer/Helpers.pm
+++ b/PVE/QemuServer/Helpers.pm
@@ -8,11 +8,13 @@ use JSON;
use PVE::INotify;
use PVE::ProcFSTools;
+use PVE::Tools qw(get_host_arch);
use base 'Exporter';
our @EXPORT_OK = qw(
min_version
config_aware_timeout
+kvm_user_version
parse_number_sets
windows_version
);
@@ -32,6 +34,35 @@ sub get_command_for_arch($) {
return $cmd;
}
+my $kvm_user_version = {};
+my $kvm_mtime = {};
+
+sub kvm_user_version {
+my ($binary) = @_;
+
+$binary //= get_command_for_arch(get_host_arch()); # get the native arch
by default
+my $st = stat($binary);
+
+my $cachedmtime = $kvm_mtime->{$binary} // -1;
+return $kvm_user_version->{$binary} if $kvm_user_version->{$binary} &&
+ $cachedmtime == $st->mtime;
+
+$kvm_user_version->{$binary} = 'unknown';
+$kvm_mtime->{$binary} = $st->mtime;
+
+my $code = sub {
+ my $line = shift;
+ if ($line =~ m/^QEMU( PC)? emulator version
(\d+\.\d+(\.\d+)?)(\.\d+)?[,\s]/) {
+ $kvm_user_version->{$binary} = $2;
+ }
+};
+
+eval { PVE::Tools::run_command([$binary, '--version'], outfunc => $code);
};
+warn $@ if $@;
+
+return $kvm_user_version->{$binary};
+}
+
# Paths and directories
our $var_run_tmpdir = "/var/run/qemu-server";
diff --git a/test/run_config2command_tests.pl b/test/run_config2command_tests.pl
index 5308b1fc..2feebd4a 100755
--- a/test/run_config2command_tests.pl
+++ b/test/run_config2command_tests.pl
@@ -15,6 +15,7 @@ use PVE::SysFSTools;
use PVE::QemuConfig;
use PVE::QemuServer;
+use PVE::QemuServer::Helpers;
use PVE::QemuServer::Monitor;
use PVE::QemuServer::QMPHelpers;
use PVE::QemuServer::CPUConfig;
@@ -72,7 +73,7 @@ my $base_env = {
}
},
vmid => 8006,
-real_qemu_version => PVE::QemuServer::kvm_user_version(), # not yet mocked
+
[pve-devel] [PATCH qemu-server v2 14/18] api: update vm config: pin machine version when switching to windows os type
During virtual machine creation, the machine version is pinned when
the guest OS is Windows. The same should be done when the guest OS
type is newly set to Windows for consistency.
Signed-off-by: Fiona Ebner
---
PVE/API2/Qemu.pm | 18 ++
1 file changed, 18 insertions(+)
diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
index b1410df6..8acd8d9f 100644
--- a/PVE/API2/Qemu.pm
+++ b/PVE/API2/Qemu.pm
@@ -2109,6 +2109,24 @@ my $update_vm_api = sub {
my $machine_conf =
PVE::QemuServer::Machine::parse_machine($param->{$opt});
PVE::QemuServer::Machine::assert_valid_machine_property($machine_conf);
$conf->{pending}->{$opt} = $param->{$opt};
+ } elsif ($opt eq 'ostype') {
+ # Check if machine version pinning is needed when switching
OS type, just like
+ # upon creation. Skip if 'machine' is explicitly set or
removed at the same time
+ # to honor the users request. While it should be enough to
look at $modified,
+ # because 'machine' is sorted before 'ostype', be explicit
just to be sure.
+ if (
+ !defined($param->{machine})
+ && !defined($conf->{pending}->{machine})
+ && !$modified->{machine} # detects deletion
+ ) {
+ eval {
+ $conf->{pending}->{machine} =
+
PVE::QemuServer::Machine::check_and_pin_machine_string(
+ $conf->{machine}, $param->{ostype});
+ };
+ print "automatic pinning of machine version failed -
$@" if $@;
+ }
+ $conf->{pending}->{$opt} = $param->{$opt};
} elsif ($opt eq 'cipassword') {
if
(!PVE::QemuServer::Helpers::windows_version($conf->{ostype})) {
# Same logic as in cloud-init (but with the regex
fixed...)
--
2.39.5
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH qemu-server v2 12/18] machine: get vm machine: fallback to creation QEMU version for windows starting with 9.1
Starting from QEMU 9.1, pin to the creation version instead. Support
for machine version 5.1 is expected to drop with QEMU 11.1 and it
would still be good to handle Windows VMs that do not have explicit
machine version for whatever reason. For example, explicitly setting
the machine without a version on the CLI/API after creation is one way
to end up with such a machine.
Signed-off-by: Fiona Ebner
---
PVE/QemuServer/Machine.pm | 19 ---
1 file changed, 16 insertions(+), 3 deletions(-)
diff --git a/PVE/QemuServer/Machine.pm b/PVE/QemuServer/Machine.pm
index 915913c0..679d69c7 100644
--- a/PVE/QemuServer/Machine.pm
+++ b/PVE/QemuServer/Machine.pm
@@ -4,6 +4,7 @@ use strict;
use warnings;
use PVE::QemuServer::Helpers;
+use PVE::QemuServer::MetaInfo;
use PVE::QemuServer::Monitor;
use PVE::JSONSchema qw(get_standard_option parse_property_string
print_property_string);
@@ -221,11 +222,23 @@ sub get_vm_machine {
if (!$machine || $machine =~ m/^(?:pc|q35|virt)$/) {
my $kvmversion //= PVE::QemuServer::Helpers::kvm_user_version();
- # we must pin Windows VMs without a specific version to 5.1, as 5.2
fixed a bug in ACPI
- # layout which confuses windows quite a bit and may result in various
regressions..
+ # we must pin Windows VMs without a specific version and no meta info
about creation QEMU to
+ # 5.1, as 5.2 fixed a bug in ACPI layout which confuses windows quite a
bit and may result
+ # in various regressions..
# see:
https://lists.gnu.org/archive/html/qemu-devel/2021-02/msg08484.html
+ # Starting from QEMU 9.1, pin to the creation version instead. Support
for 5.1 is expected
+ # to drop with QEMU 11.1 and it would still be good to handle Windows
VMs that do not have
+ # an explicit machine version for whatever reason.
if (PVE::QemuServer::Helpers::windows_version($conf->{ostype})) {
- $machine = windows_get_pinned_machine_version($machine, '5.1',
$kvmversion);
+ my $base_version = '5.1';
+ # TODO PVE 10 - die early if there is a Windows VM both without
explicit machine version
+ # and without meta info.
+ if (my $meta =
PVE::QemuServer::MetaInfo::parse_meta_info($conf->{meta})) {
+ $base_version = $meta->{'creation-qemu'}
+ if
PVE::QemuServer::Helpers::min_version($meta->{'creation-qemu'}, 9, 1);
+ ($base_version) = ($base_version =~ m/^(\d+.\d+)/); # need only
major.minor
+ }
+ $machine = windows_get_pinned_machine_version($machine,
$base_version, $kvmversion);
}
$arch //= 'x86_64';
$machine ||= default_machine_for_arch($arch);
--
2.39.5
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH qemu v2 01/18] adapt machine version deprecation for Proxmox VE
In commit a35f8577a0 ("include/hw: add macros for deprecation &
removal of versioned machines"), a new machine version deprecation and
removal policy was introduced. After only 3 years a machine version
will be deprecated while being removed after 6 years.
The deprecation is a bit early considering major PVE releases are
approximately every 2 years. This means that a deprecation warning can
already happen for a machine version that was introduced during the
previous major release. This would scare users for no good reason, so
avoid deprecating machine versions in PVE too early and define a
baseline of machine versions that will be supported throughout a
single major PVE release.
Reported-by: Martin Maurer
Signed-off-by: Fiona Ebner
---
Changes in v2:
* Rather than just weakening the warning from the QEMU side, adapt the
deprecation logic for PVE already here.
...e-version-deprecation-for-Proxmox-VE.patch | 137 ++
debian/patches/series | 1 +
2 files changed, 138 insertions(+)
create mode 100644
debian/patches/pve/0052-adapt-machine-version-deprecation-for-Proxmox-VE.patch
diff --git
a/debian/patches/pve/0052-adapt-machine-version-deprecation-for-Proxmox-VE.patch
b/debian/patches/pve/0052-adapt-machine-version-deprecation-for-Proxmox-VE.patch
new file mode 100644
index 000..6c1a73a
--- /dev/null
+++
b/debian/patches/pve/0052-adapt-machine-version-deprecation-for-Proxmox-VE.patch
@@ -0,0 +1,137 @@
+From Mon Sep 17 00:00:00 2001
+From: Fiona Ebner
+Date: Fri, 3 Jan 2025 14:03:12 +0100
+Subject: [PATCH] adapt machine version deprecation for Proxmox VE
+
+In commit a35f8577a0 ("include/hw: add macros for deprecation &
+removal of versioned machines"), a new machine version deprecation and
+removal policy was introduced. After only 3 years a machine version
+will be deprecated while being removed after 6 years.
+
+The deprecation is a bit early considering major PVE releases are
+approximately every 2 years. This means that a deprecation warning can
+already happen for a machine version that was introduced during the
+previous major release. This would scare users for no good reason, so
+avoid deprecating machine versions in PVE too early and define a
+baseline of machine versions that will be supported throughout a
+single major PVE release.
+
+Signed-off-by: Fiona Ebner
+---
+ include/hw/boards.h | 78 +
+ 1 file changed, 51 insertions(+), 27 deletions(-)
+
+diff --git a/include/hw/boards.h b/include/hw/boards.h
+index 5cddeb7fcb..b1e7787499 100644
+--- a/include/hw/boards.h
b/include/hw/boards.h
+@@ -607,42 +607,66 @@ struct MachineState {
+
+
+ /*
+- * How many years/major releases for each phase
+- * of the life cycle. Assumes use of versioning
+- * scheme where major is bumped each year
++ * Baseline of machine versions that are still considered supported throughout
++ * current major Proxmox VE release. Machine versions older than this are
++ * considered to be deprecated in Proxmox VE.
++ *
++ * Machine versions older than 6 years are removed just like in upstream QEMU.
++ * (policy takes effect with QEMU 10.1). Assumes yearly major QEMU release.
++ *
++ * QEMU release cylce N.0 in ~April, N.1 in ~August, N.2 in ~December
++ * Debian/PVE release cylce ~every two years in summer
++ *
++ * PVE - last QEMU - machine versions dropped - baseline
++ * 8 9.2 2.3 and older2.4
++ * 911.2 5.2 and older6.0
++ * 1013.2 7.2 and older8.0
++ */
++#define MACHINE_VER_BASELINE_PVE_MAJOR 2
++#define MACHINE_VER_BASELINE_PVE_MINOR 4
++#define MACHINE_VER_DELETION_MAJOR (QEMU_VERSION_MAJOR - 6)
++#define MACHINE_VER_DELETION_MINOR QEMU_VERSION_MINOR
++
++/*
++ * Proxmox VE needs to support the baseline throughout a major PVE release. So
++ * a QEMU release where the baseline is already deleted cannot be used.
++ * Removal policy after 6 years takes effect with QEMU 10.1.
+ */
+-#define MACHINE_VER_DELETION_MAJOR 6
+-#define MACHINE_VER_DEPRECATION_MAJOR 3
++#if ((QEMU_VERSION_MAJOR > 10) || ((QEMU_VERSION_MAJOR == 10) &&
(QEMU_VERSION_MINOR >= 1)))
++#if ((MACHINE_VER_BASELINE_PVE_MAJOR < MACHINE_VER_DELETION_MAJOR) || \
++ ((MACHINE_VER_BASELINE_PVE_MAJOR == MACHINE_VER_DELETION_MAJOR) && \
++ (MACHINE_VER_BASELINE_PVE_MINOR < MACHINE_VER_DELETION_MINOR)))
++#error "Baseline machine version needed by Proxmox VE not supported anymore
by this QEMU release"
++#endif
++#endif
+
+ /*
+ * Expands to a static string containing a deprecation
+ * message for a versioned machine type
+ */
+ #define MACHINE_VER_DEPRECATION_MSG \
+-"machines more than " stringify(MACHINE_VER_DEPRECATION_MAJOR) \
+-" years old are subject to deletion after " \
+-stringify(MACHINE_VER_DELETION_MAJOR) " years"
+-
+-#define _MACHINE_VER_IS_EXPIRED_IMPL(cutoff, major, minor) \
+-((
[pve-devel] [PATCH qemu-server v2 04/18] helpers: improve name for variable for mapping arch to binary
Signed-off-by: Fiona Ebner
---
PVE/QemuServer/Helpers.pm | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/PVE/QemuServer/Helpers.pm b/PVE/QemuServer/Helpers.pm
index d58bad2b..be10a92a 100644
--- a/PVE/QemuServer/Helpers.pm
+++ b/PVE/QemuServer/Helpers.pm
@@ -19,7 +19,7 @@ windows_version
my $nodename = PVE::INotify::nodename();
-my $Arch2Qemu = {
+my $arch_to_qemu_binary = {
aarch64 => '/usr/bin/qemu-system-aarch64',
x86_64 => '/usr/bin/qemu-system-x86_64',
};
@@ -27,7 +27,7 @@ sub get_command_for_arch($) {
my ($arch) = @_;
return '/usr/bin/kvm' if get_host_arch() eq $arch; # i.e. native arch
-my $cmd = $Arch2Qemu->{$arch}
+my $cmd = $arch_to_qemu_binary->{$arch}
or die "don't know how to emulate architecture '$arch'\n";
return $cmd;
}
--
2.39.5
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH qemu-server v2 13/18] machine: add check_and_pin_machine_string() helper
Extract the logic for guest OS-type dependent machine version pinning
into a dedicated helper, so it can be re-used.
Signed-off-by: Fiona Ebner
---
PVE/API2/Qemu.pm | 14 +++---
PVE/QemuServer/Machine.pm | 16
2 files changed, 19 insertions(+), 11 deletions(-)
diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
index 60b8a4cc..b1410df6 100644
--- a/PVE/API2/Qemu.pm
+++ b/PVE/API2/Qemu.pm
@@ -1242,17 +1242,9 @@ __PACKAGE__->register_method({
$conf->{vmgenid} = PVE::QemuServer::generate_uuid();
}
- my $machine_conf =
PVE::QemuServer::Machine::parse_machine($conf->{machine});
- my $machine = $machine_conf->{type};
- if (!$machine || $machine =~ m/^(?:pc|q35|virt)$/) {
- # always pin Windows' machine version on create, they
get to easily confused
- if
(PVE::QemuServer::Helpers::windows_version($conf->{ostype})) {
- $machine_conf->{type} =
-
PVE::QemuServer::Machine::windows_get_pinned_machine_version($machine);
- $conf->{machine} =
PVE::QemuServer::Machine::print_machine($machine_conf);
- }
- }
-
PVE::QemuServer::Machine::assert_valid_machine_property($machine_conf);
+ # always pin Windows' machine version on create, they get
confused too easily
+ $conf->{machine} =
PVE::QemuServer::Machine::check_and_pin_machine_string(
+ $conf->{machine}, $conf->{ostype});
$conf->{lock} = 'import' if $live_import_mapping;
diff --git a/PVE/QemuServer/Machine.pm b/PVE/QemuServer/Machine.pm
index 679d69c7..cf00da6d 100644
--- a/PVE/QemuServer/Machine.pm
+++ b/PVE/QemuServer/Machine.pm
@@ -260,4 +260,20 @@ sub get_vm_machine {
return $machine;
}
+sub check_and_pin_machine_string {
+my ($machine_string, $ostype) = @_;
+
+my $machine_conf = parse_machine($machine_string);
+my $machine = $machine_conf->{type};
+if (!$machine || $machine =~ m/^(?:pc|q35|virt)$/) {
+ # always pin Windows' machine version on create, they get confused too
easily
+ if (PVE::QemuServer::Helpers::windows_version($ostype)) {
+ $machine_conf->{type} =
windows_get_pinned_machine_version($machine);
+ }
+}
+
+assert_valid_machine_property($machine_conf);
+return print_machine($machine_conf);
+}
+
1;
--
2.39.5
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH qemu] add fix for crash during live migration in combination with block flush
Am 16.01.25 um 11:30 schrieb Fiona Ebner: > That more complete change is in the package provided to the forum user. > The change should still be an improvement over the status quo, however, > the user reported that it didn't help with the specific crash. I don't > see other code paths that would fit the provided backtraces right now :/ > I'll ask the user to try again with a more complete GDB script in the > hope of discovering something I missed. Ack, and maybe also ensure they actually restarted the VM after installing the package ^^ ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH proxmox-ve-config] cargo: update dependencies to latest
Obsolete. This has been done here already: https://git.proxmox.com/?p=proxmox-ve-rs.git;a=commit;h=a8f9c5ca270b6d25249c4c85805ed90c1b02ff88 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [RFC PATCH storage] pvesm export: convert dd's \r in status=progress output to \n
On 1/15/25 12:28, Thomas Lamprecht wrote:
Am 15.01.25 um 10:59 schrieb Dominik Csapak:
pvesm export is mostly used for (remote) migrations, where the
status progress output lands in a task log. For task logs we want to
have line based output (since it's not a terminal), but dd uses \r
to overwrite the same line which does not work in every situation, e.g.
browsers sometimes simply don't show them, making the dd output a long
line instead of separate ones.
To fix this, use run_command's `errfunc` to log the lines. run_command
will split also on \r, but with warn we print a \n so this does the
conversion.
This fixes an issue where the remote migration task log on PDM does not
display that part in new lines. (ExtJS works because it does things
differently and some browser quirks convert \r to \n)
Signed-off-by: Dominik Csapak
---
Not sure if we want to take this approach because we lose the
functionalty of overwriting progress on the terminal.
FWIW, you could test with `-t STDERR` if the std error FD is a terminal
and differ between replacing \r or not.
not sure if that would work here since we do quite some redirection for
the worker task (to be able to display + putting it in the task log at
the same time), but yes, I'll try that
AFAICS there is no easy way to only do this for the task log, since
we simply pipe the output fh of the worker task to the task log fh.
Alternatively we could patch the task log api to parse \r as newlines or
patch the yew widget toolkit to replace \r with \n.
Wouldn't be one alternative also be to do that in the UI?
thats what i meant in that sentence. (replacing in yew widget toolkit)
No real preference from my side, but this patch fixes the task log file
without touching our central worker task code, so it seemed sensible.
src/PVE/Storage/ISCSIPlugin.pm | 6 +-
src/PVE/Storage/LVMPlugin.pm | 6 +-
src/PVE/Storage/Plugin.pm | 10 --
3 files changed, 18 insertions(+), 4 deletions(-)
diff --git a/src/PVE/Storage/ISCSIPlugin.pm b/src/PVE/Storage/ISCSIPlugin.pm
index eb70453..33604cd 100644
--- a/src/PVE/Storage/ISCSIPlugin.pm
+++ b/src/PVE/Storage/ISCSIPlugin.pm
@@ -631,7 +631,11 @@ sub volume_export {
$size = int($1);
});
PVE::Storage::Plugin::write_common_header($fh, $size);
-run_command(['dd', "if=$file", "bs=64k", "status=progress"], output =>
'>&'.fileno($fh));
+run_command(['dd', "if=$file", "bs=64k", "status=progress"], output =>
'>&'.fileno($fh), errfunc => sub {
+ # convert dd's \r to \n
I'd move the comment into the line re-printing the output, shorter
and also avoids having something before extracting the parameters,
which should always be first in perl for now; once we widely switch
to using signatures that wouldn't be a problem anymore.
Maybe also add some reasoning to the comment, like: "convert \r to \n to avoid
issues in browsers"
sure, makes sense
+ my ($line) = @_;
+ warn "$line\n";
+});
Why not use `print STDERR "$line\n";`?
Because warn could be caught by a $SIG{__WARN__} handler from the call chain and
interpreted as problem. For relaying stderr messages to stderr again printing
directly to STDERR feels a bit more expressive and safer to me.
OK
return;
}
diff --git a/src/PVE/Storage/LVMPlugin.pm b/src/PVE/Storage/LVMPlugin.pm
index 38f7fa1..d41647b 100644
--- a/src/PVE/Storage/LVMPlugin.pm
+++ b/src/PVE/Storage/LVMPlugin.pm
@@ -647,7 +647,11 @@ sub volume_export {
$size = int($1);
});
PVE::Storage::Plugin::write_common_header($fh, $size);
-run_command(['dd', "if=$file", "bs=64k", "status=progress"], output =>
'>&'.fileno($fh));
+run_command(['dd', "if=$file", "bs=64k", "status=progress"], output =>
'>&'.fileno($fh), errfunc => sub {
+ # convert dd's \r to \n
+ my ($line) = @_;
+ warn "$line\n";
same as above
+});
}
sub volume_import_formats {
diff --git a/src/PVE/Storage/Plugin.pm b/src/PVE/Storage/Plugin.pm
index 65cf43f..c42a675 100644
--- a/src/PVE/Storage/Plugin.pm
+++ b/src/PVE/Storage/Plugin.pm
@@ -1703,11 +1703,17 @@ sub volume_export {
my $file_format = ($class->parse_volname($volname))[6];
my $size = file_size_info($file, undef, $file_format);
+ # convert dd's \r to \n
+ my $errfunc = sub {
+ my ($line) = @_;
+ warn "$line\n";
same here
+ };
+
if ($format eq 'raw+size') {
die $err_msg if $with_snapshots || $file_format eq 'subvol';
write_common_header($fh, $size);
if ($file_format eq 'raw') {
- run_command(['dd', "if=$file", "bs=4k", "status=progress"], output =>
'>&'.fileno($fh));
+ run_command(['dd', "if=$file", "bs=64k", "status=progress"], output =>
'>&'.fileno($fh), errfunc => $errfunc);
} else {
run_command(['qemu-img', 'convert', '-f', $file_format, '-O',
'raw', $file, '/dev/stdout'],
[pve-devel] [PATCH proxmox-ve-config] cargo: update dependencies to latest
Major proxmox-sectionc-config and minor proxmox-sys update, shouldn't change anything. Signed-off-by: Gabriel Goller --- proxmox-ve-config/Cargo.toml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/proxmox-ve-config/Cargo.toml b/proxmox-ve-config/Cargo.toml index 0c8f6166e75d..f04272c007ac 100644 --- a/proxmox-ve-config/Cargo.toml +++ b/proxmox-ve-config/Cargo.toml @@ -17,6 +17,6 @@ serde_json = "1" serde_plain = "1" serde_with = "3" -proxmox-schema = "3.1.2" -proxmox-sys = "0.6.4" +proxmox-schema = "4.0.0" +proxmox-sys = "0.6.5" proxmox-sortable-macro = "0.1.3" -- 2.39.5 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [RFC PATCH storage] pvesm export: convert dd's \r in status=progress output to \n
On 1/16/25 10:21, Thomas Lamprecht wrote: Am 16.01.25 um 09:13 schrieb Dominik Csapak: FWIW, you could test with `-t STDERR` if the std error FD is a terminal and differ between replacing \r or not. not sure if that would work here since we do quite some redirection for the worker task (to be able to display + putting it in the task log at the same time), but yes, I'll try that AFAICS there is no easy way to only do this for the task log, since we simply pipe the output fh of the worker task to the task log fh. Alternatively we could patch the task log api to parse \r as newlines or patch the yew widget toolkit to replace \r with \n. Wouldn't be one alternative also be to do that in the UI? thats what i meant in that sentence. (replacing in yew widget toolkit) Ah OK, seems I sorta skipped reading after the first half of the sentence, sorry. Well, I see some merit in having this done at backend side, having some consistency and avoiding that other API consumer need to to such transformations, but it then might be worth thinking about doing it more generally. I agree with it being done on the backend side, otherwise the output lands in the tasklog and the api as a single line which is imho confusing. I shortly looked into that (fork_worker and run_command) and I initiially got slightly confused, as in run_command we already do `print STDERR "$laststderr\n" if $laststderr;` for the case there is no errfunc defined, with $laststderr being a single line without any limiter (\r and/or \n or \b, fwiw), but that's only called if there is a errmsg parameter; bleh, run_command should really be reworked (or die). So FWIW, this could be also "fixed" by passing something like errmsg => "failed to write file '$file'" ... <.< I find it weird behaviour of run_command do do that kind of output transformation with a more or less "unrelated" option? Shouldn't that transformation happen always or never? (or with an explicit option?) Anyway would you prefer this approach to the manual errfunc? Would still warrant a comment IMHO since one might optimize the option away in the future without noticing it's there for the transformation... Another completely different option, skip DD and do the write ourself, might be even be done relatively easily with the sendfile syscall in a loop of 64K blocks and some output every few seconds, i.e. not totally trivial, but also not _that_ hard, just mentioning for sake of completeness, even if we want to go in that direction we can employ transforming the \r to \n as a stop gap. Also thought about something like this, Advantage would be that we're in control of the output and could more easily integrate that into a (potential future) feature where tasks can have meta info like percentage done/stages/etc. I'd still probably do the transformation fix now (one way or another) and look into how we can maybe more generalize such "streaming" commands away, that maybe also does format conversion (instead of qemu-img) and other exports (rbd/zfs/lvm?) as well? (Writing this, I'm noticing this easily becomes a much larger change/feature than what I was trying to fix though :P) I'm happy to do any of these, just let me know what you'd prefer. ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH manager] pvestatd: improve broadcast of node version-info
Until now, the pvestatd did broadcast the pve-manager version only once
after startup of the service. But there are some situations, where the
local pmxcfs (pve-cluster) restarts and loses that information.
Basically everytime we restart the pmxcfs without restarting pvestatd
too.
For example, on a cluster join, or if the pmxcfs has been restarted
manually.
By additionally checking if the local kv-store of the pmxcfs has any
version info for the node, we can decide if another broadcast is
necessary.
Therefore after the next run of pvestatd, we should have the full
version info available again.
Signed-off-by: Aaron Lauterer
---
This patch is preparation to get reliable version infos as I am picking
of the patch series of Folke to include more metrics into the RRD data
and summary graphs. [0]
This was a big blocker and now with the major version change coming up,
we at least can assume the latest 8.x installed as part of the update to
PVE 9.
Therefore, we should get this in with PVE 8. Additional patches for PVE
8 will follow to make the transition smoother. But as mentioned, this
here is one of the things that needs to work reliably, which is why I
submit the patch already now.
[0]
https://lore.proxmox.com/pve-devel/20231211144721.212071-1-f.gleu...@proxmox.com/
PVE/Service/pvestatd.pm | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/PVE/Service/pvestatd.pm b/PVE/Service/pvestatd.pm
index 7fa003fe..03c578e1 100755
--- a/PVE/Service/pvestatd.pm
+++ b/PVE/Service/pvestatd.pm
@@ -527,7 +527,10 @@ sub update_sdn_status {
my $broadcast_version_info_done = 0;
my sub broadcast_version_info : prototype() {
-if (!$broadcast_version_info_done) {
+if (
+ !$broadcast_version_info_done
+ || !keys PVE::Cluster::get_node_kv('version-info', $nodename)->%*
+) {
PVE::Cluster::broadcast_node_kv(
'version-info',
encode_json(PVE::pvecfg::version_info()),
--
2.39.5
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH manager] pvestatd: improve broadcast of node version-info
On 2025-01-16 17:35, Christian Ebner wrote: On 1/16/25 17:30, Aaron Lauterer wrote: […] This will close issue 5894 I guess [0]? [0] https://bugzilla.proxmox.com/show_bug.cgi?id=5894 Specifically, the 'version-info', yes. Are there other properties too? ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH manager] pvestatd: improve broadcast of node version-info
On 1/16/25 17:30, Aaron Lauterer wrote:
Until now, the pvestatd did broadcast the pve-manager version only once
after startup of the service. But there are some situations, where the
local pmxcfs (pve-cluster) restarts and loses that information.
Basically everytime we restart the pmxcfs without restarting pvestatd
too.
For example, on a cluster join, or if the pmxcfs has been restarted
manually.
By additionally checking if the local kv-store of the pmxcfs has any
version info for the node, we can decide if another broadcast is
necessary.
Therefore after the next run of pvestatd, we should have the full
version info available again.
Signed-off-by: Aaron Lauterer
---
This patch is preparation to get reliable version infos as I am picking
of the patch series of Folke to include more metrics into the RRD data
and summary graphs. [0]
This was a big blocker and now with the major version change coming up,
we at least can assume the latest 8.x installed as part of the update to
PVE 9.
Therefore, we should get this in with PVE 8. Additional patches for PVE
8 will follow to make the transition smoother. But as mentioned, this
here is one of the things that needs to work reliably, which is why I
submit the patch already now.
[0]
https://lore.proxmox.com/pve-devel/20231211144721.212071-1-f.gleu...@proxmox.com/
PVE/Service/pvestatd.pm | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/PVE/Service/pvestatd.pm b/PVE/Service/pvestatd.pm
index 7fa003fe..03c578e1 100755
--- a/PVE/Service/pvestatd.pm
+++ b/PVE/Service/pvestatd.pm
@@ -527,7 +527,10 @@ sub update_sdn_status {
my $broadcast_version_info_done = 0;
my sub broadcast_version_info : prototype() {
-if (!$broadcast_version_info_done) {
+if (
+ !$broadcast_version_info_done
+ || !keys PVE::Cluster::get_node_kv('version-info', $nodename)->%*
+) {
PVE::Cluster::broadcast_node_kv(
'version-info',
encode_json(PVE::pvecfg::version_info()),
This will close issue 5894 I guess [0]?
[0] https://bugzilla.proxmox.com/show_bug.cgi?id=5894
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH manager] pvestatd: improve broadcast of node version-info
On 1/16/25 17:38, Aaron Lauterer wrote: On 2025-01-16 17:35, Christian Ebner wrote: On 1/16/25 17:30, Aaron Lauterer wrote: […] This will close issue 5894 I guess [0]? [0] https://bugzilla.proxmox.com/show_bug.cgi?id=5894 Specifically, the 'version-info', yes. Are there other properties too? If I remember correctly all are cleared. But only the `version-info` is not re-broadcasted as far as I can see from a quick glance at the code. But might be worth to double check! ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH v3 qemu-server 08/11] blockdev: convert drive_mirror to blockdev_mirror
--- Begin Message ---
>>Yes, we don't need much to get enough collision-resistance. Just
>>wanted
>>to make sure and check it explicitly.
I have done some test with sha1, with base62 encode ('0..9', 'A..Z',
'a..z)
the node-name require to start with an alpha character prefix
encodebase62(sha1("$volid-$snapid") : 5zU4nVxN7gIUWMaskKc4y6EawWu
28characters
so we have space to prefix like for example:
f-5zU4nVxN7gIUWMaskKc4y6EawWu for fmt-node
e-5zU4nVxN7gIUWMaskKc4y6EawWu for file-node
sub encode_base62 {
my ($input) = @_;
my @chars = ('0'..'9', 'A'..'Z', 'a'..'z');
my $base = 62;
my $value = 0;
foreach my $byte (unpack('C*', $input)) {
$value = $value * 256 + $byte;
}
my $result = '';
while ($value > 0) {
$result = $chars[$value % $base] . $result;
$value = int($value / $base);
}
return $result || '0';
}
--- End Message ---
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH v2 qemu-server, manager 00/12] bugzilla #4225 -- improve handling of unavailable ISOs
On 1/13/25 09:55, Daniel Herzig wrote: This patch series addresses bugzilla entry #4225.> Currently VMs refuse to to start if a configured isofile becomes unavailable, be it a deleted file or an unavailable network storage. This patch series introduces a new parameter in Drive.pm, called 'required'. Depending on whether this parameter is set or not, the situation will be handled differently. Thanks for tackling this, this is a great addition to the CDROM drives! I'm looking forward to this being merged as it happens quite frequently when setting up a lot of VMs with different ISOs! If the parameter is set to 0, the configuration will temporarily changed to use 'none' as file for the cd drive, which allows qemu to start up the machine. The configuration is not changed in this process to avoid unexpected behaviour. Instead a log_warn will be issued. For transition reasons an unset parameter acts like 'required=1'. In this case the startup process will die earlier than currently, if the file is missing or the underlying storage not available. Hm, I have discussed with Friedrich about this off-list, because I'm thinking about "optional" being another name for this flag, since it should be required by default for VMs that are not explicitly setting this option, i.e. `optional=0`, and if someone sets it explicitly to `optional=1` the CDROM can be ignored if it is non-existent. I think this could also simplify the logic overall, but it depends on how we want to present this to users (i.e. the WebGUI). Are there reasons against this? What do you think? If however a new VM is created from the WebGUI, the corresponding added checkbox is not checked by default, and the resulting 'required=0' will be written to the configuration. IMO, I also think that new VMs should be set to `required=0` by default, but this change should probably be postponed to 9.0 as it would break the current WebGUI "user-API". To allow for proper testing and building some additions and minor changes where made to to the testing framework as well. Not exactly part of #4225, but related to it, this patch series adds an 'Eject' button to the hardwareview in the WebGUI, which can be used as a convenience shortcut to manually editing the missing ISO file to 'Do not use any media'. In this case it is better to move unrelated changes into a separate patch series, so they can be reviewed on their own :). This series supersedes: https://lore.proxmox.com/pve-devel/20241025150243.134466-1-d.her...@proxmox.com/ Changes from initial series: * rebased onto current master * fix cifs mocking in run_config2command_tests.pl * fix expected outcome in ide-required.conf.cmd qemu-server: Daniel Herzig (9): fix #4225: qemuserver: drive: add optional required parameter qemuserver: add helper function for mocking files fix #4225: qemuserver: add function to eject isofiles test: chomp all trailing newlines from errors and warnings test: mock cifs-store test: add nfs-offline storage test: mock existing files test: mock log_warn warnings test: cfg2cmd: add tests for testing the iso required parameter PVE/QemuServer.pm | 44 +++ PVE/QemuServer/Drive.pm | 9 +++- test/cfg2cmd/ide-required-iso-missing.conf| 12 + .../cfg2cmd/ide-required-iso-missing.conf.cmd | 0 .../cfg2cmd/ide-required-iso-offline-nfs.conf | 12 + .../ide-required-iso-offline-nfs.conf.cmd | 0 test/cfg2cmd/ide-required.conf| 14 ++ test/cfg2cmd/ide-required.conf.cmd| 39 test/cfg2cmd/ide-unrequired-iso-missing.conf | 12 + .../ide-unrequired-iso-missing.conf.cmd | 33 ++ .../ide-unrequired-iso-offline-nfs.conf | 12 + .../ide-unrequired-iso-offline-nfs.conf.cmd | 33 ++ test/run_config2command_tests.pl | 44 ++- 13 files changed, 261 insertions(+), 3 deletions(-) create mode 100644 test/cfg2cmd/ide-required-iso-missing.conf create mode 100644 test/cfg2cmd/ide-required-iso-missing.conf.cmd create mode 100644 test/cfg2cmd/ide-required-iso-offline-nfs.conf create mode 100644 test/cfg2cmd/ide-required-iso-offline-nfs.conf.cmd create mode 100644 test/cfg2cmd/ide-required.conf create mode 100644 test/cfg2cmd/ide-required.conf.cmd create mode 100644 test/cfg2cmd/ide-unrequired-iso-missing.conf create mode 100644 test/cfg2cmd/ide-unrequired-iso-missing.conf.cmd create mode 100644 test/cfg2cmd/ide-unrequired-iso-offline-nfs.conf create mode 100644 test/cfg2cmd/ide-unrequired-iso-offline-nfs.conf.cmd manager: Daniel Herzig (3): fix #4225: ui: form: isoselector: add optional required checkbox fix #4225: ui: qemu: cdedit: enable required checkbox for isos ui: qemu: hardware: add eject button for cdroms www/manager6/form/IsoSelector.js | 22 www/manager6/qemu/CDEdit.js |
Re: [pve-devel] [PATCH v2 3/12] fix #4225: qemuserver: add function to eject isofiles
On 1/13/25 09:55, Daniel Herzig wrote:
Current behaviour prevents a VM from starting, if an ISO file defined
in the configuration becomes unavailable.
The function eject_nonrequired_isos checks on whether a cdrom drive is
marked as 'required' or not. If the parameter 'required' is not
defined, it will assume 'required' to be true and keep the current
behaviour.
If 'required' is set to 0, the function 'ejects' the ISO file by
setting the drive's file value to 'none', if the underlying storage is
unavailable or if the defined file is unavailable for another reason.
The function is called while config_to_command iterates over all
volumes to allow for early storage activation and early exit in the
case of missing required files.
Signed-off-by: Daniel Herzig
---
PVE/QemuServer.pm | 39 +++
1 file changed, 39 insertions(+)
diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
index d07c170e..f72878d3 100644
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -4041,6 +4041,8 @@ sub config_to_command {
PVE::QemuConfig->foreach_volume($conf, sub {
my ($ds, $drive) = @_;
+ eject_nonrequired_isos($ds, $drive, $vmid, $storecfg, $conf);
+
This change will unfortunately make two config2cmd test cases fail and
therefore the build process will also fail. It is important that the
package can be built at each individual commit so to make the package
bisectable.
IMO this patch could be split into "introduce eject_norequired_isos" and
patches #4-#9 could be squashed and put together with adding
"eject_nonrequired_isos" to config_to_command in the same patch.
Therefore someone reviewing (now or in the future) and know what tests
needed to be added/changed when adding this function call to
config_to_command.
if (PVE::Storage::parse_volume_id($drive->{file}, 1)) {
check_volume_storage_type($storecfg, $drive->{file});
push @$vollist, $drive->{file};
@@ -8999,6 +9001,43 @@ sub delete_ifaces_ipams_ips {
}
}
+sub eject_nonrequired_isos {
+my ($ds, $drive, $vmid, $storecfg, $conf) = @_;
+# set 1 to exclude cloudinit. cloudinit isos are always required.
+if (drive_is_cdrom($drive, 1)
+ && $drive->{file} ne 'none'
+ && $drive->{file} ne 'cdrom') {
nit: IMO, this could be an early return:
return if !drive_is_cdrom($drive, 1);
return if $drive->{file} eq 'none' || $drive->{file} eq 'cdrom';
So that we can reduce the following to only 2 indentation levels.
+ $drive->{required} = 1 if !defined($drive->{required});
+ my $iso_volid = $drive->{file};
+ my $iso_path = get_iso_path($storecfg, $vmid, $drive->{file});
nit: third argument could be $iso_volid
+ my $store_err;
+ if ($iso_volid !~ m|^/|) {
+ my $iso_storage = PVE::Storage::parse_volume_id($iso_volid, 1);
+ eval { PVE::Storage::activate_storage($storecfg, $iso_storage); };
+ $store_err = $@;
+ }
+ if ($store_err) {
+ if ($drive->{required}) {
+ die "cannot access required file: '${ds}: ${iso_volid}':
${store_err}\n";
+ } else {
+ log_warn("eject '${ds}: ${iso_volid}': ${store_err}");
+ $drive->{file} = 'none';
+ $conf->{$ds} = print_drive($drive);
+ }
+ } else {
+ if (!file_exists($iso_path)) {
+ if ($drive->{required}) {
+ die "required file does not exist: '${ds}: ${iso_volid}'\n";
+ } else {
+ log_warn("eject '${ds}: ${iso_volid}': file does not
exist");
+ $drive->{file} = 'none';
+ $conf->{$ds} = print_drive($drive);
+ }
+ }
+ }
nit: the logic between an unavailable storage and an unavailable ISO
image are very similar (both `$drive->{required} && $store_err` as well
as `$drive->{required} && !file_exists($iso_path)` have the same exit
control path), so we could simplify this e.g. to this (changes the
warning message to a generic message for unavailable storages too):
if ($drive->{required}) {
die "cannot access required file: '${ds}: ${iso_volid}':
${store_err}\n" if $store_err;
die "required file does not exist: '${ds}: ${iso_volid}'\n" if
!file_exists($iso_path);
}
return if !$store_err && file_exists($iso_path);
log_warn("eject '${ds}: ${iso_volid}': storage unavailable or file does
not exist");
$drive->{file} = 'none';
$conf->{$ds} = print_drive($drive);
+}
+}
+
sub file_exists {
my $file_path = shift;
return -e $file_path
Else consider this:
Reviewed-by: Daniel Kral
Tested-by: Daniel Kral
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH v2 11/12] fix #4225: ui: qemu: cdedit: enable required checkbox for isos
On 1/13/25 09:56, Daniel Herzig wrote: Enables the 'required' checkbox for the IsoSelector. If the parameter is not set, the backend will use the default (set to 1). Behaviour: * Only send parameter if not default (required=0) * Checked if parameter is missing (default) * Unchecked when adding a new CD-ROM IMO the part of this patch where new VMs get created with `required=0` CDROMs should be split into its own patch and marked as "for-9.0" or else add a TODO comment for the 9.0 release. Else this looks good, so consider this: Reviewed-by: Daniel Kral Tested-by: Daniel Kral ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH v2 2/12] qemuserver: add helper function for mocking files
On 1/13/25 09:55, Daniel Herzig wrote:
This stub function can be used for mocking a file's existance in testruns.
Signed-off-by: Daniel Herzig
---
PVE/QemuServer.pm | 5 +
1 file changed, 5 insertions(+)
diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
index 5cde94a1..d07c170e 100644
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -8999,4 +8999,9 @@ sub delete_ifaces_ipams_ips {
}
}
+sub file_exists {
+my $file_path = shift;
+return -e $file_path
+}
+
1;
nit: I think this could be squashed into patch #3.
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH v2 12/12] ui: qemu: hardware: add eject button for cdroms
On 1/13/25 09:56, Daniel Herzig wrote: Eject by setting file to none. That's a great addition to the WebGUI! One small thing: What about also allow ejecting physical CDROMs? :) Else consider this: Reviewed-by: Daniel Kral Tested-by: Daniel Kral ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH proxmox-ve-rs] config: fix a few clippy warnings
.. as newly introduced with 1.84.
Namely `elided_named_lifetimes` for the change of `SdnConfig` and
`clippy::needless_lifetimes` for the rest.
Signed-off-by: Christoph Heiss
---
proxmox-ve-config/src/firewall/parse.rs | 6 +++---
proxmox-ve-config/src/sdn/config.rs | 2 +-
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/proxmox-ve-config/src/firewall/parse.rs
b/proxmox-ve-config/src/firewall/parse.rs
index 7bf00c0..8cf4757 100644
--- a/proxmox-ve-config/src/firewall/parse.rs
+++ b/proxmox-ve-config/src/firewall/parse.rs
@@ -316,7 +316,7 @@ pub mod serde_option_log_ratelimit {
#[derive(Clone, Copy, Debug)]
pub struct SomeStrDeserializer<'a, E>(serde::de::value::StrDeserializer<'a,
E>);
-impl<'de, 'a, E> serde::de::Deserializer<'de> for SomeStrDeserializer<'a, E>
+impl<'de, E> serde::de::Deserializer<'de> for SomeStrDeserializer<'_, E>
where
E: serde::de::Error,
{
@@ -379,7 +379,7 @@ impl<'a> From<&'a str> for SomeStr<'a> {
}
}
-impl<'de, 'a, E> serde::de::IntoDeserializer<'de, E> for SomeStr<'a>
+impl<'a, E> serde::de::IntoDeserializer<'_, E> for SomeStr<'a>
where
E: serde::de::Error,
{
@@ -465,7 +465,7 @@ impl From for SomeString {
}
}
-impl<'de, E> serde::de::IntoDeserializer<'de, E> for SomeString
+impl serde::de::IntoDeserializer<'_, E> for SomeString
where
E: serde::de::Error,
{
diff --git a/proxmox-ve-config/src/sdn/config.rs
b/proxmox-ve-config/src/sdn/config.rs
index 7ee1101..880efc2 100644
--- a/proxmox-ve-config/src/sdn/config.rs
+++ b/proxmox-ve-config/src/sdn/config.rs
@@ -544,7 +544,7 @@ impl SdnConfig {
pub fn ipsets<'a>(
&'a self,
filter: Option<&'a Allowlist>,
-) -> impl Iterator + '_ {
+) -> impl Iterator + 'a {
self.zones
.values()
.flat_map(|zone| zone.vnets())
--
2.47.0
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
