Quoting Fiona Ebner (2025-07-02 18:27:38)
> 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.eb...@proxmox.com>
> ---
> src/PVE/Storage.pm | 17 ++++++++++++
> src/PVE/Storage/Plugin.pm | 56 +++++++++++++++++++++++++++++++++++++++
> 2 files changed, 73 insertions(+)
>
> diff --git a/src/PVE/Storage.pm b/src/PVE/Storage.pm
> index 69eb435..ec8b753 100755
> --- a/src/PVE/Storage.pm
> +++ b/src/PVE/Storage.pm
> @@ -719,6 +719,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 53b9848..680bb6b 100644
> --- a/src/PVE/Storage/Plugin.pm
> +++ b/src/PVE/Storage/Plugin.pm
> @@ -1961,6 +1961,62 @@ 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.
> +
> +The volume is activated before the function is called.
> +
> +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);
sorry for missing this in the earlier revisions - but I think(!) we should call
$class->path() here, as that is the plugin API method?
see https://lore.proxmox.com/pve-devel/1743427728.0lo886zlbq.astr...@yuna.none/
for our plugin hierarchy it should make no practical difference, but an
external plugin might just have overriden `path` (as that is what gets called
via PVE::Storage)..
but if you look at this part of the series, it is quite obvious (all *our*
plugins that require overriding qemu_blockdev_options also provide their own
path, but not their own filesystem_path)
> +
> + 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.47.2
>
>
>
> _______________________________________________
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
>
>
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
