Ok, LGTM
On Wed, May 14, 2014 at 10:42 AM, Jose A. Lopes <[email protected]>wrote: > On May 13 19:45, Hrvoje Ribicic wrote: > > On Tue, May 13, 2014 at 10:43 AM, 'Jose A. Lopes' via ganeti-devel < > > [email protected]> wrote: > > > > > This patch gives just the code structure. Implementation will follow. > > > > > > Signed-off-by: Jose A. Lopes <[email protected]> > > > --- > > > lib/cmdlib/instance.py | 41 ++++++++++++++++++++++++++++++++++------- > > > 1 file changed, 34 insertions(+), 7 deletions(-) > > > > > > diff --git a/lib/cmdlib/instance.py b/lib/cmdlib/instance.py > > > index ee6a8eb..94818b4 100644 > > > --- a/lib/cmdlib/instance.py > > > +++ b/lib/cmdlib/instance.py > > > @@ -40,6 +40,7 @@ from ganeti import pathutils > > > from ganeti import serializer > > > import ganeti.rpc.node as rpc > > > from ganeti import utils > > > +from ganeti.utils import retry > > > > > > from ganeti.cmdlib.base import NoHooksLU, LogicalUnit, ResultWithJobs > > > > > > @@ -50,13 +51,14 @@ from ganeti.cmdlib.common import INSTANCE_DOWN, \ > > > IsExclusiveStorageEnabledNode, CheckHVParams, CheckOSParams, > > > CheckOSImage, \ > > > AnnotateDiskParams, GetUpdatedParams, ExpandInstanceUuidAndName, \ > > > ComputeIPolicySpecViolation, CheckInstanceState, > ExpandNodeUuidAndName, > > > \ > > > - CheckDiskTemplateEnabled, IsValidDiskAccessModeCombination > > > + CheckDiskTemplateEnabled, IsValidDiskAccessModeCombination, \ > > > + DetermineImageSize, IsInstanceRunning > > > from ganeti.cmdlib.instance_storage import CreateDisks, \ > > > CheckNodesFreeDiskPerVG, WipeDisks, WipeOrCleanupDisks, ImageDisks, > \ > > > WaitForSync, IsExclusiveStorageEnabledNodeUuid, > CreateSingleBlockDev, \ > > > ComputeDisks, CheckRADOSFreeSpace, ComputeDiskSizePerVG, \ > > > GenerateDiskTemplate, StartInstanceDisks, ShutdownInstanceDisks, \ > > > - AssembleInstanceDisks, CheckSpindlesExclusiveStorage > > > + AssembleInstanceDisks, CheckSpindlesExclusiveStorage, TemporaryDisk > > > from ganeti.cmdlib.instance_utils import > BuildInstanceHookEnvByObject, \ > > > GetClusterDomainSecret, BuildInstanceHookEnv, NICListToTuple, \ > > > NICToTuple, CheckNodeNotDrained, RemoveInstance, CopyLockList, \ > > > @@ -1699,7 +1701,36 @@ class LUInstanceCreate(LogicalUnit): > > > # Release all node resource locks > > > ReleaseLocks(self, locking.LEVEL_NODE_RES) > > > > > > - self.RunOsScripts(feedback_fn, iobj) > > > + if iobj.os: > > > + result = > > > self.rpc.call_os_diagnose([iobj.primary_node])[iobj.primary_node] > > > + result.Raise("Failed to get OS '%s'" % iobj.os) > > > + > > > + trusted = None > > > + > > > + for (name, _, _, _, _, _, _, os_trusted) in result.payload: > > > + if name == objects.OS.GetName(iobj.os): > > > + trusted = os_trusted > > > > > > > Just out of curiosity: is there any special reason why there is no break > in > > the loop here? > > It's not a logical error, just that most programmers would put it there > :) > > > > diff --git a/lib/cmdlib/instance.py b/lib/cmdlib/instance.py > index eab47d6..703d4be 100644 > --- a/lib/cmdlib/instance.py > +++ b/lib/cmdlib/instance.py > @@ -1744,6 +1744,7 @@ class LUInstanceCreate(LogicalUnit): > for (name, _, _, _, _, _, _, os_trusted) in result.payload: > if name == objects.OS.GetName(iobj.os): > trusted = os_trusted > + break > > if trusted is None: > raise errors.OpPrereqError("OS '%s' is not available in node > '%s'" % > > > > > > > + > > > + if trusted is None: > > > + raise errors.OpPrereqError("OS '%s' is not available in node > > > '%s'" % > > > + (iobj.os, iobj.primary_node)) > > > + elif trusted: > > > + self.RunOsScripts(feedback_fn, iobj) > > > + else: > > > + self.UpdateInstanceOsInstallPackage(feedback_fn, iobj) > > > + UpdateMetadata(feedback_fn, self.rpc, iobj, > > > + osparams_private=self.op.osparams_private, > > > + osparams_secret=self.op.osparams_secret) > > > + self.RunOsScriptsVirtualized(feedback_fn, iobj) > > > + # Instance is modified by 'RunOsScriptsVirtualized', > > > + # therefore, it must be retrieved once again from the > > > + # configuration, otherwise there will be a config object > > > + # version mismatch. > > > + iobj = self.cfg.GetInstanceInfo(iobj.uuid) > > > > > + else: > > > + UpdateMetadata(feedback_fn, self.rpc, iobj, > > > + osparams_private=self.op.osparams_private, > > > + osparams_secret=self.op.osparams_secret) > > > > > > > > The metadata update was not conditional before - why is it conditional > now? > > > > The metadata daemon must be notified before running the OS scripts in > order to give it the path to the OS install package. > > > > > > assert not self.owned_locks(locking.LEVEL_NODE_RES) > > > > > > @@ -1713,10 +1744,6 @@ class LUInstanceCreate(LogicalUnit): > > > False, self.op.reason) > > > result.Raise("Could not start instance") > > > > > > - UpdateMetadata(feedback_fn, self.rpc, iobj, > > > - osparams_private=self.op.osparams_private, > > > - osparams_secret=self.op.osparams_secret) > > > - > > > return > > > self.cfg.GetNodeNames(list(self.cfg.GetInstanceNodes(iobj.uuid))) > > > > > > > > > -- > > > 1.9.1.423.g4596e3a > > > > > > > > -- > Jose Antonio Lopes > Ganeti Engineering > Google Germany GmbH > Dienerstr. 12, 80331, München > > Registergericht und -nummer: Hamburg, HRB 86891 > Sitz der Gesellschaft: Hamburg > Geschäftsführer: Graham Law, Christine Elizabeth Flores > Steuernummer: 48/725/00206 > Umsatzsteueridentifikationsnummer: DE813741370 >
