[pve-devel] [PATCH v5 18/19] vm_deviceunplug: raise expection if something fail

[Dietmar Maurer]

Signed-off-by: Dietmar Maurer <dietmar at proxmox.com>
---
 PVE/QemuServer.pm |   81 +++++++++++++++++++++++++++--------------------------
 1 file changed, 42 insertions(+), 39 deletions(-)

diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
index c568dfd..14989f8 100644
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -3057,8 +3057,9 @@ sub vm_deviceplug {
         my $devicefull = print_drivedevice_full($storecfg, $conf, $vmid, $device);
         qemu_deviceadd($vmid, $devicefull);
         if(!qemu_deviceaddverify($vmid, $deviceid)) {
-           qemu_drivedel($vmid, $deviceid);
-           return undef;
+	    eval { qemu_drivedel($vmid, $deviceid); };
+	    warn $@ if $@;
+	    return undef;
         }
     }
 
@@ -3075,8 +3076,9 @@ sub vm_deviceplug {
         return undef if !qemu_driveadd($storecfg, $vmid, $device);
         my $devicefull = print_drivedevice_full($storecfg, $conf, $vmid, $device);
         if(!qemu_deviceadd($vmid, $devicefull)) { # fixme: use qemu_deviceaddverify?
-           qemu_drivedel($vmid, $deviceid);
-           return undef;
+	    eval { qemu_drivedel($vmid, $deviceid); };
+	    warn $@ if $@;
+	    return undef;
         }
     }
 
@@ -3106,39 +3108,40 @@ sub vm_deviceplug {
 sub vm_deviceunplug {
     my ($vmid, $conf, $deviceid) = @_;
 
-    return 1 if !check_running ($vmid);
-
-    return 1 if !$conf->{hotplug};
+    die "internal error" if !$conf->{hotplug};
 
     my $devices_list = vm_devices_list($vmid);
     return 1 if !defined($devices_list->{$deviceid});
 
+    die "can't unplug bootdisk" if $conf->{bootdisk} && $conf->{bootdisk} eq $deviceid;
+
     if ($deviceid eq 'tablet') {
+
 	qemu_devicedel($vmid, $deviceid);
-	return 1;
-    }
 
-    die "can't unplug bootdisk" if $conf->{bootdisk} && $conf->{bootdisk} eq $deviceid;
+    } elsif ($deviceid =~ m/^(virtio)(\d+)$/) {
 
-    if ($deviceid =~ m/^(virtio)(\d+)$/) {
         qemu_devicedel($vmid, $deviceid);
-        return undef if !qemu_devicedelverify($vmid, $deviceid);
-        return undef if !qemu_drivedel($vmid, $deviceid);
-    }
-
-    if ($deviceid =~ m/^(lsi)(\d+)$/) {
-        return undef if !qemu_devicedel($vmid, $deviceid);
-    }
+        qemu_devicedelverify($vmid, $deviceid);
+        qemu_drivedel($vmid, $deviceid);
+   
+    } elsif ($deviceid =~ m/^(lsi)(\d+)$/) {
+    
+	qemu_devicedel($vmid, $deviceid);
+    
+    } elsif ($deviceid =~ m/^(scsi)(\d+)$/) {
 
-    if ($deviceid =~ m/^(scsi)(\d+)$/) {
-        return undef if !qemu_devicedel($vmid, $deviceid);
-        return undef if !qemu_drivedel($vmid, $deviceid);
-    }
+        qemu_devicedel($vmid, $deviceid);
+        qemu_drivedel($vmid, $deviceid);
+    
+    } elsif ($deviceid =~ m/^(net)(\d+)$/) {
 
-    if ($deviceid =~ m/^(net)(\d+)$/) {
         qemu_devicedel($vmid, $deviceid);
-        return undef if !qemu_devicedelverify($vmid, $deviceid);
-        return undef if !qemu_netdevdel($vmid, $deviceid);
+        qemu_devicedelverify($vmid, $deviceid);
+        qemu_netdevdel($vmid, $deviceid);
+
+    } else {
+	die "can't unplug device '$deviceid'\n";
     }
 
     return 1;
@@ -3156,8 +3159,8 @@ sub qemu_deviceadd {
 
 sub qemu_devicedel {
     my($vmid, $deviceid) = @_;
+
     my $ret = vm_mon_cmd($vmid, "device_del", id => $deviceid);
-    return 1;
 }
 
 sub qemu_driveadd {
@@ -3178,14 +3181,13 @@ sub qemu_drivedel {
 
     my $ret = vm_human_monitor_command($vmid, "drive_del drive-$deviceid");
     $ret =~ s/^\s+//;
-    if ($ret =~ m/Device \'.*?\' not found/s) {
-        # NB: device not found errors mean the drive was auto-deleted and we ignore the error
-    }
-    elsif ($ret ne "") {
-      syslog("err", "deleting drive $deviceid failed : $ret");
-      return undef;
-    }
-    return 1;
+    
+    return 1 if $ret eq "";
+  
+    # NB: device not found errors mean the drive was auto-deleted and we ignore the error
+    return 1 if $ret =~ m/Device \'.*?\' not found/s; 
+    
+    die "deleting drive $deviceid failed : $ret\n";
 }
 
 sub qemu_deviceaddverify {
@@ -3202,16 +3204,18 @@ sub qemu_deviceaddverify {
 
 
 sub qemu_devicedelverify {
-    my ($vmid,$deviceid) = @_;
+    my ($vmid, $deviceid) = @_;
+
+    # need to verify that the device is correctly removed as device_del 
+    # is async and empty return is not reliable
 
-    #need to verify the device is correctly remove as device_del is async and empty return is not reliable
     for (my $i = 0; $i <= 5; $i++) {
          my $devices_list = vm_devices_list($vmid);
          return 1 if !defined($devices_list->{$deviceid});
          sleep 1;
     }
-    syslog("err", "error on hot-unplugging device $deviceid");
-    return undef;
+
+    die "error on hot-unplugging device '$deviceid'\n";
 }
 
 sub qemu_findorcreatescsihw {
@@ -3266,7 +3270,6 @@ sub qemu_netdevdel {
     my ($vmid, $deviceid) = @_;
 
     vm_mon_cmd($vmid, "netdev_del", id => $deviceid);
-    return 1;
 }
 
 sub qemu_cpu_hotplug {
-- 
1.7.10.4