diff --git a/core/src/com/cloud/storage/template/OVAProcessor.java b/core/src/com/cloud/storage/template/OVAProcessor.java index 3d7f7a23bd7..e0003b1609c 100644 --- a/core/src/com/cloud/storage/template/OVAProcessor.java +++ b/core/src/com/cloud/storage/template/OVAProcessor.java @@ -71,7 +71,7 @@ public class OVAProcessor extends AdapterBase implements Processor { String result = command.execute(); if (result != null) { s_logger.info("failed to untar OVA package due to " + result + ". templatePath: " + templatePath + ", templateName: " + templateName); - return null; + throw new InternalErrorException("failed to untar OVA package"); } FormatInfo info = new FormatInfo(); diff --git a/core/src/com/cloud/storage/template/QCOW2Processor.java b/core/src/com/cloud/storage/template/QCOW2Processor.java index 2c66415bf96..096b0b6f32a 100644 --- a/core/src/com/cloud/storage/template/QCOW2Processor.java +++ b/core/src/com/cloud/storage/template/QCOW2Processor.java @@ -27,6 +27,7 @@ import java.util.Map; import javax.ejb.Local; import javax.naming.ConfigurationException; +import com.cloud.exception.InternalErrorException; import org.apache.log4j.Logger; import com.cloud.storage.Storage.ImageFormat; @@ -42,7 +43,7 @@ public class QCOW2Processor extends AdapterBase implements Processor { private StorageLayer _storage; @Override - public FormatInfo process(String templatePath, ImageFormat format, String templateName) { + public FormatInfo process(String templatePath, ImageFormat format, String templateName) throws InternalErrorException { if (format != null) { s_logger.debug("We currently don't handle conversion from " + format + " to QCOW2."); return null; @@ -64,10 +65,10 @@ public class QCOW2Processor extends AdapterBase implements Processor { info.size = _storage.getSize(qcow2Path); try { - info.virtualSize = getVirtualSize(qcow2File); + info.virtualSize = getTemplateVirtualSize(qcow2File); } catch (IOException e) { s_logger.error("Unable to get virtual size from " + qcow2File.getName()); - return null; + throw new InternalErrorException("unable to get virtual size from qcow2 file"); } return info; @@ -75,6 +76,16 @@ public class QCOW2Processor extends AdapterBase implements Processor { @Override public long getVirtualSize(File file) throws IOException { + try { + long size = getTemplateVirtualSize(file); + return size; + } catch (Exception e) { + s_logger.info("[ignored]" + "failed to get template virtual size for QCOW2: " + e.getLocalizedMessage()); + } + return file.length(); + } + + protected long getTemplateVirtualSize(File file) throws IOException { byte[] b = new byte[8]; try (FileInputStream strm = new FileInputStream(file)) { if (strm.skip(VIRTUALSIZE_HEADER_LOCATION) != VIRTUALSIZE_HEADER_LOCATION) { diff --git a/core/src/com/cloud/storage/template/TemplateLocation.java b/core/src/com/cloud/storage/template/TemplateLocation.java index ea785b206bd..e52a635dc68 100644 --- a/core/src/com/cloud/storage/template/TemplateLocation.java +++ b/core/src/com/cloud/storage/template/TemplateLocation.java @@ -180,7 +180,9 @@ public class TemplateLocation { deleteFormat(newInfo.format); if (!checkFormatValidity(newInfo)) { - s_logger.warn("Format is invalid "); + s_logger.warn("Format is invalid"); + s_logger.debug("Format: " + newInfo.format + " size: " + newInfo.size + " virtualsize: " + newInfo.virtualSize + " filename: " + newInfo.filename); + s_logger.debug("format, filename cannot be null and size, virtual size should be > 0 "); return false; } diff --git a/core/src/com/cloud/storage/template/VhdProcessor.java b/core/src/com/cloud/storage/template/VhdProcessor.java index 2974c75c190..0b68f895fd6 100644 --- a/core/src/com/cloud/storage/template/VhdProcessor.java +++ b/core/src/com/cloud/storage/template/VhdProcessor.java @@ -27,6 +27,7 @@ import java.util.Map; import javax.ejb.Local; import javax.naming.ConfigurationException; +import com.cloud.exception.InternalErrorException; import org.apache.log4j.Logger; import com.cloud.storage.Storage.ImageFormat; @@ -52,7 +53,7 @@ public class VhdProcessor extends AdapterBase implements Processor { private byte[][] citrixCreatorApp = { {0x74, 0x61, 0x70, 0x00}, {0x43, 0x54, 0x58, 0x53}}; /*"tap ", and "CTXS"*/ @Override - public FormatInfo process(String templatePath, ImageFormat format, String templateName) { + public FormatInfo process(String templatePath, ImageFormat format, String templateName) throws InternalErrorException { if (format != null) { s_logger.debug("We currently don't handle conversion from " + format + " to VHD."); return null; @@ -72,10 +73,10 @@ public class VhdProcessor extends AdapterBase implements Processor { info.size = _storage.getSize(vhdPath); try { - info.virtualSize = getVirtualSize(vhdFile); + info.virtualSize = getTemplateVirtualSize(vhdFile); } catch (IOException e) { s_logger.error("Unable to get the virtual size for " + vhdPath); - return null; + throw new InternalErrorException("unable to get virtual size from vhd file"); } return info; @@ -83,6 +84,16 @@ public class VhdProcessor extends AdapterBase implements Processor { @Override public long getVirtualSize(File file) throws IOException { + try { + long size = getTemplateVirtualSize(file); + return size; + } catch (Exception e) { + s_logger.info("[ignored]" + "failed to get template virtual size for VHD: " + e.getLocalizedMessage()); + } + return file.length(); + } + + protected long getTemplateVirtualSize(File file) throws IOException { byte[] currentSize = new byte[8]; byte[] creatorApp = new byte[4]; diff --git a/core/test/com/cloud/storage/template/OVAProcessorTest.java b/core/test/com/cloud/storage/template/OVAProcessorTest.java new file mode 100644 index 00000000000..f2f0f4712dc --- /dev/null +++ b/core/test/com/cloud/storage/template/OVAProcessorTest.java @@ -0,0 +1,138 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package com.cloud.storage.template; + +import com.cloud.exception.InternalErrorException; +import com.cloud.storage.Storage; +import com.cloud.storage.StorageLayer; +import com.cloud.utils.script.Script; +import org.junit.Assert; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.Mock; +import org.mockito.Mockito; +import org.powermock.api.mockito.PowerMockito; +import org.powermock.core.classloader.annotations.PrepareForTest; +import org.powermock.modules.junit4.PowerMockRunner; + +import java.io.File; +import java.util.HashMap; +import java.util.Map; + +@RunWith(PowerMockRunner.class) +@PrepareForTest(OVAProcessor.class) +public class OVAProcessorTest { + OVAProcessor processor; + + @Mock + StorageLayer mockStorageLayer; + + @Before + public void setUp() throws Exception { + processor = PowerMockito.spy(new OVAProcessor()); + Map params = new HashMap(); + params.put(StorageLayer.InstanceConfigKey, mockStorageLayer); + processor.configure("OVA Processor", params); + } + + @Test(expected = InternalErrorException.class) + public void testProcessWhenUntarFails() throws Exception { + String templatePath = "/tmp"; + String templateName = "template"; + + Mockito.when(mockStorageLayer.exists(Mockito.anyString())).thenReturn(true); + + Script mockScript = Mockito.mock(Script.class); + PowerMockito.whenNew(Script.class).withAnyArguments().thenReturn(mockScript); + PowerMockito.when(mockScript.execute()).thenReturn("error while untaring the file"); + + processor.process(templatePath, null, templateName); + } + + @Test(expected = InternalErrorException.class) + public void testProcessWhenVirtualSizeThrowsException() throws Exception { + String templatePath = "/tmp"; + String templateName = "template"; + + Mockito.when(mockStorageLayer.exists(Mockito.anyString())).thenReturn(true); + Mockito.when(mockStorageLayer.getSize(Mockito.anyString())).thenReturn(1000l); + Mockito.doThrow(new InternalErrorException("virtual size calculation failed")).when(processor).getTemplateVirtualSize(Mockito.anyString(), Mockito.anyString()); + + Script mockScript = Mockito.mock(Script.class); + PowerMockito.whenNew(Script.class).withAnyArguments().thenReturn(mockScript); + PowerMockito.when(mockScript.execute()).thenReturn(null); + + processor.process(templatePath, null, templateName); + } + + @Test + public void testProcess() throws Exception { + String templatePath = "/tmp"; + String templateName = "template"; + long virtualSize = 2000; + long actualSize = 1000; + + Mockito.when(mockStorageLayer.exists(Mockito.anyString())).thenReturn(true); + Mockito.when(mockStorageLayer.getSize(Mockito.anyString())).thenReturn(actualSize); + Mockito.doReturn(virtualSize).when(processor).getTemplateVirtualSize(Mockito.anyString(), Mockito.anyString()); + + Script mockScript = Mockito.mock(Script.class); + PowerMockito.whenNew(Script.class).withAnyArguments().thenReturn(mockScript); + PowerMockito.when(mockScript.execute()).thenReturn(null); + + Processor.FormatInfo info = processor.process(templatePath, null, templateName); + Assert.assertEquals(Storage.ImageFormat.OVA, info.format); + Assert.assertEquals("actual size:", actualSize, info.size); + Assert.assertEquals("virtual size:", virtualSize, info.virtualSize); + Assert.assertEquals("template name:", templateName + ".ova", info.filename); + } + + @Test + public void testGetVirtualSizeWhenVirtualSizeThrowsException() throws Exception { + long virtualSize = 2000; + long actualSize = 1000; + String templatePath = "/tmp"; + String templateName = "template"; + File mockFile = Mockito.mock(File.class); + Mockito.when(mockFile.length()).thenReturn(actualSize); + Mockito.when(mockFile.getParent()).thenReturn(templatePath); + Mockito.when(mockFile.getName()).thenReturn(templateName); + Mockito.doThrow(new InternalErrorException("virtual size calculation failed")).when(processor).getTemplateVirtualSize(templatePath, templateName); + Assert.assertEquals(actualSize, processor.getVirtualSize(mockFile)); + Mockito.verify(mockFile, Mockito.times(1)).length(); + } + + @Test + public void testGetVirtualSize() throws Exception { + long virtualSize = 2000; + long actualSize = 1000; + String templatePath = "/tmp"; + String templateName = "template"; + File mockFile = Mockito.mock(File.class); + Mockito.when(mockFile.length()).thenReturn(actualSize); + Mockito.when(mockFile.getParent()).thenReturn(templatePath); + Mockito.when(mockFile.getName()).thenReturn(templateName); + Mockito.doReturn(virtualSize).when(processor).getTemplateVirtualSize(templatePath, templateName); + Assert.assertEquals(virtualSize, processor.getVirtualSize(mockFile)); + Mockito.verify(mockFile, Mockito.times(0)).length(); + } + +} \ No newline at end of file diff --git a/core/test/com/cloud/storage/template/QCOW2ProcessorTest.java b/core/test/com/cloud/storage/template/QCOW2ProcessorTest.java new file mode 100644 index 00000000000..c268c413fc7 --- /dev/null +++ b/core/test/com/cloud/storage/template/QCOW2ProcessorTest.java @@ -0,0 +1,109 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package com.cloud.storage.template; + +import com.cloud.exception.InternalErrorException; +import com.cloud.storage.Storage; +import com.cloud.storage.StorageLayer; +import org.junit.Assert; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.Mock; +import org.mockito.Mockito; +import org.mockito.runners.MockitoJUnitRunner; + +import java.io.File; +import java.io.IOException; +import java.util.HashMap; +import java.util.Map; + +@RunWith(MockitoJUnitRunner.class) +public class QCOW2ProcessorTest { + QCOW2Processor processor; + + @Mock + StorageLayer mockStorageLayer; + + @Before + public void setUp() throws Exception { + processor = Mockito.spy(new QCOW2Processor()); + Map params = new HashMap(); + params.put(StorageLayer.InstanceConfigKey, mockStorageLayer); + processor.configure("VHD Processor", params); + } + + @Test(expected = InternalErrorException.class) + public void testProcessWhenVirtualSizeThrowsException() throws Exception { + String templatePath = "/tmp"; + String templateName = "template"; + + Mockito.when(mockStorageLayer.exists(Mockito.anyString())).thenReturn(true); + File mockFile = Mockito.mock(File.class); + + Mockito.when(mockStorageLayer.getFile(Mockito.anyString())).thenReturn(mockFile); + Mockito.when(mockStorageLayer.getSize(Mockito.anyString())).thenReturn(1000L); + Mockito.doThrow(new IOException("virtual size calculation failed")).when(processor).getTemplateVirtualSize((File)Mockito.any()); + + processor.process(templatePath, null, templateName); + } + + @Test + public void testProcess() throws Exception { + String templatePath = "/tmp"; + String templateName = "template"; + long virtualSize = 2000; + long actualSize = 1000; + + Mockito.when(mockStorageLayer.exists(Mockito.anyString())).thenReturn(true); + File mockFile = Mockito.mock(File.class); + + Mockito.when(mockStorageLayer.getFile(Mockito.anyString())).thenReturn(mockFile); + Mockito.when(mockStorageLayer.getSize(Mockito.anyString())).thenReturn(actualSize); + Mockito.doReturn(virtualSize).when(processor).getTemplateVirtualSize((File)Mockito.any()); + + Processor.FormatInfo info = processor.process(templatePath, null, templateName); + Assert.assertEquals(Storage.ImageFormat.QCOW2, info.format); + Assert.assertEquals(actualSize, info.size); + Assert.assertEquals(virtualSize, info.virtualSize); + Assert.assertEquals(templateName + ".qcow2", info.filename); + } + + @Test + public void testGetVirtualSizeWhenVirtualSizeThrowsException() throws Exception { + long virtualSize = 2000; + long actualSize = 1000; + File mockFile = Mockito.mock(File.class); + Mockito.when(mockFile.length()).thenReturn(actualSize); + Mockito.doThrow(new IOException("virtual size calculation failed")).when(processor).getTemplateVirtualSize((File)Mockito.any()); + Assert.assertEquals(actualSize, processor.getVirtualSize(mockFile)); + Mockito.verify(mockFile, Mockito.times(1)).length(); + } + + @Test + public void testGetVirtualSize() throws Exception { + long virtualSize = 2000; + long actualSize = 1000; + File mockFile = Mockito.mock(File.class); + Mockito.when(mockFile.length()).thenReturn(actualSize); + Mockito.doReturn(virtualSize).when(processor).getTemplateVirtualSize((File)Mockito.any()); + Assert.assertEquals(virtualSize, processor.getVirtualSize(mockFile)); + Mockito.verify(mockFile, Mockito.times(0)).length(); + } +} \ No newline at end of file diff --git a/core/test/com/cloud/storage/template/VhdProcessorTest.java b/core/test/com/cloud/storage/template/VhdProcessorTest.java new file mode 100644 index 00000000000..b8b362a017b --- /dev/null +++ b/core/test/com/cloud/storage/template/VhdProcessorTest.java @@ -0,0 +1,110 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package com.cloud.storage.template; + +import com.cloud.exception.InternalErrorException; +import com.cloud.storage.Storage; +import com.cloud.storage.StorageLayer; +import org.junit.Assert; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.Mock; +import org.mockito.Mockito; +import org.mockito.runners.MockitoJUnitRunner; + +import java.io.File; +import java.io.IOException; +import java.util.HashMap; +import java.util.Map; + +@RunWith(MockitoJUnitRunner.class) +public class VhdProcessorTest { + VhdProcessor processor; + + @Mock + StorageLayer mockStorageLayer; + + @Before + public void setUp() throws Exception { + processor = Mockito.spy(new VhdProcessor()); + Map params = new HashMap(); + params.put(StorageLayer.InstanceConfigKey, mockStorageLayer); + processor.configure("VHD Processor", params); + } + + @Test(expected = InternalErrorException.class) + public void testProcessWhenVirtualSizeThrowsException() throws Exception { + String templatePath = "/tmp"; + String templateName = "template"; + + Mockito.when(mockStorageLayer.exists(Mockito.anyString())).thenReturn(true); + File mockFile = Mockito.mock(File.class); + + Mockito.when(mockStorageLayer.getFile(Mockito.anyString())).thenReturn(mockFile); + Mockito.when(mockStorageLayer.getSize(Mockito.anyString())).thenReturn(1000L); + Mockito.doThrow(new IOException("virtual size calculation failed")).when(processor).getTemplateVirtualSize((File) Mockito.any()); + + processor.process(templatePath, null, templateName); + } + + @Test + public void testProcess() throws Exception { + String templatePath = "/tmp"; + String templateName = "template"; + long virtualSize = 2000; + long actualSize = 1000; + + Mockito.when(mockStorageLayer.exists(Mockito.anyString())).thenReturn(true); + File mockFile = Mockito.mock(File.class); + + Mockito.when(mockStorageLayer.getFile(Mockito.anyString())).thenReturn(mockFile); + Mockito.when(mockStorageLayer.getSize(Mockito.anyString())).thenReturn(actualSize); + Mockito.doReturn(virtualSize).when(processor).getTemplateVirtualSize((File) Mockito.any()); + + Processor.FormatInfo info = processor.process(templatePath, null, templateName); + Assert.assertEquals(Storage.ImageFormat.VHD, info.format); + Assert.assertEquals(actualSize, info.size); + Assert.assertEquals(virtualSize, info.virtualSize); + Assert.assertEquals(templateName + ".vhd", info.filename); + } + + @Test + public void testGetVirtualSizeWhenVirtualSizeThrowsException() throws Exception { + long virtualSize = 2000; + long actualSize = 1000; + File mockFile = Mockito.mock(File.class); + Mockito.when(mockFile.length()).thenReturn(actualSize); + Mockito.doThrow(new IOException("virtual size calculation failed")).when(processor).getTemplateVirtualSize((File) Mockito.any()); + Assert.assertEquals(actualSize, processor.getVirtualSize(mockFile)); + Mockito.verify(mockFile,Mockito.times(1)).length(); + } + + @Test + public void testGetVirtualSize() throws Exception{ + long virtualSize = 2000; + long actualSize = 1000; + File mockFile = Mockito.mock(File.class); + Mockito.when(mockFile.length()).thenReturn(actualSize); + Mockito.doReturn(virtualSize).when(processor).getTemplateVirtualSize((File) Mockito.any()); + Assert.assertEquals(virtualSize, processor.getVirtualSize(mockFile)); + Mockito.verify(mockFile,Mockito.times(0)).length(); + } +} \ No newline at end of file diff --git a/services/secondary-storage/server/src/org/apache/cloudstack/storage/template/DownloadManagerImpl.java b/services/secondary-storage/server/src/org/apache/cloudstack/storage/template/DownloadManagerImpl.java index 3bc0de7e9df..431a20425a2 100644 --- a/services/secondary-storage/server/src/org/apache/cloudstack/storage/template/DownloadManagerImpl.java +++ b/services/secondary-storage/server/src/org/apache/cloudstack/storage/template/DownloadManagerImpl.java @@ -482,7 +482,10 @@ public class DownloadManagerImpl extends ManagerBase implements DownloadManager return e.toString(); } if (info != null) { - loc.addFormat(info); + if(!loc.addFormat(info)) { + loc.purge(); + return "Unable to install due to invalid file format"; + } dnld.setTemplatesize(info.virtualSize); dnld.setTemplatePhysicalSize(info.size); break;