[pve-devel] [PATCH-SERIES storage 0/6] basic RBD import/export

[pve-devel] [PATCH-SERIES storage 0/6] basic RBD import/export

[Fiona Ebner]

For now, only 'raw+size' is supported and it's not possible to
export/import with snapshots. The volume or snapshot is mapped using
krbd 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.

Fiona Ebner (5):
  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
  plugins: volume import: align size up to 1KiB

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/LVMPlugin.pm    |   4 +-
 src/PVE/Storage/Makefile        |   2 +
 src/PVE/Storage/Plugin.pm       |   4 +-
 src/PVE/Storage/RBDPlugin.pm    | 135 ++++++++++++++++++++++++++++++--
 7 files changed, 202 insertions(+), 8 deletions(-)
 create mode 100644 src/PVE/Storage/Common.pm
 create mode 100644 src/PVE/Storage/Common/Makefile

-- 
2.39.5



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

[pve-devel] [PATCH storage 1/6] rbd plugin: schema: document default value for 'krbd' setting

[Fiona Ebner]

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

[pve-devel] [PATCH storage 2/6] export: redirect stdout to avoid any unrelated messages ending up in the export stream

[Fiona Ebner]

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

[pve-devel] [PATCH storage 3/6] rbd plugin: factor out helper to check if volume already exists

[Fiona Ebner]

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

NOTE: the helper seems a bit brittle and does not distinguish between
the volume not existing and another error. While allocation or
renaming later still fail with an error, improving the helper seems
worthwhile

 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

[pve-devel] [PATCH storage 4/6] rbd plugin: implement volume import/export

[Fiona Ebner]

For now, only 'raw+size' is supported and it's not possible to
export/import with snapshots. The volume or snapshot is mapped using
krbd 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.

The code is mostly copied from the LVM plugin.

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
 src/PVE/Storage/RBDPlugin.pm | 115 +++++++++++++++++++++++++++++++++++
 1 file changed, 115 insertions(+)

diff --git a/src/PVE/Storage/RBDPlugin.pm b/src/PVE/Storage/RBDPlugin.pm
index 301918c..0e53888 100644
--- a/src/PVE/Storage/RBDPlugin.pm
+++ b/src/PVE/Storage/RBDPlugin.pm
@@ -864,6 +864,121 @@ 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 $file = $class->map_volume($storeid, $scfg, $volname, $snapshot);
+    eval {
+
+	my $size;
+	# should be faster than querying RBD, also checks for the device file's availability
+	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));
+    };
+    my $err = $@;
+
+    eval { $class->unmap_volume($storeid, $scfg, $volname); };
+    warn $@ if $@;
+
+    die $err if $err;
+
+    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";
+	$name = undef;
+    }
+
+    my ($size) = PVE::Storage::Plugin::read_common_header($fh);
+    $size = int($size/1024);
+
+    my $mapped;
+    eval {
+	my $allocname = $class->alloc_image($storeid, $scfg, $vmid, 'raw', $name, $size);
+	my $oldname = $volname;
+	$volname = $allocname;
+	if (defined($name) && $allocname ne $oldname) {
+	    die "internal error: unexpected allocated name: '$allocname' != '$oldname'\n";
+	}
+
+	my $file = $class->map_volume($storeid, $scfg, $volname, undef);
+	$mapped = 1;
+
+	run_command(['dd', "of=$file", 'bs=64k'], input => '<&'.fileno($fh));
+    };
+    my $err = $@;
+
+    if ($mapped) {
+	eval { $class->unmap_volume($storeid, $scfg, $volname); };
+	warn $@ if $@;
+    }
+
+    if ($err) {
+	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

Re: [pve-devel] [PATCH storage 4/6] rbd plugin: implement volume import/export

[Fiona Ebner]

Am 13.12.24 um 17:30 schrieb Fiona Ebner:
> +	run_command(['dd', "if=$file", "bs=64k", "status=progress"], output => '>&'.fileno($fh));

---snip---

> +	run_command(['dd', "of=$file", 'bs=64k'], input => '<&'.fileno($fh));


Forgot to mention, I did not test with different block sizes yet. This
is just copied from the LVM plugin. Will do that next week and see if
it's worth changing.


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

Re: [pve-devel] [PATCH storage 4/6] rbd plugin: implement volume import/export

[Fiona Ebner]

Am 13.12.24 um 17:34 schrieb Fiona Ebner:
> Am 13.12.24 um 17:30 schrieb Fiona Ebner:
>> +	run_command(['dd', "if=$file", "bs=64k", "status=progress"], output => '>&'.fileno($fh));
> 
> ---snip---
> 
>> +	run_command(['dd', "of=$file", 'bs=64k'], input => '<&'.fileno($fh));
> 
> 
> Forgot to mention, I did not test with different block sizes yet. This
> is just copied from the LVM plugin. Will do that next week and see if
> it's worth changing.

Tested between a virtual cluster with integrated Ceph (A) and another
node with external Ceph (B) with a VM with a 4 GiB OS disk and 15 GiB
disk full of random data. Don't see much difference, with 64K slightly
winning:

A -> B

16 KiB

2024-12-17 13:00:49 migration finished successfully (duration 00:02:01)
qm remote-migrate 1234 1234  --target-storage rbd --target-bridge vmbr0
12.67s user 42.01s system 45% cpu 2:01.29 total

2024-12-17 13:05:40 migration finished successfully (duration 00:02:01)
qm remote-migrate 1234 1234  --target-storage rbd --target-bridge vmbr0
12.49s user 42.34s system 45% cpu 2:00.89 total

64 KiB

2024-12-17 11:59:35 migration finished successfully (duration 00:01:56)
qm remote-migrate 1234 1234  --target-storage rbd --target-bridge vmbr0
12.31s user 41.10s system 45% cpu 1:57.31 total

2024-12-17 12:05:39 migration finished successfully (duration 00:01:58)
qm remote-migrate 1234 1234  --target-storage rbd --target-bridge vmbr0
12.07s user 41.30s system 45% cpu 1:58.00 total

256 KiB

2024-12-17 12:11:00 migration finished successfully (duration 00:02:05)
qm remote-migrate 1234 1234  --target-storage rbd --target-bridge vmbr0
11.29s user 37.89s system 39% cpu 2:06.03 total

2024-12-17 12:16:54 migration finished successfully (duration 00:02:07)
qm remote-migrate 1234 1234  --target-storage rbd --target-bridge vmbr0
11.64s user 38.70s system 39% cpu 2:07.61 total


B -> A

16 KiB

2024-12-17 13:02:47 migration finished successfully (duration 00:01:28)
qm remote-migrate 1234 1234  --target-storage rbd --target-bridge vnet0
13.06s user 40.22s system 58% cpu 1:31.23 total

2024-12-17 13:07:57 migration finished successfully (duration 00:01:28)
qm remote-migrate 1234 1234  --target-storage rbd --target-bridge vnet0
13.00s user 40.25s system 58% cpu 1:30.64 total

64 KiB

2024-12-17 11:56:34 migration finished successfully (duration 00:01:22)
qm remote-migrate 1234 1234  --target-storage rbd --target-bridge vnet0
12.72s user 37.16s system 59% cpu 1:24.53 total

2024-12-17 12:03:15 migration finished successfully (duration 00:01:27)
qm remote-migrate 1234 1234  --target-storage rbd --target-bridge vnet0
13.19s user 38.45s system 57% cpu 1:30.10 total

256 KiB

2024-12-17 12:08:10 migration finished successfully (duration 00:01:31)
qm remote-migrate 1234 1234  --target-storage rbd --target-bridge vnet0
12.18s user 37.23s system 52% cpu 1:34.39 total

2024-12-17 12:14:05 migration finished successfully (duration 00:01:35)
qm remote-migrate 1234 1234  --target-storage rbd --target-bridge vnet0
11.85s user 36.74s system 49% cpu 1:37.46 total



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

[pve-devel] [PATCH storage 5/6] common: introduce common module

[Fiona Ebner]

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 also contains the `get_deprecation_warning` subroutine that makes
it easier to warn developers and/or plugin authors that a subroutine
will be removed in the future.

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>
---
 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

[pve-devel] [PATCH storage 6/6] plugins: volume import: align size up to 1KiB

[Fiona Ebner]

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 5deff76..94b3646 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 0e53888..8ce77eb 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 {
@@ -947,7 +949,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;
 
     my $mapped;
     eval {
-- 
2.39.5



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