Re: [pve-devel] [PATCH v5 qemu-server 07/11] mtunnel: add API endpoints

[Fabian Ebner <f.ebner@proxmox.com>]

Am 09.02.22 um 14:07 schrieb Fabian Grünbichler:
> +	PVE::Cluster::check_cfs_quorum();
> +
> +	my $socket_addr = "/run/qemu-server/$vmid.mtunnel";

Nit: since there is another variable with the same name inside $realcmd
below, and this one is not used until the end, it could be moved further
down.

> +
> +	my $lock = 'create';
> +	eval { PVE::QemuConfig->create_and_lock_config($vmid, 0, $lock); };
> +
> +	raise_param_exc({ vmid => "unable to create empty VM config - $@"})
> +	    if $@;
> +
> +	my $realcmd = sub {

----8<----

> +		'config' => sub {
> +		    my ($params) = @_;
> +

----8<----

> +		    # not handled by update_vm_api
> +		    my $vmgenid = delete $new_conf->{vmgenid};
> +		    my $meta = delete $new_conf->{meta};
> +
> +		    $new_conf->{vmid} = $state->{vmid};
> +		    $new_conf->{node} = $node;
> +
> +		    PVE::QemuConfig->remove_lock($state->{vmid}, 'create');
> +
> +		    eval {
> +			$update_vm_api->($new_conf, 1);
> +		    };
> +		    if (my $err = $@) {
> +			# revert to locked previous config
> +			my $conf = PVE::QemuConfig->load_config($state->{vmid});
> +			$conf->{lock} = 'create';
> +			PVE::QemuConfig->write_config($state->{vmid}, $conf);
> +
> +			die $err;
> +		    }
> +
> +		    my $conf = PVE::QemuConfig->load_config($state->{vmid});
> +		    $conf->{lock} = 'migrate';
> +		    $conf->{vmgenid} = $vmgenid if $vmgenid;
> +		    $conf->{meta} = $meta if $meta;

Nit: shouldn't matter for these two, but 'if defined(...)' feels safer

> +		    PVE::QemuConfig->write_config($state->{vmid}, $conf);
> +
> +		    $state->{lock} = 'migrate';
> +
> +		    return;
> +		},

----8<----

> +		'resume' => sub {
> +		    if (PVE::QemuServer::check_running($state->{vmid}, 1)) {

Nit: use PVE::QemuServer::Helpers::vm_running_locally instead, as the
comment above PVE::QemuServer::check_running suggests

> +			PVE::QemuServer::vm_resume($state->{vmid}, 1, 1);
> +		    } else {
> +			die "VM $state->{vmid} not running\n";
> +		    }
> +		    return;
> +		},