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.

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

Fix false ereport.fs.zfs.config_cache_write events. #6617

Merged
merged 1 commit into from
Sep 11, 2017

Conversation

ab-oe
Copy link
Contributor

@ab-oe ab-oe commented Sep 8, 2017

Description

On pool import when the old cache file is removed the ereport.fs.zfs.config_cache_write event is generated. Because zpool export always removes cache file it happens every export - import sequence.

Motivation and Context

To fix false cache write failure events on every export-import sequence.

How Has This Been Tested?

  1. Create zpool.
  2. Export zpool.
  3. Import zpool.
  4. Check if there is no ereport.fs.zfs.config_cache_write in zpool events.

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)
  • Documentation (a change to man pages or other documentation)

Checklist:

  • My code follows the ZFS on Linux code style requirements.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.
  • All commit messages are properly formatted and contain Signed-off-by.
  • Change has been approved by a ZFS on Linux member.

@loli10K
Copy link
Contributor

loli10K commented Sep 8, 2017

Because zpool export always removes cache file it happens every export - import sequence.

Is this always true? It doesn't seem to be happening on my setup, the cachefile is still alive and well after zpool export:

root@linux:~# cat /sys/module/zfs/version 
0.7.0-65_g41a4f3f
root@linux:~# zpool export $POOLNAME
root@linux:~# rm -fv /etc/zfs/zpool.cache 
removed ‘/etc/zfs/zpool.cache’
root@linux:~# zpool import -d /var/tmp/ $POOLNAME
root@linux:~# zpool list $POOLNAME
NAME       SIZE  ALLOC   FREE  EXPANDSZ   FRAG    CAP  DEDUP  HEALTH  ALTROOT
testpool    48M   106K  47.9M         -     3%     0%  1.00x  ONLINE  -
root@linux:~# md5sum /etc/zfs/zpool.cache
25efb86eaa0624bf91bda1141cf5926b  /etc/zfs/zpool.cache
root@linux:~# zpool export $POOLNAME
root@linux:~# zpool list
no pools available
root@linux:~# md5sum /etc/zfs/zpool.cache
25efb86eaa0624bf91bda1141cf5926b  /etc/zfs/zpool.cache
root@linux:~# 

@ab-oe
Copy link
Contributor Author

ab-oe commented Sep 8, 2017

@loli10K what kernel do you use? Since 4.2 there is problem that the cache is never removed. I reported issue and proposed bugfix here: openzfs/spl#648

@loli10K
Copy link
Contributor

loli10K commented Sep 8, 2017

@ab-oe thanks, that explains it, i use kernel 4.6.4 on this box. I was in fact testing this PR but still getting config_cache_write events:

root@linux:~# zpool export $POOLNAME
root@linux:~# rm -fv /etc/zfs/zpool.cache 
removed ‘/etc/zfs/zpool.cache’
root@linux:~# zpool import -d /var/tmp/ $POOLNAME
root@linux:~# zpool list $POOLNAME
NAME       SIZE  ALLOC   FREE  EXPANDSZ   FRAG    CAP  DEDUP  HEALTH  ALTROOT
testpool    48M   106K  47.9M         -     3%     0%  1.00x  ONLINE  -
root@linux:~# md5sum /etc/zfs/zpool.cache
0e2e31596ae31332d38a29822805ec12  /etc/zfs/zpool.cache
root@linux:~# zpool export $POOLNAME
root@linux:~# zpool list
no pools available
root@linux:~# md5sum /etc/zfs/zpool.cache
0e2e31596ae31332d38a29822805ec12  /etc/zfs/zpool.cache
root@linux:~# zpool events
TIME                           CLASS
Sep  8 2017 16:06:48.647968347 sysevent.fs.zfs.pool_destroy
Sep  8 2017 16:06:48.655968339 ereport.fs.zfs.config_cache_write
Sep  8 2017 16:06:48.655968339 sysevent.fs.zfs.config_sync
Sep  8 2017 16:06:48.715968282 sysevent.fs.zfs.history_event
Sep  8 2017 16:06:48.739968259 sysevent.fs.zfs.config_sync
Sep  8 2017 16:06:48.739968259 sysevent.fs.zfs.pool_import
Sep  8 2017 16:06:48.743968255 sysevent.fs.zfs.history_event
Sep  8 2017 16:06:48.767968232 sysevent.fs.zfs.config_sync
Sep  8 2017 16:06:48.819968183 sysevent.fs.zfs.pool_destroy
Sep  8 2017 16:06:48.831968171 ereport.fs.zfs.config_cache_write
Sep  8 2017 16:06:48.831968171 sysevent.fs.zfs.config_sync
root@linux:~# 

I now see it's because vn_remove() is returning EACCES instead of ENOENT:

kretprobe_trampoline -> spa_config_write (dp={.scd_link={.next=0xffff880014842cf8, .prev=0xffff880014842cf8}, .scd_path="/etc/zfs/zpool.cache"} nvl=ERROR)
spa_export_common <- spa_config_write (return=13)

Copy link
Contributor

@loli10K loli10K left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The commit message must be updated to fix the STYLE buildbot warnings, otherwise LGTM

@ab-oe ab-oe force-pushed the false_cache_error_fix branch from 41a4f3f to 6b368cc Compare September 11, 2017 06:08
On pool import when the old cache file is removed
the ereport.fs.zfs.config_cache_write event is generated.
Because zpool export always removes cache file it happens
every export - import sequence.

Signed-off-by: Arkadiusz Bubała <[email protected]>
@ab-oe
Copy link
Contributor Author

ab-oe commented Sep 11, 2017

@loli10K thanks. I fixed commit message.

@behlendorf behlendorf merged commit d9549cb into openzfs:master Sep 11, 2017
FransUrbo pushed a commit to FransUrbo/zfs that referenced this pull request Apr 28, 2019
On pool import when the old cache file is removed
the ereport.fs.zfs.config_cache_write event is generated.
Because zpool export always removes cache file it happens
every export - import sequence.

Reviewed-by: George Melikov <[email protected]>
Reviewed-by: loli10K <[email protected]>
Reviewed-by: Giuseppe Di Natale <[email protected]>
Signed-off-by: Arkadiusz Bubała <[email protected]>
Closes openzfs#6617
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants