Re: LVM snapshot broke between 4.14 and 4.16

[Linus Torvalds]

On Fri, Aug 3, 2018 at 11:39 AM Mike Snitzer <snitzer@xxxxxxxxxx> wrote:
>
> Please stop with the overreaction and making this something it isn't.

It's not an overreaction when people get their scripts broken, and
some developers then argue "that's not a serious bug".

Guys, this needs to be fixed.  With all the stupid and fundamentyally
incorrect excuses, I am now no longer even willing to maintain any
other course of action.

If you develop for the Linux kernel, you need to realize that
"breaking user space" is simply not acceptable. And if you cannot live
with that, then you should stop working on the kernel. Because I will
refuse to continue to pull from you as a developer.

At worst, I'll just revert the original commit entirely. I was hoping
we'd be able to avoid that, partly because the commit looks fine, but
partly because it also doesn't revert cleanly.

Or I'll just do something like this, since it seems like it's the lvm
people who have the hardest time with understanding the simple rules:

diff --git a/drivers/md/dm-ioctl.c b/drivers/md/dm-ioctl.c
index b810ea77e6b1..fcfab812e025 100644
--- a/drivers/md/dm-ioctl.c
+++ b/drivers/md/dm-ioctl.c
@@ -1049,7 +1049,12 @@ static int do_resume(struct dm_ioctl *param)
                        return PTR_ERR(old_map);
                }

-               if (dm_table_get_mode(new_map) & FMODE_WRITE)
+               /*
+                * This used to do
+                *    dm_table_get_mode(new_map) & FMODE_WRITE
+                * but the lvm tools got this wrong, and will
+                * continue to write to "read-only" volumes.
+               if (0)
                        set_disk_ro(dm_disk(md), 0);
                else
                        set_disk_ro(dm_disk(md), 1);

which seems to target the actual problem more directly.

              Linus