incubator-cloudstack-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Donal Lafferty" <donal.laffe...@citrix.com>
Subject Re: Review Request: Hyper-V 2012 Plugin Phase 1
Date Thu, 31 Jan 2013 00:30:02 GMT


> On Jan. 30, 2013, 1:18 p.m., Murali Reddy wrote:
> > Donal,
> > 
> > I need some details to review your change, which i could not find from the FS. 
> > 
> > How is Hyper-V host discovery is supposed to happen? Do you expect agent to be started
on the Hyper-V host by admin or agent is started by CloudStack?
> > 
> > How does the bootstrapping happen to copy templates from secondary to primary? Do
you assume a secondary storage? 
> > 
> > For phase 1, you plan to support only local storage as primary storage?
> > 
> > Is there a notion of Hyper-V cluster or all of them are going to be individual hosts?
> > 
> > 
> > couple of comments and more questions?
> > 
> >

The Hyper-V connected agent and Discoverer base the hosts GUID on the IP address used to manager
the Hyper-V server.  When you add a Hyper-V server using the GUI, what should happen is that
'find' will look up the host by GUID and update the zone/pod/cluster identifiers to valid
values.

Templates are copied to a Hyper-V server via the image motion service.  The default image
motion service driver uses a PrimaryStorageDownloadCommand to send URLs to the agent, which
it then downloads.  Two types of schema are supported:  nfs:// and http://.  HTTP URIs are
downloaded.  NFS schemas are accessed via a mount.  The admin is responsible for mounting
the NFS server on the Hyper-V server.  Where the Hyper-V server does not have an NFS client,
I recommend mounting a Windows Server 2012 share exposed as NFS and SMB.  The admin must tell
the agent on Hyper-V the name of this share using an option in the agent.properties file.

For Phase 1, the primary storage is local storage, and the name of the folder is given in
agent.properties.  The admin can manually mount iSCSI, SMB, etc.

The cluster group is imposed by the management server.  The Hyper-V servers have no knowledge
of who they're grouped with.


> On Jan. 30, 2013, 1:18 p.m., Murali Reddy wrote:
> > plugins/api/rate-limit/test/org/apache/cloudstack/ratelimit/ApiRateLimitTest.java,
line 219
> > <https://reviews.apache.org/r/9143/diff/1/?file=253041#file253041line219>
> >
> >     is this change intended?

Yes.  The fix overcomes platform-specific assumptions in the code WRT Java's clock.


> On Jan. 30, 2013, 1:18 p.m., Murali Reddy wrote:
> > agent/src/com/cloud/agent/resource/DummyResource.java, line 171
> > <https://reviews.apache.org/r/9143/diff/1/?file=253034#file253034line171>
> >
> >     Why is dummy resource required in Hyper-V case? I think KVM code is no longer
using it.

KVM continues to use a Dummy resource.  AFAIK, the management server needs a reference to
a plugin's ServerResource.  The dummy is a proxy for this resource when the resource sits
on a remote server.


> On Jan. 30, 2013, 1:18 p.m., Murali Reddy wrote:
> > agent/conf/agent.properties, line 39
> > <https://reviews.apache.org/r/9143/diff/1/?file=253031#file253031line39>
> >
> >     Why is cluster, pod etc details are required to be in agent.properties by default?

This is inherited from the KVM approach.  The defaults are required to populate these values
if the agent is not passed them in the agent command line parameters.  

With Hyper-V, we can't pass the options, so I've left them in the agent.properties to avoid
making the command line launch complex.


> On Jan. 30, 2013, 1:18 p.m., Murali Reddy wrote:
> > agent/src/com/cloud/agent/AgentShell.java, line 406
> > <https://reviews.apache.org/r/9143/diff/1/?file=253033#file253033line406>
> >
> >     why is this hard coded instead of getting version from package

AFAIK, there is no code in the package to generate the version number.  


> On Jan. 30, 2013, 1:18 p.m., Murali Reddy wrote:
> > plugins/hypervisors/hyperv/src/com/cloud/hypervisor/hyperv/discoverer/HypervServerDiscoverer.java,
line 202
> > <https://reviews.apache.org/r/9143/diff/1/?file=253058#file253058line202>
> >
> >     How does find() launches the agent on Hyper-V host? I could not understand how
waitForHostConnect() will succeed.

find() can't launch the agent.  That has to be done by an admin.  What find should do is change
the dc/pod/cluster values from the defaults given by the agent to the values that the 'find'
method wants.


> On Jan. 30, 2013, 1:18 p.m., Murali Reddy wrote:
> > plugins/hypervisors/hyperv/scripts/hyperv/baseops.py, line 4
> > <https://reviews.apache.org/r/9143/diff/1/?file=253049#file253049line4>
> >
> >     Check the license on all python files. there is Citrix copy  right on them.

The notices reflect the OpenStack origins of the code.  I've sent an email to the mailing
group to see whether anything needs to be done.


- Donal


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9143/#review15833
-----------------------------------------------------------


On Jan. 29, 2013, 9:43 p.m., Donal Lafferty wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/9143/
> -----------------------------------------------------------
> 
> (Updated Jan. 29, 2013, 9:43 p.m.)
> 
> 
> Review request for cloudstack and Chip Childers.
> 
> 
> Description
> -------
> 
> To include Hyper-V plugin in a build, add '-DhyperV' to the mvn options.  E.g. mvn clean
install -DhyperV
> 
> Overview https://cwiki.apache.org/CLOUDSTACK/hyper-v-2012-30-support.html
> 
> tl;dr: First cut of HyperV plugin, cloud-api changes to support VHDX image type, and
test changes to deal with Windows clock accuracy and URI semantics.
> 
> 
> This addresses bug CLOUDSTACK-999.
> 
> 
> Diffs
> -----
> 
>   agent/conf/agent.properties 74cfd1c21d6b2910be3859b4f570feee3ba172d5 
>   agent/conf/log4j-cloud.xml PRE-CREATION 
>   agent/src/com/cloud/agent/AgentShell.java e3d1063e6b8c148d765c2b185896ead2091769b3

>   agent/src/com/cloud/agent/resource/DummyResource.java 573f639b06d92db614e0cc60eee63d94d1160426

>   agent/test/com/cloud/agent/TestAgentShell.java d7210acbef30044e86d734c31bea870381653557

>   api/src/com/cloud/storage/Storage.java fba12b62d3d5205ff32ab2f67065b45d011bcb05 
>   client/pom.xml 7ebe50c48f9a692fc610871cfdb21c12370afd3a 
>   client/tomcatconf/components.xml.in c41d4f4f18f3a505ea97a032d348718a915bbf10 
>   core/src/com/cloud/hypervisor/hyperv/resource/HypervDummyResourceBase.java 6e52924db28bf9dc4749287d2ccbb7836b6d55e3

>   core/src/com/cloud/hypervisor/hyperv/resource/HypervResource.java ede6301d9c354f69a3a9db85830d0d0d87008495

>   plugins/api/rate-limit/test/org/apache/cloudstack/ratelimit/ApiRateLimitTest.java 502b15cf316374104ff64eb5c7f3b9026303efc5

>   plugins/hypervisors/hyperv/conf/agent.properties PRE-CREATION 
>   plugins/hypervisors/hyperv/conf/developer.properties.template PRE-CREATION 
>   plugins/hypervisors/hyperv/conf/environment.properties.in PRE-CREATION 
>   plugins/hypervisors/hyperv/conf/log4j-cloud.xml PRE-CREATION 
>   plugins/hypervisors/hyperv/conf/log4j-cloud.xml.in PRE-CREATION 
>   plugins/hypervisors/hyperv/pom.xml PRE-CREATION 
>   plugins/hypervisors/hyperv/scripts/hyperv/.pydevproject PRE-CREATION 
>   plugins/hypervisors/hyperv/scripts/hyperv/baseops.py PRE-CREATION 
>   plugins/hypervisors/hyperv/scripts/hyperv/cloudstackcmds.py PRE-CREATION 
>   plugins/hypervisors/hyperv/scripts/hyperv/constants.py PRE-CREATION 
>   plugins/hypervisors/hyperv/scripts/hyperv/hypervlog.conf PRE-CREATION 
>   plugins/hypervisors/hyperv/scripts/hyperv/log.py PRE-CREATION 
>   plugins/hypervisors/hyperv/scripts/hyperv/power_state.py PRE-CREATION 
>   plugins/hypervisors/hyperv/scripts/hyperv/vmops.py PRE-CREATION 
>   plugins/hypervisors/hyperv/scripts/hyperv/vmutils.py PRE-CREATION 
>   plugins/hypervisors/hyperv/scripts/hyperv/volumeops.py PRE-CREATION 
>   plugins/hypervisors/hyperv/src/com/cloud/hypervisor/hyperv/discoverer/HypervServerDiscoverer.java
PRE-CREATION 
>   plugins/hypervisors/hyperv/src/com/cloud/hypervisor/hyperv/guru/HypervGuru.java PRE-CREATION

>   plugins/hypervisors/hyperv/src/com/cloud/hypervisor/hyperv/resource/HypervDummyResourceBase.java
PRE-CREATION 
>   plugins/hypervisors/hyperv/src/com/cloud/hypervisor/hyperv/resource/HypervResource.java
PRE-CREATION 
>   plugins/hypervisors/hyperv/src/com/cloud/hypervisor/hyperv/resource/PythonUtils.java
PRE-CREATION 
>   plugins/hypervisors/hyperv/src/com/cloud/hypervisor/hyperv/storage/HypervPhysicalDisk.java
PRE-CREATION 
>   plugins/hypervisors/hyperv/src/com/cloud/hypervisor/hyperv/storage/HypervStoragePool.java
PRE-CREATION 
>   plugins/hypervisors/hyperv/src/com/cloud/hypervisor/hyperv/storage/HypervStoragePoolManager.java
PRE-CREATION 
>   plugins/hypervisors/hyperv/src/com/cloud/hypervisor/hyperv/storage/StorageAdaptor.java
PRE-CREATION 
>   plugins/hypervisors/hyperv/src/com/cloud/hypervisor/hyperv/storage/WindowsStorageAdaptor.java
PRE-CREATION 
>   plugins/hypervisors/hyperv/src/com/cloud/hypervisor/hyperv/storage/WindowsStoragePool.java
PRE-CREATION 
>   plugins/hypervisors/hyperv/test/com/cloud/hypervisor/hyperv/test/HypervResourceTest.java
PRE-CREATION 
>   plugins/pom.xml 7bb60a990fbb3d65f514e8b53155162a61602a33 
>   server/src/com/cloud/configuration/Config.java 4ae144e6ce116b34f6a62b9bdfc5f9262984a448

>   server/src/com/cloud/hypervisor/guru/HypervGuru.java 1d59afd93a7a6e989e49548741de263ad6626baf

>   server/src/com/cloud/hypervisor/hyperv/HypervServerDiscoverer.java 6a1cd67d8f3d3b5d2ce99fae15ee58a9583795a2

>   server/src/com/cloud/resource/ResourceManagerImpl.java f82424a10c25aa7cd51acaed5744242c885920ee

>   server/src/com/cloud/storage/StorageManagerImpl.java 07f4d8ac7cb3dcf7dbb57baf473dbe7b5f597b7b

>   server/src/com/cloud/template/HyervisorTemplateAdapter.java c80d1de0fbf1f7b58c5526ad7e3744ed382f0017

>   ui/scripts/templates.js 040ce4a92c145c7f79c474b7b722be1d019c42a7 
> 
> Diff: https://reviews.apache.org/r/9143/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Donal Lafferty
> 
>


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