Re: [Xen-devel] [PATCH v3 14/31] libxl_qmp_ev: Introduce libxl__ev_qmp_start() to connect to QMP

[Ian Jackson <ian.jackson@xxxxxxxxxx>]

Anthony PERARD writes ("Re: [PATCH v3 14/31] libxl_qmp_ev: Introduce 
libxl__ev_qmp_start() to connect to QMP"):
> So what the interface looks like at the end of the series is:
> 
> void libxl__ev_qmp_init(libxl__ev_qmp *ev);
> int libxl__ev_qmp_register(libxl__gc *gc, libxl__ev_qmp *ev,
>                            libxl__ev_qmp_callback *,
>                            uint32_t domid,
>                            const char *cmd, libxl__json_object *args);
> void libxl__ev_qmp_deregister(libxl__gc *gc, libxl__ev_qmp *ev);
> int libxl__ev_qmp_isregistered(const libxl__ev_qmp *ev);
> 
> The libxl__qmp_state is not exposed. The _register() will attempt to
> find the current _state and use it, or create a new one (connect).

Ah, I see.

Err, the global (ctx) state is definitely wrong then.  Multiple
operations might be in flight at once with the same ctx[1], so libxl
might need to be connected to multiple qemus (or connected to the same
qemu multiple times, if you don't want to deal with demultiplexing).

If you intend for each _ev to handle only one command at a time, and
map commands 1:1 to connections, then the connection state needs to be
in your ev structure.

TBH the API you quote about does seem very similar to the other
libxl_ev_FOO but it is very counterintuitive that "register" is the
function to send a qmp command.  I think some better names would help,
even if it means they are less regular.

[1] The ctx is not shared between threads, but it is shared between
independent aos etc.

> I'll try to write better documentation about the possible states of both
> libxl__ev_qmp_state and libxl__ev_qmp, and how they relate to each
> other.

That would really help, although I think what I am saying above
may imply that they need to be unified.

> BTW, I think qmp is kind of similair to libxl__ev_xswatch, which would
> _ev_fd_register(xenstore_fd) on the first path to watch, and _deregister
> once there is no more watches. The one more thing that qmp need to do is
> open the socket and close it.

If you are sharing the qmp connection between multiple concurrent qmp
commands then I think you need a data structure in the ctx which can
find an existing connection given a domid.

Ian.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel