date:20110727

[libvirt] [libvirt-test-API][PATCH 2/2] Add four helpful functions in utils.py.

2011-07-27 Thread Guannan Ren

* get_local_hostname() get hostname on local machine
* exec_cmd() execute shell command
* remote_exec_pexpect() execture command on remote host
* scp_file() copy file to remote host
---
 utils/Python/utils.py |   69 +
 1 files changed, 69 insertions(+), 0 deletions(-)

diff --git a/utils/Python/utils.py b/utils/Python/utils.py
index eaa8eab..6787e87 100644
--- a/utils/Python/utils.py
+++ b/utils/Python/utils.py
@@ -26,6 +26,9 @@ import commands
 import socket
 import fcntl
 import struct
+import pexpect
+import string
+import subprocess
 from xml.dom import minidom
 
 class Utils(object):
@@ -58,6 +61,10 @@ class Utils(object):
 arch = ret.split( )[-2]
 return arch
 
+def get_local_hostname(self):
+ get local host name 
+return socket.gethostname()
+
 def get_libvirt_version(self, ver = ''):
 ver = commands.getoutput(rpm -q libvirt|head -1)
 if ver.split('-')[0] == 'libvirt':
@@ -364,3 +371,65 @@ class Utils(object):
 (ret, out) = commands.getstatusoutput(cmd)
 return ret
 
+def exec_cmd(self, command, sudo=False, cwd=None, infile=None, 
outfile=None, shell=False, data=None):
+
+Executes an external command, optionally via sudo.
+
+if sudo:
+if type(command) == type():
+command = sudo  + command
+else:
+command = [sudo] + command
+if infile == None:
+infile = subprocess.PIPE
+if outfile == None:
+outfile = subprocess.PIPE
+p = subprocess.Popen(command, shell=shell, close_fds=True, cwd=cwd,
+stdin=infile, stdout=outfile, stderr=subprocess.PIPE)
+(out, err) = p.communicate(data)
+if out == None:
+# Prevent splitlines() from barfing later on
+out = 
+return (p.returncode, out.splitlines())
+
+def remote_exec_pexpect(self, hostname, username, password, cmd):
+ Remote exec function via pexpect 
+user_hostname = %s@%s % (username, hostname)
+child = pexpect.spawn(/usr/bin/ssh, [user_hostname, cmd],
+  timeout = 60, maxread = 2000, logfile = None)
+while True:
+index = child.expect(['(yes\/no)', 'password:', pexpect.EOF,
+ pexpect.TIMEOUT])
+if index == 0:
+child.sendline(yes)
+elif index == 1:
+child.sendline(password)
+elif index == 2:
+child.close()
+return 0
+elif index == 3:
+child.close()
+return 1
+
+return 0
+
+def scp_file(self, hostname, username, password, target_path, file):
+ Scp file to remote host 
+user_hostname = %s@%s:%s % (username, hostname, target_path)
+child = pexpect.spawn(/usr/bin/scp, [file, user_hostname])
+while True:
+index = child.expect(['yes\/no', 'password: ',
+   pexpect.EOF,
+   pexpect.TIMEOUT])
+if index == 0:
+child.sendline(yes)
+elif index == 1:
+child.sendline(password)
+elif index == 2:
+child.close()
+return 0
+elif index == 3:
+child.close()
+return 1
+
+return 0
-- 
1.7.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [libvirt-test-API][PATCH 1/2] add remoteAccess directory and tls authentication testcase

2011-07-27 Thread Guannan Ren

---
 repos/remoteAccess/tls_setup.py |  374 +++
 1 files changed, 374 insertions(+), 0 deletions(-)
 create mode 100644 repos/remoteAccess/__init__.py
 create mode 100644 repos/remoteAccess/tls_setup.py

diff --git a/repos/remoteAccess/__init__.py b/repos/remoteAccess/__init__.py
new file mode 100644
index 000..e69de29
diff --git a/repos/remoteAccess/tls_setup.py b/repos/remoteAccess/tls_setup.py
new file mode 100644
index 000..f703ee3
--- /dev/null
+++ b/repos/remoteAccess/tls_setup.py
@@ -0,0 +1,374 @@
+#!/usr/bin/env python
+ Setup tls authentication between two hosts and configure libvirt
+to use it for connection.
+remoteAccess:tls_setup
+target_machine
+xx.xx.xx.xx
+username
+root
+password
+xx
+
+
+__author__ = 'Guannan Ren: g...@redhat.com'
+__date__ = 'Mon July 25, 2011'
+__version__ = '0.1.0'
+__credits__ = 'Copyright (C) 2011 Red Hat, Inc.'
+__all__ = ['usage']
+
+import os
+import re
+import sys
+import pexpect
+import string
+import commands
+import shutil
+
+def append_path(path):
+Append root path of package
+if path in sys.path:
+pass
+else:
+sys.path.append(path)
+
+pwd = os.getcwd()
+result = re.search('(.*)libvirt-test-API', pwd)
+append_path(result.group(0))
+
+from lib import connectAPI
+from utils.Python import utils
+from exception import LibvirtAPI
+
+CERTTOOL = /usr/bin/certtool
+CP = /bin/cp
+MKDIR = /bin/mkdir
+CA_FOLDER = /etc/pki/CA
+PRIVATE_KEY_FOLDER = /etc/pki/libvirt/private
+CERTIFICATE_FOLDER = /etc/pki/libvirt
+
+TEMP_TLS_FOLDER = /tmp/libvirt_test_API_tls
+CAKEY = os.path.join(TEMP_TLS_FOLDER, 'cakey.pem')
+CACERT = os.path.join(TEMP_TLS_FOLDER, 'cacert.pem')
+SERVERKEY = os.path.join(TEMP_TLS_FOLDER, 'serverkey.pem')
+SERVERCERT = os.path.join(TEMP_TLS_FOLDER, 'servercert.pem')
+CLIENTKEY = os.path.join(TEMP_TLS_FOLDER, 'clientkey.pem')
+CLIENTCERT = os.path.join(TEMP_TLS_FOLDER, 'clientcert.pem')
+
+def check_params(params):
+check out the arguments requried for migration
+logger = params['logger']
+keys = ['target_machine', 'username', 'password']
+for key in keys:
+if key not in params:
+logger.error(Argument %s is required % key)
+return 1
+return 0
+
+def CA_setting_up(util, logger):
+ setting up a Certificate Authority 
+# Create a private key for CA
+logger.info(generate CA certificates)
+
+cakey_fd = open(CAKEY, 'w')
+ret, out = util.exec_cmd([CERTTOOL, '--generate-privkey'], 
outfile=cakey_fd)
+cakey_fd.close()
+if ret != 0:
+logger.error(failed to create CA private key)
+return 1
+
+
+# ca.info file
+cainfo = os.path.join(TEMP_TLS_FOLDER, 'ca.info')
+cainfo_fd = open(cainfo, 'w')
+cainfo_str = cn = Libvirt_test_API\n  + \
+ ca\n + \
+ cert_signing_key\n
+
+cainfo_fd.write(cainfo_str)
+cainfo_fd.close()
+
+# Generate cacert.pem
+cacert_args = [CERTTOOL, '--generate-self-signed', '--load-privkey', 
CAKEY, '--template', cainfo]
+cacert_fd = open(CACERT, 'w')
+ret, out = util.exec_cmd(cacert_args, outfile=cacert_fd)
+cacert_fd.close()
+if ret != 0:
+logger.error(failed to create cacert.pem)
+return 1
+
+logger.info(done the CA certificates job)
+return 0
+
+def tls_server_cert(target_machine, util, logger):
+ generating server certificates 
+# Create tls server key
+logger.info(generate server certificates)
+
+serverkey_fd = open(SERVERKEY, 'w')
+ret, out = util.exec_cmd([CERTTOOL, '--generate-privkey'], 
outfile=serverkey_fd)
+serverkey_fd.close()
+if ret != 0:
+logger.error(failed to create server key)
+return 1
+
+# server.info
+serverinfo = os.path.join(TEMP_TLS_FOLDER, 'server.info')
+serverinfo_fd = open(serverinfo, 'w')
+serverinfo_str = organization = Libvirt_test_API\n + \
+ cn = %s\n % target_machine+ \
+ tls_www_server\n  + \
+ encryption_key\n  + \
+ signing_key\n
+
+serverinfo_fd.write(serverinfo_str)
+serverinfo_fd.close()
+
+# Generate servercert.pem
+servercert_args = [CERTTOOL,
+   '--generate-certificate',
+   '--load-privkey', SERVERKEY,
+   '--load-ca-certificate', CACERT,
+   '--load-ca-privkey', CAKEY,
+   '--template', serverinfo
+  ]
+servercert_fd = open(SERVERCERT, 'w')
+ret, out = util.exec_cmd(servercert_args, outfile=servercert_fd)
+servercert_fd.close()
+if ret != 0:
+logger.error(failed to create servercert.pem)
+return 1
+
+logger.info(done the server certificates job)
+return 0
+
+def tls_client_cert(local_machine, util,

Re: [libvirt] [PATCHv3 1/2] conf: add listen subelement to domain graphics element

2011-07-27 Thread Laine Stump


On 07/26/2011 07:10 PM, Eric Blake wrote:

On 07/26/2011 08:50 AM, Laine Stump wrote:

I think it is a somewhat overkill to have 'autoport' be a setting
per-listen element. I can't imagine people want a fixed port
number on one IP addr, but a dynamic port number on another IP
addr. So we could just keep that on the top level element.


Although I agree with you for config purposes, it looks to me like the
real use of autoport is that in the live XML it allows differentiating
between ports that were manually specified and those that were
automatically allocated (really, that seems like its main purpose, since
simply not giving a port also implies autoport). If we have only a
single autoport attribute for all the listens, we'll have to put in
extra code that makes sure if they specify port for one listen, they do
it for all of them.


Is it that hard to add that additional validation?



Anything can be done, given enough time to think, code, and test all the 
corner cases. I don't see where the gain is in this case, though. I see 
autoport more as an attribute that's useful when examining live XML than 
as something useful to set in the config. Having a separate autoport for 
each port is then not overkill, but just natural. Trying to use a single 
autoport to indicate whether all of a set of ports were generated 
automatically or were manually configured is what's overkill - you're 
going to a lot of trouble to enforce an unnatural restriction for no 
practical gain.


Maybe the best thing is to only allow autoport in listen when (flags  
INACTIVE) is false (live XML). This would mean that, as far as config 
was concerned, listen would *never* have any autoport in it (which 
would be fine with me); it would however still be in the data structure, 
and still output in live XML.



Regardless of that answer, and whether this requires a v4, I'll go 
ahead and review v3 code as if we decide that autoport-per-listen is 
acceptable.




--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [libvirt-test-API][PATCH 1/2] add remoteAccess directory and tls authentication testcase

2011-07-27 Thread Osier Yang


? 2011?07?27? 14:58, Guannan Ren ??:

---
  repos/remoteAccess/tls_setup.py |  374 +++
  1 files changed, 374 insertions(+), 0 deletions(-)
  create mode 100644 repos/remoteAccess/__init__.py
  create mode 100644 repos/remoteAccess/tls_setup.py

diff --git a/repos/remoteAccess/__init__.py b/repos/remoteAccess/__init__.py
new file mode 100644
index 000..e69de29
diff --git a/repos/remoteAccess/tls_setup.py b/repos/remoteAccess/tls_setup.py
new file mode 100644
index 000..f703ee3
--- /dev/null
+++ b/repos/remoteAccess/tls_setup.py
@@ -0,0 +1,374 @@
+#!/usr/bin/env python
+ Setup tls authentication between two hosts and configure libvirt
+to use it for connection.
+remoteAccess:tls_setup
+target_machine
+xx.xx.xx.xx
+username
+root
+password
+xx
+
+
+__author__ = 'Guannan Ren: g...@redhat.com'
+__date__ = 'Mon July 25, 2011'
+__version__ = '0.1.0'
+__credits__ = 'Copyright (C) 2011 Red Hat, Inc.'
+__all__ = ['usage']
+
+import os
+import re
+import sys
+import pexpect
+import string
+import commands
+import shutil
+
+def append_path(path):
+Append root path of package
+if path in sys.path:
+pass
+else:
+sys.path.append(path)
+
+pwd = os.getcwd()
+result = re.search('(.*)libvirt-test-API', pwd)
+append_path(result.group(0))
+
+from lib import connectAPI
+from utils.Python import utils
+from exception import LibvirtAPI
+
+CERTTOOL = /usr/bin/certtool
+CP = /bin/cp
+MKDIR = /bin/mkdir
+CA_FOLDER = /etc/pki/CA
+PRIVATE_KEY_FOLDER = /etc/pki/libvirt/private
+CERTIFICATE_FOLDER = /etc/pki/libvirt


Hardcode these paths might not be good idea, there is new parameter
for libvirt connection URI to define where the keys and CA locates. See

http://libvirt.org/remote.html[pkipath] http://libvirt.org/remote.html


+
+TEMP_TLS_FOLDER = /tmp/libvirt_test_API_tls
+CAKEY = os.path.join(TEMP_TLS_FOLDER, 'cakey.pem')
+CACERT = os.path.join(TEMP_TLS_FOLDER, 'cacert.pem')
+SERVERKEY = os.path.join(TEMP_TLS_FOLDER, 'serverkey.pem')
+SERVERCERT = os.path.join(TEMP_TLS_FOLDER, 'servercert.pem')
+CLIENTKEY = os.path.join(TEMP_TLS_FOLDER, 'clientkey.pem')
+CLIENTCERT = os.path.join(TEMP_TLS_FOLDER, 'clientcert.pem')
+
+def check_params(params):
+check out the arguments requried for migration
+logger = params['logger']
+keys = ['target_machine', 'username', 'password']
+for key in keys:
+if key not in params:
+logger.error(Argument %s is required % key)
+return 1
+return 0
+
+def CA_setting_up(util, logger):
+ setting up a Certificate Authority 
+# Create a private key for CA
+logger.info(generate CA certificates)
+
+cakey_fd = open(CAKEY, 'w')
+ret, out = util.exec_cmd([CERTTOOL, '--generate-privkey'], 
outfile=cakey_fd)
+cakey_fd.close()
+if ret != 0:
+logger.error(failed to create CA private key)
+return 1
+
+
+# ca.info file
+cainfo = os.path.join(TEMP_TLS_FOLDER, 'ca.info')
+cainfo_fd = open(cainfo, 'w')
+cainfo_str = cn = Libvirt_test_API\n  + \
+ ca\n + \
+ cert_signing_key\n
+
+cainfo_fd.write(cainfo_str)
+cainfo_fd.close()
+
+# Generate cacert.pem
+cacert_args = [CERTTOOL, '--generate-self-signed', '--load-privkey', 
CAKEY, '--template', cainfo]
+cacert_fd = open(CACERT, 'w')
+ret, out = util.exec_cmd(cacert_args, outfile=cacert_fd)
+cacert_fd.close()
+if ret != 0:
+logger.error(failed to create cacert.pem)
+return 1
+
+logger.info(done the CA certificates job)
+return 0
+
+def tls_server_cert(target_machine, util, logger):
+ generating server certificates 
+# Create tls server key
+logger.info(generate server certificates)
+
+serverkey_fd = open(SERVERKEY, 'w')
+ret, out = util.exec_cmd([CERTTOOL, '--generate-privkey'], 
outfile=serverkey_fd)
+serverkey_fd.close()
+if ret != 0:
+logger.error(failed to create server key)
+return 1
+
+# server.info
+serverinfo = os.path.join(TEMP_TLS_FOLDER, 'server.info')
+serverinfo_fd = open(serverinfo, 'w')
+serverinfo_str = organization = Libvirt_test_API\n + \
+ cn = %s\n % target_machine+ \
+ tls_www_server\n  + \
+ encryption_key\n  + \
+ signing_key\n
+
+serverinfo_fd.write(serverinfo_str)
+serverinfo_fd.close()
+
+# Generate servercert.pem
+servercert_args = [CERTTOOL,
+   '--generate-certificate',
+   '--load-privkey', SERVERKEY,
+   '--load-ca-certificate', CACERT,
+   '--load-ca-privkey', CAKEY,
+   '--template', serverinfo
+  ]
+servercert_fd = open(SERVERCERT, 'w')
+ret, out =

Re: [libvirt] [PATCH ] send-key: Implement Python API

2011-07-27 Thread Wen Congyang

At 07/26/2011 06:26 PM, Daniel P. Berrange Write:
 On Thu, Jul 21, 2011 at 05:21:10PM +0800, Lai Jiangshan wrote:

 Signed-off-by: Lai Jiangshan la...@cn.fujitsu.com
 ---
 diff --git a/python/libvirt-override.c b/python/libvirt-override.c
 index b713b6a..1ef5bfa 100644
 --- a/python/libvirt-override.c
 +++ b/python/libvirt-override.c
 @@ -3789,6 +3789,53 @@ libvirt_virStreamSend(PyObject *self ATTRIBUTE_UNUSED,
  return py_retval;
  }
  
 +static PyObject *
 +libvirt_virDomainSendKey(PyObject *self ATTRIBUTE_UNUSED,
 + PyObject *args)
 +{
 +PyObject *py_retval;
 +virDomainPtr domain;
 +PyObject *pyobj_domain;
 +PyObject *pyobj_list;
 +int codeset;
 +int holdtime;
 +unsigned int flags;
 +int ret;
 +int i;
 +unsigned int keycodes[VIR_DOMAIN_SEND_KEY_MAX_KEYS];
 +unsigned int nkeycodes;
 +
 +if (!PyArg_ParseTuple(args, (char *)OiiOii:virDomainSendKey,
 +  pyobj_domain, codeset, holdtime, pyobj_list,
 +  nkeycodes, flags)) {
 +DEBUG(%s failed to parse tuple\n, __FUNCTION__);
 +return VIR_PY_INT_FAIL;
 +}
 +domain = (virDomainPtr) PyvirDomain_Get(pyobj_domain);
 +
 +if (!PyList_Check(pyobj_list)) {
 +return VIR_PY_INT_FAIL;
 +}
 +
 +if (nkeycodes != PyList_Size(pyobj_list) ||
 +nkeycodes  VIR_DOMAIN_SEND_KEY_MAX_KEYS) {
 +return VIR_PY_INT_FAIL;
 +}
 +
 +for (i = 0; i  nkeycodes; i++) {
 +keycodes[i] = (int)PyInt_AsLong(PyList_GetItem(pyobj_list, i));
 +}
 +
 +LIBVIRT_BEGIN_ALLOW_THREADS;
 +ret = virDomainSendKey(domain, codeset, holdtime, keycodes, nkeycodes, 
 flags);
 +LIBVIRT_END_ALLOW_THREADS;
 +
 +DEBUG(virDomainSendKey ret=%d\n, ret);
 +
 +py_retval = libvirt_intWrap(ret);
 +return py_retval;
 +}
 +
  /
   *  *
   *  The registration stuff  *
 @@ -3872,6 +3919,7 @@ static PyMethodDef libvirtMethods[] = {
  {(char *) virDomainGetJobInfo, libvirt_virDomainGetJobInfo, 
 METH_VARARGS, NULL},
  {(char *) virDomainSnapshotListNames, 
 libvirt_virDomainSnapshotListNames, METH_VARARGS, NULL},
  {(char *) virDomainRevertToSnapshot, 
 libvirt_virDomainRevertToSnapshot, METH_VARARGS, NULL},
 +{(char *) virDomainSendKey, libvirt_virDomainSendKey, METH_VARARGS, 
 NULL},
  {NULL, NULL, 0, NULL}
  };
 
 ACK

I read the other patch about implementing Python API, and find that the file
python/libvirt-override-api.xml is updated in the other patch. But it is not 
updated
in this patch. Is there no need to update this file here?

Thanks
Wen Congyang

 
 Daniel

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [libvirt-test-API][PATCH 2/2] Add four helpful functions in utils.py.

2011-07-27 Thread Guannan Sun

ACK

Thanks!
Best Regards!
Wayne Sun
Redhat QE, Beijing, China
+86-10-6260-8238

- Original Message -
From: Guannan Ren g...@redhat.com
To: libvir-list@redhat.com
Sent: Wednesday, July 27, 2011 2:58:33 PM
Subject: [libvirt] [libvirt-test-API][PATCH 2/2] Add four helpful functions 
in utils.py.

* get_local_hostname() get hostname on local machine
* exec_cmd() execute shell command
* remote_exec_pexpect() execture command on remote host
* scp_file() copy file to remote host
---
 utils/Python/utils.py |   69 +
 1 files changed, 69 insertions(+), 0 deletions(-)

diff --git a/utils/Python/utils.py b/utils/Python/utils.py
index eaa8eab..6787e87 100644
--- a/utils/Python/utils.py
+++ b/utils/Python/utils.py
@@ -26,6 +26,9 @@ import commands
 import socket
 import fcntl
 import struct
+import pexpect
+import string
+import subprocess
 from xml.dom import minidom
 
 class Utils(object):
@@ -58,6 +61,10 @@ class Utils(object):
 arch = ret.split( )[-2]
 return arch
 
+def get_local_hostname(self):
+ get local host name 
+return socket.gethostname()
+
 def get_libvirt_version(self, ver = ''):
 ver = commands.getoutput(rpm -q libvirt|head -1)
 if ver.split('-')[0] == 'libvirt':
@@ -364,3 +371,65 @@ class Utils(object):
 (ret, out) = commands.getstatusoutput(cmd)
 return ret
 
+def exec_cmd(self, command, sudo=False, cwd=None, infile=None, 
outfile=None, shell=False, data=None):
+
+Executes an external command, optionally via sudo.
+
+if sudo:
+if type(command) == type():
+command = sudo  + command
+else:
+command = [sudo] + command
+if infile == None:
+infile = subprocess.PIPE
+if outfile == None:
+outfile = subprocess.PIPE
+p = subprocess.Popen(command, shell=shell, close_fds=True, cwd=cwd,
+stdin=infile, stdout=outfile, stderr=subprocess.PIPE)
+(out, err) = p.communicate(data)
+if out == None:
+# Prevent splitlines() from barfing later on
+out = 
+return (p.returncode, out.splitlines())
+
+def remote_exec_pexpect(self, hostname, username, password, cmd):
+ Remote exec function via pexpect 
+user_hostname = %s@%s % (username, hostname)
+child = pexpect.spawn(/usr/bin/ssh, [user_hostname, cmd],
+  timeout = 60, maxread = 2000, logfile = None)
+while True:
+index = child.expect(['(yes\/no)', 'password:', pexpect.EOF,
+ pexpect.TIMEOUT])
+if index == 0:
+child.sendline(yes)
+elif index == 1:
+child.sendline(password)
+elif index == 2:
+child.close()
+return 0
+elif index == 3:
+child.close()
+return 1
+
+return 0
+
+def scp_file(self, hostname, username, password, target_path, file):
+ Scp file to remote host 
+user_hostname = %s@%s:%s % (username, hostname, target_path)
+child = pexpect.spawn(/usr/bin/scp, [file, user_hostname])
+while True:
+index = child.expect(['yes\/no', 'password: ',
+   pexpect.EOF,
+   pexpect.TIMEOUT])
+if index == 0:
+child.sendline(yes)
+elif index == 1:
+child.sendline(password)
+elif index == 2:
+child.close()
+return 0
+elif index == 3:
+child.close()
+return 1
+
+return 0
-- 
1.7.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] Fix for newer yajl

2011-07-27 Thread Brecht Sanders


Hi,
I just notices that compiling libvirt against yail version 2 there were 
some errors, mainly because yajl_parser_config is no longer used.

The patch below fixed this for me.
Regards
Brecht Sanders


--- src/util/json.c  2011-03-24 08:10:26 +0100
+++ src/util/json.c  2011-07-27 09:29:52 +0200
@@ -34,2 +34,3 @@
 # include yajl/yajl_parse.h
+# include yajl/yajl_version.h
 #endif
@@ -896,3 +897,5 @@
 {
+#if YAJL_MAJOR  2
 yajl_parser_config cfg = { 1, 1 };
+#endif
 yajl_handle hand;
@@ -903,3 +906,7 @@

+#if YAJL_MAJOR  2
 hand = yajl_alloc(parserCallbacks, cfg, NULL, parser);
+#else
+hand = yajl_alloc(parserCallbacks, NULL, parser);
+#endif

@@ -1004,3 +1011,5 @@
 {
+#if YAJL_MAJOR  2
 yajl_gen_config conf = { 0,   }; /* Turns off pretty printing 
since QEMUcan't cope */

+#endif
 yajl_gen g;
@@ -1012,3 +1021,7 @@

+#if YAJL_MAJOR  2
 g = yajl_gen_alloc(conf, NULL);
+#else
+g = yajl_gen_alloc(NULL);
+#endif


--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCHv3 1/2] conf: add listen subelement to domain graphics element

2011-07-27 Thread Laine Stump


On 07/26/2011 08:09 PM, Eric Blake wrote:

On 07/25/2011 03:00 AM, Laine Stump wrote:

Once it's plugged in, thelisten element will be an optional
replacement for the listen, port, tlsPort, and autoport
attributes that graphics elements already have. If thelisten
type='address', it will have an attribute called 'address' which will
contain an IP address or dns name that the guest's display server
should listen on. If, however, type='network', thelisten  element
should have an attribute called 'network' that will be set to the name
of a network configuration to get the IP address from.

  91 files changed, 1274 insertions(+), 343 deletions(-)


Big diff, but like you said mostly mechanical.


@@ -2110,6 +2116,70 @@ qemu-kvm -net nic,model=? /dev/null
/dd
/dl

+p
+  Rather than putting the information used to setup the listening


setup is a noun, but here you want a verb form:

s/setup/set up/


+dtcodeautoport/code/dt
+ddIf set to 'yes', a listen port will be determined
+automatically at runtime, and reflected in the domain's live
+XML.
+/dd


May need tweaks depending on the decision on where autoport should live.


@@ -1500,6 +1503,49 @@
/choice
/element
/define
+
+define name=listenElements
+zeroOrMore
+element name=listen
+optional
+attribute name=type
+choice
+valueaddress/value
+valuenetwork/value


/me curses thunderbird for squashing whitespace in my reply

I'm not sure this is right.  I think we want to require either 
type=address (which implies address=nnn must also be present, and 
network=nnn must not be present) or type=network (which implies that 
network=nnn must be present, and address=nnn might show up in live XML 
dumpxml output to show what was resolved, but is ignored in input 
parsing.  Also, autoport=yes and port=nnn should be a forbidden 
combination.



Actually it's not forbidden, that's really the only practical use I can 
see for the autoport attribute - in live XML autoport='yes' port='nnn' 
is used to indicate that the port was auto-generated and this time 'nnn' 
was used. (and not that it's any indication of what is right or wrong, 
but the current RNG file allows it).


(Personally, I would prefer if the currently in-use port was stored in a 
separate attribute, and autoport simply didn't exist. But



  That is, I think this should be as follows (assuming we keep 
autoport in listen, further changes if autoport is global only):



I think if we made autoport a single setting global to all instances of 
listen, and tried to prevent simultaneous (global) autoport and (local 
to one listen) port, the RNG would be quite unwieldy. (you would have to 
have a listenElementsPort and a listenElementsAutoport, and the graphics 
RNG would put the two of them in a choice depending on the setting of 
autoport)






define name=listenElements
zeroOrMore
element name=listen
choice
group
attribute name=type
valueaddress/value
/attribute
attribute name=address
ref name=addrIPorName/
/attribute
/group
group
attribute name=type
valuenetwork/value
/attribute
attribute name=network
text/
/attribute
optional
attribute name=address
ref name=addrIPorName/
/attribute
/optional
/group
/choice
choice
attribute name=autoport
valueyes/value
/attribute
group
optional
attribute name=autoport
valueno/value
/attribute
/optional
optional
attribute name=port
ref name=PortNumber/
/attribute
/optional
optional
attribute name=tlsPort
ref name=PortNumber/
/attribute
/optional
/group
/choice
/element


+/zeroOrMore
+/define


Two spaces too much indentation on /define (not like it shows up any 
better in my nitpick, though).


@@ -4010,19 +4029,118 @@ virDomainGraphicsAuthDefParseXML(xmlNodePtr 
node,

  return 0;
  }

+static int
+virDomainGraphicsListenParseXML(virDomainGraphicsListenDefPtr def,
+xmlNodePtr node,
+enum virDomainGraphicsType 
graphicsType,

+unsigned int flags)
+{
+int ret = -1;
+char *type = virXMLPropString(node, type);
+char *address  = virXMLPropString(node, address);
+char *network  = virXMLPropString(node, network);
+char *port = virXMLPropString(node, port);
+char *tlsPort  = virXMLPropString(node, tlsPort);
+char *autoport = virXMLPropString(node, autoport);
+
+if (type  (def-type = 
virDomainGraphicsListenTypeFromString(type))  0) {

+virDomainReportError(VIR_ERR_XML_ERROR,
+ _(unknown graphics listen type '%s'), 
type);

+goto error;
+}


Shouldn't this fail if type is not found?  That is, I think type is a 
mandatory attribute (must be address or network).



No. It's also possible that no address/network information is given (in 
which case type would be default, which ends up as just no type 
showing up at all). In that case, the qemu driver has default listen 
addresses for VNC and for Spice. So it's completely valid to give a 
port, but no address.






+
+if (address

Re: [libvirt] Fix for newer yajl

2011-07-27 Thread Daniel P. Berrange

On Wed, Jul 27, 2011 at 09:40:47AM +0200, Brecht Sanders wrote:
 Hi,
 I just notices that compiling libvirt against yail version 2 there
 were some errors, mainly because yajl_parser_config is no longer
 used.

What version of libvirt is your patch against ?  The latest releases
already have fixed compatibility with YAJL 2, so I'm assuming you have
an old release ?

Regards,
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: support warnings on RHEL 5

2011-07-27 Thread Daniel P. Berrange

On Tue, Jul 26, 2011 at 02:25:40PM -0600, Eric Blake wrote:
 Without this, a configure built by autoconf 2.59 was broken when
 trying to detect which compiler warning flags were supported.
 
 * .gnulib: Update to latest, for warnings.m4 fix.
 * src/qemu/qemu_conf.c (includes): Drop unused include.
 * src/uml/uml_conf.c (include): Likewise.
 Reported by Daniel P. Berrange.


ACK

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] [Qemu-devel] [PATCH v3] Add support for fd: protocol

2011-07-27 Thread Kevin Wolf

Am 26.07.2011 18:57, schrieb Corey Bryant:
 
 Kevin, thanks for the input.
 
 On 07/26/2011 11:18 AM, Kevin Wolf wrote:
 Am 26.07.2011 14:51, schrieb Corey Bryant:
  sVirt provides SELinux MAC isolation for Qemu guest processes and their
  corresponding resources (image files). sVirt provides this support
  by labeling guests and resources with security labels that are stored
  in file system extended attributes. Some file systems, such as NFS, do
  not support the extended attribute security namespace, which is needed
  for image file isolation when using the sVirt SELinux security driver
  in libvirt.

  The proposed solution entails a combination of Qemu, libvirt, and
  SELinux patches that work together to isolate multiple guests' images
  when they're stored in the same NFS mount. This results in an
  environment where sVirt isolation and NFS image file isolation can both
  be provided.

  This patch contains the Qemu code to support this solution. I would
  like to solicit input from the libvirt community prior to starting
  the libvirt patch.

  Currently, Qemu opens an image file in addition to performing the
  necessary read and write operations. The proposed solution will move
  the open out of Qemu and into libvirt. Once libvirt opens an image
  file for the guest, it will pass the file descriptor to Qemu via a
  new fd: protocol.

  If the image file resides in an NFS mount, the following SELinux policy
  changes will provide image isolation:

 - A new SELinux boolean is created (e.g. virt_read_write_nfs) to
   allow Qemu (svirt_t) to only have SELinux read and write
   permissions on nfs_t files

 - Qemu (svirt_t) also gets SELinux use permissions on libvirt
   (virtd_t) file descriptors

  Following is a sample invocation of Qemu using the fd: protocol on
  the command line:

   qemu -drive file=fd:4,format=qcow2

  The fd: protocol is also supported by the drive_add monitor command.
  This requires that the specified file descriptor is passed to the
  monitor alongside a prior getfd monitor command.

  There are some additional features provided by certain image types
  where Qemu reopens the image file. All of these scenarios will be
  unsupported for the fd: protocol, at least for this patch:

 - The -snapshot command line option
 - The savevm monitor command
 - The snapshot_blkdev monitor command
 - Use of copy-on-write image files
 - The -cdrom command line option
 - The -drive command line option with media=cdrom
 - The change monitor command

  The thought is that this support can be added in the future, but is
  not required for the initial fd: support.

  This patch was tested with the following formats: raw, cow, qcow,
  qcow2, qed, and vmdk, using the fd: protocol from the command line
  and the monitor. Tests were also run to verify existing file name
  support and qemu-img were not regressed. Non-valid file descriptors,
  fd: without format, snapshot and backing files, and cdrom were also
  tested.

  v2:
 - Add drive_add monitor command support
 - Fence off unsupported features that re-open image file

  v3:
 - Fence off cdrom and change monitor command support

  Signed-off-by: Corey Bryantcor...@linux.vnet.ibm.com
  ---
block.c   |   16 ++
block.h   |1 +
block/cow.c   |5 +++
block/qcow.c  |5 +++
block/qcow2.c |5 +++
block/qed.c   |4 ++
block/raw-posix.c |   81 
 +++--
block/vmdk.c  |5 +++
block_int.h   |1 +
blockdev.c|   19 
monitor.c |5 +++
monitor.h |1 +
qemu-options.hx   |8 +++--
qemu-tool.c   |5 +++
14 files changed, 149 insertions(+), 12 deletions(-)

  diff --git a/block.c b/block.c
  index 24a25d5..500db84 100644
  --- a/block.c
  +++ b/block.c
  @@ -536,6 +536,10 @@ int bdrv_open(BlockDriverState *bs, const char 
 *filename, int flags,
char tmp_filename[PATH_MAX];
char backing_filename[PATH_MAX];

  +if (bdrv_is_fd_protocol(bs)) {
  +return -ENOTSUP;
  +}
 Hm, shouldn't that just work even with fd?

 
 My thought was that the proposed SELinux changes would prevent the open 
 of the temporary backing file if the file corresponding to fd resides on 
 NFS.  But perhaps the backing file is not opened on NFS?

Depends on how broken your SELinux restrictions are. ;-)

I would argue that allowing qemu to create temporary files is a
reasonable thing to do. Maybe libvirt comes to a different conclusion,
but that doesn't mean that every other management tool comes to the same.

Also, as Alex already said, you shouldn't think of your use case as the
only valid use case. What a fd driver implementation is about is just
working with an fd for images. If libvirts puts more restrictions on it,
that's fine, but don't force other users

Re: [libvirt] [Qemu-devel] [PATCH v3] Add support for fd: protocol

2011-07-27 Thread Daniel P. Berrange

On Wed, Jul 27, 2011 at 10:11:06AM +0200, Kevin Wolf wrote:
 Am 26.07.2011 18:57, schrieb Corey Bryant:
   diff --git a/block/cow.c b/block/cow.c
   index 4cf543c..e17f8e7 100644
   --- a/block/cow.c
   +++ b/block/cow.c
   @@ -82,6 +82,11 @@ static int cow_open(BlockDriverState *bs, int flags)
 pstrcpy(bs-backing_file, sizeof(bs-backing_file),
 cow_header.backing_file);
 
   +if (bs-backing_file[0] != '\0'  bdrv_is_fd_protocol(bs)) {
   +/* backing file currently not supported by fd: protocol */
   +goto fail;
   +}
  I don't think these checks are strictly needed. Without the check you
  can open the image itself using an fd, and the backing file using good
  old raw-posix. We shouldn't decide for our users that this isn't useful.
 
  In any case, it would have to move into block.c, you can't modify
  independent drivers for this.
 
  
  I understand the point on not modifying independent drivers.
  
  But if the backing file resides on NFS, wouldn't the the proposed 
  SELinux changes prevent the open?
 
 Probably. But what about cases where the backing file is local? Or even
 a non-libvirt, non-SELinux use case?
 
  Or are you talking about support where libvirt opens the backing file 
  and passes the fd to Qemu?  If so there was some discussion about future 
  support for this here: 
  http://lists.gnu.org/archive/html/qemu-devel/2011-06/msg01496.html
 
 Not really, but implementing this will be a bit easier if you don't
 forbid using backing files with fd.

In any case, for 'fd:' to be actually used by libvirt, we need to have
backing files supported. There are major users of libvirt who rely on
NFS and also use backing files, so an 'fd:' impl which can't deal with
the backing file problem isn't much use to libvirt.

Regards,
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] Fix for newer yajl

2011-07-27 Thread Brecht Sanders




Hi,
I just notices that compiling libvirt against yail version 2 there
were some errors, mainly because yajl_parser_config is no longer
used.

What version of libvirt is your patch against ?  The latest releases
already have fixed compatibility with YAJL 2, so I'm assuming you have
an old release ?

Regards,
Daniel

Hmm, you're right. My bad.
I forgot I was compiling 0.9.0 because I had other issues compiling 
later versions under MinGW/MSYS on Windows.
In case you were not aware of this I'm including the end of my 0.9.3 
output below.


libtool: compile:  gcc -std=gnu99 -DHAVE_CONFIG_H -I. -I.. 
-I../gnulib/lib -I../gnulib/lib -I../include -I../src/util -I../include 
-DIN_LIBVIRT -I/custombuilt32/include/libxml2 -Wall -W -Wformat-y2k 
-Wformat-security -Winit-self -Wmissing-include-dirs -Wunused 
-Wunknown-pragmas -Wstrict-aliasing -Wshadow -Wpointer-arith 
-Wbad-function-cast -Wcast-align -Wwrite-strings -Wlogical-op 
-Waggregate-return -Wstrict-prototypes -Wold-style-definition 
-Wmissing-prototypes -Wmissing-declarations -Wmissing-noreturn 
-Wmissing-format-attribute -Wredundant-decls -Wnested-externs -Winline 
-Winvalid-pch -Wvolatile-register-var -Wdisabled-optimization 
-Wbuiltin-macro-redefined -Wmudflap -Wpacked-bitfield-compat -Wsync-nand 
-Wattributes -Wcoverage-mismatch -Wmultichar 
-Wno-missing-field-initializers -Wno-sign-compare -Wjump-misses-init 
-Wframe-larger-than=4096 -Wp,-D_FORTIFY_SOURCE=2 -fexceptions 
-fasynchronous-unwind-tables -fdiagnostics-show-option -funit-at-a-time 
-Drestrict=__restrict -DWSAAPI -Dsiginfo_t=int -MT 
libvirt_net_rpc_server_la-virnetserver.lo -MD -MP -MF 
.deps/libvirt_net_rpc_server_la-virnetserver.Tpo -c rpc/virnetserver.c  
-DDLL_EXPORT -DPIC -o .libs/libvirt_net_rpc_server_la-virnetserver.o

rpc/virnetserver.c: In function 'virNetServerSignalHandler':
rpc/virnetserver.c:392:12: error: request for member 'si_signo' in 
something not a structure or union

rpc/virnetserver.c: In function 'virNetServerSignalEvent':
rpc/virnetserver.c:423:20: error: request for member 'si_signo' in 
something not a structure or union
rpc/virnetserver.c:432:5: error: request for member 'si_signo' in 
something not a structure or union

make[3]: *** [libvirt_net_rpc_server_la-virnetserver.lo] Error 1
make[3]: Leaving directory `/home/win32/libvirt-0.9.3/src'
make[2]: *** [all] Error 2
make[2]: Leaving directory `/home/win32/libvirt-0.9.3/src'
make[1]: *** [all-recursive] Error 1
make[1]: Leaving directory `/home/win32/libvirt-0.9.3'
make: *** [all] Error 2

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH] bandwidth: Integrate bandwidth into portgroups

2011-07-27 Thread Michal Privoznik


On 26.07.2011 20:04, Laine Stump wrote:

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 031862a..f99294c 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -8867,8 +8867,7 @@ virDomainNetDefFormat(virBufferPtr buf,
  virBufferAddLit(buf,   /tune\n);
  }

-if (virBandwidthDefFormat(buf, virDomainNetGetActualBandwidth(def),
-)  0)
+if (virBandwidthDefFormat(buf, def-bandwidth,   )  0)
  return -1;

  if (virDomainDeviceInfoFormat(buf,def-info, flags)  0)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 0fa26dd..a406100 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -337,6 +337,7 @@ virDomainLoadAllConfigs;
  virDomainMemballoonModelTypeFromString;
  virDomainMemballoonModelTypeToString;
  virDomainNetDefFree;
+virDomainNetGetActualBandwidth;
  virDomainNetGetActualBridgeName;
  virDomainNetGetActualDirectDev;
  virDomainNetGetActualDirectMode;
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index f8fd4ee..35d6b1e 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -132,7 +132,8 @@ qemuPhysIfaceConnect(virDomainDefPtr def,
  vnet_hdr, def-uuid,
  virDomainNetGetActualDirectVirtPortProfile(net),
  res_ifname,
-vmop, driver-stateDir, net-bandwidth);
+vmop, driver-stateDir,
+virDomainNetGetActualBandwidth(net));
  if (rc= 0) {
  virDomainAuditNetDevice(def, net, res_ifname, true);
  VIR_FREE(net-ifname);
@@ -299,7 +300,8 @@ qemuNetworkIfaceConnect(virDomainDefPtr def,
  }

  if (tapfd= 0
-virBandwidthEnable(net-bandwidth, net-ifname)  0) {
+virBandwidthEnable(virDomainNetGetActualBandwidth(net),
+   net-ifname)  0) {
  qemuReportError(VIR_ERR_INTERNAL_ERROR,
  _(cannot set bandwidth limits on %s),
  net-ifname);
diff --git a/src/util/network.c b/src/util/network.c
index 314cabe..6fcdab2 100644
--- a/src/util/network.c
+++ b/src/util/network.c
@@ -1284,6 +1284,7 @@ virBandwidthCopy(virBandwidthPtr *dest,
  return -1;
  }

+*dest = NULL;
  if (!src) {
  /* nothing to be copied */
  return 0;
@@ -1305,6 +1306,7 @@ virBandwidthCopy(virBandwidthPtr *dest,
  if (src-out) {
  if (VIR_ALLOC((*dest)-out)  0) {
  virReportOOMError();
+VIR_FREE((*dest)-in);
  goto cleanup;
  }

Thanks, pushed.

Michal

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [Qemu-devel] [PATCH v3] Add support for fd: protocol

2011-07-27 Thread Kevin Wolf

Am 27.07.2011 10:22, schrieb Daniel P. Berrange:
 On Wed, Jul 27, 2011 at 10:11:06AM +0200, Kevin Wolf wrote:
 Am 26.07.2011 18:57, schrieb Corey Bryant:
  diff --git a/block/cow.c b/block/cow.c
  index 4cf543c..e17f8e7 100644
  --- a/block/cow.c
  +++ b/block/cow.c
  @@ -82,6 +82,11 @@ static int cow_open(BlockDriverState *bs, int flags)
pstrcpy(bs-backing_file, sizeof(bs-backing_file),
cow_header.backing_file);

  +if (bs-backing_file[0] != '\0'  bdrv_is_fd_protocol(bs)) {
  +/* backing file currently not supported by fd: protocol */
  +goto fail;
  +}
 I don't think these checks are strictly needed. Without the check you
 can open the image itself using an fd, and the backing file using good
 old raw-posix. We shouldn't decide for our users that this isn't useful.

 In any case, it would have to move into block.c, you can't modify
 independent drivers for this.


 I understand the point on not modifying independent drivers.

 But if the backing file resides on NFS, wouldn't the the proposed 
 SELinux changes prevent the open?

 Probably. But what about cases where the backing file is local? Or even
 a non-libvirt, non-SELinux use case?

 Or are you talking about support where libvirt opens the backing file 
 and passes the fd to Qemu?  If so there was some discussion about future 
 support for this here: 
 http://lists.gnu.org/archive/html/qemu-devel/2011-06/msg01496.html

 Not really, but implementing this will be a bit easier if you don't
 forbid using backing files with fd.
 
 In any case, for 'fd:' to be actually used by libvirt, we need to have
 backing files supported. There are major users of libvirt who rely on
 NFS and also use backing files, so an 'fd:' impl which can't deal with
 the backing file problem isn't much use to libvirt.

I'm perfectly aware of that. But seriously, repeating it over and over
again isn't going to make it happen any sooner. It requires -blockdev
which we may or may not have by 1.0.

Kevin

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] libvirt-0.9.1 to 0.9.3-r1: managedsave/save won't start/restore at saved state

2011-07-27 Thread Nicolas Sebrecht

I'm seeing strange behaviour, here.  Any guests saved using both
managedsave and save commands from virsh won't restore at saved state. A
new full boot sequence happen.

- Tested against libvirt v0.9.1, v0.9.2, v0.9.3-r1 (Gentoo)
- Confirmed on three different hosts Gentoo amd64 systems.
- Tested with gentoo and ubuntu guests.
- Nothing relevant in /var/log/libvirt/libvirt.log or
/var/log/libvirt/qemu/dom.log

The state file /var/lib/libvirt/qemu/save/dom.save exists and is
deleted when 'virsh start' is called.

The new boot sequence is confirmed by :
- VNC console checks
- previous screen sessions lost
- uptime

I've open a bug at https://bugs.gentoo.org/show_bug.cgi?id=376333 but
had no answer.

Any idea on what could happen or how to inspect it?

-- 
Nicolas Sebrecht

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [Qemu-devel] [PATCH v3] Add support for fd: protocol

2011-07-27 Thread Daniel P. Berrange

On Wed, Jul 27, 2011 at 10:36:25AM +0200, Kevin Wolf wrote:
 Am 27.07.2011 10:22, schrieb Daniel P. Berrange:
  On Wed, Jul 27, 2011 at 10:11:06AM +0200, Kevin Wolf wrote:
  Am 26.07.2011 18:57, schrieb Corey Bryant:
   diff --git a/block/cow.c b/block/cow.c
   index 4cf543c..e17f8e7 100644
   --- a/block/cow.c
   +++ b/block/cow.c
   @@ -82,6 +82,11 @@ static int cow_open(BlockDriverState *bs, int 
  flags)
 pstrcpy(bs-backing_file, sizeof(bs-backing_file),
 cow_header.backing_file);
 
   +if (bs-backing_file[0] != '\0'  bdrv_is_fd_protocol(bs)) {
   +/* backing file currently not supported by fd: protocol */
   +goto fail;
   +}
  I don't think these checks are strictly needed. Without the check you
  can open the image itself using an fd, and the backing file using good
  old raw-posix. We shouldn't decide for our users that this isn't useful.
 
  In any case, it would have to move into block.c, you can't modify
  independent drivers for this.
 
 
  I understand the point on not modifying independent drivers.
 
  But if the backing file resides on NFS, wouldn't the the proposed 
  SELinux changes prevent the open?
 
  Probably. But what about cases where the backing file is local? Or even
  a non-libvirt, non-SELinux use case?
 
  Or are you talking about support where libvirt opens the backing file 
  and passes the fd to Qemu?  If so there was some discussion about future 
  support for this here: 
  http://lists.gnu.org/archive/html/qemu-devel/2011-06/msg01496.html
 
  Not really, but implementing this will be a bit easier if you don't
  forbid using backing files with fd.
  
  In any case, for 'fd:' to be actually used by libvirt, we need to have
  backing files supported. There are major users of libvirt who rely on
  NFS and also use backing files, so an 'fd:' impl which can't deal with
  the backing file problem isn't much use to libvirt.
 
 I'm perfectly aware of that. But seriously, repeating it over and over
 again isn't going to make it happen any sooner. It requires -blockdev
 which we may or may not have by 1.0.

Yes, I understand we need also -blockdev for this. Other messages in this
mail thread have impied that this proposed patch on its own is useful for
libvirt's requirements, so I just wanted to remind people in general that
we can't use this patch on its own until we have something like -blockdev.

Regards,
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] [libvirt-test-API][PATCH V2] add remoteAccess directory and tls authentication testcase

2011-07-27 Thread Guannan Ren

---
 repos/remoteAccess/tls_setup.py |  386 +++
 1 files changed, 386 insertions(+), 0 deletions(-)
 create mode 100644 repos/remoteAccess/__init__.py
 create mode 100644 repos/remoteAccess/tls_setup.py

diff --git a/repos/remoteAccess/__init__.py b/repos/remoteAccess/__init__.py
new file mode 100644
index 000..e69de29
diff --git a/repos/remoteAccess/tls_setup.py b/repos/remoteAccess/tls_setup.py
new file mode 100644
index 000..6d0b27c
--- /dev/null
+++ b/repos/remoteAccess/tls_setup.py
@@ -0,0 +1,386 @@
+#!/usr/bin/env python
+ Setup tls authentication between two hosts and configure libvirt
+to use it for connection.
+remoteAccess:tls_setup
+target_machine
+xx.xx.xx.xx
+username
+root
+password
+xx
+pkipath(optional)
+/tmp/pkipath
+
+
+__author__ = 'Guannan Ren: g...@redhat.com'
+__date__ = 'Mon July 25, 2011'
+__version__ = '0.1.0'
+__credits__ = 'Copyright (C) 2011 Red Hat, Inc.'
+__all__ = ['usage']
+
+import os
+import re
+import sys
+import pexpect
+import string
+import commands
+import shutil
+
+def append_path(path):
+Append root path of package
+if path in sys.path:
+pass
+else:
+sys.path.append(path)
+
+pwd = os.getcwd()
+result = re.search('(.*)libvirt-test-API', pwd)
+append_path(result.group(0))
+
+from lib import connectAPI
+from utils.Python import utils
+from exception import LibvirtAPI
+
+CERTTOOL = /usr/bin/certtool
+CP = /bin/cp
+MKDIR = /bin/mkdir
+CA_FOLDER = /etc/pki/CA
+PRIVATE_KEY_FOLDER = /etc/pki/libvirt/private
+CERTIFICATE_FOLDER = /etc/pki/libvirt
+
+TEMP_TLS_FOLDER = /tmp/libvirt_test_API_tls
+CAKEY = os.path.join(TEMP_TLS_FOLDER, 'cakey.pem')
+CACERT = os.path.join(TEMP_TLS_FOLDER, 'cacert.pem')
+SERVERKEY = os.path.join(TEMP_TLS_FOLDER, 'serverkey.pem')
+SERVERCERT = os.path.join(TEMP_TLS_FOLDER, 'servercert.pem')
+CLIENTKEY = os.path.join(TEMP_TLS_FOLDER, 'clientkey.pem')
+CLIENTCERT = os.path.join(TEMP_TLS_FOLDER, 'clientcert.pem')
+
+def check_params(params):
+check out the arguments requried for migration
+logger = params['logger']
+keys = ['target_machine', 'username', 'password']
+for key in keys:
+if key not in params:
+logger.error(Argument %s is required % key)
+return 1
+return 0
+
+def CA_setting_up(util, logger):
+ setting up a Certificate Authority 
+# Create a private key for CA
+logger.info(generate CA certificates)
+
+cakey_fd = open(CAKEY, 'w')
+ret, out = util.exec_cmd([CERTTOOL, '--generate-privkey'], 
outfile=cakey_fd)
+cakey_fd.close()
+if ret != 0:
+logger.error(failed to create CA private key)
+return 1
+
+
+# ca.info file
+cainfo = os.path.join(TEMP_TLS_FOLDER, 'ca.info')
+cainfo_fd = open(cainfo, 'w')
+cainfo_str = cn = Libvirt_test_API\n  + \
+ ca\n + \
+ cert_signing_key\n
+
+cainfo_fd.write(cainfo_str)
+cainfo_fd.close()
+
+# Generate cacert.pem
+cacert_args = [CERTTOOL, '--generate-self-signed', '--load-privkey', 
CAKEY, '--template', cainfo]
+cacert_fd = open(CACERT, 'w')
+ret, out = util.exec_cmd(cacert_args, outfile=cacert_fd)
+cacert_fd.close()
+if ret != 0:
+logger.error(failed to create cacert.pem)
+return 1
+
+logger.info(done the CA certificates job)
+return 0
+
+def tls_server_cert(target_machine, util, logger):
+ generating server certificates 
+# Create tls server key
+logger.info(generate server certificates)
+
+serverkey_fd = open(SERVERKEY, 'w')
+ret, out = util.exec_cmd([CERTTOOL, '--generate-privkey'], 
outfile=serverkey_fd)
+serverkey_fd.close()
+if ret != 0:
+logger.error(failed to create server key)
+return 1
+
+# server.info
+serverinfo = os.path.join(TEMP_TLS_FOLDER, 'server.info')
+serverinfo_fd = open(serverinfo, 'w')
+serverinfo_str = organization = Libvirt_test_API\n + \
+ cn = %s\n % target_machine+ \
+ tls_www_server\n  + \
+ encryption_key\n  + \
+ signing_key\n
+
+serverinfo_fd.write(serverinfo_str)
+serverinfo_fd.close()
+
+# Generate servercert.pem
+servercert_args = [CERTTOOL,
+   '--generate-certificate',
+   '--load-privkey', SERVERKEY,
+   '--load-ca-certificate', CACERT,
+   '--load-ca-privkey', CAKEY,
+   '--template', serverinfo
+  ]
+servercert_fd = open(SERVERCERT, 'w')
+ret, out = util.exec_cmd(servercert_args, outfile=servercert_fd)
+servercert_fd.close()
+if ret != 0:
+logger.error(failed to create servercert.pem)
+return 1
+
+logger.info(done the server certificates job)
+

[libvirt] [PATCH] daemon: Unlink unix sockets at daemon shutdown

2011-07-27 Thread Osier Yang

---
 daemon/libvirtd.c |6 ++
 1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c
index 9e044e2..61f5a2d 100644
--- a/daemon/libvirtd.c
+++ b/daemon/libvirtd.c
@@ -1572,6 +1572,12 @@ cleanup:
 if (pid_file)
 unlink (pid_file);
 
+if (sock_file  sock_file[0] != '@')
+unlink(sock_file);
+
+if (sock_file_ro  sock_file_ro[0] != '@')
+unlink(sock_file_ro);
+
 VIR_FREE(sock_file);
 VIR_FREE(sock_file_ro);
 VIR_FREE(pid_file);
-- 
1.7.6

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH] daemon: Unlink unix sockets at daemon shutdown

2011-07-27 Thread Osier Yang

于 2011年07月27日 18:07, Osier Yang 写道:
 ---
  daemon/libvirtd.c |6 ++
  1 files changed, 6 insertions(+), 0 deletions(-)

 diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c
 index 9e044e2..61f5a2d 100644
 --- a/daemon/libvirtd.c
 +++ b/daemon/libvirtd.c
 @@ -1572,6 +1572,12 @@ cleanup:
  if (pid_file)
  unlink (pid_file);
  
 +if (sock_file  sock_file[0] != '@')
 +unlink(sock_file);
 +
 +if (sock_file_ro  sock_file_ro[0] != '@')
 +unlink(sock_file_ro);
 +

With this patch, the socket files could be removed successfully.

But it looks to me virNetSocketFree already unlink the socket files,
it's weired
the unix socket files still exists after daemon shutdown. So, this patch
might be
not quite right.

  VIR_FREE(sock_file);
  VIR_FREE(sock_file_ro);
  VIR_FREE(pid_file);

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH] daemon: Unlink unix sockets at daemon shutdown

2011-07-27 Thread Daniel P. Berrange

On Wed, Jul 27, 2011 at 05:34:19PM +0800, Osier Yang wrote:
 于 2011年07月27日 18:07, Osier Yang 写道:
  ---
   daemon/libvirtd.c |6 ++
   1 files changed, 6 insertions(+), 0 deletions(-)
 
  diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c
  index 9e044e2..61f5a2d 100644
  --- a/daemon/libvirtd.c
  +++ b/daemon/libvirtd.c
  @@ -1572,6 +1572,12 @@ cleanup:
   if (pid_file)
   unlink (pid_file);
   
  +if (sock_file  sock_file[0] != '@')
  +unlink(sock_file);
  +
  +if (sock_file_ro  sock_file_ro[0] != '@')
  +unlink(sock_file_ro);
  +

NACK to this - cleanup should be done in the RPC classes

 With this patch, the socket files could be removed successfully.
 
 But it looks to me virNetSocketFree already unlink the socket files,
 it's weired
 the unix socket files still exists after daemon shutdown. So, this patch
 might be
 not quite right.

Yes, that suggests that virNetServerFree or something that
it calls is not cleaning up properly. It could be a missing
free somewhere, or a reference being held open.

It might be desirable to add an explicit virNetServerClose()
API which closes all sockets  unlinks them, separately
from virNetServerFree, so we ensure cleanup even in the event
of a reference count being leaked

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] Fix for newer yajl

2011-07-27 Thread Brecht Sanders


On 27/07/2011 10:24, Brecht Sanders wrote:



Hi,
I just notices that compiling libvirt against yail version 2 there
were some errors, mainly because yajl_parser_config is no longer
used.

What version of libvirt is your patch against ?  The latest releases
already have fixed compatibility with YAJL 2, so I'm assuming you have
an old release ?

Regards,
Daniel

Hmm, you're right. My bad.
I forgot I was compiling 0.9.0 because I had other issues compiling 
later versions under MinGW/MSYS on Windows.
In case you were not aware of this I'm including the end of my 0.9.3 
output below.

Please ignore my previous mail.
I just noticed this was caused by -Dsiginfo_t=int which I still had in 
there to fix older compiles.

Removing this works fine.

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH] daemon: Unlink unix sockets at daemon shutdown

2011-07-27 Thread Osier Yang


于 2011年07月27日 17:51, Daniel P. Berrange 写道:

On Wed, Jul 27, 2011 at 05:34:19PM +0800, Osier Yang wrote:

于 2011年07月27日 18:07, Osier Yang 写道:

---
  daemon/libvirtd.c |6 ++
  1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c
index 9e044e2..61f5a2d 100644
--- a/daemon/libvirtd.c
+++ b/daemon/libvirtd.c
@@ -1572,6 +1572,12 @@ cleanup:
  if (pid_file)
  unlink (pid_file);

+if (sock_file  sock_file[0] != '@')
+unlink(sock_file);
+
+if (sock_file_ro  sock_file_ro[0] != '@')
+unlink(sock_file_ro);
+

NACK to this - cleanup should be done in the RPC classes


With this patch, the socket files could be removed successfully.

But it looks to me virNetSocketFree already unlink the socket files,
it's weired
the unix socket files still exists after daemon shutdown. So, this patch
might be
not quite right.

Yes, that suggests that virNetServerFree or something that
it calls is not cleaning up properly. It could be a missing
free somewhere, or a reference being held open.



It looks to me the ref is right from the debug log, (not missed 
somewehre).



It might be desirable to add an explicit virNetServerClose()
API which closes all sockets  unlinks them, separately
from virNetServerFree, so we ensure cleanup even in the event
of a reference count being leaked


Yes, agree.


Daniel


--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] RFC: API additions for enhanced snapshot support

2011-07-27 Thread Stefan Hajnoczi

On Thu, Jun 16, 2011 at 6:41 AM, Eric Blake ebl...@redhat.com wrote:
 Right now, libvirt has a snapshot API via virDomainSnapshotCreateXML,
 but for qemu domains, it only works if all the guest disk images are
 qcow2, and qemu rather than libvirt does all the work.  However, it has
 a couple of drawbacks: it is inherently tied to domains (there is no way
 to manage snapshots of storage volumes not tied to a domain, even though
 libvirt does that for qcow2 images associated with offline qemu domains
 by using the qemu-img application).  And it necessarily operates on all
 of the images associated with a domain in parallel - if any disk image
 is not qcow2, the snapshot fails, and there is no way to select a subset
 of disks to save.  However, it works on both active (disk and memory
 state) and inactive domains (just disk state).

Hi Eric,
Any updates on your proposed snapshot API enhancements?

The use case I am particularly interested in is a backup solution
using libvirt snapshot APIs to take consistent backups of guests.  The
two workflows are reading out full snapshots of disk images (full
backup) and reading only those blocks that have changed since the last
backup (incremental backup).

Incremental backups can be done just like full backups except with an
API call to get a dirty blocks list.  The client only reads out those
dirty blocks from the snapshot.

Stefan

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [v2] qemu: Improve docs for virsh dump format

2011-07-27 Thread Osier Yang

The error in getCompressionType will never be reported, change
the errors codes into warning (VIR_WARN(%s, _(foo)); doesn't break
syntax-check rule), and also improve the docs in qemu.conf to tell
user the truth.
---
 src/qemu/qemu.conf |   10 --
 src/qemu/qemu_driver.c |   15 ---
 2 files changed, 16 insertions(+), 9 deletions(-)

diff --git a/src/qemu/qemu.conf b/src/qemu/qemu.conf
index 145062c..7de6863 100644
--- a/src/qemu/qemu.conf
+++ b/src/qemu/qemu.conf
@@ -196,8 +196,14 @@
 # saving a domain in order to save disk space; the list above is in descending
 # order by performance and ascending order by compression ratio.
 #
-# save_image_format is used when you use 'virsh save' at scheduled saving.
-# dump_image_format is used when you use 'virsh dump' at emergency crashdump.
+# save_image_format is used when you use 'virsh save' at scheduled
+# saving, and it is an error if the specified save_image_format is
+# not valid, or the according compression program can't be found.
+#
+# dump_image_format is used when you use 'virsh dump' at emergency
+# crashdump, and if the specified dump_image_format is not valid, or
+# the according compression program can't be found, this falls
+# back to raw compression.
 #
 # save_image_format = raw
 # dump_image_format = raw
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 0066c55..e9bd421 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -2712,17 +2712,18 @@ getCompressionType(struct qemud_driver *driver)
  */
 if (driver-dumpImageFormat) {
 compress = qemudSaveCompressionTypeFromString(driver-dumpImageFormat);
+/* Use raw as the format if the specified format is not valid,
+ * or the compress program is not available,
+ */
 if (compress  0) {
-qemuReportError(VIR_ERR_OPERATION_FAILED, %s,
-_(Invalid dump image format specified in 
-  configuration file, using raw));
+VIR_WARN(%s, _(Invalid dump image format specified in 
+ configuration file, using raw));
 return QEMUD_SAVE_FORMAT_RAW;
 }
 if (!qemudCompressProgramAvailable(compress)) {
-qemuReportError(VIR_ERR_OPERATION_FAILED,
-%s, _(Compression program for dump image format 

-in configuration file isn't available, 
-using raw));
+VIR_WARN(%s, _(Compression program for dump image format 
+ in configuration file isn't available, 
+ using raw));
 return QEMUD_SAVE_FORMAT_RAW;
 }
 }
-- 
1.7.6

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [libvirt-test-API][PATCH V2] add remoteAccess directory and tls authentication testcase

2011-07-27 Thread Guannan Sun

ACK

Thanks!
Best Regards!
Wayne Sun
Redhat QE, Beijing, China
+86-10-6260-8238

- Original Message -
From: Guannan Ren g...@redhat.com
To: libvir-list@redhat.com
Sent: Wednesday, July 27, 2011 5:31:10 PM
Subject: [libvirt] [libvirt-test-API][PATCH V2] add remoteAccess directory  
and tls authentication testcase

---
 repos/remoteAccess/tls_setup.py |  386 +++
 1 files changed, 386 insertions(+), 0 deletions(-)
 create mode 100644 repos/remoteAccess/__init__.py
 create mode 100644 repos/remoteAccess/tls_setup.py

diff --git a/repos/remoteAccess/__init__.py b/repos/remoteAccess/__init__.py
new file mode 100644
index 000..e69de29
diff --git a/repos/remoteAccess/tls_setup.py b/repos/remoteAccess/tls_setup.py
new file mode 100644
index 000..6d0b27c
--- /dev/null
+++ b/repos/remoteAccess/tls_setup.py
@@ -0,0 +1,386 @@
+#!/usr/bin/env python
+ Setup tls authentication between two hosts and configure libvirt
+to use it for connection.
+remoteAccess:tls_setup
+target_machine
+xx.xx.xx.xx
+username
+root
+password
+xx
+pkipath(optional)
+/tmp/pkipath
+
+
+__author__ = 'Guannan Ren: g...@redhat.com'
+__date__ = 'Mon July 25, 2011'
+__version__ = '0.1.0'
+__credits__ = 'Copyright (C) 2011 Red Hat, Inc.'
+__all__ = ['usage']
+
+import os
+import re
+import sys
+import pexpect
+import string
+import commands
+import shutil
+
+def append_path(path):
+Append root path of package
+if path in sys.path:
+pass
+else:
+sys.path.append(path)
+
+pwd = os.getcwd()
+result = re.search('(.*)libvirt-test-API', pwd)
+append_path(result.group(0))
+
+from lib import connectAPI
+from utils.Python import utils
+from exception import LibvirtAPI
+
+CERTTOOL = /usr/bin/certtool
+CP = /bin/cp
+MKDIR = /bin/mkdir
+CA_FOLDER = /etc/pki/CA
+PRIVATE_KEY_FOLDER = /etc/pki/libvirt/private
+CERTIFICATE_FOLDER = /etc/pki/libvirt
+
+TEMP_TLS_FOLDER = /tmp/libvirt_test_API_tls
+CAKEY = os.path.join(TEMP_TLS_FOLDER, 'cakey.pem')
+CACERT = os.path.join(TEMP_TLS_FOLDER, 'cacert.pem')
+SERVERKEY = os.path.join(TEMP_TLS_FOLDER, 'serverkey.pem')
+SERVERCERT = os.path.join(TEMP_TLS_FOLDER, 'servercert.pem')
+CLIENTKEY = os.path.join(TEMP_TLS_FOLDER, 'clientkey.pem')
+CLIENTCERT = os.path.join(TEMP_TLS_FOLDER, 'clientcert.pem')
+
+def check_params(params):
+check out the arguments requried for migration
+logger = params['logger']
+keys = ['target_machine', 'username', 'password']
+for key in keys:
+if key not in params:
+logger.error(Argument %s is required % key)
+return 1
+return 0
+
+def CA_setting_up(util, logger):
+ setting up a Certificate Authority 
+# Create a private key for CA
+logger.info(generate CA certificates)
+
+cakey_fd = open(CAKEY, 'w')
+ret, out = util.exec_cmd([CERTTOOL, '--generate-privkey'], 
outfile=cakey_fd)
+cakey_fd.close()
+if ret != 0:
+logger.error(failed to create CA private key)
+return 1
+
+
+# ca.info file
+cainfo = os.path.join(TEMP_TLS_FOLDER, 'ca.info')
+cainfo_fd = open(cainfo, 'w')
+cainfo_str = cn = Libvirt_test_API\n  + \
+ ca\n + \
+ cert_signing_key\n
+
+cainfo_fd.write(cainfo_str)
+cainfo_fd.close()
+
+# Generate cacert.pem
+cacert_args = [CERTTOOL, '--generate-self-signed', '--load-privkey', 
CAKEY, '--template', cainfo]
+cacert_fd = open(CACERT, 'w')
+ret, out = util.exec_cmd(cacert_args, outfile=cacert_fd)
+cacert_fd.close()
+if ret != 0:
+logger.error(failed to create cacert.pem)
+return 1
+
+logger.info(done the CA certificates job)
+return 0
+
+def tls_server_cert(target_machine, util, logger):
+ generating server certificates 
+# Create tls server key
+logger.info(generate server certificates)
+
+serverkey_fd = open(SERVERKEY, 'w')
+ret, out = util.exec_cmd([CERTTOOL, '--generate-privkey'], 
outfile=serverkey_fd)
+serverkey_fd.close()
+if ret != 0:
+logger.error(failed to create server key)
+return 1
+
+# server.info
+serverinfo = os.path.join(TEMP_TLS_FOLDER, 'server.info')
+serverinfo_fd = open(serverinfo, 'w')
+serverinfo_str = organization = Libvirt_test_API\n + \
+ cn = %s\n % target_machine+ \
+ tls_www_server\n  + \
+ encryption_key\n  + \
+ signing_key\n
+
+serverinfo_fd.write(serverinfo_str)
+serverinfo_fd.close()
+
+# Generate servercert.pem
+servercert_args = [CERTTOOL,
+   '--generate-certificate',
+   '--load-privkey', SERVERKEY,
+   '--load-ca-certificate', CACERT,
+   '--load-ca-privkey', CAKEY,
+   '--template',

Re: [libvirt] [Qemu-devel] [PATCH v3] Add support for fd: protocol

2011-07-27 Thread Corey Bryant




On 07/27/2011 04:43 AM, Daniel P. Berrange wrote:

On Wed, Jul 27, 2011 at 10:36:25AM +0200, Kevin Wolf wrote:

Am 27.07.2011 10:22, schrieb Daniel P. Berrange:

On Wed, Jul 27, 2011 at 10:11:06AM +0200, Kevin Wolf wrote:

Am 26.07.2011 18:57, schrieb Corey Bryant:

  diff --git a/block/cow.c b/block/cow.c
  index 4cf543c..e17f8e7 100644
  --- a/block/cow.c
  +++ b/block/cow.c
  @@ -82,6 +82,11 @@ static int cow_open(BlockDriverState *bs, int flags)
pstrcpy(bs-backing_file, sizeof(bs-backing_file),
cow_header.backing_file);

  +if (bs-backing_file[0] != '\0'   bdrv_is_fd_protocol(bs)) {
  +/* backing file currently not supported by fd: protocol */
  +goto fail;
  +}

I don't think these checks are strictly needed. Without the check you
can open the image itself using an fd, and the backing file using good
old raw-posix. We shouldn't decide for our users that this isn't useful.

In any case, it would have to move into block.c, you can't modify
independent drivers for this.



I understand the point on not modifying independent drivers.

But if the backing file resides on NFS, wouldn't the the proposed
SELinux changes prevent the open?


Probably. But what about cases where the backing file is local? Or even
a non-libvirt, non-SELinux use case?


Or are you talking about support where libvirt opens the backing file
and passes the fd to Qemu?  If so there was some discussion about future
support for this here:
http://lists.gnu.org/archive/html/qemu-devel/2011-06/msg01496.html


Not really, but implementing this will be a bit easier if you don't
forbid using backing files with fd.


In any case, for 'fd:' to be actually used by libvirt, we need to have
backing files supported. There are major users of libvirt who rely on
NFS and also use backing files, so an 'fd:' impl which can't deal with
the backing file problem isn't much use to libvirt.


I'm perfectly aware of that. But seriously, repeating it over and over
again isn't going to make it happen any sooner. It requires -blockdev
which we may or may not have by 1.0.


Yes, I understand we need also -blockdev for this. Other messages in this
mail thread have impied that this proposed patch on its own is useful for
libvirt's requirements, so I just wanted to remind people in general that
we can't use this patch on its own until we have something like -blockdev.

Regards,
Daniel


Kevin/Daniel, thanks a lot for your input.

In terms of the support that libvirt requires, I just want to make sure 
all bases are covered.


In order for this support to be useful to libvirt, the following are 
required (sorry if this is repetitive):


1) -blockdev (backing file support)
2) savevm (snapshot support)
3) snapshot_blkdev (snapshot support)
4) 'change' monitor command
5) -cdrom

and as far as I know, the status of the above is:

1) Someone is slated to work on this (not me)
2) I need to figure out how to re-open file without an open (me)
3) This will be covered by live snapshots feature (not me)
4) Should just work as is (me)
5) May also just work as is (me)

In other words, 1 and 3 will not be implemented by me (except perhaps 
some re-usable infrastructure).  Could you confirm my understanding?


Regards,
Corey

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 00/10] Rollback migration when libvirtd restarts

2011-07-27 Thread Daniel P. Berrange

On Tue, Jul 19, 2011 at 02:27:29AM +0200, Jiri Denemark wrote:
 This is the rest of the original 19 patch series updated with some bugfixes
 and rebased on current master, which is also available at
 
 https://gitorious.org/~jirka/libvirt/jirka-staging/commits/migration-recovery
 
 I didn't manage to run this through the libvirt-tck migration test but I did
 some additional testing with the following combinations of libvirt (current
 contains this series):

I've tested many combinations now with the TCK and it all passes, so
an ACK for this series from me. I also tested this with Eric's suggested
additions.

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] Memory leak of the remoteDomainSet* functions

2011-07-27 Thread Osier Yang

Hello, there

Per bug https://bugzilla.redhat.com/show_bug.cgi?id=725322, should
we change the remoteDomainSet* functions into skipgen, and fix the
leaks like below? (NB, new VIR_FREE statements)

static int
remoteDomainSetBlkioParameters(virDomainPtr dom,
virTypedParameterPtr params,
int nparams,
unsigned int flags)
{
int rv = -1;
int i;
struct private_data *priv = dom-conn-privateData;
remote_domain_set_blkio_parameters_args args;
remote_domain_set_blkio_parameters_args args;

remoteDriverLock(priv);

make_nonnull_domain(args.dom, dom);
args.flags = flags;

if (remoteSerializeTypedParameters(params,
nparams,
args.params.params_val,
args.params.params_len)  0) {
for (i = 0; i  nparams; i++) {
VIR_FREE(args.params.params_val[i].field);
}
VIR_FREE(args.params.params_val);
xdr_free((xdrproc_t)xdr_remote_domain_set_blkio_parameters_args, (char
*)args);
goto done;
}

if (call(dom-conn, priv, 0, REMOTE_PROC_DOMAIN_SET_BLKIO_PARAMETERS,
(xdrproc_t)xdr_remote_domain_set_blkio_parameters_args, (char *)args,
(xdrproc_t)xdr_void, (char *)NULL) == -1) {
goto done;
}

rv = 0;

done:
if (args.params.params_val) {
for (i = 0; i  nparams; i++) {
VIR_FREE(args.params.params_val[i].field);
}
VIR_FREE(args.params.params_val);
}
remoteDriverUnlock(priv);
return rv;
}

Regards,
Osier

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] RFC: API additions for enhanced snapshot support

2011-07-27 Thread Eric Blake


On 07/27/2011 04:04 AM, Stefan Hajnoczi wrote:

On Thu, Jun 16, 2011 at 6:41 AM, Eric Blakeebl...@redhat.com  wrote:

Right now, libvirt has a snapshot API via virDomainSnapshotCreateXML,
but for qemu domains, it only works if all the guest disk images are
qcow2, and qemu rather than libvirt does all the work.  However, it has
a couple of drawbacks: it is inherently tied to domains (there is no way
to manage snapshots of storage volumes not tied to a domain, even though
libvirt does that for qcow2 images associated with offline qemu domains
by using the qemu-img application).  And it necessarily operates on all
of the images associated with a domain in parallel - if any disk image
is not qcow2, the snapshot fails, and there is no way to select a subset
of disks to save.  However, it works on both active (disk and memory
state) and inactive domains (just disk state).


Hi Eric,
Any updates on your proposed snapshot API enhancements?


I still need to post a v2 RFC that gives the XML changes needed to 
support disk snapshots on top of virDomainSnapshotCreateXML.


Meanwhile, I still want to add additional API to make it easier to 
manage offline storage volume snapshots (storage volumes that are not in 
use by a defined or running domain), although it obviously won't happen 
by 0.9.4.




The use case I am particularly interested in is a backup solution
using libvirt snapshot APIs to take consistent backups of guests.  The
two workflows are reading out full snapshots of disk images (full
backup) and reading only those blocks that have changed since the last
backup (incremental backup).


Full backup should be supported via virDomainSnapshotCreateXML, once I 
have the new XML in place, by using the new qemu snapshot_blkdev monitor 
commands.




Incremental backups can be done just like full backups except with an
API call to get a dirty blocks list.  The client only reads out those
dirty blocks from the snapshot.


Incremental backups will need more work - qemu does not yet have the 
monitor commands for exposing which blocks are dirty.  Of course, as 
code is written for qemu, it would be nice to also be thinking about how 
to support that in libvirt, so once I do propose my XML changes for full 
snapshots, it would be nice to remember in your review to think about 
whether it remains extensible to the incremental case.


--
Eric Blake   ebl...@redhat.com+1-801-349-2682
Libvirt virtualization library http://libvirt.org

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH] build: support warnings on RHEL 5

2011-07-27 Thread Eric Blake


On 07/27/2011 02:03 AM, Daniel P. Berrange wrote:

On Tue, Jul 26, 2011 at 02:25:40PM -0600, Eric Blake wrote:

Without this, a configure built by autoconf 2.59 was broken when
trying to detect which compiler warning flags were supported.

* .gnulib: Update to latest, for warnings.m4 fix.
* src/qemu/qemu_conf.c (includes): Drop unused include.
* src/uml/uml_conf.c (include): Likewise.
Reported by Daniel P. Berrange.



ACK


Thanks; pushed.

--
Eric Blake   ebl...@redhat.com+1-801-349-2682
Libvirt virtualization library http://libvirt.org

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] Memory leak of the remoteDomainSet* functions

2011-07-27 Thread Matthias Bolte

2011/7/27 Osier Yang jy...@redhat.com:
 Hello, there

 Per bug https://bugzilla.redhat.com/show_bug.cgi?id=725322, should
 we change the remoteDomainSet* functions into skipgen, and fix the
 leaks like below? (NB, new VIR_FREE statements)

Why not fix the generator instead?

 static int
 remoteDomainSetBlkioParameters(virDomainPtr dom,
 virTypedParameterPtr params,
 int nparams,
 unsigned int flags)
 {
 int rv = -1;
 int i;
 struct private_data *priv = dom-conn-privateData;
 remote_domain_set_blkio_parameters_args args;
 remote_domain_set_blkio_parameters_args args;

 remoteDriverLock(priv);

 make_nonnull_domain(args.dom, dom);
 args.flags = flags;

 if (remoteSerializeTypedParameters(params,
 nparams,
 args.params.params_val,
 args.params.params_len)  0) {
 for (i = 0; i  nparams; i++) {
 VIR_FREE(args.params.params_val[i].field);
 }
 VIR_FREE(args.params.params_val);

This loop should not be needed as remoteSerializeTypedParameters
cleans up in the error case.

 xdr_free((xdrproc_t)xdr_remote_domain_set_blkio_parameters_args, (char
 *)args);
 goto done;
 }

 if (call(dom-conn, priv, 0, REMOTE_PROC_DOMAIN_SET_BLKIO_PARAMETERS,
 (xdrproc_t)xdr_remote_domain_set_blkio_parameters_args, (char *)args,
 (xdrproc_t)xdr_void, (char *)NULL) == -1) {
 goto done;
 }

 rv = 0;

 done:
 if (args.params.params_val) {
 for (i = 0; i  nparams; i++) {
 VIR_FREE(args.params.params_val[i].field);
 }
 VIR_FREE(args.params.params_val);
 }
 remoteDriverUnlock(priv);
 return rv;
 }

This can be fixed in the generator, but why no use xdr_free here
instead of open coding the free loop?

-- 
Matthias Bolte
http://photron.blogspot.com

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] FW: Error configuring my Virtual machine network

2011-07-27 Thread Matthew Omogbai Ehiwere



I am a newbie to linux and was given a task to fine out what could be wrong 
with the configuration of the software we are working on.
The error displayed on my screen is as capture below.Can you help me out.

ncftool define netmask.xml
xmlXPathCompiledEval: evaluation failed
error: XSLT transformation failed
error: runtime error: file /usr/share/netcf/xml/suse-get.xsl line 204 element 
node


NB.

the content of my xml file is as shown bellow.What i noticed is whenever the 
prefix goes above 24,that is when the error comes up.but below 24 it works well.

thanks

interface type=ethernet name=eth5
start mode=onboot/
mac address=00:21:28:44:E3:2B/
protocol family=ipv4
ip address=204.10.0.25 prefix=26/
/protocol
/interface
~
~
~
~
~
~
~
~
~
~
~
~
~
~
~
~
netmask.xml 7 lines, 194 characters





Ehiwere matthew omogbai
Graduate Student,
Computer Networks Engineering.
Sweden
Private: 0046736942168
email:ehiw...@kth.semailto:ehiw...@kth.se
MOTTO:The past is my heritage,
 The present is my responsibility
 The future is my challenge

  Chose to stand,not because there is no seat but because I want to be 
counted
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [Qemu-devel] [PATCH v3] Add support for fd: protocol

2011-07-27 Thread Kevin Wolf

Am 27.07.2011 15:09, schrieb Corey Bryant:
 Kevin/Daniel, thanks a lot for your input.
 
 In terms of the support that libvirt requires, I just want to make sure 
 all bases are covered.
 
 In order for this support to be useful to libvirt, the following are 
 required (sorry if this is repetitive):
 
 1) -blockdev (backing file support)
 2) savevm (snapshot support)

savevm is harmless and doesn't need any special treatment.

 3) snapshot_blkdev (snapshot support)
 4) 'change' monitor command
 5) -cdrom
 
 and as far as I know, the status of the above is:
 
 1) Someone is slated to work on this (not me)
 2) I need to figure out how to re-open file without an open (me)
 3) This will be covered by live snapshots feature (not me)
 4) Should just work as is (me)
 5) May also just work as is (me)
 
 In other words, 1 and 3 will not be implemented by me (except perhaps 
 some re-usable infrastructure).  Could you confirm my understanding?

Yes, at least not at this point.

Kevin

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCHv3 1/2] conf: add listen subelement to domain graphics element

2011-07-27 Thread Laine Stump


On 07/27/2011 02:41 AM, Laine Stump wrote:

On 07/26/2011 07:10 PM, Eric Blake wrote:

On 07/26/2011 08:50 AM, Laine Stump wrote:

I think it is a somewhat overkill to have 'autoport' be a setting
per-listen element. I can't imagine people want a fixed port
number on one IP addr, but a dynamic port number on another IP
addr. So we could just keep that on the top level element.


Although I agree with you for config purposes, it looks to me like the
real use of autoport is that in the live XML it allows differentiating
between ports that were manually specified and those that were
automatically allocated (really, that seems like its main purpose, 
since

simply not giving a port also implies autoport). If we have only a
single autoport attribute for all the listens, we'll have to put in
extra code that makes sure if they specify port for one listen, they do
it for all of them.


Is it that hard to add that additional validation?



Anything can be done, given enough time to think, code, and test all 
the corner cases. I don't see where the gain is in this case, though. 
I see autoport more as an attribute that's useful when examining live 
XML than as something useful to set in the config. Having a separate 
autoport for each port is then not overkill, but just natural. Trying 
to use a single autoport to indicate whether all of a set of ports 
were generated automatically or were manually configured is what's 
overkill - you're going to a lot of trouble to enforce an unnatural 
restriction for no practical gain.


Maybe the best thing is to only allow autoport in listen when (flags 
 INACTIVE) is false (live XML). This would mean that, as far as 
config was concerned, listen would *never* have any autoport in it 
(which would be fine with me); it would however still be in the data 
structure, and still output in live XML.




After more thinking, her eare two possible plans:


1) see above - the autoport attribute will only show up (or be 
recognized) for live XML; when writing the configuration, lack of a port 
will imply autoport=yes, and presence of port will imply autoport=no.


2) remove autoport from listen and store it in graphics. During the 
parsing of graphics, after all the listens are parsed, loop through 
them and verify that either all of them have a port, or none of them do 
(inactive XML only - for live XML they actually all should have a port).


Votes?

(After looking through the code again, I'm completely willing to 
implement (2), just want to make sure that's really what you want.)


--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCHv3 1/2] conf: add listen subelement to domain graphics element

2011-07-27 Thread Daniel P. Berrange

On Wed, Jul 27, 2011 at 11:20:42AM -0400, Laine Stump wrote:
 On 07/27/2011 02:41 AM, Laine Stump wrote:
 On 07/26/2011 07:10 PM, Eric Blake wrote:
 On 07/26/2011 08:50 AM, Laine Stump wrote:
 I think it is a somewhat overkill to have 'autoport' be a setting
 per-listen element. I can't imagine people want a fixed port
 number on one IP addr, but a dynamic port number on another IP
 addr. So we could just keep that on the top level element.
 
 Although I agree with you for config purposes, it looks to me like the
 real use of autoport is that in the live XML it allows differentiating
 between ports that were manually specified and those that were
 automatically allocated (really, that seems like its main
 purpose, since
 simply not giving a port also implies autoport). If we have only a
 single autoport attribute for all the listens, we'll have to put in
 extra code that makes sure if they specify port for one listen, they do
 it for all of them.
 
 Is it that hard to add that additional validation?
 
 
 Anything can be done, given enough time to think, code, and test
 all the corner cases. I don't see where the gain is in this case,
 though. I see autoport more as an attribute that's useful when
 examining live XML than as something useful to set in the config.
 Having a separate autoport for each port is then not overkill, but
 just natural. Trying to use a single autoport to indicate whether
 all of a set of ports were generated automatically or were
 manually configured is what's overkill - you're going to a lot of
 trouble to enforce an unnatural restriction for no practical gain.
 
 Maybe the best thing is to only allow autoport in listen when
 (flags  INACTIVE) is false (live XML). This would mean that, as
 far as config was concerned, listen would *never* have any
 autoport in it (which would be fine with me); it would however
 still be in the data structure, and still output in live XML.
 
 
 After more thinking, her eare two possible plans:
 
 
 1) see above - the autoport attribute will only show up (or be
 recognized) for live XML; when writing the configuration, lack of a
 port will imply autoport=yes, and presence of port will imply
 autoport=no.

This would be a change in behaviour - currently 'autoport' attribute
is always present when we output XML, even for inactive guests, so
there is a consistent way to determine if autoport is in use.

 2) remove autoport from listen and store it in graphics. During
 the parsing of graphics, after all the listens are parsed, loop
 through them and verify that either all of them have a port, or none
 of them do (inactive XML only - for live XML they actually all
 should have a port).

One further thought, is should we even store 'port' on the listen
element. I know technically this lets you configure different
port numbers for different interfaces, but this feels somewhat
error prone for clients.

eg consider a guest A

listen addr='10.0.0.1' port='5902'
listen addr='192.168.0.1' port='5909'

and guest B

listen addr='10.0.0.1' port='5904'
listen addr='192.168.0.1' port='5902'


And DNS enter 'somehost.example.com' which has two A
records

 somehost.example.com A 10.0.0.1
 somehost.example.com A 192.168.0.1

Connecting to 'somehost.example.com' guest A you need to
be very careful not to accidentally get guest B.

So it has become much more tedious for a client app to connect
to a guest if they have to worry about the fact that a VM may
be listening on different ports for each IP address associated
with a DNS name. Using different ports may sound unlikely, but
it actually could be a fairly common problem if you use 'autoport'
feature.

I feel it is worthwhile for app developer sanity to *not* allow
different port numbers per listen element, only IP addresses
Should we find we need the extra flexibility in the future (unlikely)
then we can still add it to teh XML at that time.

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] [PATCHv3 1/2] conf: add listen subelement to domain graphics element

2011-07-27 Thread Eric Blake


On 07/27/2011 01:47 AM, Laine Stump wrote:

I'm not sure this is right. I think we want to require either
type=address (which implies address=nnn must also be present, and
network=nnn must not be present) or type=network (which implies that
network=nnn must be present, and address=nnn might show up in live XML
dumpxml output to show what was resolved, but is ignored in input
parsing. Also, autoport=yes and port=nnn should be a forbidden
combination.



Actually it's not forbidden, that's really the only practical use I can
see for the autoport attribute - in live XML autoport='yes' port='nnn'
is used to indicate that the port was auto-generated and this time 'nnn'
was used. (and not that it's any indication of what is right or wrong,
but the current RNG file allows it).

...

 Well, existing code doesn't do that. Do no harm.

 Certainly in live XML this is acceptable, and for inactive XML there may
 be cases where someone grabs live XML, modifies it, and sends the
 results back to the parse as config (aka inactive).

Yeah, on thinking more, it would be nice if the rng file accepted all 
live xml (it doesn't, because of other aspects like alias and 
actual, but in general we tend to ignore rather than reject stuff we 
don't recognize).


So maybe your rng is okay as is - it accepts more than what is strictly 
necessary, but as long as it doesn't reject anything that the code 
accepts, we're okay.  The question comes down to whether it is easy to 
make the rng reject stuff that the code rejects, or whether to keep the 
rng simple and over-permissive.




I think if we made autoport a single setting global to all instances of
listen, and tried to prevent simultaneous (global) autoport and (local
to one listen) port, the RNG would be quite unwieldy. (you would have to
have a listenElementsPort and a listenElementsAutoport, and the graphics
RNG would put the two of them in a choice depending on the setting of
autoport)


Interesting point.  Overall, I think it boils down to whether we ever 
envision having more than one listen, where one specified a port and 
the other let the port be chosen automatically.  Conversely, if we start 
strict, we can always relax later (especially since it won't be till 
later that we support multiple listen), whereas if we start relaxed, 
it's harder to tighten things up.  But I'm starting to swing towards 
having autoport remain with listen.



Shouldn't this fail if type is not found? That is, I think type is a
mandatory attribute (must be address or network).



No. It's also possible that no address/network information is given (in
which case type would be default, which ends up as just no type
showing up at all). In that case, the qemu driver has default listen
addresses for VNC and for Spice. So it's completely valid to give a
port, but no address.


Makes sense.

So I think this can be expressed in rng as:

optional
  choice
group
  attribute...type=address
  attribute...address=iporname
/group
group
  attribute...type=network
  attribute...network=name
  optional
attribute...address=ip
  /optional
/group
  /choice
/optional

That is, type is optional, but if present, then it determines whether 
address or network must also be present.  Meanwhile, address can appear 
with either type, but if address appears, then type must appear.








+
+ if (address address[0]) {
+ def-address = address;
+ address = NULL;


Should we reject address if not VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_ADDRESS?



No. Although I haven't found the right place to put it in, it's possible
that the listen type may be network, and the resolved address will be
filled in so that it can show up as status in the live XML. (I actually
originally had exactly that check put in, but removed it for this reason).


Yep, I agree that we want to be able to parse existing live output, even 
if we ignore that parse on INACTIVE.







I learned with my virDomainSaveImageDefineXML patch that parsing must
be symmetric to output format. Since your output function produces
address for network mode on live xml but skips it for inactive, you
_must_ make the parser leave address as NULL on parsing if (flags 
VIR_DOMAIN_XML_INACTIVE) and network mode is detected.



I hadn't thought of that. But should we reject it in that case, or
should we *ignore* it? (Will it ever be the case that someone will feed
live XML back to the parser as config? And should we complain in that
case, or silently ignore the extra, just as we would if it was something
completely unknown to us?) (I usually tend to follow the IETF mantra of
being lenient in what you accept, and strict in what you give out)


In general, we ignore (not reject) live-only information when doing an 
INACTIVE parse.  See for example how alias is treated - the format 
routines only output alias on live images, and when parsing INACTIVE, 
any existing alias is silently ignored.



Possibly, I can modify the formatter to (1) never

Re: [libvirt] [PATCHv3 1/2] conf: add listen subelement to domain graphics element

2011-07-27 Thread Daniel P. Berrange

On Wed, Jul 27, 2011 at 04:37:13PM +0100, Daniel P. Berrange wrote:
 On Wed, Jul 27, 2011 at 11:20:42AM -0400, Laine Stump wrote:
 One further thought, is should we even store 'port' on the listen
 element. I know technically this lets you configure different
 port numbers for different interfaces, but this feels somewhat
 error prone for clients.
 
 eg consider a guest A
 
 listen addr='10.0.0.1' port='5902'
 listen addr='192.168.0.1' port='5909'
 
 and guest B
 
 listen addr='10.0.0.1' port='5904'
 listen addr='192.168.0.1' port='5902'
 
 
 And DNS enter 'somehost.example.com' which has two A
 records
 
  somehost.example.com A 10.0.0.1
  somehost.example.com A 192.168.0.1
 
 Connecting to 'somehost.example.com' guest A you need to
 be very careful not to accidentally get guest B.

It actually gets even worse if you consider a not uncommon
DMZ setup where each host is configured with addresses from
a private range like 10.0.0.x, but all users connect to the
machine using completely different a public IP address. In
such a case there'd be no way to reliably determine which
port to use for a guest when conencting to the public IP
if multiple different ports per VM allowed.

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] [PATCHv3 2/2] qemu: support type=network in domain graphics listen

2011-07-27 Thread Eric Blake


On 07/25/2011 03:00 AM, Laine Stump wrote:

The domain XML now understands thelisten  subelement of its
graphics  element (including when listen type='network'), and the
network driver has an internal API that will turn a network name into
an IP address, so the final logical step is to put the glue into the
qemu driver so that when it is starting up a domain, if it finds
listen type='network' network='xyz'/  in the XML, it will call the
network driver to get an IPv4 address associated with network xyz, and
tell qemu to listen for vnc (or spice) on that address rather than the
default address (localhost).


There may be some fallout to this patch, based on what happens to 1/2, 
but in general:



@@ -4127,7 +4126,37 @@ qemuBuildCommandLine(virConnectPtr conn,
def-graphics[0]-data.vnc.socket);

  } else if (qemuCapsGet(qemuCaps, QEMU_CAPS_VNC_COLON)) {
+const char *listenNetwork;
+const char *listenAddr = NULL;
+char *netAddr = NULL;
  bool escapeAddr;
+int ret;
+
+switch (virDomainGraphicsListenGetType(def-graphics[0], 0)) {
+case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_ADDRESS:
+listenAddr = 
virDomainGraphicsListenGetAddress(def-graphics[0], 0);
+break;
+
+case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_NETWORK:
+listenNetwork = 
virDomainGraphicsListenGetNetwork(def-graphics[0], 0);
+if (!listenNetwork)
+break;
+ret = networkGetNetworkAddress(listenNetwork,netAddr);
+if (ret= -2) {
+qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+%s, _(network-based listen not possible, 

+network driver not present));
+goto error;
+}
+if (ret  0) {
+qemuReportError(VIR_ERR_XML_ERROR,
+_(listen network '%s' had no usable 
address),
+listenNetwork);
+goto error;
+}
+listenAddr = netAddr;
+break;
+}


this looks good.

ACK.

--
Eric Blake   ebl...@redhat.com+1-801-349-2682
Libvirt virtualization library http://libvirt.org

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 00/10] Rollback migration when libvirtd restarts

2011-07-27 Thread Eric Blake


On 07/27/2011 07:29 AM, Daniel P. Berrange wrote:

On Tue, Jul 19, 2011 at 02:27:29AM +0200, Jiri Denemark wrote:

This is the rest of the original 19 patch series updated with some bugfixes
and rebased on current master, which is also available at

https://gitorious.org/~jirka/libvirt/jirka-staging/commits/migration-recovery

I didn't manage to run this through the libvirt-tck migration test but I did
some additional testing with the following combinations of libvirt (current
contains this series):


I've tested many combinations now with the TCK and it all passes, so
an ACK for this series from me. I also tested this with Eric's suggested
additions.


I've now pushed the series, with my squash-in for 3/10, and will be 
shortly submitting a virsh patch to expose the new flag.


--
Eric Blake   ebl...@redhat.com+1-801-349-2682
Libvirt virtualization library http://libvirt.org

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH] virsh: expose change-protection during migration

2011-07-27 Thread Eric Blake

* tools/virsh.c (doMigrate): Add --change-protection flag.
* tools/virsh.pod (migrate): Document it.
---

As promised here:
https://www.redhat.com/archives/libvir-list/2011-July/msg01919.html

 tools/virsh.c   |4 
 tools/virsh.pod |9 +++--
 2 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/tools/virsh.c b/tools/virsh.c
index 113124f..34036f9 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -4830,6 +4830,8 @@ static const vshCmdOptDef opts_migrate[] = {
 {suspend, VSH_OT_BOOL, 0, N_(do not restart the domain on the 
destination host)},
 {copy-storage-all, VSH_OT_BOOL, 0, N_(migration with non-shared storage 
with full disk copy)},
 {copy-storage-inc, VSH_OT_BOOL, 0, N_(migration with non-shared storage 
with incremental copy (same base image shared between source and 
destination))},
+{change-protection, VSH_OT_BOOL, 0,
+ N_(prevent any configuration changes to source until migration ends))},
 {verbose, VSH_OT_BOOL, 0, N_(display the progress of migration)},
 {domain, VSH_OT_DATA, VSH_OFLAG_REQ, N_(domain name, id or uuid)},
 {desturi, VSH_OT_DATA, VSH_OFLAG_REQ, N_(connection URI of the 
destination host as seen from the client(normal migration) or source(p2p 
migration))},
@@ -4906,6 +4908,8 @@ doMigrate (void *opaque)
 if (vshCommandOptBool (cmd, copy-storage-inc))
 flags |= VIR_MIGRATE_NON_SHARED_INC;

+if (vshCommandOptBool (cmd, change-protection))
+flags |= VIR_MIGRATE_CHANGE_PROTECTION;

 if (xmlfile 
 virFileReadAll(xmlfile, 8192, xml)  0)
diff --git a/tools/virsh.pod b/tools/virsh.pod
index e9aaa80..6c3c960 100644
--- a/tools/virsh.pod
+++ b/tools/virsh.pod
@@ -564,8 +564,9 @@ type attribute for the domain element of XML.

 =item Bmigrate [I--live] [I--direct] [I--p2p [I--tunnelled]]
 [I--persistent] [I--undefinesource] [I--suspend] [I--copy-storage-all]
-[I--copy-storage-inc] [I--verbose] Idomain-id Idesturi [Imigrateuri]
-[Idname] [I--timeout Bseconds] [I--xml Bfile]
+[I--copy-storage-inc] [I--change-protection] [I--verbose]
+Idomain-id Idesturi [Imigrateuri] [Idname]
+[I--timeout Bseconds] [I--xml Bfile]

 Migrate domain to another host.  Add I--live for live migration; I--p2p
 for peer-2-peer migration; I--direct for direct migration; or I--tunnelled
@@ -575,6 +576,10 @@ and I--suspend leaves the domain paused on the 
destination host.
 I--copy-storage-all indicates migration with non-shared storage with full
 disk copy, I--copy-storage-inc indicates migration with non-shared storage
 with incremental copy (same base image shared between source and destination).
+I--change-protection enforces that no incompatible configuration changes
+will be made to the domain while the migration is underway; this flag is
+implicitly enabled when supported by the hypervisor, but can be used to
+reject the migration if the hypervisor lacks change protection support.
 I--verbose displays the progress of migration.

 The Idesturi is the connection URI of the destination host, and
-- 
1.7.4.4

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [v2] qemu: Improve docs for virsh dump format

2011-07-27 Thread Eric Blake


On 07/27/2011 04:58 AM, Osier Yang wrote:

The error in getCompressionType will never be reported, change
the errors codes into warning (VIR_WARN(%s, _(foo)); doesn't break
syntax-check rule), and also improve the docs in qemu.conf to tell
user the truth.
---
  src/qemu/qemu.conf |   10 --
  src/qemu/qemu_driver.c |   15 ---
  2 files changed, 16 insertions(+), 9 deletions(-)

diff --git a/src/qemu/qemu.conf b/src/qemu/qemu.conf
index 145062c..7de6863 100644
--- a/src/qemu/qemu.conf
+++ b/src/qemu/qemu.conf
@@ -196,8 +196,14 @@
  # saving a domain in order to save disk space; the list above is in descending
  # order by performance and ascending order by compression ratio.
  #
-# save_image_format is used when you use 'virsh save' at scheduled saving.
-# dump_image_format is used when you use 'virsh dump' at emergency crashdump.
+# save_image_format is used when you use 'virsh save' at scheduled
+# saving, and it is an error if the specified save_image_format is
+# not valid, or the according compression program can't be found.


s/according/requested/


+#
+# dump_image_format is used when you use 'virsh dump' at emergency
+# crashdump, and if the specified dump_image_format is not valid, or
+# the according compression program can't be found, this falls


s/according/requested/


+++ b/src/qemu/qemu_driver.c
@@ -2712,17 +2712,18 @@ getCompressionType(struct qemud_driver *driver)
   */
  if (driver-dumpImageFormat) {
  compress = 
qemudSaveCompressionTypeFromString(driver-dumpImageFormat);
+/* Use raw as the format if the specified format is not valid,
+ * or the compress program is not available,
+ */


s/,/./ at the comment end.

ACK with those nits fixed.

--
Eric Blake   ebl...@redhat.com+1-801-349-2682
Libvirt virtualization library http://libvirt.org

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] libvirt-0.9.1 to 0.9.3-r1: managedsave/save won't start/restore at saved state

2011-07-27 Thread Eric Blake


On 07/27/2011 02:37 AM, Nicolas Sebrecht wrote:

I'm seeing strange behaviour, here.  Any guests saved using both
managedsave and save commands from virsh won't restore at saved state. A
new full boot sequence happen.

- Tested against libvirt v0.9.1, v0.9.2, v0.9.3-r1 (Gentoo)
- Confirmed on three different hosts Gentoo amd64 systems.
- Tested with gentoo and ubuntu guests.
- Nothing relevant in /var/log/libvirt/libvirt.log or
/var/log/libvirt/qemu/dom.log

The state file /var/lib/libvirt/qemu/save/dom.save exists and is
deleted when 'virsh start' is called.

The new boot sequence is confirmed by :
- VNC console checks
- previous screen sessions lost
- uptime

I've open a bug at https://bugs.gentoo.org/show_bug.cgi?id=376333 but
had no answer.

Any idea on what could happen or how to inspect it?


Does /var/log/libvirt/qemu/dom.log show the qemu process getting 
started with the -incoming fd:nnn flag?  While you claim that nothing 
appeared to be relevant in that log, it might actually help to post a 
few lines of it for confirmation.


It's working for me with libvirt 0.9.3 on RHEL 6, so I'm not sure what 
to suggest that you try next.


--
Eric Blake   ebl...@redhat.com+1-801-349-2682
Libvirt virtualization library http://libvirt.org

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [virt-tools-list] virt-manager/libvirt backwards compatibility problem?

2011-07-27 Thread Cole Robinson

On 07/27/2011 12:28 PM, Whit Blauvelt wrote:
 Looks like my real problem may be not incorporating a Debian/Ubuntu patch
 before building 0.9.x, since netcat differs:
 
 https://bugs.launchpad.net/ubuntu/+source/libvirt/+bug/517478
 

Hmm, what was the error message you were getting? We've made efforts at
the libvirt level to have useful error reporting here, so that the cause
of failure is more obvious.

Also, I patched virt-manager a while ago to use a shell script to handle
both RH and debian 'nc' versions. Script is here:

http://git.fedorahosted.org/git?p=virt-manager.git;a=blob;f=src/virtManager/console.py;h=1bdf26b38ba50b95379a664d202af66626436f81;hb=HEAD#l95

Yeah, it's complete and utter crack, but this has been a constant
frustration for users, so maybe worth revisiting at the libvirt level
(and hey, launching nc over ssh is kinda crack to being with :) ).

Even if we solved this in a clean way by providing our own
/usr/libexec/libvirt_nc binary, we'd still probably have to play shell
script games over SSH to handle connecting to older libvirt (or what if
libvirt_nc was installed somewhere other than libexec?)

- Cole


--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [virt-tools-list] virt-manager/libvirt backwards compatibility problem?

2011-07-27 Thread Eric Blake


[adding libvir-list]

On 07/27/2011 10:28 AM, Whit Blauvelt wrote:

Looks like my real problem may be not incorporating a Debian/Ubuntu patch
before building 0.9.x, since netcat differs:

https://bugs.launchpad.net/ubuntu/+source/libvirt/+bug/517478


Reading that bug report, it looks like upstream libvirt.git should be 
patched to add a runtime test on whether a remote nc supports the -q 
option, rather than blindly assuming that 'nc -q' exists on the remote side.


--
Eric Blake   ebl...@redhat.com+1-801-349-2682
Libvirt virtualization library http://libvirt.org

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH ] send-key: Implement Python API

2011-07-27 Thread Eric Blake


On 07/26/2011 04:26 AM, Daniel P. Berrange wrote:

On Thu, Jul 21, 2011 at 05:21:10PM +0800, Lai Jiangshan wrote:


Signed-off-by: Lai Jiangshanla...@cn.fujitsu.com


ACK


Pushed.

--
Eric Blake   ebl...@redhat.com+1-801-349-2682
Libvirt virtualization library http://libvirt.org

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] Add support for image write mode in virtualbox driver

2011-07-27 Thread Boris FELD

Hi everyone,

We currently using libvirt for PYTI GSOC Project (it works very well!)
and we encounter a problem. We use VM with two disk images, one which
must to be snapshoted/rollbacked and the other one which must be not.
We use VirtualBox as hypervisor and to do so, VirtualBox define
Special image write modes
(http://www.virtualbox.org/manual/ch05.html#hdimagewrites). What we
need is to set the type of one image to writethrough.

This information seems defined in VM config file. For instance:
HardDisk uuid={402aef07-4df8-4518-831c-eeb9ae2a32d2}
location=/home/boris/VirtualBox VMs/PYTI.vdi format=VDI
type=Normal/
HardDisk uuid={63acf6d9-b7db-4d8d-8389-1276c3d500bb}
location=/home/boris/VirtualBox VMs/adhock.vdi format=VDI
type=Writethrough/

I found two pages mentioning the type of medium, this one:
http://www.virtualbox.org/sdkref/_virtual_box_8idl.html#b4abb9d1b9e44b997d1d2a809e66191c
and this one: 
http://www.virtualbox.org/sdkref/interface_i_medium_attachment.html.
Hope it will help.

Cheers,
FELD Boris

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] Environmental Variables inside a container

2011-07-27 Thread Devendra K. Modium

Hi All

I am trying to pass an environmental flag to the
LXC container when it is started.I found libvirt_lxc is
already doing it  in lxc_container.c as shown below


static virCommandPtr lxcContainerBuildInitCmd(virDomainDefPtr vmDef)
{
char uuidstr[VIR_UUID_STRING_BUFLEN];
virCommandPtr cmd;

virUUIDFormat(vmDef-uuid, uuidstr);

cmd = virCommandNew(vmDef-os.init);

virCommandAddEnvString(cmd, PATH=/bin:/sbin);
virCommandAddEnvString(cmd, TERM=linux-deva);
virCommandAddEnvPair(cmd, LIBVIRT_LXC_UUID, uuidstr);
virCommandAddEnvPair(cmd, LIBVIRT_LXC_NAME, vmDef-name);

return cmd;
}


But only
TERM flag is getting visible inside the container not remaining.

Does it depends on how /sbin/init is receiving them? Is /sbin/init 
only recognizing TERM flag.
If so what is intention of sending remaining flags in current libvirt.

Please let me know if there is any elegant way to send environmental
variable to container.

Thanks for your time.

Regards
Devendra 

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] Environmental Variables inside a container

2011-07-27 Thread Eric Blake


On 07/27/2011 12:44 PM, Devendra K. Modium wrote:

Hi All

I am trying to pass an environmental flag to the
LXC container when it is started.I found libvirt_lxc is
already doing it  in lxc_container.c as shown below


static virCommandPtr lxcContainerBuildInitCmd(virDomainDefPtr vmDef)
{
 char uuidstr[VIR_UUID_STRING_BUFLEN];
 virCommandPtr cmd;

 virUUIDFormat(vmDef-uuid, uuidstr);

 cmd = virCommandNew(vmDef-os.init);

 virCommandAddEnvString(cmd, PATH=/bin:/sbin);
 virCommandAddEnvString(cmd, TERM=linux-deva);
 virCommandAddEnvPair(cmd, LIBVIRT_LXC_UUID, uuidstr);
 virCommandAddEnvPair(cmd, LIBVIRT_LXC_NAME, vmDef-name);


Yes, this list is hard-coded, and only guarantees to affect the 
environment of the process at the root of the lxc process tree.



But only
TERM flag is getting visible inside the container not remaining.

Does it depends on how /sbin/init is receiving them? Is /sbin/init
only recognizing TERM flag.


Probably something in your lxc process hierarchy is scrubbing 
environment variables prior to starting additional processes, but that's 
out of libvirt's control - libvirt can only affect the environment of 
the parent process.



If so what is intention of sending remaining flags in current libvirt.

Please let me know if there is any elegant way to send environmental
variable to container.


Sounds like it might be an interesting feature to enhance the xml for 
lxc domains to allow the specification of additional environment 
variables that should be inherited into the lxc process tree, beyond the 
few variables already hard-coded by libvirt.


--
Eric Blake   ebl...@redhat.com+1-801-349-2682
Libvirt virtualization library http://libvirt.org

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH] rpc: Fix memory leak in remoteDomainSet*Parameters functions

2011-07-27 Thread Matthias Bolte

Add a new helper remoteFreeTypedParameters and teach the generator
to add it to the cleanup section.

https://bugzilla.redhat.com/show_bug.cgi?id=725322
---
 src/remote/remote_driver.c |   22 +-
 src/rpc/gendispatch.pl |5 +
 2 files changed, 22 insertions(+), 5 deletions(-)

diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
index 0652e0d..e5bfa4b 100644
--- a/src/remote/remote_driver.c
+++ b/src/remote/remote_driver.c
@@ -1208,6 +1208,22 @@ done:
 return rv;
 }
 
+/* Helper to free typed parameters. */
+static void
+remoteFreeTypedParameters(remote_typed_param *args_params_val,
+  u_int args_params_len)
+{
+u_int i;
+
+if (args_params_val == NULL)
+return;
+
+for (i = 0; i  args_params_len; i++)
+VIR_FREE(args_params_val[i].field);
+
+VIR_FREE(args_params_val);
+}
+
 /* Helper to serialize typed parameters. */
 static int
 remoteSerializeTypedParameters(virTypedParameterPtr params,
@@ -1264,11 +1280,7 @@ remoteSerializeTypedParameters(virTypedParameterPtr 
params,
 rv = 0;
 
 cleanup:
-if (val) {
-for (i = 0; i  nparams; i++)
-VIR_FREE(val[i].field);
-VIR_FREE(val);
-}
+remoteFreeTypedParameters(val, nparams);
 return rv;
 }
 
diff --git a/src/rpc/gendispatch.pl b/src/rpc/gendispatch.pl
index ceeb1f5..0d344e8 100755
--- a/src/rpc/gendispatch.pl
+++ b/src/rpc/gendispatch.pl
@@ -979,6 +979,7 @@ elsif ($opt_k) {
 my @args_check_list = ();
 my @setters_list = ();
 my @setters_list2 = ();
+my @free_list = ();
 my $priv_src = conn;
 my $priv_name = privateData;
 my $call_args = args;
@@ -1105,6 +1106,7 @@ elsif ($opt_k) {
  
xdr_free((xdrproc_t)xdr_$call-{args}, (char *)args);\n .
  goto done;\n .
  });
+push(@free_list, 
remoteFreeTypedParameters(args.params.params_val, args.params.params_len);\n);
 } elsif ($args_member =~ m/^((?:unsigned )?int) 
(\S+);\s*\/\*\s*call-by-reference\s*\*\//) {
 my $type_name = $1 *;
 my $arg_name = $2;
@@ -1500,6 +1502,9 @@ elsif ($opt_k) {
 
 print \n;
 print done:\n;
+
+print join(\n, @free_list);
+
 print remoteDriverUnlock(priv);\n;
 print return rv;\n;
 print }\n;
-- 
1.7.4.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] Memory leak of the remoteDomainSet* functions

2011-07-27 Thread Matthias Bolte

2011/7/27 Matthias Bolte matthias.bo...@googlemail.com:
 2011/7/27 Osier Yang jy...@redhat.com:
 Hello, there

 Per bug https://bugzilla.redhat.com/show_bug.cgi?id=725322, should
 we change the remoteDomainSet* functions into skipgen, and fix the
 leaks like below? (NB, new VIR_FREE statements)

 Why not fix the generator instead?

An here's the patch that does this

https://www.redhat.com/archives/libvir-list/2011-July/msg01929.html

-- 
Matthias Bolte
http://photron.blogspot.com

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [virt-tools-list] virt-manager/libvirt backwards compatibility problem?

2011-07-27 Thread Guido Günther

On Wed, Jul 27, 2011 at 10:53:01AM -0600, Eric Blake wrote:
 [adding libvir-list]
 
 On 07/27/2011 10:28 AM, Whit Blauvelt wrote:
 Looks like my real problem may be not incorporating a Debian/Ubuntu patch
 before building 0.9.x, since netcat differs:
 
 https://bugs.launchpad.net/ubuntu/+source/libvirt/+bug/517478
 
 Reading that bug report, it looks like upstream libvirt.git should
 be patched to add a runtime test on whether a remote nc supports the
 -q option, rather than blindly assuming that 'nc -q' exists on the
 remote side.
This is the patch we're currently using in Debian for that (based on a
version originally forwarded from Ubuntu):


http://anonscm.debian.org/gitweb/?p=pkg-libvirt/libvirt.git;a=blob;f=debian/patches/Autodetect-if-the-remote-nc-command-supports-the-q-o.patch

I'd be happy to push that since this is the distro specific patch I have
to adjust the most ;)
Cheers,
 -- Guido

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH] rpc: Fix memory leak in remoteDomainSet*Parameters functions

2011-07-27 Thread Eric Blake


On 07/27/2011 01:18 PM, Matthias Bolte wrote:

Add a new helper remoteFreeTypedParameters and teach the generator
to add it to the cleanup section.

https://bugzilla.redhat.com/show_bug.cgi?id=725322
---
  src/remote/remote_driver.c |   22 +-
  src/rpc/gendispatch.pl |5 +
  2 files changed, 22 insertions(+), 5 deletions(-)


ACK.



diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
index 0652e0d..e5bfa4b 100644
--- a/src/remote/remote_driver.c
+++ b/src/remote/remote_driver.c
@@ -1208,6 +1208,22 @@ done:
  return rv;
  }

+/* Helper to free typed parameters. */
+static void
+remoteFreeTypedParameters(remote_typed_param *args_params_val,
+  u_int args_params_len)


Not the typical one-arg free function, so it can't be listed in cfg.mk, 
but that's okay, since it's only used in one file.


--
Eric Blake   ebl...@redhat.com+1-801-349-2682
Libvirt virtualization library http://libvirt.org

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] Environmental Variables inside a container

2011-07-27 Thread Devendra K. Modium

Hi Eric

Thanks for the reply.
I am trying to run standard Ubuntu lxc file System(lxc-ubuntu) and
/sbin/init of Ubuntu is the root process of lxc process tree.

I believe most of the lxc containers start with /sbin/init
as first process.

Since I didn't exactly find the code of /sbin/init, I assumed 
since libvirt_lxc is currently sending environmental flags
to first process(I believe mostly it is /sbin/init) of LXC container
and hence this would be visible inside the container and assumed 
that /sbin/init in general should pass on this environment flags to
child process.

Though I know its not an libvirt issue, if its not passing on environment,
just wanted to know if you have some knowledge about what might going on.

Thanks in advance

Regards
Devendra





- Original Message -
From: Eric Blake ebl...@redhat.com
To: Devendra K. Modium dmod...@isi.edu
Cc: libvir-list@redhat.com
Sent: Wednesday, July 27, 2011 3:13:51 PM
Subject: Re: [libvirt] Environmental Variables inside a container

On 07/27/2011 12:44 PM, Devendra K. Modium wrote:
 Hi All

 I am trying to pass an environmental flag to the
 LXC container when it is started.I found libvirt_lxc is
 already doing it  in lxc_container.c as shown below


 static virCommandPtr lxcContainerBuildInitCmd(virDomainDefPtr vmDef)
 {
  char uuidstr[VIR_UUID_STRING_BUFLEN];
  virCommandPtr cmd;

  virUUIDFormat(vmDef-uuid, uuidstr);

  cmd = virCommandNew(vmDef-os.init);

  virCommandAddEnvString(cmd, PATH=/bin:/sbin);
  virCommandAddEnvString(cmd, TERM=linux-deva);
  virCommandAddEnvPair(cmd, LIBVIRT_LXC_UUID, uuidstr);
  virCommandAddEnvPair(cmd, LIBVIRT_LXC_NAME, vmDef-name);

Yes, this list is hard-coded, and only guarantees to affect the 
environment of the process at the root of the lxc process tree.

 But only
 TERM flag is getting visible inside the container not remaining.

 Does it depends on how /sbin/init is receiving them? Is /sbin/init
 only recognizing TERM flag.

Probably something in your lxc process hierarchy is scrubbing 
environment variables prior to starting additional processes, but that's 
out of libvirt's control - libvirt can only affect the environment of 
the parent process.

 If so what is intention of sending remaining flags in current libvirt.

 Please let me know if there is any elegant way to send environmental
 variable to container.

Sounds like it might be an interesting feature to enhance the xml for 
lxc domains to allow the specification of additional environment 
variables that should be inherited into the lxc process tree, beyond the 
few variables already hard-coded by libvirt.

-- 
Eric Blake   ebl...@redhat.com+1-801-349-2682
Libvirt virtualization library http://libvirt.org

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH] rpc: Fix memory leak in remoteDomainSet*Parameters functions

2011-07-27 Thread Matthias Bolte

2011/7/27 Eric Blake ebl...@redhat.com:
 On 07/27/2011 01:18 PM, Matthias Bolte wrote:

 Add a new helper remoteFreeTypedParameters and teach the generator
 to add it to the cleanup section.

 https://bugzilla.redhat.com/show_bug.cgi?id=725322
 ---
  src/remote/remote_driver.c |   22 +-
  src/rpc/gendispatch.pl     |    5 +
  2 files changed, 22 insertions(+), 5 deletions(-)

 ACK.

Thanks, pushed.

-- 
Matthias Bolte
http://photron.blogspot.com

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH] virsh: expose change-protection during migration

2011-07-27 Thread Matthias Bolte

2011/7/27 Eric Blake ebl...@redhat.com:
 * tools/virsh.c (doMigrate): Add --change-protection flag.
 * tools/virsh.pod (migrate): Document it.
 ---

 As promised here:
 https://www.redhat.com/archives/libvir-list/2011-July/msg01919.html

ACK.

-- 
Matthias Bolte
http://photron.blogspot.com

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] virNetClientPtr leak in remote driver

2011-07-27 Thread Matthias Bolte

doRemoteClose doesn't free the virNetClientPtr and this creates a
260kb leak per remote connection. This happens because
virNetClientFree doesn't remove the last ref, because virNetClientNew
creates the virNetClientPtr with a refcount of 2.

static virNetClientPtr virNetClientNew(virNetSocketPtr sock,
   const char *hostname)
{
[...]
client-refs = 1;
[...]
/* Set up a callback to listen on the socket data */
client-refs++;
if (virNetSocketAddIOCallback(client-sock,
  VIR_EVENT_HANDLE_READABLE,
  virNetClientIncomingEvent,
  client,
  virNetClientEventFree)  0) {
client-refs--;
VIR_DEBUG(Failed to add event watch, disabling events);
}
[...]
}

virNetClientNew adds a ref before calling virNetSocketAddIOCallback
but only removes it when virNetSocketAddIOCallback fails. This seems
wrong too me and the ref should be removed in the success case too.

The same pattern was added in 0302391ee643ad91fdc6d2ecf7e4cf0fc9724516
(Fix race in ref counting when handling RPC jobs)

--- a/src/rpc/virnetserverclient.c
+++ b/src/rpc/virnetserverclient.c
@@ -763,10 +763,12 @@ readmore:

 /* Send off to for normal dispatch to workers */
 if (msg) {
+client-refs++;
 if (!client-dispatchFunc ||
 client-dispatchFunc(client, msg,
client-dispatchOpaque)  0) {
 virNetMessageFree(msg);
 client-wantClose = true;
+client-refs--;
 return;
 }
 }

Again, this seems wrong and the ref should be removed in the success
case here too. Before I spent time to figure out how the refcounting
is supposed to work here I just report it and hope that Dan can easily
fix this.

-- 
Matthias Bolte
http://photron.blogspot.com

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [virt-tools-list] virt-manager/libvirt backwards compatibility problem?

2011-07-27 Thread Eric Blake


On 07/27/2011 01:32 PM, Guido Günther wrote:

On Wed, Jul 27, 2011 at 10:53:01AM -0600, Eric Blake wrote:

[adding libvir-list]

On 07/27/2011 10:28 AM, Whit Blauvelt wrote:

Looks like my real problem may be not incorporating a Debian/Ubuntu patch
before building 0.9.x, since netcat differs:

https://bugs.launchpad.net/ubuntu/+source/libvirt/+bug/517478


Reading that bug report, it looks like upstream libvirt.git should
be patched to add a runtime test on whether a remote nc supports the
-q option, rather than blindly assuming that 'nc -q' exists on the
remote side.

This is the patch we're currently using in Debian for that (based on a
version originally forwarded from Ubuntu):


http://anonscm.debian.org/gitweb/?p=pkg-libvirt/libvirt.git;a=blob;f=debian/patches/Autodetect-if-the-remote-nc-command-supports-the-q-o.patch

I'd be happy to push that since this is the distro specific patch I have
to adjust the most ;)



Pasting from that URL gave awkward results below; can you address my 
comments below, then post a v2 as a proper patch against libvirt.git?



 +++ b/src/rpc/virnetsocket.c

  22 @@ -596,9 +596,30 @@ int virNetSocketNewConnectSSH(const char *nodename,

  23  if (noTTY)

  24  virCommandAddArgList(cmd, -T, -o, BatchMode=yes,

  25   -e, none, NULL);

  26 -virCommandAddArgList(cmd, nodename,

  27 - netcat ? netcat : nc,

  28 - -U, path, NULL);

  29 +

  30 +virCommandAddArgList(cmd, nodename, sh -c, NULL);


Why is sh -c passed as a single argument, separate from the argument 
passed to that shell?  Yes, ssh will join all arguments, then resplit 
the combined argument according to shell rules (I hate the fact that ssh 
duplicates shell word parsing), but we should either be providing a 
single argument to ssh with the entire shell script, or breaking this 
into one argument per word to be joined by ssh (three total), instead of 
doing half and half (two words in one argument, the third word [hairy 
script] in another).



  31 +/*

  32 + * This ugly thing is a shell script to detect availability of

  33 + * the -q option for 'nc': debian and suse based distros need this

  34 + * flag to ensure the remote nc will exit on EOF, so it will go away

  35 + * when we close the VNC tunnel. If it doesn't go away, subsequent

  36 + * VNC connection attempts will hang.


s/VNC tunnel/connection tunnel/
s/VNC connection attempts/connection attempts/

This code is not a VNC tunnel.



  37 + *

  38 + * Fedora's 'nc' doesn't have this option, and apparently defaults

  39 + * to the desired behavior.

  40 + */

  41 +virCommandAddArgFormat(cmd, '%s -q 21 | grep -q \requires an 
argument\;


grep -q is not portable to Solaris, even though it is required by POSIX. 
 Instead, use grep with stdout and stderr redirected to /dev/null.




  42 +   if [ $? -eq 0 ] ; then


Why split the nc probe from the if?  We could use the more compact:

if %s -q 21 | grep \requires an argument\ /dev/null 21; then



  43 +  CMD=\%s -q 0 -U %s\;


I don't like this part.  It is not safe to pass %s as the pathname 
through an additional round of shell parsing, since if the pathname has 
anything that might be construed as shell metacharacters, the parse will 
be destroyed.  To some extent, that is already a pre-existing bug 
(slightly mitigated by the fact that 'path' is under libvirt's control, 
and should not be containing arbitrary characters unless you passed odd 
directory choices to ./configure).  But we aren't even evaluating the 
'netcat' variable for sanity - and that is an arbitrary string given 
completely by user input - sounds like we need an independent patch to 
reject a user-provided netcat that would be misparsed by ssh and any 
extra shell expansions.  Without that, then this patch would need the 
burden of adding an extra level of escaping to match an extra level of 
shell parsing.




  44 +   else

  45 +  CMD=\%s -U %s\;

  46 +   fi;

  47 +   eval \$CMD\;',


Yuck.  We should avoid eval at all costs, since that adds a third(!) 
level of shell parsing on top of the two we're already suffering from 
(ssh and sh -c).  I'd much rather see:


sh -c 'CMD=either empty or -q 0; nc $CMD -U path'

than

sh -c 'CMD=either nc -q 0 -U path or nc -U path; eval $CMD'

By the way, you can avoid one level of shell parsing by using this printf:

sh -c 'CMD=..; \$1\ $CMD -U \$2\' sh %s %s, netcat, path

that is, supply 'netcat' and 'path' to ssh at the same level of quoting 
they have always been used, with the shell script referring only to 
positional parameters rather than doing yet another round of parsing on 
those parameters.  But if we already have to plug the hole of 
user-provided 'netcat' being safe for

Re: [libvirt] [virt-tools-list] virt-manager/libvirt backwards compatibility problem?

2011-07-27 Thread Whit Blauvelt

On Wed, Jul 27, 2011 at 09:32:09PM +0200, Guido Günther wrote:

 This is the patch we're currently using in Debian for that (based on a
 version originally forwarded from Ubuntu):
 
   
 http://anonscm.debian.org/gitweb/?p=pkg-libvirt/libvirt.git;a=blob;f=debian/patches/Autodetect-if-the-remote-nc-command-supports-the-q-o.patch
 
 I'd be happy to push that since this is the distro specific patch I have
 to adjust the most ;)

Somewhere I read a bug report about that not working for Solaris, whose grep
doesn't have a -q flag. Suggestion there was to pipe to /dev/null instead.

Best,
Whit

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [virt-tools-list] virt-manager/libvirt backwards compatibility problem?

2011-07-27 Thread Whit Blauvelt

On Wed, Jul 27, 2011 at 12:46:02PM -0400, Cole Robinson wrote:

 Hmm, what was the error message you were getting? We've made efforts at
 the libvirt level to have useful error reporting here, so that the cause
 of failure is more obvious.

Debug-level error sequence at http://transpect.com/debug/virsh.txt

 Also, I patched virt-manager a while ago to use a shell script to handle
 both RH and debian 'nc' versions. Script is here:
 
 http://git.fedorahosted.org/git?p=virt-manager.git;a=blob;f=src/virtManager/console.py;h=1bdf26b38ba50b95379a664d202af66626436f81;hb=HEAD#l95

That's in the 0.9.0 virt-manager, but fails in my context, which where the
systems I'm trying to manage are 0.8.3 from Ubuntu debs, and I'm on the
latest versions of this all built from tar.

Whit

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [virt-tools-list] virt-manager/libvirt backwards compatibility problem?

2011-07-27 Thread Whit Blauvelt

The error from virt-manager 0.9.0's perspective is:

  Unable to open a connection to the libvirt management daemon.
  
  Libvirt URI is: qemu+ssh://root@192.168.1.65/system
  
  Verify that:
   - The 'libvirtd' daemon has been started  [yes, it is]
  
  
  Cannot recv data: 15:42:02.778: 29831: debug : virCommandHook:1959 : Hook is 
done 0: Connection reset by peer
  
  Traceback (most recent call last):
File /usr/src/virt-manager-0.9.0/src/virtManager/connection.py, line 
1146, in _open_thread
  self.vmm = self._try_open()
File /usr/src/virt-manager-0.9.0/src/virtManager/connection.py, line 
1130, in _try_open
  flags)
File /usr/local/lib/python2.7/dist-packages/libvirt.py, line 102, in 
openAuth
  if ret is None:raise libvirtError('virConnectOpenAuth() failed')
  libvirtError: Cannot recv data: 15:42:02.778: 29831: debug : 
virCommandHook:1959 : Hook is done 0: Connection reset by peer
 
- Whit

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] PCI devices passthough to LXC containers using libvirt

2011-07-27 Thread Devendra K. Modium

Hi All

Please let me know if anyone have given access to
PCI devices for a LXC container.

I have tried getting the xml from 
virsh nodedev-dumpxml pci_device and 
added to the libvirt xml file as shown below

device
  namepci__03_00_0/name
  parentpci__00_03_0/parent
  driver
namenvidia/name
  /driver
  capability type='pci'
domain0/domain
bus3/bus
slot0/slot
function0/function
product id='0x06fd' /
vendor id='0x10de'nVidia Corporation/vendor
  /capability
/device


But it didn't work. I see the logs and it says 
couldn't get physical and virtual functions of these devices with error

get_physical_function_linux:323 : Attempting to get SR IOV physical function 
for device with sysfs path '/sys/devices/pci:00/:00:00.0'
16:48:34.033: 13802: debug : get_sriov_function:270 : Attempting to resolve 
device path from device link '/sys/devices/pci:00/:00:00.0/physfn'
16:48:34.033: 13802: debug : get_sriov_function:274 : SR IOV function link 
'/sys/devices/pci:00/:00:00.0/physfn' does not exist
16:48:34.033: 13802: debug : get_virtual_functions_linux:348 : Attempting to 
get SR IOV virtual functions for devicewith sysfs path 
'/sys/devices/pci:00/:00:00.0'


If anyone got some guidelines how to debug, please let me know.


Thanks in advance

Regards
Devendra





  

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH] virsh: expose change-protection during migration

2011-07-27 Thread Eric Blake


On 07/27/2011 02:00 PM, Matthias Bolte wrote:

2011/7/27 Eric Blakeebl...@redhat.com:

* tools/virsh.c (doMigrate): Add --change-protection flag.
* tools/virsh.pod (migrate): Document it.
---

As promised here:
https://www.redhat.com/archives/libvir-list/2011-July/msg01919.html


ACK.


Thanks; pushed.

--
Eric Blake   ebl...@redhat.com+1-801-349-2682
Libvirt virtualization library http://libvirt.org

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [Qemu-devel] [PATCH v3] Add support for fd: protocol

2011-07-27 Thread Blue Swirl

On Tue, Jul 26, 2011 at 3:51 PM, Corey Bryant cor...@linux.vnet.ibm.com wrote:
 sVirt provides SELinux MAC isolation for Qemu guest processes and their
 corresponding resources (image files). sVirt provides this support
 by labeling guests and resources with security labels that are stored
 in file system extended attributes. Some file systems, such as NFS, do
 not support the extended attribute security namespace, which is needed
 for image file isolation when using the sVirt SELinux security driver
 in libvirt.

 The proposed solution entails a combination of Qemu, libvirt, and
 SELinux patches that work together to isolate multiple guests' images
 when they're stored in the same NFS mount. This results in an
 environment where sVirt isolation and NFS image file isolation can both
 be provided.

 This patch contains the Qemu code to support this solution. I would
 like to solicit input from the libvirt community prior to starting
 the libvirt patch.

 Currently, Qemu opens an image file in addition to performing the
 necessary read and write operations. The proposed solution will move
 the open out of Qemu and into libvirt. Once libvirt opens an image
 file for the guest, it will pass the file descriptor to Qemu via a
 new fd: protocol.

 If the image file resides in an NFS mount, the following SELinux policy
 changes will provide image isolation:

  - A new SELinux boolean is created (e.g. virt_read_write_nfs) to
    allow Qemu (svirt_t) to only have SELinux read and write
    permissions on nfs_t files

  - Qemu (svirt_t) also gets SELinux use permissions on libvirt
    (virtd_t) file descriptors

 Following is a sample invocation of Qemu using the fd: protocol on
 the command line:

    qemu -drive file=fd:4,format=qcow2

 The fd: protocol is also supported by the drive_add monitor command.
 This requires that the specified file descriptor is passed to the
 monitor alongside a prior getfd monitor command.

 There are some additional features provided by certain image types
 where Qemu reopens the image file. All of these scenarios will be
 unsupported for the fd: protocol, at least for this patch:

  - The -snapshot command line option
  - The savevm monitor command
  - The snapshot_blkdev monitor command
  - Use of copy-on-write image files
  - The -cdrom command line option
  - The -drive command line option with media=cdrom
  - The change monitor command

In the earlier discussion, virtio-blk and iSCSI were identified as
interesting protocols to be used with file descriptors in the future.
This patch would bind fd: protocol only to raw file interface to be
used by qcow2 etc. higher levels. I guess with small changes the
protocol could be made selectable. Maybe it's just changing command
line to format=virtio-blk or format=iscsi for those uses, though I
fear there may be a layering violation.

Considering the privilege separation side, this patch seems to be
somewhat orthogonal to that (I don't expect any command line passing
and parsing to happen between QEMU processes) but the low level fd
handling seems useful modulo the comments made by others.


 The thought is that this support can be added in the future, but is
 not required for the initial fd: support.

 This patch was tested with the following formats: raw, cow, qcow,
 qcow2, qed, and vmdk, using the fd: protocol from the command line
 and the monitor. Tests were also run to verify existing file name
 support and qemu-img were not regressed. Non-valid file descriptors,
 fd: without format, snapshot and backing files, and cdrom were also
 tested.

 v2:
  - Add drive_add monitor command support
  - Fence off unsupported features that re-open image file

 v3:
  - Fence off cdrom and change monitor command support

 Signed-off-by: Corey Bryant cor...@linux.vnet.ibm.com
 ---
  block.c           |   16 ++
  block.h           |    1 +
  block/cow.c       |    5 +++
  block/qcow.c      |    5 +++
  block/qcow2.c     |    5 +++
  block/qed.c       |    4 ++
  block/raw-posix.c |   81 
 +++--
  block/vmdk.c      |    5 +++
  block_int.h       |    1 +
  blockdev.c        |   19 
  monitor.c         |    5 +++
  monitor.h         |    1 +
  qemu-options.hx   |    8 +++--
  qemu-tool.c       |    5 +++
  14 files changed, 149 insertions(+), 12 deletions(-)

 diff --git a/block.c b/block.c
 index 24a25d5..500db84 100644
 --- a/block.c
 +++ b/block.c
 @@ -536,6 +536,10 @@ int bdrv_open(BlockDriverState *bs, const char 
 *filename, int flags,
         char tmp_filename[PATH_MAX];
         char backing_filename[PATH_MAX];

 +        if (bdrv_is_fd_protocol(bs)) {
 +            return -ENOTSUP;
 +        }
 +
         /* if snapshot, we create a temporary backing file and open it
            instead of opening 'filename' directly */

 @@ -585,6 +589,10 @@ int bdrv_open(BlockDriverState *bs, const char 
 *filename, int flags,

     /* Find the right image format driver */
     if (!drv) {

Re: [libvirt] [PATCH 00/19] Rollback migration when libvirtd restarts

2011-07-27 Thread Eric Blake


On 07/07/2011 05:34 PM, Jiri Denemark wrote:

This series is also available at
https://gitorious.org/~jirka/libvirt/jirka-staging/commits/migration-recovery

The series does several things:
- persists current job and its phase in status xml
- allows safe monitor commands to be run during migration/save/dump jobs
- implements recovery when libvirtd is restarted while a job is active
- consolidates some code and fixes bugs I found when working in the area


git bisect is pointing to this series as the cause of a regression in 
'virsh managedsave dom' triggering libvirtd core dumps if some other 
process is actively making queries on domain at the same time 
(virt-manager is a great process for fitting that bill).  I'm trying to 
further narrow down which patch introduced the regression, and see if I 
can plug the race (probably a case of not checking whether the monitor 
still exists when getting the condition for an asynchronous job, since 
the whole point of virsh [managed]save is that the domain will go away 
when the save completes, but that it is time-consuming enough that we 
want to query domain state in the meantime).


Program received signal SIGSEGV, Segmentation fault.
[Switching to Thread 0x706d0700 (LWP 11419)]
0x004b9ad8 in qemuMonitorSend (mon=0x7fffe815c060, 
msg=0x706cf380)

at qemu/qemu_monitor.c:801
801 while (!mon-msg-finished) {
(gdb) bt
#0  0x004b9ad8 in qemuMonitorSend (mon=0x7fffe815c060,
msg=0x706cf380) at qemu/qemu_monitor.c:801
#1  0x004c77ae in qemuMonitorJSONCommandWithFd (mon=0x7fffe815c060,
cmd=0x7fffd8000940, scm_fd=-1, reply=0x706cf480)
at qemu/qemu_monitor_json.c:225
#2  0x004c78e5 in qemuMonitorJSONCommand (mon=0x7fffe815c060,
cmd=0x7fffd8000940, reply=0x706cf480) at 
qemu/qemu_monitor_json.c:254

#3  0x004cc19c in qemuMonitorJSONGetMigrationStatus (
mon=0x7fffe815c060, status=0x706cf580, transferred=0x706cf570,
remaining=0x706cf568, total=0x706cf560)
at qemu/qemu_monitor_json.c:1920
#4  0x004bc1b3 in qemuMonitorGetMigrationStatus 
(mon=0x7fffe815c060,

status=0x706cf580, transferred=0x706cf570,
remaining=0x706cf568, total=0x706cf560) at 
qemu/qemu_monitor.c:1532
#5  0x004b201b in qemuMigrationUpdateJobStatus 
(driver=0x7fffe80089f0,

vm=0x7fffe8015cd0, job=0x5427b6 domain save job)
at qemu/qemu_migration.c:765
#6  0x004b2383 in qemuMigrationWaitForCompletion (
driver=0x7fffe80089f0, vm=0x7fffe8015cd0) at qemu/qemu_migration.c:846
#7  0x004b7806 in qemuMigrationToFile (driver=0x7fffe80089f0,
vm=0x7fffe8015cd0, fd=27, offset=4096,
path=0x7fffd8000990 /var/lib/libvirt/qemu/save/fedora_12.save,
compressor=0x0, is_reg=true, bypassSecurityDriver=true)
at qemu/qemu_migration.c:2766
#8  0x0046a90d in qemuDomainSaveInternal (driver=0x7fffe80089f0,
dom=0x7fffd8000ad0, vm=0x7fffe8015cd0,
path=0x7fffd8000990 /var/lib/libvirt/qemu/save/fedora_12.save,
compressed=0, bypass_cache=false) at qemu/qemu_driver.c:2386


--
Eric Blake   ebl...@redhat.com+1-801-349-2682
Libvirt virtualization library http://libvirt.org

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 04/19] qemu: Allow all query commands to be run during long jobs

2011-07-27 Thread Eric Blake


On 07/07/2011 05:34 PM, Jiri Denemark wrote:

Query commands are safe to be called during long running jobs (such as
migration). This patch makes them all work without the need to
special-case every single one of them.


Git bisect says that this was the culprit commit that broke 'virsh 
managedsave'.



+static int
  qemuDomainObjEnterMonitorInternal(struct qemud_driver *driver,
virDomainObjPtr obj)
  {
  qemuDomainObjPrivatePtr priv = obj-privateData;

+if (priv-job.active == QEMU_JOB_NONE  priv-job.asyncJob) {
+if (qemuDomainObjBeginNestedJob(obj)  0)
+return -1;
+if (!virDomainObjIsActive(obj)) {
+qemuReportError(VIR_ERR_OPERATION_FAILED, %s,
+_(domain is no longer running));
+return -1;
+}
+}


I think this is the problem.  Doing a managed save will eventually make 
the qemu process go away, so we reach a point where we cannot issue a 
query monitor command to see how the save is progressing.  But this 
function only checks that vm is still active for QEMU_JOB_NONE, not for 
QEMU_ASYNC_JOB_SAVE.


I'm trying out a patch now...

--
Eric Blake   ebl...@redhat.com+1-801-349-2682
Libvirt virtualization library http://libvirt.org

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH] qemu: improve thread documentation

2011-07-27 Thread Eric Blake

* src/qemu/THREADS.txt: Fix problems with typos, grammar, and
outdated examples.
---

Doc only, so pushing under the trivial rule.

 src/qemu/THREADS.txt |   30 --
 1 files changed, 16 insertions(+), 14 deletions(-)

diff --git a/src/qemu/THREADS.txt b/src/qemu/THREADS.txt
index 3a27a85..e73076c 100644
--- a/src/qemu/THREADS.txt
+++ b/src/qemu/THREADS.txt
@@ -6,7 +6,7 @@ the QEMU driver. The criteria for this model are:

  - Objects must never be exclusively locked for any prolonged time
  - Code which sleeps must be able to time out after suitable period
- - Must be safe against dispatch asynchronous events from monitor
+ - Must be safe against dispatch of asynchronous events from monitor


 Basic locking primitives
@@ -19,7 +19,7 @@ There are a number of locks on various objects
 This is the top level lock on the entire driver. Every API call in
 the QEMU driver is blocked while this is held, though some internal
 callbacks may still run asynchronously. This lock must never be held
-for anything which sleeps/waits (ie monitor commands)
+for anything which sleeps/waits (i.e. monitor commands)

 When obtaining the driver lock, under *NO* circumstances must
 any lock be held on a virDomainObjPtr. This *WILL* result in
@@ -44,7 +44,7 @@ There are a number of locks on various objects
 to have the driver locked when re-acquiring the dropped locked, since the
 reference count prevents it being freed by another thread.

-This lock must not be held for anything which sleeps/waits (ie monitor
+This lock must not be held for anything which sleeps/waits (i.e. monitor
 commands).


@@ -80,7 +80,7 @@ There are a number of locks on various objects
 whenever it hits a piece of code which may sleep/wait, and
 re-acquire it after the sleep/wait.  Whenever an asynchronous job
 wants to talk to the monitor, it needs to acquire nested job (a
-special kind of normla job) to obtain exclusive access to the
+special kind of normal job) to obtain exclusive access to the
 monitor.

 Since the virDomainObjPtr lock was dropped while waiting for the
@@ -139,7 +139,7 @@ To acquire the normal job condition
 - Increments ref count on virDomainObjPtr
 - Waits until the job is compatible with current async job or no
   async job is running
-- Waits job.cond condition 'job.active != 0' using virDomainObjPtr
+- Waits for job.cond condition 'job.active != 0' using virDomainObjPtr
   mutex
 - Rechecks if the job is still compatible and repeats waiting if it
   isn't
@@ -150,7 +150,7 @@ To acquire the normal job condition
 - Unlocks driver
 - Waits until the job is compatible with current async job or no
   async job is running
-- Waits job.cond condition 'job.active != 0' using virDomainObjPtr
+- Waits for job.cond condition 'job.active != 0' using virDomainObjPtr
   mutex
 - Rechecks if the job is still compatible and repeats waiting if it
   isn't
@@ -160,7 +160,7 @@ To acquire the normal job condition
 - Locks virDomainObjPtr

NB: this variant is required in order to comply with lock ordering
-   rules for virDomainObjPtr vs driver
+   rules for virDomainObjPtr vs. driver


   qemuDomainObjEndJob()
@@ -175,7 +175,7 @@ To acquire the asynchronous job condition
   qemuDomainObjBeginAsyncJob()(if driver is unlocked)
 - Increments ref count on virDomainObjPtr
 - Waits until no async job is running
-- Waits job.cond condition 'job.active != 0' using virDomainObjPtr
+- Waits for job.cond condition 'job.active != 0' using virDomainObjPtr
   mutex
 - Rechecks if any async job was started while waiting on job.cond
   and repeats waiting in that case
@@ -185,7 +185,7 @@ To acquire the asynchronous job condition
 - Increments ref count on virDomainObjPtr
 - Unlocks driver
 - Waits until no async job is running
-- Waits job.cond condition 'job.active != 0' using virDomainObjPtr
+- Waits for job.cond condition 'job.active != 0' using virDomainObjPtr
   mutex
 - Rechecks if any async job was started while waiting on job.cond
   and repeats waiting in that case
@@ -271,7 +271,7 @@ Design patterns



- * Accessing something directly todo with a virDomainObjPtr
+ * Accessing something directly to do with a virDomainObjPtr

  virDomainObjPtr obj;

@@ -285,7 +285,7 @@ Design patterns



- * Accessing something directly todo with a virDomainObjPtr and driver
+ * Accessing something directly to do with a virDomainObjPtr and driver

  virDomainObjPtr obj;

@@ -299,11 +299,11 @@ Design patterns



- * Updating something directly todo with a virDomainObjPtr
+ * Updating something directly to do with a virDomainObjPtr

  virDomainObjPtr obj;

- qemuDriverLockRO(driver);
+ qemuDriverLock(driver);
  obj = virDomainFindByUUID(driver-domains, dom-uuid);
  qemuDriverUnlock(driver);

@@ -324,7

Re: [libvirt] [v2] qemu: Improve docs for virsh dump format

2011-07-27 Thread Osier Yang


于 2011年07月28日 00:18, Eric Blake 写道:

On 07/27/2011 04:58 AM, Osier Yang wrote:

The error in getCompressionType will never be reported, change
the errors codes into warning (VIR_WARN(%s, _(foo)); doesn't break
syntax-check rule), and also improve the docs in qemu.conf to tell
user the truth.
---
  src/qemu/qemu.conf |   10 --
  src/qemu/qemu_driver.c |   15 ---
  2 files changed, 16 insertions(+), 9 deletions(-)

diff --git a/src/qemu/qemu.conf b/src/qemu/qemu.conf
index 145062c..7de6863 100644
--- a/src/qemu/qemu.conf
+++ b/src/qemu/qemu.conf
@@ -196,8 +196,14 @@
  # saving a domain in order to save disk space; the list above is in 
descending

  # order by performance and ascending order by compression ratio.
  #
-# save_image_format is used when you use 'virsh save' at scheduled 
saving.
-# dump_image_format is used when you use 'virsh dump' at emergency 
crashdump.

+# save_image_format is used when you use 'virsh save' at scheduled
+# saving, and it is an error if the specified save_image_format is
+# not valid, or the according compression program can't be found.


s/according/requested/


+#
+# dump_image_format is used when you use 'virsh dump' at emergency
+# crashdump, and if the specified dump_image_format is not valid, or
+# the according compression program can't be found, this falls


s/according/requested/


+++ b/src/qemu/qemu_driver.c
@@ -2712,17 +2712,18 @@ getCompressionType(struct qemud_driver *driver)
   */
  if (driver-dumpImageFormat) {
  compress = 
qemudSaveCompressionTypeFromString(driver-dumpImageFormat);
+/* Use raw as the format if the specified format is not 
valid,

+ * or the compress program is not available,
+ */


s/,/./ at the comment end.

ACK with those nits fixed.


Thanks, pushed with the fixing.

Osier

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH] qemu: fix nested job with driver lock held

2011-07-27 Thread Eric Blake

qemuMigrationUpdateJobStatus (called in a loop by migration
and save tasks) uses qemuDomainObjEnterMonitorWithDriver;
however, that function ended up starting a nested job without
releasing the driver.

Since no one else is making nested calls, we can inline the
internal functions to properly track driver_locked.

* src/qemu/qemu_domain.h (qemuDomainObjBeginNestedJob)
(qemuDomainObjBeginNestedJobWithDriver)
(qemuDomainObjEndNestedJob): Drop unused prototypes.
* src/qemu/qemu_domain.c (qemuDomainObjEnterMonitorInternal):
Reflect driver lock to nested job.
(qemuDomainObjBeginNestedJob)
(qemuDomainObjBeginNestedJobWithDriver)
(qemuDomainObjEndNestedJob): Drop unused functions.
---

This does not solve the crash in 'virsh managedsave', but hopefully
will make analysis easier in finding where we are freeing the monitor
too early when probing to see if outgoing migration is completed.

 src/qemu/qemu_domain.c |   59 +++
 src/qemu/qemu_domain.h |9 ---
 2 files changed, 14 insertions(+), 54 deletions(-)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index fe88ce3..2eaaf3a 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -832,31 +832,6 @@ int qemuDomainObjBeginAsyncJobWithDriver(struct 
qemud_driver *driver,
 }

 /*
- * Use this to protect monitor sections within active async job.
- *
- * The caller must call qemuDomainObjBeginAsyncJob{,WithDriver} before it can
- * use this method.  Never use this method if you only own non-async job, use
- * qemuDomainObjBeginJob{,WithDriver} instead.
- */
-int
-qemuDomainObjBeginNestedJob(struct qemud_driver *driver,
-virDomainObjPtr obj)
-{
-return qemuDomainObjBeginJobInternal(driver, false, obj,
- QEMU_JOB_ASYNC_NESTED,
- QEMU_ASYNC_JOB_NONE);
-}
-
-int
-qemuDomainObjBeginNestedJobWithDriver(struct qemud_driver *driver,
-  virDomainObjPtr obj)
-{
-return qemuDomainObjBeginJobInternal(driver, true, obj,
- QEMU_JOB_ASYNC_NESTED,
- QEMU_ASYNC_JOB_NONE);
-}
-
-/*
  * obj must be locked before calling, qemud_driver does not matter
  *
  * To be called after completing the work associated with the
@@ -888,21 +863,6 @@ qemuDomainObjEndAsyncJob(struct qemud_driver *driver, 
virDomainObjPtr obj)
 return virDomainObjUnref(obj);
 }

-void
-qemuDomainObjEndNestedJob(struct qemud_driver *driver, virDomainObjPtr obj)
-{
-qemuDomainObjPrivatePtr priv = obj-privateData;
-
-qemuDomainObjResetJob(priv);
-qemuDomainObjSaveJob(driver, obj);
-virCondSignal(priv-job.cond);
-
-/* safe to ignore since the surrounding async job increased the reference
- * counter as well */
-ignore_value(virDomainObjUnref(obj));
-}
-
-
 static int ATTRIBUTE_NONNULL(1)
 qemuDomainObjEnterMonitorInternal(struct qemud_driver *driver,
   bool driver_locked,
@@ -911,7 +871,9 @@ qemuDomainObjEnterMonitorInternal(struct qemud_driver 
*driver,
 qemuDomainObjPrivatePtr priv = obj-privateData;

 if (priv-job.active == QEMU_JOB_NONE  priv-job.asyncJob) {
-if (qemuDomainObjBeginNestedJob(driver, obj)  0)
+if (qemuDomainObjBeginJobInternal(driver, driver_locked, obj,
+  QEMU_JOB_ASYNC_NESTED,
+  QEMU_ASYNC_JOB_NONE)  0)
 return -1;
 if (!virDomainObjIsActive(obj)) {
 qemuReportError(VIR_ERR_OPERATION_FAILED, %s,
@@ -952,8 +914,15 @@ qemuDomainObjExitMonitorInternal(struct qemud_driver 
*driver,
 priv-mon = NULL;
 }

-if (priv-job.active == QEMU_JOB_ASYNC_NESTED)
-qemuDomainObjEndNestedJob(driver, obj);
+if (priv-job.active == QEMU_JOB_ASYNC_NESTED) {
+qemuDomainObjResetJob(priv);
+qemuDomainObjSaveJob(driver, obj);
+virCondSignal(priv-job.cond);
+
+/* safe to ignore since the surrounding async job increased
+ * the reference counter as well */
+ignore_value(virDomainObjUnref(obj));
+}
 }

 /*
@@ -962,7 +931,7 @@ qemuDomainObjExitMonitorInternal(struct qemud_driver 
*driver,
  * To be called immediately before any QEMU monitor API call
  * Must have already either called qemuDomainObjBeginJob() and checked
  * that the VM is still active or called qemuDomainObjBeginAsyncJob, in which
- * case this will call qemuDomainObjBeginNestedJob.
+ * case this will start a nested job.
  *
  * To be followed with qemuDomainObjExitMonitor() once complete
  */
@@ -988,7 +957,7 @@ void qemuDomainObjExitMonitor(struct qemud_driver *driver,
  * To be called immediately before any QEMU monitor API call
  * Must have already either called qemuDomainObjBeginJobWithDriver() and
  * checked that the VM is still active or called qemuDomainObjBeginAsyncJob,
- * in

Re: [libvirt] Memory leak of the remoteDomainSet* functions

2011-07-27 Thread Osier Yang


于 2011年07月28日 03:19, Matthias Bolte 写道:

2011/7/27 Matthias Boltematthias.bo...@googlemail.com:

2011/7/27 Osier Yangjy...@redhat.com:

Hello, there

Per bug https://bugzilla.redhat.com/show_bug.cgi?id=725322, should
we change the remoteDomainSet* functions into skipgen, and fix the
leaks like below? (NB, new VIR_FREE statements)

Why not fix the generator instead?

An here's the patch that does this

https://www.redhat.com/archives/libvir-list/2011-July/msg01929.html


Thanks, Matthias, ：）

Regards
Osier

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH] virsh: fix memory leak in cmdVolPath code

2011-07-27 Thread Osier Yang


于 2011年07月27日 11:48, Osier Yang 写道:

于 2011年07月27日 11:33, Alex Jia 写道:

* tools/virsh.c: avoid memory leak in cmdVolPath.

* how to reproduce?

   % dd if=/dev/zero of=/var/lib/libvirt/images/foo.img count=1 bs=10M
   % virsh pool-refresh default
   % valgrind -v --leak-check=full virsh vol-path --vol
  /var/lib/libvirt/images/foo.img

* actual results:

Detected in valgrind run:

==16436== 32 bytes in 1 blocks are definitely lost in loss record 7 
of 22

==16436==at 0x4A05FDE: malloc (vg_replace_malloc.c:236)
==16436==by 0x386A314B3D: xdr_string (in /lib64/libc-2.12.so)
==16436==by 0x3DF8CD770D: xdr_remote_nonnull_string 
(remote_protocol.c:30)

==16436==by 0x3DF8CD7EC8: xdr_remote_storage_vol_get_path_ret
(remote_protocol.c:2952)
==16436==by 0x3DF8CDF161: virNetMessageDecodePayload 
(virnetmessage.c:286)

==16436==by 0x3DF8CDE9E5: virNetClientProgramCall
(virnetclientprogram.c:318)
==16436==by 0x3DF8CC28A2: call (remote_driver.c:3929)
==16436==by 0x3DF8CC8412: remoteStorageVolGetPath
(remote_client_bodies.h:5219)
==16436==by 0x3DF8C9BF14: virStorageVolGetPath (libvirt.c:11389)
==16436==by 0x418ECA: cmdVolPath (virsh.c:8754)
==16436==by 0x410CC2: vshCommandRun (virsh.c:12758)
==16436==by 0x41F286: main (virsh.c:14110)
==16436==
==16436== LEAK SUMMARY:
==16436==definitely lost: 32 bytes in 1 blocks


Signed-off-by: Alex Jiaa...@redhat.com
---
  tools/virsh.c |9 -
  1 files changed, 8 insertions(+), 1 deletions(-)

diff --git a/tools/virsh.c b/tools/virsh.c
index 841df61..d194a8b 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -9326,6 +9326,7 @@ cmdVolPath(vshControl *ctl, const vshCmd *cmd)
  {
  virStorageVolPtr vol;
  const char *name = NULL;
+char * StorageVolPath;

  if (!vshConnectionUsability(ctl, ctl-conn))
  return false;
@@ -9334,7 +9335,13 @@ cmdVolPath(vshControl *ctl, const vshCmd *cmd)
  return false;
  }

-vshPrint(ctl, %s\n, virStorageVolGetPath(vol));
+if ((StorageVolPath = virStorageVolGetPath(vol)) == NULL) {
+virStorageVolFree(vol);
+return false;
+}
+
+vshPrint(ctl, %s\n, StorageVolPath);
+VIR_FREE(StorageVolPath);
  virStorageVolFree(vol);
  return true;
  }


ACK, but it will be better if docs of virStorageVolGetPath in 
src/libvirt.c

can be updated, (to tell the returned path must be freed by caller).

Osier

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Pushed with comments of virStorageVolGetPath updated. Thanks

Osier

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] libvirt-0.9.1 to 0.9.3-r1: managedsave/save won't start/restore at saved state

2011-07-27 Thread Laine Stump


On 07/27/2011 04:37 AM, Nicolas Sebrecht wrote:

I'm seeing strange behaviour, here.  Any guests saved using both
managedsave and save commands from virsh won't restore at saved state. A
new full boot sequence happen.

- Tested against libvirt v0.9.1, v0.9.2, v0.9.3-r1 (Gentoo)
- Confirmed on three different hosts Gentoo amd64 systems.
- Tested with gentoo and ubuntu guests.
- Nothing relevant in /var/log/libvirt/libvirt.log or
/var/log/libvirt/qemu/dom.log

The state file /var/lib/libvirt/qemu/save/dom.save exists and is
deleted when 'virsh start' is called.

The new boot sequence is confirmed by :
- VNC console checks
- previous screen sessions lost
- uptime

I've open a bug at https://bugs.gentoo.org/show_bug.cgi?id=376333 but
had no answer.

Any idea on what could happen or how to inspect it?


If you pause (virsh suspend) the guest before the save, can you then 
successfully start the domain (it should start in a paused state, and 
when the virsh commandline returns to the prompt, you can resume it with 
virsh resume.) If this is the case, it could be a race condition 
within qemu.


--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH] qemu: fix nested job with driver lock held

2011-07-27 Thread Laine Stump


On 07/27/2011 10:04 PM, Eric Blake wrote:

qemuMigrationUpdateJobStatus (called in a loop by migration
and save tasks) uses qemuDomainObjEnterMonitorWithDriver;
however, that function ended up starting a nested job without
releasing the driver.

Since no one else is making nested calls, we can inline the
internal functions to properly track driver_locked.


ACK.


* src/qemu/qemu_domain.h (qemuDomainObjBeginNestedJob)
(qemuDomainObjBeginNestedJobWithDriver)
(qemuDomainObjEndNestedJob): Drop unused prototypes.
* src/qemu/qemu_domain.c (qemuDomainObjEnterMonitorInternal):
Reflect driver lock to nested job.
(qemuDomainObjBeginNestedJob)
(qemuDomainObjBeginNestedJobWithDriver)
(qemuDomainObjEndNestedJob): Drop unused functions.
---

This does not solve the crash in 'virsh managedsave', but hopefully
will make analysis easier in finding where we are freeing the monitor
too early when probing to see if outgoing migration is completed.

  src/qemu/qemu_domain.c |   59 +++
  src/qemu/qemu_domain.h |9 ---
  2 files changed, 14 insertions(+), 54 deletions(-)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index fe88ce3..2eaaf3a 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -832,31 +832,6 @@ int qemuDomainObjBeginAsyncJobWithDriver(struct 
qemud_driver *driver,
  }

  /*
- * Use this to protect monitor sections within active async job.
- *
- * The caller must call qemuDomainObjBeginAsyncJob{,WithDriver} before it can
- * use this method.  Never use this method if you only own non-async job, use
- * qemuDomainObjBeginJob{,WithDriver} instead.
- */
-int
-qemuDomainObjBeginNestedJob(struct qemud_driver *driver,
-virDomainObjPtr obj)
-{
-return qemuDomainObjBeginJobInternal(driver, false, obj,
- QEMU_JOB_ASYNC_NESTED,
- QEMU_ASYNC_JOB_NONE);
-}
-
-int
-qemuDomainObjBeginNestedJobWithDriver(struct qemud_driver *driver,
-  virDomainObjPtr obj)
-{
-return qemuDomainObjBeginJobInternal(driver, true, obj,
- QEMU_JOB_ASYNC_NESTED,
- QEMU_ASYNC_JOB_NONE);
-}
-
-/*
   * obj must be locked before calling, qemud_driver does not matter
   *
   * To be called after completing the work associated with the
@@ -888,21 +863,6 @@ qemuDomainObjEndAsyncJob(struct qemud_driver *driver, 
virDomainObjPtr obj)
  return virDomainObjUnref(obj);
  }

-void
-qemuDomainObjEndNestedJob(struct qemud_driver *driver, virDomainObjPtr obj)
-{
-qemuDomainObjPrivatePtr priv = obj-privateData;
-
-qemuDomainObjResetJob(priv);
-qemuDomainObjSaveJob(driver, obj);
-virCondSignal(priv-job.cond);
-
-/* safe to ignore since the surrounding async job increased the reference
- * counter as well */
-ignore_value(virDomainObjUnref(obj));
-}
-
-
  static int ATTRIBUTE_NONNULL(1)
  qemuDomainObjEnterMonitorInternal(struct qemud_driver *driver,
bool driver_locked,
@@ -911,7 +871,9 @@ qemuDomainObjEnterMonitorInternal(struct qemud_driver 
*driver,
  qemuDomainObjPrivatePtr priv = obj-privateData;

  if (priv-job.active == QEMU_JOB_NONE  priv-job.asyncJob) {
-if (qemuDomainObjBeginNestedJob(driver, obj)  0)
+if (qemuDomainObjBeginJobInternal(driver, driver_locked, obj,
+  QEMU_JOB_ASYNC_NESTED,
+  QEMU_ASYNC_JOB_NONE)  0)
  return -1;
  if (!virDomainObjIsActive(obj)) {
  qemuReportError(VIR_ERR_OPERATION_FAILED, %s,
@@ -952,8 +914,15 @@ qemuDomainObjExitMonitorInternal(struct qemud_driver 
*driver,
  priv-mon = NULL;
  }

-if (priv-job.active == QEMU_JOB_ASYNC_NESTED)
-qemuDomainObjEndNestedJob(driver, obj);
+if (priv-job.active == QEMU_JOB_ASYNC_NESTED) {
+qemuDomainObjResetJob(priv);
+qemuDomainObjSaveJob(driver, obj);
+virCondSignal(priv-job.cond);
+
+/* safe to ignore since the surrounding async job increased
+ * the reference counter as well */
+ignore_value(virDomainObjUnref(obj));
+}
  }

  /*
@@ -962,7 +931,7 @@ qemuDomainObjExitMonitorInternal(struct qemud_driver 
*driver,
   * To be called immediately before any QEMU monitor API call
   * Must have already either called qemuDomainObjBeginJob() and checked
   * that the VM is still active or called qemuDomainObjBeginAsyncJob, in which
- * case this will call qemuDomainObjBeginNestedJob.
+ * case this will start a nested job.
   *
   * To be followed with qemuDomainObjExitMonitor() once complete
   */
@@ -988,7 +957,7 @@ void qemuDomainObjExitMonitor(struct qemud_driver *driver,
   * To be called immediately before any QEMU monitor API call
   * Must have already either called qemuDomainObjBeginJobWithDriver() and
   *

Re: [libvirt] [PATCH] virsh: fix memory leak in cmdVolPath code

2011-07-27 Thread Alex Jia


On 07/28/2011 11:06 AM, Osier Yang wrote:

于 2011年07月27日 11:48, Osier Yang 写道:

于 2011年07月27日 11:33, Alex Jia 写道:

* tools/virsh.c: avoid memory leak in cmdVolPath.

* how to reproduce?

   % dd if=/dev/zero of=/var/lib/libvirt/images/foo.img count=1 bs=10M
   % virsh pool-refresh default
   % valgrind -v --leak-check=full virsh vol-path --vol
  /var/lib/libvirt/images/foo.img

* actual results:

Detected in valgrind run:

==16436== 32 bytes in 1 blocks are definitely lost in loss record 7 
of 22

==16436==at 0x4A05FDE: malloc (vg_replace_malloc.c:236)
==16436==by 0x386A314B3D: xdr_string (in /lib64/libc-2.12.so)
==16436==by 0x3DF8CD770D: xdr_remote_nonnull_string 
(remote_protocol.c:30)

==16436==by 0x3DF8CD7EC8: xdr_remote_storage_vol_get_path_ret
(remote_protocol.c:2952)
==16436==by 0x3DF8CDF161: virNetMessageDecodePayload 
(virnetmessage.c:286)

==16436==by 0x3DF8CDE9E5: virNetClientProgramCall
(virnetclientprogram.c:318)
==16436==by 0x3DF8CC28A2: call (remote_driver.c:3929)
==16436==by 0x3DF8CC8412: remoteStorageVolGetPath
(remote_client_bodies.h:5219)
==16436==by 0x3DF8C9BF14: virStorageVolGetPath (libvirt.c:11389)
==16436==by 0x418ECA: cmdVolPath (virsh.c:8754)
==16436==by 0x410CC2: vshCommandRun (virsh.c:12758)
==16436==by 0x41F286: main (virsh.c:14110)
==16436==
==16436== LEAK SUMMARY:
==16436==definitely lost: 32 bytes in 1 blocks


Signed-off-by: Alex Jiaa...@redhat.com
---
  tools/virsh.c |9 -
  1 files changed, 8 insertions(+), 1 deletions(-)

diff --git a/tools/virsh.c b/tools/virsh.c
index 841df61..d194a8b 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -9326,6 +9326,7 @@ cmdVolPath(vshControl *ctl, const vshCmd *cmd)
  {
  virStorageVolPtr vol;
  const char *name = NULL;
+char * StorageVolPath;

  if (!vshConnectionUsability(ctl, ctl-conn))
  return false;
@@ -9334,7 +9335,13 @@ cmdVolPath(vshControl *ctl, const vshCmd *cmd)
  return false;
  }

-vshPrint(ctl, %s\n, virStorageVolGetPath(vol));
+if ((StorageVolPath = virStorageVolGetPath(vol)) == NULL) {
+virStorageVolFree(vol);
+return false;
+}
+
+vshPrint(ctl, %s\n, StorageVolPath);
+VIR_FREE(StorageVolPath);
  virStorageVolFree(vol);
  return true;
  }


ACK, but it will be better if docs of virStorageVolGetPath in 
src/libvirt.c

can be updated, (to tell the returned path must be freed by caller).

Osier

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Pushed with comments of virStorageVolGetPath updated. Thanks

Osier

Osier, thanks for your push and update.

BTW, it seems other comments of function are also missing this 
introduction for whether caller should

free this.

Regards,
Alex

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

74 matches

Mail list logo