Dan Kenigsberg has posted comments on this change. Change subject: mkimage: setup right permissions before mkisofs ......................................................................
Patch Set 3: (2 comments) https://gerrit.ovirt.org/#/c/48538/3/vdsm/mkimage.py File vdsm/mkimage.py: Line 131: # pre-create the destination iso path with the right permissions; Line 132: # mkisofs/genisoimage will truncate the content and keep the Line 133: # permissions. Line 134: fd = os.open(isopath, os.O_CREAT | os.O_RDONLY | os.O_EXCL, 0o640) Line 135: os.close(fd) > This will fail if the file already exists. ok, let's be more cautious. if exists(isopath): log.warning() utils.rmFile() Line 136: Line 137: rc, out, err = execCmd(command, raw=True) Line 138: if rc: Line 139: # we want to use O_CREAT | O_EXCL, so we need to cleanup Line 135: os.close(fd) Line 136: Line 137: rc, out, err = execCmd(command, raw=True) Line 138: if rc: Line 139: # we want to use O_CREAT | O_EXCL, so we need to cleanup > No, this is not good enough to ensure that create will succeed. We clean he AFAIU Nir suggests to change to comment to # clean up after ourselves in case of error Line 140: removeFs(isopath) Line 141: # skip _commonCleanFs step for missing iso Line 142: isopath = None Line 143: -- To view, visit https://gerrit.ovirt.org/48538 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ibc971a39ae2a8eaf7af934bb0922647e4676d03f Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani <[email protected]> Gerrit-Reviewer: Dan Kenigsberg <[email protected]> Gerrit-Reviewer: Francesco Romani <[email protected]> Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer <[email protected]> Gerrit-Reviewer: Yaniv Bronhaim <[email protected]> Gerrit-Reviewer: gerrit-hooks <[email protected]> Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list [email protected] https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
