Nir Soffer has posted comments on this change. Change subject: v2v: cleanup v2v directory leftovers ......................................................................
Patch Set 2: Code-Review-1 (7 comments) https://gerrit.ovirt.org/#/c/39789/2/vdsm/clientIF.py File vdsm/clientIF.py: Line 105 Line 106 Line 107 Line 108 Line 109 Perform the v2v cleanup here. Line 112: port = config.getint('addresses', 'management_port') Line 113: self._createAcceptor(host, port) Line 114: self._prepareXMLRPCBinding() Line 115: self._prepareJSONRPCBinding() Line 116: v2v.clean_leftovers() This is racy, you must complete the cleanup *before* vdsm is responding to requests. Line 117: except: Line 118: self.log.error('failed to init clientIF, ' Line 119: 'shutting down storage dispatcher') Line 120: if self.irs: https://gerrit.ovirt.org/#/c/39789/2/vdsm/v2v.py File vdsm/v2v.py: Line 212: } Line 213: return ret Line 214: Line 215: Line 216: def clean_leftovers(): Log info "cleaning leftovers" would be nice Line 217: for f in glob('%s/*.ovf' % _V2V_DIR): Line 218: try: Line 219: os.remove(f) Line 220: except Exception as e: Line 213: return ret Line 214: Line 215: Line 216: def clean_leftovers(): Line 217: for f in glob('%s/*.ovf' % _V2V_DIR): > true, Please add log.debug for each file you remove. We should know about every change we make to the state of the machine. Line 218: try: Line 219: os.remove(f) Line 220: except Exception as e: Line 221: logging.warn('Cannot remove file: %s, err: %s', f, e) Line 217: for f in glob('%s/*.ovf' % _V2V_DIR): Line 218: try: Line 219: os.remove(f) Line 220: except Exception as e: Line 221: logging.warn('Cannot remove file: %s, err: %s', f, e) Should be logging.error Line 222: Line 223: for f in glob('%s/*.tmp' % _V2V_DIR): Line 224: try: Line 225: os.remove(f) Line 219: os.remove(f) Line 220: except Exception as e: Line 221: logging.warn('Cannot remove file: %s, err: %s', f, e) Line 222: Line 223: for f in glob('%s/*.tmp' % _V2V_DIR): We need a warning if we find a stale password file when vdsm starts - this may be a very bad bug if it was not caused by killing vdsm. Add log.warning for each found file. Line 224: try: Line 225: os.remove(f) Line 226: except Exception as e: Line 227: logging.warn('Cannot remove file: %s, err: %s', f, e) Line 223: for f in glob('%s/*.tmp' % _V2V_DIR): Line 224: try: Line 225: os.remove(f) Line 226: except Exception as e: Line 227: logging.warn('Cannot remove file: %s, err: %s', f, e) Should be logging.error Line 228: Line 229: Line 230: def _read_ovf(job_id): Line 231: file_name = os.path.join(_V2V_DIR, "%s.ovf" % job_id) -- To view, visit https://gerrit.ovirt.org/39789 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ic2f5b63863f4f7dc85d6af0ca015099274715441 Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Shahar Havivi <[email protected]> Gerrit-Reviewer: Dan Kenigsberg <[email protected]> Gerrit-Reviewer: Francesco Romani <[email protected]> Gerrit-Reviewer: Michal Skrivanek <[email protected]> Gerrit-Reviewer: Nir Soffer <[email protected]> Gerrit-Reviewer: Shahar Havivi <[email protected]> Gerrit-Reviewer: [email protected] Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list [email protected] https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
