Change in vdsm[master]: sparsify: integrating virt-sparsify into vdsm
by Federico Simoncelli
Federico Simoncelli has posted comments on this change.
Change subject: sparsify: integrating virt-sparsify into vdsm
......................................................................
Patch Set 2:
(9 comments)
Overall it's good. Thanks Utkarsh! We'll have to do some minor changes here and there for the block storage domains but nothing major. Keep up the good work!
http://gerrit.ovirt.org/#/c/28328/2//COMMIT_MSG
Commit Message:
Line 14: Since virt-sparsify included with CentOS does not allow in-place
Line 15: sparsification, a temporary image file is created when sparsify is
Line 16: being executed. If sparsification is successful, the temporary file is
Line 17: swapped with the original image file. If the swapping is successful
Line 18: the original image is removed.
This won't work on block domains. I also prefer engine to orchestrate the flow:
* engine sends a new createVolume to prepare a new volume as destination
* engine sends sparsifyImage specifying source and (the newly created) destination (here we need few checks to make sure that destination is usable, e.g. correct format, etc.)
* on completion engine sends (optionally) a deleteImage on the source
In this way it's also possible to test the new image before deleting the source.
Line 19:
Line 20: If at any point the sparsify task fails, a rollback task has been
Line 21: implemented which depending upon if a rollback is possible, will
Line 22: attempt to restore the image to a state before attempting sparsify.
http://gerrit.ovirt.org/#/c/28328/2/vdsm/BindingXMLRPC.py
File vdsm/BindingXMLRPC.py:
Line 669: op, postZero=False, force=False):
Line 670: image = API.Image(imgUUID, spUUID, srcDomUUID)
Line 671: return image.move(dstDomUUID, op, postZero, force)
Line 672:
Line 673: def imageSparsify(self, spUUID, sdUUID, imgUUID):
The API should be documented in the json file. Given my previous comment on source/destination I think this will become something like:
imageSparsify(self, spUUID, srcSdUUID, srcImgUUID, srcVolUUID, dstSdUUID, dstImgUUID, dstVolUUID)
dstVolUUID could be optional but I'd prefer to make it explicit.
Line 674: image = API.Image(imgUUID, spUUID, sdUUID)
Line 675: return image.sparsify()
Line 676:
Line 677: def imageCloneStructure(self, spUUID, sdUUID, imgUUID, dstSdUUID):
http://gerrit.ovirt.org/#/c/28328/2/vdsm/storage/hsm.py
File vdsm/storage/hsm.py:
Line 1596: Reduce sparse image size by converting free space on image to free
Line 1597: space on storage domain using virt-sparsify.
Line 1598: """
Line 1599:
Line 1600: pool = self.getPool(spUUID)
Shared lock on storage domain and then lock on image source and destination.
Line 1601: self._spmSchedule(spUUID, "sparsifyImage", pool.sparsifyImage, sdUUID,
Line 1602: imgUUID)
Line 1603:
Line 1604: @public
http://gerrit.ovirt.org/#/c/28328/2/vdsm/storage/image.py
File vdsm/storage/image.py:
Line 521:
Line 522: #TODO: self.isLegal(sdUUID, imgUUID)?
Line 523:
Line 524: chain = self.getChain(sdUUID, imgUUID)
Line 525: leafVol = chain[-1]
Making the volUUID explicit in the API will save us from doing the lookup.
Line 526:
Line 527: if not leafVol.isLeaf():
Line 528: self.log.error("Cannot determine leaf volume for image %s in "
Line 529: "storage domain %s", imgUUID, sdUUID)
http://gerrit.ovirt.org/#/c/28328/2/vdsm/storage/volume.py
File vdsm/storage/volume.py:
Line 583: Reduce sparse volume size by converting free space on volume to free
Line 584: space on storage domain using virt-sparsify.
Line 585: """
Line 586: if not self.isLeaf() or self.isShared():
Line 587: raise se.VolumeNonWritable(self.volUUID)
Nice, we can get rid of this because the destination is another image and we are not risking to corrupt anything.
Line 588:
Line 589: volFormat = self.getFormat()
Line 590: if volFormat != RAW_FORMAT and volFormat != COW_FORMAT:
Line 591: raise se.IncorrectFormat(self.volUUID)
Line 587: raise se.VolumeNonWritable(self.volUUID)
Line 588:
Line 589: volFormat = self.getFormat()
Line 590: if volFormat != RAW_FORMAT and volFormat != COW_FORMAT:
Line 591: raise se.IncorrectFormat(self.volUUID)
This is overly pedantic, do we have this somewhere else?
Line 592:
Line 593: if not self.isSparse():
Line 594: raise se.VolumeNotSparse(self.volUUID)
Line 595:
Line 597:
Line 598: path = self.getVolumePath()
Line 599:
Line 600: try:
Line 601: vars.task.pushRecovery(
I haven't looked into this but it seems to me that this will be redundant after the other changes (and it can be removed together with sparsifyRollback).
Line 602: task.Recovery("Sparsify volume rollback: %s" % self.volUUID,
Line 603: clsModule, clsName, "sparsifyRollback", [path])
Line 604: )
Line 605:
Line 1054: """
Line 1055: pass
Line 1056:
Line 1057:
Line 1058: def virtSparsify(vol, stop):
Let's leave this here for now but I think that we'll have to move it before merging the patch.
Line 1059: """
Line 1060: Sparsify the 'vol' volume using libguestfs virt-sparsify
Line 1061: """
Line 1062: log.debug('(virtSparsify): virt-sparsify %s START' % vol)
Line 1057:
Line 1058: def virtSparsify(vol, stop):
Line 1059: """
Line 1060: Sparsify the 'vol' volume using libguestfs virt-sparsify
Line 1061: """
There's probably more to review in this function here but I'll leave it for the next round.
Line 1062: log.debug('(virtSparsify): virt-sparsify %s START' % vol)
Line 1063:
Line 1064: vol_split = os.path.splitext(vol)
Line 1065: dst_tmp = "%s_sparsify_tmp%s" % (vol_split[0], vol_split[1])
--
To view, visit http://gerrit.ovirt.org/28328
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Id7bd2b4b6d45781fa27a128dd68d14b7561d0901
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Utkarsh Singh <utkarshsins(a)gmail.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
9 years, 11 months
Change in vdsm[master]: json-rpc: Protocol detection
by Nir Soffer
Nir Soffer has posted comments on this change.
Change subject: json-rpc: Protocol detection
......................................................................
Patch Set 32:
Please specify which engine versions are were tested with this patch, and which features were tested with each.
--
To view, visit http://gerrit.ovirt.org/26300
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Id739a40e2b37dcc175137ec91cd5ec166ad24a75
Gerrit-PatchSet: 32
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Piotr Kliczewski <piotr.kliczewski(a)gmail.com>
Gerrit-Reviewer: Barak Azulay <bazulay(a)redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com>
Gerrit-Reviewer: Liron Ar <laravot(a)redhat.com>
Gerrit-Reviewer: Michal Skrivanek <michal.skrivanek(a)redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: Piotr Kliczewski <piotr.kliczewski(a)gmail.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com>
Gerrit-Reviewer: Vinzenz Feenstra <vfeenstr(a)redhat.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com>
Gerrit-Reviewer: Yeela Kaplan <ykaplan(a)redhat.com>
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
9 years, 11 months
Change in vdsm[master]: vm: move getBalloonInfo in the stats thread
by fromani@redhat.com
Francesco Romani has uploaded a new change for review.
Change subject: vm: move getBalloonInfo in the stats thread
......................................................................
vm: move getBalloonInfo in the stats thread
To gather the ballon stats, we used to make a direct libvirt
call in the getStats() body. This is dangerous, because
getStats() should not contain costly calls, and should
just gather data.
This patch reimplements the balloon information getter as a
sampling function, and moves it into the VmStatsThread
to make things more robust.
As remainder to avoid to sneak costly calls in getStats,
this patch also adds a docstring in getStatsInternal.
Change-Id: I51fd1268b3718f3cb0c1bb32b13f38a58a67b8be
Signed-off-by: Francesco Romani <fromani(a)redhat.com>
---
M tests/vmTests.py
M vdsm/virt/vm.py
2 files changed, 43 insertions(+), 25 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/96/27696/1
diff --git a/tests/vmTests.py b/tests/vmTests.py
index 1dd75ae..13e3f5a 100644
--- a/tests/vmTests.py
+++ b/tests/vmTests.py
@@ -912,7 +912,9 @@
# bz1073478 - main case
with FakeVM(self.VM_PARAMS, self.DEV_BALLOON) as fake:
self.assertEqual(fake._dom, None)
- res = fake.getStats()
+ mock_stats_thread = vm.VmStatsThread(fake)
+ res = {}
+ mock_stats_thread._getBalloonStats(res)
self.assertIn('balloonInfo', res)
self.assertIn('balloon_cur', res['balloonInfo'])
@@ -920,7 +922,9 @@
# bz1073478 - extra case
with FakeVM(self.VM_PARAMS, self.DEV_BALLOON) as fake:
fake._dom = FakeDomain()
- res = fake.getStats()
+ mock_stats_thread = vm.VmStatsThread(fake)
+ res = {}
+ mock_stats_thread._getBalloonStats(res)
self.assertIn('balloonInfo', res)
self.assertIn('balloon_cur', res['balloonInfo'])
@@ -994,6 +998,7 @@
vmParams = {
'displayPort': -1, 'displaySecurePort': -1, 'display': 'qxl',
'displayIp': '127.0.0.1', 'vmType': 'kvm', 'devices': {},
+ 'memSize': 1024,
# HACKs
'pauseCode': 'NOERR'}
with FakeVM(vmParams) as fake:
diff --git a/vdsm/virt/vm.py b/vdsm/virt/vm.py
index f1c36f8..13ceba4 100644
--- a/vdsm/virt/vm.py
+++ b/vdsm/virt/vm.py
@@ -203,10 +203,14 @@
self._sampleNet,
config.getint('vars', 'vm_sample_net_interval'),
config.getint('vars', 'vm_sample_net_window')))
+ self.sampleBalloon = (
+ sampling.AdvancedStatsFunction(
+ self._sampleBalloon))
self.addStatsFunction(
self.highWrite, self.updateVolumes, self.sampleCpu,
- self.sampleDisk, self.sampleDiskLatency, self.sampleNet)
+ self.sampleDisk, self.sampleDiskLatency, self.sampleNet,
+ self.sampleBalloon)
def _highWrite(self):
if not self._vm.isDisksStatsCollectionEnabled():
@@ -257,6 +261,10 @@
netSamples[nic.name] = self._vm._dom.interfaceStats(nic.name)
return netSamples
+ def _sampleBalloon(self):
+ infos = self._vm._dom.info()
+ return infos[2]
+
def _diff(self, prev, curr, val):
return prev[val] - curr[val]
@@ -281,6 +289,25 @@
self._log.debug("CPU stats not available: %s", e)
stats['cpuUser'] = 0.0
stats['cpuSys'] = 0.0
+
+ def _getBalloonStats(self, stats):
+ maxMem = int(self._vm.conf.get('memSize')) * 1024
+
+ sInfo, eInfo, sampleInterval = self.sampleBalloon.getStats()
+
+ ballonInfo = {
+ 'balloon_max': str(maxMem),
+ 'balloon_min': str(
+ int(self._vm.conf.get('memGuaranteedSize', '0')) * 1024),
+ 'balloon_cur': str(sInfo) if sInfo is not None else '0',
+ 'balloon_target': '0'}
+
+ for dev in self._vm.conf['devices']:
+ if dev['type'] == BALLOON_DEVICES and \
+ dev['specParams']['model'] != 'none':
+ ballonInfo['balloon_target'] = dev.get('target', maxMem)
+
+ stats['balloonInfo'] = ballonInfo
@classmethod
def _getNicStats(cls, name, model, mac,
@@ -402,6 +429,7 @@
self._getNetworkStats(stats)
self._getDiskStats(stats)
self._getDiskLatency(stats)
+ self._getBalloonStats(stats)
return stats
@@ -2428,7 +2456,11 @@
return stats
def _getStatsInternal(self):
- # used by API.Vm.getStats
+ """
+ WARNING: this method should only gather statistics by copying data.
+ Especially avoid costly and dangerous ditrect calls to the _dom
+ attribute. Use the VmStatsThread instead!
+ """
def _getGuestStatus():
GUEST_WAIT_TIMEOUT = 60
@@ -2477,6 +2509,8 @@
stats[var] = utils.convertToStr(decStats[var])
elif var == 'network':
stats['network'] = decStats[var]
+ elif var == 'balloonInfo':
+ stats['balloonInfo'] = decStats[var]
else:
try:
stats['disks'][var] = {}
@@ -2516,7 +2550,6 @@
memUsage = (100 - float(realMemUsage) /
int(self.conf['memSize']) * 100)
stats['memUsage'] = utils.convertToStr(int(memUsage))
- stats['balloonInfo'] = self._getBalloonInfo()
if self.isMigrating():
stats['migrationProgress'] = self.migrateStatus()['progress']
return stats
@@ -4362,26 +4395,6 @@
self.deleteVm()
return {'status': doneCode}
-
- def _getBalloonInfo(self):
- for dev in self.conf['devices']:
- if dev['type'] == BALLOON_DEVICES and \
- dev['specParams']['model'] != 'none':
- max_mem = int(self.conf.get('memSize')) * 1024
- min_mem = int(self.conf.get('memGuaranteedSize', '0')) * 1024
- target_mem = dev.get('target', max_mem)
- try:
- cur_mem = self._dom.info()[2]
- except (AttributeError, libvirt.libvirtError):
- # _dom may be None (race on shutdown)
- self.log.exception(
- 'failed to retrieve the balloon stats')
- cur_mem = 0
- return {'balloon_max': str(max_mem),
- 'balloon_cur': str(cur_mem),
- 'balloon_min': str(min_mem),
- 'balloon_target': str(target_mem)}
- return {}
def setBalloonTarget(self, target):
--
To view, visit http://gerrit.ovirt.org/27696
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I51fd1268b3718f3cb0c1bb32b13f38a58a67b8be
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani <fromani(a)redhat.com>
9 years, 11 months
Change in vdsm[master]: sparsify: integrating virt-sparsify into vdsm
by Jenkins CI RO
oVirt Jenkins CI Server has posted comments on this change.
Change subject: sparsify: integrating virt-sparsify into vdsm
......................................................................
Patch Set 2:
Build Failed
http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit/9679/ : To avoid overloading the infrastructure, a whitelist for running gerrit triggered jobs has been set in place, if you feel like you should be in it, please contact infra at ovirt dot org.
http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/8746/ : To avoid overloading the infrastructure, a whitelist for running gerrit triggered jobs has been set in place, if you feel like you should be in it, please contact infra at ovirt dot org.
http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/9532/ : To avoid overloading the infrastructure, a whitelist for running gerrit triggered jobs has been set in place, if you feel like you should be in it, please contact infra at ovirt dot org.
http://jenkins.ovirt.org/job/vdsm_master_storage_functional_tests_localfs... : FAILURE
--
To view, visit http://gerrit.ovirt.org/28328
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Id7bd2b4b6d45781fa27a128dd68d14b7561d0901
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Utkarsh Singh <utkarshsins(a)gmail.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
9 years, 11 months
Change in vdsm[master]: sparsify: integrating virt-sparsify into vdsm
by Jenkins CI RO
oVirt Jenkins CI Server has posted comments on this change.
Change subject: sparsify: integrating virt-sparsify into vdsm
......................................................................
Patch Set 1:
Build Failed
http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit/9678/ : To avoid overloading the infrastructure, a whitelist for running gerrit triggered jobs has been set in place, if you feel like you should be in it, please contact infra at ovirt dot org.
http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/8745/ : To avoid overloading the infrastructure, a whitelist for running gerrit triggered jobs has been set in place, if you feel like you should be in it, please contact infra at ovirt dot org.
http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/9531/ : To avoid overloading the infrastructure, a whitelist for running gerrit triggered jobs has been set in place, if you feel like you should be in it, please contact infra at ovirt dot org.
http://jenkins.ovirt.org/job/vdsm_master_storage_functional_tests_localfs... : FAILURE
--
To view, visit http://gerrit.ovirt.org/28328
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Id7bd2b4b6d45781fa27a128dd68d14b7561d0901
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Utkarsh Singh <utkarshsins(a)gmail.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
9 years, 11 months
Change in vdsm[master]: json-rpc: Protocol detection
by Nir Soffer
Nir Soffer has posted comments on this change.
Change subject: json-rpc: Protocol detection
......................................................................
Patch Set 31:
(1 comment)
http://gerrit.ovirt.org/#/c/26300/31/lib/vdsm/sslUtils.py
File lib/vdsm/sslUtils.py:
Line 77: # result = self.connection.peek(size)
Line 78: # else:
Line 79: # result = self.connection.read(size)
Line 80: # return result
Line 81: # recv = read
> There is pull request pending for M2C once peek method is implemented we wi
If the commented code is useful, send it in another patch. I don't think commented code is allowed here.
Line 82:
Line 83: def __getattr__(self, name):
Line 84: return getattr(self.connection, name)
Line 85:
--
To view, visit http://gerrit.ovirt.org/26300
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Id739a40e2b37dcc175137ec91cd5ec166ad24a75
Gerrit-PatchSet: 31
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Piotr Kliczewski <piotr.kliczewski(a)gmail.com>
Gerrit-Reviewer: Barak Azulay <bazulay(a)redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com>
Gerrit-Reviewer: Liron Ar <laravot(a)redhat.com>
Gerrit-Reviewer: Michal Skrivanek <michal.skrivanek(a)redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: Piotr Kliczewski <piotr.kliczewski(a)gmail.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com>
Gerrit-Reviewer: Vinzenz Feenstra <vfeenstr(a)redhat.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com>
Gerrit-Reviewer: Yeela Kaplan <ykaplan(a)redhat.com>
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
9 years, 11 months
Change in vdsm[master]: json-rpc: Protocol detection
by fromani@redhat.com
Francesco Romani has posted comments on this change.
Change subject: json-rpc: Protocol detection
......................................................................
Patch Set 31:
(1 comment)
http://gerrit.ovirt.org/#/c/26300/31/lib/vdsm/sslUtils.py
File lib/vdsm/sslUtils.py:
Line 77: # result = self.connection.peek(size)
Line 78: # else:
Line 79: # result = self.connection.read(size)
Line 80: # return result
Line 81: # recv = read
> We should remove this commented code later.
That would be fine for me.
Line 82:
Line 83: def __getattr__(self, name):
Line 84: return getattr(self.connection, name)
Line 85:
--
To view, visit http://gerrit.ovirt.org/26300
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Id739a40e2b37dcc175137ec91cd5ec166ad24a75
Gerrit-PatchSet: 31
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Piotr Kliczewski <piotr.kliczewski(a)gmail.com>
Gerrit-Reviewer: Barak Azulay <bazulay(a)redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com>
Gerrit-Reviewer: Liron Ar <laravot(a)redhat.com>
Gerrit-Reviewer: Michal Skrivanek <michal.skrivanek(a)redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: Piotr Kliczewski <piotr.kliczewski(a)gmail.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com>
Gerrit-Reviewer: Vinzenz Feenstra <vfeenstr(a)redhat.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com>
Gerrit-Reviewer: Yeela Kaplan <ykaplan(a)redhat.com>
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
9 years, 11 months
Change in vdsm[master]: json-rpc: Protocol detection
by Nir Soffer
Nir Soffer has posted comments on this change.
Change subject: json-rpc: Protocol detection
......................................................................
Patch Set 31:
(2 comments)
http://gerrit.ovirt.org/#/c/26300/31/lib/vdsm/sslUtils.py
File lib/vdsm/sslUtils.py:
Line 77: # result = self.connection.peek(size)
Line 78: # else:
Line 79: # result = self.connection.read(size)
Line 80: # return result
Line 81: # recv = read
> rebase/debug relic?
We should remove this commented code later.
Line 82:
Line 83: def __getattr__(self, name):
Line 84: return getattr(self.connection, name)
Line 85:
http://gerrit.ovirt.org/#/c/26300/31/vdsm/BindingXMLRPC.py
File vdsm/BindingXMLRPC.py:
Line 1131: self.xml_binding = xml_binding
Line 1132:
Line 1133: def detect(self, data):
Line 1134: return (data.startswith("PUT /") or data.startswith("GET /") or
Line 1135: data.startswith("POST /RPC2"))
> I tested xmlrpc by running CI dev job with the latest engine. I tested it o
What do you mean by "there is a / context"?
Line 1136:
Line 1137: def handleSocket(self, client_socket, socket_address):
Line 1138: self.xml_binding.add_socket(client_socket, socket_address)
--
To view, visit http://gerrit.ovirt.org/26300
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Id739a40e2b37dcc175137ec91cd5ec166ad24a75
Gerrit-PatchSet: 31
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Piotr Kliczewski <piotr.kliczewski(a)gmail.com>
Gerrit-Reviewer: Barak Azulay <bazulay(a)redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com>
Gerrit-Reviewer: Liron Ar <laravot(a)redhat.com>
Gerrit-Reviewer: Michal Skrivanek <michal.skrivanek(a)redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: Piotr Kliczewski <piotr.kliczewski(a)gmail.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com>
Gerrit-Reviewer: Vinzenz Feenstra <vfeenstr(a)redhat.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com>
Gerrit-Reviewer: Yeela Kaplan <ykaplan(a)redhat.com>
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
9 years, 11 months
Change in vdsm[master]: json-rpc: Protocol detection
by fromani@redhat.com
Francesco Romani has posted comments on this change.
Change subject: json-rpc: Protocol detection
......................................................................
Patch Set 31:
(5 comments)
Initial and partial review, just few minor notes. So far seems really nice code.
http://gerrit.ovirt.org/#/c/26300/31/lib/vdsm/sslUtils.py
File lib/vdsm/sslUtils.py:
Line 77: # result = self.connection.peek(size)
Line 78: # else:
Line 79: # result = self.connection.read(size)
Line 80: # return result
Line 81: # recv = read
rebase/debug relic?
Line 82:
Line 83: def __getattr__(self, name):
Line 84: return getattr(self.connection, name)
Line 85:
http://gerrit.ovirt.org/#/c/26300/31/lib/vdsm/utils.py
File lib/vdsm/utils.py:
Line 232:
Line 233: class ConnectedTCPServer(SocketServer.TCPServer):
Line 234:
Line 235: def __init__(self, RequestHandlerClass):
Line 236: SocketServer.TCPServer.__init__(self, None, RequestHandlerClass, False)
what about super() here?
Line 237: self.queue = Queue()
Line 238:
Line 239: def add(self, connected_socket, socket_address):
Line 240: self.queue.put((connected_socket, socket_address))
http://gerrit.ovirt.org/#/c/26300/31/tests/bridgeTests.py
File tests/bridgeTests.py:
Line 31:
Line 32: def createFakeAPI():
Line 33: _newAPI = imp.new_module('API')
Line 34: _API = __import__('API', globals(), locals(), {}, -1)
Line 35: # setattr(_newAPI, 'Host', Host)
rebase/debug relic?
Line 36: setattr(_newAPI, 'Global', Host)
Line 37:
Line 38: # Apply the whitelist to our version of API
Line 39: for name in apiWhitelist:
http://gerrit.ovirt.org/#/c/26300/31/tests/jsonRpcHelper.py
File tests/jsonRpcHelper.py:
Line 37:
Line 38: protonReactor = None
Line 39: try:
Line 40: from yajsonrpc import protonReactor
Line 41: protonReactor # unused import
I'd add in the comment the reason why it is unused (AFAIK/IIRC to make pep8/pylint happy)
Line 42: except ImportError:
Line 43: pass
Line 44:
Line 45:
http://gerrit.ovirt.org/#/c/26300/31/vdsm/BindingXMLRPC.py
File vdsm/BindingXMLRPC.py:
Line 971: (self.setupNetworks, 'setupNetworks'),
Line 972: (self.ping, 'ping'),
Line 973: (self.setSafeNetworkConfig, 'setSafeNetworkConfig'),
Line 974: (self.fenceNode, 'fenceNode'),
Line 975: (self.stop, 'prepareForShutdown'),
Just for my better understanding, what's the reason for this name change?
Line 976: (self.setLogLevel, 'setLogLevel'),
Line 977: (self.setMOMPolicy, 'setMOMPolicy'),
Line 978: (self.setMOMPolicyParameters, 'setMOMPolicyParameters'),
Line 979: (self.setHaMaintenanceMode, 'setHaMaintenanceMode'),
--
To view, visit http://gerrit.ovirt.org/26300
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Id739a40e2b37dcc175137ec91cd5ec166ad24a75
Gerrit-PatchSet: 31
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Piotr Kliczewski <piotr.kliczewski(a)gmail.com>
Gerrit-Reviewer: Barak Azulay <bazulay(a)redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com>
Gerrit-Reviewer: Liron Ar <laravot(a)redhat.com>
Gerrit-Reviewer: Michal Skrivanek <michal.skrivanek(a)redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: Piotr Kliczewski <piotr.kliczewski(a)gmail.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com>
Gerrit-Reviewer: Vinzenz Feenstra <vfeenstr(a)redhat.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com>
Gerrit-Reviewer: Yeela Kaplan <ykaplan(a)redhat.com>
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
9 years, 11 months