public inbox for linux-btrfs@vger.kernel.org
 help / color / mirror / Atom feed
From: David Sterba <dsterba@suse.cz>
To: Miao Xie <miaox@cn.fujitsu.com>
Cc: dsterba@suse.cz, Wang Shilong <wangsl.fnst@cn.fujitsu.com>,
	linux-btrfs@vger.kernel.org
Subject: Re: [PATCH v2 2/4] Btrfs: fix protection between send and root deletion
Date: Wed, 15 Jan 2014 17:40:38 +0100	[thread overview]
Message-ID: <20140115164038.GD6498@twin.jikos.cz> (raw)
In-Reply-To: <52D49F80.6020604@cn.fujitsu.com>

On Tue, Jan 14, 2014 at 10:22:56AM +0800, Miao Xie wrote:
> On mon, 13 Jan 2014 19:27:45 +0100, David Sterba wrote:
> The root what is about to be sent out may be deleted after we get it, if
> there is no in-memory i-node in it, it will be inserted into the dead_roots list
> immediately, then the cleaner thread will drop it. But the sender doesn't know,
> the sender will access a broken tree.

Thanks, I see the race now, but I think that we should prevent the
subvolume deletion at a higher level, similar to the RO->RW protection.

Your fix makes sure that the deleted root will not get cleaned and stays
during the send. Only after it finishes it will be cleaned. Now, what if
send fails or is interrupted? There's no way to redo it. Yes the user
can be blamed for the mistake, or the tools will prevent him to do it.

I see the latter as more user-friendly. Doing a 'send and forget' where
I don't care if the data will be sent properly does not fit the primary
purpose of send/receive with backups.

My idea to fix that:
- add an internal root_item flag to denote a dead root
- set this flag in btrfs_add_dead_root()
- check the flag in send similar to the btrfs_root_readonly checks, for
  all involved roots
- in 'destroy subvolume, check if the send_in_progress is set and refuse
  to delete

The root lookup in send via btrfs_read_fs_root_no_name will check if the
root is dead or not. If it is, ENOENT, aborted send. If it's alive, it's
protected by send_in_progress, send can continue.

This has become more complex than the original patch, I'll try to think
about it again later if it still makes sense. Comments welcome of course.


david

  reply	other threads:[~2014-01-15 16:40 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-01-07  9:25 [PATCH v2 1/4] Btrfs: fix wrong send_in_progress accounting Wang Shilong
2014-01-07  9:25 ` [PATCH v2 2/4] Btrfs: fix protection between send and root deletion Wang Shilong
2014-01-13 18:27   ` David Sterba
2014-01-14  2:22     ` Miao Xie
2014-01-15 16:40       ` David Sterba [this message]
2014-01-16  2:32         ` Miao Xie
2014-01-21 18:16           ` David Sterba
2014-01-22  8:44             ` Wang Shilong
2014-01-22 12:43               ` David Sterba
2014-01-08 12:16 ` [PATCH v2 1/4] Btrfs: fix wrong send_in_progress accounting David Sterba
2014-01-08 15:09   ` Wang Shilong
2014-01-13 18:40     ` David Sterba
2014-01-14 11:52       ` Wang Shilong

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=20140115164038.GD6498@twin.jikos.cz \
    --to=dsterba@suse.cz \
    --cc=linux-btrfs@vger.kernel.org \
    --cc=miaox@cn.fujitsu.com \
    --cc=wangsl.fnst@cn.fujitsu.com \
    /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