Federico Simoncelli has posted comments on this change.
Change subject: Add NFS storage domain functional tests
......................................................................
Patch Set 5: Looks good to me, but someone else must approve
(1 inline comment)
Minor comment
....................................................
File tests/functional/xmlrpcTests.py
Line 610: exports = listNFS()
Line 611: for line in exports:
Line 612: export = line.split(" ", 1)[0]
Line 613: if fnmatch.fnmatch(export, pathPattern):
Line 614: unexportNFS(export)
You're not checking the unexportNFS rc. Just to make it clear, if it fails
you're probably going to wipe everything on the mounts in the next lines.
I know it's a test, but still this should at least deserve a comment if it's an
issue that we don't want to address now (or if it's correct).
Maybe this is somehow better?
if fnmatch.fnmatch(export, pathPattern):
rc = unexportNFS(export)
if rc == 0:
os.rmdir(export)
else:
# warnings
Line 615: for leftover in glob.glob(pathPattern):
Line 616: shutil.rmtree(leftover, ignore_errors=True)
Line 617:
Line 618:
--
To view, visit http://gerrit.ovirt.org/13105
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic9c24369af552b5d58e0f5704075d6f351792775
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Zhou Zheng Sheng <[email protected]>
Gerrit-Reviewer: Ayal Baron <[email protected]>
Gerrit-Reviewer: Dan Kenigsberg <[email protected]>
Gerrit-Reviewer: Daniel Erez <[email protected]>
Gerrit-Reviewer: Deepak C Shetty <[email protected]>
Gerrit-Reviewer: Eduardo <[email protected]>
Gerrit-Reviewer: Federico Simoncelli <[email protected]>
Gerrit-Reviewer: Saggi Mizrahi <[email protected]>
Gerrit-Reviewer: Yeela Kaplan <[email protected]>
Gerrit-Reviewer: Zhou Zheng Sheng <[email protected]>
Gerrit-Reviewer: oVirt Jenkins CI Server
_______________________________________________
vdsm-patches mailing list
[email protected]
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
