[1/2] RPC: add version check interface

Submitted by Adrian Reber on March 15, 2017, 2:26 p.m.

Details

Message ID 20170315142618.1484-1-adrian@lisas.de
State New
Series "Series without cover letter"
Headers show

Commit Message

Adrian Reber March 15, 2017, 2:26 p.m.
From: Adrian Reber <areber@redhat.com>

Instead of parsing the output of 'criu -V' this offers a RPC interface
to get CRIU's version. In a follow up patch a test script is included
to use the new interface:

 ./version.py
 Connecting to CRIU in swrk mode to check the version:
 RPC: Success
 CRIU major 2
 CRIU minor 12
 CRIU gitid v2.12-635-g6d3ae4d

This change exports the following version fields:
 * major
 * minor
 * gitid
 * sublevel (optional)
 * extra (optional)
 * name (optional)

A 'gitid' of '0' (the '0' is a string) means that it is not built from a
git checkout.

Signed-off-by: Adrian Reber <areber@redhat.com>
---
 criu/cr-service.c | 69 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 images/rpc.proto  | 14 +++++++++++
 2 files changed, 83 insertions(+)

Patch hide | download patch | download mbox

diff --git a/criu/cr-service.c b/criu/cr-service.c
index 78142b0..7e17199 100644
--- a/criu/cr-service.c
+++ b/criu/cr-service.c
@@ -15,6 +15,7 @@ 
 #include <arpa/inet.h>
 #include <sched.h>
 
+#include "version.h"
 #include "crtools.h"
 #include "cr_options.h"
 #include "external.h"
@@ -806,11 +807,76 @@  static int chk_keepopen_req(CriuReq *msg)
 		return 0;
 	else if (msg->type == CRIU_REQ_TYPE__FEATURE_CHECK)
 		return 0;
+	else if (msg->type == CRIU_REQ_TYPE__VERSION)
+		return 0;
 
 	return -1;
 }
 
 /*
+ * Return the version information, depending on the information
+ * available in version.h
+ */
+static int handle_version(int sk, CriuReq * msg)
+{
+	CriuResp resp = CRIU_RESP__INIT;
+	CriuVersion version = CRIU_VERSION__INIT;
+	int pid, status;
+	int ret;
+
+
+	pid = fork();
+	if (pid < 0) {
+		pr_perror("Can't fork");
+		goto out;
+	}
+
+	if (pid == 0) {
+		/* This assumes we will always have a major and minor version */
+		version.major = CRIU_VERSION_MAJOR;
+		version.minor = CRIU_VERSION_MINOR;
+		version.gitid = xstrdup(CRIU_GITID);
+#ifdef CRIU_VERSION_SUBLEVEL
+		version.has_sublevel = 1;
+		version.sublevel = CRIU_VERSION_SUBLEVEL;
+#endif
+#ifdef CRIU_VERSION_EXTRA
+		version.has_extra = 1;
+		version.extra = CRIU_VERSION_EXTRA;
+#endif
+#ifdef CRIU_VERSION_NAME
+		/* This is not actually exported in version.h */
+		version.name = xstrdup(CRIU_VERSION_NAME);
+#endif
+		resp.type = msg->type;
+		resp.success = true;
+		resp.version = &version;
+		return send_criu_msg(sk, &resp);
+	}
+
+	ret = waitpid(pid, &status,  0);
+	if (ret == -1)
+		goto out;
+
+	if (WIFEXITED(status) && !WEXITSTATUS(status))
+		/*
+		 * The child process exited and was able to send the answer.
+		 * Nothing more to do here.
+		 */
+		return 0;
+
+	/*
+	 * The child process was not able to send an answer. Tell
+	 * the RPC client that something did not work as expected.
+	 */
+out:
+	resp.type = msg->type;
+	resp.success = false;
+
+	return send_criu_msg(sk, &resp);
+}
+
+/*
  * Generic function to handle CRIU_REQ_TYPE__FEATURE_CHECK.
  *
  * The function will have resp.success = true for most cases
@@ -1004,6 +1070,9 @@  more:
 	case CRIU_REQ_TYPE__FEATURE_CHECK:
 		ret = handle_feature_check(sk, msg);
 		break;
+	case CRIU_REQ_TYPE__VERSION:
+		ret = handle_version(sk, msg);
+		break;
 
 	default:
 		send_criu_err(sk, "Invalid req");
diff --git a/images/rpc.proto b/images/rpc.proto
index f894ae1..bfb8dfc 100644
--- a/images/rpc.proto
+++ b/images/rpc.proto
@@ -140,6 +140,8 @@  enum criu_req_type {
 	CPUINFO_CHECK	= 8;
 
 	FEATURE_CHECK	= 9;
+
+	VERSION		= 10;
 }
 
 /*
@@ -193,4 +195,16 @@  message criu_resp {
 	optional int32			cr_errno	= 7;
 	optional criu_features		features	= 8;
 	optional string			cr_errmsg	= 9;
+	optional criu_version		version		= 10;
+}
+
+/* Answer for criu_req_type.VERSION requests */
+message criu_version {
+	required int32			major		= 1;
+	required int32			minor		= 2;
+	/* if gitid is '0' then it is not from a git checkout */
+	required string			gitid		= 3;
+	optional int32			sublevel	= 4;
+	optional int32			extra		= 5;
+	optional string			name		= 6;
 }

Comments

Andrey Vagin March 31, 2017, 9:39 p.m.
On Wed, Mar 15, 2017 at 03:26:17PM +0100, Adrian Reber wrote:
> From: Adrian Reber <areber@redhat.com>
> 
> Instead of parsing the output of 'criu -V' this offers a RPC interface
> to get CRIU's version. In a follow up patch a test script is included
> to use the new interface:
> 
>  ./version.py
>  Connecting to CRIU in swrk mode to check the version:
>  RPC: Success
>  CRIU major 2
>  CRIU minor 12
>  CRIU gitid v2.12-635-g6d3ae4d
> 
> This change exports the following version fields:
>  * major
>  * minor
>  * gitid
>  * sublevel (optional)
>  * extra (optional)
>  * name (optional)
> 
> A 'gitid' of '0' (the '0' is a string) means that it is not built from a
> git checkout.
> 
> Signed-off-by: Adrian Reber <areber@redhat.com>
> ---
>  criu/cr-service.c | 69 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  images/rpc.proto  | 14 +++++++++++
>  2 files changed, 83 insertions(+)
> 
> diff --git a/criu/cr-service.c b/criu/cr-service.c
> index 78142b0..7e17199 100644
> --- a/criu/cr-service.c
> +++ b/criu/cr-service.c
> @@ -15,6 +15,7 @@
>  #include <arpa/inet.h>
>  #include <sched.h>
>  
> +#include "version.h"
>  #include "crtools.h"
>  #include "cr_options.h"
>  #include "external.h"
> @@ -806,11 +807,76 @@ static int chk_keepopen_req(CriuReq *msg)
>  		return 0;
>  	else if (msg->type == CRIU_REQ_TYPE__FEATURE_CHECK)
>  		return 0;
> +	else if (msg->type == CRIU_REQ_TYPE__VERSION)
> +		return 0;
>  
>  	return -1;
>  }
>  
>  /*
> + * Return the version information, depending on the information
> + * available in version.h
> + */
> +static int handle_version(int sk, CriuReq * msg)
> +{
> +	CriuResp resp = CRIU_RESP__INIT;
> +	CriuVersion version = CRIU_VERSION__INIT;
> +	int pid, status;
> +	int ret;
> +
> +
> +	pid = fork();

Why do we need a new process here?

> +	if (pid < 0) {
> +		pr_perror("Can't fork");
> +		goto out;
> +	}
> +
> +	if (pid == 0) {
> +		/* This assumes we will always have a major and minor version */
> +		version.major = CRIU_VERSION_MAJOR;
> +		version.minor = CRIU_VERSION_MINOR;
> +		version.gitid = xstrdup(CRIU_GITID);
> +#ifdef CRIU_VERSION_SUBLEVEL
> +		version.has_sublevel = 1;
> +		version.sublevel = CRIU_VERSION_SUBLEVEL;
> +#endif
> +#ifdef CRIU_VERSION_EXTRA
> +		version.has_extra = 1;
> +		version.extra = CRIU_VERSION_EXTRA;
> +#endif
> +#ifdef CRIU_VERSION_NAME
> +		/* This is not actually exported in version.h */
> +		version.name = xstrdup(CRIU_VERSION_NAME);
> +#endif
> +		resp.type = msg->type;
> +		resp.success = true;
> +		resp.version = &version;
> +		return send_criu_msg(sk, &resp);
> +	}
> +
> +	ret = waitpid(pid, &status,  0);
> +	if (ret == -1)
> +		goto out;
> +
> +	if (WIFEXITED(status) && !WEXITSTATUS(status))
> +		/*
> +		 * The child process exited and was able to send the answer.
> +		 * Nothing more to do here.
> +		 */
> +		return 0;
> +
> +	/*
> +	 * The child process was not able to send an answer. Tell
> +	 * the RPC client that something did not work as expected.
> +	 */
> +out:
> +	resp.type = msg->type;
> +	resp.success = false;
> +
> +	return send_criu_msg(sk, &resp);
> +}
> +
> +/*
>   * Generic function to handle CRIU_REQ_TYPE__FEATURE_CHECK.
>   *
>   * The function will have resp.success = true for most cases
> @@ -1004,6 +1070,9 @@ more:
>  	case CRIU_REQ_TYPE__FEATURE_CHECK:
>  		ret = handle_feature_check(sk, msg);
>  		break;
> +	case CRIU_REQ_TYPE__VERSION:
> +		ret = handle_version(sk, msg);
> +		break;
>  
>  	default:
>  		send_criu_err(sk, "Invalid req");
> diff --git a/images/rpc.proto b/images/rpc.proto
> index f894ae1..bfb8dfc 100644
> --- a/images/rpc.proto
> +++ b/images/rpc.proto
> @@ -140,6 +140,8 @@ enum criu_req_type {
>  	CPUINFO_CHECK	= 8;
>  
>  	FEATURE_CHECK	= 9;
> +
> +	VERSION		= 10;
>  }
>  
>  /*
> @@ -193,4 +195,16 @@ message criu_resp {
>  	optional int32			cr_errno	= 7;
>  	optional criu_features		features	= 8;
>  	optional string			cr_errmsg	= 9;
> +	optional criu_version		version		= 10;
> +}
> +
> +/* Answer for criu_req_type.VERSION requests */
> +message criu_version {
> +	required int32			major		= 1;
> +	required int32			minor		= 2;
> +	/* if gitid is '0' then it is not from a git checkout */
> +	required string			gitid		= 3;

Maybe it should be optional? ^^^^^

> +	optional int32			sublevel	= 4;
> +	optional int32			extra		= 5;
> +	optional string			name		= 6;
>  }
> -- 
> 2.9.3
> 
> _______________________________________________
> CRIU mailing list
> CRIU@openvz.org
> https://lists.openvz.org/mailman/listinfo/criu
Adrian Reber April 4, 2017, 9:04 a.m.
On Fri, Mar 31, 2017 at 02:39:42PM -0700, Andrei Vagin wrote:
> On Wed, Mar 15, 2017 at 03:26:17PM +0100, Adrian Reber wrote:
> > From: Adrian Reber <areber@redhat.com>
> > 
> > Instead of parsing the output of 'criu -V' this offers a RPC interface
> > to get CRIU's version. In a follow up patch a test script is included
> > to use the new interface:
> > 
> >  ./version.py
> >  Connecting to CRIU in swrk mode to check the version:
> >  RPC: Success
> >  CRIU major 2
> >  CRIU minor 12
> >  CRIU gitid v2.12-635-g6d3ae4d
> > 
> > This change exports the following version fields:
> >  * major
> >  * minor
> >  * gitid
> >  * sublevel (optional)
> >  * extra (optional)
> >  * name (optional)
> > 
> > A 'gitid' of '0' (the '0' is a string) means that it is not built from a
> > git checkout.
> > 
> > Signed-off-by: Adrian Reber <areber@redhat.com>
> > ---
> >  criu/cr-service.c | 69 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
> >  images/rpc.proto  | 14 +++++++++++
> >  2 files changed, 83 insertions(+)
> > 
> > diff --git a/criu/cr-service.c b/criu/cr-service.c
> > index 78142b0..7e17199 100644
> > --- a/criu/cr-service.c
> > +++ b/criu/cr-service.c
> > @@ -15,6 +15,7 @@
> >  #include <arpa/inet.h>
> >  #include <sched.h>
> >  
> > +#include "version.h"
> >  #include "crtools.h"
> >  #include "cr_options.h"
> >  #include "external.h"
> > @@ -806,11 +807,76 @@ static int chk_keepopen_req(CriuReq *msg)
> >  		return 0;
> >  	else if (msg->type == CRIU_REQ_TYPE__FEATURE_CHECK)
> >  		return 0;
> > +	else if (msg->type == CRIU_REQ_TYPE__VERSION)
> > +		return 0;
> >  
> >  	return -1;
> >  }
> >  
> >  /*
> > + * Return the version information, depending on the information
> > + * available in version.h
> > + */
> > +static int handle_version(int sk, CriuReq * msg)
> > +{
> > +	CriuResp resp = CRIU_RESP__INIT;
> > +	CriuVersion version = CRIU_VERSION__INIT;
> > +	int pid, status;
> > +	int ret;
> > +
> > +
> > +	pid = fork();
> 
> Why do we need a new process here?

Good question ;-) This was one of the points were I was unsure about the
right solution. Pavel mentioned, that the fork is used for RPC functions
which might exit cleanly. The main problem was that it is not clear how
protobuf-c handles strings. Further down I am strdup()-ing the version
strings into the protobuf message. The protobuf-c documentation
regarding string handling says: TODO

https://github.com/protobuf-c/protobuf-c/wiki/Examples#strings

So I don't know if strdup() is the right tool for protobuf-c. It works,
but it leaves strings allocated which I don't know when or where to
free. Therefore I am going the easy way. fork() strdup() forget.

Do you know what the correct way to handle strings with protobuf-c is?
Then I could remove the fork().

> > +	if (pid < 0) {
> > +		pr_perror("Can't fork");
> > +		goto out;
> > +	}
> > +
> > +	if (pid == 0) {
> > +		/* This assumes we will always have a major and minor version */
> > +		version.major = CRIU_VERSION_MAJOR;
> > +		version.minor = CRIU_VERSION_MINOR;
> > +		version.gitid = xstrdup(CRIU_GITID);
> > +#ifdef CRIU_VERSION_SUBLEVEL
> > +		version.has_sublevel = 1;
> > +		version.sublevel = CRIU_VERSION_SUBLEVEL;
> > +#endif
> > +#ifdef CRIU_VERSION_EXTRA
> > +		version.has_extra = 1;
> > +		version.extra = CRIU_VERSION_EXTRA;
> > +#endif
> > +#ifdef CRIU_VERSION_NAME
> > +		/* This is not actually exported in version.h */
> > +		version.name = xstrdup(CRIU_VERSION_NAME);
> > +#endif
> > +		resp.type = msg->type;
> > +		resp.success = true;
> > +		resp.version = &version;
> > +		return send_criu_msg(sk, &resp);
> > +	}
> > +
> > +	ret = waitpid(pid, &status,  0);
> > +	if (ret == -1)
> > +		goto out;
> > +
> > +	if (WIFEXITED(status) && !WEXITSTATUS(status))
> > +		/*
> > +		 * The child process exited and was able to send the answer.
> > +		 * Nothing more to do here.
> > +		 */
> > +		return 0;
> > +
> > +	/*
> > +	 * The child process was not able to send an answer. Tell
> > +	 * the RPC client that something did not work as expected.
> > +	 */
> > +out:
> > +	resp.type = msg->type;
> > +	resp.success = false;
> > +
> > +	return send_criu_msg(sk, &resp);
> > +}
> > +
> > +/*
> >   * Generic function to handle CRIU_REQ_TYPE__FEATURE_CHECK.
> >   *
> >   * The function will have resp.success = true for most cases
> > @@ -1004,6 +1070,9 @@ more:
> >  	case CRIU_REQ_TYPE__FEATURE_CHECK:
> >  		ret = handle_feature_check(sk, msg);
> >  		break;
> > +	case CRIU_REQ_TYPE__VERSION:
> > +		ret = handle_version(sk, msg);
> > +		break;
> >  
> >  	default:
> >  		send_criu_err(sk, "Invalid req");
> > diff --git a/images/rpc.proto b/images/rpc.proto
> > index f894ae1..bfb8dfc 100644
> > --- a/images/rpc.proto
> > +++ b/images/rpc.proto
> > @@ -140,6 +140,8 @@ enum criu_req_type {
> >  	CPUINFO_CHECK	= 8;
> >  
> >  	FEATURE_CHECK	= 9;
> > +
> > +	VERSION		= 10;
> >  }
> >  
> >  /*
> > @@ -193,4 +195,16 @@ message criu_resp {
> >  	optional int32			cr_errno	= 7;
> >  	optional criu_features		features	= 8;
> >  	optional string			cr_errmsg	= 9;
> > +	optional criu_version		version		= 10;
> > +}
> > +
> > +/* Answer for criu_req_type.VERSION requests */
> > +message criu_version {
> > +	required int32			major		= 1;
> > +	required int32			minor		= 2;
> > +	/* if gitid is '0' then it is not from a git checkout */
> > +	required string			gitid		= 3;
> 
> Maybe it should be optional? ^^^^^

It is always set by the Makefile. sublevel and extra have a ifneq but
the gitid is always specified. It is either the string '0' or the
actual gitid. So, yes it could be optional but I tried to follow what
the Makefile gives me.


> > +	optional int32			sublevel	= 4;
> > +	optional int32			extra		= 5;
> > +	optional string			name		= 6;
> >  }
> > -- 
> > 2.9.3

		Adrian
Andrey Vagin April 5, 2017, 6:40 a.m.
On Tue, Apr 04, 2017 at 11:04:01AM +0200, Adrian Reber wrote:
> On Fri, Mar 31, 2017 at 02:39:42PM -0700, Andrei Vagin wrote:
> > On Wed, Mar 15, 2017 at 03:26:17PM +0100, Adrian Reber wrote:
> > > From: Adrian Reber <areber@redhat.com>
> > > 
> > > Instead of parsing the output of 'criu -V' this offers a RPC interface
> > > to get CRIU's version. In a follow up patch a test script is included
> > > to use the new interface:
> > > 
> > >  ./version.py
> > >  Connecting to CRIU in swrk mode to check the version:
> > >  RPC: Success
> > >  CRIU major 2
> > >  CRIU minor 12
> > >  CRIU gitid v2.12-635-g6d3ae4d
> > > 
> > > This change exports the following version fields:
> > >  * major
> > >  * minor
> > >  * gitid
> > >  * sublevel (optional)
> > >  * extra (optional)
> > >  * name (optional)
> > > 
> > > A 'gitid' of '0' (the '0' is a string) means that it is not built from a
> > > git checkout.
> > > 
> > > Signed-off-by: Adrian Reber <areber@redhat.com>
> > > ---
> > >  criu/cr-service.c | 69 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
> > >  images/rpc.proto  | 14 +++++++++++
> > >  2 files changed, 83 insertions(+)
> > > 
> > > diff --git a/criu/cr-service.c b/criu/cr-service.c
> > > index 78142b0..7e17199 100644
> > > --- a/criu/cr-service.c
> > > +++ b/criu/cr-service.c
> > > @@ -15,6 +15,7 @@
> > >  #include <arpa/inet.h>
> > >  #include <sched.h>
> > >  
> > > +#include "version.h"
> > >  #include "crtools.h"
> > >  #include "cr_options.h"
> > >  #include "external.h"
> > > @@ -806,11 +807,76 @@ static int chk_keepopen_req(CriuReq *msg)
> > >  		return 0;
> > >  	else if (msg->type == CRIU_REQ_TYPE__FEATURE_CHECK)
> > >  		return 0;
> > > +	else if (msg->type == CRIU_REQ_TYPE__VERSION)
> > > +		return 0;
> > >  
> > >  	return -1;
> > >  }
> > >  
> > >  /*
> > > + * Return the version information, depending on the information
> > > + * available in version.h
> > > + */
> > > +static int handle_version(int sk, CriuReq * msg)
> > > +{
> > > +	CriuResp resp = CRIU_RESP__INIT;
> > > +	CriuVersion version = CRIU_VERSION__INIT;
> > > +	int pid, status;
> > > +	int ret;
> > > +
> > > +
> > > +	pid = fork();
> > 
> > Why do we need a new process here?
> 
> Good question ;-) This was one of the points were I was unsure about the
> right solution. Pavel mentioned, that the fork is used for RPC functions
> which might exit cleanly. The main problem was that it is not clear how
> protobuf-c handles strings. Further down I am strdup()-ing the version
> strings into the protobuf message. The protobuf-c documentation
> regarding string handling says: TODO
> 
> https://github.com/protobuf-c/protobuf-c/wiki/Examples#strings
> 
> So I don't know if strdup() is the right tool for protobuf-c. It works,
> but it leaves strings allocated which I don't know when or where to
> free. Therefore I am going the easy way. fork() strdup() forget.
> 
> Do you know what the correct way to handle strings with protobuf-c is?
> Then I could remove the fork().

size_t criu_resp__pack
                     (const CriuResp *message,
                      uint8_t       *out)

here we can sett that the origin message is marked as const, so protobuf
is not going to change it.

I think we can set these strings without xstrdup and we don't need an
extra fork().

> 
> > > +	if (pid < 0) {
> > > +		pr_perror("Can't fork");
> > > +		goto out;
> > > +	}
> > > +
> > > +	if (pid == 0) {
> > > +		/* This assumes we will always have a major and minor version */
> > > +		version.major = CRIU_VERSION_MAJOR;
> > > +		version.minor = CRIU_VERSION_MINOR;
> > > +		version.gitid = xstrdup(CRIU_GITID);
> > > +#ifdef CRIU_VERSION_SUBLEVEL
> > > +		version.has_sublevel = 1;
> > > +		version.sublevel = CRIU_VERSION_SUBLEVEL;
> > > +#endif
> > > +#ifdef CRIU_VERSION_EXTRA
> > > +		version.has_extra = 1;
> > > +		version.extra = CRIU_VERSION_EXTRA;
> > > +#endif
> > > +#ifdef CRIU_VERSION_NAME
> > > +		/* This is not actually exported in version.h */
> > > +		version.name = xstrdup(CRIU_VERSION_NAME);
> > > +#endif
> > > +		resp.type = msg->type;
> > > +		resp.success = true;
> > > +		resp.version = &version;
> > > +		return send_criu_msg(sk, &resp);
> > > +	}
> > > +
> > > +	ret = waitpid(pid, &status,  0);
> > > +	if (ret == -1)
> > > +		goto out;
> > > +
> > > +	if (WIFEXITED(status) && !WEXITSTATUS(status))
> > > +		/*
> > > +		 * The child process exited and was able to send the answer.
> > > +		 * Nothing more to do here.
> > > +		 */
> > > +		return 0;
> > > +
> > > +	/*
> > > +	 * The child process was not able to send an answer. Tell
> > > +	 * the RPC client that something did not work as expected.
> > > +	 */
> > > +out:
> > > +	resp.type = msg->type;
> > > +	resp.success = false;
> > > +
> > > +	return send_criu_msg(sk, &resp);
> > > +}
> > > +
> > > +/*
> > >   * Generic function to handle CRIU_REQ_TYPE__FEATURE_CHECK.
> > >   *
> > >   * The function will have resp.success = true for most cases
> > > @@ -1004,6 +1070,9 @@ more:
> > >  	case CRIU_REQ_TYPE__FEATURE_CHECK:
> > >  		ret = handle_feature_check(sk, msg);
> > >  		break;
> > > +	case CRIU_REQ_TYPE__VERSION:
> > > +		ret = handle_version(sk, msg);
> > > +		break;
> > >  
> > >  	default:
> > >  		send_criu_err(sk, "Invalid req");
> > > diff --git a/images/rpc.proto b/images/rpc.proto
> > > index f894ae1..bfb8dfc 100644
> > > --- a/images/rpc.proto
> > > +++ b/images/rpc.proto
> > > @@ -140,6 +140,8 @@ enum criu_req_type {
> > >  	CPUINFO_CHECK	= 8;
> > >  
> > >  	FEATURE_CHECK	= 9;
> > > +
> > > +	VERSION		= 10;
> > >  }
> > >  
> > >  /*
> > > @@ -193,4 +195,16 @@ message criu_resp {
> > >  	optional int32			cr_errno	= 7;
> > >  	optional criu_features		features	= 8;
> > >  	optional string			cr_errmsg	= 9;
> > > +	optional criu_version		version		= 10;
> > > +}
> > > +
> > > +/* Answer for criu_req_type.VERSION requests */
> > > +message criu_version {
> > > +	required int32			major		= 1;
> > > +	required int32			minor		= 2;
> > > +	/* if gitid is '0' then it is not from a git checkout */
> > > +	required string			gitid		= 3;
> > 
> > Maybe it should be optional? ^^^^^
> 
> It is always set by the Makefile. sublevel and extra have a ifneq but
> the gitid is always specified. It is either the string '0' or the
> actual gitid. So, yes it could be optional but I tried to follow what
> the Makefile gives me.

I think we can make it optional, because criu -V prints gitid only if it is
set.
                case 'V':
                        pr_msg("Version: %s\n", CRIU_VERSION);
                        if (strcmp(CRIU_GITID, "0"))
                                pr_msg("GitID: %s\n", CRIU_GITID);
                        return 0;


> 
> 
> > > +	optional int32			sublevel	= 4;
> > > +	optional int32			extra		= 5;
> > > +	optional string			name		= 6;
> > >  }
> > > -- 
> > > 2.9.3
> 
> 		Adrian