[libvirt] [PATCH v2] RPC: Don't accept client if it would overcommit max_clients
Currently, even if max_client limit is hit, we accept() incoming
connection request, but close it immediately. This has disadvantage of
not using listen() queue. We should accept() only those clients we
know we can serve and let all other wait in the (limited) queue.
---
diff to v1:
-moved logic from virNetServerDispatchCheck to virNetServerAddClient
src/rpc/virnetserver.c | 33 -
1 file changed, 28 insertions(+), 5 deletions(-)
diff --git a/src/rpc/virnetserver.c b/src/rpc/virnetserver.c
index cb770c3..2306e10 100644
--- a/src/rpc/virnetserver.c
+++ b/src/rpc/virnetserver.c
@@ -115,6 +115,8 @@ struct _virNetServer {
static virClassPtr virNetServerClass;
static void virNetServerDispose(void *obj);
+static void virNetServerUpdateServicesLocked(virNetServerPtr srv,
+ bool enabled);
static int virNetServerOnceInit(void)
{
@@ -253,30 +255,36 @@ cleanup:
static int virNetServerAddClient(virNetServerPtr srv,
virNetServerClientPtr client)
{
virObjectLock(srv);
if (srv-nclients = srv-nclients_max) {
virReportError(VIR_ERR_RPC,
_(Too many active clients (%zu), dropping connection
from %s),
srv-nclients_max,
virNetServerClientRemoteAddrString(client));
goto error;
}
if (virNetServerClientInit(client) 0)
goto error;
if (VIR_EXPAND_N(srv-clients, srv-nclients, 1) 0)
goto error;
srv-clients[srv-nclients-1] = client;
virObjectRef(client);
+if (srv-nclients == srv-nclients_max) {
+/* Temporarily stop accepting new clients */
+VIR_DEBUG(Temporarily suspending services due to max_clients);
+virNetServerUpdateServicesLocked(srv, false);
+}
+
virNetServerClientSetDispatcher(client,
virNetServerDispatchNewMessage,
srv);
virNetServerClientInitKeepAlive(client, srv-keepaliveInterval,
srv-keepaliveCount);
virObjectUnlock(srv);
return 0;
@@ -1034,98 +1042,113 @@ static void virNetServerAutoShutdownTimer(int timerid
ATTRIBUTE_UNUSED,
}
+static void
+virNetServerUpdateServicesLocked(virNetServerPtr srv,
+ bool enabled)
+{
+size_t i;
+
+for (i = 0; i srv-nservices; i++)
+virNetServerServiceToggle(srv-services[i], enabled);
+}
+
+
void virNetServerUpdateServices(virNetServerPtr srv,
bool enabled)
{
-size_t i;
-
virObjectLock(srv);
-for (i = 0; i srv-nservices; i++)
-virNetServerServiceToggle(srv-services[i], enabled);
-
+virNetServerUpdateServicesLocked(srv, enabled);
virObjectUnlock(srv);
}
void virNetServerRun(virNetServerPtr srv)
{
int timerid = -1;
bool timerActive = false;
size_t i;
virObjectLock(srv);
if (srv-mdns
virNetServerMDNSStart(srv-mdns) 0)
goto cleanup;
srv-quit = 0;
if (srv-autoShutdownTimeout
(timerid = virEventAddTimeout(-1,
virNetServerAutoShutdownTimer,
srv, NULL)) 0) {
virReportError(VIR_ERR_INTERNAL_ERROR, %s,
_(Failed to register shutdown timeout));
goto cleanup;
}
VIR_DEBUG(srv=%p quit=%d, srv, srv-quit);
while (!srv-quit) {
/* A shutdown timeout is specified, so check
* if any drivers have active state, if not
* shutdown after timeout seconds
*/
if (srv-autoShutdownTimeout) {
if (timerActive) {
if (srv-clients) {
VIR_DEBUG(Deactivating shutdown timer %d, timerid);
virEventUpdateTimeout(timerid, -1);
timerActive = false;
}
} else {
if (!srv-clients) {
VIR_DEBUG(Activating shutdown timer %d, timerid);
virEventUpdateTimeout(timerid,
srv-autoShutdownTimeout * 1000);
timerActive = true;
}
}
}
virObjectUnlock(srv);
if (virEventRunDefaultImpl() 0) {
virObjectLock(srv);
VIR_DEBUG(Loop iteration error, exiting);
break;
}
virObjectLock(srv);
reprocess:
for (i = 0; i srv-nclients; i++) {
/* Coverity 5.3.0 couldn't see that srv-clients is non-NULL
* if srv-nclients is non-zero. */
sa_assert(srv-clients);
if (virNetServerClientWantClose(srv-clients[i]))
virNetServerClientClose(srv-clients[i]);
if (virNetServerClientIsClosed(srv-clients[i])) {
Re: [libvirt] [PATCH 0/2] Split driver StateAutoStart from StateInitialization
On 25.07.2013 20:37, Daniel P. Berrange wrote: On Thu, Jul 25, 2013 at 02:32:55PM -0400, John Ferlan wrote: The post push review/comments for the chap authentication determined that trying to connect to qemu driver from within the storage auto start would not be successful, see the following and followups https://www.redhat.com/archives/libvir-list/2013-July/msg01409.html These patches will split the virStateInitialize() into two phases - the first being the bulk of the existing code and the second being just running the auto start functionality for each of the drivers that needs/support it. I realize it's probably too late for 1.1.1, but figured it'd be good to be ready when the barn door opens again. I don't know about others, but I feel the change is simple enough that I wouldn't be against including it in 1.1.1, given that it solves a real world bug we have in startup ordering. Agreed. Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Use flock() instead of fcntl()
Am Donnerstag, 25. Juli 2013, 11:29:37 schrieb Daniel P. Berrange: On Thu, Jul 25, 2013 at 12:07:44PM +0200, David Weber wrote: Thank you for your quick response! Am Donnerstag, 25. Juli 2013, 10:31:40 schrieb Daniel P. Berrange: On Thu, Jul 25, 2013 at 08:23:24AM +, David Weber wrote: Hi, we are interested in using virtlockd on an OCFS2 shared filesystem. We are now facing the problem that virtlockd uses fcntl() locks which aren't supported by OCFS2 with the o2cb cluster stack and we want to avoid using indirect leases. OCFS2 instead supports flock() which is quite similar to fcntl(). I attached a patch which makes libvirt use flock() *instead* of fcntl() and it seems to work. NFS on the contrast only supports fcntl() so it should be configurable which lock type to use. I'm not very experienced with locking, so would such a patch be acceptable or do you see possible problems with it? We definitely can't use flock() unconditionally like that, as it is less widely supported than fcntl() locking and is known broken on most network filesystems (flock() will only lock wrt the local node, not network-wide). flock() locks are cluster aware in recent versions of OCFS2 and I would try to make it configurable which lock type to use. I'm pretty surprised that you can fcntl() is not supported in ocfs2. Are you really sure about that ? This mail message suggests it is supported https://oss.oracle.com/pipermail/ocfs2-users/2009-June/003572.html Support for fcntl locking aka file-range locking aka posix locking is provided by vfs for all file systems. However, that support is appropriate only for local file systems. In ocfs2, we have added support for cluster-aware fcntl locking via the userspace clustering framework that allows one to use ocfs2 with different cluster-stacks. OCFS2 supports fcntl() only with the userspace cluster stacks (pacemaker and cman) which we want to avoid. Looking again at flock() I see it cannot support locking of ranges, only the entire file. This makes it unsuitable as a replacement for libvirt's use of fcntl() I'm afraid. I can only sugggest you configure OCFS2 so that it supports fcntl(), or setup virtlockd to use separate indirect leases on a diffrent shared filesystem, or perhaps try sanlock instead which doesn't require any special filesystem support. It's true that flock() doesn't support locking of ranges but I can't see how this is necessary. I attached a patch with extends virtlockd so it can either use flock() *or* fcntl(). Configurable in the configuration file. SUSE advises to use indirect leases which we also want to avoid: https://www.suse.com/documentation//sled11/book_kvm/?page=/documentation// sled11/book_kvm/data/sec_libvirt_storage_locking.html virtlockd's default configuration does not allow you to lock disk files placed on a shared file system (for example NFS or OCFS2). Locking on these file systems requires you to specify a lockspace directory on the VM Host Server by setting Although that's not completely correct because NFS supports fcntl() That's just badly written explanation for that config setting. It should really be saying that the default configuration does not provide protection across multiple hosts for file paths which are not visible via a shared filesystem. eg a SAN LUN in /dev/sdNNN won't be protected in the default config. Daniel From 2a91cc505b4fb5b467d2add416b2ebf4baa53311 Mon Sep 17 00:00:00 2001 From: David Weber w...@munzinger.de Date: Thu, 25 Jul 2013 14:10:23 +0200 Subject: [PATCH] Add flock() support for locking Configurable in the virtlockd config --- src/locking/libvirt_lockd.aug | 1 + src/locking/lock_daemon_dispatch.c | 6 ++- src/locking/lock_driver_lockd.c| 14 ++- src/locking/lock_protocol.x| 3 +- src/locking/lockd.conf | 7 src/util/virfile.c | 59 +++- src/util/virfile.h | 4 ++ src/util/virlockspace.c| 80 -- src/util/virlockspace.h| 1 + 9 files changed, 148 insertions(+), 27 deletions(-) diff --git a/src/locking/libvirt_lockd.aug b/src/locking/libvirt_lockd.aug index 6a3bcba..7df715f 100644 --- a/src/locking/libvirt_lockd.aug +++ b/src/locking/libvirt_lockd.aug @@ -19,6 +19,7 @@ module Libvirt_lockd = (* Each enty in the config is one of the following three ... *) let entry = bool_entry auto_disk_leases | bool_entry require_lease_for_disks + | bool_entry useFlock | str_entry file_lockspace_dir | str_entry lvm_lockspace_dir | str_entry scsi_lockspace_dir diff --git a/src/locking/lock_daemon_dispatch.c b/src/locking/lock_daemon_dispatch.c index c2e1000..930018c 100644
Re: [libvirt] Use flock() instead of fcntl()
On Fri, Jul 26, 2013 at 10:44:24AM +0200, David Weber wrote: Looking again at flock() I see it cannot support locking of ranges, only the entire file. This makes it unsuitable as a replacement for libvirt's use of fcntl() I'm afraid. I can only sugggest you configure OCFS2 so that it supports fcntl(), or setup virtlockd to use separate indirect leases on a diffrent shared filesystem, or perhaps try sanlock instead which doesn't require any special filesystem support. It's true that flock() doesn't support locking of ranges but I can't see how this is necessary. The code may not currently use ranges, but that doesn't mean it'll stay that way. By adding support for flock() we're preventing us from making use of this feature in the future, and I don't want to see that. Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] conf:Fix a copy paste error
Signed-off-by: Alex Jia a...@redhat.com
---
src/conf/domain_conf.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 10cb7f6..0e74039 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -5153,7 +5153,7 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt,
xmlStrEqual(cur-name, BAD_CAST product)) {
product = (char *)xmlNodeGetContent(cur);
-if (strlen(vendor) PRODUCT_LEN) {
+if (strlen(product) PRODUCT_LEN) {
virReportError(VIR_ERR_XML_ERROR, %s,
_(disk product is more than 16
characters));
goto error;
--
1.8.3.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] conf:Fix a copy paste error
On 07/26/2013 11:19 AM, Alex Jia wrote:
Signed-off-by: Alex Jia a...@redhat.com
---
src/conf/domain_conf.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 10cb7f6..0e74039 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -5153,7 +5153,7 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt,
xmlStrEqual(cur-name, BAD_CAST product)) {
product = (char *)xmlNodeGetContent(cur);
-if (strlen(vendor) PRODUCT_LEN) {
+if (strlen(product) PRODUCT_LEN) {
virReportError(VIR_ERR_XML_ERROR, %s,
_(disk product is more than 16
characters));
goto error;
ACKed and pushed,
Martin
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] formatdomain.html.in: Correctly use code/ in #elementQoS
Interestingly, we had codefloorcode ... /codeoutbound/code which results in much larger block of text to be written in code style that intended. --- docs/formatdomain.html.in | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 7601aaa..d2cee67 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -3539,12 +3539,12 @@ qemu-kvm -net nic,model=? /dev/null forward type of route, nat, or no forward at all). Moreover, the virtual network the interface is connected to is required to have at least inbound QoS set (codeaverage/code at least). Moreover, with - codefloorcode attribute users don't need to specify + codefloor/code attribute users don't need to specify codeaverage/code. However, codepeak/code and codeburst/code attributes still require codeaverage/code. Currently, linux kernel doesn't allow ingress qdiscs to have any classes therefore codefloor/code can be applied only on codeinbound/code and not - /codeoutbound/code. span class=sinceSince 1.0.1/span + codeoutbound/code. span class=sinceSince 1.0.1/span /p h5a name=elementVlanTagSetting VLAN tag (on supported network types only)/a/h5 -- 1.8.1.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] formatdomain.html.in: Document implementation limitation of QoS
The outbound/@peak is ignored (since QoS was introduced). This is due to kernel limitation of know allowing ingress filters to have peak just average rate. However, we should document this limitation to not confuse users. --- docs/formatdomain.html.in | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index d2cee67..78e132e 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -3530,7 +3530,9 @@ qemu-kvm -net nic,model=? /dev/null codepeak/code speed. Accepted values for attributes are integer numbers. The units for codeaverage/code and codepeak/code attributes are kilobytes per second, and for the codeburst/code just kilobytes. - span class=sinceSince 0.9.4/span The codeinbound/code can + Note the limitation of implementation: the codepeak/code attribute in + codeoutbound/code element is ignored (as linux ingress filters don't + know it yet). span class=sinceSince 0.9.4/span The codeinbound/code can optionally have codefloor/code attribute. This is there for guaranteeing minimal throughput for shaped interfaces. This, however, requires that all traffic goes through one point where QoS decisions can -- 1.8.1.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] conf:Fix a copy paste error
On 26.07.2013 11:19, Alex Jia wrote:
Signed-off-by: Alex Jia a...@redhat.com
---
src/conf/domain_conf.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 10cb7f6..0e74039 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -5153,7 +5153,7 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt,
xmlStrEqual(cur-name, BAD_CAST product)) {
product = (char *)xmlNodeGetContent(cur);
-if (strlen(vendor) PRODUCT_LEN) {
+if (strlen(product) PRODUCT_LEN) {
virReportError(VIR_ERR_XML_ERROR, %s,
_(disk product is more than 16
characters));
Seeing this context makes me think we should make error message us
PRODUCT_LEN instead of hardcoding 16:
virReportError(VIR_ERR_XML_ERROR,
_(disk product is more than %d characters),
PRODUCT_LEN);
Same applies for VENDOR_LEN just a few lines above.
Michal
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] formatdomain.html.in: Correctly use code/ in #elementQoS
On 07/26/2013 11:46 AM, Michal Privoznik wrote: Interestingly, we had codefloorcode ... /codeoutbound/code which results in much larger block of text to be written in code style that intended. --- docs/formatdomain.html.in | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 7601aaa..d2cee67 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -3539,12 +3539,12 @@ qemu-kvm -net nic,model=? /dev/null forward type of route, nat, or no forward at all). Moreover, the virtual network the interface is connected to is required to have at least inbound QoS set (codeaverage/code at least). Moreover, with - codefloorcode attribute users don't need to specify + codefloor/code attribute users don't need to specify codeaverage/code. However, codepeak/code and codeburst/code attributes still require codeaverage/code. Currently, linux kernel doesn't allow ingress qdiscs to have any classes therefore codefloor/code can be applied only on codeinbound/code and not - /codeoutbound/code. span class=sinceSince 1.0.1/span + codeoutbound/code. span class=sinceSince 1.0.1/span /p h5a name=elementVlanTagSetting VLAN tag (on supported network types only)/a/h5 ACK, trivial. Martin -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] formatdomain.html.in: Document implementation limitation of QoS
On 07/26/2013 11:46 AM, Michal Privoznik wrote: The outbound/@peak is ignored (since QoS was introduced). This is due to kernel limitation of know allowing ingress filters to have peak just average rate. However, we should document this limitation to not confuse users. --- docs/formatdomain.html.in | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index d2cee67..78e132e 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -3530,7 +3530,9 @@ qemu-kvm -net nic,model=? /dev/null codepeak/code speed. Accepted values for attributes are integer numbers. The units for codeaverage/code and codepeak/code attributes are kilobytes per second, and for the codeburst/code just kilobytes. - span class=sinceSince 0.9.4/span The codeinbound/code can + Note the limitation of implementation: the codepeak/code attribute in + codeoutbound/code element is ignored (as linux ingress filters don't + know it yet). span class=sinceSince 0.9.4/span The codeinbound/code can optionally have codefloor/code attribute. This is there for guaranteeing minimal throughput for shaped interfaces. This, however, requires that all traffic goes through one point where QoS decisions can ACK, Martin -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] conf:Fix a copy paste error
On Fri 26 Jul 2013 11:50:17 AM CEST, Michal Privoznik wrote:
On 26.07.2013 11:19, Alex Jia wrote:
Signed-off-by: Alex Jia a...@redhat.com
---
src/conf/domain_conf.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 10cb7f6..0e74039 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -5153,7 +5153,7 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt,
xmlStrEqual(cur-name, BAD_CAST product)) {
product = (char *)xmlNodeGetContent(cur);
-if (strlen(vendor) PRODUCT_LEN) {
+if (strlen(product) PRODUCT_LEN) {
virReportError(VIR_ERR_XML_ERROR, %s,
_(disk product is more than 16
characters));
Seeing this context makes me think we should make error message us
PRODUCT_LEN instead of hardcoding 16:
virReportError(VIR_ERR_XML_ERROR,
_(disk product is more than %d characters),
PRODUCT_LEN);
Same applies for VENDOR_LEN just a few lines above.
Michal
Makes sense, ACK if you want to do that.
Martin
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/4] Fix a pair of crashes in DNS XML parsing
*** BLUAAARRRGHB HERE *** Ján Tomko (4): Set the number of elements to 0 in virNetwork*Clear Don't check validity of missing attributes in DNS SRV XML Remove double space in error messages Remove redundant free in virNetworkDNSHostDefParseXML src/conf/network_conf.c | 14 +- 1 file changed, 9 insertions(+), 5 deletions(-) -- 1.8.1.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/4] Set the number of elements to 0 in virNetwork*Clear
Leaving it at -1 causes invalid free in virNetworkDefUpdateDNSHost if virNetworkDNSHostDefParseXML fails and virNetworkDNSHostDefClear gets called twice. --- src/conf/network_conf.c | 4 1 file changed, 4 insertions(+) diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c index d616e12..e3e0e89 100644 --- a/src/conf/network_conf.c +++ b/src/conf/network_conf.c @@ -136,6 +136,7 @@ virNetworkIpDefClear(virNetworkIpDefPtr def) while (def-nhosts--) virNetworkDHCPHostDefClear(def-hosts[def-nhosts]); +def-nhosts = 0; VIR_FREE(def-hosts); VIR_FREE(def-tftproot); @@ -161,6 +162,7 @@ virNetworkDNSHostDefClear(virNetworkDNSHostDefPtr def) while (def-nnames--) VIR_FREE(def-names[def-nnames]); VIR_FREE(def-names); +def-nnames = 0; } static void @@ -190,6 +192,7 @@ virNetworkDNSDefClear(virNetworkDNSDefPtr def) virNetworkDNSSrvDefClear(def-srvs[def-nsrvs]); VIR_FREE(def-srvs); } +def-ntxts = def-nhosts = def-nsrvs = 0; } static void @@ -206,6 +209,7 @@ virNetworkForwardDefClear(virNetworkForwardDefPtr def) virNetworkForwardIfDefClear(def-ifs[i]); } VIR_FREE(def-ifs); +def-nifs = def-npfs = 0; } void -- 1.8.1.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 4/4] Remove redundant free in virNetworkDNSHostDefParseXML
ip has to be NULL at this point. --- src/conf/network_conf.c | 1 - 1 file changed, 1 deletion(-) diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c index 2561c35..2040d26 100644 --- a/src/conf/network_conf.c +++ b/src/conf/network_conf.c @@ -859,7 +859,6 @@ virNetworkDNSHostDefParseXML(const char *networkName, virReportError(VIR_ERR_XML_DETAIL, _(Missing IP address in network '%s' DNS HOST record), networkName); -VIR_FREE(ip); goto error; } -- 1.8.1.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 3/4] Remove double space in error messages
---
src/conf/network_conf.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
index 5faac92..2561c35 100644
--- a/src/conf/network_conf.c
+++ b/src/conf/network_conf.c
@@ -920,7 +920,7 @@ virNetworkDNSSrvDefParseXML(const char *networkName,
if (!(def-service = virXMLPropString(node, service)) !partialOkay) {
virReportError(VIR_ERR_XML_DETAIL,
_(Missing required service attribute in DNS SRV record
- of network %s), networkName);
+ of network %s), networkName);
goto error;
}
if (def-service strlen(def-service) DNS_RECORD_LENGTH_SRV) {
@@ -943,7 +943,7 @@ virNetworkDNSSrvDefParseXML(const char *networkName,
(STRNEQ(def-protocol, udp))) {
virReportError(VIR_ERR_XML_DETAIL,
_(Invalid protocol attribute value '%s'
- in DNS SRV record of network %s),
+ in DNS SRV record of network %s),
def-protocol, networkName);
goto error;
}
--
1.8.1.5
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/4] Don't check validity of missing attributes in DNS SRV XML
This fixes a crash if one of them is missing.
https://bugzilla.redhat.com/show_bug.cgi?id=988718
---
src/conf/network_conf.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
index e3e0e89..5faac92 100644
--- a/src/conf/network_conf.c
+++ b/src/conf/network_conf.c
@@ -923,7 +923,7 @@ virNetworkDNSSrvDefParseXML(const char *networkName,
of network %s), networkName);
goto error;
}
-if (strlen(def-service) DNS_RECORD_LENGTH_SRV) {
+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);
@@ -939,7 +939,8 @@ virNetworkDNSSrvDefParseXML(const char *networkName,
}
/* Check whether protocol value is the supported one */
-if (STRNEQ(def-protocol, tcp) (STRNEQ(def-protocol, udp))) {
+if (def-protocol STRNEQ(def-protocol, tcp)
+(STRNEQ(def-protocol, udp))) {
virReportError(VIR_ERR_XML_DETAIL,
_(Invalid protocol attribute value '%s'
in DNS SRV record of network %s),
--
1.8.1.5
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Use flock() instead of fcntl()
Am Freitag, 26. Juli 2013, 10:14:59 schrieb Daniel P. Berrange: On Fri, Jul 26, 2013 at 10:44:24AM +0200, David Weber wrote: Looking again at flock() I see it cannot support locking of ranges, only the entire file. This makes it unsuitable as a replacement for libvirt's use of fcntl() I'm afraid. I can only sugggest you configure OCFS2 so that it supports fcntl(), or setup virtlockd to use separate indirect leases on a diffrent shared filesystem, or perhaps try sanlock instead which doesn't require any special filesystem support. It's true that flock() doesn't support locking of ranges but I can't see how this is necessary. The code may not currently use ranges, but that doesn't mean it'll stay that way. By adding support for flock() we're preventing us from making use of this feature in the future, and I don't want to see that. Just curious, what would be a possible feature which would require range based locking? I would really like to see flock() support in virtlockd because all other solutions have major drawbacks for me. Daniel -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Use flock() instead of fcntl()
On Fri, Jul 26, 2013 at 12:31:35PM +0200, David Weber wrote: Am Freitag, 26. Juli 2013, 10:14:59 schrieb Daniel P. Berrange: On Fri, Jul 26, 2013 at 10:44:24AM +0200, David Weber wrote: Looking again at flock() I see it cannot support locking of ranges, only the entire file. This makes it unsuitable as a replacement for libvirt's use of fcntl() I'm afraid. I can only sugggest you configure OCFS2 so that it supports fcntl(), or setup virtlockd to use separate indirect leases on a diffrent shared filesystem, or perhaps try sanlock instead which doesn't require any special filesystem support. It's true that flock() doesn't support locking of ranges but I can't see how this is necessary. The code may not currently use ranges, but that doesn't mean it'll stay that way. By adding support for flock() we're preventing us from making use of this feature in the future, and I don't want to see that. Just curious, what would be a possible feature which would require range based locking? I would really like to see flock() support in virtlockd because all other solutions have major drawbacks for me. Currently we use locks to protect the content of disk images. During startup/shutdown, however, libvirt also makes changes to the metadata of images by setting SELinux labels, uid/gid ownership and potentially ACLs. Currently we've delibrately crippled some of our code during shutdown since it isn't safe in the face of multiple libvirt's running. We need to introduce locking of file metadata to protect this code. The metadata locks, however, must not conflict with the content lock. Thus the reason why we only lock a single byte (range 0-1) for content locks, is that we want to be able to additional locks (range 1-2 or similar) for the metadata locks on the same files. Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/4] Set the number of elements to 0 in virNetwork*Clear
On 26.07.2013 12:28, Ján Tomko wrote: Leaving it at -1 causes invalid free in virNetworkDefUpdateDNSHost if virNetworkDNSHostDefParseXML fails and virNetworkDNSHostDefClear gets called twice. --- src/conf/network_conf.c | 4 1 file changed, 4 insertions(+) diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c index d616e12..e3e0e89 100644 --- a/src/conf/network_conf.c +++ b/src/conf/network_conf.c @@ -136,6 +136,7 @@ virNetworkIpDefClear(virNetworkIpDefPtr def) while (def-nhosts--) virNetworkDHCPHostDefClear(def-hosts[def-nhosts]); +def-nhosts = 0; I think we need a sligthly different approach here. What if def-nhost is 0 before entering the while() loop? It gets decreased to -1 and ... We need: while (def-nhosts) virNetworkDHCPHostDefCelar( [--def-nhosts]); or just use for (i = 0; i def-nhosts; i++) theFunction( ...[i]); Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/4] Remove double space in error messages
On 26.07.2013 12:28, Ján Tomko wrote:
---
src/conf/network_conf.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
index 5faac92..2561c35 100644
--- a/src/conf/network_conf.c
+++ b/src/conf/network_conf.c
@@ -920,7 +920,7 @@ virNetworkDNSSrvDefParseXML(const char *networkName,
if (!(def-service = virXMLPropString(node, service)) !partialOkay)
{
virReportError(VIR_ERR_XML_DETAIL,
_(Missing required service attribute in DNS SRV
record
- of network %s), networkName);
+ of network %s), networkName);
goto error;
}
if (def-service strlen(def-service) DNS_RECORD_LENGTH_SRV) {
@@ -943,7 +943,7 @@ virNetworkDNSSrvDefParseXML(const char *networkName,
(STRNEQ(def-protocol, udp))) {
virReportError(VIR_ERR_XML_DETAIL,
_(Invalid protocol attribute value '%s'
- in DNS SRV record of network %s),
+ in DNS SRV record of network %s),
def-protocol, networkName);
goto error;
}
ACK
Michal
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/4] Don't check validity of missing attributes in DNS SRV XML
On 26.07.2013 12:28, Ján Tomko wrote:
This fixes a crash if one of them is missing.
https://bugzilla.redhat.com/show_bug.cgi?id=988718
---
src/conf/network_conf.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
index e3e0e89..5faac92 100644
--- a/src/conf/network_conf.c
+++ b/src/conf/network_conf.c
@@ -923,7 +923,7 @@ virNetworkDNSSrvDefParseXML(const char *networkName,
of network %s), networkName);
goto error;
}
-if (strlen(def-service) DNS_RECORD_LENGTH_SRV) {
+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);
@@ -939,7 +939,8 @@ virNetworkDNSSrvDefParseXML(const char *networkName,
}
/* Check whether protocol value is the supported one */
-if (STRNEQ(def-protocol, tcp) (STRNEQ(def-protocol, udp))) {
+if (def-protocol STRNEQ(def-protocol, tcp)
+(STRNEQ(def-protocol, udp))) {
virReportError(VIR_ERR_XML_DETAIL,
_(Invalid protocol attribute value '%s'
in DNS SRV record of network %s),
ACK
Michal
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 4/4] Remove redundant free in virNetworkDNSHostDefParseXML
On 26.07.2013 12:28, Ján Tomko wrote: ip has to be NULL at this point. --- src/conf/network_conf.c | 1 - 1 file changed, 1 deletion(-) diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c index 2561c35..2040d26 100644 --- a/src/conf/network_conf.c +++ b/src/conf/network_conf.c @@ -859,7 +859,6 @@ virNetworkDNSHostDefParseXML(const char *networkName, virReportError(VIR_ERR_XML_DETAIL, _(Missing IP address in network '%s' DNS HOST record), networkName); -VIR_FREE(ip); goto error; } ACK Increasing context size would show the line just above these where we can clearly see without need to open the network_conf.c that @ip has to be NULL to take this path. Nevermind. Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2] Set the number of elements 0 in virNetwork*Clear
Decrementing it when it was already 0 causes an invalid free
in virNetworkDefUpdateDNSHost if virNetworkDNSHostDefParseXML
fails and virNetworkDNSHostDefClear gets called twice.
virNetworkForwardDefClear left the number untouched even if it
freed all the elements.
---
src/conf/network_conf.c | 21 +++--
1 file changed, 11 insertions(+), 10 deletions(-)
diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
index d616e12..490b04d 100644
--- a/src/conf/network_conf.c
+++ b/src/conf/network_conf.c
@@ -134,8 +134,8 @@ virNetworkIpDefClear(virNetworkIpDefPtr def)
VIR_FREE(def-family);
VIR_FREE(def-ranges);
-while (def-nhosts--)
-virNetworkDHCPHostDefClear(def-hosts[def-nhosts]);
+while (def-nhosts)
+virNetworkDHCPHostDefClear(def-hosts[--def-nhosts]);
VIR_FREE(def-hosts);
VIR_FREE(def-tftproot);
@@ -158,8 +158,8 @@ virNetworkDNSTxtDefClear(virNetworkDNSTxtDefPtr def)
static void
virNetworkDNSHostDefClear(virNetworkDNSHostDefPtr def)
{
-while (def-nnames--)
-VIR_FREE(def-names[def-nnames]);
+while (def-nnames)
+VIR_FREE(def-names[--def-nnames]);
VIR_FREE(def-names);
}
@@ -176,18 +176,18 @@ static void
virNetworkDNSDefClear(virNetworkDNSDefPtr def)
{
if (def-txts) {
-while (def-ntxts--)
-virNetworkDNSTxtDefClear(def-txts[def-ntxts]);
+while (def-ntxts)
+virNetworkDNSTxtDefClear(def-txts[--def-ntxts]);
VIR_FREE(def-txts);
}
if (def-hosts) {
-while (def-nhosts--)
-virNetworkDNSHostDefClear(def-hosts[def-nhosts]);
+while (def-nhosts)
+virNetworkDNSHostDefClear(def-hosts[--def-nhosts]);
VIR_FREE(def-hosts);
}
if (def-srvs) {
-while (def-nsrvs--)
-virNetworkDNSSrvDefClear(def-srvs[def-nsrvs]);
+while (def-nsrvs)
+virNetworkDNSSrvDefClear(def-srvs[--def-nsrvs]);
VIR_FREE(def-srvs);
}
}
@@ -206,6 +206,7 @@ virNetworkForwardDefClear(virNetworkForwardDefPtr def)
virNetworkForwardIfDefClear(def-ifs[i]);
}
VIR_FREE(def-ifs);
+def-nifs = def-npfs = 0;
}
void
--
1.8.1.5
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2] Set the number of elements 0 in virNetwork*Clear
On 26.07.2013 12:54, Ján Tomko wrote:
Decrementing it when it was already 0 causes an invalid free
in virNetworkDefUpdateDNSHost if virNetworkDNSHostDefParseXML
fails and virNetworkDNSHostDefClear gets called twice.
virNetworkForwardDefClear left the number untouched even if it
freed all the elements.
---
src/conf/network_conf.c | 21 +++--
1 file changed, 11 insertions(+), 10 deletions(-)
diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
index d616e12..490b04d 100644
--- a/src/conf/network_conf.c
+++ b/src/conf/network_conf.c
@@ -134,8 +134,8 @@ virNetworkIpDefClear(virNetworkIpDefPtr def)
VIR_FREE(def-family);
VIR_FREE(def-ranges);
-while (def-nhosts--)
-virNetworkDHCPHostDefClear(def-hosts[def-nhosts]);
+while (def-nhosts)
+virNetworkDHCPHostDefClear(def-hosts[--def-nhosts]);
VIR_FREE(def-hosts);
VIR_FREE(def-tftproot);
@@ -158,8 +158,8 @@ virNetworkDNSTxtDefClear(virNetworkDNSTxtDefPtr def)
static void
virNetworkDNSHostDefClear(virNetworkDNSHostDefPtr def)
{
-while (def-nnames--)
-VIR_FREE(def-names[def-nnames]);
+while (def-nnames)
+VIR_FREE(def-names[--def-nnames]);
VIR_FREE(def-names);
}
@@ -176,18 +176,18 @@ static void
virNetworkDNSDefClear(virNetworkDNSDefPtr def)
{
if (def-txts) {
-while (def-ntxts--)
-virNetworkDNSTxtDefClear(def-txts[def-ntxts]);
+while (def-ntxts)
+virNetworkDNSTxtDefClear(def-txts[--def-ntxts]);
VIR_FREE(def-txts);
}
if (def-hosts) {
-while (def-nhosts--)
-virNetworkDNSHostDefClear(def-hosts[def-nhosts]);
+while (def-nhosts)
+virNetworkDNSHostDefClear(def-hosts[--def-nhosts]);
VIR_FREE(def-hosts);
}
if (def-srvs) {
-while (def-nsrvs--)
-virNetworkDNSSrvDefClear(def-srvs[def-nsrvs]);
+while (def-nsrvs)
+virNetworkDNSSrvDefClear(def-srvs[--def-nsrvs]);
VIR_FREE(def-srvs);
}
}
@@ -206,6 +206,7 @@ virNetworkForwardDefClear(virNetworkForwardDefPtr def)
virNetworkForwardIfDefClear(def-ifs[i]);
}
VIR_FREE(def-ifs);
+def-nifs = def-npfs = 0;
}
void
ACK
Michal
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 4/4] Add inputpool to storagevolxml2argvtest
...snip...? diff --git a/tests/storagevolxml2argvtest.c
b/tests/storagevolxml2argvtest.c
index b220994..3c338ce 100644
--- a/tests/storagevolxml2argvtest.c
+++ b/tests/storagevolxml2argvtest.c
@@ -11,10 +11,32 @@
const char create_tool[] = qemu-img;
+/* createVol sets this on volume creation */
+static void
+testSetVolumeType(virStorageVolDefPtr vol,
+ virStoragePoolDefPtr pool)
+{
+if (!vol)
+return;
+
+switch (pool-type) {
+case VIR_STORAGE_POOL_DIR:
+case VIR_STORAGE_POOL_FS:
+case VIR_STORAGE_POOL_NETFS:
+vol-type = VIR_STORAGE_VOL_FILE;
+return;
+
+case VIR_STORAGE_POOL_LOGICAL:
+vol-type = VIR_STORAGE_VOL_BLOCK;
+return;
+}
+}
+
static int
testCompareXMLToArgvFiles(bool shouldFail,
const char *poolxml,
const char *volxml,
+ const char *inputpoolxml,
const char *inputvolxml,
const char *cmdline,
unsigned int flags,
@@ -22,6 +44,7 @@ testCompareXMLToArgvFiles(bool shouldFail,
{
char *volXmlData = NULL;
char *poolXmlData = NULL;
+char *inputpoolXmlData = NULL;
char *inputvolXmlData = NULL;
char *expectedCmdline = NULL;
char *actualCmdline = NULL;
@@ -34,6 +57,7 @@ testCompareXMLToArgvFiles(bool shouldFail,
virStorageVolDefPtr vol = NULL, inputvol = NULL;
virStoragePoolDefPtr pool = NULL;
+virStoragePoolDefPtr inputpool = NULL;
virStoragePoolObj poolobj = {.def = NULL };
@@ -53,13 +77,23 @@ testCompareXMLToArgvFiles(bool shouldFail,
poolobj.def = pool;
+if (inputpoolxml) {
+if (virtTestLoadFile(inputpoolxml, inputpoolXmlData) 0)
+goto cleanup;
+if (!(inputpool = virStoragePoolDefParseString(inputpoolXmlData)))
+goto cleanup;
+}
+
if (!(vol = virStorageVolDefParseString(pool, volXmlData)))
goto cleanup;
if (inputvolxml
-!(inputvol = virStorageVolDefParseString(pool, inputvolXmlData)))
+!(inputvol = virStorageVolDefParseString(inputpool,
inputvolXmlData)))
goto cleanup;
+testSetVolumeType(vol, pool);
+testSetVolumeType(inputvol, inputpool);
Coverity is grumpy (FORWARD_NULL) right here as inputpool could be NULL
since setting 'inputpool' has been based upon 'inputpoolxml' being true
up to this point
94 testSetVolumeType(vol, pool);
(13) Event var_deref_model: Passing null pointer inputpool to
function testSetVolumeType(virStorageVolDefPtr, virStoragePoolDefPtr),
which dereferences it. [details]
Also see events:[assign_zero]
95 testSetVolumeType(inputvol, inputpool);
+
cmd = virStorageBackendCreateQemuImgCmd(conn, poolobj, vol, inputvol,
flags, create_tool, imgformat);
if (!cmd) {
@@ -88,11 +122,13 @@ testCompareXMLToArgvFiles(bool shouldFail,
cleanup:
virStoragePoolDefFree(pool);
+virStoragePoolDefFree(inputpool);
virStorageVolDefFree(vol);
virStorageVolDefFree(inputvol);
virCommandFree(cmd);
VIR_FREE(actualCmdline);
VIR_FREE(expectedCmdline);
+VIR_FREE(inputpoolXmlData);
VIR_FREE(poolXmlData);
VIR_FREE(volXmlData);
VIR_FREE(inputvolXmlData);
@@ -104,6 +140,7 @@ struct testInfo {
bool shouldFail;
const char *pool;
const char *vol;
+const char *inputpool;
const char *inputvol;
const char *cmdline;
unsigned int flags;
@@ -116,6 +153,7 @@ testCompareXMLToArgvHelper(const void *data)
int result = -1;
const struct testInfo *info = data;
char *poolxml = NULL;
+char *inputpoolxml = NULL;
char *volxml = NULL;
char *inputvolxml = NULL;
char *cmdline = NULL;
@@ -124,6 +162,10 @@ testCompareXMLToArgvHelper(const void *data)
virAsprintf(inputvolxml, %s/storagevolxml2xmlin/%s.xml,
abs_srcdir, info-inputvol) 0)
goto cleanup;
+if (info-inputpool
+virAsprintf(inputpoolxml, %s/storagepoolxml2xmlin/%s.xml,
+abs_srcdir, info-inputpool) 0)
+goto cleanup;
if (virAsprintf(poolxml, %s/storagepoolxml2xmlin/%s.xml,
abs_srcdir, info-pool) 0 ||
virAsprintf(volxml, %s/storagevolxml2xmlin/%s.xml,
@@ -135,13 +177,15 @@ testCompareXMLToArgvHelper(const void *data)
goto cleanup;
result = testCompareXMLToArgvFiles(info-shouldFail, poolxml, volxml,
- inputvolxml, cmdline, info-flags,
+ inputpoolxml, inputvolxml,
+ cmdline, info-flags,
info-imgformat);
cleanup:
VIR_FREE(poolxml);
Re: [libvirt] [PATCH v2] Set the number of elements 0 in virNetwork*Clear
On 07/26/2013 01:00 PM, Michal Privoznik wrote: On 26.07.2013 12:54, Ján Tomko wrote: Decrementing it when it was already 0 causes an invalid free in virNetworkDefUpdateDNSHost if virNetworkDNSHostDefParseXML fails and virNetworkDNSHostDefClear gets called twice. virNetworkForwardDefClear left the number untouched even if it freed all the elements. --- src/conf/network_conf.c | 21 +++-- 1 file changed, 11 insertions(+), 10 deletions(-) ACK Michal Thanks, I've pushed the series. Jan -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/4] Fix a pair of crashes in DNS XML parsing
On Fri, Jul 26, 2013 at 12:28:20PM +0200, Ján Tomko wrote: *** BLUAAARRRGHB HERE *** Ján Tomko (4): Set the number of elements to 0 in virNetwork*Clear Don't check validity of missing attributes in DNS SRV XML Remove double space in error messages Remove redundant free in virNetworkDNSHostDefParseXML src/conf/network_conf.c | 14 +- 1 file changed, 9 insertions(+), 5 deletions(-) IMHO you should be including one or more test data files to demonstrate the crash fix. Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] Resolve Coverity complaint in storagevolxml2argvtest
Ignore NULL pool in testSetVolumeType to silence Coverity,
even though we only call it with NULL pool when vol is also NULL.
(13) Event var_deref_model: Passing null pointer inputpool to
function testSetVolumeType(virStorageVolDefPtr, virStoragePoolDefPtr),
which dereferences it. [details]
Also see events: [assign_zero]
95testSetVolumeType(inputvol, inputpool);
---
tests/storagevolxml2argvtest.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tests/storagevolxml2argvtest.c b/tests/storagevolxml2argvtest.c
index 7774617..b1cf09f 100644
--- a/tests/storagevolxml2argvtest.c
+++ b/tests/storagevolxml2argvtest.c
@@ -16,7 +16,7 @@ static void
testSetVolumeType(virStorageVolDefPtr vol,
virStoragePoolDefPtr pool)
{
-if (!vol)
+if (!vol || !pool)
return;
switch (pool-type) {
--
1.8.1.5
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] virt-login-shell joins users into lxc container.
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1
I can't seem to get the error reporting to turn on, what am I doing wrong.,
if (virInitialize() 0) {
fprintf(stderr, _(Failed to initialize libvirt));
return EXIT_FAILURE;
}
if (virErrorInitialize() 0) {
fprintf(stderr, _(Failed to initialize libvirt Error Handling));
return EXIT_FAILURE;
}
virSetErrorFunc(NULL, NULL);
virReportSystemError(EINVAL, %s, _(Test));
And I get no output, I thought I would get error on stderr?
-BEGIN PGP SIGNATURE-
Version: GnuPG v1.4.13 (GNU/Linux)
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/
iEYEARECAAYFAlHyX7cACgkQrlYvE4MpobNrbQCgmR5hp4Lz9pgCv93V2Fb6r0ec
VZsAn13t/fiFqoOEmb6PIb5xa3Gzbr9o
=fhdb
-END PGP SIGNATURE-
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] virt-login-shell joins users into lxc container.
On Fri, Jul 26, 2013 at 07:38:31AM -0400, Daniel J Walsh wrote:
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1
I can't seem to get the error reporting to turn on, what am I doing wrong.,
if (virInitialize() 0) {
fprintf(stderr, _(Failed to initialize libvirt));
return EXIT_FAILURE;
}
if (virErrorInitialize() 0) {
fprintf(stderr, _(Failed to initialize libvirt Error Handling));
return EXIT_FAILURE;
}
virSetErrorFunc(NULL, NULL);
virReportSystemError(EINVAL, %s, _(Test));
And I get no output, I thought I would get error on stderr?
You would, except that you just turned off printing to stderr by
calling virSetErrorFunc in that way.
Daniel
--
|: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Resolve Coverity complaint in storagevolxml2argvtest
On 07/26/2013 07:27 AM, Ján Tomko wrote:
Ignore NULL pool in testSetVolumeType to silence Coverity,
even though we only call it with NULL pool when vol is also NULL.
(13) Event var_deref_model: Passing null pointer inputpool to
function testSetVolumeType(virStorageVolDefPtr, virStoragePoolDefPtr),
which dereferences it. [details]
Also see events: [assign_zero]
95testSetVolumeType(inputvol, inputpool);
---
tests/storagevolxml2argvtest.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tests/storagevolxml2argvtest.c b/tests/storagevolxml2argvtest.c
index 7774617..b1cf09f 100644
--- a/tests/storagevolxml2argvtest.c
+++ b/tests/storagevolxml2argvtest.c
@@ -16,7 +16,7 @@ static void
testSetVolumeType(virStorageVolDefPtr vol,
virStoragePoolDefPtr pool)
{
-if (!vol)
+if (!vol || !pool)
return;
switch (pool-type) {
ACK.
I ran the coverity analysis and it's happy.
John
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Entering freeze for libvirt-1.1.1
On Fri, Jul 26, 2013 at 06:01:23AM -0400, Alex Jia wrote: Hello Daniel, I gave a basic try for libvirt-1.1.1-rc1 on RHEL6. [root@202 libvirt-1.1.1]# make Notes, sometimes, I can meet the following errors, it's not reproducible each time, maybe, it's a network issue. slice I/O error : Attempt to load network entity http://www.w3.org/TR/xhtml1/DTD/xhtml1-strict.dtd formatdomain.html.in:2: warning: failed to load external entity http://www.w3.org/TR/xhtml1/DTD/xhtml1-strict.dtd; C//DTD XHTML 1.0 Strict//EN http://www.w3.org/TR/xhtml1/DTD/xhtml1-strict.dtd; ^ formatdomain.html.in:4582: parser error : Entity 'mdash' not defined li 'i6300esb' mdash; the recommended device, you need to install xhtml1-dtds for local validation of generated HTML [...] TEST: virnetsockettest !!!.!!! 15 FAIL weird ... Notes, works well with PATH environment setting, but it still is failed for running 'make -C tests valgrind' due to previous 'make check' fails, whether we my bypass/fix this for 'make check'. BTW, I also checked libvirt upstream with make -C tests valgrind, it's fine and without memory leaks. [root@202 libvirt-1.1.1]# make syntax-check GEN HACKING GEN bracket-spacing-check ./build-aux/vc-list-files: Failed to determine type of version control used in /home/ajia/Workspace/rc1/libvirt-1.1.1 Can't open perl script ./build-aux/bracket-spacing.pl: No such file or directory maint.mk: incorrect whitespace, see HACKING for rules make: *** [bracket-spacing-check] Error 1 [root@202 libvirt-1.1.1]# ls ./build-aux/bracket-spacing.pl ls: cannot access ./build-aux/bracket-spacing.pl: No such file or directory Notes, it's okay on libvirt upstream and can find ./build-aux/bracket-spacing.pl file. To run coverity on libvirt-1.1.1-rc1 and attach report as an attachment. thanks ! Daniel -- Daniel Veillard | Open Source and Standards, Red Hat veill...@redhat.com | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | virtualization library http://libvirt.org/ -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 0/2] Split driver StateAutoStart from StateInitialization
This is an update to change the 'int' AutoStart function to 'void' as was discussed in the initial review: https://www.redhat.com/archives/libvir-list/2013-July/msg01674.html John Ferlan (2): Separate out StateAutoStart from StateInitialize virStateDriver - Separate AutoStart from Initialize src/driver.h | 4 src/libvirt.c| 14 +- src/libxl/libxl_driver.c | 16 +--- src/lxc/lxc_driver.c | 16 ++-- src/network/bridge_driver.c | 18 +- src/qemu/qemu_driver.c | 17 +++-- src/storage/storage_driver.c | 18 +- src/uml/uml_driver.c | 17 +++-- 8 files changed, 108 insertions(+), 12 deletions(-) -- 1.8.1.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 2/2] virStateDriver - Separate AutoStart from Initialize
Adjust these drivers to handle their Autostart functionality after each
of the drivers has gone through their Initialization functions
---
src/libxl/libxl_driver.c | 16 +---
src/lxc/lxc_driver.c | 16 ++--
src/network/bridge_driver.c | 18 +-
src/qemu/qemu_driver.c | 17 +++--
src/storage/storage_driver.c | 18 +-
src/uml/uml_driver.c | 17 +++--
6 files changed, 91 insertions(+), 11 deletions(-)
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 98b1985..eb252d0 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -1348,9 +1348,6 @@ libxlStateInitialize(bool privileged,
NULL, NULL) 0)
goto error;
-virDomainObjListForEach(libxl_driver-domains, libxlAutostartDomain,
-libxl_driver);
-
virDomainObjListForEach(libxl_driver-domains, libxlDomainManagedSaveLoad,
libxl_driver);
@@ -1368,6 +1365,18 @@ fail:
return ret;
}
+static void
+libxlStateAutoStart(void)
+{
+if (!libxl_driver)
+return;
+
+libxlDriverLock(libxl_driver);
+virDomainObjListForEach(libxl_driver-domains, libxlAutostartDomain,
+libxl_driver);
+libxlDriverUnlock(libxl_driver);
+}
+
static int
libxlStateReload(void)
{
@@ -4887,6 +4896,7 @@ static virDriver libxlDriver = {
static virStateDriver libxlStateDriver = {
.name = LIBXL,
.stateInitialize = libxlStateInitialize,
+.stateAutoStart = libxlStateAutoStart,
.stateCleanup = libxlStateCleanup,
.stateReload = libxlStateReload,
};
diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
index 21cf2e3..bafbe93 100644
--- a/src/lxc/lxc_driver.c
+++ b/src/lxc/lxc_driver.c
@@ -1455,8 +1455,6 @@ static int lxcStateInitialize(bool privileged,
NULL, NULL) 0)
goto cleanup;
-virLXCProcessAutostartAll(lxc_driver);
-
virNWFilterRegisterCallbackDriver(lxcCallbackDriver);
return 0;
@@ -1466,6 +1464,19 @@ cleanup:
return -1;
}
+/**
+ * lxcStateAutoStart:
+ *
+ * Function to autostart the LXC daemons
+ */
+static void lxcStateAutoStart(void)
+{
+if (!lxc_driver)
+return;
+
+virLXCProcessAutostartAll(lxc_driver);
+}
+
static void lxcNotifyLoadDomain(virDomainObjPtr vm, int newVM, void *opaque)
{
virLXCDriverPtr driver = opaque;
@@ -4665,6 +4676,7 @@ static virDriver lxcDriver = {
static virStateDriver lxcStateDriver = {
.name = LXC_DRIVER_NAME,
.stateInitialize = lxcStateInitialize,
+.stateAutoStart = lxcStateAutoStart,
.stateCleanup = lxcStateCleanup,
.stateReload = lxcStateReload,
};
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index 0bb57ea..ea09663 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -430,7 +430,6 @@ networkStateInitialize(bool privileged,
networkFindActiveConfigs(driverState);
networkReloadFirewallRules(driverState);
networkRefreshDaemons(driverState);
-networkAutostartConfigs(driverState);
networkDriverUnlock(driverState);
@@ -474,6 +473,22 @@ error:
}
/**
+ * networkStateAutoStart:
+ *
+ * Function to AutoStart the bridge configs
+ */
+static void
+networkStateAutoStart(void)
+{
+if (!driverState)
+return;
+
+networkDriverLock(driverState);
+networkAutostartConfigs(driverState);
+networkDriverUnlock(driverState);
+}
+
+/**
* networkStateReload:
*
* Function to restart the QEmu daemon, it will recheck the configuration
@@ -3693,6 +3708,7 @@ static virNetworkDriver networkDriver = {
static virStateDriver networkStateDriver = {
.name = Network,
.stateInitialize = networkStateInitialize,
+.stateAutoStart = networkStateAutoStart,
.stateCleanup = networkStateCleanup,
.stateReload = networkStateReload,
};
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 2adf6e2..5634abf 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -837,8 +837,6 @@ qemuStateInitialize(bool privileged,
if (!qemu_driver-workerPool)
goto error;
-qemuAutostartDomains(qemu_driver);
-
if (conn)
virConnectClose(conn);
@@ -855,6 +853,20 @@ error:
return -1;
}
+/**
+ * qemuStateAutoStart:
+ *
+ * Function to auto start the QEmu daemons
+ */
+static void
+qemuStateAutoStart(void)
+{
+if (!qemu_driver)
+return;
+
+qemuAutostartDomains(qemu_driver);
+}
+
static void qemuNotifyLoadDomain(virDomainObjPtr vm, int newVM, void *opaque)
{
virQEMUDriverPtr driver = opaque;
@@ -16276,6 +16288,7 @@ static virDriver qemuDriver = {
static virStateDriver qemuStateDriver = {
.name = QEMU,
.stateInitialize = qemuStateInitialize,
+.stateAutoStart = qemuStateAutoStart,
.stateCleanup = qemuStateCleanup,
.stateReload =
[libvirt] [PATCH v2 1/2] Separate out StateAutoStart from StateInitialize
Separation allows for dependent drivers to be make a connection during
the AutoStart phase of state initialization.
---
src/driver.h | 4
src/libvirt.c | 14 +-
2 files changed, 17 insertions(+), 1 deletion(-)
diff --git a/src/driver.h b/src/driver.h
index cc03e9f..be64333 100644
--- a/src/driver.h
+++ b/src/driver.h
@@ -1800,6 +1800,9 @@ typedef int
virStateInhibitCallback callback,
void *opaque);
+typedef void
+(*virDrvStateAutoStart)(void);
+
typedef int
(*virDrvStateCleanup)(void);
@@ -1815,6 +1818,7 @@ typedef virStateDriver *virStateDriverPtr;
struct _virStateDriver {
const char *name;
virDrvStateInitialize stateInitialize;
+virDrvStateAutoStart stateAutoStart;
virDrvStateCleanup stateCleanup;
virDrvStateReload stateReload;
virDrvStateStop stateStop;
diff --git a/src/libvirt.c b/src/libvirt.c
index 444c1c3..8157488 100644
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -808,7 +808,11 @@ virRegisterStateDriver(virStateDriverPtr driver)
* @callback: callback to invoke to inhibit shutdown of the daemon
* @opaque: data to pass to @callback
*
- * Initialize all virtualization drivers.
+ * Initialize all virtualization drivers. Accomplished in two phases,
+ * the first being state and structure initialization followed by any
+ * auto start supported by the driver. This is done to ensure dependencies
+ * that some drivers may have on another driver having been initialized
+ * will exist, such as the storage driver's need to use the secret driver.
*
* Returns 0 if all succeed, -1 upon any failure.
*/
@@ -836,6 +840,14 @@ int virStateInitialize(bool privileged,
}
}
}
+
+for (i = 0; i virStateDriverTabCount; i++) {
+if (virStateDriverTab[i]-stateAutoStart) {
+VIR_DEBUG(Running global auto start for %s state driver,
+ virStateDriverTab[i]-name);
+virStateDriverTab[i]-stateAutoStart();
+}
+}
return 0;
}
--
1.8.1.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/4] Fix a pair of crashes in DNS XML parsing
On 07/26/2013 01:18 PM, Daniel P. Berrange wrote: On Fri, Jul 26, 2013 at 12:28:20PM +0200, Ján Tomko wrote: Ján Tomko (4): Set the number of elements to 0 in virNetwork*Clear Don't check validity of missing attributes in DNS SRV XML Remove double space in error messages Remove redundant free in virNetworkDNSHostDefParseXML src/conf/network_conf.c | 14 +- 1 file changed, 9 insertions(+), 5 deletions(-) IMHO you should be including one or more test data files to demonstrate the crash fix. That can't be done with just data files, a new test for network XML sections is needed. virNetworkDefUpdateSection seems like the right function to test. Jan -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Resolve Coverity complaint in storagevolxml2argvtest
On 07/26/2013 01:42 PM, John Ferlan wrote: On 07/26/2013 07:27 AM, Ján Tomko wrote: Ignore NULL pool in testSetVolumeType to silence Coverity, even though we only call it with NULL pool when vol is also NULL. (13) Event var_deref_model: Passing null pointer inputpool to function testSetVolumeType(virStorageVolDefPtr, virStoragePoolDefPtr), which dereferences it. [details] Also see events: [assign_zero] 95testSetVolumeType(inputvol, inputpool); --- tests/storagevolxml2argvtest.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) ACK. I ran the coverity analysis and it's happy. Yay! Pushed. Jan -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] build: avoid -lgcrypt with newer gnutls
On Thu, Jul 25, 2013 at 04:13:28PM -0600, Eric Blake wrote: https://bugzilla.redhat.com/show_bug.cgi?id=951637 Newer gnutls uses nettle, rather than gcrypt, which is a lot nicer regarding initialization. Yet we were unconditionally initializing gcrypt even when gnutls wouldn't be using it, and having two crypto libraries linked into libvirt.so is pointless. The ldd probe in configure borrows from our libnl-1 vs. libnl-3 code. * configure.ac (WITH_GNUTLS): Probe whether to add -lgcrypt, and define a witness WITH_GNUTLS_GCRYPT. * src/libvirt.c (virTLSMutexInit, virTLSMutexDestroy) (virTLSMutexLock, virTLSMutexUnlock, virTLSThreadImpl) (virGlobalInit): Honor the witness. * libvirt.spec.in (BuildRequires): Make gcrypt usage conditional, no longer needed in Fedora 19. Signed-off-by: Eric Blake ebl...@redhat.com --- Tested with 'ldd src/.libs/libvirt.so | grep -E (gcry|net|tls)': - on RHEL 6.4 and Fedora 18, pre- and post-patch remain unchanged (use of just libgnutls/libgcrypt) - on Fedora 19, pre-patch linked against libgnutls, libgcrypt, and libnettle, post-patch linked against just libgnutls and libnettle This should probably go in for 1.1.1, but it's not a build-breaker so it needs review. configure.ac| 27 +-- libvirt.spec.in | 2 ++ src/libvirt.c | 10 ++ 3 files changed, 29 insertions(+), 10 deletions(-) diff --git a/configure.ac b/configure.ac index cc9942a..de209e2 100644 --- a/configure.ac +++ b/configure.ac @@ -1098,13 +1098,28 @@ if test x$with_gnutls != xno; then AC_MSG_ERROR([You must install the GnuTLS library in order to compile and run libvirt]) fi else -dnl Not all versions of gnutls include -lgcrypt, and so we add -dnl it explicitly for the calls to gcry_control/check_version -GNUTLS_LIBS=$GNUTLS_LIBS -lgcrypt -dnl We're not using gcrypt deprecated features so define -dnl GCRYPT_NO_DEPRECATED to avoid deprecated warnings -GNUTLS_CFLAGS=$GNUTLS_CFLAGS -DGCRYPT_NO_DEPRECATED +dnl If gnutls linked against -lgcrypt, then we must initialize gcrypt +dnl prior to using gnutls. Newer versions of gnutls use -lnettle, in +dnl which case we don't want to drag in gcrypt ourselves. +gnutls_ldd= +for dir in /usr/lib64 /usr/lib /usr/lib/*-linux-gnu*; do +if test -f $dir/libgnutls.so; then +gnutls_ldd=`(ldd $dir/libgnutls.so) 21` +break +fi +done Not sure this approach to finding libgnutls.so is going to work reliably. eg, we allow --with-gnutls=/some/dir to point to say /usr/local, or /opt/gnutls. Also with pkg-config, the library can be located basically anywhere in the filesystem Gnutls had a hard cutover point from gcrypt to nettle in the 3.0.0 release. So could we just check the GNUTLS_VERSION_MAJOR value = 3 in the header ? Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] build: avoid -lgcrypt with newer gnutls
On Fri, Jul 26, 2013 at 01:26:56PM +0100, Daniel P. Berrange wrote: On Thu, Jul 25, 2013 at 04:13:28PM -0600, Eric Blake wrote: https://bugzilla.redhat.com/show_bug.cgi?id=951637 Newer gnutls uses nettle, rather than gcrypt, which is a lot nicer regarding initialization. Yet we were unconditionally initializing gcrypt even when gnutls wouldn't be using it, and having two crypto libraries linked into libvirt.so is pointless. The ldd probe in configure borrows from our libnl-1 vs. libnl-3 code. * configure.ac (WITH_GNUTLS): Probe whether to add -lgcrypt, and define a witness WITH_GNUTLS_GCRYPT. * src/libvirt.c (virTLSMutexInit, virTLSMutexDestroy) (virTLSMutexLock, virTLSMutexUnlock, virTLSThreadImpl) (virGlobalInit): Honor the witness. * libvirt.spec.in (BuildRequires): Make gcrypt usage conditional, no longer needed in Fedora 19. Signed-off-by: Eric Blake ebl...@redhat.com --- Tested with 'ldd src/.libs/libvirt.so | grep -E (gcry|net|tls)': - on RHEL 6.4 and Fedora 18, pre- and post-patch remain unchanged (use of just libgnutls/libgcrypt) - on Fedora 19, pre-patch linked against libgnutls, libgcrypt, and libnettle, post-patch linked against just libgnutls and libnettle This should probably go in for 1.1.1, but it's not a build-breaker so it needs review. configure.ac| 27 +-- libvirt.spec.in | 2 ++ src/libvirt.c | 10 ++ 3 files changed, 29 insertions(+), 10 deletions(-) diff --git a/configure.ac b/configure.ac index cc9942a..de209e2 100644 --- a/configure.ac +++ b/configure.ac @@ -1098,13 +1098,28 @@ if test x$with_gnutls != xno; then AC_MSG_ERROR([You must install the GnuTLS library in order to compile and run libvirt]) fi else -dnl Not all versions of gnutls include -lgcrypt, and so we add -dnl it explicitly for the calls to gcry_control/check_version -GNUTLS_LIBS=$GNUTLS_LIBS -lgcrypt -dnl We're not using gcrypt deprecated features so define -dnl GCRYPT_NO_DEPRECATED to avoid deprecated warnings -GNUTLS_CFLAGS=$GNUTLS_CFLAGS -DGCRYPT_NO_DEPRECATED +dnl If gnutls linked against -lgcrypt, then we must initialize gcrypt +dnl prior to using gnutls. Newer versions of gnutls use -lnettle, in +dnl which case we don't want to drag in gcrypt ourselves. +gnutls_ldd= +for dir in /usr/lib64 /usr/lib /usr/lib/*-linux-gnu*; do +if test -f $dir/libgnutls.so; then +gnutls_ldd=`(ldd $dir/libgnutls.so) 21` +break +fi +done Not sure this approach to finding libgnutls.so is going to work reliably. eg, we allow --with-gnutls=/some/dir to point to say /usr/local, or /opt/gnutls. Also with pkg-config, the library can be located basically anywhere in the filesystem Gnutls had a hard cutover point from gcrypt to nettle in the 3.0.0 release. So could we just check the GNUTLS_VERSION_MAJOR value = 3 in the header ? Oh, actually we don't even need todo that. We can rely on pkgconfig PKG_CHECK_MODULES(GNUTLS, gnutls = 3.0.0, [GNUTLS_FOUND=yes GNUTLS_NETTLE=0], [ GNUTLS_GCRYPT=1 PKG_CHECK_MODULES(GNUTLS, gnutls = $GNUTLS_REQUIRED, [GNUTLS_FOUND=yes], [GNUTLS_FOUND=no])]) Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/2] Split driver StateAutoStart from StateInitialization
On Fri, Jul 26, 2013 at 09:19:48AM +0200, Michal Privoznik wrote: On 25.07.2013 20:37, Daniel P. Berrange wrote: On Thu, Jul 25, 2013 at 02:32:55PM -0400, John Ferlan wrote: The post push review/comments for the chap authentication determined that trying to connect to qemu driver from within the storage auto start would not be successful, see the following and followups https://www.redhat.com/archives/libvir-list/2013-July/msg01409.html These patches will split the virStateInitialize() into two phases - the first being the bulk of the existing code and the second being just running the auto start functionality for each of the drivers that needs/support it. I realize it's probably too late for 1.1.1, but figured it'd be good to be ready when the barn door opens again. I don't know about others, but I feel the change is simple enough that I wouldn't be against including it in 1.1.1, given that it solves a real world bug we have in startup ordering. Agreed. ACK then, John please push :-) Daniel -- Daniel Veillard | Open Source and Standards, Red Hat veill...@redhat.com | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | virtualization library http://libvirt.org/ -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 0/3] ensure diskchain is not broken on guest bootup
v1-v2: Add a helper function qemuDiskChainCheckBroken to check diskchain after invoking qemuDomainDetermineDiskChain() This patchset try to check whether the diskchain is broken or not when domain boot up. If dischain is broken, report like: virsh start rhel6qcow2 error: Failed to start domain rhel6qcow2 error: invalid argument: Backing file '/var/lib/libvirt/images/thirddisk' of \ image '/var/lib/libvirt/images/thirddisk.snap3' is missing. For storage pool, it still can list volumes out even if their diskchain is broken Guannan Ren(3) qemu: refactor qemuDomainCheckDiskPresence for only disk presence check qemu: add helper functions for diskchain checking qemu: check presence of each disk and its backing file as well src/libvirt_private.syms | 1 + src/qemu/qemu_domain.c| 133 +- src/qemu/qemu_domain.h| 3 ++ src/qemu/qemu_process.c | 6 src/util/virstoragefile.c | 47 +++ src/util/virstoragefile.h | 2 ++ 6 files changed, 141 insertions(+), 51 deletions(-) -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 1/3] qemu: refactor qemuDomainCheckDiskPresence for only disk presence check
Refactor this function to make it focus on disk presence checking,
including diskchain checking, and not only for CDROM and Floppy.
This change is good for the following patches.
---
src/qemu/qemu_domain.c | 98 +-
1 file changed, 58 insertions(+), 40 deletions(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index da3b768..03a2aa6 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -2026,6 +2026,61 @@ cleanup:
virObjectUnref(cfg);
}
+static int
+qemuDomainCheckDiskStartupPolicy(virQEMUDriverPtr driver,
+ virDomainObjPtr vm,
+ virDomainDiskDefPtr disk,
+ bool cold_boot)
+{
+char uuid[VIR_UUID_STRING_BUFLEN];
+virDomainEventPtr event = NULL;
+int startupPolicy = disk-startupPolicy;
+
+virUUIDFormat(vm-def-uuid, uuid);
+
+switch ((enum virDomainStartupPolicy) startupPolicy) {
+case VIR_DOMAIN_STARTUP_POLICY_OPTIONAL:
+break;
+
+case VIR_DOMAIN_STARTUP_POLICY_MANDATORY:
+virReportSystemError(errno,
+ _(cannot access file '%s'),
+ disk-src);
+goto error;
+break;
+
+case VIR_DOMAIN_STARTUP_POLICY_REQUISITE:
+if (cold_boot) {
+virReportSystemError(errno,
+ _(cannot access file '%s'),
+ disk-src);
+goto error;
+}
+break;
+
+case VIR_DOMAIN_STARTUP_POLICY_DEFAULT:
+case VIR_DOMAIN_STARTUP_POLICY_LAST:
+/* this should never happen */
+break;
+}
+
+VIR_DEBUG(Dropping disk '%s' on domain '%s' (UUID '%s')
+ due to inaccessible source '%s',
+ disk-dst, vm-def-name, uuid, disk-src);
+
+event = virDomainEventDiskChangeNewFromObj(vm, disk-src, NULL,
disk-info.alias,
+
VIR_DOMAIN_EVENT_DISK_CHANGE_MISSING_ON_START);
+if (event)
+qemuDomainEventQueue(driver, event);
+
+VIR_FREE(disk-src);
+
+return 0;
+
+error:
+return -1;
+}
+
int
qemuDomainCheckDiskPresence(virQEMUDriverPtr driver,
virDomainObjPtr vm,
@@ -2034,12 +2089,8 @@ qemuDomainCheckDiskPresence(virQEMUDriverPtr driver,
int ret = -1;
size_t i;
virDomainDiskDefPtr disk;
-char uuid[VIR_UUID_STRING_BUFLEN];
-virDomainEventPtr event = NULL;
virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
-virUUIDFormat(vm-def-uuid, uuid);
-
for (i = 0; i vm-def-ndisks; i++) {
disk = vm-def-disks[i];
@@ -2053,42 +2104,9 @@ qemuDomainCheckDiskPresence(virQEMUDriverPtr driver,
continue;
}
-switch ((enum virDomainStartupPolicy) disk-startupPolicy) {
-case VIR_DOMAIN_STARTUP_POLICY_OPTIONAL:
-break;
-
-case VIR_DOMAIN_STARTUP_POLICY_MANDATORY:
-virReportSystemError(errno,
- _(cannot access file '%s'),
- disk-src);
-goto cleanup;
-break;
-
-case VIR_DOMAIN_STARTUP_POLICY_REQUISITE:
-if (cold_boot) {
-virReportSystemError(errno,
- _(cannot access file '%s'),
- disk-src);
-goto cleanup;
-}
-break;
-
-case VIR_DOMAIN_STARTUP_POLICY_DEFAULT:
-case VIR_DOMAIN_STARTUP_POLICY_LAST:
-/* this should never happen */
-break;
-}
-
-VIR_DEBUG(Dropping disk '%s' on domain '%s' (UUID '%s')
- due to inaccessible source '%s',
- disk-dst, vm-def-name, uuid, disk-src);
-
-event = virDomainEventDiskChangeNewFromObj(vm, disk-src, NULL,
disk-info.alias,
-
VIR_DOMAIN_EVENT_DISK_CHANGE_MISSING_ON_START);
-if (event)
-qemuDomainEventQueue(driver, event);
-
-VIR_FREE(disk-src);
+if (qemuDomainCheckDiskStartupPolicy(driver, vm, disk,
+ cold_boot) 0)
+goto cleanup;
}
ret = 0;
--
1.8.3.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 3/3] qemu: check presence of each disk and its backing file as well
For disk with startupPolicy support, such as cdrom and floppy
when its chain is broken, the startup policy will apply,
otherwise, report an error on chain issue.
---
src/qemu/qemu_domain.c | 21 -
src/qemu/qemu_process.c | 6 --
2 files changed, 12 insertions(+), 15 deletions(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index be77991..b607454 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -2091,22 +2091,25 @@ qemuDomainCheckDiskPresence(virQEMUDriverPtr driver,
virDomainDiskDefPtr disk;
virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
+VIR_DEBUG(Checking for disk presence);
for (i = 0; i vm-def-ndisks; i++) {
disk = vm-def-disks[i];
-if (!disk-startupPolicy || !disk-src)
+if (!disk-src)
continue;
-if (virFileAccessibleAs(disk-src, F_OK,
-cfg-user,
-cfg-group) = 0) {
-/* disk accessible */
-continue;
+if (qemuDomainDetermineDiskChain(driver, disk, false) = 0)
+if (qemuDiskChainCheckBroken(disk) = 0)
+continue;
+
+if (disk-startupPolicy) {
+virResetLastError();
+if (qemuDomainCheckDiskStartupPolicy(driver, vm, disk,
+ cold_boot) = 0)
+continue;
}
-if (qemuDomainCheckDiskStartupPolicy(driver, vm, disk,
- cold_boot) 0)
-goto cleanup;
+goto cleanup;
}
ret = 0;
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index e8e459e..61a897c 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -3621,16 +3621,10 @@ int qemuProcessStart(virConnectPtr conn,
if (qemuAssignDeviceAliases(vm-def, priv-qemuCaps) 0)
goto cleanup;
-VIR_DEBUG(Checking for CDROM and floppy presence);
if (qemuDomainCheckDiskPresence(driver, vm,
flags VIR_QEMU_PROCESS_START_COLD) 0)
goto cleanup;
-for (i = 0; i vm-def-ndisks; i++) {
-if (qemuDomainDetermineDiskChain(driver, vm-def-disks[i],
- false) 0)
-goto cleanup;
-}
/* Get the advisory nodeset from numad if 'placement' of
* either vcpu or numatune is 'auto'.
--
1.8.3.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 2/3] qemu: add helper functions for diskchain checking
*src/util/virstoragefile.c: Add a helper function to get
the first name of missing backing files, if the name is NULL,
it means the diskchain is not broken.
*src/qemu/qemu_domain.c: qemuDiskChainCheckBroken(disk) to
check if its chain is broken
---
src/libvirt_private.syms | 1 +
src/qemu/qemu_domain.c| 22 ++
src/qemu/qemu_domain.h| 3 +++
src/util/virstoragefile.c | 47 +++
src/util/virstoragefile.h | 2 ++
5 files changed, 75 insertions(+)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index d9615ea..f7166d6 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1882,6 +1882,7 @@ virSocketAddrSetPort;
# util/virstoragefile.h
+virStorageFileChainGetBroken;
virStorageFileChainLookup;
virStorageFileFeatureTypeFromString;
virStorageFileFeatureTypeToString;
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 03a2aa6..be77991 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -2187,6 +2187,28 @@ qemuDomainCleanupRun(virQEMUDriverPtr driver,
}
int
+qemuDiskChainCheckBroken(virDomainDiskDefPtr disk)
+{
+char *brokenFile = NULL;
+
+if (!disk-src || !disk-backingChain)
+return 0;
+
+if (virStorageFileChainGetBroken(disk-backingChain, brokenFile) 0)
+return -1;
+
+if (brokenFile) {
+virReportError(VIR_ERR_INVALID_ARG,
+ _(Backing file '%s' of image '%s' is missing.),
+ brokenFile, disk-src);
+VIR_FREE(brokenFile);
+return -1;
+}
+
+return 0;
+}
+
+int
qemuDomainDetermineDiskChain(virQEMUDriverPtr driver,
virDomainDiskDefPtr disk,
bool force)
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index 9a959d6..0a4a51e 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -347,6 +347,9 @@ bool qemuDomainJobAllowed(qemuDomainObjPrivatePtr priv,
int qemuDomainCheckDiskPresence(virQEMUDriverPtr driver,
virDomainObjPtr vm,
bool start_with_state);
+
+int qemuDiskChainCheckBroken(virDomainDiskDefPtr disk);
+
int qemuDomainDetermineDiskChain(virQEMUDriverPtr driver,
virDomainDiskDefPtr disk,
bool force);
diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
index cb6df91..e02e28a 100644
--- a/src/util/virstoragefile.c
+++ b/src/util/virstoragefile.c
@@ -572,6 +572,13 @@ virFindBackingFile(const char *start, bool start_is_dir,
const char *path,
goto cleanup;
}
+if (virFileAccessibleAs(combined, F_OK, getuid(), getgid()) 0) {
+virReportSystemError(errno,
+ _(Cannot access backing file '%s'),
+ combined);
+goto cleanup;
+}
+
if (!(*canonical = canonicalize_file_name(combined))) {
virReportSystemError(errno,
_(Can't canonicalize path '%s'), path);
@@ -1097,6 +1104,46 @@ virStorageFileGetMetadata(const char *path, int format,
}
/**
+ * virStorageFileChainCheckBroken
+ *
+ * Check whether CHAIN is broken, return the broken file name if yes,
+ * otherwise return NULL
+ */
+int
+virStorageFileChainGetBroken(virStorageFileMetadataPtr chain,
+ char **brokenFile)
+{
+virStorageFileMetadataPtr tmp;
+char *file = NULL;
+int ret = -1;
+
+if (!chain)
+return 0;
+
+*brokenFile = NULL;
+
+tmp = chain;
+while (tmp) {
+/* No backing store or backing store is not file */
+ if (!tmp-backingStoreRaw)
+ break;
+ if (!tmp-backingStore) {
+ if (VIR_STRDUP(file, tmp-backingStoreRaw) 0)
+ goto error;
+ break;
+ }
+ tmp = tmp-backingMeta;
+}
+
+*brokenFile = file;
+ret = 0;
+
+error:
+return ret;
+}
+
+
+/**
* virStorageFileFreeMetadata:
*
* Free pointers in passed structure and structure itself.
diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h
index 4cb47e6..1f89839 100644
--- a/src/util/virstoragefile.h
+++ b/src/util/virstoragefile.h
@@ -90,6 +90,8 @@ virStorageFileMetadataPtr virStorageFileGetMetadata(const
char *path,
virStorageFileMetadataPtr virStorageFileGetMetadataFromFD(const char *path,
int fd,
int format);
+int virStorageFileChainGetBroken(virStorageFileMetadataPtr chain,
+ char **broken_file);
const char *virStorageFileChainLookup(virStorageFileMetadataPtr chain,
const char *start,
--
1.8.3.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [Qemu-devel] [PATCH 4/7] qemu: Add monitor APIs to fetch CPUID data from QEMU
Am 25.07.2013 20:02, schrieb Eduardo Habkost: On Thu, Jul 25, 2013 at 04:09:18PM +0200, Andreas Färber wrote: Am 25.07.2013 16:00, schrieb Eduardo Habkost: libvirt needs a way to find out how exactly -machine foo-1.0 -cpu bar looks different from -machine foo-1.1 -cpu bar, Why? (What's the actual use case?) libvirt API allows individual CPU features to be configured, so libvirt needs to know what exactly will be the result of using a machine-type/CPU-model combination to make sure it will be exactly what was requested: http://libvirt.org/formatdomain.html#elementsCPU That's exactly what you added properties for last minute in v1.5! libvirt instantiates qemu-system-x86_64 -cpu foo,+x,+y and then checks that it got what it wanted - if not, die, otherwise continue with virtualization. One process. Also, libvirt needs to be able to check if migration to a host is possible (i.e. if all features enabled by a machine-type/CPU-model combination are supported by the host) before actually starting the migration process. That's one process on the destination with one -machine pc-i440-x.y. Is the problem possibly rather that -incoming and QMP exclude each other? Then we should fix that instead by starting incoming migration from QMP in the same process that we used to check that migration will be possible without guest-visible changes. Andreas -- SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] build: avoid -lgcrypt with newer gnutls
On 07/26/2013 06:29 AM, Daniel P. Berrange wrote: This should probably go in for 1.1.1, but it's not a build-breaker so it needs review. Glad I waited for a review. Not sure this approach to finding libgnutls.so is going to work reliably. eg, we allow --with-gnutls=/some/dir to point to say /usr/local, or /opt/gnutls. Also with pkg-config, the library can be located basically anywhere in the filesystem Gnutls had a hard cutover point from gcrypt to nettle in the 3.0.0 release. So could we just check the GNUTLS_VERSION_MAJOR value = 3 in the header ? Oh, actually we don't even need todo that. We can rely on pkgconfig Good idea; I'll post a v2. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/4] qemu: report error if disk backing files doesn't exist
On 07/25/2013 10:17 PM, Martin Kletzander wrote:
On 07/18/2013 01:32 PM, Guannan Ren wrote:
s/doesn't/don't/ in $SUBJ
Adding virFileAccessibleAs() to check if the backing file described in
disk meta exist in real path. If not, report error. The uid and gid
arguments don't take effect on F_OK mode for access, so use gid and gid
of current process.
This patch doesn't break anything new, but thanks to the
getuid()/getgid(), it leaves the previous problem in the code. Even
though F_OK doesn't need uid/gid to check whether the file exists, root
may not have permissions for upper directories on NFS storage for
example, so ...
---
src/util/virstoragefile.c | 23 +++
tests/virstoragetest.c| 16
2 files changed, 23 insertions(+), 16 deletions(-)
diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
index cb6df91..b678eb8 100644
--- a/src/util/virstoragefile.c
+++ b/src/util/virstoragefile.c
@@ -572,6 +572,13 @@ virFindBackingFile(const char *start, bool start_is_dir,
const char *path,
goto cleanup;
}
+if (virFileAccessibleAs(combined, F_OK, getuid(), getgid()) 0) {
+virReportSystemError(errno,
+ _(Backing file '%s' does not exist),
+ combined);
+goto cleanup;
+}
+
if (!(*canonical = canonicalize_file_name(combined))) {
virReportSystemError(errno,
_(Can't canonicalize path '%s'), path);
... this hunk does make the code report better errors, but in the future
it should canonicalize the filename using root/qemu/domain users.
ACK to this hunk, with the error reworded (e.g. Cannot access backing
file %s) and, of course, commit message changed appropriately, but ...
@@ -857,14 +864,10 @@ virStorageFileGetMetadataInternal(const char *path,
!!directory, backing,
meta-directory,
meta-backingStore) 0) {
-/* the backing file is (currently) unavailable, treat this
- * file as standalone:
- * backingStoreRaw is kept to mark broken image chains */
-meta-backingStoreIsFile = false;
-backingFormat = VIR_STORAGE_FILE_NONE;
-VIR_WARN(Backing file '%s' of image '%s' is missing.,
- meta-backingStoreRaw, path);
-
+VIR_FREE(backing);
+VIR_ERROR(_(Backing file '%s' of image '%s' is missing.),
+ meta-backingStoreRaw, path);
+goto cleanup;
To fix a pre-existing error, we should (instead of this change) just add
virResetLastError() here as the error is used somewhere else in the code
and should be kept in virFindBackingFile(). Having it in the logs seems
OK to me.
It makes sense, but it seems that the tests/virstoragetest.c testcase is
using the
last error in checking warning flag, see:
if (data-flags EXP_WARN) {
if (!virGetLastError()) {
fprintf(stderr, call should have warned\n);
goto cleanup;
}
virResetLastError();
It is not serious issue without calling virResetLastError() here.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [v2 PATCH] caps: use -device for primary video when qemu =1.6
https://bugzilla.redhat.com/show_bug.cgi?id=981094 The commit 0ad9025ef introduce qemu flag QEMU_CAPS_DEVICE_VIDEO_PRIMARY for using -device VGA, -device cirrus-vga, -device vmware-svga and -device qxl-vga. In use, for -device qxl-vga, mouse doesn't display in guest window like the desciption in above bug. This patch try to use -device for primary video when qemu =1.6 which contains the bug fix patch --- src/qemu/qemu_capabilities.c | 6 +++--- tests/qemuhelptest.c | 2 -- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 5dc3c9e..08406b8 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -1189,8 +1189,6 @@ virQEMUCapsComputeCmdFlags(const char *help, virQEMUCapsSet(qemuCaps, QEMU_CAPS_VNC_SHARE_POLICY); } -if (version = 1002000) -virQEMUCapsSet(qemuCaps, QEMU_CAPS_DEVICE_VIDEO_PRIMARY); return 0; } @@ -2424,7 +2422,6 @@ virQEMUCapsInitQMPBasic(virQEMUCapsPtr qemuCaps) virQEMUCapsSet(qemuCaps, QEMU_CAPS_MACHINE_OPT); virQEMUCapsSet(qemuCaps, QEMU_CAPS_DUMP_GUEST_CORE); virQEMUCapsSet(qemuCaps, QEMU_CAPS_VNC_SHARE_POLICY); -virQEMUCapsSet(qemuCaps, QEMU_CAPS_DEVICE_VIDEO_PRIMARY); } /* Capabilities that are architecture depending @@ -2597,6 +2594,9 @@ virQEMUCapsInitQMP(virQEMUCapsPtr qemuCaps, if (qemuCaps-version = 1003001) virQEMUCapsSet(qemuCaps, QEMU_CAPS_VNC_WEBSOCKET); +if (qemuCaps-version = 1006000) +virQEMUCapsSet(qemuCaps, QEMU_CAPS_DEVICE_VIDEO_PRIMARY); + if (virQEMUCapsProbeQMPCommands(qemuCaps, mon) 0) goto cleanup; if (virQEMUCapsProbeQMPEvents(qemuCaps, mon) 0) diff --git a/tests/qemuhelptest.c b/tests/qemuhelptest.c index d2fd794..d6cc04b 100644 --- a/tests/qemuhelptest.c +++ b/tests/qemuhelptest.c @@ -929,7 +929,6 @@ mymain(void) QEMU_CAPS_DEVICE_VGA, QEMU_CAPS_DEVICE_CIRRUS_VGA, QEMU_CAPS_DEVICE_VMWARE_SVGA, -QEMU_CAPS_DEVICE_VIDEO_PRIMARY, QEMU_CAPS_DEVICE_USB_SERIAL, QEMU_CAPS_DEVICE_USB_NET, QEMU_CAPS_DTB, @@ -1041,7 +1040,6 @@ mymain(void) QEMU_CAPS_DEVICE_VGA, QEMU_CAPS_DEVICE_CIRRUS_VGA, QEMU_CAPS_DEVICE_VMWARE_SVGA, -QEMU_CAPS_DEVICE_VIDEO_PRIMARY, QEMU_CAPS_DEVICE_USB_SERIAL, QEMU_CAPS_DEVICE_USB_NET, QEMU_CAPS_DTB, -- 1.8.3.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [Qemu-devel] [PATCH 4/7] qemu: Add monitor APIs to fetch CPUID data from QEMU
On Fri, Jul 26, 2013 at 02:31:24PM +0200, Andreas Färber wrote: Am 25.07.2013 20:02, schrieb Eduardo Habkost: On Thu, Jul 25, 2013 at 04:09:18PM +0200, Andreas Färber wrote: Am 25.07.2013 16:00, schrieb Eduardo Habkost: libvirt needs a way to find out how exactly -machine foo-1.0 -cpu bar looks different from -machine foo-1.1 -cpu bar, Why? (What's the actual use case?) libvirt API allows individual CPU features to be configured, so libvirt needs to know what exactly will be the result of using a machine-type/CPU-model combination to make sure it will be exactly what was requested: http://libvirt.org/formatdomain.html#elementsCPU That's exactly what you added properties for last minute in v1.5! libvirt instantiates qemu-system-x86_64 -cpu foo,+x,+y and then checks that it got what it wanted - if not, die, otherwise continue with virtualization. One process. I believe libvirt needs to know what are the results of the CPU model + machine-type combination at other moments, even before building the QEMU command-line. But I may be mistaken, so I hope the libvirt developers can clarify that. Also, libvirt needs to be able to check if migration to a host is possible (i.e. if all features enabled by a machine-type/CPU-model combination are supported by the host) before actually starting the migration process. That's one process on the destination with one -machine pc-i440-x.y. Is the problem possibly rather that -incoming and QMP exclude each other? Then we should fix that instead by starting incoming migration from QMP in the same process that we used to check that migration will be possible without guest-visible changes. I don't know the answer for that, and I also don't know if it is acceptable for libvirt to be required to execute QEMU just to find out if migration to a host is possible. I am thinking of cases where there may be dozens or hundreds of hosts available, and some management system needs to find out quickly what are the best candidates to which a large set of VMs can be migrated. -- Eduardo -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 0/2] Split driver StateAutoStart from StateInitialization
On 07/26/2013 07:50 AM, John Ferlan wrote: This is an update to change the 'int' AutoStart function to 'void' as was discussed in the initial review: https://www.redhat.com/archives/libvir-list/2013-July/msg01674.html John Ferlan (2): Separate out StateAutoStart from StateInitialize virStateDriver - Separate AutoStart from Initialize src/driver.h | 4 src/libvirt.c| 14 +- src/libxl/libxl_driver.c | 16 +--- src/lxc/lxc_driver.c | 16 ++-- src/network/bridge_driver.c | 18 +- src/qemu/qemu_driver.c | 17 +++-- src/storage/storage_driver.c | 18 +- src/uml/uml_driver.c | 17 +++-- 8 files changed, 108 insertions(+), 12 deletions(-) Since the review request was to make void functions - that's what I pushed. John -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [v2 PATCH] caps: use -device for primary video when qemu =1.6
On 07/26/2013 06:53 AM, Guannan Ren wrote: https://bugzilla.redhat.com/show_bug.cgi?id=981094 The commit 0ad9025ef introduce qemu flag QEMU_CAPS_DEVICE_VIDEO_PRIMARY for using -device VGA, -device cirrus-vga, -device vmware-svga and -device qxl-vga. In use, for -device qxl-vga, mouse doesn't display in guest window like the desciption in above bug. This patch try to use -device for primary video when qemu =1.6 which contains the bug fix patch --- src/qemu/qemu_capabilities.c | 6 +++--- tests/qemuhelptest.c | 2 -- 2 files changed, 3 insertions(+), 5 deletions(-) ACK. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] memory leak in snapshot and since at least 1.0.2?
Hi, https://bugs.launchpad.net/ubuntu/+source/libvirt/+bug/1201938 documents a memory leak we're seeing in libvirt. I've reproduced it in 1.0.2, 1.0.6, and an hourly snapshot from yesterday morning (which is built at https://launchpad.net/~serge-hallyn/+archive/libvirt-mav) To reproduce it, I define and start one domain and run virt-manager locally (just leaving it running in a vnc server). The RSS as observed by top grows over time, starting at 10M and becoming 87M in about 12 hours. When I stop the running vm, the memleak does seem to stop. Presumably having 1 domain would speed up the memleak (explaining the original bug reporter's woes) thanks, -serge -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] Set default partition in libvirtd instead of libvirt_lxc
From: Daniel P. Berrange berra...@redhat.com
By setting the default partition in libvirt_lxc it is not
visible when querying the live XML. Move setting of the
default partition into libvirtd virLXCProcessStart
Signed-off-by: Daniel P. Berrange berra...@redhat.com
---
src/lxc/lxc_cgroup.c | 14 --
src/lxc/lxc_process.c | 14 ++
2 files changed, 14 insertions(+), 14 deletions(-)
diff --git a/src/lxc/lxc_cgroup.c b/src/lxc/lxc_cgroup.c
index af91b04..0b0ca02 100644
--- a/src/lxc/lxc_cgroup.c
+++ b/src/lxc/lxc_cgroup.c
@@ -433,20 +433,6 @@ virCgroupPtr virLXCCgroupCreate(virDomainDefPtr def)
{
virCgroupPtr cgroup = NULL;
-if (!def-resource) {
-virDomainResourceDefPtr res;
-
-if (VIR_ALLOC(res) 0)
-goto cleanup;
-
-if (VIR_STRDUP(res-partition, /machine) 0) {
-VIR_FREE(res);
-goto cleanup;
-}
-
-def-resource = res;
-}
-
if (def-resource-partition[0] != '/') {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_(Resource partition '%s' must start with '/'),
diff --git a/src/lxc/lxc_process.c b/src/lxc/lxc_process.c
index 1a5686f..247e516 100644
--- a/src/lxc/lxc_process.c
+++ b/src/lxc/lxc_process.c
@@ -1007,6 +1007,20 @@ int virLXCProcessStart(virConnectPtr conn,
return -1;
}
+if (!vm-def-resource) {
+virDomainResourceDefPtr res;
+
+if (VIR_ALLOC(res) 0)
+goto cleanup;
+
+if (VIR_STRDUP(res-partition, /machine) 0) {
+VIR_FREE(res);
+goto cleanup;
+}
+
+vm-def-resource = res;
+}
+
if (virAsprintf(logfile, %s/%s.log,
cfg-logDir, vm-def-name) 0)
return -1;
--
1.8.1.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 4/4] Enable support for systemd-machined in cgroups creation
From: Daniel P. Berrange berra...@redhat.com
Make the virCgroupNewMachine method try to use systemd-machined
first. If that fails, then fallback to using the traditional
cgroup setup code path.
Signed-off-by: Daniel P. Berrange berra...@redhat.com
---
src/lxc/lxc_process.c | 10 +--
src/qemu/qemu_cgroup.c | 1 +
src/util/vircgroup.c | 187 -
src/util/vircgroup.h | 1 +
src/util/virsystemd.c | 5 +-
5 files changed, 182 insertions(+), 22 deletions(-)
diff --git a/src/lxc/lxc_process.c b/src/lxc/lxc_process.c
index 247e516..0a28305 100644
--- a/src/lxc/lxc_process.c
+++ b/src/lxc/lxc_process.c
@@ -1203,8 +1203,9 @@ int virLXCProcessStart(virConnectPtr conn,
goto cleanup;
}
-if (virCgroupNewDetectMachine(vm-def-name, lxc,
- vm-pid, -1, priv-cgroup) 0)
+if (virCgroupNewDetectMachine(vm-def-name, lxc, vm-pid,
+ vm-def-resource-partition,
+ -1, priv-cgroup) 0)
goto error;
if (!priv-cgroup) {
@@ -1411,8 +1412,9 @@ virLXCProcessReconnectDomain(virDomainObjPtr vm,
if (!(priv-monitor = virLXCProcessConnectMonitor(driver, vm)))
goto error;
-if (virCgroupNewDetectMachine(vm-def-name, lxc,
- vm-pid, -1, priv-cgroup) 0)
+if (virCgroupNewDetectMachine(vm-def-name, lxc, vm-pid,
+ vm-def-resource-partition,
+ -1, priv-cgroup) 0)
goto error;
if (!priv-cgroup) {
diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c
index 9f6b251..787ddeb 100644
--- a/src/qemu/qemu_cgroup.c
+++ b/src/qemu/qemu_cgroup.c
@@ -707,6 +707,7 @@ qemuConnectCgroup(virQEMUDriverPtr driver,
if (virCgroupNewDetectMachine(vm-def-name,
qemu,
vm-pid,
+ vm-def-resource-partition,
cfg-cgroupControllers,
priv-cgroup) 0)
goto cleanup;
diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c
index 94f6692..e857362 100644
--- a/src/util/vircgroup.c
+++ b/src/util/vircgroup.c
@@ -50,6 +50,7 @@
#include virhash.h
#include virhashcode.h
#include virstring.h
+#include virsystemd.h
#define CGROUP_MAX_VAL 512
@@ -100,11 +101,13 @@ static bool
virCgroupValidateMachineGroup(virCgroupPtr group,
const char *name,
const char *drivername,
+ const char *partition,
bool stripEmulatorSuffix)
{
size_t i;
bool valid = false;
char *partname;
+char *scopename;
if (virAsprintf(partname, %s.libvirt-%s,
name, drivername) 0)
@@ -113,6 +116,15 @@ virCgroupValidateMachineGroup(virCgroupPtr group,
if (virCgroupPartitionEscape(partname) 0)
goto cleanup;
+if (!partition)
+partition = /machine;
+
+if (!(scopename = virSystemdMakeScopeName(name, drivername, partition)))
+goto cleanup;
+
+if (virCgroupPartitionEscape(scopename) 0)
+goto cleanup;
+
for (i = 0; i VIR_CGROUP_CONTROLLER_LAST; i++) {
char *tmp;
@@ -140,9 +152,10 @@ virCgroupValidateMachineGroup(virCgroupPtr group,
tmp++;
if (STRNEQ(tmp, name)
-STRNEQ(tmp, partname)) {
-VIR_DEBUG(Name '%s' for controller '%s' does not match '%s' or
'%s',
- tmp, virCgroupControllerTypeToString(i), name, partname);
+STRNEQ(tmp, partname)
+STRNEQ(tmp, scopename)) {
+VIR_DEBUG(Name '%s' for controller '%s' does not match '%s', '%s'
or '%s',
+ tmp, virCgroupControllerTypeToString(i), name, partname,
scopename);
goto cleanup;
}
}
@@ -151,6 +164,7 @@ virCgroupValidateMachineGroup(virCgroupPtr group,
cleanup:
VIR_FREE(partname);
+VIR_FREE(scopename);
return valid;
}
@@ -1638,6 +1652,7 @@ int virCgroupNewDetect(pid_t pid ATTRIBUTE_UNUSED,
int virCgroupNewDetectMachine(const char *name,
const char *drivername,
pid_t pid,
+ const char *partition,
int controllers,
virCgroupPtr *group)
{
@@ -1647,7 +1662,7 @@ int virCgroupNewDetectMachine(const char *name,
return -1;
}
-if (!virCgroupValidateMachineGroup(*group, name, drivername, true)) {
+if (!virCgroupValidateMachineGroup(*group, name, drivername, partition,
true)) {
VIR_DEBUG(Failed to validate machine name for '%s' driver '%s',
name, drivername);
virCgroupFree(group);
@@ -1657,22 +1672,124 @@ int
[libvirt] [PATCH 0/4] Support for integrating cgroups with systemd
From: Daniel P. Berrange berra...@redhat.com This is a much changed / expanded version of my previous work to create cgroups via systemd. The difference is that this time it actually works :-) I'm not proposing this for merge until after the 1.1.1 release. Daniel P. Berrange (4): Add APIs for formatting systemd slice/scope names Add support for systemd cgroup mount Cope with races while killing processes Enable support for systemd-machined in cgroups creation src/libvirt_private.syms | 2 + src/lxc/lxc_process.c| 10 +- src/qemu/qemu_cgroup.c | 1 + src/util/vircgroup.c | 270 +-- src/util/vircgroup.h | 2 + src/util/virsystemd.c| 96 - src/util/virsystemd.h| 5 + tests/vircgrouptest.c| 9 ++ tests/virsystemdtest.c | 48 + 9 files changed, 403 insertions(+), 40 deletions(-) -- 1.8.1.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/4] Add APIs for formatting systemd slice/scope names
From: Daniel P. Berrange berra...@redhat.com
There are some interesting escaping rules to consider when dealing
with systemd slice/scope names. Thus it is helpful to have APIs
for formatting names
Signed-off-by: Daniel P. Berrange berra...@redhat.com
---
src/libvirt_private.syms | 2 ++
src/util/virsystemd.c| 91 ++--
src/util/virsystemd.h| 5 +++
tests/virsystemdtest.c | 48 +
4 files changed, 144 insertions(+), 2 deletions(-)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index d9615ea..0247a46 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1935,6 +1935,8 @@ virSysinfoSetup;
# util/virsystemd.h
virSystemdCreateMachine;
+virSystemdMakeScopeName;
+virSystemdMakeSliceName;
# util/virthread.h
diff --git a/src/util/virsystemd.c b/src/util/virsystemd.c
index 11d1153..251b846 100644
--- a/src/util/virsystemd.c
+++ b/src/util/virsystemd.c
@@ -27,9 +27,96 @@
#include viralloc.h
#include virutil.h
#include virlog.h
+#include virerror.h
#define VIR_FROM_THIS VIR_FROM_SYSTEMD
+
+static void virSystemdEscapeName(virBufferPtr buf,
+ const char *name)
+{
+static const char hextable[16] = 0123456789abcdef;
+
+#define ESCAPE(c) \
+do {\
+virBufferAddChar(buf, '\\');\
+virBufferAddChar(buf, 'x'); \
+virBufferAddChar(buf, hextable[(c 4) 15]); \
+virBufferAddChar(buf, hextable[c 15]);\
+} while (0)
+
+#define VALID_CHARS \
+0123456789\
+abcdefghijklmnopqrstuvwxyz\
+ABCDEFGHIJKLMNOPQRSTUVWXYZ\
+:-_.\\
+
+if (*name == '.') {
+ESCAPE(*name);
+name++;
+}
+
+while (*name) {
+if (*name == '/')
+virBufferAddChar(buf, '-');
+else if (*name == '-' ||
+ *name == '\\' ||
+ !strchr(VALID_CHARS, *name))
+ESCAPE(*name);
+else
+virBufferAddChar(buf, *name);
+name++;
+}
+
+#undef ESCAPE
+#undef VALID_CHARS
+}
+
+
+char *virSystemdMakeScopeName(const char *name,
+ const char *drivername,
+ const char *partition)
+{
+virBuffer buf = VIR_BUFFER_INITIALIZER;
+
+if (*partition == '/')
+partition++;
+
+virSystemdEscapeName(buf, partition);
+virBufferAddChar(buf, '-');
+virSystemdEscapeName(buf, drivername);
+virBufferAddLit(buf, \\x2d);
+virSystemdEscapeName(buf, name);
+virBufferAddLit(buf, .scope);
+
+if (virBufferError(buf)) {
+virReportOOMError();
+return NULL;
+}
+
+return virBufferContentAndReset(buf);
+}
+
+
+char *virSystemdMakeSliceName(const char *partition)
+{
+virBuffer buf = VIR_BUFFER_INITIALIZER;
+
+if (*partition == '/')
+partition++;
+
+virSystemdEscapeName(buf, partition);
+virBufferAddLit(buf, .slice);
+
+if (virBufferError(buf)) {
+virReportOOMError();
+return NULL;
+}
+
+return virBufferContentAndReset(buf);
+}
+
+
/**
* virSystemdCreateMachine:
* @name: driver unique name of the machine
@@ -75,8 +162,8 @@ int virSystemdCreateMachine(const char *name,
goto cleanup;
if (partition) {
-if (virAsprintf(slicename, %s.slice, partition) 0)
-goto cleanup;
+if (!(slicename = virSystemdMakeSliceName(partition)))
+ goto cleanup;
} else {
if (VIR_STRDUP(slicename, ) 0)
goto cleanup;
diff --git a/src/util/virsystemd.h b/src/util/virsystemd.h
index 9ca4e0b..414ae5a 100644
--- a/src/util/virsystemd.h
+++ b/src/util/virsystemd.h
@@ -24,6 +24,11 @@
# include internal.h
+char *virSystemdMakeScopeName(const char *name,
+ const char *drivername,
+ const char *slicename);
+char *virSystemdMakeSliceName(const char *partition);
+
int virSystemdCreateMachine(const char *name,
const char *drivername,
bool privileged,
diff --git a/tests/virsystemdtest.c b/tests/virsystemdtest.c
index bcf3ad3..784c45c 100644
--- a/tests/virsystemdtest.c
+++ b/tests/virsystemdtest.c
@@ -138,6 +138,38 @@ static int testCreateBadSystemd(const void *opaque
ATTRIBUTE_UNUSED)
return 0;
}
+
+struct testScopeData {
+const char *name;
+const char *partition;
+const char *expected;
+};
+
+static int
+testScopeName(const void *opaque)
+{
+const struct testScopeData *data = opaque;
+int ret = -1;
+char *actual = NULL;
+
+if (!(actual = virSystemdMakeScopeName(data-name,
+
[libvirt] [PATCH 3/4] Cope with races while killing processes
From: Daniel P. Berrange berra...@redhat.com
When systemd is involved in managing processes, it may start
killing off tearing down croups associated with the process
while we're still doing virCgroupKillPainfully. We must
explicitly check for ENOENT and treat it as if we had finished
killing processes
Signed-off-by: Daniel P. Berrange berra...@redhat.com
---
src/util/vircgroup.c | 15 ++-
1 file changed, 14 insertions(+), 1 deletion(-)
diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c
index 5ff8850..94f6692 100644
--- a/src/util/vircgroup.c
+++ b/src/util/vircgroup.c
@@ -2539,6 +2539,12 @@ static int virCgroupKillInternal(virCgroupPtr group, int
signum, virHashTablePtr
while (!done) {
done = true;
if (!(fp = fopen(keypath, r))) {
+if (errno == ENOENT) {
+VIR_DEBUG(No file %s, assuming done, keypath);
+killedAny = false;
+goto done;
+}
+
virReportSystemError(errno,
_(Failed to read %s),
keypath);
@@ -2578,6 +2584,7 @@ static int virCgroupKillInternal(virCgroupPtr group, int
signum, virHashTablePtr
}
}
+ done:
ret = killedAny ? 1 : 0;
cleanup:
@@ -2647,8 +2654,13 @@ static int virCgroupKillRecursiveInternal(virCgroupPtr
group, int signum, virHas
if (rc == 1)
killedAny = true;
-VIR_DEBUG(Iterate over children of %s, keypath);
+VIR_DEBUG(Iterate over children of %s (killedAny=%d), keypath,
killedAny);
if (!(dp = opendir(keypath))) {
+if (errno == ENOENT) {
+VIR_DEBUG(Path %s does not exist, assuming done, keypath);
+killedAny = false;
+goto done;
+}
virReportSystemError(errno,
_(Cannot open %s), keypath);
return -1;
@@ -2678,6 +2690,7 @@ static int virCgroupKillRecursiveInternal(virCgroupPtr
group, int signum, virHas
virCgroupFree(subgroup);
}
+ done:
ret = killedAny ? 1 : 0;
cleanup:
--
1.8.1.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/4] Add support for systemd cgroup mount
From: Daniel P. Berrange berra...@redhat.com
Systemd uses a named cgroup mount for tracking processes. Add
it as another type of controller, albeit one which we have to
special case in a number of places. In particular we must
never create/delete directories there, nor add tasks. Essentially
the systemd mount is to be considered read-only for libvirt.
Signed-off-by: Daniel P. Berrange berra...@redhat.com
---
src/util/vircgroup.c | 68 +++
src/util/vircgroup.h | 1 +
tests/vircgrouptest.c | 9 +++
3 files changed, 63 insertions(+), 15 deletions(-)
diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c
index 68ec953..5ff8850 100644
--- a/src/util/vircgroup.c
+++ b/src/util/vircgroup.c
@@ -57,7 +57,8 @@
VIR_ENUM_IMPL(virCgroupController, VIR_CGROUP_CONTROLLER_LAST,
cpu, cpuacct, cpuset, memory, devices,
- freezer, blkio, net_cls, perf_event);
+ freezer, blkio, net_cls, perf_event,
+ name=systemd);
typedef enum {
VIR_CGROUP_NONE = 0, /* create subdir under each cgroup if possible. */
@@ -115,6 +116,9 @@ virCgroupValidateMachineGroup(virCgroupPtr group,
for (i = 0; i VIR_CGROUP_CONTROLLER_LAST; i++) {
char *tmp;
+if (i == VIR_CGROUP_CONTROLLER_SYSTEMD)
+continue;
+
if (!group-controllers[i].placement)
continue;
@@ -320,6 +324,9 @@ static int virCgroupCopyPlacement(virCgroupPtr group,
if (!group-controllers[i].mountPoint)
continue;
+if (i == VIR_CGROUP_CONTROLLER_SYSTEMD)
+continue;
+
if (path[0] == '/') {
if (VIR_STRDUP(group-controllers[i].placement, path) 0)
return -1;
@@ -375,6 +382,8 @@ static int virCgroupDetectPlacement(virCgroupPtr group,
int ret = -1;
char *procfile;
+VIR_DEBUG(Detecting placement for pid %lld path %s,
+ (unsigned long long)pid, path);
if (pid == -1) {
if (VIR_STRDUP(procfile, /proc/self/cgroup) 0)
goto cleanup;
@@ -411,6 +420,7 @@ static int virCgroupDetectPlacement(virCgroupPtr group,
const char *typestr = virCgroupControllerTypeToString(i);
int typelen = strlen(typestr);
char *tmp = controllers;
+
while (tmp) {
char *next = strchr(tmp, ',');
int len;
@@ -427,13 +437,20 @@ static int virCgroupDetectPlacement(virCgroupPtr group,
* selfpath==/libvirt.service + path=foo -
/libvirt.service/foo
*/
if (typelen == len STREQLEN(typestr, tmp, len)
-group-controllers[i].mountPoint != NULL) {
-if (virAsprintf(group-controllers[i].placement,
-%s%s%s, selfpath,
-(STREQ(selfpath, /) ||
- STREQ(path, ) ? : /),
-path) 0)
-goto cleanup;
+group-controllers[i].mountPoint != NULL
+group-controllers[i].placement == NULL) {
+if (i == VIR_CGROUP_CONTROLLER_SYSTEMD) {
+if (VIR_STRDUP(group-controllers[i].placement,
+ selfpath) 0)
+goto cleanup;
+} else {
+if (virAsprintf(group-controllers[i].placement,
+%s%s%s, selfpath,
+(STREQ(selfpath, /) ||
+ STREQ(path, ) ? : /),
+path) 0)
+goto cleanup;
+}
}
tmp = next;
@@ -524,13 +541,16 @@ static int virCgroupDetect(virCgroupPtr group,
return -1;
}
-if (parent || path[0] == '/') {
-if (virCgroupCopyPlacement(group, path, parent) 0)
-return -1;
-} else {
-if (virCgroupDetectPlacement(group, pid, path) 0)
-return -1;
-}
+/* In some cases we can copy part of the placement info
+ * based on the parent cgroup...
+ */
+if ((parent || path[0] == '/')
+virCgroupCopyPlacement(group, path, parent) 0)
+return -1;
+
+/* ... but use /proc/cgroups to fill in the rest */
+if (virCgroupDetectPlacement(group, pid, path) 0)
+return -1;
/* Check that for every mounted controller, we found our placement */
for (i = 0; i VIR_CGROUP_CONTROLLER_LAST; i++) {
@@ -822,6 +842,12 @@ static int virCgroupMakeGroup(virCgroupPtr parent,
for (i = 0; i VIR_CGROUP_CONTROLLER_LAST; i++) {
char *path = NULL;
+/* We must never mkdir() in systemd's hierachy */
+if (i == VIR_CGROUP_CONTROLLER_SYSTEMD) {
+VIR_DEBUG(Not creating
Re: [libvirt] [PATCH] Set default partition in libvirtd instead of libvirt_lxc
On 07/26/2013 09:47 AM, Daniel P. Berrange wrote: From: Daniel P. Berrange berra...@redhat.com By setting the default partition in libvirt_lxc it is not visible when querying the live XML. Move setting of the default partition into libvirtd virLXCProcessStart Signed-off-by: Daniel P. Berrange berra...@redhat.com --- src/lxc/lxc_cgroup.c | 14 -- src/lxc/lxc_process.c | 14 ++ 2 files changed, 14 insertions(+), 14 deletions(-) ACK. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] memory leak in snapshot and since at least 1.0.2?
Quoting Serge Hallyn (serge.hal...@ubuntu.com): Hi, https://bugs.launchpad.net/ubuntu/+source/libvirt/+bug/1201938 documents a memory leak we're seeing in libvirt. I've reproduced it in 1.0.2, 1.0.6, and an hourly snapshot from yesterday morning (which is built at https://launchpad.net/~serge-hallyn/+archive/libvirt-mav) To reproduce it, I define and start one domain and run virt-manager locally (just leaving it running in a vnc server). The RSS as observed by top grows over time, starting at 10M and becoming 87M in about 12 hours. When I stop the running vm, the memleak does seem to stop. Presumably having 1 domain would speed up the memleak (explaining the original bug reporter's woes) No, the memory leak did not stop altogether with the VM stopped. Since sending that email RSS has gone up by about 2M. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [v2 PATCH] caps: use -device for primary video when qemu =1.6
On Fri, Jul 26, 2013 at 10:17 AM, Eric Blake ebl...@redhat.com wrote: On 07/26/2013 06:53 AM, Guannan Ren wrote: https://bugzilla.redhat.com/show_bug.cgi?id=981094 The commit 0ad9025ef introduce qemu flag QEMU_CAPS_DEVICE_VIDEO_PRIMARY for using -device VGA, -device cirrus-vga, -device vmware-svga and -device qxl-vga. In use, for -device qxl-vga, mouse doesn't display in guest window like the desciption in above bug. This patch try to use -device for primary video when qemu =1.6 which contains the bug fix patch --- src/qemu/qemu_capabilities.c | 6 +++--- tests/qemuhelptest.c | 2 -- 2 files changed, 3 insertions(+), 5 deletions(-) ACK. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org Please push this into v1.1.0-maint as well. -- Doug Goldstein -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Use flock() instead of fcntl()
Daniel P. Berrange wrote: On Thu, Jul 25, 2013 at 12:07:44PM +0200, David Weber wrote: SUSE advises to use indirect leases which we also want to avoid: https://www.suse.com/documentation//sled11/book_kvm/?page=/documentation//sled11/book_kvm/data/sec_libvirt_storage_locking.html virtlockd's default configuration does not allow you to lock disk files placed on a shared file system (for example NFS or OCFS2). Locking on these file systems requires you to specify a lockspace directory on the VM Host Server by setting Although that's not completely correct because NFS supports fcntl() That's just badly written explanation for that config setting. It should really be saying that the default configuration does not provide protection across multiple hosts for file paths which are not visible via a shared filesystem. eg a SAN LUN in /dev/sdNNN won't be protected in the default config. Agreed, poorly worded. I've submitted feedback to the doc team with suggested text improvement. Regards, Jim -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] virt-login-shell joins users into lxc container.
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1
On 07/26/2013 07:40 AM, Daniel P. Berrange wrote:
On Fri, Jul 26, 2013 at 07:38:31AM -0400, Daniel J Walsh wrote:
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1
I can't seem to get the error reporting to turn on, what am I doing
wrong.,
if (virInitialize() 0) { fprintf(stderr, _(Failed to initialize
libvirt)); return EXIT_FAILURE; }
if (virErrorInitialize() 0) { fprintf(stderr, _(Failed to initialize
libvirt Error Handling)); return EXIT_FAILURE; }
virSetErrorFunc(NULL, NULL);
virReportSystemError(EINVAL, %s, _(Test));
And I get no output, I thought I would get error on stderr?
You would, except that you just turned off printing to stderr by calling
virSetErrorFunc in that way.
Daniel
Am I misreading this?
* virSetErrorFunc:
* @userData: pointer to the user data provided in the handler callback
* @handler: the function to get called in case of error or NULL
*
* Set a library global error handling function, if @handler is NULL,
* it will reset to default printing on stderr. The error raised there
* are those for which no handler at the connection level could caught.
*/
Looks like setting handler to Null reset default printing on stderr?
But I am getting no output whether or not I set this.
I am attaching a hacked virt-login-shell.c which gives me no output.
-BEGIN PGP SIGNATURE-
Version: GnuPG v1.4.13 (GNU/Linux)
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/
iEYEARECAAYFAlHyozwACgkQrlYvE4MpobMjAACePralBci9M6O0wshnO1+bXXVC
a4EAn1/cfC8ng8XlLTO9DpiFetmDr9wv
=+h5o
-END PGP SIGNATURE-
/*
* virt-login-shell.c: a shell to connect to a container
*
* Copyright (C) 2013 Red Hat, Inc.
*
* This library is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public
* License as published by the Free Software Foundation; either
* version 2.1 of the License, or (at your option) any later version.
*
* This library is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
* Lesser General Public License for more details.
*
* You should have received a copy of the GNU Lesser General Public
* License along with this library. If not, see
* http://www.gnu.org/licenses/.
*
* Daniel Walsh dwa...@redhat.com
*/
#include stdio.h
#include stdlib.h
#include virerror.h
#define VIR_FROM_THIS VIR_FROM_NONE
int
main(int argc, char **argv)
{
virReportSystemError(1, %s %d %s, Test1, argc, argv[0]);
if (virInitialize() 0) {
fprintf(stderr, _(Failed to initialize libvirt Error Handling));
return EXIT_FAILURE;
}
virReportSystemError(EINVAL, %s, Test2);
if (virErrorInitialize() 0) {
fprintf(stderr, _(Failed to initialize libvirt Error Handling));
return EXIT_FAILURE;
}
virReportSystemError(EINVAL, %s, Test3);
virSetErrorFunc(NULL, NULL);
virReportSystemError(EINVAL, %s, Test4);
virSetErrorLogPriorityFunc(NULL);
virReportSystemError(EINVAL, %s, Test5);
}
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH V5] add console support in libxl
Bamvor Jian Zhang wrote:
this patch introduce the console api in libxl driver for both pv and
hvm guest. and import and update the libxlMakeChrdevStr function
which was deleted in commit dfa1e1dd.
ACK, looks good to me now. Unless there are other review comments, I'll
commit this once 1.1.1 is released.
Regards,
Jim
Signed-off-by: Bamvor Jian Zhang bjzh...@suse.com
---
Changes since V4:
1), using proper error instead of VIR_ERR_INTERNAL_ERROR.
2), treat safe as unsupported flags in libxl openConsole api.
3), some format and minor logic changes.
Changes since V3:
implicity forbit dev_name pass to libxl driver due to only one
console supported by libxl.
Changes since V2:
1), forbid parallel configure because libxl do not support it
2), only support one serial on libxl driver.
3), also remove console code in libxl driver, AFAICS serial is enough for
connecting to libxl console.
Changes since V1:
1), add virDomainOpenConsoleEnsureACL
3), remove virReportOOMErrorFull when virAsprintf fail.
4), change size_t for non-nagetive number in libxlDomainOpenConsole
size_t i;
size_t num = 0;
5), fix for make check
(1), replace virAsprintf with VIR_STRDUP in two places
(2), delete space.
src/libxl/libxl_conf.c | 104
+++
src/libxl/libxl_conf.h | 3 ++
src/libxl/libxl_driver.c | 93 ++
3 files changed, 200 insertions(+)
diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
index 5273a26..827dfdd 100644
--- a/src/libxl/libxl_conf.c
+++ b/src/libxl/libxl_conf.c
@@ -331,6 +331,92 @@ error:
}
static int
+libxlMakeChrdevStr(virDomainChrDefPtr def, char **buf)
+{
+virDomainChrSourceDef srcdef = def-source;
+const char *type = virDomainChrTypeToString(srcdef.type);
+
+if (!type) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ %s, _(unknown chrdev type));
+return -1;
+}
+
+switch (srcdef.type) {
+case VIR_DOMAIN_CHR_TYPE_NULL:
+case VIR_DOMAIN_CHR_TYPE_STDIO:
+case VIR_DOMAIN_CHR_TYPE_VC:
+case VIR_DOMAIN_CHR_TYPE_PTY:
+if (VIR_STRDUP(*buf, type) 0)
+return -1;
+break;
+
+case VIR_DOMAIN_CHR_TYPE_FILE:
+case VIR_DOMAIN_CHR_TYPE_PIPE:
+if (virAsprintf(buf, %s:%s, type, srcdef.data.file.path) 0)
+return -1;
+break;
+
+case VIR_DOMAIN_CHR_TYPE_DEV:
+if (VIR_STRDUP(*buf, srcdef.data.file.path) 0)
+return -1;
+break;
+
+case VIR_DOMAIN_CHR_TYPE_UDP: {
+const char *connectHost = srcdef.data.udp.connectHost;
+const char *bindHost = srcdef.data.udp.bindHost;
+const char *bindService = srcdef.data.udp.bindService;
+
+if (connectHost == NULL)
+connectHost = ;
+if (bindHost == NULL)
+bindHost = ;
+if (bindService == NULL)
+bindService = 0;
+
+if (virAsprintf(buf, udp:%s:%s@%s:%s,
+connectHost,
+srcdef.data.udp.connectService,
+bindHost,
+bindService) 0)
+return -1;
+break;
+}
+
+case VIR_DOMAIN_CHR_TYPE_TCP: {
+const char *prefix;
+
+if (srcdef.data.tcp.protocol == VIR_DOMAIN_CHR_TCP_PROTOCOL_TELNET)
+prefix = telnet;
+else
+prefix = tcp;
+
+if (virAsprintf(buf, %s:%s:%s%s,
+prefix,
+srcdef.data.tcp.host,
+srcdef.data.tcp.service,
+srcdef.data.tcp.listen ? ,server,nowait : ) 0)
+return -1;
+break;
+}
+
+case VIR_DOMAIN_CHR_TYPE_UNIX:
+if (virAsprintf(buf, unix:%s%s,
+srcdef.data.nix.path,
+srcdef.data.nix.listen ? ,server,nowait : ) 0)
+return -1;
+break;
+
+default:
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _(unsupported chardev '%s'), type);
+return -1;
+}
+
+return 0;
+}
+
+static int
libxlMakeDomBuildInfo(virDomainObjPtr vm, libxl_domain_config *d_config)
{
virDomainDefPtr def = vm-def;
@@ -411,6 +497,24 @@ libxlMakeDomBuildInfo(virDomainObjPtr vm,
libxl_domain_config *d_config)
if (VIR_STRDUP(b_info-u.hvm.boot, bootorder) 0)
goto error;
+if (def-nserials) {
+if (def-nserials 1) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ %s,
+ _(Only one serial device is supported by
libxl));
+goto error;
+}
+if (libxlMakeChrdevStr(def-serials[0], b_info-u.hvm.serial)
0)
+goto error;
+}
+
+
[libvirt] [PATCH] Fix probing of legacy Xen driver to not leave URI set
From: Daniel P. Berrange berra...@redhat.com
When the legacy Xen driver probes with a NULL URI, and
finds itself running on Xen, it will set conn-uri. A
little bit later though it checks to see if libxl support
exists, and if so declines the driver. This leaves the
conn-uri set to 'xen:///', so if libxl also declines
it, it prevents probing of the QEMU driver.
Once a driver has set the conn-uri, it must *never*
decline an open request. So we must move the libxl
check earlier
Signed-off-by: Daniel P. Berrange berra...@redhat.com
---
src/xen/xen_driver.c | 20 +---
1 file changed, 13 insertions(+), 7 deletions(-)
diff --git a/src/xen/xen_driver.c b/src/xen/xen_driver.c
index 39334b7..4ae38d3 100644
--- a/src/xen/xen_driver.c
+++ b/src/xen/xen_driver.c
@@ -365,6 +365,13 @@ xenUnifiedConnectOpen(virConnectPtr conn,
virConnectAuthPtr auth, unsigned int f
if (!xenUnifiedProbe())
return VIR_DRV_OPEN_DECLINED;
+#ifdef WITH_LIBXL
+/* Decline xen:// URI if xend is not running and libxenlight
+ * driver is potentially available. */
+if (!xenUnifiedXendProbe())
+return VIR_DRV_OPEN_DECLINED;
+#endif
+
if (!(conn-uri = virURIParse(xen:///)))
return VIR_DRV_OPEN_ERROR;
} else {
@@ -374,6 +381,12 @@ xenUnifiedConnectOpen(virConnectPtr conn,
virConnectAuthPtr auth, unsigned int f
STRCASENEQ(conn-uri-scheme, http))
return VIR_DRV_OPEN_DECLINED;
+#ifdef WITH_LIBXL
+/* Decline xen:// URI if xend is not running and libxenlight
+ * driver is potentially available. */
+if (!xenUnifiedXendProbe())
+return VIR_DRV_OPEN_DECLINED;
+#endif
/* Return an error if the path isn't '' or '/' */
if (conn-uri-path
@@ -395,13 +408,6 @@ xenUnifiedConnectOpen(virConnectPtr conn,
virConnectAuthPtr auth, unsigned int f
}
}
-#ifdef WITH_LIBXL
-/* Decline xen:// URI if xend is not running and libxenlight
- * driver is potentially available. */
-if (!xenUnifiedXendProbe())
-return VIR_DRV_OPEN_DECLINED;
-#endif
-
/* We now know the URI is definitely for this driver, so beyond
* here, don't return DECLINED, always use ERROR */
--
1.8.3.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/3] qemu: handle new 'setup' migration state
From: Michael R. Hines mrhi...@us.ibm.com
Previously, QEMU's 'setup' state was no a formal state in their
state machine, but it is now. This state is used by RDMA to optionally
perform memory pinning. This state is now exposed over the monitor
and also measured in the migration info status.
This patch consumes both the new setup state as well as the timestamp
of the total time spent in that state as reported by QEMU.
RDMA migrations perform an optional 'pin-all' operation du
Signed-off-by: Michael R. Hines mrhi...@us.ibm.com
---
include/libvirt/libvirt.h.in |2 ++
src/qemu/qemu_migration.c|6 ++
src/qemu/qemu_monitor.c |2 +-
src/qemu/qemu_monitor.h | 11 +++
src/qemu/qemu_monitor_json.c | 18 ++
5 files changed, 38 insertions(+), 1 deletion(-)
diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
index c0eb25b..31fb37e 100644
--- a/include/libvirt/libvirt.h.in
+++ b/include/libvirt/libvirt.h.in
@@ -4048,6 +4048,8 @@ struct _virDomainJobInfo {
/* Time is measured in mill-seconds */
unsigned long long timeElapsed;/* Always set */
unsigned long long timeRemaining; /* Only for VIR_DOMAIN_JOB_BOUNDED */
+unsigned long long setupTime; /* length of the SETUP phase */
+double mbps; /* Migration throughput in Mbps */
/* Data is measured in bytes unless otherwise specified
* and is measuring the job as a whole
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index 3dedfe8..19001b9 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -1667,6 +1667,12 @@ qemuMigrationUpdateJobStatus(virQEMUDriverPtr driver,
priv-job.status.ram_transferred +
priv-job.status.disk_transferred;
+priv-job.info.mbps = priv-job.status.mbps;
+priv-job.info.setupTime = priv-job.status.setup_time;
+
+ret = 0;
+break;
+case QEMU_MONITOR_MIGRATION_STATUS_SETUP:
ret = 0;
break;
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index 0b73411..86aed75 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -108,7 +108,7 @@ VIR_ONCE_GLOBAL_INIT(qemuMonitor)
VIR_ENUM_IMPL(qemuMonitorMigrationStatus,
QEMU_MONITOR_MIGRATION_STATUS_LAST,
- inactive, active, completed, failed, cancelled)
+ inactive, setup, active, completed, failed,
cancelled)
VIR_ENUM_IMPL(qemuMonitorMigrationCaps,
QEMU_MONITOR_MIGRATION_CAPS_LAST,
diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
index 4a55501..82e6ae2 100644
--- a/src/qemu/qemu_monitor.h
+++ b/src/qemu/qemu_monitor.h
@@ -348,6 +348,7 @@ int qemuMonitorSetMigrationCacheSize(qemuMonitorPtr mon,
enum {
QEMU_MONITOR_MIGRATION_STATUS_INACTIVE,
+QEMU_MONITOR_MIGRATION_STATUS_SETUP,
QEMU_MONITOR_MIGRATION_STATUS_ACTIVE,
QEMU_MONITOR_MIGRATION_STATUS_COMPLETED,
QEMU_MONITOR_MIGRATION_STATUS_ERROR,
@@ -366,6 +367,16 @@ struct _qemuMonitorMigrationStatus {
/* total or expected depending on status */
bool downtime_set;
unsigned long long downtime;
+/*
+ * Duration of the QEMU 'setup' state.
+ * for RDMA, this may be on the order of several seconds
+ * if pinning support is requested before the migration begins.
+ */
+unsigned long long setup_time;
+/*
+ * Migration throughput in Mbps.
+ */
+double mbps;
unsigned long long ram_transferred;
unsigned long long ram_remaining;
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index 12f7e69..20ca05e 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -2426,6 +2426,14 @@ qemuMonitorJSONGetMigrationStatusReply(virJSONValuePtr
reply,
virJSONValueObjectGetNumberUlong(ram, normal-bytes,
status-ram_normal_bytes);
+if (virJSONValueObjectGetNumberDouble(ram, mbps,
+ status-mbps) 0) {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+ _(migration was active, but RAM 'mbps'
+ data was missing));
+return -1;
+}
+
virJSONValuePtr disk = virJSONValueObjectGet(ret, disk);
if (disk) {
rc = virJSONValueObjectGetNumberUlong(disk, transferred,
@@ -2504,6 +2512,16 @@ qemuMonitorJSONGetMigrationStatusReply(virJSONValuePtr
reply,
return -1;
}
}
+
+rc = virJSONValueObjectGetNumberUlong(ret, setup-time,
+ status-setup_time);
+
+if (rc 0) {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+ _(migration was active, but no setup-time was
set));
+return -1;
+}
+
}
return 0;
--
1.7.10.4
--
libvir-list mailing list
[libvirt] [PATCH 0/3] qemu: libvirt RDMA live migration support
From: Michael R. Hines mrhi...@us.ibm.com QEMU has in tree now planned for 1.6 support for RDMA-based live migration. Changes to libvirt: 1. QEMU has a new 'setup' phase in their state machine. 2. Expose the 'x-rdma' migration protocol URI. 3. Expose the 'x-rdma-pin-all' capability for pre-registration of memory. Michael R. Hines (3): qemu: handle new 'setup' migration state qemu: RDMA migration support using 'x-rdma' URI qemu: memory pre-pinning support for RDMA migration include/libvirt/libvirt.h.in |3 + src/qemu/qemu_capabilities.c |7 +++ src/qemu/qemu_capabilities.h |4 ++ src/qemu/qemu_command.c |8 +++ src/qemu/qemu_migration.c| 131 -- src/qemu/qemu_migration.h|3 +- src/qemu/qemu_monitor.c |7 ++- src/qemu/qemu_monitor.h | 13 + src/qemu/qemu_monitor_json.c | 18 ++ tools/virsh-domain.c |7 +++ 10 files changed, 178 insertions(+), 23 deletions(-) -- 1.7.10.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 3/3] qemu: memory pre-pinning support for RDMA migration
From: Michael R. Hines mrhi...@us.ibm.com
RDMA Live migration requires registering memory with the hardware,
and thus QEMU offers a new 'capability' which supports the ability
to pre-register / mlock() the guest memory in advance for higher
RDMA performance before the migration begins.
This patch exposes this capability with the following example usage:
virsh migrate --live --x-rdma-pin-all --migrateuri x-rdma:hostname domain
qemu+tcp://hostname/system
This capability is disabled by default, and thus ommiting it will
cause QEMU to register the memory with the hardware in an on-demand basis.
Signed-off-by: Michael R. Hines mrhi...@us.ibm.com
---
include/libvirt/libvirt.h.in |1 +
src/qemu/qemu_migration.c| 50 ++
src/qemu/qemu_migration.h|3 ++-
src/qemu/qemu_monitor.c |2 +-
src/qemu/qemu_monitor.h |1 +
tools/virsh-domain.c |7 ++
6 files changed, 62 insertions(+), 2 deletions(-)
diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
index 31fb37e..d21cb74 100644
--- a/include/libvirt/libvirt.h.in
+++ b/include/libvirt/libvirt.h.in
@@ -1193,6 +1193,7 @@ typedef enum {
VIR_MIGRATE_OFFLINE = (1 10), /* offline migrate */
VIR_MIGRATE_COMPRESSED= (1 11), /* compress data during
migration */
VIR_MIGRATE_ABORT_ON_ERROR= (1 12), /* abort migration on I/O
errors happened during migration */
+VIR_MIGRATE_X_RDMA_PIN_ALL= (1 13), /* RDMA memory pinning */
} virDomainMigrateFlags;
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index de20d23..f5d9b16 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -1574,6 +1574,46 @@ cleanup:
}
static int
+qemuMigrationSetPinAll(virQEMUDriverPtr driver,
+virDomainObjPtr vm,
+enum qemuDomainAsyncJob job)
+{
+qemuDomainObjPrivatePtr priv = vm-privateData;
+int ret;
+
+if (qemuDomainObjEnterMonitorAsync(driver, vm, job) 0)
+return -1;
+
+ret = qemuMonitorGetMigrationCapability(
+priv-mon,
+QEMU_MONITOR_MIGRATION_CAPS_X_RDMA_PIN_ALL);
+
+if (ret 0) {
+goto cleanup;
+} else if (ret == 0) {
+if (job == QEMU_ASYNC_JOB_MIGRATION_IN) {
+virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, %s,
+ _(rdma pinning migration is not supported by
+ target QEMU binary));
+} else {
+virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, %s,
+ _(rdma pinning migration is not supported by
+ source QEMU binary));
+}
+ret = -1;
+goto cleanup;
+}
+
+ret = qemuMonitorSetMigrationCapability(
+priv-mon,
+QEMU_MONITOR_MIGRATION_CAPS_X_RDMA_PIN_ALL);
+
+cleanup:
+qemuDomainObjExitMonitor(driver, vm);
+return ret;
+}
+
+static int
qemuMigrationWaitForSpice(virQEMUDriverPtr driver,
virDomainObjPtr vm)
{
@@ -2363,6 +2403,11 @@ qemuMigrationPrepareAny(virQEMUDriverPtr driver,
QEMU_ASYNC_JOB_MIGRATION_IN) 0)
goto stop;
+if (flags VIR_MIGRATE_X_RDMA_PIN_ALL
+qemuMigrationSetPinAll(driver, vm,
+QEMU_ASYNC_JOB_MIGRATION_IN) 0)
+goto stop;
+
if (mig-lockState) {
VIR_DEBUG(Received lockstate %s, mig-lockState);
VIR_FREE(priv-lockState);
@@ -3156,6 +3201,11 @@ qemuMigrationRun(virQEMUDriverPtr driver,
QEMU_ASYNC_JOB_MIGRATION_OUT) 0)
goto cleanup;
+if (flags VIR_MIGRATE_X_RDMA_PIN_ALL
+qemuMigrationSetPinAll(driver, vm,
+QEMU_ASYNC_JOB_MIGRATION_OUT) 0)
+goto cleanup;
+
if (qemuDomainObjEnterMonitorAsync(driver, vm,
QEMU_ASYNC_JOB_MIGRATION_OUT) 0)
goto cleanup;
diff --git a/src/qemu/qemu_migration.h b/src/qemu/qemu_migration.h
index 0f6c5f7..d08979f 100644
--- a/src/qemu/qemu_migration.h
+++ b/src/qemu/qemu_migration.h
@@ -39,7 +39,8 @@
VIR_MIGRATE_UNSAFE | \
VIR_MIGRATE_OFFLINE | \
VIR_MIGRATE_COMPRESSED | \
- VIR_MIGRATE_ABORT_ON_ERROR)
+ VIR_MIGRATE_ABORT_ON_ERROR | \
+ VIR_MIGRATE_X_RDMA_PIN_ALL)
/* All supported migration parameters and their types. */
# define QEMU_MIGRATION_PARAMETERS \
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index ce95174..34d6840 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -112,7 +112,7 @@ VIR_ENUM_IMPL(qemuMonitorMigrationStatus,
VIR_ENUM_IMPL(qemuMonitorMigrationCaps,
QEMU_MONITOR_MIGRATION_CAPS_LAST,
- xbzrle)
+
[libvirt] [PATCH 2/3] qemu: RDMA migration support using 'x-rdma' URI
From: Michael R. Hines mrhi...@us.ibm.com
QEMU has in tree now for version 1.6 support for RDMA Live migration.
Full documenation of the feature:
http://wiki.qemu.org/Features/RDMALiveMigration
This patch includes mainly making all the locations in libvirt where
the 'tcp' string was hard-coded to be more flexible to use more than
one protocol.
While the RDMA protocol has been extensively tested (from multiple
companies as well as virt-test), the protocol 'x-rdma' will later be
renamed to 'rdma' after the community has allowed the feature more cooking.
Example usage:
virsh migrate --live --migrateuri x-rdma:hostname domain
qemu+tcp://hostname/system
Signed-off-by: Michael R. Hines mrhi...@us.ibm.com
---
src/qemu/qemu_capabilities.c |7
src/qemu/qemu_capabilities.h |4 +++
src/qemu/qemu_command.c |8 +
src/qemu/qemu_migration.c| 75 +++---
src/qemu/qemu_monitor.c |3 +-
src/qemu/qemu_monitor.h |1 +
6 files changed, 78 insertions(+), 20 deletions(-)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 5dc3c9e..94d17c6 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -234,6 +234,8 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST,
vnc-share-policy, /* 150 */
device-del-event,
+
+ x-rdma, /* 152 */
);
struct _virQEMUCaps {
@@ -1101,6 +1103,7 @@ virQEMUCapsComputeCmdFlags(const char *help,
* -incoming unix (qemu = 0.12.0)
* -incoming fd (qemu = 0.12.0)
* -incoming stdio (all earlier kvm)
+ * -incoming x-rdma (qemu = 1.6.0)
*
* NB, there was a pre-kvm-79 'tcp' support, but it
* was broken, because it blocked the monitor console
@@ -2437,6 +2440,10 @@ virQEMUCapsInitArchQMPBasic(virQEMUCapsPtr qemuCaps,
char *archstr = NULL;
int ret = -1;
+if (qemuCaps-version = MIN_X_RDMA_VERSION) {
+virQEMUCapsSet(qemuCaps, QEMU_CAPS_MIGRATE_QEMU_X_RDMA);
+}
+
if (!(archstr = qemuMonitorGetTargetArch(mon)))
return -1;
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index f5f685d..5069552 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -191,9 +191,13 @@ enum virQEMUCapsFlags {
QEMU_CAPS_VNC_SHARE_POLICY = 150, /* set display sharing policy */
QEMU_CAPS_DEVICE_DEL_EVENT = 151, /* DEVICE_DELETED event */
+QEMU_CAPS_MIGRATE_QEMU_X_RDMA = 152, /* have qemu x-rdma migration */
+
QEMU_CAPS_LAST, /* this must always be the last item */
};
+#define MIN_X_RDMA_VERSION 1006000
+
typedef struct _virQEMUCaps virQEMUCaps;
typedef virQEMUCaps *virQEMUCapsPtr;
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index aa3a2fd..a26acd7 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -8657,6 +8657,14 @@ qemuBuildCommandLine(virConnectPtr conn,
goto error;
}
virCommandAddArg(cmd, migrateFrom);
+} else if (STRPREFIX(migrateFrom, x-rdma)) {
+if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_MIGRATE_QEMU_X_RDMA)) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ %s, _(RDMA migration is not supported with
+ this QEMU binary));
+goto error;
+}
+virCommandAddArg(cmd, migrateFrom);
} else if (STREQ(migrateFrom, stdio)) {
if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_MIGRATE_QEMU_FD)) {
virCommandAddArgFormat(cmd, fd:%d, migrateFd);
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index 19001b9..de20d23 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -2169,7 +2169,8 @@ qemuMigrationPrepareAny(virQEMUDriverPtr driver,
virDomainDefPtr *def,
virStreamPtr st,
unsigned int port,
-unsigned long flags)
+unsigned long flags,
+const char *protocol)
{
virDomainObjPtr vm = NULL;
virDomainEventPtr event = NULL;
@@ -2280,7 +2281,8 @@ qemuMigrationPrepareAny(virQEMUDriverPtr driver,
* and there is at least one IPv6 address configured
*/
if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_IPV6_MIGRATION)
-getaddrinfo(::, NULL, hints, info) == 0) {
+getaddrinfo(::, NULL, hints, info) == 0
+!strstr(protocol, rdma)) {
freeaddrinfo(info);
listenAddr = [::];
} else {
@@ -2291,7 +2293,8 @@ qemuMigrationPrepareAny(virQEMUDriverPtr driver,
/* QEMU will be started with -incoming [::]:port
* or -incoming 0.0.0.0:port
*/
-if (virAsprintf(migrateFrom, tcp:%s:%d, listenAddr, port) 0)
+if (virAsprintf(migrateFrom,
Re: [libvirt] [PATCH 0/3] qemu: libvirt RDMA live migration support
On 07/26/2013 11:47 AM, mrhi...@linux.vnet.ibm.com wrote: From: Michael R. Hines mrhi...@us.ibm.com QEMU has in tree now planned for 1.6 support for RDMA-based live migration. Changes to libvirt: 1. QEMU has a new 'setup' phase in their state machine. 2. Expose the 'x-rdma' migration protocol URI. 3. Expose the 'x-rdma-pin-all' capability for pre-registration of memory. The x- prefix means that the migration is still experimental; do we want to be codifying the use of experimental API into libvirt, or is it time for a patch to qemu to remove the x- prefix? Back in the 1.5 timeframe, when RDMA was first proposed, the x- prefix made sense, but now that we are closer to qemu 1.6, and you are trying to get libvirt to drive it, that's a declaration of stability. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/3] qemu: handle new 'setup' migration state
On Fri, Jul 26, 2013 at 13:47:43 -0400, mrhi...@linux.vnet.ibm.com wrote:
From: Michael R. Hines mrhi...@us.ibm.com
Previously, QEMU's 'setup' state was no a formal state in their
state machine, but it is now. This state is used by RDMA to optionally
perform memory pinning. This state is now exposed over the monitor
and also measured in the migration info status.
This patch consumes both the new setup state as well as the timestamp
of the total time spent in that state as reported by QEMU.
RDMA migrations perform an optional 'pin-all' operation du
Signed-off-by: Michael R. Hines mrhi...@us.ibm.com
---
include/libvirt/libvirt.h.in |2 ++
src/qemu/qemu_migration.c|6 ++
src/qemu/qemu_monitor.c |2 +-
src/qemu/qemu_monitor.h | 11 +++
src/qemu/qemu_monitor_json.c | 18 ++
5 files changed, 38 insertions(+), 1 deletion(-)
diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
index c0eb25b..31fb37e 100644
--- a/include/libvirt/libvirt.h.in
+++ b/include/libvirt/libvirt.h.in
@@ -4048,6 +4048,8 @@ struct _virDomainJobInfo {
/* Time is measured in mill-seconds */
unsigned long long timeElapsed;/* Always set */
unsigned long long timeRemaining; /* Only for VIR_DOMAIN_JOB_BOUNDED */
+unsigned long long setupTime; /* length of the SETUP phase */
+double mbps; /* Migration throughput in Mbps */
/* Data is measured in bytes unless otherwise specified
* and is measuring the job as a whole
NACK
You can't change content of existing C structures that are part or
public API. Running make syntax-check will warn you when you try to do
that. Look at virDomainGetJobStats (commits v1.0.2-239-g4dd00f4 to
v1.0.2-244-g4121a77) which can return extended migration statistics.
Jirka
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/3] qemu: libvirt RDMA live migration support
On 07/26/2013 02:16 PM, Eric Blake wrote: On 07/26/2013 11:47 AM, mrhi...@linux.vnet.ibm.com wrote: From: Michael R. Hines mrhi...@us.ibm.com QEMU has in tree now planned for 1.6 support for RDMA-based live migration. Changes to libvirt: 1. QEMU has a new 'setup' phase in their state machine. 2. Expose the 'x-rdma' migration protocol URI. 3. Expose the 'x-rdma-pin-all' capability for pre-registration of memory. The x- prefix means that the migration is still experimental; do we want to be codifying the use of experimental API into libvirt, or is it time for a patch to qemu to remove the x- prefix? Back in the 1.5 timeframe, when RDMA was first proposed, the x- prefix made sense, but now that we are closer to qemu 1.6, and you are trying to get libvirt to drive it, that's a declaration of stability. That's a good question. I'll consult with the community for guidance. It has been very extensively tested, though - by several parties, but I'll find out and be in touch. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/3] qemu: RDMA migration support using 'x-rdma' URI
On Fri, Jul 26, 2013 at 13:47:44 -0400, mrhi...@linux.vnet.ibm.com wrote:
From: Michael R. Hines mrhi...@us.ibm.com
QEMU has in tree now for version 1.6 support for RDMA Live migration.
Full documenation of the feature:
http://wiki.qemu.org/Features/RDMALiveMigration
This patch includes mainly making all the locations in libvirt where
the 'tcp' string was hard-coded to be more flexible to use more than
one protocol.
While the RDMA protocol has been extensively tested (from multiple
companies as well as virt-test), the protocol 'x-rdma' will later be
renamed to 'rdma' after the community has allowed the feature more cooking.
This does not prevent us from calling the protocol rdma right away and
possibly translating it to x-rdma. However, I don't think we actually
want to commit patches for rdma migration before QEMU changes the name
to rdma.
Example usage:
virsh migrate --live --migrateuri x-rdma:hostname domain
qemu+tcp://hostname/system
Signed-off-by: Michael R. Hines mrhi...@us.ibm.com
---
src/qemu/qemu_capabilities.c |7
src/qemu/qemu_capabilities.h |4 +++
src/qemu/qemu_command.c |8 +
src/qemu/qemu_migration.c| 75
+++---
src/qemu/qemu_monitor.c |3 +-
src/qemu/qemu_monitor.h |1 +
6 files changed, 78 insertions(+), 20 deletions(-)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 5dc3c9e..94d17c6 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -234,6 +234,8 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST,
vnc-share-policy, /* 150 */
device-del-event,
+
+ x-rdma, /* 152 */
);
struct _virQEMUCaps {
@@ -1101,6 +1103,7 @@ virQEMUCapsComputeCmdFlags(const char *help,
* -incoming unix (qemu = 0.12.0)
* -incoming fd (qemu = 0.12.0)
* -incoming stdio (all earlier kvm)
+ * -incoming x-rdma (qemu = 1.6.0)
*
* NB, there was a pre-kvm-79 'tcp' support, but it
* was broken, because it blocked the monitor console
@@ -2437,6 +2440,10 @@ virQEMUCapsInitArchQMPBasic(virQEMUCapsPtr qemuCaps,
char *archstr = NULL;
int ret = -1;
+if (qemuCaps-version = MIN_X_RDMA_VERSION) {
+virQEMUCapsSet(qemuCaps, QEMU_CAPS_MIGRATE_QEMU_X_RDMA);
+}
+
if (!(archstr = qemuMonitorGetTargetArch(mon)))
return -1;
This is wrong. First, you're adding this into a totally wrong place and
second, we need a better detection which is not based on qemu version.
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index f5f685d..5069552 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -191,9 +191,13 @@ enum virQEMUCapsFlags {
QEMU_CAPS_VNC_SHARE_POLICY = 150, /* set display sharing policy */
QEMU_CAPS_DEVICE_DEL_EVENT = 151, /* DEVICE_DELETED event */
+QEMU_CAPS_MIGRATE_QEMU_X_RDMA = 152, /* have qemu x-rdma migration */
+
QEMU_CAPS_LAST, /* this must always be the last item */
};
+#define MIN_X_RDMA_VERSION 1006000
+
typedef struct _virQEMUCaps virQEMUCaps;
typedef virQEMUCaps *virQEMUCapsPtr;
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index aa3a2fd..a26acd7 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -8657,6 +8657,14 @@ qemuBuildCommandLine(virConnectPtr conn,
goto error;
}
virCommandAddArg(cmd, migrateFrom);
+} else if (STRPREFIX(migrateFrom, x-rdma)) {
+if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_MIGRATE_QEMU_X_RDMA)) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ %s, _(RDMA migration is not supported with
+ this QEMU binary));
+goto error;
+}
+virCommandAddArg(cmd, migrateFrom);
} else if (STREQ(migrateFrom, stdio)) {
if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_MIGRATE_QEMU_FD)) {
virCommandAddArgFormat(cmd, fd:%d, migrateFd);
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index 19001b9..de20d23 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -2169,7 +2169,8 @@ qemuMigrationPrepareAny(virQEMUDriverPtr driver,
virDomainDefPtr *def,
virStreamPtr st,
unsigned int port,
-unsigned long flags)
+unsigned long flags,
+const char *protocol)
{
virDomainObjPtr vm = NULL;
virDomainEventPtr event = NULL;
@@ -2280,7 +2281,8 @@ qemuMigrationPrepareAny(virQEMUDriverPtr driver,
* and there is at least one IPv6 address configured
*/
if
Re: [libvirt] [PATCH 1/3] qemu: handle new 'setup' migration state
On 07/26/2013 11:47 AM, mrhi...@linux.vnet.ibm.com wrote:
From: Michael R. Hines mrhi...@us.ibm.com
Previously, QEMU's 'setup' state was no a formal state in their
state machine, but it is now. This state is used by RDMA to optionally
perform memory pinning. This state is now exposed over the monitor
and also measured in the migration info status.
This patch consumes both the new setup state as well as the timestamp
of the total time spent in that state as reported by QEMU.
RDMA migrations perform an optional 'pin-all' operation du
Signed-off-by: Michael R. Hines mrhi...@us.ibm.com
---
include/libvirt/libvirt.h.in |2 ++
src/qemu/qemu_migration.c|6 ++
src/qemu/qemu_monitor.c |2 +-
src/qemu/qemu_monitor.h | 11 +++
src/qemu/qemu_monitor_json.c | 18 ++
5 files changed, 38 insertions(+), 1 deletion(-)
diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
index c0eb25b..31fb37e 100644
--- a/include/libvirt/libvirt.h.in
+++ b/include/libvirt/libvirt.h.in
@@ -4048,6 +4048,8 @@ struct _virDomainJobInfo {
/* Time is measured in mill-seconds */
unsigned long long timeElapsed;/* Always set */
unsigned long long timeRemaining; /* Only for VIR_DOMAIN_JOB_BOUNDED */
+unsigned long long setupTime; /* length of the SETUP phase */
+double mbps; /* Migration throughput in Mbps */
This series adds a new feature, and we're already in freeze for 1.1.1,
so it would have to be post-freeze.
NACK; this is a change to ABI. We cannot change the existing struct
size without invalidating apps compiled against an earlier version of
the library (ie. virDomainGetJobInfo is cast in stone). Rather, we
should be adding new VIR_DOMAIN_JOB_* macros for use in the
virTypedParameters of virDomainGetJobStats().
It does point out a bug that can be fixed during freeze: we should fix
our documentation so that virDomainGetJobInfo() refers to the better
virDomainGetJobStats().
Somewhat related: I mentioned in another thread that neither
virDomainGetJob{Info,Stats} nor virDomainGetBlockJobInfo play nicely
with the idea of being able to have multiple jobs running in parallel,
which is necessary if we want to be able to support fleecing of
point-in-time snapshots from multiple clients. Ultimately, that means I
think we need to enhance the job mechanism so that new jobs return a
handle, then we add new API that can query and abort jobs based on the
handle instead of being limited to only the current job; I guess for
back-compat, it would also help to have an API that sets the current job
out of a set of jobs so that the old APIs are still usable. If we do an
upgrade of the job API, we would make sure that the new query method
sticks to virTypedParameters in the same was as virDomainGetJobStats().
--
Eric Blake eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/3] qemu: handle new 'setup' migration state
On 07/26/2013 02:17 PM, Jiri Denemark wrote:
On Fri, Jul 26, 2013 at 13:47:43 -0400, mrhi...@linux.vnet.ibm.com wrote:
From: Michael R. Hines mrhi...@us.ibm.com
Previously, QEMU's 'setup' state was no a formal state in their
state machine, but it is now. This state is used by RDMA to optionally
perform memory pinning. This state is now exposed over the monitor
and also measured in the migration info status.
This patch consumes both the new setup state as well as the timestamp
of the total time spent in that state as reported by QEMU.
RDMA migrations perform an optional 'pin-all' operation du
Signed-off-by: Michael R. Hines mrhi...@us.ibm.com
---
include/libvirt/libvirt.h.in |2 ++
src/qemu/qemu_migration.c|6 ++
src/qemu/qemu_monitor.c |2 +-
src/qemu/qemu_monitor.h | 11 +++
src/qemu/qemu_monitor_json.c | 18 ++
5 files changed, 38 insertions(+), 1 deletion(-)
diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
index c0eb25b..31fb37e 100644
--- a/include/libvirt/libvirt.h.in
+++ b/include/libvirt/libvirt.h.in
@@ -4048,6 +4048,8 @@ struct _virDomainJobInfo {
/* Time is measured in mill-seconds */
unsigned long long timeElapsed;/* Always set */
unsigned long long timeRemaining; /* Only for VIR_DOMAIN_JOB_BOUNDED */
+unsigned long long setupTime; /* length of the SETUP phase */
+double mbps; /* Migration throughput in Mbps */
/* Data is measured in bytes unless otherwise specified
* and is measuring the job as a whole
NACK
You can't change content of existing C structures that are part or
public API. Running make syntax-check will warn you when you try to do
that. Look at virDomainGetJobStats (commits v1.0.2-239-g4dd00f4 to
v1.0.2-244-g4121a77) which can return extended migration statistics.
Jirka
Acknowledged - Will re-work it.
- Michael
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/3] qemu: RDMA migration support using 'x-rdma' URI
On 07/26/2013 02:27 PM, Jiri Denemark wrote:
On Fri, Jul 26, 2013 at 13:47:44 -0400, mrhi...@linux.vnet.ibm.com wrote:
From: Michael R. Hines mrhi...@us.ibm.com
QEMU has in tree now for version 1.6 support for RDMA Live migration.
Full documenation of the feature:
http://wiki.qemu.org/Features/RDMALiveMigration
This patch includes mainly making all the locations in libvirt where
the 'tcp' string was hard-coded to be more flexible to use more than
one protocol.
While the RDMA protocol has been extensively tested (from multiple
companies as well as virt-test), the protocol 'x-rdma' will later be
renamed to 'rdma' after the community has allowed the feature more cooking.
This does not prevent us from calling the protocol rdma right away and
possibly translating it to x-rdma. However, I don't think we actually
want to commit patches for rdma migration before QEMU changes the name
to rdma.
Acknowledged.
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 5dc3c9e..94d17c6 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -234,6 +234,8 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST,
vnc-share-policy, /* 150 */
device-del-event,
+
+ x-rdma, /* 152 */
);
struct _virQEMUCaps {
@@ -1101,6 +1103,7 @@ virQEMUCapsComputeCmdFlags(const char *help,
* -incoming unix (qemu = 0.12.0)
* -incoming fd (qemu = 0.12.0)
* -incoming stdio (all earlier kvm)
+ * -incoming x-rdma (qemu = 1.6.0)
*
* NB, there was a pre-kvm-79 'tcp' support, but it
* was broken, because it blocked the monitor console
@@ -2437,6 +2440,10 @@ virQEMUCapsInitArchQMPBasic(virQEMUCapsPtr qemuCaps,
char *archstr = NULL;
int ret = -1;
+if (qemuCaps-version = MIN_X_RDMA_VERSION) {
+virQEMUCapsSet(qemuCaps, QEMU_CAPS_MIGRATE_QEMU_X_RDMA);
+}
+
if (!(archstr = qemuMonitorGetTargetArch(mon)))
return -1;
This is wrong. First, you're adding this into a totally wrong place and
second, we need a better detection which is not based on qemu version.
How would we detect without using the QEMU version?
This feature doesn't have any new command-line arguments
(except for -incoming )
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index 19001b9..de20d23 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -2169,7 +2169,8 @@ qemuMigrationPrepareAny(virQEMUDriverPtr driver,
virDomainDefPtr *def,
virStreamPtr st,
unsigned int port,
-unsigned long flags)
+unsigned long flags,
+const char *protocol)
{
virDomainObjPtr vm = NULL;
virDomainEventPtr event = NULL;
@@ -2280,7 +2281,8 @@ qemuMigrationPrepareAny(virQEMUDriverPtr driver,
* and there is at least one IPv6 address configured
*/
if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_IPV6_MIGRATION)
-getaddrinfo(::, NULL, hints, info) == 0) {
+getaddrinfo(::, NULL, hints, info) == 0
+!strstr(protocol, rdma)) {
freeaddrinfo(info);
listenAddr = [::];
} else {
Is there any reason why RDMA migration does not work over IPv6?
Laziness on my part - It was never implemented because QEMU's parsing of
the [::] brackets is hard-coded for TCP/IP.
I'll submit a patch to break this out on qemu-devel.
@@ -2291,7 +2293,8 @@ qemuMigrationPrepareAny(virQEMUDriverPtr driver,
/* QEMU will be started with -incoming [::]:port
* or -incoming 0.0.0.0:port
*/
-if (virAsprintf(migrateFrom, tcp:%s:%d, listenAddr, port) 0)
+if (virAsprintf(migrateFrom, %s:%s:%d, protocol,
+ listenAddr, port) 0)
goto cleanup;
}
@@ -2482,7 +2485,7 @@ qemuMigrationPrepareTunnel(virQEMUDriverPtr driver,
ret = qemuMigrationPrepareAny(driver, dconn, cookiein, cookieinlen,
cookieout, cookieoutlen, def,
- st, 0, flags);
+ st, 0, flags, tcp);
return ret;
}
@@ -2502,6 +2505,8 @@ qemuMigrationPrepareDirect(virQEMUDriverPtr driver,
static int port = 0;
int this_port;
char *hostname = NULL;
+const char *protocol = NULL;
+char *well_formed_protocol = NULL;
const char *p;
char *uri_str = NULL;
int ret = -1;
@@ -2550,20 +2555,29 @@ qemuMigrationPrepareDirect(virQEMUDriverPtr driver,
if (virAsprintf(uri_out, tcp:%s:%d, hostname, this_port) 0)
goto cleanup;
} else {
-/* Check the URI starts with tcp:. We will escape the
+/* Check the URI starts with a valid prefix. We will escape the
* URI when passing it
Re: [libvirt] [PATCH 1/3] qemu: handle new 'setup' migration state
On 07/26/2013 02:32 PM, Eric Blake wrote:
On 07/26/2013 11:47 AM, mrhi...@linux.vnet.ibm.com wrote:
From: Michael R. Hines mrhi...@us.ibm.com
Previously, QEMU's 'setup' state was no a formal state in their
state machine, but it is now. This state is used by RDMA to optionally
perform memory pinning. This state is now exposed over the monitor
and also measured in the migration info status.
This patch consumes both the new setup state as well as the timestamp
of the total time spent in that state as reported by QEMU.
RDMA migrations perform an optional 'pin-all' operation du
Signed-off-by: Michael R. Hines mrhi...@us.ibm.com
---
include/libvirt/libvirt.h.in |2 ++
src/qemu/qemu_migration.c|6 ++
src/qemu/qemu_monitor.c |2 +-
src/qemu/qemu_monitor.h | 11 +++
src/qemu/qemu_monitor_json.c | 18 ++
5 files changed, 38 insertions(+), 1 deletion(-)
diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
index c0eb25b..31fb37e 100644
--- a/include/libvirt/libvirt.h.in
+++ b/include/libvirt/libvirt.h.in
@@ -4048,6 +4048,8 @@ struct _virDomainJobInfo {
/* Time is measured in mill-seconds */
unsigned long long timeElapsed;/* Always set */
unsigned long long timeRemaining; /* Only for VIR_DOMAIN_JOB_BOUNDED */
+unsigned long long setupTime; /* length of the SETUP phase */
+double mbps; /* Migration throughput in Mbps */
This series adds a new feature, and we're already in freeze for 1.1.1,
so it would have to be post-freeze.
No problem - we're not in a hurry to get libvirt RDMA support
merged - just wanted to make sure we get these reviews through
so all the code plays nicely.
NACK; this is a change to ABI. We cannot change the existing struct
size without invalidating apps compiled against an earlier version of
the library (ie. virDomainGetJobInfo is cast in stone). Rather, we
should be adding new VIR_DOMAIN_JOB_* macros for use in the
virTypedParameters of virDomainGetJobStats().
Acknowledged.
It does point out a bug that can be fixed during freeze: we should fix
our documentation so that virDomainGetJobInfo() refers to the better
virDomainGetJobStats().
Somewhat related: I mentioned in another thread that neither
virDomainGetJob{Info,Stats} nor virDomainGetBlockJobInfo play nicely
with the idea of being able to have multiple jobs running in parallel,
which is necessary if we want to be able to support fleecing of
point-in-time snapshots from multiple clients. Ultimately, that means I
think we need to enhance the job mechanism so that new jobs return a
handle, then we add new API that can query and abort jobs based on the
handle instead of being limited to only the current job; I guess for
back-compat, it would also help to have an API that sets the current job
out of a set of jobs so that the old APIs are still usable. If we do an
upgrade of the job API, we would make sure that the new query method
sticks to virTypedParameters in the same was as virDomainGetJobStats().
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/3] qemu: RDMA migration support using 'x-rdma' URI
On 07/26/2013 12:48 PM, Michael R. Hines wrote:
int ret = -1;
+if (qemuCaps-version = MIN_X_RDMA_VERSION) {
+virQEMUCapsSet(qemuCaps, QEMU_CAPS_MIGRATE_QEMU_X_RDMA);
+}
+
if (!(archstr = qemuMonitorGetTargetArch(mon)))
return -1;
This is wrong. First, you're adding this into a totally wrong place and
second, we need a better detection which is not based on qemu version.
How would we detect without using the QEMU version?
Call 'query-migrate-capabilities' as part of the one-time capability
probing in qemu_capabilities.c; if [x-]rdma-pin-all is listed, then RDMA
is supported and you set the qemuCaps bit. There's also the matter of
handshaking between source and dest - we can't enable it on the source
unless the dest also supports rdma; there, the migration cookie should
be handy to pass destination support of the qemuCaps bit back to the
source, in time for the source to react.
--
Eric Blake eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Fix probing of legacy Xen driver to not leave URI set
On 07/26/2013 10:30 AM, Daniel P. Berrange wrote: From: Daniel P. Berrange berra...@redhat.com When the legacy Xen driver probes with a NULL URI, and finds itself running on Xen, it will set conn-uri. A little bit later though it checks to see if libxl support exists, and if so declines the driver. This leaves the conn-uri set to 'xen:///', so if libxl also declines it, it prevents probing of the QEMU driver. Once a driver has set the conn-uri, it must *never* decline an open request. So we must move the libxl check earlier Signed-off-by: Daniel P. Berrange berra...@redhat.com --- src/xen/xen_driver.c | 20 +--- 1 file changed, 13 insertions(+), 7 deletions(-) ACK. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/3] qemu: RDMA migration support using 'x-rdma' URI
On 07/26/2013 02:56 PM, Eric Blake wrote:
On 07/26/2013 12:48 PM, Michael R. Hines wrote:
int ret = -1;
+if (qemuCaps-version = MIN_X_RDMA_VERSION) {
+virQEMUCapsSet(qemuCaps, QEMU_CAPS_MIGRATE_QEMU_X_RDMA);
+}
+
if (!(archstr = qemuMonitorGetTargetArch(mon)))
return -1;
This is wrong. First, you're adding this into a totally wrong place and
second, we need a better detection which is not based on qemu version.
How would we detect without using the QEMU version?
Call 'query-migrate-capabilities' as part of the one-time capability
probing in qemu_capabilities.c; if [x-]rdma-pin-all is listed, then RDMA
is supported and you set the qemuCaps bit. There's also the matter of
handshaking between source and dest - we can't enable it on the source
unless the dest also supports rdma; there, the migration cookie should
be handy to pass destination support of the qemuCaps bit back to the
source, in time for the source to react.
Acknowledged.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv2] build: avoid -lgcrypt with newer gnutls
https://bugzilla.redhat.com/show_bug.cgi?id=951637
Newer gnutls uses nettle, rather than gcrypt, which is a lot nicer
regarding initialization. Yet we were unconditionally initializing
gcrypt even when gnutls wouldn't be using it, and having two crypto
libraries linked into libvirt.so is pointless.
Assume that the switch to gnutls 3.0 is a reliable witness, when
pkg-config is present; otherwise be pessimistic and use gcrypt.
* configure.ac (WITH_GNUTLS): Probe whether to add -lgcrypt, and
define a witness WITH_GNUTLS_GCRYPT.
* src/libvirt.c (virTLSMutexInit, virTLSMutexDestroy)
(virTLSMutexLock, virTLSMutexUnlock, virTLSThreadImpl)
(virGlobalInit): Honor the witness.
* libvirt.spec.in (BuildRequires): Make gcrypt usage conditional,
no longer needed in Fedora 19.
Signed-off-by: Eric Blake ebl...@redhat.com
---
v2: use second pkg-config invocation rather than ldd to determine
whether gnutls uses gcrypt
configure.ac| 27 +++
libvirt.spec.in | 2 ++
src/libvirt.c | 10 ++
3 files changed, 27 insertions(+), 12 deletions(-)
diff --git a/configure.ac b/configure.ac
index cc9942a..eb56b63 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1076,12 +1076,19 @@ if test x$with_gnutls != xno; then
LIBS=$LIBS $GNUTLS_LIBS
GNUTLS_FOUND=no
+ GNUTLS_GCRYPT=no
if test -x $PKG_CONFIG ; then
+dnl double probe, since we know that gnutls 3.0 switched to nettle instead
of
+dnl gcrypt
PKG_CHECK_MODULES(GNUTLS, gnutls = $GNUTLS_REQUIRED,
- [GNUTLS_FOUND=yes], [GNUTLS_FOUND=no])
+ [GNUTLS_FOUND=yes
+ PKG_CHECK_MODULES([GNUTLS], [gnutls = 3.0], [], [GNUTLS_GCRYPT=yes])],
+ [GNUTLS_FOUND=no])
fi
if test $GNUTLS_FOUND = no; then
+dnl pkg-config couldn't help us, assume gcrypt is necessary
fail=0
+GNUTLS_GCRYPT=yes
AC_CHECK_HEADER([gnutls/gnutls.h], [], [fail=1])
AC_CHECK_LIB([gnutls], [gnutls_handshake],[], [fail=1], [-lgcrypt])
@@ -1098,13 +1105,17 @@ if test x$with_gnutls != xno; then
AC_MSG_ERROR([You must install the GnuTLS library in order to compile
and run libvirt])
fi
else
-dnl Not all versions of gnutls include -lgcrypt, and so we add
-dnl it explicitly for the calls to gcry_control/check_version
-GNUTLS_LIBS=$GNUTLS_LIBS -lgcrypt
-
-dnl We're not using gcrypt deprecated features so define
-dnl GCRYPT_NO_DEPRECATED to avoid deprecated warnings
-GNUTLS_CFLAGS=$GNUTLS_CFLAGS -DGCRYPT_NO_DEPRECATED
+dnl If gnutls linked against -lgcrypt, then we must initialize gcrypt
+dnl prior to using gnutls. Newer versions of gnutls use -lnettle, in
+dnl which case we don't want to drag in gcrypt ourselves.
+if test $GNUTLS_GCRYPT = yes; then
+ GNUTLS_LIBS=$GNUTLS_LIBS -lgcrypt
+ dnl We're not using gcrypt deprecated features so define
+ dnl GCRYPT_NO_DEPRECATED to avoid deprecated warnings
+ GNUTLS_CFLAGS=$GNUTLS_CFLAGS -DGCRYPT_NO_DEPRECATED
+ AC_DEFINE_UNQUOTED([WITH_GNUTLS_GCRYPT], 1,
+[set to 1 if it is known or assumed that GNUTLS uses gcrypt])
+fi
dnl gnutls 3.x moved some declarations to a new header
AC_CHECK_HEADERS([gnutls/crypto.h], [], [], [[
diff --git a/libvirt.spec.in b/libvirt.spec.in
index e0e0004..4320281 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -422,7 +422,9 @@ BuildRequires: readline-devel
BuildRequires: ncurses-devel
BuildRequires: gettext
BuildRequires: libtasn1-devel
+%if (0%{?rhel} 0%{?rhel} 7) || (0%{?fedora} 0%{?fedora} 19)
BuildRequires: libgcrypt-devel
+%endif
BuildRequires: gnutls-devel
BuildRequires: libattr-devel
%if %{with_libvirtd}
diff --git a/src/libvirt.c b/src/libvirt.c
index 8157488..66e8248 100644
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -55,7 +55,9 @@
#include intprops.h
#include virconf.h
#if WITH_GNUTLS
-# include gcrypt.h
+# if WITH_GNUTLS_GCRYPT
+# include gcrypt.h
+# endif
# include rpc/virnettlscontext.h
#endif
#include vircommand.h
@@ -270,7 +272,7 @@ winsock_init(void)
#endif
-#ifdef WITH_GNUTLS
+#ifdef WITH_GNUTLS_GCRYPT
static int virTLSMutexInit(void **priv)
{
virMutexPtr lock = NULL;
@@ -323,7 +325,7 @@ static struct gcry_thread_cbs virTLSThreadImpl = {
virTLSMutexUnlock,
NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL
};
-#endif
+#endif /* WITH_GNUTLS_GCRYPT */
/* Helper macros to implement VIR_DOMAIN_DEBUG using just C99. This
* assumes you pass fewer than 15 arguments to VIR_DOMAIN_DEBUG, but
@@ -407,7 +409,7 @@ virGlobalInit(void)
virErrorInitialize() 0)
goto error;
-#ifdef WITH_GNUTLS
+#ifdef WITH_GNUTLS_GCRYPT
/*
* This sequence of API calls it copied exactly from
* gnutls 2.12.23 source lib/gcrypt/init.c, with
--
1.8.3.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2] RPC: Don't accept client if it would overcommit max_clients
On 07/26/2013 01:13 AM, Michal Privoznik wrote: Currently, even if max_client limit is hit, we accept() incoming connection request, but close it immediately. This has disadvantage of not using listen() queue. We should accept() only those clients we know we can serve and let all other wait in the (limited) queue. --- diff to v1: -moved logic from virNetServerDispatchCheck to virNetServerAddClient src/rpc/virnetserver.c | 33 - 1 file changed, 28 insertions(+), 5 deletions(-) Looks like a better location to me. You may still want to wait for Dan's review, but you have my ACK, and I think this is probably safe for 1.1.1 as a bug-fix. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] build: avoid uninitialized use warning
Otherwise, with new enough gcc compiling at -O2, the build fails with:
../../src/conf/domain_conf.c: In function ‘virDomainDeviceDefPostParse’:
../../src/conf/domain_conf.c:2821:29: error: ‘cnt’ may be used uninitialized in
this function [-Werror=maybe-uninitialized]
for (i = 0; i *cnt; i++) {
^
../../src/conf/domain_conf.c:2795:20: note: ‘cnt’ was declared here
size_t i, *cnt;
^
../../src/conf/domain_conf.c:2794:30: error: ‘arrPtr’ may be used uninitialized
in this function [-Werror=maybe-uninitialized]
virDomainChrDefPtr **arrPtr;
^
* src/conf/domain_conf.c (virDomainChrGetDomainPtrs): Always
assign into output parameters.
Signed-off-by: Eric Blake ebl...@redhat.com
---
Pushing under the build-breaker rule.
I actually found this while running ./autobuild.sh; but there,
we run libtool in both static and shared modes, and libtool, in
its infinite wisdom, assumes that if the static build succeeds,
then the shared build can have compiler output redirected to
/dev/null, which meant I was stuck debugging this output:
CC libvirt_driver_test_la-test_driver.lo
CC libvirt_driver_remote_la-remote_driver.lo
CC libvirt_driver_remote_la-remote_protocol.lo
make[3]: *** [libvirt_conf_la-domain_conf.lo] Error 1
make[3]: *** Waiting for unfinished jobs
thanks for nothing, libtool.
src/conf/domain_conf.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 0e74039..a86be8c 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -10221,6 +10221,8 @@ virDomainChrGetDomainPtrs(virDomainDefPtr vmdef,
break;
case VIR_DOMAIN_CHR_DEVICE_TYPE_LAST:
+*arrPtr = NULL;
+*cntPtr = NULL;
break;
}
}
--
1.8.3.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv2] build: avoid -lgcrypt with newer gnutls
On Fri, Jul 26, 2013 at 5:04 PM, Eric Blake ebl...@redhat.com wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=951637
Newer gnutls uses nettle, rather than gcrypt, which is a lot nicer
regarding initialization. Yet we were unconditionally initializing
gcrypt even when gnutls wouldn't be using it, and having two crypto
libraries linked into libvirt.so is pointless.
Assume that the switch to gnutls 3.0 is a reliable witness, when
pkg-config is present; otherwise be pessimistic and use gcrypt.
* configure.ac (WITH_GNUTLS): Probe whether to add -lgcrypt, and
define a witness WITH_GNUTLS_GCRYPT.
* src/libvirt.c (virTLSMutexInit, virTLSMutexDestroy)
(virTLSMutexLock, virTLSMutexUnlock, virTLSThreadImpl)
(virGlobalInit): Honor the witness.
* libvirt.spec.in (BuildRequires): Make gcrypt usage conditional,
no longer needed in Fedora 19.
Signed-off-by: Eric Blake ebl...@redhat.com
---
v2: use second pkg-config invocation rather than ldd to determine
whether gnutls uses gcrypt
configure.ac| 27 +++
libvirt.spec.in | 2 ++
src/libvirt.c | 10 ++
3 files changed, 27 insertions(+), 12 deletions(-)
diff --git a/configure.ac b/configure.ac
index cc9942a..eb56b63 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1076,12 +1076,19 @@ if test x$with_gnutls != xno; then
LIBS=$LIBS $GNUTLS_LIBS
GNUTLS_FOUND=no
+ GNUTLS_GCRYPT=no
if test -x $PKG_CONFIG ; then
+dnl double probe, since we know that gnutls 3.0 switched to nettle
instead of
+dnl gcrypt
PKG_CHECK_MODULES(GNUTLS, gnutls = $GNUTLS_REQUIRED,
- [GNUTLS_FOUND=yes], [GNUTLS_FOUND=no])
+ [GNUTLS_FOUND=yes
+ PKG_CHECK_MODULES([GNUTLS], [gnutls = 3.0], [],
[GNUTLS_GCRYPT=yes])],
+ [GNUTLS_FOUND=no])
fi
if test $GNUTLS_FOUND = no; then
+dnl pkg-config couldn't help us, assume gcrypt is necessary
fail=0
+GNUTLS_GCRYPT=yes
AC_CHECK_HEADER([gnutls/gnutls.h], [], [fail=1])
AC_CHECK_LIB([gnutls], [gnutls_handshake],[], [fail=1], [-lgcrypt])
@@ -1098,13 +1105,17 @@ if test x$with_gnutls != xno; then
AC_MSG_ERROR([You must install the GnuTLS library in order to compile
and run libvirt])
fi
else
-dnl Not all versions of gnutls include -lgcrypt, and so we add
-dnl it explicitly for the calls to gcry_control/check_version
-GNUTLS_LIBS=$GNUTLS_LIBS -lgcrypt
-
-dnl We're not using gcrypt deprecated features so define
-dnl GCRYPT_NO_DEPRECATED to avoid deprecated warnings
-GNUTLS_CFLAGS=$GNUTLS_CFLAGS -DGCRYPT_NO_DEPRECATED
+dnl If gnutls linked against -lgcrypt, then we must initialize gcrypt
+dnl prior to using gnutls. Newer versions of gnutls use -lnettle, in
+dnl which case we don't want to drag in gcrypt ourselves.
+if test $GNUTLS_GCRYPT = yes; then
+ GNUTLS_LIBS=$GNUTLS_LIBS -lgcrypt
+ dnl We're not using gcrypt deprecated features so define
+ dnl GCRYPT_NO_DEPRECATED to avoid deprecated warnings
+ GNUTLS_CFLAGS=$GNUTLS_CFLAGS -DGCRYPT_NO_DEPRECATED
+ AC_DEFINE_UNQUOTED([WITH_GNUTLS_GCRYPT], 1,
+[set to 1 if it is known or assumed that GNUTLS uses gcrypt])
+fi
dnl gnutls 3.x moved some declarations to a new header
AC_CHECK_HEADERS([gnutls/crypto.h], [], [], [[
diff --git a/libvirt.spec.in b/libvirt.spec.in
index e0e0004..4320281 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -422,7 +422,9 @@ BuildRequires: readline-devel
BuildRequires: ncurses-devel
BuildRequires: gettext
BuildRequires: libtasn1-devel
+%if (0%{?rhel} 0%{?rhel} 7) || (0%{?fedora} 0%{?fedora} 19)
BuildRequires: libgcrypt-devel
+%endif
BuildRequires: gnutls-devel
BuildRequires: libattr-devel
%if %{with_libvirtd}
diff --git a/src/libvirt.c b/src/libvirt.c
index 8157488..66e8248 100644
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -55,7 +55,9 @@
#include intprops.h
#include virconf.h
#if WITH_GNUTLS
-# include gcrypt.h
+# if WITH_GNUTLS_GCRYPT
+# include gcrypt.h
+# endif
# include rpc/virnettlscontext.h
#endif
#include vircommand.h
@@ -270,7 +272,7 @@ winsock_init(void)
#endif
-#ifdef WITH_GNUTLS
+#ifdef WITH_GNUTLS_GCRYPT
static int virTLSMutexInit(void **priv)
{
virMutexPtr lock = NULL;
@@ -323,7 +325,7 @@ static struct gcry_thread_cbs virTLSThreadImpl = {
virTLSMutexUnlock,
NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL
};
-#endif
+#endif /* WITH_GNUTLS_GCRYPT */
/* Helper macros to implement VIR_DOMAIN_DEBUG using just C99. This
* assumes you pass fewer than 15 arguments to VIR_DOMAIN_DEBUG, but
@@ -407,7 +409,7 @@ virGlobalInit(void)
virErrorInitialize() 0)
goto error;
-#ifdef WITH_GNUTLS
+#ifdef WITH_GNUTLS_GCRYPT
/*
* This sequence of API calls it copied exactly from
* gnutls 2.12.23 source lib/gcrypt/init.c, with
--
1.8.3.1
--
libvir-list mailing list
libvir-list@redhat.com
Re: [libvirt] [PATCHv2] build: avoid -lgcrypt with newer gnutls
On 07/26/2013 07:22 PM, Doug Goldstein wrote:
It appears it was an optional cutover and I guess Gentoo made the
plunge. Another idea, that you might hate would be to use pkg-config
directly and pass --static so we can get the private libraries. I'm
not running Fedora 19 yet so the best I can do is give you Fedora 18
as a comp, but that works out great since its using 2.12.23 as well.
stable Gentoo:
Name: GnuTLS
Description: Transport Security Layer implementation for the GNU system
URL: http://www.gnu.org/software/gnutls/
Version: 2.12.23
Libs: -L${libdir} -lgnutls
Libs.private: -L/usr/lib64 -lnettle -lgmp -lhogweed
Requires.private: libtasn1 , zlib
Cflags: -I${includedir}
$ pkg-config --libs --static gnutls
-lgnutls -ltasn1 -lz -lnettle -lgmp -lhogweed
Fedora 18:
Name: GnuTLS
Description: Transport Security Layer implementation for the GNU system
URL: http://www.gnu.org/software/gnutls/
Version: 2.12.23
Libs: -L${libdir} -lgnutls
Libs.private: -L/usr/lib64 -lgcrypt -L/usr/lib64 -lgpg-error
Requires.private: libtasn1 , zlib, p11-kit-1
Cflags: -I${includedir}
$ pkg-config --libs --static gnutls
-lgnutls -lgcrypt -lgpg-error -ltasn1 -lz -lp11-kit
With GnuTLS 3.2 I get the following:
pkg-config --libs --static gnutls
-lgnutls -lhogweed -lnettle -lz -lgmp
Maybe that helps?
Unfortunately, no:
Fedora 19:
$ pkg-config --libs --static gnutls
-lgnutls -lnettle -lhogweed -lgmp -lpthread -ltasn1 -lp11-kit -lz
Correct - nettle instead of gcrypt.
RHEL 6.4:
$ pkg-config --libs --static gnutls
-lgnutls -ltasn1
Ouch - no mention of gcrypt, even though this version still used gcrypt.
--
Eric Blake eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
