[libvirt] [PATCH] security: Use VIR_DEBUG instead of VIR_INFO in virSecurityDACSetOwnershipInternal

[Peter Krempa]

On Sat, Jul 15, 2017 at 16:31:39 +0800, xinhua.Cao wrote:
> virSecurityDACSetOwnershipInternal was called by libvirt child process,
> so if we log message by VIR_INFO at normal scene, it would probability occurs dead lock sence,
> then libvirtd will also by dead lock because libvirtd is waitting for child message.
> so our suggest is use VIR_DEBUG instead of VIR_INFO to avoid this sence.

This won't help. VIR_DEBUG and VIR_INFO macros both translate into a
virLogMessage call thus follow the same code path. So this patch
definitely will not fix it completely.

If you've witnessed a deadlock, please post a backtrace and a debug log
somewhere, since this needs to be fixed at the root.

> ---
>  src/security/security_dac.c | 22 +++++++++++-----------
>  1 file changed, 11 insertions(+), 11 deletions(-)
> 
> diff --git a/src/security/security_dac.c b/src/security/security_dac.c
> index ca7a6af..7bfd090 100644
> --- a/src/security/security_dac.c
> +++ b/src/security/security_dac.c
> @@ -552,8 +552,8 @@ virSecurityDACSetOwnershipInternal(const virSecurityDACData *priv,
>      else if (rc > 0)
>          return 0;
>  
> -    VIR_INFO("Setting DAC user and group on '%s' to '%ld:%ld'",
> -             NULLSTR(src ? src->path : path), (long) uid, (long) gid);
> +    VIR_DEBUG("Setting DAC user and group on '%s' to '%ld:%ld'",
> +              NULLSTR(src ? src->path : path), (long) uid, (long) gid);
>  
>      if (priv && src && priv->chownCallback) {
>          rc = priv->chownCallback(src, uid, gid);
> @@ -591,17 +591,17 @@ virSecurityDACSetOwnershipInternal(const virSecurityDACData *priv,
>  
>      if (rc < 0) {
>          if (errno == EOPNOTSUPP || errno == EINVAL) {
> -            VIR_INFO("Setting user and group to '%ld:%ld' on '%s' not "
> -                     "supported by filesystem",
> -                     (long) uid, (long) gid, path);
> +            VIR_DEBUG("Setting user and group to '%ld:%ld' on '%s' not "
> +                      "supported by filesystem",
> +                      (long) uid, (long) gid, path);
>          } else if (errno == EPERM) {
> -            VIR_INFO("Setting user and group to '%ld:%ld' on '%s' not "
> -                     "permitted",
> -                     (long) uid, (long) gid, path);
> +            VIR_DEBUG("Setting user and group to '%ld:%ld' on '%s' not "
> +                      "permitted",
> +                      (long) uid, (long) gid, path);
>          } else if (errno == EROFS) {
> -            VIR_INFO("Setting user and group to '%ld:%ld' on '%s' not "
> -                     "possible on readonly filesystem",
> -                     (long) uid, (long) gid, path);
> +            VIR_DEBUG("Setting user and group to '%ld:%ld' on '%s' not "
> +                      "possible on readonly filesystem",
> +                      (long) uid, (long) gid, path);

NACK

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20170717/2f71113e/attachment-0001.sig>