From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [IPv6:2a01:7e0:0:424::9]) by lore.proxmox.com (Postfix) with ESMTPS id CFE741FF165 for ; Thu, 3 Jul 2025 11:33:00 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id F0590F386; Thu, 3 Jul 2025 11:33:38 +0200 (CEST) MIME-Version: 1.0 In-Reply-To: <20250702162838.393696-6-f.ebner@proxmox.com> References: <20250702162838.393696-1-f.ebner@proxmox.com> <20250702162838.393696-6-f.ebner@proxmox.com> From: Fabian =?utf-8?q?Gr=C3=BCnbichler?= To: Fiona Ebner , pve-devel@lists.proxmox.com Date: Thu, 03 Jul 2025 11:33:29 +0200 Message-ID: <175153520907.199248.10506541694985902664@yuna.proxmox.com> User-Agent: alot/0.10 X-SPAM-LEVEL: Spam detection results: 0 AWL -0.075 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment RCVD_IN_VALIDITY_CERTIFIED_BLOCKED 0.237 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. RCVD_IN_VALIDITY_RPBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. RCVD_IN_VALIDITY_SAFE_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record URIBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to URIBL was blocked. See http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more information. [readthedocs.io, proxmox.com, plugin.pm, storage.pm] Subject: Re: [pve-devel] [PATCH storage v5 05/51] plugin: add method to get qemu blockdevice options for volume X-BeenThere: pve-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox VE development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: Proxmox VE development discussion Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: pve-devel-bounces@lists.proxmox.com Sender: "pve-devel" 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 > --- > 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 driver. For files, the C driver can be used. For > +host block devices, the C driver can be used. The plugin must not set options like > +C or C. Those are managed by qemu-server and will be overwritten. For other available > +drivers and the exact specification of the options, see > +L > + > +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 and C for booleans, C<"$value"> for strings, and C 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.astroid@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