* [pve-devel] [PATCH storage v3 01/13] plugin: export/import: fix calls to path() method
2024-12-19 10:43 [pve-devel] [PATCH storage/container v3 00/13] import/export for shared storages Fiona Ebner
@ 2024-12-19 10:43 ` Fiona Ebner
2024-12-19 10:43 ` [pve-devel] [PATCH storage v3 02/13] rbd plugin: schema: document default value for 'krbd' setting Fiona Ebner
` (12 subsequent siblings)
13 siblings, 0 replies; 17+ messages in thread
From: Fiona Ebner @ 2024-12-19 10:43 UTC (permalink / raw)
To: pve-devel
The plugin API does not require call context detection for the
returned value of the path() method. See other plugins like
ISCSIDirect/ZFS that do not implement it. So do not expect it for
external plugins either.
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
New in v3.
src/PVE/Storage/Plugin.pm | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/src/PVE/Storage/Plugin.pm b/src/PVE/Storage/Plugin.pm
index 5deff76..92609ad 100644
--- a/src/PVE/Storage/Plugin.pm
+++ b/src/PVE/Storage/Plugin.pm
@@ -1697,7 +1697,7 @@ sub volume_export {
my $err_msg = "volume export format $format not available for $class\n";
if ($scfg->{path} && !defined($snapshot) && !defined($base_snapshot)) {
- my $file = $class->path($scfg, $volname, $storeid) or die $err_msg;
+ my ($file) = $class->path($scfg, $volname, $storeid) or die $err_msg;
my $file_format = ($class->parse_volname($volname))[6];
my $size = file_size_info($file, undef, $file_format);
@@ -1731,7 +1731,7 @@ sub volume_export {
sub volume_export_formats {
my ($class, $scfg, $storeid, $volname, $snapshot, $base_snapshot, $with_snapshots) = @_;
if ($scfg->{path} && !defined($snapshot) && !defined($base_snapshot)) {
- my $file = $class->path($scfg, $volname, $storeid)
+ my ($file) = $class->path($scfg, $volname, $storeid)
or return;
my $format = ($class->parse_volname($volname))[6];
my $size = file_size_info($file, undef, $format);
@@ -1769,7 +1769,7 @@ sub volume_import {
# Check for an existing file first since interrupting alloc_image doesn't
# free it.
- my $file = $class->path($scfg, $volname, $storeid);
+ my ($file) = $class->path($scfg, $volname, $storeid);
if (-e $file) {
die "file '$file' already exists\n" if !$allow_rename;
warn "file '$file' already exists - importing with a different name\n";
@@ -1786,7 +1786,7 @@ sub volume_import {
if (defined($name) && $allocname ne $oldname) {
die "internal error: unexpected allocated name: '$allocname' != '$oldname'\n";
}
- my $file = $class->path($scfg, $volname, $storeid)
+ my ($file) = $class->path($scfg, $volname, $storeid)
or die "internal error: failed to get path to newly allocated volume $volname\n";
if ($data_format eq 'raw' || $data_format eq 'qcow2' || $data_format eq 'vmdk') {
run_command(['dd', "of=$file", 'conv=sparse', 'bs=64k'],
--
2.39.5
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 17+ messages in thread* [pve-devel] [PATCH storage v3 02/13] rbd plugin: schema: document default value for 'krbd' setting
2024-12-19 10:43 [pve-devel] [PATCH storage/container v3 00/13] import/export for shared storages Fiona Ebner
2024-12-19 10:43 ` [pve-devel] [PATCH storage v3 01/13] plugin: export/import: fix calls to path() method Fiona Ebner
@ 2024-12-19 10:43 ` Fiona Ebner
2024-12-19 10:43 ` [pve-devel] [PATCH storage v3 03/13] export: redirect stdout to avoid any unrelated messages ending up in the export stream Fiona Ebner
` (11 subsequent siblings)
13 siblings, 0 replies; 17+ messages in thread
From: Fiona Ebner @ 2024-12-19 10:43 UTC (permalink / raw)
To: pve-devel
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
src/PVE/Storage/RBDPlugin.pm | 1 +
1 file changed, 1 insertion(+)
diff --git a/src/PVE/Storage/RBDPlugin.pm b/src/PVE/Storage/RBDPlugin.pm
index f45ad3f..680e922 100644
--- a/src/PVE/Storage/RBDPlugin.pm
+++ b/src/PVE/Storage/RBDPlugin.pm
@@ -389,6 +389,7 @@ sub properties {
krbd => {
description => "Always access rbd through krbd kernel module.",
type => 'boolean',
+ default => 0,
},
keyring => {
description => "Client keyring contents (for external clusters).",
--
2.39.5
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 17+ messages in thread* [pve-devel] [PATCH storage v3 03/13] export: redirect stdout to avoid any unrelated messages ending up in the export stream
2024-12-19 10:43 [pve-devel] [PATCH storage/container v3 00/13] import/export for shared storages Fiona Ebner
2024-12-19 10:43 ` [pve-devel] [PATCH storage v3 01/13] plugin: export/import: fix calls to path() method Fiona Ebner
2024-12-19 10:43 ` [pve-devel] [PATCH storage v3 02/13] rbd plugin: schema: document default value for 'krbd' setting Fiona Ebner
@ 2024-12-19 10:43 ` Fiona Ebner
2024-12-19 10:43 ` [pve-devel] [PATCH storage v3 04/13] rbd plugin: factor out helper to check if volume already exists Fiona Ebner
` (10 subsequent siblings)
13 siblings, 0 replies; 17+ messages in thread
From: Fiona Ebner @ 2024-12-19 10:43 UTC (permalink / raw)
To: pve-devel
Current export implementations luckily seems to not run into this
issue yet. However, for the upcoming implementation for RBD, mapping a
volume would print the device path to STDOUT, thus messing up the
export stream.
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
src/PVE/CLI/pvesm.pm | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/src/PVE/CLI/pvesm.pm b/src/PVE/CLI/pvesm.pm
index 9b9676b..d308b3d 100755
--- a/src/PVE/CLI/pvesm.pm
+++ b/src/PVE/CLI/pvesm.pm
@@ -315,7 +315,10 @@ __PACKAGE__->register_method ({
my $outfh;
if ($filename eq '-') {
- $outfh = \*STDOUT;
+ # No other messages must go to STDOUT if it's used for the export stream!
+ open($outfh, '>&', STDOUT) or die "unable to dup() STDOUT - $!\n";
+ close(STDOUT);
+ open(STDOUT, '>', '/dev/null');
} else {
sysopen($outfh, $filename, O_CREAT|O_WRONLY|O_TRUNC)
or die "open($filename): $!\n";
--
2.39.5
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 17+ messages in thread* [pve-devel] [PATCH storage v3 04/13] rbd plugin: factor out helper to check if volume already exists
2024-12-19 10:43 [pve-devel] [PATCH storage/container v3 00/13] import/export for shared storages Fiona Ebner
` (2 preceding siblings ...)
2024-12-19 10:43 ` [pve-devel] [PATCH storage v3 03/13] export: redirect stdout to avoid any unrelated messages ending up in the export stream Fiona Ebner
@ 2024-12-19 10:43 ` Fiona Ebner
2024-12-19 10:43 ` [pve-devel] [PATCH storage v3 05/13] rbd plugin: implement volume import/export Fiona Ebner
` (9 subsequent siblings)
13 siblings, 0 replies; 17+ messages in thread
From: Fiona Ebner @ 2024-12-19 10:43 UTC (permalink / raw)
To: pve-devel
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
src/PVE/Storage/RBDPlugin.pm | 17 ++++++++++++-----
1 file changed, 12 insertions(+), 5 deletions(-)
diff --git a/src/PVE/Storage/RBDPlugin.pm b/src/PVE/Storage/RBDPlugin.pm
index 680e922..301918c 100644
--- a/src/PVE/Storage/RBDPlugin.pm
+++ b/src/PVE/Storage/RBDPlugin.pm
@@ -348,6 +348,16 @@ sub rbd_volume_du {
die "got no matching image from rbd du\n";
}
+my sub rbd_volume_exists {
+ my ($scfg, $storeid, $volname) = @_;
+
+ eval {
+ my $cmd = $rbd_cmd->($scfg, $storeid, 'info', $volname);
+ run_rbd_command($cmd, errmsg => "exist check", quiet => 1);
+ };
+ return $@ ? undef : 1;
+}
+
# Configuration
sub type {
@@ -869,11 +879,8 @@ sub rename_volume {
$target_volname = $class->find_free_diskname($storeid, $scfg, $target_vmid, $format)
if !$target_volname;
- eval {
- my $cmd = $rbd_cmd->($scfg, $storeid, 'info', $target_volname);
- run_rbd_command($cmd, errmsg => "exist check", quiet => 1);
- };
- die "target volume '${target_volname}' already exists\n" if !$@;
+ die "target volume '${target_volname}' already exists\n"
+ if rbd_volume_exists($scfg, $storeid, $target_volname);
my $cmd = $rbd_cmd->($scfg, $storeid, 'rename', $source_image, $target_volname);
--
2.39.5
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 17+ messages in thread* [pve-devel] [PATCH storage v3 05/13] rbd plugin: implement volume import/export
2024-12-19 10:43 [pve-devel] [PATCH storage/container v3 00/13] import/export for shared storages Fiona Ebner
` (3 preceding siblings ...)
2024-12-19 10:43 ` [pve-devel] [PATCH storage v3 04/13] rbd plugin: factor out helper to check if volume already exists Fiona Ebner
@ 2024-12-19 10:43 ` Fiona Ebner
2024-12-19 10:43 ` [pve-devel] [PATCH storage v3 06/13] rbd plugin: improve volume exists helper Fiona Ebner
` (8 subsequent siblings)
13 siblings, 0 replies; 17+ messages in thread
From: Fiona Ebner @ 2024-12-19 10:43 UTC (permalink / raw)
To: pve-devel
For now, only 'raw+size' is supported and it's not possible to
export/import with snapshots. The volume or snapshot is exported or
imported via the corresponding 'rbd' commands.
Introducing an 'rbd' transport format might be feasible for more
complete (i.e. with snapshots, incremental) transfer between two RBD
storages.
Use the '--dest-pool' switch rather than '-p' for import, because the
latter is deprecated.
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
Changes in v3:
* use rbd export/import
src/PVE/Storage/RBDPlugin.pm | 102 ++++++++++++++++++++++++++++++++++-
1 file changed, 101 insertions(+), 1 deletion(-)
diff --git a/src/PVE/Storage/RBDPlugin.pm b/src/PVE/Storage/RBDPlugin.pm
index 301918c..44770b0 100644
--- a/src/PVE/Storage/RBDPlugin.pm
+++ b/src/PVE/Storage/RBDPlugin.pm
@@ -88,7 +88,13 @@ my $build_cmd = sub {
my $cmd_option = PVE::CephConfig::ceph_connect_option($scfg, $storeid);
my $pool = $scfg->{pool} ? $scfg->{pool} : 'rbd';
- my $cmd = [$binary, '-p', $pool];
+
+ my $cmd = [$binary];
+ if ($op eq 'import') {
+ push $cmd->@*, '--dest-pool', $pool;
+ } else {
+ push $cmd->@*, '-p', $pool;
+ }
if (defined(my $namespace = $scfg->{namespace})) {
# some subcommands will fail if the --namespace parameter is present
@@ -864,6 +870,100 @@ sub volume_has_feature {
return undef;
}
+sub volume_export_formats {
+ my ($class, $scfg, $storeid, $volname, $snapshot, $base_snapshot, $with_snapshots) = @_;
+
+ return $class->volume_import_formats(
+ $scfg, $storeid, $volname, $snapshot, $base_snapshot, $with_snapshots);
+}
+
+sub volume_export {
+ my (
+ $class,
+ $scfg,
+ $storeid,
+ $fh,
+ $volname,
+ $format,
+ $snapshot,
+ $base_snapshot,
+ $with_snapshots,
+ ) = @_;
+
+ die "volume export format $format not available for $class\n" if $format ne 'raw+size';
+ die "cannot export volumes together with their snapshots in $class\n" if $with_snapshots;
+ die "cannot export an incremental stream in $class\n" if defined($base_snapshot);
+
+ my ($size) = $class->volume_size_info($scfg, $storeid, $volname);
+ PVE::Storage::Plugin::write_common_header($fh, $size);
+ my $cmd = $rbd_cmd->($scfg, $storeid, 'export', '--export-format', '1', $volname, '-');
+ run_rbd_command(
+ $cmd,
+ errmsg => 'could not export image',
+ output => '>&'.fileno($fh),
+ );
+
+ return;
+}
+
+sub volume_import_formats {
+ my ($class, $scfg, $storeid, $volname, $snapshot, $base_snapshot, $with_snapshots) = @_;
+ return () if $with_snapshots; # not supported
+ return () if defined($base_snapshot); # not supported
+ return ('raw+size');
+}
+
+sub volume_import {
+ my (
+ $class,
+ $scfg,
+ $storeid,
+ $fh,
+ $volname,
+ $format,
+ $snapshot,
+ $base_snapshot,
+ $with_snapshots,
+ $allow_rename,
+ ) = @_;
+
+ die "volume import format $format not available for $class\n" if $format ne 'raw+size';
+ die "cannot import volumes together with their snapshots in $class\n" if $with_snapshots;
+ die "cannot import an incremental stream in $class\n" if defined($base_snapshot);
+
+ my (undef, $name, $vmid, undef, undef, undef, $file_format) = $class->parse_volname($volname);
+ die "cannot import format $format into a volume of format $file_format\n"
+ if $file_format ne 'raw';
+
+ if (rbd_volume_exists($scfg, $storeid, $name)) {
+ die "volume $name already exists\n" if !$allow_rename;
+ warn "volume $name already exists - importing with a different name\n";
+ $volname = $class->find_free_diskname($storeid, $scfg, $vmid, $file_format);
+ }
+
+ my ($size) = PVE::Storage::Plugin::read_common_header($fh);
+ $size = int($size/1024);
+
+ eval {
+ my $cmd = $rbd_cmd->($scfg, $storeid, 'import', '--export-format', '1', '-', $volname);
+ run_rbd_command(
+ $cmd,
+ errmsg => 'could not import image',
+ input => '<&'.fileno($fh),
+ );
+ };
+ if (my $err = $@) {
+ # FIXME there is a slight race between finding the free disk name and removal here
+ # Does not only affect this plugin, see:
+ # https://lore.proxmox.com/pve-devel/20240403150712.262773-1-h.duerr@proxmox.com/
+ eval { $class->free_image($storeid, $scfg, $volname, 0, $file_format); };
+ warn $@ if $@;
+ die $err;
+ }
+
+ return "$storeid:$volname";
+}
+
sub rename_volume {
my ($class, $scfg, $storeid, $source_volname, $target_vmid, $target_volname) = @_;
--
2.39.5
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 17+ messages in thread* [pve-devel] [PATCH storage v3 06/13] rbd plugin: improve volume exists helper
2024-12-19 10:43 [pve-devel] [PATCH storage/container v3 00/13] import/export for shared storages Fiona Ebner
` (4 preceding siblings ...)
2024-12-19 10:43 ` [pve-devel] [PATCH storage v3 05/13] rbd plugin: implement volume import/export Fiona Ebner
@ 2024-12-19 10:43 ` Fiona Ebner
2024-12-19 10:43 ` [pve-devel] [PATCH storage v3 07/13] iscsi plugin: support volume export Fiona Ebner
` (7 subsequent siblings)
13 siblings, 0 replies; 17+ messages in thread
From: Fiona Ebner @ 2024-12-19 10:43 UTC (permalink / raw)
To: pve-devel
Currently, the helper would not distinguish between different kinds
of errors. Instead of relying on an error, list the images and check
there.
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
Changes in v3:
* do not match error message, list images instead
src/PVE/Storage/RBDPlugin.pm | 23 ++++++++++++++++++-----
1 file changed, 18 insertions(+), 5 deletions(-)
diff --git a/src/PVE/Storage/RBDPlugin.pm b/src/PVE/Storage/RBDPlugin.pm
index 44770b0..1d1cfad 100644
--- a/src/PVE/Storage/RBDPlugin.pm
+++ b/src/PVE/Storage/RBDPlugin.pm
@@ -357,11 +357,24 @@ sub rbd_volume_du {
my sub rbd_volume_exists {
my ($scfg, $storeid, $volname) = @_;
- eval {
- my $cmd = $rbd_cmd->($scfg, $storeid, 'info', $volname);
- run_rbd_command($cmd, errmsg => "exist check", quiet => 1);
- };
- return $@ ? undef : 1;
+ my $cmd = $rbd_cmd->($scfg, $storeid, 'ls', '--format', 'json');
+ my $raw = '';
+ run_rbd_command(
+ $cmd, errmsg => "rbd error", errfunc => sub {}, outfunc => sub { $raw .= shift; });
+
+ my $list;
+ if ($raw =~ m/^(\[.*\])$/s) { # untaint
+ $list = eval { JSON::decode_json($1); };
+ die "invalid JSON output from 'rbd ls': $@\n" if $@;
+ } else {
+ die "got unexpected data from 'rbd ls': '$raw'\n";
+ }
+
+ for my $name ($list->@*) {
+ return 1 if $name eq $volname;
+ }
+
+ return 0;
}
# Configuration
--
2.39.5
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 17+ messages in thread* [pve-devel] [PATCH storage v3 07/13] iscsi plugin: support volume export
2024-12-19 10:43 [pve-devel] [PATCH storage/container v3 00/13] import/export for shared storages Fiona Ebner
` (5 preceding siblings ...)
2024-12-19 10:43 ` [pve-devel] [PATCH storage v3 06/13] rbd plugin: improve volume exists helper Fiona Ebner
@ 2024-12-19 10:43 ` Fiona Ebner
2024-12-19 10:43 ` [pve-devel] [PATCH storage v3 08/13] iscsi direct " Fiona Ebner
` (6 subsequent siblings)
13 siblings, 0 replies; 17+ messages in thread
From: Fiona Ebner @ 2024-12-19 10:43 UTC (permalink / raw)
To: pve-devel
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
src/PVE/Storage/ISCSIPlugin.pm | 48 ++++++++++++++++++++++++++++++++++
1 file changed, 48 insertions(+)
diff --git a/src/PVE/Storage/ISCSIPlugin.pm b/src/PVE/Storage/ISCSIPlugin.pm
index 6de7610..eb70453 100644
--- a/src/PVE/Storage/ISCSIPlugin.pm
+++ b/src/PVE/Storage/ISCSIPlugin.pm
@@ -596,5 +596,53 @@ sub volume_has_feature {
return undef;
}
+sub volume_export_formats {
+ my ($class, $scfg, $storeid, $volname, $snapshot, $base_snapshot, $with_snapshots) = @_;
+
+ return () if defined($snapshot); # not supported
+ return () if defined($base_snapshot); # not supported
+ return () if $with_snapshots; # not supported
+ return ('raw+size');
+}
+
+sub volume_export {
+ my (
+ $class,
+ $scfg,
+ $storeid,
+ $fh,
+ $volname,
+ $format,
+ $snapshot,
+ $base_snapshot,
+ $with_snapshots,
+ ) = @_;
+
+ die "volume export format $format not available for $class\n" if $format ne 'raw+size';
+ die "cannot export volumes together with their snapshots in $class\n" if $with_snapshots;
+ die "cannot export an incremental stream in $class\n" if defined($base_snapshot);
+ die "cannot export a snapshot in $class\n" if defined($snapshot);
+
+ my $file = $class->filesystem_path($scfg, $volname, $snapshot);
+ my $size;
+ run_command(['/sbin/blockdev', '--getsize64', $file], outfunc => sub {
+ my ($line) = @_;
+ die "unexpected output from /sbin/blockdev: $line\n" if $line !~ /^(\d+)$/;
+ $size = int($1);
+ });
+ PVE::Storage::Plugin::write_common_header($fh, $size);
+ run_command(['dd', "if=$file", "bs=64k", "status=progress"], output => '>&'.fileno($fh));
+ return;
+}
+
+sub volume_import_formats {
+ my ($class, $scfg, $storeid, $volname, $snapshot, $base_snapshot, $with_snapshots) = @_;
+
+ return ();
+}
+
+sub volume_import {
+ die "volume import is not possible on iscsi storage\n";
+}
1;
--
2.39.5
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 17+ messages in thread* [pve-devel] [PATCH storage v3 08/13] iscsi direct plugin: support volume export
2024-12-19 10:43 [pve-devel] [PATCH storage/container v3 00/13] import/export for shared storages Fiona Ebner
` (6 preceding siblings ...)
2024-12-19 10:43 ` [pve-devel] [PATCH storage v3 07/13] iscsi plugin: support volume export Fiona Ebner
@ 2024-12-19 10:43 ` Fiona Ebner
2024-12-19 10:46 ` Fiona Ebner
2024-12-19 10:43 ` [pve-devel] [PATCH storage v3 09/13] common: introduce common module Fiona Ebner
` (5 subsequent siblings)
13 siblings, 1 reply; 17+ messages in thread
From: Fiona Ebner @ 2024-12-19 10:43 UTC (permalink / raw)
To: pve-devel
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
src/PVE/Storage/ISCSIDirectPlugin.pm | 63 ++++++++++++++++++++++++++++
1 file changed, 63 insertions(+)
diff --git a/src/PVE/Storage/ISCSIDirectPlugin.pm b/src/PVE/Storage/ISCSIDirectPlugin.pm
index eb329d4..0aca4b6 100644
--- a/src/PVE/Storage/ISCSIDirectPlugin.pm
+++ b/src/PVE/Storage/ISCSIDirectPlugin.pm
@@ -2,9 +2,12 @@ package PVE::Storage::ISCSIDirectPlugin;
use strict;
use warnings;
+
use IO::File;
+use JSON qw(decode_json);
use HTTP::Request;
use LWP::UserAgent;
+
use PVE::Tools qw(run_command file_read_firstline trim dir_glob_regex dir_glob_foreach);
use PVE::Storage::Plugin;
use PVE::JSONSchema qw(get_standard_option);
@@ -252,4 +255,64 @@ sub volume_has_feature {
return undef;
}
+sub volume_export_formats {
+ my ($class, $scfg, $storeid, $volname, $snapshot, $base_snapshot, $with_snapshots) = @_;
+
+ return () if defined($snapshot); # not supported
+ return () if defined($base_snapshot); # not supported
+ return () if $with_snapshots; # not supported
+ return ('raw+size');
+}
+
+sub volume_export {
+ my (
+ $class,
+ $scfg,
+ $storeid,
+ $fh,
+ $volname,
+ $format,
+ $snapshot,
+ $base_snapshot,
+ $with_snapshots,
+ ) = @_;
+
+ die "volume export format $format not available for $class\n" if $format ne 'raw+size';
+ die "cannot export volumes together with their snapshots in $class\n" if $with_snapshots;
+ die "cannot export an incremental stream in $class\n" if defined($base_snapshot);
+ die "cannot export a snapshot in $class\n" if defined($snapshot);
+
+ my ($file) = $class->path($scfg, $volname, $storeid, $snapshot);
+
+ my $json = '';
+ run_command(
+ ['/usr/bin/qemu-img', 'info', '-f', 'raw', '--output=json', $file],
+ outfunc => sub { $json .= shift },
+ );
+ die "failed to query size information for '$file' with qemu-img\n" if !$json;
+ my $info = eval { decode_json($json) };
+ die "could not parse qemu-img info command output for '$file' - $@\n" if $@;
+
+ my ($size) = ($info->{'virtual-size'} =~ /^(\d+)$/); # untaint
+ die "size '$size' not an integer\n" if !defined($size);
+ $size = int($size); # coerce back from string
+
+ PVE::Storage::Plugin::write_common_header($fh, $size);
+ run_command(
+ ['qemu-img', 'dd', 'bs=64k', "if=$file", '-f', 'raw', '-O', 'raw'],
+ output => '>&'.fileno($fh),
+ );
+ return;
+}
+
+sub volume_import_formats {
+ my ($class, $scfg, $storeid, $volname, $snapshot, $base_snapshot, $with_snapshots) = @_;
+
+ return ();
+}
+
+sub volume_import {
+ die "volume import is not possible on iscsi storage\n";
+}
+
1;
--
2.39.5
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 17+ messages in thread* [pve-devel] [PATCH storage v3 09/13] common: introduce common module
2024-12-19 10:43 [pve-devel] [PATCH storage/container v3 00/13] import/export for shared storages Fiona Ebner
` (7 preceding siblings ...)
2024-12-19 10:43 ` [pve-devel] [PATCH storage v3 08/13] iscsi direct " Fiona Ebner
@ 2024-12-19 10:43 ` Fiona Ebner
2024-12-19 10:43 ` [pve-devel] [PATCH storage v3 10/13] plugins: volume import: align size up to 1KiB Fiona Ebner
` (4 subsequent siblings)
13 siblings, 0 replies; 17+ messages in thread
From: Fiona Ebner @ 2024-12-19 10:43 UTC (permalink / raw)
To: pve-devel
From: Max Carrara <m.carrara@proxmox.com>
This module's purpose is to provide shared functions, constants, etc.
for storage plugins and storage-related operations.
It starts out with a align_size_up() function, that will (initially)
be used for volume import.
Originally-by: Max Carrara <m.carrara@proxmox.com>
[FE: start out with a different function for my use case
fixup Makefile]
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
Changes in v3:
* fix commit message
src/PVE/Storage/Common.pm | 54 +++++++++++++++++++++++++++++++++
src/PVE/Storage/Common/Makefile | 6 ++++
src/PVE/Storage/Makefile | 2 ++
3 files changed, 62 insertions(+)
create mode 100644 src/PVE/Storage/Common.pm
create mode 100644 src/PVE/Storage/Common/Makefile
diff --git a/src/PVE/Storage/Common.pm b/src/PVE/Storage/Common.pm
new file mode 100644
index 0000000..3ae20dd
--- /dev/null
+++ b/src/PVE/Storage/Common.pm
@@ -0,0 +1,54 @@
+package PVE::Storage::Common;
+
+use strict;
+use warnings;
+
+=pod
+
+=head1 NAME
+
+PVE::Storage::Common - Shared functions and utilities for storage plugins and storage operations
+
+=head1 DESCRIPTION
+
+This module contains common subroutines that are mainly to be used by storage
+plugins. This module's submodules contain subroutines that are tailored towards
+a more specific or related purpose.
+
+Functions concerned with storage-related C<PVE::SectionConfig> things, helpers
+for the C<PVE::Storage> API can be found in this module. Functions that can't
+be grouped in a submodule can also be found here.
+
+=head1 SUBMODULES
+
+=over
+
+=back
+
+=head1 FUNCTIONS
+
+=cut
+
+=pod
+
+=head3 align_size_up
+
+ $aligned_size = align_size_up($size, $granularity)
+
+Returns the next size bigger than or equal to C<$size> that is aligned with a
+granularity of C<$granularity>. Prints a message if the aligned size is not
+equal to the aligned size.
+
+=cut
+
+sub align_size_up : prototype($$) {
+ my ($size, $granularity) = @_;
+
+ my $padding = ($granularity - $size % $granularity) % $granularity;
+ my $aligned_size = $size + $padding;
+ print "size $size is not aligned to granularity $granularity, rounding up to $aligned_size\n"
+ if $aligned_size != $size;
+ return $aligned_size;
+}
+
+1;
diff --git a/src/PVE/Storage/Common/Makefile b/src/PVE/Storage/Common/Makefile
new file mode 100644
index 0000000..0c4bba5
--- /dev/null
+++ b/src/PVE/Storage/Common/Makefile
@@ -0,0 +1,6 @@
+SOURCES = \
+
+
+.PHONY: install
+install:
+ for i in ${SOURCES}; do install -D -m 0644 $$i ${DESTDIR}${PERLDIR}/PVE/Storage/Common/$$i; done
diff --git a/src/PVE/Storage/Makefile b/src/PVE/Storage/Makefile
index d5cc942..ce3fd68 100644
--- a/src/PVE/Storage/Makefile
+++ b/src/PVE/Storage/Makefile
@@ -1,4 +1,5 @@
SOURCES= \
+ Common.pm \
Plugin.pm \
DirPlugin.pm \
LVMPlugin.pm \
@@ -18,5 +19,6 @@ SOURCES= \
.PHONY: install
install:
+ make -C Common install
for i in ${SOURCES}; do install -D -m 0644 $$i ${DESTDIR}${PERLDIR}/PVE/Storage/$$i; done
make -C LunCmd install
--
2.39.5
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 17+ messages in thread* [pve-devel] [PATCH storage v3 10/13] plugins: volume import: align size up to 1KiB
2024-12-19 10:43 [pve-devel] [PATCH storage/container v3 00/13] import/export for shared storages Fiona Ebner
` (8 preceding siblings ...)
2024-12-19 10:43 ` [pve-devel] [PATCH storage v3 09/13] common: introduce common module Fiona Ebner
@ 2024-12-19 10:43 ` Fiona Ebner
2024-12-19 10:43 ` [pve-devel] [PATCH storage v3 11/13] rbd plugin: list: drop outdated error message check Fiona Ebner
` (3 subsequent siblings)
13 siblings, 0 replies; 17+ messages in thread
From: Fiona Ebner @ 2024-12-19 10:43 UTC (permalink / raw)
To: pve-devel
Previously, the size was rounded down which, in case of an image with
non-1KiB-aligned sze (only possible for external plugins or manually
created images) would lead to errors when attempting to write beyond
the end of the too small allocated target image.
For image allocation, the size is already rounded up to the
granularity of the storage. Do the same for import.
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
src/PVE/Storage/LVMPlugin.pm | 4 +++-
src/PVE/Storage/Plugin.pm | 4 +++-
src/PVE/Storage/RBDPlugin.pm | 4 +++-
3 files changed, 9 insertions(+), 3 deletions(-)
diff --git a/src/PVE/Storage/LVMPlugin.pm b/src/PVE/Storage/LVMPlugin.pm
index 88fd612..38f7fa1 100644
--- a/src/PVE/Storage/LVMPlugin.pm
+++ b/src/PVE/Storage/LVMPlugin.pm
@@ -9,6 +9,8 @@ use PVE::Tools qw(run_command trim);
use PVE::Storage::Plugin;
use PVE::JSONSchema qw(get_standard_option);
+use PVE::Storage::Common;
+
use base qw(PVE::Storage::Plugin);
# lvm helper functions
@@ -677,7 +679,7 @@ sub volume_import {
}
my ($size) = PVE::Storage::Plugin::read_common_header($fh);
- $size = int($size/1024);
+ $size = PVE::Storage::Common::align_size_up($size, 1024) / 1024;
eval {
my $allocname = $class->alloc_image($storeid, $scfg, $vmid, 'raw', $name, $size);
diff --git a/src/PVE/Storage/Plugin.pm b/src/PVE/Storage/Plugin.pm
index 92609ad..65cf43f 100644
--- a/src/PVE/Storage/Plugin.pm
+++ b/src/PVE/Storage/Plugin.pm
@@ -15,6 +15,8 @@ use PVE::Tools qw(run_command);
use PVE::JSONSchema qw(get_standard_option register_standard_option);
use PVE::Cluster qw(cfs_register_file);
+use PVE::Storage::Common;
+
use JSON;
use base qw(PVE::SectionConfig);
@@ -1777,7 +1779,7 @@ sub volume_import {
}
my ($size) = read_common_header($fh);
- $size = int($size/1024);
+ $size = PVE::Storage::Common::align_size_up($size, 1024) / 1024;
eval {
my $allocname = $class->alloc_image($storeid, $scfg, $vmid, $file_format, $name, $size);
diff --git a/src/PVE/Storage/RBDPlugin.pm b/src/PVE/Storage/RBDPlugin.pm
index 1d1cfad..a362529 100644
--- a/src/PVE/Storage/RBDPlugin.pm
+++ b/src/PVE/Storage/RBDPlugin.pm
@@ -18,6 +18,8 @@ use PVE::RPCEnvironment;
use PVE::Storage::Plugin;
use PVE::Tools qw(run_command trim file_read_firstline);
+use PVE::Storage::Common;
+
use base qw(PVE::Storage::Plugin);
my $get_parent_image_name = sub {
@@ -955,7 +957,7 @@ sub volume_import {
}
my ($size) = PVE::Storage::Plugin::read_common_header($fh);
- $size = int($size/1024);
+ $size = PVE::Storage::Common::align_size_up($size, 1024) / 1024;
eval {
my $cmd = $rbd_cmd->($scfg, $storeid, 'import', '--export-format', '1', '-', $volname);
--
2.39.5
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 17+ messages in thread* [pve-devel] [PATCH storage v3 11/13] rbd plugin: list: drop outdated error message check
2024-12-19 10:43 [pve-devel] [PATCH storage/container v3 00/13] import/export for shared storages Fiona Ebner
` (9 preceding siblings ...)
2024-12-19 10:43 ` [pve-devel] [PATCH storage v3 10/13] plugins: volume import: align size up to 1KiB Fiona Ebner
@ 2024-12-19 10:43 ` Fiona Ebner
2024-12-19 10:43 ` [pve-devel] [PATCH container v3 12/13] migration: allow rbd storages for remote migration Fiona Ebner
` (2 subsequent siblings)
13 siblings, 0 replies; 17+ messages in thread
From: Fiona Ebner @ 2024-12-19 10:43 UTC (permalink / raw)
To: pve-devel
This became outdated after Ceph commit ac547a5b7dc ("rbd: return 0 and
an empty list when pool is entirely empty") 11 years ago. See also:
https://tracker.ceph.com/issues/6693
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
New in v3.
src/PVE/Storage/RBDPlugin.pm | 7 +------
1 file changed, 1 insertion(+), 6 deletions(-)
diff --git a/src/PVE/Storage/RBDPlugin.pm b/src/PVE/Storage/RBDPlugin.pm
index a362529..ef4faa6 100644
--- a/src/PVE/Storage/RBDPlugin.pm
+++ b/src/PVE/Storage/RBDPlugin.pm
@@ -216,12 +216,7 @@ sub rbd_ls {
my $parser = sub { $raw .= shift };
my $cmd = $rbd_cmd->($scfg, $storeid, 'ls', '-l', '--format', 'json');
- eval {
- run_rbd_command($cmd, errmsg => "rbd error", errfunc => sub {}, outfunc => $parser);
- };
- my $err = $@;
-
- die $err if $err && $err !~ m/doesn't contain rbd images/ ;
+ run_rbd_command($cmd, errmsg => "rbd error", errfunc => sub {}, outfunc => $parser);
my $result;
if ($raw eq '') {
--
2.39.5
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 17+ messages in thread* [pve-devel] [PATCH container v3 12/13] migration: allow rbd storages for remote migration
2024-12-19 10:43 [pve-devel] [PATCH storage/container v3 00/13] import/export for shared storages Fiona Ebner
` (10 preceding siblings ...)
2024-12-19 10:43 ` [pve-devel] [PATCH storage v3 11/13] rbd plugin: list: drop outdated error message check Fiona Ebner
@ 2024-12-19 10:43 ` Fiona Ebner
2024-12-19 10:43 ` [pve-devel] [PATCH container v3 13/13] migration: add reminder to evaluate dropping seemingly useless check for PVE 9 Fiona Ebner
2024-12-19 11:45 ` [pve-devel] partially-applied: [PATCH storage/container v3 00/13] import/export for shared storages Fabian Grünbichler
13 siblings, 0 replies; 17+ messages in thread
From: Fiona Ebner @ 2024-12-19 10:43 UTC (permalink / raw)
To: pve-devel
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
src/PVE/LXC/Migrate.pm | 1 +
1 file changed, 1 insertion(+)
diff --git a/src/PVE/LXC/Migrate.pm b/src/PVE/LXC/Migrate.pm
index 0debb9e..e1e6cab 100644
--- a/src/PVE/LXC/Migrate.pm
+++ b/src/PVE/LXC/Migrate.pm
@@ -303,6 +303,7 @@ sub phase1 {
if ($remote) {
push @$migratable_storages, 'cifs';
push @$migratable_storages, 'nfs';
+ push @$migratable_storages, 'rbd';
}
die "storage type '$scfg->{type}' not supported\n"
--
2.39.5
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 17+ messages in thread* [pve-devel] [PATCH container v3 13/13] migration: add reminder to evaluate dropping seemingly useless check for PVE 9
2024-12-19 10:43 [pve-devel] [PATCH storage/container v3 00/13] import/export for shared storages Fiona Ebner
` (11 preceding siblings ...)
2024-12-19 10:43 ` [pve-devel] [PATCH container v3 12/13] migration: allow rbd storages for remote migration Fiona Ebner
@ 2024-12-19 10:43 ` Fiona Ebner
2024-12-19 11:45 ` [pve-devel] partially-applied: [PATCH storage/container v3 00/13] import/export for shared storages Fabian Grünbichler
13 siblings, 0 replies; 17+ messages in thread
From: Fiona Ebner @ 2024-12-19 10:43 UTC (permalink / raw)
To: pve-devel
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
src/PVE/LXC/Migrate.pm | 2 ++
1 file changed, 2 insertions(+)
diff --git a/src/PVE/LXC/Migrate.pm b/src/PVE/LXC/Migrate.pm
index e1e6cab..a550d70 100644
--- a/src/PVE/LXC/Migrate.pm
+++ b/src/PVE/LXC/Migrate.pm
@@ -292,6 +292,8 @@ sub phase1 {
my ($sid, $volname) = PVE::Storage::parse_volume_id($volid);
my $scfg = PVE::Storage::storage_config($self->{storecfg}, $sid);
+ # FIXME PVE 9 - why is this even here, can't it just be dropped completely? The storage
+ # layer already dies if there is no valid transport format.
# TODO move to storage plugin layer?
my $migratable_storages = [
'dir',
--
2.39.5
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 17+ messages in thread* [pve-devel] partially-applied: [PATCH storage/container v3 00/13] import/export for shared storages
2024-12-19 10:43 [pve-devel] [PATCH storage/container v3 00/13] import/export for shared storages Fiona Ebner
` (12 preceding siblings ...)
2024-12-19 10:43 ` [pve-devel] [PATCH container v3 13/13] migration: add reminder to evaluate dropping seemingly useless check for PVE 9 Fiona Ebner
@ 2024-12-19 11:45 ` Fabian Grünbichler
2024-12-19 12:17 ` Fiona Ebner
13 siblings, 1 reply; 17+ messages in thread
From: Fabian Grünbichler @ 2024-12-19 11:45 UTC (permalink / raw)
To: Proxmox VE development discussion
Folded in Filip's T-B from v2, and applied all but the last
pve-container patch - those checks are there to cancel the migration
early on if a storage that is not supported is contained in the mix..
ideally they should move to the storage layer, as the original comment
indicates.. basically a helper that checks export and import formats
(and snapshots/base status) and returns a bool?
On December 19, 2024 11:43 am, Fiona Ebner wrote:
> Chnages in v3:
> * add fix for path() calls in Plugin.pm
> * use rbd export/import instead of mapping+dd
> * list images for volume exists helper
> * fix commit message for patch adding Common module
> * add patch dropping outdated error message check
>
> Changes in v2:
> * add fix for path() in iSCSI direct plugin
> * add export for iSCSI plugins
> * add RFC for improving RBD volume exists helper
>
> For remote migration, export/import functionality is also useful for
> storages that are shared within a single cluster. While file-based
> network storages already can rely on the default file-based
> implementation in Plugin.pm, RBD and iSCSI plugins did not have the
> functionality yet.
>
>
> Regarding RBD:
>
> For now, only 'raw+size' is supported and it's not possible to
> export/import with snapshots. The volume or snapshot is mapped using
> rbd and then the data is read via 'dd'.
>
> Introducing an 'rbd' transport format might be feasible for more
> complete (i.e. with snapshots, incremental) transfer between two RBD
> storages.
>
>
> For iSCSI plugins, remote migration will still fail further up the
> stack though:
>
>> can't migrate local disk 'iscsidirect:lun0': owned by other VM (owner = VM )
>> can't migrate local disk 'iscsi:0.0.0.scsi-360014055b29367fb79a46b0bdb179fae': owned by other VM (owner = VM )
>
> pve-storage:
>
> Fiona Ebner (10):
> plugin: export/import: fix calls to path() method
> rbd plugin: schema: document default value for 'krbd' setting
> export: redirect stdout to avoid any unrelated messages ending up in
> the export stream
> rbd plugin: factor out helper to check if volume already exists
> rbd plugin: implement volume import/export
> rbd plugin: improve volume exists helper
> iscsi plugin: support volume export
> iscsi direct plugin: support volume export
> plugins: volume import: align size up to 1KiB
> rbd plugin: list: drop outdated error message check
>
> Max Carrara (1):
> common: introduce common module
>
> src/PVE/CLI/pvesm.pm | 5 +-
> src/PVE/Storage/Common.pm | 54 ++++++++++
> src/PVE/Storage/Common/Makefile | 6 ++
> src/PVE/Storage/ISCSIDirectPlugin.pm | 63 ++++++++++++
> src/PVE/Storage/ISCSIPlugin.pm | 48 +++++++++
> src/PVE/Storage/LVMPlugin.pm | 4 +-
> src/PVE/Storage/Makefile | 2 +
> src/PVE/Storage/Plugin.pm | 12 ++-
> src/PVE/Storage/RBDPlugin.pm | 142 ++++++++++++++++++++++++---
> 9 files changed, 317 insertions(+), 19 deletions(-)
> create mode 100644 src/PVE/Storage/Common.pm
> create mode 100644 src/PVE/Storage/Common/Makefile
>
>
> pve-container:
>
> Fiona Ebner (2):
> migration: allow rbd storages for remote migration
> migration: add reminder to evaluate dropping seemingly useless check
> for PVE 9
>
> src/PVE/LXC/Migrate.pm | 3 +++
> 1 file changed, 3 insertions(+)
>
>
> Summary over all repositories:
> 10 files changed, 320 insertions(+), 19 deletions(-)
>
> --
> Generated by git-murpp 0.5.0
>
>
> _______________________________________________
> 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
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [pve-devel] partially-applied: [PATCH storage/container v3 00/13] import/export for shared storages
2024-12-19 11:45 ` [pve-devel] partially-applied: [PATCH storage/container v3 00/13] import/export for shared storages Fabian Grünbichler
@ 2024-12-19 12:17 ` Fiona Ebner
0 siblings, 0 replies; 17+ messages in thread
From: Fiona Ebner @ 2024-12-19 12:17 UTC (permalink / raw)
To: Proxmox VE development discussion, Fabian Grünbichler
Am 19.12.24 um 12:45 schrieb Fabian Grünbichler:
> Folded in Filip's T-B from v2, and applied all but the last
> pve-container patch - those checks are there to cancel the migration
> early on if a storage that is not supported is contained in the mix..
>
> ideally they should move to the storage layer, as the original comment
> indicates.. basically a helper that checks export and import formats
> (and snapshots/base status) and returns a bool?
Okay, I see. Yes, sounds nice to have for early checks. QemuMigrate can
benefit from that too. I guess splitting out the beginning of
storage_migrate() with volume_transfer_formats() should be enough?
We do need to make sure that plugins correctly treat "subvol" before
claiming they can transfer a volume of course, but at least the ones
already in the list should already do that (for those it would be a
pre-existing bug if not).
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 17+ messages in thread