[libvirt] [PATCH] qemuDomainGetSchedulerType: Prefer qemuDomObjFromDomain
In all qemu APIs we tend to prefer qemuDomObjFromDomain over
virDomainObjListFindByUUID. But somehow the
qemuDomainGetSchedulerType left unattended.
---
src/qemu/qemu_driver.c | 8 ++--
1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 4c90794..e2a94bc 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -7436,18 +7436,14 @@ cleanup:
static char *qemuDomainGetSchedulerType(virDomainPtr dom,
int *nparams)
{
-virQEMUDriverPtr driver = dom-conn-privateData;
char *ret = NULL;
int rc;
virDomainObjPtr vm = NULL;
qemuDomainObjPrivatePtr priv;
-vm = virDomainObjListFindByUUID(driver-domains, dom-uuid);
-if (vm == NULL) {
-virReportError(VIR_ERR_INTERNAL_ERROR,
- _(No such domain %s), dom-uuid);
+if (!(vm = qemuDomObjFromDomain(dom)))
goto cleanup;
-}
+
priv = vm-privateData;
if (virDomainGetSchedulerTypeEnsureACL(dom-conn, vm-def) 0)
--
1.8.1.5
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemuDomainGetSchedulerType: Prefer qemuDomObjFromDomain
On 07/17/2013 08:09 AM, Michal Privoznik wrote: In all qemu APIs we tend to prefer qemuDomObjFromDomain over virDomainObjListFindByUUID. But somehow the qemuDomainGetSchedulerType left unattended. --- src/qemu/qemu_driver.c | 8 ++-- 1 file changed, 2 insertions(+), 6 deletions(-) ACK Jan -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] Why does libvirt not unlink qemu-ga or any other socket in function qemuProcessStop()
Hello, I have defined a vm with two UNIX domain sockets. One is for qemu-ga and the other is for my service. These two devices will act as UNIX domain socket server. They are automatically generated by qemu. But when the vm was dead, these two sockets were not deleted. I find that libvirt will unlink vm's monitor socket(/var/lib/libvirt/qemu) in function qemuProcessStop(). Why does libvirt not unlink qemu-ga or any other socket in function qemuProcessStop()? xml file is given below. domain type='kvm' namekvm-1/name memory4194304/memory currentMemory4194304/currentMemory vcpu4/vcpu cpu topology sockets='1' cores='4' threads='1'/ /cpu os type arch='x86_64'hvm/type boot dev='hd'/ /os features acpi/ apic/ pae/ /features clock offset='utc' timer name='hpet' present='no'/ /clock on_poweroffdestroy/on_poweroff on_rebootrestart/on_reboot on_crashrestart/on_crash devices emulator/usr/bin/qemu-kvm/emulator disk type='file' device='disk' driver name='qemu' type='raw' cache='none' io='native'/ source file='/home/linux.img'/ target dev='sda' bus='virtio'/ /disk channel type='unix' source mode='bind' path='/var/run/libvirt/qemu/kvm-1'/ target type='virtio' name='org.qemu.guest_agent.1'/ address type='virtio-serial' controller='0' bus='0' port='1'/ /channel channel type='unix' source mode='bind' path='/var/run/libvirt/qemu/kvm-1.agent'/ target type='virtio' name='org.qemu.guest_agent.0'/ address type='virtio-serial' controller='0' bus='0' port='2'/ /channel interface type='bridge' source bridge='br0'/ mac address='00:22:3e:56:55:a7'/ model type='virtio'/ /interface input type='tablet' bus='usb'/ input type='mouse' bus='ps2'/ graphics type='vnc' autoport='yes' listen='0.0.0.0' listen type='address' address='0.0.0.0'/ /graphics video model type='cirrus' vram='9216' heads='1'/ /video memballon model='none'/ /devices /domain Thank you! -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [test-API][PATCH V2] Modify repos/network/network_list.py and add network_list case to conf
Modify the old network_list.py. The new network_list.py covers all
flags of listAllNetworks and the following api. and add network_list to
basic_network.conf.
virNetwork:
name()
bridgeName()
isActive()
isPersistent()
virConnect:
listAllNetworks()
Changes from V2 to V1 as follows
1.Change the flag in conf from digit to string
2.Remove the checking method using the result of virsh with flags
3.Add checking method via checking network autostart dir
4.Modify the basic_network.conf
---
cases/basic_network.conf | 35 -
repos/network/network_list.py | 277 ++---
2 files changed, 148 insertions(+), 164 deletions(-)
diff --git a/cases/basic_network.conf b/cases/basic_network.conf
index 991ad99..91d7f21 100644
--- a/cases/basic_network.conf
+++ b/cases/basic_network.conf
@@ -14,10 +14,18 @@ network:define
netmode
nat
+network:network_list
+flags
+ inactive
+
network:start
networkname
$defaultnetname
+network:network_list
+flags
+ active
+
network:autostart
networkname
$defaultnetname
@@ -28,6 +36,10 @@ network:destroy
networkname
$defaultnetname
+network:network_list
+flags
+ default
+
network:undefine
networkname
$defaultnetname
@@ -48,6 +60,10 @@ network:create
netmode
nat
+network:network_list
+flags
+transient
+
network:destroy
networkname
$defaultnetname
@@ -68,6 +84,10 @@ network:define
netmode
route
+network:network_list
+flags
+ persistent
+
network:start
networkname
$defaultnetname
@@ -106,7 +126,6 @@ network:destroy
networkname
$defaultnetname
-
network:define
networkname
$defaultnetname
@@ -127,12 +146,20 @@ network:start
networkname
$defaultnetname
+network:network_list
+flags
+ noautostart
+
network:autostart
networkname
$defaultnetname
autostart
enable
+network:network_list
+flags
+ autostart
+
network:destroy
networkname
$defaultnetname
@@ -141,7 +168,6 @@ network:undefine
networkname
$defaultnetname
-
network:create
networkname
$defaultnetname
@@ -162,8 +188,3 @@ network:destroy
networkname
$defaultnetname
-
-
-
-
-
diff --git a/repos/network/network_list.py b/repos/network/network_list.py
index 7c34f69..175d6fa 100644
--- a/repos/network/network_list.py
+++ b/repos/network/network_list.py
@@ -1,184 +1,147 @@
#!/usr/bin/env python
# To test virsh net-list command
-import os
-import sys
-import re
-import commands
-
import libvirt
from libvirt import libvirtError
from src import sharedmod
from utils import utils
-required_params = ('netlistopt',)
+required_params = ('flags',)
optional_params = {}
-VIRSH_QUIET_NETLIST = virsh --quiet net-list %s|awk '{print $1}'
-VIRSH_NETLIST = virsh net-list %s
-GET_BRIDGE_IP = /sbin/ifconfig %s | grep 'inet addr:' | cut -d: -f2 | awk
'{print $1}'
-CONFIG_DIR = /etc/libvirt/qemu/networks/
-
-def get_option_list(params):
-return options we need to test
-
-logger = params['logger']
-option_list=[]
-
-value = params['netlistopt']
-
-if value == 'all':
-option_list = [' ', '--all', '--inactive']
-elif value == '--all' or value == '--inactive':
-option_list.append(value)
+VIRSH_NETWORK_LIST = virsh net-list %s|sed -n '3,$'p|awk '{print $1}'
+GET_BRIDGE_IP = /sbin/ifconfig %s | grep 'inet addr:' | cut -d: -f2 | awk \
+'{print $1}'
+LS_NETWORK_DIR = ls /etc/libvirt/qemu/networks/
+LS_AUTOSTART_NET = ls /etc/libvirt/qemu/networks/autostart/
+
+def check_bridge_ip(bridgename):
+ Check if the bridge has ip
+
+(status, output) = utils.exec_cmd(GET_BRIDGE_IP % bridgename,\
+ shell=True)
+if not status and utils.do_ping(output[0], 50):
+logger.info(Bridge %s is active % bridgename)
+logger.info(%s has ip: %s % (bridgename, output[0]))
+return True
else:
-logger.error(value %s is not supported % value)
-return 1, option_list
-
-return 0, option_list
+logger.error(Bridge %s has no ip or fails to ping % bridgename)
+return False
-def get_output(logger, command, flag):
-execute shell command
+def check_persistent_netxml(networkname):
-status, ret = commands.getstatusoutput(command)
-if not flag and status:
-logger.error(executing + \ + command + \ + failed)
-logger.error(ret)
-return status, ret
-
-def check_all_option(conn, logger):
-check the output of virsh net-list with --all option
+Check if the network is persistent via checking network xml dir
+if the network is persistent, return True, or return False
-all_network = []
-entries = os.listdir(CONFIG_DIR)
-logger.debug(%s in %s % (entries, CONFIG_DIR))
-status,
[libvirt] [PATCH] lxc_container: Don't call virGetGroupList during exec
Commit 75c1256 states that virGetGroupList must not be called
between fork and exec, then commit ee777e99 promptly violated
that for lxc.
Patch originally posted by Eric Blake ebl...@redhat.com.
---
src/lxc/lxc_container.c | 8 +---
1 file changed, 1 insertion(+), 7 deletions(-)
diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c
index b51d7a2..37d2ba6 100644
--- a/src/lxc/lxc_container.c
+++ b/src/lxc/lxc_container.c
@@ -351,24 +351,18 @@ int lxcContainerWaitForContinue(int control)
*/
static int lxcContainerSetID(virDomainDefPtr def)
{
-gid_t *groups;
-int ngroups;
-
/* Only call virSetUIDGID when user namespace is enabled
* for this container. And user namespace is only enabled
* when nuidmapngidmap is not zero */
VIR_DEBUG(Set UID/GID to 0/0);
if (def-idmap.nuidmap
-((ngroups = virGetGroupList(0, 0, groups) 0) ||
- virSetUIDGID(0, 0, groups, ngroups) 0)) {
+virSetUIDGID(0, 0, groups, ngroups) 0) {
virReportSystemError(errno, %s,
_(setuid or setgid failed));
-VIR_FREE(groups);
return -1;
}
-VIR_FREE(groups);
return 0;
}
--
1.8.1.5
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3 1/3] libxl: implement NUMA capabilities reporting
On mar, 2013-07-16 at 11:18 +0100, Daniel P. Berrange wrote: On Tue, Jul 16, 2013 at 11:10:25AM +0100, Dario Faggioli wrote: There are indeed a couple of possible reasons. Actually, I saw that the qemu driver does pretty much the same, i.e., if retrieving NUMA information fails, it gives up on that, but does not make things explode, and I really think it is something that makes sense. The reason the QEMU driver does that is that libnuma will return an error if the host machine does not expose NUMA info in its BIOS. This is an expected, valid scenario, so we have to ignore the error and libnuma provides no way to distinguish this valid scenario from other errors. Ok, I see. The actual possible failure reasons are: (1) it cannot prepare the parameters for the hypercall, or (2) the hypercall fails. It is true that, in both cases, something really serious might have happened, but there is no way to tell it from here. Thus, I honestly think that trying to carry on is sound... If it is really the case that some critical component died, we'll find out soon enough. The only scenario in which it is acceptable to ignore the failure is if the physical hardware does not support NUMA. The question is whether the Xen API lets you distinguish that scenario, from other types of errors. Mmm... I see what you mean. Well, I guess it depends on what you mean by 'not support'. If we're talking about a box with only 1 NUMA node, and call it a non-NUMA box, then libxl will just tell you that there is only that 1 node, and that is already being taken care of properly (see Jim's testing of the series). If host machine does not expose NUMA info in its BIOS means something different than that, then I've just never put hands on one of such machine, hence I really am not sure what Xen would tell libxl about them. Anyway, it really looks to me too now that I should not special case errors happening in libxl_get_numainfo(), and treat them as all the other ones. I'll do that. Thanks again and Regards, Dario -- This happens because I choose it to happen! (Raistlin Majere) - Dario Faggioli, Ph.D, http://about.me/dario.faggioli Senior Software Engineer, Citrix Systems RD Ltd., Cambridge (UK) 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 v4 2/4] storage: report error rather than warning if backing files doesn't exist
On Wed, Jul 17, 2013 at 10:08:55AM +0800, Guannan Ren wrote:
On 07/16/2013 04:40 PM, Daniel P. Berrange wrote:
On Tue, Jul 16, 2013 at 08:14:54AM +0200, Martin Kletzander wrote:
On 07/15/2013 05:31 PM, Ján Tomko wrote:
On 07/02/2013 11:35 AM, Guannan Ren wrote:
If one of backing files for disk source doesn't exist, the guest will not
be able to find and use the disk even though the disk still exists in
guest xml definition. So reporting an error make more sense.
Adding virFileAccessibleAs() to check if the backing file described in
disk meta exist in real path. If not, report error. the uid and gid
arguments don't have so much meannings for F_OK, so give 0 for them.
---
src/util/virstoragefile.c | 23 +++
tests/virstoragetest.c| 16
2 files changed, 23 insertions(+), 16 deletions(-)
diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
index 27aa4fe..cb61e5b 100644
--- a/src/util/virstoragefile.c
+++ b/src/util/virstoragefile.c
@@ -870,14 +877,10 @@ virStorageFileGetMetadataInternal(const char *path,
!!directory, backing,
meta-directory,
meta-backingStore) 0) {
-/* the backing file is (currently) unavailable,
treat this
- * file as standalone:
- * backingStoreRaw is kept to mark broken image
chains */
-meta-backingStoreIsFile = false;
-backingFormat = VIR_STORAGE_FILE_NONE;
-VIR_WARN(Backing file '%s' of image '%s' is
missing.,
- meta-backingStoreRaw, path);
-
+VIR_ERROR(_(Backing file '%s' of image '%s' is
missing.),
+ meta-backingStoreRaw, path);
+VIR_FREE(backing);
+goto cleanup;
}
}
VIR_FREE(backing);
This change means you won't be able to start the pool if one of the files
is
missing a backing file. I've forwarded a patch [1] from/for [2] that
ignores
missing files on pool start and there is a bug [3] requesting that we
ignore
other files as well. I feel like this is going in the other direction.
Wouldn't it be enough to check for them on domain start-up and leave the
pool
running even if one of those volumes doesn't have an existing backing file?
How about making it configurable for the pool? There are definitely
some users who want the pool to reflect actual info after pool-refresh.
I don't think this needs to be configurable. The pool should show *every*
single file, regardless of whether the file has a broken backing file.
We shouldn't be trying to second guess what the user wants to do with a
image with broken backing file. Just expose as much info as we have and
let them deal with the problem.
Daniel,
How about for guest cold bootup in this situation where one of its
disks has a broken file
we should throw an error out or give a warn in the log as what it is
doing currently.
If we can't access the file on guest boot, then we need to raise errors,
since it wouldn't be possible to set labelling or permissions. Of course
we have to make sure we don't give bogus errors in the case of root
squash NFS too. We need to consider storage pool enumeration separately
from guest bootup though. Both have specific semantics and we shouldn't
force them to behave the same way - the API needs to serve both their
needs.
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] lxc_container: Don't call virGetGroupList during exec
On Wed, Jul 17, 2013 at 11:28:42AM +0200, Michal Privoznik wrote:
Commit 75c1256 states that virGetGroupList must not be called
between fork and exec, then commit ee777e99 promptly violated
that for lxc.
Patch originally posted by Eric Blake ebl...@redhat.com.
---
src/lxc/lxc_container.c | 8 +---
1 file changed, 1 insertion(+), 7 deletions(-)
diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c
index b51d7a2..37d2ba6 100644
--- a/src/lxc/lxc_container.c
+++ b/src/lxc/lxc_container.c
@@ -351,24 +351,18 @@ int lxcContainerWaitForContinue(int control)
*/
static int lxcContainerSetID(virDomainDefPtr def)
{
-gid_t *groups;
-int ngroups;
-
/* Only call virSetUIDGID when user namespace is enabled
* for this container. And user namespace is only enabled
* when nuidmapngidmap is not zero */
VIR_DEBUG(Set UID/GID to 0/0);
if (def-idmap.nuidmap
-((ngroups = virGetGroupList(0, 0, groups) 0) ||
- virSetUIDGID(0, 0, groups, ngroups) 0)) {
+virSetUIDGID(0, 0, groups, ngroups) 0) {
How does this compile ? You're removing the 'groups' and 'ngroups'
variables but still referencing them here. Don't you mean to use
NULL, 0 as the args for virSetUIDGID }
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] qemu_conf: Properly mark driver-config as self-locking APIs
On Tue, Jul 16, 2013 at 08:25:25PM +0200, Michal Privoznik wrote:
Since a9e97e0c we don't require driver-config, or
virQEMUDriverGetConfig() to be called with locked QEMU driver. In fact,
calling it with the driver locked would cause deadlock. Hence, the
annotation to the struct field is not right.
---
src/qemu/qemu_conf.h | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h
index 8229cfc..fd5cc57 100644
--- a/src/qemu/qemu_conf.h
+++ b/src/qemu/qemu_conf.h
@@ -160,8 +160,7 @@ struct _virQEMUDriverConfig {
struct _virQEMUDriver {
virMutex lock;
-/* Require lock to get reference on 'config',
- * then lockless thereafter */
+/* Immutable pointer, self-locking APIs */
virQEMUDriverConfigPtr config;
Actually, it is 'immutable pointer, immutable+lockless data', since
we don't require any locks to be held in order to access fields in
the config object.
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] cgroup: Free line even if no characters were read
On 07/17/2013 04:03 AM, Eric Blake wrote:
On 07/16/2013 06:14 AM, Ján Tomko wrote:
Even if getline doesn't read any characters it allocates a buffer.
==404== 120 bytes in 1 blocks are definitely lost in loss record 1,344 of
1,671
==404==at 0x4C2C71B: malloc (in
/usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==404==by 0x906F862: getdelim (iogetdelim.c:68)
==404==by 0x52A48FB: virCgroupPartitionNeedsEscaping (vircgroup.c:1136)
==404==by 0x52A0FB4: virCgroupPartitionEscape (vircgroup.c:1171)
==404==by 0x52A0EA4: virCgroupNewDomainPartition (vircgroup.c:1450)
Introduced by f366273.
---
Can STRPREFIX(path, line) be possibly true if tmp is NULL?
path[NULL - line] would be accessed in that case.
[1] good question; answer below (you didn't ask quite the right
question, but it made me read the code - both the original code and this
patch are broken; we need a v2).
src/util/vircgroup.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c
index 5a98393..2419d80 100644
--- a/src/util/vircgroup.c
+++ b/src/util/vircgroup.c
@@ -1136,38 +1136,38 @@ static int virCgroupPartitionNeedsEscaping(const
char *path)
while (getline(line, len, fp) 0) {
if (STRPREFIX(line, #subsys_name)) {
VIR_FREE(line);
This VIR_FREE(line) is spurious, and should be deleted. The whole point
of getline() is that it reuses a malloc'd buffer, possibly realloc()ing
it to be larger, to minimize the malloc overhead. But by freeing every
iteration of the loop, we've lost that advantage.
continue;
}
char *tmp = strchr(line, ' ');
if (tmp)
*tmp = '\0';
len = tmp - line;
This is bogus. If tmp is NULL, then len is extremely large, and will
mess up the next iteration call to getline(). However, I could live with:
It doesn't mess it up right now, since we set line to NULL every time.
if (tmp) {
*tmp = '\0';
len = tmp - line;
}
which is slightly sub-optimal (it makes the next getline() call think it
needs to call realloc(), when it really might already be large enough),
but otherwise harmless (realloc() doesn't pay attention to *len, and is
smart enough to be a no-op if the new size is no bigger than the
existing real size).
This would leave len untouched if no space was found and tmp is NULL..
if (STRPREFIX(path, line)
[1] To answer your question, STRPREFIX() is unsafe to call on NULL (it
is a macro that boils down to strncmp, which must NOT have null
arguments). But we are guaranteed that path and line are non-null here.
path[len] == '.') {
.. and path[len] would off by one instead of ~2^64.
ret = 1;
-VIR_FREE(line);
goto cleanup;
}
VIR_FREE(line);
This is another VIR_FREE that could be safely nuked, given the semantics
of readline().
}
if (ferror(fp)) {
ret = -EIO;
goto cleanup;
}
cleanup:
+VIR_FREE(line);
Yes, I agree that we need this, but we also need to fix the other bugs
in the area. Looking forward to v2.
I've sent v2 as 'cgroup: reuse buffer for getline'
Jan
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2] cgroup: reuse buffer for getline
Reuse the buffer for getline and track buffer allocation
separately from the string length to prevent unlikely
out-of-bounds memory access.
This fixes the following leak that happened when zero bytes were read:
==404== 120 bytes in 1 blocks are definitely lost in loss record 1,344 of 1,671
==404==at 0x4C2C71B: malloc (in
/usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==404==by 0x906F862: getdelim (iogetdelim.c:68)
==404==by 0x52A48FB: virCgroupPartitionNeedsEscaping (vircgroup.c:1136)
==404==by 0x52A0FB4: virCgroupPartitionEscape (vircgroup.c:1171)
==404==by 0x52A0EA4: virCgroupNewDomainPartition (vircgroup.c:1450)
---
v1: cgroup: Free line even if no characters were read
https://www.redhat.com/archives/libvir-list/2013-July/msg01030.html
src/util/vircgroup.c | 20 ++--
1 file changed, 10 insertions(+), 10 deletions(-)
diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c
index 5a98393..9dfe98d 100644
--- a/src/util/vircgroup.c
+++ b/src/util/vircgroup.c
@@ -1098,13 +1098,13 @@ cleanup:
#if defined HAVE_MNTENT_H defined HAVE_GETMNTENT_R
static int virCgroupPartitionNeedsEscaping(const char *path)
{
FILE *fp = NULL;
int ret = 0;
char *line = NULL;
-size_t len;
+size_t buflen;
/* If it starts with 'cgroup.' or a '_' of any
* of the controller names from /proc/cgroups,
* then we must prefix a '_'
*/
if (STRPREFIX(path, cgroup.))
@@ -1130,37 +1130,37 @@ static int virCgroupPartitionNeedsEscaping(const char
*path)
* cpuacct 3 48 1
* memory 4 4 1
* devices 5 4 1
* freezer 6 4 1
* net_cls 7 1 1
*/
-while (getline(line, len, fp) 0) {
-if (STRPREFIX(line, #subsys_name)) {
-VIR_FREE(line);
+while (getline(line, buflen, fp) 0) {
+char *tmp;
+size_t len;
+
+if (STRPREFIX(line, #subsys_name))
continue;
-}
-char *tmp = strchr(line, ' ');
-if (tmp)
-*tmp = '\0';
+
+tmp = strchrnul(line, ' ');
+*tmp = '\0';
len = tmp - line;
if (STRPREFIX(path, line)
path[len] == '.') {
ret = 1;
-VIR_FREE(line);
goto cleanup;
}
-VIR_FREE(line);
}
if (ferror(fp)) {
ret = -EIO;
goto cleanup;
}
cleanup:
+VIR_FREE(line);
VIR_FORCE_FCLOSE(fp);
return ret;
}
static int virCgroupPartitionEscape(char **path)
{
--
1.8.1.5
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/2] lxcCapsInit: Allocate primary security driver unconditionally
On Mon, Jul 15, 2013 at 03:58:27PM +0200, Michal Privoznik wrote:
Currently, if the primary security driver is 'none', we skip
initializing caps-host.secModels. This means, later, when LXC domain
XML is parsed and seclabel type='none'/ is found (see
virSecurityLabelDefsParseXML), the model name is not copied to the
seclabel. This leads to subsequent crash in virSecurityManagerGenLabel
where we call STREQ() over the model (note, that we are expecting model
to be !NULL).
---
src/lxc/lxc_conf.c | 18 --
1 file changed, 8 insertions(+), 10 deletions(-)
diff --git a/src/lxc/lxc_conf.c b/src/lxc/lxc_conf.c
index 4e859c5..78b1559 100644
--- a/src/lxc/lxc_conf.c
+++ b/src/lxc/lxc_conf.c
@@ -114,16 +114,14 @@ virCapsPtr lxcCapsInit(virLXCDriverPtr driver)
doi = virSecurityManagerGetDOI(driver-securityManager);
model = virSecurityManagerGetModel(driver-securityManager);
-if (STRNEQ(model, none)) {
-/* Allocate just the primary security driver for LXC. */
-if (VIR_ALLOC(caps-host.secModels) 0)
-goto error;
-caps-host.nsecModels = 1;
-if (VIR_STRDUP(caps-host.secModels[0].model, model) 0)
-goto error;
-if (VIR_STRDUP(caps-host.secModels[0].doi, doi) 0)
-goto error;
-}
+/* Allocate the primary security driver for LXC. */
+if (VIR_ALLOC(caps-host.secModels) 0)
+goto error;
+caps-host.nsecModels = 1;
+if (VIR_STRDUP(caps-host.secModels[0].model, model) 0)
+goto error;
+if (VIR_STRDUP(caps-host.secModels[0].doi, doi) 0)
+goto error;
VIR_DEBUG(Initialized caps for security driver \%s\ with
DOI \%s\, model, doi);
The QEMU driver does not have any special handling of the none sec
model type. So I'm inclined to say that removing the special handling
from LXC is good from the POV of consistency.
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 2/2] virSecurityManagerGenLabel: Skip seclabels without model
On Mon, Jul 15, 2013 at 03:58:28PM +0200, Michal Privoznik wrote:
While generating seclabels, we check the seclabel stack if required
driver is in the stack. If not, an error is returned. However, it is
possible for a seclabel to not have any model set (happens with LXC
domains that have just seclabel type='none'). If that's the case,
we should just skip the iteration instead of calling STREQ(NULL, ...)
and SIGSEGV-ing subsequently.
---
src/security/security_manager.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/src/security/security_manager.c b/src/security/security_manager.c
index 6946637..411a909 100644
--- a/src/security/security_manager.c
+++ b/src/security/security_manager.c
@@ -436,6 +436,9 @@ int virSecurityManagerGenLabel(virSecurityManagerPtr mgr,
virObjectLock(mgr);
for (i = 0; i vm-nseclabels; i++) {
+if (!vm-seclabels[i]-model)
+continue;
+
for (j = 0; sec_managers[j]; j++)
if (STREQ(vm-seclabels[i]-model, sec_managers[j]-drv-name))
break;
ACK to this one too. Even though we can fix the LXC driver in your
first patch, adding this second patch is useful crash protection.
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] Why does libvirt not unlink qemu-ga or any other socket in function qemuProcessStop()
On Wed, Jul 17, 2013 at 08:14:35AM +, Wangyufei (A) wrote: Hello, I have defined a vm with two UNIX domain sockets. One is for qemu-ga and the other is for my service. These two devices will act as UNIX domain socket server. They are automatically generated by qemu. But when the vm was dead, these two sockets were not deleted. I find that libvirt will unlink vm's monitor socket(/var/lib/libvirt/qemu) in function qemuProcessStop(). Why does libvirt not unlink qemu-ga or any other socket in function qemuProcessStop()? I guess it just never occurred to anyone to implement code to unlink these other unix sockets. While QEMU could unlink them itself, a crashing QEMU certainly would not, so it would be good practice for libvirt to explicitly clean up any UNIX domain sockets after a guest shuts down. If you want to add such clean up code, please do send a patch. If you can't write it yourself, then at least file a bug to record this issue, so that we can track the issue. 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 RFC] lib: Forbid guest interaction with RO connections in virDomainGetVcpusFlags
On Tue, Jul 16, 2013 at 09:46:49AM -0600, Eric Blake wrote: On 07/16/2013 08:37 AM, Peter Krempa wrote: Don't allow guest agent interaction by read-only connections as the agent may be mailicious. s/mailicious/malicious/ --- src/libvirt.c | 6 ++ 1 file changed, 6 insertions(+) Do we have any other commands that a read-only connection can use to interact with a guest agent? A quick check shows that many other commands with an AGENT flag already require read-only connections at all times (such as virDomainReboot, virDomainSendProcessSignal, virDomainSetVcpusFlags, and virDomainSnapshotCreateXML), but at least virDomainGetHostname is permitted on a read-only connection with an allowance for guest agent interaction. Also, I'm wondering if we also need any work in the ACL framework for controlling whether a command is permitted to require guest interaction. For example, does it make sense to have an ACL that says a guest shutdown via ACPI is permitted (it does not matter if the guest responds), but a guest shutdown via the agent should be prevented (because interacting with the agent of a malicious guest is too risky)? At any rate, I think we need a v2 that covers all possible agent interaction commands, if we are going to go with this approach (but the idea does make sense to me). Yes, the ACL code is intended to obsolete the read-only flag. So anything that can be expressed with the read-only flag, must also be doable using the ACLs. We don't want to end up with one ACL permission for every guest agent command though. I think it would be sufficient to just use the generic domani 'write' permission bit to enforce this. 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] lxc_container: Don't call virGetGroupList during exec
On 17.07.2013 11:43, Daniel P. Berrange wrote:
On Wed, Jul 17, 2013 at 11:28:42AM +0200, Michal Privoznik wrote:
Commit 75c1256 states that virGetGroupList must not be called
between fork and exec, then commit ee777e99 promptly violated
that for lxc.
Patch originally posted by Eric Blake ebl...@redhat.com.
---
src/lxc/lxc_container.c | 8 +---
1 file changed, 1 insertion(+), 7 deletions(-)
diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c
index b51d7a2..37d2ba6 100644
--- a/src/lxc/lxc_container.c
+++ b/src/lxc/lxc_container.c
@@ -351,24 +351,18 @@ int lxcContainerWaitForContinue(int control)
*/
static int lxcContainerSetID(virDomainDefPtr def)
{
-gid_t *groups;
-int ngroups;
-
/* Only call virSetUIDGID when user namespace is enabled
* for this container. And user namespace is only enabled
* when nuidmapngidmap is not zero */
VIR_DEBUG(Set UID/GID to 0/0);
if (def-idmap.nuidmap
-((ngroups = virGetGroupList(0, 0, groups) 0) ||
- virSetUIDGID(0, 0, groups, ngroups) 0)) {
+virSetUIDGID(0, 0, groups, ngroups) 0) {
How does this compile ? You're removing the 'groups' and 'ngroups'
variables but still referencing them here. Don't you mean to use
NULL, 0 as the args for virSetUIDGID }
Daniel
Ugrh, yes.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu_conf: Properly mark driver-config as self-locking APIs
On 17.07.2013 11:46, Daniel P. Berrange wrote:
On Tue, Jul 16, 2013 at 08:25:25PM +0200, Michal Privoznik wrote:
Since a9e97e0c we don't require driver-config, or
virQEMUDriverGetConfig() to be called with locked QEMU driver. In fact,
calling it with the driver locked would cause deadlock. Hence, the
annotation to the struct field is not right.
---
src/qemu/qemu_conf.h | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h
index 8229cfc..fd5cc57 100644
--- a/src/qemu/qemu_conf.h
+++ b/src/qemu/qemu_conf.h
@@ -160,8 +160,7 @@ struct _virQEMUDriverConfig {
struct _virQEMUDriver {
virMutex lock;
-/* Require lock to get reference on 'config',
- * then lockless thereafter */
+/* Immutable pointer, self-locking APIs */
virQEMUDriverConfigPtr config;
Actually, it is 'immutable pointer, immutable+lockless data', since
we don't require any locks to be held in order to access fields in
the config object.
Regards,
Daniel
Consider this squashed in:
diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c
index 37d2ba6..ca8a39e 100644
--- a/src/lxc/lxc_container.c
+++ b/src/lxc/lxc_container.c
@@ -357,7 +357,7 @@ static int lxcContainerSetID(virDomainDefPtr def)
VIR_DEBUG(Set UID/GID to 0/0);
if (def-idmap.nuidmap
-virSetUIDGID(0, 0, groups, ngroups) 0) {
+virSetUIDGID(0, 0, NULL, 0) 0) {
virReportSystemError(errno, %s,
_(setuid or setgid failed));
return -1;
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu_conf: Properly mark driver-config as self-locking APIs
On Wed, Jul 17, 2013 at 12:32:57PM +0200, Michal Privoznik wrote:
On 17.07.2013 11:46, Daniel P. Berrange wrote:
On Tue, Jul 16, 2013 at 08:25:25PM +0200, Michal Privoznik wrote:
Since a9e97e0c we don't require driver-config, or
virQEMUDriverGetConfig() to be called with locked QEMU driver. In fact,
calling it with the driver locked would cause deadlock. Hence, the
annotation to the struct field is not right.
---
src/qemu/qemu_conf.h | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h
index 8229cfc..fd5cc57 100644
--- a/src/qemu/qemu_conf.h
+++ b/src/qemu/qemu_conf.h
@@ -160,8 +160,7 @@ struct _virQEMUDriverConfig {
struct _virQEMUDriver {
virMutex lock;
-/* Require lock to get reference on 'config',
- * then lockless thereafter */
+/* Immutable pointer, self-locking APIs */
virQEMUDriverConfigPtr config;
Actually, it is 'immutable pointer, immutable+lockless data', since
we don't require any locks to be held in order to access fields in
the config object.
Regards,
Daniel
Consider this squashed in:
diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c
index 37d2ba6..ca8a39e 100644
--- a/src/lxc/lxc_container.c
+++ b/src/lxc/lxc_container.c
@@ -357,7 +357,7 @@ static int lxcContainerSetID(virDomainDefPtr def)
VIR_DEBUG(Set UID/GID to 0/0);
if (def-idmap.nuidmap
-virSetUIDGID(0, 0, groups, ngroups) 0) {
+virSetUIDGID(0, 0, NULL, 0) 0) {
virReportSystemError(errno, %s,
_(setuid or setgid failed));
return -1;
You replied to the wrong message :-) But ACK to the patch since I know
what you meant to reply to.
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] maint: Make ctags work out of the box
The .ctags file specifies default options for ctags so that it does not ignore libvirt.h.in and ignores uninteresting files. As a result, you can just run ctags and navigating to a public API won't get you to a useless entry in api.html. --- .ctags | 5 + 1 file changed, 5 insertions(+) create mode 100644 .ctags diff --git a/.ctags b/.ctags new file mode 100644 index 000..75e9753 --- /dev/null +++ b/.ctags @@ -0,0 +1,5 @@ +--recurse +--exclude=*.orig +--exclude=*.html +--exclude=*.html.in +--langmap=c:+.h.in -- 1.8.3.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2] cgroup: reuse buffer for getline
On 07/17/2013 04:00 AM, Ján Tomko wrote: Reuse the buffer for getline and track buffer allocation separately from the string length to prevent unlikely out-of-bounds memory access. This fixes the following leak that happened when zero bytes were read: ==404== 120 bytes in 1 blocks are definitely lost in loss record 1,344 of 1,671 ==404==at 0x4C2C71B: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so) ==404==by 0x906F862: getdelim (iogetdelim.c:68) ==404==by 0x52A48FB: virCgroupPartitionNeedsEscaping (vircgroup.c:1136) ==404==by 0x52A0FB4: virCgroupPartitionEscape (vircgroup.c:1171) ==404==by 0x52A0EA4: virCgroupNewDomainPartition (vircgroup.c:1450) --- v1: cgroup: Free line even if no characters were read https://www.redhat.com/archives/libvir-list/2013-July/msg01030.html src/util/vircgroup.c | 20 ++-- 1 file changed, 10 insertions(+), 10 deletions(-) ACK. -- 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] lxc_container: Don't call virGetGroupList during exec
On 07/17/2013 04:30 AM, Michal Privoznik wrote:
On 17.07.2013 11:43, Daniel P. Berrange wrote:
On Wed, Jul 17, 2013 at 11:28:42AM +0200, Michal Privoznik wrote:
Commit 75c1256 states that virGetGroupList must not be called
between fork and exec, then commit ee777e99 promptly violated
that for lxc.
Patch originally posted by Eric Blake ebl...@redhat.com.
---
src/lxc/lxc_container.c | 8 +---
1 file changed, 1 insertion(+), 7 deletions(-)
Thanks; I had the same changes locally, but guess I hadn't posted them yet.
VIR_DEBUG(Set UID/GID to 0/0);
if (def-idmap.nuidmap
-((ngroups = virGetGroupList(0, 0, groups) 0) ||
- virSetUIDGID(0, 0, groups, ngroups) 0)) {
+virSetUIDGID(0, 0, groups, ngroups) 0) {
How does this compile ? You're removing the 'groups' and 'ngroups'
variables but still referencing them here. Don't you mean to use
NULL, 0 as the args for virSetUIDGID }
Yes, the 'NULL, 0' change squashed in is required :)
--
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] maint: Make ctags work out of the box
On 07/17/2013 05:10 AM, Jiri Denemark wrote: The .ctags file specifies default options for ctags so that it does not ignore libvirt.h.in and ignores uninteresting files. As a result, you can just run ctags and navigating to a public API won't get you to a useless entry in api.html. --- .ctags | 5 + 1 file changed, 5 insertions(+) create mode 100644 .ctags ACK - having this file doesn't hurt anyone that doesn't use ctags, and helps those that do. Hmm, should this file also be part of EXTRA_DIST? diff --git a/.ctags b/.ctags new file mode 100644 index 000..75e9753 --- /dev/null +++ b/.ctags @@ -0,0 +1,5 @@ +--recurse +--exclude=*.orig +--exclude=*.html +--exclude=*.html.in +--langmap=c:+.h.in -- 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 v2] cgroup: reuse buffer for getline
On 07/17/2013 01:24 PM, Eric Blake wrote: On 07/17/2013 04:00 AM, Ján Tomko wrote: Reuse the buffer for getline and track buffer allocation separately from the string length to prevent unlikely out-of-bounds memory access. This fixes the following leak that happened when zero bytes were read: ==404== 120 bytes in 1 blocks are definitely lost in loss record 1,344 of 1,671 ==404==at 0x4C2C71B: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so) ==404==by 0x906F862: getdelim (iogetdelim.c:68) ==404==by 0x52A48FB: virCgroupPartitionNeedsEscaping (vircgroup.c:1136) ==404==by 0x52A0FB4: virCgroupPartitionEscape (vircgroup.c:1171) ==404==by 0x52A0EA4: virCgroupNewDomainPartition (vircgroup.c:1450) --- v1: cgroup: Free line even if no characters were read https://www.redhat.com/archives/libvir-list/2013-July/msg01030.html src/util/vircgroup.c | 20 ++-- 1 file changed, 10 insertions(+), 10 deletions(-) ACK. Thanks, pushed now. Jan -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] Introduce virDBusCallMethod virDBusMessageRead methods
From: Daniel P. Berrange berra...@redhat.com
Doing DBus method calls using libdbus.so is tedious in the
extreme. systemd developers came up with a nice high level
API for DBus method calls (sd_bus_call_method). While
systemd doesn't use libdbus.so, their API design can easily
be ported to libdbus.so.
This patch thus introduces methods virDBusCallMethod
virDBusMessageRead, which are based on the code used for
sd_bus_call_method and sd_bus_message_read. This code in
systemd is under the LGPLv2+, so we're license compatible.
This code is probably pretty unintelligible unless you are
familiar with the DBus type system. So I added some API
docs trying to explain how to use them, as well as test
cases to validate that I didn't screw up the adaptation
from the original systemd code.
NB: this is being done as a pre-requisite to updating
libvirt's cgroups code to talk to systemd via DBus.
Signed-off-by: Daniel P. Berrange berra...@redhat.com
---
.gitignore | 1 +
src/libvirt_private.syms | 4 +
src/util/virdbus.c | 958 ++-
src/util/virdbus.h | 11 +
src/util/virdbuspriv.h | 43 +++
tests/Makefile.am| 6 +
tests/virdbustest.c | 391 +++
7 files changed, 1413 insertions(+), 1 deletion(-)
create mode 100644 src/util/virdbuspriv.h
create mode 100644 tests/virdbustest.c
diff --git a/.gitignore b/.gitignore
index 3efc2e4..851c6e4 100644
--- a/.gitignore
+++ b/.gitignore
@@ -191,6 +191,7 @@
/tests/virbitmaptest
/tests/virbuftest
/tests/vircgrouptest
+/tests/virdbustest
/tests/virdrivermoduletest
/tests/virendiantest
/tests/virhashtest
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 0ab7632..f337637 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1269,8 +1269,12 @@ virConfWriteMem;
# util/virdbus.h
+virDBusCallMethod;
virDBusGetSessionBus;
virDBusGetSystemBus;
+virDBusMessageDecode;
+virDBusMessageEncode;
+virDBusMessageRead;
# util/virdnsmasq.h
diff --git a/src/util/virdbus.c b/src/util/virdbus.c
index 52b6ca9..8c2c783 100644
--- a/src/util/virdbus.c
+++ b/src/util/virdbus.c
@@ -21,11 +21,12 @@
#include config.h
-#include virdbus.h
+#include virdbuspriv.h
#include viralloc.h
#include virerror.h
#include virlog.h
#include virthread.h
+#include virstring.h
#define VIR_FROM_THIS VIR_FROM_DBUS
@@ -223,6 +224,940 @@ static void virDBusToggleWatch(DBusWatch *watch,
(void)virEventUpdateHandle(info-watch, flags);
}
+# define VIR_DBUS_TYPE_STACK_MAX_DEPTH 32
+
+static const char virDBusBasicTypes[] = {
+DBUS_TYPE_BYTE,
+DBUS_TYPE_BOOLEAN,
+DBUS_TYPE_INT16,
+DBUS_TYPE_UINT16,
+DBUS_TYPE_INT32,
+DBUS_TYPE_UINT32,
+DBUS_TYPE_INT64,
+DBUS_TYPE_UINT64,
+DBUS_TYPE_DOUBLE,
+DBUS_TYPE_STRING,
+DBUS_TYPE_OBJECT_PATH,
+DBUS_TYPE_SIGNATURE,
+DBUS_TYPE_UNIX_FD
+};
+
+static bool virDBusIsBasicType(char c) {
+return !!memchr(virDBusBasicTypes, c,
ARRAY_CARDINALITY(virDBusBasicTypes));
+}
+
+/*
+ * All code related to virDBusMessageIterEncode and
+ * virDBusMessageIterDecode is derived from systemd
+ * bus_message_append_ap()/message_read_ap() in
+ * bus-message.c under the terms of the LGPLv2+
+ */
+static int
+virDBusSignatureLengthInternal(const char *s,
+ bool allowDict,
+ unsigned arrayDepth,
+ unsigned structDepth,
+ size_t *l)
+{
+if (virDBusIsBasicType(*s) || *s == DBUS_TYPE_VARIANT) {
+*l = 1;
+return 0;
+}
+
+if (*s == DBUS_TYPE_ARRAY) {
+size_t t;
+
+if (arrayDepth = VIR_DBUS_TYPE_STACK_MAX_DEPTH) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ _(Signature '%s' too deeply nested),
+ s);
+return -1;
+}
+
+if (virDBusSignatureLengthInternal(s + 1, true, arrayDepth+1,
structDepth, t) 0)
+return -1;
+
+*l = t + 1;
+return 0;
+}
+
+if (*s == DBUS_STRUCT_BEGIN_CHAR) {
+const char *p = s + 1;
+
+if (structDepth = VIR_DBUS_TYPE_STACK_MAX_DEPTH) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ _(Signature '%s' too deeply nested),
+ s);
+return -1;
+}
+
+while (*p != DBUS_STRUCT_END_CHAR) {
+size_t t;
+
+if (virDBusSignatureLengthInternal(p, false, arrayDepth,
structDepth+1, t) 0)
+return -1;
+
+p += t;
+}
+
+*l = p - s + 1;
+return 0;
+}
+
+if (*s == DBUS_DICT_ENTRY_BEGIN_CHAR allowDict) {
+const char *p = s + 1;
+unsigned n = 0;
+if (structDepth = VIR_DBUS_TYPE_STACK_MAX_DEPTH) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ _(Signature '%s' too deeply
[libvirt] [PATCH 7/9] lxc: Make activeUsbHostdevs use locks
The activeUsbHostdevs item in LXCDriver are lockable, but the lock has
to be called explicitly. Call the virObject(Un)Lock() in order to
achieve mutual exclusion once lxcDriverLock is removed.
---
src/lxc/lxc_driver.c | 2 ++
src/lxc/lxc_hostdev.c | 8
2 files changed, 10 insertions(+)
diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
index df9fb92..1210e77 100644
--- a/src/lxc/lxc_driver.c
+++ b/src/lxc/lxc_driver.c
@@ -4139,7 +4139,9 @@ lxcDomainDetachDeviceHostdevUSBLive(virLXCDriverPtr
driver,
VIR_WARN(cannot deny device %s for domain %s,
dst, vm-def-name);
+virObjectLock(driver-activeUsbHostdevs);
virUSBDeviceListDel(driver-activeUsbHostdevs, usb);
+virObjectUnlock(driver-activeUsbHostdevs);
virDomainHostdevRemove(vm-def, idx);
virDomainHostdevDefFree(def);
diff --git a/src/lxc/lxc_hostdev.c b/src/lxc/lxc_hostdev.c
index 257e93b..3b371fc 100644
--- a/src/lxc/lxc_hostdev.c
+++ b/src/lxc/lxc_hostdev.c
@@ -62,10 +62,13 @@ virLXCUpdateActiveUsbHostdevs(virLXCDriverPtr driver,
virUSBDeviceSetUsedBy(usb, def-name);
+virObjectLock(driver-activeUsbHostdevs);
if (virUSBDeviceListAdd(driver-activeUsbHostdevs, usb) 0) {
+virObjectUnlock(driver-activeUsbHostdevs);
virUSBDeviceFree(usb);
return -1;
}
+virObjectUnlock(driver-activeUsbHostdevs);
}
return 0;
@@ -83,6 +86,7 @@ virLXCPrepareHostdevUSBDevices(virLXCDriverPtr driver,
count = virUSBDeviceListCount(list);
+virObjectLock(driver-activeUsbHostdevs);
for (i = 0; i count; i++) {
virUSBDevicePtr usb = virUSBDeviceListGet(list, i);
if ((tmp = virUSBDeviceListFind(driver-activeUsbHostdevs, usb))) {
@@ -110,6 +114,7 @@ virLXCPrepareHostdevUSBDevices(virLXCDriverPtr driver,
if (virUSBDeviceListAdd(driver-activeUsbHostdevs, usb) 0)
goto error;
}
+virObjectUnlock(driver-activeUsbHostdevs);
return 0;
error:
@@ -117,6 +122,7 @@ error:
tmp = virUSBDeviceListGet(list, i);
virUSBDeviceListSteal(driver-activeUsbHostdevs, tmp);
}
+virObjectUnlock(driver-activeUsbHostdevs);
return -1;
}
@@ -341,6 +347,7 @@ virLXCDomainReAttachHostUsbDevices(virLXCDriverPtr driver,
{
size_t i;
+virObjectLock(driver-activeUsbHostdevs);
for (i = 0; i nhostdevs; i++) {
virDomainHostdevDefPtr hostdev = hostdevs[i];
virUSBDevicePtr usb, tmp;
@@ -392,6 +399,7 @@ virLXCDomainReAttachHostUsbDevices(virLXCDriverPtr driver,
virUSBDeviceListDel(driver-activeUsbHostdevs, tmp);
}
}
+virObjectUnlock(driver-activeUsbHostdevs);
}
void virLXCDomainReAttachHostDevices(virLXCDriverPtr driver,
--
1.8.1.5
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 5/9] lxc: switch to virCloseCallbacks API
---
src/libvirt_private.syms | 1 +
src/lxc/lxc_conf.h | 5 +-
src/lxc/lxc_driver.c | 6 +--
src/lxc/lxc_process.c| 113 ---
src/lxc/lxc_process.h| 1 -
src/util/virclosecallbacks.c | 24 +
src/util/virclosecallbacks.h | 3 ++
7 files changed, 53 insertions(+), 100 deletions(-)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 53b1153..f55ab57 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1206,6 +1206,7 @@ virCgroupSetMemSwapHardLimit;
# util/virclosecallbacks.h
virCloseCallbacksGet;
+virCloseCallbacksGetConn;
virCloseCallbacksNew;
virCloseCallbacksRun;
virCloseCallbacksSet;
diff --git a/src/lxc/lxc_conf.h b/src/lxc/lxc_conf.h
index 831e3e5..6c81368 100644
--- a/src/lxc/lxc_conf.h
+++ b/src/lxc/lxc_conf.h
@@ -35,6 +35,7 @@
# include vircgroup.h
# include virsysinfo.h
# include virusb.h
+# include virclosecallbacks.h
# define LXC_DRIVER_NAME LXC
@@ -103,8 +104,8 @@ struct _virLXCDriver {
/* Immutable pointer. self-locking APIs */
virSecurityManagerPtr securityManager;
-/* Immutable pointer. Unsafe APIs. XXX */
-virHashTablePtr autodestroy;
+/* Immutable pointer, self-locking APIs */
+virCloseCallbacksPtr closeCallbacks;
};
virLXCDriverConfigPtr virLXCDriverConfigNew(void);
diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
index 855dad3..60d21e3 100644
--- a/src/lxc/lxc_driver.c
+++ b/src/lxc/lxc_driver.c
@@ -163,7 +163,7 @@ static int lxcConnectClose(virConnectPtr conn)
virLXCDriverPtr driver = conn-privateData;
lxcDriverLock(driver);
-virLXCProcessAutoDestroyRun(driver, conn);
+virCloseCallbacksRun(driver-closeCallbacks, conn, driver-domains,
driver);
lxcDriverUnlock(driver);
conn-privateData = NULL;
@@ -1563,7 +1563,7 @@ static int lxcStateInitialize(bool privileged,
if (!(lxc_driver-xmlopt = lxcDomainXMLConfInit()))
goto cleanup;
-if (virLXCProcessAutoDestroyInit(lxc_driver) 0)
+if (!(lxc_driver-closeCallbacks = virCloseCallbacksNew()))
goto cleanup;
/* Get all the running persistent or transient configs first */
@@ -1653,7 +1653,7 @@ static int lxcStateCleanup(void)
virObjectUnref(lxc_driver-domains);
virDomainEventStateFree(lxc_driver-domainEventState);
-virLXCProcessAutoDestroyShutdown(lxc_driver);
+virObjectUnref(lxc_driver-closeCallbacks);
virSysinfoDefFree(lxc_driver-hostsysinfo);
diff --git a/src/lxc/lxc_process.c b/src/lxc/lxc_process.c
index 4b83729..931aee9 100644
--- a/src/lxc/lxc_process.c
+++ b/src/lxc/lxc_process.c
@@ -54,122 +54,45 @@
#define START_POSTFIX : starting up\n
-int virLXCProcessAutoDestroyInit(virLXCDriverPtr driver)
+static virDomainObjPtr
+lxcProcessAutoDestroy(void *drv,
+ virDomainObjPtr dom,
+ virConnectPtr conn)
{
-if (!(driver-autodestroy = virHashCreate(5, NULL)))
-return -1;
-
-return 0;
-}
-
-struct virLXCProcessAutoDestroyData {
-virLXCDriverPtr driver;
-virConnectPtr conn;
-};
-
-static void virLXCProcessAutoDestroyDom(void *payload,
-const void *name,
-void *opaque)
-{
-struct virLXCProcessAutoDestroyData *data = opaque;
-virConnectPtr conn = payload;
-const char *uuidstr = name;
-unsigned char uuid[VIR_UUID_BUFLEN];
-virDomainObjPtr dom;
+virLXCDriverPtr driver = drv;
virDomainEventPtr event = NULL;
virLXCDomainObjPrivatePtr priv;
-VIR_DEBUG(conn=%p uuidstr=%s thisconn=%p, conn, uuidstr, data-conn);
-
-if (data-conn != conn)
-return;
-
-if (virUUIDParse(uuidstr, uuid) 0) {
-VIR_WARN(Failed to parse %s, uuidstr);
-return;
-}
-
-if (!(dom = virDomainObjListFindByUUID(data-driver-domains,
- uuid))) {
-VIR_DEBUG(No domain object to kill);
-return;
-}
+VIR_DEBUG(driver=%p dom=%s conn=%p, driver, dom-def-name, conn);
priv = dom-privateData;
VIR_DEBUG(Killing domain);
-virLXCProcessStop(data-driver, dom, VIR_DOMAIN_SHUTOFF_DESTROYED);
+virLXCProcessStop(driver, dom, VIR_DOMAIN_SHUTOFF_DESTROYED);
virDomainAuditStop(dom, destroyed);
event = virDomainEventNewFromObj(dom,
VIR_DOMAIN_EVENT_STOPPED,
VIR_DOMAIN_EVENT_STOPPED_DESTROYED);
priv-doneStopEvent = true;
-if (dom !dom-persistent)
-virDomainObjListRemove(data-driver-domains, dom);
+if (dom !dom-persistent) {
+virDomainObjListRemove(driver-domains, dom);
+dom = NULL;
+}
-if (dom)
-virObjectUnlock(dom);
if (event)
-virDomainEventStateQueue(data-driver-domainEventState, event);
-virHashRemoveEntry(data-driver-autodestroy, uuidstr);
+
[libvirt] [PATCH 4/9] Introduce annotations for virLXCDriverPtr fields
Annotate the fields in virLXCDriverPtr to indicate the locking
rules for their use
---
src/lxc/lxc_conf.h | 14 ++
1 file changed, 14 insertions(+)
diff --git a/src/lxc/lxc_conf.h b/src/lxc/lxc_conf.h
index f9a3e53..831e3e5 100644
--- a/src/lxc/lxc_conf.h
+++ b/src/lxc/lxc_conf.h
@@ -67,29 +67,43 @@ struct _virLXCDriverConfig {
struct _virLXCDriver {
virMutex lock;
+/* Require lock to get reference on 'config',
+ * then lockless thereafter */
virLXCDriverConfigPtr config;
+/* Require lock while using. Unsafe. XXX */
virCapsPtr caps;
+ /* Immutable pointer. Unsafe APIs XXX */
virCgroupPtr cgroup;
+/* Immutable pointer, Immutable object */
virDomainXMLOptionPtr xmlopt;
+/* Immutable pointer, lockless APIs*/
virSysinfoDefPtr hostsysinfo;
+/* Atomic inc/dec only */
unsigned int nactive;
+/* Immutable pointers. Caller must provide locking */
virStateInhibitCallback inhibitCallback;
void *inhibitOpaque;
+/* Immutable pointer, self-locking APIs */
virDomainObjListPtr domains;
+/* Immutable pointer. Requires lock to be held before
+ * calling APIs. */
virUSBDeviceListPtr activeUsbHostdevs;
+/* Immutable pointer, self-locking APIs */
virDomainEventStatePtr domainEventState;
+/* Immutable pointer. self-locking APIs */
virSecurityManagerPtr securityManager;
+/* Immutable pointer. Unsafe APIs. XXX */
virHashTablePtr autodestroy;
};
--
1.8.1.5
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/9] qemu: Move close callbacks handling into util/virclosecallbacks.c
---
po/POTFILES.in | 1 +
src/Makefile.am | 4 +-
src/libvirt_private.syms | 7 +
src/qemu/qemu_conf.c | 295 +-
src/qemu/qemu_conf.h | 25 +---
src/qemu/qemu_driver.c | 4 +-
src/qemu/qemu_migration.c| 18 +--
src/qemu/qemu_migration.h| 2 +-
src/qemu/qemu_process.c | 14 +-
src/util/virclosecallbacks.c | 332 +++
src/util/virclosecallbacks.h | 53 +++
11 files changed, 418 insertions(+), 337 deletions(-)
create mode 100644 src/util/virclosecallbacks.c
create mode 100644 src/util/virclosecallbacks.h
diff --git a/po/POTFILES.in b/po/POTFILES.in
index 0b65765..2e4ebc8 100644
--- a/po/POTFILES.in
+++ b/po/POTFILES.in
@@ -146,6 +146,7 @@ src/util/viraudit.c
src/util/virauth.c
src/util/virauthconfig.c
src/util/vircgroup.c
+src/util/virclosecallbacks.c
src/util/vircommand.c
src/util/virconf.c
src/util/virdbus.c
diff --git a/src/Makefile.am b/src/Makefile.am
index d9e703f..8fa8680 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -83,6 +83,7 @@ UTIL_SOURCES =
\
util/virbitmap.c util/virbitmap.h \
util/virbuffer.c util/virbuffer.h \
util/vircgroup.c util/vircgroup.h util/vircgrouppriv.h \
+ util/virclosecallbacks.c util/virclosecallbacks.h
\
util/vircommand.c util/vircommand.h \
util/virconf.c util/virconf.h \
util/virdbus.c util/virdbus.h \
@@ -882,7 +883,8 @@ libvirt_util_la_SOURCES =
\
$(UTIL_SOURCES)
libvirt_util_la_CFLAGS = $(CAPNG_CFLAGS) $(YAJL_CFLAGS) $(LIBNL_CFLAGS) \
$(AM_CFLAGS) $(AUDIT_CFLAGS) $(DEVMAPPER_CFLAGS) \
- $(DBUS_CFLAGS) $(LDEXP_LIBM) $(NUMACTL_CFLAGS)
+ $(DBUS_CFLAGS) $(LDEXP_LIBM) $(NUMACTL_CFLAGS) \
+ -I$(top_srcdir)/src/conf
libvirt_util_la_LIBADD = $(CAPNG_LIBS) $(YAJL_LIBS) $(LIBNL_LIBS) \
$(THREAD_LIBS) $(AUDIT_LIBS) $(DEVMAPPER_LIBS) \
$(LIB_CLOCK_GETTIME) $(DBUS_LIBS) $(MSCOM_LIBS) $(LIBXML_LIBS) \
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index fc4e750..53b1153 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1204,6 +1204,13 @@ virCgroupSetMemorySoftLimit;
virCgroupSetMemSwapHardLimit;
+# util/virclosecallbacks.h
+virCloseCallbacksGet;
+virCloseCallbacksNew;
+virCloseCallbacksRun;
+virCloseCallbacksSet;
+virCloseCallbacksUnset;
+
# util/vircommand.h
virCommandAbort;
virCommandAddArg;
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index c91551f..64214b9 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -56,25 +56,8 @@
#define VIR_FROM_THIS VIR_FROM_QEMU
-typedef struct _qemuDriverCloseDef qemuDriverCloseDef;
-typedef qemuDriverCloseDef *qemuDriverCloseDefPtr;
-struct _qemuDriverCloseDef {
-virConnectPtr conn;
-virQEMUCloseCallback cb;
-};
-
-struct _virQEMUCloseCallbacks {
-virObjectLockable parent;
-
-/* UUID string to qemuDriverCloseDef mapping */
-virHashTablePtr list;
-};
-
-
static virClassPtr virQEMUDriverConfigClass;
-static virClassPtr virQEMUCloseCallbacksClass;
static void virQEMUDriverConfigDispose(void *obj);
-static void virQEMUCloseCallbacksDispose(void *obj);
static int virQEMUConfigOnceInit(void)
{
@@ -83,12 +66,7 @@ static int virQEMUConfigOnceInit(void)
sizeof(virQEMUDriverConfig),
virQEMUDriverConfigDispose);
-virQEMUCloseCallbacksClass = virClassNew(virClassForObjectLockable(),
- virQEMUCloseCallbacks,
- sizeof(virQEMUCloseCallbacks),
- virQEMUCloseCallbacksDispose);
-
-if (!virQEMUDriverConfigClass || !virQEMUCloseCallbacksClass)
+if (!virQEMUDriverConfigClass)
return -1;
else
return 0;
@@ -662,277 +640,6 @@ virCapsPtr virQEMUDriverGetCapabilities(virQEMUDriverPtr
driver,
return ret;
}
-
-static void
-virQEMUCloseCallbacksFreeData(void *payload,
- const void *name ATTRIBUTE_UNUSED)
-{
-VIR_FREE(payload);
-}
-
-virQEMUCloseCallbacksPtr
-virQEMUCloseCallbacksNew(void)
-{
-virQEMUCloseCallbacksPtr closeCallbacks;
-
-if (virQEMUConfigInitialize() 0)
-return NULL;
-
-if (!(closeCallbacks = virObjectLockableNew(virQEMUCloseCallbacksClass)))
-return NULL;
-
-closeCallbacks-list = virHashCreate(5, virQEMUCloseCallbacksFreeData);
-if (!closeCallbacks-list) {
-virObjectUnref(closeCallbacks);
-return NULL;
-}
-
-return closeCallbacks;
-}
-
[libvirt] [PATCH 0/9] Fine grained locking in LXC driver
The first round of my patches that breaks LXC driver into smaller pieces. With my testing on 10 domains which are started up and destroyed in a loop with 10 iterations (each loop is run in a separate thread in client): Before: real0m45.973s user0m0.080s sys 0m0.020s After: real0m14.951s user0m0.080s sys 0m0.020s Michal Privoznik (9): qemu: Move close callbacks handling into util/virclosecallbacks.c Introduce a virLXCDriverConfigPtr object lxc: Use atomic ops for driver-nactive Introduce annotations for virLXCDriverPtr fields lxc: switch to virCloseCallbacks API Stop accessing driver-caps directly in LXC driver lxc: Make activeUsbHostdevs use locks Remove lxcDriverLock from almost everywhere Introduce lxcDomObjFromDomain po/POTFILES.in | 1 + src/Makefile.am | 4 +- src/libvirt_private.syms | 8 + src/lxc/lxc_conf.c | 120 ++-- src/lxc/lxc_conf.h | 64 ++-- src/lxc/lxc_controller.c | 2 +- src/lxc/lxc_driver.c | 705 +++ src/lxc/lxc_hostdev.c| 8 + src/lxc/lxc_process.c| 181 --- src/lxc/lxc_process.h| 1 - src/qemu/qemu_conf.c | 295 +- src/qemu/qemu_conf.h | 25 +- src/qemu/qemu_driver.c | 4 +- src/qemu/qemu_migration.c| 18 +- src/qemu/qemu_migration.h| 2 +- src/qemu/qemu_process.c | 14 +- src/util/virclosecallbacks.c | 356 ++ src/util/virclosecallbacks.h | 56 18 files changed, 900 insertions(+), 964 deletions(-) create mode 100644 src/util/virclosecallbacks.c create mode 100644 src/util/virclosecallbacks.h -- 1.8.1.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 8/9] Remove lxcDriverLock from almost everywhere
With the majority of fields in the virLXCDriverPtr struct
now immutable or self-locking, there is no need for practically
any methods to be using the LXC driver lock. Only a handful
of helper APIs now need it.
---
src/lxc/lxc_conf.c| 14 -
src/lxc/lxc_conf.h| 3 -
src/lxc/lxc_driver.c | 171 +-
src/lxc/lxc_process.c | 14 +
4 files changed, 27 insertions(+), 175 deletions(-)
diff --git a/src/lxc/lxc_conf.c b/src/lxc/lxc_conf.c
index 6739df9..c1cee3f 100644
--- a/src/lxc/lxc_conf.c
+++ b/src/lxc/lxc_conf.c
@@ -167,16 +167,22 @@ error:
virCapsPtr virLXCDriverGetCapabilities(virLXCDriverPtr driver,
bool refresh)
{
+virCapsPtr ret;
if (refresh) {
virCapsPtr caps = NULL;
if ((caps = virLXCDriverCapsInit(driver)) == NULL)
return NULL;
+lxcDriverLock(driver);
virObjectUnref(driver-caps);
driver-caps = caps;
+} else {
+lxcDriverLock(driver);
}
-return virObjectRef(driver-caps);
+ret = virObjectRef(driver-caps);
+lxcDriverUnlock(driver);
+return ret;
}
@@ -273,7 +279,11 @@ done:
virLXCDriverConfigPtr virLXCDriverGetConfig(virLXCDriverPtr driver)
{
-return virObjectRef(driver-config);
+virLXCDriverConfigPtr cfg;
+lxcDriverLock(driver);
+cfg = virObjectRef(driver-config);
+lxcDriverUnlock(driver);
+return cfg;
}
static void
diff --git a/src/lxc/lxc_conf.h b/src/lxc/lxc_conf.h
index f8caebe..a6208a2 100644
--- a/src/lxc/lxc_conf.h
+++ b/src/lxc/lxc_conf.h
@@ -76,9 +76,6 @@ struct _virLXCDriver {
* lockless access thereafter */
virCapsPtr caps;
- /* Immutable pointer. Unsafe APIs XXX */
-virCgroupPtr cgroup;
-
/* Immutable pointer, Immutable object */
virDomainXMLOptionPtr xmlopt;
diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
index 1210e77..92f0d15 100644
--- a/src/lxc/lxc_driver.c
+++ b/src/lxc/lxc_driver.c
@@ -162,10 +162,7 @@ static int lxcConnectClose(virConnectPtr conn)
{
virLXCDriverPtr driver = conn-privateData;
-lxcDriverLock(driver);
virCloseCallbacksRun(driver-closeCallbacks, conn, driver-domains,
driver);
-lxcDriverUnlock(driver);
-
conn-privateData = NULL;
return 0;
}
@@ -199,15 +196,11 @@ static char *lxcConnectGetCapabilities(virConnectPtr
conn) {
if (virConnectGetCapabilitiesEnsureACL(conn) 0)
return NULL;
-lxcDriverLock(driver);
-if (!(caps = virLXCDriverGetCapabilities(driver, false))) {
-lxcDriverUnlock(driver);
+if (!(caps = virLXCDriverGetCapabilities(driver, false)))
return NULL;
-}
if ((xml = virCapabilitiesFormatXML(caps)) == NULL)
virReportOOMError();
-lxcDriverUnlock(driver);
virObjectUnref(caps);
return xml;
@@ -221,9 +214,7 @@ static virDomainPtr lxcDomainLookupByID(virConnectPtr conn,
virDomainObjPtr vm;
virDomainPtr dom = NULL;
-lxcDriverLock(driver);
vm = virDomainObjListFindByID(driver-domains, id);
-lxcDriverUnlock(driver);
if (!vm) {
virReportError(VIR_ERR_NO_DOMAIN,
@@ -251,9 +242,7 @@ static virDomainPtr lxcDomainLookupByUUID(virConnectPtr
conn,
virDomainObjPtr vm;
virDomainPtr dom = NULL;
-lxcDriverLock(driver);
vm = virDomainObjListFindByUUID(driver-domains, uuid);
-lxcDriverUnlock(driver);
if (!vm) {
char uuidstr[VIR_UUID_STRING_BUFLEN];
@@ -283,9 +272,7 @@ static virDomainPtr lxcDomainLookupByName(virConnectPtr
conn,
virDomainObjPtr vm;
virDomainPtr dom = NULL;
-lxcDriverLock(driver);
vm = virDomainObjListFindByName(driver-domains, name);
-lxcDriverUnlock(driver);
if (!vm) {
virReportError(VIR_ERR_NO_DOMAIN,
_(No domain with matching name '%s'), name);
@@ -312,9 +299,7 @@ static int lxcDomainIsActive(virDomainPtr dom)
virDomainObjPtr obj;
int ret = -1;
-lxcDriverLock(driver);
obj = virDomainObjListFindByUUID(driver-domains, dom-uuid);
-lxcDriverUnlock(driver);
if (!obj) {
char uuidstr[VIR_UUID_STRING_BUFLEN];
virUUIDFormat(dom-uuid, uuidstr);
@@ -341,9 +326,7 @@ static int lxcDomainIsPersistent(virDomainPtr dom)
virDomainObjPtr obj;
int ret = -1;
-lxcDriverLock(driver);
obj = virDomainObjListFindByUUID(driver-domains, dom-uuid);
-lxcDriverUnlock(driver);
if (!obj) {
char uuidstr[VIR_UUID_STRING_BUFLEN];
virUUIDFormat(dom-uuid, uuidstr);
@@ -369,9 +352,7 @@ static int lxcDomainIsUpdated(virDomainPtr dom)
virDomainObjPtr obj;
int ret = -1;
-lxcDriverLock(driver);
obj = virDomainObjListFindByUUID(driver-domains, dom-uuid);
-lxcDriverUnlock(driver);
if (!obj) {
char uuidstr[VIR_UUID_STRING_BUFLEN];
virUUIDFormat(dom-uuid, uuidstr);
@@ -398,10 +379,8 @@ static int
[libvirt] [PATCH 2/9] Introduce a virLXCDriverConfigPtr object
Currently the virLXCDriverPtr struct contains an wide variety
of data with varying access needs. Move all the static config
data into a dedicated virLXCDriverConfigPtr object. The only
locking requirement is to hold the driver lock, while obtaining
an instance of virLXCDriverConfigPtr. Once a reference is held
on the config object, it can be used completely lockless since
it is immutable.
NB, not all APIs correctly hold the driver lock while getting
a reference to the config object in this patch. This is safe
for now since the config is never updated on the fly. Later
patches will address this fully.
---
src/lxc/lxc_conf.c| 81 +---
src/lxc/lxc_conf.h| 41 +-
src/lxc/lxc_driver.c | 145 ++
src/lxc/lxc_process.c | 39 +-
4 files changed, 214 insertions(+), 92 deletions(-)
diff --git a/src/lxc/lxc_conf.c b/src/lxc/lxc_conf.c
index 78b1559..4e97f10 100644
--- a/src/lxc/lxc_conf.c
+++ b/src/lxc/lxc_conf.c
@@ -41,6 +41,22 @@
#define VIR_FROM_THIS VIR_FROM_LXC
+static virClassPtr virLXCDriverConfigClass;
+static void virLXCDriverConfigDispose(void *obj);
+
+static int virLXCConfigOnceInit(void)
+{
+if (!(virLXCDriverConfigClass = virClassNew(virClassForObject(),
+ virLXCDriverConfig,
+ sizeof(virLXCDriverConfig),
+ virLXCDriverConfigDispose)))
+return -1;
+
+return 0;
+}
+
+VIR_ONCE_GLOBAL_INIT(virLXCConfig)
+
/* Functions */
virCapsPtr lxcCapsInit(virLXCDriverPtr driver)
@@ -146,28 +162,42 @@ lxcDomainXMLConfInit(void)
}
-int lxcLoadDriverConfig(virLXCDriverPtr driver)
+virLXCDriverConfigPtr
+virLXCDriverConfigNew(void)
{
-char *filename;
-virConfPtr conf;
-virConfValuePtr p;
+virLXCDriverConfigPtr cfg;
+
+if (virLXCConfigInitialize() 0)
+return NULL;
-driver-securityDefaultConfined = false;
-driver-securityRequireConfined = false;
+if (!(cfg = virObjectNew(virLXCDriverConfigClass)))
+return NULL;
+
+cfg-securityDefaultConfined = false;
+cfg-securityRequireConfined = false;
/* Set the container configuration directory */
-if (VIR_STRDUP(driver-configDir, LXC_CONFIG_DIR) 0)
+if (VIR_STRDUP(cfg-configDir, LXC_CONFIG_DIR) 0)
goto error;
-if (VIR_STRDUP(driver-stateDir, LXC_STATE_DIR) 0)
+if (VIR_STRDUP(cfg-stateDir, LXC_STATE_DIR) 0)
goto error;
-if (VIR_STRDUP(driver-logDir, LXC_LOG_DIR) 0)
+if (VIR_STRDUP(cfg-logDir, LXC_LOG_DIR) 0)
goto error;
-if (VIR_STRDUP(driver-autostartDir, LXC_AUTOSTART_DIR) 0)
+if (VIR_STRDUP(cfg-autostartDir, LXC_AUTOSTART_DIR) 0)
goto error;
+return cfg;
+error:
+virObjectUnref(cfg);
+return NULL;
+}
-if (VIR_STRDUP(filename, SYSCONFDIR /libvirt/lxc.conf) 0)
-goto error;
+int
+virLXCLoadDriverConfig(virLXCDriverConfigPtr cfg,
+ const char *filename)
+{
+virConfPtr conf;
+virConfValuePtr p;
/* Avoid error from non-existant or unreadable file. */
if (access(filename, R_OK) == -1)
@@ -186,12 +216,12 @@ int lxcLoadDriverConfig(virLXCDriverPtr driver)
p = virConfGetValue(conf, log_with_libvirtd);
CHECK_TYPE(log_with_libvirtd, VIR_CONF_LONG);
-if (p) driver-log_libvirtd = p-l;
+if (p) cfg-log_libvirtd = p-l;
p = virConfGetValue(conf, security_driver);
CHECK_TYPE(security_driver, VIR_CONF_STRING);
if (p p-str) {
-if (VIR_STRDUP(driver-securityDriverName, p-str) 0) {
+if (VIR_STRDUP(cfg-securityDriverName, p-str) 0) {
virConfFree(conf);
return -1;
}
@@ -199,11 +229,11 @@ int lxcLoadDriverConfig(virLXCDriverPtr driver)
p = virConfGetValue(conf, security_default_confined);
CHECK_TYPE(security_default_confined, VIR_CONF_LONG);
-if (p) driver-securityDefaultConfined = p-l;
+if (p) cfg-securityDefaultConfined = p-l;
p = virConfGetValue(conf, security_require_confined);
CHECK_TYPE(security_require_confined, VIR_CONF_LONG);
-if (p) driver-securityRequireConfined = p-l;
+if (p) cfg-securityRequireConfined = p-l;
#undef CHECK_TYPE
@@ -211,9 +241,22 @@ int lxcLoadDriverConfig(virLXCDriverPtr driver)
virConfFree(conf);
done:
-VIR_FREE(filename);
return 0;
+}
-error:
-return -1;
+virLXCDriverConfigPtr virLXCDriverGetConfig(virLXCDriverPtr driver)
+{
+return virObjectRef(driver-config);
+}
+
+static void
+virLXCDriverConfigDispose(void *obj)
+{
+virLXCDriverConfigPtr cfg = obj;
+
+VIR_FREE(cfg-configDir);
+VIR_FREE(cfg-autostartDir);
+VIR_FREE(cfg-stateDir);
+VIR_FREE(cfg-logDir);
+VIR_FREE(cfg-securityDriverName);
}
diff --git a/src/lxc/lxc_conf.h b/src/lxc/lxc_conf.h
index 5a5b9aa..6ca6198 100644
--- a/src/lxc/lxc_conf.h
+++
[libvirt] [PATCH 3/9] lxc: Use atomic ops for driver-nactive
---
src/lxc/lxc_conf.h| 2 +-
src/lxc/lxc_process.c | 10 --
2 files changed, 5 insertions(+), 7 deletions(-)
diff --git a/src/lxc/lxc_conf.h b/src/lxc/lxc_conf.h
index 6ca6198..f9a3e53 100644
--- a/src/lxc/lxc_conf.h
+++ b/src/lxc/lxc_conf.h
@@ -77,7 +77,7 @@ struct _virLXCDriver {
virSysinfoDefPtr hostsysinfo;
-size_t nactive;
+unsigned int nactive;
virStateInhibitCallback inhibitCallback;
void *inhibitOpaque;
diff --git a/src/lxc/lxc_process.c b/src/lxc/lxc_process.c
index 1024576..4b83729 100644
--- a/src/lxc/lxc_process.c
+++ b/src/lxc/lxc_process.c
@@ -48,6 +48,7 @@
#include lxc_hostdev.h
#include virhook.h
#include virstring.h
+#include viratomic.h
#define VIR_FROM_THIS VIR_FROM_LXC
@@ -256,8 +257,7 @@ static void virLXCProcessCleanup(virLXCDriverPtr driver,
vm-pid = -1;
vm-def-id = -1;
-driver-nactive--;
-if (!driver-nactive driver-inhibitCallback)
+if (virAtomicIntDecAndTest(driver-nactive) driver-inhibitCallback)
driver-inhibitCallback(false, driver-inhibitOpaque);
virLXCDomainReAttachHostDevices(driver, vm-def);
@@ -1273,9 +1273,8 @@ int virLXCProcessStart(virConnectPtr conn,
virDomainObjSetState(vm, VIR_DOMAIN_RUNNING, reason);
priv-doneStopEvent = false;
-if (!driver-nactive driver-inhibitCallback)
+if (virAtomicIntInc(driver-nactive) driver-inhibitCallback)
driver-inhibitCallback(true, driver-inhibitOpaque);
-driver-nactive++;
if (lxcContainerWaitForContinue(handshakefds[0]) 0) {
char out[1024];
@@ -1458,9 +1457,8 @@ virLXCProcessReconnectDomain(virDomainObjPtr vm,
virDomainObjSetState(vm, VIR_DOMAIN_RUNNING,
VIR_DOMAIN_RUNNING_UNKNOWN);
-if (!driver-nactive driver-inhibitCallback)
+if (virAtomicIntInc(driver-nactive) driver-inhibitCallback)
driver-inhibitCallback(true, driver-inhibitOpaque);
-driver-nactive++;
if (!(priv-monitor = virLXCProcessConnectMonitor(driver, vm)))
goto error;
--
1.8.1.5
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 9/9] Introduce lxcDomObjFromDomain
Similarly to qemu driver, we can use a helper function to
lookup a domain instead of copying multiple lines around.
---
src/lxc/lxc_driver.c | 344 +++
1 file changed, 73 insertions(+), 271 deletions(-)
diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
index 92f0d15..0c92de5 100644
--- a/src/lxc/lxc_driver.c
+++ b/src/lxc/lxc_driver.c
@@ -109,6 +109,35 @@ static virNWFilterCallbackDriver lxcCallbackDriver = {
.vmDriverUnlock = lxcVMDriverUnlock,
};
+/**
+ * lxcDomObjFromDomain:
+ * @domain: Domain pointer that has to be looked up
+ *
+ * This function looks up @domain and returns the appropriate
+ * virDomainObjPtr.
+ *
+ * Returns the domain object which is locked on success, NULL
+ * otherwise.
+ */
+static virDomainObjPtr
+lxcDomObjFromDomain(virDomainPtr domain)
+{
+virDomainObjPtr vm;
+virLXCDriverPtr driver = domain-conn-privateData;
+char uuidstr[VIR_UUID_STRING_BUFLEN];
+
+vm = virDomainObjListFindByUUID(driver-domains, domain-uuid);
+if (!vm) {
+virUUIDFormat(domain-uuid, uuidstr);
+virReportError(VIR_ERR_NO_DOMAIN,
+ _(no domain with matching uuid '%s' (%s)),
+ uuidstr, domain-name);
+return NULL;
+}
+
+return vm;
+}
+
/* Functions */
static virDrvOpenStatus lxcConnectOpen(virConnectPtr conn,
@@ -295,18 +324,11 @@ cleanup:
static int lxcDomainIsActive(virDomainPtr dom)
{
-virLXCDriverPtr driver = dom-conn-privateData;
virDomainObjPtr obj;
int ret = -1;
-obj = virDomainObjListFindByUUID(driver-domains, dom-uuid);
-if (!obj) {
-char uuidstr[VIR_UUID_STRING_BUFLEN];
-virUUIDFormat(dom-uuid, uuidstr);
-virReportError(VIR_ERR_NO_DOMAIN,
- _(No domain with matching uuid '%s'), uuidstr);
+if (!(obj = lxcDomObjFromDomain(dom)))
goto cleanup;
-}
if (virDomainIsActiveEnsureACL(dom-conn, obj-def) 0)
goto cleanup;
@@ -322,18 +344,11 @@ cleanup:
static int lxcDomainIsPersistent(virDomainPtr dom)
{
-virLXCDriverPtr driver = dom-conn-privateData;
virDomainObjPtr obj;
int ret = -1;
-obj = virDomainObjListFindByUUID(driver-domains, dom-uuid);
-if (!obj) {
-char uuidstr[VIR_UUID_STRING_BUFLEN];
-virUUIDFormat(dom-uuid, uuidstr);
-virReportError(VIR_ERR_NO_DOMAIN,
- _(No domain with matching uuid '%s'), uuidstr);
+if (!(obj = lxcDomObjFromDomain(dom)))
goto cleanup;
-}
if (virDomainIsPersistentEnsureACL(dom-conn, obj-def) 0)
goto cleanup;
@@ -348,18 +363,11 @@ cleanup:
static int lxcDomainIsUpdated(virDomainPtr dom)
{
-virLXCDriverPtr driver = dom-conn-privateData;
virDomainObjPtr obj;
int ret = -1;
-obj = virDomainObjListFindByUUID(driver-domains, dom-uuid);
-if (!obj) {
-char uuidstr[VIR_UUID_STRING_BUFLEN];
-virUUIDFormat(dom-uuid, uuidstr);
-virReportError(VIR_ERR_NO_DOMAIN,
- _(No domain with matching uuid '%s'), uuidstr);
+if (!(obj = lxcDomObjFromDomain(dom)))
goto cleanup;
-}
if (virDomainIsUpdatedEnsureACL(dom-conn, obj-def) 0)
goto cleanup;
@@ -506,14 +514,8 @@ static int lxcDomainUndefineFlags(virDomainPtr dom,
virCheckFlags(0, -1);
-vm = virDomainObjListFindByUUID(driver-domains, dom-uuid);
-if (!vm) {
-char uuidstr[VIR_UUID_STRING_BUFLEN];
-virUUIDFormat(dom-uuid, uuidstr);
-virReportError(VIR_ERR_NO_DOMAIN,
- _(No domain with matching uuid '%s'), uuidstr);
+if (!(vm = lxcDomObjFromDomain(dom)))
goto cleanup;
-}
if (virDomainUndefineFlagsEnsureACL(dom-conn, vm-def) 0)
goto cleanup;
@@ -559,20 +561,12 @@ static int lxcDomainUndefine(virDomainPtr dom)
static int lxcDomainGetInfo(virDomainPtr dom,
virDomainInfoPtr info)
{
-virLXCDriverPtr driver = dom-conn-privateData;
virDomainObjPtr vm;
int ret = -1, rc;
virLXCDomainObjPrivatePtr priv;
-vm = virDomainObjListFindByUUID(driver-domains, dom-uuid);
-
-if (!vm) {
-char uuidstr[VIR_UUID_STRING_BUFLEN];
-virUUIDFormat(dom-uuid, uuidstr);
-virReportError(VIR_ERR_NO_DOMAIN,
- _(No domain with matching uuid '%s'), uuidstr);
+if (!(vm = lxcDomObjFromDomain(dom)))
goto cleanup;
-}
priv = vm-privateData;
@@ -618,21 +612,13 @@ lxcDomainGetState(virDomainPtr dom,
int *reason,
unsigned int flags)
{
-virLXCDriverPtr driver = dom-conn-privateData;
virDomainObjPtr vm;
int ret = -1;
virCheckFlags(0, -1);
-vm = virDomainObjListFindByUUID(driver-domains, dom-uuid);
-
-if (!vm) {
-char uuidstr[VIR_UUID_STRING_BUFLEN];
-virUUIDFormat(dom-uuid, uuidstr);
-
Re: [libvirt] [PATCH 0/9] Fine grained locking in LXC driver
On 17.07.2013 15:04, Michal Privoznik wrote: The first round of my patches that breaks LXC driver into smaller pieces. With my testing on 10 domains which are started up and destroyed in a loop with 10 iterations (each loop is run in a separate thread in client): Before: real0m45.973s user0m0.080s sys 0m0.020s After: real0m14.951s user0m0.080s sys 0m0.020s Forgot to mention, patches are available at my gitorious too: https://gitorious.org/~zippy2/libvirt/michal-staging/commits/lxc_lock Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] xen: Add interface versions for Xen 4.3
Stefan Bader wrote:
I tried to follow previous changes. This worked for me using Xen-4.3
and the xm stack (I know rather deprecated but as long as one can get
it working, still).
Agreed, we should keep the legacy driver working with the latest Xen
release until its dependencies (xm/xend) are completely removed.
-Stefan
---
From 4c36fb22f01689472f6e170a8df6e08fd8c27a42 Mon Sep 17 00:00:00 2001
From: Stefan Bader stefan.ba...@canonical.com
Date: Mon, 15 Jul 2013 19:25:23 +0200
Subject: [PATCH] xen: Add interface versions for Xen 4.3
Xen 4.3 changes sysctl version to 0xA and domctl version to 0x9. Update
the hypervisor driver to work with those.
Signed-off-by: Stefan Bader stefan.ba...@canonical.com
---
src/xen/xen_hypervisor.c | 129
++
1 file changed, 97 insertions(+), 32 deletions(-)
diff --git a/src/xen/xen_hypervisor.c b/src/xen/xen_hypervisor.c
index 39e641e..ff92556 100644
--- a/src/xen/xen_hypervisor.c
+++ b/src/xen/xen_hypervisor.c
@@ -271,6 +271,24 @@ struct xen_v2d8_getdomaininfo {
};
typedef struct xen_v2d8_getdomaininfo xen_v2d8_getdomaininfo;
+struct xen_v2d9_getdomaininfo {
+domid_t domain; /* the domain number */
+uint32_t flags; /* flags, see before */
+uint64_t tot_pages ALIGN_64; /* total number of pages used */
+uint64_t max_pages ALIGN_64; /* maximum number of pages allowed */
+uint64_t outstanding_pages ALIGN_64;
+uint64_t shr_pages ALIGN_64;/* number of shared pages */
+uint64_t paged_pages ALIGN_64;/* number of paged pages */
+uint64_t shared_info_frame ALIGN_64; /* MFN of shared_info struct */
+uint64_t cpu_time ALIGN_64; /* CPU time used */
+uint32_t nr_online_vcpus; /* Number of VCPUs currently online. */
+uint32_t max_vcpu_id; /* Maximum VCPUID in use by this domain. */
+uint32_t ssidref;
+xen_domain_handle_t handle;
+uint32_t cpupool;
+};
+typedef struct xen_v2d9_getdomaininfo xen_v2d9_getdomaininfo;
+
union xen_getdomaininfo {
struct xen_v0_getdomaininfo v0;
struct xen_v2_getdomaininfo v2;
@@ -278,6 +296,7 @@ union xen_getdomaininfo {
struct xen_v2d6_getdomaininfo v2d6;
struct xen_v2d7_getdomaininfo v2d7;
struct xen_v2d8_getdomaininfo v2d8;
+struct xen_v2d9_getdomaininfo v2d9;
};
typedef union xen_getdomaininfo xen_getdomaininfo;
@@ -288,6 +307,7 @@ union xen_getdomaininfolist {
struct xen_v2d6_getdomaininfo *v2d6;
struct xen_v2d7_getdomaininfo *v2d7;
struct xen_v2d8_getdomaininfo *v2d8;
+struct xen_v2d8_getdomaininfo *v2d9;
};
typedef union xen_getdomaininfolist xen_getdomaininfolist;
@@ -325,7 +345,9 @@ typedef struct xen_v2s5_availheap xen_v2s5_availheap;
#define XEN_GETDOMAININFOLIST_ALLOC(domlist, size) \
(hv_versions.hypervisor 2 ? \
(VIR_ALLOC_N(domlist.v0, (size)) == 0) : \
- (hv_versions.dom_interface = 8 ? \
+ (hv_versions.dom_interface = 9 ? \
+ (VIR_ALLOC_N(domlist.v2d9, (size)) == 0) :\
+ (hv_versions.dom_interface == 8 ? \
(VIR_ALLOC_N(domlist.v2d8, (size)) == 0) :\
(hv_versions.dom_interface == 7 ? \
(VIR_ALLOC_N(domlist.v2d7, (size)) == 0) :\
@@ -333,12 +355,14 @@ typedef struct xen_v2s5_availheap xen_v2s5_availheap;
(VIR_ALLOC_N(domlist.v2d6, (size)) == 0) :\
(hv_versions.dom_interface == 5 ? \
(VIR_ALLOC_N(domlist.v2d5, (size)) == 0) :\
- (VIR_ALLOC_N(domlist.v2, (size)) == 0))
+ (VIR_ALLOC_N(domlist.v2, (size)) == 0)))
I'll be happy when we can ignore changes to sysctl and domctl interface
in this driver so this madness can stop :).
[...]
@@ -1916,6 +1968,19 @@ xenHypervisorInit(struct xenHypervisorVersions
*override_versions)
}
}
+/* Xen 4.3
+ * sysctl version A - xen-4.3 release
+ * domctl version 9 - xen-4.3 release
I like to reference the commits that bump the versions, making it easy
to look at the interface changes.
+ */
+hv_versions.sys_interface = 0xA; /* XEN_SYSCTL_INTERFACE_VERSION */
+if (virXen_getdomaininfo(fd, 0, info) == 1) {
+ hv_versions.dom_interface = 0x9; /* XEN_DOMCTL_INTERFACE_VERSION */
+ if (virXen_getvcpusinfo(fd, 0, 0, ipt, NULL, 0) == 0) {
+ VIR_DEBUG(Using hypervisor call v2, sys verA dom ver9);
+ goto done;
+ }
Tabs here instead of spaces, caught by 'make syntax-check'.
All of the existing logic uses decimal for the interface version
numbers. I've changed all uses of hex to decimal, added the commits
Re: [libvirt] [PATCH v3 2/3] libxl: advertise the support for VIR_TYPED_PARAM_STRING
Daniel P. Berrange wrote: On Sat, Jul 13, 2013 at 02:27:10AM +0200, Dario Faggioli wrote: domainGetNumaParameters has a string typed parameter, hence it is necessary for the libxl driver to support this. This change implements the connectSupportsFeature hook for the libxl driver, advertising that VIR_DRV_FEATURE_TYPED_PARAM_STRING is supported. Signed-off-by: Dario Faggioli dario.faggi...@citrix.com Cc: Eric Blake ebl...@redhat.com --- Changes from v1: * commit message improved, as requested during review; * added VIR_TYPED_PARAM_STRING_OKAY handling code in libxlDomainGetSchedulerParametersFlags(); --- src/libxl/libxl_driver.c | 20 +++- 1 file changed, 19 insertions(+), 1 deletion(-) ACK I've pushed patches 2 and 3 now. Thanks Dario! Regards, Jim -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] tests: split long lines
Long lines are harder to read and harder to diff; in fact, if lines get too long ( 1000 bytes), it starts causing issues where git send-email refuses to send patches for the file. I've cleaned up the tests directory in the past (see commits bd6c46f, 3b750d1), but new long lines have been introduced in the meantime. Victim files found with: $ wc -L tests/qemuxml2argvdata/*.args | sort -k1,1n \ | sed -n '/\b\(9[0-9]\|[1-9][0-9][0-9]\)\b/p' * tests/qemuxml2argvdata/qemuxml2argv-*.args: Split lines of any file with content longer than 90 columns. Signed-off-by: Eric Blake ebl...@redhat.com --- Whitespace-only, but it's big enough that I'll wait for comments before pushing. .../qemuxml2argv-boot-complex-bootindex.args | 15 ++- tests/qemuxml2argvdata/qemuxml2argv-boot-complex.args | 6 -- tests/qemuxml2argvdata/qemuxml2argv-boot-order.args | 9 ++--- .../qemuxml2argvdata/qemuxml2argv-console-virtio-ccw.args | 3 ++- tests/qemuxml2argvdata/qemuxml2argv-controller-order.args | 8 +--- .../qemuxml2argv-cpu-host-model-fallback.args | 3 ++- tests/qemuxml2argvdata/qemuxml2argv-disk-cdrom-tray.args | 6 -- .../qemuxml2argvdata/qemuxml2argv-disk-copy_on_read.args | 12 .../qemuxml2argvdata/qemuxml2argv-disk-drive-discard.args | 9 ++--- .../qemuxml2argv-disk-drive-network-gluster.args | 8 +++- .../qemuxml2argv-disk-drive-network-iscsi-auth.args | 7 ++- .../qemuxml2argv-disk-drive-network-iscsi.args| 9 - tests/qemuxml2argvdata/qemuxml2argv-disk-ioeventfd.args | 9 ++--- tests/qemuxml2argvdata/qemuxml2argv-disk-order.args | 6 -- .../qemuxml2argv-disk-scsi-disk-split.args| 12 .../qemuxml2argvdata/qemuxml2argv-disk-scsi-disk-wwn.args | 6 -- .../qemuxml2argv-disk-scsi-lun-passthrough.args | 6 -- tests/qemuxml2argvdata/qemuxml2argv-disk-scsi-vscsi.args | 10 ++ .../qemuxml2argv-disk-virtio-scsi-ccw.args| 6 -- .../qemuxml2argv-disk-virtio-scsi-num_queues.args | 3 ++- tests/qemuxml2argvdata/qemuxml2argv-event_idx.args| 9 ++--- .../qemuxml2argv-graphics-spice-agentmouse.args | 9 ++--- .../qemuxml2argv-graphics-spice-timeout.args | 6 -- .../qemuxml2argv-graphics-spice-usb-redir.args| 11 --- tests/qemuxml2argvdata/qemuxml2argv-graphics-spice.args | 6 -- .../qemuxml2argvdata/qemuxml2argv-hostdev-scsi-boot.args | 3 ++- tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-lsi.args | 6 -- .../qemuxml2argv-hostdev-scsi-readonly.args | 3 ++- .../qemuxml2argv-hostdev-scsi-virtio-scsi.args| 3 ++- .../qemuxml2argv-input-usbmouse-addr.args | 6 +- tests/qemuxml2argvdata/qemuxml2argv-pseries-basic.args| 8 +++- .../qemuxml2argv-pseries-vio-user-assigned.args | 6 -- tests/qemuxml2argvdata/qemuxml2argv-pseries-vio.args | 6 -- tests/qemuxml2argvdata/qemuxml2argv-tpm-passthrough.args | 3 ++- tests/qemuxml2argvdata/qemuxml2argv-usb-controller.args | 6 +- tests/qemuxml2argvdata/qemuxml2argv-usb-hub.args | 7 ++- tests/qemuxml2argvdata/qemuxml2argv-usb-ports.args| 11 ++- tests/qemuxml2argvdata/qemuxml2argv-usb-redir-filter.args | 14 ++ tests/qemuxml2argvdata/qemuxml2argv-usb-redir.args| 8 ++-- tests/qemuxml2argvdata/qemuxml2argv-virtio-lun.args | 6 -- tests/qemuxml2argvdata/qemuxml2argv-virtio-rng-ccw.args | 3 ++- tests/qemuxml2argvdata/qemuxml2argv-virtio-rng-egd.args | 8 +++- 42 files changed, 216 insertions(+), 85 deletions(-) diff --git a/tests/qemuxml2argvdata/qemuxml2argv-boot-complex-bootindex.args b/tests/qemuxml2argvdata/qemuxml2argv-boot-complex-bootindex.args index 2fde757..a2f9067 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-boot-complex-bootindex.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-boot-complex-bootindex.args @@ -10,21 +10,26 @@ LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test \ -no-acpi \ -usb \ -drive file=/tmp/vda.img,if=none,id=drive-virtio-disk0 \ --device virtio-blk-pci,scsi=off,bus=pci.0,addr=0x5,drive=drive-virtio-disk0,id=virtio-disk0,bootindex=3 \ +-device virtio-blk-pci,scsi=off,bus=pci.0,addr=0x5,drive=drive-virtio-disk0,\ +id=virtio-disk0,bootindex=3 \ -drive file=/tmp/vdb.img,if=none,id=drive-virtio-disk1 \ --device virtio-blk-pci,scsi=off,bus=pci.0,addr=0x6,drive=drive-virtio-disk1,id=virtio-disk1 \ +-device virtio-blk-pci,scsi=off,bus=pci.0,addr=0x6,drive=drive-virtio-disk1,\ +id=virtio-disk1 \ -drive file=/dev/HostVG/hda,if=none,id=drive-ide0-0-0 \ -device ide-drive,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0 \ -drive file=/dev/HostVG/hdb,if=none,id=drive-ide0-0-1 \ -device ide-drive,bus=ide.0,unit=1,drive=drive-ide0-0-1,id=ide0-0-1 \ -drive
[libvirt] [PATCH] Check for link_addr more thoroughly
Some versions of kFreeBSD (like 9.0) declare link_addr in a header but lack an implementation. This makes ./configure pass but breaks compilation later with a undefined reference to `link_addr' Althought that's a bug in the OS header we can detect it easily by also trying to link. http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=715320 --- configure.ac | 14 -- 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/configure.ac b/configure.ac index b5af0d3..4e8b9b8 100644 --- a/configure.ac +++ b/configure.ac @@ -2391,13 +2391,15 @@ AC_CHECK_MEMBERS([struct ifreq.ifr_newname, [#include sys/socket.h #include net/if.h ]) + # Check for BSD approach for setting MAC addr -AC_CHECK_DECLS([link_addr], - [], [], - [#include sys/types.h -#include sys/socket.h -#include net/if_dl.h - ]) +AC_TRY_LINK([#include sys/types.h + #include sys/socket.h + #include net/if_dl.h], +[link_addr(NULL, NULL)], +[AC_DEFINE([HAVE_DECL_LINK_ADDR], + [1], + [whether link_addr is available])]) # Check for BSD approach for bridge management AC_CHECK_DECLS([BRDGSFD, BRDGADD, BRDGDEL], -- 1.8.3.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Check for link_addr more thoroughly
On 07/17/2013 01:05 PM, Guido Günther wrote: Some versions of kFreeBSD (like 9.0) declare link_addr in a header but lack an implementation. This makes ./configure pass but breaks compilation later with a undefined reference to `link_addr' Althought that's a bug in the OS header we can detect it easily by also trying to link. http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=715320 --- configure.ac | 14 -- 1 file changed, 8 insertions(+), 6 deletions(-) + # Check for BSD approach for setting MAC addr -AC_CHECK_DECLS([link_addr], - [], [], - [#include sys/types.h -#include sys/socket.h -#include net/if_dl.h - ]) +AC_TRY_LINK([#include sys/types.h I'd prefer we use AC_LINK_IFELSE here (upstream autoconf has deprecated AC_TRY_LINK on the grounds that AC_LINK_IFELSE has saner quoting rules in relation to the rest of the configure.ac file). + #include sys/socket.h + #include net/if_dl.h], +[link_addr(NULL, NULL)], +[AC_DEFINE([HAVE_DECL_LINK_ADDR], + [1], + [whether link_addr is available])]) The concept makes sense, but it's probably worth seeing a v2 just to make sure the modernized autoconf magic is correct. -- 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] xen: Add interface versions for Xen 4.3
On 17.07.2013 18:42, Jim Fehlig wrote:
Stefan Bader wrote:
I tried to follow previous changes. This worked for me using Xen-4.3
and the xm stack (I know rather deprecated but as long as one can get
it working, still).
Agreed, we should keep the legacy driver working with the latest Xen
release until its dependencies (xm/xend) are completely removed.
-Stefan
---
From 4c36fb22f01689472f6e170a8df6e08fd8c27a42 Mon Sep 17 00:00:00 2001
From: Stefan Bader stefan.ba...@canonical.com
Date: Mon, 15 Jul 2013 19:25:23 +0200
Subject: [PATCH] xen: Add interface versions for Xen 4.3
Xen 4.3 changes sysctl version to 0xA and domctl version to 0x9. Update
the hypervisor driver to work with those.
Signed-off-by: Stefan Bader stefan.ba...@canonical.com
---
src/xen/xen_hypervisor.c | 129
++
1 file changed, 97 insertions(+), 32 deletions(-)
diff --git a/src/xen/xen_hypervisor.c b/src/xen/xen_hypervisor.c
index 39e641e..ff92556 100644
--- a/src/xen/xen_hypervisor.c
+++ b/src/xen/xen_hypervisor.c
@@ -271,6 +271,24 @@ struct xen_v2d8_getdomaininfo {
};
typedef struct xen_v2d8_getdomaininfo xen_v2d8_getdomaininfo;
+struct xen_v2d9_getdomaininfo {
+domid_t domain;/* the domain number */
+uint32_t flags; /* flags, see before */
+uint64_t tot_pages ALIGN_64;/* total number of pages used */
+uint64_t max_pages ALIGN_64;/* maximum number of pages allowed */
+uint64_t outstanding_pages ALIGN_64;
+uint64_t shr_pages ALIGN_64;/* number of shared pages */
+uint64_t paged_pages ALIGN_64;/* number of paged pages */
+uint64_t shared_info_frame ALIGN_64; /* MFN of shared_info struct */
+uint64_t cpu_time ALIGN_64; /* CPU time used */
+uint32_t nr_online_vcpus; /* Number of VCPUs currently online. */
+uint32_t max_vcpu_id; /* Maximum VCPUID in use by this domain. */
+uint32_t ssidref;
+xen_domain_handle_t handle;
+uint32_t cpupool;
+};
+typedef struct xen_v2d9_getdomaininfo xen_v2d9_getdomaininfo;
+
union xen_getdomaininfo {
struct xen_v0_getdomaininfo v0;
struct xen_v2_getdomaininfo v2;
@@ -278,6 +296,7 @@ union xen_getdomaininfo {
struct xen_v2d6_getdomaininfo v2d6;
struct xen_v2d7_getdomaininfo v2d7;
struct xen_v2d8_getdomaininfo v2d8;
+struct xen_v2d9_getdomaininfo v2d9;
};
typedef union xen_getdomaininfo xen_getdomaininfo;
@@ -288,6 +307,7 @@ union xen_getdomaininfolist {
struct xen_v2d6_getdomaininfo *v2d6;
struct xen_v2d7_getdomaininfo *v2d7;
struct xen_v2d8_getdomaininfo *v2d8;
+struct xen_v2d8_getdomaininfo *v2d9;
};
typedef union xen_getdomaininfolist xen_getdomaininfolist;
@@ -325,7 +345,9 @@ typedef struct xen_v2s5_availheap xen_v2s5_availheap;
#define XEN_GETDOMAININFOLIST_ALLOC(domlist, size) \
(hv_versions.hypervisor 2 ? \
(VIR_ALLOC_N(domlist.v0, (size)) == 0) : \
- (hv_versions.dom_interface = 8 ? \
+ (hv_versions.dom_interface = 9 ? \
+ (VIR_ALLOC_N(domlist.v2d9, (size)) == 0) :\
+ (hv_versions.dom_interface == 8 ? \
(VIR_ALLOC_N(domlist.v2d8, (size)) == 0) :\
(hv_versions.dom_interface == 7 ? \
(VIR_ALLOC_N(domlist.v2d7, (size)) == 0) :\
@@ -333,12 +355,14 @@ typedef struct xen_v2s5_availheap xen_v2s5_availheap;
(VIR_ALLOC_N(domlist.v2d6, (size)) == 0) :\
(hv_versions.dom_interface == 5 ? \
(VIR_ALLOC_N(domlist.v2d5, (size)) == 0) :\
- (VIR_ALLOC_N(domlist.v2, (size)) == 0))
+ (VIR_ALLOC_N(domlist.v2, (size)) == 0)))
I'll be happy when we can ignore changes to sysctl and domctl interface
in this driver so this madness can stop :).
[...]
@@ -1916,6 +1968,19 @@ xenHypervisorInit(struct xenHypervisorVersions
*override_versions)
}
}
+/* Xen 4.3
+ * sysctl version A - xen-4.3 release
+ * domctl version 9 - xen-4.3 release
I like to reference the commits that bump the versions, making it easy
to look at the interface changes.
+ */
+hv_versions.sys_interface = 0xA; /* XEN_SYSCTL_INTERFACE_VERSION */
+if (virXen_getdomaininfo(fd, 0, info) == 1) {
+hv_versions.dom_interface = 0x9; /* XEN_DOMCTL_INTERFACE_VERSION */
+if (virXen_getvcpusinfo(fd, 0, 0, ipt, NULL, 0) == 0) {
+VIR_DEBUG(Using hypervisor call v2, sys verA dom ver9);
+goto done;
+}
Tabs here instead of spaces, caught by 'make syntax-check'.
Ah good to know/remember that. A bit of a problem jumping between projects (the
[libvirt] [PATCH] esx: Support for disk-only and quiescing snapshots.
Add support for creating disk-only (no memory) snapshots in esx, and
for quiescing the VM before taking the snapshot. The VMware API
supports these operations directly, so adding support to libvirt is
just a matter of setting the flags correctly when calling
VMware. VIR_DOMAIN_SNAPSHOT_CREATE_DISK_ONLY and
VIR_DOMAIN_SNAPSHOT_CREATE_QUIESCE are now valid flags for esx.
---
src/esx/esx_driver.c | 14 ++
1 files changed, 10 insertions(+), 4 deletions(-)
diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c
index fbe43c2..d69576d 100644
--- a/src/esx/esx_driver.c
+++ b/src/esx/esx_driver.c
@@ -4209,9 +4209,14 @@ esxDomainSnapshotCreateXML(virDomainPtr domain, const
char *xmlDesc,
esxVI_TaskInfoState taskInfoState;
char *taskInfoErrorMessage = NULL;
virDomainSnapshotPtr snapshot = NULL;
+bool diskOnly = (flags VIR_DOMAIN_SNAPSHOT_CREATE_DISK_ONLY) != 0;
+bool quiesce = (flags VIR_DOMAIN_SNAPSHOT_CREATE_QUIESCE) != 0;
-/* ESX has no snapshot metadata, so this flag is trivial. */
-virCheckFlags(VIR_DOMAIN_SNAPSHOT_CREATE_NO_METADATA, NULL);
+/* ESX supports disk-only and quiesced snapshots, but has no snapshot *
+ * metadata. */
+virCheckFlags(VIR_DOMAIN_SNAPSHOT_CREATE_DISK_ONLY |
+ VIR_DOMAIN_SNAPSHOT_CREATE_QUIESCE |
+ VIR_DOMAIN_SNAPSHOT_CREATE_NO_METADATA, NULL);
if (esxVI_EnsureSession(priv-primary) 0) {
return NULL;
@@ -4249,8 +4254,9 @@ esxDomainSnapshotCreateXML(virDomainPtr domain, const
char *xmlDesc,
if (esxVI_CreateSnapshot_Task(priv-primary, virtualMachine-obj,
def-name, def-description,
- esxVI_Boolean_True,
- esxVI_Boolean_False, task) 0 ||
+ diskOnly ? esxVI_Boolean_False :
esxVI_Boolean_True,
+ quiesce ? esxVI_Boolean_True :
esxVI_Boolean_False,
+ task) 0 ||
esxVI_WaitForTaskCompletion(priv-primary, task, domain-uuid,
esxVI_Occurrence_RequiredItem,
priv-parsedUri-autoAnswer,
taskInfoState,
--
1.7.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] xen: Add interface versions for Xen 4.3
Stefan Bader wrote: On 17.07.2013 18:42, Jim Fehlig wrote: All of the existing logic uses decimal for the interface version numbers. I've changed all uses of hex to decimal, added the commits that bumped the versions, and pushed the patch. Thanks, yes I was struggling myself a bit which way to go. As for the references to commits. I would rather use git, previous references looked to be from mecurial. Right. The previous ones predated the xen project's move to git, when the project was using mercurial. Regards, Jim -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] add console support in libxl
Bamvor Jian Zhang wrote:
Hi, Jim
thanks your reply. one comment below.
已写入 Jim Fehlig jfeh...@suse.com On 07/04/2013 05:58 AM, Bamvor Jian
Zhang wrote:
BTW, do all of these types work with Xen? I've only tested with type 'pty',
which works fine with both pv and hvm guests.
i only test the pty type too. lots of type is introduced in 2b84e445(Add
libxenlight driver), i add the missing type compare with qemu driver. maybe
it will be used in future. for now, only pty is needed for my console patch.
I found some time today to test type={unix,tcp,file,udp} and they seem
to work fine with Xen 4.3, but HVM guests only. I suppose that is
expected since qemu is not being used for serial ports for PV guests.
Can you fix all my previous comments, rebase the patch, and send a V2?
Oh, and one more thing...
@@ -4739,6 +4857,7 @@ static virDriver libxlDriver = {
.domainManagedSave = libxlDomainManagedSave, /* 0.9.2 */
.domainHasManagedSaveImage = libxlDomainHasManagedSaveImage, /* 0.9.2 */
.domainManagedSaveRemove = libxlDomainManagedSaveRemove, /* 0.9.2 */
+.domainOpenConsole = libxlDomainOpenConsole, /* 1.0.8 */
.domainIsActive = libxlDomainIsActive, /* 0.9.0 */
.domainIsPersistent = libxlDomainIsPersistent, /* 0.9.0 */
.domainIsUpdated = libxlDomainIsUpdated, /* 0.9.0 */
AFAIK, the next release will be 1.1.1.
Regards,
Jim
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] add console support in libxl
On 07/17/2013 03:21 PM, Jim Fehlig wrote:
Oh, and one more thing...
@@ -4739,6 +4857,7 @@ static virDriver libxlDriver = {
.domainManagedSave = libxlDomainManagedSave, /* 0.9.2 */
.domainHasManagedSaveImage = libxlDomainHasManagedSaveImage, /* 0.9.2 */
.domainManagedSaveRemove = libxlDomainManagedSaveRemove, /* 0.9.2 */
+.domainOpenConsole = libxlDomainOpenConsole, /* 1.0.8 */
.domainIsActive = libxlDomainIsActive, /* 0.9.0 */
.domainIsPersistent = libxlDomainIsPersistent, /* 0.9.0 */
.domainIsUpdated = libxlDomainIsUpdated, /* 0.9.0 */
AFAIK, the next release will be 1.1.1.
Correct; after 1.0.6, we released 1.1.0, and are now working towards 1.1.1.
--
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] LXC: autostart feature does set all interfaces to state up.
Am 12.07.2013 03:36, schrieb Gao feng: On 07/11/2013 07:58 PM, Richard Weinberger wrote: Am 11.07.2013 11:49, schrieb Daniel P. Berrange: On Thu, Jul 11, 2013 at 11:44:48AM +0200, Richard Weinberger wrote: Am 11.07.2013 11:42, schrieb Gao feng: On 07/11/2013 03:18 PM, Richard Weinberger wrote: This morning I've installed a wrapper around ip to show me the process tree upon ip link ... down is used. The log showed this: 769 ?Ss 0:00 /usr/lib/systemd/systemd-udevd 17759 ?S 0:00 \_ /usr/lib/systemd/systemd-udevd 17764 ?S 0:00 \_ /usr/lib/systemd/systemd-udevd 17772 ?S 0:00 \_ /usr/lib/systemd/systemd-udevd 19477 ?S 0:00 | \_ /bin/bash /sbin/ifdown veth5 -o hotplug 19910 ?S 0:00 | \_ /sbin/ip link set dev veth5 down Now I have to urge to use a Kantholz. ;-) hmmm... it's systemd... I have no idea now... :( TBH it is not systemd's fault. OpenSUSE's /usr/lib/udev/rules.d/77-network.rules did not white list veth* devices. Therefore systemd-udevd called ifup/down and other hotplug magic. Ah ha, that's a nice issue :-) I assume you've filed a bug against opensuse to fix this ? Can you post a link to the bug here for the sake of archive records. Sure: https://bugzilla.novell.com/show_bug.cgi?id=829033 It's good news we know what causes veth device down. :) How does Fedora deal with veth devices? SUSE folks think that this is a more likely a libvirt issue and closed my bug report as invalid... Thanks, //richard -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] esx: Support for disk-only and quiescing snapshots.
On 07/17/2013 12:28 PM, Geoff Hickey wrote: Add support for creating disk-only (no memory) snapshots in esx, and for quiescing the VM before taking the snapshot. The VMware API supports these operations directly, so adding support to libvirt is just a matter of setting the flags correctly when calling VMware. VIR_DOMAIN_SNAPSHOT_CREATE_DISK_ONLY and VIR_DOMAIN_SNAPSHOT_CREATE_QUIESCE are now valid flags for esx. --- src/esx/esx_driver.c | 14 ++ 1 files changed, 10 insertions(+), 4 deletions(-) ACK and pushed. diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c index fbe43c2..d69576d 100644 --- a/src/esx/esx_driver.c +++ b/src/esx/esx_driver.c @@ -4209,9 +4209,14 @@ esxDomainSnapshotCreateXML(virDomainPtr domain, const char *xmlDesc, esxVI_TaskInfoState taskInfoState; char *taskInfoErrorMessage = NULL; virDomainSnapshotPtr snapshot = NULL; +bool diskOnly = (flags VIR_DOMAIN_SNAPSHOT_CREATE_DISK_ONLY) != 0; +bool quiesce = (flags VIR_DOMAIN_SNAPSHOT_CREATE_QUIESCE) != 0; -/* ESX has no snapshot metadata, so this flag is trivial. */ -virCheckFlags(VIR_DOMAIN_SNAPSHOT_CREATE_NO_METADATA, NULL); +/* ESX supports disk-only and quiesced snapshots, but has no snapshot * + * metadata. */ Comment is a bit misleading; I reworded it slightly to emphasize that the flag is accepted and trivial because it is libvirt that tracks no metadata. Hmm, the ESX driver is missing support for virDomainListAllSnapshots, along with its ability to filter based on whether a snapshot was disk-only or included memory. Is that something you are interested in tackling as a followup? -- 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] esx: Support for disk-only and quiescing snapshots.
Thanks Eric! I'll have a look at the missing virDomainListAllSnapshots. It doesn't block what I'm doing at the moment, so it's not my highest priority but I might need it in the future. - Geoff Hickey - On Wed, Jul 17, 2013 at 5:42 PM, Eric Blake ebl...@redhat.com wrote: On 07/17/2013 12:28 PM, Geoff Hickey wrote: Add support for creating disk-only (no memory) snapshots in esx, and for quiescing the VM before taking the snapshot. The VMware API supports these operations directly, so adding support to libvirt is just a matter of setting the flags correctly when calling VMware. VIR_DOMAIN_SNAPSHOT_CREATE_DISK_ONLY and VIR_DOMAIN_SNAPSHOT_CREATE_QUIESCE are now valid flags for esx. --- src/esx/esx_driver.c | 14 ++ 1 files changed, 10 insertions(+), 4 deletions(-) ACK and pushed. diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c index fbe43c2..d69576d 100644 --- a/src/esx/esx_driver.c +++ b/src/esx/esx_driver.c @@ -4209,9 +4209,14 @@ esxDomainSnapshotCreateXML(virDomainPtr domain, const char *xmlDesc, esxVI_TaskInfoState taskInfoState; char *taskInfoErrorMessage = NULL; virDomainSnapshotPtr snapshot = NULL; +bool diskOnly = (flags VIR_DOMAIN_SNAPSHOT_CREATE_DISK_ONLY) != 0; +bool quiesce = (flags VIR_DOMAIN_SNAPSHOT_CREATE_QUIESCE) != 0; -/* ESX has no snapshot metadata, so this flag is trivial. */ -virCheckFlags(VIR_DOMAIN_SNAPSHOT_CREATE_NO_METADATA, NULL); +/* ESX supports disk-only and quiesced snapshots, but has no snapshot * + * metadata. */ Comment is a bit misleading; I reworded it slightly to emphasize that the flag is accepted and trivial because it is libvirt that tracks no metadata. Hmm, the ESX driver is missing support for virDomainListAllSnapshots, along with its ability to filter based on whether a snapshot was disk-only or included memory. Is that something you are interested in tackling as a followup? -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv2 0/2] lxc/dac: avoid getgrouplist between fork/exec
v1 was here: https://www.redhat.com/archives/libvir-list/2013-July/msg00853.html Changes since then: split into two patches, and delay supplemental group computation until just before forking Eric Blake (2): security: framework for driver PreFork handler security_dac: compute supplemental groups before fork src/qemu/qemu_process.c | 3 +- src/security/security_dac.c | 63 - src/security/security_driver.h | 4 +++ src/security/security_manager.c | 16 +-- src/security/security_manager.h | 2 +- src/security/security_stack.c | 23 +++ 6 files changed, 88 insertions(+), 23 deletions(-) -- 1.8.3.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv2 1/2] security: framework for driver PreFork handler
A future patch wants the DAC security manager to be able to safely
get the supplemental group list for a given uid, but at the time
of a fork rather than during initialization so as to pick up on
live changes to the system's group database. This patch adds the
framework, including the possibility of a pre-fork callback
failing.
For now, any driver that implements a prefork callback must be
robust against the possibility of being part of a security stack
where a later element in the chain fails prefork. This means
that drivers cannot do any action that requires a call to postfork
for proper cleanup (no grabbing a mutex, for example). If this
is too prohibitive in the future, we would have to switch to a
transactioning sequence, where each driver has (up to) 3 callbacks:
PreForkPrepare, PreForkCommit, and PreForkAbort, to either clean
up or commit changes made during prepare.
* src/security/security_driver.h (virSecurityDriverPreFork): New
callback.
* src/security/security_manager.h (virSecurityManagerPreFork):
Change signature.
* src/security/security_manager.c (virSecurityManagerPreFork):
Optionally call into driver, and allow returning failure.
* src/security/security_stack.c (virSecurityDriverStack):
Wrap the handler for the stack driver.
* src/qemu/qemu_process.c (qemuProcessStart): Adjust caller.
Signed-off-by: Eric Blake ebl...@redhat.com
---
src/qemu/qemu_process.c | 3 ++-
src/security/security_driver.h | 4
src/security/security_manager.c | 16 ++--
src/security/security_manager.h | 2 +-
src/security/security_stack.c | 23 +++
5 files changed, 44 insertions(+), 4 deletions(-)
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 3d5e8f6..a594cc6 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -3730,7 +3730,8 @@ int qemuProcessStart(virConnectPtr conn,
virCommandDaemonize(cmd);
virCommandRequireHandshake(cmd);
-virSecurityManagerPreFork(driver-securityManager);
+if (virSecurityManagerPreFork(driver-securityManager) 0)
+goto cleanup;
ret = virCommandRun(cmd, NULL);
virSecurityManagerPostFork(driver-securityManager);
diff --git a/src/security/security_driver.h b/src/security/security_driver.h
index cc401e1..8735558 100644
--- a/src/security/security_driver.h
+++ b/src/security/security_driver.h
@@ -47,6 +47,8 @@ typedef int (*virSecurityDriverClose) (virSecurityManagerPtr
mgr);
typedef const char *(*virSecurityDriverGetModel) (virSecurityManagerPtr mgr);
typedef const char *(*virSecurityDriverGetDOI) (virSecurityManagerPtr mgr);
+typedef int (*virSecurityDriverPreFork) (virSecurityManagerPtr mgr);
+
typedef int (*virSecurityDomainRestoreImageLabel) (virSecurityManagerPtr mgr,
virDomainDefPtr def,
virDomainDiskDefPtr disk);
@@ -119,6 +121,8 @@ struct _virSecurityDriver {
virSecurityDriverGetModel getModel;
virSecurityDriverGetDOI getDOI;
+virSecurityDriverPreFork preFork;
+
virSecurityDomainSecurityVerify domainSecurityVerify;
virSecurityDomainSetImageLabel domainSetSecurityImageLabel;
diff --git a/src/security/security_manager.c b/src/security/security_manager.c
index 411a909..92fb504 100644
--- a/src/security/security_manager.c
+++ b/src/security/security_manager.c
@@ -193,11 +193,23 @@ virSecurityManagerPtr virSecurityManagerNew(const char
*name,
/*
* Must be called before fork()'ing to ensure mutex state
- * is sane for the child to use
+ * is sane for the child to use. A negative return means the
+ * child must not be forked; a successful return must be
+ * followed by a call to virSecurityManagerPostFork() in both
+ * parent and child.
*/
-void virSecurityManagerPreFork(virSecurityManagerPtr mgr)
+int virSecurityManagerPreFork(virSecurityManagerPtr mgr)
{
+int ret = 0;
+
virObjectLock(mgr);
+if (mgr-drv-preFork) {
+ret = mgr-drv-preFork(mgr);
+if (ret 0)
+virObjectUnlock(mgr);
+}
+
+return ret;
}
diff --git a/src/security/security_manager.h b/src/security/security_manager.h
index 711b354..9252830 100644
--- a/src/security/security_manager.h
+++ b/src/security/security_manager.h
@@ -47,7 +47,7 @@ virSecurityManagerPtr virSecurityManagerNewDAC(const char
*virtDriver,
bool requireConfined,
bool dynamicOwnership);
-void virSecurityManagerPreFork(virSecurityManagerPtr mgr);
+int virSecurityManagerPreFork(virSecurityManagerPtr mgr);
void virSecurityManagerPostFork(virSecurityManagerPtr mgr);
void *virSecurityManagerGetPrivateData(virSecurityManagerPtr mgr);
diff --git a/src/security/security_stack.c b/src/security/security_stack.c
index 1e229d7..ed69b9c 100644
--- a/src/security/security_stack.c
+++ b/src/security/security_stack.c
@@ -112,6 +112,27 @@
[libvirt] [PATCHv2 2/2] security_dac: compute supplemental groups before fork
Commit 75c1256 states that virGetGroupList must not be called
between fork and exec, then commit ee777e99 promptly violated
that for lxc's use of virSecurityManagerSetProcessLabel. Hoist
the supplemental group detection to the time that the security
manager is created. Qemu is safe, as it uses
virSecurityManagerSetChildProcessLabel which in turn uses
virCommand to determine supplemental groups.
This does not fix the fact that virSecurityManagerSetProcessLabel
calls virSecurityDACParseIds calls parseIds which eventually
calls getpwnam_r, which also violates fork/exec async-signal-safe
safety rules, but so far no one has complained of hitting
deadlock in that case.
* src/security/security_dac.c (_virSecurityDACData): Track groups
in private data.
(virSecurityDACPreFork): New function, to set them.
(virSecurityDACClose): Clean up new fields.
(virSecurityDACGetIds): Alter signature.
(virSecurityDACSetSecurityHostdevLabelHelper)
(virSecurityDACSetChardevLabel, virSecurityDACSetProcessLabel)
(virSecurityDACSetChildProcessLabel): Update callers.
Signed-off-by: Eric Blake ebl...@redhat.com
---
src/security/security_dac.c | 63 +++--
1 file changed, 44 insertions(+), 19 deletions(-)
diff --git a/src/security/security_dac.c b/src/security/security_dac.c
index 9b5eaa8..00d04bf 100644
--- a/src/security/security_dac.c
+++ b/src/security/security_dac.c
@@ -43,6 +43,8 @@ typedef virSecurityDACData *virSecurityDACDataPtr;
struct _virSecurityDACData {
uid_t user;
gid_t group;
+gid_t *groups;
+int ngroups;
bool dynamicOwnership;
};
@@ -146,7 +148,8 @@ virSecurityDACParseIds(virDomainDefPtr def, uid_t *uidPtr,
gid_t *gidPtr)
static int
virSecurityDACGetIds(virDomainDefPtr def, virSecurityDACDataPtr priv,
- uid_t *uidPtr, gid_t *gidPtr)
+ uid_t *uidPtr, gid_t *gidPtr,
+ gid_t **groups, int *ngroups)
{
int ret;
@@ -157,8 +160,13 @@ virSecurityDACGetIds(virDomainDefPtr def,
virSecurityDACDataPtr priv,
return -1;
}
-if ((ret = virSecurityDACParseIds(def, uidPtr, gidPtr)) = 0)
+if ((ret = virSecurityDACParseIds(def, uidPtr, gidPtr)) = 0) {
+if (groups)
+*groups = NULL;
+if (ngroups)
+ngroups = 0;
return ret;
+}
if (!priv) {
virReportError(VIR_ERR_INTERNAL_ERROR,
@@ -171,6 +179,10 @@ virSecurityDACGetIds(virDomainDefPtr def,
virSecurityDACDataPtr priv,
*uidPtr = priv-user;
if (gidPtr)
*gidPtr = priv-group;
+if (groups)
+*groups = priv-groups;
+if (ngroups)
+*ngroups = priv-ngroups;
return 0;
}
@@ -250,8 +262,10 @@ virSecurityDACOpen(virSecurityManagerPtr mgr
ATTRIBUTE_UNUSED)
}
static int
-virSecurityDACClose(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED)
+virSecurityDACClose(virSecurityManagerPtr mgr)
{
+virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr);
+VIR_FREE(priv-groups);
return 0;
}
@@ -269,6 +283,21 @@ virSecurityDACGetDOI(virSecurityManagerPtr mgr
ATTRIBUTE_UNUSED)
}
static int
+virSecurityDACPreFork(virSecurityManagerPtr mgr)
+{
+virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr);
+int ngroups;
+
+VIR_FREE(priv-groups);
+priv-ngroups = 0;
+if ((ngroups = virGetGroupList(priv-user, priv-group,
+ priv-groups)) 0)
+return -1;
+priv-ngroups = ngroups;
+return 0;
+}
+
+static int
virSecurityDACSetOwnership(const char *path, uid_t uid, gid_t gid)
{
VIR_INFO(Setting DAC user and group on '%s' to '%ld:%ld',
@@ -448,7 +477,7 @@ virSecurityDACSetSecurityHostdevLabelHelper(const char
*file,
uid_t user;
gid_t group;
-if (virSecurityDACGetIds(def, priv, user, group))
+if (virSecurityDACGetIds(def, priv, user, group, NULL, NULL))
return -1;
return virSecurityDACSetOwnership(file, user, group);
@@ -702,7 +731,7 @@ virSecurityDACSetChardevLabel(virSecurityManagerPtr mgr,
uid_t user;
gid_t group;
-if (virSecurityDACGetIds(def, priv, user, group))
+if (virSecurityDACGetIds(def, priv, user, group, NULL, NULL))
return -1;
switch (dev-type) {
@@ -996,26 +1025,20 @@ virSecurityDACSetProcessLabel(virSecurityManagerPtr mgr,
{
uid_t user;
gid_t group;
-virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr);
gid_t *groups;
int ngroups;
-int ret = -1;
+virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr);
-if (virSecurityDACGetIds(def, priv, user, group))
-return -1;
-ngroups = virGetGroupList(user, group, groups);
-if (ngroups 0)
+if (virSecurityDACGetIds(def, priv, user, group, groups, ngroups))
return -1;
-VIR_DEBUG(Dropping privileges of DEF to %u:%u,
- (unsigned int) user, (unsigned int) group);
+VIR_DEBUG(Dropping privileges of DEF to %u:%u,
Re: [libvirt] [PATCH] Split up platfrom specifics from bridge driver
On 07/04/2013 10:25 AM, Roman Bogorodskiy wrote: This is a second step for making bridge_driver more portable. This finally percolated to the top of my todo list; I'll have a review shortly. This one splits up bridge driver into platform specific and general parts. Platform specifc parts are mostly firewalling stuff. So the support for new platforms should be added by implementing bridge_driver_$platform.c However, you might notice I didn't move some ports of the platform specific code. E.g. I left networkEnableIpForwarding in bridge_driver.c because it feels like the implementation for e.g. all BSDs would be the same (while firewalling stuff would be different on BSDs) to avoid code copy/paste. Roman Bogorodskiy (1): Split up platfrom specifics from bridge driver po/POTFILES.in |1 + src/Makefile.am |5 +- src/network/bridge_driver.c | 729 +- src/network/bridge_driver_linux.c| 709 + src/network/bridge_driver_noop.c | 80 src/network/bridge_driver_platform.c | 32 ++ src/network/bridge_driver_platform.h | 77 7 files changed, 915 insertions(+), 718 deletions(-) Looks big, but the bulk of it is code motion, so it should be reasonable. create mode 100644 src/network/bridge_driver_linux.c create mode 100644 src/network/bridge_driver_noop.c create mode 100644 src/network/bridge_driver_platform.c create mode 100644 src/network/bridge_driver_platform.h -- 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] Split up platfrom specifics from bridge driver
On 07/04/2013 10:25 AM, Roman Bogorodskiy wrote:
* Functions were renamed: s/Iptables/Firewall/
to make names more general.
* Platform specific things (e.g. firewalling and route
collision checks) were moved into bridge_driver_platform
* Created two platform specific implementations:
- bridge_driver_linux: Linux implementation using iptables,
it's actually the code moved from bridge_driver.c
- bridge_driver_noop: dump implementation that does nothing
I've let review sit for so long that it no longer applies, and needs a
rebasing; but I'll at least skim the patch looking for potential issues
to be aware of during the rebase...
---
po/POTFILES.in |1 +
src/Makefile.am |5 +-
src/network/bridge_driver.c | 729
+-
src/network/bridge_driver_linux.c| 709 +
src/network/bridge_driver_noop.c | 80
src/network/bridge_driver_platform.c | 32 ++
src/network/bridge_driver_platform.h | 77
7 files changed, 915 insertions(+), 718 deletions(-)
create mode 100644 src/network/bridge_driver_linux.c
create mode 100644 src/network/bridge_driver_noop.c
create mode 100644 src/network/bridge_driver_platform.c
create mode 100644 src/network/bridge_driver_platform.h
If you generate the email with 'git send-email/format-patch
-O/path/to/file', then you can use a file that prioritizes .h files to
come first in the diff listing; this can sometimes help in interface
reviews.
-/* Main driver state */
-struct network_driver {
You moved this struct out of a .c file into driver_platform.h; as a
result, this poor choice of naming will potentially cause problems to
more files that include the .h. Can you please do a separate cleanup
pre-patch that s/network_driver/virNetworkDriver/, and uses a typedef so
that you don't have to repeat 'struct' everywhere?
-virMutex lock;
-
-virNetworkObjList networks;
Also, it might be worth a patch to make this struct inherit from
virObjectLockable, instead of open-coding the locking ourselves. But
that's separate from your goal of splitting the files, so it's not a
prerequisite.
@@ -114,7 +99,7 @@ static int networkStartNetworkExternal(struct
network_driver *driver,
static int networkShutdownNetworkExternal(struct network_driver *driver,
virNetworkObjPtr network);
-static void networkReloadIptablesRules(struct network_driver *driver);
+static void networkReloadFirewallRules(struct network_driver *driver);
For ease of review, it might be worth splitting the function renames
into a separate patch from the code motion.
+++ b/src/network/bridge_driver_platform.c
@@ -0,0 +1,32 @@
+#include bridge_driver_platform.h
+
+#if defined(__linux__)
+# include bridge_driver_linux.c
+#else
+# include bridge_driver_noop.c
One .c including another is not typical, but we've done it before for
virthread.c; seems like a reasonable way to split things, as long as all
branches of the file declare the same functions.
The other alternative for splitting things is the way we've done it in
src/security - the public interface is in virSecurityManager, and
basically forwards all calls into a private interface, where each
backend registers a struct of callback pointers. That method is a bit
more robust - you can add a callback to one driver without having to
remember to add it to all drivers simultaneously. With your approach,
if we add a new function in bridge_driver_linux.c, we MUST remember to
add the same function in bridge_driver_noop.c, or it will break
compilation on non-Linux, but developers that don't use Linux will be
unaware of the breakage. With the callback approach, not adding a
callback to bridge_driver_noop.c is not fatal; the manager wrapper would
simply know to do nothing for a NULL callback.
So, although I like the split, I can't help but wonder if your rebase
should take the road of adjusting things to use a callback struct,
rather than requiring matching implementations across multiple files.
--
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
