If I click https://reviews.apache.org/r/8123/diff/5/, I get: The patch to 'pom.xml' didn't apply cleanly. The temporary files have been left in '/tmp/reviewboard.gkgGgx' for debugging purposes. `patch` returned: patching file /tmp/reviewboard.gkgGgx/tmpvi_JO8 Reversed (or previously applied) patch detected! Assume -R? [n] Apply anyway? [n] Skipping patch. 1 out of 1 hunk ignored -- saving rejects to file /tmp/reviewboard.gkgGgx/tmpvi_JO8-new.rej
In diff5, there is lot of changes on java import in ConfigurationManagerImpl, while there is no code change in the class itself, Something like: +++ b/server/src/com/cloud/configuration/ConfigurationManagerImpl.java @@ -16,35 +16,110 @@ // under the License. package com.cloud.configuration; +import java.net.URI; +import java.sql.PreparedStatement; +import java.sql.ResultSet; +import java.sql.SQLException; +import java.util.ArrayList; +import java.util.Collection; +import java.util.HashMap; +import java.util.HashSet; +import java.util.Hashtable; +import java.util.Iterator; +import java.util.List; +import java.util.Map; +import java.util.Set; +import java.util.UUID; + +import javax.ejb.Local; +import javax.naming.ConfigurationException; +import javax.naming.Context; +import javax.naming.NamingException; +import javax.naming.directory.DirContext; +import javax.naming.directory.InitialDirContext; + +import org.apache.log4j.Logger; + import com.cloud.acl.SecurityChecker; import com.cloud.alert.AlertManager; import com.cloud.api.ApiConstants.LDAPParams; import com.cloud.api.ApiDBUtils; -import com.cloud.api.commands.*; +import com.cloud.api.commands.CreateDiskOfferingCmd; +import com.cloud.api.commands.CreateNetworkOfferingCmd; +import com.cloud.api.commands.CreateServiceOfferingCmd; +import com.cloud.api.commands.CreateVlanIpRangeCmd; +import com.cloud.api.commands.CreateZoneCmd; +import com.cloud.api.commands.DeleteDiskOfferingCmd; +import com.cloud.api.commands.DeleteNetworkOfferingCmd; +import com.cloud.api.commands.DeletePodCmd; +import com.cloud.api.commands.DeleteServiceOfferingCmd; +import com.cloud.api.commands.DeleteVlanIpRangeCmd; +import com.cloud.api.commands.DeleteZoneCmd; +import com.cloud.api.commands.LDAPConfigCmd; +import com.cloud.api.commands.LDAPRemoveCmd; +import com.cloud.api.commands.ListNetworkOfferingsCmd; +import com.cloud.api.commands.UpdateCfgCmd; +import com.cloud.api.commands.UpdateDiskOfferingCmd; +import com.cloud.api.commands.UpdateNetworkOfferingCmd; +import com.cloud.api.commands.UpdatePodCmd; +import com.cloud.api.commands.UpdateServiceOfferingCmd; +import com.cloud.api.commands.UpdateZoneCmd; import com.cloud.capacity.dao.CapacityDao; import com.cloud.configuration.Resource.ResourceType; import com.cloud.configuration.dao.ConfigurationDao; -import com.cloud.dc.*; +import com.cloud.dc.AccountVlanMapVO; +import com.cloud.dc.ClusterVO; +import com.cloud.dc.DataCenter; import com.cloud.dc.DataCenter.NetworkType; +import com.cloud.dc.DataCenterIpAddressVO; +import com.cloud.dc.DataCenterLinkLocalIpAddressVO; +import com.cloud.dc.DataCenterVO; +import com.cloud.dc.HostPodVO; +import com.cloud.dc.Pod; +import com.cloud.dc.PodVlanMapVO; +import com.cloud.dc.Vlan; import com.cloud.dc.Vlan.VlanType; -import com.cloud.dc.dao.*; +import com.cloud.dc.VlanVO; +import com.cloud.dc.dao.AccountVlanMapDao; +import com.cloud.dc.dao.ClusterDao; +import com.cloud.dc.dao.DataCenterDao; +import com.cloud.dc.dao.DataCenterIpAddressDao; +import com.cloud.dc.dao.DataCenterLinkLocalIpAddressDaoImpl; +import com.cloud.dc.dao.HostPodDao; +import com.cloud.dc.dao.PodVlanMapDao; +import com.cloud.dc.dao.VlanDao; import com.cloud.deploy.DataCenterDeployment; import com.cloud.domain.Domain; import com.cloud.domain.DomainVO; import com.cloud.domain.dao.DomainDao; import com.cloud.event.ActionEvent; import com.cloud.event.EventTypes; -import com.cloud.exception.*; +import com.cloud.exception.ConcurrentOperationException; +import com.cloud.exception.InsufficientCapacityException; +import com.cloud.exception.InvalidParameterValueException; +import com.cloud.exception.PermissionDeniedException; +import com.cloud.exception.ResourceAllocationException; +import com.cloud.exception.ResourceUnavailableException; import com.cloud.host.HostVO; import com.cloud.hypervisor.Hypervisor.HypervisorType; -import com.cloud.network.*; +import com.cloud.network.IPAddressVO; +import com.cloud.network.Network; import com.cloud.network.Network.Capability; import com.cloud.network.Network.GuestType; import com.cloud.network.Network.Provider; import com.cloud.network.Network.Service; +import com.cloud.network.NetworkManager; +import com.cloud.network.NetworkVO; import com.cloud.network.Networks.BroadcastDomainType; import com.cloud.network.Networks.TrafficType; -import com.cloud.network.dao.*; +import com.cloud.network.PhysicalNetwork; +import com.cloud.network.PhysicalNetworkVO; +import com.cloud.network.dao.FirewallRulesDao; +import com.cloud.network.dao.IPAddressDao; +import com.cloud.network.dao.NetworkDao; +import com.cloud.network.dao.PhysicalNetworkDao; +import com.cloud.network.dao.PhysicalNetworkTrafficTypeDao; +import com.cloud.network.dao.PhysicalNetworkTrafficTypeVO; import com.cloud.network.vpc.VpcManager; import com.cloud.offering.DiskOffering; import com.cloud.offering.NetworkOffering; Are all these imports are necessary? > -----Original Message----- > From: John Burwell [mailto:nore...@reviews.apache.org] On Behalf Of John > Burwell > Sent: Wednesday, December 19, 2012 1:40 PM > To: Edison Su > Cc: cloudstack; John Burwell > Subject: Re: Review Request: S3-backed Secondary Storage > > > > > On Dec. 17, 2012, 5:51 p.m., edison su wrote: > > > Hi John, thanks for the patch. Few comments: 1. I can't apply the patch > against master top, there is conflict in pom.xml. 2. Could you remove the > unnecessary change in ConfigurationManagerImpl file? > > Edison, > > Which change in ConfigurationManagerImpl is unnecessary? Looking > through the diff, I cant find any unnecessary changes. Also, can you specify > the merge issue you are encountering with pom.xml? I just merged my > feature branch with cloudstack/master and had no conflicts. > > Thanks, > -John > > > - John > > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/8123/#review14588 > ----------------------------------------------------------- > > > On Dec. 14, 2012, 11:54 p.m., John Burwell wrote: > > > > ----------------------------------------------------------- > > This is an automatically generated e-mail. To reply, visit: > > https://reviews.apache.org/r/8123/ > > ----------------------------------------------------------- > > > > (Updated Dec. 14, 2012, 11:54 p.m.) > > > > > > Review request for cloudstack and edison su. > > > > > > Description > > ------- > > > > Backs NFS-based secondary storage with an S3-compatible object store. > Periodically, a reaper thread synchronizes templates and ISOs stored on a > NFS secondary storage mount with a configured S3 object store. It also > pushes snapshots to the object store when they are created and downloads > them in other zones on-demand. In addition to permitting the use of > commodity or IaaS storage solutions for static assets, it provides a means of > automatically synchronizing template and ISO assets across multiple zones. > > > > For more information about the design of the patch, please see the design > document (https://cwiki.apache.org/confluence/display/CLOUDSTACK/S3- > backed+Secondary+Storage). > > > > > > This addresses bug CLOUDSTACK-509. > > > > > > Diffs > > ----- > > > > api/src/com/cloud/api/commands/ListS3sCmd.java PRE-CREATION > > build/package.xml 09ed939 > > pom.xml 4a4276e > > server/src/com/cloud/configuration/ConfigurationManagerImpl.java > ef940e8 > > server/src/com/cloud/server/ManagementServerImpl.java 117be57 > > server/src/com/cloud/storage/dao/VMTemplateDao.java f5b6913 > > server/src/com/cloud/storage/dao/VMTemplateDaoImpl.java 2a0dfc8 > > server/src/com/cloud/storage/s3/S3ManagerImpl.java PRE-CREATION > > tools/marvin/marvin/cloudstackConnection.py c805213 > > tools/marvin/marvin/deployDataCenter.py bdf08cc > > utils/src/com/cloud/utils/db/DbUtil.java feef7b3 > > > > Diff: https://reviews.apache.org/r/8123/diff/ > > > > > > Testing > > ------- > > > > I am submitting patch to begin the feedback process while we complete > integration testing. I have verified that it does not interfere with normal > CloudStack operations when S3-backed Secondary Storage is disabled (the > default setting) . I have successfully tested operation of single zone > template and ISO scenarios on devcloud described in the design document. I > am currently working through some issues in our multi-zone test > environment to complete all scenarios described. The following are the > known deficiencies of the current implementation which I plan to correct in a > subsequent patch: > > > > * Cross zone garbage collection: When a global asset is deleted from one > zone's secondary storage, it is not deleted from the secondary storage of > other zones which have downloaded it from the object store > > * S3 Configuration Update: The API only supports adding an object store > configuration. Users should be able to edit the access key, secret key, > connection timeout. max error retries, and socket timeout. > > * Multi-threaded Uploads: Permit the upload of multiple assets to the > object store simultaneously to decrease the propagation latency across all > zones. > > > > > > Screenshots > > ----------- > > > > S3 Configuration Form > > https://reviews.apache.org/r/8123/s/13/ > > S3 Enable Menu on the Zone Tab > > https://reviews.apache.org/r/8123/s/14/ > > > > > > Thanks, > > > > John Burwell > > > >
