10:29 a.m.
This is a follow up to:
https://www.redhat.com/archives/libvir-list/2017-March/msg00650.html
so that management apps can query the current value. The series needs to be
applied on top of the block threshold series. For convenience I've pushed it
to the same remote repository as the original series. You can fetch it here:
git fetch git://pipo.sk/pipo/libvirt.git block-threshold-1
Peter Krempa (3):
util: json: Make function to free JSON values in virHash universal
qemu: block: Add code to fetch block node data by node name
qemu: stats: Display the block threshold size in bulk stats
src/libvirt-domain.c | 4 ++++
src/libvirt_private.syms | 1 +
src/qemu/qemu_block.c | 46 +++++++++++++++++++++++++++++++++++++++
src/qemu/qemu_block.h | 3 +++
src/qemu/qemu_driver.c | 51 +++++++++++++++++++++++++++++++++++++++++---
src/qemu/qemu_monitor_json.c | 10 +--------
src/util/virjson.c | 8 +++++++
src/util/virjson.h | 1 +
tools/virsh.pod | 3 +++
9 files changed, 115 insertions(+), 12 deletions(-)
--
2.12.0
10:29 a.m.
New subject: [libvirt] [PATCH 1/3] util: json: Make function to free JSON values in virHash universal
Move the helper that frees JSON entries put into hash tables into the
JSON module so that it does not have to be reimplemented.
---
src/libvirt_private.syms | 1 +
src/qemu/qemu_monitor_json.c | 10 +---------
src/util/virjson.c | 8 ++++++++
src/util/virjson.h | 1 +
4 files changed, 11 insertions(+), 9 deletions(-)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index db0e11e33..84619f01f 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1832,6 +1832,7 @@ virJSONValueGetNumberLong;
virJSONValueGetNumberUint;
virJSONValueGetNumberUlong;
virJSONValueGetString;
+virJSONValueHashFree;
virJSONValueIsArray;
virJSONValueIsNull;
virJSONValueNewArray;
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index 75af5cb93..e6b73d340 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -7449,14 +7449,6 @@ qemuMonitorJSONFillQMPSchema(size_t pos ATTRIBUTE_UNUSED,
}
-static void
-qemuMonitorJSONFreeSchemaEntry(void *opaque,
- const void *name ATTRIBUTE_UNUSED)
-{
- virJSONValueFree(opaque);
-}
-
-
virHashTablePtr
qemuMonitorJSONQueryQMPSchema(qemuMonitorPtr mon)
{
@@ -7477,7 +7469,7 @@ qemuMonitorJSONQueryQMPSchema(qemuMonitorPtr mon)
arr = virJSONValueObjectGet(reply, "return");
- if (!(schema = virHashCreate(512, qemuMonitorJSONFreeSchemaEntry)))
+ if (!(schema = virHashCreate(512, virJSONValueHashFree)))
goto cleanup;
if (virJSONValueArrayForeachSteal(arr, qemuMonitorJSONFillQMPSchema,
diff --git a/src/util/virjson.c b/src/util/virjson.c
index c907b5ded..b49b29b0f 100644
--- a/src/util/virjson.c
+++ b/src/util/virjson.c
@@ -376,6 +376,14 @@ virJSONValueFree(virJSONValuePtr value)
}
+void
+virJSONValueHashFree(void *opaque,
+ const void *name ATTRIBUTE_UNUSED)
+{
+ virJSONValueFree(opaque);
+}
+
+
virJSONValuePtr
virJSONValueNewString(const char *data)
{
diff --git a/src/util/virjson.h b/src/util/virjson.h
index 5e32cb9a4..14b74c061 100644
--- a/src/util/virjson.h
+++ b/src/util/virjson.h
@@ -81,6 +81,7 @@ struct _virJSONValue {
};
void virJSONValueFree(virJSONValuePtr value);
+void virJSONValueHashFree(void *opaque, const void *name);
int virJSONValueObjectCreate(virJSONValuePtr *obj, ...)
ATTRIBUTE_NONNULL(1) ATTRIBUTE_SENTINEL;
--
2.12.0
9:39 a.m.
New subject: [libvirt] [PATCH 1/3] util: json: Make function to free JSON values in virHash universal
On 03/16/2017 10:29 AM, Peter Krempa wrote:
Move the helper that frees JSON entries put into hash tables into
the
JSON module so that it does not have to be reimplemented.
---
src/libvirt_private.syms | 1 +
src/qemu/qemu_monitor_json.c | 10 +---------
src/util/virjson.c | 8 ++++++++
src/util/virjson.h | 1 +
4 files changed, 11 insertions(+), 9 deletions(-)
ACK
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
10:29 a.m.
New subject: [libvirt] [PATCH 2/3] qemu: block: Add code to fetch block node data by node name
To allow updating stats based on the node name, add a helper function
that will fetch the required data from 'query-named-block-nodes' and
return it in hash table for easy lookup.
---
src/qemu/qemu_block.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++
src/qemu/qemu_block.h | 3 +++
2 files changed, 49 insertions(+)
diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c
index e31907842..586d56809 100644
--- a/src/qemu/qemu_block.c
+++ b/src/qemu/qemu_block.c
@@ -380,3 +380,49 @@ qemuBlockNodeNamesDetect(virQEMUDriverPtr driver,
return ret;
}
+
+
+static int
+qemuBlockFillNodeData(size_t pos ATTRIBUTE_UNUSED,
+ virJSONValuePtr item,
+ void *opaque)
+{
+ virHashTablePtr table = opaque;
+ const char *name;
+
+ if (!(name = virJSONValueObjectGetString(item, "node-name")))
+ return 1;
+
+ if (virHashAddEntry(table, name, item) < 0)
+ return -1;
+
+ return 0;
+}
+
+
+/**
+ * qemuBlockGetNodeData:
+ * @data: JSON object returned from query-named-block-nodes
+ *
+ * Returns a hash table organized by the node name of the JSON value objects of
+ * data for given qemu block nodes.
+ *
+ * Returns a filled virHashTablePtr on success NULL on error.
+ */
+virHashTablePtr
+qemuBlockGetNodeData(virJSONValuePtr data)
+{
+ virHashTablePtr ret = NULL;
+
+ if (!(ret = virHashCreate(50, virJSONValueHashFree)))
+ return NULL;
+
+ if (virJSONValueArrayForeachSteal(data, qemuBlockFillNodeData, ret) < 0)
+ goto error;
+
+ return ret;
+
+ error:
+ virHashFree(ret);
+ return NULL;
+}
diff --git a/src/qemu/qemu_block.h b/src/qemu/qemu_block.h
index 56f4a74dd..9d6a24643 100644
--- a/src/qemu/qemu_block.h
+++ b/src/qemu/qemu_block.h
@@ -48,4 +48,7 @@ int
qemuBlockNodeNamesDetect(virQEMUDriverPtr driver,
virDomainObjPtr vm);
+virHashTablePtr
+qemuBlockGetNodeData(virJSONValuePtr data);
+
#endif /* __QEMU_BLOCK_H__ */
--
2.12.0
9:41 a.m.
New subject: [libvirt] [PATCH 2/3] qemu: block: Add code to fetch block node data by node name
On 03/16/2017 10:29 AM, Peter Krempa wrote:
To allow updating stats based on the node name, add a helper
function
that will fetch the required data from 'query-named-block-nodes' and
return it in hash table for easy lookup.
---
src/qemu/qemu_block.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++
src/qemu/qemu_block.h | 3 +++
2 files changed, 49 insertions(+)
ACK
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
10:29 a.m.
New subject: [libvirt] [PATCH 3/3] qemu: stats: Display the block threshold size in bulk stats
Management tools may want to check whether the threshold is still set if
they missed an event. Add the data to the bulk stats API where they can
also query the current backing size at the same time.
---
src/libvirt-domain.c | 4 ++++
src/qemu/qemu_driver.c | 51 +++++++++++++++++++++++++++++++++++++++++++++++---
tools/virsh.pod | 3 +++
3 files changed, 55 insertions(+), 3 deletions(-)
diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
index fd367bcb0..815cbb1e1 100644
--- a/src/libvirt-domain.c
+++ b/src/libvirt-domain.c
@@ -11206,6 +11206,10 @@ virConnectGetDomainCapabilities(virConnectPtr conn,
* backing image as unsigned long long.
* "block.<num>.physical" - physical size in bytes of the container
of the
* backing image as unsigned long long.
+ * "block.<num>.threshold.storage - current threshold for delivering the
+ * VIR_DOMAIN_EVENT_ID_BLOCK_THRESHOLD
+ * event in bytes.
+ * See virDomainSetBlockThreshold.
*
* VIR_DOMAIN_STATS_PERF:
* Return perf event statistics.
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 22cf866cd..a541b671d 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -19186,6 +19186,32 @@ qemuDomainGetStatsOneBlockFallback(virQEMUDriverPtr driver,
static int
+qemuDomainGetStatsOneBlockNode(virDomainStatsRecordPtr record,
+ int *maxparams,
+ virStorageSourcePtr src,
+ size_t block_idx,
+ virHashTablePtr nodedata)
+{
+ virJSONValuePtr data;
+ unsigned long long tmp;
+ int ret = -1;
+
+ if (src->nodebacking &&
+ (data = virHashLookup(nodedata, src->nodebacking))) {
+ if (virJSONValueObjectGetNumberUlong(data, "write_threshold", &tmp)
== 0 &&
+ tmp > 0)
+ QEMU_ADD_BLOCK_PARAM_ULL(record, maxparams, block_idx,
+ "threshold.storage", tmp);
+ }
+
+ ret = 0;
+
+ cleanup:
+ return ret;
+}
+
+
+static int
qemuDomainGetStatsOneBlock(virQEMUDriverPtr driver,
virQEMUDriverConfigPtr cfg,
virDomainObjPtr dom,
@@ -19195,7 +19221,8 @@ qemuDomainGetStatsOneBlock(virQEMUDriverPtr driver,
virStorageSourcePtr src,
size_t block_idx,
unsigned int backing_idx,
- virHashTablePtr stats)
+ virHashTablePtr stats,
+ virHashTablePtr nodedata)
{
qemuBlockStats *entry;
int ret = -1;
@@ -19264,6 +19291,10 @@ qemuDomainGetStatsOneBlock(virQEMUDriverPtr driver,
}
}
+ if (qemuDomainGetStatsOneBlockNode(record, maxparams, src, block_idx,
+ nodedata) < 0)
+ goto cleanup;
+
ret = 0;
cleanup:
VIR_FREE(alias);
@@ -19282,8 +19313,12 @@ qemuDomainGetStatsBlock(virQEMUDriverPtr driver,
int ret = -1;
int rc;
virHashTablePtr stats = NULL;
+ virHashTablePtr nodestats = NULL;
+ virJSONValuePtr nodedata = NULL;
qemuDomainObjPrivatePtr priv = dom->privateData;
virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
+ bool fetchnodedata = virQEMUCapsGet(priv->qemuCaps,
+ QEMU_CAPS_QUERY_NAMED_BLOCK_NODES);
int count_index = -1;
size_t visited = 0;
bool visitBacking = !!(privflags & QEMU_DOMAIN_STATS_BACKING);
@@ -19295,14 +19330,22 @@ qemuDomainGetStatsBlock(virQEMUDriverPtr driver,
if (rc >= 0)
ignore_value(qemuMonitorBlockStatsUpdateCapacity(priv->mon, stats,
visitBacking));
+
+ if (fetchnodedata)
+ nodedata = qemuMonitorQueryNamedBlockNodes(priv->mon);
+
if (qemuDomainObjExitMonitor(driver, dom) < 0)
goto cleanup;
/* failure to retrieve stats is fine at this point */
- if (rc < 0)
+ if (rc < 0 || (fetchnodedata && !nodedata))
virResetLastError();
}
+ if (nodedata &&
+ !(nodestats = qemuBlockGetNodeData(nodedata)))
+ goto cleanup;
+
/* When listing backing chains, it's easier to fix up the count
* after the iteration than it is to iterate twice; but we still
* want count listed first. */
@@ -19317,7 +19360,7 @@ qemuDomainGetStatsBlock(virQEMUDriverPtr driver,
while (src && (backing_idx == 0 || visitBacking)) {
if (qemuDomainGetStatsOneBlock(driver, cfg, dom, record, maxparams,
disk, src, visited, backing_idx,
- stats) < 0)
+ stats, nodestats) < 0)
goto cleanup;
visited++;
backing_idx++;
@@ -19330,6 +19373,8 @@ qemuDomainGetStatsBlock(virQEMUDriverPtr driver,
cleanup:
virHashFree(stats);
+ virHashFree(nodestats);
+ virJSONValueFree(nodedata);
virObjectUnref(cfg);
return ret;
}
diff --git a/tools/virsh.pod b/tools/virsh.pod
index 48be19234..c2a70f07f 100644
--- a/tools/virsh.pod
+++ b/tools/virsh.pod
@@ -1005,6 +1005,9 @@ Information listed includes:
"block.<num>.allocation" - offset of highest written sector in bytes
"block.<num>.capacity" - logical size of source file in bytes
"block.<num>.physical" - physical size of source file in bytes
+ "block.<num>.threshold.storage" - threshold (in bytes) for delivering
the
+ VIR_DOMAIN_EVENT_ID_BLOCK_THRESHOLD event
+ See domblkthreshold.
Selecting a specific statistics groups doesn't guarantee that the
daemon supports the selected group of stats. Flag I<--enforce>
--
2.12.0
9:45 a.m.
New subject: [libvirt] [PATCH 3/3] qemu: stats: Display the block threshold size in bulk stats
On 03/16/2017 10:29 AM, Peter Krempa wrote:
Management tools may want to check whether the threshold is still set
if
they missed an event. Add the data to the bulk stats API where they can
also query the current backing size at the same time.
Another way of reading it: the presence of the threshold in the bulk
stats means the event has NOT fired yet. If the threshold is absent,
either it was never registered, or an event was missed and a (new)
threshold needs to be registered.
---
src/libvirt-domain.c | 4 ++++
src/qemu/qemu_driver.c | 51 +++++++++++++++++++++++++++++++++++++++++++++++---
tools/virsh.pod | 3 +++
3 files changed, 55 insertions(+), 3 deletions(-)
diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
index fd367bcb0..815cbb1e1 100644
--- a/src/libvirt-domain.c
+++ b/src/libvirt-domain.c
@@ -11206,6 +11206,10 @@ virConnectGetDomainCapabilities(virConnectPtr conn,
* backing image as unsigned long long.
* "block.<num>.physical" - physical size in bytes of the container
of the
* backing image as unsigned long long.
+ * "block.<num>.threshold.storage - current threshold for delivering the
Rather long name; would block.<num>.threshold be sufficient?
+ *
VIR_DOMAIN_EVENT_ID_BLOCK_THRESHOLD
+ * event in bytes.
+ * See virDomainSetBlockThreshold.
*
* VIR_DOMAIN_STATS_PERF:
* Return perf event statistics.
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 22cf866cd..a541b671d 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -19186,6 +19186,32 @@ qemuDomainGetStatsOneBlockFallback(virQEMUDriverPtr driver,
static int
+qemuDomainGetStatsOneBlockNode(virDomainStatsRecordPtr record,
+ int *maxparams,
+ virStorageSourcePtr src,
+ size_t block_idx,
+ virHashTablePtr nodedata)
+{
+ virJSONValuePtr data;
+ unsigned long long tmp;
+ int ret = -1;
+
+ if (src->nodebacking &&
+ (data = virHashLookup(nodedata, src->nodebacking))) {
+ if (virJSONValueObjectGetNumberUlong(data, "write_threshold",
&tmp) == 0 &&
+ tmp > 0)
+ QEMU_ADD_BLOCK_PARAM_ULL(record, maxparams, block_idx,
+ "threshold.storage", tmp);
Again, the name feels long, but I'm bikeshedding. The statistic itself
is useful.
ACK, whether or not you rename it
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
9:51 a.m.
New subject: [libvirt] [PATCH 3/3] qemu: stats: Display the block threshold size in bulk stats
On Fri, Mar 24, 2017 at 09:45:14 -0500, Eric Blake wrote:
On 03/16/2017 10:29 AM, Peter Krempa wrote:
> Management tools may want to check whether the threshold is still set if
> they missed an event. Add the data to the bulk stats API where they can
> also query the current backing size at the same time.
Another way of reading it: the presence of the threshold in the bulk
stats means the event has NOT fired yet. If the threshold is absent,
either it was never registered, or an event was missed and a (new)
threshold needs to be registered.
> ---
> src/libvirt-domain.c | 4 ++++
> src/qemu/qemu_driver.c | 51 +++++++++++++++++++++++++++++++++++++++++++++++---
> tools/virsh.pod | 3 +++
> 3 files changed, 55 insertions(+), 3 deletions(-)
>
> diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
> index fd367bcb0..815cbb1e1 100644
> --- a/src/libvirt-domain.c
> +++ b/src/libvirt-domain.c
> @@ -11206,6 +11206,10 @@ virConnectGetDomainCapabilities(virConnectPtr conn,
> * backing image as unsigned long long.
> * "block.<num>.physical" - physical size in bytes of the
container of the
> * backing image as unsigned long long.
> + * "block.<num>.threshold.storage - current threshold for delivering
the
Rather long name; would block.<num>.threshold be sufficient?
I don't mind shortening it. I chose to add the suffix if for some reason
any user would want to add a threshold for the guest offset (offset in
the qcow2 write). I don't know if it makes sense though.
Peter
12:41 p.m.
New subject: [libvirt] [PATCH 3/3] qemu: stats: Display the block threshold size in bulk stats
On 03/24/2017 09:51 AM, Peter Krempa wrote:
On Fri, Mar 24, 2017 at 09:45:14 -0500, Eric Blake wrote:
> On 03/16/2017 10:29 AM, Peter Krempa wrote:
>> Management tools may want to check whether the threshold is still set if
>> they missed an event. Add the data to the bulk stats API where they can
>> also query the current backing size at the same time.
>
> Another way of reading it: the presence of the threshold in the bulk
> stats means the event has NOT fired yet. If the threshold is absent,
> either it was never registered, or an event was missed and a (new)
> threshold needs to be registered.
>
>> ---
>> src/libvirt-domain.c | 4 ++++
>> src/qemu/qemu_driver.c | 51 +++++++++++++++++++++++++++++++++++++++++++++++---
>> tools/virsh.pod | 3 +++
>> 3 files changed, 55 insertions(+), 3 deletions(-)
>>
>> diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
>> index fd367bcb0..815cbb1e1 100644
>> --- a/src/libvirt-domain.c
>> +++ b/src/libvirt-domain.c
>> @@ -11206,6 +11206,10 @@ virConnectGetDomainCapabilities(virConnectPtr conn,
>> * backing image as unsigned long long.
>> * "block.<num>.physical" - physical size in bytes of the
container of the
>> * backing image as unsigned long long.
>> + * "block.<num>.threshold.storage - current threshold for
delivering the
>
> Rather long name; would block.<num>.threshold be sufficient?
I don't mind shortening it. I chose to add the suffix if for some reason
any user would want to add a threshold for the guest offset (offset in
the qcow2 write). I don't know if it makes sense though.
Hmm. The API is just SetThreshold, but it has a flags argument so that
we could feasibly set more than one threshold by the use of flags. Then
again, if we ever do add such a flag, all we have to make sure of is
that names are unique at that point (we can't break any existing names,
but it should be too hard to come up with new names that don't clash,
whether or not the original name is short). I'm leaning 60-40 towards
the shorter name, but I'll leave it up to you or anyone else that wants
to chime in.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
2800
days inactive
2808
days old
8 comments
2 participants
participants (2)
-
Eric Blake -
Peter Krempa