[libvirt] virsh --connect not working

2012-09-28 Thread hiren panchasara
I've installed libvirt-0.9.13 from ports on my freebsd machine.

I started libvirtd:

$ ps awwux | grep libvirtd
root   11470   0.0  0.4  103100  31948  -  I10:41PM   0:00.35
libvirtd -v -d

$ sudo virsh --connect qemu:///system
error: no connection driver available for No connection for URI
qemu:///system
error: failed to connect to the hypervisor

I am sure I am missing something obvious.

Thanks in advance,
Hiren
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] virsh --connect not working

2012-09-28 Thread hiren panchasara
On Thu, Sep 27, 2012 at 11:15 PM, hiren panchasara 
hiren.panchas...@gmail.com wrote:

 I've installed libvirt-0.9.13 from ports on my freebsd machine.

 I started libvirtd:

 $ ps awwux | grep libvirtd
 root   11470   0.0  0.4  103100  31948  -  I10:41PM   0:00.35
 libvirtd -v -d

 $ sudo virsh --connect qemu:///system
 error: no connection driver available for No connection for URI
 qemu:///system
 error: failed to connect to the hypervisor

 I am sure I am missing something obvious.

Just wanted to add that I have qemu installed. Not sure how to connect that
with virsh.

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

Re: [libvirt] [PATCH 1/2] python: cleanup vcpu related binding APIs

2012-09-28 Thread Guannan Ren

On 09/27/2012 09:51 PM, Michal Privoznik wrote:

On 26.09.2012 19:33, Guannan Ren wrote:

libvirt_virDomainGetVcpus: add error handling, return -1 instead of None
libvirt_virDomainPinVcpu and libvirt_virDomainPinVcpuFlags:
   make use of libvirt_boolUnwrap

   Set bitmap according to these values which are contained in given
   argument of vcpu tuple and turn off these bit corresponding to
   missing vcpus in argument tuple

   The original way ignored the error info from PyTuple_GetItem
   if index is out of range.
   IndexError: tuple index out of range
   The error message will only be raised on next command in interactive mode.
---
  python/libvirt-override.c | 171 +-
  1 file changed, 123 insertions(+), 48 deletions(-)

diff --git a/python/libvirt-override.c b/python/libvirt-override.c
index 25f9d3f..b69f5cf 100644
--- a/python/libvirt-override.c
+++ b/python/libvirt-override.c
@@ -1333,9 +1333,11 @@ cleanup:
  
  static PyObject *

  libvirt_virDomainGetVcpus(PyObject *self ATTRIBUTE_UNUSED,
-  PyObject *args) {
+  PyObject *args)
+{
  virDomainPtr domain;
  PyObject *pyobj_domain, *pyretval = NULL, *pycpuinfo = NULL, *pycpumap = 
NULL;
+PyObject *error = NULL;

You are not setting this variable before every 'goto cleanup;' and I
think it should be done. Or is it okay to return NULL?


  we set error variable in two places.
 error = PyErr_NoMemory();
 error = VIR_PY_INT_FAIL;

 For the other places before goto cleanup, we don't need to set 
specific error because
 the cpython function did it already. what left to me is to return 
NULL to show these

 error messages to user.

 In common,  if cpython function(e.g PyTuple_New()) reports errors, 
we just return NULL.





No. I know python bindings code are mess but this won't make it any
better. First of all:
if (cond1)
   code_block;
else if (cond2)
   the_very_same_codeblock;

can be joined like this:
if (cond1 || cond2)
   code_block;


  Thanks I will change to this.




Second - drop the do { } while(0) and use a label instead:
if (cond1 || cond2)
   goto label;


   without do () while, we  have to have two labels
   I think it will be a little messy.
   I tried the following before, it is ok to walk back.

   if (cond1 || cond2)
  goto label;

   continue;
   label:
  Py_DECREF(info);
  ..
  goto cleanup



This label can in this special case be at the end of the parent for
loop; However, there needs to be 'continue' just before the label so the
for loop doesn't execute the label itself.





 For the following, I will change according to your comments.
 Thanks  for this review. :)

 Guannan


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


Re: [libvirt] [PATCH] Move most of qemuProcessKill into virProcessKillPainfully

2012-09-28 Thread Daniel P. Berrange
On Thu, Sep 27, 2012 at 04:31:10PM -0400, Laine Stump wrote:
 On 09/26/2012 10:44 AM, Daniel P. Berrange wrote:
  From: Daniel P. Berrange berra...@redhat.com
 
  In the cgroups APIs we have a virCgroupKillPainfully function
  which does the loop sending SIGTERM, then SIGKILL and waiting
  for the process to exit. There is similar functionality for
  simple processes in qemuProcessKill, but it is tangled with
  the QEMU code. Untangle it to provide a virProcessKillPainfuly
  function
  ---
   src/libvirt_private.syms |  1 +
   src/qemu/qemu_driver.c   |  8 ++---
   src/qemu/qemu_process.c  | 79 
  
   src/util/virprocess.c| 57 ++
   src/util/virprocess.h|  2 ++
   5 files changed, 76 insertions(+), 71 deletions(-)
 
  diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
  index 4635a4d..dab607a 100644
  --- a/src/libvirt_private.syms
  +++ b/src/libvirt_private.syms
  @@ -1708,6 +1708,7 @@ virPidFileDeletePath;
   # virprocess.h
   virProcessAbort;
   virProcessKill;
  +virProcessKillPainfully;
   virProcessTranslateStatus;
   virProcessWait;
   
  diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
  index 7ac53ac..22fef7a 100644
  --- a/src/qemu/qemu_driver.c
  +++ b/src/qemu/qemu_driver.c
  @@ -2006,13 +2006,11 @@ qemuDomainDestroyFlags(virDomainPtr dom,
* it now means the job will be released
*/
   if (flags  VIR_DOMAIN_DESTROY_GRACEFUL) {
  -if (qemuProcessKill(driver, vm, 0)  0) {
  -virReportError(VIR_ERR_OPERATION_FAILED, %s,
  -   _(failed to kill qemu process with SIGTERM));
  +if (qemuProcessKill(driver, vm, 0)  0)
   goto cleanup;
  -}
   } else {
  -ignore_value(qemuProcessKill(driver, vm, 
  VIR_QEMU_PROCESS_KILL_FORCE));
  +if (qemuProcessKill(driver, vm, VIR_QEMU_PROCESS_KILL_FORCE)  0)
  +goto cleanup;
   }
   
   /* We need to prevent monitor EOF callback from doing our work (and 
  sending
  diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
  index 3cd30af..70b72af 100644
  --- a/src/qemu/qemu_process.c
  +++ b/src/qemu/qemu_process.c
  @@ -3877,9 +3877,7 @@ int
   qemuProcessKill(struct qemud_driver *driver,
   virDomainObjPtr vm, unsigned int flags)
   {
  -int i, ret = -1;
  -const char *signame = TERM;
  -bool driver_unlocked = false;
  +int ret;
   
   VIR_DEBUG(vm=%s pid=%d flags=%x,
 vm-def-name, vm-pid, flags);
  @@ -3891,78 +3889,27 @@ qemuProcessKill(struct qemud_driver *driver,
   }
   }
   
  -/* This loop sends SIGTERM (or SIGKILL if flags has
  - * VIR_QEMU_PROCESS_KILL_FORCE and VIR_QEMU_PROCESS_KILL_NOWAIT),
  - * then waits a few iterations (10 seconds) to see if it dies. If
  - * the qemu process still hasn't exited, and
  - * VIR_QEMU_PROCESS_KILL_FORCE is requested, a SIGKILL will then
  - * be sent, and qemuProcessKill will wait up to 5 seconds more for
  - * the process to exit before returning.  Note that the FORCE mode
  - * could result in lost data in the guest, so it should only be
  - * used if the guest is hung and can't be destroyed in any other
  - * manner.
  - */
  -for (i = 0 ; i  75; i++) {
  -int signum;
  -if (i == 0) {
  -if ((flags  VIR_QEMU_PROCESS_KILL_FORCE) 
  -(flags  VIR_QEMU_PROCESS_KILL_NOWAIT)) {
  -signum = SIGKILL; /* kill it immediately */
  -signame=KILL;
  -} else {
  -signum = SIGTERM; /* kindly suggest it should exit */
  -}
  -} else if ((i == 50)  (flags  VIR_QEMU_PROCESS_KILL_FORCE)) {
  -VIR_WARN(Timed out waiting after SIG%s to process %d, 
  - sending SIGKILL, signame, vm-pid);
  -signum = SIGKILL; /* kill it after a grace period */
  -signame=KILL;
  -} else {
  -signum = 0; /* Just check for existence */
  -}
  -
  -if (virProcessKill(vm-pid, signum)  0) {
  -if (errno != ESRCH) {
  -char ebuf[1024];
  -VIR_WARN(Failed to terminate process %d with SIG%s: %s,
  - vm-pid, signame,
  - virStrerror(errno, ebuf, sizeof(ebuf)));
  -goto cleanup;
  -}
  -ret = 0;
  -goto cleanup; /* process is dead */
  -}
  +if ((flags  VIR_QEMU_PROCESS_KILL_NOWAIT)) {
  +virProcessKill(vm-pid,
  +   (flags  VIR_QEMU_PROCESS_KILL_FORCE) ?
  +   SIGKILL : SIGTERM);
  +return 0;
  +}
   
  -if (i == 0  (flags  VIR_QEMU_PROCESS_KILL_NOWAIT)) {
  -ret = 0;
  -goto cleanup;
  -}
  +if (driver)
  +

Re: [libvirt] [PATCH 2/9] s/long long/size_t/ for file line numbers in logging code

2012-09-28 Thread Daniel P. Berrange
On Thu, Sep 27, 2012 at 01:05:55PM -0600, Eric Blake wrote:
 On 09/27/2012 10:44 AM, Daniel P. Berrange wrote:
  From: Daniel P. Berrange berra...@redhat.com
  
  The __LINE__ macro value is specified to fit in the size_t
 
 Not quite accurate.  C99 merely says:
 
 6.10.4 p3:
 ... a line number as specified by the digit sequence (interpreted as
 a decimal integer). The digit sequence shall not specify zero, nor a
 number greater than
 2147483647.
 
 6.10.8 p1:
 _ _LINE_ _ The presumed line number (within the current source file) of
 the current
 source line (an integer constant).
 
 So in fact, __LINE__ is guaranteed to fit within a 32-bit signed
 integer, and you could s/size_t/int/ with no loss in functionality since
 we don't port to 16-bit 'int' platforms.  That said, I'm not too fussed
 with things; size_t is definitely better than 'long long', so I'm okay
 even if you don't further relax to 'int'.

Based on discussion in the later patches, I'm happy to change this
to just 'int'.  Still surprised it wasn't actually size_t in the
standard, but oh well.

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 7/9] Add systemd journal support

2012-09-28 Thread Daniel P. Berrange
On Thu, Sep 27, 2012 at 02:51:35PM -0600, Eric Blake wrote:
 On 09/27/2012 10:44 AM, Daniel P. Berrange wrote:
  From: Daniel P. Berrange berra...@redhat.com
  
  Add support for logging to the systemd journal, using its
  simple client library. The benefit over syslog is that it
  accepts structured log data, so the journald can store
  individual items like code file/line/func separately from
  the string message. Tools which require structured log
  data can then query the journal to extract exactly what
  they desire without resorting to string parsing
  
  While systemd provides a simple client library for logging,
  it is more convenient for libvirt to directly write its
  own client code. This lets us build up the iovec's on
  the stack, avoiding the need to alloc memory when writing
  log messages.
  
  Signed-off-by: Daniel P. Berrange berra...@redhat.com
  ---
   cfg.mk |   2 +-
   src/util/logging.c | 177 
  +
   src/util/logging.h |   1 +
   3 files changed, 179 insertions(+), 1 deletion(-)
 
 I'll need a v2 on this one.
 
 Fails to compile:
 
 util/logging.c: In function 'virLogOutputToJournald':
 util/logging.c:1124:84: error: ordered comparison of pointer with
 integer zero [-Werror=extra]
 util/logging.c:1124:18: error: ignoring return value of 'virStrcpy',
 declared with attribute warn_unused_result [-Werror=unused-result]
 cc1: all warnings being treated as errors
 
  
  diff --git a/cfg.mk b/cfg.mk
  index bbfd4a2..4482d70 100644
  --- a/cfg.mk
  +++ b/cfg.mk
  @@ -771,7 +771,7 @@ exclude_file_name_regexp--sc_prohibit_asprintf = \
 
  ^(bootstrap.conf$$|src/util/util\.c$$|examples/domain-events/events-c/event-test\.c$$)
   
   exclude_file_name_regexp--sc_prohibit_close = \
  -  (\.p[yl]$$|^docs/|^(src/util/virfile\.c|src/libvirt\.c)$$)
  +  
  (\.p[yl]$$|^docs/|^(src/util/virfile\.c|src/libvirt\.c|src/util/logging\.c)$$)
 
 That's a bit of a heavy hammer, just to avoid a recursively-logged
 close().  I'd rather omit this and use VIR_LOG_CLOSE().

Ahhh, I didn't notice VIR_LOG_CLOSE. I did indeed hit a
recursively logging close :-)

  +
  +
  +# define IOVEC_SET_STRING(iov, str)  \
  +do { \
  +struct iovec *_i = (iov);   \
  +_i-iov_base = (char*)str;   \
  +_i-iov_len = strlen(str);   \
  +} while (0)
  +
  +# define IOVEC_SET_INT(iov, buf, fmt, val)  \
  +do {\
  +struct iovec *_i = (iov);  \
  +snprintf(buf, sizeof(buf), fmt, val);   \
 
 Technically, snprintf is not async-signal safe, which kind of goes
 against the idea of using it in our logging functions.  I'd much rather
 see us do a hand-rolled conversion of int to %d or %zu (which appear to
 be the only formats you ever pass here).

Ok, should be easy enough, although I notice that the
code we use for generating the timestamp string also
uses snprintf(), so we should fix that sometime too.

  + * encode the string length, since we can't
  + * rely on the newline for the field separator
  + */
  +IOVEC_SET_STRING(iov[niov++], MESSAGE\n);
  +nstr = htole64(strlen(rawstr));
 
 htole64() is a glibc extension which does not exist on mingw, and has
 not (yet) been ported to gnulib.  Then again, this code is inside #ifdef
 HAVE_SYSLOG_H, so you can probably get away with it (although I'm not
 sure whether all platforms that provide syslog.h happen to also
 provide htole64(), I at least know that it will exclude mingw).

I think I'll #ifdef __linux__ the whole block just to be sure,
since systemd is only ever going to be Linux based

  +/* Message was too large, so dump to temporary file
  + * and pass an FD to the journal
  + */
  +
  +if ((buffd = mkostemp(path, O_CLOEXEC|O_RDWR))  0)
 
 Is mkostemp async-signal safe?  But if it isn't, I don't know how else
 to generate a safe file name.  Maybe we create ourselves a safe
 temporary directory at process start where we don't care about the async
 safety issues, and then in this function, we track a static counter that
 we increment each time we create a new file within that directory.

Yep, we never need multiple concurrent temp files, so I can just
create one when we initialize logging, and hold open its file
descriptor.

 
  +return;
  +
  +if (unlink(path)  0)
  +goto cleanup;
  +
  +if (writev(buffd, iov, niov)  0)
 
 Again, mingw and gnulib lacks writev(), but this function isn't compiled
 on mingw, so we're safe for now.
 
  +goto cleanup;
  +
  +mh.msg_iov = NULL;
  +mh.msg_iovlen = 0;
  +
  +memset(control, 0, sizeof(control));
  +mh.msg_control = control;
  +mh.msg_controllen = sizeof(control);
  +
  +cmsg = CMSG_FIRSTHDR(mh);
  +cmsg-cmsg_level = SOL_SOCKET;
  +cmsg-cmsg_type = SCM_RIGHTS;
  +cmsg-cmsg_len = 

Re: [libvirt] [PATCH 0/9] Use the systemd journal for logging

2012-09-28 Thread Daniel P. Berrange
On Thu, Sep 27, 2012 at 07:31:41PM -0500, Doug Goldstein wrote:
 On Thu, Sep 27, 2012 at 11:44 AM, Daniel P. Berrange
 berra...@redhat.com wrote:
  The systemd journal provides a highly efficient way for apps
  to generate structured log messages. Since systemd will be
  used by default on future Fedora/RHEL distros (and likely
  many others), we should take advantage of the journal out of
  the box for improved logging capabilities.
 
  Amongst other things, this allows very easy browsing of the
  libvirt logs after the fact, filtering on filename / line
  number / function name and more. This is a big benefit
  to debugging, compared to current case where we need to
  decide the filters when starting libvirtd. For example
  it becomes possible to configure libvirt to include all
  QEMU logging, and then later ask the journal for only
  messages related to the QEMU json monitor.
 
  --
 
 Someone else posted a patchset for structured logging via lumberjack
 which would support systemd as well as the other syslog daemons in an
 abstract way without directly tying the project to systemd?
 
 That seems like a better option IMHO. So I would vote for a NACK for the 
 series.

First off, we're not tieing the project to systemd, because this is an
entirely optional feature. If you read my response to the lumberjack
posting, you'll see that there are a number of serious architectural
flaws in the design of lumberjack for use as a structured logging
system. Unless they radically change the design, which appears unlikely,
it is not going to be suitable as a logging impl for libvirt (or arguably
any system daemon) to use.

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] virsh --connect not working

2012-09-28 Thread Daniel P. Berrange
On Thu, Sep 27, 2012 at 11:15:56PM -0700, hiren panchasara wrote:
 I've installed libvirt-0.9.13 from ports on my freebsd machine.
 
 I started libvirtd:
 
 $ ps awwux | grep libvirtd
 root   11470   0.0  0.4  103100  31948  -  I10:41PM   0:00.35
 libvirtd -v -d
 
 $ sudo virsh --connect qemu:///system
 error: no connection driver available for No connection for URI
 qemu:///system
 error: failed to connect to the hypervisor
 
 I am sure I am missing something obvious.


Try collecting some debugging output using

  LIBVIRT_DEBUG=1 sudo virsh --connect qemu:///system

It should give a better idea why it failed.

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 v3] Fix dnsmasq/radvd on bridges not being able to be bound to IPv6 address on recent kernels

2012-09-28 Thread Benjamin Cama
Le jeudi 27 septembre 2012 à 11:18 -0600, Eric Blake a écrit :
 ACK with those changes, so I fixed it and pushed, along with your
 .mailmap update.

Thanks a lot!

-- 
Benjamin Cama benjamin.c...@telecom-bretagne.eu

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

Re: [libvirt] [PATCH 2/2] python: return error if PyObject obj is NULL for unwrapper helper functions

2012-09-28 Thread Guannan Ren

On 09/27/2012 09:51 PM, Michal Privoznik wrote:

On 26.09.2012 19:33, Guannan Ren wrote:

The result is indeterminate for NULL argument to python
functions as follows. It's better to return negative value in
these situations.

PyObject_IsTrue will segfault if the argument is NULL
PyFloat_AsDouble(NULL) is -1.00
PyLong_AsUnsignedLongLong(NULL) is 0.00
---
  python/typewrappers.c | 43 +--
  1 file changed, 41 insertions(+), 2 deletions(-)

ACK

Michal


Thanks and pushed

Guannan

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


[libvirt] [PATCH v3] python: cleanup vcpu related binding APIs

2012-09-28 Thread Guannan Ren
libvirt_virDomainGetVcpus: add error handling, return -1 instead of None
libvirt_virDomainPinVcpu and libvirt_virDomainPinVcpuFlags:
  check the type of argument
  make use of libvirt_boolUnwrap

  Set bitmap according to these values which are contained in given
  argument of vcpu tuple and turn off these bit corresponding to
  missing vcpus in argument tuple

  The original way ignored the error info from PyTuple_GetItem
  if index is out of range.
  IndexError: tuple index out of range
  The error message will only be raised on next command in interactive mode.
---
 python/libvirt-override.c | 163 ++
 1 file changed, 121 insertions(+), 42 deletions(-)

diff --git a/python/libvirt-override.c b/python/libvirt-override.c
index 25f9d3f..81099b1 100644
--- a/python/libvirt-override.c
+++ b/python/libvirt-override.c
@@ -1333,9 +1333,11 @@ cleanup:
 
 static PyObject *
 libvirt_virDomainGetVcpus(PyObject *self ATTRIBUTE_UNUSED,
-  PyObject *args) {
+  PyObject *args)
+{
 virDomainPtr domain;
 PyObject *pyobj_domain, *pyretval = NULL, *pycpuinfo = NULL, *pycpumap = 
NULL;
+PyObject *error = NULL;
 virNodeInfo nodeinfo;
 virDomainInfo dominfo;
 virVcpuInfoPtr cpuinfo = NULL;
@@ -1352,29 +1354,33 @@ libvirt_virDomainGetVcpus(PyObject *self 
ATTRIBUTE_UNUSED,
 i_retval = virNodeGetInfo(virDomainGetConnect(domain), nodeinfo);
 LIBVIRT_END_ALLOW_THREADS;
 if (i_retval  0)
-return VIR_PY_NONE;
+return VIR_PY_INT_FAIL;
 
 LIBVIRT_BEGIN_ALLOW_THREADS;
 i_retval = virDomainGetInfo(domain, dominfo);
 LIBVIRT_END_ALLOW_THREADS;
 if (i_retval  0)
-return VIR_PY_NONE;
+return VIR_PY_INT_FAIL;
 
 if (VIR_ALLOC_N(cpuinfo, dominfo.nrVirtCpu)  0)
-return VIR_PY_NONE;
+return PyErr_NoMemory();
 
 cpumaplen = VIR_CPU_MAPLEN(VIR_NODEINFO_MAXCPUS(nodeinfo));
 if (xalloc_oversized(dominfo.nrVirtCpu, cpumaplen) ||
-VIR_ALLOC_N(cpumap, dominfo.nrVirtCpu * cpumaplen)  0)
+VIR_ALLOC_N(cpumap, dominfo.nrVirtCpu * cpumaplen)  0) {
+error = PyErr_NoMemory();
 goto cleanup;
+}
 
 LIBVIRT_BEGIN_ALLOW_THREADS;
 i_retval = virDomainGetVcpus(domain,
  cpuinfo, dominfo.nrVirtCpu,
  cpumap, cpumaplen);
 LIBVIRT_END_ALLOW_THREADS;
-if (i_retval  0)
+if (i_retval  0) {
+error = VIR_PY_INT_FAIL;
 goto cleanup;
+}
 
 /* convert to a Python tuple of long objects */
 if ((pyretval = PyTuple_New(2)) == NULL)
@@ -1386,13 +1392,35 @@ libvirt_virDomainGetVcpus(PyObject *self 
ATTRIBUTE_UNUSED,
 
 for (i = 0 ; i  dominfo.nrVirtCpu ; i++) {
 PyObject *info = PyTuple_New(4);
+PyObject *item = NULL;
+
 if (info == NULL)
 goto cleanup;
-PyTuple_SetItem(info, 0, PyInt_FromLong((long)cpuinfo[i].number));
-PyTuple_SetItem(info, 1, PyInt_FromLong((long)cpuinfo[i].state));
-PyTuple_SetItem(info, 2, PyLong_FromLongLong((long 
long)cpuinfo[i].cpuTime));
-PyTuple_SetItem(info, 3, PyInt_FromLong((long)cpuinfo[i].cpu));
-PyList_SetItem(pycpuinfo, i, info);
+
+if ((item = PyInt_FromLong((long)cpuinfo[i].number)) == NULL ||
+PyTuple_SetItem(info, 0, item)  0)
+goto itemError;
+
+if ((item = PyInt_FromLong((long)cpuinfo[i].state)) == NULL ||
+PyTuple_SetItem(info, 1, item)  0)
+goto itemError;
+
+if ((item = PyLong_FromLongLong((long long)cpuinfo[i].cpuTime)) == 
NULL ||
+PyTuple_SetItem(info, 2, item)  0)
+goto itemError;
+
+if ((item = PyInt_FromLong((long)cpuinfo[i].cpu)) == NULL ||
+PyTuple_SetItem(info, 3, item)  0)
+goto itemError;
+
+if (PyList_SetItem(pycpuinfo, i, info)  0)
+goto itemError;
+
+continue;
+itemError:
+Py_DECREF(info);
+Py_XDECREF(item);
+goto cleanup;
 }
 for (i = 0 ; i  dominfo.nrVirtCpu ; i++) {
 PyObject *info = PyTuple_New(VIR_NODEINFO_MAXCPUS(nodeinfo));
@@ -1400,36 +1428,48 @@ libvirt_virDomainGetVcpus(PyObject *self 
ATTRIBUTE_UNUSED,
 if (info == NULL)
 goto cleanup;
 for (j = 0 ; j  VIR_NODEINFO_MAXCPUS(nodeinfo) ; j++) {
-PyTuple_SetItem(info, j, PyBool_FromLong(VIR_CPU_USABLE(cpumap, 
cpumaplen, i, j)));
+PyObject *item = NULL;
+if ((item = PyBool_FromLong(VIR_CPU_USABLE(cpumap, cpumaplen, i, 
j))) == NULL ||
+PyTuple_SetItem(info, j, item)  0) {
+Py_DECREF(info);
+Py_XDECREF(item);
+goto cleanup;
+}
+}
+if (PyList_SetItem(pycpumap, i, info)  0) {
+Py_DECREF(info);
+goto cleanup;
 }
-PyList_SetItem(pycpumap, i, 

Re: [libvirt] [PATCH 7/9] Add systemd journal support

2012-09-28 Thread Daniel P. Berrange
On Thu, Sep 27, 2012 at 02:51:35PM -0600, Eric Blake wrote:
 On 09/27/2012 10:44 AM, Daniel P. Berrange wrote:
  +/* Message was too large, so dump to temporary file
  + * and pass an FD to the journal
  + */
  +
  +if ((buffd = mkostemp(path, O_CLOEXEC|O_RDWR))  0)
 
 Is mkostemp async-signal safe?  But if it isn't, I don't know how else
 to generate a safe file name.  Maybe we create ourselves a safe
 temporary directory at process start where we don't care about the async
 safety issues, and then in this function, we track a static counter that
 we increment each time we create a new file within that directory.

I've looked the glibc source and the only functions they use are
open() and gettimeofday(), and the latter isn't actually used on
most architectures, instead it uses inline asm to read a CPU
timesource like the TSC. So IMHO, since this will be protected by
a #ifdef __linux__ we will be safe in using it.

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] fix-up an implicit build dead-loop in cfg.mk

2012-09-28 Thread Eric Blake
On 09/27/2012 09:51 PM, liguang wrote:
 if gnulib submodule happened to be dirty, build
 process  will fall into '_autogen' target trap
 in cfg.mk recursively, so break this dead-loop.

Yes, I've run into this before; thanks for trying to tackle it.

 
 Signed-off-by: liguang lig.f...@cn.fujitsu.com
 ---
  cfg.mk |2 +-
  1 files changed, 1 insertions(+), 1 deletions(-)
 
 diff --git a/cfg.mk b/cfg.mk
 index bbfd4a2..92966d5 100644
 --- a/cfg.mk
 +++ b/cfg.mk
 @@ -699,7 +699,7 @@ ifeq (0,$(MAKELEVEL))
test $$stamp = $$actual; echo $$?)
_clean_requested = $(filter %clean,$(MAKECMDGOALS))
ifeq (1,$(_update_required)$(_clean_requested))
 -$(info INFO: gnulib update required; running ./autogen.sh first)
 +$(error ERR: gnulib update required; running ./autogen.sh first)

However, this is not the right fix - it will error out even when .gnulib
is not dirty, but just out of date.  I'll spend some time on this
getting it right today.

-- 
Eric Blake   ebl...@redhat.com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH] python: keep consistent handling of Python integer conversion

2012-09-28 Thread Guannan Ren
libvirt_ulonglongUnwrap requires the integer type of python obj.
But libvirt_longlongUnwrap still could handle python obj of
Pyfloat_type which causes the float value to be rounded up
to an integer.

For example
  dom.setSchedulerParameters({'vcpu_quota': 0.88})
 0
libvirt_longlongUnwrap treats 0.88 as a valid value 0

However
  dom.setSchedulerParameters({'cpu_shares': 1000.22})
libvirt_ulonglongUnwrap will throw out an error
TypeError: an integer is required

The patch make this consistent.
---
 python/typewrappers.c | 8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/python/typewrappers.c b/python/typewrappers.c
index 7580689..d633603 100644
--- a/python/typewrappers.c
+++ b/python/typewrappers.c
@@ -220,7 +220,7 @@ libvirt_ulongUnwrap(PyObject *obj, unsigned long *val)
 int
 libvirt_longlongUnwrap(PyObject *obj, long long *val)
 {
-long long llong_val;
+long long llong_val = -1;
 
 if (!obj) {
 PyErr_SetString(PyExc_TypeError, unexpected type);
@@ -230,7 +230,11 @@ libvirt_longlongUnwrap(PyObject *obj, long long *val)
 /* If obj is of PyInt_Type, PyLong_AsLongLong
  * will call PyInt_AsLong() to handle it automatically.
  */
-llong_val = PyLong_AsLongLong(obj);
+if (PyInt_Check(obj) || PyLong_Check(obj))
+llong_val = PyLong_AsLongLong(obj);
+else
+PyErr_SetString(PyExc_TypeError, an integer is required);
+
 if ((llong_val == -1)  PyErr_Occurred())
 return -1;
 
-- 
1.7.11.2

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


Re: [libvirt] [PATCH 7/9] Add systemd journal support

2012-09-28 Thread Eric Blake
On 09/28/2012 02:08 AM, Daniel P. Berrange wrote:

 +static int virLogAddOutputToJournald(int priority)
 +{
 +if ((journalfd = socket(AF_UNIX, SOCK_DGRAM, 0))  0)
 +return -1;
 +if (virSetInherit(journalfd, false)  0) {

 Why not use SOCK_DGRAM | SOCK_CLOEXEC in the socket() call?
 
 It wasn't clear to me whether GNULIB provides compat for
 SOCK_CLOEXEC or not. Even though we're only Linux, older
 glibc won't provide that

I looked; gnulib guarantees SOCK_CLOEXEC for accept4(), but has not yet
ported it to socket().  I guess we stick with two calls for now.

 
 @@ -1114,6 +1285,12 @@ virLogParseOutputs(const char *outputs)
  count++;
  VIR_FREE(name);
  VIR_FREE(abspath);
 +} else if (STREQLEN(cur, journald, 8)) {
 +cur += 8;
 +#if HAVE_SYSLOG_H

 Do we really want to silently parse and ignore 'journald' on systems
 that lack syslog.h?
 
 I just followed the same pattern that we used with the
 syslog parsing.

Reading patch 8/9 makes me concur - there, we try a log style, then if
is it silently ignored, virLogGetNbOutputs() returns 0, so we try the
next log style.  This is fine after all.

-- 
Eric Blake   ebl...@redhat.com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 7/9] Add systemd journal support

2012-09-28 Thread Eric Blake
On 09/28/2012 04:09 AM, Daniel P. Berrange wrote:
 On Thu, Sep 27, 2012 at 02:51:35PM -0600, Eric Blake wrote:
 On 09/27/2012 10:44 AM, Daniel P. Berrange wrote:
 +/* Message was too large, so dump to temporary file
 + * and pass an FD to the journal
 + */
 +
 +if ((buffd = mkostemp(path, O_CLOEXEC|O_RDWR))  0)

 Is mkostemp async-signal safe?  But if it isn't, I don't know how else
 to generate a safe file name.  Maybe we create ourselves a safe
 temporary directory at process start where we don't care about the async
 safety issues, and then in this function, we track a static counter that
 we increment each time we create a new file within that directory.
 
 I've looked the glibc source and the only functions they use are
 open() and gettimeofday(), and the latter isn't actually used on
 most architectures, instead it uses inline asm to read a CPU
 timesource like the TSC. So IMHO, since this will be protected by
 a #ifdef __linux__ we will be safe in using it.

Sure, but add a big fat comment explaining our choice, so that people
reading the code later remember our discussion :)

-- 
Eric Blake   ebl...@redhat.com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH v2] Add systemd journal support

2012-09-28 Thread Daniel P. Berrange
From: Daniel P. Berrange berra...@redhat.com

Add support for logging to the systemd journal, using its
simple client library. The benefit over syslog is that it
accepts structured log data, so the journald can store
individual items like code file/line/func separately from
the string message. Tools which require structured log
data can then query the journal to extract exactly what
they desire without resorting to string parsing

While systemd provides a simple client library for logging,
it is more convenient for libvirt to directly write its
own client code. This lets us build up the iovec's on
the stack, avoiding the need to alloc memory when writing
log messages.

Changed in v2:

 - Add virFormatIntDecimal instead of using snprintf
 - Add comment about mkostemp
 - Fix declaration of linestr var to use size of linenr
 - Wrap in #ifdef __linux__

Signed-off-by: Daniel P. Berrange berra...@redhat.com
---
 src/libvirt_private.syms |   1 +
 src/util/logging.c   | 184 +++
 src/util/logging.h   |   1 +
 src/util/util.c  |  30 
 src/util/util.h  |   5 +-
 5 files changed, 220 insertions(+), 1 deletion(-)

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index dab607a..eebc52a 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1233,6 +1233,7 @@ virFileUnlock;
 virFileWaitForDevices;
 virFileWriteStr;
 virFindFileInPath;
+virFormatIntDecimal;
 virGetGroupID;
 virGetGroupName;
 virGetHostname;
diff --git a/src/util/logging.c b/src/util/logging.c
index cdd94fd..9e44880 100644
--- a/src/util/logging.c
+++ b/src/util/logging.c
@@ -35,6 +35,10 @@
 #if HAVE_SYSLOG_H
 # include syslog.h
 #endif
+#include sys/socket.h
+#if HAVE_SYS_UN_H
+# include sys/un.h
+#endif
 
 #include virterror_internal.h
 #include logging.h
@@ -44,6 +48,7 @@
 #include threads.h
 #include virfile.h
 #include virtime.h
+#include intprops.h
 
 #define VIR_FROM_THIS VIR_FROM_NONE
 
@@ -146,6 +151,8 @@ virLogOutputString(virLogDestination ldest)
 return syslog;
 case VIR_LOG_TO_FILE:
 return file;
+case VIR_LOG_TO_JOURNALD:
+return journald;
 }
 return unknown;
 }
@@ -1020,6 +1027,177 @@ virLogAddOutputToSyslog(virLogPriority priority,
 }
 return 0;
 }
+
+
+# ifdef __linux__
+#  define IOVEC_SET_STRING(iov, str) \
+do { \
+struct iovec *_i = (iov);   \
+_i-iov_base = (char*)str;   \
+_i-iov_len = strlen(str);   \
+} while (0)
+
+#  define IOVEC_SET_INT(iov, buf, val)  \
+do {\
+struct iovec *_i = (iov);  \
+_i-iov_base = virFormatIntDecimal(buf, sizeof(buf), val);  \
+_i-iov_len = strlen(buf);  \
+} while (0)
+
+static int journalfd = -1;
+
+static void
+virLogOutputToJournald(virLogSource source,
+   virLogPriority priority,
+   const char *filename,
+   int linenr,
+   const char *funcname,
+   const char *timestamp ATTRIBUTE_UNUSED,
+   unsigned int flags,
+   const char *rawstr,
+   const char *str ATTRIBUTE_UNUSED,
+   void *data ATTRIBUTE_UNUSED)
+{
+virCheckFlags(VIR_LOG_STACK_TRACE,);
+int buffd = -1;
+size_t niov = 0;
+struct msghdr mh;
+struct sockaddr_un sa;
+union {
+struct cmsghdr cmsghdr;
+uint8_t buf[CMSG_SPACE(sizeof(int))];
+} control;
+struct cmsghdr *cmsg;
+/* We use /dev/shm instead of /tmp here, since we want this to
+ * be a tmpfs, and one that is available from early boot on
+ * and where unprivileged users can create files. */
+char path[] = /dev/shm/journal.XX;
+char priostr[INT_BUFSIZE_BOUND(priority)];
+char linestr[INT_BUFSIZE_BOUND(linenr)];
+
+/* First message takes upto 4 iovecs, and each
+ * other field needs 3, assuming they don't have
+ * newlines in them
+ */
+#  define IOV_SIZE (4 + (5 * 3))
+struct iovec iov[IOV_SIZE];
+
+if (strchr(rawstr, '\n')) {
+uint64_t nstr;
+/* If 'str' containes a newline, then we must
+ * encode the string length, since we can't
+ * rely on the newline for the field separator
+ */
+IOVEC_SET_STRING(iov[niov++], MESSAGE\n);
+nstr = htole64(strlen(rawstr));
+iov[niov].iov_base = (char*)nstr;
+iov[niov].iov_len = sizeof(nstr);
+niov++;
+} else {
+IOVEC_SET_STRING(iov[niov++], MESSAGE=);
+}
+IOVEC_SET_STRING(iov[niov++], rawstr);
+IOVEC_SET_STRING(iov[niov++], \n);
+
+IOVEC_SET_STRING(iov[niov++], PRIORITY=);
+IOVEC_SET_INT(iov[niov++], priostr, 

Re: [libvirt] [PATCH v3 20/20] Add support for detecting capablities using QMP commands

2012-09-28 Thread Daniel P. Berrange
On Thu, Sep 27, 2012 at 03:00:14PM +0200, Jiri Denemark wrote:
 On Tue, Sep 25, 2012 at 19:00:13 +0100, Daniel P. Berrange wrote:
  From: Daniel P. Berrange berra...@redhat.com
  
  Start a QEMU process using
  
 $QEMU -S -no-user-config -nodefconfig -nodefaults \
   -nographic -M none -qmp stdio
 
 -nodefconfig should not ever be used if QEMU supports -no-user-config. The
 reason is that -nodefconfig disables loading of all files even those that
 reside somewhere in /usr/share and may contain required data, such as CPU
 definitions (although they were moved back to the code recently) or machine
 type definitions if they ever implement the ideas to separate them from the
 code.

Hmm, yes, I forgot about that.
  @@ -1282,6 +1285,7 @@ static struct qemuCapsStringFlags 
  qemuCapsObjectPropsVirtioNet[] = {
   };
   
   static struct qemuCapsStringFlags qemuCapsObjectPropsPciAssign[] = {
  +{ rombar, QEMU_CAPS_PCI_ROMBAR },
   { configfd, QEMU_CAPS_PCI_CONFIGFD },
   { bootindex, QEMU_CAPS_PCI_BOOTINDEX },
   };
  @@ -1857,6 +1861,10 @@ qemuCapsProbeQMPCommands(qemuCapsPtr caps,
   qemuCapsSet(caps, QEMU_CAPS_BLOCKJOB_ASYNC);
   else if (STREQ(name, dump-guest-memory))
   qemuCapsSet(caps, QEMU_CAPS_DUMP_GUEST_MEMORY);
  +else if (STREQ(name, query-spice))
  +qemuCapsSet(caps, QEMU_CAPS_SPICE);
  +else if (STREQ(name, query-kvm))
  +qemuCapsSet(caps, QEMU_CAPS_KVM);
   VIR_FREE(name);
   }
   VIR_FREE(commands);
 
 Hmm, looks like these two hunks should rather go into the previous patch,
 shouldn't they?

No, the previous patch was just refactoring existing code. The existing
approach to detecting spice/kvm was to use -help parsing. Only with the
new QMP code in this patch, do we now detect it via the QMP command list


  @@ -1979,18 +2078,252 @@ qemuCapsPtr qemuCapsNewForBinary(const char 
  *binary)
* understands the 0.13.0+ notion of -device driver,.  */
   if (qemuCapsGet(caps, QEMU_CAPS_DEVICE) 
   strstr(help, -device driver,?) 
  -qemuCapsExtractDeviceStr(binary, caps)  0)
  -goto error;
  +qemuCapsExtractDeviceStr(caps-binary, caps)  0)
  +goto cleanup;
   
   if (qemuCapsProbeCPUModels(caps)  0)
  -goto error;
  +goto cleanup;
   
   if (qemuCapsProbeMachineTypes(caps)  0)
  -goto error;
  +goto cleanup;
  +
  +ret = 0;
  +cleanup:
  +virCommandFree(cmd);
  +return ret;
  +}
 
 I think you actually didn't want to remove VIR_FREE(help); from the cleanup
 section.

Yeah, bug.

  +static int
  +qemuCapsInitQMP(qemuCapsPtr caps)
  +{
  +int ret = -1;
  +virCommandPtr cmd = NULL;
  +virDomainObjPtr vm = NULL;
  +int socks[2] = { -1, -1 };
  +qemuMonitorPtr mon = NULL;
  +int major, minor, micro;
  +char *package;
  +
  +VIR_DEBUG(caps=%p, caps);
  +
  +if (socketpair(PF_UNIX, SOCK_STREAM, 0, socks)  0) {
  +virReportSystemError(errno, %s,
  + _(unable to create socket pair));
  +goto cleanup;
  +}
  +
  +if (!(vm = virDomainObjNew(NULL)))
  +goto cleanup;
 
 Heh, AFAICT, vm in qemuMonitorOpenFD is only used to fill in mon-vm so that
 the VM pointer can be passed to various callbacks when something asynchronous
 happens in the monitor. Looks like we can just safely pass NULL to
 qemuMonitorOpenFD() instead.

There is one place in the monitor which uses it, but we can avoid it.

 
  +
  +cmd = virCommandNewArgList(caps-binary,
  +   -S,
  +   -no-user-config,
  +   -nodefconfig,
 
 Remove the -nodefconfig parameter, since this code requires at least qemu-1.2
 and thus it will either support -no-user-config or be unusable anyway.
 
  +   -nodefaults,
  +   -nographic,
  +   -M, none,
  +   -qmp, stdio,
  +   NULL);
  +virCommandAddEnvPassCommon(cmd);
  +virCommandSetInputFD(cmd, socks[1]);
  +virCommandSetOutputFD(cmd, socks[1]);
  +virCommandClearCaps(cmd);
  +
  +if (virCommandRunAsync(cmd, NULL)  0)
  +goto cleanup;
 
 However, if qemu is not new enough to support -no-user-config, virCommand will
 fail and this would consider it a fatal error instead of falling back to help
 parsing.

Actually not, because we're running async here, so we don't detect the
failed QEMU at this point, only later in this method. There is a problem
here though - my current code will cause log messages to be generated
when hitting old QEMU which is undesirable. So I'm changing this to use
virCommandRun and -daemonize QEMU, so we can detect exit status for old
QEMU without polluting the log.

Enough changes that I'll re-post this.

Daniel
-- 
|: http://berrange.com  

Re: [libvirt] [PATCH v2] Add systemd journal support

2012-09-28 Thread Eric Blake
On 09/28/2012 07:08 AM, Daniel P. Berrange wrote:
 From: Daniel P. Berrange berra...@redhat.com
 
 Add support for logging to the systemd journal, using its
 simple client library. The benefit over syslog is that it
 accepts structured log data, so the journald can store
 individual items like code file/line/func separately from
 the string message. Tools which require structured log
 data can then query the journal to extract exactly what
 they desire without resorting to string parsing
 
 While systemd provides a simple client library for logging,
 it is more convenient for libvirt to directly write its
 own client code. This lets us build up the iovec's on
 the stack, avoiding the need to alloc memory when writing
 log messages.
 
 Changed in v2:
 
  - Add virFormatIntDecimal instead of using snprintf
  - Add comment about mkostemp
  - Fix declaration of linestr var to use size of linenr
  - Wrap in #ifdef __linux__
 

ACK with one nit:

 +# ifdef __linux__

 +static void
 +virLogOutputToJournald(virLogSource source,

 +# endif /* __linux__ */
  #endif /* HAVE_SYSLOG_H */
  
  #define IS_SPACE(cur)   \
 @@ -1114,6 +1292,12 @@ virLogParseOutputs(const char *outputs)
  count++;
  VIR_FREE(name);
  VIR_FREE(abspath);
 +} else if (STREQLEN(cur, journald, 8)) {
 +cur += 8;
 +#if HAVE_SYSLOG_H
 +if (virLogAddOutputToJournald(prio) == 0)

This will fail to compile on non-Linux systems that have syslog.h.
Make the condition:

#if HAVE_SYSLOG_H  defined __linux__

-- 
Eric Blake   ebl...@redhat.com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] Memory free in libvirt JNA

2012-09-28 Thread Daniel Veillard
  Hi Claudio,

sorry for the delay, I need to focuse one something else ATM !

On Mon, Sep 24, 2012 at 04:11:19PM +0200, Claudio Bley wrote:
 At Wed, 12 Sep 2012 20:23:28 +0800,
 Daniel Veillard wrote:
  
   I think we must provide the free functions for all the memory allocated 
   by libvirt.
  
Okay, can you work on making a patch ? To be honnest I'm very unlikely
to have time for this in the short term,
 
 I did notice the same thing (but was on vacation for quite some
 time). I already have a fix for virDomainGetSchedulerType in my local
 repository (see below).

  okay i see

 I'm using virFree to free the allocated memory which adds another
 level of indirection (PointerByReference). This is because JNA does
 not provide access to the C free() function by itself, AFAICS.

 -- 8 ---
 From cb58e9538afbfd7ce09c5c2a93e403a809e77113 Mon Sep 17 00:00:00 2001
 From: Claudio Bley cb...@av-test.de
 Date: Fri, 24 Aug 2012 11:09:49 +0200
 Subject: [PATCH] Fix memory leak for virDomainGetSchedulerType.
 
 The string returned by virDomainGetSchedulerType must be free'd
 explicitly.
 
 JNA docs say that we have to use a Pointer as a return type and
 free it ourselves. So, simply use virFree for that matter.
 ---
  src/main/java/org/libvirt/Domain.java  | 13 +
  src/main/java/org/libvirt/jna/Libvirt.java |  6 +-
  2 files changed, 14 insertions(+), 5 deletions(-)
 
 diff --git a/src/main/java/org/libvirt/Domain.java 
 b/src/main/java/org/libvirt/Domain.java
 index 1f15ee8..fc1f665 100644
 --- a/src/main/java/org/libvirt/Domain.java
 +++ b/src/main/java/org/libvirt/Domain.java
 @@ -14,7 +14,9 @@ import org.libvirt.jna.virVcpuInfo;
  
  import com.sun.jna.Native;
  import com.sun.jna.NativeLong;
 +import com.sun.jna.Pointer;
  import com.sun.jna.ptr.IntByReference;
 +import com.sun.jna.ptr.PointerByReference;
  
  /**
   * A virtual machine defined within libvirt.
 @@ -443,9 +445,11 @@ public class Domain {
  public SchedParameter[] getSchedulerParameters() throws LibvirtException 
 {
  IntByReference nParams = new IntByReference();
  SchedParameter[] returnValue = new SchedParameter[0];
 -String scheduler = libvirt.virDomainGetSchedulerType(VDP, nParams);
 +Pointer pScheduler = libvirt.virDomainGetSchedulerType(VDP, nParams);
  processError();
 -if (scheduler != null) {
 +if (pScheduler != null) {
 +String scheduler = pScheduler.getString(0);
 +libvirt.virFree(new PointerByReference(pScheduler));
  virSchedParameter[] nativeParams = new 
 virSchedParameter[nParams.getValue()];
  returnValue = new SchedParameter[nParams.getValue()];
  libvirt.virDomainGetSchedulerParameters(VDP, nativeParams, 
 nParams);
 @@ -470,10 +474,11 @@ public class Domain {
   */
  public String[] getSchedulerType() throws LibvirtException {
  IntByReference nParams = new IntByReference();
 -String returnValue = libvirt.virDomainGetSchedulerType(VDP, nParams);
 +Pointer pScheduler = libvirt.virDomainGetSchedulerType(VDP, nParams);
  processError();
  String[] array = new String[1];
 -array[0] = returnValue;
 +array[0] = pScheduler.getString(0);
 +libvirt.virFree(new PointerByReference(pScheduler));
  return array;
  }
  
 diff --git a/src/main/java/org/libvirt/jna/Libvirt.java 
 b/src/main/java/org/libvirt/jna/Libvirt.java
 index dc3a54b..e68d9ed 100644
 --- a/src/main/java/org/libvirt/jna/Libvirt.java
 +++ b/src/main/java/org/libvirt/jna/Libvirt.java
 @@ -7,6 +7,7 @@ import com.sun.jna.NativeLong;
  import com.sun.jna.Pointer;
  import com.sun.jna.ptr.IntByReference;
  import com.sun.jna.ptr.LongByReference;
 +import com.sun.jna.ptr.PointerByReference;
  
  /**
   * The libvirt interface which is exposed via JNA. The complete API is
 @@ -55,6 +56,9 @@ import com.sun.jna.ptr.LongByReference;
   *
   */
  public interface Libvirt extends Library {
 +// Memory management
 +void virFree(PointerByReference ptr);
 +
  // Callbacks
  /**
   * Callback interface for authorization
 @@ -186,7 +190,7 @@ public interface Libvirt extends Library {
  public String virDomainGetOSType(DomainPointer virDomainPtr);
  public int virDomainGetSchedulerParameters(DomainPointer virDomainPtr, 
 virSchedParameter[] params,
  IntByReference nparams);
 -public String virDomainGetSchedulerType(DomainPointer virDomainPtr, 
 IntByReference nparams);
 +Pointer virDomainGetSchedulerType(DomainPointer virDomainPtr, 
 IntByReference nparams);
  public int virDomainGetUUID(DomainPointer virDomainPtr, byte[] 
 uuidString);
  public int virDomainGetUUIDString(DomainPointer virDomainPtr, byte[] 
 uuidString);
  public int virDomainGetVcpus(DomainPointer virDomainPtr, virVcpuInfo[] 
 info, int maxInfo, byte[] cpumaps, int maplen);

  Okay, that makes sense. First do you have a small pointer 

[libvirt] [PATCH 4/4] Ignore error from query-cpu-definitions

2012-09-28 Thread Daniel P. Berrange
From: Daniel P. Berrange berra...@redhat.com

Some architectures provide the query-cpu-definitions command,
but are set to always return a GenericError from it :-(
Catch this  treat it as if there was an empty list of CPUs
returned

Signed-off-by: Daniel P. Berrange berra...@redhat.com
---
 src/qemu/qemu_capabilities.c |  2 +-
 src/qemu/qemu_monitor.c  |  1 -
 src/qemu/qemu_monitor_json.c | 12 
 3 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 682d3e6..2ae6fec 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -2240,7 +2240,7 @@ qemuCapsInitQMP(qemuCapsPtr caps)
 VIR_DEBUG(Got version %d.%d.%d (%s),
   major, minor, micro, NULLSTR(package));
 
-if (!(major = 1  minor = 1  micro = 90)) {
+if (!(major = 1 || (major == 1  minor = 1))) {
 VIR_DEBUG(Not new enough for QMP capabilities detection);
 ret = 0;
 goto cleanup;
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index 6c3f09e..85b0bc2 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -843,7 +843,6 @@ void qemuMonitorClose(qemuMonitorPtr mon)
 mon-watch = 0;
 }
 VIR_FORCE_CLOSE(mon-fd);
-fprintf(stderr, Closing monitr\n);
 }
 
 /* In case another thread is waiting for its monitor command to be
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index 166c431..bd52ce4 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -3957,6 +3957,18 @@ int qemuMonitorJSONGetCPUDefinitions(qemuMonitorPtr mon,
 
 ret = qemuMonitorJSONCommand(mon, cmd, reply);
 
+if (ret == 0) {
+/* Urgh, some QEMU architectures have the query-cpu-definitions
+ * command, but return 'GenericError' with string Not supported,
+ * instead of simply omitting the command entirely :-(
+ */
+if (qemuMonitorJSONHasError(reply, GenericError)) {
+ret = 0;
+goto cleanup;
+}
+ret = qemuMonitorJSONCheckError(cmd, reply);
+}
+
 if (ret == 0)
 ret = qemuMonitorJSONCheckError(cmd, reply);
 
-- 
1.7.11.4

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


[libvirt] [PATCH 1/4] Remove need to pass in a virDomainObjPtr instance to qemuMonitorOpen

2012-09-28 Thread Daniel P. Berrange
From: Daniel P. Berrange berra...@redhat.com

The qemuMonitorOpen method only needs a virDomainObjPtr in order
to access the QEMU pid. This is not critical when detecting the
QEMU capabilties, so can easily be skipped

Signed-off-by: Daniel P. Berrange berra...@redhat.com
---
 src/qemu/qemu_monitor.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index cd41dd7..cb121e8 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -283,7 +283,7 @@ qemuMonitorOpenUnix(const char *monitor, pid_t cpid)
 break;
 
 if ((errno == ENOENT || errno == ECONNREFUSED) 
-virProcessKill(cpid, 0) == 0) {
+cpid  virProcessKill(cpid, 0) == 0) {
 /* ENOENT   : Socket may not have shown up yet
  * ECONNREFUSED : Leftover socket hasn't been removed yet */
 continue;
@@ -788,7 +788,7 @@ qemuMonitorOpen(virDomainObjPtr vm,
 switch (config-type) {
 case VIR_DOMAIN_CHR_TYPE_UNIX:
 hasSendFD = true;
-if ((fd = qemuMonitorOpenUnix(config-data.nix.path, vm-pid))  0)
+if ((fd = qemuMonitorOpenUnix(config-data.nix.path, vm ? vm-pid : 
0))  0)
 return NULL;
 break;
 
-- 
1.7.11.4

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


[libvirt] [PATCH 0/4] Switch to using QMP for capabilities detection

2012-09-28 Thread Daniel P. Berrange
This is a fixup of the last patch in my previous series based on
the feedback obtained, and some flaws I identified myself

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


[libvirt] [PATCH 3/4] Add support for detecting capablities using QMP commands

2012-09-28 Thread Daniel P. Berrange
From: Daniel P. Berrange berra...@redhat.com

Start a QEMU process using

   $QEMU -S -no-user-config -nodefaults \
 -nographic -M none -qmp unix:/some/path,server,nowait

and talk QMP over stdio to discover what capabilities the
binary supports. This works for QEMU 1.2.0 or later and
for older QEMU automatically fallback to the old approach
of parsing -help and related command line args.

Signed-off-by: Daniel P. Berrange berra...@redhat.com
---
 src/qemu/qemu_capabilities.c | 394 +++
 1 file changed, 358 insertions(+), 36 deletions(-)

diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index f0442ee..682d3e6 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -35,6 +35,7 @@
 #include command.h
 #include bitmap.h
 #include virnodesuspend.h
+#include qemu_monitor.h
 
 #include sys/stat.h
 #include unistd.h
@@ -189,6 +190,8 @@ VIR_ENUM_IMPL(qemuCaps, QEMU_CAPS_LAST,
 struct _qemuCaps {
 virObject object;
 
+bool usedQMP;
+
 char *binary;
 time_t mtime;
 
@@ -1286,6 +1289,7 @@ static struct qemuCapsStringFlags 
qemuCapsObjectPropsVirtioNet[] = {
 };
 
 static struct qemuCapsStringFlags qemuCapsObjectPropsPciAssign[] = {
+{ rombar, QEMU_CAPS_PCI_ROMBAR },
 { configfd, QEMU_CAPS_PCI_CONFIGFD },
 { bootindex, QEMU_CAPS_PCI_BOOTINDEX },
 };
@@ -1866,6 +1870,10 @@ qemuCapsProbeQMPCommands(qemuCapsPtr caps,
 qemuCapsSet(caps, QEMU_CAPS_BLOCKJOB_ASYNC);
 else if (STREQ(name, dump-guest-memory))
 qemuCapsSet(caps, QEMU_CAPS_DUMP_GUEST_MEMORY);
+else if (STREQ(name, query-spice))
+qemuCapsSet(caps, QEMU_CAPS_SPICE);
+else if (STREQ(name, query-kvm))
+qemuCapsSet(caps, QEMU_CAPS_KVM);
 VIR_FREE(name);
 }
 VIR_FREE(commands);
@@ -1898,11 +1906,117 @@ qemuCapsProbeQMPEvents(qemuCapsPtr caps,
 }
 
 
+static int
+qemuCapsProbeQMPObjects(qemuCapsPtr caps,
+qemuMonitorPtr mon)
+{
+int nvalues;
+char **values;
+size_t i;
+
+if ((nvalues = qemuMonitorGetObjectTypes(mon, values))  0)
+return -1;
+qemuCapsProcessStringFlags(caps,
+   ARRAY_CARDINALITY(qemuCapsObjectTypes),
+   qemuCapsObjectTypes,
+   nvalues, values);
+qemuCapsFreeStringList(nvalues, values);
+
+for (i = 0 ; i  ARRAY_CARDINALITY(qemuCapsObjectProps); i++) {
+const char *type = qemuCapsObjectProps[i].type;
+if ((nvalues = qemuMonitorGetObjectProps(mon,
+ type,
+ values))  0)
+return -1;
+qemuCapsProcessStringFlags(caps,
+   qemuCapsObjectProps[i].nprops,
+   qemuCapsObjectProps[i].props,
+   nvalues, values);
+qemuCapsFreeStringList(nvalues, values);
+}
+
+/* Prefer -chardev spicevmc (detected earlier) over -device spicevmc */
+if (qemuCapsGet(caps, QEMU_CAPS_CHARDEV_SPICEVMC))
+qemuCapsClear(caps, QEMU_CAPS_DEVICE_SPICEVMC);
+
+return 0;
+}
+
+
+static int
+qemuCapsProbeQMPMachineTypes(qemuCapsPtr caps,
+ qemuMonitorPtr mon)
+{
+qemuMonitorMachineInfoPtr *machines = NULL;
+int nmachines = 0;
+int ret = -1;
+size_t i;
+
+if ((nmachines = qemuMonitorGetMachines(mon, machines))  0)
+goto cleanup;
+
+if (VIR_ALLOC_N(caps-machineTypes, nmachines)  0) {
+virReportOOMError();
+goto cleanup;
+}
+if (VIR_ALLOC_N(caps-machineAliases, nmachines)  0) {
+virReportOOMError();
+goto cleanup;
+}
+
+for (i = 0 ; i  nmachines ; i++) {
+if (machines[i]-alias) {
+if (!(caps-machineAliases[i] = strdup(machines[i]-name))) {
+virReportOOMError();
+goto cleanup;
+}
+if (!(caps-machineTypes[i] = strdup(machines[i]-alias))) {
+virReportOOMError();
+goto cleanup;
+}
+} else {
+if (!(caps-machineTypes[i] = strdup(machines[i]-name))) {
+virReportOOMError();
+goto cleanup;
+}
+}
+}
+
+ret = 0;
+
+cleanup:
+for (i = 0 ; i  nmachines ; i++)
+qemuMonitorMachineInfoFree(machines[i]);
+VIR_FREE(machines);
+return ret;
+}
+
+
+static int
+qemuCapsProbeQMPCPUDefinitions(qemuCapsPtr caps,
+   qemuMonitorPtr mon)
+{
+int ncpuDefinitions;
+char **cpuDefinitions;
+
+if ((ncpuDefinitions = qemuMonitorGetCPUDefinitions(mon, cpuDefinitions)) 
 0)
+return -1;
+
+caps-ncpuDefinitions = ncpuDefinitions;
+caps-cpuDefinitions = cpuDefinitions;
+
+return 0;
+}
+
+
 int qemuCapsProbeQMP(qemuCapsPtr caps,
   

[libvirt] [PATCH 2/4] Avoid bogus I/O event errors when closing the QEMU monitor

2012-09-28 Thread Daniel P. Berrange
From: Daniel P. Berrange berra...@redhat.com

After calling qemuMonitorClose(), it is still possible for
the QEMU monitor I/O event callback to get invoked. This
will trigger an error message because mon-fd has been set
to -1 at this point. Silently ignore the case where mon-fd
is -1, likewise for mon-watch being zero.

Signed-off-by: Daniel P. Berrange berra...@redhat.com
---
 src/qemu/qemu_monitor.c | 13 -
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index cb121e8..6c3f09e 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -538,6 +538,9 @@ static void qemuMonitorUpdateWatch(qemuMonitorPtr mon)
 VIR_EVENT_HANDLE_HANGUP |
 VIR_EVENT_HANDLE_ERROR;
 
+if (!mon-watch)
+return;
+
 if (mon-lastError.code == VIR_ERR_OK) {
 events |= VIR_EVENT_HANDLE_READABLE;
 
@@ -563,6 +566,11 @@ qemuMonitorIO(int watch, int fd, int events, void *opaque) 
{
 #if DEBUG_IO
 VIR_DEBUG(Monitor %p I/O on watch %d fd %d events %d, mon, watch, fd, 
events);
 #endif
+if (mon-fd == -1 || mon-watch == 0) {
+qemuMonitorUnlock(mon);
+virObjectUnref(mon);
+return;
+}
 
 if (mon-fd != fd || mon-watch != watch) {
 if (events  (VIR_EVENT_HANDLE_HANGUP | VIR_EVENT_HANDLE_ERROR))
@@ -830,9 +838,12 @@ void qemuMonitorClose(qemuMonitorPtr mon)
   mon=%p refs=%d, mon, mon-object.refs);
 
 if (mon-fd = 0) {
-if (mon-watch)
+if (mon-watch) {
 virEventRemoveHandle(mon-watch);
+mon-watch = 0;
+}
 VIR_FORCE_CLOSE(mon-fd);
+fprintf(stderr, Closing monitr\n);
 }
 
 /* In case another thread is waiting for its monitor command to be
-- 
1.7.11.4

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


Re: [libvirt] [PATCH 1/4] Remove need to pass in a virDomainObjPtr instance to qemuMonitorOpen

2012-09-28 Thread Eric Blake
On 09/28/2012 08:58 AM, Daniel P. Berrange wrote:
 From: Daniel P. Berrange berra...@redhat.com
 
 The qemuMonitorOpen method only needs a virDomainObjPtr in order
 to access the QEMU pid. This is not critical when detecting the
 QEMU capabilties, so can easily be skipped
 
 Signed-off-by: Daniel P. Berrange berra...@redhat.com
 ---
  src/qemu/qemu_monitor.c | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

ACK.

-- 
Eric Blake   ebl...@redhat.com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 2/4] Avoid bogus I/O event errors when closing the QEMU monitor

2012-09-28 Thread Eric Blake
On 09/28/2012 08:58 AM, Daniel P. Berrange wrote:
 From: Daniel P. Berrange berra...@redhat.com
 
 After calling qemuMonitorClose(), it is still possible for
 the QEMU monitor I/O event callback to get invoked. This
 will trigger an error message because mon-fd has been set
 to -1 at this point. Silently ignore the case where mon-fd
 is -1, likewise for mon-watch being zero.
 
 Signed-off-by: Daniel P. Berrange berra...@redhat.com
 ---
  src/qemu/qemu_monitor.c | 13 -
  1 file changed, 12 insertions(+), 1 deletion(-)
 

 @@ -830,9 +838,12 @@ void qemuMonitorClose(qemuMonitorPtr mon)
mon=%p refs=%d, mon, mon-object.refs);
  
  if (mon-fd = 0) {
 -if (mon-watch)
 +if (mon-watch) {
  virEventRemoveHandle(mon-watch);
 +mon-watch = 0;
 +}
  VIR_FORCE_CLOSE(mon-fd);
 +fprintf(stderr, Closing monitr\n);

Oops.

ACK with the debugging cruft elided.

-- 
Eric Blake   ebl...@redhat.com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 3/4] Add support for detecting capablities using QMP commands

2012-09-28 Thread Eric Blake
On 09/28/2012 08:58 AM, Daniel P. Berrange wrote:
 From: Daniel P. Berrange berra...@redhat.com
 
 Start a QEMU process using
 
$QEMU -S -no-user-config -nodefaults \
  -nographic -M none -qmp unix:/some/path,server,nowait
 
 and talk QMP over stdio to discover what capabilities the
 binary supports. This works for QEMU 1.2.0 or later and
 for older QEMU automatically fallback to the old approach
 of parsing -help and related command line args.
 
 Signed-off-by: Daniel P. Berrange berra...@redhat.com
 ---
  src/qemu/qemu_capabilities.c | 394 
 +++
  1 file changed, 358 insertions(+), 36 deletions(-)
 

 +
 +/* Capabilities that we assume are always enabled
 + * for QEMU = 1.2.0
 + */
 +static void
 +qemuCapsInitQMPBasic(qemuCapsPtr caps)
 +{
 +qemuCapsSet(caps, QEMU_CAPS_VNC_COLON);
 +qemuCapsSet(caps, QEMU_CAPS_NO_REBOOT);

I think qemuCapsSetList() would be nicer than setting one bit at a time,
but that doesn't change semantics.


 +
 +static int
 +qemuCapsInitQMP(qemuCapsPtr caps)
 +{
 +int ret = -1;
 +virCommandPtr cmd = NULL;
 +qemuMonitorPtr mon = NULL;
 +int major, minor, micro;
 +char *package;
 +int status = 0;
 +virDomainChrSourceDef config;
 +
 +memset(config, 0, sizeof(config));
 +config.type = VIR_DOMAIN_CHR_TYPE_UNIX;
 +config.data.nix.path = (char*)/tmp/qemu.sock;

This does not seem like a safe file name (it is easily guessable).
Should we instead be generating something under /var/run?

 +config.data.nix.listen = false;
 +
 +VIR_DEBUG(Try to get caps via QMP caps=%p, caps);
 +
 +cmd = virCommandNewArgList(caps-binary,
 +   -S,
 +   -no-user-config,
 +   -nodefaults,
 +   -nographic,
 +   -M, none,
 +   -qmp, unix:/tmp/qemu.sock,server,nowait,

And here, we should match whatever filename we actually use.

Other than that, this looks nice.

-- 
Eric Blake   ebl...@redhat.com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 4/4] Ignore error from query-cpu-definitions

2012-09-28 Thread Eric Blake
On 09/28/2012 08:58 AM, Daniel P. Berrange wrote:
 From: Daniel P. Berrange berra...@redhat.com
 
 Some architectures provide the query-cpu-definitions command,
 but are set to always return a GenericError from it :-(
 Catch this  treat it as if there was an empty list of CPUs
 returned
 
 Signed-off-by: Daniel P. Berrange berra...@redhat.com
 ---
  src/qemu/qemu_capabilities.c |  2 +-
  src/qemu/qemu_monitor.c  |  1 -
  src/qemu/qemu_monitor_json.c | 12 
  3 files changed, 13 insertions(+), 2 deletions(-)

ACK.

 +++ b/src/qemu/qemu_capabilities.c
 @@ -2240,7 +2240,7 @@ qemuCapsInitQMP(qemuCapsPtr caps)
  VIR_DEBUG(Got version %d.%d.%d (%s),
major, minor, micro, NULLSTR(package));
  
 -if (!(major = 1  minor = 1  micro = 90)) {
 +if (!(major = 1 || (major == 1  minor = 1))) {
  VIR_DEBUG(Not new enough for QMP capabilities detection);
  ret = 0;
  goto cleanup;

This needs to be squashed into 3/4.

 +++ b/src/qemu/qemu_monitor.c
 @@ -843,7 +843,6 @@ void qemuMonitorClose(qemuMonitorPtr mon)
  mon-watch = 0;
  }
  VIR_FORCE_CLOSE(mon-fd);
 -fprintf(stderr, Closing monitr\n);
  }
  

This needs to be squashed into 2/4.

-- 
Eric Blake   ebl...@redhat.com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 4/4] Ignore error from query-cpu-definitions

2012-09-28 Thread Daniel P. Berrange
On Fri, Sep 28, 2012 at 03:58:23PM +0100, Daniel P. Berrange wrote:
 From: Daniel P. Berrange berra...@redhat.com
 
 Some architectures provide the query-cpu-definitions command,
 but are set to always return a GenericError from it :-(
 Catch this  treat it as if there was an empty list of CPUs
 returned
 
 Signed-off-by: Daniel P. Berrange berra...@redhat.com
 ---
  src/qemu/qemu_capabilities.c |  2 +-
  src/qemu/qemu_monitor.c  |  1 -
  src/qemu/qemu_monitor_json.c | 12 
  3 files changed, 13 insertions(+), 2 deletions(-)
 
 diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
 index 682d3e6..2ae6fec 100644
 --- a/src/qemu/qemu_capabilities.c
 +++ b/src/qemu/qemu_capabilities.c
 @@ -2240,7 +2240,7 @@ qemuCapsInitQMP(qemuCapsPtr caps)
  VIR_DEBUG(Got version %d.%d.%d (%s),
major, minor, micro, NULLSTR(package));
  
 -if (!(major = 1  minor = 1  micro = 90)) {
 +if (!(major = 1 || (major == 1  minor = 1))) {
  VIR_DEBUG(Not new enough for QMP capabilities detection);
  ret = 0;
  goto cleanup;

Opps, this was intended for the previous patch.

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 3/4] Add support for detecting capablities using QMP commands

2012-09-28 Thread Daniel P. Berrange
On Fri, Sep 28, 2012 at 09:20:01AM -0600, Eric Blake wrote:
 On 09/28/2012 08:58 AM, Daniel P. Berrange wrote:
  From: Daniel P. Berrange berra...@redhat.com
  
  Start a QEMU process using
  
 $QEMU -S -no-user-config -nodefaults \
   -nographic -M none -qmp unix:/some/path,server,nowait
  
  and talk QMP over stdio to discover what capabilities the
  binary supports. This works for QEMU 1.2.0 or later and
  for older QEMU automatically fallback to the old approach
  of parsing -help and related command line args.
  
  Signed-off-by: Daniel P. Berrange berra...@redhat.com
  ---
   src/qemu/qemu_capabilities.c | 394 
  +++
   1 file changed, 358 insertions(+), 36 deletions(-)
  
 
  +
  +/* Capabilities that we assume are always enabled
  + * for QEMU = 1.2.0
  + */
  +static void
  +qemuCapsInitQMPBasic(qemuCapsPtr caps)
  +{
  +qemuCapsSet(caps, QEMU_CAPS_VNC_COLON);
  +qemuCapsSet(caps, QEMU_CAPS_NO_REBOOT);
 
 I think qemuCapsSetList() would be nicer than setting one bit at a time,
 but that doesn't change semantics.
 
 
  +
  +static int
  +qemuCapsInitQMP(qemuCapsPtr caps)
  +{
  +int ret = -1;
  +virCommandPtr cmd = NULL;
  +qemuMonitorPtr mon = NULL;
  +int major, minor, micro;
  +char *package;
  +int status = 0;
  +virDomainChrSourceDef config;
  +
  +memset(config, 0, sizeof(config));
  +config.type = VIR_DOMAIN_CHR_TYPE_UNIX;
  +config.data.nix.path = (char*)/tmp/qemu.sock;
 
 This does not seem like a safe file name (it is easily guessable).
 Should we instead be generating something under /var/run?

Of course it isn't :-( I blame the end of week exhaustion. Locally
I have an addition to make it use driver-libDir, as we do for normal
monitor sockets for actual VMs.


Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|

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


[libvirt] [PATCH] node_memory: Add new parameter field to tune the new sysfs knob

2012-09-28 Thread Osier Yang
Upstream kernel introduced new sysfs knob merge_across_nodes to
specify if pages from different numa nodes can be merged. When set
to 0, only pages which physically reside in the memory area of
same NUMA node can be merged. When set to 1, pages from all nodes
can be merged.

This patch supports the tuning by adding new param field
shm-merge-across-nodes.

---
Perhaps two bool options like --disable-shm-merge-across-nodes,
and --enable-shm-merge-across-nodes are good, but could the
kernel sysfs knob have other values beyond 0 and 1 in future?
---
 include/libvirt/libvirt.h.in |   10 ++
 src/nodeinfo.c   |   26 +++---
 tools/virsh-host.c   |   20 
 tools/virsh.pod  |5 -
 4 files changed, 57 insertions(+), 4 deletions(-)

diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
index 81f12a4..56ee2f5 100644
--- a/include/libvirt/libvirt.h.in
+++ b/include/libvirt/libvirt.h.in
@@ -4489,6 +4489,16 @@ typedef virMemoryParameter *virMemoryParameterPtr;
  */
 # define VIR_NODE_MEMORY_SHARED_FULL_SCANS shm_full_scans
 
+/* VIR_NODE_MEMORY_SHARED_MERGE_ACROSS_NODES:
+ *
+ * Macro for typed parameter that represents whether pages from
+ * different NUMA nodes can be merged, when its value is 0, only
+ * pages which physically reside in the memory area of same NUMA
+ * node are merged, otherwise pages from all nodes can be merged.
+ */
+# define VIR_NODE_MEMORY_SHARED_MERGE_ACROSS_NODES shm_merge_across_nodes
+
+
 int virNodeGetMemoryParameters(virConnectPtr conn,
virTypedParameterPtr params,
int *nparams,
diff --git a/src/nodeinfo.c b/src/nodeinfo.c
index 021eb05..6002789 100644
--- a/src/nodeinfo.c
+++ b/src/nodeinfo.c
@@ -1004,6 +1004,13 @@ nodeSetMemoryParameters(virConnectPtr conn 
ATTRIBUTE_UNUSED,
 /* Out of memory */
 if (ret == -2)
 return -1;
+} else if (STREQ(param-field,
+ VIR_NODE_MEMORY_SHARED_MERGE_ACROSS_NODES)) {
+ret = nodeSetMemoryParameterValue(merge_across_nodes, param);
+
+/* Out of memory */
+if (ret == -2)
+return -1;
 }
 }
 
@@ -1039,8 +1046,9 @@ nodeGetMemoryParameterValue(const char *field,
 if ((tmp = strchr(buf, '\n')))
 *tmp = '\0';
 
-if (STREQ(field, pages_to_scan) ||
-STREQ(field, sleep_millisecs))
+if (STREQ(field, pages_to_scan)   ||
+STREQ(field, sleep_millisecs) ||
+STREQ(field, merge_across_nodes))
 rc = virStrToLong_ui(buf, NULL, 10, (unsigned int *)value);
 else if (STREQ(field, pages_shared)||
  STREQ(field, pages_sharing)   ||
@@ -1063,7 +1071,7 @@ cleanup:
 }
 #endif
 
-#define NODE_MEMORY_PARAMETERS_NUM 7
+#define NODE_MEMORY_PARAMETERS_NUM 8
 int
 nodeGetMemoryParameters(virConnectPtr conn ATTRIBUTE_UNUSED,
 virTypedParameterPtr params ATTRIBUTE_UNUSED,
@@ -1075,6 +1083,7 @@ nodeGetMemoryParameters(virConnectPtr conn 
ATTRIBUTE_UNUSED,
 #ifdef __linux__
 unsigned int pages_to_scan;
 unsigned int sleep_millisecs;
+unsigned int merge_across_nodes;
 unsigned long long pages_shared;
 unsigned long long pages_sharing;
 unsigned long long pages_unshared;
@@ -1168,6 +1177,17 @@ nodeGetMemoryParameters(virConnectPtr conn 
ATTRIBUTE_UNUSED,
 
 break;
 
+case 7:
+if (nodeGetMemoryParameterValue(merge_across_nodes,
+merge_across_nodes)  0)
+return -1;
+
+if (virTypedParameterAssign(param, 
VIR_NODE_MEMORY_SHARED_MERGE_ACROSS_NODES,
+VIR_TYPED_PARAM_UINT, 
merge_across_nodes)  0)
+return -1;
+
+break;
+
 default:
 break;
 }
diff --git a/tools/virsh-host.c b/tools/virsh-host.c
index 2c46336..62ecafc 100644
--- a/tools/virsh-host.c
+++ b/tools/virsh-host.c
@@ -900,6 +900,8 @@ static const vshCmdOptDef opts_node_memory_tune[] = {
 {shm-sleep-millisecs, VSH_OT_INT, VSH_OFLAG_NONE,
   N_(number of millisecs the shared memory service should 
  sleep before next scan)},
+{shm-merge-across-nodes, VSH_OT_INT, VSH_OFLAG_NONE,
+  N_(Specifies if pages from different numa nodes can be merged)},
 {NULL, 0, 0, NULL}
 };
 
@@ -911,6 +913,7 @@ cmdNodeMemoryTune(vshControl *ctl, const vshCmd *cmd)
 unsigned int flags = 0;
 unsigned int shm_pages_to_scan = 0;
 unsigned int shm_sleep_millisecs = 0;
+unsigned int shm_merge_across_nodes = 0;
 bool ret = false;
 int i = 0;
 
@@ -926,12 +929,21 @@ cmdNodeMemoryTune(vshControl *ctl, const vshCmd *cmd)
 return false;
 }
 
+if (vshCommandOptUInt(cmd, shm-merge-across-nodes,
+  shm_merge_across_nodes)  0) {
+vshError(ctl, %s, _(invalid 

Re: [libvirt] virsh --connect not working

2012-09-28 Thread hiren panchasara
On Fri, Sep 28, 2012 at 1:11 AM, Daniel P. Berrange berra...@redhat.comwrote:

 On Thu, Sep 27, 2012 at 11:15:56PM -0700, hiren panchasara wrote:
  I've installed libvirt-0.9.13 from ports on my freebsd machine.
 
  I started libvirtd:
 
  $ ps awwux | grep libvirtd
  root   11470   0.0  0.4  103100  31948  -  I10:41PM   0:00.35
  libvirtd -v -d
 
  $ sudo virsh --connect qemu:///system
  error: no connection driver available for No connection for URI
  qemu:///system
  error: failed to connect to the hypervisor
 
  I am sure I am missing something obvious.


 Try collecting some debugging output using

   LIBVIRT_DEBUG=1 sudo virsh --connect qemu:///system

 It should give a better idea why it failed.


Not getting any useful info:

$ ps awwux | grep libvirtd
root   3396   0.0  0.4  103100  32248  -  I 8:24AM  0:01.05
libvirtd -v -d

$ LIBVIRT_DEBUG=1 sudo virsh --connect qemu:///system
error: no connection driver available for No connection for URI
qemu:///system
error: failed to connect to the hypervisor

Tried 1-4 debug levels with the same response.

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

[libvirt] [PATCH] Add support for detecting capablities using QMP commands

2012-09-28 Thread Daniel P. Berrange
From: Daniel P. Berrange berra...@redhat.com

Start a QEMU process using

   $QEMU -S -no-user-config -nodefaults \
 -nographic -M none -qmp unix:/some/path,server,nowait

and talk QMP over stdio to discover what capabilities the
binary supports. This works for QEMU 1.2.0 or later and
for older QEMU automatically fallback to the old approach
of parsing -help and related command line args.

Signed-off-by: Daniel P. Berrange berra...@redhat.com
---
 src/qemu/qemu_capabilities.c | 419 +++
 src/qemu/qemu_capabilities.h |   5 +-
 src/qemu/qemu_driver.c   |   2 +-
 3 files changed, 385 insertions(+), 41 deletions(-)

diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index f0442ee..c977e25 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -35,6 +35,7 @@
 #include command.h
 #include bitmap.h
 #include virnodesuspend.h
+#include qemu_monitor.h
 
 #include sys/stat.h
 #include unistd.h
@@ -189,6 +190,8 @@ VIR_ENUM_IMPL(qemuCaps, QEMU_CAPS_LAST,
 struct _qemuCaps {
 virObject object;
 
+bool usedQMP;
+
 char *binary;
 time_t mtime;
 
@@ -210,6 +213,7 @@ struct _qemuCaps {
 struct _qemuCapsCache {
 virMutex lock;
 virHashTablePtr binaries;
+char *libDir;
 };
 
 
@@ -1286,6 +1290,7 @@ static struct qemuCapsStringFlags 
qemuCapsObjectPropsVirtioNet[] = {
 };
 
 static struct qemuCapsStringFlags qemuCapsObjectPropsPciAssign[] = {
+{ rombar, QEMU_CAPS_PCI_ROMBAR },
 { configfd, QEMU_CAPS_PCI_CONFIGFD },
 { bootindex, QEMU_CAPS_PCI_BOOTINDEX },
 };
@@ -1866,6 +1871,10 @@ qemuCapsProbeQMPCommands(qemuCapsPtr caps,
 qemuCapsSet(caps, QEMU_CAPS_BLOCKJOB_ASYNC);
 else if (STREQ(name, dump-guest-memory))
 qemuCapsSet(caps, QEMU_CAPS_DUMP_GUEST_MEMORY);
+else if (STREQ(name, query-spice))
+qemuCapsSet(caps, QEMU_CAPS_SPICE);
+else if (STREQ(name, query-kvm))
+qemuCapsSet(caps, QEMU_CAPS_KVM);
 VIR_FREE(name);
 }
 VIR_FREE(commands);
@@ -1898,11 +1907,117 @@ qemuCapsProbeQMPEvents(qemuCapsPtr caps,
 }
 
 
+static int
+qemuCapsProbeQMPObjects(qemuCapsPtr caps,
+qemuMonitorPtr mon)
+{
+int nvalues;
+char **values;
+size_t i;
+
+if ((nvalues = qemuMonitorGetObjectTypes(mon, values))  0)
+return -1;
+qemuCapsProcessStringFlags(caps,
+   ARRAY_CARDINALITY(qemuCapsObjectTypes),
+   qemuCapsObjectTypes,
+   nvalues, values);
+qemuCapsFreeStringList(nvalues, values);
+
+for (i = 0 ; i  ARRAY_CARDINALITY(qemuCapsObjectProps); i++) {
+const char *type = qemuCapsObjectProps[i].type;
+if ((nvalues = qemuMonitorGetObjectProps(mon,
+ type,
+ values))  0)
+return -1;
+qemuCapsProcessStringFlags(caps,
+   qemuCapsObjectProps[i].nprops,
+   qemuCapsObjectProps[i].props,
+   nvalues, values);
+qemuCapsFreeStringList(nvalues, values);
+}
+
+/* Prefer -chardev spicevmc (detected earlier) over -device spicevmc */
+if (qemuCapsGet(caps, QEMU_CAPS_CHARDEV_SPICEVMC))
+qemuCapsClear(caps, QEMU_CAPS_DEVICE_SPICEVMC);
+
+return 0;
+}
+
+
+static int
+qemuCapsProbeQMPMachineTypes(qemuCapsPtr caps,
+ qemuMonitorPtr mon)
+{
+qemuMonitorMachineInfoPtr *machines = NULL;
+int nmachines = 0;
+int ret = -1;
+size_t i;
+
+if ((nmachines = qemuMonitorGetMachines(mon, machines))  0)
+goto cleanup;
+
+if (VIR_ALLOC_N(caps-machineTypes, nmachines)  0) {
+virReportOOMError();
+goto cleanup;
+}
+if (VIR_ALLOC_N(caps-machineAliases, nmachines)  0) {
+virReportOOMError();
+goto cleanup;
+}
+
+for (i = 0 ; i  nmachines ; i++) {
+if (machines[i]-alias) {
+if (!(caps-machineAliases[i] = strdup(machines[i]-name))) {
+virReportOOMError();
+goto cleanup;
+}
+if (!(caps-machineTypes[i] = strdup(machines[i]-alias))) {
+virReportOOMError();
+goto cleanup;
+}
+} else {
+if (!(caps-machineTypes[i] = strdup(machines[i]-name))) {
+virReportOOMError();
+goto cleanup;
+}
+}
+}
+
+ret = 0;
+
+cleanup:
+for (i = 0 ; i  nmachines ; i++)
+qemuMonitorMachineInfoFree(machines[i]);
+VIR_FREE(machines);
+return ret;
+}
+
+
+static int
+qemuCapsProbeQMPCPUDefinitions(qemuCapsPtr caps,
+   qemuMonitorPtr mon)
+{
+int ncpuDefinitions;
+char **cpuDefinitions;
+
+if ((ncpuDefinitions = 

Re: [libvirt] virsh --connect not working

2012-09-28 Thread Daniel P. Berrange
On Fri, Sep 28, 2012 at 08:37:54AM -0700, hiren panchasara wrote:
 On Fri, Sep 28, 2012 at 1:11 AM, Daniel P. Berrange 
 berra...@redhat.comwrote:
 
  On Thu, Sep 27, 2012 at 11:15:56PM -0700, hiren panchasara wrote:
   I've installed libvirt-0.9.13 from ports on my freebsd machine.
  
   I started libvirtd:
  
   $ ps awwux | grep libvirtd
   root   11470   0.0  0.4  103100  31948  -  I10:41PM   0:00.35
   libvirtd -v -d
  
   $ sudo virsh --connect qemu:///system
   error: no connection driver available for No connection for URI
   qemu:///system
   error: failed to connect to the hypervisor
  
   I am sure I am missing something obvious.
 
 
  Try collecting some debugging output using
 
LIBVIRT_DEBUG=1 sudo virsh --connect qemu:///system
 
  It should give a better idea why it failed.
 
 
 Not getting any useful info:
 
 $ ps awwux | grep libvirtd
 root   3396   0.0  0.4  103100  32248  -  I 8:24AM  0:01.05
 libvirtd -v -d
 
 $ LIBVIRT_DEBUG=1 sudo virsh --connect qemu:///system
 error: no connection driver available for No connection for URI
 qemu:///system
 error: failed to connect to the hypervisor
 
 Tried 1-4 debug levels with the same response.

Sounds like sudo is stripping the env variable. Try running it as root
directly, without sudo.

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] Add support for detecting capablities using QMP commands

2012-09-28 Thread Eric Blake
On 09/28/2012 09:43 AM, Daniel P. Berrange wrote:
 From: Daniel P. Berrange berra...@redhat.com
 
 Start a QEMU process using
 
$QEMU -S -no-user-config -nodefaults \
  -nographic -M none -qmp unix:/some/path,server,nowait
 
 and talk QMP over stdio to discover what capabilities the
 binary supports. This works for QEMU 1.2.0 or later and
 for older QEMU automatically fallback to the old approach
 of parsing -help and related command line args.
 
 Signed-off-by: Daniel P. Berrange berra...@redhat.com
 ---
  src/qemu/qemu_capabilities.c | 419 
 +++
  src/qemu/qemu_capabilities.h |   5 +-
  src/qemu/qemu_driver.c   |   2 +-
  3 files changed, 385 insertions(+), 41 deletions(-)

 +char *monarg = NULL;
 +char *monpath = NULL;
 +
 +if (virAsprintf(monpath, %s/%s, libDir, capabilities.monitor.sock) 
  0) {
 +virReportOOMError();
 +goto cleanup;
 +}
 +if (virAsprintf(monarg, unix:%s,server,nowait, monpath)  0) {
 +virReportOOMError();
 +goto cleanup;
 +}

I would have used virCommandAddArgFormat() instead of creating my own
temporary string here, but that's minor.

ACK; you fixed my concerns from the previous version.

Also, be sure you run 'make check'; somewhere between commit de29867 and
7022b09, you broke qemuhelptest (I'm assuming it will get fixed soon,
perhaps by this commit, so I'm not chasing which commit did the actual
breakage):

16) QEMU Help String Parsing qemu-1.2.0
... qemu-1.2.0: computed flags do not match: got
0xb0f2f1bffefff4cffd62fffddc6e, expected
0xb0f2f1bffefff4effd72fffddc6e
Missing flag 36
Missing flag 53
FAILED

-- 
Eric Blake   ebl...@redhat.com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] virsh --connect not working

2012-09-28 Thread hiren panchasara
On Fri, Sep 28, 2012 at 8:40 AM, Daniel P. Berrange berra...@redhat.comwrote:

 On Fri, Sep 28, 2012 at 08:37:54AM -0700, hiren panchasara wrote:
  On Fri, Sep 28, 2012 at 1:11 AM, Daniel P. Berrange berra...@redhat.com
 wrote:
 
   On Thu, Sep 27, 2012 at 11:15:56PM -0700, hiren panchasara wrote:
I've installed libvirt-0.9.13 from ports on my freebsd machine.
   
I started libvirtd:
   
$ ps awwux | grep libvirtd
root   11470   0.0  0.4  103100  31948  -  I10:41PM   0:00.35
libvirtd -v -d
   
$ sudo virsh --connect qemu:///system
error: no connection driver available for No connection for URI
qemu:///system
error: failed to connect to the hypervisor
   
I am sure I am missing something obvious.
  
  
   Try collecting some debugging output using
  
 LIBVIRT_DEBUG=1 sudo virsh --connect qemu:///system
  
   It should give a better idea why it failed.
  
 
  Not getting any useful info:
 
  $ ps awwux | grep libvirtd
  root   3396   0.0  0.4  103100  32248  -  I 8:24AM  0:01.05
  libvirtd -v -d
 
  $ LIBVIRT_DEBUG=1 sudo virsh --connect qemu:///system
  error: no connection driver available for No connection for URI
  qemu:///system
  error: failed to connect to the hypervisor
 
  Tried 1-4 debug levels with the same response.

 Sounds like sudo is stripping the env variable. Try running it as root
 directly, without sudo.


Aah, right, o/p is huge so putting it in pastebin:
http://pastebin.com/YVrK0fRb

Thanks a lot,
Hiren
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] virsh --connect not working

2012-09-28 Thread Daniel P. Berrange
On Fri, Sep 28, 2012 at 08:51:40AM -0700, hiren panchasara wrote:
 On Fri, Sep 28, 2012 at 8:40 AM, Daniel P. Berrange 
 berra...@redhat.comwrote:
 
  On Fri, Sep 28, 2012 at 08:37:54AM -0700, hiren panchasara wrote:
   On Fri, Sep 28, 2012 at 1:11 AM, Daniel P. Berrange berra...@redhat.com
  wrote:
  
On Thu, Sep 27, 2012 at 11:15:56PM -0700, hiren panchasara wrote:
 I've installed libvirt-0.9.13 from ports on my freebsd machine.

 I started libvirtd:

 $ ps awwux | grep libvirtd
 root   11470   0.0  0.4  103100  31948  -  I10:41PM   0:00.35
 libvirtd -v -d

 $ sudo virsh --connect qemu:///system
 error: no connection driver available for No connection for URI
 qemu:///system
 error: failed to connect to the hypervisor

 I am sure I am missing something obvious.
   
   
Try collecting some debugging output using
   
  LIBVIRT_DEBUG=1 sudo virsh --connect qemu:///system
   
It should give a better idea why it failed.
   
  
   Not getting any useful info:
  
   $ ps awwux | grep libvirtd
   root   3396   0.0  0.4  103100  32248  -  I 8:24AM  0:01.05
   libvirtd -v -d
  
   $ LIBVIRT_DEBUG=1 sudo virsh --connect qemu:///system
   error: no connection driver available for No connection for URI
   qemu:///system
   error: failed to connect to the hypervisor
  
   Tried 1-4 debug levels with the same response.
 
  Sounds like sudo is stripping the env variable. Try running it as root
  directly, without sudo.
 
 
 Aah, right, o/p is huge so putting it in pastebin:
 http://pastebin.com/YVrK0fRb

Looks somewhat like your libvirt has been built without support
for the QEMU driver

Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|

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


[libvirt] [PATCH] Fix QEMU test with 1.2.0 help output

2012-09-28 Thread Daniel P. Berrange
From: Daniel P. Berrange berra...@redhat.com

The help output for QEMU 1.2.0 changed 'pci-assign' to 'kvm-pci-assign'.
Since the new capabilities code does exact device name matching
instead of substring matching, this caused the capabilities to go
missing.

Pushed as a build break fix

Signed-off-by: Daniel P. Berrange berra...@redhat.com
---
 src/qemu/qemu_capabilities.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index f0442ee..cac4917 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -1321,6 +1321,8 @@ static struct qemuCapsObjectTypeProps 
qemuCapsObjectProps[] = {
   ARRAY_CARDINALITY(qemuCapsObjectPropsVirtioNet) },
 { pci-assign, qemuCapsObjectPropsPciAssign,
   ARRAY_CARDINALITY(qemuCapsObjectPropsPciAssign) },
+{ kvm-pci-assign, qemuCapsObjectPropsPciAssign,
+  ARRAY_CARDINALITY(qemuCapsObjectPropsPciAssign) },
 { scsi-disk, qemuCapsObjectPropsScsiDisk,
   ARRAY_CARDINALITY(qemuCapsObjectPropsScsiDisk) },
 { ide-drive, qemuCapsObjectPropsIDEDrive,
-- 
1.7.11.4

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


Re: [libvirt] [PATCH 2/3] openvswitch: Add utility functions for getting and setting Open vSwitch per-port data

2012-09-28 Thread Laine Stump
On 09/21/2012 05:16 PM, Kyle Mestery wrote:
 Add utility functions for Open vSwitch to both save
 per-port data before a live migration, and restore the
 per-port data after a live migration.

 Signed-off-by: Kyle Mestery kmest...@cisco.com
 ---
  src/libvirt_private.syms|  2 ++
  src/util/virnetdevopenvswitch.c | 70 
 +
  src/util/virnetdevopenvswitch.h |  6 
  3 files changed, 78 insertions(+)

 diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
 index 0b6068d..54c591b 100644
 --- a/src/libvirt_private.syms
 +++ b/src/libvirt_private.syms
 @@ -1479,7 +1479,9 @@ virNetDevMacVLanVPortProfileRegisterCallback;
  
  # virnetdevopenvswitch.h
  virNetDevOpenvswitchAddPort;
 +virNetDevOpenvswitchGetMigrateData;
  virNetDevOpenvswitchRemovePort;
 +virNetDevOpenvswitchSetMigrateData;
  
  
  # virnetdevtap.h
 diff --git a/src/util/virnetdevopenvswitch.c b/src/util/virnetdevopenvswitch.c
 index a6993b6..c30cbaa 100644
 --- a/src/util/virnetdevopenvswitch.c
 +++ b/src/util/virnetdevopenvswitch.c
 @@ -179,3 +179,73 @@ int virNetDevOpenvswitchRemovePort(const char *brname 
 ATTRIBUTE_UNUSED, const ch
  virCommandFree(cmd);
  return ret;
  }
 +
 +/**
 + * virNetDevOpenvswitchGetMigrateData:
 + * @migrate: a pointer to store the data into, allocated by this function
 + * @ifname: name of the interface for which data is being migrated
 + *
 + * Allocates data to be migrated specific to Open vSwitch
 + *
 + * Returns 0 in case of success or -1 in case of failure
 + */
 +int virNetDevOpenvswitchGetMigrateData(char **migrate, const char *ifname)
 +{
 +virCommandPtr cmd = NULL;
 +int ret = 0;
 +
 +cmd = virCommandNewArgList(OVSVSCTL, get, Interface,
 +   ifname, external_ids:PortData, NULL);
 +
 +virCommandSetOutputBuffer(cmd, migrate);
 +
 +/* Run the command */
 +if (virCommandRun(cmd, NULL)  0) {
 +virReportSystemError(VIR_ERR_INTERNAL_ERROR,
 + _(Unable to run command to get OVS port data 
 for 
 + interface %s), ifname);
 +ret = -1;
 +goto error;
 +}
 +
 +/* Wipeout the newline */
 +(*migrate)[strlen(*migrate) - 1] = '\0';
 +
 +error:
 +return ret;
 +}
 +
 +/**
 + * virNetDevOpenvswitchSetMigrateData:
 + * @migrate: the data which was transferred during migration
 + * @ifname: the name of the interface the data is associated with
 + *
 + * Repopulates OVS per-port data on destination host
 + *
 + * Returns 0 in case of success or -1 in case of failure
 + */
 +int virNetDevOpenvswitchSetMigrateData(char *migrate, const char *ifname)
 +{
 +virCommandPtr cmd = NULL;
 +int ret = 0;
 +virBufferPtr buf;
 +
 +if (VIR_ALLOC(buf)  0) {
 +ret = -1;
 +goto error;
 +}
 +
 +virBufferAsprintf(buf, external_ids:PortData=%s, migrate);
 +
 +cmd = virCommandNewArgList(OVSVSCTL, set, Interface, ifname,
 +   virBufferCurrentContent(buf), NULL);


You need to add the --timeout=5 option as I did for add-port and
del-port, to avoid an infinite wait when ovs-vswitchd isn't running.

ACK with that change.

 +/* Run the command */
 +if ((ret = virCommandRun(cmd, NULL))  0) {
 +virReportSystemError(VIR_ERR_INTERNAL_ERROR,
 + _(Unable to run command to set OVS port data 
 for 
 + interface %s), ifname);
 +}
 +
 +error:
 +return ret;
 +}
 diff --git a/src/util/virnetdevopenvswitch.h b/src/util/virnetdevopenvswitch.h
 index 7e5b618..147cd6f 100644
 --- a/src/util/virnetdevopenvswitch.h
 +++ b/src/util/virnetdevopenvswitch.h
 @@ -42,4 +42,10 @@ int virNetDevOpenvswitchAddPort(const char *brname,
  int virNetDevOpenvswitchRemovePort(const char *brname, const char *ifname)
  ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK;
  
 +int virNetDevOpenvswitchGetMigrateData(char **migrate, const char *ifname)
 +ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK;
 +
 +int virNetDevOpenvswitchSetMigrateData(char *migrate, const char *ifname)
 +ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK;
 +
  #endif /* __VIR_NETDEV_OPENVSWITCH_H__ */

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


Re: [libvirt] [PATCH 1/3] qemu_migration: Add hooks to transport network port data during migration

2012-09-28 Thread Laine Stump
On 09/21/2012 05:16 PM, Kyle Mestery wrote:
 Add the ability for the Qemu V3 migration protocol to
 include transporting network configuration. A generic
 framework is proposed with this patch to allow for the
 transfer of opaque data.

Functionally this all looks good (and sounds like it lives up to that in
practice :-). There are some code style issues, though...


 Signed-off-by: Kyle Mestery kmest...@cisco.com
 ---
  src/qemu/qemu_migration.c | 278 
 +-
  1 file changed, 276 insertions(+), 2 deletions(-)

 diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
 index 8e85875..06981db 100644
 --- a/src/qemu/qemu_migration.c
 +++ b/src/qemu/qemu_migration.c
 @@ -70,6 +70,7 @@ enum qemuMigrationCookieFlags {
  QEMU_MIGRATION_COOKIE_FLAG_GRAPHICS,
  QEMU_MIGRATION_COOKIE_FLAG_LOCKSTATE,
  QEMU_MIGRATION_COOKIE_FLAG_PERSISTENT,
 +QEMU_MIGRATION_COOKIE_FLAG_NETWORK,
  
  QEMU_MIGRATION_COOKIE_FLAG_LAST
  };
 @@ -77,12 +78,13 @@ enum qemuMigrationCookieFlags {
  VIR_ENUM_DECL(qemuMigrationCookieFlag);
  VIR_ENUM_IMPL(qemuMigrationCookieFlag,
QEMU_MIGRATION_COOKIE_FLAG_LAST,
 -  graphics, lockstate, persistent);
 +  graphics, lockstate, persistent, network);
  
  enum qemuMigrationCookieFeatures {
  QEMU_MIGRATION_COOKIE_GRAPHICS  = (1  
 QEMU_MIGRATION_COOKIE_FLAG_GRAPHICS),
  QEMU_MIGRATION_COOKIE_LOCKSTATE = (1  
 QEMU_MIGRATION_COOKIE_FLAG_LOCKSTATE),
  QEMU_MIGRATION_COOKIE_PERSISTENT = (1  
 QEMU_MIGRATION_COOKIE_FLAG_PERSISTENT),
 +QEMU_MIGRATION_COOKIE_NETWORK = (1  
 QEMU_MIGRATION_COOKIE_FLAG_NETWORK),
  };
  
  typedef struct _qemuMigrationCookieGraphics qemuMigrationCookieGraphics;
 @@ -95,6 +97,35 @@ struct _qemuMigrationCookieGraphics {
  char *tlsSubject;
  };
  
 +VIR_ENUM_DECL(virInterface);
 +VIR_ENUM_IMPL(virInterface, VIR_NETDEV_VPORT_PROFILE_LAST,
 +  none,
 +  802.1QBg,
 +  802.1QBh,
 +  openvswitch);

(I'd never before considered using VIR_ENUM_*() and giving an enum type
identifier that didn't match the enum being translated)

Since you're using the enum virNetDevVPortProfile for these values
anyway, you should just use the already-existing functions:

 virNetDevVPortTypeFromString
 virNetDevVPortTypeToString

(these are declared/defined in virnetdevvportprofile.h, and included in
libvirt_private.syms).


 +
 +typedef struct _qemuMigrationCookieNetdata qemuMigrationCookieNetdata;
 +typedef qemuMigrationCookieNetdata *qemuMigrationCookieNetdataPtr;
 +struct _qemuMigrationCookieNetdata {
 +/* What type is each interace ? virInterface above ... */
 +char *interfacetype;

1) Since the xml attribute is called vporttype, you should give the
variable in the struct the same name, to avoid confusion.


2) Internally, we usually store the enum value rather than the string,
and translate to/from string form during XML parse/format. (For some
reason, normal usage is to specify it in the struct as an int, with a
comment giving the actual type:


int vporttype; /* enum virNetDevVPortProfile */


I'm not sure why that is, to be truthful - Eric or Dan?)


 +
 +/*
 + * Array of pointers to saved data. Each VIF will have it's own
 + * data to transfer.
 + */
 +char *portdata;
 +};
 +
 +typedef struct _qemuMigrationCookieNetwork qemuMigrationCookieNetwork;
 +typedef qemuMigrationCookieNetwork *qemuMigrationCookieNetworkPtr;
 +struct _qemuMigrationCookieNetwork {
 +/* How many virtual NICs are we saving data for? */
 +int nnets;
 +
 +qemuMigrationCookieNetdataPtr *net;
 +};
 +
  typedef struct _qemuMigrationCookie qemuMigrationCookie;
  typedef qemuMigrationCookie *qemuMigrationCookiePtr;
  struct _qemuMigrationCookie {
 @@ -120,6 +151,9 @@ struct _qemuMigrationCookie {
  
  /* If (flags  QEMU_MIGRATION_COOKIE_PERSISTENT) */
  virDomainDefPtr persistent;
 +
 +/* If (flags  QEMU_MIGRATION_COOKIE_NETWORK) */
 +qemuMigrationCookieNetworkPtr network;
  };
  
  static void qemuMigrationCookieGraphicsFree(qemuMigrationCookieGraphicsPtr 
 grap)
 @@ -132,6 +166,28 @@ static void 
 qemuMigrationCookieGraphicsFree(qemuMigrationCookieGraphicsPtr grap)
  }
  
  
 +static void qemuMigrationCookieNetworkFree(qemuMigrationCookieNetworkPtr
 +   network)
 +{
 +int i;
 +
 +if (!network)
 +return;
 +
 +for (i = 0; i  network-nnets; i++) {
 +if (network-net[i]) {
 +if (network-net[i]-interfacetype)
 +VIR_FREE(network-net[i]-interfacetype);

VIR_FREE() automatically checks for NULL, and is a NOP if the pointer is
NULL (it also sets the pointer to NULL when it's finished, so calling it
twice doesn't create a problem. make syntax-check will actually find
such redundant null-check-before-free and complain about it. So, you
should remove all checks for NULL before calling 

Re: [libvirt] [PATCH 3/3] qemu_migration: Transport Open vSwitch per-port data during live migration

2012-09-28 Thread Laine Stump
On 09/21/2012 05:16 PM, Kyle Mestery wrote:
 Transport Open vSwitch per-port data during live
 migration by using the utility functions
 virNetDevOpenvswitchGetMigrateData() and
 virNetDevOpenvswitchSetMigrateData().

I like how the first part is all re-usable infrastructure, and the final
patch that actually makes things work is so small! That implies that it
will be easy to add other new functionality in the future...


 Signed-off-by: Kyle Mestery kmest...@cisco.com
 ---
  src/qemu/qemu_migration.c | 23 +++
  1 file changed, 19 insertions(+), 4 deletions(-)

 diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
 index 06981db..33cdb20 100644
 --- a/src/qemu/qemu_migration.c
 +++ b/src/qemu/qemu_migration.c
 @@ -322,7 +322,7 @@ qemuMigrationCookieNetworkAlloc(struct qemud_driver 
 *driver ATTRIBUTE_UNUSED,
  {
  qemuMigrationCookieNetworkPtr mig;
  int i;
 -virDomainNetDefPtr netptr ATTRIBUTE_UNUSED;
 +virDomainNetDefPtr netptr;
  const char *interfacetype;
  
  if (VIR_ALLOC(mig)  0)
 @@ -357,7 +357,13 @@ qemuMigrationCookieNetworkAlloc(struct qemud_driver 
 *driver ATTRIBUTE_UNUSED,
  mig-net[i]-portdata = NULL;
  break;
  case VIR_NETDEV_VPORT_PROFILE_OPENVSWITCH:
 -mig-net[i]-portdata = NULL;
 +if 
 (virNetDevOpenvswitchGetMigrateData(mig-net[i]-portdata,
 +   netptr-ifname) != 0) 
 {
 +virReportSystemError(VIR_ERR_INTERNAL_ERROR,
 + _(Unable to run command to get 
 OVS port data for 
 + interface %s), 
 netptr-ifname);
 +goto error;
 +}
  break;
  default:
  mig-net[i]-portdata = NULL;
 @@ -366,6 +372,7 @@ qemuMigrationCookieNetworkAlloc(struct qemud_driver 
 *driver ATTRIBUTE_UNUSED,
  }
  }
  
 +error:
  return mig;
  
  no_memory:
 @@ -1284,7 +1291,7 @@ qemuDomainMigrateOPDRelocate(struct qemud_driver 
 *driver ATTRIBUTE_UNUSED,
   virDomainObjPtr vm,
   qemuMigrationCookiePtr cookie)
  {
 -virDomainNetDefPtr netptr ATTRIBUTE_UNUSED;
 +virDomainNetDefPtr netptr;
  int ret = 0;

These days we tend to 1) initialize ret = -1 (assume failure), call
the early exit label cleanup rather than error (if a successful exit
will run through that label - it avoids confusion when someone reads the
code later), and put a ret = 0; on the line just before cleanup:.
That way, you don't need to put in all the ret = -1; lines whenever
there is a failure.

ACK with that change (and after re-basing to account for the changes
requested in 1/3).

  int i;
  int interfacetype;
 @@ -1305,7 +1312,14 @@ qemuDomainMigrateOPDRelocate(struct qemud_driver 
 *driver ATTRIBUTE_UNUSED,
  cookie-network-net[i]-portdata = NULL;
  break;
  case VIR_NETDEV_VPORT_PROFILE_OPENVSWITCH:
 -cookie-network-net[i]-portdata = NULL;
 +if 
 (virNetDevOpenvswitchSetMigrateData(cookie-network-net[i]-portdata,
 +   netptr-ifname) != 0) 
 {
 +virReportSystemError(VIR_ERR_INTERNAL_ERROR,
 + _(Unable to run command to set 
 OVS port data for 
 + interface %s), 
 netptr-ifname);
 +ret = -1;
 +goto error;
 +}
  break;
  default:
  cookie-network-net[i]-portdata = NULL;
 @@ -1314,6 +1328,7 @@ qemuDomainMigrateOPDRelocate(struct qemud_driver 
 *driver ATTRIBUTE_UNUSED,
  }
  }
  
 +error:
  return ret;
  }
  

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


Re: [libvirt] virsh --connect not working

2012-09-28 Thread Jason Helfman
On Fri, Sep 28, 2012 at 8:53 AM, Daniel P. Berrange berra...@redhat.comwrote:

 On Fri, Sep 28, 2012 at 08:51:40AM -0700, hiren panchasara wrote:
  On Fri, Sep 28, 2012 at 8:40 AM, Daniel P. Berrange berra...@redhat.com
 wrote:
 
   On Fri, Sep 28, 2012 at 08:37:54AM -0700, hiren panchasara wrote:
On Fri, Sep 28, 2012 at 1:11 AM, Daniel P. Berrange 
 berra...@redhat.com
   wrote:
   
 On Thu, Sep 27, 2012 at 11:15:56PM -0700, hiren panchasara wrote:
  I've installed libvirt-0.9.13 from ports on my freebsd machine.
 
  I started libvirtd:
 
  $ ps awwux | grep libvirtd
  root   11470   0.0  0.4  103100  31948  -  I10:41PM
 0:00.35
  libvirtd -v -d
 
  $ sudo virsh --connect qemu:///system
  error: no connection driver available for No connection for URI
  qemu:///system
  error: failed to connect to the hypervisor
 
  I am sure I am missing something obvious.


 Try collecting some debugging output using

   LIBVIRT_DEBUG=1 sudo virsh --connect qemu:///system

 It should give a better idea why it failed.

   
Not getting any useful info:
   
$ ps awwux | grep libvirtd
root   3396   0.0  0.4  103100  32248  -  I 8:24AM  0:01.05
libvirtd -v -d
   
$ LIBVIRT_DEBUG=1 sudo virsh --connect qemu:///system
error: no connection driver available for No connection for URI
qemu:///system
error: failed to connect to the hypervisor
   
Tried 1-4 debug levels with the same response.
  
   Sounds like sudo is stripping the env variable. Try running it as root
   directly, without sudo.
  
 
  Aah, right, o/p is huge so putting it in pastebin:
  http://pastebin.com/YVrK0fRb

 Looks somewhat like your libvirt has been built without support
 for the QEMU driver

 Daniel


Libvirt is currently incomplete for FreeBSD, and there is an effort to get
it fully supported right now. Libvirtd daemon should not start, and I would
be surprised if it did. QEMU drivers aren't enabled either, but if you
would like to contribute anything please contact me and I can get you in
contact with the right people to contribute. There is also efforts to
implement the BSD Hypervisor into the libvirt codebase, as well.

Thanks!
-jgh
--
Jason Helfman  | FreeBSD Committer
j...@freebsd.org | http://people.freebsd.org/~jgh
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] virsh --connect not working

2012-09-28 Thread hiren panchasara
On Fri, Sep 28, 2012 at 9:14 AM, Jason Helfman j...@freebsd.org wrote:

 On Fri, Sep 28, 2012 at 8:53 AM, Daniel P. Berrange 
 berra...@redhat.comwrote:

 Looks somewhat like your libvirt has been built without support
 for the QEMU driver


 Daniel


Thanks for looking into it, Daniel.


 Libvirt is currently incomplete for FreeBSD, and there is an effort to get
 it fully supported right now. Libvirtd daemon should not start, and I would
 be surprised if it did. QEMU drivers aren't enabled either, but if you
 would like to contribute anything please contact me and I can get you in
 contact with the right people to contribute. There is also efforts to
 implement the BSD Hypervisor into the libvirt codebase, as well.


Hi Jason,
Sending you separate email to talk about this.

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

Re: [libvirt] [PATCH 0/3] Transport network port data during live migration

2012-09-28 Thread Daniel P. Berrange
On Fri, Sep 21, 2012 at 05:16:45PM -0400, Kyle Mestery wrote:
 This series of commits has the end goal of allowing per-port data stored
 in the Open vSwitch DB to be transported during live migration. This is
 done by first providing a generic infrastructure for transporting network
 data, adding some utility functions specific to Open vSwitch, and hooking
 the two together.
 
 The framework provided is generic in that other networking data could be
 transferred as well by simply adding in additional hooks as needed.
 
 An example of the XML being transported is shown here:
 
 network
   interface index='1' vporttype='openvswitch' netdata='blah blah blah'/
   interface index='7' vporttype='openvswitch' netdata='blah blah blah'/
 /network

Could you give an example of what goes in the netdata='blah blah blah'
part of the XML ?  I'm interesting in seeing the type of data, and its
indicitive size


Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|

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


Re: [libvirt] [PATCH 1/3] qemu_migration: Add hooks to transport network port data during migration

2012-09-28 Thread Eric Blake
On 09/28/2012 10:14 AM, Laine Stump wrote:
 On 09/21/2012 05:16 PM, Kyle Mestery wrote:
 Add the ability for the Qemu V3 migration protocol to
 include transporting network configuration. A generic
 framework is proposed with this patch to allow for the
 transfer of opaque data.
 
 Functionally this all looks good (and sounds like it lives up to that in
 practice :-). There are some code style issues, though...
 

 2) Internally, we usually store the enum value rather than the string,
 and translate to/from string form during XML parse/format. (For some
 reason, normal usage is to specify it in the struct as an int, with a
 comment giving the actual type:
 
 
 int vporttype; /* enum virNetDevVPortProfile */
 
 
 I'm not sure why that is, to be truthful - Eric or Dan?)

For public API - ABI stability.  The C standard does not give any
guarantees about the sizeof an enum member in a struct, and at least gcc
allows compiler flags that change struct layout based on whether enums
are packed to the smallest integer size that holds all the values of the
enum or is as wide as an int.  Since we don't control the compiler flags
used by a client app that includes our public headers, we're better off
avoiding the issue.  Using 'int' instead of the enum type frees us from
the ABI questions.

For private API - consistent coding style.  Besides, C is not as
typesafe as C++ with regards to enums, so it's not like we're losing
much functionality.

-- 
Eric Blake   ebl...@redhat.com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [libvirt-glib 1/2] gobject: Handle PMSUSPENDED state from libvirt

2012-09-28 Thread Christophe Fergeau
Emit a signal when switching to the PMSUSPENDED state, and add
an enum entry to describe this state. This avoids runtime warnings
with newer libvirt.
---
 libvirt-gobject/libvirt-gobject-connection.c |  7 +++
 libvirt-gobject/libvirt-gobject-domain.c | 11 +++
 libvirt-gobject/libvirt-gobject-domain.h | 19 +++
 3 files changed, 29 insertions(+), 8 deletions(-)

diff --git a/libvirt-gobject/libvirt-gobject-connection.c 
b/libvirt-gobject/libvirt-gobject-connection.c
index 428ae38..ad7aa07 100644
--- a/libvirt-gobject/libvirt-gobject-connection.c
+++ b/libvirt-gobject/libvirt-gobject-connection.c
@@ -389,6 +389,13 @@ static int domain_event_cb(virConnectPtr conn 
G_GNUC_UNUSED,
 case VIR_DOMAIN_EVENT_SHUTDOWN:
 break;
 
+case VIR_DOMAIN_EVENT_PMSUSPENDED:
+if (detail == VIR_DOMAIN_EVENT_PMSUSPENDED_MEMORY)
+g_signal_emit_by_name(gdom, pmsuspended::memory);
+else
+g_warn_if_reached();
+break;
+
 default:
 g_warn_if_reached();
 }
diff --git a/libvirt-gobject/libvirt-gobject-domain.c 
b/libvirt-gobject/libvirt-gobject-domain.c
index bcfad2a..f8ad493 100644
--- a/libvirt-gobject/libvirt-gobject-domain.c
+++ b/libvirt-gobject/libvirt-gobject-domain.c
@@ -55,6 +55,7 @@ enum {
 VIR_RESUMED,
 VIR_STOPPED,
 VIR_UPDATED,
+VIR_PMSUSPENDED,
 LAST_SIGNAL
 };
 
@@ -225,6 +226,16 @@ static void gvir_domain_class_init(GVirDomainClass *klass)
 G_TYPE_NONE,
 0);
 
+signals[VIR_PMSUSPENDED] = g_signal_new(pmsuspended,
+G_OBJECT_CLASS_TYPE(object_class),
+G_SIGNAL_RUN_LAST | 
G_SIGNAL_NO_RECURSE |
+G_SIGNAL_NO_HOOKS | G_SIGNAL_DETAILED,
+G_STRUCT_OFFSET(GVirDomainClass, 
pmsuspended),
+NULL, NULL,
+g_cclosure_marshal_VOID__VOID,
+G_TYPE_NONE,
+0);
+
 g_type_class_add_private(klass, sizeof(GVirDomainPrivate));
 }
 
diff --git a/libvirt-gobject/libvirt-gobject-domain.h 
b/libvirt-gobject/libvirt-gobject-domain.h
index 7810d1b..fc2db7c 100644
--- a/libvirt-gobject/libvirt-gobject-domain.h
+++ b/libvirt-gobject/libvirt-gobject-domain.h
@@ -66,19 +66,22 @@ struct _GVirDomainClass
 void (*resumed)(GVirDomain *dom);
 void (*updated)(GVirDomain *dom);
 void (*suspended)(GVirDomain *dom);
+void (*pmsuspended)(GVirDomain *dom);
 
-gpointer padding[20];
+gpointer padding[19];
 };
 
 
 typedef enum {
-GVIR_DOMAIN_STATE_NONE= 0, /* no state */
-GVIR_DOMAIN_STATE_RUNNING = 1, /* the domain is running */
-GVIR_DOMAIN_STATE_BLOCKED = 2, /* the domain is blocked on resource */
-GVIR_DOMAIN_STATE_PAUSED  = 3, /* the domain is paused by user */
-GVIR_DOMAIN_STATE_SHUTDOWN= 4, /* the domain is being shut down */
-GVIR_DOMAIN_STATE_SHUTOFF = 5, /* the domain is shut off */
-GVIR_DOMAIN_STATE_CRASHED = 6  /* the domain is crashed */
+GVIR_DOMAIN_STATE_NONE= 0, /* no state */
+GVIR_DOMAIN_STATE_RUNNING = 1, /* the domain is running */
+GVIR_DOMAIN_STATE_BLOCKED = 2, /* the domain is blocked on resource */
+GVIR_DOMAIN_STATE_PAUSED  = 3, /* the domain is paused by user */
+GVIR_DOMAIN_STATE_SHUTDOWN= 4, /* the domain is being shut down */
+GVIR_DOMAIN_STATE_SHUTOFF = 5, /* the domain is shut off */
+GVIR_DOMAIN_STATE_CRASHED = 6, /* the domain is crashed */
+GVIR_DOMAIN_STATE_PMSUSPENDED = 7  /* the domain is suspended by guest
+  power management */
 } GVirDomainState;
 
 
-- 
1.7.11.4

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


[libvirt] [libvirt-glib 2/2] gobject: Emit a signal on WAKEUP events

2012-09-28 Thread Christophe Fergeau
This avoids a runtime warning about this kind of event not
being handled.
---
 libvirt-gobject/libvirt-gobject-connection.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/libvirt-gobject/libvirt-gobject-connection.c 
b/libvirt-gobject/libvirt-gobject-connection.c
index ad7aa07..9628989 100644
--- a/libvirt-gobject/libvirt-gobject-connection.c
+++ b/libvirt-gobject/libvirt-gobject-connection.c
@@ -327,6 +327,8 @@ static int domain_event_cb(virConnectPtr conn G_GNUC_UNUSED,
 g_signal_emit_by_name(gdom, started::restored);
 else if (detail == VIR_DOMAIN_EVENT_STARTED_FROM_SNAPSHOT)
 g_signal_emit_by_name(gdom, started::from-snapshot);
+else if (detail == VIR_DOMAIN_EVENT_STARTED_WAKEUP)
+g_signal_emit_by_name(gdom, started::wakeup);
 else
 g_warn_if_reached();
 break;
-- 
1.7.11.4

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


[libvirt] Patch to default selinuxfs mount point to /sys/fs/selinux

2012-09-28 Thread Daniel J Walsh
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

Currently if you build on a machine that does not support SELinux we end up
with the default mount point being /selinux, since this is moved to
/sys/fs/selinux, we should start defaulting there.

I believe this is causing a bug in libvirt-lxc when /selinux does not exists,
even though /sys/fs/selinux exists.
-BEGIN PGP SIGNATURE-
Version: GnuPG v1.4.12 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://www.enigmail.net/

iEYEARECAAYFAlBl6F8ACgkQrlYvE4MpobPz0ACgxPdENx/KDtQY7YGT7BDXoLP3
AVIAoJwUvTib72U0VJ3dnVoGU0PYjMXZ
=XU8L
-END PGP SIGNATURE-
diff --git a/.gnulib b/.gnulib
index 440a1db..dbd9144 16
--- a/.gnulib
+++ b/.gnulib
@@ -1 +1 @@
-Subproject commit 440a1dbe523e37f206252cb034c3a62f26867e42
+Subproject commit dbd914496c99c52220e5f5ba4121d6cb55fb3beb
diff --git a/configure.ac b/configure.ac
index ae26de7..5cc5cbe 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1455,11 +1455,10 @@ fi
 if test $with_selinux = yes; then
   AC_MSG_CHECKING([SELinux mount point])
   if test $with_selinux_mount = check || test -z $with_selinux_mount; then
-if test -d /sys/fs/selinux ; then
-  SELINUX_MOUNT=/sys/fs/selinux
-else
-  SELINUX_MOUNT=/selinux
-fi
+ SELINUX_MOUNT=/sys/fs/selinux
+ if ! test -d ${SELINUX_MOUNT}  test -d /selinux ; then
+ SELINUX_MOUNT=/selinux
+ fi
   else
 SELINUX_MOUNT=$with_selinux_mount
   fi
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] Patch to default selinuxfs mount point to /sys/fs/selinux

2012-09-28 Thread Eric Blake
On 09/28/2012 12:11 PM, Daniel J Walsh wrote:
 OpenPGP: *Attachments to this message have not been signed or encrypted*
 
 Currently if you build on a machine that does not support SELinux we end up
 with the default mount point being /selinux, since this is moved to
 /sys/fs/selinux, we should start defaulting there.

The spec files are already doing this correctly, so this patch will only
help someone doing a self-build rather than relying on a spec file, and
only then if selinux is not mounted on the machine where the build is
done, in relation to where the built binary is run.

 +++ b/.gnulib
 @@ -1 +1 @@
 -Subproject commit 440a1dbe523e37f206252cb034c3a62f26867e42
 +Subproject commit dbd914496c99c52220e5f5ba4121d6cb55fb3beb

This should not be part of the patch.

 diff --git a/configure.ac b/configure.ac
 index ae26de7..5cc5cbe 100644
 --- a/configure.ac
 +++ b/configure.ac
 @@ -1455,11 +1455,10 @@ fi
  if test $with_selinux = yes; then
AC_MSG_CHECKING([SELinux mount point])
if test $with_selinux_mount = check || test -z $with_selinux_mount; 
 then
 -if test -d /sys/fs/selinux ; then
 -  SELINUX_MOUNT=/sys/fs/selinux
 -else
 -  SELINUX_MOUNT=/selinux
 -fi
 + SELINUX_MOUNT=/sys/fs/selinux
 + if ! test -d ${SELINUX_MOUNT}  test -d /selinux ; then
 + SELINUX_MOUNT=/selinux
 + fi
else

ACK to this hunk, and I will push shortly.

-- 
Eric Blake   ebl...@redhat.com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 0/3] Transport network port data during live migration

2012-09-28 Thread Kyle Mestery (kmestery)
On Sep 28, 2012, at 11:42 AM, Daniel P. Berrange wrote:
 On Fri, Sep 21, 2012 at 05:16:45PM -0400, Kyle Mestery wrote:
 This series of commits has the end goal of allowing per-port data stored
 in the Open vSwitch DB to be transported during live migration. This is
 done by first providing a generic infrastructure for transporting network
 data, adding some utility functions specific to Open vSwitch, and hooking
 the two together.
 
 The framework provided is generic in that other networking data could be
 transferred as well by simply adding in additional hooks as needed.
 
 An example of the XML being transported is shown here:
 
 network
  interface index='1' vporttype='openvswitch' netdata='blah blah blah'/
  interface index='7' vporttype='openvswitch' netdata='blah blah blah'/
 /network
 
 Could you give an example of what goes in the netdata='blah blah blah'
 part of the XML ?  I'm interesting in seeing the type of data, and its
 indicitive size
 
 
 Daniel


Hi Daniel:

From a hypothetical angle:

As an example, an OpenFlow controller may have certain information about the
port, specific to this controller, which it may want to store with the port 
itself on the
host. This especially true if an agent exists on the host which needs to read 
this data,
update it, and use it to perform some tasks. It's convenient to have this data 
stored
as close to the port itself, which in this case is the OVS DB, and having it 
transferred
as part of the migration protocol is also very handy.

Thanks,
Kyle

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


Re: [libvirt] [PATCH 1/3] qemu_migration: Add hooks to transport network port data during migration

2012-09-28 Thread Kyle Mestery (kmestery)
On Sep 28, 2012, at 11:14 AM, Laine Stump wrote:
 On 09/21/2012 05:16 PM, Kyle Mestery wrote:
 Add the ability for the Qemu V3 migration protocol to
 include transporting network configuration. A generic
 framework is proposed with this patch to allow for the
 transfer of opaque data.
 
 Functionally this all looks good (and sounds like it lives up to that in
 practice :-). There are some code style issues, though…
 
Thanks for all the comments Laine. I'll address them all and
send out new versions of the patches ASAP.

Kyle

 
 Signed-off-by: Kyle Mestery kmest...@cisco.com
 ---
 src/qemu/qemu_migration.c | 278 
 +-
 1 file changed, 276 insertions(+), 2 deletions(-)
 
 diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
 index 8e85875..06981db 100644
 --- a/src/qemu/qemu_migration.c
 +++ b/src/qemu/qemu_migration.c
 @@ -70,6 +70,7 @@ enum qemuMigrationCookieFlags {
 QEMU_MIGRATION_COOKIE_FLAG_GRAPHICS,
 QEMU_MIGRATION_COOKIE_FLAG_LOCKSTATE,
 QEMU_MIGRATION_COOKIE_FLAG_PERSISTENT,
 +QEMU_MIGRATION_COOKIE_FLAG_NETWORK,
 
 QEMU_MIGRATION_COOKIE_FLAG_LAST
 };
 @@ -77,12 +78,13 @@ enum qemuMigrationCookieFlags {
 VIR_ENUM_DECL(qemuMigrationCookieFlag);
 VIR_ENUM_IMPL(qemuMigrationCookieFlag,
   QEMU_MIGRATION_COOKIE_FLAG_LAST,
 -  graphics, lockstate, persistent);
 +  graphics, lockstate, persistent, network);
 
 enum qemuMigrationCookieFeatures {
 QEMU_MIGRATION_COOKIE_GRAPHICS  = (1  
 QEMU_MIGRATION_COOKIE_FLAG_GRAPHICS),
 QEMU_MIGRATION_COOKIE_LOCKSTATE = (1  
 QEMU_MIGRATION_COOKIE_FLAG_LOCKSTATE),
 QEMU_MIGRATION_COOKIE_PERSISTENT = (1  
 QEMU_MIGRATION_COOKIE_FLAG_PERSISTENT),
 +QEMU_MIGRATION_COOKIE_NETWORK = (1  
 QEMU_MIGRATION_COOKIE_FLAG_NETWORK),
 };
 
 typedef struct _qemuMigrationCookieGraphics qemuMigrationCookieGraphics;
 @@ -95,6 +97,35 @@ struct _qemuMigrationCookieGraphics {
 char *tlsSubject;
 };
 
 +VIR_ENUM_DECL(virInterface);
 +VIR_ENUM_IMPL(virInterface, VIR_NETDEV_VPORT_PROFILE_LAST,
 +  none,
 +  802.1QBg,
 +  802.1QBh,
 +  openvswitch);
 
 (I'd never before considered using VIR_ENUM_*() and giving an enum type
 identifier that didn't match the enum being translated)
 
 Since you're using the enum virNetDevVPortProfile for these values
 anyway, you should just use the already-existing functions:
 
 virNetDevVPortTypeFromString
 virNetDevVPortTypeToString
 
 (these are declared/defined in virnetdevvportprofile.h, and included in
 libvirt_private.syms).
 
 
 +
 +typedef struct _qemuMigrationCookieNetdata qemuMigrationCookieNetdata;
 +typedef qemuMigrationCookieNetdata *qemuMigrationCookieNetdataPtr;
 +struct _qemuMigrationCookieNetdata {
 +/* What type is each interace ? virInterface above ... */
 +char *interfacetype;
 
 1) Since the xml attribute is called vporttype, you should give the
 variable in the struct the same name, to avoid confusion.
 
 
 2) Internally, we usually store the enum value rather than the string,
 and translate to/from string form during XML parse/format. (For some
 reason, normal usage is to specify it in the struct as an int, with a
 comment giving the actual type:
 
 
int vporttype; /* enum virNetDevVPortProfile */
 
 
 I'm not sure why that is, to be truthful - Eric or Dan?)
 
 
 +
 +/*
 + * Array of pointers to saved data. Each VIF will have it's own
 + * data to transfer.
 + */
 +char *portdata;
 +};
 +
 +typedef struct _qemuMigrationCookieNetwork qemuMigrationCookieNetwork;
 +typedef qemuMigrationCookieNetwork *qemuMigrationCookieNetworkPtr;
 +struct _qemuMigrationCookieNetwork {
 +/* How many virtual NICs are we saving data for? */
 +int nnets;
 +
 +qemuMigrationCookieNetdataPtr *net;
 +};
 +
 typedef struct _qemuMigrationCookie qemuMigrationCookie;
 typedef qemuMigrationCookie *qemuMigrationCookiePtr;
 struct _qemuMigrationCookie {
 @@ -120,6 +151,9 @@ struct _qemuMigrationCookie {
 
 /* If (flags  QEMU_MIGRATION_COOKIE_PERSISTENT) */
 virDomainDefPtr persistent;
 +
 +/* If (flags  QEMU_MIGRATION_COOKIE_NETWORK) */
 +qemuMigrationCookieNetworkPtr network;
 };
 
 static void qemuMigrationCookieGraphicsFree(qemuMigrationCookieGraphicsPtr 
 grap)
 @@ -132,6 +166,28 @@ static void 
 qemuMigrationCookieGraphicsFree(qemuMigrationCookieGraphicsPtr grap)
 }
 
 
 +static void qemuMigrationCookieNetworkFree(qemuMigrationCookieNetworkPtr
 +   network)
 +{
 +int i;
 +
 +if (!network)
 +return;
 +
 +for (i = 0; i  network-nnets; i++) {
 +if (network-net[i]) {
 +if (network-net[i]-interfacetype)
 +VIR_FREE(network-net[i]-interfacetype);
 
 VIR_FREE() automatically checks for NULL, and is a NOP if the pointer is
 NULL (it also sets the pointer to NULL when it's finished, so calling it
 twice doesn't create a problem. 

Re: [libvirt] TSC scaling interface to management

2012-09-28 Thread Marcelo Tosatti
On Tue, Sep 25, 2012 at 11:08:58AM +0100, Daniel P. Berrange wrote:
 On Wed, Sep 12, 2012 at 12:39:39PM -0300, Marcelo Tosatti wrote:
  
  
  HW TSC scaling is a feature of AMD processors that allows a
  multiplier to be specified to the TSC frequency exposed to the guest.
  
  KVM also contains provision to trap TSC (KVM: Infrastructure for
  software and hardware based TSC rate scaling cc578287e3224d0da)
  or advance TSC frequency.
  
  This is useful when migrating to a host with different frequency and
  the guest is possibly using direct RDTSC instructions for purposes
  other than measuring cycles (that is, it previously calculated
  cycles-per-second, and uses that information which is stale after
  migration).
  
  qemu-x86: Set tsc_khz in kvm when supported (e7429073ed1a76518)
  added support for tsc_khz= option in QEMU.
  
  I am proposing the following changes so that management applications
  can work with this:
  
  1) New option for tsc_khz, which is tsc_khz=host (QEMU command line
  option). Host means that QEMU is responsible for retrieving the 
  TSC frequency of the host processor and use that.
  Management application does not have to deal with the burden.
 
 FYI, libvirt already has support for expressing a number of different
 TSC related config options, for support of Xen and VMWare's capabilities
 in this area. What we currently allow for is
 
timer name='tsc' frequency='NNN'  mode='auto|native|emulate|smpsafe'/
 
 In this context the frequency attribute provides the HZ value to
 provide to the guest.
 
   - auto == Emulate if TSC is unstable, else allow native TSC access
   - native == Always allow native TSC access
   - emulate = Always emulate TSC
   - smpsafe == Always emulate TSC, and interlock SMP

These options can be mapped into KVM if necessary (they can map to
tsc_khz=XXX or to the module options (unfortunately not per-guest ATM)).

  Therefore it appears that this tsc_khz=auto option can be specified
  only if the user specifies so (it can be a per-guest flag hidden
  in the management configuration/manual).
  
  Sending this email to gather suggestions (or objections)
  to this interface.
 
 
 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 :|

Karen had the suggestion to remove the burden of choice from the user,
which we can achieve by knowing whether or not the guest is using
a paravirtual clock.

The problem is that opens a can of races: Did migration happen before or
after guest boot process enabled the paravirtual clock etc.

I suppose leaving the option to the user is fine: if you run an obscure
operating system, which does not support paravirtual clock, then it
must be dealt with specialy (its in the manual, no big deal).

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