[libvirt] xen live migration with libvirt
The format of live migration command is virsh migrate --live domain xen:/// xenmigr://remotehost or another? My source and destination is rhel5.4 and xen is 3.1.0, libvirt is 0.8.2. I configured a shared storage between source and destination and modified /etc/xen/xend-config.sxp. I test the command xm migrate --live domain remotehost and it's going well. But when I use virsh migrate --live domain xen:/// xenmigr://remotehost, a error appeared. error:post operation failed:xend_post:error from xen daemon:(xend.err '/usr/lib64/xen/bin/xc_save 23 2 0 0 5 failed') Regards, Best wishes. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/3] Remove wrong check for uml monitor response size
On Sun, Aug 15, 2010 at 11:08:43PM +0200, Soren Hansen wrote:
Unless multiple threads are reading from the fd (?), I'm quite
confident in my data. I dumped the whole monitor_response buffer right
before and right after the recvfrom call along with the return value
of recvfrom. I'll rerun the tests tomorrow and show you the results.
I'm running this on another kernel right now and I'm not seeing the
problem. I'll try again with the kernel I used a couple of days ago.
I'm not sure if I pointed this out in my initial e-mail, but even if I
didn't have this recvfrom problem, the check seems odd to my eye. Is
the monitor really going to send a max sized datagram each time? I
would have expected it to only send a datagram the size of the header
+ the length of the data.
My hunch here seems correct though.
I've applied this diff:
diff --git a/src/uml/uml_driver.c b/src/uml/uml_driver.c
index 9cf669f..c289ad3 100644
--- a/src/uml/uml_driver.c
+++ b/src/uml/uml_driver.c
@@ -729,8 +733,13 @@ static int umlMonitorCommand(const struct uml_driver
*driver,
do {
ssize_t nbytes;
addrlen = sizeof(addr);
+VIR_DEBUG(res.error: %d, res.extra: %d, res.length: %d, res.data:
%.*s,
+ res.error, res.extra, res.length, res.length,
res.data);
nbytes = recvfrom(priv-monitor, res, sizeof res, 0,
(struct sockaddr *)addr, addrlen);
+VIR_DEBUG(nbytes: %d, nbytes);
+VIR_DEBUG(res.error: %d, res.extra: %d, res.length: %d, res.data:
%.*s,
+ res.error, res.extra, res.length, res.length,
res.data);
if (nbytes 0) {
if (errno == EAGAIN || errno == EINTR)
continue;
And I get this output:
11:28:14.602: debug : umlMonitorCommand:702 : Run command 'config con0'
11:28:14.602: debug : umlMonitorCommand:737 : res.error: 5, res.extra: 0,
res.length: 4096, res.data:
11:28:14.602: debug : umlMonitorCommand:740 : nbytes: 28
11:28:14.602: debug : umlMonitorCommand:742 : res.error: 0, res.extra: 0,
res.length: 16, res.data: pts:/dev/pts/31
11:28:14.602: debug : umlMonitorCommand:771 : Command reply is 'pts:/dev/pts/31'
So the size of the response datagram isn't sizeof(res) as the check in
uml_driver.c expects, but rather sizeof(res.error) + sizeof(res.extra) +
sizeof(res.length) + res.length.
--
Soren Hansen so...@linux2go.dk
Systems Architect, The Rackspace Cloud
Ubuntu Developer
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/3] Remove wrong check for uml monitor response size
On Mon, Aug 16, 2010 at 11:37:07AM +0200, Soren Hansen wrote: I'm running this on another kernel right now and I'm not seeing the problem. I'll try again with the kernel I used a couple of days ago. Ok, found the other kernel. Same diff as in my previous e-mail, same action. These are my results: 09:41:01.134: debug : umlMonitorCommand:698 : Run command 'config con0' 09:41:01.134: debug : umlMonitorCommand:733 : res.error: 6, res.extra: 0, res.length: 4096, res.data: 09:41:01.134: debug : umlMonitorCommand:736 : nbytes: 0 09:41:01.134: debug : umlMonitorCommand:738 : res.error: 0, res.extra: 0, res.length: 4, res.data: pts 09:41:01.134: debug : umlMonitorCommand:761 : Command reply is 'pts' 09:41:01.144: debug : umlMonitorCommand:698 : Run command 'config con0' 09:41:01.144: debug : umlMonitorCommand:733 : res.error: 0, res.extra: 0, res.length: 4, res.data: pts 09:41:01.144: debug : umlMonitorCommand:736 : nbytes: 0 09:41:01.144: debug : umlMonitorCommand:738 : res.error: 0, res.extra: 0, res.length: 4, res.data: pts 09:41:01.144: debug : umlMonitorCommand:761 : Command reply is 'pts' 09:41:01.154: debug : umlMonitorCommand:698 : Run command 'config con0' 09:41:01.154: debug : umlMonitorCommand:733 : res.error: 0, res.extra: 0, res.length: 4, res.data: pts 09:41:01.155: debug : umlMonitorCommand:736 : nbytes: 0 09:41:01.155: debug : umlMonitorCommand:738 : res.error: 0, res.extra: 0, res.length: 4, res.data: pts 09:41:01.155: debug : umlMonitorCommand:761 : Command reply is 'pts' 09:41:01.165: debug : umlMonitorCommand:698 : Run command 'config con0' 09:41:01.165: debug : umlMonitorCommand:733 : res.error: 0, res.extra: 0, res.length: 4, res.data: pts 09:41:01.169: debug : umlMonitorCommand:736 : nbytes: 0 09:41:01.169: debug : umlMonitorCommand:738 : res.error: 0, res.extra: 0, res.length: 4, res.data: pts 09:41:01.169: debug : umlMonitorCommand:761 : Command reply is 'pts' 09:41:01.179: debug : umlMonitorCommand:698 : Run command 'config con0' 09:41:01.179: debug : umlMonitorCommand:733 : res.error: 0, res.extra: 0, res.length: 4, res.data: pts 09:41:01.179: debug : umlMonitorCommand:736 : nbytes: 0 09:41:01.179: debug : umlMonitorCommand:738 : res.error: 0, res.extra: 0, res.length: 15, res.data: pts:/dev/pts/7 09:41:01.179: debug : umlMonitorCommand:761 : Command reply is 'pts:/dev/pts/7' This one's a 2.6.34.1 kernel. The one where I didn't see the problem is a 2.6.32.something-or-the-other kernel. Mindboggling. -- Soren Hansen so...@linux2go.dk Systems Architect, The Rackspace Cloud Ubuntu Developer -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] Updated application development guide on libvirt.org
I have published an updated draft of the application development guide at: http://libvirt.org/guide/ It includes expanded content, editing and formatting for: * Chapter 3 - Connections * Chapter 4 - Guest Domains * Chapter 7 - Network Interfaces Additional content for Chapter 3 has been provided to me, and will be included soon. Thanks David -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [RFC: PATCH 0/4] Update array allocation macros
On 08/14/2010 02:05 PM, Daniel P. Berrange wrote: It sounds like it might have some appeal for reducing some of the user's book-keeping, but would require a careful audit of code to safely match VIR_ALLOC_N exactly with VIR_FREE_N. Thoughts on this approach? The #1 goal of the memory allocation APIs is to make it hard to make programming mistakes. Having a VIR_FREE and VIR_FREE_N somewhat compromises that goal, for only a small convenience, so I don't think we need to go down that route. Thanks for the feedback. I agree with ditching the idea of VIR_FREE_N and any notion of storing the allocation size as part of the array - too much complexity to make it easier to write safe programs. Now back to the question in my original cover letter: does VIR_RESIZE_N look worthwhile, or should I confine my rework of VIR_REALLOC_N to just use VIR_EXPAND_N? -- Eric Blake ebl...@redhat.com+1-801-349-2682 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] esx: Fix memory leak when looking up an non-existing domain by name
On 08/15/2010 12:34 PM, Matthias Bolte wrote: such that you only have to have one label, instead of adding a new label? All three instances follow the same pattern. Here's v2 attached. I also removed two already existing success labels. ACK to this version. -- Eric Blake ebl...@redhat.com+1-801-349-2682 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/3] Remove wrong check for uml monitor response size
On 08/16/2010 03:37 AM, Soren Hansen wrote: And I get this output: 11:28:14.602: debug : umlMonitorCommand:702 : Run command 'config con0' 11:28:14.602: debug : umlMonitorCommand:737 : res.error: 5, res.extra: 0, res.length: 4096, res.data: 11:28:14.602: debug : umlMonitorCommand:740 : nbytes: 28 11:28:14.602: debug : umlMonitorCommand:742 : res.error: 0, res.extra: 0, res.length: 16, res.data: pts:/dev/pts/31 11:28:14.602: debug : umlMonitorCommand:771 : Command reply is 'pts:/dev/pts/31' So the size of the response datagram isn't sizeof(res) as the check in uml_driver.c expects, but rather sizeof(res.error) + sizeof(res.extra) + sizeof(res.length) + res.length. I agree with this analysis. In other words, the check should be more like this (two conditions - did we get enough bytes to even have a valid res.length, and did we get enough bytes to match with what res.length stated): if (nbytes offsetof(struct monitor_request, data) || nbytes res.length + offsetof(struct monitor_request, data)) incomplete reply instead of the two current but incorrect checks: if (nbytes sizeof ret) /* incorrectly true except when res.length==MONITOR_BUFLEN */ incomplete reply; if (sizeof res.data res.length) /* res.length of MONITOR_BUFLEN+1 does not trigger this check, but leads to a buffer overflow in subsequent memmove */ invalid length; But before I write such a patch, I'm going to look in more details at your other reply. -- Eric Blake ebl...@redhat.com+1-801-349-2682 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/3] Remove wrong check for uml monitor response size
On 08/16/2010 03:54 AM, Soren Hansen wrote: On Mon, Aug 16, 2010 at 11:37:07AM +0200, Soren Hansen wrote: I'm running this on another kernel right now and I'm not seeing the problem. I'll try again with the kernel I used a couple of days ago. Ok, found the other kernel. Same diff as in my previous e-mail, same action. These are my results: 09:41:01.134: debug : umlMonitorCommand:698 : Run command 'config con0' 09:41:01.134: debug : umlMonitorCommand:733 : res.error: 6, res.extra: 0, res.length: 4096, res.data: 09:41:01.134: debug : umlMonitorCommand:736 : nbytes: 0 09:41:01.134: debug : umlMonitorCommand:738 : res.error: 0, res.extra: 0, res.length: 4, res.data: pts This one's a 2.6.34.1 kernel. The one where I didn't see the problem is a 2.6.32.something-or-the-other kernel. Mindboggling. Oh my. This really does look like a kernel bug. Can you confirm it with an strace? Have you reported this regression to the right kernel folks? I guess it would help if we could write a simpler test program to isolate whether this recvfrom bug exists in a bare minimum number of syscalls. Meanwhile, I have no idea how to work around a buggy recvfrom that doesn't return the correct number of bytes. -- Eric Blake ebl...@redhat.com+1-801-349-2682 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3] nwfilter: extend nwfilter reload support
On Fri, Aug 13, 2010 at 04:36:32PM -0400, Stefan Berger wrote:
This time with a proper title.
v3:
- Fixed an indentation problem
- added bool parameter to function terminating the IP address
learner threads to determine whether future threads may still run
(needed in case of driver reload) or all must terminate (need in case of
libvirtd termination)
v2:
- Fixes to the nwfilter driver reload function that also needs a
valid virConnectPtr.
In this patch I am extending and fixing the nwfilter module's reload
support to stop all ongoing threads (for learning IP addresses of
interfaces) and rebuild the filtering rules of all interfaces of all VMs
when libvirt is started. Now libvirtd rebuilds the filters upon the
SIGHUP signal and libvirtd restart.
About the patch: The nwfilter functions require a virConnectPtr.
Therefore I am opening a connection in qemudStartup, which later on
needs to be closed outside where the driver lock is held since otherwise
it ends up in a deadlock due to virConnectClose() trying to lock the
driver as well.
I have tested this now for a while with several machines running and
needing the IP address learner thread(s). The rebuilding of the firewall
rules seems to work fine following libvirtd restart or a SIGHUP. Also
the termination of libvirtd worked fine.
Signed-off-by: Stefan Bergerstef...@us.ibm.com
---
src/nwfilter/nwfilter_driver.c | 21 +++---
src/nwfilter/nwfilter_learnipaddr.c | 16 ---
src/nwfilter/nwfilter_learnipaddr.h |1
src/qemu/qemu_driver.c | 52
+---
4 files changed, 77 insertions(+), 13 deletions(-)
Index: libvirt-acl/src/nwfilter/nwfilter_driver.c
===
--- libvirt-acl.orig/src/nwfilter/nwfilter_driver.c
+++ libvirt-acl/src/nwfilter/nwfilter_driver.c
@@ -143,15 +143,26 @@ conf_init_err:
*/
static int
nwfilterDriverReload(void) {
+virConnectPtr conn;
+
if (!driverState) {
return -1;
}
-nwfilterDriverLock(driverState);
-virNWFilterPoolLoadAllConfigs(NULL,
- driverState-pools,
- driverState-configDir);
-nwfilterDriverUnlock(driverState);
+conn = virConnectOpen(qemu:///system);
+
+if (conn) {
+/* shut down all threads -- qemud for example will restart them */
+virNWFilterLearnThreadsTerminate(true);
+
+nwfilterDriverLock(driverState);
+virNWFilterPoolLoadAllConfigs(conn,
+ driverState-pools,
+ driverState-configDir);
+nwfilterDriverUnlock(driverState);
+
+virConnectClose(conn);
+}
This identation still seems to be wrong - so was the code being
replaced. ACK aside from that
Daniel
--
|: Red Hat, Engineering, London-o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :|
|: http://autobuild.org-o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] PATCH 1/4: AppArmor updates
On Fri, Aug 13, 2010 at 04:58:57PM -0500, Jamie Strandboge wrote:
Attached is 0001-apparmor-dont-ignore-open.patch
--
Jamie Strandboge | http://www.canonical.com
Index: libvirt-0.8.3/src/security/virt-aa-helper.c
===
--- libvirt-0.8.3.orig/src/security/virt-aa-helper.c 2010-08-12
09:51:42.0 -0500
+++ libvirt-0.8.3/src/security/virt-aa-helper.c 2010-08-12
09:58:42.0 -0500
@@ -853,11 +853,25 @@
* careful than just ignoring them */
int ret = virDomainDiskDefForeachPath(ctl-def-disks[i],
ctl-allowDiskFormatProbing,
- true,
+ false,
add_file_path,
buf);
-if (ret != 0)
+/*
+ * If virDomainDiskDefForeachPath() fails, then exit with error,
+ * unless the disk doesn't exist, in which case we just skip it
+ * without error in order to preserve previous behavior.
+ */
+if (ret != 0) {
+if (ctl-def-disks[i] ctl-def-disks[i]-src) {
+if (!virFileExists(ctl-def-disks[i]-src)) {
+continue;
+} else {
+vah_warning(ctl-def-disks[i]-src);
+vah_warning( skipped (bad disk format));
+}
+}
goto clean;
+}
Why do we need special behaviour to skip files which don't exist ?
If this is solely to keep the test suite happy, then IMHO the
fix should be in the test suite. Runtime production code shouldn't
have workarounds for test code.
Regards,
Daniel
--
|: Red Hat, Engineering, London-o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :|
|: http://autobuild.org-o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] PATCH 2/4: AppArmor updates
On Fri, Aug 13, 2010 at 04:59:30PM -0500, Jamie Strandboge wrote: Attached is 0002-apparmor-chardev.patch -- Jamie Strandboge | http://www.canonical.com Author: Jamie Strandboge ja...@canonical.com Description: fix serial ports, parallel ports and channels Forwarded: yes Bug-Ubuntu: LP: #578527, LP: #609055 Index: libvirt-0.8.3/src/security/virt-aa-helper.c === --- libvirt-0.8.3.orig/src/security/virt-aa-helper.c 2010-08-12 12:00:04.0 -0500 +++ libvirt-0.8.3/src/security/virt-aa-helper.c 2010-08-12 12:00:04.0 -0500 @@ -877,13 +877,27 @@ for (i = 0; i ctl-def-nserials; i++) if (ctl-def-serials[i] ctl-def-serials[i]-data.file.path) if (vah_add_file(buf, - ctl-def-serials[i]-data.file.path, w) != 0) + ctl-def-serials[i]-data.file.path, rw) != 0) goto clean; if (ctl-def-console ctl-def-console-data.file.path) -if (vah_add_file(buf, ctl-def-console-data.file.path, w) != 0) +if (vah_add_file(buf, ctl-def-console-data.file.path, rw) != 0) goto clean; +for (i = 0 ; i ctl-def-nparallels; i++) +if (ctl-def-parallels[i] ctl-def-parallels[i]-data.file.path) +if (vah_add_file(buf, + ctl-def-parallels[i]-data.file.path, + rw) != 0) +goto clean; + +for (i = 0 ; i ctl-def-nchannels; i++) +if (ctl-def-channels[i] ctl-def-channels[i]-data.file.path) +if (vah_add_file(buf, + ctl-def-channels[i]-data.file.path, + rw) != 0) +goto clean; You can't blindly de-reference data.file.path - The 'file' struct is inside a union and is only valid for certain types of character device VIR_DOMAIN_CHR_TYPE_PTY, TYPE_DEV, TYPE_FILE and TYPE_PIPE. The existing code for serial devices is broken too can crash due to this Regards, Daniel -- |: Red Hat, Engineering, London-o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :| |: http://autobuild.org-o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] PATCH 3/4: AppArmor updates
On Fri, Aug 13, 2010 at 05:00:06PM -0500, Jamie Strandboge wrote:
Attached is 0003-apparmor-examples.patch
Can you include full commit messages with each patch,
since it makes it easier to review understand, and
will be needed when the patches are applied to GIT.
diff -Naurp libvirt.orig/examples/apparmor/libvirt-qemu
libvirt/examples/apparmor/libvirt-qemu
--- libvirt.orig/examples/apparmor/libvirt-qemu 2010-04-06
16:14:52.0 -0500
+++ libvirt/examples/apparmor/libvirt-qemu2010-08-13 16:46:34.0
-0500
@@ -1,4 +1,4 @@
-# Last Modified: Mon Apr 5 15:11:27 2010
+# Last Modified: Fri Aug 13 16:38:32 2010
#include abstractions/base
#include abstractions/consoles
@@ -9,6 +9,10 @@
capability dac_read_search,
capability chown,
+ # needed to drop privileges
+ capability setgid,
+ capability setuid,
+
network inet stream,
network inet6 stream,
diff -Naurp libvirt.orig/examples/apparmor/usr.lib.libvirt.virt-aa-helper
libvirt/examples/apparmor/usr.lib.libvirt.virt-aa-helper
--- libvirt.orig/examples/apparmor/usr.lib.libvirt.virt-aa-helper
2010-04-06 16:14:52.0 -0500
+++ libvirt/examples/apparmor/usr.lib.libvirt.virt-aa-helper 2010-08-13
16:44:01.0 -0500
@@ -1,8 +1,9 @@
-# Last Modified: Mon Apr 5 15:10:27 2010
+# Last Modified: Fri Aug 13 16:38:32 2010
#include tunables/global
/usr/lib/libvirt/virt-aa-helper {
#include abstractions/base
+ #include abstractions/user-tmp
# needed for searching directories
capability dac_override,
@@ -12,11 +13,16 @@
network inet,
deny @{PROC}/[0-9]*/mounts r,
+ @{PROC}/[0-9]*/net/psched r,
@{PROC}/filesystems r,
# for hostdev
/sys/devices/ r,
/sys/devices/** r,
+ /sys/bus/usb/devices/ r,
+ deny /dev/sd* r,
+ deny /dev/mapper/ r,
+ deny /dev/mapper/* r,
/usr/lib/libvirt/virt-aa-helper mr,
/sbin/apparmor_parser Ux,
@@ -24,8 +30,11 @@
/etc/apparmor.d/libvirt/* r,
/etc/apparmor.d/libvirt/libvirt-[0-9a-f]*-[0-9a-f]*-[0-9a-f]*-[0-9a-f]*-[0-9a-f]*
rw,
- # for backingstore -- allow access to non-hidden files in @{HOME} as well
- # as storage pools
+ # For backingstore, virt-aa-helper may need to peek inside the disk image,
so
+ # allow access to non-hidden files in @{HOME} as well as storage pools, and
+ # removable media and filesystems, and certain file extentions. A
+ # virt-aa-helper failure when checking a disk for backinsgstore is
non-fatal
+ # (but obviously the backingstore won't be added).
audit deny @{HOME}/.* mrwkl,
audit deny @{HOME}/.*/ rw,
audit deny @{HOME}/.*/** mrwkl,
ACK
Daniel
--
|: Red Hat, Engineering, London-o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :|
|: http://autobuild.org-o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] PATCH 4/4: AppArmor updates
On Fri, Aug 13, 2010 at 05:00:31PM -0500, Jamie Strandboge wrote: Attached is 0004-apparmor-fix-warn.patch -- Jamie Strandboge | http://www.canonical.com diff -Naurp libvirt.orig/src/security/virt-aa-helper.c libvirt/src/security/virt-aa-helper.c --- libvirt.orig/src/security/virt-aa-helper.c2010-08-13 16:23:44.0 -0500 +++ libvirt/src/security/virt-aa-helper.c 2010-08-13 16:49:27.0 -0500 @@ -19,6 +19,7 @@ #include unistd.h #include errno.h #include sys/types.h +#include sys/stat.h #include fcntl.h #include getopt.h #include stdbool.h ACK Daniel -- |: Red Hat, Engineering, London-o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :| |: http://autobuild.org-o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/3] Remove wrong check for uml monitor response size
On Mon, Aug 16, 2010 at 10:08:37AM -0600, Eric Blake wrote: On 08/16/2010 03:54 AM, Soren Hansen wrote: On Mon, Aug 16, 2010 at 11:37:07AM +0200, Soren Hansen wrote: I'm running this on another kernel right now and I'm not seeing the problem. I'll try again with the kernel I used a couple of days ago. Ok, found the other kernel. Same diff as in my previous e-mail, same action. These are my results: 09:41:01.134: debug : umlMonitorCommand:698 : Run command 'config con0' 09:41:01.134: debug : umlMonitorCommand:733 : res.error: 6, res.extra: 0, res.length: 4096, res.data: 09:41:01.134: debug : umlMonitorCommand:736 : nbytes: 0 09:41:01.134: debug : umlMonitorCommand:738 : res.error: 0, res.extra: 0, res.length: 4, res.data: pts This one's a 2.6.34.1 kernel. The one where I didn't see the problem is a 2.6.32.something-or-the-other kernel. Mindboggling. Oh my. This really does look like a kernel bug. Can you confirm it with an strace? Have you reported this regression to the right kernel folks? I guess it would help if we could write a simpler test program to isolate whether this recvfrom bug exists in a bare minimum number of syscalls. Meanwhile, I have no idea how to work around a buggy recvfrom that doesn't return the correct number of bytes. If there is an identifiable kernel bug then that should be reported and fixed ASAP. IMHO we shouldn't try to workaround such serious brokenness as this is showing. Regards, Daniel -- |: Red Hat, Engineering, London-o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :| |: http://autobuild.org-o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/3] qemu: Re-reserve all PCI addresses on libvirtd restart
On Fri, Aug 13, 2010 at 05:26:45PM +0200, Jiri Denemark wrote:
When reconnecting to existing VMs, we re-reserved only those PCI
addresses which were explicitly mentioned in domain XML. Since some
addresses are always reserved (e.g., 0:0:0 and 0:0:1), we need to handle
those too.
Also all this should only be done if device flag is supported by qemu.
---
src/qemu/qemu_driver.c |8 +---
1 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index e718816..2ca3940 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -1455,11 +1455,13 @@ qemuReconnectDomain(void *payload, const char *name
ATTRIBUTE_UNUSED, void *opaq
if (qemudExtractVersionInfo(obj-def-emulator,
NULL,
qemuCmdFlags) = 0
-(qemuCmdFlags QEMUD_CMD_FLAG_DEVICE))
+(qemuCmdFlags QEMUD_CMD_FLAG_DEVICE)) {
priv-persistentAddrs = 1;
-if (!(priv-pciaddrs = qemuDomainPCIAddressSetCreate(obj-def)))
-goto error;
+if (!(priv-pciaddrs = qemuDomainPCIAddressSetCreate(obj-def)) ||
+qemuAssignDevicePCISlots(obj-def, priv-pciaddrs) 0)
+goto error;
+}
if (driver-securityDriver
driver-securityDriver-domainReserveSecurityLabel
ACK
Daniel
--
|: Red Hat, Engineering, London-o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :|
|: http://autobuild.org-o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/3] qemu: Release PCI slot when detaching disk and net devices
On Fri, Aug 13, 2010 at 05:26:46PM +0200, Jiri Denemark wrote: --- src/qemu/qemu_driver.c |8 1 files changed, 8 insertions(+), 0 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 2ca3940..108f67b 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -8649,6 +8649,10 @@ static int qemudDomainDetachPciDiskDevice(struct qemud_driver *driver, } qemuDomainObjExitMonitorWithDriver(driver, vm); +if ((qemuCmdFlags QEMUD_CMD_FLAG_DEVICE) +qemuDomainPCIAddressReleaseAddr(priv-pciaddrs, detach-info) 0) +VIR_WARN(Unable to release PCI address on %s, dev-data.disk-src); + qemudShrinkDisks(vm-def, i); virDomainDiskDefFree(detach); @@ -8890,6 +8894,10 @@ qemudDomainDetachNetDevice(struct qemud_driver *driver, } qemuDomainObjExitMonitorWithDriver(driver, vm); +if ((qemuCmdFlags QEMUD_CMD_FLAG_DEVICE) +qemuDomainPCIAddressReleaseAddr(priv-pciaddrs, detach-info) 0) +VIR_WARN0(Unable to release PCI address on NIC); + virDomainConfNWFilterTeardown(detach); #if WITH_MACVTAP ACK Daniel -- |: Red Hat, Engineering, London-o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :| |: http://autobuild.org-o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/3] Fix up qemu domain save/managed save locking.
On 08/13/2010 08:53 AM, Chris Lalancette wrote: The current version of the qemu managed save implementation is subject to a race where the domain shuts down between the time that we start the command and the time that we actually try to do the save. Close this race by making qemuDomainSaveFlags() expect both the driver and the passed-in vm object to be locked before executing. Signed-off-by: Chris Lalancette clala...@redhat.com --- src/qemu/qemu_driver.c | 79 ++- 1 files changed, 30 insertions(+), 49 deletions(-) ACK. -- Eric Blake ebl...@redhat.com+1-801-349-2682 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/3] qemu: Fix copypaste error in warning message
On Fri, Aug 13, 2010 at 05:26:47PM +0200, Jiri Denemark wrote:
This also makes the message consistent with the message used in error
path of qemudDomainAttachHostPciDevice.
---
src/qemu/qemu_driver.c |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 108f67b..dbc3d5c 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -9020,7 +9020,7 @@ static int qemudDomainDetachHostPciDevice(struct
qemud_driver *driver,
if ((qemuCmdFlags QEMUD_CMD_FLAG_DEVICE)
qemuDomainPCIAddressReleaseAddr(priv-pciaddrs, detach-info) 0)
-VIR_WARN0(Unable to release PCI address on controller);
+VIR_WARN0(Unable to release PCI address on host device);
if (vm-def-nhostdevs 1) {
memmove(vm-def-hostdevs + i,
ACK
Daniel
--
|: Red Hat, Engineering, London-o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :|
|: http://autobuild.org-o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/4] memory: make it safer to expand arrays
On Sat, Aug 14, 2010 at 10:23:53AM -0600, Eric Blake wrote:
Note - I did _not_ update HACKING. Do we have a makefile rule
for doing an auto-xsl translation from hacking.html.in yet, or
is something I'll have to do manually before pushing this?
* src/util/memory.h (VIR_REALLOC_N): Update docs.
(VIR_EXPAND_N, VIR_SHRINK_N): New macros.
(virAlloc, virAllocN, virReallocN, virAllocVar, virFree): Add some
gcc attributes.
* src/util/memory.c (virExpandN, virShrinkN): New functions.
(virReallocN): Update docs.
* docs/hacking.html.in: Prefer newer interfaces over
VIR_REALLOC_N, since uninitialized memory can bite us.
---
docs/hacking.html.in | 24 +++
src/util/memory.c| 59
+-
src/util/memory.h| 51 ++-
3 files changed, 117 insertions(+), 17 deletions(-)
diff --git a/docs/hacking.html.in b/docs/hacking.html.in
index deab530..4cc92f4 100644
--- a/docs/hacking.html.in
+++ b/docs/hacking.html.in
@@ -332,11 +332,11 @@
Use of the malloc/free/realloc/calloc APIs is deprecated in the libvirt
codebase, because they encourage a number of serious coding bugs and do
not enable compile time verification of checks for NULL. Instead of
these
- routines, use the macros from memory.h
+ routines, use the macros from memory.h.
/p
ul
- lipeg to allocate a single object:/p
+ lipTo allocate a single object:/p
pre
virDomainPtr domain;
@@ -347,11 +347,11 @@
}
/pre/li
- lipeg to allocate an array of objects/p
+ lipTo allocate an array of objects/p
pre
virDomainPtr domains;
- int ndomains = 10;
+ size_t ndomains = 10;
if (VIR_ALLOC_N(domains, ndomains) lt; 0) {
virReportOOMError();
@@ -359,7 +359,7 @@
}
/pre/li
- lipeg to allocate an array of object pointers/p
+ lipTo allocate an array of object pointers/p
pre
virDomainPtr *domains;
@@ -371,18 +371,22 @@
}
/pre/li
- lipeg to re-allocate the array of domains to be longer/p
+ lipTo re-allocate the array of domains to be 10 elements longer/p
pre
- ndomains = 20
-
- if (VIR_REALLOC_N(domains, ndomains) lt; 0) {
+ if (VIR_EXPAND_N(domains, ndomains, 10) lt; 0) {
virReportOOMError();
return NULL;
}
/pre/li
- lipeg to free the domain/p
+ lipTo trim an array of domains to have one less element/p
+
+pre
+ VIR_SHRINK_N(domains, ndomains, 1);
+/pre/li
+
+ lipTo free the domain/p
pre
VIR_FREE(domain);
diff --git a/src/util/memory.c b/src/util/memory.c
index dd1216b..e95aa47 100644
--- a/src/util/memory.c
+++ b/src/util/memory.c
@@ -1,6 +1,7 @@
/*
* memory.c: safer memory allocation
*
+ * Copyright (C) 2010 Red Hat, Inc.
* Copyright (C) 2008 Daniel P. Berrange
*
* This library is free software; you can redistribute it and/or
@@ -24,6 +25,7 @@
#include stddef.h
#include memory.h
+#include ignore-value.h
#if TEST_OOM
@@ -141,7 +143,7 @@ int virAllocN(void *ptrptr, size_t size, size_t count)
* 'count' elements, each 'size' bytes in length. Update 'ptrptr'
* with the address of the newly allocated memory. On failure,
* 'ptrptr' is not changed and still points to the original memory
- * block. The newly allocated memory is filled with zeros.
+ * block. Any newly allocated memory in 'ptrptr' is uninitialized.
*
* Returns -1 on failure to allocate, zero on success
*/
@@ -165,6 +167,61 @@ int virReallocN(void *ptrptr, size_t size, size_t count)
}
/**
+ * virExpandN:
+ * @ptrptr: pointer to pointer for address of allocated memory
+ * @size: number of bytes per element
+ * @countptr: pointer to number of elements in array
+ * @add: number of elements to add
+ *
+ * Resize the block of memory in 'ptrptr' to be an array of
+ * '*countptr' + 'add' elements, each 'size' bytes in length.
+ * Update 'ptrptr' and 'countptr' with the details of the newly
+ * allocated memory. On failure, 'ptrptr' and 'countptr' are not
+ * changed. Any newly allocated memory in 'ptrptr' is zero-filled.
+ *
+ * Returns -1 on failure to allocate, zero on success
+ */
+int virExpandN(void *ptrptr, size_t size, size_t *countptr, size_t add)
+{
+int res;
+
+if (*countptr + add *countptr) {
+errno = ENOMEM;
+return -1;
+}
+res = virReallocN(ptrptr, size, *countptr + add);
+if (res == 0) {
+memset(*(char **)ptrptr + (size * *countptr), 0, size * add);
+*countptr += add;
+}
+return res;
+}
+
+/**
+ * virShrinkN:
+ * @ptrptr: pointer to pointer for address of allocated memory
+ * @size: number of bytes per element
+ * @countptr: pointer to number of elements in array
+ * @remove: number of elements to remove
+ *
+ *
Re: [libvirt] [PATCH 3/3] Plumb managedsave compression through to virsh.
On 08/13/2010 08:53 AM, Chris Lalancette wrote:
Signed-off-by: Chris Lalancette clala...@redhat.com
---
tools/virsh.c | 22 +-
1 files changed, 21 insertions(+), 1 deletions(-)
Depending on how the discussion on 2/3 goes:
+compress = vshCommandOptString(cmd, compression, NULL);
+if (compress) {
+if (STREQ(compress, gzip))
+flags |= VIR_DOMAIN_SAVE_COMPRESS_GZIP;
+else if (STREQ(compress, bzip2))
+flags |= VIR_DOMAIN_SAVE_COMPRESS_BZIP2;
+else if (STREQ(compress, xz))
+flags |= VIR_DOMAIN_SAVE_COMPRESS_XZ;
+else if (STREQ(compress, lzop))
+flags |= VIR_DOMAIN_SAVE_COMPRESS_LZOP;
+else {
+vshError(ctl, %s,
+ _(Invalid compression type; it must be one of 'gzip',
'bzip2', 'xz', or 'lzop'));
+return FALSE;
If we add a qemu.conf default compression setting, then virsh also needs
to support a '--compress none' to override the default.
Hmm, that also means that VIR_DOMAIN_SAVE_COMPRESS_RAW needs to be
distinct from 0 (that is, passing 0 means that you are requesting the
default, while requesting raw means that you are explicitly requesting
no compression even if the default would use compression).
--
Eric Blake ebl...@redhat.com+1-801-349-2682
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/4] daemon: use safer memory growth macros
On Sat, Aug 14, 2010 at 10:23:54AM -0600, Eric Blake wrote: * daemon/libvirtd.h (qemud_server): Change types of members tracking array sizes. * daemon/event.c (virEventLoop): Likewise. (virEventAddHandleImpl, virEventAddTimeoutImpl) (virEventCleanupTimeouts, virEventCleanupHandles): Use VIR_EXPAND_N and VIR_SHRINK_N instead of VIR_REALLOC_N. Tweak debug messages to match type changes. * daemon/libvirtd.c (qemudDispatchServer, qemudRunLoop): Use VIR_EXPAND_N and VIR_SHRINK_N. Signed-off-by: Eric Blake ebl...@redhat.com --- daemon/event.c| 44 +++- daemon/libvirtd.c | 10 -- daemon/libvirtd.h | 10 +- 3 files changed, 28 insertions(+), 36 deletions(-) ACK, looks nicer. Daniel -- |: Red Hat, Engineering, London-o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :| |: http://autobuild.org-o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/3] Managed save compression flags.
On 08/13/2010 09:06 AM, Daniel P. Berrange wrote: On Fri, Aug 13, 2010 at 10:53:48AM -0400, Chris Lalancette wrote: Add in the ability to specify compression flags from the managed save API. We map these to the supported QEMU compression flags internally. I'm not really convinced we need todo this. I think the host level setting in /etc/libvirt/qemu.conf is sufficient already. But that's a global setting for all qemu domains. I can see where it might make sense to compression for some domains but not others, at which point exposing the ability to suggest a non-default compression makes sense. On the other hand, if we agree that a qemu.conf setting is adequate, then this patch still needs a respin, in order to honor the conf-file default rather than hard-coding it to raw. So either way, we need more discussion and probably a respin. -- Eric Blake ebl...@redhat.com+1-801-349-2682 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/4] memory: make it safer to expand arrays
On 08/16/2010 10:20 AM, Daniel P. Berrange wrote: On Sat, Aug 14, 2010 at 10:23:53AM -0600, Eric Blake wrote: Note - I did _not_ update HACKING. Do we have a makefile rule for doing an auto-xsl translation from hacking.html.in yet, or is something I'll have to do manually before pushing this? Any thoughts on this question? ACK, I've wanted to add this for a long time now Good, I suspected that this patch was uncontroversial; it was more the rest of the series (and the remaining auditing work to go through more than 100 more VIR_REALLOC_N usages) that I posed this as an RFC; so I will hold of pushing this patch until the reworked series is complete. -- Eric Blake ebl...@redhat.com+1-801-349-2682 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/3] Managed save compression flags.
On Mon, Aug 16, 2010 at 10:21:31AM -0600, Eric Blake wrote: On 08/13/2010 09:06 AM, Daniel P. Berrange wrote: On Fri, Aug 13, 2010 at 10:53:48AM -0400, Chris Lalancette wrote: Add in the ability to specify compression flags from the managed save API. We map these to the supported QEMU compression flags internally. I'm not really convinced we need todo this. I think the host level setting in /etc/libvirt/qemu.conf is sufficient already. But that's a global setting for all qemu domains. I can see where it might make sense to compression for some domains but not others, at which point exposing the ability to suggest a non-default compression makes sense. I know that's a global setting. IMHO setting compression per VM is a feature in search of a problem. On the other hand, if we agree that a qemu.conf setting is adequate, then this patch still needs a respin, in order to honor the conf-file default rather than hard-coding it to raw. Yes, that's a serious bug in the current code. Daniel -- |: Red Hat, Engineering, London-o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :| |: http://autobuild.org-o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] Documentation on www.libvirt.org about using PolicyKit authentication for libvirt is out of date
Hi all, I've created a patch for the bug here: https://bugzilla.redhat.com/show_bug.cgi?id=610822 It is attached to this message. The patch updates the docs to have more up-to-date information about how to use PolicyKit with libvirt Best, Patrick Dignan From fcece6eee96adbebd735926a90b4ea53525c2d8e Mon Sep 17 00:00:00 2001 From: Patrick Dignan pat_dig...@dell.com Date: Thu, 12 Aug 2010 13:52:50 -0500 Subject: [PATCH] Updated PolicyKit documentation --- docs/auth.html.in | 37 + 1 files changed, 21 insertions(+), 16 deletions(-) diff --git a/docs/auth.html.in b/docs/auth.html.in index ab6c3e9..9e4a33a 100644 --- a/docs/auth.html.in +++ b/docs/auth.html.in @@ -65,28 +65,33 @@ auth, but does not require that the client application ultimately run as root. Default policy will still allow any application to connect to the RO socket. /p p -The default policy can be overridden by the administrator using the PolicyKit -master configuration file in code/etc/PolicyKit/PolicyKit.conf/code. The -codePolicyKit.conf(5)/code manual page provides details on the syntax -available. The two libvirt daemon actions available are named codeorg.libvirt.unix.monitor/code -for the RO socket, and codeorg.libvirt.unix.manage/code for the RW socket. +The default policy can be overridden by creating a new policy file in the local +override directory code/etc/polkit-1/localauthority/50-local.d//code. +Policy files should have a unique name ending with .pkla. Using reverse DNS naming +works well. Information on the options available can be found by reading the +pklocalauthority man page. The two libvirt daemon actions available are named +codeorg.libvirt.unix.monitor/code for the RO socket, and +codeorg.libvirt.unix.manage/code for the RW socket. /p p As an example, to allow a user codefred/code full access to the RW socket, while requiring codejoe/code to authenticate with the admin password, would require adding the following snippet to codePolicyKit.conf/code. /p -pre - lt;match action=org.libvirt.unix.managegt; -lt;match user=fredgt; - lt;return result=yes/gt; -lt;/matchgt; - lt;/matchgt; - lt;match action=org.libvirt.unix.managegt; -lt;match user=joegt; - lt;return result=auth_admin/gt; -lt;/matchgt; - lt;/matchgt; +pre +[Allow fred libvirt management permissions] +Identity=unix-user:fred +Action=org.libvirt.unix.manage +ResultAny=No +ResultInactive=No +ResultActive=Yes + +[Allow joe libvirt management with admin password] +Identity=unix-user:joe +Action=org.libvirt.unix.manage +ResultAny=No +ResultInactive=No +ResultActive=auth_admin /pre h3a name=ACL_server_usernameUsername/password auth/a/h3 p -- 1.7.2.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/2] qemu: Update next usable PCI slot on domain reconnect
When libvirtd restarted and reconnected to existing domains, we didn't update nextslot so that hotplug starts with a slot next to the last used one. Jiri Denemark (2): qemu: Use structured PCI address as hash payload qemu: Update next usable PCI slot on domain reconnect src/qemu/qemu_conf.c | 118 +--- src/qemu/qemu_conf.h |1 + src/qemu/qemu_driver.c |2 + 3 files changed, 94 insertions(+), 27 deletions(-) -- 1.7.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/2] qemu: Update next usable PCI slot on domain reconnect
---
src/qemu/qemu_conf.c | 40 +++-
src/qemu/qemu_conf.h |1 +
src/qemu/qemu_driver.c |2 ++
3 files changed, 38 insertions(+), 5 deletions(-)
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index 38d28bf..bf950f2 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -2066,6 +2066,18 @@ struct _qemuDomainPCIAddressSet {
};
+static void
+qemuDomainPCIAddressSetUpdateNextSlot(qemuDomainPCIAddressSetPtr addrs,
+ const virDomainDevicePCIAddressPtr pci)
+{
+if (pci-slot addrs-nextslot) {
+addrs-nextslot = pci-slot + 1;
+if (QEMU_PCI_ADDRESS_LAST_SLOT addrs-nextslot)
+addrs-nextslot = 0;
+}
+}
+
+
static char *qemuPCIAddressAsString(virDomainDeviceInfoPtr dev)
{
char *addr;
@@ -2174,11 +2186,7 @@ int
qemuDomainPCIAddressReserveAddr(qemuDomainPCIAddressSetPtr addrs,
VIR_FREE(addr);
-if (dev-addr.pci.slot addrs-nextslot) {
-addrs-nextslot = dev-addr.pci.slot + 1;
-if (QEMU_PCI_ADDRESS_LAST_SLOT addrs-nextslot)
-addrs-nextslot = 0;
-}
+qemuDomainPCIAddressSetUpdateNextSlot(addrs, pci);
return 0;
@@ -2236,6 +2244,28 @@ int
qemuDomainPCIAddressReleaseAddr(qemuDomainPCIAddressSetPtr addrs,
}
+static void
+qemuDomainPCIAddressSetUpdateIter(void *payload,
+ const char *name ATTRIBUTE_UNUSED,
+ void *data)
+{
+qemuDomainPCIAddressSetUpdateNextSlot(data, payload);
+}
+
+
+void
+qemuDomainPCIAddressSetUpdate(qemuDomainPCIAddressSetPtr addrs)
+{
+
+if (!addrs)
+return;
+
+virHashForEach(addrs-used, qemuDomainPCIAddressSetUpdateIter, addrs);
+
+VIR_DEBUG(nextslot updated to %d, addrs-nextslot);
+}
+
+
void qemuDomainPCIAddressSetFree(qemuDomainPCIAddressSetPtr addrs)
{
if (!addrs)
diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h
index 1aa9d2e..fb93e0e 100644
--- a/src/qemu/qemu_conf.h
+++ b/src/qemu/qemu_conf.h
@@ -330,6 +330,7 @@ int
qemuDomainPCIAddressEnsureAddr(qemuDomainPCIAddressSetPtr addrs,
int qemuDomainPCIAddressReleaseAddr(qemuDomainPCIAddressSetPtr addrs,
virDomainDeviceInfoPtr dev);
+void qemuDomainPCIAddressSetUpdate(qemuDomainPCIAddressSetPtr addrs);
void qemuDomainPCIAddressSetFree(qemuDomainPCIAddressSetPtr addrs);
int qemuAssignDevicePCISlots(virDomainDefPtr def, qemuDomainPCIAddressSetPtr
addrs);
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index dbc3d5c..5a52549 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -1461,6 +1461,8 @@ qemuReconnectDomain(void *payload, const char *name
ATTRIBUTE_UNUSED, void *opaq
if (!(priv-pciaddrs = qemuDomainPCIAddressSetCreate(obj-def)) ||
qemuAssignDevicePCISlots(obj-def, priv-pciaddrs) 0)
goto error;
+
+qemuDomainPCIAddressSetUpdate(priv-pciaddrs);
}
if (driver-securityDriver
--
1.7.2
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/2] qemu: Use structured PCI address as hash payload
Qemu driver stores a per-domain hash of used PCI addresses. Currently,
payload of each hash entry is exactly the same as its key, i.e., string
representation of a PCI address. This patch changes payload to be
virDomainDevicePCIAddressPtr. Along the road, return values of all
qemuPCIAddressAsString() calls are properly checked and error returned
if required.
---
src/qemu/qemu_conf.c | 78 --
1 files changed, 56 insertions(+), 22 deletions(-)
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index fb85220..38d28bf 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -2093,19 +2093,33 @@ static int qemuCollectPCIAddress(virDomainDefPtr def
ATTRIBUTE_UNUSED,
void *opaque)
{
qemuDomainPCIAddressSetPtr addrs = opaque;
+virDomainDevicePCIAddressPtr pci = NULL;
+char *addr = NULL;
if (dev-type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) {
-char *addr = qemuPCIAddressAsString(dev);
+if (!(addr = qemuPCIAddressAsString(dev)))
+goto error;
+
+if (VIR_ALLOC(pci) 0) {
+virReportOOMError();
+goto error;
+}
+*pci = dev-addr.pci;
VIR_DEBUG(Remembering PCI addr %s, addr);
-if (virHashAddEntry(addrs-used, addr, addr) 0) {
-VIR_FREE(addr);
-return -1;
-}
+if (virHashAddEntry(addrs-used, addr, pci) 0)
+goto error;
+
+VIR_FREE(addr);
}
return 0;
+
+error:
+VIR_FREE(addr);
+VIR_FREE(pci);
+return -1;
}
@@ -2134,25 +2148,31 @@ error:
int qemuDomainPCIAddressReserveAddr(qemuDomainPCIAddressSetPtr addrs,
virDomainDeviceInfoPtr dev)
{
-char *addr;
+char *addr = NULL;
+virDomainDevicePCIAddressPtr pci = NULL;
addr = qemuPCIAddressAsString(dev);
if (!addr)
-return -1;
+goto error;
+
+if (VIR_ALLOC(pci) 0) {
+virReportOOMError();
+goto error;
+}
+*pci = dev-addr.pci;
VIR_DEBUG(Reserving PCI addr %s, addr);
if (virHashLookup(addrs-used, addr)) {
qemuReportError(VIR_ERR_INTERNAL_ERROR,
_(unable to reserve PCI address %s), addr);
-VIR_FREE(addr);
-return -1;
+goto error;
}
-if (virHashAddEntry(addrs-used, addr, addr)) {
-VIR_FREE(addr);
-return -1;
-}
+if (virHashAddEntry(addrs-used, addr, pci))
+goto error;
+
+VIR_FREE(addr);
if (dev-addr.pci.slot addrs-nextslot) {
addrs-nextslot = dev-addr.pci.slot + 1;
@@ -2161,6 +2181,11 @@ int
qemuDomainPCIAddressReserveAddr(qemuDomainPCIAddressSetPtr addrs,
}
return 0;
+
+error:
+VIR_FREE(addr);
+VIR_FREE(pci);
+return -1;
}
int qemuDomainPCIAddressReserveSlot(qemuDomainPCIAddressSetPtr addrs,
@@ -2226,11 +2251,12 @@ int
qemuDomainPCIAddressSetNextAddr(qemuDomainPCIAddressSetPtr addrs,
{
int i;
int iteration;
+char *addr = NULL;
+virDomainDevicePCIAddressPtr pci = NULL;
for (i = addrs-nextslot, iteration = 0;
iteration = QEMU_PCI_ADDRESS_LAST_SLOT; i++, iteration++) {
virDomainDeviceInfo maybe;
-char *addr;
if (QEMU_PCI_ADDRESS_LAST_SLOT i)
i = 0;
@@ -2239,7 +2265,8 @@ int
qemuDomainPCIAddressSetNextAddr(qemuDomainPCIAddressSetPtr addrs,
maybe.addr.pci.bus = 0;
maybe.addr.pci.slot = i;
-addr = qemuPCIAddressAsString(maybe);
+if (!(addr = qemuPCIAddressAsString(maybe)))
+goto error;
if (virHashLookup(addrs-used, addr)) {
VIR_DEBUG(PCI addr %s already in use, addr);
@@ -2247,17 +2274,20 @@ int
qemuDomainPCIAddressSetNextAddr(qemuDomainPCIAddressSetPtr addrs,
continue;
}
+if (VIR_ALLOC(pci) 0) {
+virReportOOMError();
+goto error;
+}
+*pci = maybe.addr.pci;
+
VIR_DEBUG(Allocating PCI addr %s, addr);
-if (virHashAddEntry(addrs-used, addr, addr) 0) {
-VIR_FREE(addr);
-return -1;
-}
+if (virHashAddEntry(addrs-used, addr, pci) 0)
+goto error;
+VIR_FREE(addr);
dev-type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI;
-dev-addr.pci.domain = 0;
-dev-addr.pci.bus = 0;
-dev-addr.pci.slot = i;
+dev-addr.pci = maybe.addr.pci;
addrs-nextslot = i + 1;
if (QEMU_PCI_ADDRESS_LAST_SLOT addrs-nextslot)
@@ -2268,6 +2298,10 @@ int
qemuDomainPCIAddressSetNextAddr(qemuDomainPCIAddressSetPtr addrs,
qemuReportError(VIR_ERR_INTERNAL_ERROR,
%s, _(No more available PCI addresses));
+
+error:
+VIR_FREE(addr);
+VIR_FREE(pci);
return -1;
}
--
1.7.2
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3] nwfilter: extend nwfilter reload support
libvir-list-boun...@redhat.com wrote on 08/16/2010 12:09:48 PM: Please respond to Daniel P. Berrange On Fri, Aug 13, 2010 at 04:36:32PM -0400, Stefan Berger wrote: This time with a proper title. v3: - Fixed an indentation problem - added bool parameter to function terminating the IP address learner threads to determine whether future threads may still run (needed in case of driver reload) or all must terminate (need in case of libvirtd termination) v2: - Fixes to the nwfilter driver reload function that also needs a valid virConnectPtr. In this patch I am extending and fixing the nwfilter module's reload support to stop all ongoing threads (for learning IP addresses of interfaces) and rebuild the filtering rules of all interfaces of all VMs when libvirt is started. Now libvirtd rebuilds the filters upon the SIGHUP signal and libvirtd restart. About the patch: The nwfilter functions require a virConnectPtr. Therefore I am opening a connection in qemudStartup, which later on needs to be closed outside where the driver lock is held since otherwise it ends up in a deadlock due to virConnectClose() trying to lock the driver as well. I have tested this now for a while with several machines running and needing the IP address learner thread(s). The rebuilding of the firewall rules seems to work fine following libvirtd restart or a SIGHUP. Also the termination of libvirtd worked fine. Signed-off-by: Stefan Bergerstef...@us.ibm.com [...] This identation still seems to be wrong - so was the code being replaced. ACK aside from that Daniel Pushed. (This indentation issue must be related to something Thunderbird 3.1.1 does to emails). Stefan -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] web page suggestion
My XSL skills are less than stellar, so I'm throwing this out to the list in case someone else can pick it up and come up with a decent patch in less time. Right now, http://libvirt.org/ChangeLog.html is worthless; it is linked from a couple of other pages, such as http://libvirt.org/news.html. A better place to link would be a live git page: http://libvirt.org/git/?p=libvirt.git;a=log I don't know whether it is easier to update news.html.in and sitemap.html.in to point directly to the new link, or if we should keep ChangeLog.xsl but have it revamped to point to the new link. -- Eric Blake ebl...@redhat.com+1-801-349-2682 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3] nwfilter: extend nwfilter reload support
On 08/16/2010 11:23 AM, Stefan Berger wrote: This identation still seems to be wrong - so was the code being replaced. ACK aside from that Daniel Pushed. (This indentation issue must be related to something Thunderbird 3.1.1 does to emails). Odd indeed. But good to know that the patch actually in git has no problems. -- Eric Blake ebl...@redhat.com+1-801-349-2682 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] PATCH 4/4: AppArmor updates
On 08/16/2010 10:15 AM, Daniel P. Berrange wrote: On Fri, Aug 13, 2010 at 05:00:31PM -0500, Jamie Strandboge wrote: Attached is 0004-apparmor-fix-warn.patch -- Jamie Strandboge | http://www.canonical.com diff -Naurp libvirt.orig/src/security/virt-aa-helper.c libvirt/src/security/virt-aa-helper.c --- libvirt.orig/src/security/virt-aa-helper.c 2010-08-13 16:23:44.0 -0500 +++ libvirt/src/security/virt-aa-helper.c2010-08-13 16:49:27.0 -0500 @@ -19,6 +19,7 @@ #include unistd.h #include errno.h #include sys/types.h +#include sys/stat.h #include fcntl.h #include getopt.h #include stdbool.h ACK This one is independently useful, so I've pushed it in your name after inventing a trivial commit message. For the others, I'll wait for a repost that includes the commit message and resolves Daniel's concerns. Also, when posting a patch series, it is helpful if all of the patches in the series are sent in-reply-to the cover letter (in my particular case, I have my mail client preferences set to display threads in order of most recent activity, so when a patch series is not threaded, it means that a reply to one patch in the series makes it harder to find the rest of the emails related to the series since they are no longer adjacent). Using 'git send-email' can make sending a properly-threaded patch series easier to achieve. -- Eric Blake ebl...@redhat.com+1-801-349-2682 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] PATCH 1/4: AppArmor updates
Why do we need special behaviour to skip files which don't exist ? If this is solely to keep the test suite happy, then IMHO the fix should be in the test suite. Runtime production code shouldn't have workarounds for test code. This is not to work around the test suite. The intent is to help people migrating from 0.8.3 to 0.8.3. When considering the following binary states: file exists, disk format is specified, ignoreOpenFailure and allowProbing, what is in trunk (ie ignoreOpenFailure=true) and this patch work the same in all cases except when the file does not exist and the format is not specified and allowProbing=false (the new default). With what is in trunk, this is an error condition, with my patch, the domain will start but the disk is not added to the profile. Throwing an error because the format isn't correct on a disk that doesn't exist seemed an odd side-effect when migrating. I can see arguments against this, and would like to commit the changes to tests/virt-aa-helper-test regardless. -- Jamie Strandboge | http://www.canonical.com signature.asc Description: This is a digitally signed message part -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] PATCH 1/4: AppArmor updates
On Mon, Aug 16, 2010 at 01:45:45PM -0500, Jamie Strandboge wrote: Why do we need special behaviour to skip files which don't exist ? If this is solely to keep the test suite happy, then IMHO the fix should be in the test suite. Runtime production code shouldn't have workarounds for test code. This is not to work around the test suite. The intent is to help people migrating from 0.8.3 to 0.8.3. When considering the following binary states: file exists, disk format is specified, ignoreOpenFailure and allowProbing, what is in trunk (ie ignoreOpenFailure=true) and this patch work the same in all cases except when the file does not exist and the format is not specified and allowProbing=false (the new default). With what is in trunk, this is an error condition, with my patch, the domain will start but the disk is not added to the profile. Throwing an error because the format isn't correct on a disk that doesn't exist seemed an odd side-effect when migrating. I can see arguments against this, and would like to commit the changes to tests/virt-aa-helper-test regardless. How can the domain start if the configured disk file doesn't exist on the host filesystem ? QEMU will try to open a non-existant file, fail, and abort. Failing on non-existant files when setting up the security profile doesn't change that, it just makes us report the problem to the user soon in the startup process. Daniel -- |: Red Hat, Engineering, London-o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :| |: http://autobuild.org-o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/3] Remove wrong check for uml monitor response size
On 16-08-2010 18:08, Eric Blake wrote: On 08/16/2010 03:54 AM, Soren Hansen wrote: 09:41:01.134: debug : umlMonitorCommand:698 : Run command 'config con0' 09:41:01.134: debug : umlMonitorCommand:733 : res.error: 6, res.extra: 0, res.length: 4096, res.data: 09:41:01.134: debug : umlMonitorCommand:736 : nbytes: 0 09:41:01.134: debug : umlMonitorCommand:738 : res.error: 0, res.extra: 0, res.length: 4, res.data: pts This one's a 2.6.34.1 kernel. The one where I didn't see the problem is a 2.6.32.something-or-the-other kernel. Mindboggling. Oh my. This really does look like a kernel bug. Can you confirm it with an strace? Annoyingly, no. It doesn't happen in the primary libvirt thread, so I have to pass -f to strace, and uml doesn't let you PTRACE it, so I can't trigger the problem. Have you reported this regression to the right kernel folks? It doesn't happen in 2.6.35, so I seems isolated to that particular kernel. It's really very, very odd. I guess it would help if we could write a simpler test program to isolate whether this recvfrom bug exists in a bare minimum number of syscalls. Meanwhile, I have no idea how to work around a buggy recvfrom that doesn't return the correct number of bytes. No, you're right. Whatever we do to work around this is going to suck somehow. -- Soren Hansen Ubuntu Developer http://www.ubuntu.com/ 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 2/4: AppArmor updates
On Mon, 2010-08-16 at 17:14 +0100, Daniel P. Berrange wrote: You can't blindly de-reference data.file.path - The 'file' struct is inside a union and is only valid for certain types of character device VIR_DOMAIN_CHR_TYPE_PTY, TYPE_DEV, TYPE_FILE and TYPE_PIPE. The existing code for serial devices is broken too can crash due to this Hrmm. I always did it this. Maybe something changed and I missed it. Regardless, thanks for this feedback! :) Attached is an updated patch. -- Jamie Strandboge | http://www.canonical.com Author: Jamie Strandboge ja...@canonical.com Description: Check for VIR_DOMAIN_CHR_TYPE in serial ports and add 'rw' for defined serial ports, parallel ports and channels Bug-Ubuntu: LP: #578527, LP: #609055 Index: libvirt-0.8.3/src/security/virt-aa-helper.c === --- libvirt-0.8.3.orig/src/security/virt-aa-helper.c 2010-08-16 14:10:42.0 -0500 +++ libvirt-0.8.3/src/security/virt-aa-helper.c 2010-08-16 14:14:15.0 -0500 @@ -875,15 +875,44 @@ } for (i = 0; i ctl-def-nserials; i++) -if (ctl-def-serials[i] ctl-def-serials[i]-data.file.path) +if (ctl-def-serials[i] +(ctl-def-serials[i]-type == VIR_DOMAIN_CHR_TYPE_PTY || + ctl-def-serials[i]-type == VIR_DOMAIN_CHR_TYPE_DEV || + ctl-def-serials[i]-type == VIR_DOMAIN_CHR_TYPE_FILE || + ctl-def-serials[i]-type == VIR_DOMAIN_CHR_TYPE_PIPE) +ctl-def-serials[i]-data.file.path) if (vah_add_file(buf, - ctl-def-serials[i]-data.file.path, w) != 0) + ctl-def-serials[i]-data.file.path, rw) != 0) goto clean; if (ctl-def-console ctl-def-console-data.file.path) -if (vah_add_file(buf, ctl-def-console-data.file.path, w) != 0) +if (vah_add_file(buf, ctl-def-console-data.file.path, rw) != 0) goto clean; +for (i = 0 ; i ctl-def-nparallels; i++) +if (ctl-def-parallels[i] +(ctl-def-parallels[i]-type == VIR_DOMAIN_CHR_TYPE_PTY || + ctl-def-parallels[i]-type == VIR_DOMAIN_CHR_TYPE_DEV || + ctl-def-parallels[i]-type == VIR_DOMAIN_CHR_TYPE_FILE || + ctl-def-parallels[i]-type == VIR_DOMAIN_CHR_TYPE_PIPE) +ctl-def-parallels[i]-data.file.path) +if (vah_add_file(buf, + ctl-def-parallels[i]-data.file.path, + rw) != 0) +goto clean; + +for (i = 0 ; i ctl-def-nchannels; i++) +if (ctl-def-channels[i] +(ctl-def-channels[i]-type == VIR_DOMAIN_CHR_TYPE_PTY || + ctl-def-channels[i]-type == VIR_DOMAIN_CHR_TYPE_DEV || + ctl-def-channels[i]-type == VIR_DOMAIN_CHR_TYPE_FILE || + ctl-def-channels[i]-type == VIR_DOMAIN_CHR_TYPE_PIPE) +ctl-def-channels[i]-data.file.path) +if (vah_add_file(buf, + ctl-def-channels[i]-data.file.path, + rw) != 0) +goto clean; + if (ctl-def-os.kernel) if (vah_add_file(buf, ctl-def-os.kernel, r) != 0) goto clean; Index: libvirt-0.8.3/tests/virt-aa-helper-test === --- libvirt-0.8.3.orig/tests/virt-aa-helper-test 2010-08-16 14:10:42.0 -0500 +++ libvirt-0.8.3/tests/virt-aa-helper-test 2010-08-16 14:10:42.0 -0500 @@ -246,6 +246,9 @@ cat $template_xml | sed s,###UUID###,$uuid,g | sed s,###DISK###,$disk1,g | sed s,/devices,serial type='pty'target port='0'//serial/devices,g $test_xml testme 0 serial (pty) -r -u $valid_uuid $test_xml +cat $template_xml | sed s,###UUID###,$uuid,g | sed s,###DISK###,$disk1,g | sed s,/devices,serial type='dev'source path='/dev/ttyS0'/target port='0'//serial/devices,g $test_xml +testme 0 serial (dev) -r -u $valid_uuid $test_xml + cat $template_xml | sed s,###UUID###,$uuid,g | sed s,###DISK###,$disk1,g | sed s,/devices,console type='file'source path='$tmpdir/console.log'/target port='0'//console/devices,g $test_xml touch $tmpdir/console.log testme 0 console -r -u $valid_uuid $test_xml @@ -253,6 +256,16 @@ cat $template_xml | sed s,###UUID###,$uuid,g | sed s,###DISK###,$disk1,g | sed s,/devices,console type='pty'target port='0'//console/devices,g $test_xml testme 0 console (pty) -r -u $valid_uuid $test_xml +cat $template_xml | sed s,###UUID###,$uuid,g | sed s,###DISK###,$disk1,g | sed s,/devices,parallel type='pty'source path='/dev/pts/0'/target port='0'//parallel/devices,g $test_xml +testme 0 parallel (pty) -r -u $valid_uuid $test_xml + +cat $template_xml | sed s,###UUID###,$uuid,g | sed s,###DISK###,$disk1,g | sed s,/devices,channel type='unix'source mode='bind' path='$tmpdir/guestfwd'/target type='guestfwd' address='10.0.2.1' port='4600'//channel/devices,g $test_xml +touch $tmpdir/guestfwd
Re: [libvirt] PATCH 3/4: AppArmor updates
On Mon, 2010-08-16 at 17:15 +0100, Daniel P. Berrange wrote:
On Fri, Aug 13, 2010 at 05:00:06PM -0500, Jamie Strandboge wrote:
Attached is 0003-apparmor-examples.patch
Can you include full commit messages with each patch,
since it makes it easier to review understand, and
will be needed when the patches are applied to GIT.
Certainly, and I apologize. Attached is an updated patch with messages.
--
Jamie Strandboge | http://www.canonical.com
Author: Jamie Strandboge ja...@canonical.com
Description: AppArmor example profile adjustments:
- libvirt-qemu: allow guests setgid and setuid so qemu can drop privileges
- virt-aa-helper:
+ allow access to @{PROC}/[0-9]*/net/psched
+ allow searching /sys/bus/usb/devices/
+ deny access to /dev to suppress confusing, non-fatal profile denials
+ allow access to user-tmp abstraction
Bug-Ubuntu: LP: #579584, LP: #565691
diff -Naurp libvirt.orig/examples/apparmor/libvirt-qemu libvirt/examples/apparmor/libvirt-qemu
--- libvirt.orig/examples/apparmor/libvirt-qemu 2010-04-06 16:14:52.0 -0500
+++ libvirt/examples/apparmor/libvirt-qemu 2010-08-13 16:46:34.0 -0500
@@ -1,4 +1,4 @@
-# Last Modified: Mon Apr 5 15:11:27 2010
+# Last Modified: Fri Aug 13 16:38:32 2010
#include abstractions/base
#include abstractions/consoles
@@ -9,6 +9,10 @@
capability dac_read_search,
capability chown,
+ # needed to drop privileges
+ capability setgid,
+ capability setuid,
+
network inet stream,
network inet6 stream,
diff -Naurp libvirt.orig/examples/apparmor/usr.lib.libvirt.virt-aa-helper libvirt/examples/apparmor/usr.lib.libvirt.virt-aa-helper
--- libvirt.orig/examples/apparmor/usr.lib.libvirt.virt-aa-helper 2010-04-06 16:14:52.0 -0500
+++ libvirt/examples/apparmor/usr.lib.libvirt.virt-aa-helper 2010-08-13 16:44:01.0 -0500
@@ -1,8 +1,9 @@
-# Last Modified: Mon Apr 5 15:10:27 2010
+# Last Modified: Fri Aug 13 16:38:32 2010
#include tunables/global
/usr/lib/libvirt/virt-aa-helper {
#include abstractions/base
+ #include abstractions/user-tmp
# needed for searching directories
capability dac_override,
@@ -12,11 +13,16 @@
network inet,
deny @{PROC}/[0-9]*/mounts r,
+ @{PROC}/[0-9]*/net/psched r,
@{PROC}/filesystems r,
# for hostdev
/sys/devices/ r,
/sys/devices/** r,
+ /sys/bus/usb/devices/ r,
+ deny /dev/sd* r,
+ deny /dev/mapper/ r,
+ deny /dev/mapper/* r,
/usr/lib/libvirt/virt-aa-helper mr,
/sbin/apparmor_parser Ux,
@@ -24,8 +30,11 @@
/etc/apparmor.d/libvirt/* r,
/etc/apparmor.d/libvirt/libvirt-[0-9a-f]*-[0-9a-f]*-[0-9a-f]*-[0-9a-f]*-[0-9a-f]* rw,
- # for backingstore -- allow access to non-hidden files in @{HOME} as well
- # as storage pools
+ # For backingstore, virt-aa-helper may need to peek inside the disk image, so
+ # allow access to non-hidden files in @{HOME} as well as storage pools, and
+ # removable media and filesystems, and certain file extentions. A
+ # virt-aa-helper failure when checking a disk for backinsgstore is non-fatal
+ # (but obviously the backingstore won't be added).
audit deny @{HOME}/.* mrwkl,
audit deny @{HOME}/.*/ rw,
audit deny @{HOME}/.*/** mrwkl,
signature.asc
Description: This is a digitally signed message part
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/3] qemu: Several PCI device assignment fixes
qemu: Re-reserve all PCI addresses on libvirtd restart qemu: Release PCI slot when detaching disk and net devices qemu: Fix copypaste error in warning message Thanks for the review. I pushed all these patches. Jirka -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/3] Remove wrong check for uml monitor response size
On 08/16/2010 03:37 AM, Soren Hansen wrote: nbytes = recvfrom(priv-monitor, res, sizeof res, 0, (struct sockaddr *)addr, addrlen); +VIR_DEBUG(nbytes: %d, nbytes); User error, not a kernel bug. Phew. %d and ssize_t are not always compatible sizes. -- Eric Blake ebl...@redhat.com+1-801-349-2682 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/3] Remove wrong check for uml monitor response size
On 16-08-2010 18:08, Eric Blake wrote:
Ok, found the other kernel. Same diff as in my previous e-mail, same
action. These are my results:
09:41:01.134: debug : umlMonitorCommand:698 : Run command 'config con0'
09:41:01.134: debug : umlMonitorCommand:733 : res.error: 6, res.extra: 0,
res.length: 4096, res.data:
09:41:01.134: debug : umlMonitorCommand:736 : nbytes: 0
09:41:01.134: debug : umlMonitorCommand:738 : res.error: 0, res.extra: 0,
res.length: 4, res.data: pts
*blush*
Ok, we can officially stop chasing this now. While working on some of
the other patches I've sent, I apparantly managed to copy a bit of code
from one place to another and by luck it was syntactically correct, so
the compiler didn't complain. The offending *two characters* were:
nbytes = recvfrom(priv-monitor, res, sizeof res, 0,
- (struct sockaddr *)addr, addrlen);
+ (struct sockaddr *)addr, addrlen)0;
if (nbytes 0) {
Henceforth, nbytes was 0.
Sorry about the wasted brain cycles on this.
--
Soren Hansen
Ubuntu Developer
http://www.ubuntu.com/
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 3/3] Remove wrong check for uml monitor response size
On 16-08-2010 18:04, Eric Blake wrote: So the size of the response datagram isn't sizeof(res) as the check in uml_driver.c expects, but rather sizeof(res.error) + sizeof(res.extra) + sizeof(res.length) + res.length. I agree with this analysis. In other words, the check should be more like this (two conditions - did we get enough bytes to even have a valid res.length, and did we get enough bytes to match with what res.length stated): if (nbytes offsetof(struct monitor_request, data) || nbytes res.length + offsetof(struct monitor_request, data)) incomplete reply Yup, this looks good. But before I write such a patch, I'm going to look in more details at your other reply. Let's just forget all about that one, shall we? Please? :) -- Soren Hansen Ubuntu Developer http://www.ubuntu.com/ signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] cygwin: build fix
Fixing a problem in the build on cygwin due to missing #define's.
Signed-off-by: Stefan Berger stef...@us.ibm.com
---
src/remote/remote_protocol.c |6 ++
src/remote/remote_protocol.h |6 ++
src/remote/remote_protocol.x |6 ++
3 files changed, 18 insertions(+)
Index: libvirt-acl/src/remote/remote_protocol.h
===
--- libvirt-acl.orig/src/remote/remote_protocol.h
+++ libvirt-acl/src/remote/remote_protocol.h
@@ -24,6 +24,12 @@ extern C {
#ifndef IXDR_GET_INT32
# define IXDR_GET_INT32 IXDR_GET_LONG
#endif
+#ifndef IXDR_PUT_U_INT32
+# define IXDR_PUT_U_INT32 IXDR_PUT_U_LONG
+#endif
+#ifndef IXDR_GET_U_INT32
+# define IXDR_GET_U_INT32 IXDR_GET_U_LONG
+#endif
#define REMOTE_MESSAGE_MAX 262144
#define REMOTE_MESSAGE_HEADER_MAX 24
#define REMOTE_MESSAGE_PAYLOAD_MAX 262120
Index: libvirt-acl/src/remote/remote_protocol.x
===
--- libvirt-acl.orig/src/remote/remote_protocol.x
+++ libvirt-acl/src/remote/remote_protocol.x
@@ -51,6 +51,12 @@
%#ifndef IXDR_GET_INT32
%# define IXDR_GET_INT32 IXDR_GET_LONG
%#endif
+%#ifndef IXDR_PUT_U_INT32
+%# define IXDR_PUT_U_INT32 IXDR_PUT_U_LONG
+%#endif
+%#ifndef IXDR_GET_U_INT32
+%# define IXDR_GET_U_INT32 IXDR_GET_U_LONG
+%#endif
/*- Data types. -*/
Index: libvirt-acl/src/remote/remote_protocol.c
===
--- libvirt-acl.orig/src/remote/remote_protocol.c
+++ libvirt-acl/src/remote/remote_protocol.c
@@ -16,6 +16,12 @@
#ifndef IXDR_GET_INT32
# define IXDR_GET_INT32 IXDR_GET_LONG
#endif
+#ifndef IXDR_PUT_U_INT32
+# define IXDR_PUT_U_INT32 IXDR_PUT_U_LONG
+#endif
+#ifndef IXDR_GET_U_INT32
+# define IXDR_GET_U_INT32 IXDR_GET_U_LONG
+#endif
bool_t
xdr_remote_nonnull_string (XDR *xdrs, remote_nonnull_string *objp)
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] cygwin: build fix
On 08/16/2010 02:20 PM, Stefan Berger wrote: Fixing a problem in the build on cygwin due to missing #define's. Signed-off-by: Stefan Berger stef...@us.ibm.com --- src/remote/remote_protocol.c |6 ++ src/remote/remote_protocol.h |6 ++ src/remote/remote_protocol.x |6 ++ 3 files changed, 18 insertions(+) ACK. (One of these days, I hope to get around to patching cygwin's rpc/xdr.h to provide these aliases - but that is orthogonal to this patch). -- Eric Blake ebl...@redhat.com+1-801-349-2682 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] PATCH 1/4: AppArmor updates
On Mon, 2010-08-16 at 20:11 +0100, Daniel P. Berrange wrote:
How can the domain start if the configured disk file doesn't exist on
the host filesystem ? QEMU will try to open a non-existant file, fail,
and abort. Failing on non-existant files when setting up the security
profile doesn't change that, it just makes us report the problem to the
user soon in the startup process.
I got mixed up thinking of the case when the disk does exist but the
format was not originally specified (and therefore now defaults to raw)
but the disk is non-raw. In that case, the domain starts and POSTs, but
there is no disk to boot off of.
I tested this quite a bit more and you are correct that virt-aa-helper
does not have to be adjusted. I've attached an updated patch which only
adds the new test cases for the -p option.
Thanks again for your review.
--
Jamie Strandboge | http://www.canonical.com
Author: Jamie Strandboge ja...@canonical.com
Description: add extra tests to virt-aa-helper-test for new '-p' option
Index: libvirt-0.8.3/tests/virt-aa-helper-test
===
--- libvirt-0.8.3.orig/tests/virt-aa-helper-test 2010-08-12 09:51:53.0 -0500
+++ libvirt-0.8.3/tests/virt-aa-helper-test 2010-08-12 10:03:11.0 -0500
@@ -144,6 +144,7 @@
testme 1 invalid arg -z
testme 1 invalid case -A
testme 1 not enough args -c
+testme 1 not enough args -p
cat $template_xml | sed s,###UUID###,$uuid,g | sed s,###DISK###,$disk1,g $test_xml
testme 1 no -u with -c -c $test_xml
@@ -160,17 +161,25 @@
cat $template_xml | sed s,###UUID###,$uuid,g | sed s,###DISK###,$bad_disk,g $test_xml
testme 1 bad disk -c -u $valid_uuid $test_xml
-cat $template_xml | sed s,###UUID###,$uuid,g | sed s,###DISK###,$bad_disk,g | sed s,/devices,disk type='file' device='disk'source file='$disk2'/target dev='hda' bus='ide'//disk/devices,g $test_xml
+cat $template_xml | sed s,###UUID###,$uuid,g | sed s,###DISK###,$bad_disk,g | sed s,/devices,disk type='file' device='disk'driver name='qemu' type='raw'/source file='$disk2'/target dev='hda' bus='ide'//disk/devices,g $test_xml
+
testme 1 bad disk2 -c -u $valid_uuid $test_xml
cat $template_xml | sed s,###UUID###,$uuid,g | sed s,###DISK###,$disk1,g | sed s,/devices,devices,g $test_xml
testme 1 malformed xml -c -u $valid_uuid $test_xml
-cat $template_xml | sed s,###UUID###,$uuid,g | sed s,###DISK###,/boot/initrd,g $test_xml
-testme 1 disk in /boot -r -u $valid_uuid $test_xml
-
-cat $template_xml | sed s,###UUID###,$uuid,g | sed s,###DISK###,/boot/initrd,g $test_xml
-testme 1 -r with invalid -f -r -u $valid_uuid -f $bad_disk $test_xml
+initrd=`ls -1 /boot/initrd* | head -1`
+if [ -z $initrd ]; then
+echo Skipping /boot/initrd* tests. Could not find /boot/initrd*
+else
+cat $template_xml | sed s,###UUID###,$uuid,g | sed s,###DISK###,$initrd,g $test_xml
+testme 1 disk in /boot without probing -p 0 -r -u $valid_uuid $test_xml
+testme 1 disk in /boot with probing -p 1 -r -u $valid_uuid $test_xml
+
+cat $template_xml | sed s,###UUID###,$uuid,g | sed s,###DISK###,/boot/initrd,g $test_xml
+testme 1 -r with invalid -f with probing -p 1 -r -u $valid_uuid -f $bad_disk $test_xml
+testme 1 -r with invalid -f without probing -p 0 -r -u $valid_uuid -f $bad_disk $test_xml
+fi
cat $template_xml | sed s,###UUID###,$uuid,g | sed s,###DISK###,$disk1/disk,g $test_xml
testme 1 -c with malformed xml -c -u $valid_uuid $test_xml
@@ -195,8 +204,8 @@
cat $template_xml | sed s,###UUID###,$uuid,g | sed s,###DISK###,$disk1,g | sed s,arch='x86_64',arch='ppc',g $test_xml
testme 0 create (ppc) -c -u $valid_uuid $test_xml
-cat $template_xml | sed s,###UUID###,$uuid,g | sed s,###DISK###,$disk1,g | sed s,/disk,/diskdisk type='file' device='disk'source file='$disk2'/target dev='hdb' bus='ide'//disk,g $test_xml
-testme 0 create multiple disks -c -u $valid_uuid -p 1 $test_xml
+cat $template_xml | sed s,###UUID###,$uuid,g | sed s,###DISK###,$disk1,g | sed s,/disk,/diskdisk type='file' device='disk'driver name='qemu' type='raw'/source file='$disk2'/target dev='hdb' bus='ide'//disk,g $test_xml
+testme 0 create multiple disks -c -u $valid_uuid $test_xml
cat $template_xml | sed s,###UUID###,$uuid,g | sed s,###DISK###',${disk1}'/readonly,g $test_xml
testme 0 create (readonly) -c -u $valid_uuid $test_xml
signature.asc
Description: This is a digitally signed message part
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] uml: fix logic bug in checking reply length
* src/uml/uml_driver.c (umlMonitorCommand): Validate that enough
bytes were read to dereference both res.length, and that many
bytes from res.data.
Reported by Soren Hansen.
---
Yup, this looks good.
I'll wait for a third-party ack before pushing this.
Meanwhile, I discovered a bug in Solaris' implementation of
offsetof. POSIX requires that offsetof be an integer
constant expression, which means that:
sizeof offsetof(a,b)
should compile. But Solaris (and probably other buggy systems)
used too few parens in their macro, and you have to use:
sizeof(offsetof(a,b))
instead. Interesting trivia, but at least it doesn't affect
any of libvirt's existing uses of offsetof.
src/uml/uml_driver.c |7 ++-
1 files changed, 2 insertions(+), 5 deletions(-)
diff --git a/src/uml/uml_driver.c b/src/uml/uml_driver.c
index 04493ba..37ddc39 100644
--- a/src/uml/uml_driver.c
+++ b/src/uml/uml_driver.c
@@ -737,14 +737,11 @@ static int umlMonitorCommand(const struct uml_driver
*driver,
virReportSystemError(errno, _(cannot read reply %s), cmd);
goto error;
}
-if (nbytes sizeof res) {
+if (nbytes offsetof(struct monitor_request, data) ||
+nbytes res.length + offsetof(struct monitor_request, data)) {
virReportSystemError(0, _(incomplete reply %s), cmd);
goto error;
}
-if (sizeof res.data res.length) {
-virReportSystemError(0, _(invalid length in reply %s), cmd);
-goto error;
-}
if (VIR_REALLOC_N(retdata, retlen + res.length) 0) {
virReportOOMError();
--
1.7.2.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] KVM Forum 2010: presentations online
KVM Forum 2010 was quite a success, many thanks to all who participated! For those who couldn't attend, the presentations are available online now: (thanks to Andrew Cathrow for pushing them all up) http://www.linux-kvm.org/page/KVM_Forum_2010#Presentations We were also able to video the speakers, and will send a note when the videos are available. (and thanks again to Andrew Cathrow for making this happen) thanks, -chris -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
