public inbox for pve-devel@lists.proxmox.com
 help / color / mirror / Atom feed
From: Daniel Kral <d.kral@proxmox.com>
To: pve-devel@lists.proxmox.com
Subject: [pve-devel] [RFC qemu-server 4/9] api: clone_vm: add check if storage supports vm images
Date: Mon, 16 Sep 2024 18:38:34 +0200	[thread overview]
Message-ID: <20240916163839.236908-5-d.kral@proxmox.com> (raw)
In-Reply-To: <20240916163839.236908-1-d.kral@proxmox.com>

Adds a check when cloning a disk if the underlying storage, where the
disk should be cloned to, supports the content type 'images'. This
happens first when using the clone vm api and right before doing the
disk allocation.

Without this check, a vm could be cloned to an arbitrary storage and
then failed to start if the storage does not support vm images. This
will restrict the allocation of disk images when moving between storages
(api call `move_vm_disk`), cloning a vm (api call `clone_vm`) and
importint EFI disks and/or non-live disk imports (`create_disks`).

Signed-off-by: Daniel Kral <d.kral@proxmox.com>
---
 PVE/API2/Qemu.pm          |  5 +++--
 PVE/QemuServer.pm         |  6 ++----
 PVE/QemuServer/Helpers.pm | 20 ++++++++++++++++++++
 3 files changed, 25 insertions(+), 6 deletions(-)

diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
index 0cb4af89..a7931d98 100644
--- a/PVE/API2/Qemu.pm
+++ b/PVE/API2/Qemu.pm
@@ -3737,8 +3737,9 @@ __PACKAGE__->register_method({
 	    my $storecfg = PVE::Storage::config();
 
 	    if ($storage) {
-		# check if storage is enabled on local node
-		PVE::Storage::storage_check_enabled($storecfg, $storage);
+		# check if storage is enabled and supports vm images on local node
+		check_volume_alloc($storecfg, $storage);
+
 		if ($target) {
 		    # check if storage is available on target node
 		    PVE::Storage::storage_check_enabled($storecfg, $storage, $target);
diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
index e24c741c..507932d3 100644
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -50,7 +50,7 @@ use PVE::Tools qw(run_command file_read_firstline file_get_contents dir_glob_for
 
 use PVE::QMPClient;
 use PVE::QemuConfig;
-use PVE::QemuServer::Helpers qw(config_aware_timeout min_version windows_version);
+use PVE::QemuServer::Helpers qw(config_aware_timeout min_version windows_version alloc_volume_disk);
 use PVE::QemuServer::Cloudinit;
 use PVE::QemuServer::CGroup;
 use PVE::QemuServer::CPUConfig qw(print_cpu_device get_cpu_options get_cpu_bitness is_native_arch);
@@ -8326,9 +8326,7 @@ sub clone_disk {
 
 	    $size = PVE::Storage::volume_size_info($storecfg, $drive->{file}, 10);
 	}
-	$newvolid = PVE::Storage::vdisk_alloc(
-	    $storecfg, $storeid, $newvmid, $dst_format, $name, ($size/1024)
-	);
+	$newvolid = alloc_volume_disk($storecfg, $storeid, $newvmid, $dst_format, $name, $size/1024);
 	push @$newvollist, $newvolid;
 
 	PVE::Storage::activate_volumes($storecfg, [$newvolid]);
diff --git a/PVE/QemuServer/Helpers.pm b/PVE/QemuServer/Helpers.pm
index a5f6b328..937e32bf 100644
--- a/PVE/QemuServer/Helpers.pm
+++ b/PVE/QemuServer/Helpers.pm
@@ -13,6 +13,7 @@ use base 'Exporter';
 our @EXPORT_OK = qw(
 check_storage_alloc
 check_volume_alloc
+alloc_volume_disk
 min_version
 config_aware_timeout
 parse_number_sets
@@ -197,6 +198,25 @@ sub check_volume_alloc : prototype($$;$) {
     return 1;
 }
 
+=head3 alloc_volume_disk($storecfg, $storeid, $vmid, $format, $name, $size_kb)
+
+Allocates a volume disk image on C<$storeid>, that is defined in C<$storecfg> (which is typically
+retrieved with L<PVE::Storage::config>), with the VM id C<$vmid>, the format C<$format> (e.g.
+C<"raw">), the name C<$name> and the image size in kilobytes C<$size_kb>.
+
+This subroutine will check whether the storage, where the volume disk image should be allocated,
+supports the allocation beforehand with L<check_volume_alloc>.
+
+=cut
+
+sub alloc_volume_disk : prototype($$$$$$) {
+    my ($storecfg, $storeid, $vmid, $format, $name, $size_kb) = @_;
+
+    check_volume_alloc($storecfg, $storeid);
+
+    return PVE::Storage::vdisk_alloc($storecfg, $storeid, $vmid, $format, $name, $size_kb);
+}
+
 sub min_version {
     my ($verstr, $major, $minor, $pve) = @_;
 
-- 
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:[~2024-09-16 16:39 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-09-16 16:38 [pve-devel] [RFC qemu-server 0/9] consistent checks for storage content types on volume disk allocation Daniel Kral
2024-09-16 16:38 ` [pve-devel] [RFC qemu-server 1/9] test: cfg2cmd: expect error for invalid volume's storage content type Daniel Kral
2024-09-16 16:38 ` [pve-devel] [RFC qemu-server 2/9] cfg2cmd: improve error message for invalid volume " Daniel Kral
2024-09-16 16:38 ` [pve-devel] [RFC qemu-server 3/9] fix #5284: move_vm: add check if target storage supports vm images Daniel Kral
2024-09-16 16:38 ` Daniel Kral [this message]
2024-09-16 16:38 ` [pve-devel] [RFC qemu-server 5/9] api: create_vm: improve checks if storages for disks support " Daniel Kral
2024-09-16 16:38 ` [pve-devel] [RFC qemu-server 6/9] cloudinit: add check if storage for cloudinit disk supports " Daniel Kral
2024-09-16 16:38 ` [pve-devel] [RFC qemu-server 7/9] api: migrate_vm: improve check if target storages support " Daniel Kral
2024-09-16 16:38 ` [pve-devel] [RFC qemu-server 8/9] api: importdisk: improve check if storage supports " Daniel Kral
2024-09-16 16:38 ` [pve-devel] [RFC qemu-server 9/9] restore_vm: improve checks " Daniel Kral

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=20240916163839.236908-5-d.kral@proxmox.com \
    --to=d.kral@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