public inbox for pve-devel@lists.proxmox.com
 help / color / mirror / Atom feed
From: Fiona Ebner <f.ebner@proxmox.com>
To: pve-devel@lists.proxmox.com
Subject: [pve-devel] [PATCH qemu-server 3/6] introduce dedicated cfg2cmd module
Date: Wed, 24 Sep 2025 16:35:25 +0200	[thread overview]
Message-ID: <20250924143611.166858-4-f.ebner@proxmox.com> (raw)
In-Reply-To: <20250924143611.166858-1-f.ebner@proxmox.com>

Having a dedicated Cfg2Cmd class allows having a cleaner interface by
only calling into pre-defined methods. Important, global information
about the VM like machine type or OS version will be recorded by the
object and can be queried via methods. For now, there is only
windows_version(). There will be sub-classes, each concerning a
dedicated part of the configuration. The first one is for the timer.

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
 src/PVE/QemuServer.pm               | 32 +++-------
 src/PVE/QemuServer/Cfg2Cmd.pm       | 91 +++++++++++++++++++++++++++++
 src/PVE/QemuServer/Cfg2Cmd/Timer.pm | 37 ++++++++++++
 src/PVE/QemuServer/Makefile         |  2 +
 4 files changed, 137 insertions(+), 25 deletions(-)
 create mode 100644 src/PVE/QemuServer/Cfg2Cmd.pm
 create mode 100644 src/PVE/QemuServer/Cfg2Cmd/Timer.pm

diff --git a/src/PVE/QemuServer.pm b/src/PVE/QemuServer.pm
index f7f85436..b9030137 100644
--- a/src/PVE/QemuServer.pm
+++ b/src/PVE/QemuServer.pm
@@ -56,6 +56,7 @@ use PVE::QemuMigrate::Helpers;
 use PVE::QemuServer::Agent qw(qga_check_running);
 use PVE::QemuServer::Blockdev;
 use PVE::QemuServer::BlockJob;
+use PVE::QemuServer::Cfg2Cmd;
 use PVE::QemuServer::Helpers
     qw(config_aware_timeout get_iscsi_initiator_name min_version kvm_user_version windows_version);
 use PVE::QemuServer::Cloudinit;
@@ -3573,31 +3574,12 @@ sub config_to_command {
         push @$cmd, '-nographic';
     }
 
-    # time drift fix
-    my $tdf = defined($conf->{tdf}) ? $conf->{tdf} : $defaults->{tdf};
-    my $useLocaltime = $conf->{localtime};
-
-    if ($winversion >= 5) { # windows
-        $useLocaltime = 1 if !defined($conf->{localtime});
-
-        # use time drift fix when acpi is enabled
-        if (!(defined($conf->{acpi}) && $conf->{acpi} == 0)) {
-            $tdf = 1 if !defined($conf->{tdf});
-        }
-    }
-
-    if ($winversion >= 6) {
-        push $cmd->@*, '-global', 'kvm-pit.lost_tick_policy=discard';
-        push @$machineFlags, 'hpet=off';
-    }
-
-    push @$rtcFlags, 'driftfix=slew' if $tdf;
-
-    if ($conf->{startdate} && $conf->{startdate} ne 'now') {
-        push @$rtcFlags, "base=$conf->{startdate}";
-    } elsif ($useLocaltime) {
-        push @$rtcFlags, 'base=localtime';
-    }
+    # For now, handles only specific parts, but the final goal is to cover everything.
+    my $cfg2cmd = PVE::QemuServer::Cfg2Cmd->new($conf, $defaults);
+    my $generated = $cfg2cmd->generate();
+    push $cmd->@*, '-global', $_ for ($generated->global_flags() // [])->@*;
+    push $machineFlags->@*, ($generated->machine_flags() // [])->@*;
+    push $rtcFlags->@*, ($generated->rtc_flags() // [])->@*;
 
     if ($forcecpu) {
         push @$cmd, '-cpu', $forcecpu;
diff --git a/src/PVE/QemuServer/Cfg2Cmd.pm b/src/PVE/QemuServer/Cfg2Cmd.pm
new file mode 100644
index 00000000..6b26ab23
--- /dev/null
+++ b/src/PVE/QemuServer/Cfg2Cmd.pm
@@ -0,0 +1,91 @@
+package PVE::QemuServer::Cfg2Cmd;
+
+use warnings;
+use strict;
+
+use PVE::QemuServer::Cfg2Cmd::Timer;
+use PVE::QemuServer::Helpers;
+
+sub new {
+    my ($class, $conf, $defaults) = @_;
+
+    my $self = bless {
+        conf => $conf,
+        defaults => $defaults,
+    }, $class;
+
+    my $ostype = $self->get_prop('ostype');
+    $self->{'windows-version'} = PVE::QemuServer::Helpers::windows_version($ostype);
+
+    return $self;
+}
+
+=head3 get_prop
+
+    my $value = $self->get_prop($prop);
+
+Return the configured value for the property C<$prop>. If no fallback to the default value should be
+made, use C<$only_explicit>. Note that any such usage is likely an indication that the default value
+is not actually a static default, but that the default depends on context.
+
+=cut
+
+sub get_prop {
+    my ($self, $prop, $only_explicit) = @_;
+
+    my ($conf, $defaults) = $self->@{qw(conf defaults)};
+    return $conf->{$prop} if $only_explicit;
+    return defined($conf->{$prop}) ? $conf->{$prop} : $defaults->{$prop};
+}
+
+sub add_global_flag {
+    my ($self, $flag) = @_;
+
+    push $self->{'global-flags'}->@*, $flag;
+}
+
+sub global_flags {
+    my ($self) = @_;
+
+    return $self->{'global-flags'};
+}
+
+sub add_machine_flag {
+    my ($self, $flag) = @_;
+
+    push $self->{'machine-flags'}->@*, $flag;
+}
+
+sub machine_flags {
+    my ($self) = @_;
+
+    return $self->{'machine-flags'};
+}
+
+sub add_rtc_flag {
+    my ($self, $flag) = @_;
+
+    push $self->{'rtc-flags'}->@*, $flag;
+}
+
+sub rtc_flags {
+    my ($self) = @_;
+
+    return $self->{'rtc-flags'};
+}
+
+sub windows_version {
+    my ($self) = @_;
+
+    return $self->{'windows-version'};
+}
+
+sub generate {
+    my ($self) = @_;
+
+    PVE::QemuServer::Cfg2Cmd::Timer::generate($self);
+
+    return $self;
+}
+
+1;
diff --git a/src/PVE/QemuServer/Cfg2Cmd/Timer.pm b/src/PVE/QemuServer/Cfg2Cmd/Timer.pm
new file mode 100644
index 00000000..971ec6a3
--- /dev/null
+++ b/src/PVE/QemuServer/Cfg2Cmd/Timer.pm
@@ -0,0 +1,37 @@
+package PVE::QemuServer::Cfg2Cmd::Timer;
+
+use warnings;
+use strict;
+
+sub generate {
+    my ($self) = @_;
+
+    my $time_drift_fix = $self->get_prop('tdf', 1);
+    my $acpi = $self->get_prop('acpi');
+    my $localtime = $self->get_prop('localtime', 1);
+    my $startdate = $self->get_prop('startdate');
+
+    if ($self->windows_version() >= 5) { # windows
+        $localtime = 1 if !defined($localtime);
+
+        # use time drift fix when acpi is enabled, but prefer explicitly set value
+        $time_drift_fix = 1 if $acpi && !defined($time_drift_fix);
+    }
+
+    if ($self->windows_version() >= 6) {
+        $self->add_global_flag('kvm-pit.lost_tick_policy=discard');
+        $self->add_machine_flag('hpet=off');
+    }
+
+    $self->add_rtc_flag('driftfix=slew') if $time_drift_fix;
+
+    if ($startdate ne 'now') {
+        $self->add_rtc_flag("base=$startdate");
+    } elsif ($localtime) {
+        $self->add_rtc_flag('base=localtime');
+    }
+
+    return;
+}
+
+1;
diff --git a/src/PVE/QemuServer/Makefile b/src/PVE/QemuServer/Makefile
index 23c136bc..df56cffa 100644
--- a/src/PVE/QemuServer/Makefile
+++ b/src/PVE/QemuServer/Makefile
@@ -5,6 +5,7 @@ PERLDIR=$(PREFIX)/share/perl5
 SOURCES=Agent.pm	\
 	Blockdev.pm	\
 	BlockJob.pm	\
+	Cfg2Cmd.pm	\
 	CGroup.pm	\
 	Cloudinit.pm	\
 	CPUConfig.pm	\
@@ -30,3 +31,4 @@ SOURCES=Agent.pm	\
 .PHONY: install
 install: $(SOURCES)
 	for i in $(SOURCES); do install -D -m 0644 $$i $(DESTDIR)$(PERLDIR)/PVE/QemuServer/$$i; done
+	$(MAKE) -C Cfg2Cmd install
-- 
2.47.3



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


  parent reply	other threads:[~2025-09-24 14:36 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-09-24 14:35 [pve-devel] [PATCH-SERIES qemu-server 0/5] cfg2cmd: introduce dedicated module and turn off hpet timer Fiona Ebner
2025-09-24 14:35 ` [pve-devel] [PATCH qemu-server 1/6] tests: cfg2cmd: add tests for startdate parameter and disabled time drift fix Fiona Ebner
2025-09-24 14:35 ` [pve-devel] [PATCH qemu-server 2/6] tests: add tests for non-{Linux, Windows} OS types Fiona Ebner
2025-09-24 14:35 ` Fiona Ebner [this message]
2025-09-24 14:35 ` [pve-devel] [PATCH qemu-server 4/6] config: schema: define default OS type Fiona Ebner
2025-09-24 14:35 ` [pve-devel] [PATCH qemu-server 5/6] cfg2cmd: turn off hpet for Linux VMs running at least kernel 2.6 and machine type >= 10.1 Fiona Ebner
2025-09-24 14:35 ` [pve-devel] [PATCH qemu-server 6/6] tests: cfg2cmd: regenerate with QEMU 10.1 binary Fiona Ebner

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20250924143611.166858-4-f.ebner@proxmox.com \
    --to=f.ebner@proxmox.com \
    --cc=pve-devel@lists.proxmox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal