Re: [pve-devel] [PATCH manager] certs: early renew long-lived certificates

2020-04-23 Thread Thomas Lamprecht
On 4/23/20 1:59 PM, Fabian Grünbichler wrote:
> On April 23, 2020 1:07 pm, Dominik Csapak wrote:
>> LGTM
>>
>> maybe we should shorten the lifespan to 1 year already?
>> according to [0], safari on macos will reject certs
>> that are longer valid than 398 days, when issued on/after
>> 2020-09-01
>>
>> 0: https://support.apple.com/en-us/HT211025
>>
> 
> forgot to include this tidbit: that change was actually the reason for 
> looking at it, but it only affects certificates issued by CAs shipped in 
> the Apple Trust Stores, not those issued by CAs manually trusted by a 
> user. so our self-signed CA and its certificates are not affected (for 
> now).

This all makes me thinking... Wouldn't we need to have the PMG also adapt
to this? Checked a very recently from (new test) ISO installed test VM gets
me a 10 year certificate lifespan.. I mean, there more may use a "trusted"
one, but still..



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


Re: [pve-devel] [PATCH manager 5/6] ui: CPUModelSelector: use API call for store

2020-04-23 Thread Dominik Csapak

comments inline

On 4/22/20 3:39 PM, Stefan Reiter wrote:

CPU models are retrieved from the new /nodes/X/cpu call and ordered by
vendor to approximate the previous sort order (less change for accustomed
users).

With this, custom CPU models are now selectable via the GUI.

Signed-off-by: Stefan Reiter 
---
  www/manager6/form/CPUModelSelector.js | 221 ++
  1 file changed, 81 insertions(+), 140 deletions(-)

diff --git a/www/manager6/form/CPUModelSelector.js 
b/www/manager6/form/CPUModelSelector.js
index 1d28ee88..cdb7ddcb 100644
--- a/www/manager6/form/CPUModelSelector.js
+++ b/www/manager6/form/CPUModelSelector.js
@@ -1,9 +1,19 @@
+Ext.define('PVE.data.CPUModel', {
+extend: 'Ext.data.Model',
+fields: [
+   {name: 'name'},
+   {name: 'vendor'},
+   {name: 'custom'},
+   {name: 'displayname'}
+]
+});
+
  Ext.define('PVE.form.CPUModelSelector', {
  extend: 'Proxmox.form.ComboGrid',
  alias: ['widget.CPUModelSelector'],
  
-valueField: 'value',

-displayField: 'value',
+valueField: 'name',
+displayField: 'displayname',
  
  emptyText: Proxmox.Utils.defaultText + ' (kvm64)',

  allowBlank: true,
@@ -19,157 +29,88 @@ Ext.define('PVE.form.CPUModelSelector', {
columns: [
{
header: gettext('Model'),
-   dataIndex: 'value',
+   dataIndex: 'displayname',
hideable: false,
sortable: true,
-   flex: 2
+   flex: 3
},
{
header: gettext('Vendor'),
dataIndex: 'vendor',
hideable: false,
sortable: true,
-   flex: 1
+   flex: 2
}
],
-   width: 320
+   width: 360
  },
  
  store: {

-   fields: [ 'value', 'vendor' ],
-   data: [
-   {
-   value: 'athlon',
-   vendor: 'AMD'
-   },
-   {
-   value: 'phenom',
-   vendor: 'AMD'
-   },
-   {
-   value: 'Opteron_G1',
-   vendor: 'AMD'
-   },
-   {
-   value: 'Opteron_G2',
-   vendor: 'AMD'
-   },
-   {
-   value: 'Opteron_G3',
-   vendor: 'AMD'
-   },
-   {
-   value: 'Opteron_G4',
-   vendor: 'AMD'
-   },
-   {
-   value: 'Opteron_G5',
-   vendor: 'AMD'
-   },
-   {
-   value: 'EPYC',
-   vendor: 'AMD'
-   },
-   {
-   value: '486',
-   vendor: 'Intel'
-   },
-   {
-   value: 'core2duo',
-   vendor: 'Intel'
-   },
-   {
-   value: 'coreduo',
-   vendor: 'Intel'
-   },
-   {
-   value: 'pentium',
-   vendor: 'Intel'
-   },
-   {
-   value: 'pentium2',
-   vendor: 'Intel'
-   },
-   {
-   value: 'pentium3',
-   vendor: 'Intel'
-   },
-   {
-   value: 'Conroe',
-   vendor: 'Intel'
-   },
-   {
-   value: 'Penryn',
-   vendor: 'Intel'
-   },
-   {
-   value: 'Nehalem',
-   vendor: 'Intel'
-   },
-   {
-   value: 'Westmere',
-   vendor: 'Intel'
-   },
-   {
-   value: 'SandyBridge',
-   vendor: 'Intel'
-   },
-   {
-   value: 'IvyBridge',
-   vendor: 'Intel'
-   },
-   {
-   value: 'Haswell',
-   vendor: 'Intel'
-   },
-   {
-   value: 'Haswell-noTSX',
-   vendor: 'Intel'
-   },
-   {
-   value: 'Broadwell',
-   vendor: 'Intel'
-   },
-   {
-   value: 'Broadwell-noTSX',
-   vendor: 'Intel'
-   },
-   {
-   value: 'Skylake-Client',
-   vendor: 'Intel'
-   },
-   {
-   value: 'Skylake-Server',
-   vendor: 'Intel'
-   },
-   {
-   value: 'Cascadelake-Server',
-   vendor: 'Intel'
-   },
-   {
-   value: 'KnightsMill',
-   vendor: 'Intel'
-   },
-   {
-   value: 'kvm32',
-   vendor: 'QEMU'
-   },
-   {
-   value: 'kvm64',
-   vendor: 'QEMU'
-   },
-   {
-   value: 'qemu32',
-   vendor: 'QEMU'
-   },
-   {
-   value: 'qemu64',
-   vendor: 'QEMU'
-   },
-   {
-   

Re: [pve-devel] [PATCH manager 4/6] ui: ProcessorEdit: fix total core calculation and use view model

2020-04-23 Thread Dominik Csapak

nice that you cleaned up the panel :) (LGTM)

but the actual fix would be much easier ;)

since the 'total cores' is just informational
we can set
'isFormField: false'
on the totalcore field
which then will not be reset (and not tracked as a formfield)
if you do this you can drop the whole setValues function

On 4/22/20 3:39 PM, Stefan Reiter wrote:

Clean up the code in ProcessorEdit with a view model and fix a bug while at
it - previously, pressing the 'Reset' button on the form would always set
the value of the total core count field to 1.

Signed-off-by: Stefan Reiter 
---

The fix is technically only the setValues part, but I started off thinking that
using a view model would also fix it - it did not, but I still think it looks
nicer than before.

  www/manager6/qemu/ProcessorEdit.js | 65 ++
  1 file changed, 39 insertions(+), 26 deletions(-)

diff --git a/www/manager6/qemu/ProcessorEdit.js 
b/www/manager6/qemu/ProcessorEdit.js
index bc17e152..d555b2d8 100644
--- a/www/manager6/qemu/ProcessorEdit.js
+++ b/www/manager6/qemu/ProcessorEdit.js
@@ -5,28 +5,18 @@ Ext.define('PVE.qemu.ProcessorInputPanel', {
  
  insideWizard: false,
  
-controller: {

-   xclass: 'Ext.app.ViewController',
-
-   updateCores: function() {
-   var me = this.getView();
-   var sockets = me.down('field[name=sockets]').getValue();
-   var cores = me.down('field[name=cores]').getValue();
-   me.down('field[name=totalcores]').setValue(sockets*cores);
-   var vcpus = me.down('field[name=vcpus]');
-   vcpus.setMaxValue(sockets*cores);
-   vcpus.setEmptyText(sockets*cores);
-   vcpus.validate();
+viewModel: {
+   data: {
+   socketCount: 1,
+   coreCount: 1,
},
+   formulas: {
+   totalCoreCount: get => get('socketCount') * get('coreCount'),
+   },
+},
  
-	control: {

-   'field[name=sockets]': {
-   change: 'updateCores'
-   },
-   'field[name=cores]': {
-   change: 'updateCores'
-   }
-   }
+controller: {
+   xclass: 'Ext.app.ViewController',
  },
  
  onGetValues: function(values) {

@@ -76,6 +66,16 @@ Ext.define('PVE.qemu.ProcessorInputPanel', {
return values;
  },
  
+setValues: function(values) {

+   let me = this;
+
+   // set the original value here, else 'reset' clears the field
+   let totalCoreDisplay = me.lookupReference('totalcores');
+   totalCoreDisplay.originalValue = values.cores * values.sockets;
+
+   me.callParent([values]);
+},
+
  cpu: {},
  
  column1: [

@@ -86,7 +86,10 @@ Ext.define('PVE.qemu.ProcessorInputPanel', {
maxValue: 4,
value: '1',
fieldLabel: gettext('Sockets'),
-   allowBlank: false
+   allowBlank: false,
+   bind: {
+   value: '{socketCount}',
+   },
},
{
xtype: 'proxmoxintegerfield',
@@ -95,8 +98,11 @@ Ext.define('PVE.qemu.ProcessorInputPanel', {
maxValue: 128,
value: '1',
fieldLabel: gettext('Cores'),
-   allowBlank: false
-   }
+   allowBlank: false,
+   bind: {
+   value: '{coreCount}',
+   },
+   },
  ],
  
  column2: [

@@ -109,8 +115,11 @@ Ext.define('PVE.qemu.ProcessorInputPanel', {
xtype: 'displayfield',
fieldLabel: gettext('Total cores'),
name: 'totalcores',
-   value: '1'
-   }
+   reference: 'totalcores',
+   bind: {
+   value: '{totalCoreCount}',
+   },
+   },
  ],
  
  advancedColumn1: [

@@ -123,7 +132,11 @@ Ext.define('PVE.qemu.ProcessorInputPanel', {
fieldLabel: gettext('VCPUs'),
deleteEmpty: true,
allowBlank: true,
-   emptyText: '1'
+   emptyText: '1',
+   bind: {
+   emptyText: '{totalCoreCount}',
+   maxValue: '{totalCoreCount}',
+   },
},
{
xtype: 'numberfield',



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


[pve-devel] [PATCH proxmox-i18n] Update Arabic translation

2020-04-23 Thread Moayad Almalat
Signed-off-by: Moayad Almalat 
---
 ar.po | 177 +-
 1 file changed, 76 insertions(+), 101 deletions(-)

diff --git a/ar.po b/ar.po
index eb09a68..cec3aa4 100644
--- a/ar.po
+++ b/ar.po
@@ -11,14 +11,14 @@ msgstr ""
 "Project-Id-Version: pve-manager 31813246103b2582162f422dc34d8077eaee1e01\n"
 "Report-Msgid-Bugs-To: \n"
 "POT-Creation-Date: Wed Apr 22 15:07:47 2020\n"
-"PO-Revision-Date: YEAR-MO-DA HO:MI +ZONE\n"
-"Last-Translator: FULL NAME \n"
+"PO-Revision-Date: 2020-04-23 14:43+0200\n"
 "Language-Team: LANGUAGE \n"
 "Language: ar\n"
 "MIME-Version: 1.0\n"
 "Content-Type: text/plain; charset=UTF-8\n"
 "Content-Transfer-Encoding: 8bit\n"
 "X-Generator: Poedit 2.2.1\n"
+"Last-Translator: \n"
 
 #: pve-manager/www/manager6/lxc/MPEdit.js:236
 msgid "/some/path"
@@ -138,15 +138,13 @@ msgstr "إضافة CephFS جديدة إلى تكوين تخزين الكتلة."
 
 #: pve-manager/www/manager6/ceph/Pool.js:58
 msgid "Add the new pool to the cluster storage configuration."
-msgstr "Add the new pool to the cluster storage configuration."
+msgstr "إضافة المجموعة الجديدة إلى تكوين تخزين المجموعة."
 
 #: pve-manager/www/manager6/ceph/CephInstallWizard.js:224
 msgid ""
 "Additional monitors are recommended. They can be created at any time in the "
 "Monitor tab."
-msgstr ""
-"Additional monitors are recommended. They can be created at any time in the "
-"Monitor tab."
+msgstr "يوصى بوجود شاشات إضافية. يمكن إنشائها في أي وقت في تبويب الشاشة."
 
 #: pmg-gui/js/UserBlackWhiteList.js:34 pmg-gui/js/UserBlackWhiteList.js:194
 #: pve-manager/www/manager6/ceph/ServiceList.js:342
@@ -611,9 +609,8 @@ msgid "Capacity"
 msgstr "القدرات"
 
 #: pve-manager/www/manager6/ceph/Status.js:57
-#, fuzzy
 msgid "Ceph Version"
-msgstr "الإصدار"
+msgstr "الإصدار الـ Ceph"
 
 #: pve-manager/www/manager6/ceph/CephInstallWizard.js:183
 msgid "Ceph cluster configuration"
@@ -907,7 +904,7 @@ msgstr "وحدة التحكم"
 
 #: pve-manager/www/manager6/dc/OptionView.js:85
 msgid "Console Viewer"
-msgstr "عارض لوحة المفاتيح"
+msgstr "Console Viewer"
 
 #: pve-manager/www/manager6/lxc/Options.js:96
 #: pve-manager/www/manager6/lxc/Options.js:100
@@ -960,14 +957,12 @@ msgstr "تصفية نوع المحتوى"
 
 #: pve-manager/www/manager6/form/SDNControllerSelector.js:26
 #: pve-manager/www/manager6/sdn/zones/EvpnEdit.js:38
-#, fuzzy
 msgid "Controller"
-msgstr "وحدة تحكم SCSI"
+msgstr "المراقب"
 
 #: pve-manager/www/manager6/dc/Config.js:151
-#, fuzzy
 msgid "Controllers"
-msgstr "وحدة تحكم SCSI"
+msgstr "وحدات التحكم"
 
 #: proxmox-widget-toolkit/Utils.js:476 proxmox-widget-toolkit/Utils.js:494
 #: pve-manager/www/manager6/lxc/CmdMenu.js:157
@@ -1057,9 +1052,8 @@ msgid "Create Cluster"
 msgstr "إنشاء مجموعة"
 
 #: pve-manager/www/manager6/lxc/FeaturesEdit.js:67
-#, fuzzy
 msgid "Create Device Nodes"
-msgstr "أجهزة بوساطة"
+msgstr "إنشاء عقد الأجهزة"
 
 #: pve-manager/www/manager6/Workspace.js:262
 #: pve-manager/www/manager6/node/CmdMenu.js:9
@@ -1068,16 +1062,16 @@ msgstr "إنشاء VM"
 
 #: pve-manager/www/manager6/node/ACME.js:178
 msgid "Created"
-msgstr "خلقت"
+msgstr "تم الإنشاء"
 
 #: pmg-gui/js/SpamQuarantineOptions.js:18
 msgid "Custom"
-msgstr "المخصصة"
+msgstr "مخصص"
 
 #: pve-manager/www/manager6/dc/TFAEdit.js:167
 msgid ""
 "Custom 2nd factor configuration is not supported on realms with '{0}' TFA."
-msgstr "تكوين العامل الثاني المخصص غير مدعوم في النطاقات باستخدام '{0}' TFA."
+msgstr "لا يتم دعم تكوين المعامل الثاني المخصص في ملفات التخصيص مع '{0}' TFA."
 
 #: pmg-gui/js/SpamDetectorCustom.js:112 pmg-gui/js/SpamDetectorCustom.js:157
 msgid "Custom Rule Score"
@@ -1085,7 +1079,7 @@ msgstr "نقاط القاعدة المخصصة"
 
 #: pmg-gui/js/SpamDetectorConfiguration.js:28
 msgid "Custom Scores"
-msgstr "درجات مخصصة"
+msgstr "علامات مخصصة"
 
 #: pve-manager/www/manager6/ceph/OSD.js:51
 msgid "DB Disk"
@@ -1319,7 +1313,7 @@ msgstr "تدمير \"{0}\""
 
 #: proxmox-widget-toolkit/Utils.js:526
 msgid "Destroy image from unknown guest"
-msgstr ""
+msgstr "إتلاف الصورة من ضيف مجهول"
 
 #: pve-manager/www/manager6/qemu/HardwareView.js:444
 msgid "Detach"
@@ -1373,9 +1367,8 @@ msgid "Directory Storage"
 msgstr "تخزين الدليل"
 
 #: pmg-gui/js/MailProxyRelaying.js:29
-#, fuzzy
 msgid "Disable MX lookup (SMTP)"
-msgstr "تعطيل MX lookup"
+msgstr "تعطيل MX lookup (SMTP)"
 
 #: proxmox-widget-toolkit/Utils.js:43 pve-manager/www/manager6/Utils.js:250
 msgid "Disabled"
@@ -1386,10 +1379,11 @@ msgid ""
 "Disabling the limiter can potentially allow a guest to overload the host. "
 "Proceed with caution."
 msgstr ""
+"من المحتمل أن يسمح تعطيل المحدد للضيف بزيادة التحميل على المضيف. التقدم بحذر."
 
 #: pve-manager/www/manager6/qemu/HDEdit.js:230
 msgid "Discard"
-msgstr "نبذ"
+msgstr "تجاهل"
 
 #: pmg-gui/js/PostfixQShape.js:110
 msgid "Discard address verification database"
@@ -1813,7 +1807,7 @@ msgstr "مجموعات وحدات التخزين الموجودة"
 
 #: pve-manager/www/manager6/lxc/FeaturesEdit.js:68
 msgid "Experimental"
-msgstr ""
+msgstr "تجريبي"
 
 #: 

Re: [pve-devel] [PATCH guest-common 2/2] fix 2682: make sure configuration file is up-to-date for lock_config-variants

2020-04-23 Thread Fabian Ebner

On 23.04.20 13:51, Fabian Ebner wrote:

See [0] for the details. The call tree for the variants is
lock_config -> lock_config_full -> lock_config_mode
so it is sufficient to adapt lock_config_mode.

[0]: https://bugzilla.proxmox.com/show_bug.cgi?id=2682

Suggested-by: Fabian Grünbichler 
Signed-off-by: Fabian Ebner 
---
  PVE/AbstractConfig.pm | 8 +++-
  1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/PVE/AbstractConfig.pm b/PVE/AbstractConfig.pm
index f1b395c..15368de 100644
--- a/PVE/AbstractConfig.pm
+++ b/PVE/AbstractConfig.pm
@@ -292,7 +292,13 @@ sub lock_config_mode {
  
  my $filename = $class->config_file_lock($vmid);
  
-my $res = lock_file_full($filename, $timeout, $shared, $code, @param);

+# make sure configuration file is up-to-date
+my $realcode = sub {
+   PVE::Cluster::cfs_update();
+   $code->(@param);
+};
+
+my $res = lock_file_full($filename, $timeout, $shared, $realcode, @param);



@param can/should be dropped from the lock_file_full call, as it's 
unused now.



  die $@ if $@;
  



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


Re: [pve-devel] [PATCH manager] certs: early renew long-lived certificates

2020-04-23 Thread Fabian Grünbichler
On April 23, 2020 1:07 pm, Dominik Csapak wrote:
> LGTM
> 
> maybe we should shorten the lifespan to 1 year already?
> according to [0], safari on macos will reject certs
> that are longer valid than 398 days, when issued on/after
> 2020-09-01
> 
> 0: https://support.apple.com/en-us/HT211025
> 

forgot to include this tidbit: that change was actually the reason for 
looking at it, but it only affects certificates issued by CAs shipped in 
the Apple Trust Stores, not those issued by CAs manually trusted by a 
user. so our self-signed CA and its certificates are not affected (for 
now).

I don't have any objections to shortening both the issuance and the 
check here to 1 year though.

> On 4/23/20 12:20 PM, Fabian Grünbichler wrote:
>> if our self-signed certificate expires in more than 825 days, but was
>> created after July 2019 it won't be accepted by modern Apple devices. we
>> fixed the issuance to generate shorter-lived certificates in November
>> 2019, this cleans up the existing ones to fix this and similar future
>> issues.
>> 
>> two years / 730 days as cut-off was chosen since it's our new maximum
>> self-signed certificate lifetime, and should thus catch all old-style
>> certificates.
>> 
>> another positive side-effect is that we can now phase out support for
>> older certificates faster, e.g. if we want to move to bigger keys,
>> different signature algorithms, or anything else in that direction.
>> 
>> Signed-off-by: Fabian Grünbichler 
>> ---
>> I'd also be fine with reducing both even more, e.g. to 1 year ;)
>> 
>>   bin/pveupdate | 15 ---
>>   1 file changed, 12 insertions(+), 3 deletions(-)
>> 
>> diff --git a/bin/pveupdate b/bin/pveupdate
>> index 15a2accc..36ac6814 100755
>> --- a/bin/pveupdate
>> +++ b/bin/pveupdate
>> @@ -79,8 +79,9 @@ eval {
>>   my $certpath = 
>> PVE::CertHelpers::default_cert_path_prefix($nodename).".pem";
>>   my $capath = "/etc/pve/pve-root-ca.pem";
>>   
>> -# check if expiry is < 2W
>> -if (PVE::Certificate::check_expiry($certpath, time() + 14*24*60*60)) {
>> +my $renew = sub {
>> +my ($msg) = @_;
>> +
>>  # get CA info
>>  my $cainfo = PVE::Certificate::get_certificate_info($capath);
>>   
>> @@ -94,13 +95,21 @@ eval {
>>  # TODO: replace by low level ssleay interface if version 1.86 is 
>> available
>>  PVE::Tools::run_command(['/usr/bin/openssl', 'verify', '-CAfile', 
>> $capath, $certpath]);
>>   
>> -print "PVE certificate expires soon, renewing...\n";
>> +print "PVE certificate $msg\n";
>>  # create new certificate
>>  my $ip = PVE::Cluster::remote_node_ip($nodename);
>>  PVE::Cluster::Setup::gen_pve_ssl_cert(1, $nodename, $ip);
>>   
>>  print "Restarting pveproxy after renewing certificate\n";
>>  PVE::Tools::run_command(['systemctl', 'reload-or-restart', 'pveproxy']);
>> +};
>> +
>> +if (PVE::Certificate::check_expiry($certpath, time() + 14*24*60*60)) {
>> +# expires in next 2 weeks
>> +$renew->("expires soon, renewing...");
>> +} elsif (!PVE::Certificate::check_expiry($certpath, time() + 
>> 2*365*24*60*60)) {
>> +# expires in more than 2 years
>> +$renew->("expires in more than 2 years, renewing to reduce certificate 
>> life-span...");
>>   }
>>   };
>>   syslog ('err', "Checking/Renewing SSL certificate failed: $@") if $@;
>> 
> 
> ___
> pve-devel mailing list
> pve-devel@pve.proxmox.com
> https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
> 

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


[pve-devel] [PATCH guest-common 1/2] Avoid duplication by using lock_config_mode

2020-04-23 Thread Fabian Ebner
No functional change is intended.

Signed-off-by: Fabian Ebner 
---
 PVE/AbstractConfig.pm | 8 +---
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/PVE/AbstractConfig.pm b/PVE/AbstractConfig.pm
index beb10c7..f1b395c 100644
--- a/PVE/AbstractConfig.pm
+++ b/PVE/AbstractConfig.pm
@@ -259,13 +259,7 @@ sub load_current_config {
 sub lock_config_full {
 my ($class, $vmid, $timeout, $code, @param) = @_;
 
-my $filename = $class->config_file_lock($vmid);
-
-my $res = lock_file($filename, $timeout, $code, @param);
-
-die $@ if $@;
-
-return $res;
+return $class->lock_config_mode($vmid, $timeout, 0, $code, @param);
 }
 
 sub create_and_lock_config {
-- 
2.20.1


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


[pve-devel] [PATCH guest-common 2/2] fix 2682: make sure configuration file is up-to-date for lock_config-variants

2020-04-23 Thread Fabian Ebner
See [0] for the details. The call tree for the variants is
lock_config -> lock_config_full -> lock_config_mode
so it is sufficient to adapt lock_config_mode.

[0]: https://bugzilla.proxmox.com/show_bug.cgi?id=2682

Suggested-by: Fabian Grünbichler  
Signed-off-by: Fabian Ebner 
---
 PVE/AbstractConfig.pm | 8 +++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/PVE/AbstractConfig.pm b/PVE/AbstractConfig.pm
index f1b395c..15368de 100644
--- a/PVE/AbstractConfig.pm
+++ b/PVE/AbstractConfig.pm
@@ -292,7 +292,13 @@ sub lock_config_mode {
 
 my $filename = $class->config_file_lock($vmid);
 
-my $res = lock_file_full($filename, $timeout, $shared, $code, @param);
+# make sure configuration file is up-to-date
+my $realcode = sub {
+   PVE::Cluster::cfs_update();
+   $code->(@param);
+};
+
+my $res = lock_file_full($filename, $timeout, $shared, $realcode, @param);
 
 die $@ if $@;
 
-- 
2.20.1


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


Re: [pve-devel] [PATCH qemu-server 1/4] tests: print more info when ovmf parsing fails

2020-04-23 Thread Dominik Csapak

the subjects for this and the 2/4 patch are  wrong, replace ovmf with ovf^^

(damn acronyms)

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


Re: [pve-devel] [PATCH manager] certs: early renew long-lived certificates

2020-04-23 Thread Dominik Csapak

LGTM

maybe we should shorten the lifespan to 1 year already?
according to [0], safari on macos will reject certs
that are longer valid than 398 days, when issued on/after
2020-09-01

0: https://support.apple.com/en-us/HT211025

On 4/23/20 12:20 PM, Fabian Grünbichler wrote:

if our self-signed certificate expires in more than 825 days, but was
created after July 2019 it won't be accepted by modern Apple devices. we
fixed the issuance to generate shorter-lived certificates in November
2019, this cleans up the existing ones to fix this and similar future
issues.

two years / 730 days as cut-off was chosen since it's our new maximum
self-signed certificate lifetime, and should thus catch all old-style
certificates.

another positive side-effect is that we can now phase out support for
older certificates faster, e.g. if we want to move to bigger keys,
different signature algorithms, or anything else in that direction.

Signed-off-by: Fabian Grünbichler 
---
I'd also be fine with reducing both even more, e.g. to 1 year ;)

  bin/pveupdate | 15 ---
  1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/bin/pveupdate b/bin/pveupdate
index 15a2accc..36ac6814 100755
--- a/bin/pveupdate
+++ b/bin/pveupdate
@@ -79,8 +79,9 @@ eval {
  my $certpath = 
PVE::CertHelpers::default_cert_path_prefix($nodename).".pem";
  my $capath = "/etc/pve/pve-root-ca.pem";
  
-# check if expiry is < 2W

-if (PVE::Certificate::check_expiry($certpath, time() + 14*24*60*60)) {
+my $renew = sub {
+   my ($msg) = @_;
+
# get CA info
my $cainfo = PVE::Certificate::get_certificate_info($capath);
  
@@ -94,13 +95,21 @@ eval {

# TODO: replace by low level ssleay interface if version 1.86 is 
available
PVE::Tools::run_command(['/usr/bin/openssl', 'verify', '-CAfile', 
$capath, $certpath]);
  
-	print "PVE certificate expires soon, renewing...\n";

+   print "PVE certificate $msg\n";
# create new certificate
my $ip = PVE::Cluster::remote_node_ip($nodename);
PVE::Cluster::Setup::gen_pve_ssl_cert(1, $nodename, $ip);
  
  	print "Restarting pveproxy after renewing certificate\n";

PVE::Tools::run_command(['systemctl', 'reload-or-restart', 'pveproxy']);
+};
+
+if (PVE::Certificate::check_expiry($certpath, time() + 14*24*60*60)) {
+   # expires in next 2 weeks
+   $renew->("expires soon, renewing...");
+} elsif (!PVE::Certificate::check_expiry($certpath, time() + 
2*365*24*60*60)) {
+   # expires in more than 2 years
+   $renew->("expires in more than 2 years, renewing to reduce certificate 
life-span...");
  }
  };
  syslog ('err', "Checking/Renewing SSL certificate failed: $@") if $@;



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


[pve-devel] [PATCH qemu-server 4/4] importovf: die with error when disk file is missing

2020-04-23 Thread Dominik Csapak
also add missing '\n' at the end of error messages

Signed-off-by: Dominik Csapak 
---
 PVE/QemuServer/OVF.pm | 8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/PVE/QemuServer/OVF.pm b/PVE/QemuServer/OVF.pm
index 536e0eb..dbcc361 100644
--- a/PVE/QemuServer/OVF.pm
+++ b/PVE/QemuServer/OVF.pm
@@ -213,12 +213,16 @@ ovf:Item[rasd:InstanceID='%s']/rasd:ResourceType", 
$controller_id);
my $ovf_dir = realpath(dirname(File::Spec->rel2abs($ovf)));
my $backing_file_path = realpath(join ('/', $ovf_dir, $filepath));
if ($backing_file_path !~ /^\Q${ovf_dir}\E/) {
-   die "error parsing $filepath, are you using a symlink ?";
+   die "error parsing $filepath, are you using a symlink ?\n";
+   }
+
+   if (!-e $backing_file_path) {
+   die "error parsing $filepath, file seems not to exist at 
$backing_file_path\n";
}
 
my $virtual_size;
if ( !($virtual_size = 
PVE::Storage::file_size_info($backing_file_path)) ) {
-   die "error parsing $backing_file_path, size seems to be 
$virtual_size";
+   die "error parsing $backing_file_path, size seems to be 
$virtual_size\n";
}
 
$pve_disk = {
-- 
2.20.1


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


[pve-devel] [PATCH qemu-server 3/4] importovf: fix import of ovfs without default namespaces

2020-04-23 Thread Dominik Csapak
some ovfs to not declare 'rasd' as a default namespace
(in the top level Envelope element), but inline in each element
(e.g. ...)

this trips up our relative findvalue with

XPath error : Undefined namespace prefix

to avoid this, search in the global XPathContext (where we register
those namespaces ourselves) and give the item_node as context

this works in both cases

Signed-off-by: Dominik Csapak 
---
 PVE/QemuServer/OVF.pm | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/PVE/QemuServer/OVF.pm b/PVE/QemuServer/OVF.pm
index 7ee4dc8..536e0eb 100644
--- a/PVE/QemuServer/OVF.pm
+++ b/PVE/QemuServer/OVF.pm
@@ -155,7 +155,7 @@ sub parse_ovf {
 
# from Item, find corresponding Disk node
# here the dot means the search should start from the current element 
in dom
-   my $host_resource = $item_node->findvalue('./rasd:HostResource');
+   my $host_resource = $xpc->findvalue('rasd:HostResource', $item_node);
my $disk_section_path;
my $disk_id;
 
@@ -194,7 +194,7 @@ ovf:Disk[\@ovf:diskId='%s']/\@ovf:fileRef", $disk_id);
print "file path: $filepath\n" if $debug;
 
# from Item, find owning Controller type
-   my $controller_id = $item_node->findvalue('./rasd:Parent');
+   my $controller_id = $xpc->findvalue('rasd:Parent', $item_node);
my $xpath_find_parent_type = 
sprintf("/ovf:Envelope/ovf:VirtualSystem/ovf:VirtualHardwareSection/\
 ovf:Item[rasd:InstanceID='%s']/rasd:ResourceType", $controller_id);
my $controller_type = $xpc->findvalue($xpath_find_parent_type);
@@ -205,7 +205,7 @@ ovf:Item[rasd:InstanceID='%s']/rasd:ResourceType", 
$controller_id);
print "owning controller type: $controller_type\n" if $debug;
 
# extract corresponding Controller node details
-   my $adress_on_controller = 
$item_node->findvalue('./rasd:AddressOnParent');
+   my $adress_on_controller = $xpc->findvalue('rasd:AddressOnParent', 
$item_node);
my $pve_disk_address = id_to_pve($controller_type) . 
$adress_on_controller;
 
# resolve symlinks and relative path components
-- 
2.20.1


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


[pve-devel] [PATCH qemu-server 1/4] tests: print more info when ovmf parsing fails

2020-04-23 Thread Dominik Csapak
when one of the ovmf tests fails to parse at all,
we just get the 'die' message of the failing component, but not which
file actually failed to parse

to get better output, convert the parsing also to a test
and ok() and fail() respectively and then printing the error

Signed-off-by: Dominik Csapak 
---
 test/run_ovf_tests.pl | 18 --
 1 file changed, 16 insertions(+), 2 deletions(-)

diff --git a/test/run_ovf_tests.pl b/test/run_ovf_tests.pl
index b9e4825..74c53bc 100755
--- a/test/run_ovf_tests.pl
+++ b/test/run_ovf_tests.pl
@@ -12,8 +12,22 @@ use Data::Dumper;
 
 my $test_manifests = join ('/', $Bin, 'ovf_manifests');
 
-my $win2008 = 
PVE::QemuServer::OVF::parse_ovf("$test_manifests/Win_2008_R2_two-disks.ovf");
-my $win10 = PVE::QemuServer::OVF::parse_ovf("$test_manifests/Win10-Liz.ovf");
+print "parsing ovfs\n";
+
+my $win2008 = eval { 
PVE::QemuServer::OVF::parse_ovf("$test_manifests/Win_2008_R2_two-disks.ovf") };
+if (my $err = $@) {
+fail('parse win2008');
+warn("error: $err\n");
+} else {
+ok('parse win2008');
+}
+my $win10 = eval { 
PVE::QemuServer::OVF::parse_ovf("$test_manifests/Win10-Liz.ovf") };
+if (my $err = $@) {
+fail('parse win10');
+warn("error: $err\n");
+} else {
+ok('parse win10');
+}
 
 print "testing disks\n";
 
-- 
2.20.1


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


[pve-devel] [PATCH qemu-server 2/4] tests: add test for ovmf with missing default rasd namespace

2020-04-23 Thread Dominik Csapak
sometimes vendors do not put the 'rasd' namespaces in the
top level Envelope, but in every 'rasd' element
this adds a test for this

Signed-off-by: Dominik Csapak 
---
this test will fail when applied, but will be corrected by the next
patch in this series (3/4)

for a real world example of this see:
https://forum.proxmox.com/threads/qm-importovf-xpath-error-on-importing-ovf.68597/

 .../ovf_manifests/Win10-Liz_no_default_ns.ovf | 142 ++
 test/run_ovf_tests.pl |  15 ++
 2 files changed, 157 insertions(+)
 create mode 100755 test/ovf_manifests/Win10-Liz_no_default_ns.ovf

diff --git a/test/ovf_manifests/Win10-Liz_no_default_ns.ovf 
b/test/ovf_manifests/Win10-Liz_no_default_ns.ovf
new file mode 100755
index 000..b93540f
--- /dev/null
+++ b/test/ovf_manifests/Win10-Liz_no_default_ns.ovf
@@ -0,0 +1,142 @@
+
+
+http://schemas.dmtf.org/ovf/envelope/1; 
xmlns:cim="http://schemas.dmtf.org/wbem/wscim/1/common; 
xmlns:ovf="http://schemas.dmtf.org/ovf/envelope/1; 
xmlns:vmw="http://www.vmware.com/schema/ovf; 
xmlns:vssd="http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/CIM_VirtualSystemSettingData;
 xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance;>
+  
+
+  
+  
+Virtual disk information
+http://www.vmware.com/interfaces/specifications/vmdk.html#streamOptimized;
 ovf:populatedSize="16798056448"/>
+  
+  
+The list of logical networks
+
+  The bridged network
+
+  
+  
+A virtual machine
+Win10-Liz
+
+  The kind of installed guest operating system
+
+
+  Virtual hardware requirements
+  
+Virtual Hardware Family
+0
+Win10-Liz
+vmx-11
+  
+  
+http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/CIM_ResourceAllocationSettingData;>hertz
 * 10^6
+http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/CIM_ResourceAllocationSettingData;>Number
 of Virtual CPUs
+http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/CIM_ResourceAllocationSettingData;>4
 virtual CPU(s)
+http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/CIM_ResourceAllocationSettingData;>1
+http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/CIM_ResourceAllocationSettingData;>3
+http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/CIM_ResourceAllocationSettingData;>4
+  
+  
+http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/CIM_ResourceAllocationSettingData;>byte
 * 2^20
+http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/CIM_ResourceAllocationSettingData;>Memory
 Size
+http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/CIM_ResourceAllocationSettingData;>6144MB
 of memory
+http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/CIM_ResourceAllocationSettingData;>2
+http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/CIM_ResourceAllocationSettingData;>4
+http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/CIM_ResourceAllocationSettingData;>6144
+  
+  
+http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/CIM_ResourceAllocationSettingData;>0
+http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/CIM_ResourceAllocationSettingData;>SATA
 Controller
+http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/CIM_ResourceAllocationSettingData;>sataController0
+http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/CIM_ResourceAllocationSettingData;>3
+http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/CIM_ResourceAllocationSettingData;>vmware.sata.ahci
+http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/CIM_ResourceAllocationSettingData;>20
+  
+  
+http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/CIM_ResourceAllocationSettingData;>0
+http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/CIM_ResourceAllocationSettingData;>USB
 Controller (XHCI)
+http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/CIM_ResourceAllocationSettingData;>usb3
+http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/CIM_ResourceAllocationSettingData;>4
+http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/CIM_ResourceAllocationSettingData;>vmware.usb.xhci
+http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/CIM_ResourceAllocationSettingData;>23
+  
+  
+http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/CIM_ResourceAllocationSettingData;>0
+http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/CIM_ResourceAllocationSettingData;>USB
 Controller (EHCI)
+http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/CIM_ResourceAllocationSettingData;>usb
+http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/CIM_ResourceAllocationSettingData;>5
+http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/CIM_ResourceAllocationSettingData;>vmware.usb.ehci
+http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/CIM_ResourceAllocationSettingData;>23
+
+  
+  
+http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/CIM_ResourceAllocationSettingData;>0
+

Re: [pve-devel] [PATCH storage v4 00/12] Fix: #2124 zstd

2020-04-23 Thread Alwin Antreich
On Thu, Apr 23, 2020 at 12:35:34PM +0200, Dominic Jäger wrote:
> Thank you for merging the test files!
No problem. I hope I didn't miss any. :)

> 
> Love the dropdown to set a compression:
>   GZIP (good)
>   ZSTD (better)
> 
> Tests work and creating and restoring backups in the GUI with the new option, 
> too.
Thanks for testing.

> 
> Tested-by: Dominic Jäger 
> 
> On Wed, Apr 22, 2020 at 04:57:51PM +0200, Alwin Antreich wrote:
> > Zstandard (zstd) [0] is a data compression algorithm, in addition to
> > gzip, lzo for our backup/restore. It can utilize multiple core CPUs. But
> > by default it has one compression and one writer thread.
> > 
> > 
> > [0] https://facebook.github.io/zstd/
> > 
> > Alwin Antreich (12):
> > __pve-storage__
> >   storage: test: split archive format/compressor
> >   storage: replace build-in stat with File::stat
> >   test: parse_volname
> >   test: list_volumes
> >   Fix: backup: ctime was from stat not file name
> >   test: path_to_volume_id
> >   Fix: path_to_volume_id returned wrong content
> >   Fix: add missing snippets subdir
> >   backup: compact regex for backup file filter
> >   Fix: #2124 storage: add zstd support
> >   test: get_subdir
> >   test: filesystem_path
> > 

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


Re: [pve-devel] [PATCH storage v4 00/12] Fix: #2124 zstd

2020-04-23 Thread Dominic Jäger
Thank you for merging the test files!

Love the dropdown to set a compression:
  GZIP (good)
  ZSTD (better)

Tests work and creating and restoring backups in the GUI with the new option, 
too.

Tested-by: Dominic Jäger 

On Wed, Apr 22, 2020 at 04:57:51PM +0200, Alwin Antreich wrote:
> Zstandard (zstd) [0] is a data compression algorithm, in addition to
> gzip, lzo for our backup/restore. It can utilize multiple core CPUs. But
> by default it has one compression and one writer thread.
> 
> 
> [0] https://facebook.github.io/zstd/
> 
> Alwin Antreich (12):
> __pve-storage__
>   storage: test: split archive format/compressor
>   storage: replace build-in stat with File::stat
>   test: parse_volname
>   test: list_volumes
>   Fix: backup: ctime was from stat not file name
>   test: path_to_volume_id
>   Fix: path_to_volume_id returned wrong content
>   Fix: add missing snippets subdir
>   backup: compact regex for backup file filter
>   Fix: #2124 storage: add zstd support
>   test: get_subdir
>   test: filesystem_path
> 

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


Re: [pve-devel] [PATCH storage v4 04/12] test: list_volumes

2020-04-23 Thread Dominic Jäger
I like it. Having for example one $mocked_vmlist is clean.

Two ideas for further testing (in the future):
 - Passing only a subset of $types to list_volumes
 - Unintended situations, like when $mocked_vmlist has a VM without type => 
list_volumes roughly does if (defined($type) && $type eq 'lxc') ... else ...

On Wed, Apr 22, 2020 at 04:58:00PM +0200, Alwin Antreich wrote:
> Test to reduce the potential for accidental breakage on regex changes.
> 
> Co-Authored-by: Dominic Jaeger 
> Signed-off-by: Alwin Antreich 
> ---
>  test/list_volumes_test.pm | 519 ++
>  test/run_plugin_tests.pl  |   6 +-
>  2 files changed, 524 insertions(+), 1 deletion(-)
>  create mode 100644 test/list_volumes_test.pm
> 
> diff --git a/test/list_volumes_test.pm b/test/list_volumes_test.pm
> new file mode 100644
> index 000..c1428f2
> --- /dev/null
> +++ b/test/list_volumes_test.pm
> @@ -0,0 +1,519 @@
> +package PVE::Storage::TestListVolumes;
> +
> +use strict;
> 

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


[pve-devel] [PATCH manager] certs: early renew long-lived certificates

2020-04-23 Thread Fabian Grünbichler
if our self-signed certificate expires in more than 825 days, but was
created after July 2019 it won't be accepted by modern Apple devices. we
fixed the issuance to generate shorter-lived certificates in November
2019, this cleans up the existing ones to fix this and similar future
issues.

two years / 730 days as cut-off was chosen since it's our new maximum
self-signed certificate lifetime, and should thus catch all old-style
certificates.

another positive side-effect is that we can now phase out support for
older certificates faster, e.g. if we want to move to bigger keys,
different signature algorithms, or anything else in that direction.

Signed-off-by: Fabian Grünbichler 
---
I'd also be fine with reducing both even more, e.g. to 1 year ;)

 bin/pveupdate | 15 ---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/bin/pveupdate b/bin/pveupdate
index 15a2accc..36ac6814 100755
--- a/bin/pveupdate
+++ b/bin/pveupdate
@@ -79,8 +79,9 @@ eval {
 my $certpath = 
PVE::CertHelpers::default_cert_path_prefix($nodename).".pem";
 my $capath = "/etc/pve/pve-root-ca.pem";
 
-# check if expiry is < 2W
-if (PVE::Certificate::check_expiry($certpath, time() + 14*24*60*60)) {
+my $renew = sub {
+   my ($msg) = @_;
+
# get CA info
my $cainfo = PVE::Certificate::get_certificate_info($capath);
 
@@ -94,13 +95,21 @@ eval {
# TODO: replace by low level ssleay interface if version 1.86 is 
available
PVE::Tools::run_command(['/usr/bin/openssl', 'verify', '-CAfile', 
$capath, $certpath]);
 
-   print "PVE certificate expires soon, renewing...\n";
+   print "PVE certificate $msg\n";
# create new certificate
my $ip = PVE::Cluster::remote_node_ip($nodename);
PVE::Cluster::Setup::gen_pve_ssl_cert(1, $nodename, $ip);
 
print "Restarting pveproxy after renewing certificate\n";
PVE::Tools::run_command(['systemctl', 'reload-or-restart', 'pveproxy']);
+};
+
+if (PVE::Certificate::check_expiry($certpath, time() + 14*24*60*60)) {
+   # expires in next 2 weeks
+   $renew->("expires soon, renewing...");
+} elsif (!PVE::Certificate::check_expiry($certpath, time() + 
2*365*24*60*60)) {
+   # expires in more than 2 years
+   $renew->("expires in more than 2 years, renewing to reduce certificate 
life-span...");
 }
 };
 syslog ('err', "Checking/Renewing SSL certificate failed: $@") if $@;
-- 
2.20.1


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


Re: [pve-devel] [PATCH storage v4 02/12] storage: replace build-in stat with File::stat

2020-04-23 Thread Alwin Antreich
On Thu, Apr 23, 2020 at 05:52:29AM +0200, Dietmar Maurer wrote:
> > On April 22, 2020 6:00 PM Alwin Antreich  wrote:
> > 
> >  
> > On Wed, Apr 22, 2020 at 05:35:05PM +0200, Dietmar Maurer wrote:
> > > AFAIK this can have ugly side effects ...
> > Okay, I was not aware of any know side effects.
> > 
> > I took the File::stat, since we use it already in pve-cluster,
> > qemu-server, pve-common, ... . And a off-list discussion with Thomas and
> > Fabian G.
> > 
> > If there is a better solution, I am happy to work on it.
> 
> 
> # grep -r "use File::stat" /usr/share/perl5/PVE/
> /usr/share/perl5/PVE/QemuServer/Helpers.pm:use File::stat;
> /usr/share/perl5/PVE/Storage/ISCSIPlugin.pm:use File::stat;
> /usr/share/perl5/PVE/APIServer/AnyEvent.pm:use File::stat qw();
> /usr/share/perl5/PVE/AccessControl.pm:use File::stat;
> /usr/share/perl5/PVE/Cluster.pm:use File::stat qw();
> /usr/share/perl5/PVE/LXC/Setup/Base.pm:use File::stat;
> /usr/share/perl5/PVE/QemuServer.pm:use File::stat;
> /usr/share/perl5/PVE/INotify.pm:use File::stat;
> /usr/share/perl5/PVE/API2/APT.pm:use File::stat ();
> 
> So I would use:
> 
> use File::stat qw();
> 
> to avoid override the core stat() and lstat() functions.
Thank you. I will do that and add it to a v5.

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


Re: [pve-devel] [PATCH common 1/1] JSONSchema: extend pve-configid regex by '-'

2020-04-23 Thread Fabian Grünbichler
On April 23, 2020 7:56 am, Thomas Lamprecht wrote:
> On 4/9/20 4:10 PM, Dominik Csapak wrote:
>> we use this format for all 'delete' options but we have some options
>> that have a '-' in the name (e.g. 'sync-defaults-options') that cannot
>> be deleted if it is not included
>> 
>> Signed-off-by: Dominik Csapak 
>> ---
>>  src/PVE/JSONSchema.pm | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>> 
>> diff --git a/src/PVE/JSONSchema.pm b/src/PVE/JSONSchema.pm
>> index 01a3cce..1d28f36 100644
>> --- a/src/PVE/JSONSchema.pm
>> +++ b/src/PVE/JSONSchema.pm
>> @@ -169,7 +169,7 @@ register_format('pve-configid', \_verify_configid);
>>  sub pve_verify_configid {
>>  my ($id, $noerr) = @_;
>>  
>> -if ($id !~ m/^[a-z][a-z0-9_]+$/i) {
>> +if ($id !~ m/^[a-z][a-z0-9_-]+$/i) {
>>  return undef if $noerr;
>>  die "invalid configuration ID '$id'\n";
>>  }
> 
> Fabian, Wolfgang: Any objection here? I think it should be OK, but we may
> want to adapt the pve-container and qemu-server config parsers to accept
> also the minus in keys, as it will be for sure sooner or later used there
> then too. Quick-checking other parsers wouldn't hurt either. :)

had some off-list discussions with Dominik about this already, it seems 
like he checked all the relevant call sites. the only thing I'd still 
like to see as follow up is re-using this verifier for the guest config 
parsers when parsing the snapshot section header (by chance, they 
already contain the '-', but the next time we add a character that might 
not be the case and we might miss it which would cause some fun bugs ;)).

Reviewed-By: Fabian Grünbichler 

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


Re: [pve-devel] [PATCH common 1/1] JSONSchema: extend pve-configid regex by '-'

2020-04-23 Thread Thomas Lamprecht
On 4/23/20 8:44 AM, Wolfgang Bumiller wrote:
> 
>> On April 23, 2020 7:56 AM Thomas Lamprecht  wrote:
>>
>>  
>> On 4/9/20 4:10 PM, Dominik Csapak wrote:
>>> we use this format for all 'delete' options but we have some options
>>> that have a '-' in the name (e.g. 'sync-defaults-options') that cannot
>>> be deleted if it is not included
>>>
>>> Signed-off-by: Dominik Csapak 
>>> ---
>>>  src/PVE/JSONSchema.pm | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/src/PVE/JSONSchema.pm b/src/PVE/JSONSchema.pm
>>> index 01a3cce..1d28f36 100644
>>> --- a/src/PVE/JSONSchema.pm
>>> +++ b/src/PVE/JSONSchema.pm
>>> @@ -169,7 +169,7 @@ register_format('pve-configid', \_verify_configid);
>>>  sub pve_verify_configid {
>>>  my ($id, $noerr) = @_;
>>>  
>>> -if ($id !~ m/^[a-z][a-z0-9_]+$/i) {
>>> +if ($id !~ m/^[a-z][a-z0-9_-]+$/i) {
>>> return undef if $noerr;
>>> die "invalid configuration ID '$id'\n";
>>>  }
>>
>> Fabian, Wolfgang: Any objection here? I think it should be OK, but we may
>> want to adapt the pve-container and qemu-server config parsers to accept
>> also the minus in keys, as it will be for sure sooner or later used there
>> then too. Quick-checking other parsers wouldn't hurt either. :)
> 
> You know my answer ;-)

Yeah, but I do not want the "I wish for - separate keys" answer I want the
"does it break stuff we may not have noticed" answer ;-P

> 
> Btw. those parsers are extremely similar and we could probably get away with
> one in guest-common (with a parameter for the pending section name) with
> either some post-processing or a callback for some things. A line callback
> might definitely be good to handle things like `lxc.*` raw keys.
> 

Yes, the unifying our tons of mostly similar but still different parsers
would be great, Fabian and I had a few talks about that already. :)

> Oh and, neither accepts `-` in keys yet.

That's why I mentioned it here ^^

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


[pve-devel] [PATCH access-control v2 1/3] domain sync: make options actually required

2020-04-23 Thread Dominik Csapak
we want the api options to be optional, but only as long as there are
default values set in the realm config

since they are all marked as optional (else they would be required in
the api) this check did not work as intended

instead, set the result to the value of:
* the parameter
* the set default in the config
* the api default
in this order

if it is undef after this, raise a parameter exception

Signed-off-by: Dominik Csapak 
---
changes from v1:
* better structure code to see the precedence of parameters

 PVE/API2/Domains.pm | 15 ++-
 1 file changed, 6 insertions(+), 9 deletions(-)

diff --git a/PVE/API2/Domains.pm b/PVE/API2/Domains.pm
index b3c3ac4..e139869 100644
--- a/PVE/API2/Domains.pm
+++ b/PVE/API2/Domains.pm
@@ -372,21 +372,18 @@ my $parse_sync_opts = sub {
 my $sync_opts_fmt = PVE::JSONSchema::get_format('realm-sync-options');
 
 my $res = {};
+my $defaults = {};
 if (defined(my $cfg_opts = $realmconfig->{'sync-defaults-options'})) {
-   $res = PVE::JSONSchema::parse_property_string($sync_opts_fmt, 
$cfg_opts);
+   $defaults = PVE::JSONSchema::parse_property_string($sync_opts_fmt, 
$cfg_opts);
 }
 
 for my $opt (sort keys %$sync_opts_fmt) {
my $fmt = $sync_opts_fmt->{$opt};
 
-   if (exists $param->{$opt}) {
-   $res->{$opt} = $param->{$opt};
-   } elsif (!exists $res->{$opt}) {
-   raise_param_exc({
-   "$opt" => 'Not passed as parameter and not defined in realm 
default sync options.'
-   }) if !$fmt->{optional};
-   $res->{$opt} = $fmt->{default} if exists $fmt->{default};
-   }
+   $res->{$opt} = $param->{$opt} // $defaults->{$opt} // $fmt->{default};
+   raise_param_exc({
+   "$opt" => 'Not passed as parameter and not defined in realm default 
sync options.'
+   }) if !defined($res->{$opt});
 }
 return $res;
 };
-- 
2.20.1


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


[pve-devel] [PATCH access-control v2 3/3] domain sync: add 'dry-run' parameter

2020-04-23 Thread Dominik Csapak
this can be used to test the resulting config before actually changing
anything

Signed-off-by: Dominik Csapak 
---
changes from v1:
* rename parameter from 'no-write' to 'dry-run
* drop the print sub, instead just mention at the beginning and end
  that this is just a dry run
 PVE/API2/Domains.pm | 24 +++-
 1 file changed, 19 insertions(+), 5 deletions(-)

diff --git a/PVE/API2/Domains.pm b/PVE/API2/Domains.pm
index e139869..18698c0 100644
--- a/PVE/API2/Domains.pm
+++ b/PVE/API2/Domains.pm
@@ -408,7 +408,13 @@ __PACKAGE__->register_method ({
additionalProperties => 0,
properties => get_standard_option('realm-sync-options', {
realm => get_standard_option('realm'),
-   })
+   'dry-run' => {
+   description => "If set, does not write anything.",
+   type => 'boolean',
+   optional => 1,
+   default => 0,
+   },
+   }),
 },
 returns => {
description => 'Worker Task-UPID',
@@ -420,6 +426,9 @@ __PACKAGE__->register_method ({
my $rpcenv = PVE::RPCEnvironment::get();
my $authuser = $rpcenv->get_user();
 
+   my $write = !(extract_param($param, 'dry-run'));
+   my $dryrunstring = $write ? '' : ' (dry run)';
+
my $realm = $param->{realm};
my $cfg = cfs_read_file($domainconfigfile);
my $realmconfig = $cfg->{ids}->{$realm};
@@ -439,7 +448,7 @@ __PACKAGE__->register_method ({
my $plugin = PVE::Auth::Plugin->lookup($type);
 
my $worker = sub {
-   print "starting sync for realm $realm\n";
+   print "starting sync$dryrunstring for realm $realm\n";
 
my ($synced_users, $dnmap) = $plugin->get_users($realmconfig, 
$realm);
my $synced_groups = {};
@@ -459,12 +468,17 @@ __PACKAGE__->register_method ({
$update_groups->($usercfg, $realm, $synced_groups, $opts);
}
 
-   cfs_write_file("user.cfg", $usercfg);
-   print "successfully updated $whatstring configuration\n";
+   if ($write) {
+   cfs_write_file("user.cfg", $usercfg) if $write;
+   print "successfully updated $whatstring configuration\n" if 
$write;
+   } else {
+   print "NOTE: This is just a dry run. No actual data was 
written.\n";
+   }
}, "syncing $whatstring failed");
};
 
-   return $rpcenv->fork_worker('auth-realm-sync', $realm, $authuser, 
$worker);
+   my $workerid = $write ? 'auth-realm-sync' : 'auth-realm-sync-test';
+   return $rpcenv->fork_worker($workerid, $realm, $authuser, $worker);
 }});
 
 1;
-- 
2.20.1


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


[pve-devel] [PATCH access-control v2 2/3] auth ldap/ad: introduce connection 'mode'

2020-04-23 Thread Dominik Csapak
instead of having only a 'secure' flag which switches between
ldap/ldaps we now have a mode which also contains 'ldap+starttls'

our connection code in PVE::LDAP can handle this already (used in pmg)
so that is no problem

if we want to really remove the 'secure' flag, e.g. in 7.0
we'd either have to rewrite the config or have it as an error
in a pve6to7 script

Signed-off-by: Dominik Csapak 
---
changes from v1:
* refactor get_scheme_and_port

 PVE/Auth/AD.pm   |  9 -
 PVE/Auth/LDAP.pm | 25 +
 2 files changed, 25 insertions(+), 9 deletions(-)

diff --git a/PVE/Auth/AD.pm b/PVE/Auth/AD.pm
index 24b0e9f..4d64c20 100755
--- a/PVE/Auth/AD.pm
+++ b/PVE/Auth/AD.pm
@@ -27,7 +27,7 @@ sub properties {
maxLength => 256,
},
secure => {
-   description => "Use secure LDAPS protocol.",
+   description => "Use secure LDAPS protocol. DEPRECATED: use 'mode' 
instead.",
type => 'boolean',
optional => 1,
},
@@ -93,6 +93,7 @@ sub options {
group_filter => { optional => 1 },
group_classes => { optional => 1 },
'sync-defaults-options' => { optional => 1 },
+   mode => { optional => 1 },
 };
 }
 
@@ -110,9 +111,7 @@ sub authenticate_user {
 my $servers = [$config->{server1}];
 push @$servers, $config->{server2} if $config->{server2};
 
-my $default_port = $config->{secure} ? 636: 389;
-my $port = $config->{port} // $default_port;
-my $scheme = $config->{secure} ? 'ldaps' : 'ldap';
+my ($scheme, $port) = $class->get_scheme_and_port($config);
 
 my %ad_args;
 if ($config->{verify}) {
@@ -130,7 +129,7 @@ sub authenticate_user {
$ad_args{verify} = 'none';
 }
 
-if ($config->{secure}) {
+if ($scheme ne 'ldap') {
$ad_args{sslversion} = $config->{sslversion} // 'tlsv1_2';
 }
 
diff --git a/PVE/Auth/LDAP.pm b/PVE/Auth/LDAP.pm
index 6b6b184..64250cb 100755
--- a/PVE/Auth/LDAP.pm
+++ b/PVE/Auth/LDAP.pm
@@ -122,6 +122,13 @@ sub properties {
format => 'realm-sync-options',
optional => 1,
},
+   mode => {
+   description => "LDAP protocol mode.",
+   type => 'string',
+   enum => [ 'ldap', 'ldaps', 'ldap+starttls'],
+   optional => 1,
+   default => 'ldap',
+   },
 };
 }
 
@@ -151,18 +158,28 @@ sub options {
group_filter => { optional => 1 },
group_classes => { optional => 1 },
'sync-defaults-options' => { optional => 1 },
+   mode => { optional => 1 },
 };
 }
 
+sub get_scheme_and_port {
+my ($class, $config) = @_;
+
+my $scheme = $config->{mode} // ($config->{secure} ? 'ldaps' : 'ldap');
+
+my $default_port = $scheme eq 'ldaps' ? 636 : 389;
+my $port = $config->{port} // $default_port;
+
+return ($scheme, $port);
+}
+
 sub connect_and_bind {
 my ($class, $config, $realm) = @_;
 
 my $servers = [$config->{server1}];
 push @$servers, $config->{server2} if $config->{server2};
 
-my $default_port = $config->{secure} ? 636: 389;
-my $port = $config->{port} // $default_port;
-my $scheme = $config->{secure} ? 'ldaps' : 'ldap';
+my ($scheme, $port) = $class->get_scheme_and_port($config);
 
 my %ldap_args;
 if ($config->{verify}) {
@@ -180,7 +197,7 @@ sub connect_and_bind {
$ldap_args{verify} = 'none';
 }
 
-if ($config->{secure}) {
+if ($scheme ne 'ldap') {
$ldap_args{sslversion} = $config->{sslversion} || 'tlsv1_2';
 }
 
-- 
2.20.1


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


Re: [pve-devel] [PATCH common 1/1] JSONSchema: extend pve-configid regex by '-'

2020-04-23 Thread Wolfgang Bumiller


> On April 23, 2020 7:56 AM Thomas Lamprecht  wrote:
> 
>  
> On 4/9/20 4:10 PM, Dominik Csapak wrote:
> > we use this format for all 'delete' options but we have some options
> > that have a '-' in the name (e.g. 'sync-defaults-options') that cannot
> > be deleted if it is not included
> > 
> > Signed-off-by: Dominik Csapak 
> > ---
> >  src/PVE/JSONSchema.pm | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/src/PVE/JSONSchema.pm b/src/PVE/JSONSchema.pm
> > index 01a3cce..1d28f36 100644
> > --- a/src/PVE/JSONSchema.pm
> > +++ b/src/PVE/JSONSchema.pm
> > @@ -169,7 +169,7 @@ register_format('pve-configid', \_verify_configid);
> >  sub pve_verify_configid {
> >  my ($id, $noerr) = @_;
> >  
> > -if ($id !~ m/^[a-z][a-z0-9_]+$/i) {
> > +if ($id !~ m/^[a-z][a-z0-9_-]+$/i) {
> > return undef if $noerr;
> > die "invalid configuration ID '$id'\n";
> >  }
> 
> Fabian, Wolfgang: Any objection here? I think it should be OK, but we may
> want to adapt the pve-container and qemu-server config parsers to accept
> also the minus in keys, as it will be for sure sooner or later used there
> then too. Quick-checking other parsers wouldn't hurt either. :)

You know my answer ;-)

Btw. those parsers are extremely similar and we could probably get away with
one in guest-common (with a parameter for the pending section name) with
either some post-processing or a callback for some things. A line callback
might definitely be good to handle things like `lxc.*` raw keys.

Oh and, neither accepts `-` in keys yet.

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