[pve-devel] [PATCH-SERIES] Some breaking API changes/cleanups

[pve-devel] [PATCH-SERIES] Some breaking API changes/cleanups

[Fabian Ebner]

Of course all of the changes need to be recorded in the release notes.
Should I do so now or wait until the changes are actually applied?

Dependencies are noted on individual patches, most should apply independently.

There's still quite a few outstanding, but I feel like somebody with more
experience/context would be better suited to address them. I can go for another
round though if there are not enough volunteers ;)


pve-access-control:
src/PVE/Auth/AD.pm
30:         description => "Use secure LDAPS protocol. DEPRECATED: use 'mode' instead.",
src/PVE/Auth/LDAP.pm
390:    # FIXME: remove fallback with 7.0 by doing a rename on upgrade from 6.x

src/PVE/AccessControl.pm
299:    # FIXME: remove with PVE 7 and/or refactor all into PVE::Ticket ?

pve-common:
src/PVE/Ticket.pm
38:         # detected sha1 csrf token from older proxy, fallback. FIXME: remove with 7.0

pve-cluster:
quite a few // FIXME: remove openvz stuff for 7.x

pve-storage:
PVE/Storage/CephFSPlugin.pm
41:# FIXME: remove in PVE 7.0 where systemd is recent enough to not have those
# local-fs/remote-fs dependency cycles generated for _netdev mounts...


Would be glad if somebody experienced in those areas could look at them.



data/PVE/DataCenterConfig.pm has a few workarounds for deprecated options

pve-container:
src/PVE/LXC/Config.pm
652:    # Deprecated (removed with lxc 3.0):

src/PVE/LXC.pm
2178:   # FIXME: remove the 'id_map' variant when lxc-3.0 arrives


There might still be lxc/datacenter configs with the old options around?
Do we want to warn/replace as part of the pve6to7 script or just keep
the compat code longer?



pve-manager:
PVE/Ceph/Services.pm
54:     # FIXME: remove with 7.0 - for backward compat only

PVE/API2/Cluster/Ceph.pm
78:         # FIXME: remove with 7.0 depreacated by structured 'versions'


Front-end still expects the metadata.version to be present. Might not be hard to
adapt, but I didn't have the context to immediately know what's the right thing
to do. Can look at it if nobody else wants to.



pve-storage:
PVE/Storage.pm
966:    # FIXME PVE 7.0: only scan storages with the correct content types


From an off-list discussion with Fabian G: We currently still need this behavior
to pick up disks from storages with a misconfigured content types. Meaning that
we need to require content type 'images' to be set first and warn people with
misconfigured storages as part of pve6to7. We could even go further and not pick
up unreferenced disks at all, but should expose rescan in the UI and make a
rescan as part of pve6to7. I will look at this in a separate series.



pve-guest-common:

Fabian Ebner (2):
  vzdump: remove deprecated size parameter
  vzdump: defaults: keep all backups by default for 7.0

 src/PVE/VZDump/Common.pm | 17 +++++------------
 1 file changed, 5 insertions(+), 12 deletions(-)


pve-container:

Fabian Ebner (1):
  migrate: remove deprecated force parameter

 src/PVE/API2/LXC.pm    | 6 ------
 src/PVE/LXC/Migrate.pm | 8 +-------
 2 files changed, 1 insertion(+), 13 deletions(-)


qemu-server:

Fabian Ebner (3):
  Revert "revert spice_ticket prefix change in 7827de4"
  scan volids: remove superfluous parameter
  vm destroy: do not remove unreferenced disks by default

 PVE/API2/Qemu.pm   | 5 ++---
 PVE/QemuMigrate.pm | 5 ++---
 PVE/QemuServer.pm  | 7 +++----
 3 files changed, 7 insertions(+), 10 deletions(-)


pve-storage:

Fabian Ebner (3):
  postinst: move cifs credential files into subdirectory upon update
  update reminder to remove maxfiles
  api: get rid of moved 'usb' call

 PVE/API2/Storage/Scan.pm  | 45 ---------------------------------------
 PVE/Storage.pm            |  2 +-
 PVE/Storage/CIFSPlugin.pm |  3 ---
 debian/postinst           | 30 ++++++++++++++++++++++++++
 4 files changed, 31 insertions(+), 49 deletions(-)
 create mode 100644 debian/postinst


novnc-pve:

Fabian Ebner (1):
  avoid passing deprecated 'upgrade' parameter

 ...passing-deprecated-upgrade-parameter.patch | 24 +++++++++++++++++++
 debian/patches/series                         |  1 +
 2 files changed, 25 insertions(+)
 create mode 100644 debian/patches/0014-avoid-passing-deprecated-upgrade-parameter.patch

pve-manager:

Fabian Ebner (12):
  vzdump: remove deprecated size parameter
  configs: vzdump: use prune-backups instead of the deprecated maxfiles
  test: vzdump: adapt to new default
  Revert "VZDump: add TARFILE to environment for hookscripts"
  api: nodes: remove deprecated upgrade parameter for xtermjs
  api: nodes: remove deprecated upgrade parameter for spiceshell
  api: nodes: remove deprecated upgrade parameter for vncshell
  api: move cpu path into qemu/capabilities
  api: ceph: remove obsoleted disks call
  api: ceph: remove moved 'flags' calls
  api: ceph: osd: create: rename size parameters
  cli: pveceph: remove outdated comment

 PVE/API2/Capabilities.pm              |   6 +
 PVE/API2/Ceph.pm                      | 178 --------------------------
 PVE/API2/Ceph/OSD.pm                  |   8 +-
 PVE/API2/Nodes.pm                     |  48 ++-----
 PVE/API2/VZDump.pm                    |   2 -
 PVE/CLI/pveceph.pm                    |   1 -
 PVE/VZDump.pm                         |   6 -
 configs/vzdump.conf                   |   3 +-
 test/vzdump_new_test.pl               |   8 +-
 www/manager6/ceph/OSD.js              |   8 +-
 www/manager6/form/CPUModelSelector.js |   2 +-
 11 files changed, 27 insertions(+), 243 deletions(-)

-- 
2.30.2

[pve-devel] [PATCH guest-common 1/2] vzdump: remove deprecated size parameter

[Fabian Ebner]

It was deprecated for a long time (before it got move to guest-common) already,
and there also was a deprecation warning when passed as a CLI option.

Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
---
 src/PVE/VZDump/Common.pm | 8 --------
 1 file changed, 8 deletions(-)

diff --git a/src/PVE/VZDump/Common.pm b/src/PVE/VZDump/Common.pm
index bc38343..f325850 100644
--- a/src/PVE/VZDump/Common.pm
+++ b/src/PVE/VZDump/Common.pm
@@ -181,14 +181,6 @@ my $confdesc = {
 	optional => 1,
 	default => 0,
     },
-    # FIXME: Remove with PVE 7.0
-    size => {
-	type => 'integer',
-	description => "Unused, will be removed in a future release.",
-	optional => 1,
-	minimum => 500,
-	default => 1024,
-    },
     bwlimit => {
 	type => 'integer',
 	description => "Limit I/O bandwidth (KBytes per second).",
-- 
2.30.2

[pve-devel] [PATCH/RFC guest-common 2/2] vzdump: defaults: keep all backups by default for 7.0

[Fabian Ebner]

and switch to using prune-backups instead of maxfiles.

Storages created via the web UI defaulted to keeping all backups already, switch
to this safer default here as well.

Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
---

Breaks build in pve-manager, because the tests there need to be adapted,
see patch #3 for manager.

 src/PVE/VZDump/Common.pm | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/src/PVE/VZDump/Common.pm b/src/PVE/VZDump/Common.pm
index f325850..83d7413 100644
--- a/src/PVE/VZDump/Common.pm
+++ b/src/PVE/VZDump/Common.pm
@@ -210,21 +210,22 @@ my $confdesc = {
 	minimum => 0,
 	default => 10, # 10 minutes
     },
+    # FIXME remove with PVE 8.0 or PVE 9.0
     maxfiles => {
 	type => 'integer',
-	description => "Maximal number of backup files per guest system.",
+	description => "Deprecated: use 'prune-backups' instead. " .
+	    "Maximal number of backup files per guest system.",
 	optional => 1,
 	minimum => 1,
-	default => 1,
     },
     'prune-backups' => get_standard_option('prune-backups', {
 	description => "Use these retention options instead of those from the storage configuration.",
 	optional => 1,
+	default => "keep-all=1",
     }),
     remove => {
 	type => 'boolean',
-	description => "Remove old backup files if there are more than " .
-	    "'maxfiles' backup files or prune according to 'prune-backups'.",
+	description => "Prune older backups according to 'prune-backups'.",
 	optional => 1,
 	default => 1,
     },
-- 
2.30.2

[pve-devel] [PATCH container 1/1] migrate: remove deprecated force parameter

[Fabian Ebner]

Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
---
 src/PVE/API2/LXC.pm    | 6 ------
 src/PVE/LXC/Migrate.pm | 8 +-------
 2 files changed, 1 insertion(+), 13 deletions(-)

diff --git a/src/PVE/API2/LXC.pm b/src/PVE/API2/LXC.pm
index 4965f5d..936debb 100644
--- a/src/PVE/API2/LXC.pm
+++ b/src/PVE/API2/LXC.pm
@@ -1076,12 +1076,6 @@ __PACKAGE__->register_method({
 		optional => 1,
 		default => 180,
 	    },
-	    force => {
-		type => 'boolean',
-		description => "Force migration despite local bind / device" .
-		    " mounts. NOTE: deprecated, use 'shared' property of mount point instead.",
-		optional => 1,
-	    },
 	    bwlimit => {
 		description => "Override I/O bandwidth limit (in KiB/s).",
 		optional => 1,
diff --git a/src/PVE/LXC/Migrate.pm b/src/PVE/LXC/Migrate.pm
index cb1ea7a..b5917e9 100644
--- a/src/PVE/LXC/Migrate.pm
+++ b/src/PVE/LXC/Migrate.pm
@@ -44,20 +44,14 @@ sub prepare {
     }
     $self->{was_running} = $running;
 
-    my $force = $self->{opts}->{force} // 0;
-
     PVE::LXC::Config->foreach_volume($conf, sub {
 	my ($ms, $mountpoint) = @_;
 
 	my $volid = $mountpoint->{volume};
 	my $type = $mountpoint->{type};
 
-	# skip dev/bind mps when forced / shared
+	# skip dev/bind mps when shared
 	if ($type ne 'volume') {
-	    if ($force) {
-		warn "-force is deprecated, please use the 'shared' property on individual non-volume mount points instead!\n";
-		return;
-	    }
 	    if ($mountpoint->{shared}) {
 		return;
 	    } else {
-- 
2.30.2

[pve-devel] [PATCH qemu-server 1/3] Revert "revert spice_ticket prefix change in 7827de4"

[Fabian Ebner]

This reverts commit ff09c795edd12b1cc4604ee28a7cdd99cdd1afa8. We wanted to wait
until PVE 7.0 for the change to not break migration new -> old until then.

Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
---
 PVE/QemuMigrate.pm | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/PVE/QemuMigrate.pm b/PVE/QemuMigrate.pm
index 0368a6d..6375a15 100644
--- a/PVE/QemuMigrate.pm
+++ b/PVE/QemuMigrate.pm
@@ -839,9 +839,8 @@ sub phase2 {
     my $unix_socket_info = {};
     # version > 0 for unix socket support
     my $nbd_protocol_version = 1;
-    # TODO change to 'spice_ticket: <ticket>\n' in 7.0
-    my $input = $spice_ticket ? "$spice_ticket\n" : "\n";
-    $input .= "nbd_protocol_version: $nbd_protocol_version\n";
+    my $input = "nbd_protocol_version: $nbd_protocol_version\n";
+    $input .= "spice_ticket: $spice_ticket\n" if $spice_ticket;
 
     my @online_replicated_volumes = $self->filter_local_volumes('online', 1);
     foreach my $volid (@online_replicated_volumes) {
-- 
2.30.2

Re: [pve-devel] [PATCH qemu-server 1/3] Revert "revert spice_ticket prefix change in 7827de4"

[Stefan Reiter]

For the three qemu-server changes:

Reviewed-by: Stefan Reiter <s.reiter@proxmox.com>

I'll also see if I can find some more stuff to cleanup in the code I 
often traverse :)

On 6/4/21 3:49 PM, Fabian Ebner wrote:
> This reverts commit ff09c795edd12b1cc4604ee28a7cdd99cdd1afa8. We wanted to wait
> until PVE 7.0 for the change to not break migration new -> old until then.
> 
> Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
> ---
>   PVE/QemuMigrate.pm | 5 ++---
>   1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/PVE/QemuMigrate.pm b/PVE/QemuMigrate.pm
> index 0368a6d..6375a15 100644
> --- a/PVE/QemuMigrate.pm
> +++ b/PVE/QemuMigrate.pm
> @@ -839,9 +839,8 @@ sub phase2 {
>       my $unix_socket_info = {};
>       # version > 0 for unix socket support
>       my $nbd_protocol_version = 1;
> -    # TODO change to 'spice_ticket: <ticket>\n' in 7.0
> -    my $input = $spice_ticket ? "$spice_ticket\n" : "\n";
> -    $input .= "nbd_protocol_version: $nbd_protocol_version\n";
> +    my $input = "nbd_protocol_version: $nbd_protocol_version\n";
> +    $input .= "spice_ticket: $spice_ticket\n" if $spice_ticket;
>   
>       my @online_replicated_volumes = $self->filter_local_volumes('online', 1);
>       foreach my $volid (@online_replicated_volumes) {
> 

[pve-devel] [PATCH qemu-server 2/3] scan volids: remove superfluous parameter

[Fabian Ebner]

The only caller that didn't use 'images' was removed as part of the migration
refactoring in commit 62a4c963b824c923a4fc82a48c81d0f63ebaddae, so this is not
even a breaking change as the 'PVE 7' comment might've suggested.

Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
---
 PVE/QemuServer.pm | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
index 25ac052..d77981c 100644
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -6091,11 +6091,10 @@ my $restore_destroy_volumes = sub {
     }
 };
 
-# FIXME For PVE 7.0, remove $content_type and always use 'images'
 sub scan_volids {
-    my ($cfg, $vmid, $content_type) = @_;
+    my ($cfg, $vmid) = @_;
 
-    my $info = PVE::Storage::vdisk_list($cfg, undef, $vmid, undef, $content_type);
+    my $info = PVE::Storage::vdisk_list($cfg, undef, $vmid, undef, 'images');
 
     my $volid_hash = {};
     foreach my $storeid (keys %$info) {
@@ -6189,7 +6188,7 @@ sub rescan {
     my $cfg = PVE::Storage::config();
 
     print "rescan volumes...\n";
-    my $volid_hash = scan_volids($cfg, $vmid, 'images');
+    my $volid_hash = scan_volids($cfg, $vmid);
 
     my $updatefn =  sub {
 	my ($vmid) = @_;
-- 
2.30.2

[pve-devel] [PATCH qemu-server 3/3] vm destroy: do not remove unreferenced disks by default

[Fabian Ebner]

Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
---
 PVE/API2/Qemu.pm | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
index 24dba86..bc313f9 100644
--- a/PVE/API2/Qemu.pm
+++ b/PVE/API2/Qemu.pm
@@ -1592,7 +1592,7 @@ __PACKAGE__->register_method({
 		description => "If set, destroy additionally all disks not referenced in the config"
 		 ." but with a matching VMID from all enabled storages.",
 		optional => 1,
-		default => 1, # FIXME: replace to false in PVE 7.0, this is dangerous!
+		default => 0,
 	    },
 	},
     },
@@ -1643,8 +1643,7 @@ __PACKAGE__->register_method({
 		# repeat, config might have changed
 		my $ha_managed = $early_checks->();
 
-		# FIXME: drop fallback to true with 7.0, to dangerous for default
-		my $purge_unreferenced = $param->{'destroy-unreferenced-disks'} // 1;
+		my $purge_unreferenced = $param->{'destroy-unreferenced-disks'};
 
 		PVE::QemuServer::destroy_vm(
 		    $storecfg,
-- 
2.30.2

[pve-devel] [PATCH/RFC storage 1/3] postinst: move cifs credential files into subdirectory upon update

[Fabian Ebner]

and drop the compat code.

Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
---
 PVE/Storage/CIFSPlugin.pm |  3 ---
 debian/postinst           | 30 ++++++++++++++++++++++++++++++
 2 files changed, 30 insertions(+), 3 deletions(-)
 create mode 100644 debian/postinst

diff --git a/PVE/Storage/CIFSPlugin.pm b/PVE/Storage/CIFSPlugin.pm
index be06cc7..9d69b01 100644
--- a/PVE/Storage/CIFSPlugin.pm
+++ b/PVE/Storage/CIFSPlugin.pm
@@ -59,9 +59,6 @@ sub get_cred_file {
 
     if (-e $cred_file) {
 	return $cred_file;
-    } elsif (-e "/etc/pve/priv/${storeid}.cred") {
-	# FIXME: remove fallback with 7.0 by doing a rename on upgrade from 6.x
-	return "/etc/pve/priv/${storeid}.cred";
     }
     return undef;
 }
diff --git a/debian/postinst b/debian/postinst
new file mode 100644
index 0000000..e2453d2
--- /dev/null
+++ b/debian/postinst
@@ -0,0 +1,30 @@
+#!/bin/sh
+
+set -e
+
+#DEBHELPER#
+
+case "$1" in
+  configure)
+    if test -n "$2"; then
+
+        # TODO: remove once PVE 7.0 is released
+        if dpkg --compare-versions "$2" 'lt' '7.0-3'; then
+            for file in /etc/pve/priv/*.cred; do
+                if [ -f "$file" ]; then
+                    base=$(basename --suffix=".cred" "$file")
+                    if [ -f "/etc/pve/priv/storage/$base.pw" ]; then
+                        echo "Warning: found obsolete CIFS credentials file: $file" >&2
+                        echo "File: /etc/pve/priv/storage/$base.pw also exists" >&2
+                    else
+                        cp "$file" "/etc/pve/priv/storage/$base.pw" && rm "$file"
+                    fi
+                fi
+            done
+        fi
+    fi
+    ;;
+
+esac
+
+exit 0
-- 
2.30.2

Re: [pve-devel] [PATCH/RFC storage 1/3] postinst: move cifs credential files into subdirectory upon update

[Fabian Grünbichler]

On June 4, 2021 3:49 pm, Fabian Ebner wrote:
> and drop the compat code.
> 
> Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
> ---
>  PVE/Storage/CIFSPlugin.pm |  3 ---
>  debian/postinst           | 30 ++++++++++++++++++++++++++++++
>  2 files changed, 30 insertions(+), 3 deletions(-)
>  create mode 100644 debian/postinst
> 
> diff --git a/PVE/Storage/CIFSPlugin.pm b/PVE/Storage/CIFSPlugin.pm
> index be06cc7..9d69b01 100644
> --- a/PVE/Storage/CIFSPlugin.pm
> +++ b/PVE/Storage/CIFSPlugin.pm
> @@ -59,9 +59,6 @@ sub get_cred_file {
>  
>      if (-e $cred_file) {
>  	return $cred_file;
> -    } elsif (-e "/etc/pve/priv/${storeid}.cred") {
> -	# FIXME: remove fallback with 7.0 by doing a rename on upgrade from 6.x
> -	return "/etc/pve/priv/${storeid}.cred";
>      }
>      return undef;
>  }
> diff --git a/debian/postinst b/debian/postinst
> new file mode 100644
> index 0000000..e2453d2
> --- /dev/null
> +++ b/debian/postinst
> @@ -0,0 +1,30 @@
> +#!/bin/sh
> +
> +set -e
> +
> +#DEBHELPER#
> +
> +case "$1" in
> +  configure)
> +    if test -n "$2"; then
> +
> +        # TODO: remove once PVE 7.0 is released

s/7/8/ ?

because we need to do this renaming on all upgrades of 6.x to 7.x, no 
matter how far along in the 7.x release we are.. for the upgrade from 
7.x to 8.x we can then be sure that the only such files are leftover 
garbage ;)

> +        if dpkg --compare-versions "$2" 'lt' '7.0-3'; then
> +            for file in /etc/pve/priv/*.cred; do
> +                if [ -f "$file" ]; then
> +                    base=$(basename --suffix=".cred" "$file")
> +                    if [ -f "/etc/pve/priv/storage/$base.pw" ]; then
> +                        echo "Warning: found obsolete CIFS credentials file: $file" >&2
> +                        echo "File: /etc/pve/priv/storage/$base.pw also exists" >&2
> +                    else
> +                        cp "$file" "/etc/pve/priv/storage/$base.pw" && rm "$file"

why not 'mv'?

should we maybe print an info in both cases? e.g.

Warning: found possibly obsolete CIFS credentials file '$file', but '...' already exists.

and

Info: found CIFS credentials file '$file', moving to '...'.

> +                    fi
> +                fi
> +            done
> +        fi
> +    fi
> +    ;;
> +
> +esac
> +
> +exit 0
> -- 
> 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/RFC storage 1/3] postinst: move cifs credential files into subdirectory upon update

[Fabian Ebner]

Am 08.06.21 um 15:18 schrieb Fabian Grünbichler:
> On June 4, 2021 3:49 pm, Fabian Ebner wrote:
>> and drop the compat code.
>>
>> Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
>> ---
>>   PVE/Storage/CIFSPlugin.pm |  3 ---
>>   debian/postinst           | 30 ++++++++++++++++++++++++++++++
>>   2 files changed, 30 insertions(+), 3 deletions(-)
>>   create mode 100644 debian/postinst
>>
>> diff --git a/PVE/Storage/CIFSPlugin.pm b/PVE/Storage/CIFSPlugin.pm
>> index be06cc7..9d69b01 100644
>> --- a/PVE/Storage/CIFSPlugin.pm
>> +++ b/PVE/Storage/CIFSPlugin.pm
>> @@ -59,9 +59,6 @@ sub get_cred_file {
>>   
>>       if (-e $cred_file) {
>>   	return $cred_file;
>> -    } elsif (-e "/etc/pve/priv/${storeid}.cred") {
>> -	# FIXME: remove fallback with 7.0 by doing a rename on upgrade from 6.x
>> -	return "/etc/pve/priv/${storeid}.cred";
>>       }
>>       return undef;
>>   }
>> diff --git a/debian/postinst b/debian/postinst
>> new file mode 100644
>> index 0000000..e2453d2
>> --- /dev/null
>> +++ b/debian/postinst
>> @@ -0,0 +1,30 @@
>> +#!/bin/sh
>> +
>> +set -e
>> +
>> +#DEBHELPER#
>> +
>> +case "$1" in
>> +  configure)
>> +    if test -n "$2"; then
>> +
>> +        # TODO: remove once PVE 7.0 is released
> 
> s/7/8/ ?
> 
> because we need to do this renaming on all upgrades of 6.x to 7.x, no
> matter how far along in the 7.x release we are.. for the upgrade from
> 7.x to 8.x we can then be sure that the only such files are leftover
> garbage ;)
> 

Right, it's not like everybody has to go through a specific version.

>> +        if dpkg --compare-versions "$2" 'lt' '7.0-3'; then
>> +            for file in /etc/pve/priv/*.cred; do
>> +                if [ -f "$file" ]; then
>> +                    base=$(basename --suffix=".cred" "$file")
>> +                    if [ -f "/etc/pve/priv/storage/$base.pw" ]; then
>> +                        echo "Warning: found obsolete CIFS credentials file: $file" >&2
>> +                        echo "File: /etc/pve/priv/storage/$base.pw also exists" >&2
>> +                    else
>> +                        cp "$file" "/etc/pve/priv/storage/$base.pw" && rm "$file"
> 
> why not 'mv'?
> 

Bad muscle memory. I got used to this for other scenarios, but since 
it's only a single file and no file-system boundaries are involved here, 
mv is of course better.

> should we maybe print an info in both cases? e.g.
> 
> Warning: found possibly obsolete CIFS credentials file '$file', but '...' already exists.
> 
> and
> 
> Info: found CIFS credentials file '$file', moving to '...'.
> 

Ok, sure.

>> +                    fi
>> +                fi
>> +            done
>> +        fi
>> +    fi
>> +    ;;
>> +
>> +esac
>> +
>> +exit 0
>> -- 
>> 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 storage 2/3] update reminder to remove maxfiles

[Fabian Ebner]

Remove it later, once it's been deprecated for a while.

Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
---
 PVE/Storage.pm | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/PVE/Storage.pm b/PVE/Storage.pm
index aa36bad..0b3671a 100755
--- a/PVE/Storage.pm
+++ b/PVE/Storage.pm
@@ -125,7 +125,7 @@ sub lock_storage_config {
     }
 }
 
-# FIXME remove maxfiles for PVE 7.0
+# FIXME remove maxfiles for PVE 8.0 or PVE 9.0
 my $convert_maxfiles_to_prune_backups = sub {
     my ($scfg) = @_;
 
-- 
2.30.2

Re: [pve-devel] [PATCH storage 2/3] update reminder to remove maxfiles

[Fabian Grünbichler]

On June 4, 2021 3:49 pm, Fabian Ebner wrote:
> Remove it later, once it's been deprecated for a while.

shouldn't we start warning that it is deprecated? or at least add a 
deprecation notice to the parameter description?

> 
> Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
> ---
>  PVE/Storage.pm | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/PVE/Storage.pm b/PVE/Storage.pm
> index aa36bad..0b3671a 100755
> --- a/PVE/Storage.pm
> +++ b/PVE/Storage.pm
> @@ -125,7 +125,7 @@ sub lock_storage_config {
>      }
>  }
>  
> -# FIXME remove maxfiles for PVE 7.0
> +# FIXME remove maxfiles for PVE 8.0 or PVE 9.0
>  my $convert_maxfiles_to_prune_backups = sub {
>      my ($scfg) = @_;
>  
> -- 
> 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 storage 2/3] update reminder to remove maxfiles

[Thomas Lamprecht]

On 08.06.21 15:04, Fabian Grünbichler wrote:
> On June 4, 2021 3:49 pm, Fabian Ebner wrote:
>> Remove it later, once it's been deprecated for a while.
> 
> shouldn't we start warning that it is deprecated? or at least add a 
> deprecation notice to the parameter description?
> 

that can be done in 8.0 or so, it's a really cheap translation which has
zero cost to do

Re: [pve-devel] [PATCH storage 2/3] update reminder to remove maxfiles

[Fabian Ebner]

Am 08.06.21 um 18:23 schrieb Thomas Lamprecht:
> On 08.06.21 15:04, Fabian Grünbichler wrote:
>> On June 4, 2021 3:49 pm, Fabian Ebner wrote:
>>> Remove it later, once it's been deprecated for a while.
>>
>> shouldn't we start warning that it is deprecated? or at least add a
>> deprecation notice to the parameter description?
>>
> 
> that can be done in 8.0 or so, it's a really cheap translation which has
> zero cost to do
> 

Warning about it now might be a bit noisy. The parameter is 
auto-converted on read, so once we are at 8.0, it might be gone from 
many setups already. But sure, mentioning the deprecation status in the 
parameter description should be done (not only in the vzdump schema...). 
Will send it in a followup or v2.

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

[pve-devel] [PATCH storage 3/3] api: get rid of moved 'usb' call

[Fabian Ebner]

pve-manger commit bd328734deb1dcea296858bb38d085e392adb99e changed the frontend
to use the new call.

Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
---
 PVE/API2/Storage/Scan.pm | 45 ----------------------------------------
 1 file changed, 45 deletions(-)

diff --git a/PVE/API2/Storage/Scan.pm b/PVE/API2/Storage/Scan.pm
index 15b39b5..d7a8743 100644
--- a/PVE/API2/Storage/Scan.pm
+++ b/PVE/API2/Storage/Scan.pm
@@ -48,8 +48,6 @@ __PACKAGE__->register_method({
 	    { method => 'lvm' },
 	    { method => 'nfs' },
 	    { method => 'pbs' },
-	    # FIXME: remove with 7.0 (replaced by /nodes/<node>/hardware/usb)
-	    { method => 'usb' },
 	    { method => 'zfs' },
 	];
 
@@ -448,47 +446,4 @@ __PACKAGE__->register_method({
 	return PVE::Storage::scan_zfs();
     }});
 
-# FIXME: remove with 7.0 (replaced by /nodes/<node>/hardware/usb)
-__PACKAGE__->register_method({
-    name => 'usbscan',
-    path => 'usb',
-    method => 'GET',
-    description => "List local USB devices.",
-    protected => 1,
-    proxyto => "node",
-    permissions => {
-	check => ['perm', '/', ['Sys.Modify']],
-    },
-    parameters => {
-	additionalProperties => 0,
-	properties => {
-	    node => get_standard_option('pve-node'),
-	},
-    },
-    returns => {
-	type => 'array',
-	items => {
-	    type => "object",
-	    properties => {
-		busnum => { type => 'integer'},
-		class => { type => 'integer'},
-		devnum => { type => 'integer'},
-		level => { type => 'integer'},
-		manufacturer => { type => 'string', optional => 1 },
-		port => { type => 'integer'},
-		prodid => { type => 'string'},
-		product => { type => 'string', optional => 1 },
-		serial => { type => 'string', optional => 1 },
-		speed => { type => 'string'},
-		usbpath => { type => 'string', optional => 1},
-		vendid => { type => 'string'},
-	    },
-	},
-    },
-    code => sub {
-	my ($param) = @_;
-
-	return PVE::SysFSTools::scan_usb();
-    }});
-
 1;
-- 
2.30.2

[pve-devel] [PATCH novnc-pve 1/1] avoid passing deprecated 'upgrade' parameter

[Fabian Ebner]

Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
---
 ...passing-deprecated-upgrade-parameter.patch | 24 +++++++++++++++++++
 debian/patches/series                         |  1 +
 2 files changed, 25 insertions(+)
 create mode 100644 debian/patches/0014-avoid-passing-deprecated-upgrade-parameter.patch

diff --git a/debian/patches/0014-avoid-passing-deprecated-upgrade-parameter.patch b/debian/patches/0014-avoid-passing-deprecated-upgrade-parameter.patch
new file mode 100644
index 0000000..3e03c2a
--- /dev/null
+++ b/debian/patches/0014-avoid-passing-deprecated-upgrade-parameter.patch
@@ -0,0 +1,24 @@
+From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
+From: Fabian Ebner <f.ebner@proxmox.com>
+Date: Fri, 4 Jun 2021 08:27:23 +0200
+Subject: [PATCH] avoid passing deprecated 'upgrade' parameter
+
+Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
+---
+ app/pve.js | 2 +-
+ 1 file changed, 1 insertion(+), 1 deletion(-)
+
+diff --git a/app/pve.js b/app/pve.js
+index 9cf4b09..0ab825a 100644
+--- a/app/pve.js
++++ b/app/pve.js
+@@ -45,7 +45,7 @@ export default function PVEUI(UI){
+ 	    break;
+ 	case 'upgrade':
+ 	    url =  baseUrl + '/vncshell';
+-	    params.upgrade = 1;
++	    params.cmd = 'upgrade';
+ 	    title = 'System upgrade on node ' + this.nodename;
+ 	    break;
+ 	case 'cmd':
+
diff --git a/debian/patches/series b/debian/patches/series
index 0d32df5..217c61c 100644
--- a/debian/patches/series
+++ b/debian/patches/series
@@ -11,3 +11,4 @@
 0011-add-localCursor-setting-to-rfb.patch
 0012-pass-custom-command-to-vnc.patch
 0013-Revert-Remove-the-default-value-of-wsProtocols.patch
+0014-avoid-passing-deprecated-upgrade-parameter.patch
-- 
2.30.2

[pve-devel] [PATCH manager 01/12] vzdump: remove deprecated size parameter

[Fabian Ebner]

Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
---
 PVE/API2/VZDump.pm  | 2 --
 PVE/VZDump.pm       | 4 ----
 configs/vzdump.conf | 1 -
 3 files changed, 7 deletions(-)

diff --git a/PVE/API2/VZDump.pm b/PVE/API2/VZDump.pm
index 4093d82f..10847260 100644
--- a/PVE/API2/VZDump.pm
+++ b/PVE/API2/VZDump.pm
@@ -230,8 +230,6 @@ __PACKAGE__->register_method ({
 	    delete $res->{pool};
 	}
 
-	delete $res->{size}; # deprecated, to be dropped with PVE 7.0
-
 	return $res;
     }});
 
diff --git a/PVE/VZDump.pm b/PVE/VZDump.pm
index 40ce01dd..8213ed66 100644
--- a/PVE/VZDump.pm
+++ b/PVE/VZDump.pm
@@ -1241,10 +1241,6 @@ sub verify_vzdump_parameters {
 
     $param->{all} = 1 if (defined($param->{exclude}) && !$param->{pool});
 
-    warn "option 'size' is deprecated and will be removed in a future " .
-	 "release, please update your script/configuration!\n"
-	if defined($param->{size});
-
     return if !$check_missing;
 
     raise_param_exc({ vmid => "property is missing"})
diff --git a/configs/vzdump.conf b/configs/vzdump.conf
index f69b69c4..d14942dd 100644
--- a/configs/vzdump.conf
+++ b/configs/vzdump.conf
@@ -8,7 +8,6 @@
 #ionice: PRI
 #lockwait: MINUTES
 #stopwait: MINUTES
-#size: MB
 #stdexcludes: BOOLEAN
 #mailto: ADDRESSLIST
 #maxfiles: N
-- 
2.30.2

[pve-devel] [PATCH manager 02/12] configs: vzdump: use prune-backups instead of the deprecated maxfiles

[Fabian Ebner]

Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
---
 configs/vzdump.conf | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/configs/vzdump.conf b/configs/vzdump.conf
index d14942dd..a0075cfa 100644
--- a/configs/vzdump.conf
+++ b/configs/vzdump.conf
@@ -10,7 +10,7 @@
 #stopwait: MINUTES
 #stdexcludes: BOOLEAN
 #mailto: ADDRESSLIST
-#maxfiles: N
+#prune-backups: keep-INTERVAL=N[,...]
 #script: FILENAME
 #exclude-path: PATHLIST
 #pigz: N
-- 
2.30.2

[pve-devel] [PATCH manager 03/12] test: vzdump: adapt to new default

[Fabian Ebner]

Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
---

Build-depends on the new default in pve-guest-common

 test/vzdump_new_test.pl | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/test/vzdump_new_test.pl b/test/vzdump_new_test.pl
index a8f70d62..23676239 100755
--- a/test/vzdump_new_test.pl
+++ b/test/vzdump_new_test.pl
@@ -92,9 +92,9 @@ my @tests = (
 	description => 'no params',
 	expected => {
 	    'prune-backups' => {
-		'keep-last' => 1,
+		'keep-all' => 1,
 	    },
-	    remove => 1,
+	    remove => 0,
 	},
     },
     # TODO make parse error critical?
@@ -105,9 +105,9 @@ my @tests = (
 	},
 	expected => {
 	    'prune-backups' => {
-		'keep-last' => 1,
+		'keep-all' => 1,
 	    },
-	    remove => 1,
+	    remove => 0,
 	},
     },
     {
-- 
2.30.2

[pve-devel] [PATCH manager 04/12] Revert "VZDump: add TARFILE to environment for hookscripts"

[Fabian Ebner]

This reverts commit b3dec633d07e992d626e4c1878dbfa72ac30027d.

Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
---
 PVE/VZDump.pm | 2 --
 1 file changed, 2 deletions(-)

diff --git a/PVE/VZDump.pm b/PVE/VZDump.pm
index 8213ed66..e6a3a16f 100644
--- a/PVE/VZDump.pm
+++ b/PVE/VZDump.pm
@@ -652,8 +652,6 @@ sub run_hook_script {
     foreach my $ek (qw(vmtype hostname target logfile)) {
 	$ENV{uc($ek)} = $task->{$ek} if $task->{$ek};
     }
-    # FIXME: for backwards compatibility - drop with PVE 7.0
-    $ENV{TARFILE} = $task->{target} if $task->{target};
 
     run_command ($logfd, $cmd);
 }
-- 
2.30.2

[pve-devel] [PATCH manager 05/12] api: nodes: remove deprecated upgrade parameter for xtermjs

[Fabian Ebner]

The switch to 'cmd' was made by commit af39a6f09651e15d1c83536e25493a2212efd7d3
in the pve-xtermjs repo and is included in 4.7.0

Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
---

Dependency bump on pve-xtermjs needed.

 PVE/API2/Nodes.pm | 10 ----------
 1 file changed, 10 deletions(-)

diff --git a/PVE/API2/Nodes.pm b/PVE/API2/Nodes.pm
index e58d9c10..36cb70b4 100644
--- a/PVE/API2/Nodes.pm
+++ b/PVE/API2/Nodes.pm
@@ -1053,12 +1053,6 @@ __PACKAGE__->register_method ({
 	additionalProperties => 0,
 	properties => {
 	    node => get_standard_option('pve-node'),
-	    upgrade => {
-		type => 'boolean',
-		description => "Deprecated, use the 'cmd' property instead! Run 'apt-get dist-upgrade' instead of normal shell.",
-		optional => 1,
-		default => 0,
-	    },
 	    cmd => {
 		type => 'string',
 		description => "Run specific command or default to login.",
@@ -1097,10 +1091,6 @@ __PACKAGE__->register_method ({
 
 	my ($port, $remcmd) = $get_vnc_connection_info->($node);
 
-	# FIXME: remove with 7.0
-	if ($param->{upgrade}) {
-	    $param->{cmd} = 'upgrade';
-	}
 	my $shcmd = get_shell_command($user, $param->{cmd}, $param->{'cmd-opts'});
 
 	my $realcmd = sub {
-- 
2.30.2

[pve-devel] [PATCH manager 06/12] api: nodes: remove deprecated upgrade parameter for spiceshell

[Fabian Ebner]

The change not to pass the 'upgrade' parameter in the frontend was made in
81a896883678d27c44e6407f2ec1e86c32e03403 (the commit doesn't talk about it, it's
likely an accidental squash of two changes)

Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
---
 PVE/API2/Nodes.pm | 16 +++++-----------
 1 file changed, 5 insertions(+), 11 deletions(-)

diff --git a/PVE/API2/Nodes.pm b/PVE/API2/Nodes.pm
index 36cb70b4..7feb5662 100644
--- a/PVE/API2/Nodes.pm
+++ b/PVE/API2/Nodes.pm
@@ -1187,12 +1187,6 @@ __PACKAGE__->register_method ({
 	properties => {
 	    node => get_standard_option('pve-node'),
 	    proxy => get_standard_option('spice-proxy', { optional => 1 }),
-	    upgrade => {
-		type => 'boolean',
-		description => "Deprecated, use the 'cmd' property instead! Run 'apt-get dist-upgrade' instead of normal shell.",
-		optional => 1,
-		default => 0,
-	    },
 	    cmd => {
 		type => 'string',
 		description => "Run specific command or default to login.",
@@ -1219,17 +1213,17 @@ __PACKAGE__->register_method ({
 	my ($user, undef, $realm) = PVE::AccessControl::verify_username($authuser);
 
 	raise_perm_exc("realm != pam") if $realm ne 'pam';
-	raise_perm_exc('user != root@pam') if $param->{upgrade} && $user ne 'root@pam';
+
+	if (defined($param->{cmd}) && $param->{cmd} eq 'upgrade' && $user ne 'root@pam') {
+	    raise_perm_exc('user != root@pam');
+	}
 
 	my $node = $param->{node};
 	my $proxy = $param->{proxy};
 
 	my $authpath = "/nodes/$node";
 	my $permissions = 'Sys.Console';
-	# FIXME: remove with 6.0
-	if ($param->{upgrade}) {
-	    $param->{cmd} = 'upgrade';
-	}
+
 	my $shcmd = get_shell_command($user, $param->{cmd}, $param->{'cmd-opts'});
 
 	my $title = "Shell on '$node'";
-- 
2.30.2

[pve-devel] [PATCH manager 07/12] api: nodes: remove deprecated upgrade parameter for vncshell

[Fabian Ebner]

Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
---

Depends on novnc-pve patch (and clearing the browser cache ;)

 PVE/API2/Nodes.pm | 15 ++++-----------
 1 file changed, 4 insertions(+), 11 deletions(-)

diff --git a/PVE/API2/Nodes.pm b/PVE/API2/Nodes.pm
index 7feb5662..369c911c 100644
--- a/PVE/API2/Nodes.pm
+++ b/PVE/API2/Nodes.pm
@@ -906,12 +906,6 @@ __PACKAGE__->register_method ({
 	additionalProperties => 0,
 	properties => {
 	    node => get_standard_option('pve-node'),
-	    upgrade => {
-		type => 'boolean',
-		description => "Deprecated, use the 'cmd' property instead! Run 'apt-get dist-upgrade' instead of normal shell.",
-		optional => 1,
-		default => 0,
-	    },
 	    cmd => {
 		type => 'string',
 		description => "Run specific command or default to login.",
@@ -964,7 +958,10 @@ __PACKAGE__->register_method ({
 	my ($user, undef, $realm) = PVE::AccessControl::verify_username($rpcenv->get_user());
 
 	raise_perm_exc("realm != pam") if $realm ne 'pam';
-	raise_perm_exc('user != root@pam') if $param->{upgrade} && $user ne 'root@pam';
+
+	if (defined($param->{cmd}) && $param->{cmd} eq 'upgrade' && $user ne 'root@pam') {
+	    raise_perm_exc('user != root@pam');
+	}
 
 	my $node = $param->{node};
 
@@ -976,10 +973,6 @@ __PACKAGE__->register_method ({
 
 	my ($port, $remcmd) = $get_vnc_connection_info->($node);
 
-	# FIXME: remove with 6.0
-	if ($param->{upgrade}) {
-	    $param->{cmd} = 'upgrade';
-	}
 	my $shcmd = get_shell_command($user, $param->{cmd}, $param->{'cmd-opts'});
 
 	my $timeout = 10;
-- 
2.30.2

[pve-devel] [PATCH manager 08/12] api: move cpu path into qemu/capabilities

[Fabian Ebner]

Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
---
 PVE/API2/Capabilities.pm              | 6 ++++++
 PVE/API2/Nodes.pm                     | 7 -------
 www/manager6/form/CPUModelSelector.js | 2 +-
 3 files changed, 7 insertions(+), 8 deletions(-)

diff --git a/PVE/API2/Capabilities.pm b/PVE/API2/Capabilities.pm
index d7e99b55..c88c6c46 100644
--- a/PVE/API2/Capabilities.pm
+++ b/PVE/API2/Capabilities.pm
@@ -10,6 +10,11 @@ use PVE::API2::Qemu::Machine;
 
 use base qw(PVE::RESTHandler);
 
+__PACKAGE__->register_method ({
+    subclass => "PVE::API2::Qemu::CPU",
+    path => 'qemu/cpu',
+});
+
 __PACKAGE__->register_method ({
     subclass => "PVE::API2::Qemu::Machine",
     path => 'qemu/machines',
@@ -72,6 +77,7 @@ __PACKAGE__->register_method ({
 	my ($param) = @_;
 
 	my $result = [
+	    { name => 'cpu' },
 	    { name => 'machines' },
 	];
 
diff --git a/PVE/API2/Nodes.pm b/PVE/API2/Nodes.pm
index 369c911c..f4d3382c 100644
--- a/PVE/API2/Nodes.pm
+++ b/PVE/API2/Nodes.pm
@@ -70,12 +70,6 @@ __PACKAGE__->register_method ({
     path => 'qemu',
 });
 
-# FIXME: move into capabilities/qemu
-__PACKAGE__->register_method ({
-    subclass => "PVE::API2::Qemu::CPU",
-    path => 'cpu',
-});
-
 __PACKAGE__->register_method ({
     subclass => "PVE::API2::LXC",
     path => 'lxc',
@@ -228,7 +222,6 @@ __PACKAGE__->register_method ({
 	    { name => 'ceph' },
 	    { name => 'certificates' },
 	    { name => 'config' },
-	    { name => 'cpu' },
 	    { name => 'disks' },
 	    { name => 'dns' },
 	    { name => 'firewall' },
diff --git a/www/manager6/form/CPUModelSelector.js b/www/manager6/form/CPUModelSelector.js
index 225cdaa1..a30dd1fb 100644
--- a/www/manager6/form/CPUModelSelector.js
+++ b/www/manager6/form/CPUModelSelector.js
@@ -50,7 +50,7 @@ Ext.define('PVE.form.CPUModelSelector', {
 	model: 'PVE.data.CPUModel',
 	proxy: {
 	    type: 'proxmox',
-	    url: '/api2/json/nodes/localhost/cpu',
+	    url: '/api2/json/nodes/localhost/capabilities/qemu/cpu',
 	},
 	sorters: [
 	    {
-- 
2.30.2

[pve-devel] [PATCH manager 09/12] api: ceph: remove obsoleted disks call

[Fabian Ebner]

replaced by /nodes/{node}/disks/list

Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
---
 PVE/API2/Ceph.pm | 69 ------------------------------------------------
 1 file changed, 69 deletions(-)

diff --git a/PVE/API2/Ceph.pm b/PVE/API2/Ceph.pm
index 3fd029cb..d6eb5563 100644
--- a/PVE/API2/Ceph.pm
+++ b/PVE/API2/Ceph.pm
@@ -100,7 +100,6 @@ __PACKAGE__->register_method ({
 	    { name => 'crush' },
 	    { name => 'config' },
 	    { name => 'log' },
-	    { name => 'disks' }, # FIXME: remove with 7.0
 	    { name => 'flags' }, # FIXME: remove with 7.0
 	    { name => 'rules' },
 	];
@@ -108,74 +107,6 @@ __PACKAGE__->register_method ({
 	return $result;
     }});
 
-# FIXME: Remove with PVE 7.0
-__PACKAGE__->register_method ({
-    name => 'disks',
-    path => 'disks',
-    method => 'GET',
-    description => "List local disks.",
-    proxyto => 'node',
-    protected => 1,
-    permissions => {
-	check => ['perm', '/', [ 'Sys.Audit', 'Datastore.Audit' ], any => 1],
-    },
-    parameters => {
-	additionalProperties => 0,
-	properties => {
-	    node => get_standard_option('pve-node'),
-	    type => {
-		description => "Only list specific types of disks.",
-		type => 'string',
-		enum => ['unused', 'journal_disks'],
-		optional => 1,
-	    },
-	},
-    },
-    returns => {
-	type => 'array',
-	items => {
-	    type => "object",
-	    properties => {
-		dev => { type => 'string' },
-		used => { type => 'string', optional => 1 },
-		gpt => { type => 'boolean' },
-		size => { type => 'integer' },
-		osdid => { type => 'integer' },
-		vendor =>  { type => 'string', optional => 1 },
-		model =>  { type => 'string', optional => 1 },
-		serial =>  { type => 'string', optional => 1 },
-	    },
-	},
-	# links => [ { rel => 'child', href => "{}" } ],
-    },
-    code => sub {
-	my ($param) = @_;
-
-	PVE::Ceph::Tools::check_ceph_inited();
-
-	my $disks = PVE::Diskmanage::get_disks(undef, 1);
-
-	my $res = [];
-	foreach my $dev (keys %$disks) {
-	    my $d = $disks->{$dev};
-	    if ($param->{type}) {
-		if ($param->{type} eq 'journal_disks') {
-		    next if $d->{osdid} >= 0;
-		    next if !$d->{gpt};
-		} elsif ($param->{type} eq 'unused') {
-		    next if $d->{used};
-		} else {
-		    die "internal error"; # should not happen
-		}
-	    }
-
-	    $d->{dev} = "/dev/$dev";
-	    push @$res, $d;
-	}
-
-	return $res;
-    }});
-
 __PACKAGE__->register_method ({
     name => 'config',
     path => 'config',
-- 
2.30.2

[pve-devel] [PATCH manager 10/12] api: ceph: remove moved 'flags' calls

[Fabian Ebner]

living below /cluster/ceph now. Frontend was adapted as part of the move in
commit 90bb3598b5e257d06d4a49b44a4f5de02eb587a8

Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
---
 PVE/API2/Ceph.pm | 109 -----------------------------------------------
 1 file changed, 109 deletions(-)

diff --git a/PVE/API2/Ceph.pm b/PVE/API2/Ceph.pm
index d6eb5563..3bbcfe4c 100644
--- a/PVE/API2/Ceph.pm
+++ b/PVE/API2/Ceph.pm
@@ -100,7 +100,6 @@ __PACKAGE__->register_method ({
 	    { name => 'crush' },
 	    { name => 'config' },
 	    { name => 'log' },
-	    { name => 'flags' }, # FIXME: remove with 7.0
 	    { name => 'rules' },
 	];
 
@@ -493,114 +492,6 @@ __PACKAGE__->register_method ({
     }});
 
 
-my $possible_flags = PVE::Ceph::Tools::get_possible_osd_flags();
-my $possible_flags_list = [ sort keys %$possible_flags ];
-
-# FIXME: Remove with PVE 7.0
-__PACKAGE__->register_method ({
-    name => 'get_flags',
-    path => 'flags',
-    method => 'GET',
-    description => "get all set ceph flags",
-    proxyto => 'node',
-    protected => 1,
-    permissions => {
-	check => ['perm', '/', [ 'Sys.Audit' ]],
-    },
-    parameters => {
-	additionalProperties => 0,
-	properties => {
-	    node => get_standard_option('pve-node'),
-	},
-    },
-    returns => { type => 'string' },
-    code => sub {
-	my ($param) = @_;
-
-	PVE::Ceph::Tools::check_ceph_configured();
-
-	my $rados = PVE::RADOS->new();
-
-	my $stat = $rados->mon_command({ prefix => 'osd dump' });
-
-	return $stat->{flags} // '';
-    }});
-
-# FIXME: Remove with PVE 7.0
-__PACKAGE__->register_method ({
-    name => 'set_flag',
-    path => 'flags/{flag}',
-    method => 'POST',
-    description => "Set a specific ceph flag",
-    proxyto => 'node',
-    protected => 1,
-    permissions => {
-	check => ['perm', '/', [ 'Sys.Modify' ]],
-    },
-    parameters => {
-	additionalProperties => 0,
-	properties => {
-	    node => get_standard_option('pve-node'),
-	    flag => {
-		description => 'The ceph flag to set',
-		type => 'string',
-		enum => $possible_flags_list,
-	    },
-	},
-    },
-    returns => { type => 'null' },
-    code => sub {
-	my ($param) = @_;
-
-	PVE::Ceph::Tools::check_ceph_configured();
-
-	my $rados = PVE::RADOS->new();
-
-	$rados->mon_command({
-	    prefix => "osd set",
-	    key => $param->{flag},
-	});
-
-	return undef;
-    }});
-
-__PACKAGE__->register_method ({
-    name => 'unset_flag',
-    path => 'flags/{flag}',
-    method => 'DELETE',
-    description => "Unset a ceph flag",
-    proxyto => 'node',
-    protected => 1,
-    permissions => {
-	check => ['perm', '/', [ 'Sys.Modify' ]],
-    },
-    parameters => {
-	additionalProperties => 0,
-	properties => {
-	    node => get_standard_option('pve-node'),
-	    flag => {
-		description => 'The ceph flag to unset',
-		type => 'string',
-		enum => $possible_flags_list,
-	    },
-	},
-    },
-    returns => { type => 'null' },
-    code => sub {
-	my ($param) = @_;
-
-	PVE::Ceph::Tools::check_ceph_configured();
-
-	my $rados = PVE::RADOS->new();
-
-	$rados->mon_command({
-	    prefix => "osd unset",
-	    key => $param->{flag},
-	});
-
-	return undef;
-    }});
-
 __PACKAGE__->register_method ({
     name => 'crush',
     path => 'crush',
-- 
2.30.2

[pve-devel] [PATCH manager 11/12] api: ceph: osd: create: rename size parameters

[Fabian Ebner]

Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
---
 PVE/API2/Ceph/OSD.pm     | 8 ++------
 www/manager6/ceph/OSD.js | 8 ++++----
 2 files changed, 6 insertions(+), 10 deletions(-)

diff --git a/PVE/API2/Ceph/OSD.pm b/PVE/API2/Ceph/OSD.pm
index 27e0642c..79548b2f 100644
--- a/PVE/API2/Ceph/OSD.pm
+++ b/PVE/API2/Ceph/OSD.pm
@@ -223,7 +223,7 @@ __PACKAGE__->register_method ({
 		optional => 1,
 		type => 'string',
 	    },
-	    db_size => {
+	    db_dev_size => {
 		description => "Size in GiB for block.db.",
 		verbose_description => "If a block.db is requested but the size is not given, ".
 		    "will be automatically selected by: bluestore_block_db_size from the ".
@@ -241,7 +241,7 @@ __PACKAGE__->register_method ({
 		optional => 1,
 		type => 'string',
 	    },
-	    wal_size => {
+	    wal_dev_size => {
 		description => "Size in GiB for block.wal.",
 		verbose_description => "If a block.wal is requested but the size is not given, ".
 		    "will be automatically selected by: bluestore_block_wal_size from the ".
@@ -300,10 +300,6 @@ __PACKAGE__->register_method ({
 	    }
 	}
 
-	# FIXME: rename params on next API compatibillity change (7.0)
-	$param->{wal_dev_size} = delete $param->{wal_size};
-	$param->{db_dev_size} = delete $param->{db_size};
-
 	for my $type ( qw(dev db_dev wal_dev) ) {
 	    next if !$param->{$type};
 
diff --git a/www/manager6/ceph/OSD.js b/www/manager6/ceph/OSD.js
index 13cd2fc5..aea38d6c 100644
--- a/www/manager6/ceph/OSD.js
+++ b/www/manager6/ceph/OSD.js
@@ -55,13 +55,13 @@ Ext.define('PVE.CephCreateOsd', {
 			    emptyText: 'use OSD disk',
 			    listeners: {
 				change: function(field, val) {
-				    me.down('field[name=db_size]').setDisabled(!val);
+				    me.down('field[name=db_dev_size]').setDisabled(!val);
 				},
 			    },
 			},
 			{
 			    xtype: 'numberfield',
-			    name: 'db_size',
+			    name: 'db_dev_size',
 			    fieldLabel: gettext('DB size') + ' (GiB)',
 			    minValue: 1,
 			    maxValue: 128*1024,
@@ -108,13 +108,13 @@ Ext.define('PVE.CephCreateOsd', {
 			    emptyText: 'use OSD/DB disk',
 			    listeners: {
 				change: function(field, val) {
-				    me.down('field[name=wal_size]').setDisabled(!val);
+				    me.down('field[name=wal_dev_size]').setDisabled(!val);
 				},
 			    },
 			},
 			{
 			    xtype: 'numberfield',
-			    name: 'wal_size',
+			    name: 'wal_dev_size',
 			    fieldLabel: gettext('WAL size') + ' (GiB)',
 			    minValue: 0.5,
 			    maxValue: 128*1024,
-- 
2.30.2

[pve-devel] [PATCH manager 12/12] cli: pveceph: remove outdated comment

[Fabian Ebner]

Already removed by commit d9ce53fdc324729d47dcca7adefcd564664df5b1.

Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
---
 PVE/CLI/pveceph.pm | 1 -
 1 file changed, 1 deletion(-)

diff --git a/PVE/CLI/pveceph.pm b/PVE/CLI/pveceph.pm
index 7021b23b..033eab4e 100755
--- a/PVE/CLI/pveceph.pm
+++ b/PVE/CLI/pveceph.pm
@@ -116,7 +116,6 @@ __PACKAGE__->register_method ({
 	properties => {
 	    version => {
 		type => 'string',
-		# for buster, luminous kept for testing/upgrade purposes only! - FIXME: remove with 6.2?
 		enum => ['octopus', 'pacific'],
 		default => 'pacific',
 		description => "Ceph version to install.",
-- 
2.30.2

[pve-devel] applied-partially: [PATCH-SERIES] Some breaking API changes/cleanups

[Fabian Grünbichler]

applied (with some added version bumps/versioned dependency bumps), 
except for the two storage patches where I replied with nits.

please add stuff to breaking changes and send patches for pve6to7 where 
appropriate!

On June 4, 2021 3:49 pm, Fabian Ebner wrote:
> 
> Of course all of the changes need to be recorded in the release notes.
> Should I do so now or wait until the changes are actually applied?
> 
> Dependencies are noted on individual patches, most should apply independently.
> 
> There's still quite a few outstanding, but I feel like somebody with more
> experience/context would be better suited to address them. I can go for another
> round though if there are not enough volunteers ;)
> 
> 
> pve-access-control:
> src/PVE/Auth/AD.pm
> 30:         description => "Use secure LDAPS protocol. DEPRECATED: use 'mode' instead.",
> src/PVE/Auth/LDAP.pm
> 390:    # FIXME: remove fallback with 7.0 by doing a rename on upgrade from 6.x
> 
> src/PVE/AccessControl.pm
> 299:    # FIXME: remove with PVE 7 and/or refactor all into PVE::Ticket ?
> 
> pve-common:
> src/PVE/Ticket.pm
> 38:         # detected sha1 csrf token from older proxy, fallback. FIXME: remove with 7.0
> 
> pve-cluster:
> quite a few // FIXME: remove openvz stuff for 7.x
> 
> pve-storage:
> PVE/Storage/CephFSPlugin.pm
> 41:# FIXME: remove in PVE 7.0 where systemd is recent enough to not have those
> # local-fs/remote-fs dependency cycles generated for _netdev mounts...
> 
> 
> Would be glad if somebody experienced in those areas could look at them.
> 
> 
> 
> data/PVE/DataCenterConfig.pm has a few workarounds for deprecated options
> 
> pve-container:
> src/PVE/LXC/Config.pm
> 652:    # Deprecated (removed with lxc 3.0):
> 
> src/PVE/LXC.pm
> 2178:   # FIXME: remove the 'id_map' variant when lxc-3.0 arrives
> 
> 
> There might still be lxc/datacenter configs with the old options around?
> Do we want to warn/replace as part of the pve6to7 script or just keep
> the compat code longer?
> 
> 
> 
> pve-manager:
> PVE/Ceph/Services.pm
> 54:     # FIXME: remove with 7.0 - for backward compat only
> 
> PVE/API2/Cluster/Ceph.pm
> 78:         # FIXME: remove with 7.0 depreacated by structured 'versions'
> 
> 
> Front-end still expects the metadata.version to be present. Might not be hard to
> adapt, but I didn't have the context to immediately know what's the right thing
> to do. Can look at it if nobody else wants to.
> 
> 
> 
> pve-storage:
> PVE/Storage.pm
> 966:    # FIXME PVE 7.0: only scan storages with the correct content types
> 
> 
> From an off-list discussion with Fabian G: We currently still need this behavior
> to pick up disks from storages with a misconfigured content types. Meaning that
> we need to require content type 'images' to be set first and warn people with
> misconfigured storages as part of pve6to7. We could even go further and not pick
> up unreferenced disks at all, but should expose rescan in the UI and make a
> rescan as part of pve6to7. I will look at this in a separate series.
> 
> 
> 
> pve-guest-common:
> 
> Fabian Ebner (2):
>   vzdump: remove deprecated size parameter
>   vzdump: defaults: keep all backups by default for 7.0
> 
>  src/PVE/VZDump/Common.pm | 17 +++++------------
>  1 file changed, 5 insertions(+), 12 deletions(-)
> 
> 
> pve-container:
> 
> Fabian Ebner (1):
>   migrate: remove deprecated force parameter
> 
>  src/PVE/API2/LXC.pm    | 6 ------
>  src/PVE/LXC/Migrate.pm | 8 +-------
>  2 files changed, 1 insertion(+), 13 deletions(-)
> 
> 
> qemu-server:
> 
> Fabian Ebner (3):
>   Revert "revert spice_ticket prefix change in 7827de4"
>   scan volids: remove superfluous parameter
>   vm destroy: do not remove unreferenced disks by default
> 
>  PVE/API2/Qemu.pm   | 5 ++---
>  PVE/QemuMigrate.pm | 5 ++---
>  PVE/QemuServer.pm  | 7 +++----
>  3 files changed, 7 insertions(+), 10 deletions(-)
> 
> 
> pve-storage:
> 
> Fabian Ebner (3):
>   postinst: move cifs credential files into subdirectory upon update
>   update reminder to remove maxfiles
>   api: get rid of moved 'usb' call
> 
>  PVE/API2/Storage/Scan.pm  | 45 ---------------------------------------
>  PVE/Storage.pm            |  2 +-
>  PVE/Storage/CIFSPlugin.pm |  3 ---
>  debian/postinst           | 30 ++++++++++++++++++++++++++
>  4 files changed, 31 insertions(+), 49 deletions(-)
>  create mode 100644 debian/postinst
> 
> 
> novnc-pve:
> 
> Fabian Ebner (1):
>   avoid passing deprecated 'upgrade' parameter
> 
>  ...passing-deprecated-upgrade-parameter.patch | 24 +++++++++++++++++++
>  debian/patches/series                         |  1 +
>  2 files changed, 25 insertions(+)
>  create mode 100644 debian/patches/0014-avoid-passing-deprecated-upgrade-parameter.patch
> 
> pve-manager:
> 
> Fabian Ebner (12):
>   vzdump: remove deprecated size parameter
>   configs: vzdump: use prune-backups instead of the deprecated maxfiles
>   test: vzdump: adapt to new default
>   Revert "VZDump: add TARFILE to environment for hookscripts"
>   api: nodes: remove deprecated upgrade parameter for xtermjs
>   api: nodes: remove deprecated upgrade parameter for spiceshell
>   api: nodes: remove deprecated upgrade parameter for vncshell
>   api: move cpu path into qemu/capabilities
>   api: ceph: remove obsoleted disks call
>   api: ceph: remove moved 'flags' calls
>   api: ceph: osd: create: rename size parameters
>   cli: pveceph: remove outdated comment
> 
>  PVE/API2/Capabilities.pm              |   6 +
>  PVE/API2/Ceph.pm                      | 178 --------------------------
>  PVE/API2/Ceph/OSD.pm                  |   8 +-
>  PVE/API2/Nodes.pm                     |  48 ++-----
>  PVE/API2/VZDump.pm                    |   2 -
>  PVE/CLI/pveceph.pm                    |   1 -
>  PVE/VZDump.pm                         |   6 -
>  configs/vzdump.conf                   |   3 +-
>  test/vzdump_new_test.pl               |   8 +-
>  www/manager6/ceph/OSD.js              |   8 +-
>  www/manager6/form/CPUModelSelector.js |   2 +-
>  11 files changed, 27 insertions(+), 243 deletions(-)
> 
> -- 
> 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-partially: [PATCH-SERIES] Some breaking API changes/cleanups

[Fabian Ebner]

Am 09.06.21 um 11:44 schrieb Fabian Grünbichler:
> applied (with some added version bumps/versioned dependency bumps),
> except for the two storage patches where I replied with nits.
> 
> please add stuff to breaking changes and send patches for pve6to7 where
> appropriate!
> 

I added entries for the breaking changes in the intranet wiki.

There's not really much for pve6to7, we could:

1. Warn when there's a backup storage without retention settings on a 
node without retention settings, which could lead to a storage 
unexpectedly filling up, because of the new default.

2. Warn if there's CIFS credentials in the old location already pre-upgrade.

Should I go for these, or what did you have in mind?

> On June 4, 2021 3:49 pm, Fabian Ebner wrote:
>>
>> Of course all of the changes need to be recorded in the release notes.
>> Should I do so now or wait until the changes are actually applied?
>>
>> Dependencies are noted on individual patches, most should apply independently.
>>
>> There's still quite a few outstanding, but I feel like somebody with more
>> experience/context would be better suited to address them. I can go for another
>> round though if there are not enough volunteers ;)
>>
>>
>> pve-access-control:
>> src/PVE/Auth/AD.pm
>> 30:         description => "Use secure LDAPS protocol. DEPRECATED: use 'mode' instead.",
>> src/PVE/Auth/LDAP.pm
>> 390:    # FIXME: remove fallback with 7.0 by doing a rename on upgrade from 6.x
>>
>> src/PVE/AccessControl.pm
>> 299:    # FIXME: remove with PVE 7 and/or refactor all into PVE::Ticket ?
>>
>> pve-common:
>> src/PVE/Ticket.pm
>> 38:         # detected sha1 csrf token from older proxy, fallback. FIXME: remove with 7.0
>>
>> pve-cluster:
>> quite a few // FIXME: remove openvz stuff for 7.x
>>
>> pve-storage:
>> PVE/Storage/CephFSPlugin.pm
>> 41:# FIXME: remove in PVE 7.0 where systemd is recent enough to not have those
>> # local-fs/remote-fs dependency cycles generated for _netdev mounts...
>>
>>
>> Would be glad if somebody experienced in those areas could look at them.
>>
>>
>>
>> data/PVE/DataCenterConfig.pm has a few workarounds for deprecated options
>>
>> pve-container:
>> src/PVE/LXC/Config.pm
>> 652:    # Deprecated (removed with lxc 3.0):
>>
>> src/PVE/LXC.pm
>> 2178:   # FIXME: remove the 'id_map' variant when lxc-3.0 arrives
>>
>>
>> There might still be lxc/datacenter configs with the old options around?
>> Do we want to warn/replace as part of the pve6to7 script or just keep
>> the compat code longer?
>>
>>
>>
>> pve-manager:
>> PVE/Ceph/Services.pm
>> 54:     # FIXME: remove with 7.0 - for backward compat only
>>
>> PVE/API2/Cluster/Ceph.pm
>> 78:         # FIXME: remove with 7.0 depreacated by structured 'versions'
>>
>>
>> Front-end still expects the metadata.version to be present. Might not be hard to
>> adapt, but I didn't have the context to immediately know what's the right thing
>> to do. Can look at it if nobody else wants to.
>>
>>
>>
>> pve-storage:
>> PVE/Storage.pm
>> 966:    # FIXME PVE 7.0: only scan storages with the correct content types
>>
>>
>>  From an off-list discussion with Fabian G: We currently still need this behavior
>> to pick up disks from storages with a misconfigured content types. Meaning that
>> we need to require content type 'images' to be set first and warn people with
>> misconfigured storages as part of pve6to7. We could even go further and not pick
>> up unreferenced disks at all, but should expose rescan in the UI and make a
>> rescan as part of pve6to7. I will look at this in a separate series.
>>
>>
>>
>> pve-guest-common:
>>
>> Fabian Ebner (2):
>>    vzdump: remove deprecated size parameter
>>    vzdump: defaults: keep all backups by default for 7.0
>>
>>   src/PVE/VZDump/Common.pm | 17 +++++------------
>>   1 file changed, 5 insertions(+), 12 deletions(-)
>>
>>
>> pve-container:
>>
>> Fabian Ebner (1):
>>    migrate: remove deprecated force parameter
>>
>>   src/PVE/API2/LXC.pm    | 6 ------
>>   src/PVE/LXC/Migrate.pm | 8 +-------
>>   2 files changed, 1 insertion(+), 13 deletions(-)
>>
>>
>> qemu-server:
>>
>> Fabian Ebner (3):
>>    Revert "revert spice_ticket prefix change in 7827de4"
>>    scan volids: remove superfluous parameter
>>    vm destroy: do not remove unreferenced disks by default
>>
>>   PVE/API2/Qemu.pm   | 5 ++---
>>   PVE/QemuMigrate.pm | 5 ++---
>>   PVE/QemuServer.pm  | 7 +++----
>>   3 files changed, 7 insertions(+), 10 deletions(-)
>>
>>
>> pve-storage:
>>
>> Fabian Ebner (3):
>>    postinst: move cifs credential files into subdirectory upon update
>>    update reminder to remove maxfiles
>>    api: get rid of moved 'usb' call
>>
>>   PVE/API2/Storage/Scan.pm  | 45 ---------------------------------------
>>   PVE/Storage.pm            |  2 +-
>>   PVE/Storage/CIFSPlugin.pm |  3 ---
>>   debian/postinst           | 30 ++++++++++++++++++++++++++
>>   4 files changed, 31 insertions(+), 49 deletions(-)
>>   create mode 100644 debian/postinst
>>
>>
>> novnc-pve:
>>
>> Fabian Ebner (1):
>>    avoid passing deprecated 'upgrade' parameter
>>
>>   ...passing-deprecated-upgrade-parameter.patch | 24 +++++++++++++++++++
>>   debian/patches/series                         |  1 +
>>   2 files changed, 25 insertions(+)
>>   create mode 100644 debian/patches/0014-avoid-passing-deprecated-upgrade-parameter.patch
>>
>> pve-manager:
>>
>> Fabian Ebner (12):
>>    vzdump: remove deprecated size parameter
>>    configs: vzdump: use prune-backups instead of the deprecated maxfiles
>>    test: vzdump: adapt to new default
>>    Revert "VZDump: add TARFILE to environment for hookscripts"
>>    api: nodes: remove deprecated upgrade parameter for xtermjs
>>    api: nodes: remove deprecated upgrade parameter for spiceshell
>>    api: nodes: remove deprecated upgrade parameter for vncshell
>>    api: move cpu path into qemu/capabilities
>>    api: ceph: remove obsoleted disks call
>>    api: ceph: remove moved 'flags' calls
>>    api: ceph: osd: create: rename size parameters
>>    cli: pveceph: remove outdated comment
>>
>>   PVE/API2/Capabilities.pm              |   6 +
>>   PVE/API2/Ceph.pm                      | 178 --------------------------
>>   PVE/API2/Ceph/OSD.pm                  |   8 +-
>>   PVE/API2/Nodes.pm                     |  48 ++-----
>>   PVE/API2/VZDump.pm                    |   2 -
>>   PVE/CLI/pveceph.pm                    |   1 -
>>   PVE/VZDump.pm                         |   6 -
>>   configs/vzdump.conf                   |   3 +-
>>   test/vzdump_new_test.pl               |   8 +-
>>   www/manager6/ceph/OSD.js              |   8 +-
>>   www/manager6/form/CPUModelSelector.js |   2 +-
>>   11 files changed, 27 insertions(+), 243 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 mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
> 
> 

Re: [pve-devel] applied-partially: [PATCH-SERIES] Some breaking API changes/cleanups

[Fabian Grünbichler]

On June 9, 2021 1:15 pm, Fabian Ebner wrote:
> Am 09.06.21 um 11:44 schrieb Fabian Grünbichler:
>> applied (with some added version bumps/versioned dependency bumps),
>> except for the two storage patches where I replied with nits.
>> 
>> please add stuff to breaking changes and send patches for pve6to7 where
>> appropriate!
>> 
> 
> I added entries for the breaking changes in the intranet wiki.
> 
> There's not really much for pve6to7, we could:
> 
> 1. Warn when there's a backup storage without retention settings on a 
> node without retention settings, which could lead to a storage 
> unexpectedly filling up, because of the new default.
> 
> 2. Warn if there's CIFS credentials in the old location already pre-upgrade.
> 
> Should I go for these, or what did you have in mind?

those sound good. maxfiles being set in either vzdump or storage config 
or a backup job entry in vzdump.cron might be another one? (just as a 
warning/deprecation, not hard error)

> 
>> On June 4, 2021 3:49 pm, Fabian Ebner wrote:
>>>
>>> Of course all of the changes need to be recorded in the release notes.
>>> Should I do so now or wait until the changes are actually applied?
>>>
>>> Dependencies are noted on individual patches, most should apply independently.
>>>
>>> There's still quite a few outstanding, but I feel like somebody with more
>>> experience/context would be better suited to address them. I can go for another
>>> round though if there are not enough volunteers ;)
>>>
>>>
>>> pve-access-control:
>>> src/PVE/Auth/AD.pm
>>> 30:         description => "Use secure LDAPS protocol. DEPRECATED: use 'mode' instead.",
>>> src/PVE/Auth/LDAP.pm
>>> 390:    # FIXME: remove fallback with 7.0 by doing a rename on upgrade from 6.x
>>>
>>> src/PVE/AccessControl.pm
>>> 299:    # FIXME: remove with PVE 7 and/or refactor all into PVE::Ticket ?
>>>
>>> pve-common:
>>> src/PVE/Ticket.pm
>>> 38:         # detected sha1 csrf token from older proxy, fallback. FIXME: remove with 7.0
>>>
>>> pve-cluster:
>>> quite a few // FIXME: remove openvz stuff for 7.x
>>>
>>> pve-storage:
>>> PVE/Storage/CephFSPlugin.pm
>>> 41:# FIXME: remove in PVE 7.0 where systemd is recent enough to not have those
>>> # local-fs/remote-fs dependency cycles generated for _netdev mounts...
>>>
>>>
>>> Would be glad if somebody experienced in those areas could look at them.
>>>
>>>
>>>
>>> data/PVE/DataCenterConfig.pm has a few workarounds for deprecated options
>>>
>>> pve-container:
>>> src/PVE/LXC/Config.pm
>>> 652:    # Deprecated (removed with lxc 3.0):
>>>
>>> src/PVE/LXC.pm
>>> 2178:   # FIXME: remove the 'id_map' variant when lxc-3.0 arrives
>>>
>>>
>>> There might still be lxc/datacenter configs with the old options around?
>>> Do we want to warn/replace as part of the pve6to7 script or just keep
>>> the compat code longer?
>>>
>>>
>>>
>>> pve-manager:
>>> PVE/Ceph/Services.pm
>>> 54:     # FIXME: remove with 7.0 - for backward compat only
>>>
>>> PVE/API2/Cluster/Ceph.pm
>>> 78:         # FIXME: remove with 7.0 depreacated by structured 'versions'
>>>
>>>
>>> Front-end still expects the metadata.version to be present. Might not be hard to
>>> adapt, but I didn't have the context to immediately know what's the right thing
>>> to do. Can look at it if nobody else wants to.
>>>
>>>
>>>
>>> pve-storage:
>>> PVE/Storage.pm
>>> 966:    # FIXME PVE 7.0: only scan storages with the correct content types
>>>
>>>
>>>  From an off-list discussion with Fabian G: We currently still need this behavior
>>> to pick up disks from storages with a misconfigured content types. Meaning that
>>> we need to require content type 'images' to be set first and warn people with
>>> misconfigured storages as part of pve6to7. We could even go further and not pick
>>> up unreferenced disks at all, but should expose rescan in the UI and make a
>>> rescan as part of pve6to7. I will look at this in a separate series.
>>>
>>>
>>>
>>> pve-guest-common:
>>>
>>> Fabian Ebner (2):
>>>    vzdump: remove deprecated size parameter
>>>    vzdump: defaults: keep all backups by default for 7.0
>>>
>>>   src/PVE/VZDump/Common.pm | 17 +++++------------
>>>   1 file changed, 5 insertions(+), 12 deletions(-)
>>>
>>>
>>> pve-container:
>>>
>>> Fabian Ebner (1):
>>>    migrate: remove deprecated force parameter
>>>
>>>   src/PVE/API2/LXC.pm    | 6 ------
>>>   src/PVE/LXC/Migrate.pm | 8 +-------
>>>   2 files changed, 1 insertion(+), 13 deletions(-)
>>>
>>>
>>> qemu-server:
>>>
>>> Fabian Ebner (3):
>>>    Revert "revert spice_ticket prefix change in 7827de4"
>>>    scan volids: remove superfluous parameter
>>>    vm destroy: do not remove unreferenced disks by default
>>>
>>>   PVE/API2/Qemu.pm   | 5 ++---
>>>   PVE/QemuMigrate.pm | 5 ++---
>>>   PVE/QemuServer.pm  | 7 +++----
>>>   3 files changed, 7 insertions(+), 10 deletions(-)
>>>
>>>
>>> pve-storage:
>>>
>>> Fabian Ebner (3):
>>>    postinst: move cifs credential files into subdirectory upon update
>>>    update reminder to remove maxfiles
>>>    api: get rid of moved 'usb' call
>>>
>>>   PVE/API2/Storage/Scan.pm  | 45 ---------------------------------------
>>>   PVE/Storage.pm            |  2 +-
>>>   PVE/Storage/CIFSPlugin.pm |  3 ---
>>>   debian/postinst           | 30 ++++++++++++++++++++++++++
>>>   4 files changed, 31 insertions(+), 49 deletions(-)
>>>   create mode 100644 debian/postinst
>>>
>>>
>>> novnc-pve:
>>>
>>> Fabian Ebner (1):
>>>    avoid passing deprecated 'upgrade' parameter
>>>
>>>   ...passing-deprecated-upgrade-parameter.patch | 24 +++++++++++++++++++
>>>   debian/patches/series                         |  1 +
>>>   2 files changed, 25 insertions(+)
>>>   create mode 100644 debian/patches/0014-avoid-passing-deprecated-upgrade-parameter.patch
>>>
>>> pve-manager:
>>>
>>> Fabian Ebner (12):
>>>    vzdump: remove deprecated size parameter
>>>    configs: vzdump: use prune-backups instead of the deprecated maxfiles
>>>    test: vzdump: adapt to new default
>>>    Revert "VZDump: add TARFILE to environment for hookscripts"
>>>    api: nodes: remove deprecated upgrade parameter for xtermjs
>>>    api: nodes: remove deprecated upgrade parameter for spiceshell
>>>    api: nodes: remove deprecated upgrade parameter for vncshell
>>>    api: move cpu path into qemu/capabilities
>>>    api: ceph: remove obsoleted disks call
>>>    api: ceph: remove moved 'flags' calls
>>>    api: ceph: osd: create: rename size parameters
>>>    cli: pveceph: remove outdated comment
>>>
>>>   PVE/API2/Capabilities.pm              |   6 +
>>>   PVE/API2/Ceph.pm                      | 178 --------------------------
>>>   PVE/API2/Ceph/OSD.pm                  |   8 +-
>>>   PVE/API2/Nodes.pm                     |  48 ++-----
>>>   PVE/API2/VZDump.pm                    |   2 -
>>>   PVE/CLI/pveceph.pm                    |   1 -
>>>   PVE/VZDump.pm                         |   6 -
>>>   configs/vzdump.conf                   |   3 +-
>>>   test/vzdump_new_test.pl               |   8 +-
>>>   www/manager6/ceph/OSD.js              |   8 +-
>>>   www/manager6/form/CPUModelSelector.js |   2 +-
>>>   11 files changed, 27 insertions(+), 243 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 mailing list
>> pve-devel@lists.proxmox.com
>> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
>> 
>> 
>