[pve-devel] [PATCH v4 qemu-server 5/8] refactor: extract QEMU machine related helpers to package

[Thomas Lamprecht]

On 11/19/19 12:23 PM, Stefan Reiter wrote:
> ...PVE::QemuServer::Machine.
> 
> qemu_machine_feature_enabled is exported since it has a *lot* of users
> in PVE::QemuServer and a long enough name as it is.
> 
> Signed-off-by: Stefan Reiter <s.reiter at proxmox.com>
> ---
> 
> I left the code exporting qemu_machine_feature_enabled from v2 and only remove
> it in the next patch, just to make sure this patch works standalone as well.
> 
>  PVE/QemuConfig.pm         |   3 +-
>  PVE/QemuMigrate.pm        |   3 +-
>  PVE/QemuServer.pm         | 103 +++-----------------------------------
>  PVE/QemuServer/Machine.pm | 101 +++++++++++++++++++++++++++++++++++++
>  PVE/QemuServer/Makefile   |   1 +
>  PVE/VZDump/QemuServer.pm  |   3 +-
>  6 files changed, 116 insertions(+), 98 deletions(-)
>  create mode 100644 PVE/QemuServer/Machine.pm
> 
> ---- 8< SNIP 8< ----  
>
> diff --git a/PVE/QemuServer/Machine.pm b/PVE/QemuServer/Machine.pm
> new file mode 100644
> index 0000000..c3e0004
> --- /dev/null
> +++ b/PVE/QemuServer/Machine.pm
> @@ -0,0 +1,101 @@
> +package PVE::QemuServer::Machine;
> +
> +use strict;
> +use warnings;
> +
> +use PVE::QemuServer::Monitor;
> +
> +use base 'Exporter';
> +our @EXPORT_OK = qw(
> +qemu_machine_feature_enabled
> +);
> +
> +sub machine_type_is_q35 {
> +    my ($conf) = @_;
> +
> +    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');
> +
> +    my ($current, $default);
> +    foreach my $e (@$res) {
> +	$default = $e->{name} if $e->{'is-default'};
> +	$current = $e->{name} if $e->{'is-current'};
> +    }
> +
> +    # fallback to the default machine if current is not supported by qemu
> +    return $current || $default || 'pc';
> +}
> +
> +sub qemu_machine_feature_enabled {
> +    my ($machine, $kvmver, $version_major, $version_minor) = @_;
> +
> +    my $current_major;
> +    my $current_minor;
> +
> +    if ($machine && $machine =~ m/^((?:pc(-i440fx|-q35)?|virt)-(\d+)\.(\d+))/) {
> +
> +	$current_major = $3;
> +	$current_minor = $4;
> +
> +    } elsif ($kvmver =~ m/^(\d+)\.(\d+)/) {
> +
> +	$current_major = $1;
> +	$current_minor = $2;
> +    }
> +
> +    return 1 if version_cmp($current_major, $version_major, $current_minor, $version_minor) >= 0;
> +}
> +
> +# gets in pairs the versions you want to compares, i.e.:
> +# ($a-major, $b-major, $a-minor, $b-minor, $a-extra, $b-extra, ...)
> +# returns 0 if same, -1 if $a is older than $b, +1 if $a is newer than $b
> +sub version_cmp {
> +    my @versions = @_;
> +
> +    my $size = scalar(@versions);
> +
> +    return 0 if $size == 0;
> +    die "cannot compare odd count of versions" if $size & 1;
> +
> +    for (my $i = 0; $i < $size; $i += 2) {
> +	my ($a, $b) = splice(@versions, 0, 2);
> +	$a //= 0;
> +	$b //= 0;
> +
> +	return 1 if $a > $b;
> +	return -1 if $a < $b;
> +    }
> +    return 0;
> +}
> +
> +# dies if a) VM not running or not exisiting b) Version query failed
> +# So, any defined return value is valid, any invalid state can be caught by eval
> +sub runs_at_least_qemu_version {
> +    my ($vmid, $major, $minor, $extra) = @_;
> +
> +    my $v = PVE::QemuServer::Monitor::mon_cmd($vmid, 'query-version');
> +    die "could not query currently running version for VM $vmid\n" if !defined($v);
> +    $v = $v->{qemu};
> +
> +    return version_cmp($v->{major}, $major, $v->{minor}, $minor, $v->{micro}, $extra) >= 0;

^^^^ this broke a few things like backup.. version_vmp is a (non-exported)
method of the PVE::QemuServer::Helpers module, so this cannot work.

Can you see if you add some tests for this, either standalone or integrated
into the cfg2cmd tests?