public inbox for pve-devel@lists.proxmox.com
 help / color / mirror / Atom feed
* [pve-devel] [RFC qemu/pve-storage] storage plugin method to get qemu blockdevice options for volume
@ 2025-05-09 14:15 Fiona Ebner
  2025-05-09 14:15 ` [pve-devel] [RFC qemu 1/1] block/rbd: add @keyring-file option to BlockdevOptionsRbd Fiona Ebner
                   ` (4 more replies)
  0 siblings, 5 replies; 26+ messages in thread
From: Fiona Ebner @ 2025-05-09 14:15 UTC (permalink / raw)
  To: pve-devel

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.

This is an early sneak peek to get feedback on the idea itself and to
keep Alexandre in the loop :)

See the individual patches for more comments.


qemu:

Fiona Ebner (1):
  block/rbd: add @keyring-file option to BlockdevOptionsRbd

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


pve-storage:

Fiona Ebner (3):
  plugin: add method to get qemu blockdevice options for volume
  iscsi direct plugin: implement method to get qemu blockdevice options
  rbd plugin: implement new method to get qemu blockdevice options

 src/PVE/Storage.pm                   | 19 +++++++++
 src/PVE/Storage/ISCSIDirectPlugin.pm | 17 ++++++++
 src/PVE/Storage/Plugin.pm            | 64 ++++++++++++++++++++++++++++
 src/PVE/Storage/RBDPlugin.pm         | 43 +++++++++++++++++++
 4 files changed, 143 insertions(+)

-- 
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] 26+ messages in thread

* [pve-devel] [RFC qemu 1/1] block/rbd: add @keyring-file option to BlockdevOptionsRbd
  2025-05-09 14:15 [pve-devel] [RFC qemu/pve-storage] storage plugin method to get qemu blockdevice options for volume Fiona Ebner
@ 2025-05-09 14:15 ` Fiona Ebner
  2025-05-12 10:57   ` DERUMIER, Alexandre via pve-devel
       [not found]   ` <dfc78aa17b9c1c8496fa74cb6e6d2517337b65c0.camel@groupe-cyllene.com>
  2025-05-09 14:15 ` [pve-devel] [RFC storage 1/3] plugin: add method to get qemu blockdevice options for volume Fiona Ebner
                   ` (3 subsequent siblings)
  4 siblings, 2 replies; 26+ messages in thread
From: Fiona Ebner @ 2025-05-09 14:15 UTC (permalink / raw)
  To: pve-devel

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

I'll also submit this upstream.

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

diff --git a/block/rbd.c b/block/rbd.c
index 24e820d056..53a9a785f0 100644
--- a/block/rbd.c
+++ b/block/rbd.c
@@ -272,6 +272,14 @@ static int qemu_rbd_set_auth(rados_t cluster, BlockdevOptionsRbd *opts,
         }
     }
 
+    if (opts->keyring_file) {
+        r = rados_conf_set(cluster, "keyring", opts->keyring_file);
+        if (r < 0) {
+            error_setg_errno(errp, -r, "Could not set 'keyring'");
+            return r;
+        }
+    }
+
     if (opts->has_auth_client_required) {
         accu = g_string_new("");
         for (auth = opts->auth_client_required; auth; auth = auth->next) {
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 02c043f0f7..100277b371 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -4671,6 +4671,9 @@
 #     authentication.  This maps to Ceph configuration option "key".
 #     (Since 3.0)
 #
+# @keyring-file: File for the keyring used for cephx authentication.
+#     This maps to Ceph configuration option "keyring". (Since 10.1)
+#
 # @server: Monitor host address and port.  This maps to the "mon_host"
 #     Ceph option.
 #
@@ -4686,6 +4689,7 @@
             '*user': 'str',
             '*auth-client-required': ['RbdAuthMode'],
             '*key-secret': 'str',
+            '*keyring-file': 'str',
             '*server': ['InetSocketAddressBase'] } }
 
 ##
-- 
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] 26+ messages in thread

* [pve-devel] [RFC storage 1/3] plugin: add method to get qemu blockdevice options for volume
  2025-05-09 14:15 [pve-devel] [RFC qemu/pve-storage] storage plugin method to get qemu blockdevice options for volume Fiona Ebner
  2025-05-09 14:15 ` [pve-devel] [RFC qemu 1/1] block/rbd: add @keyring-file option to BlockdevOptionsRbd Fiona Ebner
@ 2025-05-09 14:15 ` Fiona Ebner
  2025-05-23  8:19   ` DERUMIER, Alexandre via pve-devel
                     ` (3 more replies)
  2025-05-09 14:15 ` [pve-devel] [RFC storage 2/3] iscsi direct plugin: implement method to get qemu blockdevice options Fiona Ebner
                   ` (2 subsequent siblings)
  4 siblings, 4 replies; 26+ messages in thread
From: Fiona Ebner @ 2025-05-09 14:15 UTC (permalink / raw)
  To: pve-devel

There intentionally is only handling for absolute paths in the default
plugin implementation. Any plugin requiring more needs to implement
the method itself.

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

I discussed this with Fabian off-list. With PVE 9 being a major
release and most popular plugins not using special protocols like
'rbd://', this seems acceptable. He'll also give a heads-up to known
plugin developers with other changes for PVE 9 in time.

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

Still missing API bump + Changelog

Did not test snapshots yet.

 src/PVE/Storage.pm        | 19 ++++++++++++
 src/PVE/Storage/Plugin.pm | 64 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 83 insertions(+)

diff --git a/src/PVE/Storage.pm b/src/PVE/Storage.pm
index d0a696a..5915395 100755
--- a/src/PVE/Storage.pm
+++ b/src/PVE/Storage.pm
@@ -710,6 +710,25 @@ sub abs_filesystem_path {
     return $path;
 }
 
+sub qemu_blockdev_options {
+    my ($cfg, $volid, $snapname) = @_;
+
+    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';
+
+    die "QEMU blockdevice - 'snapname' argument is not supported for vtype '$vtype'"
+	if $snapname && $vtype ne 'images';
+
+    return $plugin->qemu_blockdev_options($scfg, $storeid, $volname, $snapname);
+}
+
 # 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..dc3c6df 100644
--- a/src/PVE/Storage/Plugin.pm
+++ b/src/PVE/Storage/Plugin.pm
@@ -1880,6 +1880,70 @@ sub rename_volume {
     return "${storeid}:${base}${target_vmid}/${target_volname}";
 }
 
+=pod
+
+=head3 qemu_blockdev_options
+
+    $blockdev = $plugin->qemu_blockdev_options($scfg, $storeid, $volname, $snapname)
+
+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.
+
+=item C<$snapname>
+
+(optional) The snapshot name. Set when the associated snapshot should be opened
+rather than the volume itself.
+
+=back
+
+=cut
+sub qemu_blockdev_options {
+    my ($class, $scfg, $storeid, $volname, $snapname) = @_;
+
+    my $blockdev = {};
+
+    my ($path) = $class->path($scfg, $volname, $storeid, $snapname);
+
+    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);
+	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


^ permalink raw reply	[flat|nested] 26+ messages in thread

* [pve-devel] [RFC storage 2/3] iscsi direct plugin: implement method to get qemu blockdevice options
  2025-05-09 14:15 [pve-devel] [RFC qemu/pve-storage] storage plugin method to get qemu blockdevice options for volume Fiona Ebner
  2025-05-09 14:15 ` [pve-devel] [RFC qemu 1/1] block/rbd: add @keyring-file option to BlockdevOptionsRbd Fiona Ebner
  2025-05-09 14:15 ` [pve-devel] [RFC storage 1/3] plugin: add method to get qemu blockdevice options for volume Fiona Ebner
@ 2025-05-09 14:15 ` Fiona Ebner
  2025-05-12 13:14   ` Fiona Ebner
  2025-05-09 14:15 ` [pve-devel] [RFC storage 3/3] rbd plugin: implement new " Fiona Ebner
  2025-05-09 14:21 ` [pve-devel] [RFC qemu/pve-storage] storage plugin method to get qemu blockdevice options for volume Fiona Ebner
  4 siblings, 1 reply; 26+ messages in thread
From: Fiona Ebner @ 2025-05-09 14:15 UTC (permalink / raw)
  To: pve-devel

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

for me, it fails with
VM 102 qmp command 'blockdev-add' failed - LUN is write protected
but adding as a drive fails. Will need to investigate.

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

diff --git a/src/PVE/Storage/ISCSIDirectPlugin.pm b/src/PVE/Storage/ISCSIDirectPlugin.pm
index d54dcd8..a6381c2 100644
--- a/src/PVE/Storage/ISCSIDirectPlugin.pm
+++ b/src/PVE/Storage/ISCSIDirectPlugin.pm
@@ -105,6 +105,23 @@ sub path {
     return ($path, $vmid, $vtype);
 }
 
+sub qemu_blockdev_options {
+    my ($class, $scfg, $storeid, $volname, $snapname) = @_;
+
+    die "volume snapshot is not possible on iscsi device\n"
+	if defined($snapname);
+
+    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


^ permalink raw reply	[flat|nested] 26+ messages in thread

* [pve-devel] [RFC storage 3/3] rbd plugin: implement new method to get qemu blockdevice options
  2025-05-09 14:15 [pve-devel] [RFC qemu/pve-storage] storage plugin method to get qemu blockdevice options for volume Fiona Ebner
                   ` (2 preceding siblings ...)
  2025-05-09 14:15 ` [pve-devel] [RFC storage 2/3] iscsi direct plugin: implement method to get qemu blockdevice options Fiona Ebner
@ 2025-05-09 14:15 ` Fiona Ebner
  2025-05-09 14:21 ` [pve-devel] [RFC qemu/pve-storage] storage plugin method to get qemu blockdevice options for volume Fiona Ebner
  4 siblings, 0 replies; 26+ messages in thread
From: Fiona Ebner @ 2025-05-09 14:15 UTC (permalink / raw)
  To: pve-devel

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

The mon host parsing is adapted from other places where we do that,
but I did not test it with IPv6 yet.

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

diff --git a/src/PVE/Storage/RBDPlugin.pm b/src/PVE/Storage/RBDPlugin.pm
index 73bc97e..6ae8216 100644
--- a/src/PVE/Storage/RBDPlugin.pm
+++ b/src/PVE/Storage/RBDPlugin.pm
@@ -493,6 +493,49 @@ sub path {
     return ($path, $vmid, $vtype);
 }
 
+sub qemu_blockdev_options {
+    my ($class, $scfg, $storeid, $volname, $snapname) = @_;
+
+    my $cmd_option = PVE::CephConfig::ceph_connect_option($scfg, $storeid);
+    my ($name) = ($class->parse_volname($volname))[1];
+    $name .= '@'.$snapname if $snapname;
+
+    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->{'keyring-file'} = "$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


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [pve-devel] [RFC qemu/pve-storage] storage plugin method to get qemu blockdevice options for volume
  2025-05-09 14:15 [pve-devel] [RFC qemu/pve-storage] storage plugin method to get qemu blockdevice options for volume Fiona Ebner
                   ` (3 preceding siblings ...)
  2025-05-09 14:15 ` [pve-devel] [RFC storage 3/3] rbd plugin: implement new " Fiona Ebner
@ 2025-05-09 14:21 ` Fiona Ebner
  4 siblings, 0 replies; 26+ messages in thread
From: Fiona Ebner @ 2025-05-09 14:21 UTC (permalink / raw)
  To: pve-devel

And here is a small script I used for testing to see if QMP blockdev-add
can deal with the results:

> [I] root@pve8a1 ~# cat test-storage-blockdev.pl 
> #!/usr/bin/perl
> 
> use strict;
> use warnings;
> 
> use JSON;
> use PVE::QemuServer::Monitor qw(mon_cmd);
> use PVE::Storage;
> 
> my $vmid = shift or die "need to specify VM ID\n";
> my $volid = shift or die "need to specify volume ID\n";
> my $snapname; # TODO
> 
> my $conf = PVE::Storage::config();
> 
> my $blockdev = PVE::Storage::qemu_blockdev_options($conf, $volid, $snapname);
> $blockdev->{'node-name'} = "a" . rand(1);
> 
> print to_json($blockdev, { canonical => 1, pretty => 1 });
> 
> mon_cmd($vmid, 'blockdev-add', $blockdev->%*);



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


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [pve-devel] [RFC qemu 1/1] block/rbd: add @keyring-file option to BlockdevOptionsRbd
  2025-05-09 14:15 ` [pve-devel] [RFC qemu 1/1] block/rbd: add @keyring-file option to BlockdevOptionsRbd Fiona Ebner
@ 2025-05-12 10:57   ` DERUMIER, Alexandre via pve-devel
       [not found]   ` <dfc78aa17b9c1c8496fa74cb6e6d2517337b65c0.camel@groupe-cyllene.com>
  1 sibling, 0 replies; 26+ messages in thread
From: DERUMIER, Alexandre via pve-devel @ 2025-05-12 10:57 UTC (permalink / raw)
  To: pve-devel, f.ebner; +Cc: DERUMIER, Alexandre

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

From: "DERUMIER, Alexandre" <alexandre.derumier@groupe-cyllene.com>
To: "pve-devel@lists.proxmox.com" <pve-devel@lists.proxmox.com>, "f.ebner@proxmox.com" <f.ebner@proxmox.com>
Subject: Re: [RFC qemu 1/1] block/rbd: add @keyring-file option to BlockdevOptionsRbd
Date: Mon, 12 May 2025 10:57:11 +0000
Message-ID: <dfc78aa17b9c1c8496fa74cb6e6d2517337b65c0.camel@groupe-cyllene.com>

for blockdev, do we still use a ceph config file in /var/run for
potential others rbd client options ?



-------- Message initial --------
De: Fiona Ebner <f.ebner@proxmox.com>
À: pve-devel@lists.proxmox.com
Cc: alexandre.derumier@groupe-cyllene.com
Objet: [RFC qemu 1/1] block/rbd: add @keyring-file option to
BlockdevOptionsRbd
Date: 09/05/2025 16:15:29

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

I'll also submit this upstream.

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

diff --git a/block/rbd.c b/block/rbd.c
index 24e820d056..53a9a785f0 100644
--- a/block/rbd.c
+++ b/block/rbd.c
@@ -272,6 +272,14 @@ static int qemu_rbd_set_auth(rados_t cluster,
BlockdevOptionsRbd *opts,
         }
     }
 
+    if (opts->keyring_file) {
+        r = rados_conf_set(cluster, "keyring", opts->keyring_file);
+        if (r < 0) {
+            error_setg_errno(errp, -r, "Could not set 'keyring'");
+            return r;
+        }
+    }
+
     if (opts->has_auth_client_required) {
         accu = g_string_new("");
         for (auth = opts->auth_client_required; auth; auth = auth-
>next) {
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 02c043f0f7..100277b371 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -4671,6 +4671,9 @@
 #     authentication.  This maps to Ceph configuration option "key".
 #     (Since 3.0)
 #
+# @keyring-file: File for the keyring used for cephx authentication.
+#     This maps to Ceph configuration option "keyring". (Since 10.1)
+#
 # @server: Monitor host address and port.  This maps to the "mon_host"
 #     Ceph option.
 #
@@ -4686,6 +4689,7 @@
             '*user': 'str',
             '*auth-client-required': ['RbdAuthMode'],
             '*key-secret': 'str',
+            '*keyring-file': 'str',
             '*server': ['InetSocketAddressBase'] } }
 
 ##


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

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [pve-devel] [RFC qemu 1/1] block/rbd: add @keyring-file option to BlockdevOptionsRbd
       [not found]   ` <dfc78aa17b9c1c8496fa74cb6e6d2517337b65c0.camel@groupe-cyllene.com>
@ 2025-05-12 11:25     ` Fiona Ebner
  2025-05-12 13:39       ` DERUMIER, Alexandre via pve-devel
       [not found]       ` <330ddb6da2469b425acda6ceb9cdaf5a510a854f.camel@groupe-cyllene.com>
  0 siblings, 2 replies; 26+ messages in thread
From: Fiona Ebner @ 2025-05-12 11:25 UTC (permalink / raw)
  To: DERUMIER, Alexandre, pve-devel

Am 12.05.25 um 12:57 schrieb DERUMIER, Alexandre:
> for blockdev, do we still use a ceph config file in /var/run for
> potential others rbd client options ?

Not currently, but we can add that later if we consider it worth it. We
would need to merge with the storage's already existing ceph.conf and
not only write the new options. For now, users can adapt their storage's
ceph.conf as desired.

Best Regards,
Fiona


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


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [pve-devel] [RFC storage 2/3] iscsi direct plugin: implement method to get qemu blockdevice options
  2025-05-09 14:15 ` [pve-devel] [RFC storage 2/3] iscsi direct plugin: implement method to get qemu blockdevice options Fiona Ebner
@ 2025-05-12 13:14   ` Fiona Ebner
  0 siblings, 0 replies; 26+ messages in thread
From: Fiona Ebner @ 2025-05-12 13:14 UTC (permalink / raw)
  To: pve-devel

Am 09.05.25 um 16:15 schrieb Fiona Ebner:
> Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
> ---
> 
> for me, it fails with
> VM 102 qmp command 'blockdev-add' failed - LUN is write protected
> but adding as a drive fails. Will need to investigate.

After fixing up my ACLs on the iSCSI server-side, attaching the block
device worked :)


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


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [pve-devel] [RFC qemu 1/1] block/rbd: add @keyring-file option to BlockdevOptionsRbd
  2025-05-12 11:25     ` Fiona Ebner
@ 2025-05-12 13:39       ` DERUMIER, Alexandre via pve-devel
       [not found]       ` <330ddb6da2469b425acda6ceb9cdaf5a510a854f.camel@groupe-cyllene.com>
  1 sibling, 0 replies; 26+ messages in thread
From: DERUMIER, Alexandre via pve-devel @ 2025-05-12 13:39 UTC (permalink / raw)
  To: pve-devel, f.ebner; +Cc: DERUMIER, Alexandre

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

From: "DERUMIER, Alexandre" <alexandre.derumier@groupe-cyllene.com>
To: "pve-devel@lists.proxmox.com" <pve-devel@lists.proxmox.com>, "f.ebner@proxmox.com" <f.ebner@proxmox.com>
Subject: Re: [RFC qemu 1/1] block/rbd: add @keyring-file option to BlockdevOptionsRbd
Date: Mon, 12 May 2025 13:39:27 +0000
Message-ID: <330ddb6da2469b425acda6ceb9cdaf5a510a854f.camel@groupe-cyllene.com>


Am 12.05.25 um 12:57 schrieb DERUMIER, Alexandre:
> for blockdev, do we still use a ceph config file in /var/run for
> potential others rbd client options ?

>>Not currently, but we can add that later if we consider it worth it.
>>We
>>would need to merge with the storage's already existing ceph.conf and
>>not only write the new options. For now, users can adapt their
>>storage's
>>ceph.conf as desired.

they still are this rbd_cache_policy for efidisk to fix
https://bugzilla.proxmox.com/show_bug.cgi?id=3329


# SPI flash does lots of read-modify-write OPs, without writeback this
gets really slow #3329
      if ($path =~ m/^rbd:/) {
          $var_drive_str .= ',cache=writeback';
          $path .= ':rbd_cache_policy=writeback'; # avoid write-around,
we *need* to cache writes too
      }



I'm not sure, but maybe it's fixed in qemu , the biggest problem was
that every single byte write was push to the storage without any buffer
(so it was pretty slow with rbd crush).
but maybe it ok now with:
https://github.com/qemu/qemu/commit/284a7ee2e290e0c9b8cd3ea6164d92386933054f

(I don't have tested it)

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

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [pve-devel] [RFC qemu 1/1] block/rbd: add @keyring-file option to BlockdevOptionsRbd
       [not found]       ` <330ddb6da2469b425acda6ceb9cdaf5a510a854f.camel@groupe-cyllene.com>
@ 2025-05-12 14:36         ` Fiona Ebner
  2025-05-12 14:53           ` DERUMIER, Alexandre via pve-devel
  0 siblings, 1 reply; 26+ messages in thread
From: Fiona Ebner @ 2025-05-12 14:36 UTC (permalink / raw)
  To: DERUMIER, Alexandre, pve-devel

Am 12.05.25 um 15:39 schrieb DERUMIER, Alexandre:
> Am 12.05.25 um 12:57 schrieb DERUMIER, Alexandre:
>> for blockdev, do we still use a ceph config file in /var/run for
>> potential others rbd client options ?
> 
>>> Not currently, but we can add that later if we consider it worth it.
>>> We
>>> would need to merge with the storage's already existing ceph.conf and
>>> not only write the new options. For now, users can adapt their
>>> storage's
>>> ceph.conf as desired.
> 
> they still are this rbd_cache_policy for efidisk to fix
> https://bugzilla.proxmox.com/show_bug.cgi?id=3329
> 
> 
> # SPI flash does lots of read-modify-write OPs, without writeback this
> gets really slow #3329
>       if ($path =~ m/^rbd:/) {
>           $var_drive_str .= ',cache=writeback';
>           $path .= ':rbd_cache_policy=writeback'; # avoid write-around,
> we *need* to cache writes too
>       }
> 
> 
> 
> I'm not sure, but maybe it's fixed in qemu , the biggest problem was
> that every single byte write was push to the storage without any buffer
> (so it was pretty slow with rbd crush).
> but maybe it ok now with:
> https://github.com/qemu/qemu/commit/284a7ee2e290e0c9b8cd3ea6164d92386933054f
> 
> (I don't have tested it)

Good point!

Unfortunately, it's still very slow without the additional options in
current QEMU 9.2 (i.e. even after that commit).

I suppose this does require us to have a per-drive configuration already.

It's not ideal that qemu-server knows about storage-internal details
though and would need to re-write the Ceph config, I might abstract that
away by passing an additional $hints parameter or something (e.g.
'writeback-cache' => 1, for EFI disk).

We do have a similar situation (but with KRBD):
https://lore.proxmox.com/pve-devel/20241025111304.99680-1-f.weber@proxmox.com/

Replying to stuff from your other mail here too:

> They are interesting rbd client option that we could add later
> https://bugzilla.proxmox.com/show_bug.cgi?id=6290
> crush_location=host:myhost|datacenter:mydc
> read_from_replica=localize

Those can/should simply be set in the storage's ceph.conf, or do they
need to be different per-volume or per-VM?


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


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [pve-devel] [RFC qemu 1/1] block/rbd: add @keyring-file option to BlockdevOptionsRbd
  2025-05-12 14:36         ` Fiona Ebner
@ 2025-05-12 14:53           ` DERUMIER, Alexandre via pve-devel
  0 siblings, 0 replies; 26+ messages in thread
From: DERUMIER, Alexandre via pve-devel @ 2025-05-12 14:53 UTC (permalink / raw)
  To: pve-devel, f.ebner; +Cc: DERUMIER, Alexandre

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

From: "DERUMIER, Alexandre" <alexandre.derumier@groupe-cyllene.com>
To: "pve-devel@lists.proxmox.com" <pve-devel@lists.proxmox.com>, "f.ebner@proxmox.com" <f.ebner@proxmox.com>
Subject: Re: [RFC qemu 1/1] block/rbd: add @keyring-file option to BlockdevOptionsRbd
Date: Mon, 12 May 2025 14:53:16 +0000
Message-ID: <a43d5425e7e942d20fa1d43a7ac17fbfe775a480.camel@groupe-cyllene.com>

> crush_location=host:myhost|datacenter:mydc
> read_from_replica=localize

>>Those can/should simply be set in the storage's ceph.conf, or do they
>>need to be different per-volume or per-VM?

read_from_replica=localize  ---> it's global for the whole storage in
ceph.cfg


crush_location=host:myhost|datacenter:mydc --->  it's per storage, but
need to be different for each host.  

(This is to tell to rbd client (the host) where he's running, to be
able to reach to nearest osd  when localize is used)


maybe check first in  /etc/ceph/<storeid.conf>,  then in
/etc/pve/ceph/storeid.cfg ?



Thinking about that, I don't have tested it yet, but maybe it's
possible with a single shared ceph.conf

[client.host1]
crush_location=host:host1|datacenter:dc1

[client.host2]
crush_location=host:host2|datacenter:dc2

?

I'll try to test it this week.









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

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [pve-devel] [RFC storage 1/3] plugin: add method to get qemu blockdevice options for volume
  2025-05-09 14:15 ` [pve-devel] [RFC storage 1/3] plugin: add method to get qemu blockdevice options for volume Fiona Ebner
@ 2025-05-23  8:19   ` DERUMIER, Alexandre via pve-devel
  2025-05-23  8:30   ` DERUMIER, Alexandre via pve-devel
                     ` (2 subsequent siblings)
  3 siblings, 0 replies; 26+ messages in thread
From: DERUMIER, Alexandre via pve-devel @ 2025-05-23  8:19 UTC (permalink / raw)
  To: pve-devel, f.ebner; +Cc: DERUMIER, Alexandre

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

From: "DERUMIER, Alexandre" <alexandre.derumier@groupe-cyllene.com>
To: "pve-devel@lists.proxmox.com" <pve-devel@lists.proxmox.com>, "f.ebner@proxmox.com" <f.ebner@proxmox.com>
Subject: Re: [pve-devel] [RFC storage 1/3] plugin: add method to get qemu blockdevice options for volume
Date: Fri, 23 May 2025 08:19:22 +0000
Message-ID: <eeb11ec08d36c3a6f5290134158e91ad7be8b432.camel@groupe-cyllene.com>

Hi Fiona,

do we still support glusterfs for pve9 (as it's deprecated)?

-------- Message initial --------
De: Fiona Ebner <f.ebner@proxmox.com>
Répondre à: Proxmox VE development discussion <pve-
devel@lists.proxmox.com>
À: pve-devel@lists.proxmox.com
Objet: [pve-devel] [RFC storage 1/3] plugin: add method to get qemu
blockdevice options for volume
Date: 09/05/2025 16:15:30

There intentionally is only handling for absolute paths in the default
plugin implementation. Any plugin requiring more needs to implement
the method itself.

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

I discussed this with Fabian off-list. With PVE 9 being a major
release and most popular plugins not using special protocols like
'rbd://', this seems acceptable. He'll also give a heads-up to known
plugin developers with other changes for PVE 9 in time.

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

Still missing API bump + Changelog

Did not test snapshots yet.

 src/PVE/Storage.pm        | 19 ++++++++++++
 src/PVE/Storage/Plugin.pm | 64 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 83 insertions(+)

diff --git a/src/PVE/Storage.pm b/src/PVE/Storage.pm
index d0a696a..5915395 100755
--- a/src/PVE/Storage.pm
+++ b/src/PVE/Storage.pm
@@ -710,6 +710,25 @@ sub abs_filesystem_path {
     return $path;
 }
 
+sub qemu_blockdev_options {
+    my ($cfg, $volid, $snapname) = @_;
+
+    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';
+
+    die "QEMU blockdevice - 'snapname' argument is not supported for
vtype '$vtype'"
+	if $snapname && $vtype ne 'images';
+
+    return $plugin->qemu_blockdev_options($scfg, $storeid, $volname,
$snapname);
+}
+
 # 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..dc3c6df 100644
--- a/src/PVE/Storage/Plugin.pm
+++ b/src/PVE/Storage/Plugin.pm
@@ -1880,6 +1880,70 @@ sub rename_volume {
     return "${storeid}:${base}${target_vmid}/${target_volname}";
 }
 
+=pod
+
+=head3 qemu_blockdev_options
+
+    $blockdev = $plugin->qemu_blockdev_options($scfg, $storeid,
$volname, $snapname)
+
+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://antiphishing.vadesecure.com/v4?f=UU1XcHkxazJBTmIySlBBMWR_zLc
v5a0_cIOSeWbIK2KapikXx8zXaPqWjlzjA2kV1Dznkr2dclNPHmvBavMw9A&i=RTNLd2NGe
E1RTDFrR25iaOm2xB-
k3c1aAk9xq9J3vLE&k=ywmE&r=ZnVkMm1UMHFmWHNzejI1TlffqTgRgYngUnr3SAFiUmsWi
qzGK6dBlW9tfep801MY&s=7964585ab390894467d9e21a57498f84ada52f1951c00b01c
c9facbdff1f8b32&u=https%3A%2F%2Fqemu.readthedocs.io%2Fen%2Fmaster%2Fint
erop%2Fqemu-qmp-ref.html%23object-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.
+
+=item C<$snapname>
+
+(optional) The snapshot name. Set when the associated snapshot should
be opened
+rather than the volume itself.
+
+=back
+
+=cut
+sub qemu_blockdev_options {
+    my ($class, $scfg, $storeid, $volname, $snapname) = @_;
+
+    my $blockdev = {};
+
+    my ($path) = $class->path($scfg, $volname, $storeid, $snapname);
+
+    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);
+	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.
 #

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

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [pve-devel] [RFC storage 1/3] plugin: add method to get qemu blockdevice options for volume
  2025-05-09 14:15 ` [pve-devel] [RFC storage 1/3] plugin: add method to get qemu blockdevice options for volume Fiona Ebner
  2025-05-23  8:19   ` DERUMIER, Alexandre via pve-devel
@ 2025-05-23  8:30   ` DERUMIER, Alexandre via pve-devel
       [not found]   ` <eeb11ec08d36c3a6f5290134158e91ad7be8b432.camel@groupe-cyllene.com>
       [not found]   ` <175dd76aa95365010c8448bdd15eddf30aa39641.camel@groupe-cyllene.com>
  3 siblings, 0 replies; 26+ messages in thread
From: DERUMIER, Alexandre via pve-devel @ 2025-05-23  8:30 UTC (permalink / raw)
  To: pve-devel, f.ebner; +Cc: DERUMIER, Alexandre

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

From: "DERUMIER, Alexandre" <alexandre.derumier@groupe-cyllene.com>
To: "pve-devel@lists.proxmox.com" <pve-devel@lists.proxmox.com>, "f.ebner@proxmox.com" <f.ebner@proxmox.com>
Subject: Re: [pve-devel] [RFC storage 1/3] plugin: add method to get qemu blockdevice options for volume
Date: Fri, 23 May 2025 08:30:41 +0000
Message-ID: <175dd76aa95365010c8448bdd15eddf30aa39641.camel@groupe-cyllene.com>

>>
>>    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);
>>+	my $driver = (S_ISCHR($st->mode) || S_ISBLK($st->mode)) ?
'host_device' : 'file';

This is breaking my qemu-server config unit tests because the file
don't exist.

Also, for lvm volume, they are not currently activate at vm command
line generation. (but anyway, I'll need it to retrieve backing chain,
so maybe it's not a problem)


Do you think it's enough safe to rely on /dev/.. path?



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

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [pve-devel] [RFC storage 1/3] plugin: add method to get qemu blockdevice options for volume
       [not found]   ` <eeb11ec08d36c3a6f5290134158e91ad7be8b432.camel@groupe-cyllene.com>
@ 2025-05-23  8:32     ` Fiona Ebner
  2025-05-23  8:42       ` DERUMIER, Alexandre via pve-devel
       [not found]       ` <2efc51be0c973a3055e8214beef06ea9a1c6583b.camel@groupe-cyllene.com>
  0 siblings, 2 replies; 26+ messages in thread
From: Fiona Ebner @ 2025-05-23  8:32 UTC (permalink / raw)
  To: DERUMIER, Alexandre, pve-devel

Hi Alexandre,

Am 23.05.25 um 10:19 schrieb DERUMIER, Alexandre:
> do we still support glusterfs for pve9 (as it's deprecated)?

no, the plan is to drop the storage plugin for it with Proxmox VE 9.
We'll warn about it in the pve8to9 upgrade script. QEMU is expected to
drop support with 10.1 in a few months. People that want to continue
using it, can still configure it as a shared directory storage.

Best Regards,
Fiona


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


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [pve-devel] [RFC storage 1/3] plugin: add method to get qemu blockdevice options for volume
       [not found]   ` <175dd76aa95365010c8448bdd15eddf30aa39641.camel@groupe-cyllene.com>
@ 2025-05-23  8:38     ` Fiona Ebner
  2025-05-23  8:50       ` DERUMIER, Alexandre via pve-devel
       [not found]       ` <67db7959a03a391df39e9b5af24edc2bed48a21d.camel@groupe-cyllene.com>
  0 siblings, 2 replies; 26+ messages in thread
From: Fiona Ebner @ 2025-05-23  8:38 UTC (permalink / raw)
  To: DERUMIER, Alexandre, pve-devel

Am 23.05.25 um 10:30 schrieb DERUMIER, Alexandre:
>>>
>>>     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);
>>> +	my $driver = (S_ISCHR($st->mode) || S_ISBLK($st->mode)) ?
> 'host_device' : 'file';
> 
> This is breaking my qemu-server config unit tests because the file
> don't exist.
> 
> Also, for lvm volume, they are not currently activate at vm command
> line generation. (but anyway, I'll need it to retrieve backing chain,
> so maybe it's not a problem)

Thanks for testing! I'll add an activate_volume() call in the
PVE::Storage::qemu_blockdev_options() function. I'd guess (almost?) all
calls of the function will be followed by actually using the device
afterwards, so that seems sensible enough.

> Do you think it's enough safe to rely on /dev/.. path?

Can you mock File::stat::stat() or
PVE::Storage::qemu_blockdev_options()? I'd rather not make the
production code less precise, just for easier tests if it can be avoided.

I'm expecting to send the next version of the series later today, hope
I'll get around to it :)

Best Regards,
Fiona


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

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [pve-devel] [RFC storage 1/3] plugin: add method to get qemu blockdevice options for volume
  2025-05-23  8:32     ` Fiona Ebner
@ 2025-05-23  8:42       ` DERUMIER, Alexandre via pve-devel
       [not found]       ` <2efc51be0c973a3055e8214beef06ea9a1c6583b.camel@groupe-cyllene.com>
  1 sibling, 0 replies; 26+ messages in thread
From: DERUMIER, Alexandre via pve-devel @ 2025-05-23  8:42 UTC (permalink / raw)
  To: pve-devel, f.ebner; +Cc: DERUMIER, Alexandre

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

From: "DERUMIER, Alexandre" <alexandre.derumier@groupe-cyllene.com>
To: "pve-devel@lists.proxmox.com" <pve-devel@lists.proxmox.com>, "f.ebner@proxmox.com" <f.ebner@proxmox.com>
Subject: Re: [pve-devel] [RFC storage 1/3] plugin: add method to get qemu blockdevice options for volume
Date: Fri, 23 May 2025 08:42:17 +0000
Message-ID: <2efc51be0c973a3055e8214beef06ea9a1c6583b.camel@groupe-cyllene.com>

>>no, the plan is to drop the storage plugin for it with Proxmox VE 9.
>>We'll warn about it in the pve8to9 upgrade script. QEMU is expected
>>to
>>drop support with 10.1 in a few months. People that want to continue
>>using it, can still configure it as a shared directory storage.

Ok, I wanted to know if we need to add qemu_blockdev_options in the
plugin.


BTW, it's missing qemu_blockdev_options for zfs over iscsi.  (it's the
same than iscsi direct)


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

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [pve-devel] [RFC storage 1/3] plugin: add method to get qemu blockdevice options for volume
       [not found]       ` <2efc51be0c973a3055e8214beef06ea9a1c6583b.camel@groupe-cyllene.com>
@ 2025-05-23  8:46         ` Fiona Ebner
  0 siblings, 0 replies; 26+ messages in thread
From: Fiona Ebner @ 2025-05-23  8:46 UTC (permalink / raw)
  To: DERUMIER, Alexandre, pve-devel

Am 23.05.25 um 10:42 schrieb DERUMIER, Alexandre:
>>> no, the plan is to drop the storage plugin for it with Proxmox VE 9.
>>> We'll warn about it in the pve8to9 upgrade script. QEMU is expected
>>> to
>>> drop support with 10.1 in a few months. People that want to continue
>>> using it, can still configure it as a shared directory storage.
> 
> Ok, I wanted to know if we need to add qemu_blockdev_options in the
> plugin.
> 
> 
> BTW, it's missing qemu_blockdev_options for zfs over iscsi.  (it's the
> same than iscsi direct)

Good catch! I didn't get around to test all plugins yet, so I missed this.


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


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [pve-devel] [RFC storage 1/3] plugin: add method to get qemu blockdevice options for volume
  2025-05-23  8:38     ` Fiona Ebner
@ 2025-05-23  8:50       ` DERUMIER, Alexandre via pve-devel
       [not found]       ` <67db7959a03a391df39e9b5af24edc2bed48a21d.camel@groupe-cyllene.com>
  1 sibling, 0 replies; 26+ messages in thread
From: DERUMIER, Alexandre via pve-devel @ 2025-05-23  8:50 UTC (permalink / raw)
  To: pve-devel, f.ebner; +Cc: DERUMIER, Alexandre

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

From: "DERUMIER, Alexandre" <alexandre.derumier@groupe-cyllene.com>
To: "pve-devel@lists.proxmox.com" <pve-devel@lists.proxmox.com>, "f.ebner@proxmox.com" <f.ebner@proxmox.com>
Subject: Re: [pve-devel] [RFC storage 1/3] plugin: add method to get qemu blockdevice options for volume
Date: Fri, 23 May 2025 08:50:50 +0000
Message-ID: <67db7959a03a391df39e9b5af24edc2bed48a21d.camel@groupe-cyllene.com>

> 
> Also, for lvm volume, they are not currently activate at vm command
> line generation. (but anyway, I'll need it to retrieve backing chain,
> so maybe it's not a problem)

>>Thanks for testing! I'll add an activate_volume() call in the
>>PVE::Storage::qemu_blockdev_options() function. 
>>I'd guess (almost?) >>all
>>calls of the function will be followed by actually using the device
>>afterwards, so that seems sensible enough.

ok. (I'll also verify in qemu-server to deactivate_volumes, if vm
command line generation is failing, as I think we only doing it in
vm_start currently)


> Do you think it's enough safe to rely on /dev/.. path?

>>Can you mock File::stat::stat() or
>>PVE::Storage::qemu_blockdev_options()? I'd rather not make the
>>production code less precise, just for easier tests if it can be
>>avoided.

sure,no problem!

(maybe also add a check if the file exist before calling st->mode,
because I got

# got unexpected error: Can't call method "mode" on an undefined value 



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

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [pve-devel] [RFC storage 1/3] plugin: add method to get qemu blockdevice options for volume
       [not found]       ` <67db7959a03a391df39e9b5af24edc2bed48a21d.camel@groupe-cyllene.com>
@ 2025-05-23  8:54         ` Fiona Ebner
  2025-05-23  9:15           ` DERUMIER, Alexandre via pve-devel
       [not found]           ` <abbb8159177112d0f1f44d1dccc8fc3907bccb73.camel@groupe-cyllene.com>
  0 siblings, 2 replies; 26+ messages in thread
From: Fiona Ebner @ 2025-05-23  8:54 UTC (permalink / raw)
  To: DERUMIER, Alexandre, pve-devel

Am 23.05.25 um 10:50 schrieb DERUMIER, Alexandre:
>>
>> Also, for lvm volume, they are not currently activate at vm command
>> line generation. (but anyway, I'll need it to retrieve backing chain,
>> so maybe it's not a problem)
> 
>>> Thanks for testing! I'll add an activate_volume() call in the
>>> PVE::Storage::qemu_blockdev_options() function. 
>>> I'd guess (almost?) >>all
>>> calls of the function will be followed by actually using the device
>>> afterwards, so that seems sensible enough.
> 
> ok. (I'll also verify in qemu-server to deactivate_volumes, if vm
> command line generation is failing, as I think we only doing it in
> vm_start currently)

Good point! I guess it's better to also do the activation in qemu-server
then, so that we can match it up nicely without going over package
boundaries.

>> Do you think it's enough safe to rely on /dev/.. path?
> 
>>> Can you mock File::stat::stat() or
>>> PVE::Storage::qemu_blockdev_options()? I'd rather not make the
>>> production code less precise, just for easier tests if it can be
>>> avoided.
> 
> sure,no problem!
> 
> (maybe also add a check if the file exist before calling st->mode,
> because I got
> 
> # got unexpected error: Can't call method "mode" on an undefined value 

Ack, will add a check that stat() worked :)

Best Regards,
Fiona


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

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [pve-devel] [RFC storage 1/3] plugin: add method to get qemu blockdevice options for volume
  2025-05-23  8:54         ` Fiona Ebner
@ 2025-05-23  9:15           ` DERUMIER, Alexandre via pve-devel
       [not found]           ` <abbb8159177112d0f1f44d1dccc8fc3907bccb73.camel@groupe-cyllene.com>
  1 sibling, 0 replies; 26+ messages in thread
From: DERUMIER, Alexandre via pve-devel @ 2025-05-23  9:15 UTC (permalink / raw)
  To: pve-devel, f.ebner; +Cc: DERUMIER, Alexandre

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

From: "DERUMIER, Alexandre" <alexandre.derumier@groupe-cyllene.com>
To: "pve-devel@lists.proxmox.com" <pve-devel@lists.proxmox.com>, "f.ebner@proxmox.com" <f.ebner@proxmox.com>
Subject: Re: [pve-devel] [RFC storage 1/3] plugin: add method to get qemu blockdevice options for volume
Date: Fri, 23 May 2025 09:15:43 +0000
Message-ID: <abbb8159177112d0f1f44d1dccc8fc3907bccb73.camel@groupe-cyllene.com>

>>Good point! I guess it's better to also do the activation in qemu-
>>server
>>then, so that we can match it up nicely without going over package
>>boundaries.

Ok, I'll add the activate in qemu-server too



Just found another bug:

PVE/Storage.pm 

sub qemu_blockdev_options {
    my ($cfg, $volid, $snapname) = @_;

    my ($storeid, $volname) = parse_volume_id($volid);


This fail with $volid=/dev/cdrom   (with ide2:cdrom)
# got unexpected error: unable to parse volume ID 'cdrom'

and maybe (not have tested it) , if user have configured manually dev
passthrough like : "scsi1: /dev/...."





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

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [pve-devel] [RFC storage 1/3] plugin: add method to get qemu blockdevice options for volume
       [not found]           ` <abbb8159177112d0f1f44d1dccc8fc3907bccb73.camel@groupe-cyllene.com>
@ 2025-05-23  9:18             ` Fiona Ebner
  2025-05-23  9:23               ` DERUMIER, Alexandre via pve-devel
                                 ` (2 more replies)
  0 siblings, 3 replies; 26+ messages in thread
From: Fiona Ebner @ 2025-05-23  9:18 UTC (permalink / raw)
  To: DERUMIER, Alexandre, pve-devel

Am 23.05.25 um 11:15 schrieb DERUMIER, Alexandre:
>>> Good point! I guess it's better to also do the activation in qemu-
>>> server
>>> then, so that we can match it up nicely without going over package
>>> boundaries.
> 
> Ok, I'll add the activate in qemu-server too
> 
> 
> 
> Just found another bug:
> 
> PVE/Storage.pm 
> 
> sub qemu_blockdev_options {
>     my ($cfg, $volid, $snapname) = @_;
> 
>     my ($storeid, $volname) = parse_volume_id($volid);
> 
> 
> This fail with $volid=/dev/cdrom   (with ide2:cdrom)
> # got unexpected error: unable to parse volume ID 'cdrom'
> 
> and maybe (not have tested it) , if user have configured manually dev
> passthrough like : "scsi1: /dev/...."

I intentionally do not handle CD-ROMs, qemu-server should be concerned
with doing that. There is a comment about this ;)

> +	# 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.

Best Regards,
Fiona


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


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [pve-devel] [RFC storage 1/3] plugin: add method to get qemu blockdevice options for volume
  2025-05-23  9:18             ` Fiona Ebner
@ 2025-05-23  9:23               ` DERUMIER, Alexandre via pve-devel
  2025-05-23  9:34               ` DERUMIER, Alexandre via pve-devel
       [not found]               ` <abebd4ee7f1197d9e549203355c9482bd7b1004a.camel@groupe-cyllene.com>
  2 siblings, 0 replies; 26+ messages in thread
From: DERUMIER, Alexandre via pve-devel @ 2025-05-23  9:23 UTC (permalink / raw)
  To: pve-devel, f.ebner; +Cc: DERUMIER, Alexandre

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

From: "DERUMIER, Alexandre" <alexandre.derumier@groupe-cyllene.com>
To: "pve-devel@lists.proxmox.com" <pve-devel@lists.proxmox.com>, "f.ebner@proxmox.com" <f.ebner@proxmox.com>
Subject: Re: [pve-devel] [RFC storage 1/3] plugin: add method to get qemu blockdevice options for volume
Date: Fri, 23 May 2025 09:23:03 +0000
Message-ID: <c0af28f7e3e00a9a0ec8fad9654e82a113bac6f3.camel@groupe-cyllene.com>

>>
>>I intentionally do not handle CD-ROMs, qemu-server should be
>>concerned
>>with doing that. There is a comment about this ;)

ok. I think this isœ in general when no "storeid:volid"  is defined
right ?  could be a device or a file  directly configured in the vm
configuration too  

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

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [pve-devel] [RFC storage 1/3] plugin: add method to get qemu blockdevice options for volume
  2025-05-23  9:18             ` Fiona Ebner
  2025-05-23  9:23               ` DERUMIER, Alexandre via pve-devel
@ 2025-05-23  9:34               ` DERUMIER, Alexandre via pve-devel
       [not found]               ` <abebd4ee7f1197d9e549203355c9482bd7b1004a.camel@groupe-cyllene.com>
  2 siblings, 0 replies; 26+ messages in thread
From: DERUMIER, Alexandre via pve-devel @ 2025-05-23  9:34 UTC (permalink / raw)
  To: pve-devel, f.ebner; +Cc: DERUMIER, Alexandre

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

From: "DERUMIER, Alexandre" <alexandre.derumier@groupe-cyllene.com>
To: "pve-devel@lists.proxmox.com" <pve-devel@lists.proxmox.com>, "f.ebner@proxmox.com" <f.ebner@proxmox.com>
Subject: Re: [pve-devel] [RFC storage 1/3] plugin: add method to get qemu blockdevice options for volume
Date: Fri, 23 May 2025 09:34:37 +0000
Message-ID: <abebd4ee7f1197d9e549203355c9482bd7b1004a.camel@groupe-cyllene.com>

>>I intentionally do not handle CD-ROMs, qemu-server should be
>>concerned
>>with doing that. There is a comment about this ;)

I mean, could it better to have something like this ? :


sub qemu_blockdev_options {
    my ($cfg, $volid, $snapname) = @_;

    my ($storeid, $volname) = parse_volume_id($volid, 1);

    if($storeid) {
        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';

        die "QEMU blockdevice - 'snapname' argument is not supported
for vtype '$vtype'"
            if $snapname && $vtype ne 'images';
    } elsif ($volid =~ m|^/|) {
        my $st = File::stat::stat($volid);
        my $driver = (S_ISCHR($st->mode) || S_ISBLK($st->mode)) ?
'host_device' : 'file';
        return { driver => $driver, filename => $volid };
    }

    return $plugin->qemu_blockdev_options($scfg, $storeid, $volname,
$snapname);
}

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

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [pve-devel] [RFC storage 1/3] plugin: add method to get qemu blockdevice options for volume
       [not found]               ` <abebd4ee7f1197d9e549203355c9482bd7b1004a.camel@groupe-cyllene.com>
@ 2025-05-23  9:53                 ` Fiona Ebner
  2025-05-23 10:30                   ` DERUMIER, Alexandre via pve-devel
  0 siblings, 1 reply; 26+ messages in thread
From: Fiona Ebner @ 2025-05-23  9:53 UTC (permalink / raw)
  To: DERUMIER, Alexandre, pve-devel

Am 23.05.25 um 11:34 schrieb DERUMIER, Alexandre:
>>> I intentionally do not handle CD-ROMs, qemu-server should be
>>> concerned
>>> with doing that. There is a comment about this ;)
> 
> I mean, could it better to have something like this ? :
> 
> 
> sub qemu_blockdev_options {
>     my ($cfg, $volid, $snapname) = @_;
> 
>     my ($storeid, $volname) = parse_volume_id($volid, 1);
> 
>     if($storeid) {
>         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';
> 
>         die "QEMU blockdevice - 'snapname' argument is not supported
> for vtype '$vtype'"
>             if $snapname && $vtype ne 'images';
>     } elsif ($volid =~ m|^/|) {
>         my $st = File::stat::stat($volid);
>         my $driver = (S_ISCHR($st->mode) || S_ISBLK($st->mode)) ?
> 'host_device' : 'file';
>         return { driver => $driver, filename => $volid };
>     }
> 
>     return $plugin->qemu_blockdev_options($scfg, $storeid, $volname,
> $snapname);
> }

Yes, it would be possible, and it is a close call. But I briefly chatted
with Fabian off-list and we think it's better to do this in qemu-server,
together with the CD-ROM handling. Since the whole use-case is related
to a QEMU-specific interface already.


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


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [pve-devel] [RFC storage 1/3] plugin: add method to get qemu blockdevice options for volume
  2025-05-23  9:53                 ` Fiona Ebner
@ 2025-05-23 10:30                   ` DERUMIER, Alexandre via pve-devel
  0 siblings, 0 replies; 26+ messages in thread
From: DERUMIER, Alexandre via pve-devel @ 2025-05-23 10:30 UTC (permalink / raw)
  To: pve-devel, f.ebner; +Cc: DERUMIER, Alexandre

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

From: "DERUMIER, Alexandre" <alexandre.derumier@groupe-cyllene.com>
To: "pve-devel@lists.proxmox.com" <pve-devel@lists.proxmox.com>, "f.ebner@proxmox.com" <f.ebner@proxmox.com>
Subject: Re: [pve-devel] [RFC storage 1/3] plugin: add method to get qemu blockdevice options for volume
Date: Fri, 23 May 2025 10:30:48 +0000
Message-ID: <7f0f072b3040d29d21dc06032b496033ba1b19a3.camel@groupe-cyllene.com>

>>Yes, it would be possible, and it is a close call. But I briefly
>>chatted
>>with Fabian off-list and we think it's better to do this in qemu-
>>server,
>>together with the CD-ROM handling. Since the whole use-case is
>>related
>>to a QEMU-specific interface already.

Ok, thanks !


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

^ permalink raw reply	[flat|nested] 26+ messages in thread

end of thread, other threads:[~2025-05-23 10:30 UTC | newest]

Thread overview: 26+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-05-09 14:15 [pve-devel] [RFC qemu/pve-storage] storage plugin method to get qemu blockdevice options for volume Fiona Ebner
2025-05-09 14:15 ` [pve-devel] [RFC qemu 1/1] block/rbd: add @keyring-file option to BlockdevOptionsRbd Fiona Ebner
2025-05-12 10:57   ` DERUMIER, Alexandre via pve-devel
     [not found]   ` <dfc78aa17b9c1c8496fa74cb6e6d2517337b65c0.camel@groupe-cyllene.com>
2025-05-12 11:25     ` Fiona Ebner
2025-05-12 13:39       ` DERUMIER, Alexandre via pve-devel
     [not found]       ` <330ddb6da2469b425acda6ceb9cdaf5a510a854f.camel@groupe-cyllene.com>
2025-05-12 14:36         ` Fiona Ebner
2025-05-12 14:53           ` DERUMIER, Alexandre via pve-devel
2025-05-09 14:15 ` [pve-devel] [RFC storage 1/3] plugin: add method to get qemu blockdevice options for volume Fiona Ebner
2025-05-23  8:19   ` DERUMIER, Alexandre via pve-devel
2025-05-23  8:30   ` DERUMIER, Alexandre via pve-devel
     [not found]   ` <eeb11ec08d36c3a6f5290134158e91ad7be8b432.camel@groupe-cyllene.com>
2025-05-23  8:32     ` Fiona Ebner
2025-05-23  8:42       ` DERUMIER, Alexandre via pve-devel
     [not found]       ` <2efc51be0c973a3055e8214beef06ea9a1c6583b.camel@groupe-cyllene.com>
2025-05-23  8:46         ` Fiona Ebner
     [not found]   ` <175dd76aa95365010c8448bdd15eddf30aa39641.camel@groupe-cyllene.com>
2025-05-23  8:38     ` Fiona Ebner
2025-05-23  8:50       ` DERUMIER, Alexandre via pve-devel
     [not found]       ` <67db7959a03a391df39e9b5af24edc2bed48a21d.camel@groupe-cyllene.com>
2025-05-23  8:54         ` Fiona Ebner
2025-05-23  9:15           ` DERUMIER, Alexandre via pve-devel
     [not found]           ` <abbb8159177112d0f1f44d1dccc8fc3907bccb73.camel@groupe-cyllene.com>
2025-05-23  9:18             ` Fiona Ebner
2025-05-23  9:23               ` DERUMIER, Alexandre via pve-devel
2025-05-23  9:34               ` DERUMIER, Alexandre via pve-devel
     [not found]               ` <abebd4ee7f1197d9e549203355c9482bd7b1004a.camel@groupe-cyllene.com>
2025-05-23  9:53                 ` Fiona Ebner
2025-05-23 10:30                   ` DERUMIER, Alexandre via pve-devel
2025-05-09 14:15 ` [pve-devel] [RFC storage 2/3] iscsi direct plugin: implement method to get qemu blockdevice options Fiona Ebner
2025-05-12 13:14   ` Fiona Ebner
2025-05-09 14:15 ` [pve-devel] [RFC storage 3/3] rbd plugin: implement new " Fiona Ebner
2025-05-09 14:21 ` [pve-devel] [RFC qemu/pve-storage] storage plugin method to get qemu blockdevice options for volume Fiona Ebner

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal