Change in vdsm[master]: do not use OOP for padding snapshot's memory volume

2014-05-21 Thread danken
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

2014-05-19 Thread ahadas
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

2014-05-19 Thread oVirt Jenkins CI Server
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

2014-04-08 Thread ahadas
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

2014-04-08 Thread oVirt Jenkins CI Server
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

2014-04-08 Thread Federico Simoncelli
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

2014-04-08 Thread michal . skrivanek
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

2014-04-08 Thread danken
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

2014-04-08 Thread michal . skrivanek
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