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) server-digest SHA256) (No client certificate requested) by lists.proxmox.com (Postfix) with ESMTPS id 294E16111A for ; Mon, 17 Aug 2020 08:59:43 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 14C902EA74 for ; Mon, 17 Aug 2020 08:59:13 +0200 (CEST) Received: from mailpro.odiso.net (mailpro.odiso.net [89.248.211.110]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by firstgate.proxmox.com (Proxmox) with ESMTPS id E7E862EA67 for ; Mon, 17 Aug 2020 08:59:10 +0200 (CEST) Received: from localhost (localhost [127.0.0.1]) by mailpro.odiso.net (Postfix) with ESMTP id 0650B14BF31E for ; Mon, 17 Aug 2020 08:59:03 +0200 (CEST) Received: from mailpro.odiso.net ([127.0.0.1]) by localhost (mailpro.odiso.net [127.0.0.1]) (amavisd-new, port 10032) with ESMTP id 4JncquaeWAk7 for ; Mon, 17 Aug 2020 08:59:02 +0200 (CEST) Received: from localhost (localhost [127.0.0.1]) by mailpro.odiso.net (Postfix) with ESMTP id DE80614BF31F for ; Mon, 17 Aug 2020 08:59:02 +0200 (CEST) X-Virus-Scanned: amavisd-new at mailpro.odiso.com Received: from mailpro.odiso.net ([127.0.0.1]) by localhost (mailpro.odiso.net [127.0.0.1]) (amavisd-new, port 10026) with ESMTP id zNxYU-Mz5oLf for ; Mon, 17 Aug 2020 08:59:02 +0200 (CEST) Received: from mailpro.odiso.net (mailpro.odiso.net [10.1.31.111]) by mailpro.odiso.net (Postfix) with ESMTP id C95CC14BF31E for ; Mon, 17 Aug 2020 08:59:02 +0200 (CEST) Date: Mon, 17 Aug 2020 08:59:02 +0200 (CEST) From: Alexandre DERUMIER To: Proxmox VE development discussion Message-ID: <2089539399.2068990.1597647542668.JavaMail.zimbra@odiso.com> In-Reply-To: <20200814144657.30063-2-a.lauterer@proxmox.com> References: <20200814144657.30063-1-a.lauterer@proxmox.com> <20200814144657.30063-2-a.lauterer@proxmox.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-Mailer: Zimbra 8.8.12_GA_3866 (ZimbraWebClient - GC83 (Linux)/8.8.12_GA_3844) Thread-Topic: disk reassign: add API endpoint Thread-Index: UVapBHJ9teazo2uKwE3NE7E782C8bw== X-SPAM-LEVEL: Spam detection results: 0 KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment RCVD_IN_DNSWL_NONE -0.0001 Sender listed at https://www.dnswl.org/, no trust SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Subject: Re: [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: Mon, 17 Aug 2020 06:59:43 -0000 Hi, thanks for this feature, it can be really usefull. >>+ die "Cannot reassign disk while the source VM is running\n"=20 >>+ if PVE::QemuServer::check_running($vmid);=20 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" =C3=80: "Proxmox VE development discussion" Envoy=C3=A9: Vendredi 14 Ao=C3=BBt 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=20 disk between VMs as this was only possible with manual intervention=20 until now. Either by renaming the VM disk or by manually adding the=20 disks volid to the config of the other VM.=20 The latter can easily cause unexpected behavior such as disks attached=20 to VM B would be deleted if it used to be a disk of VM A. This happens=20 because PVE assumes that the VMID in the volname always matches the VM=20 the disk is attached to and thus, would remove any disk with VMID A=20 when VM A was deleted.=20 The term `reassign` was chosen as it is not yet used=20 for disk VMs.=20 Signed-off-by: Aaron Lauterer =20 ---=20 I did have a bit of a discussion with Dominik off list on how to=20 implement the locking of source and target VM and we did have different=20 POVs, but in the end came to the conclusion that it might not be that=20 important anyway. Most checks are not costing a lot of performance and=20 this task will only be called occasionally.=20 The possible approaches:=20 * As in this RFC: do first checks -> lock source -> do more checks -> lock= =20 target -> do work=20 * Lock target -> lock source -> do checks -> do renaming and remove form=20 source config -> lose lock on source -> add to target=20 * lock both right away -> do checks -> do work -> lose locks=20 PVE/API2/Qemu.pm | 94 ++++++++++++++++++++++++++++++++++++++++++++++++=20 1 file changed, 94 insertions(+)=20 diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm=20 index 8da616a..a24bb71 100644=20 --- a/PVE/API2/Qemu.pm=20 +++ b/PVE/API2/Qemu.pm=20 @@ -4265,4 +4265,98 @@ __PACKAGE__->register_method({=20 return PVE::QemuServer::Cloudinit::dump_cloudinit_config($conf, $param->{vm= id}, $param->{type});=20 }});=20 +__PACKAGE__->register_method({=20 + name =3D> 'reassign_vm_disk',=20 + path =3D> '{vmid}/reassign_disk',=20 + method =3D> 'POST',=20 + protected =3D> 1,=20 + proxyto =3D> 'node',=20 + description =3D> "Reassign a disk to another VM",=20 + permissions =3D> {=20 + description =3D> "You need 'VM.Config.Disk' permissions on /vms/{vmid}, a= nd 'Datastore.Allocate' permissions on the storage.",=20 + check =3D> [ 'and',=20 + ['perm', '/vms/{vmid}', [ 'VM.Config.Disk' ]],=20 + ['perm', '/storage/{storage}', [ 'Datastore.Allocate' ]],=20 + ],=20 + },=20 + parameters =3D> {=20 + additionalProperties =3D> 0,=20 + properties =3D> {=20 + node =3D> get_standard_option('pve-node'),=20 + vmid =3D> get_standard_option('pve-vmid', { completion =3D> \&PVE::QemuSe= rver::complete_vmid }),=20 + target_vmid =3D> get_standard_option('pve-vmid', { completion =3D> \&PVE:= :QemuServer::complete_vmid }),=20 + disk =3D> {=20 + type =3D> 'string',=20 + description =3D> "The config key of the disk to move (for example, ide0 o= r scsi1).",=20 + enum =3D> [PVE::QemuServer::Drive::valid_drive_names()],=20 + },=20 + digest =3D> {=20 + type =3D> 'string',=20 + description =3D> 'Prevent changes if current configuration file has diffe= rent SHA1 digest. This can be used to prevent concurrent modifications.',= =20 + maxLength =3D> 40,=20 + optional =3D> 1,=20 + },=20 + },=20 + },=20 + returns =3D> {=20 + type =3D> 'string',=20 + description =3D> "the task ID.",=20 + },=20 + code =3D> sub {=20 + my ($param) =3D @_;=20 +=20 + my $rpcenv =3D PVE::RPCEnvironment::get();=20 + my $authuser =3D $rpcenv->get_user();=20 +=20 + my $node =3D extract_param($param, 'node');=20 + my $vmid =3D extract_param($param, 'vmid');=20 + my $target_vmid =3D extract_param($param, 'target_vmid');=20 + my $digest =3D extract_param($param, 'digest');=20 + my $disk =3D extract_param($param, 'disk');=20 +=20 + my $storecfg =3D PVE::Storage::config();=20 + my $vmlist =3D PVE::QemuServer::vzlist();=20 +=20 + die "You cannot reassign a disk to the same VM\n"=20 + if $vmid eq $target_vmid;=20 +=20 + die "Both VMs need to be on the same node\n"=20 + if !$vmlist->{$vmid}->{exists} || !$vmlist->{$target_vmid}->{exists};=20 +=20 + return PVE::QemuConfig->lock_config($vmid, sub {=20 + my $conf =3D PVE::QemuConfig->load_config($vmid);=20 + PVE::QemuConfig->check_lock($conf);=20 +=20 + die "VM config checksum missmatch (file change by other user?)\n"=20 + if $digest && $digest ne $conf->{digest};=20 +=20 + die "Cannot reassign disk while the source VM is running\n"=20 + if PVE::QemuServer::check_running($vmid);=20 +=20 + my $drive =3D PVE::QemuServer::parse_drive($disk, $conf->{$disk});=20 +=20 + die "disk '$disk' has no associated volume\n" if !$drive->{file};=20 + die "you can't reassign a cdrom\n" if PVE::QemuServer::drive_is_cdrom($dr= ive, 1);=20 +=20 + return PVE::QemuConfig->lock_config($target_vmid, sub {=20 + my $target_conf =3D PVE::QemuConfig->load_config($target_vmid);=20 + PVE::QemuConfig->check_lock($target_conf);=20 +=20 + PVE::Cluster::log_msg('info', $authuser, "reassign disk VM $vmid: reassig= n --disk $disk --target_vmid $target_vmid");=20 +=20 + my $realcmd =3D sub {=20 + my $new_volid =3D PVE::Storage::reassign_volume($storecfg, $drive->{file}= , $target_vmid);=20 +=20 + delete $conf->{$disk};=20 + PVE::QemuConfig->write_config($vmid, $conf);=20 +=20 + PVE::QemuConfig->add_unused_volume($target_conf, $new_volid);=20 + PVE::QemuConfig->write_config($target_vmid, $target_conf);=20 + };=20 +=20 + return $rpcenv->fork_worker('qmreassign', $vmid, $authuser, $realcmd);=20 + });=20 + });=20 + }});=20 +=20 1;=20 --=20 2.20.1=20 _______________________________________________=20 pve-devel mailing list=20 pve-devel@lists.proxmox.com=20 https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel=20