Merge pull request #237 from stgraber/import

Import LXD changes

.github/workflows/tests.yml

@@ -171,6 +171,7 @@ jobs:
 
           mkdir -p ~/sdk/gotip
           tar -C ~/sdk/gotip -xzf gotip.tar.gz
+          rm gotip.tar.gz
           ~/sdk/gotip/bin/go version
           echo "PATH=$HOME/go/bin:$HOME/sdk/gotip/bin/:$PATH" >> $GITHUB_ENV
         if: matrix.go == 'tip'

Makefile

@@ -93,9 +93,10 @@ ifneq "$(INCUS_OFFLINE)" ""
 	exit 1
 endif
 	$(GO) get -t -v -d -u ./...
-	go get github.com/mdlayher/socket@v0.4.1
-	go get github.com/openfga/go-sdk@v0.2.2
+	$(GO) get github.com/mdlayher/socket@v0.4.1
+	$(GO) get github.com/openfga/go-sdk@v0.2.2
 	$(GO) mod tidy --go=1.20
+	$(GO) get toolchain@none
 
 	cd cmd/lxd-to-incus && $(GO) get -t -v -d -u ./...
 	cd cmd/lxd-to-incus && $(GO) mod tidy --go=1.20
@@ -126,7 +127,7 @@ endif
 .PHONY: update-metadata
 update-metadata: build
 	@echo "Generating golang documentation metadata"
-	cd internal/server/config/generate && CGO_ENABLED=0 go build -o $(GOPATH)/bin/incus-doc
+	cd internal/server/config/generate && CGO_ENABLED=0 $(GO) build -o $(GOPATH)/bin/incus-doc
 	$(GOPATH)/bin/incus-doc . --json ./internal/server/metadata/configuration.json --txt ./doc/config_options.txt
 
 .PHONY: doc-setup
@@ -301,7 +302,7 @@ endif
 .PHONY: staticcheck
 staticcheck:
 ifeq ($(shell command -v staticcheck),)
-	(cd / ; go install -v -x honnef.co/go/tools/cmd/staticcheck@latest)
+	(cd / ; $(GO) install -v -x honnef.co/go/tools/cmd/staticcheck@latest)
 endif
 	# To get advance notice of deprecated function usage, consider running:
 	#   sed -i 's/^go 1\.[0-9]\+$/go 1.18/' go.mod

client/incus_instances.go

@@ -1197,6 +1197,10 @@ func (r *ProtocolIncus) ExecInstance(instanceName string, exec api.InstanceExecP
 				return nil, err
 			}
 
+			go func() {
+				_, _, _ = conn.ReadMessage() // Consume pings from server.
+			}()
+
 			go args.Control(conn)
 		}
 
@@ -1236,10 +1240,16 @@ func (r *ProtocolIncus) ExecInstance(instanceName string, exec api.InstanceExecP
 					return nil, err
 				}
 
+				go func() {
+					_, _, _ = conn.ReadMessage() // Consume pings from server.
+				}()
+
 				conns = append(conns, conn)
 				dones[0] = ws.MirrorRead(conn, args.Stdin)
 			}
 
+			waitConns := 0 // Used for keeping track of when stdout and stderr have finished.
+
 			// Handle stdout
 			if fds["1"] != "" {
 				conn, err := r.GetOperationWebsocket(opAPI.ID, fds["1"])
@@ -1249,6 +1259,7 @@ func (r *ProtocolIncus) ExecInstance(instanceName string, exec api.InstanceExecP
 
 				conns = append(conns, conn)
 				dones[1] = ws.MirrorWrite(conn, args.Stdout)
+				waitConns++
 			}
 
 			// Handle stderr
@@ -1260,33 +1271,36 @@ func (r *ProtocolIncus) ExecInstance(instanceName string, exec api.InstanceExecP
 
 				conns = append(conns, conn)
 				dones[2] = ws.MirrorWrite(conn, args.Stderr)
+				waitConns++
 			}
 
 			// Wait for everything to be done
 			go func() {
-				for i, chDone := range dones {
-					// Skip stdin, dealing with it separately below
-					if i == 0 {
-						continue
+				for {
+					select {
+					case <-dones[0]:
+						// Handle stdin finish, but don't wait for it if output channels
+						// have all finished.
+						dones[0] = nil
+						_ = conns[0].Close()
+					case <-dones[1]:
+						dones[1] = nil
+						_ = conns[1].Close()
+						waitConns--
+					case <-dones[2]:
+						dones[2] = nil
+						_ = conns[2].Close()
+						waitConns--
 					}
 
-					<-chDone
-				}
+					if waitConns <= 0 {
+						// Close stdin websocket if defined and not already closed.
+						if dones[0] != nil {
+							conns[0].Close()
+						}
 
-				if fds["0"] != "" {
-					if args.Stdin != nil {
-						_ = args.Stdin.Close()
+						break
 					}
-
-					// Empty the stdin channel but don't block on it as
-					// stdin may be stuck in Read()
-					go func() {
-						<-dones[0]
-					}()
-				}
-
-				for _, conn := range conns {
-					_ = conn.Close()
 				}
 
 				if args.DataDone != nil {

client/interfaces.go

@@ -581,13 +581,13 @@ type InstanceConsoleLogArgs struct {
 // The InstanceExecArgs struct is used to pass additional options during instance exec.
 type InstanceExecArgs struct {
 	// Standard input
-	Stdin io.ReadCloser
+	Stdin io.Reader
 
 	// Standard output
-	Stdout io.WriteCloser
+	Stdout io.Writer
 
 	// Standard error
-	Stderr io.WriteCloser
+	Stderr io.Writer
 
 	// Control message handler (window resize, signals, ...)
 	Control func(conn *websocket.Conn)

cmd/incus/exec.go

@@ -166,10 +166,10 @@ func (c *cmdExec) Run(cmd *cobra.Command, args []string) error {
 		}
 	}
 
-	var stdin io.ReadCloser
+	var stdin io.Reader
 	stdin = os.Stdin
 	if c.flagDisableStdin {
-		stdin = io.NopCloser(bytes.NewReader(nil))
+		stdin = bytes.NewReader(nil)
 	}
 
 	stdout := getStdout()

cmd/incusd/instance_exec.go

@@ -103,23 +103,23 @@ func (s *execWs) Connect(op *operations.Operation, r *http.Request, w http.Respo
 					if err != nil {
 						logger.Warn("Failed setting TCP timeouts on remote connection", logger.Ctx{"err": err})
 					}
+				}
 
-					// Start channel keep alive to run until channel is closed.
-					go func() {
-						pingInterval := time.Second * 10
-						t := time.NewTicker(pingInterval)
-						defer t.Stop()
-
-						for {
-							err := conn.WriteControl(websocket.PingMessage, []byte("keepalive"), time.Now().Add(5*time.Second))
-							if err != nil {
-								return
-							}
+				// Start channel keep alive to run until channel is closed.
+				go func() {
+					pingInterval := time.Second * 10
+					t := time.NewTicker(pingInterval)
+					defer t.Stop()
 
-							<-t.C
+					for {
+						err := conn.WriteControl(websocket.PingMessage, []byte("keepalive"), time.Now().Add(5*time.Second))
+						if err != nil {
+							return
 						}
-					}()
-				}
+
+						<-t.C
+					}
+				}()
 
 				if fd == execWSControl {
 					s.waitControlConnected.Cancel() // Control connection connected.

doc/config_options.txt

@@ -837,7 +837,7 @@ The original VLAN used when moving a VF into an instance.
 
 ```{config:option} volatile.apply_nvram instance-volatile
 :shortdesc: "Whether to regenerate VM NVRAM the next time the instance starts"
-:type: "string"
+:type: "bool"
 
 ```
 
@@ -1284,7 +1284,7 @@ Specify the number of days after which the unused cached image expires.
 ```{config:option} acme.ca_url server-acme
 :defaultdesc: "`https://acme-v02.api.letsencrypt.org/directory`"
 :scope: "global"
-:shortdesc: "Agree to ACME terms of service"
+:shortdesc: "URL to the directory resource of the ACME service"
 :type: "string"
 
 ```

internal/instance/config.go

@@ -981,7 +981,7 @@ var InstanceConfigKeysVM = map[string]func(value string) error{
 	// gendoc:generate(entity=instance, group=volatile, key=volatile.apply_nvram)
 	//
 	// ---
-	//  type: string
+	//  type: bool
 	//  shortdesc: Whether to regenerate VM NVRAM the next time the instance starts
 	"volatile.apply_nvram": validate.Optional(validate.IsBool),
 

internal/server/cluster/config/config.go

@@ -285,7 +285,7 @@ var ConfigSchema = config.Schema{
 	//  type: string
 	//  scope: global
 	//  defaultdesc: `https://acme-v02.api.letsencrypt.org/directory`
-	//  shortdesc: Agree to ACME terms of service
+	//  shortdesc: URL to the directory resource of the ACME service
 	"acme.ca_url": {},
 
 	// gendoc:generate(entity=server, group=acme, key=acme.domain)

internal/server/instance/drivers/driver_qemu.go

@@ -1933,15 +1933,9 @@ func (d *qemu) architectureSupportsUEFI(arch int) bool {
 }
 
 func (d *qemu) setupNvram() error {
-	d.logger.Debug("Generating NVRAM")
-
-	// Mount the instance's config volume.
-	_, err := d.mount()
-	if err != nil {
-		return err
-	}
+	var err error
 
-	defer func() { _ = d.unmount() }()
+	d.logger.Debug("Generating NVRAM")
 
 	// Cleanup existing variables.
 	for _, firmwares := range [][]ovmfFirmware{ovmfGenericFirmwares, ovmfSecurebootFirmwares, ovmfCSMFirmwares} {
@@ -5343,6 +5337,19 @@ func (d *qemu) Update(args db.InstanceArgs, userRequested bool) error {
 	}
 
 	if d.architectureSupportsUEFI(d.architecture) && (util.ValueInSlice("security.secureboot", changedConfig) || util.ValueInSlice("security.csm", changedConfig)) {
+		// setupNvram() requires instance's config volume to be mounted.
+		// The easiest way to detect that is to check if instance is running.
+		// TODO: extend storage API to be able to check if volume is already mounted?
+		if !isRunning {
+			// Mount the instance's config volume.
+			_, err := d.mount()
+			if err != nil {
+				return err
+			}
+
+			defer func() { _ = d.unmount() }()
+		}
+
 		// Re-generate the NVRAM.
 		err = d.setupNvram()
 		if err != nil {

internal/server/metadata/configuration.json

@@ -903,7 +903,7 @@
 						"volatile.apply_nvram": {
 							"longdesc": "",
 							"shortdesc": "Whether to regenerate VM NVRAM the next time the instance starts",
-							"type": "string"
+							"type": "bool"
 						}
 					},
 					{
@@ -1398,7 +1398,7 @@
 							"defaultdesc": "`https://acme-v02.api.letsencrypt.org/directory`",
 							"longdesc": "",
 							"scope": "global",
-							"shortdesc": "Agree to ACME terms of service",
+							"shortdesc": "URL to the directory resource of the ACME service",
 							"type": "string"
 						}
 					},

internal/server/storage/drivers/driver_btrfs.go

@@ -198,13 +198,13 @@ func (d *btrfs) Create() error {
 		hostPath := d.config["source"]
 		if d.isSubvolume(hostPath) {
 			// Existing btrfs subvolume.
-			subvols, err := d.getSubvolumes(hostPath)
+			hasSubvolumes, err := d.hasSubvolumes(hostPath)
 			if err != nil {
 				return fmt.Errorf("Could not determine if existing btrfs subvolume is empty: %w", err)
 			}
 
 			// Check that the provided subvolume is empty.
-			if len(subvols) > 0 {
+			if hasSubvolumes {
 				return fmt.Errorf("Requested btrfs subvolume exists but is not empty")
 			}
 		} else {

internal/server/storage/drivers/driver_btrfs_utils.go

@@ -95,6 +95,17 @@ func (d *btrfs) isSubvolume(path string) bool {
 	return true
 }
 
+func (d *btrfs) hasSubvolumes(path string) (bool, error) {
+	var stdout strings.Builder
+
+	err := subprocess.RunCommandWithFds(d.state.ShutdownCtx, nil, &stdout, "btrfs", "subvolume", "list", "-o", path)
+	if err != nil {
+		return false, err
+	}
+
+	return stdout.Len() > 0, nil
+}
+
 func (d *btrfs) getSubvolumes(path string) ([]string, error) {
 	poolMountPath := GetPoolMountPath(d.name)
 	if !strings.HasPrefix(path, poolMountPath+"/") {

shared/ws/mirror.go

@@ -17,8 +17,8 @@ func Mirror(conn *websocket.Conn, rwc io.ReadWriteCloser) (chan error, chan erro
 	return chRead, chWrite
 }
 
-// MirrorRead is a uni-directional mirror which replicates an io.ReadCloser to a websocket.
-func MirrorRead(conn *websocket.Conn, rc io.ReadCloser) chan error {
+// MirrorRead is a uni-directional mirror which replicates an io.Reader to a websocket.
+func MirrorRead(conn *websocket.Conn, rc io.Reader) chan error {
 	chDone := make(chan error, 1)
 	if rc == nil {
 		close(chDone)
@@ -30,8 +30,6 @@ func MirrorRead(conn *websocket.Conn, rc io.ReadCloser) chan error {
 	connRWC := NewWrapper(conn)
 
 	go func() {
-		defer close(chDone)
-
 		_, err := io.Copy(connRWC, rc)
 
 		logger.Debug("Websocket: Stopped read mirror", logger.Ctx{"address": conn.RemoteAddr().String(), "err": err})
@@ -40,13 +38,14 @@ func MirrorRead(conn *websocket.Conn, rc io.ReadCloser) chan error {
 		connRWC.Close()
 
 		chDone <- err
+		close(chDone)
 	}()
 
 	return chDone
 }
 
-// MirrorWrite is a uni-directional mirror which replicates a websocket to an io.WriteCloser.
-func MirrorWrite(conn *websocket.Conn, wc io.WriteCloser) chan error {
+// MirrorWrite is a uni-directional mirror which replicates a websocket to an io.Writer.
+func MirrorWrite(conn *websocket.Conn, wc io.Writer) chan error {
 	chDone := make(chan error, 1)
 	if wc == nil {
 		close(chDone)
@@ -58,11 +57,11 @@ func MirrorWrite(conn *websocket.Conn, wc io.WriteCloser) chan error {
 	connRWC := NewWrapper(conn)
 
 	go func() {
-		defer close(chDone)
 		_, err := io.Copy(wc, connRWC)
 
 		logger.Debug("Websocket: Stopped write mirror", logger.Ctx{"address": conn.RemoteAddr().String(), "err": err})
 		chDone <- err
+		close(chDone)
 	}()
 
 	return chDone