Re: [Devel] [PATCH hci-8.1] ve/vtty: fix console attach failing with EBUSY while the CT closes the tty

[Konstantin Khorenko <khorenko@virtuozzo.com>]

Hi Denis,

> Return -EAGAIN for the dying pair. Nothing else on the
> VE_CONFIGURE_OPEN_TTY path returns -EAGAIN, so it can only mean "try
> again", which is what vzctl will do.

Agreed, together with your vzctl change ("vzctl console: retry opening
a CT tty the CT is still closing") the bug is fixed. But only for the new
vzctl: every vzctl already installed tries the ioctl once and now fails
with "Resource temporarily unavailable" instead of "Device or resource
busy".

> An in-CT open of such a pair gets
> -EAGAIN from tty_reopen() and tty_open() retries it the same way.
> Retrying inside vtty_open_master() instead would have to drop tty_mutex
> for release_tty() to progress, and the global vtty context with it.

We can fix it for old and new vzctl alike by doing what mainline did
for the same window in 7f22f6c935cda ("tty: Retry failed reopen if tty
teardown in-progress"), retry in the kernel:

        if (IS_ERR(tty)) {
                tty_free_file(filp);
                retval = PTR_ERR(tty);
                if (retval != -EAGAIN || signal_pending(current))
                        return retval;
                schedule();
                goto retry_open;
        }

Dropping tty_mutex and the vtty context is not a problem: err_unlock
already does exactly that. So in vtty_open_master():

        retry:
                mutex_lock(&tty_mutex);
                vtty_set_context(veid);
                ...
                if (!created && slave->count == 0) {
                        tty_unlock(slave);
                        vtty_drop_context();
                        mutex_unlock(&tty_mutex);
                        if (signal_pending(current)) {
                                ret = -EAGAIN;
                                goto err_free_file;     /* tty_free_file() onwards */
                        }
                        schedule();
                        goto retry;
                }

The fd and the file stay allocated across the loop, nothing else
changes.

The loop always ends: we get there only with master->count == 0 and
slave->count == 0, i.e. tty_release() has decided on the final close,
so the closer is already in tty_release_struct() on its way to
release_tty() -> vtty_shutdown() -> vtty_map_clear(). Once we drop
tty_mutex it gets through, and the next lookup misses and allocates a
fresh pair. The BSD master pty that Peter excluded from the retry in
mainline (its slave fds may stay open forever) is not our case: here
both counts are already zero.

What we get:
- old vzctl works with the kernel update alone, the vzctl change becomes
  optional;
- -EBUSY keeps meaning "console attached" only, -EAGAIN is left only for
  a pending signal;
- it goes on top of what is committed, no revert needed.

Could you send that as a follow-up on VSTOR-152365?

--
Konstantin
_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel