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
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to