[pve-devel] applied: [PATCH storage] plugins: untaint volume_size_info retuns

2021-06-23 Thread Thomas Lamprecht
On 22.06.21 18:39, Stoiko Ivanov wrote:
> the size returned by volume_size_info is used for creating the new
> destination image in PVE::QemuServer::clone_disk (and probably
> elsewhere). In certain cases the return values are tainted - they are
> obtained by a run_command call and depending on the format and length
> of the parsed output can still have their tainted attribute.
> 
> One example of a tainted return has been reported in our
> community-forum:
> https://forum.proxmox.com/threads/cannot-clone-vm-or-move-disk-with-more-than-13-snapshots.89628/
> 
> A qcow2 image with 13 snapshots generates a output > 4k in length from
> `qemu-img info --output=json`, which in turn causes the output to be
> considered tainted.
> 
> This patch untaints the returns where applicable. The other
> storage-plugins are not affected:
> * LVMPlugin returns a single number and a newline (thus gets untainted
>   by run_command)
> * RBDPlugin untaints the complete json before decoding
> * ZFSPoolplugin and ISCSIDirectPlugin explicitly untaint their
>   returns.
> 
> Signed-off-by: Stoiko Ivanov 
> ---
> 
> Note:
> Not really a v2, since it's a different patch, but addresses the same issue
> as in https://lists.proxmox.com/pipermail/pve-devel/2021-June/048910.html

Aren't the version rather tied to the issue they try to solve, as 
implementations
and approaches can always significantly change? So I'd be OK with this being
marked as v2, but no hard feelings.

> 
>  PVE/Storage/PBSPlugin.pm | 4 +++-
>  PVE/Storage/Plugin.pm| 6 ++
>  2 files changed, 9 insertions(+), 1 deletion(-)
> 
>

applied to master and a newly branched stable-6, thanks!

I made two followups:

1) - fix nit that we have a space in comments after the #
   - actually die with error message if untaint fails

2) return early if decode JSON fails, compensates issues where the qemu-img
   command somehow fails (e.g., file was removed since the stat check) and
   the semantic change from 1)

Please check if this still looks alright to you.


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



[pve-devel] applied: [PATCH i18n]: Update Arabic translationst

2021-06-23 Thread Thomas Lamprecht
On 22.06.21 17:11, Moayad Almalat wrote:
> ---
>  ar.po | 2942 +++--
>  1 file changed, 1197 insertions(+), 1745 deletions(-)
> 
>

applied, thanks!

FYI, I updated the translation strings from latest pve-manager, pmg, pbs git 
afterwards,
so there may be a few new/fuzyy ones now again.


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



[pve-devel] applied: [PATCH docs] pbs: add information about master key support

2021-06-23 Thread Thomas Lamprecht
On 22.06.21 13:43, Fabian Grünbichler wrote:
> Signed-off-by: Fabian Grünbichler 
> ---
>  pve-storage-pbs.adoc | 19 +++
>  1 file changed, 19 insertions(+)
> 
>

applied, thanks!


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


[pve-devel] [PATCH-SERIES v2 manager/docs] add warning for disabling users

2021-06-23 Thread Lorenz Stechauner
changes to v1:
* e.g. -> for example
* small sentence structure changes


pve-manager:
Lorenz Stechauner (1):
  ui: dc/UserEdit: add warning for disabling users

 www/manager6/dc/UserEdit.js | 14 ++
 1 file changed, 14 insertions(+)


pve-doc:
Lorenz Stechauner (1):
  pveum: add warning for disabling or deleting users

 pveum.adoc | 5 +
 1 file changed, 5 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 v2 docs 1/1] pveum: add warning for disabling or deleting users

2021-06-23 Thread Lorenz Stechauner
see #3101

Signed-off-by: Lorenz Stechauner 
---
 pveum.adoc | 5 +
 1 file changed, 5 insertions(+)

diff --git a/pveum.adoc b/pveum.adoc
index 71ea7ef..5feb2c2 100644
--- a/pveum.adoc
+++ b/pveum.adoc
@@ -56,6 +56,11 @@ Each user entry in this file contains the following 
information:
 * Whether this user is enabled or disabled
 * Optional two-factor authentication keys
 
+CAUTION: After disabling or deleting a user, this user will not be able to
+log in to new sessions or start new tasks. All tasks which already have
+been started by this user (for example terminal sessions) will **not** be
+terminated automatically.
+
 
 System administrator
 
-- 
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 manager 1/1] ui: dc/UserEdit: add warning for disabling users

2021-06-23 Thread Lorenz Stechauner
see #3101

Signed-off-by: Lorenz Stechauner 
---
 www/manager6/dc/UserEdit.js | 14 ++
 1 file changed, 14 insertions(+)

diff --git a/www/manager6/dc/UserEdit.js b/www/manager6/dc/UserEdit.js
index b637cd53..65a612fd 100644
--- a/www/manager6/dc/UserEdit.js
+++ b/www/manager6/dc/UserEdit.js
@@ -73,6 +73,12 @@ Ext.define('PVE.dc.UserEdit', {
uncheckedValue: 0,
defaultValue: 1,
checked: true,
+   listeners: {
+   change: function(checkbox) {
+   let taskWarning = me.lookup('taskWarning');
+   taskWarning.setHidden(!(me.wasEnabled && 
!checkbox.value));
+   },
+   },
},
];
 
@@ -93,6 +99,13 @@ Ext.define('PVE.dc.UserEdit', {
fieldLabel: gettext('E-Mail'),
vtype: 'proxmoxMail',
},
+   {
+   xtype: 'displayfield',
+   reference: 'taskWarning',
+   userCls: 'pmx-hint',
+   value: gettext('Note: Already running tasks of user will not be 
terminated automatically!'),
+   hidden: true,
+   },
];
 
if (me.isCreate) {
@@ -161,6 +174,7 @@ Ext.define('PVE.dc.UserEdit', {
success: function(response, options) {
var data = response.result.data;
me.setValues(data);
+   me.wasEnabled = data.enable;
if (data.keys) {
if (data.keys === 'x!oath' || data.keys === 'x!u2f') {
me.down('[name="keys"]').setDisabled(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 docs] package repos: fix typo

2021-06-23 Thread Fabian Ebner
Signed-off-by: Fabian Ebner 
---
 pve-package-repos.adoc | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/pve-package-repos.adoc b/pve-package-repos.adoc
index 20e881f..4d4051c 100644
--- a/pve-package-repos.adoc
+++ b/pve-package-repos.adoc
@@ -96,7 +96,7 @@ testing new features or bug fixes.
 Ceph Pacific Repository
 ~~~
 
-NOTE: Ceph Octopus (16.2) was declared stable with {pve} 7.0
+NOTE: Ceph Pacific (16.2) was declared stable with {pve} 7.0.
 
 This repository holds the main {pve} Ceph Pacific packages. They are suitable
 for production. Use this repository if you run the Ceph client or a full Ceph
-- 
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 spiceterm] screen.c: fix exiting on client disconnect

2021-06-23 Thread Dominik Csapak
the way we detected a client_disconnect does not work anymore with
spice-server >= 0.14.3, but there is a convienient function
that exits the spice-server on client disconnect

there is a slight behaviour change that should not make a big difference:

previously, if a user used the same '.vv' file twice, the first client
got disconnected and the second would connect

now, because the server closes on disconnect, the second client is
not be able to connect anymore

using our ui though, every click on 'shell>spice' generates a new '.vv'
file that works though (and for container shells we use dtach to mux)

i could not (in reasonable time) find out why the first client
disconnects at all, and also could not find another way to detect a
client disconnect that is similar to our previous method

Signed-off-by: Dominik Csapak 
---
 src/screen.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/src/screen.c b/src/screen.c
index 871d16f..ee6e34a 100644
--- a/src/screen.c
+++ b/src/screen.c
@@ -789,6 +789,8 @@ spice_screen_new(
 }
 }
 
+spice_server_set_exit_on_disconnect(server, 1);
+
 int res = spice_server_init(server, core);
 if (res != 0) {
 g_error("spice_server_init failed, res = %d\n", res);
-- 
2.20.1



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



Re: [pve-devel] [PATCH spiceterm] screen.c: fix exiting on client disconnect

2021-06-23 Thread Dominik Csapak

to answer my own question about multiple clients:

https://www.spice-space.org/multiple-clients.html

it seems that feature is still experimental..


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



[pve-devel] applied: [PATCH docs] package repos: fix typo

2021-06-23 Thread Thomas Lamprecht
On 23.06.21 09:39, Fabian Ebner wrote:
> Signed-off-by: Fabian Ebner 
> ---
>  pve-package-repos.adoc | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
>

applied, thanks!


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



[pve-devel] applied: [PATCH spiceterm] screen.c: fix exiting on client disconnect

2021-06-23 Thread Thomas Lamprecht
On 23.06.21 09:45, Dominik Csapak wrote:
> the way we detected a client_disconnect does not work anymore with
> spice-server >= 0.14.3, but there is a convienient function
> that exits the spice-server on client disconnect
> 
> there is a slight behaviour change that should not make a big difference:
> 
> previously, if a user used the same '.vv' file twice, the first client
> got disconnected and the second would connect
> 
> now, because the server closes on disconnect, the second client is
> not be able to connect anymore
> 
> using our ui though, every click on 'shell>spice' generates a new '.vv'
> file that works though (and for container shells we use dtach to mux)
> 
> i could not (in reasonable time) find out why the first client
> disconnects at all, and also could not find another way to detect a
> client disconnect that is similar to our previous method
> 
> Signed-off-by: Dominik Csapak 
> ---
>  src/screen.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
>

applied, thanks!


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



[pve-devel] [PATCH v2 manager] postinst: set custom LVM settings

2021-06-23 Thread Fabian Grünbichler
now that we no longer ship our own LVM packages, set the relevant
filtering options here if they are missing.

for an upgrade from PVE 6.x, the following two scenarios are likely:

A: user edited config provided by our old lvm2 package. it likely
contains our (or a modified) global_filter, but the old scan_lvs
default. in this case we ignore global_filter as long as it contains our
'don't scan zvols' entry, and set scan_lvs to false.

B: config provided by our old lvm2 package was taken over by default
config from stock lvm2 package. scan_lvs defaults to false already, but
global_filter is unset (scan everything), so we need to set our own
global_filter excluding zvols.

other combinations should be handled fine as well.

for new installs (installer, install on top of Debian Bullseye) we are
always in scenario B.

Signed-off-by: Fabian Grünbichler 
---

Notes:
once other difference between our old config and the stock one is that we 
had
'issue_discards' enabled. we could either put this in the release notes, or
also enable it here automatically - but it is less straight-forward since 
the
default is not "almost certainly wrong" like for the filtering options..

we could drop the "check for marker" and just do this once on initial 
install
and upgrades from 6.x, but since the fallout from not having these in place 
can
be data corruption (activating multiple VGs with same name, using one from a
guest on the host!) I'd rather play it safe..

v2:
- move more variables into if branch
- export env variable to suppress LVM warnings (thanks Thomas)
- fix grep for marker ("any lines that don't match" vs "no lines that 
match")

 debian/postinst | 56 +
 1 file changed, 56 insertions(+)

diff --git a/debian/postinst b/debian/postinst
index 85f2a70a..0f6296c3 100755
--- a/debian/postinst
+++ b/debian/postinst
@@ -8,6 +8,60 @@ set -e
 # done its automatic conffile handling, and all the packages we depend
 # of are already fully installed and configured.
 
+set_lvm_conf() {
+LVM_CONF_MARKER="# added by pve-manager to avoid scanning"
+
+# only do these changes once
+# keep user changes afterwards provided marker is still there..
+if ! grep -qLF "$LVM_CONF_MARKER" /etc/lvm/lvm.conf; then
+   OLD_VALUE="$(lvmconfig --typeconfig full devices/global_filter)"
+   NEW_VALUE='global_filter=["r|/dev/zd.*|"]'
+
+   export LVM_SUPPRESS_FD_WARNINGS=1
+
+   # check global_filter
+   # keep previous setting from our custom packaging if it is still there
+   if echo "$OLD_VALUE" | grep -qvF 'r|/dev/zd.*|'; then
+   SET_FILTER=1
+   BACKUP=1
+   fi
+   # should be the default since bullseye
+   if lvmconfig --typeconfig full devices/scan_lvs | grep -qv 
'scan_lvs=0'; then
+   SET_SCAN_LVS=1
+   BACKUP=1
+   fi
+   if test -n "$BACKUP"; then
+   echo "Backing up lvm.conf before setting pve-manager specific 
settings.."
+   cp -vb /etc/lvm/lvm.conf /etc/lvm/lvm.conf.bak
+   fi
+   if test -n "$SET_FILTER"; then
+   echo "Setting 'global_filter' in /etc/lvm/lvm.conf to prevent zvols 
from being scanned:"
+   echo "$OLD_VALUE => $NEW_VALUE"
+   # comment out existing setting
+   sed -i -e 's/^\([[:space:]]*global_filter[[:space:]]*=\)/#\1/' 
/etc/lvm/lvm.conf
+   # add new section with our setting
+   cat >> /etc/lvm/lvm.conf <> /etc/lvm/lvm.conf 

[pve-devel] applied: [PATCH v2 manager] postinst: set custom LVM settings

2021-06-23 Thread Thomas Lamprecht
On 23.06.21 11:00, Fabian Grünbichler wrote:
> now that we no longer ship our own LVM packages, set the relevant
> filtering options here if they are missing.
> 
> for an upgrade from PVE 6.x, the following two scenarios are likely:
> 
> A: user edited config provided by our old lvm2 package. it likely
> contains our (or a modified) global_filter, but the old scan_lvs
> default. in this case we ignore global_filter as long as it contains our
> 'don't scan zvols' entry, and set scan_lvs to false.
> 
> B: config provided by our old lvm2 package was taken over by default
> config from stock lvm2 package. scan_lvs defaults to false already, but
> global_filter is unset (scan everything), so we need to set our own
> global_filter excluding zvols.
> 
> other combinations should be handled fine as well.
> 
> for new installs (installer, install on top of Debian Bullseye) we are
> always in scenario B.
> 
> Signed-off-by: Fabian Grünbichler 
> ---
> 
> Notes:
> once other difference between our old config and the stock one is that we 
> had
> 'issue_discards' enabled. we could either put this in the release notes, 
> or
> also enable it here automatically - but it is less straight-forward since 
> the
> default is not "almost certainly wrong" like for the filtering options..
> 
> we could drop the "check for marker" and just do this once on initial 
> install
> and upgrades from 6.x, but since the fallout from not having these in 
> place can
> be data corruption (activating multiple VGs with same name, using one 
> from a
> guest on the host!) I'd rather play it safe..
> 
> v2:
> - move more variables into if branch
> - export env variable to suppress LVM warnings (thanks Thomas)
> - fix grep for marker ("any lines that don't match" vs "no lines that 
> match")
> 
>  debian/postinst | 56 +
>  1 file changed, 56 insertions(+)
> 
>

applied, thanks!


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


[pve-devel] applied: [PATCH v2 docs 1/1] pveum: add warning for disabling or deleting users

2021-06-23 Thread Thomas Lamprecht
On 23.06.21 09:37, Lorenz Stechauner wrote:
> see #3101
> 
> Signed-off-by: Lorenz Stechauner 
> ---
>  pveum.adoc | 5 +
>  1 file changed, 5 insertions(+)
> 
>

applied, added also that this is true for hitting the expiry date too, thanks!


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



Re: [pve-devel] [PATCH v2 manager 1/1] ui: dc/UserEdit: add warning for disabling users

2021-06-23 Thread Thomas Lamprecht
On 23.06.21 09:37, Lorenz Stechauner wrote:
> see #3101
> 
> Signed-off-by: Lorenz Stechauner 
> ---

I should have given this a closer look when commenting on the docs v1 patch, 
sorry.

In general this would miss for removal and when the expiry date would be set to 
the
past, but it may make sense to place a API call for running tasks from the user 
and
only show the warning if there are any.

But, with the docs change applied it's at least clearly documented, so IMO not 
a very
pressing matter.

>  www/manager6/dc/UserEdit.js | 14 ++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/www/manager6/dc/UserEdit.js b/www/manager6/dc/UserEdit.js
> index b637cd53..65a612fd 100644
> --- a/www/manager6/dc/UserEdit.js
> +++ b/www/manager6/dc/UserEdit.js
> @@ -73,6 +73,12 @@ Ext.define('PVE.dc.UserEdit', {
>   uncheckedValue: 0,
>   defaultValue: 1,
>   checked: true,
> + listeners: {
> + change: function(checkbox) {

the change listener gives you the new value as second parameter, please us that 
one.

https://docs.sencha.com/extjs/6.0.1/classic/Ext.form.field.Checkbox.html#event-change

> + let taskWarning = me.lookup('taskWarning');
> + taskWarning.setHidden(!(me.wasEnabled && 
> !checkbox.value));

for single use, where we are sure that the variable isn't undefined/null, it's 
often nicer
to just use chaining, e.g., here:

me.lookup('taskWarning').setHidden(!(me.wasEnabled && !value));

also 


> + },
> + },
>   },
>   ];
>  
> @@ -93,6 +99,13 @@ Ext.define('PVE.dc.UserEdit', {
>   fieldLabel: gettext('E-Mail'),
>   vtype: 'proxmoxMail',
>   },
> + {
> + xtype: 'displayfield',
> + reference: 'taskWarning',
> + userCls: 'pmx-hint',
> + value: gettext('Note: Already running tasks of user will not be 
> terminated automatically!'),
> + hidden: true,
> + },

I'd move that to the end of (non-advanced) columnB, looks better there IMO.

>   ];
>  
>   if (me.isCreate) {
> @@ -161,6 +174,7 @@ Ext.define('PVE.dc.UserEdit', {
>   success: function(response, options) {
>   var data = response.result.data;
>   me.setValues(data);
> + me.wasEnabled = data.enable;
>   if (data.keys) {
>   if (data.keys === 'x!oath' || data.keys === 'x!u2f') {
>   me.down('[name="keys"]').setDisabled(1);
> 



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



[pve-devel] [PATCH series/flutter 0/4] fixes related to PVE 7

2021-06-23 Thread Aaron Lauterer
This series contains a few fixes for problems that I came across when
testing the flutter app against Proxmox VE 7.

The first bigger one, affecting the first 3 patches, is handling of
different types when parsing API responses. The PVE API is not always
returning the expected type. In most situations it is numbers formatted
as strings instead of numbers.

For dynamically types languages this is usually much less of a problem
as it is in statically typed languages as dart.
To handle that, I introduced new custom serializers for Strings,
integers and doubles. If the value is not of the expected type, they
should be able to handle situations where the number is formatted as
string or where the expected string is formatted as number and
cast/parse them accordingly.

The last patch addresses a problem that showed up when trying to open
the power menu for a guest that had been powered off. The handling of
the states template value being null has not been done correctly. I used
the same approach as throughout the other parts where the template
status is checked -> falling back to false.

dart_api_client: Aaron Lauterer (3):
  Add string serializer to handle int and double values
  Add int serializer to handle string values
  Add double serializer to handle String and int values

 lib/src/models/pve_double_serializer.dart | 28 +++
 lib/src/models/pve_int_serializer.dart| 25 
 lib/src/models/pve_string_serializer.dart | 25 
 lib/src/models/serializers.dart   |  6 +
 4 files changed, 84 insertions(+)
 create mode 100644 lib/src/models/pve_double_serializer.dart
 create mode 100644 lib/src/models/pve_int_serializer.dart
 create mode 100644 lib/src/models/pve_string_serializer.dart

flutter_frontend: Aaron Lauterer (1):
  power settings: fix handling null for template status

 lib/widgets/pve_qemu_power_settings_widget.dart | 4 ++--
 1 file changed, 2 insertions(+), 2 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 dart_api_client 3/4] Add double serializer to handle String and int values

2021-06-23 Thread Aaron Lauterer
Adding a custom serializer for double values allows us to handle
situations in which the PVE API provides values in other types. The most
likely possibility is that numbers are formatted as string in the JSON
response which needs to be parsed to double.

We must also handle the situation that the value is an integer which
needs to be cast to double.

Signed-off-by: Aaron Lauterer 
---
 lib/src/models/pve_double_serializer.dart | 28 +++
 lib/src/models/serializers.dart   |  2 ++
 2 files changed, 30 insertions(+)
 create mode 100644 lib/src/models/pve_double_serializer.dart

diff --git a/lib/src/models/pve_double_serializer.dart 
b/lib/src/models/pve_double_serializer.dart
new file mode 100644
index 000..3743a80
--- /dev/null
+++ b/lib/src/models/pve_double_serializer.dart
@@ -0,0 +1,28 @@
+import 'package:built_collection/built_collection.dart';
+import 'package:built_value/serializer.dart';
+
+class PveDoubleSerializer implements PrimitiveSerializer {
+  final bool structured = false;
+  @override
+  final Iterable types = BuiltList([double]);
+  @override
+  final String wireName = 'double';
+
+  @override
+  Object serialize(Serializers serializers, double value,
+  {FullType specifiedType = FullType.unspecified}) {
+return value;
+  }
+
+  @override
+  double deserialize(Serializers serializers, Object? serialized,
+  {FullType specifiedType = FullType.unspecified}) {
+if (serialized is String) {
+  return double.parse(serialized);
+}
+if (serialized is int) {
+  return (serialized).toDouble();
+}
+return serialized as double;
+  }
+}
diff --git a/lib/src/models/serializers.dart b/lib/src/models/serializers.dart
index 2a0d501..0be80fc 100644
--- a/lib/src/models/serializers.dart
+++ b/lib/src/models/serializers.dart
@@ -3,6 +3,7 @@ import 'package:built_value/json_object.dart';
 import 'package:built_value/serializer.dart';
 import 'package:built_value/standard_json_plugin.dart';
 import 'package:proxmox_dart_api_client/src/models/pve_bool_serializer.dart';
+import 'package:proxmox_dart_api_client/src/models/pve_double_serializer.dart';
 import 'package:proxmox_dart_api_client/src/models/pve_int_serializer.dart';
 import 'package:proxmox_dart_api_client/src/models/pve_string_serializer.dart';
 import 
'package:proxmox_dart_api_client/src/models/pve_datetime_from_epoch_serializer.dart';
@@ -43,6 +44,7 @@ part 'serializers.g.dart';
 final Serializers serializers = (_$serializers.toBuilder()
   ..addPlugin(StandardJsonPlugin())
   ..add(PveBoolSerializer())
+  ..add(PveDoubleSerializer())
   ..add(PveIntSerializer())
   ..add(PveStringSerializer())
   ..add(PveDateTimeFromEpoch()))
-- 
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 flutter_frontend 4/4] power settings: fix handling null for template status

2021-06-23 Thread Aaron Lauterer
The template status of a guest can be null and we need to catch it.

This patch aligns that with all the other instances where we do check
the template status.

Signed-off-by: Aaron Lauterer 
---
 lib/widgets/pve_qemu_power_settings_widget.dart | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lib/widgets/pve_qemu_power_settings_widget.dart 
b/lib/widgets/pve_qemu_power_settings_widget.dart
index 65b3c0c..1c1d2ed 100644
--- a/lib/widgets/pve_qemu_power_settings_widget.dart
+++ b/lib/widgets/pve_qemu_power_settings_widget.dart
@@ -26,7 +26,7 @@ class PveQemuPowerSettings extends StatelessWidget {
 mainAxisSize: MainAxisSize.min,
 children: [
   if (qemuStatus == PveResourceStatusType.stopped &&
-  !state.currentStatus.template)
+  !(state.currentStatus.template ?? false))
 ListTile(
   leading: Icon(Icons.play_arrow),
   title: Text(
@@ -41,7 +41,7 @@ class PveQemuPowerSettings extends StatelessWidget {
 PveResourceStatusType.paused,
 PveResourceStatusType.suspended
   ].contains(qemuStatus) &&
-  !state.currentStatus.template)
+  !(state.currentStatus.template ?? false))
 ListTile(
   leading: Icon(Icons.play_arrow),
   title: Text(
-- 
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 dart_api_client 1/4] Add string serializer to handle int and double values

2021-06-23 Thread Aaron Lauterer
Adding a custom serializer for Strings allows us to catch situations
where the PVE API provides an int or a double instead of the expected
string and convert that to a string.

Signed-off-by: Aaron Lauterer 
---
 lib/src/models/pve_string_serializer.dart | 25 +++
 lib/src/models/serializers.dart   |  2 ++
 2 files changed, 27 insertions(+)
 create mode 100644 lib/src/models/pve_string_serializer.dart

diff --git a/lib/src/models/pve_string_serializer.dart 
b/lib/src/models/pve_string_serializer.dart
new file mode 100644
index 000..0622360
--- /dev/null
+++ b/lib/src/models/pve_string_serializer.dart
@@ -0,0 +1,25 @@
+import 'package:built_collection/built_collection.dart';
+import 'package:built_value/serializer.dart';
+
+class PveStringSerializer implements PrimitiveSerializer {
+  final bool structured = false;
+  @override
+  final Iterable types = BuiltList([String]);
+  @override
+  final String wireName = 'String';
+
+  @override
+  Object serialize(Serializers serializers, String string,
+  {FullType specifiedType = FullType.unspecified}) {
+return string;
+  }
+
+  @override
+  String deserialize(Serializers serializers, Object? serialized,
+  {FullType specifiedType = FullType.unspecified}) {
+if (serialized is int || serialized is double) {
+  return serialized.toString();
+}
+return serialized as String;
+  }
+}
diff --git a/lib/src/models/serializers.dart b/lib/src/models/serializers.dart
index c079ee3..57b06f4 100644
--- a/lib/src/models/serializers.dart
+++ b/lib/src/models/serializers.dart
@@ -3,6 +3,7 @@ import 'package:built_value/json_object.dart';
 import 'package:built_value/serializer.dart';
 import 'package:built_value/standard_json_plugin.dart';
 import 'package:proxmox_dart_api_client/src/models/pve_bool_serializer.dart';
+import 'package:proxmox_dart_api_client/src/models/pve_string_serializer.dart';
 import 
'package:proxmox_dart_api_client/src/models/pve_datetime_from_epoch_serializer.dart';
 import 'package:proxmox_dart_api_client/src/models/pve_models.dart';
 
@@ -41,5 +42,6 @@ part 'serializers.g.dart';
 final Serializers serializers = (_$serializers.toBuilder()
   ..addPlugin(StandardJsonPlugin())
   ..add(PveBoolSerializer())
+  ..add(PveStringSerializer())
   ..add(PveDateTimeFromEpoch()))
 .build();
-- 
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 dart_api_client 2/4] Add int serializer to handle string values

2021-06-23 Thread Aaron Lauterer
Adding a custom serializer for integers, allows us to handle situations
in which the PVE API provides the values in other types. For now, the
most likely situation is that the API formats the JSON value as string
instead of a number, therefore we try to catch that and parse it as int.

Signed-off-by: Aaron Lauterer 
---
 lib/src/models/pve_int_serializer.dart | 25 +
 lib/src/models/serializers.dart|  2 ++
 2 files changed, 27 insertions(+)
 create mode 100644 lib/src/models/pve_int_serializer.dart

diff --git a/lib/src/models/pve_int_serializer.dart 
b/lib/src/models/pve_int_serializer.dart
new file mode 100644
index 000..8986463
--- /dev/null
+++ b/lib/src/models/pve_int_serializer.dart
@@ -0,0 +1,25 @@
+import 'package:built_collection/built_collection.dart';
+import 'package:built_value/serializer.dart';
+
+class PveIntSerializer implements PrimitiveSerializer {
+  final bool structured = false;
+  @override
+  final Iterable types = BuiltList([int]);
+  @override
+  final String wireName = 'String';
+
+  @override
+  Object serialize(Serializers serializers, int value,
+  {FullType specifiedType = FullType.unspecified}) {
+return value;
+  }
+
+  @override
+  int deserialize(Serializers serializers, Object? serialized,
+  {FullType specifiedType = FullType.unspecified}) {
+if (serialized is String) {
+  return int.parse(serialized);
+}
+return serialized as int;
+  }
+}
diff --git a/lib/src/models/serializers.dart b/lib/src/models/serializers.dart
index 57b06f4..2a0d501 100644
--- a/lib/src/models/serializers.dart
+++ b/lib/src/models/serializers.dart
@@ -3,6 +3,7 @@ import 'package:built_value/json_object.dart';
 import 'package:built_value/serializer.dart';
 import 'package:built_value/standard_json_plugin.dart';
 import 'package:proxmox_dart_api_client/src/models/pve_bool_serializer.dart';
+import 'package:proxmox_dart_api_client/src/models/pve_int_serializer.dart';
 import 'package:proxmox_dart_api_client/src/models/pve_string_serializer.dart';
 import 
'package:proxmox_dart_api_client/src/models/pve_datetime_from_epoch_serializer.dart';
 import 'package:proxmox_dart_api_client/src/models/pve_models.dart';
@@ -42,6 +43,7 @@ part 'serializers.g.dart';
 final Serializers serializers = (_$serializers.toBuilder()
   ..addPlugin(StandardJsonPlugin())
   ..add(PveBoolSerializer())
+  ..add(PveIntSerializer())
   ..add(PveStringSerializer())
   ..add(PveDateTimeFromEpoch()))
 .build();
-- 
2.30.2



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



[pve-devel] applied-series: [PATCH v2 qemu-server 1/2] enable io-uring support

2021-06-23 Thread Thomas Lamprecht
On 21.06.21 17:33, Stefan Reiter wrote:
> Note that the value in this enum directly represents the value passed to
> QEMU, so we need to use the underscore.
> 
> Signed-off-by: Stefan Reiter 
> ---
> 
> same as v1, par the commit message
> 
>  PVE/QemuServer/Drive.pm | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
>

applied both patches, thanks!


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



[pve-devel] applied: [PATCH v4 qemu-server] vm_start: check if storages of volumes support correct content-type

2021-06-23 Thread Thomas Lamprecht
On 22.06.21 14:30, Lorenz Stechauner wrote:
> Signed-off-by: Lorenz Stechauner 
> ---
> changes to v3:
> * dropped already applied patch
> * moved check to cfg2cmd (with helper)
> * 'images' content-type is not anymore hard-coded
> 
> vm state files are not included anymore and efi disks have type 'images' - 
> therefore they should work
> 
>  PVE/QemuServer.pm | 14 ++
>  1 file changed, 14 insertions(+)
> 
>

applied, thanks!


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



[pve-devel] applied: [PATCH qemu-server 1/2] use KillMode 'process' for systemd scope

2021-06-23 Thread Thomas Lamprecht
On 21.06.21 18:35, Stefan Reiter wrote:
> KillMode 'none' is deprecated, and systemd loudly complains about that
> in the journal. To avoid the warning, but keep the behaviour the same,
> use KillMode 'process'.
> 
> This mode does two things differently, which we have to stop it from
> doing:
> * it sends SIGTERM right when the scope is cancelled (e.g. on shutdown)
>  -> but only to the "root" process, which in our case is the worker
>  instance forking QEMU, so it is already dead by the time this happens
> * it sends SIGKILL to *all* children after a timeout
>  -> can be avoided by setting either SendSIGKILL to false, or
>  TimeoutStopUSec to infinity - for safety, we do both
> 
> In my testing, this replicated the previous behaviour exactly, but
> without using the deprecated 'none' mode.
> 
> Signed-off-by: Stefan Reiter 
> ---
> 
> Depends on updated pve-common from patch 2.
> 
>  PVE/QemuServer.pm | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
>

applied, thanks!


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



[pve-devel] partially-applied: [PATCH qemu-server 0/6] fix #2862: more template backup fixes

2021-06-23 Thread Thomas Lamprecht
On 04.06.21 11:47, Fabian Grünbichler wrote:
> first one is unrelated, rest should fix the remaining cases where
> template backups to PBS fail.
> 
> Fabian Grünbichler (6):
>   test: unbreak restore_config_test
>   drive: factor out read-only helper
>   template: mark efidisk as read-only
>   test: add template drive read-only tests
>   template: add -snapshot to KVM command

applied above, i.e., all but the RFC below. I adapted the EFI-disk one to what
Stefan proposed, i.e., just drop the readonly if off. There also was the need to
solve a merge conflict, so I just squashed it all into that one.

>   template: start VM for VMA backup


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


[pve-devel] applied: [PATCH v2 qemu-server] allow migrating raw btrfs volumes

2021-06-23 Thread Thomas Lamprecht
On 22.06.21 14:18, Wolfgang Bumiller wrote:
> Signed-off-by: Wolfgang Bumiller 
> ---
> No changes to v1.
> Also: see comment on the container side.
> 
>  PVE/QemuMigrate.pm | 7 +--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
>

applied this one already to avoid a re-bump if the storage plugin is going in,
and while that's not the case this does not hurts, it nowhere shows up.
thanks!


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



[pve-devel] applied: [PATCH qemu-server 9/9] api: update vm: correctly handle warnings status for delayed task

2021-06-23 Thread Thomas Lamprecht
On 12.05.21 14:32, Fabian Ebner wrote:
> Signed-off-by: Fabian Ebner 
> ---
>  PVE/API2/Qemu.pm | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
>

applied, thanks!


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



[pve-devel] applied: [PATCH qemu-server 8/9] cli tools: correctly handle warnings task status

2021-06-23 Thread Thomas Lamprecht
On 12.05.21 14:32, Fabian Ebner wrote:
> Signed-off-by: Fabian Ebner 
> ---
> 
> Dependency bump for pve-common is needed.
> 
>  PVE/CLI/qm.pm| 2 +-
>  PVE/CLI/qmrestore.pm | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
>

applied, thanks!


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



Re: [pve-devel] [PATCH v2 container 1/3] migration: fix snapshots boolean accounting

2021-06-23 Thread Fabian Grünbichler
some indication on what was broken would be nice..

AFAICT, storage_migrate fixes this to a boolean anyway, and this is only 
passed to storage_migrate, so.. !?

On June 22, 2021 2:18 pm, Wolfgang Bumiller wrote:
> Signed-off-by: Wolfgang Bumiller 
> ---
> No changes to v1.
> 
>  src/PVE/LXC/Migrate.pm | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/src/PVE/LXC/Migrate.pm b/src/PVE/LXC/Migrate.pm
> index 3cd895d..ce1f7dd 100644
> --- a/src/PVE/LXC/Migrate.pm
> +++ b/src/PVE/LXC/Migrate.pm
> @@ -146,7 +146,7 @@ sub phase1 {
>   }
>  
>   $volhash->{$volid}->{ref} = defined($snapname) ? 'snapshot' : 'config';
> - $volhash->{$volid}->{snapshots} = defined($snapname);
> + $volhash->{$volid}->{snapshots} = 1 if defined($snapname);
>  
>   my ($path, $owner) = PVE::Storage::path($self->{storecfg}, $volid);
>  
> -- 
> 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] applied-series: [PATCH series/flutter 0/4] fixes related to PVE 7

2021-06-23 Thread Thomas Lamprecht
On 23.06.21 12:04, Aaron Lauterer wrote:
> This series contains a few fixes for problems that I came across when
> testing the flutter app against Proxmox VE 7.
> 
> The first bigger one, affecting the first 3 patches, is handling of
> different types when parsing API responses. The PVE API is not always
> returning the expected type. In most situations it is numbers formatted
> as strings instead of numbers.
> 
> For dynamically types languages this is usually much less of a problem
> as it is in statically typed languages as dart.
> To handle that, I introduced new custom serializers for Strings,
> integers and doubles. If the value is not of the expected type, they
> should be able to handle situations where the number is formatted as
> string or where the expected string is formatted as number and
> cast/parse them accordingly.
> 
> The last patch addresses a problem that showed up when trying to open
> the power menu for a guest that had been powered off. The handling of
> the states template value being null has not been done correctly. I used
> the same approach as throughout the other parts where the template
> status is checked -> falling back to false.
> 
> dart_api_client: Aaron Lauterer (3):
>   Add string serializer to handle int and double values
>   Add int serializer to handle string values
>   Add double serializer to handle String and int values
> 
>  lib/src/models/pve_double_serializer.dart | 28 +++
>  lib/src/models/pve_int_serializer.dart| 25 
>  lib/src/models/pve_string_serializer.dart | 25 
>  lib/src/models/serializers.dart   |  6 +
>  4 files changed, 84 insertions(+)
>  create mode 100644 lib/src/models/pve_double_serializer.dart
>  create mode 100644 lib/src/models/pve_int_serializer.dart
>  create mode 100644 lib/src/models/pve_string_serializer.dart
> 
> flutter_frontend: Aaron Lauterer (1):
>   power settings: fix handling null for template status
> 
>  lib/widgets/pve_qemu_power_settings_widget.dart | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> 



applied series, thanks!


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



Re: [pve-devel] [PATCH v10 storage 1/3] factoring out regex'es for backup and vztmpl

2021-06-23 Thread Thomas Lamprecht
On 22.06.21 11:19, Lorenz Stechauner wrote:
> uniformly stores these regex definitions in PVE::Storage.
> 
> One test had to be adapted because it tested obsolete code. Namely:
> it expects vztmpl to only end with .tar.gz, but the new regex also
> includes .tar.xz, there is nothing against allowing .tar.xz files as
> vztmpl files.
> 
> Signed-off-by: Lorenz Stechauner 
> ---
>  PVE/API2/Storage/Status.pm |  6 +++---
>  PVE/Storage.pm | 11 ---
>  PVE/Storage/Plugin.pm  | 15 +--
>  test/path_to_volume_id_test.pm |  7 ---
>  4 files changed, 24 insertions(+), 15 deletions(-)
> 
> diff --git a/PVE/API2/Storage/Status.pm b/PVE/API2/Storage/Status.pm
> index 8a36aef..7069244 100644
> --- a/PVE/API2/Storage/Status.pm
> +++ b/PVE/API2/Storage/Status.pm
> @@ -423,12 +423,12 @@ __PACKAGE__->register_method ({
>  
>   if ($content eq 'iso') {
>   if ($filename !~ m![^/]+$PVE::Storage::iso_extension_re$!) {
> - raise_param_exc({ filename => "missing '.iso' or '.img' 
> extension" });
> + raise_param_exc({ filename => "wrong file extension" });
>   }
>   $path = PVE::Storage::get_iso_dir($cfg, $param->{storage});
>   } elsif ($content eq 'vztmpl') {
> - if ($filename !~ m![^/]+\.tar\.[gx]z$!) {
> - raise_param_exc({ filename => "missing '.tar.gz' or '.tar.xz' 
> extension" });
> + if ($filename !~ m![^/]+$PVE::Storage::vztmpl_extension_re$!) {
> + raise_param_exc({ filename => "wrong file extension" });
>   }
>   $path = PVE::Storage::get_vztmpl_dir($cfg, $param->{storage});
>   } else {
> diff --git a/PVE/Storage.pm b/PVE/Storage.pm
> index ea887a4..519431c 100755
> --- a/PVE/Storage.pm
> +++ b/PVE/Storage.pm
> @@ -101,6 +101,11 @@ PVE::Storage::Plugin->init();
>  
>  our $iso_extension_re = qr/\.(?:iso|img)/i;
>  
> +our $vztmpl_extension_re = qr/\.tar\.([gx]z)/i;
> +
> +# Caution: because of the '$' inside, this regex may only used to match at 
> the end of strings
> +our $backup_extension_re = 
> qr/\.(tgz$|tar|vma)(?:\.(${\PVE::Storage::Plugin::COMPRESSOR_RE}))?/i;

should the $ be at the actual end of the regex, not just for .tgz? I mean, then 
below usage
would need to adapt and it would not be ideal, as it makes fixed choices for 
any user, but
at least not confusing. Otherwise, maybe even preferring that, I'd drop *any* $ 
anchor.

our $backup_extension_re = 
qr/\.(?:(?:(tar|vma)(?:\.(${\PVE::Storage::Plugin::COMPRESSOR_RE}))?|tgz))/i;

But it really needs to be either all $-anchored or none, a mix is just weird 
and confusing,
commented or not.

> +
>  #  PVE::Storage utility functions
>  
>  sub config {
> @@ -573,13 +578,13 @@ sub path_to_volume_id {
>   } elsif ($path =~ m!^$isodir/([^/]+$iso_extension_re)$!) {
>   my $name = $1;
>   return ('iso', "$sid:iso/$name");
> - } elsif ($path =~ m!^$tmpldir/([^/]+\.tar\.gz)$!) {
> + } elsif ($path =~ m!^$tmpldir/([^/]+$vztmpl_extension_re)$!) {
>   my $name = $1;
>   return ('vztmpl', "$sid:vztmpl/$name");
>   } elsif ($path =~ m!^$privatedir/(\d+)$!) {
>   my $vmid = $1;
>   return ('rootdir', "$sid:rootdir/$vmid");
> - } elsif ($path =~ 
> m!^$backupdir/([^/]+\.(?:tgz|(?:(?:tar|vma)(?:\.(?:${\PVE::Storage::Plugin::COMPRESSOR_RE}))?)))$!)
>  {
> + } elsif ($path =~ m!^$backupdir/([^/]+$backup_extension_re)$!) {
>   my $name = $1;
>   return ('backup', "$sid:backup/$name");
>   } elsif ($path =~ m!^$snippetsdir/([^/]+)$!) {
> @@ -1463,7 +1468,7 @@ sub archive_info {
>  my $info;
>  
>  my $volid = basename($archive);
> -if ($volid =~ 
> /^(vzdump-(lxc|openvz|qemu)-.+\.(tgz$|tar|vma)(?:\.(${\PVE::Storage::Plugin::COMPRESSOR_RE}))?)$/)
>  {
> +if ($volid =~ /^(vzdump-(lxc|openvz|qemu)-.+$backup_extension_re)$/) {
>   my $filename = "$1"; # untaint
>   my ($type, $format, $comp) = ($2, $3, $4);
>   my $format_re = defined($comp) ? "$format.$comp" : "$format";
> diff --git a/PVE/Storage/Plugin.pm b/PVE/Storage/Plugin.pm
> index d330845..afb05ed 100644
> --- a/PVE/Storage/Plugin.pm
> +++ b/PVE/Storage/Plugin.pm
> @@ -518,11 +518,11 @@ sub parse_volname {
>   return ('images', $name, $vmid, undef, undef, $isBase, $format);
>  } elsif ($volname =~ m!^iso/([^/]+$PVE::Storage::iso_extension_re)$!) {
>   return ('iso', $1);
> -} elsif ($volname =~ m!^vztmpl/([^/]+\.tar\.[gx]z)$!) {
> +} elsif ($volname =~ 
> m!^vztmpl/([^/]+$PVE::Storage::vztmpl_extension_re)$!) {
>   return ('vztmpl', $1);
>  } elsif ($volname =~ m!^rootdir/(\d+)$!) {
>   return ('rootdir', $1, $1);
> -} elsif ($volname =~ 
> m!^backup/([^/]+(?:\.(?:tgz|(?:(?:tar|vma)(?:\.(?:${\COMPRESSOR_RE}))?$!) 
> {
> +} elsif ($volname =~ 
> m!^backup/([^/]+$PVE::Storage::backup_extension_re)$!) {
>   my $fn = $1;
>   if ($fn =~ m/^vzdump-(openvz|lxc|qemu)-(\d+)-.+/) {
>   re

Re: [pve-devel] [PATCH v2 storage 3/5] btrfs: add 'btrfs' import/export format

2021-06-23 Thread Fabian Grünbichler
On June 22, 2021 2:18 pm, Wolfgang Bumiller wrote:
> Signed-off-by: Wolfgang Bumiller 
> ---
> Changes to v1:
>   * import api parameters reordered du to diff in patch 2/5
> 
>  PVE/CLI/pvesm.pm   |   2 +-
>  PVE/Storage.pm |   2 +-
>  PVE/Storage/BTRFSPlugin.pm | 248 +++--
>  3 files changed, 240 insertions(+), 12 deletions(-)
> 
> diff --git a/PVE/CLI/pvesm.pm b/PVE/CLI/pvesm.pm
> index 4491107..668170a 100755
> --- a/PVE/CLI/pvesm.pm
> +++ b/PVE/CLI/pvesm.pm
> @@ -30,7 +30,7 @@ use PVE::CLIHandler;
>  
>  use base qw(PVE::CLIHandler);
>  
> -my $KNOWN_EXPORT_FORMATS = ['raw+size', 'tar+size', 'qcow2+size', 
> 'vmdk+size', 'zfs'];
> +my $KNOWN_EXPORT_FORMATS = ['raw+size', 'tar+size', 'qcow2+size', 
> 'vmdk+size', 'zfs', 'btrfs'];
>  
>  my $nodename = PVE::INotify::nodename();
>  
> diff --git a/PVE/Storage.pm b/PVE/Storage.pm
> index 5ca052f..c45d339 100755
> --- a/PVE/Storage.pm
> +++ b/PVE/Storage.pm
> @@ -690,7 +690,7 @@ sub storage_migrate {
>  
>  my $migration_snapshot;
>  if (!defined($snapshot)) {
> - if ($scfg->{type} eq 'zfspool') {
> + if ($scfg->{type} eq 'zfspool' || $scfg->{type} eq 'btrfs') {
>   $migration_snapshot = 1;
>   $snapshot = '__migration__';
>   }
> diff --git a/PVE/Storage/BTRFSPlugin.pm b/PVE/Storage/BTRFSPlugin.pm
> index 370d848..072dfe0 100644
> --- a/PVE/Storage/BTRFSPlugin.pm
> +++ b/PVE/Storage/BTRFSPlugin.pm
> @@ -9,8 +9,9 @@ use Fcntl qw(S_ISDIR O_WRONLY O_CREAT O_EXCL);
>  use File::Basename qw(dirname);
>  use File::Path qw(mkpath);
>  use IO::Dir;
> +use POSIX qw(EEXIST);
>  
> -use PVE::Tools qw(run_command);
> +use PVE::Tools qw(run_command dir_glob_foreach);
>  
>  use PVE::Storage::DirPlugin;
>  
> @@ -612,23 +613,250 @@ sub list_images {
>  return $res;
>  }
>  
> -# For now we don't implement `btrfs send/recv` as it needs some updates to 
> our import/export API
> -# first!
> -
>  sub volume_export_formats {
> -return PVE::Storage::DirPlugin::volume_export_formats(@_);
> -}
> +my ($class, $scfg, $storeid, $volname, $snapshot, $base_snapshot, 
> $with_snapshots) = @_;
>  
> -sub volume_export {
> -return PVE::Storage::DirPlugin::volume_export(@_);
> +# We can do whatever `DirPlugin` can do.
> +my @result = PVE::Storage::Plugin::volume_export_formats(@_);
> +
> +# `btrfs send` only works on snapshots:
> +return @result if !defined $snapshot;
> +
> +# Incremental stream with snapshots is only supported if the snapshots 
> are listed (new api):
> +return @result if defined($base_snapshot) && $with_snapshots && 
> ref($with_snapshots) ne 'ARRAY';
> +
> +# Otherwise we do also support `with_snapshots`.
> +
> +# Finally, `btrfs send` only works on formats where we actually use 
> btrfs subvolumes:
> +my $format = ($class->parse_volname($volname))[6];
> +return @result if $format ne 'raw' && $format ne 'subvol';
> +
> +return ('btrfs', @result);
>  }
>  
>  sub volume_import_formats {
> -return PVE::Storage::DirPlugin::volume_import_formats(@_);
> +my ($class, $scfg, $storeid, $volname, $snapshot, $base_snapshot, 
> $with_snapshots) = @_;
> +
> +# Same as export-formats, beware the parameter order:
> +return volume_export_formats(
> + $class,
> + $scfg,
> + $storeid,
> + $volname,
> + $snapshot,
> + $base_snapshot,
> + $with_snapshots,
> +);
> +}
> +
> +sub volume_export {
> +my (
> + $class,
> + $scfg,
> + $storeid,
> + $fh,
> + $volname,
> + $format,
> + $snapshot,
> + $base_snapshot,
> + $with_snapshots,
> +) = @_;
> +
> +if ($format ne 'btrfs') {
> + return PVE::Storage::Plugin::volume_export(@_);
> +}
> +
> +die "format 'btrfs' only works on snapshots\n"
> + if !defined $snapshot;
> +
> +die "'btrfs' format in incremental mode requires snapshots to be listed 
> explicitly\n"
> + if defined($base_snapshot) && $with_snapshots && ref($with_snapshots) 
> ne 'ARRAY';
> +
> +my $volume_format = ($class->parse_volname($volname))[6];
> +
> +die "btrfs-sending volumes of type $volume_format ('$volname') is not 
> supported\n"
> + if $volume_format ne 'raw' && $volume_format ne 'subvol';

the three checks here are the same as in volume_export, maybe they could 
go into a single export-helper?

> +
> +my $path = $class->path($scfg, $volname, $storeid);
> +
> +if ($volume_format eq 'raw') {
> + $path = raw_file_to_subvol($path);
> +}
> +
> +my $cmd = ['btrfs', '-q', 'send', '-e'];
> +if ($base_snapshot) {
> + my $base = $class->path($scfg, $volname, $storeid, $base_snapshot);
> + if ($volume_format eq 'raw') {
> + $base = raw_file_to_subvol($base);
> + }
> + push @$cmd, '-p', $base;
> +}
> +push @$cmd, '--';
> +if (ref($with_snapshots) eq 'ARRAY') {
> + push @$cmd, (map { "$path\@$_" } ($with_snapshots // [])->@*), $path;
> +} els

Re: [pve-devel] [PATCH v2 storage 1/5] add BTRFS storage plugin

2021-06-23 Thread Fabian Grünbichler
On June 22, 2021 2:18 pm, Wolfgang Bumiller wrote:
> This is mostly the same as a directory storage, with 2 major
> differences:
> 
> * 'subvol' volumes are actual btrfs subvolumes and therefore
>   allow snapshots
> * 'raw' files are placed *into* a subvolume and therefore
>   also allow snapshots, the raw file for volume
>   `btrstore:100/vm-100-disk-1.raw` can be found under
>   `$path/images/100/vm-100-disk-1/disk.raw`
> * in both cases, snapshots add an '@name' suffix to the
>   subvolume's directory name, so snapshot 'foo' of the above
>   would be found under
>   `$path/images/100/vm-100-disk-1@foo/disk.raw`
>   or for format "subvol":
>   `$path/images/100/subvol-100-disk-1.subvol@foo`
> 
> Note that qgroups aren't included in btrfs-send streams,
> therefore for now we will only be using *unsized* subvolumes
> for containers and place a regular raw+ext4 file for sized
> containers.
> We could extend the import/export stream format to include
> the information at the front (similar to how we do the
> "tar+size" format, but we need to include the size of all
> the contained snapshots as well, since they can technically
> change). (But before enabling quotas we should do some
> performance testing on bigger file systems with multiple
> snapshots as there are quite a few reports of the fs slowing
> down considerably in such scenarios).
> 
> Signed-off-by: Wolfgang Bumiller 
> ---
> Changes to v1:
>   raw files are created manually with sysopen/truncate and
>   ioctls for setting the NOCOW bit have been added (this
>   are made optional later in the series)
> 
> 
>  PVE/Storage.pm |   2 +
>  PVE/Storage/BTRFSPlugin.pm | 634 +
>  PVE/Storage/Makefile   |   1 +
>  3 files changed, 637 insertions(+)
>  create mode 100644 PVE/Storage/BTRFSPlugin.pm
> 
> diff --git a/PVE/Storage.pm b/PVE/Storage.pm
> index ea887a4..e0d6f44 100755
> --- a/PVE/Storage.pm
> +++ b/PVE/Storage.pm
> @@ -38,6 +38,7 @@ use PVE::Storage::GlusterfsPlugin;
>  use PVE::Storage::ZFSPoolPlugin;
>  use PVE::Storage::ZFSPlugin;
>  use PVE::Storage::PBSPlugin;
> +use PVE::Storage::BTRFSPlugin;
>  
>  # Storage API version. Increment it on changes in storage API interface.
>  use constant APIVER => 8;
> @@ -60,6 +61,7 @@ PVE::Storage::GlusterfsPlugin->register();
>  PVE::Storage::ZFSPoolPlugin->register();
>  PVE::Storage::ZFSPlugin->register();
>  PVE::Storage::PBSPlugin->register();
> +PVE::Storage::BTRFSPlugin->register();
>  
>  # load third-party plugins
>  if ( -d '/usr/share/perl5/PVE/Storage/Custom' ) {
> diff --git a/PVE/Storage/BTRFSPlugin.pm b/PVE/Storage/BTRFSPlugin.pm
> new file mode 100644
> index 000..370d848
> --- /dev/null
> +++ b/PVE/Storage/BTRFSPlugin.pm
> @@ -0,0 +1,634 @@
> +package PVE::Storage::BTRFSPlugin;
> +
> +use strict;
> +use warnings;
> +
> +use base qw(PVE::Storage::Plugin);
> +
> +use Fcntl qw(S_ISDIR O_WRONLY O_CREAT O_EXCL);
> +use File::Basename qw(dirname);
> +use File::Path qw(mkpath);
> +use IO::Dir;
> +
> +use PVE::Tools qw(run_command);
> +
> +use PVE::Storage::DirPlugin;
> +
> +use constant {
> +BTRFS_FIRST_FREE_OBJECTID => 256,
> +FS_NOCOW_FL => 0x0080,
> +FS_IOC_GETFLAGS => 0x40086602,
> +FS_IOC_SETFLAGS => 0x80086601,
> +};
> +
> +# Configuration (similar to DirPlugin)
> +
> +sub type {
> +return 'btrfs';
> +}
> +
> +sub plugindata {
> +return {
> + content => [
> + {
> + images => 1,
> + rootdir => 1,
> + vztmpl => 1,
> + iso => 1,
> + backup => 1,
> + snippets => 1,
> + none => 1,
> + },
> + { images => 1, rootdir => 1 },
> + ],
> + format => [ { raw => 1, qcow2 => 1, vmdk => 1, subvol => 1 }, 'raw', ],

I am not really convinced we need this (or rather, that it doesn't 
cause more confusion than convenience. if I want to use btrfs without 
any btrfs features, I can already use any btrfs-backed dir as dir 
storage (just like with ZFS, or $storage-of-my-choice-providing-a-directory).

e.g., having a qcow2 file on btrfs storage support snapshots but not use 
btrfs functionality for that seems... weird?

> +};
> +}
> +
> +sub options {
> +return {
> + path => { fixed => 1 },
> + nodes => { optional => 1 },
> + shared => { optional => 1 },
> + disable => { optional => 1 },
> + maxfiles => { optional => 1 },
> + content => { optional => 1 },
> + format => { optional => 1 },
> + is_mountpoint => { optional => 1 },
> + # TODO: The new variant of mkdir with  `populate` vs `create`...
> +};
> +}
> +
> +# Storage implementation
> +#
> +# We use the same volume names are directory plugins, but map *raw* disk 
> image file names into a
> +# subdirectory.
> +#
> +# `vm-VMID-disk-ID.raw`
> +#   -> `images/VMID/vm-VMID-disk-ID/disk.raw`
> +#   where the `vm-VMID-disk-ID/` subdirectory is a btrfs subvolume
> +
> +# Reuse `DirPlugin`'s `check_config`. This simply c

Re: [pve-devel] [PATCH v2 storage 3/5] btrfs: add 'btrfs' import/export format

2021-06-23 Thread Wolfgang Bumiller
On Wed, Jun 23, 2021 at 02:14:48PM +0200, Fabian Grünbichler wrote:
> On June 22, 2021 2:18 pm, Wolfgang Bumiller wrote:
> > Signed-off-by: Wolfgang Bumiller 

(...)

> > +sub volume_export {
> > +my (
> > +   $class,
> > +   $scfg,
> > +   $storeid,
> > +   $fh,
> > +   $volname,
> > +   $format,
> > +   $snapshot,
> > +   $base_snapshot,
> > +   $with_snapshots,
> > +) = @_;
> > +
> > +if ($format ne 'btrfs') {
> > +   return PVE::Storage::Plugin::volume_export(@_);
> > +}
> > +
> > +die "format 'btrfs' only works on snapshots\n"
> > +   if !defined $snapshot;
> > +
> > +die "'btrfs' format in incremental mode requires snapshots to be 
> > listed explicitly\n"
> > +   if defined($base_snapshot) && $with_snapshots && ref($with_snapshots) 
> > ne 'ARRAY';
> > +
> > +my $volume_format = ($class->parse_volname($volname))[6];
> > +
> > +die "btrfs-sending volumes of type $volume_format ('$volname') is not 
> > supported\n"
> > +   if $volume_format ne 'raw' && $volume_format ne 'subvol';
> 
> the three checks here are the same as in volume_export, maybe they could 
> go into a single export-helper?

we get nicer errors this way, though (and more easily placed in the code
when a user runs into them, since we don't use Carp ;-) )

(...)

> > +
> > +   # Now go through the remaining snapshots (if any)
> > +   foreach my $snap (@snapshots) {
> > +   $class->btrfs_cmd(['property', 'set', "$tmppath/$diskname\@$snap", 
> > 'ro', 'false']);
> > +   PVE::Tools::renameat2(
> > +   -1,
> > +   "$tmppath/$diskname\@$snap",
> > +   -1,
> > +   "$destination\@$snap",
> > +   &PVE::Tools::RENAME_NOREPLACE,
> > +   ) or die "failed to move received snapshot 
> > '$tmppath/$diskname\@$snap'"
> > +   . " into place at '$destination\@$snap' - $!\n";
> > +   eval { $class->btrfs_cmd(['property', 'set', "$destination\@$snap", 
> > 'ro', 'true']) };
> > +   warn "failed to make $destination\@$snap read-only - $!\n" if $@;
> > +   }
> > +};
> > +my $err = $@;
> > +
> > +eval {
> > +   # Cleanup all the received snapshots we did not move into place, so we 
> > can remove the temp
> > +   # directory.
> > +   if ($dh) {
> > +   $dh->rewind;
> > +   while (defined(my $entry = $dh->read)) {
> > +   next if $entry eq '.' || $entry eq '..';
> > +   eval { $class->btrfs_cmd(['subvolume', 'delete', '--', 
> > "$tmppath/$entry"]) };
> > +   warn $@ if $@;
> > +   }
> > +   $dh->close; undef $dh;
> > +   }
> > +   if (!rmdir($tmppath)) {
> > +   warn "failed to remove temporary directory '$tmppath' - $!\n"
> > +   }
> > +};
> > +warn $@ if $@;
> > +if ($err) {
> > +   # clean up if the directory ended up being empty after an error
> > +   rmdir($tmppath);
> > +   die $err;
> > +}
> 
> I am a bit wary about this (also w.r.t. future btrfs features) - a 
> privileged container can do quite a lot of stuff with btrfs
> 
> - create snapshots from within container
> - set default subvolume
> - set properties
> - ... (the above is what I found in a few minutes without much BTRFS 
>   experience)
> 
> unprivileged containers can do the following
> - create new subvols
> - create new snapshots
> - set ro property
> 
> (same caveat of not looking too much at what else is possible, lots of 
> stuff does not work)
> 
> how do we guarantee that the send stream is not affected in a dangerous 
> fashion by an attacker inside the container?

Shouldn't be dangerous sine it's not recursive anyway.

However, we could just by default disable the corresponding ioctls via
seccomp for unprivileged containers...


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



Re: [pve-devel] [PATCH v2 container 1/3] migration: fix snapshots boolean accounting

2021-06-23 Thread Wolfgang Bumiller
On Wed, Jun 23, 2021 at 12:59:58PM +0200, Fabian Grünbichler wrote:
> some indication on what was broken would be nice..
> 
> AFAICT, storage_migrate fixes this to a boolean anyway, and this is only 
> passed to storage_migrate, so.. !?

The type is not the problem. It's assigned - as in overwritten - in each
check, and it's called last for the current state, so this was always
set to false.

> 
> On June 22, 2021 2:18 pm, Wolfgang Bumiller wrote:
> > Signed-off-by: Wolfgang Bumiller 
> > ---
> > No changes to v1.
> > 
> >  src/PVE/LXC/Migrate.pm | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/src/PVE/LXC/Migrate.pm b/src/PVE/LXC/Migrate.pm
> > index 3cd895d..ce1f7dd 100644
> > --- a/src/PVE/LXC/Migrate.pm
> > +++ b/src/PVE/LXC/Migrate.pm
> > @@ -146,7 +146,7 @@ sub phase1 {
> > }
> >  
> > $volhash->{$volid}->{ref} = defined($snapname) ? 'snapshot' : 'config';
> > -   $volhash->{$volid}->{snapshots} = defined($snapname);
> > +   $volhash->{$volid}->{snapshots} = 1 if defined($snapname);
> >  
> > my ($path, $owner) = PVE::Storage::path($self->{storecfg}, $volid);
> >  
> > -- 
> > 2.30.2


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



Re: [pve-devel] [PATCH v2 storage 1/5] add BTRFS storage plugin

2021-06-23 Thread Wolfgang Bumiller
On Wed, Jun 23, 2021 at 02:15:00PM +0200, Fabian Grünbichler wrote:
> On June 22, 2021 2:18 pm, Wolfgang Bumiller wrote:
(...)
> > +sub plugindata {
> > +return {
> > +   content => [
> > +   {
> > +   images => 1,
> > +   rootdir => 1,
> > +   vztmpl => 1,
> > +   iso => 1,
> > +   backup => 1,
> > +   snippets => 1,
> > +   none => 1,
> > +   },
> > +   { images => 1, rootdir => 1 },
> > +   ],
> > +   format => [ { raw => 1, qcow2 => 1, vmdk => 1, subvol => 1 }, 'raw', ],
> 
> I am not really convinced we need this (or rather, that it doesn't 
> cause more confusion than convenience. if I want to use btrfs without 
> any btrfs features, I can already use any btrfs-backed dir as dir 
> storage (just like with ZFS, or $storage-of-my-choice-providing-a-directory).
> 
> e.g., having a qcow2 file on btrfs storage support snapshots but not use 
> btrfs functionality for that seems... weird?

It's a directory storage after all.
But I really don't mind much if we just remove that...

> 
> > +};
> > +}
> > +
> > +sub options {
> > +return {
> > +   path => { fixed => 1 },
> > +   nodes => { optional => 1 },
> > +   shared => { optional => 1 },
> > +   disable => { optional => 1 },
> > +   maxfiles => { optional => 1 },
> > +   content => { optional => 1 },
> > +   format => { optional => 1 },
> > +   is_mountpoint => { optional => 1 },
> > +   # TODO: The new variant of mkdir with  `populate` vs `create`...
> > +};
> > +}
> > +
> > +# Storage implementation
> > +#
> > +# We use the same volume names are directory plugins, but map *raw* disk 
> > image file names into a
> > +# subdirectory.
> > +#
> > +# `vm-VMID-disk-ID.raw`
> > +#   -> `images/VMID/vm-VMID-disk-ID/disk.raw`
> > +#   where the `vm-VMID-disk-ID/` subdirectory is a btrfs subvolume
> > +
> > +# Reuse `DirPlugin`'s `check_config`. This simply checks for invalid paths.
> > +sub check_config {
> > +my ($self, $sectionId, $config, $create, $skipSchemaCheck) = @_;
> > +return PVE::Storage::DirPlugin::check_config($self, $sectionId, 
> > $config, $create, $skipSchemaCheck);
> > +}
> > +
> > +sub activate_storage {
> > +my ($class, $storeid, $scfg, $cache) = @_;
> > +return PVE::Storage::DirPlugin::activate_storage($class, $storeid, 
> > $scfg, $cache);
> 
> shouldn't this check that we can actually do btrfs stuff on $path?

Right. I had a method for this at some point but hadn't used it here.
Should be easily added in a follow up.

> 
> > +}
> > +
> > +sub status {
> > +my ($class, $storeid, $scfg, $cache) = @_;
> > +return PVE::Storage::DirPlugin::status($class, $storeid, $scfg, 
> > $cache);
> > +}
> > +
> > +# TODO: sub get_volume_notes {}
> > +
> > +# TODO: sub update_volume_notes {}
> > +
> > +# croak would not include the caller from within this module
> > +sub __error {
> > +my ($msg) = @_;
> > +my (undef, $f, $n) = caller(1);
> > +die "$msg at $f: $n\n";
> > +}
> 
> this is kind of a new style of error handling - might be worthy of a 
> mention somewhere ;)

debug helper... shouldn't be possible to reach this... I should probably
prefix the 2 subs below with a `my` though...
(I mostly ran into such stuff while playing with different ways of
naming/organising snapshot directories and not seeing *where* stuff
happened was annoying...)

> 
> > +
> > +# Given a name (eg. `vm-VMID-disk-ID.raw`), take the part up to the format 
> > suffix as the name of
> > +# the subdirectory (subvolume).
> > +sub raw_name_to_dir($) {
> > +my ($raw) = @_;
> > +
> > +# For the subvolume directory Strip the `.` suffix:
> > +if ($raw =~ /^(.*)\.raw$/) {
> > +   return $1;
> > +}
> > +
> > +__error "internal error: bad disk name: $raw";
> > +}
> > +
> > +sub raw_file_to_subvol($) {
> > +my ($file) = @_;
> > +
> > +if ($file =~ m|^(.*)/disk\.raw$|) {
> > +   return "$1";
> > +}
> > +
> > +__error "internal error: bad raw path: $file";
> > +}
> > +
(...)
> > +
> > +sub btrfs_get_subvol_id {
> > +my ($class, $path) = @_;
> > +my $info = $class->btrfs_cmd(['subvolume', 'show', '--', $path]);
> > +if ($info !~ /^\s*(?:Object|Subvolume) ID:\s*(\d+)$/m) {
> > +   die "failed to get btrfs subvolume ID from: $info\n";
> 
> ugh at yet another parser for CLI tool output.. (not your fault I 
> know..)

This one I can actually replace. I just am not sure if I want to do it
all in pure perl or add some btrfs helper to the emerging `pve-rs` lib,
and since this code already existed in the old series, I postponed that
decision.


(...)
> > +my $imagedir = $class->get_subdir($scfg, 'images');
> > +$imagedir .= "/$vmid";
> > +mkpath $imagedir;
> > +
> > +my $path = $class->filesystem_path($scfg, $volname);
> > +my $newname = $class->find_free_diskname($storeid, $scfg, $vmid, 
> > $format, 1);
> > +
> > +# For btrfs subvolumes we don't actually need the "link":
> > +#my $newvolname = "$basevmid/$bas

[pve-devel] [RFC docs] add a basic BTRFS section

2021-06-23 Thread Wolfgang Bumiller
Signed-off-by: Wolfgang Bumiller 
---
Please give some feedback as to what else to add/change/remove.

 local-btrfs.adoc   | 177 +
 pve-storage-btrfs.adoc |  55 +
 pvesm.adoc |   1 +
 sysadmin.adoc  |   2 +
 4 files changed, 235 insertions(+)
 create mode 100644 local-btrfs.adoc
 create mode 100644 pve-storage-btrfs.adoc

diff --git a/local-btrfs.adoc b/local-btrfs.adoc
new file mode 100644
index 000..e9b0c34
--- /dev/null
+++ b/local-btrfs.adoc
@@ -0,0 +1,177 @@
+[[chapter_btrfs]]
+BTRFS
+-
+ifdef::wiki[]
+:pve-toplevel:
+endif::wiki[]
+
+BTRFS is a modern copy on write file system natively supported by the Linux
+kernel, implementing features such as snapshots, built-in RAID and self healing
+via checksums for data and metadata. Starting with {pve} 7.0, BTRFS is
+introduced as optional selection for the root file system.
+
+.General BTRFS advantages
+
+* Main system setup almost identical to the traditional ext4 based setup
+
+* Snapshots
+
+* Data compression on file system level
+
+* Copy-on-write clone
+
+* RAID0, RAID1 and RAID10
+
+* Protection against data corruption
+
+* Self healing
+
+* natively supported by the Linux kernel
+
+* ...
+
+.Caveats
+
+* RAID levels 5/6 are experimental and dangerous
+
+Installation as Root File System
+
+
+When you install using the {pve} installer, you can choose BTRFS for the root
+file system. You need to select the RAID type at installation time:
+
+[horizontal]
+RAID0:: Also called ``striping''. The capacity of such volume is the sum
+of the capacities of all disks. But RAID0 does not add any redundancy,
+so the failure of a single drive makes the volume unusable.
+
+RAID1:: Also called ``mirroring''. Data is written identically to all
+disks. This mode requires at least 2 disks with the same size. The
+resulting capacity is that of a single disk.
+
+RAID10:: A combination of RAID0 and RAID1. Requires at least 4 disks.
+
+The installer automatically partitions the disks and creates an additional
+subvolume at `/var/lib/pve/local-btrfs`.  In order to use that with the {pve}
+tools, the installer creates the following configuration entry in
+`/etc/pve/storage.cfg`:
+
+
+dir: local
+   path /var/lib/vz
+   content iso,vztmpl,backup
+   disable
+
+btrfs: local-btrfs
+   path /var/lib/pve/local-btrfs
+   content iso,vztmpl,backup,images,rootdir
+
+
+This explicitly disables the default `local` storage in favor of a btrfs
+specific storage entry on the additional subvolume.
+
+The `btrfs` command is used to configure and manage the btrfs file system,
+After the installation, the following command lists all additional subvolumes:
+
+
+# btrfs subvolume list /
+ID 256 gen 6 top level 5 path var/lib/pve/local-btrfs
+
+
+BTRFS Administration
+
+
+This section gives you some usage examples for common tasks.
+
+Creating a BTRFS file system
+
+
+To create BTRFS file systems, `mkfs.btrfs` is used. The `-d` and `-m` 
parameters
+are used to set the profile for metadata and data respectively. With the
+optional `-L` parameter, a label can be set.
+
+Generally, the following modes are supported: `single`, `raid0`, `raid1`,
+`raid10`.
+
+Create a BTRFS file system on `/dev/sdb1`
+
+
+ # mkfs.btrfs -m single -d single -L My-Storage /dev/sdb1
+
+
+Or create a RAID1 on `/dev/sdb1` and `/dev/sdc1`
+
+
+ # mkfs.btrfs -m raid1 -d raid1 -L My-Storage /dev/sdb1 /dev/sdc1
+
+
+This can then be mounted or used in `/etc/fstab` like any other mount point.
+
+For example
+
+
+ # mkdir /my-storage
+ # mount /dev/sdb1 /my-storage
+
+
+Creating a subvolume
+
+
+Creating a subvolume links it to a path in the btrfs file system, where it will
+appear as a regular directory.
+
+
+# btrfs subvolume create /some/path
+
+
+Afterwards `/some/path` will act like a regular directory.
+
+Deleting a subvolume
+
+
+Contrary to directories removed via `rmdir`, subvolumes do not need to be empty
+in order to be deleted via the `btrfs` command.
+
+
+# btrfs subvolume delete /some/path
+
+
+Creating a snapshot of a subvolume
+^^
+
+BTRFS does not actually distinguish between snapshots and normal subvolumes, so
+taking a snapshot can also be seen as creating an arbitrary copy of a 
subvolume.
+By convention, {pve} will use the read-only flag when creating snapshots of
+guest disks or subvolumes, but this flag can also be changed later on.
+
+
+# btrfs subvolume snapshot -r /some/path /a/new/path
+
+
+This will create a read-only "clone" of the subvolume on `/some/path` at
+`/a/new/path`. Any future modifications to `/some/path` cause the modified data
+to be copied before modification.
+
+If the read-only (`-r`) option is left out, both subvolumes will be writable.
+
+Enabling compression
+^^^

[pve-devel] [PATCH storage] factoring out regex for vztmpl

2021-06-23 Thread Lorenz Stechauner
stores the regex definition in PVE::Storage.

One test had to be adapted because it tested obsolete code. Namely:
it expects vztmpl to only end with .tar.gz, but the new regex also
includes .tar.xz, there is nothing against allowing .tar.xz files as
vztmpl files.

Signed-off-by: Lorenz Stechauner 
---
updates "[PATCH v10 storage 1/3] factoring out regex'es for backup and vztmpl"
changes:
* only factor vztmpl_extension_re out

 PVE/API2/Storage/Status.pm | 6 +++---
 PVE/Storage.pm | 4 +++-
 PVE/Storage/Plugin.pm  | 4 ++--
 test/path_to_volume_id_test.pm | 7 ---
 4 files changed, 12 insertions(+), 9 deletions(-)

diff --git a/PVE/API2/Storage/Status.pm b/PVE/API2/Storage/Status.pm
index 8a36aef..7069244 100644
--- a/PVE/API2/Storage/Status.pm
+++ b/PVE/API2/Storage/Status.pm
@@ -423,12 +423,12 @@ __PACKAGE__->register_method ({
 
if ($content eq 'iso') {
if ($filename !~ m![^/]+$PVE::Storage::iso_extension_re$!) {
-   raise_param_exc({ filename => "missing '.iso' or '.img' 
extension" });
+   raise_param_exc({ filename => "wrong file extension" });
}
$path = PVE::Storage::get_iso_dir($cfg, $param->{storage});
} elsif ($content eq 'vztmpl') {
-   if ($filename !~ m![^/]+\.tar\.[gx]z$!) {
-   raise_param_exc({ filename => "missing '.tar.gz' or '.tar.xz' 
extension" });
+   if ($filename !~ m![^/]+$PVE::Storage::vztmpl_extension_re$!) {
+   raise_param_exc({ filename => "wrong file extension" });
}
$path = PVE::Storage::get_vztmpl_dir($cfg, $param->{storage});
} else {
diff --git a/PVE/Storage.pm b/PVE/Storage.pm
index ea887a4..60cd19e 100755
--- a/PVE/Storage.pm
+++ b/PVE/Storage.pm
@@ -101,6 +101,8 @@ PVE::Storage::Plugin->init();
 
 our $iso_extension_re = qr/\.(?:iso|img)/i;
 
+our $vztmpl_extension_re = qr/\.tar\.([gx]z)/i;
+
 #  PVE::Storage utility functions
 
 sub config {
@@ -573,7 +575,7 @@ sub path_to_volume_id {
} elsif ($path =~ m!^$isodir/([^/]+$iso_extension_re)$!) {
my $name = $1;
return ('iso', "$sid:iso/$name");
-   } elsif ($path =~ m!^$tmpldir/([^/]+\.tar\.gz)$!) {
+   } elsif ($path =~ m!^$tmpldir/([^/]+$vztmpl_extension_re)$!) {
my $name = $1;
return ('vztmpl', "$sid:vztmpl/$name");
} elsif ($path =~ m!^$privatedir/(\d+)$!) {
diff --git a/PVE/Storage/Plugin.pm b/PVE/Storage/Plugin.pm
index d330845..fe6365c 100644
--- a/PVE/Storage/Plugin.pm
+++ b/PVE/Storage/Plugin.pm
@@ -518,7 +518,7 @@ sub parse_volname {
return ('images', $name, $vmid, undef, undef, $isBase, $format);
 } elsif ($volname =~ m!^iso/([^/]+$PVE::Storage::iso_extension_re)$!) {
return ('iso', $1);
-} elsif ($volname =~ m!^vztmpl/([^/]+\.tar\.[gx]z)$!) {
+} elsif ($volname =~ 
m!^vztmpl/([^/]+$PVE::Storage::vztmpl_extension_re)$!) {
return ('vztmpl', $1);
 } elsif ($volname =~ m!^rootdir/(\d+)$!) {
return ('rootdir', $1, $1);
@@ -1041,7 +1041,7 @@ my $get_subdir_files = sub {
$info = { volid => "$sid:iso/$1", format => 'iso' };
 
} elsif ($tt eq 'vztmpl') {
-   next if $fn !~ m!/([^/]+\.tar\.([gx]z))$!;
+   next if $fn !~ m!/([^/]+$PVE::Storage::vztmpl_extension_re)$!;
 
$info = { volid => "$sid:vztmpl/$1", format => "t$2" };
 
diff --git a/test/path_to_volume_id_test.pm b/test/path_to_volume_id_test.pm
index 5eee2f6..e0c46d9 100644
--- a/test/path_to_volume_id_test.pm
+++ b/test/path_to_volume_id_test.pm
@@ -166,12 +166,13 @@ my @tests = (
'local:snippets/hookscript.pl',
],
 },
-
-# no matches
 {
description => 'CT template, tar.xz',
volname => 
"$storage_dir/template/cache/debian-10.0-standard_10.0-1_amd64.tar.xz",
-   expected=> [''],
+   expected=> [
+   'vztmpl',
+   'local:vztmpl/debian-10.0-standard_10.0-1_amd64.tar.xz',
+   ],
 },
 
 # no matches, path or files with failures
-- 
2.30.2



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



Re: [pve-devel] applied: [PATCH storage] plugins: untaint volume_size_info retuns

2021-06-23 Thread Stoiko Ivanov
On Wed, 23 Jun 2021 09:13:40 +0200
Thomas Lamprecht  wrote:

> On 22.06.21 18:39, Stoiko Ivanov wrote:
> > the size returned by volume_size_info is used for creating the new
> > destination image in PVE::QemuServer::clone_disk (and probably
> > elsewhere). In certain cases the return values are tainted - they are
> > obtained by a run_command call and depending on the format and length
> > of the parsed output can still have their tainted attribute.
> > 
> > One example of a tainted return has been reported in our
> > community-forum:
> > https://forum.proxmox.com/threads/cannot-clone-vm-or-move-disk-with-more-than-13-snapshots.89628/
> > 
> > A qcow2 image with 13 snapshots generates a output > 4k in length from
> > `qemu-img info --output=json`, which in turn causes the output to be
> > considered tainted.
> > 
> > This patch untaints the returns where applicable. The other
> > storage-plugins are not affected:
> > * LVMPlugin returns a single number and a newline (thus gets untainted
> >   by run_command)
> > * RBDPlugin untaints the complete json before decoding
> > * ZFSPoolplugin and ISCSIDirectPlugin explicitly untaint their
> >   returns.
> > 
> > Signed-off-by: Stoiko Ivanov 
> > ---
> > 
> > Note:
> > Not really a v2, since it's a different patch, but addresses the same issue
> > as in https://lists.proxmox.com/pipermail/pve-devel/2021-June/048910.html  
> 
> Aren't the version rather tied to the issue they try to solve, as 
> implementations
> and approaches can always significantly change? So I'd be OK with this being
> marked as v2, but no hard feelings.
> 
> > 
> >  PVE/Storage/PBSPlugin.pm | 4 +++-
> >  PVE/Storage/Plugin.pm| 6 ++
> >  2 files changed, 9 insertions(+), 1 deletion(-)
> > 
> >  
> 
> applied to master and a newly branched stable-6, thanks!
> 
> I made two followups:
> 
> 1) - fix nit that we have a space in comments after the #
>- actually die with error message if untaint fails
> 
> 2) return early if decode JSON fails, compensates issues where the qemu-img
>command somehow fails (e.g., file was removed since the stat check) and
>the semantic change from 1)
> 
> Please check if this still looks alright to you.

Thanks for the improvements - Look fine to me! (and survived a few quick
tests of disk-moving)


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



[pve-devel] [PATCH v7 proxmox-apt 5/5] bump version to 0.2.0-1

2021-06-23 Thread Fabian Ebner
Signed-off-by: Fabian Ebner 
---

Changes from v6:
* different changes in the changelog

 Cargo.toml   | 2 +-
 debian/changelog | 8 
 debian/control   | 8 
 3 files changed, 13 insertions(+), 5 deletions(-)

diff --git a/Cargo.toml b/Cargo.toml
index 24f734b..9bed970 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -1,6 +1,6 @@
 [package]
 name = "proxmox-apt"
-version = "0.1.0"
+version = "0.2.0"
 authors = [
 "Fabian Ebner ",
 "Proxmox Support Team ",
diff --git a/debian/changelog b/debian/changelog
index 11e26ed..21a429e 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,11 @@
+rust-proxmox-apt (0.2.0-1) unstable; urgency=medium
+
+  * Add functions to check repositories.
+
+  * Add handling of standard Proxmox repositories.
+
+ -- Proxmox Support Team   Wed, 23 Jun 2021 14:57:52 +0200
+
 rust-proxmox-apt (0.1.0-1) unstable; urgency=medium
 
   * Initial release.
diff --git a/debian/control b/debian/control
index e0938bf..99ac284 100644
--- a/debian/control
+++ b/debian/control
@@ -34,10 +34,10 @@ Provides:
  librust-proxmox-apt+default-dev (= ${binary:Version}),
  librust-proxmox-apt-0-dev (= ${binary:Version}),
  librust-proxmox-apt-0+default-dev (= ${binary:Version}),
- librust-proxmox-apt-0.1-dev (= ${binary:Version}),
- librust-proxmox-apt-0.1+default-dev (= ${binary:Version}),
- librust-proxmox-apt-0.1.0-dev (= ${binary:Version}),
- librust-proxmox-apt-0.1.0+default-dev (= ${binary:Version})
+ librust-proxmox-apt-0.2-dev (= ${binary:Version}),
+ librust-proxmox-apt-0.2+default-dev (= ${binary:Version}),
+ librust-proxmox-apt-0.2.0-dev (= ${binary:Version}),
+ librust-proxmox-apt-0.2.0+default-dev (= ${binary:Version})
 Description: Proxmox library for APT - Rust source code
  This package contains the source for the Rust proxmox-apt crate, packaged by
  debcargo for use with cargo and dh-cargo.
-- 
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 v7 proxmox-apt 2/5] add files for Debian packaging

2021-06-23 Thread Fabian Ebner
The Makefile is based on the one from Mira's conntrack series, as it already got
some review.

Signed-off-by: Fabian Ebner 
---

Changes from v6:
* (automatically) update debian/control for bullseye

 .gitignore   |  5 
 Makefile | 67 
 debian/changelog |  5 
 debian/control   | 43 
 debian/copyright | 16 +++
 debian/debcargo.toml |  7 +
 6 files changed, 143 insertions(+)
 create mode 100644 Makefile
 create mode 100644 debian/changelog
 create mode 100644 debian/control
 create mode 100644 debian/copyright
 create mode 100644 debian/debcargo.toml

diff --git a/.gitignore b/.gitignore
index 24917d4..db6f13e 100644
--- a/.gitignore
+++ b/.gitignore
@@ -2,3 +2,8 @@ Cargo.lock
 target/
 tests/sources.list.d.actual
 tests/sources.list.d.digest
+proxmox-apt-*/
+*proxmox-apt*.buildinfo
+*proxmox-apt*.tar.?z
+*proxmox-apt*.changes
+*proxmox-apt*.deb
diff --git a/Makefile b/Makefile
new file mode 100644
index 000..e7f0725
--- /dev/null
+++ b/Makefile
@@ -0,0 +1,67 @@
+include /usr/share/dpkg/pkg-info.mk
+include /usr/share/dpkg/architecture.mk
+
+PACKAGE=proxmox-apt
+BUILDDIR ?= $(PACKAGE)-$(DEB_VERSION_UPSTREAM)
+BUILDDIR_TMP ?= $(BUILDDIR).tmp
+
+DEB=librust-$(PACKAGE)-dev_$(DEB_VERSION_UPSTREAM_REVISION)_$(DEB_BUILD_ARCH).deb
+DSC=rust-$(PACKAGE)_$(DEB_VERSION_UPSTREAM_REVISION).dsc
+
+ifeq ($(BUILD_MODE), release)
+CARGO_BUILD_ARGS += --release
+COMPILEDIR := target/release
+else
+COMPILEDIR := target/debug
+endif
+
+all: cargo-build $(SUBDIRS)
+
+.PHONY: cargo-build
+cargo-build:
+   cargo build $(CARGO_BUILD_ARGS)
+
+.PHONY: build
+build:
+   rm -rf $(BUILDDIR) $(BUILDDIR_TMP); mkdir $(BUILDDIR_TMP)
+   rm -f debian/control
+   debcargo package \
+   --config debian/debcargo.toml \
+   --changelog-ready \
+   --no-overlay-write-back \
+   --directory $(BUILDDIR_TMP) \
+   $(PACKAGE) \
+   $(shell dpkg-parsechangelog -l debian/changelog -SVersion | sed -e 
's/-.*//')
+   cp $(BUILDDIR_TMP)/debian/control debian/control
+   rm -f $(BUILDDIR_TMP)/Cargo.lock
+   find $(BUILDDIR_TMP)/debian -name "*.hint" -delete
+   mv $(BUILDDIR_TMP) $(BUILDDIR)
+
+.PHONY: deb
+deb: $(DEB)
+$(DEB): build
+   cd $(BUILDDIR); dpkg-buildpackage -b -us -uc --no-pre-clean
+   lintian $(DEB)
+
+.PHONY: dsc
+dsc: $(DSC)
+$(DSC): build
+   cd $(BUILDDIR); dpkg-buildpackage -S -us -uc -d -nc
+   lintian $(DSC)
+
+.PHONY: dinstall
+dinstall: $(DEB)
+   dpkg -i $(DEB)
+
+.PHONY: upload
+upload: $(DEB) $(DBG_DEB)
+   tar cf - $(DEB) $(DBG_DEB) | ssh -X repo...@repo.proxmox.com -- upload 
--product pbs,pmg,pve --dist buster --arch $(DEB_BUILD_ARCH)
+
+.PHONY: distclean
+distclean: clean
+
+.PHONY: clean
+clean:
+   cargo clean
+   rm -rf *.deb *.buildinfo *.changes *.dsc rust-$(PACKAGE)_*.tar.?z 
$(BUILDDIR) $(BUILDDIR_TMP)
+   find . -name '*~' -exec rm {} ';'
diff --git a/debian/changelog b/debian/changelog
new file mode 100644
index 000..11e26ed
--- /dev/null
+++ b/debian/changelog
@@ -0,0 +1,5 @@
+rust-proxmox-apt (0.1.0-1) unstable; urgency=medium
+
+  * Initial release.
+
+ -- Proxmox Support Team   Thu, 18 Feb 2021 10:20:44 +0100
diff --git a/debian/control b/debian/control
new file mode 100644
index 000..e0938bf
--- /dev/null
+++ b/debian/control
@@ -0,0 +1,43 @@
+Source: rust-proxmox-apt
+Section: rust
+Priority: optional
+Build-Depends: debhelper (>= 12),
+ dh-cargo (>= 24),
+ cargo:native ,
+ rustc:native ,
+ libstd-rust-dev ,
+ librust-anyhow-1+default-dev ,
+ librust-openssl-0.10+default-dev ,
+ librust-proxmox-0.11+api-macro-dev (>= 0.11.5-~~) ,
+ librust-proxmox-0.11+default-dev (>= 0.11.5-~~) ,
+ librust-serde-1+default-dev ,
+ librust-serde-1+derive-dev 
+Maintainer: Proxmox Support Team 
+Standards-Version: 4.5.1
+Vcs-Git: git://git.proxmox.com/git/proxmox-apt.git
+Vcs-Browser: https://git.proxmox.com/?p=proxmox-apt.git
+Homepage: https://www.proxmox.com
+Rules-Requires-Root: no
+
+Package: librust-proxmox-apt-dev
+Architecture: any
+Multi-Arch: same
+Depends:
+ ${misc:Depends},
+ librust-anyhow-1+default-dev,
+ librust-openssl-0.10+default-dev,
+ librust-proxmox-0.11+api-macro-dev (>= 0.11.5-~~),
+ librust-proxmox-0.11+default-dev (>= 0.11.5-~~),
+ librust-serde-1+default-dev,
+ librust-serde-1+derive-dev
+Provides:
+ librust-proxmox-apt+default-dev (= ${binary:Version}),
+ librust-proxmox-apt-0-dev (= ${binary:Version}),
+ librust-proxmox-apt-0+default-dev (= ${binary:Version}),
+ librust-proxmox-apt-0.1-dev (= ${binary:Version}),
+ librust-proxmox-apt-0.1+default-dev (= ${binary:Version}),
+ librust-proxmox-apt-0.1.0-dev (= ${binary:Version}),
+ librust-proxmox-apt-0.1.0+default-dev (= ${binary:Version})
+Description: Proxmox library for APT - Rust source code
+ This package contains the source for the Rust proxmox-apt crate, packaged by
+ debcargo for use with cargo and dh-cargo.
diff --git

[pve-devel] [PATCH v7 proxmox-apt 4/5] add handling of Proxmox standard repositories

2021-06-23 Thread Fabian Ebner
Get handles for the available repositories along with their current
configuration status and make it possible to add them.

Signed-off-by: Fabian Ebner 
---

New in v7.

This also generalizes the Proxmox repository detection logic that was present in
v6 and replaces the is_enterprise_enabled and is_nosubscription_enabled
functions.

 src/repositories/mod.rs|  83 +++
 src/repositories/repository.rs |  14 +++
 src/repositories/standard.rs   | 180 +
 tests/repositories.rs  |  82 ++-
 4 files changed, 358 insertions(+), 1 deletion(-)
 create mode 100644 src/repositories/standard.rs

diff --git a/src/repositories/mod.rs b/src/repositories/mod.rs
index fc54857..eba65f4 100644
--- a/src/repositories/mod.rs
+++ b/src/repositories/mod.rs
@@ -12,6 +12,10 @@ mod file;
 pub use file::{APTRepositoryFile, APTRepositoryFileError, APTRepositoryInfo};
 
 mod release;
+use release::get_current_release_codename;
+
+mod standard;
+pub use standard::{APTRepositoryHandle, APTStandardRepository};
 
 const APT_SOURCES_LIST_FILENAME: &str = "/etc/apt/sources.list";
 const APT_SOURCES_LIST_DIRECTORY: &str = "/etc/apt/sources.list.d/";
@@ -56,6 +60,85 @@ pub fn check_repositories(files: &[APTRepositoryFile]) -> 
Result Result<(APTRepository, String), Error> {
+let suite = get_current_release_codename()?;
+
+let repo = handle.to_repository(product, &suite);
+let path = handle.path(product);
+
+Ok((repo, path))
+}
+
+/// Return handles for standard Proxmox repositories and whether their status, 
where
+/// None means not configured, and Some(bool) indicates enabled or disabled
+pub fn standard_repositories(
+product: &str,
+files: &[APTRepositoryFile],
+) -> Vec {
+let mut result = vec![
+APTStandardRepository {
+handle: APTRepositoryHandle::Enterprise,
+status: None,
+name: APTRepositoryHandle::Enterprise.name(product),
+},
+APTStandardRepository {
+handle: APTRepositoryHandle::NoSubscription,
+status: None,
+name: APTRepositoryHandle::NoSubscription.name(product),
+},
+APTStandardRepository {
+handle: APTRepositoryHandle::Test,
+status: None,
+name: APTRepositoryHandle::Test.name(product),
+},
+];
+
+if product == "pve" {
+result.append(&mut vec![
+APTStandardRepository {
+handle: APTRepositoryHandle::CephPacific,
+status: None,
+name: APTRepositoryHandle::CephPacific.name(product),
+},
+APTStandardRepository {
+handle: APTRepositoryHandle::CephPacificTest,
+status: None,
+name: APTRepositoryHandle::CephPacificTest.name(product),
+},
+APTStandardRepository {
+handle: APTRepositoryHandle::CephOctopus,
+status: None,
+name: APTRepositoryHandle::CephOctopus.name(product),
+},
+APTStandardRepository {
+handle: APTRepositoryHandle::CephOctopusTest,
+status: None,
+name: APTRepositoryHandle::CephOctopusTest.name(product),
+},
+]);
+}
+
+for file in files.iter() {
+for repo in file.repositories.iter() {
+for entry in result.iter_mut() {
+if entry.status == Some(true) {
+continue;
+}
+
+if repo.is_referenced_repository(entry.handle, product) {
+entry.status = Some(repo.enabled);
+}
+}
+}
+}
+
+result
+}
+
 /// Returns all APT repositories configured in `/etc/apt/sources.list` and
 /// in `/etc/apt/sources.list.d` including disabled repositories.
 ///
diff --git a/src/repositories/repository.rs b/src/repositories/repository.rs
index 875e4ee..d0a2b81 100644
--- a/src/repositories/repository.rs
+++ b/src/repositories/repository.rs
@@ -7,6 +7,8 @@ use serde::{Deserialize, Serialize};
 
 use proxmox::api::api;
 
+use crate::repositories::standard::APTRepositoryHandle;
+
 #[api]
 #[derive(Debug, Copy, Clone, Serialize, Deserialize, PartialEq)]
 #[serde(rename_all = "lowercase")]
@@ -266,6 +268,18 @@ impl APTRepository {
 Ok(())
 }
 
+/// Checks if the repository is the one referenced by the handle.
+pub fn is_referenced_repository(&self, handle: APTRepositoryHandle, 
product: &str) -> bool {
+let (package_type, uri, component) = handle.info(product);
+
+self.types.contains(&package_type)
+&& self
+.uris
+.iter()
+.any(|self_uri| self_uri.trim_end_matches('/') == uri)
+&& self.components.contains(&component)
+}
+
 /// Check if a variant of the given suite is configured in this repository
 pub fn has_suite_variant(&s

[pve-devel] [PATCH v7 pve-manager 1/3] api: apt: add call for repository information

2021-06-23 Thread Fabian Ebner
Signed-off-by: Fabian Ebner 
---

Dependency bump for pve-rs needed.

Changes from v6:
* adapt to backend API changes
* merged the checkrepositories call into this one
* the call now also gives a list of standard repositories and their
  configuration status.

 PVE/API2/APT.pm | 200 
 1 file changed, 200 insertions(+)

diff --git a/PVE/API2/APT.pm b/PVE/API2/APT.pm
index fb4954e7..36d0e67a 100644
--- a/PVE/API2/APT.pm
+++ b/PVE/API2/APT.pm
@@ -19,6 +19,7 @@ use PVE::INotify;
 use PVE::Exception;
 use PVE::RESTHandler;
 use PVE::RPCEnvironment;
+use PVE::RS::APT::Repositories;
 use PVE::API2Tools;
 
 use JSON;
@@ -66,6 +67,7 @@ __PACKAGE__->register_method({
 
my $res = [ 
{ id => 'changelog' },
+   { id => 'repositories' },
{ id => 'update' },
{ id => 'versions' },
];
@@ -478,6 +480,204 @@ __PACKAGE__->register_method({
return $data;
 }});
 
+__PACKAGE__->register_method({
+name => 'repositories',
+path => 'repositories',
+method => 'GET',
+proxyto => 'node',
+description => "Get APT repository information.",
+permissions => {
+   check => ['perm', '/nodes/{node}', [ 'Sys.Audit' ]],
+},
+parameters => {
+   additionalProperties => 0,
+   properties => {
+   node => get_standard_option('pve-node'),
+   },
+},
+returns => {
+   type => "object",
+   description => "Result from parsing the APT repository files in 
/etc/apt/.",
+   properties => {
+   files => {
+   type => "array",
+   description => "List of parsed repository files.",
+   items => {
+   type => "object",
+   properties => {
+   path => {
+   type => "string",
+   description => "Path to the problematic file.",
+   },
+   'file-type' => {
+   type => "string",
+   enum => [ 'list', 'sources' ],
+   description => "Format of the file.",
+   },
+   repositories => {
+   type => "array",
+   description => "The parsed repositories.",
+   items => {
+   type => "object",
+   properties => {
+   Types => {
+   type => "array",
+   description => "List of package types.",
+   items => {
+   type => "string",
+   enum => [ 'deb', 'deb-src' ],
+   },
+   },
+   URIs => {
+   description => "List of repository 
URIs.",
+   type => "array",
+   items => {
+   type => "string",
+   },
+   },
+   Suites => {
+   type => "array",
+   description => "List of package 
distribuitions",
+   items => {
+   type => "string",
+   },
+   },
+   Components => {
+   type => "array",
+   description => "List of repository 
components",
+   optional => 1, # not present if suite 
is absolute
+   items => {
+   type => "string",
+   },
+   },
+   Options => {
+   type => "array",
+   description => "Additional options",
+   optional => 1,
+   items => {
+   type => "object",
+   properties => {
+   Key => {
+   type => "string",
+   },
+   Values => {
+   type => "array",
+   

[pve-devel] [PATCH v7 proxmox-apt 3/5] add more functions to check repositories

2021-06-23 Thread Fabian Ebner
Currently includes check for suites and check for official URIs

Signed-off-by: Fabian Ebner 
---

Changes from v6:
* move impl blocks and APTRepositoryInfo type  and helpers into
  repository.rs and mod.rs
* limit host_from_uri to http(s)
* fix test by not using assert_eq!(a.sort(), b.sort()) (sort() returns (),
  so this was always true...)
* simplify host matching in check_uris()
* make check_suites dependent on the current release from /etc/os-release
* add 'property' to APTRepositoryInfo to know where it originates from
* use index (starting from 0) instead of number (starting from 1) in
  APTRepositoryInfo. Originally it came from line number, but each
  APTRepositoryFile contains an array of repos and thus index is the better
  fit.

 src/repositories/file.rs  | 119 +-
 src/repositories/mod.rs   |  21 +++-
 src/repositories/release.rs   |  42 
 src/repositories/repository.rs|  58 +++
 tests/repositories.rs | 106 ++-
 tests/sources.list.d.expected/bad.sources |  30 ++
 tests/sources.list.d.expected/pve.list|   2 +
 tests/sources.list.d/bad.sources  |  29 ++
 tests/sources.list.d/pve.list |   2 +
 9 files changed, 405 insertions(+), 4 deletions(-)
 create mode 100644 src/repositories/release.rs
 create mode 100644 tests/sources.list.d.expected/bad.sources
 create mode 100644 tests/sources.list.d/bad.sources

diff --git a/src/repositories/file.rs b/src/repositories/file.rs
index bc48bf2..6225f1c 100644
--- a/src/repositories/file.rs
+++ b/src/repositories/file.rs
@@ -2,10 +2,13 @@ use std::convert::TryFrom;
 use std::fmt::Display;
 use std::path::{Path, PathBuf};
 
-use anyhow::{format_err, Error};
+use anyhow::{bail, format_err, Error};
 use serde::{Deserialize, Serialize};
 
-use crate::repositories::repository::{APTRepository, APTRepositoryFileType};
+use crate::repositories::release::{get_current_release_codename, 
DEBIAN_SUITES};
+use crate::repositories::repository::{
+APTRepository, APTRepositoryFileType, APTRepositoryPackageType,
+};
 
 use proxmox::api::api;
 
@@ -85,6 +88,29 @@ impl std::error::Error for APTRepositoryFileError {
 }
 }
 
+#[api]
+#[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord, Serialize, Deserialize)]
+#[serde(rename_all = "kebab-case")]
+/// Additional information for a repository.
+pub struct APTRepositoryInfo {
+/// Path to the defining file.
+#[serde(skip_serializing_if = "String::is_empty")]
+pub path: String,
+
+/// Index of the associated respository within the file (starting from 0).
+pub index: usize,
+
+/// The property from which the info originates (e.g. "Suites")
+#[serde(skip_serializing_if = "Option::is_none")]
+pub property: Option,
+
+/// Info kind (e.g. "warning")
+pub kind: String,
+
+/// Info message
+pub message: String,
+}
+
 impl APTRepositoryFile {
 /// Creates a new `APTRepositoryFile` without parsing.
 ///
@@ -271,4 +297,93 @@ impl APTRepositoryFile {
 
 Ok(())
 }
+
+/// Checks if old or unstable suites are configured and also that the
+/// `stable` keyword is not used.
+pub fn check_suites(&self) -> Result, Error> {
+let mut infos = vec![];
+
+for (n, repo) in self.repositories.iter().enumerate() {
+if !repo
+.types
+.iter()
+.any(|package_type| *package_type == 
APTRepositoryPackageType::Deb)
+{
+continue;
+}
+
+let mut add_info = |kind, message| {
+infos.push(APTRepositoryInfo {
+path: self.path.clone(),
+index: n,
+property: Some("Suites".to_string()),
+kind,
+message,
+})
+};
+
+let current_suite = get_current_release_codename()?;
+
+let current_index = match DEBIAN_SUITES
+.iter()
+.position(|&suite| suite == current_suite)
+{
+Some(index) => index,
+None => bail!("unknown release {}", current_suite),
+};
+
+for (n, suite) in DEBIAN_SUITES.iter().enumerate() {
+if repo.has_suite_variant(suite) {
+if n < current_index {
+add_info(
+"warning".to_string(),
+format!("old suite '{}' configured!", suite),
+);
+}
+
+if n == current_index + 1 {
+add_info(
+"ignore-pre-upgrade-warning".to_string(),
+format!("suite '{}' should not be used in 
production!", suite),
+);
+}

[pve-devel] [PATCH-SERIES v7] APT repositories API/UI

2021-06-23 Thread Fabian Ebner
List the configured repositories and have some basic checks for them.
Allow adding standard Proxmox repositories and enabling/disabling repositories.


Changes from v6:
* do not include the upgrade functionality for now, focus on being
  able to add standard repositories and enable/disable them
* put impl blocks and struct declarations together in proxmox-apt
* merge repositories and check_repositories get calls
* see individual patches for more

Older changes can be found here:
https://lists.proxmox.com/pipermail/pve-devel/2021-June/048598.html


The dependency for pve-rs to proxmox-apt is already in the patches.
Additionally, pve-manager depends on pve-rs and proxmox-widget-toolkit.


proxmox-apt:

Fabian Ebner (5):
  initial commit
  add files for Debian packaging
  add more functions to check repositories
  add handling of Proxmox standard repositories
  bump version to 0.2.0-1


pve-rs:

Fabian Ebner (1):
  add bindings for proxmox-apt

 Cargo.toml  |   2 +
 Makefile|   6 +-
 src/apt/mod.rs  |   1 +
 src/apt/repositories.rs | 128 
 src/lib.rs  |   4 +-
 5 files changed, 136 insertions(+), 5 deletions(-)
 create mode 100644 src/apt/mod.rs
 create mode 100644 src/apt/repositories.rs


proxmox-widget-toolkit:

Fabian Ebner (2):
  add UI for APT repositories
  add buttons for add/enable/disable

 src/Makefile|   1 +
 src/node/APTRepositories.js | 503 
 2 files changed, 504 insertions(+)
 create mode 100644 src/node/APTRepositories.js


pve-manager:

Fabian Ebner (3):
  api: apt: add call for repository information
  api: apt: add PUT and POST handler for repositories
  ui: add panel for listing APT repositories

 PVE/API2/APT.pm | 288 
 www/manager6/node/Config.js |   9 ++
 2 files changed, 297 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 v7 pve-manager 3/3] ui: add panel for listing APT repositories

2021-06-23 Thread Fabian Ebner
Signed-off-by: Fabian Ebner 
---

Dependency bump for proxmox-widget-toolkit needed.

Changes from v6:
* remove majorUpgradeAllowed parameter (the plan is to have an API call
  returning release information and use that in proxmox-widget-toolkit)
* Have 'Updates' be our parent in the config panel/tree.

 www/manager6/node/Config.js | 9 +
 1 file changed, 9 insertions(+)

diff --git a/www/manager6/node/Config.js b/www/manager6/node/Config.js
index 235a7480..0d2c690c 100644
--- a/www/manager6/node/Config.js
+++ b/www/manager6/node/Config.js
@@ -242,6 +242,15 @@ Ext.define('PVE.node.Config', {
},
nodename: nodename,
});
+
+   me.items.push({
+   xtype: 'proxmoxNodeAPTRepositories',
+   title: gettext('APT Repositories'),
+   iconCls: 'fa fa-files-o',
+   itemId: 'aptrepositories',
+   nodename: nodename,
+   groups: ['apt'],
+   });
}
}
 
-- 
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 v7 proxmox-widget-toolkit 2/2] add buttons for add/enable/disable

2021-06-23 Thread Fabian Ebner
Signed-off-by: Fabian Ebner 
---

New in v7.

 src/node/APTRepositories.js | 80 +
 1 file changed, 80 insertions(+)

diff --git a/src/node/APTRepositories.js b/src/node/APTRepositories.js
index 30c31ec..2121a0f 100644
--- a/src/node/APTRepositories.js
+++ b/src/node/APTRepositories.js
@@ -63,6 +63,46 @@ Ext.define('Proxmox.node.APTRepositoriesGrid', {
me.up('proxmoxNodeAPTRepositories').reload();
},
},
+   {
+   text: gettext('Add'),
+   menu: {
+   plain: true,
+   itemId: "addMenu",
+   items: [],
+   },
+   },
+   {
+   xtype: 'proxmoxButton',
+   text: gettext('Enable') + '/' + gettext('Disable'),
+   disabled: true,
+   handler: function(button, event, record) {
+   let me = this;
+   let panel = me.up('proxmoxNodeAPTRepositories');
+
+   let params = {
+   path: record.data.Path,
+   index: record.data.Index,
+   enabled: record.data.Enabled ? 0 : 1, // invert
+   };
+
+   if (panel.digest !== undefined) {
+  params.digest = panel.digest;
+   }
+
+   Proxmox.Utils.API2Request({
+   url: `/nodes/${panel.nodename}/apt/repositories`,
+   method: 'POST',
+   params: params,
+   failure: function(response, opts) {
+   Ext.Msg.alert(gettext('Error'), response.htmlStatus);
+   panel.reload();
+   },
+   success: function(response, opts) {
+   panel.reload();
+   },
+   });
+   },
+   },
 ],
 
 sortableColumns: false,
@@ -340,6 +380,9 @@ Ext.define('Proxmox.node.APTRepositories', {
let me = this;
let vm = me.getViewModel();
 
+   let menu = me.down('#addMenu');
+   menu.removeAll();
+
for (const standardRepo of standardRepos) {
const handle = standardRepo.handle;
const status = standardRepo.status;
@@ -349,6 +392,43 @@ Ext.define('Proxmox.node.APTRepositories', {
} else if (handle === "no-subscription") {
vm.set('noSubscriptionRepo', status);
}
+
+   let status_text = '';
+   if (status !== undefined && status !== null) {
+   status_text = Ext.String.format(
+   ' ({0}, {1})',
+   gettext('configured'),
+   status ? gettext('enabled') : gettext('disabled'),
+   );
+   }
+
+   menu.add({
+   text: standardRepo.name + status_text,
+   disabled: status !== undefined && status !== null,
+   repoHandle: handle,
+   handler: function(menuItem) {
+  let params = {
+  handle: menuItem.repoHandle,
+  };
+
+  if (me.digest !== undefined) {
+  params.digest = me.digest;
+  }
+
+   Proxmox.Utils.API2Request({
+   url: `/nodes/${me.nodename}/apt/repositories`,
+   method: 'PUT',
+   params: params,
+   failure: function(response, opts) {
+   Ext.Msg.alert(gettext('Error'), 
response.htmlStatus);
+   me.reload();
+   },
+   success: function(response, opts) {
+   me.reload();
+   },
+   });
+   },
+   });
}
 },
 
-- 
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 v7 pve-manager 2/3] api: apt: add PUT and POST handler for repositories

2021-06-23 Thread Fabian Ebner
To allow adding/modifying them. Currently the only possible modification is
enable/disable.

Signed-off-by: Fabian Ebner 
---

New in v7.

 PVE/API2/APT.pm | 88 +
 1 file changed, 88 insertions(+)

diff --git a/PVE/API2/APT.pm b/PVE/API2/APT.pm
index 36d0e67a..c58203a7 100644
--- a/PVE/API2/APT.pm
+++ b/PVE/API2/APT.pm
@@ -678,6 +678,94 @@ __PACKAGE__->register_method({
return PVE::RS::APT::Repositories::repositories();
 }});
 
+__PACKAGE__->register_method({
+name => 'add_repository',
+path => 'repositories',
+method => 'PUT',
+description => "Add a standard repository to the configuration",
+permissions => {
+   check => ['perm', '/nodes/{node}', [ 'Sys.Modify' ]],
+},
+protected => 1,
+proxyto => 'node',
+parameters => {
+   additionalProperties => 0,
+   properties => {
+   node => get_standard_option('pve-node'),
+   handle => {
+   type => 'string',
+   description => "Handle that identifies a repository.",
+   },
+   digest => {
+   type => "string",
+   description => "Digest to detect modifications.",
+   maxLength => 80,
+   optional => 1,
+   },
+   },
+},
+returns => {
+   type => 'null',
+},
+code => sub {
+   my ($param) = @_;
+
+   PVE::RS::APT::Repositories::add_repository($param->{handle}, 
$param->{digest});
+}});
+
+__PACKAGE__->register_method({
+name => 'change_repository',
+path => 'repositories',
+method => 'POST',
+description => "Change the properties of a repository. Currently only 
allows enabling/disabling.",
+permissions => {
+   check => ['perm', '/nodes/{node}', [ 'Sys.Modify' ]],
+},
+protected => 1,
+proxyto => 'node',
+parameters => {
+   additionalProperties => 0,
+   properties => {
+   node => get_standard_option('pve-node'),
+   path => {
+   type => 'string',
+   description => "Path to the containing file.",
+   },
+   index => {
+   type => 'integer',
+   description => "Index within the file (starting from 0).",
+   },
+   enabled => {
+   type => 'boolean',
+   description => "Whether the repository should be enabled or 
not.",
+   optional => 1,
+   },
+   digest => {
+   type => "string",
+   description => "Digest to detect modifications.",
+   maxLength => 80,
+   optional => 1,
+   },
+   },
+},
+returns => {
+   type => 'null',
+},
+code => sub {
+   my ($param) = @_;
+
+   my $options = {
+   enabled => $param->{enabled},
+   };
+
+   PVE::RS::APT::Repositories::change_repository(
+   $param->{path},
+   $param->{index},
+   $options,
+   $param->{digest}
+   );
+}});
+
 __PACKAGE__->register_method({
 name => 'versions', 
 path => 'versions', 
-- 
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 v7 pve-rs 1/1] add bindings for proxmox-apt

2021-06-23 Thread Fabian Ebner
which contains includes logic for repository handling.

Signed-off-by: Fabian Ebner 
---

Changes from v6:
* have repositories() return everything at once
* base on now existing pve-rs
* don't use raw_return
* add bindings for add/change

 Cargo.toml  |   2 +
 Makefile|   6 +-
 src/apt/mod.rs  |   1 +
 src/apt/repositories.rs | 128 
 src/lib.rs  |   4 +-
 5 files changed, 136 insertions(+), 5 deletions(-)
 create mode 100644 src/apt/mod.rs
 create mode 100644 src/apt/repositories.rs

diff --git a/Cargo.toml b/Cargo.toml
index 9c274ff..d39c1ad 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -19,4 +19,6 @@ crate-type = [ "cdylib" ]
 anyhow = "1.0"
 proxmox = { version = "0.11.5", default-features = false }
 perlmod = { version = "0.5.1", features = [ "exporter" ] }
+proxmox-apt = "0.2.0"
 proxmox-openid = "0.5.0"
+serde = "1.0"
diff --git a/Makefile b/Makefile
index e340623..9701c8e 100644
--- a/Makefile
+++ b/Makefile
@@ -14,10 +14,12 @@ DEBS=$(MAIN_DEB) $(DBGSYM_DEB)
 
 DESTDIR=
 
-PM_DIRS :=
+PM_DIRS := \
+   PVE/RS/APT
 
 PM_FILES := \
-   PVE/RS/OpenId.pm
+   PVE/RS/OpenId.pm \
+   PVE/RS/APT/Repositories.pm
 
 ifeq ($(BUILD_MODE), release)
 CARGO_BUILD_ARGS += --release
diff --git a/src/apt/mod.rs b/src/apt/mod.rs
new file mode 100644
index 000..574c1a7
--- /dev/null
+++ b/src/apt/mod.rs
@@ -0,0 +1 @@
+mod repositories;
diff --git a/src/apt/repositories.rs b/src/apt/repositories.rs
new file mode 100644
index 000..3a421f0
--- /dev/null
+++ b/src/apt/repositories.rs
@@ -0,0 +1,128 @@
+#[perlmod::package(name = "PVE::RS::APT::Repositories", lib = "pve_rs")]
+mod export {
+use std::convert::TryInto;
+
+use anyhow::{bail, Error};
+use serde::{Deserialize, Serialize};
+
+use proxmox_apt::repositories::{
+APTRepositoryFile, APTRepositoryFileError, APTRepositoryInfo, 
APTStandardRepository,
+};
+
+#[derive(Deserialize, Serialize)]
+#[serde(rename_all = "kebab-case")]
+/// Result for the repositories() function
+pub struct RepositoriesResult {
+/// Successfully parsed files.
+pub files: Vec,
+
+/// Errors for files that could not be parsed or read.
+pub errors: Vec,
+
+/// Common digest for successfully parsed files.
+pub digest: String,
+
+/// Additional information/warnings about repositories.
+pub infos: Vec,
+
+/// Standard repositories and their configuration status.
+pub standard_repos: Vec,
+}
+
+#[derive(Deserialize, Serialize)]
+#[serde(rename_all = "kebab-case")]
+/// For changing an existing repository.
+pub struct ChangeProperties {
+/// Whether the repository should be enabled or not.
+pub enabled: Option,
+}
+
+/// Get information about configured and standard repositories.
+#[export]
+pub fn repositories() -> Result {
+let (files, errors, digest) = 
proxmox_apt::repositories::repositories()?;
+let digest = proxmox::tools::digest_to_hex(&digest);
+
+let infos = proxmox_apt::repositories::check_repositories(&files)?;
+let standard_repos = 
proxmox_apt::repositories::standard_repositories("pve", &files);
+
+Ok(RepositoriesResult {
+files,
+errors,
+digest,
+infos,
+standard_repos,
+})
+}
+
+/// Add the repository identified by the `handle`.
+///
+/// The `digest` parameter asserts that the configuration has not been 
modified.
+#[export]
+pub fn add_repository(handle: &str, digest: Option<&str>) -> Result<(), 
Error> {
+let (mut files, _errors, current_digest) = 
proxmox_apt::repositories::repositories()?;
+
+if let Some(digest) = digest {
+let expected_digest = proxmox::tools::hex_to_digest(digest)?;
+if expected_digest != current_digest {
+bail!("detected modified configuration - file changed by other 
user? Try again.");
+}
+}
+
+let (repo, path) =
+
proxmox_apt::repositories::get_standard_repository(handle.try_into()?, "pve")?;
+
+if let Some(file) = files.iter_mut().find(|file| file.path == path) {
+file.repositories.push(repo);
+
+file.write()?;
+} else {
+let mut file = match APTRepositoryFile::new(&path)? {
+Some(file) => file,
+None => bail!("invalid path - {}", path),
+};
+
+file.repositories.push(repo);
+
+file.write()?;
+}
+
+Ok(())
+}
+
+/// Change the properties of the specified repository.
+///
+/// The `digest` parameter asserts that the configuration has not been 
modified.
+#[export]
+pub fn change_repository(
+path: &str,
+index: usize,
+options: ChangeProperties,
+digest: O

[pve-devel] [PATCH v7 proxmox-widget-toolkit 1/2] add UI for APT repositories

2021-06-23 Thread Fabian Ebner
Signed-off-by: Fabian Ebner 
---

Changes from v6:
* adapt to new API
* squashed patch adding warnings/checks into this one
* say 'enabled' instead of 'configured' in warnings
* move tbar to grid component (selection model is needed for future buttons)
* use greyed-out icon if repository is disabled

 src/Makefile|   1 +
 src/node/APTRepositories.js | 423 
 2 files changed, 424 insertions(+)
 create mode 100644 src/node/APTRepositories.js

diff --git a/src/Makefile b/src/Makefile
index 37da480..23f2360 100644
--- a/src/Makefile
+++ b/src/Makefile
@@ -72,6 +72,7 @@ JSSRC=\
window/ACMEDomains.js   \
window/FileBrowser.js   \
node/APT.js \
+   node/APTRepositories.js \
node/NetworkEdit.js \
node/NetworkView.js \
node/DNSEdit.js \
diff --git a/src/node/APTRepositories.js b/src/node/APTRepositories.js
new file mode 100644
index 000..30c31ec
--- /dev/null
+++ b/src/node/APTRepositories.js
@@ -0,0 +1,423 @@
+Ext.define('apt-repolist', {
+extend: 'Ext.data.Model',
+fields: [
+   'Path',
+   'Index',
+   'OfficialHost',
+   'FileType',
+   'Enabled',
+   'Comment',
+   'Types',
+   'URIs',
+   'Suites',
+   'Components',
+   'Options',
+],
+});
+
+Ext.define('Proxmox.node.APTRepositoriesErrors', {
+extend: 'Ext.grid.GridPanel',
+
+xtype: 'proxmoxNodeAPTRepositoriesErrors',
+
+title: gettext('Errors'),
+
+store: {},
+
+viewConfig: {
+   stripeRows: false,
+   getRowClass: () => 'proxmox-invalid-row',
+},
+
+columns: [
+   {
+   header: gettext('File'),
+   dataIndex: 'path',
+   renderer: function(value, cell, record) {
+   return "" + value;
+   },
+   width: 350,
+   },
+   {
+   header: gettext('Error'),
+   dataIndex: 'error',
+   flex: 1,
+   },
+],
+});
+
+Ext.define('Proxmox.node.APTRepositoriesGrid', {
+extend: 'Ext.grid.GridPanel',
+
+xtype: 'proxmoxNodeAPTRepositoriesGrid',
+
+title: gettext('APT Repositories'),
+
+tbar: [
+   {
+   text: gettext('Reload'),
+   iconCls: 'fa fa-refresh',
+   handler: function() {
+   let me = this;
+   me.up('proxmoxNodeAPTRepositories').reload();
+   },
+   },
+],
+
+sortableColumns: false,
+
+columns: [
+   {
+   header: gettext('Official'),
+   dataIndex: 'OfficialHost',
+   renderer: function(value, cell, record) {
+   let icon = (cls) => ``;
+
+   const enabled = record.data.Enabled;
+
+   if (value === undefined || value === null) {
+   return icon('fa-question-circle-o');
+   }
+   if (!value) {
+   return icon('fa-times ' + (enabled ? 'critical' : 'faded'));
+   }
+   return icon('fa-check ' + (enabled ? 'good' : 'faded'));
+   },
+   width: 70,
+   },
+   {
+   header: gettext('Enabled'),
+   dataIndex: 'Enabled',
+   renderer: Proxmox.Utils.format_enabled_toggle,
+   width: 90,
+   },
+   {
+   header: gettext('Types'),
+   dataIndex: 'Types',
+   renderer: function(types, cell, record) {
+   return types.join(' ');
+   },
+   width: 100,
+   },
+   {
+   header: gettext('URIs'),
+   dataIndex: 'URIs',
+   renderer: function(uris, cell, record) {
+   return uris.join(' ');
+   },
+   width: 350,
+   },
+   {
+   header: gettext('Suites'),
+   dataIndex: 'Suites',
+   renderer: function(suites, cell, record) {
+   return suites.join(' ');
+   },
+   width: 130,
+   },
+   {
+   header: gettext('Components'),
+   dataIndex: 'Components',
+   renderer: function(components, cell, record) {
+   return components.join(' ');
+   },
+   width: 170,
+   },
+   {
+   header: gettext('Options'),
+   dataIndex: 'Options',
+   renderer: function(options, cell, record) {
+   if (!options) {
+   return '';
+   }
+
+   let filetype = record.data.FileType;
+   let text = '';
+
+   options.forEach(function(option) {
+   let key = option.Key;
+   if (filetype === 'list') {
+   let values = option.Values.join(',');
+   text += `${key}=${values} `;
+   } else if (filetype === 'sources') {
+   let values = option.Values.

[pve-devel] [PATCH v7 proxmox-apt 1/5] initial commit

2021-06-23 Thread Fabian Ebner
Signed-off-by: Fabian Ebner 
---

Changes from v6:
* make common_digest helper private and return the common digest together
  with the parsed repository files.
* keep impl blocks and type declarations together
* base tests on bullseye
* ignore file extensions ignored by APT (e.g. ".orig")
* file.write() now removes a file if no repositories are left
* get rid of write_repositories() function: It's just a loop over
  file.write() and returning a Vec<> of errors is akward. Just let the
  caller iterate themselves and decide how to react when an error occurs.
* prefer kebab-case over lowercase (only affects one parameter, 'file-type')

 .cargo/config |   5 +
 .gitignore|   4 +
 Cargo.toml|  23 ++
 rustfmt.toml  |   1 +
 src/lib.rs|   1 +
 src/repositories/file.rs  | 274 ++
 src/repositories/file/list_parser.rs  | 172 +
 src/repositories/file/sources_parser.rs   | 204 ++
 src/repositories/mod.rs   | 107 ++
 src/repositories/repository.rs| 353 ++
 tests/repositories.rs | 162 
 .../absolute_suite.list   |   5 +
 .../absolute_suite.sources|   5 +
 tests/sources.list.d.expected/case.sources|  16 +
 .../sources.list.d.expected/multiline.sources |  10 +
 .../options_comment.list  |   6 +
 .../pbs-enterprise.list   |   2 +
 tests/sources.list.d.expected/pve.list|  13 +
 tests/sources.list.d.expected/standard.list   |   7 +
 .../sources.list.d.expected/standard.sources  |  11 +
 tests/sources.list.d/absolute_suite.list  |   4 +
 tests/sources.list.d/absolute_suite.sources   |   5 +
 tests/sources.list.d/case.sources |  17 +
 tests/sources.list.d/multiline.sources|  11 +
 tests/sources.list.d/options_comment.list |   3 +
 tests/sources.list.d/pbs-enterprise.list  |   1 +
 tests/sources.list.d/pve.list |  10 +
 tests/sources.list.d/standard.list|   6 +
 tests/sources.list.d/standard.sources |  10 +
 29 files changed, 1448 insertions(+)
 create mode 100644 .cargo/config
 create mode 100644 .gitignore
 create mode 100644 Cargo.toml
 create mode 100644 rustfmt.toml
 create mode 100644 src/lib.rs
 create mode 100644 src/repositories/file.rs
 create mode 100644 src/repositories/file/list_parser.rs
 create mode 100644 src/repositories/file/sources_parser.rs
 create mode 100644 src/repositories/mod.rs
 create mode 100644 src/repositories/repository.rs
 create mode 100644 tests/repositories.rs
 create mode 100644 tests/sources.list.d.expected/absolute_suite.list
 create mode 100644 tests/sources.list.d.expected/absolute_suite.sources
 create mode 100644 tests/sources.list.d.expected/case.sources
 create mode 100644 tests/sources.list.d.expected/multiline.sources
 create mode 100644 tests/sources.list.d.expected/options_comment.list
 create mode 100644 tests/sources.list.d.expected/pbs-enterprise.list
 create mode 100644 tests/sources.list.d.expected/pve.list
 create mode 100644 tests/sources.list.d.expected/standard.list
 create mode 100644 tests/sources.list.d.expected/standard.sources
 create mode 100644 tests/sources.list.d/absolute_suite.list
 create mode 100644 tests/sources.list.d/absolute_suite.sources
 create mode 100644 tests/sources.list.d/case.sources
 create mode 100644 tests/sources.list.d/multiline.sources
 create mode 100644 tests/sources.list.d/options_comment.list
 create mode 100644 tests/sources.list.d/pbs-enterprise.list
 create mode 100644 tests/sources.list.d/pve.list
 create mode 100644 tests/sources.list.d/standard.list
 create mode 100644 tests/sources.list.d/standard.sources

diff --git a/.cargo/config b/.cargo/config
new file mode 100644
index 000..3b5b6e4
--- /dev/null
+++ b/.cargo/config
@@ -0,0 +1,5 @@
+[source]
+[source.debian-packages]
+directory = "/usr/share/cargo/registry"
+[source.crates-io]
+replace-with = "debian-packages"
diff --git a/.gitignore b/.gitignore
new file mode 100644
index 000..24917d4
--- /dev/null
+++ b/.gitignore
@@ -0,0 +1,4 @@
+Cargo.lock
+target/
+tests/sources.list.d.actual
+tests/sources.list.d.digest
diff --git a/Cargo.toml b/Cargo.toml
new file mode 100644
index 000..24f734b
--- /dev/null
+++ b/Cargo.toml
@@ -0,0 +1,23 @@
+[package]
+name = "proxmox-apt"
+version = "0.1.0"
+authors = [
+"Fabian Ebner ",
+"Proxmox Support Team ",
+]
+edition = "2018"
+license = "AGPL-3"
+description = "Proxmox library for APT"
+homepage = "https://www.proxmox.com";
+
+exclude = [ "debian" ]
+
+[lib]
+name = "proxmox_apt"
+path = "src/lib.rs"
+
+[dependencies]
+anyhow = "1.0"
+openssl = "0.10"
+proxmox = { version = "0.11.5", features = [ "api-macro" ] }
+

Re: [pve-devel] [PATCH v2 container 3/3] special case btrfs+quotas to use subvolumes

2021-06-23 Thread Fabian Grünbichler
On June 22, 2021 2:18 pm, Wolfgang Bumiller wrote:
> Signed-off-by: Wolfgang Bumiller 
> ---
> New in v2.
> 
>  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 0a8a532..fc06842 100644
> --- a/src/PVE/LXC.pm
> +++ b/src/PVE/LXC.pm
> @@ -1893,7 +1893,7 @@ sub alloc_disk {
>  eval {
>   my $do_format = 0;
>   if ($scfg->{content}->{rootdir} && $scfg->{path}) {
> - if ($size_kb > 0) {
> + if ($size_kb > 0 && !($scfg->{type} eq 'btrfs' && $scfg->{quotas})) 
> {

this is actually not enough.. btrfs with quotas now creates subvols with 
size 0 (== no quota), and then everything fails because the storage 
plugin expects a quota to be there but there is none. even with that 
fixed up, at least resize_disk is still broken:

$ pct resize 123 mp0 +1G
btrfs error: ERROR: invalid qgroupid or subvolume path: 0/0/286

the actual qgroupid should be 0/286..

>   $volid = PVE::Storage::vdisk_alloc($storecfg, $storage, $vmid, 
> 'raw', undef, $size_kb);
>   $do_format = 1;
>   } else {
> -- 
> 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 manager] ui: node summary: show repository configuration status

2021-06-23 Thread Fabian Ebner
I tried to use itemid and lookupreference for the nodeStatus item, but couldn't
get it to work, so I factored it out.

Signed-off-by: Fabian Ebner 
---

Depends on the APT repositories API/UI series.
https://lists.proxmox.com/pipermail/pve-devel/2021-June/048963.html

 www/manager6/node/StatusView.js | 102 
 www/manager6/node/Summary.js|  49 +--
 2 files changed, 145 insertions(+), 6 deletions(-)

diff --git a/www/manager6/node/StatusView.js b/www/manager6/node/StatusView.js
index 564658c4..f4a55d14 100644
--- a/www/manager6/node/StatusView.js
+++ b/www/manager6/node/StatusView.js
@@ -2,6 +2,69 @@ Ext.define('PVE.node.StatusView', {
 extend: 'Proxmox.panel.StatusView',
 alias: 'widget.pveNodeStatus',
 
+viewModel: {
+   data: {
+   subscriptionActive: '',
+   noSubscriptionRepo: '',
+   enterpriseRepo: '',
+   testRepo: '',
+   },
+   formulas: {
+   repoStatus: function(get) {
+   if (get('subscriptionActive') === '' ||
+   get('enterpriseRepo') === '') {
+   return '';
+   }
+
+   if (!get('subscriptionActive') && get('enterpriseRepo')) {
+   return 'no-sub';
+   }
+
+   if (get('noSubscriptionRepo') || get('testRepo')) {
+   return 'non-production';
+   }
+
+   if (!get('enterpriseRepo') || !get('noSubscriptionRepo') || 
!get('testRepo')) {
+   return 'no-repo';
+   }
+
+   return 'ok';
+   },
+   repoStatusMessage: function(get) {
+   const status = get('repoStatus');
+
+   if (status === 'ok') {
+   return gettext('Enterprise repository and subscription 
active');
+   } else if (status === 'no-sub') {
+   return gettext('Enterprise repository enabled, but no 
active subscription');
+   } else if (status === 'non-production') {
+   return gettext('No-subscription or test repository in use');
+   } else if (status === 'no-repo') {
+   return gettext('No PVE repository is enabled!');
+   }
+
+   return Proxmox.Utils.unknownText;
+   },
+   repoStatusIconCls: function(get) {
+   const status = get('repoStatus');
+
+   let iconCls = (cls) => `fa fa-fw ${cls}`;
+
+   if (status === 'ok') {
+   return iconCls('fa-check good');
+   } else if (status === 'no-sub') {
+   return iconCls('fa-exclamation-triangle critical');
+   } else if (status === 'non-production') {
+   return iconCls('fa-exclamation-triangle warning');
+   } else if (status === 'no-repo') {
+   return iconCls('fa-exclamation-triangle critical');
+   }
+
+   return iconCls('fa-question-circle-o');
+   },
+   },
+},
+
 height: 300,
 bodyPadding: '20 15 20 15',
 
@@ -113,6 +176,21 @@ Ext.define('PVE.node.StatusView', {
textField: 'pveversion',
value: '',
},
+   {
+   itemId: 'repositoryStatus',
+   colspan: 2,
+   printBar: false,
+   title: gettext('Repository Configuration Status'),
+   // for bind
+   setValue: function(value) {
+   let me = this;
+   me.updateValue(value);
+   },
+   bind: {
+   iconCls: '{repoStatusIconCls}',
+   value: '{repoStatusMessage}',
+   },
+   },
 ],
 
 updateTitle: function() {
@@ -121,4 +199,28 @@ Ext.define('PVE.node.StatusView', {
me.setTitle(me.pveSelNode.data.node + ' (' + gettext('Uptime') + ': ' + 
uptime + ')');
 },
 
+setRepositoryInfo: function(standardRepos) {
+   let me = this;
+   let vm = me.getViewModel();
+
+   for (const standardRepo of standardRepos) {
+   const handle = standardRepo.handle;
+   const status = standardRepo.status;
+
+   if (handle === "enterprise") {
+   vm.set('enterpriseRepo', status);
+   } else if (handle === "no-subscription") {
+   vm.set('noSubscriptionRepo', status);
+   } else if (handle === "test") {
+   vm.set('testRepo', status);
+   }
+   }
+},
+
+setSubscriptionStatus: function(status) {
+   let me = this;
+   let vm = me.getViewModel();
+
+   vm.set('subscriptionActive', status);
+},
 });
diff --git a/www/manager6/node/Summary.js b/www/manager6/node/Summary.js
index 1c93ef04..f3709dfc 100644
--- a/www/manager6/node/Summary.js
+++ b/www/manager6/node/Summary.js
@@ -83,6 +83,38 @@ Ext.define('PVE.node.Summary', {
});
 },
 
+updateRepositoryStatus: function() {
+   let me = this;
+   let no

Re: [pve-devel] [POC qemu-server] fix 3303: allow "live" upgrade of qemu version

2021-06-23 Thread Laurent GUERBY
On Thu, 2021-04-08 at 18:44 +0200, Thomas Lamprecht wrote:
> On 08.04.21 12:33, Fabian Ebner wrote:
> > The code is in a very early state, I'm just sending this to discuss
> > the idea.
> > I didn't do a whole lot of testing yet, but it does seem to work.
> > 
> > The idea is rather simple:
> > 1. save the state to ramfs
> > 2. stop the VM
> > 3. start the VM loading the state
> 
> For the record, as we (Dietmar, you and I) discussed this a bit off-
> list:
> 
> The issue we see here is that one temporarily requires a potential
> big chunk of
> free memory, i.e., another time the amount the guest is assigned. So
> tens to
> hundreds of GiB, which (educated guess) > 90 % of our users just do
> not have
> available, at least for the bigger VMs of theirs.
> 
> So, it would be nicer if we could makes this more QEMU internal,
> e.g., just save
> the state out (as that one may not be compatible 1:1 for reuse with
> the new QEMU
> version) and re-use the guest memory directly, e.g., start new QEMU
> process
> migrate state and map over the guest-memory, then pause old one, cont
> new one and
> be done (very condensed).
> That may have it's own difficulties/edge-cases, but it would not
> require having
> so much extra memory freely available...

Hi,

I'm wondering how much ksm would help reduce the extra memory
requirement during same host migration.

May be there's a sweet spot by changing ksm to be more aggressive just
before starting the migration and slowing down the migration using
bandwidth control parameter so all new pages created by the migration
process end up shared quickly? And returning ksmtuned to default after
it's done.

Or may be only lowering migration bandwidth will be enough with ksm
settings unchanged (still has to be faster than mutation rate though so
can't be too low).

I assume for most users even if the migration to same host is slow it's
fine since it will not consume network ressources, just a bit more cpu.

Sincerely,

Laurent

PS: thanks Stefan_R for pointing this thread
https://forum.proxmox.com/threads/upgrade-of-pve-qemu-kvm-and-running-v
m.91236/

> > 
> > This approach solves the problem that our stack is (currently) not
> > designed to
> > have multiple instances with the same VM ID running. To do so, we'd
> > need to
> > handle config locking, sockets, pid file, passthrough resources?,
> > etc.
> > 
> > Another nice feature of this approach is that it doesn't require
> > touching the
> > vm_start or migration code at all, avoiding further bloating.
> > 
> > 
> > Thanks to Fabian G. and Stefan for inspiring this idea:
> > 
> > Fabian G. suggested using the suspend to disk + start route if the
> > required
> > changes to our stack would turn out to be infeasable.
> > 
> > Stefan suggested migrating to a dummy VM (outside our stack) which
> > just holds
> > the state and migrating back right away. It seems that dummy VM is
> > in fact not
> > even needed ;) If we really really care about smallest possible
> > downtime, this
> > approach might still be the best, and we'd need to start the dummy
> > VM while the
> > backwards migration runs (resulting in two times the migration
> > downtime). But
> > it does have more moving parts and requires some migration/startup
> > changes.
> > 
> > 
> > Fabian Ebner (6):
> >   create vmstate_size helper
> >   create savevm_monitor helper
> >   draft of upgrade_qemu function
> >   draft of qemuupgrade API call
> >   add timing for testing
> >   add usleep parameter to savevm_monitor
> > 
> >  PVE/API2/Qemu.pm  |  60 ++
> >  PVE/QemuConfig.pm |  10 +---
> >  PVE/QemuServer.pm | 125 +++---
> > 
> >  3 files changed, 170 insertions(+), 25 deletions(-)
> > 
> 
> 
> 
> ___
> 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] partially-applied: [PATCH-SERIES v7] APT repositories API/UI

2021-06-23 Thread Thomas Lamprecht
On 23.06.21 15:38, Fabian Ebner wrote:
> List the configured repositories and have some basic checks for them.
> Allow adding standard Proxmox repositories and enabling/disabling 
> repositories.
> 
> 
> Changes from v6:
> * do not include the upgrade functionality for now, focus on being
>   able to add standard repositories and enable/disable them
> * put impl blocks and struct declarations together in proxmox-apt
> * merge repositories and check_repositories get calls
> * see individual patches for more
> 
> Older changes can be found here:
> https://lists.proxmox.com/pipermail/pve-devel/2021-June/048598.html
> 
> 
> The dependency for pve-rs to proxmox-apt is already in the patches.
> Additionally, pve-manager depends on pve-rs and proxmox-widget-toolkit.
> 
> 
> proxmox-apt:
> 
> Fabian Ebner (5):
>   initial commit
>   add files for Debian packaging
>   add more functions to check repositories
>   add handling of Proxmox standard repositories
>   bump version to 0.2.0-1


> proxmox-widget-toolkit:
> 
> Fabian Ebner (2):
>   add UI for APT repositories
>   add buttons for add/enable/disable
> 
>  src/Makefile|   1 +
>  src/node/APTRepositories.js | 503 
>  2 files changed, 504 insertions(+)
>  create mode 100644 src/node/APTRepositories.js
> 

applied above, i.e., proxmox-apt and widget-toolkit patches, as those itself do 
not show up
anywhere and that should make it easier to adapt and test in collaboration - 
thanks!

FYI:  The widget-toolkit got some followups on top.


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



[pve-devel] applied: [PATCH storage] factoring out regex for vztmpl

2021-06-23 Thread Thomas Lamprecht
On 23.06.21 15:10, Lorenz Stechauner wrote:
> stores the regex definition in PVE::Storage.
> 
> One test had to be adapted because it tested obsolete code. Namely:
> it expects vztmpl to only end with .tar.gz, but the new regex also
> includes .tar.xz, there is nothing against allowing .tar.xz files as
> vztmpl files.
> 
> Signed-off-by: Lorenz Stechauner 
> ---
> updates "[PATCH v10 storage 1/3] factoring out regex'es for backup and vztmpl"
> changes:
> * only factor vztmpl_extension_re out
> 
>  PVE/API2/Storage/Status.pm | 6 +++---
>  PVE/Storage.pm | 4 +++-
>  PVE/Storage/Plugin.pm  | 4 ++--
>  test/path_to_volume_id_test.pm | 7 ---
>  4 files changed, 12 insertions(+), 9 deletions(-)
> 
>

applied, thanks!


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



[pve-devel] applied: [PATCH container 7/9] pct: correctly handle warnings task status

2021-06-23 Thread Thomas Lamprecht
On 12.05.21 14:32, Fabian Ebner wrote:
> Signed-off-by: Fabian Ebner 
> ---
> 
> Dependency bump for pve-common is needed.
> 
>  src/PVE/CLI/pct.pm | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
>

applied this already Monday, but forgot to reply.. any how, thanks!


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



[pve-devel] applied-partially: [PATCH v2 multiple] btrfs, file system for the brave

2021-06-23 Thread Thomas Lamprecht
applied all from pve-container, and all but 5/9 from pve-storage, Fabian 
reported
that he run into quite some issues using subvols, most of them exposed by that 
patch.


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



[pve-devel] applied: [PATCH storage 6/9] api: content: correctly handle warnings status for delayed task

2021-06-23 Thread Thomas Lamprecht
On 12.05.21 14:32, Fabian Ebner wrote:
> Signed-off-by: Fabian Ebner 
> ---
> 
> Dependency bump for pve-common is needed.
> 
>  PVE/API2/Storage/Content.pm | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
>

applied, but had to resolve a merge conflict due to changes in context, thanks!


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



[pve-devel] applied: [RFC docs] add a basic BTRFS section

2021-06-23 Thread Thomas Lamprecht
On 23.06.21 14:46, Wolfgang Bumiller wrote:
> Signed-off-by: Wolfgang Bumiller 
> ---
> Please give some feedback as to what else to add/change/remove.
> 
>  local-btrfs.adoc   | 177 +
>  pve-storage-btrfs.adoc |  55 +
>  pvesm.adoc |   1 +
>  sysadmin.adoc  |   2 +
>  4 files changed, 235 insertions(+)
>  create mode 100644 local-btrfs.adoc
>  create mode 100644 pve-storage-btrfs.adoc
> 
>

applied,  thanks!

FYI: splitted out mounting in its own section, added a very small new section 
about
how to add an exisiting FS to PVE (basically just an example for `pvesm add 
...`),
added BTRFS to the storage plugin overview table in pvesm.adoc and noted that 
this
all is a technology preview for now.


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



[pve-devel] applied: [PATCH v10 storage 2/3] status: factoring out normalize_content_filename

2021-06-23 Thread Thomas Lamprecht
On 22.06.21 11:19, Lorenz Stechauner wrote:
> Signed-off-by: Lorenz Stechauner 
> ---
>  PVE/API2/Storage/Status.pm |  6 +-
>  PVE/Storage.pm | 12 
>  2 files changed, 13 insertions(+), 5 deletions(-)
> 
>

applied, thanks!


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



[pve-devel] applied: [PATCH v10 storage 3/3] status: add download_url method

2021-06-23 Thread Thomas Lamprecht
On 22.06.21 11:19, Lorenz Stechauner wrote:
> uses common function PVE::Tools::download_file_from_url to download
> iso files.
> 
> Only users with permissions `Sys.Audit` and `Sys.Modify` on `/` are
> permitted to perform this action. This restriction is due to the
> fact, that the download function is able to download files from
> internal networks (which are not visible/accessible from outside).
> Users with these permissions anyway have the means to alter node
> (network) config, so this does not create any further security risk.
> 
> Signed-off-by: Lorenz Stechauner 
> ---
>  PVE/API2/Storage/Status.pm | 116 +
>  1 file changed, 116 insertions(+)
> 
> diff --git a/PVE/API2/Storage/Status.pm b/PVE/API2/Storage/Status.pm
> index 11ad60f..5395ddd 100644
> --- a/PVE/API2/Storage/Status.pm
> +++ b/PVE/API2/Storage/Status.pm
> @@ -222,6 +222,7 @@ __PACKAGE__->register_method ({
>  
>   my $res = [
>   { subdir => 'content' },
> + { subdir => 'download-url' },
>   { subdir => 'file-restore' },
>   { subdir => 'prunebackups' },
>   { subdir => 'rrd' },
> @@ -494,4 +495,119 @@ __PACKAGE__->register_method ({
>   return $upid;
> }});
>  
> +__PACKAGE__->register_method({
> +name => 'download_url',
> +path => '{storage}/download-url',
> +method => 'POST',
> +description => "Download templates and ISO images by using an URL.",
> +proxyto => 'node',
> +permissions => {
> + check => [ 'and',
> + ['perm', '/storage/{storage}', [ 'Datastore.AllocateTemplate' ]],
> + ['perm', '/', [ 'Sys.Audit', 'Sys.Modify' ]],
> + ],
> +},
> +protected => 1,
> +parameters => {
> + additionalProperties => 0,
> + properties => {
> + node => get_standard_option('pve-node'),
> + storage => get_standard_option('pve-storage-id'),
> + url => {
> + description => "The URL to download the file from.",
> + type => 'string',
> + pattern => 'https?://.*',
> + },
> + content => {
> + description => "Content type.",
> + type => 'string', format => 'pve-storage-content',
> + enum => ['iso', 'vztmpl'],
> + },
> + filename => {
> + description => "The name of the file to create. Caution: This 
> will be normalized!",

FYI: I added a maxLength to this with 255, quite some FS have that as limit
per file anyway (or had, it is generally quite long).

> + type => 'string',
> + },
> + checksum => {
> + description => "The expected checksum of the file.",
> + type => 'string',
> + requires => 'checksum-algorithm',
> + optional => 1,
> + },
> + 'checksum-algorithm' => {
> + description => "The algorithm to calculate the checksum of the 
> file.",
> + type => 'string',
> + enum => ['md5', 'sha1', 'sha224', 'sha256', 'sha384', 'sha512'],
> + requires => 'checksum',
> + optional => 1,
> + },
> + 'verify-certificates' => {
> + description => "If false, no SSL/TLS certificates will be 
> verified.",
> + type => 'boolean',
> + optional => 1,
> + default => 1,
> + }
> + },
> +},
> +returns => {
> + type => "string"
> +},
> +code => sub {
> + my ($param) = @_;
> +
> + my $rpcenv = PVE::RPCEnvironment::get();
> + my $user = $rpcenv->get_user();
> +
> + my $cfg = PVE::Storage::config();
> +
> + my ($node, $storage) = $param->@{'node', 'storage'};
> + my $scfg = PVE::Storage::storage_check_enabled($cfg, $storage, $node);
> +
> + die "can't upload to storage type '$scfg->{type}', not a file based 
> storage!\n"
> + if !defined($scfg->{path});
> +
> + my ($content, $url) = $param->@{'content', 'url'};
> +
> + die "storage '$storage' is not configured for content-type '$content'\n"
> + if !$scfg->{content}->{$content};
> +
> + my $filename = 
> PVE::Storage::normalize_content_filename($param->{filename});
> +
> + my $path;
> + if ($content eq 'iso') {
> + if ($filename !~ m![^/]+$PVE::Storage::iso_extension_re$!) {
> + raise_param_exc({ filename => "wrong file extension" });
> + }
> + $path = PVE::Storage::get_iso_dir($cfg, $storage);
> + } elsif ($content eq 'vztmpl') {
> + if ($filename !~ m![^/]+$PVE::Storage::vztmpl_extension_re$!) {
> + raise_param_exc({ filename => "wrong file extension" });
> + }
> + $path = PVE::Storage::get_vztmpl_dir($cfg, $storage);
> + } else {
> + raise_param_exc({ content => "upload content-type '$content' is not 
> allowed" });
> + }
> +
> + PVE::Storage::activate_storage($cfg, $storage);
> + File::Path::make_path($path);
> +
> + my $dccfg =