On Tue, Apr 20, 2010 at 11:21:08AM +0100, Guido Trotter wrote: > 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.
OK then, LGTM. iustin -- Subscription settings: http://groups.google.com/group/ganeti-devel/subscribe?hl=en
