[libvirt] Zombie process after open libvirt connection
by Carlos Rodrigues
Hello,
I have the following Perl test script and when open libvirt connection
with qemu+tls i get zombie process.
Let me show the output of script for different versions:
* connection with qemu+libssh2 or test:///
$ perl test-chldhandle-bug.pl
init... pid=15305
while...
fork 1
end... pid=15307
receive chld
fork 2
end... pid=15308
receive chld
connection open
fork 3
end... pid=15350
receive chld
fork 4
receive chld
go next...
* connection with qemu+tls
$ perl test-chldhandle-bug.pl
init... pid=13723
while...
fork 1
end... pid=13725
receive chld
fork 2
end... pid=13726
receive chld
2014-03-13 18:15:56.639+0000: 13723: 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-13 18:15:56.639+0000: 13723: warning : virNetTLSContextCheckCertificate:1099 : Certificate check failed Certificate [session] owner does not match the hostname 10.10.4.249
connection open
fork 3
end... pid=13773
fork 4
end... pid=13800
go next...
Does anyone know how can i solve this issue?
Regards,
--
Carlos Rodrigues
Engenheiro de Software Sénior
Eurotux Informática, S.A. | www.eurotux.com
(t) +351 253 680 300 (m) +351 911 926 110
10 years, 8 months
[libvirt] [PATCH v2] nwfilter: Fix double free of pointer
by Stefan Berger
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
10 years, 8 months
[libvirt] [PATCH] network: fix problems with SRV
by Laine Stump
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
10 years, 8 months
[libvirt] [PATCH] Give virNWFilterVarCombIterNext saner semantics
by Daniel P. Berrange
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
10 years, 8 months
[libvirt] [PATCH] Fix leak on OOM when creating nwfilter rule instances
by Daniel P. Berrange
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
10 years, 8 months
[libvirt] [PATCH] nwfilter: Fix double free of pointer
by Stefan Berger
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
10 years, 8 months
[libvirt] [PATCH] conf: consistent comments about disk enum usage
by Eric Blake
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
10 years, 8 months
[libvirt] [PATCH] build: Make sure src/util/virprobe.h is distributed
by Jiri Denemark
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
10 years, 8 months
[libvirt] [RFC] [PATCH] Fix flawed testcases in qemuhotplugtest.c
by Nehal J Wani
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
10 years, 8 months
[libvirt] [PATCH] build: Fix make distcheck
by Martin Kletzander
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
10 years, 8 months
