- Devel - libvirt List Archives

[libvirt] [PATCH] nwfilter: Get rid of regular expressions for testing of var names and values
by Stefan Berger 04 Apr '10

04 Apr '10

Get rid of the regular expressions when evaluating variable names and values. Rather use the strspn() function. Along with this cleanup the initialization function for the code that used the regular expression can also be removed. Signed-off-by: Stefan Berger <stefanb(a)us.ibm.com> --- src/conf/nwfilter_conf.c | 5 ---- src/conf/nwfilter_conf.h | 3 -- src/conf/nwfilter_params.c | 50 +++++++++++++++------------------------------ src/conf/nwfilter_params.h | 6 +++++ 4 files changed, 24 insertions(+), 40 deletions(-) Index: libvirt-acl/src/conf/nwfilter_conf.h =================================================================== --- libvirt-acl.orig/src/conf/nwfilter_conf.h +++ libvirt-acl/src/conf/nwfilter_conf.h @@ -568,9 +568,6 @@ void virNWFilterPoolObjUnlock(virNWFilte int virNWFilterConfLayerInit(virHashIterator domUpdateCB); void virNWFilterConfLayerShutdown(void); -int virNWFilterParamConfLayerInit(void); -void virNWFilterParamConfLayerShutdown(void); - # define virNWFilterReportError(conn, code, fmt...) \ (void)conn; \ virReportErrorHelper(NULL, VIR_FROM_NWFILTER, code, __FILE__, \ Index: libvirt-acl/src/conf/nwfilter_params.c =================================================================== --- libvirt-acl.orig/src/conf/nwfilter_params.c +++ libvirt-acl/src/conf/nwfilter_params.c @@ -22,8 +22,6 @@ #include <config.h> -#include <regex.h> - #include "internal.h" #include "memory.h" @@ -35,13 +33,6 @@ #define VIR_FROM_THIS VIR_FROM_NWFILTER -/* - * regular expressions for parameter names and values - */ -static regex_t regex_nam; -static regex_t regex_val; - - static void hashDealloc(void *payload, const char *name ATTRIBUTE_UNUSED) { @@ -215,6 +206,21 @@ err_exit: #ifndef PROXY + +static bool +isValidVarName(const char *var) +{ + return var[strspn(var, VALID_VARNAME)] == 0; +} + + +static bool +isValidVarValue(const char *value) +{ + return value[strspn(value, VALID_VARVALUE)] == 0; +} + + virNWFilterHashTablePtr virNWFilterParseParamAttributes(xmlNodePtr cur) { @@ -234,9 +240,9 @@ virNWFilterParseParamAttributes(xmlNodeP nam = virXMLPropString(cur, "name"); val = virXMLPropString(cur, "value"); if (nam != NULL && val != NULL) { - if (regexec(&regex_nam, nam, 0, NULL, 0) != 0) + if (!isValidVarName(nam)) goto skip_entry; - if (regexec(&regex_val, val, 0, NULL, 0) != 0) + if (!isValidVarValue(nam)) goto skip_entry; if (virNWFilterHashTablePut(table, nam, val, 1)) { VIR_FREE(nam); @@ -296,25 +302,3 @@ virNWFilterFormatParamAttributes(virNWFi return virBufferContentAndReset(&buf); } - - -int virNWFilterParamConfLayerInit(void) { - - if (regcomp(&regex_nam, "^[a-zA-Z0-9_]+$" , - REG_NOSUB|REG_EXTENDED) != 0) - return 1; - - if (regcomp(&regex_val, "^[a-zA-Z0-9_.:]+$", - REG_NOSUB|REG_EXTENDED) != 0) { - regfree(&regex_nam); - return 1; - } - - return 0; -} - - -void virNWFilterParamConfLayerShutdown(void) { - regfree(&regex_nam); - regfree(&regex_val); -} Index: libvirt-acl/src/conf/nwfilter_params.h =================================================================== --- libvirt-acl.orig/src/conf/nwfilter_params.h +++ libvirt-acl/src/conf/nwfilter_params.h @@ -50,4 +50,10 @@ int virNWFilterHashTablePutAll(virConnec virNWFilterHashTablePtr src, virNWFilterHashTablePtr dest); +#define VALID_VARNAME \ + "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789_" + +#define VALID_VARVALUE \ + "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789_.:" + #endif /* NWFILTER_PARAMS_H */ Index: libvirt-acl/src/conf/nwfilter_conf.c =================================================================== --- libvirt-acl.orig/src/conf/nwfilter_conf.c +++ libvirt-acl/src/conf/nwfilter_conf.c @@ -2634,16 +2634,13 @@ int virNWFilterConfLayerInit(virHashIter if (virMutexInit(&updateMutex)) return 1; - if (virNWFilterParamConfLayerInit()) - return 1; - return 0; } void virNWFilterConfLayerShutdown(void) { - virNWFilterParamConfLayerShutdown(); + virMutexDestroy(&updateMutex); }

2 1

[libvirt] [RFC PATCH 0/8]: Domain Snapshot API
by Chris Lalancette 03 Apr '10

03 Apr '10

Hello, Here is the first set of RFC patches for the snapshot API. Note that this definitely isn't finished; in particular, we are going to be changing the semantic for virDomainCreateFromSnapshot, and there are still a couple of known outstanding bugs. Nevertheless, that will probably be pretty minor in terms of code, so we'd like to get review on the rest of these patches for now. I've broken the series down along lines that I believe will compile independently, though I haven't yet done a git-bisect to confirm. Unfortunately it leaves patch 3/8 as a monster patch implementing the skeleton and remote driver. The good news is that it is mostly mechanical. Suggestions for how to better split it up are welcome. Thanks go to Jirka for lots of code contribution to this series, and all of the others who took part in the discussion of this API. Comments are welcome. Chris Lalancette

3 25

[libvirt] [PATCH] build: improve check for out-of-date .gnulib submodule
by Eric Blake 02 Apr '10

02 Apr '10

git reset --hard 96e5a2d4d5b13bf2cc887562dc11d146b78d5950 ./autogen.sh make -s git pull make -s <-- expecting auto-bootstrap here, doesn't happen Use git diff to expose whether the submodule has untracked changes, which are typical on an incremental pull if .gnulib was updated but the user did not manually run 'git submodule update'. After this patch is applied, I encountered a new problem when following the reproducing pattern. Basically, the change to .gnulib between libvirt's commit 96e5a2d4 and this patch introduced a change to sys_ioctl.in.h, but gnulib (intentionally) does not make the replacement headers depend on Makefile changes. Therefore, I ended up with the generated replacement header being broken: gnulib/lib/sys/ioctl.h complained about a use of @. But that seems like something that should be fixed upstream in gnulib's bootstrap script (that is, when doing a gnulib update, all files created from .in.h file should probably be deleted). Without the benefit of that proposed gnulib fix, I worked around the problem by manually removing the stale gnulib/lib/sys/ioctl.h. * autogen.sh (t): Also run bootstrap if the gnulib submodule needs to be updated. * cfg.mk (_autogen): Likewise. Reported by Matthias Bolte. --- That took a lot longer to fix than I originally expected. But I think this patch fixes the libvirt side of things, so that make will properly rerun autogen when the .gnulib submodule changes. Thanks again to Matthias for boiling it down to a reproducible step. And now I'm off to figure out whether gnulib's bootstrap script could do a better job of deleting any generated headers that might be rendered invalid by an update in gnulib modules. autogen.sh | 2 +- cfg.mk | 6 +++++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/autogen.sh b/autogen.sh index a618d86..2f5b42d 100755 --- a/autogen.sh +++ b/autogen.sh @@ -75,7 +75,7 @@ bootstrap_hash() # Ensure that whenever we pull in a gnulib update or otherwise change to a # different version (i.e., when switching branches), we also rerun ./bootstrap. curr_status=.git-module-status -t=$(bootstrap_hash) +t=$(bootstrap_hash; git diff .gnulib) if test "$t" = "$(cat $curr_status 2>/dev/null)"; then : # good, it's up to date, all we need is autoreconf autoreconf -if diff --git a/cfg.mk b/cfg.mk index b6b2530..74fe5ed 100644 --- a/cfg.mk +++ b/cfg.mk @@ -294,7 +294,8 @@ ifeq (0,$(MAKELEVEL)) _submodule_hash = sed 's/^[ +-]//;s/ .*//' _update_required := $(shell \ actual=$$(git submodule status | $(_submodule_hash); \ - git hash-object bootstrap.conf); \ + git hash-object bootstrap.conf; \ + git diff .gnulib); \ stamp="$$($(_submodule_hash) $(_curr_status) 2>/dev/null)"; \ test "$$stamp" = "$$actual"; echo $$?) ifeq (1,$(_update_required)) @@ -303,9 +304,12 @@ Makefile: _autogen endif endif +# It is necessary to call autogen any time gnulib changes. Autogen +# reruns configure, then we regenerate all Makefiles at once. .PHONY: _autogen _autogen: $(srcdir)/autogen.sh + ./config.status # Exempt @...@ uses of these symbols. _makefile_at_at_check_exceptions = ' && !/(SCHEMA|SYSCONF)DIR/' -- 1.6.6.1

2 2

[libvirt] [PATCH] [0/4] Managed save APIs
by Daniel Veillard 02 Apr '10

02 Apr '10

As posted earlier, I have implemented the small set of managed save APIs, where libvirt stores the domain state itself and can then recover that state when the domain is started up. I think the code is complete, but not really tested (I still need to debug a failure which seems unrelated), with the exception of the virsh commands which probably need to be extended for convenience. Also I implemented it only for the qemu driver, I would not be surprized if an ESX backend could be implemented since there is no file path in this API. More documentation is needed too. Thanks Chris Lalancette who wrote a large part of this code ! Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel(a)veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/

3 8

[libvirt] [PATCH 0/3] Add dnsmasq module
by Satoru SATOH 02 Apr '10

02 Apr '10

The following series of patches is a prototype implementation of dnsmasq module. It implements an idea originally suggested by Daniel-san and can address the problem that too many --dhcp-host args hitting ARG_MAX limit I reported last year [1]. [1] https://www.redhat.com/archives/libvir-list/2009-October/msg00216.html Thanks, Satoru SATOH

1 3

[libvirt] [PATCH] Optimization of the check for valid interface name
by Stefan Berger 02 Apr '10

02 Apr '10

The attached patch optimizes the validation of the name of an interface. Signed-off-by: Stefan Berger Index: libvirt-acl/src/conf/domain_conf.c =================================================================== --- libvirt-acl.orig/src/conf/domain_conf.c +++ libvirt-acl/src/conf/domain_conf.c @@ -1795,7 +1795,7 @@ cleanup: static bool isValidIfname(const char *ifname) { - return (strspn(ifname, VALID_IFNAME_CHARS) == strlen(ifname)); + return ifname[strspn(ifname, VALID_IFNAME_CHARS)] == 0; }

3 2

[libvirt] [PATCH] Use virStrToLong_ui rather than virStrToLong_i where possible
by Stefan Berger 02 Apr '10

02 Apr '10

Use the virStrToLong_ui() function rather than the virStrToLong_i() where possible. Signed-off-by: Stefan Berger <stefanb(a)us.ibm.com> --- src/conf/nwfilter_conf.c | 33 +++++++++++++++++---------------- 1 file changed, 17 insertions(+), 16 deletions(-) Index: libvirt-acl/src/conf/nwfilter_conf.c =================================================================== --- libvirt-acl.orig/src/conf/nwfilter_conf.c +++ libvirt-acl/src/conf/nwfilter_conf.c @@ -1126,6 +1126,7 @@ virNWFilterRuleDetailsParse(virConnectPt enum virNWFilterEntryItemFlags *flags ,match_flag = 0, flags_set = 0; nwItemDesc *item; int int_val; + unsigned int uint_val; void *data_ptr, *storage_ptr; valueValidator validator; char *match = virXMLPropString(node, "match"); @@ -1175,12 +1176,12 @@ virNWFilterRuleDetailsParse(virConnectPt case DATATYPE_UINT8: storage_ptr = &item->u.u8; - if (virStrToLong_i(prop, NULL, 10, &int_val) >= 0) { - if (int_val >= 0 && int_val <= 0xff) { + if (virStrToLong_ui(prop, NULL, 10, &uint_val) >= 0) { + if (uint_val <= 0xff) { if (!validator) - *(uint8_t *)storage_ptr = int_val; + *(uint8_t *)storage_ptr = uint_val; found = 1; - data_ptr = &int_val; + data_ptr = &uint_val; } else rc = -1; } else @@ -1189,12 +1190,12 @@ virNWFilterRuleDetailsParse(virConnectPt case DATATYPE_UINT16: storage_ptr = &item->u.u16; - if (virStrToLong_i(prop, NULL, 10, &int_val) >= 0) { - if (int_val >= 0 && int_val <= 0xffff) { + if (virStrToLong_ui(prop, NULL, 10, &uint_val) >= 0) { + if (uint_val <= 0xffff) { if (!validator) - *(uint16_t *)storage_ptr = int_val; + *(uint16_t *)storage_ptr = uint_val; found = 1; - data_ptr = &int_val; + data_ptr = &uint_val; } else rc = -1; } else @@ -1212,13 +1213,13 @@ virNWFilterRuleDetailsParse(virConnectPt case DATATYPE_IPMASK: storage_ptr = &item->u.u8; - if (virStrToLong_i(prop, NULL, 10, &int_val) == 0) { - if (int_val >= 0 && int_val <= 32) { + if (virStrToLong_ui(prop, NULL, 10, &uint_val) == 0) { + if (uint_val <= 32) { if (!validator) *(uint8_t *)storage_ptr = - (uint8_t)int_val; + (uint8_t)uint_val; found = 1; - data_ptr = &int_val; + data_ptr = &uint_val; } else rc = -1; } else { @@ -1266,13 +1267,13 @@ virNWFilterRuleDetailsParse(virConnectPt case DATATYPE_IPV6MASK: storage_ptr = &item->u.u8; - if (virStrToLong_i(prop, NULL, 10, &int_val) == 0) { - if (int_val >= 0 && int_val <= 128) { + if (virStrToLong_ui(prop, NULL, 10, &uint_val) == 0) { + if (uint_val <= 128) { if (!validator) *(uint8_t *)storage_ptr = - (uint8_t)int_val; + (uint8_t)uint_val; found = 1; - data_ptr = &int_val; + data_ptr = &uint_val; } else rc = -1; } else {

2 1

[libvirt] [PATCH v1 0/2] Fixes to network filter rules instantiator + XML parser and XML parser test cases
by Stefan Berger 02 Apr '10

02 Apr '10

The following two patches fix some problems when instantiating the network filtering rules provided by the test cases in the 2nd patch on a running virtual machine using 'nwfilter-define'. There's one known problem realted to 'dscp' values not being capped at '63'. This will be fixed later. Regards, Stefan

2 5

[libvirt] [PATCH] build: import latest gnulib
by Eric Blake 02 Apr '10

02 Apr '10

* .gnulib: Update to latest. * cfg.mk (sc_prohibit_test_minus_ao): Delete, it was moved into gnulib's maint.mk. --- Here's the full list of gnulib changes: * .gnulib 4f01268...e01b458 (44): > maint.mk: check for [ as well as test > gnumakefile: don't squelch _version output > sys_time: avoid compiler warnings > maint.mk: split a long line > maint.mk: s/_header_without_use/_sc_header_without_use/ > maint: improve module indicators > fdopendir: work around FreeBSD bug > Emit #pragma system_header after the inclusion guard, not before. > Fix indentation. > save-cwd: don't leak a file descriptor when the caller execs > Remove vestiges of fts-lgpl module. > Fix definition of tests witness macro. > Fix definition of tests witness macro. > Fix ioctl's protoype on glibc systems. > Normalize copyright line formatting. > exclude: fix the case of globs vs. EXCLUDE_INCLUDE > maint.mk: prohibit use of test's -o and -a operators > Remove unused GNULIB_XYZ macro definitions. > Mark privileged tests modules. > Split C++ tests into separate tests modules. > gnulib-tool: Allow 'foo-tests' module even if there is no module 'foo'. > gnulib-tool: Add support for special categories of tests. > ansi-c++-opt: Allow turning off the C++ build by default. > unistd: Avoid #define replacements in C++ mode. > Fix link errors on mingw. > lib-ignore: Determine different options for different compilers. > Remove unused GNULIB_XYZ macro definitions. > Add a remark about certain modules. > Resolve conflict between the two kinds of module indicators. > autoupdate > Distinguish two kinds of module indicators. > New module description field 'Comment'. > Addendum to 2010-02-07 commit. > Use GNULIB_POSIXCHECK instead of GNULIB_PORTCHECK. > Avoid side effects from tests-related modules on the compilation of lib. > Factorize common .m4 code. > Fix a compilation error on Cygwin with g++ >= 4.3. > Rename gl_GETLOGIN. > Rename gl_GETLOGIN_R. > Rename gl_INET_NTOP. > Rename gl_INET_PTON. > Rename gl_MBSLEN. > Rename gl_FORKPTY and gl_OPENPTY. > top/maint.mk (sc_texinfo_acronym): Don't infloop if there is no *.texi files. .gnulib | 2 +- cfg.mk | 8 -------- 2 files changed, 1 insertions(+), 9 deletions(-) diff --git a/.gnulib b/.gnulib index 4f01268..e01b458 160000 --- a/.gnulib +++ b/.gnulib @@ -1 +1 @@ -Subproject commit 4f01268d0c584c20704e42527f4fa125e7525aae +Subproject commit e01b458f40440e64ce6f59beb0b446219bb6302d diff --git a/cfg.mk b/cfg.mk index 2e7b68a..07360b0 100644 --- a/cfg.mk +++ b/cfg.mk @@ -272,14 +272,6 @@ sc_preprocessor_indentation: echo '$(ME): skipping test $@: cppi not installed' 1>&2; \ fi -_m1 = use "test C1 && test C2", not "test C1 -''a C2" -_m2 = use "test C1 || test C2", not "test C1 -''o C2" -# Using test's -a and -o operators is not portable. -sc_prohibit_test_minus_ao: - @re='(\<test| \[+) .+ -[ao] ' \ - msg='$(_m1); $(_m2)' \ - $(_prohibit_regexp) - sc_copyright_format: @$(VC_LIST_EXCEPT) | xargs grep -ni 'copyright .*Red 'Hat \ | grep -v Inc \ -- 1.6.6.1

2 3

[libvirt] Pushed qemuDomainRestore fix
by Chris Lalancette 02 Apr '10

02 Apr '10

FYI, I just pushed this fix (with blessing from DV): commit d62f4c36c2c28b96952143247a1360e8c18f64c7 Author: Chris Lalancette <clalance(a)redhat.com> Date: Tue Mar 23 09:11:29 2010 -0400 Don't use virFileReadLimFD in qemuDomainRestore. virFileReadLimFD is a poor fit for reading the header of the restore file. The problem is that virFileReadLimFD returns an error when there is more data after the amount you ask to read, but that is *expected* in this case. This patch is essentially a revert of 1a4d5c9543641c444dccd682f6256ee3faf22a80, but I don't think that commit does what it says anyway. It purports to prevent an unwarranted OOM error, but since virFileReadLimFD will allocate memory up to the maximum anyway, the upper limit on the total amount of memory allocated is the same for either the old version or the new version. Since the old saferead actually works and virFileReadLimFD does not, revert to using saferead. Signed-off-by: Chris Lalancette <clalance(a)redhat.com> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 4291bc7..70d2781 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -5547,7 +5547,12 @@ static int qemudDomainRestore(virConnectPtr conn, goto cleanup; } - if (virFileReadLimFD(fd, header.xml_len, &xml) != header.xml_len) { + if (VIR_ALLOC_N(xml, header.xml_len) < 0) { + virReportOOMError(); + goto cleanup; + } + + if (saferead(fd, xml, header.xml_len) != header.xml_len) { qemuReportError(VIR_ERR_OPERATION_FAILED, "%s", _("failed to read XML")); goto cleanup; -- Chris Lalancette

2 1