Re: [libvirt] [PATCH v2 04/16] conf, schema: add 'id' field for cells
On Fri, Jul 11, 2014 at 05:11:05PM +0200, Michal Privoznik wrote:
On 08.07.2014 13:50, Martin Kletzander wrote:
In XML format, by definition, order of fields should not matter, so
order of parsing the elements doesn't affect the end result. When
specifying guest NUMA cells, we depend only on the order of the 'cell'
elements. With this patch all older domain XMLs are parsed as before,
but with the 'id' attribute they are parsed and formatted according to
that field. This will be useful when we have tuning settings for
particular guest NUMA node.
Signed-off-by: Martin Kletzander mklet...@redhat.com
---
docs/formatdomain.html.in | 11 +++---
docs/schemas/domaincommon.rng | 5 +++
src/conf/cpu_conf.c| 39 +++---
src/conf/cpu_conf.h| 3 +-
src/qemu/qemu_command.c| 2 +-
tests/qemuxml2argvdata/qemuxml2argv-cpu-numa1.xml | 6 ++--
tests/qemuxml2argvdata/qemuxml2argv-cpu-numa2.xml | 6 ++--
tests/qemuxml2argvdata/qemuxml2argv-cpu-numa3.xml | 25 ++
tests/qemuxml2argvtest.c | 1 +
.../qemuxml2xmlout-cpu-numa1.xml | 28
.../qemuxml2xmlout-cpu-numa2.xml | 28
tests/qemuxml2xmltest.c| 3 ++
12 files changed, 139 insertions(+), 18 deletions(-)
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-cpu-numa3.xml
create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-cpu-numa1.xml
create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-cpu-numa2.xml
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index b69da4c..ad87b7c 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
[...]
@@ -1041,8 +1041,11 @@
Each codecell/code element specifies a NUMA cell or a NUMA node.
codecpus/code specifies the CPU or range of CPUs that are part of
the node. codememory/code specifies the node memory in kibibytes
- (i.e. blocks of 1024 bytes). Each cell or node is assigned cellid
- or nodeid in the increasing order starting from 0.
+ (i.e. blocks of 1024 bytes). All cells should have codeid/code
+ attribute in case referring to some cell is necessary in the code,
+ otherwise the cells are assigned ids in the increasing order starting
+ from 0. Mixing cells with and without the codeid/code attribute
+ is not recommended as it may result in unwanted behaviour.
I'd note here, that the @id attribute is since 1.2.7
I wasn't sure this is needed for attributes, but it cannot hurt,
right? The new hunk would now look like this:
@@ -1039,10 +1039,15 @@
p
Each codecell/code element specifies a NUMA cell or a NUMA node.
- codecpus/code specifies the CPU or range of CPUs that are part of
- the node. codememory/code specifies the node memory in kibibytes
- (i.e. blocks of 1024 bytes). Each cell or node is assigned cellid
- or nodeid in the increasing order starting from 0.
+ codecpus/code specifies the CPU or range of CPUs that are
+ part of the node. codememory/code specifies the node memory
+ in kibibytes (i.e. blocks of 1024 bytes).
+ span class=sinceSince 1.2.7/span all cells should
+ have codeid/code attribute in case referring to some cell is
+ necessary in the code, otherwise the cells are
+ assigned codeid/codes in the increasing order starting from
+ 0. Mixing cells with and without the codeid/code attribute
+ is not recommended as it may result in unwanted behaviour.
/p
p
--
Is that OK?
[...]
@@ -438,17 +437,46 @@ virCPUDefParseXML(xmlNodePtr node,
for (i = 0; i n; i++) {
char *cpus, *memory;
int ret, ncpus = 0;
+unsigned int cur_cell;
+char *tmp = NULL;
+
+tmp = virXMLPropString(nodes[i], id);
+if (!tmp) {
+cur_cell = i;
+} else {
+ret = virStrToLong_ui(tmp, NULL, 10, cur_cell);
+VIR_FREE(tmp);
+if (ret == -1) {
+virReportError(VIR_ERR_XML_ERROR, %s,
+ _(Invalid 'id' attribute in NUMA cell));
+goto error;
+}
+}
If there's a typo in the @id, I think this can make users lives easier:
You mean like a typo in an integer, right? :-) If the user cannot find
a typo that causes our code not to parse it as an integer; then I
guess such user's biggest issue isn't the typo :-) But I squashed in
your suggestion.
Martin
signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [libvirt-users] LVM Volume Creation
Hi All, I'm having issue with creating LVM Volume via libvirt. We are running libvirtd 1.2 with KVM. We are creating Volume Group (VG01) outside of libvirt and defining a storage pool for it. Here is the StoragePool XML for the Volume Group created outside libvirt. pool type=logical nameVG01/name target path/dev/VG01/path /target /pool We are creating Logical Volume (ub_test01.img) through libvirt in the Volume Group (VG01) Here is the XML to create Storage Volume in LVM Storage Pool VG01. volume type=block nameub_test01.img/name allocation0/allocation capacity unit=M1/capacity target format type=lvm2 / /target /volume When I create the Logical Volume from libvirt using above XML and run lvs command to list logical volumes, this is what I see. LV VG AttrLSize Origin Snap% Move Log Copy% Convert --- --- --- --- --- --- --- --- --- --- foo VG01-wi-a-1.00g ub_test01.img VG01swi-a-4.00m [ub_test01.img_vorigin] 0.20 rootops-02 -wi-ao 227.08g swap_1 ops-02 -wi-ao5.75g As you see, ub_test01.img shows that it has an Origin indicating that it was created as a snapshot, but, that Origin doesn't exist and wasn't specified. I'd appreciate if you anyone can help me understand what is going on and/or describe how to make libvirt create logical volumes not as a snapshot. I'm happy to enable debugging to see what command libvirt is running to create the volume if someone case describe how to enable debugging. Regards, Ravi Message sent via Atmail Open - http://atmail.org/ -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 07/16] numatune: Encapsulate numatune configuration in order to unify results
On Fri, Jul 11, 2014 at 05:11:00PM +0200, Michal Privoznik wrote:
On 08.07.2014 13:50, Martin Kletzander wrote:
There were numerous places where numatune configuration (and thus
domain config as well) was changed in different ways. On some
places this even resulted in persistent domain definition not to be
stable (it would change with daemon's restart).
In order to uniformly change how numatune config is dealt with, all
the internals are now accessible directly only in numatune_conf.c and
outside this file accessors must be used.
Signed-off-by: Martin Kletzander mklet...@redhat.com
---
po/POTFILES.in | 1 +
src/conf/domain_conf.c | 159 ++-
src/conf/domain_conf.h | 8 +-
src/conf/numatune_conf.c | 316 +
src/conf/numatune_conf.h | 72 -
src/libvirt_private.syms | 11 +
src/lxc/lxc_cgroup.c | 19 +-
src/lxc/lxc_controller.c | 5 +-
src/lxc/lxc_native.c | 15 +-
src/parallels/parallels_driver.c | 7 +-
src/qemu/qemu_cgroup.c | 23 +-
src/qemu/qemu_driver.c | 84 +++---
src/qemu/qemu_process.c| 8 +-
src/util/virnuma.c | 48 ++--
src/util/virnuma.h | 2 +-
.../qemuxml2argv-numatune-auto-prefer.xml | 29 ++
.../qemuxml2xmlout-numatune-auto-prefer.xml| 29 ++
tests/qemuxml2xmltest.c| 2 +
18 files changed, 553 insertions(+), 285 deletions(-)
create mode 100644
tests/qemuxml2argvdata/qemuxml2argv-numatune-auto-prefer.xml
create mode 100644
tests/qemuxml2xmloutdata/qemuxml2xmlout-numatune-auto-prefer.xml
Nice.
Thanks :)
[...]
+tmp = virXMLPropString(node, nodeset);
+if (tmp virBitmapParse(tmp, 0, nodeset, VIR_DOMAIN_CPUMASK_LEN) 0)
+goto cleanup;
+VIR_FREE(tmp);
+
+if (virDomainNumatuneSet(def, placement, mode, nodeset) 0)
The virDomainNumatuneSet() takes a copy of @nodeset, so you need to call
virBitmaskFree(nodeset); at the cleanup label.
Yep, that happens when you change the behaviour of a function that
used to steal a pointer, in a rebase. Thanks!
+goto cleanup;
+
+if (!n) {
+ret = 0;
+goto cleanup;
+}
+
+ret = 0;
+ cleanup:
+VIR_FREE(tmp);
+return ret;
+}
+
+int
+virDomainNumatuneFormatXML(virBufferPtr buf,
+ virDomainNumatunePtr numatune)
+{
+const char *tmp = NULL;
s /const// ..
+
+if (!numatune)
+return 0;
+
+virBufferAddLit(buf, numatune\n);
+virBufferAdjustIndent(buf, 2);
+
+tmp = virDomainNumatuneMemModeTypeToString(numatune-memory.mode);
+virBufferAsprintf(buf, memory mode='%s' , tmp);
+
+if (numatune-memory.placement == VIR_DOMAIN_NUMATUNE_PLACEMENT_STATIC) {
+if (!(tmp = virBitmapFormat(numatune-memory.nodeset)))
+return -1;
+virBufferAsprintf(buf, nodeset='%s'/\n, tmp);
+VIR_FREE(tmp);
.. because free()-ing a const char * is not nice. If you, however, do
this I bet you'll get error in TypeToString(). So just leave tmp as
const char * and introduce char *nodeset;
I take the 'const' as a sign of the fact that I won't be modifying
any part of the string. Just adding 'const' to a pointer should be
perfectly OK, but I have not objections to your idea, so I squashed
this in:
diff --git i/src/conf/numatune_conf.c w/src/conf/numatune_conf.c
index 8b66558..375428c 100644
--- i/src/conf/numatune_conf.c
+++ w/src/conf/numatune_conf.c
@@ -141,6 +142,7 @@ virDomainNumatuneFormatXML(virBufferPtr buf,
virDomainNumatunePtr numatune)
{
const char *tmp = NULL;
+char *nodeset = NULL;
if (!numatune)
return 0;
@@ -152,10 +154,10 @@ virDomainNumatuneFormatXML(virBufferPtr buf,
virBufferAsprintf(buf, memory mode='%s' , tmp);
if (numatune-memory.placement == VIR_DOMAIN_NUMATUNE_PLACEMENT_STATIC) {
-if (!(tmp = virBitmapFormat(numatune-memory.nodeset)))
+if (!(nodeset = virBitmapFormat(numatune-memory.nodeset)))
return -1;
virBufferAsprintf(buf, nodeset='%s'/\n, tmp);
-VIR_FREE(tmp);
+VIR_FREE(nodeset);
} else if (numatune-memory.placement) {
tmp =
virDomainNumatunePlacementTypeToString(numatune-memory.placement);
virBufferAsprintf(buf, placement='%s'/\n, tmp);
--
Martin
signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [libvirt-users] LVM Volume Creation
On 07/15/2014 08:28 AM, Ravi Samji wrote: Hi All, I'm having issue with creating LVM Volume via libvirt. We are running libvirtd 1.2 with KVM. We are creating Volume Group (VG01) outside of libvirt and defining a storage pool for it. Here is the StoragePool XML for the Volume Group created outside libvirt. pool type=logical nameVG01/name target path/dev/VG01/path /target /pool We are creating Logical Volume (ub_test01.img) through libvirt in the Volume Group (VG01) Here is the XML to create Storage Volume in LVM Storage Pool VG01. volume type=block nameub_test01.img/name allocation0/allocation capacity unit=M1/capacity If the allocation is not equal to the capacity, libvirt calls lvcreate with the --virtualsize option, which creates this snapshot that should take up less space. For a regular volume, just omit the allocation element, or use the same values for both. target format type=lvm2 / /target /volume When I create the Logical Volume from libvirt using above XML and run lvs command to list logical volumes, this is what I see. LVVG AttrLSize Origin Snap% Move Log Copy% Convert --- --- --- --- --- --- --- --- --- --- foo VG01-wi-a-1.00g ub_test01.img VG01swi-a-4.00m [ub_test01.img_vorigin] 0.20 root ops-02 -wi-ao 227.08g swap_1ops-02 -wi-ao5.75g As you see, ub_test01.img shows that it has an Origin indicating that it was created as a snapshot, but, that Origin doesn't exist and wasn't specified. I'd appreciate if you anyone can help me understand what is going on and/or describe how to make libvirt create logical volumes not as a snapshot. I'm happy to enable debugging to see what command libvirt is running to create the volume if someone case describe how to enable debugging. See http://libvirt.org/logging.html Jan signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/2] Introduce virDomainYesNo enum type
On Mon, Jul 14, 2014 at 10:58:27AM -0600, Eric Blake wrote:
On 07/14/2014 10:40 AM, Daniel P. Berrange wrote:
}
-def-os.bios.useserial = VIR_DOMAIN_BIOS_USESERIAL_YES;
+def-os.bios.useserial = VIR_DOMAIN_YES_NO_ENABLED;
} else {
-def-os.bios.useserial = VIR_DOMAIN_BIOS_USESERIAL_NO;
+def-os.bios.useserial = VIR_DOMAIN_YES_NO_DISABLED;
}
if (def-data.spice.filetransfer)
virBufferAsprintf(buf, filetransfer enable='%s'/\n,
-
virDomainGraphicsSpiceAgentFileTransferTypeToString(def-data.spice.filetransfer));
+
virDomainYesNoTypeToString(def-data.spice.filetransfer));
}
I'm not really a fan of this cleanup, as IMHO the result is less clear
harder to follow than the original code.
How so? The original code was very repetitive, with multiple enums (all
with long names) copying the same few enum elements. We're not painting
ourselves into a corner - if any of the replaced enums ever grows a
third value (such as on, hybrid, off), then we just break that one
enum back into a named list rather than using the generic on/off enum.
I'm actually in favor of this cleanup.
Specifically a enum constant name like YES_NO_DISABLED is just awful IMHO
compared to the original desriptive name.
Is it just a matter of coming up with a better name? Maybe:
VIR_TRISTATE_ABSENT = 0,
VIR_TRISTATE_NO,
VIR_TRISTATE_YES,
Yes, that would be much nicer
Regards,
Daniel
--
|: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/4] lxc: print ENOTSUP when usernamespace is not supported
In lxcContainerStart, when user namespace is not supported,
the virReportSystemError is called. But the first argument
should be ENOTSUPP, instead of VIR_ERR_CONFIG_UNSUPPORTED.
Signed-off-by: Jincheng Miao jm...@redhat.com
---
src/lxc/lxc_container.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c
index 4d89677..343df47 100644
--- a/src/lxc/lxc_container.c
+++ b/src/lxc/lxc_container.c
@@ -2020,7 +2020,7 @@ int lxcContainerStart(virDomainDefPtr def,
VIR_DEBUG(Enable user namespace);
cflags |= CLONE_NEWUSER;
} else {
-virReportSystemError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
+virReportSystemError(ENOTSUP, %s,
_(Kernel doesn't support user namespace));
VIR_FREE(stack);
return -1;
--
1.8.3.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/4] qemu: fix wrong errno report in qemuMonitorOpenUnix
In qemuMonitorOpenUnix, after connect(), virProcessKill will be
invoked when cpid is not empty. But if the qemu-kvm process exited
early, virProcessKill will flush errno as ESRCH, so the wrong
message will be recorded in log as:
error: qemuMonitorOpenUnix:309 : failed to connect to monitor socket: No such
process
After patched:
error : qemuMonitorOpenUnix:312 : failed to connect to monitor socket:
Connection refused
Signed-off-by: Jincheng Miao jm...@redhat.com
---
src/qemu/qemu_monitor.c | 7 +--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index db3dd73..c8284fe 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -293,19 +293,22 @@ qemuMonitorOpenUnix(const char *monitor, pid_t cpid)
}
do {
+int orig_errno;
ret = connect(monfd, (struct sockaddr *) addr, sizeof(addr));
if (ret == 0)
break;
-if ((errno == ENOENT || errno == ECONNREFUSED)
+orig_errno = errno;
+
+if ((orig_errno == ENOENT || orig_errno == ECONNREFUSED)
(!cpid || virProcessKill(cpid, 0) == 0)) {
/* ENOENT : Socket may not have shown up yet
* ECONNREFUSED : Leftover socket hasn't been removed yet */
continue;
}
-virReportSystemError(errno, %s,
+virReportSystemError(orig_errno, %s,
_(failed to connect to monitor socket));
goto error;
--
1.8.3.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/4] fix virReportSystemError misuse
virReportSystemError() reports some OS errors, and the first argument of it should be the error number defined in errno.h. virReportError() reports libvirt errors, and the first argument should be the error number defined in virerrno.h. This patch set fix some misuse of virReportSystemError: passing virerrno instead of errno. Jincheng Miao (4): qemu: fix wrong errno report in qemuMonitorOpenUnix lxc: print ENOTSUP when usernamespace is not supported openvz: print EOVERFLOW when barrier:limit are too long util: print errno in virObjectLockableNew src/lxc/lxc_container.c | 2 +- src/openvz/openvz_conf.c | 2 +- src/qemu/qemu_monitor.c | 7 +-- src/util/virobject.c | 2 +- 4 files changed, 8 insertions(+), 5 deletions(-) -- 1.8.3.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 4/4] util: print errno in virObjectLockableNew
In virObjectLockableNew, when virMutexInit fails,
virReportSystemError should use errno to get the right
error number, instead of VIR_ERR_INTERNAL_ERROR.
Signed-off-by: Jincheng Miao jm...@redhat.com
---
src/util/virobject.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/util/virobject.c b/src/util/virobject.c
index 6cb84b4..b5d2c02 100644
--- a/src/util/virobject.c
+++ b/src/util/virobject.c
@@ -220,7 +220,7 @@ void *virObjectLockableNew(virClassPtr klass)
return NULL;
if (virMutexInit(obj-lock) 0) {
-virReportSystemError(VIR_ERR_INTERNAL_ERROR, %s,
+virReportSystemError(errno, %s,
_(Unable to initialize mutex));
virObjectUnref(obj);
return NULL;
--
1.8.3.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 3/4] openvz: print EOVERFLOW when barrier:limit are too long
In openvzReadFSConf, when barrier:limit are so long to
overflow, pass EOVERFLOW to virReportSystemError, instead
of VIR_ERR_OVERFLOW.
Signed-off-by: Jincheng Miao jm...@redhat.com
---
src/openvz/openvz_conf.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/openvz/openvz_conf.c b/src/openvz/openvz_conf.c
index 856c9f5..8fdd4e9 100644
--- a/src/openvz/openvz_conf.c
+++ b/src/openvz/openvz_conf.c
@@ -405,7 +405,7 @@ openvzReadFSConf(virDomainDefPtr def,
/* Ensure that we can multiply by 1024 without overflowing. */
if (barrier ULLONG_MAX / 1024 ||
limit ULLONG_MAX / 1024) {
-virReportSystemError(VIR_ERR_OVERFLOW, %s,
+virReportSystemError(EOVERFLOW, %s,
_(Unable to parse quota));
goto error;
}
--
1.8.3.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] qemu: blockcopy: Initialize correct source structure
4cc1f1a01fb338de939ba88eb933931687b22336 introduced a crash when doing a block copy as virStorageSourceInitChainElement was called on disk-mirror that is still NULL at that point instead of mirror which temporarily holds the mirror source struct until it's fully initialized. This resulted into a crash as a NULL was dereferenced. Reported by: Shanzi Yu s...@redhat.com --- Fortunately unreleased. src/qemu/qemu_driver.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 8d40bc9..c0ad446 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -15309,7 +15309,7 @@ qemuDomainBlockCopy(virDomainObjPtr vm, if (VIR_STRDUP(mirror-path, dest) 0) goto endjob; -if (virStorageSourceInitChainElement(disk-mirror, disk-src, false) 0) +if (virStorageSourceInitChainElement(mirror, disk-src, false) 0) goto endjob; if (qemuDomainPrepareDiskChainElement(driver, vm, mirror, -- 2.0.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 07/16] numatune: Encapsulate numatune configuration in order to unify results
On 15.07.2014 08:33, Martin Kletzander wrote:
On Fri, Jul 11, 2014 at 05:11:00PM +0200, Michal Privoznik wrote:
On 08.07.2014 13:50, Martin Kletzander wrote:
There were numerous places where numatune configuration (and thus
domain config as well) was changed in different ways. On some
places this even resulted in persistent domain definition not to be
stable (it would change with daemon's restart).
In order to uniformly change how numatune config is dealt with, all
the internals are now accessible directly only in numatune_conf.c and
outside this file accessors must be used.
Signed-off-by: Martin Kletzander mklet...@redhat.com
---
po/POTFILES.in | 1 +
src/conf/domain_conf.c | 159 ++-
src/conf/domain_conf.h | 8 +-
src/conf/numatune_conf.c | 316
+
src/conf/numatune_conf.h | 72 -
src/libvirt_private.syms | 11 +
src/lxc/lxc_cgroup.c | 19 +-
src/lxc/lxc_controller.c | 5 +-
src/lxc/lxc_native.c | 15 +-
src/parallels/parallels_driver.c | 7 +-
src/qemu/qemu_cgroup.c | 23 +-
src/qemu/qemu_driver.c | 84 +++---
src/qemu/qemu_process.c| 8 +-
src/util/virnuma.c | 48 ++--
src/util/virnuma.h | 2 +-
.../qemuxml2argv-numatune-auto-prefer.xml | 29 ++
.../qemuxml2xmlout-numatune-auto-prefer.xml| 29 ++
tests/qemuxml2xmltest.c| 2 +
18 files changed, 553 insertions(+), 285 deletions(-)
create mode 100644
tests/qemuxml2argvdata/qemuxml2argv-numatune-auto-prefer.xml
create mode 100644
tests/qemuxml2xmloutdata/qemuxml2xmlout-numatune-auto-prefer.xml
Nice.
Thanks :)
[...]
+tmp = virXMLPropString(node, nodeset);
+if (tmp virBitmapParse(tmp, 0, nodeset,
VIR_DOMAIN_CPUMASK_LEN) 0)
+goto cleanup;
+VIR_FREE(tmp);
+
+if (virDomainNumatuneSet(def, placement, mode, nodeset) 0)
The virDomainNumatuneSet() takes a copy of @nodeset, so you need to call
virBitmaskFree(nodeset); at the cleanup label.
Yep, that happens when you change the behaviour of a function that
used to steal a pointer, in a rebase. Thanks!
+goto cleanup;
+
+if (!n) {
+ret = 0;
+goto cleanup;
+}
+
+ret = 0;
+ cleanup:
+VIR_FREE(tmp);
+return ret;
+}
+
+int
+virDomainNumatuneFormatXML(virBufferPtr buf,
+ virDomainNumatunePtr numatune)
+{
+const char *tmp = NULL;
s /const// ..
+
+if (!numatune)
+return 0;
+
+virBufferAddLit(buf, numatune\n);
+virBufferAdjustIndent(buf, 2);
+
+tmp = virDomainNumatuneMemModeTypeToString(numatune-memory.mode);
+virBufferAsprintf(buf, memory mode='%s' , tmp);
+
+if (numatune-memory.placement ==
VIR_DOMAIN_NUMATUNE_PLACEMENT_STATIC) {
+if (!(tmp = virBitmapFormat(numatune-memory.nodeset)))
+return -1;
+virBufferAsprintf(buf, nodeset='%s'/\n, tmp);
+VIR_FREE(tmp);
.. because free()-ing a const char * is not nice. If you, however, do
this I bet you'll get error in TypeToString(). So just leave tmp as
const char * and introduce char *nodeset;
I take the 'const' as a sign of the fact that I won't be modifying
any part of the string. Just adding 'const' to a pointer should be
perfectly OK, but I have not objections to your idea, so I squashed
this in:
Well, I look at free()-ing as modification of the pointee. Therefore
freeing a const pointer is in fact its modification and hence should be
rejected. It's just that our VIR_FREE throws away the const-ness of
passed pointers. Maybe (as completely separate patchset) we may fix the
VIR_FREE() macro which is obviously const-incorrect.
Michal
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 04/16] conf, schema: add 'id' field for cells
On 15.07.2014 08:23, Martin Kletzander wrote: On Fri, Jul 11, 2014 at 05:11:05PM +0200, Michal Privoznik wrote: On 08.07.2014 13:50, Martin Kletzander wrote: In XML format, by definition, order of fields should not matter, so order of parsing the elements doesn't affect the end result. When specifying guest NUMA cells, we depend only on the order of the 'cell' elements. With this patch all older domain XMLs are parsed as before, but with the 'id' attribute they are parsed and formatted according to that field. This will be useful when we have tuning settings for particular guest NUMA node. Signed-off-by: Martin Kletzander mklet...@redhat.com --- docs/formatdomain.html.in | 11 +++--- docs/schemas/domaincommon.rng | 5 +++ src/conf/cpu_conf.c| 39 +++--- src/conf/cpu_conf.h| 3 +- src/qemu/qemu_command.c| 2 +- tests/qemuxml2argvdata/qemuxml2argv-cpu-numa1.xml | 6 ++-- tests/qemuxml2argvdata/qemuxml2argv-cpu-numa2.xml | 6 ++-- tests/qemuxml2argvdata/qemuxml2argv-cpu-numa3.xml | 25 ++ tests/qemuxml2argvtest.c | 1 + .../qemuxml2xmlout-cpu-numa1.xml | 28 .../qemuxml2xmlout-cpu-numa2.xml | 28 tests/qemuxml2xmltest.c| 3 ++ 12 files changed, 139 insertions(+), 18 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-cpu-numa3.xml create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-cpu-numa1.xml create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-cpu-numa2.xml diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index b69da4c..ad87b7c 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in [...] @@ -1041,8 +1041,11 @@ Each codecell/code element specifies a NUMA cell or a NUMA node. codecpus/code specifies the CPU or range of CPUs that are part of the node. codememory/code specifies the node memory in kibibytes - (i.e. blocks of 1024 bytes). Each cell or node is assigned cellid - or nodeid in the increasing order starting from 0. + (i.e. blocks of 1024 bytes). All cells should have codeid/code + attribute in case referring to some cell is necessary in the code, + otherwise the cells are assigned ids in the increasing order starting + from 0. Mixing cells with and without the codeid/code attribute + is not recommended as it may result in unwanted behaviour. I'd note here, that the @id attribute is since 1.2.7 I wasn't sure this is needed for attributes, but it cannot hurt, right? The new hunk would now look like this: @@ -1039,10 +1039,15 @@ p Each codecell/code element specifies a NUMA cell or a NUMA node. - codecpus/code specifies the CPU or range of CPUs that are part of - the node. codememory/code specifies the node memory in kibibytes - (i.e. blocks of 1024 bytes). Each cell or node is assigned cellid - or nodeid in the increasing order starting from 0. + codecpus/code specifies the CPU or range of CPUs that are + part of the node. codememory/code specifies the node memory + in kibibytes (i.e. blocks of 1024 bytes). + span class=sinceSince 1.2.7/span all cells should + have codeid/code attribute in case referring to some cell is + necessary in the code, otherwise the cells are + assigned codeid/codes in the increasing order starting from + 0. Mixing cells with and without the codeid/code attribute + is not recommended as it may result in unwanted behaviour. /p p -- Is that OK? Okay. Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH V3 2/2] add nocow test case
Add file in storagevolxml2xmlin and storagevolxml2xmlout, let storagevolxml2xmltest and storagevolschematest cover 'nocow'. Add test case to storagevolxml2argvtest to cover 'nocow'. Signed-off-by: Chunyan Liu cy...@suse.com --- .../storagevolxml2argvdata/qcow2-nocow-compat.argv | 3 ++ tests/storagevolxml2argvdata/qcow2-nocow.argv | 3 ++ tests/storagevolxml2argvtest.c | 6 tests/storagevolxml2xmlin/vol-qcow2-nocow.xml | 32 ++ tests/storagevolxml2xmlout/vol-qcow2-nocow.xml | 31 + 5 files changed, 75 insertions(+) create mode 100644 tests/storagevolxml2argvdata/qcow2-nocow-compat.argv create mode 100644 tests/storagevolxml2argvdata/qcow2-nocow.argv create mode 100644 tests/storagevolxml2xmlin/vol-qcow2-nocow.xml create mode 100644 tests/storagevolxml2xmlout/vol-qcow2-nocow.xml diff --git a/tests/storagevolxml2argvdata/qcow2-nocow-compat.argv b/tests/storagevolxml2argvdata/qcow2-nocow-compat.argv new file mode 100644 index 000..d5a7547 --- /dev/null +++ b/tests/storagevolxml2argvdata/qcow2-nocow-compat.argv @@ -0,0 +1,3 @@ +qemu-img create -f qcow2 -b /dev/null \ +-o backing_fmt=raw,encryption=on,nocow=on,compat=0.10 \ +/var/lib/libvirt/images/OtherDemo.img 5242880K diff --git a/tests/storagevolxml2argvdata/qcow2-nocow.argv b/tests/storagevolxml2argvdata/qcow2-nocow.argv new file mode 100644 index 000..e54801c --- /dev/null +++ b/tests/storagevolxml2argvdata/qcow2-nocow.argv @@ -0,0 +1,3 @@ +qemu-img create -f qcow2 -b /dev/null \ +-o backing_fmt=raw,encryption=on,nocow=on \ +/var/lib/libvirt/images/OtherDemo.img 5242880K diff --git a/tests/storagevolxml2argvtest.c b/tests/storagevolxml2argvtest.c index 11d70e1..2a45f6f 100644 --- a/tests/storagevolxml2argvtest.c +++ b/tests/storagevolxml2argvtest.c @@ -296,6 +296,12 @@ mymain(void) DO_TEST(pool-logical, vol-logical, pool-dir, vol-qcow2-nobacking, logical-from-qcow2, 0, FMT_COMPAT); +DO_TEST(pool-dir, vol-qcow2-nocow, +NULL, NULL, +qcow2-nocow, 0, FMT_OPTIONS); +DO_TEST(pool-dir, vol-qcow2-nocow, +NULL, NULL, +qcow2-nocow-compat, 0, FMT_COMPAT); return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE; } diff --git a/tests/storagevolxml2xmlin/vol-qcow2-nocow.xml b/tests/storagevolxml2xmlin/vol-qcow2-nocow.xml new file mode 100644 index 000..661475b --- /dev/null +++ b/tests/storagevolxml2xmlin/vol-qcow2-nocow.xml @@ -0,0 +1,32 @@ +volume + nameOtherDemo.img/name + key/var/lib/libvirt/images/OtherDemo.img/key + source + /source + capacity unit=G5/capacity + allocation294912/allocation + target +path/var/lib/libvirt/images/OtherDemo.img/path +format type='qcow2'/ +permissions + mode0644/mode + owner0/owner + group0/group + labelunconfined_u:object_r:virt_image_t:s0/label +/permissions +encryption format='qcow' + secret type='passphrase' uuid='e78d4b51-a2af-485f-b0f5-afca709a80f4'/ +/encryption +nocow/ + /target + backingStore +path/dev/null/path +format type='raw'/ +permissions + mode0644/mode + owner0/owner + group0/group + labelunconfined_u:object_r:virt_image_t:s0/label +/permissions + /backingStore +/volume diff --git a/tests/storagevolxml2xmlout/vol-qcow2-nocow.xml b/tests/storagevolxml2xmlout/vol-qcow2-nocow.xml new file mode 100644 index 000..31dc578 --- /dev/null +++ b/tests/storagevolxml2xmlout/vol-qcow2-nocow.xml @@ -0,0 +1,31 @@ +volume type='file' + nameOtherDemo.img/name + key/var/lib/libvirt/images/OtherDemo.img/key + source + /source + capacity unit='bytes'5368709120/capacity + allocation unit='bytes'294912/allocation + target +path/var/lib/libvirt/images/OtherDemo.img/path +format type='qcow2'/ +permissions + mode0644/mode + owner0/owner + group0/group + labelunconfined_u:object_r:virt_image_t:s0/label +/permissions +encryption format='qcow' + secret type='passphrase' uuid='e78d4b51-a2af-485f-b0f5-afca709a80f4'/ +/encryption + /target + backingStore +path/dev/null/path +format type='raw'/ +permissions + mode0644/mode + owner0/owner + group0/group + labelunconfined_u:object_r:virt_image_t:s0/label +/permissions + /backingStore +/volume -- 1.8.4.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH V3 0/2] storagevol: add 'nocow' option to vol xml
Add 'nocow' option to vol xml, so that user can have a chance to create a volume with NOCOW flag set on btrfs. It improves performance when the volume is a file image on btrfs and is used in VM environment. --- Changes: * fix typo in V2 * add test case for 'nocow', in separate patch V2 is here: http://www.redhat.com/archives/libvir-list/2014-July/msg00361.html Chunyan Liu (2): storagevol: add nocow to vol xml add nocow test case docs/formatstorage.html.in | 7 + docs/schemas/storagevol.rng| 5 src/conf/storage_conf.c| 3 ++ src/storage/storage_backend.c | 22 +++ src/util/virstoragefile.h | 1 + .../storagevolxml2argvdata/qcow2-nocow-compat.argv | 3 ++ tests/storagevolxml2argvdata/qcow2-nocow.argv | 3 ++ tests/storagevolxml2argvtest.c | 6 tests/storagevolxml2xmlin/vol-qcow2-nocow.xml | 32 ++ tests/storagevolxml2xmlout/vol-qcow2-nocow.xml | 31 + 10 files changed, 113 insertions(+) create mode 100644 tests/storagevolxml2argvdata/qcow2-nocow-compat.argv create mode 100644 tests/storagevolxml2argvdata/qcow2-nocow.argv create mode 100644 tests/storagevolxml2xmlin/vol-qcow2-nocow.xml create mode 100644 tests/storagevolxml2xmlout/vol-qcow2-nocow.xml -- 1.8.4.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH V3 1/2] storagevol: add nocow to vol xml
Add 'nocow' to storage volume xml so that user can have an option
to set NOCOW flag to the newly created volume. It's useful on btrfs
file system to enhance performance.
Btrfs has low performance when hosting VM images, even more when the guest
in those VM are also using btrfs as file system. One way to mitigate this
bad performance is to turn off COW attributes on VM files. Generally, there
are two ways to turn off COW on btrfs: a) by mounting fs with nodatacow,
then all newly created files will be NOCOW. b) per file. Add the NOCOW file
attribute. It could only be done to empty or new files.
This patch tries the second way, according to 'nocow' option, it could set
NOCOW flag per file:
for raw file images, handle 'nocow' in libvirt code; for non-raw file images,
pass 'nocow=on' option to qemu-img, and let qemu-img to handle that (requires
qemu-img version = 2.1).
Signed-off-by: Chunyan Liu cy...@suse.com
---
Changes:
* fix typo
docs/formatstorage.html.in| 7 +++
docs/schemas/storagevol.rng | 5 +
src/conf/storage_conf.c | 3 +++
src/storage/storage_backend.c | 22 ++
src/util/virstoragefile.h | 1 +
5 files changed, 38 insertions(+)
diff --git a/docs/formatstorage.html.in b/docs/formatstorage.html.in
index 1cd82b4..8d51402 100644
--- a/docs/formatstorage.html.in
+++ b/docs/formatstorage.html.in
@@ -385,6 +385,7 @@
lt;labelgt;virt_image_tlt;/labelgt;
lt;/permissionsgt;
lt;compatgt;1.1lt;/compatgt;
+ lt;nocow/gt;
lt;featuresgt;
lt;lazy_refcounts/gt;
lt;/featuresgt;
@@ -424,6 +425,12 @@
1.1 is used. If omitted, qemu-img default is used.
span class=sinceSince 1.1.0/span
/dd
+ dtcodenocow/code/dt
+ ddTurn off COW of the newly created volume. So far, this is only valid
+for a file image in btrfs file system. It will improve performance when
+the file image is used in VM. To create non-raw file images, it
+requires QEMU version since 2.1. span class=sinceSince 1.2.6/span
+ /dd
dtcodefeatures/code/dt
ddFormat-specific features. Only used for codeqcow2/code now.
Valid sub-elements are:
diff --git a/docs/schemas/storagevol.rng b/docs/schemas/storagevol.rng
index 3798476..1b2d4cc 100644
--- a/docs/schemas/storagevol.rng
+++ b/docs/schemas/storagevol.rng
@@ -138,6 +138,11 @@
ref name='compat'/
/optional
optional
+ element name='nocow'
+empty/
+ /element
+/optional
+optional
ref name='fileFormatFeatures'/
/optional
/interleave
diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c
index aa29658..f28a314 100644
--- a/src/conf/storage_conf.c
+++ b/src/conf/storage_conf.c
@@ -1285,6 +1285,9 @@ virStorageVolDefParseXML(virStoragePoolDefPtr pool,
virStringFreeList(version);
}
+if (virXPathNode(./target/nocow, ctxt))
+ret-target.nocow = true;
+
if (options-featureFromString virXPathNode(./target/features, ctxt))
{
if ((n = virXPathNodeSet(./target/features/*, ctxt, nodes)) 0)
goto error;
diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c
index 7b17ca4..5f2dc5d 100644
--- a/src/storage/storage_backend.c
+++ b/src/storage/storage_backend.c
@@ -37,6 +37,9 @@
#ifdef __linux__
# include sys/ioctl.h
# include linux/fs.h
+# ifndef FS_NOCOW_FL
+# define FS_NOCOW_FL 0x0080 /* Do not cow file */
+# endif
#endif
#if WITH_SELINUX
@@ -453,6 +456,21 @@ virStorageBackendCreateRaw(virConnectPtr conn
ATTRIBUTE_UNUSED,
goto cleanup;
}
+if (vol-target.nocow) {
+#ifdef __linux__
+int attr;
+
+/* Set NOCOW flag. This is an optimisation for btrfs.
+ * The FS_IOC_SETFLAGS ioctl return value will be ignored since any
+ * failure of this operation should not block the left work.
+ */
+if (ioctl(fd, FS_IOC_GETFLAGS, attr) == 0) {
+attr |= FS_NOCOW_FL;
+ioctl(fd, FS_IOC_SETFLAGS, attr);
+}
+#endif
+}
+
if ((ret = createRawFile(fd, vol, inputvol)) 0)
/* createRawFile already reported the exact error. */
ret = -1;
@@ -718,6 +736,7 @@ virStorageBackendCreateQemuImgOpts(char **opts,
bool preallocate,
int format,
const char *compat,
+ bool nocow,
virBitmapPtr features)
{
virBuffer buf = VIR_BUFFER_INITIALIZER;
@@ -730,6 +749,8 @@ virStorageBackendCreateQemuImgOpts(char **opts,
virBufferAddLit(buf, encryption=on,);
if (preallocate)
virBufferAddLit(buf, preallocation=metadata,);
+if (nocow)
+virBufferAddLit(buf, nocow=on,);
if (compat)
virBufferAsprintf(buf,
[libvirt] [PATCHv3] Rework lxc apparmor profile
Rework the apparmor lxc profile abstraction to mimic ubuntu's container-default.
This profile allows quite a lot, but strives to restrict access to
dangerous resources.
Removing the explicit authorizations to bash, systemd and cron files,
forces them to keep the lxc profile for all applications inside the
container. PUx permissions where leading to running systemd (and others
tasks) unconfined.
Put the generic files, network and capabilities restrictions directly
in the TEMPLATE.lxc: this way, users can restrict them on a per
container basis.
---
Diff to v2:
* Fixed missing goto cleanup
examples/apparmor/Makefile.am | 6 +-
examples/apparmor/TEMPLATE.lxc| 15
examples/apparmor/{TEMPLATE = TEMPLATE.qemu} | 2 +-
examples/apparmor/libvirt-lxc | 119 +++---
src/security/security_apparmor.c | 21 +++--
src/security/virt-aa-helper.c | 29 +--
6 files changed, 149 insertions(+), 43 deletions(-)
create mode 100644 examples/apparmor/TEMPLATE.lxc
rename examples/apparmor/{TEMPLATE = TEMPLATE.qemu} (75%)
diff --git a/examples/apparmor/Makefile.am b/examples/apparmor/Makefile.am
index a741e94..7a20e16 100644
--- a/examples/apparmor/Makefile.am
+++ b/examples/apparmor/Makefile.am
@@ -15,7 +15,8 @@
## http://www.gnu.org/licenses/.
EXTRA_DIST=\
- TEMPLATE\
+ TEMPLATE.qemu \
+ TEMPLATE.lxc\
libvirt-qemu\
libvirt-lxc \
usr.lib.libvirt.virt-aa-helper \
@@ -36,6 +37,7 @@ abstractions_DATA = \
templatesdir = $(apparmordir)/libvirt
templates_DATA = \
- TEMPLATE \
+ TEMPLATE.qemu \
+ TEMPLATE.lxc \
$(NULL)
endif WITH_APPARMOR_PROFILES
diff --git a/examples/apparmor/TEMPLATE.lxc b/examples/apparmor/TEMPLATE.lxc
new file mode 100644
index 000..7b64885
--- /dev/null
+++ b/examples/apparmor/TEMPLATE.lxc
@@ -0,0 +1,15 @@
+#
+# This profile is for the domain whose UUID matches this file.
+#
+
+#include tunables/global
+
+profile LIBVIRT_TEMPLATE {
+ #include abstractions/libvirt-lxc
+
+ # Globally allows everything to run under this profile
+ # These can be narrowed depending on the container's use.
+ file,
+ capability,
+ network,
+}
diff --git a/examples/apparmor/TEMPLATE b/examples/apparmor/TEMPLATE.qemu
similarity index 75%
rename from examples/apparmor/TEMPLATE
rename to examples/apparmor/TEMPLATE.qemu
index 187dec5..008a221 100644
--- a/examples/apparmor/TEMPLATE
+++ b/examples/apparmor/TEMPLATE.qemu
@@ -5,5 +5,5 @@
#include tunables/global
profile LIBVIRT_TEMPLATE {
- #include abstractions/libvirt-driver
+ #include abstractions/libvirt-qemu
}
diff --git a/examples/apparmor/libvirt-lxc b/examples/apparmor/libvirt-lxc
index d404328..4bfb503 100644
--- a/examples/apparmor/libvirt-lxc
+++ b/examples/apparmor/libvirt-lxc
@@ -2,16 +2,115 @@
#include abstractions/base
- # Needed for lxc-enter-namespace
- capability sys_admin,
- capability sys_chroot,
+ umount,
- # Added for lxc-enter-namespace --cmd /bin/bash
- /bin/bash PUx,
+ # ignore DENIED message on / remount
+ deny mount options=(ro, remount) - /,
- /usr/sbin/cron PUx,
- /usr/lib/systemd/systemd PUx,
+ # allow tmpfs mounts everywhere
+ mount fstype=tmpfs,
- /usr/lib/libsystemd-*.so.* mr,
- /usr/lib/libudev-*.so.* mr,
- /etc/ld.so.cache mr,
+ # allow mqueue mounts everywhere
+ mount fstype=mqueue,
+
+ # allow fuse mounts everywhere
+ mount fstype=fuse.*,
+
+ # deny writes in /proc/sys/fs but allow binfmt_misc to be mounted
+ mount fstype=binfmt_misc - /proc/sys/fs/binfmt_misc/,
+ deny @{PROC}/sys/fs/** wklx,
+
+ # allow efivars to be mounted, writing to it will be blocked though
+ mount fstype=efivarfs - /sys/firmware/efi/efivars/,
+
+ # block some other dangerous paths
+ deny @{PROC}/sysrq-trigger rwklx,
+ deny @{PROC}/mem rwklx,
+ deny @{PROC}/kmem rwklx,
+
+ # deny writes in /sys except for /sys/fs/cgroup, also allow
+ # fusectl, securityfs and debugfs to be mounted there (read-only)
+ mount fstype=fusectl - /sys/fs/fuse/connections/,
+ mount fstype=securityfs - /sys/kernel/security/,
+ mount fstype=debugfs - /sys/kernel/debug/,
+ mount fstype=proc - /proc/,
+ mount fstype=sysfs - /sys/,
+ deny /sys/firmware/efi/efivars/** rwklx,
+ deny /sys/kernel/security/** rwklx,
+
+ # generated by: lxc-generate-aa-rules.py container-rules.base
+ deny /proc/sys/[^kn]*{,/**} wklx,
+ deny /proc/sys/k[^e]*{,/**} wklx,
+ deny /proc/sys/ke[^r]*{,/**} wklx,
+ deny /proc/sys/ker[^n]*{,/**} wklx,
+ deny /proc/sys/kern[^e]*{,/**} wklx,
+ deny /proc/sys/kerne[^l]*{,/**} wklx,
+ deny /proc/sys/kernel/[^smhd]*{,/**} wklx,
+ deny /proc/sys/kernel/d[^o]*{,/**} wklx,
+ deny /proc/sys/kernel/do[^m]*{,/**} wklx,
+ deny /proc/sys/kernel/dom[^a]*{,/**} wklx,
+ deny /proc/sys/kernel/doma[^i]*{,/**}
Re: [libvirt] [PATCH 2/2] Rework lxc apparmor profile
Hi Serge,
On Mon, 2014-07-14 at 13:55 +, Serge Hallyn wrote:
Quoting Cédric Bosdonnat (cbosdon...@suse.com):
diff --git a/examples/apparmor/libvirt-lxc b/examples/apparmor/libvirt-lxc
index d404328..4bfb503 100644
--- a/examples/apparmor/libvirt-lxc
+++ b/examples/apparmor/libvirt-lxc
@@ -2,16 +2,115 @@
Hi,
this being a verbatim copy from lxc's policy, is there any plan for
keeping the policy uptodate as the lxc policy is updated?
Well... ATM I have nothing planned to keep it up to date. But I can
write a script to check if there are changes in the lxc policy we could
merge.
Does lxc-enter-namespace --cmd /bin/bash still work? (I would expect so)
Yes, it still works.
diff --git a/src/security/security_apparmor.c
b/src/security/security_apparmor.c
index 1e2a38b..778d233 100644
--- a/src/security/security_apparmor.c
+++ b/src/security/security_apparmor.c
@@ -351,26 +351,36 @@ AppArmorSetSecuritySCSILabel(virSCSIDevicePtr dev
ATTRIBUTE_UNUSED,
static int
AppArmorSecurityManagerProbe(const char *virtDriver ATTRIBUTE_UNUSED)
{
-char *template = NULL;
+char *template_qemu = NULL;
+char *template_lxc = NULL;
int rc = SECURITY_DRIVER_DISABLE;
if (use_apparmor() 0)
return rc;
/* see if template file exists */
-if (virAsprintf(template, %s/TEMPLATE,
+if (virAsprintf(template_qemu, %s/TEMPLATE.qemu,
APPARMOR_DIR /libvirt) == -1)
return rc;
-if (!virFileExists(template)) {
+if (virAsprintf(template_lxc, %s/TEMPLATE.lxc,
+ APPARMOR_DIR /libvirt) == -1)
(This remains a bug, a 'goto cleanup' is needed here)
Oops, indeed... seems like I went blind. Patch v3 just sent with that
fix in.
--
Cedric
+
+if (!virFileExists(template_qemu)) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ _(template \'%s\' does not exist), template_qemu);
+goto cleanup;
+}
+if (!virFileExists(template_lxc)) {
virReportError(VIR_ERR_INTERNAL_ERROR,
- _(template \'%s\' does not exist), template);
+ _(template \'%s\' does not exist), template_lxc);
goto cleanup;
}
rc = SECURITY_DRIVER_ENABLE;
cleanup:
-VIR_FREE(template);
+VIR_FREE(template_qemu);
+VIR_FREE(template_lxc);
return rc;
}
diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c
index d563b98..2a09145 100644
--- a/src/security/virt-aa-helper.c
+++ b/src/security/virt-aa-helper.c
@@ -336,24 +336,20 @@ create_profile(const char *profile, const char
*profile_name,
char *pcontent = NULL;
char *replace_name = NULL;
char *replace_files = NULL;
-char *replace_driver = NULL;
const char *template_name = \nprofile LIBVIRT_TEMPLATE;
const char *template_end = \n};
-const char *template_driver = libvirt-driver;
int tlen, plen;
int fd;
int rc = -1;
-const char *driver_name = qemu;
-
-if (virtType == VIR_DOMAIN_VIRT_LXC)
-driver_name = lxc;
if (virFileExists(profile)) {
vah_error(NULL, 0, _(profile exists));
goto end;
}
-if (virAsprintfQuiet(template, %s/TEMPLATE, APPARMOR_DIR
/libvirt) 0) {
+
+if (virAsprintfQuiet(template, %s/TEMPLATE.%s, APPARMOR_DIR
/libvirt,
+ virDomainVirtTypeToString(virtType)) 0) {
vah_error(NULL, 0, _(template name exceeds maximum length));
goto end;
}
@@ -378,11 +374,6 @@ create_profile(const char *profile, const char
*profile_name,
goto clean_tcontent;
}
-if (strstr(tcontent, template_driver) == NULL) {
-vah_error(NULL, 0, _(no replacement string in template));
-goto clean_tcontent;
-}
-
/* '\nprofile profile_name\0' */
if (virAsprintfQuiet(replace_name, \nprofile %s, profile_name) ==
-1) {
vah_error(NULL, 0, _(could not allocate memory for profile
name));
@@ -397,15 +388,7 @@ create_profile(const char *profile, const char
*profile_name,
goto clean_tcontent;
}
-/* 'libvirt-driver_name\0' */
-if (virAsprintfQuiet(replace_driver, libvirt-%s, driver_name) ==
-1) {
-vah_error(NULL, 0, _(could not allocate memory for profile
driver));
-VIR_FREE(replace_driver);
-goto clean_tcontent;
-}
-
-plen = tlen + strlen(replace_name) - strlen(template_name) +
- strlen(replace_driver) - strlen(template_driver) + 1;
+plen = tlen + strlen(replace_name) - strlen(template_name) + 1;
if (virtType != VIR_DOMAIN_VIRT_LXC)
plen += strlen(replace_files) - strlen(template_end);
@@ -422,9 +405,6 @@ create_profile(const char *profile, const
Re: [libvirt] [PATCH 4/4] util: print errno in virObjectLockableNew
On Tue, Jul 15, 2014 at 04:32:03PM +0800, Jincheng Miao wrote:
In virObjectLockableNew, when virMutexInit fails,
virReportSystemError should use errno to get the right
error number, instead of VIR_ERR_INTERNAL_ERROR.
Signed-off-by: Jincheng Miao jm...@redhat.com
---
src/util/virobject.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/util/virobject.c b/src/util/virobject.c
index 6cb84b4..b5d2c02 100644
--- a/src/util/virobject.c
+++ b/src/util/virobject.c
@@ -220,7 +220,7 @@ void *virObjectLockableNew(virClassPtr klass)
return NULL;
if (virMutexInit(obj-lock) 0) {
-virReportSystemError(VIR_ERR_INTERNAL_ERROR, %s,
+virReportSystemError(errno, %s,
_(Unable to initialize mutex));
I'm not sure errno is set when using our virMutexInit(). Most of the
code uses virReportError instead, I suggest using that. This should
be changed everywhere in the code. Rough idea of the places could be
gotten by the following command:
git grep -nA5 virMutexInit | grep SystemError
but as I said, only rough idea :)
Martin
signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/4] lxc: print ENOTSUP when usernamespace is not supported
On Tue, Jul 15, 2014 at 04:32:01PM +0800, Jincheng Miao wrote:
In lxcContainerStart, when user namespace is not supported,
the virReportSystemError is called. But the first argument
should be ENOTSUPP, instead of VIR_ERR_CONFIG_UNSUPPORTED.
Signed-off-by: Jincheng Miao jm...@redhat.com
---
src/lxc/lxc_container.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c
index 4d89677..343df47 100644
--- a/src/lxc/lxc_container.c
+++ b/src/lxc/lxc_container.c
@@ -2020,7 +2020,7 @@ int lxcContainerStart(virDomainDefPtr def,
VIR_DEBUG(Enable user namespace);
cflags |= CLONE_NEWUSER;
} else {
-virReportSystemError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
+virReportSystemError(ENOTSUP, %s,
_(Kernel doesn't support user namespace));
VIR_FREE(stack);
return -1;
--
1.8.3.1
Using virReportSystemError() with anything else than 'errno' is a
misuse by my standards. Just use virReportError() instead if there's
VIR_ERR_CONFIG_UNSUPPORTED used.
Martin
P.S.: The same applies for following patches.
signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 08/16] conf, schema: add support for memnode elements
On Fri, Jul 11, 2014 at 05:10:53PM +0200, Michal Privoznik wrote:
On 08.07.2014 13:50, Martin Kletzander wrote:
Signed-off-by: Martin Kletzander mklet...@redhat.com
---
docs/formatdomain.html.in | 15 ++
docs/schemas/domaincommon.rng | 17 ++
src/conf/numatune_conf.c | 183 +++--
.../qemuxml2argv-numatune-memnode-no-memory.xml| 30
.../qemuxml2argv-numatune-memnode-nocpu.xml| 25 +++
.../qemuxml2argv-numatune-memnode.xml | 31
.../qemuxml2argv-numatune-memnodes-problematic.xml | 31
tests/qemuxml2argvtest.c | 2 +
.../qemuxml2xmlout-numatune-memnode.xml| 33
tests/qemuxml2xmltest.c| 2 +
10 files changed, 356 insertions(+), 13 deletions(-)
create mode 100644
tests/qemuxml2argvdata/qemuxml2argv-numatune-memnode-no-memory.xml
create mode 100644
tests/qemuxml2argvdata/qemuxml2argv-numatune-memnode-nocpu.xml
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-numatune-memnode.xml
create mode 100644
tests/qemuxml2argvdata/qemuxml2argv-numatune-memnodes-problematic.xml
create mode 100644
tests/qemuxml2xmloutdata/qemuxml2xmlout-numatune-memnode.xml
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index ad87b7c..d845c1b 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -709,6 +709,8 @@
...
lt;numatunegt;
lt;memory mode=strict nodeset=1-4,^3/gt;
+lt;memnode cellid=0 mode=strict nodeset=1/gt;
+lt;memnode cellid=2 mode=preferred nodeset=2/gt;
lt;/numatunegt;
...
lt;/domaingt;
@@ -745,6 +747,19 @@
span class='since'Since 0.9.3/span
/dd
+ dtcodememnode/code/dt
+ dd
+Optional codememnode/code elements can specify memory allocation
+policies per each guest NUMA node. For those nodes having no
+corresponding codememnode/code element, the default from
+element codememory/code will be used. Attribute
codecellid/code
+addresses guest NUMA node for which the settings are applied.
+Attributes codemode/code and codenodeset/code have the same
+meaning and syntax as in codememory/code element.
+
+This setting is not compatible with automatic placement.
+span class='since'QEMU Since 1.2.6/span
1.2.8 actually
Are you suggesting I should wait yet another release or did you mean
1.2.7 by any chance? :)
+ /dd
/dl
+static int
+virDomainNumatuneNodeParseXML(virDomainDefPtr def,
+ xmlXPathContextPtr ctxt)
+{
+int n = 0;;
+int ret = -1;
+size_t i = 0;
+xmlNodePtr *nodes = NULL;
+
+if ((n = virXPathNodeSet(./numatune/memnode, ctxt, nodes)) 0) {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+ _(Cannot extract memnode nodes));
+goto cleanup;
+}
+
+if (!n)
+return 0;
+
+if (def-numatune def-numatune-memory.specified
+def-numatune-memory.placement == VIR_DOMAIN_NUMATUNE_PLACEMENT_AUTO)
{
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
+ _(Per-node binding is not compatible with
+ automatic NUMA placement.));
+goto cleanup;
+}
+
+if (!def-cpu || !def-cpu-ncells) {
+virReportError(VIR_ERR_XML_ERROR, %s,
+ _(Element 'memnode' is invalid without
+ any guest NUMA cells));
+goto cleanup;
+}
+
+if (!def-numatune VIR_ALLOC(def-numatune) 0)
+goto cleanup;
Here you allow def-numatune to be allocated already.
+
+if (VIR_ALLOC_N(def-numatune-mem_nodes, def-cpu-ncells) 0)
+goto cleanup;
Which means, this can exists too. VIR_REALLOC_N() is safer IMO.
But that won't zero the memory. VIR_FREE(); VIR_ALLOC_N(); should
cover all cases.
+
+def-numatune-nmem_nodes = def-cpu-ncells;
+
+for (i = 0; i n; i++) {
+const char *tmp = NULL;
No. s/const//.
+int mode = 0;
+unsigned int cellid = 0;
+virDomainNumatuneNodePtr mem_node = NULL;
+xmlNodePtr cur_node = nodes[i];
+
+tmp = virXMLPropString(cur_node, cellid);
+if (!tmp) {
+virReportError(VIR_ERR_XML_ERROR, %s,
+ _(Missing required cellid attribute
+ in memnode element));
+goto cleanup;
+}
+if (virStrToLong_uip(tmp, NULL, 10, cellid) 0) {
+virReportError(VIR_ERR_XML_ERROR, %s,
+ _(Invalid cellid attribute in memnode element));
Moreover, @tmp is leaked here. And it would be nice to tell users in the error
message @tmp somehow.
You mean if the user has a typo in an integer? I guess that such user
has more issues that that typo then and needs more than that to make
his life easier. ;)
Re: [libvirt] [PATCH 1/2] Introduce virDomainYesNo enum type
On 07/14/2014 06:58 PM, Eric Blake wrote:
On 07/14/2014 10:40 AM, Daniel P. Berrange wrote:
}
-def-os.bios.useserial = VIR_DOMAIN_BIOS_USESERIAL_YES;
+def-os.bios.useserial = VIR_DOMAIN_YES_NO_ENABLED;
} else {
-def-os.bios.useserial = VIR_DOMAIN_BIOS_USESERIAL_NO;
+def-os.bios.useserial = VIR_DOMAIN_YES_NO_DISABLED;
}
if (def-data.spice.filetransfer)
virBufferAsprintf(buf, filetransfer enable='%s'/\n,
-
virDomainGraphicsSpiceAgentFileTransferTypeToString(def-data.spice.filetransfer));
+
virDomainYesNoTypeToString(def-data.spice.filetransfer));
}
I'm not really a fan of this cleanup, as IMHO the result is less clear
harder to follow than the original code.
How so? The original code was very repetitive, with multiple enums (all
with long names) copying the same few enum elements. We're not painting
ourselves into a corner - if any of the replaced enums ever grows a
third value (such as on, hybrid, off), then we just break that one
enum back into a named list rather than using the generic on/off enum.
I'm actually in favor of this cleanup.
Specifically a enum constant name like YES_NO_DISABLED is just awful IMHO
compared to the original desriptive name.
Agreed, my constant names are awful. But it's the original type names I don't
like: I'd expect virDomainGraphicsSpiceAgentFileTransfer to be an enum of
different modes of transfer, not just default/no/yes.
Is it just a matter of coming up with a better name? Maybe:
VIR_TRISTATE_ABSENT = 0,
VIR_TRISTATE_NO,
VIR_TRISTATE_YES,
Without the DOMAIN prefix, this could be used for network_conf.c too.
How about:
VIR_TRISTATE_SWITCH_ABSENT = 0,
VIR_TRISTATE_SWITCH_OFF
VIR_TRISTATE_SWITCH_ON
for the other enum? (And maybe VIR_TRISTATE_BOOL for the first one?)
def-os.bios.useserial = VIR_TRISTATE_NO;
if (def-data.spice.filetransfer) {
virBufferAsprintf(buf, filetransfer enable='%s'/\n,
virTristateToString(def-data.spice.filetransfer));
Jan
signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 09/16] numatune: add support for per-node memory bindings in private APIs
On Fri, Jul 11, 2014 at 05:11:02PM +0200, Michal Privoznik wrote:
On 08.07.2014 13:50, Martin Kletzander wrote:
Signed-off-by: Martin Kletzander mklet...@redhat.com
---
src/conf/numatune_conf.c | 111 ---
src/conf/numatune_conf.h | 14 --
src/libvirt_private.syms | 1 +
src/lxc/lxc_cgroup.c | 3 +-
src/qemu/qemu_cgroup.c | 2 +-
src/qemu/qemu_driver.c | 10 ++---
src/util/virnuma.c | 6 +--
7 files changed, 117 insertions(+), 30 deletions(-)
diff --git a/src/conf/numatune_conf.c b/src/conf/numatune_conf.c
index 67fc799..60b6867 100644
--- a/src/conf/numatune_conf.c
+++ b/src/conf/numatune_conf.c
@@ -63,6 +63,18 @@ struct _virDomainNumatune {
};
+static inline bool
+numa_cell_specified(virDomainNumatunePtr numatune,
Whoa, when I met this function call I thought to myself that this is
some libnuma function. Please name it differently to match our
virSomethingShiny pattern.
I wanted this function to stand out as it is a macro (or static
inline) and I've seen it somewhere else in the code. I changed it to
virDomainNumatuneNodeSpecified(ewww, gross) and all the calls too
(with proper wrapping as well).
+int cellid)
+{
+if (numatune
+cellid = 0
+cellid numatune-nmem_nodes)
+return numatune-mem_nodes[cellid].nodeset;
+
+return false;
+}
+
static int
virDomainNumatuneNodeParseXML(virDomainDefPtr def,
xmlXPathContextPtr ctxt)
@@ -312,6 +324,7 @@ void
virDomainNumatuneFree(virDomainNumatunePtr numatune)
{
size_t i = 0;
+
This change is spurious. Either move it to 8/16 or drop this one.
Done.
[...]
@@ -469,6 +500,35 @@ virDomainNumatuneSet(virDomainDefPtr def,
return ret;
}
+static bool
+virDomainNumatuneNodesEqual(virDomainNumatunePtr n1,
+virDomainNumatunePtr n2)
+{
+size_t i = 0;
+
+if (n1-nmem_nodes != n2-nmem_nodes)
+return false;
+
+for (i = 0; i n1-nmem_nodes; i++) {
+virDomainNumatuneNodePtr nd1 = n1-mem_nodes[i];
+virDomainNumatuneNodePtr nd2 = n2-mem_nodes[i];
+
+if (!nd1-nodeset !nd2-nodeset)
+continue;
So if both are missing nodeset, they are considered equal? What if they
differ in mode?
Yes, because !nd-nodeset means there was no memnode/ with that
cellid. Therefore mode doesn't make sense (and is not set anyway).
Martin
signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2] conf, schema: add support for memnode elements
Signed-off-by: Martin Kletzander mklet...@redhat.com
---
docs/formatdomain.html.in | 15 ++
docs/schemas/domaincommon.rng | 17 ++
src/conf/numatune_conf.c | 187 +++--
.../qemuxml2argv-numatune-memnode-no-memory.xml| 30
.../qemuxml2argv-numatune-memnode-nocpu.xml| 25 +++
.../qemuxml2argv-numatune-memnode.xml | 33
.../qemuxml2argv-numatune-memnodes-problematic.xml | 31
tests/qemuxml2argvtest.c | 2 +
.../qemuxml2xmlout-numatune-memnode.xml| 33
tests/qemuxml2xmltest.c| 2 +
10 files changed, 362 insertions(+), 13 deletions(-)
create mode 100644
tests/qemuxml2argvdata/qemuxml2argv-numatune-memnode-no-memory.xml
create mode 100644
tests/qemuxml2argvdata/qemuxml2argv-numatune-memnode-nocpu.xml
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-numatune-memnode.xml
create mode 100644
tests/qemuxml2argvdata/qemuxml2argv-numatune-memnodes-problematic.xml
create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-numatune-memnode.xml
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 9f1082b..1301639 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -709,6 +709,8 @@
...
lt;numatunegt;
lt;memory mode=strict nodeset=1-4,^3/gt;
+lt;memnode cellid=0 mode=strict nodeset=1/gt;
+lt;memnode cellid=2 mode=preferred nodeset=2/gt;
lt;/numatunegt;
...
lt;/domaingt;
@@ -745,6 +747,19 @@
span class='since'Since 0.9.3/span
/dd
+ dtcodememnode/code/dt
+ dd
+Optional codememnode/code elements can specify memory allocation
+policies per each guest NUMA node. For those nodes having no
+corresponding codememnode/code element, the default from
+element codememory/code will be used. Attribute
codecellid/code
+addresses guest NUMA node for which the settings are applied.
+Attributes codemode/code and codenodeset/code have the same
+meaning and syntax as in codememory/code element.
+
+This setting is not compatible with automatic placement.
+span class='since'QEMU Since 1.2.7/span
+ /dd
/dl
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 155a33e..0b31261 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -789,6 +789,23 @@
/choice
/element
/optional
+ zeroOrMore
+element name=memnode
+ attribute name=cellid
+ref name=unsignedInt/
+ /attribute
+ attribute name=mode
+choice
+ valuestrict/value
+ valuepreferred/value
+ valueinterleave/value
+/choice
+ /attribute
+ attribute name='nodeset'
+ref name='cpuset'/
+ /attribute
+/element
+ /zeroOrMore
/element
/define
diff --git a/src/conf/numatune_conf.c b/src/conf/numatune_conf.c
index 6ce1e2d..a39c028 100644
--- a/src/conf/numatune_conf.c
+++ b/src/conf/numatune_conf.c
@@ -42,17 +42,140 @@ VIR_ENUM_IMPL(virDomainNumatunePlacement,
static,
auto);
+typedef struct _virDomainNumatuneNode virDomainNumatuneNode;
+typedef virDomainNumatuneNode *virDomainNumatuneNodePtr;
+
struct _virDomainNumatune {
struct {
+bool specified;
virBitmapPtr nodeset;
virDomainNumatuneMemMode mode;
virDomainNumatunePlacement placement;
} memory; /* pinning for all the memory */
+struct _virDomainNumatuneNode {
+virBitmapPtr nodeset;
+virDomainNumatuneMemMode mode;
+} *mem_nodes; /* fine tuning per guest node */
+size_t nmem_nodes;
+
/* Future NUMA tuning related stuff should go here. */
};
+static int
+virDomainNumatuneNodeParseXML(virDomainDefPtr def,
+ xmlXPathContextPtr ctxt)
+{
+char *tmp = NULL;
+int n = 0;;
+int ret = -1;
+size_t i = 0;
+xmlNodePtr *nodes = NULL;
+
+if ((n = virXPathNodeSet(./numatune/memnode, ctxt, nodes)) 0) {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+ _(Cannot extract memnode nodes));
+goto cleanup;
+}
+
+if (!n)
+return 0;
+
+if (def-numatune def-numatune-memory.specified
+def-numatune-memory.placement == VIR_DOMAIN_NUMATUNE_PLACEMENT_AUTO)
{
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
+ _(Per-node binding is not compatible with
+ automatic NUMA placement.));
+goto cleanup;
+}
+
+if (!def-cpu || !def-cpu-ncells) {
+virReportError(VIR_ERR_XML_ERROR, %s,
+ _(Element 'memnode' is invalid without
+ any guest NUMA cells));
+goto
[libvirt] [PATCH v2] qemu: pass numa node binding preferences to qemu
Currently, we only bind the whole QEMU domain to memory nodes
specified in nodemask altogether. That, however, doesn't make much
sense when one wants to control from where the memory for particular
guest nodes should be allocated. QEMU allows us to do that by
specifying 'host-nodes' parameter for the 'memory-backend-ram' object,
so let's use that.
Signed-off-by: Martin Kletzander mklet...@redhat.com
---
src/qemu/qemu_command.c| 59 +-
.../qemuxml2argv-numatune-memnode-no-memory.args | 8 +++
.../qemuxml2argv-numatune-memnode.args | 11
tests/qemuxml2argvtest.c | 7 +++
4 files changed, 84 insertions(+), 1 deletion(-)
create mode 100644
tests/qemuxml2argvdata/qemuxml2argv-numatune-memnode-no-memory.args
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-numatune-memnode.args
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index f1dbb34..6235a74 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -150,6 +150,11 @@ VIR_ENUM_IMPL(qemuDomainFSDriver,
VIR_DOMAIN_FS_DRIVER_TYPE_LAST,
NULL,
NULL);
+VIR_ENUM_DECL(qemuNumaPolicy)
+VIR_ENUM_IMPL(qemuNumaPolicy, VIR_DOMAIN_NUMATUNE_MEM_LAST,
+ bind,
+ preferred,
+ interleave);
/**
* qemuPhysIfaceConnect:
@@ -6383,13 +6388,23 @@ qemuBuildNumaArgStr(const virDomainDef *def,
size_t i;
virBuffer buf = VIR_BUFFER_INITIALIZER;
char *cpumask = NULL, *tmpmask = NULL, *next = NULL;
+char *nodemask = NULL;
int ret = -1;
+if (virDomainNumatuneHasPerNodeBinding(def-numatune)
+!virQEMUCapsGet(qemuCaps, QEMU_CAPS_OBJECT_MEMORY_RAM)) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
+ _(Per-node memory binding is not supported
+ with this QEMU));
+goto cleanup;
+}
+
for (i = 0; i def-cpu-ncells; i++) {
int cellmem = VIR_DIV_UP(def-cpu-cells[i].mem, 1024);
def-cpu-cells[i].mem = cellmem * 1024;
VIR_FREE(cpumask);
+VIR_FREE(nodemask);
if (!(cpumask = virBitmapFormat(def-cpu-cells[i].cpumask)))
goto cleanup;
@@ -6402,6 +6417,43 @@ qemuBuildNumaArgStr(const virDomainDef *def,
goto cleanup;
}
+if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_OBJECT_MEMORY_RAM)) {
+virDomainNumatuneMemMode mode;
+const char *policy = NULL;
+
+mode = virDomainNumatuneGetMode(def-numatune, i);
+policy = qemuNumaPolicyTypeToString(mode);
+
+virBufferAsprintf(buf,
memory-backend-ram,size=%dM,id=ram-node%zu,
+ cellmem, i);
+
+if (virDomainNumatuneMaybeFormatNodeset(def-numatune, NULL,
+nodemask, i) 0)
+goto cleanup;
+
+if (nodemask) {
+if (strchr(nodemask, ',')
+!virQEMUCapsGet(qemuCaps, QEMU_CAPS_NUMA)) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
+ _(disjoint NUMA node ranges are not
supported
+ with this QEMU));
+goto cleanup;
+}
+
+for (tmpmask = nodemask; tmpmask; tmpmask = next) {
+if ((next = strchr(tmpmask, ',')))
+*(next++) = '\0';
+virBufferAddLit(buf, ,host-nodes=);
+virBufferAdd(buf, tmpmask, -1);
+}
+
+virBufferAsprintf(buf, ,policy=%s, policy);
+}
+
+virCommandAddArg(cmd, -object);
+virCommandAddArgBuffer(cmd, buf);
+}
+
virCommandAddArg(cmd, -numa);
virBufferAsprintf(buf, node,nodeid=%zu, i);
@@ -6412,7 +6464,11 @@ qemuBuildNumaArgStr(const virDomainDef *def,
virBufferAdd(buf, tmpmask, -1);
}
-virBufferAsprintf(buf, ,mem=%d, cellmem);
+if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_OBJECT_MEMORY_RAM)) {
+virBufferAsprintf(buf, ,memdev=ram-node%zu, i);
+} else {
+virBufferAsprintf(buf, ,mem=%d, cellmem);
+}
virCommandAddArgBuffer(cmd, buf);
}
@@ -6420,6 +6476,7 @@ qemuBuildNumaArgStr(const virDomainDef *def,
cleanup:
VIR_FREE(cpumask);
+VIR_FREE(nodemask);
virBufferFreeAndReset(buf);
return ret;
}
diff --git
a/tests/qemuxml2argvdata/qemuxml2argv-numatune-memnode-no-memory.args
b/tests/qemuxml2argvdata/qemuxml2argv-numatune-memnode-no-memory.args
new file mode 100644
index 000..b0e274c
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-numatune-memnode-no-memory.args
@@ -0,0 +1,8 @@
+LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test QEMU_AUDIO_DRV=none \
+/usr/bin/kvm -S -M pc -m 64 -smp 2 \
+-object
[libvirt] [PATCH 0/4] Resolve const correctness isues
Okay, okay. The approach in 4/4 can be considered hackish, but hey - it works! Michal Privoznik (4): Fix const correctness viralloc: Honor const correctness in VIR_FREE VIR_FREE: Avoid doing side work in callees virFree: Check const correctness src/conf/network_conf.c | 12 src/locking/lock_driver_lockd.c | 2 +- src/qemu/qemu_capabilities.c | 2 +- src/remote/remote_driver.c | 2 +- src/util/viralloc.c | 6 -- src/util/viralloc.h | 33 - src/xenapi/xenapi_utils.c| 3 ++- tools/virsh-domain.c | 4 ++-- tools/wireshark/src/packet-libvirt.c | 6 +++--- tools/wireshark/src/packet-libvirt.h | 4 ++-- 10 files changed, 40 insertions(+), 34 deletions(-) -- 1.8.5.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/4] viralloc: Honor const correctness in VIR_FREE
Since we've corrected all the callers that passed a const pointer to VIR_FREE() we may drop the typecasting horribility and let the macro be a simple wrapper over the virFree() function. Signed-off-by: Michal Privoznik mpriv...@redhat.com --- src/util/viralloc.h | 15 +-- 1 file changed, 1 insertion(+), 14 deletions(-) diff --git a/src/util/viralloc.h b/src/util/viralloc.h index 7125e67..71b4a45 100644 --- a/src/util/viralloc.h +++ b/src/util/viralloc.h @@ -547,20 +547,7 @@ void virFree(void *ptrptr) ATTRIBUTE_NONNULL(1); * * This macro is safe to use on arguments with side effects. */ -# if !STATIC_ANALYSIS -/* The ternary ensures that ptr is a pointer and not an integer type, - * while evaluating ptr only once. This gives us extra compiler - * safety when compiling under gcc. For now, we intentionally cast - * away const, since a number of callers safely pass const char *. - */ -# define VIR_FREE(ptr) virFree((void *) (1 ? (const void *) (ptr) : (ptr))) -# else -/* The Coverity static analyzer considers the else path of the ?: and - * flags the VIR_FREE() of the address of the address of memory as a - * RESOURCE_LEAK resulting in numerous false positives (eg, VIR_FREE(ptr)) - */ -# define VIR_FREE(ptr) virFree((void *) (ptr)) -# endif +# define VIR_FREE(ptr) virFree((ptr)) void virAllocTestInit(void); int virAllocTestCount(void); -- 1.8.5.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 4/4] virFree: Check const correctness
Up to now it's possible to do something like this:
const char *ptr;
ptr = strdup(my example string);
VIR_FREE(ptr);
The problem is, const char * pointers should not be modified (and
freeing them is kind of modification). We should avoid this. A little
trick is used: assigning a const pointer into 'void *' triggers
compiler warning about discarding 'const' qualifier from pointer. So
the virFree() function gains new dummy argument, that is not touched
anyhow, just fulfills the const correctness check duty.
Signed-off-by: Michal Privoznik mpriv...@redhat.com
---
src/util/viralloc.c | 6 --
src/util/viralloc.h | 20
src/xenapi/xenapi_utils.c | 2 +-
3 files changed, 21 insertions(+), 7 deletions(-)
diff --git a/src/util/viralloc.c b/src/util/viralloc.c
index be9f0fe..0134e67 100644
--- a/src/util/viralloc.c
+++ b/src/util/viralloc.c
@@ -372,7 +372,7 @@ void virShrinkN(void *ptrptr, size_t size, size_t
*countptr, size_t toremove)
ignore_value(virReallocN(ptrptr, size, *countptr -= toremove,
false, 0, NULL, NULL, 0));
else {
-virFree(ptrptr);
+virFree(NULL, ptrptr);
*countptr = 0;
}
}
@@ -569,13 +569,15 @@ int virAllocVar(void *ptrptr,
/**
* virFree:
+ * @ptr: dummy pointer to check const correctness
* @ptrptr: pointer to pointer for address of memory to be freed
*
* Release the chunk of memory in the pointer pointed to by
* the 'ptrptr' variable. After release, 'ptrptr' will be
* updated to point to NULL.
*/
-void virFree(void *ptrptr)
+void virFree(void *ptr ATTRIBUTE_UNUSED,
+ void *ptrptr)
{
int save_errno = errno;
diff --git a/src/util/viralloc.h b/src/util/viralloc.h
index 71b4a45..8fbe56f 100644
--- a/src/util/viralloc.h
+++ b/src/util/viralloc.h
@@ -76,7 +76,7 @@ int virAllocVar(void *ptrptr, size_t struct_size, size_t
element_size, size_t co
bool report, int domcode, const char *filename,
const char *funcname, size_t linenr)
ATTRIBUTE_RETURN_CHECK ATTRIBUTE_NONNULL(1);
-void virFree(void *ptrptr) ATTRIBUTE_NONNULL(1);
+void virFree(void *ptr, void *ptrptr) ATTRIBUTE_NONNULL(2);
/**
* VIR_ALLOC:
@@ -543,11 +543,23 @@ void virFree(void *ptrptr) ATTRIBUTE_NONNULL(1);
* @ptr: pointer holding address to be freed
*
* Free the memory stored in 'ptr' and update to point
- * to NULL.
+ * to NULL. Moreover, this macro has a side effect in
+ * form of evaluating passed argument multiple times.
+ * But advantage is, it checks the cont correctness
+ * (that you are not freeing a const pointer).
+ */
+# define VIR_FREE(ptr) virFree(ptr, (ptr))
+
+/**
+ * VIR_FREE_BROKEN:
+ * @ptr: pointer holding address to be freed
*
- * This macro is safe to use on arguments with side effects.
+ * Twin macro of VIR_FREE. While it does evaluate
+ * argument only once, it does not check const
+ * correctness and therefore you want to use it if and
+ * only if necessary.
*/
-# define VIR_FREE(ptr) virFree((ptr))
+# define VIR_FREE_BROKEN(ptr) virFree(NULL, (ptr))
void virAllocTestInit(void);
int virAllocTestCount(void);
diff --git a/src/xenapi/xenapi_utils.c b/src/xenapi/xenapi_utils.c
index a80d136..db555d2 100644
--- a/src/xenapi/xenapi_utils.c
+++ b/src/xenapi/xenapi_utils.c
@@ -50,7 +50,7 @@ xenSessionFree(xen_session *session)
VIR_FREE(session-error_description);
}
/* The session_id member is type of 'const char *'. Sigh. */
-VIR_FREE(session-session_id);
+VIR_FREE_BROKEN(session-session_id);
VIR_FREE(session);
}
--
1.8.5.5
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 3/4] VIR_FREE: Avoid doing side work in callees
There are just two places where we rely on the fact that VIR_FREE()
macro is without any side effects. In the future, this property of the
macro is going to change, so we need the code to be adjusted to deal
with argument passed to VIR_FREE() being evaluated multiple times.
Signed-off-by: Michal Privoznik mpriv...@redhat.com
---
src/conf/network_conf.c | 12
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
index ce4d4d8..d60a60e 100644
--- a/src/conf/network_conf.c
+++ b/src/conf/network_conf.c
@@ -180,8 +180,10 @@ virNetworkDNSTxtDefClear(virNetworkDNSTxtDefPtr def)
static void
virNetworkDNSHostDefClear(virNetworkDNSHostDefPtr def)
{
-while (def-nnames)
-VIR_FREE(def-names[--def-nnames]);
+size_t i;
+
+for (i = 0; i def-nnames; i++)
+VIR_FREE(def-names[i]);
VIR_FREE(def-names);
}
@@ -197,9 +199,11 @@ virNetworkDNSSrvDefClear(virNetworkDNSSrvDefPtr def)
static void
virNetworkDNSDefClear(virNetworkDNSDefPtr def)
{
+size_t i;
+
if (def-forwarders) {
-while (def-nfwds)
-VIR_FREE(def-forwarders[--def-nfwds]);
+for (i = 0; i def-nfwds; i++)
+VIR_FREE(def-forwarders[i]);
VIR_FREE(def-forwarders);
}
if (def-txts) {
--
1.8.5.5
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/4] Fix const correctness
In many places we define a variable as a 'const char *' when in fact
we modify it just a few lines below. Or even free it. We should not do
that.
There's one exception though, in xenSessionFree() xenapi_utils.c. We
are freeing the xen_session structure which is defined in
xen/api/xen_common.h public header. The structure contains session_id
which is type of 'const char *' when in fact it should have been just
'char *'. So I'm leaving this unmodified, just noticing the fact in
comment.
Signed-off-by: Michal Privoznik mpriv...@redhat.com
---
src/locking/lock_driver_lockd.c | 2 +-
src/qemu/qemu_capabilities.c | 2 +-
src/remote/remote_driver.c | 2 +-
src/xenapi/xenapi_utils.c| 1 +
tools/virsh-domain.c | 4 ++--
tools/wireshark/src/packet-libvirt.c | 6 +++---
tools/wireshark/src/packet-libvirt.h | 4 ++--
7 files changed, 11 insertions(+), 10 deletions(-)
diff --git a/src/locking/lock_driver_lockd.c b/src/locking/lock_driver_lockd.c
index 1ca7772..367d0ce 100644
--- a/src/locking/lock_driver_lockd.c
+++ b/src/locking/lock_driver_lockd.c
@@ -243,7 +243,7 @@ static virNetClientPtr
virLockManagerLockDaemonConnectionNew(bool privileged,
{
virNetClientPtr client = NULL;
char *lockdpath;
-const char *daemonPath = NULL;
+char *daemonPath = NULL;
*prog = NULL;
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 37e0588..8271e28 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -2652,7 +2652,7 @@ static int
virQEMUCapsSaveCache(virQEMUCapsPtr qemuCaps, const char *filename)
{
virBuffer buf = VIR_BUFFER_INITIALIZER;
-const char *xml = NULL;
+char *xml = NULL;
int ret = -1;
size_t i;
diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
index 9d8120f..9a1d78f 100644
--- a/src/remote/remote_driver.c
+++ b/src/remote/remote_driver.c
@@ -582,7 +582,7 @@ doRemoteOpen(virConnectPtr conn,
trans_tcp,
} transport;
#ifndef WIN32
-const char *daemonPath = NULL;
+char *daemonPath = NULL;
#endif
/* We handle *ALL* URIs here. The caller has rejected any
diff --git a/src/xenapi/xenapi_utils.c b/src/xenapi/xenapi_utils.c
index 8b28914..a80d136 100644
--- a/src/xenapi/xenapi_utils.c
+++ b/src/xenapi/xenapi_utils.c
@@ -49,6 +49,7 @@ xenSessionFree(xen_session *session)
VIR_FREE(session-error_description[i]);
VIR_FREE(session-error_description);
}
+/* The session_id member is type of 'const char *'. Sigh. */
VIR_FREE(session-session_id);
VIR_FREE(session);
}
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 5a17aff..ba47258 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -10349,8 +10349,8 @@ vshPrepareDiskXML(xmlNodePtr disk_node,
int type)
{
xmlNodePtr cur = NULL;
-const char *disk_type = NULL;
-const char *device_type = NULL;
+char *disk_type = NULL;
+char *device_type = NULL;
xmlNodePtr new_node = NULL;
char *ret = NULL;
diff --git a/tools/wireshark/src/packet-libvirt.c
b/tools/wireshark/src/packet-libvirt.c
index 0fe5f03..5c3c369 100644
--- a/tools/wireshark/src/packet-libvirt.c
+++ b/tools/wireshark/src/packet-libvirt.c
@@ -105,7 +105,7 @@ dissect_xdr_string(tvbuff_t *tvb, proto_tree *tree, XDR
*xdrs, int hf,
}
}
-static gchar *
+static const gchar *
format_xdr_bytes(guint8 *bytes, guint32 length)
{
gchar *buf;
@@ -206,7 +206,7 @@ dissect_xdr_iterable(tvbuff_t *tvb, proto_item *ti, XDR
*xdrs, gint ett, int rhf
static gboolean
dissect_xdr_vector(tvbuff_t *tvb, proto_tree *tree, XDR *xdrs, int hf, gint
ett,
- int rhf, gchar *rtype, guint32 size, vir_xdr_dissector_t
dissect)
+ int rhf, const gchar *rtype, guint32 size,
vir_xdr_dissector_t dissect)
{
goffset start;
proto_item *ti;
@@ -219,7 +219,7 @@ dissect_xdr_vector(tvbuff_t *tvb, proto_tree *tree, XDR
*xdrs, int hf, gint ett,
static gboolean
dissect_xdr_array(tvbuff_t *tvb, proto_tree *tree, XDR *xdrs, int hf, gint ett,
- int rhf, gchar *rtype, guint32 maxlen, vir_xdr_dissector_t
dissect)
+ int rhf, const gchar *rtype, guint32 maxlen,
vir_xdr_dissector_t dissect)
{
goffset start;
proto_item *ti;
diff --git a/tools/wireshark/src/packet-libvirt.h
b/tools/wireshark/src/packet-libvirt.h
index af54407..5f99fdf 100644
--- a/tools/wireshark/src/packet-libvirt.h
+++ b/tools/wireshark/src/packet-libvirt.h
@@ -105,9 +105,9 @@ static gboolean dissect_xdr_bytes(tvbuff_t *tvb, proto_tree
*tree, XDR *xdrs, in
static gboolean dissect_xdr_pointer(tvbuff_t *tvb, proto_tree *tree, XDR
*xdrs, int hf,
vir_xdr_dissector_t dp);
static gboolean dissect_xdr_vector(tvbuff_t *tvb, proto_tree *tree, XDR *xdrs,
int hf, gint ett,
- int rhf, gchar *rtype, guint32 size,
Re: [libvirt] [PATCH 3/4] VIR_FREE: Avoid doing side work in callees
On 07/15/2014 02:38 PM, Michal Privoznik wrote: There are just two places where we rely on the fact that VIR_FREE() macro is without any side effects. In the future, this property of the macro is going to change, so we need the code to be adjusted to deal with argument passed to VIR_FREE() being evaluated multiple times. Signed-off-by: Michal Privoznik mpriv...@redhat.com --- src/conf/network_conf.c | 12 1 file changed, 8 insertions(+), 4 deletions(-) NACK, this completely removes the side effect. You need to adjust the callers for that. IMO we should set nelems to 0 in all *Clear functions, not just virNetworkDNSHostDefClear, after which NULL might be dereferenced based on the nelems variable. Jan signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/2] Fix assignment of comparison against zero
Assign the value we're comparing:
(val = func()) 0
instead of assigning the comparison value:
(val = func() 0)
Both were introduced along with the code,
the TLS tests by commit bd789df in 0.9.4
net events by commit de87691 in 1.2.2.
Note that the event id type fix is a no-op:
vshNetworkEventIdTypeFromString can only return
-1 (failure) and the event is never used or
0 (the only possible event) and the value of 0 0 is still 0.
---
tests/virnettlshelpers.c | 4 ++--
tools/virsh-network.c| 2 +-
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/tests/virnettlshelpers.c b/tests/virnettlshelpers.c
index 47a1b12..6e667d1 100644
--- a/tests/virnettlshelpers.c
+++ b/tests/virnettlshelpers.c
@@ -383,7 +383,7 @@ testTLSGenerateCert(struct testTLSCertReq *req,
* If no 'ca' is set then we are self signing
* the cert. This is done for the root CA certs
*/
-if ((err = gnutls_x509_crt_sign(crt, ca ? ca : crt, privkey) 0)) {
+if ((err = gnutls_x509_crt_sign(crt, ca ? ca : crt, privkey)) 0) {
VIR_WARN(Failed to sign certificate %s, gnutls_strerror(err));
abort();
}
@@ -391,7 +391,7 @@ testTLSGenerateCert(struct testTLSCertReq *req,
/*
* Finally write the new cert out to disk
*/
-if ((err = gnutls_x509_crt_export(crt, GNUTLS_X509_FMT_PEM, buffer, size)
0)) {
+if ((err = gnutls_x509_crt_export(crt, GNUTLS_X509_FMT_PEM, buffer,
size)) 0) {
VIR_WARN(Failed to export certificate %s, gnutls_strerror(err));
abort();
}
diff --git a/tools/virsh-network.c b/tools/virsh-network.c
index 7f4f4ce..fc08b09 100644
--- a/tools/virsh-network.c
+++ b/tools/virsh-network.c
@@ -1238,7 +1238,7 @@ cmdNetworkEvent(vshControl *ctl, const vshCmd *cmd)
vshError(ctl, %s, _(either --list or event type is required));
return false;
}
-if ((event = vshNetworkEventIdTypeFromString(eventName) 0)) {
+if ((event = vshNetworkEventIdTypeFromString(eventName)) 0) {
vshError(ctl, _(unknown event type %s), eventName);
return false;
}
--
1.8.5.5
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/2] Fix condition value assignments in conditions
Split into two patches, as we might want to backport the first one somewhere. Ján Tomko (2): Fix error on fs pool build failure Fix assignment of comparison against zero src/storage/storage_backend_fs.c | 2 +- tests/virnettlshelpers.c | 4 ++-- tools/virsh-network.c| 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) -- 1.8.5.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/2] Fix error on fs pool build failure
https://bugzilla.redhat.com/show_bug.cgi?id=1119592
Introduced by commit 62927dd v0.7.6.
---
src/storage/storage_backend_fs.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c
index 1615c12..bfaa41f 100644
--- a/src/storage/storage_backend_fs.c
+++ b/src/storage/storage_backend_fs.c
@@ -798,7 +798,7 @@ virStorageBackendFileSystemBuild(virConnectPtr conn
ATTRIBUTE_UNUSED,
VIR_DIR_CREATE_FORCE_PERMS |
VIR_DIR_CREATE_ALLOW_EXIST |
(pool-def-type == VIR_STORAGE_POOL_NETFS
-? VIR_DIR_CREATE_AS_UID : 0)) 0)) {
+? VIR_DIR_CREATE_AS_UID : 0))) 0) {
virReportSystemError(-err, _(cannot create path '%s'),
pool-def-target.path);
goto error;
--
1.8.5.5
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] spec: Update polkit dependencies for CVE-2013-4311
Use secured polkit on distros which provide it. However, RHEL-6 will
still allow for older polkit-0.93 rather than forcing polkit-0.96-5
which is not available in all RHEL-6 releases.
Signed-off-by: Jiri Denemark jdene...@redhat.com
---
libvirt.spec.in | 8 ++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/libvirt.spec.in b/libvirt.spec.in
index 8d1acfa..f32ab00 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -535,7 +535,9 @@ BuildRequires: module-init-tools
BuildRequires: cyrus-sasl-devel
%endif
%if %{with_polkit}
-%if 0%{?fedora} = 12 || 0%{?rhel} = 6
+%if 0%{?fedora} = 21 || 0%{?rhel} = 7
+BuildRequires: polkit-devel = 0.112
+%elif 0%{?fedora} = 12 || 0%{?rhel} = 6
BuildRequires: polkit-devel = 0.93
%else
BuildRequires: PolicyKit-devel = 0.6
@@ -698,7 +700,9 @@ Requires: avahi-libs
%endif
%endif
%if %{with_polkit}
-%if 0%{?fedora} = 12 || 0%{?rhel} =6
+%if 0%{?fedora} = 21 || 0%{?rhel} = 7
+Requires: polkit = 0.112
+%elif 0%{?fedora} = 12 || 0%{?rhel} =6
Requires: polkit = 0.93
%else
Requires: PolicyKit = 0.6
--
2.0.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 3/6] storage: backend: fs: Touch up coding style
virStorageBackendFileSystemRefresh() used cleanup label just for error
exits and didn't meet libvirt's standard for braces in one case.
---
src/storage/storage_backend_fs.c | 23 ---
1 file changed, 12 insertions(+), 11 deletions(-)
diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c
index 8b0beea..6455505 100644
--- a/src/storage/storage_backend_fs.c
+++ b/src/storage/storage_backend_fs.c
@@ -840,7 +840,7 @@ virStorageBackendFileSystemRefresh(virConnectPtr conn
ATTRIBUTE_UNUSED,
virReportSystemError(errno,
_(cannot open path '%s'),
pool-def-target.path);
-goto cleanup;
+goto error;
}
while ((direrr = virDirRead(dir, ent, pool-def-target.path)) 0) {
@@ -849,20 +849,20 @@ virStorageBackendFileSystemRefresh(virConnectPtr conn
ATTRIBUTE_UNUSED,
int backingStoreFormat;
if (VIR_ALLOC(vol) 0)
-goto cleanup;
+goto error;
if (VIR_STRDUP(vol-name, ent-d_name) 0)
-goto cleanup;
+goto error;
vol-type = VIR_STORAGE_VOL_FILE;
vol-target.format = VIR_STORAGE_FILE_RAW; /* Real value is filled in
during probe */
if (virAsprintf(vol-target.path, %s/%s,
pool-def-target.path,
vol-name) == -1)
-goto cleanup;
+goto error;
if (VIR_STRDUP(vol-key, vol-target.path) 0)
-goto cleanup;
+goto error;
if ((ret = virStorageBackendProbeTarget(vol-target,
backingStore,
@@ -881,8 +881,9 @@ virStorageBackendFileSystemRefresh(virConnectPtr conn
ATTRIBUTE_UNUSED,
* break virStorageVolTargetDefFormat() generating the line
* format type='...'/. */
backingStoreFormat = VIR_STORAGE_FILE_RAW;
-} else
-goto cleanup;
+} else {
+goto error;
+}
}
/* directory based volume */
@@ -891,7 +892,7 @@ virStorageBackendFileSystemRefresh(virConnectPtr conn
ATTRIBUTE_UNUSED,
if (backingStore != NULL) {
if (VIR_ALLOC(vol-target.backingStore) 0)
-goto cleanup;
+goto error;
vol-target.backingStore-path = backingStore;
vol-target.backingStore-format = backingStoreFormat;
@@ -906,10 +907,10 @@ virStorageBackendFileSystemRefresh(virConnectPtr conn
ATTRIBUTE_UNUSED,
if (VIR_APPEND_ELEMENT(pool-volumes.objs, pool-volumes.count, vol)
0)
-goto cleanup;
+goto error;
}
if (direrr 0)
-goto cleanup;
+goto error;
closedir(dir);
if (statvfs(pool-def-target.path, sb) 0) {
@@ -926,7 +927,7 @@ virStorageBackendFileSystemRefresh(virConnectPtr conn
ATTRIBUTE_UNUSED,
return 0;
- cleanup:
+ error:
if (dir)
closedir(dir);
virStorageVolDefFree(vol);
--
2.0.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/6] storage: backend: Fix formatting of function arguments
--- src/storage/storage_backend.c| 12 ++-- src/storage/storage_backend_fs.c | 6 +++--- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c index 7b17ca4..a36996f 100644 --- a/src/storage/storage_backend.c +++ b/src/storage/storage_backend.c @@ -1455,16 +1455,16 @@ virStorageBackendUpdateVolInfo(virStorageVolDefPtr vol, int ret; if ((ret = virStorageBackendUpdateVolTargetInfo(vol-target, -updateCapacity, -withBlockVolFormat, -openflags)) 0) +updateCapacity, +withBlockVolFormat, +openflags)) 0) return ret; if (vol-backingStore.path (ret = virStorageBackendUpdateVolTargetInfo(vol-backingStore, -updateCapacity, -withBlockVolFormat, -VIR_STORAGE_VOL_OPEN_DEFAULT)) 0) +updateCapacity, +withBlockVolFormat, + VIR_STORAGE_VOL_OPEN_DEFAULT)) 0) return ret; return 0; diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c index 1615c12..5e65f53 100644 --- a/src/storage/storage_backend_fs.c +++ b/src/storage/storage_backend_fs.c @@ -893,9 +893,9 @@ virStorageBackendFileSystemRefresh(virConnectPtr conn ATTRIBUTE_UNUSED, vol-backingStore.path = backingStore; vol-backingStore.format = backingStoreFormat; -ignore_value(virStorageBackendUpdateVolTargetInfo( - vol-backingStore, true, false, - VIR_STORAGE_VOL_OPEN_DEFAULT)); + ignore_value(virStorageBackendUpdateVolTargetInfo(vol-backingStore, + true, false, + VIR_STORAGE_VOL_OPEN_DEFAULT)); /* If this failed, the backing file is currently unavailable, * the capacity, allocation, owner, group and mode are unknown. * An error message was raised, but we just continue. */ -- 2.0.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/6] storage: Track backing store of a volume in the target struct
As we have a nested pointer for storing the backing store of a volume
there's no need to store it in a separate struct.
---
src/conf/storage_conf.c | 53 ---
src/conf/storage_conf.h | 1 -
src/storage/storage_backend.c | 27 +-
src/storage/storage_backend_fs.c | 9 --
src/storage/storage_backend_gluster.c | 20 +
src/storage/storage_backend_logical.c | 11 +---
6 files changed, 72 insertions(+), 49 deletions(-)
diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c
index aa29658..c79aebd 100644
--- a/src/conf/storage_conf.c
+++ b/src/conf/storage_conf.c
@@ -331,7 +331,6 @@ virStorageVolDefFree(virStorageVolDefPtr def)
VIR_FREE(def-source.extents);
virStorageSourceClear(def-target);
-virStorageSourceClear(def-backingStore);
VIR_FREE(def);
}
@@ -1168,6 +1167,7 @@ virStorageVolDefParseXML(virStoragePoolDefPtr pool,
char *allocation = NULL;
char *capacity = NULL;
char *unit = NULL;
+char *backingStore = NULL;
xmlNodePtr node;
xmlNodePtr *nodes = NULL;
size_t i;
@@ -1252,21 +1252,35 @@ virStorageVolDefParseXML(virStoragePoolDefPtr pool,
goto error;
}
-ret-backingStore.path = virXPathString(string(./backingStore/path),
ctxt);
-if (options-formatFromString) {
-char *format = virXPathString(string(./backingStore/format/@type),
ctxt);
-if (format == NULL)
-ret-backingStore.format = options-defaultFormat;
-else
-ret-backingStore.format = (options-formatFromString)(format);
+if ((backingStore = virXPathString(string(./backingStore/path), ctxt))) {
+if (VIR_ALLOC(ret-target.backingStore) 0)
+goto error;
-if (ret-backingStore.format 0) {
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- _(unknown volume format type %s), format);
+ret-target.backingStore-path = backingStore;
+backingStore = NULL;
+
+if (options-formatFromString) {
+char *format =
virXPathString(string(./backingStore/format/@type), ctxt);
+if (format == NULL)
+ret-target.backingStore-format = options-defaultFormat;
+else
+ret-target.backingStore-format =
(options-formatFromString)(format);
+
+if (ret-target.backingStore-format 0) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _(unknown volume format type %s), format);
+VIR_FREE(format);
+goto error;
+}
VIR_FREE(format);
-goto error;
}
-VIR_FREE(format);
+
+if (VIR_ALLOC(ret-target.backingStore-perms) 0)
+goto error;
+if (virStorageDefParsePerms(ctxt, ret-target.backingStore-perms,
+./backingStore/permissions,
+DEFAULT_VOL_PERM_MODE) 0)
+goto error;
}
ret-target.compat = virXPathString(string(./target/compat), ctxt);
@@ -1308,19 +1322,13 @@ virStorageVolDefParseXML(virStoragePoolDefPtr pool,
VIR_FREE(nodes);
}
-if (VIR_ALLOC(ret-backingStore.perms) 0)
-goto error;
-if (virStorageDefParsePerms(ctxt, ret-backingStore.perms,
-./backingStore/permissions,
-DEFAULT_VOL_PERM_MODE) 0)
-goto error;
-
cleanup:
VIR_FREE(nodes);
VIR_FREE(allocation);
VIR_FREE(capacity);
VIR_FREE(unit);
VIR_FREE(type);
+VIR_FREE(backingStore);
return ret;
error:
@@ -1544,9 +1552,10 @@ virStorageVolDefFormat(virStoragePoolDefPtr pool,
def-target, target) 0)
goto cleanup;
-if (def-backingStore.path
+if (def-target.backingStore
virStorageVolTargetDefFormat(options, buf,
- def-backingStore, backingStore) 0)
+ def-target.backingStore,
+ backingStore) 0)
goto cleanup;
virBufferAdjustIndent(buf, -2);
diff --git a/src/conf/storage_conf.h b/src/conf/storage_conf.h
index 47f769b..badf7a3 100644
--- a/src/conf/storage_conf.h
+++ b/src/conf/storage_conf.h
@@ -68,7 +68,6 @@ struct _virStorageVolDef {
virStorageVolSource source;
virStorageSource target;
-virStorageSource backingStore;
};
typedef struct _virStorageVolDefList virStorageVolDefList;
diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c
index a36996f..f5bfdee 100644
--- a/src/storage/storage_backend.c
+++ b/src/storage/storage_backend.c
@@ -844,11 +844,11 @@ virStorageBackendCreateQemuImgCmd(virConnectPtr conn,
}
-if (vol-backingStore.path) {
+if (vol-target.backingStore) {
int accessRetCode = -1;
char
[libvirt] [PATCH 5/6] storage: fs: Properly parse backing store info
Use the backing store parser to properly create the information about a
volume's backing store. Unfortunately as the storage driver isn't
perpared to allow volumes backed by networked filesystems add a
workaroud that will avoid changing the XML output.
---
src/storage/storage_backend_fs.c | 21 -
1 file changed, 16 insertions(+), 5 deletions(-)
diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c
index 0d01dca..728e640 100644
--- a/src/storage/storage_backend_fs.c
+++ b/src/storage/storage_backend_fs.c
@@ -96,16 +96,27 @@ virStorageBackendProbeTarget(virStorageSourcePtr target,
goto cleanup;
if (meta-backingStoreRaw) {
-if (VIR_ALLOC(target-backingStore) 0)
+if (!(target-backingStore = virStorageSourceNewFromBacking(meta)))
goto cleanup;
-target-backingStore-path = meta-backingStoreRaw;
-meta-backingStoreRaw = NULL;
target-backingStore-format = backingStoreFormat;
+/* XXX: Remote storage doesn't play nicely with volumes backed by
+ * remote storage. To avoid trouble, just fake the ou */
+if (!virStorageSourceIsLocalStorage(target-backingStore)) {
+virStorageSourceFree(target-backingStore);
+
+if (VIR_ALLOC(target-backingStore) 0)
+goto cleanup;
+
+target-backingStore-type = VIR_STORAGE_TYPE_FILE;
+target-backingStore-path = meta-backingStoreRaw;
+meta-backingStoreRaw = NULL;
+target-backingStore-format = VIR_STORAGE_FILE_RAW;
+}
+
if (target-backingStore-format == VIR_STORAGE_FILE_AUTO) {
-if (!virStorageIsFile(target-backingStore-path) ||
-(rc = virStorageFileProbeFormat(target-backingStore-path,
+if ((rc = virStorageFileProbeFormat(target-backingStore-path,
-1, -1)) 0) {
/* If the backing file is currently unavailable or is
* accessed via remote protocol only log an error, fake the
--
2.0.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/6] Fix regression with relative backing names in storage pools
Our recent refactors broke relative backing names in storage pools, fix it partially. Peter Krempa (6): storage: backend: Fix formatting of function arguments storage: Track backing store of a volume in the target struct storage: backend: fs: Touch up coding style storage: fs: Process backing store data in virStorageBackendProbeTarget storage: fs: Properly parse backing store info storage: fs: Don't fail volume update if backing store isn't accessible src/conf/storage_conf.c | 53 + src/conf/storage_conf.h | 1 - src/storage/storage_backend.c | 40 ++--- src/storage/storage_backend_fs.c | 103 ++ src/storage/storage_backend_gluster.c | 20 +-- src/storage/storage_backend_logical.c | 11 ++-- 6 files changed, 129 insertions(+), 99 deletions(-) -- 2.0.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: blockcopy: Initialize correct source structure
On 07/15/2014 02:35 AM, Peter Krempa wrote: 4cc1f1a01fb338de939ba88eb933931687b22336 introduced a crash when doing a block copy as virStorageSourceInitChainElement was called on disk-mirror that is still NULL at that point instead of mirror which temporarily holds the mirror source struct until it's fully initialized. This resulted into a crash as a NULL was dereferenced. Reported by: Shanzi Yu s...@redhat.com --- Fortunately unreleased. src/qemu/qemu_driver.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) ACK diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 8d40bc9..c0ad446 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -15309,7 +15309,7 @@ qemuDomainBlockCopy(virDomainObjPtr vm, if (VIR_STRDUP(mirror-path, dest) 0) goto endjob; -if (virStorageSourceInitChainElement(disk-mirror, disk-src, false) 0) +if (virStorageSourceInitChainElement(mirror, disk-src, false) 0) goto endjob; if (qemuDomainPrepareDiskChainElement(driver, vm, mirror, -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 6/6] storage: fs: Don't fail volume update if backing store isn't accessible
When the backing store of a volume wasn't accessible while updating the volume definition the call would fail alltogether. In cases where we currently (incorrectly) treat remote backing stores as local one this might lead to strange errors. Ignore the opening errors until we figure out how to track proper volume metadata. --- src/storage/storage_backend.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c index f5bfdee..fdcaada 100644 --- a/src/storage/storage_backend.c +++ b/src/storage/storage_backend.c @@ -1465,7 +1465,8 @@ virStorageBackendUpdateVolInfo(virStorageVolDefPtr vol, (ret = virStorageBackendUpdateVolTargetInfo(vol-target.backingStore, updateCapacity, withBlockVolFormat, - VIR_STORAGE_VOL_OPEN_DEFAULT)) 0) + VIR_STORAGE_VOL_OPEN_DEFAULT | + VIR_STORAGE_VOL_OPEN_NOERROR) 0)) return ret; return 0; -- 2.0.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 4/6] storage: fs: Process backing store data in virStorageBackendProbeTarget
Move the processing of the backend metadata directly to the helper
instead of passing it through arguments to the function.
---
src/storage/storage_backend_fs.c | 72 ++--
1 file changed, 33 insertions(+), 39 deletions(-)
diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c
index 6455505..0d01dca 100644
--- a/src/storage/storage_backend_fs.c
+++ b/src/storage/storage_backend_fs.c
@@ -61,20 +61,17 @@ VIR_LOG_INIT(storage.storage_backend_fs);
#define VIR_STORAGE_VOL_FS_PROBE_FLAGS (VIR_STORAGE_VOL_FS_OPEN_FLAGS | \
VIR_STORAGE_VOL_OPEN_NOERROR)
-static int ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3)
+static int
virStorageBackendProbeTarget(virStorageSourcePtr target,
- char **backingStore,
- int *backingStoreFormat,
virStorageEncryptionPtr *encryption)
{
+int backingStoreFormat;
int fd = -1;
int ret = -1;
int rc;
virStorageSourcePtr meta = NULL;
struct stat sb;
-*backingStore = NULL;
-*backingStoreFormat = VIR_STORAGE_FILE_AUTO;
if (encryption)
*encryption = NULL;
@@ -95,32 +92,41 @@ virStorageBackendProbeTarget(virStorageSourcePtr target,
if (!(meta = virStorageFileGetMetadataFromFD(target-path,
fd,
VIR_STORAGE_FILE_AUTO,
- backingStoreFormat)))
-goto cleanup;
-
-if (VIR_STRDUP(*backingStore, meta-backingStoreRaw) 0)
+ backingStoreFormat)))
goto cleanup;
-/* Default to success below this point */
-ret = 0;
+if (meta-backingStoreRaw) {
+if (VIR_ALLOC(target-backingStore) 0)
+goto cleanup;
+
+target-backingStore-path = meta-backingStoreRaw;
+meta-backingStoreRaw = NULL;
+target-backingStore-format = backingStoreFormat;
+
+if (target-backingStore-format == VIR_STORAGE_FILE_AUTO) {
+if (!virStorageIsFile(target-backingStore-path) ||
+(rc = virStorageFileProbeFormat(target-backingStore-path,
+-1, -1)) 0) {
+/* If the backing file is currently unavailable or is
+ * accessed via remote protocol only log an error, fake the
+ * format as RAW and continue. Returning -1 here would
+ * disable the whole storage pool, making it unavailable for
+ * even maintenance. */
+target-backingStore-format = VIR_STORAGE_FILE_RAW;
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ _(cannot probe backing volume format: %s),
+ target-backingStore-path);
+ret = -3;
+} else {
+target-backingStore-format = rc;
+}
+}
+}
target-format = meta-format;
-if (*backingStore
-*backingStoreFormat == VIR_STORAGE_FILE_AUTO
-virStorageIsFile(*backingStore)) {
-if ((rc = virStorageFileProbeFormat(*backingStore, -1, -1)) 0) {
-/* If the backing file is currently unavailable, only log an error,
- * but continue. Returning -1 here would disable the whole storage
- * pool, making it unavailable for even maintenance. */
-virReportError(VIR_ERR_INTERNAL_ERROR,
- _(cannot probe backing volume format: %s),
- *backingStore);
-ret = -3;
-} else {
-*backingStoreFormat = rc;
-}
-}
+/* Default to success below this point */
+ret = 0;
if (meta-capacity)
target-capacity = meta-capacity;
@@ -845,8 +851,6 @@ virStorageBackendFileSystemRefresh(virConnectPtr conn
ATTRIBUTE_UNUSED,
while ((direrr = virDirRead(dir, ent, pool-def-target.path)) 0) {
int ret;
-char *backingStore;
-int backingStoreFormat;
if (VIR_ALLOC(vol) 0)
goto error;
@@ -865,8 +869,6 @@ virStorageBackendFileSystemRefresh(virConnectPtr conn
ATTRIBUTE_UNUSED,
goto error;
if ((ret = virStorageBackendProbeTarget(vol-target,
-backingStore,
-backingStoreFormat,
vol-target.encryption)) 0)
{
if (ret == -2) {
/* Silently ignore non-regular files,
@@ -880,7 +882,6 @@ virStorageBackendFileSystemRefresh(virConnectPtr conn
ATTRIBUTE_UNUSED,
* failed: continue with faked RAW format, since AUTO will
* break virStorageVolTargetDefFormat() generating the line
* format
Re: [libvirt] [PATCH 1/4] qemu: fix wrong errno report in qemuMonitorOpenUnix
On 07/15/2014 02:32 AM, Jincheng Miao wrote:
In qemuMonitorOpenUnix, after connect(), virProcessKill will be
invoked when cpid is not empty. But if the qemu-kvm process exited
early, virProcessKill will flush errno as ESRCH, so the wrong
message will be recorded in log as:
error: qemuMonitorOpenUnix:309 : failed to connect to monitor socket: No such
process
After patched:
error : qemuMonitorOpenUnix:312 : failed to connect to monitor socket:
Connection refused
Signed-off-by: Jincheng Miao jm...@redhat.com
---
src/qemu/qemu_monitor.c | 7 +--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index db3dd73..c8284fe 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -293,19 +293,22 @@ qemuMonitorOpenUnix(const char *monitor, pid_t cpid)
}
do {
+int orig_errno;
ret = connect(monfd, (struct sockaddr *) addr, sizeof(addr));
if (ret == 0)
break;
-if ((errno == ENOENT || errno == ECONNREFUSED)
+orig_errno = errno;
+
+if ((orig_errno == ENOENT || orig_errno == ECONNREFUSED)
(!cpid || virProcessKill(cpid, 0) == 0)) {
virProcessKill can be called on cleanup paths. I think it might be wiser
to rewrite virProcessKill() to guarantee that it does not modify errno
than it is to make all callers have to store a temporary.
--
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 4/4] virFree: Check const correctness
On Tue, Jul 15, 2014 at 02:38:36PM +0200, Michal Privoznik wrote: Up to now it's possible to do something like this: const char *ptr; ptr = strdup(my example string); VIR_FREE(ptr); The problem is, const char * pointers should not be modified (and freeing them is kind of modification). We should avoid this. A little trick is used: assigning a const pointer into 'void *' triggers compiler warning about discarding 'const' qualifier from pointer. So the virFree() function gains new dummy argument, that is not touched anyhow, just fulfills the const correctness check duty. Signed-off-by: Michal Privoznik mpriv...@redhat.com --- src/util/viralloc.c | 6 -- src/util/viralloc.h | 20 src/xenapi/xenapi_utils.c | 2 +- 3 files changed, 21 insertions(+), 7 deletions(-) diff --git a/src/util/viralloc.c b/src/util/viralloc.c index be9f0fe..0134e67 100644 --- a/src/util/viralloc.c +++ b/src/util/viralloc.c [...] @@ -569,13 +569,15 @@ int virAllocVar(void *ptrptr, /** * virFree: + * @ptr: dummy pointer to check const correctness * @ptrptr: pointer to pointer for address of memory to be freed * * Release the chunk of memory in the pointer pointed to by * the 'ptrptr' variable. After release, 'ptrptr' will be * updated to point to NULL. */ -void virFree(void *ptrptr) +void virFree(void *ptr ATTRIBUTE_UNUSED, + void *ptrptr) What if you don't add another argument, but just change the void *ptrptr to void **ptrptr. Compiler shouldn't be mad about not knowing the size resulting of de-referencing ptrptr, you get the check you want and keep the macro without side-effects. Martin signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 4/4] virFree: Check const correctness
On 15.07.2014 15:27, Martin Kletzander wrote: On Tue, Jul 15, 2014 at 02:38:36PM +0200, Michal Privoznik wrote: Up to now it's possible to do something like this: const char *ptr; ptr = strdup(my example string); VIR_FREE(ptr); The problem is, const char * pointers should not be modified (and freeing them is kind of modification). We should avoid this. A little trick is used: assigning a const pointer into 'void *' triggers compiler warning about discarding 'const' qualifier from pointer. So the virFree() function gains new dummy argument, that is not touched anyhow, just fulfills the const correctness check duty. Signed-off-by: Michal Privoznik mpriv...@redhat.com --- src/util/viralloc.c | 6 -- src/util/viralloc.h | 20 src/xenapi/xenapi_utils.c | 2 +- 3 files changed, 21 insertions(+), 7 deletions(-) diff --git a/src/util/viralloc.c b/src/util/viralloc.c index be9f0fe..0134e67 100644 --- a/src/util/viralloc.c +++ b/src/util/viralloc.c [...] @@ -569,13 +569,15 @@ int virAllocVar(void *ptrptr, /** * virFree: + * @ptr: dummy pointer to check const correctness * @ptrptr: pointer to pointer for address of memory to be freed * * Release the chunk of memory in the pointer pointed to by * the 'ptrptr' variable. After release, 'ptrptr' will be * updated to point to NULL. */ -void virFree(void *ptrptr) +void virFree(void *ptr ATTRIBUTE_UNUSED, + void *ptrptr) What if you don't add another argument, but just change the void *ptrptr to void **ptrptr. Compiler shouldn't be mad about not knowing the size resulting of de-referencing ptrptr, you get the check you want and keep the macro without side-effects. That won't work. Consider: char *tmp; VIR_FREE(tmp); which in turn is equal to: virFree(tmp); so the tmp is type of 'char **' while virFree() would expect 'void **' which confuses compiler. Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: blockcopy: Initialize correct source structure
On 07/15/14 15:24, Eric Blake wrote: On 07/15/2014 02:35 AM, Peter Krempa wrote: 4cc1f1a01fb338de939ba88eb933931687b22336 introduced a crash when doing a block copy as virStorageSourceInitChainElement was called on disk-mirror that is still NULL at that point instead of mirror which temporarily holds the mirror source struct until it's fully initialized. This resulted into a crash as a NULL was dereferenced. Reported by: Shanzi Yu s...@redhat.com --- Fortunately unreleased. src/qemu/qemu_driver.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) ACK Pushed; Thanks. Peter 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/4] VIR_FREE: Avoid doing side work in callees
On 15.07.2014 14:58, Ján Tomko wrote:
On 07/15/2014 02:38 PM, Michal Privoznik wrote:
There are just two places where we rely on the fact that VIR_FREE()
macro is without any side effects. In the future, this property of the
macro is going to change, so we need the code to be adjusted to deal
with argument passed to VIR_FREE() being evaluated multiple times.
Signed-off-by: Michal Privoznik mpriv...@redhat.com
---
src/conf/network_conf.c | 12
1 file changed, 8 insertions(+), 4 deletions(-)
NACK, this completely removes the side effect. You need to adjust the callers
for that.
Well, the arrays that nelems refer to are freed immediately, but I
agree that we should keep the code clean.
IMO we should set nelems to 0 in all *Clear functions, not just
virNetworkDNSHostDefClear, after which NULL might be dereferenced based on the
nelems variable.
I don't see how that could be possible with current code. The
virNetworkDNSHostDefClear is called only on cleanup paths where
accessing def-names or def-forwarders is not possible anymore.
Jan
Okay, consider this squashed in:
diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
index d60a60e..dcb521f 100644
--- a/src/conf/network_conf.c
+++ b/src/conf/network_conf.c
@@ -184,6 +184,7 @@ virNetworkDNSHostDefClear(virNetworkDNSHostDefPtr def)
for (i = 0; i def-nnames; i++)
VIR_FREE(def-names[i]);
+def-nnames = 0;
VIR_FREE(def-names);
}
@@ -204,6 +205,7 @@ virNetworkDNSDefClear(virNetworkDNSDefPtr def)
if (def-forwarders) {
for (i = 0; i def-nfwds; i++)
VIR_FREE(def-forwarders[i]);
+def-nfwds = 0;
VIR_FREE(def-forwarders);
}
if (def-txts) {
Michal
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/4] viralloc: Honor const correctness in VIR_FREE
On 07/15/2014 06:38 AM, Michal Privoznik wrote: Since we've corrected all the callers that passed a const pointer to VIR_FREE() we may drop the typecasting horribility and let the macro be a simple wrapper over the virFree() function. Signed-off-by: Michal Privoznik mpriv...@redhat.com --- src/util/viralloc.h | 15 +-- 1 file changed, 1 insertion(+), 14 deletions(-) NACK. I agree that we want to fix the macro to not cast away const, but you simplified it too far. diff --git a/src/util/viralloc.h b/src/util/viralloc.h index 7125e67..71b4a45 100644 --- a/src/util/viralloc.h +++ b/src/util/viralloc.h @@ -547,20 +547,7 @@ void virFree(void *ptrptr) ATTRIBUTE_NONNULL(1); * * This macro is safe to use on arguments with side effects. */ -# if !STATIC_ANALYSIS -/* The ternary ensures that ptr is a pointer and not an integer type, - * while evaluating ptr only once. This gives us extra compiler - * safety when compiling under gcc. For now, we intentionally cast - * away const, since a number of callers safely pass const char *. - */ -# define VIR_FREE(ptr) virFree((void *) (1 ? (const void *) (ptr) : (ptr))) This definition was doing two things - casting away const, AND doing type validation. virFree(foo) will compile even if foo is an int, but we want to ensure that foo is of type char*. I think what you want is: # define VIR_FREE(ptr) virFree(1 ? (ptr) : (ptr)) -# else -/* The Coverity static analyzer considers the else path of the ?: and - * flags the VIR_FREE() of the address of the address of memory as a - * RESOURCE_LEAK resulting in numerous false positives (eg, VIR_FREE(ptr)) - */ -# define VIR_FREE(ptr) virFree((void *) (ptr)) and keep this alternative to hush up coverity. -- 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 4/4] virFree: Check const correctness
On 07/15/2014 06:38 AM, Michal Privoznik wrote: Up to now it's possible to do something like this: const char *ptr; ptr = strdup(my example string); VIR_FREE(ptr); The problem is, const char * pointers should not be modified (and freeing them is kind of modification). We should avoid this. A little trick is used: assigning a const pointer into 'void *' triggers compiler warning about discarding 'const' qualifier from pointer. So the virFree() function gains new dummy argument, that is not touched anyhow, just fulfills the const correctness check duty. Signed-off-by: Michal Privoznik mpriv...@redhat.com --- src/util/viralloc.c | 6 -- src/util/viralloc.h | 20 src/xenapi/xenapi_utils.c | 2 +- 3 files changed, 21 insertions(+), 7 deletions(-) But if you take my suggestion in 2/4 about merely removing the 'cast-away-const' while still keeping type safety, then a single-argument virFree() should still be noisy on attempts to VIR_FREE a const pointer. @@ -543,11 +543,23 @@ void virFree(void *ptrptr) ATTRIBUTE_NONNULL(1); * @ptr: pointer holding address to be freed * * Free the memory stored in 'ptr' and update to point - * to NULL. + * to NULL. Moreover, this macro has a side effect in + * form of evaluating passed argument multiple times. NACK. I think it is possible to use sizeof() to come up with a construct that will only do side effects once, rather than having to weaken the guarantee of VIR_FREE. Please give me some time to propose an alternative. -- 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 2/4] viralloc: Honor const correctness in VIR_FREE
On 07/15/2014 09:41 AM, Eric Blake wrote: ...snip... This definition was doing two things - casting away const, AND doing type validation. virFree(foo) will compile even if foo is an int, but we want to ensure that foo is of type char*. I think what you want is: # define VIR_FREE(ptr) virFree(1 ? (ptr) : (ptr)) -# else -/* The Coverity static analyzer considers the else path of the ?: and - * flags the VIR_FREE() of the address of the address of memory as a - * RESOURCE_LEAK resulting in numerous false positives (eg, VIR_FREE(ptr)) - */ -# define VIR_FREE(ptr) virFree((void *) (ptr)) and keep this alternative to hush up coverity. FWIW: I got notice a couple weeks ago that Coverity 7.5.0 (released June 27, 2014) has a fix for this issue. Since there's no guarantee that everyone who runs a Coverity scan will update to that new version - I'd be hesitant to say that we could just do away with it since without this condition- Coverity will be very noisy. John -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/4] VIR_FREE: Avoid doing side work in callees
On 07/15/2014 03:38 PM, Michal Privoznik wrote: On 15.07.2014 14:58, Ján Tomko wrote: On 07/15/2014 02:38 PM, Michal Privoznik wrote: IMO we should set nelems to 0 in all *Clear functions, not just virNetworkDNSHostDefClear, after which NULL might be dereferenced based on the nelems variable. I don't see how that could be possible with current code. The virNetworkDNSHostDefClear is called only on cleanup paths where accessing def-names or def-forwarders is not possible anymore. In virNetworkDefUpdateDNSHost when parsing in virNetworkDNSHostDefParseXML fails, DNSHostDefClear is called in both functions. Jan signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/2] Fix condition value assignments in conditions
On Tue, Jul 15, 2014 at 02:59:50PM +0200, Ján Tomko wrote: Split into two patches, as we might want to backport the first one somewhere. Ján Tomko (2): Fix error on fs pool build failure Fix assignment of comparison against zero src/storage/storage_backend_fs.c | 2 +- tests/virnettlshelpers.c | 4 ++-- tools/virsh-network.c| 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) -- 1.8.5.5 ACK series. Maybe libvirt will now work after 4 years (the commit in 1/2) :) Martin signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] spec: Update polkit dependencies for CVE-2013-4311
On 07/15/2014 07:23 AM, Jiri Denemark wrote:
Use secured polkit on distros which provide it. However, RHEL-6 will
still allow for older polkit-0.93 rather than forcing polkit-0.96-5
which is not available in all RHEL-6 releases.
Signed-off-by: Jiri Denemark jdene...@redhat.com
---
libvirt.spec.in | 8 ++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/libvirt.spec.in b/libvirt.spec.in
index 8d1acfa..f32ab00 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -535,7 +535,9 @@ BuildRequires: module-init-tools
BuildRequires: cyrus-sasl-devel
%endif
%if %{with_polkit}
-%if 0%{?fedora} = 12 || 0%{?rhel} = 6
+%if 0%{?fedora} = 21 || 0%{?rhel} = 7
+BuildRequires: polkit-devel = 0.112
Fedora 20 has polkit-devel-0.112-2 - so we might as well use = 20 in
that conditional.
%if %{with_polkit}
-%if 0%{?fedora} = 12 || 0%{?rhel} =6
+%if 0%{?fedora} = 21 || 0%{?rhel} = 7
+Requires: polkit = 0.112
and again.
ACK with that tweak.
--
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 2/2] Rework lxc apparmor profile
Quoting Cedric Bosdonnat (cbosdon...@suse.com):
Hi Serge,
On Mon, 2014-07-14 at 13:55 +, Serge Hallyn wrote:
Quoting Cédric Bosdonnat (cbosdon...@suse.com):
diff --git a/examples/apparmor/libvirt-lxc b/examples/apparmor/libvirt-lxc
index d404328..4bfb503 100644
--- a/examples/apparmor/libvirt-lxc
+++ b/examples/apparmor/libvirt-lxc
@@ -2,16 +2,115 @@
Hi,
this being a verbatim copy from lxc's policy, is there any plan for
keeping the policy uptodate as the lxc policy is updated?
Well... ATM I have nothing planned to keep it up to date. But I can
write a script to check if there are changes in the lxc policy we could
merge.
Does lxc-enter-namespace --cmd /bin/bash still work? (I would expect so)
Yes, it still works.
diff --git a/src/security/security_apparmor.c
b/src/security/security_apparmor.c
index 1e2a38b..778d233 100644
--- a/src/security/security_apparmor.c
+++ b/src/security/security_apparmor.c
@@ -351,26 +351,36 @@ AppArmorSetSecuritySCSILabel(virSCSIDevicePtr dev
ATTRIBUTE_UNUSED,
static int
AppArmorSecurityManagerProbe(const char *virtDriver ATTRIBUTE_UNUSED)
{
-char *template = NULL;
+char *template_qemu = NULL;
+char *template_lxc = NULL;
int rc = SECURITY_DRIVER_DISABLE;
if (use_apparmor() 0)
return rc;
/* see if template file exists */
-if (virAsprintf(template, %s/TEMPLATE,
+if (virAsprintf(template_qemu, %s/TEMPLATE.qemu,
APPARMOR_DIR /libvirt) == -1)
return rc;
-if (!virFileExists(template)) {
+if (virAsprintf(template_lxc, %s/TEMPLATE.lxc,
+ APPARMOR_DIR /libvirt) == -1)
(This remains a bug, a 'goto cleanup' is needed here)
Oops, indeed... seems like I went blind. Patch v3 just sent with that
fix in.
Oh, bother. I think I just deleted that. Assuming that was the only change,
Acked-by: Serge E. Hallyn serge.hal...@ubuntu.com
--
Cedric
+
+if (!virFileExists(template_qemu)) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ _(template \'%s\' does not exist),
template_qemu);
+goto cleanup;
+}
+if (!virFileExists(template_lxc)) {
virReportError(VIR_ERR_INTERNAL_ERROR,
- _(template \'%s\' does not exist), template);
+ _(template \'%s\' does not exist),
template_lxc);
goto cleanup;
}
rc = SECURITY_DRIVER_ENABLE;
cleanup:
-VIR_FREE(template);
+VIR_FREE(template_qemu);
+VIR_FREE(template_lxc);
return rc;
}
diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c
index d563b98..2a09145 100644
--- a/src/security/virt-aa-helper.c
+++ b/src/security/virt-aa-helper.c
@@ -336,24 +336,20 @@ create_profile(const char *profile, const char
*profile_name,
char *pcontent = NULL;
char *replace_name = NULL;
char *replace_files = NULL;
-char *replace_driver = NULL;
const char *template_name = \nprofile LIBVIRT_TEMPLATE;
const char *template_end = \n};
-const char *template_driver = libvirt-driver;
int tlen, plen;
int fd;
int rc = -1;
-const char *driver_name = qemu;
-
-if (virtType == VIR_DOMAIN_VIRT_LXC)
-driver_name = lxc;
if (virFileExists(profile)) {
vah_error(NULL, 0, _(profile exists));
goto end;
}
-if (virAsprintfQuiet(template, %s/TEMPLATE, APPARMOR_DIR
/libvirt) 0) {
+
+if (virAsprintfQuiet(template, %s/TEMPLATE.%s, APPARMOR_DIR
/libvirt,
+ virDomainVirtTypeToString(virtType)) 0) {
vah_error(NULL, 0, _(template name exceeds maximum length));
goto end;
}
@@ -378,11 +374,6 @@ create_profile(const char *profile, const char
*profile_name,
goto clean_tcontent;
}
-if (strstr(tcontent, template_driver) == NULL) {
-vah_error(NULL, 0, _(no replacement string in template));
-goto clean_tcontent;
-}
-
/* '\nprofile profile_name\0' */
if (virAsprintfQuiet(replace_name, \nprofile %s, profile_name) ==
-1) {
vah_error(NULL, 0, _(could not allocate memory for profile
name));
@@ -397,15 +388,7 @@ create_profile(const char *profile, const char
*profile_name,
goto clean_tcontent;
}
-/* 'libvirt-driver_name\0' */
-if (virAsprintfQuiet(replace_driver, libvirt-%s, driver_name) ==
-1) {
-vah_error(NULL, 0, _(could not allocate memory for profile
driver));
-VIR_FREE(replace_driver);
-goto clean_tcontent;
-}
-
-plen = tlen + strlen(replace_name) -
Re: [libvirt] [PATCH] spec: Update polkit dependencies for CVE-2013-4311
On Tue, Jul 15, 2014 at 07:56:28 -0600, Eric Blake wrote:
On 07/15/2014 07:23 AM, Jiri Denemark wrote:
Use secured polkit on distros which provide it. However, RHEL-6 will
still allow for older polkit-0.93 rather than forcing polkit-0.96-5
which is not available in all RHEL-6 releases.
Signed-off-by: Jiri Denemark jdene...@redhat.com
---
libvirt.spec.in | 8 ++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/libvirt.spec.in b/libvirt.spec.in
index 8d1acfa..f32ab00 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -535,7 +535,9 @@ BuildRequires: module-init-tools
BuildRequires: cyrus-sasl-devel
%endif
%if %{with_polkit}
-%if 0%{?fedora} = 12 || 0%{?rhel} = 6
+%if 0%{?fedora} = 21 || 0%{?rhel} = 7
+BuildRequires: polkit-devel = 0.112
Fedora 20 has polkit-devel-0.112-2 - so we might as well use = 20 in
that conditional.
%if %{with_polkit}
-%if 0%{?fedora} = 12 || 0%{?rhel} =6
+%if 0%{?fedora} = 21 || 0%{?rhel} = 7
+Requires: polkit = 0.112
and again.
ACK with that tweak.
Fixed and pushed, thanks.
Jirka
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PREPOST 15/17] src/xenxs:Refactor Vif formating code
Thanks for the review, am applying changes to address your comments.
On Tue, Jul 15, 2014 at 1:49 AM, Jim Fehlig jfeh...@suse.com wrote:
David Kiarie wrote:
From: Kiarie Kahurani davidkiar...@gmail.com
Introduce the function
xenFormatXMDomainNet(..)
On the parsing side, you called this xenParseXMVif. To be consistent,
this should be xenFormatXMVif.
I think this could be done in a cleaner way
signed-of-by: David Kiariedavidkiar...@gmail.com
---
src/xenxs/xen_xm.c | 49 +++--
1 file changed, 31 insertions(+), 18 deletions(-)
diff --git a/src/xenxs/xen_xm.c b/src/xenxs/xen_xm.c
index ee5dc19..8dd2823 100644
--- a/src/xenxs/xen_xm.c
+++ b/src/xenxs/xen_xm.c
@@ -1821,6 +1821,36 @@ static int xenFormatXMCharDev(virConfPtr conf,
virDomainDefPtr def)
cleanup:
return -1;
}
+static int xenFormatXMDomainNet(virConfPtr conf, virConnectPtr conn,
+ virDomainDefPtr def, int xendConfigVersion)
+{
+ virConfValuePtr netVal = NULL;
+ size_t i;
+
+ int hvm = STREQ(def-os.type, hvm);
+
+ if (VIR_ALLOC(netVal) 0)
+goto cleanup;
+netVal-type = VIR_CONF_LIST;
+netVal-list = NULL;
+
+for (i = 0; i def-nnets; i++) {
+if (xenFormatXMNet(conn, netVal, def-nets[i],
+ hvm, xendConfigVersion) 0)
Ah, I see xenFormatXMNet already exists. So maybe xenFormatXMVifs for
the list of VIFs and xenFormatXMVif for each individual VIF is clearer.
Regards,
Jim
+ return -1;
+}
+if (netVal-list != NULL) {
+int ret = virConfSetValue(conf, vif, netVal);
+netVal = NULL;
+if (ret 0)
+return -1;
+}
+VIR_FREE(netVal);
+return 0;
+ cleanup:
+VIR_FREE(netVal);
+return -1;
+}
virConfPtr xenFormatXM(virConnectPtr conn,
virDomainDefPtr def,
int xendConfigVersion)
@@ -2124,25 +2154,8 @@ virConfPtr xenFormatXM(virConnectPtr conn,
goto cleanup;
}
VIR_FREE(diskVal);
-
-if (VIR_ALLOC(netVal) 0)
+if (xenFormatXMDomainNet(conf, conn, def, xendConfigVersion) 0)
goto cleanup;
-netVal-type = VIR_CONF_LIST;
-netVal-list = NULL;
-
-for (i = 0; i def-nnets; i++) {
-if (xenFormatXMNet(conn, netVal, def-nets[i],
- hvm, xendConfigVersion) 0)
-goto cleanup;
-}
-if (netVal-list != NULL) {
-int ret = virConfSetValue(conf, vif, netVal);
-netVal = NULL;
-if (ret 0)
-goto cleanup;
-}
-VIR_FREE(netVal);
-
if (xenFormatXMPCI(conf, def) 0)
goto cleanup;
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/6] storage: backend: Fix formatting of function arguments
On 07/15/2014 07:25 AM, Peter Krempa wrote: --- src/storage/storage_backend.c| 12 ++-- src/storage/storage_backend_fs.c | 6 +++--- 2 files changed, 9 insertions(+), 9 deletions(-) ACK; mechanical. -- 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/6] storage: backend: fs: Touch up coding style
On 07/15/2014 07:25 AM, Peter Krempa wrote: virStorageBackendFileSystemRefresh() used cleanup label just for error exits and didn't meet libvirt's standard for braces in one case. --- src/storage/storage_backend_fs.c | 23 --- 1 file changed, 12 insertions(+), 11 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] [PATCHv3] Rework lxc apparmor profile
On 07/15/2014 03:02 AM, Cédric Bosdonnat wrote: Rework the apparmor lxc profile abstraction to mimic ubuntu's container-default. This profile allows quite a lot, but strives to restrict access to dangerous resources. Removing the explicit authorizations to bash, systemd and cron files, forces them to keep the lxc profile for all applications inside the container. PUx permissions where leading to running systemd (and others tasks) unconfined. Put the generic files, network and capabilities restrictions directly in the TEMPLATE.lxc: this way, users can restrict them on a per container basis. --- Diff to v2: * Fixed missing goto cleanup Will push shortly, based on the ack given here: https://www.redhat.com/archives/libvir-list/2014-July/msg00745.html -- 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/2] Introduce virDomainYesNo enum type
On 07/15/2014 04:41 AM, Ján Tomko wrote: Is it just a matter of coming up with a better name? Maybe: VIR_TRISTATE_ABSENT = 0, VIR_TRISTATE_NO, VIR_TRISTATE_YES, Without the DOMAIN prefix, this could be used for network_conf.c too. How about: VIR_TRISTATE_SWITCH_ABSENT = 0, VIR_TRISTATE_SWITCH_OFF VIR_TRISTATE_SWITCH_ON for the other enum? (And maybe VIR_TRISTATE_BOOL for the first one?) Yes, that seems reasonable as well. Looking forward to the v2. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 07/16] numatune: Encapsulate numatune configuration in order to unify results
On 07/15/2014 02:44 AM, Michal Privoznik wrote: I take the 'const' as a sign of the fact that I won't be modifying any part of the string. Just adding 'const' to a pointer should be perfectly OK, but I have not objections to your idea, so I squashed this in: Well, I look at free()-ing as modification of the pointee. Therefore freeing a const pointer is in fact its modification and hence should be rejected. I agree. It's just that our VIR_FREE throws away the const-ness of passed pointers. Maybe (as completely separate patchset) we may fix the VIR_FREE() macro which is obviously const-incorrect. That's due to the number of legacy callers that were already const-incorrect at the time I beefed up VIR_FREE months ago to be more type-safe. But now that the tree is a lot cleaner, I'm in favor of such a cleanup (I see you already started it, but I had more comments in that thread, and now I'm on the hook to provide a v2...). -- 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] Enable kvm on aarch64, Cleanup F-16/18 conditionals
reposting to the upstream list - we'd like to keep the downstream .spec
file in sync with upstream, rather than needlessly diverging.
On 07/15/2014 10:52 AM, Peter Robinson wrote:
commit ae37ed3500672f12383825c84dd5ae940fb90ff8
Author: Peter Robinson pbrobin...@gmail.com
Date: Tue Jul 15 17:52:18 2014 +0100
Enable kvm on aarch64, Cleanup F-16/18 conditionals
libvirt.spec | 28
1 files changed, 8 insertions(+), 20 deletions(-)
---
diff --git a/libvirt.spec b/libvirt.spec
index fad21d9..213760d 100644
--- a/libvirt.spec
+++ b/libvirt.spec
@@ -55,14 +55,10 @@
%define with_qemu_tcg %{with_qemu}
# Change if we ever provide qemu-kvm binaries on non-x86 hosts
-%if 0%{?fedora} = 18
-%if 0%{?fedora} = 20
-%define qemu_kvm_arches%{ix86} x86_64 ppc64 s390x %{arm}
-%else
-%define qemu_kvm_arches%{ix86} x86_64 ppc64 s390x
-%endif
+%if 0%{?fedora} = 20
+%define qemu_kvm_arches%{ix86} x86_64 %{power64} s390x %{arm} aarch64
%else
-%define qemu_kvm_arches%{ix86} x86_64
+%define qemu_kvm_arches%{ix86} x86_64 %{power64} s390x
%endif
%ifarch %{qemu_kvm_arches}
@@ -212,18 +208,6 @@
%define with_xen 0
%endif
-# Fedora doesn't have any QEMU on ppc64 until FC16 - only ppc
-%if 0%{?fedora} 0%{?fedora} 16
-%ifarch ppc64
-%define with_qemu 0
-%endif
-%endif
-
-# Fedora doesn't have new enough Xen for libxl until F18
-%if 0%{?fedora} 0%{?fedora} 18
-%define with_libxl 0
-%endif
-
# PolicyKit was introduced in Fedora 8 / RHEL-6 or newer
%if 0%{?fedora} = 8 || 0%{?rhel} = 6
%define with_polkit0%{!?_without_polkit:1}
@@ -385,7 +369,7 @@
Summary: Library providing a simple virtualization API
Name: libvirt
Version: 1.2.6
-Release: 1%{?dist}%{?extra_release}
+Release: 2%{?dist}%{?extra_release}
License: LGPLv2+
Group: Development/Libraries
BuildRoot: %{_tmppath}/%{name}-%{version}-%{release}-root
@@ -2236,6 +2220,10 @@ exit 0
%doc examples/systemtap
%changelog
+* Tue Jul 15 2014 Peter Robinson pbrobin...@fedoraproject.org 1.2.6-2
+- Enable kvm on aarch64
+- Cleanup F-16/18 conditionals
+
* Wed Jul 2 2014 Daniel P. Berrange berra...@redhat.com - 1.2.6-1
- Update to 1.2.6 release
--
Eric Blake eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/3] libxl: support hotplug of interface device
Chunyan Liu wrote: This patch series is to add support for attach/detaching an interface device. At the same time, add two fixes (1/3 and 3/3) Chunyan Liu (3): libxl: add HOSTDEV type in libxlDomainDetachDeviceConfig libxl: support hotplug of interface libxl: fix return value error Attach|DetachDeviceFlags .gnulib | 2 +- Odd that your cover letter includes the gnulib bump but patches do not. Did you use an old cover letter? Note: Chunyan and I have iterated on this series off-list. We started looking at an internal bug report, stumbled across further issues, and this series was the result. ACK series. I'll push this in a bit. Regards, Jim src/libxl/libxl_domain.c | 12 ++- src/libxl/libxl_driver.c | 193 +-- 3 files changed, 180 insertions(+), 27 deletions(-) -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Enable kvm on aarch64, Cleanup F-16/18 conditionals
On Tue, Jul 15, 2014 at 11:14:54AM -0600, Eric Blake wrote:
reposting to the upstream list - we'd like to keep the downstream .spec
file in sync with upstream, rather than needlessly diverging.
On 07/15/2014 10:52 AM, Peter Robinson wrote:
commit ae37ed3500672f12383825c84dd5ae940fb90ff8
Author: Peter Robinson pbrobin...@gmail.com
Date: Tue Jul 15 17:52:18 2014 +0100
Enable kvm on aarch64, Cleanup F-16/18 conditionals
libvirt.spec | 28
1 files changed, 8 insertions(+), 20 deletions(-)
---
diff --git a/libvirt.spec b/libvirt.spec
index fad21d9..213760d 100644
--- a/libvirt.spec
+++ b/libvirt.spec
@@ -55,14 +55,10 @@
%define with_qemu_tcg %{with_qemu}
# Change if we ever provide qemu-kvm binaries on non-x86 hosts
-%if 0%{?fedora} = 18
-%if 0%{?fedora} = 20
-%define qemu_kvm_arches%{ix86} x86_64 ppc64 s390x %{arm}
-%else
-%define qemu_kvm_arches%{ix86} x86_64 ppc64 s390x
-%endif
+%if 0%{?fedora} = 20
+%define qemu_kvm_arches%{ix86} x86_64 %{power64} s390x %{arm}
aarch64
%else
-%define qemu_kvm_arches%{ix86} x86_64
+%define qemu_kvm_arches%{ix86} x86_64 %{power64} s390x
%endif
%ifarch %{qemu_kvm_arches}
@@ -212,18 +208,6 @@
%define with_xen 0
%endif
-# Fedora doesn't have any QEMU on ppc64 until FC16 - only ppc
-%if 0%{?fedora} 0%{?fedora} 16
-%ifarch ppc64
-%define with_qemu 0
-%endif
-%endif
-
-# Fedora doesn't have new enough Xen for libxl until F18
-%if 0%{?fedora} 0%{?fedora} 18
-%define with_libxl 0
-%endif
-
Historically we've kept support for building libvirt against
Fedora versions even if unsupported by Fedora itself, because
we've found people often stick on old Fedora versions. At some
point though it does get a bit insane. eg we don't really need
Fedora 8 support at this point. If we are going todo a cleanup
though, we should do it throughout the spec, not just in these
few places
# PolicyKit was introduced in Fedora 8 / RHEL-6 or newer
%if 0%{?fedora} = 8 || 0%{?rhel} = 6
%define with_polkit0%{!?_without_polkit:1}
@@ -385,7 +369,7 @@
Summary: Library providing a simple virtualization API
Name: libvirt
Version: 1.2.6
-Release: 1%{?dist}%{?extra_release}
+Release: 2%{?dist}%{?extra_release}
License: LGPLv2+
Group: Development/Libraries
BuildRoot: %{_tmppath}/%{name}-%{version}-%{release}-root
@@ -2236,6 +2220,10 @@ exit 0
%doc examples/systemtap
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 0/2] Implement interface stats for BSD
Michal Privoznik wrote: ACK to both patches. Pushed, thanks! Roman Bogorodskiy pgpdVFTqkiyz4.pgp Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/6] storage: Track backing store of a volume in the target struct
On 07/15/2014 07:25 AM, Peter Krempa wrote: As we have a nested pointer for storing the backing store of a volume there's no need to store it in a separate struct. --- src/conf/storage_conf.c | 53 --- src/conf/storage_conf.h | 1 - src/storage/storage_backend.c | 27 +- src/storage/storage_backend_fs.c | 9 -- src/storage/storage_backend_gluster.c | 20 + src/storage/storage_backend_logical.c | 11 +--- 6 files changed, 72 insertions(+), 49 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 1/4] Fix const correctness
On 07/15/2014 06:38 AM, Michal Privoznik wrote: In many places we define a variable as a 'const char *' when in fact we modify it just a few lines below. Or even free it. We should not do that. There's one exception though, in xenSessionFree() xenapi_utils.c. We are freeing the xen_session structure which is defined in xen/api/xen_common.h public header. The structure contains session_id which is type of 'const char *' when in fact it should have been just 'char *'. So I'm leaving this unmodified, just noticing the fact in comment. Signed-off-by: Michal Privoznik mpriv...@redhat.com --- src/locking/lock_driver_lockd.c | 2 +- src/qemu/qemu_capabilities.c | 2 +- src/remote/remote_driver.c | 2 +- src/xenapi/xenapi_utils.c| 1 + tools/virsh-domain.c | 4 ++-- tools/wireshark/src/packet-libvirt.c | 6 +++--- tools/wireshark/src/packet-libvirt.h | 4 ++-- 7 files changed, 11 insertions(+), 10 deletions(-) ACK; safe to apply this one even while waiting for me to propose a v2 for the rest of the series. +++ b/src/xenapi/xenapi_utils.c @@ -49,6 +49,7 @@ xenSessionFree(xen_session *session) VIR_FREE(session-error_description[i]); VIR_FREE(session-error_description); } +/* The session_id member is type of 'const char *'. Sigh. */ VIR_FREE(session-session_id); VIR_FREE(session); By the way, once VIR_FREE is fixed, we can work around this instance by use of a temporary variable: /* cast away bogus const from xen header */ char *tmp = (char *)session-session_id; VIR_FREE(tmp); VIR_FREE(session); +++ b/tools/wireshark/src/packet-libvirt.c @@ -105,7 +105,7 @@ dissect_xdr_string(tvbuff_t *tvb, proto_tree *tree, XDR *xdrs, int hf, } } -static gchar * +static const gchar * Side note - gchar is a pointless type. But not our fault that wireshark is using it instead of plain char. -- 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 4/6] storage: fs: Process backing store data in virStorageBackendProbeTarget
On 07/15/2014 07:25 AM, Peter Krempa wrote:
Move the processing of the backend metadata directly to the helper
instead of passing it through arguments to the function.
---
src/storage/storage_backend_fs.c | 72
++--
1 file changed, 33 insertions(+), 39 deletions(-)
+
+if (target-backingStore-format == VIR_STORAGE_FILE_AUTO) {
+if (!virStorageIsFile(target-backingStore-path) ||
+(rc = virStorageFileProbeFormat(target-backingStore-path,
+-1, -1)) 0) {
Indentation is off.
ACK with that nit fixed.
--
Eric Blake eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2] util: forbid freeing const pointers
Now that we've finally fixed all the violators, it's time to
enforce that any pointer to a const object is never freed (it
is aliasing some other memory, where the non-const original
should be freed instead). Alas, the code still needs a normal
vs. Coverity version, but at least we are still guaranteeing
that the macro call evaluates its argument exactly once.
I verified that we still get the following compiler warnings,
which in turn halts the build thanks to -Werror on gcc (hmm,
gcc 4.8.3's placement of the ^ for ?: type mismatch is a bit
off, but that's not our problem):
int oops1 = 0;
VIR_FREE(oops1);
const char *oops2 = NULL;
VIR_FREE(oops2);
struct blah { int dummy; } oops3;
VIR_FREE(oops3);
util/virauthconfig.c:159:35: error: pointer/integer type mismatch in
conditional expression [-Werror]
VIR_FREE(oops1);
^
util/virauthconfig.c:161:5: error: passing argument 1 of 'virFree' discards
'const' qualifier from pointer target type [-Werror]
VIR_FREE(oops2);
^
In file included from util/virauthconfig.c:28:0:
util/viralloc.h:79:6: note: expected 'void *' but argument is of type 'const
void *'
void virFree(void *ptrptr) ATTRIBUTE_NONNULL(1);
^
util/virauthconfig.c:163:35: error: type mismatch in conditional expression
VIR_FREE(oops3);
^
* src/util/viralloc.h (VIR_FREE): No longer cast away const.
* src/xenapi/xenapi_utils.c (xenSessionFree): Work around bogus
header.
Signed-off-by: Eric Blake ebl...@redhat.com
---
v2: this depends on the existing 1/4, while being a replacement
to all of 2-4/4 at once.
https://www.redhat.com/archives/libvir-list/2014-July/msg00716.html
src/util/viralloc.h | 11 +--
src/xenapi/xenapi_utils.c | 4 +++-
2 files changed, 8 insertions(+), 7 deletions(-)
diff --git a/src/util/viralloc.h b/src/util/viralloc.h
index 7125e67..bf85c16 100644
--- a/src/util/viralloc.h
+++ b/src/util/viralloc.h
@@ -548,18 +548,17 @@ void virFree(void *ptrptr) ATTRIBUTE_NONNULL(1);
* This macro is safe to use on arguments with side effects.
*/
# if !STATIC_ANALYSIS
-/* The ternary ensures that ptr is a pointer and not an integer type,
- * while evaluating ptr only once. This gives us extra compiler
- * safety when compiling under gcc. For now, we intentionally cast
- * away const, since a number of callers safely pass const char *.
+/* The ternary ensures that ptr is a non-const pointer and not an
+ * integer type, all while evaluating ptr only once. This gives us
+ * extra compiler safety when compiling under gcc.
*/
-# define VIR_FREE(ptr) virFree((void *) (1 ? (const void *) (ptr) : (ptr)))
+# define VIR_FREE(ptr) virFree(1 ? (void *) (ptr) : (ptr))
# else
/* The Coverity static analyzer considers the else path of the ?: and
* flags the VIR_FREE() of the address of the address of memory as a
* RESOURCE_LEAK resulting in numerous false positives (eg, VIR_FREE(ptr))
*/
-# define VIR_FREE(ptr) virFree((void *) (ptr))
+# define VIR_FREE(ptr) virFree((ptr))
# endif
void virAllocTestInit(void);
diff --git a/src/xenapi/xenapi_utils.c b/src/xenapi/xenapi_utils.c
index a80d136..ef89f42 100644
--- a/src/xenapi/xenapi_utils.c
+++ b/src/xenapi/xenapi_utils.c
@@ -44,13 +44,15 @@ void
xenSessionFree(xen_session *session)
{
size_t i;
+char *tmp;
if (session-error_description != NULL) {
for (i = 0; i session-error_description_count; i++)
VIR_FREE(session-error_description[i]);
VIR_FREE(session-error_description);
}
/* The session_id member is type of 'const char *'. Sigh. */
-VIR_FREE(session-session_id);
+tmp = (char *)session-session_id;
+VIR_FREE(tmp);
VIR_FREE(session);
}
--
1.9.3
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/4] VIR_FREE: Avoid doing side work in callees
On 07/15/2014 06:38 AM, Michal Privoznik wrote: There are just two places where we rely on the fact that VIR_FREE() macro is without any side effects. In the future, this property of the macro is going to change, so we need the code to be adjusted to deal with argument passed to VIR_FREE() being evaluated multiple times. Signed-off-by: Michal Privoznik mpriv...@redhat.com --- src/conf/network_conf.c | 12 1 file changed, 8 insertions(+), 4 deletions(-) NACK; my v2 keeps the property, so we don't need to work around it changing after all. https://www.redhat.com/archives/libvir-list/2014-July/msg00760.html -- 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 4/4] virFree: Check const correctness
On 07/15/2014 07:46 AM, Eric Blake wrote: But if you take my suggestion in 2/4 about merely removing the 'cast-away-const' while still keeping type safety, then a single-argument virFree() should still be noisy on attempts to VIR_FREE a const pointer. @@ -543,11 +543,23 @@ void virFree(void *ptrptr) ATTRIBUTE_NONNULL(1); * @ptr: pointer holding address to be freed * * Free the memory stored in 'ptr' and update to point - * to NULL. + * to NULL. Moreover, this macro has a side effect in + * form of evaluating passed argument multiple times. NACK. I think it is possible to use sizeof() to come up with a construct that will only do side effects once, rather than having to weaken the guarantee of VIR_FREE. Please give me some time to propose an alternative. I didn't even need to resort to sizeof(). Here's v2, which replaces 2, 3, and 4 of this series: https://www.redhat.com/archives/libvir-list/2014-July/msg00760.html -- 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 5/6] storage: fs: Properly parse backing store info
On 07/15/2014 07:25 AM, Peter Krempa wrote:
Use the backing store parser to properly create the information about a
volume's backing store. Unfortunately as the storage driver isn't
perpared to allow volumes backed by networked filesystems add a
s/perpared/prepared/
workaroud that will avoid changing the XML output.
s/workaroud/workaround/
---
src/storage/storage_backend_fs.c | 21 -
1 file changed, 16 insertions(+), 5 deletions(-)
+/* XXX: Remote storage doesn't play nicely with volumes backed by
+ * remote storage. To avoid trouble, just fake the ou */
Incomplete thought?
+
if (target-backingStore-format == VIR_STORAGE_FILE_AUTO) {
-if (!virStorageIsFile(target-backingStore-path) ||
-(rc = virStorageFileProbeFormat(target-backingStore-path,
+if ((rc =
virStorageFileProbeFormat(target-backingStore-path,
-1, -1)) 0) {
Indentation is off.
ACK with those fixes.
--
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 6/6] storage: fs: Don't fail volume update if backing store isn't accessible
On 07/15/2014 07:25 AM, Peter Krempa wrote: When the backing store of a volume wasn't accessible while updating the volume definition the call would fail alltogether. In cases where we s/alltogether/altogether/ currently (incorrectly) treat remote backing stores as local one this might lead to strange errors. Ignore the opening errors until we figure out how to track proper volume metadata. --- src/storage/storage_backend.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c index f5bfdee..fdcaada 100644 --- a/src/storage/storage_backend.c +++ b/src/storage/storage_backend.c @@ -1465,7 +1465,8 @@ virStorageBackendUpdateVolInfo(virStorageVolDefPtr vol, (ret = virStorageBackendUpdateVolTargetInfo(vol-target.backingStore, updateCapacity, withBlockVolFormat, - VIR_STORAGE_VOL_OPEN_DEFAULT)) 0) + VIR_STORAGE_VOL_OPEN_DEFAULT | + VIR_STORAGE_VOL_OPEN_NOERROR) 0)) return ret; Works for me as a stopgap. (On IRC, we were discussing that we probably want to update the storage volume XML for backingStore to look a bit more like domain disk backingstore, at least for cases where we know the backing store is not in the same pool as the source - but that's a bigger project so worth later patches). 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] [PREPOST 03/17] src/xenxm:Refactor network config parsing code
Jim Fehlig wrote:
David Kiarie wrote:
From: Kiarie Kahurani davidkiar...@gmail.com
I introduced the function
xenFormatXMVif(virConfPtr conf, ..);
which parses network configuration
signed-off-by: David Kiarie davidkiar...@gmail.com
---
src/xenxs/xen_xm.c | 298
-
1 file changed, 155 insertions(+), 143 deletions(-)
diff --git a/src/xenxs/xen_xm.c b/src/xenxs/xen_xm.c
index dc840e5..26ebd5a 100644
--- a/src/xenxs/xen_xm.c
+++ b/src/xenxs/xen_xm.c
@@ -309,6 +309,159 @@ static int xenParseXMMem(virConfPtr conf,
virDomainDefPtr def)
return 0;
}
+static int xenParseXMVif(virConfPtr conf, virDomainDefPtr def)
Same comment here about blank lines between functions. I won't bore you
with repeating myself in all the patches. Also, remember Eric's comment
about function return type on one line and name and params on another.
E.g.
static int
xenParseXMVif(virConfPtr conf, virDomainDefPtr def)
{
...
}
+{
+char *script = NULL;
+virDomainNetDefPtr net = NULL;
+virConfValuePtr list = virConfGetValue(conf, vif);
Style is to have a blank line after local variable declarations.
I think you should also define a local variable ret, initialized to -1.
+if (list list-type == VIR_CONF_LIST) {
+list = list-list;
+while (list) {
+char model[10];
+char type[10];
+char ip[16];
+char mac[18];
+char bridge[50];
+char vifname[50];
+char *key;
+
+bridge[0] = '\0';
+mac[0] = '\0';
+ip[0] = '\0';
+model[0] = '\0';
+type[0] = '\0';
+vifname[0] = '\0';
+
+if ((list-type != VIR_CONF_STRING) || (list-str == NULL))
+goto skipnic;
+
+key = list-str;
+while (key) {
+char *data;
+char *nextkey = strchr(key, ',');
+
+if (!(data = strchr(key, '=')))
+goto skipnic;
+data++;
+
+if (STRPREFIX(key, mac=)) {
+int len = nextkey ? (nextkey - data) : sizeof(mac) - 1;
+if (virStrncpy(mac, data, len, sizeof(mac)) == NULL) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ _(MAC address %s too big for
destination),
+ data);
+goto skipnic;
+}
+} else if (STRPREFIX(key, bridge=)) {
+int len = nextkey ? (nextkey - data) : sizeof(bridge) -
1;
+if (virStrncpy(bridge, data, len, sizeof(bridge)) ==
NULL) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ _(Bridge %s too big for
destination),
+ data);
+goto skipnic;
+}
+} else if (STRPREFIX(key, script=)) {
+int len = nextkey ? (nextkey - data) : strlen(data);
+VIR_FREE(script);
+if (VIR_STRNDUP(script, data, len) 0)
+goto cleanup;
+} else if (STRPREFIX(key, model=)) {
+int len = nextkey ? (nextkey - data) : sizeof(model) -
1;
+if (virStrncpy(model, data, len, sizeof(model)) ==
NULL) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ _(Model %s too big for
destination), data);
+goto skipnic;
+}
+} else if (STRPREFIX(key, type=)) {
+int len = nextkey ? (nextkey - data) : sizeof(type) - 1;
+if (virStrncpy(type, data, len, sizeof(type)) == NULL) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ _(Type %s too big for
destination), data);
+goto skipnic;
+}
+} else if (STRPREFIX(key, vifname=)) {
+int len = nextkey ? (nextkey - data) : sizeof(vifname)
- 1;
+if (virStrncpy(vifname, data, len, sizeof(vifname)) ==
NULL) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ _(Vifname %s too big for
destination),
+ data);
+goto skipnic;
+}
+} else if (STRPREFIX(key, ip=)) {
+int len = nextkey ? (nextkey - data) : sizeof(ip) - 1;
+if (virStrncpy(ip, data, len, sizeof(ip)) == NULL) {
+
Re: [libvirt] [PREPOST 15/17] src/xenxs:Refactor Vif formating code
David kiarie wrote: Thanks for the review, am applying changes to address your comments. Thanks David, looking forward to a V2. BTW, 16/17 and 17/17 look good with exception of the usual whitespace comments. Regards, Jim -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PREPOST 05/17] src/xenxs:Refactor PCI parsing code
On 07/11/2014 11:09 AM, Jim Fehlig wrote:
David Kiarie wrote:
+
+/* pci=[':00:1b.0',':00:13.0'] */
+if (!(key = list-str))
+goto skippci;
+if (!(nextkey = strchr(key, ':')))
+goto skippci;
+
+if (virStrncpy(domain, key, (nextkey - key), sizeof(domain)) ==
NULL) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ _(Domain %s too big for destination), key);
Pre-existing, but while we are touching the code, I wonder if the use of
virReportError here is correct? The device is skipped if there is a
problem parsing it. I think these errors should be logged via VIR_WARN,
but would like confirmation from another libvirt dev before asking you
to change them. At the very least, the error should be changed to
VIR_ERR_CONF_SYNTAX.
How easy is it to trigger this error path via user input? If the string
that we are splitting is normally provided from a sane source, using
VIR_ERR_INTERNAL_ERROR is okay; if the string we are splitting can come
from a user that can easily try to fuzz things to confuse us, then
VIR_ERR_CONF_SYNTAX is indeed nicer.
As for whether to skip a device we can't parse, or outright fail, I'm
not sure which is better. If you are just skipping the device, then
using VIR_WARN instead of virReportError will avoid the odd case of
returning success to the user while still having an error set.
Don't know if I helped or just made it more confusing.
--
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] Enable kvm on aarch64, Cleanup F-16/18 conditionals
On 07/15/2014 04:00 PM, Peter Robinson wrote: [reformatting to avoid top-posting] On Tue, Jul 15, 2014 at 6:52 PM, Daniel P. Berrange berra...@redhat.com wrote: Historically we've kept support for building libvirt against Fedora versions even if unsupported by Fedora itself, because we've found people often stick on old Fedora versions. At some point though it does get a bit insane. eg we don't really need Fedora 8 support at this point. If we are going todo a cleanup though, we should do it throughout the spec, not just in these few places Sorry, I missed the Fedora 8 stuff. Would it even work on Fedora 8 with all the various other changes? Not sure it's worth the effort for the few people that want that release as a hypervisor, I can understand for the supported elX releases but not sure the Fedora releases that are systemd based give value to elX releases and are generally relatively ancient history. Dan do you really have know users of the latest release running it on F-8? In terms of pushing upstream Cole has said he's happy to push the commits to ensure it fits your work flows. Doing an out-of-the-box build on RHEL 5 is the oldest configuration still actively (if marginally) supported, ideally for as long as RHEL 5 remains a live platform (several more years to go). We have build-bots that ensure that we can build on RHEL 5, although I'm not sure if those buildbots are exercising 'make rpm' to test the older parts of the spec file. Historically, RHEL 5.10 is based off of libvirt-0.8.2, and that was the release in use during Fedora 13. So it's _definitely_ worth culling any conditionals older than F13; but stuff between F13 and F18 might be shared with RHEL 5, and therefore more effort to cull the Fedora side while still leaving the RHEL side intact. I could go either way if we were to set some sort of policy (maybe: any fedora release that is unsupported for more than a year is no longer required to be supported in the spec file), and use that to justify cleanups of older parts of the spec file as long as it doesn't break RHEL 5. F16 is definitely more than a year out-of-date, F18 is a bit more borderline on whether we are ready to call it unsupported. Anyone else on the libvirt list have an opinion on how far back we can clean without annoying people that are slow on the upgrade to modern Fedora? -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [libvirt-glib v3 3/4] Add GVirConfigDomainCpuModel class
Add a class to represent 'model' node under domain/cpu.
---
libvirt-gconfig/Makefile.am| 2 +
libvirt-gconfig/libvirt-gconfig-domain-cpu-model.c | 75 ++
libvirt-gconfig/libvirt-gconfig-domain-cpu-model.h | 68
libvirt-gconfig/libvirt-gconfig.h | 1 +
libvirt-gconfig/libvirt-gconfig.sym| 3 +
5 files changed, 149 insertions(+)
create mode 100644 libvirt-gconfig/libvirt-gconfig-domain-cpu-model.c
create mode 100644 libvirt-gconfig/libvirt-gconfig-domain-cpu-model.h
diff --git a/libvirt-gconfig/Makefile.am b/libvirt-gconfig/Makefile.am
index 02240d4..a9a6591 100644
--- a/libvirt-gconfig/Makefile.am
+++ b/libvirt-gconfig/Makefile.am
@@ -38,6 +38,7 @@ GCONFIG_HEADER_FILES = \
libvirt-gconfig-domain-controller-usb.h \
libvirt-gconfig-domain-cpu.h \
libvirt-gconfig-domain-cpu-feature.h \
+ libvirt-gconfig-domain-cpu-model.h \
libvirt-gconfig-domain-device.h \
libvirt-gconfig-domain-disk.h \
libvirt-gconfig-domain-disk-driver.h \
@@ -127,6 +128,7 @@ GCONFIG_SOURCE_FILES = \
libvirt-gconfig-domain-controller-usb.c \
libvirt-gconfig-domain-cpu.c \
libvirt-gconfig-domain-cpu-feature.c \
+ libvirt-gconfig-domain-cpu-model.c \
libvirt-gconfig-domain-device.c \
libvirt-gconfig-domain-disk.c \
libvirt-gconfig-domain-disk-driver.c \
diff --git a/libvirt-gconfig/libvirt-gconfig-domain-cpu-model.c
b/libvirt-gconfig/libvirt-gconfig-domain-cpu-model.c
new file mode 100644
index 000..03024a6
--- /dev/null
+++ b/libvirt-gconfig/libvirt-gconfig-domain-cpu-model.c
@@ -0,0 +1,75 @@
+/*
+ * libvirt-gconfig-domain-cpu-model.c: libvirt domain CPU model
+ *
+ * Copyright (C) 2014 Red Hat, Inc.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library. If not, see
+ * http://www.gnu.org/licenses/.
+ *
+ * Authors: Zeeshan Ali zee...@redhat.com
+ */
+
+#include config.h
+
+#include libvirt-gconfig/libvirt-gconfig.h
+#include libvirt-gconfig/libvirt-gconfig-private.h
+
+#define GVIR_CONFIG_DOMAIN_CPU_MODEL_GET_PRIVATE(obj) \
+(G_TYPE_INSTANCE_GET_PRIVATE((obj), GVIR_CONFIG_TYPE_DOMAIN_CPU_MODEL,
GVirConfigDomainCpuModelPrivate))
+
+struct _GVirConfigDomainCpuModelPrivate
+{
+gboolean unused;
+};
+
+G_DEFINE_TYPE(GVirConfigDomainCpuModel,
+ gvir_config_domain_cpu_model,
+ GVIR_CONFIG_TYPE_CAPABILITIES_CPU_MODEL);
+
+static void
gvir_config_domain_cpu_model_class_init(GVirConfigDomainCpuModelClass *klass)
+{
+g_type_class_add_private(klass, sizeof(GVirConfigDomainCpuModelPrivate));
+}
+
+static void gvir_config_domain_cpu_model_init(GVirConfigDomainCpuModel *model)
+{
+g_debug(Init GVirConfigDomainCpuModel=%p, model);
+
+model-priv = GVIR_CONFIG_DOMAIN_CPU_MODEL_GET_PRIVATE(model);
+}
+
+GVirConfigDomainCpuModel *gvir_config_domain_cpu_model_new(void)
+{
+GVirConfigObject *object;
+
+object = gvir_config_object_new(GVIR_CONFIG_TYPE_DOMAIN_CPU_MODEL,
+model,
+NULL);
+
+return GVIR_CONFIG_DOMAIN_CPU_MODEL(object);
+}
+
+GVirConfigDomainCpuModel *
+gvir_config_domain_cpu_model_new_from_xml(const gchar *xml, GError **error)
+{
+GVirConfigObject *object;
+
+object = gvir_config_object_new_from_xml(GVIR_CONFIG_TYPE_DOMAIN_CPU_MODEL,
+ model,
+ NULL,
+ xml,
+ error);
+
+return GVIR_CONFIG_DOMAIN_CPU_MODEL(object);
+}
diff --git a/libvirt-gconfig/libvirt-gconfig-domain-cpu-model.h
b/libvirt-gconfig/libvirt-gconfig-domain-cpu-model.h
new file mode 100644
index 000..c6cb36f
--- /dev/null
+++ b/libvirt-gconfig/libvirt-gconfig-domain-cpu-model.h
@@ -0,0 +1,68 @@
+/*
+ * libvirt-gconfig-domain-cpu-model.h: libvirt domain CPU model
+ *
+ * Copyright (C) 2014 Red Hat, Inc.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
[libvirt] CPU model API (v3)
v3: Two classes for CPU model: * CapabilitiesCpuModel * DomainCpuModel that derives from CapabilitiesCpuModel. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [libvirt-glib v3 2/4] Add gvir_config_capabilities_cpu_get_model()
Add a method to get the model of the CPU from capabilities.
---
libvirt-gconfig/libvirt-gconfig-capabilities-cpu.c | 23 ++
libvirt-gconfig/libvirt-gconfig-capabilities-cpu.h | 3 +++
libvirt-gconfig/libvirt-gconfig.sym| 2 ++
3 files changed, 28 insertions(+)
diff --git a/libvirt-gconfig/libvirt-gconfig-capabilities-cpu.c
b/libvirt-gconfig/libvirt-gconfig-capabilities-cpu.c
index f4753ff..a2d5c3e 100644
--- a/libvirt-gconfig/libvirt-gconfig-capabilities-cpu.c
+++ b/libvirt-gconfig/libvirt-gconfig-capabilities-cpu.c
@@ -185,3 +185,26 @@
gvir_config_capabilities_cpu_set_topology(GVirConfigCapabilitiesCpu *cpu,
topology,
GVIR_CONFIG_OBJECT(topology));
}
+
+/**
+ * gvir_config_capabilities_cpu_get_model:
+ *
+ * Gets the model of the cpu.
+ *
+ * Returns: (transfer full): a new #GVirConfigCapabilitiesCpuModel.
+ */
+GVirConfigCapabilitiesCpuModel *
+gvir_config_capabilities_cpu_get_model(GVirConfigCapabilitiesCpu *cpu)
+{
+GVirConfigObject *object;
+
+g_return_val_if_fail(GVIR_CONFIG_IS_CAPABILITIES_CPU(cpu), NULL);
+
+object = gvir_config_object_get_child_with_type
+(GVIR_CONFIG_OBJECT(cpu),
+ model,
+ GVIR_CONFIG_TYPE_CAPABILITIES_CPU_MODEL);
+
+return GVIR_CONFIG_CAPABILITIES_CPU_MODEL(object);
+}
+
diff --git a/libvirt-gconfig/libvirt-gconfig-capabilities-cpu.h
b/libvirt-gconfig/libvirt-gconfig-capabilities-cpu.h
index ce3613f..57ad48b 100644
--- a/libvirt-gconfig/libvirt-gconfig-capabilities-cpu.h
+++ b/libvirt-gconfig/libvirt-gconfig-capabilities-cpu.h
@@ -30,6 +30,7 @@
#include libvirt-gconfig-capabilities-cpu-topology.h
#include libvirt-gconfig-capabilities-cpu-feature.h
+#include libvirt-gconfig-capabilities-cpu-model.h
G_BEGIN_DECLS
@@ -75,6 +76,8 @@
gvir_config_capabilities_cpu_get_topology(GVirConfigCapabilitiesCpu *cpu);
void
gvir_config_capabilities_cpu_set_topology(GVirConfigCapabilitiesCpu *cpu,
GVirConfigCapabilitiesCpuTopology
*topology);
+GVirConfigCapabilitiesCpuModel *
+gvir_config_capabilities_cpu_get_model(GVirConfigCapabilitiesCpu *cpu);
G_END_DECLS
diff --git a/libvirt-gconfig/libvirt-gconfig.sym
b/libvirt-gconfig/libvirt-gconfig.sym
index 76dde70..76b0d03 100644
--- a/libvirt-gconfig/libvirt-gconfig.sym
+++ b/libvirt-gconfig/libvirt-gconfig.sym
@@ -689,6 +689,8 @@ global:
LIBVIRT_GCONFIG_0.1.9 {
global:
+ gvir_config_capabilities_cpu_get_model;
+
gvir_config_capabilities_cpu_model_get_name;
gvir_config_capabilities_cpu_model_get_type;
gvir_config_capabilities_cpu_model_new;
--
1.9.3
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [libvirt-glib v3 1/4] Add GVirConfigCapabilitiesCpuModel class
Add a class to represent 'model' node under capabilities/host/cpu.
---
libvirt-gconfig/Makefile.am| 2 +
.../libvirt-gconfig-capabilities-cpu-model.c | 94 ++
.../libvirt-gconfig-capabilities-cpu-model.h | 72 +
libvirt-gconfig/libvirt-gconfig.h | 1 +
libvirt-gconfig/libvirt-gconfig.sym| 5 ++
5 files changed, 174 insertions(+)
create mode 100644 libvirt-gconfig/libvirt-gconfig-capabilities-cpu-model.c
create mode 100644 libvirt-gconfig/libvirt-gconfig-capabilities-cpu-model.h
diff --git a/libvirt-gconfig/Makefile.am b/libvirt-gconfig/Makefile.am
index 8a3ff0d..02240d4 100644
--- a/libvirt-gconfig/Makefile.am
+++ b/libvirt-gconfig/Makefile.am
@@ -15,6 +15,7 @@ GCONFIG_HEADER_FILES = \
libvirt-gconfig-capabilities-host.h \
libvirt-gconfig-capabilities-cpu.h \
libvirt-gconfig-capabilities-cpu-feature.h \
+ libvirt-gconfig-capabilities-cpu-model.h \
libvirt-gconfig-capabilities-cpu-topology.h \
libvirt-gconfig-capabilities-guest.h \
libvirt-gconfig-capabilities-guest-arch.h \
@@ -103,6 +104,7 @@ GCONFIG_SOURCE_FILES = \
libvirt-gconfig-capabilities-host.c \
libvirt-gconfig-capabilities-cpu.c \
libvirt-gconfig-capabilities-cpu-feature.c \
+ libvirt-gconfig-capabilities-cpu-model.c \
libvirt-gconfig-capabilities-cpu-topology.c \
libvirt-gconfig-capabilities-guest.c \
libvirt-gconfig-capabilities-guest-arch.c \
diff --git a/libvirt-gconfig/libvirt-gconfig-capabilities-cpu-model.c
b/libvirt-gconfig/libvirt-gconfig-capabilities-cpu-model.c
new file mode 100644
index 000..002b77f
--- /dev/null
+++ b/libvirt-gconfig/libvirt-gconfig-capabilities-cpu-model.c
@@ -0,0 +1,94 @@
+/*
+ * libvirt-gconfig-capabilities-cpu-model.c: libvirt CPU model capabilities
+ *
+ * Copyright (C) 2014 Red Hat, Inc.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library. If not, see
+ * http://www.gnu.org/licenses/.
+ *
+ * Authors: Zeeshan Ali zee...@redhat.com
+ */
+
+#include config.h
+
+#include libvirt-gconfig/libvirt-gconfig.h
+#include libvirt-gconfig/libvirt-gconfig-private.h
+
+#define GVIR_CONFIG_CAPABILITIES_CPU_MODEL_GET_PRIVATE(obj)
\
+(G_TYPE_INSTANCE_GET_PRIVATE((obj),
GVIR_CONFIG_TYPE_CAPABILITIES_CPU_MODEL, GVirConfigCapabilitiesCpuModelPrivate))
+
+struct _GVirConfigCapabilitiesCpuModelPrivate
+{
+gboolean unused;
+};
+
+G_DEFINE_TYPE(GVirConfigCapabilitiesCpuModel,
gvir_config_capabilities_cpu_model, GVIR_CONFIG_TYPE_OBJECT);
+
+static void
gvir_config_capabilities_cpu_model_class_init(GVirConfigCapabilitiesCpuModelClass
*klass)
+{
+g_type_class_add_private(klass,
sizeof(GVirConfigCapabilitiesCpuModelPrivate));
+}
+
+static void
gvir_config_capabilities_cpu_model_init(GVirConfigCapabilitiesCpuModel *model)
+{
+g_debug(Init GVirConfigCapabilitiesCpuModel=%p, model);
+
+model-priv = GVIR_CONFIG_CAPABILITIES_CPU_MODEL_GET_PRIVATE(model);
+}
+
+GVirConfigCapabilitiesCpuModel *gvir_config_capabilities_cpu_model_new(void)
+{
+GVirConfigObject *object;
+
+object = gvir_config_object_new(GVIR_CONFIG_TYPE_CAPABILITIES_CPU_MODEL,
+model,
+NULL);
+
+return GVIR_CONFIG_CAPABILITIES_CPU_MODEL(object);
+}
+
+GVirConfigCapabilitiesCpuModel *
+gvir_config_capabilities_cpu_model_new_from_xml(const gchar *xml, GError
**error)
+{
+GVirConfigObject *object;
+
+object =
gvir_config_object_new_from_xml(GVIR_CONFIG_TYPE_CAPABILITIES_CPU_MODEL,
+ model,
+ NULL,
+ xml,
+ error);
+
+return GVIR_CONFIG_CAPABILITIES_CPU_MODEL(object);
+}
+
+void
+gvir_config_capabilities_cpu_model_set_name(GVirConfigCapabilitiesCpuModel
*model,
+const gchar *name)
+{
+g_return_if_fail(GVIR_CONFIG_IS_CAPABILITIES_CPU_MODEL(model));
+
+gvir_config_object_set_node_content
+
[libvirt] [libvirt-glib v3 4/4] Add gvir_config_domain_cpu_set_model()
Add a method to set model of domain CPU.
---
libvirt-gconfig/libvirt-gconfig-domain-cpu.c | 11 +++
libvirt-gconfig/libvirt-gconfig-domain-cpu.h | 4
libvirt-gconfig/libvirt-gconfig.sym | 2 ++
3 files changed, 17 insertions(+)
diff --git a/libvirt-gconfig/libvirt-gconfig-domain-cpu.c
b/libvirt-gconfig/libvirt-gconfig-domain-cpu.c
index e7b9575..0037763 100644
--- a/libvirt-gconfig/libvirt-gconfig-domain-cpu.c
+++ b/libvirt-gconfig/libvirt-gconfig-domain-cpu.c
@@ -136,3 +136,14 @@ void gvir_config_domain_cpu_set_mode(GVirConfigDomainCpu
*cpu,
mode, GVIR_CONFIG_TYPE_DOMAIN_CPU_MODE, mode,
NULL);
}
+
+void gvir_config_domain_cpu_set_model(GVirConfigDomainCpu *cpu,
+ GVirConfigDomainCpuModel *model)
+{
+g_return_if_fail(GVIR_CONFIG_IS_DOMAIN_CPU(cpu));
+g_return_if_fail(model == NULL || GVIR_CONFIG_IS_DOMAIN_CPU_MODEL(model));
+
+gvir_config_object_attach_replace(GVIR_CONFIG_OBJECT(cpu),
+ model,
+ GVIR_CONFIG_OBJECT(model));
+}
diff --git a/libvirt-gconfig/libvirt-gconfig-domain-cpu.h
b/libvirt-gconfig/libvirt-gconfig-domain-cpu.h
index 7efb7eb..f7c0a93 100644
--- a/libvirt-gconfig/libvirt-gconfig-domain-cpu.h
+++ b/libvirt-gconfig/libvirt-gconfig-domain-cpu.h
@@ -28,6 +28,8 @@
#ifndef __LIBVIRT_GCONFIG_DOMAIN_CPU_H__
#define __LIBVIRT_GCONFIG_DOMAIN_CPU_H__
+#include libvirt-gconfig/libvirt-gconfig-domain-cpu-model.h
+
G_BEGIN_DECLS
#define GVIR_CONFIG_TYPE_DOMAIN_CPU
(gvir_config_domain_cpu_get_type ())
@@ -80,6 +82,8 @@ GVirConfigDomainCpuMatchPolicy
gvir_config_domain_cpu_get_match_policy(GVirConfigDomainCpu *cpu);
void gvir_config_domain_cpu_set_mode(GVirConfigDomainCpu *cpu,
GVirConfigDomainCpuMode mode);
+void gvir_config_domain_cpu_set_model(GVirConfigDomainCpu *cpu,
+ GVirConfigDomainCpuModel *model);
GVirConfigDomainCpuMode
gvir_config_domain_cpu_get_mode(GVirConfigDomainCpu *cpu);
diff --git a/libvirt-gconfig/libvirt-gconfig.sym
b/libvirt-gconfig/libvirt-gconfig.sym
index 7b49126..8614126 100644
--- a/libvirt-gconfig/libvirt-gconfig.sym
+++ b/libvirt-gconfig/libvirt-gconfig.sym
@@ -710,6 +710,8 @@ global:
gvir_config_domain_cpu_model_get_type;
gvir_config_domain_cpu_model_new;
+
+ gvir_config_domain_cpu_set_model;
} LIBVIRT_GCONFIG_0.1.8;
# define new API here using predicted next version number
--
1.9.3
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] spec: Update polkit dependencies for CVE-2013-4311
On 07/15/2014 07:23 AM, Jiri Denemark wrote:
Use secured polkit on distros which provide it. However, RHEL-6 will
still allow for older polkit-0.93 rather than forcing polkit-0.96-5
which is not available in all RHEL-6 releases.
Signed-off-by: Jiri Denemark jdene...@redhat.com
---
libvirt.spec.in | 8 ++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/libvirt.spec.in b/libvirt.spec.in
index 8d1acfa..f32ab00 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -535,7 +535,9 @@ BuildRequires: module-init-tools
BuildRequires: cyrus-sasl-devel
%endif
%if %{with_polkit}
-%if 0%{?fedora} = 12 || 0%{?rhel} = 6
+%if 0%{?fedora} = 21 || 0%{?rhel} = 7
+BuildRequires: polkit-devel = 0.112
+%elif 0%{?fedora} = 12 || 0%{?rhel} = 6
BuildRequires: polkit-devel = 0.93
Ouch - make rpm now complains:
error: line 519: Unknown tag: %elif (020) || 0 = 6
I don't think %elif is a valid spec file construct (too much shell
programming for you lately?), and we have to break it into nested %if.
I'll push the obvious fixup patch momentarily.
--
Eric Blake eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] spec: fix invalid syntax
Commit 20e01504 broke 'make rpm':
error: line 540: Unknown tag: %elif 020 = 12 || 0 = 6
Apparently, even though shell has elif so that you can do a chain
of conditionals, the rpm spec file does not, and you have to nest
things instead.
* libvirt.spec.in: Convert %elif to proper nested %if.
Signed-off-by: Eric Blake ebl...@redhat.com
---
Pushing under the build-breaker rule.
libvirt.spec.in | 12
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/libvirt.spec.in b/libvirt.spec.in
index 47bfec5..9c7b241 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -537,10 +537,12 @@ BuildRequires: cyrus-sasl-devel
%if %{with_polkit}
%if 0%{?fedora} = 20 || 0%{?rhel} = 7
BuildRequires: polkit-devel = 0.112
-%elif 0%{?fedora} = 12 || 0%{?rhel} = 6
-BuildRequires: polkit-devel = 0.93
%else
+%if 0%{?fedora} = 12 || 0%{?rhel} = 6
+BuildRequires: polkit-devel = 0.93
+%else
BuildRequires: PolicyKit-devel = 0.6
+%endif
%endif
%endif
%if %{with_storage_fs}
@@ -702,10 +704,12 @@ Requires: avahi-libs
%if %{with_polkit}
%if 0%{?fedora} = 20 || 0%{?rhel} = 7
Requires: polkit = 0.112
-%elif 0%{?fedora} = 12 || 0%{?rhel} =6
-Requires: polkit = 0.93
%else
+%if 0%{?fedora} = 12 || 0%{?rhel} =6
+Requires: polkit = 0.93
+%else
Requires: PolicyKit = 0.6
+%endif
%endif
%endif
%if %{with_cgconfig}
--
1.9.3
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Enable kvm on aarch64, Cleanup F-16/18 conditionals
On 07/15/2014 04:29 PM, Peter Robinson wrote: Doing an out-of-the-box build on RHEL 5 is the oldest configuration still actively (if marginally) supported, ideally for as long as RHEL 5 remains a live platform (several more years to go). We have build-bots that ensure that we can build on RHEL 5, although I'm not sure if those buildbots are exercising 'make rpm' to test the older parts of the spec file. Historically, RHEL 5.10 is based off of libvirt-0.8.2, and that was the release in use during Fedora 13. So it's _definitely_ worth culling any conditionals older than F13; but stuff between F13 and F18 might be shared with RHEL 5, and therefore more effort to cull the Fedora side while still leaving the RHEL side intact. Yes, and you'll note in my change that I didn't change anything that affected EL based releases. In terms of F-13 style tags you should be capturing that in appropriate and equivalent EL tags to ensure you get right and consistent conditionals for the appropriate release as opposed to relying on a translation as you have EL conditionals there already why mix the two. Not sure I follow you here; a patch may be worth more than words (and I'm planning on posting a tentative patch soon). Anyone else on the libvirt list have an opinion on how far back we can clean without annoying people that are slow on the upgrade to modern Fedora? You have known users that are actively upgrading to the latest libvirt and no other components on old versions of Fedora? I don't honestly know - which is why I'm asking the list if anyone reading here would care if we pruned F18 code. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] spec: drop anything older than Fedora 13
RHEL 5 is based on libvirt 0.8.2, as was Fedora 13. RHEL 5 also
happens to be the oldest box that we actively support with a
buildbot, so it is time to clean up some crufty conditionals in
the spec file that no longer are necessary for modern Fedora.
Although it is probably okay to make further simplifications to
a newer minimum Fedora version, that can be done as a later patch.
This patch just focuses on cleaning any comparison of %{?fedora}
that will always be true or false once we assume a minimum of F13.
* libvirt.spec.in: Make with_audit default to on. Move other
conditionals to a single RHEL-5 block. Simplify any fedora
comparison older than 13. Document our assumptions.
Signed-off-by: Eric Blake ebl...@redhat.com
---
libvirt.spec.in | 69 -
1 file changed, 24 insertions(+), 45 deletions(-)
diff --git a/libvirt.spec.in b/libvirt.spec.in
index 9c7b241..091eec8 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -1,5 +1,7 @@
# -*- rpm-spec -*-
+# This spec file assumes you are building for Fedora 13 or newer,
+# or for RHEL 5 or newer. It may need some tweaks for other distros.
# If neither fedora nor rhel was defined, try to guess them from %{dist}
%if !0%{?rhel} !0%{?fedora}
%{expand:%(echo %{?dist} | \
@@ -122,7 +124,6 @@
%define with_libpcap 0%{!?_without_libpcap:0}
%define with_macvtap 0%{!?_without_macvtap:0}
%define with_libnl 0%{!?_without_libnl:0}
-%define with_audit 0%{!?_without_audit:0}
%define with_dtrace0%{!?_without_dtrace:0}
%define with_cgconfig 0%{!?_without_cgconfig:0}
%define with_sanlock 0%{!?_without_sanlock:0}
@@ -136,6 +137,7 @@
# Non-server/HV driver defaults which are always enabled
%define with_sasl 0%{!?_without_sasl:1}
+%define with_audit 0%{!?_without_audit:1}
# Finally set the OS / architecture specific special cases
@@ -224,31 +226,21 @@
%define with_libxl 0
%endif
-# PolicyKit was introduced in Fedora 8 / RHEL-6 or newer
-%if 0%{?fedora} = 8 || 0%{?rhel} = 6
-%define with_polkit0%{!?_without_polkit:1}
-%endif
-
-# libcapng is used to manage capabilities in Fedora 12 / RHEL-6 or newer
-%if 0%{?fedora} = 12 || 0%{?rhel} = 6
-%define with_capng 0%{!?_without_capng:1}
-%endif
-
# fuse is used to provide virtualized /proc for LXC
%if 0%{?fedora} = 17 || 0%{?rhel} = 7
%define with_fuse 0%{!?_without_fuse:1}
%endif
-# netcf is used to manage network interfaces in Fedora 12 / RHEL-6 or newer
-%if 0%{?fedora} = 12 || 0%{?rhel} = 6
-%define with_netcf 0%{!?_without_netcf:%{server_drivers}}
-%endif
-
-# udev is used to manage host devices in Fedora 12 / RHEL-6 or newer
-%if 0%{?fedora} = 12 || 0%{?rhel} = 6
-%define with_udev 0%{!?_without_udev:%{server_drivers}}
-%else
+# RHEL 5 lacks newer tools
+%if 0%{?rhel} == 5
%define with_hal 0%{!?_without_hal:%{server_drivers}}
+%else
+%define with_polkit0%{!?_without_polkit:1}
+%define with_capng 0%{!?_without_capng:1}
+%define with_netcf 0%{!?_without_netcf:%{server_drivers}}
+%define with_udev 0%{!?_without_udev:%{server_drivers}}
+%define with_yajl 0%{!?_without_yajl:%{server_drivers}}
+%define with_dtrace 1
%endif
# interface requires netcf
@@ -256,11 +248,6 @@
%define with_interface 0
%endif
-# Enable yajl library for JSON mode with QEMU
-%if 0%{?fedora} = 13 || 0%{?rhel} = 6
-%define with_yajl 0%{!?_without_yajl:%{server_drivers}}
-%endif
-
# Enable sanlock library for lock management with QEMU
# Sanlock is available only on x86_64 for RHEL
%if 0%{?fedora} = 16
@@ -321,16 +308,8 @@
%define with_libnl 1
%endif
-%if 0%{?fedora} = 11 || 0%{?rhel} = 5
-%define with_audit0%{!?_without_audit:1}
-%endif
-
-%if 0%{?fedora} = 13 || 0%{?rhel} = 6
-%define with_dtrace 1
-%endif
-
# Pull in cgroups config system
-%if 0%{?fedora} = 12 || 0%{?rhel} = 6
+%if 0%{?fedora} || 0%{?rhel} = 6
%if %{with_qemu} || %{with_lxc}
%define with_cgconfig 0%{!?_without_cgconfig:1}
%endif
@@ -350,7 +329,7 @@
# Force QEMU to run as non-root
-%if 0%{?fedora} = 12 || 0%{?rhel} = 6
+%if 0%{?fedora} || 0%{?rhel} = 6
%define qemu_user qemu
%define qemu_group qemu
%else
@@ -473,7 +452,7 @@ BuildRequires: libattr-devel
# For pool-build probing for existing pools
BuildRequires: libblkid-devel = 2.17
%endif
-%if 0%{?fedora} = 12 || 0%{?rhel} = 6
+%if 0%{?fedora} || 0%{?rhel} = 6
# for augparse, optionally used in testing
BuildRequires: augeas
%endif
@@ -538,7 +517,7 @@ BuildRequires: cyrus-sasl-devel
%if 0%{?fedora} = 20 || 0%{?rhel} = 7
BuildRequires: polkit-devel = 0.112
%else
-%if 0%{?fedora} = 12 || 0%{?rhel} = 6
+%if 0%{?fedora} || 0%{?rhel} = 6
BuildRequires: polkit-devel = 0.93
%else
BuildRequires: PolicyKit-devel = 0.6
@@ -621,7 +600,7 @@ BuildRequires: netcf-devel = 0.1.4
%endif
Re: [libvirt] [PATCH 4/4] util: print errno in virObjectLockableNew
- Original Message -
I'm not sure errno is set when using our virMutexInit(). Most of the
code uses virReportError instead, I suggest using that. This should
Actually, the implement of virMutexInit() already set errno when failure:
int virMutexInit(virMutexPtr m)
{
int ret;
pthread_mutexattr_t attr;
pthread_mutexattr_init(attr);
pthread_mutexattr_settype(attr, PTHREAD_MUTEX_NORMAL);
ret = pthread_mutex_init(m-lock, attr);
pthread_mutexattr_destroy(attr);
if (ret != 0) {
errno = ret;
return -1;
}
return 0;
}
be changed everywhere in the code. Rough idea of the places could be
I think it worthy of adding after all virMutexInit, I will include it in
my V2 patchset.
gotten by the following command:
git grep -nA5 virMutexInit | grep SystemError
but as I said, only rough idea :)
Martin
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/4] qemu: fix wrong errno report in qemuMonitorOpenUnix
- Original Message - virProcessKill can be called on cleanup paths. I think it might be wiser to rewrite virProcessKill() to guarantee that it does not modify errno than it is to make all callers have to store a temporary. Hi Eric, this virProcessKill only test whether this process is alive, I think it shouldn't be moved to cleanup path. :P Yes, this is a special use case of virProcessKill in qemuMonitorOpenUnix. But for other use case of virProcessKill, I think errno is really needed, some useful error information will be stored to errno: EINVAL, EPERM, ESRCH. So if we want to guarantee caller no need to store a temporary errno in this curcirmstance, we could just rewrite virProcessKill when signum == 0, do not overwrite errno when failure. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] Changing the default qemu path in libvirt
Hi All, I would like to change the default qemu path in libvirt xml and the changed path has to be used while installing VMs using virt-install. There was a similar question rasied some years back, I could not find the answer to the same. http://www.redhat.com/archives/libvir-list/2011-December/msg01100.html Is that possible right now?, If we have the support already, please point to the same. Thanks in advance. Regards, -Satheesh. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
