Skip to content

Commit

Permalink
incusd/device/disk: Remove virtfs-proxy-helper dependency
Browse files Browse the repository at this point in the history 
Signed-off-by: Benjamin Somers <[email protected]>
  • Loading branch information
@bensmrs
bensmrs committed Dec 24, 2024
1 parent a9cd481 commit d6d456b
Show file tree
Hide file tree
Showing 4 changed files with 12 additions and 184 deletions.
121 changes: 0 additions & 121 deletions internal/server/device/device_utils_disk.go
View file
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
View file
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
View file
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
19 changes: 9 additions & 10 deletions internal/server/instance/drivers/driver_qemu_templates.go
View file
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

0 comments on commit d6d456b

Please sign in to comment.