Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

KVM #438

Draft
wants to merge 13 commits into
base: master
Choose a base branch
from
Draft

KVM #438

wants to merge 13 commits into from

Conversation

fepitre
Copy link
Member

@fepitre fepitre commented Nov 21, 2021

@kalkin
Copy link
Member

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 ))"


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
5 participants