Bug 1564515 - [RFE] Add lstat() to ioprocess
Summary: [RFE] Add lstat() to ioprocess
Keywords:
Status: CLOSED CURRENTRELEASE
Alias: None
Product: ovirt-distribution
Classification: oVirt
Component: ioprocess
Version: ---
Hardware: Unspecified
OS: Unspecified
unspecified
medium
Target Milestone: ovirt-4.2.6
: ---
Assignee: Nir Soffer
QA Contact: Elad
URL:
Whiteboard:
Depends On:
Blocks: 1560460
TreeView+ depends on / blocked
 
Reported: 2018-04-06 13:30 UTC by Nir Soffer
Modified: 2018-09-03 15:11 UTC (History)
4 users (show)

Fixed In Version:
Doc Type: If docs needed, set a value
Doc Text:
Clone Of:
Environment:
Last Closed: 2018-09-03 15:11:05 UTC
oVirt Team: Storage
Embargoed:
rule-engine: ovirt-4.2+
ebenahar: testing_plan_complete-
ylavi: planning_ack+
rule-engine: devel_ack+
ebenahar: testing_ack+


Attachments (Terms of Use)


Links
System ID Private Priority Status Summary Last Updated
oVirt gerrit 89899 0 None None None 2018-04-06 13:53:48 UTC
oVirt gerrit 89900 0 None None None 2018-04-06 13:54:16 UTC

Description Nir Soffer 2018-04-06 13:30:06 UTC
Description of problem:

vdsm needs lstat() to implement os.path.islink(). Since lstat() may block if the 
link is on shared storage, we need to implement it in ioprocess.

How to test the new feature:

1. Create a symlink

$ ln -s /no/such/file test-link

2. Print symlink stats

$ python -c 'import ioprocess; print ioprocess.IOProcess().lstat("test-link")'
StatResult(st_mode=41471, st_ino=8521781, st_dev=64772, st_nlink=1, st_uid=1000, st_gid=1000, st_size=13, st_atime=1523021206.0, st_mtime=1523021206.0, st_ctime=1523021206.0, st_blocks=0)

Comment 1 Nir Soffer 2018-04-06 14:01:00 UTC
Tal, we need this in 4.2.z. Can you add the necessary flags?

Comment 2 Allon Mureinik 2018-04-11 08:38:27 UTC
Nir, both patches are merged. Should this be MODIFIED? Are we going to have another ioprocess build for 4.2?

Comment 3 Yaniv Lavi 2018-04-11 08:39:08 UTC
We is the functional impact of this RFE?

Comment 4 Nir Soffer 2018-04-11 08:48:05 UTC
Allon: yes, it is modified, ioprocess bugzilla integration is broken for some
reason. I will build a new ioprocess for 4.2.z.

Yaniv: see the vdsm bug depending on this.

Comment 5 Sandro Bonazzola 2018-06-01 06:33:37 UTC
I haven't seen an ioprocess build for 4.2.4 including this fix. Did you build?

Comment 6 Tal Nisan 2018-06-08 08:55:57 UTC
ioprocess was not build and doubt if it will make it to 4.2.4, it's a minor issue so I'm deferring to 4.2.5

Comment 7 Elad 2018-06-17 08:43:59 UTC
Nir, is there a specific flow we should test here?

Comment 8 Sandro Bonazzola 2018-06-29 08:11:45 UTC
Moving back to POST since patches have been merged to master but not backported to ioprocess-1.0 branch for 4.2.5

Comment 9 Nir Soffer 2018-07-02 17:52:18 UTC
(In reply to Sandro Bonazzola from comment #5)
> I haven't seen an ioprocess build for 4.2.4 including this fix. Did you
> build?

Eyal is working on the build for 4.2.5.

Comment 10 Elad 2018-07-12 14:20:55 UTC
Re adding need info from comment #7

Comment 11 Nir Soffer 2018-07-12 14:51:18 UTC
(In reply to Elad from comment #7)
> Nir, is there a specific flow we should test here?

You can try to reproduce bug 1560460, probably by adding a symlink in storage
manually. I'm not sure it will be easy to reproduce and I never tried it.

The new lstat() api has automated tests so I don't see a need to waste time on 
checking the new api manually.

The new api is used deep inside vdsm so many flows may be affected, so the standard
regression tests is the best way to verify this.

Comment 12 Elad 2018-07-12 14:54:17 UTC
Thanks Nir, we will verify based on our regression tests results.

Comment 13 Nir Soffer 2018-08-13 12:40:31 UTC
This is available in ioprocess 1.1.2 for some time.

Comment 14 Elad 2018-08-13 15:15:29 UTC
Haven't seen any related issue in the last regression automation executions.

vdsm-4.20.35-1.el7ev.x86_64
ioprocess-1.1.2-1.el7ev.x86_64


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