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.

Sign up for GitHub

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

Restore guest agent unix socket functionality #2006

Merged
merged 1 commit into from
Dec 25, 2023
Merged

Restore guest agent unix socket functionality #2006

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 22 additions & 8 deletions cmd/lima-guestagent/daemon_linux.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ import (
"github.com/lima-vm/lima/pkg/guestagent"
"github.com/lima-vm/lima/pkg/guestagent/api/server"
"github.com/lima-vm/lima/pkg/guestagent/serialport"
"github.com/lima-vm/lima/pkg/store/filenames"
"github.com/mdlayher/vsock"
"github.com/sirupsen/logrus"
"github.com/spf13/cobra"
Expand All @@ -25,21 +24,23 @@ func newDaemonCommand() *cobra.Command {
}
daemonCommand.Flags().Duration("tick", 3*time.Second, "tick for polling events")
daemonCommand.Flags().Int("vsock-port", 0, "use vsock server instead a UNIX socket")
daemonCommand.Flags().String("virtio-port", "", "use virtio server instead a UNIX socket")
return daemonCommand
}

func daemonAction(cmd *cobra.Command, _ []string) error {
socket := "/run/lima-guestagent.sock"
tick, err := cmd.Flags().GetDuration("tick")
if err != nil {
return err
}
vSockPortOverride, err := cmd.Flags().GetInt("vsock-port")
vSockPort, err := cmd.Flags().GetInt("vsock-port")
if err != nil {
return err
}
vSockPort := 0
if vSockPortOverride != 0 {
vSockPort = vSockPortOverride
virtioPort, err := cmd.Flags().GetString("virtio-port")
if err != nil {
return err
}
if tick == 0 {
return errors.New("tick must be specified")
Expand Down Expand Up @@ -67,11 +68,14 @@ func daemonAction(cmd *cobra.Command, _ []string) error {
r := mux.NewRouter()
server.AddRoutes(r, backend)
srv := &http.Server{Handler: r}
err = os.RemoveAll(socket)
if err != nil {
return err
}

var l net.Listener
virtioPort := "/dev/virtio-ports/" + filenames.VirtioPort
if _, err := os.Stat(virtioPort); err == nil {
qemuL, err := serialport.Listen(virtioPort)
if virtioPort != "" {
Copy link
Member

Choose a reason for hiding this comment

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

Instead of using virtioPort as a arg, can we simply do os.Stat and fallback to unix sock ?

Copy link
Member Author

Choose a reason for hiding this comment

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

Having it as an explicit argument was a feature, but it should probably be named serialSock

Copy link
Member

Choose a reason for hiding this comment

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

The communication medium is internal to us. I don't think there is a need to make it a argument. We can hardcode it like we do for unix sock just to simplify stuff

Copy link
Member Author

Choose a reason for hiding this comment

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

If the filename is indeed fixed we can hardcode it, but then it might not even need the constant

qemuL, err := serialport.Listen("/dev/virtio-ports/" + virtioPort)
if err != nil {
return err
}
Expand All @@ -84,6 +88,16 @@ func daemonAction(cmd *cobra.Command, _ []string) error {
}
l = vsockL
logrus.Infof("serving the guest agent on vsock port: %d", vSockPort)
} else {
socketL, err := net.Listen("unix", socket)
if err != nil {
return err
}
if err := os.Chmod(socket, 0o777); err != nil {
return err
}
l = socketL
logrus.Infof("serving the guest agent on %q", socket)
}
return srv.Serve(l)
}
12 changes: 10 additions & 2 deletions cmd/lima-guestagent/install_systemd_linux.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ func newInstallSystemdCommand() *cobra.Command {
RunE: installSystemdAction,
}
installSystemdCommand.Flags().Int("vsock-port", 0, "use vsock server on specified port")
installSystemdCommand.Flags().String("virtio-port", "", "use virtio server instead a UNIX socket")
return installSystemdCommand
}

Expand All @@ -29,7 +30,11 @@ func installSystemdAction(cmd *cobra.Command, _ []string) error {
if err != nil {
return err
}
unit, err := generateSystemdUnit(vsockPort)
virtioPort, err := cmd.Flags().GetString("virtio-port")
if err != nil {
return err
}
unit, err := generateSystemdUnit(vsockPort, virtioPort)
if err != nil {
return err
}
Expand Down Expand Up @@ -67,7 +72,7 @@ func installSystemdAction(cmd *cobra.Command, _ []string) error {
//go:embed lima-guestagent.TEMPLATE.service
var systemdUnitTemplate string

func generateSystemdUnit(vsockPort int) ([]byte, error) {
func generateSystemdUnit(vsockPort int, virtioPort string) ([]byte, error) {
selfExeAbs, err := os.Executable()
if err != nil {
return nil, err
Expand All @@ -77,6 +82,9 @@ func generateSystemdUnit(vsockPort int) ([]byte, error) {
if vsockPort != 0 {
args = append(args, fmt.Sprintf("--vsock-port %d", vsockPort))
}
if virtioPort != "" {
args = append(args, fmt.Sprintf("--virtio-port %s", virtioPort))
}

m := map[string]string{
"Binary": selfExeAbs,
Expand Down
10 changes: 8 additions & 2 deletions pkg/cidata/cidata.TEMPLATE.d/boot/25-guestagent-base.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ name="lima-guestagent"
description="Forward ports to the lima-hostagent"

command=${LIMA_CIDATA_GUEST_INSTALL_PREFIX}/bin/lima-guestagent
command_args="daemon --vsock-port "${LIMA_CIDATA_VSOCK_PORT}""
command_args="daemon --vsock-port "${LIMA_CIDATA_VSOCK_PORT}" --virtio-port "${LIMA_CIDATA_VIRTIO_PORT}""
command_background=true
pidfile="/run/lima-guestagent.pid"
EOF
Expand All @@ -47,5 +47,11 @@ else
# Remove legacy systemd service
rm -f "${LIMA_CIDATA_HOME}/.config/systemd/user/lima-guestagent.service"

sudo "${LIMA_CIDATA_GUEST_INSTALL_PREFIX}"/bin/lima-guestagent install-systemd --vsock-port "${LIMA_CIDATA_VSOCK_PORT}"
if [ "${LIMA_CIDATA_VSOCK_PORT}" != "0" ]; then
sudo "${LIMA_CIDATA_GUEST_INSTALL_PREFIX}"/bin/lima-guestagent install-systemd --vsock-port "${LIMA_CIDATA_VSOCK_PORT}"
elif [ "${LIMA_CIDATA_VIRTIO_PORT}" != "" ]; then
sudo "${LIMA_CIDATA_GUEST_INSTALL_PREFIX}"/bin/lima-guestagent install-systemd --virtio-port "${LIMA_CIDATA_VIRTIO_PORT}"
else
sudo "${LIMA_CIDATA_GUEST_INSTALL_PREFIX}"/bin/lima-guestagent install-systemd
fi
fi
1 change: 1 addition & 0 deletions pkg/cidata/cidata.TEMPLATE.d/lima.env
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ LIMA_CIDATA_SKIP_DEFAULT_DEPENDENCY_RESOLUTION=
{{- end}}
LIMA_CIDATA_VMTYPE={{ .VMType }}
LIMA_CIDATA_VSOCK_PORT={{ .VSockPort }}
LIMA_CIDATA_VIRTIO_PORT={{ .VirtioPort}}
{{- if .Plain}}
LIMA_CIDATA_PLAIN=1
{{- else}}
Expand Down
3 changes: 2 additions & 1 deletion pkg/cidata/cidata.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ func setupEnv(y *limayaml.LimaYAML, args TemplateArgs) (map[string]string, error
return env, nil
}

func GenerateISO9660(instDir, name string, y *limayaml.LimaYAML, udpDNSLocalPort, tcpDNSLocalPort int, nerdctlArchive string, vsockPort int) error {
func GenerateISO9660(instDir, name string, y *limayaml.LimaYAML, udpDNSLocalPort, tcpDNSLocalPort int, nerdctlArchive string, vsockPort int, virtioPort string) error {
if err := limayaml.Validate(*y, false); err != nil {
return err
}
Expand All @@ -135,6 +135,7 @@ func GenerateISO9660(instDir, name string, y *limayaml.LimaYAML, udpDNSLocalPort
RosettaBinFmt: *y.Rosetta.BinFmt,
VMType: *y.VMType,
VSockPort: vsockPort,
VirtioPort: virtioPort,
Plain: *y.Plain,
}

Expand Down
1 change: 1 addition & 0 deletions pkg/cidata/template.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@ type TemplateArgs struct {
SkipDefaultDependencyResolution bool
VMType string
VSockPort int
VirtioPort string
Plain bool
}

Expand Down
13 changes: 12 additions & 1 deletion pkg/driver/driver.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,10 @@ type Driver interface {

ListSnapshots(_ context.Context) (string, error)

// ForwardGuestAgent returns if the guest agent sock needs forwarding by host agent.
ForwardGuestAgent() bool

// GuestAgentConn returns the guest agent connection, or nil (if forwarded by ssh).
GuestAgentConn(_ context.Context) (net.Conn, error)
}

Expand All @@ -73,6 +77,7 @@ type BaseDriver struct {

SSHLocalPort int
VSockPort int
VirtioPort string
}

var _ Driver = (*BaseDriver)(nil)
Expand Down Expand Up @@ -137,6 +142,12 @@ func (d *BaseDriver) ListSnapshots(_ context.Context) (string, error) {
return "", fmt.Errorf("unimplemented")
}

func (d *BaseDriver) ForwardGuestAgent() bool {
Copy link
Member

Choose a reason for hiding this comment

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

My concern is still here.

My point is we don't need this. Instead of treating unix sock as fallback am saying lets treat it as a way drivers can use. But drivers need to decide on the communication medium.

For eg: vz will always work on vsock, virtbox will always work on unix sock (This individual drivers should decide). Hostagent should not have that code

Copy link
Member Author

Choose a reason for hiding this comment

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

I wanted to keep the hostagent tunneling as a fallback. It was handy when experimenting with external VMs.

For the VBox driver, it can probably use a serial console. Will do a rebase of it someday, it is not important...

Copy link
Member

Choose a reason for hiding this comment

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

When we say fallback i wanted this to be a universal one not just for un configured driver.

Something like this 4640684. This is just a idea how we can do, based on the error we can fallback to this universal unix sock

Copy link
Member Author

Choose a reason for hiding this comment

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

As long as it is done in the hostagent, and not copy/paste (or explicit) in the driver, that would work as well

Copy link
Member Author

Choose a reason for hiding this comment

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

Not sure where to cancel the forwarding process, but maybe it would just die when the instance does?

// if driver is not providing, use host agent
return d.VSockPort == 0 && d.VirtioPort == ""
}

func (d *BaseDriver) GuestAgentConn(_ context.Context) (net.Conn, error) {
return nil, fmt.Errorf("unimplemented")
// use the unix socket forwarded by host agent
return nil, nil
}
35 changes: 32 additions & 3 deletions pkg/hostagent/hostagent.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,8 @@ type HostAgent struct {
eventEnc *json.Encoder
eventEncMu sync.Mutex

vSockPort int
vSockPort int
virtioPort string

clientMu sync.RWMutex
client guestagentclient.GuestAgentClient
Expand Down Expand Up @@ -117,6 +118,7 @@ func New(instName string, stdout io.Writer, sigintCh chan os.Signal, opts ...Opt
}

vSockPort := 0
virtioPort := ""
if *y.VMType == limayaml.VZ {
vSockPort = 2222
} else if *y.VMType == limayaml.WSL2 {
Expand All @@ -125,9 +127,11 @@ func New(instName string, stdout io.Writer, sigintCh chan os.Signal, opts ...Opt
logrus.WithError(err).Error("failed to get free VSock port")
}
vSockPort = port
} else if *y.VMType == limayaml.QEMU {
virtioPort = filenames.VirtioPort
}

if err := cidata.GenerateISO9660(inst.Dir, instName, y, udpDNSLocalPort, tcpDNSLocalPort, o.nerdctlArchive, vSockPort); err != nil {
if err := cidata.GenerateISO9660(inst.Dir, instName, y, udpDNSLocalPort, tcpDNSLocalPort, o.nerdctlArchive, vSockPort, virtioPort); err != nil {
return nil, err
}

Expand Down Expand Up @@ -160,6 +164,7 @@ func New(instName string, stdout io.Writer, sigintCh chan os.Signal, opts ...Opt
Yaml: y,
SSHLocalPort: sshLocalPort,
VSockPort: vSockPort,
VirtioPort: virtioPort,
})

a := &HostAgent{
Expand All @@ -176,6 +181,7 @@ func New(instName string, stdout io.Writer, sigintCh chan os.Signal, opts ...Opt
sigintCh: sigintCh,
eventEnc: json.NewEncoder(stdout),
vSockPort: vSockPort,
virtioPort: virtioPort,
guestAgentAliveCh: make(chan struct{}),
}
return a, nil
Expand Down Expand Up @@ -561,6 +567,9 @@ func (a *HostAgent) watchGuestAgentEvents(ctx context.Context) {
}
}

localUnix := filepath.Join(a.instDir, filenames.GuestAgentSock)
remoteUnix := "/run/lima-guestagent.sock"

a.onClose = append(a.onClose, func() error {
logrus.Debugf("Stop forwarding unix sockets")
var errs []error
Expand All @@ -573,9 +582,19 @@ func (a *HostAgent) watchGuestAgentEvents(ctx context.Context) {
}
}
}
if a.driver.ForwardGuestAgent() {
if err := forwardSSH(context.Background(), a.sshConfig, a.sshLocalPort, localUnix, remoteUnix, verbCancel, false); err != nil {
errs = append(errs, err)
}
}
return errors.Join(errs...)
})
for {
if a.client == nil || !isGuestAgentSocketAccessible(ctx, a.client) {
if a.driver.ForwardGuestAgent() {
_ = forwardSSH(ctx, a.sshConfig, a.sshLocalPort, localUnix, remoteUnix, verbForward, false)
}
}
client, err := a.getOrCreateClient(ctx)
if err == nil {
if err := a.processGuestAgentEvents(ctx, client); err != nil {
Expand Down Expand Up @@ -610,8 +629,18 @@ func (a *HostAgent) getOrCreateClient(ctx context.Context) (guestagentclient.Gue
return a.client, err
}

func (a *HostAgent) createClient(ctx context.Context) (guestagentclient.GuestAgentClient, error) {
func (a *HostAgent) createConnection(ctx context.Context) (net.Conn, error) {
conn, err := a.driver.GuestAgentConn(ctx)
// default to forwarded sock
if conn == nil && err == nil {
var d net.Dialer
conn, err = d.DialContext(ctx, "unix", filepath.Join(a.instDir, filenames.GuestAgentSock))
}
return conn, err
}

func (a *HostAgent) createClient(ctx context.Context) (guestagentclient.GuestAgentClient, error) {
conn, err := a.createConnection(ctx)
if err != nil {
return nil, err
}
Expand Down
4 changes: 3 additions & 1 deletion website/content/en/docs/dev/Internals/_index.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,10 +72,12 @@ VNC:

Guest agent:

Each drivers use their own mode of communication
Each drivers use their own mode of communication
- `qemu`: uses virtio-port `io.lima-vm.guest_agent.0`
- `vz`: uses vsock port 2222
- `wsl2`: uses free random vsock port
The fallback is to use port forward over ssh port
- `ga.sock`: Forwarded to `/run/lima-guestagent.sock` in the guest, via SSH

Host agent:
- `ha.pid`: hostagent PID
Expand Down
Loading