Re: [libvirt] [PATCH 0/4] qemu: export disk snapshot capability

2014-02-19 Thread Francesco Romani
- Original Message -
  This patch series extend the QEMU capabilities XML to report
  if the underlying QEMU binary supports, or not, the live
  disk snapshotting.
 Could please someone take a look and review this? Any comment would be really
 appreciated!

ping

-- 
Francesco Romani
RedHat Engineering Virtualization R  D
Phone: 8261328
IRC: fromani

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


Re: [libvirt] [PATCH v9 0/7] Support keyboard device

2014-02-19 Thread Ján Tomko
On 02/19/2014 07:17 AM, Li Zhang wrote:
 On 2014年02月18日 20:52, Ján Tomko wrote:
 On 02/18/2014 09:58 AM, Li Zhang wrote:
 Hi Jan and Daniel,

 Would you like to accept this patchset?

 Thanks.

 Hi,

 it looks good to me and I'd like to push it this week with the changes I
 pointed out, unless someone has a different opinion.
 
 Thanks a lot.
 

I've pushed it now.

Jan




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 v9 2/7] conf: Add a keyboard input device type

2014-02-19 Thread Ján Tomko
On 02/19/2014 07:22 AM, Li Zhang wrote:
 On 2014年02月18日 20:51, Ján Tomko wrote:
 On 02/17/2014 11:17 AM, Li Zhang wrote:
 From: Li Zhang zhlci...@linux.vnet.ibm.com

 There is no keyboard support currently in libvirt .
 For some platforms, it needs to add a USB keyboard when graphics are 
 enabled.

 This patch is to add keyboard input device type.

 Signed-off-by: Li Zhang zhlci...@linux.vnet.ibm.com
 ---
   docs/schemas/domaincommon.rng  |  1 +
   src/conf/domain_conf.c | 70
 --
   src/conf/domain_conf.h |  1 +
 ...
   tests/vmx2xmldata/vmx2xml-graphics-vnc.xml |  1 +
   27 files changed, 66 insertions(+), 31 deletions(-)

 This fails 'make check' for me:
 FAIL: sexpr2xmltest
 FAIL: xmconfigtest
 
 Ah, I can't get any errors when doing 'make check' on my side. :(
 

Libvirt needs to be built --with-xen for those tests to work. You probably
don't have the xen library headers on your system.

Jan




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

Re: [libvirt] Is it possible to set a timeout to the Connection class?

2014-02-19 Thread Claudio Bley

Am 18.02.2014 15:02, schrieb Pasquale Dir:

I am using the java api bindings.

I'd like to set a connection timeout on the Connect class as sometimes
it takes just too long.


This depends on the driver you're using.

If the connection is established over the network, you'd have to change 
the TCP connection timeout for your OS. (Other than that, libvirt does 
not provide a method to change this per connection AFAIK)



Is there any way/workaround?


Create your Connect object in another thread using a Future. Then, await 
the result of that future using a custom timeout.


Claudio

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


[libvirt] [bug-report] libvirtd 1.1.4-maint with lastest patches can't be connected

2014-02-19 Thread Wangpan
Hi experts,
Is there anybody could help me to fix this bug:
https://bugzilla.redhat.com/show_bug.cgi?id=1066801

my libvirt version is 1.1.4-manit without the newest patches after this one:
13 days ago Eric Blake  event: move event filtering to daemon 
(regression fix)

Description of problem:
After I cherry-picked these patches of 1.1.4-maint branch(I want to fix the 
dead lock and a libvirtd crash bugs of libvirtd,
https://bugzilla.redhat.com/show_bug.cgi?id=929412
https://bugzilla.redhat.com/show_bug.cgi?id=1058149),
my libvirtd can be started, but the connection can't be established,
and libvirtd failed to stop after 'service libvirt-bin stop', as well as 
'ctrl+c' when I run 'libvirtd -v' on the foreground.

Steps to Reproduce:
1. create many VMs(40 in my env) on the host
2. kill -9 `pid of libvirtd`
3. start libvirtd by using service libvirt-bin start
4. virsh list or virsh version

Thanks a lot!

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


Re: [libvirt] java api bindings for detecting domain events

2014-02-19 Thread Claudio Bley

Am 18.02.2014 18:15, schrieb Avanti Ajay:

Hi..

I am doing a project in which I need to detect KVM domain events and I
am using java for my project. On the net I found many patches for the
libvirt Java API bindings for event detection. Does the latest version
0.5 have these patches included


No, not yet. Currently, v2 of these patches[1] was posted for discussion 
on this list and will be pushed in a week if nobody objects.



ie can I use the method
Domain.domainEventRegisterAny() directly. Also can u please tell me
exactly how to use this method?


Don't use this method. It won't work.

I'd suggest you checkout the project from the git repo[2], apply the 
whole patchset[1] and build the jar yourself. Then have a look at the 
JUnit test for domain lifecycle events[3] for a usage example.


If you have any questions, just don't hesitate to ask. Comments are also 
welcome.


Claudio

[1]: https://www.redhat.com/archives/libvir-list/2014-February/msg00823.html
[2]: http://libvirt.org/git/?p=libvirt-java.git;a=summary
[3]: https://www.redhat.com/archives/libvir-list/2014-February/msg00875.html

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


Re: [libvirt] Is it possible to set a timeout to the Connection class?

2014-02-19 Thread Daniel P. Berrange
On Tue, Feb 18, 2014 at 04:13:19PM -0700, Eric Blake wrote:
 On 02/18/2014 07:02 AM, Pasquale Dir wrote:
  I am using the java api bindings.
  
  I'd like to set a connection timeout on the Connect class as sometimes it
  takes just too long.
 
 Which particular API takes too long?  There are some APIs like migration
 that take a long time, but where you can use another thread on the same
 connection object (or even a separate connection object) to track the
 status of it.  Newer APIs such as block-pull were designed to start an
 asynchronous job and return immediately, where you can then track job
 status and/or abort the job early via separate API calls.  We already
 need to enhance our job control APIs to support parallel jobs (qemu just
 added the concept of a block-backup for image fleecing, and there is
 some desire to be able to fleece images from multiple points in time
 which would require multiple jobs) - as part of enhancing job support
 there, we might also be able to enhance migration to have a asynchronous
 mode instead of blocking for the entire operation.  We also want to add
 job control for various storage volume operations, which are currently
 long-running but can't be interrupted easily.
 
 Most APIs return as fast as possible after taking effect (although on a
 heavily loaded machine, that can take a while).  Depending on the
 command, aborting early because of a timeout may actually be worse than
 using a different thread to track progress, unless the command is
 already associated with job control.

FWIW, for the initial 'virConnectOpen' API I think it probably would
be worthwhile us supporting a standardized timeout URI parameter.
That way if the remote service doesn't respond at all for some
reason users can have fine control.  That's a sufficiently targetted
use case that it'd be easy to do, compared to timeouts for arbitrary
APIs.

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] make virDomainGetMaxVcpus work on inactive domains

2014-02-19 Thread Claudio Bley

Signed-off-by: Claudio Bley cb...@av-test.de
---
OK, how about this patch?

While at it, should I convert the VIR_DOMAIN_VCPU_* instances to
VIR_DOMAIN_AFFECT_* instances for consistency?

 src/esx/esx_driver.c   |2 +-
 src/openvz/openvz_driver.c |2 +-
 src/phyp/phyp_driver.c |2 +-
 src/qemu/qemu_driver.c |2 +-
 src/test/test_driver.c |2 +-
 src/vbox/vbox_tmpl.c   |2 +-
 src/xen/xen_driver.c   |2 +-
 src/xenapi/xenapi_driver.c |2 +-
 8 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c
index 886d984..6e05d78 100644
--- a/src/esx/esx_driver.c
+++ b/src/esx/esx_driver.c
@@ -2646,7 +2646,7 @@ esxDomainGetVcpusFlags(virDomainPtr domain, unsigned int 
flags)
 static int
 esxDomainGetMaxVcpus(virDomainPtr domain)
 {
-return esxDomainGetVcpusFlags(domain, (VIR_DOMAIN_AFFECT_LIVE |
+return esxDomainGetVcpusFlags(domain, (VIR_DOMAIN_AFFECT_CURRENT |
VIR_DOMAIN_VCPU_MAXIMUM));
 }
 
diff --git a/src/openvz/openvz_driver.c b/src/openvz/openvz_driver.c
index b27ac4c..e1f9738 100644
--- a/src/openvz/openvz_driver.c
+++ b/src/openvz/openvz_driver.c
@@ -1318,7 +1318,7 @@ openvzDomainGetVcpusFlags(virDomainPtr dom 
ATTRIBUTE_UNUSED,
 
 static int openvzDomainGetMaxVcpus(virDomainPtr dom)
 {
-return openvzDomainGetVcpusFlags(dom, (VIR_DOMAIN_AFFECT_LIVE |
+return openvzDomainGetVcpusFlags(dom, (VIR_DOMAIN_AFFECT_CURRENT |
VIR_DOMAIN_VCPU_MAXIMUM));
 }
 
diff --git a/src/phyp/phyp_driver.c b/src/phyp/phyp_driver.c
index 9adb6b0..4510b82 100644
--- a/src/phyp/phyp_driver.c
+++ b/src/phyp/phyp_driver.c
@@ -1372,7 +1372,7 @@ phypDomainGetVcpusFlags(virDomainPtr dom, unsigned int 
flags)
 static int
 phypDomainGetMaxVcpus(virDomainPtr dom)
 {
-return phypDomainGetVcpusFlags(dom, (VIR_DOMAIN_VCPU_LIVE |
+return phypDomainGetVcpusFlags(dom, (VIR_DOMAIN_VCPU_CURRENT |
  VIR_DOMAIN_VCPU_MAXIMUM));
 }
 
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 6ea837e..2068ace 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -4986,7 +4986,7 @@ cleanup:
 static int
 qemuDomainGetMaxVcpus(virDomainPtr dom)
 {
-return qemuDomainGetVcpusFlags(dom, (VIR_DOMAIN_AFFECT_LIVE |
+return qemuDomainGetVcpusFlags(dom, (VIR_DOMAIN_AFFECT_CURRENT |
  VIR_DOMAIN_VCPU_MAXIMUM));
 }
 
diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index b724f82..a320241 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -2620,7 +2620,7 @@ cleanup:
 static int
 testDomainGetMaxVcpus(virDomainPtr domain)
 {
-return testDomainGetVcpusFlags(domain, (VIR_DOMAIN_AFFECT_LIVE |
+return testDomainGetVcpusFlags(domain, (VIR_DOMAIN_AFFECT_CURRENT |
 VIR_DOMAIN_VCPU_MAXIMUM));
 }
 
diff --git a/src/vbox/vbox_tmpl.c b/src/vbox/vbox_tmpl.c
index 382d7b4..fbae940 100644
--- a/src/vbox/vbox_tmpl.c
+++ b/src/vbox/vbox_tmpl.c
@@ -2208,7 +2208,7 @@ vboxDomainGetVcpusFlags(virDomainPtr dom, unsigned int 
flags)
 static int
 vboxDomainGetMaxVcpus(virDomainPtr dom)
 {
-return vboxDomainGetVcpusFlags(dom, (VIR_DOMAIN_AFFECT_LIVE |
+return vboxDomainGetVcpusFlags(dom, (VIR_DOMAIN_AFFECT_CURRENT |
  VIR_DOMAIN_VCPU_MAXIMUM));
 }
 
diff --git a/src/xen/xen_driver.c b/src/xen/xen_driver.c
index 7506e8c..38bbce0 100644
--- a/src/xen/xen_driver.c
+++ b/src/xen/xen_driver.c
@@ -1539,7 +1539,7 @@ cleanup:
 static int
 xenUnifiedDomainGetMaxVcpus(virDomainPtr dom)
 {
-return xenUnifiedDomainGetVcpusFlags(dom, (VIR_DOMAIN_VCPU_LIVE |
+return xenUnifiedDomainGetVcpusFlags(dom, (VIR_DOMAIN_VCPU_CURRENT |
VIR_DOMAIN_VCPU_MAXIMUM));
 }
 
diff --git a/src/xenapi/xenapi_driver.c b/src/xenapi/xenapi_driver.c
index a547306..688d162 100644
--- a/src/xenapi/xenapi_driver.c
+++ b/src/xenapi/xenapi_driver.c
@@ -1347,7 +1347,7 @@ xenapiDomainGetVcpusFlags(virDomainPtr dom, unsigned int 
flags)
 static int
 xenapiDomainGetMaxVcpus(virDomainPtr dom)
 {
-return xenapiDomainGetVcpusFlags(dom, (VIR_DOMAIN_VCPU_LIVE |
+return xenapiDomainGetVcpusFlags(dom, (VIR_DOMAIN_VCPU_CURRENT |
VIR_DOMAIN_VCPU_MAXIMUM));
 }
 
-- 
1.7.9.5

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


[libvirt] LSN-2013-0018: Unsafe usage of paths under /proc/$PID/root by the LXC driver

2014-02-19 Thread Daniel P. Berrange
Libvirt Security Notice: LSN-2013-0018
==

   Summary: Unsafe usage of paths under /proc/$PID/root by the
LXC driver
   Reported on: 20131217
  Published on: 20131217
  Fixed on: 20140219
   Reported by: Reco recovery...@gmail.com
Patched by: Reco recovery...@gmail.com,
Eric Blake ebl...@redhat.com,
Daniel Berrange berra...@redhat.com
  See also: CVE-2013-6456, debian bug #732394

Description
---

The LXC driver will open paths under /proc/$PID/root for some
operations it performs on running guests. For the virDomainShutdown
and virDomainReboot APIs it will use this to access the /dev/initctl
path in the container. For the virDomainDeviceAttach /
virDomainDeviceDettach APIs it will use this to create device nodes
in the container's /dev filesystem. If any of the path components
under control of the container are symlinks the container can cause
the libvirtd daemon to access the incorrect files.

Impact
--

A container can cause the administrator to shutdown or reboot the
host OS if /dev/initctl in the container is made to be an absolute
symlink back to itself or /run/initctl. A container can cause the
host administrator to mknod in an arbitrary host directory when
invoking the virDomainDeviceAttach API by replacing '/dev' with an
absolute symlink. A container can cause the host administrator to
delete host device when invoking the virDomainDeviceDettach API by
replacing '/dev' with an absolute symlink.

Workaround
--

Do not use the virDomainShutdown or virDomainReboot APIs without
also passing the VIR_DOMAIN_SHUTDOWN_SIGNAL or
VIR_DOMAIN_REBOOT_SIGNAL flags respectively. These will cause the
LXC driver to send a SIGTERM or SIGHUP signal respectively, to the
init process instead of using /dev/initct.. Do not use the
virDomainDeviceAttach or virDomainDeviceDetach APIs at all unless
the guest OS is trusted.

Affected product


Name: libvirt
  Repository: git://libvirt.org/git/libvirt.git
  http://libvirt.org/git/?p=libvirt.git

  Branch: master
   Broken in: v1.0.1
   Broken in: v1.0.2
   Broken in: v1.0.3
   Broken in: v1.0.4
   Broken in: v1.0.5
   Broken in: v1.0.6
   Broken in: v1.0.0
   Broken in: v1.1.1
   Broken in: v1.1.2
   Broken in: v1.1.3
   Broken in: v1.1.4
   Broken in: v1.2.0
   Broken in: v1.2.1
   Broken by: cbb106f807b32f1f6af22d1e92fe0ff9ba6d73b3
   Broken by: de858e3fa7ffcab5f80d07f8a74d94cbaf8716b9
   Broken by: ed77abc58bc5a6837a5021f26e1a335dbfb477bf
   Broken by: a5efb3190913b6903775ca3756f79443d4ea8a5b
   Broken by: 4ad6a013304f6fe29b0866742c902054bfbcf23f
Fixed by: aebbcdd33c8c18891f0bdbbf8924599a28152c9c
Fixed by: 4dd3a7d5bc44980135a1b11810ba9aeab42a4a59
Fixed by: 7fba01c15c1f886b4235825692b4c13e88dd9f7b
Fixed by: 1754c7f0ab1407dcf7c89636a35711dd9b1febe1
Fixed by: 1cadeafcaa422844a27ef622e2a7041d0235bcb3
Fixed by: 5fc590ad9f4071350a8df4d567ba88baacc8334d

  Branch: v1.0.1-maint
   Broken by: cbb106f807b32f1f6af22d1e92fe0ff9ba6d73b3

  Branch: v1.0.2-maint
   Broken by: cbb106f807b32f1f6af22d1e92fe0ff9ba6d73b3
   Broken by: de858e3fa7ffcab5f80d07f8a74d94cbaf8716b9
   Broken by: ed77abc58bc5a6837a5021f26e1a335dbfb477bf
   Broken by: a5efb3190913b6903775ca3756f79443d4ea8a5b
   Broken by: 4ad6a013304f6fe29b0866742c902054bfbcf23f

  Branch: v1.0.3-maint
   Broken by: cbb106f807b32f1f6af22d1e92fe0ff9ba6d73b3
   Broken by: de858e3fa7ffcab5f80d07f8a74d94cbaf8716b9
   Broken by: ed77abc58bc5a6837a5021f26e1a335dbfb477bf
   Broken by: a5efb3190913b6903775ca3756f79443d4ea8a5b
   Broken by: 4ad6a013304f6fe29b0866742c902054bfbcf23f

  Branch: v1.0.4-maint
   Broken by: cbb106f807b32f1f6af22d1e92fe0ff9ba6d73b3
   Broken by: de858e3fa7ffcab5f80d07f8a74d94cbaf8716b9
   Broken by: ed77abc58bc5a6837a5021f26e1a335dbfb477bf
   Broken by: a5efb3190913b6903775ca3756f79443d4ea8a5b
   Broken by: 4ad6a013304f6fe29b0866742c902054bfbcf23f

  Branch: v1.0.5-maint
   Broken in: v1.0.5.1
   Broken in: v1.0.5.2
   Broken in: v1.0.5.3
   Broken in: v1.0.5.4
   Broken in: v1.0.5.5
   Broken in: v1.0.5.6
   Broken in: v1.0.5.7
   Broken in: v1.0.5.8
   Broken in: v1.0.5.9
   Broken by: cbb106f807b32f1f6af22d1e92fe0ff9ba6d73b3
   Broken by: de858e3fa7ffcab5f80d07f8a74d94cbaf8716b9
   Broken by: ed77abc58bc5a6837a5021f26e1a335dbfb477bf
   Broken by: a5efb3190913b6903775ca3756f79443d4ea8a5b
   Broken by: 4ad6a013304f6fe29b0866742c902054bfbcf23f
Fixed by: f84056cf6166332b1f15f3e6584a88f5d42273fe
Fixed by: 0e9fee68b3bff24e4d3ab48de8129946202f3bc0
Fixed by: 9849cf6d89e5665667a0df449ddc3fd5582da242
Fixed by: 21821ed4d1faf5bf563a26e8ac7cd2eb0450d322
Fixed by: e57058cfe827b1971ca0dee224ff273c9cad7756
Fixed by: e1e7e05376faf1ed471cb5c1d1e0415458f2af7d

  Branch: v1.0.6-maint
   Broken by: cbb106f807b32f1f6af22d1e92fe0ff9ba6d73b3
   Broken

Re: [libvirt] Is it possible to set a timeout to the Connection class?

2014-02-19 Thread Claudio Bley
[please, don't top-post on technical lists and keep the conversation on list]

At Wed, 19 Feb 2014 11:59:29 +0100,
Pasquale Dir wrote:
 
 Thank you I didn't think about the Future, I'll try it and if it doesn't
 work I'll just try to edit the tcp connection timeout file.
 Can you answer me to the other question too, please?
 
  what new apis are you talking about? I am doing migration by executing
 the Domain.migrate function in a separate thread and obtaining progress by
 DomainJobInfo class. Did you introduce some new api who automatically does
 this? I can't find it here http://libvirt.org/sources/java/javadoc/ 

Eric was talking about the block-pull API functions, like
virDomainBlockPull[1] which was added to libvirt in version 0.9.4.

These functions have not been wrapped in libvirt-java, yet. (Patches welcome!)

As Eric pointed out, there is curently no equivalent async _migration_
function in libvirt.

Claudio

[1]: http://libvirt.org/html/libvirt-libvirt.html#virDomainBlockPull

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


Re: [libvirt] Reporting log/error messages through capabilities

2014-02-19 Thread Michal Privoznik

On 19.02.2014 00:11, Richard W.M. Jones wrote:

When qemu is completely broken, libvirtd starts up OK but exists in a
kind of broken state where no guests can possibly be run.  I hit this
problem ... again ... today:

https://bugzilla.redhat.com/show_bug.cgi?id=1066630#c0

There is a libvirt bug here, which is that it's very hard to diagnose
what is going on when qemu fails to work at all.  The logging system
in libvirt(d) is trememdously powerful, but ultimately confusing to
use, and requires users to edit config files which makes it a
non-starter for programs using libvirt through the API [1].

From a libguestfs point of view, it's impossible for us to report back
to the user that there is a problem two layers below in qemu.

So my idea is that libvirt capabilities output should have an info
section containing log messages/errors.

   capabilities
 ...
 info
 Could not run qemu-system-blah:
 symbol lookup error: /usr/bin/qemu-system-s390x: undefined symbol: 
glfs_discard_async
 /info
   /capabilities


This makes sense, although we should make it more versatile to 
distinguish different qemu targets. For example -s390x can be missing a 
symbol, while -x86_64 can be missing a shared library, or have denied 
access somewhere, whatever. If that's the case, we should be able to 
report errors independently.




Libguestfs queries for libvirt capabilities anyway.  If we don't get a
satisfactory set of guest/ elements, then we could list out the
info/ section.  Easy for us.

The problem is the info/ element hardly fits into capabilities.  If
we didn't put it there, could it go some other place?  Or a new API?

Are there other unanticipated problems here?  I think one is that
libvirt doesn't appear to collect detailed log information by default,
(unless the user edits log_level).  That's assuming I understand the
code correctly.  Personally I think libvirt should always collect
debug information, because you never know when it could be useful, but
for the above, collecting errors  warnings unconditionally is
sufficient.

Rich.




[1] By the way, this is a general complaint about libvirt.  Please
DON'T add any more stuff to the configuration file.  Everything should
be configurable through the API, or not at all.  There are two other
settings I can think of that libguestfs would like to adjust but
cannot because they are only available in a configuration file.



This all will be solved by administration module, once we implement it. 
I don't know about anybody working on it though.


Michal

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


Re: [libvirt] Reporting log/error messages through capabilities

2014-02-19 Thread Kashyap Chamarthy
On Tue, Feb 18, 2014 at 11:11:10PM +, Richard W.M. Jones wrote:

[. . .]

 Are there other unanticipated problems here?  I think one is that
 libvirt doesn't appear to collect detailed log information by default,
 (unless the user edits log_level).  That's assuming I understand the
 code correctly.  Personally I think libvirt should always collect
 debug information, 

Interestingly, recently there was a similar debate[1] in OpenStack
upstream and RDO lands. A CI fix landed to enable libvirt debug logs on
OpenStack Jenkins machines, after a discussion on the list, it got
reverted, and settled on using these filters, per Dan's suggestion:

  log_filters=1:libvirt 1:qemu 1:conf 1:security 3:event 3:json 3:file 
1:util
  log_outputs=1:file:/var/log/libvirt/libvirtd.log


 [1] https://bugzilla.redhat.com/show_bug.cgi?id=1061753


-- 
/kashyap

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


Re: [libvirt] Reporting log/error messages through capabilities

2014-02-19 Thread Daniel P. Berrange
On Tue, Feb 18, 2014 at 11:11:10PM +, Richard W.M. Jones wrote:
 When qemu is completely broken, libvirtd starts up OK but exists in a
 kind of broken state where no guests can possibly be run.  I hit this
 problem ... again ... today:
 
 https://bugzilla.redhat.com/show_bug.cgi?id=1066630#c0
 
 There is a libvirt bug here, which is that it's very hard to diagnose
 what is going on when qemu fails to work at all.  The logging system
 in libvirt(d) is trememdously powerful, but ultimately confusing to
 use, and requires users to edit config files which makes it a
 non-starter for programs using libvirt through the API [1].
 
 From a libguestfs point of view, it's impossible for us to report back
 to the user that there is a problem two layers below in qemu.
 
 So my idea is that libvirt capabilities output should have an info
 section containing log messages/errors.
 
   capabilities
 ...
 info
 Could not run qemu-system-blah:
 symbol lookup error: /usr/bin/qemu-system-s390x: undefined symbol: 
 glfs_discard_async
 /info
   /capabilities
 
 Libguestfs queries for libvirt capabilities anyway.  If we don't get a
 satisfactory set of guest/ elements, then we could list out the
 info/ section.  Easy for us.
 
 The problem is the info/ element hardly fits into capabilities.  If
 we didn't put it there, could it go some other place?  Or a new API?

Yeah, I don't really like the idea of doing this in the capabilities
XML. 

I'm not even really convinced this should be in the API at all in fact.

What we could usefully do in libvirt though is to log a structured
message to the systemd journal when we find a QEMU binary that we
fail to extract capabilities from. Apps that care about it could
directly query the journal for the precise well-known log UUID.

And of course it goes without saying we should never have got into
this scenario in the first place. We need better testing of QEMU
binaries to make sure such brokenness can get detected at build
time.

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] Reporting log/error messages through capabilities

2014-02-19 Thread Daniel P. Berrange
On Wed, Feb 19, 2014 at 02:01:43PM +0100, Michal Privoznik wrote:
 On 19.02.2014 00:11, Richard W.M. Jones wrote:
 [1] By the way, this is a general complaint about libvirt.  Please
 DON'T add any more stuff to the configuration file.  Everything should
 be configurable through the API, or not at all.  There are two other
 settings I can think of that libguestfs would like to adjust but
 cannot because they are only available in a configuration file.
 
 
 This all will be solved by administration module, once we implement
 it. I don't know about anybody working on it though.

Yeah, we really need to get our act together on that. I might even be
able to squeeze out some free time for this in the next few weeks. At
least to get a proof of concept working with 1 or 2 example APIs.

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] Reporting log/error messages through capabilities

2014-02-19 Thread Daniel P. Berrange
On Tue, Feb 18, 2014 at 11:11:10PM +, Richard W.M. Jones wrote:
 There is a libvirt bug here, which is that it's very hard to diagnose
 what is going on when qemu fails to work at all.  The logging system
 in libvirt(d) is trememdously powerful, but ultimately confusing to
 use, and requires users to edit config files which makes it a
 non-starter for programs using libvirt through the API [1].

The problem with allowing apps to change the logging config is that
it is global state, not per client. So multiple apps would conflict
in what they could do with changes here. While we could probably
make it possible for apps to register their own callback to receive
log messages, the setting of actual log levels would still be global.

 [1] By the way, this is a general complaint about libvirt.  Please
 DON'T add any more stuff to the configuration file.  Everything should
 be configurable through the API, or not at all.  There are two other
 settings I can think of that libguestfs would like to adjust but
 cannot because they are only available in a configuration file.

What are the other settings you're thinking of here ? The stuff in the
global config file is primarily intended for things that are globally
affecting libvirt behaviour and so would not be appropriate for individual
apps to change independantly. Or for things where we want to make global
defaults tweakable, but still allow app overrides - eg  VNC listen
address is global, but still tweakable in XML per VM.

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] networkRunHook: Run hook only if possible

2014-02-19 Thread Michal Privoznik
Currently, networkRunHook() is called in networkAllocateActualDevice and
friends. These functions, however, doesn't necessarily work on networks,
For example, if domain's interface is defined in this fashion:

interface type='bridge'
  mac address='52:54:00:0b:3b:16'/
  source bridge='virbr1'/
  model type='rtl8139'/
  address type='pci' domain='0x' bus='0x00' slot='0x09' 
function='0x0'/
/interface

The networkAllocateActualDevice jumps directly onto 'validate' label as
the interface is not type of 'network'. Hence, @network is left
initialized to NULL and networkRunHook(network, ...) is called. One of
the things that the hook function does is dereference @network. Soupir.

Signed-off-by: Michal Privoznik mpriv...@redhat.com
---
 src/network/bridge_driver.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index ee264b9..a6c719d 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -150,6 +150,12 @@ networkRunHook(virNetworkObjPtr network,
 int ret = -1;
 
 if (virHookPresent(VIR_HOOK_DRIVER_NETWORK)) {
+if (!network) {
+VIR_DEBUG(Not running hook as @network is NULL);
+ret = 0;
+goto cleanup;
+}
+
 virBufferAddLit(buf, hookData\n);
 virBufferAdjustIndent(buf, 2);
 if (virNetworkDefFormatBuf(buf, network-def, 0)  0)
-- 
1.9.0

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


Re: [libvirt] Is it possible to set a timeout to the Connection class?

2014-02-19 Thread Eric Blake
On 02/19/2014 04:06 AM, Daniel P. Berrange wrote:

 FWIW, for the initial 'virConnectOpen' API I think it probably would
 be worthwhile us supporting a standardized timeout URI parameter.
 That way if the remote service doesn't respond at all for some
 reason users can have fine control.  That's a sufficiently targetted
 use case that it'd be easy to do, compared to timeouts for arbitrary
 APIs.

Indeed - having a timeout on the initial connection attempt is much more
useful than worrying about individual APIs when you have a responsive
connection, since it is the indeterminate time of establishing a remote
connection that may be the problem here.  But does that mean yet another
C API?  We already have virConnectOpen{,ReadOnly,Auth}.  Or are you
envisioning this just in the language bindings (Java, python - but not C)?

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



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

Re: [libvirt] [PATCH v10] bhyve: add a basic driver

2014-02-19 Thread Daniel P. Berrange
On Tue, Feb 18, 2014 at 02:08:10PM +0400, Roman Bogorodskiy wrote:
 At this point it has a limited functionality and is highly
 experimental. Supported domain operations are:
 
   * define
   * start
   * destroy
   * dumpxml
   * dominfo
 
 It's only possible to have only one disk device and only one
 network, which should be of type bridge.
 ---
  configure.ac|   7 +
  daemon/libvirtd.c   |   9 +
  include/libvirt/virterror.h |   1 +
  m4/virt-driver-bhyve.m4 |  57 +
  po/POTFILES.in  |   3 +
  src/Makefile.am |  32 +++
  src/bhyve/bhyve_command.c   | 331 
  src/bhyve/bhyve_command.h   |  41 +++
  src/bhyve/bhyve_driver.c| 612 
 
  src/bhyve/bhyve_driver.h|  28 ++
  src/bhyve/bhyve_process.c   | 224 
  src/bhyve/bhyve_process.h   |  36 +++
  src/bhyve/bhyve_utils.h |  48 
  src/conf/domain_conf.c  |   3 +-
  src/conf/domain_conf.h  |   1 +
  src/driver.h|   1 +
  src/libvirt.c   |   3 +
  src/util/virerror.c |   1 +
  18 files changed, 1437 insertions(+), 1 deletion(-)
  create mode 100644 m4/virt-driver-bhyve.m4
  create mode 100644 src/bhyve/bhyve_command.c
  create mode 100644 src/bhyve/bhyve_command.h
  create mode 100644 src/bhyve/bhyve_driver.c
  create mode 100644 src/bhyve/bhyve_driver.h
  create mode 100644 src/bhyve/bhyve_process.c
  create mode 100644 src/bhyve/bhyve_process.h
  create mode 100644 src/bhyve/bhyve_utils.h

ACK, and pushed to GIT.

Great job getting this new driver up  running, and welcome to
the libvirt committers team.

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] Is it possible to set a timeout to the Connection class?

2014-02-19 Thread Daniel P. Berrange
On Wed, Feb 19, 2014 at 07:54:54AM -0700, Eric Blake wrote:
 On 02/19/2014 04:06 AM, Daniel P. Berrange wrote:
 
  FWIW, for the initial 'virConnectOpen' API I think it probably would
  be worthwhile us supporting a standardized timeout URI parameter.
  That way if the remote service doesn't respond at all for some
  reason users can have fine control.  That's a sufficiently targetted
  use case that it'd be easy to do, compared to timeouts for arbitrary
  APIs.
 
 Indeed - having a timeout on the initial connection attempt is much more
 useful than worrying about individual APIs when you have a responsive
 connection, since it is the indeterminate time of establishing a remote
 connection that may be the problem here.  But does that mean yet another
 C API?  We already have virConnectOpen{,ReadOnly,Auth}.  Or are you
 envisioning this just in the language bindings (Java, python - but not C)?

I was actually thinking of (ab)using the URI for this eg

qemu+tcp://somehost/system?timeout=60

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 V2 00/13] libxl: add job support

2014-02-19 Thread Jim Fehlig
Jim Fehlig wrote:
 This patch series adds job support to the libxl driver, using a simplified
 version of the technique used in the qemu driver.  One benefit of this
 series is no longer blocking get operations during long running modify
 operations.  E.g. with these patches 'vish dominfo dom' will work while
 'virsh save dom ...' is in progress.

 The first two patches are new to the series, following review of V1.
 Patch1 changes the cleanup logic to unconditionally set dom id to -1 on
 domain shutdown.  Patch2 removes libxlVmReap, giving callers more control
 over domain destruction/cleanup.

 The remaining patches are updates to V1 based on Daniel's and Michal's
 comments.
   

Ping.

This series would be a nice little improvement in the libxl driver for
1.2.2 :-).

Regards,
Jim

 Jim Fehlig (13):
   libxl: always set vm id to -1 on shutdown
   libxl: remove libxlVmReap function
   libxl: Add job support to libxl driver
   libxl: use job functions in libxlVmStart
   libxl: use job functions in libxlDomainSetMemoryFlags
   libxl: use job functions in libxlDomain{Suspend,Resume}
   libxl: use job functions when cleaning up a domain
   libxl: use job functions in domain save operations
   libxl: use job functions in libxlDomainCoreDump
   libxl: use job functions in vcpu set and pin functions
   libxl: use job functions in device attach and detach functions
   libxl: use job functions in libxlDomainSetAutostart
   libxl: use job functions in libxlDomainSetSchedulerParametersFlags

  src/libxl/libxl_domain.c | 128 +++
  src/libxl/libxl_domain.h |  38 +
  src/libxl/libxl_driver.c | 405 
 +++
  3 files changed, 432 insertions(+), 139 deletions(-)

   

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


Re: [libvirt] [PATCH] networkRunHook: Run hook only if possible

2014-02-19 Thread John Ferlan


On 02/19/2014 09:54 AM, Michal Privoznik wrote:
 Currently, networkRunHook() is called in networkAllocateActualDevice and
 friends. These functions, however, doesn't necessarily work on networks,

s/doesn't/don't/

 For example, if domain's interface is defined in this fashion:
 
 interface type='bridge'
   mac address='52:54:00:0b:3b:16'/
   source bridge='virbr1'/
   model type='rtl8139'/
   address type='pci' domain='0x' bus='0x00' slot='0x09' 
 function='0x0'/
 /interface
 
 The networkAllocateActualDevice jumps directly onto 'validate' label as
 the interface is not type of 'network'. Hence, @network is left
 initialized to NULL and networkRunHook(network, ...) is called. One of
 the things that the hook function does is dereference @network. Soupir.
 
 Signed-off-by: Michal Privoznik mpriv...@redhat.com
 ---
  src/network/bridge_driver.c | 6 ++
  1 file changed, 6 insertions(+)
 
I'll give an ACK to the change...

Although I will also note that while the patch does resolve the current
issue and perhaps protect against future errant callers... Currently the
only caller that could get into the hook function with network == NULL
is networkAllocateActualDevice()

Other places that would end up using the hook will bail when iface-type
!= VIR_DOMAIN_NET_TYPE_NETWORK or when/by checking
virNetworkObjIsActive(network).

Your other option would seem to be checking (iface-type ==
VIR_DOMAIN_NET_TYPE_NETWORK) before calling the hook in the validate:
section.

John


 diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
 index ee264b9..a6c719d 100644
 --- a/src/network/bridge_driver.c
 +++ b/src/network/bridge_driver.c
 @@ -150,6 +150,12 @@ networkRunHook(virNetworkObjPtr network,
  int ret = -1;
  
  if (virHookPresent(VIR_HOOK_DRIVER_NETWORK)) {
 +if (!network) {
 +VIR_DEBUG(Not running hook as @network is NULL);
 +ret = 0;
 +goto cleanup;
 +}
 +
  virBufferAddLit(buf, hookData\n);
  virBufferAdjustIndent(buf, 2);
  if (virNetworkDefFormatBuf(buf, network-def, 0)  0)
 

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


Re: [libvirt] [PATCH v2] qemu: Use virtio network device for aarch64/virt

2014-02-19 Thread Cole Robinson
On 02/14/2014 09:09 AM, Oleg Strikov wrote:
 This patch changes network device type used by default from rtl8139
 to virtio when architecture type is aarch64 and machine type is virt.
 Qemu doesn't support any other machine types for aarch64 right now and
 we can't make any other aarch64-specific tuning in this function yet.
 
 Signed-off-by: Oleg Strikov oleg.stri...@canonical.com
 ---
 
 Changes since v1:
  * qemuxml2argvtest suite has been extended to validate correct setup
for aarch64 guests when no NIC model is provided in the XML
 
 

ACK, pushed now.

- Cole

  src/qemu/qemu_domain.c |3 ++-
  .../qemuxml2argv-aarch64-virt-default-nic.args |6 ++
  .../qemuxml2argv-aarch64-virt-default-nic.xml  |   22 
 
  tests/qemuxml2argvtest.c   |3 +++
  4 files changed, 33 insertions(+), 1 deletion(-)
  create mode 100644 
 tests/qemuxml2argvdata/qemuxml2argv-aarch64-virt-default-nic.args
  create mode 100644 
 tests/qemuxml2argvdata/qemuxml2argv-aarch64-virt-default-nic.xml
 
 diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
 index a665061..9a040ee 100644
 --- a/src/qemu/qemu_domain.c
 +++ b/src/qemu/qemu_domain.c
 @@ -797,7 +797,8 @@ qemuDomainDefaultNetModel(const virDomainDef *def)
  def-os.arch == VIR_ARCH_S390X)
  return virtio;
  
 -if (def-os.arch == VIR_ARCH_ARMV7L) {
 +if (def-os.arch == VIR_ARCH_ARMV7L ||
 +def-os.arch == VIR_ARCH_AARCH64) {
  if (STREQ(def-os.machine, versatilepb))
  return smc91c111;
  
 diff --git 
 a/tests/qemuxml2argvdata/qemuxml2argv-aarch64-virt-default-nic.args 
 b/tests/qemuxml2argvdata/qemuxml2argv-aarch64-virt-default-nic.args
 new file mode 100644
 index 000..d4d403b
 --- /dev/null
 +++ b/tests/qemuxml2argvdata/qemuxml2argv-aarch64-virt-default-nic.args
 @@ -0,0 +1,6 @@
 +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test 
 QEMU_AUDIO_DRV=none \
 +/usr/bin/qemu-system-aarch64 -S -M virt -m 1024 -smp 1 -nographic \
 +-nodefconfig -nodefaults -monitor unix:/tmp/test-monitor,server,nowait \
 +-boot c -kernel /aarch64.kernel -initrd /aarch64.initrd -append 
 console=ttyAMA0 \
 +-usb -device virtio-net-device,vlan=0,id=net0,mac=52:54:00:09:a4:37 \
 +-net user,vlan=0,name=hostnet0
 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-aarch64-virt-default-nic.xml 
 b/tests/qemuxml2argvdata/qemuxml2argv-aarch64-virt-default-nic.xml
 new file mode 100644
 index 000..868de94
 --- /dev/null
 +++ b/tests/qemuxml2argvdata/qemuxml2argv-aarch64-virt-default-nic.xml
 @@ -0,0 +1,22 @@
 +domain type=qemu
 +  nameaarch64-virt-default-nic/name
 +  uuid6ba410c5-1e5c-4d57-bee7-2228e7ffa32f/uuid
 +  memory1048576/memory
 +  currentMemory1048576/currentMemory
 +  vcpu1/vcpu
 +  features
 +acpi/
 +  /features
 +  os
 +type arch=aarch64 machine=virthvm/type
 +kernel/aarch64.kernel/kernel
 +initrd/aarch64.initrd/initrd
 +cmdlineconsole=ttyAMA0/cmdline
 +  /os
 +  devices
 +emulator/usr/bin/qemu-system-aarch64/emulator
 +interface type='user'
 +  mac address='52:54:00:09:a4:37'/
 +/interface
 +  /devices
 +/domain
 diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
 index 7a5b50d..43ac1d1 100644
 --- a/tests/qemuxml2argvtest.c
 +++ b/tests/qemuxml2argvtest.c
 @@ -1335,6 +1335,9 @@ mymain(void)
  QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG, QEMU_CAPS_DTB,
  QEMU_CAPS_DRIVE, QEMU_CAPS_DEVICE_VIRTIO_MMIO,
  QEMU_CAPS_DEVICE_VIRTIO_RNG, QEMU_CAPS_OBJECT_RNG_RANDOM);
 +DO_TEST(aarch64-virt-default-nic,
 +QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG,
 +QEMU_CAPS_DEVICE_VIRTIO_MMIO);
  
  DO_TEST(kvm-pit-device, QEMU_CAPS_KVM_PIT_TICK_POLICY);
  DO_TEST(kvm-pit-delay, QEMU_CAPS_NO_KVM_PIT);
 

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


Re: [libvirt] [libvirt-users] event-test.py cannot detects domain shutdown

2014-02-19 Thread Cole Robinson
On 02/18/2014 10:45 PM, Jim Fehlig wrote:
 Cole Robinson wrote:
 On 02/18/2014 05:12 AM, Kim Larry wrote:
   
 The thing I found today is that if libvirt uses xend driver, shutdown events
 are delivered, but if libvirt uses libxl drvier, doesn't show up anything. 
 It
 seems there are bugs on shutdown event, so I did dig into the libvirt source
 briefly, but I couldn't find where libxl driver calls event callback.

 Any help will be greatly appreciated. Especially someone who is familiar 
 with
 xen code.

 

 Jim, does this sound familiar?
   
 
 Looks like the event handler only queues a crashed event when receiving
 the shutdown event from libxl.  The attached patch should fix it, but I
 think the logic could be further improved.  I'll look at doing that
 tomorrow.
 
 Regards,
 Jim
 
 From c05615b29f8870d20b4457a2e8abe5e4195275c2 Mon Sep 17 00:00:00 2001
 From: Jim Fehlig jfeh...@suse.com
 Date: Tue, 18 Feb 2014 20:34:47 -0700
 Subject: [PATCH] libxl: queue shutdown event on domain shutdown
 
 Emmit libvirt shutdown event when receiving LIBXL_SHUTDOWN_REASON_POWEROFF
 event from libxl.
 
 Signed-off-by: Jim Fehlig jfeh...@suse.com
 ---
  src/libxl/libxl_driver.c | 3 +++
  1 file changed, 3 insertions(+)
 
 diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
 index 8e4242a..8d5e101 100644
 --- a/src/libxl/libxl_driver.c
 +++ b/src/libxl/libxl_driver.c
 @@ -383,6 +383,9 @@ libxlDomainShutdownThread(void *opaque)
 VIR_DOMAIN_EVENT_STOPPED_CRASHED);
  reason = VIR_DOMAIN_SHUTOFF_CRASHED;
  } else {
 +dom_event = virDomainEventLifecycleNewFromObj(vm,
 +   VIR_DOMAIN_EVENT_STOPPED,
 +   
 VIR_DOMAIN_EVENT_STOPPED_SHUTDOWN);
  reason = VIR_DOMAIN_SHUTOFF_SHUTDOWN;
  }
  libxlVmReap(driver, vm, reason);

ACK, looks fine to me, but I'm guessing the REBOOT handling right below also
needs an event dispatched. Probably best to see what the qemu or test driver
do for reboot and copy it.

- Cole

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


Re: [libvirt] [PATCH V2 00/13] libxl: add job support

2014-02-19 Thread Daniel P. Berrange
On Wed, Feb 19, 2014 at 08:35:21AM -0700, Jim Fehlig wrote:
 Jim Fehlig wrote:
  This patch series adds job support to the libxl driver, using a simplified
  version of the technique used in the qemu driver.  One benefit of this
  series is no longer blocking get operations during long running modify
  operations.  E.g. with these patches 'vish dominfo dom' will work while
  'virsh save dom ...' is in progress.
 
  The first two patches are new to the series, following review of V1.
  Patch1 changes the cleanup logic to unconditionally set dom id to -1 on
  domain shutdown.  Patch2 removes libxlVmReap, giving callers more control
  over domain destruction/cleanup.
 
  The remaining patches are updates to V1 based on Daniel's and Michal's
  comments.

 
 Ping.
 
 This series would be a nice little improvement in the libxl driver for
 1.2.2 :-).

Yep, will re-review this shortly. Shouldn't be any problem geting
it into 1.2.2 since previous posting was almost there.

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] [PATCHv6 1/5] virstring.h/c: Util method for finding regexp patterns in some strings

2014-02-19 Thread Daniel P. Berrange
On Thu, Jan 23, 2014 at 10:28:29AM +0100, Manuel VIVES wrote:
 ---
  po/POTFILES.in   |1 +
  src/libvirt_private.syms |1 +
  src/util/virstring.c |   97 
 ++
  src/util/virstring.h |3 ++
  4 files changed, 102 insertions(+)

Can you add something to tests/virstringtest.c to validate your
new method with various interesting input strings.

 diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
 index d1a58f9..68ca39d 100644
 --- a/src/libvirt_private.syms
 +++ b/src/libvirt_private.syms
 @@ -1763,6 +1763,7 @@ virStorageFileResize;
  # util/virstring.h
  virArgvToString;
  virAsprintfInternal;
 +virSearchRegex;

Lets call this new method 'virStringSearchRegex' so we have a normal
name prefix

 diff --git a/src/util/virstring.c b/src/util/virstring.c
 index 8d0ca70..3c93450 100644
 --- a/src/util/virstring.c
 +++ b/src/util/virstring.c
 @@ -23,6 +23,7 @@
  
  #include stdlib.h
  #include stdio.h
 +#include regex.h
  
  #include c-ctype.h
  #include virstring.h
 @@ -645,3 +646,99 @@ int virStringSortRevCompare(const void *a, const void *b)
  
  return strcmp(*sb, *sa);
  }
 +
 +/**
 + * virSearchRegex:
 + * Allows you to get the nth occurrence of a substring in sourceString which 
 matches
 + * a POSIX Extended regular expression pattern.
 + * If there is no substring, result is not modified.
 + * return -1 on error, 0 if not found and 1 if found.
 + *
 + * @sourceString: String to parse
 + * @occurrence: return occurrence 'n' (starting from 0) of a sub-string that
 + *  matches the pattern.
 + * @regexp: POSIX Extended regular expression pattern used for matching
 + * @result: nth occurrence substring matching the @regexp pattern
 + * @code
 +char *source = 6853a496-1c10-472e-867a-8244937bd6f0
 +773ab075-4cd7-4fc2-8b6e-21c84e9cb391
 +bbb3c75c-d60f-43b0-b802-fd56b84a4222
 +60c04aa1-0375-4654-8d9f-e149d9885273
 +4548d465-9891-4c34-a184-3b1c34a26aa8;
 +char *ret1=NULL;
 +char *ret2=NULL;
 +char *ret3=NULL;
 +virSearchRegex(source,
 +   4,
 +   
 ([a-f0-9]{8}-[a-f0-9]{4}-[a-f0-9]{4}-[a-f0-9]{4}-[a-f0-9]{12}),
 +   ret1);
 +//ret1 = 4548d465-9891-4c34-a184-3b1c34a26aa8
 +virSearchRegex(source,
 +   0,
 +   
 ([a-f0-9]{8}-[a-f0-9]{4}-[a-f0-9]{4}-[a-f0-9]{4}-[a-f0-9]{12}),
 +   ret2);
 +//ret2 = 6853a496-1c10-472e-867a-8244937bd6f0
 +virSearchRegex(source,
 +   1,
 +   
 ([a-f0-9]{8}-[a-f0-9]{4}-[a-f0-9]{4}-[a-f0-9]{4}-[a-f0-9]{12}),
 +   ret3);
 +//ret3 = 773ab075-4cd7-4fc2-8b6e-21c84e9cb391
 + * @endcode

 +int
 +virSearchRegex(const char *sourceString,
 +   unsigned int occurrence,
 +   const char *regexp,
 +   char **result)

This kind of usage leads to pretty inefficient code. Why not just
change it to return 'char ***matches' and thus give the caller all
possible matches in one call. And 'unsigned int occurrence' could
be  'size_t maxMatches' to limit it.




 +{
 +regex_t pregUuidBracket;

This variable name is a little odd - lets just call it 're'.

 +size_t i = 0;
 +size_t nmatch = 0;
 +regmatch_t *pmatch = NULL;
 +int ret = -1;
 +int regError = -1;
 +
 +regError = regcomp(pregUuidBracket, regexp, REG_EXTENDED);
 +if (regError != 0) {
 +virReportError(VIR_ERR_INTERNAL_ERROR,
 +   _(Error while compiling regular expression: %d),
 +   regError);
 +goto cleanup;
 +}
 +nmatch = pregUuidBracket.re_nsub;
 +if (VIR_ALLOC_N(pmatch, nmatch)  0)
 +goto cleanup;
 +
 +while (i  (occurrence+1)) {
 +if (regexec(pregUuidBracket, sourceString, nmatch, pmatch, 0) == 0) 
 {
 +regoff_t start = pmatch[0].rm_so;
 +regoff_t end = pmatch[0].rm_eo;
 +if (i == occurrence ||
 +(occurrence  i  regexec(pregUuidBracket, 
 sourceString[end],
 + nmatch, pmatch, 0) != 0)) {
 +/* We copy only if i == position (so that it is the uuid 
 we're looking for),
 + * or position  i AND there is no matches left in the rest 
 of the string
 + * (this is the case where we give a biggest @occurence than 
 the
 + * number of matches and we want to return the last one)
 + */
 +if (VIR_STRNDUP(*result, sourceString + start, end - start) 
  0)
 +goto cleanup;
 +
 +ret = 1;
 +goto cleanup;
 +}
 +sourceString = sourceString[end];
 +} else {
 +break;
 +ret = 0;
 +goto 

Re: [libvirt] Reporting log/error messages through capabilities

2014-02-19 Thread Richard W.M. Jones
On Wed, Feb 19, 2014 at 01:41:21PM +, Daniel P. Berrange wrote:
 And of course it goes without saying we should never have got into
 this scenario in the first place. We need better testing of QEMU
 binaries to make sure such brokenness can get detected at build
 time.

To be fair in this case it was because I was cherry picking packages
(qemu) from Rawhide, without updating the whole system.  Although if
gluster had symbol versioning, I guess dependencies would have pulled
in the updated glusterfs-libs package too ...

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
libguestfs lets you edit virtual machines.  Supports shell scripting,
bindings from many languages.  http://libguestfs.org

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


Re: [libvirt] Reporting log/error messages through capabilities

2014-02-19 Thread Richard W.M. Jones
On Wed, Feb 19, 2014 at 01:56:24PM +, Daniel P. Berrange wrote:
 On Wed, Feb 19, 2014 at 02:01:43PM +0100, Michal Privoznik wrote:
  On 19.02.2014 00:11, Richard W.M. Jones wrote:
  [1] By the way, this is a general complaint about libvirt.  Please
  DON'T add any more stuff to the configuration file.  Everything should
  be configurable through the API, or not at all.  There are two other
  settings I can think of that libguestfs would like to adjust but
  cannot because they are only available in a configuration file.
  
  
  This all will be solved by administration module, once we implement
  it. I don't know about anybody working on it though.
 
 Yeah, we really need to get our act together on that. I might even be
 able to squeeze out some free time for this in the next few weeks. At
 least to get a proof of concept working with 1 or 2 example APIs.

Is there some background reading on this feature?

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Fedora Windows cross-compiler. Compile Windows programs, test, and
build Windows installers. Over 100 libraries supported.
http://fedoraproject.org/wiki/MinGW

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


Re: [libvirt] [PATCHv6 2/5] virstring.h/c: Util method for making some find and replace in strings

2014-02-19 Thread Daniel P. Berrange
On Thu, Jan 23, 2014 at 10:28:30AM +0100, Manuel VIVES wrote:
 ---
  src/libvirt_private.syms |1 +
  src/util/virstring.c |   66 
 +-
  src/util/virstring.h |1 +
  3 files changed, 67 insertions(+), 1 deletion(-)

Can you also add a test case to src/virstringtest.c for this one
too. In particular test what happens when matches are exactly at
the start and end of the string  so we validate proper boundary
handling.

 +
 +/*
 + virStrReplace(haystack, oldneedle, newneedle) --
 +  Search haystack and replace all occurences of oldneedle with newneedle.
 +  Return a string with all the replacements in case of success, NULL in case
 +  of failure
 +*/
 +char *
 +virStrReplace(char *haystack,
 +  const char *oldneedle, const char *newneedle)

Can we call this  virStringReplace instead.

 +{
 +char *ret = NULL;
 +size_t i = strlen(haystack);
 +size_t count = 0;
 +size_t newneedle_len = strlen(newneedle);
 +size_t oldneedle_len = strlen(oldneedle);
 +int diff_len = newneedle_len - oldneedle_len;
 +size_t totalLength = 0;
 +size_t numberOfElementsToAllocate = 0;
 +if (!haystack || !oldneedle || !newneedle) {
 +virReportError(VIR_ERR_INTERNAL_ERROR, %s,
 +   _(null value for haystack, oldneedle or newneedle));
 +goto cleanup;
 +}
 +if (oldneedle_len == 0) {
 +virReportError(VIR_ERR_INTERNAL_ERROR, %s,
 +   _(oldneedle cannot be empty));
 +goto cleanup;
 +}
 +if (diff_len == 0  memcmp(oldneedle, newneedle, newneedle_len) == 0) {
 +ignore_value(VIR_STRDUP(ret, haystack));
 +goto cleanup;
 +}
 +
 +for (i = 0; haystack[i] != '\0'; i++) {
 +if (memcmp(haystack[i], oldneedle, oldneedle_len) == 0) {

This looks like a out of bounds read. eg consider

   haystack = foo
   oldneedle = baar

You're going to be making memcmp read beyond the end of the haystack
array I believe.

 +count ++;
 +i += oldneedle_len - 1;
 +}
 +}
 +numberOfElementsToAllocate = (i + count * (newneedle_len - 
 oldneedle_len));
 +
 +if (VIR_ALLOC_N(ret, numberOfElementsToAllocate)  0)
 +goto cleanup;

Rather than trying to pre-caculate buffer lengths, I thin it would
probably be simpler if you just used the virBuffer APIs to construct
the new string, since that does grow-on-demand.
 +
 +totalLength = sizeof(char) * numberOfElementsToAllocate;
 +i = 0;
 +while (*haystack) {
 +if (memcmp(haystack, oldneedle, oldneedle_len) == 0) {
 +if (virStrcpy(ret[i], newneedle, totalLength) == NULL) {
 +VIR_FREE(ret);
 +goto cleanup;
 +}
 +totalLength -= newneedle_len;
 +i += newneedle_len;
 +haystack += oldneedle_len;
 +} else {
 +ret[i++] = *haystack++;
 +totalLength --;
 +}
 +}
 +ret[i] = '\0';
 +cleanup:
 +return ret;
 +}


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] Reporting log/error messages through capabilities

2014-02-19 Thread Daniel P. Berrange
On Wed, Feb 19, 2014 at 04:50:42PM +, Richard W.M. Jones wrote:
 On Wed, Feb 19, 2014 at 01:41:21PM +, Daniel P. Berrange wrote:
  And of course it goes without saying we should never have got into
  this scenario in the first place. We need better testing of QEMU
  binaries to make sure such brokenness can get detected at build
  time.
 
 To be fair in this case it was because I was cherry picking packages
 (qemu) from Rawhide, without updating the whole system.  Although if
 gluster had symbol versioning, I guess dependencies would have pulled
 in the updated glusterfs-libs package too ...

Or in the absence of symbol versioning,  QEMU's RPM spec must be clear
about using a versioned dependancy on gluster, instead of relying just
on the automatic ELF deps.

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] Reporting log/error messages through capabilities

2014-02-19 Thread Daniel P. Berrange
On Wed, Feb 19, 2014 at 04:51:17PM +, Richard W.M. Jones wrote:
 On Wed, Feb 19, 2014 at 01:56:24PM +, Daniel P. Berrange wrote:
  On Wed, Feb 19, 2014 at 02:01:43PM +0100, Michal Privoznik wrote:
   On 19.02.2014 00:11, Richard W.M. Jones wrote:
   [1] By the way, this is a general complaint about libvirt.  Please
   DON'T add any more stuff to the configuration file.  Everything should
   be configurable through the API, or not at all.  There are two other
   settings I can think of that libguestfs would like to adjust but
   cannot because they are only available in a configuration file.
   
   
   This all will be solved by administration module, once we implement
   it. I don't know about anybody working on it though.
  
  Yeah, we really need to get our act together on that. I might even be
  able to squeeze out some free time for this in the next few weeks. At
  least to get a proof of concept working with 1 or 2 example APIs.
 
 Is there some background reading on this feature?

Nothing nicely written up in any one place.

The general idea though is that we'll create an administrative API for
libvirtd. eg a libvirtadmin.so that connects to a dedicated UNIX socket
like /var/run/libvirt/libvirt-admin which has its own RPC program running
separate from the main RPC program. This library / RPC protocol would be
thus independant of any specific HV connection. The original motivation
was to provide the host admin with a way to turn on/off logging levels
without having to restart libvirtd itself. We also wanted a way to inspect
what clients are connected and what API calls they were waiting for
completion of.

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] [PATCHv6 4/5] vbox_tmpl.c: Patch for redefining snapshots

2014-02-19 Thread Daniel P. Berrange
On Thu, Jan 23, 2014 at 10:28:32AM +0100, Manuel VIVES wrote:
 The snapshots are saved in xml files, and then can be redefined
 ---
  src/vbox/vbox_tmpl.c | 1083 
 +-
  1 file changed, 1075 insertions(+), 8 deletions(-)

My comment here is that you ought to separate the XML handling
out from the driver. Create a new file vbox_snapshot_conf.{c,h}
In the header define a struct containing the data to be saved
in the XML. Then provide an API for writing the struct to an
XML doc, and another API for reading the XML to create a struct.
Then the vbox_tmpl.c would call those methods.

Also, a bit more info about the XML format in the commit message
would be useful. In particular i'm unclear if this XML document
is something you're inventing specifically for use by libvirt
here, or is a format the VirtualBox itself has defined.

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] [PATCHv6 5/5] vbox_tmpl.c: Add methods for undefining snapshots

2014-02-19 Thread Daniel P. Berrange
On Thu, Jan 23, 2014 at 10:28:33AM +0100, Manuel VIVES wrote:
 All the informations concerning snapshots (and snapshot disks)
 will be deleted from the vbox xml. But the differencing disks will be
 kept so you will be able to redefine the snapshots.
 ---
  src/vbox/vbox_tmpl.c |  448 
 +-
  1 file changed, 447 insertions(+), 1 deletion(-)

Same comments as previous patch about separating the XML parsing
from the driver code.

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 V2 01/13] libxl: always set vm id to -1 on shutdown

2014-02-19 Thread Daniel P. Berrange
On Wed, Feb 12, 2014 at 06:56:15PM -0700, Jim Fehlig wrote:
 Once a domain has reached the shutdown state, set its ID to -1.
 
 Signed-off-by: Jim Fehlig jfeh...@suse.com
 ---
  src/libxl/libxl_driver.c | 6 +++---
  1 file changed, 3 insertions(+), 3 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 V2 02/13] libxl: remove libxlVmReap function

2014-02-19 Thread Daniel P. Berrange
On Wed, Feb 12, 2014 at 06:56:16PM -0700, Jim Fehlig wrote:
 This function, which only has five call sites, simply calls
 libxl_domain_destroy and libxlVmCleanup.  Call those functions
 directly at the call sites, allowing more control over how a
 domain is destroyed and cleaned up.  This patch maintains the
 existing semantic, leaving changes to a subsequent patch.
 
 Signed-off-by: Jim Fehlig jfeh...@suse.com
 ---
  src/libxl/libxl_driver.c | 39 ---
  1 file changed, 12 insertions(+), 27 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] Reporting log/error messages through capabilities

2014-02-19 Thread Richard W.M. Jones
On Wed, Feb 19, 2014 at 02:00:21PM +, Daniel P. Berrange wrote:
 On Tue, Feb 18, 2014 at 11:11:10PM +, Richard W.M. Jones wrote:
  There is a libvirt bug here, which is that it's very hard to diagnose
  what is going on when qemu fails to work at all.  The logging system
  in libvirt(d) is trememdously powerful, but ultimately confusing to
  use, and requires users to edit config files which makes it a
  non-starter for programs using libvirt through the API [1].
 
 The problem with allowing apps to change the logging config is that
 it is global state, not per client. So multiple apps would conflict
 in what they could do with changes here. While we could probably
 make it possible for apps to register their own callback to receive
 log messages, the setting of actual log levels would still be global.

This comes back to the whole private libvirtd thing.  Even sharing a
single session libvirtd with the current user has proven problematic
for libguestfs, and it's a big mess for root.  See bugs passim.  In an
ideal world we'd have one private libvirtd per connection.

  [1] By the way, this is a general complaint about libvirt.  Please
  DON'T add any more stuff to the configuration file.  Everything should
  be configurable through the API, or not at all.  There are two other
  settings I can think of that libguestfs would like to adjust but
  cannot because they are only available in a configuration file.
 
 What are the other settings you're thinking of here?

- log_level / log messages as discussed.

- qemu user/group: when libguestfs runs as root, we'd like to set it
  to root/root

- max_clients

There are lots of problems (still) with max_clients.  The default
setting is far too low.  The default was recently increased but we
cannot read what it is, thus cannot make a decision on how many
threads we can run safely.  And being a global setting (and us being a
local library) it wouldn't help much even if we could read it, because
another libguestfs instance might be running threads.  Ideally it
would be just a safety mechanism, stopping someone from connecting
thousands of times, and would also depend on the size of the main
memory.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming blog: http://rwmj.wordpress.com
Fedora now supports 80 OCaml packages (the OPEN alternative to F#)

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


Re: [libvirt] [libvirt-users] event-test.py cannot detects domain shutdown

2014-02-19 Thread Jim Fehlig
Cole Robinson wrote:
 On 02/18/2014 10:45 PM, Jim Fehlig wrote:
   
 Cole Robinson wrote:
 
 On 02/18/2014 05:12 AM, Kim Larry wrote:
   
   
 The thing I found today is that if libvirt uses xend driver, shutdown 
 events
 are delivered, but if libvirt uses libxl drvier, doesn't show up anything. 
 It
 seems there are bugs on shutdown event, so I did dig into the libvirt 
 source
 briefly, but I couldn't find where libxl driver calls event callback.

 Any help will be greatly appreciated. Especially someone who is familiar 
 with
 xen code.

 
 
 Jim, does this sound familiar?
   
   
 Looks like the event handler only queues a crashed event when receiving
 the shutdown event from libxl.  The attached patch should fix it, but I
 think the logic could be further improved.  I'll look at doing that
 tomorrow.

 Regards,
 Jim

 From c05615b29f8870d20b4457a2e8abe5e4195275c2 Mon Sep 17 00:00:00 2001
 From: Jim Fehlig jfeh...@suse.com
 Date: Tue, 18 Feb 2014 20:34:47 -0700
 Subject: [PATCH] libxl: queue shutdown event on domain shutdown

 Emmit libvirt shutdown event when receiving LIBXL_SHUTDOWN_REASON_POWEROFF
 event from libxl.

 Signed-off-by: Jim Fehlig jfeh...@suse.com
 ---
  src/libxl/libxl_driver.c | 3 +++
  1 file changed, 3 insertions(+)

 diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
 index 8e4242a..8d5e101 100644
 --- a/src/libxl/libxl_driver.c
 +++ b/src/libxl/libxl_driver.c
 @@ -383,6 +383,9 @@ libxlDomainShutdownThread(void *opaque)
 
 VIR_DOMAIN_EVENT_STOPPED_CRASHED);
  reason = VIR_DOMAIN_SHUTOFF_CRASHED;
  } else {
 +dom_event = virDomainEventLifecycleNewFromObj(vm,
 +   VIR_DOMAIN_EVENT_STOPPED,
 +   
 VIR_DOMAIN_EVENT_STOPPED_SHUTDOWN);
  reason = VIR_DOMAIN_SHUTOFF_SHUTDOWN;
  }
  libxlVmReap(driver, vm, reason);
 

 ACK, looks fine to me, but I'm guessing the REBOOT handling right below also
 needs an event dispatched. Probably best to see what the qemu or test driver
 do for reboot and copy it.
   

Looks like there is no explicit reboot event emitted, only a shutdown
event when on_reboot is set to 'destroy'.  Sadly, the libxl driver is
currently ignoring the on_* event configuration :-(.  I'll work on a
fix for this.

Regards,
Jim

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


Re: [libvirt] Reporting log/error messages through capabilities

2014-02-19 Thread Daniel P. Berrange
On Wed, Feb 19, 2014 at 05:00:43PM +, Richard W.M. Jones wrote:
 On Wed, Feb 19, 2014 at 02:00:21PM +, Daniel P. Berrange wrote:
  On Tue, Feb 18, 2014 at 11:11:10PM +, Richard W.M. Jones wrote:
   There is a libvirt bug here, which is that it's very hard to diagnose
   what is going on when qemu fails to work at all.  The logging system
   in libvirt(d) is trememdously powerful, but ultimately confusing to
   use, and requires users to edit config files which makes it a
   non-starter for programs using libvirt through the API [1].
  
  The problem with allowing apps to change the logging config is that
  it is global state, not per client. So multiple apps would conflict
  in what they could do with changes here. While we could probably
  make it possible for apps to register their own callback to receive
  log messages, the setting of actual log levels would still be global.
 
 This comes back to the whole private libvirtd thing.  Even sharing a
 single session libvirtd with the current user has proven problematic
 for libguestfs, and it's a big mess for root.  See bugs passim.  In an
 ideal world we'd have one private libvirtd per connection.

Yep, I don't have a perfect answer for that yet. Conceptually the
idea of having a dedicated root session libvirtd but it does raise
co-ordination issues. eg when libvirtd tracks what vms are using
what PCI devices it is assuming there's only one privileged libvirtd
instance. We could avoid this by saying that the 'session' instance
that runs as root is unprivileged and thus not allowed to use PCI
devices and other such things, but there could be dragons here.

   [1] By the way, this is a general complaint about libvirt.  Please
   DON'T add any more stuff to the configuration file.  Everything should
   be configurable through the API, or not at all.  There are two other
   settings I can think of that libguestfs would like to adjust but
   cannot because they are only available in a configuration file.
  
  What are the other settings you're thinking of here?
 
 - log_level / log messages as discussed.
 
 - qemu user/group: when libguestfs runs as root, we'd like to set it
   to root/root

We do have an override for that in the XML now, though I do
recall there were some issues. Conceptually though, the goal
of the XML override is that it /ought/ to be functionally
identical to changing the global config file.

 - max_clients
 
 There are lots of problems (still) with max_clients.  The default
 setting is far too low.  The default was recently increased but we
 cannot read what it is, thus cannot make a decision on how many
 threads we can run safely.  And being a global setting (and us being a
 local library) it wouldn't help much even if we could read it, because
 another libguestfs instance might be running threads.  Ideally it
 would be just a safety mechanism, stopping someone from connecting
 thousands of times, and would also depend on the size of the main
 memory.

We are partway through changing this. The end goal is that we'll have a
new 'max_anonymous_clients' setting that will be a low value and just
prevents DOS from clients which have not yet authenticated. The existing
'max_clients' value will then only apply to authenticated clients, and
can thus be raised to a very high value such that people won't hit it
in any reasonably normal usage.

I thought we'd finished it, but I see it is outstanding review still

  https://www.redhat.com/archives/libvir-list/2013-December/msg00453.html

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 V2 04/13] libxl: use job functions in libxlVmStart

2014-02-19 Thread Daniel P. Berrange
On Wed, Feb 12, 2014 at 06:56:18PM -0700, Jim Fehlig wrote:
 Creating a large domain could potentially be time consuming.  Use the
 recently added job functions and unlock the virDomainObj while
 the create operation is in progress.
 
 Signed-off-by: Jim Fehlig jfeh...@suse.com
 ---
 
 V2:
   Defer getting libxlDriverConfig until aquiring a job, simplifying
   cleanup handling a bit.
   Check libxlDomainObjEndJob() return value
 
  src/libxl/libxl_driver.c | 66 
 +++-
  1 file changed, 37 insertions(+), 29 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 V2 03/13] libxl: Add job support to libxl driver

2014-02-19 Thread Daniel P. Berrange
On Wed, Feb 12, 2014 at 06:56:17PM -0700, Jim Fehlig wrote:
 Follows the pattern used in the QEMU driver for managing multiple,
 simultaneous jobs within the driver.
 
 Signed-off-by: Jim Fehlig jfeh...@suse.com
 ---
 
 V2:
   Add ATTRIBUTE_RETURN_CHECK to libxlDomainObjEndJob
 
  src/libxl/libxl_domain.c | 128 
 +++
  src/libxl/libxl_domain.h |  38 ++
  2 files changed, 166 insertions(+)

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 V2 06/13] libxl: use job functions in libxlDomain{Suspend, Resume}

2014-02-19 Thread Daniel P. Berrange
On Wed, Feb 12, 2014 at 06:56:20PM -0700, Jim Fehlig wrote:
 These operations aren't necessarily time consuming, but need to
 wait in the queue of modify jobs.
 
 Signed-off-by: Jim Fehlig jfeh...@suse.com
 ---
 
 V2: Check libxlDomainObjEndJob() return value
 
  src/libxl/libxl_driver.c | 26 --
  1 file changed, 20 insertions(+), 6 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 V2 05/13] libxl: use job functions in libxlDomainSetMemoryFlags

2014-02-19 Thread Daniel P. Berrange
On Wed, Feb 12, 2014 at 06:56:19PM -0700, Jim Fehlig wrote:
 Large balloon operation can be time consuming.  Use the recently
 added job functions and unlock the virDomainObj while ballooning.
 
 Signed-off-by: Jim Fehlig jfeh...@suse.com
 ---
 
 V2: Check libxlDomainObjEndJob() return value
 
  src/libxl/libxl_driver.c | 33 +++--
  1 file changed, 23 insertions(+), 10 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 V2 07/13] libxl: use job functions when cleaning up a domain

2014-02-19 Thread Daniel P. Berrange
On Wed, Feb 12, 2014 at 06:56:21PM -0700, Jim Fehlig wrote:
 When explicitly destroying a domain (libxlDomainDestroyFlags), or
 handling an out-of-band domain shutdown event, cleanup the domain
 in the context of a job.  Introduce libxlVmCleanupJob to wrap
 libxlVmCleanup in a job block.
 
 Signed-off-by: Jim Fehlig jfeh...@suse.com
 ---
 
 V2:
   Introduce libxlVmCleanupJob and call it when needing libxlVmCleanup
   in a job block.
   Check libxlDomainObjEndJob() return value
 
  src/libxl/libxl_driver.c | 38 ++
  1 file changed, 30 insertions(+), 8 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 V2 08/13] libxl: use job functions in domain save operations

2014-02-19 Thread Daniel P. Berrange
On Wed, Feb 12, 2014 at 06:56:22PM -0700, Jim Fehlig wrote:
 Saving domain memory and cpu state can take considerable time.
 Use the recently added job functions and unlock the virDomainObj
 while saving the domain.
 
 Signed-off-by: Jim Fehlig jfeh...@suse.com
 ---
 
 V2: Check libxlDomainObjEndJob() return value
 
  src/libxl/libxl_driver.c | 56 
 +++-
  1 file changed, 41 insertions(+), 15 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 V2 11/13] libxl: use job functions in device attach and detach functions

2014-02-19 Thread Daniel P. Berrange
On Wed, Feb 12, 2014 at 06:56:25PM -0700, Jim Fehlig wrote:
 These operations aren't necessarily time consuming, but need to
 wait in the queue of modify jobs.
 
 Signed-off-by: Jim Fehlig jfeh...@suse.com
 ---
 
 V2: Check libxlDomainObjEndJob() return value
 
  src/libxl/libxl_driver.c | 42 --
  1 file changed, 28 insertions(+), 14 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 V2 09/13] libxl: use job functions in libxlDomainCoreDump

2014-02-19 Thread Daniel P. Berrange
On Wed, Feb 12, 2014 at 06:56:23PM -0700, Jim Fehlig wrote:
 Dumping a domain's core can take considerable time.  Use the
 recently added job functions and unlock the virDomainObj while
 dumping core.
 
 Signed-off-by: Jim Fehlig jfeh...@suse.com
 ---
 
 V2: Check libxlDomainObjEndJob() return value
 
  src/libxl/libxl_driver.c | 38 +++---
  1 file changed, 27 insertions(+), 11 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 V2 10/13] libxl: use job functions in vcpu set and pin functions

2014-02-19 Thread Daniel P. Berrange
On Wed, Feb 12, 2014 at 06:56:24PM -0700, Jim Fehlig wrote:
 These operations aren't necessarily time consuming, but need to
 wait in the queue of modify jobs.
 
 Signed-off-by: Jim Fehlig jfeh...@suse.com
 ---
 
 V2: Check libxlDomainObjEndJob() return value
 
  src/libxl/libxl_driver.c | 48 
 +++-
  1 file changed, 31 insertions(+), 17 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 V2 12/13] libxl: use job functions in libxlDomainSetAutostart

2014-02-19 Thread Daniel P. Berrange
On Wed, Feb 12, 2014 at 06:56:26PM -0700, Jim Fehlig wrote:
 Setting autostart is a modify operation that needs to wait in the
 queue of modify jobs.
 
 Signed-off-by: Jim Fehlig jfeh...@suse.com
 ---
 
 V2: Check libxlDomainObjEndJob() return value
 
  src/libxl/libxl_driver.c | 19 +--
  1 file changed, 13 insertions(+), 6 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/2] Introduce max_anonymous_clients

2014-02-19 Thread Daniel P. Berrange
On Tue, Jan 21, 2014 at 11:04:37AM +0100, Michal Privoznik wrote:
 On 09.12.2013 15:35, Michal Privoznik wrote:
  https://bugzilla.redhat.com/show_bug.cgi?id=981729
  
  So far we can limit how many clients are connected,
  how many are waiting in accept() line but we could
  not control the count of accepted but not
  authenticated yet.
  
  Michal Privoznik (2):
virNetServer: Introduce unauth clients counter
daemon: Introduce max_anonymous_clients
  
   daemon/libvirtd-config.c|  1 +
   daemon/libvirtd-config.h|  1 +
   daemon/libvirtd.aug |  1 +
   daemon/libvirtd.c   |  1 +
   daemon/libvirtd.conf|  3 ++
   daemon/remote.c | 21 -
   daemon/test_libvirtd.aug.in |  1 +
   src/locking/lock_daemon.c   |  4 +--
   src/lxc/lxc_controller.c|  2 +-
   src/rpc/virnetserver.c  | 73 
  +
   src/rpc/virnetserver.h  |  3 ++
   11 files changed, 95 insertions(+), 16 deletions(-)
  
 
 Ping3? Now that we are after the release it's a great time to merge this
 and have as long testing phase as possible.

Seems we dropped the ball on this patch. Can you re-post so we can get
it in just after 1.2.2 is released.

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 V2 13/13] libxl: use job functions in libxlDomainSetSchedulerParametersFlags

2014-02-19 Thread Daniel P. Berrange
On Wed, Feb 12, 2014 at 06:56:27PM -0700, Jim Fehlig wrote:
 Modify operation that needs to wait in the queue of modify jobs.
 
 Signed-off-by: Jim Fehlig jfeh...@suse.com
 ---
 
 V2: Check libxlDomainObjEndJob() return value
 
  src/libxl/libxl_driver.c | 16 
  1 file changed, 12 insertions(+), 4 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] [PATCHv6 1/5] virstring.h/c: Util method for finding regexp patterns in some strings

2014-02-19 Thread Daniel P. Berrange
On Wed, Feb 19, 2014 at 04:46:22PM +, Daniel P. Berrange wrote:
 On Thu, Jan 23, 2014 at 10:28:29AM +0100, Manuel VIVES wrote:
  ---
   po/POTFILES.in   |1 +
   src/libvirt_private.syms |1 +
   src/util/virstring.c |   97 
  ++
   src/util/virstring.h |3 ++
   4 files changed, 102 insertions(+)
 
 Can you add something to tests/virstringtest.c to validate your
 new method with various interesting input strings.

Actually,  I'll finish up the changes/fixes for the 3 virstring
related patches here, so you can just focus on the last 3 vbox
specific 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] [PATCH 0/2] Introduce max_anonymous_clients

2014-02-19 Thread Eric Blake
On 02/19/2014 10:55 AM, Daniel P. Berrange wrote:
 On Tue, Jan 21, 2014 at 11:04:37AM +0100, Michal Privoznik wrote:
 On 09.12.2013 15:35, Michal Privoznik wrote:
 https://bugzilla.redhat.com/show_bug.cgi?id=981729

 So far we can limit how many clients are connected,
 how many are waiting in accept() line but we could
 not control the count of accepted but not
 authenticated yet.

 Ping3? Now that we are after the release it's a great time to merge this
 and have as long testing phase as possible.
 
 Seems we dropped the ball on this patch. Can you re-post so we can get
 it in just after 1.2.2 is released.

Any reason we shouldn't get it in now, before the 1.2.2 freeze?  It's
less testing time than if we had done it right after 1.2.1, but still a
week and a half before the final release.

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



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

Re: [libvirt] [PATCH 0/2] Introduce max_anonymous_clients

2014-02-19 Thread Daniel P. Berrange
On Wed, Feb 19, 2014 at 11:03:00AM -0700, Eric Blake wrote:
 On 02/19/2014 10:55 AM, Daniel P. Berrange wrote:
  On Tue, Jan 21, 2014 at 11:04:37AM +0100, Michal Privoznik wrote:
  On 09.12.2013 15:35, Michal Privoznik wrote:
  https://bugzilla.redhat.com/show_bug.cgi?id=981729
 
  So far we can limit how many clients are connected,
  how many are waiting in accept() line but we could
  not control the count of accepted but not
  authenticated yet.
 
  Ping3? Now that we are after the release it's a great time to merge this
  and have as long testing phase as possible.
  
  Seems we dropped the ball on this patch. Can you re-post so we can get
  it in just after 1.2.2 is released.
 
 Any reason we shouldn't get it in now, before the 1.2.2 freeze?  It's
 less testing time than if we had done it right after 1.2.1, but still a
 week and a half before the final release.

Guess I'm just a little more wary of changes to the core rpc apis. eg
If we mess this up it is quite likely to become a security vulnerability.

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 V2 13/13] libxl: use job functions in libxlDomainSetSchedulerParametersFlags

2014-02-19 Thread Jim Fehlig
Daniel P. Berrange wrote:
 On Wed, Feb 12, 2014 at 06:56:27PM -0700, Jim Fehlig wrote:
   
 Modify operation that needs to wait in the queue of modify jobs.

 Signed-off-by: Jim Fehlig jfeh...@suse.com
 ---

 V2: Check libxlDomainObjEndJob() return value

  src/libxl/libxl_driver.c | 16 
  1 file changed, 12 insertions(+), 4 deletions(-)
 

 ACK
   

Thanks for the reviews!  I've pushed this series now.

Regards,
Jim

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


Re: [libvirt] [PATCHv3 00/10] Add BlkIO and CPU/mem stat API implementations for lxc

2014-02-19 Thread Thorsten Behrens
I wrote:
 This patch set adds block io, memory and domain cpu statistics API
 slot implementations to the LXC driver, in order to get linux
 container monitoring and accounting a bit closer to qemu standards.

Hi guys,

hmm, this would be cool to get into 1.2.2 - any chance for that? ;)

Some patches were ACKed already, and I've addressed the remaining
review comments on 02, 04 and 07. For 05, I did not modify the qemu
driver, but relaxed the docs in 06 already (there was no explicit
failure mode for that case anyway, in libvirt.c).

Thanks a lot,

-- Thorsten


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

[libvirt] [PATCH 0/2] Add two string helper methods

2014-02-19 Thread Daniel P. Berrange
As mentioned in my review, I took the first two patches from
this series and polished them up ready for merge

   https://www.redhat.com/archives/libvir-list/2014-January/msg01067.html

Hopefully this should be good enough for the vbox snapshot
work to use.

Changes since that posting

 - Added unit tests
 - Changed virStringSearch API to return all matches in one
   go, rather than need repeated calls
 - Rewrote virStringReplace to use virBuffer APIs

Daniel P. Berrange (1):
  Add virStringReplace method for substring replacement

Manuel VIVES (1):
  Add virStringSearch method for regex matching

 po/POTFILES.in   |   1 +
 src/libvirt_private.syms |   2 +
 src/util/virstring.c | 146 ++-
 src/util/virstring.h |  12 
 tests/virstringtest.c| 159 +++
 5 files changed, 319 insertions(+), 1 deletion(-)

-- 
1.8.5.3

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


[libvirt] [PATCH 2/2] Add virStringReplace method for substring replacement

2014-02-19 Thread Daniel P. Berrange
Add a virStringReplace method to virstring.{h,c} to perform
substring matching and replacement

Signed-off-by: Daniel P. Berrange berra...@redhat.com
---
 src/libvirt_private.syms |  1 +
 src/util/virstring.c | 44 +++-
 src/util/virstring.h |  5 
 tests/virstringtest.c| 59 
 4 files changed, 108 insertions(+), 1 deletion(-)

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index f26190d..f7379a2 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1794,6 +1794,7 @@ virStringArrayHasString;
 virStringFreeList;
 virStringJoin;
 virStringListLength;
+virStringReplace;
 virStringSearch;
 virStringSortCompare;
 virStringSortRevCompare;
diff --git a/src/util/virstring.c b/src/util/virstring.c
index 67a87d3..3e42b06 100644
--- a/src/util/virstring.c
+++ b/src/util/virstring.c
@@ -619,7 +619,6 @@ size_t virStringListLength(char **strings)
 return i;
 }
 
-
 /**
  * virStringSortCompare:
  *
@@ -747,3 +746,46 @@ cleanup:
 }
 return ret;
 }
+
+/**
+ * virStringReplace:
+ * @haystack: the source string to process
+ * @oldneedle: the substring to locate
+ * @newneedle: the substring to insert
+ *
+ * Search @haystack and replace all occurences of @oldneedle with @newneedle.
+ *
+ * Returns: a new string with all the replacements, or NULL on error
+ */
+char *
+virStringReplace(const char *haystack,
+ const char *oldneedle,
+ const char *newneedle)
+{
+virBuffer buf = VIR_BUFFER_INITIALIZER;
+const char *tmp1, *tmp2;
+size_t oldneedlelen = strlen(oldneedle);
+size_t newneedlelen = strlen(newneedle);
+
+tmp1 = haystack;
+tmp2 = NULL;
+
+while (tmp1) {
+tmp2 = strstr(tmp1, oldneedle);
+
+if (tmp2) {
+virBufferAdd(buf, tmp1, (tmp2 - tmp1));
+virBufferAdd(buf, newneedle, newneedlelen);
+tmp2 += oldneedlelen;
+} else {
+virBufferAdd(buf, tmp1, -1);
+}
+
+tmp1 = tmp2;
+}
+
+if (virBufferError(buf))
+return NULL;
+
+return virBufferContentAndReset(buf);
+}
diff --git a/src/util/virstring.h b/src/util/virstring.h
index 8efc80c..5b77581 100644
--- a/src/util/virstring.h
+++ b/src/util/virstring.h
@@ -232,5 +232,10 @@ ssize_t virStringSearch(const char *str,
 char ***matches)
 ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(4);
 
+char *virStringReplace(const char *haystack,
+   const char *oldneedle,
+   const char *newneedle)
+ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3);
+
 
 #endif /* __VIR_STRING_H__ */
diff --git a/tests/virstringtest.c b/tests/virstringtest.c
index b8c6115..43023d5 100644
--- a/tests/virstringtest.c
+++ b/tests/virstringtest.c
@@ -338,6 +338,38 @@ testStringSearch(const void *opaque ATTRIBUTE_UNUSED)
 return ret;
 }
 
+
+struct stringReplaceData {
+const char *haystack;
+const char *oldneedle;
+const char *newneedle;
+const char *result;
+};
+
+static int
+testStringReplace(const void *opaque ATTRIBUTE_UNUSED)
+{
+const struct stringReplaceData *data = opaque;
+char *result;
+int ret = -1;
+
+result = virStringReplace(data-haystack,
+  data-oldneedle,
+  data-newneedle);
+
+if (STRNEQ_NULLABLE(data-result, result)) {
+fprintf(stderr, Expected '%s' but got '%s'\n,
+data-result, NULLSTR(result));
+goto cleanup;
+}
+
+ret = 0;
+
+ cleanup:
+return ret;
+}
+
+
 static int
 mymain(void)
 {
@@ -428,6 +460,33 @@ mymain(void)
 const char *matches3[] = { foo, bar };
 TEST_SEARCH(1foo2bar3eek, ([a-z]+), 2, 2, matches3, false);
 
+#define TEST_REPLACE(h, o, n, r)\
+do {\
+struct stringReplaceData data = {   \
+.haystack = h,  \
+.oldneedle = o, \
+.newneedle = n, \
+.result = r \
+};  \
+if (virtTestRun(virStringReplace  h, testStringReplace, data)  0) \
+ret = -1;   \
+} while (0)
+
+/* no matches */
+TEST_REPLACE(foo, bar, eek, foo);
+
+/* complete match */
+TEST_REPLACE(foo, foo, bar, bar);
+
+/* middle match */
+TEST_REPLACE(foobarwizz, bar, eek, fooeekwizz);
+
+/* many matches */
+TEST_REPLACE(foofoofoofoo, foo, bar, barbarbarbar);
+
+/* many matches */
+TEST_REPLACE(fof, foo, bar, barooobaroo);
+
 return 

[libvirt] [PATCH 1/2] Add virStringSearch method for regex matching

2014-02-19 Thread Daniel P. Berrange
From: Manuel VIVES manuel.vi...@diateam.net

Add a virStringSearch method to virstring.{c,h} which performs
a regex match against a string and returns the matching substrings.

Signed-off-by: Daniel P. Berrange berra...@redhat.com
---
 po/POTFILES.in   |   1 +
 src/libvirt_private.syms |   1 +
 src/util/virstring.c | 102 +++
 src/util/virstring.h |   7 
 tests/virstringtest.c| 100 ++
 5 files changed, 211 insertions(+)

diff --git a/po/POTFILES.in b/po/POTFILES.in
index 5d02062..c565771 100644
--- a/po/POTFILES.in
+++ b/po/POTFILES.in
@@ -192,6 +192,7 @@ src/util/virscsi.c
 src/util/virsocketaddr.c
 src/util/virstatslinux.c
 src/util/virstoragefile.c
+src/util/virstring.c
 src/util/virsysinfo.c
 src/util/virerror.c
 src/util/virerror.h
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index ec786e4..f26190d 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1794,6 +1794,7 @@ virStringArrayHasString;
 virStringFreeList;
 virStringJoin;
 virStringListLength;
+virStringSearch;
 virStringSortCompare;
 virStringSortRevCompare;
 virStringSplit;
diff --git a/src/util/virstring.c b/src/util/virstring.c
index 8d0ca70..67a87d3 100644
--- a/src/util/virstring.c
+++ b/src/util/virstring.c
@@ -23,12 +23,14 @@
 
 #include stdlib.h
 #include stdio.h
+#include regex.h
 
 #include c-ctype.h
 #include virstring.h
 #include viralloc.h
 #include virbuffer.h
 #include virerror.h
+#include virlog.h
 
 #define VIR_FROM_THIS VIR_FROM_NONE
 
@@ -645,3 +647,103 @@ int virStringSortRevCompare(const void *a, const void *b)
 
 return strcmp(*sb, *sa);
 }
+
+/**
+ * virStringSearch:
+ * @str: string to search
+ * @regexp: POSIX Extended regular expression pattern used for matching
+ * @max_matches: maximum number of substrings to return
+ * @result: pointer to an array to be filled with NULL terminated list of 
matches
+ *
+ * Performs a POSIX extended regex search against a string and return all 
matching substrings.
+ * The @result value should be freed with  virStringFreeList() when no longer
+ * required.
+ *
+ * @code
+ *  char *source = 6853a496-1c10-472e-867a-8244937bd6f0
+ *  773ab075-4cd7-4fc2-8b6e-21c84e9cb391
+ *  bbb3c75c-d60f-43b0-b802-fd56b84a4222
+ *  60c04aa1-0375-4654-8d9f-e149d9885273
+ *  4548d465-9891-4c34-a184-3b1c34a26aa8;
+ *  char **matches = NULL;
+ *  virSearchRegex(source,
+ * 
([a-f0-9]{8}-[a-f0-9]{4}-[a-f0-9]{4}-[a-f0-9]{4}-[a-f0-9]{12}),
+ *  3,
+ *  matches);
+ *
+ *  // matches[0] == 4548d465-9891-4c34-a184-3b1c34a26aa8;
+ *  // matches[1] == 6853a496-1c10-472e-867a-8244937bd6f0;
+ *  // matches[2] == 773ab075-4cd7-4fc2-8b6e-21c84e9cb391
+ *  // matches[3] == NULL;
+ *
+ *  virStringFreeList(matches);
+ * @endcode
+ *
+ * Returns: -1 on error, or number of matches
+ */
+ssize_t
+virStringSearch(const char *str,
+const char *regexp,
+size_t max_matches,
+char ***matches)
+{
+regex_t re;
+regmatch_t rem;
+size_t nmatches = 0;
+ssize_t ret = -1;
+int rv = -1;
+
+*matches = NULL;
+
+VIR_DEBUG(search '%s' for '%s', str, regexp);
+
+if ((rv = regcomp(re, regexp, REG_EXTENDED)) != 0) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _(Error while compiling regular expression '%s': %d),
+   regexp, rv);
+return -1;
+}
+
+if (re.re_nsub != 1) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _(Regular expression '%s' must have exactly 1 match 
group, not %zu),
+   regexp, re.re_nsub);
+goto cleanup;
+}
+
+/* '*matches' must always be NULL terminated in every iteration
+ * of the loop, so start by allocating 1 element
+ */
+if (VIR_EXPAND_N(*matches, nmatches, 1)  0)
+goto cleanup;
+
+while ((nmatches - 1)  max_matches) {
+char *match;
+
+if (regexec(re, str, 1, rem, 0) != 0)
+break;
+
+if (VIR_EXPAND_N(*matches, nmatches, 1)  0)
+goto cleanup;
+
+if (VIR_STRNDUP(match, str + rem.rm_so,
+rem.rm_eo - rem.rm_so)  0)
+goto cleanup;
+
+VIR_DEBUG(Got '%s', match);
+
+(*matches)[nmatches-2] = match;
+
+str = str + rem.rm_eo;
+}
+
+ret = nmatches - 1; /* don't count the trailing null */
+
+cleanup:
+regfree(re);
+if (ret  0) {
+virStringFreeList(*matches);
+*matches = NULL;
+}
+return ret;
+}
diff --git a/src/util/virstring.h b/src/util/virstring.h
index 13a6e5a..8efc80c 100644
--- a/src/util/virstring.h
+++ b/src/util/virstring.h
@@ -226,4 +226,11 @@ size_t virStringListLength(char **strings);
 int virStringSortCompare(const void *a, const void *b);
 int virStringSortRevCompare(const 

Re: [libvirt] [PATCHv3 00/10] Add BlkIO and CPU/mem stat API implementations for lxc

2014-02-19 Thread Daniel P. Berrange
On Wed, Feb 19, 2014 at 08:38:33PM +0100, Thorsten Behrens wrote:
 I wrote:
  This patch set adds block io, memory and domain cpu statistics API
  slot implementations to the LXC driver, in order to get linux
  container monitoring and accounting a bit closer to qemu standards.
 
 Hi guys,
 
 hmm, this would be cool to get into 1.2.2 - any chance for that? ;)
 
 Some patches were ACKed already, and I've addressed the remaining
 review comments on 02, 04 and 07. For 05, I did not modify the qemu
 driver, but relaxed the docs in 06 already (there was no explicit
 failure mode for that case anyway, in libvirt.c).

I'll review the rest of this tomorrow if no one beats me to it.
Getting into 1.2.2 is a reasonable target.

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 v1 0/2] support dumping guest memory in compressed format

2014-02-19 Thread Qiao Nuohan

dumping guest's memroy is introduced without compression supported, and this is
a freature regression of 'virsh dump --memory-only'. This patchset is used to
add support in libvirt side to make qemu dump guest's memory in kdump-compressed
format and please refer the following address to see implementation of the qemu
side, the lastest version of qemu side is v9(ready for being queued).

http://lists.nongnu.org/archive/html/qemu-devel/2014-02/msg03016.html

qiaonuohan (2):
  make qemu dump memory in kdump-compressed format
  add dump_memory_format in qemu.conf

 include/libvirt/libvirt.h.in   | 19 ++
 src/qemu/libvirtd_qemu.aug |  1 +
 src/qemu/qemu.conf | 13 +-
 src/qemu/qemu_conf.c   |  3 +++
 src/qemu/qemu_conf.h   |  2 ++
 src/qemu/qemu_driver.c | 51 +++---
 src/qemu/qemu_monitor.c|  6 ++---
 src/qemu/qemu_monitor.h|  3 ++-
 src/qemu/qemu_monitor_json.c   |  4 ++-
 src/qemu/qemu_monitor_json.h   |  3 ++-
 src/qemu/test_libvirtd_qemu.aug.in |  1 +
 tests/qemumonitorjsontest.c|  2 +-
 tools/virsh-domain.c   | 42 +++
 13 files changed, 133 insertions(+), 17 deletions(-)

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


[libvirt] [PATCH v1 1/2] make qemu dump memory in kdump-compressed format

2014-02-19 Thread Qiao Nuohan

--memory-only option is introduced without compression supported. Therefore,
this is a freature regression of virsh dump. Now qemu has support dumping memory
in kdump-compressed format. This patch is used to add --compress and
[--compression-format] string to virsh dump --memory-only and send
dump-guest-memory command to qemu with dump format specified to one of elf,
kdump-zlib, kdump-lzo and kdump-snappy.

Signed-off-by: Qiao Nuohan qiaonuo...@cn.fujitsu.com
---
 include/libvirt/libvirt.h.in | 19 ++-
 src/qemu/qemu_driver.c   | 20 
 src/qemu/qemu_monitor.c  |  6 +++---
 src/qemu/qemu_monitor.h  |  3 ++-
 src/qemu/qemu_monitor_json.c |  4 +++-
 src/qemu/qemu_monitor_json.h |  3 ++-
 tests/qemumonitorjsontest.c  |  2 +-
 tools/virsh-domain.c | 42 ++
 8 files changed, 83 insertions(+), 16 deletions(-)

diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
index 295d551..df62918 100644
--- a/include/libvirt/libvirt.h.in
+++ b/include/libvirt/libvirt.h.in
@@ -1173,11 +1173,20 @@ typedef virDomainMemoryStatStruct 
*virDomainMemoryStatPtr;

 /* Domain core dump flags. */
 typedef enum {
-VIR_DUMP_CRASH= (1  0), /* crash after dump */
-VIR_DUMP_LIVE = (1  1), /* live dump */
-VIR_DUMP_BYPASS_CACHE = (1  2), /* avoid file system cache pollution */
-VIR_DUMP_RESET= (1  3), /* reset domain after dump finishes */
-VIR_DUMP_MEMORY_ONLY  = (1  4), /* use dump-guest-memory */
+VIR_DUMP_CRASH   = (1  0), /* crash after dump */
+VIR_DUMP_LIVE= (1  1), /* live dump */
+VIR_DUMP_BYPASS_CACHE= (1  2), /* avoid file system cache pollution 
*/
+VIR_DUMP_RESET   = (1  3), /* reset domain after dump finishes */
+VIR_DUMP_MEMORY_ONLY = (1  4), /* use dump-guest-memory */
+VIR_DUMP_COMPRESS_ZLIB   = (1  5), /* dump guest memory in
+kdump-compressed format, with
+zlib-compressed */
+VIR_DUMP_COMPRESS_LZO= (1  6), /* dump guest memory in
+kdump-compressed format, with
+lzo-compressed */
+VIR_DUMP_COMPRESS_SNAPPY = (1  7), /* dump guest memory in
+kdump-compressed format, with
+snappy-compressed */
 } virDomainCoreDumpFlags;

 /* Domain migration flags. */
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 59e018d..19b4dd2 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -3383,7 +3383,8 @@ cleanup:
 }

 static int qemuDumpToFd(virQEMUDriverPtr driver, virDomainObjPtr vm,
-int fd, enum qemuDomainAsyncJob asyncJob)
+int fd, enum qemuDomainAsyncJob asyncJob,
+const char* dump_format)
 {
 qemuDomainObjPrivatePtr priv = vm-privateData;
 int ret = -1;
@@ -3403,7 +3404,7 @@ static int qemuDumpToFd(virQEMUDriverPtr driver, 
virDomainObjPtr vm,

 if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob)  0)
 return -1;

-ret = qemuMonitorDumpToFd(priv-mon, fd);
+ret = qemuMonitorDumpToFd(priv-mon, fd, dump_format);
 qemuDomainObjExitMonitor(driver, vm);

 return ret;
@@ -3421,6 +3422,7 @@ doCoreDump(virQEMUDriverPtr driver,
 virFileWrapperFdPtr wrapperFd = NULL;
 int directFlag = 0;
 unsigned int flags = VIR_FILE_WRAPPER_NON_BLOCKING;
+const char *dump_format;

 /* Create an empty file with appropriate ownership.  */
 if (dump_flags  VIR_DUMP_BYPASS_CACHE) {
@@ -3444,7 +3446,16 @@ doCoreDump(virQEMUDriverPtr driver,
 goto cleanup;

 if (dump_flags  VIR_DUMP_MEMORY_ONLY) {
-ret = qemuDumpToFd(driver, vm, fd, QEMU_ASYNC_JOB_DUMP);
+if (dump_flags  VIR_DUMP_COMPRESS_ZLIB)
+dump_format = kdump-zlib;
+else if (dump_flags  VIR_DUMP_COMPRESS_LZO)
+dump_format = kdump-lzo;
+else if (dump_flags  VIR_DUMP_COMPRESS_SNAPPY)
+dump_format = kdump-snappy;
+else
+dump_format = elf;
+ret = qemuDumpToFd(driver, vm, fd, QEMU_ASYNC_JOB_DUMP,
+   dump_format);
 } else {
 ret = qemuMigrationToFile(driver, vm, fd, 0, path,
   qemuCompressProgramName(compress), false,
@@ -3520,7 +3531,8 @@ static int qemuDomainCoreDump(virDomainPtr dom,

 virCheckFlags(VIR_DUMP_LIVE | VIR_DUMP_CRASH |
   VIR_DUMP_BYPASS_CACHE | VIR_DUMP_RESET |
-  VIR_DUMP_MEMORY_ONLY, -1);
+  VIR_DUMP_MEMORY_ONLY | VIR_DUMP_COMPRESS_ZLIB |
+  VIR_DUMP_COMPRESS_LZO | VIR_DUMP_COMPRESS_SNAPPY, -1);

 if (!(vm = qemuDomObjFromDomain(dom)))
 return -1;
diff --git a/src/qemu/qemu_monitor.c 

[libvirt] [PATCH v1 2/2] add dump_memory_format in qemu.conf

2014-02-19 Thread Qiao Nuohan

This patch is used to add dump_memory_format to qemu.conf and libvirt will use
it to specify the default format in which qemu dumps guest's memory. But when
--compress is specified with virsh dump --memory-only, the format configured
by dump_memory_format will be overrided. dump_memory_format can one of elf,
kdump-zlib, kdump-lzo and kdump-snappy. And elf is the default value.

Signed-off-by: Qiao Nuohan qiaonuo...@cn.fujitsu.com
---
 src/qemu/libvirtd_qemu.aug |  1 +
 src/qemu/qemu.conf | 13 -
 src/qemu/qemu_conf.c   |  3 +++
 src/qemu/qemu_conf.h   |  2 ++
 src/qemu/qemu_driver.c | 37 ++---
 src/qemu/test_libvirtd_qemu.aug.in |  1 +
 6 files changed, 53 insertions(+), 4 deletions(-)

diff --git a/src/qemu/libvirtd_qemu.aug b/src/qemu/libvirtd_qemu.aug
index a9ff421..d3704fc 100644
--- a/src/qemu/libvirtd_qemu.aug
+++ b/src/qemu/libvirtd_qemu.aug
@@ -61,6 +61,7 @@ module Libvirtd_qemu =
let save_entry =  str_entry save_image_format
  | str_entry dump_image_format
  | str_entry snapshot_image_format
+ | str_entry dump_memory_format
  | str_entry auto_dump_path
  | bool_entry auto_dump_bypass_cache
  | bool_entry auto_start_bypass_cache
diff --git a/src/qemu/qemu.conf b/src/qemu/qemu.conf
index e436084..148a6bc 100644
--- a/src/qemu/qemu.conf
+++ b/src/qemu/qemu.conf
@@ -292,7 +292,8 @@
 # dump_image_format is used when you use 'virsh dump' at emergency
 # crashdump, and if the specified dump_image_format is not valid, or
 # the requested compression program can't be found, this falls
-# back to raw compression.
+# back to raw compression. Note, dump_image_format doesn't work when you use
+# virsh dump --memory-only.
 #
 # snapshot_image_format specifies the compression algorithm of the memory save
 # image when an external snapshot of a domain is taken. This does not apply
@@ -303,6 +304,16 @@
 #dump_image_format = raw
 #snapshot_image_format = raw

+# Qemu can dump guest's memory in elf format or in kdump-compressed format, and
+# the compression of kdump_compressed format can be zlib/lzo/snappy.
+#
+# dump_memory_format is used to specify the format in which qemu dumps guest's
+# memory. However, if --compress is specified with 'virsh dump 
--memory-only',
+# dump_memory_format will not work.
+# dump_memory_format can be elf, kdump-zlib, kdump-lzo and kdump-snappy.
+#
+#dump_memory_format = elf
+
 # When a domain is configured to be auto-dumped when libvirtd receives a
 # watchdog event from qemu guest, libvirtd will save dump files in directory
 # specified by auto_dump_path. Default value is /var/lib/libvirt/qemu/dump
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index ecaaf81..7d225da 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -296,6 +296,7 @@ static void virQEMUDriverConfigDispose(void *obj)

 VIR_FREE(cfg-saveImageFormat);
 VIR_FREE(cfg-dumpImageFormat);
+VIR_FREE(cfg-dumpMemoryFormat);
 VIR_FREE(cfg-autoDumpPath);

 virStringFreeList(cfg-securityDriverNames);
@@ -548,6 +549,8 @@ int virQEMUDriverConfigLoadFile(virQEMUDriverConfigPtr cfg,
 GET_VALUE_STR(dump_image_format, cfg-dumpImageFormat);
 GET_VALUE_STR(snapshot_image_format, cfg-snapshotImageFormat);

+GET_VALUE_STR(dump_memory_format, cfg-dumpMemoryFormat);
+
 GET_VALUE_STR(auto_dump_path, cfg-autoDumpPath);
 GET_VALUE_BOOL(auto_dump_bypass_cache, cfg-autoDumpBypassCache);
 GET_VALUE_BOOL(auto_start_bypass_cache, cfg-autoStartBypassCache);
diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h
index 1f44a76..97b0f67 100644
--- a/src/qemu/qemu_conf.h
+++ b/src/qemu/qemu_conf.h
@@ -149,6 +149,8 @@ struct _virQEMUDriverConfig {
 char *dumpImageFormat;
 char *snapshotImageFormat;

+char *dumpMemoryFormat;
+
 char *autoDumpPath;
 bool autoDumpBypassCache;
 bool autoStartBypassCache;
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 19b4dd2..c6f8ae9 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -3410,6 +3410,21 @@ static int qemuDumpToFd(virQEMUDriverPtr driver, 
virDomainObjPtr vm,

 return ret;
 }

+typedef enum {
+QEMU_DUMP_MEMORY_FORMAT_ELF = 0,
+QEMU_DUMP_MEMORY_FORMAT_KDUMP_ZLIB = 1,
+QEMU_DUMP_MEMORY_FORMAT_KDUMP_LZO = 2,
+QEMU_DUMP_MEMORY_FORMAT_KDUMP_SNAPPY = 3,
+QEMU_DUMP_MEMORY_FORMAT_LAST
+} virQEMUDumpMemoryFormat;
+
+VIR_ENUM_DECL(qemuDumpMemoryFormat)
+VIR_ENUM_IMPL(qemuDumpMemoryFormat, QEMU_DUMP_MEMORY_FORMAT_LAST,
+  elf,
+  kdump-zlib,
+  kdump-lzo,
+  kdump-snappy)
+
 static int
 doCoreDump(virQEMUDriverPtr driver,
virDomainObjPtr vm,
@@ -3422,7 +3437,9 @@ doCoreDump(virQEMUDriverPtr driver,
 virFileWrapperFdPtr wrapperFd = NULL;
 int directFlag = 0;
 unsigned int flags = 

[libvirt] [PATCH] libxl: always use libxlVmCleanupJob in shutdown thread

2014-02-19 Thread Jim Fehlig
Commit e4a0e900 missed calling libxlVmCleanupJob in the shutdown
handler when processing a reboot event.

Signed-off-by: Jim Fehlig jfeh...@suse.com
---
 src/libxl/libxl_driver.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index f7ca91c..9f87e71 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -393,7 +393,7 @@ libxlDomainShutdownThread(void *opaque)
 break;
 case LIBXL_SHUTDOWN_REASON_REBOOT:
 libxl_domain_destroy(ctx, vm-def-id, NULL);
-libxlVmCleanup(driver, vm, VIR_DOMAIN_SHUTOFF_SHUTDOWN);
+libxlVmCleanupJob(driver, vm, VIR_DOMAIN_SHUTOFF_SHUTDOWN);
 libxlVmStart(driver, vm, 0, -1);
 break;
 default:
-- 
1.8.1.4

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


[libvirt] [PATCH 03/10] util: make it easier to reflect child exit status

2014-02-19 Thread Eric Blake
Thanks to namespaces, we have a couple of places in the code
base that want to reflect a child exit status, including the
ability to detect death by a signal, back to a grandparent.
Best to make it a reusable function.

* src/util/virprocess.h (virProcessExitWithStatus): New prototype.
* src/libvirt_private.syms (util/virprocess.h): Export it.
* src/util/virprocess.c (virProcessExitWithStatus): New function.
* tests/commandtest.c (test23): Test it.

Signed-off-by: Eric Blake ebl...@redhat.com
---
 src/libvirt_private.syms |  1 +
 src/util/virprocess.c| 41 ++-
 src/util/virprocess.h|  4 ++-
 tests/commandtest.c  | 64 
 4 files changed, 108 insertions(+), 2 deletions(-)

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index ec786e4..d7a9ee7 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1673,6 +1673,7 @@ virPortAllocatorRelease;

 # util/virprocess.h
 virProcessAbort;
+virProcessExitWithStatus;
 virProcessGetAffinity;
 virProcessGetNamespaces;
 virProcessGetStartTime;
diff --git a/src/util/virprocess.c b/src/util/virprocess.c
index 305c095..68c4c14 100644
--- a/src/util/virprocess.c
+++ b/src/util/virprocess.c
@@ -1,7 +1,7 @@
 /*
  * virprocess.c: interaction with processes
  *
- * Copyright (C) 2010-2013 Red Hat, Inc.
+ * Copyright (C) 2010-2014 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
@@ -25,6 +25,7 @@
 #include fcntl.h
 #include signal.h
 #include errno.h
+#include stdlib.h
 #include sys/wait.h
 #if HAVE_SETRLIMIT
 # include sys/time.h
@@ -983,3 +984,41 @@ virProcessRunInMountNamespace(pid_t pid ATTRIBUTE_UNUSED,
 return -1;
 }
 #endif
+
+
+/**
+ * virProcessExitWithStatus:
+ * @status: raw status to be reproduced when this process dies
+ *
+ * Given a raw status obtained by waitpid() or similar, attempt to
+ * make this process exit in the same manner.  If the child died by
+ * signal, reset that signal handler to default and raise the same
+ * signal; if that doesn't kill this process, then exit with 128 +
+ * signal number.  If @status can't be deciphered, use
+ * EXIT_CANNOT_INVOKE.
+ *
+ * Never returns.
+ */
+void
+virProcessExitWithStatus(int status)
+{
+int value = EXIT_CANNOT_INVOKE;
+
+if (WIFEXITED(status)) {
+value = WEXITSTATUS(status);
+} else if (WIFSIGNALED(status)) {
+struct sigaction act;
+sigset_t sigs;
+
+if (sigemptyset(sigs) == 0 
+sigaddset(sigs, WTERMSIG(status)) == 0)
+sigprocmask(SIG_UNBLOCK, sigs, NULL);
+memset(act, 0, sizeof(act));
+act.sa_handler = SIG_DFL;
+sigfillset(act.sa_mask);
+sigaction(WTERMSIG(status), act, NULL);
+raise(WTERMSIG(status));
+value = 128 + WTERMSIG(status);
+}
+exit(value);
+}
diff --git a/src/util/virprocess.h b/src/util/virprocess.h
index 5c173b0..b96dbd4 100644
--- a/src/util/virprocess.h
+++ b/src/util/virprocess.h
@@ -1,7 +1,7 @@
 /*
  * virprocess.h: interaction with processes
  *
- * Copyright (C) 2010-2013 Red Hat, Inc.
+ * Copyright (C) 2010-2014 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
@@ -33,6 +33,8 @@ virProcessTranslateStatus(int status);
 void
 virProcessAbort(pid_t pid);

+void virProcessExitWithStatus(int status) ATTRIBUTE_NORETURN;
+
 int
 virProcessWait(pid_t pid, int *exitstatus)
 ATTRIBUTE_RETURN_CHECK;
diff --git a/tests/commandtest.c b/tests/commandtest.c
index 042f049..fcda5e6 100644
--- a/tests/commandtest.c
+++ b/tests/commandtest.c
@@ -38,6 +38,7 @@
 #include virerror.h
 #include virthread.h
 #include virstring.h
+#include virprocess.h

 #define VIR_FROM_THIS VIR_FROM_NONE

@@ -937,6 +938,68 @@ cleanup:
 return ret;
 }

+
+static int
+test23(const void *unused ATTRIBUTE_UNUSED)
+{
+/* Not strictly a virCommand test, but this is the easiest place
+ * to test this lower-level interface.  It takes a double fork to
+ * test virProcessExitWithStatus.  */
+int ret = -1;
+int status = -1;
+pid_t pid;
+
+if (virFork(pid)  0)
+goto cleanup;
+if (pid  0)
+goto cleanup;
+if (pid == 0) {
+if (virFork(pid)  0)
+_exit(EXIT_FAILURE);
+if (pid == 0)
+_exit(42);
+if (virProcessWait(pid, status)  0)
+_exit(EXIT_FAILURE);
+virProcessExitWithStatus(status);
+_exit(EXIT_FAILURE);
+}
+
+if (virProcessWait(pid, status)  0)
+goto cleanup;
+if (!WIFEXITED(status) || WEXITSTATUS(status) != 42) {
+printf(Unexpected status %d\n, status);
+goto cleanup;
+}
+
+if (virFork(pid)  0)
+goto cleanup;
+if (pid  0)
+goto cleanup;
+if (pid == 0) {
+if (virFork(pid)  0)
+

[libvirt] [PATCH 01/10] nwfilter: don't ignore child process failures

2014-02-19 Thread Eric Blake
While auditing all callers of virCommandRun, I noticed that nwfilter
code never paid attention to commands with a non-zero status.
In the cases where status was captured, either the callers required
that the status was 0, or they discarded any failures from
virCommandRun.  Collecting status manually means that a non-zero
child exit status is not logged, but I could not see the benefit
in avoiding the logging in any command issued in the code.
Therefore, it was simpler to just nuke the wasted effort of
manually checking or ignoring non-zero status.

While at it, I also noticed that ebiptablesRemoveRules would
actually report success if the child process failed for a
reason other than non-zero status, such as OOM.

* src/nwfilter/nwfilter_ebiptables_driver.c (ebiptablesExecCLI):
Drop parameter.
(ebtablesApplyBasicRules, ebtablesApplyDHCPOnlyRules)
(ebtablesApplyDropAllRules, ebtablesCleanAll)
(ebiptablesApplyNewRules, ebiptablesTearNewRules)
(ebiptablesTearOldRules, ebiptablesAllTeardown)
(ebiptablesDriverInitWithFirewallD)
(ebiptablesDriverTestCLITools, ebiptablesDriverProbeStateMatch):
Adjust all clients.
(ebiptablesRemoveRules): Likewise, and fix return value on failure.

Signed-off-by: Eric Blake ebl...@redhat.com
---
 src/nwfilter/nwfilter_ebiptables_driver.c | 89 ---
 1 file changed, 35 insertions(+), 54 deletions(-)

diff --git a/src/nwfilter/nwfilter_ebiptables_driver.c 
b/src/nwfilter/nwfilter_ebiptables_driver.c
index dc651a2..002a844 100644
--- a/src/nwfilter/nwfilter_ebiptables_driver.c
+++ b/src/nwfilter/nwfilter_ebiptables_driver.c
@@ -1,7 +1,7 @@
 /*
  * nwfilter_ebiptables_driver.c: driver for ebtables/iptables on tap devices
  *
- * Copyright (C) 2011-2013 Red Hat, Inc.
+ * Copyright (C) 2011-2014 Red Hat, Inc.
  * Copyright (C) 2010-2012 IBM Corp.
  * Copyright (C) 2010-2012 Stefan Berger
  *
@@ -2799,8 +2799,6 @@ ebiptablesDisplayRuleInstance(void *_inst)
  * ebiptablesExecCLI:
  * @buf : pointer to virBuffer containing the string with the commands to
  *execute.
- * @status: Pointer to an integer for returning the WEXITSTATUS of the
- *commands executed via the script the was run.
  * @outbuf: Optional pointer to a string that will hold the buffer with
  *  output of the executed command. The actual buffer holding
  *  the message will be newly allocated by this function and
@@ -2815,15 +2813,11 @@ ebiptablesDisplayRuleInstance(void *_inst)
  * NULL, then the script must exit with status 0).
  */
 static int
-ebiptablesExecCLI(virBufferPtr buf,
-  int *status, char **outbuf)
+ebiptablesExecCLI(virBufferPtr buf, char **outbuf)
 {
 int rc = -1;
 virCommandPtr cmd;

-if (status)
- *status = 0;
-
 if (!virBufferError(buf)  !virBufferUse(buf))
 return 0;

@@ -2837,7 +2831,7 @@ ebiptablesExecCLI(virBufferPtr buf,

 virMutexLock(execCLIMutex);

-rc = virCommandRun(cmd, status);
+rc = virCommandRun(cmd, NULL);

 virMutexUnlock(execCLIMutex);

@@ -3293,7 +3287,7 @@ ebtablesApplyBasicRules(const char *ifname,
 ebtablesLinkTmpRootChain(buf, 1, ifname, 1);
 ebtablesRenameTmpRootChain(buf, 1, ifname);

-if (ebiptablesExecCLI(buf, NULL, NULL)  0)
+if (ebiptablesExecCLI(buf, NULL)  0)
 goto tear_down_tmpebchains;

 return 0;
@@ -3441,7 +3435,7 @@ ebtablesApplyDHCPOnlyRules(const char *ifname,
 ebtablesRenameTmpRootChain(buf, 0, ifname);
 }

-if (ebiptablesExecCLI(buf, NULL, NULL)  0)
+if (ebiptablesExecCLI(buf, NULL)  0)
 goto tear_down_tmpebchains;

 return 0;
@@ -3511,7 +3505,7 @@ ebtablesApplyDropAllRules(const char *ifname)
 ebtablesRenameTmpRootChain(buf, 1, ifname);
 ebtablesRenameTmpRootChain(buf, 0, ifname);

-if (ebiptablesExecCLI(buf, NULL, NULL)  0)
+if (ebiptablesExecCLI(buf, NULL)  0)
 goto tear_down_tmpebchains;

 return 0;
@@ -3537,7 +3531,6 @@ ebtablesRemoveBasicRules(const char *ifname)
 static int ebtablesCleanAll(const char *ifname)
 {
 virBuffer buf = VIR_BUFFER_INITIALIZER;
-int cli_status;

 if (!ebtables_cmd_path)
 return 0;
@@ -3556,7 +3549,7 @@ static int ebtablesCleanAll(const char *ifname)
 ebtablesRemoveTmpRootChain(buf, 1, ifname);
 ebtablesRemoveTmpRootChain(buf, 0, ifname);

-ebiptablesExecCLI(buf, cli_status, NULL);
+ebiptablesExecCLI(buf, NULL);
 return 0;
 }

@@ -3704,7 +3697,6 @@ ebiptablesApplyNewRules(const char *ifname,
 void **_inst)
 {
 size_t i, j;
-int cli_status;
 ebiptablesRuleInstPtr *inst = (ebiptablesRuleInstPtr *)_inst;
 virBuffer buf = VIR_BUFFER_INITIALIZER;
 virHashTablePtr chains_in_set  = virHashCreate(10, NULL);
@@ -3752,7 +3744,7 @@ ebiptablesApplyNewRules(const char *ifname,
 ebtablesRemoveTmpSubChains(buf, ifname);
 ebtablesRemoveTmpRootChain(buf, 1, ifname);
 ebtablesRemoveTmpRootChain(buf, 0, ifname);
-

[libvirt] [PATCH 02/10] virFork: give specific status on failure prior to exec

2014-02-19 Thread Eric Blake
When a child fails without exec'ing, we want a well-known status;
best is to match what env(1), nice(1), su(1), and other wrapper
programs do.  This patch adds enum values that later patches will
use, and sets up virFork as the first client of EXIT_CANCELED
for errors detected prior to even attempting exec, as well as
virExec to distinguish between a missing executable vs. a binary
that cannot be executed.

This is a slight semantic change in the unlikely case of a child
process failing to restore its signal mask - we now kill the
child with a known status instead of relying on the caller to
notice and do an appropriate _exit().  A subsequent patch will
make further cleanups based on an audit of all callers.

* src/internal.h (EXIT_CANCELED, EXIT_CANNOT_INVOKE)
(EXIT_ENOENT): New enum.
* src/util/vircommand.c (virFork): Document specific exit value if
child aborts early.
(virExec): Distinguish between various exec failures.
* tests/commandtest.c (test1): Enhance test.
(test22): New test.

Signed-off-by: Eric Blake ebl...@redhat.com
---
 src/internal.h|  7 +++
 src/util/vircommand.c | 15 +--
 tests/commandtest.c   | 43 +--
 3 files changed, 57 insertions(+), 8 deletions(-)

diff --git a/src/internal.h b/src/internal.h
index cef3da0..5a38448 100644
--- a/src/internal.h
+++ b/src/internal.h
@@ -438,5 +438,12 @@
   #NAME :  FMT, __VA_ARGS__);
 # endif

+/* Specific error values for use in forwarding programs such as
+ * virt-login-shell; these values match what GNU env does.  */
+enum {
+EXIT_CANCELED = 125, /* Failed before attempting exec */
+EXIT_CANNOT_INVOKE = 126, /* Exists but couldn't exec */
+EXIT_ENOENT = 127, /* Could not find program to exec */
+};

 #endif  /* __VIR_INTERNAL_H__ */
diff --git a/src/util/vircommand.c b/src/util/vircommand.c
index b137436..b9e5f37 100644
--- a/src/util/vircommand.c
+++ b/src/util/vircommand.c
@@ -1,7 +1,7 @@
 /*
  * vircommand.c: Child command execution
  *
- * Copyright (C) 2010-2013 Red Hat, Inc.
+ * Copyright (C) 2010-2014 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
@@ -199,7 +199,7 @@ virCommandFDSet(virCommandPtr cmd,
  * @pid - a pointer to a pid_t that will receive the return value from
  *fork()
  *
- * fork a new process while avoiding various race/deadlock conditions
+ * Wrapper around fork() that avoids various race/deadlock conditions.
  *
  * on return from virFork(), if *pid  0, the fork failed and there is
  * no new process. Otherwise, just like fork(), if *pid == 0, it is the
@@ -208,7 +208,7 @@ virCommandFDSet(virCommandPtr cmd,
  * Even if *pid = 0, if the return value from virFork() is  0, it
  * indicates a failure that occurred in the parent or child process
  * after the fork. In this case, the child process should call
- * _exit(EXIT_FAILURE) after doing any additional error reporting.
+ * _exit(EXIT_CANCELED) after doing any additional error reporting.
  */
 int
 virFork(pid_t *pid)
@@ -304,7 +304,8 @@ virFork(pid_t *pid)
 if (pthread_sigmask(SIG_SETMASK, newmask, NULL) != 0) {
 saved_errno = errno; /* save for caller */
 virReportSystemError(errno, %s, _(cannot unblock signals));
-goto cleanup;
+virDispatchError(NULL);
+_exit(EXIT_CANCELED);
 }
 ret = 0;
 }
@@ -518,6 +519,7 @@ virExec(virCommandPtr cmd)

 /* child */

+ret = EXIT_CANCELED;
 if (forkRet  0) {
 /* The fork was successful, but after that there was an error
  * in the child (which was already logged).
@@ -603,7 +605,7 @@ virExec(virCommandPtr cmd)
  cmd-pidfile, pid);
 goto fork_error;
 }
-_exit(0);
+_exit(EXIT_SUCCESS);
 }
 }

@@ -703,13 +705,14 @@ virExec(virCommandPtr cmd)
 else
 execv(binary, cmd-args);

+ret = errno == ENOENT ? EXIT_ENOENT : EXIT_CANNOT_INVOKE;
 virReportSystemError(errno,
  _(cannot execute binary %s),
  cmd-args[0]);

  fork_error:
 virDispatchError(NULL);
-_exit(EXIT_FAILURE);
+_exit(ret);

  cleanup:
 /* This is cleanup of parent process only - child
diff --git a/tests/commandtest.c b/tests/commandtest.c
index 2ae8871..042f049 100644
--- a/tests/commandtest.c
+++ b/tests/commandtest.c
@@ -1,7 +1,7 @@
 /*
  * commandtest.c: Test the libCommand API
  *
- * Copyright (C) 2010-2013 Red Hat, Inc.
+ * Copyright (C) 2010-2014 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
@@ -140,7 +140,7 @@ static int test1(const void *unused ATTRIBUTE_UNUSED)
 cmd = virCommandNew(abs_builddir /commandhelper-doesnotexist);
 if (virCommandRun(cmd, 

[libvirt] [PATCH 05/10] util: make it easier to grab only regular process exit

2014-02-19 Thread Eric Blake
Right now, a caller waiting for a child process either requires
the child to have status 0, or must use WIFEXITED() and friends
itself.  But in many cases, we want the middle ground of treating
fatal signals as an error, and directly accessing the normal exit
value without having to use WEXITSTATUS(), in order to easily
detect an expected non-zero exit status.  This adds the middle
ground to the low-level virProcessWait; the next patch will add
it to virCommand.

* src/util/virprocess.h (virProcessWait): Alter signature.
* src/util/virprocess.c (virProcessWait): Add parameter.
(virProcessRunInMountNamespace): Adjust caller.
* src/util/vircommand.c (virCommandWait): Likewise.
* src/util/virfile.c (virFileAccessibleAs): Likewise.
* src/lxc/lxc_container.c (lxcContainerHasReboot)
(lxcContainerAvailable): Likewise.
* daemon/libvirtd.c (daemonForkIntoBackground): Likewise.
* tools/virt-login-shell.c (main): Likewise.
* tools/virsh-domain.c (cmdLxcEnterNamespace): Likewise.
* tests/testutils.c (virtTestCaptureProgramOutput): Likewise.
* tests/commandtest.c (test23): Likewise.

Signed-off-by: Eric Blake ebl...@redhat.com
---
 daemon/libvirtd.c|  4 ++--
 src/lxc/lxc_container.c  |  6 +++---
 src/util/vircommand.c|  2 +-
 src/util/virfile.c   | 14 --
 src/util/virprocess.c| 46 ++
 src/util/virprocess.h|  2 +-
 tests/commandtest.c  |  8 
 tests/testutils.c|  4 ++--
 tools/virsh-domain.c |  6 +++---
 tools/virt-login-shell.c |  4 ++--
 10 files changed, 52 insertions(+), 44 deletions(-)

diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c
index b27c6fd..72f0e81 100644
--- a/daemon/libvirtd.c
+++ b/daemon/libvirtd.c
@@ -1,7 +1,7 @@
 /*
  * libvirtd.c: daemon start of day, guest process  i/o management
  *
- * Copyright (C) 2006-2012 Red Hat, Inc.
+ * Copyright (C) 2006-2014 Red Hat, Inc.
  * Copyright (C) 2006 Daniel P. Berrange
  *
  * This library is free software; you can redistribute it and/or
@@ -206,7 +206,7 @@ static int daemonForkIntoBackground(const char *argv0)
 VIR_FORCE_CLOSE(statuspipe[1]);

 /* We wait to make sure the first child forked successfully */
-if (virProcessWait(pid, NULL)  0)
+if (virProcessWait(pid, NULL, false)  0)
 goto error;

 /* If we get here, then the grandchild was spawned, so we
diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c
index f08dbc2..c63a470 100644
--- a/src/lxc/lxc_container.c
+++ b/src/lxc/lxc_container.c
@@ -173,11 +173,11 @@ int lxcContainerHasReboot(void)
 virReportSystemError(errno, %s,
  _(Unable to clone to check reboot support));
 return -1;
-} else if (virProcessWait(cpid, status)  0) {
+} else if (virProcessWait(cpid, status, false)  0) {
 return -1;
 }

-if (WEXITSTATUS(status) != 1) {
+if (status != 1) {
 VIR_DEBUG(Containerized reboot support is missing 
   (kernel probably too old  3.4));
 return 0;
@@ -2075,7 +2075,7 @@ int lxcContainerAvailable(int features)
 VIR_DEBUG(clone call returned %s, container support is not enabled,
   virStrerror(errno, ebuf, sizeof(ebuf)));
 return -1;
-} else if (virProcessWait(cpid, NULL)  0) {
+} else if (virProcessWait(cpid, NULL, false)  0) {
 return -1;
 }

diff --git a/src/util/vircommand.c b/src/util/vircommand.c
index b9e5f37..a4397b4 100644
--- a/src/util/vircommand.c
+++ b/src/util/vircommand.c
@@ -2372,7 +2372,7 @@ virCommandWait(virCommandPtr cmd, int *exitstatus)
  * message is not as detailed as what we can provide.  So, we
  * guarantee that virProcessWait only fails due to failure to wait,
  * and repeat the exitstatus check code ourselves.  */
-ret = virProcessWait(cmd-pid, exitstatus ? exitstatus : status);
+ret = virProcessWait(cmd-pid, exitstatus ? exitstatus : status, true);
 if (cmd-flags  VIR_EXEC_ASYNC_IO) {
 cmd-flags = ~VIR_EXEC_ASYNC_IO;
 virThreadJoin(cmd-asyncioThread);
diff --git a/src/util/virfile.c b/src/util/virfile.c
index 96f078d..6fb7d6f 100644
--- a/src/util/virfile.c
+++ b/src/util/virfile.c
@@ -1,7 +1,7 @@
 /*
  * virfile.c: safer file handling
  *
- * Copyright (C) 2010-2013 Red Hat, Inc.
+ * Copyright (C) 2010-2014 Red Hat, Inc.
  * Copyright (C) 2010 IBM Corporation
  * Copyright (C) 2010 Stefan Berger
  * Copyright (C) 2010 Eric Blake
@@ -1739,19 +1739,13 @@ virFileAccessibleAs(const char *path, int mode,

 if (pid) { /* parent */
 VIR_FREE(groups);
-if (virProcessWait(pid, status)  0) {
-/* virProcessWait() already
- * reported error */
-return -1;
-}
-
-if (!WIFEXITED(status)) {
-errno = EINTR;
+if (virProcessWait(pid, status, false)  0) {
+/* virProcessWait() already reported error */
 

Re: [libvirt] [PATCH] libxl: always use libxlVmCleanupJob in shutdown thread

2014-02-19 Thread Eric Blake
On 02/19/2014 07:54 PM, Jim Fehlig wrote:
 Commit e4a0e900 missed calling libxlVmCleanupJob in the shutdown
 handler when processing a reboot event.
 
 Signed-off-by: Jim Fehlig jfeh...@suse.com
 ---
  src/libxl/libxl_driver.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

ACK

 
 diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
 index f7ca91c..9f87e71 100644
 --- a/src/libxl/libxl_driver.c
 +++ b/src/libxl/libxl_driver.c
 @@ -393,7 +393,7 @@ libxlDomainShutdownThread(void *opaque)
  break;
  case LIBXL_SHUTDOWN_REASON_REBOOT:
  libxl_domain_destroy(ctx, vm-def-id, NULL);
 -libxlVmCleanup(driver, vm, VIR_DOMAIN_SHUTOFF_SHUTDOWN);
 +libxlVmCleanupJob(driver, vm, VIR_DOMAIN_SHUTOFF_SHUTDOWN);
  libxlVmStart(driver, vm, 0, -1);
  break;
  default:
 

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



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

[libvirt] [PATCH 10/10] virsh: report exit status of failed lxc-enter-namespace

2014-02-19 Thread Eric Blake
'virsh lxc-enter-namespace' does not have a way to reflect exit
status to the caller in single-command mode, but we might as well
at least report the exit status.  Prior to this patch,

$ virsh -c lxc:/// lxc-enter-namespace shell /bin/sh 'exit 3'; echo $?
1

now it gives some details:

$ virsh -c lxc:/// lxc-enter-namespace shell /bin/sh -c 'exit 3'; echo $?
error: internal error: Child process (31557) unexpected exit status 3
1

Also useful:

$ virsh -c lxc:/// lxc-enter-namespace shell /bin/sh -c 'kill $$'; echo $?
error: internal error: Child process (31585) unexpected fatal signal 15
1

* tools/virsh-domain.c (cmdLxcEnterNamespace): Avoid magic numbers.
Dispatch any error.
* tools/virsh.pod: Document that non-zero exit status is collapsed.

Signed-off-by: Eric Blake ebl...@redhat.com
---
 tools/virsh-domain.c | 21 -
 tools/virsh.pod  |  5 +++--
 2 files changed, 15 insertions(+), 11 deletions(-)

diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 4400d18..f9d85c3 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -8204,12 +8204,14 @@ cmdLxcEnterNamespace(vshControl *ctl, const vshCmd *cmd)
 if ((pid = virFork())  0)
 goto cleanup;
 if (pid == 0) {
+int status;
+
 if (setlabel 
 virDomainLxcEnterSecurityLabel(secmodel,
seclabel,
NULL,
0)  0)
-_exit(255);
+_exit(EXIT_CANCELED);

 if (virDomainLxcEnterNamespace(dom,
nfdlist,
@@ -8217,27 +8219,28 @@ cmdLxcEnterNamespace(vshControl *ctl, const vshCmd *cmd)
NULL,
NULL,
0)  0)
-_exit(255);
+_exit(EXIT_CANCELED);

 /* Fork a second time because entering the
  * pid namespace only takes effect after fork
  */
 if ((pid = virFork())  0)
-_exit(255);
+_exit(EXIT_CANCELED);
 if (pid == 0) {
 execv(cmdargv[0], cmdargv);
-_exit(255);
-} else {
-if (virProcessWait(pid, NULL, false)  0)
-_exit(255);
+_exit(errno == ENOENT ? EXIT_ENOENT : EXIT_CANNOT_INVOKE);
 }
-_exit(0);
+if (virProcessWait(pid, status, true)  0)
+_exit(EXIT_CANNOT_INVOKE);
+virProcessExitWithStatus(status);
 } else {
 for (i = 0; i  nfdlist; i++)
 VIR_FORCE_CLOSE(fdlist[i]);
 VIR_FREE(fdlist);
-if (virProcessWait(pid, NULL, false)  0)
+if (virProcessWait(pid, NULL, false)  0) {
+vshReportError(ctl);
 goto cleanup;
+}
 }

 ret = true;
diff --git a/tools/virsh.pod b/tools/virsh.pod
index f221475..49e1f63 100644
--- a/tools/virsh.pod
+++ b/tools/virsh.pod
@@ -3332,7 +3332,8 @@ Enter the namespace of Idomain and execute the command 
C/path/to/binary
 passing the requested args. The binary path is relative to the container
 root filesystem, not the host root filesystem. The binary will inherit the
 environment variables / console visible to virsh. This command only works
-when connected to the LXC hypervisor driver.
+when connected to the LXC hypervisor driver.  This command succeeds only
+if C/path/to/binary has 0 exit status.

 =back

@@ -3447,7 +3448,7 @@ Alternatively report bugs to your software distributor / 
vendor.

 =head1 COPYRIGHT

-Copyright (C) 2005, 2007-2010 Red Hat, Inc., and the authors listed in the
+Copyright (C) 2005, 2007-2014 Red Hat, Inc., and the authors listed in the
 libvirt AUTHORS file.

 =head1 LICENSE
-- 
1.8.5.3

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


[libvirt] [PATCH 07/10] virFork: simplify semantics

2014-02-19 Thread Eric Blake
The old semantics of virFork() violates the priciple of good
usability: it requires the caller to check the pid argument
after use, *even when virFork returned -1*, in order to properly
abort a child process that failed setup done immediately after
fork() - that is, the caller must call _exit() in the child.
While uses in virfile.c did this correctly, uses in 'virsh
lxc-enter-namespace' and 'virt-login-shell' would happily return
from the calling function in both the child and the parent,
leading to very confusing results. [Thankfully, I found the
problem by inspection, and can't actually trigger the double
return on error without an LD_PRELOAD library.]

It is much better if the semantics of virFork are impossible
to abuse.  Looking at virFork(), the parent could only ever
return -1 with a non-negative pid if it misused pthread_sigmask,
but this never happens.  Up until this patch series, the child
could return -1 with non-negative pid if it fails to set up
signals correctly, but we recently fixed that to make the child
call _exit() at that point instead of forcing the caller to do
it.  Thus, the return value and contents of the pid argument are
now redundant (a -1 return now happens only for failure to fork,
a child 0 return only happens for a successful 0 pid, and a
parent 0 return only happens for a successful non-zero pid),
so we might as well return the pid directly rather than an
integer of whether it succeeded or failed; this is also good
from the interface design perspective as users are already
familiar with fork() semantics.

One last change in this patch: before returning the pid directly,
I found cases where using virProcessWait unconditionally on a
cleanup path of a virFork's -1 pid return would be nicer if there
were a way to avoid it overwriting an earlier message.  While
such paths are a bit harder to come by with my change to a direct
pid return, I decided to keep the virProcessWait change in this
patch.

* src/util/vircommand.h (virFork): Change signature.
* src/util/vircommand.c (virFork): Guarantee that child will only
return on success, to simplify callers.  Return pid rather than
status, now that the situations are always the same.
(virExec): Adjust caller, also avoid open-coding process death.
* src/util/virprocess.c (virProcessWait): Tweak semantics when pid
is -1.
(virProcessRunInMountNamespace): Adjust caller.
* src/util/virfile.c (virFileAccessibleAs, virFileOpenForked)
(virDirCreate): Likewise.
* tools/virt-login-shell.c (main): Likewise.
* tools/virsh-domain.c (cmdLxcEnterNamespace): Likewise.
* tests/commandtest.c (test23): Likewise.

Signed-off-by: Eric Blake ebl...@redhat.com
---
 src/util/vircommand.c| 120 ++-
 src/util/vircommand.h|   2 +-
 src/util/virfile.c   |  23 ++---
 src/util/virprocess.c|  31 ++--
 tests/commandtest.c  |  12 ++---
 tools/virsh-domain.c |   7 +--
 tools/virt-login-shell.c |  12 +++--
 7 files changed, 81 insertions(+), 126 deletions(-)

diff --git a/src/util/vircommand.c b/src/util/vircommand.c
index 415b8c3..db4166f 100644
--- a/src/util/vircommand.c
+++ b/src/util/vircommand.c
@@ -197,28 +197,28 @@ virCommandFDSet(virCommandPtr cmd,

 /**
  * virFork:
- * @pid - a pointer to a pid_t that will receive the return value from
- *fork()
  *
  * Wrapper around fork() that avoids various race/deadlock conditions.
  *
- * on return from virFork(), if *pid  0, the fork failed and there is
- * no new process. Otherwise, just like fork(), if *pid == 0, it is the
- * child process returning, and if *pid  0, it is the parent.
- *
- * Even if *pid = 0, if the return value from virFork() is  0, it
- * indicates a failure that occurred in the parent or child process
- * after the fork. In this case, the child process should call
- * _exit(EXIT_CANCELED) after doing any additional error reporting.
+ * Like fork(), there are several return possibilities:
+ * 1. No child was created: the return is -1, errno is set, and an error
+ * message has been reported.  The semantics of virWaitProcess() recognize
+ * this to avoid clobbering the error message from here.
+ * 2. This is the parent: the return is  0.  The parent can now attempt
+ * to interact with the child (but be aware that unlike raw fork(), the
+ * child may not return - some failures in the child result in this
+ * function calling _exit(EXIT_CANCELED) if the child cannot be set up
+ * correctly).
+ * 3. This is the child: the return is 0.  If this happens, the parent
+ * is also guaranteed to return.
  */
-int
-virFork(pid_t *pid)
+pid_t
+virFork(void)
 {
 sigset_t oldmask, newmask;
 struct sigaction sig_action;
-int saved_errno, ret = -1;
-
-*pid = -1;
+int saved_errno;
+pid_t pid;

 /*
  * Need to block signals now, so that child process can safely
@@ -226,54 +226,47 @@ virFork(pid_t *pid)
  */
 sigfillset(newmask);
 if (pthread_sigmask(SIG_SETMASK, newmask, oldmask) != 0) 

[libvirt] [PATCH 04/10] util: preserve exit status from mount namespace callback

2014-02-19 Thread Eric Blake
The documentation of namespace callbacks was inconsistent on whether
it preserved positive return values.  Now that we have a dedicated
EXIT_CANCELED to flag all errors before getting to the callback,
it is possible to use positive return values (not that any of the
current callers do, but it better matches the docs).

Also, while vircommand.c is careful to close fds that a child should
not have, it's still better to be in the practice of setting
FD_CLOEXEC up front.

* src/util/virprocess.c (virProcessRunInMountNamespace): Tweak
return value to pass back non-zero status.  Avoid leaking pipe fds
to other threads.
* src/util/virprocess.h: Fix comment.

Signed-off-by: Eric Blake ebl...@redhat.com
---
 src/util/virprocess.c | 19 +++
 src/util/virprocess.h |  2 +-
 2 files changed, 12 insertions(+), 9 deletions(-)

diff --git a/src/util/virprocess.c b/src/util/virprocess.c
index 68c4c14..bd406db 100644
--- a/src/util/virprocess.c
+++ b/src/util/virprocess.c
@@ -923,9 +923,9 @@ static int virProcessNamespaceHelper(int errfd,

 /* Run cb(opaque) in the mount namespace of pid.  Return -1 with error
  * message raised if we fail to run the child, if the child dies from
- * a signal, or if the child has status 1; otherwise return the exit
- * status of the child. The callback will be run in a child process
- * so must be careful to only use async signal safe functions.
+ * a signal, or if the child has status EXIT_CANCELED; otherwise return
+ * the exit status of the child. The callback will be run in a child
+ * process so must be careful to only use async signal safe functions.
  */
 int
 virProcessRunInMountNamespace(pid_t pid,
@@ -936,7 +936,7 @@ virProcessRunInMountNamespace(pid_t pid,
 pid_t child = -1;
 int errfd[2] = { -1, -1 };

-if (pipe(errfd)  0) {
+if (pipe2(errfd, O_CLOEXEC)  0) {
 virReportSystemError(errno, %s,
  _(Cannot create pipe for child));
 return -1;
@@ -946,7 +946,7 @@ virProcessRunInMountNamespace(pid_t pid,

 if (ret  0 || child  0) {
 if (child == 0)
-_exit(1);
+_exit(EXIT_CANCELED);

 /* parent */
 virProcessAbort(child);
@@ -958,13 +958,16 @@ virProcessRunInMountNamespace(pid_t pid,
 ret = virProcessNamespaceHelper(errfd[1], pid,
 cb, opaque);
 VIR_FORCE_CLOSE(errfd[1]);
-_exit(ret  0 ? 1 : 0);
+_exit(ret  0 ? EXIT_CANCELED : ret);
 } else {
 char *buf = NULL;
-VIR_FORCE_CLOSE(errfd[1]);
+int status;

+VIR_FORCE_CLOSE(errfd[1]);
 ignore_value(virFileReadHeaderFD(errfd[0], 1024, buf));
-ret = virProcessWait(child, NULL);
+ret = virProcessWait(child, status);
+if (!ret)
+ret = status == EXIT_CANCELED ? -1 : status;
 VIR_FREE(buf);
 }

diff --git a/src/util/virprocess.h b/src/util/virprocess.h
index b96dbd4..abe3635 100644
--- a/src/util/virprocess.h
+++ b/src/util/virprocess.h
@@ -67,7 +67,7 @@ int virProcessSetMaxFiles(pid_t pid, unsigned int files);
  * pid.  This function must use only async-signal-safe functions, as
  * it gets run after a fork of a multi-threaded process.  The return
  * value of this function is passed to _exit(), except that a
- * negative value is treated as an error.  */
+ * negative value is treated as EXIT_CANCELED.  */
 typedef int (*virProcessNamespaceCallback)(pid_t pid, void *opaque);

 int virProcessRunInMountNamespace(pid_t pid,
-- 
1.8.5.3

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


[libvirt] [PATCH 09/10] virt-login-shell: saner exit value

2014-02-19 Thread Eric Blake
virt-login-shell was exiting with status 0, regardless of what the
wrapped shell returned.  This is unkind to users; we should behave
more like env(1), nice(1), su(1), and other wrapper programs, by
preserving the invoked application's status (which includes the
distinction between death due to signal vs. normal death).

* tools/virt-login-shell.c (main): Pass through child exit status.
* tools/virt-login-shell.pod: Document exit status.

Signed-off-by: Eric Blake ebl...@redhat.com
---
 tools/virt-login-shell.c   | 35 +++
 tools/virt-login-shell.pod | 25 +++--
 2 files changed, 42 insertions(+), 18 deletions(-)

diff --git a/tools/virt-login-shell.c b/tools/virt-login-shell.c
index 75a5223..3ea7ade 100644
--- a/tools/virt-login-shell.c
+++ b/tools/virt-login-shell.c
@@ -21,13 +21,14 @@
  */
 #include config.h

-#include stdarg.h
-#include getopt.h
-#include stdio.h
 #include errno.h
-#include stdlib.h
 #include fnmatch.h
+#include getopt.h
 #include locale.h
+#include signal.h
+#include stdarg.h
+#include stdio.h
+#include stdlib.h

 #include internal.h
 #include virerror.h
@@ -168,8 +169,8 @@ main(int argc, char **argv)
 {
 virConfPtr conf = NULL;
 const char *login_shell_path = conf_file;
-pid_t cpid;
-int ret = EXIT_FAILURE;
+pid_t cpid = -1;
+int ret = EXIT_CANCELED;
 int status;
 uid_t uid = getuid();
 gid_t gid = getgid();
@@ -195,8 +196,8 @@ main(int argc, char **argv)
 {NULL, 0, NULL, 0}
 };
 if (virInitialize()  0) {
-fprintf(stderr, _(Failed to initialize libvirt Error Handling));
-return EXIT_FAILURE;
+fprintf(stderr, _(Failed to initialize libvirt error handling));
+return EXIT_CANCELED;
 }

 setenv(PATH, /bin:/usr/bin, 1);
@@ -231,7 +232,7 @@ main(int argc, char **argv)
 case '?':
 default:
 usage();
-exit(EXIT_FAILURE);
+exit(EXIT_CANCELED);
 }
 }

@@ -330,15 +331,13 @@ main(int argc, char **argv)
 if (execv(shargv[0], (char *const*) shargv)  0) {
 virReportSystemError(errno, _(Unable to exec shell %s),
  shargv[0]);
-return EXIT_FAILURE;
+virDispatchError(NULL);
+return errno == ENOENT ? EXIT_ENOENT : EXIT_CANNOT_INVOKE;
 }
-return EXIT_SUCCESS;
 }

-if (virProcessWait(cpid, status, true)  0)
-goto cleanup;
-ret = EXIT_SUCCESS;
-
+/* At this point, the parent is now waiting for the child to exit,
+ * but as that may take a long time, we release resources now.  */
 cleanup:
 for (i = 0; i  nfdlist; i++)
 VIR_FORCE_CLOSE(fdlist[i]);
@@ -354,7 +353,11 @@ cleanup:
 VIR_FREE(seclabel);
 VIR_FREE(secmodel);
 VIR_FREE(groups);
-if (ret)
+
+if (virProcessWait(cpid, status, true) == 0)
+virProcessExitWithStatus(status);
+
+if (virGetLastError())
 virDispatchError(NULL);
 return ret;
 }
diff --git a/tools/virt-login-shell.pod b/tools/virt-login-shell.pod
index bcd7855..56861f7 100644
--- a/tools/virt-login-shell.pod
+++ b/tools/virt-login-shell.pod
@@ -4,7 +4,7 @@ virt-login-shell - tool to execute a shell within a container 
matching the users

 =head1 SYNOPSIS

-Bvirt-login-shell
+Bvirt-login-shell [IOPTION]

 =head1 DESCRIPTION

@@ -47,6 +47,27 @@ variable in /etc/libvirt/virt-login-shell.conf.

 eg. allowed_users = [ tom, dick, harry ]

+=head1 EXIT STATUS
+
+Bvirt-login-shell normally returns the exit status of the command it
+executed. If the command was killed by a signal, but that signal is not
+fatal to virt-login-shell, then it returns the signal number plus 128.
+
+Exit status generated by Bvirt-login-shell itself:
+
+=over 4
+
+=item B0 An option was used to learn more about this binary.
+
+=item B125 Generic error before attempting execution of the configured
+shell; for example, if libvirtd is not running.
+
+=item B126 The configured shell exists but could not be executed.
+
+=item B127 The configured shell could not be found.
+
+=back
+
 =head1 BUGS

 Report any bugs discovered to the libvirt community via the mailing
@@ -61,7 +82,7 @@ Alternatively report bugs to your software distributor / 
vendor.

 =head1 COPYRIGHT

-Copyright (C) 2013 Red Hat, Inc., and the authors listed in the
+Copyright (C) 2013-2014 Red Hat, Inc., and the authors listed in the
 libvirt AUTHORS file.

 =head1 LICENSE
-- 
1.8.5.3

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


[libvirt] [PATCH 08/10] virt-login-shell: use single instead of double fork

2014-02-19 Thread Eric Blake
Note that 'virsh lxc-enter-namespace' must double-fork, for two
reasons: some namespaces can only be done from a single thread,
while virsh is multithreaded; and because virsh can be run in
batch mode where we must not corrupt the namespace of that
execution upon return from the subsidiary command.

When virt-login-shell was first written, it blindly copied from
'virsh lxc-enter-namespace', including the double-fork.  But
neither of the reasons for double forking apply to
virt-login-shell (we are single-threaded, and we have nothing to
do after the child completes that would require us to preserve a
namespace), so we can simplify life by using a single fork.
In turn, this will make it easier for a future patch to pass the
child's exit status on to the invoking shell.

In flattening to a single fork, note that closing the fds must
be done after fork, because the parent process still needs to
use fds to control the virConnectPtr; meanwhile, chdir can be
done prior to forking (in fact, it's easier to report errors
on anything attempted before forking).

* tools/virt-login-shell.c (main): Single rather than double fork.
(virLoginShellFini): Delete, by inlining actions instead.

Signed-off-by: Eric Blake ebl...@redhat.com
---
 tools/virt-login-shell.c | 116 ++-
 1 file changed, 43 insertions(+), 73 deletions(-)

diff --git a/tools/virt-login-shell.c b/tools/virt-login-shell.c
index 666facf..75a5223 100644
--- a/tools/virt-login-shell.c
+++ b/tools/virt-login-shell.c
@@ -41,24 +41,8 @@
 #include vircommand.h
 #define VIR_FROM_THIS VIR_FROM_NONE

-static ssize_t nfdlist;
-static int *fdlist;
 static const char *conf_file = SYSCONFDIR /libvirt/virt-login-shell.conf;

-static void virLoginShellFini(virConnectPtr conn, virDomainPtr dom)
-{
-size_t i;
-
-for (i = 0; i  nfdlist; i++)
-VIR_FORCE_CLOSE(fdlist[i]);
-VIR_FREE(fdlist);
-nfdlist = 0;
-if (dom)
-virDomainFree(dom);
-if (conn)
-virConnectClose(conn);
-}
-
 static int virLoginShellAllowedUser(virConfPtr conf,
 const char *name,
 gid_t *groups)
@@ -187,7 +171,6 @@ main(int argc, char **argv)
 pid_t cpid;
 int ret = EXIT_FAILURE;
 int status;
-int status2;
 uid_t uid = getuid();
 gid_t gid = getgid();
 char *name = NULL;
@@ -201,6 +184,10 @@ main(int argc, char **argv)
 int longindex = -1;
 int ngroups;
 gid_t *groups = NULL;
+ssize_t nfdlist = 0;
+int *fdlist = NULL;
+int openmax;
+size_t i;

 struct option opt[] = {
 {help, no_argument, NULL, 'h'},
@@ -298,6 +285,13 @@ main(int argc, char **argv)
 }
 }

+openmax = sysconf(_SC_OPEN_MAX);
+if (openmax  0) {
+virReportSystemError(errno,  %s,
+ _(sysconf(_SC_OPEN_MAX) failed));
+goto cleanup;
+}
+
 if ((nfdlist = virDomainLxcOpenNamespace(dom, fdlist, 0))  0)
 goto cleanup;
 if (VIR_ALLOC(secmodel)  0)
@@ -308,76 +302,52 @@ main(int argc, char **argv)
 goto cleanup;
 if (virDomainGetSecurityLabel(dom, seclabel)  0)
 goto cleanup;
+if (virSetUIDGID(0, 0, NULL, 0)  0)
+goto cleanup;
+if (virDomainLxcEnterSecurityLabel(secmodel, seclabel, NULL, 0)  0)
+goto cleanup;
+if (nfdlist  0 
+virDomainLxcEnterNamespace(dom, nfdlist, fdlist, NULL, NULL, 0)  0)
+goto cleanup;
+if (virSetUIDGID(uid, gid, groups, ngroups)  0)
+goto cleanup;
+if (chdir(homedir)  0) {
+virReportSystemError(errno, _(Unable to chdir(%s)), homedir);
+goto cleanup;
+}

-/* Fork once because we don't want to affect virt-login-shell's
- * namespace itself.  FIXME: is this necessary?
- */
+/* A fork is required to create new process in correct pid namespace.  */
 if ((cpid = virFork())  0)
 goto cleanup;

 if (cpid == 0) {
-pid_t ccpid;
-
-int openmax = sysconf(_SC_OPEN_MAX);
-int fd;
+int tmpfd;

-if (virSetUIDGID(0, 0, NULL, 0)  0)
-return EXIT_FAILURE;
-
-if (virDomainLxcEnterSecurityLabel(secmodel,
-   seclabel,
-   NULL,
-   0)  0)
-return EXIT_FAILURE;
-
-if (nfdlist  0) {
-if (virDomainLxcEnterNamespace(dom,
-   nfdlist,
-   fdlist,
-   NULL,
-   NULL,
-   0)  0)
-return EXIT_FAILURE;
-}
-
-ret = virSetUIDGID(uid, gid, groups, ngroups);
-VIR_FREE(groups);
-if (ret  0)
-return EXIT_FAILURE;
-
-if (openmax  0) {
-

Re: [libvirt] [PATCH v10] bhyve: add a basic driver

2014-02-19 Thread Roman Bogorodskiy
  Daniel P. Berrange wrote:

 ACK, and pushed to GIT.
 
 Great job getting this new driver up  running, and welcome to
 the libvirt committers team.

Superb! Thanks!

Roman Bogorodskiy

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


Re: [libvirt] [PATCH v12 04/11] qemu_hostdev: parse BACKEND_DEFAULT outside

2014-02-19 Thread Chunyan Liu
2014-02-17 23:26 GMT+08:00 Laine Stump la...@laine.org:

 On 02/17/2014 11:38 AM, Cedric Bosdonnat wrote:
  On Mon, 2014-02-17 at 14:32 +0800, Chunyan Liu wrote:
  For extracting hostdev codes from qemu_hostdev.c to common library,
 change
  original paring VIR_DOMAIN_HOSTDEV_PCI_BACKEND_DEFAULT in hostdev
 function to
  qemuDomainDeviceDefPostParse.
  typo: paring - parsing.
 
  Signed-off-by: Chunyan Liu cy...@suse.com
  ---
   src/qemu/qemu_domain.c |   22
 +++
   src/qemu/qemu_hostdev.c|   28
 +++-
   src/qemu/qemu_hostdev.h|2 -
   src/qemu/qemu_hotplug.c|2 +-
   src/qemu/qemu_process.c|3 +-
   .../qemuxml2argv-hostdev-pci-address.xml   |1 +
   .../qemuxml2argvdata/qemuxml2argv-net-hostdev.xml  |1 +
   tests/qemuxml2argvdata/qemuxml2argv-pci-rom.xml|2 +
   8 files changed, 32 insertions(+), 29 deletions(-)
 
  diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
  index a665061..55e707e 100644
  --- a/src/qemu/qemu_domain.c
  +++ b/src/qemu/qemu_domain.c
  @@ -38,6 +38,7 @@
   #include virtime.h
   #include virstoragefile.h
   #include virstring.h
  +#include qemu_hostdev.h
 
   #include sys/time.h
   #include fcntl.h
  @@ -821,6 +822,7 @@ qemuDomainDeviceDefPostParse(virDomainDeviceDefPtr
 dev,
   int ret = -1;
   virQEMUDriverPtr driver = opaque;
   virQEMUDriverConfigPtr cfg = NULL;
  +virQEMUCapsPtr qemuCaps = NULL;
 
   if (dev-type == VIR_DOMAIN_DEVICE_NET 
   dev-data.net-type != VIR_DOMAIN_NET_TYPE_HOSTDEV 
  @@ -899,6 +901,26 @@ qemuDomainDeviceDefPostParse(virDomainDeviceDefPtr
 dev,
   dev-data.chr-source.data.nix.listen = true;
   }
 
  +if (dev-type == VIR_DOMAIN_DEVICE_HOSTDEV) {
  +virDomainHostdevDefPtr hostdev = dev-data.hostdev;
  +if (hostdev-mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS 
  +hostdev-source.subsys.type ==
 VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI 
  +hostdev-source.subsys.u.pci.backend ==
 VIR_DOMAIN_HOSTDEV_PCI_BACKEND_DEFAULT) {
  +
  +hostdev-source.subsys.u.pci.backend =
 VIR_DOMAIN_HOSTDEV_PCI_BACKEND_KVM;
  +if (driver  driver-qemuCapsCache) {
  +bool supportsPassthroughVFIO =
 qemuHostdevHostSupportsPassthroughVFIO();
  +qemuCaps =
 virQEMUCapsCacheLookupCopy(driver-qemuCapsCache,
  +  def-emulator);
  +if (supportsPassthroughVFIO  qemuCaps 
  +virQEMUCapsGet(qemuCaps,
 QEMU_CAPS_DEVICE_VFIO_PCI))
  +hostdev-source.subsys.u.pci.backend =
 VIR_DOMAIN_HOSTDEV_PCI_BACKEND_VFIO;
  +
  +virObjectUnref(qemuCaps);
  +}
  +}
  +}
  +
   ret = 0;
  KVM passthrough support isn't checked here but was checked in the
  removed code, is that intended?

 The fact that the code doing the KVM check is fairly new, suggests to me
 that this omission *wasn't* intentional. I'm really glad that you're
 looking at that in detail, because it's the potential omission of recent
 bugfixes that has me nervous.

 Beyond that, this isn't the proper place to be moving this to - anything
 that is done by the PostParse callback function ends up being written to
 persistent config and is there effectively forever, but the
 interpretation of the hostdev driver default backend is something that
 must be re-done exactly at the moment the device it going to be
 attached. This patch instead causes that decision to be made when the
 domain is defined, and forever encoded in the config.

 I think that instead you need to either call it from
 qemuPrepareHostDevices() and qemuDomainAttachHostPciDevice(), OR send a
 callback function pointer into your new virHostdevPreparePciHostdevs(),
 with that function returning the backend to use this time based on the
 config setting and current system state.


Thanks for explanation. Will correct that.



   cleanup:
  diff --git a/src/qemu/qemu_hostdev.c b/src/qemu/qemu_hostdev.c
  index ce5012d..80552cd 100644
  --- a/src/qemu/qemu_hostdev.c
  +++ b/src/qemu/qemu_hostdev.c
  @@ -583,8 +583,7 @@ qemuHostdevHostSupportsPassthroughLegacy(void)
 
   static bool
   qemuPrepareHostdevPCICheckSupport(virDomainHostdevDefPtr *hostdevs,
  -  size_t nhostdevs,
  -  virQEMUCapsPtr qemuCaps)
  +  size_t nhostdevs)
   {
   bool supportsPassthroughKVM =
 qemuHostdevHostSupportsPassthroughLegacy();
   bool supportsPassthroughVFIO =
 qemuHostdevHostSupportsPassthroughVFIO();
  @@ -601,23 +600,6 @@
 qemuPrepareHostdevPCICheckSupport(virDomainHostdevDefPtr *hostdevs,
   continue;
 
   switch ((virDomainHostdevSubsysPciBackendType) *backend) {
  -case