Change in vdsm[master]: do not use OOP for padding snapshot's memory volume
Dan Kenigsberg has posted comments on this change. Change subject: do not use OOP for padding snapshot's memory volume .. Patch Set 2: Code-Review-2 (1 comment) http://gerrit.ovirt.org/#/c/26538/2//COMMIT_MSG Commit Message: Line 8: Line 9: The padding is made right after libvirt dumps the memory content into Line 10: the volume, so the volumes is accessible and writable - doing the Line 11: padding in different process looks like an expensive operation which is Line 12: not required in that case. Thus, all the padding will now be made As I said, this is wrong. All you know is that the volume WAS accessible and writable. It may no longer be in that state. All writes to NFS must happen from another process. Otherwise, if the storage disappear, the whole of Vdsm would freeze in D state. It does not matter that qemu just finished writing to there - the race condition is small but it exists. It does not matter that libvirt may have a similar bug. (well, it does matter, but we should fix it there, not reinroduce it here). I do not know enough about the kernel implementation of other file storage domains; they may well have similar bugs, but nowhere is Vdsm do we handle these bugs. In NFS we saw the bugs and do not want to see it again. Line 13: without OOP. Line 14: Line 15: Change-Id: I2a94354e188019f3afd209633979ec5a5b35293b -- To view, visit http://gerrit.ovirt.org/26538 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I2a94354e188019f3afd209633979ec5a5b35293b Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Arik Hadas aha...@redhat.com Gerrit-Reviewer: Arik Hadas aha...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Francesco Romani from...@redhat.com Gerrit-Reviewer: Michal Skrivanek michal.skriva...@redhat.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: do not use OOP for padding snapshot's memory volume
Arik Hadas has posted comments on this change. Change subject: do not use OOP for padding snapshot's memory volume .. Patch Set 1: (1 comment) http://gerrit.ovirt.org/#/c/26538/1//COMMIT_MSG Commit Message: Line 3: AuthorDate: 2014-04-08 13:58:13 +0300 Line 4: Commit: Arik Hadas aha...@redhat.com Line 5: CommitDate: 2014-04-08 13:58:13 +0300 Line 6: Line 7: do not use OOP for padding snapshot's memory volume How could it disappear when QEMU just finished writing data to it? I added an explanation to the commit msg, Federico Dan - do you agree? Line 8: Line 9: Change-Id: I2a94354e188019f3afd209633979ec5a5b35293b -- To view, visit http://gerrit.ovirt.org/26538 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I2a94354e188019f3afd209633979ec5a5b35293b Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Arik Hadas aha...@redhat.com Gerrit-Reviewer: Arik Hadas aha...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Francesco Romani from...@redhat.com Gerrit-Reviewer: Michal Skrivanek michal.skriva...@redhat.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: do not use OOP for padding snapshot's memory volume
oVirt Jenkins CI Server has posted comments on this change. Change subject: do not use OOP for padding snapshot's memory volume .. Patch Set 2: Build Failed http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/8985/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit/9126/ : FAILURE http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/8197/ : SUCCESS -- To view, visit http://gerrit.ovirt.org/26538 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I2a94354e188019f3afd209633979ec5a5b35293b Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Arik Hadas aha...@redhat.com Gerrit-Reviewer: Arik Hadas aha...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Francesco Romani from...@redhat.com Gerrit-Reviewer: Michal Skrivanek michal.skriva...@redhat.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: do not use OOP for padding snapshot's memory volume
Arik Hadas has uploaded a new change for review. Change subject: do not use OOP for padding snapshot's memory volume .. do not use OOP for padding snapshot's memory volume Change-Id: I2a94354e188019f3afd209633979ec5a5b35293b Signed-off-by: Arik Hadas aha...@redhat.com --- M vdsm/virt/vm.py 1 file changed, 3 insertions(+), 9 deletions(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/38/26538/1 diff --git a/vdsm/virt/vm.py b/vdsm/virt/vm.py index 6711bc6..b8ce533 100644 --- a/vdsm/virt/vm.py +++ b/vdsm/virt/vm.py @@ -3543,15 +3543,9 @@ '_srcDomXML': self._dom.XMLDesc(0), 'elapsedTimeOffset': time.time() - self._startTime} -def _padMemoryVolume(memoryVolPath, sdUUID): -sdType = sd.name2type( -self.cif.irs.getStorageDomainInfo(sdUUID)['info']['type']) -if sdType in sd.FILE_DOMAIN_TYPES: -if sdType == sd.NFS_DOMAIN: -oop.getProcessPool(sdUUID).fileUtils. \ -padToBlockSize(memoryVolPath) -else: -fileUtils.padToBlockSize(memoryVolPath) +def _padMemoryVolume(memoryVolPath): +if not utils.isBlockDevice(memoryVolPath): +fileUtils.padToBlockSize(memoryVolPath) snap = xml.dom.minidom.Element('domainsnapshot') disks = xml.dom.minidom.Element('disks') -- To view, visit http://gerrit.ovirt.org/26538 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I2a94354e188019f3afd209633979ec5a5b35293b Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Arik Hadas aha...@redhat.com ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: do not use OOP for padding snapshot's memory volume
oVirt Jenkins CI Server has posted comments on this change. Change subject: do not use OOP for padding snapshot's memory volume .. Patch Set 1: Build Successful http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/7902/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/7112/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/8014/ : SUCCESS -- To view, visit http://gerrit.ovirt.org/26538 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I2a94354e188019f3afd209633979ec5a5b35293b Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Arik Hadas aha...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Michal Skrivanek michal.skriva...@redhat.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: do not use OOP for padding snapshot's memory volume
Federico Simoncelli has posted comments on this change. Change subject: do not use OOP for padding snapshot's memory volume .. Patch Set 1: (1 comment) http://gerrit.ovirt.org/#/c/26538/1//COMMIT_MSG Commit Message: Line 3: AuthorDate: 2014-04-08 13:58:13 +0300 Line 4: Commit: Arik Hadas aha...@redhat.com Line 5: CommitDate: 2014-04-08 13:58:13 +0300 Line 6: Line 7: do not use OOP for padding snapshot's memory volume Why? Line 8: Line 9: Change-Id: I2a94354e188019f3afd209633979ec5a5b35293b -- To view, visit http://gerrit.ovirt.org/26538 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I2a94354e188019f3afd209633979ec5a5b35293b Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Arik Hadas aha...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Michal Skrivanek michal.skriva...@redhat.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: do not use OOP for padding snapshot's memory volume
Michal Skrivanek has posted comments on this change. Change subject: do not use OOP for padding snapshot's memory volume .. Patch Set 1: (1 comment) http://gerrit.ovirt.org/#/c/26538/1//COMMIT_MSG Commit Message: Line 3: AuthorDate: 2014-04-08 13:58:13 +0300 Line 4: Commit: Arik Hadas aha...@redhat.com Line 5: CommitDate: 2014-04-08 13:58:13 +0300 Line 6: Line 7: do not use OOP for padding snapshot's memory volume Why? I'd say it's not needed since the QEMU just finished writing all the RAM content to that file…so it's quite likely it's accessible since we get so far. Other alternative would be to use OOP for all types…but I'm not really sure it's needed Line 8: Line 9: Change-Id: I2a94354e188019f3afd209633979ec5a5b35293b -- To view, visit http://gerrit.ovirt.org/26538 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I2a94354e188019f3afd209633979ec5a5b35293b Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Arik Hadas aha...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Michal Skrivanek michal.skriva...@redhat.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: do not use OOP for padding snapshot's memory volume
Dan Kenigsberg has posted comments on this change. Change subject: do not use OOP for padding snapshot's memory volume .. Patch Set 1: Code-Review-1 (1 comment) http://gerrit.ovirt.org/#/c/26538/1//COMMIT_MSG Commit Message: Line 3: AuthorDate: 2014-04-08 13:58:13 +0300 Line 4: Commit: Arik Hadas aha...@redhat.com Line 5: CommitDate: 2014-04-08 13:58:13 +0300 Line 6: Line 7: do not use OOP for padding snapshot's memory volume I'd say it's not needed since the QEMU just finished writing all the RAM co Without oop we have a chance that a single disappearing NFS export moves all of Vdsm into D state. This must be maintained. Line 8: Line 9: Change-Id: I2a94354e188019f3afd209633979ec5a5b35293b -- To view, visit http://gerrit.ovirt.org/26538 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I2a94354e188019f3afd209633979ec5a5b35293b Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Arik Hadas aha...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Michal Skrivanek michal.skriva...@redhat.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: do not use OOP for padding snapshot's memory volume
Michal Skrivanek has posted comments on this change. Change subject: do not use OOP for padding snapshot's memory volume .. Patch Set 1: (1 comment) http://gerrit.ovirt.org/#/c/26538/1//COMMIT_MSG Commit Message: Line 3: AuthorDate: 2014-04-08 13:58:13 +0300 Line 4: Commit: Arik Hadas aha...@redhat.com Line 5: CommitDate: 2014-04-08 13:58:13 +0300 Line 6: Line 7: do not use OOP for padding snapshot's memory volume Without oop we have a chance that a single disappearing NFS export moves al How could it disappear when QEMU just finished writing data to it? We're not protecting the libvirt call either, btw. Anyway, if we do want to cover ourselves in this case as well, OOP would always be better, not just for NFS. Line 8: Line 9: Change-Id: I2a94354e188019f3afd209633979ec5a5b35293b -- To view, visit http://gerrit.ovirt.org/26538 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I2a94354e188019f3afd209633979ec5a5b35293b Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Arik Hadas aha...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Michal Skrivanek michal.skriva...@redhat.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches