[pve-devel] [RFC qemu-server 1/5] disk reassign: add API endpoint

[Alexandre DERUMIER]

Hi,

thanks for this feature, it can be really usefull.


>>+ die "Cannot reassign disk while the source VM is running\n" 
>>+ if PVE::QemuServer::check_running($vmid); 

could it be possible to add support for unused disk for running vms ?
(Like this user can safely hot-unplug disk if needed, and reassign them)


Alexandre

----- Mail original -----
De: "Aaron Lauterer" <a.lauterer at proxmox.com>
À: "Proxmox VE development discussion" <pve-devel at lists.proxmox.com>
Envoyé: Vendredi 14 Août 2020 16:46:53
Objet: [pve-devel] [RFC qemu-server 1/5] disk reassign: add API endpoint

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 <a.lauterer at proxmox.com> 
--- 

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 



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