[libvirt] [PATCH]lxc: improve readability of lxcContainer[Send|Waitfor]Continue

From: Chen Hanxiao <chenhanxiao@cn.fujitsu.com> Currently, lxcContainer[Send|Waitfor]Continue only tell us 'fd', but we had to deal with the interaction between lxc_[container|controller|process]. This patch adds parameters to identify the caller. Signed-off-by: Chen Hanxiao <chenhanxiao@cn.fujitsu.com> --- src/lxc/lxc_container.c | 16 +++++++++------- src/lxc/lxc_container.h | 4 ++-- src/lxc/lxc_controller.c | 6 +++--- src/lxc/lxc_process.c | 2 +- 4 files changed, 15 insertions(+), 13 deletions(-) diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c index ed1fe29..c7371ca 100644 --- a/src/lxc/lxc_container.c +++ b/src/lxc/lxc_container.c @@ -351,19 +351,20 @@ cleanup: /** * lxcContainerSendContinue: * @control: control FD to child + * @caller: function invoker * * Sends the continue message via the socket pair stored in the vm * structure. * * Returns 0 on success or -1 in case of error */ -int lxcContainerSendContinue(int control) +int lxcContainerSendContinue(int control, const char *caller) { int rc = -1; lxc_message_t msg = LXC_CONTINUE_MSG; int writeCount = 0; - VIR_DEBUG("Send continue on fd %d", control); + VIR_DEBUG("Send continue on fd %d by %s", control, caller); writeCount = safewrite(control, &msg, sizeof(msg)); if (writeCount != sizeof(msg)) { goto error_out; @@ -377,6 +378,7 @@ error_out: /** * lxcContainerWaitForContinue: * @control: Control FD from parent + * @caller: function invoker * * This function will wait for the container continue message from the * parent process. It will send this message on the socket pair stored in @@ -384,14 +386,14 @@ error_out: * * Returns 0 on success or -1 in case of error */ -int lxcContainerWaitForContinue(int control) +int lxcContainerWaitForContinue(int control, const char *caller) { lxc_message_t msg; int readLen; - VIR_DEBUG("Wait continue on fd %d", control); + VIR_DEBUG("%s wait continue on fd %d", caller, control); readLen = saferead(control, &msg, sizeof(msg)); - VIR_DEBUG("Got continue on fd %d %d", control, readLen); + VIR_DEBUG("%s got continue on fd %d %d", caller, control, readLen); if (readLen != sizeof(msg)) { if (readLen >= 0) errno = EIO; @@ -1813,7 +1815,7 @@ static int lxcContainerChild(void *data) /* Wait for controller to finish setup tasks, including * things like move of network interfaces, uid/gid mapping */ - if (lxcContainerWaitForContinue(argv->monitor) < 0) { + if (lxcContainerWaitForContinue(argv->monitor, __func__) < 0) { virReportSystemError(errno, "%s", _("Failed to read the container continue message")); goto cleanup; @@ -1880,7 +1882,7 @@ static int lxcContainerChild(void *data) if (lxcContainerDropCapabilities(!!hasReboot) < 0) goto cleanup; - if (lxcContainerSendContinue(argv->handshakefd) < 0) { + if (lxcContainerSendContinue(argv->handshakefd, __func__) < 0) { virReportSystemError(errno, "%s", _("Failed to send continue signal to controller")); goto cleanup; diff --git a/src/lxc/lxc_container.h b/src/lxc/lxc_container.h index e74a7d7..77fb2a0 100644 --- a/src/lxc/lxc_container.h +++ b/src/lxc/lxc_container.h @@ -49,8 +49,8 @@ enum { # define LXC_DEV_MAJ_FUSE 10 # define LXC_DEV_MIN_FUSE 229 -int lxcContainerSendContinue(int control); -int lxcContainerWaitForContinue(int control); +int lxcContainerSendContinue(int control, const char *caller); +int lxcContainerWaitForContinue(int control, const char *caller); int lxcContainerStart(virDomainDefPtr def, virSecurityManagerPtr securityDriver, diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c index 851a773..25dbbec 100644 --- a/src/lxc/lxc_controller.c +++ b/src/lxc/lxc_controller.c @@ -319,7 +319,7 @@ static int virLXCControllerConsoleSetNonblocking(virLXCControllerConsolePtr cons static int virLXCControllerDaemonHandshake(virLXCControllerPtr ctrl) { - if (lxcContainerSendContinue(ctrl->handshakeFd) < 0) { + if (lxcContainerSendContinue(ctrl->handshakeFd, __func__) < 0) { virReportSystemError(errno, "%s", _("error sending continue signal to daemon")); return -1; @@ -2176,13 +2176,13 @@ virLXCControllerRun(virLXCControllerPtr ctrl) if (virLXCControllerMoveInterfaces(ctrl) < 0) goto cleanup; - if (lxcContainerSendContinue(control[0]) < 0) { + if (lxcContainerSendContinue(control[0], __func__) < 0) { virReportSystemError(errno, "%s", _("Unable to send container continue message")); goto cleanup; } - if (lxcContainerWaitForContinue(containerhandshake[0]) < 0) { + if (lxcContainerWaitForContinue(containerhandshake[0], __func__) < 0) { virReportSystemError(errno, "%s", _("error receiving signal from container")); goto cleanup; diff --git a/src/lxc/lxc_process.c b/src/lxc/lxc_process.c index f088e8e..de20dd0 100644 --- a/src/lxc/lxc_process.c +++ b/src/lxc/lxc_process.c @@ -1264,7 +1264,7 @@ int virLXCProcessStart(virConnectPtr conn, if (virAtomicIntInc(&driver->nactive) == 1 && driver->inhibitCallback) driver->inhibitCallback(true, driver->inhibitOpaque); - if (lxcContainerWaitForContinue(handshakefds[0]) < 0) { + if (lxcContainerWaitForContinue(handshakefds[0], __func__) < 0) { char out[1024]; if (!(virLXCProcessReadLogOutput(vm, logfile, pos, out, 1024) < 0)) { -- 1.8.2.1

On 16.10.2013 08:27, Chen Hanxiao wrote:
From: Chen Hanxiao <chenhanxiao@cn.fujitsu.com>
Currently, lxcContainer[Send|Waitfor]Continue only tell us 'fd', but we had to deal with the interaction between lxc_[container|controller|process]. This patch adds parameters to identify the caller.
Signed-off-by: Chen Hanxiao <chenhanxiao@cn.fujitsu.com> --- src/lxc/lxc_container.c | 16 +++++++++------- src/lxc/lxc_container.h | 4 ++-- src/lxc/lxc_controller.c | 6 +++--- src/lxc/lxc_process.c | 2 +- 4 files changed, 15 insertions(+), 13 deletions(-)
I think this can be achieved even without this hack. We already can produce a stack trace on VIR_DEBUG() [1]. And if you don't want to keep only some debug messages, apply filters. Michal 1: http://libvirt.org/logging.html#log_syntax

-----Original Message----- From: Michal Privoznik [mailto:mprivozn@redhat.com] Sent: Monday, October 21, 2013 9:29 PM To: Chen Hanxiao Cc: libvir-list@redhat.com Subject: Re: [libvirt] [PATCH]lxc: improve readability of lxcContainer[Send|Waitfor]Continue
On 16.10.2013 08:27, Chen Hanxiao wrote:
From: Chen Hanxiao <chenhanxiao@cn.fujitsu.com>
I think this can be achieved even without this hack. We already can produce a stack trace on VIR_DEBUG() [1]. And if you don't want to keep only some debug messages, apply filters.
If we only config debug option for output values, we got: 604+0000: 12010: debug : lxcContainerWaitForContinue:392 : Wait continue on fd 21 If we config filter option as "1:+lxc", we got: 2013-10-22 02:24:30.365+0000: 13579: debug : lxcContainerWaitForContinue:392 : Wait continue on fd 12 /usr/libexec/libvirt_lxc(virLogMessage+0x97)[0x7f2e1bdfa387] /usr/libexec/libvirt_lxc(lxcContainerWaitForContinue+0x4b)[0x7f2e1bd7346b] /usr/libexec/libvirt_lxc(+0x3197b)[0x7f2e1bd7b97b] /usr/libexec/libvirt_lxc(main+0xcdf)[0x7f2e1bd70e4f] /lib64/libc.so.6(__libc_start_main+0xf5)[0x7f2e19761b75] /usr/libexec/libvirt_lxc(+0x27065)[0x7f2e1bd71065] We still can't see who is the caller, and we got too many logs. Additionally, filters with long name like ' lxcContainerWaitForContinue' will not take effect. With this patch, we got: 17931: debug : lxcContainerWaitForContinue:394 : virLXCControllerRun wait continue on fd 12 I think we still need this patch. Thanks!
Michal

Subject: Re: [libvirt] [PATCH]lxc: improve readability of lxcContainer[Send|Waitfor]Continue
On 16.10.2013 08:27, Chen Hanxiao wrote:
From: Chen Hanxiao <chenhanxiao@cn.fujitsu.com>
I think this can be achieved even without this hack. We already can produce a stack trace on VIR_DEBUG() [1]. And if you don't want to keep only some debug messages, apply filters.
If we only config debug option for output values, we got: 604+0000: 12010: debug : lxcContainerWaitForContinue:392 : Wait continue on fd 21
If we config filter option as "1:+lxc", we got: 2013-10-22 02:24:30.365+0000: 13579: debug : lxcContainerWaitForContinue:392 : Wait continue on fd 12 /usr/libexec/libvirt_lxc(virLogMessage+0x97)[0x7f2e1bdfa387] /usr/libexec/libvirt_lxc(lxcContainerWaitForContinue+0x4b)[0x7f2e1bd7346b] /usr/libexec/libvirt_lxc(+0x3197b)[0x7f2e1bd7b97b] /usr/libexec/libvirt_lxc(main+0xcdf)[0x7f2e1bd70e4f] /lib64/libc.so.6(__libc_start_main+0xf5)[0x7f2e19761b75] /usr/libexec/libvirt_lxc(+0x27065)[0x7f2e1bd71065]
We still can't see who is the caller, and we got too many logs. Additionally, filters with long name like ' lxcContainerWaitForContinue' will not take effect.
With this patch, we got: 17931: debug : lxcContainerWaitForContinue:394 : virLXCControllerRun wait continue on fd 12
I think we still need this patch.
Thanks!
Hi Any comments? Thanks!

On Thu, Oct 24, 2013 at 05:55:59PM +0800, Chen Hanxiao wrote:
Subject: Re: [libvirt] [PATCH]lxc: improve readability of lxcContainer[Send|Waitfor]Continue
On 16.10.2013 08:27, Chen Hanxiao wrote:
From: Chen Hanxiao <chenhanxiao@cn.fujitsu.com>
I think this can be achieved even without this hack. We already can produce a stack trace on VIR_DEBUG() [1]. And if you don't want to keep only some debug messages, apply filters.
If we only config debug option for output values, we got: 604+0000: 12010: debug : lxcContainerWaitForContinue:392 : Wait continue on fd 21
If we config filter option as "1:+lxc", we got: 2013-10-22 02:24:30.365+0000: 13579: debug : lxcContainerWaitForContinue:392 : Wait continue on fd 12 /usr/libexec/libvirt_lxc(virLogMessage+0x97)[0x7f2e1bdfa387] /usr/libexec/libvirt_lxc(lxcContainerWaitForContinue+0x4b)[0x7f2e1bd7346b] /usr/libexec/libvirt_lxc(+0x3197b)[0x7f2e1bd7b97b] /usr/libexec/libvirt_lxc(main+0xcdf)[0x7f2e1bd70e4f] /lib64/libc.so.6(__libc_start_main+0xf5)[0x7f2e19761b75] /usr/libexec/libvirt_lxc(+0x27065)[0x7f2e1bd71065]
You can convert those using addr2line.
We still can't see who is the caller, and we got too many logs. Additionally, filters with long name like ' lxcContainerWaitForContinue' will not take effect.
With this patch, we got: 17931: debug : lxcContainerWaitForContinue:394 : virLXCControllerRun wait continue on fd 12
I think we still need this patch.
I don't, though. This adds unnecessary code. Sorry, Martin

-----Original Message----- From: Martin Kletzander [mailto:mkletzan@redhat.com] Sent: Thursday, October 24, 2013 6:52 PM To: Chen Hanxiao Cc: 'Michal Privoznik'; libvir-list@redhat.com Subject: Re: [libvirt] [PATCH]lxc: improve readability of lxcContainer[Send|Waitfor]Continue
On Thu, Oct 24, 2013 at 05:55:59PM +0800, Chen Hanxiao wrote:
Subject: Re: [libvirt] [PATCH]lxc: improve readability of lxcContainer[Send|Waitfor]Continue /usr/libexec/libvirt_lxc(+0x3197b)[0x7f2e1bd7b97b] /usr/libexec/libvirt_lxc(main+0xcdf)[0x7f2e1bd70e4f] /lib64/libc.so.6(__libc_start_main+0xf5)[0x7f2e19761b75] /usr/libexec/libvirt_lxc(+0x27065)[0x7f2e1bd71065]
You can convert those using addr2line.
Thanks
We still can't see who is the caller, and we got too many logs. Additionally, filters with long name like '
lxcContainerWaitForContinue'
will not take effect.
With this patch, we got: 17931: debug : lxcContainerWaitForContinue:394 : virLXCControllerRun wait continue on fd 12
I think we still need this patch.
I don't, though. This adds unnecessary code.
Thanks. I see.
Sorry, Martin

On Mon, Oct 21, 2013 at 02:28:37PM +0100, Michal Privoznik wrote:
On 16.10.2013 08:27, Chen Hanxiao wrote:
From: Chen Hanxiao <chenhanxiao@cn.fujitsu.com>
Currently, lxcContainer[Send|Waitfor]Continue only tell us 'fd', but we had to deal with the interaction between lxc_[container|controller|process]. This patch adds parameters to identify the caller.
Signed-off-by: Chen Hanxiao <chenhanxiao@cn.fujitsu.com> --- src/lxc/lxc_container.c | 16 +++++++++------- src/lxc/lxc_container.h | 4 ++-- src/lxc/lxc_controller.c | 6 +++--- src/lxc/lxc_process.c | 2 +- 4 files changed, 15 insertions(+), 13 deletions(-)
I think this can be achieved even without this hack. We already can produce a stack trace on VIR_DEBUG() [1]. And if you don't want to keep only some debug messages, apply filters.
FYI, you don't even need to look at stack traces. The debug message includes the PID - the LXC controller will have a large random PID number, while what will become the container init process will have PID ==1 eg # grep Continue wait9.log 2013-09-10 14:54:54.416+0000: 17150: debug : lxcContainerSendContinue:366 : Send continue on fd 8 2013-09-10 14:54:54.416+0000: 17150: debug : lxcContainerWaitForContinue:392 : Wait continue on fd 10 2013-09-10 14:54:54.416+0000: 1: debug : lxcContainerWaitForContinue:392 : Wait continue on fd 9 2013-09-10 14:54:54.416+0000: 1: debug : lxcContainerWaitForContinue:394 : Got continue on fd 9 1 2013-09-10 14:54:54.437+0000: 1: debug : lxcContainerSendContinue:366 : Send continue on fd 11 2013-09-10 14:54:54.437+0000: 17150: debug : lxcContainerWaitForContinue:394 : Got continue on fd 10 1 2013-09-10 14:54:54.437+0000: 17150: debug : lxcContainerSendContinue:366 : Send continue on fd 52 So I agree, this patch is not really compelling enough to add 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 :|

-----Original Message----- From: Daniel P. Berrange [mailto:berrange@redhat.com] Sent: Monday, October 28, 2013 7:29 PM To: Michal Privoznik Cc: Chen Hanxiao; libvir-list@redhat.com Subject: Re: [libvirt] [PATCH]lxc: improve readability of lxcContainer[Send|Waitfor]Continue
On Mon, Oct 21, 2013 at 02:28:37PM +0100, Michal Privoznik wrote:
On 16.10.2013 08:27, Chen Hanxiao wrote:
From: Chen Hanxiao <chenhanxiao@cn.fujitsu.com>
Currently, lxcContainer[Send|Waitfor]Continue only tell us 'fd', but we had to deal with the interaction between lxc_[container|controller|process]. This patch adds parameters to identify the caller.
Signed-off-by: Chen Hanxiao <chenhanxiao@cn.fujitsu.com> --- src/lxc/lxc_container.c | 16 +++++++++------- src/lxc/lxc_container.h | 4 ++-- src/lxc/lxc_controller.c | 6 +++--- src/lxc/lxc_process.c | 2 +- 4 files changed, 15 insertions(+), 13 deletions(-)
I think this can be achieved even without this hack. We already can produce a stack trace on VIR_DEBUG() [1]. And if you don't want to keep only some debug messages, apply filters.
FYI, you don't even need to look at stack traces. The debug message includes the PID - the LXC controller will have a large random PID number, while what will become the container init process will have PID ==1
That would be very helpful. Thanks for your debug hint.
participants (4)
-
Chen Hanxiao
-
Daniel P. Berrange
-
Martin Kletzander
-
Michal Privoznik