CLOUDSTACK-8808: Successfully registered VHD template is downloaded again due to missing virtualsize property in template.properties by karuturi · Pull Request #901 · apache/cloudstack

karuturi · 2015-09-30T10:26:42Z

We have multiple file processors to process different types of image
formats. The processor interface has two methods getVirtualSize() and
process().

getVirtualSize() as the name says, returns the virtual size of
the file and is used at get the size while copying files from NFS to s3
process() returns FormatInfo struct which has fileType, size,
virutalSize, filename. on successfully downloading a template, each
file is passed to all the processors.process() and whichever returns a
FormatInfo, that will be used to create template.properties file. If
process() throws an InternalErrorException, template installation fails.
But, if process() returns null, template registration is successful with
template.properties missing some attributes like virtualSize, file
format etc. which results in this bug on restart of ssvm/cloud
service/management server.

failing the template download if virutalsize or some other properties
cannot be determined.

The following changes are done:
getVirtualSize() to always return size(if it can calculate, get virtual
size else return file size). This would mean the following changes

QCOW2Processor.getVirtualSize() to return file size if virtual
size calculation fails
VHDProcessor.getVirtualSize() to return file size if virtual size
calculation fails

process() to throw InternalErrorException if virtual size calculation
fails or any other exceptions occur. This would mean the following
changes

OVAProcessor to throw InternalErrorException if untar fails
QCOW2Processor to throw InternalErrorException if virtual size
calculation fails
VHDProcessor to throw InternalErrorException if virtual size
calculation fails

Testing:
added unittests for the changes in the file processors.
manual test:
setup: host xenserver 6.5, management server centos 6.7
template: disk created using the process specified by andy at https://issues.apache.org/jira/browse/CLOUDSTACK-8808?focusedCommentId=14933368
tried to register the template and it failed with an error. Template never moved to Ready state.

again due to missing virtualsize property in template.properties We have multiple file processors to process different types of image formats. The processor interface has two methods getVirtualSize() and process(). 1. getVirtualSize() as the name says, returns the virtual size of the file and is used at get the size while copying files from NFS to s3 2. process() returns FormatInfo struct which has fileType, size, virutalSize, filename. on successfully downloading a template, each file is passed to all the processors.process() and whichever returns a FormatInfo, that will be used to create template.properties file. If process() throws an InternalErrorException, template installation fails. But, if process() returns null, template registration is successful with template.properties missing some attributes like virtualSize, file format etc. which results in this bug on restart of ssvm/cloud service/management server. failing the template download if virutalsize or some other properties cannot be determined. The following changes are done: getVirtualSize() to always return size(if it can calculate, get virtual size else return file size). This would mean the following changes 1. QCOW2Processor.getVirtualSize() to return file size if virtual size calculation fails 2. VHDProcessor.getVirtualSize() to return file size if virtual size calculation fails process() to throw InternalErrorException if virtual size calculation fails or any other exceptions occur. This would mean the following changes 1. OVAProcessor to throw InternalErrorException if untar fails 2. QCOW2Processor to throw InternalErrorException if virtual size calculation fails 3. VHDProcessor to throw InternalErrorException if virtual size calculation fails

borisroman · 2015-09-30T11:51:13Z

@karuturi Could you please use the QCOW2Utils.getVirtualSize() from the com.cloud.utils.storage package instead of using the function defined in the QCOW2Processor? :)

The utils one should be a recent implementation. I wonder why the author hasnt done cleanup and left two implementation of the same. I will take a look at the code later and see if it can be reused here.

remibergsma · 2015-09-30T15:05:05Z

Thanks @karuturi for picking this up. Will have a look soon!

remibergsma · 2015-10-01T21:02:24Z

@karuturi Thanks a lot, works great! LGTM

I registered a template with invalid headers, as per CLOUDSTACK-8808.

At first everything is normal:

It then starts installing (and unzipping):

And eventually disappears from the UI. The logs show Format is invalid as expected.

Full logs:

2015-10-01 20:55:00,624 WARN  [storage.template.TemplateLocation] (pool-1-thread-2:null) Format is invalid
2015-10-01 20:55:00,625 DEBUG [storage.template.TemplateLocation] (pool-1-thread-2:null) Format: VHD size: 21475270656 virtualsize: 0 filename: 4db55a50-1775-3f5d-b982-962eb2d0f282.vhd
2015-10-01 20:55:00,625 DEBUG [storage.template.TemplateLocation] (pool-1-thread-2:null) format, filename cannot be null and size, virtual size should be  > 0 
2015-10-01 20:55:02,241 DEBUG [storage.template.TemplateLocation] (pool-1-thread-2:null) Remove /mnt/SecStorage/6cc407de-bf89-3f72-8985-1b9df6a63827/template/tmpl/2/201/4db55a50-1775-3f5d-b982-962eb2d0f282.vhd
2015-10-01 20:55:02,302 DEBUG [storage.template.TemplateLocation] (pool-1-thread-2:null) Remove /mnt/SecStorage/6cc407de-bf89-3f72-8985-1b9df6a63827/template/tmpl/2/201/template.properties

2015-10-01 20:55:02,867 DEBUG [cloud.agent.Agent] (agentRequest-Handler-2:null) Processing command: org.apache.cloudstack.storage.command.DownloadProgressCommand
2015-10-01 20:55:02,870 DEBUG [cloud.agent.Agent] (agentRequest-Handler-2:null) Seq 3-5925611209712730174:  { Ans: , MgmtId: 90520732674659, via: 3, Ver: v1, Flags: 10, [{"com.cloud.agent.api.storage.DownloadAnswer":{"jobId":"d0965877-d8b8-4e12-a132-703fb7ee6b54","downloadPct":100,"errorString":"Failed post download script: Unable to install due to invalid file format","downloadStatus":"DOWNLOAD_ERROR","downloadPath":"/mnt/SecStorage/6cc407de-bf89-3f72-8985-1b9df6a63827/template/tmpl/2/201/dnld3817046421300354449tmp_","installPath":"template/tmpl/2/201/4db55a50-1775-3f5d-b982-962eb2d0f282.vhd","templateSize":0,"templatePhySicalSize":0,"checkSum":"49c737f858448e11eb181f250e4efaeb","result":true,"details":"Failed post download script: Unable to install due to invalid file format","wait":0}}] }

When someone else reviews this we can merge it and solve another blocker.

remibergsma · 2015-10-02T06:26:47Z

@borisroman: Are the answers to your questions sufficient? Can you review again please?

borisroman · 2015-10-02T12:21:05Z

@remibergsma Anwser is sufficient. I'll cleanup after 4.6 is released.

Regarding the PR: 👍 LGTM

I ran the test_vm_lifecycle which succeeded and also manually ran the tests described in CLOUDSTACK-8808, same results as you showed.

remibergsma · 2015-10-02T14:26:29Z

Thanks @borisroman, merged!

CLOUDSTACK-8808: Successfully registered VHD template is downloaded again due to missing virtualsize property in template.propertiesWe have multiple file processors to process different types of image formats. The processor interface has two methods getVirtualSize() and process(). 1. getVirtualSize() as the name says, returns the virtual size of the file and is used at get the size while copying files from NFS to s3 2. process() returns FormatInfo struct which has fileType, size, virutalSize, filename. on successfully downloading a template, each file is passed to all the processors.process() and whichever returns a FormatInfo, that will be used to create template.properties file. If process() throws an InternalErrorException, template installation fails. But, if process() returns null, template registration is successful with template.properties missing some attributes like virtualSize, file format etc. which results in this bug on restart of ssvm/cloud service/management server. failing the template download if virutalsize or some other properties cannot be determined. The following changes are done: getVirtualSize() to always return size(if it can calculate, get virtual size else return file size). This would mean the following changes 1. QCOW2Processor.getVirtualSize() to return file size if virtual size calculation fails 2. VHDProcessor.getVirtualSize() to return file size if virtual size calculation fails process() to throw InternalErrorException if virtual size calculation fails or any other exceptions occur. This would mean the following changes 1. OVAProcessor to throw InternalErrorException if untar fails 2. QCOW2Processor to throw InternalErrorException if virtual size calculation fails 3. VHDProcessor to throw InternalErrorException if virtual size calculation fails Testing: added unittests for the changes in the file processors. manual test: setup: host xenserver 6.5, management server centos 6.7 template: disk created using the process specified by andy at https://issues.apache.org/jira/browse/CLOUDSTACK-8808?focusedCommentId=14933368 tried to register the template and it failed with an error. Template never moved to Ready state. ![screen shot 2015-09-30 at 3 53 34 pm](https://cloud.githubusercontent.com/assets/186833/10190608/76bcce92-678b-11e5-8f52-b449d149300b.png) * pr/901: CLOUDSTACK-8808: Successfully registered VHD template is downloaded again due to missing virtualsize property in template.properties Signed-off-by: Remi Bergsma <github@remi.nl>

…901) Signed-off-by: Rohit Yadav <rohit.yadav@shapeblue.com>

borisroman reviewed Sep 30, 2015
View reviewed changes

asfgit merged commit 1056171 into apache:master Oct 2, 2015

karuturi deleted the CLOUDSTACK-8808 branch October 5, 2015 10:18

yadvr pushed a commit that referenced this pull request Jan 20, 2021

createnetwork: Show tabs only when supported networks are determined (#…

5ee41a5

…901) Signed-off-by: Rohit Yadav <rohit.yadav@shapeblue.com>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

CLOUDSTACK-8808: Successfully registered VHD template is downloaded again due to missing virtualsize property in template.properties#901

CLOUDSTACK-8808: Successfully registered VHD template is downloaded again due to missing virtualsize property in template.properties#901
asfgit merged 1 commit into
apache:masterfrom
karuturi:CLOUDSTACK-8808

karuturi commented Sep 30, 2015

Uh oh!

borisroman Sep 30, 2015

Uh oh!

karuturi Oct 1, 2015

Uh oh!

remibergsma commented Sep 30, 2015

Uh oh!

remibergsma commented Oct 1, 2015

Uh oh!

remibergsma commented Oct 2, 2015

Uh oh!

borisroman commented Oct 2, 2015

Uh oh!

remibergsma commented Oct 2, 2015

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

karuturi commented Sep 30, 2015

Uh oh!

borisroman Sep 30, 2015

Choose a reason for hiding this comment

Uh oh!

karuturi Oct 1, 2015

Choose a reason for hiding this comment

Uh oh!

remibergsma commented Sep 30, 2015

Uh oh!

remibergsma commented Oct 1, 2015

Uh oh!

remibergsma commented Oct 2, 2015

Uh oh!

borisroman commented Oct 2, 2015

Uh oh!

remibergsma commented Oct 2, 2015

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants