Attached is a patch to significantly increase scalability / performance of
the xenDaemonLookupByID method. The current implementation would get a
list of all domain names from XenD, and then iterate doing a HTTP GET on
/xend/domain/[name] until the domain with match ID was found. THis had
O(n) complexity, with the result that when running on a system with 20
actives domains, 'virsh list' would have O(n^2) complexity needing ~230
HTTP calls, giving a runtime of ~9 seconds.
The patch is to make the code do a HTTP GET on /xend/domain/[id] which we
just discovered is a valid URL to access. This makes the method call O(1),
and 'virsh list' is now a saner O(n), and completes in ~1 second. While
still not great performance, this is certainly much better. I think it
ought to be possible to optimize the code still further so that XenD is
avoided altogether for simple commands which can be fullfilled purely
with data available from Hypervisor, but that will need further
investigation.
Please review the patch in case I missed any bugs / edge cases
Regards,
Dan.
--
|=- Red Hat, Engineering, Emerging Technologies, Boston. +1 978 392 2496 -=|
|=- Perl modules: http://search.cpan.org/~danberr/ -=|
|=- Projects: http://freshmeat.net/~danielpb/ -=|
|=- GnuPG: 7D3B9505 F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 -=|
diff -ru libvirt/proxy/libvirt_proxy.c libvirt-new/proxy/libvirt_proxy.c
--- libvirt/proxy/libvirt_proxy.c 2006-07-03 07:12:12.000000000 -0400
+++ libvirt-new/proxy/libvirt_proxy.c 2006-07-07 10:36:17.000000000 -0400
@@ -454,33 +454,14 @@
}
break;
case VIR_PROXY_LOOKUP_ID: {
- char **names;
- char **tmp;
- int ident, len;
char *name = NULL;
unsigned char uuid[16];
+ int len;
if (req->len != sizeof(virProxyPacket))
goto comm_error;
- /*
- * Xend API forces to collect the full domain list by names, and
- * then query each of them until the id is found
- */
- names = xenDaemonListDomainsOld(conn);
- tmp = names;
-
- if (names != NULL) {
- while (*tmp != NULL) {
- ident = xenDaemonDomainLookupByName_ids(conn, *tmp, &uuid[0]);
- if (ident == req->data.arg) {
- name = *tmp;
- break;
- }
- tmp++;
- }
- }
- if (name == NULL) {
+ if (xenDaemonDomainLookupByID(conn, req->data.arg, &name, uuid) < 0) {
req->data.arg = -1;
} else {
len = strlen(name);
@@ -492,7 +473,8 @@
memcpy(&request.extra.str[0], uuid, 16);
strcpy(&request.extra.str[16], name);
}
- free(names);
+ if (name)
+ free(name);
break;
}
case VIR_PROXY_LOOKUP_UUID: {
diff -ru libvirt/src/xend_internal.c libvirt-new/src/xend_internal.c
--- libvirt/src/xend_internal.c 2006-07-07 09:59:24.000000000 -0400
+++ libvirt-new/src/xend_internal.c 2006-07-07 10:32:07.000000000 -0400
@@ -1082,7 +1082,7 @@
*/
int
xenDaemonDomainLookupByName_ids(virConnectPtr xend, const char *domname,
- unsigned char *uuid)
+ unsigned char *uuid)
{
struct sexpr *root;
const char *value;
@@ -1119,6 +1119,62 @@
return (ret);
}
+
+/**
+ * xenDaemonDomainLookupByID:
+ * @xend: A xend instance
+ * @id: The id of the domain
+ * @name: return value for the name if not NULL
+ * @uuid: return value for the UUID if not NULL
+ *
+ * This method looks up the name of a domain based on its id
+ *
+ * Returns the 0 on success; -1 (with errno) on error
+ */
+int
+xenDaemonDomainLookupByID(virConnectPtr xend,
+ int id,
+ char **domname,
+ unsigned char *uuid)
+{
+ const char *name = NULL;
+ char *dst_uuid;
+ struct sexpr *root;
+
+ memset(uuid, 0, 16);
+
+ root = sexpr_get(xend, "/xend/domain/%d?detail=1", id);
+ if (root == NULL)
+ goto error;
+
+ name = sexpr_node(root, "domain/name");
+ if (name == NULL) {
+ virXendError(xend, VIR_ERR_INTERNAL_ERROR,
+ "domain informations incomplete, missing name");
+ goto error;
+ }
+ if (domname)
+ *domname = strdup(name);
+
+ dst_uuid = (char *)&uuid[0];
+ if (sexpr_uuid(&dst_uuid, root, "domain/uuid") == NULL) {
+ virXendError(xend, VIR_ERR_INTERNAL_ERROR,
+ "domain informations incomplete, missing uuid");
+ goto error;
+ }
+
+ sexpr_free(root);
+ return (0);
+
+error:
+ sexpr_free(root);
+ if (*domname) {
+ free(*domname);
+ *domname = NULL;
+ }
+ return (-1);
+}
+
/**
* xend_get_node:
* @xend: A xend instance
@@ -2264,33 +2320,13 @@
*/
static virDomainPtr
xenDaemonLookupByID(virConnectPtr conn, int id) {
- char **names;
- char **tmp;
- int ident;
char *name = NULL;
unsigned char uuid[16];
virDomainPtr ret;
- /*
- * Xend API forces to collect the full domain list by names, and then
- * query each of them until the id is found
- */
- names = xenDaemonListDomainsOld(conn);
- tmp = names;
-
- if (names != NULL) {
- while (*tmp != NULL) {
- ident = xenDaemonDomainLookupByName_ids(conn, *tmp, &uuid[0]);
- if (ident == id) {
- name = strdup(*tmp);
- break;
- }
- tmp++;
- }
- free(names);
- }
- if (name == NULL)
+ if (xenDaemonDomainLookupByID(conn, id, &name, uuid) < 0) {
goto error;
+ }
ret = virGetDomain(conn, name, uuid);
if (ret == NULL) {
@@ -2298,13 +2334,12 @@
goto error;
}
ret->handle = id;
- if (name != NULL)
- free(name);
+ free(name);
return (ret);
-error:
+ error:
if (name != NULL)
- free(name);
+ free(name);
return (NULL);
}
diff -ru libvirt/src/xend_internal.h libvirt-new/src/xend_internal.h
--- libvirt/src/xend_internal.h 2006-07-07 08:30:36.000000000 -0400
+++ libvirt-new/src/xend_internal.h 2006-07-07 10:31:18.000000000 -0400
@@ -535,6 +535,20 @@
/**
+ * \brief Lookup the name of a domain
+ * \param xend A xend instance
+ * \param id The id of the domain
+ * \param name pointer to store a copy of the name
+ * \param uuid pointer to store a copy of the uuid
+ *
+ * This method looks up the name/uuid of a domain
+ */
+int xenDaemonDomainLookupByID(virConnectPtr xend,
+ int id,
+ char **name, unsigned char *uuid);
+
+
+/**
* \brief Lookup information about the host machine
* \param xend A xend instance
* \return node info on success; NULL (with errno) on error