{"action":"created","issue":{"url":"https://api.github.com/repos/zfsonlinux/zfs/issues/3653","labels_url":"https://api.github.com/repos/zfsonlinux/zfs/issues/3653/labels{/name}","comments_url":"https://api.github.com/repos/zfsonlinux/zfs/issues/3653/comments","events_url":"https://api.github.com/repos/zfsonlinux/zfs/issues/3653/events","html_url":"https://github.com/zfsonlinux/zfs/pull/3653","id":98284798,"number":3653,"title":"Linux 4.2 compat: vfs_rename()","user":{"login":"behlendorf","id":148917,"avatar_url":"https://avatars.githubusercontent.com/u/148917?v=3","gravatar_id":"","url":"https://api.github.com/users/behlendorf","html_url":"https://github.com/behlendorf","followers_url":"https://api.github.com/users/behlendorf/followers","following_url":"https://api.github.com/users/behlendorf/following{/other_user}","gists_url":"https://api.github.com/users/behlendorf/gists{/gist_id}","starred_url":"https://api.github.com/users/behlendorf/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/behlendorf/subscriptions","organizations_url":"https://api.github.com/users/behlendorf/orgs","repos_url":"https://api.github.com/users/behlendorf/repos","events_url":"https://api.github.com/users/behlendorf/events{/privacy}","received_events_url":"https://api.github.com/users/behlendorf/received_events","type":"User","site_admin":false},"labels":[{"url":"https://api.github.com/repos/zfsonlinux/zfs/labels/Build%20Issue","name":"Build Issue","color":"e102d8"},{"url":"https://api.github.com/repos/zfsonlinux/zfs/labels/Difficulty%20-%20Easy","name":"Difficulty - Easy","color":"207de5"}],"state":"open","locked":false,"assignee":null,"milestone":{"url":"https://api.github.com/repos/zfsonlinux/zfs/milestones/25","html_url":"https://github.com/zfsonlinux/zfs/milestones/0.6.5","labels_url":"https://api.github.com/repos/zfsonlinux/zfs/milestones/25/labels","id":175736,"number":25,"title":"0.6.5","description":"ZFS on Linux v0.6.5 - Functionality / Integration","creator":{"login":"behlendorf","id":148917,"avatar_url":"https://avatars.githubusercontent.com/u/148917?v=3","gravatar_id":"","url":"https://api.github.com/users/behlendorf","html_url":"https://github.com/behlendorf","followers_url":"https://api.github.com/users/behlendorf/followers","following_url":"https://api.github.com/users/behlendorf/following{/other_user}","gists_url":"https://api.github.com/users/behlendorf/gists{/gist_id}","starred_url":"https://api.github.com/users/behlendorf/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/behlendorf/subscriptions","organizations_url":"https://api.github.com/users/behlendorf/orgs","repos_url":"https://api.github.com/users/behlendorf/repos","events_url":"https://api.github.com/users/behlendorf/events{/privacy}","received_events_url":"https://api.github.com/users/behlendorf/received_events","type":"User","site_admin":false},"open_issues":14,"closed_issues":82,"state":"open","created_at":"2012-09-16T18:38:32Z","updated_at":"2015-08-18T18:27:42Z","due_on":null,"closed_at":null},"comments":4,"created_at":"2015-07-30T23:21:12Z","updated_at":"2015-08-18T23:00:20Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/zfsonlinux/zfs/pulls/3653","html_url":"https://github.com/zfsonlinux/zfs/pull/3653","diff_url":"https://github.com/zfsonlinux/zfs/pull/3653.diff","patch_url":"https://github.com/zfsonlinux/zfs/pull/3653.patch"},"body":"The spa_config_write() function relies on the classic method of\r\nmaking sure updates to the /etc/zfs/zpool.cache file are atomic.\r\nIt writes out a temporary version of the file and then uses\r\nvn_rename() to switch it in to place.  This way there can never\r\nexist a partial version of the file, it's all or nothing.\r\n\r\nConceptually this is a good strategy and it makes good sense\r\nfor platforms where it's easy to do a rename within the kernel.\r\nUnfortunately, Linux is not one of those platforms.  Even doing\r\nbasic I/O to a file system from within the kernel is strongly\r\ndiscouraged.  In order to support this at all the vn_rename()\r\nimplementation ends up being complex and fragile.  So fragile\r\nthat recent Linux 4.2 changes have broken it.\r\n\r\nWhile it is possible to update vn_rename() to work with the\r\nlatest kernels a better long term strategy is to stop using\r\nvn_rename() entirely.  Then all this complex, fragile code can\r\nbe removed.  Achieving this is straight forward because\r\nconfig_write() is the only consumer of vn_rename().\r\n\r\nThis patch reworks spa_config_write() to update the cache file\r\nin place.  The file will be truncated, written out, and then\r\nsynced to disk.  If an error is encountered the file will be\r\nunlinked leaving the system in a consistent state.\r\n\r\nThis does expose a tiny tiny tiny window where a system could\r\ncrash at exactly the wrong moment could leave a partially written\r\ncache file.  However, this is highly unlikely because the cache\r\nfile is 1) infrequently updated, 2) only a few kilobytes in size,\r\nand 3) written with a single vn_rdwr() call.\r\n\r\nIf this were to somehow happen it poses no risk to pool.  Simply\r\nremoving the cache file will allow the pool to be imported cleanly.\r\nGoing forward this will be even less of an issue as we intend to\r\ndisable the use of a cache file by default.\r\n\r\nBottom line not using vn_rename() allows us to make ZoL more\r\nrobust against upstream kernel changes.\r\n\r\nSigned-off-by: Brian Behlendorf <behlendorf1@llnl.gov>"},"comment":{"url":"https://api.github.com/repos/zfsonlinux/zfs/issues/comments/132385361","html_url":"https://github.com/zfsonlinux/zfs/pull/3653#issuecomment-132385361","issue_url":"https://api.github.com/repos/zfsonlinux/zfs/issues/3653","id":132385361,"user":{"login":"behlendorf","id":148917,"avatar_url":"https://avatars.githubusercontent.com/u/148917?v=3","gravatar_id":"","url":"https://api.github.com/users/behlendorf","html_url":"https://github.com/behlendorf","followers_url":"https://api.github.com/users/behlendorf/followers","following_url":"https://api.github.com/users/behlendorf/following{/other_user}","gists_url":"https://api.github.com/users/behlendorf/gists{/gist_id}","starred_url":"https://api.github.com/users/behlendorf/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/behlendorf/subscriptions","organizations_url":"https://api.github.com/users/behlendorf/orgs","repos_url":"https://api.github.com/users/behlendorf/repos","events_url":"https://api.github.com/users/behlendorf/events{/privacy}","received_events_url":"https://api.github.com/users/behlendorf/received_events","type":"User","site_admin":false},"created_at":"2015-08-18T23:00:20Z","updated_at":"2015-08-18T23:00:20Z","body":"@ryao even on multipath systems you must never rely on the cache file to ensure the right pools are imported.  This is the job for proper high availability package such as corosync or pacemaker to make sure the pool isn't importable concurrently on two systems.  The cache files only real purpose is to speed up pool imports when there are a large number (100's) of devices.\r\n\r\nRegardless, to support 4.2 kernels we either need to do this (which is very very very virtually zero risk).  Or someone needs to propose an alternative."}}