[libvirt] [PATCH] Require at least one console for LXC domain
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 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; } VIR_DEBUG("Container TTY path: %s", ttyPath); diff --git a/src/lxc/lxc_process.c b/src/lxc/lxc_process.c index ed30c37..6c83fdb 100644 --- a/src/lxc/lxc_process.c +++ b/src/lxc/lxc_process.c @@ -1144,6 +1144,12 @@ int virLXCProcessStart(virConnectPtr conn, vm->def, NULL) < 0) goto cleanup; + if (vm->def->nconsoles == 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("At least one PTY console is required")); + goto cleanup; + } + for (i = 0; i < vm->def->nconsoles; i++) { char *ttyPath; if (vm->def->consoles[i]->source.type != VIR_DOMAIN_CHR_TYPE_PTY) { -- 2.0.4
-----Original Message----- From: libvir-list-bounces@redhat.com [mailto:libvir-list-bounces@redhat.com] On Behalf Of Ján Tomko Sent: Friday, October 31, 2014 5:22 PM To: libvir-list@redhat.com Subject: [libvirt] [PATCH] Require at least one console for LXC domain
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
Report an error early instead of trying to start it.
Reviewed-by: Chen Hanxiao <chenhanxiao@cn.fujitsu.com> Thanks, - Chen
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 ;)
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? 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. Martin
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. Jan
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
-----Original Message----- From: libvir-list-bounces@redhat.com [mailto:libvir-list-bounces@redhat.com] On Behalf Of Martin Kletzander Sent: Wednesday, November 05, 2014 4:10 PM To: Ján Tomko Cc: libvir-list@redhat.com [snip]
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,
Agree. But we could do some improvements such as adding a console for containers automatically if we do not provided one in XML. Thanks - Chen
participants (3)
-
Chen, Hanxiao -
Ján Tomko -
Martin Kletzander