cloudstack-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Min Chen <min.c...@citrix.com>
Subject Re: S3 as secondary storage is broken in master
Date Sat, 02 Nov 2013 16:10:05 GMT
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" <shadowsor@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.chen@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

Mime
View raw message