On Tue, Jul 24, 2012 at 14:22:51 +0100, Daniel P. Berrange wrote:
> From: "Daniel P. Berrange" <berrange@xxxxxxxxxx>
>
> This defines a new RPC protocol to be used between the LXC
> controller and the libvirtd LXC driver. There is only a
> single RPC message defined thus far, an asynchronous "EXIT"
> event that is emitted just before the LXC controller process
> exits. This provides the LXC driver with details about how
> the container shutdown - normally, or abnormally (crashed),
> thus allowing the driver to emit better libvirt events.
>
> Emitting the event in the LXC controller requires a few
> little tricks with the RPC service. Simply calling the
> virNetServiceClientSendMessage does not work, since this
> merely queues the message for asynchronous processing.
> In addition the main event loop is no longer running at
> the point the event is emitted, so no I/O is processed.
>
> Thus after invoking virNetServiceClientSendMessage it is
> necessary to mark the client as being in "delayed close"
> mode. Then the event loop is run again, until the client
> completes its close - this happens only after the queued
> message has been fully transmitted. The final complexity
> is that it is not safe to run virNetServerQuit() from the
> client close callback, since that is invoked from a
> context where the server is locked. Thus a zero-second
> timer is used to trigger shutdown of the event loop,
> causing the controller to finally exit.
>
> * src/Makefile.am: Add rules for generating RPC protocol
> files and dispatch methods
> * src/lxc/lxc_controller.c: Emit an RPC event immediately
> before exiting
> * src/lxc/lxc_domain.h: Record the shutdown reason
> given by the controller
> * src/lxc/lxc_monitor.c, src/lxc/lxc_monitor.h: Register
> RPC program and event handler. Add callback to let
> driver receive EXIT event.
> * src/lxc/lxc_process.c: Use monitor exit event to decide
> what kind of domain event to emit
> * src/lxc/lxc_protocol.x: Define wire protocol for LXC
> controller monitor.
>
> Signed-off-by: Daniel P. Berrange <berrange@xxxxxxxxxx>
> ---
> .gitignore | 4 ++
> src/Makefile.am | 50 +++++++++++++++++++
> src/lxc/lxc_controller.c | 125 +++++++++++++++++++++++++++++++++++++++++++++-
> src/lxc/lxc_domain.h | 1 +
> src/lxc/lxc_monitor.c | 41 +++++++++++++++
> src/lxc/lxc_monitor.h | 6 +++
> src/lxc/lxc_process.c | 27 ++++++++++
> src/lxc/lxc_protocol.x | 21 ++++++++
> 8 files changed, 274 insertions(+), 1 deletion(-)
> create mode 100644 src/lxc/lxc_protocol.x
>
> diff --git a/.gitignore b/.gitignore
> index b4cbb5f..e4b3932 100644
> --- a/.gitignore
> +++ b/.gitignore
> @@ -105,6 +105,10 @@
> /src/locking/qemu-sanlock.conf
> /src/locking/test_libvirt_sanlock.aug
> /src/lxc/test_libvirtd_lxc.aug
> +/src/lxc/lxc_controller_dispatch.h
> +/src/lxc/lxc_monitor_dispatch.h
> +/src/lxc/lxc_protocol.c
> +/src/lxc/lxc_protocol.h
> /src/qemu/test_libvirtd_qemu.aug
> /src/remote/*_client_bodies.h
> /src/remote/*_protocol.[ch]
> diff --git a/src/Makefile.am b/src/Makefile.am
> index 492ce73..44da1fa 100644
> --- a/src/Makefile.am
> +++ b/src/Makefile.am
> @@ -347,7 +347,55 @@ if WITH_XEN_INOTIFY
> XEN_DRIVER_SOURCES += xen/xen_inotify.c xen/xen_inotify.h
> endif
>
> +EXTRA_DIST += \
> + dtrace2systemtap.pl \
> + rpc/gendispatch.pl \
> + rpc/genprotocol.pl \
> + rpc/gensystemtap.pl \
> + rpc/virnetprotocol.x \
> + rpc/virkeepaliveprotocol.x
Did you want to move this EXTRA_DIST part and copied it instead? I don't see
the corresponding - lines.
> +
> +LXC_PROTOCOL_GENERATED = \
> + $(srcdir)/lxc/lxc_protocol.h \
> + $(srcdir)/lxc/lxc_protocol.c \
> + $(NULL)
Did you add $(NULL) because you want to be able to add new entries here
without the need append '\' to the last line or is there another reason for
it?
> +
> +LXC_MONITOR_GENERATED = \
> + $(srcdir)/lxc/lxc_monitor_dispatch.h \
> + $(NULL)
> +
> +LXC_CONTROLLER_GENERATED = \
> + $(srcdir)/lxc/lxc_controller_dispatch.h \
> + $(NULL)
> +
> +LXC_GENERATED = \
> + $(LXC_PROTOCOL_GENERATED) \
> + $(LXC_MONITOR_GENERATED) \
> + $(LXC_CONTROLLER_GENERATED) \
> + $(NULL)
> +
> +LXC_PROTOCOL = $(srcdir)/lxc/lxc_protocol.x
> +
> +$(srcdir)/lxc/lxc_monitor_dispatch.h: $(srcdir)/rpc/gendispatch.pl \
> + $(LXC_PROTOCOL)
> + $(AM_V_GEN)$(PERL) -w $(srcdir)/rpc/gendispatch.pl \
> + -k virLXCProtocol VIR_LXC_PROTOCOL $(LXC_PROTOCOL) > $@
> +
> +$(srcdir)/lxc/lxc_controller_dispatch.h: $(srcdir)/../src/rpc/gendispatch.pl \
"../src" is redundant here.
> + $(REMOTE_PROTOCOL)
> + $(AM_V_GEN)$(PERL) -w $(srcdir)/rpc/gendispatch.pl \
> + -b virLXCProtocol VIR_LXC_PROTOCOL $(LXC_PROTOCOL) > $@
> +
> +EXTRA_DIST += \
> + $(LXC_PROTOCOL) \
> + $(LXC_GENERATED) \
> + $(NULL)
> +
> +BUILT_SOURCES += $(LXC_GENERATED)
> +
> LXC_DRIVER_SOURCES = \
> + $(LXC_PROTOCOL_GENERATED) \
> + $(LXC_MONITOR_GENERATED) \
> lxc/lxc_conf.c lxc/lxc_conf.h \
> lxc/lxc_container.c lxc/lxc_container.h \
> lxc/lxc_cgroup.c lxc/lxc_cgroup.h \
> @@ -357,6 +405,8 @@ LXC_DRIVER_SOURCES = \
> lxc/lxc_driver.c lxc/lxc_driver.h
>
> LXC_CONTROLLER_SOURCES = \
> + $(LXC_PROTOCOL_GENERATED) \
> + $(LXC_CONTROLLER_GENERATED) \
> lxc/lxc_conf.c lxc/lxc_conf.h \
> lxc/lxc_container.c lxc/lxc_container.h \
> lxc/lxc_cgroup.c lxc/lxc_cgroup.h \
> diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c
> index e39c236..7fcc953 100644
> --- a/src/lxc/lxc_controller.c
> +++ b/src/lxc/lxc_controller.c
...
> @@ -1219,6 +1267,79 @@ virLXCControllerSetupConsoles(virLXCControllerPtr ctrl,
> }
>
>
> +static void
> +virLXCControllerEventSend(virLXCControllerPtr ctrl,
> + int procnr,
> + xdrproc_t proc,
> + void *data)
> +{
> + virNetMessagePtr msg;
> +
> + if (!ctrl->client)
> + return;
> +
> + VIR_DEBUG("Send event %d client=%p", procnr, ctrl->client);
> + if (!(msg = virNetMessageNew(false)))
> + goto cleanup;
> +
> + msg->header.prog = virNetServerProgramGetID(ctrl->prog);
> + msg->header.vers = virNetServerProgramGetVersion(ctrl->prog);
> + msg->header.proc = procnr;
> + msg->header.type = VIR_NET_MESSAGE;
> + msg->header.serial = 1;
> + msg->header.status = VIR_NET_OK;
> +
> + if (virNetMessageEncodeHeader(msg) < 0)
> + goto cleanup;
> +
> + if (virNetMessageEncodePayload(msg, proc, data) < 0)
> + goto cleanup;
> +
> + VIR_DEBUG("Queue event %d %zu", procnr, msg->bufferLength);
> + virNetServerClientSendMessage(ctrl->client, msg);
> +
> + xdr_free(proc, data);
> + return;
> +
> +cleanup:
I'd rename this label as "error" as it is only ever called in error path.
> + virNetMessageFree(msg);
> + xdr_free(proc, data);
> +}
...
> diff --git a/src/lxc/lxc_protocol.x b/src/lxc/lxc_protocol.x
> new file mode 100644
> index 0000000..4fdbe34
> --- /dev/null
> +++ b/src/lxc/lxc_protocol.x
> @@ -0,0 +1,21 @@
> +/* -*- c -*-
> + * Define wire protocol for communication between the
> + * LXC driver in libvirtd, and the LXC controller in
> + * the libvirt_lxc helper program.
> + */
> +
> +enum virLXCProtocolExitStatus {
> + VIR_LXC_PROTOCOL_EXIT_STATUS_ERROR,
> + VIR_LXC_PROTOCOL_EXIT_STATUS_SHUTDOWN
> +};
> +
> +struct virLXCProtocolExitEventMsg {
> + enum virLXCProtocolExitStatus status;
> +};
> +
> +const VIR_LXC_PROTOCOL_PROGRAM = 0x12341234;
Hopefully nobody will find anything offending in this constant :-)
> +const VIR_LXC_PROTOCOL_PROGRAM_VERSION = 1;
> +
> +enum virLXCProtocolProcedure {
> + VIR_LXC_PROTOCOL_PROC_EXIT_EVENT = 1 /* skipgen skipgen */
> +};
ACK with the nits addressed.
Jirka
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list