I've got no problem with adding this as a kickstart-only feature, though
perhaps Jeremy or Peter might have more things to say along those lines.
However I do have a couple comments about things in your patch. I've
put them under the relevant parts of the patch. Don't know a better way
to do this.
Index: fsset.py
===================================================================
RCS file: /usr/local/CVS/anaconda/fsset.py,v
retrieving revision 1.334
diff -u -r1.334 fsset.py
--- fsset.py 17 Aug 2007 14:20:44 -0000 1.334
+++ fsset.py 27 Aug 2007 18:18:05 -0000
@@ -1505,7 +1505,17 @@
vgs[entry.device.name] = entry.device
# then set up the logical volumes
+ nonLVMirrors = []
for entry in self.entries:
+ #We should do the mirrored stuff first :)
+ if isinstance(entry.device, LogicalVolumeDevice) and entry.device.mirrors > 0:
+ vg = None
+ if vgs.has_key(entry.device.vgname):
+ vg = vgs[entry.device.vgname]
+ entry.device.setupDevice(chroot, vgdevice = vg)
+ else:
+ nonLVMirrors.append(entry)
+ for nonMirrors in nonLVMirrors:
if isinstance(entry.device, LogicalVolumeDevice):
vg = None
if vgs.has_key(entry.device.vgname):
Why do we need to do things in two loops here? Could you not move the
contents of the second for loop into the else case of the first loop?
Or is there a reason the mirrored ones need to be setup before the
non-mirrored ones?
@@ -2160,7 +2170,12 @@
class LogicalVolumeDevice(Device):
# note that size is in megabytes!
- def __init__(self, vgname, size, lvname, vg, existing = 0):
+ def __init__(self, vgname, size, lvname, vg, existing = 0, mirrors = 0, devs = [], corelog = True):
+ """
+ mirrors - The number of mirrors.
+ devs - Specific devices to use for mirrors.
+ corelog - Wether to use corelog or not.
+ """
Device.__init__(self)
self.vgname = vgname
self.size = size
@@ -2169,6 +2184,9 @@
self.isSetup = existing
self.doLabel = None
self.vg = vg
+ self.mirrors = mirrors #If mirrors is set to 0 mirroring is ignored.
+ self.devs = devs
+ self.corelog = corelog
# these are attributes we might want to expose. or maybe not.
# self.chunksize
@@ -2179,7 +2197,7 @@
def setupDevice(self, chroot="/", devPrefix='/tmp', vgdevice = None):
if not self.isSetup:
- lvm.lvcreate(self.name, self.vgname, self.size)
+ lvm.lvcreate(self.name, self.vgname, self.size, mirrors=self.mirrors, corelog=self.corelog, devs=self.devs)
self.isSetup = 1
if vgdevice and vgdevice.isNetdev():
How about making a new class
MirroredLogicalVolumeDevice(LogicalVolumeDevice) instead? You'd need to
override the __init__ and setupDevice methods, but everything else would
be the same. This keeps the mirroring stuff out of the way, though
perhaps other people don't like this idea.
Index: kickstart.py
===================================================================
RCS file: /usr/local/CVS/anaconda/kickstart.py,v
retrieving revision 1.385
diff -u -r1.385 kickstart.py
--- kickstart.py 23 Aug 2007 19:38:51 -0000 1.385
+++ kickstart.py 27 Aug 2007 18:18:05 -0000
@@ -695,7 +735,7 @@
"poweroff": Reboot,
"raid": Raid,
"reboot": Reboot,
- "repo": commands.repo.F8_Repo,
+ "repo": commands.repo.FC6_Repo,
"rootpw": RootPw,
"selinux": SELinux,
"services": commands.services.FC6_Services,
This must have snuck in due to not having the latest version of
kickstart.py checked out.
Index: lvm.py
===================================================================
RCS file: /usr/local/CVS/anaconda/lvm.py,v
retrieving revision 1.48
diff -u -r1.48 lvm.py
--- lvm.py 21 May 2007 14:30:19 -0000 1.48
+++ lvm.py 27 Aug 2007 18:18:05 -0000
@@ -157,12 +157,15 @@
log.error("running vgchange failed: %s" %(rc,))
# lvmDevicePresent = 0
-def lvcreate(lvname, vgname, size):
+def lvcreate(lvname, vgname, size, mirrors=0, corelog=True, devs=[]):
"""Creates a new logical volume.
lvname - name of logical volume to create.
vgname - name of volume group lv will be in.
size - size of lv, in megabytes.
+ mirrors - the amount of mirrors to create
+ devs - a list of string representing valid devices of the form "/dev/deviceName
+ corelog - whether or not to use corelog.
"""
global lvmDevicePresent
if flags.test or lvmDevicePresent == 0:
@@ -171,6 +174,8 @@
vgscan()
args = ["lvcreate", "-v", "-L", "%dM" %(size,), "-n", lvname, "-An", vgname]
+ if mirrors > 0:
+ args = args[:4]+["-m","%s"%mirrors, corelog and "--corelog"]+args[4:]+devs
try:
rc = lvmExec(*args)
except:
Again, I think here a new method would be much more straightforward. I
don't like all the trickery with the args array. Perhaps adding a
mirroredlvcreate() method would be better. Or at least, you could
change it to something like this:
if mirrors > 0:
args = ["lvcreate", "-v", ... "-m", mirrors, ...]
else:
args = ["lvcreate", ... ]
Index: partRequests.py
===================================================================
RCS file: /usr/local/CVS/anaconda/partRequests.py,v
retrieving revision 1.74
diff -u -r1.74 partRequests.py
--- partRequests.py 3 Aug 2007 19:32:24 -0000 1.74
+++ partRequests.py 27 Aug 2007 18:18:05 -0000
@@ -930,3 +930,84 @@
+ def getDevice(self, partitions):
+ """Return a device which can be solidified."""
+ devices = []
+ for dev in self.devs:
+ devices.append("/dev/%s"%partitions.getRequestByID(dev).device)
+ vg = partitions.getRequestByID(self.volumeGroup)
+ vgname = vg.volumeGroupName
+ self.dev = fsset.LogicalVolumeDevice(vgname, self.mirroredSize,
+ self.logicalVolumeName,
+ vg = vg,
+ existing = self.preexist,
+ mirrors=self.mirrors,
+ corelog=self.corelog,
+ devs=devices)
+ return self.dev
This is some of the complication I was talking about avoiding earlier
with the MirroredLogicalVolumeDevice class.
I have a general comment about error messages, but I can go back and
clean those little things up afterwards. I don't think that affects the
patch in the least.
On your pykickstart patch, you got the important part in the command
parser done, but there's a couple other things. You'll want to make a
new F8_LogVolData to hold the new attributes you created, and then you
will want to edit pykickstart/handlers/control.py to make sure F8 uses
these two new classes. It's a little less straightforward than I would
like.
- Chris
