On Wed, May 28, 2014 at 9:54 AM, 'Jose A. Lopes' via ganeti-devel <
[email protected]> wrote:

> The call to update the metadata must be done from inside the
> 'TemporaryDisk' because this call updates the instance configuration
> in the metadata daemon, and we want this configuration to contain
> information about the temporary disk that is being created.  If the
> call was outside the instance configuration would only contain
> information about the instance's default disk.
>
> Signed-off-by: Jose A. Lopes <[email protected]>
> ---
>  lib/cmdlib/instance.py | 17 +++++++++--------
>  1 file changed, 9 insertions(+), 8 deletions(-)
>
> diff --git a/lib/cmdlib/instance.py b/lib/cmdlib/instance.py
> index 36a7a5c..4b42483 100644
> --- a/lib/cmdlib/instance.py
> +++ b/lib/cmdlib/instance.py
> @@ -1571,6 +1571,11 @@ class LUInstanceCreate(LogicalUnit):
>                         instance,
>                         [(constants.DT_PLAIN, constants.DISK_RDWR,
> disk_size)],
>                         feedback_fn):
> +      self.UpdateInstanceOsInstallPackage(feedback_fn, instance)
> +      UpdateMetadata(feedback_fn, self.rpc, instance,
> +                     osparams_private=self.op.osparams_private,
> +                     osparams_secret=self.op.osparams_secret)
> +
>        feedback_fn("Activating instance disks")
>        StartInstanceDisks(self, instance, False)
>
> @@ -1749,20 +1754,16 @@ class LUInstanceCreate(LogicalUnit):
>        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)
> +
> +    UpdateMetadata(feedback_fn, self.rpc, iobj,
> +                   osparams_private=self.op.osparams_private,
> +                   osparams_secret=self.op.osparams_secret)
>

There are legitimate reasons to update metadata twice: to remove the
temporary disk information, to allow other scripts which are not yet
virtualized to see proper disk names, etc.

Which one of those are you addressing here? And please formulate the answer
as a comment in the code so this non-obvious behaviour gets documented ;)


>
>      assert not self.owned_locks(locking.LEVEL_NODE_RES)
>
> --
> 1.9.1.423.g4596e3a
>
>

Reply via email to