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 838B16B0C8 for ; Mon, 8 Mar 2021 13:57:15 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 79A3B1E81B for ; Mon, 8 Mar 2021 13:57:15 +0100 (CET) Received: from proxmox-new.maurer-it.com (proxmox-new.maurer-it.com [212.186.127.180]) (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 id 58C881E80F for ; Mon, 8 Mar 2021 13:57:14 +0100 (CET) Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1]) by proxmox-new.maurer-it.com (Proxmox) with ESMTP id 2070B4581E for ; Mon, 8 Mar 2021 13:57:14 +0100 (CET) To: Stefan Reiter , Proxmox VE development discussion References: <20210301155327.24841-1-f.ebner@proxmox.com> <6d7ed73f-3c13-ccf0-2f52-01db26457eec@proxmox.com> From: Fabian Ebner Message-ID: <7dfcd614-289c-1364-3571-895cd640cffb@proxmox.com> Date: Mon, 8 Mar 2021 13:57:13 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.8.0 MIME-Version: 1.0 In-Reply-To: <6d7ed73f-3c13-ccf0-2f52-01db26457eec@proxmox.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 8bit X-SPAM-LEVEL: Spam detection results: 0 AWL 0.001 Adjusted score from AWL reputation of From: address KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment NICE_REPLY_A -0.001 Looks like a legit reply (A) RCVD_IN_DNSWL_MED -2.3 Sender listed at https://www.dnswl.org/, medium trust 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 1/3] machine: split out helper for handling query-machines qmp command result 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: Mon, 08 Mar 2021 12:57:15 -0000 On 04.03.21 13:51, Stefan Reiter wrote: > LGTM, for both qemu-server patches: > > Reviewed-by: Stefan Reiter > > Not sure about the formatting in the GUI, but I'm also the wrong person > to ask there. Maybe don't capitalize "Qemu", as we also don't do that > for "running"/"stopped"/... either? > Thanks for the review! I agree that the "Qemu" looks a bit out of place, and even with lower-case "qemu" it doesn't feel completely right to me now (using "QEMU" feels slightly better). I'll send a v2 with the version as a separate field, hopefully that's better. > On 01/03/2021 16:53, Fabian Ebner wrote: >> to be re-used in the vmstatus() call. >> >> Signed-off-by: Fabian Ebner >> --- >>   PVE/QemuServer/Machine.pm | 16 +++++++++++----- >>   1 file changed, 11 insertions(+), 5 deletions(-) >> >> diff --git a/PVE/QemuServer/Machine.pm b/PVE/QemuServer/Machine.pm >> index c168ade..2474951 100644 >> --- a/PVE/QemuServer/Machine.pm >> +++ b/PVE/QemuServer/Machine.pm >> @@ -18,11 +18,8 @@ sub machine_type_is_q35 { >>       return $conf->{machine} && ($conf->{machine} =~ m/q35/) ? 1 : 0; >>   } >> -# this only works if VM is running >> -sub get_current_qemu_machine { >> -    my ($vmid) = @_; >> - >> -    my $res = PVE::QemuServer::Monitor::mon_cmd($vmid, >> 'query-machines'); >> +sub current_from_query_machines { >> +    my ($res) = @_; >>       my ($current, $pve_version, $default); >>       foreach my $e (@$res) { >> @@ -37,6 +34,15 @@ sub get_current_qemu_machine { >>       return $current || $default || 'pc'; >>   } >> +# this only works if VM is running >> +sub get_current_qemu_machine { >> +    my ($vmid) = @_; >> + >> +    my $res = PVE::QemuServer::Monitor::mon_cmd($vmid, >> 'query-machines'); >> + >> +    return current_from_query_machines($res); >> +} >> + >>   # returns a string with major.minor+pve, patch version-part >> is ignored >>   # as it's seldom ressembling a real QEMU machine type, so it would >> be '0' 99% of >>   # the time anyway.. This explicitly separates pveversion from the >> machine. >>