Re: [Qemu-block] [PATCH v9 8/9] qcow2: Set the default cache-clean-interval to 10 minutes

[Leonid Bloch]

On 9/21/18 3:35 PM, Alberto Garcia wrote:
> On Tue 18 Sep 2018 05:29:22 PM CEST, Leonid Bloch wrote:
> >       /* New interval for cache cleanup timer */
> >       r->cache_clean_interval =
> >           qemu_opt_get_number(opts, QCOW2_OPT_CACHE_CLEAN_INTERVAL,
> > -                            s->cache_clean_interval);
> > +                            DEFAULT_CACHE_CLEAN_INTERVAL);
>
> I just realized we're ignoring the old value (s->cache_clean_interval)
> here. What's the consequence of this? (this was a change made by Kevin
> Wolf in 5b0959a7d432062dcd740f8065004285b15695fa).
>
> >   #ifndef CONFIG_LINUX
> >       if (r->cache_clean_interval != 0) {
> >           error_setg(errp, QCOW2_OPT_CACHE_CLEAN_INTERVAL
> >                      " not supported on this host");
>
> Another thing that I noticed (see below)...
>
> > diff --git a/block/qcow2.h b/block/qcow2.h
> > index 0f0e3534bf..8c863897b0 100644
> > --- a/block/qcow2.h
> > +++ b/block/qcow2.h
> > @@ -82,6 +82,7 @@
> >   
> >   #define DEFAULT_CLUSTER_SIZE S_64KiB
> >   
> > +#define DEFAULT_CACHE_CLEAN_INTERVAL 600  /* seconds */
>
> Shouldn't this be set to 0 in non-Linux platforms? Otherwise it will try
> to set it to 600 by default in all platforms and will complain with the
> "not supported on this host" error message that I quoted above.

You're right! Will fix, thanks.

Leonid.

> Berto