David Lehman wrote:
I opted to maintain the lv information in a simple dict which we
convert to a temporary vg/lv instance set when we need to. There
is no trampling on objects, and the code is just simpler. The big
difference is that we have to go through and figure out what all
was done at the very end, right before returning to the partition
gui. So far, it is considerably more stable than the previous
implementation was.
continuation of review...
<snip already reviewed part>
@@ -771,7 +759,8 @@ class VolumeGroupEditor:
def logvolActivateCb(self, view, path, col):
self.editCurrentLogicalVolume()
- def getSelectedPhysicalVolumes(self, model):
+ def getSelectedPhysicalVolumes(self):
+ model = self.lvmlist.get_model()
pv = []
next = model.get_iter_first()
currow = 0
Indentation again (of model = line)
@@ -792,10 +781,10 @@ class VolumeGroupEditor:
def computeVGSize(self, pvlist, curpe):
availSpaceMB = 0L
for pv in pvlist:
- # XXX why the subtraction?
+ # have to clamp pvsize to multiple of PE
+ # XXX why the subtraction? fudging metadata?
pvsize = lvm.clampSize(pv.size, curpe) - (curpe/1024)
- # have to clamp pvsize to multiple of PE
availSpaceMB = availSpaceMB + pvsize
log.info("computeVGSize: vgsize is %s" % (availSpaceMB,))
Indentation again (of second comment line)
@@ -909,12 +880,173 @@ class VolumeGroupEditor:
# everything ok
break
- # pvs, pesize are taken care of in widget callbacks
- # (clickCB, peChangeCB)
- if self.isNew:
+ # here we have to figure out what all was done and convert it to
+ # devices and actions
+ #
+ # set up the vg with the right pvs
+ # set up the lvs
+ # set up the lvs' formats
+ #
+ log.debug("finished editing vg")
+ log.debug("pvs: %s" % [p.name for p in self.pvs])
+ log.debug("luks: %s" % self.luks.keys())
+ for lv in self.lvs.itervalues():
+ log.debug("lv %s" % lv)
+ _luks = self.luks.get(lv['name'])
+ if _luks:
+ log.debug(" luks: %s" % _luks)
+
+ actions = []
+ origlvs = self.vg.lvs
+ if not self.vg.exists:
+ log.debug("non-existing vg -- setting up lvs, pvs, name, pesize")
+ # remove all of the lvs
+ for lv in self.vg.lvs:
+ self.vg._removeLogVol(lv)
+
+ # set up the pvs
+ for pv in self.vg.pvs:
+ if pv not in self.pvs:
+ self.vg._removePV(pv)
+ for pv in self.pvs:
+ if pv not in self.vg.pvs:
+ self.vg._addPV(pv)
+
self.vg.name = volname
- self.actions.insert(0, ActionCreateDevice(self.vg))
- return self.actions
+ self.vg.pesize = pesize
+ actions = [ActionCreateDevice(self.vg)]
+
+ # Schedule destruction of all non-existing lvs, their formats,
+ # luks devices, &c. Also destroy devices that have been removed.
+ for lv in origlvs:
+ log.debug("old lv %s..." % lv.lvname)
+ if not lv.exists or lv.lvname not in self.lvs or \
+ (not self.lvs[lv.lvname]['exists'] and lv.exists):
+ log.debug("removing lv %s" % lv.lvname)
+ if lv.format.type == "luks":
+ try:
+ _luksdev = self.storage.devicetree.getChildren(lv)[0]
+ except IndexError:
+ pass
+ else:
+ if _luksdev.format.type:
+ actions.append(ActionDestroyFormat(_luksdev))
+
+ actions.append(ActionDestroyDevice(_luksdev))
+
+ if lv.format.type:
+ actions.append(ActionDestroyFormat(lv))
+
+ if lv in self.vg.lvs:
+ self.vg._removeLogVol(lv)
+
+ actions.append(ActionDestroyDevice(lv))
+
+ # schedule creation of all new lvs, their formats, luks devices, &c
+ tempvg = self.getTempVG()
+ for lv in tempvg.lvs:
+ log.debug("new lv %s" % lv)
+ if not lv.exists:
+ log.debug("creating lv %s" % lv.lvname)
+ # create the device
+ newlv = LVMLogicalVolumeDevice(lv.lvname,
+ self.vg,
+ size=lv.size)
+ actions.append(ActionCreateDevice(newlv))
+
+ # create the format
+ mountpoint = getattr(lv.format, "mountpoint", None)
+ format = getFormat(lv.format.type,
+ mountpoint=mountpoint,
+ device=newlv.path)
+ actions.append(ActionCreateFormat(newlv, format))
+
+ if lv.format.type == "luks":
+ # create the luks device
+ newluks = LUKSDevice("luks-%s" % newlv.name,
+ parents=[newlv])
+ actions.append(ActionCreateDevice(newluks))
+
+ # create the luks format
+ oldfmt = self.luks[lv.lvname]
+ mountpoint = getattr(oldfmt, "mountpoint", None)
+ format = getFormat(oldfmt.type,
+ mountpoint=mountpoint,
+ device=newluks.path)
+ actions.append(ActionCreateFormat(newluks, format))
+ else:
+ log.debug("lv %s already exists" % lv.lvname)
+ # this lv is preexisting. check for resize and reformat.
+ # first, get the real/original lv
+ origlv = None
+ for _lv in self.vg.lvs:
+ if _lv.lvname == lv.lvname:
+ origlv = _lv
+ break
+
+ if not origlv:
+ log.error("failed to locate preexisting lv %s" % lv.lvname)
+
You will backtrace at the next line, where you use origlv , so either remove the check
or do something else.
+ if lv.resizable and lv.targetSize != origlv.currentSize:
+ actions.append(ActionResizeDevice(origlv, lv.targetSize))
+
+ if lv.format.exists:
+ log.debug("format already exists")
+ if hasattr(origlv.format, "mountpoint"):
+ origlv.format.mountpoint = lv.format.mountpoint
+
+ if lv.format.migratable and lv.format.migrate and \
+ not origlv.format.migrate:
+ origlv.format.migrate = lv.format.migrate
+ actions.append(ActionMigrateFormat(origlv))
+
+ if lv.format.resizable and \
+ lv.format.targetSize != lv.format.currentSize:
+ new_size = lv.format.targetSize
+ actions.append(ActionResizeFormat(origlv, new_size))
+ elif lv.format.type:
+ log.debug("new format: %s" % lv.format.type)
+ # destroy old format and any associated luks devices
+ if origlv.format.type:
+ if origlv.format.type == "luks":
+ # destroy the luks device and its format
+ try:
+ _luksdev = self.storage.devicetree.getChildren(origlv)[0]
+ except IndexError:
+ pass
+ else:
+ if _luksdev.format.type:
+ # this is probably unnecessary
+ actions.append(ActionDestroyFormat(_luksdev))
+
+ actions.append(ActionDestroyDevice(_luksdev))
+
+ actions.append(ActionDestroyFormat(origlv))
+
+ # create the format
+ mountpoint = getattr(lv.format, "mountpoint", None)
+ format = getFormat(lv.format.type,
+ mountpoint=mountpoint,
+ device=origlv.path)
+ actions.append(ActionCreateFormat(origlv, format))
+
+ if lv.format.type == "luks":
+ # create the luks device
+ newluks = LUKSDevice("luks-%s" % origlv.name,
+ parents=[origlv])
+ actions.append(ActionCreateDevice(newluks))
+
+ # create the luks format
+ tmpfmt = self.luks[lv.lvname]
+ mountpoint = getattr(tmpfmt, "mountpoint", None)
+ format = getFormat(tmpfmt.type,
+ mountpoint=mountpoint,
+ device=newluks.path)
+ actions.append(ActionCreateFormat(newluks, format))
+ else:
+ log.debug("no format!?")
Wouldn't that be an error, or is allowed to create LV's without formatting them
for later use?
@@ -923,15 +1055,40 @@ class VolumeGroupEditor:
def __init__(self, anaconda, intf, parent, vg, isNew = 0):
self.storage = anaconda.id.storage
+
+ # the vg instance we were passed
self.vg = vg
+ self.peSize = vg.peSize
+
+ self.pvs = self.vg.pvs[:]
+
+ # a dict of dicts
+ # keys are lv names
+ # values are dicts representing the lvs
+ # name, size, format instance, exists
self.lvs = {}
+
+ # a dict of luks devices
+ # keys are lv names
+ # values are formats of the mapped devices
+ self.luks = {}
+
self.isNew = isNew
self.intf = intf
self.parent = parent
self.actions = []
for lv in self.vg.lvs:
- self.lvs[lv.lvname] = lv
+ self.lvs[lv.lvname] = {"name": lv.lvname,
+ "size": lv.size,
+ "format": copy.copy(lv.format),
+ "exists": lv.exists}
+
+ if lv.format.type == "luks":
+ try:
+ self.luks[lv.lvname] = self.storage.devicetree.getChildren(lv)[0].format
+ except IndexError:
+ self.luks[lv.lvname] = lv.format
self.availlvmparts = self.storage.unusedPVs(vg=vg)
Indentation is mixed again.
Other then the indentation issues, and the few other remarks it looks good.
Regards,
Hans
_______________________________________________
Anaconda-devel-list mailing list
Anaconda-devel-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/anaconda-devel-list
