[pve-devel] [PATCH qemu-server 6/8] add exec(-status) to qm

[Thomas Lamprecht]

On 6/7/18 1:16 PM, Dominik Csapak wrote:
> on the commandline the implementation for exec is a bit different
> because there we want (by default) to wait for the result,
> as opposed to the api, where it is enough to return the pid and
> let the client handle the polling
> 
> this behaviour is optional and can be turned off, as well as the
> timeout of 30 seconds
> 
> Signed-off-by: Dominik Csapak <d.csapak at proxmox.com>
> ---
>  PVE/CLI/qm.pm | 74 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 74 insertions(+)
> 
> diff --git a/PVE/CLI/qm.pm b/PVE/CLI/qm.pm
> index ee52c4b..de58c15 100755
> --- a/PVE/CLI/qm.pm
> +++ b/PVE/CLI/qm.pm
> @@ -18,9 +18,11 @@ use PVE::Cluster;
>  use PVE::SafeSyslog;
>  use PVE::INotify;
>  use PVE::RPCEnvironment;
> +use PVE::Exception qw(raise_param_exc);
>  use PVE::QemuServer;
>  use PVE::QemuServer::ImportDisk;
>  use PVE::QemuServer::OVF;
> +use PVE::QemuServer::Agent qw(raise_agent_error check_agent_available);
>  use PVE::API2::Qemu;
>  use PVE::API2::Qemu::Agent;
>  use JSON;
> @@ -643,6 +645,76 @@ __PACKAGE__->register_method ({
>      }
>  });
>  
> +__PACKAGE__->register_method({
> +    name => 'exec',
> +    path => 'exec',
> +    method => 'POST',
> +    protected => 1,
> +    description => "Executes the given command via the guest agent",
> +    parameters => {
> +	additionalProperties => 0,
> +	properties => {
> +	    node => get_standard_option('pve-node'),
> +	    vmid => get_standard_option('pve-vmid', {
> +		    completion => \&PVE::QemuServer::complete_vmid_running }),
> +	    synchronous => {
> +		type => 'boolean',
> +		optional => 1,
> +		default => 1,
> +		description => "If not set, returns the pid instead of waiting for the commmand to finish.",

misleading as default is '1', so if not set means that it only
returns the PID if the sync-timeout has passed...

Maybe: "If set to false, ..."
(not totally happy with that either but makes it clearer, IMO)



> +	    },
> +	    'sync-timeout' => {
> +		type => 'integer',
> +		description => "The maximum time to wait for the command to finish. Set to 0 to deactivate",

Sounds a bit like a kill timeout.

maybe something like:
"The maximum time to wait before moving the command to the background."

Also, would it makes sense to combine synchronous and sync-timeout
to a single parameter? if it's zero we stay in sync forever, else
only until timeout happened?

Maybe output also some feedback over STDERR? Could be confusing for
some users else, or what do you think?


Besides my constant nits, works quite well itself! :-)

> +		minimum => 0,
> +		optional => 1,
> +		default => 30,
> +		requires => 'synchronous'
> +	    },
> +	    'extra-args' => get_standard_option('extra-args'),
> +	},
> +    },
> +    returns => {
> +	type => 'object',
> +	description => "Returns an object with a single `result` property.",
> +    },
> +    code => sub {
> +	my ($param) = @_;
> +
> +	my $vmid = $param->{vmid};
> +
> +	my $conf = PVE::QemuConfig->load_config ($vmid); # check if VM exists
> +
> +	check_agent_available($vmid, $conf);
> +
> +	if (!$param->{'extra-args'} || !@{$param->{'extra-args'}}) {
> +	    raise_param_exc( { 'extra-args' => "No command given" });
> +	}
> +	my $res = PVE::QemuServer::Agent::qemu_exec($vmid, $param->{'extra-args'});
> +	my $sync = $param->{synchronous} // 1;
> +
> +	if (defined($param->{'sync-timeout'}) && !$sync) {
> +	    raise_param_exc({ synchronous => "needs to be set for 'sync-timeout'"});
> +	}
> +
> +	if ($sync) {
> +	    my $pid = $res->{pid};
> +	    my $timeout = $param->{timeout} // 30;
> +	    my $starttime = time();
> +
> +	    while ($timeout == 0 || (time() - $starttime) < $timeout) {
> +		my $out = PVE::QemuServer::Agent::qemu_exec_status($vmid, $pid);
> +		if ($out->{exited}) {
> +		    $res = $out;
> +		    last;
> +		}
> +		sleep 1;
> +	    }
> +	}
> +
> +	return { result => $res };
> +    }});
> +
>  my $print_agent_result = sub {
>      my ($data) = @_;
>  
> @@ -824,6 +896,8 @@ our $cmddef = {
>  		{ node => $nodename }, $print_agent_result ],
>  
>      'set-user-password' => [ "PVE::API2::Qemu::Agent", 'set-user-password', [ 'vmid', 'username' ], { node => $nodename }],
> +    exec => [ __PACKAGE__, 'exec', [ 'vmid', 'extra-args' ], { node => $nodename }, $print_agent_result],
> +    'exec-status' => [ "PVE::API2::Qemu::Agent", 'exec-status', [ 'vmid', 'pid' ], { node => $nodename }, $print_agent_result],
>  
>      mtunnel => [ __PACKAGE__, 'mtunnel', []],
>  
>