From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:40919) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YxHuy-00011i-5E for qemu-devel@nongnu.org; Tue, 26 May 2015 12:41:29 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YxHuo-0007fh-MD for qemu-devel@nongnu.org; Tue, 26 May 2015 12:41:27 -0400 Received: from mga14.intel.com ([192.55.52.115]:39790) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YxHuo-0007fO-G1 for qemu-devel@nongnu.org; Tue, 26 May 2015 12:41:18 -0400 Message-ID: <5564A224.20407@intel.com> Date: Tue, 26 May 2015 17:41:08 +0100 From: Ikey Doherty MIME-Version: 1.0 References: <55645794.3020202@redhat.com> <1432644847-7566-1-git-send-email-michael.i.doherty@intel.com> <1432644847-7566-2-git-send-email-michael.i.doherty@intel.com> <20150526163859.GK28075@thinpad.lan.raisama.net> In-Reply-To: <20150526163859.GK28075@thinpad.lan.raisama.net> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v3 2/2] qemu-bridge-helper: Use stateless configuration for bridge.conf List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eduardo Habkost Cc: qemu-devel@nongnu.org, Stefan Hajnoczi 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 >> --- >> 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? -- Clear Linux Project for Intel Architecture Intel Open Source Technology Center http://www.clearlinux.org