[pve-devel] [PATCH qemu-server 2/8] vm_start: condense signature

Fabian Grünbichler f.gruenbichler at proxmox.com
Mon Mar 30 13:41:30 CEST 2020


as preparation for refactoring it further. remote migration will add
another 1-2 parameters, and it is already unwieldly enough as it is.

Signed-off-by: Fabian Grünbichler <f.gruenbichler at proxmox.com>
---
 PVE/API2/Qemu.pm         | 23 ++++++++++++++++----
 PVE/QemuConfig.pm        |  6 ++++-
 PVE/QemuServer.pm        | 47 +++++++++++++++++++++++++++++-----------
 PVE/VZDump/QemuServer.pm |  6 ++++-
 test/snapshot-test.pm    |  4 ++--
 5 files changed, 65 insertions(+), 21 deletions(-)

diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
index 396879a..788db93 100644
--- a/PVE/API2/Qemu.pm
+++ b/PVE/API2/Qemu.pm
@@ -2114,9 +2114,24 @@ __PACKAGE__->register_method({
 
 		syslog('info', "start VM $vmid: $upid\n");
 
-		PVE::QemuServer::vm_start($storecfg, $vmid, $stateuri, $skiplock, $migratedfrom, undef, $machine,
-					  $spice_ticket, $migration_network, $migration_type, $targetstorage, $timeout,
-					  $nbd_protocol_version, $replicated_volumes);
+		my $migrate_opts = {
+		    migratedfrom => $migratedfrom,
+		    spice_ticket => $spice_ticket,
+		    network => $migration_network,
+		    type => $migration_type,
+		    targetstorage => $targetstorage,
+		    nbd_proto_version => $nbd_protocol_version,
+		    replicated_volumes => $replicated_volumes,
+		};
+
+		my $params = {
+		    statefile => $stateuri,
+		    skiplock => $skiplock,
+		    forcemachine => $machine,
+		    timeout => $timeout,
+		};
+
+		PVE::QemuServer::vm_start($storecfg, $vmid, $params, $migrate_opts);
 		return;
 	    };
 
@@ -2584,7 +2599,7 @@ __PACKAGE__->register_method({
 		PVE::QemuServer::vm_resume($vmid, $skiplock, $nocheck);
 	    } else {
 		my $storecfg = PVE::Storage::config();
-		PVE::QemuServer::vm_start($storecfg, $vmid, undef, $skiplock);
+		PVE::QemuServer::vm_start($storecfg, $vmid, { skiplock => $skiplock });
 	    }
 
 	    return;
diff --git a/PVE/QemuConfig.pm b/PVE/QemuConfig.pm
index 6322089..f32618e 100644
--- a/PVE/QemuConfig.pm
+++ b/PVE/QemuConfig.pm
@@ -392,7 +392,11 @@ sub __snapshot_rollback_vm_start {
     my ($class, $vmid, $vmstate, $data) = @_;
 
     my $storecfg = PVE::Storage::config();
-    PVE::QemuServer::vm_start($storecfg, $vmid, $vmstate, undef, undef, undef, $data->{forcemachine});
+    my $params = {
+	statefile => $vmstate,
+	forcemachine => $data->{forcemachine},
+    };
+    PVE::QemuServer::vm_start($storecfg, $vmid, $params);
 }
 
 sub __snapshot_rollback_get_unused {
diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
index 2e77064..ad48c74 100644
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -4709,12 +4709,30 @@ sub vmconfig_update_disk {
     vm_deviceplug($storecfg, $conf, $vmid, $opt, $drive, $arch, $machine_type);
 }
 
+# params:
+#   statefile => 'tcp', 'unix' for migration or path/volid for RAM state
+#   skiplock => 0/1, skip checking for config lock
+#   forcemachine => to force Qemu machine (rollback/migration)
+#   timeout => in seconds
+#   paused => start VM in paused state (backup)
+# migrate_opts:
+#   migratedfrom => source node
+#   spice_ticket => used for spice migration, passed via tunnel/stdin
+#   network => CIDR of migration network
+#   type => secure/insecure - tunnel over encrypted connection or plain-text
+#   targetstorage = storageid/'1' - target storage for disks migrated over NBD
+#   nbd_proto_version => int, 0 for TCP, 1 for UNIX
+#   replicated_volumes = which volids should be re-used with bitmaps for nbd migration
 sub vm_start {
-    my ($storecfg, $vmid, $statefile, $skiplock, $migratedfrom, $paused,
-	$forcemachine, $spice_ticket, $migration_network, $migration_type,
-	$targetstorage, $timeout, $nbd_protocol_version, $replicated_volumes) = @_;
+    my ($storecfg, $vmid, $params, $migrate_opts) = @_;
 
     PVE::QemuConfig->lock_config($vmid, sub {
+	my $statefile = $params->{statefile};
+
+	my $migratedfrom = $migrate_opts->{migratedfrom};
+	my $migration_type = $migrate_opts->{type};
+	my $targetstorage = $migrate_opts->{targetstorage};
+
 	my $conf = PVE::QemuConfig->load_config($vmid, $migratedfrom);
 
 	die "you can't start a vm if it's a template\n" if PVE::QemuConfig->is_template($conf);
@@ -4722,7 +4740,7 @@ sub vm_start {
 	my $is_suspended = PVE::QemuConfig->has_lock($conf, 'suspended');
 
 	PVE::QemuConfig->check_lock($conf)
-	    if !($skiplock || $is_suspended);
+	    if !($params->{skiplock} || $is_suspended);
 
 	die "VM $vmid already running\n" if check_running($vmid, undef, $migratedfrom);
 
@@ -4766,7 +4784,7 @@ sub vm_start {
 	    foreach my $opt (sort keys %$local_volumes) {
 
 		my ($volid, $storeid, $volname) = @{$local_volumes->{$opt}};
-		if ($replicated_volumes->{$volid}) {
+		if ($migrate_opts->{replicated_volumes}->{$volid}) {
 		    # re-use existing, replicated volume with bitmap on source side
 		    $local_volumes->{$opt} = $conf->{${opt}};
 		    print "re-using replicated volume: $opt - $volid\n";
@@ -4801,6 +4819,7 @@ sub vm_start {
 
 	PVE::GuestHelpers::exec_hookscript($conf, $vmid, 'pre-start', 1);
 
+	my $forcemachine = $params->{forcemachine};
 	if ($is_suspended) {
 	    # enforce machine type on suspended vm to ensure HW compatibility
 	    $forcemachine = $conf->{runningmachine};
@@ -4811,10 +4830,12 @@ sub vm_start {
 
 	my $migration_ip;
 	my $get_migration_ip = sub {
-	    my ($cidr, $nodename) = @_;
+	    my ($nodename) = @_;
 
 	    return $migration_ip if defined($migration_ip);
 
+	    my $cidr = $migrate_opts->{network};
+
 	    if (!defined($cidr)) {
 		my $dc_conf = PVE::Cluster::cfs_read_file('datacenter.cfg');
 		$cidr = $dc_conf->{migration}->{network};
@@ -4854,7 +4875,7 @@ sub vm_start {
 		}
 
 		if ($migration_type eq 'insecure') {
-		    $localip = $get_migration_ip->($migration_network, $nodename);
+		    $localip = $get_migration_ip->($nodename);
 		    $localip = "[$localip]" if Net::IP::ip_is_ipv6($localip);
 		}
 
@@ -4883,7 +4904,7 @@ sub vm_start {
 		push @$vollist, $statefile;
 		push @$cmd, '-loadstate', $statepath;
 	    }
-	} elsif ($paused) {
+	} elsif ($params->{paused}) {
 	    push @$cmd, '-S';
 	}
 
@@ -4924,7 +4945,7 @@ sub vm_start {
 	my $cpuunits = defined($conf->{cpuunits}) ? $conf->{cpuunits}
 	                                          : $defaults->{cpuunits};
 
-	my $start_timeout = $timeout // config_aware_timeout($conf, $is_suspended);
+	my $start_timeout = $params->{timeout} // config_aware_timeout($conf, $is_suspended);
 	my %run_params = (
 	    timeout => $statefile ? undef : $start_timeout,
 	    umask => 0077,
@@ -4996,7 +5017,7 @@ sub vm_start {
 
 	#start nbd server for storage migration
 	if ($targetstorage) {
-	    $nbd_protocol_version //= 0;
+	    my $nbd_protocol_version = $migrate_opts->{nbd_proto_version} // 0;
 
 	    my $migrate_storage_uri;
 	    # nbd_protocol_version > 0 for unix socket support
@@ -5006,7 +5027,7 @@ sub vm_start {
 		$migrate_storage_uri = "nbd:unix:$socket_path";
 	    } else {
 		my $nodename = nodename();
-		my $localip = $get_migration_ip->($migration_network, $nodename);
+		my $localip = $get_migration_ip->($nodename);
 		my $pfamily = PVE::Tools::get_host_address_family($nodename);
 		my $storage_migrate_port = PVE::Tools::next_migrate_port($pfamily);
 
@@ -5030,8 +5051,8 @@ sub vm_start {
 
 	    if ($spice_port) {
 	        print "spice listens on port $spice_port\n";
-		if ($spice_ticket) {
-		    mon_cmd($vmid, "set_password", protocol => 'spice', password => $spice_ticket);
+		if ($migrate_opts->{spice_ticket}) {
+		    mon_cmd($vmid, "set_password", protocol => 'spice', password => $migrate_opts->{spice_ticket});
 		    mon_cmd($vmid, "expire_password", protocol => 'spice', time => "+30");
 		}
 	    }
diff --git a/PVE/VZDump/QemuServer.pm b/PVE/VZDump/QemuServer.pm
index 35a5d21..08c29ba 100644
--- a/PVE/VZDump/QemuServer.pm
+++ b/PVE/VZDump/QemuServer.pm
@@ -676,7 +676,11 @@ sub enforce_vm_running_for_backup {
     eval {
 	$self->loginfo("starting kvm to execute backup task");
 	# start with skiplock
-	PVE::QemuServer::vm_start($self->{storecfg}, $vmid, undef, 1, undef, 1);
+	my $params = {
+	    skiplock => 1,
+	    paused => 1,
+	};
+	PVE::QemuServer::vm_start($self->{storecfg}, $vmid, $params);
     };
     die $@ if $@;
 }
diff --git a/test/snapshot-test.pm b/test/snapshot-test.pm
index ee9fc13..f79db6b 100644
--- a/test/snapshot-test.pm
+++ b/test/snapshot-test.pm
@@ -356,13 +356,13 @@ sub do_snapshots_with_qemu {
 }
 
 sub vm_start {
-    my ($storecfg, $vmid, $statefile, $skiplock, $migratedfrom, $paused, $forcemachine) = @_;
+    my ($storecfg, $vmid, $params, $migrate_opts) = @_;
 
     die "Storage config not mocked! aborting\n"
 	if defined($storecfg);
 
     die "statefile and forcemachine must be both defined or undefined! aborting\n"
-	if defined($statefile) xor defined($forcemachine);
+	if defined($params->{statefile}) xor defined($params->{forcemachine});
 
     return;
 }
-- 
2.20.1





More information about the pve-devel mailing list