On wed, 15 Jan 2014 17:40:38 +0100, David Sterba wrote: > 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 don't think so. The users should be responsible for their behavior if they destroy the subvolume. > > 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 It is similar to our approach. But I think our idea is better because - we needn't add a new flag - The subvolumes are special directory, the most operations of them should be similar to the common directory. Since we can remove a directory while someone is accessing it, it is better that we can destroy a subvolume while we are using it as a send parent. Thanks Miao > > 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 > -- 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
