Change in vdsm[master]: vm: safer removal of the recovery file

mzamazal at redhat.com mzamazal at redhat.com
Wed Jan 13 12:08:37 UTC 2016


Milan Zamazal has posted comments on this change.

Change subject: vm: safer removal of the recovery file
......................................................................


Patch Set 3:

(1 comment)

https://gerrit.ovirt.org/#/c/51387/3/vdsm/virt/vm.py
File vdsm/virt/vm.py:

Line 827:             with tempfile.NamedTemporaryFile(dir=constants.P_VDSM_RUN,
Line 828:                                              delete=False) as f:
Line 829:                 pickle.dump(vmState, f)
Line 830: 
Line 831:             os.rename(f.name, self._recoveryFile)
> good point, either store reference inside the lock or extend the critical r
> either store reference inside the lock or extend the critical region.

Storing the reference is not enough because if cleanup gets called in the meantime, we get stale recovery file again. So either the critical region must be extended or there must be additional check + file delete after the file is stored.
Line 832:         else:
Line 833:             self.log.debug('saveState after cleanup for status=%s',
Line 834:                            self.lastStatus)
Line 835: 


-- 
To view, visit https://gerrit.ovirt.org/51387
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I9a8d2b34599da17bee35c9bb694122ecd9d58068
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani <fromani at redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani at redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik <mpolednik at redhat.com>
Gerrit-Reviewer: Milan Zamazal <mzamazal at redhat.com>
Gerrit-Reviewer: Vinzenz Feenstra <vfeenstr at redhat.com>
Gerrit-Reviewer: gerrit-hooks <automation at ovirt.org>
Gerrit-HasComments: Yes


More information about the vdsm-patches mailing list