[pve-devel] [PATCH proxmox-backup/pve-storage/pve-manager v2] show/edit backup comments

[pve-devel] [PATCH proxmox-backup/pve-storage/pve-manager v2] show/edit backup comments

[Dominik Csapak]

implement editing/showing notes from pve side
for directory and pbs storages

this series technically consists of a breaking api change
(comments -> notes) in content listing, but since we only just built
packages with the relevant commits, this should be ok...

i also omitted the suggested 'snapshot list' part of proxmox-backup
but will look into implementing an invisible alias for the clicommand
in rust soon

changes from v1:
* rename all occurences of 'comment' to 'notes' where possible
(everywhere except the snapshots api of pbs)
* split patches so that pbs ones could be applied later

apply order/dependency:

pbs patches (can be applied later):
pve-manager 3/3 depends on pve-storage 4/4 depends on proxmxo-backup

else:
pve-manager 1/3 depends on pve-storage 1/4
pve-manager 2/3 depends on pve-storage 2,3/4

proxmox-backup:

Dominik Csapak (1):
  client: add 'snapshot notes show/update' command

 src/bin/proxmox-backup-client.rs          |   1 +
 src/bin/proxmox_backup_client/mod.rs      |   2 +
 src/bin/proxmox_backup_client/snapshot.rs | 126 ++++++++++++++++++++++
 3 files changed, 129 insertions(+)
 create mode 100644 src/bin/proxmox_backup_client/snapshot.rs

pve-storage:

Dominik Csapak (4):
  rename comment to notes
  api2/storage/content: change to volume_size_info and add return
    properties
  Storage/Plugin: add get/update_volume_comment and implement for dir
  Storage/PBSPlugin: implement get/update_volume_notes for pbs

 PVE/API2/Storage/Content.pm | 96 ++++++++++++++++++++++++++++++++++---
 PVE/Storage.pm              | 20 ++++++++
 PVE/Storage/DirPlugin.pm    | 30 ++++++++++++
 PVE/Storage/PBSPlugin.pm    | 22 ++++++++-
 PVE/Storage/Plugin.pm       | 22 +++++++--
 5 files changed, 177 insertions(+), 13 deletions(-)

pve-manager:

Dominik Csapak (3):
  ui: change comment column to notes
  ui: add ability to show and edit comments for backups
  ui: enable notesedit for pbs

 www/manager6/grid/BackupView.js     | 47 +++++++++++++++++++++++++++--
 www/manager6/storage/Browser.js     |  2 +-
 www/manager6/storage/ContentView.js | 12 ++++----
 3 files changed, 51 insertions(+), 10 deletions(-)

-- 
2.20.1

[pve-devel] [PATCH proxmox-backup v2 1/1] client: add 'snapshot notes show/update' command

[Dominik Csapak]

to show and update snapshot notes from the cli

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 src/bin/proxmox-backup-client.rs          |   1 +
 src/bin/proxmox_backup_client/mod.rs      |   2 +
 src/bin/proxmox_backup_client/snapshot.rs | 126 ++++++++++++++++++++++
 3 files changed, 129 insertions(+)
 create mode 100644 src/bin/proxmox_backup_client/snapshot.rs

diff --git a/src/bin/proxmox-backup-client.rs b/src/bin/proxmox-backup-client.rs
index c961e390..f8a3fe70 100644
--- a/src/bin/proxmox-backup-client.rs
+++ b/src/bin/proxmox-backup-client.rs
@@ -2110,6 +2110,7 @@ fn main() {
         .insert("prune", prune_cmd_def)
         .insert("restore", restore_cmd_def)
         .insert("snapshots", snapshots_cmd_def)
+        .insert("snapshot", snapshot_mgtm_cli())
         .insert("files", files_cmd_def)
         .insert("status", status_cmd_def)
         .insert("key", key::cli())
diff --git a/src/bin/proxmox_backup_client/mod.rs b/src/bin/proxmox_backup_client/mod.rs
index 0c4bffb9..a14b0dc1 100644
--- a/src/bin/proxmox_backup_client/mod.rs
+++ b/src/bin/proxmox_backup_client/mod.rs
@@ -8,6 +8,8 @@ mod task;
 pub use task::*;
 mod catalog;
 pub use catalog::*;
+mod snapshot;
+pub use snapshot::*;
 
 pub mod key;
 
diff --git a/src/bin/proxmox_backup_client/snapshot.rs b/src/bin/proxmox_backup_client/snapshot.rs
new file mode 100644
index 00000000..fd5f543b
--- /dev/null
+++ b/src/bin/proxmox_backup_client/snapshot.rs
@@ -0,0 +1,126 @@
+use anyhow::Error;
+use serde_json::{json, Value};
+
+use proxmox::api::{api, cli::*};
+use proxmox_backup::tools;
+
+use crate::{
+    complete_backup_snapshot, connect, extract_repository_from_value, BackupDir, REPO_URL_SCHEMA,
+};
+
+#[api(
+    input: {
+        properties: {
+            repository: {
+                schema: REPO_URL_SCHEMA,
+                optional: true,
+            },
+            snapshot: {
+                type: String,
+                description: "Snapshot path.",
+            },
+            "output-format": {
+                schema: OUTPUT_FORMAT,
+                optional: true,
+            },
+        }
+    }
+)]
+/// Show notes
+async fn show_notes(param: Value) -> Result<Value, Error> {
+    let repo = extract_repository_from_value(&param)?;
+    let path = tools::required_string_param(&param, "snapshot")?;
+
+    let snapshot: BackupDir = path.parse()?;
+    let client = connect(&repo)?;
+
+    let path = format!("api2/json/admin/datastore/{}/notes", repo.store());
+
+    let args = json!({
+        "backup-type": snapshot.group().backup_type(),
+        "backup-id": snapshot.group().backup_id(),
+        "backup-time": snapshot.backup_time(),
+    });
+
+    let output_format = get_output_format(&param);
+
+    let mut result = client.get(&path, Some(args)).await?;
+
+    let notes = result["data"].take();
+
+    if output_format == "text" {
+        if let Some(notes) = notes.as_str() {
+            println!("{}", notes);
+        }
+    } else {
+        format_and_print_result(
+            &json!({
+                "notes": notes,
+            }),
+            &output_format,
+        );
+    }
+
+    Ok(Value::Null)
+}
+
+#[api(
+    input: {
+        properties: {
+            repository: {
+                schema: REPO_URL_SCHEMA,
+                optional: true,
+            },
+            snapshot: {
+                type: String,
+                description: "Snapshot path.",
+            },
+            notes: {
+                type: String,
+                description: "The Notes.",
+            },
+        }
+    }
+)]
+/// Update Notes
+async fn update_notes(param: Value) -> Result<Value, Error> {
+    let repo = extract_repository_from_value(&param)?;
+    let path = tools::required_string_param(&param, "snapshot")?;
+    let notes = tools::required_string_param(&param, "notes")?;
+
+    let snapshot: BackupDir = path.parse()?;
+    let mut client = connect(&repo)?;
+
+    let path = format!("api2/json/admin/datastore/{}/notes", repo.store());
+
+    let args = json!({
+        "backup-type": snapshot.group().backup_type(),
+        "backup-id": snapshot.group().backup_id(),
+        "backup-time": snapshot.backup_time(),
+        "notes": notes,
+    });
+
+    client.put(&path, Some(args)).await?;
+
+    Ok(Value::Null)
+}
+
+fn notes_cli() -> CliCommandMap {
+    CliCommandMap::new()
+        .insert(
+            "show",
+            CliCommand::new(&API_METHOD_SHOW_NOTES)
+                .arg_param(&["snapshot"])
+                .completion_cb("snapshot", complete_backup_snapshot),
+        )
+        .insert(
+            "update",
+            CliCommand::new(&API_METHOD_UPDATE_NOTES)
+                .arg_param(&["snapshot", "notes"])
+                .completion_cb("snapshot", complete_backup_snapshot),
+        )
+}
+
+pub fn snapshot_mgtm_cli() -> CliCommandMap {
+    CliCommandMap::new().insert("notes", notes_cli())
+}
-- 
2.20.1

Re: [pve-devel] [PATCH proxmox-backup v2 1/1] client: add 'snapshot notes show/update' command

[Wolfgang Bumiller]

On Tue, Nov 24, 2020 at 10:09:29AM +0100, Dominik Csapak wrote:
> to show and update snapshot notes from the cli
> 
> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
> ---
>  src/bin/proxmox-backup-client.rs          |   1 +
>  src/bin/proxmox_backup_client/mod.rs      |   2 +
>  src/bin/proxmox_backup_client/snapshot.rs | 126 ++++++++++++++++++++++
>  3 files changed, 129 insertions(+)
>  create mode 100644 src/bin/proxmox_backup_client/snapshot.rs
> 
> diff --git a/src/bin/proxmox-backup-client.rs b/src/bin/proxmox-backup-client.rs
> index c961e390..f8a3fe70 100644
> --- a/src/bin/proxmox-backup-client.rs
> +++ b/src/bin/proxmox-backup-client.rs
> @@ -2110,6 +2110,7 @@ fn main() {
>          .insert("prune", prune_cmd_def)
>          .insert("restore", restore_cmd_def)
>          .insert("snapshots", snapshots_cmd_def)
> +        .insert("snapshot", snapshot_mgtm_cli())
>          .insert("files", files_cmd_def)
>          .insert("status", status_cmd_def)
>          .insert("key", key::cli())
> diff --git a/src/bin/proxmox_backup_client/mod.rs b/src/bin/proxmox_backup_client/mod.rs
> index 0c4bffb9..a14b0dc1 100644
> --- a/src/bin/proxmox_backup_client/mod.rs
> +++ b/src/bin/proxmox_backup_client/mod.rs
> @@ -8,6 +8,8 @@ mod task;
>  pub use task::*;
>  mod catalog;
>  pub use catalog::*;
> +mod snapshot;
> +pub use snapshot::*;
>  
>  pub mod key;
>  
> diff --git a/src/bin/proxmox_backup_client/snapshot.rs b/src/bin/proxmox_backup_client/snapshot.rs
> new file mode 100644
> index 00000000..fd5f543b
> --- /dev/null
> +++ b/src/bin/proxmox_backup_client/snapshot.rs
> @@ -0,0 +1,126 @@
> +use anyhow::Error;
> +use serde_json::{json, Value};
> +
> +use proxmox::api::{api, cli::*};
> +use proxmox_backup::tools;
> +
> +use crate::{
> +    complete_backup_snapshot, connect, extract_repository_from_value, BackupDir, REPO_URL_SCHEMA,
> +};
> +
> +#[api(
> +    input: {
> +        properties: {
> +            repository: {
> +                schema: REPO_URL_SCHEMA,
> +                optional: true,
> +            },
> +            snapshot: {
> +                type: String,
> +                description: "Snapshot path.",
> +            },
> +            "output-format": {
> +                schema: OUTPUT_FORMAT,
> +                optional: true,
> +            },
> +        }
> +    }
> +)]
> +/// Show notes
> +async fn show_notes(param: Value) -> Result<Value, Error> {
> +    let repo = extract_repository_from_value(&param)?;
> +    let path = tools::required_string_param(&param, "snapshot")?;

^ FYI a mix of regular params with a catch-all `param: Value` should
also work with an `#[api]` fn.

> +
> +    let snapshot: BackupDir = path.parse()?;
> +    let client = connect(&repo)?;
> +
> +    let path = format!("api2/json/admin/datastore/{}/notes", repo.store());
> +
> +    let args = json!({
> +        "backup-type": snapshot.group().backup_type(),
> +        "backup-id": snapshot.group().backup_id(),
> +        "backup-time": snapshot.backup_time(),
> +    });
> +
> +    let output_format = get_output_format(&param);
> +
> +    let mut result = client.get(&path, Some(args)).await?;
> +
> +    let notes = result["data"].take();
> +
> +    if output_format == "text" {
> +        if let Some(notes) = notes.as_str() {
> +            println!("{}", notes);
> +        }
> +    } else {
> +        format_and_print_result(
> +            &json!({
> +                "notes": notes,
> +            }),
> +            &output_format,
> +        );
> +    }
> +
> +    Ok(Value::Null)
> +}
> +
> +#[api(
> +    input: {
> +        properties: {
> +            repository: {
> +                schema: REPO_URL_SCHEMA,
> +                optional: true,
> +            },
> +            snapshot: {
> +                type: String,
> +                description: "Snapshot path.",
> +            },
> +            notes: {
> +                type: String,
> +                description: "The Notes.",
> +            },
> +        }
> +    }
> +)]
> +/// Update Notes
> +async fn update_notes(param: Value) -> Result<Value, Error> {
> +    let repo = extract_repository_from_value(&param)?;
> +    let path = tools::required_string_param(&param, "snapshot")?;
> +    let notes = tools::required_string_param(&param, "notes")?;
> +
> +    let snapshot: BackupDir = path.parse()?;
> +    let mut client = connect(&repo)?;
> +
> +    let path = format!("api2/json/admin/datastore/{}/notes", repo.store());
> +
> +    let args = json!({
> +        "backup-type": snapshot.group().backup_type(),
> +        "backup-id": snapshot.group().backup_id(),
> +        "backup-time": snapshot.backup_time(),
> +        "notes": notes,
> +    });
> +
> +    client.put(&path, Some(args)).await?;
> +
> +    Ok(Value::Null)
> +}
> +
> +fn notes_cli() -> CliCommandMap {
> +    CliCommandMap::new()
> +        .insert(
> +            "show",
> +            CliCommand::new(&API_METHOD_SHOW_NOTES)
> +                .arg_param(&["snapshot"])
> +                .completion_cb("snapshot", complete_backup_snapshot),
> +        )
> +        .insert(
> +            "update",
> +            CliCommand::new(&API_METHOD_UPDATE_NOTES)
> +                .arg_param(&["snapshot", "notes"])
> +                .completion_cb("snapshot", complete_backup_snapshot),
> +        )
> +}
> +
> +pub fn snapshot_mgtm_cli() -> CliCommandMap {
> +    CliCommandMap::new().insert("notes", notes_cli())
> +}
> -- 
> 2.20.1

[pve-devel] applied: [pbs-devel] [PATCH proxmox-backup v2 1/1] client: add 'snapshot notes show/update' command

[Dietmar Maurer]

applied

[pve-devel] [PATCH storage v2 1/4] rename comment to notes

[Dominik Csapak]

so that we are more consistent with pbs

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 PVE/API2/Storage/Content.pm |  4 ++--
 PVE/Storage/PBSPlugin.pm    |  2 +-
 PVE/Storage/Plugin.pm       | 10 +++++-----
 3 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/PVE/API2/Storage/Content.pm b/PVE/API2/Storage/Content.pm
index 8d2ff32..d4e2392 100644
--- a/PVE/API2/Storage/Content.pm
+++ b/PVE/API2/Storage/Content.pm
@@ -87,8 +87,8 @@ __PACKAGE__->register_method ({
 		    minimum => 0,
 		    optional => 1,
 		},
-		comment => {
-		    description => "An optional Comment.",
+		notes => {
+		    description => "Optional notes. If they contain multiple lines, only the first one is returned here.",
 		    type => 'string',
 		    optional => 1,
 		},
diff --git a/PVE/Storage/PBSPlugin.pm b/PVE/Storage/PBSPlugin.pm
index ef9bc79..fbffbdf 100644
--- a/PVE/Storage/PBSPlugin.pm
+++ b/PVE/Storage/PBSPlugin.pm
@@ -544,7 +544,7 @@ sub list_volumes {
 	};
 
 	$info->{verification} = $item->{verification} if defined($item->{verification});
-	$info->{comment} = $item->{comment} if defined($item->{comment});
+	$info->{notes} = $item->{comment} if defined($item->{comment});
 
 	push @$res, $info;
     }
diff --git a/PVE/Storage/Plugin.pm b/PVE/Storage/Plugin.pm
index c60d84b..1c86666 100644
--- a/PVE/Storage/Plugin.pm
+++ b/PVE/Storage/Plugin.pm
@@ -19,7 +19,7 @@ use base qw(PVE::SectionConfig);
 
 use constant COMPRESSOR_RE => 'gz|lzo|zst';
 
-use constant COMMENT_EXT => ".comment";
+use constant NOTES_EXT => ".notes";
 
 our @COMMON_TAR_FLAGS = qw(
     --one-file-system
@@ -1035,10 +1035,10 @@ my $get_subdir_files = sub {
 		$info->{vmid} = $vmid // $1;
 	    }
 
-	    my $comment_fn = $original.COMMENT_EXT;
-	    if (-f $comment_fn) {
-		my $comment = PVE::Tools::file_read_firstline($comment_fn);
-		$info->{comment} = $comment if defined($comment);
+	    my $notes_fn = $original.NOTES_EXT;
+	    if (-f $notes_fn) {
+		my $notes = PVE::Tools::file_read_firstline($notes_fn);
+		$info->{notes} = $notes if defined($notes);
 	    }
 
 	} elsif ($tt eq 'snippets') {
-- 
2.20.1

[pve-devel] [PATCH storage v2 2/4] api2/storage/content: change to volume_size_info and add return properties

[Dominik Csapak]

'file_size_info' only works for directory based storages, while
'volume_size_info' should work for all

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 PVE/API2/Storage/Content.pm | 29 ++++++++++++++++++++++++++---
 1 file changed, 26 insertions(+), 3 deletions(-)

diff --git a/PVE/API2/Storage/Content.pm b/PVE/API2/Storage/Content.pm
index d4e2392..7b81171 100644
--- a/PVE/API2/Storage/Content.pm
+++ b/PVE/API2/Storage/Content.pm
@@ -263,7 +263,30 @@ __PACKAGE__->register_method ({
 	    },
 	},
     },
-    returns => { type => 'object' },
+    returns => {
+	type => 'object',
+	properties => {
+	    path => {
+		description => "The Path",
+		type => 'string',
+	    },
+	    size => {
+		description => "Volume size in bytes.",
+		type => 'integer',
+		renderer => 'bytes',
+	    },
+	    used => {
+		description => "Used space. Please note that most storage plugins " .
+		"do not report anything useful here.",
+		type => 'integer',
+		renderer => 'bytes',
+	    },
+	    format => {
+		description => "Format identifier ('raw', 'qcow2', 'subvol', 'iso', 'tgz' ...)",
+		type => 'string',
+	    },
+	},
+    },
     code => sub {
 	my ($param) = @_;
 
@@ -277,8 +300,8 @@ __PACKAGE__->register_method ({
 	PVE::Storage::check_volume_access($rpcenv, $authuser, $cfg, undef, $volid);
 
 	my $path = PVE::Storage::path($cfg, $volid);
-	my ($size, $format, $used, $parent) =  PVE::Storage::file_size_info($path);
-	die "file_size_info on '$volid' failed\n" if !($format && $size);
+	my ($size, $format, $used, $parent) =  PVE::Storage::volume_size_info($cfg, $volid);
+	die "volume_size_info on '$volid' failed\n" if !($format && $size);
 
 	# fixme: return more attributes?
 	return {
-- 
2.20.1

[pve-devel] [PATCH storage v2 3/4] Storage/Plugin: add get/update_volume_comment and implement for dir

[Dominik Csapak]

and add the appropriate api call to set and get the comment
we need to bump APIVER for this and can bump APIAGE, since
we only use it at this new call that can work with the default
implementation

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 PVE/API2/Storage/Content.pm | 63 +++++++++++++++++++++++++++++++++++--
 PVE/Storage.pm              | 20 ++++++++++++
 PVE/Storage/DirPlugin.pm    | 30 ++++++++++++++++++
 PVE/Storage/Plugin.pm       | 12 +++++++
 4 files changed, 123 insertions(+), 2 deletions(-)

diff --git a/PVE/API2/Storage/Content.pm b/PVE/API2/Storage/Content.pm
index 7b81171..eac6343 100644
--- a/PVE/API2/Storage/Content.pm
+++ b/PVE/API2/Storage/Content.pm
@@ -285,6 +285,11 @@ __PACKAGE__->register_method ({
 		description => "Format identifier ('raw', 'qcow2', 'subvol', 'iso', 'tgz' ...)",
 		type => 'string',
 	    },
+	    notes => {
+		description => "Optional notes.",
+		optional => 1,
+		type => 'string',
+	    }
 	},
     },
     code => sub {
@@ -303,13 +308,67 @@ __PACKAGE__->register_method ({
 	my ($size, $format, $used, $parent) =  PVE::Storage::volume_size_info($cfg, $volid);
 	die "volume_size_info on '$volid' failed\n" if !($format && $size);
 
-	# fixme: return more attributes?
-	return {
+	my $entry = {
 	    path => $path,
 	    size => $size,
             used => $used,
 	    format => $format,
 	};
+
+	# not all storages/types support notes, so ignore errors here
+	eval {
+	    my $notes = PVE::Storage::get_volume_notes($cfg, $volid);
+	    $entry->{notes} = $notes if defined($notes);
+	};
+
+	return $entry;
+    }});
+
+__PACKAGE__->register_method ({
+    name => 'updateattributes',
+    path => '{volume}',
+    method => 'PUT',
+    description => "Update volume attributes",
+    permissions => {
+	description => "You need read access for the volume.",
+	user => 'all',
+    },
+    protected => 1,
+    proxyto => 'node',
+    parameters => {
+	additionalProperties => 0,
+	properties => {
+	    node => get_standard_option('pve-node'),
+	    storage => get_standard_option('pve-storage-id', { optional => 1 }),
+	    volume => {
+		description => "Volume identifier",
+		type => 'string',
+	    },
+	    notes => {
+		description => "The new notes.",
+		type => 'string',
+		optional => 1,
+	    },
+	},
+    },
+    returns => { type => 'null' },
+    code => sub {
+	my ($param) = @_;
+
+	my $rpcenv = PVE::RPCEnvironment::get();
+	my $authuser = $rpcenv->get_user();
+
+	my ($volid, $storeid) = &$real_volume_id($param->{storage}, $param->{volume});
+
+	my $cfg = PVE::Storage::config();
+
+	PVE::Storage::check_volume_access($rpcenv, $authuser, $cfg, undef, $volid);
+
+	if (my $notes = $param->{notes}) {
+	    PVE::Storage::update_volume_notes($cfg, $volid, $notes);
+	}
+
+	return undef;
     }});
 
 __PACKAGE__->register_method ({
diff --git a/PVE/Storage.pm b/PVE/Storage.pm
index 8d904d7..aded60e 100755
--- a/PVE/Storage.pm
+++ b/PVE/Storage.pm
@@ -217,6 +217,26 @@ sub file_size_info {
     return PVE::Storage::Plugin::file_size_info($filename, $timeout);
 }
 
+sub get_volume_notes {
+    my ($cfg, $volid, $timeout) = @_;
+
+    my ($storeid, $volname) = parse_volume_id($volid);
+    my $scfg = storage_config($cfg, $storeid);
+    my $plugin = PVE::Storage::Plugin->lookup($scfg->{type});
+
+    return $plugin->get_volume_notes($scfg, $storeid, $volname, $timeout);
+}
+
+sub update_volume_notes {
+    my ($cfg, $volid, $notes, $timeout) = @_;
+
+    my ($storeid, $volname) = parse_volume_id($volid);
+    my $scfg = storage_config($cfg, $storeid);
+    my $plugin = PVE::Storage::Plugin->lookup($scfg->{type});
+
+    $plugin->update_volume_notes($scfg, $storeid, $volname, $notes, $timeout);
+}
+
 sub volume_size_info {
     my ($cfg, $volid, $timeout) = @_;
 
diff --git a/PVE/Storage/DirPlugin.pm b/PVE/Storage/DirPlugin.pm
index 3c81d24..b36eec5 100644
--- a/PVE/Storage/DirPlugin.pm
+++ b/PVE/Storage/DirPlugin.pm
@@ -87,6 +87,36 @@ sub parse_is_mountpoint {
     return $is_mp; # contains a path
 }
 
+sub get_volume_notes {
+    my ($class, $scfg, $storeid, $volname, $timeout) = @_;
+    my $path = $class->filesystem_path($scfg, $volname);
+    $path .= $class->SUPER::NOTES_EXT;
+
+    my $notes = "";
+
+    if (-f $path) {
+	$notes = PVE::Tools::file_get_contents($path);
+    }
+
+    return $notes;
+}
+
+sub update_volume_notes {
+    my ($class, $scfg, $storeid, $volname, $notes, $timeout) = @_;
+    my $path = $class->filesystem_path($scfg, $volname);
+    my ($vtype, undef, undef, undef, undef, undef, undef) = $class->parse_volname($volname);
+
+    if ($vtype ne 'backup') {
+	die "only backups can have notes\n";
+    }
+
+    $path .= $class->SUPER::NOTES_EXT;
+
+    PVE::Tools::file_set_contents($path, $notes);
+
+    return undef;
+}
+
 sub status {
     my ($class, $storeid, $scfg, $cache) = @_;
 
diff --git a/PVE/Storage/Plugin.pm b/PVE/Storage/Plugin.pm
index 1c86666..57c58a9 100644
--- a/PVE/Storage/Plugin.pm
+++ b/PVE/Storage/Plugin.pm
@@ -826,6 +826,18 @@ sub file_size_info {
     return wantarray ? ($size, $format, $used, $parent, $st->ctime) : $size;
 }
 
+sub get_volume_notes {
+    my ($class, $scfg, $storeid, $volname, $timeout) = @_;
+
+    die "volume notes are not supported for $class";
+}
+
+sub update_volume_notes {
+    my ($class, $scfg, $storeid, $volname, $notes, $timeout) = @_;
+
+    die "volume notes are not supported for $class";
+}
+
 sub volume_size_info {
     my ($class, $scfg, $storeid, $volname, $timeout) = @_;
     my $path = $class->filesystem_path($scfg, $volname);
-- 
2.20.1

[pve-devel] [PATCH storage v2 4/4] Storage/PBSPlugin: implement get/update_volume_notes for pbs

[Dominik Csapak]

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 PVE/Storage/PBSPlugin.pm | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/PVE/Storage/PBSPlugin.pm b/PVE/Storage/PBSPlugin.pm
index fbffbdf..7d3db34 100644
--- a/PVE/Storage/PBSPlugin.pm
+++ b/PVE/Storage/PBSPlugin.pm
@@ -608,6 +608,26 @@ sub deactivate_volume {
     return 1;
 }
 
+sub get_volume_notes {
+    my ($class, $scfg, $storeid, $volname, $timeout) = @_;
+
+    my (undef, $name,  undef, undef, undef, undef, $format) = $class->parse_volname($volname);
+
+    my $data = run_client_cmd($scfg, $storeid, "snapshot", [ "notes", "show", $name ]);
+
+    return $data->{notes};
+}
+
+sub update_volume_notes {
+    my ($class, $scfg, $storeid, $volname, $notes, $timeout) = @_;
+
+    my (undef, $name,  undef, undef, undef, undef, $format) = $class->parse_volname($volname);
+
+    run_client_cmd($scfg, $storeid, "snapshot", [ "notes", "update", $name, $notes ], 1);
+
+    return undef;
+}
+
 sub volume_size_info {
     my ($class, $scfg, $storeid, $volname, $timeout) = @_;
 
-- 
2.20.1

[pve-devel] [PATCH manager v2 1/3] ui: change comment column to notes

[Dominik Csapak]

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 www/manager6/grid/BackupView.js     |  4 ++--
 www/manager6/storage/Browser.js     |  2 +-
 www/manager6/storage/ContentView.js | 12 ++++++------
 3 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/www/manager6/grid/BackupView.js b/www/manager6/grid/BackupView.js
index ec0b60b2..191b2e3f 100644
--- a/www/manager6/grid/BackupView.js
+++ b/www/manager6/grid/BackupView.js
@@ -238,8 +238,8 @@ Ext.define('PVE.grid.BackupView', {
 		    dataIndex: 'volid'
 		},
 		{
-		    header: gettext('Comment'),
-		    dataIndex: 'comment',
+		    header: gettext('Notes'),
+		    dataIndex: 'notes',
 		    flex: 1,
 		    renderer: Ext.htmlEncode,
 		},
diff --git a/www/manager6/storage/Browser.js b/www/manager6/storage/Browser.js
index 64c492d8..6efa0e65 100644
--- a/www/manager6/storage/Browser.js
+++ b/www/manager6/storage/Browser.js
@@ -53,7 +53,7 @@ Ext.define('PVE.storage.Browser', {
 		    title: gettext('Backups'),
 		    iconCls: 'fa fa-floppy-o',
 		    itemId: 'contentBackup',
-		    hasCommentColumn: true,
+		    hasNotesColumn: true,
 		});
 	    }
 	    if (contents.includes('images')) {
diff --git a/www/manager6/storage/ContentView.js b/www/manager6/storage/ContentView.js
index eb487553..a5e4cdee 100644
--- a/www/manager6/storage/ContentView.js
+++ b/www/manager6/storage/ContentView.js
@@ -333,11 +333,11 @@ Ext.define('PVE.storage.ContentView', {
 		renderer: PVE.Utils.render_storage_content,
 		dataIndex: 'text',
 	    },
-	    'comment': {
-		header: gettext('Comment'),
+	    'notes': {
+		header: gettext('Notes'),
 		flex: 1,
 		renderer: Ext.htmlEncode,
-		dataIndex: 'comment',
+		dataIndex: 'notes',
 	    },
 	    'date': {
 		header: gettext('Date'),
@@ -359,8 +359,8 @@ Ext.define('PVE.storage.ContentView', {
 
 	if (me.hideColumns) {
 	    me.hideColumns.forEach(key => delete availableColumns[key]);
-	} else if (!me.hasCommentColumn) {
-	    delete availableColumns.comment;
+	} else if (!me.hasNotesColumn) {
+	    delete availableColumns.notes;
 	}
 	const columns = Object.values(availableColumns);
 
@@ -381,7 +381,7 @@ Ext.define('PVE.storage.ContentView', {
 	extend: 'Ext.data.Model',
 	fields: [
 	    'volid', 'content', 'format', 'size', 'used', 'vmid',
-	    'channel', 'id', 'lun', 'comment', 'verification',
+	    'channel', 'id', 'lun', 'notes', 'verification',
 	    {
 		name: 'text',
 		convert: function(value, record) {
-- 
2.20.1

[pve-devel] [PATCH manager v2 2/3] ui: add ability to show and edit comments for backups

[Dominik Csapak]

for now we filter out pbs storages

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 www/manager6/grid/BackupView.js | 51 ++++++++++++++++++++++++++++++++-
 1 file changed, 50 insertions(+), 1 deletion(-)

diff --git a/www/manager6/grid/BackupView.js b/www/manager6/grid/BackupView.js
index 191b2e3f..d43f5a3f 100644
--- a/www/manager6/grid/BackupView.js
+++ b/www/manager6/grid/BackupView.js
@@ -227,7 +227,56 @@ Ext.define('PVE.grid.BackupView', {
 	    selModel: sm,
 	    tbar: {
 		overflowHandler: 'scroller',
-		items: [ backup_btn, restore_btn, delete_btn,config_btn, '->', storagesel, '-', vmidfilterCB, storagefilter ],
+		items: [
+		    backup_btn,
+		    restore_btn,
+		    delete_btn,
+		    config_btn,
+		    {
+			xtype: 'proxmoxButton',
+			text: gettext('Edit Notes'),
+			disabled: true,
+			enableFn: function() {
+			    let storageVal = storagesel.getValue();
+			    let storage = storagesel.getStore().findRecord('storage', storageVal);
+			    if (storage) {
+				return storage.data.type !== 'pbs';
+			    }
+			    return true;
+			},
+			handler: function() {
+			    let volid = sm.getSelection()[0].data.volid;
+			    var storage = storagesel.getValue();
+			    Ext.create('Proxmox.window.Edit', {
+				autoLoad: true,
+				width: 600,
+				height: 400,
+				resizable: true,
+				title: gettext('Comment'),
+				url: `/api2/extjs/nodes/${nodename}/storage/${storage}/content/${volid}`,
+				layout: 'fit',
+				items: [
+				    {
+					xtype: 'textarea',
+					layout: 'fit',
+					name: 'notes',
+					height: '100%',
+				    },
+				],
+				listeners: {
+				    destroy: function() {
+					reload();
+				    },
+				},
+			    }).show();
+			},
+		    },
+		    '->',
+		    storagesel,
+		    '-',
+		    vmidfilterCB,
+		    storagefilter
+		],
 	    },
 	    columns: [
 		{
-- 
2.20.1

[pve-devel] [PATCH manager v2 3/3] ui: enable notesedit for pbs

[Dominik Csapak]

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 www/manager6/grid/BackupView.js | 8 --------
 1 file changed, 8 deletions(-)

diff --git a/www/manager6/grid/BackupView.js b/www/manager6/grid/BackupView.js
index d43f5a3f..28326a79 100644
--- a/www/manager6/grid/BackupView.js
+++ b/www/manager6/grid/BackupView.js
@@ -236,14 +236,6 @@ Ext.define('PVE.grid.BackupView', {
 			xtype: 'proxmoxButton',
 			text: gettext('Edit Notes'),
 			disabled: true,
-			enableFn: function() {
-			    let storageVal = storagesel.getValue();
-			    let storage = storagesel.getStore().findRecord('storage', storageVal);
-			    if (storage) {
-				return storage.data.type !== 'pbs';
-			    }
-			    return true;
-			},
 			handler: function() {
 			    let volid = sm.getSelection()[0].data.volid;
 			    var storage = storagesel.getValue();
-- 
2.20.1

[pve-devel] applied: [PATCH manager v2 3/3] ui: enable notesedit for pbs

[Thomas Lamprecht]

On 24.11.20 10:09, Dominik Csapak wrote:
> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
> ---
>  www/manager6/grid/BackupView.js | 8 --------
>  1 file changed, 8 deletions(-)
> 
>

applied, had to resolve a rather trivial merge-conflict, as there was a patch
fixing findRecord usage in-between (fyi). thanks!

[pve-devel] applied-partially: [PATCH proxmox-backup/pve-storage/pve-manager v2] show/edit backup comments

[Thomas Lamprecht]

On 24.11.20 10:09, Dominik Csapak wrote:
> implement editing/showing notes from pve side
> for directory and pbs storages
> 
> this series technically consists of a breaking api change
> (comments -> notes) in content listing, but since we only just built
> packages with the relevant commits, this should be ok...
> 
> i also omitted the suggested 'snapshot list' part of proxmox-backup
> but will look into implementing an invisible alias for the clicommand
> in rust soon
> 
> changes from v1:
> * rename all occurences of 'comment' to 'notes' where possible
> (everywhere except the snapshots api of pbs)
> * split patches so that pbs ones could be applied later
> 


>   ui: enable notesedit for pbs


applied all but above, thanks! Please remind me to apply that one, once
we bumped PBS again.