[PATCH rhel6-branch] Increment MD container child counter even if its volumes are ignored (#1120640)
Volumes which are not selected in Specialized Storage Devices are ignored in addUdevDevice. This causes that their parent container won't have its child counter incremented, which can later cause an incorrect unusedRaidMembersWarning.
Resolves: rhbz#1120640 Signed-off-by: Artur Paszkiewicz artur.paszkiewicz@intel.com --- storage/devicetree.py | 7 +++++++ 1 file changed, 7 insertions(+)
diff --git a/storage/devicetree.py b/storage/devicetree.py index 4045cdf..b9ea399 100644 --- a/storage/devicetree.py +++ b/storage/devicetree.py @@ -1328,6 +1328,13 @@ class DeviceTree(object): # slice off the "/dev/" part, lvm filter cares only about the rest partitions_paths = [p[5:] for p in partitions_paths] map(lvm.lvm_cc_addFilterRejectRegexp, partitions_paths) + + if udev_device_get_md_container(info): + parentName = devicePathToName(udev_device_get_md_container(info)) + container = self.getDeviceByName(parentName) + if container: + container.addChild() + return
log.debug("scanning %s (%s)..." % (name, sysfs_path))
On 07/17/2014 06:39 AM, Artur Paszkiewicz wrote:
Volumes which are not selected in Specialized Storage Devices are ignored in addUdevDevice. This causes that their parent container won't have its child counter incremented, which can later cause an incorrect unusedRaidMembersWarning.
Why would you not select all members/components of a container you plan to use?
David
Resolves: rhbz#1120640 Signed-off-by: Artur Paszkiewicz artur.paszkiewicz@intel.com
storage/devicetree.py | 7 +++++++ 1 file changed, 7 insertions(+)
diff --git a/storage/devicetree.py b/storage/devicetree.py index 4045cdf..b9ea399 100644 --- a/storage/devicetree.py +++ b/storage/devicetree.py @@ -1328,6 +1328,13 @@ class DeviceTree(object): # slice off the "/dev/" part, lvm filter cares only about the rest partitions_paths = [p[5:] for p in partitions_paths] map(lvm.lvm_cc_addFilterRejectRegexp, partitions_paths)
if udev_device_get_md_container(info):parentName = devicePathToName(udev_device_get_md_container(info))container = self.getDeviceByName(parentName)if container:container.addChild()return log.debug("scanning %s (%s)..." % (name, sysfs_path))
On 07/17/2014 05:30 PM, David Lehman wrote:
On 07/17/2014 06:39 AM, Artur Paszkiewicz wrote:
Volumes which are not selected in Specialized Storage Devices are ignored in addUdevDevice. This causes that their parent container won't have its child counter incremented, which can later cause an incorrect unusedRaidMembersWarning.
Why would you not select all members/components of a container you plan to use?
I may not want to use some particular container for installation. There can be multiple containers on different disks. This results in a misleading warning, that "disks contain BIOS RAID metadata, but are not part of any recognized BIOS RAID sets". This is not true - the RAID arrays are assembled and recognized, just not selected in the installer.
Artur
David
Resolves: rhbz#1120640 Signed-off-by: Artur Paszkiewicz artur.paszkiewicz@intel.com
storage/devicetree.py | 7 +++++++ 1 file changed, 7 insertions(+)
diff --git a/storage/devicetree.py b/storage/devicetree.py index 4045cdf..b9ea399 100644 --- a/storage/devicetree.py +++ b/storage/devicetree.py @@ -1328,6 +1328,13 @@ class DeviceTree(object): # slice off the "/dev/" part, lvm filter cares only about the rest partitions_paths = [p[5:] for p in partitions_paths] map(lvm.lvm_cc_addFilterRejectRegexp, partitions_paths)
if udev_device_get_md_container(info):parentName = devicePathToName(udev_device_get_md_container(info))container = self.getDeviceByName(parentName)if container:container.addChild()return log.debug("scanning %s (%s)..." % (name, sysfs_path))
anaconda-patches mailing list anaconda-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/anaconda-patches
On Fri, 2014-07-18 at 09:32 +0200, Artur Paszkiewicz wrote:
On 07/17/2014 05:30 PM, David Lehman wrote:
On 07/17/2014 06:39 AM, Artur Paszkiewicz wrote:
Volumes which are not selected in Specialized Storage Devices are ignored in addUdevDevice. This causes that their parent container won't have its child counter incremented, which can later cause an incorrect unusedRaidMembersWarning.
Why would you not select all members/components of a container you plan to use?
I may not want to use some particular container for installation. There can be multiple containers on different disks. This results in a misleading warning, that "disks contain BIOS RAID metadata, but are not part of any recognized BIOS RAID sets". This is not true - the RAID arrays are assembled and recognized, just not selected in the installer.
But only if you select some devices from the container for installation, right? That's seems like a situation dangerous enough to show a warning even if a bit misleading.
On 07/18/2014 09:49 AM, Vratislav Podzimek wrote:
On Fri, 2014-07-18 at 09:32 +0200, Artur Paszkiewicz wrote:
On 07/17/2014 05:30 PM, David Lehman wrote:
On 07/17/2014 06:39 AM, Artur Paszkiewicz wrote:
Volumes which are not selected in Specialized Storage Devices are ignored in addUdevDevice. This causes that their parent container won't have its child counter incremented, which can later cause an incorrect unusedRaidMembersWarning.
Why would you not select all members/components of a container you plan to use?
I may not want to use some particular container for installation. There can be multiple containers on different disks. This results in a misleading warning, that "disks contain BIOS RAID metadata, but are not part of any recognized BIOS RAID sets". This is not true - the RAID arrays are assembled and recognized, just not selected in the installer.
But only if you select some devices from the container for installation, right? That's seems like a situation dangerous enough to show a warning even if a bit misleading.
No, this happens when I don't select any devices (BIOS RAID sets) from the container. I can't see why this could be dangerous. I'm not selecting any of the component disks of the container, in fact they are not even shown on any of the tabs, so they must be properly identified as RAID component devices. Here are the steps to reproduce this:
1. Create 2 IMSM RAID arrays on different disks 2. Run the RHEL installer 3. Select "Specialized Storage Devices" 4. On the "Firmware RAID" tab select only one RAID array and click "Next"
Artur
On Fri, 2014-07-18 at 10:21 +0200, Artur Paszkiewicz wrote:
On 07/18/2014 09:49 AM, Vratislav Podzimek wrote:
On Fri, 2014-07-18 at 09:32 +0200, Artur Paszkiewicz wrote:
On 07/17/2014 05:30 PM, David Lehman wrote:
On 07/17/2014 06:39 AM, Artur Paszkiewicz wrote:
Volumes which are not selected in Specialized Storage Devices are ignored in addUdevDevice. This causes that their parent container won't have its child counter incremented, which can later cause an incorrect unusedRaidMembersWarning.
Why would you not select all members/components of a container you plan to use?
I may not want to use some particular container for installation. There can be multiple containers on different disks. This results in a misleading warning, that "disks contain BIOS RAID metadata, but are not part of any recognized BIOS RAID sets". This is not true - the RAID arrays are assembled and recognized, just not selected in the installer.
But only if you select some devices from the container for installation, right? That's seems like a situation dangerous enough to show a warning even if a bit misleading.
No, this happens when I don't select any devices (BIOS RAID sets) from the container. I can't see why this could be dangerous. I'm not selecting any of the component disks of the container, in fact they are not even shown on any of the tabs, so they must be properly identified as RAID component devices. Here are the steps to reproduce this:
- Create 2 IMSM RAID arrays on different disks
- Run the RHEL installer
- Select "Specialized Storage Devices"
- On the "Firmware RAID" tab select only one RAID array and click
"Next"
Ahh, okay, thanks for clarification. I think your patch is perfectly okay then.
On 07/18/2014 03:21 AM, Artur Paszkiewicz wrote:
On 07/18/2014 09:49 AM, Vratislav Podzimek wrote:
On Fri, 2014-07-18 at 09:32 +0200, Artur Paszkiewicz wrote:
On 07/17/2014 05:30 PM, David Lehman wrote:
On 07/17/2014 06:39 AM, Artur Paszkiewicz wrote:
Volumes which are not selected in Specialized Storage Devices are ignored in addUdevDevice. This causes that their parent container won't have its child counter incremented, which can later cause an incorrect unusedRaidMembersWarning.
Why would you not select all members/components of a container you plan to use?
I may not want to use some particular container for installation. There can be multiple containers on different disks. This results in a misleading warning, that "disks contain BIOS RAID metadata, but are not part of any recognized BIOS RAID sets". This is not true - the RAID arrays are assembled and recognized, just not selected in the installer.
But only if you select some devices from the container for installation, right? That's seems like a situation dangerous enough to show a warning even if a bit misleading.
No, this happens when I don't select any devices (BIOS RAID sets) from the container. I can't see why this could be dangerous. I'm not selecting any of the component disks of the container, in fact they are
But what if you were selecting one of them? This patch would disable a valid warning.
My take is that we shouldn't be showing a warning about a fwraid array on unselected disks. Rather than lie about what we've found, we should be more careful about what conditions we warn under.
David
not even shown on any of the tabs, so they must be properly identified as RAID component devices. Here are the steps to reproduce this:
- Create 2 IMSM RAID arrays on different disks
- Run the RHEL installer
- Select "Specialized Storage Devices"
- On the "Firmware RAID" tab select only one RAID array and click
"Next"
Artur _______________________________________________ anaconda-patches mailing list anaconda-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/anaconda-patches
On 07/18/2014 02:50 PM, David Lehman wrote:
On 07/18/2014 03:21 AM, Artur Paszkiewicz wrote:
On 07/18/2014 09:49 AM, Vratislav Podzimek wrote:
On Fri, 2014-07-18 at 09:32 +0200, Artur Paszkiewicz wrote:
On 07/17/2014 05:30 PM, David Lehman wrote:
On 07/17/2014 06:39 AM, Artur Paszkiewicz wrote:
Volumes which are not selected in Specialized Storage Devices are ignored in addUdevDevice. This causes that their parent container won't have its child counter incremented, which can later cause an incorrect unusedRaidMembersWarning.
Why would you not select all members/components of a container you plan to use?
I may not want to use some particular container for installation. There can be multiple containers on different disks. This results in a misleading warning, that "disks contain BIOS RAID metadata, but are not part of any recognized BIOS RAID sets". This is not true - the RAID arrays are assembled and recognized, just not selected in the installer.
But only if you select some devices from the container for installation, right? That's seems like a situation dangerous enough to show a warning even if a bit misleading.
No, this happens when I don't select any devices (BIOS RAID sets) from the container. I can't see why this could be dangerous. I'm not selecting any of the component disks of the container, in fact they are
But what if you were selecting one of them? This patch would disable a valid warning.
My take is that we shouldn't be showing a warning about a fwraid array on unselected disks. Rather than lie about what we've found, we should be more careful about what conditions we warn under.
I see it this way, that this patch does not really disable the warning, but it tries to workaround a problem with MD RAID arrays detection: - a MD container is never ignored - a MD volume which belongs to a container is ignored when it is not selected, and its container's child counter won't be incremented
The warning is displayed for containers having kids == 0, so I get a warning about unrecognized BIOS RAID sets, even though they are recognized, just not selected for use.
For a container, which really does not have any recognized volumes, not visible in the Firmware RAID tab, the warning is still displayed (I've tested this). I believe that this is the way it was supposed to work, please correct me if I'm wrong.
Artur
David
not even shown on any of the tabs, so they must be properly identified as RAID component devices. Here are the steps to reproduce this:
- Create 2 IMSM RAID arrays on different disks
- Run the RHEL installer
- Select "Specialized Storage Devices"
- On the "Firmware RAID" tab select only one RAID array and click
"Next"
Artur
On 07/18/2014 09:00 AM, Artur Paszkiewicz wrote:
On 07/18/2014 02:50 PM, David Lehman wrote:
On 07/18/2014 03:21 AM, Artur Paszkiewicz wrote:
On 07/18/2014 09:49 AM, Vratislav Podzimek wrote:
On Fri, 2014-07-18 at 09:32 +0200, Artur Paszkiewicz wrote:
On 07/17/2014 05:30 PM, David Lehman wrote:
On 07/17/2014 06:39 AM, Artur Paszkiewicz wrote: > Volumes which are not selected in Specialized Storage Devices are > ignored in addUdevDevice. This causes that their parent container > won't have its child counter incremented, which can later cause an > incorrect unusedRaidMembersWarning.
Why would you not select all members/components of a container you plan to use?
I may not want to use some particular container for installation. There can be multiple containers on different disks. This results in a misleading warning, that "disks contain BIOS RAID metadata, but are not part of any recognized BIOS RAID sets". This is not true - the RAID arrays are assembled and recognized, just not selected in the installer.
But only if you select some devices from the container for installation, right? That's seems like a situation dangerous enough to show a warning even if a bit misleading.
No, this happens when I don't select any devices (BIOS RAID sets) from the container. I can't see why this could be dangerous. I'm not selecting any of the component disks of the container, in fact they are
But what if you were selecting one of them? This patch would disable a valid warning.
My take is that we shouldn't be showing a warning about a fwraid array on unselected disks. Rather than lie about what we've found, we should be more careful about what conditions we warn under.
I see it this way, that this patch does not really disable the warning, but it tries to workaround a problem with MD RAID arrays detection:
- a MD container is never ignored
- a MD volume which belongs to a container is ignored when it is not selected, and its container's child counter won't be incremented
The warning is displayed for containers having kids == 0, so I get a warning about unrecognized BIOS RAID sets, even though they are recognized, just not selected for use.
This is still a bit of a hack. A device with kids > 0 is expected to be the parent of some other device. I don't know if that would cause any problems in the branch you are operating on, but it most likely would in the newer branches.
Normally if a fwraid array is not selected, its member devices will be ignored along with it. As a result, no warning about unused raid members is shown. I'm not sure why the container has any children if all of its member disks are ignored. I still think this is the root of the problem. Whatever mechanism we use for non-intel fwraid should be the mechanism used for intel fwraid as well.
David
I believe this is the best approach to deal with the inaccurate warning shown for FW RAID containers with no members selected.
Vratislav Podzimek (1): Ignore MD RAID containers if all their members are ignored (#1120640)
storage/devicetree.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)
If all members of an MD (FW) RAID container are ignored, the container itself should be ignored too. Otherwise it is later checked for a number of (non-ignored) children which is zero and a cryptic/inaccurate warning is shown to the user (as described in the bug).
Signed-off-by: Vratislav Podzimek vpodzime@redhat.com --- storage/devicetree.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/storage/devicetree.py b/storage/devicetree.py index f0733f0..a85c50e 100644 --- a/storage/devicetree.py +++ b/storage/devicetree.py @@ -946,7 +946,13 @@ class DeviceTree(object): # the filter ui. Note that making the ui use md names instead is not # possible as the md names are simpy md# and we cannot predict the # if udev_device_get_md_level(info) == "container": - return False + # we should ignore the container if all its members are ignored + container_paths = info["DEVLINKS"].split() + container_paths.append(info["DEVNAME"]) + devices = udev_get_block_devices() + members = (dev_info for dev_info in devices + if udev_device_get_md_container(dev_info) in container_paths) + return all(self.isIgnored(member) for member in members)
if udev_device_get_md_container(info) and \ udev_device_get_md_name(info):
Unfortunately, this doesn't work. Sending a version 2 which works on my VM where I faked some BIOS RAID.
If all (block) devices of an MD (FW) RAID container are ignored, the container itself should be ignored too. Otherwise it is later checked for a number of (non-ignored) children which is zero and a cryptic/inaccurate warning is shown to the user (as described in the bug).
Signed-off-by: Vratislav Podzimek vpodzime@redhat.com --- storage/devicetree.py | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-)
diff --git a/storage/devicetree.py b/storage/devicetree.py index f0733f0..d6f8197 100644 --- a/storage/devicetree.py +++ b/storage/devicetree.py @@ -41,6 +41,7 @@ import iutil import platform import parted import _ped +import block
import gettext _ = lambda x: gettext.ldgettext("anaconda", x) @@ -946,7 +947,18 @@ class DeviceTree(object): # the filter ui. Note that making the ui use md names instead is not # possible as the md names are simpy md# and we cannot predict the # if udev_device_get_md_level(info) == "container": - return False + if name in self.exclusiveDisks: + # XXX: should we do this somewhere above in general? + return False + # we should ignore the container if all its members are ignored + all_rsets = block.getRaidSets() + # rsets this device belongs to + rsets = [] + for rs in all_rsets: + member_devs = set(member.devpath for member in rs.members if isinstance(member, block.device.RaidDev)) + if udev_device_get_name(info) in member_devs: + rsets.append(rs) + return not any(rs.rs.name in self.exclusiveDisks for rs in rsets)
if udev_device_get_md_container(info) and \ udev_device_get_md_name(info):
Just adding a note that this patch has been confirmed to fix the bug #1120640 and to not break the related use-cases.
On Fri, 2016-01-08 at 14:08 +0100, Vratislav Podzimek wrote:
If all (block) devices of an MD (FW) RAID container are ignored, the container itself should be ignored too. Otherwise it is later checked for a number of (non-ignored) children which is zero and a cryptic/inaccurate warning is shown to the user (as described in the bug).
Signed-off-by: Vratislav Podzimek vpodzime@redhat.com
storage/devicetree.py | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-)
diff --git a/storage/devicetree.py b/storage/devicetree.py index f0733f0..d6f8197 100644 --- a/storage/devicetree.py +++ b/storage/devicetree.py @@ -41,6 +41,7 @@ import iutil import platform import parted import _ped +import block import gettext _ = lambda x: gettext.ldgettext("anaconda", x) @@ -946,7 +947,18 @@ class DeviceTree(object): # the filter ui. Note that making the ui use md names instead is not # possible as the md names are simpy md# and we cannot predict the # if udev_device_get_md_level(info) == "container": - return False + if name in self.exclusiveDisks: + # XXX: should we do this somewhere above in general? + return False + # we should ignore the container if all its members are ignored + all_rsets = block.getRaidSets() + # rsets this device belongs to + rsets = [] + for rs in all_rsets: + member_devs = set(member.devpath for member in rs.members if isinstance(member, block.device.RaidDev)) + if udev_device_get_name(info) in member_devs: + rsets.append(rs) + return not any(rs.rs.name in self.exclusiveDisks for rs in rsets) if udev_device_get_md_container(info) and \ udev_device_get_md_name(info):
If there is no one who can answer you to XXX question here please change the XXX to TODO. Otherwise ACK.
anaconda-patches@lists.fedorahosted.org
-
Artur Paszkiewicz -
David Lehman -
Jiří Konečný -
Vratislav Podzimek