Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow partitioning of md arrays whose members are all disks #138

Merged
merged 2 commits into from
Jun 24, 2015

Conversation

dwlehman
Copy link
Contributor

The motivation here is to enable hardware vendors to use regular md arrays for fwraid without the need for mdmonitor. The difficulty is that there will be nothing to distinguish these fwraid arrays from any other user-created array whose members are disks.

Existing arrays whose members/parents are all partitionable will be partitionable.
Existing arrays whose members/parents are all disks will be treated as disks.

It shouldn't be possible to delete such arrays in anaconda. They should be treated like fwraid arrays. It will be possible to delete them in blivet by scheduling an ActionDestroyFormat/ActionDestroyDevice action pair, but calling DeviceTree.recursiveRemove on them will not destroy the array.

@bcl
Copy link
Contributor

bcl commented May 28, 2015

The code looks ok to me. I think you should add docstrings on the new methods/attrs.

Also, is it possible that it could end up not being partitionable and not a disk? eg. if some parents are disks and some are partitions both methods will return false. Is there protection to make sure all the parents are consistent?

@dwlehman
Copy link
Contributor Author

Having parents of mixed types is allowed, but precludes treating the array as partitionable or as a disk.

@vpodzime
Copy link
Contributor

vpodzime commented Jun 1, 2015

Other than the nitpick abovet this looks good to me.

@vpodzime vpodzime added the ACK label Jun 1, 2015
@vathpela
Copy link
Contributor

vathpela commented Jun 3, 2015

I think you've got this right - if we preserve the capabilities that all of the constituents share, we should get a reasonable thing out. If any constituent doesn't have some capability, the resulting device doesn't have it.

@dwlehman
Copy link
Contributor Author

I found what I think is a better implementation of the second patch. What do you think?

https://gist.github.com/dwlehman/d9263e8b180c189d9362

I would like to replace the second patch ("Do format handling when...") with the equivalent of the above gist (ported to populator.py instead of devicetree.py).

@vpodzime
Copy link
Contributor

I found what I think is a better implementation of the second patch. What do you think?

https://gist.github.com/dwlehman/d9263e8b180c189d9362

I would like to replace the second patch ("Do format handling when...") with the equivalent of the above gist (ported to populator.py instead of devicetree.py).

The latter approach looks better to me. However, I'd change the name of the parameter to updateOrigFmt as that's what it causes/does.

If we don't do it right away the device could be looked up successfully
and cause confusion due to inaccurate(missing) format data.

This is only needed in the places where we add a new device from a
format handler. The device additions from add*Device are not vulnerable
to this issue.

Related: rhbz#1192004
Related: rhbz#1197582
Similarly, if all members of an existing array are partitionable
the array is also partitionable.

Resolves: rhbz#1197582
@mulkieran
Copy link
Contributor

When is it wrong to update the origFormat from the format when building a device tree? In other words, why do you need that parameter at all?

@dwlehman
Copy link
Contributor Author

It is possible to call DeviceTree.populate at any point, including after scheduling a reformat. It is important that originalFormat actually be the original format and not just the format it had when addUdevDevice ran last. The most obvious example of this is with network-attached block devices that get added one at a time from anaconda.

dwlehman added a commit that referenced this pull request Jun 24, 2015
Allow partitioning of md arrays whose members are all disks
@dwlehman dwlehman merged commit 71e35dd into storaged-project:master Jun 24, 2015
@dwlehman dwlehman deleted the partitionable-md branch June 24, 2015 21:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants