X-Git-Url: https://code.grnet.gr/git/ganeti-local/blobdiff_plain/6f4070cd32d7bf651d86cdac886162e1353a9d91..a80a1d162137c10322b29d24c1eb7be155c7f0c4:/lib/hypervisor/hv_kvm.py

diff --git a/lib/hypervisor/hv_kvm.py b/lib/hypervisor/hv_kvm.py
index a25c32d..4876b73 100644
--- a/lib/hypervisor/hv_kvm.py
+++ b/lib/hypervisor/hv_kvm.py
@@ -1,7 +1,7 @@
 #
 #
 
-# Copyright (C) 2008, 2009, 2010, 2011, 2012 Google Inc.
+# Copyright (C) 2008, 2009, 2010, 2011, 2012, 2013 Google Inc.
 #
 # This program is free software; you can redistribute it and/or modify
 # it under the terms of the GNU General Public License as published by
@@ -37,10 +37,15 @@ import shutil
 import socket
 import stat
 import StringIO
+from bitarray import bitarray
 try:
   import affinity   # pylint: disable=F0401
 except ImportError:
   affinity = None
+try:
+  import fdsend   # pylint: disable=F0401
+except ImportError:
+  fdsend = None
 
 from ganeti import utils
 from ganeti import constants
@@ -79,8 +84,151 @@ _SPICE_ADDITIONAL_PARAMS = frozenset([
   constants.HV_KVM_SPICE_USE_TLS,
   ])
 
+# Constant bitarray that reflects to a free pci slot
+# Use it with bitarray.search()
+_AVAILABLE_PCI_SLOT = bitarray("0")
+
+# below constants show the format of runtime file
+# the nics are in second possition, while the disks in 4th (last)
+# moreover disk entries are stored as a list of in tuples
+# (L{objects.Disk}, link_name)
+_KVM_NICS_RUNTIME_INDEX = 1
+_KVM_DISKS_RUNTIME_INDEX = 3
+_DEVICE_RUNTIME_INDEX = {
+  constants.HOTPLUG_TARGET_DISK: _KVM_DISKS_RUNTIME_INDEX,
+  constants.HOTPLUG_TARGET_NIC: _KVM_NICS_RUNTIME_INDEX
+  }
+_FIND_RUNTIME_ENTRY = {
+  constants.HOTPLUG_TARGET_NIC:
+    lambda nic, kvm_nics: [n for n in kvm_nics if n.uuid == nic.uuid],
+  constants.HOTPLUG_TARGET_DISK:
+    lambda disk, kvm_disks: [(d, l) for (d, l) in kvm_disks
+                             if d.uuid == disk.uuid]
+  }
+_RUNTIME_DEVICE = {
+  constants.HOTPLUG_TARGET_NIC: lambda d: d,
+  constants.HOTPLUG_TARGET_DISK: lambda (d, e): d
+  }
+_RUNTIME_ENTRY = {
+  constants.HOTPLUG_TARGET_NIC: lambda d, e: d,
+  constants.HOTPLUG_TARGET_DISK: lambda d, e: (d, e)
+  }
+
+
+def _GenerateDeviceKVMId(dev_type, dev):
+  """Helper function to generate a unique device name used by KVM
+
+  QEMU monitor commands use names to identify devices. Here we use their pci
+  slot and a part of their UUID to name them. dev.pci might be None for old
+  devices in the cluster.
+
+  @type dev_type: sting
+  @param dev_type: device type of param dev
+  @type dev: L{objects.Disk} or L{objects.NIC}
+  @param dev: the device object for which we generate a kvm name
+  @raise errors.HotplugError: in case a device has no pci slot (old devices)
+
+  """
+
+  if not dev.pci:
+    raise errors.HotplugError("Hotplug is not supported for %s with UUID %s" %
+                              (dev_type, dev.uuid))
+
+  return "%s-%s-pci-%d" % (dev_type.lower(), dev.uuid.split("-")[0], dev.pci)
+
+
+def _UpdatePCISlots(dev, pci_reservations):
+  """Update pci configuration for a stopped instance
+
+  If dev has a pci slot then reserve it, else find first available
+  in pci_reservations bitarray. It acts on the same objects passed
+  as params so there is no need to return anything.
+
+  @type dev: L{objects.Disk} or L{objects.NIC}
+  @param dev: the device object for which we update its pci slot
+  @type pci_reservations: bitarray
+  @param pci_reservations: existing pci reservations for an instance
+  @raise errors.HotplugError: in case an instance has all its slot occupied
+
+  """
+  if dev.pci:
+    free = dev.pci
+  else: # pylint: disable=E1103
+    [free] = pci_reservations.search(_AVAILABLE_PCI_SLOT, 1)
+    if not free:
+      raise errors.HypervisorError("All PCI slots occupied")
+    dev.pci = int(free)
+
+  pci_reservations[free] = True
+
+
+def _GetExistingDeviceInfo(dev_type, device, runtime):
+  """Helper function to get an existing device inside the runtime file
+
+  Used when an instance is running. Load kvm runtime file and search
+  for a device based on its type and uuid.
+
+  @type dev_type: sting
+  @param dev_type: device type of param dev
+  @type device: L{objects.Disk} or L{objects.NIC}
+  @param device: the device object for which we generate a kvm name
+  @type runtime: tuple (cmd, nics, hvparams, disks)
+  @param runtime: the runtime data to search for the device
+  @raise errors.HotplugError: in case the requested device does not
+    exist (e.g. device has been added without --hotplug option) or
+    device info has not pci slot (e.g. old devices in the cluster)
+
+  """
+  index = _DEVICE_RUNTIME_INDEX[dev_type]
+  found = _FIND_RUNTIME_ENTRY[dev_type](device, runtime[index])
+  if not found:
+    raise errors.HotplugError("Cannot find runtime info for %s with UUID %s" %
+                              (dev_type, device.uuid))
+
+  return found[0]
+
+
+def _AnalyzeSerializedRuntime(serialized_runtime):
+  """Return runtime entries for a serialized runtime file
 
-def _ProbeTapVnetHdr(fd):
+  @type serialized_runtime: string
+  @param serialized_runtime: raw text data read from actual runtime file
+  @return: (cmd, nics, hvparams, bdevs)
+  @rtype: list
+
+  """
+  loaded_runtime = serializer.Load(serialized_runtime)
+  if len(loaded_runtime) == 3:
+    serialized_disks = []
+    kvm_cmd, serialized_nics, hvparams = loaded_runtime
+  else:
+    kvm_cmd, serialized_nics, hvparams, serialized_disks = loaded_runtime
+
+  kvm_nics = [objects.NIC.FromDict(snic) for snic in serialized_nics]
+  kvm_disks = [(objects.Disk.FromDict(sdisk), link)
+               for sdisk, link in serialized_disks]
+
+  return (kvm_cmd, kvm_nics, hvparams, kvm_disks)
+
+
+def _GetTunFeatures(fd, _ioctl=fcntl.ioctl):
+  """Retrieves supported TUN features from file descriptor.
+
+  @see: L{_ProbeTapVnetHdr}
+
+  """
+  req = struct.pack("I", 0)
+  try:
+    buf = _ioctl(fd, TUNGETFEATURES, req)
+  except EnvironmentError, err:
+    logging.warning("ioctl(TUNGETFEATURES) failed: %s", err)
+    return None
+  else:
+    (flags, ) = struct.unpack("I", buf)
+    return flags
+
+
+def _ProbeTapVnetHdr(fd, _features_fn=_GetTunFeatures):
   """Check whether to enable the IFF_VNET_HDR flag.
 
   To do this, _all_ of the following conditions must be met:
@@ -97,20 +245,19 @@ def _ProbeTapVnetHdr(fd):
    @param fd: the file descriptor of /dev/net/tun
 
   """
-  req = struct.pack("I", 0)
-  try:
-    res = fcntl.ioctl(fd, TUNGETFEATURES, req)
-  except EnvironmentError:
-    logging.warning("TUNGETFEATURES ioctl() not implemented")
-    return False
+  flags = _features_fn(fd)
 
-  tunflags = struct.unpack("I", res)[0]
-  if tunflags & IFF_VNET_HDR:
-    return True
-  else:
-    logging.warning("Host does not support IFF_VNET_HDR, not enabling")
+  if flags is None:
+    # Not supported
     return False
 
+  result = bool(flags & IFF_VNET_HDR)
+
+  if not result:
+    logging.warning("Kernel does not support IFF_VNET_HDR, not enabling")
+
+  return result
+
 
 def _OpenTap(vnet_hdr=True):
   """Open a new tap device and return its file descriptor.
@@ -139,39 +286,15 @@ def _OpenTap(vnet_hdr=True):
 
   try:
     res = fcntl.ioctl(tapfd, TUNSETIFF, ifr)
-  except EnvironmentError:
-    raise errors.HypervisorError("Failed to allocate a new TAP device")
+  except EnvironmentError, err:
+    raise errors.HypervisorError("Failed to allocate a new TAP device: %s" %
+                                 err)
 
   # Get the interface name from the ioctl
   ifname = struct.unpack("16sh", res)[0].strip("\x00")
   return (ifname, tapfd)
 
 
-def _BuildNetworkEnv(name, network, gateway, network6, gateway6,
-                     network_type, mac_prefix, tags, env):
-  """Build environment variables concerning a Network.
-
-  """
-  if name:
-    env["NETWORK_NAME"] = name
-  if network:
-    env["NETWORK_SUBNET"] = network
-  if gateway:
-    env["NETWORK_GATEWAY"] = gateway
-  if network6:
-    env["NETWORK_SUBNET6"] = network6
-  if gateway6:
-    env["NETWORK_GATEWAY6"] = gateway6
-  if mac_prefix:
-    env["NETWORK_MAC_PREFIX"] = mac_prefix
-  if network_type:
-    env["NETWORK_TYPE"] = network_type
-  if tags:
-    env["NETWORK_TAGS"] = " ".join(tags)
-
-  return env
-
-
 class QmpMessage:
   """QEMU Messaging Protocol (QMP) message.
 
@@ -226,30 +349,15 @@ class QmpMessage:
     return self.data == other.data
 
 
-class QmpConnection:
-  """Connection to the QEMU Monitor using the QEMU Monitor Protocol (QMP).
-
-  """
-  _FIRST_MESSAGE_KEY = "QMP"
-  _EVENT_KEY = "event"
-  _ERROR_KEY = "error"
-  _RETURN_KEY = RETURN_KEY = "return"
-  _ACTUAL_KEY = ACTUAL_KEY = "actual"
-  _ERROR_CLASS_KEY = "class"
-  _ERROR_DATA_KEY = "data"
-  _ERROR_DESC_KEY = "desc"
-  _EXECUTE_KEY = "execute"
-  _ARGUMENTS_KEY = "arguments"
-  _CAPABILITIES_COMMAND = "qmp_capabilities"
-  _MESSAGE_END_TOKEN = "\r\n"
+class MonitorSocket(object):
   _SOCKET_TIMEOUT = 5
 
   def __init__(self, monitor_filename):
-    """Instantiates the QmpConnection object.
+    """Instantiates the MonitorSocket object.
 
     @type monitor_filename: string
     @param monitor_filename: the filename of the UNIX raw socket on which the
-                             QMP monitor is listening
+                             monitor (QMP or simple one) is listening
 
     """
     self.monitor_filename = monitor_filename
@@ -258,7 +366,6 @@ class QmpConnection:
     # in a reasonable amount of time
     self.sock.settimeout(self._SOCKET_TIMEOUT)
     self._connected = False
-    self._buf = ""
 
   def _check_socket(self):
     sock_stat = None
@@ -266,29 +373,27 @@ class QmpConnection:
       sock_stat = os.stat(self.monitor_filename)
     except EnvironmentError, err:
       if err.errno == errno.ENOENT:
-        raise errors.HypervisorError("No qmp socket found")
+        raise errors.HypervisorError("No monitor socket found")
       else:
-        raise errors.HypervisorError("Error checking qmp socket: %s",
+        raise errors.HypervisorError("Error checking monitor socket: %s",
                                      utils.ErrnoOrStr(err))
     if not stat.S_ISSOCK(sock_stat.st_mode):
-      raise errors.HypervisorError("Qmp socket is not a socket")
+      raise errors.HypervisorError("Monitor socket is not a socket")
 
   def _check_connection(self):
     """Make sure that the connection is established.
 
     """
     if not self._connected:
-      raise errors.ProgrammerError("To use a QmpConnection you need to first"
+      raise errors.ProgrammerError("To use a MonitorSocket you need to first"
                                    " invoke connect() on it")
 
   def connect(self):
-    """Connects to the QMP monitor.
+    """Connects to the monitor.
 
-    Connects to the UNIX socket and makes sure that we can actually send and
-    receive data to the kvm instance via QMP.
+    Connects to the UNIX socket
 
     @raise errors.HypervisorError: when there are communication errors
-    @raise errors.ProgrammerError: when there are data serialization errors
 
     """
     if self._connected:
@@ -303,12 +408,53 @@ class QmpConnection:
       raise errors.HypervisorError("Can't connect to qmp socket")
     self._connected = True
 
+  def close(self):
+    """Closes the socket
+
+    It cannot be used after this call.
+
+    """
+    self.sock.close()
+
+
+class QmpConnection(MonitorSocket):
+  """Connection to the QEMU Monitor using the QEMU Monitor Protocol (QMP).
+
+  """
+  _FIRST_MESSAGE_KEY = "QMP"
+  _EVENT_KEY = "event"
+  _ERROR_KEY = "error"
+  _RETURN_KEY = RETURN_KEY = "return"
+  _ACTUAL_KEY = ACTUAL_KEY = "actual"
+  _ERROR_CLASS_KEY = "class"
+  _ERROR_DATA_KEY = "data"
+  _ERROR_DESC_KEY = "desc"
+  _EXECUTE_KEY = "execute"
+  _ARGUMENTS_KEY = "arguments"
+  _CAPABILITIES_COMMAND = "qmp_capabilities"
+  _MESSAGE_END_TOKEN = "\r\n"
+
+  def __init__(self, monitor_filename):
+    super(QmpConnection, self).__init__(monitor_filename)
+    self._buf = ""
+
+  def connect(self):
+    """Connects to the QMP monitor.
+
+    Connects to the UNIX socket and makes sure that we can actually send and
+    receive data to the kvm instance via QMP.
+
+    @raise errors.HypervisorError: when there are communication errors
+    @raise errors.ProgrammerError: when there are data serialization errors
+
+    """
+    super(QmpConnection, self).connect()
     # Check if we receive a correct greeting message from the server
     # (As per the QEMU Protocol Specification 0.1 - section 2.2)
     greeting = self._Recv()
     if not greeting[self._FIRST_MESSAGE_KEY]:
       self._connected = False
-      raise errors.HypervisorError("kvm: qmp communication error (wrong"
+      raise errors.HypervisorError("kvm: QMP communication error (wrong"
                                    " server greeting")
 
     # Let's put the monitor in command mode using the qmp_capabilities
@@ -471,7 +617,7 @@ class KVMHypervisor(hv_base.BaseHypervisor):
     constants.HV_VNC_BIND_ADDRESS:
       (False, lambda x: (netutils.IP4Address.IsValid(x) or
                          utils.IsNormAbsPath(x)),
-       "the VNC bind address must be either a valid IP address or an absolute"
+       "The VNC bind address must be either a valid IP address or an absolute"
        " pathname", None, None),
     constants.HV_VNC_TLS: hv_base.NO_CHECK,
     constants.HV_VNC_X509: hv_base.OPT_DIR_CHECK,
@@ -481,7 +627,7 @@ class KVMHypervisor(hv_base.BaseHypervisor):
     constants.HV_KVM_SPICE_IP_VERSION:
       (False, lambda x: (x == constants.IFACE_NO_IP_VERSION_SPECIFIED or
                          x in constants.VALID_IP_VERSIONS),
-       "the SPICE IP version should be 4 or 6",
+       "The SPICE IP version should be 4 or 6",
        None, None),
     constants.HV_KVM_SPICE_PASSWORD_FILE: hv_base.OPT_FILE_CHECK,
     constants.HV_KVM_SPICE_LOSSLESS_IMG_COMPR:
@@ -541,8 +687,13 @@ class KVMHypervisor(hv_base.BaseHypervisor):
     constants.HV_VGA: hv_base.NO_CHECK,
     constants.HV_KVM_EXTRA: hv_base.NO_CHECK,
     constants.HV_KVM_MACHINE_VERSION: hv_base.NO_CHECK,
+    constants.HV_VNET_HDR: hv_base.NO_CHECK,
     }
 
+  _VIRTIO = "virtio"
+  _VIRTIO_NET_PCI = "virtio-net-pci"
+  _VIRTIO_BLK_PCI = "virtio-blk-pci"
+
   _MIGRATION_STATUS_RE = re.compile("Migration\s+status:\s+(\w+)",
                                     re.M | re.I)
   _MIGRATION_PROGRESS_RE = \
@@ -569,11 +720,23 @@ class KVMHypervisor(hv_base.BaseHypervisor):
   _ENABLE_KVM_RE = re.compile(r"^-enable-kvm\s", re.M)
   _DISABLE_KVM_RE = re.compile(r"^-disable-kvm\s", re.M)
   _NETDEV_RE = re.compile(r"^-netdev\s", re.M)
+  _DISPLAY_RE = re.compile(r"^-display\s", re.M)
+  _MACHINE_RE = re.compile(r"^-machine\s", re.M)
+  _VIRTIO_NET_RE = re.compile(r"^name \"%s\"" % _VIRTIO_NET_PCI, re.M)
+  _VIRTIO_BLK_RE = re.compile(r"^name \"%s\"" % _VIRTIO_BLK_PCI, re.M)
   # match  -drive.*boot=on|off on different lines, but in between accept only
   # dashes not preceeded by a new line (which would mean another option
   # different than -drive is starting)
   _BOOT_RE = re.compile(r"^-drive\s([^-]|(?<!^)-)*,boot=on\|off", re.M | re.S)
 
+  _INFO_PCI_RE = re.compile(r'Bus.*device[ ]*(\d+).*')
+  _INFO_PCI_CMD = "info pci"
+  _INFO_VERSION_RE = \
+    re.compile(r'^QEMU (\d+)\.(\d+)(\.(\d+))?.*monitor.*', re.M)
+  _INFO_VERSION_CMD = "info version"
+
+  _DEFAULT_PCI_RESERVATIONS = "11110000000000000000000000000000"
+
   ANCILLARY_FILES = [
     _KVM_NETWORK_SCRIPT,
     ]
@@ -584,9 +747,14 @@ class KVMHypervisor(hv_base.BaseHypervisor):
   # Supported kvm options to get output from
   _KVMOPT_HELP = "help"
   _KVMOPT_MLIST = "mlist"
+  _KVMOPT_DEVICELIST = "devicelist"
+
+  # Command to execute to get the output from kvm, and whether to
+  # accept the output even on failure.
   _KVMOPTS_CMDS = {
-    _KVMOPT_HELP: ["--help"],
-    _KVMOPT_MLIST: ["-M", "?"],
+    _KVMOPT_HELP: (["--help"], False),
+    _KVMOPT_MLIST: (["-M", "?"], False),
+    _KVMOPT_DEVICELIST: (["-device", "?"], True),
   }
 
   def __init__(self):
@@ -846,17 +1014,15 @@ class KVMHypervisor(hv_base.BaseHypervisor):
 
     if nic.network:
       n = objects.Network.FromDict(nic.netinfo)
-      _BuildNetworkEnv(nic.network, n.network, n.gateway,
-                       n.network6, n.gateway6, n.network_type,
-                       n.mac_prefix, n.tags, env)
+      env.update(n.HooksDict())
 
     if nic.nicparams[constants.NIC_MODE] == constants.NIC_MODE_BRIDGED:
       env["BRIDGE"] = nic.nicparams[constants.NIC_LINK]
 
     result = utils.RunCmd([pathutils.KVM_IFUP, tap], env=env)
     if result.failed:
-      raise errors.HypervisorError("Failed to configure interface %s: %s."
-                                   " Network configuration script output: %s" %
+      raise errors.HypervisorError("Failed to configure interface %s: %s;"
+                                   " network configuration script output: %s" %
                                    (tap, result.fail_reason, result.output))
 
   @staticmethod
@@ -1019,6 +1185,90 @@ class KVMHypervisor(hv_base.BaseHypervisor):
         data.append(info)
     return data
 
+  def _GenerateKVMBlockDevicesOptions(self, instance, kvm_disks,
+                                      kvmhelp, devlist):
+    """Generate KVM options regarding instance's block devices.
+
+    @type instance: L{objects.Instance}
+    @param instance: the instance object
+    @type kvm_disks: list of tuples
+    @param kvm_disks: list of tuples [(disk, link_name)..]
+    @type kvmhelp: string
+    @param kvmhelp: output of kvm --help
+    @type devlist: string
+    @param devlist: output of kvm -device ?
+    @rtype: list
+    @return: list of command line options eventually used by kvm executable
+
+    """
+    hvp = instance.hvparams
+    kernel_path = hvp[constants.HV_KERNEL_PATH]
+    if kernel_path:
+      boot_disk = False
+    else:
+      boot_disk = hvp[constants.HV_BOOT_ORDER] == constants.HT_BO_DISK
+
+    # whether this is an older KVM version that uses the boot=on flag
+    # on devices
+    needs_boot_flag = self._BOOT_RE.search(kvmhelp)
+
+    dev_opts = []
+    device_driver = None
+    disk_type = hvp[constants.HV_DISK_TYPE]
+    if disk_type == constants.HT_DISK_PARAVIRTUAL:
+      if_val = ",if=%s" % self._VIRTIO
+      try:
+        if self._VIRTIO_BLK_RE.search(devlist):
+          if_val = ",if=none"
+          # will be passed in -device option as driver
+          device_driver = self._VIRTIO_BLK_PCI
+      except errors.HypervisorError, _:
+        pass
+    else:
+      if_val = ",if=%s" % disk_type
+    # Cache mode
+    disk_cache = hvp[constants.HV_DISK_CACHE]
+    if instance.disk_template in constants.DTS_EXT_MIRROR:
+      if disk_cache != "none":
+        # TODO: make this a hard error, instead of a silent overwrite
+        logging.warning("KVM: overriding disk_cache setting '%s' with 'none'"
+                        " to prevent shared storage corruption on migration",
+                        disk_cache)
+      cache_val = ",cache=none"
+    elif disk_cache != constants.HT_CACHE_DEFAULT:
+      cache_val = ",cache=%s" % disk_cache
+    else:
+      cache_val = ""
+    for cfdev, link_name in kvm_disks:
+      if cfdev.mode != constants.DISK_RDWR:
+        raise errors.HypervisorError("Instance has read-only disks which"
+                                     " are not supported by KVM")
+      # TODO: handle FD_LOOP and FD_BLKTAP (?)
+      boot_val = ""
+      if boot_disk:
+        dev_opts.extend(["-boot", "c"])
+        boot_disk = False
+        if needs_boot_flag and disk_type != constants.HT_DISK_IDE:
+          boot_val = ",boot=on"
+      drive_val = "file=%s,format=raw%s%s%s" % \
+                  (dev_path, if_val, boot_val, cache_val)
+
+      if device_driver:
+        # kvm_disks are the 4th entry of runtime file that did not exist in
+        # the past. That means that cfdev should always have pci slot and
+        # _GenerateDeviceKVMId() will not raise a exception.
+        kvm_devid = _GenerateDeviceKVMId(constants.HOTPLUG_TARGET_DISK, cfdev)
+        drive_val += (",id=%s" % kvm_devid)
+        drive_val += (",bus=0,unit=%d" % cfdev.pci)
+        dev_val = ("%s,drive=%s,id=%s" %
+                   (device_driver, kvm_devid, kvm_devid))
+        dev_val += ",bus=pci.0,addr=%s" % hex(cfdev.pci)
+        dev_opts.extend(["-device", dev_val])
+
+      dev_opts.extend(["-drive", drive_val])
+
+    return dev_opts
+
   def _GenerateKVMRuntime(self, instance, block_devices, startup_paused,
                           kvmhelp):
     """Generate KVM information to start an instance.
@@ -1035,6 +1285,7 @@ class KVMHypervisor(hv_base.BaseHypervisor):
     """
     # pylint: disable=R0912,R0914,R0915
     hvp = instance.hvparams
+    self.ValidateParameters(hvp)
 
     pidfile = self._InstancePidFile(instance.name)
     kvm = hvp[constants.HV_KVM_PATH]
@@ -1065,29 +1316,36 @@ class KVMHypervisor(hv_base.BaseHypervisor):
     mversion = hvp[constants.HV_KVM_MACHINE_VERSION]
     if not mversion:
       mversion = self._GetDefaultMachineVersion(kvm)
-    kvm_cmd.extend(["-M", mversion])
+    if self._MACHINE_RE.search(kvmhelp):
+      # TODO (2.8): kernel_irqchip and kvm_shadow_mem machine properties, as
+      # extra hypervisor parameters. We should also investigate whether and how
+      # shadow_mem should be considered for the resource model.
+      if (hvp[constants.HV_KVM_FLAG] == constants.HT_KVM_ENABLED):
+        specprop = ",accel=kvm"
+      else:
+        specprop = ""
+      machinespec = "%s%s" % (mversion, specprop)
+      kvm_cmd.extend(["-machine", machinespec])
+    else:
+      kvm_cmd.extend(["-M", mversion])
+      if (hvp[constants.HV_KVM_FLAG] == constants.HT_KVM_ENABLED and
+          self._ENABLE_KVM_RE.search(kvmhelp)):
+        kvm_cmd.extend(["-enable-kvm"])
+      elif (hvp[constants.HV_KVM_FLAG] == constants.HT_KVM_DISABLED and
+            self._DISABLE_KVM_RE.search(kvmhelp)):
+        kvm_cmd.extend(["-disable-kvm"])
 
     kernel_path = hvp[constants.HV_KERNEL_PATH]
     if kernel_path:
-      boot_disk = boot_cdrom = boot_floppy = boot_network = False
+      boot_cdrom = boot_floppy = boot_network = False
     else:
-      boot_disk = hvp[constants.HV_BOOT_ORDER] == constants.HT_BO_DISK
       boot_cdrom = hvp[constants.HV_BOOT_ORDER] == constants.HT_BO_CDROM
       boot_floppy = hvp[constants.HV_BOOT_ORDER] == constants.HT_BO_FLOPPY
       boot_network = hvp[constants.HV_BOOT_ORDER] == constants.HT_BO_NETWORK
 
-    self.ValidateParameters(hvp)
-
     if startup_paused:
       kvm_cmd.extend([_KVM_START_PAUSED_FLAG])
 
-    if (hvp[constants.HV_KVM_FLAG] == constants.HT_KVM_ENABLED and
-        self._ENABLE_KVM_RE.search(kvmhelp)):
-      kvm_cmd.extend(["-enable-kvm"])
-    elif (hvp[constants.HV_KVM_FLAG] == constants.HT_KVM_DISABLED and
-          self._DISABLE_KVM_RE.search(kvmhelp)):
-      kvm_cmd.extend(["-disable-kvm"])
-
     if boot_network:
       kvm_cmd.extend(["-boot", "n"])
 
@@ -1096,38 +1354,6 @@ class KVMHypervisor(hv_base.BaseHypervisor):
     needs_boot_flag = self._BOOT_RE.search(kvmhelp)
 
     disk_type = hvp[constants.HV_DISK_TYPE]
-    if disk_type == constants.HT_DISK_PARAVIRTUAL:
-      if_val = ",if=virtio"
-    else:
-      if_val = ",if=%s" % disk_type
-    # Cache mode
-    disk_cache = hvp[constants.HV_DISK_CACHE]
-    if instance.disk_template in constants.DTS_EXT_MIRROR:
-      if disk_cache != "none":
-        # TODO: make this a hard error, instead of a silent overwrite
-        logging.warning("KVM: overriding disk_cache setting '%s' with 'none'"
-                        " to prevent shared storage corruption on migration",
-                        disk_cache)
-      cache_val = ",cache=none"
-    elif disk_cache != constants.HT_CACHE_DEFAULT:
-      cache_val = ",cache=%s" % disk_cache
-    else:
-      cache_val = ""
-    for cfdev, dev_path in block_devices:
-      if cfdev.mode != constants.DISK_RDWR:
-        raise errors.HypervisorError("Instance has read-only disks which"
-                                     " are not supported by KVM")
-      # TODO: handle FD_LOOP and FD_BLKTAP (?)
-      boot_val = ""
-      if boot_disk:
-        kvm_cmd.extend(["-boot", "c"])
-        boot_disk = False
-        if needs_boot_flag and disk_type != constants.HT_DISK_IDE:
-          boot_val = ",boot=on"
-
-      drive_val = "file=%s,format=raw%s%s%s" % (dev_path, if_val, boot_val,
-                                                cache_val)
-      kvm_cmd.extend(["-drive", drive_val])
 
     #Now we can specify a different device type for CDROM devices.
     cdrom_disk_type = hvp[constants.HV_KVM_CDROM_DISK_TYPE]
@@ -1223,9 +1449,9 @@ class KVMHypervisor(hv_base.BaseHypervisor):
           else:
             vnc_arg = "%s:%d" % (vnc_bind_address, display)
         else:
-          logging.error("Network port is not a valid VNC display (%d < %d)."
-                        " Not starting VNC", instance.network_port,
-                        constants.VNC_BASE_PORT)
+          logging.error("Network port is not a valid VNC display (%d < %d),"
+                        " not starting VNC",
+                        instance.network_port, constants.VNC_BASE_PORT)
           vnc_arg = "none"
 
         # Only allow tls and other option when not binding to a file, for now.
@@ -1261,7 +1487,7 @@ class KVMHypervisor(hv_base.BaseHypervisor):
         # have that kind of IP addresses, throw an exception
         if spice_ip_version != constants.IFACE_NO_IP_VERSION_SPECIFIED:
           if not addresses[spice_ip_version]:
-            raise errors.HypervisorError("spice: unable to get an IPv%s address"
+            raise errors.HypervisorError("SPICE: Unable to get an IPv%s address"
                                          " for %s" % (spice_ip_version,
                                                       spice_bind))
 
@@ -1278,7 +1504,7 @@ class KVMHypervisor(hv_base.BaseHypervisor):
         elif addresses[constants.IP6_VERSION]:
           spice_ip_version = constants.IP6_VERSION
         else:
-          raise errors.HypervisorError("spice: unable to get an IP address"
+          raise errors.HypervisorError("SPICE: Unable to get an IP address"
                                        " for %s" % (spice_bind))
 
         spice_address = addresses[spice_ip_version][0]
@@ -1333,15 +1559,22 @@ class KVMHypervisor(hv_base.BaseHypervisor):
         # Enable the spice agent communication channel between the host and the
         # agent.
         kvm_cmd.extend(["-device", "virtio-serial-pci"])
-        kvm_cmd.extend(["-device", "virtserialport,chardev=spicechannel0,"
-                                                   "name=com.redhat.spice.0"])
+        kvm_cmd.extend([
+          "-device",
+          "virtserialport,chardev=spicechannel0,name=com.redhat.spice.0",
+          ])
         kvm_cmd.extend(["-chardev", "spicevmc,id=spicechannel0,name=vdagent"])
 
       logging.info("KVM: SPICE will listen on port %s", instance.network_port)
       kvm_cmd.extend(["-spice", spice_arg])
 
     else:
-      kvm_cmd.extend(["-nographic"])
+      # From qemu 1.4 -nographic is incompatible with -daemonize. The new way
+      # also works in earlier versions though (tested with 1.1 and 1.3)
+      if self._DISPLAY_RE.search(kvmhelp):
+        kvm_cmd.extend(["-display", "none"])
+      else:
+        kvm_cmd.extend(["-nographic"])
 
     if hvp[constants.HV_USE_LOCALTIME]:
       kvm_cmd.extend(["-localtime"])
@@ -1370,14 +1603,22 @@ class KVMHypervisor(hv_base.BaseHypervisor):
         kvm_cmd.extend(["-usbdevice", dev])
 
     if hvp[constants.HV_KVM_EXTRA]:
-      kvm_cmd.extend([hvp[constants.HV_KVM_EXTRA]])
+      kvm_cmd.extend(hvp[constants.HV_KVM_EXTRA].split(" "))
+
+    pci_reservations = bitarray(self._DEFAULT_PCI_RESERVATIONS)
+    kvm_disks = []
+    for disk, link_name in block_devices:
+      _UpdatePCISlots(disk, pci_reservations)
+      kvm_disks.append((disk, link_name))
+
+    kvm_nics = []
+    for nic in instance.nics:
+      _UpdatePCISlots(nic, pci_reservations)
+      kvm_nics.append(nic)
 
-    # Save the current instance nics, but defer their expansion as parameters,
-    # as we'll need to generate executable temp files for them.
-    kvm_nics = instance.nics
     hvparams = hvp
 
-    return (kvm_cmd, kvm_nics, hvparams)
+    return (kvm_cmd, kvm_nics, hvparams, kvm_disks)
 
   def _WriteKVMRuntime(self, instance_name, data):
     """Write an instance's KVM runtime
@@ -1403,9 +1644,14 @@ class KVMHypervisor(hv_base.BaseHypervisor):
     """Save an instance's KVM runtime
 
     """
-    kvm_cmd, kvm_nics, hvparams = kvm_runtime
+    kvm_cmd, kvm_nics, hvparams, kvm_disks = kvm_runtime
+
     serialized_nics = [nic.ToDict() for nic in kvm_nics]
-    serialized_form = serializer.Dump((kvm_cmd, serialized_nics, hvparams))
+    serialized_disks = [(blk.ToDict(), link)
+                            for blk, link in kvm_disks]
+    serialized_form = serializer.Dump((kvm_cmd, serialized_nics, hvparams,
+                                      serialized_disks))
+
     self._WriteKVMRuntime(instance.name, serialized_form)
 
   def _LoadKVMRuntime(self, instance, serialized_runtime=None):
@@ -1414,10 +1660,8 @@ class KVMHypervisor(hv_base.BaseHypervisor):
     """
     if not serialized_runtime:
       serialized_runtime = self._ReadKVMRuntime(instance.name)
-    loaded_runtime = serializer.Load(serialized_runtime)
-    kvm_cmd, serialized_nics, hvparams = loaded_runtime
-    kvm_nics = [objects.NIC.FromDict(snic) for snic in serialized_nics]
-    return (kvm_cmd, kvm_nics, hvparams)
+
+    return _AnalyzeSerializedRuntime(serialized_runtime)
 
   def _RunKVMCmd(self, name, kvm_cmd, tap_fds=None):
     """Run the KVM cmd and check for errors
@@ -1442,6 +1686,8 @@ class KVMHypervisor(hv_base.BaseHypervisor):
     if not self._InstancePidAlive(name)[2]:
       raise errors.HypervisorError("Failed to start instance %s" % name)
 
+  # too many local variables
+  # pylint: disable=R0914
   def _ExecuteKVMRuntime(self, instance, kvm_runtime, kvmhelp, incoming=None):
     """Execute a KVM cmd, after completing it with some last minute data.
 
@@ -1465,11 +1711,11 @@ class KVMHypervisor(hv_base.BaseHypervisor):
 
     temp_files = []
 
-    kvm_cmd, kvm_nics, up_hvp = kvm_runtime
+    kvm_cmd, kvm_nics, up_hvp, kvm_disks = kvm_runtime
+    # the first element of kvm_cmd is always the path to the kvm binary
+    kvm_path = kvm_cmd[0]
     up_hvp = objects.FillDict(conf_hvp, up_hvp)
 
-    _, v_major, v_min, _ = self._ParseKVMVersion(kvmhelp)
-
     # We know it's safe to run as a different user upon migration, so we'll use
     # the latest conf, from conf_hvp.
     security_model = conf_hvp[constants.HV_SECURITY_MODEL]
@@ -1491,6 +1737,7 @@ class KVMHypervisor(hv_base.BaseHypervisor):
     # related parameters we'll use up_hvp
     tapfds = []
     taps = []
+    devlist = self._GetKVMOutput(kvm_path, self._KVMOPT_DEVICELIST)
     if not kvm_nics:
       kvm_cmd.extend(["-net", "none"])
     else:
@@ -1498,12 +1745,15 @@ class KVMHypervisor(hv_base.BaseHypervisor):
       tap_extra = ""
       nic_type = up_hvp[constants.HV_NIC_TYPE]
       if nic_type == constants.HT_NIC_PARAVIRTUAL:
-        # From version 0.12.0, kvm uses a new sintax for network configuration.
-        if (v_major, v_min) >= (0, 12):
-          nic_model = "virtio-net-pci"
-          vnet_hdr = True
-        else:
-          nic_model = "virtio"
+        nic_model = self._VIRTIO
+        try:
+          if self._VIRTIO_NET_RE.search(devlist):
+            nic_model = self._VIRTIO_NET_PCI
+            vnet_hdr = up_hvp[constants.HV_VNET_HDR]
+        except errors.HypervisorError, _:
+          # Older versions of kvm don't support DEVICE_LIST, but they don't
+          # have new virtio syntax either.
+          pass
 
         if up_hvp[constants.HV_VHOST_NET]:
           # check for vhost_net support
@@ -1518,12 +1768,22 @@ class KVMHypervisor(hv_base.BaseHypervisor):
       kvm_supports_netdev = self._NETDEV_RE.search(kvmhelp)
 
       for nic_seq, nic in enumerate(kvm_nics):
-        tapname, tapfd = _OpenTap(vnet_hdr)
+        tapname, tapfd = _OpenTap(vnet_hdr=vnet_hdr)
         tapfds.append(tapfd)
         taps.append(tapname)
         if kvm_supports_netdev:
-          nic_val = "%s,mac=%s,netdev=netdev%s" % (nic_model, nic.mac, nic_seq)
-          tap_val = "type=tap,id=netdev%s,fd=%d%s" % (nic_seq, tapfd, tap_extra)
+          nic_val = "%s,mac=%s" % (nic_model, nic.mac)
+          try:
+            # kvm_nics already exist in old runtime files and thus there might
+            # be some entries without pci slot (therefore try: except:)
+            kvm_devid = _GenerateDeviceKVMId(constants.HOTPLUG_TARGET_NIC, nic)
+            netdev = kvm_devid
+            nic_val += (",id=%s,bus=pci.0,addr=%s" % (kvm_devid, hex(nic.pci)))
+          except errors.HotplugError:
+            netdev = "netdev%d" % nic_seq
+          nic_val += (",netdev=%s" % netdev)
+          tap_val = ("type=tap,id=%s,fd=%d%s" %
+                     (netdev, tapfd, tap_extra))
           kvm_cmd.extend(["-netdev", tap_val, "-device", nic_val])
         else:
           nic_val = "nic,vlan=%s,macaddr=%s,model=%s" % (nic_seq,
@@ -1565,6 +1825,11 @@ class KVMHypervisor(hv_base.BaseHypervisor):
         continue
       self._ConfigureNIC(instance, nic_seq, nic, taps[nic_seq])
 
+    bdev_opts = self._GenerateKVMBlockDevicesOptions(instance,
+                                                     kvm_disks,
+                                                     kvmhelp,
+                                                     devlist)
+    kvm_cmd.extend(bdev_opts)
     # CPU affinity requires kvm to start paused, so we set this flag if the
     # instance is not already paused and if we are not going to accept a
     # migrating instance. In the latter case, pausing is not needed.
@@ -1658,24 +1923,187 @@ class KVMHypervisor(hv_base.BaseHypervisor):
     self._SaveKVMRuntime(instance, kvm_runtime)
     self._ExecuteKVMRuntime(instance, kvm_runtime, kvmhelp)
 
-  def _CallMonitorCommand(self, instance_name, command):
+  def _CallMonitorCommand(self, instance_name, command, timeout=None):
     """Invoke a command on the instance monitor.
 
     """
-    socat = ("echo %s | %s STDIO UNIX-CONNECT:%s" %
+    if timeout is not None:
+      timeout_cmd = "timeout %s" % (timeout, )
+    else:
+      timeout_cmd = ""
+
+    # TODO: Replace monitor calls with QMP once KVM >= 0.14 is the minimum
+    # version. The monitor protocol is designed for human consumption, whereas
+    # QMP is made for programmatic usage. In the worst case QMP can also
+    # execute monitor commands. As it is, all calls to socat take at least
+    # 500ms and likely more: socat can't detect the end of the reply and waits
+    # for 500ms of no data received before exiting (500 ms is the default for
+    # the "-t" parameter).
+    socat = ("echo %s | %s %s STDIO UNIX-CONNECT:%s" %
              (utils.ShellQuote(command),
+              timeout_cmd,
               constants.SOCAT_PATH,
               utils.ShellQuote(self._InstanceMonitor(instance_name))))
+
     result = utils.RunCmd(socat)
     if result.failed:
-      msg = ("Failed to send command '%s' to instance %s."
-             " output: %s, error: %s, fail_reason: %s" %
-             (command, instance_name,
-              result.stdout, result.stderr, result.fail_reason))
+      msg = ("Failed to send command '%s' to instance '%s', reason '%s',"
+             " output: %s" %
+             (command, instance_name, result.fail_reason, result.output))
       raise errors.HypervisorError(msg)
 
     return result
 
+  def _GetFreePCISlot(self, instance, dev):
+    """Get the first available pci slot of a runnung instance.
+
+    """
+    slots = bitarray(32)
+    slots.setall(False) # pylint: disable=E1101
+    output = self._CallMonitorCommand(instance.name, self._INFO_PCI_CMD)
+    for line in output.stdout.splitlines():
+      match = self._INFO_PCI_RE.search(line)
+      if match:
+        slot = int(match.group(1))
+        slots[slot] = True
+
+    [free] = slots.search(_AVAILABLE_PCI_SLOT, 1) # pylint: disable=E1101
+    if not free:
+      raise errors.HypervisorError("All PCI slots occupied")
+
+    dev.pci = int(free)
+
+  def VerifyHotplugSupport(self, instance, action, dev_type):
+    """Verifies that hotplug is supported.
+
+    Hotplug is *not* supported in case of:
+     - qemu versions < 1.0
+     - security models and chroot (disk hotplug)
+     - fdsend module is missing (nic hot-add)
+
+    @raise errors.HypervisorError: in one of the previous cases
+
+    """
+    output = self._CallMonitorCommand(instance.name, self._INFO_VERSION_CMD)
+    # TODO: search for netdev_add, drive_add, device_add.....
+    match = self._INFO_VERSION_RE.search(output.stdout)
+    if not match:
+      raise errors.HotplugError("Try hotplug only in running instances.")
+    v_major, v_min, _, _ = match.groups()
+    if (int(v_major), int(v_min)) < (1, 0):
+      raise errors.HotplugError("Hotplug not supported for qemu versions < 1.0")
+
+    if dev_type == constants.HOTPLUG_TARGET_DISK:
+      hvp = instance.hvparams
+      security_model = hvp[constants.HV_SECURITY_MODEL]
+      use_chroot = hvp[constants.HV_KVM_USE_CHROOT]
+      if use_chroot:
+        raise errors.HotplugError("Disk hotplug is not supported"
+                                  " in case of chroot.")
+      if security_model != constants.HT_SM_NONE:
+        raise errors.HotplugError("Disk Hotplug is not supported in case"
+                                  " security models are used.")
+
+    if (dev_type == constants.HOTPLUG_TARGET_NIC and
+        action == constants.HOTPLUG_ACTION_ADD and not fdsend):
+      raise errors.HotplugError("Cannot hot-add NIC."
+                                " fdsend python module is missing.")
+
+  def _CallHotplugCommand(self, name, cmd):
+    output = self._CallMonitorCommand(name, cmd)
+    # TODO: parse output and check if succeeded
+    for line in output.stdout.splitlines():
+      logging.info("%s", line)
+
+  def HotAddDevice(self, instance, dev_type, device, extra, seq):
+    """ Helper method to hot-add a new device
+
+    It gets free pci slot generates the device name and invokes the
+    device specific method.
+
+    """
+    # in case of hot-mod this is given
+    if device.pci is None:
+      self._GetFreePCISlot(instance, device)
+    kvm_devid = _GenerateDeviceKVMId(dev_type, device)
+    runtime = self._LoadKVMRuntime(instance)
+    if dev_type == constants.HOTPLUG_TARGET_DISK:
+      command = "drive_add dummy file=%s,if=none,id=%s,format=raw\n" % \
+                 (extra, kvm_devid)
+      command += ("device_add virtio-blk-pci,bus=pci.0,addr=%s,drive=%s,id=%s" %
+                  (hex(device.pci), kvm_devid, kvm_devid))
+    elif dev_type == constants.HOTPLUG_TARGET_NIC:
+      (tap, fd) = _OpenTap()
+      self._ConfigureNIC(instance, seq, device, tap)
+      self._PassTapFd(instance, fd, device)
+      command = "netdev_add tap,id=%s,fd=%s\n" % (kvm_devid, kvm_devid)
+      args = "virtio-net-pci,bus=pci.0,addr=%s,mac=%s,netdev=%s,id=%s" % \
+               (hex(device.pci), device.mac, kvm_devid, kvm_devid)
+      command += "device_add %s" % args
+      utils.WriteFile(self._InstanceNICFile(instance.name, seq), data=tap)
+
+    self._CallHotplugCommand(instance.name, command)
+    # update relevant entries in runtime file
+    index = _DEVICE_RUNTIME_INDEX[dev_type]
+    entry = _RUNTIME_ENTRY[dev_type](device, extra)
+    runtime[index].append(entry)
+    self._SaveKVMRuntime(instance, runtime)
+
+  def HotDelDevice(self, instance, dev_type, device, _, seq):
+    """ Helper method for hot-del device
+
+    It gets device info from runtime file, generates the device name and
+    invokes the device specific method.
+
+    """
+    runtime = self._LoadKVMRuntime(instance)
+    entry = _GetExistingDeviceInfo(dev_type, device, runtime)
+    kvm_device = _RUNTIME_DEVICE[dev_type](entry)
+    kvm_devid = _GenerateDeviceKVMId(dev_type, kvm_device)
+    if dev_type == constants.HOTPLUG_TARGET_DISK:
+      command = "device_del %s" % kvm_devid
+    elif dev_type == constants.HOTPLUG_TARGET_NIC:
+      command = "device_del %s\n" % kvm_devid
+      command += "netdev_del %s" % kvm_devid
+      utils.RemoveFile(self._InstanceNICFile(instance.name, seq))
+    self._CallHotplugCommand(instance.name, command)
+    index = _DEVICE_RUNTIME_INDEX[dev_type]
+    runtime[index].remove(entry)
+    self._SaveKVMRuntime(instance, runtime)
+
+    return kvm_device.pci
+
+  def HotModDevice(self, instance, dev_type, device, _, seq):
+    """ Helper method for hot-mod device
+
+    It gets device info from runtime file, generates the device name and
+    invokes the device specific method. Currently only NICs support hot-mod
+
+    """
+    if dev_type == constants.HOTPLUG_TARGET_NIC:
+      # putting it back in the same pci slot
+      device.pci = self.HotDelDevice(instance, dev_type, device, _, seq)
+      # TODO: remove sleep when socat gets removed
+      time.sleep(2)
+      self.HotAddDevice(instance, dev_type, device, _, seq)
+
+  def _PassTapFd(self, instance, fd, nic):
+    """Pass file descriptor to kvm process via monitor socket using SCM_RIGHTS
+
+    """
+    # TODO: factor out code related to unix sockets.
+    #       squash common parts between monitor and qmp
+    kvm_devid = _GenerateDeviceKVMId(constants.HOTPLUG_TARGET_NIC, nic)
+    command = "getfd %s\n" % kvm_devid
+    fds = [fd]
+    logging.info("%s", fds)
+    try:
+      monsock = MonitorSocket(self._InstanceMonitor(instance.name))
+      monsock.connect()
+      fdsend.sendfds(monsock.sock, command, fds=fds)
+    finally:
+      monsock.close()
+
   @classmethod
   def _ParseKVMVersion(cls, text):
     """Parse the KVM version from the --help output.
@@ -1703,16 +2131,22 @@ class KVMHypervisor(hv_base.BaseHypervisor):
   def _GetKVMOutput(cls, kvm_path, option):
     """Return the output of a kvm invocation
 
+    @type kvm_path: string
+    @param kvm_path: path to the kvm executable
+    @type option: a key of _KVMOPTS_CMDS
+    @param option: kvm option to fetch the output from
     @return: output a supported kvm invocation
     @raise errors.HypervisorError: when the KVM help output cannot be retrieved
 
     """
     assert option in cls._KVMOPTS_CMDS, "Invalid output option"
 
-    result = utils.RunCmd([kvm_path] + cls._KVMOPTS_CMDS[option])
-    if result.failed:
-      raise errors.HypervisorError("Unable to get KVM % output" %
-                                    ' '.join(cls._KVMOPTS_CMDS[option]))
+    optlist, can_fail = cls._KVMOPTS_CMDS[option]
+
+    result = utils.RunCmd([kvm_path] + optlist)
+    if result.failed and not can_fail:
+      raise errors.HypervisorError("Unable to get KVM %s output" %
+                                    " ".join(optlist))
     return result.output
 
   @classmethod
@@ -1737,10 +2171,13 @@ class KVMHypervisor(hv_base.BaseHypervisor):
     else:
       return "pc"
 
-  def StopInstance(self, instance, force=False, retry=False, name=None):
+  def StopInstance(self, instance, force=False, retry=False, name=None,
+                   timeout=None):
     """Stop an instance.
 
     """
+    assert(timeout is None or force is not None)
+
     if name is not None and not force:
       raise errors.HypervisorError("Cannot shutdown cleanly by name only")
     if name is None:
@@ -1753,7 +2190,7 @@ class KVMHypervisor(hv_base.BaseHypervisor):
       if force or not acpi:
         utils.KillProcess(pid)
       else:
-        self._CallMonitorCommand(name, "system_powerdown")
+        self._CallMonitorCommand(name, "system_powerdown", timeout)
 
   def CleanupInstance(self, instance_name):
     """Cleanup after a stopped instance
@@ -2009,16 +2446,22 @@ class KVMHypervisor(hv_base.BaseHypervisor):
   def Verify(self):
     """Verify the hypervisor.
 
-    Check that the binary exists.
+    Check that the required binaries exist.
+
+    @return: Problem description if something is wrong, C{None} otherwise
 
     """
-    # FIXME: this is the global kvm version, but the actual version can be
-    # customized as an hv parameter. we should use the nodegroup's default kvm
-    # path parameter here.
+    msgs = []
+    # FIXME: this is the global kvm binary, but the actual path can be
+    # customized as an hv parameter; we should use the nodegroup's
+    # default kvm path parameter here.
     if not os.path.exists(constants.KVM_PATH):
-      return "The kvm binary ('%s') does not exist." % constants.KVM_PATH
+      msgs.append("The KVM binary ('%s') does not exist" % constants.KVM_PATH)
     if not os.path.exists(constants.SOCAT_PATH):
-      return "The socat binary ('%s') does not exist." % constants.SOCAT_PATH
+      msgs.append("The socat binary ('%s') does not exist" %
+                  constants.SOCAT_PATH)
+
+    return self._FormatVerifyResults(msgs)
 
   @classmethod
   def CheckParameterSyntax(cls, hvparams):
@@ -2076,13 +2519,13 @@ class KVMHypervisor(hv_base.BaseHypervisor):
         # IP of that family
         if (netutils.IP4Address.IsValid(spice_bind) and
             spice_ip_version != constants.IP4_VERSION):
-          raise errors.HypervisorError("spice: got an IPv4 address (%s), but"
+          raise errors.HypervisorError("SPICE: Got an IPv4 address (%s), but"
                                        " the specified IP version is %s" %
                                        (spice_bind, spice_ip_version))
 
         if (netutils.IP6Address.IsValid(spice_bind) and
             spice_ip_version != constants.IP6_VERSION):
-          raise errors.HypervisorError("spice: got an IPv6 address (%s), but"
+          raise errors.HypervisorError("SPICE: Got an IPv6 address (%s), but"
                                        " the specified IP version is %s" %
                                        (spice_bind, spice_ip_version))
     else:
@@ -2090,7 +2533,7 @@ class KVMHypervisor(hv_base.BaseHypervisor):
       # error if any of them is set without it.
       for param in _SPICE_ADDITIONAL_PARAMS:
         if hvparams[param]:
-          raise errors.HypervisorError("spice: %s requires %s to be set" %
+          raise errors.HypervisorError("SPICE: %s requires %s to be set" %
                                        (param, constants.HV_KVM_SPICE_BIND))
 
   @classmethod
@@ -2119,21 +2562,21 @@ class KVMHypervisor(hv_base.BaseHypervisor):
     if spice_bind:
       # only one of VNC and SPICE can be used currently.
       if hvparams[constants.HV_VNC_BIND_ADDRESS]:
-        raise errors.HypervisorError("both SPICE and VNC are configured, but"
+        raise errors.HypervisorError("Both SPICE and VNC are configured, but"
                                      " only one of them can be used at a"
-                                     " given time.")
+                                     " given time")
 
       # check that KVM supports SPICE
       kvmhelp = cls._GetKVMOutput(kvm_path, cls._KVMOPT_HELP)
       if not cls._SPICE_RE.search(kvmhelp):
-        raise errors.HypervisorError("spice is configured, but it is not"
-                                     " supported according to kvm --help")
+        raise errors.HypervisorError("SPICE is configured, but it is not"
+                                     " supported according to 'kvm --help'")
 
       # if spice_bind is not an IP address, it must be a valid interface
-      bound_to_addr = (netutils.IP4Address.IsValid(spice_bind)
-                       or netutils.IP6Address.IsValid(spice_bind))
+      bound_to_addr = (netutils.IP4Address.IsValid(spice_bind) or
+                       netutils.IP6Address.IsValid(spice_bind))
       if not bound_to_addr and not netutils.IsValidInterface(spice_bind):
-        raise errors.HypervisorError("spice: the %s parameter must be either"
+        raise errors.HypervisorError("SPICE: The %s parameter must be either"
                                      " a valid IP address or interface name" %
                                      constants.HV_KVM_SPICE_BIND)