[PATCH manager v2 0/2] ceph: pool: create EC pools from an existing profile

[PATCH manager v2 0/2] ceph: pool: create EC pools from an existing profile

[Kefu Chai]

Hi,

the docs suggest building an erasure code profile with the Ceph tools
and passing it to 'pveceph pool create' as 'profile', but the schema
still requires 'k' and 'm':

  # pveceph pool create test-ec --erasure-coding profile=default
  erasure-coding.m: property is missing and it is not optional
  erasure-coding.k: property is missing and it is not optional

Patch 2 makes them optional with a profile, and refuses values that
don't match it instead of ignoring them. Patch 1 has to go in first:
destroypool removes the pool's profile by default, which would take
'default' with it. Now it only removes the profile createpool made for
that pool, unless asked to.

Thanks to Maximiliano for reviewing and testing v1 [1]. Changes in v2:

patch 1:
- reword the remove_ecprofile description as suggested
- check the profile in one line with length(my $ecprofile = ...). The
  suggested form doesn't compile under strict, since && binds tighter
  than = and the "my" isn't visible yet in its own statement.
- a second destroypool with remove_ecprofile=1 fails once the pool is
  gone, so the message for a kept profile now points to 'ceph osd
  erasure-code-profile rm <profile>' instead
- use v5.36 in the new test file

patch 2:
- describe 'k', 'm', 'failure-domain' and 'device-class' with "If
  'profile' is set, it must match the ... of the profile"
- rename $actual to $actual_profile, and the loop variables to $key,
  $value and $profile_value

Both are rebased onto current master. I kept Maximiliano's Reviewed-by
and Tested-by, since v2 only addresses his nits.

I reran the tests and the reproducer from the review on a three node
cluster: the kept profile can be removed with the command the message
names, mismatching values are still refused before any pool is
created, and pveceph's own profile is still removed by default.

[1] https://lore.proxmox.com/pve-devel/20261021054425.3271742-1-k.chai@proxmox.com/

Kefu Chai (2):
  api: ceph: pool: destroypool: keep EC profiles pveceph did not create
  api: ceph: pool: createpool: make 'k' and 'm' optional with a profile

 PVE/API2/Ceph/Pool.pm |  77 +++++++++---
 PVE/Ceph/Tools.pm     |  25 ++--
 test/CephPool_test.pl | 274 ++++++++++++++++++++++++++++++++++++++++++
 test/Makefile         |   6 +-
 4 files changed, 356 insertions(+), 26 deletions(-)
 create mode 100755 test/CephPool_test.pl

-- 
2.47.3

[PATCH manager v2 1/2] api: ceph: pool: destroypool: keep EC profiles pveceph did not create

[Kefu Chai]

destroypool removes the erasure code profile of the pool it destroys
unless remove_ecprofile=0 is passed. When createpool made that profile
itself, as pve_ec_<name> for the pool <name>-data, this cleans up after
it. But a pool can also use an existing profile through the 'profile'
option, such as 'default' or one built with the Ceph tools, and then
destroying the pool removes that profile too once no other pool uses
it.

So by default only remove the profile createpool made for the pool
being destroyed. An explicit remove_ecprofile=1 still removes any
profile. When the profile is kept, say so and how to remove it later
with the Ceph tools, since running destroypool again with
remove_ecprofile=1 fails once the pool is gone.

While at it, test for a profile with length() rather than truthiness,
so a profile named '0' still gets its CRUSH rule and profile cleaned
up.

Reviewed-by: Maximiliano Sandoval <m.sandoval@proxmox.com>
Tested-by: Maximiliano Sandoval <m.sandoval@proxmox.com>
Signed-off-by: Kefu Chai <k.chai@proxmox.com>
---
 PVE/API2/Ceph/Pool.pm | 17 +++++---
 PVE/Ceph/Tools.pm     |  6 +++
 test/CephPool_test.pl | 96 +++++++++++++++++++++++++++++++++++++++++++
 test/Makefile         |  6 ++-
 4 files changed, 119 insertions(+), 6 deletions(-)
 create mode 100755 test/CephPool_test.pl

diff --git a/PVE/API2/Ceph/Pool.pm b/PVE/API2/Ceph/Pool.pm
index 8fd11b82..dd505872 100644
--- a/PVE/API2/Ceph/Pool.pm
+++ b/PVE/API2/Ceph/Pool.pm
@@ -574,11 +574,10 @@ __PACKAGE__->register_method({
                 default => 0,
             },
             remove_ecprofile => {
-                description =>
-                    "Remove the erasure code profile. Defaults to true, if applicable.",
+                description => "Remove the erasure code profile. Enabled by default if"
+                    . " 'pveceph pool create' made the profile for this pool, disabled otherwise.",
                 type => 'boolean',
                 optional => 1,
-                default => 1,
             },
         },
     },
@@ -618,16 +617,24 @@ __PACKAGE__->register_method({
 
             PVE::Ceph::Tools::destroy_pool($pool, $rados);
 
-            if (my $ecprofile = $pool_properties->{erasure_code_profile}) {
+            # a replicated pool has no profile, or an empty one
+            if (length(my $ecprofile = $pool_properties->{erasure_code_profile})) {
                 print "found erasure coded profile '$ecprofile', destroying its CRUSH rule\n";
                 my $crush_rule = $pool_properties->{crush_rule};
                 eval { PVE::Ceph::Tools::destroy_crush_rule($crush_rule, $rados); };
                 warn "removing crush rule '${crush_rule}' failed: $@\n" if $@;
 
-                if ($param->{remove_ecprofile} // 1) {
+                my $remove = $param->{remove_ecprofile}
+                    // PVE::Ceph::Tools::is_own_ecprofile($ecprofile, $pool);
+                if ($remove) {
                     print "destroying erasure coded profile '$ecprofile'\n";
                     eval { PVE::Ceph::Tools::destroy_ecprofile($ecprofile, $rados) };
                     warn "removing EC profile '${ecprofile}' failed: $@\n" if $@;
+                } elsif (!defined($param->{remove_ecprofile})) {
+                    print
+                        "keeping erasure coded profile '$ecprofile', pveceph did not create it"
+                        . " for this pool. To remove it once no pool uses it, run"
+                        . " 'ceph osd erasure-code-profile rm $ecprofile'.\n";
                 }
             }
 
diff --git a/PVE/Ceph/Tools.pm b/PVE/Ceph/Tools.pm
index 1ca0722d..cfd0c296 100644
--- a/PVE/Ceph/Tools.pm
+++ b/PVE/Ceph/Tools.pm
@@ -691,6 +691,12 @@ sub get_ecprofile_name {
     return "pve_ec_${name}";
 }
 
+# whether createpool made $profile for the data pool $pool, which it names <name>-data
+sub is_own_ecprofile {
+    my ($profile, $pool) = @_;
+    return $pool =~ m/^(.+)-data$/ && $profile eq get_ecprofile_name($1);
+}
+
 sub destroy_crush_rule {
     my ($rule, $rados) = @_;
     $rados = PVE::RADOS->new() if !$rados;
diff --git a/test/CephPool_test.pl b/test/CephPool_test.pl
new file mode 100755
index 00000000..9077aacf
--- /dev/null
+++ b/test/CephPool_test.pl
@@ -0,0 +1,96 @@
+#!/usr/bin/perl
+
+use v5.36;
+
+use lib ('.', '..');
+
+use Test::More;
+use PVE::API2::Ceph::Pool;
+use PVE::Ceph::Tools;
+
+{
+
+    package PoolRPCEnvironment;
+
+    sub get_user { return 'root@pam'; }
+
+    sub check { }
+
+    sub fork_worker {
+        my ($self, $type, $id, $user, $worker) = @_;
+        $worker->('test-upid');
+        return 'test-upid';
+    }
+}
+
+# call an API method, run its worker inline, and return what it printed
+my $run = sub {
+    my ($method, $param) = @_;
+    my $out = '';
+    open(my $stdout, '>', \$out) or die $!;
+    local *STDOUT = $stdout;
+    PVE::API2::Ceph::Pool->map_method_by_name($method)->{code}->($param);
+    return $out;
+};
+
+# --- the profile a pool owns --------------------------------------------------------------
+my @own_cases = (
+    # profile, pool, owned, name
+    ['pve_ec_foo', 'foo-data', 1, 'the profile made for the pool is its own'],
+    ['default', 'foo-data', 0, "'default' is not"],
+    ['pve_ec_bar', 'foo-data', 0, "another pool's profile is not"],
+    ['pve_ec_foo', 'foo', 0, "a pool without '-data' has no own profile"],
+    ['pve_ec_x-data', 'x-data-data', 1, "only the last '-data' is stripped"],
+);
+for my $case (@own_cases) {
+    my ($profile, $pool, $owned, $name) = @$case;
+    is(PVE::Ceph::Tools::is_own_ecprofile($profile, $pool) ? 1 : 0, $owned, $name);
+}
+
+# --- what destroypool removes -------------------------------------------------------------
+{
+    no warnings qw(redefine once);
+    my ($props, @removed);
+
+    local *PVE::RPCEnvironment::get = sub { bless {}, 'PoolRPCEnvironment' };
+    local *PVE::Ceph::Tools::check_ceph_inited = sub { };
+    local *PVE::Storage::config = sub { { ids => {} } };
+    local *PVE::RADOS::new = sub { 'rados' };
+    local *PVE::Ceph::Tools::get_pool_properties = sub { $props };
+    local *PVE::Ceph::Tools::destroy_pool = sub { };
+    local *PVE::Ceph::Tools::destroy_crush_rule = sub { push @removed, "rule $_[0]" };
+    local *PVE::Ceph::Tools::destroy_ecprofile = sub { push @removed, "profile $_[0]" };
+
+    my @cases = (
+        # profile, extra parameters, what gets removed, whether it says it keeps one, name
+        ['pve_ec_foo', {}, 'rule r, profile pve_ec_foo', 0, 'its own profile is removed'],
+        ['default', {}, 'rule r', 1, 'a profile the user made is kept, and the log says so'],
+        [
+            'default',
+            { remove_ecprofile => 1 },
+            'rule r, profile default',
+            0,
+            'remove_ecprofile=1 removes any profile',
+        ],
+        [
+            'pve_ec_foo',
+            { remove_ecprofile => 0 },
+            'rule r',
+            0,
+            'remove_ecprofile=0 keeps even its own profile',
+        ],
+        ['0', { remove_ecprofile => 1 }, 'rule r, profile 0', 0, "a profile named '0' counts"],
+        [undef, {}, '', 0, 'a replicated pool has no profile to remove'],
+        ['', {}, '', 0, 'nor has one that reports an empty profile'],
+    );
+    for my $case (@cases) {
+        my ($profile, $extra, $expected, $keeps, $name) = @$case;
+        $props = { crush_rule => 'r', erasure_code_profile => $profile };
+        @removed = ();
+        my $out = $run->('destroypool', { name => 'foo-data', %$extra });
+        is(join(', ', @removed), $expected, $name);
+        is($out =~ /keeping erasure coded profile/ ? 1 : 0, $keeps, "$name (log)");
+    }
+}
+
+done_testing();
diff --git a/test/Makefile b/test/Makefile
index 2dfd20a4..b8a3edde 100644
--- a/test/Makefile
+++ b/test/Makefile
@@ -5,7 +5,7 @@ all:
 export PERLLIB=..
 
 .PHONY: check
-check: test-replication test-balloon test-vzdump test-osd test-ceph-health test-ceph-services test-ceph-auth test-ceph-key-migration test-ceph-lockbox-migration test-custom-cpu-models test-pvesh
+check: test-replication test-balloon test-vzdump test-osd test-ceph-health test-ceph-pool test-ceph-services test-ceph-auth test-ceph-key-migration test-ceph-lockbox-migration test-custom-cpu-models test-pvesh
 
 .PHONY: test-balloon
 test-balloon:
@@ -36,6 +36,10 @@ test-osd:
 test-ceph-health:
 	./CephHealth_test.pl
 
+.PHONY: test-ceph-pool
+test-ceph-pool:
+	./CephPool_test.pl
+
 .PHONY: test-ceph-services
 test-ceph-services:
 	./CephServices_test.pl
-- 
2.47.3

[PATCH manager v2 2/2] api: ceph: pool: createpool: make 'k' and 'm' optional with a profile

[Kefu Chai]

The documentation tells the user to build a custom EC profile with the
Ceph tools and then pass it as 'profile'. That call is rejected:

  # pveceph pool create test-ec --erasure-coding profile=default
  400 Parameter verification failed.
  erasure-coding: invalid format - format error
  erasure-coding.m: property is missing and it is not optional
  erasure-coding.k: property is missing and it is not optional

'k' and 'm' only feed create_ecprofile(), which createpool skips for a
pool that gets an existing profile. So mark them optional and require
them only when no profile is given. The schema cannot say that, so the
descriptions do.

Until now, 'k', 'm', 'failure-domain' and 'device-class' given next to
a profile were dropped without a word, and the pool got whatever the
profile defines. Reject values that differ from the profile instead,
so a mistake shows up before any pool is created. Values that match
are still accepted. A single 'osd erasure-code-profile get' reads the
profile and also tells whether it exists, so it replaces
ecprofile_exists(), which nothing else uses.

This changes the API for one kind of caller: since the old schema
required 'k' and 'm', a script may pass placeholder values next to a
profile. If they differ from the profile, the call now fails, and the
script should drop them. The GUI does not set 'profile'.

Also check whether a profile was given with defined(), here and in
create_pool(), so a profile named '0' is not taken as missing.

Reviewed-by: Maximiliano Sandoval <m.sandoval@proxmox.com>
Tested-by: Maximiliano Sandoval <m.sandoval@proxmox.com>
Signed-off-by: Kefu Chai <k.chai@proxmox.com>
---
 PVE/API2/Ceph/Pool.pm |  60 +++++++++++---
 PVE/Ceph/Tools.pm     |  19 +++--
 test/CephPool_test.pl | 178 ++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 237 insertions(+), 20 deletions(-)

diff --git a/PVE/API2/Ceph/Pool.pm b/PVE/API2/Ceph/Pool.pm
index dd505872..cb461497 100644
--- a/PVE/API2/Ceph/Pool.pm
+++ b/PVE/API2/Ceph/Pool.pm
@@ -379,33 +379,40 @@ my $ec_format = {
     k => {
         type => 'integer',
         description => "Number of data chunks. Will create an erasure coded pool plus a"
-            . " replicated pool for metadata.",
+            . " replicated pool for metadata. Required if 'profile' is not set. If 'profile'"
+            . " is set, it must match the 'k' of the profile.",
         minimum => 2,
+        optional => 1,
     },
     m => {
         type => 'integer',
         description => "Number of coding chunks. Will create an erasure coded pool plus a"
-            . " replicated pool for metadata.",
+            . " replicated pool for metadata. Required if 'profile' is not set. If 'profile'"
+            . " is set, it must match the 'm' of the profile.",
         minimum => 1,
+        optional => 1,
     },
     'failure-domain' => {
         type => 'string',
-        description => "CRUSH failure domain. Default is 'host'. Will create an erasure"
-            . " coded pool plus a replicated pool for metadata.",
+        description => "CRUSH failure domain. Default is 'host' if 'profile' is not set."
+            . " Will create an erasure coded pool plus a replicated pool for metadata. If"
+            . " 'profile' is set, it must match the failure domain of the profile.",
         format_description => 'domain',
         optional => 1,
-        default => 'host',
     },
     'device-class' => {
         type => 'string',
         description => "CRUSH device class. Will create an erasure coded pool plus a"
-            . " replicated pool for metadata.",
+            . " replicated pool for metadata. If 'profile' is set, it must match the device"
+            . " class of the profile.",
         format_description => 'class',
         optional => 1,
     },
     profile => {
-        description => "Override the erasure code (EC) profile to use. Will create an"
-            . " erasure coded pool plus a replicated pool for metadata.",
+        description => "Use this existing erasure code (EC) profile instead of creating one"
+            . " from 'k', 'm', 'failure-domain' and 'device-class'. Any of those given too"
+            . " must match the profile. Will create an erasure coded pool plus a replicated"
+            . " pool for metadata.",
         type => 'string',
         format_description => 'profile',
         optional => 1,
@@ -418,8 +425,34 @@ sub ec_parse_and_check {
 
     my $ec = parse_property_string($ec_format, $property);
 
-    die "Erasure code profile '$ec->{profile}' does not exist.\n"
-        if $ec->{profile} && !PVE::Ceph::Tools::ecprofile_exists($ec->{profile}, $rados);
+    my $name = $ec->{profile};
+    if (!defined($name)) {
+        die "'k' and 'm' are required if no 'profile' is given.\n"
+            if !defined($ec->{k}) || !defined($ec->{m});
+        return $ec;
+    }
+
+    my $profile = PVE::Ceph::Tools::get_ecprofile($name, $rados)
+        // die "Erasure code profile '$name' does not exist.\n";
+
+    # the profile decides these, so refuse values it does not have
+    # Ceph keeps integers as typed, e.g. '02' or '+2'
+    my $int = sub { my ($v) = @_; return PVE::JSONSchema::is_integer($v) ? $v + 0 : $v };
+    my $actual_profile = {
+        k => $int->($profile->{k} // ''),
+        m => $int->($profile->{m} // ''),
+        'failure-domain' => $profile->{'crush-failure-domain'} || 'host',
+        'device-class' => $profile->{'crush-device-class'} // '',
+    };
+    for my $key (sort keys %$actual_profile) {
+        my $value = $ec->{$key};
+        next if !defined($value);
+        $value += 0 if $key eq 'k' || $key eq 'm'; # the schema checked it is an integer
+        my $profile_value = $actual_profile->{$key};
+        next if $value eq $profile_value;
+        my $has = $profile_value ne '' ? "'$profile_value'" : 'none';
+        die "'$key' is '$ec->{$key}', but erasure code profile '$name' has $has.\n";
+    }
 
     return $ec;
 }
@@ -448,8 +481,9 @@ __PACKAGE__->register_method({
             },
             'erasure-coding' => {
                 description => "Create an erasure coded pool for RBD with an accompaning"
-                    . " replicated pool for metadata storage. With EC, the common ceph options 'size',"
-                    . " 'min_size' and 'crush_rule' parameters will be applied to the metadata pool.",
+                    . " replicated pool for metadata storage. Needs either 'profile' or both 'k'"
+                    . " and 'm'. With EC, the common ceph options 'size', 'min_size' and"
+                    . " 'crush_rule' parameters will be applied to the metadata pool.",
                 type => 'string',
                 format => $ec_format,
                 optional => 1,
@@ -499,7 +533,7 @@ __PACKAGE__->register_method({
                 PVE::RADOS->new(timeout => PVE::Ceph::Tools::get_config('long_rados_timeout'));
 
             if ($ec) {
-                if (!$ec->{profile}) {
+                if (!defined($ec->{profile})) {
                     $ec->{profile} = PVE::Ceph::Tools::get_ecprofile_name($pool, $rados);
                     eval {
                         PVE::Ceph::Tools::create_ecprofile(
diff --git a/PVE/Ceph/Tools.pm b/PVE/Ceph/Tools.pm
index cfd0c296..804ae262 100644
--- a/PVE/Ceph/Tools.pm
+++ b/PVE/Ceph/Tools.pm
@@ -8,6 +8,7 @@ use File::Basename;
 use IO::File;
 use JSON;
 use List::Util qw(any);
+use POSIX qw(ENOENT);
 
 use PVE::Tools qw(run_command dir_glob_foreach extract_param);
 use PVE::Cluster qw(cfs_read_file);
@@ -329,7 +330,8 @@ sub get_pool_properties {
 sub get_pool_type {
     my ($pool, $rados) = @_;
     $rados = PVE::RADOS->new() if !defined($rados);
-    return 'erasure' if get_pool_properties($pool, $rados)->{erasure_code_profile};
+    # a profile may be named '0'
+    return 'erasure' if length(get_pool_properties($pool, $rados)->{erasure_code_profile});
     return 'replicated';
 }
 
@@ -346,8 +348,9 @@ sub create_pool {
         format => 'plain',
     };
     $mon_params->{pool_type} = extract_param($param, 'pool_type') if $param->{pool_type};
+    # a profile may be named '0'
     $mon_params->{erasure_code_profile} = extract_param($param, 'erasure_code_profile')
-        if $param->{erasure_code_profile};
+        if defined($param->{erasure_code_profile});
 
     # Ceph checks mon_max_pg_per_osd against the rule given on create, not on a later rule change
     $mon_params->{rule} = extract_param($param, 'crush_rule')
@@ -645,14 +648,16 @@ sub ceph_cluster_status {
     return $status;
 }
 
-sub ecprofile_exists {
+# returns undef if there is no such profile
+sub get_ecprofile {
     my ($name, $rados) = @_;
     $rados = PVE::RADOS->new() if !$rados;
 
-    my $res = $rados->mon_command({ prefix => 'osd erasure-code-profile ls' });
-
-    my $profiles = { map { $_ => 1 } @$res };
-    return $profiles->{$name};
+    my $cmd = { prefix => 'osd erasure-code-profile get', name => $name };
+    my $res = $rados->mon_cmd($cmd, 1);
+    return if $res->{return_code} == -ENOENT;
+    die "error with '$cmd->{prefix}': $res->{status_message}\n" if $res->{return_code} < 0;
+    return $res->{data};
 }
 
 sub create_ecprofile {
diff --git a/test/CephPool_test.pl b/test/CephPool_test.pl
index 9077aacf..32752c5b 100755
--- a/test/CephPool_test.pl
+++ b/test/CephPool_test.pl
@@ -23,6 +23,40 @@ use PVE::Ceph::Tools;
     }
 }
 
+{
+
+    package FakeRados;
+
+    use POSIX qw(ENOENT);
+
+    # a profile given as a number makes 'get' fail with that error code
+    sub new {
+        my ($class, %profiles) = @_;
+        return bless { profiles => \%profiles, cmds => [] }, $class;
+    }
+
+    sub mon_cmd {
+        my ($self, $cmd, $no_result_catch) = @_;
+        die "unexpected command '$cmd->{prefix}'\n"
+            if $cmd->{prefix} ne 'osd erasure-code-profile get';
+        my $reply = $self->{profiles}->{ $cmd->{name} } // -ENOENT;
+        my $res =
+            ref($reply)
+            ? { return_code => 0, data => $reply }
+            : { return_code => $reply, status_message => "error $reply" };
+        # like PVE::RADOS::mon_cmd
+        die "error with '$cmd->{prefix}': mon_cmd failed - $res->{status_message}\n"
+            if !$no_result_catch && $res->{return_code} < 0;
+        return $res;
+    }
+
+    sub mon_command {
+        my ($self, $cmd) = @_;
+        push $self->{cmds}->@*, $cmd;
+        return;
+    }
+}
+
 # call an API method, run its worker inline, and return what it printed
 my $run = sub {
     my ($method, $param) = @_;
@@ -93,4 +127,148 @@ for my $case (@own_cases) {
     }
 }
 
+# --- what an erasure coding property string accepts ---------------------------------------
+# as Ceph reports them: numbers as strings, unset keys left out
+my $rados = FakeRados->new(
+    default => { k => '2', m => '2', plugin => 'isa', technique => 'reed_sol_van' },
+    ssd => {
+        k => '4',
+        m => '2',
+        'crush-failure-domain' => 'osd',
+        'crush-device-class' => 'ssd',
+    },
+    0 => { k => '3', m => '1' },
+    typed => { k => '02', m => '+1' },
+    broken => -5,
+);
+
+my $parse = sub { PVE::API2::Ceph::Pool::ec_parse_and_check($_[0], $rados) };
+
+is_deeply($parse->('profile=default'), { profile => 'default' }, "a profile needs no 'k' and 'm'");
+is_deeply($parse->('k=4,m=2'), { k => 4, m => 2 }, "'k' and 'm' without a profile");
+is_deeply($parse->('profile=0'), { profile => '0' }, "a profile named '0' counts as given");
+is($parse->(undef), undef, 'no erasure coding at all is fine');
+
+my @parse_cases = (
+    # property, expected error or undef if accepted, name
+    ['profile=ssd', undef, "a profile with another failure domain is accepted on its own"],
+    ['profile=default,k=2,m=2,failure-domain=host', undef, 'values matching the profile'],
+    [
+        'profile=ssd,k=4,m=2,failure-domain=osd,device-class=ssd',
+        undef,
+        'values matching a profile with all keys set',
+    ],
+    ['profile=default,k=02,m=+2', undef, "'k' and 'm' are compared as numbers"],
+    [
+        'profile=typed,k=2,m=1',
+        undef,
+        "'k' and 'm' Ceph keeps as typed are compared as numbers too",
+    ],
+    [
+        'profile=default,k=4,m=2',
+        qr/'k' is '4', but .* 'default' has '2'/,
+        "a 'k' the profile does not have is rejected",
+    ],
+    [
+        'profile=ssd,failure-domain=host',
+        qr/'failure-domain' is 'host', but .* has 'osd'/,
+        'a different failure domain is rejected',
+    ],
+    [
+        'profile=default,device-class=ssd',
+        qr/'device-class' is 'ssd', but .* has none/,
+        'a device class the profile lacks is rejected',
+    ],
+    ['k=4', qr/'k' and 'm' are required/, "'k' without 'm' is rejected"],
+    ['m=2', qr/'k' and 'm' are required/, "'m' without 'k' is rejected"],
+    ['failure-domain=osd', qr/'k' and 'm' are required/, 'a bare failure domain is not enough'],
+    [
+        'profile=does-not-exist',
+        qr/^Erasure code profile .* does not exist/,
+        'an unknown profile',
+    ],
+    [
+        'profile=broken',
+        qr/error with .*: error -5/,
+        'other errors reading the profile are passed on',
+    ],
+);
+for my $case (@parse_cases) {
+    my ($property, $error, $name) = @$case;
+    eval { $parse->($property) };
+    if (defined($error)) {
+        like($@, $error, $name);
+    } else {
+        is($@, '', $name);
+    }
+}
+
+# --- what reaches the monitor -------------------------------------------------------------
+{
+    no warnings 'redefine';
+    local *PVE::Ceph::Tools::set_pool = sub { };
+
+    my $rados = FakeRados->new();
+    my $param = { pool_type => 'erasure', erasure_code_profile => '0' };
+    PVE::Ceph::Tools::create_pool('foo-data', $param, $rados);
+    is($rados->{cmds}->[0]->{erasure_code_profile}, '0', "create_pool passes the profile '0' on");
+    ok(!exists($param->{erasure_code_profile}), 'and leaves it out of what set_pool gets');
+}
+
+{
+    no warnings 'redefine';
+    my $props;
+    local *PVE::Ceph::Tools::get_pool_properties = sub { $props };
+
+    my @cases = (
+        # pool properties, type, name
+        [{ erasure_code_profile => '0' }, 'erasure', "a pool on profile '0' is erasure"],
+        [{}, 'replicated', 'a pool without a profile is not'],
+        [{ erasure_code_profile => '' }, 'replicated', 'nor is one with an empty profile'],
+    );
+    for my $case (@cases) {
+        ($props, my $type, my $name) = @$case;
+        is(PVE::Ceph::Tools::get_pool_type('foo', 'rados'), $type, $name);
+    }
+}
+
+# --- what createpool creates --------------------------------------------------------------
+{
+    no warnings qw(redefine once);
+    my (@profiles, @pools);
+    my $rados = FakeRados->new(0 => { k => '2', m => '1' });
+
+    local *PVE::RPCEnvironment::get = sub { bless {}, 'PoolRPCEnvironment' };
+    local *PVE::Cluster::check_cfs_quorum = sub { };
+    local *PVE::Ceph::Tools::check_ceph_configured = sub { };
+    local *PVE::Ceph::Tools::get_config = sub { 30 };
+    local *PVE::RADOS::new = sub { $rados };
+    local *PVE::Ceph::Tools::create_ecprofile = sub { push @profiles, join(' ', @_[0 .. 2]) };
+    local *PVE::Ceph::Tools::create_pool = sub {
+        my ($pool, $param) = @_;
+        push @pools, "$pool:" . ($param->{erasure_code_profile} // '');
+    };
+
+    my @cases = (
+        # erasure coding, profiles created, pools created with their profile, name
+        ['profile=0', '', 'foo-data:0, foo-metadata:', "the profile '0' is used as given"],
+        [
+            'k=2,m=1',
+            'pve_ec_foo 2 1',
+            'foo-data:pve_ec_foo, foo-metadata:',
+            "without a profile, one is made from 'k' and 'm'",
+        ],
+    );
+    for my $case (@cases) {
+        my ($property, $profiles, $pools, $name) = @$case;
+        (@profiles, @pools) = ();
+        $run->(
+            'createpool',
+            { name => 'foo', node => 'node', 'erasure-coding' => $property, add_storages => 0 },
+        );
+        is(join(', ', @profiles), $profiles, "$name (profiles)");
+        is(join(', ', @pools), $pools, "$name (pools)");
+    }
+}
+
 done_testing();
-- 
2.47.3