Restore guest agent unix socket functionality

[afbjorklund]

Make both vsock and virtio explicit, instead of hardcoding internal filenames in the agents. Fallback to to the unix.

• Support for native driver specific guest agent connection #1998

If neither vsock nor virtio is being used, then the driver silently falls back to forwarding the unix socket over ssh.

This means that new drivers don't have to do anything, unless overriding.

[balajiv113]

@afbjorklund Thanks for addressing this usecase

How about we expose a util to forward unix sock ?? Drivers lile vbox can use this and follow the same mechanism of dial unix sock

In guest side, we will have last default as unix sock without any additional options.

This way, the guest communication will fully be taken care by drivers only and host will not hardcode any functionality for guest connection

[balajiv113]

Yes i agree on bringing the forwarded sock. But decision should be done from each driver and not on host agent.

The reason is, we are using ForwardUnixGuestAgent just for comparability reasons. Which doesn't give much value for a driver interface and also because of this hostagent has to take care of forwarding unix socket

The advantage of driver deciding is the separation of guest agent connection from host agent and keeping the driver interface clean of other.

I agree some changes needed on vbox for this but it will be very minimal. We could do that just to keep the driver interface clear

[afbjorklund]

I agree, keeping it in the interface was more of a workaround (due to vsock/virtio ports and conn being exposed already)

could just copy the d.VSockPort == 0 && d.VirtioPort == "" to the approximately two places actually needing it

We could add more callbacks (in the driver) to set up and cancel the sockets, and do the implementation in the basedriver.

just didn't like have any exception or boiler plate code in the "empty" driver, it would be nice if it could inherit

[afbjorklund]

Kept the implementation in the hostagent (since it was there before, and had all the functions needed)

We could move it (2-3 functions) to the BaseDriver with some effort, but I am not sure if it is worth spending

localUnix := filepath.Join(a.instDir, filenames.GuestAgentSock)
remoteUnix := "/run/lima-guestagent.sock"
forwardSSH(context.Background(), a.sshConfig, a.sshLocalPort, localUnix, remoteUnix, verbCancel, false)
forwardSSH(ctx, a.sshConfig, a.sshLocalPort, localUnix, remoteUnix, verbForward, false)

d.DialContext(ctx, "unix", filepath.Join(a.instDir, filenames.GuestAgentSock))

There is some similar empty hook in the Register/Unregister callbacks, but that doesn't have shared code

Thanks for keeping the interface clean, it did feel a bit rushed (but interface doesn't expose any variables)

[afbjorklund]

Added some documentation about the previous/fallback functionality, best I could come up with.

[balajiv113]

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

[afbjorklund]

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

[balajiv113]

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

[afbjorklund]

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

[balajiv113]

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

[afbjorklund]

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...

[balajiv113]

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

[afbjorklund]

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

[afbjorklund]

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

[AkihiroSuda]

Needs rebase

[afbjorklund]

Needs rewrite, without the changes to the interface (as suggested)

[afbjorklund]

Rebase done.

• still passes the virtio socket as an explicit argument (like vsock), instead of fixed path

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

• still uses the driver interface to propagate the vsock and virtio fields from the basedriver

func (d *BaseDriver) ForwardGuestAgent() bool {
       // if driver is not providing, use host agent
       return d.VSockPort == 0 && d.VirtioPort == ""
}

So now the old code works again, at the cost of making the interface a bit more muddy...

[AkihiroSuda]

CI is failing

[AkihiroSuda]

Thanks