Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix locale-specific time #15879

Merged
merged 1 commit into from
Apr 8, 2024
Merged

Fix locale-specific time #15879

merged 1 commit into from
Apr 8, 2024

Conversation

part1zano
Copy link
Contributor

@part1zano part1zano commented Feb 10, 2024 •
edited
Loading

Motivation and Context

Fix #15878

Description

See the issue for details

How Has This Been Tested?

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Performance enhancement (non-breaking change which improves efficiency)
  • Code cleanup (non-breaking change which makes code smaller or more readable)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Library ABI change (libzfs, libzfs_core, libnvpair, libuutil and libzfsbootenv)
  • Documentation (a change to man pages or other documentation)

Checklist:

@part1zano part1zano force-pushed the fix_locale_time branch 7 times, most recently from 9510f2a to c39011c Compare February 10, 2024 20:28
@behlendorf behlendorf added the Status: Code Review Needed Ready for review and testing label Feb 12, 2024
@part1zano part1zano force-pushed the fix_locale_time branch 3 times, most recently from 20b3bd3 to 9305448 Compare February 17, 2024 12:15
@part1zano part1zano force-pushed the fix_locale_time branch 4 times, most recently from 306a650 to 9649565 Compare February 27, 2024 14:59
@mcmilk
Copy link
Contributor

mcmilk commented Mar 4, 2024

Some small commit message and the Signed-off-by: Name <some-mail@example.com> would be nice.

@part1zano part1zano force-pushed the fix_locale_time branch 2 times, most recently from df9b923 to df6f351 Compare March 6, 2024 10:12
@part1zano part1zano force-pushed the fix_locale_time branch 3 times, most recently from 9c1d53c to 3570115 Compare March 19, 2024 16:43
@part1zano
Copy link
Contributor Author

Could someone (including, but not limited to @behlendorf ) please review this already?

@tonyhutter
Copy link
Contributor

@part1zano sorry for the lack of response. Your patch works for me:

Before:

$ LANG=ru_RU.UTF-8 ./zpool status -t
  pool: tank
 state: ONLINE
  scan: scrub repaired 0B in 00:00:00 with 0 errors on Thu Mar 28 15:03:39 2024
config:

	NAME          STATE     READ WRITE CKSUM
	tank          ONLINE       0     0     0
	  /tmp/file1  ONLINE       0     0     0  (100% trimmed, completed at Чт 28 мар 2024 15:02:54)
	  /tmp/file2  ONLINE       0     0     0  (100% trimmed, completed at Чт 28 мар 2024 15:02:54)
	  /tmp/file3  ONLINE       0     0     0  (100% trimmed, completed at Чт 28 мар 2024 15:02:54)
	  /tmp/file4  ONLINE       0     0     0  (100% trimmed, completed at Чт 28 мар 2024 15:02:54)

After

LANG=ru_RU.UTF-8 ./zpool status -t
  pool: tank
 state: ONLINE
  scan: scrub repaired 0B in 00:00:00 with 0 errors on Thu Mar 28 15:03:39 2024
config:

	NAME          STATE     READ WRITE CKSUM
	tank          ONLINE       0     0     0
	  /tmp/file1  ONLINE       0     0     0  (100% trimmed, completed at Thu Mar 28 15:02:54 2024)
	  /tmp/file2  ONLINE       0     0     0  (100% trimmed, completed at Thu Mar 28 15:02:54 2024)
	  /tmp/file3  ONLINE       0     0     0  (100% trimmed, completed at Thu Mar 28 15:02:54 2024)
	  /tmp/file4  ONLINE       0     0     0  (100% trimmed, completed at Thu Mar 28 15:02:54 2024)

@part1zano
Copy link
Contributor Author

@part1zano sorry for the lack of response. Your patch works for me:

Before:

$ LANG=ru_RU.UTF-8 ./zpool status -t
  pool: tank
 state: ONLINE
  scan: scrub repaired 0B in 00:00:00 with 0 errors on Thu Mar 28 15:03:39 2024
config:

	NAME          STATE     READ WRITE CKSUM
	tank          ONLINE       0     0     0
	  /tmp/file1  ONLINE       0     0     0  (100% trimmed, completed at Чт 28 мар 2024 15:02:54)
	  /tmp/file2  ONLINE       0     0     0  (100% trimmed, completed at Чт 28 мар 2024 15:02:54)
	  /tmp/file3  ONLINE       0     0     0  (100% trimmed, completed at Чт 28 мар 2024 15:02:54)
	  /tmp/file4  ONLINE       0     0     0  (100% trimmed, completed at Чт 28 мар 2024 15:02:54)

After

LANG=ru_RU.UTF-8 ./zpool status -t
  pool: tank
 state: ONLINE
  scan: scrub repaired 0B in 00:00:00 with 0 errors on Thu Mar 28 15:03:39 2024
config:

	NAME          STATE     READ WRITE CKSUM
	tank          ONLINE       0     0     0
	  /tmp/file1  ONLINE       0     0     0  (100% trimmed, completed at Thu Mar 28 15:02:54 2024)
	  /tmp/file2  ONLINE       0     0     0  (100% trimmed, completed at Thu Mar 28 15:02:54 2024)
	  /tmp/file3  ONLINE       0     0     0  (100% trimmed, completed at Thu Mar 28 15:02:54 2024)
	  /tmp/file4  ONLINE       0     0     0  (100% trimmed, completed at Thu Mar 28 15:02:54 2024)

Great to see it works for you! Thanks for the approval! Could you kindly merge it since everything's ok?

@part1zano
Copy link
Contributor Author

Since this has been approved by someone with write access, could someone please merge it?

@behlendorf
Copy link
Contributor

behlendorf commented Apr 3, 2024 •
edited
Loading

@part1zano the formatting changes to print_history_records() break the history_007_pos test case. Which is why most of the CI runs failed. Given that the original format here is all numerical I don't think there's a need to convert this instance. Plus if there are any other scripts out there parsing the history logs it'd be nice not to break them. Can you revert this hunk, then I think this should be good to go.

Test: /usr/share/zfs/zfs-tests/tests/functional/history/history_007_pos (run as root) [00:00] [FAIL]
19:05:05.60 /etc/sudoers:89:24: invalid operator "+=" for "exempt_group"
19:05:05.63 ASSERTION: Verify command history moves with migrated pool.
19:05:05.63 SUCCESS: mkdir -p /mnt/testdir/importdir.1110339
19:05:05.64 SUCCESS: cp /usr/share/zfs/zfs-tests/tests/functional/history/i386.orig_history.txt /mnt/testdir/importdir.1110339
19:05:05.64 SUCCESS: cp /usr/share/zfs/zfs-tests/tests/functional/history/i386.migratedpool.DAT.Z /mnt/testdir/importdir.1110339
19:05:05.89 SUCCESS: uncompress -f /mnt/testdir/importdir.1110339/i386.migratedpool.DAT.Z
19:05:06.06 SUCCESS: zpool import -d /mnt/testdir/importdir.1110339 history_pool
19:05:06.07 SUCCESS: eval TZ=US/Mountain zpool history history_pool | grep -v "^$" >/mnt/testdir/importdir.1110339/migrated_history.1110339
19:05:06.08 2,12c2,12
19:05:06.08 < Fri Oct 20 13:18:37 2006 zpool create history_pool /var/tmp/i386.migratedpool.DAT
19:05:06.08 < Fri Oct 20 13:18:37 2006 zfs create history_pool/fs
19:05:06.08 < Fri Oct 20 13:18:37 2006 zfs set compression=on history_pool/fs
19:05:06.08 < Fri Oct 20 13:18:37 2006 zfs set checksum=on history_pool
19:05:06.08 < Fri Oct 20 13:18:37 2006 zfs snapshot history_pool/fs@snap
19:05:06.08 < Fri Oct 20 13:18:37 2006 zfs clone history_pool/fs@snap history_pool/clone
19:05:06.08 < Fri Oct 20 13:18:37 2006 zfs promote history_pool/clone
19:05:06.08 < Fri Oct 20 13:18:37 2006 zfs promote history_pool/fs
19:05:06.08 < Fri Oct 20 13:18:37 2006 zfs destroy -r -R history_pool/fs
19:05:06.08 < Fri Oct 20 13:18:37 2006 zpool export history_pool
19:05:06.08 < Thu Apr  5 00:05:38 2007 zpool upgrade history_pool
19:05:06.08 ---
19:05:06.08 > 2006-10-20.13:18:37 zpool create history_pool /var/tmp/i386.migratedpool.DAT
19:05:06.08 > 2006-10-20.13:18:37 zfs create history_pool/fs
19:05:06.08 > 2006-10-20.13:18:37 zfs set compression=on history_pool/fs
19:05:06.08 > 2006-10-20.13:18:37 zfs set checksum=on history_pool
19:05:06.08 > 2006-10-20.13:18:37 zfs snapshot history_pool/fs@snap
19:05:06.08 > 2006-10-20.13:18:37 zfs clone history_pool/fs@snap history_pool/clone
19:05:06.08 > 2006-10-20.13:18:37 zfs promote history_pool/clone
19:05:06.08 > 2006-10-20.13:18:37 zfs promote history_pool/fs
19:05:06.08 > 2006-10-20.13:18:37 zfs destroy -r -R history_pool/fs
19:05:06.08 > 2006-10-20.13:18:37 zpool export history_pool
19:05:06.08 > 2007-04-05.00:05:38 zpool upgrade history_pool

@part1zano
Fix locale-specific time
2bbf2d9 
In `zpool status -t`, scrub date/time is reported using the C locale,
while trim time is reported using the current one. This is inconsistent.
This patch fixes that.

Signed-off-by: Maxim Filimonov <che@bein.link>
Closes #15878
@part1zano
Copy link
Contributor Author

@behlendorf reverted!

@behlendorf behlendorf added Status: Accepted Ready to integrate (reviewed, tested) and removed Status: Code Review Needed Ready for review and testing labels Apr 8, 2024
@behlendorf behlendorf merged commit f07389d into openzfs:master Apr 8, 2024
24 of 26 checks passed
tonyhutter pushed a commit that referenced this pull request May 2, 2024
@part1zano @tonyhutter
Fix locale-specific time
86b39b4 
In `zpool status -t`, scrub date/time is reported using the C locale,
while trim time is reported using the current one. This is inconsistent.
This patch fixes that.

Reviewed-by: Tino Reichardt <milky-zfs@mcmilk.de>
Reviewed-by: Tony Hutter <hutter2@llnl.gov>
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: Maxim Filimonov <che@bein.link>
Closes #15878
Closes #15879
lundman pushed a commit to openzfsonwindows/openzfs that referenced this pull request Sep 4, 2024
@part1zano @lundman
Fix locale-specific time
b2f4b0b 
In `zpool status -t`, scrub date/time is reported using the C locale,
while trim time is reported using the current one. This is inconsistent.
This patch fixes that.

Reviewed-by: Tino Reichardt <milky-zfs@mcmilk.de>
Reviewed-by: Tony Hutter <hutter2@llnl.gov>
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: Maxim Filimonov <che@bein.link>
Closes openzfs#15878
Closes openzfs#15879
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@glebius glebius glebius left review comments

@tonyhutter tonyhutter tonyhutter approved these changes

@mcmilk mcmilk mcmilk approved these changes

@behlendorf behlendorf behlendorf approved these changes

Assignees
No one assigned
Labels
Status: Accepted Ready to integrate (reviewed, tested)
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Locale-specific time in trim status
5 participants
@part1zano @mcmilk @tonyhutter @behlendorf @glebius