- Devel - libvirt List Archives

[libvirt] libvirt-gconfig patches
by Christophe Fergeau 23 Nov '11

23 Nov '11

Hi, This is the 2nd version of my libvirt-gconfig patches. I think I have addressed all of the issues that were raised (or told why I didn't address them). If I missed anything, just let me know. Some of the patches have been split/merged after the review so the series look a bit different from the first one even if the content is the same. One notable change is that I dropped the chardev classes for now since I need to go back to the drawing board a bit for these Christophe

2 66

[libvirt] [PATCH v2] nwfilter: Initialize virNWFilterAddIpAddrForIfname return variable
by Stefan Berger 23 Nov '11

23 Nov '11

Thanks for trying to fix this. This one tries to address Jiri's concern about the return value from virNWFilterHashTablePut with a not to fix it in the future. Latest nwfilter patch ad6c67cf introduced uninitialized return value. This was spotted by 4.6.2 gcc. --- src/nwfilter/nwfilter_learnipaddr.c | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) Index: libvirt-acl/src/nwfilter/nwfilter_learnipaddr.c =================================================================== --- libvirt-acl.orig/src/nwfilter/nwfilter_learnipaddr.c +++ libvirt-acl/src/nwfilter/nwfilter_learnipaddr.c @@ -323,7 +323,7 @@ virNWFilterDeregisterLearnReq(int ifinde static int virNWFilterAddIpAddrForIfname(const char *ifname, char *addr) { - int ret; + int ret = -1; virNWFilterVarValuePtr val; virMutexLock(&ipAddressMapLock); @@ -333,16 +333,21 @@ virNWFilterAddIpAddrForIfname(const char val = virNWFilterVarValueCreateSimple(addr); if (!val) { virReportOOMError(); - ret = -1; - goto err_exit; + goto cleanup; } ret = virNWFilterHashTablePut(ipAddressMap, ifname, val, 1); + /* FIXME: fix when return code of virNWFilterHashTablePut changes */ + if (ret) + ret = -1; + goto cleanup; } else { if (virNWFilterVarValueAddValue(val, addr) < 0) - ret = -1; + goto cleanup; } -err_exit: + ret = 0; + +cleanup: virMutexUnlock(&ipAddressMapLock); return ret;

2 2

[libvirt] [PATCH] nwfilter: Initialize virNWFilterAddIpAddrForIfname return variable
by Michal Privoznik 23 Nov '11

23 Nov '11

Latest nwfilter patch ad6c67cf introduced uninitialized return value. This was spotted by 4.6.2 gcc. --- src/nwfilter/nwfilter_learnipaddr.c | 12 +++++++----- 1 files changed, 7 insertions(+), 5 deletions(-) diff --git a/src/nwfilter/nwfilter_learnipaddr.c b/src/nwfilter/nwfilter_learnipaddr.c index 675ef5b..2e373a6 100644 --- a/src/nwfilter/nwfilter_learnipaddr.c +++ b/src/nwfilter/nwfilter_learnipaddr.c @@ -323,7 +323,7 @@ virNWFilterDeregisterLearnReq(int ifindex) { static int virNWFilterAddIpAddrForIfname(const char *ifname, char *addr) { - int ret; + int ret = -1; virNWFilterVarValuePtr val; virMutexLock(&ipAddressMapLock); @@ -333,16 +333,18 @@ virNWFilterAddIpAddrForIfname(const char *ifname, char *addr) val = virNWFilterVarValueCreateSimple(addr); if (!val) { virReportOOMError(); - ret = -1; - goto err_exit; + goto cleanup; } ret = virNWFilterHashTablePut(ipAddressMap, ifname, val, 1); + goto cleanup; } else { if (virNWFilterVarValueAddValue(val, addr) < 0) - ret = -1; + goto cleanup; } -err_exit: + ret = 0; + +cleanup: virMutexUnlock(&ipAddressMapLock); return ret; -- 1.7.3.4

2 1

[libvirt] [PATCH v4] virsh: Increase device-detach intelligence
by Michal Privoznik 23 Nov '11

23 Nov '11

From: Michal Prívozník <mprivozn(a)redhat.com> Up to now users have to give a full XML description on input when device-detaching. If they omitted something it lead to unclear error messages (like generated MAC wasn't found, etc.). With this patch users can specify only those information which specify one device sufficiently precise. Remaining information is completed from domain. --- diff to v3: - Eric's review: https://www.redhat.com/archives/libvir-list/2011-September/msg00472.html tools/virsh.c | 284 +++++++++++++++++++++++++++++++++++++++++++++++++++++--- 1 files changed, 268 insertions(+), 16 deletions(-) diff --git a/tools/virsh.c b/tools/virsh.c index 89fb4e7..a798561 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -60,6 +60,7 @@ #include "command.h" #include "virkeycode.h" #include "virnetdevbandwidth.h" +#include "util/bitmap.h" static char *progname; @@ -11502,6 +11503,243 @@ cmdAttachDevice(vshControl *ctl, const vshCmd *cmd) return true; } +/** + * Check if n1 is superset of n2, meaning n1 contains all elements and + * attributes as n2 at least. Including children. + * @n1 first node + * @n2 second node + * return 1 in case n1 covers n2, 0 otherwise. + */ +static bool +vshNodeIsSuperset(xmlNodePtr n1, xmlNodePtr n2) { + xmlNodePtr child1, child2; + xmlAttrPtr attr; + char *prop1, *prop2; + bool found; + bool visited; + bool ret = false; + unsigned long n1_child_size, n1_iter; + virBitmapPtr bitmap; + + if (!n1 && !n2) + return true; + + if (!n1 || !n2) + return false; + + if (!xmlStrEqual(n1->name, n2->name)) + return false; + + /* Iterate over n2 attributes and check if n1 contains them*/ + attr = n2->properties; + while (attr) { + if (attr->type == XML_ATTRIBUTE_NODE) { + prop1 = virXMLPropString(n1, (const char *) attr->name); + prop2 = virXMLPropString(n2, (const char *) attr->name); + if (STRNEQ_NULLABLE(prop1, prop2)) { + xmlFree(prop1); + xmlFree(prop2); + return false; + } + xmlFree(prop1); + xmlFree(prop2); + } + attr = attr->next; + } + + n1_child_size = xmlChildElementCount(n1); + if (!n1_child_size) { + return !xmlChildElementCount(n2); + } + + if (!(bitmap = virBitmapAlloc(n1_child_size))) { + virReportOOMError(); + return false; + } + + child2 = n2->children; + while (child2) { + if (child2->type != XML_ELEMENT_NODE) { + child2 = child2->next; + continue; + } + + child1 = n1->children; + n1_iter = 0; + found = false; + while (child1) { + if (child1->type != XML_ELEMENT_NODE) { + child1 = child1->next; + continue; + } + + if (virBitmapGetBit(bitmap, n1_iter, &visited) < 0) { + vshError(NULL, "%s", _("Bad child elements counting.")); + goto cleanup; + } + + if (visited) { + child1 = child1->next; + n1_iter++; + continue; + } + + if (xmlStrEqual(child1->name, child2->name)) { + found = true; + if (virBitmapSetBit(bitmap, n1_iter) < 0) { + vshError(NULL, "%s", _("Bad child elements counting.")); + goto cleanup; + } + + if (!vshNodeIsSuperset(child1, child2)) + goto cleanup; + + break; + } + + child1 = child1->next; + n1_iter++; + } + + if (!found) + goto cleanup; + + child2 = child2->next; + } + + ret = true; + +cleanup: + virBitmapFree(bitmap); + return ret; +} + +/** + * vshCompleteXMLFromDomain: + * @ctl vshControl for error messages printing + * @dom domain + * @oldXML device XML before + * @newXML and after completion + * + * For given domain and (probably incomplete) device XML specification try to + * find such device in domain and complete missing parts. This is however + * possible only when given device XML is sufficiently precise so it addresses + * only one device. + * + * Returns -2 when no such device exists in domain, -3 when given XML selects many + * (is too ambiguous), 0 in case of success. Otherwise returns -1. @newXML + * is touched only in case of success. + */ +static int +vshCompleteXMLFromDomain(vshControl *ctl, virDomainPtr dom, char *oldXML, + char **newXML) { + int funcRet = -1; + char *domXML = NULL; + xmlDocPtr domDoc = NULL, devDoc = NULL; + xmlNodePtr node = NULL; + xmlXPathContextPtr domCtxt = NULL, devCtxt = NULL; + xmlNodePtr *devices = NULL; + xmlSaveCtxtPtr sctxt = NULL; + int devices_size; + char *xpath; + xmlBufferPtr buf = NULL; + int i = 0; + int indx = -1; + + if (!(domXML = virDomainGetXMLDesc(dom, 0))) { + vshError(ctl, _("couldn't get XML description of domain %s"), + virDomainGetName(dom)); + goto cleanup; + } + + domDoc = virXMLParseStringCtxt(domXML, _("(domain_definition)"), &domCtxt); + if (!domDoc) { + vshError(ctl, _("Failed to parse domain definition xml")); + goto cleanup; + } + + devDoc = virXMLParseStringCtxt(oldXML, _("(device_definition)"), &devCtxt); + if (!devDoc) { + vshError(ctl, _("Failed to parse device definition xml")); + goto cleanup; + } + + node = xmlDocGetRootElement(devDoc); + + buf = xmlBufferCreate(); + if (!buf) { + vshError(ctl, "%s", _("out of memory")); + goto cleanup; + } + + /* Get all possible devices */ + virAsprintf(&xpath, "/domain/devices/%s", node->name); + if (!xpath) { + virReportOOMError(); + goto cleanup; + } + devices_size = virXPathNodeSet(xpath, domCtxt, &devices); + + if (devices_size < 0) { + /* error */ + vshError(ctl, "%s", _("error when selecting nodes")); + goto cleanup; + } else if (devices_size == 0) { + /* no such device */ + funcRet = -2; + goto cleanup; + } + + /* and refine */ + for (i = 0; i < devices_size; i++) { + if (vshNodeIsSuperset(devices[i], node)) { + if (indx >= 0) { + funcRet = -3; /* ambiguous */ + goto cleanup; + } + indx = i; + } + } + + if (indx < 0) { + funcRet = -2; /* no such device */ + goto cleanup; + } + + vshDebug(ctl, VSH_ERR_DEBUG, "Found device at pos %d\n", indx); + + if (newXML) { + sctxt = xmlSaveToBuffer(buf, NULL, 0); + if (!sctxt) { + vshError(ctl, "%s", _("failed to create document saving context")); + goto cleanup; + } + + xmlSaveTree(sctxt, devices[indx]); + xmlSaveClose(sctxt); + *newXML = (char *) xmlBufferContent(buf); + if (!*newXML) { + virReportOOMError(); + goto cleanup; + } + buf->content = NULL; + } + + vshDebug(ctl, VSH_ERR_DEBUG, "Old xml:\n%s\nNew xml:\n%s\n", oldXML, + newXML ? NULLSTR(*newXML) : "(null)"); + + funcRet = 0; + +cleanup: + xmlBufferFree(buf); + VIR_FREE(devices); + xmlXPathFreeContext(devCtxt); + xmlXPathFreeContext(domCtxt); + xmlFreeDoc(devDoc); + xmlFreeDoc(domDoc); + VIR_FREE(domXML); + return funcRet; +} /* * "detach-device" command @@ -11522,10 +11760,11 @@ static const vshCmdOptDef opts_detach_device[] = { static bool cmdDetachDevice(vshControl *ctl, const vshCmd *cmd) { - virDomainPtr dom; + virDomainPtr dom = NULL; const char *from = NULL; - char *buffer; + char *buffer = NULL, *new_buffer = NULL; int ret; + bool funcRet = false; unsigned int flags; if (!vshConnectionUsability(ctl, ctl->conn)) @@ -11534,37 +11773,50 @@ cmdDetachDevice(vshControl *ctl, const vshCmd *cmd) if (!(dom = vshCommandOptDomain(ctl, cmd, NULL))) return false; - if (vshCommandOptString(cmd, "file", &from) <= 0) { - virDomainFree(dom); - return false; - } + if (vshCommandOptString(cmd, "file", &from) <= 0) + goto cleanup; if (virFileReadAll(from, VIRSH_MAX_XML_FILE, &buffer) < 0) { virshReportError(ctl); - virDomainFree(dom); - return false; + goto cleanup; + } + + ret = vshCompleteXMLFromDomain(ctl, dom, buffer, &new_buffer); + if (ret < 0) { + if (ret == -2) { + vshError(ctl, _("no such device in %s"), virDomainGetName(dom)); + } else if (ret == -3) { + vshError(ctl, "%s", _("given XML selects too many devices. " + "Please, be more specific")); + } else { + /* vshCompleteXMLFromDomain() already printed error message, + * so nothing to do here. */ + } + goto cleanup; } if (vshCommandOptBool(cmd, "persistent")) { flags = VIR_DOMAIN_AFFECT_CONFIG; if (virDomainIsActive(dom) == 1) flags |= VIR_DOMAIN_AFFECT_LIVE; - ret = virDomainDetachDeviceFlags(dom, buffer, flags); + ret = virDomainDetachDeviceFlags(dom, new_buffer, flags); } else { - ret = virDomainDetachDevice(dom, buffer); + ret = virDomainDetachDevice(dom, new_buffer); } - VIR_FREE(buffer); if (ret < 0) { vshError(ctl, _("Failed to detach device from %s"), from); - virDomainFree(dom); - return false; - } else { - vshPrint(ctl, "%s", _("Device detached successfully\n")); + goto cleanup; } + vshPrint(ctl, "%s", _("Device detached successfully\n")); + funcRet = true; + +cleanup: + VIR_FREE(new_buffer); + VIR_FREE(buffer); virDomainFree(dom); - return true; + return funcRet; } -- 1.7.3.4

2 2

[libvirt] [PATCH 2/8] Add virDomain{Set, Get}BlockIoTune support to the remote driver
by Lei Li 23 Nov '11

23 Nov '11

Support Block I/O Throttle setting and query to remote driver. Signed-off-by: Lei Li <lilei(a)linux.vnet.ibm.com> Signed-off-by: Zhi Yong Wu <wuzhy(a)linux.vnet.ibm.com> --- daemon/remote.c | 109 ++++++++++++++++++++++++++++++++++++++++++ src/remote/remote_driver.c | 96 +++++++++++++++++++++++++++++++++++++ src/remote/remote_protocol.x | 27 ++++++++++- src/remote_protocol-structs | 24 +++++++++ 4 files changed, 255 insertions(+), 1 deletions(-) diff --git a/daemon/remote.c b/daemon/remote.c index aa3f768..227d36e 100644 --- a/daemon/remote.c +++ b/daemon/remote.c @@ -1886,6 +1886,115 @@ cleanup: return rv; } +static int +remoteDispatchDomainSetBlockIoThrottle(virNetServerPtr server ATTRIBUTE_UNUSED, + virNetServerClientPtr client ATTRIBUTE_UNUSED, + virNetMessagePtr hdr ATTRIBUTE_UNUSED, + virNetMessageErrorPtr rerr, + remote_domain_set_block_io_throttle_args *args) +{ + virDomainPtr dom = NULL; + int rv = -1; + virTypedParameterPtr params = NULL; + int nparams; + struct daemonClientPrivate *priv = + virNetServerClientGetPrivateData(client); + + if (!priv->conn) { + virNetError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not open")); + goto cleanup; + } + + if (!(dom = get_nonnull_domain(priv->conn, args->dom))) + goto cleanup; + + if ((params = remoteDeserializeTypedParameters(args->params.params_val, + args->params.params_len, + REMOTE_DOMAIN_BLKIOTHROTTLE_PARAMETERS_MAX, + &nparams)) == NULL) + goto cleanup; + + rv = virDomainSetBlockIoTune(dom, args->disk, params, nparams, args->flags); + + if (rv < 0) + goto cleanup; + +cleanup: + if (rv < 0) + virNetMessageSaveError(rerr); + if (dom) + virDomainFree(dom); + return rv; +} + +static int +remoteDispatchDomainGetBlockIoThrottle(virNetServerPtr server ATTRIBUTE_UNUSED, + virNetServerClientPtr client ATTRIBUTE_UNUSED, + virNetMessagePtr hdr ATTRIBUTE_UNUSED, + virNetMessageErrorPtr rerr, + remote_domain_get_block_io_throttle_args *args, + remote_domain_get_block_io_throttle_ret *ret) +{ + virDomainPtr dom = NULL; + int rv = -1; + int i; + virTypedParameterPtr params; + int nparams = args->nparams; + struct daemonClientPrivate *priv = + virNetServerClientGetPrivateData(client); + + if (!priv->conn) { + virNetError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not open")); + goto cleanup; + } + + if (nparams > REMOTE_DOMAIN_BLKIOTHROTTLE_PARAMETERS_MAX) { + virNetError(VIR_ERR_INTERNAL_ERROR, "%s", _("nparams too large")); + goto cleanup; + } + + if (VIR_ALLOC_N(params, nparams) < 0) { + virReportOOMError(); + goto cleanup; + } + + if (!(dom = get_nonnull_domain(priv->conn, args->dom))) + goto cleanup; + + if (virDomainGetBlockIoTune(dom, args->disk, params, &nparams, args->flags) < 0) + goto cleanup; + + /* In this case, we need to send back the number of parameters + * supported + */ + if (args->nparams == 0) { + ret->nparams = nparams; + goto success; + } + + /* Serialise the block I/O throttle. */ + if (remoteSerializeTypedParameters(params, nparams, + &ret->params.params_val, + &ret->params.params_len, + args->flags) < 0) + goto cleanup; + +success: + rv = 0; + +cleanup: + if (rv < 0) { + virNetMessageSaveError(rerr); + if (ret->params.params_val) { + for (i = 0; i < nparams; i++) + VIR_FREE(ret->params.params_val[i].field); + VIR_FREE(ret->params.params_val); + } + } + if (dom) + virDomainFree(dom); + return rv; +} /*-------------------------------------------------------------*/ diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index 94fd3e7..fa2d2c7 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -2178,6 +2178,100 @@ done: return rv; } +static int remoteDomainSetBlockIoTune(virDomainPtr domain, + const char *disk, + virTypedParameterPtr params, + int nparams, + unsigned int flags) +{ + int rv = -1; + remote_domain_set_block_io_throttle_args args; + struct private_data *priv = domain->conn->privateData; + + remoteDriverLock(priv); + + memset(&args, 0, sizeof(args)); + + make_nonnull_domain(&args.dom, domain); + args.disk = (char *)disk; + args.flags = flags; + + if (remoteSerializeTypedParameters(params, nparams, &args.params.params_val, &args.params.params_len) < 0) { + xdr_free((xdrproc_t)xdr_remote_domain_set_block_io_throttle_args, (char *)&args); + goto done; + } + + if (call(domain->conn, priv, 0, REMOTE_PROC_DOMAIN_SET_BLOCK_IO_THROTTLE, + (xdrproc_t) xdr_remote_domain_set_block_io_throttle_args, + (char *) &args, + (xdrproc_t) xdr_void, + (char *) NULL) == -1) { + goto done; + } + rv = 0; + +done: + remoteDriverUnlock(priv); + return rv; +} + +static int remoteDomainGetBlockIoTune(virDomainPtr domain, + const char *disk, + virTypedParameterPtr params, + int *nparams, + unsigned int flags) +{ + int rv = -1; + remote_domain_get_block_io_throttle_args args; + remote_domain_get_block_io_throttle_ret ret; + struct private_data *priv = domain->conn->privateData; + + remoteDriverLock(priv); + + make_nonnull_domain(&args.dom, domain); + args.disk = (char *)disk; + args.nparams = *nparams; + args.flags = flags; + + memset(&ret, 0, sizeof(ret)); + + + if (call(domain->conn, priv, 0, REMOTE_PROC_DOMAIN_GET_BLOCK_IO_THROTTLE, + (xdrproc_t) xdr_remote_domain_get_block_io_throttle_args, + (char *) &args, + (xdrproc_t) xdr_remote_domain_get_block_io_throttle_ret, + (char *) &ret) == -1) { + goto done; + } + + /* Handle the case when the caller does not know the number of parameters + * and is asking for the number of parameters supported + */ + if (*nparams == 0) { + *nparams = ret.nparams; + rv = 0; + goto cleanup; + } + + if (remoteDeserializeTypedParameters(ret.params.params_val, + ret.params.params_len, + REMOTE_DOMAIN_MEMORY_PARAMETERS_MAX, + params, + nparams) < 0) + goto cleanup; + + rv = 0; + +cleanup: + xdr_free ((xdrproc_t) xdr_remote_domain_get_block_io_throttle_ret, + (char *) &ret); + rv = 0; + +done: + remoteDriverUnlock(priv); + return rv; +} + /*----------------------------------------------------------------------*/ static virDrvOpenStatus ATTRIBUTE_NONNULL (1) @@ -4550,6 +4644,8 @@ static virDriver remote_driver = { .domainGetBlockJobInfo = remoteDomainGetBlockJobInfo, /* 0.9.4 */ .domainBlockJobSetSpeed = remoteDomainBlockJobSetSpeed, /* 0.9.4 */ .domainBlockPull = remoteDomainBlockPull, /* 0.9.4 */ + .domainSetBlockIoTune = remoteDomainSetBlockIoTune, /* 0.9.8 */ + .domainGetBlockIoTune = remoteDomainGetBlockIoTune, /* 0.9.8 */ }; static virNetworkDriver network_driver = { diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x index 5ea1114..9ab3314 100644 --- a/src/remote/remote_protocol.x +++ b/src/remote/remote_protocol.x @@ -125,6 +125,9 @@ const REMOTE_DOMAIN_BLKIO_PARAMETERS_MAX = 16; /* Upper limit on list of memory parameters. */ const REMOTE_DOMAIN_MEMORY_PARAMETERS_MAX = 16; +/* Upper limit on list of blockio throttle parameters. */ +const REMOTE_DOMAIN_BLKIOTHROTTLE_PARAMETERS_MAX = 16; + /* Upper limit on list of node cpu stats. */ const REMOTE_NODE_CPU_STATS_MAX = 16; @@ -1075,6 +1078,25 @@ struct remote_domain_block_pull_args { unsigned int flags; }; +struct remote_domain_set_block_io_throttle_args { + remote_nonnull_domain dom; + remote_nonnull_string disk; + remote_typed_param params<REMOTE_DOMAIN_BLKIOTHROTTLE_PARAMETERS_MAX>; + unsigned int flags; +}; + +struct remote_domain_get_block_io_throttle_args { + remote_nonnull_domain dom; + remote_nonnull_string disk; + int nparams; + unsigned int flags; +}; + +struct remote_domain_get_block_io_throttle_ret { + remote_typed_param params<REMOTE_DOMAIN_BLKIOTHROTTLE_PARAMETERS_MAX>; + int nparams; +}; + /* Network calls: */ struct remote_num_of_networks_ret { @@ -2564,7 +2586,10 @@ enum remote_procedure { REMOTE_PROC_DOMAIN_SNAPSHOT_NUM_CHILDREN = 246, /* autogen autogen priority:high */ REMOTE_PROC_DOMAIN_SNAPSHOT_LIST_CHILDREN_NAMES = 247, /* autogen autogen priority:high */ REMOTE_PROC_DOMAIN_EVENT_DISK_CHANGE = 248, /* skipgen skipgen */ - REMOTE_PROC_DOMAIN_OPEN_GRAPHICS = 249 /* skipgen skipgen */ + REMOTE_PROC_DOMAIN_OPEN_GRAPHICS = 249, /* skipgen skipgen */ + REMOTE_PROC_DOMAIN_SET_BLOCK_IO_THROTTLE = 250, /* skipgen skipgen */ + REMOTE_PROC_DOMAIN_GET_BLOCK_IO_THROTTLE = 251 /* skipgen skipgen */ + /* * Notice how the entries are grouped in sets of 10 ? diff --git a/src/remote_protocol-structs b/src/remote_protocol-structs index b460b77..eabf5e5 100644 --- a/src/remote_protocol-structs +++ b/src/remote_protocol-structs @@ -759,6 +759,28 @@ struct remote_domain_block_pull_args { uint64_t bandwidth; u_int flags; }; +struct remote_domain_set_block_io_throttle_args { + remote_nonnull_domain dom; + remote_nonnull_string disk; + struct { + u_int params_len; + remote_typed_param * params_val; + } params; + u_int flags; +}; +struct remote_domain_get_block_io_throttle_args { + remote_nonnull_domain dom; + remote_nonnull_string disk; + int nparams; + u_int flags; +}; +struct remote_domain_get_block_io_throttle_ret { + struct { + u_int params_len; + remote_typed_param * params_val; + } params; + int nparams; +}; struct remote_num_of_networks_ret { int num; }; @@ -2007,4 +2029,6 @@ enum remote_procedure { REMOTE_PROC_DOMAIN_SNAPSHOT_LIST_CHILDREN_NAMES = 247, REMOTE_PROC_DOMAIN_EVENT_DISK_CHANGE = 248, REMOTE_PROC_DOMAIN_OPEN_GRAPHICS = 249, + REMOTE_PROC_DOMAIN_SET_BLOCK_IO_THROTTLE = 250, + REMOTE_PROC_DOMAIN_GET_BLOCK_IO_THROTTLE = 251, }; -- 1.7.1

2 1

[libvirt] [PATCH] Give correct error message when configure a wrong URI aliase.
by taget＠linux.vnet.ibm.com 23 Nov '11

23 Nov '11

From: Eli Qiao <taget(a)linux.vnet.ibm.com> Signed-off-by: Eli Qiao <taget(a)linux.vnet.ibm.com> When configure the URI aliase like this in 'libvirt.conf': uri_aliases = [ "jj#j=qemu+ssh://root@127.0.0.1/system", "sleet=qemu+ssh://root@sleet.cloud.example.com/system", ] virsh -c jj#j It will show this error message: 'no connection driver available for No connection for URI jj#j' Actually ��we expect this message below�� Malformed 'uri_aliases' config entry 'jj#j=qemu+ssh://root@127.0.0.1/system', aliases may only container 'a-Z, 0-9, _, -' Give this patch to fix this error. --- src/libvirt.c | 5 ----- 1 files changed, 0 insertions(+), 5 deletions(-) diff --git a/src/libvirt.c b/src/libvirt.c index 1518ed2..17e073e 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -1054,11 +1054,6 @@ virConnectOpenResolveURIAlias(const char *alias, char **uri) *uri = NULL; - /* Short circuit to avoid doing URI alias resolution - * when it clearly isn't an valid alias */ - if (strspn(alias, URI_ALIAS_CHARS) != strlen(alias)) - return 0; - if (!(config = virConnectConfigFile())) goto cleanup; -- 1.7.4.4

6 6

[libvirt] [PATCH 1/8] Add new API virDomain{Set, Get}BlockIoTune
by Lei Li 23 Nov '11

23 Nov '11

This patch add new pulic API virDomainSetBlockIoTune and virDomainGetBlockIoTune. Signed-off-by: Lei Li <lilei(a)linux.vnet.ibm.com> Signed-off-by: Zhi Yong Wu <wuzhy(a)linux.vnet.ibm.com> --- include/libvirt/libvirt.h.in | 63 +++++++++++++++++++ src/driver.h | 20 ++++++ src/libvirt.c | 141 ++++++++++++++++++++++++++++++++++++++++++ src/libvirt_public.syms | 6 ++ 4 files changed, 230 insertions(+), 0 deletions(-) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 2ab89f5..dfa8f76 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -1671,6 +1671,69 @@ int virDomainBlockPull(virDomainPtr dom, const char *path, unsigned long bandwidth, unsigned int flags); +/* Block I/O throttling support */ + +/** + * VIR_DOMAIN_BLOCK_IOTUNE_TOTAL_BYTES_SEC: + * + * Macro for the BlockIoTune tunable weight: it represents the total + * bytes per second permitted through a block device, as a ullong. + */ +#define VIR_DOMAIN_BLOCK_IOTUNE_TOTAL_BYTES_SEC "total_bytes_sec" + +/** + * VIR_DOMAIN_BLOCK_IOTUNE_READ_BYTES_SEC: + * + * Macro for the BlockIoTune tunable weight: it repersents the read + * bytes per second permitted through a block device, as a ullong. + */ +#define VIR_DOMAIN_BLOCK_IOTUNE_READ_BYTES_SEC "read_bytes_sec" + +/** + * VIR_DOMAIN_BLOCK_IOTUNE_WRITE_BYTES_SEC: + * + * Macro for the BlockIoTune tunable weight: it repersents the write + * bytes per second permitted through a block device, as a ullong. + */ +#define VIR_DOMAIN_BLOCK_IOTUNE_WRITE_BYTES_SEC "write_bytes_sec" + +/** + * VIR_DOMAIN_BLOCK_IOTUNE_TOTAL_IOPS_SEC: + * + * Macro for the BlockIoTune tunable weight: it repersents the total + * I/O operations per second permitted through a block device, as a ullong. + */ +#define VIR_DOMAIN_BLOCK_IOTUNE_TOTAL_IOPS_SEC "total_iops_sec" + +/** + * VIR_DOMAIN_BLOCK_IOTUNE_READ_IOPS_SEC: + * + * Macro for the BlockIoTune tunable weight: it repersents the read + * I/O operations per second permitted through a block device, as a ullong. + */ +#define VIR_DOMAIN_BLOCK_IOTUNE_READ_IOPS_SEC "read_iops_sec" + +/** + * VIR_DOMAIN_BLOCK_IOTUNE_WRITE_IOPS_SEC: + * Macro for the BlockIoTune tunable weight: it repersents the write + * I/O operations per second permitted through a block device, as a ullong. + */ +#define VIR_DOMAIN_BLOCK_IOTUNE_WRITE_IOPS_SEC "write_iops_sec" + +int +virDomainSetBlockIoTune(virDomainPtr dom, + const char *disk, + virTypedParameterPtr params, + int nparams, + unsigned int flags); +int +virDomainGetBlockIoTune(virDomainPtr dom, + const char *disk, + virTypedParameterPtr params, + int *nparams, + unsigned int flags); + + /* * NUMA support */ diff --git a/src/driver.h b/src/driver.h index 4c14aaa..23e96d9 100644 --- a/src/driver.h +++ b/src/driver.h @@ -740,6 +740,24 @@ typedef int (*virDrvDomainBlockPull)(virDomainPtr dom, const char *path, unsigned long bandwidth, unsigned int flags); +/* + * Block I/O throttling support + */ + +typedef int + (*virDrvDomainSetBlockIoTune)(virDomainPtr dom, + const char *disk, + virTypedParameterPtr params, + int nparams, + unsigned int flags); + +typedef int + (*virDrvDomainGetBlockIoTune)(virDomainPtr dom, + const char *disk, + virTypedParameterPtr params, + int *nparams, + unsigned int flags); + /** * _virDriver: @@ -899,6 +917,8 @@ struct _virDriver { virDrvDomainGetBlockJobInfo domainGetBlockJobInfo; virDrvDomainBlockJobSetSpeed domainBlockJobSetSpeed; virDrvDomainBlockPull domainBlockPull; + virDrvDomainSetBlockIoTune domainSetBlockIoTune; + virDrvDomainGetBlockIoTune domainGetBlockIoTune; }; typedef int diff --git a/src/libvirt.c b/src/libvirt.c index 1518ed2..2a39958 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -17149,3 +17149,144 @@ error: virDispatchError(dom->conn); return -1; } + +/** + * virDomainSetBlockIoTune: + * @dom: pointer to domain object + * @disk: Fully-qualified disk name (or device shothand name) + * @params: Pointer to blkio parameter objects + * @nparams: Number of blkio parameters (this value can be the same or + * less than the number of parameters supported) + * @flags: An OR'ed set of virDomainModificationImpact + * + * Change all or a subset of the per-device block IO tunables. + * + * The @disk parameter is the name of the block device. Get this + * by calling virDomainGetXMLDesc and finding the <target dev='...'> + * attribute within //domain/devices/disk. (For example, "xvda"). + * + * Returns -1 in case of error, 0 in case of success. + */ +int virDomainSetBlockIoTune(virDomainPtr dom, + const char *disk, + virTypedParameterPtr params, + int nparams, + unsigned int flags) +{ + virConnectPtr conn; + + VIR_DOMAIN_DEBUG(dom, "disk=%p, params=%p, nparams=%d, flags=%x", + disk, params, nparams, flags); + + virResetLastError(); + + if (!VIR_IS_CONNECTED_DOMAIN (dom)) { + virLibDomainError(VIR_ERR_INVALID_DOMAIN, __FUNCTION__); + virDispatchError(NULL); + return -1; + } + + if (dom->conn->flags & VIR_CONNECT_RO) { + virLibDomainError(VIR_ERR_OPERATION_DENIED, __FUNCTION__); + goto error; + } + + if (!disk || (nparams <= 0) || (params == NULL)) { + virLibDomainError(VIR_ERR_INVALID_ARG, __FUNCTION__); + goto error; + } + + if (virTypedParameterValidateSet(dom, params, nparams) < 0) + return -1; + + conn = dom->conn; + + if (conn->driver->domainSetBlockIoTune) { + int ret; + ret = conn->driver->domainSetBlockIoTune(dom, disk, params, nparams, flags); + if (ret < 0) + goto error; + return ret; + } + + virLibDomainError(VIR_ERR_NO_SUPPORT, __FUNCTION__); + +error: + virDispatchError(dom->conn); + return -1; +} + +/** + * virDomainGetBlockIoTune: + * @dom: pointer to domain object + * @disk: Fully-qualified disk name(or device shothand name) + * @params: Pointer to blkio parameter object + * (return value, allocated by the caller) + * @nparams: Pointer to number of blkio parameters + * @flags: An OR'ed set of virDomainModificationImpact + * + * Get all block IO tunable parameters for a given device. On input, + * @nparams gives the size of the @params array; on output, @nparams + * gives how many slots were filled with parameter information, which + * might be less but will not exceed the input value. + * + * As a special case, calling with @params as NULL and @nparams as 0 on + * input will cause @nparams on output to contain the number of parameters + * supported by the hypervisor. The caller should then allocate @params + * array, i.e. (sizeof(@virTypedParameter) * @nparams) bytes and call the API + * agiain. + * + * See virDomainGetMemoryParameters() for an equivalent usage example. + * + * The @disk parameter is the name of the block device. Get this + * by calling virDomainGetXMLDesc and finding the <target dev='...'> + * attribute within //domain/devices/disk. (For example, "xvda"). + * + * Returns -1 in case of error, 0 in case of success. + */ +int virDomainGetBlockIoTune(virDomainPtr dom, + const char *disk, + virTypedParameterPtr params, + int *nparams, + unsigned int flags) +{ + virConnectPtr conn; + + VIR_DOMAIN_DEBUG(dom, "disk=%p, params=%p, nparams=%d, flags=%x", + disk, params, (nparams) ? *nparams : -1, flags); + + virResetLastError(); + + if (!VIR_IS_CONNECTED_DOMAIN (dom)) { + virLibDomainError(VIR_ERR_INVALID_DOMAIN, __FUNCTION__); + virDispatchError(NULL); + return -1; + } + + if (!disk || (nparams == NULL) || (*nparams < 0) || + (params == NULL && *nparams != 0)) { + virLibDomainError(VIR_ERR_INVALID_ARG, __FUNCTION__); + goto error; + } + + if (VIR_DRV_SUPPORTS_FEATURE(dom->conn->driver, dom->conn, + VIR_DRV_FEATURE_TYPED_PARAM_STRING)) + flags |= VIR_TYPED_PARAM_STRING_OKAY; + + conn = dom->conn; + + if (conn->driver->domainGetBlockIoTune) { + int ret; + ret = conn->driver->domainGetBlockIoTune(dom, disk, params, nparams, flags); + if (ret < 0) + goto error; + return ret; + } + + virLibDomainError(VIR_ERR_NO_SUPPORT, __FUNCTION__); + +error: + virDispatchError(dom->conn); + return -1; +} + diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms index bcefb10..29e34d7 100644 --- a/src/libvirt_public.syms +++ b/src/libvirt_public.syms @@ -498,4 +498,10 @@ LIBVIRT_0.9.7 { virDomainSnapshotNumChildren; } LIBVIRT_0.9.5; +LIBVIRT_0.9.8 { + global: + virDomainSetBlockIoTune; + virDomainGetBlockIoTune; +} LIBVIRT_0.9.7; + # .... define new API here using predicted next version number .... -- 1.7.1

2 1

[libvirt] [PATCH v2] nwfilter: Enable detection of multiple IP addresses
by Stefan Berger 22 Nov '11

22 Nov '11

In preparation of DHCP Snooping and the detection of multiple IP addresses per interface: The hash table that is used to collect the detected IP address of an interface can so far only handle one IP address per interface. With this patch we extend this to allow it to handle a list of IP addresses. Above changes the returned variable type of virNWFilterGetIpAddrForIfname() from char * to virNWFilterVarValuePtr; adapt all existing functions calling this function. --- v2: - fixed logic bug pointed out by Eric Blake - sorted symbols --- src/conf/nwfilter_params.c | 62 ++++++++++++++++++-- src/conf/nwfilter_params.h | 7 +- src/libvirt_private.syms | 6 + src/nwfilter/nwfilter_gentech_driver.c | 21 ++---- src/nwfilter/nwfilter_gentech_driver.h | 2 src/nwfilter/nwfilter_learnipaddr.c | 100 ++++++++++++++++++++++++++------- src/nwfilter/nwfilter_learnipaddr.h | 6 + 7 files changed, 158 insertions(+), 46 deletions(-) Index: libvirt-acl/src/nwfilter/nwfilter_learnipaddr.c =================================================================== --- libvirt-acl.orig/src/nwfilter/nwfilter_learnipaddr.c +++ libvirt-acl/src/nwfilter/nwfilter_learnipaddr.c @@ -310,41 +310,99 @@ virNWFilterDeregisterLearnReq(int ifinde return res; } - - +/* Add an IP address to the list of IP addresses an interface is + * known to use. This function feeds the per-interface cache that + * is used to instantiate filters with variable '$IP'. + * + * @ifname: The name of the (tap) interface + * @addr: An IPv4 address in dotted decimal format that the (tap) + * interface is known to use. + * + * This function returns 0 on success, -1 otherwise + */ static int -virNWFilterAddIpAddrForIfname(const char *ifname, char *addr) { +virNWFilterAddIpAddrForIfname(const char *ifname, char *addr) +{ int ret; - virNWFilterVarValuePtr val = virNWFilterVarValueCreateSimple(addr); - - if (!val) - return 1; + virNWFilterVarValuePtr val; virMutexLock(&ipAddressMapLock); - ret = virNWFilterHashTablePut(ipAddressMap, ifname, val, 1); + val = virHashLookup(ipAddressMap->hashTable, ifname); + if (!val) { + val = virNWFilterVarValueCreateSimple(addr); + if (!val) { + virReportOOMError(); + ret = -1; + goto err_exit; + } + ret = virNWFilterHashTablePut(ipAddressMap, ifname, val, 1); + } else { + if (virNWFilterVarValueAddValue(val, addr) < 0) + ret = -1; + } +err_exit: virMutexUnlock(&ipAddressMapLock); return ret; } #endif - -void -virNWFilterDelIpAddrForIfname(const char *ifname) { +/* Delete all or a specific IP address from an interface. After this + * call either all or the given IP address will not be associated + * with the interface anymore. + * + * @ifname: The name of the (tap) interface + * @addr: An IPv4 address in dotted decimal format that the (tap) + * interface is not using anymore; provide NULL to remove all IP + * addresses associated with the given interface + * + * This function returns the number of IP addresses that are still + * known to be associated with this interface, in case of an error + * -1 is returned. Error conditions are: + * - IP addresses is not known to be associated with the interface + */ +int +virNWFilterDelIpAddrForIfname(const char *ifname, const char *ipaddr) +{ + int ret = -1; + virNWFilterVarValuePtr val = NULL; virMutexLock(&ipAddressMapLock); - if (virHashLookup(ipAddressMap->hashTable, ifname)) - virNWFilterHashTableRemoveEntry(ipAddressMap, ifname); + if (ipaddr != NULL) { + val = virHashLookup(ipAddressMap->hashTable, ifname); + if (val) { + if (virNWFilterVarValueGetCardinality(val) == 1 && + STREQ(ipaddr, + virNWFilterVarValueGetNthValue(val, 0))) + goto remove_entry; + virNWFilterVarValueDelValue(val, ipaddr); + ret = virNWFilterVarValueGetCardinality(val); + } + } else { +remove_entry: + /* remove whole entry */ + val = virNWFilterHashTableRemoveEntry(ipAddressMap, ifname); + virNWFilterVarValueFree(val); + ret = 0; + } virMutexUnlock(&ipAddressMapLock); -} + return ret; +} -const char * -virNWFilterGetIpAddrForIfname(const char *ifname) { +/* Get the list of IP addresses known to be in use by an interface + * + * This function returns NULL in case no IP address is known to be + * associated with the interface, a virNWFilterVarValuePtr otherwise + * that then can contain one or multiple entries. + */ +virNWFilterVarValuePtr +virNWFilterGetIpAddrForIfname(const char *ifname) +{ virNWFilterVarValuePtr res; virMutexLock(&ipAddressMapLock); @@ -353,10 +411,7 @@ virNWFilterGetIpAddrForIfname(const char virMutexUnlock(&ipAddressMapLock); - if (res) - return virNWFilterVarValueGetSimple(res); - - return NULL; + return res; } @@ -642,7 +697,10 @@ learnIPAddressThread(void *arg) char *inetaddr; if ((inetaddr = virSocketAddrFormat(&sa))!= NULL) { - virNWFilterAddIpAddrForIfname(req->ifname, inetaddr); + if (virNWFilterAddIpAddrForIfname(req->ifname, inetaddr) < 0) { + VIR_ERROR(_("Failed to add IP address %s to IP address " + "cache for interface %s"), inetaddr, req->ifname); + } ret = virNWFilterInstantiateFilterLate(NULL, req->ifname, Index: libvirt-acl/src/libvirt_private.syms =================================================================== --- libvirt-acl.orig/src/libvirt_private.syms +++ libvirt-acl/src/libvirt_private.syms @@ -846,8 +846,14 @@ virNWFilterVarCombIterCreate; virNWFilterVarCombIterFree; virNWFilterVarCombIterGetVarValue; virNWFilterVarCombIterNext; +virNWFilterVarValueAddValue; +virNWFilterVarValueCopy; virNWFilterVarValueCreateSimple; virNWFilterVarValueCreateSimpleCopyValue; +virNWFilterVarValueDelValue; +virNWFilterVarValueFree; +virNWFilterVarValueGetCardinality; +virNWFilterVarValueGetNthValue; virNWFilterVarValueGetSimple; Index: libvirt-acl/src/conf/nwfilter_params.c =================================================================== --- libvirt-acl.orig/src/conf/nwfilter_params.c +++ libvirt-acl/src/conf/nwfilter_params.c @@ -37,7 +37,7 @@ static bool isValidVarValue(const char *value); -static void +void virNWFilterVarValueFree(virNWFilterVarValuePtr val) { unsigned i; @@ -60,7 +60,7 @@ virNWFilterVarValueFree(virNWFilterVarVa VIR_FREE(val); } -static virNWFilterVarValuePtr +virNWFilterVarValuePtr virNWFilterVarValueCopy(const virNWFilterVarValuePtr val) { virNWFilterVarValuePtr res; @@ -222,6 +222,56 @@ virNWFilterVarValueAddValue(virNWFilterV return rc; } +static int +virNWFilterVarValueDelNthValue(virNWFilterVarValuePtr val, unsigned int pos) +{ + switch (val->valType) { + case NWFILTER_VALUE_TYPE_SIMPLE: + return -1; + + case NWFILTER_VALUE_TYPE_ARRAY: + if (pos < val->u.array.nValues) { + VIR_FREE(val->u.array.values[pos]); + val->u.array.nValues--; + + if (pos < val->u.array.nValues) + memmove(&val->u.array.values[pos], + &val->u.array.values[pos + 1], + sizeof(val->u.array.values[0]) * + (val->u.array.nValues - pos)); + return 0; + } + break; + + case NWFILTER_VALUE_TYPE_LAST: + break; + } + + return -1; +} + +int +virNWFilterVarValueDelValue(virNWFilterVarValuePtr val, const char *value) +{ + unsigned int i; + + switch (val->valType) { + case NWFILTER_VALUE_TYPE_SIMPLE: + return -1; + + case NWFILTER_VALUE_TYPE_ARRAY: + for (i = 0; i < val->u.array.nValues; i++) + if (STREQ(value, val->u.array.values[i])) + return virNWFilterVarValueDelNthValue(val, i); + break; + + case NWFILTER_VALUE_TYPE_LAST: + break; + } + + return -1; +} + void virNWFilterVarCombIterFree(virNWFilterVarCombIterPtr ci) { @@ -521,14 +571,14 @@ virNWFilterHashTableCreate(int n) { } -int +void * virNWFilterHashTableRemoveEntry(virNWFilterHashTablePtr ht, const char *entry) { int i; - int rc = virHashRemoveEntry(ht->hashTable, entry); + void *value = virHashSteal(ht->hashTable, entry); - if (rc == 0) { + if (value) { for (i = 0; i < ht->nNames; i++) { if (STREQ(ht->names[i], entry)) { VIR_FREE(ht->names[i]); @@ -538,7 +588,7 @@ virNWFilterHashTableRemoveEntry(virNWFil } } } - return rc; + return value; } Index: libvirt-acl/src/conf/nwfilter_params.h =================================================================== --- libvirt-acl.orig/src/conf/nwfilter_params.h +++ libvirt-acl/src/conf/nwfilter_params.h @@ -50,11 +50,14 @@ struct _virNWFilterVarValue { virNWFilterVarValuePtr virNWFilterVarValueCreateSimple(char *); virNWFilterVarValuePtr virNWFilterVarValueCreateSimpleCopyValue(const char *); +virNWFilterVarValuePtr virNWFilterVarValueCopy(const virNWFilterVarValuePtr); +void virNWFilterVarValueFree(virNWFilterVarValuePtr val); const char *virNWFilterVarValueGetSimple(const virNWFilterVarValuePtr val); const char *virNWFilterVarValueGetNthValue(virNWFilterVarValuePtr val, unsigned int idx); unsigned int virNWFilterVarValueGetCardinality(const virNWFilterVarValuePtr); int virNWFilterVarValueAddValue(virNWFilterVarValuePtr val, char *value); +int virNWFilterVarValueDelValue(virNWFilterVarValuePtr val, const char *value); typedef struct _virNWFilterHashTable virNWFilterHashTable; typedef virNWFilterHashTable *virNWFilterHashTablePtr; @@ -77,8 +80,8 @@ int virNWFilterHashTablePut(virNWFilterH const char *name, virNWFilterVarValuePtr val, int freeName); -int virNWFilterHashTableRemoveEntry(virNWFilterHashTablePtr table, - const char *name); +void *virNWFilterHashTableRemoveEntry(virNWFilterHashTablePtr table, + const char *name); int virNWFilterHashTablePutAll(virNWFilterHashTablePtr src, virNWFilterHashTablePtr dest); Index: libvirt-acl/src/nwfilter/nwfilter_gentech_driver.h =================================================================== --- libvirt-acl.orig/src/nwfilter/nwfilter_gentech_driver.h +++ libvirt-acl/src/nwfilter/nwfilter_gentech_driver.h @@ -61,7 +61,7 @@ int virNWFilterInstantiateFilterLate(vir int virNWFilterTeardownFilter(const virDomainNetDefPtr net); virNWFilterHashTablePtr virNWFilterCreateVarHashmap(char *macaddr, - char *ipaddr); + const virNWFilterVarValuePtr); void virNWFilterDomainFWUpdateCB(void *payload, const void *name, Index: libvirt-acl/src/nwfilter/nwfilter_gentech_driver.c =================================================================== --- libvirt-acl.orig/src/nwfilter/nwfilter_gentech_driver.c +++ libvirt-acl/src/nwfilter/nwfilter_gentech_driver.c @@ -145,7 +145,7 @@ virNWFilterRuleInstFree(virNWFilterRuleI static int virNWFilterVarHashmapAddStdValues(virNWFilterHashTablePtr table, char *macaddr, - char *ipaddr) + const virNWFilterVarValuePtr ipaddr) { virNWFilterVarValue *val; @@ -164,7 +164,7 @@ virNWFilterVarHashmapAddStdValues(virNWF } if (ipaddr) { - val = virNWFilterVarValueCreateSimple(ipaddr); + val = virNWFilterVarValueCopy(ipaddr); if (!val) return 1; @@ -194,7 +194,8 @@ virNWFilterVarHashmapAddStdValues(virNWF * is attached to the virConnect object. */ virNWFilterHashTablePtr -virNWFilterCreateVarHashmap(char *macaddr, char *ipaddr) { +virNWFilterCreateVarHashmap(char *macaddr, + const virNWFilterVarValuePtr ipaddr) { virNWFilterHashTablePtr table = virNWFilterHashTableCreate(0); if (!table) { virReportOOMError(); @@ -796,7 +797,7 @@ __virNWFilterInstantiateFilter(virConnec virNWFilterDefPtr filter; char vmmacaddr[VIR_MAC_STRING_BUFLEN] = {0}; char *str_macaddr = NULL; - const char *ipaddr; + virNWFilterVarValuePtr ipaddr; char *str_ipaddr = NULL; techdriver = virNWFilterTechDriverForName(drvname); @@ -836,16 +837,8 @@ __virNWFilterInstantiateFilter(virConnec } ipaddr = virNWFilterGetIpAddrForIfname(ifname); - if (ipaddr) { - str_ipaddr = strdup(ipaddr); - if (!str_ipaddr) { - virReportOOMError(); - rc = 1; - goto err_exit; - } - } - vars1 = virNWFilterCreateVarHashmap(str_macaddr, str_ipaddr); + vars1 = virNWFilterCreateVarHashmap(str_macaddr, ipaddr); if (!vars1) { rc = 1; goto err_exit; @@ -1101,7 +1094,7 @@ _virNWFilterTeardownFilter(const char *i techdriver->allTeardown(ifname); - virNWFilterDelIpAddrForIfname(ifname); + virNWFilterDelIpAddrForIfname(ifname, NULL); virNWFilterUnlockIface(ifname); Index: libvirt-acl/src/nwfilter/nwfilter_learnipaddr.h =================================================================== --- libvirt-acl.orig/src/nwfilter/nwfilter_learnipaddr.h +++ libvirt-acl/src/nwfilter/nwfilter_learnipaddr.h @@ -25,6 +25,8 @@ #ifndef __NWFILTER_LEARNIPADDR_H # define __NWFILTER_LEARNIPADDR_H +# include "conf/nwfilter_params.h" + enum howDetect { DETECT_DHCP = 1, DETECT_STATIC = 2, @@ -63,8 +65,8 @@ int virNWFilterLearnIPAddress(virNWFilte virNWFilterIPAddrLearnReqPtr virNWFilterLookupLearnReq(int ifindex); int virNWFilterTerminateLearnReq(const char *ifname); -void virNWFilterDelIpAddrForIfname(const char *ifname); -const char *virNWFilterGetIpAddrForIfname(const char *ifname); +int virNWFilterDelIpAddrForIfname(const char *ifname, const char *ipaddr); +virNWFilterVarValuePtr virNWFilterGetIpAddrForIfname(const char *ifname); int virNWFilterLockIface(const char *ifname) ATTRIBUTE_RETURN_CHECK; void virNWFilterUnlockIface(const char *ifname);

2 1

[libvirt] nwfilter: Enable detection of multiple IP addresses
by Stefan Berger 22 Nov '11

22 Nov '11

In preparation of DHCP Snooping and the detection of multiple IP addresses per interface: The hash table that is used to collect the detected IP address of an interface can so far only handle one IP address per interface. With this patch we extend this to allow it to handle a list of IP addresses. Above changes the returned variable type of virNWFilterGetIpAddrForIfname() from char * to virNWFilterVarValuePtr; adapt all existing functions calling this function. --- src/conf/nwfilter_params.c | 62 ++++++++++++++++++-- src/conf/nwfilter_params.h | 7 +- src/libvirt_private.syms | 5 + src/nwfilter/nwfilter_gentech_driver.c | 21 ++----- src/nwfilter/nwfilter_gentech_driver.h | 2 src/nwfilter/nwfilter_learnipaddr.c | 98 +++++++++++++++++++++++++-------- src/nwfilter/nwfilter_learnipaddr.h | 6 +- 7 files changed, 155 insertions(+), 46 deletions(-) Index: libvirt-acl/src/nwfilter/nwfilter_learnipaddr.c =================================================================== --- libvirt-acl.orig/src/nwfilter/nwfilter_learnipaddr.c +++ libvirt-acl/src/nwfilter/nwfilter_learnipaddr.c @@ -310,41 +310,97 @@ virNWFilterDeregisterLearnReq(int ifinde return res; } - - +/* Add an IP address to the list of IP addresses an interface is + * known to use. This function feeds the per-interface cache that + * is used to instantiate filters with variable '$IP'. + * + * @ifname: The name of the (tap) interface + * @addr: An IPv4 address in dotted decimal format that the (tap) + * interface is known to use. + * + * This function returns 0 on success, -1 otherwise + */ static int -virNWFilterAddIpAddrForIfname(const char *ifname, char *addr) { +virNWFilterAddIpAddrForIfname(const char *ifname, char *addr) +{ int ret; - virNWFilterVarValuePtr val = virNWFilterVarValueCreateSimple(addr); - - if (!val) - return 1; + virNWFilterVarValuePtr val; virMutexLock(&ipAddressMapLock); - ret = virNWFilterHashTablePut(ipAddressMap, ifname, val, 1); + val = virHashLookup(ipAddressMap->hashTable, ifname); + if (!val) { + val = virNWFilterVarValueCreateSimple(addr); + if (!val) { + virReportOOMError(); + ret = -1; + goto err_exit; + } + ret = virNWFilterHashTablePut(ipAddressMap, ifname, val, 1); + } else { + if (virNWFilterVarValueAddValue(val, addr) < 0) + ret = -1; + } +err_exit: virMutexUnlock(&ipAddressMapLock); return ret; } #endif - -void -virNWFilterDelIpAddrForIfname(const char *ifname) { +/* Delete all or a specific IP address from an interface. + * + * @ifname: The name of the (tap) interface + * @addr: An IPv4 address in dotted decimal format that the (tap) + * interface is not using anymore; provide NULL to remove all IP + * addresses associated with the given interface + * + * This function returns the number of IP addresses that are still + * known to be associated with this interface, in case of an error + * -1 is returned. Error conditions are: + * - no IP addresses is known to be associated with an interface + */ +int +virNWFilterDelIpAddrForIfname(const char *ifname, const char *ipaddr) +{ + int ret = -1; + virNWFilterVarValuePtr val = NULL; virMutexLock(&ipAddressMapLock); - if (virHashLookup(ipAddressMap->hashTable, ifname)) - virNWFilterHashTableRemoveEntry(ipAddressMap, ifname); + if (ipaddr != NULL) { + val = virHashLookup(ipAddressMap->hashTable, ifname); + if (val) { + if (virNWFilterVarValueGetCardinality(val) == 1) + goto remove_entry; + virNWFilterVarValueDelValue(val, ipaddr); + ret = virNWFilterVarValueGetCardinality(val); + } + } else { +remove_entry: + /* remove whole entry */ + val = virNWFilterHashTableRemoveEntry(ipAddressMap, ifname); + if (val) { + ret = 0; + virNWFilterVarValueFree(val); + } + } virMutexUnlock(&ipAddressMapLock); -} + return ret; +} -const char * -virNWFilterGetIpAddrForIfname(const char *ifname) { +/* Get the list of IP addresses known to be in use by an interface + * + * This function returns NULL in case no IP address is known to be + * associated with the interface, a virNWFilterVarValuePtr otherwise + * that then can contain one or multiple entries. + */ +virNWFilterVarValuePtr +virNWFilterGetIpAddrForIfname(const char *ifname) +{ virNWFilterVarValuePtr res; virMutexLock(&ipAddressMapLock); @@ -353,10 +409,7 @@ virNWFilterGetIpAddrForIfname(const char virMutexUnlock(&ipAddressMapLock); - if (res) - return virNWFilterVarValueGetSimple(res); - - return NULL; + return res; } @@ -642,7 +695,10 @@ learnIPAddressThread(void *arg) char *inetaddr; if ((inetaddr = virSocketAddrFormat(&sa))!= NULL) { - virNWFilterAddIpAddrForIfname(req->ifname, inetaddr); + if (virNWFilterAddIpAddrForIfname(req->ifname, inetaddr) < 0) { + VIR_ERROR("Failed to add IP address %s to IP address cache " + "for interface %s", inetaddr, req->ifname); + } ret = virNWFilterInstantiateFilterLate(NULL, req->ifname, Index: libvirt-acl/src/libvirt_private.syms =================================================================== --- libvirt-acl.orig/src/libvirt_private.syms +++ libvirt-acl/src/libvirt_private.syms @@ -846,9 +846,14 @@ virNWFilterVarCombIterCreate; virNWFilterVarCombIterFree; virNWFilterVarCombIterGetVarValue; virNWFilterVarCombIterNext; +virNWFilterVarValueAddValue; +virNWFilterVarValueCopy; virNWFilterVarValueCreateSimple; virNWFilterVarValueCreateSimpleCopyValue; +virNWFilterVarValueDelValue; +virNWFilterVarValueFree; virNWFilterVarValueGetSimple; +virNWFilterVarValueGetCardinality; # pci.h Index: libvirt-acl/src/conf/nwfilter_params.c =================================================================== --- libvirt-acl.orig/src/conf/nwfilter_params.c +++ libvirt-acl/src/conf/nwfilter_params.c @@ -37,7 +37,7 @@ static bool isValidVarValue(const char *value); -static void +void virNWFilterVarValueFree(virNWFilterVarValuePtr val) { unsigned i; @@ -60,7 +60,7 @@ virNWFilterVarValueFree(virNWFilterVarVa VIR_FREE(val); } -static virNWFilterVarValuePtr +virNWFilterVarValuePtr virNWFilterVarValueCopy(const virNWFilterVarValuePtr val) { virNWFilterVarValuePtr res; @@ -222,6 +222,56 @@ virNWFilterVarValueAddValue(virNWFilterV return rc; } +static int +virNWFilterVarValueDelNthValue(virNWFilterVarValuePtr val, unsigned int pos) +{ + switch (val->valType) { + case NWFILTER_VALUE_TYPE_SIMPLE: + return -1; + + case NWFILTER_VALUE_TYPE_ARRAY: + if (pos < val->u.array.nValues) { + VIR_FREE(val->u.array.values[pos]); + val->u.array.nValues--; + + if (pos < val->u.array.nValues) + memmove(&val->u.array.values[pos], + &val->u.array.values[pos + 1], + sizeof(val->u.array.values[0]) * + (val->u.array.nValues - pos)); + return 0; + } + break; + + case NWFILTER_VALUE_TYPE_LAST: + break; + } + + return -1; +} + +int +virNWFilterVarValueDelValue(virNWFilterVarValuePtr val, const char *value) +{ + unsigned int i; + + switch (val->valType) { + case NWFILTER_VALUE_TYPE_SIMPLE: + return -1; + + case NWFILTER_VALUE_TYPE_ARRAY: + for (i = 0; i < val->u.array.nValues; i++) + if (STREQ(value, val->u.array.values[i])) + return virNWFilterVarValueDelNthValue(val, i); + break; + + case NWFILTER_VALUE_TYPE_LAST: + break; + } + + return -1; +} + void virNWFilterVarCombIterFree(virNWFilterVarCombIterPtr ci) { @@ -521,14 +571,14 @@ virNWFilterHashTableCreate(int n) { } -int +void * virNWFilterHashTableRemoveEntry(virNWFilterHashTablePtr ht, const char *entry) { int i; - int rc = virHashRemoveEntry(ht->hashTable, entry); + void *value = virHashSteal(ht->hashTable, entry); - if (rc == 0) { + if (value) { for (i = 0; i < ht->nNames; i++) { if (STREQ(ht->names[i], entry)) { VIR_FREE(ht->names[i]); @@ -538,7 +588,7 @@ virNWFilterHashTableRemoveEntry(virNWFil } } } - return rc; + return value; } Index: libvirt-acl/src/conf/nwfilter_params.h =================================================================== --- libvirt-acl.orig/src/conf/nwfilter_params.h +++ libvirt-acl/src/conf/nwfilter_params.h @@ -50,11 +50,14 @@ struct _virNWFilterVarValue { virNWFilterVarValuePtr virNWFilterVarValueCreateSimple(char *); virNWFilterVarValuePtr virNWFilterVarValueCreateSimpleCopyValue(const char *); +virNWFilterVarValuePtr virNWFilterVarValueCopy(const virNWFilterVarValuePtr); +void virNWFilterVarValueFree(virNWFilterVarValuePtr val); const char *virNWFilterVarValueGetSimple(const virNWFilterVarValuePtr val); const char *virNWFilterVarValueGetNthValue(virNWFilterVarValuePtr val, unsigned int idx); unsigned int virNWFilterVarValueGetCardinality(const virNWFilterVarValuePtr); int virNWFilterVarValueAddValue(virNWFilterVarValuePtr val, char *value); +int virNWFilterVarValueDelValue(virNWFilterVarValuePtr val, const char *value); typedef struct _virNWFilterHashTable virNWFilterHashTable; typedef virNWFilterHashTable *virNWFilterHashTablePtr; @@ -77,8 +80,8 @@ int virNWFilterHashTablePut(virNWFilterH const char *name, virNWFilterVarValuePtr val, int freeName); -int virNWFilterHashTableRemoveEntry(virNWFilterHashTablePtr table, - const char *name); +void *virNWFilterHashTableRemoveEntry(virNWFilterHashTablePtr table, + const char *name); int virNWFilterHashTablePutAll(virNWFilterHashTablePtr src, virNWFilterHashTablePtr dest); Index: libvirt-acl/src/nwfilter/nwfilter_gentech_driver.h =================================================================== --- libvirt-acl.orig/src/nwfilter/nwfilter_gentech_driver.h +++ libvirt-acl/src/nwfilter/nwfilter_gentech_driver.h @@ -61,7 +61,7 @@ int virNWFilterInstantiateFilterLate(vir int virNWFilterTeardownFilter(const virDomainNetDefPtr net); virNWFilterHashTablePtr virNWFilterCreateVarHashmap(char *macaddr, - char *ipaddr); + const virNWFilterVarValuePtr); void virNWFilterDomainFWUpdateCB(void *payload, const void *name, Index: libvirt-acl/src/nwfilter/nwfilter_gentech_driver.c =================================================================== --- libvirt-acl.orig/src/nwfilter/nwfilter_gentech_driver.c +++ libvirt-acl/src/nwfilter/nwfilter_gentech_driver.c @@ -145,7 +145,7 @@ virNWFilterRuleInstFree(virNWFilterRuleI static int virNWFilterVarHashmapAddStdValues(virNWFilterHashTablePtr table, char *macaddr, - char *ipaddr) + const virNWFilterVarValuePtr ipaddr) { virNWFilterVarValue *val; @@ -164,7 +164,7 @@ virNWFilterVarHashmapAddStdValues(virNWF } if (ipaddr) { - val = virNWFilterVarValueCreateSimple(ipaddr); + val = virNWFilterVarValueCopy(ipaddr); if (!val) return 1; @@ -194,7 +194,8 @@ virNWFilterVarHashmapAddStdValues(virNWF * is attached to the virConnect object. */ virNWFilterHashTablePtr -virNWFilterCreateVarHashmap(char *macaddr, char *ipaddr) { +virNWFilterCreateVarHashmap(char *macaddr, + const virNWFilterVarValuePtr ipaddr) { virNWFilterHashTablePtr table = virNWFilterHashTableCreate(0); if (!table) { virReportOOMError(); @@ -796,7 +797,7 @@ __virNWFilterInstantiateFilter(virConnec virNWFilterDefPtr filter; char vmmacaddr[VIR_MAC_STRING_BUFLEN] = {0}; char *str_macaddr = NULL; - const char *ipaddr; + virNWFilterVarValuePtr ipaddr; char *str_ipaddr = NULL; techdriver = virNWFilterTechDriverForName(drvname); @@ -836,16 +837,8 @@ __virNWFilterInstantiateFilter(virConnec } ipaddr = virNWFilterGetIpAddrForIfname(ifname); - if (ipaddr) { - str_ipaddr = strdup(ipaddr); - if (!str_ipaddr) { - virReportOOMError(); - rc = 1; - goto err_exit; - } - } - vars1 = virNWFilterCreateVarHashmap(str_macaddr, str_ipaddr); + vars1 = virNWFilterCreateVarHashmap(str_macaddr, ipaddr); if (!vars1) { rc = 1; goto err_exit; @@ -1101,7 +1094,7 @@ _virNWFilterTeardownFilter(const char *i techdriver->allTeardown(ifname); - virNWFilterDelIpAddrForIfname(ifname); + virNWFilterDelIpAddrForIfname(ifname, NULL); virNWFilterUnlockIface(ifname); Index: libvirt-acl/src/nwfilter/nwfilter_learnipaddr.h =================================================================== --- libvirt-acl.orig/src/nwfilter/nwfilter_learnipaddr.h +++ libvirt-acl/src/nwfilter/nwfilter_learnipaddr.h @@ -25,6 +25,8 @@ #ifndef __NWFILTER_LEARNIPADDR_H # define __NWFILTER_LEARNIPADDR_H +# include "conf/nwfilter_params.h" + enum howDetect { DETECT_DHCP = 1, DETECT_STATIC = 2, @@ -63,8 +65,8 @@ int virNWFilterLearnIPAddress(virNWFilte virNWFilterIPAddrLearnReqPtr virNWFilterLookupLearnReq(int ifindex); int virNWFilterTerminateLearnReq(const char *ifname); -void virNWFilterDelIpAddrForIfname(const char *ifname); -const char *virNWFilterGetIpAddrForIfname(const char *ifname); +int virNWFilterDelIpAddrForIfname(const char *ifname, const char *ipaddr); +virNWFilterVarValuePtr virNWFilterGetIpAddrForIfname(const char *ifname); int virNWFilterLockIface(const char *ifname) ATTRIBUTE_RETURN_CHECK; void virNWFilterUnlockIface(const char *ifname);

2 2

[libvirt] [PATCH] nwfilter: Pass additional parameter into applyDHCPOnly function
by Stefan Berger 22 Nov '11

22 Nov '11

In preparation for the DHCP Snooping code: Pass an additional parameter into the applyDHCPOnly function of the 'techdriver'. --- src/conf/nwfilter_conf.h | 3 ++- src/nwfilter/nwfilter_ebiptables_driver.c | 13 ++++++++++--- src/nwfilter/nwfilter_learnipaddr.c | 2 +- 3 files changed, 13 insertions(+), 5 deletions(-) Index: libvirt-acl/src/conf/nwfilter_conf.h =================================================================== --- libvirt-acl.orig/src/conf/nwfilter_conf.h +++ libvirt-acl/src/conf/nwfilter_conf.h @@ -630,7 +630,8 @@ typedef int (*virNWFilterApplyBasicRules typedef int (*virNWFilterApplyDHCPOnlyRules)(const char *ifname, const unsigned char *macaddr, - const char *dhcpserver); + const char *dhcpserver, + bool leaveTemporary); typedef int (*virNWFilterRemoveBasicRules)(const char *ifname); Index: libvirt-acl/src/nwfilter/nwfilter_ebiptables_driver.c =================================================================== --- libvirt-acl.orig/src/nwfilter/nwfilter_ebiptables_driver.c +++ libvirt-acl/src/nwfilter/nwfilter_ebiptables_driver.c @@ -3191,6 +3191,9 @@ tear_down_tmpebchains: * interface * @dhcpserver: The DHCP server from which the VM may receive traffic * from; may be NULL + * @leaveTemporary: Whether to leave the table names with their temporary + * names (true) or also perform the renaming to their final names as + * part of this call (false) * * Returns 0 on success, 1 on failure with the rules removed * @@ -3200,7 +3203,8 @@ tear_down_tmpebchains: static int ebtablesApplyDHCPOnlyRules(const char *ifname, const unsigned char *macaddr, - const char *dhcpserver) + const char *dhcpserver, + bool leaveTemporary) { virBuffer buf = VIR_BUFFER_INITIALIZER; char chain_in [MAX_CHAINNAME_LENGTH], @@ -3281,8 +3285,11 @@ ebtablesApplyDHCPOnlyRules(const char *i ebtablesLinkTmpRootChain(&buf, 1, ifname, 1); ebtablesLinkTmpRootChain(&buf, 0, ifname, 1); - ebtablesRenameTmpRootChain(&buf, 1, ifname); - ebtablesRenameTmpRootChain(&buf, 0, ifname); + + if (!leaveTemporary) { + ebtablesRenameTmpRootChain(&buf, 1, ifname); + ebtablesRenameTmpRootChain(&buf, 0, ifname); + } if (ebiptablesExecCLI(&buf, NULL, NULL) < 0) goto tear_down_tmpebchains; Index: libvirt-acl/src/nwfilter/nwfilter_learnipaddr.c =================================================================== --- libvirt-acl.orig/src/nwfilter/nwfilter_learnipaddr.c +++ libvirt-acl/src/nwfilter/nwfilter_learnipaddr.c @@ -460,7 +460,7 @@ learnIPAddressThread(void *arg) case DETECT_DHCP: if (techdriver->applyDHCPOnlyRules(req->ifname, req->macaddr, - NULL)) { + NULL, false)) { req->status = EINVAL; goto done; }

2 1