| Message ID | 1447966722-18204-3-git-send-email-azhou@ovn.org |
|---|---|
| State | Changes Requested |
| Headers | show |
On Thu, Nov 19, 2015 at 12:58:38PM -0800, Andy Zhou wrote: > Change ovsdb file ownership to match "$OVS_USER":"$OVS_GROUP" when > we either create it for the first time, or upgrade it. > > Signed-off-by: Andy Zhou <azhou@ovn.org> > --- > utilities/ovs-lib.in | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/utilities/ovs-lib.in b/utilities/ovs-lib.in > index 34e2041..3fbc2f5 100644 > --- a/utilities/ovs-lib.in > +++ b/utilities/ovs-lib.in > @@ -376,4 +376,8 @@ upgrade_db () { > create_db "$DB_FILE" "$DB_SCHEMA" > fi > fi > + > + # Make sure we change the ownership of related files, such as lock files, by changing > + # the entire directory, not just the '$DB_FILE'. > + chown -R "$OVS_USER":"$OVS_GROUP" `dirname $DB_FILE` > } I'm a little nervous about this. Given a typo, etc., it could end up destroying the permissions in a large directory tree. I have two thoughts: 1. Change permissions for the directory and for the files in it (e.g. `dirname $DB_FILE`/*). 2. Change permissions for the directory, the database file, and the lock file.
On Tue, Nov 24, 2015 at 2:19 PM, Ben Pfaff <blp@ovn.org> wrote: > On Thu, Nov 19, 2015 at 12:58:38PM -0800, Andy Zhou wrote: > > Change ovsdb file ownership to match "$OVS_USER":"$OVS_GROUP" when > > we either create it for the first time, or upgrade it. > > > > Signed-off-by: Andy Zhou <azhou@ovn.org> > > --- > > utilities/ovs-lib.in | 4 ++++ > > 1 file changed, 4 insertions(+) > > > > diff --git a/utilities/ovs-lib.in b/utilities/ovs-lib.in > > index 34e2041..3fbc2f5 100644 > > --- a/utilities/ovs-lib.in > > +++ b/utilities/ovs-lib.in > > @@ -376,4 +376,8 @@ upgrade_db () { > > create_db "$DB_FILE" "$DB_SCHEMA" > > fi > > fi > > + > > + # Make sure we change the ownership of related files, such as lock > files, by changing > > + # the entire directory, not just the '$DB_FILE'. > > + chown -R "$OVS_USER":"$OVS_GROUP" `dirname $DB_FILE` > > } > > I'm a little nervous about this. Given a typo, etc., it could end up > destroying the permissions in a large directory tree. I have two > thoughts: > > 1. Change permissions for the directory and for the files in it > (e.g. `dirname $DB_FILE`/*). > > 2. Change permissions for the directory, the database file, and > the lock file. > Good point. #1 requires the script to have less knowledge of the C code (i.e. the lock filename), it seems more attractive. What do you think?
On Tue, Nov 24, 2015 at 03:02:08PM -0800, Andy Zhou wrote: > On Tue, Nov 24, 2015 at 2:19 PM, Ben Pfaff <blp@ovn.org> wrote: > > > On Thu, Nov 19, 2015 at 12:58:38PM -0800, Andy Zhou wrote: > > > Change ovsdb file ownership to match "$OVS_USER":"$OVS_GROUP" when > > > we either create it for the first time, or upgrade it. > > > > > > Signed-off-by: Andy Zhou <azhou@ovn.org> > > > --- > > > utilities/ovs-lib.in | 4 ++++ > > > 1 file changed, 4 insertions(+) > > > > > > diff --git a/utilities/ovs-lib.in b/utilities/ovs-lib.in > > > index 34e2041..3fbc2f5 100644 > > > --- a/utilities/ovs-lib.in > > > +++ b/utilities/ovs-lib.in > > > @@ -376,4 +376,8 @@ upgrade_db () { > > > create_db "$DB_FILE" "$DB_SCHEMA" > > > fi > > > fi > > > + > > > + # Make sure we change the ownership of related files, such as lock > > files, by changing > > > + # the entire directory, not just the '$DB_FILE'. > > > + chown -R "$OVS_USER":"$OVS_GROUP" `dirname $DB_FILE` > > > } > > > > I'm a little nervous about this. Given a typo, etc., it could end up > > destroying the permissions in a large directory tree. I have two > > thoughts: > > > > 1. Change permissions for the directory and for the files in it > > (e.g. `dirname $DB_FILE`/*). > > > > 2. Change permissions for the directory, the database file, and > > the lock file. > > > Good point. #1 requires the script to have less knowledge of the C code > (i.e. the lock filename), it seems more attractive. What do you think? It seems fine to me.
diff --git a/utilities/ovs-lib.in b/utilities/ovs-lib.in index 34e2041..3fbc2f5 100644 --- a/utilities/ovs-lib.in +++ b/utilities/ovs-lib.in @@ -376,4 +376,8 @@ upgrade_db () { create_db "$DB_FILE" "$DB_SCHEMA" fi fi + + # Make sure we change the ownership of related files, such as lock files, by changing + # the entire directory, not just the '$DB_FILE'. + chown -R "$OVS_USER":"$OVS_GROUP" `dirname $DB_FILE` }
Change ovsdb file ownership to match "$OVS_USER":"$OVS_GROUP" when we either create it for the first time, or upgrade it. Signed-off-by: Andy Zhou <azhou@ovn.org> --- utilities/ovs-lib.in | 4 ++++ 1 file changed, 4 insertions(+)