Re: [libvirt] [PATCH 0/4] qemu: export disk snapshot capability
- 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
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
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?
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
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
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?
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
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
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?
[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
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
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
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
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
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
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?
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
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?
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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}
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
--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
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
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
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
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
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
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
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
'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
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
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
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
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
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-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