* [pve-devel] [PATCH-SERIES pve-manager/qemu-server] fix#4689 autofind node with proxyto_callback
@ 2023-05-31 22:28 Alexandre Derumier
2023-05-31 22:28 ` [pve-devel] [PATCH pve-manager 1/1] api2: add /guests path Alexandre Derumier
` (2 more replies)
0 siblings, 3 replies; 5+ messages in thread
From: Alexandre Derumier @ 2023-05-31 22:28 UTC (permalink / raw)
To: pve-devel
Hi,
Currently, to manage qemu && lxc vms, we always need to specify nodename in uri.
This is a problem with automation tools like terraform, where is node is registered
in the state of terraform.
(That mean, than if we move the vm on another node, terraform don't known it, and try to create the vm
again or can't delete the vm,...)
https://github.com/Telmate/terraform-provider-proxmox/issues/168
This can also be a potential problem with race, if we need to query /cluster/ressources to find the node, then another
query on the vm.
I have some discussion with fabian about it:
https://bugzilla.proxmox.com/show_bug.cgi?id=4689
This patch series, find the nodename with proxyto_callback.
a new api endpoint /guests/ is defined:
/guests/(qemu|lxc)/vmid
This patch series currently implement callback for api2::qemu
I'm not sure how to create vm_create when vmid && nodename is not defined.
Currently the callback return localhost, so the vm is created on the called
node.
todo (if this patch serie is ok for you):
api2::qemu::agent (/guest/qemu/vmid/agent)
api2::lxc (/guest/lxc/vmid)
api2::lxc::config (/guest/lxc/vmid/config)
api2::lxc::status (/guest/lxc/vmid/status)
api2::lxc::snapshot (/guest/lxc/vmid/snapshot)
api2::firewall:vm (/guest/(qemu|lxc)/vmid/firewall )
Alexandre Derumier (1):
api2: add /guests path
PVE/API2.pm | 6 +++++
PVE/API2/Guests.pm | 55 ++++++++++++++++++++++++++++++++++++++++++++++
PVE/API2/Makefile | 1 +
3 files changed, 62 insertions(+)
create mode 100644 PVE/API2/Guests.pm
Alexandre Derumier (1):
api2: qemu: add proxyto_callback to find node if not defined
PVE/API2/Qemu.pm | 157 ++++++++++++++++++++++++++++++++---------------
1 file changed, 109 insertions(+), 48 deletions(-)
--
2.30.2
^ permalink raw reply [flat|nested] 5+ messages in thread
* [pve-devel] [PATCH pve-manager 1/1] api2: add /guests path
2023-05-31 22:28 [pve-devel] [PATCH-SERIES pve-manager/qemu-server] fix#4689 autofind node with proxyto_callback Alexandre Derumier
@ 2023-05-31 22:28 ` Alexandre Derumier
2023-05-31 22:28 ` [pve-devel] [PATCH qemu-server 1/1] api2: qemu: add proxyto_callback to find node if not defined Alexandre Derumier
2023-06-03 13:57 ` [pve-devel] [PATCH-SERIES pve-manager/qemu-server] fix#4689 autofind node with proxyto_callback Thomas Lamprecht
2 siblings, 0 replies; 5+ messages in thread
From: Alexandre Derumier @ 2023-05-31 22:28 UTC (permalink / raw)
To: pve-devel
Signed-off-by: Alexandre Derumier <aderumier@odiso.com>
---
PVE/API2.pm | 6 +++++
PVE/API2/Guests.pm | 55 ++++++++++++++++++++++++++++++++++++++++++++++
PVE/API2/Makefile | 1 +
3 files changed, 62 insertions(+)
create mode 100644 PVE/API2/Guests.pm
diff --git a/PVE/API2.pm b/PVE/API2.pm
index 42941fd2..092c6df7 100644
--- a/PVE/API2.pm
+++ b/PVE/API2.pm
@@ -17,6 +17,7 @@ use PVE::API2::Nodes;
use PVE::API2::Pool;
use PVE::API2::AccessControl;
use PVE::API2::Storage::Config;
+use PVE::API2::Guests;
__PACKAGE__->register_method ({
subclass => "PVE::API2::Cluster",
@@ -38,6 +39,11 @@ __PACKAGE__->register_method ({
path => 'access',
});
+__PACKAGE__->register_method ({
+ subclass => "PVE::API2::Guests",
+ path => 'guests',
+});
+
__PACKAGE__->register_method ({
subclass => "PVE::API2::Pool",
path => 'pools',
diff --git a/PVE/API2/Guests.pm b/PVE/API2/Guests.pm
new file mode 100644
index 00000000..9be96393
--- /dev/null
+++ b/PVE/API2/Guests.pm
@@ -0,0 +1,55 @@
+package PVE::API2::Guests;
+
+use strict;
+use warnings;
+
+use PVE::RESTHandler;
+
+use base qw(PVE::RESTHandler);
+
+# preload classes
+use PVE::API2::Qemu;
+use PVE::API2::LXC;
+
+
+__PACKAGE__->register_method ({
+ subclass => "PVE::API2::Qemu",
+ path => 'qemu',
+});
+
+__PACKAGE__->register_method ({
+ subclass => "PVE::API2::LXC",
+ path => 'lxc',
+});
+
+__PACKAGE__->register_method ({
+ name => 'index',
+ path => '',
+ method => 'GET',
+ permissions => { user => 'all' },
+ description => "Directory index.",
+ parameters => {
+ additionalProperties => 0,
+ properties => {},
+ },
+ returns => {
+ type => 'array',
+ items => {
+ type => "object",
+ properties => {
+ subdir => { type => 'string' },
+ },
+ },
+ links => [ { rel => 'child', href => "{subdir}" } ],
+ },
+ code => sub {
+ my ($param) = @_;
+
+ my $res = [
+ { subdir => 'qemu' },
+ { subdir => 'lxc' },
+ ];
+
+ return $res;
+ }});
+1;
diff --git a/PVE/API2/Makefile b/PVE/API2/Makefile
index 97f1cc20..b1ece7d6 100644
--- a/PVE/API2/Makefile
+++ b/PVE/API2/Makefile
@@ -12,6 +12,7 @@ PERLSOURCE = \
Ceph.pm \
Certificates.pm \
Cluster.pm \
+ Guests.pm \
HAConfig.pm \
Hardware.pm \
Network.pm \
--
2.30.2
^ permalink raw reply [flat|nested] 5+ messages in thread
* [pve-devel] [PATCH qemu-server 1/1] api2: qemu: add proxyto_callback to find node if not defined
2023-05-31 22:28 [pve-devel] [PATCH-SERIES pve-manager/qemu-server] fix#4689 autofind node with proxyto_callback Alexandre Derumier
2023-05-31 22:28 ` [pve-devel] [PATCH pve-manager 1/1] api2: add /guests path Alexandre Derumier
@ 2023-05-31 22:28 ` Alexandre Derumier
2023-06-03 13:57 ` [pve-devel] [PATCH-SERIES pve-manager/qemu-server] fix#4689 autofind node with proxyto_callback Thomas Lamprecht
2 siblings, 0 replies; 5+ messages in thread
From: Alexandre Derumier @ 2023-05-31 22:28 UTC (permalink / raw)
To: pve-devel
Signed-off-by: Alexandre Derumier <aderumier@odiso.com>
---
PVE/API2/Qemu.pm | 157 ++++++++++++++++++++++++++++++++---------------
1 file changed, 109 insertions(+), 48 deletions(-)
diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
index 587bb22..52daf5a 100644
--- a/PVE/API2/Qemu.pm
+++ b/PVE/API2/Qemu.pm
@@ -59,6 +59,19 @@ use base qw(PVE::RESTHandler);
my $opt_force_description = "Force physical removal. Without this, we simple remove the disk from the config file and create an additional configuration entry called 'unused[n]', which contains the volume ID. Unlink of unused[n] always cause physical removal.";
+my $find_vm_node = sub {
+ my ($rpcenv, $proxyto, $param) = @_;
+ return $param->{node} if $param->{node};
+ # proxy to the node where the guest resides
+ my $vmid = $param->{vmid};
+ return 'localhost' if !$vmid;
+
+ my $vms = PVE::Cluster::get_vmlist();
+
+ return 'localhost' if !$vms->{ids}->{$vmid};
+ return $vms->{ids}->{$vmid}->{node};
+};
+
my $resolve_cdrom_alias = sub {
my $param = shift;
@@ -659,11 +672,12 @@ __PACKAGE__->register_method({
user => 'all',
},
proxyto => 'node',
+ proxyto_callback => \&$find_vm_node,
protected => 1, # qemu pid files are only readable by root
parameters => {
additionalProperties => 0,
properties => {
- node => get_standard_option('pve-node'),
+ node => get_standard_option('pve-node', { optional => 1}),
full => {
type => 'boolean',
optional => 1,
@@ -684,8 +698,20 @@ __PACKAGE__->register_method({
my $rpcenv = PVE::RPCEnvironment::get();
my $authuser = $rpcenv->get_user();
+ my $vmstatus = undef;
- my $vmstatus = PVE::QemuServer::vmstatus(undef, $param->{full});
+ if (!$param->{node}) {
+ my $vmlist = PVE::Cluster::get_vmlist()->{ids};
+ my $rrd = PVE::Cluster::rrd_dump();
+ for my $vmid (sort keys %$vmlist) {
+ my $data = $vmlist->{$vmid};
+ next if $data->{type} ne 'qemu';
+ my $entry = PVE::API2Tools::extract_vm_stats($vmid, $data, $rrd);
+ $vmstatus->{$vmid} = $entry;
+ }
+ } else {
+ $vmstatus = PVE::QemuServer::vmstatus(undef, $param->{full});
+ }
my $res = [];
foreach my $vmid (keys %$vmstatus) {
@@ -733,11 +759,12 @@ __PACKAGE__->register_method({
},
protected => 1,
proxyto => 'node',
+ proxyto_callback => \&$find_vm_node,
parameters => {
additionalProperties => 0,
properties => PVE::QemuServer::json_config_properties(
{
- node => get_standard_option('pve-node'),
+ node => get_standard_option('pve-node', { optional => 1}),
vmid => get_standard_option('pve-vmid', { completion => \&PVE::Cluster::complete_next_vmid }),
archive => {
description => "The backup archive. Either the file system path to a .tar or .vma file (use '-' to pipe data from stdin) or a proxmox storage backup volume identifier.",
@@ -878,7 +905,6 @@ __PACKAGE__->register_method({
&$check_vm_create_serial_perm($rpcenv, $authuser, $vmid, $pool, $param);
&$check_vm_create_usb_perm($rpcenv, $authuser, $vmid, $pool, $param);
-
&$check_cpu_model_access($rpcenv, $authuser, $param);
$check_drive_param->($param, $storecfg);
@@ -1053,6 +1079,7 @@ __PACKAGE__->register_method({
path => '{vmid}',
method => 'GET',
proxyto => 'node',
+ proxyto_callback => \&$find_vm_node,
description => "Directory index",
permissions => {
user => 'all',
@@ -1060,7 +1087,7 @@ __PACKAGE__->register_method({
parameters => {
additionalProperties => 0,
properties => {
- node => get_standard_option('pve-node'),
+ node => get_standard_option('pve-node', { optional => 1}),
vmid => get_standard_option('pve-vmid'),
},
},
@@ -1125,7 +1152,7 @@ __PACKAGE__->register_method({
parameters => {
additionalProperties => 0,
properties => {
- node => get_standard_option('pve-node'),
+ node => get_standard_option('pve-node', { optional => 1}),
vmid => get_standard_option('pve-vmid'),
timeframe => {
description => "Specify the time frame you are interested in.",
@@ -1171,7 +1198,7 @@ __PACKAGE__->register_method({
parameters => {
additionalProperties => 0,
properties => {
- node => get_standard_option('pve-node'),
+ node => get_standard_option('pve-node', { optional => 1}),
vmid => get_standard_option('pve-vmid'),
timeframe => {
description => "Specify the time frame you are interested in.",
@@ -1200,12 +1227,12 @@ __PACKAGE__->register_method({
"pve2-vm/$param->{vmid}", $param->{timeframe}, $param->{cf});
}});
-
__PACKAGE__->register_method({
name => 'vm_config',
path => '{vmid}/config',
method => 'GET',
proxyto => 'node',
+ proxyto_callback => \&$find_vm_node,
description => "Get the virtual machine configuration with pending configuration " .
"changes applied. Set the 'current' parameter to get the current configuration instead.",
permissions => {
@@ -1214,7 +1241,7 @@ __PACKAGE__->register_method({
parameters => {
additionalProperties => 0,
properties => {
- node => get_standard_option('pve-node'),
+ node => get_standard_option('pve-node', { optional => 1}),
vmid => get_standard_option('pve-vmid', { completion => \&PVE::QemuServer::complete_vmid }),
current => {
description => "Get current values (instead of pending values).",
@@ -1265,6 +1292,7 @@ __PACKAGE__->register_method({
path => '{vmid}/pending',
method => 'GET',
proxyto => 'node',
+ proxyto_callback => \&$find_vm_node,
description => "Get the virtual machine configuration with both current and pending values.",
permissions => {
check => ['perm', '/vms/{vmid}', [ 'VM.Audit' ]],
@@ -1272,7 +1300,7 @@ __PACKAGE__->register_method({
parameters => {
additionalProperties => 0,
properties => {
- node => get_standard_option('pve-node'),
+ node => get_standard_option('pve-node', { optional => 1}),
vmid => get_standard_option('pve-vmid', { completion => \&PVE::QemuServer::complete_vmid }),
},
},
@@ -1324,6 +1352,7 @@ __PACKAGE__->register_method({
path => '{vmid}/cloudinit',
method => 'GET',
proxyto => 'node',
+ proxyto_callback => \&$find_vm_node,
description => "Get the cloudinit configuration with both current and pending values.",
permissions => {
check => ['perm', '/vms/{vmid}', [ 'VM.Audit' ]],
@@ -1331,7 +1360,7 @@ __PACKAGE__->register_method({
parameters => {
additionalProperties => 0,
properties => {
- node => get_standard_option('pve-node'),
+ node => get_standard_option('pve-node', { optional => 1}),
vmid => get_standard_option('pve-vmid', { completion => \&PVE::QemuServer::complete_vmid }),
},
},
@@ -1396,6 +1425,7 @@ __PACKAGE__->register_method({
method => 'PUT',
protected => 1,
proxyto => 'node',
+ proxyto_callback => \&$find_vm_node,
description => "Regenerate and change cloudinit config drive.",
permissions => {
check => ['perm', '/vms/{vmid}', 'VM.Config.Cloudinit'],
@@ -1403,7 +1433,7 @@ __PACKAGE__->register_method({
parameters => {
additionalProperties => 0,
properties => {
- node => get_standard_option('pve-node'),
+ node => get_standard_option('pve-node', { optional => 1}),
vmid => get_standard_option('pve-vmid'),
},
},
@@ -1873,6 +1903,7 @@ __PACKAGE__->register_method({
method => 'POST',
protected => 1,
proxyto => 'node',
+ proxyto_callback => \&$find_vm_node,
description => "Set virtual machine options (asynchrounous API).",
permissions => {
check => ['perm', '/vms/{vmid}', $vm_config_perm_list, any => 1],
@@ -1881,7 +1912,7 @@ __PACKAGE__->register_method({
additionalProperties => 0,
properties => PVE::QemuServer::json_config_properties(
{
- node => get_standard_option('pve-node'),
+ node => get_standard_option('pve-node', { optional => 1}),
vmid => get_standard_option('pve-vmid'),
skiplock => get_standard_option('skiplock'),
delete => {
@@ -1930,6 +1961,7 @@ __PACKAGE__->register_method({
method => 'PUT',
protected => 1,
proxyto => 'node',
+ proxyto_callback => \&$find_vm_node,
description => "Set virtual machine options (synchrounous API) - You should consider using the POST method instead for any actions involving hotplug or storage allocation.",
permissions => {
check => ['perm', '/vms/{vmid}', $vm_config_perm_list, any => 1],
@@ -1938,7 +1970,7 @@ __PACKAGE__->register_method({
additionalProperties => 0,
properties => PVE::QemuServer::json_config_properties(
{
- node => get_standard_option('pve-node'),
+ node => get_standard_option('pve-node', { optional => 1}),
vmid => get_standard_option('pve-vmid', { completion => \&PVE::QemuServer::complete_vmid }),
skiplock => get_standard_option('skiplock'),
delete => {
@@ -1981,6 +2013,7 @@ __PACKAGE__->register_method({
method => 'DELETE',
protected => 1,
proxyto => 'node',
+ proxyto_callback => \&$find_vm_node,
description => "Destroy the VM and all used/owned volumes. Removes any VM specific permissions"
." and firewall rules",
permissions => {
@@ -1989,7 +2022,7 @@ __PACKAGE__->register_method({
parameters => {
additionalProperties => 0,
properties => {
- node => get_standard_option('pve-node'),
+ node => get_standard_option('pve-node', { optional => 1}),
vmid => get_standard_option('pve-vmid', { completion => \&PVE::QemuServer::complete_vmid_stopped }),
skiplock => get_standard_option('skiplock'),
purge => {
@@ -2089,6 +2122,7 @@ __PACKAGE__->register_method({
method => 'PUT',
protected => 1,
proxyto => 'node',
+ proxyto_callback => \&$find_vm_node,
description => "Unlink/delete disk images.",
permissions => {
check => [ 'perm', '/vms/{vmid}', ['VM.Config.Disk']],
@@ -2096,7 +2130,7 @@ __PACKAGE__->register_method({
parameters => {
additionalProperties => 0,
properties => {
- node => get_standard_option('pve-node'),
+ node => get_standard_option('pve-node', { optional => 1}),
vmid => get_standard_option('pve-vmid', { completion => \&PVE::QemuServer::complete_vmid }),
idlist => {
type => 'string', format => 'pve-configid-list',
@@ -2151,7 +2185,7 @@ __PACKAGE__->register_method({
parameters => {
additionalProperties => 0,
properties => {
- node => get_standard_option('pve-node'),
+ node => get_standard_option('pve-node', { optional => 1}),
vmid => get_standard_option('pve-vmid'),
websocket => {
optional => 1,
@@ -2313,7 +2347,7 @@ __PACKAGE__->register_method({
parameters => {
additionalProperties => 0,
properties => {
- node => get_standard_option('pve-node'),
+ node => get_standard_option('pve-node', { optional => 1}),
vmid => get_standard_option('pve-vmid'),
serial=> {
optional => 1,
@@ -2409,7 +2443,7 @@ __PACKAGE__->register_method({
parameters => {
additionalProperties => 0,
properties => {
- node => get_standard_option('pve-node'),
+ node => get_standard_option('pve-node', { optional => 1}),
vmid => get_standard_option('pve-vmid'),
vncticket => {
description => "Ticket from previous call to vncproxy.",
@@ -2461,6 +2495,7 @@ __PACKAGE__->register_method({
method => 'POST',
protected => 1,
proxyto => 'node',
+ proxyto_callback => \&$find_vm_node,
permissions => {
check => ['perm', '/vms/{vmid}', [ 'VM.Console' ]],
},
@@ -2468,7 +2503,7 @@ __PACKAGE__->register_method({
parameters => {
additionalProperties => 0,
properties => {
- node => get_standard_option('pve-node'),
+ node => get_standard_option('pve-node', { optional => 1}),
vmid => get_standard_option('pve-vmid'),
proxy => get_standard_option('spice-proxy', { optional => 1 }),
},
@@ -2505,6 +2540,7 @@ __PACKAGE__->register_method({
path => '{vmid}/status',
method => 'GET',
proxyto => 'node',
+ proxyto_callback => \&$find_vm_node,
description => "Directory index",
permissions => {
user => 'all',
@@ -2512,7 +2548,7 @@ __PACKAGE__->register_method({
parameters => {
additionalProperties => 0,
properties => {
- node => get_standard_option('pve-node'),
+ node => get_standard_option('pve-node', { optional => 1}),
vmid => get_standard_option('pve-vmid'),
},
},
@@ -2550,6 +2586,7 @@ __PACKAGE__->register_method({
path => '{vmid}/status/current',
method => 'GET',
proxyto => 'node',
+ proxyto_callback => \&$find_vm_node,
protected => 1, # qemu pid files are only readable by root
description => "Get virtual machine status.",
permissions => {
@@ -2558,7 +2595,7 @@ __PACKAGE__->register_method({
parameters => {
additionalProperties => 0,
properties => {
- node => get_standard_option('pve-node'),
+ node => get_standard_option('pve-node', { optional => 1}),
vmid => get_standard_option('pve-vmid'),
},
},
@@ -2610,6 +2647,7 @@ __PACKAGE__->register_method({
method => 'POST',
protected => 1,
proxyto => 'node',
+ proxyto_callback => \&$find_vm_node,
description => "Start virtual machine.",
permissions => {
check => ['perm', '/vms/{vmid}', [ 'VM.PowerMgmt' ]],
@@ -2617,7 +2655,7 @@ __PACKAGE__->register_method({
parameters => {
additionalProperties => 0,
properties => {
- node => get_standard_option('pve-node'),
+ node => get_standard_option('pve-node', { optional => 1}),
vmid => get_standard_option('pve-vmid',
{ completion => \&PVE::QemuServer::complete_vmid_stopped }),
skiplock => get_standard_option('skiplock'),
@@ -2775,6 +2813,7 @@ __PACKAGE__->register_method({
method => 'POST',
protected => 1,
proxyto => 'node',
+ proxyto_callback => \&$find_vm_node,
description => "Stop virtual machine. The qemu process will exit immediately. This" .
"is akin to pulling the power plug of a running computer and may damage the VM data",
permissions => {
@@ -2783,7 +2822,7 @@ __PACKAGE__->register_method({
parameters => {
additionalProperties => 0,
properties => {
- node => get_standard_option('pve-node'),
+ node => get_standard_option('pve-node', { optional => 1}),
vmid => get_standard_option('pve-vmid',
{ completion => \&PVE::QemuServer::complete_vmid_running }),
skiplock => get_standard_option('skiplock'),
@@ -2864,6 +2903,7 @@ __PACKAGE__->register_method({
method => 'POST',
protected => 1,
proxyto => 'node',
+ proxyto_callback => \&$find_vm_node,
description => "Reset virtual machine.",
permissions => {
check => ['perm', '/vms/{vmid}', [ 'VM.PowerMgmt' ]],
@@ -2871,7 +2911,7 @@ __PACKAGE__->register_method({
parameters => {
additionalProperties => 0,
properties => {
- node => get_standard_option('pve-node'),
+ node => get_standard_option('pve-node', { optional => 1}),
vmid => get_standard_option('pve-vmid',
{ completion => \&PVE::QemuServer::complete_vmid_running }),
skiplock => get_standard_option('skiplock'),
@@ -2914,6 +2954,7 @@ __PACKAGE__->register_method({
method => 'POST',
protected => 1,
proxyto => 'node',
+ proxyto_callback => \&$find_vm_node,
description => "Shutdown virtual machine. This is similar to pressing the power button on a physical machine." .
"This will send an ACPI event for the guest OS, which should then proceed to a clean shutdown.",
permissions => {
@@ -2922,7 +2963,7 @@ __PACKAGE__->register_method({
parameters => {
additionalProperties => 0,
properties => {
- node => get_standard_option('pve-node'),
+ node => get_standard_option('pve-node', { optional => 1}),
vmid => get_standard_option('pve-vmid',
{ completion => \&PVE::QemuServer::complete_vmid_running }),
skiplock => get_standard_option('skiplock'),
@@ -3022,6 +3063,7 @@ __PACKAGE__->register_method({
method => 'POST',
protected => 1,
proxyto => 'node',
+ proxyto_callback => \&$find_vm_node,
description => "Reboot the VM by shutting it down, and starting it again. Applies pending changes.",
permissions => {
check => ['perm', '/vms/{vmid}', [ 'VM.PowerMgmt' ]],
@@ -3029,7 +3071,7 @@ __PACKAGE__->register_method({
parameters => {
additionalProperties => 0,
properties => {
- node => get_standard_option('pve-node'),
+ node => get_standard_option('pve-node', { optional => 1}),
vmid => get_standard_option('pve-vmid',
{ completion => \&PVE::QemuServer::complete_vmid_running }),
timeout => {
@@ -3073,6 +3115,7 @@ __PACKAGE__->register_method({
method => 'POST',
protected => 1,
proxyto => 'node',
+ proxyto_callback => \&$find_vm_node,
description => "Suspend virtual machine.",
permissions => {
description => "You need 'VM.PowerMgmt' on /vms/{vmid}, and if you have set 'todisk',".
@@ -3083,7 +3126,7 @@ __PACKAGE__->register_method({
parameters => {
additionalProperties => 0,
properties => {
- node => get_standard_option('pve-node'),
+ node => get_standard_option('pve-node', { optional => 1}),
vmid => get_standard_option('pve-vmid',
{ completion => \&PVE::QemuServer::complete_vmid_running }),
skiplock => get_standard_option('skiplock'),
@@ -3167,6 +3210,7 @@ __PACKAGE__->register_method({
method => 'POST',
protected => 1,
proxyto => 'node',
+ proxyto_callback => \&$find_vm_node,
description => "Resume virtual machine.",
permissions => {
check => ['perm', '/vms/{vmid}', [ 'VM.PowerMgmt' ]],
@@ -3174,7 +3218,7 @@ __PACKAGE__->register_method({
parameters => {
additionalProperties => 0,
properties => {
- node => get_standard_option('pve-node'),
+ node => get_standard_option('pve-node', { optional => 1}),
vmid => get_standard_option('pve-vmid',
{ completion => \&PVE::QemuServer::complete_vmid_running }),
skiplock => get_standard_option('skiplock'),
@@ -3241,6 +3285,7 @@ __PACKAGE__->register_method({
method => 'PUT',
protected => 1,
proxyto => 'node',
+ proxyto_callback => \&$find_vm_node,
description => "Send key event to virtual machine.",
permissions => {
check => ['perm', '/vms/{vmid}', [ 'VM.Console' ]],
@@ -3248,7 +3293,7 @@ __PACKAGE__->register_method({
parameters => {
additionalProperties => 0,
properties => {
- node => get_standard_option('pve-node'),
+ node => get_standard_option('pve-node', { optional => 1}),
vmid => get_standard_option('pve-vmid',
{ completion => \&PVE::QemuServer::complete_vmid_running }),
skiplock => get_standard_option('skiplock'),
@@ -3284,6 +3329,7 @@ __PACKAGE__->register_method({
path => '{vmid}/feature',
method => 'GET',
proxyto => 'node',
+ proxyto_callback => \&$find_vm_node,
protected => 1,
description => "Check if feature for virtual machine is available.",
permissions => {
@@ -3292,7 +3338,7 @@ __PACKAGE__->register_method({
parameters => {
additionalProperties => 0,
properties => {
- node => get_standard_option('pve-node'),
+ node => get_standard_option('pve-node', { optional => 1}),
vmid => get_standard_option('pve-vmid'),
feature => {
description => "Feature to check.",
@@ -3351,6 +3397,7 @@ __PACKAGE__->register_method({
method => 'POST',
protected => 1,
proxyto => 'node',
+ proxyto_callback => \&$find_vm_node,
description => "Create a copy of virtual machine/template.",
permissions => {
description => "You need 'VM.Clone' permissions on /vms/{vmid}, and 'VM.Allocate' permissions " .
@@ -3368,7 +3415,7 @@ __PACKAGE__->register_method({
parameters => {
additionalProperties => 0,
properties => {
- node => get_standard_option('pve-node'),
+ node => get_standard_option('pve-node', { optional => 1}),
vmid => get_standard_option('pve-vmid', { completion => \&PVE::QemuServer::complete_vmid }),
newid => get_standard_option('pve-vmid', {
completion => \&PVE::Cluster::complete_next_vmid,
@@ -3708,6 +3755,7 @@ __PACKAGE__->register_method({
method => 'POST',
protected => 1,
proxyto => 'node',
+ proxyto_callback => \&$find_vm_node,
description => "Move volume to different storage or to a different VM.",
permissions => {
description => "You need 'VM.Config.Disk' permissions on /vms/{vmid}, " .
@@ -3718,7 +3766,7 @@ __PACKAGE__->register_method({
parameters => {
additionalProperties => 0,
properties => {
- node => get_standard_option('pve-node'),
+ node => get_standard_option('pve-node', { optional => 1}),
vmid => get_standard_option('pve-vmid', { completion => \&PVE::QemuServer::complete_vmid }),
'target-vmid' => get_standard_option('pve-vmid', {
completion => \&PVE::QemuServer::complete_vmid,
@@ -4170,6 +4218,7 @@ __PACKAGE__->register_method({
method => 'GET',
protected => 1,
proxyto => 'node',
+ proxyto_callback => \&$find_vm_node,
description => "Get preconditions for migration.",
permissions => {
check => ['perm', '/vms/{vmid}', [ 'VM.Migrate' ]],
@@ -4177,7 +4226,7 @@ __PACKAGE__->register_method({
parameters => {
additionalProperties => 0,
properties => {
- node => get_standard_option('pve-node'),
+ node => get_standard_option('pve-node', { optional => 1}),
vmid => get_standard_option('pve-vmid', { completion => \&PVE::QemuServer::complete_vmid }),
target => get_standard_option('pve-node', {
description => "Target node.",
@@ -4271,6 +4320,7 @@ __PACKAGE__->register_method({
method => 'POST',
protected => 1,
proxyto => 'node',
+ proxyto_callback => \&$find_vm_node,
description => "Migrate virtual machine. Creates a new migration task.",
permissions => {
check => ['perm', '/vms/{vmid}', [ 'VM.Migrate' ]],
@@ -4278,7 +4328,7 @@ __PACKAGE__->register_method({
parameters => {
additionalProperties => 0,
properties => {
- node => get_standard_option('pve-node'),
+ node => get_standard_option('pve-node', { optional => 1}),
vmid => get_standard_option('pve-vmid', { completion => \&PVE::QemuServer::complete_vmid }),
target => get_standard_option('pve-node', {
description => "Target node.",
@@ -4436,6 +4486,7 @@ __PACKAGE__->register_method({
method => 'POST',
protected => 1,
proxyto => 'node',
+ proxyto_callback => \&$find_vm_node,
description => "Migrate virtual machine to a remote cluster. Creates a new migration task. EXPERIMENTAL feature!",
permissions => {
check => ['perm', '/vms/{vmid}', [ 'VM.Migrate' ]],
@@ -4443,7 +4494,7 @@ __PACKAGE__->register_method({
parameters => {
additionalProperties => 0,
properties => {
- node => get_standard_option('pve-node'),
+ node => get_standard_option('pve-node', { optional => 1}),
vmid => get_standard_option('pve-vmid', { completion => \&PVE::QemuServer::complete_vmid }),
'target-vmid' => get_standard_option('pve-vmid', { optional => 1 }),
'target-endpoint' => get_standard_option('proxmox-remote', {
@@ -4601,6 +4652,7 @@ __PACKAGE__->register_method({
method => 'POST',
protected => 1,
proxyto => 'node',
+ proxyto_callback => \&$find_vm_node,
description => "Execute QEMU monitor commands.",
permissions => {
description => "Sys.Modify is required for (sub)commands which are not read-only ('info *' and 'help')",
@@ -4609,7 +4661,7 @@ __PACKAGE__->register_method({
parameters => {
additionalProperties => 0,
properties => {
- node => get_standard_option('pve-node'),
+ node => get_standard_option('pve-node', { optional => 1}),
vmid => get_standard_option('pve-vmid'),
command => {
type => 'string',
@@ -4652,6 +4704,7 @@ __PACKAGE__->register_method({
method => 'PUT',
protected => 1,
proxyto => 'node',
+ proxyto_callback => \&$find_vm_node,
description => "Extend volume size.",
permissions => {
check => ['perm', '/vms/{vmid}', [ 'VM.Config.Disk' ]],
@@ -4659,7 +4712,7 @@ __PACKAGE__->register_method({
parameters => {
additionalProperties => 0,
properties => {
- node => get_standard_option('pve-node'),
+ node => get_standard_option('pve-node', { optional => 1}),
vmid => get_standard_option('pve-vmid', { completion => \&PVE::QemuServer::complete_vmid }),
skiplock => get_standard_option('skiplock'),
disk => {
@@ -4780,12 +4833,13 @@ __PACKAGE__->register_method({
check => ['perm', '/vms/{vmid}', [ 'VM.Audit' ]],
},
proxyto => 'node',
+ proxyto_callback => \&$find_vm_node,
protected => 1, # qemu pid files are only readable by root
parameters => {
additionalProperties => 0,
properties => {
vmid => get_standard_option('pve-vmid', { completion => \&PVE::QemuServer::complete_vmid }),
- node => get_standard_option('pve-node'),
+ node => get_standard_option('pve-node', { optional => 1}),
},
},
returns => {
@@ -4864,6 +4918,7 @@ __PACKAGE__->register_method({
method => 'POST',
protected => 1,
proxyto => 'node',
+ proxyto_callback => \&$find_vm_node,
description => "Snapshot a VM.",
permissions => {
check => ['perm', '/vms/{vmid}', [ 'VM.Snapshot' ]],
@@ -4871,7 +4926,7 @@ __PACKAGE__->register_method({
parameters => {
additionalProperties => 0,
properties => {
- node => get_standard_option('pve-node'),
+ node => get_standard_option('pve-node', { optional => 1}),
vmid => get_standard_option('pve-vmid', { completion => \&PVE::QemuServer::complete_vmid }),
snapname => get_standard_option('pve-snapshot-name'),
vmstate => {
@@ -4930,7 +4985,7 @@ __PACKAGE__->register_method({
additionalProperties => 0,
properties => {
vmid => get_standard_option('pve-vmid'),
- node => get_standard_option('pve-node'),
+ node => get_standard_option('pve-node', { optional => 1}),
snapname => get_standard_option('pve-snapshot-name'),
},
},
@@ -4959,6 +5014,7 @@ __PACKAGE__->register_method({
method => 'PUT',
protected => 1,
proxyto => 'node',
+ proxyto_callback => \&$find_vm_node,
description => "Update snapshot metadata.",
permissions => {
check => ['perm', '/vms/{vmid}', [ 'VM.Snapshot' ]],
@@ -4966,7 +5022,7 @@ __PACKAGE__->register_method({
parameters => {
additionalProperties => 0,
properties => {
- node => get_standard_option('pve-node'),
+ node => get_standard_option('pve-node', { optional => 1}),
vmid => get_standard_option('pve-vmid'),
snapname => get_standard_option('pve-snapshot-name'),
description => {
@@ -5015,6 +5071,7 @@ __PACKAGE__->register_method({
path => '{vmid}/snapshot/{snapname}/config',
method => 'GET',
proxyto => 'node',
+ proxyto_callback => \&$find_vm_node,
description => "Get snapshot configuration",
permissions => {
check => ['perm', '/vms/{vmid}', [ 'VM.Snapshot', 'VM.Snapshot.Rollback', 'VM.Audit' ], any => 1],
@@ -5022,7 +5079,7 @@ __PACKAGE__->register_method({
parameters => {
additionalProperties => 0,
properties => {
- node => get_standard_option('pve-node'),
+ node => get_standard_option('pve-node', { optional => 1}),
vmid => get_standard_option('pve-vmid'),
snapname => get_standard_option('pve-snapshot-name'),
},
@@ -5054,6 +5111,7 @@ __PACKAGE__->register_method({
method => 'POST',
protected => 1,
proxyto => 'node',
+ proxyto_callback => \&$find_vm_node,
description => "Rollback VM state to specified snapshot.",
permissions => {
check => ['perm', '/vms/{vmid}', [ 'VM.Snapshot', 'VM.Snapshot.Rollback' ], any => 1],
@@ -5061,7 +5119,7 @@ __PACKAGE__->register_method({
parameters => {
additionalProperties => 0,
properties => {
- node => get_standard_option('pve-node'),
+ node => get_standard_option('pve-node', { optional => 1}),
vmid => get_standard_option('pve-vmid', { completion => \&PVE::QemuServer::complete_vmid }),
snapname => get_standard_option('pve-snapshot-name'),
start => {
@@ -5113,6 +5171,7 @@ __PACKAGE__->register_method({
method => 'DELETE',
protected => 1,
proxyto => 'node',
+ proxyto_callback => \&$find_vm_node,
description => "Delete a VM snapshot.",
permissions => {
check => ['perm', '/vms/{vmid}', [ 'VM.Snapshot' ]],
@@ -5120,7 +5179,7 @@ __PACKAGE__->register_method({
parameters => {
additionalProperties => 0,
properties => {
- node => get_standard_option('pve-node'),
+ node => get_standard_option('pve-node', { optional => 1}),
vmid => get_standard_option('pve-vmid', { completion => \&PVE::QemuServer::complete_vmid }),
snapname => get_standard_option('pve-snapshot-name'),
force => {
@@ -5175,6 +5234,7 @@ __PACKAGE__->register_method({
method => 'POST',
protected => 1,
proxyto => 'node',
+ proxyto_callback => \&$find_vm_node,
description => "Create a Template.",
permissions => {
description => "You need 'VM.Allocate' permissions on /vms/{vmid}",
@@ -5183,7 +5243,7 @@ __PACKAGE__->register_method({
parameters => {
additionalProperties => 0,
properties => {
- node => get_standard_option('pve-node'),
+ node => get_standard_option('pve-node', { optional => 1}),
vmid => get_standard_option('pve-vmid', { completion => \&PVE::QemuServer::complete_vmid_stopped }),
disk => {
optional => 1,
@@ -5249,6 +5309,7 @@ __PACKAGE__->register_method({
path => '{vmid}/cloudinit/dump',
method => 'GET',
proxyto => 'node',
+ proxyto_callback => \&$find_vm_node,
description => "Get automatically generated cloudinit config.",
permissions => {
check => ['perm', '/vms/{vmid}', [ 'VM.Audit' ]],
@@ -5256,7 +5317,7 @@ __PACKAGE__->register_method({
parameters => {
additionalProperties => 0,
properties => {
- node => get_standard_option('pve-node'),
+ node => get_standard_option('pve-node', { optional => 1}),
vmid => get_standard_option('pve-vmid', { completion => \&PVE::QemuServer::complete_vmid }),
type => {
description => 'Config type.',
@@ -5294,7 +5355,7 @@ __PACKAGE__->register_method({
parameters => {
additionalProperties => 0,
properties => {
- node => get_standard_option('pve-node'),
+ node => get_standard_option('pve-node', { optional => 1}),
vmid => get_standard_option('pve-vmid'),
storages => {
type => 'string',
--
2.30.2
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [pve-devel] [PATCH-SERIES pve-manager/qemu-server] fix#4689 autofind node with proxyto_callback
2023-05-31 22:28 [pve-devel] [PATCH-SERIES pve-manager/qemu-server] fix#4689 autofind node with proxyto_callback Alexandre Derumier
2023-05-31 22:28 ` [pve-devel] [PATCH pve-manager 1/1] api2: add /guests path Alexandre Derumier
2023-05-31 22:28 ` [pve-devel] [PATCH qemu-server 1/1] api2: qemu: add proxyto_callback to find node if not defined Alexandre Derumier
@ 2023-06-03 13:57 ` Thomas Lamprecht
2023-06-04 6:40 ` DERUMIER, Alexandre
2 siblings, 1 reply; 5+ messages in thread
From: Thomas Lamprecht @ 2023-06-03 13:57 UTC (permalink / raw)
To: Proxmox VE development discussion, Alexandre Derumier
Hi!
Am 01/06/2023 um 00:28 schrieb Alexandre Derumier:
> Currently, to manage qemu && lxc vms, we always need to specify nodename in uri.
>
> This is a problem with automation tools like terraform, where is node is registered
> in the state of terraform.
What do you need here, the whole API, just some operation on existing VMs
like start, stop, migrate, or just that + VM creation?
> (That mean, than if we move the vm on another node, terraform don't known it, and try to create the vm
> again or can't delete the vm,...)
> https://github.com/Telmate/terraform-provider-proxmox/issues/168
>
> This can also be a potential problem with race, if we need to query /cluster/ressources to find the node, then another
> query on the vm.
>
> I have some discussion with fabian about it:
> https://bugzilla.proxmox.com/show_bug.cgi?id=4689
>
>
> This patch series, find the nodename with proxyto_callback.
>> a new api endpoint /guests/ is defined:
>
> /guests/(qemu|lxc)/vmid
>
exposing the same API through multiple points isn't really REST-y design,
could even break things and would def. need special handling to make this
actually visible in the API schema, and thus pvesh and the api viewer, among
other things.
>
> This patch series currently implement callback for api2::qemu
>
> I'm not sure how to create vm_create when vmid && nodename is not defined.
> Currently the callback return localhost, so the vm is created on the called
> node.
>
> todo (if this patch serie is ok for you):
ATM I'd rather strongly object this, for one to avoid that more work is done that
then might not get accepted, which would be avoidable waste for all of us, and as
temporary reason: this definitively won't get into Proxmox VE 8.0, but as new
functionality, which doesn't breaks existing stuff, it can be added just fine to
8.1 or 8.2 – and so I'd like to postpone in-depth review and accepting it into
the source tree until a few weeks after 8.0 Release where I got a bit more time
to calmly think about this – because the base idea of having such a feature
is definitively compelling and I think quite a few admins and devs that re-integrate
our API would like to see this.
Still, some thoughts that I couldn't suppress ;P:
- the datacenter manager might avoid a lot of such issues already, there we
need to resolve Guest ID's to nodes anyway. But, it'd require having that
setup always, which might not be wanted.
- could putting an adapter between Terraform <-> Proxmox VE API work?
Did not really use Terraform, so I'm just guesstimating here.
- FWIW; the HA stack already is somewhat of a automagic node resolver, but
naturally only for operative things like start/stop/migrate, but if one would
just require those actions then it might be a feasible way that already exists.
- We got the Batch-process API calls Endpoint already and while I actually planned
to remove that for PVE 8 (for other, mostly security reasons), if we'd keep (and
fix) that, one could potentially also add proxying and relaying support there.
That said, I have to admit that the solution you choose, while being a bit hacky is
really not a invasive change code-wise; But, and here still assuming we go that
direction in the first place, I still don't like doing that in such a subtle way.
Rather, I'd add something like a "inherit" property that register_method understands
and then we could re-register API endpoint paths under another path, while letting
the schema and routing actually know about it.
I'd would probably do that even in a lightweight way, i.e., resolved dynamically and
then also calling the "actual" original code site, to avoid potential bugs w.r.t.
imports and global variables from more in-depth copies.
I.e., usage would look something like:
pve-manager:# cat PVE/API2/Guests/Qemu.pm
# ...
use PVE::API2::Qemu; # <- required to ensure the methods we want to inherit from got registered
use base qw(PVE::RESTHandler);
__PACKAGE__->register_method({
name => 'index',
path => '{vmid}',
# ...
code => sub {
return [
{ name => 'config' },
# ...
];
});
__PACKAGE__->register_method({
name => 'set_config',
path => '{vmid}',
method => 'GET',
inherit => 'nodes/{nodes}/qemu/{vmid}/config',
proxyto_callback => \&PVE::API2Tools::resolve_vmid_node,
});
# ... other api endpoint's we like to expose in a node-independent way.
1;
For that to work we'd (probably, I did not check _that_ closely) need to adapt the base
RestHandler's register_method and map_path_to_methods, but for either the changes should
be in reasonable size.
For starters I'd only allow-list a few properties that can be overridden on such a
inheritance, as e.g., exposing the same thing with different privileges might be
questionable at best and give us some security woes.
I had something like above approach in my mind for a few other things already in the past,
e.g., in the Proxmox Mail Gateway we have a few places where we register the essentially
same API endpoint a few times, and do that with some adhoc loop and a bit of not _that_
nice code; but mostly that wasn't so bad to require change and thus I did not felt that
such inheritance was required just due to that.
But, if we do something like you want to have here as end result, above would be for me the
way I'd find the most acceptable for this. Albeit, with the disclaimer that without thinking
this fully through and having my brain somewhat melted by the absolutely huge amount of
packages, and churn it has been through for upcoming PVE 8 ;-P
IMO it would be nice as it would be really explicit, ensure that we can get it easily into
API schema dumps without having to adapt the code managing that (at least not in a big way)
and one could even easily create a CLI tool for cluster-local convenience things like
"start that VM but I don't care where it currently is"
ps. sorry for dumping a lot of loosely organized info/questions/idea here in this reply,
but I tried to get as much into it to remember when returning to this, after PVE 8.0 release
and post-release work is done.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [pve-devel] [PATCH-SERIES pve-manager/qemu-server] fix#4689 autofind node with proxyto_callback
2023-06-03 13:57 ` [pve-devel] [PATCH-SERIES pve-manager/qemu-server] fix#4689 autofind node with proxyto_callback Thomas Lamprecht
@ 2023-06-04 6:40 ` DERUMIER, Alexandre
0 siblings, 0 replies; 5+ messages in thread
From: DERUMIER, Alexandre @ 2023-06-04 6:40 UTC (permalink / raw)
To: pve-devel, t.lamprecht, aderumier
Le samedi 03 juin 2023 à 15:57 +0200, Thomas Lamprecht a écrit :
> Hi!
>
> Am 01/06/2023 um 00:28 schrieb Alexandre Derumier:
> > Currently, to manage qemu && lxc vms, we always need to specify
> > nodename in uri.
> >
> > This is a problem with automation tools like terraform, where is
> > node is registered
> > in the state of terraform.
>
> What do you need here, the whole API, just some operation on existing
> VMs
> like start, stop, migrate, or just that + VM creation?
It's really for all vm operations including modification.
Basicaly, terraform is statefull, than mean than if you create a vm
with param --node X , It'll keep the node x in his state.
if you (or the HA) is moving the vm, terraform don't known about it.
(then if you relaunch terraform, it'll try to recreate the vm again)
>
> > (That mean, than if we move the vm on another node, terraform don't
> > known it, and try to create the vm
> > again or can't delete the vm,...)
> >
> >
> > This can also be a potential problem with race, if we need to query
> > /cluster/ressources to find the node, then another
> > query on the vm.
> >
> > I have some discussion with fabian about it:
> >
> >
> > This patch series, find the nodename with proxyto_callback.
> > > a new api endpoint /guests/ is defined:
> >
> > /guests/(qemu|lxc)/vmid
> >
>
> exposing the same API through multiple points isn't really REST-y
> design,
> could even break things and would def. need special handling to make
> this
> actually visible in the API schema, and thus pvesh and the api
> viewer, among
> other things.
Yes, that's why my v1 was a simple uri rewrite.
>
> >
> > This patch series currently implement callback for api2::qemu
> >
> > I'm not sure how to create vm_create when vmid && nodename is not
> > defined.
> > Currently the callback return localhost, so the vm is created on
> > the called
> > node.
> >
> > todo (if this patch serie is ok for you):
>
>
> ATM I'd rather strongly object this, for one to avoid that more work
> is done that
> then might not get accepted, which would be avoidable waste for all
> of us, and as
> temporary reason: this definitively won't get into Proxmox VE 8.0,
> but as new
> functionality, which doesn't breaks existing stuff, it can be added
> just fine to
> 8.1 or 8.2 – and so I'd like to postpone in-depth review and
> accepting it into
> the source tree until a few weeks after 8.0 Release where I got a bit
> more time
> to calmly think about this – because the base idea of having such a
> feature
> is definitively compelling and I think quite a few admins and devs
> that re-integrate
> our API would like to see this.
>
yes, no problem.
> Still, some thoughts that I couldn't suppress ;P:
>
> - the datacenter manager might avoid a lot of such issues already,
> there we
> need to resolve Guest ID's to nodes anyway. But, it'd require
> having that
> setup always, which might not be wanted.
>
> - could putting an adapter between Terraform <-> Proxmox VE API work?
> Did not really use Terraform, so I'm just guesstimating here.
>
> - FWIW; the HA stack already is somewhat of a automagic node
> resolver, but
> naturally only for operative things like start/stop/migrate, but if
> one would
> just require those actions then it might be a feasible way that
> already exists.
>
> - We got the Batch-process API calls Endpoint already and while I
> actually planned
> to remove that for PVE 8 (for other, mostly security reasons), if
> we'd keep (and
> fix) that, one could potentially also add proxying and relaying
> support there.
>
> That said, I have to admit that the solution you choose, while being
> a bit hacky is
> really not a invasive change code-wise; But, and here still assuming
> we go that
> direction in the first place, I still don't like doing that in such
> a subtle way.
>
> Rather, I'd add something like a "inherit" property that
> register_method understands
> and then we could re-register API endpoint paths under another path,
> while letting
> the schema and routing actually know about it.
>
> I'd would probably do that even in a lightweight way, i.e., resolved
> dynamically and
> then also calling the "actual" original code site, to avoid potential
> bugs w.r.t.
> imports and global variables from more in-depth copies.
>
> I.e., usage would look something like:
>
>
> pve-manager:# cat PVE/API2/Guests/Qemu.pm
>
> # ...
>
> use PVE::API2::Qemu; # <- required to ensure the methods we want to
> inherit from got registered
>
> use base qw(PVE::RESTHandler);
>
> __PACKAGE__->register_method({
> name => 'index',
> path => '{vmid}',
> # ...
> code => sub {
> return [
> { name => 'config' },
> # ...
> ];
> });
>
> __PACKAGE__->register_method({
> name => 'set_config',
> path => '{vmid}',
> method => 'GET',
> inherit => 'nodes/{nodes}/qemu/{vmid}/config',
> proxyto_callback => \&PVE::API2Tools::resolve_vmid_node,
> });
>
> # ... other api endpoint's we like to expose in a node-independent
> way.
>
> 1;
>
>
> For that to work we'd (probably, I did not check _that_ closely) need
> to adapt the base
> RestHandler's register_method and map_path_to_methods, but for either
> the changes should
> be in reasonable size.
>
> For starters I'd only allow-list a few properties that can be
> overridden on such a
> inheritance, as e.g., exposing the same thing with different
> privileges might be
> questionable at best and give us some security woes.
>
> I had something like above approach in my mind for a few other things
> already in the past,
> e.g., in the Proxmox Mail Gateway we have a few places where we
> register the essentially
> same API endpoint a few times, and do that with some adhoc loop and a
> bit of not _that_
> nice code; but mostly that wasn't so bad to require change and thus I
> did not felt that
> such inheritance was required just due to that.
>
> But, if we do something like you want to have here as end result,
> above would be for me the
> way I'd find the most acceptable for this. Albeit, with the
> disclaimer that without thinking
> this fully through and having my brain somewhat melted by the
> absolutely huge amount of
> packages, and churn it has been through for upcoming PVE 8 ;-P
>
> IMO it would be nice as it would be really explicit, ensure that we
> can get it easily into
> API schema dumps without having to adapt the code managing that (at
> least not in a big way)
> and one could even easily create a CLI tool for cluster-local
> convenience things like
> "start that VM but I don't care where it currently is"
>
> ps. sorry for dumping a lot of loosely organized info/questions/idea
> here in this reply,
> but I tried to get as much into it to remember when returning to
> this, after PVE 8.0 release
> and post-release work is done.
>
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2023-06-04 6:40 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-05-31 22:28 [pve-devel] [PATCH-SERIES pve-manager/qemu-server] fix#4689 autofind node with proxyto_callback Alexandre Derumier
2023-05-31 22:28 ` [pve-devel] [PATCH pve-manager 1/1] api2: add /guests path Alexandre Derumier
2023-05-31 22:28 ` [pve-devel] [PATCH qemu-server 1/1] api2: qemu: add proxyto_callback to find node if not defined Alexandre Derumier
2023-06-03 13:57 ` [pve-devel] [PATCH-SERIES pve-manager/qemu-server] fix#4689 autofind node with proxyto_callback Thomas Lamprecht
2023-06-04 6:40 ` DERUMIER, Alexandre
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox