public inbox for pbs-devel@lists.proxmox.com
 help / color / mirror / Atom feed
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


  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
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal