[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH 09/11] Run an RPC protocol over the LXC controller monitor



On Tue, Jul 24, 2012 at 14:22:51 +0100, Daniel P. Berrange wrote:
> From: "Daniel P. Berrange" <berrange redhat com>
> 
> 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 redhat com>
> ---
>  .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


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]