[PATCH] Count with the extra metadata extents for RAID consistently (#1065737)
Vratislav Podzimek
vpodzime at redhat.com
Mon Feb 24 09:41:25 UTC 2014
On Fri, 2014-02-21 at 08:55 -0500, Anne Mulhern wrote:
>
>
>
> ----- Original Message -----
> > From: "Vratislav Podzimek" <vpodzime at redhat.com>
> > To: anaconda-patches at lists.fedorahosted.org
> > Sent: Friday, February 21, 2014 5:23:43 AM
> > Subject: [PATCH] Count with the extra metadata extents for RAID consistently (#1065737)
> >
> > When creating an LVM setup on top of RAID, we add some extra extents to make
> > sure the metadata will fit in. However, when reporting free space in a VG or
> > when creating an LV in a VG the size of which was pushed to the limit, we
> > need
> > to take those extra metadata into account as well.
> >
> > Signed-off-by: Vratislav Podzimek <vpodzime at redhat.com>
> > ---
> > blivet/devicefactory.py | 9 ++++++++-
> > blivet/devices.py | 10 ++++++++++
> > 2 files changed, 18 insertions(+), 1 deletion(-)
> >
> > diff --git a/blivet/devicefactory.py b/blivet/devicefactory.py
> > index 2d04005..5e0b7f1 100644
> > --- a/blivet/devicefactory.py
> > +++ b/blivet/devicefactory.py
> > @@ -1138,7 +1138,7 @@ class LVMFactory(DeviceFactory):
> > if self.container and self.container.exists:
> > return size
> >
> > - if self.container_size == 0:
> > + if self.container_size == SIZE_POLICY_AUTO:
> > # automatic container size management
> > if self.container:
> > size += sum([p.size for p in self.container.parents])
> > @@ -1213,6 +1213,13 @@ class LVMFactory(DeviceFactory):
> >
> > def _get_new_device(self, *args, **kwargs):
> > """ Create and return the factory device as a StorageDevice. """
> > +
> > + if self.container_raid_level and self.container_size in
> > [SIZE_POLICY_AUTO,
> > +
> > SIZE_POLICY_MAX]:
> > + # container pushed to the limit, but we need some extra space
> > for
> > + # metadata, so we need to make the LV smaller
> > + extra_md_space = LVM_PE_SIZE * len(self.disks) * 5
> > + kwargs["size"] -= extra_md_space
> > return self.storage.newLV(*args, **kwargs)
> >
> > def _set_name(self):
> > diff --git a/blivet/devices.py b/blivet/devices.py
> > index a607c1b..215b464 100644
> > --- a/blivet/devices.py
> > +++ b/blivet/devices.py
> > @@ -2424,9 +2424,19 @@ class LVMVolumeGroupDevice(DMDevice):
> > """ The amount of free space in this VG (in MB). """
> > # TODO: just ask lvm if isModified returns False
> >
> > + # get the number of disks used by PVs on RAID (if any)
> > + raid_disks = 0
> > + for pv in self.pvs:
> > + if isinstance(pv, MDRaidArrayDevice):
> > + raid_disks = max([raid_disks, len(pv.disks)])
>
> I'm puzzled by this chunk. It seems like it could be written equivalently as:
>
> raid_disks = max([0] + [len(pv.disks) for pv in self.pvs if isinstance(pv, MDRaidArrayDevice)])
That's true and it looks better to me, changing locally.
>
> It puzzles me that you're not adding something somewhere, since there might be more than one MDRaidArrayDevice in the pvs and I would have thought that both should contribute to this quantity, somehow.
Well, yeah, it puzzles me as well. But the reason is that the
LVMFactory._get_total_space uses such algorithm. Here is the code
snippet:
> if self.container_raid_level:
> # add five extents per disk to account for md metadata
> # (it was originally one per disk but that wasn't enough for raid5)
> size += LVM_PE_SIZE * len(self.disks) * 5
--
Vratislav Podzimek
Anaconda Rider | Red Hat, Inc. | Brno - Czech Republic
More information about the anaconda-patches
mailing list