public inbox for pve-devel@lists.proxmox.com
 help / color / mirror / Atom feed
From: "Fabian Grünbichler" <f.gruenbichler@proxmox.com>
To: Proxmox VE development discussion <pve-devel@lists.proxmox.com>
Subject: [pve-devel] applied-series: [PATCH-SERIES qemu-server] clone_disk-related improvments
Date: Tue, 08 Feb 2022 09:36:30 +0100	[thread overview]
Message-ID: <1644309293.4sgyoi996y.astroid@nora.none> (raw)
In-Reply-To: <20220127140155.66141-1-f.ebner@proxmox.com>

with following addition on top (shown here with `-w`), to repeat even 
more of the checks after forking/locking:

diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
index 59e083eb..a359d096 100644
--- a/PVE/API2/Qemu.pm
+++ b/PVE/API2/Qemu.pm
@@ -3046,7 +3046,6 @@ __PACKAGE__->register_method({
 	my $vmid = extract_param($param, 'vmid');
 	my $newid = extract_param($param, 'newid');
 	my $pool = extract_param($param, 'pool');
-	$rpcenv->check_pool_exist($pool) if defined($pool);
 
         my $snapname = extract_param($param, 'snapname');
 	my $storage = extract_param($param, 'storage');
@@ -3059,6 +3058,10 @@ __PACKAGE__->register_method({
 	    undef $target;
 	}
 
+	my $running = PVE::QemuServer::check_running($vmid) || 0;
+
+	my $load_and_check = sub {
+	    $rpcenv->check_pool_exist($pool) if defined($pool);
 	    PVE::Cluster::check_node_exists($target) if $target;
 
 	    my $storecfg = PVE::Storage::config();
@@ -3071,15 +3074,13 @@ __PACKAGE__->register_method({
 		    PVE::Storage::storage_check_enabled($storecfg, $storage, $target);
 		    # clone only works if target storage is shared
 		    my $scfg = PVE::Storage::storage_config($storecfg, $storage);
-		die "can't clone to non-shared storage '$storage'\n" if !$scfg->{shared};
+		    die "can't clone to non-shared storage '$storage'\n"
+			if !$scfg->{shared};
 		}
 	    }
 
 	    PVE::Cluster::check_cfs_quorum();
 
-	my $running = PVE::QemuServer::check_running($vmid) || 0;
-
-	my $load_and_check = sub {
 	    my $conf = PVE::QemuConfig->load_config($vmid);
 	    PVE::QemuConfig->check_lock($conf);
 
@@ -3159,6 +3160,7 @@ __PACKAGE__->register_method({
 
 	my $clonefn = sub {
 	    my ($conffile, $newconf, $oldconf, $vollist, $drives, $fullclone) = $load_and_check->();
+	    my $storecfg = PVE::Storage::config();
 
 	    # auto generate a new uuid
 	    my $smbios1 = PVE::QemuServer::parse_smbios1($newconf->{smbios1} || '');

On January 27, 2022 3:01 pm, Fabian Ebner wrote:
> The second patch is fixing a concrete bug with vm_{resume,suspend} not being
> issued for a full clone of a running VM without guest agent. Other patches are
> minor improvements, mostly trying to prevent future bugs.
> 
> Fabian Ebner (4):
>   drive mirror monitor: warn when suspend/resume/freeze/thaw calls fail
>   api: clone: fork before locking
>   api: move disk: fork before locking
>   clone disk: don't modify drive parameter
> 
>  PVE/API2/Qemu.pm  | 382 ++++++++++++++++++++++++----------------------
>  PVE/QemuServer.pm |  10 +-
>  2 files changed, 205 insertions(+), 187 deletions(-)
> 
> -- 
> 2.30.2
> 
> 
> 
> _______________________________________________
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
> 
> 
> 




      parent reply	other threads:[~2022-02-08  8:37 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-01-27 14:01 [pve-devel] " Fabian Ebner
2022-01-27 14:01 ` [pve-devel] [PATCH qemu-server 1/4] drive mirror monitor: warn when suspend/resume/freeze/thaw calls fail Fabian Ebner
2022-01-27 14:01 ` [pve-devel] [PATCH qemu-server 2/4] api: clone: fork before locking Fabian Ebner
2022-01-31 12:34   ` Fabian Grünbichler
2022-02-03  9:31     ` Fabian Ebner
2022-02-03 12:49       ` Fabian Grünbichler
2022-01-27 14:01 ` [pve-devel] [PATCH qemu-server 3/4] api: move disk: " Fabian Ebner
2022-01-27 14:01 ` [pve-devel] [PATCH qemu-server 4/4] clone disk: don't modify drive parameter Fabian Ebner
2022-02-08  8:36 ` Fabian Grünbichler [this message]

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=1644309293.4sgyoi996y.astroid@nora.none \
    --to=f.gruenbichler@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