[libvirt] [PATCH] Require at least one console for LXC domain

[Martin Kletzander]

On Tue, Nov 04, 2014 at 03:21:53PM +0100, Ján Tomko wrote:
>On 10/31/2014 11:53 AM, Martin Kletzander wrote:
>> On Fri, Oct 31, 2014 at 10:22:25AM +0100, Ján Tomko wrote:
>>> A domain without a console quietly dies soon after start,
>>> because we try to set /dev/null as a controlling TTY
>>> 2014-10-30 15:10:59.705+0000: 1: error : lxcContainerSetupFDs:283 :
>>> ioctl(TIOCSTTY) failed: Inappropriate ioctl for device
>>>
>>
>> s/TIOCSTTY/TIOCSCTTY/ would make it more greppable (is that even a
>> word?) and it's now true since you pushed your trivial fix for that ;)
>>
>
>I have amended the commit message.
>
>>> Report an error early instead of trying to start it.
>>>
>>> https://bugzilla.redhat.com/show_bug.cgi?id=1155410
>>> ---
>>> src/lxc/lxc_container.c | 6 ++++--
>>> src/lxc/lxc_process.c   | 6 ++++++
>>> 2 files changed, 10 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c
>>> index f02b959..8aba3ba 100644
>>> --- a/src/lxc/lxc_container.c
>>> +++ b/src/lxc/lxc_container.c
>>> @@ -2093,8 +2093,10 @@ static int lxcContainerChild(void *data)
>>>         if (virAsprintf(&ttyPath, "%s/%s.devpts/%s",
>>>                         LXC_STATE_DIR, vmDef->name, tty) < 0)
>>>             goto cleanup;
>>> -    } else if (VIR_STRDUP(ttyPath, "/dev/null") < 0) {
>>> -            goto cleanup;
>>> +    } else {
>>> +        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
>>> +                       _("At least one tty is required"));
>>> +        goto cleanup;
>>>     }
>>
>> How about we don't attach any tty and use the ioctl(TIOCSTTY) to steal
>> controlling terminal (or call open() with O_NOCTTY, or just don't call
>> ioctl() at all, I don't know what we normally use)?  Then there would
>> be no controlling terminal, but there would still be a session leader,
>> wouldn't it?
>>
>
>I don't know if we want to run a container without a controlling terminal.
>Looking at git history, back when we only supported one console and none was
>specified, it seems we quietly opened one anyway and didn't report it in the XML.
>
>Alternatively, we could auto-add a console in the XML.
>
>> Anyway, this version looks OK too since it doesn't break any use case
>> (there is nothing to break now).  ACK after release if you argue why
>> not to use the idea above.
>
>It's been broken long enough to make me think nobody cares about this
>functionality, but I could make it work if you think it's worthwhile.
>

Not worth while, I agree with you.  It can be done later on if someone
really wants it.

ACK,

Martin
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20141105/a3a6b479/attachment-0001.sig>