* Re: bug#69532: mv's new -x option should be made orthogonal to -t/-T/default
From: Bernhard Voelker @ 2024-03-23 10:24 UTC (permalink / raw)
To: Karel Zak
Cc: Paul Eggert, Pádraig Brady, 69532, util-linux,
Dominique Martinet, Petr Malat, Rob Landley
In-Reply-To: <20240322102257.ovv5kpnm4zsgtf7n@ws.net.home>
On 3/22/24 11:22, Karel Zak wrote:
> On Wed, Mar 20, 2024 at 11:53:05PM +0100, Bernhard Voelker wrote:>> On userland OTOH, we have broader choice.
>> Karel did his choice in util-linux for exch(1), and coreutils could expose
>> the same functionality.
>>
>> For other feature requests, we were much more reluctant in coreutils ... for
>> good reasons: feature bloat, maintainability, etc.
>>
>> So I'm asking myself what is different this time?
>> - The feature already exists -> util-linux.
>
> Note that we can move exch(1) from util-linux to coreutils if, at the
> end of the brainstorming session, the conclusion will be that mv(1) is
> a bad choice :-)
I'd be for that as well, because ...
>> I'm currently only 20:80 for adding it to mv(1).
>
> I think the functionality will be lost in the mv(1) for many users.
... that's a fair point.
The code for the functionality is in copy.c, so - as with mv.c/cp.c/install.c -
we could have a exch.c using just that part, and thus expose a clearer interface
to the users.
Have a nice day,
Berny
^ permalink raw reply
* Re: bug#69532: mv's new -x option should be made orthogonal to -t/-T/default
From: Bernhard Voelker @ 2024-03-23 10:24 UTC (permalink / raw)
To: Paul Eggert, Pádraig Brady
Cc: 69532, util-linux, Dominique Martinet, Petr Malat, Karel Zak,
Rob Landley
In-Reply-To: <47c15dcf-3054-4a0a-9ca9-d9f7601db3ca@cs.ucla.edu>
On 3/23/24 02:44, Paul Eggert wrote:
> I installed the attached patches to do the above. (Basically, the
> problem was that my earlier patches were too ambitious; these patches
> scale things back to avoid some optimizations so that mv --exchange is
> more like ordinary mv.)
>
> The first patch simplifies the code (and fixes a diagnostic to be more
> useful) without otherwise changing behavior; it's more of a refactoring.
> The second patch does the real work.
Thanks.
We should put adding more tests on our TODO list still.
Have a nice day,
Berny
^ permalink raw reply
* Re: bug#69532: mv's new -x option should be made orthogonal to -t/-T/default
From: Paul Eggert @ 2024-03-23 1:44 UTC (permalink / raw)
To: Bernhard Voelker, Pádraig Brady
Cc: 69532, util-linux, Dominique Martinet, Petr Malat, Karel Zak,
Rob Landley
In-Reply-To: <d6530404-4fc7-40d0-be99-cb37426be32d@bernhard-voelker.de>
[-- Attachment #1: Type: text/plain, Size: 1344 bytes --]
On 3/21/24 14:45, Bernhard Voelker wrote:
> On 3/21/24 00:56, Paul Eggert wrote:
>> On 3/20/24 15:53, Bernhard Voelker wrote:
>> Yes, that's the expected behavior for this contrived case. Just as one
>> would get odd behavior if one did the same thing without --exchange.
>
> There's another which is not consistent with/without --exchange:
>
> $ src/mv -v a a
> src/mv: 'a' and 'a' are the same file
>
> $ src/mv -v --exchange a a
> renamed 'a' -> 'a'
>
> RENAME_EXCHANGE is allowed (but useless?) for 1 file.
Yes, thanks, --exchange should act more like non --exchange there.
> BTW: shouldn't the -v diagnostic better say "exchanged 'a' <-> 'a'"
> because that's what happened?
Good suggestion.
> It seems that -i is skipped:
>
> $ src/mv -iv --exchange a b
> renamed 'a' -> 'b'
Yes, I suppose -i should be treated similarly too.
I installed the attached patches to do the above. (Basically, the
problem was that my earlier patches were too ambitious; these patches
scale things back to avoid some optimizations so that mv --exchange is
more like ordinary mv.)
The first patch simplifies the code (and fixes a diagnostic to be more
useful) without otherwise changing behavior; it's more of a refactoring.
The second patch does the real work.
Thanks again.
[-- Attachment #2: 0001-cp-ln-mv-improve-dir-vs-nondir-diagnostics.patch --]
[-- Type: text/x-patch, Size: 5009 bytes --]
From ff42adb55e99226f55a7b141316ee2a7b84a4857 Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Fri, 22 Mar 2024 12:02:41 -0700
Subject: [PATCH 1/2] cp,ln,mv: improve dir vs nondir diagnostics
* src/copy.c (copy_internal): Simplify logic for copying
from directory to non-directory or vice versa, and always
diagnose with both source and destination file names.
---
src/copy.c | 88 ++++++++++++++++--------------------------------------
1 file changed, 26 insertions(+), 62 deletions(-)
diff --git a/src/copy.c b/src/copy.c
index e7bf6022f..8b4a29692 100644
--- a/src/copy.c
+++ b/src/copy.c
@@ -2451,72 +2451,36 @@ skip:
if (return_now)
return return_val;
- if (!S_ISDIR (dst_sb.st_mode))
+ /* Copying a directory onto a non-directory, or vice versa,
+ is ok only with --backup. */
+ if (!S_ISDIR (src_mode) != !S_ISDIR (dst_sb.st_mode)
+ && x->backup_type == no_backups)
{
- if (S_ISDIR (src_mode))
- {
- if (x->move_mode && x->backup_type != no_backups)
- {
- /* Moving a directory onto an existing
- non-directory is ok only with --backup. */
- }
- else
- {
- error (0, 0,
- _("cannot overwrite non-directory %s with directory %s"),
- quoteaf_n (0, dst_name), quoteaf_n (1, src_name));
- return false;
- }
- }
-
- /* Don't let the user destroy their data, even if they try hard:
- This mv command must fail (likewise for cp):
- rm -rf a b c; mkdir a b c; touch a/f b/f; mv a/f b/f c
- Otherwise, the contents of b/f would be lost.
- In the case of 'cp', b/f would be lost if the user simulated
- a move using cp and rm.
- Note that it works fine if you use --backup=numbered. */
- if (command_line_arg
- && x->backup_type != numbered_backups
- && seen_file (x->dest_info, dst_relname, &dst_sb))
- {
- error (0, 0,
- _("will not overwrite just-created %s with %s"),
- quoteaf_n (0, dst_name), quoteaf_n (1, src_name));
- return false;
- }
- }
-
- if (!S_ISDIR (src_mode))
- {
- if (S_ISDIR (dst_sb.st_mode))
- {
- if (x->move_mode && x->backup_type != no_backups)
- {
- /* Moving a non-directory onto an existing
- directory is ok only with --backup. */
- }
- else
- {
- error (0, 0,
- _("cannot overwrite directory %s with non-directory"),
- quoteaf (dst_name));
- return false;
- }
- }
+ error (0, 0,
+ _(S_ISDIR (src_mode)
+ ? ("cannot overwrite non-directory %s "
+ "with directory %s")
+ : ("cannot overwrite directory %s "
+ "with non-directory %s")),
+ quoteaf_n (0, dst_name), quoteaf_n (1, src_name));
+ return false;
}
- if (x->move_mode)
+ /* Don't let the user destroy their data, even if they try hard:
+ This mv command must fail (likewise for cp):
+ rm -rf a b c; mkdir a b c; touch a/f b/f; mv a/f b/f c
+ Otherwise, the contents of b/f would be lost.
+ In the case of 'cp', b/f would be lost if the user simulated
+ a move using cp and rm.
+ Note that it works fine if you use --backup=numbered. */
+ if (!S_ISDIR (dst_sb.st_mode) && command_line_arg
+ && x->backup_type != numbered_backups
+ && seen_file (x->dest_info, dst_relname, &dst_sb))
{
- /* Don't allow user to move a directory onto a non-directory. */
- if (S_ISDIR (src_sb.st_mode) && !S_ISDIR (dst_sb.st_mode)
- && x->backup_type == no_backups)
- {
- error (0, 0,
- _("cannot move directory onto non-directory: %s -> %s"),
- quotef_n (0, src_name), quotef_n (0, dst_name));
- return false;
- }
+ error (0, 0,
+ _("will not overwrite just-created %s with %s"),
+ quoteaf_n (0, dst_name), quoteaf_n (1, src_name));
+ return false;
}
char const *srcbase;
--
2.44.0
[-- Attachment #3: 0002-mv-treat-exchange-more-like-non-exchange.patch --]
[-- Type: text/x-patch, Size: 6162 bytes --]
From 5a1d00e4504204dc4c84eb641abb961e8074a218 Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Fri, 22 Mar 2024 18:38:08 -0700
Subject: [PATCH 2/2] mv: treat --exchange more like non-exchange
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Also, improve quality of diagnostics.
Problems/suggestions by Bernhard Voelker in
<https://bugs.gnu.org/69532#82>.
* src/copy.c (emit_verbose): New arg FORMAT. All uses changed,
to improve quality of diagnostics when --exchange is used.
(copy_internal): Don’t try to optimize --exchange so much; this
simplifies the code and keeps it closer to the non --exchange case.
---
src/copy.c | 48 +++++++++++++++++++++++-------------------------
1 file changed, 23 insertions(+), 25 deletions(-)
diff --git a/src/copy.c b/src/copy.c
index 8b4a29692..817d5b13b 100644
--- a/src/copy.c
+++ b/src/copy.c
@@ -2075,9 +2075,10 @@ abandon_move (const struct cp_options *x,
If BACKUP_DST_NAME is non-null, then also indicate that it is
the name of a backup file. */
static void
-emit_verbose (char const *src, char const *dst, char const *backup_dst_name)
+emit_verbose (char const *format, char const *src, char const *dst,
+ char const *backup_dst_name)
{
- printf ("%s -> %s", quoteaf_n (0, src), quoteaf_n (1, dst));
+ printf (format, quoteaf_n (0, src), quoteaf_n (1, dst));
if (backup_dst_name)
printf (_(" (backup: %s)"), quoteaf (backup_dst_name));
putchar ('\n');
@@ -2219,15 +2220,13 @@ copy_internal (char const *src_name, char const *dst_name,
*copy_into_self = false;
int rename_errno = x->rename_errno;
- if (x->move_mode)
+ if (x->move_mode && !x->exchange)
{
if (rename_errno < 0)
rename_errno = (renameatu (AT_FDCWD, src_name, dst_dirfd, drelname,
- (x->exchange
- ? RENAME_EXCHANGE : RENAME_NOREPLACE))
+ RENAME_NOREPLACE)
? errno : 0);
- *rename_succeeded = rename_errno == 0;
- nonexistent_dst = *rename_succeeded && !x->exchange;
+ nonexistent_dst = *rename_succeeded = rename_errno == 0;
}
if (rename_errno == 0
@@ -2248,7 +2247,7 @@ copy_internal (char const *src_name, char const *dst_name,
src_mode = src_sb.st_mode;
- if (S_ISDIR (src_mode) && !x->recursive && !x->exchange)
+ if (S_ISDIR (src_mode) && !x->recursive)
{
error (0, 0, ! x->install_mode /* cp */
? _("-r not specified; omitting directory %s")
@@ -2291,7 +2290,7 @@ copy_internal (char const *src_name, char const *dst_name,
treated the same as nonexistent files. */
bool new_dst = 0 < nonexistent_dst;
- if (! new_dst && ! x->exchange)
+ if (! new_dst)
{
/* Normally, fill in DST_SB or set NEW_DST so that later code
can use DST_SB if NEW_DST is false. However, don't bother
@@ -2452,9 +2451,9 @@ skip:
return return_val;
/* Copying a directory onto a non-directory, or vice versa,
- is ok only with --backup. */
+ is ok only with --backup or --exchange. */
if (!S_ISDIR (src_mode) != !S_ISDIR (dst_sb.st_mode)
- && x->backup_type == no_backups)
+ && x->backup_type == no_backups && !x->exchange)
{
error (0, 0,
_(S_ISDIR (src_mode)
@@ -2472,9 +2471,9 @@ skip:
Otherwise, the contents of b/f would be lost.
In the case of 'cp', b/f would be lost if the user simulated
a move using cp and rm.
- Note that it works fine if you use --backup=numbered. */
+ Nothing is lost if you use --backup=numbered or --exchange. */
if (!S_ISDIR (dst_sb.st_mode) && command_line_arg
- && x->backup_type != numbered_backups
+ && x->backup_type != numbered_backups && !x->exchange
&& seen_file (x->dest_info, dst_relname, &dst_sb))
{
error (0, 0,
@@ -2591,7 +2590,7 @@ skip:
sure we'll create a directory. Also don't announce yet when moving
so we can distinguish renames versus copies. */
if (x->verbose && !x->move_mode && !S_ISDIR (src_mode))
- emit_verbose (src_name, dst_name, dst_backup);
+ emit_verbose ("%s -> %s", src_name, dst_name, dst_backup);
/* Associate the destination file name with the source device and inode
so that if we encounter a matching dev/ino pair in the source tree
@@ -2718,17 +2717,19 @@ skip:
if (x->move_mode)
{
- if (rename_errno == EEXIST && !x->exchange)
- rename_errno = (renameat (AT_FDCWD, src_name, dst_dirfd, drelname) == 0
+ if (rename_errno == EEXIST)
+ rename_errno = ((renameatu (AT_FDCWD, src_name, dst_dirfd, drelname,
+ x->exchange ? RENAME_EXCHANGE : 0)
+ == 0)
? 0 : errno);
if (rename_errno == 0)
{
if (x->verbose)
- {
- printf (_("renamed "));
- emit_verbose (src_name, dst_name, dst_backup);
- }
+ emit_verbose (_(x->exchange
+ ? "exchanged %s <-> %s"
+ : "renamed %s -> %s"),
+ src_name, dst_name, dst_backup);
if (x->set_security_context)
{
@@ -2853,10 +2854,7 @@ skip:
}
if (x->verbose && !S_ISDIR (src_mode))
- {
- printf (_("copied "));
- emit_verbose (src_name, dst_name, dst_backup);
- }
+ emit_verbose (_("copied %s -> %s"), src_name, dst_name, dst_backup);
new_dst = true;
}
@@ -2956,7 +2954,7 @@ skip:
if (x->move_mode)
printf (_("created directory %s\n"), quoteaf (dst_name));
else
- emit_verbose (src_name, dst_name, nullptr);
+ emit_verbose ("%s -> %s", src_name, dst_name, nullptr);
}
}
else
--
2.44.0
^ permalink raw reply related
* Re: bug#69532: mv's new -x option should be made orthogonal to -t/-T/default
From: Karel Zak @ 2024-03-22 10:22 UTC (permalink / raw)
To: Bernhard Voelker
Cc: Paul Eggert, Pádraig Brady, 69532, util-linux,
Dominique Martinet, Petr Malat, Rob Landley
In-Reply-To: <dcf073d9-a60b-429d-b9e2-40f6069e2641@bernhard-voelker.de>
On Wed, Mar 20, 2024 at 11:53:05PM +0100, Bernhard Voelker wrote:
> On userland OTOH, we have broader choice.
> Karel did his choice in util-linux for exch(1), and coreutils could expose
> the same functionality.
>
> For other feature requests, we were much more reluctant in coreutils ... for
> good reasons: feature bloat, maintainability, etc.
>
> So I'm asking myself what is different this time?
> - The feature already exists -> util-linux.
Note that we can move exch(1) from util-linux to coreutils if, at the
end of the brainstorming session, the conclusion will be that mv(1) is
a bad choice :-)
> I'm currently only 20:80 for adding it to mv(1).
I think the functionality will be lost in the mv(1) for many users.
Karel
--
Karel Zak <kzak@redhat.com>
http://karelzak.blogspot.com
^ permalink raw reply
* Re: bug#69532: mv's new -x option should be made orthogonal to -t/-T/default
From: Bernhard Voelker @ 2024-03-21 21:45 UTC (permalink / raw)
To: Paul Eggert, Pádraig Brady
Cc: 69532, util-linux, Dominique Martinet, Petr Malat, Karel Zak,
Rob Landley
In-Reply-To: <e1269c19-c2b9-45ac-b683-47a473326662@cs.ucla.edu>
On 3/21/24 00:56, Paul Eggert wrote:
> On 3/20/24 15:53, Bernhard Voelker wrote:
> Yes, that's the expected behavior for this contrived case. Just as one
> would get odd behavior if one did the same thing without --exchange.
There's another which is not consistent with/without --exchange:
$ src/mv -v a a
src/mv: 'a' and 'a' are the same file
$ src/mv -v --exchange a a
renamed 'a' -> 'a'
RENAME_EXCHANGE is allowed (but useless?) for 1 file.
BTW: shouldn't the -v diagnostic better say "exchanged 'a' <-> 'a'"
because that's what happened?
>> - not sure if exchange works well together with -f.
>
> What problems do you see there?
it's up to the tests to proof that.
>> why does exchange not work to exchange a regular with a
>> directory file?
>
> It works. I don't see a problem there.
>
> $ touch a
> $ mkdir d
> $ ./mv -T --exchange a d
> $ ls -ld a d
> drwxr-xr-x. 2 eggert eggert 4096 Mar 20 16:52 a
> -rw-r--r--. 1 eggert eggert 0 Mar 20 16:52 d
indeed, it works. It seems my test was wrong, sorry.
>> Finally, the test cases are very sparse:
>
> Feel free to add some. :-)
Unfortunately, I cannot currently spend as much time as I'd love to.
It seems that -i is skipped:
$ src/mv -iv --exchange a b
renamed 'a' -> 'b'
As far as I know Padraig would like to do the next release very soon,
so I would recommend to not hurrying this one into it, and instead
ironing all out after the release.
Have a nice day,
Berny
^ permalink raw reply
* Re: bug#69532: mv's new -x option should be made orthogonal to -t/-T/default
From: Paul Eggert @ 2024-03-20 23:56 UTC (permalink / raw)
To: Bernhard Voelker, Pádraig Brady
Cc: Petr Malat, util-linux, Dominique Martinet, 69532, Karel Zak,
Rob Landley
In-Reply-To: <dcf073d9-a60b-429d-b9e2-40f6069e2641@bernhard-voelker.de>
On 3/20/24 15:53, Bernhard Voelker wrote:
> $ echo 1 > a
> $ mkdir d
> $ echo 2 > d/a
> $ src/mv -v --exchange a a a d
> renamed 'a' -> 'd/a'
> renamed 'a' -> 'd/a'
> renamed 'a' -> 'd/a'
> $ cat a
> 2
> $ src/mv -v --exchange a a a d
> renamed 'a' -> 'd/a'
> renamed 'a' -> 'd/a'
> renamed 'a' -> 'd/a'
> $ cat a
> 1
> $ src/mv -v --exchange a a a a d
> renamed 'a' -> 'd/a'
> renamed 'a' -> 'd/a'
> renamed 'a' -> 'd/a'
> renamed 'a' -> 'd/a'
> $ cat a
> 1
Yes, that's the expected behavior for this contrived case. Just as one
would get odd behavior if one did the same thing without --exchange.
> I remember some implementation where mv(1) really was just a rename(2),
> which failed when crossing file systems. Was it some HP-UX or Solaris mv(1)?
I doubt it. Even 7th Edition 'mv' (1979) fell back on 'cp' when the link
syscall failed (this was before 'rename' existed).
> My point is that "exchange" is a different functionality.
Yes, but it's closely related. Arguably --backup is also a different
functionality too (and arguably --exchange is simply an alternative
backup scheme!) but 'mv' has --backup.
> - How large is the useful overlap with the existing code of mv(1)?
> Not much: no traditional rename nor copy.
I don't follow this point. The code change was fairly small, which
indicates there was a lot of overlap with existing functionality.
> - How large is the useful overlap with the existing options/modes of mv(1)?
> - exchange contradicts --backup,
That could be fixed for regular files, if there's a need, by backing up
the destination via 'link' before exchanging. For directories it's
admittedly a problem, but that's also the case for plain 'mv' (or for
'cp' or 'ln', for that matter) so there's not much new here.
> - exchange is not useful together with options working with a regular
> rename of copy, at least: --update, -Z, -n.
It should work with --update and -Z. -n of course is logically
incompatible, but this not the only set of logically incompatible
options (e.g., -t vs -T).
> - not sure if exchange works well together with -f.
What problems do you see there?
> why does exchange not work to exchange a regular with a
> directory file?
It works. I don't see a problem there.
$ touch a
$ mkdir d
$ ./mv -T --exchange a d
$ ls -ld a d
drwxr-xr-x. 2 eggert eggert 4096 Mar 20 16:52 a
-rw-r--r--. 1 eggert eggert 0 Mar 20 16:52 d
> Finally, the test cases are very sparse:
Feel free to add some. :-)
^ permalink raw reply
* Re: bug#69532: mv's new -x option should be made orthogonal to -t/-T/default
From: Rob Landley @ 2024-03-21 0:03 UTC (permalink / raw)
To: Bernhard Voelker, Paul Eggert, Pádraig Brady
Cc: Petr Malat, util-linux, Dominique Martinet, 69532, Karel Zak
In-Reply-To: <11c9a6a2-c73d-4b99-b071-48e7eab2fd19@bernhard-voelker.de>
On 3/20/24 14:43, Bernhard Voelker wrote:
> On 3/17/24 07:10, Paul Eggert wrote:
> Now, extending "exchange" to more arguments is confusing and the
> use is not intuitive:
> mv -v --exchange a b c d
It's also pointless. An atomic exchange on more than 2 files ISN'T ATOMIC.
That's why I didn't do it.
You already had "mv -T" requiring exactly two arguments, so thinking mv -x has
cooties because it works the same way is just weird.
> I have the gut feeling that we didn't think through all cases,
Sounds like. Having mv modify its source directory during recursive descent is
creepy.
Toybox implemented:
-x Atomically exchange source/dest (--swap)
Which behaves like:
$ ./mv -x one two
$ ./mv -x one two three
mv: -x needs 2 args
My change from this discussion was adding the "--swap" synonym you wanted.
Rob
^ permalink raw reply
* Re: bug#69532: mv's new -x option should be made orthogonal to -t/-T/default
From: Bernhard Voelker @ 2024-03-20 22:53 UTC (permalink / raw)
To: Paul Eggert, Pádraig Brady
Cc: 69532, util-linux, Dominique Martinet, Petr Malat, Karel Zak,
Rob Landley
In-Reply-To: <4356f3eb-544e-4ce9-b9ea-374cd01663d5@cs.ucla.edu>
On 3/20/24 21:56, Paul Eggert wrote:
> On 3/20/24 12:43, Bernhard Voelker wrote:
>
>> This stems from the fact that although mv(1) is a userland frontend
>> for renameat(2), the user interface is different:
>> while renameat(2) deals exactly with 2 operands, mv(1) has always
>> been able to work on more arguments.
>
> Yes, that's mv's original sin, which we cannot realistically change now.
I wouldn't go that far that it was a sin. It's useful and people got
used to it without having to think about it.
>> I have the gut feeling that we didn't think through all cases,
>> and that some might be surprising, e.g.:
>>
>> $ mkdir d; echo 1 > a; echo 2 > d/a
>> $ src/mv --exchange a a a a d/a
>>
>> versus
>>
>> $ src/mv --exchange a a a a d/a
>
> I don't understand the word "versus" here, as the two examples look the
> same to me.
sorry, I messed the example up.
$ echo 1 > a
$ mkdir d
$ echo 2 > d/a
$ src/mv -v --exchange a a a d
renamed 'a' -> 'd/a'
renamed 'a' -> 'd/a'
renamed 'a' -> 'd/a'
$ cat a
2
$ src/mv -v --exchange a a a d
renamed 'a' -> 'd/a'
renamed 'a' -> 'd/a'
renamed 'a' -> 'd/a'
$ cat a
1
$ src/mv -v --exchange a a a a d
renamed 'a' -> 'd/a'
renamed 'a' -> 'd/a'
renamed 'a' -> 'd/a'
renamed 'a' -> 'd/a'
$ cat a
1
I remember some implementation where mv(1) really was just a rename(2),
which failed when crossing file systems. Was it some HP-UX or Solaris mv(1)?
mv(1) has learned to copy+delete over time, which is what people would
expect from a "move".
My point is that "exchange" is a different functionality.
It's well somehow belonging and related to what renameat(2) is doing in the kernel,
and therefore it comes in handy that we can simply call it with an additional flag.
Yet it's IMO a different operation. I bet there had been discussions whether
to create a new syscall, but apparently it was easier to put it with a flag
into an existing one. Fine for the kernel.
On userland OTOH, we have broader choice.
Karel did his choice in util-linux for exch(1), and coreutils could expose
the same functionality.
For other feature requests, we were much more reluctant in coreutils ... for
good reasons: feature bloat, maintainability, etc.
So I'm asking myself what is different this time?
- The feature already exists -> util-linux.
- Okay, we're using the same syscall, renameat(2) -> it's tempting.
- How large is the useful overlap with the existing code of mv(1)?
Not much: no traditional rename nor copy.
- How large is the useful overlap with the existing options/modes of mv(1)?
- exchange contradicts --backup,
- exchange is not useful together with options working with a regular
rename of copy, at least: --update, -Z, -n.
- not sure if exchange works well together with -f.
I'm currently only 20:80 for adding it to mv(1).
The functionality is cool, but do we need to press it into mv(1) with so many
incompatibilities just because it's requiring renameat(2) we already use?
Maybe to consider: One tool for one thing ... means another tool for another thing.
Again, I have the gut feeling that we've missed some cases to think about.
And once the feature would be in ...
Furthermore, why does exchange not work to exchange a regular with a directory file?
We've all learned that everything's a file, so it cannot be explained to users that
exchanging a regular file with a directory doesn't work.
Finally, the test cases are very sparse: no cases with different owners, different
directory permissions, different file types (if we know already f<->d doesn't work),
triggering races, etc.
I don't really want to object to add it, but I find it quite odd as of today.
Have a nice day,
Berny
^ permalink raw reply
* Re: bug#69532: mv's new -x option should be made orthogonal to -t/-T/default
From: Paul Eggert @ 2024-03-20 22:10 UTC (permalink / raw)
To: Pádraig Brady
Cc: 69532, Petr Malat, Rob Landley, util-linux, Dominique Martinet,
Karel Zak
In-Reply-To: <5308840a-9de4-33d0-2520-5f3addc9718c@draigBrady.com>
On 3/17/24 04:32, Pádraig Brady wrote:
> I think the --no-copy situation is brittle, as scripts not using it now
> would be atomic, but then if we ever supported cross fs swaps
> it may become non atomic. I'd at least doc with a line in the --exchange
> description in usage() to say something like:
> "Use --no-copy to enforce atomic operation"
But --no-copy doesn't mean atomic operation; it simply means don't copy.
On systems that don't have an atomic exchange, we can emulate "mv
--exchange --no-copy a b" with three calls link("b", "b.tmp");
rename("a","b"); rename("b.tmp","a"). This wouldn't copy, but it
wouldn't be atomic.
Although atomicity is important, currently the coreutils documentation
doesn't cover it and the code doesn't always handle it well. For
example, if A and B are regular files "mv -b A B" briefly has a moment
when B stops existing. To my mind this is a bug: an existing destination
shouldn't stop existing merely because you're replacing it. At some
point this stuff should be documented better and this (and probably some
other) atomicity bugs fixed.
One thought I had while looking into this was that we could add an
--atomic option to mv and ln, such that the command fails if the
destination cannot be updated atomically. That would be a stronger
option than --no-copy. (In some cases we could support --atomic even for
'cp', no?)
Anyway, for now I installed the patch with some minor changes to the
documentation's --exchange section to try to document this tricky area
more clearly. Here's the revised doc section. It also incorporates your
later suggestion to mention both data and metadata.
----
@item --exchange
@opindex --exchange
Exchange source and destination instead of renaming source to destination.
Both files must exist; they need not be the same type.
This exchanges all data and metadata.
This option can be used to replace one directory with another.
When used this way, it should be combined with
@code{--no-target-directory} (@option{-T})
to avoid confusion about the destination location.
For example, you might use @samp{mv -T --exchange @var{d1} @var{d2}}
to exchange two directories @var{d1} and @var{d2}.
Exchanges are atomic if the source and destination are both in a
single file system that supports atomic exchange.
Non-atomic exchanges are not yet supported.
If the source and destination might not be on the same file system,
using @code{--no-copy} will prevent future versions of @command{mv}
from implementing the exchange by copying.
^ permalink raw reply
* Re: bug#69532: mv's new -x option should be made orthogonal to -t/-T/default
From: Paul Eggert @ 2024-03-20 20:56 UTC (permalink / raw)
To: Bernhard Voelker, Pádraig Brady
Cc: Petr Malat, util-linux, Dominique Martinet, 69532, Karel Zak,
Rob Landley
In-Reply-To: <11c9a6a2-c73d-4b99-b071-48e7eab2fd19@bernhard-voelker.de>
On 3/20/24 12:43, Bernhard Voelker wrote:
> This stems from the fact that although mv(1) is a userland frontend
> for renameat(2), the user interface is different:
> while renameat(2) deals exactly with 2 operands, mv(1) has always
> been able to work on more arguments.
Yes, that's mv's original sin, which we cannot realistically change now.
> Now, extending "exchange" to more arguments is confusing and the
> use is not intuitive:
> mv -v --exchange a b c d
>
> An "exchange" can literally only be applied to 2 files,
Sure, but that's true for "rename" too: a "rename" can be applied only
to 2 files.
When d is a directory, "mv a b c d" does three renames so it is like "mv
a d/a; mv b d/b; mv c d/c". This remains true if you uniformly replace
"mv" with "mv --exchange", which does three exchanges.
> I have the gut feeling that we didn't think through all cases,
> and that some might be surprising, e.g.:
>
> $ mkdir d; echo 1 > a; echo 2 > d/a
> $ src/mv --exchange a a a a d/a
>
> versus
>
> $ src/mv --exchange a a a a d/a
I don't understand the word "versus" here, as the two examples look the
same to me.
If d/a is not a directory, the example is an error, just as it would be
without --exchange.
If d/a is a directory and you have permissions etc., "mv a a a a d/a" is
like attempting "mv -T a d/a/a; mv -T a d/a/a; mv -T a d/a/a; mv -T a
d/a/a". If you use plain "mv" only the first "mv -T a d/a/a" succeeds
because "a" goes away, so you get three diagnostics for the remaining
three "a"s. If you use "mv --exchange" all four "mv --exchange -T a
d/a/a" attempts succeed, and since there are an even number of exchanges
the end result is a no-op except for updated directory timestamps. So I
don't see any ambiguity about what mv should do with this contrived example.
^ permalink raw reply
* Re: bug#69532: mv's new -x option should be made orthogonal to -t/-T/default
From: Bernhard Voelker @ 2024-03-20 19:43 UTC (permalink / raw)
To: Paul Eggert, Pádraig Brady
Cc: Petr Malat, util-linux, Dominique Martinet, 69532, Karel Zak,
Rob Landley
In-Reply-To: <c5622a1c-b59e-4eb3-9d81-111acc1fbddc@cs.ucla.edu>
On 3/17/24 07:10, Paul Eggert wrote:
> Although removing that "mv --swap" implementation was a win, I don't
> think we can simply delegate this to util-linux's exch command.
I still have some headache adding this.
This stems from the fact that although mv(1) is a userland frontend
for renameat(2), the user interface is different:
while renameat(2) deals exactly with 2 operands, mv(1) has always
been able to work on more arguments.
Now, extending "exchange" to more arguments is confusing and the
use is not intuitive:
mv -v --exchange a b c d
An "exchange" can literally only be applied to 2 files,
and 'exch' is IMO fine.
I have the gut feeling that we didn't think through all cases,
and that some might be surprising, e.g.:
$ mkdir d; echo 1 > a; echo 2 > d/a
$ src/mv --exchange a a a a d/a
versus
$ src/mv --exchange a a a a d/a
Have a nice day,
Berny
^ permalink raw reply
* Re: [PATCH] chcpu(8): document limitations of -g
From: Karel Zak @ 2024-03-20 14:44 UTC (permalink / raw)
To: Stanislav Brabec; +Cc: util-linux@vger.kernel.org, Heikki Ylipiessa
In-Reply-To: <52efc277-463c-441e-8284-595cefa254e9@suse.cz>
On Mon, Mar 18, 2024 at 06:39:36PM +0100, Stanislav Brabec wrote:
> sys-utils/chcpu.8.adoc | 2 ++
> 1 file changed, 2 insertions(+)
Applied, thanks.
Karel
--
Karel Zak <kzak@redhat.com>
http://karelzak.blogspot.com
^ permalink raw reply
* [PATCH] chcpu(8): document limitations of -g
From: Stanislav Brabec @ 2024-03-18 17:39 UTC (permalink / raw)
To: util-linux@vger.kernel.org; +Cc: Heikki Ylipiessa
Document that chcpu -g is not supported on IBM z/VM because the detach cpu
would CLEAR the running zVM guest memory.
References:
https://www.ibm.com/docs/en/linux-on-z?topic=mc-changing-state-1
https://www.ibm.com/docs/en/zvm/7.3?topic=commands-detach-cpu
Reported-by: Heikki Ylipiessa <heikki.ylipiessa@suse.com>
Signed-off-by: Stanislav Brabec <sbrabec@suse.cz>
---
sys-utils/chcpu.8.adoc | 2 ++
1 file changed, 2 insertions(+)
diff --git a/sys-utils/chcpu.8.adoc b/sys-utils/chcpu.8.adoc
index c5797dfb3..5b28ef8d2 100644
--- a/sys-utils/chcpu.8.adoc
+++ b/sys-utils/chcpu.8.adoc
@@ -37,6 +37,8 @@ Enable the specified CPUs. Enabling a CPU means that
the kernel sets it online.
*-g*, *--deconfigure* _cpu-list_::
Deconfigure the specified CPUs. Deconfiguring a CPU means that the
hypervisor removes the CPU from the virtual hardware on which the Linux
instance runs and returns it to the CPU pool. A CPU must be offline, see
*-d*, before it can be deconfigured.
++
+*chcpu -g* is not supported on IBM z/VM, CPUs are always in a
configured state there.
*-p*, *--dispatch* _mode_::
Set the CPU dispatching _mode_ (polarization). This option has an
effect only if your hardware architecture and hypervisor support CPU
polarization. Available _modes_ are:
--
2.43.0
--
Best Regards / S pozdravem,
Stanislav Brabec
software developer
---------------------------------------------------------------------
SUSE LINUX, s. r. o. e-mail: sbrabec@suse.com
Křižíkova 148/34 (Corso IIa) tel: +420 284 084 060
186 00 Praha 8-Karlín fax: +420 284 084 001
Czech Republic http://www.suse.cz/
PGP: 830B 40D5 9E05 35D8 5E27 6FA3 717C 209F A04F CD76
^ permalink raw reply related
* Re: bug#69532: mv's new -x option should be made orthogonal to -t/-T/default
From: Pádraig Brady @ 2024-03-17 11:40 UTC (permalink / raw)
To: Paul Eggert
Cc: 69532, Petr Malat, Rob Landley, util-linux, Dominique Martinet,
Karel Zak
In-Reply-To: <5308840a-9de4-33d0-2520-5f3addc9718c@draigBrady.com>
On 17/03/2024 11:32, Pádraig Brady wrote:
> On 17/03/2024 06:10, Paul Eggert wrote:
>> On 2024-03-05 06:16, Pádraig Brady wrote:
>>> I think I'll remove the as yet unreleased mv --swap from coreutils,
>>> given that
>>> util-linux is as widely available as coreutils on GNU/Linux platforms.
>>
>> Although removing that "mv --swap" implementation was a win, I don't
>> think we can simply delegate this to util-linux's exch command.
>> Exchanging files via a renameat-like call is not limited to the Linux
>> kernel; it's also possible on macOS via renameatx_np with RENAME_SWAP,
>> and there have been noises about adding similar things to other
>> operating systems.
>>
>> I just now added support for macOS renameatx_np to Gnulib, so coreutils
>> does not need to worry about the macOS details; it can simply use
>> renameatu with the Linux flags. See:
>>
>> https://git.savannah.gnu.org/cgit/gnulib.git/commit/?id=af32ee824ee18255839f9812b8ed61aa5257a82b
>>
>> Even with Linux it's dicey. People may have older util-linux installed
>> and so lack the 'exch' utility; this is true for both Fedora 39 and
>> Ubuntu 23.10, the current releases. Ubuntu is also odd in that it
>> doesn't install all the util-linux utilities as part of the util-linux
>> package, so it's not clear what they will do with 'exch'.
>>
>> So I propose that we implement the idea in coreutils in a better way,
>> that interacts more nicely with -t, -T, etc. Also, I suggest using the
>> Linuxish name "--exchange" instead of the macOSish name "--swap", and
>> (for now at least) not giving the option a single-letter equivalent as I
>> expect it to be useful from scripts, not interactively.
>>
>> After looking at various ways to do it I came up with the attached
>> proposed patch. This should work on both GNU/Linux and macOS, if your OS
>> is recent enough and the file system supports atomic exchange.
>
> The implementation looks good.
>
> Re exch(1) on macos, I see util-linux is on homebrew,
> so it would be relatively easy to ifdef renameatx_np in util-linux also.
>
> I think the --no-copy situation is brittle, as scripts not using it now
> would be atomic, but then if we ever supported cross fs swaps
> it may become non atomic. I'd at least doc with a line in the --exchange
> description in usage() to say something like:
> "Use --no-copy to enforce atomic operation"
>
> While the most flexible, it's also quite awkward to need
> `mv --exchange --no-copy --no-target-directory` for most uses.
> I.e. it's tempting to imply the --no-... options with --exchange,
> but I suppose since scripting is the primary use case for this
> flexibility trumps conciseness, so I'm ok with the verbosity I think.
Oh also in the texinfo I think it's important to mention that the swap
will "exchange all data and metadata". That's not obvious otherwise.
For example users may be wondering if only data was being exchanged
with the macos exchangedata(2) or equivalent.
cheers,
Pádraig
^ permalink raw reply
* Re: bug#69532: mv's new -x option should be made orthogonal to -t/-T/default
From: Pádraig Brady @ 2024-03-17 11:32 UTC (permalink / raw)
To: Paul Eggert
Cc: 69532, Petr Malat, Rob Landley, util-linux, Dominique Martinet,
Karel Zak
In-Reply-To: <c5622a1c-b59e-4eb3-9d81-111acc1fbddc@cs.ucla.edu>
On 17/03/2024 06:10, Paul Eggert wrote:
> On 2024-03-05 06:16, Pádraig Brady wrote:
>> I think I'll remove the as yet unreleased mv --swap from coreutils,
>> given that
>> util-linux is as widely available as coreutils on GNU/Linux platforms.
>
> Although removing that "mv --swap" implementation was a win, I don't
> think we can simply delegate this to util-linux's exch command.
> Exchanging files via a renameat-like call is not limited to the Linux
> kernel; it's also possible on macOS via renameatx_np with RENAME_SWAP,
> and there have been noises about adding similar things to other
> operating systems.
>
> I just now added support for macOS renameatx_np to Gnulib, so coreutils
> does not need to worry about the macOS details; it can simply use
> renameatu with the Linux flags. See:
>
> https://git.savannah.gnu.org/cgit/gnulib.git/commit/?id=af32ee824ee18255839f9812b8ed61aa5257a82b
>
> Even with Linux it's dicey. People may have older util-linux installed
> and so lack the 'exch' utility; this is true for both Fedora 39 and
> Ubuntu 23.10, the current releases. Ubuntu is also odd in that it
> doesn't install all the util-linux utilities as part of the util-linux
> package, so it's not clear what they will do with 'exch'.
>
> So I propose that we implement the idea in coreutils in a better way,
> that interacts more nicely with -t, -T, etc. Also, I suggest using the
> Linuxish name "--exchange" instead of the macOSish name "--swap", and
> (for now at least) not giving the option a single-letter equivalent as I
> expect it to be useful from scripts, not interactively.
>
> After looking at various ways to do it I came up with the attached
> proposed patch. This should work on both GNU/Linux and macOS, if your OS
> is recent enough and the file system supports atomic exchange.
The implementation looks good.
Re exch(1) on macos, I see util-linux is on homebrew,
so it would be relatively easy to ifdef renameatx_np in util-linux also.
I think the --no-copy situation is brittle, as scripts not using it now
would be atomic, but then if we ever supported cross fs swaps
it may become non atomic. I'd at least doc with a line in the --exchange
description in usage() to say something like:
"Use --no-copy to enforce atomic operation"
While the most flexible, it's also quite awkward to need
`mv --exchange --no-copy --no-target-directory` for most uses.
I.e. it's tempting to imply the --no-... options with --exchange,
but I suppose since scripting is the primary use case for this
flexibility trumps conciseness, so I'm ok with the verbosity I think.
thanks,
Pádraig
^ permalink raw reply
* Re: bug#69532: mv's new -x option should be made orthogonal to -t/-T/default
From: Paul Eggert @ 2024-03-17 6:10 UTC (permalink / raw)
To: Pádraig Brady
Cc: 69532, Petr Malat, Rob Landley, util-linux, Dominique Martinet,
Karel Zak
In-Reply-To: <636f1247-0de0-2f32-cb04-f6952b6807aa@draigBrady.com>
[-- Attachment #1: Type: text/plain, Size: 1760 bytes --]
On 2024-03-05 06:16, Pádraig Brady wrote:
> I think I'll remove the as yet unreleased mv --swap from coreutils,
> given that
> util-linux is as widely available as coreutils on GNU/Linux platforms.
Although removing that "mv --swap" implementation was a win, I don't
think we can simply delegate this to util-linux's exch command.
Exchanging files via a renameat-like call is not limited to the Linux
kernel; it's also possible on macOS via renameatx_np with RENAME_SWAP,
and there have been noises about adding similar things to other
operating systems.
I just now added support for macOS renameatx_np to Gnulib, so coreutils
does not need to worry about the macOS details; it can simply use
renameatu with the Linux flags. See:
https://git.savannah.gnu.org/cgit/gnulib.git/commit/?id=af32ee824ee18255839f9812b8ed61aa5257a82b
Even with Linux it's dicey. People may have older util-linux installed
and so lack the 'exch' utility; this is true for both Fedora 39 and
Ubuntu 23.10, the current releases. Ubuntu is also odd in that it
doesn't install all the util-linux utilities as part of the util-linux
package, so it's not clear what they will do with 'exch'.
So I propose that we implement the idea in coreutils in a better way,
that interacts more nicely with -t, -T, etc. Also, I suggest using the
Linuxish name "--exchange" instead of the macOSish name "--swap", and
(for now at least) not giving the option a single-letter equivalent as I
expect it to be useful from scripts, not interactively.
After looking at various ways to do it I came up with the attached
proposed patch. This should work on both GNU/Linux and macOS, if your OS
is recent enough and the file system supports atomic exchange.
[-- Attachment #2: 0001-mv-new-option-exchange.patch --]
[-- Type: text/x-patch, Size: 12415 bytes --]
From d522aba06107d3532ad6103470727bf9057f8d2c Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Sat, 16 Mar 2024 22:50:17 -0700
Subject: [PATCH] mv: new option --exchange
* src/copy.h (struct cp_options): New member 'exchange'.
* src/copy.c (copy_internal): Support the new member.
* src/mv.c (EXCHANGE_OPTION): New constant.
(long_options): Add --exchange.
(usage): Document --exchange.
(main): Support --exchange.
* tests/mv/mv-exchange.sh: New test case.
* tests/local.mk (all_tests): Add it.
---
NEWS | 7 ++++++
doc/coreutils.texi | 18 ++++++++++++++
src/copy.c | 54 +++++++++++++++++++++++------------------
src/copy.h | 4 +++
src/mv.c | 16 +++++++++---
tests/local.mk | 1 +
tests/mv/mv-exchange.sh | 41 +++++++++++++++++++++++++++++++
7 files changed, 114 insertions(+), 27 deletions(-)
create mode 100755 tests/mv/mv-exchange.sh
diff --git a/NEWS b/NEWS
index f21efc7c0..67bb27ebb 100644
--- a/NEWS
+++ b/NEWS
@@ -81,6 +81,13 @@ GNU coreutils NEWS -*- outline -*-
and the command exits with failure status if existing files.
The -n,--no-clobber option is best avoided due to platform differences.
+ mv now accepts an --exchange option, which causes the source and
+ destination to be exchanged. It should be combined with
+ --no-target-directory (-T) if the destination is a directory.
+ The exchange is atomic if source and destination are on a single
+ file system that supports atomic exchange; --exchange is not yet
+ supported in other situations.
+
od now supports printing IEEE half precision floating point with -t fH,
or brain 16 bit floating point with -t fB, where supported by the compiler.
diff --git a/doc/coreutils.texi b/doc/coreutils.texi
index d07ed7e76..c456a03d9 100644
--- a/doc/coreutils.texi
+++ b/doc/coreutils.texi
@@ -10269,6 +10269,24 @@ skip existing files but not fail.
If a file cannot be renamed because the destination file system differs,
fail with a diagnostic instead of copying and then removing the file.
+@item --exchange
+@opindex --exchange
+Exchange source and destination instead of renaming source to destination.
+Both files must exist; they need not be the same type.
+The exchange is atomic if the source and destination are both in a
+single file system that supports atomic exchange;
+exchanges are not yet supported in other situations.
+
+This option can be used to replace one directory with another, atomically.
+When used this way, it should be combined with
+@code{--no-target-directory} (@option{-T})
+to avoid confusion about the destination location.
+Also, if the two directories might not be on the same file system,
+using @code{--no-copy} will prevent future
+versions of @command{mv} from implementing the exchange by copying.
+For example, you might use @samp{mv -T --exchange --no-copy
+@var{d1} @var{d2}} to exchange the directories @var{d1} and @var{d2}.
+
@item -u
@itemx --update
@opindex -u
diff --git a/src/copy.c b/src/copy.c
index 8d99f8562..e7bf6022f 100644
--- a/src/copy.c
+++ b/src/copy.c
@@ -2223,9 +2223,11 @@ copy_internal (char const *src_name, char const *dst_name,
{
if (rename_errno < 0)
rename_errno = (renameatu (AT_FDCWD, src_name, dst_dirfd, drelname,
- RENAME_NOREPLACE)
+ (x->exchange
+ ? RENAME_EXCHANGE : RENAME_NOREPLACE))
? errno : 0);
- nonexistent_dst = *rename_succeeded = rename_errno == 0;
+ *rename_succeeded = rename_errno == 0;
+ nonexistent_dst = *rename_succeeded && !x->exchange;
}
if (rename_errno == 0
@@ -2246,7 +2248,7 @@ copy_internal (char const *src_name, char const *dst_name,
src_mode = src_sb.st_mode;
- if (S_ISDIR (src_mode) && !x->recursive)
+ if (S_ISDIR (src_mode) && !x->recursive && !x->exchange)
{
error (0, 0, ! x->install_mode /* cp */
? _("-r not specified; omitting directory %s")
@@ -2289,7 +2291,7 @@ copy_internal (char const *src_name, char const *dst_name,
treated the same as nonexistent files. */
bool new_dst = 0 < nonexistent_dst;
- if (! new_dst)
+ if (! new_dst && ! x->exchange)
{
/* Normally, fill in DST_SB or set NEW_DST so that later code
can use DST_SB if NEW_DST is false. However, don't bother
@@ -2657,7 +2659,7 @@ skip:
Also, with --recursive, record dev/ino of each command-line directory.
We'll use that info to detect this problem: cp -R dir dir. */
- if (rename_errno == 0)
+ if (rename_errno == 0 || x->exchange)
earlier_file = nullptr;
else if (x->recursive && S_ISDIR (src_mode))
{
@@ -2752,7 +2754,7 @@ skip:
if (x->move_mode)
{
- if (rename_errno == EEXIST)
+ if (rename_errno == EEXIST && !x->exchange)
rename_errno = (renameat (AT_FDCWD, src_name, dst_dirfd, drelname) == 0
? 0 : errno);
@@ -2781,7 +2783,7 @@ skip:
_destination_ dev/ino, since the rename above can't have
changed those, and 'mv' always uses lstat.
We could limit it further by operating
- only on non-directories. */
+ only on non-directories when !x->exchange. */
record_file (x->dest_info, dst_relname, &src_sb);
}
@@ -2828,7 +2830,7 @@ skip:
where you'd replace '18' with the integer in parentheses that
was output from the perl one-liner above.
If necessary, of course, change '/tmp' to some other directory. */
- if (rename_errno != EXDEV || x->no_copy)
+ if (rename_errno != EXDEV || x->no_copy || x->exchange)
{
/* There are many ways this can happen due to a race condition.
When something happens between the initial follow_fstatat and the
@@ -2841,25 +2843,29 @@ skip:
destination file are made too restrictive, the rename will
fail. Etc. */
char const *quoted_dst_name = quoteaf_n (1, dst_name);
- switch (rename_errno)
- {
- case EDQUOT: case EEXIST: case EISDIR: case EMLINK:
- case ENOSPC: case ETXTBSY:
+ if (x->exchange)
+ error (0, rename_errno, _("cannot exchange %s and %s"),
+ quoteaf_n (0, src_name), quoted_dst_name);
+ else
+ switch (rename_errno)
+ {
+ case EDQUOT: case EEXIST: case EISDIR: case EMLINK:
+ case ENOSPC: case ETXTBSY:
#if ENOTEMPTY != EEXIST
- case ENOTEMPTY:
+ case ENOTEMPTY:
#endif
- /* The destination must be the problem. Don't mention
- the source as that is more likely to confuse the user
- than be helpful. */
- error (0, rename_errno, _("cannot overwrite %s"),
- quoted_dst_name);
- break;
+ /* The destination must be the problem. Don't mention
+ the source as that is more likely to confuse the user
+ than be helpful. */
+ error (0, rename_errno, _("cannot overwrite %s"),
+ quoted_dst_name);
+ break;
- default:
- error (0, rename_errno, _("cannot move %s to %s"),
- quoteaf_n (0, src_name), quoted_dst_name);
- break;
- }
+ default:
+ error (0, rename_errno, _("cannot move %s to %s"),
+ quoteaf_n (0, src_name), quoted_dst_name);
+ break;
+ }
forget_created (src_sb.st_ino, src_sb.st_dev);
return false;
}
diff --git a/src/copy.h b/src/copy.h
index dfa9435b3..ab89c75fd 100644
--- a/src/copy.h
+++ b/src/copy.h
@@ -155,6 +155,10 @@ struct cp_options
If that fails and NO_COPY, fail instead of copying. */
bool move_mode, no_copy;
+ /* Exchange instead of renaming. Valid only if MOVE_MODE and if
+ BACKUP_TYPE == no_backups. */
+ bool exchange;
+
/* If true, install(1) is the caller. */
bool install_mode;
diff --git a/src/mv.c b/src/mv.c
index 9dc40fe3e..692943a70 100644
--- a/src/mv.c
+++ b/src/mv.c
@@ -48,6 +48,7 @@
enum
{
DEBUG_OPTION = CHAR_MAX + 1,
+ EXCHANGE_OPTION,
NO_COPY_OPTION,
STRIP_TRAILING_SLASHES_OPTION
};
@@ -67,6 +68,7 @@ static struct option const long_options[] =
{"backup", optional_argument, nullptr, 'b'},
{"context", no_argument, nullptr, 'Z'},
{"debug", no_argument, nullptr, DEBUG_OPTION},
+ {"exchange", no_argument, nullptr, EXCHANGE_OPTION},
{"force", no_argument, nullptr, 'f'},
{"interactive", no_argument, nullptr, 'i'},
{"no-clobber", no_argument, nullptr, 'n'}, /* Deprecated. */
@@ -271,6 +273,9 @@ Rename SOURCE to DEST, or move SOURCE(s) to DIRECTORY.\n\
"), stdout);
fputs (_("\
--debug explain how a file is copied. Implies -v\n\
+"), stdout);
+ fputs (_("\
+ --exchange exchange source and destination\n\
"), stdout);
fputs (_("\
-f, --force do not prompt before overwriting\n\
@@ -361,6 +366,9 @@ main (int argc, char **argv)
case DEBUG_OPTION:
x.debug = x.verbose = true;
break;
+ case EXCHANGE_OPTION:
+ x.exchange = true;
+ break;
case NO_COPY_OPTION:
x.no_copy = true;
break;
@@ -469,7 +477,7 @@ main (int argc, char **argv)
else
{
char const *lastfile = file[n_files - 1];
- if (n_files == 2)
+ if (n_files == 2 && !x.exchange)
x.rename_errno = (renameatu (AT_FDCWD, file[0], AT_FDCWD, lastfile,
RENAME_NOREPLACE)
? errno : 0);
@@ -514,11 +522,13 @@ main (int argc, char **argv)
strip_trailing_slashes (file[i]);
if (make_backups
- && (x.interactive == I_ALWAYS_SKIP
+ && (x.exchange
+ || x.interactive == I_ALWAYS_SKIP
|| x.interactive == I_ALWAYS_NO))
{
error (0, 0,
- _("--backup is mutually exclusive with -n or --update=none-fail"));
+ _("cannot combine --backup with "
+ "--exchange, -n, or --update=none-fail"));
usage (EXIT_FAILURE);
}
diff --git a/tests/local.mk b/tests/local.mk
index 7cd1ef7b5..f0ac0386f 100644
--- a/tests/local.mk
+++ b/tests/local.mk
@@ -698,6 +698,7 @@ all_tests = \
tests/mv/into-self-3.sh \
tests/mv/into-self-4.sh \
tests/mv/leak-fd.sh \
+ tests/mv/mv-exchange.sh \
tests/mv/mv-n.sh \
tests/mv/mv-special-1.sh \
tests/mv/no-copy.sh \
diff --git a/tests/mv/mv-exchange.sh b/tests/mv/mv-exchange.sh
new file mode 100755
index 000000000..485403a1d
--- /dev/null
+++ b/tests/mv/mv-exchange.sh
@@ -0,0 +1,41 @@
+#!/bin/sh
+# Test mv --exchange.
+
+# Copyright (C) 2024 Free Software Foundation, Inc.
+
+# This program is free software: you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+
+# You should have received a copy of the GNU General Public License
+# along with this program. If not, see <https://www.gnu.org/licenses/>.
+
+. "${srcdir=.}/tests/init.sh"; path_prepend_ ./src
+print_ver_ mv
+
+
+# Test exchanging files.
+touch a || framework_failure_
+mkdir b || framework_failure_
+if ! mv -T --exchange a b 2>exchange_err; then
+ grep 'not supported' exchange_err || { cat exchange_err; fail=1; }
+else
+ test -d a || fail=1
+ test -f b || fail=1
+fi
+
+# Test wrong number of arguments.
+touch c || framework_failure_
+returns_ 1 mv --exchange a 2>/dev/null || fail=1
+returns_ 1 mv --exchange a b c 2>/dev/null || fail=1
+
+# Both files must exist.
+returns_ 1 mv --exchange a d 2>/dev/null || fail=1
+
+Exit $fail
--
2.40.1
^ permalink raw reply related
* [PATCH RFC] coresched: Manage core scheduling cookies for tasks
From: Thijs Raymakers @ 2024-03-16 17:10 UTC (permalink / raw)
To: util-linux; +Cc: Thijs Raymakers
Hi all,
I'm looking for any comments on coresched, a program that allows you to
manage core scheduling cookies for tasks.
=== What is Core Scheduling ===
Core Scheduling can be used to ensure that certain tasks will never be
scheduled on the same physical core. This can be a useful, alternative,
mitigation to hardware vulnerabilities like L1tf or MDS.
The full software mitigation for these vulnerabilities would be to disable
SMT/Hyper-Threading. However, this can be prohibitively expensive and
therefore often not done in practice.
With Core Scheduling you can mitigate in these issues in some scenarios,
while keeping SMT enabled.
Core Scheduling works by adding a random "cookie" to a process. Only
processes with the same core scheduling cookie are allowed to run on
sibling cores. Tasks that trust each other can be given the same
cookie and untrusted tasks are given a different cookie.
This is important when running VMs that don't trust each other, as
it prevents a guest VM to leak data from another guest VM with L1tf or MDS.
=== Motivation ===
The kernel exposes a prctl uapi to manage core scheduling cookies (see
https://www.kernel.org/doc/html/latest/admin-guide/hw-vuln/core-scheduling.html)
Last week, I wanted to use core scheduling on some programs. Adding the
prctl calls and recompiling felt a bit inconvenient, so I looked for a
program like taskset that could do the job without having to modify the
target program. I couldn't find any, and so I wrote a small program that
does this. Hopefully it saves the next person some time :)
=== RFC ===
I'm looking forward to any comments that you might have on the patch!
Please note that I haven't written the manpage and the bash completion
script yet. I first wanted to get some feedback on the program before I
start documenting it in more detail.
I'm particularly curious about your thoughts on the following things:
- General comments about interacting with the program: Do the options
make sense? Are there any necessary functions missing? Are the error
messages helpful? Is the output too verbose/not verbose enough?
- How should the program behave if the prctl core scheduling API is not
available? It has been in Linus' tree since november 2021
(commit a41b74451b35f7a6529689760eb8c05241feecbc) but it can be
disabled with CONFIG_SCHED_CORE=n
- Most of the options require the user to have the CAP_SYS_PTRACE
capability. Should the program notify the user that the capability
is missing when the prctl call returns -EPREM, or does a mention in the
man page suffice?
- I've currently licensed it under the EUPL v1.2, which is easier to
enforce in my jurisdiction than the GPL. It is GPL compatible so it
shouldn't be an issue, but if anybody has any remarks on this, please
let me know.
Thanks for taking the time!
Best regards,
Thijs Raymakers
Signed-off-by: Thijs Raymakers <thijs@raymakers.nl>
---
.gitignore | 1 +
bash-completion/coresched | 0
configure.ac | 12 +-
meson.build | 16 +-
meson_options.txt | 2 +-
schedutils/Makemodule.am | 8 +
schedutils/coresched.1.adoc | 16 ++
schedutils/coresched.c | 340 ++++++++++++++++++++++++++++++++++++
8 files changed, 389 insertions(+), 6 deletions(-)
create mode 100644 bash-completion/coresched
create mode 100644 schedutils/coresched.1.adoc
create mode 100644 schedutils/coresched.c
diff --git a/.gitignore b/.gitignore
index 6ecbfa7fe..316f3cdcc 100644
--- a/.gitignore
+++ b/.gitignore
@@ -94,6 +94,7 @@ ylwrap
/colcrt
/colrm
/column
+/coresched
/ctrlaltdel
/delpart
/dmesg
diff --git a/bash-completion/coresched b/bash-completion/coresched
new file mode 100644
index 000000000..e69de29bb
diff --git a/configure.ac b/configure.ac
index ab7c98636..3a189a075 100644
--- a/configure.ac
+++ b/configure.ac
@@ -2500,9 +2500,9 @@ UL_REQUIRES_HAVE([setterm], [ncursesw, ncurses], [ncursesw or ncurses library])
AM_CONDITIONAL([BUILD_SETTERM], [test "x$build_setterm" = xyes])
# build_schedutils= is just configure-only variable to control
-# ionice, taskset and chrt
+# ionice, taskset, coresched and chrt
AC_ARG_ENABLE([schedutils],
- AS_HELP_STRING([--disable-schedutils], [do not build chrt, ionice, taskset]),
+ AS_HELP_STRING([--disable-schedutils], [do not build chrt, ionice, taskset, coresched]),
[], [UL_DEFAULT_ENABLE([schedutils], [check])]
)
@@ -2545,6 +2545,14 @@ UL_REQUIRES_SYSCALL_CHECK([taskset],
AM_CONDITIONAL([BUILD_TASKSET], [test "x$build_taskset" = xyes])
+UL_ENABLE_ALIAS([coresched], [schedutils])
+UL_BUILD_INIT([coresched])
+UL_REQUIRES_SYSCALL_CHECK([coresched],
+ [UL_CHECK_SYSCALL([prctl])],
+ [prctl])
+AM_CONDITIONAL([BUILD_CORESCHED], [test "x$build_coresched" = xyes])
+
+
have_schedsetter=no
AS_IF([test "x$ac_cv_func_sched_setscheduler" = xyes], [have_schedsetter=yes],
[test "x$ac_cv_func_sched_setattr" = xyes], [have_schedsetter=yes])
diff --git a/meson.build b/meson.build
index f7baab7a2..8244c43a9 100644
--- a/meson.build
+++ b/meson.build
@@ -3107,13 +3107,23 @@ exe4 = executable(
install : opt,
build_by_default : opt)
+exe5 = executable(
+ 'coresched',
+ 'schedutils/coresched.c',
+ include_directories : includes,
+ link_with : lib_common,
+ install_dir : usrbin_exec_dir,
+ install : opt,
+ build_by_default : opt)
+
if opt and not is_disabler(exe)
- exes += [exe, exe2, exe3, exe4]
+ exes += [exe, exe2, exe3, exe4, exe5]
manadocs += ['schedutils/chrt.1.adoc',
'schedutils/ionice.1.adoc',
'schedutils/taskset.1.adoc',
- 'schedutils/uclampset.1.adoc']
- bashcompletions += ['chrt', 'ionice', 'taskset', 'uclampset']
+ 'schedutils/uclampset.1.adoc',
+ 'schedutils/coresched.1.adoc']
+ bashcompletions += ['chrt', 'ionice', 'taskset', 'uclampset', 'coresched']
endif
############################################################
diff --git a/meson_options.txt b/meson_options.txt
index 7b8cf3f35..3405c1b73 100644
--- a/meson_options.txt
+++ b/meson_options.txt
@@ -162,7 +162,7 @@ option('build-pipesz', type : 'feature',
option('build-setterm', type : 'feature',
description : 'build setterm')
option('build-schedutils', type : 'feature',
- description : 'build chrt, ionice, taskset')
+ description : 'build chrt, ionice, taskset, coresched')
option('build-wall', type : 'feature',
description : 'build wall')
option('build-write', type : 'feature',
diff --git a/schedutils/Makemodule.am b/schedutils/Makemodule.am
index 1040da85f..0cb655401 100644
--- a/schedutils/Makemodule.am
+++ b/schedutils/Makemodule.am
@@ -29,3 +29,11 @@ dist_noinst_DATA += schedutils/uclampset.1.adoc
uclampset_SOURCES = schedutils/uclampset.c schedutils/sched_attr.h
uclampset_LDADD = $(LDADD) libcommon.la
endif
+
+if BUILD_CORESCHED
+usrbin_exec_PROGRAMS += coresched
+MANPAGES += schedutils/coresched.1
+dist_noinst_DATA += schedutils/coresched.1.adoc
+coresched_SOURCES = schedutils/coresched.c
+coresched_LDADD = $(LDADD) libcommon.la
+endif
diff --git a/schedutils/coresched.1.adoc b/schedutils/coresched.1.adoc
new file mode 100644
index 000000000..60a21cd01
--- /dev/null
+++ b/schedutils/coresched.1.adoc
@@ -0,0 +1,16 @@
+//po4a: entry man manual
+////
+coresched(1) manpage
+////
+= coresched(1)
+:doctype: manpage
+:man manual: User Commands
+:man source: util-linux {release-version}
+:page-layout: base
+:command: coresched
+:colon: :
+:copyright: ©
+
+== NAME
+
+coresched - manage core scheduling cookies for tasks
diff --git a/schedutils/coresched.c b/schedutils/coresched.c
new file mode 100644
index 000000000..4be8f9fda
--- /dev/null
+++ b/schedutils/coresched.c
@@ -0,0 +1,340 @@
+/**
+ * SPDX-License-Identifier: EUPL-1.2
+ *
+ * coresched.c - manage core scheduling cookies for tasks
+ *
+ * Copyright (C) 2024 Thijs Raymakers
+ * Licensed under the EUPL v1.2
+ */
+
+#include <getopt.h>
+#include <stdbool.h>
+#include <stdio.h>
+#include <sys/prctl.h>
+#include <sys/wait.h>
+
+#include "c.h"
+#include "closestream.h"
+#include "nls.h"
+#include "strutils.h"
+
+typedef enum {
+ SCHED_CORE_SCOPE_PID = PR_SCHED_CORE_SCOPE_THREAD,
+ SCHED_CORE_SCOPE_TGID = PR_SCHED_CORE_SCOPE_THREAD_GROUP,
+ SCHED_CORE_SCOPE_PGID = PR_SCHED_CORE_SCOPE_PROCESS_GROUP,
+} core_sched_type_t;
+
+typedef enum {
+ SCHED_CORE_CMD_NONE = 0,
+ SCHED_CORE_CMD_GET = 1,
+ SCHED_CORE_CMD_CREATE = 2,
+ SCHED_CORE_CMD_COPY = 4,
+ SCHED_CORE_CMD_EXEC = 8,
+} core_sched_cmd_t;
+
+struct args {
+ pid_t from_pid;
+ pid_t to_pid;
+ core_sched_type_t type;
+ core_sched_cmd_t cmd;
+ int exec_argv_offset;
+};
+
+unsigned long core_sched_get_cookie(struct args *args);
+void core_sched_create_cookie(struct args *args);
+void core_sched_pull_cookie(pid_t from);
+void core_sched_push_cookie(pid_t to, core_sched_type_t type);
+void core_sched_copy_cookie(struct args *args);
+void core_sched_exec_with_cookie(struct args *args, char **argv);
+
+core_sched_type_t parse_core_sched_type(char *str);
+bool verify_arguments(struct args *args);
+void parse_arguments(int argc, char **argv, struct args *args);
+
+unsigned long core_sched_get_cookie(struct args *args)
+{
+ unsigned long cookie = 0;
+ int prctl_errno = prctl(PR_SCHED_CORE, PR_SCHED_CORE_GET,
+ args->from_pid, SCHED_CORE_SCOPE_PID, &cookie);
+ if (prctl_errno) {
+ errx(-prctl_errno, "Failed to get cookie from PID %d",
+ args->from_pid);
+ }
+ return cookie;
+}
+
+void core_sched_create_cookie(struct args *args)
+{
+ int prctl_errno = prctl(PR_SCHED_CORE, PR_SCHED_CORE_CREATE,
+ args->from_pid, args->type, 0);
+ if (prctl_errno) {
+ errx(-prctl_errno, "Failed to create cookie for PID %d",
+ args->from_pid);
+ }
+}
+
+void core_sched_pull_cookie(pid_t from)
+{
+ int prctl_errno = prctl(PR_SCHED_CORE, PR_SCHED_CORE_SHARE_FROM, from,
+ SCHED_CORE_SCOPE_PID, 0);
+ if (prctl_errno) {
+ errx(-prctl_errno, "Failed to pull cookie from PID %d", from);
+ }
+}
+
+void core_sched_push_cookie(pid_t to, core_sched_type_t type)
+{
+ int prctl_errno =
+ prctl(PR_SCHED_CORE, PR_SCHED_CORE_SHARE_TO, to, type, 0);
+ if (prctl_errno) {
+ errx(-prctl_errno, "Failed to push cookie to PID %d", to);
+ }
+}
+
+void core_sched_copy_cookie(struct args *args)
+{
+ core_sched_pull_cookie(args->from_pid);
+ core_sched_push_cookie(args->to_pid, args->type);
+}
+
+void core_sched_exec_with_cookie(struct args *args, char **argv)
+{
+ if (!args->exec_argv_offset) {
+ errx(EINVAL, "when --exec is provided, a program name "
+ "has to be given.");
+ }
+
+ // Move the argument list to the first argument of the program
+ argv = &argv[args->exec_argv_offset];
+
+ pid_t pid = fork();
+ if (pid == -1) {
+ errx(errno, "Failed to spawn new process");
+ }
+
+ if (!pid) {
+ // If a source PID is provided, try to copy the cookie from
+ // that PID. Otherwise, create a brand new cookie with the
+ // provided type.
+ if (args->from_pid) {
+ core_sched_pull_cookie(args->from_pid);
+ } else {
+ args->from_pid = getpid();
+ core_sched_create_cookie(args);
+ }
+ if (execvp(argv[0], argv)) {
+ errexec(argv[0]);
+ }
+ } else {
+ int status = 0;
+ waitpid(pid, &status, 0);
+ exit(status);
+ }
+}
+
+core_sched_type_t parse_core_sched_type(char *str)
+{
+ if (!strncmp(str, "pid\0", 4)) {
+ return SCHED_CORE_SCOPE_PID;
+ } else if (!strncmp(str, "tgid\0", 5)) {
+ return SCHED_CORE_SCOPE_TGID;
+ } else if (!strncmp(str, "pgid\0", 5)) {
+ return SCHED_CORE_SCOPE_PGID;
+ }
+
+ errx(EINVAL, "'%s' is an invalid option. Must be one of pid/tgid/pgid",
+ str);
+ __builtin_unreachable();
+}
+
+static void __attribute__((__noreturn__)) usage(void)
+{
+ fputs(USAGE_HEADER, stdout);
+ fprintf(stdout, _(" %s --get <PID>\n"), program_invocation_short_name);
+ fprintf(stdout, _(" %s --new <PID> [-t <TYPE>]\n"),
+ program_invocation_short_name);
+ fprintf(stdout, _(" %s --copy -s <PID> -d <PID> [-t <TYPE>]\n"),
+ program_invocation_short_name);
+ fprintf(stdout, _(" %s --exec [-s <PID>] -- PROGRAM ARGS... \n"),
+ program_invocation_short_name);
+
+ fputs(USAGE_SEPARATOR, stdout);
+ fputsln(_("Manage core scheduling cookies for tasks."), stdout);
+
+ fputs(USAGE_FUNCTIONS, stdout);
+ fputsln(_(" -g, --get <PID> get the core scheduling cookie of a PID"),
+ stdout);
+ fputsln(_(" -n, --new <PID> assign a new core scheduling cookie to PID"),
+ stdout);
+ fputsln(_(" -c, --copy copy the core scheduling cookie from PID to\n"
+ " another PID, requires the --source and --dest option"),
+ stdout);
+ fputsln(_(" -e, --exec execute a program with a new core scheduling\n"
+ " cookie."),
+ stdout);
+
+ fputs(USAGE_OPTIONS, stdout);
+ fputsln(_(" -s, --source <PID> where to copy the core scheduling cookie from."),
+ stdout);
+ fputsln(_(" -d, --dest <PID> where to copy the core scheduling cookie to."),
+ stdout);
+ fputsln(_(" -t, --type type of the destination PID, or the type of\n"
+ " the PID when a new core scheduling cookie\n"
+ " is created. Can be one of the following:\n"
+ " pid, tgid or pgid. Defaults to tgid."),
+ stdout);
+ fputs(USAGE_SEPARATOR, stdout);
+ fprintf(stdout,
+ USAGE_HELP_OPTIONS(
+ 25)); /* char offset to align option descriptions */
+ fprintf(stdout, USAGE_MAN_TAIL("coresched(1)"));
+ exit(EXIT_SUCCESS);
+}
+
+bool verify_arguments(struct args *args)
+{
+ if (args->cmd == SCHED_CORE_CMD_NONE) {
+ usage();
+ }
+
+ // Check if the value of args->cmd is a power of 2
+ // In that case, only a single function option was set.
+ if (!(args->cmd && !(args->cmd & (args->cmd - 1)))) {
+ errx(EINVAL, "Cannot do more than one function at a time.");
+ }
+
+ if (args->from_pid < 0) {
+ errx(EINVAL, "source PID cannot be negative");
+ }
+
+ if (args->to_pid < 0) {
+ errx(EINVAL, "destination PID cannot be negative");
+ }
+
+ if (args->from_pid == 0 && args->cmd == SCHED_CORE_CMD_COPY) {
+ errx(EINVAL, "valid argument to --source is required");
+ }
+
+ if (args->to_pid == 0 && args->cmd == SCHED_CORE_CMD_COPY) {
+ errx(EINVAL, "valid argument to --dest is required");
+ }
+
+ if (args->from_pid == 0 && args->cmd != SCHED_CORE_CMD_EXEC) {
+ errx(EINVAL, "PID cannot be zero");
+ }
+
+ return true;
+}
+
+void parse_arguments(int argc, char **argv, struct args *args)
+{
+ int c;
+
+ enum {
+ OPT_GET = 'g',
+ OPT_NEW = 'n',
+ OPT_COPY = 'c',
+ OPT_EXEC = 'e',
+ OPT_SRC = 's',
+ OPT_DEST = 'd',
+ OPT_TYPE = 't',
+ OPT_VERSION = 'V',
+ OPT_HELP = 'h'
+ };
+
+ static const struct option longopts[] = {
+ { "get", required_argument, NULL, OPT_GET },
+ { "new", required_argument, NULL, OPT_NEW },
+ { "copy", no_argument, NULL, OPT_COPY },
+ { "exec", no_argument, NULL, OPT_EXEC },
+ { "source", required_argument, NULL, OPT_SRC },
+ { "destination", required_argument, NULL, OPT_DEST },
+ { "type", required_argument, NULL, OPT_TYPE },
+ { "version", no_argument, NULL, OPT_VERSION },
+ { "help", no_argument, NULL, OPT_HELP },
+ { NULL, 0, NULL, 0 }
+ };
+
+ while ((c = getopt_long(argc, argv, "g:n:ces:d:t:Vh", longopts,
+ NULL)) != -1)
+ switch (c) {
+ case OPT_GET:
+ args->cmd |= SCHED_CORE_CMD_GET;
+ args->from_pid = strtos32_or_err(
+ optarg, "Failed to parse PID for --get");
+ break;
+ case OPT_NEW:
+ args->cmd |= SCHED_CORE_CMD_CREATE;
+ args->from_pid = strtos32_or_err(
+ optarg, "Failed to parse PID for --new");
+ break;
+ case OPT_COPY:
+ args->cmd |= SCHED_CORE_CMD_COPY;
+ break;
+ case OPT_EXEC:
+ args->cmd |= SCHED_CORE_CMD_EXEC;
+ break;
+ case OPT_SRC:
+ args->from_pid = strtos32_or_err(
+ optarg, "Failed to parse PID for --source");
+ break;
+ case OPT_DEST:
+ args->to_pid = strtos32_or_err(
+ optarg, "Failed to parse PID for --dest");
+ break;
+ case OPT_TYPE:
+ args->type = parse_core_sched_type(optarg);
+ break;
+ case OPT_VERSION:
+ print_version(EXIT_SUCCESS);
+ case OPT_HELP:
+ usage();
+ default:
+ errtryhelp(EXIT_FAILURE);
+ }
+
+ if (argc > optind) {
+ args->exec_argv_offset = optind;
+ }
+ verify_arguments(args);
+}
+
+int main(int argc, char **argv)
+{
+ struct args arguments = { 0 };
+ arguments.type = SCHED_CORE_SCOPE_TGID;
+
+ setlocale(LC_ALL, "");
+ bindtextdomain(PACKAGE, LOCALEDIR);
+ textdomain(PACKAGE);
+ close_stdout_atexit();
+
+ parse_arguments(argc, argv, &arguments);
+
+ unsigned long cookie = 0;
+ switch (arguments.cmd) {
+ case SCHED_CORE_CMD_GET:
+ cookie = core_sched_get_cookie(&arguments);
+ if (cookie) {
+ printf("core scheduling cookie of pid %d is 0x%lx\n",
+ arguments.from_pid, cookie);
+ } else {
+ printf("pid %d doesn't have a core scheduling cookie\n",
+ arguments.from_pid);
+ exit(1);
+ }
+ break;
+ case SCHED_CORE_CMD_CREATE:
+ core_sched_create_cookie(&arguments);
+ break;
+ case SCHED_CORE_CMD_COPY:
+ core_sched_copy_cookie(&arguments);
+ break;
+ case SCHED_CORE_CMD_EXEC:
+ core_sched_exec_with_cookie(&arguments, argv);
+ break;
+ default:
+ usage();
+ exit(1);
+ }
+}
--
2.44.0
^ permalink raw reply related
* Re: bug#69532: mv's new -x option should be made orthogonal to -t/-T/default
From: Masatake YAMATO @ 2024-03-05 22:13 UTC (permalink / raw)
To: P; +Cc: eggert, asmadeus, 69532, oss, rob, util-linux
In-Reply-To: <636f1247-0de0-2f32-cb04-f6952b6807aa@draigBrady.com>
When I knew RENAME_EXCHANGE, I thought we should extend mv command as
you did: adding --swap. However, after researching the past
challenges, I decided not to propose the feature to coreutils.
https://www.gnu.org/software/coreutils/rejected_requests.html
https://lists.gnu.org/archive/html/coreutils/2018-12/msg00004.html
https://www.mail-archive.com/coreutils@gnu.org/msg10276.html
Masatake YAMATO
> On 05/03/2024 04:10, Paul Eggert wrote:
>> On 3/4/24 16:43, Dominique Martinet wrote:
>>> Adding Rob to the loop because this impacts compatibility with
>>> toybox/maybe busybox implementations
>> Busybox does not use RENAME_EXCHANGE, so this isn't a Busybox issue.
>> Toybox mv added -x to its development version yesterday:
>> https://github.com/landley/toybox/commit/a2419ad52d489bf1a84a9f3aa73afb351642c765
>> so there's little prior art there, and there's still plenty of time to
>> fix its problems before exposing it to the world.
>>
>>> I also see --swap mostly used by scripts and this actually feels a bit
>>> dangerous to me -- I'd *always* use this with -T.
>> Yes, it's a problem.
>> By "see --swap mostly used by scripts" I assume you mean scripts that
>> haven't been written yet, assuming that nobody had -x until
>> yesterday....
>>
>>> (by the way, what's this "rename" command you speak of?
>> https://mirrors.edge.kernel.org/pub/linux/utils/util-linux/
>> Now that I've looked into it further, util-linux already has an "exch"
>> command that does exactly what you want. This is the command that
>> toybox
>> should implement rather than try to simulate it with "mv -x" (which
>> causes all sorts of problems).
>> That is, toybox should revert yesterday's change to "mv", and should
>> implement "exch" instead.
>
> I think having the functionality in mv(1) is better than in rename(1),
> but since exch(1) is already released that's probably
> the best place for this functionality now.
>
> A separate exch command may be overkill for just this,
> but perhaps related functionality might be added to that command in
> future.
> For e.g. some of the discussed functionality for a "replace" command
> might reside there.
>
> So I think I'll remove the as yet unreleased mv --swap from coreutils,
> given that
> util-linux is as widely available as coreutils on GNU/Linux platforms.
>
> cheers,
> Pádraig
>
>
^ permalink raw reply
* Re: bug#69532: mv's new -x option should be made orthogonal to -t/-T/default
From: Karel Zak @ 2024-03-05 20:41 UTC (permalink / raw)
To: Pádraig Brady
Cc: Paul Eggert, Dominique Martinet, 69532, Petr Malat, Rob Landley,
util-linux
In-Reply-To: <636f1247-0de0-2f32-cb04-f6952b6807aa@draigBrady.com>
On Tue, Mar 05, 2024 at 02:16:05PM +0000, Pádraig Brady wrote:
> I think having the functionality in mv(1) is better than in rename(1),
> but since exch(1) is already released that's probably
> the best place for this functionality now.
>
> A separate exch command may be overkill for just this,
rename(1) was also my initial idea, but it's too complex and rarely used
by users for simple tasks like those we can now achieve with the new simple
command exch(1).
> but perhaps related functionality might be added to that command in future.
> For e.g. some of the discussed functionality for a "replace" command
> might reside there.
>
> So I think I'll remove the as yet unreleased mv --swap from coreutils, given that
> util-linux is as widely available as coreutils on GNU/Linux platforms.
Yes, it seems better to have this Linux-specific feature in
util-linux. We should discuss such changes early next time ;-)
Thanks for CC:
Karel
--
Karel Zak <kzak@redhat.com>
http://karelzak.blogspot.com
^ permalink raw reply
* Re: bug#69532: mv's new -x option should be made orthogonal to -t/-T/default
From: Pádraig Brady @ 2024-03-05 14:16 UTC (permalink / raw)
To: Paul Eggert, Dominique Martinet
Cc: 69532, Petr Malat, Rob Landley, util-linux
In-Reply-To: <5914e8b2-48ac-456b-9753-6a7bae7a9bbb@cs.ucla.edu>
On 05/03/2024 04:10, Paul Eggert wrote:
> On 3/4/24 16:43, Dominique Martinet wrote:
>> Adding Rob to the loop because this impacts compatibility with
>> toybox/maybe busybox implementations
>
> Busybox does not use RENAME_EXCHANGE, so this isn't a Busybox issue.
>
> Toybox mv added -x to its development version yesterday:
>
> https://github.com/landley/toybox/commit/a2419ad52d489bf1a84a9f3aa73afb351642c765
>
> so there's little prior art there, and there's still plenty of time to
> fix its problems before exposing it to the world.
>
>
>> I also see --swap mostly used by scripts and this actually feels a bit
>> dangerous to me -- I'd *always* use this with -T.
>
> Yes, it's a problem.
>
> By "see --swap mostly used by scripts" I assume you mean scripts that
> haven't been written yet, assuming that nobody had -x until yesterday....
>
>
>> (by the way, what's this "rename" command you speak of?
>
> https://mirrors.edge.kernel.org/pub/linux/utils/util-linux/
>
> Now that I've looked into it further, util-linux already has an "exch"
> command that does exactly what you want. This is the command that toybox
> should implement rather than try to simulate it with "mv -x" (which
> causes all sorts of problems).
>
> That is, toybox should revert yesterday's change to "mv", and should
> implement "exch" instead.
I think having the functionality in mv(1) is better than in rename(1),
but since exch(1) is already released that's probably
the best place for this functionality now.
A separate exch command may be overkill for just this,
but perhaps related functionality might be added to that command in future.
For e.g. some of the discussed functionality for a "replace" command
might reside there.
So I think I'll remove the as yet unreleased mv --swap from coreutils, given that
util-linux is as widely available as coreutils on GNU/Linux platforms.
cheers,
Pádraig
^ permalink raw reply
* Re: [PATCH] su: fix use after free in run_shell
From: Karel Zak @ 2024-03-05 9:15 UTC (permalink / raw)
To: Tanish Yadav; +Cc: util-linux
In-Reply-To: <479fd0b8-bf67-4370-8250-bfd136a08195@gmail.com>
On Tue, Mar 05, 2024 at 12:51:41AM +0530, Tanish Yadav wrote:
> login-utils/su-common.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
Applied, thanks.
--
Karel Zak <kzak@redhat.com>
http://karelzak.blogspot.com
^ permalink raw reply
* [PATCH] su: fix use after free in run_shell
From: Tanish Yadav @ 2024-03-04 19:21 UTC (permalink / raw)
To: util-linux
Do not free tmp for non login branch as basename may return a pointer to
some part of it.
Signed-off-by: Tanish Yadav <devtany@gmail.com>
---
login-utils/su-common.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/login-utils/su-common.c b/login-utils/su-common.c
index 242b6ce4e..8cb54e1c1 100644
--- a/login-utils/su-common.c
+++ b/login-utils/su-common.c
@@ -851,10 +851,10 @@ static void run_shell(
arg0[0] = '-';
strcpy(arg0 + 1, shell_basename);
args[0] = arg0;
+ free(tmp);
} else {
- args[0] = basename(tmp);
- }
- free(tmp);
+ args[0] = basename(tmp);
+ }
if (su->fast_startup)
args[argno++] = "-f";
--
2.44.0
^ permalink raw reply related
* Re: Escape sequences in /var/log/auth.log
From: Alejandro Colomar @ 2024-03-04 13:59 UTC (permalink / raw)
To: Karel Zak, Skyler Ferrante (RIT Student); +Cc: Serge E. Hallyn, util-linux
In-Reply-To: <20240304123359.ruh64pobvg53r7f7@ws.net.home>
[-- Attachment #1: Type: text/plain, Size: 696 bytes --]
[TO += Skyler]
Hi Karel, Skyler,
On Mon, Mar 04, 2024 at 01:33:59PM +0100, Karel Zak wrote:
> On Sun, Mar 03, 2024 at 11:59:51AM +0100, Alejandro Colomar wrote:
> > This seems to be a bug in util-linux, not shadow, so I've added
> > util-linux@ to the thread.
>
> Fixed. Thanks for your report.
Thank you.
Skyler, it's been fixed here:
<https://git.kernel.org/pub/scm/utils/util-linux/util-linux.git/commit/?id=677a3168b261f3289e282a02dfd85d7f37de0447>
Have a lovely day!
Alex
> Karel
>
>
> --
> Karel Zak <kzak@redhat.com>
> http://karelzak.blogspot.com
--
<https://www.alejandro-colomar.es/>
Looking for a remote C programming job at the moment.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: Finding a block device quickly with libblkid
From: Karel Zak @ 2024-03-04 13:11 UTC (permalink / raw)
To: Eric Curtin; +Cc: util-linux, systemd-devel, Eric Chanudet, Brian Masney
In-Reply-To: <CAOgh=FwPHHW7fyTZDF422+WLuXps_qNcTUDDQopAHT_dJNZ6Sw@mail.gmail.com>
On Fri, Mar 01, 2024 at 06:30:15PM +0000, Eric Curtin wrote:
> We are looking into optimizing the boot sequence of a device with many
> partitions.
Nice topic :-)
> On boot in the default systemd implementation, all the block devices
> are queried via libblkid and the various symlinks are set up in
> /dev/disk/* from the results of those queries. The problem is on a
> device with many partitions this can delay the boot by hundreds of
> milliseconds, which is not ideal, especially when in many cases all
> you really care about is mounting the block device that represents the
> rootfs partition.
It's a little bit more complex. It's not just about rootfs. The reason
why udevd scans all the devices is to create a udev database and fill
it with data usable for running the system. For example, when you type
'lsblk --fs', it reads the data from the database (findmnt, mount,
etc.), which is also used by udev rules and for dependency evaluations
in systemd, etc. This is the reason why the system gathers all the
data about a new device when it's detected.
If you want to avoid all of this, you need to customize udev rules
where you can filter out what and how to scan.
> We can sort of guess "/dev/sde38" is the correct
> one, but that's not deterministic.
>
> So we started digging and came across blkid_find_dev_with_tag and
> blkid_dev_devname, which you can call like this:
>
> blkid_dev_devname(blkid_find_dev_with_tag(cache, "PARTLABEL", "system_a")))
>
> blkid_dev_devname(blkid_find_dev_with_tag(cache, "PARTLABEL", "system_b")))
You're on the right track. PARTLABEL and PARTUUID are stored in the
partition table, so it's unnecessary to scan partitions for their
content (filesystems).
> On first glance this looks useful as you don't have to loop through
> all the devices to use.
>
> But this function only seems to work if the data is already cached, so
> it's not so useful on boot.
Yes, using blkid_dev_devname() is not advisable. It's part of the old
high-level libblkid API from the pre-udev era.
If you truly need to read data from the device, then utilizing the
low-level probing API is recommended. This can be done from the
command line with 'blkid -p', but you'll need to disable scanning for
all unwanted data (using '--usage no*'). For instance:
blkid -o udev -p --usages nofilesystem,raid,crypto,others /dev/sda1
This command will only return ID_PART_ENTRY_* data from the partition
table.
You can use the LIBBLKID_DEBUG=all environment variable to see
the library's operations.
The question arises whether using blkid is the ideal solution if you
only require PARTLABELs and PARTUUIDs. For example, sfdisk could be a
more efficient approach:
sfdisk -l /dev/sda -o+NAME,UUID
However, a potential issue is that sfdisk only provides the guessed
partition names (paths); the name used by the kernel might be different.
> Has anyone any ideas on how we can optimize the identification of a
> block device via UUID, LABEL, PARTUUID, PARTLABEL, etc.? Because the
> current implementations don't scale well when you have many block
> devices.
It depends on your goal. You can heavily customize your system to
speed up boot (all the necessary tools are available for this
purpose). However, the problem I see is the issue of portability and
the maintenance overhead.
Karel
--
Karel Zak <kzak@redhat.com>
http://karelzak.blogspot.com
^ permalink raw reply
* Re: Escape sequences in /var/log/auth.log
From: Karel Zak @ 2024-03-04 12:33 UTC (permalink / raw)
To: Alejandro Colomar; +Cc: Serge E. Hallyn, util-linux
In-Reply-To: <ZeRYJ6HD77humJzg@debian>
On Sun, Mar 03, 2024 at 11:59:51AM +0100, Alejandro Colomar wrote:
> This seems to be a bug in util-linux, not shadow, so I've added
> util-linux@ to the thread.
Fixed. Thanks for your report.
Karel
--
Karel Zak <kzak@redhat.com>
http://karelzak.blogspot.com
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox