From b4b6650a5be089cfe357ee8511c5765d1a935fa1 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 | 9 ++++++++- pkg/bios/bios.go | 9 ++++++++- pkg/block/block.go | 9 ++++++++- pkg/chassis/chassis.go | 9 ++++++++- pkg/context/context.go | 26 +++++++++++++++++++++++++- pkg/cpu/cpu.go | 9 ++++++++- pkg/gpu/gpu.go | 9 ++++++++- pkg/memory/memory.go | 9 ++++++++- pkg/net/net.go | 9 ++++++++- pkg/pci/pci.go | 9 ++++++++- pkg/product/product.go | 9 ++++++++- pkg/topology/topology.go | 6 +++++- 12 files changed, 110 insertions(+), 12 deletions(-) diff --git a/pkg/baseboard/baseboard.go b/pkg/baseboard/baseboard.go index ed3a713a..6c5cf5f6 100644 --- a/pkg/baseboard/baseboard.go +++ b/pkg/baseboard/baseboard.go @@ -50,7 +50,14 @@ 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...)) +} + +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..86b06aed 100644 --- a/pkg/bios/bios.go +++ b/pkg/bios/bios.go @@ -50,7 +50,14 @@ 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...)) +} + +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..7cf3ac9c 100644 --- a/pkg/block/block.go +++ b/pkg/block/block.go @@ -134,7 +134,14 @@ 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...)) +} + +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..ec74e373 100644 --- a/pkg/chassis/chassis.go +++ b/pkg/chassis/chassis.go @@ -94,7 +94,14 @@ 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...)) +} + +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..7272b02b 100644 --- a/pkg/cpu/cpu.go +++ b/pkg/cpu/cpu.go @@ -117,7 +117,14 @@ 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...)) +} + +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..d0ff9a0e 100644 --- a/pkg/gpu/gpu.go +++ b/pkg/gpu/gpu.go @@ -56,7 +56,14 @@ 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...)) +} + +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..5417fe4f 100644 --- a/pkg/memory/memory.go +++ b/pkg/memory/memory.go @@ -35,7 +35,14 @@ type Info struct { } func New(opts ...*option.Option) (*Info, error) { - ctx := context.New(opts...) + return newWithContext(context.New(opts...)) +} + +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..3b02c5f9 100644 --- a/pkg/net/net.go +++ b/pkg/net/net.go @@ -49,7 +49,14 @@ 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...)) +} + +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..cac50a6d 100644 --- a/pkg/pci/pci.go +++ b/pkg/pci/pci.go @@ -151,7 +151,14 @@ 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...)) +} + +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..3c106059 100644 --- a/pkg/product/product.go +++ b/pkg/product/product.go @@ -73,7 +73,14 @@ 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...)) +} + +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