Skip to content

Conversation

fepitre
Copy link
Member

@fepitre fepitre commented Nov 21, 2021

@kalkin
Copy link
Contributor

kalkin commented Nov 24, 2021

Sorry for showing up unexpectedly. I just noticed the few things. I hope it's not too much bikeshading. I do a lot of Makefile programming and have also integrated the qubes build system in to my monorepo, and I often stumble about suboptimal Makefiles.

local name="$1"
local hypervisor

if [[ $(cat /sys/hypervisor/type 2>/dev/null) == 'xen' ]]; then
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if [[ $(cat /sys/hypervisor/type 2>/dev/null) == 'xen' ]]; then
if hypervisor=$(cat /sys/hypervisor/type 2>/dev/null) && [[ "$hypervisor" = 'xen' ]]; then

echo "$hypervisor"
return 0
fi
if [ "$name" == "$hypervisor" ]; then
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if [ "$name" == "$hypervisor" ]; then
if [ "$name" = "$hypervisor" ]; then

@@ -1,14 +1,21 @@
#!/bin/sh
#!/usr/bin/sh
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
#!/usr/bin/sh
#!/usr/bin/bash --
set -euo pipefail

#### KVM:
if hypervisor xen; then
/usr/lib/qubes/fix-dir-perms.sh
DOM0_MAXMEM=$(/usr/sbin/xl list 0 | tail -1 | awk '{ print $3 }')
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This runs into an xl bug: xl doesn’t check for write errors on stdout. The following helps but does not fix the problem:

Suggested change
DOM0_MAXMEM=$(/usr/sbin/xl list 0 | tail -1 | awk '{ print $3 }')
DOM0_MAXMEM=$(/usr/sbin/xl list 0 | awk 'NR == 1 && NF > 3 && $3 ~ /^[0-9]+$/ { print $3 }')

if hypervisor xen; then
/usr/lib/qubes/fix-dir-perms.sh
DOM0_MAXMEM=$(/usr/sbin/xl list 0 | tail -1 | awk '{ print $3 }')
xenstore-write /local/domain/0/memory/static-max $[ $DOM0_MAXMEM * 1024 ]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
xenstore-write /local/domain/0/memory/static-max $[ $DOM0_MAXMEM * 1024 ]
xenstore-write /local/domain/0/memory/static-max "$(( $DOM0_MAXMEM * 1024 ))"

fi
########

cp /var/lib/qubes/qubes.xml /var/lib/qubes/backup/qubes-$(date +%F-%T).xml
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
cp /var/lib/qubes/qubes.xml /var/lib/qubes/backup/qubes-$(date +%F-%T).xml
s=$(date +%F-%T) && cp /var/lib/qubes/qubes.xml "/var/lib/qubes/backup/qubes-$s.xml"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants