From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <pve-devel-bounces@lists.proxmox.com>
Received: from firstgate.proxmox.com (firstgate.proxmox.com [IPv6:2a01:7e0:0:424::9])
	by lore.proxmox.com (Postfix) with ESMTPS id B95E91FF170
	for <inbox@lore.proxmox.com>; Thu, 12 Jun 2025 16:04:37 +0200 (CEST)
Received: from firstgate.proxmox.com (localhost [127.0.0.1])
	by firstgate.proxmox.com (Proxmox) with ESMTP id 44FAC1F5D8;
	Thu, 12 Jun 2025 16:03:13 +0200 (CEST)
From: Fiona Ebner <f.ebner@proxmox.com>
To: pve-devel@lists.proxmox.com
Date: Thu, 12 Jun 2025 16:02:51 +0200
Message-Id: <20250612140253.106555-21-f.ebner@proxmox.com>
X-Mailer: git-send-email 2.39.5
In-Reply-To: <20250612140253.106555-1-f.ebner@proxmox.com>
References: <20250612140253.106555-1-f.ebner@proxmox.com>
MIME-Version: 1.0
X-SPAM-LEVEL: Spam detection results:  0
 AWL -0.032 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
 SPF_HELO_NONE           0.001 SPF: HELO does not publish an SPF Record
 SPF_PASS               -0.001 SPF: sender matches SPF record
Subject: [pve-devel] [PATCH qemu-server 20/22] blockdev: add helpers to
 generate blockdev commandline
X-BeenThere: pve-devel@lists.proxmox.com
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: Proxmox VE development discussion <pve-devel.lists.proxmox.com>
List-Unsubscribe: <https://lists.proxmox.com/cgi-bin/mailman/options/pve-devel>, 
 <mailto:pve-devel-request@lists.proxmox.com?subject=unsubscribe>
List-Archive: <http://lists.proxmox.com/pipermail/pve-devel/>
List-Post: <mailto:pve-devel@lists.proxmox.com>
List-Help: <mailto:pve-devel-request@lists.proxmox.com?subject=help>
List-Subscribe: <https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel>, 
 <mailto:pve-devel-request@lists.proxmox.com?subject=subscribe>
Reply-To: Proxmox VE development discussion <pve-devel@lists.proxmox.com>
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Errors-To: pve-devel-bounces@lists.proxmox.com
Sender: "pve-devel" <pve-devel-bounces@lists.proxmox.com>

The drive device and node structure is:
front-end device {ide-hd,scsi-hd,virtio-blk-pci} (id=$drive_id)
 - throttle node (node-name=$drive_id)
  - format node  (node-name=f$encoded-info)
   - file node   (node-name=e$encoded-info)

The node-name can only be 31 characters long and needs to start with a
letter. The throttle node will stay inserted below the front-end
device. The other nodes might change however, because of drive
mirroring and similar. There currently are no good helpers to
query/walk the block graph via QMP, x-debug-query-block-graph is
experimental and for debugging only. Therefore, necessary information
is encoded in the node name to be able to find it again. In
particular, this is the volume ID, the drive ID and optionally a
snapshot name. As long as the configuration file matches with the
running instance, this is enough to find the correct node for
block operations like mirror and resize.

The 'snapshot' option, for QEMU's snapshot mode, i.e. writes are only
temporary, is not yet supported.

Originally-by: Alexandre Derumier <alexandre.derumier@groupe-cyllene.com>
[FE: split up patch
     expand commit message
     explicitly test for drivers with aio setting
     improve readonly handling
     improve CD-ROM handling
     fix failure for storage named 'nbd' by always using full regex
     improve node name generation
     fail when drive->{snapshot} is set]
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---

If we do not get around to implement the 'snapshot' option in time for
PVE 9, we can still fall back to the legacy drive for that (and warn
users that not all operations might work with such drives).

 PVE/QemuServer/Blockdev.pm | 172 ++++++++++++++++++++++++++++++++++++-
 PVE/QemuServer/Makefile    |   1 +
 2 files changed, 172 insertions(+), 1 deletion(-)

diff --git a/PVE/QemuServer/Blockdev.pm b/PVE/QemuServer/Blockdev.pm
index b1150141..2d3760f0 100644
--- a/PVE/QemuServer/Blockdev.pm
+++ b/PVE/QemuServer/Blockdev.pm
@@ -3,7 +3,42 @@ package PVE::QemuServer::Blockdev;
 use strict;
 use warnings;
 
-use PVE::QemuServer::Drive;
+use Digest::SHA;
+use Fcntl qw(S_ISBLK S_ISCHR);
+use File::stat;
+use JSON;
+
+use PVE::Storage;
+
+use PVE::QemuServer::Drive qw(drive_is_cdrom);
+
+my sub get_node_name {
+    my ($type, $drive_id, $volid, $snap) = @_;
+
+    my $info = "drive=$drive_id,";
+    $info .= "snap=$snap," if defined($snap);
+    $info .= "volid=$volid";
+
+    my $hash = substr(Digest::SHA::sha256_hex($info), 0, 30);
+
+    my $prefix = "";
+    if ($type eq 'fmt') {
+	$prefix = 'f';
+    } elsif ($type eq 'file') {
+	$prefix = 'e';
+    } else {
+	die "unknown node type '$type'";
+    }
+    # node-name must start with an alphabetical character
+    return "${prefix}${hash}";
+}
+
+my sub read_only_json_option {
+    my ($drive, $options) = @_;
+
+    return JSON::true if $drive->{ro} || drive_is_cdrom($drive) || $options->{'read-only'};
+    return JSON::false;
+}
 
 sub generate_throttle_group {
     my ($drive) = @_;
@@ -41,4 +76,139 @@ sub generate_throttle_group {
     };
 }
 
+sub generate_blockdev_drive_cache {
+    my ($drive, $scfg) = @_;
+
+    my $cache_direct = PVE::QemuServer::Drive::drive_uses_cache_direct($drive, $scfg);
+    my $cache = {};
+    $cache->{direct} = $cache_direct ? JSON::true : JSON::false;
+    $cache->{'no-flush'} = $drive->{cache} && $drive->{cache} eq 'unsafe' ? JSON::true : JSON::false;
+    return $cache;
+}
+
+sub generate_file_blockdev {
+    my ($storecfg, $drive, $options) = @_;
+
+    my $blockdev = {};
+    my $scfg = undef;
+
+    die "generate_file_blockdev called without volid/path\n" if !$drive->{file};
+    die "generate_file_blockdev called with 'none'\n" if $drive->{file} eq 'none';
+    # FIXME use overlay and new config option to define storage for temp write device
+    die "'snapshot' option is not yet supported for '-blockdev'\n" if $drive->{snapshot};
+
+    my $drive_id = PVE::QemuServer::Drive::get_drive_id($drive);
+
+    if ($drive->{file} eq 'cdrom') {
+	my $path = PVE::QemuServer::Drive::get_iso_path($storecfg, $drive->{file});
+	$blockdev = { driver => 'host_cdrom', filename => $path };
+    } elsif ($drive->{file} =~ m|^/|) {
+	my $path = $drive->{file};
+	# The 'file' driver only works for regular files. The check below is taken from
+	# block/file-posix.c:hdev_probe_device() in QEMU. To detect CD-ROM host devices, QEMU issues
+	# an ioctl, while the code here relies on the media=cdrom flag instead.
+	my $st = File::stat::stat($path) or die "stat for '$path' failed - $!\n";
+	my $driver = 'file';
+	if (S_ISCHR($st->mode) || S_ISBLK($st->mode)) {
+	    $driver = drive_is_cdrom($drive) ? 'host_cdrom' : 'host_device';
+	}
+	$blockdev = { driver => $driver, filename => $path };
+    } else {
+	my $volid = $drive->{file};
+	my ($storeid) = PVE::Storage::parse_volume_id($volid);
+
+	my $vtype = (PVE::Storage::parse_volname($storecfg, $drive->{file}))[0];
+	die "$drive_id: explicit media parameter is required for iso images\n"
+	    if !defined($drive->{media}) && defined($vtype) && $vtype eq 'iso';
+
+	my $storage_opts = { hints => {} };
+	$storage_opts->{hints}->{'efi-disk'} = 1 if $drive->{interface} eq 'efidisk';
+	$storage_opts->{'snapshot-name'} = $options->{'snapshot-name'}
+	    if defined($options->{'snapshot-name'});
+	$blockdev = PVE::Storage::qemu_blockdev_options($storecfg, $volid, $storage_opts);
+	$scfg = PVE::Storage::storage_config($storecfg, $storeid);
+    }
+
+    $blockdev->{cache} = generate_blockdev_drive_cache($drive, $scfg);
+
+    my $driver = $blockdev->{driver};
+    # only certain drivers have the aio setting
+    if ($driver eq 'file' || $driver eq 'host_cdrom' || $driver eq 'host_device') {
+	$blockdev->{aio} = PVE::QemuServer::Drive::aio_cmdline_option(
+	    $scfg, $drive, $blockdev->{cache}->{direct});
+    }
+
+    if (!drive_is_cdrom($drive)) {
+	$blockdev->{discard} = $drive->{discard} && $drive->{discard} eq 'on' ? 'unmap' : 'ignore';
+	$blockdev->{'detect-zeroes'} = PVE::QemuServer::Drive::detect_zeroes_cmdline_option($drive);
+    }
+
+    $blockdev->{'node-name'} = get_node_name(
+	'file', $drive_id, $drive->{file}, $options->{'snapshot-name'});
+
+    $blockdev->{'read-only'} = read_only_json_option($drive, $options);
+
+    return $blockdev;
+}
+
+sub generate_format_blockdev {
+    my ($storecfg, $drive, $child, $options) = @_;
+
+    die "generate_format_blockdev called without volid/path\n" if !$drive->{file};
+    die "generate_format_blockdev called with 'none'\n" if $drive->{file} eq 'none';
+
+    my $scfg;
+    my $format;
+    my $volid = $drive->{file};
+    my $drive_id = PVE::QemuServer::Drive::get_drive_id($drive);
+    my ($storeid) = PVE::Storage::parse_volume_id($volid, 1);
+
+    # For PVE-managed volumes, use the format from the storage layer and prevent overrides via the
+    # drive's 'format' option. For unmanaged volumes, fallback to 'raw' to avoid auto-detection by
+    # QEMU.
+    if ($storeid) {
+	$scfg = PVE::Storage::storage_config($storecfg, $storeid);
+	$format = PVE::QemuServer::Drive::checked_volume_format($storecfg, $volid);
+	if ($drive->{format} && $drive->{format} ne $format) {
+	    die "drive '$drive->{interface}$drive->{index}' - volume '$volid'"
+		." - 'format=$drive->{format}' option different from storage format '$format'\n";
+	}
+    } else {
+	$format = $drive->{format} // 'raw';
+    }
+
+    # define cache option on both format && file node like libvirt does
+    my $cache = generate_blockdev_drive_cache($drive, $scfg);
+
+    my $node_name = get_node_name('fmt', $drive_id, $drive->{file}, $options->{'snapshot-name'});
+
+    return {
+	'node-name' => "$node_name",
+	driver => "$format",
+	file => $child,
+	cache => $cache,
+	'read-only' => read_only_json_option($drive, $options),
+    };
+}
+
+sub generate_drive_blockdev {
+    my ($storecfg, $drive, $options) = @_;
+
+    my $drive_id = PVE::QemuServer::Drive::get_drive_id($drive);
+
+    die "generate_drive_blockdev called without volid/path\n" if !$drive->{file};
+    die "generate_drive_blockdev called with 'none'\n" if $drive->{file} eq 'none';
+
+    my $child = generate_file_blockdev($storecfg, $drive, $options);
+    $child = generate_format_blockdev($storecfg, $drive, $child, $options);
+
+    # this is the top filter entry point, use $drive-drive_id as nodename
+    return {
+	driver => "throttle",
+	'node-name' => "drive-$drive_id",
+	'throttle-group' => "throttle-drive-$drive_id",
+	file => $child,
+    };
+}
+
 1;
diff --git a/PVE/QemuServer/Makefile b/PVE/QemuServer/Makefile
index 8054a93b..e3671e5b 100644
--- a/PVE/QemuServer/Makefile
+++ b/PVE/QemuServer/Makefile
@@ -12,6 +12,7 @@ SOURCES=PCI.pm		\
 	MetaInfo.pm	\
 	CPUConfig.pm	\
 	CGroup.pm	\
+	Blockdev.pm	\
 	Drive.pm	\
 	QMPHelpers.pm	\
 	StateFile.pm	\
-- 
2.39.5



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