From 0c2739a866a5ecc05b2f211110ea7b2f021cdcc5 Mon Sep 17 00:00:00 2001 From: Francesco Romani Date: Sun, 11 Apr 2021 18:16:41 +0200 Subject: [PATCH] context: introduce dependent context The PR https://github.com/jaypipes/ghw/pull/236 fixes a snapshot directory leakage issue, but also highlight a context lifecycle/ownership issue. In general in ghw each package creates its own context, including the cases on which a package is consumed by another, for example `topology` and `pci`. However, in this case, it would make more sense to reuse the context from the parent package, because the two packages are tightly coupled. Before the introduction of the the transparent snapshot support (https://github.com/jaypipes/ghw/pull/202), the above mechanism worked, because each context, each time, was consuming over and over again the same parameters (e.g. environment variables); besides some resource waste, this had no negative effect. When introducing snapshots in the picture, repeatedly unpacking the same snapshot to consume the same data is much more wasteful. So it makes sense to introduce explicitly the concept of dependent context. We add a functio to create a context subordinate to another. The subordinate context will effectively borrow all the resources from the parent one. The parent is in charge to perform any setup/teardown (e.g. for snapshot, to learn which chroot should be used, if at all) and so forth. Making this dependency explicit allow the client code to manage correctly the lifecycle of the topmost context, hence of all the resources. Signed-off-by: Francesco Romani --- pkg/baseboard/baseboard.go | 13 ++++++++++++- pkg/bios/bios.go | 13 ++++++++++++- pkg/block/block.go | 13 ++++++++++++- pkg/chassis/chassis.go | 13 ++++++++++++- pkg/context/context.go | 26 +++++++++++++++++++++++++- pkg/cpu/cpu.go | 13 ++++++++++++- pkg/gpu/gpu.go | 13 ++++++++++++- pkg/memory/memory.go | 15 ++++++++++++++- pkg/net/net.go | 13 ++++++++++++- pkg/pci/pci.go | 13 ++++++++++++- pkg/product/product.go | 13 ++++++++++++- pkg/topology/topology.go | 6 +++++- 12 files changed, 152 insertions(+), 12 deletions(-) diff --git a/pkg/baseboard/baseboard.go b/pkg/baseboard/baseboard.go index ed3a713a..cfd0af6e 100644 --- a/pkg/baseboard/baseboard.go +++ b/pkg/baseboard/baseboard.go @@ -50,7 +50,18 @@ func (i *Info) String() string { // New returns a pointer to an Info struct containing information about the // host's baseboard func New(opts ...*option.Option) (*Info, error) { - ctx := context.New(opts...) + return newWithContext(context.New(opts...)) +} + +// New returns a pointer to an Info struct containing information about the +// host's baseboard, reusing a given context. +// Use this function when you want to consume this package from another, +// ensuring the two see a coherent set of resources. +func NewWithContext(ctx *context.Context) (*Info, error) { + return newWithContext(context.FromParent(ctx)) +} + +func newWithContext(ctx *context.Context) (*Info, error) { info := &Info{ctx: ctx} if err := ctx.Do(info.load); err != nil { return nil, err diff --git a/pkg/bios/bios.go b/pkg/bios/bios.go index 85a7c64b..28941dd4 100644 --- a/pkg/bios/bios.go +++ b/pkg/bios/bios.go @@ -50,7 +50,18 @@ func (i *Info) String() string { // New returns a pointer to a Info struct containing information // about the host's BIOS func New(opts ...*option.Option) (*Info, error) { - ctx := context.New(opts...) + return newWithContext(context.New(opts...)) +} + +// New returns a pointer to a Info struct containing information +// about the host's BIOS, reusing a given context. +// Use this function when you want to consume this package from another, +// ensuring the two see a coherent set of resources. +func NewWithContext(ctx *context.Context) (*Info, error) { + return newWithContext(context.FromParent(ctx)) +} + +func newWithContext(ctx *context.Context) (*Info, error) { info := &Info{ctx: ctx} if err := ctx.Do(info.load); err != nil { return nil, err diff --git a/pkg/block/block.go b/pkg/block/block.go index 7a2e4e38..77c4c411 100644 --- a/pkg/block/block.go +++ b/pkg/block/block.go @@ -134,7 +134,18 @@ type Info struct { // New returns a pointer to an Info struct that describes the block storage // resources of the host system. func New(opts ...*option.Option) (*Info, error) { - ctx := context.New(opts...) + return newWithContext(context.New(opts...)) +} + +// New returns a pointer to an Info struct that describes the block storage +// resources of the host system, reusing a given context. +// Use this function when you want to consume this package from another, +// ensuring the two see a coherent set of resources. +func NewWithContext(ctx *context.Context) (*Info, error) { + return newWithContext(context.FromParent(ctx)) +} + +func newWithContext(ctx *context.Context) (*Info, error) { info := &Info{ctx: ctx} if err := ctx.Do(info.load); err != nil { return nil, err diff --git a/pkg/chassis/chassis.go b/pkg/chassis/chassis.go index e11f0447..a24fe7ea 100644 --- a/pkg/chassis/chassis.go +++ b/pkg/chassis/chassis.go @@ -94,7 +94,18 @@ func (i *Info) String() string { // New returns a pointer to a Info struct containing information // about the host's chassis func New(opts ...*option.Option) (*Info, error) { - ctx := context.New(opts...) + return newWithContext(context.New(opts...)) +} + +// New returns a pointer to a Info struct containing information +// about the host's chassis, reusing a given context. +// Use this function when you want to consume this package from another, +// ensuring the two see a coherent set of resources. +func NewWithContext(ctx *context.Context) (*Info, error) { + return newWithContext(context.FromParent(ctx)) +} + +func newWithContext(ctx *context.Context) (*Info, error) { info := &Info{ctx: ctx} if err := ctx.Do(info.load); err != nil { return nil, err diff --git a/pkg/context/context.go b/pkg/context/context.go index 255d5ebf..b22a7f2d 100644 --- a/pkg/context/context.go +++ b/pkg/context/context.go @@ -20,6 +20,7 @@ type Context struct { SnapshotRoot string SnapshotExclusive bool alert option.Alerter + parent *Context } // New returns a Context struct pointer that has had various options set on it @@ -50,7 +51,7 @@ func New(opts ...*option.Option) *Context { return ctx } -// FromEnv returns an Option that has been populated from the environs or +// FromEnv returns a Context that has been populated from the environs or // default options values func FromEnv() *Context { chrootVal := option.EnvOrDefaultChroot() @@ -67,6 +68,21 @@ func FromEnv() *Context { } } +// FromParent returns a Context which is subordinate to another one. +// This means only the parent Context is in charge to the environment (e.g. setting up the snapshot). +// Subordinate contexts will defer any environment-changing action to their parent. +func FromParent(ctx *Context) *Context { + return &Context{ + Chroot: ctx.Chroot, + EnableTools: ctx.EnableTools, + SnapshotPath: ctx.SnapshotPath, + SnapshotRoot: ctx.SnapshotRoot, + SnapshotExclusive: ctx.SnapshotExclusive, + alert: ctx.alert, + parent: ctx, + } +} + // Do wraps a Setup/Teardown pair around the given function func (ctx *Context) Do(fn func() error) error { err := ctx.Setup() @@ -83,6 +99,10 @@ func (ctx *Context) Do(fn func() error) error { // You should call `Setup` just once. It is safe to call `Setup` if you don't make // use of optional extra features - `Setup` will do nothing. func (ctx *Context) Setup() error { + if ctx.parent != nil { + // nothing to do - the parent is in charge + return nil + } if ctx.SnapshotPath == "" { // nothing to do! return nil @@ -111,6 +131,10 @@ func (ctx *Context) Setup() error { // You should always call `Teardown` if you called `Setup` to free any resources // acquired by `Setup`. Check `Do` for more automated management. func (ctx *Context) Teardown() error { + if ctx.parent != nil { + // nothing to do - the parent is in charge + return nil + } if ctx.SnapshotRoot != "" { // if the client code provided the unpack directory, // then it is also in charge of the cleanup. diff --git a/pkg/cpu/cpu.go b/pkg/cpu/cpu.go index 2fa0cd2d..0312d50c 100644 --- a/pkg/cpu/cpu.go +++ b/pkg/cpu/cpu.go @@ -117,7 +117,18 @@ type Info struct { // New returns a pointer to an Info struct that contains information about the // CPUs on the host system func New(opts ...*option.Option) (*Info, error) { - ctx := context.New(opts...) + return newWithContext(context.New(opts...)) +} + +// New returns a pointer to an Info struct that contains information about the +// CPUs on the host system, reusing a given context. +// Use this function when you want to consume this package from another, +// ensuring the two see a coherent set of resources. +func NewWithContext(ctx *context.Context) (*Info, error) { + return newWithContext(context.FromParent(ctx)) +} + +func newWithContext(ctx *context.Context) (*Info, error) { info := &Info{ctx: ctx} if err := ctx.Do(info.load); err != nil { return nil, err diff --git a/pkg/gpu/gpu.go b/pkg/gpu/gpu.go index 65864c7e..3ce9d28b 100644 --- a/pkg/gpu/gpu.go +++ b/pkg/gpu/gpu.go @@ -56,7 +56,18 @@ type Info struct { // New returns a pointer to an Info struct that contains information about the // graphics cards on the host system func New(opts ...*option.Option) (*Info, error) { - ctx := context.New(opts...) + return newWithContext(context.New(opts...)) +} + +// New returns a pointer to an Info struct that contains information about the +// graphics cards on the host system, reusing a given context. +// Use this function when you want to consume this package from another, +// ensuring the two see a coherent set of resources. +func NewWithContext(ctx *context.Context) (*Info, error) { + return newWithContext(context.FromParent(ctx)) +} + +func newWithContext(ctx *context.Context) (*Info, error) { info := &Info{ctx: ctx} if err := ctx.Do(info.load); err != nil { return nil, err diff --git a/pkg/memory/memory.go b/pkg/memory/memory.go index 93605d2e..de789857 100644 --- a/pkg/memory/memory.go +++ b/pkg/memory/memory.go @@ -34,8 +34,21 @@ type Info struct { Modules []*Module `json:"modules"` } +// New returns a pointer to an Info struct containing information about the +// host's memory. func New(opts ...*option.Option) (*Info, error) { - ctx := context.New(opts...) + return newWithContext(context.New(opts...)) +} + +// New returns a pointer to an Info struct containing information about the +// host's memory, reusing a given context. +// Use this function when you want to consume this package from another, +// ensuring the two see a coherent set of resources. +func NewWithContext(ctx *context.Context) (*Info, error) { + return newWithContext(context.FromParent(ctx)) +} + +func newWithContext(ctx *context.Context) (*Info, error) { info := &Info{ctx: ctx} if err := ctx.Do(info.load); err != nil { return nil, err diff --git a/pkg/net/net.go b/pkg/net/net.go index 8994d112..c25f6c65 100644 --- a/pkg/net/net.go +++ b/pkg/net/net.go @@ -49,7 +49,18 @@ type Info struct { // New returns a pointer to an Info struct that contains information about the // network interface controllers (NICs) on the host system func New(opts ...*option.Option) (*Info, error) { - ctx := context.New(opts...) + return newWithContext(context.New(opts...)) +} + +// New returns a pointer to an Info struct that contains information about the +// network interface controllers (NICs) on the host system, reusing a given context. +// Use this function when you want to consume this package from another, +// ensuring the two see a coherent set of resources. +func NewWithContext(ctx *context.Context) (*Info, error) { + return newWithContext(context.FromParent(ctx)) +} + +func newWithContext(ctx *context.Context) (*Info, error) { info := &Info{ctx: ctx} if err := ctx.Do(info.load); err != nil { return nil, err diff --git a/pkg/pci/pci.go b/pkg/pci/pci.go index 50f3b9f3..742c4a53 100644 --- a/pkg/pci/pci.go +++ b/pkg/pci/pci.go @@ -151,7 +151,18 @@ func (i *Info) String() string { // New returns a pointer to an Info struct that contains information about the // PCI devices on the host system func New(opts ...*option.Option) (*Info, error) { - ctx := context.New(opts...) + return newWithContext(context.New(opts...)) +} + +// New returns a pointer to an Info struct that contains information about the +// PCI devices on the host system, reusing a given context. +// Use this function when you want to consume this package from another, +// ensuring the two see a coherent set of resources. +func NewWithContext(ctx *context.Context) (*Info, error) { + return newWithContext(context.FromParent(ctx)) +} + +func newWithContext(ctx *context.Context) (*Info, error) { // by default we don't report NUMA information; // we will only if are sure we are running on NUMA architecture arch := topology.ARCHITECTURE_SMP diff --git a/pkg/product/product.go b/pkg/product/product.go index 6a2e1ee0..80999804 100644 --- a/pkg/product/product.go +++ b/pkg/product/product.go @@ -73,7 +73,18 @@ func (i *Info) String() string { // New returns a pointer to a Info struct containing information // about the host's product func New(opts ...*option.Option) (*Info, error) { - ctx := context.New(opts...) + return newWithContext(context.New(opts...)) +} + +// New returns a pointer to a Info struct containing information +// about the host's product, reusing a given context. +// Use this function when you want to consume this package from another, +// ensuring the two see a coherent set of resources. +func NewWithContext(ctx *context.Context) (*Info, error) { + return newWithContext(context.FromParent(ctx)) +} + +func newWithContext(ctx *context.Context) (*Info, error) { info := &Info{ctx: ctx} if err := ctx.Do(info.load); err != nil { return nil, err diff --git a/pkg/topology/topology.go b/pkg/topology/topology.go index a846584d..7e846377 100644 --- a/pkg/topology/topology.go +++ b/pkg/topology/topology.go @@ -79,13 +79,17 @@ type Info struct { // New returns a pointer to an Info struct that contains information about the // NUMA topology on the host system func New(opts ...*option.Option) (*Info, error) { - return NewWithContext(context.New(opts...)) + return newWithContext(context.New(opts...)) } // NewWithContext returns a pointer to an Info struct that contains information about // the NUMA topology on the host system. Use this function when you want to consume // the topology package from another package (e.g. pci, gpu) func NewWithContext(ctx *context.Context) (*Info, error) { + return newWithContext(context.FromParent(ctx)) +} + +func newWithContext(ctx *context.Context) (*Info, error) { info := &Info{ctx: ctx} if err := ctx.Do(info.load); err != nil { return nil, err