> storage/devices.py | 23 +++++++++++++-------- > storage/devicetree.py | 50 ++++++++++++++++++++++++------------------------ > storage/errors.py | 3 ++ > 3 files changed, 42 insertions(+), 34 deletions(-) Yes, I definitely think this patch is an improvement. Comments to follow inline... > @@ -568,8 +569,8 @@ class DiskDevice(StorageDevice): > _type = "disk" > > def __init__(self, device, format=None, > - size=None, major=None, minor=None, > - sysfsPath='', parents=None, init = False, labeltype = None): > + size=None, major=None, minor=None, sysfsPath='', \ > + parents=None, initcb=None, kwargsinitcb=None): > """ Create a DiskDevice instance. > > Arguments: > @@ -586,8 +587,8 @@ class DiskDevice(StorageDevice): > parents -- a list of required Device instances > removable -- whether or not this is a removable device > > - init -- initialize label on this drive > - labeltype -- type of the label to use during initialization > + initcb -- the call back to be used when initiating disk. > + kwargs -- arguments for the intcb. If no args then = {} > > > DiskDevices always exist. I'd get rid of the kwargsinitcb parameter here and treat initcb as a lambda. Also, there's no need for a slash in the arguments list. > @@ -603,12 +604,16 @@ class DiskDevice(StorageDevice): > if not self.partedDevice: > raise DeviceError("cannot find parted device instance") > log.debug("creating parted Disk: %s" % self.path) > - if init: > - self.partedDisk = parted.freshDisk(device=self.partedDevice, ty = labeltype) > - else: > + try: > self.partedDisk = parted.Disk(device=self.partedDevice) > - if not self.partedDisk: > - raise DeviceError("failed to create parted Disk") > + except: > + # if we have a cb function use it. else an error. > + if initcb is not None and kwargsinitcb is not None and \ > + initcb(**kwargsinitcb): > + self.partedDisk = parted.freshDisk(device=self.partedDevice, \ > + ty = platform.getPlatform(None).diskType) > + else: > + raise DeviceUserDeniedFormatError("User prefered to not format.") > > self.probe() > Then here, you just call initcb() instead of messing with kwargsinitcb as well. > @@ -749,29 +766,12 @@ class DeviceTree(object): > device = DiskDevice(name, > major=udev_device_get_major(info), > minor=udev_device_get_minor(info), > - sysfsPath=sysfs_path) > + sysfsPath=sysfs_path, > + initcb=questionInitializeDisk, > + kwargsinitcb={"intf":self.intf, "name":name}) > self._addDevice(device) > - except parted.IOException: #drive not initialized? > - if not self.intf: > - self.ignoredDisks.append(name) > - else: > - rc = self.intf.messageWindow(_("Warning"), > - _("Error processing drive %s.\n" > - "Maybe it needs to be reinitialized." > - "YOU WILL LOSE ALL DATA ON THIS DRIVE!") % (name,), > - type="custom", > - custom_buttons = [ _("_Ignore drive"), > - _("_Re-initialize drive") ], > - custom_icon="question") > - if rc == 0: > - self.ignoredDisks.append(name) > - else: > - device = DiskDevice(name, > - major=udev_device_get_major(info), > - minor=udev_device_get_minor(info), > - sysfsPath=sysfs_path, init = True, > - labeltype = platform.getPlatform(None).diskType) > - self._addDevice(device) > + except DeviceUserDeniedFormatError: #drive not initialized? > + self.ignoredDisks.append(name) > elif udev_device_is_partition(info): > log.debug("%s is a partition" % name) > device = self.getDeviceByName(name) And finally, your initcb parameter looks like this: initcb=lambda intf,name: questionInitializeDisk(intf, name) Just a thought, anyway. That's the kind of way I'd handle this but then I do enjoy a little excessive use of lambdas. I guess the main problem I had was that you don't really need to pass the arguments around through several different functions. That to me clutters things up a bit more. But I do like how you've moved the checking into the DiskDevice class. - Chris _______________________________________________ Anaconda-devel-list mailing list Anaconda-devel-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/anaconda-devel-list