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

mdadm: add new init features; bug fixes #1713

Closed
wants to merge 6 commits into
base: master
from

Conversation

Projects
None yet
6 participants
@josephtingiris
Copy link

josephtingiris commented Jan 3, 2019

Thanks for your contribution to OpenWrt!

To help keep the codebase consistent and readable,
and to help people review your contribution,
we ask you to follow the rules you find in the wiki at this link
https://openwrt.org/submitting-patches

Please remove this message before posting the pull request.

@josephtingiris josephtingiris changed the title do not build with defunct device & uuid; check block info for active linux_raid_members & dynamically update uci mdadm config prior to starting mdadm @ S13 mdadm: do not build with defunct device & uuid; check block info for active linux_raid_members & dynamically update uci mdadm config prior to starting mdadm @ S13; install /etc/init.d/mdadm_config Jan 3, 2019

@cshoredaniel

This comment has been minimized.

Copy link
Contributor

cshoredaniel commented on package/utils/mdadm/files/mdadm_config.init in 94b91dd Jan 3, 2019

Dynamically updating uci config and especially auto-commit is a no-no.

@cshoredaniel

This comment has been minimized.

Copy link
Contributor

cshoredaniel commented on package/utils/mdadm/files/mdadm_config.init in 94b91dd Jan 3, 2019

Dynamically altering uci config is considered bad.

This comment has been minimized.

Copy link
Contributor

cshoredaniel replied Jan 3, 2019

Perhaps have an option 'auto_update_config' -- although if you're going to do this auto stuff I'm not sure of the advantage over what was already being done. Perhaps instead you want 'auto_something' that does like before and if disabled requires that the specified sections exist in order to start the MD device.

@josephtingiris

This comment has been minimized.

Copy link

josephtingiris commented Jan 4, 2019

Thanks for your insight. That would explain why I couldn't find any examples in the other init scripts of explicit uci sets. I understand the point that uci commit is a big hammer.

My first foray into OpenWrt was this week. I appreciate the goal. I'm new to this project, though not to other Linux distributions.

Modern versions of Debian, Fedora, RHEL, et. al. dynamically assemble meta devices based on their drive header and automatically assign an unused device name e.g. /dev/md0. There's generally no need for a human's intervention. They've also all incorporated systemd and by comparison, older sysvinit styled distributions are lacking.

Having 'auto stuff' is helpful. That's what computers do best. The advantage is that humans don't have to do it. These are tasks computers can do better, faster, and much more reliably.

I think the idea of having a uci option, e.g. 'auto_config' is a good idea. It should be the default. If someone wants to change it and revert back to manual configuration then that should take precedence.

I'll modify this PR to support both methods, as well as re-comment the mdadmin.config to remove the defunct uuid and enable auto_config. When auto_config is used then it wont use uci for dynamic state.

Thanks again.

@cshoredaniel

This comment has been minimized.

Copy link
Contributor

cshoredaniel commented Jan 4, 2019

actually mdadm already auto-assembles all MD devices based on what's there (although if you want to extended that to raid on top of another dm I'm not sure...but that's really getting outside of the scope of a router distribution IMO).

@@ -4,9 +4,9 @@ config mdadm
# list devices /dev/sd*
# list devices partitions

config array

This comment has been minimized.

@cshoredaniel

cshoredaniel Jan 4, 2019

Contributor

Hmmmm....this [EDIT] shouldn't have been there....someone accidentally committed their personal config here. If there is not config (and hence no mdadm.conf) auto-assemble should 'just happen'.

This comment has been minimized.

@josephtingiris

josephtingiris Jan 4, 2019

That's what got me started. I've used mdadm a lot on other distros. My expectation was that it'd auto-assemble the array but it didn't. I traced it back to the Openwrt config & how its init builds /var/etc/mdadm.conf. Needless to say it's broken ootb.

It's apparently been this way for 2+ years and is still that way in openwrt/openwrt master. I reported that with more detail, here.

https://bugs.openwrt.org/index.php?do=details&task_id=2043&opened=1797&type%5B0%5D=&sev%5B0%5D=&due%5B0%5D=&cat%5B0%5D=&status%5B0%5D=open&percent%5B0%5D=&reported%5B0%5D=

Once I'd noticed that my UUIDs didn't match the config the package installed, I tested first by commenting them out with the expectation that it'd scan and auto-assemble. Didn't work. Maybe my bad. I've got a better grasp of uci & procd now as a result.

Updating the UUID in uci did work. That's when I noticed /etc/init.d/umount is K99. mdadm is K98. That's the second, or third problem. Rebooting causes mdadm stop to fail and umount remounts everything read only. It's messy. This should all work seamlessly as it does in other distros.

Further testing shows that auto-assemble does work if it's already been assembled at least once. At this point, I'm shooting for security and stability. There are obvious, albeit minor, issues with the way mdadm, cryptsetup, lvm2, and umount are set to init on Openwrt 18+

FWIW; I've got raw -> md -> cryptsetup -> lvm2 -> fs. all working seamlessly. I want it to stay working reliably, without concern that a package upgrade is going to break it.

This comment has been minimized.

@cshoredaniel

cshoredaniel Jan 4, 2019

Contributor

Ah, I checked the history -- I originally just did 813efe5#diff-020e9c6e02016d2840b3f057677eb0dd which didn't really have an uci config but @jow- add uci config in 813efe5#diff-020e9c6e02016d2840b3f057677eb0dd but apparently no one noticed for a long time there was an issue ... I wasn't active with OpenWrt/LEDE at the time and I guess it's not (yet) a major use case.

This comment has been minimized.

@cshoredaniel

cshoredaniel Jan 4, 2019

Contributor

I look forward to the result!

mdadm: Update init logic for autoconfig
This adds autoconfig support to mdadm.  /etc/config/mdadm now recognizes a uci 'defaults' section with an 'autoconfig' option.  When set, mdadm init will automatically determine the meta device UUIDs & appropriate device name(s).

The new mdadm_autoconfig() function will dynamically update /var/etc/mdadm.conf, rather than using uci.  If mdadm.@defaults[0].autoconfig is false then the previous manual logic will be used.

START & STOP order were also modified to allot more space before fstab starts, i.e for a cryptsetup block layer, and to stop mdadm last, to prevent conflicts with mdadm attempting to go offline prior to umount running.

Signed-off-by: Joseph Tingiris <joseph.tingiris@gmail.com>
@josephtingiris

This comment has been minimized.

Copy link

josephtingiris commented Jan 4, 2019

(although if you want to extended that to raid on top of another dm I'm not sure...but that's really getting outside of the scope of a router distribution IMO).

My Netgear R7800s have eSATA and USB 3 ports. Lots of modern routers are media gateways too. It's in Netgear's, ASUS's, and many other proprietary OSs. I want my media on a raid. I want it encrypted. I want to know if a drive fails all's not lost and if my drives get stolen then they better know what a block device and uuid is and how to get my key files. I want to sleep at night knowing all that and if there's a power outage everything comes back online without me thinking about it beyond this (and a few other) threads. And, I want it all on my 'router'. OpenWrt via Linux holds that promise, with a little work.

josephtingiris added some commits Jan 4, 2019

mdadm: add autoconfig feature; init bug fixes
This adds autoconfig support to mdadm.  /etc/config/mdadm now
recognizes a uci 'defaults' section with an 'autoconfig' option.
When set, mdadm init will automatically determine the meta device
UUIDs & appropriate device name(s).

The new mdadm_autoconfig() function will dynamically update
/var/etc/mdadm.conf, rather than using uci.  If
mdadm.@defaults[0].autoconfig is false then the previous manual
logic will be used.

START & STOP order were changed to allot more space
before fstab starts, i.e for a cryptsetup block layer, and
to stop mdadm last, to fix conflicts with mdadm
attempting to go offline prior to umount running.

An active, defunct device & uuid were left in the mdadm.config that
prevented mdadm from starting properly.  These were commented.

Signed-off-by: Joseph Tingiris <joseph.tingiris@gmail.com>
mdadm: corrected indentations
Signed-off-by: Joseph Tingiris <joseph.tingiris@gmail.com>

@josephtingiris josephtingiris changed the title mdadm: do not build with defunct device & uuid; check block info for active linux_raid_members & dynamically update uci mdadm config prior to starting mdadm @ S13; install /etc/init.d/mdadm_config mdadm: add autoconfig feature; init bug fixes Jan 4, 2019

@cshoredaniel

This comment has been minimized.

Copy link
Contributor

cshoredaniel commented Jan 4, 2019

Ah...don't forget to rebase your feature branch rather than keep adding commits: https://github.com/openwrt/packages/blob/master/CONTRIBUTING.md#advice-on-pull-requests

START=13
STOP=98
START=12
STOP=99

This comment has been minimized.

@cshoredaniel

cshoredaniel Jan 4, 2019

Contributor

Perhaps umount should be moved up one ... the problem is that at 99 I'm not sure it's guaranteed to occur before the done script.

This comment has been minimized.

@josephtingiris

josephtingiris Jan 5, 2019

I'll rebase once the dust settles. I really appreciate the feedback. This is how better things are built.

The START & STOP values are important bits. They directly relate to a larger scope across multiple packages. I've given it some thought; sysvinit is limited. They do need to be changed, though. I'm not quite done discovering what the 'best' value would be given the constraints. Here's my reasoning.

Block device mapped chains like this are fairly typical these days,

raw -> md -> cryptsetup -> lvm2 -> fs

Maybe there's no md, or no crypt, or no lvm, or maybe they're ordered differently. If lvm2 is activated before cryptsetup is opened then the order of operation would differ, etc. The point is, they all have a well defined place in the device block chain and should be started & stopped in a specific order depending on the specific chain's configuration.

Given the different possibilities, there's more than one linear use case. IMO, device mapping init is better suited to something that can track dependencies, such as systemd. With sysvinit it's more complicated to support all possibilities dynamically. I'm not familiar enough with uci or procd to clearly see how it'd be best integrated there.

That said, and given Openwrt 18+ still uses sysvinit patterns, a decision seems to be warranted. I know systemd is out of the question (for now) and in the meantime, all I've got is Life without systemd. So, I'm opting to get something done sooner than later. I'm trying to set the stage for correcting the most common use case in Openwrt 18+.

One of the cryptsetup FAQs is 2.8 Encryption on top of RAID or the other way round? and clear advice is given.

place encryption between RAID and filesystem

That brings us back to raw -> md -> cryptsetup -> lvm2 -> fs as the most probable common use case. Openwrt 18+ can support that now.

I'll come back to this thought with more detail. I'm in process of evaluating all included base packages & mapping their sequences. mdadm is right in the middle ...

This comment has been minimized.

@cshoredaniel

cshoredaniel Jan 5, 2019

Contributor

Yeah, it was probably 6-10 years ago that I wrote up https://openwrt.org/docs/techref/requirements.boot.process, but was talking with a fellow by the name of Bernard Loos(sp?) about the need to deal with dependencies since that initial document was exposing some ordering challenges already. Part of the issue for OpenWrt is having an init system that doesn't need an initramfs (for space/waste reasons), which is not a challenge systemd even attempts (ot my knowledge). I'd have to dig to find those discussion / design documents, but we were thinking about this problem before systemd came around, but 'reel jobs' interfered with actually working on the problem. Unfortunately I don't think I ever posted the dependency bits on the wiki, else procd might have supported them.

This comment has been minimized.

@josephtingiris

josephtingiris Jan 5, 2019

Thanks for sharing that. It helped me understand where you're coming from. I completely understand the challenges involved. I've been programming embedded systems for decades, with a sharp focus on Linux. I have a reel job too & know how it goes.

OpenWrt 18 already has most of what systemd requires. Others have done it and do it with busybox. The nay saying is tiresome & the writing has been all over the walls for years. Every modern distro uses it and for good reason.

I'm glad I learned systemd with an open mind. I've grown to appreciate it more and more. My finger memory is slowly forgetting service name start. That was my biggest complaint.

It's the little things. Yes, systemd brings additional complexity and alongside the benefit of great new capabilities. Maybe I'll whip up a systemd branch for Openwrt in my infinite spare time. ;)

At the moment, I want to get through this PR and continue learning how you all like things done. So far, I'm impressed. Back to the task at hand; incremental change. Good, better, best. This is the result of my init discovery with a fairly broad proposal. While systemd may be best, this is certainly better.

v18.06.x & master (proposal)
============================
network                        START=12 STOP=93 * can network be up this early & stay up a little later?
> mdadm.init                     START=13 STOP=99 * must start before cryptsetup; must stop (last) after umount, cryptsetup, & lvm2
cryptsetup.init                START=14 STOP=98 * must start before lvm2; second; must stop after umount & lvm2, but before mdadm
lvm2.init                      START=15 STOP=97 * must start before fstab; must stop after umount, but before cryptsetup & mdadm
blockd.init                    START=41 STOP=94 * must start after fstab; stop before lvm2
kdump.init                     START=41 STOP=95 * must start after fstab; must stop before umount
umount                         START=NO STOP=96 * must stop before mdadm
done                           START=99         * done is a nice conceptual name, but sorts incorrectly;  should be dead last; i.e. S99Zz

v18.06.1 & master (current)
===========================
cryptsetup.init                START=?? STOP=?? * Bug. cryptsetup.init is missing entirely
umount                         START=NO STOP=99 * Bug. other block devices must stop afterwards.
boot                           START=10 STOP=98 * OK? point of reference; runs /sbin/mount_root; has STOP value set, though unused?
sysctl                         START=11         * OK. point of reference
mdadm.init                     START=13 STOP=98 * Bug. umount is K99
lvm2.init                      START=15         * Bug. STOP undefined
network                        START=20 STOP=90 * starts after dnsmasq, dropbear, firewall, etc?  nice to have up before cryptsetup, so key files can be curl'ed?                                                                                                             
fstab.init                     START=40         * OK. point of reference; runs /sbin/block mount (and /sbin/block umount)
kdump.init                     START=41 STOP=98 * must be before umount & change if umount does
blockd.init                    START=80         * automounter.  should start much sooner, after fstab, & explicitly stop.
done                           START=95         * runs /etc/rc.local  S95?  many things are run after S95 ... START rc.local rant
led                            START=96         * rc.local rant
rssileds.init                  START=96 STOP=96 * rc.local rant
dsl_control                    START=97         * rc.local rant
linksys_recovery               START=97         * rc.local rant
hwmon_fancontrol               START=98         * rc.local rant
sysntpd                        START=98         * rc.local rant
adb-enablemodem                START=99         * rc.local rant
bootcount                      START=99         * rc.local rant
bugcheck.initd                 START=99         * rc.local rant
igmpproxy.init                 START=99         * rc.local rant; would still run after S99done
netdev-cpu                     START=99         * rc.local rant; would still run after S99done
omcproxy.init                  START=99         * rc.local rant; would still run after S99done
set-irq-affinity               START=99         * rc.local rant; would still run after S99done
urandom_seed                   START=99         * OK. saves /etc/urandom.seed; entropy.

I've got a more thorough assessment of everything included in master. Many were removed and quickly overlooked because they fell within acceptable ranges and were out of scope for the block device sequence being discussed here and/or didn't clearly write or seem to have any files left open for writing.

config_load mdadm
config_foreach mdadm_common mdadm
config_foreach mdadm_array array
fi

$PROG --assemble --scan --config="$CONF"

This comment has been minimized.

@cshoredaniel

cshoredaniel Jan 4, 2019

Contributor

Can you make CONF optional?

This comment has been minimized.

@josephtingiris

josephtingiris Jan 5, 2019

Yes, I could ... though ... to what end? Simply --assemble --scan ??

How? Add another defaults similar to autoconfig ?? I'm using $CONF there too.

To paraphrase from man.

Taking out --config completely would revert back to use /etc/mdadm.conf and /etc/mdadm.conf.d, or if those are missing then /etc/mdadm/mdadm.conf and /etc/mdadm/mdadm.conf.d.

Using $CONF=partitions, or --config=partitions, then nothing will be read and mdadm will act as though the config file contained exactly DEVICE partitions containers and will read /proc/partitions to find a list of devices to scan and /proc/mdstat to find a list of containers to examine.

Aside, that may be a clue to where/why it doesn't initially assemble correctly when everything's commented out of uci.

Using $CONF=none, or --config=none, would cause mdadm to act as though the config file were empty.

If it's a directory then files ending in .conf are sorted lexically and processed as individual config files.

I'm assuming that if no config file(s) exist then mdadm will act as though the config file were empty (which is the case when initially commenting out the defunct device & uuid from /etc/config/mdadm).

--scan will scan the config file or /proc/mdstat for missing information. NOT /proc/partitions.

So it would seem the original author's logic to use $CONF makes good sense. The compiled in default configs don't exist in Openwrt. In their absence only /proc/mdstat is consulted. That will only exist if an array's already been assembled at least once.

This comment has been minimized.

@cshoredaniel

cshoredaniel Jan 5, 2019

Contributor

will comment more later (but kudos for all the work you're doing on this).

Thist is what I'm referring to though '--assemble with --scan' is special-cased to do:

   Auto Assembly
       When  --assemble is used with --scan and no devices are listed, mdadm will first attempt to assemble
       all the arrays listed in the config file.

       If no arrays are listed in the config (other than those marked <ignore>) it will  look  through  the
       available devices for possible arrays and will try to assemble anything that it finds.  Arrays which
       are tagged as belonging to the given homehost will be assembled and started normally.  Arrays  which
       do not obviously belong to this host are given names that are expected not to conflict with anything
       local, and are started "read-auto" so that nothing is written to any device until the array is writ‐
       ten to. i.e.  automatic resync etc is delayed.

This comment has been minimized.

@cshoredaniel

cshoredaniel Jan 5, 2019

Contributor

Although I'm think that would be best with an empty config file, so skipping --conf bit isn't really necessary (I think back when I did it, I relied on shipping an entirely commented (or non-existent) mdadm.conf file so that the kernel would auto assemble. However that was based on a fairly linear setup and doesn't deal with e.g. hotplug which is the norm in openwrt (in my case I was using old x86 hardware with internal hard drives).

This comment has been minimized.

@josephtingiris

josephtingiris Jan 5, 2019

I think --config=none would produce the desired results with minimal effort. It would be fairly trivial to tweak $CONF=none rather than rely on /var/etc/mdadm.conf. It's certainly worth testing.

Introducing a second uci defaults option increases complexity and flexibility, e.g. option config none could pass through the actual (valid) values of none, partition, or (writable) file. If config is unset or empty then stay using the global value of /var/etc/mdadm.conf. If autoconfig is true (or false) and config is 'none' then attempt stateless Auto Assembly regardless? Or, should it always prefer to use a stateful config if autoconfig is true and config is 'none'?

Another approach would be to use just one config option with 3 (or more) validated values and some would pass through. Valid values would be auto, partition, none, or a (writable) file location. It would only fall back to the global /var/etc/mdadm.conf for auto, a file not writable, or '' (zero length string).

This comment has been minimized.

@cshoredaniel

cshoredaniel Jan 5, 2019

Contributor

I'm not a core dev (was at one point, but 'resigned' as I was not well. Thing are much better, but I haven't tried to get back in...also not really doing much with the core system these days), so take what I say with a gain of salt, but my preference is the latter. I think the use is much more clear. @jow- @blogic thoughts?

@diizzyy

This comment has been minimized.

Copy link
Contributor

diizzyy commented Jan 4, 2019

@josephtingiris
Additionally (if you want) since you're already playing around with mdadm perhaps bump it to 4.1?

@chunkeey chunkeey added the packages label Jan 4, 2019

@josephcsible

This comment has been minimized.

Copy link
Contributor

josephcsible commented Jan 4, 2019

@diizzyy I don't think I'm who you meant to ping.

@diizzyy

This comment has been minimized.

Copy link
Contributor

diizzyy commented Jan 4, 2019

@josephcsible
Correct, sorry about that :/

@josephtingiris
Copy link

josephtingiris left a comment

Yeah, I will rev it up. I'll refocus on that when I'm done sorting out the device chain inits. kmod-md-raid456 as well?

START=13
STOP=98
START=12
STOP=99

This comment has been minimized.

@josephtingiris

josephtingiris Jan 5, 2019

I'll rebase once the dust settles. I really appreciate the feedback. This is how better things are built.

The START & STOP values are important bits. They directly relate to a larger scope across multiple packages. I've given it some thought; sysvinit is limited. They do need to be changed, though. I'm not quite done discovering what the 'best' value would be given the constraints. Here's my reasoning.

Block device mapped chains like this are fairly typical these days,

raw -> md -> cryptsetup -> lvm2 -> fs

Maybe there's no md, or no crypt, or no lvm, or maybe they're ordered differently. If lvm2 is activated before cryptsetup is opened then the order of operation would differ, etc. The point is, they all have a well defined place in the device block chain and should be started & stopped in a specific order depending on the specific chain's configuration.

Given the different possibilities, there's more than one linear use case. IMO, device mapping init is better suited to something that can track dependencies, such as systemd. With sysvinit it's more complicated to support all possibilities dynamically. I'm not familiar enough with uci or procd to clearly see how it'd be best integrated there.

That said, and given Openwrt 18+ still uses sysvinit patterns, a decision seems to be warranted. I know systemd is out of the question (for now) and in the meantime, all I've got is Life without systemd. So, I'm opting to get something done sooner than later. I'm trying to set the stage for correcting the most common use case in Openwrt 18+.

One of the cryptsetup FAQs is 2.8 Encryption on top of RAID or the other way round? and clear advice is given.

place encryption between RAID and filesystem

That brings us back to raw -> md -> cryptsetup -> lvm2 -> fs as the most probable common use case. Openwrt 18+ can support that now.

I'll come back to this thought with more detail. I'm in process of evaluating all included base packages & mapping their sequences. mdadm is right in the middle ...

config_load mdadm
config_foreach mdadm_common mdadm
config_foreach mdadm_array array
fi

$PROG --assemble --scan --config="$CONF"

This comment has been minimized.

@josephtingiris

josephtingiris Jan 5, 2019

Yes, I could ... though ... to what end? Simply --assemble --scan ??

How? Add another defaults similar to autoconfig ?? I'm using $CONF there too.

To paraphrase from man.

Taking out --config completely would revert back to use /etc/mdadm.conf and /etc/mdadm.conf.d, or if those are missing then /etc/mdadm/mdadm.conf and /etc/mdadm/mdadm.conf.d.

Using $CONF=partitions, or --config=partitions, then nothing will be read and mdadm will act as though the config file contained exactly DEVICE partitions containers and will read /proc/partitions to find a list of devices to scan and /proc/mdstat to find a list of containers to examine.

Aside, that may be a clue to where/why it doesn't initially assemble correctly when everything's commented out of uci.

Using $CONF=none, or --config=none, would cause mdadm to act as though the config file were empty.

If it's a directory then files ending in .conf are sorted lexically and processed as individual config files.

I'm assuming that if no config file(s) exist then mdadm will act as though the config file were empty (which is the case when initially commenting out the defunct device & uuid from /etc/config/mdadm).

--scan will scan the config file or /proc/mdstat for missing information. NOT /proc/partitions.

So it would seem the original author's logic to use $CONF makes good sense. The compiled in default configs don't exist in Openwrt. In their absence only /proc/mdstat is consulted. That will only exist if an array's already been assembled at least once.

@cshoredaniel

This comment has been minimized.

Copy link
Contributor

cshoredaniel commented Jan 5, 2019

@diizzyy

This comment has been minimized.

Copy link
Contributor

diizzyy commented Jan 5, 2019

@cshoredaniel
Unless someone says so it's not a requirement at all and even if there is people need to adopt both paths as it's agreed to use both. Doing otherwise is just counterproductive and wastes peoples time not to mention contradicts suggested paths for contributing.

@cshoredaniel

This comment has been minimized.

Copy link
Contributor

cshoredaniel commented Jan 5, 2019

@diizzyy So that wasn't my understanding, so I've asked on the list[0] to clarify.

[0]http://lists.infradead.org/pipermail/openwrt-devel/2019-January/015415.html

@diizzyy

This comment has been minimized.

Copy link
Contributor

diizzyy commented Jan 5, 2019

@cshoredaniel
https://openwrt.org/submitting-patches
That's the official guidelines

@cshoredaniel

This comment has been minimized.

Copy link
Contributor

cshoredaniel commented Jan 5, 2019

@diizzyy However the point is question is not a single patch but potentially a number of them -- the discussion extends beyond the current PR or any single PR, hence my pointing to the list.

@josephtingiris josephtingiris changed the title mdadm: add autoconfig feature; init bug fixes mdadm: add new init features; bug fixes Jan 7, 2019

@josephtingiris

This comment has been minimized.

Copy link

josephtingiris commented Jan 7, 2019

I'd originally created this PR from my fork's master branch. After reading the official guidelines (sorry), I followed the directions and created a new branch: https://github.com/josephtingiris/openwrt/tree/PR1713

I had some time today and finished what I was working on there. I've incorporated everything discussed here, tested, documented, and signed off. Hopefully it's not too much for one day.

In any event, we could ...

  1. Merge that back to my master and it should update this PR automatically. Or,
  2. Submit a new PR. Or,
  3. An admin could update this PR and change the base to my PR1713 branch.

Which is preferred?

@cshoredaniel

This comment has been minimized.

Copy link
Contributor

cshoredaniel commented Jan 7, 2019

@jow- @chunkeey @dedeckeh (most likely to respond quickly I think)

I've only skimmed the new commit so far but I'm impressed. Excellent work! (Nice finding X99 too).

@cshoredaniel

This comment has been minimized.

Copy link
Contributor

cshoredaniel commented Jan 7, 2019

@josephtingiris The lack of comments is because there is a time when OpenWrt ran on devices with 2 MB of flash and 8 MB of RAM. Currently 8 / 64 is a major part of the userbase and there are folks not happy that 4 / 32 is no longer possible without lots of custom work. If there were more dev time I think it'd be useful support multiple paths (micro, mini, normal, and big, for example), but reality is that there probably isn't enough dev resources for that.

@josephtingiris

This comment has been minimized.

Copy link

josephtingiris commented Jan 7, 2019

Thanks. I agree. Most products get by with S,M,L. You know it's time to review the roadmap when the need to support XS,XL, & XXL shows up. The new AC2600+ stuff is awesome. On the other end it's IoT gear. Linux has a place across the board. It all depends on how well the SDLC works and if everyone gets along.

There are only 3 files & 1 of them is the Makefile PKG_RELEASE bump. These are the significant two on my PR1713 branch.

https://github.com/josephtingiris/openwrt/blob/PR1713/package/utils/mdadm/files/mdadm.config
https://github.com/josephtingiris/openwrt/blob/PR1713/package/utils/mdadm/files/mdadm.init

@cshoredaniel

This comment has been minimized.

Copy link
Contributor

cshoredaniel commented Jan 9, 2019

@diizzyy http://lists.infradead.org/pipermail/openwrt-devel/2019-January/015437.html -- basically the reply from Jo is that most devs will miss conversations in GitHub PR's.

@diizzyy

This comment has been minimized.

Copy link
Contributor

diizzyy commented Jan 9, 2019

If that case, close down GitHub if it's going to some half hearted attempt and go official about it. This is one of many management issues....

@neheb

This comment has been minimized.

Copy link
Contributor

neheb commented Jan 10, 2019

Just saw this thread.

I've given up on pull requests here. The last two came out of laziness (used GitHub's edit function).

That being said, patchwork still has dormant patches.

@josephtingiris

This comment has been minimized.

Copy link

josephtingiris commented Jan 10, 2019

Yeah, it's a bit confusing. I went ahead and submitted a patch that mimics this PR to the list and see it in patchwork. Still wondering if I should merge back to my master & update this PR or close it?

@neheb

This comment has been minimized.

Copy link
Contributor

neheb commented Jan 10, 2019

Probably close. This won’t get any attention here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment