Re: [PATCH] Make pyblock compatible with new pyparted.

[Joel Granados <jgranado redhat com>]

----- "David Cantrell" <dcantrell redhat com> wrote:

> Comments:
> 
> 1) I've pushed a change to pyparted for the 'partitions' property on a
> 
> parted.Disk object.  You have lines like 'for part in
> disk.partitions'. 
>   That won't work without the patch I just submitted because I 
> originally made the partitions property a hash table.  After patching
> 
> anaconda and seeing your patch for pyblock here, I decided that a 
> partitions hash table isn't as useful as just as partitions list. 
> Just 
> a comment, you don't have to change your code.  Make sure you have the
> 
> latest pyparted if you are testing locally.
> 
> 2) When you create a parted.Disk, there are two exceptions that can be
> 
> raised.  If there is a low-level read error on the device, you'll get
> a 
> parted.IOException.  The other error that could be raised is a 
> parted.DiskException, which currently only happens if you give 
> __init__() on parted.Disk something invalid.

Ok, will modify the code to take into account the exceptions.  And will also get some testing to see what happens :)

Thx for the review.

> 
> Other than that, the patch looks fine.  You picked up all of the 
> property name changes and things like is_active() to just active.
> 
> Thanks for jumping on this really quickly.
> 
> Joel Granados Moreno wrote:
> > ---
> >  device.py |   42 +++++++++++++++++++++---------------------
> >  1 files changed, 21 insertions(+), 21 deletions(-)
> > 
> > diff --git a/device.py b/device.py
> > index 828f198..2d26b52 100644
> > --- a/device.py
> > +++ b/device.py
> > @@ -335,7 +335,7 @@ class MultiPath:
> >          
> >      def get_PedDevice(self):
> >          import parted as _parted
> > -        ret = _parted.PedDevice.get(self.bdev.path)
> > +        ret = _parted.getDevice(self.bdev.path)
> >          del _parted
> >          return ret
> >      PedDevice = property(get_PedDevice, None, None,
> "parted.PedDevice")
> > @@ -476,22 +476,22 @@ class MultiPath:
> >          dev = self.PedDevice
> >          dev.open()
> >          try:
> > -            disk = _parted.PedDisk.new(dev)
> > -        except _parted.error, msg:
> > +            disk = _parted.Disk(dev)
> > +        except _parted.DiskException, msg:
> >              dev.close()
> >              del dev
> >              return
> > -        part = disk.next_partition()
> > -        while part:
> > -            if part.is_active():
> > -                name = "%sp%s" % (self.name, part.num)
> > +
> > +        for part in disk.partitions:
> > +            if part.active:
> > +                name = "%sp%s" % (self.name, part.number)
> >                  bdev = BlockDev(self.prefix + self.name)
> >                  bdev.mode = 0600
> > -                map = _PartitionDeviceMap(0, part.geom.length,
> bdev,
> > -                    part.geom.start, part.num)
> > +                map = _PartitionDeviceMap(0, part.geometry.length,
> bdev,
> > +                    part.geometry.start, part.number)
> >                  map.name = name
> >                  yield map
> > -            part = disk.next_partition(part)
> > +
> >          del disk
> >          dev.close()
> >          del dev
> > @@ -588,7 +588,7 @@ class RaidSet:
> >          
> >      def get_PedDevice(self):
> >          import parted as _parted
> > -        ret = _parted.PedDevice.get(self.bdev.path)
> > +        ret = _parted.getDevice(self.bdev.path)
> >          del _parted
> >          return ret
> >      PedDevice = property(get_PedDevice, None, None,
> "parted.PedDevice")
> > @@ -757,23 +757,23 @@ class RaidSet:
> >          dev = self.PedDevice
> >          dev.open()
> >          try:
> > -            disk = _parted.PedDisk.new(dev)
> > -        except _parted.error, msg:
> > +            disk = _parted.Disk(device=dev)
> > +        except _parted.DiskException, msg:
> >              dev.close()
> >              del dev
> >              return
> > -        part = disk.next_partition()
> > -        while part:
> > -            if part.is_active():
> > -                name = "%sp%s" % (self.name, part.num)
> > +
> > +        for part in disk.partitions:
> > +            if part.active:
> > +                name = "%sp%s" % (self.name, part.number)
> >                  bdev = BlockDev(self.prefix + self.name)
> >                  bdev.mode = 0600
> > -                map = _PartitionDeviceMap(0, part.geom.length,
> bdev,
> > -                    part.geom.start, part.num)
> > +                map = _PartitionDeviceMap(0, part.geometry.length,
> bdev,
> > +                    part.geometry.start, part.number)
> >                  map.name = name
> >                  map.create()
> >                  self._RaidSet__parts.append(map)
> > -            part = disk.next_partition(part)
> > +
> >          del disk
> >          dev.close()
> >          del dev
> > @@ -868,7 +868,7 @@ class RaidDev:
> >  
> >      def get_PedDevice(self):
> >          import parted as _parted
> > -        ret = _parted.PedDevice.get(self.bdev.path)
> > +        ret = _parted.getDevice(self.bdev.path)
> >          del _parted
> >          return ret
> >      PedDevice = property(get_PedDevice, None, None,
> "parted.PedDevice")
> 
> 
> -- 
> David Cantrell <dcantrell redhat com>
> Red Hat / Honolulu, HI
> 
> _______________________________________________
> Anaconda-devel-list mailing list
> Anaconda-devel-list redhat com
> https://www.redhat.com/mailman/listinfo/anaconda-devel-list

-- 
Joel Andres Granados
Red Hat / Brno Czech Republic