[pve-devel] [PATCH manager] fix #4068: expose fw_cfg through the GUI

2023-03-01 Thread Leo Nunner
Introduces a new UI element to set/edit/delete any number of fw_cfg
arguments in a table-like manner.

Signed-off-by: Leo Nunner 
---
 www/manager6/Makefile|   1 +
 www/manager6/qemu/FirmwareCfgEdit.js | 224 +++
 www/manager6/qemu/Options.js |   6 +
 3 files changed, 231 insertions(+)
 create mode 100644 www/manager6/qemu/FirmwareCfgEdit.js

diff --git a/www/manager6/Makefile b/www/manager6/Makefile
index 05afeda4..bf3c8b20 100644
--- a/www/manager6/Makefile
+++ b/www/manager6/Makefile
@@ -217,6 +217,7 @@ JSSRC=  
\
qemu/Config.js  \
qemu/CreateWizard.js\
qemu/DisplayEdit.js \
+   qemu/FirmwareCfgEdit.js \
qemu/HDEdit.js  \
qemu/HDEfi.js   \
qemu/HDTPM.js   \
diff --git a/www/manager6/qemu/FirmwareCfgEdit.js 
b/www/manager6/qemu/FirmwareCfgEdit.js
new file mode 100644
index ..031042e3
--- /dev/null
+++ b/www/manager6/qemu/FirmwareCfgEdit.js
@@ -0,0 +1,224 @@
+Ext.define('pve-fw-cfg-option', {
+extend: 'Ext.data.Model',
+fields: [
+   { name: 'opt', type: 'string' },
+   { name: 'val', type: 'string' },
+],
+});
+
+Ext.define('PVE.qemu.FirmwareCfgPanel', {
+extend: 'Ext.form.FieldContainer',
+alias: 'widget.pveQemuFirmwareCfgPanel',
+
+config: {}, // store loaded vm config
+store: undefined,
+
+inUpdate: false,
+controller: {
+   xclass: 'Ext.app.ViewController',
+
+   init: function(view) {
+   let me = this;
+   let grid = me.lookup('grid');
+
+   view.store = Ext.create('Ext.data.Store', {
+   model: 'pve-fw-cfg-option',
+   });
+
+   grid.setStore(view.store);
+   },
+
+   addOption: function() {
+   let me = this;
+   me.lookup('grid').getStore().add({});
+   },
+
+   removeOption: function(field) {
+   let me = this;
+   let record = field.getWidgetRecord();
+   me.lookup('grid').getStore().remove(record);
+   me.setMarkerValue();
+   },
+
+   onUpdate: function(record, property, value) {
+   let me = this;
+
+   if (record === undefined) {
+   return;
+   }
+
+   record.set(property, value);
+   record.commit();
+   me.setMarkerValue();
+   },
+
+   onUpdateOption: function(field, value) {
+   let me = this;
+   let record = field.getWidgetRecord();
+
+   me.onUpdate(record, "opt", value);
+   },
+
+   onUpdateValue: function(field, value) {
+   let me = this;
+   let record = field.getWidgetRecord();
+
+   me.onUpdate(record, "val", value);
+   },
+
+   setMarkerValue() {
+   let me = this;
+   let view = me.getView();
+
+   view.inUpdate = true;
+   me.lookup('marker').setValue(view.calculateValue());
+   view.inUpdate = false;
+   },
+
+   control: {
+   "grid textfield[dest=opt]": {
+   change: "onUpdateOption",
+   },
+   "grid textfield[dest=val]": {
+   change: "onUpdateValue",
+   },
+   },
+},
+
+loadConfig: function(config) {
+   let me = this;
+   let marker = me.lookup('marker');
+   let list = PVE.Parser.parsePropertyString(config.fw_cfg);
+   let options = [];
+
+   me.config = config;
+   me.store.removeAll();
+
+   for (const [key, value] of Object.entries(list)) {
+   options.push({
+   opt: key,
+   val: value,
+   });
+   }
+
+   marker.originalValue = config.fw_cfg;
+   marker.value = config.fw_cfg;
+   me.store.setData(options);
+},
+
+calculateValue: function() {
+   let me = this;
+   let ret = [];
+   me.store.each((record) => {
+   ret.push(record.data.opt + "=" + record.data.val);
+   });
+   return ret.join(",");
+},
+
+items: [
+   {
+   xtype: 'grid',
+   reference: 'grid',
+   margin: '0 0 5 0',
+   height: 150,
+   defaults: {
+   sortable: false,
+   hideable: false,
+   draggable: false,
+   },
+   columns: [
+   {
+   xtype: 'widgetcolumn',
+   text: gettext('Option'),
+   dataIndex: 'opt',
+   flex: 1,
+   isFormField: false,
+   widget: {
+   xtype: 'textfield',
+   allowBlank: false,
+   dest: 'opt',
+   emptyText: 'opt/...',
+   },
+   },
+   

[pve-devel] [PATCH qemu-server docs manager] Implement support for fw_cfg

2023-03-01 Thread Leo Nunner
This patch introduces an interface for passing values to a guest via the
fw_cfg parameter. Settings are in the format

opt/$key=$value

Where $key should start with a rfqdn (but doesn't need to). Both plain
strings and files (only from the snippets directory) can be passed through.
An example application for this parameter is for provisioning CoreOS
systems [1].

Files are currently passed through if the user specifies the value as
"storage:snippets/file", there is no implicit file flag.

[1] https://coreos.github.io/ignition/

qemu-server:

Leo Nunner (2):
  fix #4068: implement support for fw_cfg
  test: add cfg2cmd tests for fw_cfg

 PVE/API2/Qemu.pm | 14 +++
 PVE/QemuServer.pm| 35 
 test/cfg2cmd/fw_cfg-files.conf   | 15 
 test/cfg2cmd/fw_cfg-files.conf.cmd   | 30 
 test/cfg2cmd/fw_cfg-strings.conf | 15 
 test/cfg2cmd/fw_cfg-strings.conf.cmd | 30 
 6 files changed, 139 insertions(+)
 create mode 100644 test/cfg2cmd/fw_cfg-files.conf
 create mode 100644 test/cfg2cmd/fw_cfg-files.conf.cmd
 create mode 100644 test/cfg2cmd/fw_cfg-strings.conf
 create mode 100644 test/cfg2cmd/fw_cfg-strings.conf.cmd

docs:

Leo Nunner (1):
  fix #4068: document fw_cfg parameter

 qm.adoc | 18 ++
 1 file changed, 18 insertions(+)

manager:

Leo Nunner (1):
  fix #4068: expose fw_cfg through the GUI

 www/manager6/Makefile|   1 +
 www/manager6/qemu/FirmwareCfgEdit.js | 224 +++
 www/manager6/qemu/Options.js |   6 +
 3 files changed, 231 insertions(+)
 create mode 100644 www/manager6/qemu/FirmwareCfgEdit.js

-- 
2.30.2



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



[pve-devel] [PATCH qemu-server 1/2] fix #4068: implement support for fw_cfg

2023-03-01 Thread Leo Nunner
Implements support for passing values to the fw_cfg argument for QEMU.
If the value looks like a file, the backend checks for the correct
permissions/path and if everything is right, includes it as a file
instead of as a string.

Setting the argument requires the VM.Config.Options permission on the
guest. Including files requires Datastore.Audit and Datastore.Allocate
on the specific storage.

Signed-off-by: Leo Nunner 
---
RFC: I feel like a more implicit option for passing files would be
nicer, but I can't really think of a nice way…

 PVE/API2/Qemu.pm  | 14 ++
 PVE/QemuServer.pm | 35 +++
 2 files changed, 49 insertions(+)

diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
index 587bb22..c03394f 100644
--- a/PVE/API2/Qemu.pm
+++ b/PVE/API2/Qemu.pm
@@ -639,6 +639,8 @@ my $check_vm_modify_config_perm = sub {
# the user needs Disk and PowerMgmt privileges to change the vmstate
# also needs privileges on the storage, that will be checked later
$rpcenv->check_vm_perm($authuser, $vmid, $pool, ['VM.Config.Disk', 
'VM.PowerMgmt' ]);
+   } elsif ($opt eq 'fw_cfg') {
+   $rpcenv->check_vm_perm($authuser, $vmid, $pool, 
['VM.Config.Options']);
} else {
# catches hostpci\d+, args, lock, etc.
# new options will be checked here
@@ -1770,6 +1772,18 @@ my $update_vm_api  = sub {
} elsif ($opt eq 'tags') {
assert_tag_permissions($vmid, $conf->{$opt}, 
$param->{$opt}, $rpcenv, $authuser);
$conf->{pending}->{$opt} = 
PVE::GuestHelpers::get_unique_tags($param->{$opt});
+   } elsif ($opt eq 'fw_cfg') {
+   foreach my $fw_cfg (PVE::Tools::split_list($param->{$opt})) 
{
+   my ($opt, $val) = split("=", $fw_cfg);
+
+   if (my $storage = PVE::Storage::parse_volume_id($val, 
1)) {
+   $rpcenv->check($authuser, "/storage/$storage", 
['Datastore.Audit', 'Datastore.Allocate']);
+
+   my ($path, undef, $type) = 
PVE::Storage::path($storecfg, $val);
+   die "File $val is not in snippets directory\n" if 
$type ne "snippets";
+   }
+   }
+   $conf->{pending}->{$opt} = $param->{$opt};
} else {
$conf->{pending}->{$opt} = $param->{$opt};
 
diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
index 40be44d..c5dea1f 100644
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -723,6 +723,12 @@ EODESCR
description => "List of host cores used to execute guest processes, for 
example: 0,5,8-11",
optional => 1,
 },
+fw_cfg => {
+   type => 'string',
+   optional => 1,
+   description => 'Pass values to the guest via the fw_cfg parameter.',
+   format => 'pve-fw-cfg-list',
+},
 };
 
 my $cicustom_fmt = {
@@ -1076,6 +1082,21 @@ sub verify_volume_id_or_absolute_path {
 return $volid;
 }
 
+PVE::JSONSchema::register_format('pve-fw-cfg', \&verify_fw_cfg);
+sub verify_fw_cfg {
+my ($value, $noerr) = @_;
+
+my $FW_CFG_REGEX = qr/[a-z0-9\.\/:\-]+/;
+
+if ($value =~ m/^(opt\/$FW_CFG_REGEX)=($FW_CFG_REGEX)$/) {
+   return $value;
+}
+
+return if $noerr;
+
+die "unable to parse fw_cfg option\n";
+}
+
 my $usb_fmt = {
 host => {
default_key => 1,
@@ -4181,6 +4202,20 @@ sub config_to_command {
push @$cmd, '-snapshot';
 }
 
+if ($conf->{fw_cfg}) {
+   foreach my $conf (PVE::Tools::split_list($conf->{fw_cfg})) {
+   my ($opt, $val) = split("=", $conf);
+
+   push @$cmd, "-fw_cfg";
+   if (PVE::Storage::parse_volume_id($val, 1)) {
+   my $path = PVE::Storage::path($storecfg, $val);
+   push @$cmd, "$opt,file=$path";
+   } else {
+   push @$cmd, "$opt,string=$val";
+   }
+   }
+}
+
 # add custom args
 if ($conf->{args}) {
my $aa = PVE::Tools::split_args($conf->{args});
-- 
2.30.2



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


[pve-devel] [PATCH docs] fix #4068: document fw_cfg parameter

2023-03-01 Thread Leo Nunner
Signed-off-by: Leo Nunner 
---
 qm.adoc | 18 ++
 1 file changed, 18 insertions(+)

diff --git a/qm.adoc b/qm.adoc
index bd535a2..0c587ad 100644
--- a/qm.adoc
+++ b/qm.adoc
@@ -1139,6 +1139,24 @@ http://localhost:9843 in a browser in the guest.
 
 It can help to restart the SPICE session.
 
+[[qm_fw_cfg]]
+QEMU Firmware Configuration
+~~~
+
+QEMU allows passing values to the guest via the 'fw_cfg' parameter. Settings
+are defined on a key/value basis, where the key follows a directory-like 
structure:
+
+
+opt/com.proxmox/test1=local:snippets/config.txt
+opt/com.proxmox/test2=foobar
+
+
+These settings can be edited in the GUI via 'Options' -> 'QEMU Firmware 
Configuration'.
+User-supplied keys *must* be prefixed with `opt/` and are recommended to start 
with a
+reverse fully qualified domain name. `opt/ovmf/` and `opt/org.qemu/` are 
reserved for
+internal use by QEMU and should not be set. Setting a value in the format
+`storage:snippets/file` causes the contents of the file to be included for the 
key.
+
 [[qm_migration]]
 Migration
 -
-- 
2.30.2



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



[pve-devel] [PATCH qemu-server 2/2] test: add cfg2cmd tests for fw_cfg

2023-03-01 Thread Leo Nunner
Signed-off-by: Leo Nunner 
---
 test/cfg2cmd/fw_cfg-files.conf   | 15 ++
 test/cfg2cmd/fw_cfg-files.conf.cmd   | 30 
 test/cfg2cmd/fw_cfg-strings.conf | 15 ++
 test/cfg2cmd/fw_cfg-strings.conf.cmd | 30 
 4 files changed, 90 insertions(+)
 create mode 100644 test/cfg2cmd/fw_cfg-files.conf
 create mode 100644 test/cfg2cmd/fw_cfg-files.conf.cmd
 create mode 100644 test/cfg2cmd/fw_cfg-strings.conf
 create mode 100644 test/cfg2cmd/fw_cfg-strings.conf.cmd

diff --git a/test/cfg2cmd/fw_cfg-files.conf b/test/cfg2cmd/fw_cfg-files.conf
new file mode 100644
index 000..a8117d1
--- /dev/null
+++ b/test/cfg2cmd/fw_cfg-files.conf
@@ -0,0 +1,15 @@
+# TEST: Config with file values for fw_cfg
+# QEMU_VERSION: 7.0.0
+bios: ovmf
+bootdisk: scsi0
+cores: 2
+efidisk0: local:100/vm-100-disk-1.qcow2,size=128K
+fw_cfg: 
opt/com.proxmox/test1=local:snippets/test1,opt/com.proxmox/test2=cifs-store:snippets/test2
+machine: q35
+meta: creation-qemu=6.1
+memory: 512
+net0: virtio=2E:01:68:F9:9C:87,bridge=vmbr0
+ostype: l26
+scsihw: virtio-scsi-pci
+smbios1: uuid=3dd750ce-d910-44d0-9493-525c0be4e687
+vmgenid: 54d1c06c-8f5b-440f-b5b2-6eab1380e13d
diff --git a/test/cfg2cmd/fw_cfg-files.conf.cmd 
b/test/cfg2cmd/fw_cfg-files.conf.cmd
new file mode 100644
index 000..7d14053
--- /dev/null
+++ b/test/cfg2cmd/fw_cfg-files.conf.cmd
@@ -0,0 +1,30 @@
+/usr/bin/kvm \
+  -id 8006 \
+  -name 'vm8006,debug-threads=on' \
+  -no-shutdown \
+  -chardev 
'socket,id=qmp,path=/var/run/qemu-server/8006.qmp,server=on,wait=off' \
+  -mon 'chardev=qmp,mode=control' \
+  -chardev 'socket,id=qmp-event,path=/var/run/qmeventd.sock,reconnect=5' \
+  -mon 'chardev=qmp-event,mode=control' \
+  -pidfile /var/run/qemu-server/8006.pid \
+  -daemonize \
+  -smbios 'type=1,uuid=3dd750ce-d910-44d0-9493-525c0be4e687' \
+  -drive 
'if=pflash,unit=0,format=raw,readonly=on,file=/usr/share/pve-edk2-firmware//OVMF_CODE.fd'
 \
+  -drive 
'if=pflash,unit=1,id=drive-efidisk0,format=qcow2,file=/var/lib/vz/images/100/vm-100-disk-1.qcow2'
 \
+  -smp '2,sockets=1,cores=2,maxcpus=2' \
+  -nodefaults \
+  -boot 
'menu=on,strict=on,reboot-timeout=1000,splash=/usr/share/qemu-server/bootsplash.jpg'
 \
+  -vnc 'unix:/var/run/qemu-server/8006.vnc,password=on' \
+  -cpu kvm64,enforce,+kvm_pv_eoi,+kvm_pv_unhalt,+lahf_lm,+sep \
+  -m 512 \
+  -readconfig /usr/share/qemu-server/pve-q35-4.0.cfg \
+  -device 'vmgenid,guid=54d1c06c-8f5b-440f-b5b2-6eab1380e13d' \
+  -device 'usb-tablet,id=tablet,bus=ehci.0,port=1' \
+  -device 'VGA,id=vga,bus=pcie.0,addr=0x1' \
+  -device 
'virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3,free-page-reporting=on' \
+  -iscsi 'initiator-name=iqn.1993-08.org.debian:01:aabbccddeeff' \
+  -netdev 
'type=tap,id=net0,ifname=tap8006i0,script=/var/lib/qemu-server/pve-bridge,downscript=/var/lib/qemu-server/pve-bridgedown,vhost=on'
 \
+  -device 
'virtio-net-pci,mac=2E:01:68:F9:9C:87,netdev=net0,bus=pci.0,addr=0x12,id=net0,bootindex=300'
 \
+  -machine 'type=q35+pve0' \
+  -fw_cfg 'opt/com.proxmox/test1,file=/var/lib/vz/snippets/test1' \
+  -fw_cfg 'opt/com.proxmox/test2,file=/mnt/pve/cifs-store/snippets/test2'
diff --git a/test/cfg2cmd/fw_cfg-strings.conf b/test/cfg2cmd/fw_cfg-strings.conf
new file mode 100644
index 000..1d3631b
--- /dev/null
+++ b/test/cfg2cmd/fw_cfg-strings.conf
@@ -0,0 +1,15 @@
+# TEST: Config with simple string values for fw_cfg
+# QEMU_VERSION: 7.0.0
+bios: ovmf
+bootdisk: scsi0
+cores: 2
+efidisk0: local:100/vm-100-disk-1.qcow2,size=128K
+fw_cfg: opt/com.proxmox/test1=first,opt/com.proxmox/test2=second
+machine: q35
+meta: creation-qemu=6.1
+memory: 512
+net0: virtio=2E:01:68:F9:9C:87,bridge=vmbr0
+ostype: l26
+scsihw: virtio-scsi-pci
+smbios1: uuid=3dd750ce-d910-44d0-9493-525c0be4e687
+vmgenid: 54d1c06c-8f5b-440f-b5b2-6eab1380e13d
diff --git a/test/cfg2cmd/fw_cfg-strings.conf.cmd 
b/test/cfg2cmd/fw_cfg-strings.conf.cmd
new file mode 100644
index 000..9ff7dc2
--- /dev/null
+++ b/test/cfg2cmd/fw_cfg-strings.conf.cmd
@@ -0,0 +1,30 @@
+/usr/bin/kvm \
+  -id 8006 \
+  -name 'vm8006,debug-threads=on' \
+  -no-shutdown \
+  -chardev 
'socket,id=qmp,path=/var/run/qemu-server/8006.qmp,server=on,wait=off' \
+  -mon 'chardev=qmp,mode=control' \
+  -chardev 'socket,id=qmp-event,path=/var/run/qmeventd.sock,reconnect=5' \
+  -mon 'chardev=qmp-event,mode=control' \
+  -pidfile /var/run/qemu-server/8006.pid \
+  -daemonize \
+  -smbios 'type=1,uuid=3dd750ce-d910-44d0-9493-525c0be4e687' \
+  -drive 
'if=pflash,unit=0,format=raw,readonly=on,file=/usr/share/pve-edk2-firmware//OVMF_CODE.fd'
 \
+  -drive 
'if=pflash,unit=1,id=drive-efidisk0,format=qcow2,file=/var/lib/vz/images/100/vm-100-disk-1.qcow2'
 \
+  -smp '2,sockets=1,cores=2,maxcpus=2' \
+  -nodefaults \
+  -boot 
'menu=on,strict=on,reboot-timeout=1000,splash=/usr/share/qemu-server/bootsplash.jpg'
 \
+  -vnc 'unix:/var/run/qemu-server/8006.vnc,password=on' \
+  -cpu kvm64,enforce,+kvm_pv_eoi,+kvm_pv_unhalt,+lahf_lm

Re: [pve-devel] [PATCH manager 2/2] ui: cert upload: fix private key field sending empty string

2023-03-01 Thread Matthias Heiserer

On 28.02.2023 17:36, Max Carrara wrote:

The private key's field is now excluded from the upload form's
JSON data if it's considered empty by Ext.js.

Prior to this change, the form still sent an empty string if no
private key was provided by the user, even though the private key's
field is marked as optional in `pve-manager/PVE/API2/Certificates.pm`,
causing the JSONSchema validation to fail.

Signed-off-by: Max Carrara 
---
  www/manager6/node/Certificates.js | 7 +++
  1 file changed, 7 insertions(+)

diff --git a/www/manager6/node/Certificates.js 
b/www/manager6/node/Certificates.js
index 34013b44..4c0b18d1 100644
--- a/www/manager6/node/Certificates.js
+++ b/www/manager6/node/Certificates.js
@@ -173,6 +173,13 @@ Ext.define('PVE.node.CertUpload', {
emptyText: gettext('No change'),
name: 'key',
xtype: 'textarea',
+   getSubmitValue: function() {
+   let value = this.getValue();
+   if (Ext.isEmpty(value)) {
+   return null;
+   }
+   return value;

I guess you could save space and write "return this.getValue() || null;"
The SubmitValue is a string anyways, so behaviour for Ext.isEmpty and || 
should be the same 
(https://docs.sencha.com/extjs/7.0.0/classic/Ext.html#method-isEmpty and 
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Logical_OR)

+   },
},
{
xtype: 'filebutton',




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



Re: [pve-devel] [PATCH common 1/2] certificate: add subroutine that checks if cert and key match

2023-03-01 Thread Fabian Grünbichler
On February 28, 2023 5:36 pm, Max Carrara wrote:
> This is done here in order to allow other packages to make use of
> this subroutine.
> 
> Signed-off-by: Max Carrara 
> ---
>  src/PVE/Certificate.pm | 26 ++
>  1 file changed, 26 insertions(+)
> 
> diff --git a/src/PVE/Certificate.pm b/src/PVE/Certificate.pm
> index 31a7722..5ec1c6b 100644
> --- a/src/PVE/Certificate.pm
> +++ b/src/PVE/Certificate.pm
> @@ -228,6 +228,32 @@ sub get_certificate_fingerprint {
>  return $fp;
>  }
>  
> +sub certificate_matches_key {
> +my ($cert_path, $key_path) = @_;
> +
> +die "No certificate path given!\n" if !$cert_path;
> +die "No certificate key path given!\n" if !$key_path;
> +
> +die "Certificate at '$cert_path' does not exist!\n" if ! -e $cert_path;
> +die "Certificate key '$key_path' does not exist!\n" if ! -e $key_path;
> +
> +my $ctx = Net::SSLeay::CTX_new()
> + or $ssl_die->(
> + "failed to create SSL context in order to verify private key\n"
> + );

probably everything after this point [continued at [0]]

> +
> +Net::SSLeay::set_cert_and_key($ctx, $cert_path, $key_path);

this can also fail, so should get "or $ssl_die->(..)"

> +
> +my $result = Net::SSLeay::CTX_check_private_key($ctx);
> +
> +$ssl_warn->("Failed to validate private key and certificate\n")
> + if !$result;

this could simply be CTX_check_private_key($ctx) or $ssl_die->(..);

> +

[0]: until this should go into an eval block, so that we can capture $@

> +Net::SSLeay::CTX_free($ctx);

then always run this, and then re-die if we had an error

> +
> +return $result;

this could then return 1;, since all errors above would already be handled.. or
return nothing at all since it would just die in case of an error anyway..

so basically

...
..setup ctx or ssl_die..
eval {
all the stuff that could fail with or ssl_die
}
my $err = $@;
..destroy ctx..
die "... $err" if $err;

> +}
> +
>  sub get_certificate_info {
>  my ($cert_path) = @_;
>  
> -- 
> 2.30.2
> 
> 
> 
> ___
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
> 
> 
> 


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



Re: [pve-devel] [PATCH common 2/2] certificate: fix formatting and whitespace

2023-03-01 Thread Fabian Grünbichler
On February 28, 2023 5:36 pm, Max Carrara wrote:
> Signed-off-by: Max Carrara 
> ---
>  src/PVE/Certificate.pm | 23 ---
>  1 file changed, 12 insertions(+), 11 deletions(-)
> 
> diff --git a/src/PVE/Certificate.pm b/src/PVE/Certificate.pm
> index 5ec1c6b..31def4c 100644
> --- a/src/PVE/Certificate.pm
> +++ b/src/PVE/Certificate.pm
> @@ -385,19 +385,19 @@ sub generate_csr {
>  $ssl_die->("Failed to allocate X509_NAME object\n") if !$name;
>  my $add_name_entry = sub {
>   my ($k, $v) = @_;
> - if (!Net::SSLeay::X509_NAME_add_entry_by_txt($name,
> -  $k,
> -  
> &Net::SSLeay::MBSTRING_UTF8,
> -  encode('utf-8', $v))) {
> + if (!Net::SSLeay::X509_NAME_add_entry_by_txt(
> + $name, $k, &Net::SSLeay::MBSTRING_UTF8, encode('utf-8', $v)

not sure if I wouldn't prefer

if (!Net::SSLeay::X509_NAME_add_entry_by_txt(
$name,
$k,
&Net::SSLeay::MBSTRING_UTF8,
encode('utf-8', $v),
)) {

here, it's kinda ugly in both variants to be honest ;)

maybe 

my $res = Net::SSLeay::X509_NAME_add_entry_by_txt(
$name,
$k,
&Net::SSLeay::MBSTRING_UTF8,
encode('utf-8', $v),
);

$cleanup->(..) if !$res;

would be nicer?

> + )
> + ) {
>   $cleanup->(1, "Failed to add '$k'='$v' to DN\n");
>   }
>  };
>  
>  $add_name_entry->('CN', $common_name);
>  for (qw(C ST L O OU)) {
> -if (defined(my $v = $attr{$_})) {
> + if (defined(my $v = $attr{$_})) {
>   $add_name_entry->($_, $v);
> -}
> + }
>  }
>  
>  if (defined($pem_key)) {
> @@ -431,11 +431,12 @@ sub generate_csr {
>   if (!Net::SSLeay::X509_REQ_set_subject_name($req, $name));
>  
>  $cleanup->(1, "Failed to add extensions to CSR\n")
> - if !Net::SSLeay::P_X509_REQ_add_extensions($req,
> - &Net::SSLeay::NID_key_usage => 
> 'digitalSignature,keyEncipherment',
> - &Net::SSLeay::NID_basic_constraints => 'CA:FALSE',
> - &Net::SSLeay::NID_ext_key_usage => 'serverAuth,clientAuth',
> - &Net::SSLeay::NID_subject_alt_name => join(',', map { "DNS:$_" 
> } @$san),
> + if !Net::SSLeay::P_X509_REQ_add_extensions(
> + $req,
> + &Net::SSLeay::NID_key_usage => 'digitalSignature,keyEncipherment',
> + &Net::SSLeay::NID_basic_constraints => 'CA:FALSE',
> + &Net::SSLeay::NID_ext_key_usage => 'serverAuth,clientAuth',
> + &Net::SSLeay::NID_subject_alt_name => join(',', map { "DNS:$_" } 
> @$san),
>   );

this one is actually against our style guide in both old and new, it should use
a regular if with proper wrapping, not a post-if, like the first hunk of this
patch :)

https://pve.proxmox.com/wiki/Perl_Style_Guide#Breaking_long_lines_and_strings

could also use the 'store result in variable' style if it helps with 
readability.

>  
>  $cleanup->(1, "Failed to set public key\n")
> -- 
> 2.30.2
> 
> 
> 
> ___
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
> 
> 
> 


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



[pve-devel] [PATCH-SERIES v3 storage/docs] fix #2920: add options parameter to CIFS plugin

2023-03-01 Thread Fiona Ebner
similar to the already existing parameter for NFS.

Changes v2 -> v3:
* Rebase on current master.
* Minor style fixes.

Changes v1 -> v2:
# pve-storage (1/2)
* fixed nitpicks

# pve-docs (2/2)
* extended options explanation
* changed example option to `echo_interval=30` as second parameter


storage:

Stefan Hrdlicka (1):
  fix #2920: cifs: add options parameter

 PVE/Storage/CIFSPlugin.pm | 4 +++-
 PVE/Storage/NFSPlugin.pm  | 4 
 PVE/Storage/Plugin.pm | 6 ++
 3 files changed, 9 insertions(+), 5 deletions(-)


docs:

Stefan Hrdlicka (1):
  fix #2920: add cifs options parameter

 pve-storage-cifs.adoc | 8 
 1 file changed, 8 insertions(+)

-- 
2.30.2



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



[pve-devel] [PATCH v3 docs 1/1] fix #2920: add cifs options parameter

2023-03-01 Thread Fiona Ebner
From: Stefan Hrdlicka 

Signed-off-by: Stefan Hrdlicka 
[FE: rebase + style fix]
Signed-off-by: Fiona Ebner 
---

Changes from v2:
* use {pve} instead of PVE

 pve-storage-cifs.adoc | 8 
 1 file changed, 8 insertions(+)

diff --git a/pve-storage-cifs.adoc b/pve-storage-cifs.adoc
index e0d4106..6008730 100644
--- a/pve-storage-cifs.adoc
+++ b/pve-storage-cifs.adoc
@@ -61,6 +61,13 @@ content-dirs::
 
 Overrides for the default directory layout. Optional.
 
+options::
+
+Additional CIFS mount options (see `man mount.cifs`). Some options are set
+automatically and shouldn't be set here. {pve} will always set the option
+`soft`. Depending on the configuration, these options are set automatically:
+`username`, `credentials`, `guest`, `domain`, `vers`.
+
 .Configuration Example (`/etc/pve/storage.cfg`)
 
 cifs: backup
@@ -68,6 +75,7 @@ cifs: backup
server 10.0.0.11
share VMData
content backup
+   options noserverino,echo_interval=30
username anna
smbversion 3
 
-- 
2.30.2



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



[pve-devel] [PATCH v3 storage 1/1] fix #2920: cifs: add options parameter

2023-03-01 Thread Fiona Ebner
From: Stefan Hrdlicka 

This makes it possible to add all mount options offered by mount.cifs.
NFS & CIFS now share the options parameter since they use it for the
same purpose.

Signed-off-by: Stefan Hrdlicka 
[FE: rebase + style fixes]
Signed-off-by: Fiona Ebner 
---

Changes from v2:
* minor improvements in commit message
* adapt to recently changed cifs_mount function interface

 PVE/Storage/CIFSPlugin.pm | 4 +++-
 PVE/Storage/NFSPlugin.pm  | 4 
 PVE/Storage/Plugin.pm | 6 ++
 3 files changed, 9 insertions(+), 5 deletions(-)

diff --git a/PVE/Storage/CIFSPlugin.pm b/PVE/Storage/CIFSPlugin.pm
index 6e20f4b..996ef44 100644
--- a/PVE/Storage/CIFSPlugin.pm
+++ b/PVE/Storage/CIFSPlugin.pm
@@ -69,7 +69,7 @@ sub get_cred_file {
 sub cifs_mount : prototype($) {
 my ($scfg, $storeid, $smbver, $user, $domain) = @_;
 
-my ($mountpoint, $server, $share) = $scfg->@{'path', 'server', 'share'};
+my ($mountpoint, $server, $share, $options) = $scfg->@{'path', 'server', 
'share', 'options'};
 my $subdir = $scfg->{subdir} // "/";
 
 $server = "[$server]" if Net::IP::ip_is_ipv6($server);
@@ -85,6 +85,7 @@ sub cifs_mount : prototype($) {
 }
 
 push @$cmd, '-o', defined($smbver) ? "vers=$smbver" : "vers=default";
+push @$cmd, '-o', $options if $options;
 
 run_command($cmd, errmsg => "mount error");
 }
@@ -152,6 +153,7 @@ sub options {
mkdir => { optional => 1 },
bwlimit => { optional => 1 },
preallocation => { optional => 1 },
+   options => { optional => 1 },
 };
 }
 
diff --git a/PVE/Storage/NFSPlugin.pm b/PVE/Storage/NFSPlugin.pm
index c2d4176..54423cd 100644
--- a/PVE/Storage/NFSPlugin.pm
+++ b/PVE/Storage/NFSPlugin.pm
@@ -69,10 +69,6 @@ sub properties {
description => "Server IP or DNS name.",
type => 'string', format => 'pve-storage-server',
},
-   options => {
-   description => "NFS mount options (see 'man nfs')",
-   type => 'string',  format => 'pve-storage-options',
-   },
 };
 }
 
diff --git a/PVE/Storage/Plugin.pm b/PVE/Storage/Plugin.pm
index ca7a0d4..d08c8aa 100644
--- a/PVE/Storage/Plugin.pm
+++ b/PVE/Storage/Plugin.pm
@@ -191,6 +191,12 @@ my $defaultData = {
type => "string", format => "pve-dir-override-list",
optional => 1,
},
+   options => {
+   description => "NFS/CIFS mount options (see 'man nfs' or 'man 
mount.cifs')",
+   type => 'string',
+   format => 'pve-storage-options',
+   optional => 1,
+   },
 },
 };
 
-- 
2.30.2



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



[pve-devel] [PATCH container 2/3] net: Drop unused `firewall` argument to add_bridge_fdb()

2023-03-01 Thread Christoph Heiss
PVE::Network::SDN::Zones::add_bridge_fdb() does not actually have a
`firewall` parameter, so drop it.

No functional changes.

Signed-off-by: Christoph Heiss 
---
 src/PVE/LXC.pm | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/PVE/LXC.pm b/src/PVE/LXC.pm
index 54ee0d9..cd0aeb1 100644
--- a/src/PVE/LXC.pm
+++ b/src/PVE/LXC.pm
@@ -923,7 +923,7 @@ sub net_tap_plug : prototype($$;$) {

 if ($have_sdn) {
PVE::Network::SDN::Zones::tap_plug($iface, $bridge, $tag, $firewall, 
$trunks, $rate);
-   PVE::Network::SDN::Zones::add_bridge_fdb($iface, $opts->{mac}, $bridge, 
$firewall)
+   PVE::Network::SDN::Zones::add_bridge_fdb($iface, $opts->{mac}, $bridge)
if defined($opts->{mac});
 } else {
PVE::Network::tap_plug($iface, $bridge, $tag, $firewall, $trunks, 
$rate, $opts);
--
2.39.2



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



[pve-devel] [PATCH qemu-server 3/3] net: Drop unused `firewall` argument to {add, del}_bridge_fdb()

2023-03-01 Thread Christoph Heiss
PVE::Network::{add,del}_bridge_fdb() do not actually have a `firewall`
parameter, so drop it. Same for the SDN equivalents.

No functional changes.

Signed-off-by: Christoph Heiss 
---
 PVE/QemuServer.pm | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
index 40be44d..9e5b0a0 100644
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -8561,9 +8561,9 @@ sub add_nets_bridge_fdb {
 
my $bridge = $net->{bridge};
if ($have_sdn) {
-   PVE::Network::SDN::Zones::add_bridge_fdb($iface, $mac, $bridge, 
$net->{firewall});
+   PVE::Network::SDN::Zones::add_bridge_fdb($iface, $mac, $bridge);
} elsif (-d "/sys/class/net/$bridge/bridge") { # avoid fdb management 
with OVS for now
-   PVE::Network::add_bridge_fdb($iface, $mac, $net->{firewall});
+   PVE::Network::add_bridge_fdb($iface, $mac);
}
 }
 }
@@ -8580,9 +8580,9 @@ sub del_nets_bridge_fdb {
 
my $bridge = $net->{bridge};
if ($have_sdn) {
-   PVE::Network::SDN::Zones::del_bridge_fdb($iface, $mac, $bridge, 
$net->{firewall});
+   PVE::Network::SDN::Zones::del_bridge_fdb($iface, $mac, $bridge);
} elsif (-d "/sys/class/net/$bridge/bridge") { # avoid fdb management 
with OVS for now
-   PVE::Network::del_bridge_fdb($iface, $mac, $net->{firewall});
+   PVE::Network::del_bridge_fdb($iface, $mac);
}
 }
 }
-- 
2.39.2



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



[pve-devel] [PATCH network 1/3] zones: Drop unused `firewall` argument to {add, del}_bridge_fdb()

2023-03-01 Thread Christoph Heiss
PVE::Network::{add,del}_bridge_fdb() do not actually have a `firewall`
parameter, so drop it. And since it wasn't used anywhere else in these
subroutines, drop it completely.

No functional changes.

Signed-off-by: Christoph Heiss 
---
 PVE/Network/SDN/Zones.pm | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/PVE/Network/SDN/Zones.pm b/PVE/Network/SDN/Zones.pm
index f8e40b1..58f9df8 100644
--- a/PVE/Network/SDN/Zones.pm
+++ b/PVE/Network/SDN/Zones.pm
@@ -326,31 +326,31 @@ sub tap_plug {
 }

 sub add_bridge_fdb {
-my ($iface, $macaddr, $bridge, $firewall) = @_;
+my ($iface, $macaddr, $bridge) = @_;

 my $vnet = PVE::Network::SDN::Vnets::get_vnet($bridge, 1);
 if (!$vnet) { # fallback for classic bridge
-   PVE::Network::add_bridge_fdb($iface, $macaddr, $firewall);
+   PVE::Network::add_bridge_fdb($iface, $macaddr);
return;
 }

 my $plugin_config = get_plugin_config($vnet);
 my $plugin = 
PVE::Network::SDN::Zones::Plugin->lookup($plugin_config->{type});
-PVE::Network::add_bridge_fdb($iface, $macaddr, $firewall) if 
$plugin_config->{'bridge-disable-mac-learning'};
+PVE::Network::add_bridge_fdb($iface, $macaddr) if 
$plugin_config->{'bridge-disable-mac-learning'};
 }

 sub del_bridge_fdb {
-my ($iface, $macaddr, $bridge, $firewall) = @_;
+my ($iface, $macaddr, $bridge) = @_;

 my $vnet = PVE::Network::SDN::Vnets::get_vnet($bridge, 1);
 if (!$vnet) { # fallback for classic bridge
-   PVE::Network::del_bridge_fdb($iface, $macaddr, $firewall);
+   PVE::Network::del_bridge_fdb($iface, $macaddr);
return;
 }

 my $plugin_config = get_plugin_config($vnet);
 my $plugin = 
PVE::Network::SDN::Zones::Plugin->lookup($plugin_config->{type});
-PVE::Network::del_bridge_fdb($iface, $macaddr, $firewall) if 
$plugin_config->{'bridge-disable-mac-learning'};
+PVE::Network::del_bridge_fdb($iface, $macaddr) if 
$plugin_config->{'bridge-disable-mac-learning'};
 }

 1;
--
2.39.2



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



[pve-devel] [PATCH network/container/qemu-server 0/3] Drop unused `firewall` argument to {add, del}_bridge_fdb()

2023-03-01 Thread Christoph Heiss
While working on this code, I noticed that ACAICS the `firewall`
argument is never used (nor even declared!) [0] in both of
PVE::Network::{add,del}_bridge_fdb().

Thus drop it everywhere and avoid needlessly passing around things which
are never used anyway.

Did some quick smoke-testing and everything kept working fine, but as
there are really no functional changes, this should not effect anything.

[0] 
https://git.proxmox.com/?p=pve-common.git;a=blob;f=src/PVE/Network.pm;h=355637b1;hb=HEAD#l299

pve-network:

Christoph Heiss (1):
  zones: Drop unused `firewall` argument to {add,del}_bridge_fdb()

 PVE/Network/SDN/Zones.pm | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

pve-container:

Christoph Heiss (1):
  net: Drop unused `firewall` argument to add_bridge_fdb()

 src/PVE/LXC.pm | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

qemu-server:

Christoph Heiss (1):
  net: Drop unused `firewall` argument to {add,del}_bridge_fdb()

 PVE/QemuServer.pm | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

--
2.39.2



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



[pve-devel] [PATCH v2 qemu-server] feature #3937: config: store user in meta property

2023-03-01 Thread Leo Nunner
Adds a field to the "meta" config property which stores the user who
created the VM.

Signed-off-by: Leo Nunner 
---
 PVE/QemuServer.pm | 8 
 1 file changed, 8 insertions(+)

diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
index 40be44d..0a7a6b0 100644
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -287,6 +287,11 @@ my $meta_info_fmt = {
pattern => '\d+(\.\d+)+',
optional => 1,
 },
+'creation-user' => {
+   type => 'string',
+   description => "The user who created the VM.",
+   optional => 1,
+},
 };
 
 my $confdesc = {
@@ -2205,10 +2210,13 @@ sub parse_meta_info {
 sub new_meta_info_string {
 my () = @_; # for now do not allow to override any value
 
+my $rpcenv = PVE::RPCEnvironment->get();
+
 return PVE::JSONSchema::print_property_string(
{
'creation-qemu' => kvm_user_version(),
ctime => "". int(time()),
+   'creation-user' => $rpcenv->get_user(),
},
$meta_info_fmt
 );
-- 
2.30.2



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



[pve-devel] [PATCH v2 container 2/2] feature #3937: config: introduce meta property

2023-03-01 Thread Leo Nunner
Introduces a 'meta' property like the one already existing for VMs.
Currently, it holds the creation time (ctime), the LXC version at the
time of creation, and the user who created the container (cuser).

Signed-off-by: Leo Nunner 
---
 src/PVE/API2/LXC.pm   |  2 ++
 src/PVE/LXC/Config.pm | 42 ++
 2 files changed, 44 insertions(+)

diff --git a/src/PVE/API2/LXC.pm b/src/PVE/API2/LXC.pm
index 50c9eaf..93c23fa 100644
--- a/src/PVE/API2/LXC.pm
+++ b/src/PVE/API2/LXC.pm
@@ -317,6 +317,8 @@ __PACKAGE__->register_method({
 
my $conf = {};
 
+   $conf->{meta} = PVE::LXC::Config::new_meta_info_string();
+
my $is_root = $authuser eq 'root@pam';
 
my $no_disk_param = {};
diff --git a/src/PVE/LXC/Config.pm b/src/PVE/LXC/Config.pm
index aca72ae..544f148 100644
--- a/src/PVE/LXC/Config.pm
+++ b/src/PVE/LXC/Config.pm
@@ -442,6 +442,26 @@ my $features_desc = {
 },
 };
 
+my $meta_info_fmt = {
+'ctime' => {
+   type => 'integer',
+   description => "The container creation timestamp as UNIX epoch time",
+   minimum => 0,
+   optional => 1,
+},
+'creation-lxc' => {
+   type => 'string',
+   description => "The LXC version from the time this VM was created.",
+   pattern => '\d+\.\d+',
+   optional => 1,
+},
+'creation-user' => {
+   type => 'string',
+   description => "The user who created the container.",
+   optional => 1,
+},
+};
+
 my $confdesc = {
 lock => {
optional => 1,
@@ -612,6 +632,12 @@ my $confdesc = {
description => "Try to be more verbose. For now this only enables debug 
log-level on start.",
default => 0,
 },
+meta => {
+   type => 'string',
+   format => $meta_info_fmt,
+   description => "Some (read-only) meta-information about this guest.",
+   optional => 1,
+},
 };
 
 my $valid_lxc_conf_keys = {
@@ -1185,6 +1211,7 @@ sub json_config_properties {
 my $skip_json_config_opts = {
parent => 1,
snaptime => 1,
+   meta => 1,
 };
 
 foreach my $opt (keys %$confdesc) {
@@ -1737,4 +1764,19 @@ sub get_backup_volumes {
 return $return_volumes;
 }
 
+sub new_meta_info_string {
+my () = @_;
+
+my $rpcenv = PVE::RPCEnvironment->get();
+
+return PVE::JSONSchema::print_property_string(
+   {
+   'creation-lxc' => join('.', PVE::LXC::get_lxc_version()),
+   ctime => "". int(time()),
+   'creation-user' => $rpcenv->get_user(),
+   },
+   $meta_info_fmt
+);
+}
+
 1;
-- 
2.30.2



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



[pve-devel] [PATCH v2 container 1/2] cleanup: json config: factor out ignored properties into hash

2023-03-01 Thread Leo Nunner
Signed-off-by: Leo Nunner 
---
 src/PVE/LXC/Config.pm | 7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/src/PVE/LXC/Config.pm b/src/PVE/LXC/Config.pm
index af25a96..aca72ae 100644
--- a/src/PVE/LXC/Config.pm
+++ b/src/PVE/LXC/Config.pm
@@ -1182,8 +1182,13 @@ sub check_type {
 sub json_config_properties {
 my ($class, $prop) = @_;
 
+my $skip_json_config_opts = {
+   parent => 1,
+   snaptime => 1,
+};
+
 foreach my $opt (keys %$confdesc) {
-   next if $opt eq 'parent' || $opt eq 'snaptime';
+   next if $skip_json_config_opts->{$opt};
next if $prop->{$opt};
$prop->{$opt} = $confdesc->{$opt};
 }
-- 
2.30.2



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



[pve-devel] [PATCH v2 container qemu-server] feature #3937: config: store user in meta property

2023-03-01 Thread Leo Nunner
Changes from v1:
- rename property from "cuser" to "creation-user"
- introduce the meta property for containers too

container:

Leo Nunner (2):
  cleanup: json config: factor out ignored properties into hash
  feature #3937: config: introduce meta property

 src/PVE/API2/LXC.pm   |  2 ++
 src/PVE/LXC/Config.pm | 49 ++-
 2 files changed, 50 insertions(+), 1 deletion(-)

qemu-server:

Leo Nunner (1):
  feature #3937: config: store user in meta property

 PVE/QemuServer.pm | 8 
 1 file changed, 8 insertions(+)

-- 
2.30.2



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



[pve-devel] [PATCH manager 1/2] ui: bulk start/stop: align capability checks with backend

2023-03-01 Thread Fiona Ebner
The backend requires VM.PowerMgmt, not Sys.PowerMgmt for bulk start
and bulk stop.

Signed-off-by: Fiona Ebner 
---
 www/manager6/node/CmdMenu.js | 4 +++-
 www/manager6/node/Config.js  | 6 +++---
 2 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/www/manager6/node/CmdMenu.js b/www/manager6/node/CmdMenu.js
index 71548e9c..dc56ef08 100644
--- a/www/manager6/node/CmdMenu.js
+++ b/www/manager6/node/CmdMenu.js
@@ -126,9 +126,11 @@ Ext.define('PVE.node.CmdMenu', {
if (!caps.vms['VM.Migrate']) {
me.getComponent('bulkmigrate').setDisabled(true);
}
-   if (!caps.nodes['Sys.PowerMgmt']) {
+   if (!caps.vms['VM.PowerMgmt']) {
me.getComponent('bulkstart').setDisabled(true);
me.getComponent('bulkstop').setDisabled(true);
+   }
+   if (!caps.nodes['Sys.PowerMgmt']) {
me.getComponent('wakeonlan').setDisabled(true);
}
if (!caps.nodes['Sys.Console']) {
diff --git a/www/manager6/node/Config.js b/www/manager6/node/Config.js
index 9269e892..0cc23fb4 100644
--- a/www/manager6/node/Config.js
+++ b/www/manager6/node/Config.js
@@ -34,13 +34,13 @@ Ext.define('PVE.node.Config', {
var actionBtn = Ext.create('Ext.Button', {
text: gettext('Bulk Actions'),
iconCls: 'fa fa-fw fa-ellipsis-v',
-   disabled: !caps.nodes['Sys.PowerMgmt'] && !caps.vms['VM.Migrate'],
+   disabled: !caps.vms['VM.PowerMgmt'] && !caps.vms['VM.Migrate'],
menu: new Ext.menu.Menu({
items: [
{
text: gettext('Bulk Start'),
iconCls: 'fa fa-fw fa-play',
-   disabled: !caps.nodes['Sys.PowerMgmt'],
+   disabled: !caps.vms['VM.PowerMgmt'],
handler: function() {
Ext.create('PVE.window.BulkAction', {
autoShow: true,
@@ -54,7 +54,7 @@ Ext.define('PVE.node.Config', {
{
text: gettext('Bulk Shutdown'),
iconCls: 'fa fa-fw fa-stop',
-   disabled: !caps.nodes['Sys.PowerMgmt'],
+   disabled: !caps.vms['VM.PowerMgmt'],
handler: function() {
Ext.create('PVE.window.BulkAction', {
autoShow: true,
-- 
2.30.2



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



[pve-devel] [PATCH-SERIES manager] improve bulk action permissions

2023-03-01 Thread Fiona Ebner
In the UI, fix the checks to use the same permission as the backend,
i.e. VM.PowerMgmt rather than Sys.PowerMgmt.

In the backend, also allow the bulk action when the user has the
appropriate permission for each guest in the passed-in list.


Fiona Ebner (2):
  ui: bulk start/stop: align capability checks with backend
  api: node: bulk actions: allow when user has permission for each guest

 PVE/API2/Nodes.pm| 39 +---
 www/manager6/node/CmdMenu.js |  4 +++-
 www/manager6/node/Config.js  |  6 +++---
 3 files changed, 42 insertions(+), 7 deletions(-)

-- 
2.30.2



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



[pve-devel] [PATCH manager 2/2] api: node: bulk actions: allow when user has permission for each guest

2023-03-01 Thread Fiona Ebner
Users with permissions for some guests can already start a task for
each sequentially.

Signed-off-by: Fiona Ebner 
---
 PVE/API2/Nodes.pm | 39 ---
 1 file changed, 36 insertions(+), 3 deletions(-)

diff --git a/PVE/API2/Nodes.pm b/PVE/API2/Nodes.pm
index 47c2d741..c9bf2831 100644
--- a/PVE/API2/Nodes.pm
+++ b/PVE/API2/Nodes.pm
@@ -1756,7 +1756,9 @@ __PACKAGE__->register_method ({
 method => 'POST',
 protected => 1,
 permissions => {
-   check => ['perm', '/', [ 'VM.PowerMgmt' ]],
+   description => "The 'VM.PowerMgmt' permission is required on '/' or on 
'/vms/' for "
+   ."each ID passed via the 'vms' parameter.",
+   user => 'all',
 },
 proxyto => 'node',
 description => "Start all VMs and containers located on this node (by 
default only those with onboot=1).",
@@ -1786,6 +1788,15 @@ __PACKAGE__->register_method ({
my $rpcenv = PVE::RPCEnvironment::get();
my $authuser = $rpcenv->get_user();
 
+   if (!$rpcenv->check($authuser, "/", [ 'VM.PowerMgmt' ], 1)) {
+   my @vms = PVE::Tools::split_list($param->{vms});
+   if (scalar(@vms) > 0) {
+   $rpcenv->check($authuser, "/vms/$_", [ 'VM.PowerMgmt' ]) for 
@vms;
+   } else {
+   raise_perm_exc("/, VM.PowerMgmt");
+   }
+   }
+
my $nodename = $param->{node};
$nodename = PVE::INotify::nodename() if $nodename eq 'localhost';
 
@@ -1891,7 +1902,9 @@ __PACKAGE__->register_method ({
 method => 'POST',
 protected => 1,
 permissions => {
-   check => ['perm', '/', [ 'VM.PowerMgmt' ]],
+   description => "The 'VM.PowerMgmt' permission is required on '/' or on 
'/vms/' for "
+   ."each ID passed via the 'vms' parameter.",
+   user => 'all',
 },
 proxyto => 'node',
 description => "Stop all VMs and Containers.",
@@ -1930,6 +1943,15 @@ __PACKAGE__->register_method ({
my $rpcenv = PVE::RPCEnvironment::get();
my $authuser = $rpcenv->get_user();
 
+   if (!$rpcenv->check($authuser, "/", [ 'VM.PowerMgmt' ], 1)) {
+   my @vms = PVE::Tools::split_list($param->{vms});
+   if (scalar(@vms) > 0) {
+   $rpcenv->check($authuser, "/vms/$_", [ 'VM.PowerMgmt' ]) for 
@vms;
+   } else {
+   raise_perm_exc("/, VM.PowerMgmt");
+   }
+   }
+
my $nodename = $param->{node};
$nodename = PVE::INotify::nodename() if $nodename eq 'localhost';
 
@@ -2056,7 +2078,9 @@ __PACKAGE__->register_method ({
 proxyto => 'node',
 protected => 1,
 permissions => {
-   check => ['perm', '/', [ 'VM.Migrate' ]],
+   description => "The 'VM.Migrate' permission is required on '/' or on 
'/vms/' for each "
+   ."ID passed via the 'vms' parameter.",
+   user => 'all',
 },
 description => "Migrate all VMs and Containers.",
 parameters => {
@@ -2092,6 +2116,15 @@ __PACKAGE__->register_method ({
my $rpcenv = PVE::RPCEnvironment::get();
my $authuser = $rpcenv->get_user();
 
+   if (!$rpcenv->check($authuser, "/", [ 'VM.Migrate' ], 1)) {
+   my @vms = PVE::Tools::split_list($param->{vms});
+   if (scalar(@vms) > 0) {
+   $rpcenv->check($authuser, "/vms/$_", [ 'VM.Migrate' ]) for @vms;
+   } else {
+   raise_perm_exc("/, VM.Migrate");
+   }
+   }
+
my $nodename = $param->{node};
$nodename = PVE::INotify::nodename() if $nodename eq 'localhost';
 
-- 
2.30.2



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