[libvirt] [PATCH] Move the expected output data from virshdata/*.txt into virshtest.c.

[Jim Meyering]

This is mainly just reorganization and factorization, with an eye
towards making it easier to add more tests in virshtest.c later.

From 21352a07b073d52bcd3c442c76d7fb985aeef845 Mon Sep 17 00:00:00 2001
From: Jim Meyering [EMAIL PROTECTED]
Date: Thu, 20 Nov 2008 20:35:57 +0100
Subject: [PATCH] Move the expected output data from virshdata/*.txt into 
virshtest.c.

* tests/virshtest.c: Embed literal, expected output here, rather than
using virshdata/*.txt file names.
Factor out some common constructs.
(testCompareOutputLit): New function.
(testCompareOutput): #ifdef-out, not that it's unused.
* tests/Makefile.am (SUBDIRS): Remove virshdata/ and all files in it.
* docs/testnode.xml: Fix typo in a comment.
---
 docs/testnode.xml|2 +-
 tests/Makefile.am|2 +-
 tests/virshdata/.cvsignore   |2 -
 tests/virshdata/.gitignore   |2 -
 tests/virshdata/Makefile.am  |2 -
 tests/virshdata/domid-fc4.txt|2 -
 tests/virshdata/dominfo-fc4.txt  |   10 --
 tests/virshdata/domname-fc4.txt  |2 -
 tests/virshdata/domstate-fc4.txt |2 -
 tests/virshdata/domuuid-fc4.txt  |2 -
 tests/virshdata/list-custom.txt  |5 -
 tests/virshdata/list-default.txt |4 -
 tests/virshdata/nodeinfo-custom.txt  |9 --
 tests/virshdata/nodeinfo-default.txt |9 --
 tests/virshtest.c|  264 ++
 15 files changed, 111 insertions(+), 208 deletions(-)
 delete mode 100644 tests/virshdata/.cvsignore
 delete mode 100644 tests/virshdata/.gitignore
 delete mode 100644 tests/virshdata/Makefile.am
 delete mode 100644 tests/virshdata/domid-fc4.txt
 delete mode 100644 tests/virshdata/dominfo-fc4.txt
 delete mode 100644 tests/virshdata/domname-fc4.txt
 delete mode 100644 tests/virshdata/domstate-fc4.txt
 delete mode 100644 tests/virshdata/domuuid-fc4.txt
 delete mode 100644 tests/virshdata/list-custom.txt
 delete mode 100644 tests/virshdata/list-default.txt
 delete mode 100644 tests/virshdata/nodeinfo-custom.txt
 delete mode 100644 tests/virshdata/nodeinfo-default.txt

diff --git a/docs/testnode.xml b/docs/testnode.xml
index 7df10b0..be7121d 100644
--- a/docs/testnode.xml
+++ b/docs/testnode.xml
@@ -1,6 +1,6 @@
 node
 !-- This file gives an example config for the mock 'test' backend
- driver to libvirt. This is intended to allow relible unit testing
+ driver to libvirt. This is intended to allow reliable unit testing
  of applications using libvirt. To use this with virsh, run something
  like:

diff --git a/tests/Makefile.am b/tests/Makefile.am
index 3b4f0dc..bdbf927 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -2,7 +2,7 @@

 SHELL = $(PREFERABLY_POSIX_SHELL)

-SUBDIRS = virshdata confdata sexpr2xmldata \
+SUBDIRS = confdata sexpr2xmldata \
   xml2sexprdata xmconfigdata xencapsdata

 INCLUDES = \
diff --git a/tests/virshdata/.cvsignore b/tests/virshdata/.cvsignore
deleted file mode 100644
index 282522d..000
--- a/tests/virshdata/.cvsignore
+++ /dev/null
@@ -1,2 +0,0 @@
-Makefile
-Makefile.in
diff --git a/tests/virshdata/.gitignore b/tests/virshdata/.gitignore
deleted file mode 100644
index 282522d..000
--- a/tests/virshdata/.gitignore
+++ /dev/null
@@ -1,2 +0,0 @@
-Makefile
-Makefile.in
diff --git a/tests/virshdata/Makefile.am b/tests/virshdata/Makefile.am
deleted file mode 100644
index c46fc93..000
--- a/tests/virshdata/Makefile.am
+++ /dev/null
@@ -1,2 +0,0 @@
-
-EXTRA_DIST = $(wildcard *.txt)
diff --git a/tests/virshdata/domid-fc4.txt b/tests/virshdata/domid-fc4.txt
deleted file mode 100644
index 9a7456b..000
--- a/tests/virshdata/domid-fc4.txt
+++ /dev/null
@@ -1,2 +0,0 @@
-2
-
diff --git a/tests/virshdata/dominfo-fc4.txt b/tests/virshdata/dominfo-fc4.txt
deleted file mode 100644
index 0cce9be..000
--- a/tests/virshdata/dominfo-fc4.txt
+++ /dev/null
@@ -1,10 +0,0 @@
-Id: 2
-Name:   fc4
-UUID:   ef861801-45b9-11cb-88e3-afbfe5370493
-OS Type:linux
-State:  running
-CPU(s): 1
-Max memory: 261072 kB
-Used memory:131072 kB
-Autostart:  disable
-
diff --git a/tests/virshdata/domname-fc4.txt b/tests/virshdata/domname-fc4.txt
deleted file mode 100644
index fd55058..000
--- a/tests/virshdata/domname-fc4.txt
+++ /dev/null
@@ -1,2 +0,0 @@
-fc4
-
diff --git a/tests/virshdata/domstate-fc4.txt b/tests/virshdata/domstate-fc4.txt
deleted file mode 100644
index 9c724a4..000
--- a/tests/virshdata/domstate-fc4.txt
+++ /dev/null
@@ -1,2 +0,0 @@
-running
-
diff --git a/tests/virshdata/domuuid-fc4.txt b/tests/virshdata/domuuid-fc4.txt
deleted file mode 100644
index face70e..000
--- a/tests/virshdata/domuuid-fc4.txt
+++ /dev/null
@@ -1,2 +0,0 @@
-ef861801-45b9-11cb-88e3-afbfe5370493
-
diff --git a/tests/virshdata/list-custom.txt b/tests/virshdata/list-custom.txt
deleted file mode 100644
index 2b48b26..000
--- a/tests/virshdata/list-custom.txt
+++ /dev/null
@@ -1,5 +0,0 

Re: [libvirt] [PATCH] Move the expected output data from virshdata/*.txt into virshtest.c.

[Jim Meyering]

Jim Meyering [EMAIL PROTECTED] wrote:
 This is mainly just reorganization and factorization, with an eye
 towards making it easier to add more tests in virshtest.c later.

From 21352a07b073d52bcd3c442c76d7fb985aeef845 Mon Sep 17 00:00:00 2001
 From: Jim Meyering [EMAIL PROTECTED]
 Date: Thu, 20 Nov 2008 20:35:57 +0100
 Subject: [PATCH] Move the expected output data from virshdata/*.txt into 
 virshtest.c.

 * tests/virshtest.c: Embed literal, expected output here, rather than
 using virshdata/*.txt file names.
 Factor out some common constructs.
 (testCompareOutputLit): New function.
 (testCompareOutput): #ifdef-out, not that it's unused.
 * tests/Makefile.am (SUBDIRS): Remove virshdata/ and all files in it.
 * docs/testnode.xml: Fix typo in a comment.

Since I'm removing a directory containing a Makefile.am,
I've amended the patch to do this, too:

* configure.in (AC_OUTPUT): Remove tests/virshdata/Makefile.

diff --git a/configure.in b/configure.in
index 21931a1..fab5e0c 100644
--- a/configure.in
+++ b/configure.in
@@ -1093,7 +1093,7 @@ AC_OUTPUT(Makefile src/Makefile include/Makefile 
docs/Makefile \
   tests/sexpr2xmldata/Makefile \
   tests/xmconfigdata/Makefile \
   tests/xencapsdata/Makefile \
-  tests/virshdata/Makefile tests/confdata/Makefile \
+  tests/confdata/Makefile \
   examples/domain-events/events-c/Makefile)

 AC_MSG_NOTICE([])

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

Re: [libvirt] PATCH: 1/7: Public API

[Daniel Veillard]

On Thu, Nov 20, 2008 at 05:51:39PM +, Daniel P. Berrange wrote:
 This patch contains the public API for node devices. Changes since last
 post are
 
  - Remove the ByCap methods - the main list method now takes an optionally
NULL, cap arguent. This simplified virsh usage later too which is nice
 
  - Change VIR_FROM_DEVMONITOR to VIR_FROM_NODEDEV to match rest of public
naming scheme
 
  - Remove unused NodeDeviceCreate/Destroy methods. We can re-add them
when we need them

  Okay, that follows the earlier feedback from Mark and Dave,

+1

Daniel

-- 
Daniel Veillard  | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
[EMAIL PROTECTED]  | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library  http://libvirt.org/

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

Re: [libvirt] [PATCH] Allow rpm to build without uml

[Daniel P. Berrange]

On Thu, Nov 20, 2008 at 05:25:47PM -0500, Ben Guthro wrote:
 Allow to build minimal rpm without uml

Thanks, I've committed this.

Daniel
-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

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

Re: [libvirt] --with-xen=foo

[Daniel P. Berrange]

On Thu, Nov 20, 2008 at 06:30:38PM +, Daniel P. Berrange wrote:
 On Thu, Nov 20, 2008 at 01:27:40PM -0500, Ben Guthro wrote:
  I tried this...didn't seem to help:
 
 Sorry, not paying close enough attention. In fact this file was
 built in an earlier module. Find the line which says
 
 libvirt_driver_la_SOURCES = \
 $(DRIVER_SOURCES)   \
 $(DOMAIN_CONF_SOURCES)  \
 $(NETWORK_CONF_SOURCES) \
 $(STORAGE_CONF_SOURCES) \
 $(NODE_DEVICE_CONF_SOURCES)
 
 And add in a line following that
 
 libvirt_driver_la_CFLAGS = $(XEN_CFLAGS)

On the assumption that this fixed it for you, I've committed this change

Regards,
Daniel
-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

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

[libvirt] [PATCH] tests: new test: virsh-synopsis

[Jim Meyering]

I wanted help SYNOPSIS output that was regular enough to parse.
Enforcing/correcting the following is the first step:

From fa58b801ea80f52a10d48ab75a359ffe06c4be40 Mon Sep 17 00:00:00 2001
From: Jim Meyering [EMAIL PROTECTED]
Date: Fri, 21 Nov 2008 10:58:36 +0100
Subject: [PATCH] tests: new test: virsh-synopsis

* tests/virsh-synopsis: new file
* tests/Makefile.am (test_scripts): Add virsh-synopsis.
* src/virsh.c: Correct help SYNOPSIS for each of seven commands.

When I first ran this script, make check failed like this:
  ...
  invalid help SYNOPSIS for net-create:
  create a network from an XML file
  invalid help SYNOPSIS for net-define:
  define a network from an XML file
  invalid help SYNOPSIS for net-start:
  start network
  invalid help SYNOPSIS for pool-create:
  create a pool from an XML file
  invalid help SYNOPSIS for pool-define:
  define a pool from an XML file
  invalid help SYNOPSIS for pool-start:
  start pool
  invalid help SYNOPSIS for vol-create:
  create file
  FAIL: virsh-synopsis
---
 src/virsh.c  |   17 +
 tests/Makefile.am|1 +
 tests/virsh-synopsis |   43 +++
 3 files changed, 53 insertions(+), 8 deletions(-)
 create mode 100755 tests/virsh-synopsis

diff --git a/src/virsh.c b/src/virsh.c
index 4195781..78c536e 100644
--- a/src/virsh.c
+++ b/src/virsh.c
@@ -2365,7 +2365,7 @@ cmdNetworkAutostart(vshControl *ctl, const vshCmd *cmd)
  * net-create command
  */
 static const vshCmdInfo info_network_create[] = {
-{syntax, create a network from an XML file},
+{syntax, net-create file},
 {help, gettext_noop(create a network from an XML file)},
 {desc, gettext_noop(Create a network.)},
 {NULL, NULL}
@@ -2413,7 +2413,7 @@ cmdNetworkCreate(vshControl *ctl, const vshCmd *cmd)
  * net-define command
  */
 static const vshCmdInfo info_network_define[] = {
-{syntax, define a network from an XML file},
+{syntax, net-define file},
 {help, gettext_noop(define (but don't start) a network from an XML 
file)},
 {desc, gettext_noop(Define a network.)},
 {NULL, NULL}
@@ -2697,7 +2697,7 @@ cmdNetworkName(vshControl *ctl, const vshCmd *cmd)
  * net-start command
  */
 static const vshCmdInfo info_network_start[] = {
-{syntax, start network},
+{syntax, net-start network},
 {help, gettext_noop(start a (previously defined) inactive network)},
 {desc, gettext_noop(Start a network.)},
 {NULL, NULL}
@@ -2862,14 +2862,15 @@ cmdPoolAutostart(vshControl *ctl, const vshCmd *cmd)
  * pool-create command
  */
 static const vshCmdInfo info_pool_create[] = {
-{syntax, create a pool from an XML file},
+{syntax, pool-create file},
 {help, gettext_noop(create a pool from an XML file)},
 {desc, gettext_noop(Create a pool.)},
 {NULL, NULL}
 };

 static const vshCmdOptDef opts_pool_create[] = {
-{file, VSH_OT_DATA, VSH_OFLAG_REQ, gettext_noop(file containing an XML 
pool description)},
+{file, VSH_OT_DATA, VSH_OFLAG_REQ,
+ gettext_noop(file containing an XML pool description)},
 {NULL, 0, 0, NULL}
 };

@@ -3000,7 +3001,7 @@ cmdPoolCreateAs(vshControl *ctl, const vshCmd *cmd)
  * pool-define command
  */
 static const vshCmdInfo info_pool_define[] = {
-{syntax, define a pool from an XML file},
+{syntax, pool-define file},
 {help, gettext_noop(define (but don't start) a pool from an XML file)},
 {desc, gettext_noop(Define a pool.)},
 {NULL, NULL}
@@ -3727,7 +3728,7 @@ cmdPoolName(vshControl *ctl, const vshCmd *cmd)
  * pool-start command
  */
 static const vshCmdInfo info_pool_start[] = {
-{syntax, start pool},
+{syntax, pool-start pool},
 {help, gettext_noop(start a (previously defined) inactive pool)},
 {desc, gettext_noop(Start a pool.)},
 {NULL, NULL}
@@ -3965,7 +3966,7 @@ cmdPoolUuid(vshControl *ctl, const vshCmd *cmd)
  * vol-create command
  */
 static const vshCmdInfo info_vol_create[] = {
-{syntax, create file},
+{syntax, vol-create file},
 {help, gettext_noop(create a vol from an XML file)},
 {desc, gettext_noop(Create a vol.)},
 {NULL, NULL}
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 0b4eebd..bec3791 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -58,6 +58,7 @@ test_scripts += \
undefine \
vcpupin
virsh-all
+   virsh-synopsis
 endif

 EXTRA_DIST += $(test_scripts)
diff --git a/tests/virsh-synopsis b/tests/virsh-synopsis
new file mode 100755
index 000..6ce58a2
--- /dev/null
+++ b/tests/virsh-synopsis
@@ -0,0 +1,43 @@
+#!/bin/sh
+# ensure that each command's help SYNOPSIS line starts with the command name
+
+# Copyright (C) 2008 Free Software Foundation, Inc.
+
+# This program is free software: you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.

Re: [libvirt] [PATCH] fix libvirtd crash in qemu driver

[Daniel P. Berrange]

On Thu, Nov 20, 2008 at 04:48:46PM -0500, David Lively wrote:
 I noticed that the following sequence of events would crash libvirtd
 when using the qemu driver:
   (1) establish a connection that successfully registers for domain  
   events (either of the event-test programs will do, though the
   python one is currently broken -- another patch on the way for 
   that)
   (2) close this connection
   (3) open another connection
   (4) do something (like start a domain) that issues a domain event
 
 The problem is that qemudClose() isn't removing registered domain event
 callbacks when the connection closes.  This patch does that, and fixes
 the crash.

Ah ha, that explains the crash I was seeing. I've committed this fix.

Regards,
Daniel
-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

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

Re: [libvirt] PATCH: 6/7: Python binding

[Daniel Veillard]

On Thu, Nov 20, 2008 at 05:58:16PM +, Daniel P. Berrange wrote:
 This is the python API, again just changed to cope with removal of the
 ByCaps API calls

  looks fine,

[...]
 diff -r 105e73557ef8 python/libvir.c
 --- a/python/libvir.c Thu Nov 20 16:27:06 2008 +
 +++ b/python/libvir.c Thu Nov 20 16:43:08 2008 +
[...]
 +static PyObject *
 +libvirt_virNodeDeviceListCaps(PyObject *self ATTRIBUTE_UNUSED,
 +  PyObject *args) {
 +PyObject *py_retval;
 +char **names = NULL;
 +int c_retval, i;
 +virNodeDevicePtr dev;
 +PyObject *pyobj_dev;
 +
 +if (!PyArg_ParseTuple(args, (char *)O:virNodeDeviceListCaps, 
 pyobj_dev))
 +return(NULL);
 +dev = (virNodeDevicePtr) PyvirNodeDevice_Get(pyobj_dev);
 +
 +c_retval = virNodeDeviceNumOfCaps(dev);
 +if (c_retval  0)
 +return VIR_PY_NONE;
 +
 +if (c_retval) {
 +names = malloc(sizeof(*names) * c_retval);
 +if (!names)
 +return VIR_PY_NONE;
 +c_retval = virNodeDeviceListCaps(dev, names, c_retval);
 +if (c_retval  0) {
 +free(names);
 +return VIR_PY_NONE;
 +}
 +}
 +py_retval = PyList_New(c_retval);
 +
 +if (names) {
 +for (i = 0;i  c_retval;i++) {
 +PyList_SetItem(py_retval, i, libvirt_constcharPtrWrap(names[i]));
 +free(names[i]);
 +}
 +free(names);
 +}
 +
 +return(py_retval);
 +}

  But I don't think we need that one anymore, right ?

  +1

Daniel

-- 
Daniel Veillard  | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
[EMAIL PROTECTED]  | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library  http://libvirt.org/

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

Re: [libvirt] [PATCH] tests: new test: virsh-synopsis

[Daniel P. Berrange]

On Fri, Nov 21, 2008 at 11:13:03AM +0100, Jim Meyering wrote:
 I wanted help SYNOPSIS output that was regular enough to parse.


  static const vshCmdInfo info_network_create[] = {
 -{syntax, create a network from an XML file},
 +{syntax, net-create file},

I'm kind of wondering why we manually write the syntax string at all.
We know the name of the command, and we know the names of all the 
args, so we can auto-generate this entire string.

Daniel
-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

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

[libvirt] [PATCH] tests: virsh-all: new script

[Jim Meyering]

Actually, this was the first step.
This tiny script merely invokes all commands blindly,
solely to get test coverage, even if they (as most do) fail.
Additional tests will be smarter about providing sensible arguments.

From 058681eab07936f4ecbb1ff514528eb921d27074 Mon Sep 17 00:00:00 2001
From: Jim Meyering [EMAIL PROTECTED]
Date: Fri, 21 Nov 2008 10:18:43 +0100
Subject: [PATCH] tests: virsh-all: new script

* tests/virsh-all: New script.
* tests/Makefile.am (test_scripts): Add virsh-all.
---
 tests/Makefile.am |1 +
 tests/virsh-all   |   39 +++
 2 files changed, 40 insertions(+), 0 deletions(-)
 create mode 100755 tests/virsh-all

diff --git a/tests/Makefile.am b/tests/Makefile.am
index bdbf927..0b4eebd 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -57,6 +57,7 @@ test_scripts += \
read-non-seekable \
undefine \
vcpupin
+   virsh-all
 endif

 EXTRA_DIST += $(test_scripts)
diff --git a/tests/virsh-all b/tests/virsh-all
new file mode 100755
index 000..af7e09d
--- /dev/null
+++ b/tests/virsh-all
@@ -0,0 +1,39 @@
+#!/bin/sh
+# blindly run each and every command listed by virsh help
+
+# Copyright (C) 2008 Free Software Foundation, Inc.
+
+# This program is free software: you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see http://www.gnu.org/licenses/.
+
+if test $VERBOSE = yes; then
+  set -x
+  virsh --version
+fi
+
+. $srcdir/test-lib.sh
+
+fail=0
+
+test_url=test:///default
+
+virsh -c $test_url help  cmds || framework_failure
+cmds=$(sed -n 's/^\([^ ][^ ]*\) .*/\1/p' cmds) || framework_failure
+test -n $cmds || framework_failure
+
+for i in $cmds; do
+echo testing $i... 12
+virsh -c $test_url $i  /dev/null
+done
+
+(exit $fail); exit $fail
--
1.6.0.4.1021.g4320

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

Re: [libvirt] [PATCH] tests: new test: virsh-synopsis

[Jim Meyering]

Daniel P. Berrange [EMAIL PROTECTED] wrote:
 On Fri, Nov 21, 2008 at 11:13:03AM +0100, Jim Meyering wrote:
 I wanted help SYNOPSIS output that was regular enough to parse.

  static const vshCmdInfo info_network_create[] = {
 -{syntax, create a network from an XML file},
 +{syntax, net-create file},

 I'm kind of wondering why we manually write the syntax string at all.
 We know the name of the command, and we know the names of all the
 args, so we can auto-generate this entire string.

Right.  Everything we need is already in e.g., the
associated vshCmdOptDef table.

Any objection to committing that for now?
Once those lines are all generated, the test
won't be needed any more, but in the mean time
it might help.

BTW, I don't see a way to distinguish mutually-exclusive
boolean options (like --all and --inactive, which appear in each
synopsis like [ --inactive | --all ]) from ones that can be
combined like those of attach-disk

attach-disk domain source target [--driver driver] \
  [--subdriver subdriver] [--type type] [--mode mode]

which may be specified independently.
Sure, all of the latter type happen to be non-boolean,
and we could use that,... until we need to add a command
with exclusive non-bool options.

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

Re: [libvirt] [PATCH] tests: new test: virsh-synopsis

[Daniel P. Berrange]

On Fri, Nov 21, 2008 at 12:06:30PM +0100, Jim Meyering wrote:
 Daniel P. Berrange [EMAIL PROTECTED] wrote:
  On Fri, Nov 21, 2008 at 11:13:03AM +0100, Jim Meyering wrote:
  I wanted help SYNOPSIS output that was regular enough to parse.
 
   static const vshCmdInfo info_network_create[] = {
  -{syntax, create a network from an XML file},
  +{syntax, net-create file},
 
  I'm kind of wondering why we manually write the syntax string at all.
  We know the name of the command, and we know the names of all the
  args, so we can auto-generate this entire string.
 
 Right.  Everything we need is already in e.g., the
 associated vshCmdOptDef table.
 
 Any objection to committing that for now?
 Once those lines are all generated, the test
 won't be needed any more, but in the mean time
 it might help.

Ok.

 BTW, I don't see a way to distinguish mutually-exclusive
 boolean options (like --all and --inactive, which appear in each
 synopsis like [ --inactive | --all ]) from ones that can be
 combined like those of attach-disk

I don't think that's a huge problem - just list them all, and use the
description text to specify the constraints on their use.

Daniel
-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

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

Re: [libvirt] [PATCH] Fix UML syntax

[Daniel Veillard]

On Thu, Nov 20, 2008 at 03:04:40PM -0500, Ben Guthro wrote:
 various changes to fix make syntax-check succeed with new UML driver

Heh, I was about to submit the exact same patch, so commiting it now :-)

Daniel

-- 
Daniel Veillard  | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
[EMAIL PROTECTED]  | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library  http://libvirt.org/

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

RE: [libvirt] --with-xen=foo

[Ben Guthro]

Oh - sorry for not getting back to you about this.
Yes - it does solve it for me.

+1

-Original Message-
From: Daniel P. Berrange [mailto:[EMAIL PROTECTED]
Sent: Fri 11/21/2008 5:12 AM
To: Ben Guthro
Cc: libvir-list
Subject: Re: [libvirt] --with-xen=foo
 
On Thu, Nov 20, 2008 at 06:30:38PM +, Daniel P. Berrange wrote:
 On Thu, Nov 20, 2008 at 01:27:40PM -0500, Ben Guthro wrote:
  I tried this...didn't seem to help:
 
 Sorry, not paying close enough attention. In fact this file was
 built in an earlier module. Find the line which says
 
 libvirt_driver_la_SOURCES = \
 $(DRIVER_SOURCES)   \
 $(DOMAIN_CONF_SOURCES)  \
 $(NETWORK_CONF_SOURCES) \
 $(STORAGE_CONF_SOURCES) \
 $(NODE_DEVICE_CONF_SOURCES)
 
 And add in a line following that
 
 libvirt_driver_la_CFLAGS = $(XEN_CFLAGS)

On the assumption that this fixed it for you, I've committed this change

Regards,
Daniel
-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

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

Re: [libvirt] PATCH: 4/7: Remove driver API calls

[Daniel Veillard]

On Thu, Nov 20, 2008 at 05:56:34PM +, Daniel P. Berrange wrote:
 This provides the remote driver implementation for node device APIs
 
 This is basically just fixing up to add the optional 'cap' arg
 to the List methods, and remove the Create/Destroy/ByCap methods

  okay, +1

Daniel

-- 
Daniel Veillard  | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
[EMAIL PROTECTED]  | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library  http://libvirt.org/

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

Re: [libvirt] [PATCH] fix EventImpl-related error paths in remote driver

[Daniel P. Berrange]

On Thu, Nov 20, 2008 at 04:55:44PM -0500, David Lively wrote:
 This patch makes the remote driver behave properly in the face of:
   (a) no registered EventImpl, or
   (b) an EventImpl that returns failure from AddHandle/Timeout
 
 In both cases, we now cleanup properly (rather than always passing bogus
 values to virEventRemoveHandle/Timeout) and fail attempts to register
 for domain events??? (w/VIR_ERR_NO_SUPPORT rather than blissfully continue
 when we can't possibly deliver events).

All makes sense - you also fixed a RemoveHandle call to take priv-watch
instead of priv-sock which I had missed in my changes. I've committed
this patch

Daniel
-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

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

Re: [libvirt] [PATCH] tests: virsh-all: new script

[Daniel P. Berrange]

On Fri, Nov 21, 2008 at 11:16:18AM +0100, Jim Meyering wrote:
 Actually, this was the first step.
 This tiny script merely invokes all commands blindly,
 solely to get test coverage, even if they (as most do) fail.

It at least verifies that the commands don't crash or leak
memory in the error paths :-)

 +
 +if test $VERBOSE = yes; then
 +  set -x
 +  virsh --version
 +fi
 +
 +. $srcdir/test-lib.sh

$srcdir won't be set if you just invoke the test case directly 
as ./virsh-all. Can you add a default value assuming non-VPATH
build, eg

   test -z $srcdir  srcdir=`pwd`

Regards,
Daniel
-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

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

Re: [libvirt] [PATCH] Move the expected output data from virshdata/*.txt into virshtest.c.

[Daniel Veillard]

On Fri, Nov 21, 2008 at 10:27:55AM +0100, Jim Meyering wrote:
 Jim Meyering [EMAIL PROTECTED] wrote:
  This is mainly just reorganization and factorization, with an eye
  towards making it easier to add more tests in virshtest.c later.
 
 From 21352a07b073d52bcd3c442c76d7fb985aeef845 Mon Sep 17 00:00:00 2001
  From: Jim Meyering [EMAIL PROTECTED]
  Date: Thu, 20 Nov 2008 20:35:57 +0100
  Subject: [PATCH] Move the expected output data from virshdata/*.txt into 
  virshtest.c.
 
  * tests/virshtest.c: Embed literal, expected output here, rather than
  using virshdata/*.txt file names.
  Factor out some common constructs.
  (testCompareOutputLit): New function.
  (testCompareOutput): #ifdef-out, not that it's unused.
  * tests/Makefile.am (SUBDIRS): Remove virshdata/ and all files in it.
  * docs/testnode.xml: Fix typo in a comment.
 
 Since I'm removing a directory containing a Makefile.am,
 I've amended the patch to do this, too:
 
 * configure.in (AC_OUTPUT): Remove tests/virshdata/Makefile.

  I'm fine with those but wondering a bit how embedding thing in the .c
makes things any simpler.

Daniel

-- 
Daniel Veillard  | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
[EMAIL PROTECTED]  | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library  http://libvirt.org/

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

Re: [libvirt] PATCH: 5/7: virsh interface

[Daniel Veillard]

On Thu, Nov 20, 2008 at 05:57:37PM +, Daniel P. Berrange wrote:
 This provides the virsh binding. The main changes is to the
 list method to not call ByCaps anymore. And I've actually
 really renamed the commands this time !

  okay :-)
  +1

Daniel

-- 
Daniel Veillard  | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
[EMAIL PROTECTED]  | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library  http://libvirt.org/

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

Re: [libvirt] PATCH: 0/7: Final node device patches to be committed

[Daniel P. Berrange]

On Thu, Nov 20, 2008 at 05:49:50PM +, Daniel P. Berrange wrote:
 FYI, this is a repost of the final node device patches I intend to
 commit tomorrow. There's a few changes based on Mark's feedback
 which  I'll note against each patch...

This series is now fully committed to CVS. Many thanks to David
Lively for doing all the hard work on this patch series.

Regards,
Daniel
-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

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

Re: [libvirt] [PATCH] Move the expected output data from virshdata/*.txt into virshtest.c.

[Jim Meyering]

Daniel Veillard [EMAIL PROTECTED] wrote:
 On Fri, Nov 21, 2008 at 10:27:55AM +0100, Jim Meyering wrote:
 Jim Meyering [EMAIL PROTECTED] wrote:
  This is mainly just reorganization and factorization, with an eye
  towards making it easier to add more tests in virshtest.c later.
 
 From 21352a07b073d52bcd3c442c76d7fb985aeef845 Mon Sep 17 00:00:00 2001
  From: Jim Meyering [EMAIL PROTECTED]
  Date: Thu, 20 Nov 2008 20:35:57 +0100
  Subject: [PATCH] Move the expected output data from virshdata/*.txt into 
  virshtest.c.
 
  * tests/virshtest.c: Embed literal, expected output here, rather than
  using virshdata/*.txt file names.
  Factor out some common constructs.
  (testCompareOutputLit): New function.
  (testCompareOutput): #ifdef-out, not that it's unused.
  * tests/Makefile.am (SUBDIRS): Remove virshdata/ and all files in it.
  * docs/testnode.xml: Fix typo in a comment.

 Since I'm removing a directory containing a Makefile.am,
 I've amended the patch to do this, too:

 * configure.in (AC_OUTPUT): Remove tests/virshdata/Makefile.

   I'm fine with those but wondering a bit how embedding thing in the .c
 makes things any simpler.

My goal was to improve proximity/maintainability.
The more stand-alone and local (with all pieces in close proximity)
a test is, the easier it is to add another like it, or to make
adjustments without having to change relatively distant pieces.
Now, if I want to change one of those test scenarios, I can do it
by changing that one file.  Well, there are still exceptions:
the initialization files in ../doc/*.xml, but that's not a big deal.

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

[libvirt] [PATCH] * gnulib/tests/test-EOVERFLOW: Remove.

[Jim Meyering]

Back on October 28, I mistakenly added a binary.
This adds the requisite .cvsignore entry and
updates the corresponding .gitignore file as well
as two stale ones.


From 30a853d37fc2102a7e4009b93867209e318014a1 Mon Sep 17 00:00:00 2001
From: Jim Meyering [EMAIL PROTECTED]
Date: Fri, 21 Nov 2008 14:15:15 +0100
Subject: [PATCH] * gnulib/tests/test-EOVERFLOW: Remove.

* gnulib/tests/.cvsignore: Add test-EOVERFLOW, then...
Run make sync-vcs-ignore-files to Update .gitignore files.
---
 examples/domain-events/events-c/.gitignore |2 ++
 gnulib/tests/.cvsignore|1 +
 gnulib/tests/.gitignore|1 +
 gnulib/tests/test-EOVERFLOW|  Bin 6680 - 0 bytes
 src/.gitignore |1 +
 5 files changed, 5 insertions(+), 0 deletions(-)
 delete mode 100755 gnulib/tests/test-EOVERFLOW

diff --git a/examples/domain-events/events-c/.gitignore 
b/examples/domain-events/events-c/.gitignore
index 7b8079b..ed9ac4d 100644
--- a/examples/domain-events/events-c/.gitignore
+++ b/examples/domain-events/events-c/.gitignore
@@ -1,3 +1,5 @@
+Makefile
+Makefile.in
 *.exe
 .deps
 .libs
diff --git a/gnulib/tests/.cvsignore b/gnulib/tests/.cvsignore
index 6ac20bc..04eb4a0 100644
--- a/gnulib/tests/.cvsignore
+++ b/gnulib/tests/.cvsignore
@@ -3,6 +3,7 @@
 Makefile
 Makefile.in
 sys
+test-EOVERFLOW
 test-alloca-opt
 test-arpa_inet
 test-c-ctype
diff --git a/gnulib/tests/.gitignore b/gnulib/tests/.gitignore
index 6ac20bc..04eb4a0 100644
--- a/gnulib/tests/.gitignore
+++ b/gnulib/tests/.gitignore
@@ -3,6 +3,7 @@
 Makefile
 Makefile.in
 sys
+test-EOVERFLOW
 test-alloca-opt
 test-arpa_inet
 test-c-ctype
diff --git a/gnulib/tests/test-EOVERFLOW b/gnulib/tests/test-EOVERFLOW
deleted file mode 100755
index 
ea3a8a27290fea9dd71beab276a9180b71a4847b..
GIT binary patch
literal 0
HcmV?d1

literal 6680
zcmcIoU2Ggz6+Zjp*eP!8n3%YUQAX-Ua*#aT7{_sdz}SxM*|3d`3ON=*^PHZ?)dF
zcE(`?MYV7XTDfkGctPT!72*K|4|zdU0*OQ[EMAIL PROTECTED]
...
zau)bXmrZ?BcK[EMAIL PROTECTED];CN`0sM9)[EMAIL PROTECTED](qd;a|KwUuHtx8go~
Q6T0rYn6HtD`jQ!Z=`m;S4c

diff --git a/src/.gitignore b/src/.gitignore
index 4aff461..cef59e0 100644
--- a/src/.gitignore
+++ b/src/.gitignore
@@ -15,3 +15,4 @@ libvirt_parthelper
 libvirt_lxc
 virsh-net-edit.c
 virsh-pool-edit.c
+libvirt_sym.version
--
1.6.0.4.1021.g4320

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

Re: [libvirt] [PATCH 1/2] Java bindings for domain events

[Daniel Veillard]

On Wed, Nov 19, 2008 at 11:22:31AM -0500, David Lively wrote:
 On Wed, 2008-11-19 at 10:35 -0500, David Lively wrote:
  The patch already synchronizes operations using virConnect objects with
  each other.  To avoid making illegal EventImpl callbacks from Java for
  the current libvirt, I have to lock every Connect object known to Java
  and hold off creating new connections (via open  friends) around an
  EventImpl callback.  This sounds rather appalling to me, but it's
  starting to sound like the only practical route in the short term
  (unless it turns out we can rely on pthreads in WIN32 ...).
 
 Hmmm ... maybe the less appalling :-) route is practical.  Currently, we
 require only the Windows equivalent of a simple pthread mutex.  We just
 need to support declaration, initialization, lock, unlock, and
 destruction, something like the following (thanks to Tom Hazel for
 pointing me to the Windows Mutex stuff):
 
 #if (defined _WIN32 || defined __WIN32__)
 #define PTHREAD_MUTEX_T(v) HANDLE v
 #define pthread_mutex_init(lk,p) ((*(lk)) = CreateMutex(0, FALSE, 0))
 #define pthread_mutex_destroy(lk) CloseHandle(*(lk))
 #define pthread_mutex_lock(lk) WaitForSingleObject(*(lk), INFINITE)
 #define pthread_mutex_unlock(lk) ReleaseMutex(*(lk))
 #define pthread_sigmask(h, s, o) sigprocmask((h), (s), (o))
 #endif
 
 I'm not a Windows guy, so maybe I'm missing something.  But this doesn't
 seem like a Big Deal ...

  note that libxml2 that we rely on has a fully ported mutex basic API
in libxml/threads.h

/*
 * xmlMutex are a simple mutual exception locks.
 */
typedef struct _xmlMutex xmlMutex;
typedef xmlMutex *xmlMutexPtr;

/*
 * xmlRMutex are reentrant mutual exception locks.
 */
typedef struct _xmlRMutex xmlRMutex;
typedef xmlRMutex *xmlRMutexPtr;

XMLPUBFUN xmlMutexPtr XMLCALL
xmlNewMutex (void);
XMLPUBFUN void XMLCALL
xmlMutexLock(xmlMutexPtr tok);
XMLPUBFUN void XMLCALL
xmlMutexUnlock  (xmlMutexPtr tok);
XMLPUBFUN void XMLCALL
xmlFreeMutex(xmlMutexPtr tok);

XMLPUBFUN xmlRMutexPtr XMLCALL
xmlNewRMutex(void);
XMLPUBFUN void XMLCALL
xmlRMutexLock   (xmlRMutexPtr tok);
XMLPUBFUN void XMLCALL
xmlRMutexUnlock (xmlRMutexPtr tok);
XMLPUBFUN void XMLCALL
xmlFreeRMutex   (xmlRMutexPtr tok);

 in case you really want to do the exclusive locking at the C level
while still being portable.

  Still it's probably better to try to implement most of this at the
Java level, at least IMHO,

Daniel

-- 
Daniel Veillard  | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
[EMAIL PROTECTED]  | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library  http://libvirt.org/

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

Re: [libvirt] --with-xen=foo

[Ben Guthro]

hmm...It looks like this is not the only problem.

If I do a clean build with the following options:
./autogen.sh --enable-compile-warnings=error \
 --with-xen \ 
 --without-uml \  
 --with-storage-disk \
 --with-storage-iscsi \   
 --prefix=/usr

I get an error when building testutilsxen.c

gcc -DHAVE_CONFIG_H -I. -I.. -I../gnulib/lib -I../gnulib/lib -I../include 
-I../src -I../include -I../src -I/usr/include/libxml2  
-DGETTEXT_PACKAGE=\libvirt\  -Wall -Wformat -Wformat-security 
-Wmissing-prototypes -Wnested-externs -Wpointer-arith -Wextra -Wshadow 
-Wcast-align -Wwrite-strings -Waggregate-return -Wstrict-prototypes -Winline 
-Wredundant-decls -Wno-sign-compare -Wp,-D_FORTIFY_SOURCE=2 -fexceptions 
-fasynchronous-unwind-tables  -O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 
-fexceptions -fstack-protector --param=ssp-buffer-size=4 -m64 -mtune=generic 
-MT testutilsxen.o -MD -MP -MF .deps/testutilsxen.Tpo -c -o testutilsxen.o 
testutilsxen.c
make[4]: *** No rule to make target `../src/libvirt_driver_xen.la', needed by 
`xml2sexprtest'.  Stop.


...it would appear that some dependency is incorrect...


Ben Guthro wrote on 11/21/2008 06:52 AM:
 
 Oh - sorry for not getting back to you about this.
 Yes - it does solve it for me.
 
 +1
 
 -Original Message-
 From: Daniel P. Berrange [mailto:[EMAIL PROTECTED]
 Sent: Fri 11/21/2008 5:12 AM
 To: Ben Guthro
 Cc: libvir-list
 Subject: Re: [libvirt] --with-xen=foo
 
 On Thu, Nov 20, 2008 at 06:30:38PM +, Daniel P. Berrange wrote:
 On Thu, Nov 20, 2008 at 01:27:40PM -0500, Ben Guthro wrote:
  I tried this...didn't seem to help:

 Sorry, not paying close enough attention. In fact this file was
 built in an earlier module. Find the line which says

 libvirt_driver_la_SOURCES = \
 $(DRIVER_SOURCES)   \
 $(DOMAIN_CONF_SOURCES)  \
 $(NETWORK_CONF_SOURCES) \
 $(STORAGE_CONF_SOURCES) \
 $(NODE_DEVICE_CONF_SOURCES)

 And add in a line following that

 libvirt_driver_la_CFLAGS = $(XEN_CFLAGS)
 
 On the assumption that this fixed it for you, I've committed this change
 
 Regards,
 Daniel
 --
 |: Red Hat, Engineering, London   -o-  
 http://people.redhat.com/berrange/ :|
 |: http://libvirt.org  -o-  http://virt-manager.org  -o- 
 http://ovirt.org :|
 |: http://autobuild.org   -o-
 http://search.cpan.org/~danberr/ :|
 |: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B
 9505 :|
 
 
 
 
 --
 Libvir-list mailing list
 Libvir-list@redhat.com
 https://www.redhat.com/mailman/listinfo/libvir-list

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

Re: [libvirt] [PATCH] fix python events

[David Lively]

Doh! ... attached :-)


On Fri, 2008-11-21 at 10:30 +0100, Jim Meyering wrote:
 David Lively [EMAIL PROTECTED] wrote:
  This patch gets python events working again after upstream changes, and
  make the test implementation properly clean up after itself and
  implement the new EventImpl API properly.
 
  Note that the Python RemoveHandle and RemoveTimeout implementations
  should return the opaque object registered by the corresponding
  AddHandle/Timeout calls, in lieu of calling the (C) freefunc.  (The
  binding code will then call freefunc if it's not NULL.)  Ignoring this
  means you'll leak memory in the same way that C RemoveHandle/Timeout
  leak if they don't (now) call the freefunc.
 
  I also moved around some of the error checking code to unclutter (and
  speed up) the common code paths.  For instance, we now check that the
  virRegisterEventImpl arguments are callable just once (and return
  failure if they're not), rather than checking them before every call and
  blithely ignoring them if they're not callable.
 
  Dave
 
   examples/domain-events/events-python/event-test.py |   29 +--
   python/libvir.c|  
  200+++
   python/libvir.py   |4
   python/libvirt_wrap.h  |8
   python/types.c |1
   python/virConnect.py   |4
   6 files changed, 194 insertions(+), 52 deletions(-)
 
 Hi Dave,
 
 It looks like this patch didn't make it to the list.
commit efd5098e9a834562cddbf1618e36eb43c272f8ea
Author: David Lively [EMAIL PROTECTED]
Date:   Thu Nov 20 16:36:14 2008 -0500

vi-patch: fix-python-events

Get python events working again after upstream changes, and make the
test implementation properly clean up after itself and implement the
new EventImpl properly.

diff --git a/examples/domain-events/events-python/event-test.py b/examples/domain-events/events-python/event-test.py
index 45aaa93..1ad436f 100644
--- a/examples/domain-events/events-python/event-test.py
+++ b/examples/domain-events/events-python/event-test.py
@@ -72,22 +72,22 @@ def myAddHandle(fd, events, cb, opaque):
 h_events = events
 h_cb = cb
 h_opaque = opaque
-
 mypoll.register(fd, myEventHandleTypeToPollEvent(events))
+return 0
 
 def myUpdateHandle(watch, event):
 global h_fd, h_events
-#print Updating Handle %s %s % (str(fd), str(events))
-h_fd = fd
+#print Updating Handle %s %s % (str(h_fd), str(event))
 h_events = event
-mypoll.unregister(watch)
-mypoll.register(watch, myEventHandleTypeToPollEvent(event))
+mypoll.unregister(h_fd)
+mypoll.register(h_fd, myEventHandleTypeToPollEvent(event))
 
 def myRemoveHandle(watch):
 global h_fd
-#print Removing Handle %s % str(fd)
+#print Removing Handle %s % str(h_fd)
+mypoll.unregister(h_fd)
 h_fd = 0
-mypoll.unregister(watch)
+return h_opaque
 
 def myAddTimeout(timeout, cb, opaque):
 global t_active, t_timeout, t_cb, t_opaque
@@ -96,16 +96,18 @@ def myAddTimeout(timeout, cb, opaque):
 t_timeout = timeout;
 t_cb = cb;
 t_opaque = opaque;
+return 0
 
 def myUpdateTimeout(timer, timeout):
 global t_timeout
-#print Updating Timeout %s % (str(timer), str(timeout))
+#print Updating Timeout %s %s % (str(timer), str(timeout))
 t_timeout = timeout;
 
 def myRemoveTimeout(timer):
 global t_active
 #print Removing Timeout %s % str(timer)
 t_active = 0;
+return t_opaque
 
 ##
 # Main
@@ -143,6 +145,14 @@ def main():
myRemoveTimeout );
 vc = libvirt.open(uri)
 
+# Close connection on exit (to test cleanup paths)
+old_exitfunc = getattr(sys, 'exitfunc', None)
+def exit():
+print Closing  + str(vc)
+vc.close()
+if (old_exitfunc): old_exitfunc()
+sys.exitfunc = exit
+
 #Add 2 callbacks to prove this works with more than just one
 vc.domainEventRegister(myDomainEventCallback1,None)
 vc.domainEventRegister(myDomainEventCallback2,None)
@@ -175,8 +185,7 @@ def main():
 
 if h_cb != None:
 #print Invoking Handle CB
-h_cb(0, h_fd, myPollEventToEventHandleType(revents  h_events),
- h_opaque[0], h_opaque[1])
+h_cb(0, h_fd, myPollEventToEventHandleType(revents  h_events), h_opaque[0], h_opaque[1])
 
 #print DEBUG EXIT
 #break
diff --git a/python/libvir.c b/python/libvir.c
index 07ed09e..6ae7cc1 100644
--- a/python/libvir.c
+++ b/python/libvir.c
@@ -35,6 +35,18 @@ extern void initcygvirtmod(void);
 #define VIR_PY_INT_FAIL (libvirt_intWrap(-1))
 #define VIR_PY_INT_SUCCESS (libvirt_intWrap(0))
 
+static char *py_str(PyObject *obj)
+{
+PyObject *str = PyObject_Str(obj);
+if (!str) {
+PyErr_Print();
+PyErr_Clear();
+return NULL;
+};
+return 

Re: [libvirt] [PATCH] fix python events

[Jim Meyering]

David Lively [EMAIL PROTECTED] wrote:
...

Hi David,

No big deal, but there are several debug printf uses here that look
like they try to print NULL pointers upon memory allocation failure.
It's ok with glibc's printf of course, but not for others.

 commit efd5098e9a834562cddbf1618e36eb43c272f8ea
 Author: David Lively [EMAIL PROTECTED]
 Date:   Thu Nov 20 16:36:14 2008 -0500

 vi-patch: fix-python-events

 Get python events working again after upstream changes, and make the
 test implementation properly clean up after itself and implement the
 new EventImpl properly.
...
 diff --git a/python/libvir.c b/python/libvir.c
...
 @@ -1725,9 +1745,18 @@ libvirt_virEventAddHandleFunc  (int fd,
   eventInvokeHandleCallback);
  if(!python_cb) {
  #if DEBUG_ERROR
 -printf(%s Error finding eventInvokeHandleCallback\n, __FUNCTION__);
 +printf(%s: Error finding eventInvokeHandleCallback\n, 
 __FUNCTION__);
  #endif
  PyErr_Print();
 +PyErr_Clear();
 +goto cleanup;
 +}
 +if (!PyCallable_Check(python_cb)) {
 +#if DEBUG_ERROR
 +char *name = py_str(python_cb);
 +printf(%s: %s is not callable\n, __FUNCTION__, name);

Please handle name == NULL.
Otherwise, with DEBUG_ERROR, the printf will segfault
on most non-glibc-based systems.

E.g., define this, then wrap the arg with NN(...).
static inline char *NN (const char *s)
{
  return s ? s : (char *) NULL;
}

   printf(%s: %s is not callable\n, __FUNCTION__, NN(name));

 +free(name);
 +#endif
  goto cleanup;
  }
  Py_INCREF(python_cb);
 @@ -1748,8 +1777,17 @@ libvirt_virEventAddHandleFunc  (int fd,
  PyTuple_SetItem(pyobj_args, 2, python_cb);
  PyTuple_SetItem(pyobj_args, 3, cb_args);

 -if(addHandleObj  PyCallable_Check(addHandleObj))
 -result = PyEval_CallObject(addHandleObj, pyobj_args);
 +result = PyEval_CallObject(addHandleObj, pyobj_args);
 +if (!result) {
 +PyErr_Print();
 +PyErr_Clear();
 +} else if (!PyInt_Check(result)) {
 +#if DEBUG_ERROR
 +printf(%s: %s should return an int\n, addHandleName, __FUNCTION__);

Same here, addHandleName can be NULL.

 +#endif
 +} else {
 +retval = (int)PyInt_AsLong(result);
 +}

  Py_XDECREF(result);
  Py_DECREF(pyobj_args);
 @@ -1757,23 +1795,26 @@ libvirt_virEventAddHandleFunc  (int fd,
  cleanup:
  LIBVIRT_RELEASE_THREAD_STATE;

 -return 0;
 +return retval;
  }

  static void
 -libvirt_virEventUpdateHandleFunc(int fd, int event)
 +libvirt_virEventUpdateHandleFunc(int watch, int event)
  {
 -PyObject *result = NULL;
 +PyObject *result;
  PyObject *pyobj_args;

  LIBVIRT_ENSURE_THREAD_STATE;

  pyobj_args = PyTuple_New(2);
 -PyTuple_SetItem(pyobj_args, 0, libvirt_intWrap(fd));
 +PyTuple_SetItem(pyobj_args, 0, libvirt_intWrap(watch));
  PyTuple_SetItem(pyobj_args, 1, libvirt_intWrap(event));

 -if(updateHandleObj  PyCallable_Check(updateHandleObj))
 -result = PyEval_CallObject(updateHandleObj, pyobj_args);
 +result = PyEval_CallObject(updateHandleObj, pyobj_args);
 +if (!result) {
 +PyErr_Print();
 +PyErr_Clear();
 +}

  Py_XDECREF(result);
  Py_DECREF(pyobj_args);
 @@ -1783,25 +1824,45 @@ libvirt_virEventUpdateHandleFunc(int fd, int event)


  static int
 -libvirt_virEventRemoveHandleFunc(int fd)
 +libvirt_virEventRemoveHandleFunc(int watch)
  {
 -PyObject *result = NULL;
 +PyObject *result;
  PyObject *pyobj_args;
 +PyObject *opaque;
 +PyObject *ff;
 +int retval = -1;
 +virFreeCallback cff;

  LIBVIRT_ENSURE_THREAD_STATE;

  pyobj_args = PyTuple_New(1);
 -PyTuple_SetItem(pyobj_args, 0, libvirt_intWrap(fd));
 +PyTuple_SetItem(pyobj_args, 0, libvirt_intWrap(watch));

 -if(removeHandleObj  PyCallable_Check(removeHandleObj))
 -result = PyEval_CallObject(removeHandleObj, pyobj_args);
 +result = PyEval_CallObject(removeHandleObj, pyobj_args);
 +if (!result) {
 +PyErr_Print();
 +PyErr_Clear();
 +} else if (!PyTuple_Check(result) || PyTuple_Size(result) != 3) {
 +#if DEBUG_ERROR
 +printf(%s: %s must return opaque obj registered with %s
 +   to avoid leaking libvirt memory\n,
 +   __FUNCTION__, removeHandleName, addHandleName);

Same here, for both.

 +#endif
 +} else {
 +opaque = PyTuple_GetItem(result, 1);
 +ff = PyTuple_GetItem(result, 2);
 +cff = PyvirFreeCallback_Get(ff);
 +if (cff)
 +(*cff)(PyvirVoidPtr_Get(opaque));
 +retval = 0;
 +}

  Py_XDECREF(result);
  Py_DECREF(pyobj_args);

  LIBVIRT_RELEASE_THREAD_STATE;

 -return 0;
 +return retval;
  }

  static int
 @@ -1810,7 +1871,7 @@ libvirt_virEventAddTimeoutFunc(int timeout,
 void *opaque,
 virFreeCallback ff)
  

Re: [libvirt] [PATCH] fix --without-xen build

[David Lively]

Hold off on considering this quite yet.  I don't think it's a complete
fix.  I'll followup with a complete one soon.

Dave


On Fri, 2008-11-21 at 11:04 -0500, David Lively wrote:
 The last change to tests/Makefile.am broke the build when --without-xen
 is specified.  This patch fixes it ...
 
 Dave
 
 
  tests/Makefile.am |9 +
  1 file changed, 9 insertions(+)
 
 
 --
 Libvir-list mailing list
 Libvir-list@redhat.com
 https://www.redhat.com/mailman/listinfo/libvir-list

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

Re: [libvirt] [PATCH] fix --without-xen build

[David Lively]

Okay, *this* one seems to be a complete fix.

Dave

 tests/Makefile.am |   29 ++---
 1 file changed, 22 insertions(+), 7 deletions(-)

On Fri, 2008-11-21 at 12:35 -0500, David Lively wrote:
 Hold off on considering this quite yet.  I don't think it's a complete
 fix.  I'll followup with a complete one soon.

diff --git a/tests/Makefile.am b/tests/Makefile.am
index d2d1254..a17fdd4 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -2,8 +2,11 @@
 
 SHELL = $(PREFERABLY_POSIX_SHELL)
 
-SUBDIRS = virshdata confdata sexpr2xmldata \
-  xml2sexprdata xmconfigdata xencapsdata
+SUBDIRS = virshdata confdata
+
+if ENABLE_XEN_TESTS
+SUBDIRS += sexpr2xmldata xml2sexprdata xmconfigdata xencapsdata
+endif
 
 INCLUDES = \
 	-I$(top_srcdir)/gnulib/lib -I../gnulib/lib \
@@ -43,10 +46,14 @@ EXTRA_DIST =		\
 	nodeinfodata \
 	domainschematest
 
-noinst_PROGRAMS = xmlrpctest xml2sexprtest sexpr2xmltest virshtest conftest \
-	reconnect xmconfigtest xencapstest \
+noinst_PROGRAMS = xmlrpctest virshtest conftest \
 nodeinfotest statstest qparamtest
 
+if ENABLE_XEN_TESTS
+noinst_PROGRAMS += xml2sexprtest sexpr2xmltest reconnect xmconfigtest \
+	xencapstest
+endif
+
 if WITH_QEMU
 noinst_PROGRAMS += qemuxml2argvtest qemuxml2xmltest
 endif
@@ -66,11 +73,10 @@ endif
 
 EXTRA_DIST += $(test_scripts)
 
-TESTS = xml2sexprtest sexpr2xmltest virshtest xmconfigtest \
-xencapstest nodeinfotest \
+TESTS = virshtest nodeinfotest \
 	statstest qparamtest $(test_scripts)
 if ENABLE_XEN_TESTS
-  TESTS += reconnect
+  TESTS += reconnect xml2sexprtest sexpr2xmltest xmconfigtest xencapstest
 endif
 if WITH_QEMU
 TESTS += qemuxml2argvtest qemuxml2xmltest
@@ -103,6 +109,7 @@ xmlrpctest_SOURCES = \
 
 xmlrpctest_LDADD = $(LDADDS)
 
+if ENABLE_XEN_TESTS
 xml2sexprtest_SOURCES = \
 	xml2sexprtest.c testutilsxen.c testutilsxen.h \
 	testutils.c testutils.h
@@ -117,6 +124,10 @@ xmconfigtest_SOURCES = \
 	xmconfigtest.c testutilsxen.c testutilsxen.h \
 	testutils.c testutils.h
 xmconfigtest_LDADD = ../src/libvirt_driver_xen.la $(LDADDS)
+else
+EXTRA_DIST += xml2sexprtest.c sexpr2xmltest.c xmconfigtest.c \
+	testutilsxen.c testutilsxen.h
+endif
 
 if WITH_QEMU
 qemuxml2argvtest_SOURCES = \
@@ -141,9 +152,13 @@ conftest_SOURCES = \
 	conftest.c
 conftest_LDADD = $(LDADDS)
 
+if ENABLE_XEN_TESTS
 xencapstest_SOURCES = \
 	xencapstest.c testutils.h testutils.c
 xencapstest_LDADD = ../src/libvirt_driver_xen.la $(LDADDS)
+else
+EXTRA_DIST += xencapstest.c
+endif
 
 nodeinfotest_SOURCES = \
 	nodeinfotest.c testutils.h testutils.c
--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH] Xen Events Updated

[Ben Guthro]

I tested these changes, and seem to be having some issues opening the xen 
driver in a post 3.0.3 codepath:

DEBUG: xen_unified.c: xenUnifiedOpen (Trying hypervisor sub-driver)
DEBUG: xen_unified.c: xenUnifiedOpen (Activated hypervisor sub-driver)
libvir: Xen Daemon error : internal error failed to create a socket   
libvir: Xen error : out of memory 
DEBUG: xen_unified.c: xenUnifiedOpen (Failed to make capabilities)
DEBUG: xen_unified.c: xenUnifiedOpen (Failed to activate a mandatory sub-driver)
DEBUG: libvirt.c: do_open (driver 1 Xen returned ERROR)


I haven't chased this down fully yet, but something is interfering...



Daniel P. Berrange wrote on 11/21/2008 11:13 AM:
 On Thu, Nov 20, 2008 at 03:49:07PM -0500, Ben Guthro wrote:
 New xen events patch attached. This removes a couple unnecessary 
 changes from my prior patch, but remains functionally the same as
 the last version.

 This will emit the following events for Xen:

 STARTED
 STOPPED
 ADDED
 REMOVED
 
 I hit a few problems with the old Xen 3.0.3 codepath for /etc/xen files
 in this patch. What follows is the patch I needed to make it work
 reliably on RHEL-5 Xen.
 
 The changes are
 
  - Remove hardcoded qemu:///system to let libvirt probe HV
  - Add a cast to workaround lack of const-ness in RHEL5 python
  - Add an explicit xenXMConfigRemoveFile() to deal with files
going away
  - Remove use of IN_MODIFY - it fired when the config was still
incompletely written resulting in wierd errors
  - Add use of IN_CLOSE_WRITE so we're notified when the file is
finished writing
  - Ignoring IN_CREATE if stat() shows zero size, because this fires
the moment the name is allocated in the FS, but before content
is created. We can't ignore it completely, because its needed
if someone creates the file via hard-linking, when the entire
content appears attomically  no IN_CLOSED_WRITE is fired.
  - Allocate capabilities info before initializing inotify driver
because loading XM config files /etc/xen requires this
  - Fix removal of file handles for inotify  xenstore, since we
need to remove based on watch number, not FD number
 
 Regards,
 Daniel
 
 diff -r fe87b41b48e3 examples/domain-events/events-c/event-test.c
 --- a/examples/domain-events/events-c/event-test.cFri Nov 21 08:02:15 
 2008 -0500
 +++ b/examples/domain-events/events-c/event-test.cFri Nov 21 08:40:58 
 2008 -0500
 @@ -308,7 +308,7 @@ int main(int argc, char **argv)
myEventRemoveTimeoutFunc);
  
  virConnectPtr dconn = NULL;
 -dconn = virConnectOpen (argv[1] ? argv[1] : qemu:///system);
 +dconn = virConnectOpen (argv[1] ? argv[1] : NULL);
  if (!dconn) {
  printf(error opening\n);
  return -1;
 diff -r fe87b41b48e3 include/libvirt/virterror.h
 --- a/include/libvirt/virterror.h Fri Nov 21 08:02:15 2008 -0500
 +++ b/include/libvirt/virterror.h Fri Nov 21 08:02:47 2008 -0500
 @@ -60,6 +60,7 @@ typedef enum {
  VIR_FROM_DOMAIN,/* Error from domain config */
  VIR_FROM_UML,   /* Error at the UML driver */
  VIR_FROM_NODEDEV, /* Error from node device monitor */
 +VIR_FROM_XEN_INOTIFY, /* Error from xen inotify layer */
  } virErrorDomain;
  
  
 diff -r fe87b41b48e3 python/libvir.c
 --- a/python/libvir.c Fri Nov 21 08:02:15 2008 -0500
 +++ b/python/libvir.c Fri Nov 21 08:36:40 2008 -0500
 @@ -1564,7 +1564,8 @@ getLibvirtModuleObject (void) {
  return libvirt_module;
  
  // PyImport_ImportModule returns a new reference
 -libvirt_module = PyImport_ImportModule(libvirt);
 +/* Bogus (char *) cast for RHEL-5 python API brokenness */
 +libvirt_module = PyImport_ImportModule((char *)libvirt);
  if(!libvirt_module) {
  #if DEBUG_ERROR
  printf(%s Error importing libvirt module\n, __FUNCTION__);
 diff -r fe87b41b48e3 src/util.c
 --- a/src/util.c  Fri Nov 21 08:02:15 2008 -0500
 +++ b/src/util.c  Fri Nov 21 11:07:03 2008 -0500
 @@ -613,6 +613,10 @@ virRun(virConnectPtr conn,
  VIR_FREE(outbuf);
  VIR_FREE(errbuf);
  VIR_FREE(argv_str);
 +if (outfd != -1)
 +close(outfd);
 +if (errfd != -1)
 +close(errfd);
  return ret;
  }
  
 diff -r fe87b41b48e3 src/xen_inotify.c
 --- a/src/xen_inotify.c   Fri Nov 21 08:02:15 2008 -0500
 +++ b/src/xen_inotify.c   Fri Nov 21 11:07:22 2008 -0500
 @@ -49,12 +49,6 @@ static const char *configDir= NU
  static const char *configDir= NULL;
  static int  useXenConfigCache = 0;
  static xenUnifiedDomainInfoListPtr configInfoList = NULL;
 -
 -/* declared in xm_internal.c */
 -virHashTablePtr xenXMGetConfigCache(void);
 -char *xenXMGetConfigDir(void);
 -int xenXMConfigCacheRefresh (virConnectPtr conn);
 -int xenXMConfigCacheAddRemoveFile(virConnectPtr conn, const char *filename);
  
  struct xenUnifiedDriver xenInotifyDriver = {
  xenInotifyOpen, /* open */
 @@ -121,7 +115,7 @@ 

Re: [libvirt] [PATCH] Xen Events Updated

[Daniel P. Berrange]

On Fri, Nov 21, 2008 at 01:06:08PM -0500, Ben Guthro wrote:
 I tested these changes, and seem to be having some issues opening the xen 
 driver in a post 3.0.3 codepath:
 
 DEBUG: xen_unified.c: xenUnifiedOpen (Trying hypervisor sub-driver)
 DEBUG: xen_unified.c: xenUnifiedOpen (Activated hypervisor sub-driver)
 libvir: Xen Daemon error : internal error failed to create a socket   
 libvir: Xen error : out of memory 
 DEBUG: xen_unified.c: xenUnifiedOpen (Failed to make capabilities)
 DEBUG: xen_unified.c: xenUnifiedOpen (Failed to activate a mandatory 
 sub-driver)
 DEBUG: libvirt.c: do_open (driver 1 Xen returned ERROR)
 
 
 I haven't chased this down fully yet, but something is interfering...

Wierd, I'm rather puzzelled as to why you'd get the 'Activated hypervisor
sub-driver' message, but not immediately see a 'Trying XenD sub-driver'
message, since there's only one line of code in between them which is a
simple assignment...

DEBUG0(Trying hypervisor sub-driver);
if (drivers[XEN_UNIFIED_HYPERVISOR_OFFSET]-open(conn, auth, flags) ==
VIR_DRV_OPEN_SUCCESS) {
DEBUG0(Activated hypervisor sub-driver);
priv-opened[XEN_UNIFIED_HYPERVISOR_OFFSET] = 1;
}
}

/* XenD is required to suceed if root.
 * If it fails as non-root, then the proxy driver may take over
 */
DEBUG0(Trying XenD sub-driver);
if (drivers[XEN_UNIFIED_XEND_OFFSET]-open(conn, auth, flags) ==
VIR_DRV_OPEN_SUCCESS) {
DEBUG0(Activated XenD sub-driver);
priv-opened[XEN_UNIFIED_XEND_OFFSET] = 1;


DEBUG: xen_unified.c: xenUnifiedOpen (Trying hypervisor sub-driver)
DEBUG: xen_unified.c: xenUnifiedOpen (Activated hypervisor sub-driver)
DEBUG: xen_unified.c: xenUnifiedOpen (Trying XenD sub-driver)
DEBUG: xen_unified.c: xenUnifiedOpen (Activated XenD sub-driver)
...snip...

One other problem I've noticed is a race condition on the @introduceDomain
watch. The watch fires as soon as the domain ID is added to xenstore.
We then query the name  uuid, but occassionally we query before XenD has
had a chance to fill them in.

I think that upon failure, we'll have to add a timer callback to retry
the lookup after 500ms, to take account of possible race. I'm going to
give this a try...

Daniel
-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

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

Re: [libvirt] [PATCH] Xen Events Updated

[Ben Guthro]

I think I found the culprit of this:

Daniel P. Berrange wrote on 11/21/2008 11:13 AM:
  - Allocate capabilities info before initializing inotify driver
because loading XM config files /etc/xen requires this
 
...
 -if (!(priv-caps = xenHypervisorMakeCapabilities(conn))) {
 -DEBUG0(Failed to make capabilities);
 -goto fail;
 -}

If I undo this part of your patch, everything else works as designed.

While I have no evidence yet to support this claim, I think this is failing 
trying to open the xend socket, perhaps before the xend driver is open...

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

Re: [libvirt] [PATCH] Xen Events Updated

[Daniel P. Berrange]

On Fri, Nov 21, 2008 at 01:56:16PM -0500, Ben Guthro wrote:
 I think I found the culprit of this:
 
 Daniel P. Berrange wrote on 11/21/2008 11:13 AM:
   - Allocate capabilities info before initializing inotify driver
 because loading XM config files /etc/xen requires this
  
 ...
  -if (!(priv-caps = xenHypervisorMakeCapabilities(conn))) {
  -DEBUG0(Failed to make capabilities);
  -goto fail;
  -}
 
 If I undo this part of your patch, everything else works as designed.
 
 While I have no evidence yet to support this claim, I think this is 
 failing trying to open the xend socket, perhaps before the xend driver
 is open...

Doh. Merge error. Originally I had moved it to be immediately before
the call to initialize xen inotify driver. For inexplicable reasons,
in the new patch I have it before the XenD driver. 

Daniel
-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

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

Re: [libvirt] [PATCH] Xen Events Updated

[Ben Guthro]

I have integrated your changes, and moved this bit referenced below, so it 
works with my setup. Could you please test with yours, and make sure I didn't 
break anything?

 configure.in |   13
 examples/domain-events/events-c/event-test.c |2
 include/libvirt/virterror.h  |1
 po/POTFILES.in   |1
 python/libvir.c  |3
 src/Makefile.am  |5
 src/util.c   |4
 src/virterror.c  |3
 src/xen_inotify.c|  453 +++
 src/xen_inotify.h|   31 +
 src/xen_unified.c|  200 +++
 src/xen_unified.h|   61 +++
 src/xm_internal.c|  198 +++
 src/xm_internal.h|6
 src/xs_internal.c|  431 +
 src/xs_internal.h|   51 +++
 16 files changed, 1393 insertions(+), 70 deletions(-)


Daniel P. Berrange wrote on 11/21/2008 02:07 PM:
 On Fri, Nov 21, 2008 at 01:56:16PM -0500, Ben Guthro wrote:
 I think I found the culprit of this:

 Daniel P. Berrange wrote on 11/21/2008 11:13 AM:
  - Allocate capabilities info before initializing inotify driver
because loading XM config files /etc/xen requires this

 ...
 -if (!(priv-caps = xenHypervisorMakeCapabilities(conn))) {
 -DEBUG0(Failed to make capabilities);
 -goto fail;
 -}
 If I undo this part of your patch, everything else works as designed.

 While I have no evidence yet to support this claim, I think this is 
 failing trying to open the xend socket, perhaps before the xend driver
 is open...
 
 Doh. Merge error. Originally I had moved it to be immediately before
 the call to initialize xen inotify driver. For inexplicable reasons,
 in the new patch I have it before the XenD driver. 
 
 Daniel
 configure.in |   13 
 examples/domain-events/events-c/event-test.c |2 
 include/libvirt/virterror.h  |1 
 po/POTFILES.in   |1 
 python/libvir.c  |3 
 src/Makefile.am  |5 
 src/util.c   |4 
 src/virterror.c  |3 
 src/xen_inotify.c|  453 +++
 src/xen_inotify.h|   31 +
 src/xen_unified.c|  200 +++
 src/xen_unified.h|   61 +++
 src/xm_internal.c|  198 +++
 src/xm_internal.h|6 
 src/xs_internal.c|  431 +
 src/xs_internal.h|   51 +++
 16 files changed, 1393 insertions(+), 70 deletions(-)
--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH] fix python events

[David Lively]

On Fri, 2008-11-21 at 17:51 +0100, Jim Meyering wrote:
 No big deal, but there are several debug printf uses here that look
 like they try to print NULL pointers upon memory allocation failure.
 It's ok with glibc's printf of course, but not for others.

You're right.  Attached patch fixes those issues.  It also fixes some
cases in which I got some printf string operands switched around ...

I'm printing the (user-supplied) object names to help in debugging
misbehaving python EventImpls (since there's no static type checking to
catch these kinds of things).  So instead of printing NULL when we
can't alloc the name, I'm printing something a little more helpful (the
appropriate generic name).

Dave

 examples/domain-events/events-python/event-test.py |   29 +--
 python/libvir.c|  203 +
 python/libvir.py   |4 
 python/libvirt_wrap.h  |8 
 python/types.c |1 
 python/virConnect.py   |4 
 6 files changed, 197 insertions(+), 52 deletions(-)


diff --git a/examples/domain-events/events-python/event-test.py b/examples/domain-events/events-python/event-test.py
index 45aaa93..1ad436f 100644
--- a/examples/domain-events/events-python/event-test.py
+++ b/examples/domain-events/events-python/event-test.py
@@ -72,22 +72,22 @@ def myAddHandle(fd, events, cb, opaque):
 h_events = events
 h_cb = cb
 h_opaque = opaque
-
 mypoll.register(fd, myEventHandleTypeToPollEvent(events))
+return 0
 
 def myUpdateHandle(watch, event):
 global h_fd, h_events
-#print Updating Handle %s %s % (str(fd), str(events))
-h_fd = fd
+#print Updating Handle %s %s % (str(h_fd), str(event))
 h_events = event
-mypoll.unregister(watch)
-mypoll.register(watch, myEventHandleTypeToPollEvent(event))
+mypoll.unregister(h_fd)
+mypoll.register(h_fd, myEventHandleTypeToPollEvent(event))
 
 def myRemoveHandle(watch):
 global h_fd
-#print Removing Handle %s % str(fd)
+#print Removing Handle %s % str(h_fd)
+mypoll.unregister(h_fd)
 h_fd = 0
-mypoll.unregister(watch)
+return h_opaque
 
 def myAddTimeout(timeout, cb, opaque):
 global t_active, t_timeout, t_cb, t_opaque
@@ -96,16 +96,18 @@ def myAddTimeout(timeout, cb, opaque):
 t_timeout = timeout;
 t_cb = cb;
 t_opaque = opaque;
+return 0
 
 def myUpdateTimeout(timer, timeout):
 global t_timeout
-#print Updating Timeout %s % (str(timer), str(timeout))
+#print Updating Timeout %s %s % (str(timer), str(timeout))
 t_timeout = timeout;
 
 def myRemoveTimeout(timer):
 global t_active
 #print Removing Timeout %s % str(timer)
 t_active = 0;
+return t_opaque
 
 ##
 # Main
@@ -143,6 +145,14 @@ def main():
myRemoveTimeout );
 vc = libvirt.open(uri)
 
+# Close connection on exit (to test cleanup paths)
+old_exitfunc = getattr(sys, 'exitfunc', None)
+def exit():
+print Closing  + str(vc)
+vc.close()
+if (old_exitfunc): old_exitfunc()
+sys.exitfunc = exit
+
 #Add 2 callbacks to prove this works with more than just one
 vc.domainEventRegister(myDomainEventCallback1,None)
 vc.domainEventRegister(myDomainEventCallback2,None)
@@ -175,8 +185,7 @@ def main():
 
 if h_cb != None:
 #print Invoking Handle CB
-h_cb(0, h_fd, myPollEventToEventHandleType(revents  h_events),
- h_opaque[0], h_opaque[1])
+h_cb(0, h_fd, myPollEventToEventHandleType(revents  h_events), h_opaque[0], h_opaque[1])
 
 #print DEBUG EXIT
 #break
diff --git a/python/libvir.c b/python/libvir.c
index 7d58442..fed1031 100644
--- a/python/libvir.c
+++ b/python/libvir.c
@@ -35,6 +35,18 @@ extern void initcygvirtmod(void);
 #define VIR_PY_INT_FAIL (libvirt_intWrap(-1))
 #define VIR_PY_INT_SUCCESS (libvirt_intWrap(0))
 
+static char *py_str(PyObject *obj)
+{
+PyObject *str = PyObject_Str(obj);
+if (!str) {
+PyErr_Print();
+PyErr_Clear();
+return NULL;
+};
+return PyString_AsString(str);
+}
+
+
 /
  *	*
  *		Statistics		*
@@ -484,7 +496,8 @@ libvirt_virErrorFuncHandler(ATTRIBUTE_UNUSED void *ctx, virErrorPtr err)
 PyObject *result;
 
 #ifdef DEBUG_ERROR
-printf(libvirt_virErrorFuncHandler(%p, %s, ...) called\n, ctx, msg);
+printf(libvirt_virErrorFuncHandler(%p, %s, ...) called\n, ctx,
+   err-message);
 #endif
 
 if ((err == NULL) || (err-code == VIR_ERR_OK))
@@ -1780,12 +1793,19 @@ libvirt_virConnectDomainEventDeregister(ATTRIBUTE_UNUSED PyObject * self,
  * Event Impl
  ***/
 static PyObject *addHandleObj = NULL;
+static char *addHandleName= 

Re: [libvirt] [PATCH 1/2] Java bindings for domain events

[David Lively]

On Fri, 2008-11-21 at 14:49 +0100, Daniel Veillard wrote:
 On Wed, Nov 19, 2008 at 11:22:31AM -0500, David Lively wrote:
  On Wed, 2008-11-19 at 10:35 -0500, David Lively wrote:
   The patch already synchronizes operations using virConnect objects with
   each other.  To avoid making illegal EventImpl callbacks from Java for
   the current libvirt, I have to lock every Connect object known to Java
   and hold off creating new connections (via open  friends) around an
   EventImpl callback.  This sounds rather appalling to me, but it's
   starting to sound like the only practical route in the short term
   (unless it turns out we can rely on pthreads in WIN32 ...).
  

   note that libxml2 that we rely on has a fully ported mutex basic API
 in libxml/threads.h
 

 
  in case you really want to do the exclusive locking at the C level
 while still being portable.

Thanks for the pointer.  This could be used.  I'd assume we'd want to
change the mutex usage in datatypes.c as well, for consistency.

   Still it's probably better to try to implement most of this at the
 Java level, at least IMHO,

Just to be clear, I've been planning on keeping the per-Connect Java
synchronization from the original patch.  (And I think you guys have
bought into that much, so far.)

So here we're discussing the concurrency requirements imposed upon the
callbacks made by an asynchronous external (non-libvirtd) EventImpl.
Currently, we must make sure that no other connection is in use (and no
other EventImpl callback is in progress).

I'm arguing this is far too restrictive since it basically means locking
ALL connections before an event can be recognized.  So if one connection
is performing some long-running operation (say something
storage-related), no events can be delivered to *any* connection until
the operation completes (and then we'd better hope another long-running
operation hasn't been started on another connection in the mean time).

Keep in mind I'm not proposing we make all libvirt C public interfaces
threadsafe.  We only need to make sure the paths reachable from external
event impls (in the remote and xen-inotify drivers) are threadsafe.  I
believe the patch I submitted (2/2 in this series) does this for the
remote driver, albeit via the PTHREADS_MUTEX macros that currently have
no Win32 impl.  (And I'll do the same for the xen-inotify driver once
it's in.  I haven't looked into it yet ...)

I'll happily change the existing mutex usage in datatypes.c to the
libxml one.  And I'll use the same mutex impl in the remote and
xen-inotify drivers.  But I'd rather not bother until you guys agree
this is desirable ...

Thanks,
Dave

P.S. I realize this isn't really a practical problem right now.  It's
more of a future limit on scalability.  But because it's part of a new
interface (virEventRegisterImpl), it seems worth it to try and specify
this nicely now (as opposed to relaxing the concurrency requirements in
a later version, which is a kind of API change).


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

Re: [libvirt] [PATCH 1/2] Java bindings for domain events

[Daniel P. Berrange]

On Fri, Nov 21, 2008 at 03:27:49PM -0500, David Lively wrote:
 On Fri, 2008-11-21 at 14:49 +0100, Daniel Veillard wrote:
  On Wed, Nov 19, 2008 at 11:22:31AM -0500, David Lively wrote:
 
note that libxml2 that we rely on has a fully ported mutex basic API
  in libxml/threads.h
  
 
  
   in case you really want to do the exclusive locking at the C level
  while still being portable.
 
 Thanks for the pointer.  This could be used.  I'd assume we'd want to
 change the mutex usage in datatypes.c as well, for consistency.
 
Still it's probably better to try to implement most of this at the
  Java level, at least IMHO,
 
 Just to be clear, I've been planning on keeping the per-Connect Java
 synchronization from the original patch.  (And I think you guys have
 bought into that much, so far.)
 
 So here we're discussing the concurrency requirements imposed upon the
 callbacks made by an asynchronous external (non-libvirtd) EventImpl.
 Currently, we must make sure that no other connection is in use (and no
 other EventImpl callback is in progress).

To me this is a killer argument for virConnectPtr being made threadsafe.
We're creating problems for ourselves by trying to hack around it.

 I'm arguing this is far too restrictive since it basically means locking
 ALL connections before an event can be recognized.  So if one connection
 is performing some long-running operation (say something
 storage-related), no events can be delivered to *any* connection until
 the operation completes (and then we'd better hope another long-running
 operation hasn't been started on another connection in the mean time).
 
 Keep in mind I'm not proposing we make all libvirt C public interfaces
 threadsafe. 

I am :-) 

The common design pattern is java is heavily multi-threaded and 
doing a half-way house solution which makes only some scenrios in
the API thread-safe is still going to leave the Java libvirt API 
limited in comparison with native Java APIs  desirable application
use cases. 

We already have no choice but to make the QEMU, LXC, UML and OpenVZ
drivers fully threadsafe in order to allow the libvirtd daemon to be
threaded for sake of scability, and I have more or less completed
this work already. Making the remaining Xen driver  thread-safe too 
is trivial by comparison, since it has very little state. The main 
remaining issue is the global  per-connection error variables which
are not thread safe. If we added a 3rd thread-local variable which 
was set in parallel with these existing ones we could have a properly
threadsafe API, and simply say don't access the global error objects
if using the API in a multi-thread context.

This isn't just a problem for Java either - we jump through some 
horrible hoops in virt-manger python code because of the thread 
restrictions in the our public API.

The only alternative to a fully threaded API is a the idea of adding
a virConnectClone() operation which gives you a duplicate handled on
to the existing connection for use in a separate thread. The more I
consider this though, the less useful it seems - we'd still have the
same issues with the global error object, and we'd have todo internal
synchronization which is just as complex as the fully threadsafe
work. 

  We only need to make sure the paths reachable from external
 event impls (in the remote and xen-inotify drivers) are threadsafe.  I
 believe the patch I submitted (2/2 in this series) does this for the
 remote driver, albeit via the PTHREADS_MUTEX macros that currently have
 no Win32 impl.  (And I'll do the same for the xen-inotify driver once
 it's in.  I haven't looked into it yet ...)
 
 I'll happily change the existing mutex usage in datatypes.c to the
 libxml one.  And I'll use the same mutex impl in the remote and
 xen-inotify drivers.  But I'd rather not bother until you guys agree
 this is desirable ...

The datatypes.c code used to use the libxml mutex definitions but
I removed them because it was not a complete solution. We need to have
more than just the mutex datatypes  libxml does not provide wrappers
for all the thread APIs  data types we require, so we have no choice
but to use proper pthread types, and a pthreads API library on Win32)
Rich Jones has already packaged up such a library for Win32 in Fedora:

http://annexia.org/tmp/mingw/fedora-10/src/SRPMS/mingw32-pthreads-2.8.0-2.fc10.src.rpm

 P.S. I realize this isn't really a practical problem right now.  It's
 more of a future limit on scalability.  But because it's part of a new
 interface (virEventRegisterImpl), it seems worth it to try and specify
 this nicely now (as opposed to relaxing the concurrency requirements in
 a later version, which is a kind of API change).

Daniel
-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 

[libvirt] libvirt + xen 3.2.1 oddities

[Guido Günther]

Hi,
I just ran across these oddities when using a bit more libvirt+xen:

1.) virsh setmaxmem:

On a running domain:
# virsh setmaxmem domain 256000
completes but virsh dumpxml as well as the config.sxp still shows the
old amount of memory. Looks as the set_maxmem hypercall simply gets
ignored. xm mem-max works as expected. Smells like a bug in the ioctl?

2.) virsh list:

Sometimes (didn't find a pattern yet) when shutting down a running
domains and restarting it I'm seeing:

 Id Name State
--
  0 Domain-0 running
  2 foo idle
libvir: Xen Daemon error : GET operation failed: xend_get: error from xen 
daemon:
libvir: Xen Daemon error : GET operation failed: xend_get: error from xen 
daemon:
libvir: Xen Daemon error : GET operation failed: xend_get: error from xen 
daemon:
libvir: Xen Daemon error : GET operation failed: xend_get: error from xen 
daemon:
 7 bar  idle

Note that the number of errors the corresponds to the number of
shutdowns. VirXen_getdomaininfolist returns 7 in the above case.
virDomainLookupByID later on fails for these additional domains.

3.) virsh list: Duplicate domains:

 Id Name State
--
  0 Domain-0 running
libvir: Xen Daemon error : GET operation failed: xend_get: error from xen 
daemon:
libvir: Xen Daemon error : GET operation failed: xend_get: error from xen 
daemon:
libvir: Xen Daemon error : GET operation failed: xend_get: error from xen 
daemon:
 14 bar  no state
libvir: Xen Daemon error : GET operation failed: xend_get: error from xen 
daemon:
 16 bar  idle

Domain 14 can't be shut down (xm list only lists domain 16).

Could be a similar problem as the above.

This is all libvirt 0.4.6 (but the code looks very similar in gurrent
CVS) and xen-3.2.1 on Debian. The detected ABI is hypervisor call v2,
sys ver6 dom ver5. And from a quick glance at the libxen-dev package the
structs seem to match.
Cheers,
 -- Guido

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

Re: [libvirt] [PATCH] tests: virsh-all: new script

[Jim Meyering]

Daniel P. Berrange [EMAIL PROTECTED] wrote:
 On Fri, Nov 21, 2008 at 11:16:18AM +0100, Jim Meyering wrote:
 Actually, this was the first step.
 This tiny script merely invokes all commands blindly,
 solely to get test coverage, even if they (as most do) fail.

 It at least verifies that the commands don't crash or leak
 memory in the error paths :-)

 +
 +if test $VERBOSE = yes; then
 +  set -x
 +  virsh --version
 +fi
 +
 +. $srcdir/test-lib.sh

 $srcdir won't be set if you just invoke the test case directly
 as ./virsh-all. Can you add a default value assuming non-VPATH
 build, eg

test -z $srcdir  srcdir=`pwd`

Sure.  Done. and in virsh-synopsis.
I'll commit these tomorrow.

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

Re: [libvirt] [PATCH] fix python events

[Jim Meyering]

David Lively [EMAIL PROTECTED] wrote:
 On Fri, 2008-11-21 at 17:51 +0100, Jim Meyering wrote:
 No big deal, but there are several debug printf uses here that look
 like they try to print NULL pointers upon memory allocation failure.
 It's ok with glibc's printf of course, but not for others.

 You're right.  Attached patch fixes those issues.  It also fixes some
 cases in which I got some printf string operands switched around ...

 I'm printing the (user-supplied) object names to help in debugging
 misbehaving python EventImpls (since there's no static type checking to
 catch these kinds of things).  So instead of printing NULL when we
 can't alloc the name, I'm printing something a little more helpful (the
 appropriate generic name).

That's better, indeed.
I prefer your NAME macro, too.
ACK, modulo syntax:

 diff --git a/examples/domain-events/events-python/event-test.py 
 b/examples/domain-events/events-python/event-test.py
 @@ -175,8 +185,7 @@ def main():

  if h_cb != None:
  #print Invoking Handle CB
 -h_cb(0, h_fd, myPollEventToEventHandleType(revents  h_events),
 - h_opaque[0], h_opaque[1])
 +h_cb(0, h_fd, myPollEventToEventHandleType(revents  h_events), 
 h_opaque[0], h_opaque[1])

Reversed diff?  Better to split long lines,
not to join.

  #print DEBUG EXIT
  #break
 diff --git a/python/libvir.c b/python/libvir.c
 index 7d58442..fed1031 100644
 --- a/python/libvir.c
 +++ b/python/libvir.c
 @@ -35,6 +35,18 @@ extern void initcygvirtmod(void);
  #define VIR_PY_INT_FAIL (libvirt_intWrap(-1))
  #define VIR_PY_INT_SUCCESS (libvirt_intWrap(0))

 +static char *py_str(PyObject *obj)
 +{
 +PyObject *str = PyObject_Str(obj);
 +if (!str) {
 +PyErr_Print();
 +PyErr_Clear();
 +return NULL;
 +};
 +return PyString_AsString(str);
 +}
 +

Trailing white space ^^.

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