qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Kevin Wolf <kwolf@redhat.com>
To: Eric Blake <eblake@redhat.com>
Cc: qemu-block@nongnu.org, Alberto Garcia <berto@igalia.com>,
	qemu-devel@nongnu.org, Stefan Hajnoczi <stefanha@redhat.com>,
	Max Reitz <mreitz@redhat.com>
Subject: Re: [Qemu-devel] [PATCH 2/3] qcow2: add option to clean unused cache entries after some time
Date: Thu, 28 May 2015 18:44:55 +0200	[thread overview]
Message-ID: <20150528164455.GF3385@noname.redhat.com> (raw)
In-Reply-To: <556730A1.40705@redhat.com>

[-- Attachment #1: Type: text/plain, Size: 3400 bytes --]

Am 28.05.2015 um 17:13 hat Eric Blake geschrieben:
> On 05/28/2015 08:56 AM, Max Reitz wrote:
> > On 27.05.2015 11:46, Alberto Garcia wrote:
> >> This adds a new 'cache-clean-interval' option that cleans all qcow2
> >> cache entries that haven't been used in a certain interval, given in
> >> seconds.
> >>
> >> This allows setting a large L2 cache size so it can handle scenarios
> >> with lots of I/O and at the same time use little memory during periods
> >> of inactivity.
> >>
> >> This feature currently relies on MADV_DONTNEED to free that memory, so
> >> it is not useful in systems that don't follow that behavior.
> >>
> 
> >> +++ b/qapi/block-core.json
> >> @@ -41,6 +41,10 @@
> >>   # @corrupt: #optional true if the image has been marked corrupt;
> >> only valid for
> >>   #           compat >= 1.1 (since 2.2)
> >>   #
> >> +# @cache-clean-interval: interval in seconds after which unused L2 and
> >> +#                        refcount cache entries are removed. If 0 then
> >> +#                        this feature is not enabled (since 2.4)
> >> +#
> >>   # @refcount-bits: width of a refcount entry in bits (since 2.3)
> >>   #
> >>   # Since: 1.7
> >> @@ -50,7 +54,8 @@
> >>         'compat': 'str',
> >>         '*lazy-refcounts': 'bool',
> >>         '*corrupt': 'bool',
> >> -      'refcount-bits': 'int'
> >> +      'refcount-bits': 'int',
> >> +      'cache-clean-interval': 'int'
> >>     } }
> > 
> > I'm not too happy about making this part of ImageInfoSpecificQCow2. Two
> > reasons for this: First, it's eventually part of ImageInfo, which is
> > defined as "Information about a QEMU image file", but this option cannot
> > be set in the image file itself but is only a run-time option.
> > 
> > Second, my original goal for ImageInfoSpecific was to provide more
> > information through qemu-img info, and this value will look pretty
> > strange there.
> > 
> > I don't know how to resolve this quandary, though. Since qemu cannot
> > change this interval by itself, I think not providing an interface for
> > reading it is fine. On the other hand, if Eric finds such an interface
> > absolutely mandatory, I can't think of a better place to return it than
> > here.
> 
> Can we mark the parameter optional, and only provide it when it is
> non-zero?  That way, qemu-img (which cannot set an interval) will not
> report it, and the only time it will appear is if it was set as part of
> opening the block device under qemu.

No, that wouldn't be right. It's not information tied to the image file.

> > The only solution which would satisfy both requirements would be another
> > structure which contains "online" flags, and thus is not evaluated by
> > qemu-img info, but only by QMP commands. But that's ugly.
> > 
> 
> Yeah, I'm not sure such duplication helps.  I'd still like it reported
> somewhere, though, as it is nice to query that a requested setting is
> actually working.

This isn't duplicated information. You can have ImageInfoSpecificQCow2
show lazy_refcounts=off because that is what the image file contains,
but the real value in effect could be lazy_refcounts=on.

Options stored in the image and runtime options are two different pieces
of information, even if the former specify the defaults for the latter.
So I think it should be possible to query both of them.

Kevin

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

  parent reply	other threads:[~2015-05-28 16:45 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-05-27  9:46 [Qemu-devel] [PATCH v3 0/3] Clean unused entries in the qcow2 L2/refcount cache Alberto Garcia
2015-05-27  9:46 ` [Qemu-devel] [PATCH 1/3] qcow2: mark the memory as no longer needed after qcow2_cache_empty() Alberto Garcia
2015-05-27  9:46 ` [Qemu-devel] [PATCH 2/3] qcow2: add option to clean unused cache entries after some time Alberto Garcia
2015-05-27 12:27   ` Eric Blake
2015-05-27 14:34     ` Alberto Garcia
2015-05-28 14:47     ` Max Reitz
2015-05-28 14:56   ` Max Reitz
2015-05-28 15:02     ` Alberto Garcia
2015-05-28 15:04       ` Max Reitz
2015-05-28 15:06         ` Max Reitz
2015-05-28 15:13     ` Eric Blake
2015-05-28 15:14       ` Max Reitz
2015-05-28 15:19         ` Alberto Garcia
2015-05-28 15:23           ` Max Reitz
2015-05-28 15:30             ` Alberto Garcia
2015-05-28 19:41             ` Eric Blake
2015-05-29  8:32               ` Kevin Wolf
2015-05-28 16:44       ` Kevin Wolf [this message]
2015-05-28 17:03         ` Alberto Garcia
2015-05-27  9:46 ` [Qemu-devel] [PATCH 3/3] qcow2: reorder fields in Qcow2CachedTable to reduce padding Alberto Garcia
  -- strict thread matches above, loose matches on Subject: below --
2015-05-29  9:24 [Qemu-devel] [PATCH v4 0/3] Clean unused entries in the qcow2 L2/refcount cache Alberto Garcia
2015-05-29  9:24 ` [Qemu-devel] [PATCH 2/3] qcow2: add option to clean unused cache entries after some time Alberto Garcia
2015-06-02 11:04   ` Kevin Wolf
2015-05-26 17:14 [Qemu-devel] [PATCH v2 0/3] Clean unused entries in the qcow2 L2/refcount cache Alberto Garcia
2015-05-26 17:14 ` [Qemu-devel] [PATCH 2/3] qcow2: add option to clean unused cache entries after some time Alberto Garcia
2015-05-26 17:26   ` Max Reitz
2015-05-26 18:52   ` Eric Blake
2015-05-26 19:10     ` Alberto Garcia
2015-05-26 19:15       ` Eric Blake
2015-05-18 16:48 [Qemu-devel] [PATCH 0/3] Clean unused entries in the qcow2 L2/refcount cache Alberto Garcia
2015-05-18 16:48 ` [Qemu-devel] [PATCH 2/3] qcow2: add option to clean unused cache entries after some time Alberto Garcia
2015-05-26 16:07   ` Max Reitz

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20150528164455.GF3385@noname.redhat.com \
    --to=kwolf@redhat.com \
    --cc=berto@igalia.com \
    --cc=eblake@redhat.com \
    --cc=mreitz@redhat.com \
    --cc=qemu-block@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    --cc=stefanha@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).