On Wed, Aug 16, 2017 at 06:24:29PM +0200, Philip Abernethy wrote:
> Where possible I reworked the code to remove the resulting inversion.
> The result should be easier to read and understand.
small nitpick: commit messages should be written in the present tense /
imperative, not past. phrases like "I implemented", "I reworked", etc
are also redundant.
e.g.:
refactor $nokvm to $kvm
for improved readability and consistency with the option name.
> I also fixed a type in the process.
s/type/typo
but see comment inline, maybe that one could be refactored (and
typo-fixed) in a separate, first commit?
> ---
> PVE/QemuServer.pm | 24 ++++++++++++------------
> 1 file changed, 12 insertions(+), 12 deletions(-)
>
> diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
> index 1f34101..88b1c5b 100644
> --- a/PVE/QemuServer.pm
> +++ b/PVE/QemuServer.pm
> @@ -1740,8 +1740,8 @@ sub print_netdev_full {
> sub print_cpu_device {
> my ($conf, $id) = @_;
>
> - my $nokvm = defined($conf->{kvm}) && $conf->{kvm} == 0 ? 1 : 0;
> - my $cpu = $nokvm ? "qemu64" : "kvm64";
> + my $kvm = $conf->{kvm} // 1;
> + my $cpu = $kvm ? "kvm64" : "qemu64";
> if (my $cputype = $conf->{cpu}) {
> my $cpuconf = PVE::JSONSchema::parse_property_string($cpu_fmt, $cputype)
> or die "Cannot parse cpu description: $cputype\n";
> @@ -2835,6 +2835,7 @@ sub config_to_command {
> my $vernum = 0; # unknown
> my $ostype = $conf->{ostype};
> my $winversion = windows_version($ostype);
> + my $kvm = $conf->{kvm} // 1;
>
> if ($kvmver =~ m/^(\d+)\.(\d+)$/) {
> $vernum = $1*1000000+$2*1000;
> @@ -3085,7 +3086,6 @@ sub config_to_command {
> # time drift fix
> my $tdf = defined($conf->{tdf}) ? $conf->{tdf} : $defaults->{tdf};
>
> - my $nokvm = defined($conf->{kvm}) && $conf->{kvm} == 0 ? 1 : 0;
> my $useLocaltime = $conf->{localtime};
>
> if ($winversion >= 5) { # windows
> @@ -3104,7 +3104,7 @@ sub config_to_command {
>
> push @$rtcFlags, 'driftfix=slew' if $tdf;
>
> - if ($nokvm) {
> + if (!$kvm) {
> push @$machineFlags, 'accel=tcg';
> } else {
> die "No accelerator found!\n" if !$cpuinfo->{hvm};
> @@ -3120,7 +3120,7 @@ sub config_to_command {
> push @$rtcFlags, 'base=localtime';
> }
>
> - my $cpu = $nokvm ? "qemu64" : "kvm64";
> + my $cpu = $kvm ? "kvm64" : "qemu64";
> if (my $cputype = $conf->{cpu}) {
> my $cpuconf = PVE::JSONSchema::parse_property_string($cpu_fmt, $cputype)
> or die "Cannot parse cpu description: $cputype\n";
> @@ -3139,13 +3139,13 @@ sub config_to_command {
>
> if (qemu_machine_feature_enabled ($machine_type, $kvmver, 2, 3)) {
>
> - push @$cpuFlags , '+kvm_pv_unhalt' if !$nokvm;
> - push @$cpuFlags , '+kvm_pv_eoi' if !$nokvm;
> + push @$cpuFlags , '+kvm_pv_unhalt' if $kvm;
> + push @$cpuFlags , '+kvm_pv_eoi' if $kvm;
> }
>
> - add_hyperv_enlighments($cpuFlags, $winversion, $machine_type, $kvmver,
> $nokvm, $conf->{bios}, $gpu_passthrough);
> + add_hyperv_enlightenments($cpuFlags, $winversion, $machine_type,
> $kvmver, $kvm, $conf->{bios}, $gpu_passthrough);
(I know this isn't originally your code, but since you are touching it)
maybe we could drop the $nokvm/$kvm parameter altogether, and
conditionally call it instead? return if $nokvm is the first line in the
method after all, and there is only one call site.
"if $kvm" is only 1 char longer than ", $kvm" ;)
>
> - push @$cpuFlags, 'enforce' if $cpu ne 'host' && !$nokvm;
> + push @$cpuFlags, 'enforce' if $cpu ne 'host' && $kvm;
>
> push @$cpuFlags, 'kvm=off' if $kvm_off;
>
> @@ -6359,10 +6359,10 @@ sub scsihw_infos {
> return ($maxdev, $controller, $controller_prefix);
> }
>
> -sub add_hyperv_enlighments {
> - my ($cpuFlags, $winversion, $machine_type, $kvmver, $nokvm, $bios,
> $gpu_passthrough) = @_;
> +sub add_hyperv_enlightenments {
> + my ($cpuFlags, $winversion, $machine_type, $kvmver, $kvm, $bios,
> $gpu_passthrough) = @_;
>
> - return if $nokvm;
> + return if !$kvm;
> return if $winversion < 6;
> return if $bios && $bios eq 'ovmf' && $winversion < 8;
>
> --
> 2.11.0
>
>
> _______________________________________________
> pve-devel mailing list
> pve-devel@pve.proxmox.com
> https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
_______________________________________________
pve-devel mailing list
pve-devel@pve.proxmox.com
https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
