From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:39291) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YxbuJ-0004Dw-Ay for qemu-devel@nongnu.org; Wed, 27 May 2015 10:02:08 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YxbuG-0008Ml-5B for qemu-devel@nongnu.org; Wed, 27 May 2015 10:02:07 -0400 Received: from mail-wi0-x22d.google.com ([2a00:1450:400c:c05::22d]:33472) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YxbuF-0008MR-SR for qemu-devel@nongnu.org; Wed, 27 May 2015 10:02:04 -0400 Received: by wicmx19 with SMTP id mx19so94733840wic.0 for ; Wed, 27 May 2015 07:02:03 -0700 (PDT) Date: Wed, 27 May 2015 15:02:00 +0100 From: Stefan Hajnoczi Message-ID: <20150527140200.GB1454@stefanha-thinkpad.redhat.com> 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> <5564A224.20407@intel.com> <20150526165741.GM28075@thinpad.lan.raisama.net> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="R3G7APHDIzY6R/pk" Content-Disposition: inline In-Reply-To: <20150526165741.GM28075@thinpad.lan.raisama.net> 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: Ikey Doherty , Stefan Hajnoczi , qemu-devel@nongnu.org --R3G7APHDIzY6R/pk Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Tue, May 26, 2015 at 01:57:41PM -0300, Eduardo Habkost wrote: > 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 config= uration > > >>from system administrator configuration. With this change we will rea= d the > > >>default configuration data from /usr/share/defaults/qemu, in the abse= nce of > > >>an overriding site administrator configuration in /etc/qemu. > > >> > > >>A key advantage of this change is enabling a sane and immutable defau= lt OS > > >>configuration, that is resiliant to upgrades. Ultimate power is still= left > > >>to the system administrator, with the ability to override the default= s 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 =3D 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) =3D=3D -1) { > > >>- fprintf(stderr, "failed to parse default acl file `%s'\n", > > >>- DEFAULT_ACL_FILE); > > >>- ret =3D EXIT_FAILURE; > > >>- goto cleanup; > > >>+ if (parse_acl_file(SITE_ACL_FILE, &acl_list) =3D=3D -1) { > > >>+ if (parse_acl_file(DEFAULT_ACL_FILE, &acl_list) =3D=3D -1) { > > >>+ fprintf(stderr, "failed to parse default acl file `%s'\n= ", > > >>+ DEFAULT_ACL_FILE); > > >>+ ret =3D 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? > > > > >=20 > > I could stat the file and fallback to default, and only parse_acl_file > > the one that exists. Better? >=20 > Or you could simply call parse_acl_file(DEFAULT_ACL_FILE) only if > parse_acl_file(SITE_ACL_FILE) set errno=3DENOENT. Yes, please. Stefan --R3G7APHDIzY6R/pk Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQEcBAEBAgAGBQJVZc5YAAoJEJykq7OBq3PIIQEH/RdWFKTBrW+qIUyJnwxLomZU 3mtUHd798gITgmZgj6b+yCYHajpFDABf1a9fMlJAJ+3OK+6C+f+Dd4643/+J9TVO cv5XXBhOVV7ufkDzyhMBBXIoUimAZ5aDfovaymJYO34QdS2gLb4f6FdBTWOw8bsC iIPU+Nj5FjxV+uJywNykv+0zB8JhPn4nlhGlx9thteZCmiF9axkWdLLI59x+THo6 knVdsOJ7xUcURGDojP6Zns8MteHLhWd3I9l2zFD3fY/f44KCHBtqcntKlcUIfjOC qwvXeaSq/7EESSWXOY+zcIi7Dbv9kTVv05EN0+AoEy5rXF53OVUV+aCnHQX5gMw= =yWu5 -----END PGP SIGNATURE----- --R3G7APHDIzY6R/pk--