Re: [libvirt] [PATCH v3 2/4] storage: conf: Don't set any default <mode> in the XML

On 05/21/2015 04:03 PM, Cole Robinson wrote: > The XML parser sets a default <mode> if none is explicitly passed in. > This is then used at pool/vol creation time, and unconditionally reported > in the XML. > > The problem with this approach is that it's impossible for other code > to determine if the user explicitly requested a storage mode. There > are some cases where we want to make this distinction, but we currently > can't. > > Handle <mode> parsing like we handle <owner>/<group>: if no value is > passed in, set it to -1, and adjust the internal consumers to handle > it. > --- > v3: > Drop needless test churn > Add docs about default <mode> > > docs/formatstorage.html.in | 4 +++ > docs/schemas/storagecommon.rng | 8 +++-- > src/conf/storage_conf.c | 34 +++++++++------------- > src/storage/storage_backend.c | 20 +++++++++---- > src/storage/storage_backend.h | 3 ++ > src/storage/storage_backend_fs.c | 9 ++++-- > src/storage/storage_backend_logical.c | 4 ++- > tests/storagepoolxml2xmlout/pool-netfs-gluster.xml | 1 - > tests/storagevolxml2xmlout/vol-gluster-dir.xml | 1 - > tests/storagevolxml2xmlout/vol-sheepdog.xml | 1 - > 10 files changed, 51 insertions(+), 34 deletions(-) > > diff --git a/docs/formatstorage.html.in b/docs/formatstorage.html.in > index f07bb5d..ccde978 100644 > --- a/docs/formatstorage.html.in > +++ b/docs/formatstorage.html.in > @@ -406,6 +406,8 @@ > namespace. It provides information about the permissions to use for the > final directory when the pool is built. The > <code>mode</code> element contains the octal permission set. > + If <code>mode</code> is unset when creating a directory, > + the value 0755 is used. Consider "The mode defaults to 0755 when not provided." The verbiage "is unset" reads strangly

> The <code>owner</code> element contains the numeric user ID. > The <code>group</code> element contains the numeric group ID. > If <code>owner</code> or <code>group</code> aren't specified when > @@ -595,6 +597,8 @@ > files. For pools where the volumes are device nodes, the hotplug > scripts determine permissions. It contains 4 child elements. The > <code>mode</code> element contains the octal permission set. > + If <code>mode</code> is unset when creating a supported volume, > + the value 0600 is used. ditto but 0600 Also, wherever you "point" the "<backing store elements> <permissions> to should be whichever default is correct for the backing store. that is from patch 1, you indicated that the elements are the same as one of these two elements, so to be technically correct be sure to redirect to either the 0755 or 0600 for which the backing store element would default to if not provided.