On Thu, Sep 25, 2014 at 01:28:27PM +0200, Michele Tartara wrote:
> On Tue, Sep 23, 2014 at 4:10 PM, 'Aaron Karper' via ganeti-devel
> <[email protected]> wrote:
> > The disk templates are going away for the instances, so the new
> > definition of being diskless is to be actually without any disks
> > attached.
> >
> > Signed-off-by: Aaron Karper <[email protected]>
> > ---
> >  lib/cmdlib/cluster.py            | 5 ++---
> >  lib/cmdlib/instance.py           | 4 ++--
> >  lib/cmdlib/instance_operation.py | 2 +-
> >  lib/cmdlib/instance_storage.py   | 2 +-
> >  4 files changed, 6 insertions(+), 7 deletions(-)
> >
> > diff --git a/lib/cmdlib/cluster.py b/lib/cmdlib/cluster.py
> > index 8b2e3ae..14af210 100644
> > --- a/lib/cmdlib/cluster.py
> > +++ b/lib/cmdlib/cluster.py
> > @@ -3178,20 +3178,19 @@ class LUClusterVerifyGroup(LogicalUnit, 
> > _VerifyErrors):
> >      node_disks_dev_inst_only = {}
> >      diskless_instances = set()
> >      nodisk_instances = set()
> > -    diskless = constants.DT_DISKLESS
> >
> >      for nuuid in node_uuids:
> >        node_inst_uuids = list(itertools.chain(node_image[nuuid].pinst,
> >                                               node_image[nuuid].sinst))
> >        diskless_instances.update(uuid for uuid in node_inst_uuids
> > -                                if instanceinfo[uuid].disk_template == 
> > diskless)
> > +                                if not instanceinfo[uuid].disks)
> >        disks = [(inst_uuid, disk)
> >                 for inst_uuid in node_inst_uuids
> >                 for disk in self.cfg.GetInstanceDisks(inst_uuid)]
> >
> >        if not disks:
> >          nodisk_instances.update(uuid for uuid in node_inst_uuids
> > -                                if instanceinfo[uuid].disk_template != 
> > diskless)
> > +                                if instanceinfo[uuid].disks)
> >          # No need to collect data
> >          continue
> >
> > diff --git a/lib/cmdlib/instance.py b/lib/cmdlib/instance.py
> > index ca856cc..58e1924 100644
> > --- a/lib/cmdlib/instance.py
> > +++ b/lib/cmdlib/instance.py
> > @@ -1370,7 +1370,7 @@ class LUInstanceCreate(LogicalUnit):
> >      @param iobj: instance object
> >
> >      """
> > -    if iobj.disk_template != constants.DT_DISKLESS and not 
> > self.adopt_disks:
> > +    if iobj.disks and not self.adopt_disks:
> >        disks = self.cfg.GetInstanceDisks(iobj.uuid)
> >        if self.op.mode == constants.INSTANCE_CREATE:
> >          os_image = objects.GetOSImage(self.op.osparams)
> > @@ -3235,7 +3235,7 @@ class LUInstanceSetParams(LogicalUnit):
> >                                       " --no-wait-for-sync given.",
> >                                       errors.ECODE_INVAL)
> >
> > -    if self.op.disks and self.instance.disk_template == 
> > constants.DT_DISKLESS:
> > +    if self.op.disks and not self.instance.disks:
> >        raise errors.OpPrereqError("Disk operations not supported for"
> >                                   " diskless instances", errors.ECODE_INVAL)
> >
> > diff --git a/lib/cmdlib/instance_operation.py 
> > b/lib/cmdlib/instance_operation.py
> > index 585a704..048b1e4 100644
> > --- a/lib/cmdlib/instance_operation.py
> > +++ b/lib/cmdlib/instance_operation.py
> > @@ -352,7 +352,7 @@ class LUInstanceReinstall(LogicalUnit):
> >      CheckNodeOnline(self, instance.primary_node, "Instance primary node"
> >                      " offline, cannot reinstall")
> >
> > -    if instance.disk_template == constants.DT_DISKLESS:
> > +    if not instance.disks:
> >        raise errors.OpPrereqError("Instance '%s' has no disks" %
> >                                   self.op.instance_name,
> >                                   errors.ECODE_INVAL)
> > diff --git a/lib/cmdlib/instance_storage.py b/lib/cmdlib/instance_storage.py
> > index f12e160..7c0e032 100644
> > --- a/lib/cmdlib/instance_storage.py
> > +++ b/lib/cmdlib/instance_storage.py
> > @@ -917,7 +917,7 @@ class LUInstanceRecreateDisks(LogicalUnit):
> >      if not self.op.iallocator:
> >        CheckNodeOnline(self, primary_node)
> >
> > -    if instance.disk_template == constants.DT_DISKLESS:
> > +    if not instance.disks:
> >        raise errors.OpPrereqError("Instance '%s' has no disks" %
> >                                   self.op.instance_name, errors.ECODE_INVAL)
> >
> > --
> > 2.1.0.rc2.206.gedb03e5
> >
> 
> The changes to the code look ok. There's only one thing that is not
> clear to me: instance.disk_template needs to have a value, until it
> disappears completely.
> What happens now that you remove all uses of DT_DISKLESS? Is it going
> to still have that value but matter-of-factly "ignoring" it as a
> temporary situation?

Yes, the idea is to carry it around as long as possible. The change only
removes the checks, not the assignments. And for the checks it's
redundant information that can be extracted from the presence of disks.
Only the last change will actually break the compatibility (ideally).

> 
> Cheers,
> Michele
> 
> 
> -- 
> 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

Reply via email to