[pve-devel] [PATCH qemu-server] cfg2cmd: turn smm off when SeaBIOS and serial display are used

[pve-devel] [PATCH qemu-server] cfg2cmd: turn smm off when SeaBIOS and serial display are used

[Fabian Ebner]

Since commit 277d33454f77ec1d1e0bc04e37621e4dd2424b67 in pve-qemu,
smm=off is no longer the default, but with SeaBIOS and serial display,
this can lead to a boot loop.

Reported in the community forum [0] and reproduced with a Debian 10
VM.

[0]: https://forum.proxmox.com/threads/pve-7-0-all-vms-with-cloud-init-seabios-fail-during-boot-process-bootloop-disk-not-found.97310/post-427129

Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
---
 PVE/QemuServer.pm                    | 12 ++++++++++
 test/cfg2cmd/seabios_serial.conf     | 16 ++++++++++++++
 test/cfg2cmd/seabios_serial.conf.cmd | 33 ++++++++++++++++++++++++++++
 3 files changed, 61 insertions(+)
 create mode 100644 test/cfg2cmd/seabios_serial.conf
 create mode 100644 test/cfg2cmd/seabios_serial.conf.cmd

diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
index 763c412..9b76512 100644
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -3403,6 +3403,16 @@ my sub get_cpuunits {
     my ($conf) = @_;
     return $conf->{cpuunits} // (PVE::CGroup::cgroup_mode() == 2 ? 100 : 1024);
 }
+
+# Since commit 277d33454f77ec1d1e0bc04e37621e4dd2424b67 in pve-qemu, smm is not off by default
+# anymore. But smm=off seems to be required when using SeaBIOS and serial display.
+my sub should_disable_smm {
+    my ($conf, $vga) = @_;
+
+    return (!defined($conf->{bios}) || $conf->{bios} eq 'seabios') &&
+	$vga->{type} && $vga->{type} =~ m/^serial\d+$/;
+}
+
 sub config_to_command {
     my ($storecfg, $vmid, $conf, $defaults, $forcemachine, $forcecpu,
         $pbs_backing) = @_;
@@ -4002,6 +4012,8 @@ sub config_to_command {
 	push @$machineFlags, 'accel=tcg';
     }
 
+    push @$machineFlags, 'smm=off' if should_disable_smm($conf, $vga);
+
     my $machine_type_min = $machine_type;
     if ($add_pve_version) {
 	$machine_type_min =~ s/\+pve\d+$//;
diff --git a/test/cfg2cmd/seabios_serial.conf b/test/cfg2cmd/seabios_serial.conf
new file mode 100644
index 0000000..7ebfa50
--- /dev/null
+++ b/test/cfg2cmd/seabios_serial.conf
@@ -0,0 +1,16 @@
+# TEST: Test for smm-related regression with SeaBIOS and serial display
+bootdisk: scsi0
+cores: 3
+ide2: none,media=cdrom
+memory: 768
+name: seabiosserial
+net0: virtio=A2:C0:43:77:08:A0,bridge=vmbr0
+numa: 0
+ostype: l26
+scsi0: local:8006/vm-8006-disk-0.qcow2,discard=on,size=104858K
+scsihw: virtio-scsi-pci
+serial0: socket
+smbios1: uuid=7b10d7af-b932-4c66-b2c3-3996152ec465
+sockets: 1
+vga: serial0
+vmgenid: c773c261-d800-4348-9f5d-167fadd53cf8
diff --git a/test/cfg2cmd/seabios_serial.conf.cmd b/test/cfg2cmd/seabios_serial.conf.cmd
new file mode 100644
index 0000000..63a3524
--- /dev/null
+++ b/test/cfg2cmd/seabios_serial.conf.cmd
@@ -0,0 +1,33 @@
+/usr/bin/kvm \
+  -id 8006 \
+  -name seabiosserial \
+  -no-shutdown \
+  -chardev 'socket,id=qmp,path=/var/run/qemu-server/8006.qmp,server=on,wait=off' \
+  -mon 'chardev=qmp,mode=control' \
+  -chardev 'socket,id=qmp-event,path=/var/run/qmeventd.sock,reconnect=5' \
+  -mon 'chardev=qmp-event,mode=control' \
+  -pidfile /var/run/qemu-server/8006.pid \
+  -daemonize \
+  -smbios 'type=1,uuid=7b10d7af-b932-4c66-b2c3-3996152ec465' \
+  -smp '3,sockets=1,cores=3,maxcpus=3' \
+  -nodefaults \
+  -boot 'menu=on,strict=on,reboot-timeout=1000,splash=/usr/share/qemu-server/bootsplash.jpg' \
+  -nographic \
+  -cpu kvm64,enforce,+kvm_pv_eoi,+kvm_pv_unhalt,+lahf_lm,+sep \
+  -m 768 \
+  -device 'pci-bridge,id=pci.1,chassis_nr=1,bus=pci.0,addr=0x1e' \
+  -device 'pci-bridge,id=pci.2,chassis_nr=2,bus=pci.0,addr=0x1f' \
+  -device 'vmgenid,guid=c773c261-d800-4348-9f5d-167fadd53cf8' \
+  -device 'piix3-usb-uhci,id=uhci,bus=pci.0,addr=0x1.0x2' \
+  -chardev 'socket,id=serial0,path=/var/run/qemu-server/8006.serial0,server=on,wait=off' \
+  -device 'isa-serial,chardev=serial0' \
+  -device 'virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3' \
+  -iscsi 'initiator-name=iqn.1993-08.org.debian:01:aabbccddeeff' \
+  -drive 'if=none,id=drive-ide2,media=cdrom,aio=io_uring' \
+  -device 'ide-cd,bus=ide.1,unit=0,drive=drive-ide2,id=ide2,bootindex=200' \
+  -device 'virtio-scsi-pci,id=scsihw0,bus=pci.0,addr=0x5' \
+  -drive 'file=/var/lib/vz/images/8006/vm-8006-disk-0.qcow2,if=none,id=drive-scsi0,discard=on,format=qcow2,cache=none,aio=io_uring,detect-zeroes=unmap' \
+  -device 'scsi-hd,bus=scsihw0.0,channel=0,scsi-id=0,lun=0,drive=drive-scsi0,id=scsi0,bootindex=100' \
+  -netdev 'type=tap,id=net0,ifname=tap8006i0,script=/var/lib/qemu-server/pve-bridge,downscript=/var/lib/qemu-server/pve-bridgedown,vhost=on' \
+  -device 'virtio-net-pci,mac=A2:C0:43:77:08:A0,netdev=net0,bus=pci.0,addr=0x12,id=net0,bootindex=300' \
+  -machine 'smm=off,type=pc+pve0'
-- 
2.30.2

Re: [pve-devel] [PATCH qemu-server] cfg2cmd: turn smm off when SeaBIOS and serial display are used

[Thomas Lamprecht]

On 05.11.21 14:06, Fabian Ebner wrote:
> Since commit 277d33454f77ec1d1e0bc04e37621e4dd2424b67 in pve-qemu,
> smm=off is no longer the default, but with SeaBIOS and serial display,
> this can lead to a boot loop.
> 
> Reported in the community forum [0] and reproduced with a Debian 10
> VM.
> 
> [0]: https://forum.proxmox.com/threads/pve-7-0-all-vms-with-cloud-init-seabios-fail-during-boot-process-bootloop-disk-not-found.97310/post-427129
> 
> Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
> ---
>  PVE/QemuServer.pm                    | 12 ++++++++++
>  test/cfg2cmd/seabios_serial.conf     | 16 ++++++++++++++
>  test/cfg2cmd/seabios_serial.conf.cmd | 33 ++++++++++++++++++++++++++++
>  3 files changed, 61 insertions(+)
>  create mode 100644 test/cfg2cmd/seabios_serial.conf
>  create mode 100644 test/cfg2cmd/seabios_serial.conf.cmd
> 
> diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
> index 763c412..9b76512 100644
> --- a/PVE/QemuServer.pm
> +++ b/PVE/QemuServer.pm
> @@ -3403,6 +3403,16 @@ my sub get_cpuunits {
>      my ($conf) = @_;
>      return $conf->{cpuunits} // (PVE::CGroup::cgroup_mode() == 2 ? 100 : 1024);
>  }
> +
> +# Since commit 277d33454f77ec1d1e0bc04e37621e4dd2424b67 in pve-qemu, smm is not off by default
> +# anymore. But smm=off seems to be required when using SeaBIOS and serial display.
> +my sub should_disable_smm {
> +    my ($conf, $vga) = @_;
> +
> +    return (!defined($conf->{bios}) || $conf->{bios} eq 'seabios') &&
> +	$vga->{type} && $vga->{type} =~ m/^serial\d+$/;
> +}
> +
>  sub config_to_command {
>      my ($storecfg, $vmid, $conf, $defaults, $forcemachine, $forcecpu,
>          $pbs_backing) = @_;
> @@ -4002,6 +4012,8 @@ sub config_to_command {
>  	push @$machineFlags, 'accel=tcg';
>      }
>  
> +    push @$machineFlags, 'smm=off' if should_disable_smm($conf, $vga);

doesn't that breaks live migration or do we know that it could never work with smm=on
under seabios + display=serial, so that there can no running VM that could be live
migrated?

Re: [pve-devel] [PATCH qemu-server] cfg2cmd: turn smm off when SeaBIOS and serial display are used

[Fabian Ebner]

Am 05.11.21 um 14:12 schrieb Thomas Lamprecht:
> On 05.11.21 14:06, Fabian Ebner wrote:
>> Since commit 277d33454f77ec1d1e0bc04e37621e4dd2424b67 in pve-qemu,
>> smm=off is no longer the default, but with SeaBIOS and serial display,
>> this can lead to a boot loop.
>>
>> Reported in the community forum [0] and reproduced with a Debian 10
>> VM.
>>
>> [0]: https://forum.proxmox.com/threads/pve-7-0-all-vms-with-cloud-init-seabios-fail-during-boot-process-bootloop-disk-not-found.97310/post-427129
>>
>> Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
>> ---
>>   PVE/QemuServer.pm                    | 12 ++++++++++
>>   test/cfg2cmd/seabios_serial.conf     | 16 ++++++++++++++
>>   test/cfg2cmd/seabios_serial.conf.cmd | 33 ++++++++++++++++++++++++++++
>>   3 files changed, 61 insertions(+)
>>   create mode 100644 test/cfg2cmd/seabios_serial.conf
>>   create mode 100644 test/cfg2cmd/seabios_serial.conf.cmd
>>
>> diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
>> index 763c412..9b76512 100644
>> --- a/PVE/QemuServer.pm
>> +++ b/PVE/QemuServer.pm
>> @@ -3403,6 +3403,16 @@ my sub get_cpuunits {
>>       my ($conf) = @_;
>>       return $conf->{cpuunits} // (PVE::CGroup::cgroup_mode() == 2 ? 100 : 1024);
>>   }
>> +
>> +# Since commit 277d33454f77ec1d1e0bc04e37621e4dd2424b67 in pve-qemu, smm is not off by default
>> +# anymore. But smm=off seems to be required when using SeaBIOS and serial display.
>> +my sub should_disable_smm {
>> +    my ($conf, $vga) = @_;
>> +
>> +    return (!defined($conf->{bios}) || $conf->{bios} eq 'seabios') &&
>> +	$vga->{type} && $vga->{type} =~ m/^serial\d+$/;
>> +}
>> +
>>   sub config_to_command {
>>       my ($storecfg, $vmid, $conf, $defaults, $forcemachine, $forcecpu,
>>           $pbs_backing) = @_;
>> @@ -4002,6 +4012,8 @@ sub config_to_command {
>>   	push @$machineFlags, 'accel=tcg';
>>       }
>>   
>> +    push @$machineFlags, 'smm=off' if should_disable_smm($conf, $vga);
> 
> doesn't that breaks live migration or do we know that it could never work with smm=on
> under seabios + display=serial, so that there can no running VM that could be live
> migrated?
> 

Stefan tested migration in both direction when the pve-qemu patch was 
dropped [0], but he did sound more confident about the smm=off -> smm=on 
direction. And now the scenario Wolfgang mentioned would actually be 
relevant, because we can get into a situation where smm=off -> smm=on -> 
smm=off by migrating from older -> old -> new. I'll test around some 
more and see if I can break it.

[0]: https://lists.proxmox.com/pipermail/pve-devel/2021-August/049778.html

Re: [pve-devel] [PATCH qemu-server] cfg2cmd: turn smm off when SeaBIOS and serial display are used

[Fabian Ebner]

Am 05.11.21 um 15:47 schrieb Fabian Ebner:
> Am 05.11.21 um 14:12 schrieb Thomas Lamprecht:
>> On 05.11.21 14:06, Fabian Ebner wrote:
>>> Since commit 277d33454f77ec1d1e0bc04e37621e4dd2424b67 in pve-qemu,
>>> smm=off is no longer the default, but with SeaBIOS and serial display,
>>> this can lead to a boot loop.
>>>
>>> Reported in the community forum [0] and reproduced with a Debian 10
>>> VM.
>>>
>>> [0]: 
>>> https://forum.proxmox.com/threads/pve-7-0-all-vms-with-cloud-init-seabios-fail-during-boot-process-bootloop-disk-not-found.97310/post-427129 
>>>
>>>
>>> Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
>>> ---
>>>   PVE/QemuServer.pm                    | 12 ++++++++++
>>>   test/cfg2cmd/seabios_serial.conf     | 16 ++++++++++++++
>>>   test/cfg2cmd/seabios_serial.conf.cmd | 33 ++++++++++++++++++++++++++++
>>>   3 files changed, 61 insertions(+)
>>>   create mode 100644 test/cfg2cmd/seabios_serial.conf
>>>   create mode 100644 test/cfg2cmd/seabios_serial.conf.cmd
>>>
>>> diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
>>> index 763c412..9b76512 100644
>>> --- a/PVE/QemuServer.pm
>>> +++ b/PVE/QemuServer.pm
>>> @@ -3403,6 +3403,16 @@ my sub get_cpuunits {
>>>       my ($conf) = @_;
>>>       return $conf->{cpuunits} // (PVE::CGroup::cgroup_mode() == 2 ? 
>>> 100 : 1024);
>>>   }
>>> +
>>> +# Since commit 277d33454f77ec1d1e0bc04e37621e4dd2424b67 in pve-qemu, 
>>> smm is not off by default
>>> +# anymore. But smm=off seems to be required when using SeaBIOS and 
>>> serial display.
>>> +my sub should_disable_smm {
>>> +    my ($conf, $vga) = @_;
>>> +
>>> +    return (!defined($conf->{bios}) || $conf->{bios} eq 'seabios') &&
>>> +    $vga->{type} && $vga->{type} =~ m/^serial\d+$/;
>>> +}
>>> +
>>>   sub config_to_command {
>>>       my ($storecfg, $vmid, $conf, $defaults, $forcemachine, $forcecpu,
>>>           $pbs_backing) = @_;
>>> @@ -4002,6 +4012,8 @@ sub config_to_command {
>>>       push @$machineFlags, 'accel=tcg';
>>>       }
>>> +    push @$machineFlags, 'smm=off' if should_disable_smm($conf, $vga);
>>
>> doesn't that breaks live migration or do we know that it could never 
>> work with smm=on
>> under seabios + display=serial, so that there can no running VM that 
>> could be live
>> migrated?

No, we can't say that. I found that the same configuration does work 
with CentOS 7.

>>
> 
> Stefan tested migration in both direction when the pve-qemu patch was 
> dropped [0], but he did sound more confident about the smm=off -> smm=on 
> direction. And now the scenario Wolfgang mentioned would actually be 
> relevant, because we can get into a situation where smm=off -> smm=on -> 
> smm=off by migrating from older -> old -> new. I'll test around some 
> more and see if I can break it.
> 
> [0]: https://lists.proxmox.com/pipermail/pve-devel/2021-August/049778.html
> 

With the following nodes:

N1: pve-qemu 6.0.0-3 -> smm=off
N2: pve-qemu 6.1.0-1 -> smm=on
N3: patched qemu-server to always turn smm off -> smm=off

I tested:

1. Migration N1 -> N2
2. soft reboot
3. migration N2 -> N3
4. soft reboot

For Windows 10 and CentOS 7 everything seemed to work. I didn't test 
Windows with serial output as I don't know how to set that up.

Still, the same might not be true for more complex (virtual) hardware 
setups.

For the problematic Debian 10 VM, step 2 exhibits the problem, i.e. 
leading to a boot loop. When I skipped step 2, it seemed to work too.

If there is a problem with live migration smm=on -> smm=off, it's at 
least not obvious and would only affect VMs running pve-qemu >= 6.0.0-4 
(and SeaBIOS with serial display). On the other hand, the patch does fix 
a user-reported regression from pve-qemu 6.0.0-4 (although it's also not 
clear how widespread it is).

> 
> 
> _______________________________________________
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
> 
> 

[pve-devel] applied: [PATCH qemu-server] cfg2cmd: turn smm off when SeaBIOS and serial display are used

[Thomas Lamprecht]

On 05.11.21 14:06, Fabian Ebner wrote:
> Since commit 277d33454f77ec1d1e0bc04e37621e4dd2424b67 in pve-qemu,
> smm=off is no longer the default, but with SeaBIOS and serial display,
> this can lead to a boot loop.
> 
> Reported in the community forum [0] and reproduced with a Debian 10
> VM.
> 
> [0]: https://forum.proxmox.com/threads/pve-7-0-all-vms-with-cloud-init-seabios-fail-during-boot-process-bootloop-disk-not-found.97310/post-427129
> 
> Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
> ---
>  PVE/QemuServer.pm                    | 12 ++++++++++
>  test/cfg2cmd/seabios_serial.conf     | 16 ++++++++++++++
>  test/cfg2cmd/seabios_serial.conf.cmd | 33 ++++++++++++++++++++++++++++
>  3 files changed, 61 insertions(+)
>  create mode 100644 test/cfg2cmd/seabios_serial.conf
>  create mode 100644 test/cfg2cmd/seabios_serial.conf.cmd
> 
>

applied, thanks!