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
