On Thu, Apr 17, 2014 at 01:09PM, Jose A. Lopes wrote:
> On Apr 16 15:19, Ilias Tsitsimpis wrote:
> > Implement functions 'RemoveDisk' and 'DetachInstanceDisk'. The first one
> > removes a disk from the config file and the second one detaches a disk
> > from an instance. A wrapper 'RemoveInstanceDisk' is provided in order to
> > detach a disk from an instance and remove it at once (since Ganeti
> > doesn't support dangling disks yet).
> > 
> > Signed-off-by: Ilias Tsitsimpis <[email protected]>
> > ---
> >  lib/config.py | 53 +++++++++++++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 53 insertions(+)
> > 
> > diff --git a/lib/config.py b/lib/config.py
> > index c0359c5..d59e9a6 100644
> > --- a/lib/config.py
> > +++ b/lib/config.py
> > @@ -455,6 +455,59 @@ class ConfigWriter(object):
> >      self._UnlockedAddDisk(disk)
> >      self._UnlockedAttachInstanceDisk(inst_uuid, disk.uuid, idx)
> >  
> > +  def _UnlockedDetachInstanceDisk(self, inst_uuid, disk_uuid):
> > +    """Detach a disk from an instance.
> > +
> > +    @type inst_uuid: string
> > +    @param inst_uuid: The UUID of the instance object
> > +    @type disk_uuid: string
> > +    @param disk_uuid: The UUID of the disk object
> > +
> > +    """
> > +    instance = self._UnlockedGetInstanceInfo(inst_uuid)
> > +    if instance is None:
> > +      raise errors.ConfigurationError("Instance %s doesn't exist"
> > +                                      % inst_uuid)
> > +    if disk_uuid not in self._ConfigData().disks:
> > +      raise errors.ConfigurationError("Disk %s doesn't exist" % disk_uuid)
> > +
> > +    # Check if disk is attached to the instance
> > +    if disk_uuid not in instance.disks:
> > +      raise errors.ProgrammerError("Disk %s is not attached to an instance"
> > +                                   % disk_uuid)
> > +
> > +    idx = instance.disks.index(disk_uuid)
> > +    instance.disks.remove(disk_uuid)
> > +    instance_disks = self._UnlockedGetInstanceDisks(inst_uuid)
> > +    _UpdateIvNames(idx, instance_disks[idx:])
> > +    instance.serial_no += 1
> > +    instance.mtime = time.time()
> > +
> > +  def _UnlockedRemoveDisk(self, disk_uuid):
> > +    """Remove the disk from the configuration.
> > +
> > +    @type disk_uuid: string
> > +    @param disk_uuid: The UUID of the disk object
> > +
> > +    """
> > +    if disk_uuid not in self._ConfigData().disks:
> > +      raise errors.ConfigurationError("Disk %s doesn't exist" % disk_uuid)
> > +
> > +    # Remove disk from config file
> > +    del self._ConfigData().disks[disk_uuid]
> > +    self._ConfigData().cluster.serial_no += 1
> 
> We should probably check if the disk is still attached to an instance
> before actually removing it, just to prevent future mistakes.

Interdiff:

diff --git a/lib/config.py b/lib/config.py
index 07f7c03..75fb59a 100644
--- a/lib/config.py
+++ b/lib/config.py
@@ -494,6 +494,13 @@ class ConfigWriter(object):
     if disk_uuid not in self._ConfigData().disks:
       raise errors.ConfigurationError("Disk %s doesn't exist" % disk_uuid)
 
+    # Disk must not be attached anywhere
+    for inst in self._ConfigData().instances.values():
+      if disk_uuid in inst.disks:
+        raise errors.ReservationError("Cannot remove disk %s. Disk is"
+                                      " attached to instance %s"
+                                      % (disk_uuid, inst.name))
+
     # Remove disk from config file
     del self._ConfigData().disks[disk_uuid]
     self._ConfigData().cluster.serial_no += 1


Thanks,
Ilias

> Rest LGTM.
> 
> Thanks,
> Jose
> 
> > +
> > +  @_ConfigSync()
> > +  def RemoveInstanceDisk(self, inst_uuid, disk_uuid):
> > +    """Detach a disk from an instance and remove it from the config.
> > +
> > +    This is a simple wrapper over L{_UnlockedDetachInstanceDisk} and
> > +    L{_UnlockedRemoveDisk}.
> > +
> > +    """
> > +    self._UnlockedDetachInstanceDisk(inst_uuid, disk_uuid)
> > +    self._UnlockedRemoveDisk(disk_uuid)
> > +
> >    def _UnlockedGetDiskInfo(self, disk_uuid):
> >      """Returns information about a disk.
> >  
> > -- 
> > 1.9.1
> > 
> 
> -- 
> 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

-- 
Ilias Tsitsimpis
OpenPGP public key ID:
pub  4096R/25F3E3EE 2012-11-02 Ilias Tsitsimpis <[email protected]>
  Key fingerprint = 1986 21F5 7024 9B25 F4E2  4EF7 6FB8 90BA 25F3 E3EE


Tiger got to hunt, Bird got to fly;
Lisper got to sit and wonder, (Y (Y Y))?

Tiger got to sleep, Bird got to land;
Lisper got to tell himself he understand.

    — Kurt Vonnegut, modified by Darius Bacon

Attachment: signature.asc
Description: Digital signature

Reply via email to