[pve-devel] [PATCH 1/3] enable x2apic by default for kvm machines

[Dietmar Maurer]

Please can you cleanup this patch (comments inline)?

>  PVE/QemuServer.pm |   25 +++++++++++++++----------
>  1 file changed, 15 insertions(+), 10 deletions(-)
> 
> diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm index
> 31d8103..196ad0a 100644
> --- a/PVE/QemuServer.pm
> +++ b/PVE/QemuServer.pm
> @@ -2225,6 +2225,7 @@ sub config_to_command {
>      my $globalFlags = [];
>      my $machineFlags = [];
>      my $rtcFlags = [];
> +    my $cpuFlags = [];
>      my $devices = [];
>      my $pciaddr = '';
>      my $bridges = {};
> @@ -2313,16 +2314,6 @@ sub config_to_command {
> 
>      push @$cmd, '-name', $vmname;
> 
> -    my $sockets = 1;
> -    $sockets = $conf->{smp} if $conf->{smp}; # old style - no longer iused
> -    $sockets = $conf->{sockets} if  $conf->{sockets};
> -
> -    my $cores = $conf->{cores} || 1;
> -
> -    push @$cmd, '-smp', "sockets=$sockets,cores=$cores";

nothing changed here, so why do you replace this?

> -
> -    push @$cmd, '-cpu', $conf->{cpu} if $conf->{cpu};
> -
>      push @$cmd, '-nodefaults';
> 
>      my $bootorder = $conf->{boot} || $confdesc->{boot}->{default}; @@ -
> 2395,6 +2386,20 @@ sub config_to_command {
>  	push @$rtcFlags, 'base=localtime';
>      }
> 
> +    my $sockets = 1;
> +    $sockets = $conf->{smp} if $conf->{smp}; # old style - no longer iused
> +    $sockets = $conf->{sockets} if  $conf->{sockets};
> +
> +    my $cores = $conf->{cores} || 1;
> +    push @$cmd, '-smp', "sockets=$sockets,cores=$cores";
> +
> +    my $cpu = $nokvm ? "qemu64":"kvm64";

coding style - we want white space like:

     my $cpu = $nokvm ? "qemu64" : "kvm64";

> +    $cpu = $conf->{cpu} if $conf->{cpu};
> +
> +    push @$cpuFlags , '+x2apic' if !$nokvm;
> +
> +    push @$cmd, '-cpu', $cpu.",".join(',', @$cpuFlags);

This looks wrong, because we can end up with "-cpu qemu64," (trailing comma).

> +
>      push @$cmd, '-S' if $conf->{freeze};
> 
>      # set keyboard layout
> --
> 1.7.10.4
> 
> _______________________________________________
> pve-devel mailing list
> pve-devel at pve.proxmox.com
> http://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel