cloudstack-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Marcus Sorensen <shadow...@gmail.com>
Subject Re: S3 as secondary storage is broken in master
Date Sat, 02 Nov 2013 14:18:03 GMT
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