[Merge] ~jchittum/livecd-rootfs:ensure-vmtools-in-vmdk-header into livecd-rootfs:ubuntu/master
Robert C Jennings
robert.jennings at canonical.com
Mon Oct 19 16:02:41 UTC 2020
Review: Needs Fixing
One fix needed on the size test (it needs to be before the truncate call)
Diff comments:
> diff --git a/live-build/functions b/live-build/functions
> index e4b9042..830a054 100644
> --- a/live-build/functions
> +++ b/live-build/functions
> @@ -248,15 +254,37 @@ modify_vmdk_header() {
> # remove the comments from vmdk-stream-converter which causes
> # VirtualBox and others to fail VMDK validation
>
> - sed -e 's|# Description file.*|# Disk DescriptorFile|' \
> - -e '/# Believe this is random*/d' \
> - -e '/# Indicates no parent/d' \
> - -e '/# The Disk Data Base/d' \
> - -e 's|ddb.comment.*|ddb.toolsVersion = "2147483647"|' \
> - "${descriptor}" > "${newdescriptor}"
> + sed -i -e 's|# Description file.*|# Disk DescriptorFile|' \
> + -e '/# Believe this is random*/d' \
> + -e '/# Indicates no parent/d' \
> + -e '/# The Disk Data Base/d' \
> + -e '/ddb.comment*/d' \
> + $newdescriptor
> +
> + # grep for removal of the ddb.comment line to ensure removal
> + if grep -q "ddb.comment" $newdescriptor; then
> + echo "ddb.comment exists and will cause Virtualbox failures"; exit 1
> + fi
> +
> + # add newline to newdescriptor
> + echo "" >> $newdescriptor
> +
> + # add tools version
> + echo -n 'ddb.toolsVersion = "2147483647"' >> $newdescriptor
> +
> + # check ddb.toolsVersion in descriptor, otherwise image will fail
> + grep -q 'ddb.toolsVersion' $newdescriptor || {
> + echo 'failed to write version. Descriptor invalid'; exit 1;
> + }
> +
> + # diff original descriptor and new descriptor for debugging
> + diff --text $descriptor $newdescriptor | cat
> +
You need to check that $newdesciptor is less than 1024 bytes before the truncate call. Truncate will make it so and we don't want to lose data. We're then using truncate to pad the file with null bytes to ensure we have a 1024 byte descriptor that will completely overwrite the prior data.
> + # reset newdescriptor to be 1024
> + truncate --no-create --size=1K $newdescriptor
>
> - # The header is cannot be bigger than 1024
> - expr $(stat --format=%s ${newdescriptor}) \< 1024 > /dev/null 2>&1 || {
> + # The header must be 1024 or less
> + expr $(stat --format=%s ${newdescriptor}) \< 1025 > /dev/null 2>&1 || {
> echo "descriptor is too large, VMDK will be invalid!"; exit 1; }
>
> # Overwrite the vmdk header with our new, modified one
--
https://code.launchpad.net/~jchittum/livecd-rootfs/+git/livecd-rootfs/+merge/392401
Your team Ubuntu Core Development Team is requested to review the proposed merge of ~jchittum/livecd-rootfs:ensure-vmtools-in-vmdk-header into livecd-rootfs:ubuntu/master.
More information about the Ubuntu-reviews
mailing list