Dan Kenigsberg has posted comments on this change. Change subject: contrib: Adding contrib/nfs-check.py ......................................................................
Patch Set 2: I would prefer that you didn't submit this (2 inline comments) a good start, but mount should be as root, but writing - as vdsm:kvm. The point of the script is to try second-guess why mount has failed - it should check whether the export exists, its ownership and mode, etc. I got Cannot execute the command: mount -t nfs localhost:/export/ext1 /mnt mount.nfs: access denied by server while mounting localhost:/export/ext1 which does not bring me newbie any closer to understanding why nfs is not working for me. .................................................... File contrib/nfs-check.py Line 25: print "Usage: " + sys.argv[0] + " [OPTIONS]" I'd prefer a simpler cmdline of server:/targer with optional local (create a dir under /tmp and remove it later) Line 41: process = subprocess.Popen(cmd, shell=True, stdout=subprocess.PIPE, stderr=subprocess.PIPE) using shell=True is a bad practice. it opens the door for nasty code injection. it's not the end of the world for this script, but still. -- To view, visit http://gerrit.ovirt.org/916 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I4d3231077fb59e11be1d61a578a96a3988559ff3 Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Douglas Schilling Landgraf <[email protected]> Gerrit-Reviewer: Dan Kenigsberg <[email protected]> Gerrit-Reviewer: Douglas Schilling Landgraf <[email protected]> _______________________________________________ vdsm-patches mailing list [email protected] https://fedorahosted.org/mailman/listinfo/vdsm-patches
