Change in vdsm[master]: pad memory volume only when the storage domain is file based
Dan Kenigsberg has posted comments on this change. Change subject: pad memory volume only when the storage domain is file based .. Patch Set 3: Code-Review+2 -- To view, visit http://gerrit.ovirt.org/26407 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I0cebdbba2cc763963b64dcbc142f979802f18c16 Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Arik Hadas Gerrit-Reviewer: Allon Mureinik Gerrit-Reviewer: Arik Hadas Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Eduardo Gerrit-Reviewer: Federico Simoncelli Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Tal Nisan Gerrit-Reviewer: Vinzenz Feenstra 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]: pad memory volume only when the storage domain is file based
Dan Kenigsberg has submitted this change and it was merged. Change subject: pad memory volume only when the storage domain is file based .. pad memory volume only when the storage domain is file based Memory volume should be padded only when the storage domain in which the memory volume resides is file-based device. We used to check the type of the storage pool, assuming that all the domains within the pool are from the same type. This assumption is not true anymore, as we can have different types of storage domains in the same storage pool, in case of shared storage. So from now on, we'll check the type of the storage domain in which the memory volume is in, and only if it is filed-based device we'll pad the volume. Change-Id: I0cebdbba2cc763963b64dcbc142f979802f18c16 Bug-Url: https://bugzilla.redhat.com/1082941 Signed-off-by: Arik Hadas Reviewed-on: http://gerrit.ovirt.org/26407 Reviewed-by: Francesco Romani Reviewed-by: Vinzenz Feenstra Reviewed-by: Federico Simoncelli Reviewed-by: Dan Kenigsberg --- M vdsm/virt/vm.py 1 file changed, 10 insertions(+), 12 deletions(-) Approvals: Federico Simoncelli: Looks good to me, but someone else must approve Vinzenz Feenstra: Looks good to me, but someone else must approve Dan Kenigsberg: Looks good to me, approved Francesco Romani: Looks good to me, but someone else must approve Arik Hadas: Verified -- To view, visit http://gerrit.ovirt.org/26407 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: merged Gerrit-Change-Id: I0cebdbba2cc763963b64dcbc142f979802f18c16 Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Arik Hadas Gerrit-Reviewer: Allon Mureinik Gerrit-Reviewer: Arik Hadas Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Eduardo Gerrit-Reviewer: Federico Simoncelli Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Tal Nisan Gerrit-Reviewer: Vinzenz Feenstra Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: pad memory volume only when the storage domain is file based
Federico Simoncelli has posted comments on this change. Change subject: pad memory volume only when the storage domain is file based .. Patch Set 3: Code-Review+1 Ok for me since we now have two other patches to address the issues. -- To view, visit http://gerrit.ovirt.org/26407 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I0cebdbba2cc763963b64dcbc142f979802f18c16 Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Arik Hadas Gerrit-Reviewer: Allon Mureinik Gerrit-Reviewer: Arik Hadas Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Eduardo Gerrit-Reviewer: Federico Simoncelli Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Tal Nisan Gerrit-Reviewer: Vinzenz Feenstra 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]: pad memory volume only when the storage domain is file based
Arik Hadas has posted comments on this change.
Change subject: pad memory volume only when the storage domain is file based
..
Patch Set 3:
(2 comments)
http://gerrit.ovirt.org/#/c/26407/3/vdsm/virt/vm.py
File vdsm/virt/vm.py:
Line 3550: if sdType == sd.NFS_DOMAIN:
Line 3551: oop.getProcessPool(sdUUID).fileUtils. \
Line 3552: padToBlockSize(memoryVolPath)
Line 3553: else:
Line 3554: fileUtils.padToBlockSize(memoryVolPath)
> we need the actual type then.
posted a patch (initially as Michal commented, but it could be changed later)
for that part: http://gerrit.ovirt.org/#/c/26538/
Line 3555:
Line 3556: snap = xml.dom.minidom.Element('domainsnapshot')
Line 3557: disks = xml.dom.minidom.Element('disks')
Line 3558: newDrives = {}
Line 3685: # This code should be removed once qemu-img will handle
files
Line 3686: # with size that is not multiple of block size correctly.
Line 3687: if memoryParams:
Line 3688: _padMemoryVolume(memoryVolPath,
memoryVol['domainID'])
Line 3689: self.cif.teardownVolumePath(memoryVol)
> ok, I will fix it in a separate patch
Federico, I'm not sure I understand where the teardown is missing, we already
do it when an exception is thrown when invoking the snapshotCreateXML verb
(#3679).
Anyway, a separate patch was posted for it: http://gerrit.ovirt.org/26544
Line 3690:
Line 3691: for drive in newDrives.values(): # Update the drive
information
Line 3692: try:
Line 3693: self.updateDriveParameters(drive)
--
To view, visit http://gerrit.ovirt.org/26407
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I0cebdbba2cc763963b64dcbc142f979802f18c16
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Arik Hadas
Gerrit-Reviewer: Allon Mureinik
Gerrit-Reviewer: Arik Hadas
Gerrit-Reviewer: Dan Kenigsberg
Gerrit-Reviewer: Eduardo
Gerrit-Reviewer: Federico Simoncelli
Gerrit-Reviewer: Francesco Romani
Gerrit-Reviewer: Michal Skrivanek
Gerrit-Reviewer: Tal Nisan
Gerrit-Reviewer: Vinzenz Feenstra
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]: pad memory volume only when the storage domain is file based
Michal Skrivanek has posted comments on this change.
Change subject: pad memory volume only when the storage domain is file based
..
Patch Set 3: -Code-Review
(1 comment)
http://gerrit.ovirt.org/#/c/26407/3/vdsm/virt/vm.py
File vdsm/virt/vm.py:
Line 3550: if sdType == sd.NFS_DOMAIN:
Line 3551: oop.getProcessPool(sdUUID).fileUtils. \
Line 3552: padToBlockSize(memoryVolPath)
Line 3553: else:
Line 3554: fileUtils.padToBlockSize(memoryVolPath)
> Note this line was here in the original code as well - let's fix the wrong
we need the actual type then.
I'd go with "no OOP" altogether. It doesn't make much sense in fact - if we got
here it means the QEMU finished writing GBs of data to the very same volume, we
just want to add few more bytes…it's quite likely we won't block.
Other alternative is to always do OOP which seems to me an overkill
Line 3555:
Line 3556: snap = xml.dom.minidom.Element('domainsnapshot')
Line 3557: disks = xml.dom.minidom.Element('disks')
Line 3558: newDrives = {}
--
To view, visit http://gerrit.ovirt.org/26407
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I0cebdbba2cc763963b64dcbc142f979802f18c16
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Arik Hadas
Gerrit-Reviewer: Allon Mureinik
Gerrit-Reviewer: Arik Hadas
Gerrit-Reviewer: Dan Kenigsberg
Gerrit-Reviewer: Eduardo
Gerrit-Reviewer: Federico Simoncelli
Gerrit-Reviewer: Francesco Romani
Gerrit-Reviewer: Michal Skrivanek
Gerrit-Reviewer: Tal Nisan
Gerrit-Reviewer: Vinzenz Feenstra
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]: pad memory volume only when the storage domain is file based
Allon Mureinik has posted comments on this change.
Change subject: pad memory volume only when the storage domain is file based
..
Patch Set 3:
(1 comment)
http://gerrit.ovirt.org/#/c/26407/3/vdsm/virt/vm.py
File vdsm/virt/vm.py:
Line 3550: if sdType == sd.NFS_DOMAIN:
Line 3551: oop.getProcessPool(sdUUID).fileUtils. \
Line 3552: padToBlockSize(memoryVolPath)
Line 3553: else:
Line 3554: fileUtils.padToBlockSize(memoryVolPath)
> if I recall correctly, I was asked to do it that way because I've been told
Note this line was here in the original code as well - let's fix the wrong
check of storage pool type in /this/ patch, and discuss the OOP usage in a
different one?
Line 3555:
Line 3556: snap = xml.dom.minidom.Element('domainsnapshot')
Line 3557: disks = xml.dom.minidom.Element('disks')
Line 3558: newDrives = {}
--
To view, visit http://gerrit.ovirt.org/26407
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I0cebdbba2cc763963b64dcbc142f979802f18c16
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Arik Hadas
Gerrit-Reviewer: Allon Mureinik
Gerrit-Reviewer: Arik Hadas
Gerrit-Reviewer: Dan Kenigsberg
Gerrit-Reviewer: Eduardo
Gerrit-Reviewer: Federico Simoncelli
Gerrit-Reviewer: Francesco Romani
Gerrit-Reviewer: Michal Skrivanek
Gerrit-Reviewer: Tal Nisan
Gerrit-Reviewer: Vinzenz Feenstra
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]: pad memory volume only when the storage domain is file based
Arik Hadas has posted comments on this change.
Change subject: pad memory volume only when the storage domain is file based
..
Patch Set 3:
(3 comments)
http://gerrit.ovirt.org/#/c/26407/3/vdsm/virt/vm.py
File vdsm/virt/vm.py:
Line 3544: 'elapsedTimeOffset': time.time() -
self._startTime}
Line 3545:
Line 3546: def _padMemoryVolume(memoryVolPath, sdUUID):
Line 3547: sdType = sd.name2type(
Line 3548:
self.cif.irs.getStorageDomainInfo(sdUUID)['info']['type'])
> Let's not use this costly (and possibly failing) call. We already have:
ok, I can switch to use it if we don't need to know the particular type of the
domain in case it is file-based storage (see the comment below), but is it less
costly to access the file than query "getStorageDomainInfo" ?
Line 3549: if sdType in sd.FILE_DOMAIN_TYPES:
Line 3550: if sdType == sd.NFS_DOMAIN:
Line 3551: oop.getProcessPool(sdUUID).fileUtils. \
Line 3552: padToBlockSize(memoryVolPath)
Line 3550: if sdType == sd.NFS_DOMAIN:
Line 3551: oop.getProcessPool(sdUUID).fileUtils. \
Line 3552: padToBlockSize(memoryVolPath)
Line 3553: else:
Line 3554: fileUtils.padToBlockSize(memoryVolPath)
> Why don't we need oop here? What about cifs, posix and gluster?
if I recall correctly, I was asked to do it that way because I've been told
that in NFS the operation might get stuck and in other types of storage the
operation will just fail on error. Edu?
Line 3555:
Line 3556: snap = xml.dom.minidom.Element('domainsnapshot')
Line 3557: disks = xml.dom.minidom.Element('disks')
Line 3558: newDrives = {}
Line 3685: # This code should be removed once qemu-img will handle
files
Line 3686: # with size that is not multiple of block size correctly.
Line 3687: if memoryParams:
Line 3688: _padMemoryVolume(memoryVolPath,
memoryVol['domainID'])
Line 3689: self.cif.teardownVolumePath(memoryVol)
> +1, but please do that in a separate patch!
ok, I will fix it in a separate patch
Line 3690:
Line 3691: for drive in newDrives.values(): # Update the drive
information
Line 3692: try:
Line 3693: self.updateDriveParameters(drive)
--
To view, visit http://gerrit.ovirt.org/26407
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I0cebdbba2cc763963b64dcbc142f979802f18c16
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Arik Hadas
Gerrit-Reviewer: Allon Mureinik
Gerrit-Reviewer: Arik Hadas
Gerrit-Reviewer: Dan Kenigsberg
Gerrit-Reviewer: Eduardo
Gerrit-Reviewer: Federico Simoncelli
Gerrit-Reviewer: Francesco Romani
Gerrit-Reviewer: Michal Skrivanek
Gerrit-Reviewer: Tal Nisan
Gerrit-Reviewer: Vinzenz Feenstra
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]: pad memory volume only when the storage domain is file based
Dan Kenigsberg has posted comments on this change. Change subject: pad memory volume only when the storage domain is file based .. Patch Set 3: -Code-Review (1 comment) http://gerrit.ovirt.org/#/c/26407/3/vdsm/virt/vm.py File vdsm/virt/vm.py: Line 3685: # This code should be removed once qemu-img will handle files Line 3686: # with size that is not multiple of block size correctly. Line 3687: if memoryParams: Line 3688: _padMemoryVolume(memoryVolPath, memoryVol['domainID']) Line 3689: self.cif.teardownVolumePath(memoryVol) > Teardown should happen anyway (also in case of exceptions). +1, but please do that in a separate patch! Line 3690: Line 3691: for drive in newDrives.values(): # Update the drive information Line 3692: try: Line 3693: self.updateDriveParameters(drive) -- To view, visit http://gerrit.ovirt.org/26407 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I0cebdbba2cc763963b64dcbc142f979802f18c16 Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Arik Hadas Gerrit-Reviewer: Allon Mureinik Gerrit-Reviewer: Arik Hadas Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Federico Simoncelli Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Tal Nisan Gerrit-Reviewer: Vinzenz Feenstra 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]: pad memory volume only when the storage domain is file based
Federico Simoncelli has posted comments on this change.
Change subject: pad memory volume only when the storage domain is file based
..
Patch Set 3: Code-Review-1
(3 comments)
http://gerrit.ovirt.org/#/c/26407/3/vdsm/virt/vm.py
File vdsm/virt/vm.py:
Line 3544: 'elapsedTimeOffset': time.time() -
self._startTime}
Line 3545:
Line 3546: def _padMemoryVolume(memoryVolPath, sdUUID):
Line 3547: sdType = sd.name2type(
Line 3548:
self.cif.irs.getStorageDomainInfo(sdUUID)['info']['type'])
Let's not use this costly (and possibly failing) call. We already have:
utils.isBlockDevice(...)
somewhere else.
Line 3549: if sdType in sd.FILE_DOMAIN_TYPES:
Line 3550: if sdType == sd.NFS_DOMAIN:
Line 3551: oop.getProcessPool(sdUUID).fileUtils. \
Line 3552: padToBlockSize(memoryVolPath)
Line 3550: if sdType == sd.NFS_DOMAIN:
Line 3551: oop.getProcessPool(sdUUID).fileUtils. \
Line 3552: padToBlockSize(memoryVolPath)
Line 3553: else:
Line 3554: fileUtils.padToBlockSize(memoryVolPath)
Why don't we need oop here? What about cifs, posix and gluster?
Line 3555:
Line 3556: snap = xml.dom.minidom.Element('domainsnapshot')
Line 3557: disks = xml.dom.minidom.Element('disks')
Line 3558: newDrives = {}
Line 3685: # This code should be removed once qemu-img will handle
files
Line 3686: # with size that is not multiple of block size correctly.
Line 3687: if memoryParams:
Line 3688: _padMemoryVolume(memoryVolPath,
memoryVol['domainID'])
Line 3689: self.cif.teardownVolumePath(memoryVol)
Teardown should happen anyway (also in case of exceptions).
Line 3690:
Line 3691: for drive in newDrives.values(): # Update the drive
information
Line 3692: try:
Line 3693: self.updateDriveParameters(drive)
--
To view, visit http://gerrit.ovirt.org/26407
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I0cebdbba2cc763963b64dcbc142f979802f18c16
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Arik Hadas
Gerrit-Reviewer: Allon Mureinik
Gerrit-Reviewer: Arik Hadas
Gerrit-Reviewer: Dan Kenigsberg
Gerrit-Reviewer: Federico Simoncelli
Gerrit-Reviewer: Francesco Romani
Gerrit-Reviewer: Michal Skrivanek
Gerrit-Reviewer: Tal Nisan
Gerrit-Reviewer: Vinzenz Feenstra
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]: pad memory volume only when the storage domain is file based
Vinzenz Feenstra has posted comments on this change. Change subject: pad memory volume only when the storage domain is file based .. Patch Set 3: The Unit test failures are unrelated to this patch. It's the pep8 change which causes this. -- To view, visit http://gerrit.ovirt.org/26407 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I0cebdbba2cc763963b64dcbc142f979802f18c16 Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Arik Hadas Gerrit-Reviewer: Allon Mureinik Gerrit-Reviewer: Arik Hadas Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Federico Simoncelli Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Tal Nisan Gerrit-Reviewer: Vinzenz Feenstra 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]: pad memory volume only when the storage domain is file based
Vinzenz Feenstra has posted comments on this change. Change subject: pad memory volume only when the storage domain is file based .. Patch Set 3: Code-Review+1 -- To view, visit http://gerrit.ovirt.org/26407 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I0cebdbba2cc763963b64dcbc142f979802f18c16 Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Arik Hadas Gerrit-Reviewer: Allon Mureinik Gerrit-Reviewer: Arik Hadas Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Federico Simoncelli Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Tal Nisan Gerrit-Reviewer: Vinzenz Feenstra 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]: pad memory volume only when the storage domain is file based
oVirt Jenkins CI Server has posted comments on this change. Change subject: pad memory volume only when the storage domain is file based .. Patch Set 3: Build Failed http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/7057/ : FAILURE http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/7959/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/7848/ : SUCCESS -- To view, visit http://gerrit.ovirt.org/26407 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I0cebdbba2cc763963b64dcbc142f979802f18c16 Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Arik Hadas Gerrit-Reviewer: Allon Mureinik Gerrit-Reviewer: Arik Hadas Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Federico Simoncelli Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Tal Nisan Gerrit-Reviewer: Vinzenz Feenstra 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]: pad memory volume only when the storage domain is file based
Arik Hadas has posted comments on this change. Change subject: pad memory volume only when the storage domain is file based .. Patch Set 2: (1 comment) http://gerrit.ovirt.org/#/c/26407/2//COMMIT_MSG Commit Message: Line 8: Line 9: Memory volume should be padded only when the storage domain in which the Line 10: memory volume resides is file-based device. We used to check the type of Line 11: the storage pool, assuming that all the domain within the pool are from Line 12: the same time. This assumption is not true anymore, as we can have > Typo "time" Done Line 13: different types of storage domains in the same storage pool, in case of Line 14: shared storage. So from now on, we'll check the type of the storage Line 15: domain in which the memory volume is in, and only if it is filed-based Line 16: device we'll pad the volume. -- To view, visit http://gerrit.ovirt.org/26407 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I0cebdbba2cc763963b64dcbc142f979802f18c16 Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Arik Hadas Gerrit-Reviewer: Allon Mureinik Gerrit-Reviewer: Arik Hadas Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Federico Simoncelli Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Tal Nisan Gerrit-Reviewer: Vinzenz Feenstra 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]: pad memory volume only when the storage domain is file based
Michal Skrivanek has posted comments on this change. Change subject: pad memory volume only when the storage domain is file based .. Patch Set 2: Code-Review+1 (1 comment) http://gerrit.ovirt.org/#/c/26407/2//COMMIT_MSG Commit Message: Line 8: Line 9: Memory volume should be padded only when the storage domain in which the Line 10: memory volume resides is file-based device. We used to check the type of Line 11: the storage pool, assuming that all the domain within the pool are from Line 12: the same time. This assumption is not true anymore, as we can have Typo "time" Line 13: different types of storage domains in the same storage pool, in case of Line 14: shared storage. So from now on, we'll check the type of the storage Line 15: domain in which the memory volume is in, and only if it is filed-based Line 16: device we'll pad the volume. -- To view, visit http://gerrit.ovirt.org/26407 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I0cebdbba2cc763963b64dcbc142f979802f18c16 Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Arik Hadas Gerrit-Reviewer: Allon Mureinik Gerrit-Reviewer: Arik Hadas Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Tal Nisan Gerrit-Reviewer: Vinzenz Feenstra 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]: pad memory volume only when the storage domain is file based
Francesco Romani has posted comments on this change. Change subject: pad memory volume only when the storage domain is file based .. Patch Set 2: Code-Review+1 -- To view, visit http://gerrit.ovirt.org/26407 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I0cebdbba2cc763963b64dcbc142f979802f18c16 Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Arik Hadas Gerrit-Reviewer: Allon Mureinik Gerrit-Reviewer: Arik Hadas Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Tal Nisan Gerrit-Reviewer: Vinzenz Feenstra 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]: pad memory volume only when the storage domain is file based
Dan Kenigsberg has posted comments on this change. Change subject: pad memory volume only when the storage domain is file based .. Patch Set 2: Code-Review+1 -- To view, visit http://gerrit.ovirt.org/26407 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I0cebdbba2cc763963b64dcbc142f979802f18c16 Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Arik Hadas Gerrit-Reviewer: Allon Mureinik Gerrit-Reviewer: Arik Hadas Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Tal Nisan Gerrit-Reviewer: Vinzenz Feenstra 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]: pad memory volume only when the storage domain is file based
Arik Hadas has posted comments on this change. Change subject: pad memory volume only when the storage domain is file based .. Patch Set 2: Verified+1 -- To view, visit http://gerrit.ovirt.org/26407 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I0cebdbba2cc763963b64dcbc142f979802f18c16 Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Arik Hadas Gerrit-Reviewer: Arik Hadas Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Vinzenz Feenstra 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]: pad memory volume only when the storage domain is file based
oVirt Jenkins CI Server has posted comments on this change. Change subject: pad memory volume only when the storage domain is file based .. Patch Set 2: Build Failed http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/7049/ : FAILURE http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/7951/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/7840/ : SUCCESS -- To view, visit http://gerrit.ovirt.org/26407 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I0cebdbba2cc763963b64dcbc142f979802f18c16 Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Arik Hadas Gerrit-Reviewer: Michal Skrivanek 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]: pad memory volume only when the storage domain is file based
oVirt Jenkins CI Server has posted comments on this change. Change subject: pad memory volume only when the storage domain is file based .. Patch Set 1: Build Failed http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/7048/ : FAILURE http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/7950/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/7839/ : SUCCESS -- To view, visit http://gerrit.ovirt.org/26407 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I0cebdbba2cc763963b64dcbc142f979802f18c16 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Arik Hadas 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]: pad memory volume only when the storage domain is file based
Arik Hadas has uploaded a new change for review.
Change subject: pad memory volume only when the storage domain is file based
..
pad memory volume only when the storage domain is file based
Memory volume should be padded only when the storage domain in which the
memory volume resides is file-based device. We used to check the type of
the storage pool, assuming that all the domain within the pool are from
the same time. This assumption is not true anymore, as we can have
different types of storage domains in the same storage pool, in case of
shared storage. So from now on, we'll check the type of the storage
domain in which the memory volume is in, and only if it is filed-based
device we'll pad the volume.
Change-Id: I0cebdbba2cc763963b64dcbc142f979802f18c16
Signed-off-by: Arik Hadas
---
M vdsm/virt/vm.py
1 file changed, 10 insertions(+), 12 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/07/26407/1
diff --git a/vdsm/virt/vm.py b/vdsm/virt/vm.py
index 0e19235..6711bc6 100644
--- a/vdsm/virt/vm.py
+++ b/vdsm/virt/vm.py
@@ -3543,12 +3543,15 @@
'_srcDomXML': self._dom.XMLDesc(0),
'elapsedTimeOffset': time.time() - self._startTime}
-def _padMemoryVolume(memoryVolPath, spType, sdUUId):
-if spType == sd.NFS_DOMAIN:
-oop.getProcessPool(sdUUID).fileUtils. \
-padToBlockSize(memoryVolPath)
-else:
-fileUtils.padToBlockSize(memoryVolPath)
+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)
snap = xml.dom.minidom.Element('domainsnapshot')
disks = xml.dom.minidom.Element('disks')
@@ -3682,12 +3685,7 @@
# This code should be removed once qemu-img will handle files
# with size that is not multiple of block size correctly.
if memoryParams:
-sdUUID = memoryVol['domainID']
-spUUID = memoryVol['poolID']
-spType = sd.name2type(
-self.cif.irs.getStoragePoolInfo(spUUID)['info']['type'])
-if spType in sd.FILE_DOMAIN_TYPES:
-_padMemoryVolume(memoryVolPath, spType, sdUUID)
+_padMemoryVolume(memoryVolPath, memoryVol['domainID'])
self.cif.teardownVolumePath(memoryVol)
for drive in newDrives.values(): # Update the drive information
--
To view, visit http://gerrit.ovirt.org/26407
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I0cebdbba2cc763963b64dcbc142f979802f18c16
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Arik Hadas
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
