All of lore.kernel.org
 help / color / mirror / Atom feed
From: Eric Blake <eblake@redhat.com>
To: "Benoît Canet" <benoit.canet@irqsave.net>
Cc: kwolf@redhat.com, pbonzini@redhat.com, qemu-devel@nongnu.org,
	stefanha@redhat.com
Subject: Re: [Qemu-devel] [RFC V1 12/14] qcow2: Add qcow2_dedup_update_metrics to compute dedup RAM usage.
Date: Thu, 17 Jan 2013 08:40:19 -0700	[thread overview]
Message-ID: <50F81B63.2050009@redhat.com> (raw)
In-Reply-To: <20130117111159.GA3448@irqsave.net>

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

On 01/17/2013 04:11 AM, Benoît Canet wrote:
> Le Wednesday 16 Jan 2013 à 13:10:12 (-0700), Eric Blake a écrit :
>> On 01/16/2013 09:25 AM, Benoît Canet wrote:
>>> ---
>>>  block/qcow2-dedup.c |   13 +++++++++++++
>>>  block/qcow2.h       |    1 +
>>>  2 files changed, 14 insertions(+)
>>>
>>> diff --git a/block/qcow2-dedup.c b/block/qcow2-dedup.c
>>> index db23b71..4305746 100644
>>> --- a/block/qcow2-dedup.c
>>> +++ b/block/qcow2-dedup.c
>>> @@ -1311,3 +1311,16 @@ void qcow2_dedup_close(BlockDriverState *bs)
>>>  {
>>>      qcow2_dedup_free(bs);
>>>  }
>>> +
>>> +#define GTREE_NODE_SIZE sizeof(int) * 5
>>
>> Improperly parenthesized.  Also, this feels like a magic number, is
>> there an actual sizeof(struct) you could use instead of hand-computing
>> how much is used per node?
> 
> No the glib implementation totally hide it's structures.

Also, are you sure that this value is correct on both 32-bit and 64-bit
machines, or does a gtree node include a void* which changes how much
memory it uses based on platform?  Even adding a comment pointing to the
internal glib implementation that you were copying from would be
helpful, so that it doesn't feel quite so magic.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 621 bytes --]

  reply	other threads:[~2013-01-17 15:40 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-01-16 16:24 [Qemu-devel] [RFC V1 00/14] QCOW2 deduplication metrics Benoît Canet
2013-01-16 16:24 ` [Qemu-devel] [RFC V1 01/14] qcow2: Add deduplication metrics structures Benoît Canet
2013-01-16 16:24 ` [Qemu-devel] [RFC V1 02/14] qcow2: Initialize deduplication metrics Benoît Canet
2013-01-16 16:24 ` [Qemu-devel] [RFC V1 03/14] qcow2: Collect unaligned writes missing data reads metric Benoît Canet
2013-01-16 16:24 ` [Qemu-devel] [RFC V1 04/14] qcow2: Collect deduplicated cluster metric Benoît Canet
2013-01-16 16:24 ` [Qemu-devel] [RFC V1 05/14] qcow2: Collect undeduplicated " Benoît Canet
2013-01-16 16:25 ` [Qemu-devel] [RFC V1 06/14] qcow2: Count QCowHashNode creation metrics Benoît Canet
2013-01-16 16:25 ` [Qemu-devel] [RFC V1 07/14] qcow2: Count QCowHashNode removal from tree for metrics Benoît Canet
2013-01-16 16:25 ` [Qemu-devel] [RFC V1 08/14] qcow2: Count cluster deleted metric Benoît Canet
2013-01-16 16:25 ` [Qemu-devel] [RFC V1 09/14] qcow2: Count deduplication refcount overflow metric Benoît Canet
2013-01-16 16:25 ` [Qemu-devel] [RFC V1 10/14] qapi: Add support for deduplication infos in qapi-schema.json Benoît Canet
2013-01-16 19:35   ` Eric Blake
2013-01-17 12:15   ` Benoît Canet
2013-01-17 15:38     ` Eric Blake
2013-01-16 16:25 ` [Qemu-devel] [RFC V1 11/14] block: Add deduplication metrics to BlockDriverInfo Benoît Canet
2013-01-16 16:25 ` [Qemu-devel] [RFC V1 12/14] qcow2: Add qcow2_dedup_update_metrics to compute dedup RAM usage Benoît Canet
2013-01-16 20:10   ` Eric Blake
2013-01-17 11:11     ` Benoît Canet
2013-01-17 15:40       ` Eric Blake [this message]
2013-01-16 16:25 ` [Qemu-devel] [RFC V1 13/14] qcow2: returns deduplication metrics and status via bdrv_get_info() Benoît Canet
2013-01-16 16:25 ` [Qemu-devel] [RFC V1 14/14] qapi: Return virtual block device deduplication metrics in QMP Benoît Canet

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=50F81B63.2050009@redhat.com \
    --to=eblake@redhat.com \
    --cc=benoit.canet@irqsave.net \
    --cc=kwolf@redhat.com \
    --cc=pbonzini@redhat.com \
    --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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.