[pve-devel] [RFC series 0/5] disk reassign: add new feature

[pve-devel] [RFC series 0/5] disk reassign: add new feature

[Aaron Lauterer]

This RFC series implements a new feature which allows users to easily
reassign disks between VMs. Currently this is only possible with one of
the following manual steps:

* rename the disk image/file and do a `qm rescan`
* configure the disk manually and use the old image name, having an
    image for VM A assigned to VM B

The latter can cause unexpected behavior because PVE expects that the
VMID in a disk name always corresponds to the VM it is assigned to. Thus
when a disk, original from VM A was manually configured as disk for VM B
it happens that, when deleting VM A, the disk in question will be
deleted as well because it still had the VMID of VM A in it's name.

To issue a reassign from the CLI run:

qm reassign_disk <source VMID> <target VMID> <disk key>

where <disk key> is the config key of the disk, e.g. ide0, scsi1 and so
on.

The following storage types are implemented at the moment:
* dir based ones
    * directory
    * NFS
    * CIFS
* ZFS
* (thin) LVM
* Ceph RBD


qemu-server: Aaron Lauterer (2):
  disk reassign: add API endpoint
  cli: disk reassign: add reassign_disk to qm command

 PVE/API2/Qemu.pm | 94 ++++++++++++++++++++++++++++++++++++++++++++++++
 PVE/CLI/qm.pm    |  2 ++
 2 files changed, 96 insertions(+)


storage: Aaron Lauterer (2):
  add disk reassign feature
  disk reassign: add not implemented yet message to storages

 PVE/Storage.pm                   | 10 ++++++++++
 PVE/Storage/CephFSPlugin.pm      |  5 +++++
 PVE/Storage/DRBDPlugin.pm        |  5 +++++
 PVE/Storage/GlusterfsPlugin.pm   |  5 +++++
 PVE/Storage/ISCSIDirectPlugin.pm |  5 +++++
 PVE/Storage/ISCSIPlugin.pm       |  4 ++++
 PVE/Storage/LVMPlugin.pm         | 15 +++++++++++++++
 PVE/Storage/Plugin.pm            | 21 +++++++++++++++++++++
 PVE/Storage/RBDPlugin.pm         | 13 +++++++++++++
 PVE/Storage/ZFSPlugin.pm         |  5 +++++
 PVE/Storage/ZFSPoolPlugin.pm     |  9 +++++++++
 11 files changed, 97 insertions(+)


widget-toolkit: Aaron Lauterer (1):
  utils: task_desc_table: add qmreassign

 src/Utils.js | 1 +
 1 file changed, 1 insertion(+)

-- 
2.20.1

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

[Aaron Lauterer]

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

[pve-devel] [RFC qemu-server 2/5] cli: disk reassign: add reassign_disk to qm command

[Aaron Lauterer]

Signed-off-by: Aaron Lauterer <a.lauterer@proxmox.com>
---
 PVE/CLI/qm.pm | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/PVE/CLI/qm.pm b/PVE/CLI/qm.pm
index 282fa86..c947884 100755
--- a/PVE/CLI/qm.pm
+++ b/PVE/CLI/qm.pm
@@ -913,6 +913,8 @@ our $cmddef = {
 
     move_disk => [ "PVE::API2::Qemu", 'move_vm_disk', ['vmid', 'disk', 'storage'], { node => $nodename }, $upid_exit ],
 
+    reassign_disk => [ "PVE::API2::Qemu", 'reassign_vm_disk', ['vmid', 'target_vmid', 'disk'], { node => $nodename } ],
+
     unlink => [ "PVE::API2::Qemu", 'unlink', ['vmid'], { node => $nodename } ],
 
     config => [ "PVE::API2::Qemu", 'vm_config', ['vmid'],
-- 
2.20.1

[pve-devel] [RFC storage 3/5] add disk reassign feature

[Aaron Lauterer]

Functionality has been added for the following storage types:

* dir based ones
    * directory
    * NFS
    * CIFS
* ZFS
* (thin) LVM
* Ceph RBD

Signed-off-by: Aaron Lauterer <a.lauterer@proxmox.com>
---
 PVE/Storage.pm               | 10 ++++++++++
 PVE/Storage/LVMPlugin.pm     | 15 +++++++++++++++
 PVE/Storage/Plugin.pm        | 21 +++++++++++++++++++++
 PVE/Storage/RBDPlugin.pm     | 13 +++++++++++++
 PVE/Storage/ZFSPoolPlugin.pm |  9 +++++++++
 5 files changed, 68 insertions(+)

diff --git a/PVE/Storage.pm b/PVE/Storage.pm
index 4a60615..919c606 100755
--- a/PVE/Storage.pm
+++ b/PVE/Storage.pm
@@ -1759,6 +1759,16 @@ sub complete_volume {
     return $res;
 }
 
+sub reassign_volume {
+    my ($cfg, $volid, $target_vmid) = @_;
+
+    my ($storeid, $volname) = parse_volume_id($volid);
+    my $scfg = storage_config($cfg, $storeid);
+    my $plugin = PVE::Storage::Plugin->lookup($scfg->{type});
+
+    return $plugin->reassign_volume($scfg, $storeid, $volname, $target_vmid);
+}
+
 # Various io-heavy operations require io/bandwidth limits which can be
 # configured on multiple levels: The global defaults in datacenter.cfg, and
 # per-storage overrides. When we want to do a restore from storage A to storage
diff --git a/PVE/Storage/LVMPlugin.pm b/PVE/Storage/LVMPlugin.pm
index c0740d4..28ff6c8 100644
--- a/PVE/Storage/LVMPlugin.pm
+++ b/PVE/Storage/LVMPlugin.pm
@@ -337,6 +337,13 @@ sub lvcreate {
     run_command($cmd, errmsg => "lvcreate '$vg/$name' error");
 }
 
+sub lvrename {
+    my ($vg, $oldname, $newname) = @_;
+
+    my $cmd = ['/sbin/lvrename', $vg, $oldname, $newname];
+    run_command($cmd, errmsg => "lvrename '${vg}/${oldname}' to '${newname}' error");
+}
+
 sub alloc_image {
     my ($class, $storeid, $scfg, $vmid, $fmt, $name, $size) = @_;
 
@@ -681,4 +688,12 @@ sub volume_import_write {
 	input => '<&'.fileno($input_fh));
 }
 
+sub reassign_volume {
+    my ($class, $scfg, $storeid, $volname, $target_vmid) = @_;
+
+    my $target_volname = $class->find_free_diskname($storeid, $scfg, $target_vmid);
+    lvrename($scfg->{vgname}, $volname, $target_volname);
+    return "${storeid}:${target_volname}";
+}
+
 1;
diff --git a/PVE/Storage/Plugin.pm b/PVE/Storage/Plugin.pm
index 8a58ff4..770a482 100644
--- a/PVE/Storage/Plugin.pm
+++ b/PVE/Storage/Plugin.pm
@@ -1411,4 +1411,25 @@ sub volume_import_formats {
     return ();
 }
 
+sub reassign_volume {
+    my ($class, $scfg, $storeid, $volname, $target_vmid) = @_;
+
+    my $basedir = $class->get_subdir($scfg, 'images');
+    my $imagedir = "${basedir}/${target_vmid}";
+
+    mkpath $imagedir;
+
+    my @parsed_volname = $class->parse_volname($volname);
+    my $format = $parsed_volname[6];
+    my $target_volname = $class->find_free_diskname($storeid, $scfg, $target_vmid, $format, 1);
+
+    my $old_path = "${basedir}/${volname}";
+    my $new_path = "${imagedir}/${target_volname}";
+
+    rename($old_path, $new_path) ||
+	die "rename '$old_path' to '$new_path' failed - $!\n";
+
+    return "${storeid}:${target_vmid}/${target_volname}";
+}
+
 1;
diff --git a/PVE/Storage/RBDPlugin.pm b/PVE/Storage/RBDPlugin.pm
index 38f2b46..c0bd74c 100644
--- a/PVE/Storage/RBDPlugin.pm
+++ b/PVE/Storage/RBDPlugin.pm
@@ -719,4 +719,17 @@ sub volume_has_feature {
     return undef;
 }
 
+sub reassign_volume {
+    my ($class, $scfg, $storeid, $volname, $target_vmid) = @_;
+
+    my $target_volname = $class->find_free_diskname($storeid, $scfg, $target_vmid);
+    my $cmd = $rbd_cmd->($scfg, $storeid, 'rename', $volname, $target_volname);
+
+    run_rbd_command(
+	$cmd,
+	errmsg => "could not rename image '$volname' to '$target_volname'",
+    );
+    return "${storeid}:${target_volname}";
+}
+
 1;
diff --git a/PVE/Storage/ZFSPoolPlugin.pm b/PVE/Storage/ZFSPoolPlugin.pm
index 10354b3..5baa5c2 100644
--- a/PVE/Storage/ZFSPoolPlugin.pm
+++ b/PVE/Storage/ZFSPoolPlugin.pm
@@ -749,4 +749,13 @@ sub volume_import_formats {
     return $class->volume_export_formats($scfg, $storeid, $volname, undef, $base_snapshot, $with_snapshots);
 }
 
+sub reassign_volume {
+    my ($class, $scfg, $storeid, $volname, $target_vmid) = @_;
+
+    my $target_volname = $class->find_free_diskname($storeid, $scfg, $target_vmid);
+    $class->zfs_request($scfg, 5, 'rename', "$scfg->{pool}/$volname", "$scfg->{pool}/$target_volname");
+
+    return "${storeid}:${target_volname}";
+}
+
 1;
-- 
2.20.1

[pve-devel] [RFC storage 4/5] disk reassign: add not implemented yet message to storages

[Aaron Lauterer]

Signed-off-by: Aaron Lauterer <a.lauterer@proxmox.com>
---
 PVE/Storage/CephFSPlugin.pm      | 5 +++++
 PVE/Storage/DRBDPlugin.pm        | 5 +++++
 PVE/Storage/GlusterfsPlugin.pm   | 5 +++++
 PVE/Storage/ISCSIDirectPlugin.pm | 5 +++++
 PVE/Storage/ISCSIPlugin.pm       | 4 ++++
 PVE/Storage/ZFSPlugin.pm         | 5 +++++
 6 files changed, 29 insertions(+)

diff --git a/PVE/Storage/CephFSPlugin.pm b/PVE/Storage/CephFSPlugin.pm
index 880ec05..ba31366 100644
--- a/PVE/Storage/CephFSPlugin.pm
+++ b/PVE/Storage/CephFSPlugin.pm
@@ -222,4 +222,9 @@ sub deactivate_storage {
     }
 }
 
+sub reassign_volume {
+    my ($class, $scfg, $storeid, $volname, $target_vmid) = @_;
+    die "Not implemented for this storage type\n";
+}
+
 1;
diff --git a/PVE/Storage/DRBDPlugin.pm b/PVE/Storage/DRBDPlugin.pm
index dbae4d1..304ade7 100644
--- a/PVE/Storage/DRBDPlugin.pm
+++ b/PVE/Storage/DRBDPlugin.pm
@@ -404,4 +404,9 @@ sub volume_has_feature {
     return undef;
 }
 
+sub reassign_volume {
+    my ($class, $scfg, $storeid, $volname, $target_vmid) = @_;
+    die "Not implemented for this storage type\n";
+}
+
 1;
diff --git a/PVE/Storage/GlusterfsPlugin.pm b/PVE/Storage/GlusterfsPlugin.pm
index 2dd414d..831d2ef 100644
--- a/PVE/Storage/GlusterfsPlugin.pm
+++ b/PVE/Storage/GlusterfsPlugin.pm
@@ -348,4 +348,9 @@ sub check_connection {
     return defined($server) ? 1 : 0;
 }
 
+sub reassign_volume {
+    my ($class, $scfg, $storeid, $volname, $target_vmid) = @_;
+    die "Not implemented for this storage type\n";
+}
+
 1;
diff --git a/PVE/Storage/ISCSIDirectPlugin.pm b/PVE/Storage/ISCSIDirectPlugin.pm
index 9777969..8d72173 100644
--- a/PVE/Storage/ISCSIDirectPlugin.pm
+++ b/PVE/Storage/ISCSIDirectPlugin.pm
@@ -252,4 +252,9 @@ sub volume_has_feature {
     return undef;
 }
 
+sub reassign_volume {
+    my ($class, $scfg, $storeid, $volname, $target_vmid) = @_;
+    die "Not implemented for this storage type\n";
+}
+
 1;
diff --git a/PVE/Storage/ISCSIPlugin.pm b/PVE/Storage/ISCSIPlugin.pm
index f2694ba..602fb1d 100644
--- a/PVE/Storage/ISCSIPlugin.pm
+++ b/PVE/Storage/ISCSIPlugin.pm
@@ -438,5 +438,9 @@ sub volume_has_feature {
     return undef;
 }
 
+sub reassign_volume {
+    my ($class, $scfg, $storeid, $volname, $target_vmid) = @_;
+    die "Not implemented for this storage type\n";
+}
 
 1;
diff --git a/PVE/Storage/ZFSPlugin.pm b/PVE/Storage/ZFSPlugin.pm
index 383f0a0..4097b1a 100644
--- a/PVE/Storage/ZFSPlugin.pm
+++ b/PVE/Storage/ZFSPlugin.pm
@@ -421,4 +421,9 @@ sub deactivate_volume {
     return 1;
 }
 
+sub reassign_volume {
+    my ($class, $scfg, $storeid, $volname, $target_vmid) = @_;
+    die "Not implemented for this storage type\n";
+}
+
 1;
-- 
2.20.1

[pve-devel] [RFC widget-toolkit 5/5] utils: task_desc_table: add qmreassign

[Aaron Lauterer]

Signed-off-by: Aaron Lauterer <a.lauterer@proxmox.com>
---
 src/Utils.js | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/Utils.js b/src/Utils.js
index 4be95b2..c022084 100644
--- a/src/Utils.js
+++ b/src/Utils.js
@@ -583,6 +583,7 @@ utilities: {
 	qmigrate: ['VM', gettext('Migrate')],
 	qmclone: ['VM', gettext('Clone')],
 	qmmove: ['VM', gettext('Move disk')],
+	qmreassign: ['VM', gettext('Reassign disk')],
 	qmtemplate: ['VM', gettext('Convert to template')],
 	qmstart: ['VM', gettext('Start')],
 	qmstop: ['VM', gettext('Stop')],
-- 
2.20.1

Re: [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@proxmox.com>
À: "Proxmox VE development discussion" <pve-devel@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@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@lists.proxmox.com 
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel 

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

[Aaron Lauterer]

On 8/17/20 8:59 AM, Alexandre DERUMIER wrote:
> 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)

Good point. At the moment `unusedX` disks don't work at all due to the way I am reusing existing API schemas.

I will try to get this working in the next version. :)

> 
> 
> Alexandre
>