Change in vdsm[master]: force reload udev rules to avoid LUN device permission error

Royce Lv has uploaded a new change for review.

Change subject: force reload udev rules to avoid LUN device permission error
......................................................................

force reload udev rules to avoid LUN device permission error

udev rules creat symlink for LUN device making it usable for qemu
but the rules are not instantly loaded by udev
result in LUN device symlink not created before vm run
force reload to make the rules take effect instantly

Change-Id: I718d62e67b0a228f2510233c5dd9d1d94c4a736c
Signed-off-by: Royce Lv
---
M vdsm/storage/hsm.py
M vdsm/supervdsmServer.py
2 files changed, 9 insertions(+), 0 deletions(-)

git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/80/6780/1
--
To view, visit http://gerrit.ovirt.org/6780
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I718d62e67b0a228f2510233c5dd9d1d94c4a736c
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Royce Lv

[Xu He Jie]

Xu He Jie has posted comments on this change.

Change subject: force reload udev rules to avoid LUN device permission error
......................................................................

Patch Set 1: Looks good to me, but someone else must approve

(1 inline comment)

....................................................
File vdsm/storage/hsm.py
Line 2630:         supervdsm.getProxy().forceReloadRules()
Just execute udevadm --reload in the function udevTrigger will be better? Are there any others will invoke forceReloadRules?

--
To view, visit http://gerrit.ovirt.org/6780
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I718d62e67b0a228f2510233c5dd9d1d94c4a736c
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Royce Lv 
Gerrit-Reviewer: Eduardo 
Gerrit-Reviewer: Xu He Jie

Eduardo has posted comments on this change.

Change subject: force reload udev rules to avoid LUN device permission error
......................................................................

Patch Set 1: I would prefer that you didn't submit this

(2 inline comments)

Reloading the udev rules is usually not needed. 
Is there an open bug on the rule reload race?
How long is the delay?
> From man:

--reload-rules
Signal udevd to reload the rules files.
The udev daemon detects changes automatically, this option is usually not needed.

....................................................
File vdsm/supervdsmServer.py
Line 221:         cmd = [EXT_UDEVADM, 'control', '--reload']
Should be --reload-rules instead of reload.

Line 224:             raise OSError(errno.EINVAL, "Could not reload rules for device \
The reload is  for all the rules not for a specific device.

--
To view, visit http://gerrit.ovirt.org/6780
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I718d62e67b0a228f2510233c5dd9d1d94c4a736c
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Royce Lv 
Gerrit-Reviewer: Eduardo 
Gerrit-Reviewer: Xu He Jie

Royce Lv has posted comments on this change.

Change subject: force reload udev rules to avoid LUN device permission error
......................................................................

Patch Set 1:

Eduardo,Thanks for reviewing, but it seems our men are differen;)
> From my man:

udevadm control command
       Modify the internal state of the running udev daemon.
--reload
           Signal udevd to reload the rules files and other databases like the kernel module index. Reloading rules and databases does not apply any changes to already
           existing devices; the new configuration will only be applied to new events.

I have just reported this bug to bz#545179:https://bugzilla.redhat.com/show_bug.cgi?id=845179

--
To view, visit http://gerrit.ovirt.org/6780
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I718d62e67b0a228f2510233c5dd9d1d94c4a736c
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Royce Lv 
Gerrit-Reviewer: Eduardo 
Gerrit-Reviewer: Royce Lv 
Gerrit-Reviewer: Xu He Jie

[Dan Kenigsberg]

Dan Kenigsberg has posted comments on this change.

Change subject: force reload udev rules to avoid LUN device permission error
......................................................................

Patch Set 1: I would prefer that you didn't submit this

what is your `udevadm --version`? My v173 has

# udevadm --reload
 udevadm: unrecognized option '--reload'

It seems that we have a race here - a rule is written but does not takes into effect quickly enough. I'm not even sure that --reload-rules is synchronous... How about waiting (with timeout) until the rule is applied?

--
To view, visit http://gerrit.ovirt.org/6780
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I718d62e67b0a228f2510233c5dd9d1d94c4a736c
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Royce Lv 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Eduardo 
Gerrit-Reviewer: Royce Lv 
Gerrit-Reviewer: Xu He Jie

[Xu He Jie]

Xu He Jie has posted comments on this change.

Change subject: force reload udev rules to avoid LUN device permission error
......................................................................

Patch Set 1:

I had tested 'udevadm trigger'. it is asynchronous. We should execute 'udevadm settle' after trigger, it will wait for trigger finished.

--
To view, visit http://gerrit.ovirt.org/6780
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I718d62e67b0a228f2510233c5dd9d1d94c4a736c
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Royce Lv 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Eduardo 
Gerrit-Reviewer: Royce Lv 
Gerrit-Reviewer: Xu He Jie

Royce Lv has posted comments on this change.

Change subject: force reload udev rules to avoid LUN device permission error
......................................................................

Patch Set 1:

Dan,
my cmd is :
#udevadm control --reload
version is 182, shipped with fc17
checked the source code,found reload rules happens at:
(1)periodically check rule file time stamp and found it's changed(3s at most)
(2)SIGHUP
(3)call reload rules
under these three condition all rules are cleared 
udevd will reload rules to mem synchronizedly.
the udev-client is blocked until it polled the ack from udevd means the control finished.
So I guess it's safe to call this cmd.

Can check the source code here:
www.kernel.org/pub/linux/utils/kernel/hotplug/udev-182.tar.bz2
(udevd.c/udev-rules.c)
According to changelog 181:
The 'udevadm control --reload-rules' option has been renamed to '--reload'.
It now also reloads the kernel module configuration. 
Thought in source code udev-182 it still restores --reload-rules, but did not expose it to man.
So I'm afraid there will be some gap of this cmd in the future.

--
To view, visit http://gerrit.ovirt.org/6780
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I718d62e67b0a228f2510233c5dd9d1d94c4a736c
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Royce Lv 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Eduardo 
Gerrit-Reviewer: Royce Lv 
Gerrit-Reviewer: Xu He Jie

Mark Wu has posted comments on this change.

Change subject: force reload udev rules to avoid LUN device permission error
......................................................................

Patch Set 1:

Royce, could you please check if just a 3s timeout between changing rule and triggering event can resolve the race between them?  Thanks!

--
To view, visit http://gerrit.ovirt.org/6780
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I718d62e67b0a228f2510233c5dd9d1d94c4a736c
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Royce Lv 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Eduardo 
Gerrit-Reviewer: Mark Wu 
Gerrit-Reviewer: Royce Lv 
Gerrit-Reviewer: Xu He Jie

Change in vdsm[master]: avoid race between load dev rules and trigger the rule

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: avoid race between load dev rules and trigger the rule
......................................................................

Patch Set 2: No score

Build Started http://jenkins.ovirt.info/job/vdsm_unit_tests_by_patch/56/

--
To view, visit http://gerrit.ovirt.org/6780
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I718d62e67b0a228f2510233c5dd9d1d94c4a736c
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Royce Lv 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Eduardo 
Gerrit-Reviewer: Mark Wu 
Gerrit-Reviewer: Royce Lv 
Gerrit-Reviewer: Xu He Jie 
Gerrit-Reviewer: oVirt Jenkins CI Server

Change in vdsm[master]: avoid race between load dev rules and trigger the rule

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: avoid race between load dev rules and trigger the rule
......................................................................

Patch Set 2:

Build Successful

http://jenkins.ovirt.info/job/vdsm_unit_tests_by_patch/56/ : SUCCESS

--
To view, visit http://gerrit.ovirt.org/6780
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I718d62e67b0a228f2510233c5dd9d1d94c4a736c
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Royce Lv 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Eduardo 
Gerrit-Reviewer: Mark Wu 
Gerrit-Reviewer: Royce Lv 
Gerrit-Reviewer: Xu He Jie 
Gerrit-Reviewer: oVirt Jenkins CI Server

Change in vdsm[master]: avoid race between load dev rules and trigger the rule

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: avoid race between load dev rules and trigger the rule
......................................................................

Patch Set 3:

Build Started http://jenkins.ovirt.info/job/vdsm_unit_tests_by_patch/268/

--
To view, visit http://gerrit.ovirt.org/6780
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I718d62e67b0a228f2510233c5dd9d1d94c4a736c
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Royce Lv 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Eduardo 
Gerrit-Reviewer: Mark Wu 
Gerrit-Reviewer: Royce Lv 
Gerrit-Reviewer: Xu He Jie 
Gerrit-Reviewer: oVirt Jenkins CI Server

Change in vdsm[master]: avoid race between load dev rules and trigger the rule

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: avoid race between load dev rules and trigger the rule
......................................................................

Patch Set 3:

Build Successful

http://jenkins.ovirt.info/job/vdsm_unit_tests_by_patch/268/ : SUCCESS

--
To view, visit http://gerrit.ovirt.org/6780
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I718d62e67b0a228f2510233c5dd9d1d94c4a736c
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Royce Lv 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Eduardo 
Gerrit-Reviewer: Mark Wu 
Gerrit-Reviewer: Royce Lv 
Gerrit-Reviewer: Xu He Jie 
Gerrit-Reviewer: oVirt Jenkins CI Server

Change in vdsm[master]: avoid race between load dev rules and trigger the rule

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: avoid race between load dev rules and trigger the rule
......................................................................

Patch Set 4:

Build Started http://jenkins.ovirt.info/job/vdsm_unit_tests_by_patch/269/

--
To view, visit http://gerrit.ovirt.org/6780
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I718d62e67b0a228f2510233c5dd9d1d94c4a736c
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Royce Lv 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Eduardo 
Gerrit-Reviewer: Mark Wu 
Gerrit-Reviewer: Royce Lv 
Gerrit-Reviewer: Xu He Jie 
Gerrit-Reviewer: oVirt Jenkins CI Server

Change in vdsm[master]: avoid race between load dev rules and trigger the rule

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: avoid race between load dev rules and trigger the rule
......................................................................

Patch Set 4:

Build Successful

http://jenkins.ovirt.info/job/vdsm_unit_tests_by_patch/269/ : SUCCESS

--
To view, visit http://gerrit.ovirt.org/6780
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I718d62e67b0a228f2510233c5dd9d1d94c4a736c
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Royce Lv 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Eduardo 
Gerrit-Reviewer: Mark Wu 
Gerrit-Reviewer: Royce Lv 
Gerrit-Reviewer: Xu He Jie 
Gerrit-Reviewer: oVirt Jenkins CI Server

Change in vdsm[master]: avoid race between load dev rules and trigger the rule

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: avoid race between load dev rules and trigger the rule
......................................................................

Patch Set 5:

Build Started http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/100/ (1/2)

--
To view, visit http://gerrit.ovirt.org/6780
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I718d62e67b0a228f2510233c5dd9d1d94c4a736c
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Royce Lv 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Eduardo 
Gerrit-Reviewer: Mark Wu 
Gerrit-Reviewer: Royce Lv 
Gerrit-Reviewer: Xu He Jie 
Gerrit-Reviewer: oVirt Jenkins CI Server

Change in vdsm[master]: avoid race between load dev rules and trigger the rule

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: avoid race between load dev rules and trigger the rule
......................................................................

Patch Set 5:

Build Started http://jenkins.ovirt.org/job/vdsm_unit_tests_manual_gerrit/134/ (2/2)

--
To view, visit http://gerrit.ovirt.org/6780
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I718d62e67b0a228f2510233c5dd9d1d94c4a736c
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Royce Lv 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Eduardo 
Gerrit-Reviewer: Mark Wu 
Gerrit-Reviewer: Royce Lv 
Gerrit-Reviewer: Xu He Jie 
Gerrit-Reviewer: oVirt Jenkins CI Server

Change in vdsm[master]: avoid race between load dev rules and trigger the rule

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: avoid race between load dev rules and trigger the rule
......................................................................

Patch Set 5:

Build Successful

http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/100/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_unit_tests_manual_gerrit/134/ : SUCCESS

--
To view, visit http://gerrit.ovirt.org/6780
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I718d62e67b0a228f2510233c5dd9d1d94c4a736c
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Royce Lv 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Eduardo 
Gerrit-Reviewer: Mark Wu 
Gerrit-Reviewer: Royce Lv 
Gerrit-Reviewer: Xu He Jie 
Gerrit-Reviewer: oVirt Jenkins CI Server

Change in vdsm[master]: avoid race between load dev rules and trigger the rule

Vered Volansky has posted comments on this change.

Change subject: avoid race between load dev rules and trigger the rule
......................................................................

Patch Set 5: (1 inline comment)

....................................................
File vdsm/storage/hsm.py
Line 2919:         Warning: Internal use only.
Line 2920:         """
Line 2921:         supervdsm.getProxy().appropriateDevice(guid, thiefId)
Line 2922:         #4s is enough for udevd to reload new rules
Line 2923:         time.sleep(4)
It's not a good idea to use timer.sleep. It would be better to use udevadm control --reload-rules inside udevTrigger, so you only block the needed amount of time, if any.
Line 2924:         supervdsm.getProxy().udevTrigger(guid)
Line 2925:         try:
Line 2926:             devPath = '/dev/mapper/%s' % guid
Line 2927:             fileUtils.validateQemuReadable(devPath)

--
To view, visit http://gerrit.ovirt.org/6780
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I718d62e67b0a228f2510233c5dd9d1d94c4a736c
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Royce Lv 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Eduardo 
Gerrit-Reviewer: Mark Wu 
Gerrit-Reviewer: Royce Lv 
Gerrit-Reviewer: Vered Volansky 
Gerrit-Reviewer: Xu He Jie 
Gerrit-Reviewer: oVirt Jenkins CI Server

Change in vdsm[master]: avoid race between load dev rules and trigger the rule

Vered Volansky has posted comments on this change.

Change subject: avoid race between load dev rules and trigger the rule
......................................................................

Patch Set 5: I would prefer that you didn't submit this

--
To view, visit http://gerrit.ovirt.org/6780
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I718d62e67b0a228f2510233c5dd9d1d94c4a736c
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Royce Lv 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Eduardo 
Gerrit-Reviewer: Mark Wu 
Gerrit-Reviewer: Royce Lv 
Gerrit-Reviewer: Vered Volansky 
Gerrit-Reviewer: Xu He Jie 
Gerrit-Reviewer: oVirt Jenkins CI Server

Change in vdsm[master]: avoid race between load dev rules and trigger the rule

Itamar Heim has posted comments on this change.

Change subject: avoid race between load dev rules and trigger the rule
......................................................................

Patch Set 5:

ping?

Change in vdsm[master]: avoid race between load dev rules and trigger the rule

Itamar Heim has posted comments on this change.

Change subject: avoid race between load dev rules and trigger the rule
......................................................................

Patch Set 5:

no reply for 30 days post last pojt

Change in vdsm[master]: avoid race between load dev rules and trigger the rule

Itamar Heim has abandoned this change.

Change subject: avoid race between load dev rules and trigger the rule
......................................................................

Abandoned

no comment for 30 days post last ping. abandoning. please re-open if/when relevant and/or ping reviewers directly / promote discussion about the patch in the mailing list.