- Devel - libvirt List Archives

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

23 Nov '11

Here is the 3rd version of my libvirt-gconfig patches, I did the renaming to put all the new classes it introduces in the GVirConfigDomain namespace. GVirConfigDomainSnapshot looks a bit out of place now though... Apart from this, I fixed the other issues that were reported, and rearranged a bit the patches introducing GVirConfigDomainInterfaceNetwork. I've also pushed the first 5 patches that were in v2 since they've already been ack'ed Christophe

2 54

[libvirt] [PATCH] qemu: fix a const-correctness issue
by Eric Blake 23 Nov '11

23 Nov '11

Generally, functions which return malloc'd strings should be typed as 'char *', not 'const char *', to make it obvious that the caller is responsible to free things. free(const char *) fails to compile, and although we have a cast embedded in VIR_FREE to work around poor code that frees const char *, it's better to not rely on that hack. * src/qemu/qemu_driver.c (qemuDiskPathToAlias): Change return type. (qemuDomainBlockJobImpl): Update callers. --- Pushing under the trivial rule. src/qemu/qemu_driver.c | 7 ++++--- 1 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 98ce695..94fbe94 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -10565,8 +10565,9 @@ cleanup: return ret; } -static const char * -qemuDiskPathToAlias(virDomainObjPtr vm, const char *path) { +static char * +qemuDiskPathToAlias(virDomainObjPtr vm, const char *path) +{ int i; char *ret = NULL; virDomainDiskDefPtr disk; @@ -10605,7 +10606,7 @@ qemuDomainBlockJobImpl(virDomainPtr dom, const char *path, virDomainObjPtr vm = NULL; qemuDomainObjPrivatePtr priv; char uuidstr[VIR_UUID_STRING_BUFLEN]; - const char *device = NULL; + char *device = NULL; int ret = -1; qemuDriverLock(driver); -- 1.7.7.3

1 0

[libvirt] [PATCH 4/8] Support block I/O throtte in XML
by Lei Li 23 Nov '11

23 Nov '11

Enable block I/O throttle for per-disk in XML. Signed-off-by: Lei Li <lilei(a)linux.vnet.ibm.com> Signed-off-by: Zhi Yong Wu <wuzhy(a)linux.vnet.ibm.com> --- src/conf/domain_conf.c | 101 +++++++++++++++++++++++++++++++++++++++++++++- src/conf/domain_conf.h | 12 ++++++ src/qemu/qemu_command.c | 33 +++++++++++++++ src/util/xml.h | 2 + 4 files changed, 145 insertions(+), 3 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 58f4d0f..a157b80 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -2318,7 +2318,8 @@ static virDomainDiskDefPtr virDomainDiskDefParseXML(virCapsPtr caps, xmlNodePtr node, virBitmapPtr bootMap, - unsigned int flags) + unsigned int flags, + xmlXPathContextPtr ctxt) { virDomainDiskDefPtr def; xmlNodePtr cur, child; @@ -2517,6 +2518,62 @@ virDomainDiskDefParseXML(virCapsPtr caps, } child = child->next; } + } else if (xmlStrEqual(cur->name, BAD_CAST "iotune")) { + if (virXPathULongLong("string(./devices/disk/iotune/total_bytes_sec)", + ctxt, &def->blkdeviotune.total_bytes_sec) < 0) { + def->blkdeviotune.total_bytes_sec = 0; + } else { + def->blkdeviotune.mark = 1; + } + + if (virXPathULongLong("string(./devices/disk/iotune/read_bytes_sec)", + ctxt, &def->blkdeviotune.read_bytes_sec) < 0) { + def->blkdeviotune.read_bytes_sec = 0; + } else { + def->blkdeviotune.mark = 1; + } + + if (virXPathULongLong("string(./devices/disk/iotune/write_bytes_sec)", + ctxt, &def->blkdeviotune.write_bytes_sec) < 0) { + def->blkdeviotune.write_bytes_sec = 0; + } else { + def->blkdeviotune.mark = 1; + } + + if (virXPathULongLong("string(./devices/disk/iotune/total_iops_sec)", + ctxt, &def->blkdeviotune.total_iops_sec) < 0) { + def->blkdeviotune.total_iops_sec = 0; + } else { + def->blkdeviotune.mark = 1; + } + + if (virXPathULongLong("string(./devices/disk/iotune/read_iops_sec)", + ctxt, &def->blkdeviotune.read_iops_sec) < 0) { + def->blkdeviotune.read_iops_sec = 0; + } else { + def->blkdeviotune.mark = 1; + } + + if (virXPathULongLong("string(./devices/disk/iotune/write_iops_sec)", + ctxt, &def->blkdeviotune.write_iops_sec) < 0) { + def->blkdeviotune.write_iops_sec = 0; + } else { + def->blkdeviotune.mark = 1; + } + + if ((def->blkdeviotune.total_bytes_sec && def->blkdeviotune.read_bytes_sec) + || (def->blkdeviotune.total_bytes_sec && def->blkdeviotune.write_bytes_sec)) { + virDomainReportError(VIR_ERR_XML_ERROR, + _("total and read/write bytes_sec cannot be set at the same time")); + goto error; + } + + if ((def->blkdeviotune.total_iops_sec && def->blkdeviotune.read_iops_sec) + || (def->blkdeviotune.total_iops_sec && def->blkdeviotune.write_iops_sec)) { + virDomainReportError(VIR_ERR_XML_ERROR, + _("total and read/write iops_sec cannot be set at the same time")); + goto error; + } } else if (xmlStrEqual(cur->name, BAD_CAST "readonly")) { def->readonly = 1; } else if (xmlStrEqual(cur->name, BAD_CAST "shareable")) { @@ -6003,7 +6060,7 @@ virDomainDeviceDefPtr virDomainDeviceDefParse(virCapsPtr caps, if (xmlStrEqual(node->name, BAD_CAST "disk")) { dev->type = VIR_DOMAIN_DEVICE_DISK; if (!(dev->data.disk = virDomainDiskDefParseXML(caps, node, - NULL, flags))) + NULL, flags, NULL))) goto error; } else if (xmlStrEqual(node->name, BAD_CAST "lease")) { dev->type = VIR_DOMAIN_DEVICE_LEASE; @@ -7076,7 +7133,8 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps, virDomainDiskDefPtr disk = virDomainDiskDefParseXML(caps, nodes[i], bootMap, - flags); + flags, + ctxt); if (!disk) goto error; @@ -9511,6 +9569,43 @@ virDomainDiskDefFormat(virBufferPtr buf, virBufferAsprintf(buf, " <target dev='%s' bus='%s'/>\n", def->dst, bus); + /*disk I/O throttling*/ + if (def->blkdeviotune.mark) { + virBufferAddLit(buf, " <iotune>\n"); + if (def->blkdeviotune.total_bytes_sec) { + virBufferAsprintf(buf, " <total_bytes_sec>%llu</total_bytes_sec>\n", + def->blkdeviotune.total_bytes_sec); + } + + if (def->blkdeviotune.read_bytes_sec) { + virBufferAsprintf(buf, " <read_bytes_sec>%llu</read_bytes_sec>\n", + def->blkdeviotune.read_bytes_sec); + + } + + if (def->blkdeviotune.write_bytes_sec) { + virBufferAsprintf(buf, " <write_bytes_sec>%llu</write_bytes_sec>\n", + def->blkdeviotune.write_bytes_sec); + } + + if (def->blkdeviotune.total_iops_sec) { + virBufferAsprintf(buf, " <total_iops_sec>%llu</total_iops_sec>\n", + def->blkdeviotune.total_iops_sec); + } + + if (def->blkdeviotune.read_iops_sec) { + virBufferAsprintf(buf, " <read_iops_sec>%llu</read_iops_sec>", + def->blkdeviotune.read_iops_sec); + } + + if (def->blkdeviotune.write_iops_sec) { + virBufferAsprintf(buf, " <write_iops_sec>%llu</write_iops_sec>", + def->blkdeviotune.write_iops_sec); + } + + virBufferAddLit(buf, " </iotune>\n"); + } + if (def->bootIndex) virBufferAsprintf(buf, " <boot order='%d'/>\n", def->bootIndex); if (def->readonly) diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index a3cb834..d95e239 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -333,6 +333,18 @@ struct _virDomainDiskDef { } auth; char *driverName; char *driverType; + + /*disk I/O throttling*/ + struct { + unsigned long long total_bytes_sec; + unsigned long long read_bytes_sec; + unsigned long long write_bytes_sec; + unsigned long long total_iops_sec; + unsigned long long read_iops_sec; + unsigned long long write_iops_sec; + unsigned int mark; + } blkdeviotune; + char *serial; int cachemode; int error_policy; /* enum virDomainDiskErrorPolicy */ diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 2fbf691..91c6508 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -1690,6 +1690,39 @@ qemuBuildDriveStr(virConnectPtr conn ATTRIBUTE_UNUSED, } } + /*block I/O throttling*/ + if (disk->blkdeviotune.mark) { + if (disk->blkdeviotune.total_bytes_sec) { + virBufferAsprintf(&opt, ",bps=%llu", + disk->blkdeviotune.total_bytes_sec); + } + + if (disk->blkdeviotune.read_bytes_sec) { + virBufferAsprintf(&opt, ",bps_rd=%llu", + disk->blkdeviotune.read_bytes_sec); + } + + if (disk->blkdeviotune.write_bytes_sec) { + virBufferAsprintf(&opt, ",bps_wr=%llu", + disk->blkdeviotune.write_bytes_sec); + } + + if (disk->blkdeviotune.total_iops_sec) { + virBufferAsprintf(&opt, ",iops=%llu", + disk->blkdeviotune.total_iops_sec); + } + + if (disk->blkdeviotune.read_iops_sec) { + virBufferAsprintf(&opt, ",iops_rd=%llu", + disk->blkdeviotune.read_iops_sec); + } + + if (disk->blkdeviotune.write_iops_sec) { + virBufferAsprintf(&opt, ",iops_wr=%llu", + disk->blkdeviotune.write_iops_sec); + } + } + if (virBufferError(&opt)) { virReportOOMError(); goto error; diff --git a/src/util/xml.h b/src/util/xml.h index c492063..5742f19 100644 --- a/src/util/xml.h +++ b/src/util/xml.h @@ -50,6 +50,8 @@ xmlNodePtr virXPathNode(const char *xpath, int virXPathNodeSet(const char *xpath, xmlXPathContextPtr ctxt, xmlNodePtr **list); +int virXMLStringToULongLong (const char *stringval, + unsigned long long *value); char * virXMLPropString(xmlNodePtr node, const char *name); -- 1.7.1

2 2

[libvirt] [PATCH] nwfilter: remove virConnectPtr from internal API calls
by Stefan Berger 23 Nov '11

23 Nov '11

Remove the virConnectPtr from the nwfilter's internal API calls as far as possible. --- src/conf/nwfilter_conf.h | 18 ++----- src/nwfilter/nwfilter_ebiptables_driver.c | 34 ++++---------- src/nwfilter/nwfilter_gentech_driver.c | 72 ++++++++++-------------------- src/nwfilter/nwfilter_gentech_driver.h | 9 +-- src/nwfilter/nwfilter_learnipaddr.c | 3 - 5 files changed, 45 insertions(+), 91 deletions(-) Index: libvirt-acl/src/conf/nwfilter_conf.h =================================================================== --- libvirt-acl.orig/src/conf/nwfilter_conf.h +++ libvirt-acl/src/conf/nwfilter_conf.h @@ -592,27 +592,22 @@ typedef void (*virNWFilterTechDrvShutdow enum virDomainNetType; -typedef int (*virNWFilterRuleCreateInstance)(virConnectPtr conn, - enum virDomainNetType nettype, +typedef int (*virNWFilterRuleCreateInstance)(enum virDomainNetType nettype, virNWFilterDefPtr filter, virNWFilterRuleDefPtr rule, const char *ifname, virNWFilterHashTablePtr vars, virNWFilterRuleInstPtr res); -typedef int (*virNWFilterRuleApplyNewRules)(virConnectPtr conn, - const char *ifname, +typedef int (*virNWFilterRuleApplyNewRules)(const char *ifname, int nruleInstances, void **_inst); -typedef int (*virNWFilterRuleTeardownNewRules)(virConnectPtr conn, - const char *ifname); +typedef int (*virNWFilterRuleTeardownNewRules)(const char *ifname); -typedef int (*virNWFilterRuleTeardownOldRules)(virConnectPtr conn, - const char *ifname); +typedef int (*virNWFilterRuleTeardownOldRules)(const char *ifname); -typedef int (*virNWFilterRuleRemoveRules)(virConnectPtr conn, - const char *ifname, +typedef int (*virNWFilterRuleRemoveRules)(const char *ifname, int nruleInstances, void **_inst); @@ -620,8 +615,7 @@ typedef int (*virNWFilterRuleAllTeardown typedef int (*virNWFilterRuleFreeInstanceData)(void * _inst); -typedef int (*virNWFilterRuleDisplayInstanceData)(virConnectPtr conn, - void *_inst); +typedef int (*virNWFilterRuleDisplayInstanceData)(void *_inst); typedef int (*virNWFilterCanApplyBasicRules)(void); 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 @@ -1188,8 +1188,7 @@ iptablesEnforceDirection(int directionIn * Convert a single rule into its representation for later instantiation * * Returns 0 in case of success with the result stored in the data structure - * pointed to by res, != 0 otherwise with the error message stored in the - * virConnect object. + * pointed to by res, != 0 otherwise. */ static int _iptablesCreateRuleInstance(int directionIn, @@ -1917,8 +1916,7 @@ iptablesCreateRuleInstance(virNWFilterDe * Convert a single rule into its representation for later instantiation * * Returns 0 in case of success with the result stored in the data structure - * pointed to by res, != 0 otherwise with the error message stored in the - * virConnect object. + * pointed to by res, != 0 otherwise. */ static int ebtablesCreateRuleInstance(char chainPrefix, @@ -2503,7 +2501,6 @@ err_exit: /* * ebiptablesCreateRuleInstance: - * @conn : Pointer to a virConnect object * @nwfilter : The filter * @rule: The rule of the filter to convert * @ifname : The name of the interface to apply the rule to @@ -2513,12 +2510,10 @@ err_exit: * Convert a single rule into its representation for later instantiation * * Returns 0 in case of success with the result stored in the data structure - * pointed to by res, != 0 otherwise with the error message stored in the - * virConnect object. + * pointed to by res, != 0 otherwise. */ static int -ebiptablesCreateRuleInstance(virConnectPtr conn ATTRIBUTE_UNUSED, - enum virDomainNetType nettype ATTRIBUTE_UNUSED, +ebiptablesCreateRuleInstance(enum virDomainNetType nettype ATTRIBUTE_UNUSED, virNWFilterDefPtr nwfilter, virNWFilterRuleDefPtr rule, const char *ifname, @@ -2610,7 +2605,6 @@ ebiptablesCreateRuleInstance(virConnectP static int ebiptablesCreateRuleInstanceIterate( - virConnectPtr conn ATTRIBUTE_UNUSED, enum virDomainNetType nettype ATTRIBUTE_UNUSED, virNWFilterDefPtr nwfilter, virNWFilterRuleDefPtr rule, @@ -2630,8 +2624,7 @@ ebiptablesCreateRuleInstanceIterate( return 1; do { - rc = ebiptablesCreateRuleInstance(conn, - nettype, + rc = ebiptablesCreateRuleInstance(nettype, nwfilter, rule, ifname, @@ -2656,8 +2649,7 @@ ebiptablesFreeRuleInstance(void *_inst) static int -ebiptablesDisplayRuleInstance(virConnectPtr conn ATTRIBUTE_UNUSED, - void *_inst) +ebiptablesDisplayRuleInstance(void *_inst) { ebiptablesRuleInstPtr inst = (ebiptablesRuleInstPtr)_inst; VIR_INFO("Command Template: '%s', Needed protocol: '%s'", @@ -3096,7 +3088,6 @@ ebiptablesCanApplyBasicRules(void) { /** * ebtablesApplyBasicRules * - * @conn: virConnect object * @ifname: name of the backend-interface to which to apply the rules * @macaddr: MAC address the VM is using in packets sent through the * interface @@ -3552,8 +3543,7 @@ ebtablesCreateTmpRootAndSubChains(virBuf } static int -ebiptablesApplyNewRules(virConnectPtr conn ATTRIBUTE_UNUSED, - const char *ifname, +ebiptablesApplyNewRules(const char *ifname, int nruleInstances, void **_inst) { @@ -3824,8 +3814,7 @@ exit_free_sets: static int -ebiptablesTearNewRules(virConnectPtr conn ATTRIBUTE_UNUSED, - const char *ifname) +ebiptablesTearNewRules(const char *ifname) { int cli_status; virBuffer buf = VIR_BUFFER_INITIALIZER; @@ -3862,8 +3851,7 @@ ebiptablesTearNewRules(virConnectPtr con static int -ebiptablesTearOldRules(virConnectPtr conn ATTRIBUTE_UNUSED, - const char *ifname) +ebiptablesTearOldRules(const char *ifname) { int cli_status; virBuffer buf = VIR_BUFFER_INITIALIZER; @@ -3911,7 +3899,6 @@ ebiptablesTearOldRules(virConnectPtr con /** * ebiptablesRemoveRules: - * @conn : pointer to virConnect object * @ifname : the name of the interface to which the rules apply * @nRuleInstance : the number of given rules * @_inst : array of rule instantiation data @@ -3922,8 +3909,7 @@ ebiptablesTearOldRules(virConnectPtr con * commands failed. */ static int -ebiptablesRemoveRules(virConnectPtr conn ATTRIBUTE_UNUSED, - const char *ifname ATTRIBUTE_UNUSED, +ebiptablesRemoveRules(const char *ifname ATTRIBUTE_UNUSED, int nruleInstances, void **_inst) { 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 @@ -98,8 +98,7 @@ virNWFilterTechDriverForName(const char * for bidirectional traffic and data needs to be added to the incoming * and outgoing chains. * - * Returns 0 in case of success, 1 in case of an error with the error - * message attached to the virConnect object. + * Returns 0 in case of success, 1 in case of an error. */ int virNWFilterRuleInstAddData(virNWFilterRuleInstPtr res, @@ -190,8 +189,7 @@ virNWFilterVarHashmapAddStdValues(virNWF * Create a hashmap used for evaluating the firewall rules. Initializes * it with the standard variable 'MAC' and 'IP' if provided. * - * Returns pointer to hashmap, NULL if an error occcurred and error message - * is attached to the virConnect object. + * Returns pointer to hashmap, NULL if an error occcurred. */ virNWFilterHashTablePtr virNWFilterCreateVarHashmap(char *macaddr, @@ -274,7 +272,6 @@ virNWFilterPrintVars(virHashTablePtr var /** * virNWFilterRuleInstantiate: - * @conn: pointer to virConnect object * @techdriver: the driver to use for instantiation * @filter: The filter the rule is part of * @rule : The rule that is to be instantiated @@ -289,8 +286,7 @@ virNWFilterPrintVars(virHashTablePtr var * from the instantiation. Returns NULL on error with error reported. */ static virNWFilterRuleInstPtr -virNWFilterRuleInstantiate(virConnectPtr conn, - virNWFilterTechDriverPtr techdriver, +virNWFilterRuleInstantiate(virNWFilterTechDriverPtr techdriver, enum virDomainNetType nettype, virNWFilterDefPtr filter, virNWFilterRuleDefPtr rule, @@ -308,7 +304,7 @@ virNWFilterRuleInstantiate(virConnectPtr ret->techdriver = techdriver; - rc = techdriver->createRuleInstance(conn, nettype, filter, + rc = techdriver->createRuleInstance(nettype, filter, rule, ifname, vars, ret); if (rc) { @@ -359,7 +355,6 @@ err_exit: /** * _virNWFilterInstantiateRec: - * @conn: pointer to virConnect object * @techdriver: The driver to use for instantiation * @filter: The filter to instantiate * @ifname: The name of the interface to apply the rules to @@ -382,8 +377,7 @@ err_exit: * resolved -- among other reasons. */ static int -_virNWFilterInstantiateRec(virConnectPtr conn, - virNWFilterTechDriverPtr techdriver, +_virNWFilterInstantiateRec(virNWFilterTechDriverPtr techdriver, enum virDomainNetType nettype, virNWFilterDefPtr filter, const char *ifname, @@ -403,8 +397,7 @@ _virNWFilterInstantiateRec(virConnectPtr virNWFilterRuleDefPtr rule = filter->filterEntries[i]->rule; virNWFilterIncludeDefPtr inc = filter->filterEntries[i]->include; if (rule) { - inst = virNWFilterRuleInstantiate(conn, - techdriver, + inst = virNWFilterRuleInstantiate(techdriver, nettype, filter, rule, @@ -461,8 +454,7 @@ _virNWFilterInstantiateRec(virConnectPtr break; } - rc = _virNWFilterInstantiateRec(conn, - techdriver, + rc = _virNWFilterInstantiateRec(techdriver, nettype, next_filter, ifname, @@ -491,8 +483,7 @@ _virNWFilterInstantiateRec(virConnectPtr static int -virNWFilterDetermineMissingVarsRec(virConnectPtr conn, - virNWFilterDefPtr filter, +virNWFilterDetermineMissingVarsRec(virNWFilterDefPtr filter, virNWFilterHashTablePtr vars, virNWFilterHashTablePtr missing_vars, int useNewFilter, @@ -559,8 +550,7 @@ virNWFilterDetermineMissingVarsRec(virCo break; } - rc = virNWFilterDetermineMissingVarsRec(conn, - next_filter, + rc = virNWFilterDetermineMissingVarsRec(next_filter, tmpvars, missing_vars, useNewFilter, @@ -617,7 +607,6 @@ virNWFilterRuleInstancesToArray(int nEnt /** * virNWFilterInstantiate: - * @conn: pointer to virConnect object * @techdriver: The driver to use for instantiation * @filter: The filter to instantiate * @ifname: The name of the interface to apply the rules to @@ -636,8 +625,7 @@ virNWFilterRuleInstancesToArray(int nEnt * Call this function while holding the NWFilter filter update lock */ static int -virNWFilterInstantiate(virConnectPtr conn, - virNWFilterTechDriverPtr techdriver, +virNWFilterInstantiate(virNWFilterTechDriverPtr techdriver, enum virDomainNetType nettype, virNWFilterDefPtr filter, const char *ifname, @@ -665,8 +653,7 @@ virNWFilterInstantiate(virConnectPtr con goto err_exit; } - rc = virNWFilterDetermineMissingVarsRec(conn, - filter, + rc = virNWFilterDetermineMissingVarsRec(filter, vars, missing_vars, useNewFilter, @@ -697,8 +684,7 @@ virNWFilterInstantiate(virConnectPtr con goto err_exit; } - rc = _virNWFilterInstantiateRec(conn, - techdriver, + rc = _virNWFilterInstantiateRec(techdriver, nettype, filter, ifname, @@ -729,10 +715,10 @@ virNWFilterInstantiate(virConnectPtr con if (virNWFilterLockIface(ifname)) goto err_exit; - rc = techdriver->applyNewRules(conn, ifname, nptrs, ptrs); + rc = techdriver->applyNewRules(ifname, nptrs, ptrs); if (teardownOld && rc == 0) - techdriver->tearOldRules(conn, ifname); + techdriver->tearOldRules(ifname); if (rc == 0 && (virNetDevValidateConfig(ifname, NULL, ifindex) <= 0)) { virResetLastError(); @@ -775,8 +761,7 @@ err_unresolvable_vars: * Call this function while holding the NWFilter filter update lock */ static int -__virNWFilterInstantiateFilter(virConnectPtr conn, - bool teardownOld, +__virNWFilterInstantiateFilter(bool teardownOld, const char *ifname, int ifindex, const char *linkdev, @@ -868,8 +853,7 @@ __virNWFilterInstantiateFilter(virConnec break; } - rc = virNWFilterInstantiate(conn, - techdriver, + rc = virNWFilterInstantiate(techdriver, nettype, filter, ifname, @@ -924,8 +908,7 @@ _virNWFilterInstantiateFilter(virConnect goto cleanup; } - rc = __virNWFilterInstantiateFilter(conn, - teardownOld, + rc = __virNWFilterInstantiateFilter(teardownOld, net->ifname, ifindex, linkdev, @@ -946,8 +929,7 @@ cleanup: int -virNWFilterInstantiateFilterLate(virConnectPtr conn, - const char *ifname, +virNWFilterInstantiateFilterLate(const char *ifname, int ifindex, const char *linkdev, enum virDomainNetType nettype, @@ -961,8 +943,7 @@ virNWFilterInstantiateFilterLate(virConn virNWFilterLockFilterUpdates(); - rc = __virNWFilterInstantiateFilter(conn, - 1, + rc = __virNWFilterInstantiateFilter(true, ifname, ifindex, linkdev, @@ -1019,8 +1000,7 @@ virNWFilterUpdateInstantiateFilter(virCo return rc; } -int virNWFilterRollbackUpdateFilter(virConnectPtr conn, - const virDomainNetDefPtr net) +int virNWFilterRollbackUpdateFilter(const virDomainNetDefPtr net) { const char *drvname = EBIPTABLES_DRIVER_ID; int ifindex; @@ -1041,13 +1021,12 @@ int virNWFilterRollbackUpdateFilter(virC else if (virNWFilterLookupLearnReq(ifindex) != NULL) return 0; - return techdriver->tearNewRules(conn, net->ifname); + return techdriver->tearNewRules(net->ifname); } int -virNWFilterTearOldFilter(virConnectPtr conn, - virDomainNetDefPtr net) +virNWFilterTearOldFilter(virDomainNetDefPtr net) { const char *drvname = EBIPTABLES_DRIVER_ID; int ifindex; @@ -1068,7 +1047,7 @@ virNWFilterTearOldFilter(virConnectPtr c else if (virNWFilterLookupLearnReq(ifindex) != NULL) return 0; - return techdriver->tearOldRules(conn, net->ifname); + return techdriver->tearOldRules(net->ifname); } @@ -1141,14 +1120,13 @@ virNWFilterDomainFWUpdateCB(void *payloa case STEP_TEAR_NEW: if ( !virHashLookup(cb->skipInterfaces, net->ifname)) { - cb->err = virNWFilterRollbackUpdateFilter(cb->conn, - net); + cb->err = virNWFilterRollbackUpdateFilter(net); } break; case STEP_TEAR_OLD: if ( !virHashLookup(cb->skipInterfaces, net->ifname)) { - cb->err = virNWFilterTearOldFilter(cb->conn, net); + cb->err = virNWFilterTearOldFilter(net); } break; } 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 @@ -42,14 +42,11 @@ int virNWFilterInstantiateFilter(virConn int virNWFilterUpdateInstantiateFilter(virConnectPtr conn, const virDomainNetDefPtr net, bool *skipIface); -int virNWFilterRollbackUpdateFilter(virConnectPtr conn, - const virDomainNetDefPtr net); +int virNWFilterRollbackUpdateFilter(const virDomainNetDefPtr net); -int virNWFilterTearOldFilter(virConnectPtr conn, - const virDomainNetDefPtr net); +int virNWFilterTearOldFilter(const virDomainNetDefPtr net); -int virNWFilterInstantiateFilterLate(virConnectPtr conn, - const char *ifname, +int virNWFilterInstantiateFilterLate(const char *ifname, int ifindex, const char *linkdev, enum virDomainNetType nettype, Index: libvirt-acl/src/nwfilter/nwfilter_learnipaddr.c =================================================================== --- libvirt-acl.orig/src/nwfilter/nwfilter_learnipaddr.c +++ libvirt-acl/src/nwfilter/nwfilter_learnipaddr.c @@ -702,8 +702,7 @@ learnIPAddressThread(void *arg) "cache for interface %s"), inetaddr, req->ifname); } - ret = virNWFilterInstantiateFilterLate(NULL, - req->ifname, + ret = virNWFilterInstantiateFilterLate(req->ifname, req->ifindex, req->linkdev, req->nettype,

2 1

[libvirt] [PATCH] blockstats: support lookup by path in blockstats
by Eric Blake 23 Nov '11

23 Nov '11

Commit 89b6284f made it possible to pass either a source name or the target device to most API demanding a disk designation, but forgot to update the documentation. It also failed to update virDomainBlockStats to take both forms. This patch fixes both the documentation and the remaining function. Xen continues to use just device shorthand (that is, I did not implement path lookup there, since xen does not track a domain_conf to quickly tie a path back to the device shorthand). * src/libvirt.c (virDomainBlockStats, virDomainBlockStatsFlags) (virDomainGetBlockInfo, virDomainBlockPeek) (virDomainBlockJobAbort, virDomainGetBlockJobInfo) (virDomainBlockJobSetSpeed, virDomainBlockPull): Document acceptable disk naming conventions. * src/qemu/qemu_driver.c (qemuDomainBlockStats) (qemuDomainBlockStatsFlags): Allow lookup by source name. * src/test/test_driver.c (testDomainBlockStats): Likewise. --- I would like to apply this prior to patch 1/8 in Lei's series: https://www.redhat.com/archives/libvir-list/2011-November/msg01145.html since that patch should be using the same copy-and-paste documentation. src/libvirt.c | 82 ++++++++++++++++++++++++++++++++++++----------- src/qemu/qemu_driver.c | 20 ++--------- src/test/test_driver.c | 11 +----- 3 files changed, 69 insertions(+), 44 deletions(-) diff --git a/src/libvirt.c b/src/libvirt.c index 17e073e..811dde6 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -6659,16 +6659,19 @@ error: /** * virDomainBlockStats: * @dom: pointer to the domain object - * @path: path to the block device + * @path: path to the block device, or device shorthand * @stats: block device stats (returned) * @size: size of stats structure * * This function returns block device (disk) stats for block * devices attached to the domain. * - * The path 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"). + * The @path parameter is either the device target shorthand (the + * <target dev='...'/> sub-element, such as "xvda"), or (since 0.9.8) + * an unambiguous source name of the block device (the <source + * file='...'/> sub-element, such as "/path/to/image"). Valid names + * can be found by calling virDomainGetXMLDesc() and inspecting + * elements within //domain/devices/disk. * * Domains may have more than one block device. To get stats for * each you should make multiple calls to this function. @@ -6719,7 +6722,7 @@ error: /** * virDomainBlockStatsFlags: * @dom: pointer to domain object - * @path: path to the block device + * @path: path to the block device, or device shorthand * @params: pointer to block stats parameter object * (return value) * @nparams: pointer to number of block stats; input and output @@ -6728,9 +6731,12 @@ error: * This function is to get block stats parameters for block * devices attached to the domain. * - * The @path 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"). + * The @path parameter is either the device target shorthand (the + * <target dev='...'/> sub-element, such as "xvda"), or (since 0.9.8) + * an unambiguous source name of the block device (the <source + * file='...'/> sub-element, such as "/path/to/image"). Valid names + * can be found by calling virDomainGetXMLDesc() and inspecting + * elements within //domain/devices/disk. * * Domains may have more than one block device. To get stats for * each you should make multiple calls to this function. @@ -6927,7 +6933,7 @@ error: /** * virDomainBlockPeek: * @dom: pointer to the domain object - * @path: path to the block device + * @path: path to the block device, or device shorthand * @offset: offset within block device * @size: size to read * @buffer: return buffer (must be at least size bytes) @@ -6946,10 +6952,12 @@ error: * remote case, nor if you don't have sufficient permission. * Hence the need for this call). * - * 'path' must be a device or file corresponding to the domain. - * In other words it must be the precise string returned in - * a <disk><source dev='...'/></disk> from - * virDomainGetXMLDesc. + * The @path parameter is either an unambiguous source name of the + * block device (the <source file='...'/> sub-element, such as + * "/path/to/image"), or (since 0.9.5) the device target shorthand + * (the <target dev='...'/> sub-element, such as "xvda"). Valid names + * can be found by calling virDomainGetXMLDesc() and inspecting + * elements within //domain/devices/disk. * * 'offset' and 'size' represent an area which must lie entirely * within the device or file. 'size' may be 0 to test if the @@ -7133,16 +7141,24 @@ error: /** * virDomainGetBlockInfo: * @domain: a domain object - * @path: path to the block device or file + * @path: path to the block device, or device shorthand * @info: pointer to a virDomainBlockInfo structure allocated by the user * @flags: currently unused, pass zero * * Extract information about a domain's block device. * + * The @path parameter is either an unambiguous source name of the + * block device (the <source file='...'/> sub-element, such as + * "/path/to/image"), or (since 0.9.5) the device target shorthand + * (the <target dev='...'/> sub-element, such as "xvda"). Valid names + * can be found by calling virDomainGetXMLDesc() and inspecting + * elements within //domain/devices/disk. + * * Returns 0 in case of success and -1 in case of failure. */ int -virDomainGetBlockInfo(virDomainPtr domain, const char *path, virDomainBlockInfoPtr info, unsigned int flags) +virDomainGetBlockInfo(virDomainPtr domain, const char *path, + virDomainBlockInfoPtr info, unsigned int flags) { virConnectPtr conn; @@ -16837,11 +16853,18 @@ error: /** * virDomainBlockJobAbort: * @dom: pointer to domain object - * @path: fully-qualified filename of disk + * @path: path to the block device, or device shorthand * @flags: currently unused, for future extension * * Cancel the active block job on the given disk. * + * The @path parameter is either an unambiguous source name of the + * block device (the <source file='...'/> sub-element, such as + * "/path/to/image"), or (since 0.9.5) the device target shorthand + * (the <target dev='...'/> sub-element, such as "xvda"). Valid names + * can be found by calling virDomainGetXMLDesc() and inspecting + * elements within //domain/devices/disk. + * * Returns -1 in case of failure, 0 when successful. */ int virDomainBlockJobAbort(virDomainPtr dom, const char *path, @@ -16889,13 +16912,20 @@ error: /** * virDomainGetBlockJobInfo: * @dom: pointer to domain object - * @path: fully-qualified filename of disk + * @path: path to the block device, or device shorthand * @info: pointer to a virDomainBlockJobInfo structure * @flags: currently unused, for future extension * * Request block job information for the given disk. If an operation is active * @info will be updated with the current progress. * + * The @path parameter is either an unambiguous source name of the + * block device (the <source file='...'/> sub-element, such as + * "/path/to/image"), or (since 0.9.5) the device target shorthand + * (the <target dev='...'/> sub-element, such as "xvda"). Valid names + * can be found by calling virDomainGetXMLDesc() and inspecting + * elements within //domain/devices/disk. + * * Returns -1 in case of failure, 0 when nothing found, 1 when info was found. */ int virDomainGetBlockJobInfo(virDomainPtr dom, const char *path, @@ -16944,13 +16974,20 @@ error: /** * virDomainBlockJobSetSpeed: * @dom: pointer to domain object - * @path: fully-qualified filename of disk + * @path: path to the block device, or device shorthand * @bandwidth: specify bandwidth limit in Mbps * @flags: currently unused, for future extension * * Set the maximimum allowable bandwidth that a block job may consume. If * bandwidth is 0, the limit will revert to the hypervisor default. * + * The @path parameter is either an unambiguous source name of the + * block device (the <source file='...'/> sub-element, such as + * "/path/to/image"), or (since 0.9.5) the device target shorthand + * (the <target dev='...'/> sub-element, such as "xvda"). Valid names + * can be found by calling virDomainGetXMLDesc() and inspecting + * elements within //domain/devices/disk. + * * Returns -1 in case of failure, 0 when successful. */ int virDomainBlockJobSetSpeed(virDomainPtr dom, const char *path, @@ -16999,7 +17036,7 @@ error: /** * virDomainBlockPull: * @dom: pointer to domain object - * @path: Fully-qualified filename of disk + * @path: path to the block device, or device shorthand * @bandwidth: (optional) specify copy bandwidth limit in Mbps * @flags: currently unused, for future extension * @@ -17010,6 +17047,13 @@ error: * the operation can be aborted with virDomainBlockJobAbort(). When finished, * an asynchronous event is raised to indicate the final status. * + * The @path parameter is either an unambiguous source name of the + * block device (the <source file='...'/> sub-element, such as + * "/path/to/image"), or (since 0.9.5) the device target shorthand + * (the <target dev='...'/> sub-element, such as "xvda"). Valid names + * can be found by calling virDomainGetXMLDesc() and inspecting + * elements within //domain/devices/disk. + * * The maximum bandwidth (in Mbps) that will be used to do the copy can be * specified with the bandwidth parameter. If set to 0, libvirt will choose a * suitable default. Some hypervisors do not support this feature and will diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index fe2ab85..98ce695 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -7082,18 +7082,12 @@ qemuDomainBlockStats(virDomainPtr dom, goto cleanup; } - for (i = 0 ; i < vm->def->ndisks ; i++) { - if (STREQ(path, vm->def->disks[i]->dst)) { - disk = vm->def->disks[i]; - break; - } - } - - if (!disk) { + if ((i = virDomainDiskIndexByName(vm->def, path, false)) < 0) { qemuReportError(VIR_ERR_INVALID_ARG, _("invalid path: %s"), path); goto cleanup; } + disk = vm->def->disks[i]; if (!disk->info.alias) { qemuReportError(VIR_ERR_INTERNAL_ERROR, @@ -7174,18 +7168,12 @@ qemuDomainBlockStatsFlags(virDomainPtr dom, } if (*nparams != 0) { - for (i = 0 ; i < vm->def->ndisks ; i++) { - if (STREQ(path, vm->def->disks[i]->dst)) { - disk = vm->def->disks[i]; - break; - } - } - - if (!disk) { + if ((i = virDomainDiskIndexByName(vm->def, path, false)) < 0) { qemuReportError(VIR_ERR_INVALID_ARG, _("invalid path: %s"), path); goto cleanup; } + disk = vm->def->disks[i]; if (!disk->info.alias) { qemuReportError(VIR_ERR_INTERNAL_ERROR, diff --git a/src/test/test_driver.c b/src/test/test_driver.c index e6ff696..f365bf4 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -2803,7 +2803,7 @@ static int testDomainBlockStats(virDomainPtr domain, virDomainObjPtr privdom; struct timeval tv; unsigned long long statbase; - int i, found = 0, ret = -1; + int ret = -1; testDriverLock(privconn); privdom = virDomainFindByName(&privconn->domains, @@ -2815,14 +2815,7 @@ static int testDomainBlockStats(virDomainPtr domain, goto error; } - for (i = 0 ; i < privdom->def->ndisks ; i++) { - if (STREQ(path, privdom->def->disks[i]->dst)) { - found = 1; - break; - } - } - - if (!found) { + if (virDomainDiskIndexByName(privdom->def, path, false) < 0) { testError(VIR_ERR_INVALID_ARG, _("invalid path: %s"), path); goto error; -- 1.7.7.3

2 4

[libvirt] [PATCH] Fix uninitialized variable in NWfilter IP learning code
by Daniel P. Berrange 23 Nov '11

23 Nov '11

From: "Daniel P. Berrange" <berrange(a)redhat.com> Pushed as a build-breaker fix * src/nwfilter/nwfilter_learnipaddr.c: Initialize ret variable --- src/nwfilter/nwfilter_learnipaddr.c | 1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/src/nwfilter/nwfilter_learnipaddr.c b/src/nwfilter/nwfilter_learnipaddr.c index 675ef5b..d924c4d 100644 --- a/src/nwfilter/nwfilter_learnipaddr.c +++ b/src/nwfilter/nwfilter_learnipaddr.c @@ -340,6 +340,7 @@ virNWFilterAddIpAddrForIfname(const char *ifname, char *addr) } else { if (virNWFilterVarValueAddValue(val, addr) < 0) ret = -1; + ret = 0; } err_exit: -- 1.7.6.4

2 2

[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