Bug 1536261 - IOProcess.fsyncPath does not fsync anything
Summary: IOProcess.fsyncPath does not fsync anything
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: ovirt-distribution
Classification: oVirt
Component: ioprocess
Version: 4.2.1
Hardware: Unspecified
OS: Unspecified
unspecified
high vote
Target Milestone: ovirt-4.2.1
: ---
Assignee: Nir Soffer
QA Contact: Lilach Zitnitski
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2018-01-19 01:20 UTC by Nir Soffer
Modified: 2018-03-06 17:18 UTC (History)
2 users (show)

Fixed In Version: ioprocess-1.0.2-1.fc27
Doc Type: If docs needed, set a value
Doc Text:
Clone Of:
Environment:
Last Closed: 2018-02-22 10:01:45 UTC
oVirt Team: Storage
rule-engine: ovirt-4.2?
tnisan: planning_ack?
tnisan: devel_ack+
rule-engine: testing_ack+


Attachments (Terms of Use)


Links
System ID Priority Status Summary Last Updated
oVirt gerrit 86568 None None None 2018-01-19 02:39:52 UTC
oVirt gerrit 86569 None None None 2018-01-19 02:40:21 UTC

Description Nir Soffer 2018-01-19 01:20:11 UTC
Description of problem:

Due to a unfortunate copy and paste, calling fsyncPath() actually call lexists().

492     def lexists(self, path):
493         return self._sendCommand("lexists", {"path": path}, self.timeout)
494 
495     def fsyncPath(self, path):                                                                                                                                                            
496         return self._sendCommand("lexists", {"path": path}, self.timeout)

The code was introduced in:

commit ad4cf3d0e8c398d08f5cf60fe1a9582bb8befb8e
Author: Saggi Mizrahi <ficoos@gmail.com>
Date:   Sun Jan 5 14:40:27 2014 +0200

    python bindings
    
    Signed-off-by: Saggi Mizrahi <ficoos@gmail.com>

Version-Release number of selected component (if applicable):
Any

How reproducible:
Always

Steps to Reproduce:
1. Run ioprocess with strace -f
2. Call IOProcessClient.fsyncPath("/path/to/file")

Actual results:
In strace output we can see that ioprocess call access()

Expected results:
ioprocess should open the file and call fsync().

Comment 1 Yaniv Kaul 2018-01-19 08:30:51 UTC
But is anyone even calling it?

Comment 2 Yaniv Kaul 2018-01-19 08:33:07 UTC
(In reply to Yaniv Kaul from comment #1)
> But is anyone even calling it?

In VDSM - lib/vdsm/storage/fileUtils.py:
def fsyncPath(path):
    fd = os.open(path, os.O_RDONLY)
    try:
        os.fsync(fd)
    finally:
        os.close(fd)


So this one doesn't (but using the same name? a bit confusing).

And in lib/vdsm/storage/outOfProcess.py :
    def fsyncPath(self, path):
        self._iop.fsyncPath(path)

Comment 3 Nir Soffer 2018-01-20 12:39:16 UTC
(In reply to Yaniv Kaul from comment #1)
> But is anyone even calling it?

$ git grep fsyncPath
lib/vdsm/storage/fileUtils.py:def fsyncPath(path):

I think this is leftover from the previous python based out-of-process
implementation (remoteFileHandler), and no code is using it.

lib/vdsm/storage/outOfProcess.py:    def fsyncPath(self, path):
lib/vdsm/storage/outOfProcess.py:        self._iop.fsyncPath(path)

This the wrapper calling IOProcessClient.fsyncPath

lib/vdsm/storage/task.py:        getProcPool().fileUtils.fsyncPath(origTaskDir)

This call is used when saving task state to master file system. The task data is
used during rollbacks. I'm not sure if this code actually works since we don't
have any tests for it, but the safest solution would to fix iprocess to do what
the original author intended.

Comment 4 Nir Soffer 2018-01-21 14:54:36 UTC
Tal, this is a bad issue in storage infrastructure, and we have a trivial fix. 
Suggesting for 4.2.1.

Comment 5 Fedora Update System 2018-01-30 17:03:38 UTC
ioprocess-1.0.0-1.fc27 has been submitted as an update to Fedora 27. https://bodhi.fedoraproject.org/updates/FEDORA-2018-fbe8141dd2

Comment 6 Fedora Update System 2018-01-31 22:45:21 UTC
ioprocess-1.0.0-1.fc27 has been pushed to the Fedora 27 testing repository. If problems still persist, please make note of it in this bug report.
See https://fedoraproject.org/wiki/QA:Updates_Testing for
instructions on how to install test updates.
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2018-fbe8141dd2

Comment 7 Fedora Update System 2018-02-04 13:25:52 UTC
ioprocess-1.0.2-1.fc27 has been submitted as an update to Fedora 27. https://bodhi.fedoraproject.org/updates/FEDORA-2018-5fc2a37e8a

Comment 10 Fedora Update System 2018-02-04 20:18:39 UTC
ioprocess-1.0.2-1.fc27 has been pushed to the Fedora 27 testing repository. If problems still persist, please make note of it in this bug report.
See https://fedoraproject.org/wiki/QA:Updates_Testing for
instructions on how to install test updates.
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2018-5fc2a37e8a

Comment 11 Lilach Zitnitski 2018-02-19 09:19:02 UTC
Full tier1 automation ran using ioprocess-1.0.2-1.el7ev.x86_64, no errors or regressions occurred related to the new ioprocess.

moving to verify.

Comment 12 Sandro Bonazzola 2018-02-22 10:01:45 UTC
This bugzilla is included in oVirt 4.2.1 release, published on Feb 12th 2018.

Since the problem described in this bug report should be
resolved in oVirt 4.2.1 release, it has been closed with a resolution of CURRENT RELEASE.

If the solution does not work for you, please open a new bug report.

Comment 13 Fedora Update System 2018-03-06 17:18:50 UTC
ioprocess-1.0.2-1.fc27 has been pushed to the Fedora 27 stable repository. If problems still persist, please make note of it in this bug report.


Note You need to log in before you can comment on or make changes to this bug.