Re: [libvirt] qemu-kvm spending time in do_info_migrate() during virDomainSave(); 50ms polling too fast?

2010-05-04 Thread Daniel Veillard
On Mon, May 03, 2010 at 02:46:35PM -0500, Charles Duffy wrote:
 On Mon, May 3, 2010 at 1:48 PM, Matthias Bolte 
 matthias.bo...@googlemail.com wrote:
 
  2010/5/3 Charles Duffy char...@dyfis.net:
   The question then -- is the 50ms poll in
  qemuDomainWaitForMigrationComplete
   (called from qemudDomainSave) perhaps too frequent?
 
  What's the libvirt version?
 
  Maybe the Poll for migration end every 50ms instead of 50us [1]
  commit (part of 0.8.1) fixes your problem, if you're currently using
  libvirt  0.8.1.
 

  Gahh, I had forgotten about the patch

 Ahh; this predates 0.8, so that sounds to be precisely on-point.

  Actually the fix is in 0.8.1, the bug was added on
Feb 3 i.e. probably part of 0.7.6

Daniel

-- 
Daniel Veillard  | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
dan...@veillard.com  | 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] configure.ac: Avoid uname, which breaks cross-compilation

2010-05-04 Thread Daniel Veillard
On Tue, May 04, 2010 at 01:41:55AM +0200, Matthias Bolte wrote:
 When cross-compiling on Linux, configure will misdetect the target as
 Linux because it uses uname instead of relying on the $host variable.
 This results in including libvirt_linux.syms into libvirt.syms and
 therefore trying to export undefined symbols.
 
 Replace uname checks with $host checks to fix this.
 ---
  configure.ac |   18 ++
  1 files changed, 14 insertions(+), 4 deletions(-)
 
 diff --git a/configure.ac b/configure.ac
 index 6ee5b90..5d68dcc 100644
 --- a/configure.ac
 +++ b/configure.ac
 @@ -210,8 +210,11 @@ if test $prefix = /usr  test $sysconfdir = 
 '${prefix}/etc' ; then
  fi
  
  dnl lxc and qemu drivers require linux headers
 -if test `uname -s` != Linux
 -then
 +case $host in
 +  *-*-linux*)
 +# match linux here so the *) case will match anything non-linux
 +;;
 +  *)
  if test x$with_lxc != xyes
  then
  with_lxc=no
 @@ -220,7 +223,8 @@ then
  then
  with_qemu=no
  fi
 -fi
 +;;
 +esac
  
  dnl Allow to build without Xen, QEMU/KVM, test or remote driver
  AC_ARG_WITH([xen],
 @@ -1983,7 +1987,13 @@ then
  fi
  AM_CONDITIONAL([WITH_NODE_DEVICES], [test $with_nodedev = yes])
  
 -AM_CONDITIONAL([WITH_LINUX], [test `uname -s` = Linux])
 +with_linux=no
 +case $host in
 +  *-*-linux*)
 +with_linux=yes
 +;;
 +esac
 +AM_CONDITIONAL([WITH_LINUX], [test $with_linux = yes])
  
  
  AC_ARG_WITH([qemu-user],

  ACK, I looked at those recently but completely forgot we were
  cross-compiling for mingw !

Daniel

-- 
Daniel Veillard  | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
dan...@veillard.com  | 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


[libvirt] [PATCH] Various fixes for the spec file

2010-05-04 Thread Daniel Veillard
This includes various things:
 - fix the Requires: libvirt-client to use %{name} to allow easy renaming
 - when building ESX support one need libcurl-devel
 - remove Makefile[.in] from xml/nwfilter in the docs, as this breaks
   parallel install ation of i686 and x86_64 packages
 - don't include nwfilter config files if not building with the daemon

 all relatively trivial which is why I packed them together,

Daniel

diff --git a/libvirt.spec.in b/libvirt.spec.in
index 47e3050..9045c9a 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -184,7 +184,7 @@ URL: http://libvirt.org/
 BuildRequires: python-devel
 
 # The client side, i.e. shared libs and virsh are in a subpackage
-Requires: libvirt-client = %{version}-%{release}
+Requires: %{name}-client = %{version}-%{release}
 
 # Used by many of the drivers, so turn it on whenever the
 # daemon is present
@@ -347,6 +347,9 @@ BuildRequires: libssh2-devel
 %if %{with_netcf}
 BuildRequires: netcf-devel = 0.1.4
 %endif
+%if %{with_esx}
+BuildRequires: libcurl-devel
+%endif
 
 # Fedora build root suckage
 BuildRequires: gawk
@@ -378,7 +381,7 @@ virtualization capabilities of recent versions of Linux 
(and other OSes).
 %package devel
 Summary: Libraries, includes, etc. to compile with the libvirt library
 Group: Development/Libraries
-Requires: libvirt-client = %{version}-%{release}
+Requires: %{name}-client = %{version}-%{release}
 Requires: pkgconfig
 %if %{with_xen}
 Requires: xen-devel
@@ -392,7 +395,7 @@ the virtualization capabilities of recent versions of Linux 
(and other OSes).
 %package python
 Summary: Python bindings for the libvirt library
 Group: Development/Libraries
-Requires: libvirt-client = %{version}-%{release}
+Requires: %{name}-client = %{version}-%{release}
 
 %description python
 The libvirt-python package contains a module that permits applications
@@ -556,7 +559,7 @@ gzip -9 ChangeLog
 rm -fr %{buildroot}
 
 %makeinstall
-for i in domain-events/events-c dominfo domsuspend hellolibvirt python
+for i in domain-events/events-c dominfo domsuspend hellolibvirt python 
xml/nwfilter
 do
   (cd examples/$i ; make clean ; rm -rf .deps .libs Makefile Makefile.in)
 done
@@ -600,6 +603,10 @@ rm -rf 
$RPM_BUILD_ROOT%{_datadir}/doc/libvirt-python-%{version}
 rm -rf $RPM_BUILD_ROOT%{_datadir}/doc/libvirt-%{version}
 %endif
 
+%if ! %{with_libvirtd}
+rm -rf $RPM_BUILD_ROOT%{_sysconfdir}/libvirt/nwfilter
+%endif
+
 %if ! %{with_qemu}
 rm -rf $RPM_BUILD_ROOT%{_sysconfdir}/libvirt/qemu.conf
 rm -rf $RPM_BUILD_ROOT%{_sysconfdir}/logrotate.d/libvirtd.qemu

-- 
Daniel Veillard  | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
dan...@veillard.com  | 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 2/3] lxc: Make SetMemory work for active domains only

2010-05-04 Thread Jiri Denemark
  @@ -642,27 +642,30 @@ static int lxcDomainSetMemory(virDomainPtr dom, 
  unsigned long newmem) {
   goto cleanup;
   }
   
  -if (virDomainObjIsActive(vm)) {
  -if (driver-cgroup == NULL) {
  -lxcError(VIR_ERR_NO_SUPPORT,
  - %s, _(cgroups must be configured on the host));
  -goto cleanup;
  -}
  +if (!virDomainObjIsActive(vm)) {
  +lxcError(VIR_ERR_OPERATION_INVALID,
  + %s, _(Domain is not running));
  +goto cleanup;
  +}
   
  -if (virCgroupForDomain(driver-cgroup, vm-def-name, cgroup, 0) 
  != 0) {
  -lxcError(VIR_ERR_INTERNAL_ERROR,
  - _(Unable to get cgroup for %s), vm-def-name);
  -goto cleanup;
  -}
  +if (driver-cgroup == NULL) {
  +lxcError(VIR_ERR_NO_SUPPORT,
  + %s, _(cgroups must be configured on the host));
  +goto cleanup;
  +}
   
  -if (virCgroupSetMemory(cgroup, newmem)  0) {
  -lxcError(VIR_ERR_OPERATION_FAILED,
  - %s, _(Failed to set memory for domain));
  -goto cleanup;
  -}
  -} else {
  -vm-def-memory = newmem;
  +if (virCgroupForDomain(driver-cgroup, vm-def-name, cgroup, 0) != 
  0) {
  +lxcError(VIR_ERR_INTERNAL_ERROR,
  + _(Unable to get cgroup for %s), vm-def-name);
  +goto cleanup;
  +}
  +
  +if (virCgroupSetMemory(cgroup, newmem)  0) {
  +lxcError(VIR_ERR_OPERATION_FAILED,
  + %s, _(Failed to set memory for domain));
  +goto cleanup;
   }
  +
   ret = 0;
   
   cleanup:
 
 I'm not 100% sure of the patch but the new sequence look more logical,
 I'm still concerned that the new code seems to not update vm-def-memory

Hmm, the patch generated by git is a bit confusing. In reality it's quite
simple... Before the patch, there was a whole bunch of code within if
(virDomainObjIsActive(vm)) and vm-def-memory = newmem; if the VM wasn't
active. Now, the function works only for active VMs (which is it's correct
behavior as it was never supposed to work offline, one has to change domain
XML to make offline changes), that is vm-def-memory = newmem; is replaced
with VIR_ERR_OPERATION_INVALID error. There is no change in semantics for
active VMs.

Jirka

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


Re: [libvirt] [PATCH 2/3] lxc: Make SetMemory work for active domains only

2010-05-04 Thread Daniel Veillard
On Tue, May 04, 2010 at 11:56:56AM +0200, Jiri Denemark wrote:
   @@ -642,27 +642,30 @@ static int lxcDomainSetMemory(virDomainPtr dom, 
   unsigned long newmem) {
goto cleanup;
}

   -if (virDomainObjIsActive(vm)) {
   -if (driver-cgroup == NULL) {
   -lxcError(VIR_ERR_NO_SUPPORT,
   - %s, _(cgroups must be configured on the host));
   -goto cleanup;
   -}
   +if (!virDomainObjIsActive(vm)) {
   +lxcError(VIR_ERR_OPERATION_INVALID,
   + %s, _(Domain is not running));
   +goto cleanup;
   +}

   -if (virCgroupForDomain(driver-cgroup, vm-def-name, cgroup, 
   0) != 0) {
   -lxcError(VIR_ERR_INTERNAL_ERROR,
   - _(Unable to get cgroup for %s), vm-def-name);
   -goto cleanup;
   -}
   +if (driver-cgroup == NULL) {
   +lxcError(VIR_ERR_NO_SUPPORT,
   + %s, _(cgroups must be configured on the host));
   +goto cleanup;
   +}

   -if (virCgroupSetMemory(cgroup, newmem)  0) {
   -lxcError(VIR_ERR_OPERATION_FAILED,
   - %s, _(Failed to set memory for domain));
   -goto cleanup;
   -}
   -} else {
   -vm-def-memory = newmem;
   +if (virCgroupForDomain(driver-cgroup, vm-def-name, cgroup, 0) != 
   0) {
   +lxcError(VIR_ERR_INTERNAL_ERROR,
   + _(Unable to get cgroup for %s), vm-def-name);
   +goto cleanup;
   +}
   +
   +if (virCgroupSetMemory(cgroup, newmem)  0) {
   +lxcError(VIR_ERR_OPERATION_FAILED,
   + %s, _(Failed to set memory for domain));
   +goto cleanup;
}
   +
ret = 0;

cleanup:
  
  I'm not 100% sure of the patch but the new sequence look more logical,
  I'm still concerned that the new code seems to not update vm-def-memory
 
 Hmm, the patch generated by git is a bit confusing. In reality it's quite
 simple... Before the patch, there was a whole bunch of code within if
 (virDomainObjIsActive(vm)) and vm-def-memory = newmem; if the VM wasn't
 active. Now, the function works only for active VMs (which is it's correct
 behavior as it was never supposed to work offline, one has to change domain
 XML to make offline changes), that is vm-def-memory = newmem; is replaced
 with VIR_ERR_OPERATION_INVALID error. There is no change in semantics for
 active VMs.

  Ah, okay, I didn't realized the vm-def-memory = newmem was only for
non-running domains,

 ACK,
 
 thanks !

Daniel

-- 
Daniel Veillard  | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
dan...@veillard.com  | 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/3] lxc: Fix domain lookup and error handling

2010-05-04 Thread Jiri Denemark
 Jiri Denemark (3):
   lxc: Use virDomainFindByUUID for domain lookup
   lxc: Make SetMemory work for active domains only
   lxc: Check domain is active/inactive as required by operation
 
  src/lxc/lxc_driver.c |  138 
 ++
  1 files changed, 94 insertions(+), 44 deletions(-)

I pushed this series. Thanks for the reviews.

Jirka

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


Re: [libvirt] why we should use a mechanical indentation checker

2010-05-04 Thread Jim Meyering
Jim Meyering wrote:
 I introduced a bug with this supposedly-safe, no-semantic-change delta:

 
 http://libvirt.org/git/?p=libvirt.git;a=commitdiff;h=b6719eab9e95c5daeeea85

 See if you can spot it.

 Here is the loop in question, as it was prior to my erroneous change:

 for (i = 0; i  nruleInstances; i++)
 switch (inst[i]-ruleType) {
 case RT_EBTABLES:
 ebiptablesInstCommand(buf,
   inst[i]-commandTemplate,
   'A', -1, 1);
 break;
 case RT_IPTABLES:
 haveIptables = true;
 break;
 case RT_IP6TABLES:
 haveIp6tables = true;
 break;
 }

 Its formatting is WRONG.
...
 I am adding something like the following to coreutils' HACKING,
 and will add the equivalent (adjusting indentation/brace-positioning style
 in the examples) to libvirt's hacking.html.in, so if you object,
 speak up soon.

 While writing the above and below,
 I noticed that with the GNU formatting style,
 this is less of a problem, since there you do not add those
 oh-so-important-to-semantics curly braces at the *end* of a line
 where it easy to miss them, but at the beginning.  In addition,
 with the GNU style, adding the opening curly brace changes the
 indentation level of the body, while in libvirt's style, it does not.

 In either case, an automatic indentation checker would catch the problem.
 This is yet another reason to enforce an indentation style.
 The longer we wait, the harder it will become.

No one objected to the patch in the parent,
and there was one ACK, so I'm about to push this change
which has essentially the same content
(the formatting tweaks make html2text + massaging
produce something slightly closer to HACKING)

From 8cd233784c6f85b6de00d2229d3bf4c42f9940ed Mon Sep 17 00:00:00 2001
From: Jim Meyering meyer...@redhat.com
Date: Thu, 15 Apr 2010 19:31:04 +0200
Subject: [PATCH] docs: hacking: explain why using curly braces well is important

* docs/hacking.html.in: Use the curly braces section from coreutils'
HACKING, adapting for libvirt's different formatting style.
* HACKING: Sync from the above, still mostly manually.
---
 docs/hacking.html.in |  133 ++---
 1 files changed, 124 insertions(+), 9 deletions(-)

diff --git a/docs/hacking.html.in b/docs/hacking.html.in
index 03a1bee..c678fb3 100644
--- a/docs/hacking.html.in
+++ b/docs/hacking.html.in
@@ -11,16 +11,15 @@
 early and listen to feedback./li

   lipPost patches in unified diff format.  A command similar to this
-  should work:/p
+should work:/p
 pre
-  diff -urp libvirt.orig/ libvirt.modified/ gt; libvirt-myfeature.patch
-/pre
+  diff -urp libvirt.orig/ libvirt.modified/ gt; libvirt-myfeature.patch/pre
 p
   or:
 /p
 pre
-  git diff  libvirt-myfeature.patch
-/pre/li
+  git diff  libvirt-myfeature.patch/pre
+  /li
   liSplit large changes into a series of smaller patches, self-contained
 if possible, with an explanation of each patch and an explanation of 
how
 the sequence of patches fits together./li
@@ -29,16 +28,14 @@
   lipRun the automated tests on your code before submitting any 
changes.
   In particular, configure with compile warnings set to -Werror:/p
 pre
-  ./configure --enable-compile-warnings=error
-/pre
+  ./configure --enable-compile-warnings=error/pre
 p
   and run the tests:
 /p
 pre
   make check
   make syntax-check
-  make -C tests valgrind
-/pre
+  make -C tests valgrind/pre
 p
   The latter test checks for memory leaks.
 /p
@@ -60,6 +57,7 @@
pre
   ./qemuxml2xmltest/pre

+  /li
   liUpdate tests and/or documentation, particularly if you are adding
 a new feature or changing the output of a program./li
 /ol
@@ -124,6 +122,123 @@
 /p


+h2a name=curly_bracesCurly braces/a/h2
+
+p
+  Omit the curly braces around an if, while, for etc. body only
+  when that body occupies a single line.  In every other case we require
+  the braces.  This ensures that it is trivially easy to identify a
+  single-*statement* loop: each has only one *line* in its body.
+/p
+p
+  Omitting braces with a single-line body is fine:
+/p
+
+pre
+  while (expr) // one-line body - omitting curly braces is ok
+  single_line_stmt ();/pre
+
+p
+  However, the moment your loop/if/else body extends onto a second
+  line, for whatever reason (even if it's just an added comment), then
+  you should add braces.  Otherwise, it would be too easy to insert a
+  statement just before that comment (without adding braces), thinking
+  it is already a multi-statement loop:
+/p
+
+pre
+  while (true) // BAD! multi-line body with no braces
+  /* comment... */
+  single_line_stmt ();/pre
+

Re: [libvirt] why we should use a mechanical indentation checker

2010-05-04 Thread Jim Meyering
Jim Meyering wrote:
...
 In either case, an automatic indentation checker would catch the problem.
 This is yet another reason to enforce an indentation style.
 The longer we wait, the harder it will become.

 No one objected to the patch in the parent,
 and there was one ACK, so I'm about to push this change
 which has essentially the same content
 (the formatting tweaks make html2text + massaging
 produce something slightly closer to HACKING)

From 8cd233784c6f85b6de00d2229d3bf4c42f9940ed Mon Sep 17 00:00:00 2001
 From: Jim Meyering meyer...@redhat.com
 Date: Thu, 15 Apr 2010 19:31:04 +0200
 Subject: [PATCH] docs: hacking: explain why using curly braces well is 
 important

That was incomplete.  Didn't include the HACKING changes
and would provoke make syntax-check failure due to the
use of _(... making the po-check rule think hacking.html.in
should be listed in po/POTFILES.in.

Here's the patch that passes the tests:

From 44258473b852ef6b8d87ad43c706b1d4f697fe4b Mon Sep 17 00:00:00 2001
From: Jim Meyering meyer...@redhat.com
Date: Thu, 15 Apr 2010 19:31:04 +0200
Subject: [PATCH] docs: hacking: explain why using curly braces well is important

* docs/hacking.html.in: Use the curly braces section from coreutils'
HACKING, adapting for libvirt's different formatting style.
* HACKING: Sync from the above, still mostly manually.
---
 HACKING  |   91 ++
 docs/hacking.html.in |  133 ++---
 2 files changed, 215 insertions(+), 9 deletions(-)

diff --git a/HACKING b/HACKING
index 5486d8e..e22d73c 100644
--- a/HACKING
+++ b/HACKING
@@ -105,6 +105,97 @@ Usually they're in macro definitions or strings, and 
should be converted
 anyhow.


+Curly braces
+
+Omit the curly braces around an if, while, for etc. body only when that
+body occupies a single line. In every other case we require the braces. This
+ensures that it is trivially easy to identify a single-*statement* loop: each
+has only one *line* in its body.
+
+Omitting braces with a single-line body is fine:
+
+   while (expr) // one-line body - omitting curly braces is ok
+   single_line_stmt ();
+
+However, the moment your loop/if/else body extends onto a second line, for
+whatever reason (even if it's just an added comment), then you should add
+braces. Otherwise, it would be too easy to insert a statement just before that
+comment (without adding braces), thinking it is already a multi-statement
+loop:
+
+   while (true) // BAD! multi-line body with no braces
+   /* comment... */
+   single_line_stmt ();
+
+Do this instead:
+
+   while (true) { // Always put braces around a multi-line body.
+   /* comment... */
+   single_line_stmt ();
+   }
+
+There is one exception: when the second body line is not at the same
+indentation level as the first body line:
+
+   if (expr)
+   die (a diagnostic that would make this line
+ extend past the 80-column limit));
+
+It is safe to omit the braces in the code above, since the further-indented
+second body line makes it obvious that this is still a single-statement body.
+
+
+To reiterate, don't do this:
+
+   if (expr)// BAD: no braces around...
+   while (expr_2) { // ... a multi-line body
+   ...
+   }
+
+Do this, instead:
+
+   if (expr) {
+   while (expr_2) {
+   ...
+   }
+   }
+
+However, there is one exception in the other direction, when even a one-line
+block should have braces. That occurs when that one-line, brace-less block is
+an else block, and the corresponding then block *does* use braces. In that
+case, either put braces around the else block, or negate the if-condition
+and swap the bodies, putting the one-line block first and making the longer,
+multi-line block be the else block.
+
+   if (expr) {
+   ...
+   ...
+   }
+   else
+   x = y;// BAD: braceless else with braced then
+
+This is preferred, especially when the multi-line body is more than a few
+lines long, because it is easier to read and grasp the semantics of an if-
+then-else block when the simpler block occurs first, rather than after the
+more involved block:
+
+   if (!expr)
+ x = y; // putting the smaller block first is more readable
+   else {
+   ...
+   ...
+   }
+
+If you'd rather not negate the condition, then at least add braces:
+
+   if (expr) {
+   ...
+   ...
+   } else {
+   x = y;
+   }
+
+
 Preprocessor
 
 For variadic macros, stick with C99 syntax:
diff --git a/docs/hacking.html.in b/docs/hacking.html.in
index 03a1bee..deab530 100644
--- a/docs/hacking.html.in
+++ b/docs/hacking.html.in
@@ -11,16 +11,15 @@
 early and listen to feedback./li

   lipPost patches in unified diff format.  A command similar to this
-  should work:/p
+should work:/p
 pre
-  diff -urp libvirt.orig/ libvirt.modified/ gt; libvirt-myfeature.patch
-/pre
+  diff -urp libvirt.orig/ 

Re: [libvirt] [PATCH] Various fixes for the spec file

2010-05-04 Thread Eric Blake
On 05/04/2010 03:14 AM, Daniel Veillard wrote:
 This includes various things:
  - fix the Requires: libvirt-client to use %{name} to allow easy renaming
  - when building ESX support one need libcurl-devel
  - remove Makefile[.in] from xml/nwfilter in the docs, as this breaks
parallel install ation of i686 and x86_64 packages
  - don't include nwfilter config files if not building with the daemon
 
  all relatively trivial which is why I packed them together,

ACK.

-- 
Eric Blake   ebl...@redhat.com+1-801-349-2682
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] Don't wipe generated iface target in active domains

2010-05-04 Thread jdenemar
From: Jiri Denemark jdene...@redhat.com

Wipe generated interface target only when reading configuration of
inactive domains.
---
 src/conf/domain_conf.c |3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 546ddf2..3e45f79 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -1889,7 +1889,8 @@ virDomainNetDefParseXML(virCapsPtr caps,
xmlStrEqual(cur-name, BAD_CAST target)) {
 ifname = virXMLPropString(cur, dev);
 if ((ifname != NULL) 
-((STRPREFIX((const char*)ifname, vnet)) ||
+(((flags  VIR_DOMAIN_XML_INACTIVE) 
+  (STRPREFIX((const char*)ifname, vnet))) ||
  (!isValidIfname(ifname {
 /* An auto-generated target name, blank it out */
 /* blank out invalid interface names */
-- 
1.7.1

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


Re: [libvirt] [PATCH] Don't wipe generated iface target in active domains

2010-05-04 Thread Eric Blake
On 05/04/2010 08:39 AM, jdene...@redhat.com wrote:
 From: Jiri Denemark jdene...@redhat.com
 
 Wipe generated interface target only when reading configuration of
 inactive domains.
 ---
  src/conf/domain_conf.c |3 ++-
  1 files changed, 2 insertions(+), 1 deletions(-)
 
 diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
 index 546ddf2..3e45f79 100644
 --- a/src/conf/domain_conf.c
 +++ b/src/conf/domain_conf.c
 @@ -1889,7 +1889,8 @@ virDomainNetDefParseXML(virCapsPtr caps,
 xmlStrEqual(cur-name, BAD_CAST target)) {
  ifname = virXMLPropString(cur, dev);
  if ((ifname != NULL) 
 -((STRPREFIX((const char*)ifname, vnet)) ||
 +(((flags  VIR_DOMAIN_XML_INACTIVE) 
 +  (STRPREFIX((const char*)ifname, vnet))) ||
   (!isValidIfname(ifname {

ACK.

-- 
Eric Blake   ebl...@redhat.com+1-801-349-2682
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] Fix SCSI disk unplugging

2010-05-04 Thread Wolfgang Mauerer
Detaching disk devices is not only possible for VIR_DOMAIN_DISK_BUS_VIRTIO,
but also for VIR_DOMAIN_DISK_BUS_SCSI, so take care of this possibility.

Additionally, when the new-style device syntax is used, we do not need to
check if the PCI address is valid since we don't need it to do the
hot-unplugging. And while we're at it, drop the pci part
in qemudDomainDetachPciDiskDevice() -- it's misleading since we
do not necessarily have to deal with PCI addresses.

Signed-off-by: Wolfgang Mauerer wolfgang.maue...@siemens.com
---
 src/qemu/qemu_driver.c |   16 +---
 1 files changed, 9 insertions(+), 7 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 704f824..f2b8517 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -7862,10 +7862,10 @@ cleanup:
 }
 
 
-static int qemudDomainDetachPciDiskDevice(struct qemud_driver *driver,
-  virDomainObjPtr vm,
-  virDomainDeviceDefPtr dev,
-  unsigned long long qemuCmdFlags)
+static int qemudDomainDetachDiskDevice(struct qemud_driver *driver,
+   virDomainObjPtr vm,
+   virDomainDeviceDefPtr dev,
+   unsigned long long qemuCmdFlags)
 {
 int i, ret = -1;
 virDomainDiskDefPtr detach = NULL;
@@ -7884,7 +7884,8 @@ static int qemudDomainDetachPciDiskDevice(struct 
qemud_driver *driver,
 goto cleanup;
 }
 
-if (!virDomainDeviceAddressIsValid(detach-info,
+if (!(qemuCmdFlags  QEMUD_CMD_FLAG_DEVICE) 
+!virDomainDeviceAddressIsValid(detach-info,
VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI)) {
 qemuReportError(VIR_ERR_OPERATION_FAILED, %s,
 _(device cannot be detached without a PCI address));
@@ -8373,8 +8374,9 @@ static int qemudDomainDetachDevice(virDomainPtr dom,
 
 if (dev-type == VIR_DOMAIN_DEVICE_DISK 
 dev-data.disk-device == VIR_DOMAIN_DISK_DEVICE_DISK 
-dev-data.disk-bus == VIR_DOMAIN_DISK_BUS_VIRTIO) {
-ret = qemudDomainDetachPciDiskDevice(driver, vm, dev, qemuCmdFlags);
+(dev-data.disk-bus == VIR_DOMAIN_DISK_BUS_VIRTIO ||
+ dev-data.disk-bus == VIR_DOMAIN_DISK_BUS_SCSI)) {
+ret = qemudDomainDetachDiskDevice(driver, vm, dev, qemuCmdFlags);
 } else if (dev-type == VIR_DOMAIN_DEVICE_NET) {
 ret = qemudDomainDetachNetDevice(driver, vm, dev, qemuCmdFlags);
 } else if (dev-type == VIR_DOMAIN_DEVICE_CONTROLLER) {
-- 
1.6.4

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


Re: [libvirt] [PATCH] Fix SCSI disk unplugging

2010-05-04 Thread Daniel P. Berrange
On Tue, May 04, 2010 at 05:18:24PM +0200, Wolfgang Mauerer wrote:
 Detaching disk devices is not only possible for VIR_DOMAIN_DISK_BUS_VIRTIO,
 but also for VIR_DOMAIN_DISK_BUS_SCSI, so take care of this possibility.
 
 Additionally, when the new-style device syntax is used, we do not need to
 check if the PCI address is valid since we don't need it to do the
 hot-unplugging. And while we're at it, drop the pci part
 in qemudDomainDetachPciDiskDevice() -- it's misleading since we
 do not necessarily have to deal with PCI addresses.
 
 Signed-off-by: Wolfgang Mauerer wolfgang.maue...@siemens.com
 ---
  src/qemu/qemu_driver.c |   16 +---
  1 files changed, 9 insertions(+), 7 deletions(-)
 
 diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
 index 704f824..f2b8517 100644
 --- a/src/qemu/qemu_driver.c
 +++ b/src/qemu/qemu_driver.c
 @@ -7862,10 +7862,10 @@ cleanup:
  }
  
  
 -static int qemudDomainDetachPciDiskDevice(struct qemud_driver *driver,
 -  virDomainObjPtr vm,
 -  virDomainDeviceDefPtr dev,
 -  unsigned long long qemuCmdFlags)
 +static int qemudDomainDetachDiskDevice(struct qemud_driver *driver,
 +   virDomainObjPtr vm,
 +   virDomainDeviceDefPtr dev,
 +   unsigned long long qemuCmdFlags)

I'd rather we introduced a separate method

   qemudDomainDetachSCSIDiskDevice

since logically these are different types of objects/operations, that just
happen to share a common monitor command with new QEMU. It also needs to
give a more explicit error in the case where QEMUD_CMD_FLAG_DEVICE is not
set for SCSI, since we can't support that for SCSI, but can for VirtIO.

  {
  int i, ret = -1;
  virDomainDiskDefPtr detach = NULL;
 @@ -7884,7 +7884,8 @@ static int qemudDomainDetachPciDiskDevice(struct 
 qemud_driver *driver,
  goto cleanup;
  }
  
 -if (!virDomainDeviceAddressIsValid(detach-info,
 +if (!(qemuCmdFlags  QEMUD_CMD_FLAG_DEVICE) 
 +!virDomainDeviceAddressIsValid(detach-info,
 VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI)) {
  qemuReportError(VIR_ERR_OPERATION_FAILED, %s,
  _(device cannot be detached without a PCI 
 address));
 @@ -8373,8 +8374,9 @@ static int qemudDomainDetachDevice(virDomainPtr dom,
  
  if (dev-type == VIR_DOMAIN_DEVICE_DISK 
  dev-data.disk-device == VIR_DOMAIN_DISK_DEVICE_DISK 
 -dev-data.disk-bus == VIR_DOMAIN_DISK_BUS_VIRTIO) {
 -ret = qemudDomainDetachPciDiskDevice(driver, vm, dev, qemuCmdFlags);
 +(dev-data.disk-bus == VIR_DOMAIN_DISK_BUS_VIRTIO ||
 + dev-data.disk-bus == VIR_DOMAIN_DISK_BUS_SCSI)) {
 +ret = qemudDomainDetachDiskDevice(driver, vm, dev, qemuCmdFlags);
  } else if (dev-type == VIR_DOMAIN_DEVICE_NET) {
  ret = qemudDomainDetachNetDevice(driver, vm, dev, qemuCmdFlags);
  } else if (dev-type == VIR_DOMAIN_DEVICE_CONTROLLER) {


Regards,
Daniel
-- 
|: Red Hat, Engineering, London-o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.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] Don't wipe generated iface target in active domains

2010-05-04 Thread Jiri Denemark
  Wipe generated interface target only when reading configuration of
  inactive domains.
 
 ACK.

Thanks, pushed.

Jirka

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


[libvirt] Fwd: Re: need your help about virito vmchannel

2010-05-04 Thread Matthew Booth
FYI for the list. I haven't looked at this yet.

Matt

 Original Message 
Subject: Re: need your help about virito vmchannel
Date: Tue, 4 May 2010 18:00:02 +0530
From: Amit Shah amit.s...@redhat.com
To: Matthew Booth mbo...@redhat.com
CC: Jianlin Liu jia...@redhat.com

On (Tue) May 04 2010 [13:18:30], Matthew Booth wrote:
 On 04/05/10 12:48, Jianlin Liu wrote:
  Hi Matthew,
  
 I want to create a virtio vm channel in my guest. So I add the 
  followiong to my guest xml file:
  channel type='pty'
target type='virtio' name='org.linux-kvm.port.0'/
  /channel
 
 Jianlin,
 
 While I wrote the libvirt-vmchannel glue, I'm not actually that
 familiar with vmchannel itself. I've cc'd Amit, who hopefully might
 recognise this problem and tell me how to fix it ;)
 
 Amit,
 
 Any idea what's going on here?

Yes:

  Then I try to start the guest:
  # virsh start winxp
  error: Failed to start domain winxp
  error: internal error Process exited while reading console log output: char 
  device redirected to /dev/pts/4
  char device redirected to /dev/pts/5
  qemu-kvm: -device 
  virtio-serial-pci,id=virtio-serial0,max_ports=0,vectors=0,bus=pci.0,addr=0x5:
   Device 'virtio-serial-pci' could not be initialized

With max_ports=0, the virtio-serial device doesn't get created and hence
this error.

Amit

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


Re: [libvirt] [PATCH] configure.ac: Avoid uname, which breaks cross-compilation

2010-05-04 Thread Eric Blake
On 05/04/2010 02:54 AM, Daniel Veillard wrote:
 On Tue, May 04, 2010 at 01:41:55AM +0200, Matthias Bolte wrote:
 When cross-compiling on Linux, configure will misdetect the target as
 Linux because it uses uname instead of relying on the $host variable.
 This results in including libvirt_linux.syms into libvirt.syms and
 therefore trying to export undefined symbols.

 Replace uname checks with $host checks to fix this.
 
   ACK, I looked at those recently but completely forgot we were
   cross-compiling for mingw !

I've gotten a lot further with an in-tree run of ./autobuild.sh with
this patch: the cross-build to mingw passed; now just creation of
mingw32-libvirt rpm fails:

Checking for unpackaged file(s): /usr/lib/rpm/check-files
/home/eblake/rpmbuild/BUILDROOT/mingw32-libvirt-0.8.1-3.fc12.eblake1272991027.i386
error: Installed (but unpackaged) file(s) found:
   /usr/i686-pc-mingw32/sys-root/mingw/share/libvirt/cpu_map.xml
   /usr/i686-pc-mingw32/sys-root/mingw/share/libvirt/schemas/nwfilter.rng

Since this has been acked, and autobuild.sh is a prereq to some of my
outstanding patches, I've gone ahead and pushed it.

-- 
Eric Blake   ebl...@redhat.com+1-801-349-2682
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] Fwd: Re: need your help about virito vmchannel

2010-05-04 Thread Cole Robinson
On 05/04/2010 12:38 PM, Matthew Booth wrote:
 FYI for the list. I haven't looked at this yet.
 
 Matt
 
  Original Message 
 Subject: Re: need your help about virito vmchannel
 Date: Tue, 4 May 2010 18:00:02 +0530
 From: Amit Shah amit.s...@redhat.com
 To: Matthew Booth mbo...@redhat.com
 CC: Jianlin Liu jia...@redhat.com
 
 On (Tue) May 04 2010 [13:18:30], Matthew Booth wrote:
 On 04/05/10 12:48, Jianlin Liu wrote:
 Hi Matthew,

I want to create a virtio vm channel in my guest. So I add the 
 followiong to my guest xml file:
 channel type='pty'
   target type='virtio' name='org.linux-kvm.port.0'/
 /channel

 Jianlin,

 While I wrote the libvirt-vmchannel glue, I'm not actually that
 familiar with vmchannel itself. I've cc'd Amit, who hopefully might
 recognise this problem and tell me how to fix it ;)

 Amit,

 Any idea what's going on here?
 
 Yes:
 
 Then I try to start the guest:
 # virsh start winxp
 error: Failed to start domain winxp
 error: internal error Process exited while reading console log output: char 
 device redirected to /dev/pts/4
 char device redirected to /dev/pts/5
 qemu-kvm: -device 
 virtio-serial-pci,id=virtio-serial0,max_ports=0,vectors=0,bus=pci.0,addr=0x5:
  Device 'virtio-serial-pci' could not be initialized
 
 With max_ports=0, the virtio-serial device doesn't get created and hence
 this error.
 
   Amit
 

I saw this issue as well, when I started adding channel support to
virt-install (since stalled, but mostly complete). Adding a channel
device creates an implicit virtio-serial controller which defaults to
ports = 0.

Since ports = 0 is useless, maybe we should just defer to the qemu
default for that case (which is 31 according to
http://fedoraproject.org/wiki/Features/VirtioSerial ). We should
probably find a way to do the same thing for the 'vectors' value as
well, although 0 is valid value there.

- Cole

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


Re: [libvirt] Fwd: Re: need your help about virito vmchannel

2010-05-04 Thread Daniel P. Berrange
On Tue, May 04, 2010 at 01:08:53PM -0400, Cole Robinson wrote:
 On 05/04/2010 12:38 PM, Matthew Booth wrote:
  FYI for the list. I haven't looked at this yet.
  
  Matt
  
   Original Message 
  Subject: Re: need your help about virito vmchannel
  Date: Tue, 4 May 2010 18:00:02 +0530
  From: Amit Shah amit.s...@redhat.com
  To: Matthew Booth mbo...@redhat.com
  CC: Jianlin Liu jia...@redhat.com
  
  On (Tue) May 04 2010 [13:18:30], Matthew Booth wrote:
  On 04/05/10 12:48, Jianlin Liu wrote:
  Hi Matthew,
 
 I want to create a virtio vm channel in my guest. So I add the 
  followiong to my guest xml file:
  channel type='pty'
target type='virtio' name='org.linux-kvm.port.0'/
  /channel
 
  Jianlin,
 
  While I wrote the libvirt-vmchannel glue, I'm not actually that
  familiar with vmchannel itself. I've cc'd Amit, who hopefully might
  recognise this problem and tell me how to fix it ;)
 
  Amit,
 
  Any idea what's going on here?
  
  Yes:
  
  Then I try to start the guest:
  # virsh start winxp
  error: Failed to start domain winxp
  error: internal error Process exited while reading console log output: 
  char device redirected to /dev/pts/4
  char device redirected to /dev/pts/5
  qemu-kvm: -device 
  virtio-serial-pci,id=virtio-serial0,max_ports=0,vectors=0,bus=pci.0,addr=0x5:
   Device 'virtio-serial-pci' could not be initialized
  
  With max_ports=0, the virtio-serial device doesn't get created and hence
  this error.
  
  Amit
  
 
 I saw this issue as well, when I started adding channel support to
 virt-install (since stalled, but mostly complete). Adding a channel
 device creates an implicit virtio-serial controller which defaults to
 ports = 0.
 
 Since ports = 0 is useless, maybe we should just defer to the qemu
 default for that case (which is 31 according to


Yep, if we're automatically creating  a virtio controller, we should just
let QEMU choose the default for it, or make libvirt use the QEMU default


Daniel
-- 
|: Red Hat, Engineering, London-o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.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] mingw32-libvirt.spec: bring up to date

2010-05-04 Thread Eric Blake
Right now, mingw32-portablexdr is not available in Fedora, but is
present in fedora-mingw.git.  With that package, plus
redhat-rpm-config and this patch, it is once again possible to build
mingw32-libvirt from a Fedora 12 host.

* mingw32-libvirt.spec.in (__debug_install_post): Override.
(%files): Mention recent additions.
---

The __debug_install_post trick is something that Richard Jones
taught me while figuring out how to get mingw32-portablexdr.
Also, during that effort, we discovered this bug in rpm:
https://bugzilla.redhat.com/show_bug.cgi?id=587818
you have to have redhat-rpm-config installed to work around it.

 mingw32-libvirt.spec.in |5 +
 1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/mingw32-libvirt.spec.in b/mingw32-libvirt.spec.in
index 867a849..fc90203 100644
--- a/mingw32-libvirt.spec.in
+++ b/mingw32-libvirt.spec.in
@@ -3,6 +3,7 @@
 %define _use_internal_dependency_generator 0
 %define __find_requires %{_mingw32_findrequires}
 %define __find_provides %{_mingw32_findprovides}
+%define __debug_install_post %{_mingw32_debug_install_post}

 Name:   mingw32-libvirt
 Version:@VERSION@
@@ -30,6 +31,7 @@ BuildRequires:  gettext

 BuildArch:  noarch

+%{?_mingw32_debug_package}

 %description
 MinGW Windows libvirt virtualization library.
@@ -95,9 +97,12 @@ rm -rf $RPM_BUILD_ROOT
 %{_mingw32_datadir}/libvirt/schemas/nodedev.rng
 %{_mingw32_datadir}/libvirt/schemas/capability.rng
 %{_mingw32_datadir}/libvirt/schemas/interface.rng
+%{_mingw32_datadir}/libvirt/schemas/nwfilter.rng
 %{_mingw32_datadir}/libvirt/schemas/secret.rng
 %{_mingw32_datadir}/libvirt/schemas/storageencryption.rng

+%{_mingw32_datadir}/libvirt/cpu_map.xml
+
 %{_mingw32_datadir}/locale/*/LC_MESSAGES/libvirt.mo

 %dir %{_mingw32_includedir}/libvirt
-- 
1.6.6.1

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


Re: [libvirt] Alternative to XML Creation and Parsing?

2010-05-04 Thread Ganesh Pagade
Resending, as the previous one hasn't appeared after waiting for more than a
day. My apologies if this is duplicate.

Hello,

We plan to develop a fancy GUI which would help creating and managing
VMs/Domains for RHEL 5.4 KVM.

However looking at the schemas provided in docs/schemas/ (Ex:
domain.rng) generating
the XML file from the inputs taken from the GUI and pass it on to
virDomainDefineXML() turns out to be a huge tasks in itself. I was wondering
if there is an better way for me to provide my configurations to the library
and also to read data from it (instead of creating, parsing XMLs)? If an
alternate approach exist, are there any limitations with it?

Also the schema given in docs/schemas/ would be generic for all hyper visor.
How can I identify the tags applicable only for a particular hyper visor?

I found:
virConnectDomainXMLFromNative(virConnectPtr conn, const char * nativeFormat,
const char * nativeConfig, unsigned int flags)

in API reference, which could solve my first problem but couldn't figure out
what nativeFormat would be.

Any inputs/pointers would be highly appreciated.

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

Re: [libvirt] [PATCH] qemu: Report cmdline output if VM dies early

2010-05-04 Thread Eric Blake
On 04/30/2010 09:44 AM, Cole Robinson wrote:
 qemuReadLogOutput early VM death detection is racy and won't always work.
 Startup then errors when connecting to the VM monitor. This won't report
 the emulator cmdline output which is typically the most useful diagnostic.
 
 Check if the VM has died at the very end of the monitor connection step,
 and if so, report the cmdline output.

 +static void
 +qemuReadLogFD(int logfd, char *buf, int maxlen, int off)
 +{
 +int ret;
 +char *tmpbuf = buf+off;

Isn't the style to separate operators by space?  'buf + off'

 +
 +ret = saferead(logfd, tmpbuf, maxlen-off-1);

Likewise, 'maxlen - off - 1'.

 +if (ret  0) {
 +ret = 0;
 +}
 +
 +*(tmpbuf+ret) = '\0';

Stylistically, I like 'tmpbuf[ret]' better than '*(tmpbuf+ret)'.

 +}
 +
  static int
  qemudWaitForMonitor(struct qemud_driver* driver,
  virDomainObjPtr vm, off_t pos)
  {
 -char buf[4096]; /* Plenty of space to get startup greeting */
 +char buf[4096] = \0; /* Plenty of space to get startup greeting */

 is sufficient; you don't have to use \0 to zero-initialize a
statically sized char[].

But overall, the patch looks sane; I didn't see any logic errors.
ACK with the style nits addressed.

-- 
Eric Blake   ebl...@redhat.com+1-801-349-2682
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] pci: Give an explicit error if device not found

2010-05-04 Thread Eric Blake
On 04/30/2010 09:44 AM, Cole Robinson wrote:
 @@ -1028,6 +1028,7 @@ pciGetDevice(unsigned domain,
   unsigned function)
  {
  pciDevice *dev;
 +char devdir[PATH_MAX];

Using PATH_MAX as an array size is dangerous; it fails on GNU Hurd where
there is no minimum size.  Also, it wastes a lot of space, given your
usage...

  char *vendor, *product;
  
  if (VIR_ALLOC(dev)  0) {
 @@ -1043,8 +1044,17 @@ pciGetDevice(unsigned domain,
  
  snprintf(dev-name, sizeof(dev-name), %.4x:%.2x:%.2x.%.1x,
   dev-domain, dev-bus, dev-slot, dev-function);
 +snprintf(devdir, sizeof(devdir),
 + PCI_SYSFS devices/%s, dev-name);

...here, of concatenating two relatively short strings.  I'd almost
rather see a virAsprintf/free.

  snprintf(dev-path, sizeof(dev-path),
 - PCI_SYSFS devices/%s/config, dev-name);
 + %s/%s/config, devdir, dev-name);
 +
 +if (access(devdir, X_OK) != 0) {

Is this ever run in a situation where the effective id might not equal
the user id (that is, as a setuid script, or as root)?  If so, would it
be better to use faccessat(...,AT_EACCESS) instead of access() to be
querying the correct permissions?  (Gnulib provides faccessat for all
platforms).

-- 
Eric Blake   ebl...@redhat.com+1-801-349-2682
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] [PATCHv2 0/5] build: update gnulib

2010-05-04 Thread Eric Blake
This replaces the first cut at this series:
https://www.redhat.com/archives/libvir-list/2010-April/msg01341.html

Changes since v1:
squash the original 1/5 and 2/5 into one patch
test every stage of the series on a cross-compile to mingw
add new 5/5, and adjust to recent syntax checks available in today's gnulib

 [PATCHv2 1/5] build: rely on gnulib's pthread module
 [PATCHv2 2/5] build: use gnulib's uname
 [PATCHv2 3/5] build: use gnulib's sys/wait.h
 [PATCHv2 4/5] build: drop more redundant configure checks
 [PATCHv2 5/5] build: update gnulib

Because of the new patch, I'll wait for a re-ACK before pushing; but 1
through 4 are pretty much unchanged from their original ACK.

 .gnulib |2 +-
 .x-sc_prohibit_always_true_header_tests |4 
 bootstrap.conf  |3 +++
 configure.ac|   22 --
 src/libvirt.c   |4 +---
 src/network/bridge_driver.c |1 -
 src/nodeinfo.c  |   12 ++--
 src/openvz/openvz_conf.c|2 +-
 src/openvz/openvz_driver.c  |2 +-
 src/phyp/phyp_driver.c  |1 -
 src/qemu/qemu_driver.c  |1 -
 src/remote/remote_driver.c  |5 +
 src/storage/storage_backend.c   |4 +---
 src/uml/uml_driver.c|1 -
 src/util/ebtables.c |7 ++-
 src/util/hooks.c|4 +---
 src/util/iptables.c |7 ++-
 src/util/processinfo.c  |6 ++
 src/util/util.c |4 +---
 19 files changed, 27 insertions(+), 65 deletions(-)

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


[libvirt] [PATCHv2 1/5] build: rely on gnulib's pthread module

2010-05-04 Thread Eric Blake
Gnulib can guarantee that pthread.h exists, but for now, it is a dummy
header with no support for most pthread_* functions.  Modify our
use of pthread to use function checks, rather than header checks,
to determine how much pthread support is present.

* bootstrap.conf (gnulib_modules): Add pthread.
* configure.ac: Drop all pthread.h checks.  Optimize function
checks.  Add check for pthread functions.
* src/Makefile.am (libvirt_lxc_LDADD): Ensure proper link.
* src/remote/remote_driver.c (remoteIOEventLoop): Depend on
pthread_sigmask, now that gnulib guarantees pthread.h.
* src/util/util.c (virFork): Likewise.
* src/util/threads.c (threads-pthread.c): Depend on
pthread_mutexattr_init, as a witness of full pthread support.
* src/util/threads.h (threads-pthread.h): Likewise.
---
 bootstrap.conf |1 +
 configure.ac   |   21 +++--
 src/Makefile.am|3 ++-
 src/remote/remote_driver.c |6 +++---
 src/util/threads.c |4 ++--
 src/util/threads.h |4 ++--
 src/util/util.c|   10 +-
 7 files changed, 26 insertions(+), 23 deletions(-)

diff --git a/bootstrap.conf b/bootstrap.conf
index 785489b..da7cc9c 100644
--- a/bootstrap.conf
+++ b/bootstrap.conf
@@ -43,6 +43,7 @@ perror
 physmem
 poll
 posix-shell
+pthread
 recv
 random_r
 send
diff --git a/configure.ac b/configure.ac
index 5d68dcc..93eb654 100644
--- a/configure.ac
+++ b/configure.ac
@@ -106,10 +106,19 @@ dnl Use --disable-largefile if you don't want this.
 AC_SYS_LARGEFILE

 dnl Availability of various common functions (non-fatal if missing).
-AC_CHECK_FUNCS([cfmakeraw regexec uname sched_getaffinity getuid getgid 
posix_fallocate mmap])
+AC_CHECK_FUNCS_ONCE([cfmakeraw regexec uname sched_getaffinity getuid getgid \
+ posix_fallocate mmap])

 dnl Availability of various not common threadsafe functions
-AC_CHECK_FUNCS([strerror_r strtok_r getmntent_r getgrnam_r getpwuid_r])
+AC_CHECK_FUNCS_ONCE([strerror_r strtok_r getmntent_r getgrnam_r getpwuid_r])
+
+dnl Availability of pthread functions (if missing, win32 threading is
+dnl assumed).  Because of $LIB_PTHREAD, we cannot use AC_CHECK_FUNCS_ONCE.
+dnl LIB_PTHREAD was set during gl_INIT by gnulib.
+old_LIBS=$LIBS
+LIBS=$LIBS $LIB_PTHREAD
+AC_CHECK_FUNCS([pthread_sigmask pthread_mutexattr_init])
+LIBS=$old_libs

 dnl Availability of various common headers (non-fatal if missing).
 AC_CHECK_HEADERS([pwd.h paths.h regex.h sys/syslimits.h sys/utsname.h \
@@ -137,14 +146,6 @@ AM_CONDITIONAL([HAVE_GLIBC_RPCGEN],
   [test x$ac_cv_path_RPCGEN != xno 
$ac_cv_path_RPCGEN -t /dev/null /dev/null 21])

-dnl pthread?
-AC_CHECK_HEADER([pthread.h],
-   [AC_CHECK_LIB([pthread],[pthread_join],[
-   AC_DEFINE([HAVE_LIBPTHREAD],[],[Define if pthread (-lpthread)])
-   AC_DEFINE([HAVE_PTHREAD_H],[],[Define if pthread.h])
-   LIBS=-lpthread $LIBS
-   ])])
-
 dnl Miscellaneous external programs.
 AC_PATH_PROG([RM], [rm], [/bin/rm])
 AC_PATH_PROG([MV], [mv], [/bin/mv])
diff --git a/src/Makefile.am b/src/Makefile.am
index 2531ac5..dc76d03 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -984,7 +984,8 @@ libvirt_lxc_SOURCES =   
\
$(CPU_CONF_SOURCES) \
$(NWFILTER_PARAM_CONF_SOURCES)
 libvirt_lxc_LDFLAGS = $(WARN_CFLAGS) $(COVERAGE_LDCFLAGS) $(CAPNG_LIBS) 
$(YAJL_LIBS)
-libvirt_lxc_LDADD = $(LIBXML_LIBS) $(NUMACTL_LIBS) ../gnulib/lib/libgnu.la
+libvirt_lxc_LDADD = $(LIBXML_LIBS) $(NUMACTL_LIBS) $(LIB_PTHREAD) \
+   ../gnulib/lib/libgnu.la
 libvirt_lxc_CFLAGS =   \
$(LIBPARTED_CFLAGS) \
$(NUMACTL_CFLAGS)   \
diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
index 317125f..bf53da4 100644
--- a/src/remote/remote_driver.c
+++ b/src/remote/remote_driver.c
@@ -9556,7 +9556,7 @@ remoteIOEventLoop(virConnectPtr conn,
 struct remote_thread_call *tmp = priv-waitDispatch;
 struct remote_thread_call *prev;
 char ignore;
-#ifdef HAVE_PTHREAD_H
+#ifdef HAVE_PTHREAD_SIGMASK
 sigset_t oldmask, blockedsigs;
 #endif

@@ -9585,7 +9585,7 @@ remoteIOEventLoop(virConnectPtr conn,
  * after the call (RHBZ#567931).  Same for SIGCHLD and SIGPIPE
  * at the suggestion of Paolo Bonzini and Daniel Berrange.
  */
-#ifdef HAVE_PTHREAD_H
+#ifdef HAVE_PTHREAD_SIGMASK
 sigemptyset (blockedsigs);
 sigaddset (blockedsigs, SIGWINCH);
 sigaddset (blockedsigs, SIGCHLD);
@@ -9598,7 +9598,7 @@ remoteIOEventLoop(virConnectPtr conn,
 if (ret  0  errno == EAGAIN)
 goto repoll;

-#ifdef HAVE_PTHREAD_H
+#ifdef HAVE_PTHREAD_SIGMASK
 ignore_value(pthread_sigmask(SIG_SETMASK, oldmask, NULL));
 #endif

diff --git a/src/util/threads.c b/src/util/threads.c
index 18f8b64..8c0a91a 100644
--- 

[libvirt] [PATCHv2 3/5] build: use gnulib's sys/wait.h

2010-05-04 Thread Eric Blake
* configure.ac: Drop sys/wait.h check.
* src/libvirt.c (includes): Use header unconditionally.
* src/remote/remote_driver.c (includes): Likewise.
* src/storage/storage_backend.c (includes): Likewise.
* src/util/ebtables.c (includes): Likewise.
* src/util/hooks.c (includes): Likewise.
* src/util/iptables.c (includes): Likewise.
* src/util/util.c (includes): Likewise.
---
 bootstrap.conf|1 +
 configure.ac  |2 +-
 src/libvirt.c |4 +---
 src/remote/remote_driver.c|5 +
 src/storage/storage_backend.c |4 +---
 src/util/ebtables.c   |7 ++-
 src/util/hooks.c  |4 +---
 src/util/iptables.c   |7 ++-
 src/util/util.c   |4 +---
 9 files changed, 11 insertions(+), 27 deletions(-)

diff --git a/bootstrap.conf b/bootstrap.conf
index e85f869..baf0bc2 100644
--- a/bootstrap.conf
+++ b/bootstrap.conf
@@ -57,6 +57,7 @@ strptime
 strsep
 strtok_r
 sys_stat
+sys_wait
 time_r
 timegm
 uname
diff --git a/configure.ac b/configure.ac
index 18da606..12646e0 100644
--- a/configure.ac
+++ b/configure.ac
@@ -122,7 +122,7 @@ LIBS=$old_libs

 dnl Availability of various common headers (non-fatal if missing).
 AC_CHECK_HEADERS([pwd.h paths.h regex.h sys/syslimits.h \
-  sys/wait.h sched.h termios.h sys/poll.h syslog.h mntent.h net/ethernet.h])
+  sched.h termios.h sys/poll.h syslog.h mntent.h net/ethernet.h])

 dnl Where are the XDR functions?
 dnl If portablexdr is installed, prefer that.
diff --git a/src/libvirt.c b/src/libvirt.c
index 028115c..eb05337 100644
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -18,9 +18,7 @@
 #include sys/stat.h
 #include unistd.h
 #include assert.h
-#ifdef HAVE_SYS_WAIT_H
-# include sys/wait.h
-#endif
+#include sys/wait.h
 #include time.h
 #include gcrypt.h

diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
index bf53da4..72cf292 100644
--- a/src/remote/remote_driver.c
+++ b/src/remote/remote_driver.c
@@ -33,6 +33,7 @@
 #include sys/stat.h
 #include fcntl.h
 #include arpa/inet.h
+#include sys/wait.h

 /* Windows socket compatibility functions. */
 #include errno.h
@@ -45,10 +46,6 @@
 # include netinet/tcp.h
 #endif

-#ifdef HAVE_SYS_WAIT_H
-# include sys/wait.h
-#endif
-
 #ifdef HAVE_PWD_H
 # include pwd.h
 #endif
diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c
index be87a81..7df61cd 100644
--- a/src/storage/storage_backend.c
+++ b/src/storage/storage_backend.c
@@ -29,9 +29,7 @@
 # include regex.h
 #endif
 #include sys/types.h
-#if HAVE_SYS_WAIT_H
-# include sys/wait.h
-#endif
+#include sys/wait.h
 #include unistd.h
 #include fcntl.h
 #include stdint.h
diff --git a/src/util/ebtables.c b/src/util/ebtables.c
index a6afdf8..e2b9608 100644
--- a/src/util/ebtables.c
+++ b/src/util/ebtables.c
@@ -1,6 +1,6 @@
 /*
- * Copyright (C) 2009 IBM Corp.
  * Copyright (C) 2007-2010 Red Hat, Inc.
+ * Copyright (C) 2009 IBM Corp.
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Lesser General Public
@@ -33,10 +33,7 @@
 #include fcntl.h
 #include sys/types.h
 #include sys/stat.h
-
-#ifdef HAVE_SYS_WAIT_H
-# include sys/wait.h
-#endif
+#include sys/wait.h

 #ifdef HAVE_PATHS_H
 # include paths.h
diff --git a/src/util/hooks.c b/src/util/hooks.c
index 507029f..dec9223 100644
--- a/src/util/hooks.c
+++ b/src/util/hooks.c
@@ -24,9 +24,7 @@
 #include config.h

 #include sys/types.h
-#if HAVE_SYS_WAIT_H
-# include sys/wait.h
-#endif
+#include sys/wait.h
 #include sys/stat.h
 #include unistd.h
 #include stdlib.h
diff --git a/src/util/iptables.c b/src/util/iptables.c
index facc4da..4f95a02 100644
--- a/src/util/iptables.c
+++ b/src/util/iptables.c
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2007-2009 Red Hat, Inc.
+ * Copyright (C) 2007-2010 Red Hat, Inc.
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Lesser General Public
@@ -31,10 +31,7 @@
 #include fcntl.h
 #include sys/types.h
 #include sys/stat.h
-
-#ifdef HAVE_SYS_WAIT_H
-# include sys/wait.h
-#endif
+#include sys/wait.h

 #ifdef HAVE_PATHS_H
 # include paths.h
diff --git a/src/util/util.c b/src/util/util.c
index 848f300..f097b2f 100644
--- a/src/util/util.c
+++ b/src/util/util.c
@@ -37,9 +37,7 @@
 #include sys/types.h
 #include sys/stat.h
 #include sys/ioctl.h
-#if HAVE_SYS_WAIT_H
-# include sys/wait.h
-#endif
+#include sys/wait.h
 #if HAVE_MMAP
 # include sys/mman.h
 #endif
-- 
1.6.6.1

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


[libvirt] [PATCHv2 4/5] build: drop more redundant configure checks

2010-05-04 Thread Eric Blake
* configure.ac (AC_CHECK_FUNCS_ONCE, AC_SYS_LARGEFILE): Rely on
gnulib for strtok_r and large file support.
(AC_OBJEXT): Drop call now done by AC_PROG_CC.
(m4_foreach_w): Drop macro guaranteed by gnulib.
(AC_C_CONST): Drop call declared obsolete by autoconf.

Signed-off-by: Eric Blake ebl...@redhat.com
---
 configure.ac |   16 +---
 1 files changed, 1 insertions(+), 15 deletions(-)

diff --git a/configure.ac b/configure.ac
index 12646e0..c643c56 100644
--- a/configure.ac
+++ b/configure.ac
@@ -48,20 +48,10 @@ AC_PROG_CC
 AC_PROG_INSTALL
 AC_PROG_CPP

-AC_OBJEXT
-
-dnl gl_INIT uses m4_foreach_w, yet that is not defined in autoconf-2.59.
-dnl In order to accommodate developers with such old tools, here's a
-dnl replacement definition.
-m4_ifndef([m4_foreach_w],
-  [m4_define([m4_foreach_w],
-[m4_foreach([$1], m4_split(m4_normalize([$2]), [ ]), [$3])])])
-
 gl_EARLY
 gl_INIT

 AM_PROG_CC_STDC
-AC_C_CONST
 AC_TYPE_UID_T

 dnl Make sure we have an ANSI compiler
@@ -101,16 +91,12 @@ fi
 AC_MSG_RESULT([$have_cpuid])


-dnl Support large files / 64 bit seek offsets.
-dnl Use --disable-largefile if you don't want this.
-AC_SYS_LARGEFILE
-
 dnl Availability of various common functions (non-fatal if missing).
 AC_CHECK_FUNCS_ONCE([cfmakeraw regexec sched_getaffinity getuid getgid \
  posix_fallocate mmap])

 dnl Availability of various not common threadsafe functions
-AC_CHECK_FUNCS_ONCE([strerror_r strtok_r getmntent_r getgrnam_r getpwuid_r])
+AC_CHECK_FUNCS_ONCE([strerror_r getmntent_r getgrnam_r getpwuid_r])

 dnl Availability of pthread functions (if missing, win32 threading is
 dnl assumed).  Because of $LIB_PTHREAD, we cannot use AC_CHECK_FUNCS_ONCE.
-- 
1.6.6.1

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


[libvirt] [PATCHv2 2/5] build: use gnulib's uname

2010-05-04 Thread Eric Blake
* bootstrap.conf (gnulib_modules): Add uname.
* configure.ac: Drop uname and sys/utsname.h checks.
* src/nodeinfo.c (nodeGetInfo): Use uname unconditionally.
---
 bootstrap.conf |1 +
 configure.ac   |4 ++--
 src/nodeinfo.c |   12 ++--
 3 files changed, 5 insertions(+), 12 deletions(-)

diff --git a/bootstrap.conf b/bootstrap.conf
index da7cc9c..e85f869 100644
--- a/bootstrap.conf
+++ b/bootstrap.conf
@@ -59,6 +59,7 @@ strtok_r
 sys_stat
 time_r
 timegm
+uname
 useless-if-before-free
 usleep
 vasprintf
diff --git a/configure.ac b/configure.ac
index 93eb654..18da606 100644
--- a/configure.ac
+++ b/configure.ac
@@ -106,7 +106,7 @@ dnl Use --disable-largefile if you don't want this.
 AC_SYS_LARGEFILE

 dnl Availability of various common functions (non-fatal if missing).
-AC_CHECK_FUNCS_ONCE([cfmakeraw regexec uname sched_getaffinity getuid getgid \
+AC_CHECK_FUNCS_ONCE([cfmakeraw regexec sched_getaffinity getuid getgid \
  posix_fallocate mmap])

 dnl Availability of various not common threadsafe functions
@@ -121,7 +121,7 @@ AC_CHECK_FUNCS([pthread_sigmask pthread_mutexattr_init])
 LIBS=$old_libs

 dnl Availability of various common headers (non-fatal if missing).
-AC_CHECK_HEADERS([pwd.h paths.h regex.h sys/syslimits.h sys/utsname.h \
+AC_CHECK_HEADERS([pwd.h paths.h regex.h sys/syslimits.h \
   sys/wait.h sched.h termios.h sys/poll.h syslog.h mntent.h net/ethernet.h])

 dnl Where are the XDR functions?
diff --git a/src/nodeinfo.c b/src/nodeinfo.c
index 4d7fac1..5ec1bcf 100644
--- a/src/nodeinfo.c
+++ b/src/nodeinfo.c
@@ -29,16 +29,13 @@
 #include stdint.h
 #include errno.h
 #include dirent.h
+#include sys/utsname.h

 #if HAVE_NUMACTL
 # define NUMA_VERSION1_COMPATIBILITY 1
 # include numa.h
 #endif

-#ifdef HAVE_SYS_UTSNAME_H
-# include sys/utsname.h
-#endif
-
 #include c-ctype.h
 #include memory.h
 #include nodeinfo.h
@@ -273,18 +270,13 @@ int linuxNodeInfoCPUPopulate(FILE *cpuinfo,
 #endif

 int nodeGetInfo(virConnectPtr conn ATTRIBUTE_UNUSED, virNodeInfoPtr nodeinfo) {
-memset(nodeinfo, 0, sizeof(*nodeinfo));
-
-#ifdef HAVE_UNAME
-{
 struct utsname info;

+memset(nodeinfo, 0, sizeof(*nodeinfo));
 uname(info);

 if (virStrcpyStatic(nodeinfo-model, info.machine) == NULL)
 return -1;
-}
-#endif /* !HAVE_UNAME */

 #ifdef __linux__
 {
-- 
1.6.6.1

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


[libvirt] [PATCHv2 5/5] build: update gnulib

2010-05-04 Thread Eric Blake
81 patches to gnulib, picks up several new syntax checks.

* .gnulib: Update to latest.
* .x-sc_prohibit_always_true_header_tests: New file.
* bootstrap.conf (gnulib_modules): Add sched.
* src/util/processinfo.c (includes): sched.h is now guaranteed.
* src/network/bridge_driver.c (includes): Drop useless
strings.h.
* src/openvz/openvz_conf.c (includes): Likewise.
* src/openvz/openvz_driver.c (includes): Likewise.
* src/phyp/phyp_driver.c (includes): Likewise.
* src/qemu/qemu_driver.c (includes): Likewise.
* src/uml/uml_driver.c (includes): Likewise.
---
 .gnulib |2 +-
 .x-sc_prohibit_always_true_header_tests |4 
 bootstrap.conf  |1 +
 src/network/bridge_driver.c |1 -
 src/openvz/openvz_conf.c|2 +-
 src/openvz/openvz_driver.c  |2 +-
 src/phyp/phyp_driver.c  |1 -
 src/qemu/qemu_driver.c  |1 -
 src/uml/uml_driver.c|1 -
 src/util/processinfo.c  |6 ++
 10 files changed, 10 insertions(+), 11 deletions(-)
 create mode 100644 .x-sc_prohibit_always_true_header_tests

diff --git a/.gnulib b/.gnulib
index 7c1b995..e2843e3 16
--- a/.gnulib
+++ b/.gnulib
@@ -1 +1 @@
-Subproject commit 7c1b995a7041ea366acafeb8632e1080f349f03f
+Subproject commit e2843e30e8c2885eb8cbc77e20c4e0f4d562d44d
diff --git a/.x-sc_prohibit_always_true_header_tests 
b/.x-sc_prohibit_always_true_header_tests
new file mode 100644
index 000..ff753ce
--- /dev/null
+++ b/.x-sc_prohibit_always_true_header_tests
@@ -0,0 +1,4 @@
+ChangeLog*
+docs/news.html.in
+python/libvirt-override.c
+python/typewrappers.c
diff --git a/bootstrap.conf b/bootstrap.conf
index baf0bc2..1e93490 100644
--- a/bootstrap.conf
+++ b/bootstrap.conf
@@ -46,6 +46,7 @@ posix-shell
 pthread
 recv
 random_r
+sched
 send
 setsockopt
 socket
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index 132392b..8432bbc 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -29,7 +29,6 @@
 #include limits.h
 #include string.h
 #include stdio.h
-#include strings.h
 #include stdarg.h
 #include stdlib.h
 #include unistd.h
diff --git a/src/openvz/openvz_conf.c b/src/openvz/openvz_conf.c
index 8735cc1..b52f4ac 100644
--- a/src/openvz/openvz_conf.c
+++ b/src/openvz/openvz_conf.c
@@ -1,6 +1,7 @@
 /*
  * openvz_conf.c: config functions for managing OpenVZ VEs
  *
+ * Copyright (C) 2010 Red Hat, Inc.
  * Copyright (C) 2006, 2007 Binary Karma
  * Copyright (C) 2006 Shuveb Hussain
  * Copyright (C) 2007 Anoop Joe Cyriac
@@ -33,7 +34,6 @@
 #include fcntl.h
 #include sys/types.h
 #include dirent.h
-#include strings.h
 #include time.h
 #include sys/stat.h
 #include unistd.h
diff --git a/src/openvz/openvz_driver.c b/src/openvz/openvz_driver.c
index 00b8a14..ce159d0 100644
--- a/src/openvz/openvz_driver.c
+++ b/src/openvz/openvz_driver.c
@@ -1,6 +1,7 @@
 /*
  * openvz_driver.c: core driver methods for managing OpenVZ VEs
  *
+ * Copyright (C) 2010 Red Hat, Inc.
  * Copyright (C) 2006, 2007 Binary Karma
  * Copyright (C) 2006 Shuveb Hussain
  * Copyright (C) 2007 Anoop Joe Cyriac
@@ -33,7 +34,6 @@
 #include limits.h
 #include string.h
 #include stdio.h
-#include strings.h
 #include stdarg.h
 #include stdlib.h
 #include unistd.h
diff --git a/src/phyp/phyp_driver.c b/src/phyp/phyp_driver.c
index 467ea19..cec99b1 100644
--- a/src/phyp/phyp_driver.c
+++ b/src/phyp/phyp_driver.c
@@ -29,7 +29,6 @@
 #include sys/stat.h
 #include limits.h
 #include string.h
-#include strings.h
 #include stdio.h
 #include stdarg.h
 #include stdlib.h
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 704f824..752551b 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -31,7 +31,6 @@
 #include string.h
 #include stdbool.h
 #include stdio.h
-#include strings.h
 #include stdarg.h
 #include stdlib.h
 #include unistd.h
diff --git a/src/uml/uml_driver.c b/src/uml/uml_driver.c
index 644ac8b..63fe808 100644
--- a/src/uml/uml_driver.c
+++ b/src/uml/uml_driver.c
@@ -29,7 +29,6 @@
 #include limits.h
 #include string.h
 #include stdio.h
-#include strings.h
 #include stdarg.h
 #include stdlib.h
 #include unistd.h
diff --git a/src/util/processinfo.c b/src/util/processinfo.c
index ed2130a..b1b1737 100644
--- a/src/util/processinfo.c
+++ b/src/util/processinfo.c
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2009 Red Hat, Inc.
+ * Copyright (C) 2009-2010 Red Hat, Inc.
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Lesser General Public
@@ -21,9 +21,7 @@

 #include config.h

-#if HAVE_SCHED_H
-# include sched.h
-#endif
+#include sched.h

 #include processinfo.h
 #include virterror_internal.h
-- 
1.6.6.1

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


[libvirt] [PATCH] util: fix va_start usage bug

2010-05-04 Thread Eric Blake
Detected by clang.  POSIX requires that the second argument to
va_start be the name of the last variable; and in some implementations,
passing *path instead of path would dereference bogus memory instead
of pulling arguments off the stack.

* src/util/util.c (virBuildPathInternal): Use correct argument to
va_start.
---

I think this falls under the trivial rule, as it silences a
compiler warning and is a one-line fix of a real bug, so I pushed it.

 src/util/util.c |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/src/util/util.c b/src/util/util.c
index 2d32952..c44d012 100644
--- a/src/util/util.c
+++ b/src/util/util.c
@@ -2799,7 +2799,7 @@ int virBuildPathInternal(char **path, ...)
 va_list ap;
 int ret = 0;

-va_start(ap, *path);
+va_start(ap, path);

 path_component = va_arg(ap, char *);
 virBufferAdd(buf, path_component, -1);
-- 
1.6.6.1

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


Re: [libvirt] (Resend with updates) Live Migration with non-shared storage for kvm

2010-05-04 Thread Eric Blake
On 05/03/2010 07:52 AM, Cole Robinson wrote:
 On 05/03/2010 04:13 AM, Kenneth Nagin wrote:

 
 ...
 
 This the patch file:
 (See attached file: libvirt_migration_ns_100503.patch)

 
 ACK, patch looks fine now.

Kenneth,

I added a commit message, broke a few lines to fit in 80 columns, added
a virCheckFlags to doNativeMigrate (which required a type change), and
removed an extra space in the generated line:

diff --git i/src/qemu/qemu_driver.c w/src/qemu/qemu_driver.c
index 5823e10..47ae52c 100644
--- i/src/qemu/qemu_driver.c
+++ w/src/qemu/qemu_driver.c
@@ -4958,7 +4958,9 @@ static int qemudDomainSaveFlag(virDomainPtr dom,
const char *path,
 if (header.compressed == QEMUD_SAVE_FORMAT_RAW) {
 const char *args[] = { cat, NULL };
 qemuDomainObjEnterMonitorWithDriver(driver, vm);
-rc = qemuMonitorMigrateToFile(priv-mon,
QEMU_MONITOR_MIGRATE_BACKGROUND, args, path, offset);
+rc = qemuMonitorMigrateToFile(priv-mon,
+  QEMU_MONITOR_MIGRATE_BACKGROUND,
+  args, path, offset);
 qemuDomainObjExitMonitorWithDriver(driver, vm);
 } else {
 const char *prog =
qemudSaveCompressionTypeToString(header.compressed);
@@ -4968,7 +4970,9 @@ static int qemudDomainSaveFlag(virDomainPtr dom,
const char *path,
 NULL
 };
 qemuDomainObjEnterMonitorWithDriver(driver, vm);
-rc = qemuMonitorMigrateToFile(priv-mon,
QEMU_MONITOR_MIGRATE_BACKGROUND, args, path, offset);
+rc = qemuMonitorMigrateToFile(priv-mon,
+  QEMU_MONITOR_MIGRATE_BACKGROUND,
+  args, path, offset);
 qemuDomainObjExitMonitorWithDriver(driver, vm);
 }

@@ -5286,7 +5290,9 @@ static int qemudDomainCoreDump(virDomainPtr dom,
 }

 qemuDomainObjEnterMonitorWithDriver(driver, vm);
-ret = qemuMonitorMigrateToFile(priv-mon,
QEMU_MONITOR_MIGRATE_BACKGROUND, args, path, 0);
+ret = qemuMonitorMigrateToFile(priv-mon,
+   QEMU_MONITOR_MIGRATE_BACKGROUND,
+   args, path, 0);
 qemuDomainObjExitMonitorWithDriver(driver, vm);
 if (ret  0)
 goto endjob;
@@ -9884,7 +9890,7 @@ cleanup:
 static int doNativeMigrate(struct qemud_driver *driver,
virDomainObjPtr vm,
const char *uri,
-   unsigned long flags ,
+   unsigned int flags,
const char *dname ATTRIBUTE_UNUSED,
unsigned long resource)
 {
@@ -9893,6 +9899,9 @@ static int doNativeMigrate(struct qemud_driver
*driver,
 qemuDomainObjPrivatePtr priv = vm-privateData;
 unsigned int background_flags = 0;

+virCheckFlags(VIR_MIGRATE_NON_SHARED_DISK | VIR_MIGRATE_NON_SHARED_INC,
+  -1);
+
 /* Issue the migrate command. */
 if (STRPREFIX(uri, tcp:)  !STRPREFIX(uri, tcp://)) {
 /* HACK: source host generates bogus URIs, so fix them up */
@@ -9925,7 +9934,8 @@ static int doNativeMigrate(struct qemud_driver
*driver,
 if (flags  VIR_MIGRATE_NON_SHARED_INC)
 background_flags |= QEMU_MONITOR_MIGRATE_NON_SHARED_INC;

-if (qemuMonitorMigrateToHost(priv-mon, background_flags,
uribits-server, uribits-port)  0) {
+if (qemuMonitorMigrateToHost(priv-mon, background_flags,
uribits-server,
+ uribits-port)  0) {
 qemuDomainObjExitMonitorWithDriver(driver, vm);
 goto cleanup;
 }
@@ -10011,6 +10021,7 @@ static int doTunnelMigrate(virDomainPtr dom,
 int status;
 unsigned long long transferred, remaining, total;
 unsigned int background_flags = QEMU_MONITOR_MIGRATE_BACKGROUND;
+
 /*
  * The order of operations is important here to avoid touching
  * the source VM until we are very sure we can successfully
@@ -10100,7 +10111,8 @@ static int doTunnelMigrate(virDomainPtr dom,
 if (flags  VIR_MIGRATE_NON_SHARED_INC)
 background_flags |= QEMU_MONITOR_MIGRATE_NON_SHARED_INC;
 if (qemuCmdFlags  QEMUD_CMD_FLAG_MIGRATE_QEMU_UNIX){
-internalret = qemuMonitorMigrateToUnix(priv-mon,
background_flags, unixfile);
+internalret = qemuMonitorMigrateToUnix(priv-mon, background_flags,
+   unixfile);
 }
 else if (qemuCmdFlags  QEMUD_CMD_FLAG_MIGRATE_QEMU_EXEC) {
 const char *args[] = { nc, -U, unixfile, NULL };
diff --git i/src/qemu/qemu_monitor_text.c w/src/qemu/qemu_monitor_text.c
index ff79663..3119600 100644
--- i/src/qemu/qemu_monitor_text.c
+++ w/src/qemu/qemu_monitor_text.c
@@ -1139,7 +1139,7 @@ static int qemuMonitorTextMigrate(qemuMonitorPtr mon,
 virReportOOMError();
 return -1;
 }
-virBufferAddLit(extra,  );
+
 if (background  QEMU_MONITOR_MIGRATE_BACKGROUND)
 virBufferAddLit(extra,  -d);
 if (background