Looks like a multiplexer with the naming ...Mux. Please call this onlineCPUMemLock

This looks like a lot ! We cannot afford to wait for so much time between two retries. Something like 50ms seems more appropriate, and maybe increase the number of retries if you think that in some cases it might take some time.

I think this should not be spawned into a go routine. This is the caller responsibility to know if it should wait or not for the return of this function.
Also, if we look at this patch, we expect to be able to return a valid error in case we could not hotplug the amount of CPUs provided as parameter.

Was thinking that nb_cpus or something with number in it, might be more appropriate as the parameter here, since we're providing a number of CPUs.

Having functions onlineCPUResources() and onlineMemResources() seems more appropriate to make the code more readable.

func onlineCPUResources(nbCPU int) error {
        resource := onlineResource{
		sysfsOnlinePath: sysfsCPUOnlinePath,
		regexpPattern:   cpuRegexpPattern,
	}

	var count uint32
	for i := uint32(0); i < onlineCPUMaxTries; i++ {
		r, err := onlineResources(resource)
		if err != nil {
			return err
		}
		count += r
		if count == nbCPU {
			return nil
		}
		time.Sleep(onlineCPUMemWaitTime)
	}

	return fmt.Errorf("only %d of %d were connected", count, expectedResources)
}

func onlineMemResources() error {
        resource := onlineResource{
		sysfsOnlinePath: sysfsMemOnlinePath,
		regexpPattern:   memRegexpPattern,
	}

        _, err := onlineResources(resource)
        return err
}

IMO, no need to specify this Wait through the spec, the caller could simply decide to run the call to OnlineCPUMem() into a go routine if needed.

In order to have a fast boot time, the go routine should run inside the VM, otherwise the runtime fails with next error

rpc error: code = Unavailable desc = transport is closing.

Ok makes sense!

You should probably check the error here. What you should do I'm not sure. Perhaps simply continue. Looking and the old code no action was taken after this line, so although the error wasn't discarded, ignoring it was harmless. But with these changes I guess it's not.

fixed, thanks

I'm not sure it's fixed correctly though. If you get one write error you'll give up entirely, i.e., the for loop in onlineCPUResources will terminate. Is this intentional?

Do the directory contents of resource.sysfsOnlinePath change between invocations of this function? If not do you need to read the directory up to 10 times and check the online files 10 times. I'm suppose I'm thinking of a situation in which in the first invocation you online 4 out of 5 cpus. If I read the code correctly you'd then re-read the online files associated with those CPUs up to 9 more times.

In systems running low on resources, yes. Read the directory in each iteration is needed

If you get one write error you'll give up entirely, i.e., the for loop in onlineCPUResources will terminate. Is this intentional?

you're right, would be better to log the error and continue

check req.NbCpus is larger than 0?

nits: move it to after count++. It only makes sense to check when count increases.

Nit: shouldn't these be strictly?:

cpuRegexpPattern = "cpu[0-9][0-9]*"
memRegexpPattern = "memory[0-9][0-9]*"

It's fine as long as files called cpu or memory are never created in that directory.

Ok makes sense!

s/until/once/

s/else/otherwise/

s/immediately/immediately./

s/Specifies/specifies/
s/online/online./

Looking at the code, there's no limit if nbResources==0 either.