]> git.proxmox.com Git - qemu-server.git/blobdiff - PVE/QemuServer.pm
vmconfig_hotplug_pending: improve hotplug error handling
[qemu-server.git] / PVE / QemuServer.pm
index a79606cdd953aee789a968fd6de7c00feaa6f155..b35c090752ac84dbbd16361704f0410fd72a4f61 100644 (file)
@@ -306,6 +306,12 @@ EODESC
        minimum => 1,
        default => 1,
     },
+    numa => {
+       optional => 1,
+       type => 'boolean',
+       description => "Enable/disable Numa.",
+       default => 0,
+    },
     maxcpus => {
        optional => 1,
        type => 'integer',
@@ -483,6 +489,19 @@ my $MAX_UNUSED_DISKS = 8;
 my $MAX_HOSTPCI_DEVICES = 4;
 my $MAX_SERIAL_PORTS = 4;
 my $MAX_PARALLEL_PORTS = 3;
+my $MAX_NUMA = 8;
+
+my $numadesc = {
+    optional => 1,
+    type => 'string', format => 'pve-qm-numanode',
+    typetext => "cpus=<id[-id],memory=<mb>[[,hostnodes=<id[-id]>][,policy=<preferred|bind|interleave>]]",
+    description => "numa topology",
+};
+PVE::JSONSchema::register_standard_option("pve-qm-numanode", $numadesc);
+
+for (my $i = 0; $i < $MAX_NUMA; $i++)  {
+    $confdesc->{"numa$i"} = $numadesc;
+}
 
 my $nic_model_list = ['rtl8139', 'ne2k_pci', 'e1000',  'pcnet',  'virtio',
                      'ne2k_isa', 'i82551', 'i82557b', 'i82559er', 'vmxnet3'];
@@ -598,9 +617,9 @@ PVE::JSONSchema::register_standard_option("pve-qm-hostpci", $hostpcidesc);
 my $serialdesc = {
        optional => 1,
        type => 'string',
-       pattern => '(/dev/ttyS\d+|socket)',
+       pattern => '(/dev/.+|socket)',
        description =>  <<EODESCR,
-Create a serial device inside the VM (n is 0 to 3), and pass through a host serial device, or create a unix socket on the host side (use 'qm terminal' to open a terminal connection).
+Create a serial device inside the VM (n is 0 to 3), and pass through a host serial device (i.e. /dev/ttyS0), or create a unix socket on the host side (use 'qm terminal' to open a terminal connection).
 
 Note: This option allows direct access to host hardware. So it is no longer possible to migrate such machines - use with special care.
 
@@ -1072,18 +1091,18 @@ sub path_is_scsi {
 
 sub machine_type_is_q35 {
     my ($conf) = @_;
+
     return $conf->{machine} && ($conf->{machine} =~ m/q35/) ? 1 : 0;
 }
 
 sub print_tabletdevice_full {
     my ($conf) = @_;
+
     my $q35 = machine_type_is_q35($conf);
 
     # we use uhci for old VMs because tablet driver was buggy in older qemu
     my $usbbus = $q35 ? "ehci" : "uhci";
-    
+
     return "usb-tablet,id=tablet,bus=$usbbus.0,port=1";
 }
 
@@ -1272,6 +1291,31 @@ sub drive_is_cdrom {
 
 }
 
+sub parse_numa {
+    my ($data) = @_;
+
+    my $res = {};
+
+    foreach my $kvp (split(/,/, $data)) {
+
+       if ($kvp =~ m/^memory=(\S+)$/) {
+           $res->{memory} = $1;
+       } elsif ($kvp =~ m/^policy=(preferred|bind|interleave)$/) {
+           $res->{policy} = $1;
+       } elsif ($kvp =~ m/^cpus=(\d+)(-(\d+))?$/) {
+           $res->{cpus}->{start} = $1;
+           $res->{cpus}->{end} = $3;
+       } elsif ($kvp =~ m/^hostnodes=(\d+)(-(\d+))?$/) {
+           $res->{hostnodes}->{start} = $1;
+           $res->{hostnodes}->{end} = $3;
+       } else {
+           return undef;
+       }
+    }
+
+    return $res;
+}
+
 sub parse_hostpci {
     my ($value) = @_;
 
@@ -1389,6 +1433,91 @@ sub add_unused_volume {
     return $key;
 }
 
+sub vm_is_volid_owner {
+    my ($storecfg, $vmid, $volid) = @_;
+
+    if ($volid !~  m|^/|) {
+       my ($path, $owner);
+       eval { ($path, $owner) = PVE::Storage::path($storecfg, $volid); };
+       if ($owner && ($owner == $vmid)) {
+           return 1;
+       }
+    }
+
+    return undef;
+}
+
+sub vmconfig_delete_pending_option {
+    my ($conf, $key) = @_;
+
+    delete $conf->{pending}->{$key};
+    my $pending_delete_hash = { $key => 1 };
+    foreach my $opt (PVE::Tools::split_list($conf->{pending}->{delete})) {
+       $pending_delete_hash->{$opt} = 1;
+    }
+    $conf->{pending}->{delete} = join(',', keys %$pending_delete_hash);
+}
+
+sub vmconfig_undelete_pending_option {
+    my ($conf, $key) = @_;
+
+    my $pending_delete_hash = {};
+    foreach my $opt (PVE::Tools::split_list($conf->{pending}->{delete})) {
+       $pending_delete_hash->{$opt} = 1;
+    }
+    delete $pending_delete_hash->{$key};
+
+    my @keylist = keys %$pending_delete_hash;
+    if (scalar(@keylist)) {
+       $conf->{pending}->{delete} = join(',', @keylist);
+    } else {
+       delete $conf->{pending}->{delete};
+    }
+}
+
+sub vmconfig_register_unused_drive {
+    my ($storecfg, $vmid, $conf, $drive) = @_;
+
+    if (!drive_is_cdrom($drive)) {
+       my $volid = $drive->{file};
+       if (vm_is_volid_owner($storecfg, $vmid, $volid)) {
+           add_unused_volume($conf, $volid, $vmid);
+       }
+    }
+}
+
+sub vmconfig_cleanup_pending {
+    my ($conf) = @_;
+
+    # remove pending changes when nothing changed
+    my $changes;
+    foreach my $opt (keys %{$conf->{pending}}) {
+       if (defined($conf->{$opt}) && ($conf->{pending}->{$opt} eq  $conf->{$opt})) {
+           $changes = 1;
+           delete $conf->{pending}->{$opt};
+       }
+    }
+
+    # remove delete if option is not set
+    my $pending_delete_hash = {};
+    foreach my $opt (PVE::Tools::split_list($conf->{pending}->{delete})) {
+       if (defined($conf->{$opt})) {
+           $pending_delete_hash->{$opt} = 1;
+       } else {
+           $changes = 1;
+       }
+    }
+
+    my @keylist = keys %$pending_delete_hash;
+    if (scalar(@keylist)) {
+       $conf->{pending}->{delete} = join(',', @keylist);
+    } else {
+       delete $conf->{pending}->{delete};
+    }
+
+    return $changes;
+}
+
 my $valid_smbios1_options = {
     manufacturer => '\S+',
     product => '\S+',
@@ -1452,6 +1581,17 @@ sub verify_bootdisk {
     die "invalid boot disk '$value'\n";
 }
 
+PVE::JSONSchema::register_format('pve-qm-numanode', \&verify_numa);
+sub verify_numa {
+    my ($value, $noerr) = @_;
+
+    return $value if parse_numa($value);
+
+    return undef if $noerr;
+
+    die "unable to parse numa options\n";
+}
+
 PVE::JSONSchema::register_format('pve-qm-net', \&verify_net);
 sub verify_net {
     my ($value, $noerr) = @_;
@@ -1782,6 +1922,7 @@ sub parse_vm_config {
     my $res = {
        digest => Digest::SHA::sha1_hex($raw),
        snapshots => {},
+       pending => {},
     };
 
     $filename =~ m|/qemu-server/(\d+)\.conf$|
@@ -1791,16 +1932,24 @@ sub parse_vm_config {
 
     my $conf = $res;
     my $descr = '';
+    my $section = '';
 
     my @lines = split(/\n/, $raw);
     foreach my $line (@lines) {
        next if $line =~ m/^\s*$/;
 
-       if ($line =~ m/^\[([a-z][a-z0-9_\-]+)\]\s*$/i) {
-           my $snapname = $1;
+       if ($line =~ m/^\[PENDING\]\s*$/i) {
+           $section = 'pending';
            $conf->{description} = $descr if $descr;
            $descr = '';
-           $conf = $res->{snapshots}->{$snapname} = {};
+           $conf = $res->{$section} = {};
+           next;
+
+       } elsif ($line =~ m/^\[([a-z][a-z0-9_\-]+)\]\s*$/i) {
+           $section = $1;
+           $conf->{description} = $descr if $descr;
+           $descr = '';
+           $conf = $res->{snapshots}->{$section} = {};
            next;
        }
 
@@ -1817,6 +1966,13 @@ sub parse_vm_config {
            my $key = $1;
            my $value = $2;
            $conf->{$key} = $value;
+       } elsif ($line =~ m/^delete:\s*(.*\S)\s*$/) {
+           my $value = $1;
+           if ($section eq 'pending') {
+               $conf->{delete} = $value; # we parse this later
+           } else {
+               warn "vm $vmid - propertry 'delete' is only allowed in [PENDING]\n";
+           }
        } elsif ($line =~ m/^([a-z][a-z_]*\d*):\s*(\S+)\s*$/) {
            my $key = $1;
            my $value = $2;
@@ -1879,12 +2035,18 @@ sub write_vm_config {
     my $used_volids = {};
 
     my $cleanup_config = sub {
-       my ($cref, $snapname) = @_;
+       my ($cref, $pending, $snapname) = @_;
 
        foreach my $key (keys %$cref) {
            next if $key eq 'digest' || $key eq 'description' || $key eq 'snapshots' ||
-               $key eq 'snapstate';
+               $key eq 'snapstate' || $key eq 'pending';
            my $value = $cref->{$key};
+           if ($key eq 'delete') {
+               die "propertry 'delete' is only allowed in [PENDING]\n"
+                   if !$pending;
+               # fixme: check syntax?
+               next;
+           }
            eval { $value = check_type($key, $value); };
            die "unable to parse value of '$key' - $@" if $@;
 
@@ -1898,8 +2060,12 @@ sub write_vm_config {
     };
 
     &$cleanup_config($conf);
+
+    &$cleanup_config($conf->{pending}, 1);
+
     foreach my $snapname (keys %{$conf->{snapshots}}) {
-       &$cleanup_config($conf->{snapshots}->{$snapname}, $snapname);
+       die "internal error" if $snapname eq 'pending';
+       &$cleanup_config($conf->{snapshots}->{$snapname}, undef, $snapname);
     }
 
     # remove 'unusedX' settings if we re-add a volume
@@ -1922,13 +2088,19 @@ sub write_vm_config {
        }
 
        foreach my $key (sort keys %$conf) {
-           next if $key eq 'digest' || $key eq 'description' || $key eq 'snapshots';
+           next if $key eq 'digest' || $key eq 'description' || $key eq 'pending' || $key eq 'snapshots';
            $raw .= "$key: $conf->{$key}\n";
        }
        return $raw;
     };
 
     my $raw = &$generate_raw_config($conf);
+
+    if (scalar(keys %{$conf->{pending}})){
+       $raw .= "\n[PENDING]\n";
+       $raw .= &$generate_raw_config($conf->{pending});
+    }
+
     foreach my $snapname (sort keys %{$conf->{snapshots}}) {
        $raw .= "\n[$snapname]\n";
        $raw .= &$generate_raw_config($conf->{snapshots}->{$snapname});
@@ -2339,7 +2511,7 @@ sub vmstatus {
        $qmpclient->queue_cmd($vmid, $statuscb, 'query-status');
     }
 
-    $qmpclient->queue_execute();
+    $qmpclient->queue_execute(undef, 1);
 
     foreach my $vmid (keys %$list) {
        next if $opt_vmid && ($vmid ne $opt_vmid);
@@ -2451,7 +2623,7 @@ sub config_to_command {
     push @$cmd, '-object', "iothread,id=iothread0" if $conf->{iothread};
 
     if ($q35) {
-       # the q35 chipset support native usb2, so we enable usb controller 
+       # the q35 chipset support native usb2, so we enable usb controller
        # by default for this machine type
         push @$devices, '-readconfig', '/usr/share/qemu-server/pve-q35.cfg';
     } else {
@@ -2493,7 +2665,7 @@ sub config_to_command {
     }
 
     push @$devices, '-device', print_tabletdevice_full($conf) if $tablet;
-    
+
     # host pci devices
     for (my $i = 0; $i < $MAX_HOSTPCI_DEVICES; $i++)  {
        my $d = parse_hostpci($conf->{"hostpci$i"});
@@ -2588,6 +2760,12 @@ sub config_to_command {
     my $cores = $conf->{cores} || 1;
     my $maxcpus = $conf->{maxcpus} if $conf->{maxcpus};
 
+    my $total_cores = $sockets * $cores;
+    my $allowed_cores = $cpuinfo->{cpus};
+
+    die "MAX $allowed_cores cores allowed per VM on this node\n"
+       if ($allowed_cores < $total_cores);
+
     if ($maxcpus) {
        push @$cmd, '-smp', "cpus=$cores,maxcpus=$maxcpus";
     } else {
@@ -2680,6 +2858,78 @@ sub config_to_command {
     # push @$cmd, '-cpu', "$cpu,enforce";
     push @$cmd, '-cpu', $cpu;
 
+    my $memory =  $conf->{memory} || $defaults->{memory};
+    push @$cmd, '-m', $memory;
+
+    if ($conf->{numa}) {
+
+       my $numa_totalmemory = undef;
+       for (my $i = 0; $i < $MAX_NUMA; $i++) {
+           next if !$conf->{"numa$i"};
+           my $numa = parse_numa($conf->{"numa$i"});
+           next if !$numa;
+           # memory
+           die "missing numa node$i memory value\n" if !$numa->{memory};
+           my $numa_memory = $numa->{memory};
+           $numa_totalmemory += $numa_memory;
+           my $numa_object = "memory-backend-ram,id=ram-node$i,size=$numa_memory"."M";
+
+           # cpus
+           my $cpus_start = $numa->{cpus}->{start};
+           die "missing numa node$i cpus\n" if !defined($cpus_start);
+           my $cpus_end = $numa->{cpus}->{end} if defined($numa->{cpus}->{end});
+           my $cpus = $cpus_start;
+           if (defined($cpus_end)) {
+               $cpus .= "-$cpus_end";
+               die "numa node$i :  cpu range $cpus is incorrect\n" if $cpus_end <= $cpus_start;
+           }
+
+           # hostnodes
+           my $hostnodes_start = $numa->{hostnodes}->{start};
+           if (defined($hostnodes_start)) {
+               my $hostnodes_end = $numa->{hostnodes}->{end} if defined($numa->{hostnodes}->{end});
+               my $hostnodes = $hostnodes_start;
+               if (defined($hostnodes_end)) {
+                   $hostnodes .= "-$hostnodes_end";
+                   die "host node $hostnodes range is incorrect\n" if $hostnodes_end <= $hostnodes_start;
+               }
+
+               my $hostnodes_end_range = defined($hostnodes_end) ? $hostnodes_end : $hostnodes_start;
+               for (my $i = $hostnodes_start; $i <= $hostnodes_end_range; $i++ ) {
+                   die "host numa node$i don't exist\n" if ! -d "/sys/devices/system/node/node$i/";
+               }
+
+               # policy
+               my $policy = $numa->{policy};
+               die "you need to define a policy for hostnode $hostnodes\n" if !$policy;
+               $numa_object .= ",host-nodes=$hostnodes,policy=$policy";
+           }
+
+           push @$cmd, '-object', $numa_object;
+           push @$cmd, '-numa', "node,nodeid=$i,cpus=$cpus,memdev=ram-node$i";
+       }
+
+       die "total memory for NUMA nodes must be equal to vm memory\n"
+           if $numa_totalmemory && $numa_totalmemory != $memory;
+
+       #if no custom tology, we split memory and cores across numa nodes
+       if(!$numa_totalmemory) {
+
+           my $numa_memory = ($memory / $sockets) . "M";
+
+           for (my $i = 0; $i < $sockets; $i++)  {
+
+               my $cpustart = ($cores * $i);
+               my $cpuend = ($cpustart + $cores - 1) if $cores && $cores > 1;
+               my $cpus = $cpustart;
+               $cpus .= "-$cpuend" if $cpuend;
+
+               push @$cmd, '-object', "memory-backend-ram,size=$numa_memory,id=ram-node$i";
+               push @$cmd, '-numa', "node,nodeid=$i,cpus=$cpus,memdev=ram-node$i";
+           }
+       }
+    }
+
     push @$cmd, '-S' if $conf->{freeze};
 
     # set keyboard layout
@@ -2692,7 +2942,7 @@ sub config_to_command {
     #push @$cmd, '-soundhw', $soundhw if $soundhw;
 
     if($conf->{agent}) {
-       my $qgasocket = qga_socket($vmid);
+       my $qgasocket = qmp_socket($vmid, 1);
        my $pciaddr = print_pci_addr("qga0", $bridges);
        push @$devices, '-chardev', "socket,path=$qgasocket,server,nowait,id=qga0";
        push @$devices, '-device', "virtio-serial,id=qga0$pciaddr";
@@ -2792,8 +3042,6 @@ sub config_to_command {
        push @$devices, '-device', print_drivedevice_full($storecfg, $conf, $vmid, $drive, $bridges);
     });
 
-    push @$cmd, '-m', $conf->{memory} || $defaults->{memory};
-
     for (my $i = 0; $i < $MAX_NETS; $i++) {
          next if !$conf->{"net$i"};
          my $d = parse_net($conf->{"net$i"});
@@ -2865,13 +3113,9 @@ sub spice_port {
 }
 
 sub qmp_socket {
-    my ($vmid) = @_;
-    return "${var_run_tmpdir}/$vmid.qmp";
-}
-
-sub qga_socket {
-    my ($vmid) = @_;
-    return "${var_run_tmpdir}/$vmid.qga";
+    my ($vmid, $qga) = @_;
+    my $sockettype = $qga ? 'qga' : 'qmp';
+    return "${var_run_tmpdir}/$vmid.$sockettype";
 }
 
 sub pidfile_name {
@@ -2883,7 +3127,6 @@ sub vm_devices_list {
     my ($vmid) = @_;
 
     my $res = vm_mon_cmd($vmid, 'query-pci');
-
     my $devices = {};
     foreach my $pcibus (@$res) {
        foreach my $device (@{$pcibus->{devices}}) {
@@ -2899,6 +3142,14 @@ sub vm_devices_list {
        }
     }
 
+    my $resmice = vm_mon_cmd($vmid, 'query-mice');
+    foreach my $mice (@$resmice) {
+       if ($mice->{name} eq 'QEMU HID Tablet') {
+           $devices->{tablet} = 1;
+           last;
+       }
+    }
+
     return $devices;
 }
 
@@ -2909,16 +3160,16 @@ sub vm_deviceplug {
 
     my $q35 = machine_type_is_q35($conf);
 
-    if ($deviceid eq 'tablet') {
-       qemu_deviceadd($vmid, print_tabletdevice_full($conf));
-       return 1;
-    }
-
     return 1 if !$conf->{hotplug};
 
     my $devices_list = vm_devices_list($vmid);
     return 1 if defined($devices_list->{$deviceid});
 
+    if ($deviceid eq 'tablet') {
+       qemu_deviceadd($vmid, print_tabletdevice_full($conf));
+       return 1;
+    }
+
     qemu_bridgeadd($storecfg, $conf, $vmid, $deviceid); #add bridge if we need it for the device
 
     if ($deviceid =~ m/^(virtio)(\d+)$/) {
@@ -2959,7 +3210,7 @@ sub vm_deviceplug {
         }
     }
 
-    
+
     if (!$q35 && $deviceid =~ m/^(pci\.)(\d+)$/) {
        my $bridgeid = $2;
        my $pciaddr = print_pci_addr($deviceid);
@@ -2976,16 +3227,16 @@ sub vm_deviceunplug {
 
     return 1 if !check_running ($vmid);
 
-    if ($deviceid eq 'tablet') {
-       qemu_devicedel($vmid, $deviceid);
-       return 1;
-    }
-
     return 1 if !$conf->{hotplug};
 
     my $devices_list = vm_devices_list($vmid);
     return 1 if !defined($devices_list->{$deviceid});
 
+    if ($deviceid eq 'tablet') {
+       qemu_devicedel($vmid, $deviceid);
+       return 1;
+    }
+
     die "can't unplug bootdisk" if $conf->{bootdisk} && $conf->{bootdisk} eq $deviceid;
 
     if ($deviceid =~ m/^(virtio)(\d+)$/) {
@@ -3136,22 +3387,25 @@ sub qemu_netdevdel {
 sub qemu_cpu_hotplug {
     my ($vmid, $conf, $cores) = @_;
 
-    die "new cores config is not defined" if !$cores;
-    die "you can't add more cores than maxcpus"
-       if $conf->{maxcpus} && ($cores > $conf->{maxcpus});
-    return if !check_running($vmid);
+    my $sockets = $conf->{sockets} || 1;
+    die "cpu hotplug only works with one socket\n"
+       if $sockets > 1;
+
+    die "maxcpus is not defined\n"
+       if !$conf->{maxcpus};
 
-    my $currentcores = $conf->{cores} if $conf->{cores};
-    die "current cores is not defined" if !$currentcores;
-    die "maxcpus is not defined" if !$conf->{maxcpus};
-    raise_param_exc({ 'cores' => "online cpu unplug is not yet possible" })
-       if($cores < $currentcores);
+    die "you can't add more cores than maxcpus\n"
+       if $cores > $conf->{maxcpus};
+
+    my $currentcores = $conf->{cores} || 1;
+    die "online cpu unplug is not yet possible\n"
+       if $cores < $currentcores;
 
     my $currentrunningcores = vm_mon_cmd($vmid, "query-cpus");
-    raise_param_exc({ 'cores' => "cores number if running vm is different than configuration" })
-       if scalar (@{$currentrunningcores}) != $currentcores;
+    die "cores number if running vm is different than configuration\n"
+       if scalar(@{$currentrunningcores}) != $currentcores;
 
-    for(my $i = $currentcores; $i < $cores; $i++) {
+    for (my $i = $currentcores; $i < $cores; $i++) {
        vm_mon_cmd($vmid, "cpu-add", id => int($i));
     }
 }
@@ -3311,18 +3565,6 @@ sub qemu_volume_snapshot_delete {
     vm_mon_cmd($vmid, "delete-drive-snapshot", device => $deviceid, name => $snap);
 }
 
-sub qga_freezefs {
-    my ($vmid) = @_;
-
-    #need to impplement call to qemu-ga
-}
-
-sub qga_unfreezefs {
-    my ($vmid) = @_;
-
-    #need to impplement call to qemu-ga
-}
-
 sub set_migration_caps {
     my ($vmid) = @_;
 
@@ -3347,6 +3589,152 @@ sub set_migration_caps {
     vm_mon_cmd_nocheck($vmid, "migrate-set-capabilities", capabilities => $cap_ref);
 }
 
+# hotplug changes in [PENDING]
+# $selection hash can be used to only apply specified options, for
+# example: { cores => 1 } (only apply changed 'cores')
+# $errors ref is used to return error messages
+sub vmconfig_hotplug_pending {
+    my ($vmid, $conf, $storecfg, $selection, $errors) = @_;
+
+    my $defaults = load_defaults();
+
+    # commit values which do not have any impact on running VM first
+    # Note: those option cannot raise errors, we we do not care about
+    # $selection and always apply them.
+
+    my $add_error = sub {
+       my ($opt, $msg) = @_;
+       $errors->{$opt} = "hotplug problem - $msg";
+    };
+
+    my $changes = 0;
+    foreach my $opt (keys %{$conf->{pending}}) { # add/change
+       if ($opt eq 'name' || $opt eq 'hotplug' || $opt eq 'onboot' || $opt eq 'shares') {
+           $conf->{$opt} = $conf->{pending}->{$opt};
+           delete $conf->{pending}->{$opt};
+           $changes = 1;
+       }
+    }
+
+    if ($changes) {
+       update_config_nolock($vmid, $conf, 1);
+       $conf = load_config($vmid); # update/reload
+    }
+
+    my $hotplug = defined($conf->{hotplug}) ? $conf->{hotplug} : $defaults->{hotplug};
+
+    my @delete = PVE::Tools::split_list($conf->{pending}->{delete});
+    foreach my $opt (@delete) {
+       next if $selection && !$selection->{$opt};
+       my $skip;
+       eval {
+           if ($opt eq 'tablet') {
+               return undef if !$hotplug;
+               if ($defaults->{tablet}) {
+                   vm_deviceplug($storecfg, $conf, $vmid, $opt);
+               } else {
+                   vm_deviceunplug($vmid, $conf, $opt);
+               }
+           } elsif ($opt eq 'cores') {
+               return undef if !$hotplug;
+               qemu_cpu_hotplug($vmid, $conf, 1);
+           } else {
+               $skip = 1; # skip non-hot-pluggable options
+               return undef;
+           }
+       };
+       if (my $err = $@) {
+           &$add_error($opt, $err);
+       } elsif (!$skip) {
+           # save new config if hotplug was successful
+           delete $conf->{$opt};
+           vmconfig_undelete_pending_option($conf, $opt);
+           update_config_nolock($vmid, $conf, 1);
+           $conf = load_config($vmid); # update/reload
+       }
+    }
+
+    foreach my $opt (keys %{$conf->{pending}}) {
+       next if $selection && !$selection->{$opt};
+       my $value = $conf->{pending}->{$opt};
+       my $skip;
+       eval {
+           if ($opt eq 'tablet') {
+               return undef if !$hotplug;
+               if ($value == 1) {
+                   vm_deviceplug($storecfg, $conf, $vmid, $opt);
+               } elsif ($value == 0) {
+                   vm_deviceunplug($vmid, $conf, $opt);
+               }
+           } elsif ($opt eq 'cores') {
+               return undef if !$hotplug;
+               qemu_cpu_hotplug($vmid, $conf, $value);
+           } elsif ($opt eq 'balloon') {
+               return undef if !(defined($conf->{shares}) && ($conf->{shares} == 0));
+               # allow manual ballooning if shares is set to zero
+               my $balloon = $conf->{pending}->{balloon} || $conf->{memory} || $defaults->{memory};
+               vm_mon_cmd($vmid, "balloon", value => $balloon*1024*1024);
+           } else {
+               $skip = 1; # skip non-hot-pluggable options
+               return undef;
+           }
+       };
+       if (my $err = $@) {
+           &$add_error($opt, $err);
+       } elsif (!$skip) {
+           # save new config if hotplug was successful
+           $conf->{$opt} = $value;
+           delete $conf->{pending}->{$opt};
+           update_config_nolock($vmid, $conf, 1);
+           $conf = load_config($vmid); # update/reload
+       }
+    }
+}
+
+sub vmconfig_apply_pending {
+    my ($vmid, $conf, $storecfg) = @_;
+
+    # cold plug
+
+    my @delete = PVE::Tools::split_list($conf->{pending}->{delete});
+    foreach my $opt (@delete) { # delete
+       die "internal error" if $opt =~ m/^unused/;
+       $conf = load_config($vmid); # update/reload
+       if (!defined($conf->{$opt})) {
+           vmconfig_undelete_pending_option($conf, $opt);
+           update_config_nolock($vmid, $conf, 1);
+       } elsif (valid_drivename($opt)) {
+           vmconfig_register_unused_drive($storecfg, $vmid, $conf, parse_drive($opt, $conf->{$opt}));
+           vmconfig_undelete_pending_option($conf, $opt);
+           delete $conf->{$opt};
+           update_config_nolock($vmid, $conf, 1);
+       } else {
+           vmconfig_undelete_pending_option($conf, $opt);
+           delete $conf->{$opt};
+           update_config_nolock($vmid, $conf, 1);
+       }
+    }
+
+    $conf = load_config($vmid); # update/reload
+
+    foreach my $opt (keys %{$conf->{pending}}) { # add/change
+       $conf = load_config($vmid); # update/reload
+
+       if (defined($conf->{$opt}) && ($conf->{$opt} eq $conf->{pending}->{$opt})) {
+           # skip if nothing changed
+       } elsif (valid_drivename($opt)) {
+           vmconfig_register_unused_drive($storecfg, $vmid, $conf, parse_drive($opt, $conf->{$opt}))
+               if defined($conf->{$opt});
+           $conf->{$opt} = $conf->{pending}->{$opt};
+       } else {
+           $conf->{$opt} = $conf->{pending}->{$opt};
+       }
+
+       delete $conf->{pending}->{$opt};
+       update_config_nolock($vmid, $conf, 1);
+    }
+}
+
 sub vm_start {
     my ($storecfg, $vmid, $statefile, $skiplock, $migratedfrom, $paused, $forcemachine, $spice_ticket) = @_;
 
@@ -3359,6 +3747,11 @@ sub vm_start {
 
        die "VM $vmid already running\n" if check_running($vmid, undef, $migratedfrom);
 
+       if (!$statefile && scalar(keys %{$conf->{pending}})) {
+           vmconfig_apply_pending($vmid, $conf, $storecfg);
+           $conf = load_config($vmid); # update/reload
+       }
+
        my $defaults = load_defaults();
 
        # set environment variable useful inside network script
@@ -3426,15 +3819,15 @@ sub vm_start {
        if ($migratedfrom) {
 
            eval {
-               PVE::QemuServer::set_migration_caps($vmid);
+               set_migration_caps($vmid);
            };
            warn $@ if $@;
 
            if ($spice_port) {
                print "spice listens on port $spice_port\n";
                if ($spice_ticket) {
-                   PVE::QemuServer::vm_mon_cmd_nocheck($vmid, "set_password", protocol => 'spice', password => $spice_ticket);
-                   PVE::QemuServer::vm_mon_cmd_nocheck($vmid, "expire_password", protocol => 'spice', time => "+30");
+                   vm_mon_cmd_nocheck($vmid, "set_password", protocol => 'spice', password => $spice_ticket);
+                   vm_mon_cmd_nocheck($vmid, "expire_password", protocol => 'spice', time => "+30");
                }
            }
 
@@ -3480,7 +3873,7 @@ sub vm_qmp_command {
     eval {
        die "VM $vmid not running\n" if !check_running($vmid, $nocheck);
        my $sname = qmp_socket($vmid);
-       if (-e $sname) {
+       if (-e $sname) { # test if VM is reasonambe new and supports qmp/qga
            my $qmpclient = PVE::QMPClient->new();
 
            $res = $qmpclient->cmd($vmid, $cmd, $timeout);
@@ -3609,10 +4002,13 @@ sub vm_stop {
 
        eval {
            if ($shutdown) {
-               $nocheck ? vm_mon_cmd_nocheck($vmid, "system_powerdown") : vm_mon_cmd($vmid, "system_powerdown");
-
+               if (!$nocheck && $conf->{agent}) {
+                   vm_qmp_command($vmid, { execute => "guest-shutdown" }, $nocheck);
+               } else {
+                   vm_qmp_command($vmid, { execute => "system_powerdown" }, $nocheck);
+               }
            } else {
-               $nocheck ? vm_mon_cmd_nocheck($vmid, "quit") : vm_mon_cmd($vmid, "quit");
+               vm_qmp_command($vmid, { execute => "quit" }, $nocheck);
            }
        };
        my $err = $@;
@@ -4498,7 +4894,7 @@ sub restore_tar_archive {
     my $storecfg = cfs_read_file('storage.cfg');
 
     # destroy existing data - keep empty config
-    my $vmcfgfn = PVE::QemuServer::config_file($vmid);
+    my $vmcfgfn = config_file($vmid);
     destroy_vm($storecfg, $vmid, 1) if -f $vmcfgfn;
 
     my $tocmd = "/usr/lib/qemu-server/qmextract";
@@ -4882,16 +5278,26 @@ my $savevm_wait = sub {
 };
 
 sub snapshot_create {
-    my ($vmid, $snapname, $save_vmstate, $freezefs, $comment) = @_;
+    my ($vmid, $snapname, $save_vmstate, $comment) = @_;
 
     my $snap = &$snapshot_prepare($vmid, $snapname, $save_vmstate, $comment);
 
-    $freezefs = $save_vmstate = 0 if !$snap->{vmstate}; # vm is not running
+    $save_vmstate = 0 if !$snap->{vmstate}; # vm is not running
 
-    my $drivehash = {};
+    my $config = load_config($vmid);
 
     my $running = check_running($vmid);
 
+    my $freezefs = $running && $config->{agent};
+    $freezefs = 0 if $snap->{vmstate}; # not needed if we save RAM
+
+    my $drivehash = {};
+
+    if ($freezefs) {
+       eval { vm_mon_cmd($vmid, "guest-fsfreeze-freeze"); };
+       warn "guest-fsfreeze-freeze problems - $@" if $@;
+    }
+
     eval {
        # create internal snapshots of all drives
 
@@ -4907,8 +5313,6 @@ sub snapshot_create {
            }
        };
 
-       qga_freezefs($vmid) if $running && $freezefs;
-
        foreach_drive($snap, sub {
            my ($ds, $drive) = @_;
 
@@ -4923,11 +5327,27 @@ sub snapshot_create {
     };
     my $err = $@;
 
-    eval { qga_unfreezefs($vmid) if $running && $freezefs; };
-    warn $@ if $@;
+    if ($running) {
+       eval { vm_mon_cmd($vmid, "savevm-end")  };
+       warn $@ if $@;
 
-    eval { vm_mon_cmd($vmid, "savevm-end") if $running; };
-    warn $@ if $@;
+       if ($freezefs) {
+           eval { vm_mon_cmd($vmid, "guest-fsfreeze-thaw"); };
+           warn "guest-fsfreeze-thaw problems - $@" if $@;
+       }
+
+       # savevm-end is async, we need to wait
+       for (;;) {
+           my $stat = vm_mon_cmd_nocheck($vmid, "query-savevm");
+           if (!$stat->{bytes}) {
+               last;
+           } else {
+               print "savevm not yet finished\n";
+               sleep(1);
+               next;
+           }
+       }
+    }
 
     if ($err) {
        warn "snapshot create failed: starting cleanup\n";
@@ -5146,94 +5566,96 @@ sub qemu_img_format {
 }
 
 sub qemu_drive_mirror {
-    my ($vmid, $drive, $dst_volid, $vmiddst, $maxwait) = @_;
+    my ($vmid, $drive, $dst_volid, $vmiddst) = @_;
 
-    my $count = 1;
+    my $count = 0;
     my $old_len = 0;
     my $frozen = undef;
+    my $maxwait = 120;
 
     my $storecfg = PVE::Storage::config();
-    my ($dst_storeid, $dst_volname) = PVE::Storage::parse_volume_id($dst_volid, 1);
+    my ($dst_storeid, $dst_volname) = PVE::Storage::parse_volume_id($dst_volid);
 
-    if ($dst_storeid) {
-       my $dst_scfg = PVE::Storage::storage_config($storecfg, $dst_storeid);
+    my $dst_scfg = PVE::Storage::storage_config($storecfg, $dst_storeid);
 
-       my $format;
-        if ($dst_volname =~ m/\.(raw|qcow2)$/){
-           $format = $1;
-       }
+    my $format;
+    if ($dst_volname =~ m/\.(raw|qcow2)$/){
+       $format = $1;
+    }
 
-       my $dst_path = PVE::Storage::path($storecfg, $dst_volid);
+    my $dst_path = PVE::Storage::path($storecfg, $dst_volid);
 
-       if ($format) {
-           #fixme : sometime drive-mirror timeout, but works fine after.
-           # (I have see the problem with big volume > 200GB), so we need to eval
-           eval { vm_mon_cmd($vmid, "drive-mirror", timeout => 10, device => "drive-$drive", mode => "existing",
-                             sync => "full", target => $dst_path, format => $format); };
-       } else {
-           eval { vm_mon_cmd($vmid, "drive-mirror", timeout => 10, device => "drive-$drive", mode => "existing",
-                             sync => "full", target => $dst_path); };
-       }
+    my $opts = { timeout => 10, device => "drive-$drive", mode => "existing", sync => "full", target => $dst_path };
+    $opts->{format} = $format if $format;
 
-       eval {
-           while (1) {
-               my $stats = vm_mon_cmd($vmid, "query-block-jobs");
-               my $stat = @$stats[0];
-               die "mirroring job seem to have die. Maybe do you have bad sectors?" if !$stat;
-               die "error job is not mirroring" if $stat->{type} ne "mirror";
-
-               my $transferred = $stat->{offset};
-               my $total = $stat->{len};
+    #fixme : sometime drive-mirror timeout, but works fine after.
+    # (I have see the problem with big volume > 200GB), so we need to eval
+    eval { vm_mon_cmd($vmid, "drive-mirror", %$opts); };
+    # ignore errors here
+
+    eval {
+       while (1) {
+           my $stats = vm_mon_cmd($vmid, "query-block-jobs");
+           my $stat = @$stats[0];
+           die "mirroring job seem to have die. Maybe do you have bad sectors?" if !$stat;
+           die "error job is not mirroring" if $stat->{type} ne "mirror";
+
+           my $busy = $stat->{busy};
+
+           if (my $total = $stat->{len}) {
+               my $transferred = $stat->{offset} || 0;
                my $remaining = $total - $transferred;
                my $percent = sprintf "%.2f", ($transferred * 100 / $total);
 
-                print "transferred: $transferred bytes remaining: $remaining bytes total: $total bytes progression: $percent %\n";
+               print "transferred: $transferred bytes remaining: $remaining bytes total: $total bytes progression: $percent % busy: $busy\n";
+           }
 
-               last if ($stat->{len} == $stat->{offset});
-               if ($old_len == $stat->{offset}) {
-                   if ($maxwait && $count > $maxwait) {
-                   # if writes to disk occurs the disk needs to be freezed
-                   # to be able to complete the migration
-                       vm_suspend($vmid,1);
-                       $count = 0;
-                       $frozen = 1;
-                   } else {
-                       $count++ unless $frozen;
-                   }
-               } elsif ($frozen) {
-                   vm_resume($vmid,1);
-                   $count = 0;
+           if ($stat->{len} == $stat->{offset}) {
+               if ($busy eq 'false') {
+
+                   last if $vmiddst != $vmid;
+
+                   # try to switch the disk if source and destination are on the same guest
+                   eval { vm_mon_cmd($vmid, "block-job-complete", device => "drive-$drive") };
+                   last if !$@;
+                   die $@ if $@ !~ m/cannot be completed/;
                }
-               $old_len = $stat->{offset};
-               sleep 1;
-           }
 
-           if ($vmiddst == $vmid) {
-               # switch the disk if source and destination are on the same guest
-               vm_mon_cmd($vmid, "block-job-complete", device => "drive-$drive");
-           }
-       };
-       if (my $err = $@) {
-           eval { vm_mon_cmd($vmid, "block-job-cancel", device => "drive-$drive"); };
-           while (1) {
-               my $stats = vm_mon_cmd($vmid, "query-block-jobs");
-               my $stat = @$stats[0];
-               last if !$stat;
-               sleep 1;
+               if ($count > $maxwait) {
+                   # if too much writes to disk occurs at the end of migration
+                   #the disk needs to be freezed to be able to complete the migration
+                   vm_suspend($vmid,1);
+                   $frozen = 1;
+               }
+               $count ++
            }
-           die "mirroring error: $err";
+           $old_len = $stat->{offset};
+           sleep 1;
        }
 
-       if ($vmiddst != $vmid) {
-           # if we clone a disk for a new target vm, we don't switch the disk
-           vm_mon_cmd($vmid, "block-job-cancel", device => "drive-$drive");
-           while (1) {
-               my $stats = vm_mon_cmd($vmid, "query-block-jobs");
-               my $stat = @$stats[0];
-               last if !$stat;
-               sleep 1;
-           }
+       vm_resume($vmid, 1) if $frozen;
+
+    };
+    my $err = $@;
+
+    my $cancel_job = sub {
+       vm_mon_cmd($vmid, "block-job-cancel", device => "drive-$drive");
+       while (1) {
+           my $stats = vm_mon_cmd($vmid, "query-block-jobs");
+           my $stat = @$stats[0];
+           last if !$stat;
+           sleep 1;
        }
+    };
+
+    if ($err) {
+       eval { &$cancel_job(); };
+       die "mirroring error: $err";
+    }
+
+    if ($vmiddst != $vmid) {
+       # if we clone a disk for a new target vm, we don't switch the disk
+       &$cancel_job(); # so we call block-job-cancel
     }
 }
 
@@ -5288,7 +5710,7 @@ sub get_current_qemu_machine {
     my ($vmid) = @_;
 
     my $cmd = { execute => 'query-machines', arguments => {} };
-    my $res = PVE::QemuServer::vm_qmp_command($vmid, $cmd);
+    my $res = vm_qmp_command($vmid, $cmd);
 
     my ($current, $default);
     foreach my $e (@$res) {