[pve-devel] [RFC v2 qemu/storage 00/10] storage plugin method to get qemu blockdevice options for volume

[pve-devel] [RFC v2 qemu/storage 00/10] storage plugin method to get qemu blockdevice options for volume

[Fiona Ebner]

Changes in v2:
* Patch QEMU to support both keyring and rbd_cache_policy settings,
  it is much cleaner than generate per-volume (or technically would
  even be per-instance, as a volume could be re-used) temporary
  ceph.conf files. For keyring, we could also make sure to write the
  path to every storage's ceph.conf in pve8to9 and require it going
  forward, but supporting the option in QEMU seems to be the simplest.
  The rbd_cache_policy is truly a per-volume setting in Proxmox VE,
  which is specifically changed for EFI disks, so ceph.conf is just
  not the right place for that setting.
* Add a special 'hints' option to indicate when it's an EFI disk, so
  the RBD plugin can set the policy.
* Drop $snapshot parameter, currently there is no need to attach
  snapshots via -blockdev. They would need to be attached read-only
  too to not fail and this can always be added later.

When using -drive, storage plugins currently give us a path that QEMU
understands, some using special protocols such as 'iscsi://'. We'd
like to switch to using the more modern -blockdev for PVE 9. The
plan is to have the storage plugins return the very basic information
required to access the image, and qemu-server can then add other
settings like cache, aio, etc. on top. In fact, pretty similar to what
we have now for -drive, just with a structured hash rather than a
string.

This is also a prerequisite for qemu-storage-deamon, that would be
useful for TPM-as-qcow2 exported via NBD or FUSE or external backup
provider restore providing an NBD export for the provider to write to.

QEMU patches submitted upstream here:
https://lore.kernel.org/qemu-devel/20250515112908.383693-1-f.ebner@proxmox.com/

qemu:

Fiona Ebner (2):
  block/rbd: support selected key-value-pairs via QAPI
  block/rbd: support keyring option via QAPI

 block/rbd.c          | 81 ++++++++++++++++++++++++++++++++++++++++++++
 qapi/block-core.json | 40 ++++++++++++++++++++++
 2 files changed, 121 insertions(+)

storage:

Fiona Ebner (8):
  plugin: add method to get qemu blockdevice options for volume
  iscsi direct plugin: implement method to get qemu blockdevice options
  zfs iscsi plugin: implement new method to get qemu blockdevice options
  zfs pool plugin: implement method to get qemu blockdevice options
  rbd plugin: implement new method to get qemu blockdevice options
  plugin: qemu block device: add hints option and EFI disk hint
  plugin: qemu block device: add support for snapshot option
  plugin api: bump api version and age

 ApiChangeLog                         | 13 ++++
 src/PVE/Storage.pm                   | 21 ++++++-
 src/PVE/Storage/ISCSIDirectPlugin.pm | 17 +++++
 src/PVE/Storage/Plugin.pm            | 93 ++++++++++++++++++++++++++++
 src/PVE/Storage/RBDPlugin.pm         | 49 +++++++++++++++
 src/PVE/Storage/ZFSPlugin.pm         | 19 ++++++
 src/PVE/Storage/ZFSPoolPlugin.pm     | 12 ++++
 7 files changed, 222 insertions(+), 2 deletions(-)

-- 
2.39.5



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

[pve-devel] [RFC v2 qemu 01/10] block/rbd: support selected key-value-pairs via QAPI

[Fiona Ebner]

Currently, most Ceph configuration options are not exposed via QAPI.
While it is possible to specify a dedicated Ceph configuration file,
specialized options are often only required for a selection of images
on the RBD storage, not all of them. To avoid the need to generate a
dedicated Ceph configuration file for each image (or for each required
combination of options), support a selection of key-value pairs via
QAPI.

Initially, this is just 'rbd_cache_policy'. For example, this is
useful with small images used as a pflash for EFI variables. Setting
the 'rbd_cache_policy' to 'writeback' yields a substantial improvement
there [0].

The function qemu_rbd_extract_key_value_pairs() was copied/adapted
from the existing qemu_rbd_extract_encryption_create_options().

[0]: https://bugzilla.proxmox.com/show_bug.cgi?id=3329#c9

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

New in v2.

 block/rbd.c          | 73 ++++++++++++++++++++++++++++++++++++++++++++
 qapi/block-core.json | 37 ++++++++++++++++++++++
 2 files changed, 110 insertions(+)

diff --git a/block/rbd.c b/block/rbd.c
index 24e820d056..3928d8fee4 100644
--- a/block/rbd.c
+++ b/block/rbd.c
@@ -293,6 +293,27 @@ static int qemu_rbd_set_auth(rados_t cluster, BlockdevOptionsRbd *opts,
     return 0;
 }
 
+static int qemu_rbd_set_key_value_pairs(rados_t cluster,
+                                        RbdKeyValuePairs *key_value_pairs,
+                                        Error **errp)
+{
+    if (!key_value_pairs) {
+        return 0;
+    }
+
+    if (key_value_pairs->has_rbd_cache_policy) {
+        RbdCachePolicy value = key_value_pairs->rbd_cache_policy;
+        int r = rados_conf_set(cluster, "rbd_cache_policy",
+                               RbdCachePolicy_str(value));
+        if (r < 0) {
+            error_setg_errno(errp, -r, "could not set 'rbd_cache_policy'");
+            return -EINVAL;
+        }
+    }
+
+    return 0;
+}
+
 static int qemu_rbd_set_keypairs(rados_t cluster, const char *keypairs_json,
                                  Error **errp)
 {
@@ -786,6 +807,44 @@ exit:
     return ret;
 }
 
+static int qemu_rbd_extract_key_value_pairs(
+        QemuOpts *opts,
+        RbdKeyValuePairs **key_value_pairs,
+        Error **errp)
+{
+    QDict *opts_qdict;
+    QDict *key_value_pairs_qdict;
+    Visitor *v;
+    int ret = 0;
+
+    opts_qdict = qemu_opts_to_qdict(opts, NULL);
+    qdict_extract_subqdict(opts_qdict, &key_value_pairs_qdict,
+                           "key-value-pairs.");
+    qobject_unref(opts_qdict);
+    if (!qdict_size(key_value_pairs_qdict)) {
+        *key_value_pairs = NULL;
+        goto exit;
+    }
+
+    /* Convert options into a QAPI object */
+    v = qobject_input_visitor_new_flat_confused(key_value_pairs_qdict, errp);
+    if (!v) {
+        ret = -EINVAL;
+        goto exit;
+    }
+
+    visit_type_RbdKeyValuePairs(v, NULL, key_value_pairs, errp);
+    visit_free(v);
+    if (!*key_value_pairs) {
+        ret = -EINVAL;
+        goto exit;
+    }
+
+exit:
+    qobject_unref(key_value_pairs_qdict);
+    return ret;
+}
+
 static int coroutine_fn qemu_rbd_co_create_opts(BlockDriver *drv,
                                                 const char *filename,
                                                 QemuOpts *opts,
@@ -795,6 +854,7 @@ static int coroutine_fn qemu_rbd_co_create_opts(BlockDriver *drv,
     BlockdevCreateOptionsRbd *rbd_opts;
     BlockdevOptionsRbd *loc;
     RbdEncryptionCreateOptions *encrypt = NULL;
+    RbdKeyValuePairs *key_value_pairs = NULL;
     Error *local_err = NULL;
     const char *keypairs, *password_secret;
     QDict *options = NULL;
@@ -843,6 +903,13 @@ static int coroutine_fn qemu_rbd_co_create_opts(BlockDriver *drv,
     loc->image       = g_strdup(qdict_get_try_str(options, "image"));
     keypairs         = qdict_get_try_str(options, "=keyvalue-pairs");
 
+    /* These are the key-value pairs coming in via the QAPI. */
+    ret = qemu_rbd_extract_key_value_pairs(opts, &key_value_pairs, errp);
+    if (ret < 0) {
+        goto exit;
+    }
+    loc->key_value_pairs = key_value_pairs;
+
     ret = qemu_rbd_do_create(create_options, keypairs, password_secret, errp);
     if (ret < 0) {
         goto exit;
@@ -932,6 +999,12 @@ static int qemu_rbd_connect(rados_t *cluster, rados_ioctx_t *io_ctx,
         goto failed_shutdown;
     }
 
+    /* For the key-value pairs coming via QAPI. */
+    r = qemu_rbd_set_key_value_pairs(*cluster, opts->key_value_pairs, errp);
+    if (r < 0) {
+        goto failed_shutdown;
+    }
+
     if (mon_host) {
         r = rados_conf_set(*cluster, "mon_host", mon_host);
         if (r < 0) {
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 02c043f0f7..360f467f6e 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -4645,6 +4645,39 @@
   'data': { 'luks': 'RbdEncryptionCreateOptionsLUKS',
             'luks2': 'RbdEncryptionCreateOptionsLUKS2' } }
 
+##
+# @RbdCachePolicy:
+#
+# An enumeration of values for the 'rbd_cache_policy' Ceph
+# configuration setting.  See the Ceph documentation for details.
+#
+# @writearound: cachable writes return immediately, reads are not
+#     served from the cache.
+#
+# @writeback: cachable writes return immediately, reads are served
+#     from the cache.
+#
+# @writethrough: writes return only when the data is on disk for all
+#     replicas, reads are served from the cache.
+#
+# Since 10.1
+##
+{ 'enum' : 'RbdCachePolicy',
+  'data' : [ 'writearound', 'writeback', 'writethrough' ] }
+
+
+##
+# @RbdKeyValuePairs:
+#
+# Key-value pairs for Ceph configuration.
+#
+# @rbd-cache-policy: Ceph configuration option 'rbd_cache_policy'.
+#
+# Since 10.1
+##
+{ 'struct': 'RbdKeyValuePairs',
+  'data': { '*rbd-cache-policy': 'RbdCachePolicy' } }
+
 ##
 # @BlockdevOptionsRbd:
 #
@@ -4671,6 +4704,9 @@
 #     authentication.  This maps to Ceph configuration option "key".
 #     (Since 3.0)
 #
+# @key-value-pairs: Key-value pairs for additional Ceph configuraton.
+#     (Since 10.1)
+#
 # @server: Monitor host address and port.  This maps to the "mon_host"
 #     Ceph option.
 #
@@ -4686,6 +4722,7 @@
             '*user': 'str',
             '*auth-client-required': ['RbdAuthMode'],
             '*key-secret': 'str',
+            '*key-value-pairs' : 'RbdKeyValuePairs',
             '*server': ['InetSocketAddressBase'] } }
 
 ##
-- 
2.39.5



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

[pve-devel] [RFC v2 qemu 02/10] block/rbd: support keyring option via QAPI

[Fiona Ebner]

In Proxmox VE, it is not always required to have a dedicated Ceph
configuration file, and using the 'key-secret' QAPI option would
require obtaining a key from the keyring first. The keyring location
is readily available however, so having support for the 'keyring'
configuration option is most convenient.

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

New in v2.

 block/rbd.c          | 8 ++++++++
 qapi/block-core.json | 5 ++++-
 2 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/block/rbd.c b/block/rbd.c
index 3928d8fee4..0eac4c1d3e 100644
--- a/block/rbd.c
+++ b/block/rbd.c
@@ -301,6 +301,14 @@ static int qemu_rbd_set_key_value_pairs(rados_t cluster,
         return 0;
     }
 
+    if (key_value_pairs->keyring) {
+        int r = rados_conf_set(cluster, "keyring", key_value_pairs->keyring);
+        if (r < 0) {
+            error_setg_errno(errp, -r, "could not set 'keyring'");
+            return -EINVAL;
+        }
+    }
+
     if (key_value_pairs->has_rbd_cache_policy) {
         RbdCachePolicy value = key_value_pairs->rbd_cache_policy;
         int r = rados_conf_set(cluster, "rbd_cache_policy",
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 360f467f6e..781a762768 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -4671,12 +4671,15 @@
 #
 # Key-value pairs for Ceph configuration.
 #
+# @keyring: Ceph configuration option 'keyring'.
+#
 # @rbd-cache-policy: Ceph configuration option 'rbd_cache_policy'.
 #
 # Since 10.1
 ##
 { 'struct': 'RbdKeyValuePairs',
-  'data': { '*rbd-cache-policy': 'RbdCachePolicy' } }
+  'data': { '*keyring': 'str',
+            '*rbd-cache-policy': 'RbdCachePolicy' } }
 
 ##
 # @BlockdevOptionsRbd:
-- 
2.39.5



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

[pve-devel] [RFC v2 storage 03/10] plugin: add method to get qemu blockdevice options for volume

[Fiona Ebner]

This is in preparation to switch qemu-server from using '-drive' to
the modern '-blockdev' in the QEMU commandline options as well as for
the qemu-storage-daemon, which only supports '-blockdev'. The plugins
know best what driver and options are needed to access an image, so
a dedicated plugin method returning the necessary parameters for
'-blockdev' is the most straight-forward.

There intentionally is only handling for absolute paths in the default
plugin implementation. Any plugin requiring more needs to implement
the method itself. With PVE 9 being a major release and most popular
plugins not using special protocols like 'rbd://', this seems
acceptable.

For NBD, etc. qemu-server should construct the blockdev object.

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

Changes in v2:
* Remove $snapname, is added in later patch as an option.
* Check result for stat() call.
* Use filesystem_path(), rather than path().

 src/PVE/Storage.pm        | 17 +++++++++++
 src/PVE/Storage/Plugin.pm | 59 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 76 insertions(+)

diff --git a/src/PVE/Storage.pm b/src/PVE/Storage.pm
index d0a696a..d671080 100755
--- a/src/PVE/Storage.pm
+++ b/src/PVE/Storage.pm
@@ -710,6 +710,23 @@ sub abs_filesystem_path {
     return $path;
 }
 
+# see the documentation for the plugin method
+sub qemu_blockdev_options {
+    my ($cfg, $volid) = @_;
+
+    my ($storeid, $volname) = parse_volume_id($volid);
+
+    my $scfg = storage_config($cfg, $storeid);
+
+    my $plugin = PVE::Storage::Plugin->lookup($scfg->{type});
+
+    my ($vtype) = $plugin->parse_volname($volname);
+    die "cannot use volume of type '$vtype' as a QEMU blockdevice\n"
+	if $vtype ne 'images' && $vtype ne 'iso' && $vtype ne 'import';
+
+    return $plugin->qemu_blockdev_options($scfg, $storeid, $volname);
+}
+
 # used as last resort to adapt volnames when migrating
 my $volname_for_storage = sub {
     my ($cfg, $storeid, $name, $vmid, $format) = @_;
diff --git a/src/PVE/Storage/Plugin.pm b/src/PVE/Storage/Plugin.pm
index 4e16420..f162a13 100644
--- a/src/PVE/Storage/Plugin.pm
+++ b/src/PVE/Storage/Plugin.pm
@@ -1880,6 +1880,65 @@ sub rename_volume {
     return "${storeid}:${base}${target_vmid}/${target_volname}";
 }
 
+=pod
+
+=head3 qemu_blockdev_options
+
+    $blockdev = $plugin->qemu_blockdev_options($scfg, $storeid, $volname)
+
+Returns a hash reference with the basic options needed to open the volume via QEMU's C<-blockdev>
+API. This at least requires a C<< $blockdev->{driver} >> and a reference to the image, e.g.
+C<< $blockdev->{filename} >> for the C<file> driver. For files, the C<file> driver can be used. For
+host block devices, the C<host_device> driver can be used. The plugin must not set options like
+C<cache> or C<aio>. Those are managed by qemu-server and will be overwritten. For other available
+drivers and the exact specification of the options, see
+L<https://qemu.readthedocs.io/en/master/interop/qemu-qmp-ref.html#object-QMP-block-core.BlockdevOptions>
+
+While Perl does not have explicit types, the result will need to be converted to JSON later and
+match the QMP specification (see link above), so implicit types are important. In the return value,
+use C<JSON::true> and C<JSON::false> for booleans, C<"$value"> for strings, and C<int($value)> for
+integers.
+
+Arguments:
+
+=over
+
+=item C<$scfg>
+
+The hash reference with the storage configuration.
+
+=item C<$storeid>
+
+The storage ID.
+
+=item C<$volume>
+
+The volume name.
+
+=back
+
+=cut
+sub qemu_blockdev_options {
+    my ($class, $scfg, $storeid, $volname) = @_;
+
+    my $blockdev = {};
+
+    my ($path) = $class->filesystem_path($scfg, $volname);
+
+    if ($path =~ m|^/|) {
+	# The 'file' driver only works for regular files. The check below is taken from
+	# block/file-posix.c:hdev_probe_device() in QEMU. Do not bother with detecting 'host_cdrom'
+	# devices here, those are not managed by the storage layer.
+	my $st = File::stat::stat($path) or die "stat for '$path' failed - $!\n";
+	my $driver = (S_ISCHR($st->mode) || S_ISBLK($st->mode)) ? 'host_device' : 'file';
+	$blockdev = { driver => $driver, filename => $path };
+    } else {
+	die "storage plugin doesn't implement qemu_blockdev_options() method\n";
+    }
+
+    return $blockdev;
+}
+
 # Used by storage plugins for external backup providers. See PVE::BackupProvider::Plugin for the API
 # the provider needs to implement.
 #
-- 
2.39.5



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

[pve-devel] [RFC v2 storage 04/10] iscsi direct plugin: implement method to get qemu blockdevice options

[Fiona Ebner]

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

Changes in v2:
* Remove $snapname, is added in later patch as an option.

 src/PVE/Storage/ISCSIDirectPlugin.pm | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/src/PVE/Storage/ISCSIDirectPlugin.pm b/src/PVE/Storage/ISCSIDirectPlugin.pm
index d54dcd8..4bf6d07 100644
--- a/src/PVE/Storage/ISCSIDirectPlugin.pm
+++ b/src/PVE/Storage/ISCSIDirectPlugin.pm
@@ -105,6 +105,20 @@ sub path {
     return ($path, $vmid, $vtype);
 }
 
+sub qemu_blockdev_options {
+    my ($class, $scfg, $storeid, $volname) = @_;
+
+    my $lun = ($class->parse_volname($volname))[1];
+
+    return {
+	driver => 'iscsi',
+	transport => 'tcp',
+	portal => "$scfg->{portal}",
+	target => "$scfg->{target}",
+	lun => int($lun),
+    };
+}
+
 sub create_base {
     my ($class, $storeid, $scfg, $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] [RFC v2 storage 05/10] zfs iscsi plugin: implement new method to get qemu blockdevice options

[Fiona Ebner]

Reported-by: Alexandre Derumier <alexandre.derumier@groupe-cyllene.com>
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---

New in v2

 src/PVE/Storage/ZFSPlugin.pm | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/src/PVE/Storage/ZFSPlugin.pm b/src/PVE/Storage/ZFSPlugin.pm
index 94cb11f..89eff3c 100644
--- a/src/PVE/Storage/ZFSPlugin.pm
+++ b/src/PVE/Storage/ZFSPlugin.pm
@@ -247,6 +247,22 @@ sub path {
     return ($path, $vmid, $vtype);
 }
 
+sub qemu_blockdev_options {
+    my ($class, $scfg, $storeid, $volname) = @_;
+
+    my $name = ($class->parse_volname($volname))[1];
+    my $guid = $class->zfs_get_lu_name($scfg, $name);
+    my $lun = $class->zfs_get_lun_number($scfg, $guid);
+
+    return {
+	driver => 'iscsi',
+	transport => 'tcp',
+	portal => "$scfg->{portal}",
+	target => "$scfg->{target}",
+	lun => int($lun),
+    };
+}
+
 sub create_base {
     my ($class, $storeid, $scfg, $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] [RFC v2 storage 06/10] zfs pool plugin: implement method to get qemu blockdevice options

[Fiona Ebner]

ZFS does not have a filesystem_path() method, so the default
implementation for qemu_blockdev_options() cannot be re-used. This is
most likely, because snapshots are currently not directly accessible
via a filesystem path in the Proxmox VE storage layer.

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

New in v2.

 src/PVE/Storage/ZFSPoolPlugin.pm | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/src/PVE/Storage/ZFSPoolPlugin.pm b/src/PVE/Storage/ZFSPoolPlugin.pm
index 26fb0a4..313d45b 100644
--- a/src/PVE/Storage/ZFSPoolPlugin.pm
+++ b/src/PVE/Storage/ZFSPoolPlugin.pm
@@ -161,6 +161,16 @@ sub path {
     return ($path, $vmid, $vtype);
 }
 
+sub qemu_blockdev_options {
+    my ($class, $scfg, $storeid, $volname) = @_;
+
+    my ($path) = $class->path($scfg, $volname, $storeid);
+
+    my $blockdev = { driver => 'host_device', filename => $path };
+
+    return $blockdev;
+}
+
 sub zfs_request {
     my ($class, $scfg, $timeout, $method, @params) = @_;
 
-- 
2.39.5



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

[pve-devel] [RFC v2 storage 07/10] rbd plugin: implement new method to get qemu blockdevice options

[Fiona Ebner]

The mon host parsing is adapted from other places. While there
currently is no support for vector notation in the storage config
(it's a pve-storage-portal-dns-list option), it doesn't hurt to
anticipate it, should the list of mon hosts come from a ceph.conf
instead anytime in the future.

Co-developed-by: Alexandre Derumier <alexandre.derumier@groupe-cyllene.com>
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---

Changes in v2:
* Remove $snapname, is added in later patch as an option.
* Adapt to updated QEMU API.

 src/PVE/Storage/RBDPlugin.pm | 42 ++++++++++++++++++++++++++++++++++++
 1 file changed, 42 insertions(+)

diff --git a/src/PVE/Storage/RBDPlugin.pm b/src/PVE/Storage/RBDPlugin.pm
index 73bc97e..522d03c 100644
--- a/src/PVE/Storage/RBDPlugin.pm
+++ b/src/PVE/Storage/RBDPlugin.pm
@@ -493,6 +493,48 @@ sub path {
     return ($path, $vmid, $vtype);
 }
 
+sub qemu_blockdev_options {
+    my ($class, $scfg, $storeid, $volname) = @_;
+
+    my $cmd_option = PVE::CephConfig::ceph_connect_option($scfg, $storeid);
+    my ($name) = ($class->parse_volname($volname))[1];
+
+    if ($scfg->{krbd}) {
+	my $rbd_dev_path = get_rbd_dev_path($scfg, $storeid, $name);
+	return { driver => 'host_device', filename => $rbd_dev_path };
+    }
+
+    my $blockdev = {
+	driver => 'rbd',
+	pool => $scfg->{pool} ? "$scfg->{pool}" : 'rbd',
+	image => "$name",
+    };
+    $blockdev->{namespace} = "$scfg->{namespace}" if defined($scfg->{namespace});
+
+    $blockdev->{conf} = $cmd_option->{ceph_conf} if $cmd_option->{ceph_conf};
+
+    if (my $monhost = $scfg->{'monhost'}) {
+	my $server = [];
+	my @mons = PVE::Tools::split_list($monhost);
+	for my $mon (@mons) {
+	    $mon =~ s/^\[?v\d\://; # remove beginning of vector
+	    $mon =~ s|/\d+\]?||; # remove end of vector
+	    my ($host, $port) = PVE::Tools::parse_host_and_port($mon);
+	    $port = '3300' if !$port;
+	    push @$server, { host => $host, port => $port };
+	}
+	$blockdev->{server} = $server;
+	$blockdev->{'auth-client-required'} = ["$cmd_option->{auth_supported}"];
+    }
+
+    if ($cmd_option->{keyring}) {
+	$blockdev->{user} = "$cmd_option->{userid}";
+	$blockdev->{'key-value-pairs'}->{keyring} = "$cmd_option->{keyring}";
+    }
+
+    return $blockdev;
+}
+
 sub find_free_diskname {
     my ($class, $storeid, $scfg, $vmid, $fmt, $add_fmt_suffix) = @_;
 
-- 
2.39.5



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

[pve-devel] [RFC v2 storage 08/10] plugin: qemu block device: add hints option and EFI disk hint

[Fiona Ebner]

For '-drive', qemu-server sets special cache options for EFI disk
using RBD. In preparation to seamlessly switch to the new '-blockdev'
interface, do the same here. Note that the issue from bug #3329, which
is solved by these cache options, still affects current versions.

With -blockdev, the cache options are split up. While cache.direct and
cache.no-flush can be set in the -blockdev options, cache.writeback is
a front-end property and was intentionally removed from the -blockdev
options by QEMU commit aaa436f998 ("block: Remove cache.writeback from
blockdev-add"). It needs to be configured as the 'write-cache'
property for the ide-hd/scsi-hd/virtio-blk device.

The default is already 'writeback' and no cache mode can be set for an
EFI drive configuration in Proxmox VE currently, so there will not be
a clash.

┌─────────────┬─────────────────┬──────────────┬────────────────┐
│             │ cache.writeback │ cache.direct │ cache.no-flush │
├─────────────┼─────────────────┼──────────────┼────────────────┤
│writeback    │ on              │ off          │ off            │
├─────────────┼─────────────────┼──────────────┼────────────────┤
│none         │ on              │ on           │ off            │
├─────────────┼─────────────────┼──────────────┼────────────────┤
│writethrough │ off             │ off          │ off            │
├─────────────┼─────────────────┼──────────────┼────────────────┤
│directsync   │ off             │ on           │ off            │
├─────────────┼─────────────────┼──────────────┼────────────────┤
│unsafe       │ on              │ off          │ on             │
└─────────────┴─────────────────┴──────────────┴────────────────┘

Table from 'man kvm'.

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

New in v2.

 src/PVE/Storage.pm                   |  4 ++--
 src/PVE/Storage/ISCSIDirectPlugin.pm |  2 +-
 src/PVE/Storage/Plugin.pm            | 27 +++++++++++++++++++++++++--
 src/PVE/Storage/RBDPlugin.pm         |  7 ++++++-
 src/PVE/Storage/ZFSPlugin.pm         |  2 +-
 src/PVE/Storage/ZFSPoolPlugin.pm     |  2 +-
 6 files changed, 36 insertions(+), 8 deletions(-)

diff --git a/src/PVE/Storage.pm b/src/PVE/Storage.pm
index d671080..3b0f07e 100755
--- a/src/PVE/Storage.pm
+++ b/src/PVE/Storage.pm
@@ -712,7 +712,7 @@ sub abs_filesystem_path {
 
 # see the documentation for the plugin method
 sub qemu_blockdev_options {
-    my ($cfg, $volid) = @_;
+    my ($cfg, $volid, $options) = @_;
 
     my ($storeid, $volname) = parse_volume_id($volid);
 
@@ -724,7 +724,7 @@ sub qemu_blockdev_options {
     die "cannot use volume of type '$vtype' as a QEMU blockdevice\n"
 	if $vtype ne 'images' && $vtype ne 'iso' && $vtype ne 'import';
 
-    return $plugin->qemu_blockdev_options($scfg, $storeid, $volname);
+    return $plugin->qemu_blockdev_options($scfg, $storeid, $volname, $options);
 }
 
 # used as last resort to adapt volnames when migrating
diff --git a/src/PVE/Storage/ISCSIDirectPlugin.pm b/src/PVE/Storage/ISCSIDirectPlugin.pm
index 4bf6d07..8f25577 100644
--- a/src/PVE/Storage/ISCSIDirectPlugin.pm
+++ b/src/PVE/Storage/ISCSIDirectPlugin.pm
@@ -106,7 +106,7 @@ sub path {
 }
 
 sub qemu_blockdev_options {
-    my ($class, $scfg, $storeid, $volname) = @_;
+    my ($class, $scfg, $storeid, $volname, $options) = @_;
 
     my $lun = ($class->parse_volname($volname))[1];
 
diff --git a/src/PVE/Storage/Plugin.pm b/src/PVE/Storage/Plugin.pm
index f162a13..7fe1b17 100644
--- a/src/PVE/Storage/Plugin.pm
+++ b/src/PVE/Storage/Plugin.pm
@@ -1884,7 +1884,7 @@ sub rename_volume {
 
 =head3 qemu_blockdev_options
 
-    $blockdev = $plugin->qemu_blockdev_options($scfg, $storeid, $volname)
+    $blockdev = $plugin->qemu_blockdev_options($scfg, $storeid, $volname, $options)
 
 Returns a hash reference with the basic options needed to open the volume via QEMU's C<-blockdev>
 API. This at least requires a C<< $blockdev->{driver} >> and a reference to the image, e.g.
@@ -1915,11 +1915,34 @@ The storage ID.
 
 The volume name.
 
+=item C<$options>
+
+A hash reference with additional options.
+
+=over
+
+=item C<< $options->{hints} >>
+
+A hash reference with hints indicating what the volume will be used for. This can be safely ignored
+if no concrete issues are known with your plugin. For certain use cases, setting additional
+(plugin-specific) options might be very beneficial however. An example is setting the correct cache
+options for an EFI disk on RBD. The list of hints might get expanded in the future.
+
+=over
+
+=item C<< $options->{hints}->{'efi-disk'} >>
+
+(optional) If set, the volume will be used as the EFI disk of a VM, containing its OMVF variables.
+
+=back
+
+=back
+
 =back
 
 =cut
 sub qemu_blockdev_options {
-    my ($class, $scfg, $storeid, $volname) = @_;
+    my ($class, $scfg, $storeid, $volname, $options) = @_;
 
     my $blockdev = {};
 
diff --git a/src/PVE/Storage/RBDPlugin.pm b/src/PVE/Storage/RBDPlugin.pm
index 522d03c..6fa0689 100644
--- a/src/PVE/Storage/RBDPlugin.pm
+++ b/src/PVE/Storage/RBDPlugin.pm
@@ -494,7 +494,7 @@ sub path {
 }
 
 sub qemu_blockdev_options {
-    my ($class, $scfg, $storeid, $volname) = @_;
+    my ($class, $scfg, $storeid, $volname, $options) = @_;
 
     my $cmd_option = PVE::CephConfig::ceph_connect_option($scfg, $storeid);
     my ($name) = ($class->parse_volname($volname))[1];
@@ -532,6 +532,11 @@ sub qemu_blockdev_options {
 	$blockdev->{'key-value-pairs'}->{keyring} = "$cmd_option->{keyring}";
     }
 
+    # SPI flash does lots of read-modify-write OPs, without writeback this gets really slow #3329
+    if ($options->{hints}->{'efi-disk'}) {
+	$blockdev->{'key-value-pairs'}->{'rbd-cache-policy'} = 'writeback';
+    }
+
     return $blockdev;
 }
 
diff --git a/src/PVE/Storage/ZFSPlugin.pm b/src/PVE/Storage/ZFSPlugin.pm
index 89eff3c..1392781 100644
--- a/src/PVE/Storage/ZFSPlugin.pm
+++ b/src/PVE/Storage/ZFSPlugin.pm
@@ -248,7 +248,7 @@ sub path {
 }
 
 sub qemu_blockdev_options {
-    my ($class, $scfg, $storeid, $volname) = @_;
+    my ($class, $scfg, $storeid, $volname, $options) = @_;
 
     my $name = ($class->parse_volname($volname))[1];
     my $guid = $class->zfs_get_lu_name($scfg, $name);
diff --git a/src/PVE/Storage/ZFSPoolPlugin.pm b/src/PVE/Storage/ZFSPoolPlugin.pm
index 313d45b..302c965 100644
--- a/src/PVE/Storage/ZFSPoolPlugin.pm
+++ b/src/PVE/Storage/ZFSPoolPlugin.pm
@@ -162,7 +162,7 @@ sub path {
 }
 
 sub qemu_blockdev_options {
-    my ($class, $scfg, $storeid, $volname) = @_;
+    my ($class, $scfg, $storeid, $volname, $options) = @_;
 
     my ($path) = $class->path($scfg, $volname, $storeid);
 
-- 
2.39.5



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

[pve-devel] [RFC v2 storage 09/10] plugin: qemu block device: add support for snapshot option

[Fiona Ebner]

This is mostly in preparation for external qcow2 snapshot support.

For internal qcow2 snapshots, which currently are the only supported
variant, it is not possible to attach the snapshot only. If access to
that is required it will need to be handled differently, e.g. via a
FUSE/NBD export.

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

New in v2.

 src/PVE/Storage/ISCSIDirectPlugin.pm |  3 +++
 src/PVE/Storage/Plugin.pm            | 13 ++++++++++++-
 src/PVE/Storage/RBDPlugin.pm         |  2 ++
 src/PVE/Storage/ZFSPlugin.pm         |  3 +++
 src/PVE/Storage/ZFSPoolPlugin.pm     |  2 ++
 5 files changed, 22 insertions(+), 1 deletion(-)

diff --git a/src/PVE/Storage/ISCSIDirectPlugin.pm b/src/PVE/Storage/ISCSIDirectPlugin.pm
index 8f25577..23e8a24 100644
--- a/src/PVE/Storage/ISCSIDirectPlugin.pm
+++ b/src/PVE/Storage/ISCSIDirectPlugin.pm
@@ -108,6 +108,9 @@ sub path {
 sub qemu_blockdev_options {
     my ($class, $scfg, $storeid, $volname, $options) = @_;
 
+    die "volume snapshot is not possible on iscsi device\n"
+	if $options->{'snapshot-name'};
+
     my $lun = ($class->parse_volname($volname))[1];
 
     return {
diff --git a/src/PVE/Storage/Plugin.pm b/src/PVE/Storage/Plugin.pm
index 7fe1b17..54e39a9 100644
--- a/src/PVE/Storage/Plugin.pm
+++ b/src/PVE/Storage/Plugin.pm
@@ -1921,6 +1921,11 @@ A hash reference with additional options.
 
 =over
 
+=item C<< $options->{'snapshot-name'} >>
+
+(optional) The snapshot name. Set when the associated snapshot should be opened rather than the
+volume itself.
+
 =item C<< $options->{hints} >>
 
 A hash reference with hints indicating what the volume will be used for. This can be safely ignored
@@ -1946,9 +1951,15 @@ sub qemu_blockdev_options {
 
     my $blockdev = {};
 
-    my ($path) = $class->filesystem_path($scfg, $volname);
+    my ($path) = $class->filesystem_path($scfg, $volname, $options->{'snapshot-name'});
 
     if ($path =~ m|^/|) {
+	# For qcow2 and qed the path of a snapshot will be the same, but it's not possible to attach
+	# the snapshot alone.
+	my $format = ($class->parse_volname($volname))[6];
+	die "cannot attach only the snapshot of a '$format' image\n"
+	    if $options->{'snapshot-name'} && ($format eq 'qcow2' || $format eq 'qed');
+
 	# The 'file' driver only works for regular files. The check below is taken from
 	# block/file-posix.c:hdev_probe_device() in QEMU. Do not bother with detecting 'host_cdrom'
 	# devices here, those are not managed by the storage layer.
diff --git a/src/PVE/Storage/RBDPlugin.pm b/src/PVE/Storage/RBDPlugin.pm
index 6fa0689..72bd7c8 100644
--- a/src/PVE/Storage/RBDPlugin.pm
+++ b/src/PVE/Storage/RBDPlugin.pm
@@ -500,6 +500,7 @@ sub qemu_blockdev_options {
     my ($name) = ($class->parse_volname($volname))[1];
 
     if ($scfg->{krbd}) {
+	$name .= '@'.$options->{'snapshot-name'} if $options->{'snapshot-name'};
 	my $rbd_dev_path = get_rbd_dev_path($scfg, $storeid, $name);
 	return { driver => 'host_device', filename => $rbd_dev_path };
     }
@@ -510,6 +511,7 @@ sub qemu_blockdev_options {
 	image => "$name",
     };
     $blockdev->{namespace} = "$scfg->{namespace}" if defined($scfg->{namespace});
+    $blockdev->{snapshot} = $options->{'snapshot-name'} if $options->{'snapshot-name'};
 
     $blockdev->{conf} = $cmd_option->{ceph_conf} if $cmd_option->{ceph_conf};
 
diff --git a/src/PVE/Storage/ZFSPlugin.pm b/src/PVE/Storage/ZFSPlugin.pm
index 1392781..963e860 100644
--- a/src/PVE/Storage/ZFSPlugin.pm
+++ b/src/PVE/Storage/ZFSPlugin.pm
@@ -250,6 +250,9 @@ sub path {
 sub qemu_blockdev_options {
     my ($class, $scfg, $storeid, $volname, $options) = @_;
 
+    die "direct access to snapshots not implemented\n"
+	if $options->{'snapshot-name'};
+
     my $name = ($class->parse_volname($volname))[1];
     my $guid = $class->zfs_get_lu_name($scfg, $name);
     my $lun = $class->zfs_get_lun_number($scfg, $guid);
diff --git a/src/PVE/Storage/ZFSPoolPlugin.pm b/src/PVE/Storage/ZFSPoolPlugin.pm
index 302c965..cd01414 100644
--- a/src/PVE/Storage/ZFSPoolPlugin.pm
+++ b/src/PVE/Storage/ZFSPoolPlugin.pm
@@ -164,6 +164,8 @@ sub path {
 sub qemu_blockdev_options {
     my ($class, $scfg, $storeid, $volname, $options) = @_;
 
+    die "cannot attach only the snapshot of a zvol\n" if $options->{'snapshot-name'};
+
     my ($path) = $class->path($scfg, $volname, $storeid);
 
     my $blockdev = { driver => 'host_device', filename => $path };
-- 
2.39.5



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

[pve-devel] [RFC v2 storage 10/10] plugin api: bump api version and age

[Fiona Ebner]

Introduce qemu_blockdev_options() plugin method.

In terms of the plugin API only, adding the qemu_blockdev_options()
method is a fully backwards-compatible change. When qemu-server will
switch to '-blockdev' however, plugins where the default implemenation
is not sufficient, will not be usable for virtual machines anymore.
Therefore, this is intended for the next major release, Proxmox VE 9.

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

New in v2.

 ApiChangeLog       | 13 +++++++++++++
 src/PVE/Storage.pm |  4 ++--
 2 files changed, 15 insertions(+), 2 deletions(-)

diff --git a/ApiChangeLog b/ApiChangeLog
index 987da54..062e75b 100644
--- a/ApiChangeLog
+++ b/ApiChangeLog
@@ -6,6 +6,19 @@ without breaking anything unaware of it.)
 
 Future changes should be documented in here.
 
+## Version 12:
+
+* Introduce `qemu_blockdev_options()` plugin method
+
+  Proxmox VE will switch to the more modern QEMU command line option `-blockdev` replacing `-drive`.
+  With `-drive`, it was enough to specify a path, where special protocol paths like `iscsi://` were
+  also supported. With `-blockdev`, the data is more structured, a driver needs to be specified
+  alongside the path to an image and each driver supports driver-specific options. Most storage
+  plugins should be fine using driver `host_device` in case of a block device and `file` in case of
+  a file and no special options, see the default implemenation of the base plugin. Implement this
+  this method for Proxmox VE 9. For available drivers and driver-specific options, see:
+  https://qemu.readthedocs.io/en/master/interop/qemu-qmp-ref.html#object-QMP-block-core.BlockdevOptions
+
 ##  Version 11:
 
 * Allow declaring storage features via plugin data
diff --git a/src/PVE/Storage.pm b/src/PVE/Storage.pm
index 3b0f07e..39bb492 100755
--- a/src/PVE/Storage.pm
+++ b/src/PVE/Storage.pm
@@ -42,11 +42,11 @@ use PVE::Storage::BTRFSPlugin;
 use PVE::Storage::ESXiPlugin;
 
 # Storage API version. Increment it on changes in storage API interface.
-use constant APIVER => 11;
+use constant APIVER => 12;
 # Age is the number of versions we're backward compatible with.
 # This is like having 'current=APIVER' and age='APIAGE' in libtool,
 # see https://www.gnu.org/software/libtool/manual/html_node/Libtool-versioning.html
-use constant APIAGE => 2;
+use constant APIAGE => 3;
 
 our $KNOWN_EXPORT_FORMATS = ['raw+size', 'tar+size', 'qcow2+size', 'vmdk+size', 'zfs', 'btrfs'];
 
-- 
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] [RFC v2 qemu/storage 00/10] storage plugin method to get qemu blockdevice options for volume

[DERUMIER, Alexandre via pve-devel]

[-- Attachment #1: Type: message/rfc822, Size: 13244 bytes --]

From: "DERUMIER, Alexandre" <alexandre.derumier@groupe-cyllene.com>
To: "pve-devel@lists.proxmox.com" <pve-devel@lists.proxmox.com>
Subject: Re: [pve-devel] [RFC v2 qemu/storage 00/10] storage plugin method to get qemu blockdevice options for volume
Date: Mon, 2 Jun 2025 16:12:34 +0000
Message-ID: <9d08a1aab040d04a6f3096dfd1b3dabd30fa9315.camel@groupe-cyllene.com>

Hi Fiona,

I'm currently finishing my new patch series

>>
>>* Drop $snapshot parameter, currently there is no need to attach
>>  snapshots via -blockdev. They would need to be attached read-only
>>  too to not fail and this can always be added later.


I need it in different places in the external snapshot code (to
generate the backing chain, in the snapshot creation to rettach renamed
snapshot volname,...)


[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

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

Re: [pve-devel] [RFC v2 qemu/storage 00/10] storage plugin method to get qemu blockdevice options for volume

[DERUMIER, Alexandre via pve-devel]

[-- Attachment #1: Type: message/rfc822, Size: 13364 bytes --]

From: "DERUMIER, Alexandre" <alexandre.derumier@groupe-cyllene.com>
To: "pve-devel@lists.proxmox.com" <pve-devel@lists.proxmox.com>
Subject: Re: [pve-devel] [RFC v2 qemu/storage 00/10] storage plugin method to get qemu blockdevice options for volume
Date: Mon, 2 Jun 2025 16:24:05 +0000
Message-ID: <4b3ad43f6c91ad6164d23280afcda8b7e7393cdb.camel@groupe-cyllene.com>


> > 
> > * Drop $snapshot parameter, currently there is no need to attach
> >   snapshots via -blockdev. They would need to be attached read-only
> >   too to not fail and this can always be added later.


>>I need it in different places in the external snapshot code (to
>>generate the backing chain, in the snapshot creation to rettach
>>renamed
>>snapshot volname,...)

Ah, sorry, I didn't have see the $options->{'snapshot-name'}.





[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

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

Re: [pve-devel] [RFC v2 qemu/storage 00/10] storage plugin method to get qemu blockdevice options for volume

[Fiona Ebner]

Am 02.06.25 um 18:24 schrieb DERUMIER, Alexandre via pve-devel:
>>>
>>> * Drop $snapshot parameter, currently there is no need to attach
>>>   snapshots via -blockdev. They would need to be attached read-only
>>>   too to not fail and this can always be added later.
> 
> 
>>> I need it in different places in the external snapshot code (to
>>> generate the backing chain, in the snapshot creation to rettach
>>> renamed
>>> snapshot volname,...)
> 
> Ah, sorry, I didn't have see the $options->{'snapshot-name'}.

Initially, I dropped it, but added it back via $options before sending
v2. Sorry about the confusion/wrong changelog entry.


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