Re: [libvirt] [PATCH] (v2) avoid chowning domain devices if higer-level mgmt does it for us

2009-11-25 Thread Daniel Veillard 
On Wed, Nov 25, 2009 at 09:27:13AM +0200, Dan Kenigsberg wrote:
 this is particularily important if said device is a file sitting on a
 root_squashing nfs export.
 
 my previous attempt for a patch missed 3 chowns that should be avoided.
 ---
  src/qemu/qemu.conf |4 
  src/qemu/qemu_conf.c   |3 +++
  src/qemu/qemu_conf.h   |1 +
  src/qemu/qemu_driver.c |8 
  4 files changed, 12 insertions(+), 4 deletions(-)
 
 diff --git a/src/qemu/qemu.conf b/src/qemu/qemu.conf
 index bca858a..892a50b 100644
 --- a/src/qemu/qemu.conf
 +++ b/src/qemu/qemu.conf
 @@ -96,6 +96,10 @@
  # The group ID for QEMU processes run by the system instance
  #group = root
  
 +# should libvirt assume that devices are accessible to the above user:group.
 +# by default, libvirt tries to chown devices before starting up a domain and
 +# restore ownership to root when domain comes down.
 +#assume_devices_accessible = 0
  
  # What cgroup controllers to make use of with QEMU guests
  #
 diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
 index b1b9e5f..520a395 100644
 --- a/src/qemu/qemu_conf.c
 +++ b/src/qemu/qemu_conf.c
 @@ -232,6 +232,9 @@ int qemudLoadDriverConfig(struct qemud_driver *driver,
  return -1;
  }
  
 +p = virConfGetValue (conf, assume_devices_accessible);
 +CHECK_TYPE (assume_devices_accessible, VIR_CONF_LONG);
 +if (p) driver-avoid_dev_chown = p-l;

  an explicit initialization of the field would be better if p is NULL.

  if (virGetGroupID(NULL, group, driver-group)  0) {
  VIR_FREE(group);
 diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h
 index 675c636..3a9da73 100644
 --- a/src/qemu/qemu_conf.h
 +++ b/src/qemu/qemu_conf.h
 @@ -87,6 +87,7 @@ struct qemud_driver {
  
  uid_t user;
  gid_t group;
 +int avoid_dev_chown;
  
  unsigned int qemuVersion;
  int nextvmid;
 diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
 index 2f273eb..5f02aa2 100644
 --- a/src/qemu/qemu_driver.c
 +++ b/src/qemu/qemu_driver.c
 @@ -1968,7 +1968,7 @@ static int qemuDomainSetDeviceOwnership(virConnectPtr 
 conn,
  uid_t uid;
  gid_t gid;
  
 -if (!driver-privileged)
 +if (!driver-privileged || driver-avoid_dev_chown)
  return 0;
  
  /* short circuit case of root:root */
 @@ -2002,7 +2002,7 @@ static int 
 qemuDomainSetAllDeviceOwnership(virConnectPtr conn,
  uid_t uid;
  gid_t gid;
  
 -if (!driver-privileged)
 +if (!driver-privileged || driver-avoid_dev_chown)
  return 0;
  
  /* short circuit case of root:root */
 @@ -3438,7 +3438,7 @@ static int qemudDomainSave(virDomainPtr dom,
  }
  fd = -1;
  
 -if (driver-privileged 
 +if (driver-privileged  !driver-avoid_dev_chown 
  chown(path, driver-user, driver-group)  0) {
  virReportSystemError(NULL, errno,
   _(unable to set ownership of '%s' to user 
 %d:%d),
 @@ -3473,7 +3473,7 @@ static int qemudDomainSave(virDomainPtr dom,
  if (rc  0)
  goto endjob;
  
 -if (driver-privileged 
 +if (driver-privileged  !driver-avoid_dev_chown 
  chown(path, 0, 0)  0) {
  virReportSystemError(NULL, errno,
   _(unable to set ownership of '%s' to user 
 %d:%d),

The core question is having this as another manual user tweak, it always
makes me a bit uncomfortable if proper working of software requires manually
editing a config file. If we really need this kind of options for proper
operations in specific conditions can we make sure it can be set via
APIs too ? I don't think we should expose something as generic as the
internal Conf APIs, but these runtime option in src/qemu/qemu.conf
start to accumulate and I start to wonder how to properly manage all
this.

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] Re: libvir: QEMU error : remoteDomainProcessEvent: unmarshalling msg

2009-11-25 Thread Daniel P. Berrange 
On Wed, Nov 25, 2009 at 10:59:27AM +0800, ajia wrote:
 Hi all,
 
 I create successfully a vm, then opening one terminal to run example 
 event-python.py from libvirt-0.7.4
 source code package with qemu:///system parameter.in another opening 
 terminal,running basic domain
 operation, such as undefine/define/start/destroy.but  original terminal 
 display  information
 libvir: QEMU error : remoteDomainProcessEvent: unmarshalling msg . 
 test environment details is
 as following:
 
 
 [r...@localhost events-python]# uname -a
 Linux localhost.localdomain 2.6.31.5-127.fc12.i686.PAE #1 SMP Sat Nov 7 
 21:25:57 EST 2009 i686 i686 i386 GNU/Linux
 [r...@localhost events-python]# lsmod|grep kvm
 libvirt-0.7.1-15.fc12.i686

I've just confirmed that this version has the bug you see. Can you file
a BZ against Fedora 12 for this problem. 

The latest libvirt-0.7.4  fixes it, so we should backport the fix to
Fedora


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

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

Re: [libvirt] [PATCH] remove ebtables rules at qemud shutdown

2009-11-25 Thread Daniel P. Berrange 
On Tue, Nov 24, 2009 at 10:42:13AM +0100, Gerhard Stenzel wrote:
 This patch removes ebtables rules at qemud shutdown time
 
 Signed-off-by: Gerhard Stenzel gerhard.sten...@de.ibm.com

This is not desirable.  Our expectation is that everything remains
active  unchanged when libvirtd shuts down.  This is neccessary to
allow the libvirtd  daemon to be upgraded without disrupting live
guests.

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

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

Re: [libvirt] crash in libvirtd when assigning PCI device (storage controller)

2009-11-25 Thread Daniel P. Berrange 
On Mon, Nov 23, 2009 at 10:08:17PM +, Terje Marthinussen wrote:
 [  987.286791] libvirtd[3193]: segfault at 10 ip 7f921b4546b4 sp 
 7f9216e165e0 error 4 in libpthread-2.10.1.so[7f921b44a000+17000]

Ok, so this is the libvirtd daemon crash

 From GDB
 
 06:45:32.434: error : qemuMonitorCommandWithHandler:290 : cannot send monitor 
 command 'info cpus': Connection reset by peer

Interestingly this suggests that QEMU itself had quit/crashed during
startup. You can check /var/log/libvirt/qemu//$GUESTNAME.log to find
out what problem QEMU itself had.

For whatever reason, this seems to then trigger the libvirtd crash

 06:45:32.434: error : qemuMonitorTextGetCPUInfo:436 : internal error cannot 
 run monitor command to fetch CPU thread info
 
 Program received signal SIGSEGV, Segmentation fault.
 [Switching to Thread 0x7f485bfff910 (LWP 18120)]
 0x7f48657fb6b4 in pthread_mutex_unlock () from /lib/libpthread.so.0
 (gdb) bt
 #0  0x7f48657fb6b4 in pthread_mutex_unlock () from /lib/libpthread.so.0
 #1  0x00431101 in qemuDomainObjExitMonitorWithDriver 
 (driver=0x11f0110, obj=0x12114f0) at qemu/qemu_driver.c:318
 #2  0x0043f436 in qemudStartVMDaemon (conn=value optimized out, 
 driver=0x11f0110, vm=0x12114f0, 
 migrateFrom=value optimized out, stdin_fd=value optimized out) at 
 qemu/qemu_driver.c:2320
 #3  0x004407c4 in qemudDomainStart (dom=0x11f0330) at 
 qemu/qemu_driver.c:4370
 #4  0x7f4865a621e7 in virDomainCreate (domain=0x11f0330) at libvirt.c:4509
 #5  0x00420d68 in remoteDispatchDomainCreate (server=value optimized 
 out, client=value optimized out, conn=0x1219da0, 
 hdr=value optimized out, rerr=0x7f485bffedf0, args=value optimized 
 out, ret=0x7f485bffeed0) at remote.c:853
 #6  0x004228e1 in remoteDispatchClientCall (server=value optimized 
 out, client=0x7f485c000d30, msg=0x7f485c080f00)
 at dispatch.c:506
 #7  0x00422c93 in remoteDispatchClientRequest (server=0x11e2790, 
 client=0x7f485c000d30, msg=0x7f485c080f00) at dispatch.c:388
 #8  0x0041625c in qemudWorker (data=value optimized out) at 
 libvirtd.c:1518
 #9  0x7f48657f7a04 in start_thread () from /lib/libpthread.so.0
 #10 0x7f48655617bd in clone () from /lib/libc.so.6
 #11 0x in ?? ()
 (gdb) select 2
 (gdb) info locals
 argv = 0x0
 tmp = value optimized out
 progenv = 0x0
 i = 1
 ret = 1
 sb = {st_dev = 64512, st_ino = 140307, st_nlink = 1, st_mode = 33261, st_uid 
 = 0, st_gid = 0, __pad0 = 0, st_rdev = 0, 
   st_size = 2293664, st_blksize = 4096, st_blocks = 4480, st_atim = {tv_sec = 
 1258937087, tv_nsec = 967226499}, st_mtim = {
 tv_sec = 1257180462, tv_nsec = 0}, st_ctim = {tv_sec = 1258886336, 
 tv_nsec = 104908677}, __unused = {0, 0, 0}}
 tapfds = 0x0
 ntapfds = 1
 qemuCmdFlags = 9436542
 keepfd = {fds_bits = {262144, 0 repeats 15 times}}
 emulator = value optimized out
 child = 18145
 pos = value optimized out
 ebuf = 
 \220v\036\001\000\000\000\000\060|WeH\177\000\000(\000\000\000\060\000\000\000\000\354\377[h\177\000\...@\353\377[h\177\000\000`\351\377[h\177\000\000\377\377\377\377h\177\000\000\233\017\037\001\000\000\000\000\326\063g\000\000\000\000\000@\247~eh\177\000\000\200\307oeh\177\000\000\240\306oeh\177\000\000jst\000\071\000\000\000\300\352\377[h\177\000\000\342\000\000\000\000\000\000\000\342\000\000\000\000\000\000\000\273\340g\000\000\000\000\000\300\352\377[h\177\000\000k\000\000\000\000\000\000\000`\351\377[h\177\000\...@\354\377[h\177\000\000p\352\377[h\177\000\000\001\000\000\000\000\000\000\000\320\352\377[h\177\000\000$\000\000\000\000\000\000\000py\255eh\177\000\000@\354\377[H\177\000\000\060|WeH\177\000\000\001\200\255\373\000\000\000\000\260\351\377[H\177\000\000\220\354\377[H\177\000\000\240\352\377[...
 pidfile = 0x0
 logfile = 15
 hookData = {conn = 0x1219da0, vm = 0x12114f0, driver = 0x11f0110}
 __FUNCTION__ = qemudStartVMDaemon
 __func__ = qemudStartVMDaemon


If you are able to reproduce this crash reliably, can you edit the
/etc/libvirt/libvirtd.conf file and add


  log_filters=1:qemu
  log_outputs=1:file:/var/log/libvirtd.log

and then restart libvirtd, and try to make it crash again. Then send
us the log file

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

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

Re: [libvirt] [PATCH] Put libraries in $LIBS, not $LDFLAGS, during configure tests.

2009-11-25 Thread Daniel P. Berrange 
On Mon, Nov 23, 2009 at 09:28:24PM +, Nix wrote:
 If libraries go in $LDFLAGS while AC_CHECK_LIBbing, they'll end up in
 front of the object file name, which rarely works well. They belong
 in $LIBS.

What platforms / scenarios show this problem ?   Empirically it has
been working fine on our commonly tested platforms, so it'll be
important to know where to look/test for this problem.


 ---
  configure.in |   25 -
  1 files changed, 12 insertions(+), 13 deletions(-)
 
 diff --git a/configure.in b/configure.in
 index f735bba..5308364 100644
 --- a/configure.in
 +++ b/configure.in
 @@ -542,14 +542,14 @@ AC_SUBST([LIBXML_LIBS])
  
  dnl xmlURI structure has query_raw?
  old_cflags=$CFLAGS
 -old_ldflags=$LDFLAGS
 +old_libs=$LIBS
  CFLAGS=$CFLAGS $LIBXML_CFLAGS
 -LDFLAGS=$LDFLAGS $LIBXML_LIBS
 +LIBS=$LIBS $LIBXML_LIBS
  AC_CHECK_MEMBER([struct _xmlURI.query_raw],
   [AC_DEFINE([HAVE_XMLURI_QUERY_RAW], [], [Have query_raw field 
 in libxml2 xmlURI structure])],,
   [#include libxml/uri.h])
  CFLAGS=$old_cflags
 -LDFLAGS=$old_ldflags
 +LIBS=$old_libs
  
  dnl GnuTLS library
  GNUTLS_CFLAGS=
 @@ -579,15 +579,15 @@ dnl Old versions of GnuTLS uses types like 
 'gnutls_session' instead
  dnl of 'gnutls_session_t'.  Try to detect this type if defined so
  dnl that we can offer backwards compatibility.
  old_cflags=$CFLAGS
 -old_ldflags=$LDFLAGS
 +old_libs=$LIBS
  CFLAGS=$CFLAGS $GNUTLS_CFLAGS
 -LDFLAGS=$LDFLAGS $GNUTLS_LIBS
 +LIBS=$LIBS $GNUTLS_LIBS
  AC_CHECK_TYPE([gnutls_session],
   AC_DEFINE([GNUTLS_1_0_COMPAT],[],
   [enable GnuTLS 1.0 compatibility macros]),,
   [#include gnutls/gnutls.h])
  CFLAGS=$old_cflags
 -LDFLAGS=$old_ldflags
 +LIBS=$old_libs
  
  
  dnl Cyrus SASL
 @@ -685,12 +685,12 @@ if test x$with_polkit = xyes -o x$with_polkit = 
 xcheck; then
  [use PolicyKit for UNIX socket access checks])
  
old_CFLAGS=$CFLAGS
 -  old_LDFLAGS=$LDFLAGS
 +  old_LIBS=$LIBS
CFLAGS=$CFLAGS $POLKIT_CFLAGS
 -  LDFLAGS=$LDFLAGS $POLKIT_LIBS
 +  LIBS=$LIBS $POLKIT_LIBS
AC_CHECK_FUNCS([polkit_context_is_caller_authorized])
CFLAGS=$old_CFLAGS
 -  LDFLAGS=$old_LDFLAGS
 +  LIBS=$old_LIBS
  
AC_PATH_PROG([POLKIT_AUTH], [polkit-auth])
if test x$POLKIT_AUTH != x; then
 @@ -1682,20 +1682,19 @@ if test x$with_hal = xyes -o x$with_hal = 
 xcheck; then
[use HAL for host device enumeration])
  
  old_CFLAGS=$CFLAGS
 -old_LDFLAGS=$LDFLAGS
 +old_LIBS=$LIBS
  CFLAGS=$CFLAGS $HAL_CFLAGS
 -LDFLAGS=$LDFLAGS $HAL_LIBS
 +LIBS=$LIBS $HAL_LIBS
  AC_CHECK_FUNCS([libhal_get_all_devices],,[with_hal=no])
  AC_CHECK_FUNCS([dbus_watch_get_unix_fd])
  CFLAGS=$old_CFLAGS
 -LDFLAGS=$old_LDFLAGS
 +LIBS=$old_LIBS
fi
  fi
  AM_CONDITIONAL([HAVE_HAL], [test x$with_hal = xyes])
  AC_SUBST([HAL_CFLAGS])
  AC_SUBST([HAL_LIBS])
  
 -
  dnl udev/libpciaccess library check for alternate host device enumeration
  UDEV_CFLAGS=
  UDEV_LIBS=


ACK


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

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

Re: [libvirt] [PATCH] Add test for legacy console ... tty= syntax

2009-11-25 Thread Daniel P. Berrange 
On Mon, Nov 23, 2009 at 01:48:30PM +, Matthew Booth wrote:
 * tests/qemuxml2argvtest.c: Test legacy syntax for QEMU argument generation
 * tests/qemuxml2argvdata/qemuxml2argv-console-legacy.(xml|args): Add test data

This test doesn't seem to test anything we're not already testing ?

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

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

Re: [libvirt] [PATCH 1/3] Make QEMU driver use -chardev everywhere when it's available

2009-11-25 Thread Daniel P. Berrange 
On Mon, Nov 23, 2009 at 12:30:27PM +, Matthew Booth wrote:
 Change -monitor, -serial and -parallel output to use -chardev if it is
 available.
 
 * src/qemu/qemu_conf.c: Update qemudBuildCommandLine to use -chardev where
   available.
 * tests/qemuxml2argvtest.c tests/qemuxml2argvdata/: Add -chardev equivalents 
 for
   all current serial and parallel tests.

ACK


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

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

Re: [libvirt] [PATCH 2/3] Extract the assigned pty device for channels

2009-11-25 Thread Daniel P. Berrange 
On Mon, Nov 23, 2009 at 12:30:28PM +, Matthew Booth wrote:
 * src/qemu/qemu_driver.c: Parse pty devices for channels
 ---
  src/qemu/qemu_driver.c |   10 ++
  1 files changed, 10 insertions(+), 0 deletions(-)
 
 diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
 index 2c5086b..ebf44b0 100644
 --- a/src/qemu/qemu_driver.c
 +++ b/src/qemu/qemu_driver.c
 @@ -1271,6 +1271,16 @@ qemudFindCharDevicePTYs(virConnectPtr conn,
  }
  }
  
 +/* then the channel devices */
 +for (i = 0 ; i  vm-def-nchannels ; i++) {
 +virDomainChrDefPtr chr = vm-def-channels[i];
 +if (chr-type == VIR_DOMAIN_CHR_TYPE_PTY) {
 +if ((ret = qemudExtractTTYPath(conn, output, offset,
 +   chr-data.file.path)) != 0)
 +return ret;
 +}
 +}
 +
  return 0;
  }
  
 -- 


ACK,


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

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

Re: [libvirt] [PATCH] Add test for legacy console ... tty= syntax

2009-11-25 Thread Matthew Booth 

On 25/11/09 11:25, Daniel P. Berrange wrote:

On Mon, Nov 23, 2009 at 01:48:30PM +, Matthew Booth wrote:

* tests/qemuxml2argvtest.c: Test legacy syntax for QEMU argument generation
* tests/qemuxml2argvdata/qemuxml2argv-console-legacy.(xml|args): Add test data


This test doesn't seem to test anything we're not already testing ?


Where's it currently tested? I couldn't see it. I also ran a make check 
with this test in place before and after the recent tty= fix. It failed 
before and succeeded afterwards.


That said, I'm not entirely sure what the console-compat test is 
testing. Is it possible that the test was supposed to be in there?


Matt
--
Matthew Booth, RHCA, RHCSS
Red Hat Engineering, Virtualisation Team

M:   +44 (0)7977 267231
GPG ID:  D33C3490
GPG FPR: 3733 612D 2D05 5458 8A8A 1600 3441 EA19 D33C 3490

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

Re: [libvirt] [PATCH 3/3] Get QEMU pty paths from the monitor

2009-11-25 Thread Daniel P. Berrange 
On Mon, Nov 23, 2009 at 12:30:29PM +, Matthew Booth wrote:
 This change makes the QEMU driver get pty paths from the output of the monitor
 'info chardev' command. This output is structured, and contains both the name 
 of
 the device and the path on the same line. This is considerably more reliable
 than parsing the startup log output, which requires the parsing code to know
 which order QEMU will print pty information in.
 
 Note that we still need to parse the log output as the monitor itself may be 
 on
 a pty. This should be rare, however, and the new code will replace all pty 
 paths
 parsed by the log output method once the monitor is available.
 
 * src/qemu/qemu_monitor.(c|h) src/qemu_monitor_text.(c|h): Implement
   qemuMonitorGetPtyPaths().
 * src/qemu/qemu_driver.c: Get pty path information using
   qemuMonitorGetPtyPaths().
 ---
  src/qemu/qemu_driver.c   |   68 +++-
  src/qemu/qemu_monitor.c  |9 +
  src/qemu/qemu_monitor.h  |3 ++
  src/qemu/qemu_monitor_text.c |   71 
 ++
  src/qemu/qemu_monitor_text.h |4 ++
  5 files changed, 153 insertions(+), 2 deletions(-)
 
 diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
 index ebf44b0..90dd9cd 100644
 --- a/src/qemu/qemu_driver.c
 +++ b/src/qemu/qemu_driver.c
 @@ -1239,6 +1239,40 @@ qemudExtractTTYPath(virConnectPtr conn,
  }
  
  static int
 +qemudFindCharDevicePTYsMonitor(virConnectPtr conn,
 +   virDomainObjPtr vm,
 +   virHashTablePtr paths)
 +{
 +int i;
 +
 +#define LOOKUP_PTYS(array, arraylen, idprefix) \
 +for (i = 0 ; i  (arraylen) ; i++) { \
 +virDomainChrDefPtr chr = (array)[i]; \
 +if (chr-type == VIR_DOMAIN_CHR_TYPE_PTY) { \
 +char id[16]; \
 +\
 +if (snprintf(id, sizeof(id), idprefix %i, i) = sizeof(id)) \
 +return -1; \
 +\
 +const char *path = (const char *) virHashLookup(paths, id); \
 +if (path == NULL) { \
 +qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, \
 + _(no assigned pty for device %s), id); \
 +return -1; \
 +} \
 +\
 +chr-data.file.path = strdup(path); \
 +} \
 +}

Can you indent the  \  to they all line up in the right hand side. 

 +
 +LOOKUP_PTYS(vm-def-serials,   vm-def-nserials,   serial);
 +LOOKUP_PTYS(vm-def-parallels, vm-def-nparallels, parallel);
 +LOOKUP_PTYS(vm-def-channels,  vm-def-nchannels,  channel);
 +

#undef LOOKUP_PTYS

 +return 0;
 +}
 +
 +static int
  qemudFindCharDevicePTYs(virConnectPtr conn,
  virDomainObjPtr vm,
  const char *output,
 @@ -1284,6 +1318,11 @@ qemudFindCharDevicePTYs(virConnectPtr conn,
  return 0;
  }
  
 +static void qemudFreePtyPath(void *payload, const char *name 
 ATTRIBUTE_UNUSED)
 +{
 +free(payload);

VIR_FREE

 +}
 +
  static int
  qemudWaitForMonitor(virConnectPtr conn,
  struct qemud_driver* driver,
 @@ -1291,7 +1330,7 @@ qemudWaitForMonitor(virConnectPtr conn,
  {
  char buf[4096]; /* Plenty of space to get startup greeting */
  int logfd;
 -int ret;
 +int ret = -1;
  
  if ((logfd = qemudLogReadFD(conn, driver-logDir, vm-def-name, pos))
   0)
 @@ -1317,7 +1356,32 @@ qemudWaitForMonitor(virConnectPtr conn,
  if (qemuConnectMonitor(vm)  0)
  return -1;
  
 -return 0;
 +/* Try to get the pty path mappings again via the monitor. This is much 
 more
 + * reliable if it's available.
 + * Note that the monitor itself can be on a pty, so we still need to try 
 the
 + * log output method. */
 +virHashTablePtr paths = virHashCreate(0);
 +if (paths == NULL) {
 +virReportOOMError(NULL);
 +goto cleanup;
 +}
 +
 +qemuDomainObjEnterMonitor(vm);

This needs to be EnterMonitorWithDriver(driver, vm), since the 'driver'
is locked in this context

 +qemuDomainObjPrivatePtr priv = vm-privateData;
 +ret = qemuMonitorGetPtyPaths(priv-mon, paths);
 +qemuDomainObjExitMonitor(vm);

And ExitMonitorWithDriver

 +
 +VIR_DEBUG(qemuMonitorGetPtyPaths returned %i, ret);
 +if (ret == 0) {
 +ret = qemudFindCharDevicePTYsMonitor(conn, vm, paths);
 +}
 +
 +cleanup:
 +if (paths) {
 +virHashFree(paths, qemudFreePtyPath);
 +}
 +
 +return ret;
  }


 +
 +
 +/* Parse the output of info chardev and return a hash of pty paths.
 + *
 + * Output is:
 + * foo: filename=pty:/dev/pts/7
 + * monitor: filename=stdio
 + * serial0: filename=vc
 + * parallel0: filename=vc
 + *
 + * Non-pty lines are ignored. In the above example, key is 'foo', value is
 + * '/dev/pty/7'. The hash will contain only a single value.
 + */
 +
 +int qemuMonitorTextGetPtyPaths(qemuMonitorPtr mon,
 +   virHashTablePtr

Re: [libvirt] [PATCH] Add test for legacy console ... tty= syntax

2009-11-25 Thread Daniel P. Berrange 
On Wed, Nov 25, 2009 at 11:27:48AM +, Matthew Booth wrote:
 On 25/11/09 11:25, Daniel P. Berrange wrote:
 On Mon, Nov 23, 2009 at 01:48:30PM +, Matthew Booth wrote:
 * tests/qemuxml2argvtest.c: Test legacy syntax for QEMU argument 
 generation
 * tests/qemuxml2argvdata/qemuxml2argv-console-legacy.(xml|args): Add test 
 data
 
 This test doesn't seem to test anything we're not already testing ?
 
 Where's it currently tested? I couldn't see it. I also ran a make check 
 with this test in place before and after the recent tty= fix. It failed 
 before and succeeded afterwards.
 
 That said, I'm not entirely sure what the console-compat test is 
 testing. Is it possible that the test was supposed to be in there?

Since you have both serial and  console elements there, console
is ignored. Essentially console is just a pointer to the first
serial port. console would only have been looked at if you hadn't
had the serial tag there, in which case it would have been copied
to the serial tag

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

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

Re: [libvirt] [PATCH] (v2) avoid chowning domain devices if higer-level mgmt does it for us

2009-11-25 Thread Dan Kenigsberg 
On Wed, Nov 25, 2009 at 10:31:03AM +0100, Daniel Veillard wrote:
 On Wed, Nov 25, 2009 at 09:27:13AM +0200, Dan Kenigsberg wrote:
  this is particularily important if said device is a file sitting on a
  root_squashing nfs export.
  
  my previous attempt for a patch missed 3 chowns that should be avoided.
  ---
   src/qemu/qemu.conf |4 
   src/qemu/qemu_conf.c   |3 +++
   src/qemu/qemu_conf.h   |1 +
   src/qemu/qemu_driver.c |8 
   4 files changed, 12 insertions(+), 4 deletions(-)
  
  diff --git a/src/qemu/qemu.conf b/src/qemu/qemu.conf
  index bca858a..892a50b 100644
  --- a/src/qemu/qemu.conf
  +++ b/src/qemu/qemu.conf
  @@ -96,6 +96,10 @@
   # The group ID for QEMU processes run by the system instance
   #group = root
   
  +# should libvirt assume that devices are accessible to the above 
  user:group.
  +# by default, libvirt tries to chown devices before starting up a domain 
  and
  +# restore ownership to root when domain comes down.
  +#assume_devices_accessible = 0
   
   # What cgroup controllers to make use of with QEMU guests
   #
  diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
  index b1b9e5f..520a395 100644
  --- a/src/qemu/qemu_conf.c
  +++ b/src/qemu/qemu_conf.c
  @@ -232,6 +232,9 @@ int qemudLoadDriverConfig(struct qemud_driver *driver,
   return -1;
   }
   
  +p = virConfGetValue (conf, assume_devices_accessible);
  +CHECK_TYPE (assume_devices_accessible, VIR_CONF_LONG);
  +if (p) driver-avoid_dev_chown = p-l;
 
   an explicit initialization of the field would be better if p is NULL.
 
   if (virGetGroupID(NULL, group, driver-group)  0) {
   VIR_FREE(group);
  diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h
  index 675c636..3a9da73 100644
  --- a/src/qemu/qemu_conf.h
  +++ b/src/qemu/qemu_conf.h
  @@ -87,6 +87,7 @@ struct qemud_driver {
   
   uid_t user;
   gid_t group;
  +int avoid_dev_chown;
   
   unsigned int qemuVersion;
   int nextvmid;
  diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
  index 2f273eb..5f02aa2 100644
  --- a/src/qemu/qemu_driver.c
  +++ b/src/qemu/qemu_driver.c
  @@ -1968,7 +1968,7 @@ static int qemuDomainSetDeviceOwnership(virConnectPtr 
  conn,
   uid_t uid;
   gid_t gid;
   
  -if (!driver-privileged)
  +if (!driver-privileged || driver-avoid_dev_chown)
   return 0;
   
   /* short circuit case of root:root */
  @@ -2002,7 +2002,7 @@ static int 
  qemuDomainSetAllDeviceOwnership(virConnectPtr conn,
   uid_t uid;
   gid_t gid;
   
  -if (!driver-privileged)
  +if (!driver-privileged || driver-avoid_dev_chown)
   return 0;
   
   /* short circuit case of root:root */
  @@ -3438,7 +3438,7 @@ static int qemudDomainSave(virDomainPtr dom,
   }
   fd = -1;
   
  -if (driver-privileged 
  +if (driver-privileged  !driver-avoid_dev_chown 
   chown(path, driver-user, driver-group)  0) {
   virReportSystemError(NULL, errno,
_(unable to set ownership of '%s' to user 
  %d:%d),
  @@ -3473,7 +3473,7 @@ static int qemudDomainSave(virDomainPtr dom,
   if (rc  0)
   goto endjob;
   
  -if (driver-privileged 
  +if (driver-privileged  !driver-avoid_dev_chown 
   chown(path, 0, 0)  0) {
   virReportSystemError(NULL, errno,
_(unable to set ownership of '%s' to user 
  %d:%d),
 
 The core question is having this as another manual user tweak, it always
 makes me a bit uncomfortable if proper working of software requires manually
 editing a config file. If we really need this kind of options for proper
 operations in specific conditions can we make sure it can be set via
 APIs too ? I don't think we should expose something as generic as the
 internal Conf APIs, but these runtime option in src/qemu/qemu.conf
 start to accumulate and I start to wonder how to properly manage all
 this.

Aren't we here to babysit software? :-)

For this particular behavior, I personally feel that the default is
wrong. I don't like it that libvirt takes the liberty to (try to) change
ownership, I like it less when it does not truely restore ownership, and
even less, that it fails to use files over nfs.

I understand that this behavior was meant to assist admins who upgrade
from running qemu as root to running it as a meager user. But personally
I believe that we could have politely asked them: if you use this new
feature, please make sure this new qemu user has access to your
files/devices. Now that there are people expecting current behavior, I
don't really see a nice way out.

Dan

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

Re: [libvirt] [PATCH] (v2) avoid chowning domain devices if higer-level mgmt does it for us

2009-11-25 Thread Daniel P. Berrange 
On Wed, Nov 25, 2009 at 01:38:52PM +0200, Dan Kenigsberg wrote:
 On Wed, Nov 25, 2009 at 10:31:03AM +0100, Daniel Veillard wrote:
  On Wed, Nov 25, 2009 at 09:27:13AM +0200, Dan Kenigsberg wrote:
   this is particularily important if said device is a file sitting on a
   root_squashing nfs export.
   
   my previous attempt for a patch missed 3 chowns that should be avoided.
   ---
src/qemu/qemu.conf |4 
src/qemu/qemu_conf.c   |3 +++
src/qemu/qemu_conf.h   |1 +
src/qemu/qemu_driver.c |8 
4 files changed, 12 insertions(+), 4 deletions(-)
   
   diff --git a/src/qemu/qemu.conf b/src/qemu/qemu.conf
   index bca858a..892a50b 100644
   --- a/src/qemu/qemu.conf
   +++ b/src/qemu/qemu.conf
   @@ -96,6 +96,10 @@
# The group ID for QEMU processes run by the system instance
#group = root

   +# should libvirt assume that devices are accessible to the above 
   user:group.
   +# by default, libvirt tries to chown devices before starting up a domain 
   and
   +# restore ownership to root when domain comes down.
   +#assume_devices_accessible = 0

# What cgroup controllers to make use of with QEMU guests
#
   diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
   index b1b9e5f..520a395 100644
   --- a/src/qemu/qemu_conf.c
   +++ b/src/qemu/qemu_conf.c
   @@ -232,6 +232,9 @@ int qemudLoadDriverConfig(struct qemud_driver *driver,
return -1;
}

   +p = virConfGetValue (conf, assume_devices_accessible);
   +CHECK_TYPE (assume_devices_accessible, VIR_CONF_LONG);
   +if (p) driver-avoid_dev_chown = p-l;
  
an explicit initialization of the field would be better if p is NULL.
  
if (virGetGroupID(NULL, group, driver-group)  0) {
VIR_FREE(group);
   diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h
   index 675c636..3a9da73 100644
   --- a/src/qemu/qemu_conf.h
   +++ b/src/qemu/qemu_conf.h
   @@ -87,6 +87,7 @@ struct qemud_driver {

uid_t user;
gid_t group;
   +int avoid_dev_chown;

unsigned int qemuVersion;
int nextvmid;
   diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
   index 2f273eb..5f02aa2 100644
   --- a/src/qemu/qemu_driver.c
   +++ b/src/qemu/qemu_driver.c
   @@ -1968,7 +1968,7 @@ static int 
   qemuDomainSetDeviceOwnership(virConnectPtr conn,
uid_t uid;
gid_t gid;

   -if (!driver-privileged)
   +if (!driver-privileged || driver-avoid_dev_chown)
return 0;

/* short circuit case of root:root */
   @@ -2002,7 +2002,7 @@ static int 
   qemuDomainSetAllDeviceOwnership(virConnectPtr conn,
uid_t uid;
gid_t gid;

   -if (!driver-privileged)
   +if (!driver-privileged || driver-avoid_dev_chown)
return 0;

/* short circuit case of root:root */
   @@ -3438,7 +3438,7 @@ static int qemudDomainSave(virDomainPtr dom,
}
fd = -1;

   -if (driver-privileged 
   +if (driver-privileged  !driver-avoid_dev_chown 
chown(path, driver-user, driver-group)  0) {
virReportSystemError(NULL, errno,
 _(unable to set ownership of '%s' to user 
   %d:%d),
   @@ -3473,7 +3473,7 @@ static int qemudDomainSave(virDomainPtr dom,
if (rc  0)
goto endjob;

   -if (driver-privileged 
   +if (driver-privileged  !driver-avoid_dev_chown 
chown(path, 0, 0)  0) {
virReportSystemError(NULL, errno,
 _(unable to set ownership of '%s' to user 
   %d:%d),
  
  The core question is having this as another manual user tweak, it always
  makes me a bit uncomfortable if proper working of software requires manually
  editing a config file. If we really need this kind of options for proper
  operations in specific conditions can we make sure it can be set via
  APIs too ? I don't think we should expose something as generic as the
  internal Conf APIs, but these runtime option in src/qemu/qemu.conf
  start to accumulate and I start to wonder how to properly manage all
  this.
 
 Aren't we here to babysit software? :-)
 
 For this particular behavior, I personally feel that the default is
 wrong. I don't like it that libvirt takes the liberty to (try to) change
 ownership, I like it less when it does not truely restore ownership, and
 even less, that it fails to use files over nfs.
 
 I understand that this behavior was meant to assist admins who upgrade
 from running qemu as root to running it as a meager user. But personally
 I believe that we could have politely asked them: if you use this new
 feature, please make sure this new qemu user has access to your
 files/devices. Now that there are people expecting current behavior, I
 don't really see a nice way out.

Yes this whole area of code didn't really turn out the way I hoped it
would. For a start, regardless of whether it works correctly or

[libvirt] [PATCH] Fix threading problems in python bindings

2009-11-25 Thread Daniel P. Berrange 
* libvirt-override.c: Add many missing calls to allow threading
  when entering C code, otherwise python blocks  then deadlocks
  when we have an async event to dispatch back into python code
---
 python/libvirt-override.c |  106 
 1 files changed, 96 insertions(+), 10 deletions(-)

diff --git a/python/libvirt-override.c b/python/libvirt-override.c
index b885190..0f7db9c 100644
--- a/python/libvirt-override.c
+++ b/python/libvirt-override.c
@@ -67,7 +67,10 @@ libvirt_virDomainBlockStats(PyObject *self ATTRIBUTE_UNUSED, 
PyObject *args) {
 return(NULL);
 domain = (virDomainPtr) PyvirDomain_Get(pyobj_domain);
 
+LIBVIRT_BEGIN_ALLOW_THREADS;
 c_retval = virDomainBlockStats(domain, path, stats, sizeof(stats));
+LIBVIRT_END_ALLOW_THREADS;
+
 if (c_retval  0)
 return VIR_PY_NONE;
 
@@ -96,7 +99,10 @@ libvirt_virDomainInterfaceStats(PyObject *self 
ATTRIBUTE_UNUSED, PyObject *args)
 return(NULL);
 domain = (virDomainPtr) PyvirDomain_Get(pyobj_domain);
 
+LIBVIRT_BEGIN_ALLOW_THREADS;
 c_retval = virDomainInterfaceStats(domain, path, stats, sizeof(stats));
+LIBVIRT_END_ALLOW_THREADS;
+
 if (c_retval  0)
 return VIR_PY_NONE;
 
@@ -128,7 +134,9 @@ libvirt_virDomainGetSchedulerType(PyObject *self 
ATTRIBUTE_UNUSED,
 return(NULL);
 domain = (virDomainPtr) PyvirDomain_Get(pyobj_domain);
 
+LIBVIRT_BEGIN_ALLOW_THREADS;
 c_retval = virDomainGetSchedulerType(domain, nparams);
+LIBVIRT_END_ALLOW_THREADS;
 if (c_retval == NULL)
 return VIR_PY_NONE;
 
@@ -150,6 +158,7 @@ libvirt_virDomainGetSchedulerParameters(PyObject *self 
ATTRIBUTE_UNUSED,
 virDomainPtr domain;
 PyObject *pyobj_domain, *info;
 char *c_retval;
+int i_retval;
 int nparams, i;
 virSchedParameterPtr params;
 
@@ -158,7 +167,10 @@ libvirt_virDomainGetSchedulerParameters(PyObject *self 
ATTRIBUTE_UNUSED,
 return(NULL);
 domain = (virDomainPtr) PyvirDomain_Get(pyobj_domain);
 
+LIBVIRT_BEGIN_ALLOW_THREADS;
 c_retval = virDomainGetSchedulerType(domain, nparams);
+LIBVIRT_END_ALLOW_THREADS;
+
 if (c_retval == NULL)
 return VIR_PY_NONE;
 free(c_retval);
@@ -166,7 +178,11 @@ libvirt_virDomainGetSchedulerParameters(PyObject *self 
ATTRIBUTE_UNUSED,
 if ((params = malloc(sizeof(*params)*nparams)) == NULL)
 return VIR_PY_NONE;
 
-if (virDomainGetSchedulerParameters(domain, params, nparams)  0) {
+LIBVIRT_BEGIN_ALLOW_THREADS;
+i_retval = virDomainGetSchedulerParameters(domain, params, nparams);
+LIBVIRT_END_ALLOW_THREADS;
+
+if (i_retval  0) {
 free(params);
 return VIR_PY_NONE;
 }
@@ -223,6 +239,7 @@ libvirt_virDomainSetSchedulerParameters(PyObject *self 
ATTRIBUTE_UNUSED,
 virDomainPtr domain;
 PyObject *pyobj_domain, *info;
 char *c_retval;
+int i_retval;
 int nparams, i;
 virSchedParameterPtr params;
 
@@ -231,7 +248,10 @@ libvirt_virDomainSetSchedulerParameters(PyObject *self 
ATTRIBUTE_UNUSED,
 return(NULL);
 domain = (virDomainPtr) PyvirDomain_Get(pyobj_domain);
 
+LIBVIRT_BEGIN_ALLOW_THREADS;
 c_retval = virDomainGetSchedulerType(domain, nparams);
+LIBVIRT_END_ALLOW_THREADS;
+
 if (c_retval == NULL)
 return VIR_PY_INT_FAIL;
 free(c_retval);
@@ -239,7 +259,11 @@ libvirt_virDomainSetSchedulerParameters(PyObject *self 
ATTRIBUTE_UNUSED,
 if ((params = malloc(sizeof(*params)*nparams)) == NULL)
 return VIR_PY_INT_FAIL;
 
-if (virDomainGetSchedulerParameters(domain, params, nparams)  0) {
+LIBVIRT_BEGIN_ALLOW_THREADS;
+i_retval = virDomainGetSchedulerParameters(domain, params, nparams);
+LIBVIRT_END_ALLOW_THREADS;
+
+if (i_retval  0) {
 free(params);
 return VIR_PY_INT_FAIL;
 }
@@ -292,7 +316,10 @@ libvirt_virDomainSetSchedulerParameters(PyObject *self 
ATTRIBUTE_UNUSED,
 }
 }
 
-if (virDomainSetSchedulerParameters(domain, params, nparams)  0) {
+LIBVIRT_BEGIN_ALLOW_THREADS;
+i_retval = virDomainSetSchedulerParameters(domain, params, nparams);
+LIBVIRT_END_ALLOW_THREADS;
+if (i_retval  0) {
 free(params);
 return VIR_PY_INT_FAIL;
 }
@@ -311,13 +338,17 @@ libvirt_virDomainGetVcpus(PyObject *self ATTRIBUTE_UNUSED,
 virVcpuInfoPtr cpuinfo = NULL;
 unsigned char *cpumap = NULL;
 int cpumaplen, i;
+int i_retval;
 
 if (!PyArg_ParseTuple(args, (char *)O:virDomainGetVcpus,
   pyobj_domain))
 return(NULL);
 domain = (virDomainPtr) PyvirDomain_Get(pyobj_domain);
 
-if (virNodeGetInfo(virDomainGetConnect(domain), nodeinfo) != 0)
+LIBVIRT_BEGIN_ALLOW_THREADS;
+i_retval = virNodeGetInfo(virDomainGetConnect(domain), nodeinfo);
+LIBVIRT_END_ALLOW_THREADS;
+if (i_retval  0)
 return VIR_PY_NONE;
 
 if (virDomainGetInfo(domain, dominfo) != 0)
@@ -330,9 +361,12 @@

Re: [libvirt] [PATCH] Fix threading problems in python bindings

2009-11-25 Thread Matthias Bolte 
2009/11/25 Daniel P. Berrange berra...@redhat.com:
 * libvirt-override.c: Add many missing calls to allow threading
  when entering C code, otherwise python blocks  then deadlocks
  when we have an async event to dispatch back into python code
 ---
  python/libvirt-override.c |  106 
  1 files changed, 96 insertions(+), 10 deletions(-)

 diff --git a/python/libvirt-override.c b/python/libvirt-override.c
 index b885190..0f7db9c 100644
 --- a/python/libvirt-override.c
 +++ b/python/libvirt-override.c
[...]
 @@ -418,10 +456,15 @@ libvirt_virDomainPinVcpu(PyObject *self 
 ATTRIBUTE_UNUSED,
             VIR_UNUSE_CPU(cpumap, i);
     }

 -    virDomainPinVcpu(domain, vcpu, cpumap, cpumaplen);
 +    LIBVIRT_BEGIN_ALLOW_THREADS;
 +    i_retval = virDomainPinVcpu(domain, vcpu, cpumap, cpumaplen);
 +    LIBVIRT_END_ALLOW_THREADS;
     Py_DECREF(truth);
     free(cpumap);

 +    if (i_retval  0)
 +        return VIR_PY_INT_FAIL;
 +
     return VIR_PY_INT_SUCCESS;
  }


You should at least mention this additional fix in the commit message.

ACK!

Matthias

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

Re: [libvirt] [PATCH] (v2) avoid chowning domain devices if higer-level mgmt does it for us

2009-11-25 Thread Hugh O. Brock 
On Wed, Nov 25, 2009 at 11:49:32AM +, Daniel P. Berrange wrote:
 On Wed, Nov 25, 2009 at 01:38:52PM +0200, Dan Kenigsberg wrote:
  On Wed, Nov 25, 2009 at 10:31:03AM +0100, Daniel Veillard wrote:
   On Wed, Nov 25, 2009 at 09:27:13AM +0200, Dan Kenigsberg wrote:
this is particularily important if said device is a file sitting on a
root_squashing nfs export.

my previous attempt for a patch missed 3 chowns that should be avoided.
---
 src/qemu/qemu.conf |4 
 src/qemu/qemu_conf.c   |3 +++
 src/qemu/qemu_conf.h   |1 +
 src/qemu/qemu_driver.c |8 
 4 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/src/qemu/qemu.conf b/src/qemu/qemu.conf
index bca858a..892a50b 100644
--- a/src/qemu/qemu.conf
+++ b/src/qemu/qemu.conf
@@ -96,6 +96,10 @@
 # The group ID for QEMU processes run by the system instance
 #group = root
 
+# should libvirt assume that devices are accessible to the above 
user:group.
+# by default, libvirt tries to chown devices before starting up a 
domain and
+# restore ownership to root when domain comes down.
+#assume_devices_accessible = 0
 
 # What cgroup controllers to make use of with QEMU guests
 #
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index b1b9e5f..520a395 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -232,6 +232,9 @@ int qemudLoadDriverConfig(struct qemud_driver 
*driver,
 return -1;
 }
 
+p = virConfGetValue (conf, assume_devices_accessible);
+CHECK_TYPE (assume_devices_accessible, VIR_CONF_LONG);
+if (p) driver-avoid_dev_chown = p-l;
   
 an explicit initialization of the field would be better if p is NULL.
   
 if (virGetGroupID(NULL, group, driver-group)  0) {
 VIR_FREE(group);
diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h
index 675c636..3a9da73 100644
--- a/src/qemu/qemu_conf.h
+++ b/src/qemu/qemu_conf.h
@@ -87,6 +87,7 @@ struct qemud_driver {
 
 uid_t user;
 gid_t group;
+int avoid_dev_chown;
 
 unsigned int qemuVersion;
 int nextvmid;
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 2f273eb..5f02aa2 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -1968,7 +1968,7 @@ static int 
qemuDomainSetDeviceOwnership(virConnectPtr conn,
 uid_t uid;
 gid_t gid;
 
-if (!driver-privileged)
+if (!driver-privileged || driver-avoid_dev_chown)
 return 0;
 
 /* short circuit case of root:root */
@@ -2002,7 +2002,7 @@ static int 
qemuDomainSetAllDeviceOwnership(virConnectPtr conn,
 uid_t uid;
 gid_t gid;
 
-if (!driver-privileged)
+if (!driver-privileged || driver-avoid_dev_chown)
 return 0;
 
 /* short circuit case of root:root */
@@ -3438,7 +3438,7 @@ static int qemudDomainSave(virDomainPtr dom,
 }
 fd = -1;
 
-if (driver-privileged 
+if (driver-privileged  !driver-avoid_dev_chown 
 chown(path, driver-user, driver-group)  0) {
 virReportSystemError(NULL, errno,
  _(unable to set ownership of '%s' to 
user %d:%d),
@@ -3473,7 +3473,7 @@ static int qemudDomainSave(virDomainPtr dom,
 if (rc  0)
 goto endjob;
 
-if (driver-privileged 
+if (driver-privileged  !driver-avoid_dev_chown 
 chown(path, 0, 0)  0) {
 virReportSystemError(NULL, errno,
  _(unable to set ownership of '%s' to 
user %d:%d),
   
   The core question is having this as another manual user tweak, it always
   makes me a bit uncomfortable if proper working of software requires 
   manually
   editing a config file. If we really need this kind of options for proper
   operations in specific conditions can we make sure it can be set via
   APIs too ? I don't think we should expose something as generic as the
   internal Conf APIs, but these runtime option in src/qemu/qemu.conf
   start to accumulate and I start to wonder how to properly manage all
   this.
  
  Aren't we here to babysit software? :-)
  
  For this particular behavior, I personally feel that the default is
  wrong. I don't like it that libvirt takes the liberty to (try to) change
  ownership, I like it less when it does not truely restore ownership, and
  even less, that it fails to use files over nfs.
  
  I understand that this behavior was meant to assist admins who upgrade
  from running qemu as root to running it as a meager user. But personally
  I believe that we could have politely asked them: if you use this new
  feature, please make sure this new qemu user has access to your
  files/devices. Now that there are people

Re: [libvirt] [PATCH] Fix threading problems in python bindings

2009-11-25 Thread Daniel Veillard 
On Wed, Nov 25, 2009 at 12:06:03PM +, Daniel P. Berrange wrote:
 * libvirt-override.c: Add many missing calls to allow threading
   when entering C code, otherwise python blocks  then deadlocks
   when we have an async event to dispatch back into python code
 ---
  python/libvirt-override.c |  106 
  1 files changed, 96 insertions(+), 10 deletions(-)
 
 diff --git a/python/libvirt-override.c b/python/libvirt-override.c
 index b885190..0f7db9c 100644
 --- a/python/libvirt-override.c
 +++ b/python/libvirt-override.c
 @@ -67,7 +67,10 @@ libvirt_virDomainBlockStats(PyObject *self 
 ATTRIBUTE_UNUSED, PyObject *args) {
  return(NULL);
  domain = (virDomainPtr) PyvirDomain_Get(pyobj_domain);
  
 +LIBVIRT_BEGIN_ALLOW_THREADS;
  c_retval = virDomainBlockStats(domain, path, stats, sizeof(stats));
 +LIBVIRT_END_ALLOW_THREADS;
 +
  if (c_retval  0)
  return VIR_PY_NONE;
  
 @@ -96,7 +99,10 @@ libvirt_virDomainInterfaceStats(PyObject *self 
 ATTRIBUTE_UNUSED, PyObject *args)
  return(NULL);
  domain = (virDomainPtr) PyvirDomain_Get(pyobj_domain);
  
 +LIBVIRT_BEGIN_ALLOW_THREADS;
  c_retval = virDomainInterfaceStats(domain, path, stats, sizeof(stats));
 +LIBVIRT_END_ALLOW_THREADS;
 +
  if (c_retval  0)
  return VIR_PY_NONE;
  
 @@ -128,7 +134,9 @@ libvirt_virDomainGetSchedulerType(PyObject *self 
 ATTRIBUTE_UNUSED,
  return(NULL);
  domain = (virDomainPtr) PyvirDomain_Get(pyobj_domain);
  
 +LIBVIRT_BEGIN_ALLOW_THREADS;
  c_retval = virDomainGetSchedulerType(domain, nparams);
 +LIBVIRT_END_ALLOW_THREADS;
  if (c_retval == NULL)
  return VIR_PY_NONE;
  
 @@ -150,6 +158,7 @@ libvirt_virDomainGetSchedulerParameters(PyObject *self 
 ATTRIBUTE_UNUSED,
  virDomainPtr domain;
  PyObject *pyobj_domain, *info;
  char *c_retval;
 +int i_retval;
  int nparams, i;
  virSchedParameterPtr params;
  
 @@ -158,7 +167,10 @@ libvirt_virDomainGetSchedulerParameters(PyObject *self 
 ATTRIBUTE_UNUSED,
  return(NULL);
  domain = (virDomainPtr) PyvirDomain_Get(pyobj_domain);
  
 +LIBVIRT_BEGIN_ALLOW_THREADS;
  c_retval = virDomainGetSchedulerType(domain, nparams);
 +LIBVIRT_END_ALLOW_THREADS;
 +
  if (c_retval == NULL)
  return VIR_PY_NONE;
  free(c_retval);
 @@ -166,7 +178,11 @@ libvirt_virDomainGetSchedulerParameters(PyObject *self 
 ATTRIBUTE_UNUSED,
  if ((params = malloc(sizeof(*params)*nparams)) == NULL)
  return VIR_PY_NONE;
  
 -if (virDomainGetSchedulerParameters(domain, params, nparams)  0) {
 +LIBVIRT_BEGIN_ALLOW_THREADS;
 +i_retval = virDomainGetSchedulerParameters(domain, params, nparams);
 +LIBVIRT_END_ALLOW_THREADS;
 +
 +if (i_retval  0) {
  free(params);
  return VIR_PY_NONE;
  }
 @@ -223,6 +239,7 @@ libvirt_virDomainSetSchedulerParameters(PyObject *self 
 ATTRIBUTE_UNUSED,
  virDomainPtr domain;
  PyObject *pyobj_domain, *info;
  char *c_retval;
 +int i_retval;
  int nparams, i;
  virSchedParameterPtr params;
  
 @@ -231,7 +248,10 @@ libvirt_virDomainSetSchedulerParameters(PyObject *self 
 ATTRIBUTE_UNUSED,
  return(NULL);
  domain = (virDomainPtr) PyvirDomain_Get(pyobj_domain);
  
 +LIBVIRT_BEGIN_ALLOW_THREADS;
  c_retval = virDomainGetSchedulerType(domain, nparams);
 +LIBVIRT_END_ALLOW_THREADS;
 +
  if (c_retval == NULL)
  return VIR_PY_INT_FAIL;
  free(c_retval);
 @@ -239,7 +259,11 @@ libvirt_virDomainSetSchedulerParameters(PyObject *self 
 ATTRIBUTE_UNUSED,
  if ((params = malloc(sizeof(*params)*nparams)) == NULL)
  return VIR_PY_INT_FAIL;
  
 -if (virDomainGetSchedulerParameters(domain, params, nparams)  0) {
 +LIBVIRT_BEGIN_ALLOW_THREADS;
 +i_retval = virDomainGetSchedulerParameters(domain, params, nparams);
 +LIBVIRT_END_ALLOW_THREADS;
 +
 +if (i_retval  0) {
  free(params);
  return VIR_PY_INT_FAIL;
  }
 @@ -292,7 +316,10 @@ libvirt_virDomainSetSchedulerParameters(PyObject *self 
 ATTRIBUTE_UNUSED,
  }
  }
  
 -if (virDomainSetSchedulerParameters(domain, params, nparams)  0) {
 +LIBVIRT_BEGIN_ALLOW_THREADS;
 +i_retval = virDomainSetSchedulerParameters(domain, params, nparams);
 +LIBVIRT_END_ALLOW_THREADS;
 +if (i_retval  0) {
  free(params);
  return VIR_PY_INT_FAIL;
  }
 @@ -311,13 +338,17 @@ libvirt_virDomainGetVcpus(PyObject *self 
 ATTRIBUTE_UNUSED,
  virVcpuInfoPtr cpuinfo = NULL;
  unsigned char *cpumap = NULL;
  int cpumaplen, i;
 +int i_retval;
  
  if (!PyArg_ParseTuple(args, (char *)O:virDomainGetVcpus,
pyobj_domain))
  return(NULL);
  domain = (virDomainPtr) PyvirDomain_Get(pyobj_domain);
  
 -if (virNodeGetInfo(virDomainGetConnect(domain), nodeinfo) != 0)
 +LIBVIRT_BEGIN_ALLOW_THREADS;
 +i_retval =

Re: [libvirt] crash in libvirtd when assigning PCI device (storage controller)

2009-11-25 Thread Daniel P. Berrange 
On Wed, Nov 25, 2009 at 11:23:07AM +, Daniel P. Berrange wrote:
 On Mon, Nov 23, 2009 at 10:08:17PM +, Terje Marthinussen wrote:
  [  987.286791] libvirtd[3193]: segfault at 10 ip 7f921b4546b4 sp 
  7f9216e165e0 error 4 in libpthread-2.10.1.so[7f921b44a000+17000]
 
 Ok, so this is the libvirtd daemon crash
 
  From GDB
  
  06:45:32.434: error : qemuMonitorCommandWithHandler:290 : cannot send 
  monitor command 'info cpus': Connection reset by peer
 
 Interestingly this suggests that QEMU itself had quit/crashed during
 startup. You can check /var/log/libvirt/qemu//$GUESTNAME.log to find
 out what problem QEMU itself had.
 
 For whatever reason, this seems to then trigger the libvirtd crash
 
  06:45:32.434: error : qemuMonitorTextGetCPUInfo:436 : internal error cannot 
  run monitor command to fetch CPU thread info
  
  Program received signal SIGSEGV, Segmentation fault.
  [Switching to Thread 0x7f485bfff910 (LWP 18120)]
  0x7f48657fb6b4 in pthread_mutex_unlock () from /lib/libpthread.so.0
  (gdb) bt
  #0  0x7f48657fb6b4 in pthread_mutex_unlock () from /lib/libpthread.so.0
  #1  0x00431101 in qemuDomainObjExitMonitorWithDriver 
  (driver=0x11f0110, obj=0x12114f0) at qemu/qemu_driver.c:318
  #2  0x0043f436 in qemudStartVMDaemon (conn=value optimized out, 
  driver=0x11f0110, vm=0x12114f0, 
  migrateFrom=value optimized out, stdin_fd=value optimized out) at 
  qemu/qemu_driver.c:2320
  #3  0x004407c4 in qemudDomainStart (dom=0x11f0330) at 
  qemu/qemu_driver.c:4370
  #4  0x7f4865a621e7 in virDomainCreate (domain=0x11f0330) at 
  libvirt.c:4509
  #5  0x00420d68 in remoteDispatchDomainCreate (server=value 
  optimized out, client=value optimized out, conn=0x1219da0, 
  hdr=value optimized out, rerr=0x7f485bffedf0, args=value optimized 
  out, ret=0x7f485bffeed0) at remote.c:853
  #6  0x004228e1 in remoteDispatchClientCall (server=value optimized 
  out, client=0x7f485c000d30, msg=0x7f485c080f00)
  at dispatch.c:506
  #7  0x00422c93 in remoteDispatchClientRequest (server=0x11e2790, 
  client=0x7f485c000d30, msg=0x7f485c080f00) at dispatch.c:388
  #8  0x0041625c in qemudWorker (data=value optimized out) at 
  libvirtd.c:1518
  #9  0x7f48657f7a04 in start_thread () from /lib/libpthread.so.0
  #10 0x7f48655617bd in clone () from /lib/libc.so.6
  #11 0x in ?? ()
  (gdb) select 2
  (gdb) info locals
  argv = 0x0
  tmp = value optimized out
  progenv = 0x0
  i = 1
  ret = 1
  sb = {st_dev = 64512, st_ino = 140307, st_nlink = 1, st_mode = 33261, 
  st_uid = 0, st_gid = 0, __pad0 = 0, st_rdev = 0, 
st_size = 2293664, st_blksize = 4096, st_blocks = 4480, st_atim = {tv_sec 
  = 1258937087, tv_nsec = 967226499}, st_mtim = {
  tv_sec = 1257180462, tv_nsec = 0}, st_ctim = {tv_sec = 1258886336, 
  tv_nsec = 104908677}, __unused = {0, 0, 0}}
  tapfds = 0x0
  ntapfds = 1
  qemuCmdFlags = 9436542
  keepfd = {fds_bits = {262144, 0 repeats 15 times}}
  emulator = value optimized out
  child = 18145
  pos = value optimized out
  ebuf = 
  \220v\036\001\000\000\000\000\060|WeH\177\000\000(\000\000\000\060\000\000\000\000\354\377[h\177\000\...@\353\377[h\177\000\000`\351\377[h\177\000\000\377\377\377\377h\177\000\000\233\017\037\001\000\000\000\000\326\063g\000\000\000\000\000@\247~eh\177\000\000\200\307oeh\177\000\000\240\306oeh\177\000\000jst\000\071\000\000\000\300\352\377[h\177\000\000\342\000\000\000\000\000\000\000\342\000\000\000\000\000\000\000\273\340g\000\000\000\000\000\300\352\377[h\177\000\000k\000\000\000\000\000\000\000`\351\377[h\177\000\...@\354\377[h\177\000\000p\352\377[h\177\000\000\001\000\000\000\000\000\000\000\320\352\377[h\177\000\000$\000\000\000\000\000\000\000py\255eh\177\000\000@\354\377[H\177\000\000\060|WeH\177\000\000\001\200\255\373\000\000\000\000\260\351\377[H\177\000\000\220\354\377[H\177\000\000\240\352\377[...
  pidfile = 0x0
  logfile = 15
  hookData = {conn = 0x1219da0, vm = 0x12114f0, driver = 0x11f0110}
  __FUNCTION__ = qemudStartVMDaemon
  __func__ = qemudStartVMDaemon
 
 
 If you are able to reproduce this crash reliably, can you edit the
 /etc/libvirt/libvirtd.conf file and add
 
 
   log_filters=1:qemu
   log_outputs=1:file:/var/log/libvirtd.log
 
 and then restart libvirtd, and try to make it crash again. Then send
 us the log file

Don't bother doing this now - I've figured out the cause of the problem
and have a fix pending..


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

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

[libvirt] [PATCH 0/3] fix migration of paused vms

2009-11-25 Thread Paolo Bonzini 
From: Paolo Bonzini pbonz...@gnu.org

This is yet another try on fixing migration of paused vms.  Since the
driver-only solution didn't work out, I'm now fixing it directly in
libvirt.c.

The first two patches are the same as for the previous series.  The
first is a bugfix for failed migration for the QEMU driver; the second
adds a virsh migrate --suspend option to leave the VM not running
on the destination machine.

As in the previous version of the patch, --suspend/VIR_MIGRATE_PAUSED
is the bulk of implementing the new feature.  I just query the domain
state and, if it is paused, I implicitly add the VIR_MIGRATE_PAUSED
flag to the migration.  This is the third patch.

I thought about transmitting the state in the end (since it works well
if VIR_MIGRATE_PAUSED is passed only to MigrateFinish and not to
MigratePerform), but I don't think it's a good idea to commit it
now as it's basically untestable.  It's in the fourth patch, but if
you want it now and think the approach is sound, tell me and I'll
test it more heavily.


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

[libvirt] [PATCH 1/3] fix migration of paused vms upon failure

2009-11-25 Thread Paolo Bonzini 
This makes a small change on the failed-migration path.  Up to now,
all VMs that failed non-live migration after the stop command
were restarted.  This must not be done when the VM was paused in
the first place.

* src/qemu/qemu_driver.c (qemudDomainMigratePerform): Do not restart
a paused VM that fails migration.  Set paused state after stop,
reset it after failure.
---
 src/qemu/qemu_driver.c |4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 356e4e7..44cec6c 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -7182,7 +7182,7 @@ qemudDomainMigratePerform (virDomainPtr dom,
 goto endjob;
 }
 
-if (!(flags  VIR_MIGRATE_LIVE)) {
+if (!(flags  VIR_MIGRATE_LIVE)  vm-state == VIR_DOMAIN_RUNNING) {
 qemuDomainObjPrivatePtr priv = vm-privateData;
 /* Pause domain for non-live migration */
 qemuDomainObjEnterMonitorWithDriver(driver, vm);
@@ -7193,6 +7193,7 @@ qemudDomainMigratePerform (virDomainPtr dom,
 qemuDomainObjExitMonitorWithDriver(driver, vm);
 paused = 1;
 
+vm-state = VIR_DOMAIN_PAUSED;
 event = virDomainEventNewFromObj(vm,
  VIR_DOMAIN_EVENT_SUSPENDED,
  VIR_DOMAIN_EVENT_SUSPENDED_MIGRATED);
@@ -7240,6 +7241,7 @@ endjob:
 }
 qemuDomainObjExitMonitorWithDriver(driver, vm);
 
+vm-state = VIR_DOMAIN_RUNNING;
 event = virDomainEventNewFromObj(vm,
  VIR_DOMAIN_EVENT_RESUMED,
  VIR_DOMAIN_EVENT_RESUMED_MIGRATED);
-- 
1.6.5.2


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

[libvirt] [PATCH 2/3] add virsh --suspend

2009-11-25 Thread Paolo Bonzini 
This adds a new flag, VIR_MIGRATE_PAUSED, that mandates pausing
the migrated VM before starting it.

* include/libvirt/libvirt.h.in (virDomainMigrateFlags): Add
VIR_MIGRATE_PAUSED.
* src/qemu/qemu_driver.c (qemudDomainMigrateFinish2): Handle
VIR_MIGRATE_PAUSED.
* tools/virsh.c (opts_migrate): Add --suspend.
(cmdMigrate): Handle it.
* tools/virsh.pod (migrate): Document it.
---
The complicated part of the patch is simply this with diff -b:

@@ -7320,6 +7320,7 @@ qemudDomainMigrateFinish2 (virConnectPtr dconn,
 qemuDomainObjPrivatePtr priv = vm-privateData;
 dom = virGetDomain (dconn, vm-def-name, vm-def-uuid);
 
+if (!(flags  VIR_MIGRATE_PAUSED)) {
 /* run 'cont' on the destination, which allows migration on 
qemu
  * = 0.10.6 to work properly.  This isn't strictly necessary 
on
  * older qemu's, but it also doesn't hurt anything there
@@ -7335,9 +7336,17 @@ qemudDomainMigrateFinish2 (virConnectPtr dconn,
 qemuDomainObjExitMonitorWithDriver(driver, vm);
 
 vm-state = VIR_DOMAIN_RUNNING;
+}

 include/libvirt/libvirt.h.in |1 +
 src/libvirt.c|1 +
 src/qemu/qemu_driver.c   |   33 +
 tools/virsh.c|5 -
 tools/virsh.pod  |7 ---
 5 files changed, 31 insertions(+), 16 deletions(-)

diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
index 5bc7694..0488cbf 100644
--- a/include/libvirt/libvirt.h.in
+++ b/include/libvirt/libvirt.h.in
@@ -341,6 +341,7 @@ typedef enum {
 VIR_MIGRATE_TUNNELLED = (1  2), /* tunnel migration data over 
libvirtd connection */
 VIR_MIGRATE_PERSIST_DEST  = (1  3), /* persist the VM on the 
destination */
 VIR_MIGRATE_UNDEFINE_SOURCE   = (1  4), /* undefine the VM on the source 
*/
+VIR_MIGRATE_PAUSED= (1  5), /* pause on remote side */
 } virDomainMigrateFlags;
 
 /* Domain migration. */
diff --git a/src/libvirt.c b/src/libvirt.c
index 05e45f3..2ced604 100644
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -3179,6 +3179,7 @@ virDomainMigrateDirect (virDomainPtr domain,
  *on the destination host.
  *   VIR_MIGRATE_UNDEFINE_SOURCE If the migration is successful, undefine the
  *   domain on the source host.
+ *   VIR_MIGRATE_PAUSEDLeave the domain suspended on the remote side.
  *
  * VIR_MIGRATE_TUNNELLED requires that VIR_MIGRATE_PEER2PEER be set.
  * Applications using the VIR_MIGRATE_PEER2PEER flag will probably
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 44cec6c..4d20fb7 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -7320,24 +7320,33 @@ qemudDomainMigrateFinish2 (virConnectPtr dconn,
 qemuDomainObjPrivatePtr priv = vm-privateData;
 dom = virGetDomain (dconn, vm-def-name, vm-def-uuid);
 
-/* run 'cont' on the destination, which allows migration on qemu
- * = 0.10.6 to work properly.  This isn't strictly necessary on
- * older qemu's, but it also doesn't hurt anything there
- */
-qemuDomainObjEnterMonitorWithDriver(driver, vm);
-if (qemuMonitorStartCPUs(priv-mon, dconn)  0) {
-if (virGetLastError() == NULL)
-qemudReportError(dconn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
- %s, _(resume operation failed));
+if (!(flags  VIR_MIGRATE_PAUSED)) {
+/* run 'cont' on the destination, which allows migration on qemu
+ * = 0.10.6 to work properly.  This isn't strictly necessary on
+ * older qemu's, but it also doesn't hurt anything there
+ */
+qemuDomainObjEnterMonitorWithDriver(driver, vm);
+if (qemuMonitorStartCPUs(priv-mon, dconn)  0) {
+if (virGetLastError() == NULL)
+qemudReportError(dconn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
+ %s, _(resume operation failed));
+qemuDomainObjExitMonitorWithDriver(driver, vm);
+goto endjob;
+}
 qemuDomainObjExitMonitorWithDriver(driver, vm);
-goto endjob;
+
+vm-state = VIR_DOMAIN_RUNNING;
 }
-qemuDomainObjExitMonitorWithDriver(driver, vm);
 
-vm-state = VIR_DOMAIN_RUNNING;
 event = virDomainEventNewFromObj(vm,
  VIR_DOMAIN_EVENT_RESUMED,
  VIR_DOMAIN_EVENT_RESUMED_MIGRATED);
+if (vm-state == VIR_DOMAIN_PAUSED) {
+qemuDomainEventQueue(driver, event);
+event = virDomainEventNewFromObj(vm,
+ VIR_DOMAIN_EVENT_SUSPENDED,
+

[libvirt] [PATCH 3/3] retrieve paused/running state at the beginning of migration

2009-11-25 Thread Paolo Bonzini 
This patch fixes the bug where paused/running state is not
transmitted during migration.  As a result, in the QEMU driver
for example the machine was always started on the destination
end.

In order to do so, I just read the state and if it is appropriate
I set the VIR_MIGRATE_PAUSED flag.

* src/libvirt.c (virDomainMigrateVersion1, virDomainMigrateVersion2):
Automatically add VIR_MIGRATE_PAUSED when appropriate.
* src/xen/xend_internal.c (xenDaemonDomainMigratePerform): Give a nicer
error message when migration of paused virtual machines is attempted.
---
 src/libvirt.c   |   12 
 src/xen/xend_internal.c |9 +
 2 files changed, 21 insertions(+), 0 deletions(-)

diff --git a/src/libvirt.c b/src/libvirt.c
index 2ced604..9d03e13 100644
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -2964,6 +2964,12 @@ virDomainMigrateVersion1 (virDomainPtr domain,
 char *uri_out = NULL;
 char *cookie = NULL;
 int cookielen = 0;
+virDomainInfo info;
+
+virDomainGetInfo (domain, info);
+if (info.state == VIR_DOMAIN_PAUSED) {
+flags |= VIR_MIGRATE_PAUSED;
+}
 
 /* Prepare the migration.
  *
@@ -3028,6 +3034,7 @@ virDomainMigrateVersion2 (virDomainPtr domain,
 char *cookie = NULL;
 char *dom_xml = NULL;
 int cookielen = 0, ret;
+virDomainInfo info;
 
 /* Prepare the migration.
  *
@@ -3054,6 +3061,11 @@ virDomainMigrateVersion2 (virDomainPtr domain,
 if (!dom_xml)
 return NULL;
 
+virDomainGetInfo (domain, info);
+if (info.state == VIR_DOMAIN_PAUSED) {
+flags |= VIR_MIGRATE_PAUSED;
+}
+
 ret = dconn-driver-domainMigratePrepare2
 (dconn, cookie, cookielen, uri, uri_out, flags, dname,
  bandwidth, dom_xml);
diff --git a/src/xen/xend_internal.c b/src/xen/xend_internal.c
index e370eb8..36148d9 100644
--- a/src/xen/xend_internal.c
+++ b/src/xen/xend_internal.c
@@ -4434,6 +4434,15 @@ xenDaemonDomainMigratePerform (virDomainPtr domain,
 if (flags  VIR_MIGRATE_PERSIST_DEST)
 flags = ~VIR_MIGRATE_PERSIST_DEST;
 
+/* This could be supported in principle (disregard VIR_MIGRATE_LIVE, pause
+ * the VM in advance) but is buggy in Xend; give a nice error message.
+ */
+if (flags  VIR_MIGRATE_PAUSED) {
+virXendError (conn, VIR_ERR_NO_SUPPORT,
+  %s, _(xenDaemonDomainMigrate: xend cannot migrate 
paused virtual machines));
+return -1;
+}
+
 /* XXX we could easily do tunnelled  peer2peer migration too
if we want to. support these... */
 if (flags != 0) {
-- 
1.6.5.2


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

[libvirt] [PATCH 4/3] [RFC] pass paused/running state at the end of the migration

2009-11-25 Thread Paolo Bonzini 
This patch would fix the case where paused/running state is changed
during live migration.  In order to do so, libvirt attaches an
event handler that tracks the paused/running state on the source side.
Then, based on the state _just before_ the end of the migration, it can
stick a VIR_MIGRATE_PAUSED flag into the flags passed to MigrateFinish
or MigrateFinish2.  It is not a problem for the QEMU driver if the flag
is not passed in the Perform callback.

This would have the advantage of working even with asynchronous changes
to the state that can happen during the migration itself.  However, since
the driver lock is held during migration, this is basically impossible to
test until asynchronous notifications are in place.  Also, it is somewhat
inherently racy: maybe when notifications are added to QEMU, it should
also have a sync monitor command to flush all notifications.

Hence, this is just an RFC.

* src/libvirt.c (struct _migrateEventState, migrateEventState,
virMigrateEventCallback): New.
(virDomainMigrateVersion1, virDomainMigrateVersion2): Use them.
---
 src/libvirt.c |   68 ++--
 1 files changed, 60 insertions(+), 8 deletions(-)

diff --git a/src/libvirt.c b/src/libvirt.c
index 9d03e13..12e90cd 100644
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -2951,6 +2951,42 @@ error:
 return NULL;
 }
 
+typedef struct _migrateEventData migrateEventData;
+
+struct _migrateEventData {
+virDomainPtr   domain;
+virDomainInfo  info;
+};
+
+static int
+virMigrateEventCallback(virConnectPtr conn ATTRIBUTE_UNUSED,
+   virDomainPtr dom,
+   int event,
+   int detail,
+   void *opaque)
+{
+migrateEventData *data = opaque;
+
+if (dom != data-domain)
+   return 0;
+
+switch (event)
+{
+case VIR_DOMAIN_EVENT_SUSPENDED:
+   if (detail != VIR_DOMAIN_EVENT_SUSPENDED_MIGRATED)
+   data-info.state = VIR_DOMAIN_PAUSED;
+   break;
+case VIR_DOMAIN_EVENT_STOPPED:
+   if (detail == VIR_DOMAIN_EVENT_STOPPED_CRASHED)
+   data-info.state = VIR_DOMAIN_CRASHED;
+   else if (detail != VIR_DOMAIN_EVENT_STOPPED_MIGRATED)
+   data-info.state = VIR_DOMAIN_SHUTOFF;
+   break;
+}
+
+return 0;
+}
+
 
 static virDomainPtr
 virDomainMigrateVersion1 (virDomainPtr domain,
@@ -2964,12 +3000,14 @@ virDomainMigrateVersion1 (virDomainPtr domain,
 char *uri_out = NULL;
 char *cookie = NULL;
 int cookielen = 0;
-virDomainInfo info;
+migrateEventData data;
 
-virDomainGetInfo (domain, info);
-if (info.state == VIR_DOMAIN_PAUSED) {
-flags |= VIR_MIGRATE_PAUSED;
+data.domain = domain;
+if (virConnectDomainEventRegister (domain-conn, virMigrateEventCallback,
+   data, NULL) == -1) {
+return NULL;
 }
+virDomainGetInfo (domain, data.info);
 
 /* Prepare the migration.
  *
@@ -3003,6 +3041,11 @@ virDomainMigrateVersion1 (virDomainPtr domain,
 (domain, cookie, cookielen, uri, flags, dname, bandwidth) == -1)
 goto done;
 
+/* TODO: what if the host has crashed during live migration? */
+if (data.info.state == VIR_DOMAIN_PAUSED) {
+flags |= VIR_MIGRATE_PAUSED;
+}
+
 /* Get the destination domain and return it or error.
  * 'domain' no longer actually exists at this point
  * (or so we hope), but we still use the object in memory
@@ -3016,6 +3059,7 @@ virDomainMigrateVersion1 (virDomainPtr domain,
 ddomain = virDomainLookupByName (dconn, dname);
 
  done:
+virConnectDomainEventDeregister (domain-conn, virMigrateEventCallback);
 VIR_FREE (uri_out);
 VIR_FREE (cookie);
 return ddomain;
@@ -3034,7 +3078,7 @@ virDomainMigrateVersion2 (virDomainPtr domain,
 char *cookie = NULL;
 char *dom_xml = NULL;
 int cookielen = 0, ret;
-virDomainInfo info;
+migrateEventData data;
 
 /* Prepare the migration.
  *
@@ -3061,10 +3105,12 @@ virDomainMigrateVersion2 (virDomainPtr domain,
 if (!dom_xml)
 return NULL;
 
-virDomainGetInfo (domain, info);
-if (info.state == VIR_DOMAIN_PAUSED) {
-flags |= VIR_MIGRATE_PAUSED;
+data.domain = domain;
+if (virConnectDomainEventRegister (domain-conn, virMigrateEventCallback,
+   data, NULL) == -1) {
+return NULL;
 }
+virDomainGetInfo (domain, data.info);
 
 ret = dconn-driver-domainMigratePrepare2
 (dconn, cookie, cookielen, uri, uri_out, flags, dname,
@@ -3088,6 +3134,11 @@ virDomainMigrateVersion2 (virDomainPtr domain,
 ret = domain-conn-driver-domainMigratePerform
 (domain, cookie, cookielen, uri, flags, dname, bandwidth);
 
+/* TODO: what if the host has crashed during live migration? */
+if (data.info.state == VIR_DOMAIN_PAUSED) {
+flags |= VIR_MIGRATE_PAUSED;
+}
+
 /* In version 2 of the

[libvirt] How do I import an old VM image to a new Install

2009-11-25 Thread Michael N. Moran 

First, I am a casual VM user. I have been using
Fedora Core 7 on my Dell laptop for a couple of
years with a single QEMU/KVM guest running Windoze XP.

I recently installed Fedora 12 on a new hard disk
and would like to use my old Windows VM image
on the new install.

Is there an easy way to do this?

I tried copying the image itself (WindozeXpDisk.raw)
to the new install and then (naively) looked for an
import on the Virtual Machine Manager GUI. The closest
that I found was the File-Restore saved machine.
I tried that on my WindozeXpDisk.raw image and it
failed as follows:

Traceback (most recent call last):
  File /usr/share/virt-manager/virtManager/manager.py, 
line 461, in restore_saved_callback

newconn.restore(file_to_load)
  File /usr/share/virt-manager/virtManager/connection.py, 
line 649, in restore

self.vmm.restore(frm)
  File /usr/lib64/python2.6/site-packages/libvirt.py, 
line 1420, in restore
if ret == -1: raise libvirtError ('virDomainRestore() 
failed', conn=self)

libvirtError: operation failed: image magic is incorrect



I *can* mount the image using kpartx et. al like this;

sudo kpartx -av /var/virt/WindozeXpDisk.raw
 add map loop1p1 (253:2): 0 31101777 linear /dev/loop1 63

sudo mount -t ntfs /dev/mapper/loop0p1 ./mnt

Any clues would be helpful.

thank you,
mike

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

Re: [libvirt] How do I import an old VM image to a new Install

2009-11-25 Thread Cole Robinson 
On 11/25/2009 11:36 AM, Michael N. Moran wrote:
 First, I am a casual VM user. I have been using
 Fedora Core 7 on my Dell laptop for a couple of
 years with a single QEMU/KVM guest running Windoze XP.
 
 I recently installed Fedora 12 on a new hard disk
 and would like to use my old Windows VM image
 on the new install.
 
 Is there an easy way to do this?
 
 I tried copying the image itself (WindozeXpDisk.raw)
 to the new install and then (naively) looked for an
 import on the Virtual Machine Manager GUI. The closest
 that I found was the File-Restore saved machine.
 I tried that on my WindozeXpDisk.raw image and it
 failed as follows:
 
 Traceback (most recent call last):
File /usr/share/virt-manager/virtManager/manager.py, 
 line 461, in restore_saved_callback
  newconn.restore(file_to_load)
File /usr/share/virt-manager/virtManager/connection.py, 
 line 649, in restore
  self.vmm.restore(frm)
File /usr/lib64/python2.6/site-packages/libvirt.py, 
 line 1420, in restore
  if ret == -1: raise libvirtError ('virDomainRestore() 
 failed', conn=self)
 libvirtError: operation failed: image magic is incorrect
 

Restore isn't what you want here, and though the above error sounds
scary it is actually correct. Restore starts a VM that was previously
'saved', which is kind of like suspend to ram on a physical machine.

You should be able to use virt-install --import to get what you want,
something like

virt-install --name foo --ram 1024 --os-variant winxp --import --disk
path=/path/to/your/disk/image ...

- Cole

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

Re: [libvirt] How do I import an old VM image to a new Install

2009-11-25 Thread Michael N. Moran 

On 11/25/2009 12:53 PM, Cole Robinson wrote:

On 11/25/2009 11:36 AM, Michael N. Moran wrote:

First, I am a casual VM user. I have been using
Fedora Core 7 on my Dell laptop for a couple of
years with a single QEMU/KVM guest running Windoze XP.

I recently installed Fedora 12 on a new hard disk
and would like to use my old Windows VM image
on the new install.

Is there an easy way to do this?



libvirtError: operation failed: image magic is incorrect



Restore isn't what you want here, and though the above error sounds
scary it is actually correct. Restore starts a VM that was previously
'saved', which is kind of like suspend to ram on a physical machine.

You should be able to use virt-install --import to get what you want,
something like

virt-install --name foo --ram 1024 --os-variant winxp --import --disk
path=/path/to/your/disk/image ...


Thanks Cole, that's what I was looking for. I used the
following line:

virt-install --name WindozeXP --ram 512 --os-variant winxp \
--import --disk path=/var/virt/WindozeXpDisk.raw

Virt Viewer came up and the XP boot screen was shown for
about a minute and then the I experienced a BSOD with an
UNMOUNTABLE_BOOT_VOLUME error.

I assume that the problem is that I need to specify a
VM that has the same virtual hardware characteristics as
my previous environment. I suppose that can be gleaned
from /etc/libvirt/qemu/WindozeXP.xml or similar.

I've attached that old WindozeXP.xml file in an attempt
to be complete.

If there is an easy way to import that information or
if I'm on the wrong track, please let me know.
domain type='kvm'
  nameWindozeXP/name
  uuid711a37f1-f06b-36ea-d413-17519ba1d717/uuid
  memory524288/memory
  currentMemory524288/currentMemory
  vcpu1/vcpu
  os
typehvm/type
boot dev='hd'/
  /os
  features
acpi/
  /features
  clock offset='utc'/
  on_poweroffdestroy/on_poweroff
  on_rebootrestart/on_reboot
  on_crashdestroy/on_crash
  devices
emulator/usr/bin/qemu-kvm/emulator
disk type='file' device='disk'
  source file='/var/virt/WindozeXpDisk.raw'/
  target dev='hda'/
/disk
disk type='file' device='cdrom'
  source file='/dev/cdrom'/
  target dev='hdc'/
/disk
interface type='network'
  mac address='00:0b:db:1b:7e:f3'/
  source network='default'/
  target dev='vnet0'/
/interface
input type='mouse' bus='ps2'/
graphics type='vnc' port='-1' listen='127.0.0.1'/
  /devices
/domain
--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] How do I import an old VM image to a new Install

2009-11-25 Thread Cole Robinson 
On 11/25/2009 01:27 PM, Michael N. Moran wrote:
 On 11/25/2009 12:53 PM, Cole Robinson wrote:
 On 11/25/2009 11:36 AM, Michael N. Moran wrote:
 First, I am a casual VM user. I have been using
 Fedora Core 7 on my Dell laptop for a couple of
 years with a single QEMU/KVM guest running Windoze XP.

 I recently installed Fedora 12 on a new hard disk
 and would like to use my old Windows VM image
 on the new install.

 Is there an easy way to do this?
 
 libvirtError: operation failed: image magic is incorrect


 Restore isn't what you want here, and though the above error sounds
 scary it is actually correct. Restore starts a VM that was previously
 'saved', which is kind of like suspend to ram on a physical machine.

 You should be able to use virt-install --import to get what you want,
 something like

 virt-install --name foo --ram 1024 --os-variant winxp --import --disk
 path=/path/to/your/disk/image ...
 
 Thanks Cole, that's what I was looking for. I used the
 following line:
 
 virt-install --name WindozeXP --ram 512 --os-variant winxp \
 --import --disk path=/var/virt/WindozeXpDisk.raw
 
 Virt Viewer came up and the XP boot screen was shown for
 about a minute and then the I experienced a BSOD with an
 UNMOUNTABLE_BOOT_VOLUME error.
 
 I assume that the problem is that I need to specify a
 VM that has the same virtual hardware characteristics as
 my previous environment. I suppose that can be gleaned
 from /etc/libvirt/qemu/WindozeXP.xml or similar.
 
 I've attached that old WindozeXP.xml file in an attempt
 to be complete.
 
 If there is an easy way to import that information or
 if I'm on the wrong track, please let me know.
 

You can use 'virsh edit vmname' on the new VM and try to make it match
the old XML as best as possible. A few things to try:

- Sync the mac addresses
- Add model type='rtl8139'/ to the interface block
- Make sure acpi + apic, memory, vcpu settings are the same

It could also be that the jump from f7 to f12 is just too drastic, and
behavior has changed enough at the qemu/kvm level to upset the guest.
Not really sure though.

- Cole

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

Re: [libvirt] [PATCH] Put libraries in $LIBS, not $LDFLAGS, during configure tests.

2009-11-25 Thread Nix 
On 25 Nov 2009, Daniel P. Berrange spake thusly:

 On Mon, Nov 23, 2009 at 09:28:24PM +, Nix wrote:
 If libraries go in $LDFLAGS while AC_CHECK_LIBbing, they'll end up in
 front of the object file name, which rarely works well. They belong
 in $LIBS.

 What platforms / scenarios show this problem ?   Empirically it has
 been working fine on our commonly tested platforms, so it'll be
 important to know where to look/test for this problem.

Autoconf has always sorted $LDFLAGS before object files. When
-Wl,--as-needed is turned on by default (as a number of distros do),
such libraries are silently dropped because nothing needs symbols from
them at that point in the link, just as if they were static
libraries. Without -Wl,--as-needed, every single library named in the
link is added to DT_NEEDED, so this works.

Nonetheless, it is plainly obvious to any Unix hand that

cc -L/usr/lib -lfoo -o test thing-that-uses-foo.o

is less likely to work than

cc -L/usr/lib -o test thing-that-uses-foo.o -lfoo

and if you put libraries in LDFLAGS, the former's often what you get.
It'll probably fail for you, too, if HAL (say) is statically linked,
because the -Wl,--as-needed behaviour for shared libraries is exactly
how static libraries have always worked.

The GNU Autoconf manual has long stated

 -- Variable: LDFLAGS
[...]
 Don't use this variable to pass library names (`-l') to the
 linker; use `LIBS' instead.


(I'm surprised that no Gentooers, for instance, have reported this
already. A *lot* of distros turn on -Wl,--as-needed by default these
days, often by modifying the built-in GCC specs and providing a fallback
specs file that specifies -Wl,--no-as-needed for those very rare things,
such as glibc's testsuite, that actually *require* linkage with shared
libraries from which no symbols are required.)

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