From: Ilya Dryomov <idryomov@gmail.com>
To: Linus Torvalds <torvalds@linux-foundation.org>
Cc: wgh@torlan.ru, Jens Axboe <axboe@kernel.dk>,
linux-block <linux-block@vger.kernel.org>,
linux-kernel@vger.kernel.org, Sagi Grimberg <sagi@grimberg.me>,
Mike Snitzer <snitzer@redhat.com>,
dm-devel@redhat.com
Subject: Re: LVM snapshot broke between 4.14 and 4.16
Date: Thu, 2 Aug 2018 20:18:44 +0200 [thread overview]
Message-ID: <CAOi1vP-SJO4AMK=XD3Lmja++2dQqoiw6Zn8Vgxf8OfvYgnuC6Q@mail.gmail.com> (raw)
In-Reply-To: <CA+55aFyAox9BTDpRsqLXiROhHOX60aa45LEZ7Azteaumdu11Fw@mail.gmail.com>
On Thu, Aug 2, 2018 at 6:41 PM Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> On Thu, Aug 2, 2018 at 8:16 AM WGH <wgh@torlan.ru> wrote:
> >
> > On 08/02/2018 04:31 PM, Ilya Dryomov wrote:
> > >
> > > From a quick look, --permission r sets DM_READONLY_FLAG, which makes dm
> > > mark the disk read-only with set_disk_ro(dm_disk(md), 1) in do_resume().
> > > A bit later it tries to write to the disk from write_header():
> > >
> > > return chunk_io(ps, ps->header_area, 0, REQ_OP_WRITE, 0, 1);
> > >
> > > Thanks,
> > >
> > > Ilya
> >
> > After further investigation, this was fixed on lvm2 side (userspace) in
> > https://sourceware.org/git/?p=lvm2.git;a=commit;h=a6fdb9d9d70f51c49ad11a87ab4243344e6701a3
> > (snapshot: keep COW writable for read-only volumes).
> >
> > So I guess that's it. Time to poke my distribution package maintainers
> > to bump the package version.
>
> That is *not* how kernel development is supposed to work. If your
> script used to work, it should continue to work.
>
> Why did it use to work despite that read-only flag? And what was it
> that actually broke this?
I think it was my commit 721c7fc701c7 ("block: fail op_is_write()
requests to read-only partitions"). The block layer was previously
allowing non-blkdev_write_iter() writes to read-only disks and
partitions. dm's chunk_io() boils down to submit_bio(), so in 4.16
it started to fail if the underlying device was marked read-only.
>
> We remain bug-for-bug compatible with older kernel versions when
> people depend on the bugs. Unless the old bugs are security issues,
> and even then we try to make it _look_ like we work the same way.
>
> Or was it a user-space lvm tool that broke in the first place, and the
> kernel release update was a red herring?
Apparently at least some versions of lvm(8) instruct dm to mark the COW
device read-only even though it is always written to. It comes up only
if the user asks for a read-only snapshot (the default is writable).
One option might be to ignore the supplied DM_READONLY_FLAG for COW
devices. Marking the COW device (i.e. the exception store) read-only
is probably never sane...
Thanks,
Ilya
next prev parent reply other threads:[~2018-08-02 18:18 UTC|newest]
Thread overview: 38+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-08-02 12:26 LVM snapshot broke between 4.14 and 4.16 WGH
2018-08-02 13:31 ` Ilya Dryomov
2018-08-02 15:10 ` WGH
2018-08-02 16:41 ` Linus Torvalds
2018-08-02 18:18 ` Ilya Dryomov [this message]
2018-08-02 18:32 ` Linus Torvalds
2018-08-02 21:32 ` WGH
2018-08-02 21:39 ` WGH
2018-08-02 21:52 ` Linus Torvalds
2018-08-03 13:31 ` Mike Snitzer
2018-08-03 15:20 ` [dm-devel] " Theodore Y. Ts'o
2018-08-03 18:39 ` Mike Snitzer
2018-08-03 18:57 ` Linus Torvalds
2018-08-03 19:06 ` Mike Snitzer
2018-08-03 19:11 ` Linus Torvalds
2018-08-03 19:33 ` Mike Snitzer
2018-08-03 19:22 ` Linus Torvalds
2018-08-04 10:01 ` WGH
2018-08-04 17:04 ` Linus Torvalds
2018-08-04 18:19 ` Mike Snitzer
2018-08-04 20:29 ` WGH
[not found] ` <20180803195636.GA31444@agk-dp.fab.redhat.com>
[not found] ` <20180803200817.GB31444@agk-dp.fab.redhat.com>
2018-08-03 20:42 ` [dm-devel] " Linus Torvalds
2018-08-03 21:26 ` Alasdair G Kergon
2018-08-03 13:31 ` Zdenek Kabelac
2018-08-03 16:37 ` Linus Torvalds
2018-08-03 18:54 ` Mike Snitzer
2018-08-03 19:09 ` Linus Torvalds
2018-08-03 19:30 ` Mike Snitzer
2018-08-03 19:36 ` Linus Torvalds
2018-08-04 5:20 ` [dm-devel] " Theodore Y. Ts'o
2018-08-04 8:36 ` Zdenek Kabelac
2018-08-04 16:22 ` Theodore Y. Ts'o
2018-08-04 18:18 ` Mike Snitzer
2018-08-04 19:37 ` Theodore Y. Ts'o
2018-08-04 21:48 ` Mike Snitzer
2018-08-04 15:19 ` Mike Snitzer
2018-08-03 19:18 ` [dm-devel] " Zdenek Kabelac
2018-08-03 19:30 ` Linus Torvalds
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CAOi1vP-SJO4AMK=XD3Lmja++2dQqoiw6Zn8Vgxf8OfvYgnuC6Q@mail.gmail.com' \
--to=idryomov@gmail.com \
--cc=axboe@kernel.dk \
--cc=dm-devel@redhat.com \
--cc=linux-block@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=sagi@grimberg.me \
--cc=snitzer@redhat.com \
--cc=torvalds@linux-foundation.org \
--cc=wgh@torlan.ru \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).