cloudstack-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Tutkowski, Mike" <Mike.Tutkow...@netapp.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:13:43 GMT
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
Mime
View raw message