Hi,
I've written a patch to include inactive domains in the guest table
available via the libvirt-snmp subagent. This makes it possible to
start domains via SNMP (a state transition which AFAICT was previously
impossible, because inactive domains never appeared in the SNMP
result).
The code now collects the IDs of running domains along with the names
of declared (but not running) domains and normalizes them into a list
of domain pointers, which is then processed by the preexisting SNMP
container row allocation code. The change was fairly straightforward,
but certainly there might be Good Reasons it wasn't already
implemented.
As part of this change, the other (noteworthy) change I made was to
move calls to virDomainFree() to the end of libvirtSnmpLoadGuests()
since I have to free all of the used domain pointers there anyway.
Some other issues which may need to be addressed:
- This will be a breaking change for anyone who assumes that the old
behavior is still in place (i.e., that the guest table only
includes non-shutdown domains). However, it might not be *that*
bad because the domain rows already included a 'state' that one
could already filter on.
- I don't know if there exists other code in libvirt-snmp that
assumes inactive domains can't appear in the table.
- I have not tested the inactive -> active state transition (because
I don't want to enable writable SNMP OIDs on my server).
Please let me know if there is anything further I need to do to make
the patch acceptable.
Thanks for libvirt-snmp!
--
Jonathan Daugherty
Software Engineer
Galois, Inc.
diff --git a/src/libvirtSnmp.c b/src/libvirtSnmp.c
index a9fa185..cd706a8 100644
--- a/src/libvirtSnmp.c
+++ b/src/libvirtSnmp.c
@@ -119,9 +119,15 @@ out:
int
libvirtSnmpLoadGuests(netsnmp_container *container)
{
- int ret = 0, i, numIds, numActiveDomains;
+ int ret = 0, i, numIds,
+ numActiveDomains = 0,
+ numInactiveDomains = 0,
+ numInactiveNames = 0,
+ numTotalDomains = 0;
int *idList = NULL;
+ char **inactiveNameList = NULL;
virDomainPtr domain = NULL;
+ virDomainPtr *domainList = NULL;
virDomainInfo info;
libvirtGuestTable_rowreq_ctx *row_ctx = NULL;
const char *name = NULL;
@@ -134,6 +140,14 @@ libvirtSnmpLoadGuests(netsnmp_container *container)
goto out;
}
+ numInactiveDomains = virConnectNumOfDefinedDomains(conn);
+ if (-1 == numInactiveDomains) {
+ ret = -1;
+ printf("Failed to get number of inactive domains\n");
+ showError(conn);
+ goto out;
+ }
+
idList = malloc(sizeof(*idList) * numActiveDomains);
if (NULL == idList) {
@@ -153,9 +167,47 @@ libvirtSnmpLoadGuests(netsnmp_container *container)
goto out;
}
- for (i = 0 ; i < numIds ; i++) {
- unsigned char uuid[16];
+ inactiveNameList = malloc(sizeof(*inactiveNameList) * numInactiveDomains);
+
+ if (NULL == inactiveNameList) {
+ ret = -1;
+ printf("Could not allocate memory for list of inactive domains\n");
+ goto out;
+ }
+ /* Set each entry to NULL so we can correctly free used
+ entries. */
+ memset(inactiveNameList, 0, sizeof(*inactiveNameList) * numInactiveDomains);
+
+ numInactiveNames = virConnectListDefinedDomains(conn,
+ inactiveNameList,
+ numInactiveDomains);
+
+ if (-1 == numInactiveNames) {
+ ret = -1;
+ printf("Could not get list of inactive domains from hypervisor\n");
+ showError(conn);
+ goto out;
+ }
+
+ /* Populate a list of all domains, active and inactive. We do this
+ * because active domains are resolved by ID while inactive
+ * domains are resolved by name. */
+ numTotalDomains = numActiveDomains + numInactiveNames;
+ domainList = malloc(sizeof(*domainList) * numTotalDomains);
+
+ if (NULL == domainList) {
+ ret = -1;
+ printf("Could not allocate list of domain info structures\n");
+ goto out;
+ }
+
+ /* Before building the list, set each entry to NULL so we can
+ correctly free used entries. */
+ memset(domainList, 0, sizeof(*domainList) * numTotalDomains);
+
+ /* Resolve active domains by ID. */
+ for (i = 0 ; i < numIds ; i++) {
domain = virDomainLookupByID(conn, *(idList + i));
if (NULL == domain) {
printf("Failed to lookup domain\n");
@@ -164,18 +216,39 @@ libvirtSnmpLoadGuests(netsnmp_container *container)
goto out;
}
+ domainList[i] = domain;
+ }
+
+ /* Resolve inactive domains by name. */
+ for (i = 0 ; i < numInactiveNames ; i++) {
+ domain = virDomainLookupByName(conn, *(inactiveNameList + i));
+ if (NULL == domain) {
+ printf("Failed to lookup inactive domain\n");
+ showError(conn);
+ ret = -1;
+ goto out;
+ }
+
+ domainList[i + numActiveDomains] = domain;
+ }
+
+ /* Build the list of guest table rows from the combined domain
+ * list */
+ for (i = 0 ; i < numTotalDomains ; i++) {
+ unsigned char uuid[16];
+
+ domain = domainList[i];
+
if (-1 == virDomainGetInfo(domain, &info)) {
printf("Failed to get domain info\n");
showError(conn);
- virDomainFree(domain);
- ret = -1;
+ ret = -1;
goto out;
}
/* create new row in the container */
row_ctx = libvirtGuestTable_allocate_rowreq_ctx(NULL);
if (!row_ctx) {
- virDomainFree(domain);
snmp_log(LOG_ERR, "Error creating row");
ret = -1;
goto out;
@@ -183,7 +256,6 @@ libvirtSnmpLoadGuests(netsnmp_container *container)
/* set the index of the row */
ret = virDomainGetUUID(domain, uuid);
if (ret) {
- virDomainFree(domain);
snmp_log(LOG_ERR, "Cannot get UUID");
libvirtGuestTable_release_rowreq_ctx(row_ctx);
ret = -1;
@@ -191,7 +263,6 @@ libvirtSnmpLoadGuests(netsnmp_container *container)
}
if (MFD_SUCCESS != libvirtGuestTable_indexes_set(row_ctx, (char*) uuid,
sizeof(uuid))) {
- virDomainFree(domain);
snmp_log(LOG_ERR, "Error setting row index");
libvirtGuestTable_release_rowreq_ctx(row_ctx);
ret = -1;
@@ -199,11 +270,11 @@ libvirtSnmpLoadGuests(netsnmp_container *container)
}
/* set the data */
- name = virDomainGetName(domain);
+ name = virDomainGetName(domainList[i]);
if (name)
row_ctx->data.libvirtGuestName = strdup(name);
else
- row_ctx->data.libvirtGuestName = strdup("");
+ row_ctx->data.libvirtGuestName = strdup("empty");
if (!row_ctx->data.libvirtGuestName) {
snmp_log(LOG_ERR, "Not enough memory for domain name '%s'", name);
libvirtGuestTable_release_rowreq_ctx(row_ctx);
@@ -220,7 +291,6 @@ libvirtSnmpLoadGuests(netsnmp_container *container)
row_ctx->data.libvirtGuestCpuTime.low = info.cpuTime & 0xFFFFFFFF;
row_ctx->data.libvirtGuestRowStatus = ROWSTATUS_ACTIVE;
- virDomainFree(domain);
ret = CONTAINER_INSERT(container, row_ctx);
if (ret) {
@@ -229,10 +299,19 @@ libvirtSnmpLoadGuests(netsnmp_container *container)
ret = -1;
goto out;
}
-
}
out:
+ for (i = 0 ; i < numTotalDomains ; i++ ) {
+ if (NULL != domainList[i])
+ virDomainFree(domainList[i]);
+ }
+
+ for (i = 0; i < numInactiveNames; i++)
+ free(inactiveNameList[i]);
+
+ free(domainList);
+ free(inactiveNameList);
free(idList);
return ret;
}
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list