* Re: [GIT PULL] MD update for 4.12
From: Linus Torvalds @ 2017-05-03 17:27 UTC (permalink / raw)
To: Shaohua Li
Cc: Linux Kernel Mailing List, linux-raid@vger.kernel.org, Neil Brown
In-Reply-To: <20170501225048.whxlxsoebnxcfyto@kernel.org>
On Mon, May 1, 2017 at 3:50 PM, Shaohua Li <shli@kernel.org> wrote:
>
> Please pull MD update for 4.12. There are conflicts between MD tree and block
> tree, so I did a merge before the pull request.
I pulled this, but *please* don't do those broken merges.
Why is it broken? Let me just count the merge message in its entirety:
"Merge branch 'md-next' into md-linus"
that's it. That doesn't really help anything. There is *no*
explanation for the merge. No "What", no "Why", nothing to explain why
you are merging with that _particular_ point in history.
In contrast, when I do merges, even if I don't have a lot of commit
message (and I try to have explanations - I will complain to
submaintainers if they don't give me enough information), there's very
much some implied information: "somebody asked Linus to pull this
point". I don't just randomly merge random points in development.
That merge, in contrast, very much merged completely random points in time.
In fact, the particular point you merged makes no sense at all and is
probably the *worst* possible kind of merge point. It's not just a
random point in time (me having just merged the LED drivers), it's a
point in the middle of the first day of the merge window. Which is
just about the *worst* possible thing to do, because that's often the
point that is the least reliable.
Seriously. DO NOT DO THIS!
I'm perfectly happy handling merge conflicts. I *much* prefer that
over this kind of "easy but crappy merge".
I appreciate it a lot if submaintainers do a "test merge" to see what
might conflict, and then explain what the conflict is and why it
happened in the pull request. That's absolutely lovely - it's a nice
heads-up about what happened, and it shows that you cared and you knew
about the conflict, and it makes me all warm and fuzzy.
But this kind of random bad merge of me tree during the most volatile
time of the merge window? That's just wrong. It doesn't even help me,
I probably spent about as much time looking at the conflct with your
merge in place as I would have when I resolved it, and then spent more
time writing this "please don't do this" rant than it would have taken
me to just do the merge.
Yes, occasionally the conflicts end up being nasty enough that I ask
people to verify my work. And _very_ occasionally I might ask you to
actually do the merge for me, but I don't recall when something was
really messy enough for that to happen. This did not look that messy
at all.
I have sent a variation of this email to people (and the mailing list)
probably approaching a hundred times over the 10+ years we've used
git.
Don't do back-merges unless you have a *really* good reason for it,
and if you have that reason, you had damn well *explain* that reason
in the merge message.
And "make the merge easier for Linus" is _never_ a good reason. If you
want to make life easier for me, please just do that test-merge and
explanation of what's going on: lots of maintainers do that, and I
really appreciate it. If you feel the merge isn't trivial, you can
even make the merge available as a ".. this is how I merged it" branch
- when people do that, I will actually end up doing the merge on my
own first (without looking at your merge), and then re-do the whole
thing *with* your merge in a test-branch, and verify the differences.
Because the differences (if there are any - it's seldom the case other
than trivial whitespace or other slight differences) are actually
really interesting markers for me, and tend to show where subtle
issues were.
But don't ask me to pull a pre-merged thing that just hides the conflicts.
Please please please don't do this.
Linus
^ permalink raw reply
* Re: [GIT PULL] MD update for 4.12
From: Shaohua Li @ 2017-05-03 18:38 UTC (permalink / raw)
To: Linus Torvalds
Cc: Linux Kernel Mailing List, linux-raid@vger.kernel.org, Neil Brown
In-Reply-To: <CA+55aFxD4mukK20ukU6LySD_Ro9XD+k1SU5O3+3-A67EG8ER6w@mail.gmail.com>
On Wed, May 03, 2017 at 10:27:47AM -0700, Linus Torvalds wrote:
> On Mon, May 1, 2017 at 3:50 PM, Shaohua Li <shli@kernel.org> wrote:
> >
> > Please pull MD update for 4.12. There are conflicts between MD tree and block
> > tree, so I did a merge before the pull request.
>
> I pulled this, but *please* don't do those broken merges.
>
> Why is it broken? Let me just count the merge message in its entirety:
>
> "Merge branch 'md-next' into md-linus"
>
> that's it. That doesn't really help anything. There is *no*
> explanation for the merge. No "What", no "Why", nothing to explain why
> you are merging with that _particular_ point in history.
>
> In contrast, when I do merges, even if I don't have a lot of commit
> message (and I try to have explanations - I will complain to
> submaintainers if they don't give me enough information), there's very
> much some implied information: "somebody asked Linus to pull this
> point". I don't just randomly merge random points in development.
>
> That merge, in contrast, very much merged completely random points in time.
>
> In fact, the particular point you merged makes no sense at all and is
> probably the *worst* possible kind of merge point. It's not just a
> random point in time (me having just merged the LED drivers), it's a
> point in the middle of the first day of the merge window. Which is
> just about the *worst* possible thing to do, because that's often the
> point that is the least reliable.
>
> Seriously. DO NOT DO THIS!
>
> I'm perfectly happy handling merge conflicts. I *much* prefer that
> over this kind of "easy but crappy merge".
>
> I appreciate it a lot if submaintainers do a "test merge" to see what
> might conflict, and then explain what the conflict is and why it
> happened in the pull request. That's absolutely lovely - it's a nice
> heads-up about what happened, and it shows that you cared and you knew
> about the conflict, and it makes me all warm and fuzzy.
>
> But this kind of random bad merge of me tree during the most volatile
> time of the merge window? That's just wrong. It doesn't even help me,
> I probably spent about as much time looking at the conflct with your
> merge in place as I would have when I resolved it, and then spent more
> time writing this "please don't do this" rant than it would have taken
> me to just do the merge.
>
> Yes, occasionally the conflicts end up being nasty enough that I ask
> people to verify my work. And _very_ occasionally I might ask you to
> actually do the merge for me, but I don't recall when something was
> really messy enough for that to happen. This did not look that messy
> at all.
>
> I have sent a variation of this email to people (and the mailing list)
> probably approaching a hundred times over the 10+ years we've used
> git.
>
> Don't do back-merges unless you have a *really* good reason for it,
> and if you have that reason, you had damn well *explain* that reason
> in the merge message.
>
> And "make the merge easier for Linus" is _never_ a good reason. If you
> want to make life easier for me, please just do that test-merge and
> explanation of what's going on: lots of maintainers do that, and I
> really appreciate it. If you feel the merge isn't trivial, you can
> even make the merge available as a ".. this is how I merged it" branch
> - when people do that, I will actually end up doing the merge on my
> own first (without looking at your merge), and then re-do the whole
> thing *with* your merge in a test-branch, and verify the differences.
> Because the differences (if there are any - it's seldom the case other
> than trivial whitespace or other slight differences) are actually
> really interesting markers for me, and tend to show where subtle
> issues were.
>
> But don't ask me to pull a pre-merged thing that just hides the conflicts.
>
> Please please please don't do this.
Get it, thanks for the explanation!
^ permalink raw reply
* RAID creation resync behaviors
From: Shaohua Li @ 2017-05-03 20:27 UTC (permalink / raw)
To: linux-raid; +Cc: jes.sorensen, neilb
Hi,
Currently we have different resync behaviors in array creation.
- raid1: copy data from disk 0 to disk 1 (overwrite)
- raid10: read both disks, compare and write if there is difference (compare-write)
- raid4/5: read first n-1 disks, calculate parity and then write parity to the last disk (overwrite)
- raid6: read all disks, calculate parity and compare, and write if there is difference (compare-write)
Write whole disk is very unfriendly for SSD, because it reduces lifetime. And
if user already does a trim before creation, the unncessary write could make
SSD slower in the future. Could we prefer compare-write to overwrite if mdadm
detects the disks are SSD? Surely sometimes compare-write is slower than
overwrite, so maybe add new option in mdadm. An option to let mdadm trim SSD
before creation sounds reasonable too.
Thanks,
Shaohua
^ permalink raw reply
* Re: RAID creation resync behaviors
From: David Brown @ 2017-05-03 21:06 UTC (permalink / raw)
To: Shaohua Li, linux-raid; +Cc: jes.sorensen, neilb
In-Reply-To: <20170503202748.7r243wj5h4polt6y@kernel.org>
On 03/05/17 22:27, Shaohua Li wrote:
> Hi,
>
> Currently we have different resync behaviors in array creation.
>
> - raid1: copy data from disk 0 to disk 1 (overwrite)
> - raid10: read both disks, compare and write if there is difference (compare-write)
> - raid4/5: read first n-1 disks, calculate parity and then write parity to the last disk (overwrite)
> - raid6: read all disks, calculate parity and compare, and write if there is difference (compare-write)
>
> Write whole disk is very unfriendly for SSD, because it reduces lifetime. And
> if user already does a trim before creation, the unncessary write could make
> SSD slower in the future. Could we prefer compare-write to overwrite if mdadm
> detects the disks are SSD? Surely sometimes compare-write is slower than
> overwrite, so maybe add new option in mdadm. An option to let mdadm trim SSD
> before creation sounds reasonable too.
>
When doing the first sync, md tracks how far its sync has got, keeping a
record in the metadata in case it has to be restarted (such as due to a
reboot while syncing). Why not simply /not/ sync stripes until you
first write to them? It may be that a counter of synced stripes is not
enough, and you need a bitmap (like the write intent bitmap), but it
would reduce the creation sync time to 0 and avoid any writes at all.
David
^ permalink raw reply
* Re: RAID creation resync behaviors
From: Andreas Klauer @ 2017-05-03 23:58 UTC (permalink / raw)
To: Shaohua Li; +Cc: linux-raid, jes.sorensen, neilb
In-Reply-To: <20170503202748.7r243wj5h4polt6y@kernel.org>
On Wed, May 03, 2017 at 01:27:48PM -0700, Shaohua Li wrote:
> Write whole disk is very unfriendly for SSD, because it reduces lifetime.
> And if user already does a trim before creation, the unncessary write
> could make SSD slower in the future.
I'm not a kernel developer so maybe I shouldn't reply. Feel free to ignore.
I don't see this as a big issue, whoever uses SSD will likely also fstrim,
so all SSD will know about free blocks regardless how the drive was added
to the RAID.
You don't resync everyday and once populated with data you just can't help
but have many writes when adding / replacing drives. No way around it.
> An option to let mdadm trim SSD before creation sounds reasonable too.
This is my personal opinion but - there is way too much trim in Linux.
On HDD if you did a botched mkfs on the wrong device you still had a chance
to recover data, with SSD it's all gone in an eyeblink, because mkfs.ext4
and other programs unfortunately do trim without asking. Lots of people
come to this list only after already playing with mdadm --create and if
mdadm simply started trimming SSDs too, then all would be lost.
LVM has these nice metadata backups but they're rendered useless
if lvm.conf has issue_discards set to 1. Etc...
And it's entirely superfluous, there was a big hullabaloo when SSD were
new, everyone was concerned about how quickly they'd die when written to,
but tests show their endurance is considerably greater than advertized.
A single RAID resync won't put a dent in even a consumer's SSD lifetime.
At the same time you have two utilities blkdiscard and fstrim so anyone
who desires to trim can already easily do so with little effort. For SSD
that return zero after TRIM you can already create like this:
blkdiscard device1
blkdiscard device2
blkdiscard device3
echo 3 > /proc/sys/vm/drop_caches # optional: Linux caches trimmed data
mdadm --create --assume-clean /dev/md ... device1 device2 device3
If you wanted mdadm to do that directly, how about a mdadm --create --trim
which implies assume-clean? But in my opinion it should not happen unasked.
If it was up to me I'd even add a prompt asking to confirm dataloss...
As for overwrite vs. compare-write, I don't know if it's possible or
how painful it would be to implement but could you start out comparing,
continue while the data actually matches, but switch to presumably much
faster overwrite mode once there are sufficient mismatches? Perhaps with a
fallback option so it can go back to compare later if data starts to match.
So kind of a smart-compare-overwrite mode which would go something like:
Compare. Match.
Compare. Match.
Compare. Mismatch. Overwrite.
Compare. Mismatch. Overwrite x2.
Compare. Mismatch. Overwrite x4.
Compare. Match.
Compare. Mismatch. Overwrite x8.
Compare. Mismatch. Overwrite x16.
Perhaps cap the overwrite multiplier at a certain point...
Maybe a silly idea, I don't know.
Regards
Andreas Klauer
^ permalink raw reply
* Re: RAID creation resync behaviors
From: NeilBrown @ 2017-05-04 1:07 UTC (permalink / raw)
To: Shaohua Li, linux-raid; +Cc: jes.sorensen, neilb
In-Reply-To: <20170503202748.7r243wj5h4polt6y@kernel.org>
[-- Attachment #1: Type: text/plain, Size: 2120 bytes --]
On Wed, May 03 2017, Shaohua Li wrote:
> Hi,
>
> Currently we have different resync behaviors in array creation.
>
> - raid1: copy data from disk 0 to disk 1 (overwrite)
> - raid10: read both disks, compare and write if there is difference (compare-write)
> - raid4/5: read first n-1 disks, calculate parity and then write parity to the last disk (overwrite)
> - raid6: read all disks, calculate parity and compare, and write if there is difference (compare-write)
The approach taken for raid1 and raid4/5 provides the fastest sync for
an array built on uninitialised spinning devices.
RAID6 could use the same approach but would involve more CPU and so
the original author of the RAID6 code (hpa) chose to go for the low-CPU
cost option. I don't know if tests were done, or if they would still be
valid on new hardware.
The raid10 approach comes from "it is too hard to optimize in general
because different RAID10 layouts have different trade-offs, so just
take the easy way out."
>
> Write whole disk is very unfriendly for SSD, because it reduces lifetime. And
> if user already does a trim before creation, the unncessary write could make
> SSD slower in the future. Could we prefer compare-write to overwrite if mdadm
> detects the disks are SSD? Surely sometimes compare-write is slower than
> overwrite, so maybe add new option in mdadm. An option to let mdadm trim SSD
> before creation sounds reasonable too.
An option to ask mdadm to trim the data space and then --assume-clean
certainly sounds reasonable.
One possible approach would be to use compare-write until some
threshold of writes were crossed, then switch to over-write. That could
work well for RAID1, but could be awkward to manage for RAID5.
Possibly mdadm could read the first few megas of each device in RAID5
and try to guess if many writes will be needed. If they will, the
current approach is best. If not, assemble the array so that
compare-write is used.
I'm in favour of providing options and making the defaults "not
terrible". I think they currently are "not terrible", but maybe they
can be better in some cases.
NeilBrown
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]
^ permalink raw reply
* Re: RAID creation resync behaviors
From: Shaohua Li @ 2017-05-04 1:54 UTC (permalink / raw)
To: David Brown; +Cc: linux-raid, jes.sorensen, neilb
In-Reply-To: <fe3c30e4-b0dd-639e-7db7-344aa93ce198@hesbynett.no>
On Wed, May 03, 2017 at 11:06:01PM +0200, David Brown wrote:
> On 03/05/17 22:27, Shaohua Li wrote:
> > Hi,
> >
> > Currently we have different resync behaviors in array creation.
> >
> > - raid1: copy data from disk 0 to disk 1 (overwrite)
> > - raid10: read both disks, compare and write if there is difference (compare-write)
> > - raid4/5: read first n-1 disks, calculate parity and then write parity to the last disk (overwrite)
> > - raid6: read all disks, calculate parity and compare, and write if there is difference (compare-write)
> >
> > Write whole disk is very unfriendly for SSD, because it reduces lifetime. And
> > if user already does a trim before creation, the unncessary write could make
> > SSD slower in the future. Could we prefer compare-write to overwrite if mdadm
> > detects the disks are SSD? Surely sometimes compare-write is slower than
> > overwrite, so maybe add new option in mdadm. An option to let mdadm trim SSD
> > before creation sounds reasonable too.
> >
>
> When doing the first sync, md tracks how far its sync has got, keeping a
> record in the metadata in case it has to be restarted (such as due to a
> reboot while syncing). Why not simply /not/ sync stripes until you first
> write to them? It may be that a counter of synced stripes is not enough,
> and you need a bitmap (like the write intent bitmap), but it would reduce
> the creation sync time to 0 and avoid any writes at all.
For raid 4/5/6, this means we always must do a full stripe write for any normal
write if it hits a range not synced. This would harm the performance of the
norma write. For raid1/10, this sounds more appealing. But since each bit in
the bitmap will stand for a range. If only part of the range is written by
normal IO, we have two choices. sync the range immediately and clear the bit,
this sync will impact normal IO. Don't do the sync immediately, but since the
bit is set (which means the range isn't synced), read IO can only access the
first disk, which is harmful too.
Thanks,
Shaohua
^ permalink raw reply
* Re: RAID creation resync behaviors
From: Shaohua Li @ 2017-05-04 2:04 UTC (permalink / raw)
To: NeilBrown; +Cc: linux-raid, jes.sorensen, neilb
In-Reply-To: <87inlhpgzu.fsf@notabene.neil.brown.name>
On Thu, May 04, 2017 at 11:07:01AM +1000, Neil Brown wrote:
> On Wed, May 03 2017, Shaohua Li wrote:
>
> > Hi,
> >
> > Currently we have different resync behaviors in array creation.
> >
> > - raid1: copy data from disk 0 to disk 1 (overwrite)
> > - raid10: read both disks, compare and write if there is difference (compare-write)
> > - raid4/5: read first n-1 disks, calculate parity and then write parity to the last disk (overwrite)
> > - raid6: read all disks, calculate parity and compare, and write if there is difference (compare-write)
>
> The approach taken for raid1 and raid4/5 provides the fastest sync for
> an array built on uninitialised spinning devices.
> RAID6 could use the same approach but would involve more CPU and so
> the original author of the RAID6 code (hpa) chose to go for the low-CPU
> cost option. I don't know if tests were done, or if they would still be
> valid on new hardware.
> The raid10 approach comes from "it is too hard to optimize in general
> because different RAID10 layouts have different trade-offs, so just
> take the easy way out."
ok, thanks for the explanation!
> >
> > Write whole disk is very unfriendly for SSD, because it reduces lifetime. And
> > if user already does a trim before creation, the unncessary write could make
> > SSD slower in the future. Could we prefer compare-write to overwrite if mdadm
> > detects the disks are SSD? Surely sometimes compare-write is slower than
> > overwrite, so maybe add new option in mdadm. An option to let mdadm trim SSD
> > before creation sounds reasonable too.
>
> An option to ask mdadm to trim the data space and then --assume-clean
> certainly sounds reasonable.
This doesn't work well. read returns 0 for trimmed data space in some SSDs, but
not all. If not, we will have trouble.
> One possible approach would be to use compare-write until some
> threshold of writes were crossed, then switch to over-write. That could
> work well for RAID1, but could be awkward to manage for RAID5.
> Possibly mdadm could read the first few megas of each device in RAID5
> and try to guess if many writes will be needed. If they will, the
> current approach is best. If not, assemble the array so that
> compare-write is used.
I think this makes sense if we do trim first, assume in most SSDs read return 0
for trimmed space. Maybe trim first, and check if read returns 0. If returns 0,
do compare-write (even assume-clean), otherwise overwrite.
> I'm in favour of providing options and making the defaults "not
> terrible". I think they currently are "not terrible", but maybe they
> can be better in some cases.
Agree, more options are required.
Thanks,
Shaohua
^ permalink raw reply
* Re: RAID creation resync behaviors
From: Shaohua Li @ 2017-05-04 2:22 UTC (permalink / raw)
To: Andreas Klauer; +Cc: linux-raid, jes.sorensen, neilb
In-Reply-To: <20170503235856.GA9698@metamorpher.de>
On Thu, May 04, 2017 at 01:58:56AM +0200, Andreas Klauer wrote:
> On Wed, May 03, 2017 at 01:27:48PM -0700, Shaohua Li wrote:
> > Write whole disk is very unfriendly for SSD, because it reduces lifetime.
> > And if user already does a trim before creation, the unncessary write
> > could make SSD slower in the future.
>
> I'm not a kernel developer so maybe I shouldn't reply. Feel free to ignore.
>
> I don't see this as a big issue, whoever uses SSD will likely also fstrim,
> so all SSD will know about free blocks regardless how the drive was added
> to the RAID.
>
> You don't resync everyday and once populated with data you just can't help
> but have many writes when adding / replacing drives. No way around it.
I agree fstrim doesn't make issue smaller. But there are still extra writes,
which if we can avoid in an easy way, we should do it.
> > An option to let mdadm trim SSD before creation sounds reasonable too.
>
> This is my personal opinion but - there is way too much trim in Linux.
>
> On HDD if you did a botched mkfs on the wrong device you still had a chance
> to recover data, with SSD it's all gone in an eyeblink, because mkfs.ext4
> and other programs unfortunately do trim without asking. Lots of people
> come to this list only after already playing with mdadm --create and if
> mdadm simply started trimming SSDs too, then all would be lost.
> LVM has these nice metadata backups but they're rendered useless
> if lvm.conf has issue_discards set to 1. Etc...
Totally understand the concerns. I think a new option is required for this and
it should not be default.
> And it's entirely superfluous, there was a big hullabaloo when SSD were
> new, everyone was concerned about how quickly they'd die when written to,
> but tests show their endurance is considerably greater than advertized.
> A single RAID resync won't put a dent in even a consumer's SSD lifetime.
From my experience, if my filesystem under a SSD is nearly full, I found the
system is unstable in at least one type of SSD. Fully write a SSD not just
reduces the lifetime, it also makes firmware of SSD has higher chance to fail.
> At the same time you have two utilities blkdiscard and fstrim so anyone
> who desires to trim can already easily do so with little effort. For SSD
> that return zero after TRIM you can already create like this:
>
> blkdiscard device1
> blkdiscard device2
> blkdiscard device3
> echo 3 > /proc/sys/vm/drop_caches # optional: Linux caches trimmed data
> mdadm --create --assume-clean /dev/md ... device1 device2 device3
Unfortunately not all SSD return zero after trim.
> If you wanted mdadm to do that directly, how about a mdadm --create --trim
> which implies assume-clean? But in my opinion it should not happen unasked.
> If it was up to me I'd even add a prompt asking to confirm dataloss...
>
> As for overwrite vs. compare-write, I don't know if it's possible or
> how painful it would be to implement but could you start out comparing,
> continue while the data actually matches, but switch to presumably much
> faster overwrite mode once there are sufficient mismatches? Perhaps with a
> fallback option so it can go back to compare later if data starts to match.
>
> So kind of a smart-compare-overwrite mode which would go something like:
>
> Compare. Match.
> Compare. Match.
> Compare. Mismatch. Overwrite.
> Compare. Mismatch. Overwrite x2.
> Compare. Mismatch. Overwrite x4.
> Compare. Match.
> Compare. Mismatch. Overwrite x8.
> Compare. Mismatch. Overwrite x16.
>
> Perhaps cap the overwrite multiplier at a certain point...
>
> Maybe a silly idea, I don't know.
This certainly is an interesting idea. Not sure if we should put complex
heuristics into kernel side though. If there is easy approach in mdadm side, it
definitely will be preferred.
Thanks,
Shaohua
^ permalink raw reply
* DM_INTEGRITY Kconfig help (was: Re: dm: add integrity target)
From: Geert Uytterhoeven @ 2017-05-04 6:03 UTC (permalink / raw)
To: Mikulas Patocka, Milan Broz, Mike Snitzer
Cc: Linux Kernel Mailing List, linux-raid
On Wed, May 3, 2017 at 7:43 PM, Linux Kernel Mailing List
<linux-kernel@vger.kernel.org> wrote:
> Web: https://git.kernel.org/torvalds/c/7eada909bfd7ac90a4522e56aa3179d1fd68cd14
> Commit: 7eada909bfd7ac90a4522e56aa3179d1fd68cd14
> Parent: 400a0befc96240f7bb2a53b9622deffd55d385fe
> Refname: refs/heads/master
> Author: Mikulas Patocka <mpatocka@redhat.com>
> AuthorDate: Wed Jan 4 20:23:53 2017 +0100
> Committer: Mike Snitzer <snitzer@redhat.com>
> CommitDate: Fri Mar 24 15:49:07 2017 -0400
>
> dm: add integrity target
>
> The dm-integrity target emulates a block device that has additional
> per-sector tags that can be used for storing integrity information.
>
> A general problem with storing integrity tags with every sector is that
> writing the sector and the integrity tag must be atomic - i.e. in case of
> crash, either both sector and integrity tag or none of them is written.
>
> To guarantee write atomicity the dm-integrity target uses a journal. It
> writes sector data and integrity tags into a journal, commits the journal
> and then copies the data and integrity tags to their respective location.
>
> The dm-integrity target can be used with the dm-crypt target - in this
> situation the dm-crypt target creates the integrity data and passes them
> to the dm-integrity target via bio_integrity_payload attached to the bio.
> In this mode, the dm-crypt and dm-integrity targets provide authenticated
> disk encryption - if the attacker modifies the encrypted device, an I/O
> error is returned instead of random data.
>
> The dm-integrity target can also be used as a standalone target, in this
> mode it calculates and verifies the integrity tag internally. In this
> mode, the dm-integrity target can be used to detect silent data
> corruption on the disk or in the I/O path.
>
> Signed-off-by: Mikulas Patocka <mpatocka@redhat.com>
> Signed-off-by: Milan Broz <gmazyland@gmail.com>
> Signed-off-by: Mike Snitzer <snitzer@redhat.com>
> --- a/drivers/md/Kconfig
> +++ b/drivers/md/Kconfig
> @@ -500,4 +500,14 @@ config DM_LOG_WRITES
>
> If unsure, say N.
>
> +config DM_INTEGRITY
> + tristate "Integrity target"
> + depends on BLK_DEV_DM
> + select BLK_DEV_INTEGRITY
> + select DM_BUFIO
> + select CRYPTO
> + select ASYNC_XOR
> + ---help---
> + This is the integrity target.
Which is...?
Can you please extend the help message for the uneducated?
Thanks!
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* Re: DM_INTEGRITY Kconfig help (was: Re: dm: add integrity target)
From: Milan Broz @ 2017-05-04 7:31 UTC (permalink / raw)
To: Geert Uytterhoeven, Mikulas Patocka, Mike Snitzer
Cc: Linux Kernel Mailing List, linux-raid
In-Reply-To: <CAMuHMdWafrUq0tbHsOS2Gm579Amv0LpYAqk=Kqg4t8fqgbWDgw@mail.gmail.com>
On 05/04/2017 08:03 AM, Geert Uytterhoeven wrote:
>> +config DM_INTEGRITY
>> + tristate "Integrity target"
>> + depends on BLK_DEV_DM
>> + select BLK_DEV_INTEGRITY
>> + select DM_BUFIO
>> + select CRYPTO
>> + select ASYNC_XOR
>> + ---help---
>> + This is the integrity target.
>
> Which is...?
>
> Can you please extend the help message for the uneducated?
Yes, sorry, this is our oversight.
Mike already promised that this text will be extended in the next patchset.
For now you can find the description in documentation
Documentation/device-mapper/dm-integrity.txt
"The dm-integrity target emulates a block device that has additional
per-sector tags that can be used for storing integrity information."
(The major designed use case is for providing metadata space for tags
for authenticated encryption in dm-crypt but it can be used also as
a standalone target.)
Thanks,
Milan
^ permalink raw reply
* Re: RAID creation resync behaviors
From: David Brown @ 2017-05-04 7:37 UTC (permalink / raw)
To: Shaohua Li; +Cc: linux-raid, jes.sorensen, neilb
In-Reply-To: <20170504015454.d4obiuume6e3yrdv@kernel.org>
On 04/05/17 03:54, Shaohua Li wrote:
> On Wed, May 03, 2017 at 11:06:01PM +0200, David Brown wrote:
>> On 03/05/17 22:27, Shaohua Li wrote:
>>> Hi,
>>>
>>> Currently we have different resync behaviors in array creation.
>>>
>>> - raid1: copy data from disk 0 to disk 1 (overwrite)
>>> - raid10: read both disks, compare and write if there is difference (compare-write)
>>> - raid4/5: read first n-1 disks, calculate parity and then write parity to the last disk (overwrite)
>>> - raid6: read all disks, calculate parity and compare, and write if there is difference (compare-write)
>>>
>>> Write whole disk is very unfriendly for SSD, because it reduces lifetime. And
>>> if user already does a trim before creation, the unncessary write could make
>>> SSD slower in the future. Could we prefer compare-write to overwrite if mdadm
>>> detects the disks are SSD? Surely sometimes compare-write is slower than
>>> overwrite, so maybe add new option in mdadm. An option to let mdadm trim SSD
>>> before creation sounds reasonable too.
>>>
>>
>> When doing the first sync, md tracks how far its sync has got, keeping a
>> record in the metadata in case it has to be restarted (such as due to a
>> reboot while syncing). Why not simply /not/ sync stripes until you first
>> write to them? It may be that a counter of synced stripes is not enough,
>> and you need a bitmap (like the write intent bitmap), but it would reduce
>> the creation sync time to 0 and avoid any writes at all.
>
> For raid 4/5/6, this means we always must do a full stripe write for any normal
> write if it hits a range not synced. This would harm the performance of the
> norma write.
Agreed. The unused sectors could be set to 0, rather than read from the
disks - that would reduce the latency and be friendly to high-end SSDs
with compression (zero blocks compress quite well!).
> For raid1/10, this sounds more appealing. But since each bit in
> the bitmap will stand for a range. If only part of the range is written by
> normal IO, we have two choices. sync the range immediately and clear the bit,
> this sync will impact normal IO. Don't do the sync immediately, but since the
> bit is set (which means the range isn't synced), read IO can only access the
> first disk, which is harmful too.
>
This could be done in a more sophisticated manner. (Yes, I appreciate
that "sophisticated" or "complex" are a serious disadvantage - I'm just
throwing up ideas that could be considered.)
Divide the array into "sync blocks", each covering a range of stripes,
with a bitmap of three states - unused, partially synced, fully synced.
All blocks start off unused. If a write is made to a previously unused
block, that block becomes partially synced, and the write has to be done
as a full stripe write. For a partially synced block, keep a list of
ranges of synced stripes (a list will normally be smaller than a bitmap
here). Whenever there are partially synced blocks in the array, have a
low priority process (like the normal array creation sync process, or
rebuild processes) sync the stripes until the block is finished as a
fully synced block.
That should let you delay the time-consuming and write intensive
creation sync until you actually need to sync the blocks, without /too/
much overhead in metadata or in delays when using the disk.
I have another couple of questions that might be relevant, but I am
really not sure about the correct answers.
First, if you have a stripe that you know is unused - it has not been
written to since the array was created - could the raid layer safely
return all zeros if an attempt was made to read the stripe?
Second, when syncing an unused stripe (such as during creation), rather
than reading the old data and copying it or generating parities, could
we simply write all zeros to all the blocks in the stripes? For many
SSDs, this is very efficient.
Best regards,
David
^ permalink raw reply
* Re: DM_INTEGRITY Kconfig help (was: Re: dm: add integrity target)
From: Geert Uytterhoeven @ 2017-05-04 7:39 UTC (permalink / raw)
To: Milan Broz
Cc: Mikulas Patocka, Mike Snitzer, Linux Kernel Mailing List,
linux-raid
In-Reply-To: <6988fc7b-1ab4-4c7a-c7c9-121271ddd970@gmail.com>
Hi Milan,
On Thu, May 4, 2017 at 9:31 AM, Milan Broz <gmazyland@gmail.com> wrote:
> On 05/04/2017 08:03 AM, Geert Uytterhoeven wrote:
>>> +config DM_INTEGRITY
>>> + tristate "Integrity target"
>>> + depends on BLK_DEV_DM
>>> + select BLK_DEV_INTEGRITY
>>> + select DM_BUFIO
>>> + select CRYPTO
>>> + select ASYNC_XOR
>>> + ---help---
>>> + This is the integrity target.
>>
>> Which is...?
>>
>> Can you please extend the help message for the uneducated?
>
> Yes, sorry, this is our oversight.
>
> Mike already promised that this text will be extended in the next patchset.
Thanks, good to know this is planned.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* Re: RAID creation resync behaviors
From: Andreas Klauer @ 2017-05-04 7:55 UTC (permalink / raw)
To: Shaohua Li; +Cc: linux-raid, jes.sorensen, neilb
In-Reply-To: <20170504022258.eov6xh2zwtbfvcch@kernel.org>
On Wed, May 03, 2017 at 07:22:58PM -0700, Shaohua Li wrote:
>
> Unfortunately not all SSD return zero after trim.
>
For example? I was under the impression that pretty much all of them do.
Even the ones that don't advertize it returned zero after trim for me.
[Sometimes you get original data but that's Linux; gone after drop_caches]
(Of course, I don't have access to that many different models of SSD...)
But what do they actually return then? Original data? This might seem
fine at first but it can't be the case, right? I mean, even if the SSD
does not erase a trimmed block right away, sooner or later it would,
in order to re-use it for new data and wear leveling. If it's never
re-used what is the point of trimming it in the first place?
So it seems to me even if an SSD attempted to keep returning original data
for a while, sooner or later it would be gone and if that happens randomly,
as far as your RAID is concerned you will be in mismatch hell anyways.
So after a full trim, you can skip the initial sync either way.
Mismatches might also be an issue for SSD that do return zero after trim,
but have different erase block sizes / trim boundaries / partition offsets.
Or does read zero after trim actually mean it works at 4K page resolution?
Never mind, then. :-)
Regards
Andreas Klauer
^ permalink raw reply
* Fwd: Re: Fwd: Please Help! Degraded Raid6
From: Bernd @ 2017-05-04 7:58 UTC (permalink / raw)
To: linux-raid; +Cc: Andreas.Klauer
In-Reply-To: <05DCE993-EEA8-420F-BB41-63612E361394@gmx.de>
-------- Ursprüngliche Nachricht --------
Von: Bernd <berndman80@gmx.de>
Gesendet: 3. Mai 2017 00:53:04 MESZ
An: Andreas Klauer <Andreas.Klauer@metamorpher.de>
Betreff: Re: Fwd: Please Help! Degraded Raid6
Hello, do this help?
berndman@berndman:~$ sudo mdadm --examine /dev/sd[a-z]1
[sudo] password for berndman:
/dev/sda1:
Magic : a92b4efc
Version : 1.2
Feature Map : 0x0
Array UUID : 26f8d8f1:530a3a85:31e4df5e:adc64cb8
Name : berndman:0 (local to host berndman)
Creation Time : Wed Dec 19 20:39:11 2012
Raid Level : raid6
Raid Devices : 8
Avail Dev Size : 3907025072 (1863.01 GiB 2000.40 GB)
Array Size : 23442112512 (11178.07 GiB 12002.36 GB)
Used Dev Size : 3907018752 (1863.01 GiB 2000.39 GB)
Data Offset : 2048 sectors
Super Offset : 8 sectors
State : active
Device UUID : 529d3611:a91ea3d1:5a643bf9:2c66715c
Update Time : Fri Mar 31 14:56:50 2017
Checksum : c781f805 - correct
Events : 1686
Layout : left-symmetric
Chunk Size : 4096K
Device Role : Active device 2
Array State : AAAAAAA. ('A' == active, '.' == missing)
/dev/sdb1:
Magic : a92b4efc
Version : 1.2
Feature Map : 0x0
Array UUID : 26f8d8f1:530a3a85:31e4df5e:adc64cb8
Name : berndman:0 (local to host berndman)
Creation Time : Wed Dec 19 20:39:11 2012
Raid Level : raid6
Raid Devices : 8
Avail Dev Size : 3907025072 (1863.01 GiB 2000.40 GB)
Array Size : 23442112512 (11178.07 GiB 12002.36 GB)
Used Dev Size : 3907018752 (1863.01 GiB 2000.39 GB)
Data Offset : 2048 sectors
Super Offset : 8 sectors
State : active
Device UUID : bb3dc649:52726bf3:5512b2c1:f0a7605e
Update Time : Fri Mar 31 14:56:50 2017
Checksum : ec3fdbec - correct
Events : 1686
Layout : left-symmetric
Chunk Size : 4096K
Device Role : Active device 6
Array State : AAAAAAA. ('A' == active, '.' == missing)
/dev/sdc1:
Magic : a92b4efc
Version : 1.2
Feature Map : 0x0
Array UUID : 26f8d8f1:530a3a85:31e4df5e:adc64cb8
Name : berndman:0 (local to host berndman)
Creation Time : Wed Dec 19 20:39:11 2012
Raid Level : raid6
Raid Devices : 8
Avail Dev Size : 3907025072 (1863.01 GiB 2000.40 GB)
Array Size : 23442112512 (11178.07 GiB 12002.36 GB)
Used Dev Size : 3907018752 (1863.01 GiB 2000.39 GB)
Data Offset : 2048 sectors
Super Offset : 8 sectors
State : active
Device UUID : cb730c6a:eb14ee35:4f1b3480:63e037c8
Update Time : Fri Mar 31 14:56:50 2017
Checksum : 7761f5e9 - correct
Events : 1686
Layout : left-symmetric
Chunk Size : 4096K
Device Role : Active device 5
Array State : AAAAAAA. ('A' == active, '.' == missing)
/dev/sdd1:
Magic : a92b4efc
Version : 1.2
Feature Map : 0x0
Array UUID : 26f8d8f1:530a3a85:31e4df5e:adc64cb8
Name : berndman:0 (local to host berndman)
Creation Time : Wed Dec 19 20:39:11 2012
Raid Level : raid6
Raid Devices : 8
Avail Dev Size : 3907025072 (1863.01 GiB 2000.40 GB)
Array Size : 23442112512 (11178.07 GiB 12002.36 GB)
Used Dev Size : 3907018752 (1863.01 GiB 2000.39 GB)
Data Offset : 2048 sectors
Super Offset : 8 sectors
State : active
Device UUID : 61ef95f8:b5d2477b:e4a454f1:34f2b27b
Update Time : Fri Mar 31 14:56:50 2017
Checksum : 6fe0caac - correct
Events : 1686
Layout : left-symmetric
Chunk Size : 4096K
Device Role : Active device 1
Array State : AAAAAAA. ('A' == active, '.' == missing)
/dev/sde1:
Magic : a92b4efc
Version : 1.2
Feature Map : 0x0
Array UUID : 26f8d8f1:530a3a85:31e4df5e:adc64cb8
Name : berndman:0 (local to host berndman)
Creation Time : Wed Dec 19 20:39:11 2012
Raid Level : raid6
Raid Devices : 8
Avail Dev Size : 3907025072 (1863.01 GiB 2000.40 GB)
Array Size : 23442112512 (11178.07 GiB 12002.36 GB)
Used Dev Size : 3907018752 (1863.01 GiB 2000.39 GB)
Data Offset : 2048 sectors
Super Offset : 8 sectors
State : active
Device UUID : 7c16aad2:99309271:a8805e9a:f8be7e51
Update Time : Fri Mar 31 14:56:50 2017
Checksum : bf14f834 - correct
Events : 1686
Layout : left-symmetric
Chunk Size : 4096K
Device Role : Active device 3
Array State : AAAAAAA. ('A' == active, '.' == missing)
/dev/sdf1:
Magic : a92b4efc
Version : 1.2
Feature Map : 0x0
Array UUID : 26f8d8f1:530a3a85:31e4df5e:adc64cb8
Name : berndman:0 (local to host berndman)
Creation Time : Wed Dec 19 20:39:11 2012
Raid Level : raid6
Raid Devices : 8
Avail Dev Size : 3907025072 (1863.01 GiB 2000.40 GB)
Array Size : 23442112512 (11178.07 GiB 12002.36 GB)
Used Dev Size : 3907018752 (1863.01 GiB 2000.39 GB)
Data Offset : 2048 sectors
Super Offset : 8 sectors
State : active
Device UUID : 5c3a8490:3b33d565:4fb9c177:859e3155
Update Time : Fri Mar 31 14:56:50 2017
Checksum : 524836e7 - correct
Events : 1686
Layout : left-symmetric
Chunk Size : 4096K
Device Role : Active device 0
Array State : AAAAAAA. ('A' == active, '.' == missing)
mdadm: No md superblock detected on /dev/sdg1.
/dev/sdh1:
Magic : a92b4efc
Version : 1.2
Feature Map : 0x2
Array UUID : 26f8d8f1:530a3a85:31e4df5e:adc64cb8
Name : berndman:0 (local to host berndman)
Creation Time : Wed Dec 19 20:39:11 2012
Raid Level : raid6
Raid Devices : 8
Avail Dev Size : 3907025072 (1863.01 GiB 2000.40 GB)
Array Size : 23442112512 (11178.07 GiB 12002.36 GB)
Used Dev Size : 3907018752 (1863.01 GiB 2000.39 GB)
Data Offset : 2048 sectors
Super Offset : 8 sectors
Recovery Offset : 0 sectors
State : clean
Device UUID : 2283bdef:86a7c96b:d8ef0a1b:3651dd62
Update Time : Tue Sep 29 15:46:56 2015
Checksum : 65a01e2f - correct
Events : 843
Layout : left-symmetric
Chunk Size : 4096K
Device Role : Active device 7
Array State : AAAAAAAA ('A' == active, '.' == missing)
/dev/sdi1:
Magic : a92b4efc
Version : 1.2
Feature Map : 0x0
Array UUID : 26f8d8f1:530a3a85:31e4df5e:adc64cb8
Name : berndman:0 (local to host berndman)
Creation Time : Wed Dec 19 20:39:11 2012
Raid Level : raid6
Raid Devices : 8
Avail Dev Size : 3907025072 (1863.01 GiB 2000.40 GB)
Array Size : 23442112512 (11178.07 GiB 12002.36 GB)
Used Dev Size : 3907018752 (1863.01 GiB 2000.39 GB)
Data Offset : 2048 sectors
Super Offset : 8 sectors
State : clean
Device UUID : bade4a58:bce5a0a2:121368d5:f6ad5857
Update Time : Fri Mar 31 14:55:06 2017
Checksum : b6a7f695 - correct
Events : 1685
Layout : left-symmetric
Chunk Size : 4096K
Device Role : Active device 4
Array State : AAAAAAA. ('A' == active, '.' == missing)
berndman@berndman:~$
Am 3. Mai 2017 00:01:54 MESZ schrieb Andreas Klauer <Andreas.Klauer@metamorpher.de>:
>On Tue, May 02, 2017 at 11:11:10PM +0200, Bernd wrote:
>> berndman@berndman:~$ sudo mdadm --examine /dev/sd[a-z]
>
>You're using the wrong devices in your commands
>so the output is not the least bit interesting.
>
>> /dev/sda:
>> MBR Magic : aa55
>> Partition[0] : 3907027120 sectors at 2048 (type fd)
>> /dev/sdb:
>> MBR Magic : aa55
>> Partition[0] : 3907027120 sectors at 2048 (type fd)
>
>> /dev/sdh:
>> Magic : a92b4efc
>> Version : 1.2
>> Feature Map : 0x2
>> Array UUID : 26f8d8f1:530a3a85:31e4df5e:adc64cb8
>
>Well except for this but need all of them.
>
>> Update Time : Fri Jul 10 22:32:34 2015
>
>I hope it's not yet another case of a drive dropped out back
>in 2015 and you never doing anything about it in two years...
>Monitoring and instant email notification, is it that difficult?
>
>Regards
>Andreas Klauer
^ permalink raw reply
* Re: RAID creation resync behaviors
From: Roman Mamedov @ 2017-05-04 8:06 UTC (permalink / raw)
To: Andreas Klauer; +Cc: Shaohua Li, linux-raid, jes.sorensen, neilb
In-Reply-To: <20170504075551.GA3929@metamorpher.de>
On Thu, 4 May 2017 09:55:51 +0200
Andreas Klauer <Andreas.Klauer@metamorpher.de> wrote:
> For example? I was under the impression that pretty much all of them do.
> Even the ones that don't advertize it returned zero after trim for me.
> [Sometimes you get original data but that's Linux; gone after drop_caches]
> (Of course, I don't have access to that many different models of SSD...)
>
> But what do they actually return then? Original data?
Consult Wikipedia at least, if not the original ATA standards documents:
https://en.wikipedia.org/wiki/Trim_(computing)#ATA
Can return "undefined", "something that is the same every time" or "zeroes".
Apparently the OS can query which is to be expected with a particular device,
but firstly, you can't necessarily trust that 100%, and secondly, that's
operating on a level lower (ATA) than where md is.
--
With respect,
Roman
^ permalink raw reply
* Fault tolerance in RAID0 with badblocks
From: Ravi (Tom) Hale @ 2017-05-04 10:04 UTC (permalink / raw)
To: linux-raid
Since btrfs doesn't support badblocks, this btrfs mailing list post[1]
suggested to use mdadm RAID0 3.1+.
Is there a way of having blocks from a spare device automatically
replacing bad blocks when they are next written to (like SMART does for
HDDs)?
Or would mdadm be able to add a "badblocks layer" to btrfs in some other
way?
My use case is mining storj - I don't mind some data loss.
[1] https://www.spinics.net/lists/linux-btrfs/msg40909.html
--
Cheers,
Tom Hale
^ permalink raw reply
* Re: [mdadm PATCH] Create: tell udev md device is not ready when first created.
From: Peter Rajnoha @ 2017-05-04 10:58 UTC (permalink / raw)
To: NeilBrown, Jes Sorensen; +Cc: linux-raid, dm-devel
In-Reply-To: <87h919ruj5.fsf@notabene.neil.brown.name>
On 04/28/2017 07:05 AM, NeilBrown wrote:
>
> When an array is created the content is not initialized,
> so it could have remnants of an old filesystem or md array
> etc on it.
> udev will see this and might try to activate it, which is almost
> certainly not what is wanted.
>
> So create a mechanism for mdadm to communicate with udev to tell
> it that the device isn't ready. This mechanism is the existance
> of a file /run/mdadm/created-mdXXX where mdXXX is the md device name.
>
> When creating an array, mdadm will create the file.
> A new udev rule file, 01-md-raid-creating.rules, will detect the
> precense of thst file and set ENV{SYSTEMD_READY}="0".
> This is fairly uniformly used to suppress actions based on the
> contents of the device.
>
...
> diff --git a/udev-md-raid-creating.rules b/udev-md-raid-creating.rules
> new file mode 100644
> index 000000000000..2be466bcefd2
> --- /dev/null
> +++ b/udev-md-raid-creating.rules
> @@ -0,0 +1,7 @@
> +# do not edit this file, it will be overwritten on update
> +# While mdadm is creating an array, it creates a file
> +# /run/mdadm/creating-mdXXX. If that file exists, then
> +# the array is not "ready" and we should make sure the
> +# content is ignored.
> +
> +KERNEL=="md*", TEST="/run/mdadm/creating-$kernel", ENV{SYSTEMD_READY}="0"
>
This should be TEST=="/run/mdadm/creating-$kernel" (double equals sign,
otherwise it's ignored by udev as incorrect rule).
--
Peter
^ permalink raw reply
* Re: [dm-devel] [PATCH v6 0/3] dm: boot a mapped device without an initramfs
From: Enric Balletbo Serra @ 2017-05-04 11:18 UTC (permalink / raw)
To: Kees Cook
Cc: Enric Balletbo i Serra, Will Drewry, Guenter Roeck, Mike Snitzer,
linux-doc@vger.kernel.org, David Zeuthen, LKML, linux-raid,
dm-devel, Shaohua Li, Alasdair Kergon
In-Reply-To: <CAGXu5jLD+0POz1sopyZNNqDAiq2MZ5ZYdjs43jX5OnWxkZt1Lg@mail.gmail.com>
Mike,
2017-04-18 19:37 GMT+02:00 Kees Cook <keescook@chromium.org>:
> On Tue, Apr 18, 2017 at 9:42 AM, Enric Balletbo i Serra
> <enric.balletbo@collabora.com> wrote:
>> Hello,
>>
>> Some of these patches were send few years back, I saw that first
>> version was send to this list in 2010, and after version 4 did not
>> land [1]. Some days ago I resend the patches [2] and few hours later I
>> noticed that one year ago was send a v5 version [3] and I was not aware.
>>
>> There was some discussion about v5 and during the discussion Mike Snitzer
>> proposed that at least a change of the syntax is required, we're really
>> interested on see this upstream as is extensively used in ChromeOS based
>> devices so I'm wondering if we can restart the discussion and hopefully
>> we will be able to do the modifications needed.
>>
>> So my first question is, apart of the change of the syntax, what more
>> should be changed?
>
> AFAIK, this was the main change needed. Change the syntax and plumb
> into the ioctl interface. The discussion ended with Mike being open to
> the idea, and for me to go work on it. I haven't had time to work on
> it, though, so it has continued to be a locally carried patch:
> https://www.redhat.com/archives/dm-devel/2016-February/msg00199.html
>
From your email:
> >> > 2) If you are able to adequately justify the need for dm=:
> >> > I'd much rather the dm= kernel commandline be a simple series of
> >> > comma-delimited dmsetup-like commands.
> >> >
> >> > You'd handle each command with extremely basic parsing:
> >> > <dm_ioctl_cmd> <args> [, <dm_ioctl_cmd> <args>]
> >> > (inventing a special token to denote <newline>, to support tables with
> >> > multiple entries, rather than relying on commas and counts, etc)
> >>
I'm wondering if a command line like this would be acceptable.
Format is:
dm="<dev_name> <uuid> <mode>, <table>[, <table>][; <dev_name>
<uuid> <mode>, <table>[, <table>]][; ... ]"
where:
<dev_name> ::= The device name
<uuid> ::= xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx | "none"
<mode> ::= "ro" | "rw"
<table> ::= <start sector> <end sector> <target name>
<target parmaters>
An example of booting to a linear array:
dm="lroot none rw, 0 2097152 linear /dev/sda2 0, 2097152 2097152
linear /dev/sda3 0, 4194304 2097152 linear /dev/sda4 0"
Equivalent dmsetup command:
echo -e "0 2097152 linear /dev/sda2 0"\\n"2097152 2097152 linear
/dev/sda3 0"\\n"4194304 2097152 linear /dev/sda4 0" | sudo dmsetup
create lroot
An example of multiple device-mappers, with the dm="..." contents shown
here split on multiple lines for readability:
vroot none ro,
0 2097152 verity 1 /dev/sdb2 /dev/sdb3 4096 4096 262144 1 sha256 \
289b52edac1ac4f4c32c8f765795615a85d4daa454677d21a6d8767c4627dc48 \
632d7fe427a23a8e88493c553298a779997478a143d86da5d56a65db8a1f2a38;
vram none rw,
0 32768 linear 1:0 0,
32768 32768 linear 1:1 0
Thanks,
Enric
> More recently David Zeuthen has been poking at this code, so I've
> included him on CC here, in case there are new developments.
>
> -Kees
>
>>
>> Thanks for your help,
>> Enric
>>
>> [1] Patchwork links:
>> https://patchwork.kernel.org/patch/104857/
>> https://patchwork.kernel.org/patch/104856/
>> https://patchwork.kernel.org/patch/104858/
>>
>> [2] https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1375276.html
>>
>> [3] https://www.redhat.com/archives/dm-devel/2016-February/msg00112.html
>>
>>
>> Brian Norris (1):
>> dm: make some mapped_device functions available
>>
>> Will Drewry (2):
>> dm: export a table+mapped device to the ioctl interface
>> init: add support to directly boot to a mapped device
>>
>> Documentation/admin-guide/kernel-parameters.rst | 1 +
>> Documentation/admin-guide/kernel-parameters.txt | 3 +
>> Documentation/device-mapper/boot.txt | 65 ++++
>> drivers/md/dm-ioctl.c | 36 ++
>> drivers/md/dm.h | 8 -
>> include/linux/device-mapper.h | 19 +
>> init/Makefile | 1 +
>> init/do_mounts.c | 1 +
>> init/do_mounts.h | 10 +
>> init/do_mounts_dm.c | 448 ++++++++++++++++++++++++
>> 10 files changed, 584 insertions(+), 8 deletions(-)
>> create mode 100644 Documentation/device-mapper/boot.txt
>> create mode 100644 init/do_mounts_dm.c
>>
>> --
>> 2.9.3
>>
>
>
>
> --
> Kees Cook
> Pixel Security
>
> --
> dm-devel mailing list
> dm-devel@redhat.com
> https://www.redhat.com/mailman/listinfo/dm-devel
^ permalink raw reply
* Re: [PATCH v6 0/3] dm: boot a mapped device without an initramfs
From: Alasdair G Kergon @ 2017-05-04 12:14 UTC (permalink / raw)
To: Enric Balletbo Serra
Cc: Will Drewry, Kees Cook, Mike Snitzer, linux-doc@vger.kernel.org,
David Zeuthen, LKML, linux-raid, dm-devel, Guenter Roeck,
Enric Balletbo i Serra, Shaohua Li, Alasdair Kergon
In-Reply-To: <CAFqH_51YLoYrNcw+Fb99LbSY63wVWg+MW7nBYQqWjV6qfOAmNQ@mail.gmail.com>
On Thu, May 04, 2017 at 01:18:41PM +0200, Enric Balletbo Serra wrote:
> I'm wondering if a command line like this would be acceptable.
1) Make sure the implementation continues to support backslash quoting
so that any characters you introduce with special meanings (comma,
semi-colon, double-quote in that example) can still be used if required.
2) "none" is of course a valid uuid:) More comma-separation or
re-ordering, perhaps?
3) Whatever final format is agreed here should be supported by dmsetup
as well, so you can both supply the format to dmsetup and ask dmsetup
to display your existing devices in this format. Choose a format
that makes this easy.
Alasdair
^ permalink raw reply
* [PATCH v1 0/2] unify stat and fstat checking blkdev into functions
From: Zhilong Liu @ 2017-05-04 12:16 UTC (permalink / raw)
To: Jes.Sorensen; +Cc: linux-raid, Zhilong Liu
Hi, Jes;
This two patches declared two functions named fstat_is_blkdev()
and stat_is_blkdev() to integrate repeated checking block device
operations, decreased the repeated codes via to utility function.
And fix a bug in WaitClean(), it forgot to check block device.
This patchset depends on patchset
"[mdadm PATCH 0/2] unify stat and fstat into functions"
Changes:
- modified the return value of them, return true/1 if it is a
block device, return false/0 when it isn't.
- rename the functions and make their name shorter.
Thanks,
Zhilong
Zhilong Liu (2):
util: unify fstat checking blkdev into function
util: unify stat checking blkdev into function
Assemble.c | 26 +++++++++-----------------
Build.c | 30 +++++++-----------------------
Create.c | 23 ++++++++++-------------
Grow.c | 10 ++++------
Incremental.c | 52 +++++++++++++++-------------------------------------
Manage.c | 13 ++-----------
Monitor.c | 16 ++++------------
bitmap.c | 13 ++++---------
mdadm.h | 2 ++
super-ddf.c | 10 ++++------
super-intel.c | 23 +++++++----------------
util.c | 34 ++++++++++++++++++++++++++++++++++
12 files changed, 102 insertions(+), 150 deletions(-)
--
2.10.2
^ permalink raw reply
* [PATCH v1 1/2] mdadm/util: unify fstat checking blkdev into function
From: Zhilong Liu @ 2017-05-04 12:16 UTC (permalink / raw)
To: Jes.Sorensen; +Cc: linux-raid, Zhilong Liu
In-Reply-To: <20170504121622.4014-1-zlliu@suse.com>
declare function fstat_is_blkdev() to integrate repeated fstat
checking block device operations, it returns true/1 when it is
a block device, and returns false/0 when it isn't.
The fd and devname are necessary parameters, *rdev is optional,
parse the pointer of dev_t *rdev, if valid, assigned the device
number to dev_t *rdev, if NULL, ignores.
Signed-off-by: Zhilong Liu <zlliu@suse.com>
---
Assemble.c | 19 +++++++------------
Build.c | 5 +++--
Create.c | 23 ++++++++++-------------
Grow.c | 10 ++++------
Incremental.c | 33 ++++++++++++---------------------
Manage.c | 2 +-
bitmap.c | 13 ++++---------
mdadm.h | 1 +
super-intel.c | 13 +++----------
util.c | 17 +++++++++++++++++
10 files changed, 62 insertions(+), 74 deletions(-)
diff --git a/Assemble.c b/Assemble.c
index a9442c8..9d0a89f 100644
--- a/Assemble.c
+++ b/Assemble.c
@@ -149,6 +149,7 @@ static int select_devices(struct mddev_dev *devlist,
struct mdinfo *content = NULL;
int report_mismatch = ((inargv && c->verbose >= 0) || c->verbose > 0);
struct domainlist *domains = NULL;
+ dev_t rdev;
tmpdev = devlist; num_devs = 0;
while (tmpdev) {
@@ -169,7 +170,6 @@ static int select_devices(struct mddev_dev *devlist,
tmpdev = tmpdev ? tmpdev->next : NULL) {
char *devname = tmpdev->devname;
int dfd;
- struct stat stb;
struct supertype *tst;
struct dev_policy *pol = NULL;
int found_container = 0;
@@ -204,14 +204,7 @@ static int select_devices(struct mddev_dev *devlist,
pr_err("cannot open device %s: %s\n",
devname, strerror(errno));
tmpdev->used = 2;
- } else if (fstat(dfd, &stb)< 0) {
- /* Impossible! */
- pr_err("fstat failed for %s: %s\n",
- devname, strerror(errno));
- tmpdev->used = 2;
- } else if ((stb.st_mode & S_IFMT) != S_IFBLK) {
- pr_err("%s is not a block device.\n",
- devname);
+ } else if (!fstat_is_blkdev(dfd, devname, &rdev)) {
tmpdev->used = 2;
} else if (must_be_container(dfd)) {
if (st) {
@@ -234,7 +227,8 @@ static int select_devices(struct mddev_dev *devlist,
devname);
tmpdev->used = 2;
} else if (auto_assem &&
- !conf_test_metadata(tst->ss->name, (pol = devid_policy(stb.st_rdev)),
+ !conf_test_metadata(tst->ss->name,
+ (pol = devid_policy(rdev)),
tst->ss->match_home(tst, c->homehost) == 1)) {
if (report_mismatch)
pr_err("%s has metadata type %s for which auto-assembly is disabled\n",
@@ -261,7 +255,8 @@ static int select_devices(struct mddev_dev *devlist,
tst->ss->name, devname);
tmpdev->used = 2;
} else if (auto_assem && st == NULL &&
- !conf_test_metadata(tst->ss->name, (pol = devid_policy(stb.st_rdev)),
+ !conf_test_metadata(tst->ss->name,
+ (pol = devid_policy(rdev)),
tst->ss->match_home(tst, c->homehost) == 1)) {
if (report_mismatch)
pr_err("%s has metadata type %s for which auto-assembly is disabled\n",
@@ -484,7 +479,7 @@ static int select_devices(struct mddev_dev *devlist,
/* Collect domain information from members only */
if (tmpdev && tmpdev->used == 1) {
if (!pol)
- pol = devid_policy(stb.st_rdev);
+ pol = devid_policy(rdev);
domain_merge(&domains, pol, tst?tst->ss->name:NULL);
}
dev_policy_free(pol);
diff --git a/Build.c b/Build.c
index 665d906..2d84b96 100644
--- a/Build.c
+++ b/Build.c
@@ -42,6 +42,7 @@ int Build(char *mddev, struct mddev_dev *devlist,
*/
int i;
struct stat stb;
+ dev_t rdev;
int subdevs = 0, missing_disks = 0;
struct mddev_dev *dv;
int bitmap_fd;
@@ -126,8 +127,8 @@ int Build(char *mddev, struct mddev_dev *devlist,
array.nr_disks = s->raiddisks;
array.raid_disks = s->raiddisks;
array.md_minor = 0;
- if (fstat(mdfd, &stb) == 0)
- array.md_minor = minor(stb.st_rdev);
+ if (fstat_is_blkdev(mdfd, mddev, &rdev))
+ array.md_minor = minor(rdev);
array.not_persistent = 1;
array.state = 0; /* not clean, but no errors */
if (s->assume_clean)
diff --git a/Create.c b/Create.c
index df1bc20..239545f 100644
--- a/Create.c
+++ b/Create.c
@@ -89,8 +89,8 @@ int Create(struct supertype *st, char *mddev,
char *maxdisc = NULL;
int dnum, raid_disk_num;
struct mddev_dev *dv;
+ dev_t rdev;
int fail = 0, warn = 0;
- struct stat stb;
int first_missing = subdevs * 2;
int second_missing = subdevs * 2;
int missing_disks = 0;
@@ -325,11 +325,8 @@ int Create(struct supertype *st, char *mddev,
dname, strerror(errno));
exit(2);
}
- if (fstat(dfd, &stb) != 0 ||
- (stb.st_mode & S_IFMT) != S_IFBLK) {
+ if (!fstat_is_blkdev(dfd, dname, NULL)) {
close(dfd);
- pr_err("%s is not a block device\n",
- dname);
exit(2);
}
close(dfd);
@@ -641,8 +638,8 @@ int Create(struct supertype *st, char *mddev,
* with, but it chooses to trust me instead. Sigh
*/
info.array.md_minor = 0;
- if (fstat(mdfd, &stb) == 0)
- info.array.md_minor = minor(stb.st_rdev);
+ if (fstat_is_blkdev(mdfd, mddev, &rdev))
+ info.array.md_minor = minor(rdev);
info.array.not_persistent = 0;
if (((s->level == 4 || s->level == 5) &&
@@ -841,7 +838,6 @@ int Create(struct supertype *st, char *mddev,
for (dnum = 0, raid_disk_num = 0, dv = devlist; dv;
dv = (dv->next) ? (dv->next) : moved_disk, dnum++) {
int fd;
- struct stat stb2;
struct mdinfo *inf = &infos[dnum];
if (dnum >= total_slots)
@@ -897,9 +893,10 @@ int Create(struct supertype *st, char *mddev,
dv->devname);
goto abort_locked;
}
- fstat(fd, &stb2);
- inf->disk.major = major(stb2.st_rdev);
- inf->disk.minor = minor(stb2.st_rdev);
+ if (!fstat_is_blkdev(fd, dv->devname, &rdev))
+ return 1;
+ inf->disk.major = major(rdev);
+ inf->disk.minor = minor(rdev);
}
if (fd >= 0)
remove_partitions(fd);
@@ -920,8 +917,8 @@ int Create(struct supertype *st, char *mddev,
if (!have_container) {
/* getinfo_super might have lost these ... */
- inf->disk.major = major(stb2.st_rdev);
- inf->disk.minor = minor(stb2.st_rdev);
+ inf->disk.major = major(rdev);
+ inf->disk.minor = minor(rdev);
}
break;
case 2:
diff --git a/Grow.c b/Grow.c
index f4bd301..a527436 100644
--- a/Grow.c
+++ b/Grow.c
@@ -109,7 +109,7 @@ int Grow_Add_device(char *devname, int fd, char *newdev)
*/
struct mdinfo info;
- struct stat stb;
+ dev_t rdev;
int nfd, fd2;
int d, nd;
struct supertype *st = NULL;
@@ -145,9 +145,7 @@ int Grow_Add_device(char *devname, int fd, char *newdev)
free(st);
return 1;
}
- fstat(nfd, &stb);
- if ((stb.st_mode & S_IFMT) != S_IFBLK) {
- pr_err("%s is not a block device!\n", newdev);
+ if (!fstat_is_blkdev(nfd, newdev, &rdev)) {
close(nfd);
free(st);
return 1;
@@ -198,8 +196,8 @@ int Grow_Add_device(char *devname, int fd, char *newdev)
*/
info.disk.number = d;
- info.disk.major = major(stb.st_rdev);
- info.disk.minor = minor(stb.st_rdev);
+ info.disk.major = major(rdev);
+ info.disk.minor = minor(rdev);
info.disk.raid_disk = d;
info.disk.state = (1 << MD_DISK_SYNC) | (1 << MD_DISK_ACTIVE);
st->ss->update_super(st, &info, "linear-grow-new", newdev,
diff --git a/Incremental.c b/Incremental.c
index 8909f2f..11a34e7 100644
--- a/Incremental.c
+++ b/Incremental.c
@@ -87,6 +87,7 @@ int Incremental(struct mddev_dev *devlist, struct context *c,
* start the array (auto-readonly).
*/
struct stat stb;
+ dev_t rdev;
struct mdinfo info, dinfo;
struct mdinfo *sra = NULL, *d;
struct mddev_ident *match;
@@ -174,21 +175,11 @@ int Incremental(struct mddev_dev *devlist, struct context *c,
/* 2/ Find metadata, reject if none appropriate (check
* version/name from args) */
- if (fstat(dfd, &stb) < 0) {
- if (c->verbose >= 0)
- pr_err("fstat failed for %s: %s.\n",
- devname, strerror(errno));
- goto out;
- }
- if ((stb.st_mode & S_IFMT) != S_IFBLK) {
- if (c->verbose >= 0)
- pr_err("%s is not a block device.\n",
- devname);
+ if (!fstat_is_blkdev(dfd, devname, &rdev))
goto out;
- }
- dinfo.disk.major = major(stb.st_rdev);
- dinfo.disk.minor = minor(stb.st_rdev);
+ dinfo.disk.major = major(rdev);
+ dinfo.disk.minor = minor(rdev);
policy = disk_policy(&dinfo);
have_target = policy_check_path(&dinfo, &target_array);
@@ -339,8 +330,8 @@ int Incremental(struct mddev_dev *devlist, struct context *c,
}
dinfo = info;
- dinfo.disk.major = major(stb.st_rdev);
- dinfo.disk.minor = minor(stb.st_rdev);
+ dinfo.disk.major = major(rdev);
+ dinfo.disk.minor = minor(rdev);
if (add_disk(mdfd, st, &info, &dinfo) != 0) {
pr_err("failed to add %s to new array %s: %s.\n",
devname, chosen_name, strerror(errno));
@@ -441,8 +432,8 @@ int Incremental(struct mddev_dev *devlist, struct context *c,
goto out_unlock;
}
}
- info.disk.major = major(stb.st_rdev);
- info.disk.minor = minor(stb.st_rdev);
+ info.disk.major = major(rdev);
+ info.disk.minor = minor(rdev);
/* add disk needs to know about containers */
if (st->ss->external)
sra->array.level = LEVEL_CONTAINER;
@@ -863,12 +854,12 @@ static int array_try_spare(char *devname, int *dfdp, struct dev_policy *pol,
* Return 0 on success, or some exit code on failure, probably 1.
*/
int rv = 1;
- struct stat stb;
+ dev_t rdev;
struct map_ent *mp, *map = NULL;
struct mdinfo *chosen = NULL;
int dfd = *dfdp;
- if (fstat(dfd, &stb) != 0)
+ if (!fstat_is_blkdev(dfd, devname, &rdev))
return 1;
/*
@@ -1038,8 +1029,8 @@ static int array_try_spare(char *devname, int *dfdp, struct dev_policy *pol,
devlist.writemostly = FlagDefault;
devlist.failfast = FlagDefault;
devlist.devname = chosen_devname;
- sprintf(chosen_devname, "%d:%d", major(stb.st_rdev),
- minor(stb.st_rdev));
+ sprintf(chosen_devname, "%d:%d", major(rdev),
+ minor(rdev));
devlist.disposition = 'a';
close(dfd);
*dfdp = -1;
diff --git a/Manage.c b/Manage.c
index 230309b..af55266 100644
--- a/Manage.c
+++ b/Manage.c
@@ -1513,7 +1513,7 @@ int Manage_subdevs(char *devname, int fd,
struct stat stb;
tfd = dev_open(dv->devname, O_RDONLY);
if (tfd >= 0) {
- fstat(tfd, &stb);
+ fstat_is_blkdev(tfd, dv->devname, &rdev);
close(tfd);
} else {
int open_err = errno;
diff --git a/bitmap.c b/bitmap.c
index 16a6b73..3653660 100644
--- a/bitmap.c
+++ b/bitmap.c
@@ -183,7 +183,6 @@ static int
bitmap_file_open(char *filename, struct supertype **stp, int node_num)
{
int fd;
- struct stat stb;
struct supertype *st = *stp;
fd = open(filename, O_RDONLY|O_DIRECT);
@@ -193,14 +192,7 @@ bitmap_file_open(char *filename, struct supertype **stp, int node_num)
return -1;
}
- if (fstat(fd, &stb) < 0) {
- pr_err("failed to determine bitmap file/device type: %s\n",
- strerror(errno));
- close(fd);
- return -1;
- }
-
- if ((stb.st_mode & S_IFMT) == S_IFBLK) {
+ if (fstat_is_blkdev(fd, filename, NULL)) {
/* block device, so we are probably after an internal bitmap */
if (!st)
st = guess_super(fd);
@@ -221,6 +213,9 @@ bitmap_file_open(char *filename, struct supertype **stp, int node_num)
}
*stp = st;
+ } else {
+ close(fd);
+ return -1;
}
return fd;
diff --git a/mdadm.h b/mdadm.h
index 07ee963..4adb840 100644
--- a/mdadm.h
+++ b/mdadm.h
@@ -1434,6 +1434,7 @@ extern int check_raid(int fd, char *name);
extern int check_partitions(int fd, char *dname,
unsigned long long freesize,
unsigned long long size);
+extern int fstat_is_blkdev(int fd, char *devname, dev_t *rdev);
extern int get_mdp_major(void);
extern int get_maj_min(char *dev, int *major, int *minor);
diff --git a/super-intel.c b/super-intel.c
index 36f77d3..c4196ea 100644
--- a/super-intel.c
+++ b/super-intel.c
@@ -6562,7 +6562,7 @@ count_volumes_list(struct md_list *devlist, char *homehost,
for (tmpdev = devlist; tmpdev; tmpdev = tmpdev->next) {
char *devname = tmpdev->devname;
- struct stat stb;
+ dev_t rdev;
struct supertype *tst;
int dfd;
if (tmpdev->used > 1)
@@ -6578,14 +6578,7 @@ count_volumes_list(struct md_list *devlist, char *homehost,
dprintf("cannot open device %s: %s\n",
devname, strerror(errno));
tmpdev->used = 2;
- } else if (fstat(dfd, &stb)< 0) {
- /* Impossible! */
- dprintf("fstat failed for %s: %s\n",
- devname, strerror(errno));
- tmpdev->used = 2;
- } else if ((stb.st_mode & S_IFMT) != S_IFBLK) {
- dprintf("%s is not a block device.\n",
- devname);
+ } else if (!fstat_is_blkdev(dfd, devname, &rdev)) {
tmpdev->used = 2;
} else if (must_be_container(dfd)) {
struct supertype *cst;
@@ -6607,7 +6600,7 @@ count_volumes_list(struct md_list *devlist, char *homehost,
if (cst)
cst->ss->free_super(cst);
} else {
- tmpdev->st_rdev = stb.st_rdev;
+ tmpdev->st_rdev = rdev;
if (tst->ss->load_super(tst,dfd, NULL)) {
dprintf("no RAID superblock on %s\n",
devname);
diff --git a/util.c b/util.c
index c7585ac..a92faf8 100644
--- a/util.c
+++ b/util.c
@@ -730,6 +730,23 @@ int check_raid(int fd, char *name)
return 1;
}
+int fstat_is_blkdev(int fd, char *devname, dev_t *rdev)
+{
+ struct stat stb;
+
+ if (fstat(fd, &stb) != 0) {
+ pr_err("fstat failed for %s: %s\n", devname, strerror(errno));
+ return 0;
+ }
+ if ((S_IFMT & stb.st_mode) != S_IFBLK) {
+ pr_err("%s is not a block device.\n", devname);
+ return 0;
+ }
+ if (rdev)
+ *rdev = stb.st_rdev;
+ return 1;
+}
+
int ask(char *mesg)
{
char *add = "";
--
2.10.2
^ permalink raw reply related
* [PATCH v1 2/2] mdadm/util: unify stat checking blkdev into function
From: Zhilong Liu @ 2017-05-04 12:16 UTC (permalink / raw)
To: Jes.Sorensen; +Cc: linux-raid, Zhilong Liu
In-Reply-To: <20170504121622.4014-1-zlliu@suse.com>
declare function stat_is_blkdev() to integrate repeated stat
checking blkdev operations, it returns 'true/1' when it is a
block device, and returns 'false/0' when it isn't.
The devname is necessary parameter, *rdev is optional, parse
the pointer of dev_t *rdev, if valid, assigned device number
to dev_t *rdev, if NULL, ignores.
Signed-off-by: Zhilong Liu <zlliu@suse.com>
---
Assemble.c | 7 ++-----
Build.c | 25 ++++---------------------
Incremental.c | 21 ++++-----------------
Manage.c | 11 +----------
Monitor.c | 16 ++++------------
mdadm.h | 1 +
super-ddf.c | 10 ++++------
super-intel.c | 10 ++++------
util.c | 17 +++++++++++++++++
9 files changed, 41 insertions(+), 77 deletions(-)
diff --git a/Assemble.c b/Assemble.c
index 9d0a89f..30d5838 100644
--- a/Assemble.c
+++ b/Assemble.c
@@ -512,15 +512,12 @@ static int select_devices(struct mddev_dev *devlist,
/* Now reject spares that don't match domains of identified members */
for (tmpdev = devlist; tmpdev; tmpdev = tmpdev->next) {
- struct stat stb;
if (tmpdev->used != 3)
continue;
- if (stat(tmpdev->devname, &stb)< 0) {
- pr_err("fstat failed for %s: %s\n",
- tmpdev->devname, strerror(errno));
+ if (!stat_is_blkdev(tmpdev->devname, &rdev)) {
tmpdev->used = 2;
} else {
- struct dev_policy *pol = devid_policy(stb.st_rdev);
+ struct dev_policy *pol = devid_policy(rdev);
int dt = domain_test(domains, pol, NULL);
if (inargv && dt != 0)
/* take this spare as domains match
diff --git a/Build.c b/Build.c
index 2d84b96..ad59867 100644
--- a/Build.c
+++ b/Build.c
@@ -41,7 +41,6 @@ int Build(char *mddev, struct mddev_dev *devlist,
* cc = chunk size factor: 0==4k, 1==8k etc.
*/
int i;
- struct stat stb;
dev_t rdev;
int subdevs = 0, missing_disks = 0;
struct mddev_dev *dv;
@@ -65,16 +64,8 @@ int Build(char *mddev, struct mddev_dev *devlist,
missing_disks++;
continue;
}
- if (stat(dv->devname, &stb)) {
- pr_err("Cannot find %s: %s\n",
- dv->devname, strerror(errno));
- return 1;
- }
- if ((stb.st_mode & S_IFMT) != S_IFBLK) {
- pr_err("%s is not a block device.\n",
- dv->devname);
+ if (!stat_is_blkdev(dv->devname, NULL))
return 1;
- }
}
if (s->raiddisks != subdevs) {
@@ -162,16 +153,8 @@ int Build(char *mddev, struct mddev_dev *devlist,
if (strcmp("missing", dv->devname) == 0)
continue;
- if (stat(dv->devname, &stb)) {
- pr_err("Weird: %s has disappeared.\n",
- dv->devname);
+ if (!stat_is_blkdev(dv->devname, &rdev))
goto abort;
- }
- if ((stb.st_mode & S_IFMT)!= S_IFBLK) {
- pr_err("Weird: %s is no longer a block device.\n",
- dv->devname);
- goto abort;
- }
fd = open(dv->devname, O_RDONLY|O_EXCL);
if (fd < 0) {
pr_err("Cannot open %s: %s\n",
@@ -187,8 +170,8 @@ int Build(char *mddev, struct mddev_dev *devlist,
disk.state = (1<<MD_DISK_SYNC) | (1<<MD_DISK_ACTIVE);
if (dv->writemostly == FlagSet)
disk.state |= 1<<MD_DISK_WRITEMOSTLY;
- disk.major = major(stb.st_rdev);
- disk.minor = minor(stb.st_rdev);
+ disk.major = major(rdev);
+ disk.minor = minor(rdev);
if (ioctl(mdfd, ADD_NEW_DISK, &disk)) {
pr_err("ADD_NEW_DISK failed for %s: %s\n",
dv->devname, strerror(errno));
diff --git a/Incremental.c b/Incremental.c
index 11a34e7..97b2e99 100644
--- a/Incremental.c
+++ b/Incremental.c
@@ -86,8 +86,7 @@ int Incremental(struct mddev_dev *devlist, struct context *c,
* - if number of OK devices match expected, or -R and there are enough,
* start the array (auto-readonly).
*/
- struct stat stb;
- dev_t rdev;
+ dev_t rdev, rdev2;
struct mdinfo info, dinfo;
struct mdinfo *sra = NULL, *d;
struct mddev_ident *match;
@@ -108,18 +107,8 @@ int Incremental(struct mddev_dev *devlist, struct context *c,
struct createinfo *ci = conf_get_create_info();
- if (stat(devname, &stb) < 0) {
- if (c->verbose >= 0)
- pr_err("stat failed for %s: %s.\n",
- devname, strerror(errno));
- return rv;
- }
- if ((stb.st_mode & S_IFMT) != S_IFBLK) {
- if (c->verbose >= 0)
- pr_err("%s is not a block device.\n",
- devname);
+ if (!stat_is_blkdev(devname, &rdev))
return rv;
- }
dfd = dev_open(devname, O_RDONLY);
if (dfd < 0) {
if (c->verbose >= 0)
@@ -158,10 +147,8 @@ int Incremental(struct mddev_dev *devlist, struct context *c,
if (!devlist) {
devlist = conf_get_devs();
for (;devlist; devlist = devlist->next) {
- struct stat st2;
- if (stat(devlist->devname, &st2) == 0 &&
- (st2.st_mode & S_IFMT) == S_IFBLK &&
- st2.st_rdev == stb.st_rdev)
+ if (stat_is_blkdev(devlist->devname, &rdev2) &&
+ rdev2 == rdev)
break;
}
}
diff --git a/Manage.c b/Manage.c
index af55266..2c82326 100644
--- a/Manage.c
+++ b/Manage.c
@@ -1510,24 +1510,16 @@ int Manage_subdevs(char *devname, int fd,
*/
rdev = makedev(mj, mn);
} else {
- struct stat stb;
tfd = dev_open(dv->devname, O_RDONLY);
if (tfd >= 0) {
fstat_is_blkdev(tfd, dv->devname, &rdev);
close(tfd);
} else {
int open_err = errno;
- if (stat(dv->devname, &stb) != 0) {
- pr_err("Cannot find %s: %s\n",
- dv->devname, strerror(errno));
- goto abort;
- }
- if ((stb.st_mode & S_IFMT) != S_IFBLK) {
+ if (!stat_is_blkdev(dv->devname, &rdev))
if (dv->disposition == 'M')
/* non-fatal. Also improbable */
continue;
- pr_err("%s is not a block device.\n",
- dv->devname);
goto abort;
}
if (dv->disposition == 'r')
@@ -1544,7 +1536,6 @@ int Manage_subdevs(char *devname, int fd,
goto abort;
}
}
- rdev = stb.st_rdev;
}
switch(dv->disposition){
default:
diff --git a/Monitor.c b/Monitor.c
index 1f15377..e2b36ff 100644
--- a/Monitor.c
+++ b/Monitor.c
@@ -993,23 +993,13 @@ static void link_containers_with_subarrays(struct state *list)
/* Not really Monitor but ... */
int Wait(char *dev)
{
- struct stat stb;
char devnm[32];
- char *tmp;
int rv = 1;
int frozen_remaining = 3;
- if (stat(dev, &stb) != 0) {
- pr_err("Cannot find %s: %s\n", dev,
- strerror(errno));
+ if (!stat_is_blkdev(dev, NULL))
return 2;
- }
- tmp = stat2devnm(&stb);
- if (!tmp) {
- pr_err("%s is not a block device.\n", dev);
- return 2;
- }
- strcpy(devnm, tmp);
+ strcpy(devnm, dev);
while(1) {
struct mdstat_ent *ms = mdstat_read(1, 0);
@@ -1068,6 +1058,8 @@ int WaitClean(char *dev, int sock, int verbose)
int rv = 1;
char devnm[32];
+ if (!stat_is_blkdev(dev, NULL))
+ return 2;
fd = open(dev, O_RDONLY);
if (fd < 0) {
if (verbose)
diff --git a/mdadm.h b/mdadm.h
index 4adb840..a92feb2 100644
--- a/mdadm.h
+++ b/mdadm.h
@@ -1435,6 +1435,7 @@ extern int check_partitions(int fd, char *dname,
unsigned long long freesize,
unsigned long long size);
extern int fstat_is_blkdev(int fd, char *devname, dev_t *rdev);
+extern int stat_is_blkdev(char *devname, dev_t *rdev);
extern int get_mdp_major(void);
extern int get_maj_min(char *dev, int *major, int *minor);
diff --git a/super-ddf.c b/super-ddf.c
index 796eaa5..9c82f4f 100644
--- a/super-ddf.c
+++ b/super-ddf.c
@@ -3490,7 +3490,7 @@ static int validate_geometry_ddf_bvd(struct supertype *st,
char *dev, unsigned long long *freesize,
int verbose)
{
- struct stat stb;
+ dev_t rdev;
struct ddf_super *ddf = st->sb;
struct dl *dl;
unsigned long long maxsize;
@@ -3526,13 +3526,11 @@ static int validate_geometry_ddf_bvd(struct supertype *st,
return 1;
}
/* This device must be a member of the set */
- if (stat(dev, &stb) < 0)
- return 0;
- if ((S_IFMT & stb.st_mode) != S_IFBLK)
+ if (!stat_is_blkdev(dev, NULL))
return 0;
for (dl = ddf->dlist ; dl ; dl = dl->next) {
- if (dl->major == (int)major(stb.st_rdev) &&
- dl->minor == (int)minor(stb.st_rdev))
+ if (dl->major == (int)major(rdev) &&
+ dl->minor == (int)minor(rdev))
break;
}
if (!dl) {
diff --git a/super-intel.c b/super-intel.c
index c4196ea..e13c940 100644
--- a/super-intel.c
+++ b/super-intel.c
@@ -6855,7 +6855,7 @@ static int validate_geometry_imsm_volume(struct supertype *st, int level,
unsigned long long *freesize,
int verbose)
{
- struct stat stb;
+ dev_t rdev;
struct intel_super *super = st->sb;
struct imsm_super *mpb;
struct dl *dl;
@@ -6920,13 +6920,11 @@ static int validate_geometry_imsm_volume(struct supertype *st, int level,
}
/* This device must be a member of the set */
- if (stat(dev, &stb) < 0)
- return 0;
- if ((S_IFMT & stb.st_mode) != S_IFBLK)
+ if (!stat_is_blkdev(dev, &rdev))
return 0;
for (dl = super->disks ; dl ; dl = dl->next) {
- if (dl->major == (int)major(stb.st_rdev) &&
- dl->minor == (int)minor(stb.st_rdev))
+ if (dl->major == (int)major(rdev) &&
+ dl->minor == (int)minor(rdev))
break;
}
if (!dl) {
diff --git a/util.c b/util.c
index a92faf8..11ff2cc 100644
--- a/util.c
+++ b/util.c
@@ -747,6 +747,23 @@ int fstat_is_blkdev(int fd, char *devname, dev_t *rdev)
return 1;
}
+int stat_is_blkdev(char *devname, dev_t *rdev)
+{
+ struct stat stb;
+
+ if (stat(devname, &stb) != 0) {
+ pr_err("stat failed for %s: %s\n", devname, strerror(errno));
+ return 0;
+ }
+ if ((S_IFMT & stb.st_mode) != S_IFBLK) {
+ pr_err("%s is not a block device.\n", devname);
+ return 0;
+ }
+ if (rdev)
+ *rdev = stb.st_rdev;
+ return 1;
+}
+
int ask(char *mesg)
{
char *add = "";
--
2.10.2
^ permalink raw reply related
* Re: [mdadm PATCH] Create: move STOP_ARRAY to abort_locked
From: Zhilong Liu @ 2017-05-04 12:20 UTC (permalink / raw)
To: Jes.Sorensen; +Cc: linux-raid
In-Reply-To: <1493190229-14329-1-git-send-email-zlliu@suse.com>
Hi Jes,
apply for review, this is a bug I ever encountered.
On 04/26/2017 03:03 PM, Zhilong Liu wrote:
> The sysfs entry and devnm would be created once create_mddev()
> performed successfully, but the creating isn't completed here,
> move STOP_ARRAY to abort_locked, the purpose is to cleanup the
> partially created array.
>
> Signed-off-by: Zhilong Liu <zlliu@suse.com>
> ---
> Create.c | 11 ++++-------
> 1 file changed, 4 insertions(+), 7 deletions(-)
>
> diff --git a/Create.c b/Create.c
> index 6ca0924..fe0ab7e 100644
> --- a/Create.c
> +++ b/Create.c
> @@ -904,10 +904,8 @@ int Create(struct supertype *st, char *mddev,
> remove_partitions(fd);
> if (st->ss->add_to_super(st, &inf->disk,
> fd, dv->devname,
> - dv->data_offset)) {
> - ioctl(mdfd, STOP_ARRAY, NULL);
> + dv->data_offset))
> goto abort_locked;
> - }
> st->ss->getinfo_super(st, inf, NULL);
> safe_mode_delay = inf->safe_mode_delay;
>
> @@ -1008,7 +1006,6 @@ int Create(struct supertype *st, char *mddev,
> sysfs_set_safemode(&info, safe_mode_delay);
> if (err) {
> pr_err("failed to activate array.\n");
> - ioctl(mdfd, STOP_ARRAY, NULL);
> goto abort;
> }
> } else if (c->readonly &&
> @@ -1018,7 +1015,6 @@ int Create(struct supertype *st, char *mddev,
> "array_state", "readonly") < 0) {
> pr_err("Failed to start array: %s\n",
> strerror(errno));
> - ioctl(mdfd, STOP_ARRAY, NULL);
> goto abort;
> }
> } else {
> @@ -1030,7 +1026,6 @@ int Create(struct supertype *st, char *mddev,
> if (info.array.chunk_size & (info.array.chunk_size-1)) {
> cont_err("Problem may be that chunk size is not a power of 2\n");
> }
> - ioctl(mdfd, STOP_ARRAY, NULL);
> goto abort;
> }
> /* if start_ro module parameter is set, array is
> @@ -1061,7 +1056,9 @@ int Create(struct supertype *st, char *mddev,
> map_remove(&map, fd2devnm(mdfd));
> map_unlock(&map);
>
> - if (mdfd >= 0)
> + if (mdfd >= 0) {
> + ioctl(mdfd, STOP_ARRAY, NULL);
> close(mdfd);
> + }
> return 1;
> }
^ permalink raw reply
* Re: [dm-devel] [PATCH v6 0/3] dm: boot a mapped device without an initramfs
From: Alasdair G Kergon @ 2017-05-04 13:20 UTC (permalink / raw)
To: Enric Balletbo Serra
Cc: Kees Cook, Will Drewry, Guenter Roeck, Mike Snitzer,
linux-doc@vger.kernel.org, David Zeuthen, LKML, linux-raid,
dm-devel, Enric Balletbo i Serra, Shaohua Li, Alasdair Kergon
In-Reply-To: <20170504121452.GF4943@agk-dp.fab.redhat.com>
Some more thoughts with your example, dmsetup might look like:
# dmsetup create --bootformat "lroot:uuid,rw,0 2097152 linear 8:2 0, \
2097152 2097152 linear 8:3 0, 4194304 2097152 linear 8:4 0"
- also supporting creating multiple devices if the semi-colon is used
- colon to separate name from uuid, like we already do major:minor
- colon to separate other flags from rw if we need them in future
- splitting first on a unescaped semi-colons, then on the first two
unescaped commas, and then on unescaped commas and then unescaped spaces
within the table
- backslash escapes the following character so it is never a treated
as a separator
- lroot\:uuid\;\\\ \"\, would be a device with no uuid and the name
lroot:uuid;\ ", (on a non-udev system without name mangling)
# dmsetup ls --bootformat lroot
dm="lroot:uuid,rw,0 2097152 linear 8:2 0, 2097152 2097152 \
linear 8:3 0, 4194304 2097152 linear 8:4 0"
# dmsetup ls --bootformat
(all devices on one output line)
While the code also supports devices in the /dev/sda2 format for
convenience, please use the preferred 8:2 format in any implementation
and documented examples (to avoid the unnecessary dependency on /dev and
its dependencies).
Or with some alternative name for the option
--boot[format|param]
--short[format]
--kernelparam
--condensed
other suggestions?
dmsetup create --condensed
dmsetup ls --condensed
Alasdair
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox