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.

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

Remove virtfs-proxy-helper dependency #1547

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
121 changes: 0 additions & 121 deletions internal/server/device/device_utils_disk.go
Original file line number Diff line number Diff line change
Expand Up @@ -300,127 +300,6 @@ func diskAddRootUserNSEntry(idmaps []idmap.Entry, hostRootID int64) []idmap.Entr
return idmaps
}

// DiskVMVirtfsProxyStart starts a new virtfs-proxy-helper process.
// If the idmaps slice is supplied then the proxy process is run inside a user namespace using the supplied maps.
// Returns a file handle to the proxy process and a revert fail function that can be used to undo this function if
// a subsequent step fails,.
func DiskVMVirtfsProxyStart(execPath string, pidPath string, sharePath string, idmaps []idmap.Entry) (*os.File, revert.Hook, error) {
revert := revert.New()
defer revert.Fail()

// Locate virtfs-proxy-helper.
cmd, err := exec.LookPath("virtfs-proxy-helper")
if err != nil {
if util.PathExists("/usr/lib/qemu/virtfs-proxy-helper") {
cmd = "/usr/lib/qemu/virtfs-proxy-helper"
} else if util.PathExists("/usr/libexec/virtfs-proxy-helper") {
cmd = "/usr/libexec/virtfs-proxy-helper"
}
}

if cmd == "" {
return nil, nil, fmt.Errorf(`Required binary "virtfs-proxy-helper" couldn't be found`)
}

listener, err := net.Listen("unix", "")
if err != nil {
return nil, nil, fmt.Errorf("Failed to create unix listener for virtfs-proxy-helper: %w", err)
}

defer func() { _ = listener.Close() }()

cDial, err := net.Dial("unix", listener.Addr().String())
if err != nil {
return nil, nil, fmt.Errorf("Failed to connect to virtfs-proxy-helper unix listener: %w", err)
}

defer func() { _ = cDial.Close() }()

cDialUnix, ok := cDial.(*net.UnixConn)
if !ok {
return nil, nil, fmt.Errorf("Dialled virtfs-proxy-helper connection isn't unix socket")
}

defer func() { _ = cDialUnix.Close() }()

cDialUnixFile, err := cDialUnix.File()
if err != nil {
return nil, nil, fmt.Errorf("Failed getting virtfs-proxy-helper unix dialed file: %w", err)
}

revert.Add(func() { _ = cDialUnixFile.Close() })

cAccept, err := listener.Accept()
if err != nil {
return nil, nil, fmt.Errorf("Failed to accept connection to virtfs-proxy-helper unix listener: %w", err)
}

defer func() { _ = cAccept.Close() }()

cAcceptUnix, ok := cAccept.(*net.UnixConn)
if !ok {
return nil, nil, fmt.Errorf("Accepted virtfs-proxy-helper connection isn't unix socket")
}

defer func() { _ = cAcceptUnix.Close() }()

acceptFile, err := cAcceptUnix.File()
if err != nil {
return nil, nil, fmt.Errorf("Failed getting virtfs-proxy-helper unix listener file: %w", err)
}

defer func() { _ = acceptFile.Close() }()

// Start the virtfs-proxy-helper process in non-daemon mode and as root so that when the VM process is
// started as an unprivileged user, we can still share directories that process cannot access.
args := []string{"--nodaemon", "--fd", "3", "--path", sharePath}
proc, err := subprocess.NewProcess(cmd, args, "", "")
if err != nil {
return nil, nil, err
}

if len(idmaps) > 0 {
idmapSet := &idmap.Set{Entries: idmaps}
proc.SetUserns(idmapSet.ToUIDMappings(), idmapSet.ToGIDMappings())
}

err = proc.StartWithFiles(context.Background(), []*os.File{acceptFile})
if err != nil {
return nil, nil, fmt.Errorf("Failed to start virtfs-proxy-helper: %w", err)
}

revert.Add(func() { _ = proc.Stop() })

err = proc.Save(pidPath)
if err != nil {
return nil, nil, fmt.Errorf("Failed to save virtfs-proxy-helper state: %w", err)
}

cleanup := revert.Clone().Fail
revert.Success()
return cDialUnixFile, cleanup, err
}

// DiskVMVirtfsProxyStop stops the virtfs-proxy-helper process.
func DiskVMVirtfsProxyStop(pidPath string) error {
if util.PathExists(pidPath) {
proc, err := subprocess.ImportProcess(pidPath)
if err != nil {
return err
}

err = proc.Stop()
if err != nil && err != subprocess.ErrNotRunning {
return err
}

// Remove PID file.
_ = os.Remove(pidPath)
}

return nil
}

// DiskVMVirtiofsdStart starts a new virtiofsd process.
// If the idmaps slice is supplied then the proxy process is run inside a user namespace using the supplied maps.
// Returns UnsupportedError error if the host system or instance does not support virtiosfd, returns normal error
Expand Down
47 changes: 2 additions & 45 deletions internal/server/device/disk.go
Original file line number Diff line number Diff line change
Expand Up @@ -978,13 +978,6 @@ func (d *disk) startContainer() (*deviceConfig.RunConfig, error) {
return &runConf, nil
}

// vmVirtfsProxyHelperPaths returns the path for PID file to use with virtfs-proxy-helper process.
func (d *disk) vmVirtfsProxyHelperPaths() string {
pidPath := filepath.Join(d.inst.DevicesPath(), fmt.Sprintf("%s.pid", d.name))

return pidPath
}

// vmVirtiofsdPaths returns the path for the socket and PID file to use with virtiofsd process.
func (d *disk) vmVirtiofsdPaths() (string, string) {
sockPath := filepath.Join(d.inst.DevicesPath(), fmt.Sprintf("virtio-fs.%s.sock", d.name))
Expand Down Expand Up @@ -1288,7 +1281,7 @@ func (d *disk) startVM() (*deviceConfig.RunConfig, error) {
}

// Start virtiofsd for virtio-fs share. The agent prefers to use this over the
// virtfs-proxy-helper 9p share. The 9p share will only be used as a fallback.
// 9p share. The 9p share will only be used as a fallback.
err = func() error {
// Check if we should start virtiofsd.
if busOption != "auto" && busOption != "virtiofs" {
Expand Down Expand Up @@ -1346,36 +1339,6 @@ func (d *disk) startVM() (*deviceConfig.RunConfig, error) {
if err != nil {
return nil, fmt.Errorf("Failed to setup virtiofsd for device %q: %w", d.name, err)
}

// We can't hotplug 9p shares, so only do 9p for stopped instances.
if !d.inst.IsRunning() {
// Start virtfs-proxy-helper for 9p share (this will rewrite mount.DevPath with
// socket FD number so must come after starting virtiofsd).
err = func() error {
// Check if we should start 9p.
if busOption != "auto" && busOption != "9p" {
return nil
}

sockFile, cleanup, err := DiskVMVirtfsProxyStart(d.state.OS.ExecPath, d.vmVirtfsProxyHelperPaths(), mount.DevPath, rawIDMaps.Entries)
if err != nil {
return err
}

revert.Add(cleanup)

// Request the unix socket is closed after QEMU has connected on startup.
runConf.PostHooks = append(runConf.PostHooks, sockFile.Close)

// Use 9p socket FD number as dev path so qemu can connect to the proxy.
mount.DevPath = fmt.Sprintf("%d", sockFile.Fd())

return nil
}()
if err != nil {
return nil, fmt.Errorf("Failed to setup virtfs-proxy-helper for device %q: %w", d.name, err)
}
}
} else {
// Confirm we're dealing with block options.
err := validate.Optional(validate.IsOneOf("nvme", "virtio-blk", "virtio-scsi"))(d.config["io.bus"])
Expand Down Expand Up @@ -2178,14 +2141,8 @@ func (d *disk) Stop() (*deviceConfig.RunConfig, error) {
}

func (d *disk) stopVM() (*deviceConfig.RunConfig, error) {
// Stop the virtfs-proxy-helper process and clean up.
err := DiskVMVirtfsProxyStop(d.vmVirtfsProxyHelperPaths())
if err != nil {
return &deviceConfig.RunConfig{}, fmt.Errorf("Failed cleaning up virtfs-proxy-helper: %w", err)
}

// Stop the virtiofsd process and clean up.
err = DiskVMVirtiofsdStop(d.vmVirtiofsdPaths())
err := DiskVMVirtiofsdStop(d.vmVirtiofsdPaths())
if err != nil {
return &deviceConfig.RunConfig{}, fmt.Errorf("Failed cleaning up virtiofsd: %w", err)
}
Expand Down
9 changes: 1 addition & 8 deletions internal/server/instance/drivers/driver_qemu.go
Original file line number Diff line number Diff line change
Expand Up @@ -3965,13 +3965,6 @@ func (d *qemu) addDriveDirConfig(cfg *[]cfgSection, bus *qemuBus, fdFiles *[]*os
if !slices.Contains(driveConf.Opts, "bus=virtiofs") {
devBus, devAddr, multi := bus.allocate(busFunctionGroup9p)

fd, err := strconv.Atoi(driveConf.DevPath)
if err != nil {
return fmt.Errorf("Invalid file descriptor %q for drive %q: %w", driveConf.DevPath, driveConf.DevName, err)
}

proxyFD := d.addFileDescriptor(fdFiles, os.NewFile(uintptr(fd), driveConf.DevName))

driveDir9pOpts := qemuDriveDirOpts{
dev: qemuDevOpts{
busName: bus.name,
Expand All @@ -3981,8 +3974,8 @@ func (d *qemu) addDriveDirConfig(cfg *[]cfgSection, bus *qemuBus, fdFiles *[]*os
},
devName: driveConf.DevName,
mountTag: mountTag,
proxyFD: proxyFD, // Pass by file descriptor
readonly: readonly,
path: driveConf.DevPath,
protocol: "9p",
}
*cfg = append(*cfg, qemuDriveDir(&driveDir9pOpts)...)
Expand Down
14 changes: 8 additions & 6 deletions internal/server/instance/drivers/driver_qemu_config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -839,15 +839,16 @@ func TestQemuConfigTemplates(t *testing.T) {
dev: qemuDevOpts{"pci", "qemu_pcie0", "00.5", true},
devName: "stub",
mountTag: "mtag",
path: "/var/9p",
protocol: "9p",
readonly: false,
proxyFD: 5,
},
`# stub drive (9p)
[fsdev "incus_stub"]
fsdriver = "proxy"
sock_fd = "5"
fsdriver = "local"
security_model = "mapped-xattr"
readonly = "off"
path = "/var/9p"

[device "dev-incus_stub-9p"]
driver = "virtio-9p-pci"
Expand Down Expand Up @@ -898,15 +899,16 @@ func TestQemuConfigTemplates(t *testing.T) {
dev: qemuDevOpts{"ccw", "qemu_pcie0", "00.0", false},
devName: "stub2",
mountTag: "mtag2",
path: "/var/9p",
protocol: "9p",
readonly: true,
proxyFD: 3,
},
`# stub2 drive (9p)
[fsdev "incus_stub2"]
fsdriver = "proxy"
sock_fd = "3"
fsdriver = "local"
security_model = "mapped-xattr"
readonly = "on"
path = "/var/9p"

[device "dev-incus_stub2-9p"]
driver = "virtio-9p-ccw"
Expand Down
19 changes: 9 additions & 10 deletions internal/server/instance/drivers/driver_qemu_templates.go
Original file line number Diff line number Diff line change
Expand Up @@ -750,21 +750,20 @@ type qemuDriveDirOpts struct {
mountTag string
path string
protocol string
proxyFD int
readonly bool
}

func qemuDriveDir(opts *qemuDriveDirOpts) []cfgSection {
return qemuHostDrive(&qemuHostDriveOpts{
dev: opts.dev,
name: fmt.Sprintf("incus_%s", opts.devName),
comment: fmt.Sprintf("%s drive (%s)", opts.devName, opts.protocol),
mountTag: opts.mountTag,
protocol: opts.protocol,
fsdriver: "proxy",
readonly: opts.readonly,
path: opts.path,
sockFd: fmt.Sprintf("%d", opts.proxyFD),
dev: opts.dev,
name: fmt.Sprintf("incus_%s", opts.devName),
comment: fmt.Sprintf("%s drive (%s)", opts.devName, opts.protocol),
mountTag: opts.mountTag,
protocol: opts.protocol,
fsdriver: "local",
readonly: opts.readonly,
path: opts.path,
securityModel: "mapped-xattr",
})
}

Expand Down
Loading