From a4e6784e0b5bbef8135271a0c1c5f990ed2115b3 Mon Sep 17 00:00:00 2001 From: Min Chen Date: Tue, 21 May 2013 14:43:32 -0700 Subject: [PATCH] CLOUDSTACK-2578: NPE in deleting template from S3. --- .../datastore/driver/CloudStackImageStoreDriverImpl.java | 5 +++-- .../storage/datastore/driver/S3ImageStoreDriverImpl.java | 9 ++++----- .../datastore/driver/SwiftImageStoreDriverImpl.java | 5 +++-- .../storage/resource/NfsSecondaryStorageResource.java | 2 +- 4 files changed, 11 insertions(+), 10 deletions(-) diff --git a/plugins/storage/image/default/src/org/apache/cloudstack/storage/datastore/driver/CloudStackImageStoreDriverImpl.java b/plugins/storage/image/default/src/org/apache/cloudstack/storage/datastore/driver/CloudStackImageStoreDriverImpl.java index 0c46d5e2ea7..71afc13e13c 100644 --- a/plugins/storage/image/default/src/org/apache/cloudstack/storage/datastore/driver/CloudStackImageStoreDriverImpl.java +++ b/plugins/storage/image/default/src/org/apache/cloudstack/storage/datastore/driver/CloudStackImageStoreDriverImpl.java @@ -276,8 +276,9 @@ public class CloudStackImageStoreDriverImpl implements ImageStoreDriver { // TODO: need to understand why we need to mark destroyed in // template_store_ref table here instead of in callback. // Currently I did that in callback, so I removed previous code to mark template_host_ref - - UsageEventUtils.publishUsageEvent(eventType, account.getId(), sZoneId, templateId, null, null, null); + if ( sZoneId != null ){ + UsageEventUtils.publishUsageEvent(eventType, account.getId(), sZoneId, templateId, null, null, null); + } // get installpath of this template on image store TemplateDataStoreVO tmplStore = _templateStoreDao.findByStoreTemplate(storeId, templateId); diff --git a/plugins/storage/image/s3/src/org/apache/cloudstack/storage/datastore/driver/S3ImageStoreDriverImpl.java b/plugins/storage/image/s3/src/org/apache/cloudstack/storage/datastore/driver/S3ImageStoreDriverImpl.java index 9487bbe2832..1c6e17e8e1b 100644 --- a/plugins/storage/image/s3/src/org/apache/cloudstack/storage/datastore/driver/S3ImageStoreDriverImpl.java +++ b/plugins/storage/image/s3/src/org/apache/cloudstack/storage/datastore/driver/S3ImageStoreDriverImpl.java @@ -286,11 +286,10 @@ public class S3ImageStoreDriverImpl implements ImageStoreDriver { eventType = EventTypes.EVENT_TEMPLATE_DELETE; } - // TODO: need to understand why we need to mark destroyed in - // template_store_ref table here instead of in callback. - // Currently I did that in callback, so I removed previous code to mark template_host_ref - - UsageEventUtils.publishUsageEvent(eventType, account.getId(), sZoneId, templateId, null, null, null); + if ( sZoneId != null ){ + //TODO: how to handle region wide usage data where sZoneId == null + UsageEventUtils.publishUsageEvent(eventType, account.getId(), sZoneId, templateId, null, null, null); + } List userVmUsingIso = _userVmJoinDao.listActiveByIsoId(templateId); // check if there is any VM using this ISO. diff --git a/plugins/storage/image/swift/src/org/apache/cloudstack/storage/datastore/driver/SwiftImageStoreDriverImpl.java b/plugins/storage/image/swift/src/org/apache/cloudstack/storage/datastore/driver/SwiftImageStoreDriverImpl.java index 69e03495b2b..590b653bdff 100644 --- a/plugins/storage/image/swift/src/org/apache/cloudstack/storage/datastore/driver/SwiftImageStoreDriverImpl.java +++ b/plugins/storage/image/swift/src/org/apache/cloudstack/storage/datastore/driver/SwiftImageStoreDriverImpl.java @@ -281,8 +281,9 @@ public class SwiftImageStoreDriverImpl implements ImageStoreDriver { // TODO: need to understand why we need to mark destroyed in // template_store_ref table here instead of in callback. // Currently I did that in callback, so I removed previous code to mark template_host_ref - - UsageEventUtils.publishUsageEvent(eventType, account.getId(), sZoneId, templateId, null, null, null); + if (sZoneId != null){ + UsageEventUtils.publishUsageEvent(eventType, account.getId(), sZoneId, templateId, null, null, null); + } List userVmUsingIso = _userVmJoinDao.listActiveByIsoId(templateId); // check if there is any VM using this ISO. diff --git a/services/secondary-storage/src/org/apache/cloudstack/storage/resource/NfsSecondaryStorageResource.java b/services/secondary-storage/src/org/apache/cloudstack/storage/resource/NfsSecondaryStorageResource.java index 24b2e0b8c2d..6fbb06b3f2c 100755 --- a/services/secondary-storage/src/org/apache/cloudstack/storage/resource/NfsSecondaryStorageResource.java +++ b/services/secondary-storage/src/org/apache/cloudstack/storage/resource/NfsSecondaryStorageResource.java @@ -1535,7 +1535,7 @@ public class NfsSecondaryStorageResource extends ServerResourceBase implements S final String bucket = s3.getBucketName(); try { S3Utils.deleteDirectory(s3, bucket, path); - return new Answer(cmd, true, String.format("Deleted template %1%s from bucket %2$s.", path, bucket)); + return new Answer(cmd, true, String.format("Deleted template %1$s from bucket %2$s.", path, bucket)); } catch (Exception e) { final String errorMessage = String.format("Failed to delete template %1$s from bucket %2$s due to the following error: %3$s", path, bucket, e.getMessage());