[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] PATCH: Allow LXC to use private /dev/pts instance



Quoting Daniel P. Berrange (berrange redhat com):
> This change seemed to fix that problem with no ill-effects.
> 
> -    if (chroot(oldroot) < 0) {
> -        virReportSystemError(NULL, errno, "%s",
> -                             _("failed to chroot into tmpfs"));
> -        goto err;
> -    }
> -
> -    if (chdir("/new") < 0) {
> -        virReportSystemError(NULL, errno, "%s",
> -                             _("failed to chdir into /new on tmpfs"));
> +    if (chdir(newroot) < 0) {
> +        virReportSystemError(NULL, errno,
> +                             _("failed to chroot into %s"), newroot);

Yes, good.  We can probably pare it down later, but I'll look at that
once other stuff settles down.

> So I'm removing this chunk:
> 
>      if (chdir("/") < 0)
>          goto err;
> 
> -    if (umount2(".oldroot", MNT_DETACH) < 0) {
> -        virReportSystemError(NULL, errno, "%s",
> -                             _("failed to lazily unmount old root"));
> -        goto err;
> -    }
> -

Yeah as I added that I actually was wondering whether that would happen
- whether libvirt would try to make later bind mounts out of the old
fs which I'd umonted.  But I couldn't find where else it was umounted.
Glad you solved it :)

...
> Index: src/lxc_container.c
> ===================================================================
...

This all looks good, though I haven't tested it yet.

> +    /*
> +     * If doing a chroot style setup, we need to prepare
> +     * a private /dev/pts for the child now, which they
> +     * will later move into position.
> +     *
> +     * This is complex because 'virsh console' needs to
> +     * use /dev/pts from the host OS, and the guest OS
> +     * needs to use /dev/pts from the guest.
> +     *
> +     * This means that we (libvirt_lxc) need to see and
> +     * use both /dev/pts instances. We're running in the
> +     * host OS context though and don't want to expose
> +     * the guest OS /dev/pts there.
> +     *
> +     * Thus we call unshare(CLONE_NS) so that we can see
> +     * the guest's new /dev/pts, without it becoming
> +     * visible to the host OS.
> +     */

Calling unshare(CLONE_NEWNS) will not prevent the host OS from
seeing the new /dev/pts if / was MS_SHARED.  That isn't taken
care of anywhere else for this process's namespace, is it?

I assume the reason you want the new devpts not visible in the
host OS is so that it will be auto-umounted when the container is
released?

Thanks for doing this, the patch looks really good (minus MS_SHARED
bit).

-serge


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]