vmconfig_apply_pending: add error handling

[qemu-server.git] / PVE / API2 / Qemu.pm

diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
index b28373e0f53b141b673e3f4fa2a3d730672d1fab..e853a8355f4456f662cb2b4cc619dffa47c77219 100644 (file)
--- a/PVE/API2/Qemu.pm
+++ b/PVE/API2/Qemu.pm
@@ -206,7 +206,7 @@ my $create_disks = sub {
 
                my $size = PVE::Storage::volume_size_info($storecfg, $volid);
 
-               die "volume $volid does not exists\n" if !$size;
+               die "volume $volid does not exist\n" if !$size;
 
                $disk->{size} = $size;
            }
@@ -315,7 +315,6 @@ my $check_vm_modify_config_perm = sub {
        # some checks (e.g., disk, serial port, usb) need to be done somewhere
        # else, as there the permission can be value dependend
        next if PVE::QemuServer::is_valid_drivename($opt);
-       next if $opt eq 'vmstate';
        next if $opt eq 'cdrom';
        next if $opt =~ m/^(?:unused|serial|usb)\d+$/;
 
@@ -338,6 +337,10 @@ my $check_vm_modify_config_perm = sub {
            $rpcenv->check_vm_perm($authuser, $vmid, $pool, ['VM.Config.Disk']);
        } elsif ($cloudinitoptions->{$opt} || ($opt =~ m/^(?:net|ipconfig)\d+$/)) {
            $rpcenv->check_vm_perm($authuser, $vmid, $pool, ['VM.Config.Network']);
+       } elsif ($opt eq 'vmstate') {
+           # the user needs Disk and PowerMgmt privileges to change the vmstate
+           # also needs privileges on the storage, that will be checked later
+           $rpcenv->check_vm_perm($authuser, $vmid, $pool, ['VM.Config.Disk', 'VM.PowerMgmt' ]);
        } else {
            # catches hostpci\d+, args, lock, etc.
            # new options will be checked here
@@ -571,17 +574,18 @@ __PACKAGE__->register_method({
                }
 
                PVE::AccessControl::add_vm_to_pool($vmid, $pool) if $pool;
-
-               if ($start_after_create) {
-                   eval { PVE::API2::Qemu->vm_start({ vmid => $vmid, node => $node }) };
-                   warn $@ if $@;
-               }
            };
 
            # ensure no old replication state are exists
            PVE::ReplicationState::delete_guest_states($vmid);
 
-           return PVE::QemuConfig->lock_config_full($vmid, 1, $realcmd);
+           PVE::QemuConfig->lock_config_full($vmid, 1, $realcmd);
+
+           if ($start_after_create) {
+               print "Execute autostart\n";
+               eval { PVE::API2::Qemu->vm_start({ vmid => $vmid, node => $node }) };
+               warn $@ if $@;
+           }
        };
 
        my $createfn = sub {
@@ -1152,8 +1156,6 @@ my $update_vm_api  = sub {
                    }
                } elsif ($opt eq 'vmstate') {
                    PVE::QemuConfig->check_protection($conf, "can't remove vmstate '$val'");
-                   # the user needs Disk and PowerMgmt privileges to remove the vmstate
-                   $rpcenv->check_vm_perm($authuser, $vmid, undef, ['VM.Config.Disk', 'VM.PowerMgmt' ]);
                    if (PVE::QemuServer::try_deallocate_drive($storecfg, $vmid, $conf, $opt, { file => $val }, $rpcenv, $authuser, 1)) {
                        delete $conf->{$opt};
                        PVE::QemuConfig->write_config($vmid, $conf);
@@ -1240,13 +1242,13 @@ my $update_vm_api  = sub {
 
            $conf = PVE::QemuConfig->load_config($vmid); # update/reload
 
+           my $errors = {};
            if ($running) {
-               my $errors = {};
                PVE::QemuServer::vmconfig_hotplug_pending($vmid, $conf, $storecfg, $modified, $errors);
-               raise_param_exc($errors) if scalar(keys %$errors);
            } else {
-               PVE::QemuServer::vmconfig_apply_pending($vmid, $conf, $storecfg, $running);
+               PVE::QemuServer::vmconfig_apply_pending($vmid, $conf, $storecfg, $running, $errors);
            }
+           raise_param_exc($errors) if scalar(keys %$errors);
 
            return;
        };
@@ -2387,6 +2389,9 @@ __PACKAGE__->register_method({
     proxyto => 'node',
     description => "Suspend virtual machine.",
     permissions => {
+       description => "You need 'VM.PowerMgmt' on /vms/{vmid}, and if you have set 'todisk',".
+           " you need also 'VM.Config.Disk' on /vms/{vmid} and 'Datastore.AllocateSpace'".
+           " on the storage for the vmstate.",
        check => ['perm', '/vms/{vmid}', [ 'VM.PowerMgmt' ]],
     },
     parameters => {
@@ -2435,6 +2440,20 @@ __PACKAGE__->register_method({
        die "Cannot suspend HA managed VM to disk\n"
            if $todisk && PVE::HA::Config::vm_is_ha_managed($vmid);
 
+       # early check for storage permission, for better user feedback
+       if ($todisk) {
+           $rpcenv->check_vm_perm($authuser, $vmid, undef, ['VM.Config.Disk']);
+
+           if (!$statestorage) {
+               # get statestorage from config if none is given
+               my $conf = PVE::QemuConfig->load_config($vmid);
+               my $storecfg = PVE::Storage::config();
+               $statestorage = PVE::QemuServer::find_vmstate_storage($conf, $storecfg);
+           }
+
+           $rpcenv->check($authuser, "/storage/$statestorage", ['Datastore.AllocateSpace']);
+       }
+
        my $realcmd = sub {
            my $upid = shift;