Re: [PATCH v2] btrfs-progs: subvol: change subvol set-default to also accept subvol path

[Misono, Tomohiro]

On 2017/10/02 18:01, Hugo Mills wrote:
> On Mon, Oct 02, 2017 at 11:39:05AM +0300, Andrei Borzenkov wrote:
>> On Mon, Oct 2, 2017 at 11:19 AM, Misono, Tomohiro
>>  wrote:
>>> This patch changes "subvol set-default" to also accept the subvolume path
>>> for convenience.
>>>
>>> This is the one of the issue on github:
>>> https://github.com/kdave/btrfs-progs/issues/35
>>>
>>> If there are two args, they are assumed as subvol id and path to the fs
>>> (the same as current behavior), and if there is only one arg, it is assumed
>>> as the path to the subvolume. Therefore there is no ambiguity between subvol
>>> id and subvol name, which is mentioned in the above issue page.
>>>
>>> Only the absolute path to the subvolume is allowed, for the safety when
>>> multiple filesystems are used.
>>>
>>> subvol id is resolved by get_subvol_info() which is used by "subvol show".
>>>
>>> change to v2:
>>> restrict the path to only allow absolute path.
>>
>> This is absolutely arbitrary restriction. Why we can do "btrfs
>> subvolume create ./relative/path" but cannot do "btrfs subvolume
>> set-default ./relative/path"?
> 
>Indeed. In fact, it's precisely the _opposite_ of the way that
> every other command works -- you provide the path to the subvolume in
> the *current namespace*.
> 
>This approach would be just a major misfeature at this point.
> 
>Hugo.
> 

Ok, I understood the point and want to revert this.
Please review the first version if possible:
https://mail-archive.com/linux-btrfs@vger.kernel.org/msg68486.html

Thanks,
Tomohiro

--
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

Re: Mount error on 32 bits, ok on 64 bits

[Jean-Denis Girard]

Le 02/10/2017 à 11:46, Goffredo Baroncelli a écrit :
> On 10/02/2017 07:59 PM, Jean-Denis Girard wrote:
>> Le 30/09/2017 à 17:29, Jean-Denis Girard a écrit :
>>> Le 28/09/2017 à 19:26, Jean-Denis Girard a écrit :
>>> The problem seems to come from commit c821e7f3 "pass bytes to
>>> btrfs_bio_alloc" (https://patchwork.kernel.org/patch/9763081/): the
>>> system is now running fine on 4.13.4 with only that patch reverted.
>>
>> Same situation with 4.14-rc3: my system cannot mount root file-system.
>> If I revert the patch, the system boots normally. This is 100%
>> reproducible. How can I help resolve that issue?
> 
> 
> Looking at the patch, it seems suspect this chunk:
> 
> @@ -2798,7 +2798,7 @@  static int submit_extent_page(int op, int op_flags, 
> struct extent_io_tree *tree,
>   }
>   }
>  
> - bio = btrfs_bio_alloc(bdev, sector);
> + bio = btrfs_bio_alloc(bdev, sector << 9);
>   bio_add_page(bio, page, page_size, offset);
>   bio->bi_end_io = end_io_func;
>   bio->bi_private = tree;
> 
> Now sector, is defined as
> 
>   sector_t   [1]
> 
> which in turn it might be defined as 
> 
>   unsigned long   [2]
> 
> which on 32bit is 32 bit if CONFIG_LBDAF is _not_ defined (CONFIG_LBDAF == 
> Support for large (2TB+) block devices and files)
> 
> The point is that 
> 
>   sector << 9 
> 
> may overflow if the disk is bigger than 4GB (and in your case it seems to be 
> 8 GB).
> 
> If I am correct, could you please so kindly to 
> 
> - repllay the patch
> - AND try to replace
>   bio = btrfs_bio_alloc(bdev, sector << 9);
> with
>   bio = btrfs_bio_alloc(bdev, (u64)sector << 9);

Hi Goffredo,

Thanks for your reply, analysis and correction: it does work, tested on
4.13-rc3, my system boots fine.


Best regards,
-- 
Jean-Denis Girard

SysNux   Systèmes   Linux   en   Polynésie  française
https://www.sysnux.pf/   Tél: +689 40.50.10.40 / GSM: +689 87.797.527

--
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

Re: Mount error on 32 bits, ok on 64 bits

[Goffredo Baroncelli]

On 10/02/2017 11:46 PM, Goffredo Baroncelli wrote:
> which on 32bit is 32 bit if CONFIG_LBDAF is _not_ defined (CONFIG_LBDAF == 
> Support for large (2TB+) block devices and files)

Only now I noticed that in the first email you attached the config; in fact in 
your config CONFIG_LBDAF is not set.

BR
G.Baroncelli
-- 
gpg @keyserver.linux.it: Goffredo Baroncelli 
Key fingerprint BBF5 1610 0B64 DAC6 5F7D  17B2 0EDA 9B37 8B82 E0B5
--
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

Re: Mount error on 32 bits, ok on 64 bits

[Goffredo Baroncelli]

On 10/02/2017 07:59 PM, Jean-Denis Girard wrote:
> Le 30/09/2017 à 17:29, Jean-Denis Girard a écrit :
>> Le 28/09/2017 à 19:26, Jean-Denis Girard a écrit :
>> The problem seems to come from commit c821e7f3 "pass bytes to
>> btrfs_bio_alloc" (https://patchwork.kernel.org/patch/9763081/): the
>> system is now running fine on 4.13.4 with only that patch reverted.
> 
> Same situation with 4.14-rc3: my system cannot mount root file-system.
> If I revert the patch, the system boots normally. This is 100%
> reproducible. How can I help resolve that issue?


Looking at the patch, it seems suspect this chunk:

@@ -2798,7 +2798,7 @@  static int submit_extent_page(int op, int op_flags, 
struct extent_io_tree *tree,
}
}
 
-   bio = btrfs_bio_alloc(bdev, sector);
+   bio = btrfs_bio_alloc(bdev, sector << 9);
bio_add_page(bio, page, page_size, offset);
bio->bi_end_io = end_io_func;
bio->bi_private = tree;

Now sector, is defined as

sector_t   [1]

which in turn it might be defined as 

unsigned long   [2]

which on 32bit is 32 bit if CONFIG_LBDAF is _not_ defined (CONFIG_LBDAF == 
Support for large (2TB+) block devices and files)

The point is that 

sector << 9 

may overflow if the disk is bigger than 4GB (and in your case it seems to be 8 
GB).

If I am correct, could you please so kindly to 

- repllay the patch
- AND try to replace
bio = btrfs_bio_alloc(bdev, sector << 9);
with
bio = btrfs_bio_alloc(bdev, (u64)sector << 9);


[1] 
http://elixir.free-electrons.com/linux/latest/source/fs/btrfs/extent_io.c#L2762
[2] 
http://elixir.free-electrons.com/linux/latest/source/include/linux/types.h#L133
> 
> 
> Thanks,
> 
BR
G.Baroncelli

-- 
gpg @keyserver.linux.it: Goffredo Baroncelli 
Key fingerprint BBF5 1610 0B64 DAC6 5F7D  17B2 0EDA 9B37 8B82 E0B5
--
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

Re: Why do full balance and deduplication reduce available free space?

[Goffredo Baroncelli]

On 10/02/2017 12:02 PM, Niccolò Belli wrote:
> Hi,
> I have several subvolumes mounted with compress-force=lzo and autodefrag. 
>> Since I use lots of snapshots (snapper keeps around 24 hourly snapshots, 7 
>> daily 
> snapshots and 4 weekly snapshots) I had to create a systemd timer to perform 
> a 
> full balance and deduplication each night. In fact data needs to be 
> already deduplicated when snapshots are created, otherwise I have no other 
> way to deduplicate snapshots.


[...] 
> Data,single: Size:44.00GiB, Used:40.00GiB
>   /dev/sda5  44.00GiB
> 
> Metadata,single: Size:5.00GiB, Used:3.78GiB
>   /dev/sda5   5.00GiB
[...]

> Data,single: Size:41.00GiB, Used:40.01GiB
>   /dev/sda5  41.00GiB
> 
> Metadata,single: Size:5.00GiB, Used:3.77GiB
>   /dev/sda5   5.00GiB
[...]

> Data,single: Size:41.00GiB, Used:40.03GiB
>   /dev/sda5  41.00GiB
> 
> Metadata,single: Size:5.00GiB, Used:3.84GiB
>   /dev/sda5   5.00GiB
[...]

> Data,single: Size:41.00GiB, Used:40.04GiB
>   /dev/sda5  41.00GiB
> 
> Metadata,single: Size:5.00GiB, Used:3.97GiB
>   /dev/sda5   5.00GiB
[]

 
> 
> It further reduced the available free space! Balance and deduplication 
> actually reduced my available free space of 400MB!
> 400MB each night!

Your data increased by 40MB (over 40GB, so about ~0.1%); instead your metadata 
increased about 200MB (over ~4GB, about ~2%); so

1) it seems to me that your data is quite "deduped"
2) (NB this is a my guessing) I think that deduping (and or re-balancing) 
rearranges the metadata leading to a increase disk usage. The only explanation 
that I found is that the deduping breaks the sharing of metadata with the 
snapshots:
- a snapshot share the metadata, which in turn refers to the data. Because the 
metadata is shared, there is only one copy. The metadata remains shared, until 
it is not changed/updated.
- dedupe, when shares a file block, updates the metadata breaking the sharing 
with its snapshot, and thus creating a copy of these.
NB: updating snapshot metadata is the same that updating subvolume metadata


> How is it possible? Should I avoid doing balances and deduplications at all?

Try few days without deduplication, and check if something change. May be that 
it would be sufficient to delay the deduping: not each night, but each week or 
month.
Another option is running dedupe on all the files (including the snapshotted 
ones). In fact this would still break the metadata sharing, but the extents 
should still be shared (IMHO :-) ). Of course the cost of deduping will 
increasing a lot (about 24+7+4 = 35 times)


> 
> Thanks,
> Niccolò

BR
G.Baroncelli

> -- 
> 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
> 


-- 
gpg @keyserver.linux.it: Goffredo Baroncelli 
Key fingerprint BBF5 1610 0B64 DAC6 5F7D  17B2 0EDA 9B37 8B82 E0B5
--
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

Re: Why do full balance and deduplication reduce available free space?

[Niccolò Belli]

Il 2017-10-02 21:35 Kai Krakow ha scritto:

Besides defragging removing the reflinks, duperemove will unshare your
snapshots when used in this way: If it sees duplicate blocks within the
subvolumes you give it, it will potentially unshare blocks from the
snapshots while rewriting extents.

BTW, you should be able to use duperemove with read-only snapshots if
used in read-only-open mode. But I'd rather suggest to use bees
instead: It works at whole-volume level, walking extents instead of
files. That way it is much faster, doesn't reprocess already
deduplicated extents, and it works with read-only snapshots.

Until my patch it didn't like mixed nodatasum/datasum workloads.
Currently this is fixed by just leaving nocow data alone as users
probably set nocow for exactly the reason to not fragment extents and
relocate blocks.


Bad Btrfs Feature Interactions: btrfs read-only snapshots (never tested, 
probably wouldn't work well)


Unfortunately it seems that bees doesn't support read-only snapshots, so 
it's a no way.


P.S.
I tried duperemove with -A, but besides taking much longer it didn't 
improve the situation.
Are you sure that the culprit is duperemove? AFAIK it shouldn't unshare 
extents...


Niccolò
--
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

Re: Why do full balance and deduplication reduce available free space?

[Kai Krakow]

Am Mon, 02 Oct 2017 12:02:16 +0200
schrieb Niccolò Belli :

> This is how I performe balance: btrfs balance start --full-balance
> rootfs This is how I perform deduplication (duperemove is from git
> master): duperemove -drh --dedupe-options=noblock
> --hashfile=../rootfs.hash 

Besides defragging removing the reflinks, duperemove will unshare your
snapshots when used in this way: If it sees duplicate blocks within the
subvolumes you give it, it will potentially unshare blocks from the
snapshots while rewriting extents.

BTW, you should be able to use duperemove with read-only snapshots if
used in read-only-open mode. But I'd rather suggest to use bees
instead: It works at whole-volume level, walking extents instead of
files. That way it is much faster, doesn't reprocess already
deduplicated extents, and it works with read-only snapshots.

Until my patch it didn't like mixed nodatasum/datasum workloads.
Currently this is fixed by just leaving nocow data alone as users
probably set nocow for exactly the reason to not fragment extents and
relocate blocks.


-- 
Regards,
Kai

Replies to list-only preferred.


--
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

Re: What means "top level" in "btrfs subvolume list" ?

[Goffredo Baroncelli]

On 09/30/2017 04:53 PM, Peter Grandi wrote:
>> My Hypothesis, it should be the ID of the root subvolume ( or
>> 5 if it is not mounted). [ ... ]
> Well, a POSIX filesystem typically has a root directory, and it
> can be mounted as the system root or any other point. A Btrfs
> filesystem has multiple root directories, that are mounted by
> default "somewhere" (a design decision that I think was unwise,
> but "whatever").

I agree that the possibility to create a subvolume/snapshot everywhere could 
create a bit of confusion; i.e. the "mountpoint" of a subvolume after a 
snapshot is a very strange beast:

ghigo@venice:/tmp$ btrfs sub cre sv1
Create subvolume './sv1'
ghigo@venice:/tmp$ btrfs sub cre sv1/sv2
Create subvolume 'sv1/sv2'
ghigo@venice:/tmp$ btrfs sub snap  sv1 sn1
Create a snapshot of 'sv1' in './sn1'
ghigo@venice:/tmp$ ls -lid sn1/
256 drwxr-xr-x 1 ghigo ghigo 6 Oct  2 20:54 sn1/
ghigo@venice:/tmp$ ls -li sn1/
total 0
2 drwxr-xr-x 1 root root 0 Oct  2 20:55 sv2
ghigo@venice:/tmp$ touch sn1/sv2/foo
touch: cannot touch 'sn1/sv2/foo': Permission denied
ghigo@venice:/tmp$ sudo touch sn1/sv2/foo
touch: cannot touch 'sn1/sv2/foo': Permission denied

It seems a directory, but it is not allowed to create file inside (!)

However the user is free to limit itself to create snapshot only under / 
(rootid==5) :-). 

 
> The subvolume containing the mountpoint directory of another
> subvolume's root directory is is no way or sense its "parent",
> as there is no derivation relationship; root directories are
> independent of each other and their mountpoint is (or should be)
> a runtime entity.

This is true. However I think that "btrfs fi list" was designed to show where 
subvolumes (or snapshots) are, for two reasons:
1) avoid to search trough all the filesystem looking for directory having 
inode=256
2) when the user mounts a subvolume as root, he has not direct access to all 
subvolume unless he also mount / (rootid=5) on another place. "btrfs sub list" 
has not this limit

BTW, my preferred choice is:
- mount a subvolume as root; 
- mount the root filesystem (rootid=5) under /var/btrfs
- make snapshot and/or subvolume only under / (rootid=5), handle these in 
/var/btrfs
- if needed, mount a subvolume in the filesystem through fstab;
- however I am still searching a way to handle the snapshots which fully 
satisfy  me (putting them under /, is not the best)

BR

-- 
gpg @keyserver.linux.it: Goffredo Baroncelli 
Key fingerprint BBF5 1610 0B64 DAC6 5F7D  17B2 0EDA 9B37 8B82 E0B5
--
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

Re: [PATCH v3 07/14] btrfs-progs: Doc/mkfs: Add extra condition for rootdir option

[Goffredo Baroncelli]

On 10/02/2017 01:47 PM, Austin S. Hemmelgarn wrote:
>>>
>>> What about doing it on a file instead of a device ? As sparse file, it 
>>> would be less expensive to enlarge then shrink. I think that who need to 
>>> build a filesystem with "shrink", doesn't need to create it on a real block 
>>> device
>>
>> For device, nothing different, just v3 patchset will handle it.
> Agreed on this point.
>>
>> For file, sparse file of course.
> But not on this one.  Unless the image gets properly compacted, a sparse file 
> will only help when you're just storing the image on a filesystem.

I think that you have misunderstood my proposal... My suggestion is to create 
the image using a file, and after image creation compact it, and then cut it at 
the end. So you don't have to care about the space needing during the process 
(before the shrinking).

Today mkfs.btrfs fails to create an image on an empty file

ghigo@venice:/tmp$ mkdir test
ghigo@venice:/tmp$ mkdir test/test1
ghigo@venice:/tmp$ touch test/test1/file
ghigo@venice:/tmp$ mkfs.btrfs --root test disk.img
btrfs-progs v4.12
See http://btrfs.wiki.kernel.org for more information.

ERROR: failed to check size for disk.img: No such file or directory
ghigo@venice:/tmp$ touch disk.img
ghigo@venice:/tmp$ mkfs.btrfs --root test disk.img
btrfs-progs v4.12
See http://btrfs.wiki.kernel.org for more information.

ERROR: 'disk.img' is too small to make a usable filesystem
ERROR: minimum size for each btrfs device is 41943040

you have to create a big enough

ghigo@venice:/tmp$ truncate -s 10G disk.img 
ghigo@venice:/tmp$ mkfs.btrfs --root test disk.img
btrfs-progs v4.12
See http://btrfs.wiki.kernel.org for more information.
[...]


BR
-- 
gpg @keyserver.linux.it: Goffredo Baroncelli 
Key fingerprint BBF5 1610 0B64 DAC6 5F7D  17B2 0EDA 9B37 8B82 E0B5
--
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

Re: Mount error on 32 bits, ok on 64 bits

[Jean-Denis Girard]

Le 30/09/2017 à 17:29, Jean-Denis Girard a écrit :
> Le 28/09/2017 à 19:26, Jean-Denis Girard a écrit :
> The problem seems to come from commit c821e7f3 "pass bytes to
> btrfs_bio_alloc" (https://patchwork.kernel.org/patch/9763081/): the
> system is now running fine on 4.13.4 with only that patch reverted.

Same situation with 4.14-rc3: my system cannot mount root file-system.
If I revert the patch, the system boots normally. This is 100%
reproducible. How can I help resolve that issue?


Thanks,
-- 
Jean-Denis Girard

SysNux   Systèmes   Linux   en   Polynésie  française
https://www.sysnux.pf/   Tél: +689 40.50.10.40 / GSM: +689 87.797.527


--
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

Re: [PATCH] btrfs-progs: right-align number columns in btrfs-debugfs output

[David Sterba]

On Sat, Sep 30, 2017 at 05:54:27PM +0200, Holger Hoffstätte  wrote:
> 
> The values for block group offset, length etc. in btrfs-debugfs' output
> are left-aligned, which creates unaligned output and makes the usage
> percentage hard to read/process further. This patch adds right-aligning
> format specifiers for the number values.
> Ideally the format values wouldn't be hardcoded but instead derived from
> the filesystem size, but this seems to work for now.
> 
> Signed-off-by: Holger Hoffstätte 

Applied, thanks.
--
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

Re: [PATCH] btrfs-progs: add option to only list parent subvolumes

[David Sterba]

On Sat, Sep 30, 2017 at 03:08:13PM +0200, Holger Hoffstätte  wrote:
> Hi,
> 
> When listing subvolumes it can be useful to filter out any snapshots;
> surprisingly enough I couldn't find an option to do this easily, only
> the opposite (list only snapshots).
> 
> A "root" subvolume is identified by a null parent UUID, so adding a new
> subvolume filter and flag -P ("Parent") does the trick. 
> 
> The same can of course be accomplished with shell hackery, e.g.:
> 
>  btrfs subvol list -q -o  | grep -i "parent_uuid -" | cut -d " " -f 11
> 
> but a built-in way seems less fragile.
> 
> I originally cooked this up for myself, but David Sterba encouraged me to
> send this to the list, so here it is. I'm not too proud of the -P but
> couldn't find a better option letter; suggestions welcome. :)

Thanks for sending it. Filtering by non-snapshots is indeed missing.
I've applied the patch as is, but I suspect the option name will change
before it ends up in a release.

We have terminology problems (again), as 'subvolume' is commonly used
for both plain subvolumes and also snapshots, but for the filtering we
need to make the clear distinction.

For the snapshot it's easy, the starting point of a snapshot is another
subvolume or snapshot, ie. there's the parent UUID. Plain subvolume gets
created by 'btrfs subvolume create'.

While the 'parent' notion reflects that a snapshot originates from
another subvolume, there's also directory structure that uses the term.
And in combination with subvolumes it's not always true that a parent
subvolume is also parent directory.

So my idea is to use '-S' as for "subvolumes", and '-s' for "snapshots"
by the strict defintion.

The output of subvolume listing will get reworked, so I'd like to keep
the final decision about the option naming open, until I get the whole
picture and some working prototype with the libsmartcols.
--
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

Re: Why do full balance and deduplication reduce available free space?

[Niccolò Belli]

Maybe this is because of the autodefrag mount option? I thought it 
wasn't supposed to unshare lots of extents...


Niccolò
--
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

RE: Is it really possible to dedupe read-only snapshots!?

[Niccolò Belli]

Il 2017-10-02 13:14 Paul Jones ha scritto:

I use bees for deduplication and it will quite happily dedupe
read-only snapshots.


AFAIK no, it isn't possible. Source: 
https://www.spinics.net/lists/linux-btrfs/msg60385.html
"It should be possible to deduplicate a read-only file to a read-write 
one, but that's probably not worth the effort in many real-world use 
cases."



You could always change them to RW while dedupe
is running then change back to RO.


AFAIK it will break send/receive, can someone confirm?

Niccolò
--
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

RE: Why do full balance and deduplication reduce available free space?

[Paul Jones]

> -Original Message-
> From: linux-btrfs-ow...@vger.kernel.org [mailto:linux-btrfs-
> ow...@vger.kernel.org] On Behalf Of Niccolò Belli
> Sent: Monday, 2 October 2017 9:29 PM
> To: Hans van Kranenburg 
> Cc: linux-btrfs@vger.kernel.org
> Subject: Re: Why do full balance and deduplication reduce available free
> space?
> 
> Il 2017-10-02 12:16 Hans van Kranenburg ha scritto:
> > On 10/02/2017 12:02 PM, Niccolò Belli wrote:
> >> [...]
> >>
> >> Since I use lots of snapshots [...] I had to create a systemd timer
> >> to perform a full balance and deduplication each night.
> >
> > Can you explain what's your reasoning behind this 'because X it needs
> > Y'? I don't follow.
> 
> Available free space is important to me, so I want snapshots to be
> deduplicated as well. Since I cannot deduplicate snapshots because they are
> read-only, then the data must be already deduplicated before the snapshots
> are taken. I do not consider the hourly snapshots because in a day they will
> be gone anyway, but daily snapshots will stay there for much longer so I want
> them to be deduplicated.

I use bees for deduplication and it will quite happily dedupe read-only 
snapshots. You could always change them to RW while dedupe is running then 
change back to RO.


Paul.

Btrfs "failed to repair damaged filesystem" - RAID10 going RO when any write attempts are made

[Timothy White]

I have a BTRFS RAID 10 filesystem that was crashing and going into RO
mode. A did a kernel upgrade, upgraded btrfs tools to the latest. A
scrub was going ok ish. btrfs check showed a number of messages such
as:

Backref 16562625503232 root 14628 owner 3609 offset 23793664 num_refs
0 not found in extent tree
Incorrect local backref count on 16562625503232 root 14628 owner 3609
offset 23793664 found 1 wanted 0 back 0x5639c37689d0
backpointer mismatch on [16562625503232 2703360]

Root 14628 was a subvolume root ID (for docker), given that I didn't
need any of that data, I removed all those subvolumes under the docker
subvolume and the docker subvolume. This still showed errors, so I
tried a btrfs check with repair, which eventually gave me the
following.

Backref 16563772112896 root 14628 owner 3608 offset 0 num_refs 0 not
found in extent tree
Incorrect local backref count on 16563772112896 root 14628 owner 3608
offset 0 found 1 wanted 0 back 0x55f3403f40b0
Backref disk bytenr does not match extent record,
bytenr=16563772112896, ref bytenr=16563813335040
Backref bytes do not match extent backref, bytenr=16563772112896, ref
bytes=134217728, backref bytes=133079040
Backref 16563772112896 root 14628 owner 3607 offset 0 num_refs 0 not
found in extent tree
Incorrect local backref count on 16563772112896 root 14628 owner 3607
offset 0 found 1 wanted 0 back 0x55f3470cfed0
Backref bytes do not match extent backref, bytenr=16563772112896, ref
bytes=134217728, backref bytes=41222144
backpointer mismatch on [16563772112896 134217728]
attempting to repair backref discrepency for bytenr 16563772112896
Ref is past the entry end, please take a btrfs-image of this file
system and send it to a btrfs developer, ref 16563813335040
failed to repair damaged filesystem, aborting

I've taken a btrfs-image, however it's 12Gb, not sure if the
developers want that, but I do have it.

Filesystem still crashes and goes read only if I try and make changes
(even deletes). The latest dmesg that includes that crash is at
https://drive.google.com/open?id=0B5bmQmu6UugIRFM0RUxwWFdqOGc. I don't
have the earlier ones.

https://drive.google.com/open?id=0B5bmQmu6UugIYjZkYnA4ZFFpdlE is the
output from running btrfs check with repair, followed by a second run
with repair to see if it got anything different.

At this stage, I expect to just blow away the filesystem and restore
from backups. However it would be nice to fix whatever the issue is.
Smartctl shows no underlying errors.

What else do the devs want from me before I blow this away? (Or is it
fixable with something I've missed, as that would save me many hours
of restoration.

The 12Gb btrfs-image can be uploaded to Google Drive (or FTP) if needed.

Thanks

Tim

$ lsb_release -a
No LSB modules are available.
Distributor ID: Debian
Description: Debian GNU/Linux 9.1 (stretch)
Release: 9.1
Codename: stretch

$ uname -a
Linux bruce 4.12.0-0.bpo.1-amd64 #1 SMP Debian 4.12.6-1~bpo9+1
(2017-08-27) x86_64 GNU/Linux

$ btrfs --version
btrfs-progs v4.9.1

$ btrfs fi show
Label: 'Butter1'  uuid: b8d081ac-0271-4481-9a58-c113c921bf49
Total devices 4 FS bytes used 5.19TiB
devid1 size 3.64TiB used 2.60TiB path /dev/sde
devid2 size 3.64TiB used 2.60TiB path /dev/sdf
devid3 size 3.64TiB used 2.60TiB path /dev/sdd
devid4 size 3.64TiB used 2.60TiB path /dev/sdc

$ btrfs fi df /mnt/Butter1
Data, RAID10: total=5.19TiB, used=5.17TiB
System, RAID10: total=128.00MiB, used=560.00KiB
Metadata, RAID10: total=12.00GiB, used=10.34GiB
Metadata, single: total=16.00MiB, used=0.00B
GlobalReserve, single: total=512.00MiB, used=0.00B

$btrfs subvolume list /mnt/Butter1
ID 258 gen 250368 top level 5 path data1
ID 259 gen 250516 top level 5 path data2
ID 3648 gen 250476 top level 5 path Photos
ID 4597 gen 203041 top level 5 path Snapshots/Photos/20160304
ID 4608 gen 203041 top level 5 path Snapshots/Photos/20160328
ID 4628 gen 203041 top level 5 path Snapshots/Photos/20160421
ID 4654 gen 250368 top level 5 path imap
ID 4656 gen 203041 top level 5 path Snapshots/Photos/20160523
ID 4893 gen 203041 top level 5 path Snapshots/Photos/20160702
ID 4946 gen 203041 top level 5 path Snapshots/Photos/20160731
ID 4947 gen 203041 top level 5 path Snapshots/Photos/20160813
ID 4948 gen 203041 top level 5 path Snapshots/Photos/2016081301
ID 4970 gen 203041 top level 5 path Snapshots/Photos/20160919
ID 5038 gen 203041 top level 5 path Snapshots/Photos/20161229_0911
ID 5063 gen 250515 top level 5 path mirror
ID 13170 gen 250428 top level 5 path BizBackups
ID 13214 gen 250368 top level 5 path SaraLaptopBackup
ID 13485 gen 250368 top level 5 path PCOMDisks
ID 16176 gen 229817 top level 5 path Snapshots/Photos/20170515
--
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

Re: Why do full balance and deduplication reduce available free space?

[Niccolò Belli]

Il 2017-10-02 12:16 Hans van Kranenburg ha scritto:

On 10/02/2017 12:02 PM, Niccolò Belli wrote:

[...]

Since I use lots of snapshots [...] I had to
create a systemd timer to perform a full balance and deduplication 
each

night.


Can you explain what's your reasoning behind this 'because X it needs
Y'? I don't follow.


Available free space is important to me, so I want snapshots to be 
deduplicated as well. Since I cannot deduplicate snapshots because they 
are read-only, then the data must be already deduplicated before the 
snapshots are taken. I do not consider the hourly snapshots because in a 
day they will be gone anyway, but daily snapshots will stay there for 
much longer so I want them to be deduplicated.


Niccolò
--
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

Re: Why do full balance and deduplication reduce available free space?

[Hans van Kranenburg]

On 10/02/2017 12:02 PM, Niccolò Belli wrote:
> [...]
> 
> Since I use lots of snapshots [...] I had to
> create a systemd timer to perform a full balance and deduplication each
> night.

Can you explain what's your reasoning behind this 'because X it needs
Y'? I don't follow.

-- 
Hans van Kranenburg
--
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

Why do full balance and deduplication reduce available free space?

[Niccolò Belli]

Hi,
I have several subvolumes mounted with compress-force=lzo and autodefrag. 
Since I use lots of snapshots (snapper keeps around 24 hourly snapshots, 7 
daily snapshots and 4 weekly snapshots) I had to create a systemd timer to 
perform a full balance and deduplication each night. In fact data needs to 
be already deduplicated when snapshots are created, otherwise I have no 
other way to deduplicate snapshots.


This is how I performe balance: btrfs balance start --full-balance rootfs
This is how I perform deduplication (duperemove is from git master):
duperemove -drh --dedupe-options=noblock --hashfile=../rootfs.hash 



Looking at the logs I noticed something weird: available free space 
actually decreases after balance or deduplication.


This is just before the timer starts:

Overall:
   Device size: 128.00GiB
   Device allocated: 49.03GiB
   Device unallocated:   78.97GiB
   Device missing:  0.00B
   Used: 43.78GiB
   Free (estimated): 82.97GiB  (min: 82.97GiB)
   Data ratio:   1.00
   Metadata ratio:   1.00
   Global reserve:  512.00MiB  (used: 0.00B)

Data,single: Size:44.00GiB, Used:40.00GiB
  /dev/sda5  44.00GiB

Metadata,single: Size:5.00GiB, Used:3.78GiB
  /dev/sda5   5.00GiB

System,single: Size:32.00MiB, Used:16.00KiB
  /dev/sda5  32.00MiB

Unallocated:
  /dev/sda5  78.97GiB



I also manually performed a full balance just before the timer starts:

Overall:
   Device size: 128.00GiB
   Device allocated: 46.03GiB
   Device unallocated:   81.97GiB
   Device missing:  0.00B
   Used: 43.78GiB
   Free (estimated): 82.96GiB  (min: 82.96GiB)
   Data ratio:   1.00
   Metadata ratio:   1.00
   Global reserve:  512.00MiB  (used: 0.00B)

Data,single: Size:41.00GiB, Used:40.01GiB
  /dev/sda5  41.00GiB

Metadata,single: Size:5.00GiB, Used:3.77GiB
  /dev/sda5   5.00GiB

System,single: Size:32.00MiB, Used:16.00KiB
  /dev/sda5  32.00MiB

Unallocated:
  /dev/sda5  81.97GiB



As you can see even doing a full balance was enough to reduce the available 
free space!


Then the timer started and it performed the deduplication:

Overall:
   Device size: 128.00GiB
   Device allocated: 46.03GiB
   Device unallocated:   81.97GiB
   Device missing:  0.00B
   Used: 43.87GiB
   Free (estimated): 82.94GiB  (min: 82.94GiB)
   Data ratio:   1.00
   Metadata ratio:   1.00
   Global reserve:  512.00MiB  (used: 176.00KiB)

Data,single: Size:41.00GiB, Used:40.03GiB
  /dev/sda5  41.00GiB

Metadata,single: Size:5.00GiB, Used:3.84GiB
  /dev/sda5   5.00GiB

System,single: Size:32.00MiB, Used:16.00KiB
  /dev/sda5  32.00MiB

Unallocated:
  /dev/sda5  81.97GiB



Once again it reduced the available free space!

Then, after the deduplication, the timer also performed a full balance:

Overall:
   Device size: 128.00GiB
   Device allocated: 46.03GiB
   Device unallocated:   81.97GiB
   Device missing:  0.00B
   Used: 44.00GiB
   Free (estimated): 82.93GiB  (min: 82.93GiB)
   Data ratio:   1.00
   Metadata ratio:   1.00
   Global reserve:  512.00MiB  (used: 0.00B)

Data,single: Size:41.00GiB, Used:40.04GiB
  /dev/sda5  41.00GiB

Metadata,single: Size:5.00GiB, Used:3.97GiB
  /dev/sda5   5.00GiB

System,single: Size:32.00MiB, Used:16.00KiB
  /dev/sda5  32.00MiB

Unallocated:
  /dev/sda5  81.97GiB




It further reduced the available free space! Balance and deduplication 
actually reduced my available free space of 400MB!

400MB each night!
How is it possible? Should I avoid doing balances and deduplications at 
all?


Thanks,
Niccolò
--
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

Re: [PATCH v2] btrfs-progs: subvol: change subvol set-default to also accept subvol path

[Hugo Mills]

On Mon, Oct 02, 2017 at 11:39:05AM +0300, Andrei Borzenkov wrote:
> On Mon, Oct 2, 2017 at 11:19 AM, Misono, Tomohiro
>  wrote:
> > This patch changes "subvol set-default" to also accept the subvolume path
> > for convenience.
> >
> > This is the one of the issue on github:
> > https://github.com/kdave/btrfs-progs/issues/35
> >
> > If there are two args, they are assumed as subvol id and path to the fs
> > (the same as current behavior), and if there is only one arg, it is assumed
> > as the path to the subvolume. Therefore there is no ambiguity between subvol
> > id and subvol name, which is mentioned in the above issue page.
> >
> > Only the absolute path to the subvolume is allowed, for the safety when
> > multiple filesystems are used.
> >
> > subvol id is resolved by get_subvol_info() which is used by "subvol show".
> >
> > change to v2:
> > restrict the path to only allow absolute path.
> 
> This is absolutely arbitrary restriction. Why we can do "btrfs
> subvolume create ./relative/path" but cannot do "btrfs subvolume
> set-default ./relative/path"?

   Indeed. In fact, it's precisely the _opposite_ of the way that
every other command works -- you provide the path to the subvolume in
the *current namespace*.

   This approach would be just a major misfeature at this point.

   Hugo.

> > documents are updated accordingly.
> >
> > Signed-off-by: Tomohiro Misono 
> > ---
> >  Documentation/btrfs-subvolume.asciidoc | 11 +++
> >  cmds-subvolume.c   | 53 
> > +++---
> >  2 files changed, 48 insertions(+), 16 deletions(-)
> >
> > diff --git a/Documentation/btrfs-subvolume.asciidoc 
> > b/Documentation/btrfs-subvolume.asciidoc
> > index 5cfe885..7973567 100644
> > --- a/Documentation/btrfs-subvolume.asciidoc
> > +++ b/Documentation/btrfs-subvolume.asciidoc
> > @@ -142,12 +142,13 @@ you can add \'\+' or \'-' in front of each items, 
> > \'+' means ascending,
> >  for --sort you can combine some items together by \',', just like
> >  --sort=+ogen,-gen,path,rootid.
> >
> > -*set-default*  ::
> > -Set the subvolume of the filesystem  which is mounted as
> > -default.
> > +*set-default* [| ]::
> > +Set the subvolume of the filesystem which is mounted as default.
> >  +
> > -The subvolume is identified by , which is returned by the *subvolume 
> > list*
> > -command.
> > +Only absolute path is allowed to specify the subvolume.
> > +Alterantively, the pair of  and  can be used. In that
> > +case, the subvolume is identified by , which is returned by the
> > +*subvolume list* command. The filesystem is specified by .
> >
> >  *show* ::
> >  Show information of a given subvolume in the .
> > diff --git a/cmds-subvolume.c b/cmds-subvolume.c
> > index 666f6e0..dda9e73 100644
> > --- a/cmds-subvolume.c
> > +++ b/cmds-subvolume.c
> > @@ -803,28 +803,59 @@ out:
> >  }
> >
> >  static const char * const cmd_subvol_set_default_usage[] = {
> > -   "btrfs subvolume set-default  ",
> > -   "Set the default subvolume of a filesystem",
> > +   "btrfs subvolume set-default [| ]",
> > +   "Set the default subvolume of the filesystem mounted as default.",
> > +   "The subvolume can be specified by its absolute path,",
> > +   "or the pair of subvolume id and mount point to the filesystem.",
> > NULL
> >  };
> >
> >  static int cmd_subvol_set_default(int argc, char **argv)
> >  {
> > -   int ret=0, fd, e;
> > -   u64 objectid;
> > -   char*path;
> > -   char*subvolid;
> > -   DIR *dirstream = NULL;
> > +   int ret = 0;
> > +   int fd, e;
> > +   u64 objectid;
> > +   char *path;
> > +   char *subvolid;
> > +   DIR *dirstream = NULL;
> >
> > clean_args_no_options(argc, argv, cmd_subvol_set_default_usage);
> >
> > -   if (check_argc_exact(argc - optind, 2))
> > +   if (check_argc_min(argc - optind, 1) ||
> > +   check_argc_max(argc - optind, 2))
> > usage(cmd_subvol_set_default_usage);
> >
> > -   subvolid = argv[optind];
> > -   path = argv[optind + 1];
> > +   if (argc - optind == 1) {
> > +   /* path to the subvolume is specified */
> > +   struct root_info ri;
> > +   char *fullpath;
> >
> > -   objectid = arg_strtou64(subvolid);
> > +   path = argv[optind];
> > +   if (path[0] != '/') {
> > +   error("only absolute path is allowed");
> > +   return 1;
> > +   }
> > +
> > +   fullpath = realpath(path, NULL);
> > +   if (!fullpath) {
> > +   error("cannot find real path for '%s': %s",
> > +   path, strerror(errno));
> > +   return 1;
> > +   }
> > +
> > +   ret = get_subvol_info(fullpath, );
> > +   free(fullpath);
> > 

[PATCH] Btrfs: fix fs_info->flags value

[Tsutomu Itoh]

Because the values of BTRFS_FS_QUOTA_OVERRIDE and BTRFS_FS_EXCL_OP overlap,
we should change the value.

Signed-off-by: Tsutomu Itoh 
---
 fs/btrfs/ctree.h | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
index 899ddaeeacec..566c0ba8dfb8 100644
--- a/fs/btrfs/ctree.h
+++ b/fs/btrfs/ctree.h
@@ -717,12 +717,11 @@ struct btrfs_delayed_root;
 #define BTRFS_FS_QUOTA_OVERRIDE14
 /* Used to record internally whether fs has been frozen */
 #define BTRFS_FS_FROZEN15
-
 /*
  * Indicate that a whole-filesystem exclusive operation is running
  * (device replace, resize, device add/delete, balance)
  */
-#define BTRFS_FS_EXCL_OP   14
+#define BTRFS_FS_EXCL_OP   16
 
 struct btrfs_fs_info {
u8 fsid[BTRFS_FSID_SIZE];
-- 
2.13.2

--
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

Re: [PATCH v2] btrfs-progs: subvol: change subvol set-default to also accept subvol path

[Andrei Borzenkov]

On Mon, Oct 2, 2017 at 11:19 AM, Misono, Tomohiro
 wrote:
> This patch changes "subvol set-default" to also accept the subvolume path
> for convenience.
>
> This is the one of the issue on github:
> https://github.com/kdave/btrfs-progs/issues/35
>
> If there are two args, they are assumed as subvol id and path to the fs
> (the same as current behavior), and if there is only one arg, it is assumed
> as the path to the subvolume. Therefore there is no ambiguity between subvol
> id and subvol name, which is mentioned in the above issue page.
>
> Only the absolute path to the subvolume is allowed, for the safety when
> multiple filesystems are used.
>
> subvol id is resolved by get_subvol_info() which is used by "subvol show".
>
> change to v2:
> restrict the path to only allow absolute path.

This is absolutely arbitrary restriction. Why we can do "btrfs
subvolume create ./relative/path" but cannot do "btrfs subvolume
set-default ./relative/path"?

> documents are updated accordingly.
>
> Signed-off-by: Tomohiro Misono 
> ---
>  Documentation/btrfs-subvolume.asciidoc | 11 +++
>  cmds-subvolume.c   | 53 
> +++---
>  2 files changed, 48 insertions(+), 16 deletions(-)
>
> diff --git a/Documentation/btrfs-subvolume.asciidoc 
> b/Documentation/btrfs-subvolume.asciidoc
> index 5cfe885..7973567 100644
> --- a/Documentation/btrfs-subvolume.asciidoc
> +++ b/Documentation/btrfs-subvolume.asciidoc
> @@ -142,12 +142,13 @@ you can add \'\+' or \'-' in front of each items, \'+' 
> means ascending,
>  for --sort you can combine some items together by \',', just like
>  --sort=+ogen,-gen,path,rootid.
>
> -*set-default*  ::
> -Set the subvolume of the filesystem  which is mounted as
> -default.
> +*set-default* [| ]::
> +Set the subvolume of the filesystem which is mounted as default.
>  +
> -The subvolume is identified by , which is returned by the *subvolume 
> list*
> -command.
> +Only absolute path is allowed to specify the subvolume.
> +Alterantively, the pair of  and  can be used. In that
> +case, the subvolume is identified by , which is returned by the
> +*subvolume list* command. The filesystem is specified by .
>
>  *show* ::
>  Show information of a given subvolume in the .
> diff --git a/cmds-subvolume.c b/cmds-subvolume.c
> index 666f6e0..dda9e73 100644
> --- a/cmds-subvolume.c
> +++ b/cmds-subvolume.c
> @@ -803,28 +803,59 @@ out:
>  }
>
>  static const char * const cmd_subvol_set_default_usage[] = {
> -   "btrfs subvolume set-default  ",
> -   "Set the default subvolume of a filesystem",
> +   "btrfs subvolume set-default [| ]",
> +   "Set the default subvolume of the filesystem mounted as default.",
> +   "The subvolume can be specified by its absolute path,",
> +   "or the pair of subvolume id and mount point to the filesystem.",
> NULL
>  };
>
>  static int cmd_subvol_set_default(int argc, char **argv)
>  {
> -   int ret=0, fd, e;
> -   u64 objectid;
> -   char*path;
> -   char*subvolid;
> -   DIR *dirstream = NULL;
> +   int ret = 0;
> +   int fd, e;
> +   u64 objectid;
> +   char *path;
> +   char *subvolid;
> +   DIR *dirstream = NULL;
>
> clean_args_no_options(argc, argv, cmd_subvol_set_default_usage);
>
> -   if (check_argc_exact(argc - optind, 2))
> +   if (check_argc_min(argc - optind, 1) ||
> +   check_argc_max(argc - optind, 2))
> usage(cmd_subvol_set_default_usage);
>
> -   subvolid = argv[optind];
> -   path = argv[optind + 1];
> +   if (argc - optind == 1) {
> +   /* path to the subvolume is specified */
> +   struct root_info ri;
> +   char *fullpath;
>
> -   objectid = arg_strtou64(subvolid);
> +   path = argv[optind];
> +   if (path[0] != '/') {
> +   error("only absolute path is allowed");
> +   return 1;
> +   }
> +
> +   fullpath = realpath(path, NULL);
> +   if (!fullpath) {
> +   error("cannot find real path for '%s': %s",
> +   path, strerror(errno));
> +   return 1;
> +   }
> +
> +   ret = get_subvol_info(fullpath, );
> +   free(fullpath);
> +
> +   if (ret)
> +   return 1;
> +
> +   objectid = ri.root_id;
> +   } else {
> +   /* subvol id and path to the filesystem are specified */
> +   subvolid = argv[optind];
> +   path = argv[optind + 1];
> +   objectid = arg_strtou64(subvolid);
> +   }
>
> fd = btrfs_open_dir(path, , 1);
> if (fd < 0)
> --
> 2.9.5
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
> the body of a message to 

[PATCH v2] btrfs-progs: subvol: change subvol set-default to also accept subvol path

[Misono, Tomohiro]

This patch changes "subvol set-default" to also accept the subvolume path
for convenience.

This is the one of the issue on github: 
https://github.com/kdave/btrfs-progs/issues/35

If there are two args, they are assumed as subvol id and path to the fs
(the same as current behavior), and if there is only one arg, it is assumed
as the path to the subvolume. Therefore there is no ambiguity between subvol
id and subvol name, which is mentioned in the above issue page.

Only the absolute path to the subvolume is allowed, for the safety when
multiple filesystems are used.

subvol id is resolved by get_subvol_info() which is used by "subvol show".

change to v2:
restrict the path to only allow absolute path.
documents are updated accordingly.

Signed-off-by: Tomohiro Misono 
---
 Documentation/btrfs-subvolume.asciidoc | 11 +++
 cmds-subvolume.c   | 53 +++---
 2 files changed, 48 insertions(+), 16 deletions(-)

diff --git a/Documentation/btrfs-subvolume.asciidoc 
b/Documentation/btrfs-subvolume.asciidoc
index 5cfe885..7973567 100644
--- a/Documentation/btrfs-subvolume.asciidoc
+++ b/Documentation/btrfs-subvolume.asciidoc
@@ -142,12 +142,13 @@ you can add \'\+' or \'-' in front of each items, \'+' 
means ascending,
 for --sort you can combine some items together by \',', just like
 --sort=+ogen,-gen,path,rootid.
 
-*set-default*  ::
-Set the subvolume of the filesystem  which is mounted as
-default.
+*set-default* [| ]::
+Set the subvolume of the filesystem which is mounted as default.
 +
-The subvolume is identified by , which is returned by the *subvolume list*
-command.
+Only absolute path is allowed to specify the subvolume.
+Alterantively, the pair of  and  can be used. In that
+case, the subvolume is identified by , which is returned by the
+*subvolume list* command. The filesystem is specified by .
 
 *show* ::
 Show information of a given subvolume in the .
diff --git a/cmds-subvolume.c b/cmds-subvolume.c
index 666f6e0..dda9e73 100644
--- a/cmds-subvolume.c
+++ b/cmds-subvolume.c
@@ -803,28 +803,59 @@ out:
 }
 
 static const char * const cmd_subvol_set_default_usage[] = {
-   "btrfs subvolume set-default  ",
-   "Set the default subvolume of a filesystem",
+   "btrfs subvolume set-default [| ]",
+   "Set the default subvolume of the filesystem mounted as default.",
+   "The subvolume can be specified by its absolute path,",
+   "or the pair of subvolume id and mount point to the filesystem.",
NULL
 };
 
 static int cmd_subvol_set_default(int argc, char **argv)
 {
-   int ret=0, fd, e;
-   u64 objectid;
-   char*path;
-   char*subvolid;
-   DIR *dirstream = NULL;
+   int ret = 0;
+   int fd, e;
+   u64 objectid;
+   char *path;
+   char *subvolid;
+   DIR *dirstream = NULL;
 
clean_args_no_options(argc, argv, cmd_subvol_set_default_usage);
 
-   if (check_argc_exact(argc - optind, 2))
+   if (check_argc_min(argc - optind, 1) ||
+   check_argc_max(argc - optind, 2))
usage(cmd_subvol_set_default_usage);
 
-   subvolid = argv[optind];
-   path = argv[optind + 1];
+   if (argc - optind == 1) {
+   /* path to the subvolume is specified */
+   struct root_info ri;
+   char *fullpath;
 
-   objectid = arg_strtou64(subvolid);
+   path = argv[optind];
+   if (path[0] != '/') {
+   error("only absolute path is allowed");
+   return 1;
+   }
+
+   fullpath = realpath(path, NULL);
+   if (!fullpath) {
+   error("cannot find real path for '%s': %s",
+   path, strerror(errno));
+   return 1;
+   }
+
+   ret = get_subvol_info(fullpath, );
+   free(fullpath);
+
+   if (ret)
+   return 1;
+
+   objectid = ri.root_id;
+   } else {
+   /* subvol id and path to the filesystem are specified */
+   subvolid = argv[optind];
+   path = argv[optind + 1];
+   objectid = arg_strtou64(subvolid);
+   }
 
fd = btrfs_open_dir(path, , 1);
if (fd < 0)
-- 
2.9.5

--
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

Re: [PATCH] btrfs-progs: subvol: change subvol set-default to also accept subvol path

[Misono, Tomohiro]

I rethink this and conclude that we should only allow the absolute path to
the subvolume in order to prevent setting wrong filesystem by mistake when 
multiple filesystems are used.

I will submit the patch again and please ignore this.

Regards,
Tomohiro

On 2017/10/02 15:25, Misono, Tomohiro wrote:
> This patch changes "subvol set-default" to also accept the subvolume path
> for convenience.
> 
> This is the one of the issue on github:
> https://github.com/kdave/btrfs-progs/issues/35
> 
> If there are two args, they are assumed as subvol id and path to the fs
> (the same as current behavior), and if there is only one arg, it is assumed
> as the path to the subvolume. Therefore there is no ambiguity between subvol
> id and subvol name, which is mentioned in the above issue page.
> 
> subvol id is resolved by get_subvol_info() which is used by "subvol show".
> 
> Signed-off-by: Tomohiro Misono 
> ---
>  Documentation/btrfs-subvolume.asciidoc | 10 +++
>  cmds-subvolume.c   | 48 
> ++
>  2 files changed, 42 insertions(+), 16 deletions(-)
> 
> diff --git a/Documentation/btrfs-subvolume.asciidoc 
> b/Documentation/btrfs-subvolume.asciidoc
> index 5cfe885..df27460 100644
> --- a/Documentation/btrfs-subvolume.asciidoc
> +++ b/Documentation/btrfs-subvolume.asciidoc
> @@ -142,12 +142,12 @@ you can add \'\+' or \'-' in front of each items, \'+' 
> means ascending,
>  for --sort you can combine some items together by \',', just like
>  --sort=+ogen,-gen,path,rootid.
>  
> -*set-default*  ::
> -Set the subvolume of the filesystem  which is mounted as
> -default.
> +*set-default* [| ]::
> +Set the subvolume of the filesystem which is mounted as default.
>  +
> -The subvolume is identified by , which is returned by the *subvolume 
> list*
> -command.
> +If  and  is given, the subvolume is identified by ,
> +which is returned by the *subvolume list* command. The filesystem
> +is specified by .
>  
>  *show* ::
>  Show information of a given subvolume in the .
> diff --git a/cmds-subvolume.c b/cmds-subvolume.c
> index 666f6e0..5bf8295 100644
> --- a/cmds-subvolume.c
> +++ b/cmds-subvolume.c
> @@ -803,28 +803,54 @@ out:
>  }
>  
>  static const char * const cmd_subvol_set_default_usage[] = {
> - "btrfs subvolume set-default  ",
> - "Set the default subvolume of a filesystem",
> + "btrfs subvolume set-default [| ]",
> + "Set the default subvolume of the filesystem mounted as default.",
> + "The subvolume can be specified by its path,",
> + "or the pair of subvolume id and path to the filesystem.",
>   NULL
>  };
>  
>  static int cmd_subvol_set_default(int argc, char **argv)
>  {
> - int ret=0, fd, e;
> - u64 objectid;
> - char*path;
> - char*subvolid;
> - DIR *dirstream = NULL;
> + int ret = 0;
> + int fd, e;
> + u64 objectid;
> + char *path;
> + char *subvolid;
> + DIR *dirstream = NULL;
>  
>   clean_args_no_options(argc, argv, cmd_subvol_set_default_usage);
>  
> - if (check_argc_exact(argc - optind, 2))
> + if (check_argc_min(argc - optind, 1) ||
> + check_argc_max(argc - optind, 2))
>   usage(cmd_subvol_set_default_usage);
>  
> - subvolid = argv[optind];
> - path = argv[optind + 1];
> + if (argc - optind == 1) {
> + /* path to the subvolume is specified */
> + struct root_info ri;
> + char *fullpath;
>  
> - objectid = arg_strtou64(subvolid);
> + path = argv[optind];
> + fullpath = realpath(path, NULL);
> + if (!fullpath) {
> + error("cannot find real path for '%s': %s",
> + path, strerror(errno));
> + return 1;
> + }
> +
> + ret = get_subvol_info(fullpath, );
> + free(fullpath);
> +
> + if (ret)
> + return 1;
> +
> + objectid = ri.root_id;
> + } else {
> + /* subvol id and path to the filesystem are specified */
> + subvolid = argv[optind];
> + path = argv[optind + 1];
> + objectid = arg_strtou64(subvolid);
> + }
>  
>   fd = btrfs_open_dir(path, , 1);
>   if (fd < 0)
> 

--
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

[PATCH] btrfs-progs: subvol: change subvol set-default to also accept subvol path

[Misono, Tomohiro]

This patch changes "subvol set-default" to also accept the subvolume path
for convenience.

This is the one of the issue on github:
https://github.com/kdave/btrfs-progs/issues/35

If there are two args, they are assumed as subvol id and path to the fs
(the same as current behavior), and if there is only one arg, it is assumed
as the path to the subvolume. Therefore there is no ambiguity between subvol
id and subvol name, which is mentioned in the above issue page.

subvol id is resolved by get_subvol_info() which is used by "subvol show".

Signed-off-by: Tomohiro Misono 
---
 Documentation/btrfs-subvolume.asciidoc | 10 +++
 cmds-subvolume.c   | 48 ++
 2 files changed, 42 insertions(+), 16 deletions(-)

diff --git a/Documentation/btrfs-subvolume.asciidoc 
b/Documentation/btrfs-subvolume.asciidoc
index 5cfe885..df27460 100644
--- a/Documentation/btrfs-subvolume.asciidoc
+++ b/Documentation/btrfs-subvolume.asciidoc
@@ -142,12 +142,12 @@ you can add \'\+' or \'-' in front of each items, \'+' 
means ascending,
 for --sort you can combine some items together by \',', just like
 --sort=+ogen,-gen,path,rootid.
 
-*set-default*  ::
-Set the subvolume of the filesystem  which is mounted as
-default.
+*set-default* [| ]::
+Set the subvolume of the filesystem which is mounted as default.
 +
-The subvolume is identified by , which is returned by the *subvolume list*
-command.
+If  and  is given, the subvolume is identified by ,
+which is returned by the *subvolume list* command. The filesystem
+is specified by .
 
 *show* ::
 Show information of a given subvolume in the .
diff --git a/cmds-subvolume.c b/cmds-subvolume.c
index 666f6e0..5bf8295 100644
--- a/cmds-subvolume.c
+++ b/cmds-subvolume.c
@@ -803,28 +803,54 @@ out:
 }
 
 static const char * const cmd_subvol_set_default_usage[] = {
-   "btrfs subvolume set-default  ",
-   "Set the default subvolume of a filesystem",
+   "btrfs subvolume set-default [| ]",
+   "Set the default subvolume of the filesystem mounted as default.",
+   "The subvolume can be specified by its path,",
+   "or the pair of subvolume id and path to the filesystem.",
NULL
 };
 
 static int cmd_subvol_set_default(int argc, char **argv)
 {
-   int ret=0, fd, e;
-   u64 objectid;
-   char*path;
-   char*subvolid;
-   DIR *dirstream = NULL;
+   int ret = 0;
+   int fd, e;
+   u64 objectid;
+   char *path;
+   char *subvolid;
+   DIR *dirstream = NULL;
 
clean_args_no_options(argc, argv, cmd_subvol_set_default_usage);
 
-   if (check_argc_exact(argc - optind, 2))
+   if (check_argc_min(argc - optind, 1) ||
+   check_argc_max(argc - optind, 2))
usage(cmd_subvol_set_default_usage);
 
-   subvolid = argv[optind];
-   path = argv[optind + 1];
+   if (argc - optind == 1) {
+   /* path to the subvolume is specified */
+   struct root_info ri;
+   char *fullpath;
 
-   objectid = arg_strtou64(subvolid);
+   path = argv[optind];
+   fullpath = realpath(path, NULL);
+   if (!fullpath) {
+   error("cannot find real path for '%s': %s",
+   path, strerror(errno));
+   return 1;
+   }
+
+   ret = get_subvol_info(fullpath, );
+   free(fullpath);
+
+   if (ret)
+   return 1;
+
+   objectid = ri.root_id;
+   } else {
+   /* subvol id and path to the filesystem are specified */
+   subvolid = argv[optind];
+   path = argv[optind + 1];
+   objectid = arg_strtou64(subvolid);
+   }
 
fd = btrfs_open_dir(path, , 1);
if (fd < 0)
-- 
2.9.5

--
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