Re: [libvirt] make docs fails on EL5
Le mercredi 16 novembre 2011 à 09:12 +0100, Daniel Berteaud a écrit : Le mercredi 16 novembre 2011 à 11:57 +0800, Osier Yang a écrit : 于 2011年11月16日 11:12, shu ming 写道: It seems tit is he same issue as the thread below, the docs can not be generated in some Linux distros. It may be caused by the incompatibility of document tools in these distros. http://www.mail-archive.com/libvir-list@redhat.com/msg47624.html They are not the same problem if I'm correct, as it seems xhtml1-dtd is installed on Daniel's host, (the process broke during generating the html files). And it looks like the error is caused by there is some tags in the html is not campatible with W3C, though not sure why it comes. Not sure if it's relevant, I'm using xhtml1-dtds 1.0-7.1.1 (from the base CentOS 5.7 repo) Nobody else has this error when building on EL5 ? (or maybe nobody builds latest release of libvirt on EL5). Could this be a problem in my mock setup ? Regards, Daniel Regards, Osier -- Daniel Berteaud FIREWALL-SERVICES SARL. Société de Services en Logiciels Libres Technopôle Montesquieu 33650 MARTILLAC Tel : 05 56 64 15 32 Fax : 05 56 64 15 32 Mail: dan...@firewall-services.com Web : http://www.firewall-services.com -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] bug: try to take disk snapshot for LVM2 Volume
Checking the source of the disk will exclude the case when the source disk is block device while the snapshot target is given manually by the users with snapshot-create --xmlfile or virsh snapshot-create-as --diskspec. Why not check the snapshot directory where it will be created? On 2011-11-16 23:45, Eric Blake wrote: On 11/15/2011 08:20 PM, Dave Allan wrote: After working on this some more, I think that identifying problematic file systems, like devtmpfs, is too tricky to be portable. But I think we can meet halfway - right now, the libvirt code blindly generates a filename if one was not provided, regardless of the file type of the backing file it will be wrapping. But maybe it would be sufficient to make the command error out if no qcow2 filename is provided and the backing file is not a regular file. As in this patch: Ok, now I understand the situation; thanks to everyone for the explanations. I'm somewhat uncomfortable using file type as a proxy for troublesome filesystem since although they are linked in this case, I'm not sure they're linked in all cases. Maybe I'm wrong about that. If there is no portable way to determine whether a particular filesystem is going to be a problem, I would just clearly document the limitations of letting libvirt choose the filename and the importance of not using that functionality on filesystems that cannot hold a useful snapshot. My patch would not be preventing snapshots of block devices, but merely requiring that the user supply the qcow2 filename that will wrap the block device. The problem with just documenting the issue is that someone will fail to read the documentation, perform a default snapshot that creates a problematic qcow2 file, then be upset that their domain fails to boot and that they lost all the changes made since the snapshot. I'm still in favor of this patch if anyone else thinks it is worthwhile. Subject: [PATCH] snapshot: refuse to generate names for non-regular backing files For whatever reason, the kernel allows you to create a regular file named /dev/sdc.12345; although this file will disappear the next time devtmpfs is remounted. If you let libvirt generate the name of the external snapshot for a disk image originally using the block device /dev/sdc, then the domain will be rendered unbootable once the qcow2 file is lost on the next devtmpfs remount. In this case, the user should have used 'virsh snapshot-create --xmlfile' or 'virsh snapshot-create-as --diskspec' to specify the name for the qcow2 file in a sane location, rather than relying on libvirt generating a name that is most likely to be wrong. We can help avoid naive mistakes by enforcing that the user provide the external name for any backing file that is not a regular file. * src/conf/domain_conf.c (virDomainSnapshotAlignDisks): Only generate names if backing file exists as regular file. Reported by MATSUDA Daiki. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv2 06/11] add VIR_DOMAIN_NUMATUNE_MEM_NONE
add VIR_DOMAIN_NUMATUNE_MEM_NONE to represent none of the numatune
modes is set
---
src/conf/domain_conf.c |1 +
src/conf/domain_conf.h |1 +
2 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 71eb209..6150b10 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -587,6 +587,7 @@ VIR_ENUM_IMPL(virDomainTimerMode,
VIR_DOMAIN_TIMER_MODE_LAST,
smpsafe);
VIR_ENUM_IMPL(virDomainNumatuneMemMode, VIR_DOMAIN_NUMATUNE_MEM_LAST,
+ none,
strict,
preferred,
interleave);
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 29ac165..1b8b15c 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -1345,6 +1345,7 @@ virDomainVcpuPinDefPtr
virDomainVcpuPinFindByVcpu(virDomainVcpuPinDefPtr *def,
int vcpu);
enum virDomainNumatuneMemMode {
+VIR_DOMAIN_NUMATUNE_MEM_NONE,
VIR_DOMAIN_NUMATUNE_MEM_STRICT,
VIR_DOMAIN_NUMATUNE_MEM_PREFERRED,
VIR_DOMAIN_NUMATUNE_MEM_INTERLEAVE,
--
1.7.3.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv2 10/11] Implement virDomain{G, S}etNumaParameters for the qemu driver
---
src/qemu/qemu_driver.c | 399
1 files changed, 399 insertions(+), 0 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 5f4a18d..5b6398c 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -94,6 +94,8 @@
#define QEMU_NB_MEM_PARAM 3
+#define QEMU_NB_NUMA_PARAM 2
+
#if HAVE_LINUX_KVM_H
# include linux/kvm.h
#endif
@@ -6524,6 +6526,401 @@ cleanup:
return ret;
}
+static int qemuDomainSetNumaParameters(virDomainPtr dom,
+ virTypedParameterPtr params,
+ int nparams,
+ unsigned int flags)
+{
+struct qemud_driver *driver = dom-conn-privateData;
+int i;
+virDomainDefPtr persistentDef = NULL;
+virCgroupPtr group = NULL;
+virDomainObjPtr vm = NULL;
+int ret = -1;
+bool isActive;
+
+virCheckFlags(VIR_DOMAIN_AFFECT_LIVE |
+ VIR_DOMAIN_AFFECT_CONFIG, -1);
+
+qemuDriverLock(driver);
+
+vm = virDomainFindByUUID(driver-domains, dom-uuid);
+
+if (vm == NULL) {
+qemuReportError(VIR_ERR_INTERNAL_ERROR,
+_(No such domain %s), dom-uuid);
+goto cleanup;
+}
+
+isActive = virDomainObjIsActive(vm);
+
+if (flags == VIR_DOMAIN_AFFECT_CURRENT) {
+if (isActive)
+flags = VIR_DOMAIN_AFFECT_LIVE;
+else
+flags = VIR_DOMAIN_AFFECT_CONFIG;
+}
+
+if (flags VIR_DOMAIN_AFFECT_LIVE) {
+if (!isActive) {
+qemuReportError(VIR_ERR_OPERATION_INVALID,
+%s, _(domain is not running));
+goto cleanup;
+}
+
+if (!qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_CPUSET))
{
+qemuReportError(VIR_ERR_OPERATION_INVALID,
+%s, _(cgroup cpuset controller is not
mounted));
+goto cleanup;
+}
+
+if (virCgroupForDomain(driver-cgroup, vm-def-name, group, 0) != 0)
{
+qemuReportError(VIR_ERR_INTERNAL_ERROR,
+_(cannot find cgroup for domain %s),
vm-def-name);
+goto cleanup;
+}
+}
+
+if (flags VIR_DOMAIN_AFFECT_CONFIG) {
+if (!vm-persistent) {
+qemuReportError(VIR_ERR_OPERATION_INVALID, %s,
+_(cannot change persistent config of a transient
domain));
+goto cleanup;
+}
+if (!(persistentDef = virDomainObjGetPersistentDef(driver-caps, vm)))
+goto cleanup;
+}
+
+ret = 0;
+for (i = 0; i nparams; i++) {
+virTypedParameterPtr param = params[i];
+
+if (STREQ(param-field, VIR_DOMAIN_NUMA_NODESET)) {
+int rc;
+if (param-type != VIR_TYPED_PARAM_STRING) {
+qemuReportError(VIR_ERR_INVALID_ARG, %s,
+_(invalid type for numa nodeset tunable,
expected a 'string'));
+ret = -1;
+continue;
+}
+
+if (flags VIR_DOMAIN_AFFECT_LIVE) {
+rc = virCgroupSetCpusetMems(group, params[i].value.s);
+if (rc != 0) {
+virReportSystemError(-rc, %s,
+ _(unable to set memory hard_limit
tunable));
+ret = -1;
+continue;
+}
+}
+
+if (flags VIR_DOMAIN_AFFECT_CONFIG) {
+char *oldnodemask =
strdup(persistentDef-numatune.memory.nodemask);
+if (!oldnodemask) {
+virReportOOMError();
+ret = -1;
+continue;
+}
+if (virDomainCpuSetParse((const char **)params[i].value.s,
+ 0,
+ persistentDef-numatune.memory.nodemask,
+ VIR_DOMAIN_CPUMASK_LEN) 0) {
+VIR_FREE(persistentDef-numatune.memory.nodemask);
+persistentDef-numatune.memory.nodemask = oldnodemask;
+ret = -1;
+continue;
+}
+}
+} else if (STREQ(param-field, VIR_DOMAIN_NUMA_MODE)) {
+int rc;
+if (param-type != VIR_TYPED_PARAM_ULLONG) {
+qemuReportError(VIR_ERR_INVALID_ARG, %s,
+_(invalid type for numa strict tunable,
expected a 'ullong'));
+ret = -1;
+continue;
+}
+
+if (flags VIR_DOMAIN_AFFECT_LIVE) {
+switch(params[i].value.i) {
+case VIR_DOMAIN_NUMATUNE_MEM_STRICT:
+rc = virCgroupSetCpusetHardwall(group, params[i].value.i);
+if (rc != 0) {
+virReportSystemError(-rc, %s,
+
[libvirt] [PATCHv2 01/11] don't modify CPU set string in virDomainCpuSetParse
As the function only parses the CPU set string, there is no good reason to modify it. --- src/conf/domain_conf.c |1 - 1 files changed, 0 insertions(+), 1 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 6b78d97..71eb209 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -9163,7 +9163,6 @@ virDomainCpuSetParse(const char **str, char sep, } else goto parse_error; } -*str = cur; return (ret); parse_error: -- 1.7.3.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv2 05/11] use cpuset to manage numa
This patch deprecates libnuma and uses cpuset to manage numa.
We can further add a `numatune' command to adjust numa parameters
through cpuset.
---
src/qemu/qemu_cgroup.c | 73
1 files changed, 73 insertions(+), 0 deletions(-)
diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c
index 2a10bd2..9dd67b7 100644
--- a/src/qemu/qemu_cgroup.c
+++ b/src/qemu/qemu_cgroup.c
@@ -365,6 +365,79 @@ int qemuSetupCgroup(struct qemud_driver *driver,
}
}
+if (vm-def-numatune.memory.nodemask
+vm-def-numatune.backend == VIR_DOMAIN_NUMATUNE_BACKEND_NONE) {
+if (qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_CPUSET)) {
+int mode = vm-def-numatune.memory.mode;
+char *mask =
virDomainCpuSetFormat(vm-def-numatune.memory.nodemask,
VIR_DOMAIN_CPUMASK_LEN);
+if (!mask) {
+qemuReportError(VIR_ERR_INTERNAL_ERROR,
+_(failed to convert memory nodemask));
+goto cleanup;
+}
+
+rc = virCgroupSetCpusetMems(cgroup, mask);
+if (rc != 0) {
+virReportSystemError(-rc,
+ _(Unable to set cpuset.mems for domain
%s),
+ vm-def-name);
+goto cleanup;
+}
+switch (mode) {
+case VIR_DOMAIN_NUMATUNE_MEM_STRICT:
+rc = virCgroupSetCpusetHardwall(cgroup, 1);
+if (rc != 0) {
+virReportSystemError(-rc,
+ _(Unable to set cpuset.mem_hardwall
for domain %s),
+ vm-def-name);
+goto cleanup;
+}
+break;
+case VIR_DOMAIN_NUMATUNE_MEM_PREFERRED:
+rc = virCgroupSetCpusetMemorySpreadPage(cgroup, 0);
+if (rc != 0) {
+virReportSystemError(-rc,
+ _(Unable to set
cpuset.memory_spread_page for domain %s),
+ vm-def-name);
+goto cleanup;
+}
+rc = virCgroupSetCpusetMemorySpreadSlab(cgroup, 0);
+if (rc != 0) {
+virReportSystemError(-rc,
+ _(Unable to set
cpuset.memory_spread_slab for domain %s),
+ vm-def-name);
+goto cleanup;
+}
+break;
+case VIR_DOMAIN_NUMATUNE_MEM_INTERLEAVE:
+rc = virCgroupSetCpusetMemorySpreadPage(cgroup, 1);
+if (rc != 0) {
+virReportSystemError(-rc,
+ _(Unable to set
cpuset.memory_spread_page for domain %s),
+ vm-def-name);
+goto cleanup;
+}
+rc = virCgroupSetCpusetMemorySpreadSlab(cgroup, 1);
+if (rc != 0) {
+virReportSystemError(-rc,
+ _(Unable to set
cpuset.memory_spread_slab for domain %s),
+ vm-def-name);
+goto cleanup;
+}
+break;
+default:
+qemuReportError(VIR_ERR_XML_ERROR,
+%s, _(Invalid mode for memory NUMA
tuning.));
+goto cleanup;
+}
+vm-def-numatune.backend =
VIR_DOMAIN_NUMATUNE_BACKEND_CGROUP_CPUSET;
+} else {
+qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+_(Unable to set numa for domain %s since cpuset
cgroup is
+ not available on this host), vm-def-name);
+goto cleanup;
+}
+}
done:
virCgroupFree(cgroup);
return 0;
--
1.7.3.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv2 02/11] enable cgroup cpuset by default
This prepares for subsequent patches which introduce dependence
on cgroup cpuset. Enable cgroup cpuset by default so users don't
have to modify configuration file before encountering a cpuset
error.
---
src/qemu/qemu.conf |5 +++--
src/qemu/qemu_conf.c |3 ++-
2 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/src/qemu/qemu.conf b/src/qemu/qemu.conf
index 4da5d5a..1c14d8f 100644
--- a/src/qemu/qemu.conf
+++ b/src/qemu/qemu.conf
@@ -157,18 +157,19 @@
# - 'devices' - use for device whitelisting
# - 'memory' - use for memory tunables
# - 'blkio' - use for block devices I/O tunables
+# - 'cpuset' - use for CPUs and memory nodes
#
# NB, even if configured here, they won't be used unless
# the administrator has mounted cgroups, e.g.:
#
# mkdir /dev/cgroup
-# mount -t cgroup -o devices,cpu,memory,blkio none /dev/cgroup
+# mount -t cgroup -o devices,cpu,memory,blkio,cpuset none /dev/cgroup
#
# They can be mounted anywhere, and different controllers
# can be mounted in different locations. libvirt will detect
# where they are located.
#
-# cgroup_controllers = [ cpu, devices, memory, blkio ]
+# cgroup_controllers = [ cpu, devices, memory, blkio, cpuset ]
# This is the basic set of devices allowed / required by
# all virtual machines.
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index d7e9d7d..dfcc6e3 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -304,7 +304,8 @@ int qemudLoadDriverConfig(struct qemud_driver *driver,
(1 VIR_CGROUP_CONTROLLER_CPU) |
(1 VIR_CGROUP_CONTROLLER_DEVICES) |
(1 VIR_CGROUP_CONTROLLER_MEMORY) |
-(1 VIR_CGROUP_CONTROLLER_BLKIO);
+(1 VIR_CGROUP_CONTROLLER_BLKIO) |
+(1 VIR_CGROUP_CONTROLLER_CPUSET);
}
for (i = 0 ; i VIR_CGROUP_CONTROLLER_LAST ; i++) {
if (driver-cgroupControllers (1 i)) {
--
1.7.3.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv2 03/11] Add functions to set/get cgroup cpuset parameters
---
src/libvirt_private.syms | 10
src/util/cgroup.c| 112 ++
src/util/cgroup.h| 11 +
3 files changed, 133 insertions(+), 0 deletions(-)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index b9d537e..d48b7dc 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -78,6 +78,11 @@ virCgroupGetCpuShares;
virCgroupGetCpuCfsPeriod;
virCgroupGetCpuCfsQuota;
virCgroupGetCpuacctUsage;
+virCgroupGetCpusetHardwall;
+virCgroupGetCpusetMemExclusive;
+virCgroupGetCpusetMemorySpreadPage;
+virCgroupGetCpusetMemorySpreadSlab;
+virCgroupGetCpusetMems;
virCgroupGetFreezerState;
virCgroupGetMemoryHardLimit;
virCgroupGetMemorySoftLimit;
@@ -93,6 +98,11 @@ virCgroupSetBlkioWeight;
virCgroupSetCpuShares;
virCgroupSetCpuCfsPeriod;
virCgroupSetCpuCfsQuota;
+virCgroupSetCpusetHardwall;
+virCgroupSetCpusetMemExclusive;
+virCgroupSetCpusetMemorySpreadPage;
+virCgroupSetCpusetMemorySpreadSlab;
+virCgroupSetCpusetMems;
virCgroupSetFreezerState;
virCgroupSetMemory;
virCgroupSetMemoryHardLimit;
diff --git a/src/util/cgroup.c b/src/util/cgroup.c
index c8d1f33..c9cd90b 100644
--- a/src/util/cgroup.c
+++ b/src/util/cgroup.c
@@ -1154,6 +1154,118 @@ int virCgroupGetMemSwapHardLimit(virCgroupPtr group,
unsigned long long *kb)
}
/**
+ * virCgroupSetCpusetMems:
+ *
+ * @group: The cgroup to set cpuset.mems for
+ * @mems: the numa nodes to set
+ *
+ * Returns: 0 on success
+ */
+int virCgroupSetCpusetMems(virCgroupPtr group, const char *mems)
+{
+return virCgroupSetValueStr(group,
+VIR_CGROUP_CONTROLLER_CPUSET,
+cpuset.mems,
+mems);
+}
+
+/**
+ * virCgroupGetCpusetMems:
+ *
+ * @group: The cgroup to get cpuset.mems for
+ * @mems: the numa nodes to get
+ *
+ * Returns: 0 on success
+ */
+int virCgroupGetCpusetMems(virCgroupPtr group, char **mems)
+{
+return virCgroupGetValueStr(group,
+VIR_CGROUP_CONTROLLER_CPUSET,
+cpuset.mems,
+mems);
+}
+
+/**
+ * virCgroupSetCpusetHardwall:
+ *
+ * @group: The cgroup to set cpuset.mems for
+ * @hardwall: the hardwall to set
+ *
+ * Returns: 0 on success
+ */
+int virCgroupSetCpusetHardwall(virCgroupPtr group, unsigned long long hardwall)
+{
+return virCgroupSetValueU64(group,
+VIR_CGROUP_CONTROLLER_CPUSET,
+cpuset.mem_hardwall,
+hardwall);
+}
+
+/**
+ * virCgroupGetCpusetHardwall:
+ *
+ * @group: The cgroup to get cpuset.mem_hardwall for
+ * @hardwall: the hardwall to get
+ *
+ * Returns: 0 on success
+ */
+int virCgroupGetCpusetHardwall(virCgroupPtr group, unsigned long long
*hardwall)
+{
+return virCgroupGetValueU64(group,
+VIR_CGROUP_CONTROLLER_CPUSET,
+cpuset.mem_hardwall,
+hardwall);
+}
+
+int virCgroupSetCpusetMemExclusive(virCgroupPtr group, unsigned long long
memExclusive)
+{
+return virCgroupSetValueU64(group,
+VIR_CGROUP_CONTROLLER_CPUSET,
+cpuset.mem_exclusive,
+memExclusive);
+}
+
+int virCgroupGetCpusetMemExclusive(virCgroupPtr group, unsigned long long
*memExclusive)
+{
+return virCgroupGetValueU64(group,
+VIR_CGROUP_CONTROLLER_CPUSET,
+cpuset.mem_exclusive,
+memExclusive);
+}
+
+int virCgroupSetCpusetMemorySpreadPage(virCgroupPtr group, unsigned long long
memSpreadPage)
+{
+return virCgroupSetValueU64(group,
+VIR_CGROUP_CONTROLLER_CPUSET,
+cpuset.memory_spread_page,
+memSpreadPage);
+}
+
+int virCgroupGetCpusetMemorySpreadPage(virCgroupPtr group, unsigned long long
*memSpreadPage)
+{
+return virCgroupGetValueU64(group,
+VIR_CGROUP_CONTROLLER_CPUSET,
+cpuset.memory_spread_page,
+memSpreadPage);
+}
+
+int virCgroupSetCpusetMemorySpreadSlab(virCgroupPtr group, unsigned long long
memSpreadSlab)
+{
+return virCgroupSetValueU64(group,
+VIR_CGROUP_CONTROLLER_CPUSET,
+cpuset.memory_spread_slab,
+memSpreadSlab);
+}
+
+int virCgroupGetCpusetMemorySpreadSlab(virCgroupPtr group, unsigned long long
*memSpreadSlab)
+{
+return virCgroupGetValueU64(group,
+VIR_CGROUP_CONTROLLER_CPUSET,
+cpuset.memory_spread_slab,
+memSpreadSlab);
+}
+
+/**
* virCgroupDenyAllDevices:
*
* @group: The cgroup to
[libvirt] [PATCHv2 11/11] add new command numatune to virsh
add new command numatune to virsh to get/set numa parameters
---
tools/virsh.c | 180 +
1 files changed, 180 insertions(+), 0 deletions(-)
diff --git a/tools/virsh.c b/tools/virsh.c
index 89fb4e7..3dfa375 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -60,6 +60,7 @@
#include command.h
#include virkeycode.h
#include virnetdevbandwidth.h
+#include conf/domain_conf.h
static char *progname;
@@ -4975,6 +4976,184 @@ cmdMemtune(vshControl * ctl, const vshCmd * cmd)
}
/*
+ * numatune command
+ */
+static const vshCmdInfo info_numatune[] = {
+{help, N_(Get or set numa parameters)},
+{desc, N_(Get or set the current numa parameters for a guest \
+ domain.\n \
+To get the numa parameters use following command: \n\n \
+virsh # numatune domain)},
+{NULL, NULL}
+
+};
+
+static const vshCmdOptDef opts_numatune[] = {
+{domain, VSH_OT_DATA, VSH_OFLAG_REQ, N_(domain name, id or uuid)},
+{nodeset, VSH_OT_DATA, VSH_OFLAG_NONE,
+ N_(NUMA node to set)},
+{mode, VSH_OT_DATA, VSH_OFLAG_NONE,
+ N_(NUMA mode, one of strict, preferred and interleave)},
+{config, VSH_OT_BOOL, 0, N_(affect next boot)},
+{live, VSH_OT_BOOL, 0, N_(affect running domain)},
+{current, VSH_OT_BOOL, 0, N_(affect current domain)},
+{NULL, 0, 0, NULL}
+};
+
+static bool
+cmdNumatune(vshControl * ctl, const vshCmd * cmd)
+{
+virDomainPtr dom;
+int nparams = 0;
+unsigned int i = 0;
+virTypedParameterPtr params = NULL, temp = NULL;
+const char *nodeset = NULL;
+bool ret = false;
+unsigned int flags = 0;
+int current = vshCommandOptBool(cmd, current);
+int config = vshCommandOptBool(cmd, config);
+int live = vshCommandOptBool(cmd, live);
+const char *mode = NULL;
+
+if (current) {
+if (live || config) {
+vshError(ctl, %s, _(--current must be specified exclusively));
+return false;
+}
+flags = VIR_DOMAIN_AFFECT_CURRENT;
+} else {
+if (config)
+flags |= VIR_DOMAIN_AFFECT_CONFIG;
+if (live)
+flags |= VIR_DOMAIN_AFFECT_LIVE;
+}
+
+if (!vshConnectionUsability(ctl, ctl-conn))
+return false;
+
+if (!(dom = vshCommandOptDomain(ctl, cmd, NULL)))
+return false;
+
+if (vshCommandOptString(cmd, nodeset, nodeset) 0) {
+vshError(ctl, %s, _(Unable to parse nodeset.));
+virDomainFree(dom);
+return false;
+}
+if (nodeset)
+nparams++;
+if (vshCommandOptString(cmd, mode, mode) 0) {
+vshError(ctl, %s, _(Unable to parse mode.));
+virDomainFree(dom);
+return false;
+}
+if (mode)
+nparams++;
+
+if (nparams == 0) {
+/* get the number of numa parameters */
+if (virDomainGetNumaParameters(dom, NULL, nparams, flags) != 0) {
+vshError(ctl, %s,
+ _(Unable to get number of memory parameters));
+goto cleanup;
+}
+
+if (nparams == 0) {
+/* nothing to output */
+ret = true;
+goto cleanup;
+}
+
+/* now go get all the numa parameters */
+params = vshCalloc(ctl, nparams, sizeof(*params));
+if (virDomainGetNumaParameters(dom, params, nparams, flags) != 0) {
+vshError(ctl, %s, _(Unable to get numa parameters));
+goto cleanup;
+}
+
+for (i = 0; i nparams; i++) {
+switch (params[i].type) {
+case VIR_TYPED_PARAM_INT:
+vshPrint(ctl, %-15s: %d\n, params[i].field,
+ params[i].value.i);
+break;
+case VIR_TYPED_PARAM_UINT:
+vshPrint(ctl, %-15s: %u\n, params[i].field,
+ params[i].value.ui);
+break;
+case VIR_TYPED_PARAM_LLONG:
+vshPrint(ctl, %-15s: %lld\n, params[i].field,
+ params[i].value.l);
+break;
+case VIR_TYPED_PARAM_ULLONG:
+if (STREQ(params[i].field, VIR_DOMAIN_NUMA_MODE))
+vshPrint(ctl, %-15s: %s\n, params[i].field,
+
virDomainNumatuneMemModeTypeToString(params[i].value.ul));
+else
+vshPrint(ctl, %-15s: %llu\n, params[i].field,
+ params[i].value.ul);
+break;
+case VIR_TYPED_PARAM_DOUBLE:
+vshPrint(ctl, %-15s: %f\n, params[i].field,
+ params[i].value.d);
+break;
+case VIR_TYPED_PARAM_BOOLEAN:
+vshPrint(ctl, %-15s: %d\n, params[i].field,
+ params[i].value.b);
+break;
+
[libvirt] [RFC][PATCHv2 00/11] add numatune command
This series does mainly two things:
1. use cgroup cpuset to manage numa parameters
2. add a virsh command numatune to allow user to change numa parameters
from command line
Current numa parameters include nodeset and mode, but these cgroup cpuset
provides don't completely match with them, details:
params cpuset
--
nodeset cpuset provides cpuset.mems
mode strict cpuset provides cpuset.mem_hardwall
mode interleave cpuset provices cpuset.memory_spread_*
mode preferred no equivalent. !spread to preferred?
Besides, only one of the mode can be set currently at a time, but
for cpuset, the parameters are independent. From the perspective
of cpuset, we can set all the modes values independently, but it
seems not be consistent with the current numatune definition in xml.
Maybe we can improve the xml definition to fit cpuset better?(there
are more cpuset parameters than listed above)
Hu Tao (11):
don't modify CPU set string in virDomainCpuSetParse
enable cgroup cpuset by default
Add functions to set/get cgroup cpuset parameters
introduce numa backend
use cpuset to manage numa
add VIR_DOMAIN_NUMATUNE_MEM_NONE
add new API virDomain{G,S}etNumaParameters
Implement main entries of virDomain{G,S}etNumaParameters
Add virDomain{G,S}etNumaParameters support to the remote driver
Implement virDomain{G,S}etNumaParameters for the qemu driver
add new command numatune to virsh
daemon/remote.c | 64 +++
include/libvirt/libvirt.h.in | 23 +++
python/generator.py |2 +
src/conf/domain_conf.c |2 +-
src/conf/domain_conf.h |9 +
src/driver.h | 15 ++
src/libvirt.c| 113
src/libvirt_private.syms | 10 +
src/libvirt_public.syms |6 +
src/qemu/qemu.conf |5 +-
src/qemu/qemu_cgroup.c | 73
src/qemu/qemu_conf.c |3 +-
src/qemu/qemu_driver.c | 399 ++
src/qemu/qemu_process.c | 11 +-
src/remote/remote_driver.c | 50 ++
src/remote/remote_protocol.x | 25 +++-
src/remote_protocol-structs | 16 ++
src/util/cgroup.c| 112
src/util/cgroup.h| 11 ++
tools/virsh.c| 180 +++
20 files changed, 1121 insertions(+), 8 deletions(-)
--
1.7.3.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv2 04/11] introduce numa backend
Currently we are using libnuma to set up numa, but it's desired to
use cgroup cpuset to do the job instead. But for old systems that
don't have cgroup cpuset, we fall back to libnuma.
---
src/conf/domain_conf.h |8
src/qemu/qemu_process.c | 11 ---
2 files changed, 16 insertions(+), 3 deletions(-)
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index c360674..29ac165 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -1352,6 +1352,12 @@ enum virDomainNumatuneMemMode {
VIR_DOMAIN_NUMATUNE_MEM_LAST
};
+enum virDomainNumatuneBackend {
+VIR_DOMAIN_NUMATUNE_BACKEND_NONE,
+VIR_DOMAIN_NUMATUNE_BACKEND_LIBNUMA,
+VIR_DOMAIN_NUMATUNE_BACKEND_CGROUP_CPUSET,
+};
+
typedef struct _virDomainNumatuneDef virDomainNumatuneDef;
typedef virDomainNumatuneDef *virDomainNumatuneDefPtr;
struct _virDomainNumatuneDef {
@@ -1360,6 +1366,8 @@ struct _virDomainNumatuneDef {
int mode;
} memory;
+int backend;
+
/* Future NUMA tuning related stuff should go here. */
};
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 2882ef8..8add0b8 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -1334,12 +1334,16 @@ qemuProcessInitNumaMemoryPolicy(virDomainObjPtr vm)
if (!vm-def-numatune.memory.nodemask)
return 0;
+if (vm-def-numatune.backend != VIR_DOMAIN_NUMATUNE_BACKEND_NONE)
+return 0;
+vm-def-numatune.backend = VIR_DOMAIN_NUMATUNE_BACKEND_LIBNUMA;
+
VIR_DEBUG(Setting NUMA memory policy);
if (numa_available() 0) {
qemuReportError(VIR_ERR_INTERNAL_ERROR,
%s, _(Host kernel is not aware of NUMA.));
-return -1;
+goto cleanup;
}
maxnode = numa_max_node() + 1;
@@ -1351,7 +1355,7 @@ qemuProcessInitNumaMemoryPolicy(virDomainObjPtr vm)
if (i NUMA_NUM_NODES) {
qemuReportError(VIR_ERR_INTERNAL_ERROR,
_(Host cannot support NUMA node %d), i);
-return -1;
+goto cleanup;
}
if (i maxnode !warned) {
VIR_WARN(nodeset is out of range, there is only %d NUMA
@@ -1397,9 +1401,10 @@ qemuProcessInitNumaMemoryPolicy(virDomainObjPtr vm)
goto cleanup;
}
-ret = 0;
+return 0;
cleanup:
+vm-def-numatune.backend = VIR_DOMAIN_NUMATUNE_BACKEND_NONE;
return ret;
}
#else
--
1.7.3.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv2 09/11] Add virDomain{G, S}etNumaParameters support to the remote driver
---
daemon/remote.c | 64 ++
src/remote/remote_driver.c | 50
src/remote/remote_protocol.x | 25 +++-
src/remote_protocol-structs | 16 ++
4 files changed, 154 insertions(+), 1 deletions(-)
diff --git a/daemon/remote.c b/daemon/remote.c
index 97c9538..9ccae47 100644
--- a/daemon/remote.c
+++ b/daemon/remote.c
@@ -1624,6 +1624,70 @@ cleanup:
}
static int
+remoteDispatchDomainGetNumaParameters(virNetServerPtr server ATTRIBUTE_UNUSED,
+ virNetServerClientPtr client
ATTRIBUTE_UNUSED,
+ virNetMessagePtr msg ATTRIBUTE_UNUSED,
+ virNetMessageErrorPtr rerr,
+ remote_domain_get_numa_parameters_args
*args,
+ remote_domain_get_numa_parameters_ret
*ret)
+{
+virDomainPtr dom = NULL;
+virTypedParameterPtr params = NULL;
+int nparams = args-nparams;
+unsigned int flags;
+int rv = -1;
+struct daemonClientPrivate *priv =
+virNetServerClientGetPrivateData(client);
+
+if (!priv-conn) {
+virNetError(VIR_ERR_INTERNAL_ERROR, %s, _(connection not open));
+goto cleanup;
+}
+
+flags = args-flags;
+
+if (nparams REMOTE_DOMAIN_MEMORY_PARAMETERS_MAX) {
+virNetError(VIR_ERR_INTERNAL_ERROR, %s, _(nparams too large));
+goto cleanup;
+}
+if (VIR_ALLOC_N(params, nparams) 0) {
+virReportOOMError();
+goto cleanup;
+}
+
+if (!(dom = get_nonnull_domain(priv-conn, args-dom)))
+goto cleanup;
+
+if (virDomainGetNumaParameters(dom, params, nparams, flags) 0)
+goto cleanup;
+
+/* In this case, we need to send back the number of parameters
+ * supported
+ */
+if (args-nparams == 0) {
+ret-nparams = nparams;
+goto success;
+}
+
+if (remoteSerializeTypedParameters(params, nparams,
+ ret-params.params_val,
+ ret-params.params_len,
+ flags) 0)
+goto cleanup;
+
+success:
+rv = 0;
+
+cleanup:
+if (rv 0)
+virNetMessageSaveError(rerr);
+if (dom)
+virDomainFree(dom);
+VIR_FREE(params);
+return rv;
+}
+
+static int
remoteDispatchDomainGetBlkioParameters(virNetServerPtr server ATTRIBUTE_UNUSED,
virNetServerClientPtr client
ATTRIBUTE_UNUSED,
virNetMessagePtr msg ATTRIBUTE_UNUSED,
diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
index 94fd3e7..32342cc 100644
--- a/src/remote/remote_driver.c
+++ b/src/remote/remote_driver.c
@@ -1514,6 +1514,54 @@ done:
}
static int
+remoteDomainGetNumaParameters (virDomainPtr domain,
+ virTypedParameterPtr params, int *nparams,
+ unsigned int flags)
+{
+int rv = -1;
+remote_domain_get_numa_parameters_args args;
+remote_domain_get_numa_parameters_ret ret;
+struct private_data *priv = domain-conn-privateData;
+
+remoteDriverLock(priv);
+
+make_nonnull_domain (args.dom, domain);
+args.nparams = *nparams;
+args.flags = flags;
+
+memset (ret, 0, sizeof ret);
+if (call (domain-conn, priv, 0, REMOTE_PROC_DOMAIN_GET_NUMA_PARAMETERS,
+ (xdrproc_t) xdr_remote_domain_get_numa_parameters_args, (char *)
args,
+ (xdrproc_t) xdr_remote_domain_get_numa_parameters_ret, (char *)
ret) == -1)
+goto done;
+
+/* Handle the case when the caller does not know the number of parameters
+ * and is asking for the number of parameters supported
+ */
+if (*nparams == 0) {
+*nparams = ret.nparams;
+rv = 0;
+goto cleanup;
+}
+
+if (remoteDeserializeTypedParameters(ret.params.params_val,
+ ret.params.params_len,
+ REMOTE_DOMAIN_NUMA_PARAMETERS_MAX,
+ params,
+ nparams) 0)
+goto cleanup;
+
+rv = 0;
+
+cleanup:
+xdr_free ((xdrproc_t) xdr_remote_domain_get_numa_parameters_ret,
+ (char *) ret);
+done:
+remoteDriverUnlock(priv);
+return rv;
+}
+
+static int
remoteDomainGetBlkioParameters (virDomainPtr domain,
virTypedParameterPtr params, int *nparams,
unsigned int flags)
@@ -4550,6 +4598,8 @@ static virDriver remote_driver = {
.domainGetBlockJobInfo = remoteDomainGetBlockJobInfo, /* 0.9.4 */
.domainBlockJobSetSpeed = remoteDomainBlockJobSetSpeed, /* 0.9.4 */
.domainBlockPull = remoteDomainBlockPull, /* 0.9.4 */
+.domainSetNumaParameters = remoteDomainSetNumaParameters,
Re: [libvirt] [PATCH 0/2] Fix build with polkit0
On Wed, Nov 16, 2011 at 11:46:27AM -0700, Jim Fehlig wrote: Commit 0f590c62 was not the proper fix for polkit0 build issue. This series reverts 0f590c62 and adds virNetServerGetDBusConn() to libvirt_private.syms Jim Fehlig (2): Revert commit 0f590c62 Fix build with polkit0 daemon/Makefile.am |4 src/libvirt_private.syms |1 + 2 files changed, 1 insertions(+), 4 deletions(-) ACK, thanks for taking the time to change this. Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv2 08/11] Implement main entries of virDomain{G, S}etNumaParameters
---
src/libvirt.c | 113 +
1 files changed, 113 insertions(+), 0 deletions(-)
diff --git a/src/libvirt.c b/src/libvirt.c
index 1518ed2..4eaf297 100644
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -3762,6 +3762,119 @@ error:
}
/**
+ * virDomainSetNumaParameters:
+ * @domain: pointer to domain object
+ * @params: pointer to numa parameter objects
+ * @nparams: number of numa parameter (this value can be the same or
+ * less than the number of parameters supported)
+ * @flags: bitwise-OR of virDomainModificationImpact
+ *
+ * Change all or a subset of the numa tunables.
+ * This function may require privileged access to the hypervisor.
+ *
+ * Returns -1 in case of error, 0 in case of success.
+ */
+int virDomainSetNumaParameters(virDomainPtr domain,
+ virTypedParameterPtr params,
+ int nparams, unsigned int flags)
+{
+virConnectPtr conn;
+
+VIR_DOMAIN_DEBUG(domain, params=%p, nparams=%d, flags=%x,
+ params, nparams, flags);
+
+virResetLastError();
+
+if (!VIR_IS_CONNECTED_DOMAIN(domain)) {
+virLibDomainError(VIR_ERR_INVALID_DOMAIN, __FUNCTION__);
+virDispatchError(NULL);
+return -1;
+}
+if (domain-conn-flags VIR_CONNECT_RO) {
+virLibDomainError(VIR_ERR_OPERATION_DENIED, __FUNCTION__);
+goto error;
+}
+if ((nparams = 0) || (params == NULL)) {
+virLibDomainError(VIR_ERR_INVALID_ARG, __FUNCTION__);
+goto error;
+}
+if (virTypedParameterValidateSet(domain, params, nparams) 0)
+return -1;
+
+conn = domain-conn;
+
+if (conn-driver-domainSetNumaParameters) {
+int ret;
+ret = conn-driver-domainSetNumaParameters (domain, params, nparams,
flags);
+if (ret 0)
+goto error;
+return ret;
+}
+
+virLibConnError(VIR_ERR_NO_SUPPORT, __FUNCTION__);
+
+error:
+virDispatchError(domain-conn);
+return -1;
+}
+
+/**
+ * virDomainGetNumaParameters:
+ * @domain: pointer to domain object
+ * @params: pointer to numa parameter object
+ * (return value, allocated by the caller)
+ * @nparams: pointer to number of numa parameters
+ * @flags: one of virDomainModificationImpact
+ *
+ * This function may require privileged access to the hypervisor. This function
+ * expects the caller to allocate the @params.
+ *
+ * Returns -1 in case of error, 0 in case of success.
+ */
+
+int
+virDomainGetNumaParameters(virDomainPtr domain,
+ virTypedParameterPtr params,
+ int *nparams, unsigned int flags)
+{
+virConnectPtr conn;
+
+VIR_DOMAIN_DEBUG(domain, params=%p, nparams=%d, flags=%x,
+ params, (nparams) ? *nparams : -1, flags);
+
+virResetLastError();
+
+if (!VIR_IS_CONNECTED_DOMAIN(domain)) {
+virLibDomainError(VIR_ERR_INVALID_DOMAIN, __FUNCTION__);
+virDispatchError(NULL);
+return -1;
+}
+if ((nparams == NULL) || (*nparams 0) ||
+(params == NULL *nparams != 0)) {
+virLibDomainError(VIR_ERR_INVALID_ARG, __FUNCTION__);
+goto error;
+}
+if (VIR_DRV_SUPPORTS_FEATURE(domain-conn-driver, domain-conn,
+ VIR_DRV_FEATURE_TYPED_PARAM_STRING))
+flags |= VIR_TYPED_PARAM_STRING_OKAY;
+
+conn = domain-conn;
+
+if (conn-driver-domainGetNumaParameters) {
+int ret;
+ret = conn-driver-domainGetNumaParameters (domain, params, nparams,
flags);
+if (ret 0)
+goto error;
+return ret;
+}
+virLibConnError(VIR_ERR_NO_SUPPORT, __FUNCTION__);
+
+error:
+virDispatchError(domain-conn);
+return -1;
+}
+
+/**
* virDomainSetBlkioParameters:
* @domain: pointer to domain object
* @params: pointer to blkio parameter objects
--
1.7.3.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv2 07/11] add new API virDomain{G, S}etNumaParameters
Set up the types for the numa functions and insert them into the
virDriver structure definition.
---
include/libvirt/libvirt.h.in | 23 +++
python/generator.py |2 ++
src/driver.h | 15 +++
src/libvirt_public.syms |6 ++
4 files changed, 46 insertions(+), 0 deletions(-)
diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
index 2ab89f5..7ce6352 100644
--- a/include/libvirt/libvirt.h.in
+++ b/include/libvirt/libvirt.h.in
@@ -1311,6 +1311,29 @@ typedef enum {
} virDomainMemoryModFlags;
+/* Manage numa parameters */
+
+/**
+ * VIR_DOMAIN_NUMA_NODESET:
+ *
+ * numa nodeset
+ */
+#define VIR_DOMAIN_NUMA_NODESET numa_nodeset
+
+/**
+ * VIR_DOMAIN_NUMA_MODE:
+ *
+ * numa mode
+ */
+#define VIR_DOMAIN_NUMA_MODE numa_mode
+
+int virDomainSetNumaParameters(virDomainPtr domain,
+ virTypedParameterPtr params,
+ int nparams, unsigned int flags);
+int virDomainGetNumaParameters(virDomainPtr domain,
+ virTypedParameterPtr params,
+ int *nparams, unsigned int flags);
+
/*
* Dynamic control of domains
*/
diff --git a/python/generator.py b/python/generator.py
index 71afdb7..5b09123 100755
--- a/python/generator.py
+++ b/python/generator.py
@@ -382,6 +382,8 @@ skip_impl = (
'virDomainGetBlkioParameters',
'virDomainSetMemoryParameters',
'virDomainGetMemoryParameters',
+'virDomainSetNumaParameters',
+'virDomainGetNumaParameters',
'virDomainGetVcpus',
'virDomainPinVcpu',
'virDomainPinVcpuFlags',
diff --git a/src/driver.h b/src/driver.h
index 4c14aaa..76cf966 100644
--- a/src/driver.h
+++ b/src/driver.h
@@ -158,6 +158,19 @@ typedef int
int *nparams,
unsigned int flags);
typedef int
+(*virDrvDomainSetNumaParameters)
+(virDomainPtr domain,
+ virTypedParameterPtr params,
+ int nparams,
+ unsigned int flags);
+typedef int
+(*virDrvDomainGetNumaParameters)
+(virDomainPtr domain,
+ virTypedParameterPtr params,
+ int *nparams,
+ unsigned int flags);
+
+typedef int
(*virDrvDomainSetBlkioParameters)
(virDomainPtr domain,
virTypedParameterPtr params,
@@ -787,6 +800,8 @@ struct _virDriver {
virDrvDomainSetMemoryFlags domainSetMemoryFlags;
virDrvDomainSetMemoryParameters domainSetMemoryParameters;
virDrvDomainGetMemoryParameters domainGetMemoryParameters;
+virDrvDomainSetNumaParameters domainSetNumaParameters;
+virDrvDomainGetNumaParameters domainGetNumaParameters;
virDrvDomainSetBlkioParameters domainSetBlkioParameters;
virDrvDomainGetBlkioParameters domainGetBlkioParameters;
virDrvDomainGetInfodomainGetInfo;
diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms
index bcefb10..d830974 100644
--- a/src/libvirt_public.syms
+++ b/src/libvirt_public.syms
@@ -498,4 +498,10 @@ LIBVIRT_0.9.7 {
virDomainSnapshotNumChildren;
} LIBVIRT_0.9.5;
+LIBVIRT_0.9.8 {
+global:
+virDomainGetNumaParameters;
+virDomainSetNumaParameters;
+} LIBVIRT_0.9.7;
+
# define new API here using predicted next version number
--
1.7.3.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 02/16] Rename and split the macvtap.c file
On Thu, Nov 17, 2011 at 05:49:10AM -0500, Laine Stump wrote: On 11/15/2011 06:14 AM, Daniel P. Berrange wrote: From: Daniel P. Berrangeberra...@redhat.com Rename the macvtap.c file to virnetdevmacvlan.c to reflect its functionality. Move the port profile association code out into virnetdevvportprofile.c. Make the APIs available unconditionally to callers One concern I have after reading this sentence - do you changes have any implications for systems that don't have the correct version of libnl, and thus cannot have macvtap enabled? (e.g. RHEL5) Assuming that the answer to the above is no, and going on faith (and the fact that make make check passed) that the gigantic diff really is just moving identical code between files (except for removing the #ifdef MACVTAPs in several places) - ACK. The goal of this entire series, is that all *header* files definitions should be unconditionally available. The idea is that callers (eg the QEMU driver) can then avoid be littered with conditionals like #ifdef MACVTAP The implementation source files will be the sole place where the conditionals live, in the #else clauses providing stubs for every API which simply raise a libvirt error with ENOSYS Overall this should make our code more robust wrt the various different conditional compile options 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 02/16] Rename and split the macvtap.c file
On 11/15/2011 06:14 AM, Daniel P. Berrange wrote: From: Daniel P. Berrangeberra...@redhat.com Rename the macvtap.c file to virnetdevmacvlan.c to reflect its functionality. Move the port profile association code out into virnetdevvportprofile.c. Make the APIs available unconditionally to callers One concern I have after reading this sentence - do you changes have any implications for systems that don't have the correct version of libnl, and thus cannot have macvtap enabled? (e.g. RHEL5) Assuming that the answer to the above is no, and going on faith (and the fact that make make check passed) that the gigantic diff really is just moving identical code between files (except for removing the #ifdef MACVTAPs in several places) - ACK. (It might be a useful exercise to build on some system with no macvtap|libnl support to make sure nothing blows up). -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 01/16] Rename Macvtap management APIs
On 11/15/2011 06:14 AM, Daniel P. Berrange wrote: From: Daniel P. Berrangeberra...@redhat.com In preparation for code re-organization, rename the Macvtap management APIs to have the following patterns virNetDevMacVLanX - macvlan/macvtap interface management virNetDevVPortProfile - virtual port profile management * src/util/macvtap.c, src/util/macvtap.h: Rename APIs * src/conf/domain_conf.c, src/network/bridge_driver.c, src/qemu/qemu_command.c, src/qemu/qemu_command.h, src/qemu/qemu_driver.c, src/qemu/qemu_hotplug.c, src/qemu/qemu_migration.c, src/qemu/qemu_process.c, src/qemu/qemu_process.h: Update for renamed APIs --- src/conf/domain_conf.c | 10 +- src/libvirt_private.syms|4 +- src/network/bridge_driver.c |8 +- src/qemu/qemu_command.c | 20 +- src/qemu/qemu_command.h |4 +- src/qemu/qemu_driver.c | 12 +- src/qemu/qemu_hotplug.c | 12 +- src/qemu/qemu_migration.c | 24 ++-- src/qemu/qemu_process.c | 12 +- src/qemu/qemu_process.h |2 +- src/util/macvtap.c | 388 ++- src/util/macvtap.h | 100 ++- tests/qemuxml2argvtest.c|2 +- tests/qemuxmlnstest.c |2 +- 14 files changed, 308 insertions(+), 292 deletions(-) (unless otherwise noted, all of my reviews in this series included a successful run of make make check make syntax-check) Looks fairly mechanical. ACK. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 05/16] Move the low level macvlan creation APIs
On 11/15/2011 06:14 AM, Daniel P. Berrange wrote: From: Daniel P. Berrangeberra...@redhat.com Move the low level macvlan creation APIs into the virnetdevmacvlan.c file where they more naturally belong * util/interface.c, util/interface.h: Remove virNetDevMacVLanCreate and virNetDevMacVLanDelete * util/virnetdevmacvlan.c, util/virnetdevmacvlan.h: Add virNetDevMacVLanCreate and virNetDevMacVLanDelete --- src/util/interface.c| 270 --- src/util/interface.h| 12 -- src/util/virnetdevmacvlan.c | 256 - src/util/virnetdevmacvlan.h | 12 ++ 4 files changed, 264 insertions(+), 286 deletions(-) ACK. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 04/16] Rename low level macvlan creation APIs
On 11/15/2011 06:14 AM, Daniel P. Berrange wrote: From: Daniel P. Berrangeberra...@redhat.com Rename ifaceMacvtapLinkAdd to virNetDevMacVLanCreate and ifaceLinkDel to virNetDevMacVLanDelete. Strictly speaking the latter isn't restricted to macvlan devices, but that's the only use libvirt has for it. Also removed the macaddrsize arg from virNetDevMacVLanCreate. This is bringing back some memory of a network device type that had a mac address 6 bytes in length, but I can't recall what it was, and anyway everything else in libvirt assumes VIR_MAC_BUFLEN (6) anyway. ACK. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 06/16] Rename interface MAC address replacement APIs
On 11/15/2011 06:14 AM, Daniel P. Berrange wrote: From: Daniel P. Berrangeberra...@redhat.com Rename ifaceReplaceMacAddress to virNetDevReplaceMacAddress and ifaceRestoreMacAddress to virNetDevRestoreMacAddress. * util/interface.c, util/interface.h, util/virnetdevmacvlan.c: Rename APIs --- src/libvirt_private.syms|4 ++-- src/util/interface.c| 14 +++--- src/util/interface.h| 15 +-- src/util/virnetdevmacvlan.c |5 ++--- 4 files changed, 20 insertions(+), 18 deletions(-) ACK. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 07/16] Move MAC address replacement functions to virnetdev.c
On 11/15/2011 06:14 AM, Daniel P. Berrange wrote: Move virNetDevReplaceMacAddress and virNetDevRestoreMacAddress to the virnetdev.c file where they naturally belong * util/interface.c, util/interface.h: Remove virNetDevReplaceMacAddress and virNetDevRestoreMacAddress * util/virnetdev.c, util/virnetdev.h: Add virNetDevReplaceMacAddress and virNetDevRestoreMacAddress --- src/util/interface.c | 84 -- src/util/interface.h | 10 - src/util/virnetdev.c | 91 ++ src/util/virnetdev.h | 12 ++ 4 files changed, 103 insertions(+), 94 deletions(-) ACK. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 04/16] Rename low level macvlan creation APIs
On Thu, Nov 17, 2011 at 06:04:32AM -0500, Laine Stump wrote: On 11/15/2011 06:14 AM, Daniel P. Berrange wrote: From: Daniel P. Berrangeberra...@redhat.com Rename ifaceMacvtapLinkAdd to virNetDevMacVLanCreate and ifaceLinkDel to virNetDevMacVLanDelete. Strictly speaking the latter isn't restricted to macvlan devices, but that's the only use libvirt has for it. Also removed the macaddrsize arg from virNetDevMacVLanCreate. This is bringing back some memory of a network device type that had a mac address 6 bytes in length, but I can't recall what it was, and anyway everything else in libvirt assumes VIR_MAC_BUFLEN (6) anyway. Yes, I should have mentioned that. Although the API had a macaddr length parameter, the caller had hardcoded '6' as the length. Given that everything in libvirt presumes VIR_MAC_BUFLEN, I didn't see the point in keeping macaddr length configurable for this one API. 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] [RFC][PATCHv2 00/11] add numatune command
于 2011年11月17日 17:44, Hu Tao 写道:
This series does mainly two things:
1. use cgroup cpuset to manage numa parameters
2. add a virsh command numatune to allow user to change numa parameters
from command line
Current numa parameters include nodeset and mode, but these cgroup cpuset
provides don't completely match with them, details:
params cpuset
--
nodeset cpuset provides cpuset.mems
mode strict cpuset provides cpuset.mem_hardwall
mode interleave cpuset provices cpuset.memory_spread_*
mode preferred no equivalent. !spread to preferred?
Besides, only one of the mode can be set currently at a time, but
for cpuset, the parameters are independent. From the perspective
of cpuset, we can set all the modes values independently,
Which of the mode will work actually? Per they are independant
with each other.
but it
seems not be consistent with the current numatune definition in xml.
Maybe we can improve the xml definition to fit cpuset better?(there
are more cpuset parameters than listed above)
As long as the XML is there, it can't be changed for backwards
compatibility, make addtiontions on the existed XML may work?
e.g.
numatune type='libnuma'
memory mode='strict' nodeset='0-10, 15,20'/
/numatune
numatune type='cgroup'
memory mode='strict' nodeset='0,1'/
memory mode='interleave' nodeset='2,3'/
memory mode='preferred' nodeset='4,5'/
...
more for the lots of cpuset.mem*?
/numatune
The type can be libnuma by default for backwards campatible.
Hu Tao (11):
don't modify CPU set string in virDomainCpuSetParse
enable cgroup cpuset by default
Add functions to set/get cgroup cpuset parameters
introduce numa backend
use cpuset to manage numa
add VIR_DOMAIN_NUMATUNE_MEM_NONE
add new API virDomain{G,S}etNumaParameters
Implement main entries of virDomain{G,S}etNumaParameters
Add virDomain{G,S}etNumaParameters support to the remote driver
Implement virDomain{G,S}etNumaParameters for the qemu driver
add new command numatune to virsh
daemon/remote.c | 64 +++
include/libvirt/libvirt.h.in | 23 +++
python/generator.py |2 +
src/conf/domain_conf.c |2 +-
src/conf/domain_conf.h |9 +
src/driver.h | 15 ++
src/libvirt.c| 113
src/libvirt_private.syms | 10 +
src/libvirt_public.syms |6 +
src/qemu/qemu.conf |5 +-
src/qemu/qemu_cgroup.c | 73
src/qemu/qemu_conf.c |3 +-
src/qemu/qemu_driver.c | 399 ++
src/qemu/qemu_process.c | 11 +-
src/remote/remote_driver.c | 50 ++
src/remote/remote_protocol.x | 25 +++-
src/remote_protocol-structs | 16 ++
src/util/cgroup.c| 112
src/util/cgroup.h| 11 ++
tools/virsh.c| 180 +++
20 files changed, 1121 insertions(+), 8 deletions(-)
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Upgrade-problems from qemu-0.14.1 + libvirt-0.8.4 to qemu-0.15.1 + libvirt-0.9.6: Why I think multifunction=on is a bad idea...
Hello Eric, Am Dienstag 15 November 2011 22:33:20 schrieb Eric Blake: Is there a way to select which ROM image qemu uses from the qemu command line? The file names are explicitly added to the qemu command-line: qemu:commandline qemu:arg value='-option-rom'/ qemu:arg value='/usr/share/kvm/pxe-virtio.bin'/ /qemu:commandline 2. libvirt defaults to add ',multifuntion=on' to the (in my case) rtl8139 network card and balloon-driver, I thought that was a bug in 0.9.5, but fixed in the 0.9.6 release just several days later. That is, we already agreed that 0.9.5 exposed the problem of blindly enabling multifunction as being an ABI breaker, and that 0.9.6 fixed things so that you have to explicitly request multifunction in the XML. But now you're saying 0.9.6 was also faulty? How about 0.9.7? Do you have a better formula for reproducing this? It seems to be fixed with 0.9.7: 0.9.6 adds multifunction=on 0.9.7 adds multifunction=off But qemu still refuses to load the image with qemu: warning: error while loading state for instance 0x0 of device 'ram' Back to using gdb to find out which specific blob fails... That won't help. The XML data should be properly translatable in an ABI-compatible manner, regardless if qemu is upgraded in the meantime; worse, you have to remember that qemu command lines change across releases, so guaranteeing ABI stability may require using _different_ command lines in 0.15 than it did in 0.14; saving the argv you handed to qemu 0.14 won't help you; but saving a complete XML description that can map to either qemu 0.14 or qemu 0.15 will. So if it is problem of an incomplete XML mapping, then we'd like to fix it, and sooner rather than later. Right you are, thank you for your feedback. Sincerely Philipp -- Philipp Hahn Open Source Software Engineer h...@univention.de Univention GmbHLinux for Your Businessfon: +49 421 22 232- 0 Mary-Somerville-Str.1 D-28359 Bremen fax: +49 421 22 232-99 http://www.univention.de/ signature.asc Description: This is a digitally signed message part. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 10/16] Rename ifaceGetIPAddress to virNetDevGetIPv4Address
On 11/15/2011 06:14 AM, Daniel P. Berrange wrote: From: Daniel P. Berrangeberra...@redhat.com To match up with the existing virNetDevSetIPv4Address, rename ifaceGetIPAddress to virNetDevGetIPv4Address * util/interface.h, util/interface.c: Rename API * network/bridge_driver.c: Update for API rename --- src/libvirt_private.syms|2 +- src/network/bridge_driver.c | 11 +++-- src/util/interface.c| 46 +++--- src/util/interface.h|3 +- 4 files changed, 32 insertions(+), 30 deletions(-) ACK. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 09/16] Move virNetDevGetIndex virNetDevGetVLanID to virnetdev.c
On 11/15/2011 06:14 AM, Daniel P. Berrange wrote: From: Daniel P. Berrangeberra...@redhat.com Move virNetDevGetIndex virNetDevGetVLanID to virnetdev.c to suit their functional purpose * util/interface.c, util/interface.h: Remove virNetDevGetIndex virNetDevGetVLanID * util/virnetdev.c, util/virnetdev.h: Add virNetDevGetIndex virNetDevGetVLanID --- src/nwfilter/nwfilter_learnipaddr.c |1 + src/util/interface.c| 111 --- src/util/interface.h|6 -- src/util/virnetdev.c| 109 ++ src/util/virnetdev.h|7 ++ 5 files changed, 117 insertions(+), 117 deletions(-) ACK. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 13/16] Move virNetDevValidateConfig to virnetdev.c
On 11/15/2011 06:14 AM, Daniel P. Berrange wrote: * src/util/interface.c, src/util/interface.h: Remove virNetDevValidateConfig * src/util/virnetdev.c, src/util/virnetdev.h: Add virNetDevValidateConfig --- src/util/interface.c | 89 -- src/util/interface.h |4 -- src/util/virnetdev.c | 77 +++ src/util/virnetdev.h |4 ++ 4 files changed, 81 insertions(+), 93 deletions(-) ACK. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 11/16] Move virNetDevGetIPv4Address to virnetdev.c
On 11/15/2011 06:14 AM, Daniel P. Berrange wrote: Move the virNetDevGetIPv4Address function to virnetdev.c * util/interface.c, util/interface.h: Remove virNetDevGetIPv4Address * util/virnetdev.c, util/virnetdev.h: Add virNetDevGetIPv4Address --- src/util/interface.c | 63 -- src/util/interface.h |3 -- src/util/virnetdev.c | 53 ++ src/util/virnetdev.h |2 + 4 files changed, 55 insertions(+), 66 deletions(-) ACK. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 16/16] Move ifaceMacvtapLinkDump and ifaceGetNthParent functions
On 11/15/2011 06:14 AM, Daniel P. Berrange wrote: Move the ifaceMacvtapLinkDump and ifaceGetNthParent functions into virnetdevvportprofile.c since they are specific to that code. This avoids polluting the headers with the Linux specific netlink data types * src/util/interface.c, src/util/interface.h: Move ifaceMacvtapLinkDump and ifaceGetNthParent functions and delete remaining file * src/util/virnetdevvportprofile.c: Add ifaceMacvtapLinkDump and ifaceGetNthParent functions * src/network/bridge_driver.c, src/nwfilter/nwfilter_gentech_driver.c, src/nwfilter/nwfilter_learnipaddr.c, src/util/virnetdevmacvlan.c: Remove include of interface.h --- po/POTFILES.in |1 - src/Makefile.am|1 - src/libvirt_private.syms |2 - src/network/bridge_driver.c|1 - src/nwfilter/nwfilter_gentech_driver.c |2 +- src/nwfilter/nwfilter_learnipaddr.c|1 - src/util/interface.c | 296 src/util/interface.h | 41 - src/util/virnetdevmacvlan.c|1 - src/util/virnetdevvportprofile.c | 199 +- 10 files changed, 195 insertions(+), 350 deletions(-) delete mode 100644 src/util/interface.c delete mode 100644 src/util/interface.h ACK. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 14/16] Rename APIs for dealing with virtual/physical functions
On 11/15/2011 06:14 AM, Daniel P. Berrange wrote: Rename ifaceIsVirtualFunction to virNetDevIsVirtualFunction, ifaceGetVirtualFunctionIndex to virNetDevGetVirtualFunctionIndex and ifaceGetPhysicalFunction to virNetDevGetPhysicalFunction * src/util/interface.c, src/util/interface.h: Rename APIs * src/util/virnetdevvportprofile.c: Update for API rename --- src/libvirt_private.syms |6 ++-- src/util/interface.c | 54 + src/util/interface.h | 12 +--- src/util/virnetdevvportprofile.c |6 ++-- 4 files changed, 39 insertions(+), 39 deletions(-) ACK. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 08/16] Rename ifaceGetIndex and ifaceGetVLAN
On 11/15/2011 06:14 AM, Daniel P. Berrange wrote:
From: Daniel P. Berrangeberra...@redhat.com
Rename the ifaceGetIndex method to virNetDevGetIndex and
ifaceGetVlanID to virNetDevGetVLanID. Also change the error
reporting behaviour to always raise errors and return -1 on
failure
* util/interface.c, util/interface.h: Rename ifaceGetIndex
and ifaceGetVLAN
* nwfilter/nwfilter_gentech_driver.c, nwfilter/nwfilter_learnipaddr.c,
nwfilter/nwfilter_learnipaddr.c, util/virnetdevvportprofile.c: Update
for API renames and error handling changes
---
src/libvirt_private.syms |4 +-
src/nwfilter/nwfilter_gentech_driver.c | 13 +++--
src/nwfilter/nwfilter_learnipaddr.c| 22 ---
src/util/interface.c | 103
src/util/interface.h |6 +-
src/util/virnetdevmacvlan.c| 17 --
src/util/virnetdevvportprofile.c |6 +-
7 files changed, 92 insertions(+), 79 deletions(-)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index d71186b..e621f79 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -576,12 +576,12 @@ virHookPresent;
# interface.h
ifaceCheck;
-ifaceGetIndex;
+virNetDevGetIndex;
ifaceGetIPAddress;
ifaceGetNthParent;
ifaceGetPhysicalFunction;
ifaceGetVirtualFunctionIndex;
-ifaceGetVlanID;
+virNetDevGetVLanID;
ifaceIsVirtualFunction;
virNetDevMacVLanCreate;
virNetDevMacVLanDelete;
diff --git a/src/nwfilter/nwfilter_gentech_driver.c
b/src/nwfilter/nwfilter_gentech_driver.c
index 899bd32..9f44aef 100644
--- a/src/nwfilter/nwfilter_gentech_driver.c
+++ b/src/nwfilter/nwfilter_gentech_driver.c
@@ -903,9 +903,10 @@ _virNWFilterInstantiateFilter(virConnectPtr conn,
/* after grabbing the filter update lock check for the interface; if
it's not there anymore its filters will be or are being removed
(while holding the lock) and we don't want to build new ones */
-if (ifaceGetIndex(false, net-ifname,ifindex) 0) {
+if (virNetDevGetIndex(net-ifname,ifindex) 0) {
/* interfaces / VMs can disappear during filter instantiation;
don't mark it as an error */
+virResetLastError();
But since the error has already been logged, it isn't really being
ignored. Based on past experience with other errors that aren't really
errors, I'm betting this will lead to bogus bug reports (unless it's
*extremely* rare, and requires doing something way out of the ordinary
such that the admin might expect to get spurious error messages). Maybe
virNetDevGetIndex could have some sort of allow/ignore/dontreport
failure flag added?
diff --git a/src/nwfilter/nwfilter_learnipaddr.c
b/src/nwfilter/nwfilter_learnipaddr.c
index 68bdcfc..9a51fc2 100644
--- a/src/nwfilter/nwfilter_learnipaddr.c
+++ b/src/nwfilter/nwfilter_learnipaddr.c
@@ -252,21 +252,23 @@ virNWFilterTerminateLearnReq(const char *ifname) {
int ifindex;
virNWFilterIPAddrLearnReqPtr req;
-if (ifaceGetIndex(false, ifname,ifindex) == 0) {
-
-IFINDEX2STR(ifindex_str, ifindex);
+if (virNetDevGetIndex(ifname,ifindex) 0) {
+virResetLastError();
+return rc;
+}
-virMutexLock(pendingLearnReqLock);
+IFINDEX2STR(ifindex_str, ifindex);
-req = virHashLookup(pendingLearnReq, ifindex_str);
-if (req) {
-rc = 0;
-req-terminate = true;
-}
+virMutexLock(pendingLearnReqLock);
-virMutexUnlock(pendingLearnReqLock);
+req = virHashLookup(pendingLearnReq, ifindex_str);
+if (req) {
+rc = 0;
+req-terminate = true;
}
+virMutexUnlock(pendingLearnReqLock);
+
return rc;
git/diff didn't go to any pains to make *that* hunk easy to follow :-/
original:
if (ifaceGetIndex(false, ifname,ifindex) == 0) {
IFINDEX2STR(ifindex_str, ifindex);
virMutexLock(pendingLearnReqLock);
req = virHashLookup(pendingLearnReq, ifindex_str);
if (req) {
rc = 0;
req-terminate = true;
}
virMutexUnlock(pendingLearnReqLock);
}
return rc;
vs new:
if (virNetDevGetIndex(ifname,ifindex) 0) {
virResetLastError();
return rc;
}
IFINDEX2STR(ifindex_str, ifindex);
virMutexLock(pendingLearnReqLock);
req = virHashLookup(pendingLearnReq, ifindex_str);
if (req) {
rc = 0;
req-terminate = true;
}
virMutexUnlock(pendingLearnReqLock);
return rc;
so that looks okay (modulo the issue with virNetDevGetIndex errors being
logged rather than ignored).
I'm uncomfortable enough with the change in error behavior that I don't
want to ACK this as-is.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 12/16] Rename ifaceCheck to virNetDevValidateConfig
On 11/15/2011 06:14 AM, Daniel P. Berrange wrote: Rename the ifaceCheck method to virNetDevValidateConfig and change so that it always raises an error and returns -1 on error. * src/util/interface.c, src/util/interface.h: Rename ifaceCheck to virNetDevValidateConfig * src/nwfilter/nwfilter_gentech_driver.c, src/nwfilter/nwfilter_learnipaddr.c: Update for API rename --- src/libvirt_private.syms |2 +- src/nwfilter/nwfilter_gentech_driver.c |6 ++- src/nwfilter/nwfilter_learnipaddr.c|6 ++- src/util/interface.c | 83 --- src/util/interface.h |5 +- 5 files changed, 55 insertions(+), 47 deletions(-) ACK. I just noticed that all of these patches rename the functions in libvirt_private.syms in place, so when it's all done, the functions aren't properly aligned with the .h file they're in, and are no longer in alphabetical order. There should be an extra patch ended at the end of the series to clean this up. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 15/16] Move functions for dealing with physical/virtual devices
On 11/15/2011 06:14 AM, Daniel P. Berrange wrote: Move virNetDevIsVirtualFunction, virNetDevGetVirtualFunctionIndex and virNetDevGetPhysicalFunction to virnetdev.c * src/util/interface.c, src/util/interface.h, src/util/virnetdev.c, src/util/virnetdev.h: Move APIs --- src/util/interface.c | 146 - src/util/interface.h | 13 - src/util/virnetdev.c | 148 ++ src/util/virnetdev.h | 10 4 files changed, 158 insertions(+), 159 deletions(-) ACK. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [Openstack] Libvirt block live migration with OpenStack Diablo
I think you should use exactly same template as below. https://github.com/openstack/nova/blob/master/nova/virt/libvirt.xml.template Kei (2011/11/17 2:41), Doude wrote: Hi all, I use OpenStack Diablo release 2011.3-0ubuntu6.2 on ubuntu 11.10 with libvirt 0.9.2-4ubuntu15.1 I try to block migrate a VM from one host to another one. OpenStack uses the 'migrateToURI' method from libvirt python library. But this call fails. Libvirt log : - Source host: 18:27:30.475: 24622: error : remoteIO:5985 : unable to set user and group to '107:118' on '/var/lib/nova/instances/instance-00b7/console.fifo.in': No such file or directory - Target host: 27:29.737: 27244: error : virSecurityDACSetOwnership:125 : unable to set user and group to '107:118' on '/var/lib/nova/instances/instance-00b7/console.fifo.in': No such file or directory 18:27:29.917: 27244: error : virSecurityDACRestoreSecurityFileLabel:143 : cannot resolve symlink /var/lib/nova/instances/instance-00b7/console.fifo.out: No such file or directory So the migration fails. VM disks are transfered to the target host but console files aren't. XML file of domain: domain type='kvm' nameinstance-00b7/name memory2097152/memory os typehvm/type boot dev=hd / /os features acpi/ /features vcpu2/vcpu devices disk type='file' driver type='qcow2'/ source file='/var/lib/nova/instances/instance-00b7/disk'/ target dev='vda' bus='virtio'/ /disk disk type='file' driver type='qcow2'/ source file='/var/lib/nova/instances/instance-00b7/disk.local'/ target dev='vdb' bus='virtio'/ /disk interface type='bridge' source bridge='br102'/ mac address='02:16:3e:36:c4:70'/ model type='virtio'/ filterref filter=nova-instance-instance-00b7-02163e36c470 parameter name=IP value=172.16.2.3 / parameter name=DHCPSERVER value=172.16.2.1 / /filterref /interface !-- The order is significant here. File must be defined first -- serial type=pipe source path='/var/lib/nova/instances/instance-00b7/console.fifo'/ target port='1'/ /serial console type='pty' tty='/dev/pts/2' source path='/dev/pts/2'/ target port='0'/ /console serial type='pty' source path='/dev/pts/2'/ target port='0'/ /serial graphics type='vnc' port='-1' autoport='yes' keymap='fr' listen='0.0.0.0'/ /devices /domain Could you help me ? Regards, Doude. ___ Mailing list: https://launchpad.net/~openstack Post to : openst...@lists.launchpad.net Unsubscribe : https://launchpad.net/~openstack More help : https://help.launchpad.net/ListHelp -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [Openstack] Libvirt block live migration with OpenStack Diablo
Hi Kei, I use the Diablo release of openStack and I didn't change anything in the libvirt XML template. I can saw a difference for the console file. The trunk version of Nova use a file for the logging console but the Diablo Release use a pipe. Can I change it manually ? Must I open a bur for Diablo release ? Regards, Doude. On Thu, Nov 17, 2011 at 2:23 PM, Kei.masumoto kei.masum...@gmail.com wrote: I think you should use exactly same template as below. https://github.com/openstack/nova/blob/master/nova/virt/libvirt.xml.template Kei (2011/11/17 2:41), Doude wrote: Hi all, I use OpenStack Diablo release 2011.3-0ubuntu6.2 on ubuntu 11.10 with libvirt 0.9.2-4ubuntu15.1 I try to block migrate a VM from one host to another one. OpenStack uses the 'migrateToURI' method from libvirt python library. But this call fails. Libvirt log : - Source host: 18:27:30.475: 24622: error : remoteIO:5985 : unable to set user and group to '107:118' on '/var/lib/nova/instances/instance-00b7/console.fifo.in': No such file or directory - Target host: 27:29.737: 27244: error : virSecurityDACSetOwnership:125 : unable to set user and group to '107:118' on '/var/lib/nova/instances/instance-00b7/console.fifo.in': No such file or directory 18:27:29.917: 27244: error : virSecurityDACRestoreSecurityFileLabel:143 : cannot resolve symlink /var/lib/nova/instances/instance-00b7/console.fifo.out: No such file or directory So the migration fails. VM disks are transfered to the target host but console files aren't. XML file of domain: domain type='kvm' nameinstance-00b7/name memory2097152/memory os typehvm/type boot dev=hd / /os features acpi/ /features vcpu2/vcpu devices disk type='file' driver type='qcow2'/ source file='/var/lib/nova/instances/instance-00b7/disk'/ target dev='vda' bus='virtio'/ /disk disk type='file' driver type='qcow2'/ source file='/var/lib/nova/instances/instance-00b7/disk.local'/ target dev='vdb' bus='virtio'/ /disk interface type='bridge' source bridge='br102'/ mac address='02:16:3e:36:c4:70'/ model type='virtio'/ filterref filter=nova-instance-instance-00b7-02163e36c470 parameter name=IP value=172.16.2.3 / parameter name=DHCPSERVER value=172.16.2.1 / /filterref /interface !-- The order is significant here. File must be defined first -- serial type=pipe source path='/var/lib/nova/instances/instance-00b7/console.fifo'/ target port='1'/ /serial console type='pty' tty='/dev/pts/2' source path='/dev/pts/2'/ target port='0'/ /console serial type='pty' source path='/dev/pts/2'/ target port='0'/ /serial graphics type='vnc' port='-1' autoport='yes' keymap='fr' listen='0.0.0.0'/ /devices /domain Could you help me ? Regards, Doude. ___ Mailing list: https://launchpad.net/~openstack Post to : openst...@lists.launchpad.net Unsubscribe : https://launchpad.net/~openstack More help : https://help.launchpad.net/ListHelp -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] make docs fails on EL5
On 11/17/2011 01:25 AM, Daniel Berteaud wrote: And it looks like the error is caused by there is some tags in the html is not campatible with W3C, though not sure why it comes. Not sure if it's relevant, I'm using xhtml1-dtds 1.0-7.1.1 (from the base CentOS 5.7 repo) Nobody else has this error when building on EL5 ? (or maybe nobody builds latest release of libvirt on EL5). Could this be a problem in my mock setup ? I've seen it as well, but haven't had time to investigate a fix for it. It may just be a matter of fixing the input files to use a different label name for the problematic line. -- Eric Blake ebl...@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] bug: try to take disk snapshot for LVM2 Volume
On 11/17/2011 02:11 AM, shu ming wrote: Checking the source of the disk will exclude the case when the source disk is block device while the snapshot target is given manually by the users with snapshot-create --xmlfile or virsh snapshot-create-as --diskspec. Why not check the snapshot directory where it will be created? Because it is assumed that if you are bothering to take the time to pass --xmlfile or --diskspec, then you are also aware of the issues of that file name and can deal with the consequences of naming a bogus location for the resulting qcow2 file. The patch below is only a crutch to make the default behavior, with no explicit name, less likely to cause confusion; while still leaving the user with the full power of the tool when they go with explicit file names. * src/conf/domain_conf.c (virDomainSnapshotAlignDisks): Only generate names if backing file exists as regular file. Reported by MATSUDA Daiki. -- Eric Blake ebl...@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] Upgrade-problems from qemu-0.14.1 + libvirt-0.8.4 to qemu-0.15.1 + libvirt-0.9.6: Why I think multifunction=on is a bad idea...
On 11/17/2011 04:54 AM, Philipp Hahn wrote: Hello Eric, Am Dienstag 15 November 2011 22:33:20 schrieb Eric Blake: Is there a way to select which ROM image qemu uses from the qemu command line? The file names are explicitly added to the qemu command-line: qemu:commandline qemu:arg value='-option-rom'/ qemu:arg value='/usr/share/kvm/pxe-virtio.bin'/ /qemu:commandline So we definitely need to expose this in the XML; we already have os/bios, so perhaps this would be the best location: domain ... os bios useserial='yes' rom/usr/share/kvm/pxe-virtio.bin/rom /bios /os ... and if /bios/rom[0] is not present, then we have to figure out how to provide a sane default. Does qemu provide a -help option that says what option-rom it will use if -option-rom was not provided on the command line? Are you interested in working on patches on this front? -- Eric Blake ebl...@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] Upgrade-problems from qemu-0.14.1 + libvirt-0.8.4 to qemu-0.15.1 + libvirt-0.9.6: Why I think multifunction=on is a bad idea...
On Thu, Nov 17, 2011 at 06:50:57AM -0700, Eric Blake wrote: On 11/17/2011 04:54 AM, Philipp Hahn wrote: Hello Eric, Am Dienstag 15 November 2011 22:33:20 schrieb Eric Blake: Is there a way to select which ROM image qemu uses from the qemu command line? The file names are explicitly added to the qemu command-line: qemu:commandline qemu:arg value='-option-rom'/ qemu:arg value='/usr/share/kvm/pxe-virtio.bin'/ /qemu:commandline So we definitely need to expose this in the XML; we already have os/bios, so perhaps this would be the best location: domain ... os bios useserial='yes' rom/usr/share/kvm/pxe-virtio.bin/rom /bios /os ... I wouldn't bother with that nesting - just put it directly inside os. Several other things in there are all associated with the bios. 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] bug: try to take disk snapshot for LVM2 Volume
On Wed, Nov 16, 2011 at 08:45:34AM -0700, Eric Blake wrote: On 11/15/2011 08:20 PM, Dave Allan wrote: After working on this some more, I think that identifying problematic file systems, like devtmpfs, is too tricky to be portable. But I think we can meet halfway - right now, the libvirt code blindly generates a filename if one was not provided, regardless of the file type of the backing file it will be wrapping. But maybe it would be sufficient to make the command error out if no qcow2 filename is provided and the backing file is not a regular file. As in this patch: Ok, now I understand the situation; thanks to everyone for the explanations. I'm somewhat uncomfortable using file type as a proxy for troublesome filesystem since although they are linked in this case, I'm not sure they're linked in all cases. Maybe I'm wrong about that. If there is no portable way to determine whether a particular filesystem is going to be a problem, I would just clearly document the limitations of letting libvirt choose the filename and the importance of not using that functionality on filesystems that cannot hold a useful snapshot. My patch would not be preventing snapshots of block devices, but merely requiring that the user supply the qcow2 filename that will wrap the block device. The problem with just documenting the issue is that someone will fail to read the documentation, perform a default snapshot that creates a problematic qcow2 file, then be upset that their domain fails to boot and that they lost all the changes made since the snapshot. I'm still in favor of this patch if anyone else thinks it is worthwhile. After an offline conversation with Eric about why file type is a good proxy for a filesystem that is unsuitable for a snapshot, I'm in agreement with this patch. The argument boils down to 1) block devices are really the only non-regular files that are useful for backing guest block devices 2) the vast majority of block devices on modern systems live in filesystems that are unsuitable for snapshots (e.g., devfs) 3) if a user has a use case that requires mknod'ing a block device in a filesystem capable of storing snapshots, they can override the libvirt check by supplying a filename which libvirt will honor. so, ACK to the design. Dave Subject: [PATCH] snapshot: refuse to generate names for non-regular backing files For whatever reason, the kernel allows you to create a regular file named /dev/sdc.12345; although this file will disappear the next time devtmpfs is remounted. If you let libvirt generate the name of the external snapshot for a disk image originally using the block device /dev/sdc, then the domain will be rendered unbootable once the qcow2 file is lost on the next devtmpfs remount. In this case, the user should have used 'virsh snapshot-create --xmlfile' or 'virsh snapshot-create-as --diskspec' to specify the name for the qcow2 file in a sane location, rather than relying on libvirt generating a name that is most likely to be wrong. We can help avoid naive mistakes by enforcing that the user provide the external name for any backing file that is not a regular file. * src/conf/domain_conf.c (virDomainSnapshotAlignDisks): Only generate names if backing file exists as regular file. Reported by MATSUDA Daiki. -- Eric Blake ebl...@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
Re: [libvirt] [PATCH V4 03/10] Make filter creation in root table more flexible
On 11/16/2011 08:02 PM, Eric Blake wrote:
On 10/26/2011 05:36 AM, Stefan Berger wrote:
Use the previously introduced chain priorities to sort the chains for access
from an interface's 'root' table and have them created in the proper order.
This gets rid of a lot of code that was previously creating the chains in a
more hardcoded way.
To determine what protocol a filter is used for evaluation do prefix-
matching, i.e., the filter 'arp' is used to filter for the 'arp' protocol,
'ipv4' for the 'ipv4' protocol and 'arp-xyz' will also be used to filter
for the 'arp' protocol following the prefix 'arp' in its name.
Signed-off-by: Stefan Bergerstef...@linux.vnet.ibm.com
---
src/nwfilter/nwfilter_ebiptables_driver.c | 130
++
1 file changed, 98 insertions(+), 32 deletions(-)
+static int
+ebiptablesFilterOrderSort(const virHashKeyValuePairPtr a,
+ const virHashKeyValuePairPtr b)
+{
+return *(virNWFilterChainPriority *)a-value -
+ *(virNWFilterChainPriority *)b-value;
+}
I tend to worry about someone passing INT_MAX and INT_MIN to a qsort
algorithm, then getting the wrong comparison sense because of integer
overflow when it uses straight subtraction; so I like to add a comment
explaining why I know that the valid input was capped and overflow is
impossible.
Values here are limited to range [-1000, 1000]. Added that as a comment now.
static void
iptablesCheckBridgeNFCallEnabled(bool isIPv6)
@@ -3327,6 +3336,56 @@ iptablesCheckBridgeNFCallEnabled(bool is
}
}
+/*
+ * Given a filtername determine the protocol it is used for evaluating
+ * We do prefix-matching to determine the protocol.
+ */
+static enum l3_proto_idx
+ebtablesGetProtoIdxByFiltername(const char *filtername)
+{
+enum l3_proto_idx idx;
+
+for (idx = 0; idx L3_PROTO_LAST_IDX; idx++) {
+if (STRPREFIX(filtername, l3_protocols[idx].val)) {
+return idx;
+}
+}
This works as long as none of the entries in l3_protocols are a prefix
of any other entry; is it worth a comment at the declaration of
l3_protocols warning about this assumption for when new protocol names
are added to the table?
Added a comment there. Though if abc comes before ab, that would
still work, though entries have to be sorted that way then.
+
+return -1;
+}
+
+static int
+ebtablesCreateTmpRootAndSubChains(virBufferPtr buf,
+ const char *ifname,
+ virHashTablePtr chains, int direction)
+{
+int rc = 0, i;
+
+if (virHashSize(chains) != 0) {
+char **filter_names;
+
+ebtablesCreateTmpRootChain(buf, direction, ifname, 1);
+filter_names = (char **)virHashGetKeys(chains,
+ ebiptablesFilterOrderSort);
This area of code will be impacted by my comments on 1/10.
Conversion done.
+if (filter_names == NULL) {
+virReportOOMError();
Your implementation of virHashGetKeys already reported OOM error for all
error paths except for numElems 0; but that means that this call to
virReportOOMError() is either redundant (a second report), or misleading
(if numElems 0, you are not OOM, but have a programming bug on hand
for passing in an invalid hash table in the first place).
So I am not reporting an error anymore. If the hash table 'chains' was
NULL, which would then also lead to '', it would not end up in this
function at all but would have been intercepted earlier.
+rc = 1;
Can we get this updated to use -1 for failures, so that there's less to
clean up later when we fix this file to match libvirt conventions?
Did that now.
There will also be a patch converting all the nwfilter code to return
'-1' upon failure. I got that in my queue, but only for later.
+goto err_exit;
+}
+for (i = 0; filter_names[i]; i++) {
+enum l3_proto_idx idx = ebtablesGetProtoIdxByFiltername(
+ filter_names[i]);
+if ((int)idx 0)
+continue;
+ebtablesCreateTmpSubChain(buf, direction, ifname, idx,
+ filter_names[i], 1);
+}
+virHashFreeKeys(chains, (void **)filter_names);
Oh, this reminds me of some additional feedback I meant to give on 1/10
- as part of the documentation you add, be sure to mention that the
returned array is only valid as long as the underlying hash table is not
modified (especially if you alter things to return in-hash pointers
rather than cloning keys) - adding or removing elements, or deleting the
hash table, will invalidate the returned key array.
I added that also as comment to the virHashGetItems() description.
@@ -3337,24 +3396,43 @@ ebiptablesApplyNewRules(virConnectPtr co
int i;
int cli_status;
ebiptablesRuleInstPtr *inst = (ebiptablesRuleInstPtr *)_inst;
-int chains_in = 0, chains_out = 0;
Re: [libvirt] New feature for libvirt
On 11/17/2011 02:04 AM, Amit Tewari wrote: Hi, I wanted to suggest a new feature development in libvirt network filter. Currently in libvirt network filter there is no support for ip aliasing, but we want to add this feature so that libvirt learns multiple ip address for a virtual machine. Great. David Stevens's patches tries to address exactly this issue by learning from DHCP requests a VM is sending. I am currently morphing the network filtering subsystem to be able to cope with multiple IP addresses per interface (lists of items). I hope we will be able to provide this functionality within 0.9.8 timeframe... Stefan With this feature we will be able to get no-ip-spoofing filter works on machine with ip aliasing. Currently if we apply no-ip-spoofing filter on a virtual machine with ip aliasing then only one ip address is earned by libvirt ,due to this other aliased ip address packets are not allowed to route out of the machine. With this new feature libvirt network filter will work on machine with multiple ip addresses on a interface(ip aliasing) Regards Amit Tewari DISCLAIMER: --- The contents of this e-mail and any attachment(s) are confidential and intended for the named recipient(s) only. It shall not attach any liability on the originator or NECHCL or its affiliates. Any views or opinions presented in this email are solely those of the author and may not necessarily reflect the opinions of NECHCL or its affiliates. Any form of reproduction, dissemination, copying, disclosure, modification, distribution and / or publication of this message without the prior written consent of the author of this e-mail is strictly prohibited. If you have received this email in error please delete it and notify the sender immediately. . --- -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH V4 04/10] Use scripting for cleaning and renaming of chains
On 10/26/2011 05:36 AM, Stefan Berger wrote:
Use scripts for the renaming and cleaning up of chains. This allows us to get
rid of some of the code that is only capable of renaming and removing chains
whose names are hardcoded.
Resuming where I left off, in the hopes that this helps before you post
v5...
A shell function 'collect_chains' is introduced that is given the name
of an ebtables chain and then recursively determines the names of all
chanins that are accessed from this chain and its sub-chains using 'jumps'.
s/chanins/chains/
The resulting list of chain names is then used to delete all the found
chains by first flushing and then deleting them.
The same function is also used for renaming temporary filters to their final
names.
I tested this with the bash and dash as script interpreters.
Well, there's still the overriding design nag that we want to avoid
shell interpretation if at all possible, but that's a _much_ bigger
rewrite for another day, so I can live with this current patch (it's not
making our use of sh any worse than it already was).
+static const char ebtables_script_func_collect_chains[] =
Reading shell code that has been quoted into a C string literal to be
passed through printf is an interesting exercise :)
+collect_chains()\n
Making sure I understand the rest of this patch, this function is
reached in two places by the rest of your patch, with code like:
+virBufferAsprintf(buf, $(collect_chains %s) , rootchain);
thus, you are using one command substitution per rootchain, where this
function then outputs words to be parsed as part of a resulting command.
That's a lot of processes; would it be worth using a shell for loop
instead of a command-substitution per rootchain
+{\n
+ local sc\n
'local' is not portable; POSIX doesn't guarantee it. I suppose we can
get away with it since both bash and dash support it, and this code is
specific to Linux where we know /bin/sh will be either bash or dash, but
it would be even nicer if we could stick to POSIX syntax (which means
avoiding 'local' and treating all variables are global, and you have to
be careful that variables used inside a function are not likely to cause
conflicts with any other variables used globally).
+ sc=$(%s -t %s -L $1 | \\\n
Just so I'm making sure I understand things, this line is paired with
ebtables_cmd_path and EBTABLES_DEFAULT_TABLE, which results in something
like this in shell ($1 being a rootchain name, fitting the pattern
libvirt-%c-%s, so I think you're okay that $1 wasn't quoted):
sc=$(/path/to/iptables -t nat -L $1 | ...
Can you please include a comment mentioning the typical output from such
a command that you intend to be parsing?
Technically, you don't need backslash-newline after pipe; a trailing
pipe is sufficient to continue a statement to the next line in shell.
But it doesn't hurt to leave it in either.
+ sed -n \/Bridge chain*/,$ s/.*\\-j \\([%s]-.*\\)/\\1/p\)\n
1 2 3 4 5
1. Did you really mean to match lines with Bridge chai or Bridge
chainnn? That * is probably superfluous.
2. $ is not portable shell. To be portable, it should be \$ or '$ '.
3. \- is not a portable sed escape. But you don't need escaping for
'-', since searching for a literal - in a sed expression works just fine.
4. This %s matches up to the 'chains' argument in this code:
+virBufferAsprintf(buf, NWFILTER_FUNC_COLLECT_CHAINS,
+ ebtables_cmd_path, EBTABLES_DEFAULT_TABLE, chains);
which I in turn traced back to:
+char chains[3] = {
+CHAINPREFIX_HOST_IN_TEMP, // 'J'
+CHAINPREFIX_HOST_OUT_TEMP, // 'P'
+0};
so it looks like you are taking a line that looks like:
Bridge chain ... -j J-...
and turning it into:
J-...
while ignoring all other lines.
5. Use of \(\)/\1 in shell is unusual (it's well-defined by POSIX that
the backslash is preserved literally if the next character is not
backquote, dollar, backslash, or newline), but for safety reason, I
prefer to write it as \\(\\)/\\1 to make it obvious that in the shell
code I meant a literal backslash rather than relying on POSIX rules.
Put all of these together, and this line should probably be:
sed -n \/Bridge chain/,\\$ s/.*-j ([%s]-.*)/1/p\)\n
+ for tmp in `echo \$sc\`; do\n
I prefer $() over ``. But this echo is a wasted process - no need to
echo when you can just use $sc directly:
for tmp in $sc; do\n
+[ -n \$tmp\ ] sc=\$sc $(collect_chains $tmp)\\n
Odd to be assigning to $sc in the middle of a loop based on the contents
of $sc, and problematic if we change $sc to be global instead of local.
Furthermore, [ -n $tmp ] will always be true (whether with your 'for
tmp in `echo $sc`' or my 'for tmp in $sc', the point remains that word
splitting is done, so each value of $tmp will be non-empty and contain
no spaces).
+ done\n
+ echo \$sc\\n
+
Re: [libvirt] bug: try to take disk snapshot for LVM2 Volume
On 11/17/2011 08:04 AM, Dave Allan wrote: After an offline conversation with Eric about why file type is a good proxy for a filesystem that is unsuitable for a snapshot, I'm in agreement with this patch. The argument boils down to 1) block devices are really the only non-regular files that are useful for backing guest block devices The other non-regular files being directories, chardevs, symlinks, fifos, and sockets. 2) the vast majority of block devices on modern systems live in filesystems that are unsuitable for snapshots (e.g., devfs) 3) if a user has a use case that requires mknod'ing a block device in a filesystem capable of storing snapshots, they can override the libvirt check by supplying a filename which libvirt will honor. so, ACK to the design. I've gone ahead and pushed this patch. * src/conf/domain_conf.c (virDomainSnapshotAlignDisks): Only generate names if backing file exists as regular file. Reported by MATSUDA Daiki. -- Eric Blake ebl...@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 V4 04/10] Use scripting for cleaning and renaming of chains
On 11/17/2011 12:33 PM, Eric Blake wrote:
On 10/26/2011 05:36 AM, Stefan Berger wrote:
The resulting list of chain names is then used to delete all the found
chains by first flushing and then deleting them.
The same function is also used for renaming temporary filters to their final
names.
I tested this with the bash and dash as script interpreters.
Well, there's still the overriding design nag that we want to avoid
shell interpretation if at all possible, but that's a _much_ bigger
rewrite for another day, so I can live with this current patch (it's not
making our use of sh any worse than it already was).
Agree.
+static const char ebtables_script_func_collect_chains[] =
Reading shell code that has been quoted into a C string literal to be
passed through printf is an interesting exercise :)
+collect_chains()\n
Making sure I understand the rest of this patch, this function is
reached in two places by the rest of your patch, with code like:
+virBufferAsprintf(buf, $(collect_chains %s) , rootchain);
thus, you are using one command substitution per rootchain, where this
function then outputs words to be parsed as part of a resulting command.
Yes, so this function starts out with the name of an ebtables chain and
tries to determine all dependent chains of it, i.e., 'subchains'.
# ebtables -t nat -L libvirt-I-tck-test205002
Bridge table: nat
Bridge chain: libvirt-I-tck-test205002, entries: 5, policy: ACCEPT
-p IPv4 -j I-tck-test205002-ipv4
-p ARP -j I-tck-test205002-arp
-p 0x8035 -j I-tck-test205002-rarp
-p 0x835 -j ACCEPT
-j DROP
Assuming I have the interface 'tck-test205002', I then run this function
to find the chains
I-tck-test205002-ipv4, I-tck-test205002-arp and I-tck-test-205002-rarp
and then visit
each one of those and try to find the chains they are 'jumping into'.
That's a lot of processes; would it be worth using a shell for loop
instead of a command-substitution per rootchain
I followed your suggested code below.
[...]
sc=$(/path/to/iptables -t nat -L $1 | ...
Can you please include a comment mentioning the typical output from such
a command that you intend to be parsing?
See above (uses ebtables rather than iptables).
Technically, you don't need backslash-newline after pipe; a trailing
pipe is sufficient to continue a statement to the next line in shell.
But it doesn't hurt to leave it in either.
+ sed -n \/Bridge chain*/,$ s/.*\\-j \\([%s]-.*\\)/\\1/p\)\n
1 2 3 4 5
1. Did you really mean to match lines with Bridge chai or Bridge
chainnn? That * is probably superfluous.
No, that was wrong.
2. $ is not portable shell. To be portable, it should be \$ or '$ '.
Didn't know...
3. \- is not a portable sed escape. But you don't need escaping for
'-', since searching for a literal - in a sed expression works just fine.
4. This %s matches up to the 'chains' argument in this code:
+virBufferAsprintf(buf, NWFILTER_FUNC_COLLECT_CHAINS,
+ ebtables_cmd_path, EBTABLES_DEFAULT_TABLE, chains);
which I in turn traced back to:
+char chains[3] = {
+CHAINPREFIX_HOST_IN_TEMP, // 'J'
+CHAINPREFIX_HOST_OUT_TEMP, // 'P'
+0};
so it looks like you are taking a line that looks like:
Bridge chain ... -j J-...
and turning it into:
J-...
while ignoring all other lines.
5. Use of \(\)/\1 in shell is unusual (it's well-defined by POSIX that
the backslash is preserved literally if the next character is not
backquote, dollar, backslash, or newline), but for safety reason, I
prefer to write it as \\(\\)/\\1 to make it obvious that in the shell
code I meant a literal backslash rather than relying on POSIX rules.
Put all of these together, and this line should probably be:
sed -n \/Bridge chain/,\\$ s/.*-j ([%s]-.*)/1/p\)\n
Thanks.
Ah, so the end result is that you echo each name found, as well as
recurse on each name found to echo any dependent names. Is order
important (all names from the first chain must be output before any
names from the recursive calls)? If not, then I can rewrite this
function to avoid local sc in the first place. Here, in shell form
(I'll leave you to convert it back into C string literal form):
collect_chains()
{
for tmp in $(iptables -t nat -L $1 | \
sed -n /Bridge chain/,\$ s/.*-j \\([JP]-.*\\)/\\1/p); do
echo $tmp
collect_chains $tmp
done
}
I took this 'as-is'. Thanks.
+
+static const char ebiptables_script_func_rm_chains[] =
+rm_chains()\n
It looks like you are calling rm_chains() with a single argument
containing a whitespace separated list of arguments.
+virBufferAddLit(buf, rm_chains \$a\\n);
Why not just call rm_chains with multiple arguments?
virBufferAddLit(buf, rm_chains $a\n);
+{\n
+ for tmp in `echo \$1\`; do [ -n \$tmp\ ] %s -t %s -F $tmp; done\n
+ for tmp in `echo \$1\`; do [ -n \$tmp\ ] %s -t %s -X $tmp; done\n
+}\n;
Is
[libvirt] [PATCH V5 01/10] Add function to get hash tables key/value pairs
Add a function to the virHashTable for getting an array of the hash table's
key-value pairs and have the keys (optionally) sorted.
Signed-off-by: Stefan Berger stef...@linux.vnet.ibm.com
---
v5:
- follwed Eric Blake's suggestions:
- added better description to new function
- return array of key-value pairs rather than only keys
---
src/libvirt_private.syms |2 ++
src/util/hash.c | 45 +
src/util/hash.h | 21 +
3 files changed, 68 insertions(+)
Index: libvirt-acl/src/util/hash.c
===
--- libvirt-acl.orig/src/util/hash.c
+++ libvirt-acl/src/util/hash.c
@@ -618,3 +618,48 @@ void *virHashSearch(virHashTablePtr tabl
return NULL;
}
+
+struct getKeysIter
+{
+virHashKeyValuePair *sortArray;
+unsigned arrayIdx;
+};
+
+static void virHashGetKeysIterator(void *payload,
+ const void *key, void *data)
+{
+struct getKeysIter *iter = data;
+
+iter-sortArray[iter-arrayIdx].key = key;
+iter-sortArray[iter-arrayIdx].value = payload;
+
+iter-arrayIdx++;
+}
+
+typedef int (*qsort_comp)(const void *, const void *);
+
+virHashKeyValuePairPtr virHashGetItems(virHashTablePtr table,
+ virHashKeyComparator compar)
+{
+int numElems = virHashSize(table);
+struct getKeysIter iter = {
+.arrayIdx = 0,
+.sortArray = NULL,
+};
+
+if (numElems 0)
+return NULL;
+
+if (VIR_ALLOC_N(iter.sortArray, numElems + 1)) {
+virReportOOMError();
+return NULL;
+}
+
+virHashForEach(table, virHashGetKeysIterator, iter);
+
+if (compar)
+qsort(iter.sortArray[0], numElems, sizeof(iter.sortArray[0]),
+ (qsort_comp)compar);
+
+return iter.sortArray;
+}
Index: libvirt-acl/src/util/hash.h
===
--- libvirt-acl.orig/src/util/hash.h
+++ libvirt-acl/src/util/hash.h
@@ -130,6 +130,27 @@ void *virHashLookup(virHashTablePtr tabl
*/
void *virHashSteal(virHashTablePtr table, const void *name);
+/*
+ * Get the hash table's key/value pairs and have them optionally sorted.
+ * The returned array contains virHashSize() elements. Aditionally,
+ * an empty element has been added to the end of the array whose key == NULL
+ * also indicates the end of the array.
+ * The key/value pairs are only valid as long as the underlying hash
+ * table is not modified, i.e., keys removed or the hash table deleted.
+ * The caller must only free the returned array using VIR_FREE().
+ * The caller must make copies of all returned keys and values if they are
+ * to be used somewhere else.
+ */
+typedef struct _virHashKeyValuePair virHashKeyValuePair;
+typedef virHashKeyValuePair *virHashKeyValuePairPtr;
+struct _virHashKeyValuePair {
+const void *key;
+const void *value;
+};
+typedef int (*virHashKeyComparator)(const virHashKeyValuePairPtr ,
+const virHashKeyValuePairPtr );
+virHashKeyValuePairPtr virHashGetItems(virHashTablePtr table,
+ virHashKeyComparator compar);
/*
* Iterators
Index: libvirt-acl/src/libvirt_private.syms
===
--- libvirt-acl.orig/src/libvirt_private.syms
+++ libvirt-acl/src/libvirt_private.syms
@@ -559,6 +559,7 @@ virHashAddEntry;
virHashCreate;
virHashForEach;
virHashFree;
+virHashGetItems;
virHashLookup;
virHashRemoveEntry;
virHashRemoveSet;
@@ -566,6 +567,7 @@ virHashSearch;
virHashSize;
virHashSteal;
virHashTableSize;
+virHashUpdateEntry;
# hooks.h
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH V5 02/10] Introduce an internal priority for chains
For better handling of the sorting of chains introduce an internally used
priority. Use a lookup table to store the priorities. For now their actual
values do not matter just that the values cause the chains to be properly
sorted through changes in the following patches. However, the values are
chosen as negative so that once they are sorted along with filtering rules
(whose priority may only be positive for now) they will always be instantiated
before them (lower values cause instantiation before higher values). This
is done to maintain backwards compatibility.
Signed-off-by: Stefan Berger stef...@linux.vnet.ibm.com
---
v3:
- increased filter priorities to have more room before them
---
src/conf/nwfilter_conf.c | 14 ++
src/conf/nwfilter_conf.h | 12
src/nwfilter/nwfilter_ebiptables_driver.c |4
src/nwfilter/nwfilter_ebiptables_driver.h |1 +
4 files changed, 31 insertions(+)
Index: libvirt-acl/src/conf/nwfilter_conf.h
===
--- libvirt-acl.orig/src/conf/nwfilter_conf.h
+++ libvirt-acl/src/conf/nwfilter_conf.h
@@ -357,8 +357,18 @@ enum virNWFilterEbtablesTableType {
};
+# define MIN_RULE_PRIORITY 0
# define MAX_RULE_PRIORITY 1000
+# define NWFILTER_MIN_FILTER_PRIORITY -1000
+# define NWFILTER_MAX_FILTER_PRIORITY MAX_RULE_PRIORITY
+
+# define NWFILTER_ROOT_FILTER_PRI 0
+# define NWFILTER_IPV4_FILTER_PRI -700
+# define NWFILTER_IPV6_FILTER_PRI -600
+# define NWFILTER_ARP_FILTER_PRI -500
+# define NWFILTER_RARP_FILTER_PRI -400
+
enum virNWFilterRuleFlags {
RULE_FLAG_NO_STATEMATCH = (1 0),
RULE_FLAG_STATE_NEW = (1 1),
@@ -436,6 +446,7 @@ enum virNWFilterChainSuffixType {
VIR_NWFILTER_CHAINSUFFIX_LAST,
};
+typedef int32_t virNWFilterChainPriority;
typedef struct _virNWFilterDef virNWFilterDef;
typedef virNWFilterDef *virNWFilterDefPtr;
@@ -445,6 +456,7 @@ struct _virNWFilterDef {
unsigned char uuid[VIR_UUID_BUFLEN];
int chainsuffix; /*enum virNWFilterChainSuffixType */
+virNWFilterChainPriority chainPriority;
int nentries;
virNWFilterEntryPtr *filterEntries;
Index: libvirt-acl/src/conf/nwfilter_conf.c
===
--- libvirt-acl.orig/src/conf/nwfilter_conf.c
+++ libvirt-acl/src/conf/nwfilter_conf.c
@@ -124,6 +124,14 @@ struct int_map {
#define INTMAP_ENTRY(ATT, VAL) { .attr = ATT, .val = VAL }
#define INTMAP_ENTRY_LAST { .val = NULL }
+static const struct int_map chain_priorities[] = {
+INTMAP_ENTRY(NWFILTER_ROOT_FILTER_PRI, root),
+INTMAP_ENTRY(NWFILTER_IPV4_FILTER_PRI, ipv4),
+INTMAP_ENTRY(NWFILTER_IPV6_FILTER_PRI, ipv6),
+INTMAP_ENTRY(NWFILTER_ARP_FILTER_PRI , arp ),
+INTMAP_ENTRY(NWFILTER_RARP_FILTER_PRI, rarp),
+INTMAP_ENTRY_LAST,
+};
/*
* only one filter update allowed
@@ -2028,6 +2036,12 @@ virNWFilterDefParseXML(xmlXPathContextPt
_(unknown chain suffix '%s'), chain);
goto cleanup;
}
+/* assign an implicit priority -- support XML attribute later */
+if (intMapGetByString(chain_priorities, chain, 0,
+ ret-chainPriority) == false) {
+ret-chainPriority = (NWFILTER_MAX_FILTER_PRIORITY +
+ NWFILTER_MIN_FILTER_PRIORITY) / 2;
+}
}
uuid = virXPathString(string(./uuid), ctxt);
Index: libvirt-acl/src/nwfilter/nwfilter_ebiptables_driver.h
===
--- libvirt-acl.orig/src/nwfilter/nwfilter_ebiptables_driver.h
+++ libvirt-acl/src/nwfilter/nwfilter_ebiptables_driver.h
@@ -36,6 +36,7 @@ typedef ebiptablesRuleInst *ebiptablesRu
struct _ebiptablesRuleInst {
char *commandTemplate;
enum virNWFilterChainSuffixType neededProtocolChain;
+virNWFilterChainPriority chainPriority;
char chainprefix;/* I for incoming, O for outgoing */
unsigned int priority;
enum RuleType ruleType;
Index: libvirt-acl/src/nwfilter/nwfilter_ebiptables_driver.c
===
--- libvirt-acl.orig/src/nwfilter/nwfilter_ebiptables_driver.c
+++ libvirt-acl/src/nwfilter/nwfilter_ebiptables_driver.c
@@ -328,6 +328,7 @@ static int
ebiptablesAddRuleInst(virNWFilterRuleInstPtr res,
char *commandTemplate,
enum virNWFilterChainSuffixType neededChain,
+ virNWFilterChainPriority chainPriority,
char chainprefix,
unsigned int priority,
enum RuleType ruleType)
@@ -341,6 +342,7 @@ ebiptablesAddRuleInst(virNWFilterRuleIns
inst-commandTemplate = commandTemplate;
inst-neededProtocolChain = neededChain;
+inst-chainPriority = chainPriority;
inst-chainprefix = chainprefix;
inst-priority =
[libvirt] [PATCH V5 09/10] Interleave jumping into chains with filtering rules in root table
The previous patch extends the priority of filtering rules into negative
numbers. We now use this possibility to interleave the jumping into
chains with filtering rules to for example create the 'root' table of
an interface with the following sequence of rules:
Bridge chain: libvirt-I-vnet0, entries: 6, policy: ACCEPT
-p IPv4 -j I-vnet0-ipv4
-p ARP -j I-vnet0-arp
-p ARP -j ACCEPT
-p 0x8035 -j I-vnet0-rarp
-p 0x835 -j ACCEPT
-j DROP
The '-p ARP -j ACCEPT' rule now appears between the jumps.
Since the 'arp' chain has been assigned priority -700 and the 'rarp'
chain -600, the above ordering can now be achieved with the following
rule:
rule action='accept' direction='out' priority='-650'
mac protocolid='arp'/
/rule
This patch now sorts the commands generating the above shown jumps into
chains and interleaves their execution with those for generating rules.
v3:
- fix memory leak
Signed-off-by: Stefan Berger stef...@linux.vnet.ibm.com
---
src/nwfilter/nwfilter_ebiptables_driver.c | 115 ++
1 file changed, 103 insertions(+), 12 deletions(-)
Index: libvirt-acl/src/nwfilter/nwfilter_ebiptables_driver.c
===
--- libvirt-acl.orig/src/nwfilter/nwfilter_ebiptables_driver.c
+++ libvirt-acl/src/nwfilter/nwfilter_ebiptables_driver.c
@@ -2718,13 +2718,17 @@ ebtablesUnlinkTmpRootChain(virBufferPtr
static int
-ebtablesCreateTmpSubChain(virBufferPtr buf,
+ebtablesCreateTmpSubChain(ebiptablesRuleInstPtr *inst,
+ int *nRuleInstances,
int incoming,
const char *ifname,
enum l3_proto_idx protoidx,
const char *filtername,
- int stopOnError)
+ int stopOnError,
+ virNWFilterChainPriority priority)
{
+virBuffer buf = VIR_BUFFER_INITIALIZER;
+ebiptablesRuleInstPtr tmp = *inst;
char rootchain[MAX_CHAINNAME_LENGTH], chain[MAX_CHAINNAME_LENGTH];
char chainPrefix = (incoming) ? CHAINPREFIX_HOST_IN_TEMP
: CHAINPREFIX_HOST_OUT_TEMP;
@@ -2733,15 +2737,22 @@ ebtablesCreateTmpSubChain(virBufferPtr b
PRINT_CHAIN(chain, chainPrefix, ifname,
(filtername) ? filtername : l3_protocols[protoidx].val);
-virBufferAsprintf(buf,
+virBufferAsprintf(buf,
+ CMD_DEF(%s -t %s -F %s) CMD_SEPARATOR
+ CMD_EXEC
+ CMD_DEF(%s -t %s -X %s) CMD_SEPARATOR
+ CMD_EXEC
CMD_DEF(%s -t %s -N %s) CMD_SEPARATOR
CMD_EXEC
%s
- CMD_DEF(%s -t %s -A %s -p 0x%x -j %s) CMD_SEPARATOR
+ CMD_DEF(%s -t %s -%%c %s %%s -p 0x%x -j %s)
+ CMD_SEPARATOR
CMD_EXEC
%s,
ebtables_cmd_path, EBTABLES_DEFAULT_TABLE, chain,
+ ebtables_cmd_path, EBTABLES_DEFAULT_TABLE, chain,
+ ebtables_cmd_path, EBTABLES_DEFAULT_TABLE, chain,
CMD_STOPONERR(stopOnError),
@@ -2750,6 +2761,24 @@ ebtablesCreateTmpSubChain(virBufferPtr b
CMD_STOPONERR(stopOnError));
+if (virBufferError(buf) ||
+VIR_REALLOC_N(tmp, (*nRuleInstances)+1) 0) {
+virReportOOMError();
+virBufferFreeAndReset(buf);
+return -1;
+}
+
+*inst = tmp;
+
+memset(tmp[*nRuleInstances], 0, sizeof(tmp[0]));
+tmp[*nRuleInstances].priority = priority;
+tmp[*nRuleInstances].commandTemplate =
+virBufferContentAndReset(buf);
+tmp[*nRuleInstances].neededProtocolChain =
+virNWFilterChainSuffixTypeToString(VIR_NWFILTER_CHAINSUFFIX_ROOT);
+
+(*nRuleInstances)++;
+
return 0;
}
@@ -3224,9 +3253,34 @@ static int ebtablesCleanAll(const char *
static int
ebiptablesRuleOrderSort(const void *a, const void *b)
{
+const ebiptablesRuleInstPtr insta = (const ebiptablesRuleInstPtr)a;
+const ebiptablesRuleInstPtr instb = (const ebiptablesRuleInstPtr)b;
+const char *root = virNWFilterChainSuffixTypeToString(
+ VIR_NWFILTER_CHAINSUFFIX_ROOT);
+bool root_a = STREQ(insta-neededProtocolChain, root);
+bool root_b = STREQ(instb-neededProtocolChain, root);
+
+/* ensure root chain commands appear before all others since
+ we will need them to create the child chains */
+if (root_a) {
+if (root_b) {
+goto normal;
+}
+return -1; /* a before b */
+}
+if (root_b) {
+return 1; /* b before a */
+}
+normal:
+return (insta-priority - instb-priority);
+}
+
+static int
+ebiptablesRuleOrderSortPtr(const void *a, const void *b)
+{
const ebiptablesRuleInstPtr *insta = a;
const
[libvirt] [PATCH V5 06/10] Extend the filter XML to support priorities of chains
This patch extends the filter XML to support priorities of chains
in the XML. An example would be:
filter name='allow-arpxyz' chain='arp-xyz' priority='200'
[...]
/filter
The permitted values for priorities are [-1000, 1000].
By setting the pririty of a chain the order in which it is accessed
from the interface root chain can be influenced.
Signed-off-by: Stefan Berger stef...@linux.vnet.ibm.com
---
docs/schemas/nwfilter.rng |7 ++-
src/conf/nwfilter_conf.c | 42 +-
2 files changed, 43 insertions(+), 6 deletions(-)
Index: libvirt-acl/src/conf/nwfilter_conf.c
===
--- libvirt-acl.orig/src/conf/nwfilter_conf.c
+++ libvirt-acl/src/conf/nwfilter_conf.c
@@ -2014,7 +2014,9 @@ virNWFilterDefParseXML(xmlXPathContextPt
xmlNodePtr curr = ctxt-node;
char *uuid = NULL;
char *chain = NULL;
+char *chain_pri_s = NULL;
virNWFilterEntryPtr entry;
+int chain_priority;
if (VIR_ALLOC(ret) 0) {
virReportOOMError();
@@ -2028,6 +2030,26 @@ virNWFilterDefParseXML(xmlXPathContextPt
goto cleanup;
}
+chain_pri_s = virXPathString(string(./@priority), ctxt);
+if (chain_pri_s) {
+if (sscanf(chain_pri_s, %d, chain_priority) != 1) {
+virNWFilterReportError(VIR_ERR_INVALID_ARG,
+ _(Could not parse chain priority '%s'),
+ chain_pri_s);
+goto cleanup;
+}
+if (chain_priority NWFILTER_MIN_FILTER_PRIORITY ||
+chain_priority NWFILTER_MAX_FILTER_PRIORITY) {
+virNWFilterReportError(VIR_ERR_INVALID_ARG,
+ _(Priority '%d' is outside valid
+ range of [%d,%d]),
+ chain_priority,
+ NWFILTER_MIN_FILTER_PRIORITY,
+ NWFILTER_MAX_FILTER_PRIORITY);
+goto cleanup;
+}
+}
+
chain = virXPathString(string(./@chain), ctxt);
if (chain) {
if (virNWFilterChainSuffixTypeFromString(chain) 0) {
@@ -2036,11 +2058,16 @@ virNWFilterDefParseXML(xmlXPathContextPt
goto cleanup;
}
ret-chainsuffix = chain;
-/* assign an implicit priority -- support XML attribute later */
-if (intMapGetByString(chain_priorities, chain, 0,
- ret-chainPriority) == false) {
-ret-chainPriority = (NWFILTER_MAX_FILTER_PRIORITY +
- NWFILTER_MIN_FILTER_PRIORITY) / 2;
+
+if (chain_pri_s) {
+ret-chainPriority = chain_priority;
+} else {
+/* assign an implicit priority -- support XML attribute later */
+if (intMapGetByString(chain_priorities, chain, 0,
+ ret-chainPriority) == false) {
+ret-chainPriority = (NWFILTER_MAX_FILTER_PRIORITY +
+ NWFILTER_MIN_FILTER_PRIORITY) / 2;
+}
}
chain = NULL;
} else {
@@ -2097,6 +2124,7 @@ virNWFilterDefParseXML(xmlXPathContextPt
}
VIR_FREE(chain);
+VIR_FREE(chain_pri_s);
return ret;
@@ -2104,6 +2132,7 @@ virNWFilterDefParseXML(xmlXPathContextPt
virNWFilterDefFree(ret);
VIR_FREE(chain);
VIR_FREE(uuid);
+VIR_FREE(chain_pri_s);
return NULL;
}
@@ -2852,6 +2881,9 @@ virNWFilterDefFormat(virNWFilterDefPtr d
virBufferAsprintf(buf, filter name='%s' chain='%s',
def-name,
def-chainsuffix);
+if (def-chainPriority != 0)
+virBufferAsprintf(buf, priority='%d',
+ def-chainPriority);
virBufferAddLit(buf, \n);
virUUIDFormat(def-uuid, uuid);
Index: libvirt-acl/docs/schemas/nwfilter.rng
===
--- libvirt-acl.orig/docs/schemas/nwfilter.rng
+++ libvirt-acl/docs/schemas/nwfilter.rng
@@ -293,6 +293,11 @@
/choice
/attribute
/optional
+optional
+ attribute name=priority
+ref name='priority-type'/
+ /attribute
+/optional
/define
define name=filterref-node-attributes
@@ -881,7 +886,7 @@
define name='priority-type'
data type=int
-param name=minInclusive0/param
+param name=minInclusive-1000/param
param name=maxInclusive1000/param
/data
/define
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH V5 04/10] Use scripting for cleaning and renaming of chains
Use scripts for the renaming and cleaning up of chains. This allows us to get
rid of some of the code that is only capable of renaming and removing chains
whose names are hardcoded.
A shell function 'collect_chains' is introduced that is given the name
of an ebtables chain and then recursively determines the names of all
chains that are accessed from this chain and its sub-chains using 'jumps'.
The resulting list of chain names is then used to delete all the found
chains by first flushing and then deleting them.
The same function is also used for renaming temporary filters to their final
names.
I tested this with the bash and dash as script interpreters.
Signed-off-by: Stefan Berger stef...@linux.vnet.ibm.com
---
v5:
- followng Eric Blake's suggestion on a major overhaul of the embedded
scripts
v2:
- setting IFS to intended value; works with bash and dash (phew!)
---
src/nwfilter/nwfilter_ebiptables_driver.c | 217 +-
1 file changed, 129 insertions(+), 88 deletions(-)
Index: libvirt-acl/src/nwfilter/nwfilter_ebiptables_driver.c
===
--- libvirt-acl.orig/src/nwfilter/nwfilter_ebiptables_driver.c
+++ libvirt-acl/src/nwfilter/nwfilter_ebiptables_driver.c
@@ -92,6 +92,59 @@ static char *gawk_cmd_path;
#define PRINT_CHAIN(buf, prefix, ifname, suffix) \
snprintf(buf, sizeof(buf), %c-%s-%s, prefix, ifname, suffix)
+/* The collect_chains() script recursively determines all names
+ * of ebtables (nat) chains that are 'children' of a given 'root' chain.
+ * The typical out of an ebtables call is as follows:
+ *
+ * # ebtables -t nat -L libvirt-I-tck-test205002
+ * Bridge table: nat
+ *
+ * Bridge chain: libvirt-I-tck-test205002, entries: 5, policy: ACCEPT
+ * -p IPv4 -j I-tck-test205002-ipv4
+ * -p ARP -j I-tck-test205002-arp
+ * -p 0x8035 -j I-tck-test205002-rarp
+ * -p 0x835 -j ACCEPT
+ * -j DROP
+ */
+static const char ebtables_script_func_collect_chains[] =
+collect_chains()\n
+{\n
+ for tmp2 in $*; do\n
+for tmp in $(%s -t %s -L $tmp2 | \\\n
+ sed -n \/Bridge chain/,\\$ s/.*-j ([%s]-.*)/1/p\);\n
+do\n
+ echo $tmp\n
+ collect_chains $tmp\n
+done\n
+ done\n
+}\n;
+
+static const char ebiptables_script_func_rm_chains[] =
+rm_chains()\n
+{\n
+ for tmp in $*; do %s -t %s -F $tmp; done\n
+ for tmp in $*; do %s -t %s -X $tmp; done\n
+}\n;
+
+static const char ebiptables_script_func_rename_chains[] =
+rename_chains()\n
+{\n
+ for tmp in $*; do\n
+case $tmp in\n
+ %c*) %s -t %s -E $tmp %c${tmp#?} ;;\n
+ %c*) %s -t %s -E $tmp %c${tmp#?} ;;\n
+esac\n
+ done\n
+}\n;
+
+static const char ebiptables_script_set_ifs[] =
+tmp='\n'\n
+IFS=' ''\t'$tmp\n;
+
+#define NWFILTER_FUNC_COLLECT_CHAINS ebtables_script_func_collect_chains
+#define NWFILTER_FUNC_RM_CHAINS ebiptables_script_func_rm_chains
+#define NWFILTER_FUNC_RENAME_CHAINS ebiptables_script_func_rename_chains
+#define NWFILTER_FUNC_SET_IFS ebiptables_script_set_ifs
#define VIRT_IN_CHAIN libvirt-in
#define VIRT_OUT_CHAIN libvirt-out
@@ -2698,95 +2751,66 @@ ebtablesCreateTmpSubChain(virBufferPtr b
return 0;
}
-
static int
-_ebtablesRemoveSubChain(virBufferPtr buf,
-int incoming,
-const char *ifname,
-enum l3_proto_idx protoidx,
-int isTempChain)
-{
-char rootchain[MAX_CHAINNAME_LENGTH], chain[MAX_CHAINNAME_LENGTH];
-char chainPrefix;
-
-if (isTempChain) {
-chainPrefix =(incoming) ? CHAINPREFIX_HOST_IN_TEMP
-: CHAINPREFIX_HOST_OUT_TEMP;
-} else {
-chainPrefix =(incoming) ? CHAINPREFIX_HOST_IN
-: CHAINPREFIX_HOST_OUT;
-}
-
-PRINT_ROOT_CHAIN(rootchain, chainPrefix, ifname);
-PRINT_CHAIN(chain, chainPrefix, ifname, l3_protocols[protoidx].val);
-
-virBufferAsprintf(buf,
- %s -t %s -D %s -p 0x%x -j %s CMD_SEPARATOR
- %s -t %s -F %s CMD_SEPARATOR
- %s -t %s -X %s CMD_SEPARATOR,
+_ebtablesRemoveSubChains(virBufferPtr buf,
+ const char *ifname,
+ const char *chains)
+{
+char rootchain[MAX_CHAINNAME_LENGTH];
+unsigned i;
+
+virBufferAsprintf(buf, NWFILTER_FUNC_COLLECT_CHAINS,
+ ebtables_cmd_path, EBTABLES_DEFAULT_TABLE, chains);
+virBufferAsprintf(buf, NWFILTER_FUNC_RM_CHAINS,
ebtables_cmd_path, EBTABLES_DEFAULT_TABLE,
- rootchain, l3_protocols[protoidx].attr, chain,
+ ebtables_cmd_path, EBTABLES_DEFAULT_TABLE);
- ebtables_cmd_path, EBTABLES_DEFAULT_TABLE, chain,
+virBufferAsprintf(buf, NWFILTER_FUNC_SET_IFS);
+virBufferAddLit(buf,
[libvirt] [PATCH V5 10/10] Add test cases
Add test case for the chain names with known prefixes and the chain priority. Signed-off-by: Stefan Berger stef...@linux.vnet.ibm.com --- tests/nwfilterxml2xmlin/chain_prefixtest1.xml | 37 + tests/nwfilterxml2xmlout/chain_prefixtest1.xml | 21 ++ tests/nwfilterxml2xmltest.c|2 + 3 files changed, 60 insertions(+) Index: libvirt-acl/tests/nwfilterxml2xmlin/chain_prefixtest1.xml === --- /dev/null +++ libvirt-acl/tests/nwfilterxml2xmlin/chain_prefixtest1.xml @@ -0,0 +1,37 @@ +filter name='testcase' chain='arp-testme' priority='-650' + uuide5700920-a333-4c05-8016-b669e46b7599/uuid + rule action='accept' direction='out' + arp srcmacaddr='1:2:3:4:5:6' srcmacmask='ff:ff:ff:ff:ff:ff' + protocolid='arp' + dstmacaddr='aa:bb:cc:dd:ee:ff' dstmacmask='ff:ff:ff:ff:ff:ff' + hwtype='12' + protocoltype='34' + opcode='Request' + arpsrcmacaddr='1:2:3:4:5:6' + arpdstmacaddr='a:b:c:d:e:f'/ + /rule + + rule action='accept' direction='out' + arp srcmacaddr='1:2:3:4:5:6' srcmacmask='ff:ff:ff:ff:ff:ff' + opcode='1' hwtype='255' protocoltype='255'/ + /rule + + rule action='accept' direction='out' + arp srcmacaddr='1:2:3:4:5:6' srcmacmask='ff:ff:ff:ff:ff:ff' + opcode='11' hwtype='256' protocoltype='256'/ + /rule + + rule action='accept' direction='out' + arp srcmacaddr='1:2:3:4:5:6' srcmacmask='ff:ff:ff:ff:ff:ff' + opcode='65535' hwtype='65535' protocoltype='65535' / + /rule + + rule action='accept' direction='out' + arp srcmacaddr='1:2:3:4:5:6' srcmacmask='ff:ff:ff:ff:ff:ff' + opcode='65536' hwtype='65536' protocoltype='65536' / + /rule + + rule action='accept' direction='in' + arp gratuitous='true'/ + /rule +/filter Index: libvirt-acl/tests/nwfilterxml2xmlout/chain_prefixtest1.xml === --- /dev/null +++ libvirt-acl/tests/nwfilterxml2xmlout/chain_prefixtest1.xml @@ -0,0 +1,21 @@ +filter name='testcase' chain='arp-testme' priority='-650' + uuide5700920-a333-4c05-8016-b669e46b7599/uuid + rule action='accept' direction='out' priority='500' +arp srcmacaddr='01:02:03:04:05:06' srcmacmask='ff:ff:ff:ff:ff:ff' dstmacaddr='aa:bb:cc:dd:ee:ff' dstmacmask='ff:ff:ff:ff:ff:ff' hwtype='12' protocoltype='34' opcode='Request' arpsrcmacaddr='01:02:03:04:05:06' arpdstmacaddr='0a:0b:0c:0d:0e:0f'/ + /rule + rule action='accept' direction='out' priority='500' +arp srcmacaddr='01:02:03:04:05:06' srcmacmask='ff:ff:ff:ff:ff:ff' hwtype='255' protocoltype='255' opcode='Request'/ + /rule + rule action='accept' direction='out' priority='500' +arp srcmacaddr='01:02:03:04:05:06' srcmacmask='ff:ff:ff:ff:ff:ff' hwtype='256' protocoltype='256' opcode='11'/ + /rule + rule action='accept' direction='out' priority='500' +arp srcmacaddr='01:02:03:04:05:06' srcmacmask='ff:ff:ff:ff:ff:ff' hwtype='65535' protocoltype='65535' opcode='65535'/ + /rule + rule action='accept' direction='out' priority='500' +arp srcmacaddr='01:02:03:04:05:06' srcmacmask='ff:ff:ff:ff:ff:ff'/ + /rule + rule action='accept' direction='in' priority='500' +arp gratuitous='true'/ + /rule +/filter Index: libvirt-acl/tests/nwfilterxml2xmltest.c === --- libvirt-acl.orig/tests/nwfilterxml2xmltest.c +++ libvirt-acl/tests/nwfilterxml2xmltest.c @@ -148,6 +148,8 @@ mymain(void) DO_TEST(example-1, false); DO_TEST(example-2, false); +DO_TEST(chain_prefixtest1, true); /* derived from arp-test */ + return (ret==0 ? EXIT_SUCCESS : EXIT_FAILURE); } -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH V5 07/10] Enable chains with names having a known prefix
This patch enables chains that have a known prefix in their name.
Known prefixes are: 'ipv4', 'ipv6', 'arp', 'rarp'. All prefixes
are also protocols that can be evaluated on the ebtables level.
Following the prefix they will be automatically connected to an interface's
'root' chain and jumped into following the protocol they evalute, i.e.,
a table 'arp-xyz' will be accessed from the root table using
ebtables -t nat -A iface root table -p arp -j I-ifname-arp-xyz
thus generating a 'root' chain like this one here:
Bridge chain: libvirt-O-vnet0, entries: 5, policy: ACCEPT
-p IPv4 -j O-vnet0-ipv4
-p ARP -j O-vnet0-arp
-p 0x8035 -j O-vnet0-rarp
-p ARP -j O-vnet0-arp-xyz
-j DROP
where the chain 'arp-xyz' is accessed for filtering of ARP packets.
v3:
- assign a priority to filters that have an allowed prefix, e.g., assign
the arp chain priority to a filter arp-xyz unless user provided a
priority in the XML
Signed-off-by: Stefan Berger stef...@linux.vnet.ibm.com
---
docs/schemas/nwfilter.rng | 16 ++--
src/conf/nwfilter_conf.c | 87 ++
src/conf/nwfilter_conf.h |3 +
3 files changed, 96 insertions(+), 10 deletions(-)
Index: libvirt-acl/src/conf/nwfilter_conf.c
===
--- libvirt-acl.orig/src/conf/nwfilter_conf.c
+++ libvirt-acl/src/conf/nwfilter_conf.c
@@ -2007,6 +2007,80 @@ err_exit:
goto cleanup;
}
+static bool
+virNWFilterIsValidChainName(const char *chainname)
+{
+return chainname[strspn(chainname, VALID_CHAINNAME)] == 0;
+}
+
+/*
+ * Test whether the name of the chain is supported.
+ * It current has to have a prefix of either one of the strings found in
+ * virNWFilterChainSuffixTypeToString().
+ */
+static const char *
+virNWFilterIsAllowedChain(const char *chainname)
+{
+enum virNWFilterChainSuffixType i;
+const char *name, *msg;
+virBuffer buf = VIR_BUFFER_INITIALIZER;
+bool printed = false;
+
+if (!virNWFilterIsValidChainName(chainname)) {
+virNWFilterReportError(VIR_ERR_INVALID_ARG,
+ _(Chain name contains illegal characters));
+return NULL;
+}
+
+if (strlen(chainname) MAX_CHAIN_SUFFIX_SIZE) {
+virNWFilterReportError(VIR_ERR_INVALID_ARG,
+ _(Name of chain is longer than %u characters),
+ MAX_CHAIN_SUFFIX_SIZE);
+return NULL;
+}
+
+for (i = 0; i VIR_NWFILTER_CHAINSUFFIX_LAST; i++) {
+name = virNWFilterChainSuffixTypeToString(i);
+if (i == VIR_NWFILTER_CHAINSUFFIX_ROOT) {
+/* allow 'root' as a complete name but not as a prefix */
+if (STREQ(chainname, name))
+return name;
+if (STRPREFIX(chainname, name))
+return NULL;
+}
+if (STRPREFIX(chainname, name))
+return name;
+}
+
+virBufferAsprintf(buf,
+ _(Illegal chain name '%s'. Please use a chain name
+ called '%s' or either one of the following prefixes: ),
+ virNWFilterChainSuffixTypeToString(
+ VIR_NWFILTER_CHAINSUFFIX_ROOT),
+ chainname);
+for (i = 0; i VIR_NWFILTER_CHAINSUFFIX_LAST; i++) {
+if (i == VIR_NWFILTER_CHAINSUFFIX_ROOT)
+continue;
+if (printed)
+virBufferAddLit(buf, , );
+virBufferAdd(buf, virNWFilterChainSuffixTypeToString(i), -1);
+printed = true;
+}
+
+if (virBufferError(buf)) {
+virReportOOMError();
+virBufferFreeAndReset(buf);
+goto err_exit;
+}
+
+msg = virBufferContentAndReset(buf);
+
+virNWFilterReportError(VIR_ERR_INVALID_ARG, _(%s), msg);
+VIR_FREE(msg);
+
+err_exit:
+return NULL;
+}
static virNWFilterDefPtr
virNWFilterDefParseXML(xmlXPathContextPtr ctxt) {
@@ -2017,6 +2091,7 @@ virNWFilterDefParseXML(xmlXPathContextPt
char *chain_pri_s = NULL;
virNWFilterEntryPtr entry;
int chain_priority;
+const char *name_prefix;
if (VIR_ALLOC(ret) 0) {
virReportOOMError();
@@ -2052,19 +2127,19 @@ virNWFilterDefParseXML(xmlXPathContextPt
chain = virXPathString(string(./@chain), ctxt);
if (chain) {
-if (virNWFilterChainSuffixTypeFromString(chain) 0) {
-virNWFilterReportError(VIR_ERR_INTERNAL_ERROR,
- _(unknown chain suffix '%s'), chain);
+name_prefix = virNWFilterIsAllowedChain(chain);
+if (name_prefix == NULL)
goto cleanup;
-}
ret-chainsuffix = chain;
if (chain_pri_s) {
ret-chainPriority = chain_priority;
} else {
-/* assign an implicit priority -- support XML attribute later */
-if (intMapGetByString(chain_priorities, chain, 0,
+/* assign an implicit priority */
+if
[libvirt] [PATCH V5 00/10] Make inner workings of nwfilters more flexible + extensions
The following series of patches re-does some of the inner workings
of nwfilters with the goal to enable users to write filters that have other
than the system-known chains supported right now ('root','arp','rarp','ipv4'
and 'ipv6'). Ideally users should be able to provide a chain name in the
chains XML attribute and either be able to jump to it as an 'action' or
have the chain created automatically as it is the case right now for those
chains enumerated before. The latter is now added in this patch series
as well.
I am first introducing internal priorities for the chains mentioned above so
that their creation can be made more flexible -- currently their creation and
the order in which they are accessed is hardcoded. This largely does away
with the hardcoded stuff. All assigned priorities have negative values.
Later on the priorities for the chains are made accessible via an XML
attribute.
Further, filters will be automatically accessed from the (ebtables)
interface 'root' chain using the prefix of the name of the chain. As an
example, the following filter will be accessed from the root chain for 'arp'
packets since its name 'arp-xyz' has the prefix 'arp'.
filter name='test-arp-xyz' chain='arp-xyz' priority='-650'
uuid94abeecc-c956-0ac8-1f49-a06ee8995688/uuid
rule action='accept' direction='out' priority='100'
arp opcode='Request_Reverse' arpsrcmacaddr='$MAC' arpdstmacaddr='$MAC'
arpsrcipaddr='0.0.0.0' arpdstipaddr='0.0.0.0'/
/rule
rule action='accept' direction='inout' priority='500'/
/filter
In another step the priorities of rules is extended to also allow negativ
values. This then allows the creation of rules and chains in the interface
'root' chain to be mixed so that the following layout becomes possible:
Bridge chain: libvirt-I-vnet0, entries: 6, policy: ACCEPT
-p IPv4 -j I-vnet0-ipv4
-p ARP -j I-vnet0-arp
-p ARP -j ACCEPT
-p 0x8035 -j I-vnet0-rarp
-p 0x835 -j ACCEPT
-j DROP
In the above list of rules the '-p ARP -j ACCEPT' can now be found in
between the 'jumps' to protocol-specific chains, which allows for more
efficient rule evaluation.
I did testing with the test cases in libvirt-tck as well as those in
the tests/ directory and did not see any regressions.
v5:
- addressed Eric Blake's comments
v4:
- assign priority to chain according to name prefix
- change default (internal) priorities of chains (by +200)
- fix memory leak
Regards,
Stefan
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [ANNOUNCE] libguestfs webinar TOMORROW (Friday)
I'll be holding a libguestfs live “webinar” tomorrow, Friday, 18th November 2011 at 16:00 UTC. To convert the date and time to your timezone, do: date -d '2011-11-17 16:00Z' The programme will include: - An introduction to libguestfs features. - Live demonstrations of guestfish, a Python program, inspection, and image resizing. - An overview of major new features in RHEL 6.3. - The opportunity to ask questions on any technical topic related to libguestfs. This is going to be a Webex conference. It's possible, albeit difficult, to get Webex working on Linux. You will need the Java plugin for your browser, and there are some other tips on my blog: https://rwmj.wordpress.com/2011/11/17/libguestfs-web-seminar-this-friday/ The Webex conference link, an accompanying handout, and other details will be posted to the above URL ^^ shortly before the session starts. Don't worry if you can't make it or can't get Webex to work, as it will all be recorded and placed online in an open format as soon as we can afterwards. This seminar is sponsored by Red Hat. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones virt-df lists disk usage of guests without needing to install any software inside the virtual machine. Supports Linux and Windows. http://et.redhat.com/~rjones/virt-df/ -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 1/2] XML definitions for guest NUMA and parsing routines
On 11/15/2011 04:48 AM, Daniel P. Berrange wrote:
On Fri, Nov 11, 2011 at 06:21:45PM +0530, Bharata B Rao wrote:
XML definitions for guest NUMA and parsing routines.
From: Bharata B Rao bhar...@linux.vnet.ibm.com
This patch adds XML definitions for guest NUMA specification and contains
routines to parse the same. The guest NUMA specification looks like this:
cpu
...
topology sockets='2' cores='4' threads='2'/
numa
cell cpus='0-7' memory='512000'/
cell cpus='8-15' memory='512000'/
/numa
...
/cpu
ACK
Now pushed, once I fixed the trailing space ('make syntax-check' catches
that). I also added you to AUTHORS; let me know if I need to update any
spellings.
--
Eric Blake ebl...@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 2/2] qemu: Generate -numa option
On 11/15/2011 04:49 AM, Daniel P. Berrange wrote:
On Fri, Nov 11, 2011 at 06:23:04PM +0530, Bharata B Rao wrote:
qemu: Generate -numa option
From: Bharata B Rao bhar...@linux.vnet.ibm.com
Add routines to generate -numa QEMU command line option based on
numa ... /numa XML specifications.
Signed-off-by: Bharata B Rao bhar...@linux.vnet.ibm.com
ACK
Applied, after fixing a leading TAB issue (again, 'make syntax-check'),
as well as fixing a use of
if
stmt
else {
stmt
stmt
}
to consistently use {} on both branches, per HACKING.
--
Eric Blake ebl...@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 V5 01/10] Add function to get hash tables key/value pairs
On 11/17/2011 01:11 PM, Stefan Berger wrote:
Add a function to the virHashTable for getting an array of the hash table's
key-value pairs and have the keys (optionally) sorted.
Signed-off-by: Stefan Berger stef...@linux.vnet.ibm.com
---
v5:
- follwed Eric Blake's suggestions:
- added better description to new function
- return array of key-value pairs rather than only keys
Yep, looks like it will be more reusable now. And the more I think
about it, the more I've convinced myself this is a useful interface
(allows you to sort the hashtable in multiple ways, based on what
comparator you pass in; using a data structure to store sorted elements
with hashtable lookup speeds still only gives you a single sort order).
---
src/libvirt_private.syms |2 ++
src/util/hash.c | 45 +
src/util/hash.h | 21 +
I'd still feel a bit better if we had coverage for the new function in
tests/hashtest.c, but I'll let that slide to another patch; this patch
already blocks enough other useful stuff that it will get some good
field testing from the tck runs you do, while we work on enhancing
hashtest.c.
}
+
+struct getKeysIter
+{
+virHashKeyValuePair *sortArray;
+unsigned arrayIdx;
+};
Lots simpler, huh? :)
+/*
+ * Get the hash table's key/value pairs and have them optionally sorted.
+ * The returned array contains virHashSize() elements. Aditionally,
s/Aditionally/Additionally/
+ * an empty element has been added to the end of the array whose key == NULL
+ * also indicates the end of the array.
We don't need both Additionally and also in the same sentence. How
about:
s/also indicates/to indicate/
+ * The key/value pairs are only valid as long as the underlying hash
+ * table is not modified, i.e., keys removed or the hash table deleted.
s/keys removed or the hash table deleted/no keys are removed or
inserted, and the hash table is not deleted/
+ * The caller must only free the returned array using VIR_FREE().
+ * The caller must make copies of all returned keys and values if they are
+ * to be used somewhere else.
+ */
+typedef struct _virHashKeyValuePair virHashKeyValuePair;
+typedef virHashKeyValuePair *virHashKeyValuePairPtr;
+struct _virHashKeyValuePair {
+const void *key;
+const void *value;
+};
+typedef int (*virHashKeyComparator)(const virHashKeyValuePairPtr ,
+const virHashKeyValuePairPtr );
The spacing looks awkward on these two lines;
s/ \([,)]\)/\1/
ACK. No need to see a v6 on this one, as the fixes are trivial.
--
Eric Blake ebl...@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] [libvirt-glib] Adjust example to pygobject-3.0
Hi, attached patch makes the example work again with recent pygobject-3.0 which doesn't allow to import gtk anymore. Cheers, -- Guido On Thu, Nov 17, 2011 at 10:36:23PM +0100, Guido Günther wrote: --- examples/event-test.py |7 +++ 1 files changed, 3 insertions(+), 4 deletions(-) diff --git a/examples/event-test.py b/examples/event-test.py index 33e7c73..4b06235 100644 --- a/examples/event-test.py +++ b/examples/event-test.py @@ -1,5 +1,5 @@ -import gtk +from gi.repository import Gtk import libvirt import getopt import sys @@ -47,16 +47,15 @@ def main(): print Using uri: + uri -LibvirtGLib.init() +LibvirtGLib.init(0, ) LibvirtGLib.event_register() -#libvirtglib.eventRegister() vc = libvirt.open(uri) #Add 2 callbacks to prove this works with more than just one vc.domainEventRegister(myDomainEventCallback1,None) vc.domainEventRegister(myDomainEventCallback2,None) -gtk.main() +Gtk.main() if __name__ == __main__: main() -- 1.7.7.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH V5 01.5/10] tests: test recent hash addition
Excercise the new hash API, to ensure we avoid regressions.
* tests/hashtest.c (testHashGetItems): New test.
---
I'd still feel a bit better if we had coverage for the new function in
tests/hashtest.c, but I'll let that slide to another patch;
On second thought, writing a test now wasn't too hard.
tests/hashtest.c | 86 ++
1 files changed, 86 insertions(+), 0 deletions(-)
diff --git a/tests/hashtest.c b/tests/hashtest.c
index f02b3a9..898a95d 100644
--- a/tests/hashtest.c
+++ b/tests/hashtest.c
@@ -9,6 +9,7 @@
#include hash.h
#include hashdata.h
#include testutils.h
+#include memory.h
#define testError(...) \
@@ -491,6 +492,90 @@ cleanup:
static int
+testHashGetItemsCompKey(const virHashKeyValuePairPtr a,
+const virHashKeyValuePairPtr b)
+{
+return strcmp (a-key, b-key);
+}
+
+static int
+testHashGetItemsCompValue(const virHashKeyValuePairPtr a,
+ const virHashKeyValuePairPtr b)
+{
+return strcmp (a-value, b-value);
+}
+
+static int
+testHashGetItems(const void *data ATTRIBUTE_UNUSED)
+{
+virHashTablePtr hash;
+virHashKeyValuePairPtr array = NULL;
+int ret = -1;
+char keya[] = a;
+char keyb[] = b;
+char keyc[] = c;
+char value1[] = 1;
+char value2[] = 2;
+char value3[] = 3;
+
+if (!(hash = virHashCreate(0, NULL)) ||
+virHashAddEntry(hash, keya, value3) 0 ||
+virHashAddEntry(hash, keyc, value1) 0 ||
+virHashAddEntry(hash, keyb, value2) 0) {
+if (virTestGetVerbose()) {
+testError(\nfailed to create hash);
+}
+goto cleanup;
+}
+
+if (!(array = virHashGetItems(hash, NULL)) ||
+array[3].key || array[3].value) {
+if (virTestGetVerbose()) {
+testError(\nfailed to get items with NULL sort);
+}
+goto cleanup;
+}
+VIR_FREE(array);
+
+if (!(array = virHashGetItems(hash, testHashGetItemsCompKey)) ||
+STRNEQ(array[0].key, a) ||
+STRNEQ(array[0].value, 3) ||
+STRNEQ(array[1].key, b) ||
+STRNEQ(array[1].value, 2) ||
+STRNEQ(array[2].key, c) ||
+STRNEQ(array[2].value, 1) ||
+array[3].key || array[3].value) {
+if (virTestGetVerbose()) {
+testError(\nfailed to get items with key sort);
+}
+goto cleanup;
+}
+VIR_FREE(array);
+
+if (!(array = virHashGetItems(hash, testHashGetItemsCompValue)) ||
+STRNEQ(array[0].key, c) ||
+STRNEQ(array[0].value, 1) ||
+STRNEQ(array[1].key, b) ||
+STRNEQ(array[1].value, 2) ||
+STRNEQ(array[2].key, a) ||
+STRNEQ(array[2].value, 3) ||
+array[3].key || array[3].value) {
+if (virTestGetVerbose()) {
+testError(\nfailed to get items with value sort);
+}
+goto cleanup;
+}
+
+ret = 0;
+
+cleanup:
+VIR_FREE(array);
+virHashFree(hash);
+return ret;
+}
+
+
+static int
mymain(void)
{
int ret = 0;
@@ -526,6 +611,7 @@ mymain(void)
DO_TEST(Forbidden ops in ForEach, ForEach);
DO_TEST(RemoveSet, RemoveSet);
DO_TEST(Search, Search);
+DO_TEST(GetItems, GetItems);
return (ret == 0) ? EXIT_SUCCESS : EXIT_FAILURE;
}
--
1.7.7.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH V5 02/10] Introduce an internal priority for chains
On 11/17/2011 01:11 PM, Stefan Berger wrote:
For better handling of the sorting of chains introduce an internally used
priority. Use a lookup table to store the priorities. For now their actual
values do not matter just that the values cause the chains to be properly
sorted through changes in the following patches. However, the values are
chosen as negative so that once they are sorted along with filtering rules
(whose priority may only be positive for now) they will always be instantiated
before them (lower values cause instantiation before higher values). This
is done to maintain backwards compatibility.
Signed-off-by: Stefan Berger stef...@linux.vnet.ibm.com
---
v3:
- increased filter priorities to have more room before them
---
src/conf/nwfilter_conf.c | 14 ++
src/conf/nwfilter_conf.h | 12
src/nwfilter/nwfilter_ebiptables_driver.c |4
src/nwfilter/nwfilter_ebiptables_driver.h |1 +
4 files changed, 31 insertions(+)
ACK.
@@ -2028,6 +2036,12 @@ virNWFilterDefParseXML(xmlXPathContextPt
_(unknown chain suffix '%s'), chain);
goto cleanup;
}
+/* assign an implicit priority -- support XML attribute later */
+if (intMapGetByString(chain_priorities, chain, 0,
+ ret-chainPriority) == false) {
I tend not to compare to true or false, but it's not wrong so you don't
have to change it. My argument is that comparison to bool is redundant,
since the other side of the comparison is already in bool context; I
would have used:
if (!intMapGetByString(chain_priorities, chain, 0,
ret-chainPriority)) {
--
Eric Blake ebl...@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 V5 03/10] Make filter creation in root table more flexible
On 11/17/2011 01:11 PM, Stefan Berger wrote: Use the previously introduced chain priorities to sort the chains for access from an interface's 'root' table and have them created in the proper order. This gets rid of a lot of code that was previously creating the chains in a more hardcoded way. To determine what protocol a filter is used for evaluation do prefix- matching, i.e., the filter 'arp' is used to filter for the 'arp' protocol, 'ipv4' for the 'ipv4' protocol and 'arp-xyz' will also be used to filter for the 'arp' protocol following the prefix 'arp' in its name. Signed-off-by: Stefan Berger stef...@linux.vnet.ibm.com --- v5: - followed Eric Blake's suggestions: - return -1 as error code - add comments as appropriate - other style fixes --- src/nwfilter/nwfilter_ebiptables_driver.c | 134 ++ 1 file changed, 102 insertions(+), 32 deletions(-) Looks like you covered my v4 findings. ACK. -- Eric Blake ebl...@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 V5 04/10] Use scripting for cleaning and renaming of chains
On 11/17/2011 01:11 PM, Stefan Berger wrote: Use scripts for the renaming and cleaning up of chains. This allows us to get rid of some of the code that is only capable of renaming and removing chains whose names are hardcoded. A shell function 'collect_chains' is introduced that is given the name of an ebtables chain and then recursively determines the names of all chains that are accessed from this chain and its sub-chains using 'jumps'. The resulting list of chain names is then used to delete all the found chains by first flushing and then deleting them. The same function is also used for renaming temporary filters to their final names. I tested this with the bash and dash as script interpreters. Signed-off-by: Stefan Berger stef...@linux.vnet.ibm.com --- v5: - followng Eric Blake's suggestion on a major overhaul of the embedded scripts Thanks for bearing with me, and the results look a lot nicer! I guess it pays to have one of the autoconf maintainers as a reviewer, since that means you have feedback from someone who knows shell inside and out ;) +++ libvirt-acl/src/nwfilter/nwfilter_ebiptables_driver.c @@ -92,6 +92,59 @@ static char *gawk_cmd_path; #define PRINT_CHAIN(buf, prefix, ifname, suffix) \ snprintf(buf, sizeof(buf), %c-%s-%s, prefix, ifname, suffix) +/* The collect_chains() script recursively determines all names + * of ebtables (nat) chains that are 'children' of a given 'root' chain. + * The typical out of an ebtables call is as follows: s/out/output/ + * + * # ebtables -t nat -L libvirt-I-tck-test205002 + * Bridge table: nat + * + * Bridge chain: libvirt-I-tck-test205002, entries: 5, policy: ACCEPT + * -p IPv4 -j I-tck-test205002-ipv4 + * -p ARP -j I-tck-test205002-arp + * -p 0x8035 -j I-tck-test205002-rarp + * -p 0x835 -j ACCEPT + * -j DROP + */ The comments go a long way to help. Thanks. +static const char ebtables_script_func_collect_chains[] = +collect_chains()\n Food for thought, but not strictly necessary, and certainly not worth delaying this patch any further: One of the interesting things learned in autoconf is that you want two levels of comments: those that explain how the generator works (and the generated file does not have them), and those that explain how the generated code works (those are useful both in the generator and for anyone reading the generated output). Right now, you are only using level one comments (nwfilter_ebiptables_driver.c has comments about what it is generating, but the generated script has no comments at all); depending on how complex the generated script is getting (and it _is_ getting more complex as we add more stuff), it may be worth starting to add level two comments so that someone reading the generated script can understand it without referring back to the C code. For example, this would mean doing: +static const char ebtables_script_func_collect_chains[] = +# collect_chains CHAIN...\n +# for each CHAIN, recursively output the list of sub-chains\n +# that it references through jump rules\n +collect_chains()\n + +static const char ebiptables_script_set_ifs[] = +tmp='\n'\n +IFS=' ''\t'$tmp\n; Much nicer ;) ACK. -- Eric Blake ebl...@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 V5 05/10] Use the actual names of chains in data structure
On 11/17/2011 01:11 PM, Stefan Berger wrote:
Use the name of the chain rather than its type index (enum).
This pushes the later enablement of chains with user-given names
into the XML parser. For now we still only allow those names that
are well known ('root', 'arp', 'rarp', 'ipv4' and 'ipv6').
Signed-off-by: Stefan Berger stef...@linux.vnet.ibm.com
---
src/conf/nwfilter_conf.c | 16
src/conf/nwfilter_conf.h |2 +-
src/nwfilter/nwfilter_ebiptables_driver.c | 13 +++--
src/nwfilter/nwfilter_ebiptables_driver.h |2 +-
4 files changed, 21 insertions(+), 12 deletions(-)
ACK; fairly mechanical.
--
Eric Blake ebl...@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 V5 06/10] Extend the filter XML to support priorities of chains
On 11/17/2011 01:11 PM, Stefan Berger wrote:
This patch extends the filter XML to support priorities of chains
in the XML. An example would be:
filter name='allow-arpxyz' chain='arp-xyz' priority='200'
[...]
/filter
The permitted values for priorities are [-1000, 1000].
By setting the pririty of a chain the order in which it is accessed
from the interface root chain can be influenced.
Signed-off-by: Stefan Berger stef...@linux.vnet.ibm.com
---
docs/schemas/nwfilter.rng |7 ++-
Missing documentation in docs/formatnwfilter.html.in. I'll live up to
my hard-line reputation on this one, and request a v6 with documentation
(for example, it's worth documenting whether priority 100 is accessed
before or after priority 200).
But as to the code...
@@ -2028,6 +2030,26 @@ virNWFilterDefParseXML(xmlXPathContextPt
goto cleanup;
}
+chain_pri_s = virXPathString(string(./@priority), ctxt);
+if (chain_pri_s) {
+if (sscanf(chain_pri_s, %d, chain_priority) != 1) {
Let's use virStrToLong_i() instead of sscanf(); much nicer on the error
handling aspect.
@@ -2036,11 +2058,16 @@ virNWFilterDefParseXML(xmlXPathContextPt
goto cleanup;
}
ret-chainsuffix = chain;
-/* assign an implicit priority -- support XML attribute later */
-if (intMapGetByString(chain_priorities, chain, 0,
- ret-chainPriority) == false) {
-ret-chainPriority = (NWFILTER_MAX_FILTER_PRIORITY +
- NWFILTER_MIN_FILTER_PRIORITY) / 2;
+
+if (chain_pri_s) {
+ret-chainPriority = chain_priority;
+} else {
+/* assign an implicit priority -- support XML attribute later */
Is this comment still accurate, now that you have an XML attribute?
@@ -2852,6 +2881,9 @@ virNWFilterDefFormat(virNWFilterDefPtr d
virBufferAsprintf(buf, filter name='%s' chain='%s',
def-name,
def-chainsuffix);
+if (def-chainPriority != 0)
+virBufferAsprintf(buf, priority='%d',
+ def-chainPriority);
That means an explicit pririoty='0' by the user is eaten and does not
appear on the output. But that's not too bad, and as long as we
document that priority is 0 unless explicitly specified, we're covered
(hence my plea for documentation...)
Everything else looks okay.
--
Eric Blake ebl...@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 V5 07/10] Enable chains with names having a known prefix
On 11/17/2011 01:11 PM, Stefan Berger wrote:
This patch enables chains that have a known prefix in their name.
Known prefixes are: 'ipv4', 'ipv6', 'arp', 'rarp'. All prefixes
are also protocols that can be evaluated on the ebtables level.
Following the prefix they will be automatically connected to an interface's
'root' chain and jumped into following the protocol they evalute, i.e.,
s/evalute/evaluate/
a table 'arp-xyz' will be accessed from the root table using
ebtables -t nat -A iface root table -p arp -j I-ifname-arp-xyz
thus generating a 'root' chain like this one here:
Bridge chain: libvirt-O-vnet0, entries: 5, policy: ACCEPT
-p IPv4 -j O-vnet0-ipv4
-p ARP -j O-vnet0-arp
-p 0x8035 -j O-vnet0-rarp
-p ARP -j O-vnet0-arp-xyz
-j DROP
where the chain 'arp-xyz' is accessed for filtering of ARP packets.
v3:
- assign a priority to filters that have an allowed prefix, e.g., assign
the arp chain priority to a filter arp-xyz unless user provided a
priority in the XML
Signed-off-by: Stefan Berger stef...@linux.vnet.ibm.com
---
docs/schemas/nwfilter.rng | 16 ++--
src/conf/nwfilter_conf.c | 87
++
src/conf/nwfilter_conf.h |3 +
3 files changed, 96 insertions(+), 10 deletions(-)
Another patch without docs. (It's okay if the docs comes as a separate
patch, and if a single doc patch covers the XML additions in both 6 and
7; my main concern is only that the docs get patched by the time the
series is completed, and I didn't see it when glancing ahead to 10/10).
Index: libvirt-acl/src/conf/nwfilter_conf.c
===
--- libvirt-acl.orig/src/conf/nwfilter_conf.c
+++ libvirt-acl/src/conf/nwfilter_conf.c
@@ -2007,6 +2007,80 @@ err_exit:
goto cleanup;
}
+static bool
+virNWFilterIsValidChainName(const char *chainname)
+{
+return chainname[strspn(chainname, VALID_CHAINNAME)] == 0;
+}
Thanks; this validation is essential since your shell scripting was
pretty loose on quoting (that is, if a user could name a chain with an
embedded space, or worse a semicolon, your script would probably blow up).
+
+if (!virNWFilterIsValidChainName(chainname)) {
+virNWFilterReportError(VIR_ERR_INVALID_ARG,
+ _(Chain name contains illegal characters));
+return NULL;
+}
+
+if (strlen(chainname) MAX_CHAIN_SUFFIX_SIZE) {
+virNWFilterReportError(VIR_ERR_INVALID_ARG,
+ _(Name of chain is longer than %u
characters),
+ MAX_CHAIN_SUFFIX_SIZE);
+return NULL;
+}
I think it would be worth inlining this second check (the strlen) into
the first one. That is, it would make more sense if
virNWFilterIsValidChainName checks both for valid characters and for
valid length; and all other callers only have to make one call for
validation purposes instead of two.
+virBufferAsprintf(buf,
+ _(Illegal chain name '%s'. Please use a chain name
+ called '%s' or either one of the following prefixes:
),
Illegal implies breaking a law; I prefer the term Invalid.
s/either one of/any of/
In English, either is only appropriate for a choice among 2 items, but
you are listing more than 2.
+ virNWFilterChainSuffixTypeToString(
+ VIR_NWFILTER_CHAINSUFFIX_ROOT),
+ chainname);
+for (i = 0; i VIR_NWFILTER_CHAINSUFFIX_LAST; i++) {
+if (i == VIR_NWFILTER_CHAINSUFFIX_ROOT)
+continue;
+if (printed)
+virBufferAddLit(buf, , );
+virBufferAdd(buf, virNWFilterChainSuffixTypeToString(i), -1);
+printed = true;
+}
[Hmm, wondering if this would be any simpler if we added the
virBufferTruncate that was mentioned as a possibility in another thread;
then you wouldn't need a 'printed' flag in your loop. But that's a
potential cleanup for another day.]
+
+if (virBufferError(buf)) {
+virReportOOMError();
+virBufferFreeAndReset(buf);
+goto err_exit;
+}
+
+msg = virBufferContentAndReset(buf);
+
+virNWFilterReportError(VIR_ERR_INVALID_ARG, _(%s), msg);
Don't mark %s for translation. 'make syntax-check' should be just
fine with unadorned virNWFilterReportError(VIR_ERR_INVALID_ARG, %s, msg).
if (chain_pri_s) {
ret-chainPriority = chain_priority;
} else {
-/* assign an implicit priority -- support XML attribute later */
-if (intMapGetByString(chain_priorities, chain, 0,
+/* assign an implicit priority */
Ah, the comment line I mentioned in patch 6 got changed here anyways.
Oh well, sorry if I'm causing you needless churn in conflict resolution
when you rebase.
--
Eric Blake ebl...@redhat.com+1-919-301-3266
Libvirt
Re: [libvirt] [PATCH V5 07/10] Enable chains with names having a known prefix
On 11/17/2011 01:11 PM, Stefan Berger wrote:
+
+if (strlen(chainname) MAX_CHAIN_SUFFIX_SIZE) {
+virNWFilterReportError(VIR_ERR_INVALID_ARG,
+ _(Name of chain is longer than %u
characters),
+ MAX_CHAIN_SUFFIX_SIZE);
+return NULL;
+}
Oh, one other thought...
+ data type=string
+param name=patternarp[a-zA-Z0-9_\.:-]*/param
+ /data
Since it's easy enough to do, we should probably enforce the strlen rule
during XML validation, by writing this with {0,9} instead of * (since
MAX_CHAIN_SUFFIX_SIZE of 12 minus strlen(arp) is 9), and so forth for
the other patterns.
--
Eric Blake ebl...@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 V5 08/10] Extend rule priorities into negative numbers
On 11/17/2011 01:11 PM, Stefan Berger wrote: So far rules' priorities have only been valid in the range [0,1000]. Now I am extending their priority into the range [-1000, 1000] for subsequently being able to sort rules and the access of (jumps into) chains following priorities. Signed-off-by: Stefan Berger stef...@linux.vnet.ibm.com --- src/conf/nwfilter_conf.c |7 --- src/conf/nwfilter_conf.h |6 -- src/nwfilter/nwfilter_ebiptables_driver.c |2 +- src/nwfilter/nwfilter_ebiptables_driver.h |2 +- 4 files changed, 10 insertions(+), 7 deletions(-) .rng change was made previously. ACK to this one. -- Eric Blake ebl...@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 V5 10/10] Add test cases
On 11/17/2011 01:11 PM, Stefan Berger wrote: Add test case for the chain names with known prefixes and the chain priority. Signed-off-by: Stefan Berger stef...@linux.vnet.ibm.com --- tests/nwfilterxml2xmlin/chain_prefixtest1.xml | 37 + tests/nwfilterxml2xmlout/chain_prefixtest1.xml | 21 ++ tests/nwfilterxml2xmltest.c|2 + 3 files changed, 60 insertions(+) Yay - more tests is always a good thing. ACK. -- Eric Blake ebl...@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 V5 09/10] Interleave jumping into chains with filtering rules in root table
On 11/17/2011 01:11 PM, Stefan Berger wrote:
The previous patch extends the priority of filtering rules into negative
numbers. We now use this possibility to interleave the jumping into
chains with filtering rules to for example create the 'root' table of
an interface with the following sequence of rules:
Bridge chain: libvirt-I-vnet0, entries: 6, policy: ACCEPT
-p IPv4 -j I-vnet0-ipv4
-p ARP -j I-vnet0-arp
-p ARP -j ACCEPT
-p 0x8035 -j I-vnet0-rarp
-p 0x835 -j ACCEPT
-j DROP
The '-p ARP -j ACCEPT' rule now appears between the jumps.
Since the 'arp' chain has been assigned priority -700 and the 'rarp'
chain -600, the above ordering can now be achieved with the following
rule:
rule action='accept' direction='out' priority='-650'
mac protocolid='arp'/
/rule
This patch now sorts the commands generating the above shown jumps into
chains and interleaves their execution with those for generating rules.
Overall, it looks like it does what you say. But it may be worth a v6.
@@ -2733,15 +2737,22 @@ ebtablesCreateTmpSubChain(virBufferPtr b
PRINT_CHAIN(chain, chainPrefix, ifname,
(filtername) ? filtername : l3_protocols[protoidx].val);
-virBufferAsprintf(buf,
+virBufferAsprintf(buf,
+ CMD_DEF(%s -t %s -F %s) CMD_SEPARATOR
+ CMD_EXEC
+ CMD_DEF(%s -t %s -X %s) CMD_SEPARATOR
+ CMD_EXEC
Looks like my comments on v4 4/10 would apply here as well - a patch
11/10 that refactored things to use a shell variable initialized once up
front, instead of passing repetitive command names through %s all over
the place, might make this generator easier to follow. But not a
problem for the context of this patch. This hunk adds 6 printf args,
CMD_DEF(%s -t %s -N %s) CMD_SEPARATOR
CMD_EXEC
%s
- CMD_DEF(%s -t %s -A %s -p 0x%x -j %s) CMD_SEPARATOR
+ CMD_DEF(%s -t %s -%%c %s %%s -p 0x%x -j %s)
+ CMD_SEPARATOR
and this hunk has no net effect, but generates a string which will be
handed as the format string to yet another printf? Wow, that's a bit
hard to follow...
CMD_EXEC
%s,
ebtables_cmd_path, EBTABLES_DEFAULT_TABLE, chain,
+ ebtables_cmd_path, EBTABLES_DEFAULT_TABLE, chain,
+ ebtables_cmd_path, EBTABLES_DEFAULT_TABLE, chain,
CMD_STOPONERR(stopOnError),
@@ -2750,6 +2761,24 @@ ebtablesCreateTmpSubChain(virBufferPtr b
CMD_STOPONERR(stopOnError));
+if (virBufferError(buf) ||
+VIR_REALLOC_N(tmp, (*nRuleInstances)+1) 0) {
+virReportOOMError();
+virBufferFreeAndReset(buf);
+return -1;
+}
+
+*inst = tmp;
+
+memset(tmp[*nRuleInstances], 0, sizeof(tmp[0]));
Using VIR_EXPAND_N instead of VIR_REALLOC_N would take care of this
memset for you.
+tmp[*nRuleInstances].priority = priority;
+tmp[*nRuleInstances].commandTemplate =
+virBufferContentAndReset(buf);
...If I followed things correctly, commandTemplate now has exactly two
print arguments, %c and %s. But looking further, it looks like you
already have other commandTemplate uses just like this.
ebiptablesRuleOrderSort(const void *a, const void *b)
{
+const ebiptablesRuleInstPtr insta = (const ebiptablesRuleInstPtr)a;
+const ebiptablesRuleInstPtr instb = (const ebiptablesRuleInstPtr)b;
+const char *root = virNWFilterChainSuffixTypeToString(
+ VIR_NWFILTER_CHAINSUFFIX_ROOT);
+bool root_a = STREQ(insta-neededProtocolChain, root);
+bool root_b = STREQ(instb-neededProtocolChain, root);
+
+/* ensure root chain commands appear before all others since
+ we will need them to create the child chains */
+if (root_a) {
+if (root_b) {
+goto normal;
+}
+return -1; /* a before b */
+}
+if (root_b) {
+return 1; /* b before a */
+}
+normal:
+return (insta-priority - instb-priority);
Refer to my review earlier in the series about adding a comment how
priority is in a bounded range, so the subtraction is safe.
@@ -3315,8 +3372,11 @@ ebtablesCreateTmpRootAndSubChains(virBuf
filter_names[i].key);
if ((int)idx 0)
continue;
-rc = ebtablesCreateTmpSubChain(buf, direction, ifname, idx,
- filter_names[i].key, 1);
+priority = virHashLookup(chains, filter_names[i].key);
Why do a virHashLookup, when you already have filter_names[i].value?
(See, I knew there was a reason to return key/value pairs).
--
Eric Blake ebl...@redhat.com+1-919-301-3266
Libvirt virtualization library
