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
