Bug 1535370 - vdsm does not call ioprocess writefile with 'direct=True' so it doesn't use Direct IO
Summary: vdsm does not call ioprocess writefile with 'direct=True' so it doesn't use D...
Keywords:
Status: CLOSED WONTFIX
Alias: None
Product: vdsm
Classification: oVirt
Component: Core
Version: 4.20.15
Hardware: Unspecified
OS: Unspecified
high
high
Target Milestone: ---
: ---
Assignee: Vojtech Juranek
QA Contact: Elad
URL:
Whiteboard:
Depends On: 1535401
Blocks:
TreeView+ depends on / blocked
 
Reported: 2018-01-17 09:02 UTC by Yaniv Kaul
Modified: 2022-04-21 06:46 UTC (History)
5 users (show)

Fixed In Version:
Clone Of:
Environment:
Last Closed: 2019-03-17 13:49:17 UTC
oVirt Team: Storage
Embargoed:
sbonazzo: ovirt-4.3-
sbonazzo: exception-


Attachments (Terms of Use)


Links
System ID Private Priority Status Summary Last Updated
Red Hat Issue Tracker RHV-45831 0 None None None 2022-04-21 06:46:47 UTC

Description Yaniv Kaul 2018-01-17 09:02:18 UTC
Description of problem:
def writeLines(ioproc, path, lines):
    data = ''.join(lines)
    return writeFile(ioproc, path, data)


def writeFile(ioproc, path, data):
    return ioproc.writefile(path, data)


And thus all calls to ioprocess write* are not setting the direct flag.
The default is False:
bindings/python/ioprocess/__init__.py:    def writefile(self, path, data, direct=False):



whereas in at least one case of read, we do set it:
def directReadLines(ioproc, path):
    fileStr = ioproc.readfile(path, direct=True)
    return fileStr.splitlines(True)

Comment 1 Nir Soffer 2018-01-17 11:04:20 UTC
One issue, ioprocess writefile with the direct flag does not perform direct I/O.
(see bug 1535401). We will have to fix this and require new ioprocess version
before we can change the vdsm code.

Alternative solution is to perform the io using dd which is better designed for 
writing and reading data.

The effected code in vdsm are:

outOfProcess.writeFile:
- storage.sdm.volume_artifacts - from spm removal effort, unused.

outOfProcess.writeLines:
- storage.fileSD.FileMetadataRW.writeLines - used for writing volume metadata
- storage.task.Task._saveMetaFile - use to storage task data in the master domain.

writeLines is old as vdsm, we never used direct I/O. writeFile was added in 
https://gerrit.ovirt.org/56415 as a wrapper over the underlying writefile that
did not expose the direct flag.

Comment 2 Nir Soffer 2018-01-17 12:50:24 UTC
Suggesting to consider for 4.2.1. But since this is broken for so many years I
don't think we need to block the version because of this, and it can also be 
delivered in 4.2.2 or later.

Comment 3 Nir Soffer 2018-01-21 20:45:26 UTC
Lowering severity, since direct I/O is not the important issue here. The real
issue is not performing fsync when the write is completed - see 1535429.
The fix for this bug does not require any change in vdsm, only require new
ioprocess version including this fix.

Using direct I/O make sense since we never want to use the host cache when reading
the metadata, but it complicates both vdsm and ioprocess code. For iprocess we
need several fixes to use direct I/O.

Lets start with deferring this to 4.2.2 or later.

Comment 4 Yaniv Kaul 2018-02-22 13:57:25 UTC
I thought Nir already fixed it?

Comment 5 Nir Soffer 2018-02-22 15:06:21 UTC
(In reply to Yaniv Kaul from comment #4)
> I thought Nir already fixed it?

I have patches for enabling direct I/O in ioproces, but I don't think this is the
right approach. ioprocess is not good at reading and writing data, better replace
it with a tool designed for this, and drop the "direct" option that never worked.

Also the metadata implementation may be replaced in 4.3, and I don't want to spend
time on code that will be deleted soon.

Comment 6 Sandro Bonazzola 2019-01-28 09:36:44 UTC
This bug has not been marked as blocker for oVirt 4.3.0.
Since we are releasing it tomorrow, January 29th, this bug has been re-targeted to 4.3.1.

Comment 7 Nir Soffer 2019-03-17 13:49:17 UTC
The blocking bug was closed as WONTFIX, closing.


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