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
>
