@wido just wanted to make sure the Boolean variable didn’t accidentally get changed later since it’s critical it keep that value.
> On Jan 20, 2018, at 8:14 AM, Tutkowski, Mike <mike.tutkow...@netapp.com> > wrote: > > I made it final per @wido’s comment (in #2415). How’s about you guys hash it > out and get back to me. :) > >> On Jan 20, 2018, at 8:06 AM, GitBox <g...@apache.org> wrote: >> >> rafaelweingartner commented on a change in pull request #2416: >> CLOUDSTACK-10244: KVM online storage migration fails >> URL: https://github.com/apache/cloudstack/pull/2416#discussion_r162784630 >> >> >> >> ########## >> File path: >> plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtMigrateCommandWrapper.java >> ########## >> @@ -132,8 +132,9 @@ Use VIR_DOMAIN_XML_SECURE (value = 1) prior to v1.0.0. >> vmsnapshots = >> libvirtComputingResource.cleanVMSnapshotMetadata(dm); >> >> Map<String, MigrateCommand.MigrateDiskInfo> mapMigrateStorage = >> command.getMigrateStorage(); >> + final boolean migrateStorage = >> MapUtils.isNotEmpty(mapMigrateStorage); >> >> Review comment: >> This variable does not need to be final. It does not bring any benefits >> here. >> >> You needed to assign the result of the first >> `MapUtils.isNotEmpty(mapMigrateStorage)` because the method `replaceStorage` >> might change the status of `mapMigrateStorage`, right? >> >> ---------------------------------------------------------------- >> This is an automated message from the Apache Git Service. >> To respond to the message, please log on GitHub and use the >> URL above to go to the specific comment. >> >> For queries about this service, please contact Infrastructure at: >> us...@infra.apache.org >> >> >> With regards, >> Apache Git Services
