linux-btrfs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Goffredo Baroncelli <kreijack@tiscalinet.it>
To: dsterba@suse.cz
Cc: kreijack@inwind.it, Filipe Brandenburger <filbranden@google.com>,
	Ian Kumlien <pomac@demius.net>,
	linux-btrfs <linux-btrfs@vger.kernel.org>
Subject: Re: [PATCH 2/6] Btrfs-progs: add btrfsck functionality to btrfs
Date: Wed, 13 Feb 2013 00:07:30 +0100	[thread overview]
Message-ID: <511ACB32.3030106@tiscalinet.it> (raw)
In-Reply-To: <20130212225207.GD12339@twin.jikos.cz>

On 02/12/2013 11:52 PM, David Sterba wrote:
> On Tue, Feb 12, 2013 at 07:01:33PM +0100, Goffredo Baroncelli wrote:
>> On 02/12/2013 06:37 PM, Filipe Brandenburger wrote:
>>> Hi,
>>>
>>> On Tue, Feb 12, 2013 at 8:39 AM, David Sterba <dsterba@suse.cz> wrote:
>>>> +# For backward compatibility, 'btrfs' changes behaviour to fsck if it's named 'btrfsck'
>>>> +btrfsck: btrfs
>>>> +       @echo "    [CP]     $@"
>>>> +       $(Q)cp btrfs btrfsck
>>>> +
>>>
>>> I think the idea was that btrfsck becomes a link (either symbolic or
>>> hardlink works) to btrfs...
>>>
>>> Maybe just replace cp with ln?
>>
>> I agree with Filipe, or even a script is reasonable. So we have only one
>> binary to update, and we avoid the risk to have a version mismatch
>> between btrfsck and btrfs. This could lead to a different behaviour
>> when the user call btrfsck instead btrfs. Finally this could save some
>> bytes of space.
> 
> Ok, I'll replace it with a hardlink. A symlink is not reliable (cannot
> be copied without breaking the path).

...mmm... the install command (invoked by the Makefile during the
installation phase) doesn't seem to preserve both the hard-link and the
soft-link:

$ touch test
$ ln test test2
$ ln -sf test lntest
$ mkdir t3
$ install test2 t3/
$ install lntest t3/
$ ls -li lntest test test2 t3/test2 t3/lntest
3005857 lrwxrwxrwx 1 ghigo ghigo 4 Feb 13 00:03 lntest -> test
3005858 -rwxr-xr-x 1 ghigo ghigo 0 Feb 13 00:03 t3/lntest
3005854 -rwxr-xr-x 1 ghigo ghigo 0 Feb 13 00:00 t3/test2
3005852 -rw-r--r-- 2 ghigo ghigo 0 Feb 13 00:00 test
3005852 -rw-r--r-- 2 ghigo ghigo 0 Feb 13 00:00 test2

I think that a bash script is a better choice.

>> Anyway my opinion would be to left this kind to decision to the
>> distribution. We (as upstream) should only remove the old btrfsck and
>> issue an WARNING/REMARK in the release note to notify this change.
>> Unfortunately btrfsck is old; now we must provide an alternative file to
>> overwrite this binary in order to avoid the mismatch above when the user
>> is used to recompile the binary from the source.
> 
> A warning is a good idea and it will start a deprecation period of
> btrfsck as separate utility. Unil some point in future, I'd rather stay
> conservative and let it exist.
> 
> david
> 


-- 
gpg @keyserver.linux.it: Goffredo Baroncelli (kreijackATinwind.it>
Key fingerprint BBF5 1610 0B64 DAC6 5F7D  17B2 0EDA 9B37 8B82 E0B5

  reply	other threads:[~2013-02-12 23:07 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-02-08  0:36 Btrfs-progs: Merge btrfs-restore, btrfsck, btrfs-select-super, btrfs-dump-super and btrfs-debug-tree in to btrfs Ian Kumlien
2013-02-08  0:36 ` [PATCH 1/6] Btrfs-progs: Rename btrfsck.c -> cmds-check.c Ian Kumlien
2013-02-08  0:36 ` [PATCH 2/6] Btrfs-progs: add btrfsck functionality to btrfs Ian Kumlien
2013-02-08 17:39   ` Goffredo Baroncelli
2013-02-08 18:17     ` Ian Kumlien
2013-02-08 23:07       ` David Sterba
2013-02-08 23:37         ` Ian Kumlien
2013-02-12 16:39   ` David Sterba
2013-02-12 17:37     ` Filipe Brandenburger
2013-02-12 18:01       ` Goffredo Baroncelli
2013-02-12 22:52         ` David Sterba
2013-02-12 23:07           ` Goffredo Baroncelli [this message]
2013-06-02 15:47   ` Dieter Ries
2013-11-13 17:13     ` David Sterba
2013-11-14  9:25       ` Ilya Dryomov
2013-11-14 12:49         ` David Sterba
2013-11-14 12:56           ` Ian Kumlien
2013-02-08  0:36 ` [PATCH 3/6] Btrfs-progs: move btrfs-select-super.c Ian Kumlien
2013-02-08  0:37 ` [PATCH 4/6] Btrfs-progs: move debug-tree.c -> cmds-rescue-debug-tree.c Ian Kumlien
2013-02-08  0:37 ` [PATCH 5/6] Btrfs-progs: restore.c -> cmds-restore.c Ian Kumlien
2013-02-08  0:37 ` [PATCH 6/6] Btrfs-progs: add the rescue section to btrfs Ian Kumlien
2013-02-08 17:39   ` Goffredo Baroncelli
2013-02-08 18:07     ` Ian Kumlien
2013-02-08 19:57   ` Ilya Dryomov
2013-02-08 20:48     ` Hugo Mills
2013-02-08 22:30   ` David Sterba
2013-02-12 15:26   ` David Sterba
2013-02-08 17:40 ` Btrfs-progs: Merge btrfs-restore, btrfsck, btrfs-select-super, btrfs-dump-super and btrfs-debug-tree in " Goffredo Baroncelli
2013-02-08 18:05   ` Ian Kumlien

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=511ACB32.3030106@tiscalinet.it \
    --to=kreijack@tiscalinet.it \
    --cc=dsterba@suse.cz \
    --cc=filbranden@google.com \
    --cc=kreijack@inwind.it \
    --cc=linux-btrfs@vger.kernel.org \
    --cc=pomac@demius.net \
    /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).