all lists on lists.proxmox.com
 help / color / mirror / Atom feed
From: Fiona Ebner <f.ebner@proxmox.com>
To: pve-devel@lists.proxmox.com
Subject: [pve-devel] [PATCH v2 storage 3/5] plugin: volume export formats: avoid superfluous file_size_info() call
Date: Mon,  3 Mar 2025 10:24:43 +0100	[thread overview]
Message-ID: <20250303092445.13873-4-f.ebner@proxmox.com> (raw)
In-Reply-To: <20250303092445.13873-1-f.ebner@proxmox.com>

The result from the file_size_info() call is not used by
volume_export_formats() and most failure scenarios of file_size_info()
lead to an undefined return value rather than a failure. This includes
the case for a non-existent file. The default path() implementation
doesn't do any existence check either.

An interesting scenario where file_size_info() does fail, is when the
volume is corrupted or not in the queried format. But this is a rare
edge case, so an early check doesn't seem worth it. It will be caught
by volume_export() itself, or in case of VM migration, also when
querying the size during scanning of local volumes.

While checking for the definedness of $size could serve as an early
sanity check, it is not currently done and other plugins don't do such
early checks in their implementation of volume_export_formats()
either. Keep the implementation abstract in Plugin.pm too and avoid
doing IO. Callers that want to do early existence checks or similar
can do so themselves explicitly, covering all plugins.

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
 src/PVE/Storage/Plugin.pm | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/src/PVE/Storage/Plugin.pm b/src/PVE/Storage/Plugin.pm
index ce1530e..ca70c03 100644
--- a/src/PVE/Storage/Plugin.pm
+++ b/src/PVE/Storage/Plugin.pm
@@ -1733,10 +1733,7 @@ sub volume_export {
 sub volume_export_formats {
     my ($class, $scfg, $storeid, $volname, $snapshot, $base_snapshot, $with_snapshots) = @_;
     if ($scfg->{path} && !defined($snapshot) && !defined($base_snapshot)) {
-	my ($file) = $class->path($scfg, $volname, $storeid)
-	    or return;
 	my $format = ($class->parse_volname($volname))[6];
-	my $size = file_size_info($file, undef, $format);
 
 	if ($with_snapshots) {
 	    return ($format.'+size') if ($format eq 'qcow2' || $format eq 'vmdk');
-- 
2.39.5



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


  parent reply	other threads:[~2025-03-03  9:25 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-03-03  9:24 [pve-devel] [PATCH-SERIES v2 storage 0/5] unsized subvol regression fix and improvements Fiona Ebner
2025-03-03  9:24 ` [pve-devel] [PATCH v2 storage 1/5] plugin: file size info: be consistent about size of directory subvol Fiona Ebner
2025-03-03  9:24 ` [pve-devel] [PATCH v2 storage 2/5] btrfs: fix volume size info for subvolumes in scalar context Fiona Ebner
2025-03-03  9:24 ` Fiona Ebner [this message]
2025-03-03  9:24 ` [pve-devel] [PATCH v2 storage 4/5] api: volume info: do not fail for zero-sized subvolumes Fiona Ebner
2025-03-03  9:24 ` [pve-devel] [PATCH v2 storage 5/5] btrfs: die early for broken subvolume resize Fiona Ebner
2025-04-06 19:59 ` [pve-devel] applied: [PATCH-SERIES v2 storage 0/5] unsized subvol regression fix and improvements Thomas Lamprecht

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20250303092445.13873-4-f.ebner@proxmox.com \
    --to=f.ebner@proxmox.com \
    --cc=pve-devel@lists.proxmox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal