From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by lists.proxmox.com (Postfix) with ESMTPS id B012A8A44C for ; Wed, 17 Aug 2022 09:37:07 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 9E86B23D5F for ; Wed, 17 Aug 2022 09:36:37 +0200 (CEST) Received: from proxmox-new.maurer-it.com (proxmox-new.maurer-it.com [94.136.29.106]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by firstgate.proxmox.com (Proxmox) with ESMTPS for ; Wed, 17 Aug 2022 09:36:36 +0200 (CEST) Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1]) by proxmox-new.maurer-it.com (Proxmox) with ESMTP id AA0DE4437D for ; Wed, 17 Aug 2022 09:36:36 +0200 (CEST) Date: Wed, 17 Aug 2022 09:36:35 +0200 From: Wolfgang Bumiller To: Markus Frank Cc: pbs-devel@lists.proxmox.com Message-ID: <20220817073635.ptxqfvesta4udyvf@casey.proxmox.com> References: <20220816091929.26309-1-m.frank@proxmox.com> <20220816091929.26309-3-m.frank@proxmox.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20220816091929.26309-3-m.frank@proxmox.com> X-SPAM-LEVEL: Spam detection results: 0 AWL 0.274 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record T_SCC_BODY_TEXT_LINE -0.01 - URIBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to URIBL was blocked. See http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more information. [metadata.rs, flags.rs] Subject: Re: [pbs-devel] [PATCH proxmox-backup 2/3] skip xattr/acl/ownership options X-BeenThere: pbs-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox Backup Server development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 17 Aug 2022 07:37:07 -0000 On Tue, Aug 16, 2022 at 11:19:28AM +0200, Markus Frank wrote: > added cases to skip xattr/acl/ownership if the flags are not set. > Also added WITH_PERMISSIONS to Default-Flags, because otherwise it > would be needed to activly set it and most filesystems that support > XATTR and ACL also support POSIX-Permissions. > > Signed-off-by: Markus Frank > --- > pbs-client/src/pxar/flags.rs | 1 + > pbs-client/src/pxar/metadata.rs | 48 +++++++++++++++++++++------------ > 2 files changed, 32 insertions(+), 17 deletions(-) > > diff --git a/pbs-client/src/pxar/flags.rs b/pbs-client/src/pxar/flags.rs > index d46c8af3..938d0c57 100644 > --- a/pbs-client/src/pxar/flags.rs > +++ b/pbs-client/src/pxar/flags.rs > @@ -135,6 +135,7 @@ bitflags! { > Flags::WITH_FLAG_PROJINHERIT.bits() | > Flags::WITH_SUBVOLUME.bits() | > Flags::WITH_SUBVOLUME_RO.bits() | > + Flags::WITH_PERMISSIONS.bits() | > Flags::WITH_XATTRS.bits() | > Flags::WITH_ACL.bits() | > Flags::WITH_SELINUX.bits() | > diff --git a/pbs-client/src/pxar/metadata.rs b/pbs-client/src/pxar/metadata.rs > index 22bc5f9d..3195fb03 100644 > --- a/pbs-client/src/pxar/metadata.rs > +++ b/pbs-client/src/pxar/metadata.rs > @@ -100,27 +100,17 @@ pub fn apply( > on_error: &mut (dyn FnMut(Error) -> Result<(), Error> + Send), > ) -> Result<(), Error> { > let c_proc_path = CString::new(format!("/proc/self/fd/{}", fd)).unwrap(); > + apply_ownership(flags, c_proc_path.as_ptr(), metadata, &mut *on_error)?; > > - unsafe { > - // UID and GID first, as this fails if we lose access anyway. > - c_result!(libc::chown( > - c_proc_path.as_ptr(), > - metadata.stat.uid, > - metadata.stat.gid > - )) > - .map(drop) > - .or_else(allow_notsupp) > - .map_err(|err| format_err!("failed to set ownership: {}", err)) > - .or_else(&mut *on_error)?; > - } > - > - let mut skip_xattrs = false; > + let mut skip_xattrs = !flags.contains(Flags::WITH_XATTRS); > apply_xattrs(flags, c_proc_path.as_ptr(), metadata, &mut skip_xattrs) > .or_else(&mut *on_error)?; > add_fcaps(flags, c_proc_path.as_ptr(), metadata, &mut skip_xattrs).or_else(&mut *on_error)?; > - apply_acls(flags, &c_proc_path, metadata, path_info) > - .map_err(|err| format_err!("failed to apply acls: {}", err)) > - .or_else(&mut *on_error)?; > + if flags.contains(Flags::WITH_ACL) { > + apply_acls(flags, &c_proc_path, metadata, path_info) > + .map_err(|err| format_err!("failed to apply acls: {}", err)) > + .or_else(&mut *on_error)?; > + } > apply_quota_project_id(flags, fd, metadata).or_else(&mut *on_error)?; > > // Finally mode and time. We may lose access with mode, but the changing the mode also > @@ -162,6 +152,30 @@ pub fn apply( > Ok(()) > } > > +pub fn apply_ownership( > + flags: Flags, > + c_proc_path: *const libc::c_char, > + metadata: &Metadata, > + on_error: &mut (dyn FnMut(Error) -> Result<(), Error> + Send), > +) -> Result<(), Error> { > + if !flags.contains(Flags::WITH_PERMISSIONS) { > + return Ok(()); > + } While we're at it, I think we should rename `WITH_PERMISSIONS` to `WITH_OWNER`, as it doesn't affect permissions ;-) We could mask the chmod call in metadata::apply with this instead and have both flags separately? But I'd like to get rid fo this confusion in the code :-) > + unsafe { > + // UID and GID first, as this fails if we lose access anyway. > + c_result!(libc::chown( > + c_proc_path, > + metadata.stat.uid, > + metadata.stat.gid > + )) > + .map(drop) > + .or_else(allow_notsupp) > + .map_err(|err| format_err!("failed to set ownership: {}", err)) > + .or_else(&mut *on_error)?; > + } > + Ok(()) > +} > + > fn add_fcaps( > flags: Flags, > c_proc_path: *const libc::c_char, > -- > 2.30.2