Thanks for looking over it. On 2018-06-21 12:20, Dietmar Maurer wrote: > comments inline > >> On June 21, 2018 at 9:25 AM René Jochum <[email protected]> wrote: >> >> >> Signed-off-by: René Jochum <[email protected]> >> --- >> v2: Added defaults per remote. >> v3: Simplified code by using "pveclient remote set" >> >> PVE/APIClient/Commands/lxc.pm | 16 ++++++++++++++-- >> PVE/APIClient/Config.pm | 29 +++++++++++++++++++++++------ >> 2 files changed, 37 insertions(+), 8 deletions(-) >> >> diff --git a/PVE/APIClient/Commands/lxc.pm b/PVE/APIClient/Commands/lxc.pm >> index 3add2dd..0601453 100644 >> --- a/PVE/APIClient/Commands/lxc.pm >> +++ b/PVE/APIClient/Commands/lxc.pm >> @@ -428,7 +428,12 @@ __PACKAGE__->register_method ({ >> '/nodes/{node}/lxc', 'POST', { >> remote => get_standard_option('pveclient-remote-name'), >> vmid => get_standard_option('pve-vmid'), >> - node => get_standard_option('pve-node'), >> + node => { >> + description => "The cluster node name.", >> + type => 'string', format => 'pve-node', >> + optional => 1, >> + default => 'localhost | default node' >> + } > You can use something like: > > node => get_standard_option('pve-node', { > optional => 1, > default => 'localhost | default node' > }), kk, thanks. > > > >> quiet => { >> description => "Suppress log output.", >> type => 'boolean', >> @@ -453,6 +458,13 @@ __PACKAGE__->register_method ({ >> my $background = PVE::APIClient::Tools::extract_param($param, >> 'background'); >> >> my $config = PVE::APIClient::Config->load(); >> + my $remote_config = PVE::APIClient::Config->lookup_remote($config, >> $remote); >> + >> + if (!$node) { >> + $node = $remote_config->{default_node} // 'localhost'; >> + } >> + $param->{storage} = $param->{storage} // >> $remote_config->{default_storage} >> // 'local'; > Is 'local' really a good default (containers are disabled on 'local' per > default)? 'local' was the only one that I could think of. > >> + >> my $conn = PVE::APIClient::Config->remote_conn($config, $remote); >> >> my $upid = $conn->post("/nodes/$node/lxc", $param); >> @@ -496,7 +508,7 @@ __PACKAGE__->register_method ({ >> }}); >> >> our $cmddef = { >> - create => [ __PACKAGE__, 'create', ['remote', 'vmid', 'node']], >> + create => [ __PACKAGE__, 'create', ['remote', 'vmid']], >> destroy => [ __PACKAGE__, 'destroy', ['remote', 'vmid']], >> enter => [ __PACKAGE__, 'enter', ['remote', 'vmid']], >> }; >> diff --git a/PVE/APIClient/Config.pm b/PVE/APIClient/Config.pm >> index a783ab3..73e8f5d 100644 >> --- a/PVE/APIClient/Config.pm >> +++ b/PVE/APIClient/Config.pm >> @@ -261,18 +261,36 @@ sub properties { >> optional => 1, >> maxLength => 4096, >> }, >> + >> + default_node => { >> + description => "The default Node to create guests on.", >> + type => 'string', >> + optional => 1, >> + maxLength => 4096, >> + default => 'localhost', >> + }, > Why don't you use the standard option definition here? Overlooked that I can use it, will do. > >> + default_storage => { >> + description => "The default storage to create guests on.", >> + type => 'string', >> + optional => 1, >> + maxLength => 4096, >> + default => 'local', >> + }, > Why don't you use the standard option 'pve-storage-id' here? Same as above. > >> }; >> } >> >> sub options { >> return { >> - name => { optional => 0 }, >> - host => { optional => 0 }, >> + name => { optional => 1 }, >> + host => { optional => 0, fixed => 1 }, > unrelated change? - should go into a separate patch. > >> comment => { optional => 1 }, >> - username => { optional => 0 }, >> + username => { optional => 0, fixed => 1 }, > same here > >> password => { optional => 1 }, >> - port => { optional => 1 }, >> - fingerprint => { optional => 1 }, >> + port => { optional => 1, fixed => 1 }, >> + fingerprint => { optional => 0, fixed => 1 }, > why 'fixed'? That makes the option none editable. > >> + >> + default_node => { optional => 1 }, >> + default_storage => { optional => 1 }, >> }; >> } >> >> @@ -295,7 +313,6 @@ sub type { >> >> sub options { >> return { >> - name => { optional => 1 }, >> username => { optional => 1 }, >> port => { optional => 1 }, >> }; >> -- >> 2.11.0
_______________________________________________ pve-devel mailing list [email protected] https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
