[pve-devel] [PATCH manager] pve5to6: list vms with active vmx/svm flag

[Thomas Lamprecht]

On 7/1/19 12:39 PM, Dominik Csapak wrote:
> list all vms with either max/host cputype or vmx/svm explicitely set
> (this can only happen in the args)
> 
> give a general warning if no is found at the moment
> 
> Signed-off-by: Dominik Csapak <d.csapak at proxmox.com>
> ---
>  PVE/CLI/pve5to6.pm | 40 +++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 39 insertions(+), 1 deletion(-)
> 
> diff --git a/PVE/CLI/pve5to6.pm b/PVE/CLI/pve5to6.pm
> index 440c58e6..7fccbd1b 100644
> --- a/PVE/CLI/pve5to6.pm
> +++ b/PVE/CLI/pve5to6.pm
> @@ -16,6 +16,7 @@ use PVE::JSONSchema;
>  use PVE::RPCEnvironment;
>  use PVE::Storage;
>  use PVE::Tools;
> +use PVE::QemuServer;
>  
>  use Term::ANSIColor;
>  
> @@ -153,6 +154,40 @@ sub check_pve_packages {
>      }
>  }
>  
> +sub warn_vms_with_vmx {
> +    my $found= 0;
> +    my $vmlist = PVE::QemuServer::vzlist();
> +    # sort numeric
> +    foreach my $vmid ( sort { $a - $b } keys %$vmlist ) {

nit: perl-style is normally $a <=> $b and I would omit the commit, just noise.

> +	my $pid = $vmlist->{$vmid}->{pid};
> +	next if !$pid; # skip not running vms
> +	my $cmdline = eval { PVE::Tools::file_get_contents("/proc/$pid/cmdline") };
> +	if ($cmdline) {
> +	    my @args = split(/\0/, $cmdline);
> +	    for (my $i = 0; $i < scalar(@args); $i++) {
> +		next if !$args[$i] || $args[$i] !~ m/^-?-cpu$/;
> +		my $cpuarg = $args[$i+1];
> +		my $reason;
> +		my $spaces = " "x(9-length($vmid));
> +		if ($cpuarg =~ m/^(host|max)/) {
> +		    $reason = "VM $vmid$spaces has cpu type $1";
> +		} elsif ($cpuarg =~ m/\+(vmx|svm)/) {
> +		    $reason = "VM $vmid$spaces has $1 enabled";
> +		}

maybe have a "result" or "vmx_enabled" hash where you save the VMID
grouped by reason, then we could output those together and save a few lines
if quite a few VMs run currently as host - as then on gets a high count of
warning, which then may result in the user overlooking another warning
more easily, which I'd like to avoid.

push @{$vmx_enabled->{$1}}, vmid;

where $1 is the reason (cpu_type and flags?)

and then a combined log for each of the reasons:

log_warn("The following guests have nested virtualization enabled, this will block live migration:\n" .
         "  Due to using 'host' or 'max' as CPU type: " . join(" ", ...) . "\n" .
         "  Due to explictily requesting the vmx/svm flag: " . join(...



> +
> +		if ($reason) {
> +		    if (!$found) {
> +			log_warn("It will not be possible to live migrate the following guests to PVE 6:");
> +			$found = 1;
> +		    }
> +		    log_warn($reason);
> +		}
> +	    }
> +	}
> +    }
> +    return $found;
> +}
> +
>  sub check_kvm_nested {
>      my $module_sysdir = "/sys/module";
>      if (-e "$module_sysdir/kvm_amd") {
> @@ -167,7 +202,10 @@ sub check_kvm_nested {
>      if (-f "$module_sysdir/nested") {
>  	my $val = eval { PVE::Tools::file_read_firstline("$module_sysdir/nested") };
>  	if ($val && $val =~ m/Y|1/) {
> -	    log_warn("KVM nested parameter set. VMs with vmx/svm flag will not be able to live migrate to PVE 6.");
> +	    log_warn("KVM nested parameter set.");
> +	    if (!warn_vms_with_vmx()) {
> +		log_warn("VMs with cputype host/max or vmx/svm flag will not be able to live migrate to PVE 6.");

So you still warn if no VMs are running with the vmx/svm flag?
May just log it as skip? Which tells the user about the check but
also that it's not applicable to them at the moment?

> +	    }
>  	} else {
>  	    log_pass("KVM nested parameter not set.")
>  	}
>