- Devel - libvirt List Archives

[libvirt] [PATCH v2] nwfilter: Fix double free of pointer
by Stefan Berger 19 Mar '14

19 Mar '14

From: Stefan Berger <stefanb(a)linux.vnet.ibm.com> https://bugzilla.redhat.com/show_bug.cgi?id=1071181 Commit 49b59a15 fixed one problem but masks another one related to pointer freeing. Avoid putting of the virNWFilterSnoopReq once the thread has been started. It belongs to the thread and the thread will call virNWFilterSnoopReqPut() on it. Signed-off-by: Stefan Berger <stefanb(a)linux.vnet.ibm.com> --- src/nwfilter/nwfilter_dhcpsnoop.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/nwfilter/nwfilter_dhcpsnoop.c b/src/nwfilter/nwfilter_dhcpsnoop.c index d2a8062..3407604 100644 --- a/src/nwfilter/nwfilter_dhcpsnoop.c +++ b/src/nwfilter/nwfilter_dhcpsnoop.c @@ -1605,6 +1605,7 @@ virNWFilterDHCPSnoopReq(virNWFilterTechDriverPtr techdriver, int tmp; virThread thread; virNWFilterVarValuePtr dhcpsrvrs; + bool threadPuts = false; virNWFilterSnoopIFKeyFMT(ifkey, vmuuid, macaddr); @@ -1698,6 +1699,8 @@ virNWFilterDHCPSnoopReq(virNWFilterTechDriverPtr techdriver, goto exit_snoopreq_unlock; } + threadPuts = true; + virAtomicIntInc(&virNWFilterSnoopState.nThreads); req->threadkey = virNWFilterSnoopActivate(req); @@ -1737,7 +1740,8 @@ exit_rem_ifnametokey: exit_snoopunlock: virNWFilterSnoopUnlock(); exit_snoopreqput: - virNWFilterSnoopReqPut(req); + if (!threadPuts) + virNWFilterSnoopReqPut(req); return -1; } -- 1.8.1.4

2 2

[libvirt] [PATCH] network: fix problems with SRV
by Laine Stump 19 Mar '14

19 Mar '14

A patch submitted by Steven Malin last week pointed out a problem with libvirt's DNS SRV record configuration: https://www.redhat.com/archives/libvir-list/2014-March/msg00536.html When searching for that message later, I found another series that had been posted by Guannan Ren back in 2012 that somehow slipped between the cracks: https://www.redhat.com/archives/libvir-list/2012-July/msg00236.html That patch was very much out of date, but also pointed out some real problems. This patch fixes all the noted problems by refactoring virNetworkDNSSrvDefParseXML() and networkDnsmasqConfContents(), then verifies those fixes by added several new records to the test case. Problems fixed: * both service and protocol now have an underscore ("_") prepended on the commandline, as required by RFC2782. <srv service='sip' protocol='udp' domain='example.com' target='tests.example.com' port='5060' priority='10' weight='150'/> before: srv-host=sip.udp.example.com,tests.example.com,5060,10,150 after: srv-host=_sip._udp.example.com,tests.example.com,5060,10,150 * if "domain" wasn't specified in the <srv> element, the extra trailing "." will no longer be added to the dnsmasq commandline. <srv service='sip' protocol='udp' target='tests.example.com' port='5060' priority='10' weight='150'/> before: srv-host=sip.udp.,tests.example.com,5060,10,150 after: srv-host=_sip._udp,tests.example.com,5060,10,150 * when optional attributes aren't specified, the separating comma is also now not placed on the dnsmasq commandline. If optional attributes in the middle of the line are not specified, they are replaced with 0 in the commandline. <srv service='sip' protocol='udp' target='tests.example.com' port='5060'/> before: srv-host=sip.udp.,tests.example.com,5060,, after: srv-host=_sip._udp,tests.example.com,5060 (actually this would have generated an error, because "optional" attributes weren't really optional). * As a safeguard, the parser checks for a leading "_" on service and protocol, and fails if it is found. (Note that we can be guaranteed that no existing valid configuration will have this, since until now the parser had only allowed "tcp" or "udp" for protocol, and the bridge driver wasn't prepending "_", making it a 100% certainty that there was no example of working SRV record use in the wild). * the "domain" attribute is no longer required in order to recognize the port, priority, and weight attributes. Only "target" is required for this. * if "target" isn't specified, port, priority, and weight are not allowed (since they are meaningless - an empty target means "this service is *not available* for this domain"). * port, priority, and weight are now truly optional, as the comments originally suggested, but which was not actually true. --- src/conf/network_conf.c | 113 +++++++++++++-------- src/network/bridge_driver.c | 75 ++++++++------ .../nat-network-dns-srv-record-minimal.conf | 2 +- .../nat-network-dns-srv-record.conf | 8 +- .../nat-network-dns-srv-record.xml | 8 +- 5 files changed, 128 insertions(+), 78 deletions(-) diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c index 9be06d3..3a28ac7 100644 --- a/src/conf/network_conf.c +++ b/src/conf/network_conf.c @@ -931,80 +931,107 @@ virNetworkDNSSrvDefParseXML(const char *networkName, virNetworkDNSSrvDefPtr def, bool partialOkay) { + int ret; + xmlNodePtr save_ctxt = ctxt->node; + + ctxt->node = node; + if (!(def->service = virXMLPropString(node, "service")) && !partialOkay) { virReportError(VIR_ERR_XML_DETAIL, _("Missing required service attribute in DNS SRV record " "of network %s"), networkName); goto error; } - if (def->service && strlen(def->service) > DNS_RECORD_LENGTH_SRV) { - virReportError(VIR_ERR_XML_DETAIL, - _("Service name '%s' in network %s is too long, limit is %d bytes"), - def->service, networkName, DNS_RECORD_LENGTH_SRV); - goto error; + if (def->service) { + if (strlen(def->service) > DNS_RECORD_LENGTH_SRV) { + virReportError(VIR_ERR_XML_DETAIL, + _("service attribute '%s' in network %s is too long, " + "limit is %d bytes"), + def->service, networkName, DNS_RECORD_LENGTH_SRV); + goto error; + } + if (def->service[0] == '_') { + virReportError(VIR_ERR_XML_DETAIL, + _("service attribute '%s' in network %s DNS SRV " + "record cannot start with '_'"), + def->service, networkName); + goto error; + } } if (!(def->protocol = virXMLPropString(node, "protocol")) && !partialOkay) { virReportError(VIR_ERR_XML_DETAIL, _("Missing required protocol attribute " - "in dns srv record '%s' of network %s"), + "in DNS SRV record '%s' of network %s"), def->service, networkName); goto error; } - - /* Check whether protocol value is the supported one */ - if (def->protocol && STRNEQ(def->protocol, "tcp") && - (STRNEQ(def->protocol, "udp"))) { + if (def->protocol && def->protocol[0] == '_') { virReportError(VIR_ERR_XML_DETAIL, - _("Invalid protocol attribute value '%s' " - "in DNS SRV record of network %s"), + _("protocol attribute '%s' in network %s DNS SRV record " + "cannot start with '_'"), def->protocol, networkName); goto error; } /* Following attributes are optional */ - if ((def->target = virXMLPropString(node, "target")) && - (def->domain = virXMLPropString(node, "domain"))) { - xmlNodePtr save_ctxt = ctxt->node; + def->domain = virXMLPropString(node, "domain"); + def->target = virXMLPropString(node, "target"); - ctxt->node = node; - if (virXPathUInt("string(./@port)", ctxt, &def->port) < 0 || - def->port > 65535) { - virReportError(VIR_ERR_XML_DETAIL, - _("Missing or invalid port attribute " - "in network %s"), networkName); - goto error; - } - - if (virXPathUInt("string(./@priority)", ctxt, &def->priority) < 0 || - def->priority > 65535) { - virReportError(VIR_ERR_XML_DETAIL, - _("Missing or invalid priority attribute " - "in network %s"), networkName); - goto error; - } - - if (virXPathUInt("string(./@weight)", ctxt, &def->weight) < 0 || - def->weight > 65535) { - virReportError(VIR_ERR_XML_DETAIL, - _("Missing or invalid weight attribute " - "in network %s"), networkName); - goto error; - } + ret = virXPathUInt("string(./@port)", ctxt, &def->port); + if (ret >= 0 && !def->target) { + virReportError(VIR_ERR_XML_DETAIL, + _("DNS SRV port attribute not permitted without " + "target for service %s in network %s"), + def->service, networkName); + goto error; + } + if (ret == -2 || (ret >= 0 && def->port > 65535)) { + virReportError(VIR_ERR_XML_DETAIL, + _("Invalid DNS SRV port attribute " + "for service %s in network %s"), + def->service, networkName); + goto error; + } - ctxt->node = save_ctxt; + ret = virXPathUInt("string(./@priority)", ctxt, &def->priority); + if (ret >= 0 && !def->target) { + virReportError(VIR_ERR_XML_DETAIL, + _("DNS SRV priority attribute not permitted without " + "target for service %s in network %s"), + def->service, networkName); + goto error; + } + if (ret == -2 || (ret >= 0 && def->priority > 65535)) { + virReportError(VIR_ERR_XML_DETAIL, + _("Invalid DNS SRV priority attribute " + "for service %s in network %s"), + def->service, networkName); + goto error; } - if (!(def->service || def->protocol)) { + ret = virXPathUInt("string(./@weight)", ctxt, &def->weight); + if (ret >= 0 && !def->target) { virReportError(VIR_ERR_XML_DETAIL, - _("Missing required service attribute or protocol " - "in DNS SRV record of network %s"), networkName); + _("DNS SRV weight attribute not permitted without " + "target for service %s in network %s"), + def->service, networkName); goto error; } + if (ret == -2 || (ret >= 0 && def->weight > 65535)) { + virReportError(VIR_ERR_XML_DETAIL, + _("Invalid DNS SRV weight attribute " + "for service %s in network %s"), + def->service, networkName); + goto error; + } + + ctxt->node = save_ctxt; return 0; error: virNetworkDNSSrvDefClear(def); + ctxt->node = save_ctxt; return -1; } diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index 181541e..94400cf 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -734,10 +734,6 @@ networkDnsmasqConfContents(virNetworkObjPtr network, int r, ret = -1; int nbleases = 0; size_t i; - char *record = NULL; - char *recordPort = NULL; - char *recordWeight = NULL; - char *recordPriority = NULL; virNetworkDNSDefPtr dns = &network->def->dns; virNetworkIpDefPtr tmpipdef, ipdef, ipv4def, ipv6def; bool ipv6SLAAC; @@ -878,33 +874,52 @@ networkDnsmasqConfContents(virNetworkObjPtr network, } for (i = 0; i < dns->nsrvs; i++) { - if (dns->srvs[i].service && dns->srvs[i].protocol) { - if (dns->srvs[i].port && - virAsprintf(&recordPort, "%d", dns->srvs[i].port) < 0) - goto cleanup; - if (dns->srvs[i].priority && - virAsprintf(&recordPriority, "%d", dns->srvs[i].priority) < 0) - goto cleanup; - if (dns->srvs[i].weight && - virAsprintf(&recordWeight, "%d", dns->srvs[i].weight) < 0) - goto cleanup; + /* service/protocol are required, and should have been validated + * by the parser. + */ + if (!dns->srvs[i].service) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Missing required 'service' " + "attribute in SRV record of network '%s'"), + network->def->name); + goto cleanup; + } + if (!dns->srvs[i].protocol) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Missing required 'service' " + "attribute in SRV record of network '%s'"), + network->def->name); + goto cleanup; + } + /* RFC2782 requires that service and protocol be preceded by + * an underscore. + */ + virBufferAsprintf(&configbuf, "srv-host=_%s._%s", + dns->srvs[i].service, dns->srvs[i].protocol); - if (virAsprintf(&record, "%s.%s.%s,%s,%s,%s,%s", - dns->srvs[i].service, - dns->srvs[i].protocol, - dns->srvs[i].domain ? dns->srvs[i].domain : "", - dns->srvs[i].target ? dns->srvs[i].target : "", - recordPort ? recordPort : "", - recordPriority ? recordPriority : "", - recordWeight ? recordWeight : "") < 0) - goto cleanup; + /* domain is optional - it defaults to the domain of this network */ + if (dns->srvs[i].domain) + virBufferAsprintf(&configbuf, ".%s", dns->srvs[i].domain); - virBufferAsprintf(&configbuf, "srv-host=%s\n", record); - VIR_FREE(record); - VIR_FREE(recordPort); - VIR_FREE(recordWeight); - VIR_FREE(recordPriority); + /* If target is empty or ".", that means "the service is + * decidedly not available at this domain" (RFC2782). In that + * case, any port, priority, or weight is irrelevant. + */ + if (dns->srvs[i].target && STRNEQ(dns->srvs[i].target, ".")) { + + virBufferAsprintf(&configbuf, ",%s", dns->srvs[i].target); + /* port, priority, and weight are optional, but are + * identified by their position in the line + */ + if (dns->srvs[i].port || + dns->srvs[i].priority || dns->srvs[i].weight) + virBufferAsprintf(&configbuf, ",%d", dns->srvs[i].port); + if (dns->srvs[i].priority || dns->srvs[i].weight) + virBufferAsprintf(&configbuf, ",%d", dns->srvs[i].priority); + if (dns->srvs[i].weight) + virBufferAsprintf(&configbuf, ",%d", dns->srvs[i].weight); } + virBufferAddLit(&configbuf, "\n"); } /* Find the first dhcp for both IPv4 and IPv6 */ @@ -1080,10 +1095,6 @@ networkDnsmasqConfContents(virNetworkObjPtr network, cleanup: virBufferFreeAndReset(&configbuf); - VIR_FREE(record); - VIR_FREE(recordPort); - VIR_FREE(recordWeight); - VIR_FREE(recordPriority); return ret; } diff --git a/tests/networkxml2confdata/nat-network-dns-srv-record-minimal.conf b/tests/networkxml2confdata/nat-network-dns-srv-record-minimal.conf index ce4dd6f..e60411b 100644 --- a/tests/networkxml2confdata/nat-network-dns-srv-record-minimal.conf +++ b/tests/networkxml2confdata/nat-network-dns-srv-record-minimal.conf @@ -12,7 +12,7 @@ listen-address=192.168.123.1 listen-address=fc00:db8:ac10:fe01::1 listen-address=fc00:db8:ac10:fd01::1 listen-address=10.24.10.1 -srv-host=name.tcp.,,,, +srv-host=_name._tcp dhcp-range=192.168.122.2,192.168.122.254 dhcp-no-override dhcp-leasefile=/var/lib/libvirt/dnsmasq/default.leases diff --git a/tests/networkxml2confdata/nat-network-dns-srv-record.conf b/tests/networkxml2confdata/nat-network-dns-srv-record.conf index b47cbe7..b6fe6f9 100644 --- a/tests/networkxml2confdata/nat-network-dns-srv-record.conf +++ b/tests/networkxml2confdata/nat-network-dns-srv-record.conf @@ -8,7 +8,13 @@ strict-order except-interface=lo bind-dynamic interface=virbr0 -srv-host=name.tcp.test-domain-name,.,1024,10,10 +srv-host=_name._tcp.test-domain-name.com,test.example.com,1111,11,111 +srv-host=_name2._udp,test2.example.com,2222,22,222 +srv-host=_name3._tcp.test3.com,test3.example.com,3333,33 +srv-host=_name4._tcp.test4.com,test4.example.com,4444 +srv-host=_name5._udp,test5.example.com,0,55,555 +srv-host=_name6._tcp.test6.com,test6.example.com,6666,0,666 +srv-host=_name7._tcp.test7.com,test7.example.com,0,0,777 dhcp-range=192.168.122.2,192.168.122.254 dhcp-no-override dhcp-leasefile=/var/lib/libvirt/dnsmasq/default.leases diff --git a/tests/networkxml2confdata/nat-network-dns-srv-record.xml b/tests/networkxml2confdata/nat-network-dns-srv-record.xml index 3dd19e6..d01b331 100644 --- a/tests/networkxml2confdata/nat-network-dns-srv-record.xml +++ b/tests/networkxml2confdata/nat-network-dns-srv-record.xml @@ -6,7 +6,13 @@ </forward> <bridge name='virbr0' stp='on' delay='0'/> <dns> - <srv service='name' protocol='tcp' domain='test-domain-name' target='.' port='1024' priority='10' weight='10'/> + <srv service='name' protocol='tcp' domain='test-domain-name.com' target='test.example.com' port='1111' priority='11' weight='111'/> + <srv service='name2' protocol='udp' target='test2.example.com' port='2222' priority='22' weight='222'/> + <srv service='name3' protocol='tcp' domain='test3.com' target='test3.example.com' port='3333' priority='33'/> + <srv service='name4' protocol='tcp' domain='test4.com' target='test4.example.com' port='4444'/> + <srv service='name5' protocol='udp' target='test5.example.com' priority='55' weight='555'/> + <srv service='name6' protocol='tcp' domain='test6.com' target='test6.example.com' port='6666' weight='666'/> + <srv service='name7' protocol='tcp' domain='test7.com' target='test7.example.com' weight='777'/> </dns> <ip address='192.168.122.1' netmask='255.255.255.0'> <dhcp> -- 1.8.5.3

2 3

[libvirt] [PATCH] Give virNWFilterVarCombIterNext saner semantics
by Daniel P. Berrange 19 Mar '14

19 Mar '14

The virNWFilterVarCombIterNext method will free its parameter when it gets to the end of the iterator. This is somewhat misleading design, making it appear as if the caller has a memory leak. Remove the free'ing of the parameter and ensure that the calling method ebiptablesCreateRuleInstanceIterate free's it instead. Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com> --- src/conf/nwfilter_params.c | 4 +--- src/nwfilter/nwfilter_ebiptables_driver.c | 12 ++++++------ 2 files changed, 7 insertions(+), 9 deletions(-) diff --git a/src/conf/nwfilter_params.c b/src/conf/nwfilter_params.c index a78c407..5393134 100644 --- a/src/conf/nwfilter_params.c +++ b/src/conf/nwfilter_params.c @@ -526,10 +526,8 @@ next: } } - if (ci->nIter == i) { - virNWFilterVarCombIterFree(ci); + if (ci->nIter == i) return NULL; - } return ci; } diff --git a/src/nwfilter/nwfilter_ebiptables_driver.c b/src/nwfilter/nwfilter_ebiptables_driver.c index 0505045..e1e0af8 100644 --- a/src/nwfilter/nwfilter_ebiptables_driver.c +++ b/src/nwfilter/nwfilter_ebiptables_driver.c @@ -2869,14 +2869,14 @@ ebiptablesCreateRuleInstanceIterate( virNWFilterRuleInstPtr res) { int rc = 0; - virNWFilterVarCombIterPtr vciter; + virNWFilterVarCombIterPtr vciter, tmp; /* rule->vars holds all the variables names that this rule will access. * iterate over all combinations of the variables' values and instantiate * the filtering rule with each combination. */ - vciter = virNWFilterVarCombIterCreate(vars, - rule->varAccess, rule->nVarAccess); + tmp = vciter = virNWFilterVarCombIterCreate(vars, + rule->varAccess, rule->nVarAccess); if (!vciter) return -1; @@ -2885,12 +2885,12 @@ ebiptablesCreateRuleInstanceIterate( nwfilter, rule, ifname, - vciter, + tmp, res); if (rc < 0) break; - vciter = virNWFilterVarCombIterNext(vciter); - } while (vciter != NULL); + tmp = virNWFilterVarCombIterNext(tmp); + } while (tmp != NULL); virNWFilterVarCombIterFree(vciter); -- 1.8.5.3

4 3

[libvirt] [PATCH] Fix leak on OOM when creating nwfilter rule instances
by Daniel P. Berrange 19 Mar '14

19 Mar '14

The ebiptablesAddRuleInst method would leak an instance of ebiptablesRuleInstPtr if it hit OOM when adding it to the list of instances. Remove the pointless helper method virNWFilterRuleInstAddData and just inline the call to VIR_APPEND_ELEMENT and free the instance on failure. Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com> --- src/nwfilter/nwfilter_ebiptables_driver.c | 6 +++++- src/nwfilter/nwfilter_gentech_driver.c | 22 ---------------------- src/nwfilter/nwfilter_gentech_driver.h | 3 --- 3 files changed, 5 insertions(+), 26 deletions(-) diff --git a/src/nwfilter/nwfilter_ebiptables_driver.c b/src/nwfilter/nwfilter_ebiptables_driver.c index 9dbd3ff..e1e0af8 100644 --- a/src/nwfilter/nwfilter_ebiptables_driver.c +++ b/src/nwfilter/nwfilter_ebiptables_driver.c @@ -505,7 +505,11 @@ ebiptablesAddRuleInst(virNWFilterRuleInstPtr res, inst->priority = priority; inst->ruleType = ruleType; - return virNWFilterRuleInstAddData(res, inst); + if (VIR_APPEND_ELEMENT(res->data, res->ndata, inst) < 0) { + VIR_FREE(inst); + return -1; + } + return 0; } diff --git a/src/nwfilter/nwfilter_gentech_driver.c b/src/nwfilter/nwfilter_gentech_driver.c index 5c3b25e..1e9b3d2 100644 --- a/src/nwfilter/nwfilter_gentech_driver.c +++ b/src/nwfilter/nwfilter_gentech_driver.c @@ -115,28 +115,6 @@ virNWFilterTechDriverForName(const char *name) } -/** - * virNWFilterRuleInstAddData: - * @res : pointer to virNWFilterRuleInst object collecting the instantiation - * data of a single firewall rule. - * @data : the opaque data that the driver wants to add - * - * Add instantiation data to a firewall rule. An instantiated firewall - * rule may hold multiple data structure representing its instantiation - * data. This may for example be the case if a rule has been defined - * 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. - */ -int -virNWFilterRuleInstAddData(virNWFilterRuleInstPtr res, - void *data) -{ - return VIR_APPEND_ELEMENT(res->data, res->ndata, data); -} - - static void virNWFilterRuleInstFree(virNWFilterRuleInstPtr inst) { diff --git a/src/nwfilter/nwfilter_gentech_driver.h b/src/nwfilter/nwfilter_gentech_driver.h index d72e040..52bd1f6 100644 --- a/src/nwfilter/nwfilter_gentech_driver.h +++ b/src/nwfilter/nwfilter_gentech_driver.h @@ -28,9 +28,6 @@ virNWFilterTechDriverPtr virNWFilterTechDriverForName(const char *name); -int virNWFilterRuleInstAddData(virNWFilterRuleInstPtr res, - void *data); - int virNWFilterTechDriversInit(bool privileged); void virNWFilterTechDriversShutdown(void); -- 1.8.5.3

3 2

[libvirt] [PATCH] nwfilter: Fix double free of pointer
by Stefan Berger 19 Mar '14

19 Mar '14

From: Stefan Berger <stefanb(a)linux.vnet.ibm.com> https://bugzilla.redhat.com/show_bug.cgi?id=1071181 Commit 49b59a15 fixed one problem but masks another one related to pointer freeing. Avoid putting of the virNWFilterSnoopReq once the thread has been started. It belongs to the thread and the thread will call virNWFilterSnoopReqPut() on it. Signed-off-by: Stefan Berger <stefanb(a)linux.vnet.ibm.com> --- src/nwfilter/nwfilter_dhcpsnoop.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/nwfilter/nwfilter_dhcpsnoop.c b/src/nwfilter/nwfilter_dhcpsnoop.c index d2a8062..6a2de98 100644 --- a/src/nwfilter/nwfilter_dhcpsnoop.c +++ b/src/nwfilter/nwfilter_dhcpsnoop.c @@ -1605,6 +1605,7 @@ virNWFilterDHCPSnoopReq(virNWFilterTechDriverPtr techdriver, int tmp; virThread thread; virNWFilterVarValuePtr dhcpsrvrs; + bool threadPuts = false; virNWFilterSnoopIFKeyFMT(ifkey, vmuuid, macaddr); @@ -1690,6 +1691,8 @@ virNWFilterDHCPSnoopReq(virNWFilterTechDriverPtr techdriver, /* prevent thread from holding req */ virNWFilterSnoopReqLock(req); + threadPuts = true; + if (virThreadCreate(&thread, false, virNWFilterDHCPSnoopThread, req) != 0) { virReportError(VIR_ERR_INTERNAL_ERROR, @@ -1737,7 +1740,8 @@ exit_rem_ifnametokey: exit_snoopunlock: virNWFilterSnoopUnlock(); exit_snoopreqput: - virNWFilterSnoopReqPut(req); + if (!threadPuts) + virNWFilterSnoopReqPut(req); return -1; } -- 1.8.1.4

2 1

[libvirt] [PATCH] conf: consistent comments about disk enum usage
by Eric Blake 19 Mar '14

19 Mar '14

Before refactoring this struct, I found it helpful to track which 'int' fields really contain an enum value. * src/conf/domain_conf.h (_virDomainDiskDef): Add comments. Signed-off-by: Eric Blake <eblake(a)redhat.com> --- Pushing under the trivial rule. src/conf/domain_conf.h | 36 ++++++++++++++++++------------------ 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 37191a8..27f07e6 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -494,7 +494,7 @@ struct _virDomainHostdevDef { virDomainDeviceInfoPtr info; /* Guest address */ }; -/* Two types of disk backends */ +/* Types of disk backends (host resource) */ enum virDomainDiskType { VIR_DOMAIN_DISK_TYPE_BLOCK, VIR_DOMAIN_DISK_TYPE_FILE, @@ -505,7 +505,7 @@ enum virDomainDiskType { VIR_DOMAIN_DISK_TYPE_LAST }; -/* Three types of disk frontend */ +/* Types of disk frontend (guest view) */ enum virDomainDiskDevice { VIR_DOMAIN_DISK_DEVICE_DISK, VIR_DOMAIN_DISK_DEVICE_CDROM, @@ -529,7 +529,7 @@ enum virDomainDiskBus { VIR_DOMAIN_DISK_BUS_LAST }; -enum virDomainDiskCache { +enum virDomainDiskCache { VIR_DOMAIN_DISK_CACHE_DEFAULT, VIR_DOMAIN_DISK_CACHE_DISABLE, VIR_DOMAIN_DISK_CACHE_WRITETHRU, @@ -540,7 +540,7 @@ enum virDomainDiskCache { VIR_DOMAIN_DISK_CACHE_LAST }; -enum virDomainDiskErrorPolicy { +enum virDomainDiskErrorPolicy { VIR_DOMAIN_DISK_ERROR_POLICY_DEFAULT, VIR_DOMAIN_DISK_ERROR_POLICY_STOP, VIR_DOMAIN_DISK_ERROR_POLICY_REPORT, @@ -580,7 +580,7 @@ enum virDomainDiskTray { VIR_DOMAIN_DISK_TRAY_LAST }; -enum virDomainDiskGeometryTrans { +enum virDomainDiskGeometryTrans { VIR_DOMAIN_DISK_TRANS_DEFAULT = 0, VIR_DOMAIN_DISK_TRANS_NONE, VIR_DOMAIN_DISK_TRANS_AUTO, @@ -598,7 +598,7 @@ struct _virDomainDiskHostDef { char *socket; /* path to unix socket */ }; -enum virDomainDiskIo { +enum virDomainDiskIo { VIR_DOMAIN_DISK_IO_DEFAULT, VIR_DOMAIN_DISK_IO_NATIVE, VIR_DOMAIN_DISK_IO_THREADS, @@ -707,14 +707,14 @@ typedef virDomainDiskSourcePoolDef *virDomainDiskSourcePoolDefPtr; /* Stores the virtual disk configuration */ struct _virDomainDiskDef { - int type; - int device; - int bus; + int type; /* enum virDomainDiskType */ + int device; /* enum virDomainDiskDevice */ + int bus; /* enum virDomainDiskBus */ char *src; char *dst; - int tray_status; - int removable; - int protocol; + int tray_status; /* enum virDomainDiskTray */ + int removable; /* enum virDomainFeatureState */ + int protocol; /* enum virDomainDiskProtocol */ size_t nhosts; virDomainDiskHostDefPtr hosts; virDomainDiskSourcePoolDefPtr srcpool; @@ -738,7 +738,7 @@ struct _virDomainDiskDef { unsigned int cylinders; unsigned int heads; unsigned int sectors; - int trans; + int trans; /* enum virDomainDiskGeometryTrans */ } geometry; struct { @@ -752,13 +752,13 @@ struct _virDomainDiskDef { char *wwn; char *vendor; char *product; - int cachemode; + int cachemode; /* enum virDomainDiskCache */ int error_policy; /* enum virDomainDiskErrorPolicy */ int rerror_policy; /* enum virDomainDiskErrorPolicy */ - int iomode; - int ioeventfd; - int event_idx; - int copy_on_read; + int iomode; /* enum virDomainDiskIo */ + int ioeventfd; /* enum virDomainIoEventFd */ + int event_idx; /* enum virDomainVirtioEventIdx */ + int copy_on_read; /* enum virDomainDiskCopyOnRead */ int snapshot; /* enum virDomainSnapshotLocation, snapshot_conf.h */ int startupPolicy; /* enum virDomainStartupPolicy */ bool readonly; -- 1.8.5.3

1 0

[libvirt] [PATCH] build: Make sure src/util/virprobe.h is distributed
by Jiri Denemark 19 Mar '14

19 Mar '14

Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com> --- src/Makefile.am | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Makefile.am b/src/Makefile.am index 5ff178b..4fdd871 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -137,6 +137,7 @@ UTIL_SOURCES = \ util/virpci.c util/virpci.h \ util/virpidfile.c util/virpidfile.h \ util/virportallocator.c util/virportallocator.h \ + util/virprobe.h \ util/virprocess.c util/virprocess.h \ util/virrandom.h util/virrandom.c \ util/virscsi.c util/virscsi.h \ -- 1.9.0

2 1

[libvirt] [RFC] [PATCH] Fix flawed testcases in qemuhotplugtest.c
by Nehal J Wani 19 Mar '14

19 Mar '14

While running qemuhotplugtest, it was found that valgrind pointed out the following memory leak: ==7906== 5 bytes in 1 blocks are definitely lost in loss record 7 of 121 ==7906== at 0x4A069EE: malloc (vg_replace_malloc.c:270) ==7906== by 0x3E782A754D: xmlStrndup (in /usr/lib64/libxml2.so.2.7.6) ==7906== by 0x4CDAE03: virDomainDeviceInfoParseXML.isra.32 (domain_conf.c:3685) ==7906== by 0x4CE3BB9: virDomainNetDefParseXML (domain_conf.c:6707) ==7906== by 0x4CFBA08: virDomainDefParseXML (domain_conf.c:12235) ==7906== by 0x4CFBC1E: virDomainDefParseNode (domain_conf.c:13039) ==7906== by 0x4CFBD95: virDomainDefParse (domain_conf.c:12981) ==7906== by 0x41FEB4: testQemuHotplug (qemuhotplugtest.c:66) ==7906== by 0x420F41: virtTestRun (testutils.c:201) ==7906== by 0x41F287: mymain (qemuhotplugtest.c:422) ==7906== by 0x4216BD: virtTestMain (testutils.c:784) ==7906== by 0x3E6CE1ED1C: (below main) (libc-start.c:226) ...and 10 more. The 'alias' attribute should *not* be parsed from the XML provided by the user. It should only be parsed in the live state XML. In the latter case no codepath should take us to qemuAssignDeviceAliases (which, in this case, is causing the above leaks). --- Caused by the test cases: DO_TEST_ATTACH("console-compat-2", "console-virtio", false, true, "chardev-add", "{\"return\": {\"pty\": \"/dev/pts/26\"}}", "device_add", QMP_OK); DO_TEST_DETACH("console-compat-2", "console-virtio", false, false, "device_del", QMP_OK, "chardev-remove", QMP_OK); Refer: http://www.redhat.com/archives/libvir-list/2013-November/msg01187.html ...qemuhotplug-console-compat-2+console-virtio.xml | 34 +++----------------- .../qemuhotplug-console-virtio.xml | 1 - .../qemuxml2argv-console-compat-2.xml | 32 +++--------------- 3 files changed, 10 insertions(+), 57 deletions(-) diff --git a/tests/qemuhotplugtestdata/qemuhotplug-console-compat-2+console-virtio.xml b/tests/qemuhotplugtestdata/qemuhotplug-console-compat-2+console-virtio.xml index 25fc120..ec1c6e8 100644 --- a/tests/qemuhotplugtestdata/qemuhotplug-console-compat-2+console-virtio.xml +++ b/tests/qemuhotplugtestdata/qemuhotplug-console-compat-2+console-virtio.xml @@ -1,4 +1,4 @@ -<domain type='kvm' id='2'> +<domain type='kvm'> <name>f17</name> <uuid>a1cd52eb-d37f-4717-fc6e-972f0774f4c9</uuid> <memory unit='KiB'>1048576</memory> @@ -30,7 +30,6 @@ <driver name='qemu' type='qcow2' cache='none'/> <source file='/var/lib/libvirt/images/f17.qcow2'/> <target dev='vda' bus='virtio'/> - <alias name='virtio-disk0'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x05' function='0x0'/> </disk> <disk type='file' device='cdrom'> @@ -38,22 +37,16 @@ <source file='/home/user/tmp/Fedora-17-x86_64-Live-KDE.iso'/> <target dev='hdc' bus='ide'/> <readonly/> - <alias name='ide0-1-0'/> <address type='drive' controller='0' bus='1' target='0' unit='0'/> </disk> <controller type='ide' index='0'> - <alias name='ide0'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x1'/> </controller> <controller type='usb' index='0'> - <alias name='usb0'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/> </controller> - <controller type='pci' index='0' model='pci-root'> - <alias name='pci.0'/> - </controller> + <controller type='pci' index='0' model='pci-root'/> <controller type='virtio-serial' index='0'> - <alias name='virtio-serial0'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x07' function='0x0'/> </controller> <interface type='network'> @@ -63,62 +56,45 @@ <inbound average='4000' peak='8000' floor='200' burst='1024'/> <outbound average='4000' peak='8000' burst='1024'/> </bandwidth> - <target dev='vnet0'/> <model type='virtio'/> - <alias name='net0'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/> </interface> <serial type='pty'> - <source path='/dev/pts/22'/> <target type='isa-serial' port='0'/> - <alias name='serial0'/> </serial> <serial type='pty'> - <source path='/dev/pts/25'/> <target port='0'/> - <alias name='serial1'/> </serial> <serial type='tcp'> <source mode='bind' host='0.0.0.0' service='2445'/> <protocol type='raw'/> <target port='1'/> - <alias name='serial2'/> </serial> - <console type='pty' tty='/dev/pts/22'> - <source path='/dev/pts/22'/> + <console type='pty'> <target type='serial' port='0'/> - <alias name='serial0'/> </console> <console type='pty'> - <source path='/dev/pts/26'/> <target type='virtio' port='1'/> - <alias name='console1'/> </console> <channel type='unix'> <source mode='bind' path='/var/lib/libvirt/qemu/f17x86_64.agent'/> <target type='virtio' name='org.qemu.guest_agent.0'/> - <alias name='channel0'/> <address type='virtio-serial' controller='0' bus='0' port='1'/> </channel> - <input type='tablet' bus='usb'> - <alias name='input0'/> - </input> + <input type='tablet' bus='usb'/> <input type='mouse' bus='ps2'/> <input type='keyboard' bus='ps2'/> - <graphics type='vnc' port='5900' autoport='yes' listen='0.0.0.0'> + <graphics type='vnc' port='-1' autoport='yes' listen='0.0.0.0'> <listen type='address' address='0.0.0.0'/> </graphics> <sound model='ich6'> - <alias name='sound0'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x0'/> </sound> <video> <model type='cirrus' vram='9216' heads='1'/> - <alias name='video0'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/> </video> <memballoon model='virtio'> - <alias name='balloon0'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x06' function='0x0'/> </memballoon> </devices> diff --git a/tests/qemuhotplugtestdata/qemuhotplug-console-virtio.xml b/tests/qemuhotplugtestdata/qemuhotplug-console-virtio.xml index bfab6ff..3eb2be9 100644 --- a/tests/qemuhotplugtestdata/qemuhotplug-console-virtio.xml +++ b/tests/qemuhotplugtestdata/qemuhotplug-console-virtio.xml @@ -1,5 +1,4 @@ <console type='pty'> <source path='/dev/pts/26'/> <target type='virtio' port='1'/> - <alias name='console1'/> </console> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-console-compat-2.xml b/tests/qemuxml2argvdata/qemuxml2argv-console-compat-2.xml index e5c45a3..4d4ac47 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-console-compat-2.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-console-compat-2.xml @@ -1,4 +1,4 @@ -<domain type='kvm' id='2'> +<domain type='kvm'> <name>f17</name> <uuid>a1cd52eb-d37f-4717-fc6e-972f0774f4c9</uuid> <memory unit='KiB'>1048576</memory> @@ -30,7 +30,6 @@ <driver name='qemu' type='qcow2' cache='none'/> <source file='/var/lib/libvirt/images/f17.qcow2'/> <target dev='vda' bus='virtio'/> - <alias name='virtio-disk0'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x05' function='0x0'/> </disk> <disk type='file' device='cdrom'> @@ -38,22 +37,16 @@ <source file='/home/user/tmp/Fedora-17-x86_64-Live-KDE.iso'/> <target dev='hdc' bus='ide'/> <readonly/> - <alias name='ide0-1-0'/> <address type='drive' controller='0' bus='1' target='0' unit='0'/> </disk> <controller type='ide' index='0'> - <alias name='ide0'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x1'/> </controller> <controller type='usb' index='0'> - <alias name='usb0'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/> </controller> - <controller type='pci' index='0' model='pci-root'> - <alias name='pci.0'/> - </controller> + <controller type='pci' index='0' model='pci-root'/> <controller type='virtio-serial' index='0'> - <alias name='virtio-serial0'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x07' function='0x0'/> </controller> <interface type='network'> @@ -63,57 +56,42 @@ <inbound average='4000' peak='8000' floor='200' burst='1024'/> <outbound average='4000' peak='8000' burst='1024'/> </bandwidth> - <target dev='vnet0'/> <model type='virtio'/> - <alias name='net0'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/> </interface> <serial type='pty'> - <source path='/dev/pts/22'/> <target type='isa-serial' port='0'/> - <alias name='serial0'/> </serial> <serial type='pty'> - <source path='/dev/pts/25'/> <target port='0'/> - <alias name='serial1'/> </serial> <serial type='tcp'> <source mode='bind' host='0.0.0.0' service='2445'/> <protocol type='raw'/> <target port='1'/> - <alias name='serial2'/> </serial> - <console type='pty' tty='/dev/pts/22'> - <source path='/dev/pts/22'/> + <console type='pty'> <target type='serial' port='0'/> - <alias name='serial0'/> </console> <channel type='unix'> <source mode='bind' path='/var/lib/libvirt/qemu/f17x86_64.agent'/> <target type='virtio' name='org.qemu.guest_agent.0'/> - <alias name='channel0'/> <address type='virtio-serial' controller='0' bus='0' port='1'/> </channel> - <input type='tablet' bus='usb'> - <alias name='input0'/> - </input> + <input type='tablet' bus='usb'/> <input type='mouse' bus='ps2'/> <input type='keyboard' bus='ps2'/> - <graphics type='vnc' port='5900' autoport='yes' listen='0.0.0.0'> + <graphics type='vnc' port='-1' autoport='yes' listen='0.0.0.0'> <listen type='address' address='0.0.0.0'/> </graphics> <sound model='ich6'> - <alias name='sound0'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x0'/> </sound> <video> <model type='cirrus' vram='9216' heads='1'/> - <alias name='video0'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/> </video> <memballoon model='virtio'> - <alias name='balloon0'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x06' function='0x0'/> </memballoon> </devices> -- 1.7.1

2 1

[libvirt] [PATCH] build: Fix make distcheck
by Martin Kletzander 19 Mar '14

19 Mar '14

I forgot to delete the underscore in object_locking_SOURCES when changing the name in one of previous cleanups. Signed-off-by: Martin Kletzander <mkletzan(a)redhat.com> --- Notes: Pushed as a build-breaker. tests/Makefile.am | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/Makefile.am b/tests/Makefile.am index f80e7ad..90f70ff 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -1016,7 +1016,7 @@ CILOPTINCS = CILOPTPACKAGES = -package unix,str,cil CILOPTLIBS = -linkpkg -object_locking_SOURCES = objectlocking.ml +objectlocking_SOURCES = objectlocking.ml %.cmx: %.ml ocamlfind ocamlopt $(CILOPTFLAGS) $(CILOPTINCS) $(CILOPTPACKAGES) -c $< -- 1.9.0

1 0

[libvirt] [PATCH] nwfilter: Fix the test for the result of atomic dec and test
by Stefan Berger 19 Mar '14

19 Mar '14

From: Stefan Berger <stefanb(a)linux.vnet.ibm.com> https://bugzilla.redhat.com/show_bug.cgi?id=1071181 Commit 49b59a15 fixed one problem but masks another one related to pointer freeing. Use virAtomicIntGet() to test for 0 rather than trying to test for 'true' after virAtomicIntDecAndTest(). Avoid putting of the virNWFilterSnoopReq once the thread has been started. Signed-off-by: Stefan Berger <stefanb(a)linux.vnet.ibm.com> --- src/nwfilter/nwfilter_dhcpsnoop.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/nwfilter/nwfilter_dhcpsnoop.c b/src/nwfilter/nwfilter_dhcpsnoop.c index d2a8062..32ca304 100644 --- a/src/nwfilter/nwfilter_dhcpsnoop.c +++ b/src/nwfilter/nwfilter_dhcpsnoop.c @@ -720,7 +720,10 @@ virNWFilterSnoopReqPut(virNWFilterSnoopReqPtr req) virNWFilterSnoopLock(); - if (virAtomicIntDecAndTest(&req->refctr)) { + virAtomicIntDecAndTest(&req->refctr); + + /* make sure it's 0; virAtomitIntDecAndTest may return true on '1' */ + if (virAtomicIntGet(&req->refctr) == 0) { /* * delete the request: * - if we don't find req on the global list anymore @@ -1605,6 +1608,7 @@ virNWFilterDHCPSnoopReq(virNWFilterTechDriverPtr techdriver, int tmp; virThread thread; virNWFilterVarValuePtr dhcpsrvrs; + bool threadPuts = false; virNWFilterSnoopIFKeyFMT(ifkey, vmuuid, macaddr); @@ -1690,6 +1694,8 @@ virNWFilterDHCPSnoopReq(virNWFilterTechDriverPtr techdriver, /* prevent thread from holding req */ virNWFilterSnoopReqLock(req); + threadPuts = true; + if (virThreadCreate(&thread, false, virNWFilterDHCPSnoopThread, req) != 0) { virReportError(VIR_ERR_INTERNAL_ERROR, @@ -1737,7 +1743,8 @@ exit_rem_ifnametokey: exit_snoopunlock: virNWFilterSnoopUnlock(); exit_snoopreqput: - virNWFilterSnoopReqPut(req); + if (!threadPuts) + virNWFilterSnoopReqPut(req); return -1; } -- 1.8.1.4

2 2

[libvirt] [PATCH] build: Fix build with old automake
by Jiri Denemark 19 Mar '14

19 Mar '14

Ancient automake (such as from RHEL5) does not provide abs_srcdir and abs_builddir variables which are used by a recent commit of mine (e562e82). Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com> --- src/Makefile.am | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/Makefile.am b/src/Makefile.am index d4d7b2b..fce9056 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -16,6 +16,10 @@ ## License along with this library. If not, see ## <http://www.gnu.org/licenses/>. +# old automake does not provide abs_{src,build}dir variables +abs_builddir = $(shell pwd) +abs_srcdir = $(shell cd $(srcdir) && pwd) + # No libraries with the exception of LIBXML should be listed # here. List them against the individual XXX_la_CFLAGS targets # that actually use them. Also keep GETTEXT_CPPFLAGS at the end. -- 1.9.0

4 11

[libvirt] [PATCH] libxl: fix framebuffer port setting for HVM domains
by Jim Fehlig 19 Mar '14

19 Mar '14

libxl uses the libxl_vnc_info and libxl_sdl_info fields from the hvm union in libxl_domain_build_info struct when generating QEMU args for VNC or SDL. These fields were left unset by the libxl driver, causing libxl to ignore any user settings. E.g. with <graphics type='vnc' port='5950'/> port would be ignored and QEMU would instead be invoked with -vnc 127.0.0.1:0,to=99 Unlike the libxl_domain_config struct, the libxl_domain_build_info contains only a single libxl_vnc_info and libxl_sdl_info, so populate these fields from the first vfb in libxl_domain_config->vfbs. Signed-off-by: Jim Fehlig <jfehlig(a)suse.com> Signed-off-by: David Kiarie <davidkiarie4(a)gmail.com> --- src/libxl/libxl_conf.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c index aa5d958..cfac847 100644 --- a/src/libxl/libxl_conf.c +++ b/src/libxl/libxl_conf.c @@ -1022,6 +1022,20 @@ libxlMakeVfbList(libxlDriverPrivatePtr driver, d_config->vkbs = x_vkbs; d_config->num_vfbs = d_config->num_vkbs = nvfbs; + /* + * VNC or SDL info must also be set in libxl_domain_build_info + * for HVM domains. Use the first vfb device. + */ + if (STREQ(def->os.type, "hvm")) { + libxl_domain_build_info *b_info = &d_config->b_info; + libxl_device_vfb vfb = d_config->vfbs[0]; + + if (libxl_defbool_val(vfb.vnc.enable)) + memcpy(&b_info->u.hvm.vnc, &vfb.vnc, sizeof(libxl_vnc_info)); + else if (libxl_defbool_val(vfb.sdl.enable)) + memcpy(&b_info->u.hvm.sdl, &vfb.sdl, sizeof(libxl_sdl_info)); + } + return 0; error: -- 1.8.1.4

3 3

[libvirt] specifying cirrus ram size
by Serge Hallyn 19 Mar '14

19 Mar '14

Hi, In order to migrate a VM from an older system with qemu-kvm to a newer one with qemu, the newer qemu needs to be told to use the same vga ram size as qemu-kvm used, 8M. virsh domxml-from-native suggests that the way to specify a 8mb cirrus vga ram size would be to add <qemu:commandline> <qemu:arg value='-global'/> <qemu:arg value='cirrus-vga.vgamem_mb=8'/> </qemu:commandline> However whenever I edit a .xml to add this, it disappears. I'm adding it right before the ending </domain> Am I doing something silly? thanks, -serge

4 4

[libvirt] [PATCH] domiftune: Reword bandwidth clearing paragraph
by Michal Privoznik 19 Mar '14

19 Mar '14

s/of value/value of/ Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com> --- tools/virsh.pod | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/virsh.pod b/tools/virsh.pod index 3e721e0..0fb8248 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -699,7 +699,7 @@ in a single burst at -I<peak> speed as described in the Network XML documentation at L<http://libvirt.org/formatnetwork.html#elementQoS>. To clear inbound or outbound settings, use I<--inbound> or I<--outbound> -respectfully with average of value zero. +respectfully with average value of zero. If I<--live> is specified, affect a running guest. If I<--config> is specified, affect the next boot of a persistent guest. -- 1.9.0

3 2

[libvirt] [PATCH] qemuDomainSetInterfaceParameters: Allow bandwidth clear out
by Michal Privoznik 19 Mar '14

19 Mar '14

We allow translation from no_bandwidth to has_bandwidth for a vnic. However, going in the opposite direction is not implemented. It's not limitation of the API rather than internal implementation. The problem is, we correctly detect that user hasn't specified any outbound (say he wants to clear out outbound). However, this gets overwritten by current vnic outbound settings. Then, virNetDevBandwidthSet doesn't change anything. We need to stop overwriting the outbound if users don't want us to. Same applies for inbound. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com> --- src/qemu/qemu_driver.c | 15 +++++++++++---- tools/virsh-domain.c | 4 ++-- tools/virsh.pod | 3 +++ 3 files changed, 16 insertions(+), 6 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index fc382a5..26ac79b 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -9782,6 +9782,7 @@ qemuDomainSetInterfaceParameters(virDomainPtr dom, virNetDevBandwidthPtr bandwidth = NULL, newBandwidth = NULL; virQEMUDriverConfigPtr cfg = NULL; virCapsPtr caps = NULL; + bool inboundSpecified = false, outboundSpecified = false; virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | VIR_DOMAIN_AFFECT_CONFIG, -1); @@ -9843,12 +9844,14 @@ qemuDomainSetInterfaceParameters(virDomainPtr dom, if (STREQ(param->field, VIR_DOMAIN_BANDWIDTH_IN_AVERAGE)) { bandwidth->in->average = params[i].value.ui; + inboundSpecified = true; } else if (STREQ(param->field, VIR_DOMAIN_BANDWIDTH_IN_PEAK)) { bandwidth->in->peak = params[i].value.ui; } else if (STREQ(param->field, VIR_DOMAIN_BANDWIDTH_IN_BURST)) { bandwidth->in->burst = params[i].value.ui; } else if (STREQ(param->field, VIR_DOMAIN_BANDWIDTH_OUT_AVERAGE)) { bandwidth->out->average = params[i].value.ui; + outboundSpecified = true; } else if (STREQ(param->field, VIR_DOMAIN_BANDWIDTH_OUT_PEAK)) { bandwidth->out->peak = params[i].value.ui; } else if (STREQ(param->field, VIR_DOMAIN_BANDWIDTH_OUT_BURST)) { @@ -9874,7 +9877,7 @@ qemuDomainSetInterfaceParameters(virDomainPtr dom, * bandwidth parameters, so merge with old bandwidth parameters * here to prevent them from being lost. */ if (bandwidth->in || - (net->bandwidth && net->bandwidth->in)) { + (!inboundSpecified && net->bandwidth && net->bandwidth->in)) { if (VIR_ALLOC(newBandwidth->in) < 0) goto cleanup; @@ -9883,7 +9886,7 @@ qemuDomainSetInterfaceParameters(virDomainPtr dom, sizeof(*newBandwidth->in)); } if (bandwidth->out || - (net->bandwidth && net->bandwidth->out)) { + (!outboundSpecified && net->bandwidth && net->bandwidth->out)) { if (VIR_ALLOC(newBandwidth->out) < 0) goto cleanup; @@ -9900,8 +9903,12 @@ qemuDomainSetInterfaceParameters(virDomainPtr dom, } virNetDevBandwidthFree(net->bandwidth); - net->bandwidth = newBandwidth; - newBandwidth = NULL; + if (newBandwidth->in || newBandwidth->out) { + net->bandwidth = newBandwidth; + newBandwidth = NULL; + } else { + net->bandwidth = NULL; + } } if (flags & VIR_DOMAIN_AFFECT_CONFIG) { if (!persistentNet->bandwidth) { diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index fbde3da..cb6bf63 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -2615,7 +2615,7 @@ cmdDomIftune(vshControl *ctl, const vshCmd *cmd) vshError(ctl, _("inbound format is incorrect")); goto cleanup; } - if (inbound.average == 0) { + if (inbound.average == 0 && (inbound.burst || inbound.peak)) { vshError(ctl, _("inbound average is mandatory")); goto cleanup; } @@ -2643,7 +2643,7 @@ cmdDomIftune(vshControl *ctl, const vshCmd *cmd) vshError(ctl, _("outbound format is incorrect")); goto cleanup; } - if (outbound.average == 0) { + if (outbound.average == 0 && (outbound.burst || outbound.peak)) { vshError(ctl, _("outbound average is mandatory")); goto cleanup; } diff --git a/tools/virsh.pod b/tools/virsh.pod index 5c74bb3..3e721e0 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -698,6 +698,9 @@ expressed in kilobytes per second, while I<burst> is expressed in kilobytes in a single burst at -I<peak> speed as described in the Network XML documentation at L<http://libvirt.org/formatnetwork.html#elementQoS>. +To clear inbound or outbound settings, use I<--inbound> or I<--outbound> +respectfully with average of value zero. + If I<--live> is specified, affect a running guest. If I<--config> is specified, affect the next boot of a persistent guest. If I<--current> is specified, affect the current guest state. -- 1.9.0

2 2

[libvirt] [PATCH v2] Add ability to register callback for virCommand dry run
by Daniel P. Berrange 19 Mar '14

19 Mar '14

To allow for fault injection of the virCommand dry run, add the ability to register a callback. The callback will be passed the argv, env and stdin buffer and is expected to return the exit status and optionally fill stdout and stderr buffers. Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com> --- src/util/vircommand.c | 52 ++++++++++++++++++++++++++++++++---------- src/util/vircommand.h | 1 + src/util/vircommandpriv.h | 13 ++++++++++- tests/virkmodtest.c | 8 +++---- tests/virnetdevbandwidthtest.c | 3 ++- 5 files changed, 59 insertions(+), 18 deletions(-) diff --git a/src/util/vircommand.c b/src/util/vircommand.c index ff135bf..79bb20c 100644 --- a/src/util/vircommand.c +++ b/src/util/vircommand.c @@ -135,6 +135,9 @@ struct _virCommand { /* See virCommandSetDryRun for description for this variable */ static virBufferPtr dryRunBuffer; +static virCommandDryRunCallback dryRunCallback; +static void *dryRunOpaque; +static int dryRunStatus; /* * virCommandFDIsSet: @@ -1860,6 +1863,11 @@ virCommandProcessIO(virCommandPtr cmd) size_t inoff = 0; int ret = 0; + if (dryRunBuffer || dryRunCallback) { + VIR_DEBUG("Dry run requested, skipping I/O processing"); + return 0; + } + /* With an input buffer, feed data to child * via pipe */ if (cmd->inbuf) @@ -2267,16 +2275,25 @@ virCommandRunAsync(virCommandPtr cmd, pid_t *pid) } str = virCommandToString(cmd); - if (dryRunBuffer) { + if (dryRunBuffer || dryRunCallback) { + dryRunStatus = 0; if (!str) { /* error already reported by virCommandToString */ goto cleanup; } - VIR_DEBUG("Dry run requested, appending stringified " - "command to dryRunBuffer=%p", dryRunBuffer); - virBufferAdd(dryRunBuffer, str, -1); - virBufferAddChar(dryRunBuffer, '\n'); + if (dryRunBuffer) { + VIR_DEBUG("Dry run requested, appending stringified " + "command to dryRunBuffer=%p", dryRunBuffer); + virBufferAdd(dryRunBuffer, str, -1); + virBufferAddChar(dryRunBuffer, '\n'); + } + if (dryRunCallback) { + dryRunCallback((const char *const*)cmd->args, + (const char *const*)cmd->env, + cmd->inbuf, cmd->outbuf, cmd->errbuf, + &dryRunStatus, dryRunOpaque); + } ret = 0; goto cleanup; } @@ -2356,10 +2373,11 @@ virCommandWait(virCommandPtr cmd, int *exitstatus) return -1; } - if (dryRunBuffer) { - VIR_DEBUG("Dry run requested, claiming success"); + if (dryRunBuffer || dryRunCallback) { + VIR_DEBUG("Dry run requested, returning status %d", + dryRunStatus); if (exitstatus) - *exitstatus = 0; + *exitstatus = dryRunStatus; return 0; } @@ -2704,6 +2722,7 @@ virCommandDoAsyncIO(virCommandPtr cmd) /** * virCommandSetDryRun: * @buf: buffer to store stringified commands + * @callback: callback to process input/output/args * * Sometimes it's desired to not actually run given command, but * see its string representation without having to change the @@ -2712,8 +2731,13 @@ virCommandDoAsyncIO(virCommandPtr cmd) * virCommandRun* API. The virCommandSetDryRun allows you to * modify this behavior: once called, every call to * virCommandRun* results in command string representation being - * appended to @buf instead of being executed. the strings are - * escaped for a shell and separated by a newline. For example: + * appended to @buf instead of being executed. If @callback is + * provided, then it is invoked with the argv, env and stdin + * data string for the command. It is expected to fill the stdout + * and stderr data strings and exit status variables. + * + * The strings stored in @buf are escaped for a shell and + * separated by a newline. For example: * * virBuffer buffer = VIR_BUFFER_INITIALIZER; * virCommandSetDryRun(&buffer); @@ -2725,10 +2749,14 @@ virCommandDoAsyncIO(virCommandPtr cmd) * * /bin/echo 'Hello world'\n * - * To cancel this effect pass NULL. + * To cancel this effect pass NULL for @buf and @callback. */ void -virCommandSetDryRun(virBufferPtr buf) +virCommandSetDryRun(virBufferPtr buf, + virCommandDryRunCallback cb, + void *opaque) { dryRunBuffer = buf; + dryRunCallback = cb; + dryRunOpaque = opaque; } diff --git a/src/util/vircommand.h b/src/util/vircommand.h index 10b4fa2..929375b 100644 --- a/src/util/vircommand.h +++ b/src/util/vircommand.h @@ -186,4 +186,5 @@ void virCommandAbort(virCommandPtr cmd); void virCommandFree(virCommandPtr cmd); void virCommandDoAsyncIO(virCommandPtr cmd); + #endif /* __VIR_COMMAND_H__ */ diff --git a/src/util/vircommandpriv.h b/src/util/vircommandpriv.h index 80247b2..8930299 100644 --- a/src/util/vircommandpriv.h +++ b/src/util/vircommandpriv.h @@ -28,5 +28,16 @@ # include "vircommand.h" -void virCommandSetDryRun(virBufferPtr buf); +typedef void (*virCommandDryRunCallback)(const char *const*args, + const char *const*env, + const char *input, + char **output, + char **error, + int *status, + void *opaque); + +void virCommandSetDryRun(virBufferPtr buf, + virCommandDryRunCallback cb, + void *opaque); + #endif /* __VIR_COMMAND_PRIV_H__ */ diff --git a/tests/virkmodtest.c b/tests/virkmodtest.c index 8899689..9473352 100644 --- a/tests/virkmodtest.c +++ b/tests/virkmodtest.c @@ -96,7 +96,7 @@ testKModLoad(const void *args) bool useBlacklist = info->useBlacklist; virBuffer buf = VIR_BUFFER_INITIALIZER; - virCommandSetDryRun(&buf); + virCommandSetDryRun(&buf, NULL, NULL); errbuf = virKModLoad(module, useBlacklist); if (errbuf) { @@ -110,7 +110,7 @@ testKModLoad(const void *args) ret = 0; cleanup: - virCommandSetDryRun(NULL); + virCommandSetDryRun(NULL, NULL, NULL); VIR_FREE(errbuf); return ret; } @@ -125,7 +125,7 @@ testKModUnload(const void *args) const char *module = info->module; virBuffer buf = VIR_BUFFER_INITIALIZER; - virCommandSetDryRun(&buf); + virCommandSetDryRun(&buf, NULL, NULL); errbuf = virKModUnload(module); if (errbuf) { @@ -139,7 +139,7 @@ testKModUnload(const void *args) ret = 0; cleanup: - virCommandSetDryRun(NULL); + virCommandSetDryRun(NULL, NULL, NULL); VIR_FREE(errbuf); return ret; } diff --git a/tests/virnetdevbandwidthtest.c b/tests/virnetdevbandwidthtest.c index 38dfbc1..f5d252c 100644 --- a/tests/virnetdevbandwidthtest.c +++ b/tests/virnetdevbandwidthtest.c @@ -77,7 +77,7 @@ testVirNetDevBandwidthSet(const void *data) if (!iface) iface = "eth0"; - virCommandSetDryRun(&buf); + virCommandSetDryRun(&buf, NULL, NULL); if (virNetDevBandwidthSet(iface, band, info->hierarchical_class) < 0) goto cleanup; @@ -101,6 +101,7 @@ testVirNetDevBandwidthSet(const void *data) ret = 0; cleanup: + virCommandSetDryRun(NULL, NULL, NULL); virNetDevBandwidthFree(band); virBufferFreeAndReset(&buf); VIR_FREE(actual_cmd); -- 1.8.5.3

2 1

[libvirt] [PATCH] virhostdev.h: remove ATTRIBUTE_NONNULL to oldStateDir
by Chunyan Liu 19 Mar '14

19 Mar '14

For libxl driver usage, it didn't support hostdev passthrough before, oldStateDir is NULL when calling virHostdevReAttachDomainHostdevs. That is allowed. Remove ATTRIBUTE_NONNULL setting to oldStateDir. Signed-off-by: Chunyan Liu <cyliu(a)suse.com> --- src/util/virhostdev.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/util/virhostdev.h b/src/util/virhostdev.h index a56a2b5..2036430 100644 --- a/src/util/virhostdev.h +++ b/src/util/virhostdev.h @@ -142,7 +142,7 @@ virHostdevReAttachDomainDevices(virHostdevManagerPtr mgr, virDomainDefPtr def, unsigned int flags, const char *oldStateDir) - ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3) ATTRIBUTE_NONNULL(5); + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3); /* functions used by NodeDevDetach/Reattach/Reset */ int virHostdevPCINodeDeviceDetach(virHostdevManagerPtr mgr, -- 1.7.12.4

2 1

[libvirt] [PATCH] maint: avoid unbalanced {} across vbox #ifdef
by Eric Blake 18 Mar '14

18 Mar '14

Emacs is fairly good about navigating across function and scope boundaries, provided that the code has balanced {}. The vbox code, however, violated that premise, by splitting 'if () {' across several #ifdef branches, but sharing the '} else {...}' outside of the branches. The extra lines of code is worth my sanity, in a function that is already a horrendous 1100+ lines long. Bad nesting is hard to maintain. * src/vbox/vbox_tmpl.c (vboxDomainGetXMLDesc) Duplicate code rather than trying to share else branch across #ifdef. Signed-off-by: Eric Blake <eblake(a)redhat.com> --- Pushing under the trivial rule. I'm tired of '^X-4-a' pulling up a changelog template for the wrong function, only to have to scroll through several hundred lines of #ifdef mess to find what function I'm really modifying. src/vbox/vbox_tmpl.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/vbox/vbox_tmpl.c b/src/vbox/vbox_tmpl.c index 7162a25..2aeddd0 100644 --- a/src/vbox/vbox_tmpl.c +++ b/src/vbox/vbox_tmpl.c @@ -2649,6 +2649,9 @@ static char *vboxDomainGetXMLDesc(virDomainPtr dom, unsigned int flags) { VRDxServer->vtbl->GetPort(VRDxServer, &VRDPport); if (VRDPport) { def->graphics[def->ngraphics]->data.rdp.port = VRDPport; + } else { + def->graphics[def->ngraphics]->data.rdp.autoport = true; + } #elif VBOX_API_VERSION < 4000000 /* 3001000 <= VBOX_API_VERSION < 4000000 */ PRUnichar *VRDPport = NULL; VRDxServer->vtbl->GetPorts(VRDxServer, &VRDPport); @@ -2656,6 +2659,9 @@ static char *vboxDomainGetXMLDesc(virDomainPtr dom, unsigned int flags) { /* even if vbox supports mutilpe ports, single port for now here */ def->graphics[def->ngraphics]->data.rdp.port = PRUnicharToInt(VRDPport); VBOX_UTF16_FREE(VRDPport); + } else { + def->graphics[def->ngraphics]->data.rdp.autoport = true; + } #else /* VBOX_API_VERSION >= 4000000 */ PRUnichar *VRDEPortsKey = NULL; PRUnichar *VRDEPortsValue = NULL; @@ -2666,10 +2672,10 @@ static char *vboxDomainGetXMLDesc(virDomainPtr dom, unsigned int flags) { /* even if vbox supports mutilpe ports, single port for now here */ def->graphics[def->ngraphics]->data.rdp.port = PRUnicharToInt(VRDEPortsValue); VBOX_UTF16_FREE(VRDEPortsValue); -#endif /* VBOX_API_VERSION >= 4000000 */ } else { def->graphics[def->ngraphics]->data.rdp.autoport = true; } +#endif /* VBOX_API_VERSION >= 4000000 */ def->graphics[def->ngraphics]->type = VIR_DOMAIN_GRAPHICS_TYPE_RDP; -- 1.8.5.3

1 0

[libvirt] [PATCH 0/6] Misc cleanups to nwfilter code
by Daniel P. Berrange 18 Mar '14

18 Mar '14

This mini-series performs a few style cleanups on the nwfilter code. There should be no functional change in any of these patches. Daniel P. Berrange (6): Change 'int incoming' to 'bool incoming' in nwfilter code Remove pointless brackets around boolean Remove 'int stopOnError' parameters in nwfilter methods Remove pointless return values in nwfilter methods Change 'int isTempChain' to bool in nwfilter Change CMD_STOPONERR(1) to use true src/nwfilter/nwfilter_ebiptables_driver.c | 523 ++++++++++++++---------------- 1 file changed, 241 insertions(+), 282 deletions(-) -- 1.8.5.3

2 15

Re: [libvirt] [PATCH 00/13] Add multiple trace backend function and add new ftrace backend for libvirt
by yangzy.fnst＠cn.fujitsu.com 18 Mar '14

18 Mar '14

> [...] > (For what it's worth, there are several different ways of executing systemtap > scripts automatically at startup. There is an initscript. > With the next version, its docs will move into 'man 8 systemtap', and grows a > new 'onboot' option, to modules load even earlier at initramfs time via dracut.) What 'onboot' option it is ? BTW, do you mean that ftrace backend is not necessary for libvirt now? Can it generate systemtap script automatically and load the module on boot time without users operation? We think adding multiple trace backends can provide more trace interfaces for the upper tool which is based on libvirt. Developer can chose more compatible trace scheme according to the actual situation with more flexibility . In the ways of usage by common user, ftrace is more lightweight ,more compact and easier. After all, writing systemTap script by users themselves and making it as an initscript is not convenient for them. Regards Yang Zhiyong > [...]

2 1

[libvirt] uses of /etc/libvirt/<driver>/
by Jim Fehlig 18 Mar '14

18 Mar '14

I received a report about an odd use case of /etc/libvirt/<driver>/ config files, and would like to hear some opinions about it. The user "preps" a host by mounting a remote fs containing VM images and config, creates links in /etc/libvirt/<driver>/dom.xml to /mnt-point/whatever/dom.xml, and starts libvirtd. All is well until there is a need to modify the VM config (e.g. virsh setmaxmem ... --config), at which point libvirt replaces the link with a file containing the new config, instead of updating the contents of the linked file. I suppose I've always considered the contents of /etc/libvirt/<driver>/ private to libvirt, with a "modify at your own risk" warning, ignoring that it is user configuration in /etc. What are the guidelines for modifying the contents of these directories? Would the above be considered valid use? Regards, Jim

4 4

[libvirt] [PATCH python 0/2] Add unit test framework
by Daniel P. Berrange 18 Mar '14

18 Mar '14

The bug fix of Michal's to handling of typed parameters demonstrated a clear need for a unit test framework for python bindings. By using the test:///default driver we can easily test a great many python APIs. Once the test driver has full API coverage, we can in fact unit test all the python bindings. The first patch does the bare minimum to plumb the unit test framework into 'python setup test.py' (requires python-nose RPM to be present) and does a test for the virConnectListAllDomains API. The second patch tests Michal's previous bug fix - this test will fail on current GIT and Michal's patch makes it pass. A nice project for someone wanting to learn more about libvirt and the python binding is to flesh out this test suite to cover as many APIs as possible. We should aim to have an addition to this test suite for any bugs we find in the bindings henceforth. Daniel P. Berrange (2): Add support for running unit tests with nose Add test for setting scheduler parameters setup.py | 5 +++++ tests/test_conn.py | 16 ++++++++++++++++ tests/test_domain.py | 19 +++++++++++++++++++ 3 files changed, 40 insertions(+) create mode 100644 tests/test_conn.py create mode 100644 tests/test_domain.py -- 1.8.5.3

2 3

[libvirt] [PATCH] cfg.mk: Fix whitespaces
by Martin Kletzander 18 Mar '14

18 Mar '14

Just to align the backslashes with most of the file. Signed-off-by: Martin Kletzander <mkletzan(a)redhat.com> --- Notes: Pushed as 'trivial'. cfg.mk | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/cfg.mk b/cfg.mk index e75323e..319210b 100644 --- a/cfg.mk +++ b/cfg.mk @@ -893,9 +893,9 @@ sc_prohibit_wrong_filename_in_comment: fi; sc_prohibit_virConnectOpen_in_virsh: - @prohibit='\bvirConnectOpen[a-zA-Z]* *\(' \ - in_vc_files='^tools/virsh-.*\.[ch]$$' \ - halt='Use vshConnect() in virsh instead of virConnectOpen*' \ + @prohibit='\bvirConnectOpen[a-zA-Z]* *\(' \ + in_vc_files='^tools/virsh-.*\.[ch]$$' \ + halt='Use vshConnect() in virsh instead of virConnectOpen*' \ $(_sc_search_regexp) -- 1.9.0

1 0

[libvirt] [PATCH] Remove double space in file name comment check
by Ján Tomko 18 Mar '14

18 Mar '14

Reported by Martin Kletzander --- Pushed as trivial. cfg.mk | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cfg.mk b/cfg.mk index a2fed11..e75323e 100644 --- a/cfg.mk +++ b/cfg.mk @@ -888,7 +888,7 @@ sc_prohibit_wrong_filename_in_comment: } \ }' $$($(VC_LIST_EXCEPT) | grep '\.[ch]$$') || fail=1; \ if test $$fail -eq 1; then \ - { echo '$(ME): The file name in comments must match the ' \ + { echo '$(ME): The file name in comments must match the' \ 'actual file name' 1>&2; exit 1; } \ fi; -- 1.8.3.2

1 0

[libvirt] [PATCH 0/7] Various cleanups
by Martin Kletzander 18 Mar '14

18 Mar '14

Just getting rid of my local 'cleanup' branch. Some are trivial, but every patch should be able to work on its own, I'm sending it in a series not to mess up the list. Martin Kletzander (7): qemu: agent availability cleanup tests: cleanup object-locking test Fix inconsistency in using curly braces around functions Don't leave empty first line in C source files Explicitly cast some switch parameters to enum Remove duplicate network model characters Require spaces around equality comparisons .gitignore | 6 +- build-aux/bracket-spacing.pl | 9 +- daemon/libvirtd-config.c | 5 +- docs/internals/command.html.in | 3 +- examples/dominfo/info1.c | 7 +- src/conf/domain_conf.c | 30 ++-- src/conf/domain_nwfilter.c | 13 +- src/conf/interface_conf.c | 54 ++++--- src/conf/nwfilter_conf.c | 21 ++- src/conf/nwfilter_params.c | 3 +- src/esx/esx_device_monitor.c | 1 - src/esx/esx_device_monitor.h | 1 - src/esx/esx_driver.c | 1 - src/esx/esx_driver.h | 1 - src/esx/esx_interface_driver.c | 1 - src/esx/esx_interface_driver.h | 1 - src/esx/esx_network_driver.c | 1 - src/esx/esx_network_driver.h | 1 - src/esx/esx_nwfilter_driver.c | 1 - src/esx/esx_nwfilter_driver.h | 1 - src/esx/esx_private.h | 1 - src/esx/esx_secret_driver.c | 1 - src/esx/esx_secret_driver.h | 1 - src/esx/esx_storage_backend_iscsi.c | 1 - src/esx/esx_storage_backend_iscsi.h | 1 - src/esx/esx_storage_backend_vmfs.c | 1 - src/esx/esx_storage_backend_vmfs.h | 1 - src/esx/esx_storage_driver.c | 1 - src/esx/esx_storage_driver.h | 1 - src/esx/esx_util.c | 1 - src/esx/esx_vi.c | 1 - src/esx/esx_vi.h | 1 - src/esx/esx_vi_methods.c | 1 - src/esx/esx_vi_methods.h | 1 - src/esx/esx_vi_types.c | 1 - src/esx/esx_vi_types.h | 1 - src/hyperv/hyperv_device_monitor.c | 1 - src/hyperv/hyperv_device_monitor.h | 1 - src/hyperv/hyperv_driver.c | 1 - src/hyperv/hyperv_driver.h | 1 - src/hyperv/hyperv_interface_driver.c | 1 - src/hyperv/hyperv_interface_driver.h | 1 - src/hyperv/hyperv_network_driver.c | 1 - src/hyperv/hyperv_network_driver.h | 1 - src/hyperv/hyperv_nwfilter_driver.c | 1 - src/hyperv/hyperv_nwfilter_driver.h | 1 - src/hyperv/hyperv_private.h | 1 - src/hyperv/hyperv_secret_driver.c | 1 - src/hyperv/hyperv_secret_driver.h | 1 - src/hyperv/hyperv_storage_driver.c | 1 - src/hyperv/hyperv_storage_driver.h | 1 - src/hyperv/hyperv_util.c | 1 - src/hyperv/hyperv_util.h | 1 - src/hyperv/hyperv_wmi.c | 1 - src/hyperv/hyperv_wmi.h | 1 - src/hyperv/hyperv_wmi_classes.c | 1 - src/hyperv/hyperv_wmi_classes.h | 1 - src/hyperv/openwsman.h | 1 - src/interface/interface_backend_netcf.c | 5 +- src/libvirt.c | 2 +- src/libxl/libxl_driver.c | 3 +- src/locking/lock_driver_sanlock.c | 4 +- src/lxc/lxc_driver.c | 27 ++-- src/network/bridge_driver.c | 45 ++++-- src/nwfilter/nwfilter_driver.c | 26 ++-- src/nwfilter/nwfilter_learnipaddr.c | 38 +++-- src/openvz/openvz_conf.c | 15 +- src/openvz/openvz_driver.c | 42 ++++-- src/qemu/qemu_agent.c | 5 +- src/qemu/qemu_command.c | 16 ++- src/qemu/qemu_domain.c | 22 +++ src/qemu/qemu_domain.h | 4 + src/qemu/qemu_driver.c | 193 +++++++++----------------- src/qemu/qemu_migration.c | 4 +- src/qemu/qemu_monitor.c | 3 +- src/qemu/qemu_monitor_json.c | 3 +- src/qemu/qemu_monitor_text.c | 17 ++- src/rpc/virnetclient.c | 10 +- src/rpc/virnetserver.c | 8 +- src/rpc/virnetserverclient.c | 5 +- src/rpc/virnettlscontext.c | 5 +- src/secret/secret_driver.c | 8 +- src/security/security_stack.c | 5 +- src/security/virt-aa-helper.c | 1 - src/storage/storage_backend_fs.c | 12 +- src/storage/storage_driver.c | 72 ++++++---- src/test/test_driver.c | 135 ++++++++++++------ src/uml/uml_driver.c | 69 ++++++--- src/util/vircgroup.c | 12 +- src/util/virconf.c | 5 +- src/util/virdbus.c | 8 +- src/util/virerror.c | 3 +- src/util/vireventpoll.c | 20 ++- src/util/virhook.c | 8 +- src/util/virrandom.c | 5 +- src/util/virsocketaddr.c | 26 ++-- src/util/virthread.c | 5 +- src/util/virthreadpool.c | 3 +- src/util/virutil.c | 17 ++- src/util/viruuid.c | 5 +- src/vbox/vbox_MSCOMGlue.c | 1 - src/vbox/vbox_MSCOMGlue.h | 1 - src/vbox/vbox_glue.c | 1 - src/vbox/vbox_glue.h | 1 - src/vbox/vbox_tmpl.c | 179 ++++++++++++++++-------- src/vmx/vmx.h | 1 - src/xen/xen_driver.c | 6 +- src/xen/xm_internal.c | 8 +- src/xenapi/xenapi_driver.c | 4 +- src/xenapi/xenapi_utils.c | 5 +- src/xenxs/xen_xm.c | 32 +++-- tests/Makefile.am | 12 +- tests/commandhelper.c | 5 +- tests/commandtest.c | 2 +- tests/domainsnapshotxml2xmltest.c | 2 +- tests/interfacexml2xmltest.c | 2 +- tests/libvirtdconftest.c | 4 +- tests/lxcxml2xmltest.c | 2 +- tests/networkxml2conftest.c | 2 +- tests/networkxml2xmltest.c | 2 +- tests/nodedevxml2xmltest.c | 2 +- tests/nodeinfotest.c | 2 +- tests/nwfilterxml2xmltest.c | 2 +- tests/{object-locking.ml => objectlocking.ml} | 10 +- tests/qemuargv2xmltest.c | 5 +- tests/qemuxml2argvtest.c | 2 +- tests/qemuxml2xmltest.c | 2 +- tests/qemuxmlnstest.c | 2 +- tests/sexpr2xmltest.c | 2 +- tests/sockettest.c | 4 +- tests/statstest.c | 2 +- tests/storagepoolxml2xmltest.c | 2 +- tests/storagevolxml2argvtest.c | 2 +- tests/storagevolxml2xmltest.c | 2 +- tests/testutils.c | 6 +- tests/testutilslxc.h | 1 - tests/testutilsxen.h | 1 - tests/virauthconfigtest.c | 4 +- tests/virbuftest.c | 2 +- tests/vircgrouptest.c | 2 +- tests/virdbustest.c | 4 +- tests/virdrivermoduletest.c | 4 +- tests/virhostdevtest.c | 2 +- tests/viridentitytest.c | 4 +- tests/virkeyfiletest.c | 4 +- tests/virkmodtest.c | 2 +- tests/virlockspacetest.c | 4 +- tests/virnetmessagetest.c | 4 +- tests/virnetsockettest.c | 2 +- tests/virnettlscontexttest.c | 4 +- tests/virnettlssessiontest.c | 4 +- tests/virpcitest.c | 4 +- tests/virportallocatortest.c | 2 +- tests/virshtest.c | 56 +++++--- tests/virstringtest.c | 5 +- tests/virsystemdtest.c | 4 +- tests/virtimetest.c | 4 +- tests/viruritest.c | 4 +- tests/virusbtest.c | 3 +- tests/xencapstest.c | 35 +++-- tests/xmconfigtest.c | 4 +- tests/xml2sexprtest.c | 3 +- tools/virsh-domain-monitor.c | 2 +- tools/virsh-pool.c | 2 +- 164 files changed, 936 insertions(+), 679 deletions(-) rename tests/{object-locking.ml => objectlocking.ml} (98%) -- 1.9.0

3 22

[libvirt] [PATCH] virNetServerNewPostExecRestart: Initialize @max_anonymous_clients
by Michal Privoznik 18 Mar '14

18 Mar '14

Coverity spotted a use of possibly undefined variable. If a server is restarting as an result of update, the JSON file that keeps current value of some variables will not contain the new variables. This is the case of @max_anonymous_clients too. We are correctly querying if there's "max_anonymous_clients" in the JSON, however, we are not setting a sane default if there's none. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com> --- src/rpc/virnetserver.c | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/src/rpc/virnetserver.c b/src/rpc/virnetserver.c index dfa3565..33a0346 100644 --- a/src/rpc/virnetserver.c +++ b/src/rpc/virnetserver.c @@ -494,12 +494,15 @@ virNetServerPtr virNetServerNewPostExecRestart(virJSONValuePtr object, _("Missing max_clients data in JSON document")); goto error; } - if (virJSONValueObjectHasKey(object, "max_anonymous_clients") && - virJSONValueObjectGetNumberUint(object, "max_anonymous_clients", - &max_anonymous_clients) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Malformed max_anonymous_clients data in JSON document")); - goto error; + if (virJSONValueObjectHasKey(object, "max_anonymous_clients")) { + if (virJSONValueObjectGetNumberUint(object, "max_anonymous_clients", + &max_anonymous_clients) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Malformed max_anonymous_clients data in JSON document")); + goto error; + } + } else { + max_anonymous_clients = max_clients; } if (virJSONValueObjectGetNumberUint(object, "keepaliveInterval", &keepaliveInterval) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", -- 1.9.0

2 1

[libvirt] [PATCH libvirt-python 1/2] setPyVirTypedParameter: Copy full field name
by Michal Privoznik 18 Mar '14

18 Mar '14

In the setPyVirTypedParameter we try to produce virTypedParameter array from a pyhthon dictionary. However, when copying field name into item in returned array, we use strncpy() as the field name is fixed length array. To determine its size we use sizeof() but mistakenly dereference it resulting in sizeof(char) which equals to 1 byte. Moreover, there's no need for using sizeof() when we have a global macro to tell us the length of the field name: VIR_TYPED_PARAM_FIELD_LENGTH. And since array is allocated using VIR_ALLOC() we are sure the memory is initially filled with zeros. Hence, there's no need to terminate string we've just copied into field name with '\0' character. It's there for sure too as we copy up to field length - 1. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com> --- libvirt-override.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/libvirt-override.c b/libvirt-override.c index 83369fc..3765a43 100644 --- a/libvirt-override.c +++ b/libvirt-override.c @@ -197,8 +197,7 @@ setPyVirTypedParameter(PyObject *info, goto cleanup; } - strncpy(temp->field, keystr, sizeof(*temp->field) - 1); - temp->field[sizeof(*temp->field) - 1] = '\0'; + strncpy(temp->field, keystr, VIR_TYPED_PARAM_FIELD_LENGTH - 1); temp->type = params[i].type; VIR_FREE(keystr); -- 1.9.0

2 3

[libvirt] [PATCH] Fix memory leak in virDomainChrSourceDefClear()
by Nehal J Wani 18 Mar '14

18 Mar '14

While running qemuxml2xmltest, it was found that valgrind pointed out the following memory leak: ==21905== 26 bytes in 1 blocks are definitely lost in loss record 23 of 69 ==21905== at 0x4A069EE: malloc (vg_replace_malloc.c:270) ==21905== by 0x3E782A754D: xmlStrndup (in /usr/lib64/libxml2.so.2.7.6) ==21905== by 0x4CD986D: virDomainChrSourceDefParseXML (domain_conf.c:7233) ==21905== by 0x4CE4199: virDomainChrDefParseXML (domain_conf.c:7512) ==21905== by 0x4CFAF3F: virDomainDefParseXML (domain_conf.c:12303) ==21905== by 0x4CFB46E: virDomainDefParseNode (domain_conf.c:13031) ==21905== by 0x4CFB5E9: virDomainDefParse (domain_conf.c:12973) ==21905== by 0x41E9D8: testCompareXMLToXMLFiles (qemuxml2xmltest.c:40) ==21905== by 0x41EBAA: testCompareXMLToXMLHelper (qemuxml2xmltest.c:93) ==21905== by 0x421D21: virtTestRun (testutils.c:199) ==21905== by 0x41FCE9: mymain.part.0 (qemuxml2xmltest.c:244) ==21905== by 0x42249D: virtTestMain (testutils.c:782) ==21905== ... and 7 more --- The leaks were specific to the tests: DO_TEST("serial-spiceport"); DO_TEST("serial-spiceport-nospice"); src/conf/domain_conf.c | 4 ++++ 1 files changed, 4 insertions(+), 0 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 92b1324..f633db7 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -1495,6 +1495,10 @@ virDomainChrSourceDefClear(virDomainChrSourceDefPtr def) case VIR_DOMAIN_CHR_TYPE_UNIX: VIR_FREE(def->data.nix.path); break; + + case VIR_DOMAIN_CHR_TYPE_SPICEPORT: + VIR_FREE(def->data.spiceport.channel); + break; } } -- 1.7.1

2 1

[libvirt] Python setInterfaceParameters function broken
by Brian Rak 18 Mar '14

18 Mar '14

I've been trying to use domain.setInterfaceParameters, and I'm finding it's throwing the mysterious error: libvirt.libvirtError: argument unsupported: parameter '' not supported Enabling debug mode in libvirt reveals this: 2014-03-17 18:41:33.780+0000: 16927: debug : virDomainSetInterfaceParameters:8001 : dom=0x7f2ecc0009b0, (VM: name=SRVID8736, uuid=5121c1d6-19aa-4494-b6a7-09af270f24da), device=vnet0, params=0x7f2ecc0011f0, nparams=6, flags=0 2014-03-17 18:41:33.780+0000: 16927: debug : virDomainSetInterfaceParameters:8002 : params[""]=(uint)0 2014-03-17 18:41:33.780+0000: 16927: debug : virDomainSetInterfaceParameters:8002 : params[""]=(uint)64000 2014-03-17 18:41:33.780+0000: 16927: debug : virDomainSetInterfaceParameters:8002 : params[""]=(uint)10 2014-03-17 18:41:33.780+0000: 16927: debug : virDomainSetInterfaceParameters:8002 : params[""]=(uint)64000 2014-03-17 18:41:33.780+0000: 16927: debug : virDomainSetInterfaceParameters:8002 : params[""]=(uint)0 2014-03-17 18:41:33.780+0000: 16927: debug : virDomainSetInterfaceParameters:8002 : params[""]=(uint)0 (irreverent lines trimmed) Which ultimately led me to setPyVirTypedParameter lines 200-202. These don't seem to work properly. I added some debugging: strncpy(temp->field, keystr, sizeof(*temp->field) - 1); temp->field[sizeof(*temp->field) - 1] = '\0'; DEBUG("current key name in: %s, out: %s field size: %i\n", keystr, temp->field, sizeof(*temp->field) - 1); temp->type = params[i].type; which was displaying: current key name in: outbound.peak, out: field size: 0 current key name in: inbound.peak, out: field size: 0 current key name in: inbound.burst, out: field size: 0 current key name in: inbound.average, out: field size: 0 current key name in: outbound.average, out: field size: 0 current key name in: outbound.burst, out: field size: 0 I am not good enough in C to determine what exactly the correct fix is here. It seems like nothing that used this function could have ever worked. My trivial patch was: --- a/python27-libvirt/libvirt-python-1.2.2/libvirt-override.c +++ b/python27-libvirt/libvirt-python-1.2.2/libvirt-override.c @@ -197,8 +197,8 @@ setPyVirTypedParameter(PyObject *info, goto cleanup; } - strncpy(temp->field, keystr, sizeof(*temp->field) - 1); - temp->field[sizeof(*temp->field) - 1] = '\0'; + strncpy(temp->field, keystr, VIR_TYPED_PARAM_FIELD_LENGTH - 1); + temp->field[VIR_TYPED_PARAM_FIELD_LENGTH - 1] = '\0'; temp->type = params[i].type; VIR_FREE(keystr); I am not sure if this is the best approach here. This change definitely fixes the issue, and allows setInterfaceParameters to actually execute successfully. I am not comfortable enough with my understanding of the issue to actually submit this as a patch though. Is this something that someone else can take a look at or fix? My trivial test script that was showing the issue: import libvirt conn = libvirt.open(None) if conn == None: print 'Failed to connect'; sys.exit(1) for id in conn.listDomainsID(): dom = conn.lookupByID(id) if dom == None: continue params = dom.interfaceParameters('vnet0', 0) print params dom.setInterfaceParameters('vnet0', params, 0)

2 1

[libvirt] [PATCH v2 0/3] Add keepalive support in virsh
by Martin Kletzander 18 Mar '14

18 Mar '14

This series adds client keepalive support into virsh. This was previously almost done by Guannan around 2 years ago, but not finished. Martin Kletzander (3): virsh: Add keepalive in new vshConnect function virsh: Prohibit virConnectOpen* functions in virsh [DO_NOT_APPLY_UPSTREAM] virsh: add connection monitoring into vshWatchJob cfg.mk | 8 ++++- tests/virsh-optparse | 4 +-- tools/virsh-domain.c | 21 +++++++++---- tools/virsh.c | 83 +++++++++++++++++++++++++++++++++++++++++++++++----- tools/virsh.h | 7 +++++ tools/virsh.pod | 12 ++++++++ 6 files changed, 118 insertions(+), 17 deletions(-) -- 1.9.0

2 7

[libvirt] [PATCH v2] Fix memory leak in virDomainSnapshotDiskDefClear()
by Nehal J Wani 17 Mar '14

17 Mar '14

While running domainsnapshotxml2xmltest, it was found that valgrind pointed out the following memory leak: ==32176== 42 (32 direct, 10 indirect) bytes in 1 blocks are definitely lost in loss record 42 of 66 ==32176== at 0x4A069EE: malloc (vg_replace_malloc.c:270) ==32176== by 0x4A06B62: realloc (vg_replace_malloc.c:662) ==32176== by 0x4C65A07: virReallocN (viralloc.c:243) ==32176== by 0x4C65B2E: virExpandN (viralloc.c:292) ==32176== by 0x4C65E30: virInsertElementsN (viralloc.c:434) ==32176== by 0x4CD71F3: virDomainDiskSourceDefParse (domain_conf.c:5078) ==32176== by 0x4CF6EF4: virDomainSnapshotDefParseNode (snapshot_conf.c:151) ==32176== by 0x4CF7314: virDomainSnapshotDefParseString (snapshot_conf.c:410) ==32176== by 0x41FB8D: testCompareXMLToXMLHelper (domainsnapshotxml2xmltest.c:100) ==32176== by 0x420FD1: virtTestRun (testutils.c:199) ==32176== by 0x41F859: mymain (domainsnapshotxml2xmltest.c:222) ==32176== by 0x42174D: virtTestMain (testutils.c:782) ==32176== ... and one more. --- v2: Updated based on Ján Tomko's and Peter Krempa's comments. v1: https://www.redhat.com/archives/libvir-list/2014-February/msg01409.html src/conf/snapshot_conf.c | 3 +++ 1 files changed, 3 insertions(+), 0 deletions(-) diff --git a/src/conf/snapshot_conf.c b/src/conf/snapshot_conf.c index d70d176..546e5b3 100644 --- a/src/conf/snapshot_conf.c +++ b/src/conf/snapshot_conf.c @@ -82,6 +82,9 @@ virDomainSnapshotDiskDefClear(virDomainSnapshotDiskDefPtr disk) { VIR_FREE(disk->name); VIR_FREE(disk->file); + while (disk->nhosts) + virDomainDiskHostDefClear(&disk->hosts[--disk->nhosts]); + VIR_FREE(disk->hosts); } void virDomainSnapshotDefFree(virDomainSnapshotDefPtr def) -- 1.7.1

2 1

[libvirt] [PATCH 0/7] POC: Saner firewall integration
by Daniel P. Berrange 17 Mar '14

17 Mar '14

We currently have three areas of code which deal with firewall changes. The bridge driver's iptables usage, the QEMU driver's ebtables usage for mac filters and the nwfilter code. These all directly invoke the iptables/ebtables commands or in the case of nwfilter invoke horrible generated shell scripts. The latter in particular has always been an unpleasant design choice, but it has been made much worse by support for firewalld. We are now invoking firewall-cmd just in order to make a DBus method call to firewalld which then invokes the real *tables commands. This has a notable performance impact. This proof of concept series introduces a new virFirewallPtr object for encapsulating all firewall changes. It provides a transactional API for making firewall changes, so the caller can define a set of rules which must either all succeed or all fail, along with a set of rules to perform rollback upon fail. It will either execute *tables commands directly or will call the DBus method for firewalld directly. The upshot is that it will become possible to unit test all the firewall code much more easily, instead of having to rely on integration testing that we currently have for nwfilter. We will also have much improved performance by avoiding the firewall-cmd tool and easier to understand code too. In this series I've only done the core infrastructure and the conversion of viriptables + virebtables source files. The work on nwfilter is a bigger job that I'm still working on. Daniel P. Berrange (7): Introduce a new set of helper macros for mocking symbols Create a re-usable DBus LD_PRELOAD mock library Switch systemd test to use generic dbus mock Add ability to register callback for virCommand dry run Introduce an object for managing firewall rulesets Convert bridge driver over to use new firewall APIs Convert ebtables code over to use firewall APIs include/libvirt/virterror.h | 1 + po/POTFILES.in | 1 + src/Makefile.am | 2 + src/libvirt_private.syms | 13 + src/network/bridge_driver_linux.c | 669 ++++++++++++++++---------------------- src/util/vircommand.c | 47 ++- src/util/vircommand.h | 12 +- src/util/virebtables.c | 187 +++-------- src/util/virerror.c | 1 + src/util/virfirewall.c | 653 +++++++++++++++++++++++++++++++++++++ src/util/virfirewall.h | 93 ++++++ src/util/virfirewallpriv.h | 45 +++ src/util/viriptables.c | 634 ++++++++++++++++-------------------- src/util/viriptables.h | 114 ++++--- tests/Makefile.am | 30 +- tests/testutils.c | 18 +- tests/virfirewalltest.c | 619 +++++++++++++++++++++++++++++++++++ tests/virkmodtest.c | 8 +- tests/virmock.h | 298 +++++++++++++++++ tests/virmockdbus.c | 64 ++++ tests/virnetdevbandwidthtest.c | 3 +- tests/virsystemdmock.c | 139 -------- tests/virsystemdtest.c | 89 ++++- 23 files changed, 2626 insertions(+), 1114 deletions(-) create mode 100644 src/util/virfirewall.c create mode 100644 src/util/virfirewall.h create mode 100644 src/util/virfirewallpriv.h create mode 100644 tests/virfirewalltest.c create mode 100644 tests/virmock.h create mode 100644 tests/virmockdbus.c delete mode 100644 tests/virsystemdmock.c -- 1.8.5.3

4 12

[libvirt] [PATCH] bhyve: allow to destroy only active domains
by Roman Bogorodskiy 17 Mar '14

17 Mar '14

Add a check that domain is active before attempting to destroy it. --- src/bhyve/bhyve_driver.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/bhyve/bhyve_driver.c b/src/bhyve/bhyve_driver.c index 9dbb299..60d49de 100644 --- a/src/bhyve/bhyve_driver.c +++ b/src/bhyve/bhyve_driver.c @@ -567,6 +567,12 @@ bhyveDomainDestroy(virDomainPtr dom) if (virDomainDestroyEnsureACL(dom->conn, vm->def) < 0) goto cleanup; + if (!virDomainObjIsActive(vm)) { + virReportError(VIR_ERR_OPERATION_INVALID, + "%s", _("Domain is not running")); + goto cleanup; + } + ret = virBhyveProcessStop(privconn, vm, VIR_DOMAIN_SHUTOFF_DESTROYED); cleanup: -- 1.8.4.2

2 2

[libvirt] [PATCH 0/7 v2] Remove overhead of disabled logging calls
by Daniel P. Berrange 17 Mar '14

17 Mar '14

A v2 of https://www.redhat.com/archives/libvir-list/2014-March/msg00124.html Looking at the performance of the libvirtd event loop when used with libvirt sandbox and apps which do lots of I/O I discovered that a huge amount of time was being spent in our logging APIs making the event loop quite poor at dealing with frequently firing poll events for small data sizes. To track this down I created the following short demo program which simulates a libvirtd event loop thread, watching 500 FDs (eg 500 QEMU or LXC console or monitor FDs). In the test no logging outputs are enabled, so ideally the log overhead would be near zero. Running the demo for 51200 iterations initially took 1 minute 40 seconds. oprofile traces show that in this test run the logging APIs were totally dominant in CPU time, over the work done in the event loop. Note the demo program's FD handler doesn't actually do any work, so this is showing the worst case behaviour, but it is none-the-less a good illustration of the loggin overhead. After refactoring the logging code, the overhead of logging calls when no outputs are enabled is reduced to a single function call which does 2 integer comparisons without any mutex locks. The demo program now completes in 3.3 seconds. IOW, 97% of the original CPU time for the demo was consumed by the logging APIs ! The main feature we loose in this series is the global logging buffer. This ring buffer currently collects all log mesages regardless of log filter settings, and dumps them to stderr upon crash of libvirtd. As the amount of logging code in libvirt has grown, the signal/noise ratio of data in the global log buffer has gone down significantly. Whenever libvirtd crashes, my first reaction to ask the reporter to reproduce with a specifically tailored set of log filters set. Thus, IMHO, the global log buffer is no longer a compelling enough feature to justify its significant performance overhead. Rather than remove it entirely though, I've changed it in two parts. The first patch removes the overhead of it, but leaves a more tailored verson of it still present. This removes the contribution it makes to performance overheads. The last patch, which is not performance relevant, simply deletes the rest of the global log buffer entirely. I'm mostly ambivalent on whether to apply patch 7 or not. IMHO the global log buffer is mostly useless once patch 1 is applied, but if people think it is worth keeping it, patch 7 can be dropped and I won't mind. The demo program used is $ cat demo.c #include <libvirt/libvirt.h> #include <unistd.h> #include <stdio.h> static void ignore(int handle, int fd, int events, void *opaque) { // nada } #define STEPS (1024 * 5) #define ITERATIONS (STEPS * 10) #define NFILES 500 int main(int argc, char **argv) { int fds[NFILES][2]; int i; virEventRegisterDefaultImpl(); for (i = 0 ; i < NFILES ; i++) { char c = '0'; if (pipe(fds[i]) < 0) { perror("pipe"); return -1; } if (write(fds[i][1], &c, 1) < 0) return -1; virEventAddHandle(fds[i][0], VIR_EVENT_HANDLE_READABLE, ignore, NULL, NULL); } for (i = 0 ; i < ITERATIONS ; i++) { if (!(i % STEPS)) fprintf(stderr, "."); virEventRunDefaultImpl(); } return 0; } Simply compile & run it with no args $ gcc -Wall `pkg-config --cflags --libs libvirt` -o demo demo.c $ time ./demo .......... real 1m39.981s user 1m34.894s sys 0m4.851s Now make it use the newly compiled libvirt version $ export LD_LIBRARY_PATH=`pwd`/src/.libs $ time ./demo .......... real 0m3.316s user 0m2.896s sys 0m0.412s 97% execution time removed ! Daniel P. Berrange (7): Reduce performance overhead of the global log buffer Refactor code that skips logging of error messages Move dtrace probe macros into separate header file Turn virLogSource into a struct instead of an enum Add virLogSource variables to all source files Switch to filtering based on log source name instead of filename Remove global log buffer feature entirely cfg.mk | 3 + daemon/libvirtd-config.c | 5 +- daemon/libvirtd-config.h | 1 - daemon/libvirtd.c | 5 +- daemon/libvirtd.conf | 10 +- daemon/libvirtd.h | 1 - daemon/remote.c | 3 + daemon/stream.c | 2 + docs/apibuild.py | 30 +++ docs/logging.html.in | 10 - src/access/viraccessdriverpolkit.c | 3 + src/access/viraccessmanager.c | 3 + src/bhyve/bhyve_command.c | 2 + src/bhyve/bhyve_driver.c | 2 + src/bhyve/bhyve_process.c | 2 + src/conf/domain_audit.c | 2 + src/conf/domain_conf.c | 2 + src/conf/domain_event.c | 1 + src/conf/network_event.c | 2 + src/conf/nwfilter_params.c | 2 + src/conf/object_event.c | 2 + src/conf/secret_conf.c | 2 + src/conf/snapshot_conf.c | 2 + src/conf/storage_conf.c | 1 - src/conf/virchrdev.c | 2 + src/cpu/cpu.c | 2 + src/cpu/cpu_powerpc.c | 2 + src/cpu/cpu_x86.c | 2 + src/datatypes.c | 2 + src/driver.c | 2 + src/esx/esx_device_monitor.c | 1 - src/esx/esx_driver.c | 2 + src/esx/esx_interface_driver.c | 2 - src/esx/esx_network_driver.c | 2 - src/esx/esx_nwfilter_driver.c | 2 - src/esx/esx_secret_driver.c | 2 - src/esx/esx_storage_backend_iscsi.c | 2 - src/esx/esx_storage_backend_vmfs.c | 2 + src/esx/esx_util.c | 2 +- src/esx/esx_vi.c | 2 +- src/esx/esx_vi_methods.c | 1 - src/esx/esx_vi_types.c | 2 +- src/fdstream.c | 2 + src/hyperv/hyperv_device_monitor.c | 2 - src/hyperv/hyperv_driver.c | 3 +- src/hyperv/hyperv_interface_driver.c | 2 - src/hyperv/hyperv_network_driver.c | 2 - src/hyperv/hyperv_nwfilter_driver.c | 2 - src/hyperv/hyperv_secret_driver.c | 2 - src/hyperv/hyperv_storage_driver.c | 2 - src/hyperv/hyperv_util.c | 2 +- src/hyperv/hyperv_wmi.c | 2 - src/interface/interface_backend_netcf.c | 2 + src/internal.h | 72 ------ src/libvirt-lxc.c | 2 + src/libvirt-qemu.c | 2 + src/libvirt.c | 2 + src/libvirt_private.syms | 2 - src/libxl/libxl_conf.c | 2 + src/libxl/libxl_domain.c | 1 + src/libxl/libxl_driver.c | 2 + src/locking/domain_lock.c | 2 + src/locking/lock_daemon.c | 4 +- src/locking/lock_daemon_config.c | 4 +- src/locking/lock_daemon_config.h | 1 - src/locking/lock_daemon_dispatch.c | 5 +- src/locking/lock_driver_lockd.c | 2 + src/locking/lock_driver_nop.c | 2 + src/locking/lock_driver_sanlock.c | 2 + src/locking/lock_manager.c | 2 + src/locking/virtlockd.conf | 10 +- src/lxc/lxc_cgroup.c | 2 + src/lxc/lxc_conf.c | 2 + src/lxc/lxc_container.c | 2 + src/lxc/lxc_controller.c | 2 + src/lxc/lxc_domain.c | 2 + src/lxc/lxc_driver.c | 1 + src/lxc/lxc_fuse.c | 1 - src/lxc/lxc_hostdev.c | 2 + src/lxc/lxc_monitor.c | 2 + src/lxc/lxc_native.c | 1 + src/lxc/lxc_process.c | 2 + src/network/bridge_driver.c | 2 + src/network/bridge_driver_linux.c | 3 + src/network/bridge_driver_platform.h | 1 - src/node_device/node_device_driver.c | 2 - src/node_device/node_device_hal.c | 2 + src/node_device/node_device_linux_sysfs.c | 2 + src/node_device/node_device_udev.c | 4 +- src/nodeinfo.c | 1 - src/nwfilter/nwfilter_dhcpsnoop.c | 2 + src/nwfilter/nwfilter_driver.c | 2 + src/nwfilter/nwfilter_ebiptables_driver.c | 2 + src/nwfilter/nwfilter_gentech_driver.c | 1 + src/nwfilter/nwfilter_learnipaddr.c | 2 + src/openvz/openvz_driver.c | 2 + src/parallels/parallels_driver.c | 2 + src/phyp/phyp_driver.c | 3 + src/qemu/qemu_agent.c | 2 + src/qemu/qemu_bridge_filter.c | 1 - src/qemu/qemu_capabilities.c | 4 +- src/qemu/qemu_cgroup.c | 2 + src/qemu/qemu_command.c | 2 + src/qemu/qemu_conf.c | 2 + src/qemu/qemu_domain.c | 2 + src/qemu/qemu_driver.c | 2 + src/qemu/qemu_hostdev.c | 2 + src/qemu/qemu_hotplug.c | 3 + src/qemu/qemu_migration.c | 2 + src/qemu/qemu_monitor.c | 3 + src/qemu/qemu_monitor_json.c | 3 + src/qemu/qemu_monitor_text.c | 3 + src/qemu/qemu_process.c | 2 + src/remote/remote_driver.c | 2 + src/rpc/virkeepalive.c | 3 + src/rpc/virnetclient.c | 3 + src/rpc/virnetclientprogram.c | 2 + src/rpc/virnetclientstream.c | 2 + src/rpc/virnetmessage.c | 2 + src/rpc/virnetsaslcontext.c | 2 + src/rpc/virnetserver.c | 47 +--- src/rpc/virnetserverclient.c | 3 + src/rpc/virnetservermdns.c | 2 + src/rpc/virnetserverprogram.c | 2 + src/rpc/virnetsocket.c | 2 + src/rpc/virnetsshsession.c | 2 + src/rpc/virnettlscontext.c | 3 + src/secret/secret_driver.c | 2 + src/security/security_apparmor.c | 3 + src/security/security_dac.c | 3 + src/security/security_driver.c | 2 + src/security/security_manager.c | 1 + src/security/security_selinux.c | 2 + src/storage/storage_backend.c | 2 + src/storage/storage_backend_disk.c | 2 + src/storage/storage_backend_fs.c | 2 + src/storage/storage_backend_gluster.c | 2 + src/storage/storage_backend_iscsi.c | 2 + src/storage/storage_backend_logical.c | 2 + src/storage/storage_backend_mpath.c | 2 + src/storage/storage_backend_rbd.c | 2 + src/storage/storage_backend_scsi.c | 2 + src/storage/storage_backend_sheepdog.c | 1 - src/storage/storage_driver.c | 2 + src/test/test_driver.c | 2 + src/uml/uml_conf.c | 1 + src/uml/uml_driver.c | 2 + src/util/viralloc.c | 2 + src/util/virarch.c | 2 + src/util/viraudit.c | 9 +- src/util/viraudit.h | 10 +- src/util/virauth.c | 2 + src/util/virauthconfig.c | 1 + src/util/vircgroup.c | 2 + src/util/virclosecallbacks.c | 2 + src/util/vircommand.c | 2 + src/util/virconf.c | 2 + src/util/virdbus.c | 2 + src/util/virdnsmasq.c | 3 + src/util/virebtables.c | 2 + src/util/virerror.c | 19 +- src/util/virevent.c | 2 + src/util/vireventpoll.c | 3 + src/util/virfile.c | 2 + src/util/virhash.c | 2 + src/util/virhook.c | 2 + src/util/viridentity.c | 1 + src/util/viriptables.c | 2 + src/util/virjson.c | 1 + src/util/virkeyfile.c | 2 + src/util/virlockspace.c | 2 + src/util/virlog.c | 381 +++++------------------------- src/util/virlog.h | 49 ++-- src/util/virnetdev.c | 2 + src/util/virnetdevmacvlan.c | 1 + src/util/virnetdevtap.c | 2 + src/util/virnetdevveth.c | 2 + src/util/virnetdevvportprofile.c | 2 + src/util/virnetlink.c | 2 + src/util/virnodesuspend.c | 2 + src/util/virnuma.c | 2 + src/util/virobject.c | 3 + src/util/virpci.c | 2 + src/util/virpidfile.c | 2 + src/util/virprobe.h | 100 ++++++++ src/util/virprocess.c | 2 + src/util/virrandom.c | 2 + src/util/virscsi.c | 1 - src/util/virstoragefile.c | 2 + src/util/virstring.c | 2 + src/util/virsysinfo.c | 1 - src/util/virsystemd.c | 1 + src/util/virusb.c | 2 + src/util/virutil.c | 2 + src/util/viruuid.c | 2 + src/vbox/vbox_MSCOMGlue.c | 2 + src/vbox/vbox_XPCOMCGlue.c | 1 + src/vbox/vbox_driver.c | 1 + src/vbox/vbox_tmpl.c | 3 + src/vmx/vmx.c | 2 + src/xen/xen_driver.c | 3 + src/xen/xen_hypervisor.c | 2 + src/xen/xen_inotify.c | 2 + src/xen/xend_internal.c | 2 + src/xen/xm_internal.c | 2 + src/xen/xs_internal.c | 2 + src/xenapi/xenapi_utils.c | 2 + src/xenxs/xen_sxpr.c | 2 + tests/domainconftest.c | 2 + tests/eventtest.c | 2 + tests/fdstreamtest.c | 2 + tests/libvirtdconftest.c | 2 + tests/qemumonitortestutils.c | 2 + tests/securityselinuxlabeltest.c | 2 + tests/securityselinuxtest.c | 2 + tests/sockettest.c | 2 + tests/testutils.c | 4 +- tests/virauthconfigtest.c | 2 + tests/vircgrouptest.c | 2 + tests/virdbustest.c | 2 + tests/virdrivermoduletest.c | 2 + tests/virhashtest.c | 2 + tests/viridentitytest.c | 1 + tests/virkeycodetest.c | 1 + tests/virkeyfiletest.c | 1 + tests/virlockspacetest.c | 2 + tests/virnetmessagetest.c | 2 + tests/virnetsockettest.c | 2 + tests/virnettlscontexttest.c | 2 + tests/virnettlshelpers.c | 2 + tests/virnettlssessiontest.c | 2 + tests/virportallocatortest.c | 1 + tests/virstoragetest.c | 2 + tests/virstringtest.c | 2 + tests/virsystemdtest.c | 2 + tests/virtimetest.c | 2 + tests/viruritest.c | 2 + tools/virsh-console.c | 2 + 238 files changed, 655 insertions(+), 554 deletions(-) create mode 100644 src/util/virprobe.h -- 1.8.5.3

2 14

[libvirt] [PATCH] bhyve: add console support through nmdm device
by Roman Bogorodskiy 17 Mar '14

17 Mar '14

Some additional info on nmdm devices that probably doesn't fit commit message. Man page: http://www.freebsd.org/cgi/man.cgi?query=nmdm&sektion=4 To use this device, we pass device name like '/dev/nmdmNA' to bhyve. Device is created automatically when bhyve accesses it. The pair device name is always '/dev/nmdmNB' which is used to provide console support. Even though this patch doesn't directly depend on virFDStreamOpenPTY() addition patch: https://www.redhat.com/archives/libvir-list/2014-March/msg00582.html it's not much useful without it because of mangled console output. Roman Bogorodskiy (1): bhyve: add console support through nmdm device src/bhyve/bhyve_command.c | 34 ++++++++++++++++++++++ src/bhyve/bhyve_driver.c | 69 ++++++++++++++++++++++++++++++++++++++++++++ src/conf/domain_audit.c | 1 + src/conf/domain_conf.c | 12 +++++++- src/conf/domain_conf.h | 1 + src/qemu/qemu_monitor_json.c | 1 + 6 files changed, 117 insertions(+), 1 deletion(-) -- 1.8.4.2

2 3

[libvirt] [PATCH 1/2] bhyve: add support for virtio block devices
by Roman Bogorodskiy 17 Mar '14

17 Mar '14

--- src/bhyve/bhyve_command.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/src/bhyve/bhyve_command.c b/src/bhyve/bhyve_command.c index b9df6d0..2ec97fc 100644 --- a/src/bhyve/bhyve_command.c +++ b/src/bhyve/bhyve_command.c @@ -216,6 +216,7 @@ static int bhyveBuildDiskArgStr(const virDomainDef *def, virCommandPtr cmd) { virDomainDiskDefPtr disk; + const char *bus_type; if (def->ndisks != 1) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", @@ -225,7 +226,14 @@ bhyveBuildDiskArgStr(const virDomainDef *def, virCommandPtr cmd) disk = def->disks[0]; - if (disk->bus != VIR_DOMAIN_DISK_BUS_SATA) { + switch (disk->bus) { + case VIR_DOMAIN_DISK_BUS_SATA: + bus_type = "ahci-hd"; + break; + case VIR_DOMAIN_DISK_BUS_VIRTIO: + bus_type = "virtio-blk"; + break; + default: virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("unsupported disk bus type")); return -1; @@ -244,7 +252,7 @@ bhyveBuildDiskArgStr(const virDomainDef *def, virCommandPtr cmd) } virCommandAddArg(cmd, "-s"); - virCommandAddArgFormat(cmd, "2:0,ahci-hd,%s", disk->src); + virCommandAddArgFormat(cmd, "2:0,%s,%s", bus_type, disk->src); return 0; } -- 1.8.4.2

2 5

[libvirt] [PATCH] Enforce attribute check of the virRegister functions
by Pavel Hrdina 17 Mar '14

17 Mar '14

Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com> --- src/driver.h | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/driver.h b/src/driver.h index fbfaac4..d106d59 100644 --- a/src/driver.h +++ b/src/driver.h @@ -2132,15 +2132,15 @@ struct _virNWFilterDriver { * TODO: also need ways to (des)activate a given driver * lookup based on the URI given in a virConnectOpen(ReadOnly) */ -int virRegisterDriver(virDriverPtr); -int virRegisterNetworkDriver(virNetworkDriverPtr); -int virRegisterInterfaceDriver(virInterfaceDriverPtr); -int virRegisterStorageDriver(virStorageDriverPtr); -int virRegisterNodeDeviceDriver(virNodeDeviceDriverPtr); -int virRegisterSecretDriver(virSecretDriverPtr); -int virRegisterNWFilterDriver(virNWFilterDriverPtr); +int virRegisterDriver(virDriverPtr) ATTRIBUTE_RETURN_CHECK; +int virRegisterNetworkDriver(virNetworkDriverPtr) ATTRIBUTE_RETURN_CHECK; +int virRegisterInterfaceDriver(virInterfaceDriverPtr) ATTRIBUTE_RETURN_CHECK; +int virRegisterStorageDriver(virStorageDriverPtr) ATTRIBUTE_RETURN_CHECK; +int virRegisterNodeDeviceDriver(virNodeDeviceDriverPtr) ATTRIBUTE_RETURN_CHECK; +int virRegisterSecretDriver(virSecretDriverPtr) ATTRIBUTE_RETURN_CHECK; +int virRegisterNWFilterDriver(virNWFilterDriverPtr) ATTRIBUTE_RETURN_CHECK; # ifdef WITH_LIBVIRTD -int virRegisterStateDriver(virStateDriverPtr); +int virRegisterStateDriver(virStateDriverPtr) ATTRIBUTE_RETURN_CHECK; # endif void virDriverModuleInitialize(const char *defmoddir); void *virDriverLoadModule(const char *name); -- 1.8.3.1

2 2

[libvirt] [PATCH v3 0/2] Introduce max_anonymous_clients
by Michal Privoznik 17 Mar '14

17 Mar '14

https://bugzilla.redhat.com/show_bug.cgi?id=981729 So far we can limit how many clients are connected, how many are waiting in accept() line but we could not control the count of accepted but not authenticated yet. diff to v2: -Dan's suggestions worked in Michal Privoznik (2): virNetServer: Introduce unauth clients counter daemon: Introduce max_anonymous_clients daemon/libvirtd-config.c | 4 +- daemon/libvirtd-config.h | 1 + daemon/libvirtd.aug | 1 + daemon/libvirtd.c | 1 + daemon/libvirtd.conf | 6 ++- daemon/remote.c | 21 +++++---- daemon/test_libvirtd.aug.in | 3 +- src/locking/lock_daemon.c | 2 +- src/lxc/lxc_controller.c | 2 +- src/rpc/virnetserver.c | 108 ++++++++++++++++++++++++++++++++++++++++---- src/rpc/virnetserver.h | 4 ++ 11 files changed, 130 insertions(+), 23 deletions(-) -- 1.9.0

2 7

[libvirt] [PATCH] bhyve: implement nodeGetInfo()
by Roman Bogorodskiy 17 Mar '14

17 Mar '14

Add bhyveNodeGetInfo() which is a simple wrapper around nodeGetInfo() from src/nodeinfo.c. --- src/bhyve/bhyve_driver.c | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/src/bhyve/bhyve_driver.c b/src/bhyve/bhyve_driver.c index 9dbb299..cb790de 100644 --- a/src/bhyve/bhyve_driver.c +++ b/src/bhyve/bhyve_driver.c @@ -601,6 +601,16 @@ bhyveNodeGetMemoryStats(virConnectPtr conn, } static int +bhyveNodeGetInfo(virConnectPtr conn, + virNodeInfoPtr nodeinfo) +{ + if (virNodeGetInfoEnsureACL(conn) < 0) + return -1; + + return nodeGetInfo(nodeinfo); +} + +static int bhyveStateCleanup(void) { VIR_DEBUG("bhyve state cleanup"); @@ -703,6 +713,7 @@ static virDriver bhyveDriver = { .domainIsPersistent = bhyveDomainIsPersistent, /* 1.2.2 */ .nodeGetCPUStats = bhyveNodeGetCPUStats, /* 1.2.2 */ .nodeGetMemoryStats = bhyveNodeGetMemoryStats, /* 1.2.2 */ + .nodeGetInfo = bhyveNodeGetInfo, /* 1.2.3 */ }; -- 1.8.4.2

2 2

[libvirt] [PATCH] Add parameter to wait for lock in file locking APIs
by Nehal J Wani 17 Mar '14

17 Mar '14

Our current pidfile acquire APis (virPidFileAcquire) simply return -1 upon failure to acquire a lock. This patch adds a parameter 'bool waitForLock' which instructs the APIs if we want to make it block and wait for the lock or not. --- Based on Daniel's comments on: https://www.redhat.com/archives/libvir-list/2014-March/msg00841.html daemon/libvirtd.c | 2 +- src/locking/lock_daemon.c | 4 ++-- src/util/virfile.c | 14 +++++++++----- src/util/virfile.h | 2 +- src/util/virlockspace.c | 6 +++--- src/util/virpidfile.c | 6 ++++-- src/util/virpidfile.h | 2 ++ 7 files changed, 22 insertions(+), 14 deletions(-) diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c index a471b3b..27c4fff 100644 --- a/daemon/libvirtd.c +++ b/daemon/libvirtd.c @@ -1369,7 +1369,7 @@ int main(int argc, char **argv) { umask(old_umask); /* Try to claim the pidfile, exiting if we can't */ - if ((pid_file_fd = virPidFileAcquirePath(pid_file, getpid())) < 0) { + if ((pid_file_fd = virPidFileAcquirePath(pid_file, false, getpid())) < 0) { ret = VIR_DAEMON_ERR_PIDFILE; goto cleanup; } diff --git a/src/locking/lock_daemon.c b/src/locking/lock_daemon.c index e047751..7dc4292 100644 --- a/src/locking/lock_daemon.c +++ b/src/locking/lock_daemon.c @@ -1020,7 +1020,7 @@ virLockDaemonPostExecRestart(const char *state_file, /* Re-claim PID file now as we will not be daemonizing */ if (pid_file && - (*pid_file_fd = virPidFileAcquirePath(pid_file, getpid())) < 0) + (*pid_file_fd = virPidFileAcquirePath(pid_file, false, getpid())) < 0) goto cleanup; if (!(lockDaemon = virLockDaemonNewPostExecRestart(object, privileged))) @@ -1382,7 +1382,7 @@ int main(int argc, char **argv) { } /* If we have a pidfile set, claim it now, exiting if already taken */ - if ((pid_file_fd = virPidFileAcquirePath(pid_file, getpid())) < 0) { + if ((pid_file_fd = virPidFileAcquirePath(pid_file, false, getpid())) < 0) { ret = VIR_LOCK_DAEMON_ERR_PIDFILE; goto cleanup; } diff --git a/src/util/virfile.c b/src/util/virfile.c index 6da564b..a4a4827 100644 --- a/src/util/virfile.c +++ b/src/util/virfile.c @@ -339,13 +339,14 @@ virFileWrapperFdFree(virFileWrapperFdPtr wfd) * @shared: type of lock to acquire * @start: byte offset to start lock * @len: length of lock (0 to acquire entire remaining file from @start) + * @waitForLock: wait for previously held lock or not * * Attempt to acquire a lock on the file @fd. If @shared * is true, then a shared lock will be acquired, * otherwise an exclusive lock will be acquired. If * the lock cannot be acquired, an error will be - * returned. This will not wait to acquire the lock if - * another process already holds it. + * returned. If @waitForLock is true, this will wait + * for the lock if another process has already acquired it. * * The lock will be released when @fd is closed. The lock * will also be released if *any* other open file descriptor @@ -356,7 +357,7 @@ virFileWrapperFdFree(virFileWrapperFdPtr wfd) * * Returns 0 on success, or -errno otherwise */ -int virFileLock(int fd, bool shared, off_t start, off_t len) +int virFileLock(int fd, bool shared, off_t start, off_t len, bool waitForLock) { struct flock fl = { .l_type = shared ? F_RDLCK : F_WRLCK, @@ -365,7 +366,9 @@ int virFileLock(int fd, bool shared, off_t start, off_t len) .l_len = len, }; - if (fcntl(fd, F_SETLK, &fl) < 0) + int cmd = waitForLock ? F_SETLKW : F_SETLK; + + if (fcntl(fd, cmd, &fl) < 0) return -errno; return 0; @@ -402,7 +405,8 @@ int virFileUnlock(int fd, off_t start, off_t len) int virFileLock(int fd ATTRIBUTE_UNUSED, bool shared ATTRIBUTE_UNUSED, off_t start ATTRIBUTE_UNUSED, - off_t len ATTRIBUTE_UNUSED) + off_t len ATTRIBUTE_UNUSED, + bool waitForLock ATTRIBUTE_UNUSED) { return -ENOSYS; } diff --git a/src/util/virfile.h b/src/util/virfile.h index 20baf6f..302b74f 100644 --- a/src/util/virfile.h +++ b/src/util/virfile.h @@ -97,7 +97,7 @@ int virFileWrapperFdClose(virFileWrapperFdPtr dfd); void virFileWrapperFdFree(virFileWrapperFdPtr dfd); -int virFileLock(int fd, bool shared, off_t start, off_t len); +int virFileLock(int fd, bool shared, off_t start, off_t len, bool waitForLock); int virFileUnlock(int fd, off_t start, off_t len); typedef int (*virFileRewriteFunc)(int fd, void *opaque); diff --git a/src/util/virlockspace.c b/src/util/virlockspace.c index 90a39bb..a187e1e 100644 --- a/src/util/virlockspace.c +++ b/src/util/virlockspace.c @@ -82,7 +82,7 @@ static void virLockSpaceResourceFree(virLockSpaceResourcePtr res) if (res->flags & VIR_LOCK_SPACE_ACQUIRE_SHARED) { /* We must upgrade to an exclusive lock to ensure * no one else still has it before trying to delete */ - if (virFileLock(res->fd, false, 0, 1) < 0) { + if (virFileLock(res->fd, false, 0, 1, false) < 0) { VIR_DEBUG("Could not upgrade shared lease to exclusive, not deleting"); } else { if (unlink(res->path) < 0 && @@ -155,7 +155,7 @@ virLockSpaceResourceNew(virLockSpacePtr lockspace, goto error; } - if (virFileLock(res->fd, shared, 0, 1) < 0) { + if (virFileLock(res->fd, shared, 0, 1, false) < 0) { if (errno == EACCES || errno == EAGAIN) { virReportError(VIR_ERR_RESOURCE_BUSY, _("Lockspace resource '%s' is locked"), @@ -202,7 +202,7 @@ virLockSpaceResourceNew(virLockSpacePtr lockspace, goto error; } - if (virFileLock(res->fd, shared, 0, 1) < 0) { + if (virFileLock(res->fd, shared, 0, 1, false) < 0) { if (errno == EACCES || errno == EAGAIN) { virReportError(VIR_ERR_RESOURCE_BUSY, _("Lockspace resource '%s' is locked"), diff --git a/src/util/virpidfile.c b/src/util/virpidfile.c index 298d57c..28db24c 100644 --- a/src/util/virpidfile.c +++ b/src/util/virpidfile.c @@ -372,6 +372,7 @@ cleanup: } int virPidFileAcquirePath(const char *path, + bool waitForLock, pid_t pid) { int fd = -1; @@ -405,7 +406,7 @@ int virPidFileAcquirePath(const char *path, return -1; } - if (virFileLock(fd, false, 0, 1) < 0) { + if (virFileLock(fd, false, 0, 1, waitForLock) < 0) { virReportSystemError(errno, _("Failed to acquire pid file '%s'"), path); @@ -448,6 +449,7 @@ int virPidFileAcquirePath(const char *path, int virPidFileAcquire(const char *dir, const char *name, + bool waitForLock, pid_t pid) { int rc = 0; @@ -463,7 +465,7 @@ int virPidFileAcquire(const char *dir, goto cleanup; } - rc = virPidFileAcquirePath(pidfile, pid); + rc = virPidFileAcquirePath(pidfile, waitForLock, pid); cleanup: VIR_FREE(pidfile); diff --git a/src/util/virpidfile.h b/src/util/virpidfile.h index 3194a89..2720206 100644 --- a/src/util/virpidfile.h +++ b/src/util/virpidfile.h @@ -56,9 +56,11 @@ int virPidFileDelete(const char *dir, int virPidFileAcquirePath(const char *path, + bool waitForLock, pid_t pid) ATTRIBUTE_RETURN_CHECK; int virPidFileAcquire(const char *dir, const char *name, + bool waitForLock, pid_t pid) ATTRIBUTE_RETURN_CHECK; int virPidFileReleasePath(const char *path, -- 1.7.1

3 2

[libvirt] get information about weighted instances
by sahid 17 Mar '14

17 Mar '14

Greetings, There is a new field I would like to add in the return of the command "vcpuinfo" to get the current "cpu shares" of a domain. The aim is to help managing weighted instances. Avoiding to look at the domain xml to compare instances. VCPU: 0 CPU: 3 State: running CPU time: 4811.3s CPU Affinity: yyyy CPU Shares: 2048 I would like to know if this solution make sens for the community? and if yes I could be work on it. The real aim at the end of this is to add the ability to weight instances in real time, like we can do with the command "vcpupin". Does it makes sense? Best, Sahid.

2 1

[libvirt] [PATCH] Fix mistakes in checking return values
by Daniel P. Berrange 17 Mar '14

17 Mar '14

Thre was a syntax error in checking virRegisterStateDriver in the remote driver, and bogus checking of a void return type of virDomainConfNWFilterRegister in nwfilter. Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com> --- src/nwfilter/nwfilter_driver.c | 3 +-- src/remote/remote_driver.c | 2 +- 2 files changed, 2 insertions(+), 3 deletions(-) Pushed as a build break fix. diff --git a/src/nwfilter/nwfilter_driver.c b/src/nwfilter/nwfilter_driver.c index 228794d..8ce2b7d 100644 --- a/src/nwfilter/nwfilter_driver.c +++ b/src/nwfilter/nwfilter_driver.c @@ -720,7 +720,6 @@ int nwfilterRegister(void) { return -1; if (virRegisterStateDriver(&stateDriver) < 0) return -1; - if (virDomainConfNWFilterRegister(&domainNWFilterDriver) < 0) - return -1; + virDomainConfNWFilterRegister(&domainNWFilterDriver); return 0; } diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index c9711bb..e85f34d 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -7840,7 +7840,7 @@ remoteRegister(void) if (virRegisterNWFilterDriver(&nwfilter_driver) < 0) return -1; #ifdef WITH_LIBVIRTD - if (virRegisterStateDriver(&state_driver) == -1) < 0) + if (virRegisterStateDriver(&state_driver) < 0) return -1; #endif -- 1.8.5.3

1 0

[libvirt] [PATCH] spec: move some dirs into appropriate subpackages
by Michael Chapman 17 Mar '14

17 Mar '14

This commit moves a few directories into more appropriate subpackages. In a few cases a directory is owned by two subpackages, however this is OK as long as the permissions and ownership for the directory are consistent between them. - %{_sysconfdir}/libvirt/qemu/ Used by the qemu and network drivers. When building with separate driver modules, this directory is only owned by l-d-d-network. l-d-d-qemu has a hard dependency on l-d-d-network, which means this directory is created with the correct permissions and ownership, however it's clearer if both subpackages own the directory independently. - %{_sysconfdir}/libvirt/nwfilter/ Used by the nwfilter driver only. This directory is currently always owned by libvirt-daemon. This commit moves it into l-d-d-nwfilter when building with separate driver modules. - %{_localstatedir}/run/libvirt/network/ Used by the network and nwfilter drivers. When building without separate driver modules, this directory is should be owned by libvirt-daemon only if either of these drivers are enabled. When building with separate driver modules, this directory should be owned by l-d-d-nwfilter in addition to l-d-d-network. - %{_datadir}/libvirt/networks/ and %{_datadir}/libvirt/networks/default.xml Used only by the %post scriptlet in libvirt-daemon-config-network. Signed-off-by: Michael Chapman <mike(a)very.puzzling.org> --- libvirt.spec.in | 24 ++++++++++++++---------- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/libvirt.spec.in b/libvirt.spec.in index 014fe5d..ac71db0 100644 --- a/libvirt.spec.in +++ b/libvirt.spec.in @@ -1837,10 +1837,6 @@ exit 0 %dir %attr(0700, root, root) %{_sysconfdir}/libvirt/ - %if %{with_nwfilter} -%dir %attr(0700, root, root) %{_sysconfdir}/libvirt/nwfilter/ - %endif - %if %{with_systemd} %{_unitdir}/libvirtd.service %{_unitdir}/virtlockd.service @@ -1903,16 +1899,21 @@ exit 0 %{_mandir}/man8/virtlockd.8* %if ! %{with_driver_modules} - %if %{with_network} + %if %{with_network} || %{with_qemu} %dir %attr(0700, root, root) %{_sysconfdir}/libvirt/qemu/ + %endif + %if %{with_network} || %{with_nwfilter} +%ghost %dir %{_localstatedir}/run/libvirt/network/ + %endif + %if %{with_network} %dir %attr(0700, root, root) %{_sysconfdir}/libvirt/qemu/networks/ %dir %attr(0700, root, root) %{_sysconfdir}/libvirt/qemu/networks/autostart -%dir %{_datadir}/libvirt/networks/ -%{_datadir}/libvirt/networks/default.xml -%ghost %dir %{_localstatedir}/run/libvirt/network/ %dir %attr(0700, root, root) %{_localstatedir}/lib/libvirt/network/ %dir %attr(0755, root, root) %{_localstatedir}/lib/libvirt/dnsmasq/ %endif + %if %{with_nwfilter} +%dir %attr(0700, root, root) %{_sysconfdir}/libvirt/nwfilter/ + %endif %if %{with_storage_disk} %attr(0755, root, root) %{_libexecdir}/libvirt_parthelper %endif @@ -1958,6 +1959,8 @@ exit 0 %if %{with_network} %files daemon-config-network %defattr(-, root, root) +%dir %{_datadir}/libvirt/networks/ +%{_datadir}/libvirt/networks/default.xml %endif %if %{with_nwfilter} @@ -1979,8 +1982,6 @@ exit 0 %dir %attr(0700, root, root) %{_sysconfdir}/libvirt/qemu/ %dir %attr(0700, root, root) %{_sysconfdir}/libvirt/qemu/networks/ %dir %attr(0700, root, root) %{_sysconfdir}/libvirt/qemu/networks/autostart -%dir %{_datadir}/libvirt/networks/ -%{_datadir}/libvirt/networks/default.xml %ghost %dir %{_localstatedir}/run/libvirt/network/ %dir %attr(0700, root, root) %{_localstatedir}/lib/libvirt/network/ %dir %attr(0755, root, root) %{_localstatedir}/lib/libvirt/dnsmasq/ @@ -1996,6 +1997,8 @@ exit 0 %if %{with_nwfilter} %files daemon-driver-nwfilter %defattr(-, root, root) +%dir %attr(0700, root, root) %{_sysconfdir}/libvirt/nwfilter/ +%ghost %dir %{_localstatedir}/run/libvirt/network/ %{_libdir}/%{name}/connection-driver/libvirt_driver_nwfilter.so %endif @@ -2015,6 +2018,7 @@ exit 0 %if %{with_qemu} %files daemon-driver-qemu %defattr(-, root, root) +%dir %attr(0700, root, root) %{_sysconfdir}/libvirt/qemu/ %dir %attr(0700, root, root) %{_localstatedir}/log/libvirt/qemu/ %config(noreplace) %{_sysconfdir}/libvirt/qemu.conf %config(noreplace) %{_sysconfdir}/libvirt/qemu-lockd.conf -- 1.8.5.3

2 1

[libvirt] [PATCH 1/1] Fix issue found by coverity and cleanup
by Pavel Hrdina 17 Mar '14

17 Mar '14

Coverity found an issue in lxc_driver and uml_driver that we don't check the return value of register functions. I've also updated all other places and unify the way we check the return value. Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com> --- src/bhyve/bhyve_driver.c | 6 ++++-- src/interface/interface_backend_netcf.c | 3 ++- src/lxc/lxc_driver.c | 6 ++++-- src/network/bridge_driver.c | 3 ++- src/nwfilter/nwfilter_driver.c | 9 ++++++--- src/qemu/qemu_driver.c | 6 ++++-- src/remote/remote_driver.c | 24 ++++++++++++++++-------- src/secret/secret_driver.c | 6 ++++-- src/storage/storage_driver.c | 3 ++- src/uml/uml_driver.c | 6 ++++-- 10 files changed, 48 insertions(+), 24 deletions(-) diff --git a/src/bhyve/bhyve_driver.c b/src/bhyve/bhyve_driver.c index 9dbb299..f1ed510 100644 --- a/src/bhyve/bhyve_driver.c +++ b/src/bhyve/bhyve_driver.c @@ -715,7 +715,9 @@ static virStateDriver bhyveStateDriver = { int bhyveRegister(void) { - virRegisterDriver(&bhyveDriver); - virRegisterStateDriver(&bhyveStateDriver); + if (virRegisterDriver(&bhyveDriver) < 0) + return -1; + if (virRegisterStateDriver(&bhyveStateDriver) < 0) + return -1; return 0; } diff --git a/src/interface/interface_backend_netcf.c b/src/interface/interface_backend_netcf.c index b4c1fe9..98ce83b 100644 --- a/src/interface/interface_backend_netcf.c +++ b/src/interface/interface_backend_netcf.c @@ -1194,6 +1194,7 @@ int netcfIfaceRegister(void) { _("failed to register netcf interface driver")); return -1; } - virRegisterStateDriver(&interfaceStateDriver); + if (virRegisterStateDriver(&interfaceStateDriver) < 0) + return -1; return 0; } diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index 0ab1ba2..27c27d8 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -5767,7 +5767,9 @@ static virStateDriver lxcStateDriver = { int lxcRegister(void) { - virRegisterDriver(&lxcDriver); - virRegisterStateDriver(&lxcStateDriver); + if (virRegisterDriver(&lxcDriver) < 0) + return -1; + if (virRegisterStateDriver(&lxcStateDriver) < 0) + return -1; return 0; } diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index c797f8f..181541e 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -3232,7 +3232,8 @@ static virStateDriver networkStateDriver = { int networkRegister(void) { if (virRegisterNetworkDriver(&networkDriver) < 0) return -1; - virRegisterStateDriver(&networkStateDriver); + if (virRegisterStateDriver(&networkStateDriver) < 0) + return -1; return 0; } diff --git a/src/nwfilter/nwfilter_driver.c b/src/nwfilter/nwfilter_driver.c index 2e89d07..228794d 100644 --- a/src/nwfilter/nwfilter_driver.c +++ b/src/nwfilter/nwfilter_driver.c @@ -716,8 +716,11 @@ static virDomainConfNWFilterDriver domainNWFilterDriver = { int nwfilterRegister(void) { - virRegisterNWFilterDriver(&nwfilterDriver); - virRegisterStateDriver(&stateDriver); - virDomainConfNWFilterRegister(&domainNWFilterDriver); + if (virRegisterNWFilterDriver(&nwfilterDriver) < 0) + return -1; + if (virRegisterStateDriver(&stateDriver) < 0) + return -1; + if (virDomainConfNWFilterRegister(&domainNWFilterDriver) < 0) + return -1; return 0; } diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 89f443f..fc382a5 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -16796,7 +16796,9 @@ static virStateDriver qemuStateDriver = { }; int qemuRegister(void) { - virRegisterDriver(&qemuDriver); - virRegisterStateDriver(&qemuStateDriver); + if (virRegisterDriver(&qemuDriver) < 0) + return -1; + if (virRegisterStateDriver(&qemuStateDriver) < 0) + return -1; return 0; } diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index 955465a..c9711bb 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -7825,15 +7825,23 @@ remoteRegister(void) { remoteDriver = &remote_driver; - if (virRegisterDriver(&remote_driver) == -1) return -1; - if (virRegisterNetworkDriver(&network_driver) == -1) return -1; - if (virRegisterInterfaceDriver(&interface_driver) == -1) return -1; - if (virRegisterStorageDriver(&storage_driver) == -1) return -1; - if (virRegisterNodeDeviceDriver(&node_device_driver) == -1) return -1; - if (virRegisterSecretDriver(&secret_driver) == -1) return -1; - if (virRegisterNWFilterDriver(&nwfilter_driver) == -1) return -1; + if (virRegisterDriver(&remote_driver) < 0) + return -1; + if (virRegisterNetworkDriver(&network_driver) < 0) + return -1; + if (virRegisterInterfaceDriver(&interface_driver) < 0) + return -1; + if (virRegisterStorageDriver(&storage_driver) < 0) + return -1; + if (virRegisterNodeDeviceDriver(&node_device_driver) < 0) + return -1; + if (virRegisterSecretDriver(&secret_driver) < 0) + return -1; + if (virRegisterNWFilterDriver(&nwfilter_driver) < 0) + return -1; #ifdef WITH_LIBVIRTD - if (virRegisterStateDriver(&state_driver) == -1) return -1; + if (virRegisterStateDriver(&state_driver) == -1) < 0) + return -1; #endif return 0; diff --git a/src/secret/secret_driver.c b/src/secret/secret_driver.c index 9f7f946..5cb6391 100644 --- a/src/secret/secret_driver.c +++ b/src/secret/secret_driver.c @@ -1194,7 +1194,9 @@ static virStateDriver stateDriver = { int secretRegister(void) { - virRegisterSecretDriver(&secretDriver); - virRegisterStateDriver(&stateDriver); + if (virRegisterSecretDriver(&secretDriver) < 0) + return -1; + if (virRegisterStateDriver(&stateDriver) < 0) + return -1; return 0; } diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c index 7dbff6c..466ceba 100644 --- a/src/storage/storage_driver.c +++ b/src/storage/storage_driver.c @@ -2696,7 +2696,8 @@ int storageRegister(void) { if (virRegisterStorageDriver(&storageDriver) < 0) return -1; - virRegisterStateDriver(&stateDriver); + if (virRegisterStateDriver(&stateDriver) < 0) + return -1; return 0; } diff --git a/src/uml/uml_driver.c b/src/uml/uml_driver.c index 3496e52..8ddf181 100644 --- a/src/uml/uml_driver.c +++ b/src/uml/uml_driver.c @@ -2909,7 +2909,9 @@ static virStateDriver umlStateDriver = { }; int umlRegister(void) { - virRegisterDriver(&umlDriver); - virRegisterStateDriver(&umlStateDriver); + if (virRegisterDriver(&umlDriver) < 0) + return -1; + if (virRegisterStateDriver(&umlStateDriver) < 0) + return -1; return 0; } -- 1.8.3.1

2 3

Re: [libvirt] Zombie process after open libvirt connection
by Carlos Rodrigues 17 Mar '14

17 Mar '14

The zimbie processes is from perl. What is your perl and libvirt version? Em 17/03/2014 12:37, Michal Privoznik <mprivozn(a)redhat.com> escreveu: > > On 17.03.2014 12:13, Carlos Rodrigues wrote: > > Hell Michal, > > > > Thank you for your answer, but this doesn't fix my problem. > > > > Run your fixed script and we get the same behavior: > > > > $ perl test-chldhandle-bug-fixed.pl > > init... pid=29713 > > while... > > fork 1 > > end... pid=29716 > > receive chld > > fork 2 > > end... pid=29717 > > receive chld > > 2014-03-17 11:10:37.234+0000: 29713: info : libvirt version: 1.0.5.7, package: 2.fc19 (Fedora Project, 2013-11-17-23:21:57, buildvm-18.phx2.fedoraproject.org) > > 2014-03-17 11:10:37.234+0000: 29713: warning : virNetTLSContextCheckCertificate:1099 : Certificate check failed Certificate [session] owner does not match the hostname 10.10.4.249 > > connection open > > fork 3 > > end... pid=29827 > > fork 4 > > end... pid=29930 > > go next... > > > > > > I'm not a perl expert, but I don't think it's a libvirt bug anyhow. > Moreover, I don't see any zombies: > > $ perl test-chldhandle-bug-fixed.pl & sleep 5 && echo && ps axf | grep > perl && echo > [1] 11239 > init... pid=11239 > while... > fork 1 > end... pid=11241 > receive chld > fork 2 > end... pid=11242 > receive chld > connection open > > 11239 pts/18 S 0:00 | \_ perl test-chldhandle-bug-fixed.pl > 11245 pts/18 S+ 0:00 | \_ grep --colour=auto perl > > fork 3 > end... pid=11246 > receive chld > fork 4 > end... pid=11247 > receive chld > go next... > > btw: with older version I'm seeing this: > > 11399 pts/18 S 0:00 | \_ perl test-chldhandle-bug.pl > 11401 pts/18 Z 0:00 | | \_ [perl] <defunct> > 11402 pts/18 Z 0:00 | | \_ [perl] <defunct> > 11405 pts/18 S+ 0:00 | \_ grep --colour=auto perl > > What zombies are you seeing? Perl ones or libvirt or ..,? > > Michal

1 0

[libvirt] RFC: viridentitytest Failure on CentOS 6.5?
by Nehal J Wani 17 Mar '14

17 Mar '14

I followed the following steps to build a clean development version of libvirt from source. Can't seem to understand what is wrong. (Entire buildLog has been attached): ➜ /tmp cat /etc/issue CentOS release 6.5 (Final) Kernel \r on an \m ➜ /tmp sestatus SELinux status: enabled SELinuxfs mount: /selinux Current mode: enforcing Mode from config file: enforcing Policy version: 24 Policy from config file: targeted ➜ /tmp git clone git://libvirt.org/libvirt.git Initialized empty Git repository in /tmp/libvirt/.git/ remote: Counting objects: 138935, done. remote: Compressing objects: 100% (21143/21143), done. remote: Total 138935 (delta 117282), reused 138935 (delta 117282) Receiving objects: 100% (138935/138935), 137.72 MiB | 559 KiB/s, done. Resolving deltas: 100% (117282/117282), done. ➜ /tmp cd libvirt ➜ libvirt git:(master) ./autogen.sh --system Running ./configure with --prefix=/usr --sysconfdir=/etc --localstatedir=/var --libdir=/usr/lib64 .. .. Now type 'make' to compile libvirt. ➜ libvirt git:(master) make .. .. make[1]: Leaving directory `/tmp/libvirt' ➜ libvirt git:(master) ✗ cd tests ➜ tests git:(master) ✗ ./viridentitytest /tmp/libvirt/tests/.libs/libsecurityselinuxhelper.so: No such file or directory ➜ tests git:(master) ✗ -- Nehal J Wani

1 0

[libvirt] [PATCH] virlog: add log lock in virLogSetDefaultPriority
by Wangrui (K) 17 Mar '14

17 Mar '14

When the global variable virLogDefaultPriority is modified, it will be better to have a lock. Signed-off-by: Zhou Yimin <zhouyimin(a)huawei.com> --- src/util/virlog.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/util/virlog.c b/src/util/virlog.c index 68af0f3..ba370e2 100644 --- a/src/util/virlog.c +++ b/src/util/virlog.c @@ -498,7 +498,10 @@ virLogSetDefaultPriority(virLogPriority priority) if (virLogInitialize() < 0) return -1; + virLogLock(); virLogDefaultPriority = priority; + virLogUnlock(); + return 0; } -- 1.7.12.4

3 2

[libvirt] [PATCH] Avoid warning message from libxl driver on non-Xen kernels
by Daniel P. Berrange 17 Mar '14

17 Mar '14

The libxl driver reads /proc/xen/capabilities to see if it is on a Dom0 kernel. If that file does not even exist though, an error is logged. Check for the file existance before trying to read its contents to avoid the log message. Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com> --- src/libxl/libxl_driver.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index ae7342a..445a61a 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -970,6 +970,10 @@ libxlDriverShouldLoad(bool privileged) return ret; } + if (!virFileExists("/proc/xen/capabilities")) { + VIR_INFO("Disabling driver as /proc/xen/capabilities does not exist"); + return false; + } /* * Don't load if not running on a Xen control domain (dom0). It is not * sufficient to check for the file to exist as any guest can mount -- 1.8.5.3

3 2

[libvirt] [PATCH] Fix leak of iterator in nwfilter instantiate code
by Daniel P. Berrange 17 Mar '14

17 Mar '14

The ebiptablesCreateRuleInstanceIterate creates a virNWFilterVarCombIterPtr instance and iterates over it. Unfortunately in doing so, it discards the original pointer. At the end of the method it will thus effectively do virNWFilterVarCombIterFree(NULL), which means it will leak the iterator. Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com> --- src/nwfilter/nwfilter_ebiptables_driver.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/nwfilter/nwfilter_ebiptables_driver.c b/src/nwfilter/nwfilter_ebiptables_driver.c index 57c0476..9dbd3ff 100644 --- a/src/nwfilter/nwfilter_ebiptables_driver.c +++ b/src/nwfilter/nwfilter_ebiptables_driver.c @@ -2865,14 +2865,14 @@ ebiptablesCreateRuleInstanceIterate( virNWFilterRuleInstPtr res) { int rc = 0; - virNWFilterVarCombIterPtr vciter; + virNWFilterVarCombIterPtr vciter, tmp; /* rule->vars holds all the variables names that this rule will access. * iterate over all combinations of the variables' values and instantiate * the filtering rule with each combination. */ - vciter = virNWFilterVarCombIterCreate(vars, - rule->varAccess, rule->nVarAccess); + tmp = vciter = virNWFilterVarCombIterCreate(vars, + rule->varAccess, rule->nVarAccess); if (!vciter) return -1; @@ -2881,12 +2881,12 @@ ebiptablesCreateRuleInstanceIterate( nwfilter, rule, ifname, - vciter, + tmp, res); if (rc < 0) break; - vciter = virNWFilterVarCombIterNext(vciter); - } while (vciter != NULL); + tmp = virNWFilterVarCombIterNext(tmp); + } while (tmp != NULL); virNWFilterVarCombIterFree(vciter); -- 1.8.5.3

2 3

[libvirt] [PATCH v5 0/3] support dumping guest memory in compressed format
by qiaonuohan＠cn.fujitsu.com 17 Mar '14

17 Mar '14

dumping guest's memroy is introduced without compression supported, and this is a freature regression of 'virsh dump --memory-only'. This patchset is used to add support in libvirt side to make qemu dump guest's memory in kdump-compressed format and please refer the following address to see implementation of the qemu side, the lastest version of qemu side is v9(ready for being queued). http://lists.nongnu.org/archive/html/qemu-devel/2014-02/msg03016.html ChangLog: Changes from v4 to v5: 1. modify some restriction check Changes from v3 to v4: 1. dropping patch "add dump_memory_format in qemu.conf" 2. fix to follow some conventions Changes from v2 to v3: 1. address Jiri Denemark's comment about adding a new public API instead of changing an old one. Changes from v1 to v2: 1. address Daniel P. Berrange's comment about using a new parameter to replace flags like VIR_DUMP_COMPRESS_ZLIB. qiaonuohan (3): add new virDomainCoreDumpWithFormat API add qemu support to virDomainCoreDumpWithFormat API allow "virsh dump --memory-only" specify dump format include/libvirt/libvirt.h.in | 31 ++++++++++++++ src/driver.h | 7 ++++ src/libvirt.c | 97 ++++++++++++++++++++++++++++++++++++++++++++ src/libvirt_public.syms | 5 +++ src/qemu/qemu_driver.c | 49 ++++++++++++++++++---- src/qemu/qemu_monitor.c | 7 ++-- src/qemu/qemu_monitor.h | 3 +- src/qemu/qemu_monitor_json.c | 4 +- src/qemu/qemu_monitor_json.h | 3 +- src/remote/remote_driver.c | 1 + src/remote/remote_protocol.x | 15 ++++++- src/remote_protocol-structs | 7 ++++ src/test/test_driver.c | 20 +++++++-- tests/qemumonitorjsontest.c | 2 +- tools/virsh-domain.c | 45 +++++++++++++++++++- 15 files changed, 275 insertions(+), 21 deletions(-) -- 1.8.5.3

4 22