6:06 p.m.
Set up functions to make it easy to map between libvirt disk
names and qemu node names. Although we won't expose the
information in XML, it is still nicer to cache the information
than to have to grab a job lock, so that we are less likely
to need to re-query the monitor when dealing with a qemu
monitor event. We need the information in two places: one in
the hash table used to collect QMP stats (as the QMP design
requires using 'query-blockstats' to learn node names,
followed by 'query-named-block-nodes' to learn statistics
about those nodes), and the other in virStorageSourcePtr (the
disk information tracked by libvirt). Making sure this
doesn't leak memory was interesting; in particular, in
qemu_driver.c:qemUDomainBlocksStatsGather(), I made sure that
the returned result does not set an allocation name, so that
the callers of that function can continue to use VIR_FREE.
* src/util/virstoragefile.h (_virStorageSource): Add a field.
* src/util/virstoragefile.c (virStorageSourceBackingStoreClear):
Clear it.
(virStorageSourceCopy): Document that it is not deep-copied.
* src/qemu/qemu_domain.c (qemuDomainDiskGetAllocationNode)
(qemuDomainDiskResolveAllocationNode): New functions, to use the
field.
(qemuDomainDiskSupportsAllocationNode): New helper.
* src/qemu/qemu_domain.h: Declare new functions.
* src/qemu/qemu_monitor.h (_qemuBlockStats): Add fields.
(qemuMonitorCleanBlockStats): New declaration.
* src/qemu/qemu_monitor.c (qemuMonitorCleanBlockStats): New
function.
(qemuMonitorGetAllBlockStatsInfo): Use it to avoid leak.
* src/qemu/qemu_migration.c (qemuMigrationCookieAddNBD):
Likewise.
* src/qemu/qemu_driver.c (qemuDomainBlocksStatsGather): Don't leak
memory.
* tests/qemumonitortest.c (testBlockInfoData): Update test.
---
src/qemu/qemu_domain.c | 105 ++++++++++++++++++++++++++++++++++++++++++++++
src/qemu/qemu_domain.h | 10 ++++-
src/qemu/qemu_driver.c | 1 +
src/qemu/qemu_migration.c | 2 +-
src/qemu/qemu_monitor.c | 12 +++++-
src/qemu/qemu_monitor.h | 4 ++
src/util/virstoragefile.c | 4 +-
src/util/virstoragefile.h | 3 +-
tests/qemumonitortest.c | 13 +++---
9 files changed, 143 insertions(+), 11 deletions(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 6213fd9..d3ce7db 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -2906,6 +2906,111 @@ qemuDomainDetermineDiskChain(virQEMUDriverPtr driver,
}
+static bool
+qemuDomainDiskSupportsAllocationNode(virDomainDiskDefPtr disk)
+{
+ /* For now, only qcow2 images backed by block devices are supported. */
+ if (disk->src->type != VIR_STORAGE_TYPE_BLOCK)
+ return false;
+ if (disk->src->format != VIR_STORAGE_FILE_QCOW2)
+ return false;
+ return true;
+}
+
+
+/* Determine the node name that qemu associates with allocation
+ * tracking. Cache the value to minimize queries. Return NULL if the
+ * storage source is not a qcow2 formatted block device (as those are
+ * the only devices where live allocation tracking is useful), or if
+ * qemu cannot determine a node name. Requires a live domain, and
+ * assumes that the job lock is held, as this may require monitor
+ * interaction. */
+const char *
+qemuDomainDiskGetAllocationNode(virQEMUDriverPtr driver,
+ virDomainObjPtr vm,
+ virDomainDiskDefPtr disk)
+{
+ virStorageSourcePtr src = disk->src;
+ char *name = NULL;
+ qemuDomainObjPrivatePtr priv = vm->privateData;
+
+ if (!qemuDomainDiskSupportsAllocationNode(disk))
+ return NULL;
+
+ if (src->allocation_node)
+ goto cleanup;
+
+ qemuDomainObjEnterMonitor(driver, vm);
+ /* TODO: allow lookup of node name of backing images */
+ name = qemuMonitorNodeNameLookup(priv->mon, disk->info.alias);
+ if (qemuDomainObjExitMonitor(driver, vm) < 0)
+ goto cleanup;
+
+ src->allocation_node = name;
+ name = NULL;
+
+ cleanup:
+ VIR_FREE(name);
+ return src->allocation_node;
+}
+
+
+/* Determine the disk name that matches a node name returned by qemu
+ * in a threshold event. In the common case, no job is needed: the
+ * node name will be cached from when the threshold was
+ * registered. But if the cache is lost (such as over a libvirtd
+ * restart), JOB_OKAY states whether it is safe to rebuild the cache
+ * (requires a live domain and monitor interaction), instead of
+ * failing. */
+virDomainDiskDefPtr
+qemuDomainDiskResolveAllocationNode(virQEMUDriverPtr driver,
+ virDomainObjPtr vm,
+ const char *node,
+ bool job_okay)
+{
+ size_t i;
+ virDomainDiskDefPtr disk;
+ virDomainDiskDefPtr ret = NULL;
+ char *name = NULL;
+ qemuDomainObjPrivatePtr priv = vm->privateData;
+
+ /* First pass - see if the node name is cached. */
+ for (i = 0; i < vm->def->ndisks; i++) {
+ disk = vm->def->disks[i];
+ if (!qemuDomainDiskSupportsAllocationNode(disk))
+ continue;
+ if (STREQ_NULLABLE(disk->src->allocation_node, node))
+ return disk;
+ }
+
+ /* Second pass - query the monitor. */
+ if (!job_okay)
+ return NULL;
+ for (i = 0; i < vm->def->ndisks; i++) {
+ disk = vm->def->disks[i];
+ if (!qemuDomainDiskSupportsAllocationNode(disk) ||
+ disk->src->allocation_node)
+ continue;
+
+ qemuDomainObjEnterMonitor(driver, vm);
+ name = qemuMonitorNodeNameLookup(priv->mon, disk->info.alias);
+ if (qemuDomainObjExitMonitor(driver, vm) < 0)
+ goto cleanup;
+
+ disk->src->allocation_node = name;
+ name = NULL;
+ if (STREQ_NULLABLE(disk->src->allocation_node, node)) {
+ ret = disk;
+ break;
+ }
+ }
+
+ cleanup:
+ VIR_FREE(name);
+ return ret;
+}
+
+
bool
qemuDomainDiskBlockJobIsActive(virDomainDiskDefPtr disk)
{
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index 54e1e7b..9550349 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -1,7 +1,7 @@
/*
* qemu_domain.h: QEMU domain private state
*
- * Copyright (C) 2006-2014 Red Hat, Inc.
+ * Copyright (C) 2006-2015 Red Hat, Inc.
* Copyright (C) 2006 Daniel P. Berrange
*
* This library is free software; you can redistribute it and/or
@@ -408,6 +408,14 @@ int qemuDomainDetermineDiskChain(virQEMUDriverPtr driver,
bool force_probe,
bool report_broken);
+const char *qemuDomainDiskGetAllocationNode(virQEMUDriverPtr driver,
+ virDomainObjPtr vm,
+ virDomainDiskDefPtr disk);
+virDomainDiskDefPtr qemuDomainDiskResolveAllocationNode(virQEMUDriverPtr driver,
+ virDomainObjPtr vm,
+ const char *node,
+ bool job_okay);
+
int qemuDomainStorageFileInit(virQEMUDriverPtr driver,
virDomainObjPtr vm,
virStorageSourcePtr src);
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index c1373de..25bc76d 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -10967,6 +10967,7 @@ qemuDomainBlocksStatsGather(virQEMUDriverPtr driver,
_("cannot find statistics for device '%s'"),
diskAlias);
goto cleanup;
}
+ VIR_FREE(stats->allocation_node);
**retstats = *stats;
} else {
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index 47d49cd..1651edc 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -569,7 +569,7 @@ qemuMigrationCookieAddNBD(qemuMigrationCookiePtr mig,
qemuBlockStats *entry;
if (!stats) {
- if (!(stats = virHashCreate(10, virHashValueFree)))
+ if (!(stats = virHashCreate(10, qemuMonitorCleanBlockStats)))
goto cleanup;
if (qemuDomainObjEnterMonitorAsync(driver, vm,
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index 5612491..86dc4be 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -1763,6 +1763,16 @@ qemuMonitorBlockInfoLookup(virHashTablePtr blockInfo,
}
+/* Callback for use in cleaning up a hash used by
+ * qemuMonitorGetAllBlockStatsInfo. */
+void
+qemuMonitorCleanBlockStats(void *opaque, const void *name ATTRIBUTE_UNUSED)
+{
+ qemuBlockStatsPtr stats = opaque;
+ VIR_FREE(stats->allocation_node);
+ VIR_FREE(stats);
+}
+
/**
* qemuMonitorGetAllBlockStatsInfo:
* @mon: monitor object
@@ -1785,7 +1795,7 @@ qemuMonitorGetAllBlockStatsInfo(qemuMonitorPtr mon,
QEMU_CHECK_MONITOR(mon);
- if (!(*ret_stats = virHashCreate(10, virHashValueFree)))
+ if (!(*ret_stats = virHashCreate(10, qemuMonitorCleanBlockStats)))
goto error;
if (mon->json) {
diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
index 826835b..c0ea2ee 100644
--- a/src/qemu/qemu_monitor.h
+++ b/src/qemu/qemu_monitor.h
@@ -379,8 +379,12 @@ struct _qemuBlockStats {
unsigned long long capacity;
unsigned long long physical;
unsigned long long wr_highest_offset;
+ char *allocation_node;
+ unsigned long long write_threshold;
};
+void qemuMonitorCleanBlockStats(void *opaque, const void *name);
+
int qemuMonitorGetAllBlockStatsInfo(qemuMonitorPtr mon,
virHashTablePtr *ret_stats,
bool backingChain)
diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
index 6c3017c..a175e1d 100644
--- a/src/util/virstoragefile.c
+++ b/src/util/virstoragefile.c
@@ -1,7 +1,7 @@
/*
* virstoragefile.c: file utility functions for FS storage backend
*
- * Copyright (C) 2007-2014 Red Hat, Inc.
+ * Copyright (C) 2007-2015 Red Hat, Inc.
* Copyright (C) 2007-2008 Daniel P. Berrange
*
* This library is free software; you can redistribute it and/or
@@ -1833,6 +1833,7 @@ virStorageSourceCopy(const virStorageSource *src,
ret->capacity = src->capacity;
ret->allocation = src->allocation;
ret->physical = src->physical;
+ /* allocation node name cache is not copied */
ret->readonly = src->readonly;
ret->shared = src->shared;
@@ -2048,6 +2049,7 @@ virStorageSourceClear(virStorageSourcePtr def)
virStorageSourceSeclabelsClear(def);
virStoragePermsFree(def->perms);
VIR_FREE(def->timestamps);
+ VIR_FREE(def->allocation_node);
virStorageNetHostDefFree(def->nhosts, def->hosts);
virStorageAuthDefFree(def->auth);
diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h
index aa17a00..8b2f491 100644
--- a/src/util/virstoragefile.h
+++ b/src/util/virstoragefile.h
@@ -1,7 +1,7 @@
/*
* virstoragefile.h: file utility functions for FS storage backend
*
- * Copyright (C) 2007-2009, 2012-2014 Red Hat, Inc.
+ * Copyright (C) 2007-2009, 2012-2015 Red Hat, Inc.
* Copyright (C) 2007-2008 Daniel P. Berrange
*
* This library is free software; you can redistribute it and/or
@@ -260,6 +260,7 @@ struct _virStorageSource {
unsigned long long capacity; /* in bytes, 0 if unknown */
unsigned long long allocation; /* in bytes, 0 if unknown */
unsigned long long physical; /* in bytes, 0 if unknown */
+ char *allocation_node; /* cache of node name for tracking allocation */
size_t nseclabels;
virSecurityDeviceLabelDefPtr *seclabels;
diff --git a/tests/qemumonitortest.c b/tests/qemumonitortest.c
index 0125962..b580416 100644
--- a/tests/qemumonitortest.c
+++ b/tests/qemumonitortest.c
@@ -93,12 +93,13 @@ struct blockInfoData {
static const struct blockInfoData testBlockInfoData[] =
{
-/* NAME, rd_req, rd_bytes, wr_req, wr_bytes, rd_total_time, wr_total_time, flush_req,
flush_total_time */
- {"vda", {11, 12, 13, 14, 15, 16, 17, 18, 0, 0, 0}},
- {"vdb", {21, 22, 23, 24, 25, 26, 27, 28, 0, 0, 0}},
- {"vdc", {31, 32, 33, -1, 35, 36, 37, 38, 0, 0, 0}},
- {"vdd", {-1, -1, -1, -1, -1, -1, -1, -1, 0, 0, 0}},
- {"vde", {41, 42, 43, 44, 45, 46, 47, 48, 0, 0, 0}}
+ /* NAME, rd_req, rd_bytes, wr_req, wr_bytes, rd_total_time, wr_total_time,
+ * flush_req, flush_total_time, allocation_node, write_threshold */
+ {"vda", {11, 12, 13, 14, 15, 16, 17, 18, 0, 0, 0, NULL, 0}},
+ {"vdb", {21, 22, 23, 24, 25, 26, 27, 28, 0, 0, 0, NULL, 0}},
+ {"vdc", {31, 32, 33, -1, 35, 36, 37, 38, 0, 0, 0, NULL, 0}},
+ {"vdd", {-1, -1, -1, -1, -1, -1, -1, -1, 0, 0, 0, NULL, 0}},
+ {"vde", {41, 42, 43, 44, 45, 46, 47, 48, 0, 0, 0, NULL, 0}},
};
static const char testBlockInfoReply[] =
--
2.4.3