Skip to content

Commit aecea71

Browse files
authored
feat(minimega): add -abssnapshot flag (#1587)
A previous PR updated the disk snapshot and rebase commands to use relative file paths for parent backing images. This works in some environments, but ends up breaking other (existing) environments. The new `-abssnapshot` flag, which defaults to false for backwards compatibility, reverts back to using absolute paths for backing images. This has to be an install-wide flag due to internal functions (like KVM launch) calling `diskSnapshot` without user input. See comments in PR #1545 for additional details.
1 parent eff5277 commit aecea71

File tree

4 files changed

+101
-74
lines changed

4 files changed

+101
-74
lines changed

cmd/minimega/disk.go

Lines changed: 28 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,6 @@ func diskInfo(image string) (DiskInfo, error) {
3838

3939
jsonOut := map[string]interface{}{}
4040
err = json.Unmarshal([]byte(out), &jsonOut)
41-
4241
if err != nil {
4342
return info, fmt.Errorf("[image %s] %v", image, err)
4443
}
@@ -169,13 +168,18 @@ func diskSnapshot(src, dst string) error {
169168
return fmt.Errorf("[image %s] error getting info: %v", src, err)
170169
}
171170

172-
relSrc, err := filepath.Rel(filepath.Dir(dst), src)
173-
if err != nil {
174-
return fmt.Errorf("[image %s] error getting src relative to dst: %v", src, err)
175-
}
171+
backing := src
176172

177-
out, err := processWrapper("qemu-img", "create", "-f", "qcow2", "-b", relSrc, "-F", info.Format, dst)
173+
if !*f_absSnapshot {
174+
var err error
175+
176+
backing, err = filepath.Rel(filepath.Dir(dst), src)
177+
if err != nil {
178+
return fmt.Errorf("[image %s] error getting src backing image relative to dst: %v", src, err)
179+
}
180+
}
178181

182+
out, err := processWrapper("qemu-img", "create", "-f", "qcow2", "-b", backing, "-F", info.Format, dst)
179183
if err != nil {
180184
return fmt.Errorf("[image %s] %v: %v", src, out, err)
181185
}
@@ -194,26 +198,39 @@ func diskCommit(image string) error {
194198

195199
func diskRebase(image, backing string, unsafe bool) error {
196200
args := []string{"qemu-img", "rebase"}
201+
197202
if backing != "" {
198203
if !strings.HasPrefix(backing, *f_iomBase) {
199204
log.Warn("minimega expects backing images to be in the files directory")
200205
}
201-
relBacking, err := filepath.Rel(filepath.Dir(image), backing)
202-
if err != nil {
203-
return fmt.Errorf("[image %s] error getting backing relative to dst: %v", backing, err)
206+
207+
parent := backing
208+
209+
if !*f_absSnapshot {
210+
var err error
211+
212+
parent, err = filepath.Rel(filepath.Dir(image), backing)
213+
if err != nil {
214+
return fmt.Errorf("[image %s] error getting parent backing relative to dst: %v", backing, err)
215+
}
204216
}
217+
205218
backingInfo, err := diskInfo(backing)
206219
if err != nil {
207220
return fmt.Errorf("[image %s] error getting info for backing file: %v", image, err)
208221
}
209-
args = append(args, "-b", relBacking, "-F", backingInfo.Format)
222+
223+
args = append(args, "-b", parent, "-F", backingInfo.Format)
210224
} else { // rebase as independent image
211225
args = append(args, "-b", "")
212226
}
227+
213228
if unsafe {
214229
args = append(args, "-u")
215230
}
231+
216232
args = append(args, image)
233+
217234
out, err := processWrapper(args...)
218235
if err != nil {
219236
return fmt.Errorf("[image %s] %v: %v", image, out, err)
@@ -373,7 +390,7 @@ func diskInject(dst, partition string, pairs map[string]string, options []string
373390
// copy files/folders into mntDir
374391
for target, source := range pairs {
375392
dir := filepath.Dir(filepath.Join(mntDir, target))
376-
os.MkdirAll(dir, 0775)
393+
os.MkdirAll(dir, 0o775)
377394

378395
out, err := processWrapper("cp", "-fr", source, filepath.Join(mntDir, target))
379396
if err != nil {

cmd/minimega/disk_cli.go

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ package main
66

77
import (
88
"errors"
9+
"fmt"
910
"os"
1011
"path"
1112
"path/filepath"
@@ -16,6 +17,8 @@ import (
1617
log "github.com/sandia-minimega/minimega/v2/pkg/minilog"
1718
)
1819

20+
var backingType = "relative"
21+
1922
var diskCLIHandlers = []minicli.Handler{
2023
{
2124
HelpShort: "creates a new disk",
@@ -83,7 +86,7 @@ The 'recursive' flag can be set to print out full details for all backing images
8386
},
8487
{
8588
HelpShort: "creates a new disk 'dst' backed by 'image'",
86-
HelpLong: `
89+
HelpLong: fmt.Sprintf(`
8790
Creates a new qcow2 image 'dst' backed by 'image'.
8891
8992
Example of taking a snapshot of a disk:
@@ -95,13 +98,13 @@ snapshot will be stored in the 'files' directory. Snapshots are always created
9598
in the 'files' directory.
9699
97100
Users may use paths relative to the 'files' directory or absolute paths for inputs;
98-
however, the backing path will always be relative to the new image.`,
101+
however, the backing path will always be %s to the new image.`, backingType),
99102
Patterns: []string{"disk snapshot <image> [dst image]"},
100103
Call: wrapSimpleCLI(cliDiskSnapshot),
101104
},
102105
{
103106
HelpShort: "rebases the disk onto a different backing image",
104-
HelpLong: `
107+
HelpLong: fmt.Sprintf(`
105108
Rebases the image 'image' onto a new backing file 'backing'.
106109
Using 'rebase' will write any differences between the original backing file and the new backing file to 'image'.
107110
@@ -116,7 +119,7 @@ The 'backing' argument can be omitted, causing all backing data to be written to
116119
disk rebase myimage.qcow2
117120
118121
Users may use paths relative to the 'files' directory or absolute paths for inputs;
119-
however, the backing path will always be relative to the rebased image.`,
122+
however, the backing path will always be %s to the rebased image.`, backingType),
120123
Patterns: []string{
121124
"disk <rebase,> <image> [backing file]",
122125
"disk <set-backing,> <image> [backing file]",

cmd/minimega/main.go

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,7 @@ var (
5555
f_pipe = flag.String("pipe", "", "read/write to or from a named pipe")
5656
f_headnode = flag.String("headnode", "", "mesh node to send all logs to and get all files from")
5757
f_hashfiles = flag.Bool("hashfiles", false, "hash files to be served by iomeshage")
58+
f_absSnapshot = flag.Bool("abssnapshot", false, "use absolute paths to backing images when creating snapshots")
5859

5960
f_e = flag.Bool("e", false, "execute command on running minimega")
6061
f_attach = flag.Bool("attach", false, "attach the minimega command line to a running instance of minimega")
@@ -100,6 +101,10 @@ func main() {
100101
containerShim()
101102
}
102103

104+
if *f_absSnapshot {
105+
backingType = "absolute" // used for command help messages
106+
}
107+
103108
cliSetup()
104109

105110
if *f_cli {
@@ -218,7 +223,7 @@ func main() {
218223
}
219224

220225
// attempt to set up the base path
221-
if err := os.MkdirAll(*f_base, os.FileMode(0770)); err != nil {
226+
if err := os.MkdirAll(*f_base, os.FileMode(0o770)); err != nil {
222227
log.Fatal("mkdir base path: %v", err)
223228
}
224229

docker/start-minimega.sh

Lines changed: 60 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -8,20 +8,20 @@ set -o pipefail
88
# 2. Variables in /etc/default/minimega
99
# 3. A set of defaults in this script
1010
if [[ -f "/etc/default/minimega" ]]; then
11-
# Check if any variables are already set
12-
while IFS='=' read -r key value; do
13-
# Skip empty lines and comments
14-
if [[ -n "$key" && -n "$value" && "$key" != \#* ]]; then
15-
# Remove surrounding quotes
16-
value="${value%\"}"
17-
value="${value#\"}"
18-
19-
# Only set the variable if it is not already set
20-
if [[ -z "${!key}" ]]; then
21-
export "${key}=${value}"
22-
fi
23-
fi
24-
done < <(grep -v '^#' "/etc/default/minimega")
11+
# Check if any variables are already set
12+
while IFS='=' read -r key value; do
13+
# Skip empty lines and comments
14+
if [[ -n "$key" && -n "$value" && "$key" != \#* ]]; then
15+
# Remove surrounding quotes
16+
value="${value%\"}"
17+
value="${value#\"}"
18+
19+
# Only set the variable if it is not already set
20+
if [[ -z "${!key}" ]]; then
21+
export "${key}=${value}"
22+
fi
23+
fi
24+
done < <(grep -v '^#' "/etc/default/minimega")
2525
fi
2626

2727
# Final default assignment (if these are not set already)
@@ -41,6 +41,7 @@ fi
4141
: "${MM_FORCE:=true}"
4242
: "${MM_RECOVER:=false}"
4343
: "${MM_CGROUP:=/sys/fs/cgroup}"
44+
: "${MM_ABSSNAPSHOT:=false}"
4445
: "${MM_APPEND:=}"
4546

4647
: "${OVS_APPEND:=}"
@@ -49,8 +50,8 @@ fi
4950
# Start Open vSwitch
5051
/usr/share/openvswitch/scripts/ovs-ctl start ${OVS_APPEND} |& tee -a ${MM_LOGFILE}
5152
if [ ${PIPESTATUS[0]} -ne 0 ]; then
52-
echo "failed to start Open vSwitch" | tee -a ${MM_LOGFILE}
53-
exit 1
53+
echo "failed to start Open vSwitch" | tee -a ${MM_LOGFILE}
54+
exit 1
5455
fi
5556

5657
# Ensure Open vSwitch is available
@@ -60,40 +61,40 @@ START=$(date +%s)
6061

6162
echo "waiting for Open vSwitch to become available (timeout: ${TIMEOUT}s)..." | tee -a ${MM_LOGFILE}
6263
while true; do
63-
current=$(date +%s)
64-
elapsed=$((current - START))
64+
current=$(date +%s)
65+
elapsed=$((current - START))
6566

66-
if [[ "$elapsed" -ge "$TIMEOUT" ]]; then
67-
echo "failed to connect to Open vSwitch" | tee -a ${MM_LOGFILE}
68-
exit 1
69-
fi
67+
if [[ "$elapsed" -ge "$TIMEOUT" ]]; then
68+
echo "failed to connect to Open vSwitch" | tee -a ${MM_LOGFILE}
69+
exit 1
70+
fi
7071

71-
if ovs-vsctl show &>/dev/null; then
72-
break
73-
else
74-
sleep $INTERVAL
75-
fi
72+
if ovs-vsctl show &>/dev/null; then
73+
break
74+
else
75+
sleep $INTERVAL
76+
fi
7677
done
7778

7879
# Check if there are bridge:port values to add
7980
if [[ -v "OVS_HOST_IFACE" ]]; then
80-
iface=(${OVS_HOST_IFACE//:/ })
81-
bridge=${iface[0]}
82-
83-
if [[ -n "${bridge}" ]]; then
84-
echo -e "\tadding '${bridge}' bridge..." | tee -a ${MM_LOGFILE}
85-
/usr/bin/ovs-vsctl --may-exist add-br ${bridge}
86-
ip link set dev ${bridge} up
87-
fi
88-
89-
if [[ -n "${iface[1]}" ]]; then
90-
ports=(${iface[1]//,/ })
91-
92-
for port in "${ports[@]}"; do
93-
echo -e "\tadding '${port}' port to '${bridge}' bridge..." | tee -a ${MM_LOGFILE}
94-
/usr/bin/ovs-vsctl --may-exist add-port ${bridge} ${port}
95-
done
96-
fi
81+
iface=(${OVS_HOST_IFACE//:/ })
82+
bridge=${iface[0]}
83+
84+
if [[ -n "${bridge}" ]]; then
85+
echo -e "\tadding '${bridge}' bridge..." | tee -a ${MM_LOGFILE}
86+
/usr/bin/ovs-vsctl --may-exist add-br ${bridge}
87+
ip link set dev ${bridge} up
88+
fi
89+
90+
if [[ -n "${iface[1]}" ]]; then
91+
ports=(${iface[1]//,/ })
92+
93+
for port in "${ports[@]}"; do
94+
echo -e "\tadding '${port}' port to '${bridge}' bridge..." | tee -a ${MM_LOGFILE}
95+
/usr/bin/ovs-vsctl --may-exist add-port ${bridge} ${port}
96+
done
97+
fi
9798
fi
9899

99100
echo "starting miniweb..." | tee -a ${MM_LOGFILE}
@@ -102,17 +103,18 @@ echo "miniweb started on ${MINIWEB_HOST}:${MINIWEB_PORT}" | tee -a ${MM_LOGFILE}
102103

103104
echo "starting minimega..." | tee -a ${MM_LOGFILE}
104105
/opt/minimega/bin/minimega \
105-
-nostdin \
106-
-force=${MM_FORCE} \
107-
-recover=${MM_RECOVER} \
108-
-base=${MM_BASE} \
109-
-filepath=${MM_FILEPATH} \
110-
-broadcast=${MM_BROADCAST} \
111-
-vlanrange=${MM_VLANRANGE} \
112-
-port=${MM_PORT} \
113-
-degree=${MM_DEGREE} \
114-
-context=${MM_CONTEXT} \
115-
-level=${MM_LOGLEVEL} \
116-
-logfile=${MM_LOGFILE} \
117-
-cgroup=${MM_CGROUP} \
118-
${MM_APPEND}
106+
-nostdin \
107+
-force=${MM_FORCE} \
108+
-recover=${MM_RECOVER} \
109+
-base=${MM_BASE} \
110+
-filepath=${MM_FILEPATH} \
111+
-broadcast=${MM_BROADCAST} \
112+
-vlanrange=${MM_VLANRANGE} \
113+
-port=${MM_PORT} \
114+
-degree=${MM_DEGREE} \
115+
-context=${MM_CONTEXT} \
116+
-level=${MM_LOGLEVEL} \
117+
-logfile=${MM_LOGFILE} \
118+
-cgroup=${MM_CGROUP} \
119+
-abssnapshot=${MM_ABSSNAPSHOT} \
120+
${MM_APPEND}

0 commit comments

Comments
 (0)