[libvirt] [PATCH 2/2] Implementation deficiency in virInitctlSetRunLevel v3

[Daniel P. Berrange]

On Fri, Dec 20, 2013 at 07:42:28PM +0400, Reco wrote:
> Use helper virProcessRunInMountNamespace in lxcDomainShutdownFlags and
> lxcDomainReboot.
> 
> ---
>  src/lxc/lxc_driver.c |   44 ++++++++++++++++++++++++++++----------------
>  1 file changed, 28 insertions(+), 16 deletions(-)
> 
> diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
> index e5298d1..7f4acbe 100644
> --- a/src/lxc/lxc_driver.c
> +++ b/src/lxc/lxc_driver.c
> @@ -2694,12 +2694,21 @@ lxcConnectListAllDomains(virConnectPtr conn,
>  
>  
>  static int
> +virDomainShutdownCallback(pid_t pid ATTRIBUTE_UNUSED,
> +                          void* opaque ATTRIBUTE_UNUSED)

Nitpick - the '*' should associate with 'opaque' rather than 'void'

> +{
> +    int rc;
> +    rc = virInitctlSetRunLevel(VIR_INITCTL_RUNLEVEL_POWEROFF, NULL);
> +    return rc;
> +}
> +
> +
> +static int
>  lxcDomainShutdownFlags(virDomainPtr dom,
>                         unsigned int flags)
>  {
>      virLXCDomainObjPrivatePtr priv;
>      virDomainObjPtr vm;
> -    char *vroot = NULL;
>      int ret = -1;
>      int rc;
>  
> @@ -2726,14 +2735,12 @@ lxcDomainShutdownFlags(virDomainPtr dom,
>          goto cleanup;
>      }
>  
> -    if (virAsprintf(&vroot, "/proc/%llu/root",
> -                    (unsigned long long)priv->initpid) < 0)
> -        goto cleanup;
> -
>      if (flags == 0 ||
>          (flags & VIR_DOMAIN_SHUTDOWN_INITCTL)) {
> -        if ((rc = virInitctlSetRunLevel(VIR_INITCTL_RUNLEVEL_POWEROFF,
> -                                        vroot)) < 0) {
> +        rc = virProcessRunInMountNamespace(priv->initpid,
> +                                           (virProcessNamespaceCallback) virDomainShutdownCallback,

I think you can leave out the (virProcessNamespaceCallback) cast
here, since the function prototype already matches what is
required.

> +                                           NULL);
> +        if (rc < 0) {
>              goto cleanup;
>          }
>          if (rc == 0 && flags != 0 &&
> @@ -2761,7 +2768,6 @@ lxcDomainShutdownFlags(virDomainPtr dom,
>      ret = 0;
>  
>  cleanup:
> -    VIR_FREE(vroot);
>      if (vm)
>          virObjectUnlock(vm);
>      return ret;
> @@ -2773,13 +2779,22 @@ lxcDomainShutdown(virDomainPtr dom)
>      return lxcDomainShutdownFlags(dom, 0);
>  }
>  
> +
> +virDomainRebootCallback(pid_t pid ATTRIBUTE_UNUSED,
> +                        void* opaque ATTRIBUTE_UNUSED)

Same nitpick

> +{
> +    int rc;
> +    rc = virInitctlSetRunLevel(VIR_INITCTL_RUNLEVEL_REBOOT, NULL);
> +    return rc;
> +}
> +
> +
>  static int
>  lxcDomainReboot(virDomainPtr dom,
>                  unsigned int flags)
>  {
>      virLXCDomainObjPrivatePtr priv;
>      virDomainObjPtr vm;
> -    char *vroot = NULL;
>      int ret = -1;
>      int rc;
>  
> @@ -2806,14 +2821,12 @@ lxcDomainReboot(virDomainPtr dom,
>          goto cleanup;
>      }
>  
> -    if (virAsprintf(&vroot, "/proc/%llu/root",
> -                    (unsigned long long)priv->initpid) < 0)
> -        goto cleanup;
> -
>      if (flags == 0 ||
>          (flags & VIR_DOMAIN_REBOOT_INITCTL)) {
> -        if ((rc = virInitctlSetRunLevel(VIR_INITCTL_RUNLEVEL_REBOOT,
> -                                        vroot)) < 0) {
> +        rc = virProcessRunInMountNamespace(priv->initpid,
> +                                           (virProcessNamespaceCallback) virDomainRebootCallback,

And here

> +                                           NULL);
> +        if (rc < 0) {
>              goto cleanup;
>          }
>          if (rc == 0 && flags != 0 &&

Regards,
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 :|