From 24ee4b8c4d3fc83fd1798c01c62a882dd975eff1 Mon Sep 17 00:00:00 2001 From: John Chittum Date: Fri, 16 Oct 2020 13:43:26 -0500 Subject: [PATCH 1/4] vmtools version in vmdk header (LP: #1893898) LP: #1893898 describes missing vmtools version from the vmdk headers. The version should be added as ddb.toolsVersion = "2147483647" however the sed was no longer replacing a ddb.comment field with the tools version. Rather than subbing ddb.comment with toolsVersion, this commit deletes ddb.comment (which the comment mentions could cause errors), and adds the correct value. There was no visibility into the descriptor during hook creation, so debug statements were added. This allows us to quickly verify in the logs that bad statements are removed (the possibly offending commetns), as well as ensuring that the toolsVersion is added --- debian/changelog | 4 ++++ live-build/functions | 44 ++++++++++++++++++++++++++++++++++++-------- 2 files changed, 40 insertions(+), 8 deletions(-) diff --git a/debian/changelog b/debian/changelog index 93e2b1a2..bc319f9a 100644 --- a/debian/changelog +++ b/debian/changelog @@ -6,6 +6,10 @@ livecd-rootfs (2.693) groovy; urgency=medium livecd-rootfs (2.692) groovy; urgency=medium + [ John Chittum ] + * Ensure vmtools version entered into vmdk header (LP: #1893898) + + [ Łukasz 'sil2100' Zemczak ] * Create a 1GB swapfile for the raspi desktop images. -- Łukasz 'sil2100' Zemczak Thu, 15 Oct 2020 11:23:47 +0200 diff --git a/live-build/functions b/live-build/functions index e4b90425..830a0549 100644 --- a/live-build/functions +++ b/live-build/functions @@ -237,6 +237,12 @@ modify_vmdk_header() { # Extract the vmdk header for manipulation dd if="${vmdk_name}" of="${descriptor}" bs=1 skip=512 count=1024 + # cat header so we are aware of the original descriptor for debugging + cat $descriptor + + # trim null bytes to treat as standard text file + tr -d '\000' < $descriptor > $newdescriptor + # The sed lines below is where the magic is. Specifically: # ddb.toolsVersion: sets the open-vm-tools so that VMware shows # the tooling as current @@ -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 + + # 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 From 4f5eacbfae3aa850a8bfc519ac70990395fdd5b8 Mon Sep 17 00:00:00 2001 From: John Chittum Date: Mon, 19 Oct 2020 10:54:59 -0500 Subject: [PATCH 2/4] Fixup Debian Changlog Rebase, fixedup changelog, opened new release --- debian/changelog | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/debian/changelog b/debian/changelog index bc319f9a..8fda6888 100644 --- a/debian/changelog +++ b/debian/changelog @@ -1,3 +1,9 @@ +livecd-rootfs (2.694) UNRELEASED; urgency=medium + + * Ensure vmtools version entered into vmdk header (LP: #1893898) + + -- John Chittum Mon, 19 Oct 2020 10:52:54 -0500 + livecd-rootfs (2.693) groovy; urgency=medium * Build classic raspi server images by default from the 'classic' branch now. @@ -6,10 +12,6 @@ livecd-rootfs (2.693) groovy; urgency=medium livecd-rootfs (2.692) groovy; urgency=medium - [ John Chittum ] - * Ensure vmtools version entered into vmdk header (LP: #1893898) - - [ Łukasz 'sil2100' Zemczak ] * Create a 1GB swapfile for the raspi desktop images. -- Łukasz 'sil2100' Zemczak Thu, 15 Oct 2020 11:23:47 +0200 From 201addb317cdf15e1d0757b809ea2cd614da11c8 Mon Sep 17 00:00:00 2001 From: John Chittum Date: Mon, 19 Oct 2020 13:22:32 -0500 Subject: [PATCH 3/4] Remove sed and move size check There was a question on if the comment removals in the `sed` were required. The comments (`#`) are created by vmdk-stream-converter and seem to cause no issues. `ddb.comment` is no longer being written by the tool anymore. Moved the check earlier to ensure the new header isn't too large before running truncate (otherwise it may be too long, and we remove bits we want) --- live-build/functions | 45 ++++++++++++++------------------------------ 1 file changed, 14 insertions(+), 31 deletions(-) diff --git a/live-build/functions b/live-build/functions index 830a0549..f53b59e3 100644 --- a/live-build/functions +++ b/live-build/functions @@ -243,50 +243,33 @@ modify_vmdk_header() { # trim null bytes to treat as standard text file tr -d '\000' < $descriptor > $newdescriptor - # The sed lines below is where the magic is. Specifically: - # ddb.toolsVersion: sets the open-vm-tools so that VMware shows - # the tooling as current - # ddb.virtualHWVersion: set the version to 7, which covers most - # current versions of VMware - # createType: make sure its set to stream Optimized - # remove the vmdk-stream-converter comment and replace with - # # Disk DescriptorFile. This is needed for Virtualbox - # remove the comments from vmdk-stream-converter which causes - # VirtualBox and others to fail VMDK validation - - 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 + # add required 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; - } + if ! grep -q 'ddb.toolsVersion' $newdescriptor; then + echo 'failed to write version. Descriptor invalid'; \ + exit 1 + fi # diff original descriptor and new descriptor for debugging + # diff exits 1 if difference. pipefail not set so piping diff + # to cat prints diff and swallows exit 1 diff --text $descriptor $newdescriptor | cat + + # The header must be 1024 or less before padding + if ! expr $(stat --format=%s ${newdescriptor}) \< 1025 > /dev/null 2>&1; then + echo "descriptor is too large, VMDK will be invalid!"; + exit 1 + fi + # reset newdescriptor to be 1024 truncate --no-create --size=1K $newdescriptor - # 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 dd conv=notrunc,nocreat \ if="${newdescriptor}" of="${vmdk_name}" \ From 4f1df739f68a47e4f215c8ca11d1d7a5a1046da9 Mon Sep 17 00:00:00 2001 From: John Chittum Date: Mon, 26 Oct 2020 09:22:32 -0500 Subject: [PATCH 4/4] Debug logging information Added context lines for debugging lines. --- live-build/functions | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/live-build/functions b/live-build/functions index f53b59e3..7197a152 100644 --- a/live-build/functions +++ b/live-build/functions @@ -236,7 +236,7 @@ modify_vmdk_header() { # Extract the vmdk header for manipulation dd if="${vmdk_name}" of="${descriptor}" bs=1 skip=512 count=1024 - + echo "Cat'ing original descriptor to console for debugging." # cat header so we are aware of the original descriptor for debugging cat $descriptor @@ -249,15 +249,10 @@ modify_vmdk_header() { # add required tools version echo -n 'ddb.toolsVersion = "2147483647"' >> $newdescriptor - # check ddb.toolsVersion in descriptor, otherwise image will fail - if ! grep -q 'ddb.toolsVersion' $newdescriptor; then - echo 'failed to write version. Descriptor invalid'; \ - exit 1 - fi - # diff original descriptor and new descriptor for debugging # diff exits 1 if difference. pipefail not set so piping diff # to cat prints diff and swallows exit 1 + echo "Printing diff of original and new descriptors." diff --text $descriptor $newdescriptor | cat