7:37 a.m.
Hello,
This is the last set of patches before the amount of shared code
between the SuSE-driver (in its current state, at least) and the
current initscripts driver is almost nil.
The last two patches were only discussed briefly, so I kept them
at the end. Feel free to react, and note that it means that all
"variables" in an ifcfg-file will get single quotes. The lens is
however able to read unquoted variables from these files.
For the SuSE-driver, I currently lack VLAN-support (which on SuSE
is a mess in some aspects), and some tests. Hopefully, these
things won't take long to fix.
/Jonas
7:37 a.m.
New subject: [PATCH 1/7] Moving of more functions to utility files
dutil.{c,h}: Added defnode, make_netcf_if, run1
drv_initscripts.c: Removed defnode, make_netcf_if, run1
---
src/drv_initscripts.c | 47 -------------------------------------------
src/dutil.c | 53 +++++++++++++++++++++++++++++++++++++++++++++++++
src/dutil.h | 11 ++++++++++
3 files changed, 64 insertions(+), 47 deletions(-)
diff --git a/src/drv_initscripts.c b/src/drv_initscripts.c
index d4e2259..dc8d471 100644
--- a/src/drv_initscripts.c
+++ b/src/drv_initscripts.c
@@ -91,30 +91,6 @@ static const char *const subif_paths[] = {
"MASTER", "BRIDGE"
};
-ATTRIBUTE_FORMAT(printf, 4, 5)
-static int defnode(struct netcf *ncf, const char *name, const char *value,
- const char *format, ...) {
- struct augeas *aug = get_augeas(ncf);
- va_list ap;
- char *expr = NULL;
- int r, created;
-
- va_start(ap, format);
- r = vasprintf (&expr, format, ap);
- va_end (ap);
- if (r < 0)
- expr = NULL;
- ERR_THROW(r < 0, ncf, ENOMEM, "failed to format node expression");
-
- r = aug_defnode(aug, name, expr, value, &created);
- ERR_THROW(r < 0, ncf, EOTHER, "failed to define node %s", name);
-
- /* Fallthrough intentional */
- error:
- free(expr);
- return (r < 0) ? -1 : created;
-}
-
static int is_slave(struct netcf *ncf, const char *intf) {
for (int s = 0; s < ARRAY_CARDINALITY(subif_paths); s++) {
int r;
@@ -376,21 +352,6 @@ int drv_num_of_interfaces(struct netcf *ncf, unsigned int flags) {
return list_interface_ids(ncf, 0, NULL, flags, "DEVICE");
}
-static struct netcf_if *make_netcf_if(struct netcf *ncf, char *name) {
- int r;
- struct netcf_if *result = NULL;
-
- r = make_ref(result);
- ERR_THROW(r < 0, ncf, ENOMEM, NULL);
- result->ncf = ref(ncf);
- result->name = name;
- return result;
-
- error:
- unref(result, netcf_if);
- return result;
-}
-
struct netcf_if *drv_lookup_by_name(struct netcf *ncf, const char *name) {
struct netcf_if *nif = NULL;
char *pathx = NULL;
@@ -836,14 +797,6 @@ const char *drv_mac_string(struct netcf_if *nif) {
* Bringing interfaces up/down
*/
-static void run1(struct netcf *ncf, const char *prog, const char *name) {
- const char *const argv[] = {
- prog, name, NULL
- };
-
- run_program(ncf, argv);
-}
-
int drv_if_up(struct netcf_if *nif) {
static const char *const ifup = "ifup";
struct netcf *ncf = nif->ncf;
diff --git a/src/dutil.c b/src/dutil.c
index bb03f39..28832b3 100644
--- a/src/dutil.c
+++ b/src/dutil.c
@@ -125,6 +125,30 @@ struct augeas *get_augeas(struct netcf *ncf) {
return NULL;
}
+ATTRIBUTE_FORMAT(printf, 4, 5)
+int defnode(struct netcf *ncf, const char *name, const char *value,
+ const char *format, ...) {
+ struct augeas *aug = get_augeas(ncf);
+ va_list ap;
+ char *expr = NULL;
+ int r, created;
+
+ va_start(ap, format);
+ r = vasprintf (&expr, format, ap);
+ va_end (ap);
+ if (r < 0)
+ expr = NULL;
+ ERR_THROW(r < 0, ncf, ENOMEM, "failed to format node expression");
+
+ r = aug_defnode(aug, name, expr, value, &created);
+ ERR_THROW(r < 0, ncf, EOTHER, "failed to define node %s", name);
+
+ /* Fallthrough intentional */
+ error:
+ free(expr);
+ return (r < 0) ? -1 : created;
+}
+
int aug_fmt_match(struct netcf *ncf, char ***matches, const char *fmt, ...) {
struct augeas *aug = NULL;
char *path = NULL;
@@ -301,6 +325,22 @@ int is_active(struct netcf *ncf, const char *intf) {
return ((ifr.ifr_flags & IFF_UP) == IFF_UP);
}
+/* Create a new netcf if instance for interface NAME */
+struct netcf_if *make_netcf_if(struct netcf *ncf, char *name) {
+ int r;
+ struct netcf_if *result = NULL;
+
+ r = make_ref(result);
+ ERR_THROW(r < 0, ncf, ENOMEM, NULL);
+ result->ncf = ref(ncf);
+ result->name = name;
+ return result;
+
+ error:
+ unref(result, netcf_if);
+ return result;
+}
+
/*
* Test interface
*/
@@ -349,6 +389,19 @@ int dutil_put_aug(struct netcf *ncf, const char *aug_xml, char
**ncf_xml) {
}
/*
+ * Bringing interfaces up/down
+ */
+
+/* Run the program PROG with the single argument ARG */
+void run1(struct netcf *ncf, const char *prog, const char *arg) {
+ const char *const argv[] = {
+ prog, arg, NULL
+ };
+
+ run_program(ncf, argv);
+}
+
+/*
* Local variables:
* indent-tabs-mode: nil
* c-indent-level: 4
diff --git a/src/dutil.h b/src/dutil.h
index b22c512..6dc05e2 100644
--- a/src/dutil.h
+++ b/src/dutil.h
@@ -48,6 +48,11 @@ int xasprintf(char **strp, const char *format, ...);
/* Get or create the augeas instance from NCF */
struct augeas *get_augeas(struct netcf *ncf);
+/* Define a node inside the augeas tree */
+ATTRIBUTE_FORMAT(printf, 4, 5)
+int defnode(struct netcf *ncf, const char *name, const char *value,
+ const char *format, ...);
+
/* Format a path by doing a printf of FMT and the var args, then call
AUG_MATCH on that path. Sets NCF->ERRCODE on error */
ATTRIBUTE_FORMAT(printf, 3, 4)
@@ -83,12 +88,18 @@ int init_ioctl_fd(struct netcf *ncf);
/* Check if the interface INTF is up using a ioctl call */
int is_active(struct netcf *ncf, const char *intf);
+/* Create a new netcf if instance for interface NAME */
+struct netcf_if *make_netcf_if(struct netcf *ncf, char *name);
+
/* Transform the interface XML NCF_XML into Augeas XML AUG_XML */
int dutil_get_aug(struct netcf *ncf, const char *ncf_xml, char **aug_xml);
/* Transform the Augeas XML AUG_XML into interface XML NCF_XML */
int dutil_put_aug(struct netcf *ncf, const char *aug_xml, char **ncf_xml);
+/* Run the program PROG with the single argument ARG */
+void run1(struct netcf *ncf, const char *prog, const char *arg);
+
#endif
/*
--
1.6.2
7:37 a.m.
New subject: [PATCH 2/7] Moving of modeprobe_(un)alias_bond
dutil_linux.{c,h}: Added modprobed_alias_bond, modprobed_unalias_bond
(earlier named modprobe_(un)alias_bond, renamed to point out that
they are using the modprobe.d file-tree)
drv_initscripts.c: Removed modprobe_alias_bond, modprobe_unalias_bond
---
src/drv_initscripts.c | 62 +-----------------------------------------------
src/dutil_linux.c | 58 +++++++++++++++++++++++++++++++++++++++++++++
src/dutil_linux.h | 9 +++++++
3 files changed, 69 insertions(+), 60 deletions(-)
diff --git a/src/drv_initscripts.c b/src/drv_initscripts.c
index dc8d471..0c69071 100644
--- a/src/drv_initscripts.c
+++ b/src/drv_initscripts.c
@@ -576,64 +576,6 @@ static int bridge_slaves(struct netcf *ncf, const char *name, char
***slaves) {
return -1;
}
-/* Add an 'alias NAME bonding' to an appropriate file in /etc/modprobe.d,
- * if none exists yet. If we need to create a new one, it goes into the
- * file netcf.conf.
- */
-static void modprobe_alias_bond(struct netcf *ncf, const char *name) {
- char *path = NULL;
- struct augeas *aug = get_augeas(ncf);
- int r, nmatches;
-
- nmatches = aug_fmt_match(ncf, NULL,
- "/files/etc/modprobe.d/*/alias[ . =
'%s']",
- name);
- ERR_BAIL(ncf);
-
- if (nmatches == 0) {
- /* Add a new alias node; this probably deserves better API support
- in Augeas, it's too convoluted */
- r = xasprintf(&path,
- "/files/etc/modprobe.d/netcf.conf/alias[last()]", name);
- ERR_COND_BAIL(r < 0, ncf, ENOMEM);
- nmatches = aug_match(aug, path, NULL);
- if (nmatches > 0) {
- r = aug_insert(aug, path, "alias", 0);
- ERR_COND_BAIL(r < 0, ncf, EOTHER);
- }
- r = aug_set(aug, path, name);
- FREE(path);
- }
-
- r = xasprintf(&path,
- "/files/etc/modprobe.d/*/alias[ . =
'%s']/modulename",
- name);
- ERR_COND_BAIL(r < 0, ncf, ENOMEM);
-
- r = aug_set(aug, path, "bonding");
- ERR_COND_BAIL(r < 0, ncf, EOTHER);
-
- error:
- FREE(path);
-}
-
-/* Remove the alias for NAME to the bonding module */
-static void modprobe_unalias_bond(struct netcf *ncf, const char *name) {
- char *path = NULL;
- struct augeas *aug = get_augeas(ncf);
- int r;
-
- r = xasprintf(&path,
- "/files/etc/modprobe.d/*/alias[ . = '%s'][modulename =
'bonding']",
- name);
- ERR_COND_BAIL(r < 0, ncf, ENOMEM);
-
- r = aug_rm(aug, path);
- ERR_COND_BAIL(r < 0, ncf, EOTHER);
- error:
- FREE(path);
-}
-
struct netcf_if *drv_define(struct netcf *ncf, const char *xml_str) {
xmlDocPtr ncf_xml = NULL, aug_xml = NULL;
char *name = NULL, *path = NULL;
@@ -666,7 +608,7 @@ struct netcf_if *drv_define(struct netcf *ncf, const char *xml_str) {
ERR_BAIL(ncf);
if (is_bond(ncf, name)) {
- modprobe_alias_bond(ncf, name);
+ modprobed_alias_bond(ncf, name);
ERR_BAIL(ncf);
}
@@ -696,7 +638,7 @@ int drv_undefine(struct netcf_if *nif) {
ERR_BAIL(ncf);
if (is_bond(ncf, nif->name)) {
- modprobe_unalias_bond(ncf, nif->name);
+ modprobed_unalias_bond(ncf, nif->name);
ERR_BAIL(ncf);
}
diff --git a/src/dutil_linux.c b/src/dutil_linux.c
index cd87d3b..4e99ef6 100644
--- a/src/dutil_linux.c
+++ b/src/dutil_linux.c
@@ -94,6 +94,64 @@ int aug_get_mac(struct netcf *ncf, const char *intf, const char **mac)
{
return r;
}
+/* Add an 'alias NAME bonding' to an appropriate file in /etc/modprobe.d,
+ * if none exists yet. If we need to create a new one, it goes into the
+ * file netcf.conf.
+ */
+void modprobed_alias_bond(struct netcf *ncf, const char *name) {
+ char *path = NULL;
+ struct augeas *aug = get_augeas(ncf);
+ int r, nmatches;
+
+ nmatches = aug_fmt_match(ncf, NULL,
+ "/files/etc/modprobe.d/*/alias[ . =
'%s']",
+ name);
+ ERR_BAIL(ncf);
+
+ if (nmatches == 0) {
+ /* Add a new alias node; this probably deserves better API support
+ in Augeas, it's too convoluted */
+ r = xasprintf(&path,
+ "/files/etc/modprobe.d/netcf.conf/alias[last()]", name);
+ ERR_COND_BAIL(r < 0, ncf, ENOMEM);
+ nmatches = aug_match(aug, path, NULL);
+ if (nmatches > 0) {
+ r = aug_insert(aug, path, "alias", 0);
+ ERR_COND_BAIL(r < 0, ncf, EOTHER);
+ }
+ r = aug_set(aug, path, name);
+ FREE(path);
+ }
+
+ r = xasprintf(&path,
+ "/files/etc/modprobe.d/*/alias[ . =
'%s']/modulename",
+ name);
+ ERR_COND_BAIL(r < 0, ncf, ENOMEM);
+
+ r = aug_set(aug, path, "bonding");
+ ERR_COND_BAIL(r < 0, ncf, EOTHER);
+
+ error:
+ FREE(path);
+}
+
+/* Remove the alias for NAME to the bonding module */
+void modprobed_unalias_bond(struct netcf *ncf, const char *name) {
+ char *path = NULL;
+ struct augeas *aug = get_augeas(ncf);
+ int r;
+
+ r = xasprintf(&path,
+ "/files/etc/modprobe.d/*/alias[ . = '%s'][modulename =
'bonding']",
+ name);
+ ERR_COND_BAIL(r < 0, ncf, ENOMEM);
+
+ r = aug_rm(aug, path);
+ ERR_COND_BAIL(r < 0, ncf, EOTHER);
+ error:
+ FREE(path);
+}
+
/*
* Local variables:
* indent-tabs-mode: nil
diff --git a/src/dutil_linux.h b/src/dutil_linux.h
index 7483459..0351b8b 100644
--- a/src/dutil_linux.h
+++ b/src/dutil_linux.h
@@ -29,6 +29,15 @@ int aug_match_mac(struct netcf *ncf, const char *mac, char
***matches);
/* Get the MAC address of the interface INTF */
int aug_get_mac(struct netcf *ncf, const char *intf, const char **mac);
+/* Add an 'alias NAME bonding' to an appropriate file in /etc/modprobe.d,
+ * if none exists yet. If we need to create a new one, it goes into the
+ * file netcf.conf.
+ */
+void modprobed_alias_bond(struct netcf *ncf, const char *name);
+
+/* Remove an 'alias NAME bonding' as created by modprobed_alias_bond */
+void modprobed_unalias_bond(struct netcf *ncf, const char *name);
+
#endif
/*
--
1.6.2
7:37 a.m.
New subject: [PATCH 3/7] Add strlower, update dutil_linux.c:aug_match_mac
dutil.{c,h}:
- Added strllower
dutil_linux.{c,h}:
- Made use of strllower and aug_fmt_match in aug_match_mac
- Fixed a typo in the comment above the function head and prototype
drv_initscruots.c:
- Updated the use of aug_match_mac
- Broke the long function head of drv_list_interfaces
---
src/drv_initscripts.c | 5 +++--
src/dutil.c | 7 +++++++
src/dutil.h | 3 +++
src/dutil_linux.c | 26 +++++++++++---------------
src/dutil_linux.h | 2 +-
5 files changed, 25 insertions(+), 18 deletions(-)
diff --git a/src/drv_initscripts.c b/src/drv_initscripts.c
index 0c69071..550df65 100644
--- a/src/drv_initscripts.c
+++ b/src/drv_initscripts.c
@@ -344,7 +344,8 @@ static int list_interface_ids(struct netcf *ncf,
return -1;
}
-int drv_list_interfaces(struct netcf *ncf, int maxnames, char **names, unsigned int
flags) {
+int drv_list_interfaces(struct netcf *ncf, int maxnames, char **names,
+ unsigned int flags) {
return list_interface_ids(ncf, maxnames, names, flags, "DEVICE");
}
@@ -677,7 +678,7 @@ int drv_lookup_by_mac_string(struct netcf *ncf, const char *mac,
ERR_BAIL(ncf);
nmatches = aug_match_mac(ncf, mac, &matches);
- ERR_THROW(nmatches < 0, ncf, EOTHER, "looking up %s failed", mac);
+ ERR_BAIL(ncf);
if (nmatches == 0) {
result = 0;
goto done;
diff --git a/src/dutil.c b/src/dutil.c
index 28832b3..2066a45 100644
--- a/src/dutil.c
+++ b/src/dutil.c
@@ -62,6 +62,13 @@ int xasprintf(char **strp, const char *format, ...) {
return result;
}
+/* Convert string of max length buflen to uppercase in place */
+void strllower(char *s, unsigned int buflen) {
+ if (s != NULL)
+ for (int i = 0; s[i] != '\0' && i < buflen; i++)
+ s[i] = tolower(s[i]);
+}
+
/* Get the Augeas instance; if we already initialized it, just return
* it. Otherwise, create a new one and return that.
*/
diff --git a/src/dutil.h b/src/dutil.h
index 6dc05e2..eee5e4e 100644
--- a/src/dutil.h
+++ b/src/dutil.h
@@ -45,6 +45,9 @@ struct augeas_pv {
/* Like asprintf, but set *STRP to NULL on error */
int xasprintf(char **strp, const char *format, ...);
+/* Convert string of max length buflen to uppercase in place */
+void strllower(char *s, unsigned int buflen);
+
/* Get or create the augeas instance from NCF */
struct augeas *get_augeas(struct netcf *ncf);
diff --git a/src/dutil_linux.c b/src/dutil_linux.c
index 4e99ef6..9727367 100644
--- a/src/dutil_linux.c
+++ b/src/dutil_linux.c
@@ -39,23 +39,20 @@
#include "dutil.h"
#include "dutil_linux.h"
-/* Returns a list of all interfaces with MAC address INTF */
+/* Returns a list of all interfaces with MAC address MAC */
int aug_match_mac(struct netcf *ncf, const char *mac, char ***matches) {
- int r, nmatches;
- char *path;
- struct augeas *aug = get_augeas(ncf);
+ int nmatches;
+ char *lcmac = NULL;
- r = xasprintf(&path,
- "/files/sys/class/net/*[address/content = '%s']", mac);
- ERR_COND_BAIL(r < 0, ncf, ENOMEM);
+ /* Make sure that the mac address is in lower case */
+ lcmac = strndup(mac, 17);
+ ERR_COND_BAIL(lcmac == NULL, ncf, ENOMEM);
+ strllower(lcmac, 18);
- r = aug_match(aug, path, matches);
- /* Messages for a aug_match-fail are handled outside this function */
- if (r < 0)
- goto error;
+ nmatches = aug_fmt_match(ncf, matches,
+ "/files/sys/class/net/*[address/content = '%s']", lcmac);
+ ERR_BAIL(ncf);
- nmatches = r;
- r = -1;
for (int i = 0; i < nmatches; i++) {
char *n = strrchr((*matches)[i], '/');
ERR_THROW(n == NULL, ncf, EINTERNAL, "missing / in sysfs path");
@@ -72,8 +69,7 @@ int aug_match_mac(struct netcf *ncf, const char *mac, char ***matches)
{
return nmatches;
error:
- FREE(path);
- return r;
+ return -1;
}
/* Get the MAC address of the interface INTF */
diff --git a/src/dutil_linux.h b/src/dutil_linux.h
index 0351b8b..085c2f9 100644
--- a/src/dutil_linux.h
+++ b/src/dutil_linux.h
@@ -23,7 +23,7 @@
#ifndef DUTIL_LINUX_H_
#define DUTIL_LINUX_H_
-/* Returns a list of all interfaces with MAC address INTF */
+/* Returns a list of all interfaces with MAC address MAC */
int aug_match_mac(struct netcf *ncf, const char *mac, char ***matches);
/* Get the MAC address of the interface INTF */
--
1.6.2
7:37 a.m.
New subject: [PATCH 4/7] Lift out test functions to tutil.{c,h}
* tutil.{c,h}:
- Added format_error, read_test_file, assert_ncf_no_error,
parse_xml, xml_attrs_subset, xml_next_element, xml_nodes_equal,
assert_xml_equals, run, setup, teardown from test-initscripts.c
- Created identify_driver, in order for setup() to create a
(correctly named) root dir for the tests.
* test-initscripts.c:
- Removed functions named above
- Added call to identify_driver("initscripts") before starting tests
---
tests/Makefile.am | 2 +-
tests/test-initscripts.c | 199 +++--------------------------------------
tests/tutil.c | 223 ++++++++++++++++++++++++++++++++++++++++++++++
tests/tutil.h | 87 ++++++++++++++++++
4 files changed, 325 insertions(+), 186 deletions(-)
create mode 100644 tests/tutil.c
create mode 100644 tests/tutil.h
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 431c734..b5688ef 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -21,7 +21,7 @@ INCLUDES = -I$(top_srcdir)/src
# FIXME: Needs to be conditionalized
check_PROGRAMS=test-initscripts
-test_initscripts_SOURCES = test-initscripts.c cutest.c cutest.h
+test_initscripts_SOURCES = test-initscripts.c cutest.c cutest.h tutil.c tutil.h
test_initscripts_LDADD = $(top_builddir)/src/libnetcf.la $(GNULIB)
# Clean up files generated by test-initscripts
diff --git a/tests/test-initscripts.c b/tests/test-initscripts.c
index 7f62c0b..9a16c17 100644
--- a/tests/test-initscripts.c
+++ b/tests/test-initscripts.c
@@ -27,195 +27,17 @@
#include "safe-alloc.h"
#include "read-file.h"
+#include "tutil.h"
+
#include <stdio.h>
#include <libxml/tree.h>
-static const char *abs_top_srcdir;
-static const char *abs_top_builddir;
-static char *root, *src_root;
-static struct netcf *ncf;
-
-#define die(msg) \
- do { \
- fprintf(stderr, "%s:%d: Fatal error: %s\n", __FILE__, __LINE__, msg);
\
- exit(EXIT_FAILURE); \
- } while(0)
-
-static void format_error(char **strp, const char *format, ...) {
- va_list args;
- int r;
-
- va_start (args, format);
- r = vasprintf (strp, format, args);
- va_end (args);
- if (r < 0)
- die("Failed to format error message (out of memory)");
-}
-
-static char *read_test_file(CuTest *tc, const char *relpath) {
- char *path = NULL;
- char *txt = NULL;
- size_t length;
-
- if (asprintf(&path, "%s/tests/%s", abs_top_srcdir, relpath) < 0)
- CuFail(tc, "Could not format path for file");
- txt = read_file(path, &length);
- if (txt == NULL)
- CuFail(tc, "Failed to read file");
- return txt;
-}
-
-static void assert_ncf_no_error(CuTest *tc) {
- CuAssertIntEquals(tc, NETCF_NOERROR, ncf_error(ncf, NULL, NULL));
-}
-
-static xmlDocPtr parse_xml(const char *xml_str) {
- xmlParserCtxtPtr pctxt;
- xmlDocPtr xml = NULL;
-
- /* Set up a parser context so we can catch the details of XML errors. */
- pctxt = xmlNewParserCtxt();
- if (pctxt == NULL || pctxt->sax == NULL)
- die("xmlNewParserCtxt failed");
-
- xml = xmlCtxtReadDoc (pctxt, BAD_CAST xml_str, "netcf.xml", NULL,
- XML_PARSE_NOENT | XML_PARSE_NONET |
- XML_PARSE_NOWARNING);
- if (xml == NULL)
- die("failed to parse xml document");
- if (xmlDocGetRootElement(xml) == NULL)
- die("missing root element");
-
- xmlFreeParserCtxt(pctxt);
- return xml;
-}
-
-static int xml_attrs_subset(xmlNodePtr n1, xmlNodePtr n2, char **err) {
- for (xmlAttrPtr a1 = n1->properties; a1 != NULL; a1 = a1->next) {
- xmlChar *v1 = xmlGetProp(n1, a1->name);
- xmlChar *v2 = xmlGetProp(n2, a1->name);
- if (!xmlStrEqual(v1, v2)) {
- format_error(err, "Different values for attribute %s/@%s: %s != %s
(lines %d and %d)", n1->name, a1->name, v1, v2, n1->line, n2->line);
- return 0;
- }
- }
- return 1;
-}
-
-static xmlNodePtr xml_next_element(xmlNodePtr n) {
- while (n != NULL && n->type != XML_ELEMENT_NODE)
- n = n->next;
- return n;
-}
-
-static int xml_nodes_equal(xmlNodePtr n1, xmlNodePtr n2, char **err) {
- if (n1 == NULL && n2 == NULL)
- return 1;
- if (n1 == NULL && n2 != NULL) {
- format_error(err, "First node null, second node %s (line %d)",
- n2->name, n2->line);
- return 0;
- }
- if (n1 != NULL && n2 == NULL) {
- format_error(err, "First node %s, second node null (line %d)",
- n1->name, n1->line);
- return 0;
- }
- if (!xmlStrEqual(n1->name, n2->name)) {
- format_error(err, "Different node names: %s != %s (lines %d and %d)",
- n1->name, n2->name, n1->line, n2->line);
- return 0;
- }
- if (! xml_attrs_subset(n1, n2, err))
- return 0;
- if (! xml_attrs_subset(n2, n1, err))
- return 0;
-
- n1 = xml_next_element(n1->children);
- n2 = xml_next_element(n2->children);
- while (n1 != NULL && n2 != NULL) {
- if (! xml_nodes_equal(n1, n2, err))
- return 0;
- n1 = xml_next_element(n1->next);
- n2 = xml_next_element(n2->next);
- }
- if (n1 != NULL) {
- format_error(err, "Additional element %s (line %d)",
- n1->name, n1->line);
- return 0;
- } else if (n2 != NULL) {
- format_error(err, "Additional element %s (line %d)",
- n2->name, n2->line);
- return 0;
- }
- return 1;
-}
-
-static void assert_xml_equals(CuTest *tc, const char *fname,
- char *exp, char *act) {
- char *err, *msg;
- xmlDocPtr exp_doc, act_doc;
- int result;
-
- exp_doc = parse_xml(exp);
- act_doc = parse_xml(act);
- result = xml_nodes_equal(xmlDocGetRootElement(exp_doc),
- xmlDocGetRootElement(act_doc), &err);
- xmlFreeDoc(exp_doc);
- xmlFreeDoc(act_doc);
-
- if (! result) {
- format_error(&msg, "%s: %s\nExpected XML:\n%s\nActual XML:\n%s\n",
- fname, err, exp, act);
- CuFail(tc, msg);
- }
-}
-
-static void run(CuTest *tc, const char *format, ...) {
- char *command;
- va_list args;
- int r;
-
- va_start(args, format);
- r = vasprintf(&command, format, args);
- va_end (args);
- if (r < 0)
- CuFail(tc, "Failed to format command (out of memory)");
- r = system(command);
- if (r < 0 || (WIFEXITED(r) && WEXITSTATUS(r) != 0)) {
- char *msg;
- r = asprintf(&msg, "Command %s failed with status %d\n",
- command, WEXITSTATUS(r));
- CuFail(tc, msg);
- free(msg);
- }
-}
-
-static void setup(CuTest *tc) {
- int r;
-
- if (asprintf(&root, "%s/build/test_initscripts/%s",
- abs_top_builddir, tc->name) < 0) {
- CuFail(tc, "failed to set root");
- }
-
- run(tc, "test -d %s && chmod -R u+w %s || :", root, root);
- run(tc, "rm -rf %s", root);
- run(tc, "mkdir -p %s", root);
- run(tc, "cp -pr %s/* %s", src_root, root);
- run(tc, "chmod -R u+w %s", root);
- run(tc, "chmod -R a-w %s/sys", root);
-
- r = ncf_init(&ncf, root);
- CuAssertIntEquals(tc, 0, r);
-}
-
-static void teardown(ATTRIBUTE_UNUSED CuTest *tc) {
- ncf_close(ncf);
- free(root);
- root = NULL;
-}
+extern const char *abs_top_srcdir;
+extern const char *abs_top_builddir;
+extern char *driver_name;
+extern char *root, *src_root;
+extern struct netcf *ncf;
static void testListInterfaces(CuTest *tc) {
int nint;
@@ -345,6 +167,11 @@ int main(void) {
die("failed to set src_root");
}
+ driver_name = strdup("initscripts");
+ if (driver_name == NULL) {
+ die("failed to set driver name");
+ }
+
CuSuiteSetup(suite, setup, teardown);
SUITE_ADD_TEST(suite, testListInterfaces);
@@ -358,6 +185,7 @@ int main(void) {
CuSuiteDetails(suite, &output);
printf("%s\n", output);
free(output);
+ free(driver_name);
return suite->failCount;
}
@@ -369,3 +197,4 @@ int main(void) {
* tab-width: 4
* End:
*/
+/* vim: set ts=4 sw=4 et: */
diff --git a/tests/tutil.c b/tests/tutil.c
new file mode 100644
index 0000000..777b2f4
--- /dev/null
+++ b/tests/tutil.c
@@ -0,0 +1,223 @@
+/*
+ * tutil.c: Global utility functions for automated testing of netcf
+ *
+ * Copyright (C) 2009 Red Hat Inc.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
+ *
+ * Author: David Lutterkort <lutter(a)redhat.com>
+ */
+
+#include <config.h>
+#include <stdio.h>
+#include <libxml/tree.h>
+
+#include "internal.h"
+#include "cutest.h"
+#include "safe-alloc.h"
+#include "read-file.h"
+
+#include "tutil.h"
+
+const char *abs_top_srcdir;
+const char *abs_top_builddir;
+char *driver_name = NULL;
+char *root, *src_root;
+struct netcf *ncf;
+
+void format_error(char **strp, const char *format, ...) {
+ va_list args;
+ int r;
+
+ va_start (args, format);
+ r = vasprintf (strp, format, args);
+ va_end (args);
+ if (r < 0)
+ die("Failed to format error message (out of memory)");
+}
+
+char *read_test_file(CuTest *tc, const char *relpath) {
+ char *path = NULL;
+ char *txt = NULL;
+ size_t length;
+
+ if (asprintf(&path, "%s/tests/%s", abs_top_srcdir, relpath) < 0)
+ CuFail(tc, "Could not format path for file");
+ txt = read_file(path, &length);
+ if (txt == NULL)
+ CuFail(tc, "Failed to read file");
+ return txt;
+}
+
+void assert_ncf_no_error(CuTest *tc) {
+ CuAssertIntEquals(tc, NETCF_NOERROR, ncf_error(ncf, NULL, NULL));
+}
+
+xmlDocPtr parse_xml(const char *xml_str) {
+ xmlParserCtxtPtr pctxt;
+ xmlDocPtr xml = NULL;
+
+ /* Set up a parser context so we can catch the details of XML errors. */
+ pctxt = xmlNewParserCtxt();
+ if (pctxt == NULL || pctxt->sax == NULL)
+ die("xmlNewParserCtxt failed");
+
+ xml = xmlCtxtReadDoc (pctxt, BAD_CAST xml_str, "netcf.xml", NULL,
+ XML_PARSE_NOENT | XML_PARSE_NONET |
+ XML_PARSE_NOWARNING);
+ if (xml == NULL)
+ die("failed to parse xml document");
+ if (xmlDocGetRootElement(xml) == NULL)
+ die("missing root element");
+
+ xmlFreeParserCtxt(pctxt);
+ return xml;
+}
+
+int xml_attrs_subset(xmlNodePtr n1, xmlNodePtr n2, char **err) {
+ for (xmlAttrPtr a1 = n1->properties; a1 != NULL; a1 = a1->next) {
+ xmlChar *v1 = xmlGetProp(n1, a1->name);
+ xmlChar *v2 = xmlGetProp(n2, a1->name);
+ if (!xmlStrEqual(v1, v2)) {
+ format_error(err, "Different values for attribute %s/@%s: %s != %s
(lines %d and %d)", n1->name, a1->name, v1, v2, n1->line, n2->line);
+ return 0;
+ }
+ }
+ return 1;
+}
+
+xmlNodePtr xml_next_element(xmlNodePtr n) {
+ while (n != NULL && n->type != XML_ELEMENT_NODE)
+ n = n->next;
+ return n;
+}
+
+int xml_nodes_equal(xmlNodePtr n1, xmlNodePtr n2, char **err) {
+ if (n1 == NULL && n2 == NULL)
+ return 1;
+ if (n1 == NULL && n2 != NULL) {
+ format_error(err, "First node null, second node %s (line %d)",
+ n2->name, n2->line);
+ return 0;
+ }
+ if (n1 != NULL && n2 == NULL) {
+ format_error(err, "First node %s, second node null (line %d)",
+ n1->name, n1->line);
+ return 0;
+ }
+ if (!xmlStrEqual(n1->name, n2->name)) {
+ format_error(err, "Different node names: %s != %s (lines %d and %d)",
+ n1->name, n2->name, n1->line, n2->line);
+ return 0;
+ }
+ if (! xml_attrs_subset(n1, n2, err))
+ return 0;
+ if (! xml_attrs_subset(n2, n1, err))
+ return 0;
+
+ n1 = xml_next_element(n1->children);
+ n2 = xml_next_element(n2->children);
+ while (n1 != NULL && n2 != NULL) {
+ if (! xml_nodes_equal(n1, n2, err))
+ return 0;
+ n1 = xml_next_element(n1->next);
+ n2 = xml_next_element(n2->next);
+ }
+ if (n1 != NULL) {
+ format_error(err, "Additional element %s (line %d)",
+ n1->name, n1->line);
+ return 0;
+ } else if (n2 != NULL) {
+ format_error(err, "Additional element %s (line %d)",
+ n2->name, n2->line);
+ return 0;
+ }
+ return 1;
+}
+
+void assert_xml_equals(CuTest *tc, const char *fname,
+ char *exp, char *act) {
+ char *err, *msg;
+ xmlDocPtr exp_doc, act_doc;
+ int result;
+
+ exp_doc = parse_xml(exp);
+ act_doc = parse_xml(act);
+ result = xml_nodes_equal(xmlDocGetRootElement(exp_doc),
+ xmlDocGetRootElement(act_doc), &err);
+ xmlFreeDoc(exp_doc);
+ xmlFreeDoc(act_doc);
+
+ if (! result) {
+ format_error(&msg, "%s: %s\nExpected XML:\n%s\nActual XML:\n%s\n",
+ fname, err, exp, act);
+ CuFail(tc, msg);
+ }
+}
+
+void run(CuTest *tc, const char *format, ...) {
+ char *command;
+ va_list args;
+ int r;
+
+ va_start(args, format);
+ r = vasprintf(&command, format, args);
+ va_end (args);
+ if (r < 0)
+ CuFail(tc, "Failed to format command (out of memory)");
+ r = system(command);
+ if (r < 0 || (WIFEXITED(r) && WEXITSTATUS(r) != 0)) {
+ char *msg;
+ r = asprintf(&msg, "Command %s failed with status %d\n",
+ command, WEXITSTATUS(r));
+ CuFail(tc, msg);
+ free(msg);
+ }
+}
+
+void setup(CuTest *tc) {
+ int r;
+
+ if (asprintf(&root, "%s/build/test_%s/%s",
+ abs_top_builddir, driver_name, tc->name) < 0) {
+ CuFail(tc, "failed to set root");
+ }
+
+ run(tc, "test -d %s && chmod -R u+w %s || :", root, root);
+ run(tc, "rm -rf %s", root);
+ run(tc, "mkdir -p %s", root);
+ run(tc, "cp -pr %s/* %s", src_root, root);
+ run(tc, "chmod -R u+w %s", root);
+ run(tc, "chmod -R a-w %s/sys", root);
+
+ r = ncf_init(&ncf, root);
+ CuAssertIntEquals(tc, 0, r);
+}
+
+void teardown(ATTRIBUTE_UNUSED CuTest *tc) {
+ ncf_close(ncf);
+ free(root);
+ root = NULL;
+}
+
+/*
+ * Local variables:
+ * indent-tabs-mode: nil
+ * c-indent-level: 4
+ * c-basic-offset: 4
+ * tab-width: 4
+ * End:
+ */
+/* vim: set ts=4 sw=4 et: */
diff --git a/tests/tutil.h b/tests/tutil.h
new file mode 100644
index 0000000..035f829
--- /dev/null
+++ b/tests/tutil.h
@@ -0,0 +1,87 @@
+/*
+ * tutil.h: Global utility functions for automated testing of netcf
+ *
+ * Copyright (C) 2009 Red Hat Inc.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
+ *
+ * Author: David Lutterkort <lutter(a)redhat.com>
+ */
+
+#ifndef _TUTIL_H
+#define _TUTIL_H
+
+#include <config.h>
+#include <libxml/tree.h>
+
+#include "internal.h"
+#include "cutest.h"
+#include "safe-alloc.h"
+
+#include "tutil.h"
+
+#define die(msg) \
+ do { \
+ fprintf(stderr, "%s:%d: Fatal error: %s\n", __FILE__, __LINE__, msg);
\
+ exit(EXIT_FAILURE); \
+ } while(0)
+
+/* Write a string of FORMAT, ... to STRP */
+void format_error(char **strp, const char *format, ...);
+
+/* Read data from file, return pointer to result */
+char *read_test_file(CuTest *tc, const char *relpath);
+
+/* Assert that the ncf instance is in the NOERROR-state */
+void assert_ncf_no_error(CuTest *tc);
+
+/* Parse the xml in XML_STR and return a xmlDocPtr to the parsed structure */
+xmlDocPtr parse_xml(const char *xml_str);
+
+/* Check that all attributes of N1 and N2 are equal */
+int xml_attrs_subset(xmlNodePtr n1, xmlNodePtr n2, char **err);
+
+/* Get the next element of type XML_ELEMENT_NODE following N, or NULL */
+xmlNodePtr xml_next_element(xmlNodePtr n);
+
+/* Check that the nodes N1 and N2 are alike (including their children) */
+int xml_nodes_equal(xmlNodePtr n1, xmlNodePtr n2, char **err);
+
+/* Assert the the EXP (expected) and ACT (actual) XML text blobs are alike.
+ * FNAME is used for error reporting in the case that they are not alike. */
+void assert_xml_equals(CuTest *tc, const char *fname,
+ char *exp, char *act);
+
+/* Run command formed by FORMAT, raise an error if the the command did not
+ * exit with status 0 */
+void run(CuTest *tc, const char *format, ...);
+
+/* Setup test directories */
+void setup(CuTest *tc);
+
+/* Close and free all resources */
+void teardown(ATTRIBUTE_UNUSED CuTest *tc);
+
+#endif
+
+/*
+ * Local variables:
+ * indent-tabs-mode: nil
+ * c-indent-level: 4
+ * c-basic-offset: 4
+ * tab-width: 4
+ * End:
+ */
+/* vim: set ts=4 sw=4 et: */
--
1.6.2
7:37 a.m.
New subject: [PATCH 5/7] Moving bonding option-handling outside of initscripts-*.xsl
initscripts-get.xsl: Creation of
/forest/tree/node[@label = 'BONDING_OPTS']/@value moved to
util-get.xsl.
initscripts-put.xsl: Creation of /interface/bond-tree moved to
util-put.xsl.
Added util-get.xsl and util-put.xsl contaning bonding opts-templates.
---
data/xml/initscripts-get.xsl | 22 +++-------------------
data/xml/initscripts-put.xsl | 36 +++++-------------------------------
data/xml/util-get.xsl | 27 +++++++++++++++++++++++++++
data/xml/util-put.xsl | 41 +++++++++++++++++++++++++++++++++++++++++
4 files changed, 76 insertions(+), 50 deletions(-)
create mode 100644 data/xml/util-get.xsl
create mode 100644 data/xml/util-put.xsl
diff --git a/data/xml/initscripts-get.xsl b/data/xml/initscripts-get.xsl
index b2a9020..46e39b1 100644
--- a/data/xml/initscripts-get.xsl
+++ b/data/xml/initscripts-get.xsl
@@ -4,6 +4,8 @@
extension-element-prefixes="ipcalc"
version="1.0">
+ <xsl:import href="util-get.xsl"/>
+
<xsl:output method="xml" indent="yes"/>
<xsl:template match="/">
@@ -98,25 +100,7 @@
<xsl:call-template name="interface-addressing"/>
<node label="BONDING_OPTS">
<xsl:attribute name="value">
- <xsl:text>'</xsl:text>
- <xsl:if test="bond/@mode">mode=<xsl:value-of
select='bond/@mode'/></xsl:if>
- <xsl:if test="bond/@mode = 'active-backup'">
primary=<xsl:value-of select='bond/interface[1]/@name'/></xsl:if>
- <xsl:if test="bond/miimon">
- <xsl:text> miimon=</xsl:text><xsl:value-of
select='bond/miimon/@freq'/>
- <xsl:if test="bond/miimon/@downdelay"><xsl:text>
downdelay=</xsl:text><xsl:value-of
select="bond/miimon/@downdelay"/></xsl:if>
- <xsl:if test="bond/miimon/@updelay"><xsl:text>
updelay=</xsl:text><xsl:value-of
select="bond/miimon/@updelay"/></xsl:if>
- <xsl:if test="bond/miimon/@carrier">
- <xsl:text> use_carrier=</xsl:text>
- <xsl:if test="bond/miimon/@carrier =
'ioctl'">0</xsl:if>
- <xsl:if test="bond/miimon/@carrier =
'netif'">1</xsl:if>
- </xsl:if>
- </xsl:if>
- <xsl:if test="bond/arpmon">
- <xsl:text> arp_interval=</xsl:text><xsl:value-of
select="bond/arpmon/@interval"/>
- <xsl:text> arp_ip_target=</xsl:text><xsl:value-of
select="bond/arpmon/@target"/>
- <xsl:if test="bond/arpmon/@validate"><xsl:text>
arp_validate=</xsl:text><xsl:value-of
select="bond/arpmon/@validate"/></xsl:if>
- </xsl:if>
- <xsl:text>'</xsl:text>
+ <xsl:call-template name="bonding-opts"/>
</xsl:attribute>
</node>
</tree>
diff --git a/data/xml/initscripts-put.xsl b/data/xml/initscripts-put.xsl
index 8df15c7..886a265 100644
--- a/data/xml/initscripts-put.xsl
+++ b/data/xml/initscripts-put.xsl
@@ -5,6 +5,8 @@
extension-element-prefixes="bond ipcalc"
version="1.0">
+ <xsl:import href="util-put.xsl"/>
+
<xsl:output method="xml" indent="yes"/>
<xsl:strip-space elements="*"/>
@@ -104,38 +106,10 @@
<xsl:call-template name="interface-addressing"/>
<bond>
<xsl:variable name="opts" select="node[@label =
'BONDING_OPTS']/@value"/>
- <xsl:variable name="mode" select="bond:option($opts,
'mode')"/>
+ <xsl:call-template name="bonding-opts">
+ <xsl:with-param name="opts" select="$opts"/>
+ </xsl:call-template>
<xsl:variable name="primary" select="bond:option($opts,
'primary')"/>
- <xsl:if test="string-length($mode) > 0">
- <xsl:attribute name="mode">
- <xsl:value-of select="$mode"/>
- </xsl:attribute>
- </xsl:if>
- <xsl:if test="bond:option($opts, 'miimon')">
- <miimon freq="{bond:option($opts, 'miimon')}">
- <xsl:if test="bond:option($opts,
'downdelay')"><xsl:attribute
name="downdelay"><xsl:value-of select="bond:option($opts,
'downdelay')"/></xsl:attribute></xsl:if>
- <xsl:if test="bond:option($opts,
'updelay')"><xsl:attribute name="updelay"><xsl:value-of
select="bond:option($opts,
'updelay')"/></xsl:attribute></xsl:if>
- <xsl:if test="bond:option($opts, 'use_carrier')">
- <xsl:attribute name="carrier">
- <xsl:if test="bond:option($opts, 'use_carrier') =
'0'">ioctl</xsl:if>
- <xsl:if test="bond:option($opts, 'use_carrier') =
'1'">netif</xsl:if>
- </xsl:attribute>
- </xsl:if>
- </miimon>
- </xsl:if>
- <xsl:if test="bond:option($opts, 'arp_interval')">
- <arpmon interval="{bond:option($opts, 'arp_interval')}"
target="{bond:option($opts, 'arp_ip_target')}">
- <xsl:variable name="val" select="bond:option($opts,
'arp_validate')"/>
- <xsl:if test="$val">
- <xsl:attribute name="validate">
- <xsl:if test="$val = 'none' or $val =
'0'">none</xsl:if>
- <xsl:if test="$val = 'active' or $val =
'1'">active</xsl:if>
- <xsl:if test="$val = 'backup' or $val =
'2'">backup</xsl:if>
- <xsl:if test="$val = 'all' or $val =
'3'">all</xsl:if>
- </xsl:attribute>
- </xsl:if>
- </arpmon>
- </xsl:if>
<xsl:for-each select="/descendant-or-self::*[node[@label =
'MASTER' and @value = $iface]][node[@label = 'DEVICE' and @value =
$primary]]">
<xsl:call-template name='bare-ethernet-interface'/>
</xsl:for-each>
diff --git a/data/xml/util-get.xsl b/data/xml/util-get.xsl
new file mode 100644
index 0000000..f3984ee
--- /dev/null
+++ b/data/xml/util-get.xsl
@@ -0,0 +1,27 @@
+<?xml version="1.0"?>
+<xsl:stylesheet xmlns:xsl="http://www.w3.org/1999/XSL/Transform"
+ version="1.0">
+
+ <xsl:template name="bonding-opts">
+ <xsl:text>'</xsl:text>
+ <xsl:if test="bond/@mode">mode=<xsl:value-of
select='bond/@mode'/></xsl:if>
+ <xsl:if test="bond/@mode = 'active-backup'">
primary=<xsl:value-of select='bond/interface[1]/@name'/></xsl:if>
+ <xsl:if test="bond/miimon">
+ <xsl:text> miimon=</xsl:text><xsl:value-of
select='bond/miimon/@freq'/>
+ <xsl:if test="bond/miimon/@downdelay"><xsl:text>
downdelay=</xsl:text><xsl:value-of
select="bond/miimon/@downdelay"/></xsl:if>
+ <xsl:if test="bond/miimon/@updelay"><xsl:text>
updelay=</xsl:text><xsl:value-of
select="bond/miimon/@updelay"/></xsl:if>
+ <xsl:if test="bond/miimon/@carrier">
+ <xsl:text> use_carrier=</xsl:text>
+ <xsl:if test="bond/miimon/@carrier =
'ioctl'">0</xsl:if>
+ <xsl:if test="bond/miimon/@carrier =
'netif'">1</xsl:if>
+ </xsl:if>
+ </xsl:if>
+ <xsl:if test="bond/arpmon">
+ <xsl:text> arp_interval=</xsl:text><xsl:value-of
select="bond/arpmon/@interval"/>
+ <xsl:text> arp_ip_target=</xsl:text><xsl:value-of
select="bond/arpmon/@target"/>
+ <xsl:if test="bond/arpmon/@validate"><xsl:text>
arp_validate=</xsl:text><xsl:value-of
select="bond/arpmon/@validate"/></xsl:if>
+ </xsl:if>
+ <xsl:text>'</xsl:text>
+ </xsl:template>
+
+</xsl:stylesheet>
diff --git a/data/xml/util-put.xsl b/data/xml/util-put.xsl
new file mode 100644
index 0000000..c1cd0b6
--- /dev/null
+++ b/data/xml/util-put.xsl
@@ -0,0 +1,41 @@
+<?xml version="1.0"?>
+<xsl:stylesheet xmlns:xsl="http://www.w3.org/1999/XSL/Transform"
+ xmlns:bond = "http://redhat.com/xslt/netcf/bond/1.0"
+ extension-element-prefixes="bond"
+ version="1.0">
+
+ <xsl:template name="bonding-opts">
+ <xsl:param name="opts" />
+ <xsl:variable name="mode" select="bond:option($opts,
'mode')"/>
+ <xsl:if test="string-length($mode) > 0">
+ <xsl:attribute name="mode">
+ <xsl:value-of select="$mode"/>
+ </xsl:attribute>
+ </xsl:if>
+ <xsl:if test="bond:option($opts, 'miimon')">
+ <miimon freq="{bond:option($opts, 'miimon')}">
+ <xsl:if test="bond:option($opts,
'downdelay')"><xsl:attribute
name="downdelay"><xsl:value-of select="bond:option($opts,
'downdelay')"/></xsl:attribute></xsl:if>
+ <xsl:if test="bond:option($opts,
'updelay')"><xsl:attribute name="updelay"><xsl:value-of
select="bond:option($opts,
'updelay')"/></xsl:attribute></xsl:if>
+ <xsl:if test="bond:option($opts, 'use_carrier')">
+ <xsl:attribute name="carrier">
+ <xsl:if test="bond:option($opts, 'use_carrier') =
'0'">ioctl</xsl:if>
+ <xsl:if test="bond:option($opts, 'use_carrier') =
'1'">netif</xsl:if>
+ </xsl:attribute>
+ </xsl:if>
+ </miimon>
+ </xsl:if>
+ <xsl:if test="bond:option($opts, 'arp_interval')">
+ <arpmon interval="{bond:option($opts, 'arp_interval')}"
target="{bond:option($opts, 'arp_ip_target')}">
+ <xsl:variable name="val" select="bond:option($opts,
'arp_validate')"/>
+ <xsl:if test="$val">
+ <xsl:attribute name="validate">
+ <xsl:if test="$val = 'none' or $val =
'0'">none</xsl:if>
+ <xsl:if test="$val = 'active' or $val =
'1'">active</xsl:if>
+ <xsl:if test="$val = 'backup' or $val =
'2'">backup</xsl:if>
+ <xsl:if test="$val = 'all' or $val =
'3'">all</xsl:if>
+ </xsl:attribute>
+ </xsl:if>
+ </arpmon>
+ </xsl:if>
+ </xsl:template>
+</xsl:stylesheet>
--
1.6.2
7:37 a.m.
New subject: [PATCH 6/7] Use a copy of shellvars.aug instead of the one in augeas
This is the first in a two-stage commit where we move from
the standard shellvars-lens to one that strips the single
quotes before and after strings. data/lenses/ifcfg.aug is
currently only a copy of the augeas lens, except for the
lens name.
src/drv_initscripts.c: Changed Shellvars.lns to Ifcfg.lns in xfm
Added data/lenses/ifcfg.aug
---
data/lenses/ifcfg.aug | 107 +++++++++++++++++++++++++++++++++++++++++++++++++
src/drv_initscripts.c | 2 +-
2 files changed, 108 insertions(+), 1 deletions(-)
create mode 100644 data/lenses/ifcfg.aug
diff --git a/data/lenses/ifcfg.aug b/data/lenses/ifcfg.aug
new file mode 100644
index 0000000..c1a82dd
--- /dev/null
+++ b/data/lenses/ifcfg.aug
@@ -0,0 +1,107 @@
+(* Generic lens for shell-script config files like the ones found *)
+(* in /etc/sysconfig *)
+module Ifcfg =
+ autoload xfm
+
+ let eol = Util.eol
+
+ let key_re = /[A-Za-z0-9_]+(\[[0-9]+\])?/ - "unset" - "export"
+ let eq = Util.del_str "="
+ let comment = Util.comment
+ let empty = Util.empty
+
+ let char = /[^() '"\t\n]|\\\\"/
+ let dquot = /"([^"\\\n]|\\\\.)*"/ (* " Emacs,
relax *)
+ let squot = /'[^'\n]*'/
+ let bquot = /`[^`\n]*`/
+
+ (* Array values of the form '(val1 val2 val3)'. We do not handle empty *)
+ (* arrays here because of typechecking headaches. Instead, they are *)
+ (* treated as a simple value *)
+ let array =
+ let array_value = store (char+ | dquot) in
+ del "(" "(" . counter "values" .
+ [ seq "values" . array_value ] .
+ [ del /[ \t]+/ " " . seq "values" . array_value ] *
+ . del ")" ")"
+
+ (* Treat an empty list () as a value '()'; that's not quite correct *)
+ (* but fairly close. *)
+ let simple_value =
+ let empty_array = /\([ \t]*\)/ in
+ store (char* | dquot | squot | bquot | empty_array)
+
+ let export = [ key "export" . Util.del_ws_spc ]
+ let kv = [ export? . key key_re . eq . (simple_value | array) . eol ]
+
+ let unset = [ key "unset" . Util.del_ws_spc . store key_re . eol ]
+
+ let source =
+ [
+ del /\.|source/ "." . label ".source" .
+ Util.del_ws_spc . store /[^= \t\n]+/ . eol
+ ]
+
+ let lns = (comment | empty | source | kv | unset) *
+
+ let sc_incl (n:string) = (incl ("/etc/sysconfig/" . n))
+ let filter_sysconfig =
+ sc_incl "atd" .
+ sc_incl "authconfig" .
+ sc_incl "autofs" .
+ sc_incl "clock" .
+ sc_incl "cpuspeed" .
+ sc_incl "crond" .
+ sc_incl "crontab" .
+ sc_incl "desktop" .
+ sc_incl "firstboot" .
+ sc_incl "grub" .
+ sc_incl "hsqldb" .
+ sc_incl "httpd" .
+ sc_incl "i18n" .
+ sc_incl "init" .
+ sc_incl "iptables-config" .
+ sc_incl "irda" .
+ sc_incl "irqbalance" .
+ sc_incl "kdump" .
+ sc_incl "kernel" .
+ sc_incl "keyboard" .
+ sc_incl "kudzu" .
+ sc_incl "libvirtd" .
+ sc_incl "lircd" .
+ sc_incl "nasd" .
+ sc_incl "netconsole" .
+ sc_incl "network" .
+ sc_incl "nfs" .
+ sc_incl "ntpd" .
+ sc_incl "prelink" .
+ sc_incl "readonly-root" .
+ sc_incl "rsyslog" .
+ sc_incl "samba" .
+ sc_incl "saslauthd" .
+ sc_incl "selinux" .
+ sc_incl "sendmail" .
+ sc_incl "smartmontools" .
+ sc_incl "spamassassin" .
+ sc_incl "sysstat" .
+ sc_incl "system-config-users" .
+ sc_incl "vncservers" .
+ sc_incl "wpa_supplicant" .
+ sc_incl "xend" .
+ sc_incl "xendomains"
+
+ let filter_ifcfg = incl "/etc/sysconfig/network-scripts/ifcfg-*"
+ let filter_default = incl "/etc/default/*"
+ let filter_misc = incl "/etc/arno-iptables-firewall/debconf.cfg"
+ . incl "/etc/cron-apt/config"
+ let filter = filter_sysconfig
+ . filter_ifcfg
+ . filter_default
+ . filter_misc
+ . Util.stdexcl
+
+ let xfm = transform lns filter
+
+(* Local Variables: *)
+(* mode: caml *)
+(* End: *)
diff --git a/src/drv_initscripts.c b/src/drv_initscripts.c
index 550df65..3fcf5cb 100644
--- a/src/drv_initscripts.c
+++ b/src/drv_initscripts.c
@@ -50,7 +50,7 @@ static const char *const ifcfg_path =
/* Augeas should only load the files we are interested in */
static const struct augeas_pv augeas_xfm[] = {
/* Ifcfg files */
- { "/augeas/load/Ifcfg/lens", "Shellvars.lns" },
+ { "/augeas/load/Ifcfg/lens", "Ifcfg.lns" },
{ "/augeas/load/Ifcfg/incl",
"/etc/sysconfig/network-scripts/ifcfg-*" },
{ "/augeas/load/Ifcfg/excl[1]", "*.augnew" },
--
1.6.2
7:37 a.m.
New subject: [PATCH 7/7] Let the lens handle single quotes instead of the XSLT and XSLT extensions
This is the second in a two-stage commit where we move from
the standard shellvars-lens to one that strips the single
quotes before and after strings.
data/lenses/ifcfg.aug: Strip single (and double) quotes and
re-insert them after save.
data/xml/util-get.xsl: Remove insertion of single quotes in XSLT
tests/initscripts/bond*.xml: Remove single quotes from
intermediate xml
---
data/lenses/ifcfg.aug | 11 ++++++-----
data/xml/util-get.xsl | 2 --
tests/initscripts/bond-arp.xml | 2 +-
tests/initscripts/bond.xml | 2 +-
4 files changed, 8 insertions(+), 9 deletions(-)
diff --git a/data/lenses/ifcfg.aug b/data/lenses/ifcfg.aug
index c1a82dd..22ebce9 100644
--- a/data/lenses/ifcfg.aug
+++ b/data/lenses/ifcfg.aug
@@ -11,15 +11,16 @@ module Ifcfg =
let empty = Util.empty
let char = /[^() '"\t\n]|\\\\"/
- let dquot = /"([^"\\\n]|\\\\.)*"/ (* " Emacs,
relax *)
- let squot = /'[^'\n]*'/
- let bquot = /`[^`\n]*`/
+ let dquotc = /([^"\\\n]|\\\\.)*/ (* " Emacs, relax *)
+ let squotc = /[^'\n]*/
+ let dquots = del "\"" "\"" . store dquotc . del
"\"" "\""
+ let squots = del "'" "'" . store squotc . del
"'" "'"
(* Array values of the form '(val1 val2 val3)'. We do not handle empty *)
(* arrays here because of typechecking headaches. Instead, they are *)
(* treated as a simple value *)
let array =
- let array_value = store (char+ | dquot) in
+ let array_value = dquots | store char+ in
del "(" "(" . counter "values" .
[ seq "values" . array_value ] .
[ del /[ \t]+/ " " . seq "values" . array_value ] *
@@ -29,7 +30,7 @@ module Ifcfg =
(* but fairly close. *)
let simple_value =
let empty_array = /\([ \t]*\)/ in
- store (char* | dquot | squot | bquot | empty_array)
+ squots | dquots | store char* | store empty_array
let export = [ key "export" . Util.del_ws_spc ]
let kv = [ export? . key key_re . eq . (simple_value | array) . eol ]
diff --git a/data/xml/util-get.xsl b/data/xml/util-get.xsl
index f3984ee..0a1405d 100644
--- a/data/xml/util-get.xsl
+++ b/data/xml/util-get.xsl
@@ -3,7 +3,6 @@
version="1.0">
<xsl:template name="bonding-opts">
- <xsl:text>'</xsl:text>
<xsl:if test="bond/@mode">mode=<xsl:value-of
select='bond/@mode'/></xsl:if>
<xsl:if test="bond/@mode = 'active-backup'">
primary=<xsl:value-of select='bond/interface[1]/@name'/></xsl:if>
<xsl:if test="bond/miimon">
@@ -21,7 +20,6 @@
<xsl:text> arp_ip_target=</xsl:text><xsl:value-of
select="bond/arpmon/@target"/>
<xsl:if test="bond/arpmon/@validate"><xsl:text>
arp_validate=</xsl:text><xsl:value-of
select="bond/arpmon/@validate"/></xsl:if>
</xsl:if>
- <xsl:text>'</xsl:text>
</xsl:template>
</xsl:stylesheet>
diff --git a/tests/initscripts/bond-arp.xml b/tests/initscripts/bond-arp.xml
index 6a53d09..884cb84 100644
--- a/tests/initscripts/bond-arp.xml
+++ b/tests/initscripts/bond-arp.xml
@@ -7,7 +7,7 @@
<node label="IPADDR" value="192.168.50.7"/>
<node label="NETMASK" value="255.255.255.0"/>
<node label="GATEWAY" value="192.168.50.1"/>
- <node label="BONDING_OPTS" value="'mode=active-backup
primary=eth1 arp_interval=100 arp_ip_target=192.168.50.1
arp_validate=active'"/>
+ <node label="BONDING_OPTS" value="mode=active-backup primary=eth1
arp_interval=100 arp_ip_target=192.168.50.1 arp_validate=active"/>
</tree>
<tree path="/files/etc/sysconfig/network-scripts/ifcfg-eth1">
<node label="DEVICE" value="eth1"/>
diff --git a/tests/initscripts/bond.xml b/tests/initscripts/bond.xml
index d97542d..4a7e9bf 100644
--- a/tests/initscripts/bond.xml
+++ b/tests/initscripts/bond.xml
@@ -11,7 +11,7 @@
<node label="IPADDR" value="192.168.50.7"/>
<node label="NETMASK" value="255.255.255.0"/>
<node label="GATEWAY" value="192.168.50.1"/>
- <node label="BONDING_OPTS" value="'mode=active-backup
primary=eth1 miimon=100 updelay=10 use_carrier=0'"/>
+ <node label="BONDING_OPTS" value="mode=active-backup primary=eth1
miimon=100 updelay=10 use_carrier=0"/>
</tree>
<tree path="/files/etc/sysconfig/network-scripts/ifcfg-eth1">
<node label="DEVICE" value="eth1"/>
--
1.6.2
5:32 a.m.
New subject: [PATCH 2/7] Moving of modeprobe_(un)alias_bond
On Thu, 2009-07-23 at 14:37 +0200, Jonas Eriksson wrote:
dutil_linux.{c,h}: Added modprobed_alias_bond,
modprobed_unalias_bond
(earlier named modprobe_(un)alias_bond, renamed to point out that
they are using the modprobe.d file-tree)
drv_initscripts.c: Removed modprobe_alias_bond, modprobe_unalias_bond
ACK.
As a stylistic comment, for non-static functions, I prefer that the
comment about what they do is only in the .h file, and not in both
the .h and .c file, since the two will invariably fall out of sync.
David
5:55 a.m.
New subject: [PATCH 3/7] Add strlower, update dutil_linux.c:aug_match_mac
On Thu, 2009-07-23 at 14:37 +0200, Jonas Eriksson wrote:
dutil.{c,h}:
- Added strllower
dutil_linux.{c,h}:
- Made use of strllower and aug_fmt_match in aug_match_mac
Heh, I just broke that with the patch I committed. I'll rebase this and
the following patches so that the patch series still applies.
- Fixed a typo in the comment above the function head and
prototype
drv_initscruots.c:
- Updated the use of aug_match_mac
- Broke the long function head of drv_list_interfaces
Such formatting changes should generally go into a separate patch.
diff --git a/src/dutil.c b/src/dutil.c
index 28832b3..2066a45 100644
--- a/src/dutil.c
+++ b/src/dutil.c
@@ -62,6 +62,13 @@ int xasprintf(char **strp, const char *format, ...) {
return result;
}
+/* Convert string of max length buflen to uppercase in place */
+void strllower(char *s, unsigned int buflen) {
I find strnlower a more natural name for this, in parallel to other
functions that only look at a limited number of characters in strings
like strndup, strncmp etc.
+ if (s != NULL)
+ for (int i = 0; s[i] != '\0' && i < buflen; i++)
To stick with the parallel to strndup etc., instead of a buflen, the
caller should pass in the number n of characters to look at, so that the
termination condition will be s[i] != '\0' && i <= n.
+ s[i] = tolower(s[i]);
+}
+
Using tolower here is wrong, since tolower's notion of lower/uppercase
depends on the user's locale, whereas we want to always map ABCDEF ->
abcdef. That's why I used c_tolower from gnulib.
diff --git a/src/dutil_linux.c b/src/dutil_linux.c
index 4e99ef6..9727367 100644
--- a/src/dutil_linux.c
+++ b/src/dutil_linux.c
@@ -39,23 +39,20 @@
#include "dutil.h"
#include "dutil_linux.h"
-/* Returns a list of all interfaces with MAC address INTF */
+/* Returns a list of all interfaces with MAC address MAC */
int aug_match_mac(struct netcf *ncf, const char *mac, char ***matches) {
- int r, nmatches;
- char *path;
- struct augeas *aug = get_augeas(ncf);
+ int nmatches;
+ char *lcmac = NULL;
- r = xasprintf(&path,
- "/files/sys/class/net/*[address/content = '%s']", mac);
- ERR_COND_BAIL(r < 0, ncf, ENOMEM);
+ /* Make sure that the mac address is in lower case */
+ lcmac = strndup(mac, 17);
+ ERR_COND_BAIL(lcmac == NULL, ncf, ENOMEM);
+ strllower(lcmac, 18);
Why do you restrict the length of lcmac here ? I'd expect the caller
makes sure that MAC is a valid MAC address, and hardcoding the length of
MAC here gains nothing and might lead to headache in the future if we
ever deal with e.g. 64bit MACs
David
11:33 a.m.
New subject: [PATCH 3/7] Add strlower, update dutil_linux.c:aug_match_mac
On Wed, 2009-07-29 at 10:55 +0000, David Lutterkort wrote:
On Thu, 2009-07-23 at 14:37 +0200, Jonas Eriksson wrote:
> dutil.{c,h}:
> - Added strllower
>
> dutil_linux.{c,h}:
> - Made use of strllower and aug_fmt_match in aug_match_mac
Heh, I just broke that with the patch I committed. I'll rebase this and
the following patches so that the patch series still applies.
As we discussed on IRC, here's a remodeled patch; since I already
addressed all-lower case MAC addresses, I removed strllower from your
patch.
Ok to commit ?
David
From 269f944fb52aa77731657c7d7c97cd107cb09ab4 Mon Sep 17 00:00:00 2001
From: Jonas Eriksson <jonas.j.eriksson(a)ericsson.com>
Date: Thu, 23 Jul 2009 14:37:04 +0200
Subject: [PATCH] Several minor cleanup changes
* src/dutil_linux.c (aug_match_mac): use aug_fmt_match; fix a typo in the
comment above the function head and prototype
* src/drv_initscruots.c (drv_list_interfaces); reformat header
* src/drv_initscruots.c (drv_lookup_by_mac_string): use ERR_BAIL
to handle error from aug_match_mac
---
src/drv_initscripts.c | 5 +++--
src/dutil_linux.c | 19 +++++--------------
src/dutil_linux.h | 2 +-
3 files changed, 9 insertions(+), 17 deletions(-)
diff --git a/src/drv_initscripts.c b/src/drv_initscripts.c
index 0c69071..550df65 100644
--- a/src/drv_initscripts.c
+++ b/src/drv_initscripts.c
@@ -344,7 +344,8 @@ static int list_interface_ids(struct netcf *ncf,
return -1;
}
-int drv_list_interfaces(struct netcf *ncf, int maxnames, char **names, unsigned int
flags) {
+int drv_list_interfaces(struct netcf *ncf, int maxnames, char **names,
+ unsigned int flags) {
return list_interface_ids(ncf, maxnames, names, flags, "DEVICE");
}
@@ -677,7 +678,7 @@ int drv_lookup_by_mac_string(struct netcf *ncf, const char *mac,
ERR_BAIL(ncf);
nmatches = aug_match_mac(ncf, mac, &matches);
- ERR_THROW(nmatches < 0, ncf, EOTHER, "looking up %s failed", mac);
+ ERR_BAIL(ncf);
if (nmatches == 0) {
result = 0;
goto done;
diff --git a/src/dutil_linux.c b/src/dutil_linux.c
index cdb5915..864bced 100644
--- a/src/dutil_linux.c
+++ b/src/dutil_linux.c
@@ -40,29 +40,20 @@
#include "dutil.h"
#include "dutil_linux.h"
-/* Returns a list of all interfaces with MAC address INTF */
+/* Returns a list of all interfaces with MAC address MAC */
int aug_match_mac(struct netcf *ncf, const char *mac, char ***matches) {
- int r, nmatches;
- char *path, *mac_lower;
- struct augeas *aug = get_augeas(ncf);
+ int nmatches;
+ char *path = NULL, *mac_lower = NULL;
mac_lower = strdup(mac);
ERR_COND_BAIL(mac_lower == NULL, ncf, ENOMEM);
for (char *s = mac_lower; *s != '\0'; s++)
*s = c_tolower(*s);
- r = xasprintf(&path,
+ nmatches = aug_fmt_match(ncf, matches,
"/files/sys/class/net/*[address/content = '%s']",
mac_lower);
- FREE(mac_lower);
- ERR_COND_BAIL(r < 0, ncf, ENOMEM);
-
- r = aug_match(aug, path, matches);
- /* Messages for a aug_match-fail are handled outside this function */
- if (r < 0)
- goto error;
+ ERR_BAIL(ncf);
- nmatches = r;
- r = -1;
for (int i = 0; i < nmatches; i++) {
char *n = strrchr((*matches)[i], '/');
ERR_THROW(n == NULL, ncf, EINTERNAL, "missing / in sysfs path");
diff --git a/src/dutil_linux.h b/src/dutil_linux.h
index 0351b8b..085c2f9 100644
--- a/src/dutil_linux.h
+++ b/src/dutil_linux.h
@@ -23,7 +23,7 @@
#ifndef DUTIL_LINUX_H_
#define DUTIL_LINUX_H_
-/* Returns a list of all interfaces with MAC address INTF */
+/* Returns a list of all interfaces with MAC address MAC */
int aug_match_mac(struct netcf *ncf, const char *mac, char ***matches);
/* Get the MAC address of the interface INTF */
--
1.6.0.6
11:33 a.m.
New subject: [PATCH 4/7] Lift out test functions to tutil.{c, h}
On Thu, 2009-07-23 at 14:37 +0200, Jonas Eriksson wrote:
* tutil.{c,h}:
- Added format_error, read_test_file, assert_ncf_no_error,
parse_xml, xml_attrs_subset, xml_next_element, xml_nodes_equal,
assert_xml_equals, run, setup, teardown from test-initscripts.c
- Created identify_driver, in order for setup() to create a
(correctly named) root dir for the tests.
* test-initscripts.c:
- Removed functions named above
- Added call to identify_driver("initscripts") before starting tests
ACK
David
11:37 a.m.
New subject: [PATCH 5/7] Moving bonding option-handling outside of initscripts-*.xsl
On Thu, 2009-07-23 at 14:37 +0200, Jonas Eriksson wrote:
initscripts-get.xsl: Creation of
/forest/tree/node[@label = 'BONDING_OPTS']/@value moved to
util-get.xsl.
initscripts-put.xsl: Creation of /interface/bond-tree moved to
util-put.xsl.
Added util-get.xsl and util-put.xsl contaning bonding opts-templates.
ACK (one minor nit: the patch had trailing whitespace in util-get.xsl -
I fixed that up)
David
11:41 a.m.
New subject: [PATCH 6/7] Use a copy of shellvars.aug instead of the one in augeas
On Thu, 2009-07-23 at 14:37 +0200, Jonas Eriksson wrote:
This is the first in a two-stage commit where we move from
the standard shellvars-lens to one that strips the single
quotes before and after strings. data/lenses/ifcfg.aug is
currently only a copy of the augeas lens, except for the
lens name.
I don't understand why that is needed - what's the issue this
addresses ?
also, since netcf does not autoload any files, and sets up what gets
loaded manually, there's no need to have the filter etc. in the lens;
you can delete the 'autload xfm' at the top of the file and everything
from 'let sc_incl ...' to the end of the file.
David
11:51 a.m.
New subject: [PATCH 7/7] Let the lens handle single quotes instead of the XSLT and XSLT extensions
On Thu, 2009-07-23 at 14:37 +0200, Jonas Eriksson wrote:
This is the second in a two-stage commit where we move from
the standard shellvars-lens to one that strips the single
quotes before and after strings.
data/lenses/ifcfg.aug: Strip single (and double) quotes and
re-insert them after save.
This doesn't work properly - when you modify a lens, you should also run
it through 'augparse data/lenses/ifcfg.aug' to typecheck it, and the
modified lens produces a typechecking error (typechecking isn't done by
default since it can be _very_ slow)
The typecheck error from augparse is
data/lenses/ifcfg.aug:22.2-27.19:Failed to compile array
data/lenses/ifcfg.aug:23.22-.42:exception: overlapping lenses in tree union.put
Example matched by both: ''
First lens: data/lenses/ifcfg.aug:16.15-.59
Second lens: data/lenses/ifcfg.aug:23.31-.42
The issue is that Augeas can't decide how to write out variable values
when it goes from tree -> file. For a variable VAR with value VAL in the
tree, Augeas can legally write this as VAR=VAL, VAR='VAL', or VAR="VAL"
- and it has no way of knowing which one is correct. For example, if VAL
contains spaces, no quotes would be incorrect.
That's the reason why the shellvars lens doesn't strip quotes
automatically :) (Plus, in general, it matters when you look at an entry
in the tree whether it's enclosed in single or double quotes, and
there's no way to decide automatically which one is correct)
data/xml/util-get.xsl: Remove insertion of single quotes in XSLT
If this is just for simplification, I would just drop this patch and 6/7
- otherwise, what problem did you run into ?
David
12:50 a.m.
New subject: [PATCH 3/7] Add strlower, update dutil_linux.c:aug_match_mac
On Wed, Jul 29, 2009 at 06:33:11PM +0200 David Lutterkort wrote:
On Wed, 2009-07-29 at 10:55 +0000, David Lutterkort wrote:
> On Thu, 2009-07-23 at 14:37 +0200, Jonas Eriksson wrote:
> > dutil.{c,h}:
> > - Added strllower
> >
> > dutil_linux.{c,h}:
> > - Made use of strllower and aug_fmt_match in aug_match_mac
>
> Heh, I just broke that with the patch I committed. I'll rebase this and
> the following patches so that the patch series still applies.
As we discussed on IRC, here's a remodeled patch; since I already
addressed all-lower case MAC addresses, I removed strllower from your
patch.
Ok to commit ?
Yes, this patch seem to contain all things not related to
strlower (which should have been splitted from the beginning).
ACK!
/Jonas
--
Jonas Eriksson
Consultant at AS/EAB/FLJ/IL
Phone: +46 8 58086281
Combitech AB
Älvsjö, Sweden
1:08 a.m.
New subject: [PATCH 6/7] Use a copy of shellvars.aug instead of the one in augeas
On Wed, Jul 29, 2009 at 06:41:17PM +0200 David Lutterkort wrote:
On Thu, 2009-07-23 at 14:37 +0200, Jonas Eriksson wrote:
> This is the first in a two-stage commit where we move from
> the standard shellvars-lens to one that strips the single
> quotes before and after strings. data/lenses/ifcfg.aug is
> currently only a copy of the augeas lens, except for the
> lens name.
I don't understand why that is needed - what's the issue this
addresses ?
This is in respose to this mail:
Subject: Re: [netcf-devel] SuSE driver
From: David Lutterkort <lutter(a)redhat.com>
Message-Id: <1246058729.4572.113.camel(a)avon.watzmann.net>
..where the ' and " surrounding ifcfg-strings were discussed. In
the case of the initscripts driver, this seems to only be handled
for the BONDING_OPTS "variable". I however assume that the
configuration still is valid if the file contains:
DEVICE='eth0'
In SuSE (at least SLES), this is a big problem. The default
ifcfg-lo has all strings surrounded by '', and as aug_get returns
const char*:s, all these have to be strdup:ed and striped before
being usable. In order to avoid this in the C-code of the driver,
the most resonable solution seem to be to ship a '"-stripping
augeas lense.
also, since netcf does not autoload any files, and sets up what gets
loaded manually, there's no need to have the filter etc. in the lens;
you can delete the 'autload xfm' at the top of the file and everything
from 'let sc_incl ...' to the end of the file.
That's very true, although I would have done it in the next
patch.
/Jonas
--
Jonas Eriksson
Consultant at AS/EAB/FLJ/IL
Combitech AB
Älvsjö, Sweden
1:47 a.m.
New subject: [PATCH 7/7] Let the lens handle single quotes instead of the XSLT and XSLT extensions
On Wed, Jul 29, 2009 at 06:51:26PM +0200 David Lutterkort wrote:
On Thu, 2009-07-23 at 14:37 +0200, Jonas Eriksson wrote:
> This is the second in a two-stage commit where we move from
> the standard shellvars-lens to one that strips the single
> quotes before and after strings.
>
> data/lenses/ifcfg.aug: Strip single (and double) quotes and
> re-insert them after save.
This doesn't work properly - when you modify a lens, you should also run
it through 'augparse data/lenses/ifcfg.aug' to typecheck it, and the
modified lens produces a typechecking error (typechecking isn't done by
default since it can be _very_ slow)
The typecheck error from augparse is
data/lenses/ifcfg.aug:22.2-27.19:Failed to compile array
data/lenses/ifcfg.aug:23.22-.42:exception: overlapping lenses in tree union.put
Example matched by both: ''
First lens: data/lenses/ifcfg.aug:16.15-.59
Second lens: data/lenses/ifcfg.aug:23.31-.42
The issue is that Augeas can't decide how to write out variable values
when it goes from tree -> file. For a variable VAR with value VAL in the
tree, Augeas can legally write this as VAR=VAL, VAR='VAL', or
VAR="VAL"
- and it has no way of knowing which one is correct. For example, if VAL
contains spaces, no quotes would be incorrect.
Ok, this is not good. I can see that this is now touched in the
schema tutorial which I should have checked with before
submitting this patch (which was written a while back).
Are there any thoughts on letting augeas have some sort of
writeback-awareness that would let it react to this? Some sort of
value-filter perhaps (foo bar => 'foo bar', fo'o =>
"fo'o").
That's the reason why the shellvars lens doesn't strip
quotes
automatically :) (Plus, in general, it matters when you look at an entry
in the tree whether it's enclosed in single or double quotes, and
there's no way to decide automatically which one is correct)
For general shellvars I can understand this, but now we have a
very much simplified case which would let us cut corners to
gravely simplify the C-code.
> data/xml/util-get.xsl: Remove insertion of single quotes in
XSLT
If this is just for simplification, I would just drop this patch and 6/7
- otherwise, what problem did you run into ?
As said in 6/7, this needs to be handled for all variables, not
just the ones that can contain spaces. What I forgot to mention
in that mail is that I believe YaST (the default config tool for
SuSE) defaults to do the single-quoting on all ifcfg-values.
/Jonas
--
Jonas Eriksson
Consultant at AS/EAB/FLJ/IL
Phone: +46 8 58086281
Combitech AB
Älvsjö, Sweden
4 a.m.
On Thu, 2009-07-23 at 14:37 +0200, Jonas Eriksson wrote:
This is the last set of patches before the amount of shared code
between the SuSE-driver (in its current state, at least) and the
current initscripts driver is almost nil.
I've just pushed the first 5 patches of the series - for the problem of
quoting the shell variable values, we need to figure out a solution
seaprately.
David
4:02 a.m.
On Fri, Jul 31, 2009 at 11:00:07AM +0200 David Lutterkort wrote:
On Thu, 2009-07-23 at 14:37 +0200, Jonas Eriksson wrote:
> This is the last set of patches before the amount of shared code
> between the SuSE-driver (in its current state, at least) and the
> current initscripts driver is almost nil.
I've just pushed the first 5 patches of the series - for the problem of
quoting the shell variable values, we need to figure out a solution
seaprately.
Thanks!
Yes, it is obvious that the shellvars point needs more discussion
and pondering.
/Jonas
--
Jonas Eriksson
Consultant at AS/EAB/FLJ/IL
Combitech AB
Älvsjö, Sweden
7:37 p.m.
New subject: [PATCH 6/7] Use a copy of shellvars.aug instead of the one in augeas
On Thu, 2009-07-23 at 14:37 +0200, Jonas Eriksson wrote:
This is the first in a two-stage commit where we move from
the standard shellvars-lens to one that strips the single
quotes before and after strings. data/lenses/ifcfg.aug is
currently only a copy of the augeas lens, except for the
lens name.
To revive this old thread: augeas-0.5.3 has support that would let us
handle quoting within the lens as needed (assuming we can always use
single quotes when quotes are needed)
I'd also like to use a custom lens for ifcfg- files to split some of the
variables that are lists, like BONDING_OPTS, inside Augeas and simplify
the stylesheets etc. somewhat.
David
5359
days inactive
5415
days old
netcf-devel@lists.fedorahosted.org
21 comments
2 participants
participants (2)
-
David Lutterkort -
Jonas Eriksson