[PATCH-SERIES qemu-server 0/3] ovmf: create temporary EFI disk below /run instead of /tmp to avoid migration issue

[PATCH-SERIES qemu-server 0/3] ovmf: create temporary EFI disk below /run instead of /tmp to avoid migration issue

[Fiona Ebner]

As reported in the enterprise support, QEMU might fail to inactivate
the block node for the temporary EFI disk during migration:

> kvm: migration_block_inactivate: bdrv_inactivate_all() failed: -1
> kvm: Error in migration completion: Bad address

The issue occurs when the file for the temporary EFI disk has been
removed from the file system. This happens on new installations since
Debian Trixie [0], where files in /tmp are regularly cleaned.

[0]: https://www.debian.org/releases/trixie/release-notes/issues.html#the-directories-tmp-and-var-tmp-are-now-regularly-cleaned

qemu-server:

Fiona Ebner (3):
  d/tmpfiles: add configuration to auto-create /run/qemu-server
    directory
  ovmf: use file module directly for file_copy()
  ovmf: create temporary EFI disk below /run instead of /tmp to avoid
    migration issue

 debian/tmpfiles                                    |  3 +++
 src/PVE/QemuServer/Helpers.pm                      |  2 ++
 src/PVE/QemuServer/OVMF.pm                         | 10 ++++++----
 src/test/cfg2cmd/efi-ovmf-without-efidisk.conf.cmd |  2 +-
 src/test/run_config2command_tests.pl               | 12 ++++++++++++
 5 files changed, 24 insertions(+), 5 deletions(-)
 create mode 100644 debian/tmpfiles


Summary over all repositories:
  5 files changed, 24 insertions(+), 5 deletions(-)

-- 
Generated by git-murpp 0.5.0

[PATCH qemu-server 1/3] d/tmpfiles: add configuration to auto-create /run/qemu-server directory

[Fiona Ebner]

Some code paths use /run/qemu-server, some code paths use
/var/run/qemu-server. Nothing currently ensures that the former
directory exists, but usually, /var/run is a symlink to /run, so it
does work in practice. The latter directory is created in the
Helpers.pm module. Usage of these paths should be aligned for the next
major release.

If /var/run is a symlink to /run like usual, then this change makes
the permissions tighter, because the directory /var/run/qemu-server
was created with 0755 permissions by the Helpers.pm module and is now
created via the tmpfiles configuration. The permissions chosen are
0750 with user root and group www-data. This is in line with what
/run/pve-cluster and /run/pve have.

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
 debian/tmpfiles               | 2 ++
 src/PVE/QemuServer/Helpers.pm | 2 ++
 2 files changed, 4 insertions(+)
 create mode 100644 debian/tmpfiles

diff --git a/debian/tmpfiles b/debian/tmpfiles
new file mode 100644
index 00000000..3bbe100c
--- /dev/null
+++ b/debian/tmpfiles
@@ -0,0 +1,2 @@
+#Type Path             Mode User Group     Age Argument
+d     /run/qemu-server 0750 root www-data  -   -
diff --git a/src/PVE/QemuServer/Helpers.pm b/src/PVE/QemuServer/Helpers.pm
index 65f4ce5f..dd17eef5 100644
--- a/src/PVE/QemuServer/Helpers.pm
+++ b/src/PVE/QemuServer/Helpers.pm
@@ -81,6 +81,8 @@ sub kvm_user_version {
 
 # Paths and directories
 
+# FIXME: MAJOR VERSION: use /run/qemu-server everywhere instead of mixing /run and /var/run and rely
+# on debian/tmpfiles config to create the directory.
 our $var_run_tmpdir = "/var/run/qemu-server";
 mkdir $var_run_tmpdir;
 
-- 
2.47.3

[PATCH qemu-server 2/3] ovmf: use file module directly for file_copy()

[Fiona Ebner]

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
 src/PVE/QemuServer/OVMF.pm | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/PVE/QemuServer/OVMF.pm b/src/PVE/QemuServer/OVMF.pm
index 13951ed8..f2faeb27 100644
--- a/src/PVE/QemuServer/OVMF.pm
+++ b/src/PVE/QemuServer/OVMF.pm
@@ -129,7 +129,7 @@ my sub print_ovmf_drive_commandlines {
     } else {
         log_warn("no efidisk configured! Using temporary efivars disk.");
         my $path = "/tmp/$vmid-ovmf.fd";
-        PVE::Tools::file_copy($ovmf_vars, $path, $ovmf_vars_size);
+        PVE::File::file_copy($ovmf_vars, $path, $ovmf_vars_size);
         $var_drive_str .= ",format=raw,file=$path";
         $var_drive_str .= ",size=" . $ovmf_vars_size if $version_guard->(4, 1, 2);
     }
@@ -221,7 +221,7 @@ my sub generate_ovmf_blockdev {
     } else {
         log_warn("no efidisk configured! Using temporary efivars disk.");
         my $path = "/tmp/$vmid-ovmf.fd";
-        PVE::Tools::file_copy($ovmf_vars, $path, file_get_size($ovmf_vars));
+        PVE::File::file_copy($ovmf_vars, $path, file_get_size($ovmf_vars));
         $drive = { file => $path, interface => 'efidisk', index => 0 };
         $format = 'raw';
     }
-- 
2.47.3

[PATCH qemu-server 3/3] ovmf: create temporary EFI disk below /run instead of /tmp to avoid migration issue

[Fiona Ebner]

As reported in the enterprise support, QEMU might fail to inactivate
the block node for the temporary EFI disk during migration:

> kvm: migration_block_inactivate: bdrv_inactivate_all() failed: -1
> kvm: Error in migration completion: Bad address

The issue occurs when the file for the temporary EFI disk has been
removed from the file system. This happens on new installations since
Debian Trixie [0], where files in /tmp are regularly cleaned.

Requires mocking file_copy() since unprivileged users are not allowed
to actually do the copying of the EFI vars to create the temporary EFI
disk. But there is no need to create the temporary test for the test
anyways.

[0]: https://www.debian.org/releases/trixie/release-notes/issues.html#the-directories-tmp-and-var-tmp-are-now-regularly-cleaned

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
 debian/tmpfiles                                    |  5 +++--
 src/PVE/QemuServer/OVMF.pm                         |  6 ++++--
 src/test/cfg2cmd/efi-ovmf-without-efidisk.conf.cmd |  2 +-
 src/test/run_config2command_tests.pl               | 12 ++++++++++++
 4 files changed, 20 insertions(+), 5 deletions(-)

diff --git a/debian/tmpfiles b/debian/tmpfiles
index 3bbe100c..978205f3 100644
--- a/debian/tmpfiles
+++ b/debian/tmpfiles
@@ -1,2 +1,3 @@
-#Type Path             Mode User Group     Age Argument
-d     /run/qemu-server 0750 root www-data  -   -
+#Type Path                     Mode User Group     Age Argument
+d     /run/qemu-server         0750 root www-data  -   -
+d     /run/qemu-server/efidisk 0750 root www-data  -   -
diff --git a/src/PVE/QemuServer/OVMF.pm b/src/PVE/QemuServer/OVMF.pm
index f2faeb27..7a765fad 100644
--- a/src/PVE/QemuServer/OVMF.pm
+++ b/src/PVE/QemuServer/OVMF.pm
@@ -55,6 +55,8 @@ my $OVMF = {
     },
 };
 
+my $temporary_efidisk_dir = '/run/qemu-server/efidisk';
+
 my sub get_ovmf_files($$$$) {
     my ($arch, $efidisk, $smm, $cvm_type) = @_;
 
@@ -128,7 +130,7 @@ my sub print_ovmf_drive_commandlines {
         $var_drive_str .= ',readonly=on' if $readonly;
     } else {
         log_warn("no efidisk configured! Using temporary efivars disk.");
-        my $path = "/tmp/$vmid-ovmf.fd";
+        my $path = "${temporary_efidisk_dir}/${vmid}-ovmf.fd";
         PVE::File::file_copy($ovmf_vars, $path, $ovmf_vars_size);
         $var_drive_str .= ",format=raw,file=$path";
         $var_drive_str .= ",size=" . $ovmf_vars_size if $version_guard->(4, 1, 2);
@@ -220,7 +222,7 @@ my sub generate_ovmf_blockdev {
         }
     } else {
         log_warn("no efidisk configured! Using temporary efivars disk.");
-        my $path = "/tmp/$vmid-ovmf.fd";
+        my $path = "${temporary_efidisk_dir}/${vmid}-ovmf.fd";
         PVE::File::file_copy($ovmf_vars, $path, file_get_size($ovmf_vars));
         $drive = { file => $path, interface => 'efidisk', index => 0 };
         $format = 'raw';
diff --git a/src/test/cfg2cmd/efi-ovmf-without-efidisk.conf.cmd b/src/test/cfg2cmd/efi-ovmf-without-efidisk.conf.cmd
index 44362710..fdeb8ba5 100644
--- a/src/test/cfg2cmd/efi-ovmf-without-efidisk.conf.cmd
+++ b/src/test/cfg2cmd/efi-ovmf-without-efidisk.conf.cmd
@@ -11,7 +11,7 @@
   -smbios 'type=1,uuid=7b10d7af-b932-4c66-b2c3-3996152ec465' \
   -object '{"id":"throttle-drive-efidisk0","limits":{},"qom-type":"throttle-group"}' \
   -blockdev '{"driver":"raw","file":{"driver":"file","filename":"/usr/share/pve-edk2-firmware//OVMF_CODE.fd"},"node-name":"pflash0","read-only":true}' \
-  -blockdev '{"detect-zeroes":"on","discard":"ignore","driver":"throttle","file":{"cache":{"direct":false,"no-flush":false},"detect-zeroes":"on","discard":"ignore","driver":"raw","file":{"aio":"io_uring","cache":{"direct":false,"no-flush":false},"detect-zeroes":"on","discard":"ignore","driver":"file","filename":"/tmp/8006-ovmf.fd","node-name":"e5b5f7a29888341a35f0f1428e70ba5","read-only":false},"node-name":"f5b5f7a29888341a35f0f1428e70ba5","read-only":false,"size":131072},"node-name":"drive-efidisk0","read-only":false,"throttle-group":"throttle-drive-efidisk0"}' \
+  -blockdev '{"detect-zeroes":"on","discard":"ignore","driver":"throttle","file":{"cache":{"direct":false,"no-flush":false},"detect-zeroes":"on","discard":"ignore","driver":"raw","file":{"aio":"io_uring","cache":{"direct":false,"no-flush":false},"detect-zeroes":"on","discard":"ignore","driver":"file","filename":"/run/qemu-server/efidisk/8006-ovmf.fd","node-name":"e5ecb54a3863bf2f22d662c53f49ac4","read-only":false},"node-name":"f5ecb54a3863bf2f22d662c53f49ac4","read-only":false,"size":131072},"node-name":"drive-efidisk0","read-only":false,"throttle-group":"throttle-drive-efidisk0"}' \
   -smp '1,sockets=1,cores=1,maxcpus=1' \
   -nodefaults \
   -boot 'menu=on,strict=on,reboot-timeout=1000,splash=/usr/share/qemu-server/bootsplash.jpg' \
diff --git a/src/test/run_config2command_tests.pl b/src/test/run_config2command_tests.pl
index 3c4a695c..ebe0dca1 100755
--- a/src/test/run_config2command_tests.pl
+++ b/src/test/run_config2command_tests.pl
@@ -420,6 +420,18 @@ $pve_common_tools->mock(
     },
 );
 
+my $pve_common_file;
+$pve_common_file = Test::MockModule->new('PVE::File');
+$pve_common_file->mock(
+    file_copy => sub {
+        my ($filename, $dst, $max, $perm) = @_;
+        if ($dst =~ m|^/run/qemu-server/efidisk|) {
+            return;
+        }
+        return $pve_common_file->original('file_copy')->($filename, $dst, $max, $perm);
+    },
+);
+
 my $pve_cpuconfig;
 $pve_cpuconfig = Test::MockModule->new('PVE::QemuServer::CPUConfig');
 $pve_cpuconfig->mock(
-- 
2.47.3