| Message ID | 1360153939-9563-15-git-send-email-benoit@irqsave.net |
|---|---|
| State | New |
| Headers | show |
On 02/06/2013 05:32 AM, Benoît Canet wrote: > --- > block/qcow2-dedup.c | 11 +++++++++++ > block/qcow2.h | 18 ++++++++++++++++++ > 2 files changed, 29 insertions(+) > > diff --git a/block/qcow2-dedup.c b/block/qcow2-dedup.c > index 308b4f6..72ce1d7 100644 > --- a/block/qcow2-dedup.c > +++ b/block/qcow2-dedup.c > @@ -1311,3 +1311,14 @@ void qcow2_dedup_close(BlockDriverState *bs) > { > qcow2_dedup_free(bs); > } > + > +void qcow2_dedup_update_metrics(BlockDriverState *bs) > +{ > + BDRVQcowState *s = bs->opaque; > + > + uint64_t nb_hashs = s->dedup_metrics.ram_hash_creations - > + s->dedup_metrics.ram_hash_deletions; > + > + s->dedup_metrics.ram_usage = nb_hashs * sizeof(GTreeNode_Copy) * 2; > + s->dedup_metrics.ram_usage += nb_hashs * sizeof(QCowHashNode); > +} > diff --git a/block/qcow2.h b/block/qcow2.h > index fc393d5..191b272 100644 > --- a/block/qcow2.h > +++ b/block/qcow2.h > @@ -76,6 +76,23 @@ > #define QCOW_STRATEGY_DISK (1 << 1) > #define QCOW_STRATEGY_RUNNING (1 << 2) > > +/* This is an internal structure of glib gtree.c > + * copy it here so we can compute it's size > + * from glib/gtree.c in glib sources > + */ > +typedef struct _GTreeNode_Copy GTreeNode_Copy; > + > +struct _GTreeNode_Copy > +{ > + gpointer key; /* key for this node */ > + gpointer value; /* value stored at this node */ > + GTreeNode_Copy *left; /* left subtree */ > + GTreeNode_Copy *right; /* right subtree */ > + gint8 balance; /* height (right) - height (left) */ > + guint8 left_child; > + guint8 right_child; > +}; This doesn't belong in the .h file; stick it in the .c since that is the only place where you use it. It still feels kind of risky to be copying this opaque struct - what if a newer version of glib changes the struct size? Do you have any way to ensure that your copy will remain in sync with the actual struct used by the version of glib that you are linked with? Would it be better to propose that upstream glib add a new function for getting memory usage of an opaque hash object, and that qemu merely omit the memory usage statistic when linked with an older version of glib that does not provide the new accessor?
diff --git a/block/qcow2-dedup.c b/block/qcow2-dedup.c index 308b4f6..72ce1d7 100644 --- a/block/qcow2-dedup.c +++ b/block/qcow2-dedup.c @@ -1311,3 +1311,14 @@ void qcow2_dedup_close(BlockDriverState *bs) { qcow2_dedup_free(bs); } + +void qcow2_dedup_update_metrics(BlockDriverState *bs) +{ + BDRVQcowState *s = bs->opaque; + + uint64_t nb_hashs = s->dedup_metrics.ram_hash_creations - + s->dedup_metrics.ram_hash_deletions; + + s->dedup_metrics.ram_usage = nb_hashs * sizeof(GTreeNode_Copy) * 2; + s->dedup_metrics.ram_usage += nb_hashs * sizeof(QCowHashNode); +} diff --git a/block/qcow2.h b/block/qcow2.h index fc393d5..191b272 100644 --- a/block/qcow2.h +++ b/block/qcow2.h @@ -76,6 +76,23 @@ #define QCOW_STRATEGY_DISK (1 << 1) #define QCOW_STRATEGY_RUNNING (1 << 2) +/* This is an internal structure of glib gtree.c + * copy it here so we can compute it's size + * from glib/gtree.c in glib sources + */ +typedef struct _GTreeNode_Copy GTreeNode_Copy; + +struct _GTreeNode_Copy +{ + gpointer key; /* key for this node */ + gpointer value; /* value stored at this node */ + GTreeNode_Copy *left; /* left subtree */ + GTreeNode_Copy *right; /* right subtree */ + gint8 balance; /* height (right) - height (left) */ + guint8 left_child; + guint8 right_child; +}; + typedef enum { QCOW_HASH_SHA256 = 0, QCOW_HASH_SHA3 = 1, @@ -507,5 +524,6 @@ void qcow2_dedup_refcount_half_max_reached(BlockDriverState *bs, bool qcow2_dedup_is_running(BlockDriverState *bs); int qcow2_dedup_init(BlockDriverState *bs); void qcow2_dedup_close(BlockDriverState *bs); +void qcow2_dedup_update_metrics(BlockDriverState *bs); #endif