From: Christian Ebner <c.ebner@proxmox.com>
To: Proxmox Backup Server development discussion
<pbs-devel@lists.proxmox.com>,
Thomas Lamprecht <t.lamprecht@proxmox.com>,
Christoph Heiss <c.heiss@proxmox.com>
Subject: Re: [pbs-devel] [PATCH proxmox v2] sys: file: use renameat2() from `nix` crate
Date: Thu, 18 Jul 2024 13:19:05 +0200 (CEST) [thread overview]
Message-ID: <1626412400.1424.1721301545661@webmail.proxmox.com> (raw)
In-Reply-To: <de3edd82-7c34-411b-9322-f4b58fe79a2c@proxmox.com>
> On 18.07.2024 12:29 CEST Thomas Lamprecht <t.lamprecht@proxmox.com> wrote:
>
>
> Am 12/07/2024 um 09:54 schrieb Christoph Heiss:
> > Saves us from converting the paths to raw C strings ourselves, thus
> > simplifying it quite a bit.
> >
> > Since the `nix` crate does not provide link() directly, only linkat(),
> > that is used instead - which behaves exactly the same as link() when
> > using AT_FDCWD and an empty flag set.
> >
> > No functional changes.
> >
>
> OK by me in general, but I get the following errors on build (well `cargo check`):
>
>
> ```
> Checking proxmox-sys v0.6.0 (/root/sources/pbs/proxmox/proxmox-sys)
> error[E0603]: struct `AtFlags` is private
> --> proxmox-sys/src/fs/file.rs:283:30
> |
> 283 | nix::unistd::AtFlags::empty(),
> | ^^^^^^^ private struct
> |
> note: the struct `AtFlags` is defined here
> --> /usr/share/cargo/registry/nix-0.26.1/src/unistd.rs:6:30
> |
> 6 | use crate::fcntl::{at_rawfd, AtFlags};
> | ^^^^^^^
> help: consider importing this struct instead
> |
> 283 | nix::fcntl::AtFlags::empty(),
> | ~~~~~~~~~~~~~~~~~~~
> help: import `AtFlags` directly
> |
> 283 | nix::fcntl::AtFlags(),
> | ~~~~~~~~~~~~~~~~~~~
>
> error[E0308]: mismatched types
> --> proxmox-sys/src/fs/file.rs:283:17
> |
> 278 | let result = nix::unistd::linkat(
> | ------------------- arguments to this function are incorrect
> ...
> 283 | nix::unistd::AtFlags::empty(),
> | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected `LinkatFlags`, found `AtFlags`
> |
> note: function defined here
> --> /usr/share/cargo/registry/nix-0.26.1/src/unistd.rs:1252:8
> |
> 1252 | pub fn linkat<P: ?Sized + NixPath>(
> | ^^^^^^
>
> Some errors have detailed explanations: E0308, E0603.
> For more information about an error, try `rustc --explain E0308`.
> error: could not compile `proxmox-sys` (lib) due to 2 previous errors
> ```
>
> Seems like you tested this with upstream nix, as the new type only works
> since nix version 0.28, while we still got 0.26.1.
>
> We might be able to upgrade nix, but that is often quite a bit more
> involved.
>
> Alternatively we could go with your v1 approach and apply
>
> ```
> diff --git a/proxmox-sys/src/fs/file.rs b/proxmox-sys/src/fs/file.rs
> index b2d6ddf6..28c7f88b 100644
> --- a/proxmox-sys/src/fs/file.rs
> +++ b/proxmox-sys/src/fs/file.rs
> @@ -280,7 +280,7 @@ pub fn atomic_open_or_create_file<P: AsRef<Path>>(
> &temp_file_name,
> None,
> &path.to_path_buf(),
> - nix::unistd::AtFlags::empty(),
> + nix::unistd::LinkatFlags::NoSymlinkFollow,
> );
> let _ = nix::unistd::unlink(&temp_file_name);
> result
> ```
>
> On top for now.
>
> Oh, please always test such changes with the crate versions provided by our
> Debian package provided crate registry.
>
Partially my fault, as I overlooked in my suggestion that linkat() does not accept `AtFlags` in version 0.26.1 just yet [0].
The suggested replacement type is however already there, but the module namespace is incorrect in the patch. Should have been [1].
[0] https://docs.rs/nix/0.26.1/nix/unistd/fn.linkat.html
[1] https://docs.rs/nix/0.26.1/nix/fcntl/struct.AtFlags.html
_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
next prev parent reply other threads:[~2024-07-18 11:19 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-07-12 7:54 Christoph Heiss
2024-07-12 8:16 ` Turkijan
2024-07-12 9:39 ` Christian Ebner
2024-07-18 10:29 ` Thomas Lamprecht
2024-07-18 11:19 ` Christian Ebner [this message]
2024-07-18 12:03 ` Thomas Lamprecht
2024-07-18 12:13 ` Christian Ebner
2024-07-18 12:19 ` Thomas Lamprecht
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=1626412400.1424.1721301545661@webmail.proxmox.com \
--to=c.ebner@proxmox.com \
--cc=c.heiss@proxmox.com \
--cc=pbs-devel@lists.proxmox.com \
--cc=t.lamprecht@proxmox.com \
/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