vdsm-patches June 2014

vdsm-patches@lists.fedorahosted.org

41 participants
740 discussions

Change in vdsm[master]: sparsify: integrating virt-sparsify into vdsm
by Federico Simoncelli 03 Jun '14

03 Jun '14

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

1 0

Change in vdsm[master]: json-rpc: Protocol detection
by Nir Soffer 03 Jun '14

03 Jun '14

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

1 0

Change in vdsm[master]: vm: move getBalloonInfo in the stats thread
by fromani＠redhat.com 03 Jun '14

03 Jun '14

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>

4 12

Change in vdsm[master]: sparsify: integrating virt-sparsify into vdsm
by Jenkins CI RO 03 Jun '14

03 Jun '14

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_g… : 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

1 0

Change in vdsm[master]: sparsify: integrating virt-sparsify into vdsm
by Jenkins CI RO 03 Jun '14

03 Jun '14

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_g… : 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

1 0

Change in vdsm[master]: json-rpc: Protocol detection
by Jenkins CI RO 03 Jun '14

03 Jun '14

oVirt Jenkins CI Server has posted comments on this change. Change subject: json-rpc: Protocol detection ...................................................................... Patch Set 32: Build Failed http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit/9672/ : FAILURE http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/8739/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_install_rpm_sanity_gerrit/675/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/9525/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_master_storage_functional_tests_localfs_g… : SUCCESS -- 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

1 0

Change in vdsm[master]: json-rpc: Protocol detection
by Nir Soffer 03 Jun '14

03 Jun '14

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

1 0

Change in vdsm[master]: json-rpc: Protocol detection
by fromani＠redhat.com 03 Jun '14

03 Jun '14

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

1 0

Change in vdsm[master]: json-rpc: Protocol detection
by Nir Soffer 03 Jun '14

03 Jun '14

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

1 0

Change in vdsm[master]: json-rpc: Protocol detection
by fromani＠redhat.com 03 Jun '14

03 Jun '14

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

1 0

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

vdsm-patches June 2014