[libvirt] [PATCH]lxc: don't always print debug log in lxcContainerSetID

[Daniel P. Berrange]

On Wed, Oct 16, 2013 at 12:08:34PM +0800, Chen Hanxiao wrote:
> From: Chen Hanxiao <chenhanxiao at cn.fujitsu.com>
> 
> lxcContainerSetID is used for user namespace.
> If we don't enable user namespace, don't print debug log.
> 
> Signed-off-by: Chen Hanxiao <chenhanxiao at cn.fujitsu.com>
> ---
>  src/lxc/lxc_container.c | 14 ++++++++------
>  1 file changed, 8 insertions(+), 6 deletions(-)
> 
> diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c
> index 56df69e..ed1fe29 100644
> --- a/src/lxc/lxc_container.c
> +++ b/src/lxc/lxc_container.c
> @@ -420,12 +420,14 @@ static int lxcContainerSetID(virDomainDefPtr def)
>       * for this container. And user namespace is only enabled
>       * when nuidmap&ngidmap is not zero */
>  
> -    VIR_DEBUG("Set UID/GID to 0/0");
> -    if (def->idmap.nuidmap &&
> -        virSetUIDGID(0, 0, NULL, 0) < 0) {
> -        virReportSystemError(errno, "%s",
> -                             _("setuid or setgid failed"));
> -        return -1;
> +    if (def->idmap.nuidmap) {
> +        if (virSetUIDGID(0, 0, NULL, 0) < 0) {
> +            virReportSystemError(errno, "%s",
> +                                 _("setuid or setgid failed"));
> +            return -1;
> +        } else {
> +            VIR_DEBUG("Set UID/GID to 0/0");
> +        }

It is better to print the debug message *before* the call to
virSetUIDGID, because then if it fails you have a log message
clearly showing where it got to.

As such I've pushed this instead:

@@ -420,9 +420,11 @@ static int lxcContainerSetID(virDomainDefPtr def)
      * for this container. And user namespace is only enabled
      * when nuidmap&ngidmap is not zero */
 
-    VIR_DEBUG("Set UID/GID to 0/0");
-    if (def->idmap.nuidmap &&
-        virSetUIDGID(0, 0, NULL, 0) < 0) {
+    if (!def->idmap.nuidmap)
+        return 0;
+
+    VIR_DEBUG("Setting UID/GID to 0/0");
+    if (virSetUIDGID(0, 0, NULL, 0) < 0) {
         virReportSystemError(errno, "%s",
                              _("setuid or setgid failed"));
         return -1;


Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|