public inbox for pve-devel@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 qemu-server 1/2] migration: alloc nbd disks: base format hint off source storage
Date: Mon, 17 Jul 2023 16:00:19 +0200	[thread overview]
Message-ID: <20230717140020.74568-2-f.ebner@proxmox.com> (raw)
In-Reply-To: <20230717140020.74568-1-f.ebner@proxmox.com>

Previously, qemu_img_format() was called with the target storage's
$scfg and the source storage's volume name.

This mismatch should only be relevant for certain special kinds of
storage plugins:
- no path, but does support an additional QEMU image format besides
  'raw', in short NPAF.
- no path, volume name can match QEMU_FORMAT_RE, in short NPVM.

Note that all integrated plugins are neither NPAF nor NPVM.

Note that for NPAF plugins, qemu_img_format() already always returns
'raw' because there is no path. It's a bit unlikely such a plugin
exists, because there were no bug reports about qemu_img_format()
misbehaving there yet.

Let's go through the cases:
- If source and target storage both have or don't have a path,
  qemu_img_format($scfg, $volname) returns the same for both $scfg's.
- If source storage has a path, but target storage does not, the
  format hint was previously 'raw', but can only be more correct now
  (being what the source image actually is):
  - For non-NPAF targets, since we know there is no path, it follows
    that 'raw' is the only supported QEMU image format.
  - For NPAF targets, the format will be preserved now (if actually
    supported).
- If source storage does not have a path, but target storage does, the
  format hint will be 'raw' now.
  - For non-NPVM sources, QEMU_FORMAT_RE didn't match when
    qemu_img_format() was called with the target storage's $scfg, so
    the hint also was 'raw' before this commit.
  - For NPVM sources, qemu_img_format() might've guessed a format from
    the source volume name when called with the target's $scfg before
    this commit. If the target storage supports the previously guessed
    format, it was preserved before this commit, but will not be
    anymore. In theory, the guess might've also been wrong, and in
    this case, this commit avoids the wrong guess.

To summarize, there is only one edge case with an exotic kind of third
party storage plugin where format preservation would be lost and in
another edge case, format preservation is gained.

In preparation to simplify the format fallback logic implementation.

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
 PVE/QemuServer.pm | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
index 430661a7..a692e32e 100644
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -5583,12 +5583,12 @@ sub vm_migrate_alloc_nbd_disks {
 	# 2. format of current volume
 	# 3. default format of storage
 	if (!$storagemap->{identity}) {
+	    my $source_scfg = PVE::Storage::storage_config($storecfg, $storeid);
 	    $storeid = PVE::JSONSchema::map_id($storagemap, $storeid);
 	    my ($defFormat, $validFormats) = PVE::Storage::storage_default_format($storecfg, $storeid);
 	    if (!$format || !grep { $format eq $_ } @$validFormats) {
 		if ($volname) {
-		    my $scfg = PVE::Storage::storage_config($storecfg, $storeid);
-		    my $fileFormat = qemu_img_format($scfg, $volname);
+		    my $fileFormat = qemu_img_format($source_scfg, $volname);
 		    $format = $fileFormat
 			if grep { $fileFormat eq $_ } @$validFormats;
 		}
-- 
2.39.2





  reply	other threads:[~2023-07-17 14:00 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-07-17 14:00 [pve-devel] [PATCH-SERIES qemu-server] migration: nbd alloc: improve format fallback Fiona Ebner
2023-07-17 14:00 ` Fiona Ebner [this message]
2023-07-17 14:00 ` [pve-devel] [PATCH qemu-server 2/2] migration: alloc nbd disks: fix fall-back for remote live migration Fiona Ebner
2023-07-27  8:29 ` [pve-devel] applied: [PATCH-SERIES qemu-server] migration: nbd alloc: improve format fallback Fabian Grünbichler

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=20230717140020.74568-2-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 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