Re: [pve-devel] [PATCH qemu-server 4/7] make an api call for each guest agent command

2018-02-13 Thread Thomas Lamprecht
On 2/13/18 4:47 PM, Dominik Csapak wrote:
> with a 'register_command' sub, which generates an api call
> we call it for each command in the list, and one time for
> the old general {vmid}/agent endpoint (for compatibility)
> 
> permissions/methods are the same as previously, but can
> be overriden with an entry in $ga_cmd_properties
> 
> Signed-off-by: Dominik Csapak 
> ---
>  PVE/API2/Qemu/Agent.pm | 99 
> --
>  1 file changed, 71 insertions(+), 28 deletions(-)
> 
> diff --git a/PVE/API2/Qemu/Agent.pm b/PVE/API2/Qemu/Agent.pm
> index 437d3f6..9d87b43 100644
> --- a/PVE/API2/Qemu/Agent.pm
> +++ b/PVE/API2/Qemu/Agent.pm
> @@ -9,6 +9,8 @@ use PVE::QemuServer;
>  
>  use base qw(PVE::RESTHandler);
>  
> +# list of commands
> +# will generate one api endpoint per command
>  my $guest_agent_commands = [
>  'ping',
>  'get-time',
> @@ -28,49 +30,90 @@ my $guest_agent_commands = [
>  'shutdown',
>  ];
>  
> -__PACKAGE__->register_method({
> -name => 'agent',
> -path => '',
> -method => 'POST',
> -protected => 1,
> -proxyto => 'node',
> -description => "Execute Qemu Guest Agent commands.",
> -permissions => {
> - check => ['perm', '/vms/{vmid}', [ 'VM.Monitor' ]],
> -},
> -parameters => {
> +# properties for each command, optional
> +# can has

What's with the early broken line? If we'd keep the semantics here I'd swap
the whole with:

# optionally set perms (default: VM.Monitor) or method (default: POST) property

To make a short concise single line comment.

> +# 'method': e.g. GET/POST
> +# 'perms': either a string like 'VM.Montior' or an array of such strings
> +#   or a permission object

s/Montior/Monitor/

> +my $ga_cmd_properties =  {};
> +
> +sub register_command {
> +my ($class, $command, $method, $perm) = @_;
> +
> +$method //= 'POST';
> +my $permission;
> +if (!$perm) {
> + $permission = { check => [ 'perm', '/vms/{vmid}', [ 'VM.Monitor' ]]};
> +} elsif (ref($perm) eq 'SCALAR') {
> + $permission = { check => [ 'perm', '/vms/{vmid}', [ $perm ]]};
> +} elsif (ref($perm) eq 'ARRAY') {
> + $permission = { check => [ 'perm', '/vms/{vmid}', $perm ]};
> +} elsif (ref($perm) eq 'HASH') {
> + $permission = $perm;
> +} else {
> + die 'invalid permissions given';
> +}

That's a bit of code which currently does not get used once after
the whole series got applied?
You sure we need this all? If so either add the respective permission
changes together if some monitor commands would need them, or just add
a very simplified version which accepts either string (falling back to
VM.Monitor) or full hash for now?

Rest looks OK.

> +
> +my $parameters = {
>   additionalProperties => 0,
>   properties => {
>   node => get_standard_option('pve-node'),
>   vmid => get_standard_option('pve-vmid', {
> -   completion => \::QemuServer::complete_vmid_running }),
> + completion => \::QemuServer::complete_vmid_running }),
>   command => {
>   type => 'string',
>   description => "The QGA command.",
>   enum => $guest_agent_commands,
>   },
>   },
> -},
> -returns => {
> - type => 'object',
> - description => "Returns an object with a single `result` property. The 
> type of that
> -property depends on the executed command.",
> -},
> -code => sub {
> - my ($param) = @_;
> +};
> +
> +my $description = "Execute Qemu Guest Agent commands.";
> +my $name = 'agent';
> +$command //= '';
> +
> +if ($command ne '') {
> + $description = "Execute $command.";
> + $name = $command;
> + delete $parameters->{properties}->{command};
> +}
> +
> +__PACKAGE__->register_method({
> + name => $name,
> + path => $command,
> + method => $method,
> + protected => 1,
> + proxyto => 'node',
> + description => $description,
> + permissions => $permission,
> + parameters => $parameters,
> + returns => {
> + type => 'object',
> + description => "Returns an object with a single `result` property.",
> + },
> + code => sub {
> + my ($param) = @_;
> +
> + my $vmid = $param->{vmid};
>  
> - my $vmid = $param->{vmid};
> + my $conf = PVE::QemuConfig->load_config ($vmid); # check if VM 
> exists
>  
> - my $conf = PVE::QemuConfig->load_config ($vmid); # check if VM exists
> + die "No Qemu Guest Agent\n" if !defined($conf->{agent});
> + die "VM $vmid is not running\n" if 
> !PVE::QemuServer::check_running($vmid);
>  
> - die "No Qemu Guest Agent\n" if !defined($conf->{agent});
> - die "VM $vmid is not running\n" if 
> !PVE::QemuServer::check_running($vmid);
> + my $cmd = $param->{command} // $command;
> + my $res = PVE::QemuServer::vm_mon_cmd($vmid, "guest-$cmd");
>  
> - 

[pve-devel] [PATCH qemu-server 4/7] make an api call for each guest agent command

2018-02-13 Thread Dominik Csapak
with a 'register_command' sub, which generates an api call
we call it for each command in the list, and one time for
the old general {vmid}/agent endpoint (for compatibility)

permissions/methods are the same as previously, but can
be overriden with an entry in $ga_cmd_properties

Signed-off-by: Dominik Csapak 
---
 PVE/API2/Qemu/Agent.pm | 99 --
 1 file changed, 71 insertions(+), 28 deletions(-)

diff --git a/PVE/API2/Qemu/Agent.pm b/PVE/API2/Qemu/Agent.pm
index 437d3f6..9d87b43 100644
--- a/PVE/API2/Qemu/Agent.pm
+++ b/PVE/API2/Qemu/Agent.pm
@@ -9,6 +9,8 @@ use PVE::QemuServer;
 
 use base qw(PVE::RESTHandler);
 
+# list of commands
+# will generate one api endpoint per command
 my $guest_agent_commands = [
 'ping',
 'get-time',
@@ -28,49 +30,90 @@ my $guest_agent_commands = [
 'shutdown',
 ];
 
-__PACKAGE__->register_method({
-name => 'agent',
-path => '',
-method => 'POST',
-protected => 1,
-proxyto => 'node',
-description => "Execute Qemu Guest Agent commands.",
-permissions => {
-   check => ['perm', '/vms/{vmid}', [ 'VM.Monitor' ]],
-},
-parameters => {
+# properties for each command, optional
+# can has
+# 'method': e.g. GET/POST
+# 'perms': either a string like 'VM.Montior' or an array of such strings
+# or a permission object
+my $ga_cmd_properties =  {};
+
+sub register_command {
+my ($class, $command, $method, $perm) = @_;
+
+$method //= 'POST';
+my $permission;
+if (!$perm) {
+   $permission = { check => [ 'perm', '/vms/{vmid}', [ 'VM.Monitor' ]]};
+} elsif (ref($perm) eq 'SCALAR') {
+   $permission = { check => [ 'perm', '/vms/{vmid}', [ $perm ]]};
+} elsif (ref($perm) eq 'ARRAY') {
+   $permission = { check => [ 'perm', '/vms/{vmid}', $perm ]};
+} elsif (ref($perm) eq 'HASH') {
+   $permission = $perm;
+} else {
+   die 'invalid permissions given';
+}
+
+my $parameters = {
additionalProperties => 0,
properties => {
node => get_standard_option('pve-node'),
vmid => get_standard_option('pve-vmid', {
-   completion => \::QemuServer::complete_vmid_running }),
+   completion => \::QemuServer::complete_vmid_running }),
command => {
type => 'string',
description => "The QGA command.",
enum => $guest_agent_commands,
},
},
-},
-returns => {
-   type => 'object',
-   description => "Returns an object with a single `result` property. The 
type of that
-property depends on the executed command.",
-},
-code => sub {
-   my ($param) = @_;
+};
+
+my $description = "Execute Qemu Guest Agent commands.";
+my $name = 'agent';
+$command //= '';
+
+if ($command ne '') {
+   $description = "Execute $command.";
+   $name = $command;
+   delete $parameters->{properties}->{command};
+}
+
+__PACKAGE__->register_method({
+   name => $name,
+   path => $command,
+   method => $method,
+   protected => 1,
+   proxyto => 'node',
+   description => $description,
+   permissions => $permission,
+   parameters => $parameters,
+   returns => {
+   type => 'object',
+   description => "Returns an object with a single `result` property.",
+   },
+   code => sub {
+   my ($param) = @_;
+
+   my $vmid = $param->{vmid};
 
-   my $vmid = $param->{vmid};
+   my $conf = PVE::QemuConfig->load_config ($vmid); # check if VM 
exists
 
-   my $conf = PVE::QemuConfig->load_config ($vmid); # check if VM exists
+   die "No Qemu Guest Agent\n" if !defined($conf->{agent});
+   die "VM $vmid is not running\n" if 
!PVE::QemuServer::check_running($vmid);
 
-   die "No Qemu Guest Agent\n" if !defined($conf->{agent});
-   die "VM $vmid is not running\n" if 
!PVE::QemuServer::check_running($vmid);
+   my $cmd = $param->{command} // $command;
+   my $res = PVE::QemuServer::vm_mon_cmd($vmid, "guest-$cmd");
 
-   my $cmd = $param->{command};
+   return { result => $res };
+   }});
+}
 
-   my $res = PVE::QemuServer::vm_mon_cmd($vmid, "guest-$cmd");
+# old {vmid}/agent POST endpoint, here for compatibility
+__PACKAGE__->register_command();
 
-   return { result => $res };
-}});
+for my $cmd (@$guest_agent_commands) {
+my $props = $ga_cmd_properties->{$cmd} // {};
+__PACKAGE__->register_command($cmd, $props->{method}, $props->{perms});
+}
 
 1;
-- 
2.11.0


___
pve-devel mailing list
pve-devel@pve.proxmox.com
https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel