nevermind, I was pointing the test at the wrong revision
On Wed, Nov 6, 2013 at 10:17 PM, Marcus Sorensen <shadow...@gmail.com> wrote:
> Min,
> I ran my basic regression test for KVM, and the original bug I
> fixed in the commit you mention seems to be back. I'm not sure if
> it's related to your check-in, and I don't see any currently obvious
> reason why it would be so, but I'm investigating. Here's the log from
> latest master, you can see the CopyCommand is falling through to "not
> implemented yet"
>
> 2013-11-06 22:01:44,682 DEBUG [cloud.agent.Agent]
> (agentRequest-Handler-5:null) Request:Seq 1-384630832: { Cmd ,
> MgmtId: 52241639751, via: 1, Ver: v1, Flags: 100111,
> [{"org.apache.cloudstack.storage.command.CopyCommand":{"srcTO":{"org.apache.cloudstack.storage.to.TemplateObjectTO":{"path":"template/tmpl/1/201/d4201b3a-9fcf-35d1-986b-133cfd64779c.qcow2","origUrl":"http://marcus.mlsorensen.com/cloudstack-extras/tiny-centos-63.qcow2","uuid":"c27d031a-b695-41d7-9996-5b8e7688c489","id":201,"format":"QCOW2","accountId":1,"checksum":"44cd0e6330a59f031460bc18a40c95a2","hvm":true,"displayText":"tiny","imageDataStore":{"com.cloud.agent.api.to.NfsTO":{"_url":"nfs://172.17.10.10:/nfs/secondary","_role":"Image"}},"name":"201-1-2379913e-f51c-3e53-a0e5-f780c4782b16","hypervisorType":"KVM"}},"destTO":{"org.apache.cloudstack.storage.to.VolumeObjectTO":{"uuid":"94812f0c-c247-46d9-8689-c1600d09b9ae","volumeType":"ROOT","dataStore":{"org.apache.cloudstack.storage.to.PrimaryDataStoreTO":{"uuid":"c6669280-c5ad-4e91-8cab-5d7cb2f1455b","id":2,"poolType":"CLVM","host":"localhost","path":"vg0","port":0}},"name":"ROOT-6","size":1073741824,"volumeId":6,"vmName":"i-1-6-VM","accountId":1,"format":"QCOW2","id":6,"hypervisorType":"KVM"}},"executeInSequence":true,"wait":10800}}]
> }
>
> 2013-11-06 22:01:44,683 DEBUG [cloud.agent.Agent]
> (agentRequest-Handler-5:null) Processing command:
> org.apache.cloudstack.storage.command.CopyCommand
>
> 2013-11-06 22:01:44,684 DEBUG [cloud.agent.Agent]
> (agentRequest-Handler-5:null) Seq 1-384630832: { Ans: , MgmtId:
> 52241639751, via: 1, Ver: v1, Flags: 110,
> [{"com.cloud.agent.api.Answer":{"result":false,"details":"not
> implemented yet","wait":0}}] }
>
> On Sat, Nov 2, 2013 at 11:31 PM, Min Chen <min.c...@citrix.com> wrote:
>> Checked in fix 99ead3419c80fc7135a95f34ced7650eda3572fd to master.
>>
>> Thanks
>> -min
>>
>> From: Marcus Sorensen <shadow...@gmail.com>
>> Date: Saturday, November 2, 2013 9:25 AM
>> To: Min Chen <min.c...@citrix.com>
>> Cc: "dev@cloudstack.apache.org" <dev@cloudstack.apache.org>, "SuichII,
>> Christopher" <chris.su...@netapp.com>, Edison Su <edison...@citrix.com>
>> Subject: Re: S3 as secondary storage is broken in master
>>
>> OK. I was thinking that S3 was acting as secondary storage now but really it
>> still requires NFS as the intermediary, hence the "image cache". In that
>> case your suggestion would work.
>>
>> On Nov 2, 2013 10:10 AM, "Min Chen" <min.c...@citrix.com> wrote:
>>>
>>> No, Marcus. That piece of code worked in 4.2 for S3. Note that image ache
>>> store is also NfsTO.
>>>
>>> Thanks
>>> -min
>>>
>>> Sent from my iPhone
>>>
>>> > On Nov 2, 2013, at 7:18 AM, "Marcus Sorensen" <shadow...@gmail.com>
>>> > wrote:
>>> >
>>> > My only issue is that I believe copyTemplateToPrimaryStorage fails if
>>> > the source datastore is not NfsTO, so I'm not sure if this change will
>>> > do anything, or where S3 copy is actually handled. se
>>> > copyTemplateToPrimaryStorage:
>>> >
>>> > KVM:
>>> > if (!(imageStore instanceof NfsTO)) {
>>> > return new CopyCmdAnswer("unsupported protocol");
>>> > }
>>> >
>>> > Xen (all logic wrapped in this if statement):
>>> > if ((srcStore instanceof NfsTO) && (srcData.getObjectType()
>>> > == DataObjectType.TEMPLATE)) {
>>> >
>>> > VMware:
>>> > if (!(srcStore instanceof NfsTO)) {
>>> > return new CopyCmdAnswer("unsupported protocol");
>>> > }
>>> >
>>> > This line of code has a history. In 4.2 it didn't work for S3, either.
>>> > It specifically only allowed NFS:
>>> >
>>> > if ((srcData.getObjectType() == DataObjectType.TEMPLATE) &&
>>> > (srcDataStore instanceof NfsTO) && (destData.getDataStore().getRole()
>>> > == DataStoreRole.Primary)) {
>>> >
>>> > Then for a short while it was changed during some refactoring Chris
>>> > and Edison were working on:
>>> >
>>> > if ((srcData.getObjectType() == DataObjectType.TEMPLATE) &&
>>> > (destData.getObjectType() == DataObjectType.TEMPLATE &&
>>> > destData.getDataStore().getRole() == DataStoreRole.Primary)) {
>>> >
>>> > That broke CLVM, so I changed it to:
>>> >
>>> > if (srcData.getObjectType() == DataObjectType.TEMPLATE &&
>>> > srcData.getDataStore().getRole() == DataStoreRole.Image &&
>>> > destData.getDataStore().getRole() == DataStoreRole.Primary) {
>>> >
>>> >> On Fri, Nov 1, 2013 at 10:34 PM, Min Chen <min.c...@citrix.com> wrote:
>>> >> Hi Marcus,
>>> >>
>>> >> Your commit a504c004bf10555e5ea67ec89fe7bf6f00fe4622 broke S3
>>> >> functionality.
>>> >> With S3 as secondary storage, system vm cannot be started. Since for
>>> >> S3,
>>> >> copy template to primary will be from an ImageCache store. Your
>>> >> following
>>> >> line of code :
>>> >>
>>> >> if (srcData.getObjectType() == DataObjectType.TEMPLATE &&
>>> >> srcData.getDataStore().getRole() == DataStoreRole.Image &&
>>> >> destData.getDataStore().getRole() == DataStoreRole.Primary) {
>>> >> //copy template to primary storage
>>> >> return processor.copyTemplateToPrimaryStorage(cmd);
>>> >> }
>>> >>
>>> >> will not cover this case. I saw that you mentioned about this commit in
>>> >> another thread about CLVM broken in master. To fix this problem, we
>>> >> can
>>> >> change the above line to:
>>> >>
>>> >> if (srcData.getObjectType() == DataObjectType.TEMPLATE &&
>>> >> (srcData.getDataStore().getRole() == DataStoreRole.Image ||
>>> >> srcData.getDataStore().getRole() == DataStoreRole.ImageCache) &&
>>> >> destData.getDataStore().getRole() == DataStoreRole.Primary) {
>>> >> //copy template to primary storage
>>> >> return processor.copyTemplateToPrimaryStorage(cmd);
>>> >> }
>>> >>
>>> >> Edison/Chris, do you see any issue with this fix?
>>> >>
>>> >> Thanks
>>> >> -min
