On 11/19/2015 04:45 PM, Joao Martins wrote:
Introduce support for domainInterfaceStats API call for querying
network interface statistics. Consequently it also enables the
use of `virsh domifstat <dom> <interface name>` command plus
seeing the interfaces names instead of "-" when doing
`virsh domiflist <dom>`.
After succesful guest creation we fill the network
interfaces names based on domain, device id and append suffix
if it's emulated in the following form: vif<domid>.<devid>[-emu].
The devid is taken from domain config which is accessible in
libxlDomainStartCallback(). On domain
cleanup we also clear ifname, in case it was set by libvirt (i.e.
being prefixed with "vif"). We also skip these two steps in case the name
of the interface was manually inserted by the adminstrator.
For getting the interface statistics we resort to virNetInterfaceStats
and let libvirt handle the platform specific nits. Note that the latter
is not yet supported in FreeBSD.
Signed-off-by: Joao Martins <joao.m.martins(a)oracle.com>
---
Changes since v3:
- Do not unlock vm if libxlDomainObjEndJob() returns false
- Set vm->def->net[i]->ifname on DomainStartCallback instead of
DomainStart.
- Change commit message reflecting the changes on the previous
item and mention correct interface names when doing domiflist.
Changes since v2:
- Clear ifname if it's autogenerated, since otherwise will persist
on successive domain starts. Change commit message reflecting this
change.
Changes since v1:
- Fill <virDomainNetDef>.ifname after domain start with generated
name from libxl based on domain id and devid returned by libxl.
After that path validation don interfaceStats is enterily based
on ifname pretty much like the other drivers.
- Modify commit message reflecting the changes mentioned in
the previous item.
- Bump version to 1.2.22
---
src/libxl/libxl_domain.c | 27 +++++++++++++++++++++++++++
src/libxl/libxl_driver.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 73 insertions(+)
diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c
index f0746f8..5533677 100644
--- a/src/libxl/libxl_domain.c
+++ b/src/libxl/libxl_domain.c
@@ -733,6 +733,17 @@ libxlDomainCleanup(libxlDriverPrivatePtr driver,
}
}
+ if ((vm->def->nnets)) {
+ ssize_t i;
+
+ for (i = 0; i < vm->def->nnets; i++) {
+ virDomainNetDefPtr net = vm->def->nets[i];
+
+ if (STRPREFIX(net->ifname, "vif"))
+ VIR_FREE(net->ifname);
+ }
+ }
+
if (virAsprintf(&file, "%s/%s.xml", cfg->stateDir,
vm->def->name) > 0) {
if (unlink(file) < 0 && errno != ENOENT && errno != ENOTDIR)
VIR_DEBUG("Failed to remove domain XML for %s",
vm->def->name);
@@ -862,6 +873,8 @@ static void
libxlDomainStartCallback(libxl_ctx *ctx, libxl_event *ev, void *for_callback)
{
virDomainObjPtr vm = for_callback;
+ libxlDomainObjPrivatePtr priv = vm->privateData;
+ libxl_domain_config *d_config = priv->config;
size_t i;
virObjectLock(vm);
@@ -888,6 +901,20 @@ libxlDomainStartCallback(libxl_ctx *ctx, libxl_event *ev, void
*for_callback)
VIR_FREE(console);
}
}
+
+ for (i = 0; i < vm->def->nnets; i++) {
+ virDomainNetDefPtr net = vm->def->nets[i];
+ libxl_device_nic *x_nic = &d_config->nics[i];
+ const char *suffix =
+ x_nic->nictype != LIBXL_NIC_TYPE_VIF ? "-emu" : "";
+
+ if (net->ifname)
+ continue;
+
+ if (virAsprintf(&net->ifname, "vif%d.%d%s",
+ ev->domid, x_nic->devid, suffix) < 0)
+ continue;
+ }
Is it possible to get the interfaces with libxl_device_nic_list(), and use those
to generate ifname in the corresponding virDomainNetDef? According to the libxl
docs, it is permitted to call libxl from within the callback.
When I first read this patch, I thought this hunk was in the
libxlDomainInterfaceStats() function below. If that was the case, you can
probably see my concern that vm->def->nets could have grown or shrunk over time
while d_config->nics remained static. Such use of the potentially stale
libxl_domain_config object is why I'm now questioning whether we should add it
to the libxlDomainObjPrivate struct. Sorry for not realizing this when making
the suggestion :-(.
Regards,
Jim
virObjectUnlock(vm);
libxl_event_free(ctx, ev);
}
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index d77a0e4..fe9c357 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -58,6 +58,7 @@
#include "virhostdev.h"
#include "network/bridge_driver.h"
#include "locking/domain_lock.h"
+#include "virstats.h"
#define VIR_FROM_THIS VIR_FROM_LIBXL
@@ -4643,6 +4644,50 @@ libxlDomainIsUpdated(virDomainPtr dom)
}
static int
+libxlDomainInterfaceStats(virDomainPtr dom,
+ const char *path,
+ virDomainInterfaceStatsPtr stats)
+{
+ libxlDriverPrivatePtr driver = dom->conn->privateData;
+ virDomainObjPtr vm;
+ ssize_t i;
+ int ret = -1;
+
+ if (!(vm = libxlDomObjFromDomain(dom)))
+ goto cleanup;
+
+ if (virDomainInterfaceStatsEnsureACL(dom->conn, vm->def) < 0)
+ goto cleanup;
+
+ if (libxlDomainObjBeginJob(driver, vm, LIBXL_JOB_QUERY) < 0)
+ goto cleanup;
+
+ if (!virDomainObjIsActive(vm)) {
+ virReportError(VIR_ERR_OPERATION_INVALID,
+ "%s", _("domain is not running"));
+ goto endjob;
+ }
+
+ /* Check the path is one of the domain's network interfaces. */
+ for (i = 0; i < vm->def->nnets; i++) {
+ if (vm->def->nets[i]->ifname &&
+ STREQ(vm->def->nets[i]->ifname, path)) {
+ ret = virNetInterfaceStats(path, stats);
+ break;
+ }
+ }
+
+ endjob:
+ if (!libxlDomainObjEndJob(driver, vm))
+ vm = NULL;
+
+ cleanup:
+ if (vm)
+ virObjectUnlock(vm);
+ return ret;
+}
+
+static int
libxlDomainGetTotalCPUStats(libxlDriverPrivatePtr driver,
virDomainObjPtr vm,
virTypedParameterPtr params,
@@ -5421,6 +5466,7 @@ static virHypervisorDriver libxlHypervisorDriver = {
#endif
.nodeGetFreeMemory = libxlNodeGetFreeMemory, /* 0.9.0 */
.nodeGetCellsFreeMemory = libxlNodeGetCellsFreeMemory, /* 1.1.1 */
+ .domainInterfaceStats = libxlDomainInterfaceStats, /* 1.2.22 */
.domainMemoryStats = libxlDomainMemoryStats, /* 1.2.22 */
.domainGetCPUStats = libxlDomainGetCPUStats, /* 1.2.22 */
.connectDomainEventRegister = libxlConnectDomainEventRegister, /* 0.9.0 */