lxd: use go-criu/crit for dump statistics

[snprajwal]

The go-criu package provides a function in the crit library to fetch dump statistics of a checkpointed process. This commit replaces the existing implementation of readCriuStatsDump() with this. It also removed the redundant definitions in the migration protobuf file.

[lxc-jenkins]

This pull request didn't trigger Jenkins as its author isn't in the allow list.

An organization member must perform one of the following:

• To have this branch tested by Jenkins, use the "ok to test" command.
• To have a one time test done, use the "test this please" command.

Those commands are simple Github comments of the format: "jenkins: COMMAND"

[snprajwal]

Do we exclude migration.pb.go from the lint check? It looks like the newer version of protoc likes to generate code without newlines.

[tomponline]

@snprajwal why are the protobuf definitions redundant?

[snprajwal]

@snprajwal why are the protobuf definitions redundant?

migration.proto maintained a copy of the stats definitions in images/stats.proto of the CRIU repo. go-criu provides the same proto bindings for stats in crit/images/stats.proto, hence the definitions in migration.proto can be removed.

[tomponline]

jenkins: test this please

[tomponline]

Do we exclude migration.pb.go from the lint check? It looks like the newer version of protoc likes to generate code without newlines.

@markylaing what do you think about this? Can we change the check easily?

@snprajwal please can we have the PR split into logical commits - so 1 commit to bring the existing proto bufs up to current version, and then 1 commit with your criu changes.

[markylaing]

@tomponline I'm not certain what the easiest way to change the check would be. Here is the line we use to get all the files we want to perform the newline check against: https://github.com/lxc/lxd/blob/aba9b14bd03ca8f4c0889c1aaa2bbc9c9419d906/test/lint/newline-after-block.sh#L5

We could probably add a flag to skip certain files without too much trouble.

[lxc-jenkins]

Testsuite passed

[snprajwal]

@markylaing we can modify the existing command to skip .pb.go files files=$(git ls-files --cached --modified --others "*.go" ":!:*.pb.go")

[markylaing]

@markylaing we can modify the existing command to skip .pb.go files files=$(git ls-files --cached --modified --others "*.go" ":!:*.pb.go")

@snprajwal Go for it :+1: thanks!

[snprajwal]

1 commit to bring the existing proto bufs up to current version, and then 1 commit with your criu changes.

@tomponline The protobuf change will have to be in the same commit which introduces go-criu, or else there will be a namespace conflict for the redefined messages across migration.proto and crit/images/stats.proto. This will cause the build to fail and break git bisect.

[tomponline]

The protobuf change will have to be in the same commit

I meant the change to the current protobufs but using the new protoc command that introduces the extra lines.

[snprajwal]

@tomponline There was no namespace conflict at build time, I could safely move all protobuf changes to a separate commit :)

[tomponline]

Excellent! Although we would still like a separate commit for changing to the new protoc before any other changes were made so that the differences introduced by removing the definitions can be seen more clearly. So 3 logical changes in 3 commits.

[snprajwal]

Oops I think I misunderstood what you wanted me to do. To clarify before I redo the commits again:

1. Run protoc on the old migration.proto and commit migration.pb.go
2. Commit go-criu and related changes
3. Commit the new migration.proto and migration.pb.go

Is this correct?

[tomponline]

Oops I think I misunderstood what you wanted me to do. To clarify before I redo the commits again:

1. Run protoc on the old `migration.proto` and commit `migration.pb.go`

2. Commit go-criu and related changes

3. Commit the new `migration.proto` and `migration.pb.go`

Is this correct?

Yep thats it exactly :)

[tomponline]

jenkins: test this please