On Tue, Apr 20, 2010 at 11:06 AM, Iustin Pop <[email protected]> wrote: > On Mon, Apr 19, 2010 at 04:54:43PM +0100, Guido Trotter wrote: >> In order not to complicate to much the _ExecuteKVMRuntime function with >> nested try/except/finally/else constructs we move the actual >> runcmd+check call in a separate _RunKVMCmd function. >> >> Signed-off-by: Guido Trotter <[email protected]> >> --- >> lib/hypervisor/hv_kvm.py | 40 +++++++++++++++++++++++++++++++--------- >> 1 files changed, 31 insertions(+), 9 deletions(-) >> >> diff --git a/lib/hypervisor/hv_kvm.py b/lib/hypervisor/hv_kvm.py >> index b73e6c7..9c9e64d 100644 >> --- a/lib/hypervisor/hv_kvm.py >> +++ b/lib/hypervisor/hv_kvm.py >> @@ -37,6 +37,7 @@ from ganeti import constants >> from ganeti import errors >> from ganeti import serializer >> from ganeti import objects >> +from ganeti import uidpool >> from ganeti.hypervisor import hv_base >> >> >> @@ -548,6 +549,22 @@ class KVMHypervisor(hv_base.BaseHypervisor): >> kvm_nics = [objects.NIC.FromDict(snic) for snic in serialized_nics] >> return (kvm_cmd, kvm_nics, hvparams) >> >> + def _RunKVMCmd(self, name, kvm_cmd): >> + """Run the KVM cmd and check for errors >> + >> + �...@type name: string >> + �...@param name: instance name >> + �...@type kvm_cmd: list of strings >> + �...@param kvm_cmd: runcmd input for kvm >> + >> + """ >> + result = utils.RunCmd(kvm_cmd) >> + if result.failed: >> + raise errors.HypervisorError("Failed to start instance %s: %s (%s)" % >> + (name, result.fail_reason, >> result.output)) >> + if not self._InstancePidAlive(name)[2]: >> + 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 >> >> @@ -596,13 +613,20 @@ class KVMHypervisor(hv_base.BaseHypervisor): >> raise errors.HypervisorError("Failed to open VNC password file %s: >> %s" >> % (vnc_pwd_file, err)) >> >> - result = utils.RunCmd(kvm_cmd) >> - if result.failed: >> - raise errors.HypervisorError("Failed to start instance %s: %s (%s)" % >> - (name, result.fail_reason, >> result.output)) >> - >> - if not self._InstancePidAlive(name)[2]: >> - raise errors.HypervisorError("Failed to start instance %s" % name) >> + if security_model == constants.HT_SM_POOL: >> + uid = uidpool.RequestUnusedUid() >> + try: >> + username = pwd.getpwuid(uid.GetUid()).pw_name >> + kvm_cmd.extend(["-runas", username]) >> + self._RunKVMCmd(name, kvm_cmd) >> + except: >> + uidpool.ReleaseUid(uid) >> + raise >> + else: >> + uid.Unlock() >> + utils.WriteFile(self._InstanceUidFile(name), data=str(uid)) >> + else: >> + self._RunKVMCmd(name, kvm_cmd) > > I see a possible problem here wth Writing the uid file only at the end. > Could you move it after the _RunKVMcmd?
I think it's better there, because if the kvm machine is started but we fail to write the uid file it's "ok", while we shouldn't release the uid just because we failed writing the uid file. Thanks, Guido -- Subscription settings: http://groups.google.com/group/ganeti-devel/subscribe?hl=en
