linux-btrfs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: David Sterba <dsterba@suse.cz>
To: linux-btrfs@vger.kernel.org
Cc: chris.mason@fusionio.com, David Sterba <dsterba@suse.cz>,
	Alex Lyakas <alex.btrfs@zadarastorage.com>,
	Josef Bacik <jbacik@fusionio.com>
Subject: [PATCH] btrfs: commit transaction after deleting a subvolume
Date: Sat, 31 Aug 2013 00:25:46 +0200	[thread overview]
Message-ID: <1377901546-22434-1-git-send-email-dsterba@suse.cz> (raw)

Alex pointed out the consequences after a transaction is not committed
when a subvolume is deleted, so in case of a crash before an actual
commit happens will let the subvolume reappear.

Original post:
http://www.spinics.net/lists/linux-btrfs/msg22088.html

Josef's objections:
http://www.spinics.net/lists/linux-btrfs/msg22256.html

While there's no need to do a full commit for regular files, a subvolume
may get a different treatment.

http://www.spinics.net/lists/linux-btrfs/msg23087.html:

"That a subvol/snapshot may appear after crash if transation commit did
not happen does not feel so good. We know that the subvol is only
scheduled for deletion and needs to be processed by cleaner.

>From that point I'd rather see the commit to happen to avoid any
unexpected surprises.  A subvolume that re-appears still holds the data
references and consumes space although the user does not assume that.

Automated snapshotting and deleting needs some guarantees about the
behaviour and what to do after a crash. So now it has to process the
backlog of previously deleted snapshots and verify that they're not
there, compared to "deleted -> will never appear, can forget about it".
"

There is a performance penalty incured by the change, but deleting a
subvolume is not a frequent operation and the tradeoff seems justified
by getting the guarantee stated above.

CC: Alex Lyakas <alex.btrfs@zadarastorage.com>
CC: Josef Bacik <jbacik@fusionio.com>
Signed-off-by: David Sterba <dsterba@suse.cz>
---
 fs/btrfs/ioctl.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
index e407f75..4394632 100644
--- a/fs/btrfs/ioctl.c
+++ b/fs/btrfs/ioctl.c
@@ -2268,7 +2268,7 @@ static noinline int btrfs_ioctl_snap_destroy(struct file *file,
 out_end_trans:
 	trans->block_rsv = NULL;
 	trans->bytes_reserved = 0;
-	ret = btrfs_end_transaction(trans, root);
+	ret = btrfs_commit_transaction(trans, root);
 	if (ret && !err)
 		err = ret;
 	inode->i_flags |= S_DEAD;
-- 
1.7.9


             reply	other threads:[~2013-08-30 22:25 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-08-30 22:25 David Sterba [this message]
2013-10-20 10:46 ` [PATCH] btrfs: commit transaction after deleting a subvolume Alex Lyakas
2013-10-20 12:19 ` Chris Mason
2013-10-20 17:21   ` David Sterba

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=1377901546-22434-1-git-send-email-dsterba@suse.cz \
    --to=dsterba@suse.cz \
    --cc=alex.btrfs@zadarastorage.com \
    --cc=chris.mason@fusionio.com \
    --cc=jbacik@fusionio.com \
    --cc=linux-btrfs@vger.kernel.org \
    /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).