On Mon, Jun 11, 2012 at 1:57 PM, Iustin Pop <[email protected]> wrote:
> 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.
LGTM
Thanks,
Guido
> ---
> lib/hypervisor/hv_kvm.py | 31 ++++++++++++++++++++-----------
> 1 files changed, 20 insertions(+), 11 deletions(-)
>
> diff --git a/lib/hypervisor/hv_kvm.py b/lib/hypervisor/hv_kvm.py
> index 0e5dbfc..77b46b2 100644
> --- 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
> --
> 1.7.7.3
>
--
Guido Trotter
Google - Corporate Computing Services SRE
Google Ireland Ltd. : Registered in Ireland with company number 368047.
Gordon House, Barrow Street, Dublin 4, Ireland.
