public inbox for pve-devel@lists.proxmox.com
 help / color / mirror / Atom feed
From: Aaron Lauterer <a.lauterer@proxmox.com>
To: pve-devel@lists.proxmox.com
Subject: [pve-devel] [RFC qemu-server 1/5] disk reassign: add API endpoint
Date: Fri, 14 Aug 2020 16:46:53 +0200	[thread overview]
Message-ID: <20200814144657.30063-2-a.lauterer@proxmox.com> (raw)
In-Reply-To: <20200814144657.30063-1-a.lauterer@proxmox.com>

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@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





  reply	other threads:[~2020-08-14 14:47 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-08-14 14:46 [pve-devel] [RFC series 0/5] disk reassign: add new feature Aaron Lauterer
2020-08-14 14:46 ` Aaron Lauterer [this message]
2020-08-17  6:59   ` [pve-devel] [RFC qemu-server 1/5] disk reassign: add API endpoint Alexandre DERUMIER
2020-08-17  7:09     ` Aaron Lauterer
2020-08-14 14:46 ` [pve-devel] [RFC qemu-server 2/5] cli: disk reassign: add reassign_disk to qm command Aaron Lauterer
2020-08-14 14:46 ` [pve-devel] [RFC storage 3/5] add disk reassign feature Aaron Lauterer
2020-08-14 14:46 ` [pve-devel] [RFC storage 4/5] disk reassign: add not implemented yet message to storages Aaron Lauterer
2020-08-14 14:46 ` [pve-devel] [RFC widget-toolkit 5/5] utils: task_desc_table: add qmreassign Aaron Lauterer

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=20200814144657.30063-2-a.lauterer@proxmox.com \
    --to=a.lauterer@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