Hi,
On 12/02/2009 07:29 PM, Chris Lumens wrote:
+def isRAID(info):
+ return udev_device_is_md(info) or udev_device_is_biosraid(info)
+
mdraid devices use partitions, not disks, and are software raid devices,
which should not be shown in the filtering UI, just like we don't show
lv's or anything else living above the "disk" level. Note that intel
BIOS RAID is special in that it does use mdraid now a days, but
udev_device_is_biosraid(), will identify Intel BIOS RAID anyways
independent of us using mdraid or dmraid (which we can still do using
the noiswmd cmdline option) for Intel BIOS RAID.
Right, I am trying to hide the components of these kind of raid devices
from the filtering UI.
I understand, but if a disk has an mdraid superblock on it (which can
happen when people use whole disks as raid sets) we should still show
it, just like we should still use it if the whole disk is used as a PV.
But with the udev_device_is_md(info) call removed, this wont be an issue
anymore.
So I believe the udev_device_is_md(info) call here should be removed.
Okay, that's easy.
ok
This seems wrong, we should not look at partitions to see if they are BIOS RAID, only at whole
disks, nor should we look at individual mpath disks in anyway, we should only every interact
with them at the multipath (devicemapper) device level which represent the single disk which
is hidden behind the mpath members for that disk. And here at the storage filtering level
we should not even do that, as we only care about the disk and not what is on it at this
point, simply displaying the disk in the mpath tab should be enough.
Nothing in the filtering UI works on a partition level. It's all disks.
Ah, sorry I got confused by this bit:
+ all_devices = filter(udev_device_is_disk, udev_get_block_devices())
+ (all_devices, mpaths, partitions) = identifyMultipaths(all_devices)
+
+ # The device list could be really long, so we really only want to
+ # iterate over it the bare minimum of times. Dividing this list up
+ # now means fewer elements to iterate over later.
+ (raid_devices, devices) = self.partition_list(lambda d: isRAID(d) and not isCCISS(d),
+ all_devices)
Notice how the second line of this bit, overrides all_devices to just be non multipath
disks, maybe it should then no longer be named all_devices? This confused me
into thinking that the last line of the quoted bit operated on, well, all (block) devices.
For multipath - I want to present the multipath device itself as
something that can be filtered out, not the various disks that make it
up. Allowing filtering out just some of the underlying is going to lead
to a very confusing situation. However, I do also want to be able to
list all the paths in the UI. From customer discussions, it's clear
that they'd prefer to ignore whole multipath devices rather than picking
and choosing the components.
I completely agree, me remakr about this was caused by the same confusion.
Also note that the name md is very confusing here, this is not a mdraid set, it
is a BIOS RAID set, which in some special cases may get assembled and handled
by mdraid in its special external metadata mode (the containers stuff), but
for anything but Intel these RAID sets are not handled by mdraid, please
s/md/raidset/ (or rs).
Okay, I can pick some type-neutral name.
Yes please :)
+ md.activate(mknod=True, mkparts=False)
Ok, this actually results in bringing the BIOS RAID set online, something which
I don't think we need (nor want) to do at this point. There should be other
ways to get the information you need from pyblock.
+ udev_settle()
+
+ partedDevice = md.PedDevice
+ size = int(partedDevice.getSize())
+ fstype = ""
+
Ah, this is why you do this, hmm, I guess I need to dig a bit into pyblock and come
up with an other way to get the size of the set, not sure if this will be easy to do
lets leave this as is for now, since a system should not have 1000's of BIOS RAID
sets this should not be much of an issue.
Right, after examining the contents of the udev data and the pyblock
module, this appears to be the only way to extract this kind of
information. You simply have to have a device node to know certain
things about the raid device.
I'm afraid that might be by far the easiest yes.
+ members = map(lambda m: m.get_devpath(), list(md.get_members()))
+ for d in raid_devices:
+ if udev_device_get_name(d) in members:
+ fstype = udev_device_get_format(d)
+ break
+
+ # biosraid devices don't really get udev data, at least not in a
+ # way that's useful to the filtering UI. So we need to fake that
+ # data now so we have something to put into the store.
+ data = {"XXX_SIZE": size, "ID_FS_TYPE": fstype, "DM_NAME": md.name,
+ "name": md.name}
+
This does not seem right, wrt ID_FS_TYPE, when calling udev_device_get_format()
on a member of for example an Intel BIOS RAID set, it will return isw_raid_member,
the fact that it is isw is already present in the md.name (which will start
with isw_), and isw_raid_member as FS_TYPE for the entire set is simply not true,
the set could contain a loopback layout with a foo filesystem, but most likely
will contain a partition table, which means that just like for a normal disk,
for the set ID_FS_TYPE will not be set.
I need ID_FS_TYPE set so that when I pull this dict out of the store
later on, I can run things like udev_device_is_biosraid and
udev_device_is_multipath_member and get results instead of tracebacks.
I don't care about whether there's a loopback layout or a filesystem or
whatever on it. The filtering UI does not deal in filesystems.
Ok, well the above code then assumes that udev_device_get_format() will return
the same for disks in a set, which it of course should, but I've sane cases where
it would not (due to a broken BIOS not writing the correct metadata). Anyways in
that case I think this is ok.
+ def partition_list(self, pred, lst):
+ pos = []
+ neg = []
+
+ for ele in lst:
+ if pred(ele):
+ pos.append(ele)
+ else:
+ neg.append(ele)
+
+ return (pos, neg)
The name partition_list for this method is a bit confusing when
doing storage stuff, maybe name it split_list instead ?
See: http://www.standardml.org/Basis/list.html#SIG:LIST.partition:VAL
This is a fairly common name for a list functional in other languages.
I suppose I could change it if I have to.
If this is a common name for such a function, then its fine to leave it
as is, lets not turn this into a color of the bikeshed discussion :)
Regards,
Hans
_______________________________________________
Anaconda-devel-list mailing list
Anaconda-devel-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/anaconda-devel-list
