Re: [libvirt] [PATCH] virsh: fix change-media bug on disk block type
On 07/23/2013 12:20 AM, Eric Blake wrote:
On 07/22/2013 01:40 AM, Guannan Ren wrote:
Resolves:https://bugzilla.redhat.com/show_bug.cgi?id=923053
When cdrom is block type, the virsh change-media failed to insert
source info because virsh uses source block='/dev/sdb'/ while
the correct name of the attribute for block disks is dev.
---
tools/virsh-domain.c | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
ACK.
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 606bcdf..8cafce4 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -9866,8 +9866,10 @@ vshPrepareDiskXML(xmlNodePtr disk_node,
if (source) {
new_node = xmlNewNode(NULL, BAD_CAST source);
-xmlNewProp(new_node, (const xmlChar *)disk_type,
- (const xmlChar *)source);
+if (STREQ(disk_type, block))
+xmlNewProp(new_node, BAD_CAST dev, BAD_CAST source);
+else
+xmlNewProp(new_node, BAD_CAST disk_type, BAD_CAST source);
xmlAddChild(disk_node, new_node);
} else if (type == VSH_PREPARE_DISK_XML_INSERT) {
vshError(NULL, _(No source is specified for inserting
media));
Thanks and pushed
Guannan
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Make locking more debuggable from logs
On 07/22/2013 06:31 PM, Eric Blake wrote: On 07/22/2013 07:58 AM, Daniel P. Berrange wrote: I'm really inclined to say that anyone wanting todo lock debugging should just use systemtap / dtrace todo it, since it is better in every way. I tend to agree on that point, with one caveat - we need to have good documentation on how to do systemtap debugging of libvirt, in order to make it easy to point someone to the steps they need to follow to benefit from it. Our existing web pages on hacking are a bit sparse on this front. OK, thanks for your input. My point was to enable this for people who don't want to (or can't) use systemtap for various reasons, but I guess nobody can be *that* lazy to do 'package_app install_command systemtap' and then cleanup afterwards. I like your idea of explaining systemtap debugging with libvirt on wiki/documentation. I'll try to combine it with additional systemtap examples and put it all together somehow. But I need you to check my grammar, since it tends to be a longer text. Thanks again and have a nice day, Martin -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [Xen-devel] [PATCH] libxl: Correctly initialize vcpu bitmap
On 22.07.2013 21:39, Konrad Rzeszutek Wilk wrote:
On Mon, Jul 22, 2013 at 12:51:05PM +0200, Stefan Bader wrote:
This fixes the basic setup but there is likely more to do if things
like manual CPU hirarchy (nodes, cores, threads) to be working.
Cross-posting to xen-devel to make sure I am doing things correctly.
-Stefan
From 1ec5e7ea0d3498b9f61b83e8aed87cc3cae106de Mon Sep 17 00:00:00 2001
From: Stefan Bader stefan.ba...@canonical.com
Date: Fri, 19 Jul 2013 15:20:00 +0200
Subject: [PATCH] libxl: Correctly initialize vcpu bitmap
The avai_vcpu bitmap has to be allocated before it can be used (using
avail_vcpu ?
*sigh* Yeah, of course I cannot spell. :-P
the maximum allowed value for that). Then for each available VCPU the
bit in the mask has to be set (libxl_bitmap_set takes a bit position
as an argument, not the number of bits to set).
Without this, I would always only get one VCPU for guests created
through libvirt/libxl.
Signed-off-by: Stefan Bader stefan.ba...@canonical.com
The libxl calling logic looks Ok to me. So from the libxl perspective
you can tack on Reviewed-by: Konrad Rzeszutek Wilk konrad.w...@oracle.com
Thanks, I will leave that (as well as the complimentary fixing of missing
lletters to the libvirt maintainers. :)
-Stefan
---
src/libxl/libxl_conf.c | 14 +++---
1 file changed, 11 insertions(+), 3 deletions(-)
diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
index 4a0fba9..7592dd2 100644
--- a/src/libxl/libxl_conf.c
+++ b/src/libxl/libxl_conf.c
@@ -331,7 +331,8 @@ error:
}
static int
-libxlMakeDomBuildInfo(virDomainDefPtr def, libxl_domain_config *d_config)
+libxlMakeDomBuildInfo(libxlDriverPrivatePtr driver, virDomainDefPtr def,
+ libxl_domain_config *d_config)
{
libxl_domain_build_info *b_info = d_config-b_info;
int hvm = STREQ(def-os.type, hvm);
@@ -343,8 +344,15 @@ libxlMakeDomBuildInfo(virDomainDefPtr def,
libxl_domain_config *d_config)
libxl_domain_build_info_init_type(b_info, LIBXL_DOMAIN_TYPE_HVM);
else
libxl_domain_build_info_init_type(b_info, LIBXL_DOMAIN_TYPE_PV);
+
b_info-max_vcpus = def-maxvcpus;
-libxl_bitmap_set((b_info-avail_vcpus), def-vcpus);
+if (libxl_cpu_bitmap_alloc(driver-ctx, b_info-avail_vcpus,
+ def-maxvcpus))
+goto error;
+libxl_bitmap_set_none(b_info-avail_vcpus);
+for (i = 0; i def-vcpus; i++)
+libxl_bitmap_set((b_info-avail_vcpus), i);
+
if (def-clock.ntimers 0
def-clock.timers[0]-name == VIR_DOMAIN_TIMER_NAME_TSC) {
switch (def-clock.timers[0]-mode) {
@@ -802,7 +810,7 @@ libxlBuildDomainConfig(libxlDriverPrivatePtr driver,
if (libxlMakeDomCreateInfo(driver, def, d_config-c_info) 0)
return -1;
-if (libxlMakeDomBuildInfo(def, d_config) 0) {
+if (libxlMakeDomBuildInfo(driver, def, d_config) 0) {
return -1;
}
--
1.7.9.5
___
Xen-devel mailing list
xen-de...@lists.xen.org
http://lists.xen.org/xen-devel
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] Add virDBusMessage(Encode,Decode) stubs
On Mon, Jul 22, 2013 at 02:32:49PM -0400, Roman Bogorodskiy wrote:
Commit 834c9c94 introduced virDBusMessageEncode and
virDBusMessageDecode functions, however corresponding stubs
were not added to !WITH_DBUS section, therefore 'make check'
started to fail when compiled w/out dbus support like that:
Expected symbol virDBusMessageDecode is not in ELF library
---
src/util/virdbus.c | 18 ++
1 file changed, 18 insertions(+)
diff --git a/src/util/virdbus.c b/src/util/virdbus.c
index ee99f7f..6221bdc 100644
--- a/src/util/virdbus.c
+++ b/src/util/virdbus.c
@@ -1222,4 +1222,22 @@ int virDBusMessageRead(DBusMessage *msg
ATTRIBUTE_UNUSED,
return -1;
}
+int virDBusMessageEncode(DBusMessage* msg ATTRIBUTE_UNUSED,
+ const char *types ATTRIBUTE_UNUSED,
+ ...)
+{
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ %s, _(DBus support not compiled into this binary));
+return -1;
+}
+
+int virDBusMessageDecode(DBusMessage* msg ATTRIBUTE_UNUSED,
+ const char *types ATTRIBUTE_UNUSED,
+ ...)
+{
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ %s, _(DBus support not compiled into this binary));
+return -1;
+}
+
#endif /* ! WITH_DBUS */
Ahh whoops. I was being too clever by assuming that my use of WITH_DBUS
in the test code would take care of this, forgetting about the symbol
file checks
ACK
Daniel
--
|: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] bridge driver: use more general function names
On Mon, Jul 22, 2013 at 02:14:22PM -0400, Roman Bogorodskiy wrote: Continue preparation for extracting platform-specific parts from bridge_driver: s/Iptables/Firewall/ for firewall related function names. --- src/network/bridge_driver.c | 70 ++--- 1 file changed, 35 insertions(+), 35 deletions(-) ACK Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu:Delete sockets which act as UNIX domain socket server
On Fri, Jul 19, 2013 at 02:12:06AM +, Wangyufei (A) wrote:
When I shutdown a vm, I found sockets which act as UNIX domain socket server
were not deleted. When I add the following code, it work out.
Signed-off-by: WangYufei
james.wangyu...@huawei.commailto:james.wangyu...@huawei.com
---
src/qemu/qemu_process.c |7 +++
1 files changed, 7 insertions(+), 0 deletions(-)
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 3d5e8f6..e794f37 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -4086,6 +4086,13 @@ void qemuProcessStop(virQEMUDriverPtr driver,
priv-monConfig = NULL;
}
+/* remove socket which acts as UNIX domain socket server */
+for (i = 0; i vm-def-nchannels; i++) {
+if ((vm-def-channels[i]-source.type == VIR_DOMAIN_CHR_TYPE_UNIX)
+vm-def-channels[i]-source.data.nix.listen)
+unlink(vm-def-channels[i]-source.data.nix.path);
+}
+
We should do the same for vm-def-serials, vm-def-parallels
and vm-def-consoles while we're here, since they all are backed
by char-devs.
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] Make locking more debuggable from logs
On Tue, Jul 23, 2013 at 08:34:35AM +0200, Martin Kletzander wrote: On 07/22/2013 06:31 PM, Eric Blake wrote: On 07/22/2013 07:58 AM, Daniel P. Berrange wrote: I'm really inclined to say that anyone wanting todo lock debugging should just use systemtap / dtrace todo it, since it is better in every way. I tend to agree on that point, with one caveat - we need to have good documentation on how to do systemtap debugging of libvirt, in order to make it easy to point someone to the steps they need to follow to benefit from it. Our existing web pages on hacking are a bit sparse on this front. OK, thanks for your input. My point was to enable this for people who don't want to (or can't) use systemtap for various reasons, but I guess nobody can be *that* lazy to do 'package_app install_command systemtap' and then cleanup afterwards. I like your idea of explaining systemtap debugging with libvirt on wiki/documentation. I'll try to combine it with additional systemtap examples and put it all together somehow. But I need you to check my grammar, since it tends to be a longer text. Could add an example systemtap script demonstrating how todo it in the examples/systemtap directory too. Let me know if you need any assistance with systemtap. There's a little bit of a learning curve, but usually not as bad as people fear it will be. IME, it is well worth any effect spent learning how to use it, you'll make back the time many times over once you know it. 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] [Qemu-devel] [Question] why x2apic's set by default without host support(on Nehalem CPU).
On Tue, Jul 23, 2013 at 10:44:48 +0800, Peter Huang(Peng) wrote: libvirt's host-passthrough uses -cpu host', and it -cpu host enables every feature that can be enabled on the host. From my test results, I found that even when use host-passthrough mode, VM's cpu features are very different from host, this doesn't match what host-passthrough mode's explanation. libvirt's option exlanation: With this mode, the CPU visible to the guest should be exactly the same as the host CPU even in the aspects that libvirt does not understand. The libvirt documentation is what needs to be updated. While host-passthrough is asking for a CPU which is as close as possible to the real host CPU, there are features that need special handling before they can be provided to a guest. And if the hypervisor does not provide that handling, it may just filter such feature out. Also if some features can be efficiently provided to a guest even though the host CPU does not provide them (x2apic is an example of such feature), they may be provided to a guest. Jirka -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] bridge driver: use more general function names
On 07/23/2013 12:15 PM, Daniel P. Berrange wrote: On Mon, Jul 22, 2013 at 02:14:22PM -0400, Roman Bogorodskiy wrote: Continue preparation for extracting platform-specific parts from bridge_driver: s/Iptables/Firewall/ for firewall related function names. --- src/network/bridge_driver.c | 70 ++--- 1 file changed, 35 insertions(+), 35 deletions(-) ACK Daniel Pushed. Jan -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Add virDBusMessage(Encode,Decode) stubs
On 07/23/2013 12:15 PM, Daniel P. Berrange wrote: On Mon, Jul 22, 2013 at 02:32:49PM -0400, Roman Bogorodskiy wrote: Commit 834c9c94 introduced virDBusMessageEncode and virDBusMessageDecode functions, however corresponding stubs were not added to !WITH_DBUS section, therefore 'make check' started to fail when compiled w/out dbus support like that: Expected symbol virDBusMessageDecode is not in ELF library --- src/util/virdbus.c | 18 ++ 1 file changed, 18 insertions(+) ACK Now pushed. Jan -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [Qemu-devel] [Question] why x2apic's set by default without host support(on Nehalem CPU).
libvirt's host-passthrough uses -cpu host', and it -cpu host enables every feature that can be enabled on the host. From my test results, I found that even when use host-passthrough mode, VM's cpu features are very different from host, this doesn't match what host-passthrough mode's explanation. libvirt's option exlanation: With this mode, the CPU visible to the guest should be exactly the same as the host CPU even in the aspects that libvirt does not understand. If the behavior of -cpu host doesn't match libvirt expectations, we need to clarify what are the requirements, and maybe have a try to be close to host CPU mode as opposed to the current enable everything that can be enabled mode. Does the results above indicates what you mean to do? or the current way doesn't work as we expect. Thanks! -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3 2/7] conf: Introduce new XML tag mode for disk source
Il 19/07/2013 14:32, John Ferlan ha scritto:
There are two ways to use a iSCSI LUN as disk source for qemu.
* The LUN's path as it shows up on host, e.g.
/dev/disk/by-path/ip-$ip:3260-iscsi-$iqn-fc18:iscsi.iscsi0-lun-1
* The libiscsi URI from the storage pool source element host attribute, e.g.
iscsi://demo.org:6000/iqn.1992-01.com.example/1
For a volume type disk, if the specified pool is of iscsi
type, we should support to use the LUN in either of above 2 ways.
That's why to introduce a new XML tag mode for the disk source
(libvirt should support iscsi pool with libiscsi, but it's another
new feature, which should be done later).
The mode can be either of host or direct. Use host to indicate
use of the LUN with the path as it shows up on host. Use direct to
indicate to use it with the source pool host URI (future patches may support
to use network type libvirt storage too, e.g. Ceph)
What is the default?
Should 'direct' be valid for non-network pools?
Paolo
---
docs/formatdomain.html.in | 11 -
docs/schemas/domaincommon.rng | 8
src/conf/domain_conf.c | 22 +-
src/conf/domain_conf.h | 22 ++
.../qemuxml2argv-disk-source-pool-mode.xml | 48
++
tests/qemuxml2xmltest.c| 1 +
6 files changed, 110 insertions(+), 2 deletions(-)
create mode 100644
tests/qemuxml2argvdata/qemuxml2argv-disk-source-pool-mode.xml
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index b1c3bfc..7601aaa 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -1601,7 +1601,16 @@
codepool/code and codevolume/code. Attribute
codepool/code
specifies the name of storage pool (managed by libvirt) where the
disk
source resides, and attribute codevolume/code specifies the name
of
-storage volume (managed by libvirt) used as the disk source.
+storage volume (managed by libvirt) used as the disk source. For a
+volume type disk, if the underlying storage pool is iscsi,
attribute
+codemode/code (span class=sincesince 1.1.1/span) can be
used
+to indicate how to represent the LUN as the disk source. The value
+host indicates to use the LUN's path as it shows up on host, e.g.
+
/dev/disk/by-path/ip-10.11.12.9:3260-iscsi-iqn.2013-06.fc:iscsi.iscsi0-lun-1).
+The value direct indicates to use the storage pool's
+codesource/code element codehost/code attribute as the
+disk source for the libiscsi URI, e.g.
+file=iscsi://demo.org:6000/iqn.1992-01.com.example/1.
span class=sinceSince 0.0.3; codetype='dir'/code since
0.7.5; codetype='network'/code since
0.8.7; codeprotocol='iscsi'/code since 1.0.4;
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 7a6852b..745b959 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -1174,6 +1174,14 @@
ref name=volName/
/attribute
optional
+ attribute name=mode
+choice
+ valuehost/value
+ valuedirect/value
+/choice
+ /attribute
+/optional
+optional
ref name=startupPolicy/
/optional
optional
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 57cd9b1..419958f 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -761,6 +761,11 @@ VIR_ENUM_IMPL(virDomainDiskDiscard,
VIR_DOMAIN_DISK_DISCARD_LAST,
default,
unmap,
ignore)
+VIR_ENUM_IMPL(virDomainDiskSourcePoolMode,
+ VIR_DOMAIN_DISK_SOURCE_POOL_MODE_LAST,
+ default,
+ host,
+ direct)
#define VIR_DOMAIN_XML_WRITE_FLAGS VIR_DOMAIN_XML_SECURE
#define VIR_DOMAIN_XML_READ_FLAGS VIR_DOMAIN_XML_INACTIVE
@@ -4645,10 +4650,12 @@ virDomainDiskSourcePoolDefParse(xmlNodePtr node,
{
char *pool = NULL;
char *volume = NULL;
+char *mode = NULL;
int ret = -1;
pool = virXMLPropString(node, pool);
volume = virXMLPropString(node, volume);
+mode = virXMLPropString(node, mode);
/* CD-ROM and Floppy allows no source */
if (!pool !volume)
@@ -4664,6 +4671,14 @@ virDomainDiskSourcePoolDefParse(xmlNodePtr node,
if (VIR_ALLOC(def-srcpool) 0)
goto cleanup;
+if (mode (def-srcpool-mode =
+ virDomainDiskSourcePoolModeTypeFromString(mode)) = 0) {
+virReportError(VIR_ERR_XML_ERROR,
+ _(unknown source mode '%s' for volume type disk),
+
Re: [libvirt] [PATCH v3 4/7] qemu: Translate the iscsi pool/volume disk source
Il 22/07/2013 12:30, Osier Yang ha scritto:
+def-srcpool-pooltype = pooldef-type;
+if (pooldef-type == VIR_STORAGE_POOL_ISCSI) {
+/* Default to use the LUN's path on host */
+if (!def-srcpool-mode)
+def-srcpool-mode =
VIR_DOMAIN_DISK_SOURCE_POOL_MODE_HOST;
+
+if (def-srcpool-mode ==
+VIR_DOMAIN_DISK_SOURCE_POOL_MODE_DIRECT) {
+if (qemuAddISCSIPoolSourceHost(def, pooldef) 0)
+goto cleanup;
+} else if (def-srcpool-mode ==
+ VIR_DOMAIN_DISK_SOURCE_POOL_MODE_HOST) {
+if (!(def-src = virStorageVolGetPath(vol)))
+goto cleanup;
+}
Ok, this answers my question. :)
I think the default mode should be direct, because otherwise things such
as persistent reservations do not work.
Paolo
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3 4/7] qemu: Translate the iscsi pool/volume disk source
On Tue, Jul 23, 2013 at 03:22:54PM +0200, Paolo Bonzini wrote:
Il 22/07/2013 12:30, Osier Yang ha scritto:
+def-srcpool-pooltype = pooldef-type;
+if (pooldef-type == VIR_STORAGE_POOL_ISCSI) {
+/* Default to use the LUN's path on host */
+if (!def-srcpool-mode)
+def-srcpool-mode =
VIR_DOMAIN_DISK_SOURCE_POOL_MODE_HOST;
+
+if (def-srcpool-mode ==
+VIR_DOMAIN_DISK_SOURCE_POOL_MODE_DIRECT) {
+if (qemuAddISCSIPoolSourceHost(def, pooldef) 0)
+goto cleanup;
+} else if (def-srcpool-mode ==
+ VIR_DOMAIN_DISK_SOURCE_POOL_MODE_HOST) {
+if (!(def-src = virStorageVolGetPath(vol)))
+goto cleanup;
+}
Ok, this answers my question. :)
I think the default mode should be direct, because otherwise things such
as persistent reservations do not work.
No, the default has to be host mode, because that is the only mode that
is guaranteed to be usable with any QEMU. The direct mode requires a
QEMU that is new enough, and a distro to have enabled it. We can't
rely on that as a default choice.
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 v5 4/5] storage: Support chap authentication for iscsi pool
Il 22/07/2013 22:31, John Ferlan ha scritto:
Although the XML for CHAP authentication with plain password
was introduced long ago, the function was never implemented. This
patch replaces the login/password mechanism by following the
'ceph' (or RBD) model of using a 'username' with a 'secret' which
has the authentication information.
This patch performs the authentication during startPool() processing
of pools with an authType of VIR_STORAGE_POOL_AUTH_CHAP specified
for iSCSI pools.
There are two types of CHAP configurations supported for iSCSI
authentication:
* Initiator Authentication
Forward, one-way; The initiator is authenticated by the target.
* Target Authentication
Reverse, Bi-directional, mutual, two-way; The target is authenticated
by the initiator; This method also requires Initiator Authentication
This only supports the Initiator Authentication. (I don't have any
enterprise iSCSI env for testing, only have a iSCSI target setup with
tgtd, which doesn't support Target Authentication).
Discovery authentication is not supported by tgt yet too. So this only
setup the session authentication by executing 3 iscsiadm commands, E.g:
% iscsiadm -m node --target iqn.2013-05.test:iscsi.foo --name \
node.session.auth.authmethod -v CHAP --op update
% iscsiadm -m node --target iqn.2013-05.test:iscsi.foo --name \
node.session.auth.username -v Jim --op update
% iscsiadm -m node --target iqn.2013-05.test:iscsi.foo --name \
node.session.auth.password -v Jimsecret --op update
---
src/storage/storage_backend_iscsi.c | 111
+++-
1 file changed, 110 insertions(+), 1 deletion(-)
diff --git a/src/storage/storage_backend_iscsi.c
b/src/storage/storage_backend_iscsi.c
index 54bcd14..388d6ed 100644
--- a/src/storage/storage_backend_iscsi.c
+++ b/src/storage/storage_backend_iscsi.c
@@ -32,6 +32,8 @@
#include unistd.h
#include sys/stat.h
+#include datatypes.h
+#include driver.h
#include virerror.h
#include storage_backend_scsi.h
#include storage_backend_iscsi.h
@@ -39,6 +41,7 @@
#include virlog.h
#include virfile.h
#include vircommand.h
+#include virobject.h
#include virrandom.h
#include virstring.h
@@ -658,9 +661,112 @@ virStorageBackendISCSICheckPool(virConnectPtr conn
ATTRIBUTE_UNUSED,
return ret;
}
+static int
+virStorageBackendISCSINodeUpdate(const char *portal,
+ const char *target,
+ const char *name,
+ const char *value)
+{
+ virCommandPtr cmd = NULL;
+ int status;
+ int ret = -1;
+
+ cmd = virCommandNewArgList(ISCSIADM,
+--mode, node,
+--portal, portal,
+--target, target,
+--op, update,
+--name, name,
+--value, value,
+NULL);
+
+if (virCommandRun(cmd, status) 0) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ _(Failed to update '%s' of node mode for target
'%s'),
+ name, target);
+goto cleanup;
+}
+
+ret = 0;
+cleanup:
+virCommandFree(cmd);
+return ret;
+}
static int
-virStorageBackendISCSIStartPool(virConnectPtr conn ATTRIBUTE_UNUSED,
+virStorageBackendISCSISetAuth(const char *portal,
+ virConnectPtr conn,
+ virStoragePoolDefPtr def)
+{
+virSecretPtr secret = NULL;
+unsigned char *secret_value = NULL;
+virStoragePoolAuthChap chap;
+int ret = -1;
+
+if (def-source.authType == VIR_STORAGE_POOL_AUTH_NONE)
+return 0;
+
+if (def-source.authType != VIR_STORAGE_POOL_AUTH_CHAP) {
+virReportError(VIR_ERR_XML_ERROR, %s,
+ _(iscsi pool only supports 'chap' auth type));
+return -1;
+}
+
+if (!conn) {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+ _(iscsi 'chap' authentication requires connection));
+return -1;
+}
Should this be more precisely not supported for autostarted pools?
+
+chap = def-source.auth.chap;
+if (chap.secret.uuidUsable)
+secret = virSecretLookupByUUID(conn, chap.secret.uuid);
+else
+secret = virSecretLookupByUsage(conn, VIR_SECRET_USAGE_TYPE_ISCSI,
+chap.secret.usage);
+
+if (secret) {
+size_t secret_size;
+secret_value =
+conn-secretDriver-secretGetValue(secret, secret_size, 0,
+
VIR_SECRET_GET_VALUE_INTERNAL_CALL);
+if (!secret_value) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ _(could not get the
Re: [libvirt] [PATCH v3 4/7] qemu: Translate the iscsi pool/volume disk source
Il 23/07/2013 15:26, Daniel P. Berrange ha scritto: Ok, this answers my question. :) I think the default mode should be direct, because otherwise things such as persistent reservations do not work. No, the default has to be host mode, because that is the only mode that is guaranteed to be usable with any QEMU. The direct mode requires a QEMU that is new enough, and a distro to have enabled it. We can't rely on that as a default choice. Volume sources are also new enough that you can assume a good QEMU. Host mode for iSCSI is broken. It also doesn't reconnect well if you have a network problem, because after a LUN rescan the inode may change. Paolo -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3 4/7] qemu: Translate the iscsi pool/volume disk source
On Tue, Jul 23, 2013 at 03:29:40PM +0200, Paolo Bonzini wrote: Il 23/07/2013 15:26, Daniel P. Berrange ha scritto: Ok, this answers my question. :) I think the default mode should be direct, because otherwise things such as persistent reservations do not work. No, the default has to be host mode, because that is the only mode that is guaranteed to be usable with any QEMU. The direct mode requires a QEMU that is new enough, and a distro to have enabled it. We can't rely on that as a default choice. Volume sources are also new enough that you can assume a good QEMU. No we can't assume that. New libvirt is frequently used with old QEMU and we want to have good default behaviour there. Host mode for iSCSI is broken. It also doesn't reconnect well if you have a network problem, because after a LUN rescan the inode may change. That's a much smaller level of brokeness, than if QEMU doesn't support the iscsi block protocol thus the default configuration won't even boot. 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] build: fix make rpm failure
util/virdbuspriv.h needed to be added to UTIL_SOURCES in the makefile. --- Pushed under build breaker rule. src/Makefile.am | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Makefile.am b/src/Makefile.am index 0eb3cb5..84372cb 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -86,7 +86,7 @@ UTIL_SOURCES = \ util/virclosecallbacks.c util/virclosecallbacks.h \ util/vircommand.c util/vircommand.h \ util/virconf.c util/virconf.h \ - util/virdbus.c util/virdbus.h \ + util/virdbus.c util/virdbus.h util/virdbuspriv.h\ util/virdnsmasq.c util/virdnsmasq.h \ util/virebtables.c util/virebtables.h \ util/virendian.h\ -- 1.7.11.7 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] Fix handling of DBus errors emitted by the bus itself
From: Daniel P. Berrange berra...@redhat.com
Current code for handling dbus errors only works for errors
received from the remote application itself. We must also
handle errors emitted by the bus itself, for example, when
it fails to spawn the target service.
Signed-off-by: Daniel P. Berrange berra...@redhat.com
---
src/util/virdbus.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/src/util/virdbus.c b/src/util/virdbus.c
index ee99f7f..e3d172e 100644
--- a/src/util/virdbus.c
+++ b/src/util/virdbus.c
@@ -1129,9 +1129,8 @@ int virDBusCallMethod(DBusConnection *conn,
call,
VIR_DBUS_METHOD_CALL_TIMEOUT_MILLIS,
error))) {
-virReportError(VIR_ERR_INTERNAL_ERROR,
- _(Cannot send to %s.%s on path %s with interface %s:
%s),
- destination, member, path, interface,
NULLSTR(error.message));
+virReportDBusServiceError(error.message ? error.message : unknown
error,
+ error.name);
goto cleanup;
}
--
1.8.1.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] Add logic for handling systemd-machined non-existance
From: Daniel P. Berrange berra...@redhat.com
If systemd machine does not exist, return -2 instead of -1,
so that applications don't need to repeat the tedious error
checking code
Signed-off-by: Daniel P. Berrange berra...@redhat.com
---
src/util/virsystemd.c | 13 ++-
tests/virsystemdmock.c | 14 +++
tests/virsystemdtest.c | 63 +++---
3 files changed, 66 insertions(+), 24 deletions(-)
diff --git a/src/util/virsystemd.c b/src/util/virsystemd.c
index 8477cd3..11d1153 100644
--- a/src/util/virsystemd.c
+++ b/src/util/virsystemd.c
@@ -26,6 +26,7 @@
#include virstring.h
#include viralloc.h
#include virutil.h
+#include virlog.h
#define VIR_FROM_THIS VIR_FROM_SYSTEMD
@@ -38,6 +39,8 @@
* @rootdir: root directory of machine filesystem
* @pidleader: PID of the leader process
* @slice: name of the slice to place the machine in
+ *
+ * Returns 0 on success, -1 on fatal error, or -2 if systemd-machine is not
available
*/
int virSystemdCreateMachine(const char *name,
const char *drivername,
@@ -117,6 +120,7 @@ int virSystemdCreateMachine(const char *name,
* allow further API calls to be made against the object.
*/
+VIR_DEBUG(Attempting to create machine via systemd);
if (virDBusCallMethod(conn,
NULL,
org.freedesktop.machine1,
@@ -135,8 +139,15 @@ int virSystemdCreateMachine(const char *name,
(unsigned int)pidleader,
rootdir ? rootdir : ,
1, Slice, s,
- slicename) 0)
+ slicename) 0) {
+virErrorPtr err = virGetLastError();
+if (err-code == VIR_ERR_DBUS_SERVICE
+STREQ(err-str2, org.freedesktop.DBus.Error.ServiceUnknown)) {
+virResetLastError();
+ret = -2;
+}
goto cleanup;
+}
ret = 0;
diff --git a/tests/virsystemdmock.c b/tests/virsystemdmock.c
index 5f9cce6..1f4413c 100644
--- a/tests/virsystemdmock.c
+++ b/tests/virsystemdmock.c
@@ -60,16 +60,20 @@ dbus_bool_t
dbus_connection_set_watch_functions(DBusConnection *connection ATTRI
DBusMessage *dbus_connection_send_with_reply_and_block(DBusConnection
*connection ATTRIBUTE_UNUSED,
DBusMessage *message,
int
timeout_milliseconds ATTRIBUTE_UNUSED,
- DBusError *error
ATTRIBUTE_UNUSED)
+ DBusError *error)
{
-DBusMessage *reply;
+DBusMessage *reply = NULL;
dbus_message_set_serial(message, 7);
-if (getenv(FAIL_NO_SERVICE))
+if (getenv(FAIL_BAD_SERVICE))
reply = dbus_message_new_error(message,
-
org.freedesktop.DBus.Error.ServiceUnknown,
- The name org.freedesktop.machine1 was
not provided by any .service files);
+ org.freedesktop.systemd.badthing,
+ Something went wrong creating the
machine);
+else if (getenv(FAIL_NO_SERVICE))
+dbus_set_error(error,
+ org.freedesktop.DBus.Error.ServiceUnknown,
+ %s, The name org.freedesktop.machine1 was not
provided by any .service files);
else
reply = dbus_message_new_method_return(message);
diff --git a/tests/virsystemdtest.c b/tests/virsystemdtest.c
index 3992722..bcf3ad3 100644
--- a/tests/virsystemdtest.c
+++ b/tests/virsystemdtest.c
@@ -82,35 +82,60 @@ static int testCreateNoSystemd(const void *opaque
ATTRIBUTE_UNUSED)
3, 3, 3, 3,
4, 4, 4, 4
};
+int rv;
setenv(FAIL_NO_SERVICE, 1, 1);
-if (virSystemdCreateMachine(demo,
-qemu,
-true,
-uuid,
-NULL,
-123,
-false,
-NULL) == 0) {
+if ((rv = virSystemdCreateMachine(demo,
+ qemu,
+ true,
+ uuid,
+ NULL,
+ 123,
+ false,
+ NULL)) == 0) {
fprintf(stderr, %s, Unexpected create machine success\n);
return -1;
}
-virErrorPtr err = virGetLastError();
+if (rv != -2) {
+fprintf(stderr, %s, Unexpected create machine error\n);
+return -1;
+}
+
+return 0;
+}
-if (!err) {
-fprintf(stderr, No error raised);
+static int testCreateBadSystemd(const void
Re: [libvirt] [PATCH v3 4/7] qemu: Translate the iscsi pool/volume disk source
Il 23/07/2013 15:36, Daniel P. Berrange ha scritto: On Tue, Jul 23, 2013 at 03:29:40PM +0200, Paolo Bonzini wrote: Il 23/07/2013 15:26, Daniel P. Berrange ha scritto: Ok, this answers my question. :) I think the default mode should be direct, because otherwise things such as persistent reservations do not work. No, the default has to be host mode, because that is the only mode that is guaranteed to be usable with any QEMU. The direct mode requires a QEMU that is new enough, and a distro to have enabled it. We can't rely on that as a default choice. Volume sources are also new enough that you can assume a good QEMU. No we can't assume that. New libvirt is frequently used with old QEMU and we want to have good default behaviour there. New libvirt, yes. But can't new libvirt features also assume a new QEMU if the old one causes more trouble than anything? Host mode for iSCSI is broken. It also doesn't reconnect well if you have a network problem, because after a LUN rescan the inode may change. That's a much smaller level of brokeness, than if QEMU doesn't support the iscsi block protocol thus the default configuration won't even boot. Failure to reconnect is not part of the brokenness. :) If you use host mode and the guest issues reservation commands, you may get data corruption. That is the brokenness. Another difference is that direct mode doesn't require the pool to be started before starting the domain. For authenticated iSCSI LUNs, this is better because you cannot autostart authenticated iSCSI pools. Perhaps the default could be specified in a configuration file (and the default should be the safe one). Paolo -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3 4/7] qemu: Translate the iscsi pool/volume disk source
On Tue, Jul 23, 2013 at 03:52:56PM +0200, Paolo Bonzini wrote: Il 23/07/2013 15:36, Daniel P. Berrange ha scritto: On Tue, Jul 23, 2013 at 03:29:40PM +0200, Paolo Bonzini wrote: Il 23/07/2013 15:26, Daniel P. Berrange ha scritto: Ok, this answers my question. :) I think the default mode should be direct, because otherwise things such as persistent reservations do not work. No, the default has to be host mode, because that is the only mode that is guaranteed to be usable with any QEMU. The direct mode requires a QEMU that is new enough, and a distro to have enabled it. We can't rely on that as a default choice. Volume sources are also new enough that you can assume a good QEMU. No we can't assume that. New libvirt is frequently used with old QEMU and we want to have good default behaviour there. New libvirt, yes. But can't new libvirt features also assume a new QEMU if the old one causes more trouble than anything? I've no problem with new features requiring new QEMU but that's not the issue here. This is a question about what the default configuration should be. For default configuration we aim for maximum compatibility not the latest possible features. Host mode for iSCSI is broken. It also doesn't reconnect well if you have a network problem, because after a LUN rescan the inode may change. That's a much smaller level of brokeness, than if QEMU doesn't support the iscsi block protocol thus the default configuration won't even boot. Failure to reconnect is not part of the brokenness. :) If you use host mode and the guest issues reservation commands, you may get data corruption. That is the brokenness. So be it, that's been the case for every version of libvirt and QEMU in existance up until iscsi protocol support was added, so it is not a new issue. That doesn't justify choosing a default that is guaranteed to not work at all. Another difference is that direct mode doesn't require the pool to be started before starting the domain. For authenticated iSCSI LUNs, this is better because you cannot autostart authenticated iSCSI pools. Autostarting of authenticated pools is simply a bug to be fixed. Perhaps the default could be specified in a configuration file (and the default should be the safe one). No, that is even worse because now the default is not predictable.. We simply default to host mode and if applications want to use the other mode they can configure the XML as desired. 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 v5 1/5] qemu: Add source pool auth info to virDomainDiskDef for iSCSI
On 07/22/2013 10:31 PM, John Ferlan wrote: During qemuTranslateDiskSourcePool() execution, if the srcpool has been defined with authentication information, then for iSCSI pools copy the authentication and host information to virDomainDiskDef. --- src/qemu/qemu_conf.c | 55 1 file changed, 55 insertions(+) ACK Jan -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v5 3/5] qemu_common: Create qemuBuildVolumeString() to process storage pool
On 07/22/2013 10:31 PM, John Ferlan wrote: Split out into its own separate routine --- src/qemu/qemu_command.c | 108 1 file changed, 64 insertions(+), 44 deletions(-) ACK, just code movement. Jan -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v5 2/5] qemu: Create a common qemuGetSecretString
On 07/22/2013 10:31 PM, John Ferlan wrote:
Make the secret fetching code common for qemuBuildRBDString() and
qemuBuildDriveURIString() using the virDomainDiskDef.
---
src/qemu/qemu_command.c | 157
+---
1 file changed, 81 insertions(+), 76 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 4a49d81..5bd8e87 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -2478,47 +2531,23 @@ qemuBuildRBDString(virConnectPtr conn,
virBufferEscape(opt, ',', ,, rbd:%s, disk-src);
if (disk-auth.username) {
+
virBufferEscape(opt, '\\', :, :id=%s, disk-auth.username);
-/* look up secret */
-switch (disk-auth.secretType) {
-case VIR_DOMAIN_DISK_SECRET_TYPE_UUID:
-sec = virSecretLookupByUUID(conn,
-disk-auth.secret.uuid);
-break;
-case VIR_DOMAIN_DISK_SECRET_TYPE_USAGE:
-sec = virSecretLookupByUsage(conn,
- VIR_SECRET_USAGE_TYPE_CEPH,
- disk-auth.secret.usage);
-break;
-}
+/* Get the secret string using the virDomainDiskDef
trailing whitespace ^
ACK
Jan
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v5 4/5] storage: Support chap authentication for iscsi pool
On 07/22/2013 10:31 PM, John Ferlan wrote: --- src/storage/storage_backend_iscsi.c | 111 +++- 1 file changed, 110 insertions(+), 1 deletion(-) I can confirm this works, but it's a shame it doesn't work on autostart. ACK if you clarify the error. Jan -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v5 5/5] Adjust 'ceph' authentication secret usage for rbd pool.
On 07/22/2013 10:31 PM, John Ferlan wrote:
Update virStorageBackendRBDOpenRADOSConn() to use the internal API to the
secret driver in order to get the secret value instead of the external
virSecretGetValue() path. Without the flag VIR_SECRET_GET_VALUE_INTERNAL_CALL
there is no way to get the value of private secret.
This also requires ensuring there is a connection which wasn't true for
for the refreshPool() path calls from storageDriverAutostart() prior to
adding support for the connection to a qemu driver. It seems calls to
virSecretLookupByUUIDString() and virSecretLookupByUsage() from the
refreshPool() path would have failed with no way to find the secret - that is
theoretically speaking since the 'conn' was NULL the failure would have been
failed to find the secret.
---
src/storage/storage_backend_rbd.c | 21 +++--
1 file changed, 19 insertions(+), 2 deletions(-)
diff --git a/src/storage/storage_backend_rbd.c
b/src/storage/storage_backend_rbd.c
index badbdac..70121bf 100644
--- a/src/storage/storage_backend_rbd.c
+++ b/src/storage/storage_backend_rbd.c
@@ -23,6 +23,7 @@
#include config.h
+#include datatypes.h
#include virerror.h
#include storage_backend_rbd.h
#include storage_conf.h
@@ -71,6 +72,12 @@ static int
virStorageBackendRBDOpenRADOSConn(virStorageBackendRBDStatePtr *ptr,
goto cleanup;
}
+if (!conn) {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+ _('ceph' authentication requires connection));
ACK if you change the error to mention autostart, as Paolo suggested in his
reply to 4/5.
Jan
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] qemu: Take error path if acquiring of job fails in qemuDomainSaveInternal
Due to a goto statement missed when refactoring in 2771f8b74c1bf50d1fa when acquiring of a domain job failed the error path was not taken. This resulted into a crash afterwards as a extra reference was removed from a domain object leading to it being freed. An attempt to list the domains afterwards leaded to a crash of the daemon afterwards. https://bugzilla.redhat.com/show_bug.cgi?id=928672 --- Sorry for breaking that in the first place :/ src/qemu/qemu_driver.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 0af76a5..96f87cd 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -2987,8 +2987,8 @@ qemuDomainSaveInternal(virQEMUDriverPtr driver, virDomainPtr dom, if (!qemuMigrationIsAllowed(driver, vm, vm-def, false, false)) goto cleanup; -if (qemuDomainObjBeginAsyncJob(driver, vm, - QEMU_ASYNC_JOB_SAVE) 0) +if (qemuDomainObjBeginAsyncJob(driver, vm, QEMU_ASYNC_JOB_SAVE) 0) +goto cleanup; memset(priv-job.info, 0, sizeof(priv-job.info)); priv-job.info.type = VIR_DOMAIN_JOB_UNBOUNDED; -- 1.8.3.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: Take error path if acquiring of job fails in qemuDomainSaveInternal
On Tue, Jul 23, 2013 at 16:21:10 +0200, Peter Krempa wrote: Due to a goto statement missed when refactoring in 2771f8b74c1bf50d1fa when acquiring of a domain job failed the error path was not taken. This resulted into a crash afterwards as a extra reference was removed from a s/as a/as an/ domain object leading to it being freed. An attempt to list the domains afterwards leaded to a crash of the daemon afterwards. ETOOMANYATERWARDS :-P https://bugzilla.redhat.com/show_bug.cgi?id=928672 --- Sorry for breaking that in the first place :/ src/qemu/qemu_driver.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 0af76a5..96f87cd 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -2987,8 +2987,8 @@ qemuDomainSaveInternal(virQEMUDriverPtr driver, virDomainPtr dom, if (!qemuMigrationIsAllowed(driver, vm, vm-def, false, false)) goto cleanup; -if (qemuDomainObjBeginAsyncJob(driver, vm, - QEMU_ASYNC_JOB_SAVE) 0) +if (qemuDomainObjBeginAsyncJob(driver, vm, QEMU_ASYNC_JOB_SAVE) 0) +goto cleanup; memset(priv-job.info, 0, sizeof(priv-job.info)); priv-job.info.type = VIR_DOMAIN_JOB_UNBOUNDED; ACK with the commit message polished. Jirka -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: Take error path if acquiring of job fails in qemuDomainSaveInternal
On 07/23/13 16:26, Jiri Denemark wrote: On Tue, Jul 23, 2013 at 16:21:10 +0200, Peter Krempa wrote: Due to a goto statement missed when refactoring in 2771f8b74c1bf50d1fa when acquiring of a domain job failed the error path was not taken. This resulted into a crash afterwards as a extra reference was removed from a s/as a/as an/ domain object leading to it being freed. An attempt to list the domains afterwards leaded to a crash of the daemon afterwards. ETOOMANYATERWARDS :-P https://bugzilla.redhat.com/show_bug.cgi?id=928672 ... ACK with the commit message polished. I upgraded the message and pushed the patch afterwards ;) Jirka Thanks. Peter -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 3/5] qemu: make QEMU_PCI_ADDRESS_(SLOT|FUNCTION)_LAST less misleading
Although these two enums are named ..._LAST, they really had the value
of ..._SIZE. This patch changes their values so that, e.g.,
QEMU_PCI_ADDRESS_SLOT_LAST really is the slot number of the last slot
on a PCI bus.
---
src/qemu/qemu_command.c | 18 +-
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index f85e896..059aa6a 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -1408,12 +1408,12 @@ cleanup:
return ret;
}
-#define QEMU_PCI_ADDRESS_SLOT_LAST 32
-#define QEMU_PCI_ADDRESS_FUNCTION_LAST 8
+#define QEMU_PCI_ADDRESS_SLOT_LAST 31
+#define QEMU_PCI_ADDRESS_FUNCTION_LAST 7
typedef struct {
/* Each bit in a slot represents one function on that slot */
-uint8_t slots[QEMU_PCI_ADDRESS_SLOT_LAST];
+uint8_t slots[QEMU_PCI_ADDRESS_SLOT_LAST + 1];
} qemuDomainPCIAddressBus;
typedef qemuDomainPCIAddressBus *qemuDomainPCIAddressBusPtr;
@@ -1448,15 +1448,15 @@ static bool
qemuPCIAddressValidate(qemuDomainPCIAddressSetPtr addrs ATTRIBUTE_UN
addrs-nbuses - 1);
return false;
}
-if (addr-function = QEMU_PCI_ADDRESS_FUNCTION_LAST) {
+if (addr-function QEMU_PCI_ADDRESS_FUNCTION_LAST) {
virReportError(VIR_ERR_XML_ERROR,
- _(Invalid PCI address: function must be %u),
+ _(Invalid PCI address: function must be = %u),
QEMU_PCI_ADDRESS_FUNCTION_LAST);
return false;
}
-if (addr-slot = QEMU_PCI_ADDRESS_SLOT_LAST) {
+if (addr-slot QEMU_PCI_ADDRESS_SLOT_LAST) {
virReportError(VIR_ERR_XML_ERROR,
- _(Invalid PCI address: slot must be %u),
+ _(Invalid PCI address: slot must be = %u),
QEMU_PCI_ADDRESS_SLOT_LAST);
return false;
}
@@ -1859,7 +1859,7 @@
qemuDomainPCIAddressGetNextSlot(qemuDomainPCIAddressSetPtr addrs,
/* Start the search at the last used bus and slot */
for (a.slot++; a.bus addrs-nbuses; a.bus++) {
-for (; a.slot QEMU_PCI_ADDRESS_SLOT_LAST; a.slot++) {
+for (; a.slot = QEMU_PCI_ADDRESS_SLOT_LAST; a.slot++) {
if (!qemuDomainPCIAddressSlotInUse(addrs, a))
goto success;
@@ -1878,7 +1878,7 @@
qemuDomainPCIAddressGetNextSlot(qemuDomainPCIAddressSetPtr addrs,
} else {
/* Check the buses from 0 up to the last used one */
for (a.bus = 0; a.bus = addrs-lastaddr.bus; a.bus++) {
-for (a.slot = 1; a.slot QEMU_PCI_ADDRESS_SLOT_LAST; a.slot++) {
+for (a.slot = 1; a.slot = QEMU_PCI_ADDRESS_SLOT_LAST; a.slot++) {
if (!qemuDomainPCIAddressSlotInUse(addrs, a))
goto success;
--
1.7.11.7
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/5] qemu: turn qemuDomainPCIAddressBus into a struct
qemuDomainPCIAddressBus was an array of QEMU_PCI_ADDRESS_SLOT_LAST
uint8_t's, which worked fine as long as every PCI bus was
identical. In the future, some PCI busses will allow connecting PCI
devices, and some will allow PCIe devices; also some will only allow
connection of a single device, while others will allow connecting 31
devices.
In order to keep track of that information for each bus, we need to
turn qemuDomainPCIAddressBus into a struct, for now with just one
member:
uint8_t slots[QEMU_PCI_ADDRESS_SLOT_LAST];
Additional members will come in later patches.
The item in qemuDomainPCIAddresSet that contains the array of
qemuDomainPCIAddressBus is now called buses to be more consistent
with the already existing nbuses (and with the new slots array).
---
src/qemu/qemu_command.c | 47 ---
1 file changed, 24 insertions(+), 23 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 4a49d81..7f5dab9 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -1411,15 +1411,16 @@ cleanup:
#define QEMU_PCI_ADDRESS_SLOT_LAST 32
#define QEMU_PCI_ADDRESS_FUNCTION_LAST 8
-/*
- * Each bit represents a function
- * Each byte represents a slot
- */
-typedef uint8_t qemuDomainPCIAddressBus[QEMU_PCI_ADDRESS_SLOT_LAST];
+typedef struct {
+/* Each bit in a slot represents one function on that slot */
+uint8_t slots[QEMU_PCI_ADDRESS_SLOT_LAST];
+} qemuDomainPCIAddressBus;
+typedef qemuDomainPCIAddressBus *qemuDomainPCIAddressBusPtr;
+
struct _qemuDomainPCIAddressSet {
-qemuDomainPCIAddressBus *used;
+qemuDomainPCIAddressBus *buses;
+size_t nbuses;
virDevicePCIAddress lastaddr;
-size_t nbuses;/* allocation of 'used' */
bool dryRun; /* on a dry run, new buses are auto-added
and addresses aren't saved in device infos */
};
@@ -1489,11 +1490,11 @@ qemuDomainPCIAddressSetGrow(qemuDomainPCIAddressSetPtr
addrs,
i = addrs-nbuses;
if (add = 0)
return 0;
-if (VIR_EXPAND_N(addrs-used, addrs-nbuses, add) 0)
+if (VIR_EXPAND_N(addrs-buses, addrs-nbuses, add) 0)
return -1;
/* reserve slot 0 on the new buses */
for (; i addrs-nbuses; i++)
-addrs-used[i][0] = 0xFF;
+addrs-buses[i].slots[0] = 0xFF;
return add;
}
@@ -1559,7 +1560,7 @@ static int qemuCollectPCIAddress(virDomainDefPtr def
ATTRIBUTE_UNUSED,
if (!(str = qemuPCIAddressAsString(addr)))
goto cleanup;
-if (addrs-used[addr-bus][addr-slot] (1 addr-function)) {
+if (addrs-buses[addr-bus].slots[addr-slot] (1 addr-function)) {
if (info-addr.pci.function != 0) {
virReportError(VIR_ERR_XML_ERROR,
_(Attempted double use of PCI Address '%s'
@@ -1575,7 +1576,7 @@ static int qemuCollectPCIAddress(virDomainDefPtr def
ATTRIBUTE_UNUSED,
if ((info-addr.pci.function == 0)
(info-addr.pci.multi != VIR_DEVICE_ADDRESS_PCI_MULTI_ON)) {
/* a function 0 w/o multifunction=on must reserve the entire slot */
-if (addrs-used[addr-bus][addr-slot]) {
+if (addrs-buses[addr-bus].slots[addr-slot]) {
virReportError(VIR_ERR_XML_ERROR,
_(Attempted double use of PCI Address on slot '%s'
(need \multifunction='off'\ for device
@@ -1583,11 +1584,11 @@ static int qemuCollectPCIAddress(virDomainDefPtr def
ATTRIBUTE_UNUSED,
str);
goto cleanup;
}
-addrs-used[addr-bus][addr-slot] = 0xFF;
+addrs-buses[addr-bus].slots[addr-slot] = 0xFF;
VIR_DEBUG(Remembering PCI slot: %s (multifunction=off), str);
} else {
VIR_DEBUG(Remembering PCI addr: %s, str);
-addrs-used[addr-bus][addr-slot] |= 1 addr-function;
+addrs-buses[addr-bus].slots[addr-slot] |= 1 addr-function;
}
ret = 0;
cleanup:
@@ -1707,7 +1708,7 @@ qemuDomainPCIAddressSetPtr
qemuDomainPCIAddressSetCreate(virDomainDefPtr def,
if (VIR_ALLOC(addrs) 0)
goto error;
-if (VIR_ALLOC_N(addrs-used, nbuses) 0)
+if (VIR_ALLOC_N(addrs-buses, nbuses) 0)
goto error;
addrs-nbuses = nbuses;
@@ -1716,7 +1717,7 @@ qemuDomainPCIAddressSetPtr
qemuDomainPCIAddressSetCreate(virDomainDefPtr def,
/* reserve slot 0 in every bus - it's used by the host bridge on bus 0
* and unusable on PCI bridges */
for (i = 0; i nbuses; i++)
-addrs-used[i][0] = 0xFF;
+addrs-buses[i].slots[0] = 0xFF;
if (virDomainDeviceInfoIterate(def, qemuCollectPCIAddress, addrs) 0)
goto error;
@@ -1734,7 +1735,7 @@ error:
static bool qemuDomainPCIAddressSlotInUse(qemuDomainPCIAddressSetPtr addrs,
virDevicePCIAddressPtr addr)
{
-return !!addrs-used[addr-bus][addr-slot];
+return !!addrs-buses[addr-bus].slots[addr-slot];
}
int
[libvirt] [PATCH 0/5] Support for pcie-root / Q35 machine type
This is *almost* usable. I still need to add support for a dmi-to-pci-bridge before the q35 machine type can be used. 1/5 and 3/5 are just cleaning up some things that bothered me while writing patch 4/5. 2/5 should help out other machine types (e.g. arm) just a bit, by eliminating the extra PIIX3 devices unless the machine actually has a PIIX3 chip. 4/5 has the bulk of the changes - it restructures the PCI address validation/allocation so that it doesn't assume that all PCI addresses are the same, or that all PCI slots are the same. It sets up an enum where different types of PCI addresses can be listed, and the validation code checks that the slot about to be used for a device is actually compatible with that device. 5/5 adds the pcie-root controller which is implicit in the q35 machinetype. Of course since we can only connect pcie devices to this controller, and there are no pcie devices supported, it is not really worth much. However, the next patch (in progress) will add a dmi-to-pci-bridge controller, which *can* plug into the pcie-root and provide plain PCI slots, making a working a35 possible. Note that the auto-allocation doesn't attempt to auto-allocate a slot for any type of device other than plain PCI. This means that even when the dmi-to-pci-bridge controller is added, its address on pcie-root (and a pci-bridge device that must be connected to the dmi-to-pci-bridge controller) will need to be manually specified. Auto-placement will be an exercise for later. Things missing: 1) I need a test case for auto-adding multiple bridges. I will add that this afternoon. 2) I need a test case for a q35 domain. That wil also be added this afternoon. Laine Stump (5): qemu: turn qemuDomainPCIAddressBus into a struct qemu: only check for PIIX3-specific device addrs on pc-* machinetypes qemu: make QEMU_PCI_ADDRESS_(SLOT|FUNCTION)_LAST less misleading qemu: set/validate slot/connection type when assigning slots for PCI devices conf: add pcie-root controller docs/formatdomain.html.in | 17 +- src/conf/domain_conf.c| 4 +- src/conf/domain_conf.h| 5 +- src/libvirt_private.syms | 1 + src/qemu/qemu_command.c | 559 ++ src/qemu/qemu_command.h | 28 ++- src/qemu/qemu_domain.c| 23 +- 7 files changed, 434 insertions(+), 203 deletions(-) -- 1.7.11.7 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 5/5] conf: add pcie-root controller
This controller is implicit on q35 machinetypes. It provides 31 PCIe
(*not* PCI) slots as controller 0.
Currently there are no devices that can connect to pcie-root. For a
usable q35 system, we still need to add a dmi-to-pci-bridge pci
controller, which can connect to pcie-root, and provides pci slots.
This patch still requires a test case, which willbe coming up, but I
wanted to include it along with the previous patch to show that it's
simpler to add new controller types now.
---
docs/formatdomain.html.in | 17 ++---
src/conf/domain_conf.c| 4 +++-
src/conf/domain_conf.h| 1 +
src/qemu/qemu_command.c | 17 +
src/qemu/qemu_command.h | 2 ++
src/qemu/qemu_domain.c| 23 +--
6 files changed, 50 insertions(+), 14 deletions(-)
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 7601aaa..41e3e2a 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -2338,10 +2338,14 @@
p
PCI controllers have an optional codemodel/code attribute with
- possible values codepci-root/code or codepci-bridge/code.
- For machine types which provide an implicit pci bus, the pci-root
+ possible values codepci-root/code, codepcie-root/code
+ or codepci-bridge/code.
+ For machine types which provide an implicit PCI bus, the pci-root
controller with index=0 is auto-added and required to use PCI devices.
- PCI root has no address.
+ pci-root has no address.
+ For machine types which provide an implicit PCI Express (PCIe)
+ bus, the pcie-root controller with index=0 is auto-added and
+ required to use PCIe devices. pcie-root has also no address.
PCI bridges are auto-added if there are too many devices to fit on
the one bus provided by pci-root, or a PCI bus number greater than zero
was specified.
@@ -2361,6 +2365,13 @@
lt;/devicesgt;
.../pre
+pre
+ ...
+ lt;devicesgt;
+lt;controller type='pci' index='0' model='pcie-root'/gt;
+ lt;/devicesgt;
+ .../pre
+
h4a name=elementsLeaseDevice leases/a/h4
p
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 10cb7f6..605f706 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -310,6 +310,7 @@ VIR_ENUM_IMPL(virDomainController,
VIR_DOMAIN_CONTROLLER_TYPE_LAST,
VIR_ENUM_IMPL(virDomainControllerModelPCI,
VIR_DOMAIN_CONTROLLER_MODEL_PCI_LAST,
pci-root,
+ pcie-root,
pci-bridge)
VIR_ENUM_IMPL(virDomainControllerModelSCSI,
VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LAST,
@@ -5715,9 +5716,10 @@ virDomainControllerDefParseXML(xmlNodePtr node,
case VIR_DOMAIN_CONTROLLER_TYPE_PCI:
switch (def-model) {
case VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT:
+case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT:
if (def-info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) {
virReportError(VIR_ERR_XML_ERROR, %s,
- _(pci-root controller should not
+ _(pci-root and pcie-root controllers should
not
have an address));
goto error;
}
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index abf024c..68f36fd 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -768,6 +768,7 @@ enum virDomainControllerType {
typedef enum {
VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT,
+VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT,
VIR_DOMAIN_CONTROLLER_MODEL_PCI_BRIDGE,
VIR_DOMAIN_CONTROLLER_MODEL_PCI_LAST
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 64787b6..7fccb98 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -1519,6 +1519,12 @@
qemuDomainPCIAddressBusSetModel(qemuDomainPCIAddressBusPtr bus,
bus-minSlot = 1;
bus-maxSlot = QEMU_PCI_ADDRESS_SLOT_LAST;
break;
+case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT:
+bus-flags = (QEMU_PCI_CONNECT_HOTPLUGGABLE |
+ QEMU_PCI_CONNECT_TYPE_PCIE);
+bus-minSlot = 1;
+bus-maxSlot = QEMU_PCI_ADDRESS_SLOT_LAST;
+break;
default:
virReportError(VIR_ERR_INTERNAL_ERROR,
_(Invalid PCI controller model %d), model);
@@ -2277,7 +2283,8 @@ qemuAssignDevicePCISlots(virDomainDefPtr def,
/* PCI controllers */
for (i = 0; i def-ncontrollers; i++) {
if (def-controllers[i]-type == VIR_DOMAIN_CONTROLLER_TYPE_PCI) {
-if (def-controllers[i]-model ==
VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT)
+if (def-controllers[i]-model ==
VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT ||
+def-controllers[i]-model ==
VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT)
continue;
if (def-controllers[i]-info.type !=
VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE)
continue;
@@ -4211,8 +4218,9 @@
Re: [libvirt] [PATCH v5 4/5] storage: Support chap authentication for iscsi pool
On 07/23/2013 10:18 AM, Ján Tomko wrote:
On 07/22/2013 10:31 PM, John Ferlan wrote:
---
src/storage/storage_backend_iscsi.c | 111
+++-
1 file changed, 110 insertions(+), 1 deletion(-)
I can confirm this works, but it's a shame it doesn't work on autostart.
ACK if you clarify the error.
Jan
The autostart changes require getting a connection to the secret
driver which I felt may take more time than I had to figure out
how to get to work properly...
In any case, I adjusted the message as follows (same in 5/5):
diff --git a/src/storage/storage_backend_iscsi.c b/src/storage/storage_backend_i
index 388d6ed..ee8dd2e 100644
--- a/src/storage/storage_backend_iscsi.c
+++ b/src/storage/storage_backend_iscsi.c
@@ -714,7 +714,8 @@ virStorageBackendISCSISetAuth(const char *portal,
if (!conn) {
virReportError(VIR_ERR_INTERNAL_ERROR, %s,
- _(iscsi 'chap' authentication requires connection));
+ _(iscsi 'chap' authentication not supported
+ for autostarted pools));
return -1;
}
John
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 4/5] qemu: set/validate slot/connection type when assigning slots for PCI devices
Since PCI bridges, PCIe bridges, PCIe switches, and PCIe root ports
all share the same namespace, they are all defined as controllers of
type='pci' in libvirt (but with a differing model attribute). Each of
these controllers has a certain connection type upstream, allows
certain connection types downstream, and each can either allow a
single downstream connection at slot 0, or connections from slot 1 -
31.
Right now, we only support the pci-root and pci-bridge devices, both
of which only allow PCI devices to connect, and both which have usable
slots 1 - 31. In preparation for adding other types of controllers
that have different capabilities, this patch 1) adds info to the
qemuDomainPCIAddressBus object to indicate the capabilities, 2) sets
those capabilities appropriately for pci-root and pci-bridge devices,
and 3) validates that the controller being connected to is the proper
type when allocating slots or validating that a user-selected slot is
appropriate for a device..
Having this infrastructure in place will make it much easier to add
support for the other PCI controller types.
While it would be possible to do all the necessary checking by just
storing the controller model in the qemyuDomainPCIAddressBus, it
greatly simplifies all the validation code to also keep a flags,
minSlot and maxSlot for each - that way we can just check those
attributes rather than requiring a nearly identical switch statement
everywhere we need to validate compatibility.
You may notice many places where the flags are seemingly hard-coded to
QEMU_PCI_CONNECT_HOTPLUGGABLE | QEMU_PCI_CONNECT_TYPE_PCI
This is currently the correct value for all PCI devices, and in the
future will be the default, with small bits of code added to change to
the flags for the few devices which are the exceptions to this rule.
Finally, there are a few places with FIXME comments. Note that these
aren't indicating places that are broken according to the currently
supported devices, they are places that will need fixing when support
for new PCI controller models is added.
---
src/conf/domain_conf.h | 4 +-
src/libvirt_private.syms | 1 +
src/qemu/qemu_command.c | 319 +++
src/qemu/qemu_command.h | 26 +++-
4 files changed, 268 insertions(+), 82 deletions(-)
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index f265966..abf024c 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -766,12 +766,12 @@ enum virDomainControllerType {
};
-enum virDomainControllerModelPCI {
+typedef enum {
VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT,
VIR_DOMAIN_CONTROLLER_MODEL_PCI_BRIDGE,
VIR_DOMAIN_CONTROLLER_MODEL_PCI_LAST
-};
+} virDomainControllerModelPCI;
enum virDomainControllerModelSCSI {
VIR_DOMAIN_CONTROLLER_MODEL_SCSI_AUTO,
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 76873ad..f0c9181 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -139,6 +139,7 @@ virDomainControllerDefFree;
virDomainControllerFind;
virDomainControllerInsert;
virDomainControllerInsertPreAlloced;
+virDomainControllerModelPCITypeToString;
virDomainControllerModelSCSITypeFromString;
virDomainControllerModelSCSITypeToString;
virDomainControllerModelUSBTypeFromString;
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 059aa6a..64787b6 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -1412,7 +1412,15 @@ cleanup:
#define QEMU_PCI_ADDRESS_FUNCTION_LAST 7
typedef struct {
-/* Each bit in a slot represents one function on that slot */
+virDomainControllerModelPCI model;
+/* flags an min/max can be computed from model, but
+ * having them ready makes life easier.
+ */
+qemuDomainPCIConnectFlags flags;
+size_t minSlot, maxSlot; /* usually 0,0 or 1,31 */
+/* Each bit in a slot represents one function on that slot. If the
+ * bit is set, that function is in use by a device.
+ */
uint8_t slots[QEMU_PCI_ADDRESS_SLOT_LAST + 1];
} qemuDomainPCIAddressBus;
typedef qemuDomainPCIAddressBus *qemuDomainPCIAddressBusPtr;
@@ -1430,9 +1438,13 @@ struct _qemuDomainPCIAddressSet {
* Check that the PCI address is valid for use
* with the specified PCI address set.
*/
-static bool qemuPCIAddressValidate(qemuDomainPCIAddressSetPtr addrs
ATTRIBUTE_UNUSED,
- virDevicePCIAddressPtr addr)
+static bool
+qemuPCIAddressValidate(qemuDomainPCIAddressSetPtr addrs ATTRIBUTE_UNUSED,
+ virDevicePCIAddressPtr addr,
+ qemuDomainPCIConnectFlags flags)
{
+qemuDomainPCIAddressBusPtr bus;
+
if (addrs-nbuses == 0) {
virReportError(VIR_ERR_XML_ERROR, %s, _(No PCI buses available));
return false;
@@ -1448,32 +1460,81 @@ static bool
qemuPCIAddressValidate(qemuDomainPCIAddressSetPtr addrs ATTRIBUTE_UN
addrs-nbuses - 1);
return false;
}
-if
[libvirt] KVM Forum 2013 Call for Participation - Extended to August 4th
We have received numerous requests to extend the CFP deadline and so we are happy to announce that the CFP deadline has been moved by two weeks to August 4th. = KVM Forum 2013: Call For Participation October 21-23, 2013 - Edinburgh International Conference Centre - Edinburgh, UK (All submissions must be received before midnight July 21, 2013) = KVM is an industry leading open source hypervisor that provides an ideal platform for datacenter virtualization, virtual desktop infrastructure, and cloud computing. Once again, it's time to bring together the community of developers and users that define the KVM ecosystem for our annual technical conference. We will discuss the current state of affairs and plan for the future of KVM, its surrounding infrastructure, and management tools. The oVirt Workshop will run in parallel with the KVM Forum again, bringing in a community focused on enterprise datacenter virtualization management built on KVM. For topics which overlap we will have shared sessions. So mark your calendar and join us in advancing KVM. http://events.linuxfoundation.org/events/kvm-forum/ Once again we are colocated with The Linux Foundation's LinuxCon Europe. KVM Forum attendees will be able to attend oVirt Workshop sessions and are eligible to attend LinuxCon Europe for a discounted rate. http://events.linuxfoundation.org/events/kvm-forum/register We invite you to lead part of the discussion by submitting a speaking proposal for KVM Forum 2013. http://events.linuxfoundation.org/cfp Suggested topics: KVM/Kernel - Scaling and performance - Nested virtualization - I/O improvements - VFIO, device assignment, SR-IOV - Driver domains - Time keeping - Resource management (cpu, memory, i/o) - Memory management (page sharing, swapping, huge pages, etc) - Network virtualization - Security - Architecture ports QEMU - Device model improvements - New devices and chipsets - Scaling and performance - Desktop virtualization - Spice - Increasing robustness and hardening - Security model - Management interfaces - QMP protocol and implementation - Image formats - Firmware (SeaBIOS, OVMF, UEFI, etc) - Live migration - Live snapshots and merging - Fault tolerance, high availability, continuous backup - Real-time guest support Virtio - Speeding up existing devices - Alternatives - Virtio on non-Linux or non-virtualized Management infrastructure - oVirt (shared track w/ oVirt Workshop) - Libvirt - KVM autotest - OpenStack - Network virtualization management - Enterprise storage management Cloud computing - Scalable storage - Virtual networking - Security - Provisioning SUBMISSION REQUIREMENTS Abstracts due: July 21, 2013 Notification: August 1, 2013 Please submit a short abstract (~150 words) describing your presentation proposal. In your submission please note how long your talk will take. Slots vary in length up to 45 minutes. Also include in your proposal the proposal type -- one of: - technical talk - end-user talk - birds of a feather (BOF) session Submit your proposal here: http://events.linuxfoundation.org/cfp You will receive a notification whether or not your presentation proposal was accepted by Aug 1st. END-USER COLLABORATION One of the big challenges as developers is to know what, where and how people actually use our software. We will reserve a few slots for end users talking about their deployment challenges and achievements. If you are using KVM in production you are encouraged submit a speaking proposal. Simply mark it as an end-user collaboration proposal. As an end user, this is a unique opportunity to get your input to developers. BOF SESSION We will reserve some slots in the evening after the main conference tracks, for birds of a feather (BOF) sessions. These sessions will be less formal than presentation tracks and targetted for people who would like to discuss specific issues with other developers and/or users. If you are interested in getting developers and/or uses together to discuss a specific problem, please submit a BOF proposal. HOTEL / TRAVEL The KVM Forum 2013 will be held in Edinburgh, UK at the Edinburgh International Conference Centre. http://events.linuxfoundation.org/events/kvm-forum/hotel Thank you for your interest in KVM. We're looking forward to your submissions and seeing you at the KVM Forum 2013 in October! Thanks, -your KVM Forum 2013 Program Committee Please contact us with any questions or comments. kvm-forum-2013...@redhat.com -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v5 4/5] storage: Support chap authentication for iscsi pool
On Tue, Jul 23, 2013 at 10:47:46AM -0400, John Ferlan wrote:
On 07/23/2013 10:18 AM, Ján Tomko wrote:
On 07/22/2013 10:31 PM, John Ferlan wrote:
---
src/storage/storage_backend_iscsi.c | 111
+++-
1 file changed, 110 insertions(+), 1 deletion(-)
I can confirm this works, but it's a shame it doesn't work on autostart.
ACK if you clarify the error.
Jan
The autostart changes require getting a connection to the secret
driver which I felt may take more time than I had to figure out
how to get to work properly...
In any case, I adjusted the message as follows (same in 5/5):
diff --git a/src/storage/storage_backend_iscsi.c
b/src/storage/storage_backend_i
index 388d6ed..ee8dd2e 100644
--- a/src/storage/storage_backend_iscsi.c
+++ b/src/storage/storage_backend_iscsi.c
@@ -714,7 +714,8 @@ virStorageBackendISCSISetAuth(const char *portal,
if (!conn) {
virReportError(VIR_ERR_INTERNAL_ERROR, %s,
- _(iscsi 'chap' authentication requires connection));
+ _(iscsi 'chap' authentication not supported
+ for autostarted pools));
return -1;
}
I noticed that the nwfilter already unconditionally calls
virConnectOpen(qemu://system); so we're already in fact suffering from the
problem with
autostart having a qemu dependency.
Given this, I'd support a patch which simply did
conn = virConnectOpen(privilege ? qemu:///system : qemu:///session);
in storageDriverAutostart, provided that we ignore any errors from
virConnectOpen, and fallback to use NULL for the connection in that
case.
Obviously this is something we'll still need to fix properly in a
future release, but at least it'll make autostart of storage pools
with auth work in the common case in the short term for this release.
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 v5 0/5] Support CHAP authentication for iscsi pool
On 07/22/2013 04:31 PM, John Ferlan wrote: This is a reworking and reposting of the authentication patches originally posted as part of my v3 reworking of Osier's original patches, see: ...snip... John Ferlan (5): qemu: Add source pool auth info to virDomainDiskDef for iSCSI qemu: Create a common qemuGetSecretString qemu_common: Create qemuBuildVolumeString() to process storage pool storage: Support chap authentication for iscsi pool Adjust 'ceph' authentication secret usage for rbd pool. src/qemu/qemu_command.c | 265 src/qemu/qemu_conf.c| 55 src/storage/storage_backend_iscsi.c | 111 ++- src/storage/storage_backend_rbd.c | 21 ++- 4 files changed, 329 insertions(+), 123 deletions(-) I updated 2/5 to remove the space, adjusted the messages in 4/5 5/5 to indicate not supported for autostarted pools and pushed the above changes. Thanks for the reviews. Now to see what it would take for a secretdriver connection... John -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/7] util: add virGetGroupList
https://bugzilla.redhat.com/show_bug.cgi?id=964358
Since neither getpwuid_r() nor initgroups() are safe to call in
between fork and exec (they obtain a mutex, but if some other
thread in the parent also held the mutex at the time of the fork,
the child will deadlock), we have to split out the functionality
that is unsafe. At least glibc's initgroups() uses getgrouplist
under the hood, so the ideal split is to expose getgrouplist for
use before a fork. Gnulib already gives us a nice wrapper via
mgetgroups; we wrap it once more to look up by uid instead of name.
* bootstrap.conf (gnulib_modules): Add mgetgroups.
* src/util/virutil.h (virGetGroupList): New declaration.
* src/util/virutil.c (virGetGroupList): New function.
* src/libvirt_private.syms (virutil.h): Export it.
Signed-off-by: Eric Blake ebl...@redhat.com
(cherry picked from commit 75c125641ac73473ba4b0542524d67a184769c8e)
Conflicts:
bootstrap.conf - not updating gnulib submodule...
configure.ac - ...so checking for getgrouplist by hand...
src/util/virutil.c - ...and copying only the getgrouplist
implementation rather than calling the gnulib function; also, file still named
util.c
src/libvirt_private.syms - context
---
configure.ac | 2 +-
src/libvirt_private.syms | 1 +
src/util/util.c | 60
src/util/util.h | 2 ++
4 files changed, 64 insertions(+), 1 deletion(-)
diff --git a/configure.ac b/configure.ac
index 5f1a273..aeff11d 100644
--- a/configure.ac
+++ b/configure.ac
@@ -171,7 +171,7 @@ AC_CHECK_SIZEOF([long])
dnl Availability of various common functions (non-fatal if missing),
dnl and various less common threadsafe functions
-AC_CHECK_FUNCS_ONCE([cfmakeraw geteuid getgid getgrnam_r getmntent_r \
+AC_CHECK_FUNCS_ONCE([cfmakeraw geteuid getgid getgrnam_r getgrouplist
getmntent_r \
getpwuid_r getuid initgroups kill mmap newlocale posix_fallocate \
posix_memalign regexec sched_getaffinity])
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 45cdace..24f7047 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1243,6 +1243,7 @@ virFileWaitForDevices;
virFileWriteStr;
virFindFileInPath;
virGetGroupID;
+virGetGroupList;
virGetGroupName;
virGetHostname;
virGetUserDirectory;
diff --git a/src/util/util.c b/src/util/util.c
index dd4e4ea..5ca5034 100644
--- a/src/util/util.c
+++ b/src/util/util.c
@@ -2575,6 +2575,66 @@ int virGetGroupID(const char *name,
}
+/* Compute the list of supplementary groups associated with @uid, and
+ * including @gid in the list (unless it is -1), storing a malloc'd
+ * result into @list. Return the size of the list on success, or -1
+ * on failure with error reported and errno set. May not be called
+ * between fork and exec. */
+int
+virGetGroupList(uid_t uid, gid_t gid, gid_t **list)
+{
+int ret = -1;
+char *user = NULL;
+
+*list = NULL;
+if (uid == (uid_t)-1)
+return 0;
+
+if (virGetUserEnt(uid, user,
+ gid == (gid_t)-1 ? gid : NULL, NULL) 0)
+return -1;
+
+# if HAVE_GETGROUPLIST
+/* Borrowing from gnulib's LGPLv2+ mgetgroups.c as of July 2013. */
+/* Avoid a bug in older glibc with size 0, by pre-allocating a
+ * list size and then enlarging if needed. */
+int max = 10;
+if (VIR_ALLOC_N(*list, max) 0)
+goto no_memory;
+while (1)
+{
+int ngroups;
+int last = max;
+
+ngroups = getgrouplist(user, gid, *list, max);
+
+/* Avoid a bug in Darwin where max is not increased. */
+if (ngroups 0 last == max)
+max *= 2;
+if (VIR_REALLOC_N(*list, max) 0) {
+VIR_FREE(*list);
+goto no_memory;
+}
+if (0 = ngroups) {
+ret = ngroups;
+break;
+}
+}
+# else
+if (VIR_ALLOC_N(*list, 1) 0)
+goto no_memory;
+(*list)[0] = gid;
+ret = 1;
+# endif
+
+cleanup:
+VIR_FREE(user);
+return ret;
+no_memory:
+virReportOOMError();
+goto cleanup;
+}
+
/* Set the real and effective uid and gid to the given values, and call
* initgroups so that the process has all the assumed group membership of
* that uid. return 0 on success, -1 on failure (the original system error
diff --git a/src/util/util.h b/src/util/util.h
index 5ab36ed..98964b2 100644
--- a/src/util/util.h
+++ b/src/util/util.h
@@ -261,6 +261,8 @@ char *virGetUserCacheDirectory(void);
char *virGetUserRuntimeDirectory(void);
char *virGetUserName(uid_t uid);
char *virGetGroupName(gid_t gid);
+int virGetGroupList(uid_t uid, gid_t group, gid_t **groups)
+ATTRIBUTE_NONNULL(3);
int virGetUserID(const char *name,
uid_t *uid) ATTRIBUTE_RETURN_CHECK;
int virGetGroupID(const char *name,
--
1.8.3.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 5/7] security: framework for driver PreFork handler
https://bugzilla.redhat.com/show_bug.cgi?id=964358
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
(cherry picked from commit fdb3bde31ccf8ff172abf00ef5aa974b87af2794)
Conflicts:
src/security/security_manager.c - context from previous backport
differences
---
src/qemu/qemu_process.c | 3 ++-
src/security/security_driver.h | 4
src/security/security_manager.c | 14 --
src/security/security_manager.h | 2 +-
src/security/security_stack.c | 23 +++
5 files changed, 42 insertions(+), 4 deletions(-)
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 01c6880..4fdad6a 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -3645,7 +3645,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 d49b401..92bed3f 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);
@@ -111,6 +113,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 fc7acff..b138cc1 100644
--- a/src/security/security_manager.c
+++ b/src/security/security_manager.c
@@ -164,11 +164,21 @@ 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 ATTRIBUTE_UNUSED)
+int virSecurityManagerPreFork(virSecurityManagerPtr mgr)
{
+int ret = 0;
+
/* XXX Grab our own mutex here instead of relying on caller's mutex */
+if (mgr-drv-preFork) {
+ret = mgr-drv-preFork(mgr);
+}
+
+return ret;
}
diff --git a/src/security/security_manager.h b/src/security/security_manager.h
index d1a5997..be8774d 100644
--- a/src/security/security_manager.h
+++ b/src/security/security_manager.h
@@ -46,7 +46,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
[libvirt] [PATCH 1/7] util: improve user lookup helper
https://bugzilla.redhat.com/show_bug.cgi?id=964358
A future patch needs to look up pw_gid; but it is wasteful
to crawl through getpwuid_r twice for two separate pieces
of information, and annoying to copy that much boilerplate
code for doing the crawl. The current internal-only
virGetUserEnt is also a rather awkward interface; it's easier
to just design it to let callers request multiple pieces of
data as needed from one traversal.
And while at it, I noticed that virGetXDGDirectory could deref
NULL if the getpwuid_r lookup fails.
* src/util/virutil.c (virGetUserEnt): Alter signature.
(virGetUserDirectory, virGetXDGDirectory, virGetUserName): Adjust
callers.
Signed-off-by: Eric Blake ebl...@redhat.com
(cherry picked from commit c1983ba4e3902308054e961fcae75cece73ef4ba)
Conflicts:
src/util/virutil.c - oom reporting/strdup changes not backported
---
src/util/util.c | 60 ++---
1 file changed, 36 insertions(+), 24 deletions(-)
diff --git a/src/util/util.c b/src/util/util.c
index 8315515..dd4e4ea 100644
--- a/src/util/util.c
+++ b/src/util/util.c
@@ -2290,21 +2290,23 @@ check_and_return:
}
#ifdef HAVE_GETPWUID_R
-enum {
-VIR_USER_ENT_DIRECTORY,
-VIR_USER_ENT_NAME,
-};
-
-static char *virGetUserEnt(uid_t uid,
- int field)
+/* Look up fields from the user database for the given user. On
+ * error, set errno, report the error, and return -1. */
+static int
+virGetUserEnt(uid_t uid, char **name, gid_t *group, char **dir)
{
char *strbuf;
-char *ret;
struct passwd pwbuf;
struct passwd *pw = NULL;
long val = sysconf(_SC_GETPW_R_SIZE_MAX);
size_t strbuflen = val;
int rc;
+int ret = -1;
+
+if (name)
+*name = NULL;
+if (dir)
+*dir = NULL;
/* sysconf is a hint; if it fails, fall back to a reasonable size */
if (val 0)
@@ -2312,7 +2314,7 @@ static char *virGetUserEnt(uid_t uid,
if (VIR_ALLOC_N(strbuf, strbuflen) 0) {
virReportOOMError();
-return NULL;
+return -1;
}
/*
@@ -2325,28 +2327,33 @@ static char *virGetUserEnt(uid_t uid,
while ((rc = getpwuid_r(uid, pwbuf, strbuf, strbuflen, pw)) == ERANGE) {
if (VIR_RESIZE_N(strbuf, strbuflen, strbuflen, strbuflen) 0) {
virReportOOMError();
-VIR_FREE(strbuf);
-return NULL;
+goto cleanup;
}
}
if (rc != 0 || pw == NULL) {
virReportSystemError(rc,
_(Failed to find user record for uid '%u'),
(unsigned int) uid);
-VIR_FREE(strbuf);
-return NULL;
+goto cleanup;
}
-if (field == VIR_USER_ENT_DIRECTORY)
-ret = strdup(pw-pw_dir);
-else
-ret = strdup(pw-pw_name);
+if (name !(*name = strdup(pw-pw_name)))
+goto no_memory;
+if (group)
+*group = pw-pw_gid;
+if (dir !(*dir = strdup(pw-pw_dir))) {
+if (name)
+VIR_FREE(*name);
+goto no_memory;
+}
+ret = 0;
+cleanup:
VIR_FREE(strbuf);
-if (!ret)
-virReportOOMError();
-
return ret;
+no_memory:
+virReportOOMError();
+goto cleanup;
}
static char *virGetGroupEnt(gid_t gid)
@@ -2401,20 +2408,23 @@ static char *virGetGroupEnt(gid_t gid)
char *virGetUserDirectory(void)
{
-return virGetUserEnt(geteuid(), VIR_USER_ENT_DIRECTORY);
+char *ret;
+virGetUserEnt(geteuid(), NULL, NULL, ret);
+return ret;
}
static char *virGetXDGDirectory(const char *xdgenvname, const char *xdgdefdir)
{
const char *path = getenv(xdgenvname);
char *ret = NULL;
-char *home = virGetUserEnt(geteuid(), VIR_USER_ENT_DIRECTORY);
+char *home = virGetUserDirectory();
if (path path[0]) {
if (virAsprintf(ret, %s/libvirt, path) 0)
goto no_memory;
} else {
-if (virAsprintf(ret, %s/%s/libvirt, home, xdgdefdir) 0)
+if (home
+virAsprintf(ret, %s/%s/libvirt, home, xdgdefdir) 0)
goto no_memory;
}
@@ -2456,7 +2466,9 @@ char *virGetUserRuntimeDirectory(void)
char *virGetUserName(uid_t uid)
{
-return virGetUserEnt(uid, VIR_USER_ENT_NAME);
+char *ret;
+virGetUserEnt(uid, ret, NULL, NULL);
+return ret;
}
char *virGetGroupName(gid_t gid)
--
1.8.3.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 4/7] Fix potential deadlock across fork() in QEMU driver
From: Daniel P. Berrange berra...@redhat.com
https://bugzilla.redhat.com/show_bug.cgi?id=964358
The hook scripts used by virCommand must be careful wrt
accessing any mutexes that may have been held by other
threads in the parent process. With the recent refactoring
there are 2 potential flaws lurking, which will become real
deadlock bugs once the global QEMU driver lock is removed.
Remove use of the QEMU driver lock from the hook function
by passing in the 'virQEMUDriverConfigPtr' instance directly.
Add functions to the virSecurityManager to be invoked before
and after fork, to ensure the mutex is held by the current
thread. This allows it to be safely used in the hook script
in the child process.
Signed-off-by: Daniel P. Berrange berra...@redhat.com
(cherry picked from commit 61b52d2e3813cc8c9ff3ab67f232bd0c65f7318d)
Conflicts:
src/libvirt_private.syms - context
src/qemu/qemu_process.c - no backport of qemud_driver struct rename
src/security/security_manager.c - no backport of making the security
driver self-locking; just expose the interface
---
src/libvirt_private.syms| 2 ++
src/qemu/qemu_process.c | 7 +++
src/security/security_manager.c | 20
src/security/security_manager.h | 3 +++
4 files changed, 32 insertions(+)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 24f7047..17e5eff 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1047,6 +1047,8 @@ virSecurityManagerGetProcessLabel;
virSecurityManagerNew;
virSecurityManagerNewStack;
virSecurityManagerNewDAC;
+virSecurityManagerPostFork;
+virSecurityManagerPreFork;
virSecurityManagerReleaseLabel;
virSecurityManagerReserveLabel;
virSecurityManagerRestoreImageLabel;
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index cfadc2c..01c6880 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -2579,6 +2579,11 @@ static int qemuProcessHook(void *data)
struct qemuProcessHookData *h = data;
int ret = -1;
int fd;
+/* This method cannot use any mutexes, which are not
+ * protected across fork()
+ */
+
+virSecurityManagerPostFork(h-driver-securityManager);
/* Some later calls want pid present */
h-vm-pid = getpid();
@@ -3640,7 +3645,9 @@ int qemuProcessStart(virConnectPtr conn,
virCommandDaemonize(cmd);
virCommandRequireHandshake(cmd);
+virSecurityManagerPreFork(driver-securityManager);
ret = virCommandRun(cmd, NULL);
+virSecurityManagerPostFork(driver-securityManager);
/* wait for qemu process to show up */
if (ret == 0) {
diff --git a/src/security/security_manager.c b/src/security/security_manager.c
index d446607..fc7acff 100644
--- a/src/security/security_manager.c
+++ b/src/security/security_manager.c
@@ -161,6 +161,26 @@ virSecurityManagerPtr virSecurityManagerNew(const char
*name,
requireConfined);
}
+
+/*
+ * Must be called before fork()'ing to ensure mutex state
+ * is sane for the child to use
+ */
+void virSecurityManagerPreFork(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED)
+{
+/* XXX Grab our own mutex here instead of relying on caller's mutex */
+}
+
+
+/*
+ * Must be called after fork()'ing in both parent and child
+ * to ensure mutex state is sane for the child to use
+ */
+void virSecurityManagerPostFork(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED)
+{
+/* XXX Release our own mutex here instead of relying on caller's mutex */
+}
+
void *virSecurityManagerGetPrivateData(virSecurityManagerPtr mgr)
{
/* This accesses the memory just beyond mgr, which was allocated
diff --git a/src/security/security_manager.h b/src/security/security_manager.h
index 1fdaf8e..d1a5997 100644
--- a/src/security/security_manager.h
+++ b/src/security/security_manager.h
@@ -46,6 +46,9 @@ virSecurityManagerPtr virSecurityManagerNewDAC(const char
*virtDriver,
bool requireConfined,
bool dynamicOwnership);
+void virSecurityManagerPreFork(virSecurityManagerPtr mgr);
+void virSecurityManagerPostFork(virSecurityManagerPtr mgr);
+
void *virSecurityManagerGetPrivateData(virSecurityManagerPtr mgr);
void virSecurityManagerFree(virSecurityManagerPtr mgr);
--
1.8.3.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 3/7] util: make virSetUIDGID async-signal-safe
https://bugzilla.redhat.com/show_bug.cgi?id=964358 POSIX states that multi-threaded apps should not use functions that are not async-signal-safe between fork and exec, yet we were using getpwuid_r and initgroups. Although rare, it is possible to hit deadlock in the child, when it tries to grab a mutex that was already held by another thread in the parent. I actually hit this deadlock when testing multiple domains being started in parallel with a command hook, with the following backtrace in the child: Thread 1 (Thread 0x7fd56bbf2700 (LWP 3212)): #0 __lll_lock_wait () at ../nptl/sysdeps/unix/sysv/linux/x86_64/lowlevellock.S:136 #1 0x7fd5761e7388 in _L_lock_854 () from /lib64/libpthread.so.0 #2 0x7fd5761e7257 in __pthread_mutex_lock (mutex=0x7fd56be00360) at pthread_mutex_lock.c:61 #3 0x7fd56bbf9fc5 in _nss_files_getpwuid_r (uid=0, result=0x7fd56bbf0c70, buffer=0x7fd55c2a65f0 , buflen=1024, errnop=0x7fd56bbf25b8) at nss_files/files-pwd.c:40 #4 0x7fd575aeff1d in __getpwuid_r (uid=0, resbuf=0x7fd56bbf0c70, buffer=0x7fd55c2a65f0 , buflen=1024, result=0x7fd56bbf0cb0) at ../nss/getXXbyYY_r.c:253 #5 0x7fd578aebafc in virSetUIDGID (uid=0, gid=0) at util/virutil.c:1031 #6 0x7fd578aebf43 in virSetUIDGIDWithCaps (uid=0, gid=0, capBits=0, clearExistingCaps=true) at util/virutil.c:1388 #7 0x7fd578a9a20b in virExec (cmd=0x7fd55c231f10) at util/vircommand.c:654 #8 0x7fd578a9dfa2 in virCommandRunAsync (cmd=0x7fd55c231f10, pid=0x0) at util/vircommand.c:2247 #9 0x7fd578a9d74e in virCommandRun (cmd=0x7fd55c231f10, exitstatus=0x0) at util/vircommand.c:2100 #10 0x7fd56326fde5 in qemuProcessStart (conn=0x7fd53c000df0, driver=0x7fd55c0dc4f0, vm=0x7fd54800b100, migrateFrom=0x0, stdin_fd=-1, stdin_path=0x0, snapshot=0x0, vmop=VIR_NETDEV_VPORT_PROFILE_OP_CREATE, flags=1) at qemu/qemu_process.c:3694 ... The solution is to split the work of getpwuid_r/initgroups into the unsafe portions (getgrouplist, called pre-fork) and safe portions (setgroups, called post-fork). * src/util/virutil.h (virSetUIDGID, virSetUIDGIDWithCaps): Adjust signature. * src/util/virutil.c (virSetUIDGID): Add parameters. (virSetUIDGIDWithCaps): Adjust clients. * src/util/vircommand.c (virExec): Likewise. * src/util/virfile.c (virFileAccessibleAs, virFileOpenForked) (virDirCreate): Likewise. * src/security/security_dac.c (virSecurityDACSetProcessLabel): Likewise. * src/lxc/lxc_container.c (lxcContainerSetID): Likewise. * configure.ac (AC_CHECK_FUNCS_ONCE): Check for setgroups, not initgroups. Signed-off-by: Eric Blake ebl...@redhat.com (cherry picked from commit ee777e994927ed5f2d427fbc5a53cbe8b5969bda) Conflicts: src/lxc/lxc_container.c - did not use setUIDGID before 1.1.0 src/util/virutil.c - oom handling changes not backported; no virSetUIDGIDWithCaps src/util/virfile.c - functions still lived in virutil.c this far back configure.ac - context with previous commit src/util/command.c - no UID/GID handling in vircommand.c... src/storage/storage_backend.c - ...so do it in the one hook user instead --- configure.ac | 4 +- src/lxc/lxc_container.c | 2 +- src/security/security_dac.c | 16 -- src/storage/storage_backend.c | 16 +- src/util/util.c | 129 +- src/util/util.h | 2 +- 6 files changed, 83 insertions(+), 86 deletions(-) diff --git a/configure.ac b/configure.ac index aeff11d..ca34ac1 100644 --- a/configure.ac +++ b/configure.ac @@ -172,8 +172,8 @@ AC_CHECK_SIZEOF([long]) dnl Availability of various common functions (non-fatal if missing), dnl and various less common threadsafe functions AC_CHECK_FUNCS_ONCE([cfmakeraw geteuid getgid getgrnam_r getgrouplist getmntent_r \ - getpwuid_r getuid initgroups kill mmap newlocale posix_fallocate \ - posix_memalign regexec sched_getaffinity]) + getpwuid_r getuid kill mmap newlocale posix_fallocate posix_memalign \ + prlimit regexec sched_getaffinity setgroups]) dnl Availability of pthread functions (if missing, win32 threading is dnl assumed). Because of $LIB_PTHREAD, we cannot use AC_CHECK_FUNCS_ONCE. diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c index 8faa664..9d94042 100644 --- a/src/lxc/lxc_container.c +++ b/src/lxc/lxc_container.c @@ -1,5 +1,5 @@ /* - * Copyright (C) 2008-2012 Red Hat, Inc. + * Copyright (C) 2008-2013 Red Hat, Inc. * Copyright (C) 2008 IBM Corp. * * lxc_container.c: file description diff --git a/src/security/security_dac.c b/src/security/security_dac.c index ae5d8aa..61b705c 100644 --- a/src/security/security_dac.c +++ b/src/security/security_dac.c @@ -839,16 +839,24 @@ virSecurityDACSetProcessLabel(virSecurityManagerPtr mgr, uid_t user; gid_t group; virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr); +gid_t *groups; +int ngroups; +int
[libvirt] [PATCH 7/7] security: fix deadlock with prefork
https://bugzilla.redhat.com/show_bug.cgi?id=964358 Attempts to start a domain with both SELinux and DAC security modules loaded will deadlock; latent problem introduced in commit fdb3bde and exposed in commit 29fe5d7. Basically, when recursing into the security manager for other driver's prefork, we have to undo the asymmetric lock taken at the manager level. Reported by Jiri Denemark, with diagnosis help from Dan Berrange. * src/security/security_stack.c (virSecurityStackPreFork): Undo extra lock grabbed during recursion. Signed-off-by: Eric Blake ebl...@redhat.com (cherry picked from commit bfc183c1e377b24cebf5cede4c00f3dc0d1b3486) --- src/security/security_stack.c | 5 + 1 file changed, 5 insertions(+) diff --git a/src/security/security_stack.c b/src/security/security_stack.c index e8133c4..38fe8b5 100644 --- a/src/security/security_stack.c +++ b/src/security/security_stack.c @@ -129,6 +129,11 @@ virSecurityStackPreFork(virSecurityManagerPtr mgr) rc = -1; break; } +/* Undo the unbalanced locking left behind after recursion; if + * PostFork ever delegates to driver callbacks, we'd instead + * need to recurse to an internal method that does not regrab + * a lock. */ +virSecurityManagerPostFork(item-securityManager); } return rc; -- 1.8.3.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/7] backport of getGroupList to v0.10.2-maint
https://bugzilla.redhat.com/show_bug.cgi?id=964358 Since it was on Fedora 18 that I first noticed the deadlock possible when a child process calls getpwuid_r while the parent owned the lock in a different thread, I'm interested in backporting my recent work on virGetGroupList to v0.10.2-maint. However, I hit enough conflict resolution that I'd like a review of my backport decisions before pushing this to the stable branch. Daniel P. Berrange (1): Fix potential deadlock across fork() in QEMU driver Eric Blake (6): util: improve user lookup helper util: add virGetGroupList util: make virSetUIDGID async-signal-safe security: framework for driver PreFork handler security_dac: compute supplemental groups before fork security: fix deadlock with prefork configure.ac| 6 +- src/libvirt_private.syms| 3 + src/lxc/lxc_container.c | 2 +- src/qemu/qemu_process.c | 8 ++ src/security/security_dac.c | 56 -- src/security/security_driver.h | 4 + src/security/security_manager.c | 30 + src/security/security_manager.h | 3 + src/security/security_stack.c | 28 + src/storage/storage_backend.c | 16 ++- src/util/util.c | 237 src/util/util.h | 4 +- 12 files changed, 285 insertions(+), 112 deletions(-) -- 1.8.3.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3 4/7] qemu: Translate the iscsi pool/volume disk source
Il 23/07/2013 16:14, Daniel P. Berrange ha scritto: On Tue, Jul 23, 2013 at 03:52:56PM +0200, Paolo Bonzini wrote: Il 23/07/2013 15:36, Daniel P. Berrange ha scritto: On Tue, Jul 23, 2013 at 03:29:40PM +0200, Paolo Bonzini wrote: Il 23/07/2013 15:26, Daniel P. Berrange ha scritto: Ok, this answers my question. :) I think the default mode should be direct, because otherwise things such as persistent reservations do not work. No, the default has to be host mode, because that is the only mode that is guaranteed to be usable with any QEMU. The direct mode requires a QEMU that is new enough, and a distro to have enabled it. We can't rely on that as a default choice. Volume sources are also new enough that you can assume a good QEMU. No we can't assume that. New libvirt is frequently used with old QEMU and we want to have good default behaviour there. New libvirt, yes. But can't new libvirt features also assume a new QEMU if the old one causes more trouble than anything? I've no problem with new features requiring new QEMU but that's not the issue here. This is a question about what the default configuration should be. For default configuration we aim for maximum compatibility not the latest possible features. Shouldn't default configuration be simply what makes the most sense? If you use host mode and the guest issues reservation commands, you may get data corruption. That is the brokenness. So be it, that's been the case for every version of libvirt and QEMU in existance up until iscsi protocol support was added, so it is not a new issue. That doesn't justify choosing a default that is guaranteed to not work at all. I started adding iSCSI support to ensure that iSCSI LUNs would work. This makes them _not_ work by default. It makes me regret asking to add support for volumes as a disk source. Paolo -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 5/5] conf: add pcie-root controller
On Tue, Jul 23, 2013 at 10:44:55AM -0400, Laine Stump wrote: This controller is implicit on q35 machinetypes. It provides 31 PCIe (*not* PCI) slots as controller 0. Currently there are no devices that can connect to pcie-root. For a usable q35 system, we still need to add a dmi-to-pci-bridge pci controller, which can connect to pcie-root, and provides pci slots. Presumably you have another patch which auto-adds a 'dmi-to-pci-bridge' device when we see a q35 machine, so that auto-PCI address assignment still works ? If we relied on the user to add that device, then legacy apps will not work against a QEMU which defaults to q35, since they'll be adding PCI devices but not know that they need to add this bridge. 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 08/13] Create + setup cgroups atomically for LXC process
From: Daniel P. Berrange berra...@redhat.com
Currently the LXC driver creates the VM's cgroup prior to
forking, and then libvirt_lxc moves the child process
into the cgroup. This won't work with systemd whose APIs
do the creation of cgroups + attachment of processes atomically.
Fortunately we simply move the entire of cgroups setup into
the libvirt_lxc child process. We make it take place before
fork'ing into the background, so by the time virCommandRun
returns in the LXC driver, the cgroup is guaranteed to be
present.
Signed-off-by: Daniel P. Berrange berra...@redhat.com
---
src/lxc/lxc_controller.c | 19 ++-
src/lxc/lxc_process.c| 40 +++-
2 files changed, 37 insertions(+), 22 deletions(-)
diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c
index 41d69b3..bbec344 100644
--- a/src/lxc/lxc_controller.c
+++ b/src/lxc/lxc_controller.c
@@ -128,6 +128,8 @@ struct _virLXCController {
bool inShutdown;
int timerShutdown;
+virCgroupPtr cgroup;
+
virLXCFusePtr fuse;
};
@@ -275,6 +277,8 @@ static void virLXCControllerFree(virLXCControllerPtr ctrl)
virObjectUnref(ctrl-server);
virLXCControllerFreeFuse(ctrl);
+virCgroupFree(ctrl-cgroup);
+
/* This must always be the last thing to be closed */
VIR_FORCE_CLOSE(ctrl-handshakeFd);
VIR_FREE(ctrl);
@@ -657,8 +661,7 @@ cleanup:
*
* Returns 0 on success or -1 in case of error
*/
-static int virLXCControllerSetupResourceLimits(virLXCControllerPtr ctrl,
- virCgroupPtr cgroup)
+static int virLXCControllerSetupResourceLimits(virLXCControllerPtr ctrl)
{
virBitmapPtr nodemask = NULL;
int ret = -1;
@@ -670,7 +673,7 @@ static int
virLXCControllerSetupResourceLimits(virLXCControllerPtr ctrl,
if (virLXCControllerSetupCpuAffinity(ctrl) 0)
goto cleanup;
-if (virLXCCgroupSetup(ctrl-def, cgroup, nodemask) 0)
+if (virLXCCgroupSetup(ctrl-def, ctrl-cgroup, nodemask) 0)
goto cleanup;
ret = 0;
@@ -2102,7 +2105,6 @@ virLXCControllerRun(virLXCControllerPtr ctrl)
int containerhandshake[2] = { -1, -1 };
char **containerTTYPaths = NULL;
size_t i;
-virCgroupPtr cgroup = NULL;
if (VIR_ALLOC_N(containerTTYPaths, ctrl-nconsoles) 0)
goto cleanup;
@@ -2122,13 +2124,10 @@ virLXCControllerRun(virLXCControllerPtr ctrl)
if (virLXCControllerSetupPrivateNS() 0)
goto cleanup;
-if (!(cgroup = virLXCCgroupJoin(ctrl-def)))
-goto cleanup;
-
if (virLXCControllerSetupLoopDevices(ctrl) 0)
goto cleanup;
-if (virLXCControllerSetupResourceLimits(ctrl, cgroup) 0)
+if (virLXCControllerSetupResourceLimits(ctrl) 0)
goto cleanup;
if (virLXCControllerSetupDevPTS(ctrl) 0)
@@ -2214,7 +2213,6 @@ cleanup:
VIR_FREE(containerTTYPaths[i]);
VIR_FREE(containerTTYPaths);
-virCgroupFree(cgroup);
virLXCControllerStopInit(ctrl);
return rc;
@@ -2390,6 +2388,9 @@ int main(int argc, char *argv[])
if (virLXCControllerValidateConsoles(ctrl) 0)
goto cleanup;
+if (!(ctrl-cgroup = virLXCCgroupJoin(ctrl-def)))
+goto cleanup;
+
if (virLXCControllerSetupServer(ctrl) 0)
goto cleanup;
diff --git a/src/lxc/lxc_process.c b/src/lxc/lxc_process.c
index 3642945..1b31cef 100644
--- a/src/lxc/lxc_process.c
+++ b/src/lxc/lxc_process.c
@@ -49,6 +49,7 @@
#include virhook.h
#include virstring.h
#include viratomic.h
+#include virprocess.h
#define VIR_FROM_THIS VIR_FROM_LXC
@@ -701,9 +702,9 @@ int virLXCProcessStop(virLXCDriverPtr driver,
return -1;
}
} else {
-/* If cgroup doesn't exist, the VM pids must have already
- * died and so we're just cleaning up stale state
- */
+/* If cgroup doesn't exist, just try cleaning up th
+ * libvirt_lxc process */
+virProcessKillPainfully(vm-pid, true);
}
virLXCProcessCleanup(driver, vm, reason);
@@ -971,33 +972,33 @@ int virLXCProcessStart(virConnectPtr conn,
virCapsPtr caps = NULL;
virErrorPtr err = NULL;
virLXCDriverConfigPtr cfg = virLXCDriverGetConfig(driver);
+virCgroupPtr selfcgroup;
-virCgroupFree(priv-cgroup);
-
-if (!(priv-cgroup = virLXCCgroupCreate(vm-def)))
+if (virCgroupNewSelf(selfcgroup) 0)
return -1;
-if (!virCgroupHasController(priv-cgroup,
+if (!virCgroupHasController(selfcgroup,
VIR_CGROUP_CONTROLLER_CPUACCT)) {
-virCgroupFree(priv-cgroup);
+virCgroupFree(selfcgroup);
virReportError(VIR_ERR_INTERNAL_ERROR, %s,
_(Unable to find 'cpuacct' cgroups controller mount));
return -1;
}
-if (!virCgroupHasController(priv-cgroup,
+if (!virCgroupHasController(selfcgroup,
VIR_CGROUP_CONTROLLER_DEVICES)) {
-
[libvirt] [PATCH 01/13] Fix handling of DBus errors emitted by the bus itself
From: Daniel P. Berrange berra...@redhat.com
Current code for handling dbus errors only works for errors
received from the remote application itself. We must also
handle errors emitted by the bus itself, for example, when
it fails to spawn the target service.
Signed-off-by: Daniel P. Berrange berra...@redhat.com
---
src/util/virdbus.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/src/util/virdbus.c b/src/util/virdbus.c
index 6221bdc..9b0977a 100644
--- a/src/util/virdbus.c
+++ b/src/util/virdbus.c
@@ -1129,9 +1129,8 @@ int virDBusCallMethod(DBusConnection *conn,
call,
VIR_DBUS_METHOD_CALL_TIMEOUT_MILLIS,
error))) {
-virReportError(VIR_ERR_INTERNAL_ERROR,
- _(Cannot send to %s.%s on path %s with interface %s:
%s),
- destination, member, path, interface,
NULLSTR(error.message));
+virReportDBusServiceError(error.message ? error.message : unknown
error,
+ error.name);
goto cleanup;
}
--
1.8.1.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/5] qemu: turn qemuDomainPCIAddressBus into a struct
On Tue, Jul 23, 2013 at 10:44:51AM -0400, Laine Stump wrote: qemuDomainPCIAddressBus was an array of QEMU_PCI_ADDRESS_SLOT_LAST uint8_t's, which worked fine as long as every PCI bus was identical. In the future, some PCI busses will allow connecting PCI devices, and some will allow PCIe devices; also some will only allow connection of a single device, while others will allow connecting 31 devices. In order to keep track of that information for each bus, we need to turn qemuDomainPCIAddressBus into a struct, for now with just one member: uint8_t slots[QEMU_PCI_ADDRESS_SLOT_LAST]; Additional members will come in later patches. The item in qemuDomainPCIAddresSet that contains the array of qemuDomainPCIAddressBus is now called buses to be more consistent with the already existing nbuses (and with the new slots array). --- src/qemu/qemu_command.c | 47 --- 1 file changed, 24 insertions(+), 23 deletions(-) ACK, no functional change here. 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 03/13] Add a virCgroupNewDetect API for finding cgroup placement
From: Daniel P. Berrange berra...@redhat.com
Add a virCgroupNewDetect API which is used to initialize a
cgroup object with the placement of an arbitrary process.
Signed-off-by: Daniel P. Berrange berra...@redhat.com
---
src/libvirt_private.syms | 1 +
src/util/vircgroup.c | 81 +++-
src/util/vircgroup.h | 3 ++
3 files changed, 57 insertions(+), 28 deletions(-)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 76873ad..49b9f9d 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1186,6 +1186,7 @@ virCgroupKill;
virCgroupKillPainfully;
virCgroupKillRecursive;
virCgroupMoveTask;
+virCgroupNewDetect;
virCgroupNewDomainDriver;
virCgroupNewDomainPartition;
virCgroupNewDriver;
diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c
index 5251611..94d19e0 100644
--- a/src/util/vircgroup.c
+++ b/src/util/vircgroup.c
@@ -306,18 +306,30 @@ static int virCgroupCopyPlacement(virCgroupPtr group,
* It then appends @path to each detected path.
*/
static int virCgroupDetectPlacement(virCgroupPtr group,
+pid_t pid,
const char *path)
{
size_t i;
FILE *mapping = NULL;
char line[1024];
int ret = -1;
+char *procfile;
-mapping = fopen(/proc/self/cgroup, r);
+if (pid == -1) {
+if (VIR_STRDUP(procfile, /proc/self/cgroup) 0)
+goto cleanup;
+} else {
+if (virAsprintf(procfile, /proc/%llu/cgroup,
+(unsigned long long)pid) 0)
+goto cleanup;
+}
+
+mapping = fopen(procfile, r);
if (mapping == NULL) {
-virReportSystemError(errno, %s,
- _(Unable to open /proc/self/cgroup));
-return -1;
+virReportSystemError(errno,
+ _(Unable to open '%s'),
+ procfile);
+goto cleanup;
}
while (fgets(line, sizeof(line), mapping) != NULL) {
@@ -371,12 +383,14 @@ static int virCgroupDetectPlacement(virCgroupPtr group,
ret = 0;
cleanup:
+VIR_FREE(procfile);
VIR_FORCE_FCLOSE(mapping);
return ret;
}
static int virCgroupDetect(virCgroupPtr group,
+ pid_t pid,
int controllers,
const char *path,
virCgroupPtr parent)
@@ -453,7 +467,7 @@ static int virCgroupDetect(virCgroupPtr group,
if (virCgroupCopyPlacement(group, path, parent) 0)
return -1;
} else {
-if (virCgroupDetectPlacement(group, path) 0)
+if (virCgroupDetectPlacement(group, pid, path) 0)
return -1;
}
@@ -470,10 +484,11 @@ static int virCgroupDetect(virCgroupPtr group,
return -1;
}
-VIR_DEBUG(Detected mount/mapping %zu:%s at %s in %s, i,
+VIR_DEBUG(Detected mount/mapping %zu:%s at %s in %s for pid %llu, i,
virCgroupControllerTypeToString(i),
group-controllers[i].mountPoint,
- group-controllers[i].placement);
+ group-controllers[i].placement,
+ (unsigned long long)pid);
}
return 0;
@@ -826,7 +841,8 @@ cleanup:
*
* Returns 0 on success, -1 on error
*/
-static int virCgroupNew(const char *path,
+static int virCgroupNew(pid_t pid,
+const char *path,
virCgroupPtr parent,
int controllers,
virCgroupPtr *group)
@@ -849,7 +865,7 @@ static int virCgroupNew(const char *path,
goto error;
}
-if (virCgroupDetect(*group, controllers, path, parent) 0)
+if (virCgroupDetect(*group, pid, controllers, path, parent) 0)
goto error;
return 0;
@@ -871,7 +887,7 @@ static int virCgroupAppRoot(virCgroupPtr *group,
if (virCgroupNewSelf(selfgrp) 0)
return -1;
-if (virCgroupNew(libvirt, selfgrp, controllers, group) 0)
+if (virCgroupNew(-1, libvirt, selfgrp, controllers, group) 0)
goto cleanup;
if (virCgroupMakeGroup(selfgrp, *group, create, VIR_CGROUP_NONE) 0)
@@ -1287,7 +1303,7 @@ int virCgroupNewPartition(const char *path,
if (virCgroupSetPartitionSuffix(path, newpath) 0)
goto cleanup;
-if (virCgroupNew(newpath, NULL, controllers, group) 0)
+if (virCgroupNew(-1, newpath, NULL, controllers, group) 0)
goto cleanup;
if (STRNEQ(newpath, /)) {
@@ -1299,7 +1315,7 @@ int virCgroupNewPartition(const char *path,
tmp++;
*tmp = '\0';
-if (virCgroupNew(parentPath, NULL, controllers, parent) 0)
+if (virCgroupNew(-1, parentPath, NULL, controllers, parent) 0)
goto cleanup;
if (virCgroupMakeGroup(parent, *group, create, VIR_CGROUP_NONE) 0) {
@@ -1350,7 +1366,7 @@ int virCgroupNewDriver(const
[libvirt] [PATCH 00/13] Support use of systemd-machined for cgroups
From: Daniel P. Berrange berra...@redhat.com This is a patch series which adds support for using systemd-machined for creating cgroups. The first 12 patches are all really just cleanups and refactoring. The actual systemd code is the last patch, but at time of posting it doesn't quite work properly. Given the freeze, I'd like to get the first 12 patches merged, while I iron out the remaining problems with the last patch. Daniel P. Berrange (13): Fix handling of DBus errors emitted by the bus itself Add logic for handling systemd-machined non-existance Add a virCgroupNewDetect API for finding cgroup placement Add API for checking if a cgroup is valid for a domain Auto-detect existing cgroup placement Remove obsolete cgroups creation apis Create + setup cgroups atomically for QEMU process Create + setup cgroups atomically for LXC process New cgroups API for atomically creating machine cgroups Convert QEMU driver to use virCgroupNewMachine Convert LXC driver to use virCgroupNewMachine Protection against doing bad stuff to the root group Enable support for systemd-machined in cgroups creation src/libvirt_private.syms | 5 +- src/lxc/lxc_cgroup.c | 82 +++--- src/lxc/lxc_cgroup.h | 2 +- src/lxc/lxc_controller.c | 19 +-- src/lxc/lxc_process.c| 52 +-- src/qemu/qemu_cgroup.c | 114 -- src/qemu/qemu_cgroup.h | 5 +- src/qemu/qemu_process.c | 41 +++-- src/util/vircgroup.c | 388 +-- src/util/vircgroup.h | 32 ++-- src/util/virdbus.c | 5 +- src/util/virsystemd.c| 19 ++- tests/vircgrouptest.c| 112 -- tests/virsystemdmock.c | 14 +- tests/virsystemdtest.c | 63 +--- 15 files changed, 496 insertions(+), 457 deletions(-) -- 1.8.1.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 09/13] New cgroups API for atomically creating machine cgroups
From: Daniel P. Berrange berra...@redhat.com
Instead of requiring one API call to create a cgroup and
another to add a task to it, introduce a new API
virCgroupNewMachine which does both jobs at once. This
will facilitate the later code to talk to systemd to
achieve this job which is also atomic.
Signed-off-by: Daniel P. Berrange berra...@redhat.com
---
src/libvirt_private.syms | 1 +
src/util/vircgroup.c | 51
src/util/vircgroup.h | 13
3 files changed, 65 insertions(+)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 5a5d112..eef6bdd 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1191,6 +1191,7 @@ virCgroupNewDetect;
virCgroupNewDomainPartition;
virCgroupNewEmulator;
virCgroupNewIgnoreError;
+virCgroupNewMachine;
virCgroupNewPartition;
virCgroupNewSelf;
virCgroupNewVcpu;
diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c
index 593caad..6f9d25a 100644
--- a/src/util/vircgroup.c
+++ b/src/util/vircgroup.c
@@ -1563,6 +1563,57 @@ int virCgroupNewDetect(pid_t pid ATTRIBUTE_UNUSED,
}
#endif
+int virCgroupNewMachine(const char *name,
+const char *drivername,
+bool privileged ATTRIBUTE_UNUSED,
+const unsigned char *uuid ATTRIBUTE_UNUSED,
+const char *rootdir ATTRIBUTE_UNUSED,
+pid_t pidleader ATTRIBUTE_UNUSED,
+bool isContainer ATTRIBUTE_UNUSED,
+const char *partition,
+int controllers,
+virCgroupPtr *group)
+{
+virCgroupPtr parent = NULL;
+int ret = -1;
+
+*group = NULL;
+
+if (virCgroupNewPartition(partition,
+ STREQ(partition, /machine),
+ controllers,
+ parent) 0) {
+if (virCgroupNewIgnoreError())
+goto done;
+
+goto cleanup;
+}
+
+if (virCgroupNewDomainPartition(parent,
+drivername,
+name,
+true,
+group) 0)
+goto cleanup;
+
+if (virCgroupAddTask(*group, pidleader) 0) {
+virErrorPtr saved = virSaveLastError();
+virCgroupRemove(*group);
+virCgroupFree(group);
+if (saved) {
+virSetError(saved);
+virFreeError(saved);
+}
+}
+
+done:
+ret = 0;
+
+cleanup:
+virCgroupFree(parent);
+return ret;
+}
+
bool virCgroupNewIgnoreError(void)
{
if (virLastErrorIsSystemErrno(ENXIO) ||
diff --git a/src/util/vircgroup.h b/src/util/vircgroup.h
index 3c05604..e47367c 100644
--- a/src/util/vircgroup.h
+++ b/src/util/vircgroup.h
@@ -83,6 +83,19 @@ int virCgroupNewEmulator(virCgroupPtr domain,
int virCgroupNewDetect(pid_t pid,
virCgroupPtr *group);
+int virCgroupNewMachine(const char *name,
+const char *drivername,
+bool privileged,
+const unsigned char *uuid,
+const char *rootdir,
+pid_t pidleader,
+bool isContainer,
+const char *partition,
+int controllers,
+virCgroupPtr *group)
+ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2)
+ATTRIBUTE_NONNULL(4);
+
bool virCgroupNewIgnoreError(void);
int virCgroupPathOfController(virCgroupPtr group,
--
1.8.1.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 12/13] Protection against doing bad stuff to the root group
From: Daniel P. Berrange berra...@redhat.com
Add protection such that the virCgroupRemove and
virCgroupKill* do not do anything to the root cgroup.
Killing all PIDs in the root cgroup does not end well.
Signed-off-by: Daniel P. Berrange berra...@redhat.com
---
src/util/vircgroup.c | 12 +++-
1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c
index 6f9d25a..2141154 100644
--- a/src/util/vircgroup.c
+++ b/src/util/vircgroup.c
@@ -546,8 +546,13 @@ int virCgroupPathOfController(virCgroupPtr group,
if (controller == -1) {
size_t i;
for (i = 0; i VIR_CGROUP_CONTROLLER_LAST; i++) {
+/* Reject any controller with a placement
+ * of '/' to avoid doing bad stuff to the root
+ * cgroup
+ */
if (group-controllers[i].mountPoint
-group-controllers[i].placement) {
+group-controllers[i].placement
+STRNEQ(group-controllers[i].placement, /)) {
controller = i;
break;
}
@@ -1002,6 +1007,11 @@ int virCgroupRemove(virCgroupPtr group)
if (!group-controllers[i].mountPoint)
continue;
+/* Don't delete the root group, if we accidentally
+ ended up in it for some reason */
+if (STREQ(group-controllers[i].placement, /))
+continue;
+
if (virCgroupPathOfController(group,
i,
NULL,
--
1.8.1.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 02/13] Add logic for handling systemd-machined non-existance
From: Daniel P. Berrange berra...@redhat.com
If systemd machine does not exist, return -2 instead of -1,
so that applications don't need to repeat the tedious error
checking code
Signed-off-by: Daniel P. Berrange berra...@redhat.com
---
src/util/virsystemd.c | 13 ++-
tests/virsystemdmock.c | 14 +++
tests/virsystemdtest.c | 63 +++---
3 files changed, 66 insertions(+), 24 deletions(-)
diff --git a/src/util/virsystemd.c b/src/util/virsystemd.c
index 8477cd3..11d1153 100644
--- a/src/util/virsystemd.c
+++ b/src/util/virsystemd.c
@@ -26,6 +26,7 @@
#include virstring.h
#include viralloc.h
#include virutil.h
+#include virlog.h
#define VIR_FROM_THIS VIR_FROM_SYSTEMD
@@ -38,6 +39,8 @@
* @rootdir: root directory of machine filesystem
* @pidleader: PID of the leader process
* @slice: name of the slice to place the machine in
+ *
+ * Returns 0 on success, -1 on fatal error, or -2 if systemd-machine is not
available
*/
int virSystemdCreateMachine(const char *name,
const char *drivername,
@@ -117,6 +120,7 @@ int virSystemdCreateMachine(const char *name,
* allow further API calls to be made against the object.
*/
+VIR_DEBUG(Attempting to create machine via systemd);
if (virDBusCallMethod(conn,
NULL,
org.freedesktop.machine1,
@@ -135,8 +139,15 @@ int virSystemdCreateMachine(const char *name,
(unsigned int)pidleader,
rootdir ? rootdir : ,
1, Slice, s,
- slicename) 0)
+ slicename) 0) {
+virErrorPtr err = virGetLastError();
+if (err-code == VIR_ERR_DBUS_SERVICE
+STREQ(err-str2, org.freedesktop.DBus.Error.ServiceUnknown)) {
+virResetLastError();
+ret = -2;
+}
goto cleanup;
+}
ret = 0;
diff --git a/tests/virsystemdmock.c b/tests/virsystemdmock.c
index 5f9cce6..1f4413c 100644
--- a/tests/virsystemdmock.c
+++ b/tests/virsystemdmock.c
@@ -60,16 +60,20 @@ dbus_bool_t
dbus_connection_set_watch_functions(DBusConnection *connection ATTRI
DBusMessage *dbus_connection_send_with_reply_and_block(DBusConnection
*connection ATTRIBUTE_UNUSED,
DBusMessage *message,
int
timeout_milliseconds ATTRIBUTE_UNUSED,
- DBusError *error
ATTRIBUTE_UNUSED)
+ DBusError *error)
{
-DBusMessage *reply;
+DBusMessage *reply = NULL;
dbus_message_set_serial(message, 7);
-if (getenv(FAIL_NO_SERVICE))
+if (getenv(FAIL_BAD_SERVICE))
reply = dbus_message_new_error(message,
-
org.freedesktop.DBus.Error.ServiceUnknown,
- The name org.freedesktop.machine1 was
not provided by any .service files);
+ org.freedesktop.systemd.badthing,
+ Something went wrong creating the
machine);
+else if (getenv(FAIL_NO_SERVICE))
+dbus_set_error(error,
+ org.freedesktop.DBus.Error.ServiceUnknown,
+ %s, The name org.freedesktop.machine1 was not
provided by any .service files);
else
reply = dbus_message_new_method_return(message);
diff --git a/tests/virsystemdtest.c b/tests/virsystemdtest.c
index 3992722..bcf3ad3 100644
--- a/tests/virsystemdtest.c
+++ b/tests/virsystemdtest.c
@@ -82,35 +82,60 @@ static int testCreateNoSystemd(const void *opaque
ATTRIBUTE_UNUSED)
3, 3, 3, 3,
4, 4, 4, 4
};
+int rv;
setenv(FAIL_NO_SERVICE, 1, 1);
-if (virSystemdCreateMachine(demo,
-qemu,
-true,
-uuid,
-NULL,
-123,
-false,
-NULL) == 0) {
+if ((rv = virSystemdCreateMachine(demo,
+ qemu,
+ true,
+ uuid,
+ NULL,
+ 123,
+ false,
+ NULL)) == 0) {
fprintf(stderr, %s, Unexpected create machine success\n);
return -1;
}
-virErrorPtr err = virGetLastError();
+if (rv != -2) {
+fprintf(stderr, %s, Unexpected create machine error\n);
+return -1;
+}
+
+return 0;
+}
-if (!err) {
-fprintf(stderr, No error raised);
+static int testCreateBadSystemd(const void
[libvirt] [PATCH 10/13] Convert QEMU driver to use virCgroupNewMachine
From: Daniel P. Berrange berra...@redhat.com
Convert the QEMU driver code to use the new atomic API
for setup of cgroups
Signed-off-by: Daniel P. Berrange berra...@redhat.com
---
src/qemu/qemu_cgroup.c | 29 +++--
1 file changed, 11 insertions(+), 18 deletions(-)
diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c
index 6455f50..bca8630 100644
--- a/src/qemu/qemu_cgroup.c
+++ b/src/qemu/qemu_cgroup.c
@@ -633,7 +633,6 @@ qemuInitCgroup(virQEMUDriverPtr driver,
{
int ret = -1;
qemuDomainObjPrivatePtr priv = vm-privateData;
-virCgroupPtr parent = NULL;
virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
if (!cfg-privileged)
@@ -664,32 +663,26 @@ qemuInitCgroup(virQEMUDriverPtr driver,
vm-def-resource-partition);
goto cleanup;
}
-/* We only auto-create the default partition. In other
- * cases we expec the sysadmin/app to have done so */
-if (virCgroupNewPartition(vm-def-resource-partition,
- STREQ(vm-def-resource-partition, /machine),
- cfg-cgroupControllers,
- parent) 0) {
+
+if (virCgroupNewMachine(vm-def-name,
+qemu,
+cfg-privileged,
+vm-def-uuid,
+NULL,
+vm-pid,
+false,
+vm-def-resource-partition,
+cfg-cgroupControllers,
+priv-cgroup) 0) {
if (virCgroupNewIgnoreError())
goto done;
goto cleanup;
}
-if (virCgroupNewDomainPartition(parent,
-qemu,
-vm-def-name,
-true,
-priv-cgroup) 0)
-goto cleanup;
-
-if (virCgroupAddTask(priv-cgroup, vm-pid) 0)
-goto cleanup;
-
done:
ret = 0;
cleanup:
-virCgroupFree(parent);
virObjectUnref(cfg);
return ret;
}
--
1.8.1.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 05/13] Auto-detect existing cgroup placement
From: Daniel P. Berrange berra...@redhat.com
Use the new virCgroupNewDetect function to determine cgroup
placement of existing running VMs. This will allow the legacy
cgroups creation APIs to be removed entirely
Signed-off-by: Daniel P. Berrange berra...@redhat.com
---
src/lxc/lxc_cgroup.c| 59 ++
src/lxc/lxc_cgroup.h| 2 +-
src/lxc/lxc_process.c | 14 ++-
src/qemu/qemu_cgroup.c | 107
src/qemu/qemu_cgroup.h | 5 +--
src/qemu/qemu_process.c | 2 +-
6 files changed, 100 insertions(+), 89 deletions(-)
diff --git a/src/lxc/lxc_cgroup.c b/src/lxc/lxc_cgroup.c
index 025720d..c230c25 100644
--- a/src/lxc/lxc_cgroup.c
+++ b/src/lxc/lxc_cgroup.c
@@ -429,12 +429,12 @@ cleanup:
}
-virCgroupPtr virLXCCgroupCreate(virDomainDefPtr def, bool startup)
+virCgroupPtr virLXCCgroupCreate(virDomainDefPtr def)
{
virCgroupPtr parent = NULL;
virCgroupPtr cgroup = NULL;
-if (!def-resource startup) {
+if (!def-resource) {
virDomainResourceDefPtr res;
if (VIR_ALLOC(res) 0)
@@ -448,41 +448,26 @@ virCgroupPtr virLXCCgroupCreate(virDomainDefPtr def, bool
startup)
def-resource = res;
}
-if (def-resource
-def-resource-partition) {
-if (def-resource-partition[0] != '/') {
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- _(Resource partition '%s' must start with '/'),
- def-resource-partition);
-goto cleanup;
-}
-/* We only auto-create the default partition. In other
- * cases we expec the sysadmin/app to have done so */
-if (virCgroupNewPartition(def-resource-partition,
- STREQ(def-resource-partition, /machine),
- -1,
- parent) 0)
-goto cleanup;
-
-if (virCgroupNewDomainPartition(parent,
-lxc,
-def-name,
-true,
-cgroup) 0)
-goto cleanup;
-} else {
-if (virCgroupNewDriver(lxc,
- true,
- -1,
- parent) 0)
-goto cleanup;
-
-if (virCgroupNewDomainDriver(parent,
- def-name,
- true,
- cgroup) 0)
-goto cleanup;
+if (def-resource-partition[0] != '/') {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _(Resource partition '%s' must start with '/'),
+ def-resource-partition);
+goto cleanup;
}
+/* We only auto-create the default partition. In other
+ * cases we expec the sysadmin/app to have done so */
+if (virCgroupNewPartition(def-resource-partition,
+ STREQ(def-resource-partition, /machine),
+ -1,
+ parent) 0)
+goto cleanup;
+
+if (virCgroupNewDomainPartition(parent,
+lxc,
+def-name,
+true,
+cgroup) 0)
+goto cleanup;
cleanup:
virCgroupFree(parent);
@@ -495,7 +480,7 @@ virCgroupPtr virLXCCgroupJoin(virDomainDefPtr def)
virCgroupPtr cgroup = NULL;
int ret = -1;
-if (!(cgroup = virLXCCgroupCreate(def, true)))
+if (!(cgroup = virLXCCgroupCreate(def)))
return NULL;
if (virCgroupAddTask(cgroup, getpid()) 0)
diff --git a/src/lxc/lxc_cgroup.h b/src/lxc/lxc_cgroup.h
index f040de2..25a427c 100644
--- a/src/lxc/lxc_cgroup.h
+++ b/src/lxc/lxc_cgroup.h
@@ -27,7 +27,7 @@
# include lxc_fuse.h
# include virusb.h
-virCgroupPtr virLXCCgroupCreate(virDomainDefPtr def, bool startup);
+virCgroupPtr virLXCCgroupCreate(virDomainDefPtr def);
virCgroupPtr virLXCCgroupJoin(virDomainDefPtr def);
int virLXCCgroupSetup(virDomainDefPtr def,
virCgroupPtr cgroup,
diff --git a/src/lxc/lxc_process.c b/src/lxc/lxc_process.c
index 5b83ccb..3642945 100644
--- a/src/lxc/lxc_process.c
+++ b/src/lxc/lxc_process.c
@@ -974,7 +974,7 @@ int virLXCProcessStart(virConnectPtr conn,
virCgroupFree(priv-cgroup);
-if (!(priv-cgroup = virLXCCgroupCreate(vm-def, true)))
+if (!(priv-cgroup = virLXCCgroupCreate(vm-def)))
return -1;
if (!virCgroupHasController(priv-cgroup,
@@ -1385,9 +1385,19 @@ virLXCProcessReconnectDomain(virDomainObjPtr vm,
if (!(priv-monitor = virLXCProcessConnectMonitor(driver, vm)))
goto error;
-if (!(priv-cgroup = virLXCCgroupCreate(vm-def, false)))
+if (virCgroupNewDetect(vm-pid, priv-cgroup)
[libvirt] [PATCH 04/13] Add API for checking if a cgroup is valid for a domain
From: Daniel P. Berrange berra...@redhat.com
Add virCgroupIsValidMachine API to check whether a auto
detected cgroup is valid for a machine. This lets us
check if a VM has just been placed into some generic
shared cgroup, or worse, the root cgroup
Signed-off-by: Daniel P. Berrange berra...@redhat.com
---
src/libvirt_private.syms | 1 +
src/util/vircgroup.c | 42 ++
src/util/vircgroup.h | 5 +
3 files changed, 48 insertions(+)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 49b9f9d..3be604b 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1182,6 +1182,7 @@ virCgroupGetMemSwapHardLimit;
virCgroupGetMemSwapUsage;
virCgroupHasController;
virCgroupIsolateMount;
+virCgroupIsValidMachineGroup;
virCgroupKill;
virCgroupKillPainfully;
virCgroupKillRecursive;
diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c
index 94d19e0..1043318 100644
--- a/src/util/vircgroup.c
+++ b/src/util/vircgroup.c
@@ -67,6 +67,8 @@ typedef enum {
*/
} virCgroupFlags;
+static int virCgroupPartitionEscape(char **path);
+
bool virCgroupAvailable(void)
{
FILE *mounts = NULL;
@@ -91,6 +93,46 @@ bool virCgroupAvailable(void)
return ret;
}
+bool virCgroupIsValidMachineGroup(virCgroupPtr group,
+ const char *name,
+ const char *drivername)
+{
+size_t i;
+bool valid = false;
+char *partname;
+
+if (virAsprintf(partname, %s.libvirt-%s,
+name, drivername) 0)
+goto cleanup;
+
+if (virCgroupPartitionEscape(partname) 0)
+goto cleanup;
+
+for (i = 0; i VIR_CGROUP_CONTROLLER_LAST; i++) {
+char *tmp;
+
+if (!group-controllers[i].placement)
+continue;
+
+tmp = strrchr(group-controllers[i].placement, '/');
+if (!tmp)
+goto cleanup;
+tmp++;
+
+if (STRNEQ(tmp, name)
+STRNEQ(tmp, partname))
+goto cleanup;
+
+}
+
+valid = true;
+
+ cleanup:
+VIR_FREE(partname);
+return valid;
+}
+
+
/**
* virCgroupFree:
*
diff --git a/src/util/vircgroup.h b/src/util/vircgroup.h
index 602d4ff..9bf0d7e 100644
--- a/src/util/vircgroup.h
+++ b/src/util/vircgroup.h
@@ -48,6 +48,11 @@ VIR_ENUM_DECL(virCgroupController);
bool virCgroupAvailable(void);
+bool virCgroupIsValidMachineGroup(virCgroupPtr group,
+ const char *machinename,
+ const char *drivername);
+
+
int virCgroupNewPartition(const char *path,
bool create,
int controllers,
--
1.8.1.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 07/13] Create + setup cgroups atomically for QEMU process
From: Daniel P. Berrange berra...@redhat.com
Currently the QEMU driver creates the VM's cgroup prior to
forking, and then uses a virCommand hook to move the child
into the cgroup. This won't work with systemd whose APIs
do the creation of cgroups + attachment of processes atomically.
Fortunately we have a handshake taking place between the
QEMU driver and the child process prior to QEMU being exec()d,
which was introduced to allow setup of disk locking. By good
fortune this synchronization point can be used to enable the
QEMU drivedr to do atomic setup of cgroups removing the use
of the hook script.
Signed-off-by: Daniel P. Berrange berra...@redhat.com
---
src/qemu/qemu_cgroup.c | 12 +---
src/qemu/qemu_process.c | 39 +--
2 files changed, 26 insertions(+), 25 deletions(-)
diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c
index 8559d26..6455f50 100644
--- a/src/qemu/qemu_cgroup.c
+++ b/src/qemu/qemu_cgroup.c
@@ -683,6 +683,9 @@ qemuInitCgroup(virQEMUDriverPtr driver,
priv-cgroup) 0)
goto cleanup;
+if (virCgroupAddTask(priv-cgroup, vm-pid) 0)
+goto cleanup;
+
done:
ret = 0;
cleanup:
@@ -738,6 +741,12 @@ qemuSetupCgroup(virQEMUDriverPtr driver,
virCapsPtr caps = NULL;
int ret = -1;
+if (!vm-pid) {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+ _(Cannot setup cgroups until process is started));
+return -1;
+}
+
if (qemuInitCgroup(driver, vm) 0)
return -1;
@@ -1009,8 +1018,5 @@ qemuAddToCgroup(virDomainObjPtr vm)
if (priv-cgroup == NULL)
return 0; /* Not supported, so claim success */
-if (virCgroupAddTask(priv-cgroup, getpid()) 0)
-return -1;
-
return 0;
}
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index c5f281a..e8e459e 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -1929,6 +1929,12 @@ qemuProcessInitCpuAffinity(virQEMUDriverPtr driver,
virBitmapPtr cpumap = NULL;
virBitmapPtr cpumapToSet = NULL;
+if (!vm-pid) {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+ _(Cannot setup CPU affinity until process is
started));
+return -1;
+}
+
if (!(cpumap = qemuPrepareCpumap(driver, nodemask)))
return -1;
@@ -1949,11 +1955,7 @@ qemuProcessInitCpuAffinity(virQEMUDriverPtr driver,
}
}
-/* We are pressuming we are running between fork/exec of QEMU
- * so use '0' to indicate our own process ID. No threads are
- * running at this point
- */
-if (virProcessSetAffinity(0 /* Self */, cpumapToSet) 0)
+if (virProcessSetAffinity(vm-pid, cpumapToSet) 0)
goto cleanup;
ret = 0;
@@ -2562,19 +2564,6 @@ static int qemuProcessHook(void *data)
if (virSecurityManagerClearSocketLabel(h-driver-securityManager,
h-vm-def) 0)
goto cleanup;
-/* This must take place before exec(), so that all QEMU
- * memory allocation is on the correct NUMA node
- */
-VIR_DEBUG(Moving process to cgroup);
-if (qemuAddToCgroup(h-vm) 0)
-goto cleanup;
-
-/* This must be done after cgroup placement to avoid resetting CPU
- * affinity */
-if (!h-vm-def-cputune.emulatorpin
-qemuProcessInitCpuAffinity(h-driver, h-vm, h-nodemask) 0)
-goto cleanup;
-
if (virNumaSetupMemoryPolicy(h-vm-def-numatune, h-nodemask) 0)
goto cleanup;
@@ -3671,10 +3660,6 @@ int qemuProcessStart(virConnectPtr conn,
goto cleanup;
}
-VIR_DEBUG(Setting up domain cgroup (if required));
-if (qemuSetupCgroup(driver, vm, nodemask) 0)
-goto cleanup;
-
if (VIR_ALLOC(priv-monConfig) 0)
goto cleanup;
@@ -3844,6 +3829,16 @@ int qemuProcessStart(virConnectPtr conn,
goto cleanup;
}
+VIR_DEBUG(Setting up domain cgroup (if required));
+if (qemuSetupCgroup(driver, vm, nodemask) 0)
+goto cleanup;
+
+/* This must be done after cgroup placement to avoid resetting CPU
+ * affinity */
+if (!vm-def-cputune.emulatorpin
+qemuProcessInitCpuAffinity(driver, vm, nodemask) 0)
+goto cleanup;
+
VIR_DEBUG(Setting domain security labels);
if (virSecurityManagerSetAllLabel(driver-securityManager,
vm-def, stdin_path) 0)
--
1.8.1.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 11/13] Convert LXC driver to use virCgroupNewMachine
From: Daniel P. Berrange berra...@redhat.com
Convert the LXC driver code to use the new atomic API
for setup of cgroups
Signed-off-by: Daniel P. Berrange berra...@redhat.com
---
src/lxc/lxc_cgroup.c | 53 +++-
src/lxc/lxc_controller.c | 2 +-
2 files changed, 17 insertions(+), 38 deletions(-)
diff --git a/src/lxc/lxc_cgroup.c b/src/lxc/lxc_cgroup.c
index c230c25..af91b04 100644
--- a/src/lxc/lxc_cgroup.c
+++ b/src/lxc/lxc_cgroup.c
@@ -431,7 +431,6 @@ cleanup:
virCgroupPtr virLXCCgroupCreate(virDomainDefPtr def)
{
-virCgroupPtr parent = NULL;
virCgroupPtr cgroup = NULL;
if (!def-resource) {
@@ -454,46 +453,26 @@ virCgroupPtr virLXCCgroupCreate(virDomainDefPtr def)
def-resource-partition);
goto cleanup;
}
-/* We only auto-create the default partition. In other
- * cases we expec the sysadmin/app to have done so */
-if (virCgroupNewPartition(def-resource-partition,
- STREQ(def-resource-partition, /machine),
- -1,
- parent) 0)
-goto cleanup;
-if (virCgroupNewDomainPartition(parent,
-lxc,
-def-name,
-true,
-cgroup) 0)
+/*
+ * XXX
+ * We should pass the PID of the LXC init process
+ * not ourselves, but this requires some more
+ * refactoring. We should also pass the root dir
+ */
+if (virCgroupNewMachine(def-name,
+lxc,
+true,
+def-uuid,
+NULL,
+getpid(),
+true,
+def-resource-partition,
+-1,
+cgroup) 0)
goto cleanup;
cleanup:
-virCgroupFree(parent);
-return cgroup;
-}
-
-
-virCgroupPtr virLXCCgroupJoin(virDomainDefPtr def)
-{
-virCgroupPtr cgroup = NULL;
-int ret = -1;
-
-if (!(cgroup = virLXCCgroupCreate(def)))
-return NULL;
-
-if (virCgroupAddTask(cgroup, getpid()) 0)
-goto cleanup;
-
-ret = 0;
-
-cleanup:
-if (ret 0) {
-virCgroupFree(cgroup);
-return NULL;
-}
-
return cgroup;
}
diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c
index bbec344..124ab19 100644
--- a/src/lxc/lxc_controller.c
+++ b/src/lxc/lxc_controller.c
@@ -2388,7 +2388,7 @@ int main(int argc, char *argv[])
if (virLXCControllerValidateConsoles(ctrl) 0)
goto cleanup;
-if (!(ctrl-cgroup = virLXCCgroupJoin(ctrl-def)))
+if (!(ctrl-cgroup = virLXCCgroupCreate(ctrl-def)))
goto cleanup;
if (virLXCControllerSetupServer(ctrl) 0)
--
1.8.1.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 13/13] Enable support for systemd-machined in cgroups creation
From: Daniel P. Berrange berra...@redhat.com
Make the virCgroupNewMachine method try to use systemd-machined
first. If that fails, then fallback to using the traditional
cgroup setup code path.
Signed-off-by: Daniel P. Berrange berra...@redhat.com
---
src/util/vircgroup.c | 115 --
src/util/virsystemd.c | 8 +++-
2 files changed, 108 insertions(+), 15 deletions(-)
diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c
index 2141154..47d9763 100644
--- a/src/util/vircgroup.c
+++ b/src/util/vircgroup.c
@@ -50,6 +50,7 @@
#include virhash.h
#include virhashcode.h
#include virstring.h
+#include virsystemd.h
#define CGROUP_MAX_VAL 512
@@ -100,6 +101,7 @@ bool virCgroupIsValidMachineGroup(virCgroupPtr group,
size_t i;
bool valid = false;
char *partname;
+char *scopename;
if (virAsprintf(partname, %s.libvirt-%s,
name, drivername) 0)
@@ -108,6 +110,13 @@ bool virCgroupIsValidMachineGroup(virCgroupPtr group,
if (virCgroupPartitionEscape(partname) 0)
goto cleanup;
+if (virAsprintf(scopename, machine-%s\\x2d%s.scope,
+drivername, name) 0)
+goto cleanup;
+
+if (virCgroupPartitionEscape(scopename) 0)
+goto cleanup;
+
for (i = 0; i VIR_CGROUP_CONTROLLER_LAST; i++) {
char *tmp;
@@ -120,7 +129,8 @@ bool virCgroupIsValidMachineGroup(virCgroupPtr group,
tmp++;
if (STRNEQ(tmp, name)
-STRNEQ(tmp, partname))
+STRNEQ(tmp, partname)
+STRNEQ(tmp, scopename))
goto cleanup;
}
@@ -129,6 +139,7 @@ bool virCgroupIsValidMachineGroup(virCgroupPtr group,
cleanup:
VIR_FREE(partname);
+VIR_FREE(scopename);
return valid;
}
@@ -1573,22 +1584,63 @@ int virCgroupNewDetect(pid_t pid ATTRIBUTE_UNUSED,
}
#endif
-int virCgroupNewMachine(const char *name,
-const char *drivername,
-bool privileged ATTRIBUTE_UNUSED,
-const unsigned char *uuid ATTRIBUTE_UNUSED,
-const char *rootdir ATTRIBUTE_UNUSED,
-pid_t pidleader ATTRIBUTE_UNUSED,
-bool isContainer ATTRIBUTE_UNUSED,
-const char *partition,
-int controllers,
-virCgroupPtr *group)
+/*
+ * Retujrns 0 on success, -1 on fatal error, -2 on systemd not available
+ */
+static int
+virCgroupNewMachineSystemd(const char *name,
+ const char *drivername,
+ bool privileged,
+ const unsigned char *uuid,
+ const char *rootdir,
+ pid_t pidleader,
+ bool isContainer,
+ const char *partition,
+ virCgroupPtr *group)
+{
+int rv;
+
+VIR_DEBUG(Trying to setup machine '%s' via systemd, name);
+if ((rv = virSystemdCreateMachine(name,
+ drivername,
+ privileged,
+ uuid,
+ rootdir,
+ pidleader,
+ isContainer,
+ partition)) 0)
+return rv;
+
+VIR_DEBUG(Detecting systemd placement);
+if (virCgroupNewDetect(pidleader,
+ group) 0)
+return -1;
+
+if (!virCgroupIsValidMachineGroup(*group,
+ name,
+ drivername)) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ _(Cgroup name is not valid for machine %s),
+ name);
+virCgroupFree(group);
+return -1;
+}
+
+return 0;
+}
+
+static int
+virCgroupNewMachineManual(const char *name,
+ const char *drivername,
+ pid_t pidleader,
+ const char *partition,
+ int controllers,
+ virCgroupPtr *group)
{
virCgroupPtr parent = NULL;
int ret = -1;
-*group = NULL;
-
+VIR_DEBUG(Fallback to non-systemd setup);
if (virCgroupNewPartition(partition,
STREQ(partition, /machine),
controllers,
@@ -1624,6 +1676,43 @@ cleanup:
return ret;
}
+int virCgroupNewMachine(const char *name,
+const char *drivername,
+bool privileged,
+const unsigned char *uuid,
+const char *rootdir,
+pid_t pidleader,
+bool isContainer,
+const char *partition,
+
Re: [libvirt] [PATCH 2/5] qemu: only check for PIIX3-specific device addrs on pc-* machinetypes
On Tue, Jul 23, 2013 at 10:44:52AM -0400, Laine Stump wrote: The implicit IDE, USB, and video controllers provided by the PIIX3 chipset in the pc-* machinetypes are not present on other machinetypes, so we shouldn't be doing the special checking for them. The diffs for this patch look hairy, but that's just because a large section was reindented (to be placed inside a conditional) and git couldn't figure out a sane diff. It really is just 1) determining if this system uses PIIX3, 2) put the stuff that's PIIX3-specific inside an if. (Note that, according to the qemuxml2argv-pseries-usb-multi test, ppc pseries machines also have a PIIX3 chip (since that test file adds a piix3-uhci usb controller). I don't know if this is really the case or not, but had to include that machine type in the checks in order for make check to succeed with no changes to the test data.) --- src/qemu/qemu_command.c | 190 +--- 1 file changed, 99 insertions(+), 91 deletions(-) I'm thinking that it would probably be better to move all the re-indented code out into a qemuValidateDevicePCISlotsPIIX3() and just call that function from qemuAssignDevicePCISlots(). That way if we need to add more validation for other machine types in the future, we have a good modular code structure. This would probably make the diff more sane too, since you wouldn't be indenting code. Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/5] qemu: make QEMU_PCI_ADDRESS_(SLOT|FUNCTION)_LAST less misleading
On Tue, Jul 23, 2013 at 10:44:53AM -0400, Laine Stump wrote: Although these two enums are named ..._LAST, they really had the value of ..._SIZE. This patch changes their values so that, e.g., QEMU_PCI_ADDRESS_SLOT_LAST really is the slot number of the last slot on a PCI bus. --- src/qemu/qemu_command.c | 18 +- 1 file changed, 9 insertions(+), 9 deletions(-) ACK Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 06/13] Remove obsolete cgroups creation apis
From: Daniel P. Berrange berra...@redhat.com
The virCgroupNewDomainDriver and virCgroupNewDriver methods
are obsolete now that we can auto-detect existing cgroup
placement. Delete them to reduce code bloat.
Signed-off-by: Daniel P. Berrange berra...@redhat.com
---
src/libvirt_private.syms | 2 -
src/util/vircgroup.c | 121 ---
src/util/vircgroup.h | 11 -
tests/vircgrouptest.c| 112 ---
4 files changed, 246 deletions(-)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 3be604b..5a5d112 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1188,9 +1188,7 @@ virCgroupKillPainfully;
virCgroupKillRecursive;
virCgroupMoveTask;
virCgroupNewDetect;
-virCgroupNewDomainDriver;
virCgroupNewDomainPartition;
-virCgroupNewDriver;
virCgroupNewEmulator;
virCgroupNewIgnoreError;
virCgroupNewPartition;
diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c
index 1043318..593caad 100644
--- a/src/util/vircgroup.c
+++ b/src/util/vircgroup.c
@@ -918,28 +918,6 @@ error:
return -1;
}
-
-static int virCgroupAppRoot(virCgroupPtr *group,
-bool create,
-int controllers)
-{
-virCgroupPtr selfgrp = NULL;
-int ret = -1;
-
-if (virCgroupNewSelf(selfgrp) 0)
-return -1;
-
-if (virCgroupNew(-1, libvirt, selfgrp, controllers, group) 0)
-goto cleanup;
-
-if (virCgroupMakeGroup(selfgrp, *group, create, VIR_CGROUP_NONE) 0)
-goto cleanup;
-
-ret = 0;
-cleanup:
-virCgroupFree(selfgrp);
-return ret;
-}
#endif
#if defined _DIRENT_HAVE_D_TYPE
@@ -1387,53 +1365,6 @@ int virCgroupNewPartition(const char *path
ATTRIBUTE_UNUSED,
}
#endif
-/**
- * virCgroupNewDriver:
- *
- * @name: name of this driver (e.g., xen, qemu, lxc)
- * @group: Pointer to returned virCgroupPtr
- *
- * Returns 0 on success, or -1 on error
- */
-#if defined HAVE_MNTENT_H defined HAVE_GETMNTENT_R
-int virCgroupNewDriver(const char *name,
- bool create,
- int controllers,
- virCgroupPtr *group)
-{
-int ret = -1;
-virCgroupPtr rootgrp = NULL;
-
-if (virCgroupAppRoot(rootgrp,
- create, controllers) 0)
-goto cleanup;
-
-if (virCgroupNew(-1, name, rootgrp, -1, group) 0)
-goto cleanup;
-
-if (virCgroupMakeGroup(rootgrp, *group, create, VIR_CGROUP_NONE) 0) {
-virCgroupRemove(*group);
-virCgroupFree(group);
-goto cleanup;
-}
-
-ret = 0;
-
-cleanup:
-virCgroupFree(rootgrp);
-return ret;
-}
-#else
-int virCgroupNewDriver(const char *name ATTRIBUTE_UNUSED,
- bool create ATTRIBUTE_UNUSED,
- int controllers ATTRIBUTE_UNUSED,
- virCgroupPtr *group ATTRIBUTE_UNUSED)
-{
-virReportSystemError(ENXIO, %s,
- _(Control groups not supported on this platform));
-return -1;
-}
-#endif
/**
* virCgroupNewSelf:
@@ -1452,58 +1383,6 @@ int virCgroupNewSelf(virCgroupPtr *group)
/**
- * virCgroupNewDomainDriver:
- *
- * @driver: group for driver owning the domain
- * @name: name of the domain
- * @group: Pointer to returned virCgroupPtr
- *
- * Returns 0 on success, or -1 on error
- */
-#if defined HAVE_MNTENT_H defined HAVE_GETMNTENT_R
-int virCgroupNewDomainDriver(virCgroupPtr driver,
- const char *name,
- bool create,
- virCgroupPtr *group)
-{
-int ret = -1;
-
-if (virCgroupNew(-1, name, driver, -1, group) 0)
-goto cleanup;
-
-/*
- * Create a cgroup with memory.use_hierarchy enabled to
- * surely account memory usage of lxc with ns subsystem
- * enabled. (To be exact, memory and ns subsystems are
- * enabled at the same time.)
- *
- * The reason why doing it here, not a upper group, say
- * a group for driver, is to avoid overhead to track
- * cumulative usage that we don't need.
- */
-if (virCgroupMakeGroup(driver, *group, create, VIR_CGROUP_MEM_HIERACHY)
0) {
-virCgroupRemove(*group);
-virCgroupFree(group);
-goto cleanup;
-}
-
-ret = 0;
-cleanup:
-return ret;
-}
-#else
-int virCgroupNewDomainDriver(virCgroupPtr driver ATTRIBUTE_UNUSED,
- const char *name ATTRIBUTE_UNUSED,
- bool create ATTRIBUTE_UNUSED,
- virCgroupPtr *group ATTRIBUTE_UNUSED)
-{
-virReportSystemError(ENXIO, %s,
- _(Control groups not supported on this platform));
-return -1;
-}
-#endif
-
-/**
* virCgroupNewDomainPartition:
*
* @partition: partition holding the domain
diff --git a/src/util/vircgroup.h b/src/util/vircgroup.h
index 9bf0d7e..3c05604 100644
--- a/src/util/vircgroup.h
+++
Re: [libvirt] [PATCH 4/5] qemu: set/validate slot/connection type when assigning slots for PCI devices
On Tue, Jul 23, 2013 at 10:44:54AM -0400, Laine Stump wrote:
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 059aa6a..64787b6 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -1412,7 +1412,15 @@ cleanup:
#define QEMU_PCI_ADDRESS_FUNCTION_LAST 7
typedef struct {
-/* Each bit in a slot represents one function on that slot */
+virDomainControllerModelPCI model;
+/* flags an min/max can be computed from model, but
+ * having them ready makes life easier.
+ */
+qemuDomainPCIConnectFlags flags;
+size_t minSlot, maxSlot; /* usually 0,0 or 1,31 */
+/* Each bit in a slot represents one function on that slot. If the
+ * bit is set, that function is in use by a device.
+ */
uint8_t slots[QEMU_PCI_ADDRESS_SLOT_LAST + 1];
} qemuDomainPCIAddressBus;
typedef qemuDomainPCIAddressBus *qemuDomainPCIAddressBusPtr;
@@ -1430,9 +1438,13 @@ struct _qemuDomainPCIAddressSet {
* Check that the PCI address is valid for use
* with the specified PCI address set.
*/
-static bool qemuPCIAddressValidate(qemuDomainPCIAddressSetPtr addrs
ATTRIBUTE_UNUSED,
- virDevicePCIAddressPtr addr)
+static bool
+qemuPCIAddressValidate(qemuDomainPCIAddressSetPtr addrs ATTRIBUTE_UNUSED,
+ virDevicePCIAddressPtr addr,
+ qemuDomainPCIConnectFlags flags)
{
+qemuDomainPCIAddressBusPtr bus;
+
if (addrs-nbuses == 0) {
virReportError(VIR_ERR_XML_ERROR, %s, _(No PCI buses available));
return false;
@@ -1448,32 +1460,81 @@ static bool
qemuPCIAddressValidate(qemuDomainPCIAddressSetPtr addrs ATTRIBUTE_UN
addrs-nbuses - 1);
return false;
}
-if (addr-function QEMU_PCI_ADDRESS_FUNCTION_LAST) {
+
+bus = addrs-buses[addr-bus];
+
+/* assure that at least one of the requested connection types is
+ * provided by this bus
+ */
+if (!(flags bus-flags QEMU_PCI_CONNECT_TYPES_MASK)) {
virReportError(VIR_ERR_XML_ERROR,
- _(Invalid PCI address: function must be = %u),
- QEMU_PCI_ADDRESS_FUNCTION_LAST);
+ _(Invalid PCI address: The PCI controller
+ providing bus %04x doesn't support
+ connections appropriate for the device
+ (%x required vs. %x provided by bus)),
+ addr-bus, flags QEMU_PCI_CONNECT_TYPES_MASK,
+ bus-flags QEMU_PCI_CONNECT_TYPES_MASK);
return false;
}
-if (addr-slot QEMU_PCI_ADDRESS_SLOT_LAST) {
+/* make sure this bus allows hot-plug if the caller demands it */
+if ((flags QEMU_PCI_CONNECT_HOTPLUGGABLE)
+!(bus-flags QEMU_PCI_CONNECT_HOTPLUGGABLE)) {
virReportError(VIR_ERR_XML_ERROR,
- _(Invalid PCI address: slot must be = %u),
- QEMU_PCI_ADDRESS_SLOT_LAST);
+ _(Invalid PCI address: hot-pluggable slot requested,
+ but bus %04x doesn't support hot-plug),
addr-bus);
return false;
}
-if (addr-slot == 0) {
-if (addr-bus) {
-virReportError(VIR_ERR_XML_ERROR, %s,
- _(Slot 0 is unusable on PCI bridges));
-} else {
-virReportError(VIR_ERR_XML_ERROR, %s,
- _(Slot 0 on bus 0 is reserved for the host
bridge));
-}
+/* some buses are really just a single port */
+if (bus-minSlot addr-slot bus-minSlot) {
+virReportError(VIR_ERR_XML_ERROR,
+ _(Invalid PCI address: slot must be = %zu),
+ bus-minSlot);
+return false;
+}
+if (addr-slot bus-maxSlot) {
+virReportError(VIR_ERR_XML_ERROR,
+ _(Invalid PCI address: slot must be = %zu),
+ bus-maxSlot);
+return false;
+}
+if (addr-function QEMU_PCI_ADDRESS_FUNCTION_LAST) {
+virReportError(VIR_ERR_XML_ERROR,
+ _(Invalid PCI address: function must be = %u),
+ QEMU_PCI_ADDRESS_FUNCTION_LAST);
+
+static int
+qemuDomainPCIAddressBusSetModel(qemuDomainPCIAddressBusPtr bus,
+virDomainControllerModelPCI model)
+{
+switch (model) {
+case VIR_DOMAIN_CONTROLLER_MODEL_PCI_BRIDGE:
+case VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT:
+bus-flags = (QEMU_PCI_CONNECT_HOTPLUGGABLE |
+ QEMU_PCI_CONNECT_TYPE_PCI);
+bus-minSlot = 1;
+bus-maxSlot = QEMU_PCI_ADDRESS_SLOT_LAST;
+break;
+default:
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ _(Invalid PCI controller model %d), model);
+return -1;
+}
+
+
Re: [libvirt] [PATCH 5/5] conf: add pcie-root controller
On Tue, Jul 23, 2013 at 10:44:55AM -0400, Laine Stump wrote: This controller is implicit on q35 machinetypes. It provides 31 PCIe (*not* PCI) slots as controller 0. Currently there are no devices that can connect to pcie-root. For a usable q35 system, we still need to add a dmi-to-pci-bridge pci controller, which can connect to pcie-root, and provides pci slots. This patch still requires a test case, which willbe coming up, but I wanted to include it along with the previous patch to show that it's simpler to add new controller types now. --- docs/formatdomain.html.in | 17 ++--- src/conf/domain_conf.c| 4 +++- src/conf/domain_conf.h| 1 + src/qemu/qemu_command.c | 17 + src/qemu/qemu_command.h | 2 ++ src/qemu/qemu_domain.c| 23 +-- 6 files changed, 50 insertions(+), 14 deletions(-) ACK once test case(s) are added. 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 v3 4/7] qemu: Translate the iscsi pool/volume disk source
Il 23/07/2013 16:14, Daniel P. Berrange ha scritto: Perhaps the default could be specified in a configuration file (and the default should be the safe one). No, that is even worse because now the default is not predictable.. We simply default to host mode and if applications want to use the other mode they can configure the XML as desired. Can we just forbid mode='default' for iSCSI and force the user to specify host vs. direct? At least the error message will be nice and independent of the QEMU version. Paolo -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v5 4/5] storage: Support chap authentication for iscsi pool
On 07/23/2013 10:56 AM, Daniel P. Berrange wrote:
...snip...
+++ b/src/storage/storage_backend_iscsi.c
@@ -714,7 +714,8 @@ virStorageBackendISCSISetAuth(const char *portal,
if (!conn) {
virReportError(VIR_ERR_INTERNAL_ERROR, %s,
- _(iscsi 'chap' authentication requires
connection));
+ _(iscsi 'chap' authentication not supported
+ for autostarted pools));
return -1;
}
I noticed that the nwfilter already unconditionally calls
virConnectOpen(qemu://system); so we're already in fact suffering from the
problem with
autostart having a qemu dependency.
Given this, I'd support a patch which simply did
conn = virConnectOpen(privilege ? qemu:///system : qemu:///session);
in storageDriverAutostart, provided that we ignore any errors from
virConnectOpen, and fallback to use NULL for the connection in that
case.
Obviously this is something we'll still need to fix properly in a
future release, but at least it'll make autostart of storage pools
with auth work in the common case in the short term for this release.
Regards,
Daniel
So resurrect the following:
https://www.redhat.com/archives/libvir-list/2013-July/msg00962.html
John
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/3] domain_event: Resolve memory leak found by Valgrind
Commit id '4421e257' strdup'd devAlias, but didn't free Running qemuhotplugtest under valgrind resulted in the following: ==7375== 9 bytes in 1 blocks are definitely lost in loss record 11 of 70 ==7375==at 0x4A0887C: malloc (vg_replace_malloc.c:270) ==7375==by 0x37C1085D71: strdup (strdup.c:42) ==7375==by 0x4CBBD5F: virStrdup (virstring.c:554) ==7375==by 0x4CFF9CB: virDomainEventDeviceRemovedNew (domain_event.c:1174) ==7375==by 0x427791: qemuDomainRemoveChrDevice (qemu_hotplug.c:2508) ==7375==by 0x42C65D: qemuDomainDetachChrDevice (qemu_hotplug.c:3357) ==7375==by 0x41C94F: testQemuHotplug (qemuhotplugtest.c:115) ==7375==by 0x41D817: virtTestRun (testutils.c:168) ==7375==by 0x41C400: mymain (qemuhotplugtest.c:322) ==7375==by 0x41DF3A: virtTestMain (testutils.c:764) ==7375==by 0x37C1021A04: (below main) (libc-start.c:225) --- src/conf/domain_event.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/conf/domain_event.c b/src/conf/domain_event.c index 640463c..16ae92b 100644 --- a/src/conf/domain_event.c +++ b/src/conf/domain_event.c @@ -520,6 +520,9 @@ void virDomainEventFree(virDomainEventPtr event) case VIR_DOMAIN_EVENT_ID_TRAY_CHANGE: VIR_FREE(event-data.trayChange.devAlias); break; +case VIR_DOMAIN_EVENT_ID_DEVICE_REMOVED: +VIR_FREE(event-data.deviceRemoved.devAlias); +break; } VIR_FREE(event-dom.name); -- 1.8.1.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/3] lxc: Resolve Coverity warning
Commit 'c8695053' resulted in the following:
Coverity error seen in the output:
ERROR: REVERSE_INULL
FUNCTION: lxcProcessAutoDestroy
Due to the 'dom' being checked before 'dom-persistent' since 'dom'
is already dereferenced prior to that.
---
src/lxc/lxc_process.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/lxc/lxc_process.c b/src/lxc/lxc_process.c
index 5b83ccb..5c4f8c8 100644
--- a/src/lxc/lxc_process.c
+++ b/src/lxc/lxc_process.c
@@ -74,7 +74,7 @@ lxcProcessAutoDestroy(virDomainObjPtr dom,
VIR_DOMAIN_EVENT_STOPPED_DESTROYED);
priv-doneStopEvent = true;
-if (dom !dom-persistent) {
+if (!dom-persistent) {
virDomainObjListRemove(driver-domains, dom);
dom = NULL;
}
--
1.8.1.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/3] Miscellaneous Coverity Valgrind changes
These patches resolve a Coverity warning and a found Valgrind leak. Added more patterns to the .valgrind.supp file to suppress the commandtests and seclabeltest output. This will thus result in a mostly clean valgrind run. All that remains is a hotplugtest failure, which some patches were posted: https://www.redhat.com/archives/libvir-list/2013-July/msg01156.html and https://www.redhat.com/archives/libvir-list/2013-July/msg01158.html but did not completely resolve the failures. John Ferlan (3): lxc: Resolve Coverity warning domain_event: Resolve memory leak found by Valgrind valgrind.supp: Add more valgrind suppression paths src/conf/domain_event.c | 3 +++ src/lxc/lxc_process.c | 2 +- tests/.valgrind.supp| 60 + 3 files changed, 64 insertions(+), 1 deletion(-) -- 1.8.1.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3 4/7] qemu: Translate the iscsi pool/volume disk source
On Tue, Jul 23, 2013 at 05:35:57PM +0200, Paolo Bonzini wrote: Il 23/07/2013 16:14, Daniel P. Berrange ha scritto: Perhaps the default could be specified in a configuration file (and the default should be the safe one). No, that is even worse because now the default is not predictable.. We simply default to host mode and if applications want to use the other mode they can configure the XML as desired. Can we just forbid mode='default' for iSCSI and force the user to specify host vs. direct? That would mean that apps cannot simply configure a guest volume without first checking to find out what type of pool it is, and then specifying this extra arg for iSCSI. IMHO the value of the volume XML is that you don't have to know anything about the pool to be able to configure it - we're completely decoupled. 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/3] domain_event: Resolve memory leak found by Valgrind
On 07/23/13 17:59, John Ferlan wrote: Commit id '4421e257' strdup'd devAlias, but didn't free Running qemuhotplugtest under valgrind resulted in the following: ==7375== 9 bytes in 1 blocks are definitely lost in loss record 11 of 70 ==7375==at 0x4A0887C: malloc (vg_replace_malloc.c:270) ==7375==by 0x37C1085D71: strdup (strdup.c:42) ==7375==by 0x4CBBD5F: virStrdup (virstring.c:554) ==7375==by 0x4CFF9CB: virDomainEventDeviceRemovedNew (domain_event.c:1174) ==7375==by 0x427791: qemuDomainRemoveChrDevice (qemu_hotplug.c:2508) ==7375==by 0x42C65D: qemuDomainDetachChrDevice (qemu_hotplug.c:3357) ==7375==by 0x41C94F: testQemuHotplug (qemuhotplugtest.c:115) ==7375==by 0x41D817: virtTestRun (testutils.c:168) ==7375==by 0x41C400: mymain (qemuhotplugtest.c:322) ==7375==by 0x41DF3A: virtTestMain (testutils.c:764) ==7375==by 0x37C1021A04: (below main) (libc-start.c:225) --- src/conf/domain_event.c | 3 +++ 1 file changed, 3 insertions(+) ACK. Peter -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v5 4/5] storage: Support chap authentication for iscsi pool
On Tue, Jul 23, 2013 at 11:50:57AM -0400, John Ferlan wrote:
On 07/23/2013 10:56 AM, Daniel P. Berrange wrote:
...snip...
+++ b/src/storage/storage_backend_iscsi.c
@@ -714,7 +714,8 @@ virStorageBackendISCSISetAuth(const char *portal,
if (!conn) {
virReportError(VIR_ERR_INTERNAL_ERROR, %s,
- _(iscsi 'chap' authentication requires
connection));
+ _(iscsi 'chap' authentication not supported
+ for autostarted pools));
return -1;
}
I noticed that the nwfilter already unconditionally calls
virConnectOpen(qemu://system); so we're already in fact suffering from
the problem with
autostart having a qemu dependency.
Given this, I'd support a patch which simply did
conn = virConnectOpen(privilege ? qemu:///system : qemu:///session);
in storageDriverAutostart, provided that we ignore any errors from
virConnectOpen, and fallback to use NULL for the connection in that
case.
Obviously this is something we'll still need to fix properly in a
future release, but at least it'll make autostart of storage pools
with auth work in the common case in the short term for this release.
Regards,
Daniel
So resurrect the following:
https://www.redhat.com/archives/libvir-list/2013-July/msg00962.html
Yep, ACK to that patch, but add XXX Remove hardcoding of QEMU URI
as a comment just before the virConnectOpen call.
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 1/3] lxc: Resolve Coverity warning
On 07/23/13 17:59, John Ferlan wrote:
Commit 'c8695053' resulted in the following:
Coverity error seen in the output:
ERROR: REVERSE_INULL
FUNCTION: lxcProcessAutoDestroy
Due to the 'dom' being checked before 'dom-persistent' since 'dom'
is already dereferenced prior to that.
---
src/lxc/lxc_process.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/lxc/lxc_process.c b/src/lxc/lxc_process.c
index 5b83ccb..5c4f8c8 100644
--- a/src/lxc/lxc_process.c
+++ b/src/lxc/lxc_process.c
@@ -74,7 +74,7 @@ lxcProcessAutoDestroy(virDomainObjPtr dom,
VIR_DOMAIN_EVENT_STOPPED_DESTROYED);
priv-doneStopEvent = true;
-if (dom !dom-persistent) {
+if (!dom-persistent) {
virDomainObjListRemove(driver-domains, dom);
dom = NULL;
}
ACK, dom is indeed dereferenced before, thus it will not crash here.
Peter
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/7] Probe QEMU binary for host CPU and use it for computations
Since QEMU and kvm may filter some host CPU features or add efficiently emulated features, asking QEMU binary for host CPU data provides better results when we later use the data for building guest CPUs. Jiri Denemark (7): cpu: Add support for loading and storing CPU data cpu: Export few x86-specific APIs x86: Ignore CPUID functions greater than 10 qemu: Add monitor APIs to fetch CPUID data from QEMU qemu: Make QMP probing process reusable qemu: Probe QEMU binary for host CPU qemu: Use host CPU from QEMU for computations src/cpu/cpu.c | 41 src/cpu/cpu.h | 13 ++ src/cpu/cpu_x86.c | 161 +++--- src/cpu/cpu_x86.h | 10 + src/cpu/cpu_x86_data.h | 1 + src/libvirt_private.syms | 9 + src/qemu/qemu_capabilities.c | 234 ++--- src/qemu/qemu_capabilities.h | 2 + src/qemu/qemu_command.c| 32 ++- src/qemu/qemu_domain.c | 21 +- src/qemu/qemu_monitor.c| 21 ++ src/qemu/qemu_monitor.h| 3 + src/qemu/qemu_monitor_json.c | 162 ++ src/qemu/qemu_monitor_json.h | 6 + tests/Makefile.am | 1 + .../qemumonitorjson-getcpu-empty.data | 2 + .../qemumonitorjson-getcpu-empty.json | 46 .../qemumonitorjson-getcpu-filtered.data | 4 + .../qemumonitorjson-getcpu-filtered.json | 46 .../qemumonitorjson-getcpu-full.data | 4 + .../qemumonitorjson-getcpu-full.json | 46 .../qemumonitorjson-getcpu-host.data | 5 + .../qemumonitorjson-getcpu-host.json | 45 tests/qemumonitorjsontest.c| 74 +++ 24 files changed, 881 insertions(+), 108 deletions(-) create mode 100644 tests/qemumonitorjsondata/qemumonitorjson-getcpu-empty.data create mode 100644 tests/qemumonitorjsondata/qemumonitorjson-getcpu-empty.json create mode 100644 tests/qemumonitorjsondata/qemumonitorjson-getcpu-filtered.data create mode 100644 tests/qemumonitorjsondata/qemumonitorjson-getcpu-filtered.json create mode 100644 tests/qemumonitorjsondata/qemumonitorjson-getcpu-full.data create mode 100644 tests/qemumonitorjsondata/qemumonitorjson-getcpu-full.json create mode 100644 tests/qemumonitorjsondata/qemumonitorjson-getcpu-host.data create mode 100644 tests/qemumonitorjsondata/qemumonitorjson-getcpu-host.json -- 1.8.3.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 6/7] qemu: Probe QEMU binary for host CPU
Since QEMU and kvm may filter some host CPU features or add efficiently
emulated features, asking QEMU binary for host CPU data provides
better results when we later use the data for building guest CPUs.
---
src/qemu/qemu_capabilities.c | 44 +++-
src/qemu/qemu_capabilities.h | 2 ++
2 files changed, 45 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 9440396..d46a059 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -253,6 +253,7 @@ struct _virQEMUCaps {
size_t ncpuDefinitions;
char **cpuDefinitions;
+virCPUDefPtr hostCPU;
size_t nmachineTypes;
char **machineTypes;
@@ -1757,6 +1758,9 @@ virQEMUCapsPtr virQEMUCapsNewCopy(virQEMUCapsPtr qemuCaps)
goto error;
}
+if (!(ret-hostCPU = virCPUDefCopy(qemuCaps-hostCPU)))
+goto error;
+
if (VIR_ALLOC_N(ret-machineTypes, qemuCaps-nmachineTypes) 0)
goto error;
if (VIR_ALLOC_N(ret-machineAliases, qemuCaps-nmachineTypes) 0)
@@ -1796,6 +1800,7 @@ void virQEMUCapsDispose(void *obj)
VIR_FREE(qemuCaps-cpuDefinitions[i]);
}
VIR_FREE(qemuCaps-cpuDefinitions);
+virCPUDefFree(qemuCaps-hostCPU);
virBitmapFree(qemuCaps-flags);
@@ -2485,7 +2490,6 @@ virQEMUCapsInitQMPCommandNew(const char *binary,
-no-user-config,
-nodefaults,
-nographic,
- -M, none,
-qmp, monitor,
-pidfile, pidfile,
-daemonize,
@@ -2617,6 +2621,7 @@ virQEMUCapsInitQMP(virQEMUCapsPtr qemuCaps,
cmd = virQEMUCapsInitQMPCommandNew(qemuCaps-binary, monarg, pidfile,
runUid, runGid);
+virCommandAddArgList(cmd, -M, none, NULL);
if ((ret = virQEMUCapsInitQMPCommandRun(cmd, qemuCaps-binary, pidfile,
config, mon, pid)) 0) {
@@ -2679,6 +2684,37 @@ virQEMUCapsInitQMP(virQEMUCapsPtr qemuCaps,
if (virQEMUCapsProbeQMPCommandLine(qemuCaps, mon) 0)
goto cleanup;
+if ((qemuCaps-arch == VIR_ARCH_I686 ||
+ qemuCaps-arch == VIR_ARCH_X86_64)
+(virQEMUCapsGet(qemuCaps, QEMU_CAPS_KVM) ||
+ virQEMUCapsGet(qemuCaps, QEMU_CAPS_ENABLE_KVM))
+virQEMUCapsGet(qemuCaps, QEMU_CAPS_CPU_HOST)
+qemuCaps-nmachineTypes) {
+virQEMUCapsInitQMPCommandAbort(cmd, mon, pid, pidfile);
+
+VIR_DEBUG(Checking host CPU data provided by %s, qemuCaps-binary);
+cmd = virQEMUCapsInitQMPCommandNew(qemuCaps-binary, monarg, pidfile,
+ runUid, runGid);
+virCommandAddArgList(cmd, -cpu, host, NULL);
+/* -cpu host gives the same CPU for all machine types so we just
+ * use the first one when probing
+ */
+virCommandAddArg(cmd, -machine);
+virCommandAddArgFormat(cmd, %s,accel=kvm,
+ qemuCaps-machineTypes[0]);
+
+if (virQEMUCapsInitQMPCommandRun(cmd, qemuCaps-binary, pidfile,
+ config, mon, pid) 0)
+goto cleanup;
+
+qemuCaps-hostCPU = qemuMonitorGetCPU(mon, qemuCaps-arch);
+if (qemuCaps-hostCPU) {
+char *cpu = virCPUDefFormat(qemuCaps-hostCPU, 0);
+VIR_DEBUG(Host CPU reported by %s: %s, qemuCaps-binary, cpu);
+VIR_FREE(cpu);
+}
+}
+
ret = 0;
cleanup:
@@ -2858,3 +2894,9 @@ virQEMUCapsUsedQMP(virQEMUCapsPtr qemuCaps)
{
return qemuCaps-usedQMP;
}
+
+virCPUDefPtr
+virQEMUCapsGetHostCPU(virQEMUCapsPtr qemuCaps)
+{
+return qemuCaps-hostCPU;
+}
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index f5f685d..e7774a3 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -272,4 +272,6 @@ int virQEMUCapsParseDeviceStr(virQEMUCapsPtr qemuCaps,
const char *str);
VIR_ENUM_DECL(virQEMUCaps);
bool virQEMUCapsUsedQMP(virQEMUCapsPtr qemuCaps);
+virCPUDefPtr virQEMUCapsGetHostCPU(virQEMUCapsPtr qemuCaps);
+
#endif /* __QEMU_CAPABILITIES_H__*/
--
1.8.3.2
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/7] cpu: Add support for loading and storing CPU data
---
src/cpu/cpu.c| 41 ++
src/cpu/cpu.h| 13 +
src/cpu/cpu_x86.c| 135 +++
src/libvirt_private.syms | 2 +
4 files changed, 170 insertions(+), 21 deletions(-)
diff --git a/src/cpu/cpu.c b/src/cpu/cpu.c
index 4124354..8bd689e 100644
--- a/src/cpu/cpu.c
+++ b/src/cpu/cpu.c
@@ -438,6 +438,47 @@ cpuHasFeature(const virCPUDataPtr data,
return driver-hasFeature(data, feature);
}
+char *
+cpuDataFormat(const virCPUDataPtr data)
+{
+struct cpuArchDriver *driver;
+
+VIR_DEBUG(data=%p, data);
+
+if (!(driver = cpuGetSubDriver(data-arch)))
+return NULL;
+
+if (!driver-dataFormat) {
+virReportError(VIR_ERR_NO_SUPPORT,
+ _(cannot format %s CPU data),
+ virArchToString(data-arch));
+return NULL;
+}
+
+return driver-dataFormat(data);
+}
+
+virCPUDataPtr
+cpuDataParse(virArch arch,
+ const char *xmlStr)
+{
+struct cpuArchDriver *driver;
+
+VIR_DEBUG(arch=%s, xmlStr=%s, virArchToString(arch), xmlStr);
+
+if (!(driver = cpuGetSubDriver(arch)))
+return NULL;
+
+if (!driver-dataParse) {
+virReportError(VIR_ERR_NO_SUPPORT,
+ _(cannot parse %s CPU data),
+ virArchToString(arch));
+return NULL;
+}
+
+return driver-dataParse(xmlStr);
+}
+
bool
cpuModelIsAllowed(const char *model,
const char **models,
diff --git a/src/cpu/cpu.h b/src/cpu/cpu.h
index 4003435..e1473fe 100644
--- a/src/cpu/cpu.h
+++ b/src/cpu/cpu.h
@@ -91,6 +91,11 @@ typedef int
(*cpuArchHasFeature) (const virCPUDataPtr data,
const char *feature);
+typedef char *
+(*cpuArchDataFormat)(const virCPUDataPtr data);
+
+typedef virCPUDataPtr
+(*cpuArchDataParse) (const char *xmlStr);
struct cpuArchDriver {
const char *name;
@@ -105,6 +110,8 @@ struct cpuArchDriver {
cpuArchBaseline baseline;
cpuArchUpdate update;
cpuArchHasFeaturehasFeature;
+cpuArchDataFormat dataFormat;
+cpuArchDataParsedataParse;
};
@@ -165,6 +172,12 @@ extern int
cpuHasFeature(const virCPUDataPtr data,
const char *feature);
+char *
+cpuDataFormat(const virCPUDataPtr data);
+
+virCPUDataPtr
+cpuDataParse(virArch arch,
+ const char *xmlStr);
bool
cpuModelIsAllowed(const char *model,
diff --git a/src/cpu/cpu_x86.c b/src/cpu/cpu_x86.c
index a388f0f..560a2a9 100644
--- a/src/cpu/cpu_x86.c
+++ b/src/cpu/cpu_x86.c
@@ -666,12 +666,42 @@ x86FeatureNames(const struct x86_map *map,
static int
+x86ParseCPUID(xmlXPathContextPtr ctxt,
+ struct cpuX86cpuid *cpuid)
+{
+unsigned long fun, eax, ebx, ecx, edx;
+int ret_fun, ret_eax, ret_ebx, ret_ecx, ret_edx;
+
+memset(cpuid, 0, sizeof(*cpuid));
+
+fun = eax = ebx = ecx = edx = 0;
+ret_fun = virXPathULongHex(string(@function), ctxt, fun);
+ret_eax = virXPathULongHex(string(@eax), ctxt, eax);
+ret_ebx = virXPathULongHex(string(@ebx), ctxt, ebx);
+ret_ecx = virXPathULongHex(string(@ecx), ctxt, ecx);
+ret_edx = virXPathULongHex(string(@edx), ctxt, edx);
+
+if (ret_fun 0 || ret_eax == -2 || ret_ebx == -2
+|| ret_ecx == -2 || ret_edx == -2)
+return -1;
+
+cpuid-function = fun;
+cpuid-eax = eax;
+cpuid-ebx = ebx;
+cpuid-ecx = ecx;
+cpuid-edx = edx;
+return 0;
+}
+
+
+static int
x86FeatureLoad(xmlXPathContextPtr ctxt,
struct x86_map *map)
{
xmlNodePtr *nodes = NULL;
xmlNodePtr ctxt_node = ctxt-node;
struct x86_feature *feature;
+struct cpuX86cpuid cpuid;
int ret = 0;
size_t i;
int n;
@@ -697,31 +727,13 @@ x86FeatureLoad(xmlXPathContextPtr ctxt,
goto ignore;
for (i = 0; i n; i++) {
-struct cpuX86cpuid cpuid;
-unsigned long fun, eax, ebx, ecx, edx;
-int ret_fun, ret_eax, ret_ebx, ret_ecx, ret_edx;
-
ctxt-node = nodes[i];
-fun = eax = ebx = ecx = edx = 0;
-ret_fun = virXPathULongHex(string(@function), ctxt, fun);
-ret_eax = virXPathULongHex(string(@eax), ctxt, eax);
-ret_ebx = virXPathULongHex(string(@ebx), ctxt, ebx);
-ret_ecx = virXPathULongHex(string(@ecx), ctxt, ecx);
-ret_edx = virXPathULongHex(string(@edx), ctxt, edx);
-
-if (ret_fun 0 || ret_eax == -2 || ret_ebx == -2
-|| ret_ecx == -2 || ret_edx == -2) {
+if (x86ParseCPUID(ctxt, cpuid) 0) {
virReportError(VIR_ERR_INTERNAL_ERROR,
- _(Invalid cpuid[%zu] in %s feature), i,
feature-name);
+ _(Invalid cpuid[%zu] in %s feature),
+ i, feature-name);
goto ignore;
}
-
-cpuid.function = fun;
-cpuid.eax = eax;
-cpuid.ebx = ebx;
-cpuid.ecx = ecx;
-cpuid.edx = edx;
-
if
[libvirt] [PATCH 4/7] qemu: Add monitor APIs to fetch CPUID data from QEMU
---
src/qemu/qemu_monitor.c| 21 +++
src/qemu/qemu_monitor.h| 3 +
src/qemu/qemu_monitor_json.c | 162 +
src/qemu/qemu_monitor_json.h | 6 +
tests/Makefile.am | 1 +
.../qemumonitorjson-getcpu-empty.data | 2 +
.../qemumonitorjson-getcpu-empty.json | 46 ++
.../qemumonitorjson-getcpu-filtered.data | 4 +
.../qemumonitorjson-getcpu-filtered.json | 46 ++
.../qemumonitorjson-getcpu-full.data | 4 +
.../qemumonitorjson-getcpu-full.json | 46 ++
.../qemumonitorjson-getcpu-host.data | 5 +
.../qemumonitorjson-getcpu-host.json | 45 ++
tests/qemumonitorjsontest.c| 74 ++
14 files changed, 465 insertions(+)
create mode 100644 tests/qemumonitorjsondata/qemumonitorjson-getcpu-empty.data
create mode 100644 tests/qemumonitorjsondata/qemumonitorjson-getcpu-empty.json
create mode 100644
tests/qemumonitorjsondata/qemumonitorjson-getcpu-filtered.data
create mode 100644
tests/qemumonitorjsondata/qemumonitorjson-getcpu-filtered.json
create mode 100644 tests/qemumonitorjsondata/qemumonitorjson-getcpu-full.data
create mode 100644 tests/qemumonitorjsondata/qemumonitorjson-getcpu-full.json
create mode 100644 tests/qemumonitorjsondata/qemumonitorjson-getcpu-host.data
create mode 100644 tests/qemumonitorjsondata/qemumonitorjson-getcpu-host.json
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index 0b73411..695cf19 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -3839,3 +3839,24 @@ qemuMonitorGetDeviceAliases(qemuMonitorPtr mon,
return qemuMonitorJSONGetDeviceAliases(mon, aliases);
}
+
+virCPUDefPtr
+qemuMonitorGetCPU(qemuMonitorPtr mon,
+ virArch arch)
+{
+VIR_DEBUG(mon=%p, arch=%s, mon, virArchToString(arch));
+
+if (!mon) {
+virReportError(VIR_ERR_INVALID_ARG, %s,
+ _(monitor must not be NULL));
+return NULL;
+}
+
+if (!mon-json) {
+virReportError(VIR_ERR_OPERATION_UNSUPPORTED, %s,
+ _(JSON monitor is required));
+return NULL;
+}
+
+return qemuMonitorJSONGetCPU(mon, arch);
+}
diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
index 4a55501..8a312bd 100644
--- a/src/qemu/qemu_monitor.h
+++ b/src/qemu/qemu_monitor.h
@@ -717,6 +717,9 @@ int qemuMonitorDetachCharDev(qemuMonitorPtr mon,
int qemuMonitorGetDeviceAliases(qemuMonitorPtr mon,
char ***aliases);
+virCPUDefPtr qemuMonitorGetCPU(qemuMonitorPtr mon,
+ virArch arch);
+
/**
* When running two dd process and using redirection, we need a
* shell that will not truncate files. These two strings serve that
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index 12f7e69..617bfdf 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -42,6 +42,7 @@
#include virerror.h
#include virjson.h
#include virstring.h
+#include cpu/cpu_x86.h
#ifdef WITH_DTRACE_PROBES
# include libvirt_qemu_probes.h
@@ -49,6 +50,7 @@
#define VIR_FROM_THIS VIR_FROM_QEMU
+#define QOM_CPU_PATH /machine/unattached/device[0]
#define LINE_ENDING \r\n
@@ -5453,3 +5455,163 @@ cleanup:
VIR_FREE(paths);
return ret;
}
+
+
+static int
+qemuMonitorJSONParseCPUFeatureWord(virJSONValuePtr data,
+ struct cpuX86cpuid *cpuid)
+{
+const char *reg;
+unsigned long long fun;
+unsigned long long features;
+
+memset(cpuid, 0, sizeof(*cpuid));
+
+if (!(reg = virJSONValueObjectGetString(data, cpuid-register))) {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+ _(missing cpuid-register in CPU data));
+return -1;
+}
+if (virJSONValueObjectGetNumberUlong(data, cpuid-input-eax, fun)) {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+ _(missing or invalid cpuid-input-eax in CPU data));
+return -1;
+}
+if (virJSONValueObjectGetNumberUlong(data, features, features) 0) {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+ _(missing or invalid features in CPU data));
+return -1;
+}
+
+cpuid-function = fun;
+if (STREQ(reg, EAX)) {
+cpuid-eax = features;
+} else if (STREQ(reg, EBX)) {
+cpuid-ebx = features;
+} else if (STREQ(reg, ECX)) {
+cpuid-ecx = features;
+} else if (STREQ(reg, EDX)) {
+cpuid-edx = features;
+} else {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ _(unknown CPU register '%s'), reg);
+return -1;
+}
+
+return 0;
+}
+
+virCPUDataPtr
+qemuMonitorJSONGetCPUData(qemuMonitorPtr mon,
+ const
[libvirt] [PATCH 5/7] qemu: Make QMP probing process reusable
---
src/qemu/qemu_capabilities.c | 192 +++
1 file changed, 120 insertions(+), 72 deletions(-)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 5dc3c9e..9440396 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -2464,6 +2464,116 @@ cleanup:
return ret;
}
+static virCommandPtr
+virQEMUCapsInitQMPCommandNew(const char *binary,
+ const char *monitor,
+ const char *pidfile,
+ uid_t runUid,
+ gid_t runGid)
+{
+virCommandPtr cmd;
+
+/*
+ * We explicitly need to use -daemonize here, rather than
+ * virCommandDaemonize, because we need to synchronize
+ * with QEMU creating its monitor socket API. Using
+ * daemonize guarantees control won't return to libvirt
+ * until the socket is present.
+ */
+cmd = virCommandNewArgList(binary,
+ -S,
+ -no-user-config,
+ -nodefaults,
+ -nographic,
+ -M, none,
+ -qmp, monitor,
+ -pidfile, pidfile,
+ -daemonize,
+ NULL);
+virCommandAddEnvPassCommon(cmd);
+virCommandClearCaps(cmd);
+virCommandSetGID(cmd, runGid);
+virCommandSetUID(cmd, runUid);
+return cmd;
+}
+
+static int
+virQEMUCapsInitQMPCommandRun(virCommandPtr cmd,
+ const char *binary,
+ const char *pidfile,
+ virDomainChrSourceDefPtr config,
+ qemuMonitorPtr *mon,
+ pid_t *pid)
+{
+int status = 0;
+virDomainObj vm;
+int ret = -1;
+
+if (virCommandRun(cmd, status) 0) {
+ret = -2;
+goto cleanup;
+}
+
+if (status != 0) {
+VIR_DEBUG(QEMU %s exited with status %d, binary, status);
+goto cleanup;
+}
+
+if (virPidFileReadPath(pidfile, pid) 0) {
+VIR_DEBUG(Failed to read pidfile %s, pidfile);
+goto cleanup;
+}
+
+memset(vm, 0, sizeof(vm));
+vm.pid = *pid;
+
+if (!(*mon = qemuMonitorOpen(vm, config, true, callbacks)))
+goto cleanup;
+
+virObjectLock(*mon);
+
+if (qemuMonitorSetCapabilities(*mon) 0) {
+virErrorPtr err = virGetLastError();
+VIR_DEBUG(Failed to set monitor capabilities %s,
+ err ? err-message : unknown problem);
+goto cleanup;
+}
+
+ret = 0;
+cleanup:
+return ret;
+}
+
+static void
+virQEMUCapsInitQMPCommandAbort(virCommandPtr *cmd,
+ qemuMonitorPtr *mon,
+ pid_t *pid,
+ const char *pidfile)
+{
+if (*mon)
+virObjectUnlock(*mon);
+qemuMonitorClose(*mon);
+*mon = NULL;
+
+virCommandAbort(*cmd);
+virCommandFree(*cmd);
+*cmd = NULL;
+
+if (*pid != 0) {
+char ebuf[1024];
+
+VIR_DEBUG(Killing QMP caps process %lld, (long long) *pid);
+if (virProcessKill(*pid, SIGKILL) 0 errno != ESRCH)
+VIR_ERROR(_(Failed to kill process %lld: %s),
+ (long long) *pid,
+ virStrerror(errno, ebuf, sizeof(ebuf)));
+*pid = 0;
+}
+
+if (pidfile)
+unlink(pidfile);
+}
+
static int
virQEMUCapsInitQMP(virQEMUCapsPtr qemuCaps,
const char *libDir,
@@ -2475,13 +2585,11 @@ virQEMUCapsInitQMP(virQEMUCapsPtr qemuCaps,
qemuMonitorPtr mon = NULL;
int major, minor, micro;
char *package = NULL;
-int status = 0;
virDomainChrSourceDef config;
char *monarg = NULL;
char *monpath = NULL;
char *pidfile = NULL;
pid_t pid = 0;
-virDomainObj vm;
/* the .sock sufix is important to avoid a possible clash with a qemu
* domain called capabilities
@@ -2507,58 +2615,15 @@ virQEMUCapsInitQMP(virQEMUCapsPtr qemuCaps,
VIR_DEBUG(Try to get caps via QMP qemuCaps=%p, qemuCaps);
-/*
- * We explicitly need to use -daemonize here, rather than
- * virCommandDaemonize, because we need to synchronize
- * with QEMU creating its monitor socket API. Using
- * daemonize guarantees control won't return to libvirt
- * until the socket is present.
- */
-cmd = virCommandNewArgList(qemuCaps-binary,
- -S,
- -no-user-config,
- -nodefaults,
- -nographic,
- -M, none,
- -qmp, monarg,
- -pidfile, pidfile,
- -daemonize,
- NULL);
-
[libvirt] [PATCH 2/7] cpu: Export few x86-specific APIs
---
src/cpu/cpu_x86.c| 21 ++---
src/cpu/cpu_x86.h| 10 ++
src/libvirt_private.syms | 7 +++
3 files changed, 35 insertions(+), 3 deletions(-)
diff --git a/src/cpu/cpu_x86.c b/src/cpu/cpu_x86.c
index 560a2a9..dbbcfd2 100644
--- a/src/cpu/cpu_x86.c
+++ b/src/cpu/cpu_x86.c
@@ -203,7 +203,7 @@ x86DataCpuid(const struct cpuX86Data *data,
}
-static void
+void
x86DataFree(struct cpuX86Data *data)
{
if (data == NULL)
@@ -215,7 +215,7 @@ x86DataFree(struct cpuX86Data *data)
}
-static virCPUDataPtr
+virCPUDataPtr
x86MakeCPUData(virArch arch, struct cpuX86Data **data)
{
virCPUDataPtr cpuData;
@@ -295,7 +295,7 @@ x86DataExpand(struct cpuX86Data *data,
}
-static int
+int
x86DataAddCpuid(struct cpuX86Data *data,
const struct cpuX86cpuid *cpuid)
{
@@ -323,6 +323,21 @@ x86DataAddCpuid(struct cpuX86Data *data,
}
+int
+x86DataSetVendor(struct cpuX86Data *data,
+ const char *vendor)
+{
+struct cpuX86cpuid cpuid;
+
+cpuid.function = 0;
+cpuid.ebx = virReadBufInt32LE(vendor);
+cpuid.edx = virReadBufInt32LE(vendor + 4);
+cpuid.ecx = virReadBufInt32LE(vendor + 8);
+
+return x86DataAddCpuid(data, cpuid);
+}
+
+
static int
x86DataAdd(struct cpuX86Data *data1,
const struct cpuX86Data *data2)
diff --git a/src/cpu/cpu_x86.h b/src/cpu/cpu_x86.h
index 77965b7..8235076 100644
--- a/src/cpu/cpu_x86.h
+++ b/src/cpu/cpu_x86.h
@@ -25,7 +25,17 @@
# define __VIR_CPU_X86_H__
# include cpu.h
+# include cpu_x86_data.h
extern struct cpuArchDriver cpuDriverX86;
+int x86DataAddCpuid(struct cpuX86Data *data,
+const struct cpuX86cpuid *cpuid);
+int x86DataSetVendor(struct cpuX86Data *data,
+ const char *vendor);
+
+void x86DataFree(struct cpuX86Data *data);
+
+virCPUDataPtr x86MakeCPUData(virArch arch, struct cpuX86Data **data);
+
#endif /* __VIR_CPU_X86_H__ */
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index e0f3876..7fe2bc0 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -724,6 +724,13 @@ cpuNodeData;
cpuUpdate;
+# cpu/cpu_x86.h
+x86DataAddCpuid;
+x86DataFree;
+x86DataSetVendor;
+x86MakeCPUData;
+
+
# datatypes.h
virConnectClass;
virDomainClass;
--
1.8.3.2
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 7/7] qemu: Use host CPU from QEMU for computations
Use the host CPU data probed by the current emulator when updating a
guest CPU according to a host CPU or when checking whether they are
compatible.
---
src/qemu/qemu_command.c | 32 ++--
src/qemu/qemu_domain.c | 21 +++--
2 files changed, 41 insertions(+), 12 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index d0aed7c..f2124d8 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -5683,13 +5683,19 @@ qemuBuildCpuArgStr(const virQEMUDriverPtr driver,
virBuffer buf = VIR_BUFFER_INITIALIZER;
size_t i;
virCapsPtr caps = NULL;
+bool filteredHost = false;
*hasHwVirt = false;
if (!(caps = virQEMUDriverGetCapabilities(driver, false)))
goto cleanup;
-host = caps-host.cpu;
+if (def-virtType == VIR_DOMAIN_VIRT_KVM)
+host = virQEMUCapsGetHostCPU(qemuCaps);
+if (host)
+filteredHost = true;
+else
+host = caps-host.cpu;
if (def-os.arch == VIR_ARCH_I686)
default_model = qemu32;
@@ -5722,12 +5728,26 @@ qemuBuildCpuArgStr(const virQEMUDriverPtr driver,
switch (cmp) {
case VIR_CPU_COMPARE_INCOMPATIBLE:
if (compare_msg) {
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- _(guest and host CPU are not compatible: %s),
- compare_msg);
+if (filteredHost) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _(requested guest CPU cannot be provided
+ by QEMU binary on this host: %s),
+ compare_msg);
+} else {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _(guest and host CPU are not
+ compatible: %s), compare_msg);
+}
} else {
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
- _(guest CPU is not compatible with host CPU));
+if (filteredHost) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
+ _(requested guest CPU cannot be provided
+ by QEMU binary on this host));
+} else {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
+ _(guest CPU is not compatible
+ with host CPU));
+}
}
/* fall through */
case VIR_CPU_COMPARE_ERROR:
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index da3b768..83197e0 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -1347,6 +1347,8 @@ qemuDomainDefFormatBuf(virQEMUDriverPtr driver,
virDomainControllerDefPtr *controllers = NULL;
int ncontrollers = 0;
virCapsPtr caps = NULL;
+virQEMUCapsPtr qemuCaps = NULL;
+virCPUDefPtr hostCPU = NULL;
if (!(caps = virQEMUDriverGetCapabilities(driver, false)))
goto cleanup;
@@ -1355,15 +1357,21 @@ qemuDomainDefFormatBuf(virQEMUDriverPtr driver,
if ((flags VIR_DOMAIN_XML_UPDATE_CPU)
def_cpu
(def_cpu-mode != VIR_CPU_MODE_CUSTOM || def_cpu-model)) {
-if (!caps-host.cpu ||
-!caps-host.cpu-model) {
-virReportError(VIR_ERR_OPERATION_FAILED,
- %s, _(cannot get host CPU capabilities));
-goto cleanup;
+if ((qemuCaps = virQEMUCapsCacheLookup(driver-qemuCapsCache,
+ def-emulator)))
+hostCPU = virQEMUCapsGetHostCPU(qemuCaps);
+
+if (!hostCPU) {
+if (!caps-host.cpu || !caps-host.cpu-model) {
+virReportError(VIR_ERR_OPERATION_FAILED, %s,
+ _(cannot get host CPU capabilities));
+goto cleanup;
+}
+hostCPU = caps-host.cpu;
}
if (!(cpu = virCPUDefCopy(def_cpu)) ||
-cpuUpdate(cpu, caps-host.cpu) 0)
+cpuUpdate(cpu, hostCPU) 0)
goto cleanup;
def-cpu = cpu;
}
@@ -1445,6 +1453,7 @@ cleanup:
def-ncontrollers = ncontrollers;
}
virObjectUnref(caps);
+virObjectUnref(qemuCaps);
return ret;
}
--
1.8.3.2
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 3/7] x86: Ignore CPUID functions greater than 10
We don't need to store any CPUID data for function we know nothing
about. However, this limit may need to be increased in the future when
libvirt learns features described by a CPUID function greater than 10.
The comparison is done after subtracting high-bits prefix, so currently
functions 0x to 0x000a and 0x8000 to 0x800a are
stored.
---
src/cpu/cpu_x86.c | 5 +
src/cpu/cpu_x86_data.h | 1 +
2 files changed, 6 insertions(+)
diff --git a/src/cpu/cpu_x86.c b/src/cpu/cpu_x86.c
index dbbcfd2..bd3f2b0 100644
--- a/src/cpu/cpu_x86.c
+++ b/src/cpu/cpu_x86.c
@@ -314,6 +314,11 @@ x86DataAddCpuid(struct cpuX86Data *data,
cpuids = data-extended;
}
+if (pos CPUX86_MAX_FUNCTION) {
+VIR_DEBUG(Ignoring unhandled function 0x%x, cpuid-function);
+return 0;
+}
+
if (x86DataExpand(data, basic_by, extended_by) 0)
return -1;
diff --git a/src/cpu/cpu_x86_data.h b/src/cpu/cpu_x86_data.h
index dc972a6..af470e4 100644
--- a/src/cpu/cpu_x86_data.h
+++ b/src/cpu/cpu_x86_data.h
@@ -36,6 +36,7 @@ struct cpuX86cpuid {
# define CPUX86_BASIC0x0
# define CPUX86_EXTENDED 0x8000
+# define CPUX86_MAX_FUNCTION 10
struct cpuX86Data {
size_t basic_len;
--
1.8.3.2
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 6/7] qemu: Probe QEMU binary for host CPU
On Tue, Jul 23, 2013 at 06:11:35PM +0200, Jiri Denemark wrote:
Since QEMU and kvm may filter some host CPU features or add efficiently
emulated features, asking QEMU binary for host CPU data provides
better results when we later use the data for building guest CPUs.
---
src/qemu/qemu_capabilities.c | 44
+++-
src/qemu/qemu_capabilities.h | 2 ++
2 files changed, 45 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 9440396..d46a059 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -253,6 +253,7 @@ struct _virQEMUCaps {
size_t ncpuDefinitions;
char **cpuDefinitions;
+virCPUDefPtr hostCPU;
size_t nmachineTypes;
char **machineTypes;
@@ -1757,6 +1758,9 @@ virQEMUCapsPtr virQEMUCapsNewCopy(virQEMUCapsPtr
qemuCaps)
goto error;
}
+if (!(ret-hostCPU = virCPUDefCopy(qemuCaps-hostCPU)))
+goto error;
+
if (VIR_ALLOC_N(ret-machineTypes, qemuCaps-nmachineTypes) 0)
goto error;
if (VIR_ALLOC_N(ret-machineAliases, qemuCaps-nmachineTypes) 0)
@@ -1796,6 +1800,7 @@ void virQEMUCapsDispose(void *obj)
VIR_FREE(qemuCaps-cpuDefinitions[i]);
}
VIR_FREE(qemuCaps-cpuDefinitions);
+virCPUDefFree(qemuCaps-hostCPU);
virBitmapFree(qemuCaps-flags);
@@ -2485,7 +2490,6 @@ virQEMUCapsInitQMPCommandNew(const char *binary,
-no-user-config,
-nodefaults,
-nographic,
- -M, none,
-qmp, monitor,
-pidfile, pidfile,
-daemonize,
@@ -2617,6 +2621,7 @@ virQEMUCapsInitQMP(virQEMUCapsPtr qemuCaps,
cmd = virQEMUCapsInitQMPCommandNew(qemuCaps-binary, monarg, pidfile,
runUid, runGid);
+virCommandAddArgList(cmd, -M, none, NULL);
if ((ret = virQEMUCapsInitQMPCommandRun(cmd, qemuCaps-binary, pidfile,
config, mon, pid)) 0) {
@@ -2679,6 +2684,37 @@ virQEMUCapsInitQMP(virQEMUCapsPtr qemuCaps,
if (virQEMUCapsProbeQMPCommandLine(qemuCaps, mon) 0)
goto cleanup;
+if ((qemuCaps-arch == VIR_ARCH_I686 ||
+ qemuCaps-arch == VIR_ARCH_X86_64)
+(virQEMUCapsGet(qemuCaps, QEMU_CAPS_KVM) ||
+ virQEMUCapsGet(qemuCaps, QEMU_CAPS_ENABLE_KVM))
+virQEMUCapsGet(qemuCaps, QEMU_CAPS_CPU_HOST)
+qemuCaps-nmachineTypes) {
+virQEMUCapsInitQMPCommandAbort(cmd, mon, pid, pidfile);
+
+VIR_DEBUG(Checking host CPU data provided by %s, qemuCaps-binary);
+cmd = virQEMUCapsInitQMPCommandNew(qemuCaps-binary, monarg, pidfile,
+ runUid, runGid);
+virCommandAddArgList(cmd, -cpu, host, NULL);
+/* -cpu host gives the same CPU for all machine types so we just
+ * use the first one when probing
+ */
+virCommandAddArg(cmd, -machine);
+virCommandAddArgFormat(cmd, %s,accel=kvm,
+ qemuCaps-machineTypes[0]);
+
+if (virQEMUCapsInitQMPCommandRun(cmd, qemuCaps-binary, pidfile,
+ config, mon, pid) 0)
+goto cleanup;
+
+qemuCaps-hostCPU = qemuMonitorGetCPU(mon, qemuCaps-arch);
+if (qemuCaps-hostCPU) {
+char *cpu = virCPUDefFormat(qemuCaps-hostCPU, 0);
+VIR_DEBUG(Host CPU reported by %s: %s, qemuCaps-binary, cpu);
+VIR_FREE(cpu);
+}
+}
This code is causing us to invoke the QEMU binary multiple times,
which is something we worked really hard to get away from. I really,
really don't like this as an approach. QEMU needs to be able to
give us the data we need here without multiple invocations.
eg, by allowing the monitor command to specify 'kvm' vs 'qemu'
when asking for data, so you can interrogate it without having
to re-launch it with different accel=XXX args.
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 v3 4/7] qemu: Translate the iscsi pool/volume disk source
Il 23/07/2013 18:01, Daniel P. Berrange ha scritto: On Tue, Jul 23, 2013 at 05:35:57PM +0200, Paolo Bonzini wrote: Il 23/07/2013 16:14, Daniel P. Berrange ha scritto: Perhaps the default could be specified in a configuration file (and the default should be the safe one). No, that is even worse because now the default is not predictable.. We simply default to host mode and if applications want to use the other mode they can configure the XML as desired. Can we just forbid mode='default' for iSCSI and force the user to specify host vs. direct? That would mean that apps cannot simply configure a guest volume without first checking to find out what type of pool it is, and then specifying this extra arg for iSCSI. IMHO the value of the volume XML is that you don't have to know anything about the pool to be able to configure it - we're completely decoupled. Thinking more about it, it would only be needed for disk type='volume' device='lun'. And for that case, some knowledge of the pool is necessary anyway (for one thing, it won't work with filesystem or LVM pools). So if we could forbid mode='default' for that case only, it would be enough as far as I'm concernde. Paolo -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 5/7] qemu: Make QMP probing process reusable
On Tue, Jul 23, 2013 at 06:11:34PM +0200, Jiri Denemark wrote: Could really do with a commit message describing what is being changed here what the need is --- src/qemu/qemu_capabilities.c | 192 +++ 1 file changed, 120 insertions(+), 72 deletions(-) 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 6/7] qemu: Probe QEMU binary for host CPU
[adding qemu] On 07/23/2013 10:19 AM, Daniel P. Berrange wrote: On Tue, Jul 23, 2013 at 06:11:35PM +0200, Jiri Denemark wrote: Since QEMU and kvm may filter some host CPU features or add efficiently emulated features, asking QEMU binary for host CPU data provides better results when we later use the data for building guest CPUs. --- +virCommandAddArg(cmd, -machine); +virCommandAddArgFormat(cmd, %s,accel=kvm, + qemuCaps-machineTypes[0]); + This code is causing us to invoke the QEMU binary multiple times, which is something we worked really hard to get away from. I really, really don't like this as an approach. QEMU needs to be able to give us the data we need here without multiple invocations. eg, by allowing the monitor command to specify 'kvm' vs 'qemu' when asking for data, so you can interrogate it without having to re-launch it with different accel=XXX args. We don't know if -machine accel=kvm is a valid option until after issuing some QMP commands, but now we are forced to reinvoke a new qemu with -machine accel=kvm enabled in order to get the query to give us accurate answers. I agree that this is less than desirable; hopefully the qemu folks can help us figure out a solution, now that we are bringing attention to the issue. -- 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 1/7] cpu: Add support for loading and storing CPU data
On Tue, Jul 23, 2013 at 06:11:30PM +0200, Jiri Denemark wrote: Could use a more verbose commit message. Looking at later patches, I see cpuDataFormat used in the test suite, but I don't see cpuDataParse used anywhere ? If these are only for the test suite, then perhaps adding them to a cpupriv.h header is preferrable. --- src/cpu/cpu.c| 41 ++ src/cpu/cpu.h| 13 + src/cpu/cpu_x86.c| 135 +++ src/libvirt_private.syms | 2 + 4 files changed, 170 insertions(+), 21 deletions(-) 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 3/7] x86: Ignore CPUID functions greater than 10
On Tue, Jul 23, 2013 at 06:11:32PM +0200, Jiri Denemark wrote: We don't need to store any CPUID data for function we know nothing about. However, this limit may need to be increased in the future when libvirt learns features described by a CPUID function greater than 10. The comparison is done after subtracting high-bits prefix, so currently functions 0x to 0x000a and 0x8000 to 0x800a are stored. --- src/cpu/cpu_x86.c | 5 + src/cpu/cpu_x86_data.h | 1 + 2 files changed, 6 insertions(+) ACK Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/7] cpu: Export few x86-specific APIs
On Tue, Jul 23, 2013 at 06:11:31PM +0200, Jiri Denemark wrote:
This is adding a new function as well as exporting some
existing functions, so should probably be split in two.
---
src/cpu/cpu_x86.c| 21 ++---
src/cpu/cpu_x86.h| 10 ++
src/libvirt_private.syms | 7 +++
3 files changed, 35 insertions(+), 3 deletions(-)
diff --git a/src/cpu/cpu_x86.c b/src/cpu/cpu_x86.c
index 560a2a9..dbbcfd2 100644
--- a/src/cpu/cpu_x86.c
+++ b/src/cpu/cpu_x86.c
@@ -203,7 +203,7 @@ x86DataCpuid(const struct cpuX86Data *data,
}
-static void
+void
x86DataFree(struct cpuX86Data *data)
These are kind of generic names to be exposing outside the file.
It would be nice to update this file to use a better naming
prefix for all its functions virCPUx86DataN
{
if (data == NULL)
@@ -215,7 +215,7 @@ x86DataFree(struct cpuX86Data *data)
}
-static virCPUDataPtr
+virCPUDataPtr
x86MakeCPUData(virArch arch, struct cpuX86Data **data)
{
virCPUDataPtr cpuData;
@@ -295,7 +295,7 @@ x86DataExpand(struct cpuX86Data *data,
}
-static int
+int
x86DataAddCpuid(struct cpuX86Data *data,
const struct cpuX86cpuid *cpuid)
{
@@ -323,6 +323,21 @@ x86DataAddCpuid(struct cpuX86Data *data,
}
+int
+x86DataSetVendor(struct cpuX86Data *data,
+ const char *vendor)
+{
+struct cpuX86cpuid cpuid;
+
+cpuid.function = 0;
+cpuid.ebx = virReadBufInt32LE(vendor);
+cpuid.edx = virReadBufInt32LE(vendor + 4);
+cpuid.ecx = virReadBufInt32LE(vendor + 8);
+
+return x86DataAddCpuid(data, cpuid);
+}
+
+
static int
x86DataAdd(struct cpuX86Data *data1,
const struct cpuX86Data *data2)
diff --git a/src/cpu/cpu_x86.h b/src/cpu/cpu_x86.h
index 77965b7..8235076 100644
--- a/src/cpu/cpu_x86.h
+++ b/src/cpu/cpu_x86.h
@@ -25,7 +25,17 @@
# define __VIR_CPU_X86_H__
# include cpu.h
+# include cpu_x86_data.h
extern struct cpuArchDriver cpuDriverX86;
+int x86DataAddCpuid(struct cpuX86Data *data,
+const struct cpuX86cpuid *cpuid);
+int x86DataSetVendor(struct cpuX86Data *data,
+ const char *vendor);
+
+void x86DataFree(struct cpuX86Data *data);
+
+virCPUDataPtr x86MakeCPUData(virArch arch, struct cpuX86Data **data);
+
#endif /* __VIR_CPU_X86_H__ */
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index e0f3876..7fe2bc0 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -724,6 +724,13 @@ cpuNodeData;
cpuUpdate;
+# cpu/cpu_x86.h
+x86DataAddCpuid;
+x86DataFree;
+x86DataSetVendor;
+x86MakeCPUData;
+
+
# datatypes.h
virConnectClass;
virDomainClass;
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 3/3] valgrind.supp: Add more valgrind suppression paths
On 07/23/13 17:59, John Ferlan wrote: Update based on recent run/failures seen --- tests/.valgrind.supp | 60 1 file changed, 60 insertions(+) diff --git a/tests/.valgrind.supp b/tests/.valgrind.supp index 10cc3c0..f04912d 100644 --- a/tests/.valgrind.supp +++ b/tests/.valgrind.supp I'm not an valgrind expert, but those look okay to me. ACK. Peter -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 6/7] qemu: Probe QEMU binary for host CPU
On Tue, Jul 23, 2013 at 17:19:03 +0100, Daniel Berrange wrote:
On Tue, Jul 23, 2013 at 06:11:35PM +0200, Jiri Denemark wrote:
Since QEMU and kvm may filter some host CPU features or add efficiently
emulated features, asking QEMU binary for host CPU data provides
better results when we later use the data for building guest CPUs.
---
src/qemu/qemu_capabilities.c | 44
+++-
src/qemu/qemu_capabilities.h | 2 ++
2 files changed, 45 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 9440396..d46a059 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -253,6 +253,7 @@ struct _virQEMUCaps {
size_t ncpuDefinitions;
char **cpuDefinitions;
+virCPUDefPtr hostCPU;
size_t nmachineTypes;
char **machineTypes;
@@ -1757,6 +1758,9 @@ virQEMUCapsPtr virQEMUCapsNewCopy(virQEMUCapsPtr
qemuCaps)
goto error;
}
+if (!(ret-hostCPU = virCPUDefCopy(qemuCaps-hostCPU)))
+goto error;
+
if (VIR_ALLOC_N(ret-machineTypes, qemuCaps-nmachineTypes) 0)
goto error;
if (VIR_ALLOC_N(ret-machineAliases, qemuCaps-nmachineTypes) 0)
@@ -1796,6 +1800,7 @@ void virQEMUCapsDispose(void *obj)
VIR_FREE(qemuCaps-cpuDefinitions[i]);
}
VIR_FREE(qemuCaps-cpuDefinitions);
+virCPUDefFree(qemuCaps-hostCPU);
virBitmapFree(qemuCaps-flags);
@@ -2485,7 +2490,6 @@ virQEMUCapsInitQMPCommandNew(const char *binary,
-no-user-config,
-nodefaults,
-nographic,
- -M, none,
-qmp, monitor,
-pidfile, pidfile,
-daemonize,
@@ -2617,6 +2621,7 @@ virQEMUCapsInitQMP(virQEMUCapsPtr qemuCaps,
cmd = virQEMUCapsInitQMPCommandNew(qemuCaps-binary, monarg, pidfile,
runUid, runGid);
+virCommandAddArgList(cmd, -M, none, NULL);
if ((ret = virQEMUCapsInitQMPCommandRun(cmd, qemuCaps-binary, pidfile,
config, mon, pid)) 0) {
@@ -2679,6 +2684,37 @@ virQEMUCapsInitQMP(virQEMUCapsPtr qemuCaps,
if (virQEMUCapsProbeQMPCommandLine(qemuCaps, mon) 0)
goto cleanup;
+if ((qemuCaps-arch == VIR_ARCH_I686 ||
+ qemuCaps-arch == VIR_ARCH_X86_64)
+(virQEMUCapsGet(qemuCaps, QEMU_CAPS_KVM) ||
+ virQEMUCapsGet(qemuCaps, QEMU_CAPS_ENABLE_KVM))
+virQEMUCapsGet(qemuCaps, QEMU_CAPS_CPU_HOST)
+qemuCaps-nmachineTypes) {
+virQEMUCapsInitQMPCommandAbort(cmd, mon, pid, pidfile);
+
+VIR_DEBUG(Checking host CPU data provided by %s,
qemuCaps-binary);
+cmd = virQEMUCapsInitQMPCommandNew(qemuCaps-binary, monarg,
pidfile,
+ runUid, runGid);
+virCommandAddArgList(cmd, -cpu, host, NULL);
+/* -cpu host gives the same CPU for all machine types so we just
+ * use the first one when probing
+ */
+virCommandAddArg(cmd, -machine);
+virCommandAddArgFormat(cmd, %s,accel=kvm,
+ qemuCaps-machineTypes[0]);
+
+if (virQEMUCapsInitQMPCommandRun(cmd, qemuCaps-binary, pidfile,
+ config, mon, pid) 0)
+goto cleanup;
+
+qemuCaps-hostCPU = qemuMonitorGetCPU(mon, qemuCaps-arch);
+if (qemuCaps-hostCPU) {
+char *cpu = virCPUDefFormat(qemuCaps-hostCPU, 0);
+VIR_DEBUG(Host CPU reported by %s: %s, qemuCaps-binary,
cpu);
+VIR_FREE(cpu);
+}
+}
This code is causing us to invoke the QEMU binary multiple times,
which is something we worked really hard to get away from. I really,
really don't like this as an approach. QEMU needs to be able to
give us the data we need here without multiple invocations.
eg, by allowing the monitor command to specify 'kvm' vs 'qemu'
when asking for data, so you can interrogate it without having
to re-launch it with different accel=XXX args.
Yeah, it's not an ideal solution but it's all we can get now. For giving
us host CPU specification QEMU needs to initialize a complete machine,
that is, it needs to be started with machine type != none, and it needs
to be started with -cpu host, which cannot be used in tcg, only kvm
supports host CPU. So the reasons for us invoking QEMU twice are very
deep inside QEMU architecture. Moreover, we only run QEMU binary twice
for i686 and x86_64 archs and when the binary supports kvm, although
this probably covers most real-life cases :/ In any case, it's all
behind our capabilities cache so this should only happen
Re: [libvirt] [PATCH 03/13] Add a virCgroupNewDetect API for finding cgroup placement
On 07/23/2013 09:21 AM, Daniel P. Berrange wrote: From: Daniel P. Berrange berra...@redhat.com Add a virCgroupNewDetect API which is used to initialize a cgroup object with the placement of an arbitrary process. Signed-off-by: Daniel P. Berrange berra...@redhat.com --- src/libvirt_private.syms | 1 + src/util/vircgroup.c | 81 +++- src/util/vircgroup.h | 3 ++ 3 files changed, 57 insertions(+), 28 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 3/3] valgrind.supp: Add more valgrind suppression paths
On 07/23/2013 09:59 AM, John Ferlan wrote:
Update based on recent run/failures seen
---
tests/.valgrind.supp | 60
1 file changed, 60 insertions(+)
diff --git a/tests/.valgrind.supp b/tests/.valgrind.supp
index 10cc3c0..f04912d 100644
--- a/tests/.valgrind.supp
+++ b/tests/.valgrind.supp
@@ -75,3 +75,63 @@
...
obj:*/lib*/libc-2.*so*
}
+#
+# commandtest validates the various threaded commands. The
+# virThreadCreate() routine allocates and passes args to the
+# new thread which now owns the 'args' and thus cannot be free'd
+#
+{
+commandtestLeak1
+Memcheck:Leak
+fun:calloc
+fun:virAlloc
+fun:virThreadCreate
+fun:mymain
+fun:virtTestMain
Can't we call VIR_FREE(args) in the forked process? Or is this one of
those inconsequential leaks right before we exec in the child process?
+}
+#
+# The Error code requires static memory that is never free'd
+# for thread local storage to store error message/data
+#
+{
+commandtestLeak2
+Memcheck:Leak
+fun:calloc
+fun:virAlloc
+...
+fun:vir*LastError*
+fun:virEventRunDefaultImpl
+fun:virCommandThreadWorker
+fun:virThreadHelper
+fun:start_thread
+fun:clone
I thought we had the ability to wire up destructors for thread local
storage, that gets called when the thread completes?
At any rate, since all of these suppressions are solely for test files,
we aren't masking leaks in libvirtd proper, so if it makes it easier to
spot the introduction of new leaks, I'm okay with this patch as-is.
--
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 v3 4/7] qemu: Translate the iscsi pool/volume disk source
On 07/23/2013 12:18 PM, Paolo Bonzini wrote:
Il 23/07/2013 18:01, Daniel P. Berrange ha scritto:
On Tue, Jul 23, 2013 at 05:35:57PM +0200, Paolo Bonzini wrote:
Il 23/07/2013 16:14, Daniel P. Berrange ha scritto:
Perhaps the default could be specified in a configuration file (and the
default should be the safe one).
No, that is even worse because now the default is not predictable..
We simply default to host mode and if applications want to use the
other mode they can configure the XML as desired.
Can we just forbid mode='default' for iSCSI and force the user to
specify host vs. direct?
That would mean that apps cannot simply configure a guest volume
without first checking to find out what type of pool it is, and
then specifying this extra arg for iSCSI. IMHO the value of the
volume XML is that you don't have to know anything about the
pool to be able to configure it - we're completely decoupled.
Thinking more about it, it would only be needed for disk type='volume'
device='lun'. And for that case, some knowledge of the pool is
necessary anyway (for one thing, it won't work with filesystem or LVM
pools). So if we could forbid mode='default' for that case only, it
would be enough as far as I'm concernde.
Paolo
Using default in the mode field would result in the following XML
error message (I just quickly changed a test to prove the point):
121) QEMU XML-2-XML disk-source-pool-mode
... libvirt: Domain Config error : XML error: unknown source mode
'default' for volume type disk
FAILED
The XML parsing code only looks for mode='direct' or mode='host'. If
mode isn't present in the XML, that's when that default comes into
play. Since 'mode' is new there could be configurations where its not in
an XML file, thus a 0 (zero e.g. default) value is provided for the field.
Once the XML is parsed we still needed a default when it's going to be
added, thus the magic to set the default to HOST is in qemu_conf.c in
qemuTranslateDiskSourcePool():
if (pooldef-type == VIR_STORAGE_POOL_ISCSI) {
/* Default to use the LUN's path on host */
if (!def-srcpool-mode)
def-srcpool-mode = VIR_DOMAIN_DISK_SOURCE_POOL_MODE_HOST;
I think this answers your primary concern - correct?
John
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/7] cpu: Add support for loading and storing CPU data
On Tue, Jul 23, 2013 at 17:27:41 +0100, Daniel Berrange wrote: On Tue, Jul 23, 2013 at 06:11:30PM +0200, Jiri Denemark wrote: Could use a more verbose commit message. Looking at later patches, I see cpuDataFormat used in the test suite, but I don't see cpuDataParse used anywhere ? If these are only for the test suite, then perhaps adding them to a cpupriv.h header is preferrable. Yeah, cpupriv.h might be better since both APIs are really there just for our testsuite. Although I guess marking them as testsuite only in cpu.h would work too. And you're right, cpuDataParse is not used anywhere but I plan to use it in other cpu related tests in the future and having them both in a single patch seemed best to me. Jirka -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3 4/7] qemu: Translate the iscsi pool/volume disk source
Il 23/07/2013 18:47, John Ferlan ha scritto:
On 07/23/2013 12:18 PM, Paolo Bonzini wrote:
Il 23/07/2013 18:01, Daniel P. Berrange ha scritto:
On Tue, Jul 23, 2013 at 05:35:57PM +0200, Paolo Bonzini wrote:
Il 23/07/2013 16:14, Daniel P. Berrange ha scritto:
Perhaps the default could be specified in a configuration file (and the
default should be the safe one).
No, that is even worse because now the default is not predictable..
We simply default to host mode and if applications want to use the
other mode they can configure the XML as desired.
Can we just forbid mode='default' for iSCSI and force the user to
specify host vs. direct?
That would mean that apps cannot simply configure a guest volume
without first checking to find out what type of pool it is, and
then specifying this extra arg for iSCSI. IMHO the value of the
volume XML is that you don't have to know anything about the
pool to be able to configure it - we're completely decoupled.
Thinking more about it, it would only be needed for disk type='volume'
device='lun'. And for that case, some knowledge of the pool is
necessary anyway (for one thing, it won't work with filesystem or LVM
pools). So if we could forbid mode='default' for that case only, it
would be enough as far as I'm concernde.
Using default in the mode field would result in the following XML
error message (I just quickly changed a test to prove the point):
121) QEMU XML-2-XML disk-source-pool-mode
... libvirt: Domain Config error : XML error: unknown source mode
'default' for volume type disk
FAILED
Sorry, by mode='default' I really meant no mode at all (I was under
the false impression that you could also specify a mode='default' with
the same effect as no mode at all).
The XML parsing code only looks for mode='direct' or mode='host'. If
mode isn't present in the XML, that's when that default comes into
play. Since 'mode' is new there could be configurations where its not in
an XML file, thus a 0 (zero e.g. default) value is provided for the field.
Once the XML is parsed we still needed a default when it's going to be
added, thus the magic to set the default to HOST is in qemu_conf.c in
qemuTranslateDiskSourcePool():
if (pooldef-type == VIR_STORAGE_POOL_ISCSI) {
/* Default to use the LUN's path on host */
if (!def-srcpool-mode)
def-srcpool-mode = VIR_DOMAIN_DISK_SOURCE_POOL_MODE_HOST;
I think this answers your primary concern - correct?
No, my concern is that mode='host' is a bad default for the device='lun'
case.
Paolo
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v5 4/5] storage: Support chap authentication for iscsi pool
On 07/23/2013 12:03 PM, Daniel P. Berrange wrote:
On Tue, Jul 23, 2013 at 11:50:57AM -0400, John Ferlan wrote:
On 07/23/2013 10:56 AM, Daniel P. Berrange wrote:
...snip...
+++ b/src/storage/storage_backend_iscsi.c
@@ -714,7 +714,8 @@ virStorageBackendISCSISetAuth(const char *portal,
if (!conn) {
virReportError(VIR_ERR_INTERNAL_ERROR, %s,
- _(iscsi 'chap' authentication requires
connection));
+ _(iscsi 'chap' authentication not supported
+ for autostarted pools));
return -1;
}
I noticed that the nwfilter already unconditionally calls
virConnectOpen(qemu://system); so we're already in fact suffering from
the problem with
autostart having a qemu dependency.
Given this, I'd support a patch which simply did
conn = virConnectOpen(privilege ? qemu:///system : qemu:///session);
in storageDriverAutostart, provided that we ignore any errors from
virConnectOpen, and fallback to use NULL for the connection in that
case.
Obviously this is something we'll still need to fix properly in a
future release, but at least it'll make autostart of storage pools
with auth work in the common case in the short term for this release.
Regards,
Daniel
So resurrect the following:
https://www.redhat.com/archives/libvir-list/2013-July/msg00962.html
Yep, ACK to that patch, but add XXX Remove hardcoding of QEMU URI
as a comment just before the virConnectOpen call.
Daniel
OK - I resurrected the patch, added the comment, and pushed.
John
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 4/7] qemu: Add monitor APIs to fetch CPUID data from QEMU
On Tue, Jul 23, 2013 at 06:11:33PM +0200, Jiri Denemark wrote: --- src/qemu/qemu_monitor.c| 21 +++ src/qemu/qemu_monitor.h| 3 + src/qemu/qemu_monitor_json.c | 162 + src/qemu/qemu_monitor_json.h | 6 + tests/Makefile.am | 1 + .../qemumonitorjson-getcpu-empty.data | 2 + .../qemumonitorjson-getcpu-empty.json | 46 ++ .../qemumonitorjson-getcpu-filtered.data | 4 + .../qemumonitorjson-getcpu-filtered.json | 46 ++ .../qemumonitorjson-getcpu-full.data | 4 + .../qemumonitorjson-getcpu-full.json | 46 ++ .../qemumonitorjson-getcpu-host.data | 5 + .../qemumonitorjson-getcpu-host.json | 45 ++ tests/qemumonitorjsontest.c| 74 ++ 14 files changed, 465 insertions(+) create mode 100644 tests/qemumonitorjsondata/qemumonitorjson-getcpu-empty.data create mode 100644 tests/qemumonitorjsondata/qemumonitorjson-getcpu-empty.json create mode 100644 tests/qemumonitorjsondata/qemumonitorjson-getcpu-filtered.data create mode 100644 tests/qemumonitorjsondata/qemumonitorjson-getcpu-filtered.json create mode 100644 tests/qemumonitorjsondata/qemumonitorjson-getcpu-full.data create mode 100644 tests/qemumonitorjsondata/qemumonitorjson-getcpu-full.json create mode 100644 tests/qemumonitorjsondata/qemumonitorjson-getcpu-host.data create mode 100644 tests/qemumonitorjsondata/qemumonitorjson-getcpu-host.json ACK, though I believe the design of this monitor API is flawed because it requires you to re-launch QEMU with different accel args 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
