Re: KVM systemvm not getting IP

2013-10-11 Thread Marcus Sorensen
Look at the qemu log for that specific vm. Are these still system VMS or user VMS? On Oct 11, 2013 11:26 AM, "Francois Gaudreault" wrote: > Thanks Kelcey. > > My VMs are starting now, but then qemu sends a sigkill and the VM shutdown > by itself after 40sec or so. Anyone experienced that? I guess

RE: KVM systemvm not getting IP

2013-10-11 Thread Rajesh Battala
Acton template won't work. In 4.2 there is a new template. You can check it by checking the release version of it. 4.2 systemvm is generated from debian wheezy. Thanks Rajesh Battala -Original Message- From: Francois Gaudreault [mailto:fgaudrea...@cloudops.com] Sent: Friday, October 1

Re: Why Secondarystroage resourcelimit contains volumes

2013-10-11 Thread sx chen
Thanks,Nitin For Upload volume It's OK. Then I have another question; If I destroy a vm,the root disk will be romoved too,but data disk is still on the primary storage, and state is not "Allocated",So these volumes will be counted as secondarystorage and they are actually in the primary storage?

RE: [DISCUSS] Add upgrade path from 4.2.1 to 4.3.0 in Master branch

2013-10-11 Thread Animesh Chaturvedi
Well I take it back, I think it is easier to add the path now in master > -Original Message- > From: Min Chen [mailto:min.c...@citrix.com] > Sent: Friday, October 11, 2013 6:05 PM > To: dev@cloudstack.apache.org > Subject: Re: [DISCUSS] Add upgrade path from 4.2.1 to 4.3.0 in Master > bran

[ACS43] Proposed schedule

2013-10-11 Thread Animesh Chaturvedi
Folks I would like to propose the following schedule for 4.3. Please note that I have pushed it out by 1 week for multiple reasons 1) Until end of September our focus was on 4.2 and master needs to be stabilized before feature freeze 2) The original RC was scheduled closer to Christmas and it s

Re: [DISCUSS] Add upgrade path from 4.2.1 to 4.3.0 in Master branch

2013-10-11 Thread Min Chen
Then what should we do to cherry-pick some commits from 4.2.1 to master? No cherry-pick until 4.3 is cut? Thanks -min On 10/11/13 5:25 PM, "Animesh Chaturvedi" wrote: >> -Original Message- >> From: Min Chen [mailto:min.c...@citrix.com] >> Sent: Friday, October 11, 2013 4:51 PM >> To: de

RE: [ACS43] [DISCUSS] Release management tasks up for grabs

2013-10-11 Thread Animesh Chaturvedi
Travis thanks for your help, really appreciate the offer. Looking forward to working closely with you. > -Original Message- > From: Travis Graham [mailto:tgra...@tgraham.us] > Sent: Friday, October 11, 2013 5:34 PM > To: dev@cloudstack.apache.org > Subject: Re: [ACS43] [DISCUSS] Release

Re: [ACS43] [DISCUSS] Release management tasks up for grabs

2013-10-11 Thread Travis Graham
Animesh, I'm volunteering to help out with the docs. I've put a hold on fixing things until there's a Review Board in place for cloudstack-docs to make it easier to submit patches. Travis On Oct 11, 2013, at 7:01 PM, Animesh Chaturvedi wrote: > Folks > > As per the thread [1] release mana

Patches in reviewboard that were updated over two months ago

2013-10-11 Thread Animesh Chaturvedi
Folks Following 27 patches were last updated over 2 months ago | Review ID | Summary |#Reviews | Submitter| People| | 11861 | double slash fix for windows based nfs se

RE: [DISCUSS] Add upgrade path from 4.2.1 to 4.3.0 in Master branch

2013-10-11 Thread Animesh Chaturvedi
> -Original Message- > From: Min Chen [mailto:min.c...@citrix.com] > Sent: Friday, October 11, 2013 4:51 PM > To: dev@cloudstack.apache.org > Subject: [DISCUSS] Add upgrade path from 4.2.1 to 4.3.0 in Master branch > > Hi there, > > Based on my understanding, 4.2.1 is scheduled to come be

[DISCUSS] Add upgrade path from 4.2.1 to 4.3.0 in Master branch

2013-10-11 Thread Min Chen
Hi there, Based on my understanding, 4.2.1 is scheduled to come before 4.3.0 release. If this is true, should we add an upgrade path from 4.2.1 to 4.3.0 in Master? This will help us to cherry-pick some commits done in 4.2.1 branch that involves schema changes to master. Any objections on this?

Re: Back end volume names in KVM

2013-10-11 Thread Marcus Sorensen
Done. Tested root/data disks for CLVM, local, NFS. On Wed, Oct 9, 2013 at 11:32 PM, Marcus Sorensen wrote: > Yeah, existing volumes have a path thats already set in stone. It would only > concern new volumes. > > It could cause problems down the line if people get used to them matching > and deci

[ACS43] [DISCUSS] Release management tasks up for grabs

2013-10-11 Thread Animesh Chaturvedi
Folks As per the thread [1] release management for CloudStack is complex and runs into many tasks and it is hard for one person to do it all. While I am taking the overall release management for 4.3 release there are several areas where we need volunteers: I have put down my thoughts please

Re: Review Request 13806: CLOUDSTACK-4407: Use extractTemplate API to get hypervisor specific template information

2013-10-11 Thread Chandan Purushothama
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/13806/#review26941 --- test_stopped_vm.py tries to clean up ISOs that are already deleted a

Re: Review Request 14577: Remove Setters from *JoinVO Classes

2013-10-11 Thread SuichII, Christopher
Just bumping this. It should be a fairly simple review. -- Chris Suich chris.su...@netapp.com NetApp Software Engineer Data Center Platforms – Cloud Solutions Citrix, Cisco & Red Hat On Oct 10, 2013, at 1:04 PM, Chris Suich mailto:chris.su...@netapp.com>> wrote: T

RE: Global setting "host" is not set to MS IP anymore on master

2013-10-11 Thread Soheil Eizadi
Since I synced my code with 4.3 Master, the Host and Management Network is getting properly for me. The storage network is empty. -Soheil From: Mike Tutkowski [mike.tutkow...@solidfire.com] Sent: Friday, October 11, 2013 11:23 AM To: dev@cloudstack.apache.o

Review Request 14609: Added Categorized Sorting of SnapshotStrategy and DataMotionStrategy

2013-10-11 Thread Chris Suich
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/14609/ --- Review request for cloudstack, Darren Shepherd and edison su. Repository: cloud

Review Request 14605: Fill the creationMonitors based on priority

2013-10-11 Thread Laszlo Hornyak
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/14605/ --- Review request for cloudstack and Darren Shepherd. Repository: cloudstack-git

Re: Review Request 14381: KVM: add connect/disconnect capabilities to StorageAdaptors so that external storage services can attach/detach devices on-demand

2013-10-11 Thread Mike Tutkowski
We will also require iscsiadm to be present, which I believe is part of the open-iscsi package on Ubuntu, so I guess what's another couple dependencies. :) On Fri, Oct 11, 2013 at 1:03 PM, Marcus Sorensen wrote: > Oops, that patch got cut off: > > > diff --git a/debian/control b/debian/control >

Re: Review Request 14381: KVM: add connect/disconnect capabilities to StorageAdaptors so that external storage services can attach/detach devices on-demand

2013-10-11 Thread Marcus Sorensen
Oops, that patch got cut off: diff --git a/debian/control b/debian/control index 46dd505..72a0fb1 100644 --- a/debian/control +++ b/debian/control @@ -22,7 +22,7 @@ Description: CloudStack server library Package: cloudstack-agent Architecture: all -Depends: openjdk-6-jre | openjdk-7-jre, clouds

Re: Review Request 14381: KVM: add connect/disconnect capabilities to StorageAdaptors so that external storage services can attach/detach devices on-demand

2013-10-11 Thread Marcus Sorensen
Yes, blockdev is a part of the util-linux package, and all but a minimal install should have it. To be safe, we could add 'util-linux' to the deb requires and 'util-linux-ng' to the rpm requires for the agents. Something like: diff --git a/debian/control b/debian/control index 46dd505..72a0fb1 100

Review Request 14604: Removing coded documentation

2013-10-11 Thread Laszlo Hornyak
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/14604/ --- Review request for cloudstack. Repository: cloudstack-git Description ---

Review Request 14603: fixed Rules

2013-10-11 Thread Laszlo Hornyak
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/14603/ --- Review request for cloudstack. Repository: cloudstack-git Description ---

Re: Review Request 14381: KVM: add connect/disconnect capabilities to StorageAdaptors so that external storage services can attach/detach devices on-demand

2013-10-11 Thread Mike Tutkowski
Hey Marcus, What do you think about me running the following to get the device size for my PhysicalDisk instances? "blockdev --getsize64 /dev/disk/by-path/ip-" + host + "-iscsi-" + iqn + "-lun-0 That should work on whatever platforms the KVM agent runs (mainly Ubuntu and CentOS), right? Thanks

Re: Global setting "host" is not set to MS IP anymore on master

2013-10-11 Thread Mike Tutkowski
Yeah, I tend to set up my CS environment manually. The first thing I do when I log in via the GUI is change the host, secstorage, and cidr Global Settings and then re-start the management server. It would be great if this were not required, of course. On Fri, Oct 11, 2013 at 11:14 AM, Min Chen

Re: [PROPOSAL] Revert to VM disk Snapshot

2013-10-11 Thread Mike Tutkowski
I agree that (1) is the better way to go here. On Fri, Oct 11, 2013 at 10:50 AM, SuichII, Christopher < chris.su...@netapp.com> wrote: > I'm going to bring the conversation back to this thread since it got > somewhat branched out. > > One hurdle in implementing this is that it has been brought t

Re: KVM systemvm not getting IP

2013-10-11 Thread Francois Gaudreault
Thanks Kelcey. My VMs are starting now, but then qemu sends a sigkill and the VM shutdown by itself after 40sec or so. Anyone experienced that? I guess this is something to bring to the libvirt mailing list :S Francois On 10/11/2013, 12:38 PM, Kelcey Jamison Damage wrote: That is the old te

RE: UI accounts wizard broken

2013-10-11 Thread Brian Federle
No worries, I must have missed the original discussion. Probably an easier way would be to add support for passing a closure to the createForm: and/or add: action block, so you can conditionally check context and return the appropriate form. We can modify the account form to use this in the fut

Re: Global setting "host" is not set to MS IP anymore on master

2013-10-11 Thread Min Chen
I am also using marvin to deploy my local environment. Do you need to restart MS after adding this globalConfig section in our .cfg file? If so, developer may be able to workaround this. But this issue still needs to be addressed for people not using marvin framework to setup environment, then they

RE: High CPU utilization on KVM hosts while doing RBD snapshot - was Re: snapshot caused host disconnected

2013-10-11 Thread Edison Su
> -Original Message- > From: Indra Pramana [mailto:in...@sg.or.id] > Sent: Wednesday, October 09, 2013 11:07 PM > To: dev@cloudstack.apache.org; us...@cloudstack.apache.org > Subject: Re: High CPU utilization on KVM hosts while doing RBD snapshot - > was Re: snapshot caused host disconnec

Re: Why Secondarystroage resourcelimit contains volumes

2013-10-11 Thread Nitin Mehta
One of the use case where volume is on secondary storage is for the uploaded volumes. Until they are attached to the vm they reside on secondary storage and hence should be counted for secondary storage usage. On 11/10/13 1:15 AM, "sx chen" wrote: >Hi,all: >In 4.2,We can limit the secondary

Re: Review Request 14549: Rename net.juniper.contrail to org.apache.cloudstack.network.contrail

2013-10-11 Thread Pedro Marques
> On Oct. 9, 2013, 2:49 p.m., Murali Reddy wrote: > > - Is there a reason why no new isolation type was not added for 'contrail > > controller'. For other overlay technologies (STT, GRE, VXLAN) that > > CloudStack support there is an isolation type and corresponding Guru that > > handles isola

Re: [PROPOSAL] Revert to VM disk Snapshot

2013-10-11 Thread SuichII, Christopher
I'm going to bring the conversation back to this thread since it got somewhat branched out. One hurdle in implementing this is that it has been brought to my attention that we do not want all snapshots to be revertible. For example, I believe it is not XenServer best practices to simply revert

Re: KVM systemvm not getting IP

2013-10-11 Thread Kelcey Jamison Damage
That is the old template and won't work with 4.2 You can follow one of the methods in http://cloud.kelceydamage.com/cloudfire/blog/2013/10/08/conquering-the-cloudstack-4-2-dragon-kvm/ if you wish to recover your cloud, or you can simply use the updated template link posted and start over again

Re: KVM systemvm not getting IP

2013-10-11 Thread Francois Gaudreault
Hmm... How do I know if I am using the old or new :P I downloaded this one: http://download.cloud.com/templates/acton/acton-systemvm-02062012.qcow2.bz2 FG On 10/11/2013, 12:12 PM, Rajesh Battala wrote: Hi Francois, Are you using the new systemvm template. System vm's ip's info (boot args) are

RE: KVM systemvm not getting IP

2013-10-11 Thread Rajesh Battala
Hi Francois, Are you using the new systemvm template. System vm's ip's info (boot args) are passed via socket in 4.2. Please check whether you are using new or old systemvm template. Thanks Rajesh Battala -Original Message- From: Francois Gaudreault [mailto:fgaudrea...@cloudops.com]

Re: [New Feature FS] SSL Offload Support for Cloudstack

2013-10-11 Thread Syed Ahmed
Thanks for your valuable feedback Murali. Here are my comments. IMO, its better we introduce new api's say registerCertifcateToLoadbalancer/deregisterCertifcateToLoadbalancer than force fit existing API's for associate/dis-associate certificates. Personally, I was going to do it this way. But

Review Request 14589: fix volume creation failure bug

2013-10-11 Thread Yoshikazu Nojima
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/14589/ --- Review request for cloudstack. Repository: cloudstack-git Description ---

KVM systemvm not getting IP

2013-10-11 Thread Francois Gaudreault
Anyone faced an issue where the KVM system VM doesn't set IPs on the network interfaces? In fact there is no interfaces definitions in /etc/network/interfaces. I am using CS 4.2. I re-installed my host to use Ubuntu 13.04 (I have the same behavior on CentOS 6). -- Francois Gaudreault Archite

Re: Cloudstack collab Hackathons

2013-10-11 Thread Mike Tutkowski
I plan on attending the hackathon. I expect to be participating in the automated testing discussion since I'd like to develop tests for the SolidFire plug-in I built and continue to enhance (and see how I can leverage a SAN in my lab for these tests and still have the tests kicked off in response

Re: [New Feature FS] SSL Offload Support for Cloudstack

2013-10-11 Thread Murali Reddy
On 09/10/13 8:08 PM, "Syed Ahmed" wrote: >Thanks Murali for your response. > >> - any reason why you choose assignTo/RemoveFrom load balancer rule API's > >I thought this made more sense than create/updateLoadbalancerRule as >we would have to call update to delete a cert which I find somewhat >co

Re: Review Request 14595: Adding https support to marvin

2013-10-11 Thread Santhosh Edukulla
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/14595/ --- (Updated Oct. 11, 2013, 12:51 p.m.) Review request for cloudstack, daan Hooglan

Review Request 14595: Adding https support to marvin

2013-10-11 Thread Santhosh Edukulla
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/14595/ --- Review request for cloudstack. Bugs: 4832 Repository: cloudstack-git Descri

Review Request 14593: CLOUDSTACK-1833: AUtomation - Adding scale virtual machine test cases

2013-10-11 Thread Gaurav Aradhye
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/14593/ --- Review request for cloudstack, Nitin Mehta and SrikanteswaraRao Talluri. Reposi

review of keepalive

2013-10-11 Thread Daan Hoogland
H Chriradeep, Can you have a look at https://reviews.apache.org/r/14320/ I'm looking to commit it next week regards, Daan

Re: Review Request 14320: add boolean option keepAliveEnabled to the network offering for use in haproxy conf

2013-10-11 Thread daan Hoogland
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/14320/ --- (Updated Oct. 11, 2013, 11:23 a.m.) Review request for cloudstack, Chiradeep Vi

Error creating bean in master branch

2013-10-11 Thread Nguyen Anh Tu
Hi guys, I'm trying to add a new table to DB in master branch. I added the new bean to applicationContext.xml as below: ** Build success. However, exception is generated when running: ERROR [o.s.w.c.ContextLoader] (main:null) Context initialization failed org.springframework.

Re: Review Request 14591: Fixng the issue of wrong netmask value is allowed in public traffic wizard while creating a zone

2013-10-11 Thread Damodar Reddy Talakanti
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/14591/ --- (Updated Oct. 11, 2013, 10:53 a.m.) Review request for cloudstack, Abhinandan P

Re: Review Request 14591: Fixng the issue of wrong netmask value is allowed in public traffic wizard while creating a zone

2013-10-11 Thread Damodar Reddy Talakanti
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/14591/ --- (Updated Oct. 11, 2013, 10:48 a.m.) Review request for cloudstack, Abhinandan P

[ASF4.2.1] Milestones

2013-10-11 Thread Abhinandan Prateek
After looking at various other releases and the volume of important tickets the 4.2.1 GA should happen on 15 Nov. Following are the important milestones: 25th October: Code complete, all the fixes should be in by this date to make it to maintenance release. As of now all Blockers and Criticals

Re: marvin create network offering incomplete?

2013-10-11 Thread Daan Hoogland
H Prasanna, I didn't get around to it and forgot about this. def createNiciraNetworkOffering(self, conn) : cno = createNetworkOffering.createNetworkOfferingCmd() cno.name = "NiciraNvpL2NoLB_" + str(random.randrange(1, 400, 1)) cno.displaytext = "offering with Nicira NVP L2 wi

Re: Review Request 13701: Automation Tests for HA Proxy Stickiness

2013-10-11 Thread suresh sadhu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/13701/#review26927 --- Ship it! patch looks ok - suresh sadhu On Oct. 11, 2013, 7:23 a.

Re: Review Request 14591: Fixng the issue of wrong netmask value is allowed in public traffic wizard while creating a zone

2013-10-11 Thread Jayapal Reddy
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/14591/#review26926 --- server/src/com/cloud/configuration/ConfigurationManagerImpl.java

Review Request 14591: Fixng the issue of wrong netmask value is allowed in public traffic wizard while creating a zone

2013-10-11 Thread Damodar Reddy Talakanti
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/14591/ --- Review request for cloudstack, Abhinandan Prateek, Jayapal Reddy, and Murali Red

RE: Autoscaling

2013-10-11 Thread Nguyen Anh Tu
Got it. Thanks Sowmya. Sent from my GT-N7000 On Oct 11, 2013 4:51 PM, "Sowmya Krishnan" wrote: > Answer inline... > > > -Original Message- > > From: Nguyen Anh Tu [mailto:ng.t...@gmail.com] > > Sent: Friday, October 11, 2013 12:14 PM > > To: dev@cloudstack.apache.org > > Subject: Re: Aut

Re: [DISCUSS] Leaky abstractions [was review requests 13238, 13896, 14320]

2013-10-11 Thread Daan Hoogland
Having thought about the more generic issue with 'leaky abstractions' a little more I would like to add the reverse of my scenario to the discussion. all implementers of a flutch have a feature Y so we implement an option in our interface. Now the one of them (or a new kid on the block) solves the

RE: Autoscaling

2013-10-11 Thread Sowmya Krishnan
Answer inline... > -Original Message- > From: Nguyen Anh Tu [mailto:ng.t...@gmail.com] > Sent: Friday, October 11, 2013 12:14 PM > To: dev@cloudstack.apache.org > Subject: Re: Autoscaling > > Dear Rajesh, > > Apologize for delay in response. > > You mean the netscaler api's used to crea

Re: [PROPOSAL] Dynamic Compute Offering

2013-10-11 Thread Bharat Kumar
Hi All, Added the Function spec at https://cwiki.apache.org/confluence/display/CLOUDSTACK/Dynamic+Compute+Offering+FS please go thorough this and give me your comments. -Bharat. On Sep 30, 2013, at 12:34 PM, Bharat Kumar wrote: > Hi All, > > Currently in cloudstack we need to associate eve

Re: Cloudstack collab Hackathons

2013-10-11 Thread Daan Hoogland
https://cwiki.apache.org/confluence/display/CLOUDSTACK/CCC+Europe contains a provisionary list. You can add there if you have wiki access or ask in this thread as Hugo suggests. On Fri, Oct 11, 2013 at 10:43 AM, Hugo Trippaers wrote: > Hey Guys, > > The CloudStack collaboration conference is righ

Cloudstack collab Hackathons

2013-10-11 Thread Hugo Trippaers
Hey Guys, The CloudStack collaboration conference is right around the corner. The first day of the conference will be dedicated to workshops and a hackathon. I'm curious which developers are planning to attend the hackthon and what subjects you are interested in. In Santa Clara we had a short l

Re: Review Request 14557: CLOUDSTACK-4747: Rename testcase name to use lesser characters

2013-10-11 Thread Girish Shilamkar
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/14557/ --- (Updated Oct. 11, 2013, 8:31 a.m.) Review request for cloudstack, Sowmya Krishn

Why Secondarystroage resourcelimit contains volumes

2013-10-11 Thread sx chen
Hi,all: In 4.2,We can limit the secondarystorage usage for a user or a domain. But when I refer to source code ,I find public long calculateSecondaryStorageForAccount(long accountId) { long totalVolumesSize = _volumeDao.secondaryStorageUsedForAccount(accountId); long totalS

Re: Review Request 14199: Adding missing test cases: Snapshots Improvement

2013-10-11 Thread Gaurav Aradhye
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/14199/ --- (Updated Oct. 11, 2013, 1:36 p.m.) Review request for cloudstack and Prasanna S

Re: Review Request 14199: Adding missing test cases: Snapshots Improvement

2013-10-11 Thread Gaurav Aradhye
> On Oct. 7, 2013, 10:48 p.m., Nitin Mehta wrote: > > > > Nitin Mehta wrote: > It would be great if you could show logs of a sample run. Please also let > me know if all the 5 cases are tested as well Log Added. > On Oct. 7, 2013, 10:48 p.m., Nitin Mehta wrote: > > test/integration/compo

Re: Review Request 13701: Automation Tests for HA Proxy Stickiness

2013-10-11 Thread Girish Shilamkar
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/13701/ --- (Updated Oct. 11, 2013, 7:23 a.m.) Review request for cloudstack, suresh sadhu

Re: bvt failure since on master

2013-10-11 Thread Daan Hoogland
The build (m/f)ails when the retrieval step doesn't work as well and sure a lot of little other things besides the actual build. so you really need to deep dive if the cause is not clear at first glance. On Fri, Oct 11, 2013 at 8:08 AM, Laszlo Hornyak wrote: > From jenkins.buildacloud.org I am ge

Re: Steps in building DEB packages for 4.2.0

2013-10-11 Thread Prasanna Santhanam
On Fri, Oct 11, 2013 at 09:59:52AM +0800, Indra Pramana wrote: > Dear all, > > I am reading the documentation on the steps in building DEB packages for > CloudStack 4.2.0: > > http://cloudstack.apache.org/docs/en-US/Apache_CloudStack/4.2.0/html/Installation_Guide/sect-source-builddebs.html > > a

Re: Review Request 14335: CLOUDSTACK-4262: Fix TestVPCNetworkGc.test_01_wait_network_gc

2013-10-11 Thread ASF Subversion and Git Services
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/14335/#review26924 --- Commit 808b96070e17976f8ae7424b437713f329be7ac4 in branch refs/heads