Skip to content

Commit 4a00db4

Browse files
committed
fix: match the HMP echo once by substring; rm takes the CNI dirs for older records
QEMU's readline echoes the typed command with redraw sequences, so a prefix test would have taken every successful echo for a rejection. The echo is the first line containing 'set_password ' and only that line. Records written before the CNI dirs were persisted resolve flag, then record, then default, so rm --cni-conf-dir/--cni-bin-dir can release their NICs.
1 parent c13e4dc commit 4a00db4

5 files changed

Lines changed: 26 additions & 4 deletions

File tree

cmd/vm/commands.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,8 @@ func Command(h *Handler) *cobra.Command {
7272
RunE: h.RM,
7373
}
7474
rmCmd.Flags().Bool("force", false, "force kill (immediate SIGKILL, skip the ACPI grace window)")
75+
rmCmd.Flags().String("cni-conf-dir", "", "CNI config dir for a VM created before the record remembered it")
76+
rmCmd.Flags().String("cni-bin-dir", "", "CNI plugin dir for a VM created before the record remembered it")
7577

7678
snapshotCmd := &cobra.Command{
7779
Use: "snapshot VM",

cmd/vm/net_linux.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -27,13 +27,13 @@ import (
2727
// netScope keys cocoon-macos's host TAP/netns families apart from a co-hosted cocoon's, so neither GC reclaims the other's live guests.
2828
const netScope = "cm"
2929

30-
// netConf is the cocoon network config: bridge/CNI provisioning shares cocoon's forwarding plane, keyed under our own device family; the CNI dirs come from the record so rm needs no flags.
30+
// netConf is the cocoon network config: bridge/CNI provisioning shares cocoon's forwarding plane, keyed under our own device family; the CNI dirs resolve flag, then record, then default.
3131
func netConf(cmd *cobra.Command, r *record) *config.Config {
3232
return &config.Config{
3333
RootDir: home.Dir(cmd),
3434
DNS: "8.8.8.8,1.1.1.1",
35-
CNIConfDir: cmp.Or(r.CNIConfDir, "/etc/cni/net.d"),
36-
CNIBinDir: cmp.Or(r.CNIBinDir, "/opt/cni/bin"),
35+
CNIConfDir: cmp.Or(flagOr(cmd, "cni-conf-dir", ""), r.CNIConfDir, "/etc/cni/net.d"),
36+
CNIBinDir: cmp.Or(flagOr(cmd, "cni-bin-dir", ""), r.CNIBinDir, "/opt/cni/bin"),
3737
NetScope: netScope,
3838
}
3939
}

cmd/vm/net_linux_test.go

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,24 @@ import (
1313
"github.com/cocoonstack/cocoon-macos/home"
1414
)
1515

16+
func TestNetConfResolvesFlagThenRecordThenDefault(t *testing.T) {
17+
cmd := &cobra.Command{}
18+
cmd.Flags().String("cni-conf-dir", "", "")
19+
cmd.Flags().String("cni-bin-dir", "", "")
20+
if got := netConf(cmd, &record{}).CNIConfDir; got != "/etc/cni/net.d" {
21+
t.Errorf("default CNIConfDir = %q", got)
22+
}
23+
if got := netConf(cmd, &record{CNIConfDir: "/rec/net.d", CNIBinDir: "/rec/bin"}).CNIBinDir; got != "/rec/bin" {
24+
t.Errorf("record CNIBinDir = %q", got)
25+
}
26+
if err := cmd.Flags().Set("cni-conf-dir", "/flag/net.d"); err != nil {
27+
t.Fatal(err)
28+
}
29+
if got := netConf(cmd, &record{CNIConfDir: "/rec/net.d"}).CNIConfDir; got != "/flag/net.d" {
30+
t.Errorf("flag CNIConfDir = %q, want the flag over the record", got)
31+
}
32+
}
33+
1634
func TestNetConfScope(t *testing.T) {
1735
conf := netConf(&cobra.Command{}, &record{})
1836
if got, want := conf.NetScope, "cm"; got != want {

cmd/vm/utils.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -371,7 +371,7 @@ func hmpReplied(out string) bool {
371371
if line == "" || strings.HasPrefix(line, strings.TrimSpace(hmpPrompt)) {
372372
continue
373373
}
374-
if !echoed && strings.HasPrefix(line, "set_password ") {
374+
if !echoed && strings.Contains(line, "set_password ") {
375375
echoed = true
376376
continue
377377
}

cmd/vm/utils_test.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -169,6 +169,8 @@ func TestHMPRepliedFlagsAnyMessage(t *testing.T) {
169169
{"invalid parameter", " set_password vnc ab cd\r\nError: invalid parameter value: cd\r\n(qemu) ", true},
170170
{"display inactive", " set_password vnc abcd\r\nCould not set password\r\n(qemu) ", true},
171171
{"unterminated quote", " set_password vnc \"abc\r\nset_password: string expected\r\nTry \"help set_password\" for more information\r\n(qemu) ", true},
172+
{"readline redraw echo", "s\x1b[K\x1b[Dse\x1b[K\x1b[D\x1b[Dset_password vnc abcd\r\n(qemu) ", false},
173+
{"readline redraw then rejection", "s\x1b[K\x1b[Dset_password vnc \"abc\r\nset_password: string expected\r\n(qemu) ", true},
172174
} {
173175
if got := hmpReplied(tc.out); got != tc.want {
174176
t.Errorf("%s: hmpReplied = %v, want %v", tc.name, got, tc.want)

0 commit comments

Comments
 (0)