Move KVM keymap from GenerateRuntime to ExecuteRuntime

Per issue 243, "side-effects" are GenerateRuntime are bad as they
execute only on the initial node of the instance. By moving the
write-out of the keymap file to ExecuteRuntime, it will be done both
at start and at migrate time.

Furthermore, we update the docstring of GenerateKVMRuntime to explain
this, and add a fixme related to the spice per-interface binding.

Signed-off-by: Iustin Pop <iustin@google.com>
Reviewed-by: Guido Trotter <ultrotter@google.com>

diff --git a/lib/hypervisor/hv_kvm.py b/lib/hypervisor/hv_kvm.py
index 0e5dbfc..77b46b2 100644 (file)
--- a/lib/hypervisor/hv_kvm.py
+++ b/lib/hypervisor/hv_kvm.py
@@ -523,6 +523,13 @@ class KVMHypervisor(hv_base.BaseHypervisor):
   def _GenerateKVMRuntime(self, instance, block_devices, startup_paused):
     """Generate KVM information to start an instance.
 
+    @attention: this function must not have any side-effects; for
+        example, it must not write to the filesystem, or read values
+        from the current system the are expected to differ between
+        nodes, since it is only run once at instance startup;
+        actions/kvm arguments that can vary between systems should be
+        done in L{_ExecuteKVMRuntime}
+
     """
     _, v_major, v_min, _ = self._GetKVMVersion()
 
@@ -675,16 +682,6 @@ class KVMHypervisor(hv_base.BaseHypervisor):
     elif vnc_bind_address:
       kvm_cmd.extend(["-usbdevice", constants.HT_MOUSE_TABLET])
 
-    keymap = hvp[constants.HV_KEYMAP]
-    if keymap:
-      keymap_path = self._InstanceKeymapFile(instance.name)
-      # If a keymap file is specified, KVM won't use its internal defaults. By
-      # first including the "en-us" layout, an error on loading the actual
-      # layout (e.g. because it can't be found) won't lead to a non-functional
-      # keyboard. A keyboard with incorrect keys is still better than none.
-      utils.WriteFile(keymap_path, data="include en-us\ninclude %s\n" % keymap)
-      kvm_cmd.extend(["-k", keymap_path])
-
     if vnc_bind_address:
       if netutils.IP4Address.IsValid(vnc_bind_address):
         if instance.network_port > constants.VNC_BASE_PORT:
@@ -720,6 +717,8 @@ class KVMHypervisor(hv_base.BaseHypervisor):
 
       kvm_cmd.extend(["-vnc", vnc_arg])
     elif spice_bind:
+      # FIXME: this is wrong here; the iface ip address differs
+      # between systems, so it should be done in _ExecuteKVMRuntime
       if netutils.IsValidInterface(spice_bind):
         # The user specified a network interface, we have to figure out the IP
         # address.
@@ -845,7 +844,7 @@ class KVMHypervisor(hv_base.BaseHypervisor):
       raise errors.HypervisorError("Failed to start instance %s" % name)
 
   def _ExecuteKVMRuntime(self, instance, kvm_runtime, incoming=None):
-    """Execute a KVM cmd, after completing it with some last minute data
+    """Execute a KVM cmd, after completing it with some last minute data.
 
     @type incoming: tuple of strings
     @param incoming: (target_host_ip, port)
@@ -876,6 +875,16 @@ class KVMHypervisor(hv_base.BaseHypervisor):
     if security_model == constants.HT_SM_USER:
       kvm_cmd.extend(["-runas", conf_hvp[constants.HV_SECURITY_DOMAIN]])
 
+    keymap = conf_hvp[constants.HV_KEYMAP]
+    if keymap:
+      keymap_path = self._InstanceKeymapFile(name)
+      # If a keymap file is specified, KVM won't use its internal defaults. By
+      # first including the "en-us" layout, an error on loading the actual
+      # layout (e.g. because it can't be found) won't lead to a non-functional
+      # keyboard. A keyboard with incorrect keys is still better than none.
+      utils.WriteFile(keymap_path, data="include en-us\ninclude %s\n" % keymap)
+      kvm_cmd.extend(["-k", keymap_path])
+
     # We have reasons to believe changing something like the nic driver/type
     # upon migration won't exactly fly with the instance kernel, so for nic
     # related parameters we'll use up_hvp