kdat: Don't open /run/criu.kdat if doesn't exists

Submitted by Radostin Stoyanov on July 1, 2018, 6:13 p.m.

Details

Message ID 20180701181358.18985-1-rstoyanov1@gmail.com
State New
Series "kdat: Don't open /run/criu.kdat if doesn't exists"
Headers show

Commit Message

Radostin Stoyanov July 1, 2018, 6:13 p.m.
When CRIU is called for a first time and the /run/criu.kdat file does
not exists, the following warning is shown:
        Warn  (criu/kerndat.c:847): Can't load /run/criu.kdat

This patch is replacing this warning with a more appropriate debug message.
        File /run/criu.kdat does not exist

Signed-off-by: Radostin Stoyanov <rstoyanov1@gmail.com>
---
 criu/kerndat.c | 5 +++++
 1 file changed, 5 insertions(+)

Patch hide | download patch | download mbox

diff --git a/criu/kerndat.c b/criu/kerndat.c
index 9b93487c..57a97886 100644
--- a/criu/kerndat.c
+++ b/criu/kerndat.c
@@ -842,6 +842,11 @@  static int kerndat_try_load_cache(void)
 {
 	int fd, ret;
 
+	if(access(KERNDAT_CACHE_FILE, F_OK) == -1) {
+		pr_debug("File %s does not exist\n", KERNDAT_CACHE_FILE);
+		return 1;
+	}
+
 	fd = open(KERNDAT_CACHE_FILE, O_RDONLY);
 	if (fd < 0) {
 		pr_warn("Can't load %s\n", KERNDAT_CACHE_FILE);

Comments

Adrian Reber July 2, 2018, 8:56 a.m.
Sounds like a good idea. When testing I am also sometimes surprised by
that warning.

Acked-by: Adrian Reber <areber@redhat.com>

On Sun, Jul 01, 2018 at 07:13:58PM +0100, Radostin Stoyanov wrote:
> When CRIU is called for a first time and the /run/criu.kdat file does
> not exists, the following warning is shown:
>         Warn  (criu/kerndat.c:847): Can't load /run/criu.kdat
> 
> This patch is replacing this warning with a more appropriate debug message.
>         File /run/criu.kdat does not exist
> 
> Signed-off-by: Radostin Stoyanov <rstoyanov1@gmail.com>
> ---
>  criu/kerndat.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/criu/kerndat.c b/criu/kerndat.c
> index 9b93487c..57a97886 100644
> --- a/criu/kerndat.c
> +++ b/criu/kerndat.c
> @@ -842,6 +842,11 @@ static int kerndat_try_load_cache(void)
>  {
>  	int fd, ret;
>  
> +	if(access(KERNDAT_CACHE_FILE, F_OK) == -1) {
> +		pr_debug("File %s does not exist\n", KERNDAT_CACHE_FILE);
> +		return 1;
> +	}
> +
>  	fd = open(KERNDAT_CACHE_FILE, O_RDONLY);
>  	if (fd < 0) {
>  		pr_warn("Can't load %s\n", KERNDAT_CACHE_FILE);
> -- 
> 2.17.1
> 
> _______________________________________________
> CRIU mailing list
> CRIU@openvz.org
> https://lists.openvz.org/mailman/listinfo/criu
Dmitry Safonov July 2, 2018, 12:53 p.m.
2018-07-01 19:13 GMT+01:00 Radostin Stoyanov <rstoyanov1@gmail.com>:
> When CRIU is called for a first time and the /run/criu.kdat file does
> not exists, the following warning is shown:
>         Warn  (criu/kerndat.c:847): Can't load /run/criu.kdat
>
> This patch is replacing this warning with a more appropriate debug message.
>         File /run/criu.kdat does not exist
>
> Signed-off-by: Radostin Stoyanov <rstoyanov1@gmail.com>
> ---
>  criu/kerndat.c | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/criu/kerndat.c b/criu/kerndat.c
> index 9b93487c..57a97886 100644
> --- a/criu/kerndat.c
> +++ b/criu/kerndat.c
> @@ -842,6 +842,11 @@ static int kerndat_try_load_cache(void)
>  {
>         int fd, ret;
>
> +       if(access(KERNDAT_CACHE_FILE, F_OK) == -1) {
> +               pr_debug("File %s does not exist\n", KERNDAT_CACHE_FILE);
> +               return 1;
> +       }
> +
>         fd = open(KERNDAT_CACHE_FILE, O_RDONLY);
>         if (fd < 0) {
>                 pr_warn("Can't load %s\n", KERNDAT_CACHE_FILE);

I don't quite mind, but why do you need access() syscall?
Will checking (ENOENT == fd) work here?

It's not critical, but it's another syscall we'll be doing each time criu
is launched, so maybe makes sense not adding more calls there,
until necessary?

Thanks,
             Dmitry
Andrei Vagin July 11, 2018, 12:01 a.m.
On Mon, Jul 02, 2018 at 01:53:22PM +0100, Dmitry Safonov wrote:
> 2018-07-01 19:13 GMT+01:00 Radostin Stoyanov <rstoyanov1@gmail.com>:
> > When CRIU is called for a first time and the /run/criu.kdat file does
> > not exists, the following warning is shown:
> >         Warn  (criu/kerndat.c:847): Can't load /run/criu.kdat
> >
> > This patch is replacing this warning with a more appropriate debug message.
> >         File /run/criu.kdat does not exist
> >
> > Signed-off-by: Radostin Stoyanov <rstoyanov1@gmail.com>
> > ---
> >  criu/kerndat.c | 5 +++++
> >  1 file changed, 5 insertions(+)
> >
> > diff --git a/criu/kerndat.c b/criu/kerndat.c
> > index 9b93487c..57a97886 100644
> > --- a/criu/kerndat.c
> > +++ b/criu/kerndat.c
> > @@ -842,6 +842,11 @@ static int kerndat_try_load_cache(void)
> >  {
> >         int fd, ret;
> >
> > +       if(access(KERNDAT_CACHE_FILE, F_OK) == -1) {
> > +               pr_debug("File %s does not exist\n", KERNDAT_CACHE_FILE);
> > +               return 1;
> > +       }
> > +
> >         fd = open(KERNDAT_CACHE_FILE, O_RDONLY);
> >         if (fd < 0) {
> >                 pr_warn("Can't load %s\n", KERNDAT_CACHE_FILE);
> 
> I don't quite mind, but why do you need access() syscall?
> Will checking (ENOENT == fd) work here?
> 
> It's not critical, but it's another syscall we'll be doing each time criu
> is launched, so maybe makes sense not adding more calls there,
> until necessary?

+1

> 
> Thanks,
>              Dmitry
> _______________________________________________
> CRIU mailing list
> CRIU@openvz.org
> https://lists.openvz.org/mailman/listinfo/criu