From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by lists.proxmox.com (Postfix) with ESMTPS id 001D39F10 for ; Thu, 7 Sep 2023 09:57:15 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id CD54731E6 for ; Thu, 7 Sep 2023 09:57:15 +0200 (CEST) Received: from proxmox-new.maurer-it.com (proxmox-new.maurer-it.com [94.136.29.106]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by firstgate.proxmox.com (Proxmox) with ESMTPS for ; Thu, 7 Sep 2023 09:57:14 +0200 (CEST) Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1]) by proxmox-new.maurer-it.com (Proxmox) with ESMTP id 0C8D942CA0 for ; Thu, 7 Sep 2023 09:57:14 +0200 (CEST) Message-ID: <199095f9-902e-0a32-897d-56eadb8a2eb4@proxmox.com> Date: Thu, 7 Sep 2023 09:57:09 +0200 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.15.0 To: Proxmox VE development discussion , Markus Frank References: <20230822124041.119554-1-m.frank@proxmox.com> <20230822124041.119554-2-m.frank@proxmox.com> Content-Language: en-US From: Fiona Ebner In-Reply-To: <20230822124041.119554-2-m.frank@proxmox.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-SPAM-LEVEL: Spam detection results: 0 AWL 0.657 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment NICE_REPLY_A -1.473 Looks like a legit reply (A) SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Subject: Re: [pve-devel] [PATCH qemu-server v6 1/4] feature #3784: Parameter for guest vIOMMU & machine as property-string X-BeenThere: pve-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox VE development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 07 Sep 2023 07:57:16 -0000 Am 22.08.23 um 14:40 schrieb Markus Frank: > diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm > index bf1de17..013792d 100644 > --- a/PVE/QemuServer.pm > +++ b/PVE/QemuServer.pm > @@ -118,12 +118,32 @@ PVE::JSONSchema::register_standard_option('pve-qm-stateuri', { > optional => 1, > }); > > -PVE::JSONSchema::register_standard_option('pve-qemu-machine', { > +my $machine_fmt = { > + type => { > + default_key => 1, > description => "Specifies the QEMU machine type.", > type => 'string', > pattern => '(pc|pc(-i440fx)?-\d+(\.\d+)+(\+pve\d+)?(\.pxe)?|q35|pc-q35-\d+(\.\d+)+(\+pve\d+)?(\.pxe)?|virt(?:-\d+(\.\d+)+)?(\+pve\d+)?)', > maxLength => 40, > + format_description => 'machine type', > optional => 1, > + }, > + viommu => { > + type => 'boolean', > + description => "Enable/disable guest vIOMMU" > + ." (needs kvm to be enabled and q35 to be set as machine type).", > + default => 0, > + optional => 1, > + }, > +}; > + > +PVE::JSONSchema::register_format('pve-qemu-machine-fmt', $machine_fmt); > + > +PVE::JSONSchema::register_standard_option('pve-qemu-machine', { > + description => "Specify the QEMU machine type & enable/disable vIOMMU.", > + type => 'string', > + optional => 1, > + format => PVE::JSONSchema::get_format('pve-qemu-machine-fmt'), > }); > > # FIXME: remove in favor of just using the INotify one, it's cached there exactly the same way > @@ -2133,6 +2153,31 @@ sub parse_watchdog { > return $res; > } > > +sub parse_machine { > + my ($value) = @_; > + > + return if !$value; > + > + my $res = parse_property_string($machine_fmt, $value); > + return $res; > +} > + > +sub check_machine_config { > + my ($conf, $machine_conf) = @_; > + my $q35 = $machine_conf->{type} && ($machine_conf->{type} =~ m/q35/) ? 1 : 0; > + my $kvm = $conf->{kvm}; > + my $arch = get_vm_arch($conf); > + $kvm //= 1 if is_native($arch); > + if ($machine_conf->{viommu} && (!$kvm || !$q35)) { > + die "to use vIOMMU please enable kvm and set the machine type to q35\n"; > + } > +} > + > +sub print_machine { > + my ($machine_conf) = @_; > + return PVE::JSONSchema::print_property_string($machine_conf, $machine_fmt); > +} > + Was there any reason why you didn't put the above, i.e. format definition/parsing/printing in the QemuServer::Machine module instead like I suggested in my review of v5? > diff --git a/PVE/QemuServer/Machine.pm b/PVE/QemuServer/Machine.pm > index d9429ed..bfbde59 100644 > --- a/PVE/QemuServer/Machine.pm > +++ b/PVE/QemuServer/Machine.pm > @@ -15,7 +15,8 @@ our $PVE_MACHINE_VERSION = { > sub machine_type_is_q35 { > my ($conf) = @_; > > - return $conf->{machine} && ($conf->{machine} =~ m/q35/) ? 1 : 0; > + my $machine_conf = PVE::QemuServer::parse_machine($conf->{machine}); > + return $machine_conf->{type} && ($machine_conf->{type} =~ m/q35/) ? 1 : 0; > } > > sub current_from_query_machines { > @@ -120,7 +121,8 @@ sub qemu_machine_pxe { > > my $machine = get_current_qemu_machine($vmid); > > - if ($conf->{machine} && $conf->{machine} =~ m/\.pxe$/) { > + my $machine_conf = PVE::QemuServer::parse_machine($conf->{machine}); > + if ($machine_conf->{type} && $machine_conf->{type} =~ m/\.pxe$/) { > $machine .= '.pxe'; > } > Because there still are these cyclic calls back into the QemuServer module (and you would need the cylcic use statement which is what we want to avoid).