[pve-devel] [PATCH pve-client] cleanup: use nested CLIHandler command definitions

2018-06-12 Thread Dietmar Maurer
Signed-off-by: Dietmar Maurer 
---
 PVE/APIClient/Helpers.pm |  44 +-
 pveclient| 208 ---
 2 files changed, 165 insertions(+), 87 deletions(-)

diff --git a/PVE/APIClient/Helpers.pm b/PVE/APIClient/Helpers.pm
index d6d1a17..b83829b 100644
--- a/PVE/APIClient/Helpers.pm
+++ b/PVE/APIClient/Helpers.pm
@@ -15,6 +15,13 @@ my $pve_api_path_hash;
 
 my $pve_api_definition_fn = "/usr/share/pve-client/pve-api-definition.dat";
 
+my $method_map = {
+create => 'POST',
+set => 'PUT',
+get => 'GET',
+delete => 'DELETE',
+};
+
 my $build_pve_api_path_hash;
 $build_pve_api_path_hash = sub {
 my ($tree) = @_;
@@ -131,16 +138,49 @@ sub complete_api_path {
$info = $pve_api_path_hash->{"/$dir"};
 }
 
+my $res = [];
 if ($info) {
if (my $children = $info->{children}) {
foreach my $c (@$children) {
if ($c->{path} =~ m!\Q$dir/$rest!) {
-   print "$c->{path}\n";
-   print "$c->{path}/\n"if $c->{children};
+   push @$res, $c->{path};
+   push @$res, "$c->{path}/" if $c->{children};
}
}
}
 }
+return $res;
+}
+
+# test for command lines with api calls (or similar bash completion calls):
+# example1: pveclient api get remote1 /cluster
+sub extract_path_info {
+
+my $info;
+
+my $test_path_properties = sub {
+   my ($args) = @_;
+
+   return if scalar(@$args) < 5;
+   return if $args->[1] ne 'api';
+
+   my $path = $args->[4];
+   if (my $method = $method_map->{$args->[2]}) {
+   $info = lookup_api_method($path, $method, 1);
+   }
+};
+
+if (defined(my $cmd = $ARGV[0])) {
+   if ($cmd eq 'api') {
+   $test_path_properties->([$0, @ARGV]);
+   } elsif ($cmd eq 'bashcomplete') {
+   my $cmdline = substr($ENV{COMP_LINE}, 0, $ENV{COMP_POINT});
+   my $args = PVE::Tools::split_args($cmdline);
+   $test_path_properties->($args);
+   }
+}
+
+return $info;
 }
 
 1;
diff --git a/pveclient b/pveclient
index f18dab9..70db84a 100755
--- a/pveclient
+++ b/pveclient
@@ -1,5 +1,7 @@
 #!/usr/bin/perl
 
+package PVE::CLI::pveclient;
+
 use strict;
 use warnings;
 use Cwd 'abs_path';
@@ -7,7 +9,7 @@ use lib '/usr/share/pve-client';
 use lib '.';
 use Data::Dumper;
 
-use PVE::JSONSchema;
+use PVE::JSONSchema qw(register_standard_option get_standard_option);
 use PVE::CLIHandler;
 
 use PVE::APIClient::LWP;
@@ -21,97 +23,21 @@ use PVE::APIClient::Commands::help;
 use JSON;
 
 sub call_method {
-my ($path, $method, $args) = @_;
+my ($remote, $path, $method, $params) = @_;
 
 die "missing API path\n" if !defined($path);
 
 my $info = PVE::APIClient::Helpers::lookup_api_method($path, $method);
-my $param = PVE::JSONSchema::get_options($info->{parameters}, $args);
-print Dumper($param);
+print Dumper($params);
 
 die "implement me";
 }
 
+use base qw(PVE::CLIHandler);
 
-my $cli_class_handlers = {
-list => 'PVE::APIClient::Commands::list',
-lxc => 'PVE::APIClient::Commands::lxc',
-remote => 'PVE::APIClient::Commands::remote',
-config => 'PVE::APIClient::Commands::config',
-help => 'PVE::APIClient::Commands::help',
-};
-
-my $cmd = shift;
-if (!defined($cmd)) {
-PVE::APIClient::Commands::help->help({});
-exit(-1);
-}
-
-my $method_map = {
-create => 'POST',
-set => 'PUT',
-get => 'GET',
-delete => 'DELETE',
-};
-
-if (my $method = $method_map->{$cmd}) {
-my $path;
-if (scalar(@ARGV) && $ARGV[0] !~ m/^\-/)  {
-   $path = shift @ARGV;
-}
-my $res = call_method($path, $method, \@ARGV);
-die "implement me";
-} elsif (my $class = $cli_class_handlers->{$cmd}) {
-$class->run_cli_handler();
-} elsif ($cmd eq 'bashcomplete') {
-
-exit(0) if !(defined($ENV{COMP_LINE}) && defined($ENV{COMP_POINT}));
-
-my $cmdlist = join('|', keys %$cli_class_handlers);
-if ($ENV{COMP_LINE} =~ m/^(.*pveclient\s+($cmdlist)\s+)(.*)$/) {
-   my $cmd = $2;
-   my $class = $cli_class_handlers->{$cmd} || die "internal error";
-
-   if ($cmd eq 'list') { # simple commands
-   $ENV{COMP_LINE} = "pveclient $3";
-   $ENV{COMP_POINT} = length($ENV{COMP_LINE});
-   @ARGV = ('bashcomplete', 'pveclient', $ARGV[1], $ARGV[2]);
-   } else {
-   $ENV{COMP_LINE} = "pveclient $3";
-   $ENV{COMP_POINT} = length($ENV{COMP_LINE});
-   @ARGV = ('bashcomplete', 'pveclient', $ARGV[1], $ARGV[2]);
-   }
-   $class->run_cli_handler();
-
-} else {
-
-   my $cmdline = substr($ENV{COMP_LINE}, 0, $ENV{COMP_POINT});
-   my ($bash_command, $cur, $prev) = @ARGV;
-   $cmdline =~ s/\Q$cur\E$//;
-
-   my $args = PVE::Tools::split_args($cmdline);
-
-   my @cmds = (keys %$method_map, keys %$cli_class_handlers);
-   if (scalar(@$args) == 1) {
-   foreach my $p 

Re: [pve-devel] [PATCH common 1/2] remove read_password_func from cli handler

2018-06-12 Thread Thomas Lamprecht
On 6/12/18 12:33 PM, Dominik Csapak wrote:
> we have a param_mapping now, with which we can impement that
> 
> e.g. instead of using:
> 
> sub read_password {
> # read password
> }
> 
> we can do:
> 
> sub param_mapping {
> my ($name) = @_;
> 
> my $password_map = ['password', sub{
>   # read password
> }, ' 
> my $mapping = {
>   'apicall1' => [$password_map],
>   'apicall2' => [$password_map],
>   ...
> };
> 
> return $mapping->{$name};
> }
> 
> this has the advantage that we can use different subs for different
> api calls (e.g. pveum passwd vs pveum ticket)

looks OK and IMO the better approach as the old read_password_func
handler, few comments inline.

Further this patch needs some coordination on release, i.e., update
of version dependencies in the using packages and a Breaks for them
here. Should I coordinate that part?

> 
> Signed-off-by: Dominik Csapak 
> ---
>  src/PVE/CLIHandler.pm  | 18 --
>  src/PVE/JSONSchema.pm  | 15 +--
>  src/PVE/RESTHandler.pm | 26 +-
>  3 files changed, 22 insertions(+), 37 deletions(-)
> 
> diff --git a/src/PVE/CLIHandler.pm b/src/PVE/CLIHandler.pm
> index 8c911b2..b398f12 100644
> --- a/src/PVE/CLIHandler.pm
> +++ b/src/PVE/CLIHandler.pm
> @@ -127,7 +127,6 @@ sub generate_usage_str {
>  $separator //= '';
>  $indent //= '';
>  
> -my $read_password_func = $cli_handler_class->can('read_password');
>  my $param_mapping_func = $cli_handler_class->can('param_mapping') ||
>   $cli_handler_class->can('string_param_file_mapping');
>  
> @@ -149,7 +148,7 @@ sub generate_usage_str {
>   $str .= $indent;
>   $str .= $class->usage_str($name, "$prefix $cmd", $arg_param,
> $fixed_param, $format,
> -   $read_password_func, 
> $param_mapping_func);
> +   $param_mapping_func);
>   $oldclass = $class;
>  
>   } elsif (defined($def->{$cmd}->{alias}) && ($format eq 
> 'asciidoc')) {
> @@ -173,7 +172,7 @@ sub generate_usage_str {
>  
>   $str .= $indent;
>   $str .= $class->usage_str($name, $prefix, $arg_param, $fixed_param, 
> $format,
> -   $read_password_func, $param_mapping_func);
> +   $param_mapping_func);
>   }
>   return $str;
>  };
> @@ -466,7 +465,7 @@ sub setup_environment {
>  }
>  
>  my $handle_cmd  = sub {
> -my ($args, $read_password_func, $preparefunc, $param_mapping_func) = @_;
> +my ($args, $preparefunc, $param_mapping_func) = @_;
>  
>  $cmddef->{help} = [ __PACKAGE__, 'help', ['extra-args'] ];
>  
> @@ -496,13 +495,13 @@ my $handle_cmd  = sub {
>  my ($class, $name, $arg_param, $uri_param, $outsub) = @{$def || []};
>  $abort->("unknown command '$cmd_str'") if !$class;
>  
> -my $res = $class->cli_handler($cmd_str, $name, $cmd_args, $arg_param, 
> $uri_param, $read_password_func, $param_mapping_func);
> +my $res = $class->cli_handler($cmd_str, $name, $cmd_args, $arg_param, 
> $uri_param, $param_mapping_func);
>  
>  &$outsub($res) if $outsub;
>  };
>  
>  my $handle_simple_cmd = sub {
> -my ($args, $read_password_func, $preparefunc, $param_mapping_func) = @_;
> +my ($args, $preparefunc, $param_mapping_func) = @_;
>  
>  my ($class, $name, $arg_param, $uri_param, $outsub) = @{$cmddef};
>  die "no class specified" if !$class;
> @@ -531,7 +530,7 @@ my $handle_simple_cmd = sub {
>  
>  &$preparefunc() if $preparefunc;
>  
> -my $res = $class->cli_handler($name, $name, \@ARGV, $arg_param, 
> $uri_param, $read_password_func, $param_mapping_func);
> +my $res = $class->cli_handler($name, $name, \@ARGV, $arg_param, 
> $uri_param, $param_mapping_func);
>  
>  &$outsub($res) if $outsub;
>  };
> @@ -552,7 +551,6 @@ sub run_cli_handler {
>  
>  my $preparefunc = $params{prepare};
>  
> -my $read_password_func = $class->can('read_password');
>  my $param_mapping_func = $cli_handler_class->can('param_mapping') ||
>   $class->can('string_param_file_mapping');
>  
> @@ -564,9 +562,9 @@ sub run_cli_handler {
>  $cmddef = ${"${class}::cmddef"};
>  
>  if (ref($cmddef) eq 'ARRAY') {
> - &$handle_simple_cmd(\@ARGV, $read_password_func, $preparefunc, 
> $param_mapping_func);
> + &$handle_simple_cmd(\@ARGV, $preparefunc, $param_mapping_func);
>  } else {
> - &$handle_cmd(\@ARGV, $read_password_func, $preparefunc, 
> $param_mapping_func);
> + &$handle_cmd(\@ARGV, $preparefunc, $param_mapping_func);

can we switch those to $handle_cmd->() syntax if we touch those here?

>  }
>  
>  exit 0;
> diff --git a/src/PVE/JSONSchema.pm b/src/PVE/JSONSchema.pm
> index f014dc3..1259195 100644
> --- a/src/PVE/JSONSchema.pm
> +++ b/src/PVE/JSONSchema.pm
> @@ -1333,7 +1333,7 @@ sub method_get_child_link {
> 

[pve-devel] applied: [PATCH access-control 1/2] fix typo in change_passsword

2018-06-12 Thread Thomas Lamprecht
On 6/12/18 12:33 PM, Dominik Csapak wrote:
> s/passsword/password/
> 
> Signed-off-by: Dominik Csapak 
> ---
>  PVE/API2/AccessControl.pm | 2 +-
>  PVE/CLI/pveum.pm  | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/PVE/API2/AccessControl.pm b/PVE/API2/AccessControl.pm
> index e48f0cb..afcd2fa 100644
> --- a/PVE/API2/AccessControl.pm
> +++ b/PVE/API2/AccessControl.pm
> @@ -283,7 +283,7 @@ __PACKAGE__->register_method ({
>  }});
>  
>  __PACKAGE__->register_method ({
> -name => 'change_passsword', 
> +name => 'change_password',
>  path => 'password', 
>  method => 'PUT',
>  permissions => { 
> diff --git a/PVE/CLI/pveum.pm b/PVE/CLI/pveum.pm
> index aef7089..dbc3034 100755
> --- a/PVE/CLI/pveum.pm
> +++ b/PVE/CLI/pveum.pm
> @@ -44,7 +44,7 @@ our $cmddef = {
>   print "$res->{ticket}\n";
>   }],
>  
> -passwd => [ 'PVE::API2::AccessControl', 'change_passsword', ['userid'] ],
> +passwd => [ 'PVE::API2::AccessControl', 'change_password', ['userid'] ],
>  
>  useradd => [ 'PVE::API2::User', 'create_user', ['userid'] ],
>  usermod => [ 'PVE::API2::User', 'update_user', ['userid'] ],
> 

applied with resolved merge conflict from context
changed by the subcommand commit

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


[pve-devel] applied: [PATCH common 2/2] add a generalized 'read and confirm password' sub

2018-06-12 Thread Thomas Lamprecht
On 6/12/18 12:33 PM, Dominik Csapak wrote:
> to use everywhere we read two passwords and compare them
> 
> Signed-off-by: Dominik Csapak 
> ---
>  src/PVE/PTY.pm | 7 +++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/src/PVE/PTY.pm b/src/PVE/PTY.pm
> index 23d76c0..e433c4e 100644
> --- a/src/PVE/PTY.pm
> +++ b/src/PVE/PTY.pm
> @@ -228,6 +228,13 @@ sub read_password($;$$) {
>  return $password;
>  }
>  
> +sub get_confirmed_password {
> +my $pw1 = read_password('Enter new password: ');
> +my $pw2 = read_password('Retype new password: ');
> +die "passwords do not match\n" if $pw1 ne $pw2;
> +return $pw1;
> +}
> +
>  # Class functions
>  
>  sub new {
> 

applied

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


Re: [pve-devel] [PATCH storage v3 2/5] Cephfs storage plugin

2018-06-12 Thread Thomas Lamprecht
On 6/11/18 11:31 AM, Alwin Antreich wrote:
>  - ability to mount through kernel and fuse client
>  - allow mount options
>  - get MONs from ceph config if not in storage.cfg
>  - allow the use of ceph config with fuse client
> 
> Signed-off-by: Alwin Antreich 
> ---
>  PVE/API2/Storage/Config.pm  |   2 +-
>  PVE/Storage.pm  |   2 +
>  PVE/Storage/CephFSPlugin.pm | 225 
> 
>  PVE/Storage/Makefile|   2 +-
>  PVE/Storage/Plugin.pm   |   1 +
>  debian/control  |   1 +
>  6 files changed, 231 insertions(+), 2 deletions(-)
>  create mode 100644 PVE/Storage/CephFSPlugin.pm
> 
> diff --git a/PVE/API2/Storage/Config.pm b/PVE/API2/Storage/Config.pm
> index 3b38304..368a5c9 100755
> --- a/PVE/API2/Storage/Config.pm
> +++ b/PVE/API2/Storage/Config.pm
> @@ -171,7 +171,7 @@ __PACKAGE__->register_method ({
>   PVE::Storage::activate_storage($cfg, $baseid);
>  
>   PVE::Storage::LVMPlugin::lvm_create_volume_group($path, 
> $opts->{vgname}, $opts->{shared});
> - } elsif ($type eq 'rbd' && !defined($opts->{monhost})) {
> + } elsif (($type eq 'rbd' || $type eq 'cephfs') && 
> !defined($opts->{monhost})) {
>   my $ceph_admin_keyring = 
> '/etc/pve/priv/ceph.client.admin.keyring';
>   my $ceph_storage_keyring = 
> "/etc/pve/priv/ceph/${storeid}.keyring";
>  
> diff --git a/PVE/Storage.pm b/PVE/Storage.pm
> index d733380..f9732fe 100755
> --- a/PVE/Storage.pm
> +++ b/PVE/Storage.pm
> @@ -28,6 +28,7 @@ use PVE::Storage::NFSPlugin;
>  use PVE::Storage::CIFSPlugin;
>  use PVE::Storage::ISCSIPlugin;
>  use PVE::Storage::RBDPlugin;
> +use PVE::Storage::CephFSPlugin;
>  use PVE::Storage::SheepdogPlugin;
>  use PVE::Storage::ISCSIDirectPlugin;
>  use PVE::Storage::GlusterfsPlugin;
> @@ -46,6 +47,7 @@ PVE::Storage::NFSPlugin->register();
>  PVE::Storage::CIFSPlugin->register();
>  PVE::Storage::ISCSIPlugin->register();
>  PVE::Storage::RBDPlugin->register();
> +PVE::Storage::CephFSPlugin->register();
>  PVE::Storage::SheepdogPlugin->register();
>  PVE::Storage::ISCSIDirectPlugin->register();
>  PVE::Storage::GlusterfsPlugin->register();
> diff --git a/PVE/Storage/CephFSPlugin.pm b/PVE/Storage/CephFSPlugin.pm
> new file mode 100644
> index 000..9882760
> --- /dev/null
> +++ b/PVE/Storage/CephFSPlugin.pm
> @@ -0,0 +1,225 @@
> +package PVE::Storage::CephFSPlugin;
> +
> +use strict;
> +use warnings;
> +use IO::File;
> +use Net::IP;
> +use File::Path;
> +use PVE::Tools qw(run_command);
> +use PVE::ProcFSTools;
> +use PVE::Storage::Plugin;
> +use PVE::JSONSchema qw(get_standard_option);
> +use PVE::CephStorageTools;

You add this module in the next patch (3/5) but use it already here,
which breaks build! Swap 3/5 before 2/5..
I'd also call it PVE::Storage::CephTools and put it there if we
have this here.

> +
> +use base qw(PVE::Storage::Plugin);
> +
> +my $parse_ceph_config = sub {

AFAIS, this is the direct copy of PVE::CephTools::parse_ceph_config ?
Why not move this into PVE::Storage::CephTools (called
PVE::CephStorageTools here) together with the monaddr function and
potential other functions from the existing package PVE::CephTools,
ideally making it obsolete so that only one is left.

> +my ($filename) = @_;
> +
> +my $cfg = {};
> +
> +return $cfg if ! -f $filename;
> +
> +my $fh = IO::File->new($filename, "r") ||
> + die "unable to open '$filename' - $!\n";
> +
> +my $section;
> +
> +while (defined(my $line = <$fh>)) {
> + $line =~ s/[;#].*$//;
> + $line =~ s/^\s+//;
> + $line =~ s/\s+$//;
> + next if !$line;
> +
> + $section = $1 if $line =~ m/^\[(\S+)\]$/;
> + if (!$section) {
> + warn "no section - skip: $line\n";
> + next;
> + }
> +
> + if ($line =~ m/^(.*?\S)\s*=\s*(\S.*)$/) {
> + $cfg->{$section}->{$1} = $2;
> + }
> +
> +}
> +
> +return $cfg;
> +};
> +
> +my $get_monaddr_list = sub {
> +my ($scfg, $configfile) = @_;
> +
> +my $server;
> +my $no_mon = !defined($scfg->{monhost});
> +
> +if (($no_mon) && defined($configfile)) {
> + my $config = $parse_ceph_config->($configfile);
> + $server = join(',', sort { $a cmp $b }

$a cmp $b is default, so not needed?

> + map { $config->{$_}->{'mon addr'} } grep {/mon/} %{$config});
> +}else {

if $no_mon is false and $configfile is also we still get here,
with an undefined $scfg->{monhost}, can this happen? If so, I'd
error out above

> + $server = PVE::CephStorageTools::hostlist($scfg->{monhost}, ',');
> +}
> +
> +return $server;
> +};
> +
> +sub cephfs_is_mounted {
> +my ($scfg, $storeid, $mountdata) = @_;
> +
> +my $cmd_option = PVE::CephStorageTools::ceph_connect_option($scfg, 
> $storeid);
> +my $configfile = $cmd_option->{ceph_conf};
> +my $server = $get_monaddr_list->($scfg, $configfile);
> +
> +my $subdir = $scfg->{subdir} ? $scfg->{subdir} : '/';


[pve-devel] [PATCH pve-client v2] Add start command

2018-06-12 Thread René Jochum
I've added the logic to poll the task given by status/start until its
"stopped", this enables an usage like:

pveclient lxc create 999 && pveclient start 999 && pveclient enter 999

Signed-off-by: René Jochum 
---
v2 is a rebase on master and I added the wait logic.

 PVE/APIClient/Commands/help.pm  |  2 ++
 PVE/APIClient/Commands/start.pm | 43 +
 PVE/APIClient/Helpers.pm| 39 +
 pveclient   |  2 ++
 4 files changed, 86 insertions(+)
 create mode 100644 PVE/APIClient/Commands/start.pm

diff --git a/PVE/APIClient/Commands/help.pm b/PVE/APIClient/Commands/help.pm
index efb964a..258a4c0 100644
--- a/PVE/APIClient/Commands/help.pm
+++ b/PVE/APIClient/Commands/help.pm
@@ -8,6 +8,7 @@ use PVE::APIClient::Commands::list;
 use PVE::APIClient::Commands::lxc;
 use PVE::APIClient::Commands::config;
 use PVE::APIClient::Commands::remote;
+use PVE::APIClient::Commands::start;
 
 use PVE::CLIHandler;
 
@@ -61,6 +62,7 @@ __PACKAGE__->register_method ({
$assemble_usage_string->('lxc', $PVE::APIClient::Commands::lxc::cmddef);
$assemble_usage_string->('remote', 
$PVE::APIClient::Commands::remote::cmddef);
$assemble_usage_string->('config', 
$PVE::APIClient::Commands::config::cmddef);
+   $assemble_usage_string->('start', 
$PVE::APIClient::Commands::start::cmddef);
 
$text .= "pveclient   {options}\n\n";
 
diff --git a/PVE/APIClient/Commands/start.pm b/PVE/APIClient/Commands/start.pm
new file mode 100644
index 000..2b217a5
--- /dev/null
+++ b/PVE/APIClient/Commands/start.pm
@@ -0,0 +1,43 @@
+package PVE::APIClient::Commands::start;
+
+use strict;
+use warnings;
+
+use PVE::APIClient::Helpers;
+use PVE::JSONSchema qw(get_standard_option);
+
+use PVE::CLIHandler;
+
+use base qw(PVE::CLIHandler);
+
+__PACKAGE__->register_method ({
+name => 'start',
+path => 'start',
+method => 'POST',
+description => "Start a Qemu VM/LinuX Container.",
+parameters => {
+   additionalProperties => 0,
+   properties => {
+   remote => get_standard_option('pveclient-remote-name'),
+   vmid => get_standard_option('pve-vmid'),
+   },
+},
+returns => { type => 'null'},
+code => sub {
+   my ($param) = @_;
+
+   my $config = PVE::APIClient::Config->load();
+   my $conn = PVE::APIClient::Config->remote_conn($config, 
$param->{remote});
+
+   my $resource = PVE::APIClient::Helpers::get_vmid_resource($conn, 
$param->{vmid});
+
+   my $upid = 
$conn->post("api2/json/nodes/$resource->{node}/$resource->{type}/$resource->{vmid}/status/start",
 {});
+
+   print PVE::APIClient::Helpers::poll_task($conn, $resource->{node}, 
$upid) . "\n";
+
+   return undef;
+}});
+
+our $cmddef = [ __PACKAGE__, 'start', ['remote', 'vmid']];
+
+1;
diff --git a/PVE/APIClient/Helpers.pm b/PVE/APIClient/Helpers.pm
index d6d1a17..ba7f83e 100644
--- a/PVE/APIClient/Helpers.pm
+++ b/PVE/APIClient/Helpers.pm
@@ -143,4 +143,43 @@ sub complete_api_path {
 }
 }
 
+sub get_vmid_resource {
+my ($conn, $vmid) = @_;
+
+my $resources = $conn->get('api2/json/cluster/resources', {type => 'vm'});
+
+my $resource;
+for my $tmp (@$resources) {
+   if ($tmp->{vmid} eq $vmid) {
+   $resource = $tmp;
+   last;
+   }
+}
+
+if (!defined($resource)) {
+   die "\"$vmid\" not found";
+}
+
+return $resource;
+}
+
+sub poll_task {
+my ($conn, $node, $upid) = @_;
+
+my $path = "api2/json/nodes/$node/tasks/$upid/status";
+
+my $task_status;
+while(1) {
+   $task_status = $conn->get($path, {});
+
+   if ($task_status->{status} eq "stopped") {
+   last;
+   }
+
+   sleep(10);
+}
+
+return $task_status->{exitstatus};
+}
+
 1;
diff --git a/pveclient b/pveclient
index f18dab9..9d34559 100755
--- a/pveclient
+++ b/pveclient
@@ -17,6 +17,7 @@ use PVE::APIClient::Commands::remote;
 use PVE::APIClient::Commands::list;
 use PVE::APIClient::Commands::lxc;
 use PVE::APIClient::Commands::help;
+use PVE::APIClient::Commands::start;
 
 use JSON;
 
@@ -38,6 +39,7 @@ my $cli_class_handlers = {
 lxc => 'PVE::APIClient::Commands::lxc',
 remote => 'PVE::APIClient::Commands::remote',
 config => 'PVE::APIClient::Commands::config',
+start => 'PVE::APIClient::Commands::start',
 help => 'PVE::APIClient::Commands::help',
 };
 
-- 
2.11.0

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


[pve-devel] [PATCH manager] replace read_password with param_mapping in pvesh

2018-06-12 Thread Dominik Csapak
Signed-off-by: Dominik Csapak 
---
this belongs to my series for removing 'read_password'
we also have to do the same in pmgsh if the patches get applied
to pve-common
 bin/pvesh | 34 --
 1 file changed, 16 insertions(+), 18 deletions(-)

diff --git a/bin/pvesh b/bin/pvesh
index 36ce7636..4ab26b8f 100755
--- a/bin/pvesh
+++ b/bin/pvesh
@@ -11,6 +11,7 @@ use Getopt::Long;
 use HTTP::Status qw(:constants :is status_message);
 use Text::ParseWords;
 use String::ShellQuote;
+use PVE::PTY;
 use PVE::JSONSchema;
 use PVE::SafeSyslog;
 use PVE::Cluster;
@@ -184,23 +185,20 @@ sub abs_path {
 return $ret;
 }
 
-my $read_password = sub {
-my $attribs = $term->Attribs;
-my $old = $attribs->{redisplay_function};
-$attribs->{redisplay_function} = $attribs->{shadow_redisplay};
-my $input = $term->readline('password: ');
-my $conf = $term->readline('Retype new password: ');
-$attribs->{redisplay_function} = $old;
-
-# remove password from history
-if ($term->Features->{autohistory}) {
-   my $historyPosition = $term->where_history();
-   $term->remove_history($historyPosition);
-   $term->remove_history($historyPosition - 1);
-}
+my $param_mapping = sub {
+my ($name) = @_;
+
+my $password_map = [
+   'password',
+   sub {
+   my ($value) = @_;
+   return $value if $value;
+   return PVE::PTY::get_confirmed_password();
+   },
+   '', 1
+];
 
-die "Passwords do not match.\n" if ($input ne $conf);
-return $input;
+return [$password_map];
 };
 
 sub reverse_map_cmd {
@@ -279,7 +277,7 @@ sub call_method {
 my ($node, $remip) = check_proxyto($info, $uri_param);
 return proxy_handler($node, $remip, $dir, $cmd, $args) if $node;
 
-my $data = $handler->cli_handler("$cmd $dir", $info->{name}, $args, [], 
$uri_param, $read_password);
+my $data = $handler->cli_handler("$cmd $dir", $info->{name}, $args, [], 
$uri_param, $param_mapping);
 
 return if $nooutput;
 
@@ -450,7 +448,7 @@ sub list_dir {
 return proxy_handler($node, $remip, $dir, 'ls', $args) if $node;
 
 
-my $data = $handler->cli_handler("ls $dir", $info->{name}, $args, [], 
$uri_param, $read_password); 
+my $data = $handler->cli_handler("ls $dir", $info->{name}, $args, [], 
$uri_param, $param_mapping);
 my $lnk = PVE::JSONSchema::method_get_child_link($info);
 my $children = extract_children($lnk, $data);
 
-- 
2.11.0


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


[pve-devel] [PATCH v3] Add list and dump commands to pveum, refactor API

2018-06-12 Thread Stoiko Ivanov
changes from v2 (sent as two separate threads for pve-common and
pve-access-control on 29.05.2018):

* only changed the coderef invocation syntax in the areas I touched anyways
  (to keep the git history)
* incorporated Thomas feedback (thanks!)
* added another property to the JSONSchema for defining the maximal width used
  for printing.

Am not really sure about the naming of the methods and the property
(print_api_list, print_width) - would be grateful for suggestions

Rene pointed out a first possible enhancement of adding also a minimal width
for printing (will be sent as a separate patch).

Stoiko Ivanov (3):
  add print_width property to JSONSchema definition
  add print_text_table, print_entry to CLIHandler
  add print_api_list, adapt handle_cmd

 src/PVE/CLIHandler.pm | 94 +--
 src/PVE/JSONSchema.pm |  5 +++
 2 files changed, 97 insertions(+), 2 deletions(-)

Stoiko Ivanov (2):
  refactor API by unifying duplicate properties
  pveum: add list and dump commands

 PVE/API2/ACL.pm  | 40 --
 PVE/API2/Group.pm| 38 +---
 PVE/API2/Role.pm | 58 ---
 PVE/API2/User.pm | 97 +++-
 PVE/AccessControl.pm |  2 +-
 PVE/Auth/Plugin.pm   |  2 +-
 PVE/CLI/pveum.pm | 20 +++
 7 files changed, 127 insertions(+), 130 deletions(-)

-- 
2.11.0


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


[pve-devel] [PATCH access-control v3 1/2] refactor API by unifying duplicate properties

2018-06-12 Thread Stoiko Ivanov
* The JSONSchema for most API calls share some properties, which this
 refactoring pulls out in a common_$type_properties hashref in order to
 minimize code duplication (All parameters, which are thus added to the API
 calls were optional)
* Additionally for some fields the title property is added (its not used
  elswhere) - as preparation for unified CLI handling
* PVE/AccessControl::role_is_special now return 0 instead of '' for false
  (Schemavalidation did complain about '')
* For 2 fields a new property (print_width) was added

Signed-off-by: Stoiko Ivanov 
---
 PVE/API2/ACL.pm  | 40 --
 PVE/API2/Group.pm| 38 +---
 PVE/API2/Role.pm | 58 ---
 PVE/API2/User.pm | 97 +++-
 PVE/AccessControl.pm |  2 +-
 PVE/Auth/Plugin.pm   |  2 +-
 6 files changed, 107 insertions(+), 130 deletions(-)

diff --git a/PVE/API2/ACL.pm b/PVE/API2/ACL.pm
index d37771b..64d84e3 100644
--- a/PVE/API2/ACL.pm
+++ b/PVE/API2/ACL.pm
@@ -13,6 +13,20 @@ use PVE::RESTHandler;
 
 use base qw(PVE::RESTHandler);
 
+my $common_acl_properties = {
+propagate => {
+   description => "Allow to propagate (inherit) permissions.",
+   type => 'boolean',
+   title => 'Propagate',
+   optional => 1,
+   default => 1,
+},
+path => {
+   description => "Access control path",
+   title => 'Path',
+   type => 'string',
+},
+};
 __PACKAGE__->register_method ({
 name => 'read_acl',
 path => '',
@@ -31,13 +45,11 @@ __PACKAGE__->register_method ({
items => {
type => "object",
additionalProperties => 0,
-   properties => {
-   path => { type => 'string' },
-   type => { type => 'string', enum => ['user', 'group'] },
-   ugid => { type => 'string' },
-   roleid => { type => 'string' },
-   propagate => { type => 'boolean' },
-   },
+   properties => { (%$common_acl_properties,
+   type => { type => 'string', title => 'Type', enum => ['user', 
'group'] },
+   ugid => { type => 'string', title => 'ID'},
+   roleid => { type => 'string', title => 'Role' },
+   ) },
},
 },
 code => sub {
@@ -89,11 +101,7 @@ __PACKAGE__->register_method ({
 description => "Update Access Control List (add or remove permissions).",
 parameters => {
additionalProperties => 0,
-   properties => {
-   path => {
-   description => "Access control path",
-   type => 'string',
-   },
+   properties => { (%$common_acl_properties,
users => {
description => "List of users.",
type => 'string',  format => 'pve-userid-list',
@@ -108,18 +116,12 @@ __PACKAGE__->register_method ({
description => "List of roles.",
type => 'string', format => 'pve-roleid-list',
},
-   propagate => {
-   description => "Allow to propagate (inherit) permissions.",
-   type => 'boolean',
-   optional => 1,
-   default => 1,
-   },
delete => {
description => "Remove permissions (instead of adding it).",
type => 'boolean',
optional => 1,
},
-   },
+   ) },
 },
 returns => { type => 'null' },
 code => sub {
diff --git a/PVE/API2/Group.pm b/PVE/API2/Group.pm
index fca8a2a..d3c91c8 100644
--- a/PVE/API2/Group.pm
+++ b/PVE/API2/Group.pm
@@ -9,6 +9,16 @@ use PVE::RESTHandler;
 
 use base qw(PVE::RESTHandler);
 
+my $common_group_properties = {
+groupid => {
+type => 'string',
+format => 'pve-groupid',
+title => 'Group ID' ,
+completion => \::AccessControl::complete_group,
+},
+comment => { type => 'string', optional => 1 },
+};
+
 __PACKAGE__->register_method ({
 name => 'index', 
 path => '', 
@@ -26,9 +36,7 @@ __PACKAGE__->register_method ({
type => 'array',
items => {
type => "object",
-   properties => {
-   groupid => { type => 'string' },
-   },
+   properties => $common_group_properties,
},
links => [ { rel => 'child', href => "{groupid}" } ],
 },
@@ -65,10 +73,7 @@ __PACKAGE__->register_method ({
 description => "Create new group.",
 parameters => {
additionalProperties => 0,
-   properties => {
-   groupid => { type => 'string', format => 'pve-groupid' },
-   comment => { type => 'string', optional => 1 },
-   },
+   properties => $common_group_properties,
 },
 returns => { type => 'null' },
 code => sub {
@@ -106,13 +111,7 @@ __PACKAGE__->register_method ({
 description => "Update group data.",
 parameters => {
additionalProperties => 0,
-   properties 

[pve-devel] [PATCH common v3 1/3] add print_width property to JSONSchema definition

2018-06-12 Thread Stoiko Ivanov
used to define the maximal width of the respective column in the CLI

Signed-off-by: Stoiko Ivanov 
---
 src/PVE/JSONSchema.pm | 5 +
 1 file changed, 5 insertions(+)

diff --git a/src/PVE/JSONSchema.pm b/src/PVE/JSONSchema.pm
index f014dc3..2865d1a 100644
--- a/src/PVE/JSONSchema.pm
+++ b/src/PVE/JSONSchema.pm
@@ -1139,6 +1139,11 @@ my $default_schema_noref = {
},
},
},
+   print_width => {
+   type => "integer",
+   description => "For CLI context, this defines the maximal width to 
print before truncating",
+   optional => 1,
+   },
 }  
 };
 
-- 
2.11.0


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


[pve-devel] [PATCH access-control v3 2/2] pveum: add list and dump commands

2018-06-12 Thread Stoiko Ivanov
* Adds list for all objects currently handled in the CLI
* Adds dump for User and Group
* Uses print_api_list and print_entry from PVE::CLIHandler

Signed-off-by: Stoiko Ivanov 
---
 PVE/CLI/pveum.pm | 20 
 1 file changed, 20 insertions(+)

diff --git a/PVE/CLI/pveum.pm b/PVE/CLI/pveum.pm
index a4e584d..e2cab7b 100755
--- a/PVE/CLI/pveum.pm
+++ b/PVE/CLI/pveum.pm
@@ -42,20 +42,40 @@ our $cmddef = {
add=> [ 'PVE::API2::User', 'create_user', ['userid'] ],
modify => [ 'PVE::API2::User', 'update_user', ['userid'] ],
delete => [ 'PVE::API2::User', 'delete_user', ['userid'] ],
+   list   => [ 'PVE::API2::User', 'index', [], {}, sub {
+   my ($data, $returnprops) = @_;
+   PVE::CLIHandler::print_api_list( [ 'userid', 'enable' ], $data, 
$returnprops);
+   } ],
+   dump   => [ 'PVE::API2::User', 'read_user', ['userid'], undef, sub {
+   PVE::CLIHandler::print_entry(shift) } ],
 },
 group => {
add=> [ 'PVE::API2::Group', 'create_group', ['groupid'] ],
modify => [ 'PVE::API2::Group', 'update_group', ['groupid'] ],
delete => [ 'PVE::API2::Group', 'delete_group', ['groupid'] ],
+   list   => [ 'PVE::API2::Group', 'index', [], {}, sub {
+   my ($data, $returnprops) = @_;
+   PVE::CLIHandler::print_api_list( [ 'groupid'], $data, $returnprops);
+   } ],
+   dump   => [ 'PVE::API2::Group', 'read_group', ['groupid'], undef, sub {
+   PVE::CLIHandler::print_entry(shift) } ],
 },
 role => {
add=> [ 'PVE::API2::Role', 'create_role', ['roleid'] ],
modify => [ 'PVE::API2::Role', 'update_role', ['roleid'] ],
delete => [ 'PVE::API2::Role', 'delete_role', ['roleid'] ],
+   list   => [ 'PVE::API2::Role', 'index', [], {}, sub {
+   my ($data, $returnprops) = @_;
+   PVE::CLIHandler::print_api_list( [ 'roleid', 'special', 'privs'], 
$data, $returnprops);
+   } ],
 },
 acl => {
modify => [ 'PVE::API2::ACL', 'update_acl', ['path'], { delete => 0 }],
delete => [ 'PVE::API2::ACL', 'update_acl', ['path'], { delete => 1 }],
+   list   => [ 'PVE::API2::ACL', 'read_acl', [], {}, sub {
+   my ($data, $returnprops) = @_;
+   PVE::CLIHandler::print_api_list( [ 'ugid', 'roleid', 'propagate', 
'path'], $data, $returnprops);
+   } ],
 },
 ticket => [ 'PVE::API2::AccessControl', 'create_ticket', ['username'], 
undef,
sub {
-- 
2.11.0


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


[pve-devel] [PATCH common v3 2/3] add print_text_table, print_entry to CLIHandler

2018-06-12 Thread Stoiko Ivanov
These two function could serve as a generic output sub for various CLI
utilities
 * print_text_table prints an array of objects in a tabular fashion,
   the formating is passed as an array containg hashes with titles, maximal
   lengths and default values. This way we can stay extensible, by adding other
   keys to the formatting options
 * print_entry prints out a single entry, handling array-refs as properties

Signed-off-by: Stoiko Ivanov 
---
 src/PVE/CLIHandler.pm | 63 +++
 1 file changed, 63 insertions(+)

diff --git a/src/PVE/CLIHandler.pm b/src/PVE/CLIHandler.pm
index 316d29d..f87720c 100644
--- a/src/PVE/CLIHandler.pm
+++ b/src/PVE/CLIHandler.pm
@@ -396,6 +396,69 @@ my $print_bash_completion = sub {
 &$print_result(@option_list);
 };
 
+# prints a formatted table with a title row.
+# $formatopts is an array of hashes, with the following keys:
+# 'key' - the key in the data-objects to use for this column
+# 'title' - the title to print above the column, defaults to 'key' - always 
gets printed in full
+# 'cutoff' - the maximal length of the data, overlong values will be truncated
+# 'default' - an optional default value for the column
+# the last column always gets printed in full
+
+sub print_text_table {
+my ($formatopts, $data) = @_;
+my ($formatstring, @keys, @titles, %cutoffs, %defaults, $last_col);
+
+$last_col = $formatopts->[$#{$formatopts}];
+foreach my $col ( @$formatopts ) {
+   my ($key, $title, $cutoff, $default) = @$col{ qw(key title cutoff 
default)};
+
+   push @keys, $key;
+   push @titles, ($title // $key);
+   $defaults{$key} = $default;
+
+   #calculate maximal print width and cutoff
+   my $titlelen = length($title);
+
+   my $longest = $titlelen;
+   foreach my $entry (@$data) {
+   my $len = length($entry->{$key}) // 0;
+   $longest = $len if $len > $longest;
+   }
+
+   if (defined($cutoff)){
+   $cutoff = $cutoff < $longest ? $cutoff : $longest;
+   } else {
+   $cutoff = $longest;
+   }
+   $cutoffs{$key} = $cutoff;
+
+   my $printalign = $cutoff > $titlelen ? '-' : '';
+   if ($col == $last_col) {
+   $formatstring .= "%$printalign$titlelen"."s\n";
+   } else {
+   $formatstring .= "%$printalign$cutoff".'s ';
+   }
+}
+
+printf $formatstring, @titles;
+
+foreach my $entry (sort { $a->{$keys[0]} cmp $b->{$keys[0]} } @$data){
+printf $formatstring, map { substr(($entry->{$_} // $defaults{$_}), 0 
, $cutoffs{$_}) } @keys;
+}
+}
+
+sub print_entry {
+my $entry = shift;
+#TODO: handle objects/hashes as well
+foreach my $item (sort keys %$entry) {
+   if (ref($entry->{$item}) eq 'ARRAY'){
+   printf "%s: [ %s ]\n", $item, join(", ", @{$entry->{$item}});
+   } else {
+   printf "%s: %s\n", $item, $entry->{$item};
+   }
+}
+}
+
 sub verify_api {
 my ($class) = @_;
 
-- 
2.11.0


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


[pve-devel] [PATCH storage 1/1] replace read_password with param_mapping

2018-06-12 Thread Dominik Csapak
we only need this for cifs as this is the only type
of storage where we expect a password

Signed-off-by: Dominik Csapak 
---
 PVE/CLI/pvesm.pm | 15 +--
 1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/PVE/CLI/pvesm.pm b/PVE/CLI/pvesm.pm
index 5774364..5113715 100755
--- a/PVE/CLI/pvesm.pm
+++ b/PVE/CLI/pvesm.pm
@@ -27,8 +27,19 @@ my $KNOWN_EXPORT_FORMATS = ['raw+size', 'tar+size', 
'qcow2+size', 'vmdk+size', '
 
 my $nodename = PVE::INotify::nodename();
 
-sub read_password {
-return PVE::PTY::read_password("Enter Password: ");
+sub param_mapping {
+my ($name) = @_;
+
+my $password_map = [ 'password', sub {
+   my ($value) = @_;
+   return $value if $value;
+   return PVE::PTY::read_password("Enter Password: ");
+   }, '', 1];
+my $mapping = {
+   'cifsscan' => [ $password_map ],
+   'create' => [ $password_map ],
+};
+return $mapping->{$name};
 }
 
 sub setup_environment {
-- 
2.11.0


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


[pve-devel] [PATCH container 1/1] replace read_password with param_mapping

2018-06-12 Thread Dominik Csapak
and replace Term::ReadLine with PVE::PTY

Signed-off-by: Dominik Csapak 
---
 src/PVE/CLI/pct.pm | 23 ++-
 1 file changed, 14 insertions(+), 9 deletions(-)

diff --git a/src/PVE/CLI/pct.pm b/src/PVE/CLI/pct.pm
index 897d595..84560b5 100755
--- a/src/PVE/CLI/pct.pm
+++ b/src/PVE/CLI/pct.pm
@@ -6,10 +6,10 @@ use warnings;
 use POSIX;
 use Fcntl;
 use File::Copy 'copy';
-use Term::ReadLine;
 
 use PVE::SafeSyslog;
 use PVE::Tools qw(extract_param);
+use PVE::PTY;
 use PVE::CpuSet;
 use PVE::Cluster;
 use PVE::INotify;
@@ -76,14 +76,19 @@ __PACKAGE__->register_method ({
return undef;
 }});
 
-sub read_password {
-my $term = new Term::ReadLine ('pct');
-my $attribs = $term->Attribs;
-$attribs->{redisplay_function} = $attribs->{shadow_redisplay};
-my $input = $term->readline('Enter password: ');
-my $conf = $term->readline('Retype password: ');
-die "Passwords do not match.\n" if ($input ne $conf);
-return $input;
+sub param_mapping {
+my ($name) = @_;
+my $mapping = {
+   'create_vm' => [
+   ['password', sub {
+   my ($value) = @_;
+   return $value if $value;
+   return PVE::PTY::get_confirmed_password();
+   }, '', 1],
+   ],
+};
+
+return $mapping->{name};
 }
 
 sub string_param_file_mapping {
-- 
2.11.0


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


[pve-devel] [PATCH access-control 1/2] fix typo in change_passsword

2018-06-12 Thread Dominik Csapak
s/passsword/password/

Signed-off-by: Dominik Csapak 
---
 PVE/API2/AccessControl.pm | 2 +-
 PVE/CLI/pveum.pm  | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/PVE/API2/AccessControl.pm b/PVE/API2/AccessControl.pm
index e48f0cb..afcd2fa 100644
--- a/PVE/API2/AccessControl.pm
+++ b/PVE/API2/AccessControl.pm
@@ -283,7 +283,7 @@ __PACKAGE__->register_method ({
 }});
 
 __PACKAGE__->register_method ({
-name => 'change_passsword', 
+name => 'change_password',
 path => 'password', 
 method => 'PUT',
 permissions => { 
diff --git a/PVE/CLI/pveum.pm b/PVE/CLI/pveum.pm
index aef7089..dbc3034 100755
--- a/PVE/CLI/pveum.pm
+++ b/PVE/CLI/pveum.pm
@@ -44,7 +44,7 @@ our $cmddef = {
print "$res->{ticket}\n";
}],
 
-passwd => [ 'PVE::API2::AccessControl', 'change_passsword', ['userid'] ],
+passwd => [ 'PVE::API2::AccessControl', 'change_password', ['userid'] ],
 
 useradd => [ 'PVE::API2::User', 'create_user', ['userid'] ],
 usermod => [ 'PVE::API2::User', 'update_user', ['userid'] ],
-- 
2.11.0


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


[pve-devel] [PATCH] remove read_password from CLIHandler

2018-06-12 Thread Dominik Csapak
and replace it with param_mapping, since that implements a very
similar, but more general behaviour

also introduce a get_confirmed_password, as suggested by Thomas[1]

[1]: https://pve.proxmox.com/pipermail/pve-devel/2018-June/032338.html

pve-common:

Dominik Csapak (2):
  remove read_password_func from cli handler
  add a generalized 'read and confirm password' sub

 src/PVE/CLIHandler.pm  | 18 --
 src/PVE/JSONSchema.pm  | 15 +--
 src/PVE/PTY.pm |  7 +++
 src/PVE/RESTHandler.pm | 26 +-
 4 files changed, 29 insertions(+), 37 deletions(-)

pve-access-control:

Dominik Csapak (2):
  fix typo in change_passsword
  replace read_password with param_mapping

 PVE/API2/AccessControl.pm |  2 +-
 PVE/CLI/pveum.pm  | 32 +---
 debian/control|  1 -
 test/auth-test.pl | 12 ++--
 4 files changed, 24 insertions(+), 23 deletions(-)

pve-storage:

Dominik Csapak (1):
  replace read_password with param_mapping

 PVE/CLI/pvesm.pm | 15 +--
 1 file changed, 13 insertions(+), 2 deletions(-)

qemu-server:

Dominik Csapak (1):
  use get_confirmed_password

 PVE/CLI/qm.pm | 6 +-
 1 file changed, 1 insertion(+), 5 deletions(-)

pve-container:

Dominik Csapak (1):
  replace read_password with param_mapping

 src/PVE/CLI/pct.pm | 23 ++-
 1 file changed, 14 insertions(+), 9 deletions(-)

-- 
2.11.0


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


[pve-devel] [PATCH common 2/2] add a generalized 'read and confirm password' sub

2018-06-12 Thread Dominik Csapak
to use everywhere we read two passwords and compare them

Signed-off-by: Dominik Csapak 
---
 src/PVE/PTY.pm | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/src/PVE/PTY.pm b/src/PVE/PTY.pm
index 23d76c0..e433c4e 100644
--- a/src/PVE/PTY.pm
+++ b/src/PVE/PTY.pm
@@ -228,6 +228,13 @@ sub read_password($;$$) {
 return $password;
 }
 
+sub get_confirmed_password {
+my $pw1 = read_password('Enter new password: ');
+my $pw2 = read_password('Retype new password: ');
+die "passwords do not match\n" if $pw1 ne $pw2;
+return $pw1;
+}
+
 # Class functions
 
 sub new {
-- 
2.11.0


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


[pve-devel] [PATCH common 1/2] remove read_password_func from cli handler

2018-06-12 Thread Dominik Csapak
we have a param_mapping now, with which we can impement that

e.g. instead of using:

sub read_password {
# read password
}

we can do:

sub param_mapping {
my ($name) = @_;

my $password_map = ['password', sub{
# read password
}, ' [$password_map],
'apicall2' => [$password_map],
...
};

return $mapping->{$name};
}

this has the advantage that we can use different subs for different
api calls (e.g. pveum passwd vs pveum ticket)

Signed-off-by: Dominik Csapak 
---
 src/PVE/CLIHandler.pm  | 18 --
 src/PVE/JSONSchema.pm  | 15 +--
 src/PVE/RESTHandler.pm | 26 +-
 3 files changed, 22 insertions(+), 37 deletions(-)

diff --git a/src/PVE/CLIHandler.pm b/src/PVE/CLIHandler.pm
index 8c911b2..b398f12 100644
--- a/src/PVE/CLIHandler.pm
+++ b/src/PVE/CLIHandler.pm
@@ -127,7 +127,6 @@ sub generate_usage_str {
 $separator //= '';
 $indent //= '';
 
-my $read_password_func = $cli_handler_class->can('read_password');
 my $param_mapping_func = $cli_handler_class->can('param_mapping') ||
$cli_handler_class->can('string_param_file_mapping');
 
@@ -149,7 +148,7 @@ sub generate_usage_str {
$str .= $indent;
$str .= $class->usage_str($name, "$prefix $cmd", $arg_param,
  $fixed_param, $format,
- $read_password_func, 
$param_mapping_func);
+ $param_mapping_func);
$oldclass = $class;
 
} elsif (defined($def->{$cmd}->{alias}) && ($format eq 
'asciidoc')) {
@@ -173,7 +172,7 @@ sub generate_usage_str {
 
$str .= $indent;
$str .= $class->usage_str($name, $prefix, $arg_param, $fixed_param, 
$format,
- $read_password_func, $param_mapping_func);
+ $param_mapping_func);
}
return $str;
 };
@@ -466,7 +465,7 @@ sub setup_environment {
 }
 
 my $handle_cmd  = sub {
-my ($args, $read_password_func, $preparefunc, $param_mapping_func) = @_;
+my ($args, $preparefunc, $param_mapping_func) = @_;
 
 $cmddef->{help} = [ __PACKAGE__, 'help', ['extra-args'] ];
 
@@ -496,13 +495,13 @@ my $handle_cmd  = sub {
 my ($class, $name, $arg_param, $uri_param, $outsub) = @{$def || []};
 $abort->("unknown command '$cmd_str'") if !$class;
 
-my $res = $class->cli_handler($cmd_str, $name, $cmd_args, $arg_param, 
$uri_param, $read_password_func, $param_mapping_func);
+my $res = $class->cli_handler($cmd_str, $name, $cmd_args, $arg_param, 
$uri_param, $param_mapping_func);
 
 &$outsub($res) if $outsub;
 };
 
 my $handle_simple_cmd = sub {
-my ($args, $read_password_func, $preparefunc, $param_mapping_func) = @_;
+my ($args, $preparefunc, $param_mapping_func) = @_;
 
 my ($class, $name, $arg_param, $uri_param, $outsub) = @{$cmddef};
 die "no class specified" if !$class;
@@ -531,7 +530,7 @@ my $handle_simple_cmd = sub {
 
 &$preparefunc() if $preparefunc;
 
-my $res = $class->cli_handler($name, $name, \@ARGV, $arg_param, 
$uri_param, $read_password_func, $param_mapping_func);
+my $res = $class->cli_handler($name, $name, \@ARGV, $arg_param, 
$uri_param, $param_mapping_func);
 
 &$outsub($res) if $outsub;
 };
@@ -552,7 +551,6 @@ sub run_cli_handler {
 
 my $preparefunc = $params{prepare};
 
-my $read_password_func = $class->can('read_password');
 my $param_mapping_func = $cli_handler_class->can('param_mapping') ||
$class->can('string_param_file_mapping');
 
@@ -564,9 +562,9 @@ sub run_cli_handler {
 $cmddef = ${"${class}::cmddef"};
 
 if (ref($cmddef) eq 'ARRAY') {
-   &$handle_simple_cmd(\@ARGV, $read_password_func, $preparefunc, 
$param_mapping_func);
+   &$handle_simple_cmd(\@ARGV, $preparefunc, $param_mapping_func);
 } else {
-   &$handle_cmd(\@ARGV, $read_password_func, $preparefunc, 
$param_mapping_func);
+   &$handle_cmd(\@ARGV, $preparefunc, $param_mapping_func);
 }
 
 exit 0;
diff --git a/src/PVE/JSONSchema.pm b/src/PVE/JSONSchema.pm
index f014dc3..1259195 100644
--- a/src/PVE/JSONSchema.pm
+++ b/src/PVE/JSONSchema.pm
@@ -1333,7 +1333,7 @@ sub method_get_child_link {
 # a way to parse command line parameters, using a 
 # schema to configure Getopt::Long
 sub get_options {
-my ($schema, $args, $arg_param, $fixed_param, $pwcallback, 
$param_mapping_hash) = @_;
+my ($schema, $args, $arg_param, $fixed_param, $param_mapping_hash) = @_;
 
 if (!$schema || !$schema->{properties}) {
raise("too many arguments\n", code => HTTP_BAD_REQUEST)
@@ -1362,11 +1362,6 @@ sub get_options {
# optional and call the mapping function afterwards.
push @getopt, "$prop:s";
push @interactive, [$prop, $mapping->{func}];
-   } elsif ($prop eq 'password' && $pwcallback) {
-  

[pve-devel] [PATCH access-control 2/2] replace read_password with param_mapping

2018-06-12 Thread Dominik Csapak
during this change, replace Term::ReadLine with PVE::PTY
we use this to only ask for the password once on
'pveum ticket'

Signed-off-by: Dominik Csapak 
---
 PVE/CLI/pveum.pm  | 30 --
 debian/control|  1 -
 test/auth-test.pl | 12 ++--
 3 files changed, 22 insertions(+), 21 deletions(-)

diff --git a/PVE/CLI/pveum.pm b/PVE/CLI/pveum.pm
index dbc3034..511cc83 100755
--- a/PVE/CLI/pveum.pm
+++ b/PVE/CLI/pveum.pm
@@ -8,7 +8,6 @@ use PVE::Cluster;
 use PVE::SafeSyslog;
 use PVE::AccessControl;
 use File::Path qw(make_path remove_tree);
-use Term::ReadLine;
 use PVE::INotify;
 use PVE::RPCEnvironment;
 use PVE::API2::User;
@@ -18,6 +17,7 @@ use PVE::API2::ACL;
 use PVE::API2::AccessControl;
 use PVE::JSONSchema qw(get_standard_option);
 use PVE::CLIHandler;
+use PVE::PTY;
 
 use base qw(PVE::CLIHandler);
 
@@ -25,16 +25,26 @@ sub setup_environment {
 PVE::RPCEnvironment->setup_default_cli_env();
 }
 
-sub read_password {
-# return $ENV{PVE_PW_TICKET} if defined($ENV{PVE_PW_TICKET});
+sub param_mapping {
+my ($name) = @_;
 
-my $term = new Term::ReadLine ('pveum');
-my $attribs = $term->Attribs;
-$attribs->{redisplay_function} = $attribs->{shadow_redisplay};
-my $input = $term->readline('Enter new password: ');
-my $conf = $term->readline('Retype new password: ');
-die "Passwords do not match.\n" if ($input ne $conf);
-return $input;
+my $mapping = {
+   'change_password' => [
+   ['password', sub {
+   my ($value) = @_;
+   return $value if $value;
+   return PVE::PTY::get_confirmed_password();
+   }, '', 1]
+   ],
+   'create_ticket' => [
+   ['password', sub {
+   # do not accept values given on cmdline
+   return PVE::PTY::read_password('Enter password: ');
+   }, '', 1]
+   ]
+};
+
+return $mapping->{$name};
 }
 
 our $cmddef = {
diff --git a/debian/control b/debian/control
index 07243f7..0f4d49b 100644
--- a/debian/control
+++ b/debian/control
@@ -22,7 +22,6 @@ Depends: libauthen-pam-perl,
  libnet-ldap-perl,
  libnet-ssleay-perl,
  libpve-common-perl,
- libterm-readline-gnu-perl,
  liburi-perl,
  libwww-perl,
  perl (>= 5.6.0-16),
diff --git a/test/auth-test.pl b/test/auth-test.pl
index 50a7f89..60429a9 100644
--- a/test/auth-test.pl
+++ b/test/auth-test.pl
@@ -1,21 +1,13 @@
 #!/usr/bin/perl -w
 
 use strict;
-use Term::ReadLine;
+use PVE::PTY;
 use PVE::AccessControl;
 
 my $username = shift;
 die "Username missing" if !$username;
-sub read_password {
 
-my $term = new Term::ReadLine ('pveum');
-my $attribs = $term->Attribs;
-$attribs->{redisplay_function} = $attribs->{shadow_redisplay};
-my $input = $term->readline('password: ');
-return $input;
-}
-
-my $password = read_password();
+my $password = PVE::PTY::read_password('password: ');
 PVE::AccessControl::authenticate_user($username,$password);
 
 print "Authentication Successful!!\n";
-- 
2.11.0


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


[pve-devel] [PATCH qemu-server 1/1] use get_confirmed_password

2018-06-12 Thread Dominik Csapak
Signed-off-by: Dominik Csapak 
---
 PVE/CLI/qm.pm | 6 +-
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/PVE/CLI/qm.pm b/PVE/CLI/qm.pm
index c017a59..fe8981f 100755
--- a/PVE/CLI/qm.pm
+++ b/PVE/CLI/qm.pm
@@ -674,11 +674,7 @@ sub param_mapping {
 my $cipassword_map = ['cipassword', sub {
my ($value) = @_;
return $value if $value;
-
-   my $pw = PVE::PTY::read_password('New cloud-init user password: ');
-   my $pw2 = PVE::PTY::read_password('Repeat password: ');
-   die "passwords do not match\n" if $pw ne $pw2;
-   return $pw;
+   return PVE::PTY::get_confirmed_password();
 }, '', 1];
 my $mapping = {
'update_vm' => [$ssh_key_map, $cipassword_map],
-- 
2.11.0


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


[pve-devel] applied: [PATCH firewall] fixup active_chains distinction when deleting chains

2018-06-12 Thread Wolfgang Bumiller
Signed-off-by: Wolfgang Bumiller 
---
With the last ebtables rule merge patches this ebtables/iptables
distinction was missing causing the disabling of a VM's firewall to
produce invalid hash accesses causing this to not be detected as a
change...

 src/PVE/Firewall.pm | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/PVE/Firewall.pm b/src/PVE/Firewall.pm
index 62cbf66..6b39d5d 100644
--- a/src/PVE/Firewall.pm
+++ b/src/PVE/Firewall.pm
@@ -3784,11 +3784,12 @@ sub get_ruleset_status {
 foreach my $chain (sort keys %$active_chains) {
next if defined($ruleset->{$chain});
my $action = 'delete';
+   my $sig = $active_chains->{$chain};
if (defined($change_only_regex)) {
$action = 'ignore' if ($chain !~ m/$change_only_regex/);
$statushash->{$chain}->{rules} = $active_chains->{$chain}->{rules};
+   $sig = $sig->{sig};
}
-   my $sig = $active_chains->{$chain}->{sig};
$statushash->{$chain}->{action} = $action;
$statushash->{$chain}->{sig} = $sig;
print "$action $chain ($sig)\n" if $verbose;
-- 
2.11.0


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


[pve-devel] [PATCH pve-client 1/2] Use get_standard_option for vmid in enter

2018-06-12 Thread René Jochum
Signed-off-by: René Jochum 
---
 PVE/APIClient/Commands/lxc.pm | 5 +
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/PVE/APIClient/Commands/lxc.pm b/PVE/APIClient/Commands/lxc.pm
index 66ad704..4e76f70 100644
--- a/PVE/APIClient/Commands/lxc.pm
+++ b/PVE/APIClient/Commands/lxc.pm
@@ -168,10 +168,7 @@ __PACKAGE__->register_method ({
additionalProperties => 0,
properties => {
remote => get_standard_option('pveclient-remote-name'),
-   vmid => {
-   description => "The container ID",
-   type => 'string',
-   },
+   vmid => get_standard_option('pve-vmid')
},
 },
 returns => { type => 'null'},
-- 
2.11.0

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


[pve-devel] [PATCH pve-client 2/2] Add start command

2018-06-12 Thread René Jochum
---
 PVE/APIClient/Commands/help.pm  |  2 ++
 PVE/APIClient/Commands/start.pm | 39 +++
 PVE/APIClient/Helpers.pm| 20 
 pveclient   |  2 ++
 4 files changed, 63 insertions(+)
 create mode 100644 PVE/APIClient/Commands/start.pm

diff --git a/PVE/APIClient/Commands/help.pm b/PVE/APIClient/Commands/help.pm
index 81d4d4a..c0de8e8 100644
--- a/PVE/APIClient/Commands/help.pm
+++ b/PVE/APIClient/Commands/help.pm
@@ -7,6 +7,7 @@ use PVE::APIClient::Commands::help;
 use PVE::APIClient::Commands::list;
 use PVE::APIClient::Commands::lxc;
 use PVE::APIClient::Commands::remote;
+use PVE::APIClient::Commands::start;
 
 use PVE::CLIHandler;
 
@@ -59,6 +60,7 @@ __PACKAGE__->register_method ({
$assemble_usage_string->('list', 
$PVE::APIClient::Commands::list::cmddef);
$assemble_usage_string->('lxc', $PVE::APIClient::Commands::lxc::cmddef);
$assemble_usage_string->('remote', 
$PVE::APIClient::Commands::remote::cmddef);
+   $assemble_usage_string->('start', 
$PVE::APIClient::Commands::start::cmddef);
 
$text .= "pveclient   {options}\n\n";
 
diff --git a/PVE/APIClient/Commands/start.pm b/PVE/APIClient/Commands/start.pm
new file mode 100644
index 000..4858bec
--- /dev/null
+++ b/PVE/APIClient/Commands/start.pm
@@ -0,0 +1,39 @@
+package PVE::APIClient::Commands::start;
+
+use strict;
+use warnings;
+
+use PVE::APIClient::Helpers;
+use PVE::JSONSchema qw(get_standard_option);
+
+use base qw(PVE::CLIHandler);
+
+__PACKAGE__->register_method ({
+name => 'start',
+path => 'start',
+method => 'POST',
+description => "Start a Qemu VM/LinuX Container.",
+parameters => {
+   additionalProperties => 0,
+   properties => {
+   remote => get_standard_option('pveclient-remote-name'),
+   vmid => get_standard_option('pve-vmid'),
+   },
+},
+returns => { type => 'null'},
+code => sub {
+   my ($param) = @_;
+
+   my $config = PVE::APIClient::Config->load();
+   my $conn = PVE::APIClient::Config->remote_conn($config, 
$param->{remote});
+
+   my $resource = PVE::APIClient::Helpers::get_vmid_resource($conn, 
$param->{vmid});
+
+   
$conn->post("api2/json/nodes/$resource->{node}/$resource->{type}/$resource->{vmid}/status/start",
 {});
+
+   return undef;
+}});
+
+our $cmddef = [ __PACKAGE__, 'start', ['remote', 'vmid']];
+
+1;
diff --git a/PVE/APIClient/Helpers.pm b/PVE/APIClient/Helpers.pm
index e7f2216..6663ce3 100644
--- a/PVE/APIClient/Helpers.pm
+++ b/PVE/APIClient/Helpers.pm
@@ -66,4 +66,24 @@ sub lookup_api_method {
 return $data;
 }
 
+sub get_vmid_resource {
+my ($conn, $vmid) = @_;
+
+my $resources = $conn->get('api2/json/cluster/resources', {type => 'vm'});
+
+my $resource;
+for my $tmp (@$resources) {
+   if ($tmp->{vmid} eq $vmid) {
+   $resource = $tmp;
+   last;
+   }
+}
+
+if (!defined($resource)) {
+   die "\"$vmid\" not found";
+}
+
+return $resource;
+}
+
 1;
diff --git a/pveclient b/pveclient
index 6a06a88..609ad9f 100755
--- a/pveclient
+++ b/pveclient
@@ -16,6 +16,7 @@ use PVE::APIClient::Commands::remote;
 use PVE::APIClient::Commands::list;
 use PVE::APIClient::Commands::lxc;
 use PVE::APIClient::Commands::help;
+use PVE::APIClient::Commands::start;
 
 use JSON;
 
@@ -35,6 +36,7 @@ my $cli_class_handlers = {
 list => 'PVE::APIClient::Commands::list',
 lxc => 'PVE::APIClient::Commands::lxc',
 remote => 'PVE::APIClient::Commands::remote',
+start => 'PVE::APIClient::Commands::start',
 help => 'PVE::APIClient::Commands::help',
 };
 
-- 
2.11.0

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


[pve-devel] applied: [PATCH pve-common] CLIHandler.pm: fix command line completion for simple commands

2018-06-12 Thread Thomas Lamprecht
On 6/12/18 10:57 AM, Dietmar Maurer wrote:
> You can simply test behavior using 'qmrestore' ...
> 
> Signed-off-by: Dietmar Maurer 
> ---
>  src/PVE/CLIHandler.pm | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/src/PVE/CLIHandler.pm b/src/PVE/CLIHandler.pm
> index 8c911b2..316d29d 100644
> --- a/src/PVE/CLIHandler.pm
> +++ b/src/PVE/CLIHandler.pm
> @@ -370,7 +370,6 @@ my $print_bash_completion = sub {
>  };
>  
>  # positional arguments
> -$pos++ if $simple_cmd;
>  if ($pos < scalar(@$arg_param)) {
>   my $pname = $arg_param->[$pos];
>   &$print_parameter_completion($pname);
> 

applied

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


[pve-devel] applied: [PATCH pve-common] api_dump: add $raw_dump options

2018-06-12 Thread Thomas Lamprecht
On 6/11/18 11:23 AM, Dietmar Maurer wrote:
> Allow to return the original tree with all refs. We use this
> with our new pveclient which needs the full api definition.
> Keeping refs makes it possible to store the tree more efficiently.
> 
> Signed-off-by: Dietmar Maurer 
> ---
>  src/PVE/RESTHandler.pm | 21 -
>  1 file changed, 12 insertions(+), 9 deletions(-)
> 
> diff --git a/src/PVE/RESTHandler.pm b/src/PVE/RESTHandler.pm
> index 5e70503..50c37c2 100644
> --- a/src/PVE/RESTHandler.pm
> +++ b/src/PVE/RESTHandler.pm
> @@ -58,7 +58,7 @@ sub api_clone_schema {
>  }
>  
>  sub api_dump_full {
> -my ($tree, $index, $class, $prefix) = @_;
> +my ($tree, $index, $class, $prefix, $raw_dump) = @_;
>  
>  $prefix = '' if !$prefix;
>  
> @@ -70,7 +70,7 @@ sub api_dump_full {
>   $path =~ s/\/+$//;
>  
>   if ($info->{subclass}) {
> - api_dump_full($tree, $index, $info->{subclass}, $path);
> + api_dump_full($tree, $index, $info->{subclass}, $path, $raw_dump);
>   } else {
>   next if !$path;
>  
> @@ -110,12 +110,15 @@ sub api_dump_full {
>   $k eq "path";
>  
>   my $d = $info->{$k};
> - 
> - if ($k eq 'parameters') {
> - $data->{$k} = api_clone_schema($d);
> - } else {
>  
> - $data->{$k} = ref($d) ? clone($d) : $d;
> + if ($raw_dump) {
> + $data->{$k} = $d;
> + } else {
> + if ($k eq 'parameters') {
> + $data->{$k} = api_clone_schema($d);
> + } else {
> + $data->{$k} = ref($d) ? clone($d) : $d;
> + }
>   }
>   } 
>   $res->{info}->{$info->{method}} = $data;
> @@ -173,12 +176,12 @@ sub api_dump_remove_refs {
>  }
>  
>  sub api_dump {
> -my ($class, $prefix) = @_;
> +my ($class, $prefix, $raw_dump) = @_;
>  
>  my $tree = [];
>  
>  my $index = {};
> -api_dump_full($tree, $index, $class);
> +api_dump_full($tree, $index, $class, $prefix, $raw_dump);
>  api_dump_cleanup_tree($tree);
>  return $tree;
>  };
> 

applied

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


Re: [pve-devel] [PATCH manager v3 2/2] Add cephfs to allowed storages for vzdump backup

2018-06-12 Thread Thomas Lamprecht
On 6/11/18 11:31 AM, Alwin Antreich wrote:
> Signed-off-by: Alwin Antreich 
> ---
>  PVE/VZDump.pm | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/PVE/VZDump.pm b/PVE/VZDump.pm
> index a0376ef9..7fc69f98 100644
> --- a/PVE/VZDump.pm
> +++ b/PVE/VZDump.pm
> @@ -223,7 +223,7 @@ sub storage_info {
>   
>  die "can't use storage type '$type' for backup\n" 
>   if (!($type eq 'dir' || $type eq 'nfs' || $type eq 'glusterfs'
> -   || $type eq 'cifs'));
> +   || $type eq 'cifs' || $type eq 'cephfs'));
>  die "can't use storage '$storage' for backups - wrong content type\n" 
>   if (!$scfg->{content}->{backup});
>  
> 

Hmm, déjà vu[1]... ;-)

This checking really needs to be improved! We need some way to tell
if that, and possible other content type related stuff can be done
per storage plugin. That would also help external plugins, as
Wolfgang replied to [1].

You do not need to to this now, but it would be really nice if
someone (maybe You, Stoiko or Rene?) could put it on their todo
list for the near future :-)

cheers,
Thomas

[1]: https://pve.proxmox.com/pipermail/pve-devel/2018-May/031890.html


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


[pve-devel] [PATCH pve-common] CLIHandler.pm: fix command line completion for simple commands

2018-06-12 Thread Dietmar Maurer
You can simply test behavior using 'qmrestore' ...

Signed-off-by: Dietmar Maurer 
---
 src/PVE/CLIHandler.pm | 1 -
 1 file changed, 1 deletion(-)

diff --git a/src/PVE/CLIHandler.pm b/src/PVE/CLIHandler.pm
index 8c911b2..316d29d 100644
--- a/src/PVE/CLIHandler.pm
+++ b/src/PVE/CLIHandler.pm
@@ -370,7 +370,6 @@ my $print_bash_completion = sub {
 };
 
 # positional arguments
-$pos++ if $simple_cmd;
 if ($pos < scalar(@$arg_param)) {
my $pname = $arg_param->[$pos];
&$print_parameter_completion($pname);
-- 
2.11.0

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


[pve-devel] applied: [PATCH qemu-server] api create: cleanup the new config log on error

2018-06-12 Thread Thomas Lamprecht
On 6/12/18 10:50 AM, Wolfgang Bumiller wrote:
> Otherwise cases like trying to restore a protected VM would
> leave a lock in the config.
> 

thanks for the followup, applied

> Signed-off-by: Wolfgang Bumiller 
> ---
>  PVE/API2/Qemu.pm | 36 
>  1 file changed, 32 insertions(+), 4 deletions(-)
> 
> diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
> index a5ab282..c15c71f 100644
> --- a/PVE/API2/Qemu.pm
> +++ b/PVE/API2/Qemu.pm
> @@ -564,7 +564,10 @@ __PACKAGE__->register_method({
>  
>   PVE::AccessControl::add_vm_to_pool($vmid, $pool) if $pool;
>  
> - PVE::API2::Qemu->vm_start({ vmid => $vmid, node => $node }) if 
> $start_after_create;
> + if ($start_after_create) {
> + eval { PVE::API2::Qemu->vm_start({ vmid => $vmid, node => 
> $node }) };
> + warn $@ if $@;
> + }
>   };
>  
>   # ensure no old replication state are exists
> @@ -617,12 +620,37 @@ __PACKAGE__->register_method({
>  
>   if ($start_after_create) {
>   print "Execute autostart\n";
> - PVE::API2::Qemu->vm_start({vmid => $vmid, node => $node});
> + eval { PVE::API2::Qemu->vm_start({vmid => $vmid, node => 
> $node}) };
> + warn $@ if $@;
>   }
>   };
>  
> - my $worker_name = $is_restore ? 'qmrestore' : 'qmcreate';
> - my $code = $is_restore ? $restorefn : $createfn;
> + my ($code, $worker_name);
> + if ($is_restore) {
> + $worker_name = 'qmrestore';
> + $code = sub {
> + eval { $restorefn->() };
> + if (my $err = $@) {
> + eval { PVE::QemuConfig->remove_lock($vmid, 'create') };
> + warn $@ if $@;
> + die $err;
> + }
> + };
> + } else {
> + $worker_name = 'qmcreate';
> + $code = sub {
> + eval { $createfn->() };
> + if (my $err = $@) {
> + eval {
> + my $conffile = PVE::QemuConfig->config_file($vmid);
> + unlink($conffile)
> + or die "failed to remove config file: $@\n";
> + };
> + warn $@ if $@;
> + die $err;
> + }
> + };
> + }
>  
>   return $rpcenv->fork_worker($worker_name, $vmid, $authuser, $code);
>  }});
> 


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


[pve-devel] [PATCH qemu-server] api create: cleanup the new config log on error

2018-06-12 Thread Wolfgang Bumiller
Otherwise cases like trying to restore a protected VM would
leave a lock in the config.

Signed-off-by: Wolfgang Bumiller 
---
 PVE/API2/Qemu.pm | 36 
 1 file changed, 32 insertions(+), 4 deletions(-)

diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
index a5ab282..c15c71f 100644
--- a/PVE/API2/Qemu.pm
+++ b/PVE/API2/Qemu.pm
@@ -564,7 +564,10 @@ __PACKAGE__->register_method({
 
PVE::AccessControl::add_vm_to_pool($vmid, $pool) if $pool;
 
-   PVE::API2::Qemu->vm_start({ vmid => $vmid, node => $node }) if 
$start_after_create;
+   if ($start_after_create) {
+   eval { PVE::API2::Qemu->vm_start({ vmid => $vmid, node => 
$node }) };
+   warn $@ if $@;
+   }
};
 
# ensure no old replication state are exists
@@ -617,12 +620,37 @@ __PACKAGE__->register_method({
 
if ($start_after_create) {
print "Execute autostart\n";
-   PVE::API2::Qemu->vm_start({vmid => $vmid, node => $node});
+   eval { PVE::API2::Qemu->vm_start({vmid => $vmid, node => 
$node}) };
+   warn $@ if $@;
}
};
 
-   my $worker_name = $is_restore ? 'qmrestore' : 'qmcreate';
-   my $code = $is_restore ? $restorefn : $createfn;
+   my ($code, $worker_name);
+   if ($is_restore) {
+   $worker_name = 'qmrestore';
+   $code = sub {
+   eval { $restorefn->() };
+   if (my $err = $@) {
+   eval { PVE::QemuConfig->remove_lock($vmid, 'create') };
+   warn $@ if $@;
+   die $err;
+   }
+   };
+   } else {
+   $worker_name = 'qmcreate';
+   $code = sub {
+   eval { $createfn->() };
+   if (my $err = $@) {
+   eval {
+   my $conffile = PVE::QemuConfig->config_file($vmid);
+   unlink($conffile)
+   or die "failed to remove config file: $@\n";
+   };
+   warn $@ if $@;
+   die $err;
+   }
+   };
+   }
 
return $rpcenv->fork_worker($worker_name, $vmid, $authuser, $code);
 }});
-- 
2.11.0


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


[pve-devel] applied: [PATCH] add create_and_lock_config and use it to implement autostart after create

2018-06-12 Thread Wolfgang Bumiller
applied series, and sending a followup commit...

On Fri, Jun 01, 2018 at 04:37:37PM +0200, Thomas Lamprecht wrote:
> This aligns the VM creation code more with the CT one.
> We to not lock and then execute a worker but rather fork the worker
> first and lock there then, which allows nested flock-ing if needed.
> 
> Add a small helper which creates a new locked (vm creation/restore to
> new) or just locks the existing one (vm restore to existing) before even
> forking the worker - which improves responsiveness and API usability
> as we abort early not only once we forked and checked in the locked
> context of the worker.
> 
> With this then we can add a property which let's the VM auto start after
> it was created successfully.
> 
> This is more a RFC for if the general approach is OK, I did not yet
> tried to test through edge cases.
> 
> Thomas Lamprecht (3):
>   API/create: move locking inside worker
>   reserve config with create lock early
>   api create: allow auto vm start after create finished
> 
>  PVE/API2/Qemu.pm | 61 ++--
>  1 file changed, 33 insertions(+), 28 deletions(-)
> 
> Thomas Lamprecht (1):
>   add create_and_lock_config
> 
>  PVE/AbstractConfig.pm | 13 +
>  1 file changed, 13 insertions(+)
> 
> -- 
> 2.17.1

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


Re: [pve-devel] [PATCH pve-client] Config.pm: add new defaults sections

2018-06-12 Thread René Jochum
LGTM


On 2018-06-12 06:24, Dietmar Maurer wrote:
> And implement a new command to setup defaults.
>
> Signed-off-by: Dietmar Maurer 
> ---
>  PVE/APIClient/Commands/help.pm   |   2 +
>  PVE/APIClient/Commands/remote.pm |  11 +--
>  PVE/APIClient/Config.pm  | 189 
> ---
>  pveclient|   2 +
>  4 files changed, 145 insertions(+), 59 deletions(-)
>
> diff --git a/PVE/APIClient/Commands/help.pm b/PVE/APIClient/Commands/help.pm
> index 407af5e..6d4477b 100644
> --- a/PVE/APIClient/Commands/help.pm
> +++ b/PVE/APIClient/Commands/help.pm
> @@ -5,6 +5,7 @@ use warnings;
>  
>  use PVE::APIClient::Commands::help;
>  use PVE::APIClient::Commands::lxc;
> +use PVE::APIClient::Commands::config;
>  use PVE::APIClient::Commands::remote;
>  
>  use PVE::CLIHandler;
> @@ -57,6 +58,7 @@ __PACKAGE__->register_method ({
>   $assemble_usage_string->('help', 
> $PVE::APIClient::Commands::help::cmddef);
>   $assemble_usage_string->('lxc', $PVE::APIClient::Commands::lxc::cmddef);
>   $assemble_usage_string->('remote', 
> $PVE::APIClient::Commands::remote::cmddef);
> + $assemble_usage_string->('config', 
> $PVE::APIClient::Commands::remote::cmddef);
>  
>   $text .= "pveclient   {options}\n\n";
>  
> diff --git a/PVE/APIClient/Commands/remote.pm 
> b/PVE/APIClient/Commands/remote.pm
> index 781fb63..0f465ea 100644
> --- a/PVE/APIClient/Commands/remote.pm
> +++ b/PVE/APIClient/Commands/remote.pm
> @@ -32,9 +32,10 @@ __PACKAGE__->register_method ({
>  
>   printf("%10s %10s %10s %10s %100s\n", "Name", "Host", "Port", 
> "Username", "Fingerprint");
>   for my $name (keys %{$config->{ids}}) {
> - my $remote = $config->{ids}->{$name};
> - printf("%10s %10s %10s %10s %100s\n", $name, $remote->{'host'},
> -$remote->{'port'} // '-', $remote->{'username'}, 
> $remote->{'fingerprint'} // '-');
> + my $data = $config->{ids}->{$name};
> + next if $data->{type} ne 'remote';
> + printf("%10s %10s %10s %10s %100s\n", $name, $data->{'host'},
> +$data->{'port'} // '-', $data->{'username'}, 
> $data->{'fingerprint'} // '-');
>   }
>  
>   return undef;
> @@ -45,7 +46,7 @@ __PACKAGE__->register_method ({
>  path => 'add',
>  method => 'POST',
>  description => "Add a remote to your config file.",
> -parameters => PVE::APIClient::Config->createSchema(1),
> +parameters => PVE::APIClient::RemoteConfig->createSchema(1),
>  returns => { type => 'null'},
>  code => sub {
>   my ($param) = @_;
> @@ -103,7 +104,7 @@ __PACKAGE__->register_method ({
>  path => 'update',
>  method => 'PUT',
>  description => "Update a remote configuration.",
> -parameters => PVE::APIClient::Config->updateSchema(1),
> +parameters => PVE::APIClient::RemoteConfig->updateSchema(1),
>  returns => { type => 'null'},
>  code => sub {
>   my ($param) = @_;
> diff --git a/PVE/APIClient/Config.pm b/PVE/APIClient/Config.pm
> index 8fa7691..bac8591 100644
> --- a/PVE/APIClient/Config.pm
> +++ b/PVE/APIClient/Config.pm
> @@ -4,84 +4,38 @@ use strict;
>  use warnings;
>  use JSON;
>  
> -use PVE::JSONSchema qw(register_standard_option get_standard_option);
> +use PVE::JSONSchema;
>  use PVE::SectionConfig;
>  use PVE::Tools qw(file_get_contents file_set_contents);
>  
>  use base qw(PVE::SectionConfig);
>  
> +my $remote_namne_regex = qw(\w+);
> +
> +my $defaults_section = '!DEFAULTS';
> +
>  my $complete_remote_name = sub {
>  
>  my $config = PVE::APIClient::Config->load();
>  return [keys %{$config->{ids}}];
>  };
>  
> -register_standard_option('pveclient-remote-name', {
> +PVE::JSONSchema::register_standard_option('pveclient-remote-name', {
>  description => "The name of the remote.",
>  type => 'string',
> -pattern => qr(\w+),
> +pattern => $remote_namne_regex,
>  completion => $complete_remote_name,
>  });
>  
> -
>  my $defaultData = {
>  propertyList => {
>   type => {
>   description => "Section type.",
>   optional => 1,
>   },
> - name => get_standard_option('pveclient-remote-name'),
> - host => {
> - description => "The host.",
> - type => 'string', format => 'address',
> - optional => 1,
> - },
> - username => {
> - description => "The username.",
> - type => 'string',
> - optional => 1,
> - },
> - password => {
> - description => "The users password.",
> - type => 'string',
> - optional => 1,
> - },
> - port => {
> - description => "The port.",
> - type => 'integer',
> - optional => 1,
> - default => 8006,
> - },
> - fingerprint => {
> - description => "Fingerprint.",
> - type => 'string',
> - optional => 1,
> - },
> - comment => {
> - description => "Description.",
> - type => 'string',
> -  

[pve-devel] applied: [PATCH storage v3 1/5] Add missing dependency to ceph-common

2018-06-12 Thread Thomas Lamprecht
On 6/11/18 11:31 AM, Alwin Antreich wrote:
> Signed-off-by: Alwin Antreich 
> ---
>  debian/control | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/debian/control b/debian/control
> index 2cf585a..908dd24 100644
> --- a/debian/control
> +++ b/debian/control
> @@ -27,6 +27,7 @@ Depends: cstream,
>   thin-provisioning-tools,
>   udev,
>   smbclient,
> + ceph-common,
>   cifs-utils,
>   ${perl:Depends},
>  Description: Proxmox VE storage management library
> 

applied, we already have this as pve-qeum-kvm dependency,
so this pulls nothing new in non-pveceph setups.

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


Re: [pve-devel] Make check for linked clones more nuanced

2018-06-12 Thread Thomas Lamprecht
On 6/7/18 2:34 PM, Wolfgang Link wrote:
> Qemu can use qcow2 format to make linked clones.
> LXC we do not have this, and so there is no way to make
> linked clone on directory storage.
> 
> With the 'lxc_clone' feature property we can distinguish between
> LXC and QEMU.
> 

I'm objecting calling it lxc_clone, if we introduce it, either
linkedclone (lclone /maybe/). We want to try to be as general as
possible in pve-storage, it shouldn't care much in it's plugins
feature list which specific technology action (e.g., cloning lxc)
can be done on a storage type, but rather _what_ that action
actually does, or needs to do. And that should be used as name.

But actually the problem could be that we say in the directory
base plugins volume_has_feature method that they can clone raw
images. But they can only fully clone (= copy) it??
And for that the 'copy' feature is already there...
So the question is if the raw format should, and more important
_could_, be removed from directory based storage plugins clone
feature?

I do not want to add extra specialized features if we can avoid
it and without good understanding why it's needed.

cheers,
Thomas

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


Re: [pve-devel] [PATCH client 2/3] config: use more xdg compliant search path

2018-06-12 Thread Wolfgang Bumiller
On Tue, Jun 12, 2018 at 06:34:54AM +0200, Dietmar Maurer wrote:
> I don't really like the idea to use different file names:
> 
>  .config
>  client
>  .pveclient
> 
> ???

It's just two, not 3 ;-)

In case we ever add more tools I didn't want to recreate the mess in
/run ;-) (/run/pve-cluster/ as directory, /run/pve-cluster.pid as file,
/run/pveproxy/pveproxy.pid with the name twice including the pve prefix,
...)

The idea is to use $CONFIG/pve/client and concatenate the parts together
for the 'single dotfile in $HOME' variant as ~/.pveclient.
(Similarly, there's ~/.config/i3/config and ~/.i3config, and of course
some tools do also duplicate the names inside .config/ as well:
~/.config/FreeCAD/FreeCAD.conf, ~/.config/htop/htoprc, ...)
Of course, if we intend to have more than one file we might even want to
make .../pve/client/ a directory or at least give the config a
'.cfg' suffix.

Anyway, I'm open to suggestions. I think Rene wanted to have a .cfg
suffix, while Stoiko argued it would make editors try to use the usual
config highlighting on it which might not work well with our
SectionConfig.

> > +# If XDG_CONFIG_HOME is not set or empty, its default is $HOME/.config
> > +my $base = $xdg || "$home/.config";
> > +
> > +my $filename = "$base/pve/client";
> > +return $filename if -e $filename;
> > +
> > +return "$home/.pveclient" if -e "$home/.pveclient";
> > +
> > +return $filename;
> >  }
> >  
> >  sub load {
> > @@ -119,6 +133,7 @@ sub save {
> >  my ($class, $cfg) = @_;
> >  
> >  my $filename = $class->config_filename();
> > +make_path(dirname($filename));
> >  my $raw = $class->write_config($filename, $cfg);
> >  
> >  file_set_contents($filename, $raw, 0600);
> > -- 
> > 2.11.0

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