* [PATCH RFC] btrfs: send: Emit all xattr of an inode if the uid/gid changed @ 2020-03-25 1:52 Marcos Paulo de Souza 2020-03-25 1:56 ` Qu Wenruo 2020-03-26 13:51 ` Filipe Manana 0 siblings, 2 replies; 5+ messages in thread From: Marcos Paulo de Souza @ 2020-03-25 1:52 UTC (permalink / raw) To: linux-btrfs, dsterba, wqu; +Cc: Marcos Paulo de Souza From: Marcos Paulo de Souza <mpdesouza@suse.com> [PROBLEM] When doing incremental send with a file with capabilities, there is a situation where the capability can be lost in the receiving side. The sequence of actions bellow show the problem: $ mount /dev/sda fs1 $ mount /dev/sdb fs2 $ touch fs1/foo.bar $ setcap cap_sys_nice+ep fs1/foo.bar $ btrfs subvol snap -r fs1 fs1/snap_init $ btrfs send fs1/snap_init | btrfs receive fs2 $ chgrp adm fs1/foo.bar $ setcap cap_sys_nice+ep fs1/foo.bar $ btrfs subvol snap -r fs1 fs1/snap_complete $ btrfs subvol snap -r fs1 fs1/snap_incremental $ btrfs send fs1/snap_complete | btrfs receive fs2 $ btrfs send -p fs1/snap_init fs1/snap_incremental | btrfs receive fs2 At this point fs/snap_increment/foo.bar lost the capability, since a chgrp was emitted by "btrfs send". The current code only checks for the items that changed, and as the XATTR kept the value only the chgrp change is emitted. [FIX] In order to fix this issue, check if the uid/gid of the inode change, and if yes, emit all XATTR again, including the capability. Fixes: https://github.com/kdave/btrfs-progs/issues/202 Signed-off-by: Marcos Paulo de Souza <mpdesouza@suse.com> --- I'm posting this patch as a RFC because I had some questions * Is this the correct place to fix? * Also, emitting all XATTR of the inode seems overkill... * Should it be fixed in userspace? fs/btrfs/send.c | 29 +++++++++++++++++++++++++---- 1 file changed, 25 insertions(+), 4 deletions(-) diff --git a/fs/btrfs/send.c b/fs/btrfs/send.c index c5f41bd86765..5cffe5da91cf 100644 --- a/fs/btrfs/send.c +++ b/fs/btrfs/send.c @@ -6187,6 +6187,14 @@ static int changed_inode(struct send_ctx *sctx, sctx->cur_inode_mode = btrfs_inode_mode( sctx->right_path->nodes[0], right_ii); } else if (result == BTRFS_COMPARE_TREE_CHANGED) { + u64 left_uid = btrfs_inode_uid(sctx->left_path->nodes[0], + left_ii); + u64 left_gid = btrfs_inode_gid(sctx->left_path->nodes[0], + left_ii); + u64 right_uid = btrfs_inode_uid(sctx->right_path->nodes[0], + right_ii); + u64 right_gid = btrfs_inode_gid(sctx->right_path->nodes[0], + right_ii); /* * We need to do some special handling in case the inode was * reported as changed with a changed generation number. This @@ -6236,15 +6244,12 @@ static int changed_inode(struct send_ctx *sctx, sctx->send_progress = sctx->cur_ino + 1; /* - * Now process all extents and xattrs of the inode as if + * Now process all extents of the inode as if * they were all new. */ ret = process_all_extents(sctx); if (ret < 0) goto out; - ret = process_all_new_xattrs(sctx); - if (ret < 0) - goto out; } else { sctx->cur_inode_gen = left_gen; sctx->cur_inode_new = 0; @@ -6255,6 +6260,22 @@ static int changed_inode(struct send_ctx *sctx, sctx->cur_inode_mode = btrfs_inode_mode( sctx->left_path->nodes[0], left_ii); } + + /* + * Process all XATTR of the inode if the generation or owner + * changed. + * + * If the inode changed it's uid/gid, but kept a + * security.capability xattr, only the uid/gid will be emitted, + * causing the related xattr to deleted. For this reason always + * emit the XATTR when an inode has changed. + */ + if (sctx->cur_inode_new_gen || left_uid != right_uid || + left_gid != right_gid) { + ret = process_all_new_xattrs(sctx); + if (ret < 0) + goto out; + } } out: -- 2.25.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH RFC] btrfs: send: Emit all xattr of an inode if the uid/gid changed 2020-03-25 1:52 [PATCH RFC] btrfs: send: Emit all xattr of an inode if the uid/gid changed Marcos Paulo de Souza @ 2020-03-25 1:56 ` Qu Wenruo 2020-03-26 13:51 ` Filipe Manana 1 sibling, 0 replies; 5+ messages in thread From: Qu Wenruo @ 2020-03-25 1:56 UTC (permalink / raw) To: Marcos Paulo de Souza, linux-btrfs, dsterba, wqu; +Cc: Marcos Paulo de Souza [-- Attachment #1.1: Type: text/plain, Size: 4427 bytes --] On 2020/3/25 上午9:52, Marcos Paulo de Souza wrote: > From: Marcos Paulo de Souza <mpdesouza@suse.com> > > [PROBLEM] > When doing incremental send with a file with capabilities, there is a > situation where the capability can be lost in the receiving side. The > sequence of actions bellow show the problem: > > $ mount /dev/sda fs1 > $ mount /dev/sdb fs2 > > $ touch fs1/foo.bar > $ setcap cap_sys_nice+ep fs1/foo.bar > $ btrfs subvol snap -r fs1 fs1/snap_init > $ btrfs send fs1/snap_init | btrfs receive fs2 > > $ chgrp adm fs1/foo.bar > $ setcap cap_sys_nice+ep fs1/foo.bar > > $ btrfs subvol snap -r fs1 fs1/snap_complete > $ btrfs subvol snap -r fs1 fs1/snap_incremental > > $ btrfs send fs1/snap_complete | btrfs receive fs2 > $ btrfs send -p fs1/snap_init fs1/snap_incremental | btrfs receive fs2 > > At this point fs/snap_increment/foo.bar lost the capability, since a > chgrp was emitted by "btrfs send". The current code only checks for the > items that changed, and as the XATTR kept the value only the chgrp change > is emitted. > > [FIX] > In order to fix this issue, check if the uid/gid of the inode change, > and if yes, emit all XATTR again, including the capability. > > Fixes: https://github.com/kdave/btrfs-progs/issues/202 > > Signed-off-by: Marcos Paulo de Souza <mpdesouza@suse.com> > --- > I'm posting this patch as a RFC because I had some questions > * Is this the correct place to fix? > * Also, emitting all XATTR of the inode seems overkill... > * Should it be fixed in userspace? +1 for fixing it in userspace. To me, although not an expert in send, the send stream looks more like a diff between two subvolumes, other than instructions to rebuild a subvolume. So in above case, since the XATTR is not changed, not sending the XATTR is OK. Thus it's the receiver side to workaround the missing XATTR. And it shouldn't be that complex for user space to backup the XATTR, and restore it after gid/uid change. Although the main chanllege is how to distinguish plain uid/gid change (with XATTR dropped) and uid/gid change with XATTR restored. Thanks, Qu > > fs/btrfs/send.c | 29 +++++++++++++++++++++++++---- > 1 file changed, 25 insertions(+), 4 deletions(-) > > diff --git a/fs/btrfs/send.c b/fs/btrfs/send.c > index c5f41bd86765..5cffe5da91cf 100644 > --- a/fs/btrfs/send.c > +++ b/fs/btrfs/send.c > @@ -6187,6 +6187,14 @@ static int changed_inode(struct send_ctx *sctx, > sctx->cur_inode_mode = btrfs_inode_mode( > sctx->right_path->nodes[0], right_ii); > } else if (result == BTRFS_COMPARE_TREE_CHANGED) { > + u64 left_uid = btrfs_inode_uid(sctx->left_path->nodes[0], > + left_ii); > + u64 left_gid = btrfs_inode_gid(sctx->left_path->nodes[0], > + left_ii); > + u64 right_uid = btrfs_inode_uid(sctx->right_path->nodes[0], > + right_ii); > + u64 right_gid = btrfs_inode_gid(sctx->right_path->nodes[0], > + right_ii); > /* > * We need to do some special handling in case the inode was > * reported as changed with a changed generation number. This > @@ -6236,15 +6244,12 @@ static int changed_inode(struct send_ctx *sctx, > sctx->send_progress = sctx->cur_ino + 1; > > /* > - * Now process all extents and xattrs of the inode as if > + * Now process all extents of the inode as if > * they were all new. > */ > ret = process_all_extents(sctx); > if (ret < 0) > goto out; > - ret = process_all_new_xattrs(sctx); > - if (ret < 0) > - goto out; > } else { > sctx->cur_inode_gen = left_gen; > sctx->cur_inode_new = 0; > @@ -6255,6 +6260,22 @@ static int changed_inode(struct send_ctx *sctx, > sctx->cur_inode_mode = btrfs_inode_mode( > sctx->left_path->nodes[0], left_ii); > } > + > + /* > + * Process all XATTR of the inode if the generation or owner > + * changed. > + * > + * If the inode changed it's uid/gid, but kept a > + * security.capability xattr, only the uid/gid will be emitted, > + * causing the related xattr to deleted. For this reason always > + * emit the XATTR when an inode has changed. > + */ > + if (sctx->cur_inode_new_gen || left_uid != right_uid || > + left_gid != right_gid) { > + ret = process_all_new_xattrs(sctx); > + if (ret < 0) > + goto out; > + } > } > > out: > [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 488 bytes --] ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH RFC] btrfs: send: Emit all xattr of an inode if the uid/gid changed 2020-03-25 1:52 [PATCH RFC] btrfs: send: Emit all xattr of an inode if the uid/gid changed Marcos Paulo de Souza 2020-03-25 1:56 ` Qu Wenruo @ 2020-03-26 13:51 ` Filipe Manana 2020-03-26 13:52 ` Filipe Manana 2020-03-26 17:14 ` David Sterba 1 sibling, 2 replies; 5+ messages in thread From: Filipe Manana @ 2020-03-26 13:51 UTC (permalink / raw) To: Marcos Paulo de Souza Cc: linux-btrfs, David Sterba, Qu Wenruo, Marcos Paulo de Souza On Wed, Mar 25, 2020 at 1:52 AM Marcos Paulo de Souza <marcos@mpdesouza.com> wrote: > > From: Marcos Paulo de Souza <mpdesouza@suse.com> > > [PROBLEM] > When doing incremental send with a file with capabilities, there is a > situation where the capability can be lost in the receiving side. The > sequence of actions bellow show the problem: > > $ mount /dev/sda fs1 > $ mount /dev/sdb fs2 > > $ touch fs1/foo.bar > $ setcap cap_sys_nice+ep fs1/foo.bar > $ btrfs subvol snap -r fs1 fs1/snap_init > $ btrfs send fs1/snap_init | btrfs receive fs2 > > $ chgrp adm fs1/foo.bar > $ setcap cap_sys_nice+ep fs1/foo.bar > > $ btrfs subvol snap -r fs1 fs1/snap_complete > $ btrfs subvol snap -r fs1 fs1/snap_incremental > > $ btrfs send fs1/snap_complete | btrfs receive fs2 > $ btrfs send -p fs1/snap_init fs1/snap_incremental | btrfs receive fs2 > > At this point fs/snap_increment/foo.bar lost the capability, since a > chgrp was emitted by "btrfs send". The current code only checks for the > items that changed, and as the XATTR kept the value only the chgrp change > is emitted. So, the explanation could be a bit more clear. First the send stream emits a "chown" command (and not a "chgrp" command), that's what is used to change both users and groups. Then mentioning that changing the group of a file causes the capability xattr to be deleted is crucial - that's why the receiving side ends up losing the capability, because we send an operation to change the group but we don't send an operation to set the capability xattr, since the value of the xattr is the same in both snapshots. > > [FIX] > In order to fix this issue, check if the uid/gid of the inode change, > and if yes, emit all XATTR again, including the capability. > > Fixes: https://github.com/kdave/btrfs-progs/issues/202 The Fixes: tag is used to identify commits, in the kernel, that introduced some problem. A "Link:" tag is more appropriate to point to the btrfs-progs github issue. > > Signed-off-by: Marcos Paulo de Souza <mpdesouza@suse.com> > --- > I'm posting this patch as a RFC because I had some questions > * Is this the correct place to fix? Nop, see below. > * Also, emitting all XATTR of the inode seems overkill... Yes, but I wouldn't worry much - first it's not common for files to have many xattrs, second they are small values and are layed out sequentially in the btree, and above all, uids/gids are mostly static and don't change often. But that can be avoided, see below. > * Should it be fixed in userspace? No. Send should emit a sequence of operations that produces correct results in the receiving side. It should never result in any data or metadata loss, crashes, etc. This is no different from rename dependencies for example, where we make send change the order of rename and other operations so that the receiving side doesn't fail - otherwise we would have to add a lot of intelligence and complicated code to btrfs receive in progs - which brings us to another point - any consumer of a send stream would have to be changed - btrfs receive from btrfs-progs, is the most well known, and very few people will use anything else, but there may be other consumers of send streams out there. > > fs/btrfs/send.c | 29 +++++++++++++++++++++++++---- > 1 file changed, 25 insertions(+), 4 deletions(-) > > diff --git a/fs/btrfs/send.c b/fs/btrfs/send.c > index c5f41bd86765..5cffe5da91cf 100644 > --- a/fs/btrfs/send.c > +++ b/fs/btrfs/send.c > @@ -6187,6 +6187,14 @@ static int changed_inode(struct send_ctx *sctx, > sctx->cur_inode_mode = btrfs_inode_mode( > sctx->right_path->nodes[0], right_ii); > } else if (result == BTRFS_COMPARE_TREE_CHANGED) { > + u64 left_uid = btrfs_inode_uid(sctx->left_path->nodes[0], > + left_ii); > + u64 left_gid = btrfs_inode_gid(sctx->left_path->nodes[0], > + left_ii); > + u64 right_uid = btrfs_inode_uid(sctx->right_path->nodes[0], > + right_ii); > + u64 right_gid = btrfs_inode_gid(sctx->right_path->nodes[0], > + right_ii); > /* > * We need to do some special handling in case the inode was > * reported as changed with a changed generation number. This > @@ -6236,15 +6244,12 @@ static int changed_inode(struct send_ctx *sctx, > sctx->send_progress = sctx->cur_ino + 1; > > /* > - * Now process all extents and xattrs of the inode as if > + * Now process all extents of the inode as if > * they were all new. > */ > ret = process_all_extents(sctx); > if (ret < 0) > goto out; > - ret = process_all_new_xattrs(sctx); > - if (ret < 0) > - goto out; > } else { > sctx->cur_inode_gen = left_gen; > sctx->cur_inode_new = 0; > @@ -6255,6 +6260,22 @@ static int changed_inode(struct send_ctx *sctx, > sctx->cur_inode_mode = btrfs_inode_mode( > sctx->left_path->nodes[0], left_ii); > } > + > + /* > + * Process all XATTR of the inode if the generation or owner > + * changed. > + * > + * If the inode changed it's uid/gid, but kept a > + * security.capability xattr, only the uid/gid will be emitted, > + * causing the related xattr to deleted. For this reason always > + * emit the XATTR when an inode has changed. > + */ > + if (sctx->cur_inode_new_gen || left_uid != right_uid || > + left_gid != right_gid) { > + ret = process_all_new_xattrs(sctx); So the correct place for fixing this issue is at send.c:finish_inode_if_needed(), in the following place: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/fs/btrfs/send.c?h=v5.6-rc7#n6000 If a chown operation is sent, then just send the xattr - and instead if sending all xattrs, just send the xattr with the name "security.capability" - check first if there are any other capabilities that use other xattr names - if there are, just emit set_xattr operations for all xattrs with a "security." prefix in their name. Thanks. > + if (ret < 0) > + goto out; > + } > } > > out: > -- > 2.25.1 > -- Filipe David Manana, “Whether you think you can, or you think you can't — you're right.” ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH RFC] btrfs: send: Emit all xattr of an inode if the uid/gid changed 2020-03-26 13:51 ` Filipe Manana @ 2020-03-26 13:52 ` Filipe Manana 2020-03-26 17:14 ` David Sterba 1 sibling, 0 replies; 5+ messages in thread From: Filipe Manana @ 2020-03-26 13:52 UTC (permalink / raw) To: Marcos Paulo de Souza Cc: linux-btrfs, David Sterba, Qu Wenruo, Marcos Paulo de Souza On Thu, Mar 26, 2020 at 1:51 PM Filipe Manana <fdmanana@gmail.com> wrote: > > On Wed, Mar 25, 2020 at 1:52 AM Marcos Paulo de Souza > <marcos@mpdesouza.com> wrote: > > > > From: Marcos Paulo de Souza <mpdesouza@suse.com> > > > > [PROBLEM] > > When doing incremental send with a file with capabilities, there is a > > situation where the capability can be lost in the receiving side. The > > sequence of actions bellow show the problem: > > > > $ mount /dev/sda fs1 > > $ mount /dev/sdb fs2 > > > > $ touch fs1/foo.bar > > $ setcap cap_sys_nice+ep fs1/foo.bar > > $ btrfs subvol snap -r fs1 fs1/snap_init > > $ btrfs send fs1/snap_init | btrfs receive fs2 > > > > $ chgrp adm fs1/foo.bar > > $ setcap cap_sys_nice+ep fs1/foo.bar > > > > $ btrfs subvol snap -r fs1 fs1/snap_complete > > $ btrfs subvol snap -r fs1 fs1/snap_incremental > > > > $ btrfs send fs1/snap_complete | btrfs receive fs2 > > $ btrfs send -p fs1/snap_init fs1/snap_incremental | btrfs receive fs2 > > > > At this point fs/snap_increment/foo.bar lost the capability, since a > > chgrp was emitted by "btrfs send". The current code only checks for the > > items that changed, and as the XATTR kept the value only the chgrp change > > is emitted. > > So, the explanation could be a bit more clear. > > First the send stream emits a "chown" command (and not a "chgrp" > command), that's what is used to change both users and groups. > > Then mentioning that changing the group of a file causes the > capability xattr to be deleted is crucial - that's why the receiving > side ends up losing the capability, because we send an operation to > change the group but we don't send an operation to set the capability > xattr, since the value of the xattr is the same in both snapshots. > > > > > [FIX] > > In order to fix this issue, check if the uid/gid of the inode change, > > and if yes, emit all XATTR again, including the capability. > > > > Fixes: https://github.com/kdave/btrfs-progs/issues/202 > > The Fixes: tag is used to identify commits, in the kernel, that > introduced some problem. > A "Link:" tag is more appropriate to point to the btrfs-progs github issue. > > > > > Signed-off-by: Marcos Paulo de Souza <mpdesouza@suse.com> > > --- > > I'm posting this patch as a RFC because I had some questions > > * Is this the correct place to fix? > > Nop, see below. > > > * Also, emitting all XATTR of the inode seems overkill... > > Yes, but I wouldn't worry much - first it's not common for files to > have many xattrs, second they are small values and are layed out > sequentially in the btree, and above all, uids/gids are mostly static > and don't change often. > But that can be avoided, see below. > > > * Should it be fixed in userspace? > > No. > > Send should emit a sequence of operations that produces correct > results in the receiving side. It should never result in any data or > metadata loss, crashes, etc. > > This is no different from rename dependencies for example, where we > make send change the order of rename and other operations so that the > receiving side doesn't fail - otherwise we would have to add a lot of > intelligence and complicated code to btrfs receive in progs - which > brings us to another point - any consumer of a send stream would have > to be changed - btrfs receive from btrfs-progs, is the most well > known, and very few people will use anything else, but there may be > other consumers of send streams out there. > > > > > fs/btrfs/send.c | 29 +++++++++++++++++++++++++---- > > 1 file changed, 25 insertions(+), 4 deletions(-) > > > > diff --git a/fs/btrfs/send.c b/fs/btrfs/send.c > > index c5f41bd86765..5cffe5da91cf 100644 > > --- a/fs/btrfs/send.c > > +++ b/fs/btrfs/send.c > > @@ -6187,6 +6187,14 @@ static int changed_inode(struct send_ctx *sctx, > > sctx->cur_inode_mode = btrfs_inode_mode( > > sctx->right_path->nodes[0], right_ii); > > } else if (result == BTRFS_COMPARE_TREE_CHANGED) { > > + u64 left_uid = btrfs_inode_uid(sctx->left_path->nodes[0], > > + left_ii); > > + u64 left_gid = btrfs_inode_gid(sctx->left_path->nodes[0], > > + left_ii); > > + u64 right_uid = btrfs_inode_uid(sctx->right_path->nodes[0], > > + right_ii); > > + u64 right_gid = btrfs_inode_gid(sctx->right_path->nodes[0], > > + right_ii); > > /* > > * We need to do some special handling in case the inode was > > * reported as changed with a changed generation number. This > > @@ -6236,15 +6244,12 @@ static int changed_inode(struct send_ctx *sctx, > > sctx->send_progress = sctx->cur_ino + 1; > > > > /* > > - * Now process all extents and xattrs of the inode as if > > + * Now process all extents of the inode as if > > * they were all new. > > */ > > ret = process_all_extents(sctx); > > if (ret < 0) > > goto out; > > - ret = process_all_new_xattrs(sctx); > > - if (ret < 0) > > - goto out; > > } else { > > sctx->cur_inode_gen = left_gen; > > sctx->cur_inode_new = 0; > > @@ -6255,6 +6260,22 @@ static int changed_inode(struct send_ctx *sctx, > > sctx->cur_inode_mode = btrfs_inode_mode( > > sctx->left_path->nodes[0], left_ii); > > } > > + > > + /* > > + * Process all XATTR of the inode if the generation or owner > > + * changed. > > + * > > + * If the inode changed it's uid/gid, but kept a > > + * security.capability xattr, only the uid/gid will be emitted, > > + * causing the related xattr to deleted. For this reason always > > + * emit the XATTR when an inode has changed. > > + */ > > + if (sctx->cur_inode_new_gen || left_uid != right_uid || > > + left_gid != right_gid) { > > + ret = process_all_new_xattrs(sctx); > > So the correct place for fixing this issue is at > send.c:finish_inode_if_needed(), in the following place: > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/fs/btrfs/send.c?h=v5.6-rc7#n6000 > > If a chown operation is sent, then just send the xattr - and instead > if sending all xattrs, just send the xattr with the name > "security.capability" - check first if there are any other > capabilities that use other xattr names - if there are, just emit > set_xattr operations for all xattrs with a "security." prefix in their > name. > > Thanks. Also, I would change the subject of the patch, so that it mentions what problems it fixes and not how it fixes a problem. > > > > + if (ret < 0) > > + goto out; > > + } > > } > > > > out: > > -- > > 2.25.1 > > > > > -- > Filipe David Manana, > > “Whether you think you can, or you think you can't — you're right.” -- Filipe David Manana, “Whether you think you can, or you think you can't — you're right.” ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH RFC] btrfs: send: Emit all xattr of an inode if the uid/gid changed 2020-03-26 13:51 ` Filipe Manana 2020-03-26 13:52 ` Filipe Manana @ 2020-03-26 17:14 ` David Sterba 1 sibling, 0 replies; 5+ messages in thread From: David Sterba @ 2020-03-26 17:14 UTC (permalink / raw) To: Filipe Manana Cc: Marcos Paulo de Souza, linux-btrfs, David Sterba, Qu Wenruo, Marcos Paulo de Souza On Thu, Mar 26, 2020 at 01:51:20PM +0000, Filipe Manana wrote: > > * Should it be fixed in userspace? > > No. > > Send should emit a sequence of operations that produces correct > results in the receiving side. It should never result in any data or > metadata loss, crashes, etc. > > This is no different from rename dependencies for example, where we > make send change the order of rename and other operations so that the > receiving side doesn't fail - otherwise we would have to add a lot of > intelligence and complicated code to btrfs receive in progs - which > brings us to another point - any consumer of a send stream would have > to be changed - btrfs receive from btrfs-progs, is the most well > known, and very few people will use anything else, but there may be > other consumers of send streams out there. Agreed, receive should do only minimal post processing of the stream or avoid it completely. There's one workaround for the capabilities and chown (process_set_xattr, process_chown) but it does not address all cases. With the send generated with the xattr update the workaround can be dropped. > > @@ -6255,6 +6260,22 @@ static int changed_inode(struct send_ctx *sctx, > > sctx->cur_inode_mode = btrfs_inode_mode( > > sctx->left_path->nodes[0], left_ii); > > } > > + > > + /* > > + * Process all XATTR of the inode if the generation or owner > > + * changed. > > + * > > + * If the inode changed it's uid/gid, but kept a > > + * security.capability xattr, only the uid/gid will be emitted, > > + * causing the related xattr to deleted. For this reason always > > + * emit the XATTR when an inode has changed. > > + */ > > + if (sctx->cur_inode_new_gen || left_uid != right_uid || > > + left_gid != right_gid) { > > + ret = process_all_new_xattrs(sctx); > > So the correct place for fixing this issue is at > send.c:finish_inode_if_needed(), in the following place: > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/fs/btrfs/send.c?h=v5.6-rc7#n6000 > > If a chown operation is sent, then just send the xattr - and instead > if sending all xattrs, just send the xattr with the name > "security.capability" - check first if there are any other > capabilities that use other xattr names - if there are, just emit > set_xattr operations for all xattrs with a "security." prefix in their > name. Sounds good. The only problem known to me are the capabilities but in case there's more, the list can be extended. The owner change implications are documented in chown and capabilities manual pages, I haven't found anything else. ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2020-03-26 17:14 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-03-25 1:52 [PATCH RFC] btrfs: send: Emit all xattr of an inode if the uid/gid changed Marcos Paulo de Souza 2020-03-25 1:56 ` Qu Wenruo 2020-03-26 13:51 ` Filipe Manana 2020-03-26 13:52 ` Filipe Manana 2020-03-26 17:14 ` David Sterba
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.