From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by lists.proxmox.com (Postfix) with ESMTPS id A957F60CD6 for ; Fri, 14 Aug 2020 16:47:31 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 9C939227A6 for ; Fri, 14 Aug 2020 16:47:01 +0200 (CEST) Received: from proxmox-new.maurer-it.com (proxmox-new.maurer-it.com [212.186.127.180]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by firstgate.proxmox.com (Proxmox) with ESMTPS id 7D1D72276F for ; Fri, 14 Aug 2020 16:46:58 +0200 (CEST) Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1]) by proxmox-new.maurer-it.com (Proxmox) with ESMTP id 4524D4460D for ; Fri, 14 Aug 2020 16:46:58 +0200 (CEST) From: Aaron Lauterer To: pve-devel@lists.proxmox.com Date: Fri, 14 Aug 2020 16:46:53 +0200 Message-Id: <20200814144657.30063-2-a.lauterer@proxmox.com> X-Mailer: git-send-email 2.20.1 In-Reply-To: <20200814144657.30063-1-a.lauterer@proxmox.com> References: <20200814144657.30063-1-a.lauterer@proxmox.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-SPAM-LEVEL: Spam detection results: 0 AWL -0.039 Adjusted score from AWL reputation of From: address KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment RCVD_IN_DNSWL_MED -2.3 Sender listed at https://www.dnswl.org/, medium trust 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. [qemu.pm] Subject: [pve-devel] [RFC qemu-server 1/5] disk reassign: add API endpoint 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: , X-List-Received-Date: Fri, 14 Aug 2020 14:47:31 -0000 The goal of this new API endpoint is to provide an easy way to move a disk between VMs as this was only possible with manual intervention until now. Either by renaming the VM disk or by manually adding the disks volid to the config of the other VM. The latter can easily cause unexpected behavior such as disks attached to VM B would be deleted if it used to be a disk of VM A. This happens because PVE assumes that the VMID in the volname always matches the VM the disk is attached to and thus, would remove any disk with VMID A when VM A was deleted. The term `reassign` was chosen as it is not yet used for disk VMs. Signed-off-by: Aaron Lauterer --- I did have a bit of a discussion with Dominik off list on how to implement the locking of source and target VM and we did have different POVs, but in the end came to the conclusion that it might not be that important anyway. Most checks are not costing a lot of performance and this task will only be called occasionally. The possible approaches: * As in this RFC: do first checks -> lock source -> do more checks -> lock target -> do work * Lock target -> lock source -> do checks -> do renaming and remove form source config -> lose lock on source -> add to target * lock both right away -> do checks -> do work -> lose locks PVE/API2/Qemu.pm | 94 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 94 insertions(+) diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm index 8da616a..a24bb71 100644 --- a/PVE/API2/Qemu.pm +++ b/PVE/API2/Qemu.pm @@ -4265,4 +4265,98 @@ __PACKAGE__->register_method({ return PVE::QemuServer::Cloudinit::dump_cloudinit_config($conf, $param->{vmid}, $param->{type}); }}); +__PACKAGE__->register_method({ + name => 'reassign_vm_disk', + path => '{vmid}/reassign_disk', + method => 'POST', + protected => 1, + proxyto => 'node', + description => "Reassign a disk to another VM", + permissions => { + description => "You need 'VM.Config.Disk' permissions on /vms/{vmid}, and 'Datastore.Allocate' permissions on the storage.", + check => [ 'and', + ['perm', '/vms/{vmid}', [ 'VM.Config.Disk' ]], + ['perm', '/storage/{storage}', [ 'Datastore.Allocate' ]], + ], + }, + parameters => { + additionalProperties => 0, + properties => { + node => get_standard_option('pve-node'), + vmid => get_standard_option('pve-vmid', { completion => \&PVE::QemuServer::complete_vmid }), + target_vmid => get_standard_option('pve-vmid', { completion => \&PVE::QemuServer::complete_vmid }), + disk => { + type => 'string', + description => "The config key of the disk to move (for example, ide0 or scsi1).", + enum => [PVE::QemuServer::Drive::valid_drive_names()], + }, + digest => { + type => 'string', + description => 'Prevent changes if current configuration file has different SHA1 digest. This can be used to prevent concurrent modifications.', + maxLength => 40, + optional => 1, + }, + }, + }, + returns => { + type => 'string', + description => "the task ID.", + }, + code => sub { + my ($param) = @_; + + my $rpcenv = PVE::RPCEnvironment::get(); + my $authuser = $rpcenv->get_user(); + + my $node = extract_param($param, 'node'); + my $vmid = extract_param($param, 'vmid'); + my $target_vmid = extract_param($param, 'target_vmid'); + my $digest = extract_param($param, 'digest'); + my $disk = extract_param($param, 'disk'); + + my $storecfg = PVE::Storage::config(); + my $vmlist = PVE::QemuServer::vzlist(); + + die "You cannot reassign a disk to the same VM\n" + if $vmid eq $target_vmid; + + die "Both VMs need to be on the same node\n" + if !$vmlist->{$vmid}->{exists} || !$vmlist->{$target_vmid}->{exists}; + + return PVE::QemuConfig->lock_config($vmid, sub { + my $conf = PVE::QemuConfig->load_config($vmid); + PVE::QemuConfig->check_lock($conf); + + die "VM config checksum missmatch (file change by other user?)\n" + if $digest && $digest ne $conf->{digest}; + + die "Cannot reassign disk while the source VM is running\n" + if PVE::QemuServer::check_running($vmid); + + my $drive = PVE::QemuServer::parse_drive($disk, $conf->{$disk}); + + die "disk '$disk' has no associated volume\n" if !$drive->{file}; + die "you can't reassign a cdrom\n" if PVE::QemuServer::drive_is_cdrom($drive, 1); + + return PVE::QemuConfig->lock_config($target_vmid, sub { + my $target_conf = PVE::QemuConfig->load_config($target_vmid); + PVE::QemuConfig->check_lock($target_conf); + + PVE::Cluster::log_msg('info', $authuser, "reassign disk VM $vmid: reassign --disk $disk --target_vmid $target_vmid"); + + my $realcmd = sub { + my $new_volid = PVE::Storage::reassign_volume($storecfg, $drive->{file}, $target_vmid); + + delete $conf->{$disk}; + PVE::QemuConfig->write_config($vmid, $conf); + + PVE::QemuConfig->add_unused_volume($target_conf, $new_volid); + PVE::QemuConfig->write_config($target_vmid, $target_conf); + }; + + return $rpcenv->fork_worker('qmreassign', $vmid, $authuser, $realcmd); + }); + }); + }}); + 1; -- 2.20.1