Skip to content

Commit 9d105b6

Browse files
template: copy md5 mismatch (#3383)
Fixes #3191 When a template is registered, code stores md5sum of the downloaded file in the vm_template table. However, this downloaded file could be deleted after template installation if it is not an actual (.qcow2, .ova, etc.) file. When the user copies a template using copyTemplate API, the actual template file will be copied across the image stores. Matching checksum for the copied templated file and the stored value from the vm_template table will result in a mismatch. Changes will set an empty checksum value for the copied template while passing to download service which allows skipping wrong checksum check for the copied while install. However, this results in a change in checksum value for concerned template entry in vm_template table post template install. Co-authored-by: dahn <daan.hoogland@gmail.com>
1 parent 6baa598 commit 9d105b6

5 files changed

Lines changed: 156 additions & 126 deletions

File tree

engine/storage/image/src/main/java/org/apache/cloudstack/storage/image/TemplateServiceImpl.java

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1052,6 +1052,10 @@ public AsyncCallFuture<TemplateApiResult> copyTemplate(TemplateInfo srcTemplate,
10521052
DataObject templateOnStore = destStore.create(tmplForCopy);
10531053
templateOnStore.processEvent(Event.CreateOnlyRequested);
10541054

1055+
if (templateOnStore instanceof TemplateObject) {
1056+
((TemplateObject)templateOnStore).getImage().setChecksum(null);
1057+
} // else we don't know what to do.
1058+
10551059
if (s_logger.isDebugEnabled()) {
10561060
s_logger.debug("Invoke datastore driver createAsync to create template on destination store");
10571061
}

server/src/main/java/com/cloud/storage/download/DownloadMonitorImpl.java

Lines changed: 34 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -53,29 +53,21 @@
5353
import com.cloud.storage.RegisterVolumePayload;
5454
import com.cloud.storage.Storage.ImageFormat;
5555
import com.cloud.storage.VMTemplateStorageResourceAssoc.Status;
56-
import com.cloud.storage.Volume;
57-
import com.cloud.storage.dao.VMTemplateDao;
58-
import com.cloud.storage.dao.VolumeDao;
5956
import com.cloud.storage.template.TemplateConstants;
6057
import com.cloud.storage.upload.UploadListener;
61-
import com.cloud.template.VirtualMachineTemplate;
6258
import com.cloud.utils.component.ComponentContext;
6359
import com.cloud.utils.component.ManagerBase;
6460
import com.cloud.utils.exception.CloudRuntimeException;
6561

6662
@Component
6763
public class DownloadMonitorImpl extends ManagerBase implements DownloadMonitor {
68-
static final Logger s_logger = Logger.getLogger(DownloadMonitorImpl.class);
64+
static final Logger LOGGER = Logger.getLogger(DownloadMonitorImpl.class);
6965

7066
@Inject
7167
private TemplateDataStoreDao _vmTemplateStoreDao;
7268
@Inject
73-
private VolumeDao _volumeDao;
74-
@Inject
7569
private VolumeDataStoreDao _volumeStoreDao;
7670
@Inject
77-
private final VMTemplateDao _templateDao = null;
78-
@Inject
7971
private AgentManager _agentMgr;
8072
@Inject
8173
private ConfigurationDao _configDao;
@@ -94,7 +86,7 @@ public boolean configure(String name, Map<String, Object> params) {
9486

9587
String cert = configs.get("secstorage.ssl.cert.domain");
9688
if (!"realhostip.com".equalsIgnoreCase(cert)) {
97-
s_logger.warn("Only realhostip.com ssl cert is supported, ignoring self-signed and other certs");
89+
LOGGER.warn("Only realhostip.com ssl cert is supported, ignoring self-signed and other certs");
9890
}
9991

10092
_copyAuthPasswd = configs.get("secstorage.copy.password");
@@ -125,7 +117,7 @@ public boolean isTemplateUpdateable(Long templateId, Long storeId) {
125117

126118
private void initiateTemplateDownload(DataObject template, AsyncCompletionCallback<DownloadAnswer> callback) {
127119
boolean downloadJobExists = false;
128-
TemplateDataStoreVO vmTemplateStore = null;
120+
TemplateDataStoreVO vmTemplateStore;
129121
DataStore store = template.getDataStore();
130122

131123
vmTemplateStore = _vmTemplateStoreDao.findByStoreTemplate(store.getId(), template.getId());
@@ -141,7 +133,6 @@ private void initiateTemplateDownload(DataObject template, AsyncCompletionCallba
141133
Long maxTemplateSizeInBytes = getMaxTemplateSizeInBytes();
142134
if (vmTemplateStore != null) {
143135
start();
144-
VirtualMachineTemplate tmpl = _templateDao.findById(template.getId());
145136
DownloadCommand dcmd = new DownloadCommand((TemplateObjectTO)(template.getTO()), maxTemplateSizeInBytes);
146137
dcmd.setProxy(getHttpProxy());
147138
if (downloadJobExists) {
@@ -153,7 +144,7 @@ private void initiateTemplateDownload(DataObject template, AsyncCompletionCallba
153144
EndPoint ep = _epSelector.select(template);
154145
if (ep == null) {
155146
String errMsg = "There is no secondary storage VM for downloading template to image store " + store.getName();
156-
s_logger.warn(errMsg);
147+
LOGGER.warn(errMsg);
157148
throw new CloudRuntimeException(errMsg);
158149
}
159150
DownloadListener dl = new DownloadListener(ep, store, template, _timer, this, dcmd, callback);
@@ -164,14 +155,14 @@ private void initiateTemplateDownload(DataObject template, AsyncCompletionCallba
164155
// DownloadListener to use
165156
// new ObjectInDataStore.State transition. TODO: fix this later
166157
// to be able to remove downloadState from template_store_ref.
167-
s_logger.info("found existing download job");
158+
LOGGER.info("found existing download job");
168159
dl.setCurrState(vmTemplateStore.getDownloadState());
169160
}
170161

171162
try {
172163
ep.sendMessageAsync(dcmd, new UploadListener.Callback(ep.getId(), dl));
173164
} catch (Exception e) {
174-
s_logger.warn("Unable to start /resume download of template " + template.getId() + " to " + store.getName(), e);
165+
LOGGER.warn("Unable to start /resume download of template " + template.getId() + " to " + store.getName(), e);
175166
dl.setDisconnected();
176167
dl.scheduleStatusCheck(RequestType.GET_OR_RESTART);
177168
}
@@ -187,12 +178,12 @@ public void downloadTemplateToStorage(DataObject template, AsyncCompletionCallba
187178
if (template.getUri() != null) {
188179
initiateTemplateDownload(template, callback);
189180
} else {
190-
s_logger.info("Template url is null, cannot download");
181+
LOGGER.info("Template url is null, cannot download");
191182
DownloadAnswer ans = new DownloadAnswer("Template url is null", Status.UNKNOWN);
192183
callback.complete(ans);
193184
}
194185
} else {
195-
s_logger.info("Template download is already in progress or already downloaded");
186+
LOGGER.info("Template download is already in progress or already downloaded");
196187
DownloadAnswer ans =
197188
new DownloadAnswer("Template download is already in progress or already downloaded", Status.UNKNOWN);
198189
callback.complete(ans);
@@ -203,7 +194,7 @@ public void downloadTemplateToStorage(DataObject template, AsyncCompletionCallba
203194
@Override
204195
public void downloadVolumeToStorage(DataObject volume, AsyncCompletionCallback<DownloadAnswer> callback) {
205196
boolean downloadJobExists = false;
206-
VolumeDataStoreVO volumeHost = null;
197+
VolumeDataStoreVO volumeHost;
207198
DataStore store = volume.getDataStore();
208199
VolumeInfo volInfo = (VolumeInfo)volume;
209200
RegisterVolumePayload payload = (RegisterVolumePayload)volInfo.getpayload();
@@ -214,7 +205,7 @@ public void downloadVolumeToStorage(DataObject volume, AsyncCompletionCallback<D
214205
volumeHost = _volumeStoreDao.findByStoreVolume(store.getId(), volume.getId());
215206
if (volumeHost == null) {
216207
volumeHost = new VolumeDataStoreVO(store.getId(), volume.getId(), new Date(), 0, Status.NOT_DOWNLOADED, null, null, "jobid0000", null, url, checkSum);
217-
_volumeStoreDao.persist(volumeHost);
208+
volumeHost = _volumeStoreDao.persist(volumeHost);
218209
} else if ((volumeHost.getJobId() != null) && (volumeHost.getJobId().length() > 2)) {
219210
downloadJobExists = true;
220211
} else {
@@ -225,35 +216,32 @@ public void downloadVolumeToStorage(DataObject volume, AsyncCompletionCallback<D
225216
}
226217

227218
Long maxVolumeSizeInBytes = getMaxVolumeSizeInBytes();
228-
if (volumeHost != null) {
229-
start();
230-
Volume vol = _volumeDao.findById(volume.getId());
231-
DownloadCommand dcmd = new DownloadCommand((VolumeObjectTO)(volume.getTO()), maxVolumeSizeInBytes, checkSum, url, format);
232-
dcmd.setProxy(getHttpProxy());
233-
if (downloadJobExists) {
234-
dcmd = new DownloadProgressCommand(dcmd, volumeHost.getJobId(), RequestType.GET_OR_RESTART);
235-
dcmd.setResourceType(ResourceType.VOLUME);
236-
}
219+
start();
220+
DownloadCommand dcmd = new DownloadCommand((VolumeObjectTO)(volume.getTO()), maxVolumeSizeInBytes, checkSum, url, format);
221+
dcmd.setProxy(getHttpProxy());
222+
if (downloadJobExists) {
223+
dcmd = new DownloadProgressCommand(dcmd, volumeHost.getJobId(), RequestType.GET_OR_RESTART);
224+
dcmd.setResourceType(ResourceType.VOLUME);
225+
}
237226

238-
EndPoint ep = _epSelector.select(volume);
239-
if (ep == null) {
240-
s_logger.warn("There is no secondary storage VM for image store " + store.getName());
241-
return;
242-
}
243-
DownloadListener dl = new DownloadListener(ep, store, volume, _timer, this, dcmd, callback);
244-
ComponentContext.inject(dl); // auto-wired those injected fields in DownloadListener
227+
EndPoint ep = _epSelector.select(volume);
228+
if (ep == null) {
229+
LOGGER.warn("There is no secondary storage VM for image store " + store.getName());
230+
return;
231+
}
232+
DownloadListener dl = new DownloadListener(ep, store, volume, _timer, this, dcmd, callback);
233+
ComponentContext.inject(dl); // auto-wired those injected fields in DownloadListener
245234

246-
if (downloadJobExists) {
247-
dl.setCurrState(volumeHost.getDownloadState());
248-
}
235+
if (downloadJobExists) {
236+
dl.setCurrState(volumeHost.getDownloadState());
237+
}
249238

250-
try {
251-
ep.sendMessageAsync(dcmd, new UploadListener.Callback(ep.getId(), dl));
252-
} catch (Exception e) {
253-
s_logger.warn("Unable to start /resume download of volume " + volume.getId() + " to " + store.getName(), e);
254-
dl.setDisconnected();
255-
dl.scheduleStatusCheck(RequestType.GET_OR_RESTART);
256-
}
239+
try {
240+
ep.sendMessageAsync(dcmd, new UploadListener.Callback(ep.getId(), dl));
241+
} catch (Exception e) {
242+
LOGGER.warn("Unable to start /resume download of volume " + volume.getId() + " to " + store.getName(), e);
243+
dl.setDisconnected();
244+
dl.scheduleStatusCheck(RequestType.GET_OR_RESTART);
257245
}
258246
}
259247

@@ -279,8 +267,7 @@ private Proxy getHttpProxy() {
279267
}
280268
try {
281269
URI uri = new URI(_proxy);
282-
Proxy prx = new Proxy(uri);
283-
return prx;
270+
return new Proxy(uri);
284271
} catch (URISyntaxException e) {
285272
return null;
286273
}

server/src/main/java/com/cloud/template/TemplateManagerImpl.java

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -927,8 +927,6 @@ public VirtualMachineTemplate copyTemplate(CopyTemplateCmd cmd) throws StorageUn
927927
}
928928
}
929929
}
930-
931-
932930
}
933931

934932
if ((destZoneIds != null) && (destZoneIds.size() > failedZones.size())){

0 commit comments

Comments
 (0)