incubator-cloudstack-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Edison Su <Edison...@citrix.com>
Subject RE: Review Request: S3-backed Secondary Storage
Date Wed, 19 Dec 2012 21:58:45 GMT
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:noreply@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
> >
> >

Mime
View raw message