cloudstack-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Rafael Weingärtner <rafaelweingart...@gmail.com>
Subject Re: [GitHub] rafaelweingartner commented on a change in pull request #2416: CLOUDSTACK-10244: KVM online storage migration fails
Date Sat, 20 Jan 2018 15:24:16 GMT
Changed by a future programmer that may develop some changes in your code?
Because, if you do not code any other assignment to that variable, it will
never happen.

On Sat, Jan 20, 2018 at 1:16 PM, Tutkowski, Mike <Mike.Tutkowski@netapp.com>
wrote:

> @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.Tutkowski@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 <git@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:
> >> users@infra.apache.org
> >>
> >>
> >> With regards,
> >> Apache Git Services
>



-- 
Rafael Weingärtner

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message