Followup fixes for 618376 and 663411.
All of these are applicable for master as well, except patch 3. Followup coming with a patch 3 for master.
The image looks strange if we let anaconda run full screen and have this code in place.
Related: rhbz#663411 --- iw/welcome_gui.py | 10 +--------- 1 files changed, 1 insertions(+), 9 deletions(-)
diff --git a/iw/welcome_gui.py b/iw/welcome_gui.py index d2e6fc5..03ca194 100644 --- a/iw/welcome_gui.py +++ b/iw/welcome_gui.py @@ -38,15 +38,7 @@ class WelcomeWindow (InstallWindow): # WelcomeWindow tag="wel" def getScreen (self, anaconda): self.anaconda = anaconda - # this is a bit ugly... but scale the image if we're not at 800x600 - (w, h) = self.ics.cw.window.get_size_request() - if w >= 800: - height = None - width = None - else: - width = 500 - height = 258 - pix = gui.readImageFromFile("splash.png", width, height, dither=False) + pix = gui.readImageFromFile("splash.png") box = gtk.EventBox () box.add (pix) return box
On Wed, 2011-02-02 at 13:30 -1000, David Cantrell wrote:
The image looks strange if we let anaconda run full screen and have this code in place.
Looks okay to me.
Related: rhbz#663411
iw/welcome_gui.py | 10 +--------- 1 files changed, 1 insertions(+), 9 deletions(-)
diff --git a/iw/welcome_gui.py b/iw/welcome_gui.py index d2e6fc5..03ca194 100644 --- a/iw/welcome_gui.py +++ b/iw/welcome_gui.py @@ -38,15 +38,7 @@ class WelcomeWindow (InstallWindow): # WelcomeWindow tag="wel" def getScreen (self, anaconda): self.anaconda = anaconda
# this is a bit ugly... but scale the image if we're not at 800x600(w, h) = self.ics.cw.window.get_size_request()if w >= 800:height = Nonewidth = Noneelse:width = 500height = 258pix = gui.readImageFromFile("splash.png", width, height, dither=False)
pix = gui.readImageFromFile("splash.png") box = gtk.EventBox () box.add (pix) return box
We no longer resize images from anywhere this function is used in anaconda.
Related: rhbz#663411 --- gui.py | 9 +-------- 1 files changed, 1 insertions(+), 8 deletions(-)
diff --git a/gui.py b/gui.py index 5e96ed8..a08adf4 100755 --- a/gui.py +++ b/gui.py @@ -351,19 +351,12 @@ def getPixbuf(file):
return pixbuf
-def readImageFromFile(file, width = None, height = None, dither = None, - image = None): +def readImageFromFile(file, dither = False, image = None): pixbuf = getPixbuf(file) if pixbuf is None: log.warning("can't find pixmap %s" %(file,)) return None
- if (width is not None and height is not None - and height != pixbuf.get_height() - and width != pixbuf.get_width()): - pixbuf = pixbuf.scale_simple(width, height, - gtk.gdk.INTERP_BILINEAR) - if image is None: p = gtk.Image() else:
On Wed, 2011-02-02 at 13:30 -1000, David Cantrell wrote:
We no longer resize images from anywhere this function is used in anaconda.
Ack.
Related: rhbz#663411
gui.py | 9 +-------- 1 files changed, 1 insertions(+), 8 deletions(-)
diff --git a/gui.py b/gui.py index 5e96ed8..a08adf4 100755 --- a/gui.py +++ b/gui.py @@ -351,19 +351,12 @@ def getPixbuf(file):
return pixbuf-def readImageFromFile(file, width = None, height = None, dither = None,
image = None):+def readImageFromFile(file, dither = False, image = None): pixbuf = getPixbuf(file) if pixbuf is None: log.warning("can't find pixmap %s" %(file,)) return None
- if (width is not None and height is not None
and height != pixbuf.get_height()and width != pixbuf.get_width()):pixbuf = pixbuf.scale_simple(width, height,gtk.gdk.INTERP_BILINEAR)- if image is None: p = gtk.Image() else:
As another way to ensure the singlePV asVol requests get full pick of what PV they want to destroy, make sure they come before the regular asVol requests.
(Code from Dave Lehman dlehman@redhat.com)
Related: rhbz#618376 --- storage/devicetree.py | 7 +++++++ 1 files changed, 7 insertions(+), 0 deletions(-)
diff --git a/storage/devicetree.py b/storage/devicetree.py index f74d9d2..5c213ff 100644 --- a/storage/devicetree.py +++ b/storage/devicetree.py @@ -610,6 +610,13 @@ class DeviceTree(object): a2.device.partedPartition.number) else: ret = cmp(a1.device.name, a2.device.name) + elif isinstance(a1.device, LVMLogicalVolumeDevice) and \ + isinstance(a2.device, LVMLogicalVolumeDevice) and \ + a1.device.vg == a2.device.vg: + if a1.device.singlePV and not a2.device.singlePV: + ret = -1 + elif not a1.device.singlePV and a2.device.singlePV: + ret = 1 elif isinstance(a1.device, PartitionDevice) and \ a2.device.partitioned: ret = 1
On Wed, 2011-02-02 at 13:30 -1000, David Cantrell wrote:
As another way to ensure the singlePV asVol requests get full pick of what PV they want to destroy, make sure they come before the regular asVol requests.
(Code from Dave Lehman dlehman@redhat.com)
Brilliant. Ack.
Related: rhbz#618376
storage/devicetree.py | 7 +++++++ 1 files changed, 7 insertions(+), 0 deletions(-)
diff --git a/storage/devicetree.py b/storage/devicetree.py index f74d9d2..5c213ff 100644 --- a/storage/devicetree.py +++ b/storage/devicetree.py @@ -610,6 +610,13 @@ class DeviceTree(object): a2.device.partedPartition.number) else: ret = cmp(a1.device.name, a2.device.name)
elif isinstance(a1.device, LVMLogicalVolumeDevice) and \isinstance(a2.device, LVMLogicalVolumeDevice) and \a1.device.vg == a2.device.vg:if a1.device.singlePV and not a2.device.singlePV:ret = -1elif not a1.device.singlePV and a2.device.singlePV:ret = 1 elif isinstance(a1.device, PartitionDevice) and \ a2.device.partitioned: ret = 1
Followup again to the "/boot on LVM" commits for s390x. The feature request itself is odd, but it's close to a morbid curiosity now as to whether or not it'll actually work. The main limitation with /boot on LVM on s390x is that the logical volume must be tied to a single physical volume in the volume group. Combined with the sorting change commit, the patch does the following:
1) Makes singlePV a boolean attribute on the PartSpec and LVMLogicalVolumeDevice objects.
2) Key on the singlePV attribute rather than the mountpoint so the support for logical volumes on a single PV is a little more generic.
3) Modify _getSinglePV() to read the output of the vgs(8) command and find a PV with enough free space on it for the logical volume we are creating. Once we find one, return it. If we don't find one, raise an ugly exception and explain things to the user.
Now, the big gap here is what happens if there are no physical volumes of at least 500MB, since we are limiting this singlePV stuff to /boot on s390x? The answer is the user can get the exception described in #3 and do manual partitioning. I personally have never seen a DASD under 2GB, but I'm sure it's possible. I don't really care. I've fulfilled the morbid curiosity of this request and know it works.
Related: rhbz#618376 --- platform.py | 3 ++- storage/devices.py | 45 +++++++++++++++++++++++++++++++++------------ storage/errors.py | 3 +++ storage/partitioning.py | 3 ++- storage/partspec.py | 14 +++++++++++--- 5 files changed, 51 insertions(+), 17 deletions(-)
diff --git a/platform.py b/platform.py index 1d38475..8772b57 100644 --- a/platform.py +++ b/platform.py @@ -483,7 +483,8 @@ class S390(Platform): def setDefaultPartitioning(self): """Return the default platform-specific partitioning information.""" return [PartSpec(mountpoint="/boot", fstype=self.defaultBootFSType, size=500, - weight=self.weight(mountpoint="/boot"), asVol=True)] + weight=self.weight(mountpoint="/boot"), asVol=True, + singlePV=True)]
def weight(self, fstype=None, mountpoint=None): if mountpoint and mountpoint == "/boot": diff --git a/storage/devices.py b/storage/devices.py index 3852f9c..38f40b3 100644 --- a/storage/devices.py +++ b/storage/devices.py @@ -2220,7 +2220,8 @@ class LVMLogicalVolumeDevice(DMDevice): def __init__(self, name, vgdev, size=None, uuid=None, stripes=1, logSize=0, snapshotSpace=0, format=None, exists=None, sysfsPath='', - grow=None, maxsize=None, percent=None): + grow=None, maxsize=None, percent=None, + singlePV=False): """ Create a LVMLogicalVolumeDevice instance.
Arguments: @@ -2238,6 +2239,7 @@ class LVMLogicalVolumeDevice(DMDevice): sysfsPath -- sysfs device path format -- a DeviceFormat instance exists -- indicates whether this is an existing device + singlePV -- if true, maps this lv to a single pv
For new (non-existent) LVs only:
@@ -2261,6 +2263,7 @@ class LVMLogicalVolumeDevice(DMDevice): self.snapshotSpace = snapshotSpace self.stripes = stripes self.logSize = logSize + self.singlePV = singlePV
self.req_grow = None self.req_max_size = 0 @@ -2427,16 +2430,35 @@ class LVMLogicalVolumeDevice(DMDevice): log.debug("vg %s teardown failed; continuing" % self.vg.name)
def _getSinglePV(self): - pvs = [] - paths = [] - - for parent in self.parents: - pvs += parent.parents - - paths = map(lambda x: x.path, pvs) + # NOTE: This runs the pvs(8) command and outputs the PV device + # name, VG name, and amount of free space on the PV in megabytes. + # A change to the Size class will require more handling of the + # units switch in this arg list. + args = ["pvs", "--noheadings", "--nosuffix", "--aligned", + "--units", "M", "-o", "pv_name,vg_name,pv_free"] + buf = iutil.execWithCapture("lvm", args, stderr="/dev/tty5") + + for line in buf.splitlines(): + fields = filter(lambda x: x != '', line.strip().split()) + + if len(fields) != 3 or fields[1] != self.vg.name: + continue + if float(fields[2]) >= self.size: + return [fields[0]] # lvm.lvcreate needs a list + + if hasattr(self.format, "mountpoint"): + mountpoint = self.format.mountpoint + else: + mountpoint = self.format.type
- paths.sort() - return paths[-1:] + raise SinglePhysicalVolumeError("Single physical volume restriction " + "for %(mountpoint)s on this platform. " + "No physical volumes available in " + "volume group %(vgname)s with " + "%(size)d MB of available space." + % {'mountpoint': mountpoint, + 'vgname': self.vg.name, + 'size': self.size})
def create(self, intf=None): """ Create the device. """ @@ -2455,8 +2477,7 @@ class LVMLogicalVolumeDevice(DMDevice): self.setupParents()
# should we use --zero for safety's sake? - if hasattr(self.format, "mountpoint") and \ - self.format.mountpoint == "/boot": + if self.singlePV: lvm.lvcreate(self.vg.name, self._name, self.size, progress=w, pvs=self._getSinglePV()) else: diff --git a/storage/errors.py b/storage/errors.py index ea7e79c..1d1fea0 100644 --- a/storage/errors.py +++ b/storage/errors.py @@ -85,6 +85,9 @@ class MDMemberError(DeviceFormatError): class PhysicalVolumeError(DeviceFormatError): pass
+class SinglePhysicalVolumeError(DeviceFormatError): + pass + class SwapSpaceError(DeviceFormatError): pass
diff --git a/storage/partitioning.py b/storage/partitioning.py index 6fc4924..b99abb9 100644 --- a/storage/partitioning.py +++ b/storage/partitioning.py @@ -181,7 +181,8 @@ def _scheduleLVs(anaconda, devs): mountpoint=request.mountpoint, grow=request.grow, maxsize=request.maxSize, - size=request.size) + size=request.size, + singlePV=request.singlePV)
# schedule the device for creation anaconda.id.storage.createDevice(dev) diff --git a/storage/partspec.py b/storage/partspec.py index 8ad81ca..a126c3f 100644 --- a/storage/partspec.py +++ b/storage/partspec.py @@ -21,7 +21,8 @@
class PartSpec(object): def __init__(self, mountpoint=None, fstype=None, size=None, maxSize=None, - grow=False, asVol=False, weight=0, requiredSpace=0): + grow=False, asVol=False, singlePV=False, weight=0, + requiredSpace=0): """ Create a new storage specification. These are used to specify the default partitioning layout as an object before we have the storage system up and running. The attributes are obvious @@ -29,6 +30,8 @@ class PartSpec(object):
asVol -- Should this be allocated as a logical volume? If not, it will be allocated as a partition. + singlePV -- Should this logical volume map to a single physical + volume in the volume group? Implies asVol=True weight -- An integer that modifies the sort algorithm for partition requests. A larger value means the partition will end up closer to the front of the disk. This is mainly used to @@ -50,17 +53,22 @@ class PartSpec(object): self.maxSize = maxSize self.grow = grow self.asVol = asVol + self.singlePV = singlePV self.weight = weight self.requiredSpace = requiredSpace
+ if self.singlePV and not self.asVol: + self.asVol = True + def __str__(self): s = ("%(type)s instance (%(id)s) -- \n" - " mountpoint = %(mountpoint)s asVol = %(asVol)s\n" + " mountpoint = %(mountpoint)s asVol = %(asVol)s singlePV = %(singlePV)s\n" " weight = %(weight)s fstype = %(fstype)s\n" " size = %(size)s maxSize = %(maxSize)s grow = %(grow)s\n" % {"type": self.__class__.__name__, "id": "%#x" % id(self), "mountpoint": self.mountpoint, "asVol": self.asVol, - "weight": self.weight, "fstype": self.fstype, "size": self.size, + "singlePV": self.singlePV, "weight": self.weight, + "fstype": self.fstype, "size": self.size, "maxSize": self.maxSize, "grow": self.grow})
return s
On Wed, 2011-02-02 at 13:30 -1000, David Cantrell wrote:
Followup again to the "/boot on LVM" commits for s390x. The feature request itself is odd, but it's close to a morbid curiosity now as to whether or not it'll actually work. The main limitation with /boot on LVM on s390x is that the logical volume must be tied to a single physical volume in the volume group. Combined with the sorting change commit, the patch does the following:
Makes singlePV a boolean attribute on the PartSpec and LVMLogicalVolumeDevice objects.
Key on the singlePV attribute rather than the mountpoint so the support for logical volumes on a single PV is a little more generic.
Modify _getSinglePV() to read the output of the vgs(8) command and find a PV with enough free space on it for the logical volume we are creating. Once we find one, return it. If we don't find one, raise an ugly exception and explain things to the user.
Now, the big gap here is what happens if there are no physical volumes of at least 500MB, since we are limiting this singlePV stuff to /boot on s390x? The answer is the user can get the exception described in #3 and do manual partitioning. I personally have never seen a DASD under 2GB, but I'm sure it's possible. I don't really care. I've fulfilled the morbid curiosity of this request and know it works.
Looks okay with one suggestion, below...
Related: rhbz#618376
platform.py | 3 ++- storage/devices.py | 45 +++++++++++++++++++++++++++++++++------------ storage/errors.py | 3 +++ storage/partitioning.py | 3 ++- storage/partspec.py | 14 +++++++++++--- 5 files changed, 51 insertions(+), 17 deletions(-)
diff --git a/platform.py b/platform.py index 1d38475..8772b57 100644 --- a/platform.py +++ b/platform.py @@ -483,7 +483,8 @@ class S390(Platform): def setDefaultPartitioning(self): """Return the default platform-specific partitioning information.""" return [PartSpec(mountpoint="/boot", fstype=self.defaultBootFSType, size=500,
weight=self.weight(mountpoint="/boot"), asVol=True)]
weight=self.weight(mountpoint="/boot"), asVol=True,singlePV=True)]def weight(self, fstype=None, mountpoint=None): if mountpoint and mountpoint == "/boot":
diff --git a/storage/devices.py b/storage/devices.py index 3852f9c..38f40b3 100644 --- a/storage/devices.py +++ b/storage/devices.py @@ -2220,7 +2220,8 @@ class LVMLogicalVolumeDevice(DMDevice): def __init__(self, name, vgdev, size=None, uuid=None, stripes=1, logSize=0, snapshotSpace=0, format=None, exists=None, sysfsPath='',
grow=None, maxsize=None, percent=None):
grow=None, maxsize=None, percent=None,singlePV=False): """ Create a LVMLogicalVolumeDevice instance. Arguments:@@ -2238,6 +2239,7 @@ class LVMLogicalVolumeDevice(DMDevice): sysfsPath -- sysfs device path format -- a DeviceFormat instance exists -- indicates whether this is an existing device
singlePV -- if true, maps this lv to a single pv For new (non-existent) LVs only:@@ -2261,6 +2263,7 @@ class LVMLogicalVolumeDevice(DMDevice): self.snapshotSpace = snapshotSpace self.stripes = stripes self.logSize = logSize
self.singlePV = singlePV self.req_grow = None self.req_max_size = 0@@ -2427,16 +2430,35 @@ class LVMLogicalVolumeDevice(DMDevice): log.debug("vg %s teardown failed; continuing" % self.vg.name)
def _getSinglePV(self):
pvs = []paths = []for parent in self.parents:pvs += parent.parentspaths = map(lambda x: x.path, pvs)
# NOTE: This runs the pvs(8) command and outputs the PV device# name, VG name, and amount of free space on the PV in megabytes.# A change to the Size class will require more handling of the# units switch in this arg list.args = ["pvs", "--noheadings", "--nosuffix", "--aligned","--units", "M", "-o", "pv_name,vg_name,pv_free"]buf = iutil.execWithCapture("lvm", args, stderr="/dev/tty5")for line in buf.splitlines():fields = filter(lambda x: x != '', line.strip().split())if len(fields) != 3 or fields[1] != self.vg.name:continueif float(fields[2]) >= self.size:return [fields[0]] # lvm.lvcreate needs a listif hasattr(self.format, "mountpoint"):mountpoint = self.format.mountpointelse:mountpoint = self.format.type
You should probably use self.format.name here instead to get the more user-friendly names for things like mdmember, lvmpv, and luks formats.
anaconda-devel@lists.fedoraproject.org
-
David Cantrell -
David Lehman