diff --git a/devices/drivers.go b/devices/drivers.go index 0dae33c963..bcf5bb0c9b 100644 --- a/devices/drivers.go +++ b/devices/drivers.go @@ -7,24 +7,28 @@ import ( "context" "fmt" + "github.com/Microsoft/go-winio/pkg/guid" "github.com/Microsoft/hcsshim/internal/cmd" "github.com/Microsoft/hcsshim/internal/guestpath" "github.com/Microsoft/hcsshim/internal/log" "github.com/Microsoft/hcsshim/internal/resources" "github.com/Microsoft/hcsshim/internal/uvm" - "github.com/pkg/errors" ) -// InstallKernelDriver mounts a specified kernel driver, then installs it in the UVM. +// InstallDriver mounts a share from the host into the UVM, installs any kernel drivers in the share, +// and configures the environment for library files and/or binaries in the share. // -// `driver` is a directory path on the host that contains driver files for standard installation. +// InstallDriver mounts a specified kernel driver, then installs it in the UVM. +// +// `share` is a directory path on the host that contains files for standard driver installation. // For windows this means files for pnp installation (.inf, .cat, .sys, .cert files). // For linux this means a vhd file that contains the drivers under /lib/modules/`uname -r` for use -// with depmod and modprobe. +// with depmod and modprobe. For GPU, this vhd may also contain library files (under /usr/lib) and +// binaries (under /usr/bin or /usr/sbin) to be used in conjunction with the modules. // // Returns a ResourceCloser for the added mount. On failure, the mounted share will be released, // the returned ResourceCloser will be nil, and an error will be returned. -func InstallKernelDriver(ctx context.Context, vm *uvm.UtilityVM, driver string) (closer resources.ResourceCloser, err error) { +func InstallDrivers(ctx context.Context, vm *uvm.UtilityVM, share string, gpuDriver bool) (closer resources.ResourceCloser, err error) { defer func() { if err != nil && closer != nil { // best effort clean up allocated resource on failure @@ -36,25 +40,53 @@ func InstallKernelDriver(ctx context.Context, vm *uvm.UtilityVM, driver string) }() if vm.OS() == "windows" { options := vm.DefaultVSMBOptions(true) - closer, err = vm.AddVSMB(ctx, driver, options) + closer, err = vm.AddVSMB(ctx, share, options) if err != nil { - return closer, fmt.Errorf("failed to add VSMB share to utility VM for path %+v: %s", driver, err) + return closer, fmt.Errorf("failed to add VSMB share to utility VM for path %+v: %s", share, err) } - uvmPath, err := vm.GetVSMBUvmPath(ctx, driver, true) + uvmPath, err := vm.GetVSMBUvmPath(ctx, share, true) if err != nil { return closer, err } + // attempt to install even if the driver has already been installed before so we + // can guarantee the device is ready for use afterwards return closer, execPnPInstallDriver(ctx, vm, uvmPath) } + + // no need to reinstall if the driver has already been added in LCOW, return early + if _, err := vm.GetScsiUvmPath(ctx, share); err != uvm.ErrNotAttached { + return nil, err + } + + // first mount driver as scsi in standard mount location uvmPathForShare := fmt.Sprintf(guestpath.LCOWGlobalMountPrefixFmt, vm.UVMMountCounter()) - scsiCloser, err := vm.AddSCSI(ctx, driver, uvmPathForShare, true, false, []string{}, uvm.VMAccessTypeIndividual) + closer, err = vm.AddSCSI(ctx, + share, + uvmPathForShare, + true, + false, + []string{}, + uvm.VMAccessTypeIndividual) if err != nil { - return closer, fmt.Errorf("failed to add SCSI disk to utility VM for path %+v: %s", driver, err) + return closer, fmt.Errorf("failed to add SCSI disk to utility VM for path %+v: %s", share, err) } - return scsiCloser, execModprobeInstallDriver(ctx, vm, uvmPathForShare) + + // construct path that the drivers will be remounted as read/write in the UVM + driverGUID, err := guid.NewV4() + if err != nil { + return closer, fmt.Errorf("failed to create a guid path for driver %+v: %s", share, err) + } + uvmReadWritePath := fmt.Sprintf(guestpath.LCOWGlobalDriverPrefixFmt, driverGUID.String()) + if gpuDriver { + // if installing gpu drivers in lcow, use the nvidia mount path instead + uvmReadWritePath = guestpath.LCOWNvidiaMountPath + } + + // install drivers using gcs tool `install-drivers` + return closer, execGCSInstallDriver(ctx, vm, uvmPathForShare, uvmReadWritePath) } -func execModprobeInstallDriver(ctx context.Context, vm *uvm.UtilityVM, driverDir string) error { +func execGCSInstallDriver(ctx context.Context, vm *uvm.UtilityVM, driverDir string, driverReadWriteDir string) error { p, l, err := cmd.CreateNamedPipeListener() if err != nil { return err @@ -68,6 +100,7 @@ func execModprobeInstallDriver(ctx context.Context, vm *uvm.UtilityVM, driverDir args := []string{ "/bin/install-drivers", + driverReadWriteDir, driverDir, } req := &cmd.CmdProcessRequest{ @@ -75,22 +108,27 @@ func execModprobeInstallDriver(ctx context.Context, vm *uvm.UtilityVM, driverDir Stderr: p, } + // A call to `ExecInUvm` may fail in the following ways: + // - The process runs and exits with a non-zero exit code. In this case we need to wait on the output + // from stderr so we can log it for debugging. + // - There's an error trying to run the process. No need to wait for stderr logs. + // - There's an error copying IO. No need to wait for stderr logs. + // + // Since we cannot distinguish between the cases above, we should always wait to read the stderr output. exitCode, execErr := cmd.ExecInUvm(ctx, vm, req) // wait to finish parsing stdout results select { case err := <-errChan: - if err != nil { - return errors.Wrap(err, execErr.Error()) + if err != nil && err != noExecOutputErr { + return fmt.Errorf("failed to get stderr output from command %s: %v", driverDir, err) } case <-ctx.Done(): - return errors.Wrap(ctx.Err(), execErr.Error()) + return fmt.Errorf("timed out waiting for the console output from installing driver %s: %v", driverDir, ctx.Err()) } - if execErr != nil && execErr != noExecOutputErr { - return errors.Wrapf(execErr, "failed to install driver %s in uvm with exit code %d: %v", driverDir, exitCode, stderrOutput) + if execErr != nil { + return fmt.Errorf("%v: failed to install driver %s in uvm with exit code %d: %v", execErr, driverDir, exitCode, stderrOutput) } - - log.G(ctx).WithField("added drivers", driverDir).Debug("installed drivers") return nil } diff --git a/guest/kmsg/kmsg.go b/guest/kmsg/kmsg.go index 112b26b2cc..f7081bea70 100644 --- a/guest/kmsg/kmsg.go +++ b/guest/kmsg/kmsg.go @@ -1,4 +1,5 @@ //go:build linux +// +build linux package kmsg diff --git a/guest/runtime/hcsv2/nvidia_utils.go b/guest/runtime/hcsv2/nvidia_utils.go index 735bb4931d..5debe2d77c 100644 --- a/guest/runtime/hcsv2/nvidia_utils.go +++ b/guest/runtime/hcsv2/nvidia_utils.go @@ -24,10 +24,9 @@ const nvidiaDebugFilePath = "/nvidia-container.log" const nvidiaToolBinary = "nvidia-container-cli" -// TODO katiewasnothere: prestart hooks will be depracated, this needs to be moved to a createRuntime hook // described here: https://github.com/opencontainers/runtime-spec/blob/39c287c415bf86fb5b7506528d471db5405f8ca8/config.md#posix-platform-hooks -// addNvidiaDevicePreHook builds the arguments for nvidia-container-cli and creates the prestart hook -func addNvidiaDevicePreHook(ctx context.Context, spec *oci.Spec) error { +// addNvidiaDeviceHook builds the arguments for nvidia-container-cli and creates the prestart hook +func addNvidiaDeviceHook(ctx context.Context, spec *oci.Spec) error { genericHookBinary := "generichook" genericHookPath, err := exec.LookPath(genericHookBinary) if err != nil { @@ -72,13 +71,27 @@ func addNvidiaDevicePreHook(ctx context.Context, spec *oci.Spec) error { hookLogDebugFileEnvOpt := fmt.Sprintf("%s=%s", generichook.LogDebugFileEnvKey, nvidiaDebugFilePath) hookEnv := append(updateEnvWithNvidiaVariables(), hookLogDebugFileEnvOpt) nvidiaHook := hooks.NewOCIHook(genericHookPath, args, hookEnv) - return hooks.AddOCIHook(spec, hooks.Prestart, nvidiaHook) + return hooks.AddOCIHook(spec, hooks.CreateRuntime, nvidiaHook) +} + +// Helper function to find the usr/lib path for the installed nvidia library files. +// This function assumes that the drivers have been installed using +// gcstool's `install-drivers` binary. +func getNvidiaDriversUsrLibPath() string { + return fmt.Sprintf("%s/content/usr/lib", guestpath.LCOWNvidiaMountPath) + +} + +// Helper function to find the usr/bin path for the installed nvidia tools. +// This function assumes that the drivers have been installed using +// gcstool's `install-drivers` binary. +func getNvidiaDriverUsrBinPath() string { + return fmt.Sprintf("%s/content/usr/bin", guestpath.LCOWNvidiaMountPath) } // updateEnvWithNvidiaVariables creates an env with the nvidia gpu vhd in PATH and insecure mode set func updateEnvWithNvidiaVariables() []string { - nvidiaBin := fmt.Sprintf("%s/bin", guestpath.LCOWNvidiaMountPath) - env := updatePathEnv(nvidiaBin) + env := updatePathEnv(getNvidiaDriverUsrBinPath()) // NVC_INSECURE_MODE allows us to run nvidia-container-cli without seccomp // we don't currently use seccomp in the uvm, so avoid using it here for now as well env = append(env, "NVC_INSECURE_MODE=1") diff --git a/guest/runtime/hcsv2/spec.go b/guest/runtime/hcsv2/spec.go index 0dbc491830..debc2aea1d 100644 --- a/guest/runtime/hcsv2/spec.go +++ b/guest/runtime/hcsv2/spec.go @@ -15,7 +15,6 @@ import ( oci "github.com/opencontainers/runtime-spec/specs-go" "github.com/pkg/errors" - "github.com/Microsoft/hcsshim/internal/hooks" "github.com/Microsoft/hcsshim/internal/log" "github.com/Microsoft/hcsshim/pkg/annotations" ) @@ -239,12 +238,6 @@ func applyAnnotationsToSpec(ctx context.Context, spec *oci.Spec) error { return nil } -// Helper function to create an oci prestart hook to run ldconfig -func addLDConfigHook(_ context.Context, spec *oci.Spec, args, env []string) error { - ldConfigHook := hooks.NewOCIHook("/sbin/ldconfig", args, env) - return hooks.AddOCIHook(spec, hooks.Prestart, ldConfigHook) -} - // devShmMountWithSize returns a /dev/shm device mount with size set to // `sizeString` if it represents a valid size in KB, returns error otherwise. func devShmMountWithSize(sizeString string) (*oci.Mount, error) { diff --git a/guest/runtime/hcsv2/workload_container.go b/guest/runtime/hcsv2/workload_container.go index 35ba49744b..38dabb601a 100644 --- a/guest/runtime/hcsv2/workload_container.go +++ b/guest/runtime/hcsv2/workload_container.go @@ -16,6 +16,7 @@ import ( specInternal "github.com/Microsoft/hcsshim/internal/guest/spec" "github.com/Microsoft/hcsshim/internal/guestpath" + "github.com/Microsoft/hcsshim/internal/hooks" "github.com/Microsoft/hcsshim/internal/oc" "github.com/Microsoft/hcsshim/pkg/annotations" ) @@ -152,14 +153,16 @@ func setupWorkloadContainerSpec(ctx context.Context, sbid, id string, spec *oci. spec.Linux.CgroupsPath = "/containers/" + id if spec.Windows != nil { + // we only support Nvidia gpus right now if specHasGPUDevice(spec) { - // we only support Nvidia gpus right now - ldConfigargs := []string{"-l", "/run/nvidia/lib"} - env := updateEnvWithNvidiaVariables() - if err := addLDConfigHook(ctx, spec, ldConfigargs, env); err != nil { + // run ldconfig as a `CreateRuntime` hook so that it's executed in the runtime namespace. This is + // necessary so that when we execute the nvidia hook to assign the device, the runtime can + // find the nvidia library files. + ldconfigHook := hooks.NewOCIHook("/sbin/ldconfig", []string{getNvidiaDriversUsrLibPath()}, os.Environ()) + if err := hooks.AddOCIHook(spec, hooks.CreateRuntime, ldconfigHook); err != nil { return err } - if err := addNvidiaDevicePreHook(ctx, spec); err != nil { + if err := addNvidiaDeviceHook(ctx, spec); err != nil { return err } } diff --git a/guestpath/paths.go b/guestpath/paths.go index be812ba075..fd1479da8c 100644 --- a/guestpath/paths.go +++ b/guestpath/paths.go @@ -22,6 +22,9 @@ const ( // LCOWGlobalMountPrefixFmt is the path format in the LCOW UVM where global // mounts are added LCOWGlobalMountPrefixFmt = "/run/mounts/m%d" + // LCOWGlobalDriverPrefixFmt is the path format in the LCOW UVM where drivers + // are mounted as read/write + LCOWGlobalDriverPrefixFmt = "/run/drivers/%s" // WCOWGlobalMountPrefixFmt is the path prefix format in the WCOW UVM where // mounts are added WCOWGlobalMountPrefixFmt = "C:\\mounts\\m%d" diff --git a/hcsoci/create.go b/hcsoci/create.go index 7b3b7ef983..6506855ed2 100644 --- a/hcsoci/create.go +++ b/hcsoci/create.go @@ -299,7 +299,7 @@ func CreateContainer(ctx context.Context, createOptions *CreateOptions) (_ cow.C // install kernel drivers if necessary. // do this before network setup in case any of the drivers requested are // network drivers - driverClosers, err := installPodDrivers(ctx, coi.HostingSystem, coi.Spec.Annotations) + driverClosers, err := addSpecGuestDrivers(ctx, coi.HostingSystem, coi.Spec.Annotations) if err != nil { return nil, r, err } diff --git a/hcsoci/devices.go b/hcsoci/devices.go index 5903d6a1da..c8fd6d042f 100644 --- a/hcsoci/devices.go +++ b/hcsoci/devices.go @@ -14,7 +14,6 @@ import ( "github.com/pkg/errors" "github.com/Microsoft/hcsshim/internal/devices" - "github.com/Microsoft/hcsshim/internal/guestpath" hcsschema "github.com/Microsoft/hcsshim/internal/hcs/schema2" "github.com/Microsoft/hcsshim/internal/log" "github.com/Microsoft/hcsshim/internal/oci" @@ -216,29 +215,21 @@ func handleAssignedDevicesLCOW( if err != nil { return resultDevs, closers, errors.Wrapf(err, "failed to add gpu vhd to %v", vm.ID()) } - // use lcowNvidiaMountPath since we only support nvidia gpus right now - // must use scsi here since DDA'ing a hyper-v pci device is not supported on VMs that have ANY virtual memory // gpuvhd must be granted VM Group access. - options := []string{"ro"} - scsiMount, err := vm.AddSCSI( - ctx, - gpuSupportVhdPath, - guestpath.LCOWNvidiaMountPath, - true, - false, - options, - uvm.VMAccessTypeNoop, - ) + driverCloser, err := devices.InstallDrivers(ctx, vm, gpuSupportVhdPath, true) if err != nil { - return resultDevs, closers, errors.Wrapf(err, "failed to add scsi device %s in the UVM %s at %s", gpuSupportVhdPath, vm.ID(), guestpath.LCOWNvidiaMountPath) + return resultDevs, closers, err + } + if driverCloser != nil { + closers = append(closers, driverCloser) } - closers = append(closers, scsiMount) } return resultDevs, closers, nil } -func installPodDrivers(ctx context.Context, vm *uvm.UtilityVM, annotations map[string]string) (closers []resources.ResourceCloser, err error) { +// addSpecGuestDrivers is a helper function to install kernel drivers specified on a spec into the guest +func addSpecGuestDrivers(ctx context.Context, vm *uvm.UtilityVM, annotations map[string]string) (closers []resources.ResourceCloser, err error) { defer func() { if err != nil { // best effort clean up allocated resources on failure @@ -256,11 +247,13 @@ func installPodDrivers(ctx context.Context, vm *uvm.UtilityVM, annotations map[s return closers, err } for _, d := range drivers { - driverCloser, err := devices.InstallKernelDriver(ctx, vm, d) + driverCloser, err := devices.InstallDrivers(ctx, vm, d, false) if err != nil { return closers, err } - closers = append(closers, driverCloser) + if driverCloser != nil { + closers = append(closers, driverCloser) + } } return closers, err } diff --git a/hcsoci/resources_wcow.go b/hcsoci/resources_wcow.go index 9e08821831..11caf77358 100644 --- a/hcsoci/resources_wcow.go +++ b/hcsoci/resources_wcow.go @@ -112,7 +112,7 @@ func allocateWindowsResources(ctx context.Context, coi *createOptionsInternal, r // when driver installation completes, we are guaranteed that the device is ready for use, // so reinstall drivers to make sure the devices are ready when we proceed. // TODO katiewasnothere: we should find a way to avoid reinstalling drivers - driverClosers, err := installPodDrivers(ctx, coi.HostingSystem, coi.Spec.Annotations) + driverClosers, err := addSpecGuestDrivers(ctx, coi.HostingSystem, coi.Spec.Annotations) if err != nil { return err }