[libvirt] [PATCH] Fix virsh inject-nmi man page

[KAMEZAWA Hiroyuki]

>From e61aba3ed00b0d84233f75cc892ac13f06f9021e Mon Sep 17 00:00:00 2001
From: KAMEZAWA Hiroyuki 
Date: Tue, 12 Jul 2011 14:22:45 +0900
Subject: [PATCH] Fix inject-nmi virsh man page

fix the placement.

---
 tools/virsh.pod |8 
 1 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/tools/virsh.pod b/tools/virsh.pod
index 8b820d2..d1c285f 100644
--- a/tools/virsh.pod
+++ b/tools/virsh.pod
@@ -261,6 +261,10 @@ description see:
   L
 The XML also show the NUMA topology information if available.
 
+=item B I
+
+Inject NMI to the guest.
+
 =item B optional I<--inactive> I<--all>
 
 Prints information about one or more domains.  If no domains are
@@ -304,10 +308,6 @@ running B.  When in a paused state the 
domain will still
 consume allocated resources like memory, but will not be eligible for
 scheduling by the hypervisor.
 
-=item B I
-
-Inject NMI to the guest
-
 =item B
 
 The domain is in the process of shutting down, i.e. the guest operating system
-- 
1.7.4.1


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

[libvirt] [Fwd: bug virt-viewer was Re: [virt-tools-list] virt-viewer 0.2.0, as listed on virt-manager download page]

[Jason Helfman]

Any ideas on this issue with virt-viewer?
Thanks!
-jgh

 Original Message 
Subject: bug virt-viewer was Re: [virt-tools-list] virt-viewer 0.2.0, as
listed on virt-manager download page
From:"Jason Helfman" 
Date:Mon, July 11, 2011 12:23 pm
To:  "Jason Helfman" 
Cc:  virt-tools-l...@redhat.com
--

On Mon, Jul 11, 2011 at 11:31:06AM -0700, Jason Helfman thus spake:
>On Mon, Jul 11, 2011 at 02:43:52PM +0100, Richard W.M. Jones thus spake:
>>On Sat, Jul 09, 2011 at 12:02:00PM -0700, Jason Helfman wrote:
>>> Hi,
>>>
>>> Is this the most recent "official" release, as listed on the webpage for
>>> download on virt-manager.org?
>>>
>>> According to the sources directory there is a newer release:
>>>  virt-viewer-0.3.1
>>
>>The web page is wrong.  The latest version does appear to be 0.3.1.
>>
>>Rich.
>>
>
>Thanks! And for some reason, I am unable to configure the build of the tool
>to install locale data to /usr/local/share/locale.
>It keeps wanting to go to /usr/local/lib/locale.
>
>Any ideas?
>
>-jgh
>
>
I've gotten around this with a post-build patch of po/Makefile, however even
if I --disable-nls, it still makes and installs the locales.

-jgh

-- 
Jason Helfman
System Administrator
experts-exchange.com
http://www.experts-exchange.com/M_4830110.html
E4AD 7CF1 1396 27F6 79DD  4342 5E92 AD66 8C8C FBA5



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

Re: [libvirt] [PATCH] build: avoid requiring -lm

[Daniel Veillard]

On Mon, Jul 11, 2011 at 09:37:22PM -0400, Dave Allan wrote:
> On Mon, Jul 11, 2011 at 05:29:24PM -0600, Eric Blake wrote:
> > log2() is heavy when ffs() can do the same thing.  But ffs()
> > requires gnulib support for mingw.
> > 
> > * .gnulib: Update to latest, for ffs.
> > * bootstrap.conf (gnulib_modules): Import ffs.
> > * src/conf/domain_conf.c (virDomainDefParseXML): Use ffs instead
> > of log2.
> > Reported by Dave Allan.
> > ---
> > 
> > Pushing under the build-breaker rule, as I reproduced the log2
> > failure on F14.
> 
> Thanks, that fixes it for me.

  Weird that worked for me on F14, but sure, fine :-)

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] build: avoid requiring -lm

[Dave Allan]

On Mon, Jul 11, 2011 at 05:29:24PM -0600, Eric Blake wrote:
> log2() is heavy when ffs() can do the same thing.  But ffs()
> requires gnulib support for mingw.
> 
> * .gnulib: Update to latest, for ffs.
> * bootstrap.conf (gnulib_modules): Import ffs.
> * src/conf/domain_conf.c (virDomainDefParseXML): Use ffs instead
> of log2.
> Reported by Dave Allan.
> ---
> 
> Pushing under the build-breaker rule, as I reproduced the log2
> failure on F14.

Thanks, that fixes it for me.

Dave

> * .gnulib 56005a2...a918da4 (28):
>   > ffs: new module
>   > * ChangeLog: Fix typo.
>   > regex: avoid compiler warning
>   > stdint: respect system's intmax_t if INTMAX_MAX
>   > pthread_sigmask tests: Avoid a compiler warning.
>   > sigprocmask tests: A better way to avoid a compiler warning.
>   > pthread_sigmask: Work around IRIX bug.
>   > pthread_sigmask: Work around Cygwin bug.
>   > pthread_sigmask: Work around bug in single-threaded implementation.
>   > test-sigprocmask: avoid compiler warning
>   > test-snprintf: avoid compiler warning
>   > Tests for module 'pthread_sigmask'.
>   > test-getopt.h: avoid warning about an unused variable
>   > maint: reduce list of files exempt from sc_prohibit_leading_TABs
>   > pthread_sigmask: Assume POSIX when not gl_THREADLIB.
>   > pthread_sigmask: fix typo when testing for libraries
>   > fts: introduce FTS_NOATIME
>   > Tests for module 'thread'.
>   > thread: Avoid gcc warnings when using gl_thread_self().
>   > signal-c++-tests: Check declaration of pthread_sigmask.
>   > pthread_sigmask: Fix link requirements on OSF/1 5.1 and with pth.
>   > pthread_sigmask: Ensure declaration in  also on Solaris 2.6.
>   > pthread_sigmask: Ensure declaration in .
>   > pthread_sigmask: Document the module.
>   > pthread_sigmask: Follow gnulib conventions.
>   > pthread_sigmask: Make declaration C++ safe.
>   > pthread_sigmask: Fix return value.
>   > getopt: more portable argv creation
> 
>  .gnulib|2 +-
>  bootstrap.conf |1 +
>  src/conf/domain_conf.c |4 ++--
>  3 files changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/.gnulib b/.gnulib
> index 56005a2..a918da4 16
> --- a/.gnulib
> +++ b/.gnulib
> @@ -1 +1 @@
> -Subproject commit 56005a21e8f9f434212a19dcb628c6d3b179fd08
> +Subproject commit a918da4d61d28be61a12605c9d35e2cf3966d866
> diff --git a/bootstrap.conf b/bootstrap.conf
> index a800534..3c3d0e0 100644
> --- a/bootstrap.conf
> +++ b/bootstrap.conf
> @@ -36,6 +36,7 @@ count-one-bits
>  crypto/md5
>  dirname-lgpl
>  fcntl-h
> +ffs
>  fnmatch
>  func
>  getaddrinfo
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 3cf7f44..d75a266 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -29,7 +29,7 @@
>  #include 
>  #include 
>  #include 
> -#include 
> +#include 
> 
>  #include "virterror_internal.h"
>  #include "datatypes.h"
> @@ -5865,7 +5865,7 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr 
> caps,
>  virDomainReportError(VIR_ERR_INTERNAL_ERROR,
>   _("unexpected domain type %s, expecting 
> %s"),
>   virDomainVirtTypeToString(def->virtType),
> - 
> virDomainVirtTypeToString(log2(expectedVirtTypes)));
> + 
> virDomainVirtTypeToString(ffs(expectedVirtTypes) - 1));
>  } else {
>  virBuffer buffer = VIR_BUFFER_INITIALIZER;
>  char *string;
> -- 
> 1.7.4.4
> 
> --
> 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] [PATCHv2 03/27] build: also check qemu_protocol for on-the-wire stability

[Eric Blake]

On 07/11/2011 02:01 PM, Daniel P. Berrange wrote:
> On Fri, Jul 08, 2011 at 01:25:45PM -0600, Eric Blake wrote:
>> Since we are going to add some libvirt-qemu.so entry points in
>> 0.9.4, we might as well start checking for RPC stability, just
>> as for libvirt.so.
>>
>> * src/Makefile.am (PROTOCOL_STRUCTS): New variable.
>> (remote_protocol-structs): Rename...
>> (%_protocol-structs): ...and make more generic.
>> * src/qemu_protocol-structs: New file.
> 
> ACK

Thanks; pushed.

> If we want real paranoia we can do  src/rpc/virnetprotocol.x too,
> though that should basically never be changed by normal patches.

I'll look into that; however, it can't quite reuse the same makefile
rule because libvirt_net_rpc_la-virnetprotocol.lo doesn't fit the
pattern libvirt_driver_remote_la-%_protocol.lo.

-- 
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 1/2] maint: rename virtaudit to match file contents

[Eric Blake]

* src/util/virtaudit.[ch]: Rename...
* src/util/viraudit.[ch]: ...to match virAudit* API.
* src/Makefile.am (UTIL_SOURCES): Reflect rename.
* daemon/libvirtd.c: Likewise.
* po/POTFILES.in: Likewise.
* src/libvirt_private.syms: Likewise.
* src/qemu/qemu_audit.c: Likewise.
---

First suggested here:
https://www.redhat.com/archives/libvir-list/2011-April/msg00368.html

although we still don't have viratomic.h or virobject.h implemented yet.

 daemon/libvirtd.c|2 +-
 po/POTFILES.in   |2 +-
 src/Makefile.am  |2 +-
 src/libvirt_private.syms |2 +-
 src/qemu/qemu_audit.c|2 +-
 src/util/{virtaudit.c => viraudit.c} |4 ++--
 src/util/{virtaudit.h => viraudit.h} |4 ++--
 7 files changed, 9 insertions(+), 9 deletions(-)
 rename src/util/{virtaudit.c => viraudit.c} (98%)
 rename src/util/{virtaudit.h => viraudit.h} (96%)

diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c
index a4198d9..97db696 100644
--- a/daemon/libvirtd.c
+++ b/daemon/libvirtd.c
@@ -52,7 +52,7 @@
 #include "remote_driver.h"
 #include "hooks.h"
 #include "uuid.h"
-#include "virtaudit.h"
+#include "viraudit.h"

 #ifdef WITH_DRIVER_MODULES
 # include "driver.h"
diff --git a/po/POTFILES.in b/po/POTFILES.in
index 32eaa2d..1b63378 100644
--- a/po/POTFILES.in
+++ b/po/POTFILES.in
@@ -120,7 +120,7 @@ src/util/stats_linux.c
 src/util/storage_file.c
 src/util/sysinfo.c
 src/util/util.c
-src/util/virtaudit.c
+src/util/viraudit.c
 src/util/virterror.c
 src/util/xml.c
 src/vbox/vbox_MSCOMGlue.c
diff --git a/src/Makefile.am b/src/Makefile.am
index cd8a7e9..ff85db3 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -79,8 +79,8 @@ UTIL_SOURCES =
\
util/threadpool.c util/threadpool.h \
util/uuid.c util/uuid.h \
util/util.c util/util.h \
+   util/viraudit.c util/viraudit.h \
util/xml.c util/xml.h   \
-   util/virtaudit.c util/virtaudit.h   \
util/virterror.c util/virterror_internal.h

 EXTRA_DIST += util/threads-pthread.c util/threads-win32.c
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 1112398..4d78fcf 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1058,7 +1058,7 @@ virUUIDGenerate;
 virUUIDParse;


-# virtaudit.h
+# viraudit.h
 virAuditClose;
 virAuditEncode;
 virAuditLog;
diff --git a/src/qemu/qemu_audit.c b/src/qemu/qemu_audit.c
index 1baef40..1d88fb5 100644
--- a/src/qemu/qemu_audit.c
+++ b/src/qemu/qemu_audit.c
@@ -27,7 +27,7 @@
 #include 

 #include "qemu_audit.h"
-#include "virtaudit.h"
+#include "viraudit.h"
 #include "uuid.h"
 #include "logging.h"
 #include "memory.h"
diff --git a/src/util/virtaudit.c b/src/util/viraudit.c
similarity index 98%
rename from src/util/virtaudit.c
rename to src/util/viraudit.c
index 560f7b7..ebf3119 100644
--- a/src/util/virtaudit.c
+++ b/src/util/viraudit.c
@@ -1,5 +1,5 @@
 /*
- * virtaudit.c: auditing support
+ * viraudit.c: auditing support
  *
  * Copyright (C) 2010-2011 Red Hat, Inc.
  *
@@ -29,7 +29,7 @@

 #include "virterror_internal.h"
 #include "logging.h"
-#include "virtaudit.h"
+#include "viraudit.h"
 #include "util.h"
 #include "files.h"
 #include "memory.h"
diff --git a/src/util/virtaudit.h b/src/util/viraudit.h
similarity index 96%
rename from src/util/virtaudit.h
rename to src/util/viraudit.h
index a558a17..9d8c359 100644
--- a/src/util/virtaudit.h
+++ b/src/util/viraudit.h
@@ -1,7 +1,7 @@
 /*
- * virtaudit.h: auditing support
+ * viraudit.h: auditing support
  *
- * Copyright (C) 2010 Red Hat, Inc.
+ * Copyright (C) 2010-2011 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
-- 
1.7.4.4

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

[libvirt] [PATCH 2/2] qemu: avoid fd leak on core dump failure

[Eric Blake]

* src/qemu/qemu_driver.c (doCoreDump): Guarantee fd is closed.
---

Spotted by hand, while working on virDomainSaveFlags.

 src/qemu/qemu_driver.c |   10 ++
 1 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 1356c54..f9d2de4 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -2525,10 +2525,11 @@ cleanup:
 return ret;
 }

-static int doCoreDump(struct qemud_driver *driver,
-  virDomainObjPtr vm,
-  const char *path,
-  enum qemud_save_formats compress)
+static int
+doCoreDump(struct qemud_driver *driver,
+   virDomainObjPtr vm,
+   const char *path,
+   enum qemud_save_formats compress)
 {
 int fd = -1;
 int ret = -1;
@@ -2554,6 +2555,7 @@ static int doCoreDump(struct qemud_driver *driver,
 ret = 0;

 cleanup:
+VIR_FORCE_CLOSE(fd);
 if (ret != 0)
 unlink(path);
 return ret;
-- 
1.7.4.4

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

Re: [libvirt] [PATCH] Add domain type checking

[Eric Blake]

On 07/11/2011 03:32 PM, Eric Blake wrote:
 +#include 
>>>
>>> What was this needed for?
>>
>> For log2 to convert 1 << x back to x in case only one bit is set in
>> expectedVirtTypes.
> 
> log2(), including -lm, is rather heavyweight.  It should be possible to
> use ffs() from  to do the same thing, and then we don't have
> to worry about dragging in -lm.
> 
> I'll work up a patch tomorrow, if no one beats me to it tonight.

Done:

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

-- 
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] build: avoid requiring -lm

[Eric Blake]

log2() is heavy when ffs() can do the same thing.  But ffs()
requires gnulib support for mingw.

* .gnulib: Update to latest, for ffs.
* bootstrap.conf (gnulib_modules): Import ffs.
* src/conf/domain_conf.c (virDomainDefParseXML): Use ffs instead
of log2.
Reported by Dave Allan.
---

Pushing under the build-breaker rule, as I reproduced the log2
failure on F14.

* .gnulib 56005a2...a918da4 (28):
  > ffs: new module
  > * ChangeLog: Fix typo.
  > regex: avoid compiler warning
  > stdint: respect system's intmax_t if INTMAX_MAX
  > pthread_sigmask tests: Avoid a compiler warning.
  > sigprocmask tests: A better way to avoid a compiler warning.
  > pthread_sigmask: Work around IRIX bug.
  > pthread_sigmask: Work around Cygwin bug.
  > pthread_sigmask: Work around bug in single-threaded implementation.
  > test-sigprocmask: avoid compiler warning
  > test-snprintf: avoid compiler warning
  > Tests for module 'pthread_sigmask'.
  > test-getopt.h: avoid warning about an unused variable
  > maint: reduce list of files exempt from sc_prohibit_leading_TABs
  > pthread_sigmask: Assume POSIX when not gl_THREADLIB.
  > pthread_sigmask: fix typo when testing for libraries
  > fts: introduce FTS_NOATIME
  > Tests for module 'thread'.
  > thread: Avoid gcc warnings when using gl_thread_self().
  > signal-c++-tests: Check declaration of pthread_sigmask.
  > pthread_sigmask: Fix link requirements on OSF/1 5.1 and with pth.
  > pthread_sigmask: Ensure declaration in  also on Solaris 2.6.
  > pthread_sigmask: Ensure declaration in .
  > pthread_sigmask: Document the module.
  > pthread_sigmask: Follow gnulib conventions.
  > pthread_sigmask: Make declaration C++ safe.
  > pthread_sigmask: Fix return value.
  > getopt: more portable argv creation

 .gnulib|2 +-
 bootstrap.conf |1 +
 src/conf/domain_conf.c |4 ++--
 3 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/.gnulib b/.gnulib
index 56005a2..a918da4 16
--- a/.gnulib
+++ b/.gnulib
@@ -1 +1 @@
-Subproject commit 56005a21e8f9f434212a19dcb628c6d3b179fd08
+Subproject commit a918da4d61d28be61a12605c9d35e2cf3966d866
diff --git a/bootstrap.conf b/bootstrap.conf
index a800534..3c3d0e0 100644
--- a/bootstrap.conf
+++ b/bootstrap.conf
@@ -36,6 +36,7 @@ count-one-bits
 crypto/md5
 dirname-lgpl
 fcntl-h
+ffs
 fnmatch
 func
 getaddrinfo
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 3cf7f44..d75a266 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -29,7 +29,7 @@
 #include 
 #include 
 #include 
-#include 
+#include 

 #include "virterror_internal.h"
 #include "datatypes.h"
@@ -5865,7 +5865,7 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr 
caps,
 virDomainReportError(VIR_ERR_INTERNAL_ERROR,
  _("unexpected domain type %s, expecting %s"),
  virDomainVirtTypeToString(def->virtType),
- 
virDomainVirtTypeToString(log2(expectedVirtTypes)));
+ 
virDomainVirtTypeToString(ffs(expectedVirtTypes) - 1));
 } else {
 virBuffer buffer = VIR_BUFFER_INITIALIZER;
 char *string;
-- 
1.7.4.4

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

Re: [libvirt] [PATCH] Fix build when using polkit0

[Jim Fehlig]

Daniel P. Berrange wrote:
> I'd like the virNetServer stuff to not have any mention of policy
> kit in it. So rather than saying 'bool usePolkit', have a
> 'bool connectDBus', and just remove those HAVE_POLKIT0 conditionals
> so that the DBus API is always available in virNetServer. The changes
> you made under daemon/ are all fine
>   

The attached patch introduces a HAVE_DBUS conditional, which AFAICT is
only used by polkit0 so is only set when polkit0 is detected during
configure.  Is this what you had in mind?

Regards,
Jim

>From c21f206a8196cef7657e30b9ee830bcc4bbfc3ff Mon Sep 17 00:00:00 2001
From: Jim Fehlig 
Date: Thu, 7 Jul 2011 15:12:26 -0600
Subject: [V2] Fix build when using polkit0

V2: Remove policy kit references from virNetServer and use DBus APIs
directly, if available.
---
 configure.ac   |5 +
 daemon/libvirtd.c  |   24 
 daemon/remote.c|   21 ++---
 src/Makefile.am|4 +++-
 src/rpc/virnetserver.c |   41 -
 src/rpc/virnetserver.h |8 
 6 files changed, 70 insertions(+), 33 deletions(-)

diff --git a/configure.ac b/configure.ac
index ae747fb..e9d5be4 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1010,6 +1010,7 @@ AC_ARG_WITH([polkit],
   [with_polkit=check])
 
 with_polkit0=no
+with_dbus=no
 with_polkit1=no
 if test "x$with_polkit" = "xyes" || test "x$with_polkit" = "xcheck"; then
   dnl Check for new polkit first - just a binary
@@ -1038,6 +1039,8 @@ if test "x$with_polkit" = "xyes" || test "x$with_polkit" = "xcheck"; then
 [use PolicyKit for UNIX socket access checks])
   AC_DEFINE_UNQUOTED([HAVE_POLKIT0], 1,
 [use PolicyKit for UNIX socket access checks])
+  AC_DEFINE_UNQUOTED([HAVE_DBUS], 1,
+[use DBus for PolicyKit])
 
   old_CFLAGS=$CFLAGS
   old_LIBS=$LIBS
@@ -1052,11 +1055,13 @@ if test "x$with_polkit" = "xyes" || test "x$with_polkit" = "xcheck"; then
 AC_DEFINE_UNQUOTED([POLKIT_AUTH],["$POLKIT_AUTH"],[Location of polkit-auth program])
   fi
   with_polkit0="yes"
+  with_dbus="yes"
 fi
   fi
 fi
 AM_CONDITIONAL([HAVE_POLKIT], [test "x$with_polkit" = "xyes"])
 AM_CONDITIONAL([HAVE_POLKIT0], [test "x$with_polkit0" = "xyes"])
+AM_CONDITIONAL([HAVE_DBUS], [test "x$with_dbus" = "xyes"])
 AM_CONDITIONAL([HAVE_POLKIT1], [test "x$with_polkit1" = "xyes"])
 AC_SUBST([POLKIT_CFLAGS])
 AC_SUBST([POLKIT_LIBS])
diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c
index a4198d9..c7ee605 100644
--- a/daemon/libvirtd.c
+++ b/daemon/libvirtd.c
@@ -576,26 +576,6 @@ static int daemonSetupNetworking(virNetServerPtr srv,
 }
 #endif
 
-#if HAVE_POLKIT0
-if (auth_unix_rw == REMOTE_AUTH_POLKIT ||
-auth_unix_ro == REMOTE_AUTH_POLKIT) {
-DBusError derr;
-
-dbus_connection_set_change_sigpipe(FALSE);
-dbus_threads_init_default();
-
-dbus_error_init(&derr);
-server->sysbus = dbus_bus_get(DBUS_BUS_SYSTEM, &derr);
-if (!(server->sysbus)) {
-VIR_ERROR(_("Failed to connect to system bus for PolicyKit auth: %s"),
-  derr.message);
-dbus_error_free(&derr);
-goto error;
-}
-dbus_connection_set_exit_on_disconnect(server->sysbus, FALSE);
-}
-#endif
-
 return 0;
 
 error:
@@ -1285,6 +1265,7 @@ int main(int argc, char **argv) {
 struct daemonConfig *config;
 bool privileged = geteuid() == 0 ? true : false;
 bool implicit_conf = false;
+bool use_polkit_dbus;
 
 struct option opts[] = {
 { "verbose", no_argument, &verbose, 1},
@@ -1445,10 +1426,13 @@ int main(int argc, char **argv) {
 umask(old_umask);
 }
 
+use_polkit_dbus = config->auth_unix_rw == REMOTE_AUTH_POLKIT ||
+config->auth_unix_ro == REMOTE_AUTH_POLKIT;
 if (!(srv = virNetServerNew(config->min_workers,
 config->max_workers,
 config->max_clients,
 config->mdns_adv ? config->mdns_name : NULL,
+use_polkit_dbus,
 remoteClientInitHook))) {
 ret = VIR_DAEMON_ERR_INIT;
 goto cleanup;
diff --git a/daemon/remote.c b/daemon/remote.c
index a2e79ef..0172626 100644
--- a/daemon/remote.c
+++ b/daemon/remote.c
@@ -43,6 +43,7 @@
 #include "command.h"
 #include "intprops.h"
 #include "virnetserverservice.h"
+#include "virnetserver.h"
 
 #include "remote_protocol.h"
 #include "qemu_protocol.h"
@@ -2115,7 +2116,7 @@ authdeny:
 }
 #elif HAVE_POLKIT0
 static int
-remoteDispatchAuthPolkit(virNetServerPtr server ATTRIBUTE_UNUSED,
+remoteDispatchAuthPolkit(virNetServerPtr server,
  virNetServerClientPtr client,
  virNetMessageHeaderPtr hdr ATTRIBUTE_UNUSED,
  virNetMessageErrorPtr rerr,
@@ -2137,21 +2138,19 @@ remoteDispatchAuthPolkit(virNetServ

[libvirt] make failure in HEAD

[Dave Allan]

I'm seeing:

/usr/bin/ld: libvirt_lxc-domain_conf.o: undefined reference
to symbol 'log2@@GLIBC_2.2.5'

when building the git HEAD just now.

It appears to be the result of:

aa14709a src/conf/domain_conf.c (Matthias Bolte 2011-07-11
19:29:09 +0200  5868)
virDomainVirtTypeToString(log2(expectedVirtTypes)));

I don't have time to look into it, but thought I should mention it.

Dave

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

Re: [libvirt] [PATCH] Add domain type checking

[Eric Blake]

On 07/11/2011 11:58 AM, Matthias Bolte wrote:
> 2011/7/11 Eric Blake :
>> On 07/11/2011 10:16 AM, Matthias Bolte wrote:
> My only regret here is that we can't really suggest the value expected
> because QEmu accepts more than one, but for other drivers we should be
> able to provide what type is expected.
>>> Yes, we can do that even for QEMU. See attached diff between v2 and v3
>>> for easier review.
>>
>>> +++ b/src/conf/domain_conf.c
>>> @@ -29,6 +29,7 @@
>>>  #include 
>>>  #include 
>>>  #include 
>>> +#include 
>>
>> What was this needed for?
> 
> For log2 to convert 1 << x back to x in case only one bit is set in
> expectedVirtTypes.

log2(), including -lm, is rather heavyweight.  It should be possible to
use ffs() from  to do the same thing, and then we don't have
to worry about dragging in -lm.

I'll work up a patch tomorrow, if no one beats me to it tonight.

-- 
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] Remove code no longer used after commit df0b57a9

[Jim Fehlig]

Eric Blake wrote:
> On 07/11/2011 02:40 PM, Jim Fehlig wrote:
>   
>> ---
>>  daemon/libvirtd.h |   30 --
>>  1 files changed, 0 insertions(+), 30 deletions(-)
>> 
>
> ACK.
>
>   
Thanks, pushed.

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

Re: [libvirt] [PATCH] Remove code no longer used after commit df0b57a9

[Eric Blake]

On 07/11/2011 02:40 PM, Jim Fehlig wrote:
> ---
>  daemon/libvirtd.h |   30 --
>  1 files changed, 0 insertions(+), 30 deletions(-)

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] Remove code no longer used after commit df0b57a9

[Jim Fehlig]

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

diff --git a/daemon/libvirtd.h b/daemon/libvirtd.h
index 8e1843c..6c9b9c3 100644
--- a/daemon/libvirtd.h
+++ b/daemon/libvirtd.h
@@ -27,10 +27,6 @@
 
 # include 
 
-# if HAVE_POLKIT0
-#  include 
-# endif
-
 # include 
 # include 
 # include "remote_protocol.h"
@@ -91,30 +87,4 @@ extern virNetSASLContextPtr saslCtxt;
 extern virNetServerProgramPtr remoteProgram;
 extern virNetServerProgramPtr qemuProgram;
 
-/* Main server state */
-struct qemud_server {
-int privileged;
-
-int sigread;
-int sigwrite;
-char *logDir;
-pthread_t eventThread;
-unsigned int hasEventThread :1;
-unsigned int quitEventThread :1;
-# ifdef HAVE_AVAHI
-struct libvirtd_mdns *mdns;
-# endif
-# if HAVE_SASL
-char **saslUsernameWhitelist;
-# endif
-# if HAVE_POLKIT0
-DBusConnection *sysbus;
-# endif
-};
-
-
-# if HAVE_POLKIT
-int qemudGetSocketIdentity(int fd, uid_t *uid, pid_t *pid);
-# endif
-
 #endif
-- 
1.7.5.4

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

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

[Daniel P. Berrange]

On Fri, Jul 08, 2011 at 01:34:05AM +0200, Jiri Denemark wrote:
> This series is also available at
> https://gitorious.org/~jirka/libvirt/jirka-staging/commits/migration-recovery
> 
> The series does several things:
> - persists current job and its phase in status xml
> - allows safe monitor commands to be run during migration/save/dump jobs
> - implements recovery when libvirtd is restarted while a job is active
> - consolidates some code and fixes bugs I found when working in the area
> 
> The series is not perfect and still needs some corner cases to be fixed but I
> think it's better to send the series for review now and add small additional
> fixes in the next version(s) instead of waiting for it to be perfect.

What level of testing has it had so far ?  In particular does it pass the
giant migration test case in the TCK ? And if so, what combinations of
client/source/dest libvirt were tested.

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

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

Re: [libvirt] [PATCH 08/19] qemu: Consolidate qemuMigrationPrepare{Direct, Tunnel}

[Daniel P. Berrange]

On Fri, Jul 08, 2011 at 01:34:13AM +0200, Jiri Denemark wrote:
> Most of the code in these two functions is supposed to be identical but
> currently it isn't (which is natural since the code is duplicated).
> Let's move common parts of these functions into qemuMigrationPrepareAny.

Were there any actual bugs fixed in this consolidation, or were
the differences in the code just cosmetic ? If so it'd be good to
mention the bugs fixed in the commit message

> ---
>  src/qemu/qemu_migration.c |  255 +++-
>  1 files changed, 87 insertions(+), 168 deletions(-)

ACK

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

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

Re: [libvirt] [PATCH] Fix build when using polkit0

[Jim Fehlig]

Jim Fehlig wrote:
> Here's a hacked attempt at fixing the build on older distros using
> polkit0.  It works and user is authorized or denied depending on
> settings in PolicyKit.conf.
>   
[...]

> diff --git a/daemon/libvirtd.h b/daemon/libvirtd.h
> index 8e1843c..18a10ef 100644
> --- a/daemon/libvirtd.h
> +++ b/daemon/libvirtd.h
> @@ -91,30 +91,4 @@ extern virNetSASLContextPtr saslCtxt;
>  extern virNetServerProgramPtr remoteProgram;
>  extern virNetServerProgramPtr qemuProgram;
>  
> -/* Main server state */
> -struct qemud_server {
> -int privileged;
> -
> -int sigread;
> -int sigwrite;
> -char *logDir;
> -pthread_t eventThread;
> -unsigned int hasEventThread :1;
> -unsigned int quitEventThread :1;
> -# ifdef HAVE_AVAHI
> -struct libvirtd_mdns *mdns;
> -# endif
> -# if HAVE_SASL
> -char **saslUsernameWhitelist;
> -# endif
> -# if HAVE_POLKIT0
> -DBusConnection *sysbus;
> -# endif
> -};
> -
> -
> -# if HAVE_POLKIT
> -int qemudGetSocketIdentity(int fd, uid_t *uid, pid_t *pid);
> -# endif
> -
>  #endif
>   

This hunk, and another in libvirtd.h, have nothing to do with polkit
build fix and are just removing dead code.  I'll submit those as a
separate patch.

Regards,
Jim


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

Re: [libvirt] [PATCH 01/19] qemu: Separate job related data into a new object

[Daniel P. Berrange]

On Fri, Jul 08, 2011 at 01:34:06AM +0200, Jiri Denemark wrote:
> ---
>  src/qemu/qemu_domain.c|  104 +
>  src/qemu/qemu_domain.h|   25 +++---
>  src/qemu/qemu_driver.c|   82 +++---
>  src/qemu/qemu_migration.c |  123 
> ++---
>  src/qemu/qemu_process.c   |5 +-
>  5 files changed, 192 insertions(+), 147 deletions(-)

ACK

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

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

Re: [libvirt] [PATCH 06/19] qemu: Recover from interrupted jobs

[Daniel P. Berrange]

On Fri, Jul 08, 2011 at 01:34:11AM +0200, Jiri Denemark wrote:
> Detect and react on situations when libvirtd was restarted or killed
> when a job was active.
> ---
>  src/qemu/qemu_domain.c  |   14 
>  src/qemu/qemu_domain.h  |2 +
>  src/qemu/qemu_process.c |   80 
> +++
>  3 files changed, 96 insertions(+), 0 deletions(-)

ACK

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

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

Re: [libvirt] [PATCH 07/19] qemu: Add support for job phase

[Daniel P. Berrange]

On Fri, Jul 08, 2011 at 01:34:12AM +0200, Jiri Denemark wrote:
> Asynchronous jobs may take long time to finish and may consist of
> several phases which we need to now about to help with recovery/rollback
> after libvirtd restarts.
> ---
>  src/qemu/qemu_domain.c |   75 
> +++-
>  src/qemu/qemu_domain.h |9 ++
>  2 files changed, 83 insertions(+), 1 deletions(-)

ACK

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

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

Re: [libvirt] [PATCH 05/19] qemu: Save job type in domain status XML

[Daniel P. Berrange]

On Fri, Jul 08, 2011 at 01:34:10AM +0200, Jiri Denemark wrote:
> If libvirtd is restarted when a job is running, the new libvirtd process
> needs to know about that to be able to recover and rollback the
> operation.
> ---
>  src/qemu/qemu_domain.c|  124 +++-
>  src/qemu/qemu_domain.h|   35 
>  src/qemu/qemu_driver.c|  138 
> +++--
>  src/qemu/qemu_hotplug.c   |   12 ++--
>  src/qemu/qemu_migration.c |   20 ---
>  src/qemu/qemu_process.c   |8 +-
>  6 files changed, 212 insertions(+), 125 deletions(-)

ACK


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

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

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

[Daniel P. Berrange]

On Fri, Jul 08, 2011 at 01:34:09AM +0200, Jiri Denemark wrote:
> Query commands are safe to be called during long running jobs (such as
> migration). This patch makes them all work without the need to
> special-case every single one of them.
> 
> The patch introduces new job.asyncCond condition and associated
> job.asyncJob which are dedicated to asynchronous (from qemu monitor
> point of view) jobs that can take arbitrarily long time to finish while
> qemu monitor is still usable for other commands.
> 
> The existing job.active (and job.cond condition) is used all other
> synchronous jobs (including the commands run during async job).
> 
> Locking schema is changed to use these two conditions. While asyncJob is
> active, only allowed set of synchronous jobs is allowed (the set can be
> different according to a particular asyncJob) so any method that
> communicates to qemu monitor needs to check if it is allowed to be
> executed during current asyncJob (if any). Once the check passes, the
> method needs to normally acquire job.cond to ensure no other command is
> running. Since domain object lock is released during that time, asyncJob
> could have been started in the meantime so the method needs to recheck
> the first condition. Then, normal jobs set job.active and asynchronous
> jobs set job.asyncJob and optionally change the list of allowed job
> groups.
> 
> Since asynchronous jobs only set job.asyncJob, other allowed commands
> can still be run when domain object is unlocked (when communicating to
> remote libvirtd or sleeping). To protect its own internal synchronous
> commands, the asynchronous job needs to start a special nested job
> before entering qemu monitor. The nested job doesn't check asyncJob, it
> only acquires job.cond and sets job.active to block other jobs.
> ---
>  src/qemu/qemu_domain.c|  219 
> +
>  src/qemu/qemu_domain.h|   82 +
>  src/qemu/qemu_driver.c|  122 +-
>  src/qemu/qemu_hotplug.c   |   38 
>  src/qemu/qemu_migration.c |  152 ++-
>  src/qemu/qemu_process.c   |   42 +
>  6 files changed, 439 insertions(+), 216 deletions(-)

ACK

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

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

Re: [libvirt] [PATCH 03/19] qemu: Consolidate {Enter, Exit}Monitor{, WithDriver}

[Daniel P. Berrange]

On Fri, Jul 08, 2011 at 01:34:08AM +0200, Jiri Denemark wrote:
> EnterMonitor and ExitMonitor methods are very similar to their
> *WithDriver variants; consolidate them into EnterMonitorInternal and
> ExitMonitorInternal to avoid (mainly future) code duplication.
> ---
>  src/qemu/qemu_domain.c |   74 ++-
>  1 files changed, 35 insertions(+), 39 deletions(-)

ACK


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

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

Re: [libvirt] [PATCH 0/6] Add auditing to LXC and UML drivers

[Daniel P. Berrange]

On Mon, Jul 04, 2011 at 12:33:31PM +0100, Daniel P. Berrange wrote:
> This series adds auditing to the LXC and UML drivers. As a side
> effect it also fixes missing auditing of filesystem passthrough
> in the QEMU driver and a bug in UML vm cleanup

Anyone got comments on this (very easy to review :-P ) series ?

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

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

Re: [libvirt] [PATCHv2 03/27] build: also check qemu_protocol for on-the-wire stability

[Daniel P. Berrange]

On Fri, Jul 08, 2011 at 01:25:45PM -0600, Eric Blake wrote:
> Since we are going to add some libvirt-qemu.so entry points in
> 0.9.4, we might as well start checking for RPC stability, just
> as for libvirt.so.
> 
> * src/Makefile.am (PROTOCOL_STRUCTS): New variable.
> (remote_protocol-structs): Rename...
> (%_protocol-structs): ...and make more generic.
> * src/qemu_protocol-structs: New file.
> ---
> 
> v2: new patch, first suggested here:
> https://www.redhat.com/archives/libvir-list/2011-July/msg00433.html
> 
>  src/Makefile.am   |   20 +++-
>  src/qemu_protocol-structs |   14 ++
>  2 files changed, 25 insertions(+), 9 deletions(-)
>  create mode 100644 src/qemu_protocol-structs
> 
> diff --git a/src/Makefile.am b/src/Makefile.am
> index cd8a7e9..bd965de 100644
> --- a/src/Makefile.am
> +++ b/src/Makefile.am
> @@ -211,16 +211,18 @@ EXTRA_DIST +=  $(REMOTE_DRIVER_PROTOCOL) \
>  r1 = (?:/\* \d+ \*/\n)?
>  r2 = /\* <[[:xdigit:]]+> \S+:\d+ \*/
> 
> -.PHONY: remote_protocol-structs
> +PROTOCOL_STRUCTS = \
> + $(srcdir)/remote_protocol-structs \
> + $(srcdir)/qemu_protocol-structs
>  if WITH_REMOTE
>  # The .o file that pdwtags parses is created as a side effect of running
>  # libtool; but from make's perspective we depend on the .lo file.
> -remote_protocol-structs: libvirt_driver_remote_la-remote_protocol.lo
> +%_protocol-structs: libvirt_driver_remote_la-%_protocol.lo
>   $(AM_V_GEN)if (pdwtags --help) > /dev/null 2>&1; then   \
> -   pdwtags --verbose libvirt_driver_remote_la-remote_protocol.$(OBJEXT) \
> +   pdwtags --verbose $(<:.lo=.$(OBJEXT)) \
>   | perl -0777 -n \
>   -e 'foreach my $$p (split m!\n\n$(r1)$(r2)\n!) {'   \
> - -e '  if ($$p =~ /^struct remote_/) {'  \
> + -e '  if ($$p =~ /^struct (remote|qemu)_/) {'   \
>   -e '$$p =~ s!\t*/\*.*?\*/!!sg;' \
>   -e '$$p =~ s!\s+\n!\n!sg;'  \
>   -e '$$p =~ s!\s+$$!!;'  \
> @@ -233,7 +235,7 @@ remote_protocol-structs: 
> libvirt_driver_remote_la-remote_protocol.lo
>   -e '  print "/* -*- c -*- */\n";'   \
>   -e '}'  \
>   -e 'END {'  \
> - -e '  if ($$n < 300) {' \
> + -e '  if ($$n < 3) {'   \
>   -e 'warn "WARNING: your pdwtags program is too old\n";' \
>   -e 'warn "WARNING: skipping the $@ test\n";'\
>   -e 'warn "WARNING: install dwarves-1.3 or newer\n";' \
> @@ -248,12 +250,12 @@ remote_protocol-structs: 
> libvirt_driver_remote_la-remote_protocol.lo
> echo 'WARNING: install the dwarves package to get pdwtags' >&2; \
>   fi
>  else !WITH_REMOTE
> -# This generated file must live in git, because it cannot be re-generated
> +# These generated files must live in git, because they cannot be re-generated
>  # when configured --without-remote.
> -remote_protocol-structs:
> +$(srcdir)/%_protocol-structs:
>  endif
> -EXTRA_DIST += remote_protocol-structs
> -check-local: remote_protocol-structs
> +EXTRA_DIST += $(PROTOCOL_STRUCTS)
> +check-local: $(PROTOCOL_STRUCTS)
> 
>  # Mock driver, covering domains, storage, networks, etc
>  TEST_DRIVER_SOURCES =\
> diff --git a/src/qemu_protocol-structs b/src/qemu_protocol-structs
> new file mode 100644
> index 000..e93e8bf
> --- /dev/null
> +++ b/src/qemu_protocol-structs
> @@ -0,0 +1,14 @@
> +/* -*- c -*- */
> +struct remote_nonnull_domain {
> +remote_nonnull_string  name;
> +remote_uuiduuid;
> +intid;
> +};
> +struct qemu_monitor_command_args {
> +remote_nonnull_domain  dom;
> +remote_nonnull_string  cmd;
> +intflags;
> +};
> +struct qemu_monitor_command_ret {
> +remote_nonnull_string  result;
> +};

ACK


If we want real paranoia we can do  src/rpc/virnetprotocol.x too,
though that should basically never be changed by normal patches.

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

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

Re: [libvirt] [PATCHv2] remote/ssh: support for no_verify.

[Daniel P. Berrange]

On Mon, Jul 11, 2011 at 10:50:31PM +0300, Oskari Saarenmaa wrote:
> Set StrictHostKeyChecking=no to auto-accept new ssh host keys if the
> no_verify extra parameter was specified.  This won't disable host key
> checking for already known hosts.  Includes a test and documentation.
> ---
>  Thanks for the review, here's an updated patch.
> 
>  docs/remote.html.in|9 +++--
>  src/remote/remote_driver.c |1 +
>  src/rpc/virnetclient.c |3 ++-
>  src/rpc/virnetclient.h |1 +
>  src/rpc/virnetsocket.c |3 +++
>  src/rpc/virnetsocket.h |1 +
>  tests/virnetsockettest.c   |   22 +++---
>  7 files changed, 34 insertions(+), 6 deletions(-)

ACK, this looks nice to me.

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

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

Re: [libvirt] [PATCH 1/8] Define a QEMU specific API to attach to a running QEMU process

[Daniel P. Berrange]

On Fri, Jul 08, 2011 at 02:52:36PM -0600, Eric Blake wrote:
> On 07/04/2011 04:28 AM, Daniel P. Berrange wrote:
> > Introduce a new API in libvirt-qemu.so
> > 
> >  virDomainPtr virDomainQemuAttach(virConnectPtr domain,
> >   unsigned long long pid,
> 
> We already assert elsewhere in our code base that pid_t will always fit
> in int.  For example, see _virDomainObj in domain_conf.h.  Passing a ull
> here seems like it might be overkill.

Last time I posted this series there was some question whether we'd be
ok with just 'unsigned int pid', so I changed to long long. I can
change it back again trivially though if we think unsigned int will
in face be safe.

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

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

[libvirt] [PATCHv2] remote/ssh: support for no_verify.

[Oskari Saarenmaa]

Set StrictHostKeyChecking=no to auto-accept new ssh host keys if the
no_verify extra parameter was specified.  This won't disable host key
checking for already known hosts.  Includes a test and documentation.
---
 Thanks for the review, here's an updated patch.

 docs/remote.html.in|9 +++--
 src/remote/remote_driver.c |1 +
 src/rpc/virnetclient.c |3 ++-
 src/rpc/virnetclient.h |1 +
 src/rpc/virnetsocket.c |3 +++
 src/rpc/virnetsocket.h |1 +
 tests/virnetsockettest.c   |   22 +++---
 7 files changed, 34 insertions(+), 6 deletions(-)

diff --git a/docs/remote.html.in b/docs/remote.html.in
index f6a0683..39d65aa 100644
--- a/docs/remote.html.in
+++ b/docs/remote.html.in
@@ -279,9 +279,14 @@ Note that parameter values must be
 
   no_verify
 
- tls 
-
-  If set to a non-zero value, this disables client checks of the
+ ssh, tls 
+
+  SSH: If set to a non-zero value, this disables client's strict host key
+  checking making it auto-accept new host keys.  Existing host keys will
+  still be validated.
+  
+  
+  TLS: If set to a non-zero value, this disables client checks of the
   server's certificate.  Note that to disable server checks of
   the client's certificate or IP address you must
   change the libvirtd
diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
index 5c0457e..6921c15 100644
--- a/src/remote/remote_driver.c
+++ b/src/remote/remote_driver.c
@@ -571,6 +571,7 @@ doRemoteOpen (virConnectPtr conn,
 command,
 username,
 no_tty,
+no_verify,
 netcat ? netcat : "nc",
 sockname)))
 goto failed;
diff --git a/src/rpc/virnetclient.c b/src/rpc/virnetclient.c
index 6a112ee..b9f0fc8 100644
--- a/src/rpc/virnetclient.c
+++ b/src/rpc/virnetclient.c
@@ -187,12 +187,13 @@ virNetClientPtr virNetClientNewSSH(const char *nodename,
const char *binary,
const char *username,
bool noTTY,
+   bool noVerify,
const char *netcat,
const char *path)
 {
 virNetSocketPtr sock;
 
-if (virNetSocketNewConnectSSH(nodename, service, binary, username, noTTY, 
netcat, path, &sock) < 0)
+if (virNetSocketNewConnectSSH(nodename, service, binary, username, noTTY, 
noVerify, netcat, path, &sock) < 0)
 return NULL;
 
 return virNetClientNew(sock, NULL);
diff --git a/src/rpc/virnetclient.h b/src/rpc/virnetclient.h
index de0782c..6acdf50 100644
--- a/src/rpc/virnetclient.h
+++ b/src/rpc/virnetclient.h
@@ -44,6 +44,7 @@ virNetClientPtr virNetClientNewSSH(const char *nodename,
const char *binary,
const char *username,
bool noTTY,
+   bool noVerify,
const char *netcat,
const char *path);
 
diff --git a/src/rpc/virnetsocket.c b/src/rpc/virnetsocket.c
index 3392047..41d9954 100644
--- a/src/rpc/virnetsocket.c
+++ b/src/rpc/virnetsocket.c
@@ -576,6 +576,7 @@ int virNetSocketNewConnectSSH(const char *nodename,
   const char *binary,
   const char *username,
   bool noTTY,
+  bool noVerify,
   const char *netcat,
   const char *path,
   virNetSocketPtr *retsock)
@@ -596,6 +597,8 @@ int virNetSocketNewConnectSSH(const char *nodename,
 if (noTTY)
 virCommandAddArgList(cmd, "-T", "-o", "BatchMode=yes",
  "-e", "none", NULL);
+if (noVerify)
+virCommandAddArgList(cmd, "-o", "StrictHostKeyChecking=no", NULL);
 virCommandAddArgList(cmd, nodename,
  netcat ? netcat : "nc",
  "-U", path, NULL);
diff --git a/src/rpc/virnetsocket.h b/src/rpc/virnetsocket.h
index 356d6c6..5f882ac 100644
--- a/src/rpc/virnetsocket.h
+++ b/src/rpc/virnetsocket.h
@@ -67,6 +67,7 @@ int virNetSocketNewConnectSSH(const char *nodename,
   const char *binary,
   const char *username,
   bool noTTY,
+  bool noVerify,
   const char *netcat,
   const char *path,
   virNetSocketPtr *addr);
diff --git a/tests/virnetsockettest.c b/tes

[libvirt] ANNOUNCE: libvirt Perl bindings Sys::Virt 0.9.2 release

[Daniel P. Berrange]

I am happy to announce a new release[1] of the libvirt Perl bindings,
Sys::Virt 0.9.2 is available for download:

  http://search.cpan.org/CPAN/authors/id/D/DA/DANBERR/Sys-Virt-0.9.2.tar.gz

Changes in this release

 - Changed version numbering to track the corresponding
   minimum required libvirt releases
 - Add all new APIs in libvirt 0.9.2
 - Requires libvirt >= 0.9.2

The permanent link for the Sys::Virt package, where all historic releases
can be found, is

  http://search.cpan.org/dist/Sys-Virt/

Thanks to everyone who contributed to this release through testing, bug
reporting, patch submission, etc

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

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

Re: [libvirt] [RFC] exporting KVM host power saving capabilities through libvirt

[Vaidyanathan Srinivasan]

* Dave Allan  [2011-07-08 17:22:53]:

> On Mon, Jul 04, 2011 at 10:05:21PM +0530, Vaidyanathan Srinivasan wrote:
> > * Dave Allan  [2011-07-01 16:56:29]:
> > 
> >  
> > > > libvirt has virConnectGetCapabilities() that would export an XML file
> > > > describing the capabilities of the host platform and guest features.
> > > > 
> > > > KVM hypervisor's capability to support S3 can be exported as a host
> > > > feature in the XML as follows:
> > > > 
> > > > 
> > > > 94a3492f-2635-2491-8c87-8de976fad119
> > > > 
> > > >   x86_64
> > > >   <<<=== New host feature fields
> > > > 
> > > > 
> > > >   
> > > 
> > > Just my $.02, but calling it  seems to be confusingly close
> > > to the existing  tag.  Maybe  ?
> > 
> > Yes, I am open to naming the tag power_management.  Any other similar
> > attributes that could be included here in future? Would calling it
> > power_management be very restrictive?
> 
> Urgh, busy week, sorry to be slow responding; I'd expect there will be
> other features you'd want to describe, but they can have their own
> tags.

Sounds good.  Let me add a  tag within host and
perhaps outside of  since this is system attribute and not a CPU
attribute.

--Vaidy


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

Re: [libvirt] [RFC] exporting KVM host power saving capabilities through libvirt

[Vaidyanathan Srinivasan]

* Dave Allan  [2011-07-08 17:23:31]:

> On Mon, Jul 04, 2011 at 10:00:20PM +0530, Vaidyanathan Srinivasan wrote:
> > * Dave Allan  [2011-07-01 17:19:06]:
> > 
> > > On Fri, Jul 01, 2011 at 04:56:29PM -0400, Dave Allan wrote:
> > > > On Tue, Jun 28, 2011 at 08:55:31PM +0530, Vaidyanathan Srinivasan wrote:
> > > > > Hi,
> > > > > 
> > > > > Linux host systems running KVM support various power management
> > > > > capabilities.  Most of the features like DVFS and sleep states can be
> > > > > independently exploited by the host system itself based on system
> > > > > utilisation subject to policies set by the administrator.
> > > > > 
> > > > > However, system-wide low power states like S3 and S4 would require
> > > > > external communication and interaction with the systems management
> > > > > stack in order to be used.  The first steps in this direction would be
> > > > > to allow systems management stack to discover host power saving
> > > > > capabilities like S3 and S4 along with various other host CPU
> > > > > capabilities.
> > > > > 
> > > > > Libvirt seems to be the main glue layer between the platform and the
> > > > > systems-management stack.  Adding host power savings capabilities as
> > > > > part of libvirt host discovery mechanism seems to be one possible
> > > > > approach without addition of any new APIs or agents.
> > > > 
> > > > Can you provide the use cases you're looking to address with this
> > > > work?
> > > 
> > > BTW, I'm intrigued by what you might be doing here, but I don't feel
> > > like I have enough information at this point to know quite what to
> > > think of it.
> > 
> > Hi Dave,
> > 
> > Thanks for taking a look.  There are several advantages for using S3
> > or S4 state for the host system instead power 'off' and 'on' for
> > starting new guest instances.  The main advantage of discovering the
> > capability through the systems management stack is to get the
> > capability reported after checking against all policies and software
> > compatibilities.  The overall idea is to allow high level systems
> > management software that work through libvirt to discover and exploit
> > these capabilities wherever possible.
> > 
> > Discovery of available power savings features is only the first step
> > and I am looking for ideas to export these capabilities through
> > libvirt.  I would expect the management stack that collects host
> > capabilities would be keep these feature list and could use them to
> > make various optimization decisions.
> 
> That certainly sounds useful to me.

Let me get the next iteration of the patch out with working code.
This will allow us to review the implementation in more details.

Thanks,
Vaidy



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

Re: [libvirt] [PATCH] Add domain type checking

[Matthias Bolte]

2011/7/11 Eric Blake :
> On 07/11/2011 10:16 AM, Matthias Bolte wrote:
>>> > My only regret here is that we can't really suggest the value expected
>>> > because QEmu accepts more than one, but for other drivers we should be
>>> > able to provide what type is expected.
>> Yes, we can do that even for QEMU. See attached diff between v2 and v3
>> for easier review.
>
>> +++ b/src/conf/domain_conf.c
>> @@ -29,6 +29,7 @@
>>  #include 
>>  #include 
>>  #include 
>> +#include 
>
> What was this needed for?

For log2 to convert 1 << x back to x in case only one bit is set in
expectedVirtTypes.

>> @@ -5846,10 +5848,42 @@ static virDomainDefPtr 
>> virDomainDefParseXML(virCapsPtr caps,
>>      }
>>      VIR_FREE(tmp);
>>
>> -    if (((1 << def->virtType) & expectedVirtTypes) == 0) {
>> -        virDomainReportError(VIR_ERR_INTERNAL_ERROR,
>> -                             _("unexpected domain type %s"),
>> -                             virDomainVirtTypeToString(def->virtType));
>> +    if ((expectedVirtTypes & (1 << def->virtType)) == 0) {
>> +        if (count_one_bits(expectedVirtTypes) == 1) {
>> +            virDomainReportError(VIR_ERR_INTERNAL_ERROR,
>> +                                 _("unexpected domain type %s, expecting 
>> %s"),
>
> I like it.  ACK to this difference.

So I have a completely ACKed v3 and pushed it, thanks.

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

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

Re: [libvirt] [PATCH] qemu: Free previous error

[Daniel P. Berrange]

On Mon, Jul 11, 2011 at 07:02:35PM +0200, Michal Privoznik wrote:
> virCopyLastError simply overwrites destination, which may lead to leak
> if there already was error. Therefore we need first to free destination.
> ---
>  src/qemu/qemu_migration.c |1 +
>  src/qemu/qemu_monitor.c   |1 +
>  2 files changed, 2 insertions(+), 0 deletions(-)
> 
> diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
> index 3e4f4fe..56dd26b 100644
> --- a/src/qemu/qemu_migration.c
> +++ b/src/qemu/qemu_migration.c
> @@ -1586,6 +1586,7 @@ static void qemuMigrationIOFunc(void *arg)
>  return;
>  
>  error:
> +virResetError(&data->err);
>  virCopyLastError(&data->err);
>  virResetLastError();
>  }
> diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
> index 8573262..66a4e48 100644
> --- a/src/qemu/qemu_monitor.c
> +++ b/src/qemu/qemu_monitor.c
> @@ -603,6 +603,7 @@ qemuMonitorIO(int watch, int fd, int events, void 
> *opaque) {
>  if (!err)
>  qemuReportError(VIR_ERR_INTERNAL_ERROR,
>  _("Error while processing monitor IO"));
> +virResetError(&mon->lastError);
>  virCopyLastError(&mon->lastError);
>  virResetLastError();
>  }

NACK, neither of these are required.

There is only one place in qemuMigrationIOFunc which sets data->err
and that is the one you're modifying. So it is impossible for data->err
to have an existing error.

The code changed in qemuMonitorIO has already validated that mon->lastError
is not set in an if(...) that the diff here doesn't show.

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

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

[libvirt] [PATCH] domain_conf: Free temporary variable

[Michal Privoznik]

Caller must free returned value of virXPathString
---
 src/conf/domain_conf.c |1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 2315c98..05c9d3e 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -5732,6 +5732,7 @@ virDomainDefParseBootXML(xmlXPathContextPtr ctxt,
 } else {
 def->os.bios.useserial = VIR_DOMAIN_BIOS_USESERIAL_NO;
 }
+VIR_FREE(useserial);
 }
 
 *bootCount = deviceBoot;
-- 
1.7.5.rc3

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

[libvirt] [PATCH] qemu: Free previous error

[Michal Privoznik]

virCopyLastError simply overwrites destination, which may lead to leak
if there already was error. Therefore we need first to free destination.
---
 src/qemu/qemu_migration.c |1 +
 src/qemu/qemu_monitor.c   |1 +
 2 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index 3e4f4fe..56dd26b 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -1586,6 +1586,7 @@ static void qemuMigrationIOFunc(void *arg)
 return;
 
 error:
+virResetError(&data->err);
 virCopyLastError(&data->err);
 virResetLastError();
 }
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index 8573262..66a4e48 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -603,6 +603,7 @@ qemuMonitorIO(int watch, int fd, int events, void *opaque) {
 if (!err)
 qemuReportError(VIR_ERR_INTERNAL_ERROR,
 _("Error while processing monitor IO"));
+virResetError(&mon->lastError);
 virCopyLastError(&mon->lastError);
 virResetLastError();
 }
-- 
1.7.5.rc3

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

Re: [libvirt] [PATCH] Add domain type checking

[Eric Blake]

On 07/11/2011 10:16 AM, Matthias Bolte wrote:
>> > My only regret here is that we can't really suggest the value expected
>> > because QEmu accepts more than one, but for other drivers we should be
>> > able to provide what type is expected.
> Yes, we can do that even for QEMU. See attached diff between v2 and v3
> for easier review.

> +++ b/src/conf/domain_conf.c
> @@ -29,6 +29,7 @@
>  #include 
>  #include 
>  #include 
> +#include 

What was this needed for?

> @@ -5846,10 +5848,42 @@ static virDomainDefPtr 
> virDomainDefParseXML(virCapsPtr caps,
>  }
>  VIR_FREE(tmp);
>  
> -if (((1 << def->virtType) & expectedVirtTypes) == 0) {
> -virDomainReportError(VIR_ERR_INTERNAL_ERROR,
> - _("unexpected domain type %s"),
> - virDomainVirtTypeToString(def->virtType));
> +if ((expectedVirtTypes & (1 << def->virtType)) == 0) {
> +if (count_one_bits(expectedVirtTypes) == 1) {
> +virDomainReportError(VIR_ERR_INTERNAL_ERROR,
> + _("unexpected domain type %s, expecting 
> %s"),

I like it.  ACK to this difference.

-- 
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] Add domain type checking

[Matthias Bolte]

2011/7/11 Daniel Veillard :
> On Sat, Jul 09, 2011 at 03:38:32PM +0200, Matthias Bolte wrote:
>> 2011/7/8 Eric Blake :
>> > On 07/08/2011 02:13 AM, Matthias Bolte wrote:
>> >> The drivers were accepting domain configs without checking if those
>> >> were actually meant for them. For example the LXC driver happily
>> >> accepts configs with type QEMU.
>> >>
>> >> For convenience add an optional check for the domain type for the
>> >> virDomainDefParse* functions. It's optional because in some places
>> >> virDomainDefParse* is used to parse a config without caring about
>> >> it's type. Also the QEMU driver has to accept 4 different types and
>> >> does this checking own it's own.
>> >
>> > Can we factor the 4 QEMU types back into the common method?  Do this by
>> > treating expectedType as a bitmask:
> [...]
>> @@ -5836,6 +5842,13 @@ static virDomainDefPtr 
>> virDomainDefParseXML(virCapsPtr caps,
>>      }
>>      VIR_FREE(tmp);
>>
>> +    if (((1 << def->virtType) & expectedVirtTypes) == 0) {
>> +        virDomainReportError(VIR_ERR_INTERNAL_ERROR,
>> +                             _("unexpected domain type %s"),
>> +                             virDomainVirtTypeToString(def->virtType));
>> +        goto error;
>> +    }
>> +
>
>  Looks fine, ACK
>
> My only regret here is that we can't really suggest the value expected
> because QEmu accepts more than one, but for other drivers we should be
> able to provide what type is expected.

Yes, we can do that even for QEMU. See attached diff between v2 and v3
for easier review.

> Anyway the main error here is when people use qemu instead of kvm and
> end up with a non-accelerated guest and there is nothing we can do there :-\

Yes, because the user might do this on purpose and not by accident.

-- 
Matthias Bolte
http://photron.blogspot.com
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index a784f4d..39ed317 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -29,6 +29,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include "virterror_internal.h"
 #include "datatypes.h"
@@ -48,6 +49,7 @@
 #include "files.h"
 #include "bitmap.h"
 #include "verify.h"
+#include "count-one-bits.h"
 
 #define VIR_FROM_THIS VIR_FROM_DOMAIN
 
@@ -5846,10 +5848,42 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps,
 }
 VIR_FREE(tmp);
 
-if (((1 << def->virtType) & expectedVirtTypes) == 0) {
-virDomainReportError(VIR_ERR_INTERNAL_ERROR,
- _("unexpected domain type %s"),
- virDomainVirtTypeToString(def->virtType));
+if ((expectedVirtTypes & (1 << def->virtType)) == 0) {
+if (count_one_bits(expectedVirtTypes) == 1) {
+virDomainReportError(VIR_ERR_INTERNAL_ERROR,
+ _("unexpected domain type %s, expecting %s"),
+ virDomainVirtTypeToString(def->virtType),
+ virDomainVirtTypeToString(log2(expectedVirtTypes)));
+} else {
+virBuffer buffer = VIR_BUFFER_INITIALIZER;
+char *string;
+
+for (i = 0; i < VIR_DOMAIN_VIRT_LAST; ++i) {
+if ((expectedVirtTypes & (1 << i)) != 0) {
+if (virBufferUse(&buffer) > 0)
+virBufferAddLit(&buffer, ", ");
+
+virBufferAdd(&buffer, virDomainVirtTypeToString(i), -1);
+}
+}
+
+if (virBufferError(&buffer)) {
+virReportOOMError();
+virBufferFreeAndReset(&buffer);
+goto error;
+}
+
+string = virBufferContentAndReset(&buffer);
+
+virDomainReportError(VIR_ERR_INTERNAL_ERROR,
+ _("unexpected domain type %s, "
+   "expecting one of these: %s"),
+ virDomainVirtTypeToString(def->virtType),
+ string);
+
+VIR_FREE(string);
+}
+
 goto error;
 }
 
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 09/10] qemu: use virDomainNetGetActual*() functions where appropriate

[Laine Stump]

On 07/08/2011 04:18 PM, Eric Blake wrote:

On 07/05/2011 01:45 AM, Laine Stump wrote:

The qemu driver accesses fields in the virDomainNetDef directly, but
with the advent of the virDomainActualNetDef, some pieces of
information may be found in a different place (the ActualNetDef) if
the network connection is of type='network' and that network is of
forward type='bridge|private|vepa|passthrough'. The previous patch
added functions to mask this difference from callers - they hide the
decision making process and just pick the value from the proper place.

This patch uses those functions in the qemu driver as a first step in
making qemu work with the new network types. At this point, it's
assumed that the virDomainActualNetDef is already properly initialized
(it isn't yet).

Is this going to bite anyone during bisection of this patch series?


No. I misused the word "initialized" there. I probably should have just 
said "set". The virDomainActualNetDef *is* "properly initialized" to 
NULL, and when it's null, all behavior with this patch in place is 
identical to what it would have been without the patch. What is being 
assumed here is that an ActualNetDef might really be present, but one 
never is, that's all.




Hopefully not, so I'm not sure how much you want to rework this while
rebasing, which means you can probably keep the code as-is.  But my
approach would have been:

patch 1 - introduce wrapper functions that make no semantic change,
while updating all callers to use wrapper functions.  Something like:

int
virDomainNetGetActualType(virDomainNetDefPtr iface)
{
 return iface->type;
}

and replace all uses of iface->type with virDomainNetGetActualType().



We can't replace *all* uses. There are times when we want to know the 
original type.




patch 2 - enhance wrapper functions to actually look into
virDomainActualNetDef, preferably while guaranteeing that it is
initialized correctly

at this stage, you fix the body of virDomainNetGetActualType to:

if (iface->type != VIR_DOMAIN_NET_TYPE_NETWORK ||
 !iface->data.network.actual)
 return iface->type;
return iface->data.network.actual->type;



I don't really see this two stage process (well, it was already 2, and 
this turns it into 3)  as necessary,


because 1) the code that added the ActualNetDef also ensured that it was 
always initialized to NULL, 2) there was no place in the code that 
changed the ActualNetDef, and 3) if the ActualNetDef is NULL, 
virDomainNetGetActualType will *always* return iface->type.





+++ b/src/qemu/qemu_driver.c
@@ -3935,9 +3935,35 @@ static char *qemuDomainXMLToNative(virConnectPtr conn,
  for (i = 0 ; i<  def->nnets ; i++) {
  virDomainNetDefPtr net = def->nets[i];
  int bootIndex = net->bootIndex;
-if (net->type == VIR_DOMAIN_NET_TYPE_NETWORK ||
-net->type == VIR_DOMAIN_NET_TYPE_DIRECT) {
+if (net->type == VIR_DOMAIN_NET_TYPE_NETWORK) {
+int actualType = virDomainNetGetActualType(net);
  VIR_FREE(net->data.network.name);
+VIR_FREE(net->data.network.portgroup);
+if (actualType == VIR_DOMAIN_NET_TYPE_BRIDGE) {

And here is an instance where you are refactoring existing code
(converting net->type to virDomainNetGetActualType(net)) as well as
adding new code (taking action if actualType is TYPE_BRIDGE).
Separating the refactoring from the introduction of new features can
make review a bit easier.



Okay, I can agree with that. I can put that in a different patch.



+char *brname = strdup(virDomainNetGetActualBridgeName(net));
+virDomainActualNetDefFree(net->data.network.actual);
+
+memset(net, 0, sizeof *net);
+
+net->type = VIR_DOMAIN_NET_TYPE_ETHERNET;
+net->data.ethernet.dev = brname;

Need to check for strdup failure, rather than setting dev to NULL.




Yep. I missed that one.

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

Re: [libvirt] [PATCH] remote: Fix memory leak

[Eric Blake]

On 07/10/2011 11:14 PM, a...@redhat.com wrote:
> From: Alex Jia 
> 
> Detected in valgrind run:
> 
> ==9184== 1 bytes in 1 blocks are definitely lost in loss record 1 of 19
> ==9184==at 0x4A04A28: calloc (vg_replace_malloc.c:467)
> ==9184==by 0x3073715F78: xdr_array (xdr_array.c:97)
> ==9184==by 0x4CF97C9: xdr_remote_domain_get_security_label_ret 
> (remote_protocol.c:1696)

> 
> * src/remote/remote_driver.c: Avoid leak on remoteDomainGetSecurityLabel
>   and remoteNodeGetSecurityModel.
> ---
>  src/remote/remote_driver.c |   12 +---
>  1 files changed, 9 insertions(+), 3 deletions(-)

ACK and pushed.  I added you to AUTHORS; let me know if I need to adjust
anything for your preferred spellings.

-- 
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] Disk snapshot mode proposal: patch for storing the snapshot mode from .vmx to .xml

[Eric Blake]

On 07/09/2011 09:52 AM, Matthias Bolte wrote:
> Anyway, you decided to add an snapshot_mode attribute to the disk
> element and exposed the VMX values there. I'm not sure that this is a
> good idea as scsi0:0.mode affects two aspects.
> 
> scsi0:0.mode can basically have three different modes
> 
> - persistent, the default, a disk with this mode will take part in
> snapshots and changes to the disk's content persist domain power
> cycles and snapshot restoring.
> 
> - independent-persistent, a disk with this mode will not take part in
> snapshots, but changes to the disk's content persist domain power
> cycles and snapshot restoring.
> 
> - independent-nonpersistent, a disk with this mode will be not take
> part in snapshots and changes to the disk's content don't persist
> domain power cycles and snapshot restoring. This is realized by
> writing all changes into an additional .vmdk instead of the original
> .vmdk. This additional .vmdk is automatically deleted on domain power
> cycles and snapshot restoring.
> 
> There are two additional but outdated modes undoable and nonpersistent
> that aren't supported anymore.
> 
> So the two aspects scsi0:0.mode affects is snapshot and the
> persistence of changes. I think it makes more sense to use two
> attributes for the disk element to expose this.
> 
> 
> 
> - snapshot=yes persistent=yes maps to scsi0:0.mode=persistent
> 
> - snapshot=yes persistent=no is unsupported for ESX
> 
> - snapshot=no persistent=yes maps to scsi0:0.mode=independent-persistent
> 
> - snapshot=no persistent=no maps to scsi0:0.mode=independent-nonpersistent

Hmm, this indeed seems like it might be reasonable to represent both
aspects in XML.  See also
https://www.redhat.com/archives/libvir-list/2011-May/msg00315.html.

At stake is whether a disk has a snapshot taken by default, and whether
a disk is treated as temporary for the life of the domain (qemu has a
-snapshot command line option that treats all disks as temporary, but
with better per-volume snapshot abilities, libvirt could certainly offer
the same fine-tuning of per-disk as esx appears to offer).

So yes, I'll need to fold something like this into my v2 proposal for
snapshot handling.

> 
> The more important question is: Is this an general concept or is it
> too ESX specific?

It's sounding generic enough that it will be worth getting it right in
the XML.

> 
> Eric is currently discussing/designing an extension to libvirt's
> snapshot/checkpoint capabilities. At the moment libvirt supports
> checkpointing a complete domain including RAM image and all storage
> volumes, but it doesn't support snapshotting of single storage volumes
> or a subset of the storage volumes of a domain.
> 
> https://www.redhat.com/archives/libvir-list/2011-June/msg00761.html
> 
> Eric suggest to extend the  and virStorageVol* APIs to
> allow to include only a subset of the domain's storage volumes in a
> checkpoint. This approach allows to specifiy for each checkpoint which
> storage volumes to include. ESX allows something similar with the
> independent modes, but you cannot define this on a per snapshot basis,
> but have to decided this before. Eric's approach is more flexible but
> doesn't work for ESX. I wonder if we could add the snapshot
> attribute/subelement to the disk element. This allows to set the
> independent mode for ESX and allows to define a preset for other
> hypervisors like QEMU that will support Eric's more flexible approach.
> 
> So when you don't explicitly define which disk to include in a
> checkpoint in the  XML then the snapshot setting from
> the the  XML apply. If there are no presets for snapshot it
> defaults to yes. For QEMU you could override the snapshot setting from
> the  XML in the  XML, for ESX you either don't
> specify this in the  XML or have to match the settings
> from the  XML due to the way ESX works.

Yes, having a per-disk default in the  XML (applicable to both
qemu and ESX), as well as a per-disk override in the 
(here, qemu can take advantage, but ESX would have to fail if the
override is not identical to the domain defaults). does make sense.

> The persistence setting might be more ESX specific, but I think
> libvirt could realize this for QEMU too, when the domain is using
> qcow2 images with a base image. In that case libvirt could clear the
> qcow2 image when the domain is restarted to realize persistent=no. I
> might be incorrect here as I'm not the QEMU expert here.

You're exactly right - qemu can implement per-disk persistent=no by
doing a qcow2 wrapper around just the disks that should be reverted when
the VM stops running.  There might be some interactions with migration
to worry about, though.

> 
> On a second thought we might want to use negative word so we don't add
> subelements for the defaults, for example
> 
> 
>   
>   
>   
>   
>   
> 

Now we're doing a bit of bike-shedding - I think there's definitely
consensus that this has to be in the XML somewhere

Re: [libvirt] [PATCH 1/3] add virCond into _virStoragePoolObj

[Dave Allan]

On Mon, Jul 11, 2011 at 06:11:47PM +0800, Guannan Ren wrote:
> On 07/11/2011 04:49 PM, Jiri Denemark wrote:
> >On Sun, Jul 10, 2011 at 22:29:03 +0800, Guannan Ren wrote:
> >>The three patches add support to delete volumes while it is building intead 
> >>of raising
> >>an warning infomation,then abort.
> >>For the volume of raw file, delete it directly without any effect on the 
> >>thread of creating
> >>it.
> >>For the volume of block, wait for its finish , then delete it.
> >Hmm, shouldn't we cancel volume creation instead of waiting for it to finish?
> >
> >Jirka
>Yes, that is expected, but for volumes in pools of disk and
> logical type, it is
> comparatively safe way to delete after its finish as the
> creation of these volume
> is changing the files system or something low-level.

No, IMO, we really need to figure out if the operation is something
that can be cancelled, and if so, cancel it.  

Dave

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

Re: [libvirt] 1/3 testsuite formatting bugs [was: [PATCH] Skip some xen tests if xend is not running]

[Eric Blake]

On 07/09/2011 09:42 AM, Matthias Bolte wrote:
> True, the current expression doesn't work for counter = 40. You're
> expression fixed this, but no reason for special casing 0 here, as
> modulo on negative values is perfectly fine and yields the right
> result here
> 
>   expr 39 - \( \( 0 - 1 \) % 40 \) is 40
> 
> ACK, to you're equation (but without special casing 0) as I already
> pushed my patch.

Here's what I pushed to clean up this thread.

diff --git c/tests/test-lib.sh w/tests/test-lib.sh
index 527dfda..918bf73 100644
--- c/tests/test-lib.sh
+++ w/tests/test-lib.sh
@@ -54,7 +54,7 @@ test_final()
   status=$2

   if test "$verbose" = "0" ; then
-len=`expr 40 - \( $counter % 40 \)`
+len=`expr 39 - \( \( $counter - 1 \) % 40 \)`
 printf "%${len}s" ""
 if test "$status" = "0" ; then
   printf " %-3d OK\n" $counter
diff --git c/tests/testutils.c w/tests/testutils.c
index c89f70f..ac5d298 100644
--- c/tests/testutils.c
+++ w/tests/testutils.c
@@ -693,9 +693,8 @@ cleanup:
 VIR_FREE(abs_srcdir);
 virResetLastError();
 if (!virTestGetVerbose() && ret != EXIT_AM_SKIP) {
-int i;
-for (i = (testCounter % 40) ; i > 0 && i < 40 ; i++)
-fprintf(stderr, " ");
+if (testCounter == 0 || testCounter % 40)
+fprintf(stderr, "%*s", 40 - (testCounter % 40), "");
 fprintf(stderr, " %-3d %s\n", testCounter, ret == 0 ? "OK" :
"FAIL");
 }
 return ret;

-- 
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] Skip some xen tests if xend is not running

[Jim Fehlig]

Jim Fehlig wrote:
> Eric Blake wrote:
>   
[...]
>> Oh, and our testsuite has a cosmetic bug.  After applying your patch, I
>> see this during 'make check':
>>
>> TEST: xencapstest
>>   ..   10  OK
>> PASS: xencapstest
>> SKIP: reconnect
>> TEST: statstest
>>0   FAIL
>> SKIP: statstest
>>
>> Bonus points for fixing up that output to say SKIP instead of FAIL and
>> to align it correctly (but that can be a separate patch).
>>   
>> 
>
> I'll look into it.
>   

Already fixed I see.  If only some of my downstream work was picked up
by others while away for a few days... :-).

Regards,
Jim

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

Re: [libvirt] [PATCH V2] Do not drop kernel cmdline for xen pv domains

[Jim Fehlig]

Eric Blake wrote:
> On 07/07/2011 05:37 PM, Jim Fehlig wrote:
>   
>> Jim Fehlig wrote:
>> 
>>> Kernel cmdline args can be passed to xen pv domains even when a
>>> bootloader is specified.  The current config-to-sxpr mapping
>>> ignores cmdline when bootloader is present.
>>>
>>> Since the xend sub-driver is used with many xen toolstack versions,
>>> this patch takes conservative approach of adding an else block to
>>> existing !def->os.bootloader, and only appends sxpr if def->os.cmdline
>>> is non-NULL.
>>>
>>> V2: Fix existing testcase broken by this patch and add new testcases
>>>   
>>>   
>> Hmm, now domainschematest is failing
>>
>> on these two xml files. If I'm reading domain.rng correctly, kernel must
>> be specified?? Can it be optional like initrd and cmdline?
>> 
>
> Reading domain_conf.c agrees with that interpretation.  I would be fine
> with you squashing this in:
>
> diff --git i/docs/schemas/domain.rng w/docs/schemas/domain.rng
> index c01801e..b659da9 100644
> --- i/docs/schemas/domain.rng
> +++ w/docs/schemas/domain.rng
> @@ -553,9 +553,11 @@
>
>
>  
> -  
> -
> -  
> +  
> +
> +  
> +
> +  
>
>  
>
>
> ACK, once you fix that and address Matthias' comment about spacing in
> the .xml file:
>   

Squashed in your change to domain.rng, removed spaces noted by Matthias,
and pushed.

Regards,
Jim

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

Re: [libvirt] [PATCH] Name domain managed state file with domain UUID

[Eric Blake]

On 07/11/2011 06:50 AM, Daniel P. Berrange wrote:
>>  
>> -if (virAsprintf(&ret, "%s/%s.save", driver->saveDir, vm->def->name) < 
>> 0) {
>> +if (virAsprintf(&ret, "%s/%s.save", driver->saveDir, uuidstr) < 0) {
>>  virReportOOMError();
>>  return(NULL);
>>  }
> 
> NACK, this is just papering over the problem IMHO, resulting in orphaned
> state files being left around forever. We should have deleted the state
> file when the guest was undefined.

I concur with the NACK.  In fact, since we have the ability to
independently clean up the managed state files via
virDomainManagedSaveRemove, I think that it might even make sense to
have a flag:

virDomainUndefineFlags(,0) - fail if virDomainHasManagedSaveImage
returns true
virDomainUndefineFlags(,VIR_DOMAIN_UNDEFINE_ALL) - force deletion of
managed save file as well (that is, a combination of
virDomainManagedSaveRemove while ignoring failures, then virDomainUndefine)

By doing that, we then guarantee that undefine will fail unless the
managed save files are also cleaned up, at which point keeping the
managed save files around by name is reasonable (name is easier than
uuid for anyone perusing the managed state directory).

Also, is it possible to rename a domain by using virDomainDefine while
reusing the same UUID but a different name of an already defined VM?  If
so, then that needs to take care of renaming any associated files, such
as existing managed save state.  In fact, it might be worth providing a
convenience API of virDomainRename that makes it easier to rename
existing domains.


To some degree, I have the same issue at hand with my proposed API for
storage volume snapshot management - cleanup really does need a flag to
state whether snapshots are cleaned up or else the snapshot delete fails
because snapshots still exist.

-- 
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] kill vm if saving config failed in v3 protocol

[Wen Congyang]

At 2011年07月11日 17:25, Daniel P. Berrange write:

On Mon, Jul 11, 2011 at 09:25:40AM +0800, Wen Congyang wrote:

At 07/01/2011 06:05 PM, Wen Congyang Write:

If virDomainSaveConfig() failed, we will return NULL to source,
and the vm is still available to restart during confirm() step in
v3 protocol. So we should kill it off in qemuMigrationFinish().

In v2 protocol, we should not set vm to NULL, because we hold
a reference of vm and should unrefernce it.

---
  src/qemu/qemu_migration.c |   21 +++--
  1 files changed, 19 insertions(+), 2 deletions(-)

diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index 800b714..fa98cba 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -2471,7 +2471,7 @@ qemuMigrationFinish(struct qemud_driver *driver,
  if (!virDomainObjIsActive(vm)) {
  qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s",
  _("guest unexpectedly quit"));
-goto cleanup;
+goto endjob;
  }

  qemuMigrationVPAssociatePortProfiles(vm->def);
@@ -2491,7 +2491,24 @@ qemuMigrationFinish(struct qemud_driver *driver,
   * Return a NULL dom pointer, and hope that this is a rare
   * situation and management tools are smart.
   */
-vm = NULL;
+
+/*
+ * In v3 protocol, the source VM is still available to
+ * restart during confirm() step, so we kill it off
+ * now.
+ * In v2 protocol, the source is dead, so we leave
+ * target in paused state, in case admin can fix
+ * things up
+ */
+if (v3proto) {
+qemuProcessStop(driver, vm, 1, VIR_DOMAIN_SHUTOFF_FAILED);
+qemuAuditDomainStop(vm, "failed");
+if (newVM) {
+if (qemuDomainObjEndJob(vm)>  0)
+virDomainRemoveInactive(&driver->domains, vm);
+vm = NULL;
+}
+}
  goto endjob;
  }



ACK


Thanks, pushed.



Daniel


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

Re: [libvirt] [PATCH] fix typo error

[Wen Congyang]

At 2011年07月11日 16:36, Daniel Veillard write:

On Mon, Jul 11, 2011 at 04:26:16PM +0800, Wen Congyang wrote:

---
  docs/news.html.in |2 +-
  1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/docs/news.html.in b/docs/news.html.in
index 6c94afc..625e3b9 100644
--- a/docs/news.html.in
+++ b/docs/news.html.in
@@ -151,7 +151,7 @@ and check thehttp://libvirt.org/git/?p=libvirt.git;a=log";>GIT log
Support automatic creation of leases for disks in sanlock (Daniel P. 
Berrange),
Support loading a configuration file for sanlock plugin (Daniel P. 
Berrange),
Allow per-driver config file for lock manager plugins (Daniel P. 
Berrange),
-  network: add domain to unqualified names defined with (Laine 
Stump),
Convert libvirtd over to the new RPC handling APIs (Daniel P. 
Berrange),
Convert the remote driver to new RPC client APIs (Daniel P. 
Berrange),
Add XDR_CFLAGS to libvirt-net-rpc.la library (Daniel P. Berrange),


   Argh , yes please push :-) ACK


Thanks, pushed.



Daniel



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

Re: [libvirt] [PATCH] RPC: fix argument's name

[Wen Congyang]

At 2011年07月11日 17:25, Daniel P. Berrange write:

On Mon, Jul 11, 2011 at 04:26:34PM +0800, Wen Congyang wrote:

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

diff --git a/src/rpc/virnetsocket.c b/src/rpc/virnetsocket.c
index 4b0c2ee..3392047 100644
--- a/src/rpc/virnetsocket.c
+++ b/src/rpc/virnetsocket.c
@@ -1042,8 +1042,8 @@ int virNetSocketAccept(virNetSocketPtr sock, 
virNetSocketPtr *clientsock)
  }


-static void virNetSocketEventHandle(int fd ATTRIBUTE_UNUSED,
-int watch ATTRIBUTE_UNUSED,
+static void virNetSocketEventHandle(int watch ATTRIBUTE_UNUSED,
+int fd ATTRIBUTE_UNUSED,
  int events,
  void *opaque)
  {


ACK


Thanks, pushed.



Daniel


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

Re: [libvirt] [PATCH] Name domain managed state file with domain UUID

[Daniel P. Berrange]

On Mon, Jul 11, 2011 at 09:06:05PM +0800, Osier Yang wrote:
> A domain managed state file named with domain name can cause
> problem if the domain is undefined, and a later new domain is
> created with the same name. The new domain will not be able to
> start.
> ---
>  src/libxl/libxl_driver.c |4 +++-
>  src/qemu/qemu_driver.c   |4 +++-
>  2 files changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
> index 586d562..7e4e261 100644
> --- a/src/libxl/libxl_driver.c
> +++ b/src/libxl/libxl_driver.c
> @@ -220,8 +220,10 @@ libxlDoNodeGetInfo(libxlDriverPrivatePtr driver, 
> virNodeInfoPtr info)
>  static char *
>  libxlDomainManagedSavePath(libxlDriverPrivatePtr driver, virDomainObjPtr vm) 
> {
>  char *ret;
> +char uuidstr[VIR_UUID_STRING_BUFLEN];
> +virUUIDFormat(vm->def->uuid, uuidstr);
>  
> -if (virAsprintf(&ret, "%s/%s.save", driver->saveDir, vm->def->name) < 0) 
> {
> +if (virAsprintf(&ret, "%s/%s.save", driver->saveDir, uuidstr) < 0) {
>  virReportOOMError();
>  return NULL;
>  }
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index a05a1ee..db77615 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -2398,8 +2398,10 @@ cleanup:
>  static char *
>  qemuDomainManagedSavePath(struct qemud_driver *driver, virDomainObjPtr vm) {
>  char *ret;
> +char uuidstr[VIR_UUID_STRING_BUFLEN];
> +virUUIDFormat(vm->def->uuid, uuidstr);
>  
> -if (virAsprintf(&ret, "%s/%s.save", driver->saveDir, vm->def->name) < 0) 
> {
> +if (virAsprintf(&ret, "%s/%s.save", driver->saveDir, uuidstr) < 0) {
>  virReportOOMError();
>  return(NULL);
>  }

NACK, this is just papering over the problem IMHO, resulting in orphaned
state files being left around forever. We should have deleted the state
file when the guest was undefined.

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

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

[libvirt] [PATCH] Name domain managed state file with domain UUID

[Osier Yang]

A domain managed state file named with domain name can cause
problem if the domain is undefined, and a later new domain is
created with the same name. The new domain will not be able to
start.
---
 src/libxl/libxl_driver.c |4 +++-
 src/qemu/qemu_driver.c   |4 +++-
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 586d562..7e4e261 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -220,8 +220,10 @@ libxlDoNodeGetInfo(libxlDriverPrivatePtr driver, 
virNodeInfoPtr info)
 static char *
 libxlDomainManagedSavePath(libxlDriverPrivatePtr driver, virDomainObjPtr vm) {
 char *ret;
+char uuidstr[VIR_UUID_STRING_BUFLEN];
+virUUIDFormat(vm->def->uuid, uuidstr);
 
-if (virAsprintf(&ret, "%s/%s.save", driver->saveDir, vm->def->name) < 0) {
+if (virAsprintf(&ret, "%s/%s.save", driver->saveDir, uuidstr) < 0) {
 virReportOOMError();
 return NULL;
 }
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index a05a1ee..db77615 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -2398,8 +2398,10 @@ cleanup:
 static char *
 qemuDomainManagedSavePath(struct qemud_driver *driver, virDomainObjPtr vm) {
 char *ret;
+char uuidstr[VIR_UUID_STRING_BUFLEN];
+virUUIDFormat(vm->def->uuid, uuidstr);
 
-if (virAsprintf(&ret, "%s/%s.save", driver->saveDir, vm->def->name) < 0) {
+if (virAsprintf(&ret, "%s/%s.save", driver->saveDir, uuidstr) < 0) {
 virReportOOMError();
 return(NULL);
 }
-- 
1.7.6

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

[libvirt] [PATCH] Name domain managed state file with domain UUID

[Osier Yang]

A domain managed state file named with domain name can cause
problem if the domain is undefined, and a later new domain is
created with the same name. The new domain will not be able to
start.
---
 src/libxl/libxl_driver.c |4 +++-
 src/qemu/qemu_driver.c   |4 +++-
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 586d562..7e4e261 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -220,8 +220,10 @@ libxlDoNodeGetInfo(libxlDriverPrivatePtr driver, 
virNodeInfoPtr info)
 static char *
 libxlDomainManagedSavePath(libxlDriverPrivatePtr driver, virDomainObjPtr vm) {
 char *ret;
+char uuidstr[VIR_UUID_STRING_BUFLEN];
+virUUIDFormat(vm->def->uuid, uuidstr);
 
-if (virAsprintf(&ret, "%s/%s.save", driver->saveDir, vm->def->name) < 0) {
+if (virAsprintf(&ret, "%s/%s.save", driver->saveDir, uuidstr) < 0) {
 virReportOOMError();
 return NULL;
 }
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index a05a1ee..db77615 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -2398,8 +2398,10 @@ cleanup:
 static char *
 qemuDomainManagedSavePath(struct qemud_driver *driver, virDomainObjPtr vm) {
 char *ret;
+char uuidstr[VIR_UUID_STRING_BUFLEN];
+virUUIDFormat(vm->def->uuid, uuidstr);
 
-if (virAsprintf(&ret, "%s/%s.save", driver->saveDir, vm->def->name) < 0) {
+if (virAsprintf(&ret, "%s/%s.save", driver->saveDir, uuidstr) < 0) {
 virReportOOMError();
 return(NULL);
 }
-- 
1.7.6

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

Re: [libvirt] [PATCH 0/7] Add support for setting QoS

[Michal Privoznik]

On 01.07.2011 09:20, Michal Privoznik wrote:

On 27.06.2011 19:37, Christian Benvenuti (benve) wrote:

-Original Message-
From: Michal Privoznik [mailto:mpriv...@redhat.com]
Sent: Monday, June 27, 2011 8:34 AM
To: Christian Benvenuti (benve)
Cc: Laine Stump; libvir-list@redhat.com
Subject: Re: [libvirt] [PATCH 0/7] Add support for setting QoS

On 24.06.2011 20:00, Christian Benvenuti (benve) wrote:

3) Similarly for macvtaps, will the network-wide

bandwidth

limiting be applied to the physical ethernet device? This would
have the side effect of including host traffic on that interface

in

the bandwidth totals, but I don't see a way around it.

With this patch as-is, shaping rules are applied only when creating

TAP

devices. This mean only network types VIR_DOMAIN_NET_TYPE_NETWORK

and

VIR_DOMAIN_NET_TYPE_BRIDGE.


4) Finally on that topic, what abouts that have a pool

of

physical ethernets to be used macvtap-style? Is there any way we

can

do bandwidth limiting on an aggregated collection of network

interfaces?

Or
will attempts to configure this necessarily result in a "config

not

supported" log message?

Huh, I didn't know it is possible to have a pool of devices within

one

. So in this case, this patch silently does nothing.


The IFB (Intermediate Functional Block) allows you to
configure aggregate QoS on multiple interfaces.

/Chris



Yes, but we would then need to create those IFB devices on the fly
(e.g.
on domain startup) and I don't think there is other way than unloading
and then loading the ifb module (with different parameter) which
however
would break other domains connections. Or am I missing something?


Adding support for dynamic creation/deletion of IFB devices (for example
via netlink) should not be that hard. If that's the only reason for not
using it, I would consider the option of extending the IFB module.

/Chris


Sorry for mystification. Simple reading of source code for iproute2 has
shown ifb module understand netlink interface and thus new ifb device
can be simply created via "ip link add name  type ifb".

So I'll rewrite and post v2.

Michal


Although, allowing to set bandwidth limits per domain as whole (and thus 
use IFB) might be considered as extension to this patch, which can be 
added later. So I'll post a v2 which will not contain IFB feature, but 
will be rebased to current HEAD. I think splitting this huge QoS feature 
in smaller parts might increase chances of getting it in.


Michal

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

Re: [libvirt] [PATCH 1/3] add virCond into _virStoragePoolObj

[Guannan Ren]

On 07/11/2011 04:49 PM, Jiri Denemark wrote:

On Sun, Jul 10, 2011 at 22:29:03 +0800, Guannan Ren wrote:

The three patches add support to delete volumes while it is building intead of 
raising
an warning infomation,then abort.
For the volume of raw file, delete it directly without any effect on the thread 
of creating
it.
For the volume of block, wait for its finish , then delete it.

Hmm, shouldn't we cancel volume creation instead of waiting for it to finish?

Jirka
   Yes, that is expected, but for volumes in pools of disk and 
logical type, it is
comparatively safe way to delete after its finish as the creation 
of these volume

is changing the files system or something low-level.


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

Re: [libvirt] [PATCH v3] bios: Add support for SGA

[Michal Privoznik]

On 08.07.2011 22:46, Eric Blake wrote:

On 07/08/2011 07:48 AM, Michal Privoznik wrote:

This patch creates new  element which, at this time has the only


s/the only/only the/


attribute useserial='yes|no'. This attribute allow users to use
Serial Graphics Adapter and see BIOS messages from the very first moment
domain boots up. Therefore, users can choose boot medium, set PXE, etc.
---
diff to v2:
-move from  to
-include Eric's and Dan's suggestions

diff to v1:
-move from  to  as Dan suggested:
https://www.redhat.com/archives/libvir-list/2011-July/msg00134.html

  docs/formatdomain.html.in |9 ++
  docs/schemas/domain.rng   |   14 +
  src/conf/domain_conf.c|   27 -
  src/conf/domain_conf.h|   13 
  src/qemu/qemu_capabilities.c  |3 ++
  src/qemu/qemu_capabilities.h  |1 +
  src/qemu/qemu_command.c   |   20 +
  tests/qemuxml2argvdata/qemuxml2argv-bios.args |6 
  tests/qemuxml2argvdata/qemuxml2argv-bios.xml  |   39 +
  tests/qemuxml2argvtest.c  |1 +
  10 files changed, 131 insertions(+), 2 deletions(-)
  create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-bios.args
  create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-bios.xml

diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 10d87a9..9cc0bca 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -86,6 +86,7 @@
  
  
  
+

...

@@ -137,6 +138,14 @@
specified, the hypervisor default is used.
Since 0.8.7

+bios
+This element has attributeuseserial  with possible
+valuesyes  orno. It enables or disables
+Serial Graphics Adapter which allows users to see BIOS messages
+on a serial port. Therefore, one need to have


s/need to have/needs to have a/


+serial port  defined.
+Since 0.9.4
+
  




+++ b/src/conf/domain_conf.h
@@ -923,6 +923,18 @@ enum virDomainLifecycleCrashAction {
  VIR_DOMAIN_LIFECYCLE_CRASH_LAST
  };

+enum virDomainBIOSUseserial {
+VIR_DOMAIN_BIOS_USESERIAL_DEFAULT = 0,
+VIR_DOMAIN_BIOS_USESERIAL_YES,
+VIR_DOMAIN_BIOS_USESERIAL_NO
+};
+
+typedef struct _virDomainBIOSDef virDomainBIOSDef;
+typedef virDomainBIOSDef *virDomainBIOSDefPtr;
+struct _virDomainBIOSDef {
+int useserial;
+};
+
  /* Operating system configuration data&  machine / arch */
  typedef struct _virDomainOSDef virDomainOSDef;
  typedef virDomainOSDef *virDomainOSDefPtr;
@@ -942,6 +954,7 @@ struct _virDomainOSDef {
  char *bootloader;
  char *bootloaderArgs;
  int smbios_mode;
+virDomainBIOSDef bios;


I'm wondering if we could just have done 'int bios_serial' here, instead
of creating the intermediate type _virDomainBIOSDef.  I guess if we ever
add more attributes or subelements to, then the struct will be
nice, but until then it seems a bit heavyweight.  But what you have is
not wrong, so no change necessary.

ACK.



Thanks pushed.

Michal

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

Re: [libvirt] Exposing qemu support for SDL via capabilities

[Daniel P. Berrange]

On Fri, Jul 08, 2011 at 09:19:46AM -0500, Adam Litke wrote:
> Hi all,
> 
> In order to nicely support domains that use qemu's SDL support,
> libvirt-cim is looking for a way to confirm if the underlying qemu
> emulator can support SDL.  Libvirt already knows this information
> internally.  It seems to me that the best way to provide this
> information is by reporting it as a guest feature via the capabilities
> API call.  I was thinking the node '/capabilities/guest/features/sdl'
> could be added when qemu supports SDL.
> 
> Is this a good idea?

Yes & no. I think in general it would be desirable to expose more info
about supported features, but this is really opening a can of worms
because figuring out just what info needs exposing & in what format
is non-trivial. Even QEMU itself still can't provide this info in any
usable fashion, hence we're still parsing -help which really sucks.
Just adding an '/sdl' element seems simple but I don't think it is
a long term sustainable strategy.

If we added info about all supported features to the capabilities XML,
we will end up with an absolutely enourmous XML document that will no
longer be practically useful to administrators because it will be 99%
irrelevant info about devices hiding the 1% they really want about the
supported virt options.

So if we are to expose info about supported device model properties
then I think it would have to be in a separate API / XML schema, and
I think we want to get QEMU to provide the data in a more usable
manner than currently.

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

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

Re: [libvirt] [PATCH 05/14] Rewrite pidfile handling to be crash safe

[Daniel P. Berrange]

On Fri, Jul 08, 2011 at 04:33:28PM -0600, Eric Blake wrote:
> On 07/07/2011 08:17 AM, Daniel P. Berrange wrote:
> > From: "Daniel P. Berrange" 
> > 
> > If libvirtd crashes then the pidfile may not be cleaned up,
> > making a later restart fail, even though the original process
> > no longer exists.
> > 
> > Instead of simply using file creation as the test for successful
> > pidfile acquisition, actually take out a lock on the pidfile.
> > 
> > To avoid races, after locking the pidfile, verify that the
> > inode hasn't changed.
> > 
> > Also make sure the unprivileged libvirtd now acquires the
> > pidfile, instead of relying on the UNIX socket to ensure
> > mutual exclusion
> 
> Cool idea.
> 
> > -static int daemonWritePidFile(const char *pidFile, const char *argv0)
> > -{
> > +static int
> > +daemonAcquirePidFile(const char *argv0, const char *pidFile) {
> 
> Why'd you hoist the { to the previous line?  Our convention has been
> (slowly) leaning towards a function body starting on column 1.
> 
> >  int fd;
> > -FILE *fh;
> >  char ebuf[1024];
> > +unsigned long long pid = (unsigned long long)getpid();
> 
> See my comments in an earlier thread about our existing assumptions that
> pid_t fits in int.  In fact, I would be okay with:
> 
> verify(sizeof(pid_t) <= sizeof(int));
> 
> int pid = getpid();
> char pidstr[INT_BUFSIZE_BOUND(pid)];
> 
> ...
> snprintf(pidstr, sizeof(pidstr), "%u", pid);
> 
> 
> > +char pidstr[INT_BUFSIZE_BOUND(pid)];
> >  
> >  if (pidFile[0] == '\0')
> >  return 0;
> >  
> > -if ((fd = open(pidFile, O_WRONLY|O_CREAT|O_EXCL, 0644)) < 0) {
> > -VIR_ERROR(_("Failed to open pid file '%s' : %s"),
> > -  pidFile, virStrerror(errno, ebuf, sizeof ebuf));
> > -return -1;
> > -}
> > +while (1) {
> > +struct stat a, b;
> > +if ((fd = open(pidFile, O_WRONLY|O_CREAT, 0644)) < 0) {
> > +VIR_ERROR(_("%s: Failed to open pid file '%s' : %s"),
> > +  argv0, pidFile, virStrerror(errno, ebuf, sizeof 
> > ebuf));
> > +return -1;
> > +}
> > +
> > +if (fstat(fd, &b) < 0) {
> > +VIR_ERROR(_("%s: Pid file '%s' disappeared: %s"),
> 
> Misleading error message.  fstat can indeed fail (although such failures
> are rare), but not because a file disappeared - after all, you have an
> fd open to the file.
> 
> > -if (fprintf(fh, "%lu\n", (unsigned long)getpid()) < 0) {
> > -VIR_ERROR(_("%s: Failed to write to pid file '%s' : %s"),
> > -  argv0, pidFile, virStrerror(errno, ebuf, sizeof ebuf));
> > -VIR_FORCE_FCLOSE(fh);
> > -return -1;
> > -}
> > +snprintf(pidstr, sizeof(pidstr), "%llu", pid);
> >  
> > -if (VIR_FCLOSE(fh) == EOF) {
> > -VIR_ERROR(_("%s: Failed to close pid file '%s' : %s"),
> > -  argv0, pidFile, virStrerror(errno, ebuf, sizeof ebuf));
> > -return -1;
> > +if (safewrite(fd, pidstr, strlen(pidstr)) < 0) {
> 
> Nice conversion from FILE* to write().
> 
> > @@ -1436,6 +1453,12 @@ int main(int argc, char **argv) {
> >  umask(old_umask);
> >  }
> >  
> > +/* Try to claim the pidfile, existing if we can't */
> 
> s/existing/exiting/
> 
> > +if ((pid_file_fd = daemonAcquirePidFile(argv[0], pid_file)) < 0) {
> > +ret = VIR_DAEMON_ERR_PIDFILE;
> > +goto cleanup;
> > +}
> > +
> >  if (!(srv = virNetServerNew(config->min_workers,
> >  config->max_workers,
> >  config->max_clients,
> > @@ -1570,8 +1593,10 @@ cleanup:
> >  }
> >  VIR_FORCE_CLOSE(statuswrite);
> >  }
> > -if (pid_file)
> > -unlink (pid_file);
> > +if (pid_file_fd != -1) {
> > +unlink(pid_file);
> > +VIR_FORCE_CLOSE(pid_file_fd);
> 
> Swap these two lines.  Not that flock (or even libvirtd) works on mingw,
> but mingw doesn't like unlink() on an open fd.

If you swap those two lines you open up a (small) race condition
in the locking scheme where you could unlink a pidfile that has now
been claimed by someone else. By unlinking while we still have it
open, we know we still hold the fcntl() lock.

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

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

Re: [libvirt] [PATCH] RPC: fix argument's name

[Daniel P. Berrange]

On Mon, Jul 11, 2011 at 04:26:34PM +0800, Wen Congyang wrote:
> ---
>  src/rpc/virnetsocket.c |4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/src/rpc/virnetsocket.c b/src/rpc/virnetsocket.c
> index 4b0c2ee..3392047 100644
> --- a/src/rpc/virnetsocket.c
> +++ b/src/rpc/virnetsocket.c
> @@ -1042,8 +1042,8 @@ int virNetSocketAccept(virNetSocketPtr sock, 
> virNetSocketPtr *clientsock)
>  }
>  
>  
> -static void virNetSocketEventHandle(int fd ATTRIBUTE_UNUSED,
> -int watch ATTRIBUTE_UNUSED,
> +static void virNetSocketEventHandle(int watch ATTRIBUTE_UNUSED,
> +int fd ATTRIBUTE_UNUSED,
>  int events,
>  void *opaque)
>  {

ACK

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

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

Re: [libvirt] [PATCH] kill vm if saving config failed in v3 protocol

[Daniel P. Berrange]

On Mon, Jul 11, 2011 at 09:25:40AM +0800, Wen Congyang wrote:
> At 07/01/2011 06:05 PM, Wen Congyang Write:
> > If virDomainSaveConfig() failed, we will return NULL to source,
> > and the vm is still available to restart during confirm() step in
> > v3 protocol. So we should kill it off in qemuMigrationFinish().
> > 
> > In v2 protocol, we should not set vm to NULL, because we hold
> > a reference of vm and should unrefernce it.
> > 
> > ---
> >  src/qemu/qemu_migration.c |   21 +++--
> >  1 files changed, 19 insertions(+), 2 deletions(-)
> > 
> > diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
> > index 800b714..fa98cba 100644
> > --- a/src/qemu/qemu_migration.c
> > +++ b/src/qemu/qemu_migration.c
> > @@ -2471,7 +2471,7 @@ qemuMigrationFinish(struct qemud_driver *driver,
> >  if (!virDomainObjIsActive(vm)) {
> >  qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> >  _("guest unexpectedly quit"));
> > -goto cleanup;
> > +goto endjob;
> >  }
> >  
> >  qemuMigrationVPAssociatePortProfiles(vm->def);
> > @@ -2491,7 +2491,24 @@ qemuMigrationFinish(struct qemud_driver *driver,
> >   * Return a NULL dom pointer, and hope that this is a rare
> >   * situation and management tools are smart.
> >   */
> > -vm = NULL;
> > +
> > +/*
> > + * In v3 protocol, the source VM is still available to
> > + * restart during confirm() step, so we kill it off
> > + * now.
> > + * In v2 protocol, the source is dead, so we leave
> > + * target in paused state, in case admin can fix
> > + * things up
> > + */
> > +if (v3proto) {
> > +qemuProcessStop(driver, vm, 1, 
> > VIR_DOMAIN_SHUTOFF_FAILED);
> > +qemuAuditDomainStop(vm, "failed");
> > +if (newVM) {
> > +if (qemuDomainObjEndJob(vm) > 0)
> > +virDomainRemoveInactive(&driver->domains, vm);
> > +vm = NULL;
> > +}
> > +}
> >  goto endjob;
> >  }
> >  

ACK

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

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

[libvirt] [BUG] event's bug?

[Wen Congyang]

Steps to produce this bug:
1. # virsh migrate vm1 --p2p qemu+tls:///system
   error: End of file while reading data: Input/output error

Now the libvirtd crashed.

This bug only happened twice.

I use gdb to analyze the core file:
(gdb) info threads 
  6 Thread 3952  0x00351fe0b43c in pthread_cond_wait@@GLIBC_2.3.2 () from 
/lib64/libpthread.so.0
  5 Thread 3951  0x00351fe0b43c in pthread_cond_wait@@GLIBC_2.3.2 () from 
/lib64/libpthread.so.0
  4 Thread 3950  0x00351fe0b43c in pthread_cond_wait@@GLIBC_2.3.2 () from 
/lib64/libpthread.so.0
  3 Thread 3949  0x00351fe0b43c in pthread_cond_wait@@GLIBC_2.3.2 () from 
/lib64/libpthread.so.0
  2 Thread 3948  0x00351fe0b43c in pthread_cond_wait@@GLIBC_2.3.2 () from 
/lib64/libpthread.so.0
* 1 Thread 3947  0x in ?? ()
(gdb) bt
#0  0x in ?? ()
#1  0x003f2a834150 in _gnutls_string_resize (dest=0x7f92740079d8, 
new_size=) at gnutls_str.c:192
#2  0x003f2a81a614 in _gnutls_io_read_buffered (session=0x7f9274006d30, 
iptr=0x7fffb3476148, sizeOfPtr=5, recv_type=) at 
gnutls_buffers.c:515
#3  0x003f2a816031 in _gnutls_recv_int (session=0x7f9274006d30, 
type=GNUTLS_APPLICATION_DATA, htype=4294967295, data=0x7f92740155e8 "", 
sizeofdata=4) at gnutls_record.c:904
#4  0x7f9285bd2ec7 in virNetTLSSessionRead (sess=0x7f927400a5d0, 
buf=0x7f92740155e8 "", len=4) at rpc/virnettlscontext.c:812
#5  0x7f9285bd50a4 in virNetSocketReadWire (sock=0x7f9274006ba0, 
buf=0x7f92740155e8 "", len=4) at rpc/virnetsocket.c:801
#6  0x7f9285bd5815 in virNetSocketRead (sock=0x7f9274006ba0, 
buf=0x7f92740155e8 "", len=4) at rpc/virnetsocket.c:981
#7  0x7f9285bce40f in virNetClientIOReadMessage (client=0x7f9274015590) at 
rpc/virnetclient.c:711
#8  0x7f9285bce461 in virNetClientIOHandleInput (client=0x7f9274015590) at 
rpc/virnetclient.c:730
#9  0x7f9285bcf0f4 in virNetClientIncomingEvent (sock=0x7f9274006ba0, 
events=1, opaque=0x7f9274015590) at rpc/virnetclient.c:1119
#10 0x7f9285bd5b87 in virNetSocketEventHandle (fd=13, watch=20, events=1, 
opaque=0x7f9274006ba0) at rpc/virnetsocket.c:1052
#11 0x7f9285b09325 in virEventPollDispatchHandles (nfds=10, fds=0xfe51d0) 
at util/event_poll.c:469
#12 0x7f9285b09a7e in virEventPollRunOnce () at util/event_poll.c:610
#13 0x7f9285b07ec5 in virEventRunDefaultImpl () at util/event.c:247
#14 0x00449cc2 in virNetServerRun (srv=0xfc3490) at 
rpc/virnetserver.c:662
#15 0x0041e3c5 in main (argc=2, argv=0x7fffb3476b68) at libvirtd.c:1561

The debug log in /var/log/libvirt/libvirtd.log:
...
11:18:27.838: 1848: debug : virEventPollRemoveHandle:171 : Remove handle w=13
11:18:27.838: 1847: debug : virEventPollDispatchHandles:454 : i=3 w=4
11:18:27.838: 1847: debug : virEventPollDispatchHandles:454 : i=4 w=5
11:18:27.838: 1847: debug : virEventPollDispatchHandles:454 : i=5 w=6
11:18:27.838: 1847: debug : virEventPollDispatchHandles:454 : i=6 w=12
11:18:27.838: 1847: debug : virEventPollDispatchHandles:454 : i=7 w=13
11:18:27.838: 1847: debug : virEventPollDispatchHandles:467 : Dispatch n=7 f=20 
w=13 e=1 0x7f00780aaa80
11:18:27.838: 1848: debug : virEventPollRemoveHandle:184 : mark delete 7 20
11:18:27.838: 1848: debug : virEventPollInterruptLocked:677 : Interrupting
11:18:27.838: 1848: debug : virNetSocketFree:627 : sock=0x7f00780aaa80 fd=20
11:18:27.838: 1848: debug : virEventPollRemoveTimeout:276 : Remove timer 10
11:18:27.838: 1848: debug : virEventPollInterruptLocked:677 : Interrupting
11:18:27.838: 1848: debug : virDomainObjUnref:1142 : obj=0x7f007800ce10 refs=2
11:18:27.838: 1848: debug : virDomainObjUnref:1142 : obj=0x7f007800ce10 refs=1


 == end of log =

The reason is that: we dispatch handle(fd = 20, watch=13) and remove the 
handle(watch=13) almost
at the same time(The order is: dispatch, remove)
We remove the handle when remote connection is closed, and we will call 
virNetSocketFree() to free sock,
but we still use sock in another thread. It's very dangerous!!!

I think we should wait dispatching a handle when removing the same handle.

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

Re: [libvirt] [PATCH 1/3] add virCond into _virStoragePoolObj

[Jiri Denemark]

On Sun, Jul 10, 2011 at 22:29:03 +0800, Guannan Ren wrote:
> The three patches add support to delete volumes while it is building intead 
> of raising
> an warning infomation,then abort.
> For the volume of raw file, delete it directly without any effect on the 
> thread of creating
> it.
> For the volume of block, wait for its finish , then delete it.

Hmm, shouldn't we cancel volume creation instead of waiting for it to finish?

Jirka

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

Re: [libvirt] [PATCH] fix typo error

[Daniel Veillard]

On Mon, Jul 11, 2011 at 04:26:16PM +0800, Wen Congyang wrote:
> ---
>  docs/news.html.in |2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/docs/news.html.in b/docs/news.html.in
> index 6c94afc..625e3b9 100644
> --- a/docs/news.html.in
> +++ b/docs/news.html.in
> @@ -151,7 +151,7 @@ and check the  href="http://libvirt.org/git/?p=libvirt.git;a=log";>GIT log
>Support automatic creation of leases for disks in sanlock (Daniel P. 
> Berrange),
>Support loading a configuration file for sanlock plugin (Daniel P. 
> Berrange),
>Allow per-driver config file for lock manager plugins (Daniel P. 
> Berrange),
> -  network: add domain to unqualified names defined with  (Laine Stump),
> +  network: add domain to unqualified names defined with  
> (Laine Stump),
>Convert libvirtd over to the new RPC handling APIs (Daniel P. 
> Berrange),
>Convert the remote driver to new RPC client APIs (Daniel P. 
> Berrange),
>Add XDR_CFLAGS to libvirt-net-rpc.la library (Daniel P. Berrange),

  Argh , yes please push :-) ACK

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] fix typo error

[Wen Congyang]

---
 docs/news.html.in |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/docs/news.html.in b/docs/news.html.in
index 6c94afc..625e3b9 100644
--- a/docs/news.html.in
+++ b/docs/news.html.in
@@ -151,7 +151,7 @@ and check the http://libvirt.org/git/?p=libvirt.git;a=log";>GIT log
   Support automatic creation of leases for disks in sanlock (Daniel P. 
Berrange),
   Support loading a configuration file for sanlock plugin (Daniel P. 
Berrange),
   Allow per-driver config file for lock manager plugins (Daniel P. 
Berrange),
-  network: add domain to unqualified names defined with  
(Laine Stump),
   Convert libvirtd over to the new RPC handling APIs (Daniel P. 
Berrange),
   Convert the remote driver to new RPC client APIs (Daniel P. 
Berrange),
   Add XDR_CFLAGS to libvirt-net-rpc.la library (Daniel P. Berrange),
-- 
1.7.1

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

[libvirt] [PATCH] RPC: fix argument's name

[Wen Congyang]

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

diff --git a/src/rpc/virnetsocket.c b/src/rpc/virnetsocket.c
index 4b0c2ee..3392047 100644
--- a/src/rpc/virnetsocket.c
+++ b/src/rpc/virnetsocket.c
@@ -1042,8 +1042,8 @@ int virNetSocketAccept(virNetSocketPtr sock, 
virNetSocketPtr *clientsock)
 }
 
 
-static void virNetSocketEventHandle(int fd ATTRIBUTE_UNUSED,
-int watch ATTRIBUTE_UNUSED,
+static void virNetSocketEventHandle(int watch ATTRIBUTE_UNUSED,
+int fd ATTRIBUTE_UNUSED,
 int events,
 void *opaque)
 {
-- 
1.7.1

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

Re: [libvirt] [PATCH] Add domain type checking

[Daniel Veillard]

On Sat, Jul 09, 2011 at 03:38:32PM +0200, Matthias Bolte wrote:
> 2011/7/8 Eric Blake :
> > On 07/08/2011 02:13 AM, Matthias Bolte wrote:
> >> The drivers were accepting domain configs without checking if those
> >> were actually meant for them. For example the LXC driver happily
> >> accepts configs with type QEMU.
> >>
> >> For convenience add an optional check for the domain type for the
> >> virDomainDefParse* functions. It's optional because in some places
> >> virDomainDefParse* is used to parse a config without caring about
> >> it's type. Also the QEMU driver has to accept 4 different types and
> >> does this checking own it's own.
> >
> > Can we factor the 4 QEMU types back into the common method?  Do this by
> > treating expectedType as a bitmask:
[...]
> @@ -5836,6 +5842,13 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr 
> caps,
>  }
>  VIR_FREE(tmp);
>  
> +if (((1 << def->virtType) & expectedVirtTypes) == 0) {
> +virDomainReportError(VIR_ERR_INTERNAL_ERROR,
> + _("unexpected domain type %s"),
> + virDomainVirtTypeToString(def->virtType));
> +goto error;
> +}
> +

  Looks fine, ACK

My only regret here is that we can't really suggest the value expected
because QEmu accepts more than one, but for other drivers we should be
able to provide what type is expected.
Anyway the main error here is when people use qemu instead of kvm and
end up with a non-accelerated guest and there is nothing we can do there :-\

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