On Tue, May 26, 2015 at 05:41:08PM +0100, Ikey Doherty wrote:
> On 26/05/15 17:38, Eduardo Habkost wrote:
> >On Tue, May 26, 2015 at 01:54:07PM +0100, Ikey Doherty wrote:
> >>The goal of stateless, and thus this change, is to separate OS configuration
> >>from system administrator configuration. With this change we will read the
> >>default configuration data from /usr/share/defaults/qemu, in the absence of
> >>an overriding site administrator configuration in /etc/qemu.
> >>
> >>A key advantage of this change is enabling a sane and immutable default OS
> >>configuration, that is resiliant to upgrades. Ultimate power is still left
> >>to the system administrator, with the ability to override the defaults if
> >>required. Lastly, given that the sane defaults are always available, the
> >>administrator may simply remove their site-config files to reset the
> >>configuration to the "factory defaults" (i.e. OS configuration).
> >>
> >>Signed-off-by: Ikey Doherty <michael.i.dohe...@intel.com>
> >>---
> >> configure | 2 ++
> >> qemu-bridge-helper.c | 15 +++++++++------
> >> 2 files changed, 11 insertions(+), 6 deletions(-)
> >>
> >[...]
> >>-#define DEFAULT_ACL_FILE CONFIG_QEMU_CONFDIR "/bridge.conf"
> >>+#define DEFAULT_ACL_FILE CONFIG_QEMU_DEFAULTDIR "/bridge.conf"
> >>+#define SITE_ACL_FILE CONFIG_QEMU_CONFDIR "/bridge.conf"
> >>
> >> enum {
> >> ACL_ALLOW = 0,
> >>@@ -272,11 +273,13 @@ int main(int argc, char **argv)
> >>
> >> /* parse default acl file */
> >> QSIMPLEQ_INIT(&acl_list);
> >>- if (parse_acl_file(DEFAULT_ACL_FILE, &acl_list) == -1) {
> >>- fprintf(stderr, "failed to parse default acl file `%s'\n",
> >>- DEFAULT_ACL_FILE);
> >>- ret = EXIT_FAILURE;
> >>- goto cleanup;
> >>+ if (parse_acl_file(SITE_ACL_FILE, &acl_list) == -1) {
> >>+ if (parse_acl_file(DEFAULT_ACL_FILE, &acl_list) == -1) {
> >>+ fprintf(stderr, "failed to parse default acl file `%s'\n",
> >>+ DEFAULT_ACL_FILE);
> >>+ ret = EXIT_FAILURE;
> >>+ goto cleanup;
> >>+ }
> >> }
> >
> >This will make syntax errors on SITE_ACL_FILE cause partial loading of
> >the rules on SITE_ACL_FILE, and trigger loading of DEFAULT_ACL_FILE,
> >instead of aborting bridge-helper.
> >
> >Wouldn't it be better to fallback to DEFAULT_ACL_FILE if and only if
> >SITE_ACL_FILE is missing?
> >
>
> I could stat the file and fallback to default, and only parse_acl_file
> the one that exists. Better?
Or you could simply call parse_acl_file(DEFAULT_ACL_FILE) only if
parse_acl_file(SITE_ACL_FILE) set errno=ENOENT.
--
Eduardo
