Re: [Xen-devel] [RFC v2 8/9] HACK libxl_exec: Check QEMU status via QMP instead of xenstore

[Ian Jackson]

Anthony PERARD writes ("Re: [RFC v2 8/9] HACK libxl_exec: Check QEMU status via 
QMP instead of xenstore"):
> But, with the to be release QEMU 2.12, there is a new interface that
> allow to pre-open a socket:
>   -chardev socket,fd=?
> See 
> https://git.qemu.org/?p=qemu.git;a=commit;h=0935700f8544033ebbd41e1f13cd528f8a58d24d
> 
> With that, we could open our usual "/var/run/xen/qmp-libxl-$domid"
> socket for QEMU. That would work when the retricted fonctionnality is
> also available. But that cann't be use unconditionnaly.

Right.

It's a bit ugly having two methods but that's the price of
compatibility.

Ian.

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

Re: [Xen-devel] [RFC v2 8/9] HACK libxl_exec: Check QEMU status via QMP instead of xenstore

[Anthony PERARD]

On Fri, Apr 20, 2018 at 07:37:17PM +0100, Ian Jackson wrote:
> Anthony PERARD writes ("[RFC v2 8/9] HACK libxl_exec: Check QEMU status via 
> QMP instead of xenstore"):
> > When QEMU is restricted, the qemu on the receiving side cann't write
> > anything to xenstore once the migration is started. So it cann't tell
> > libxl that it is ready to continue running the guest.
> ...
> > This patch creates a pipe, give the write-end to qemu, and wait for
> > something to be written to it. (We could check if it is actually the QMP
> > greeting message.)
> 
> This is indeed the kind of thing I had in mind in our IRL
> conversation.

Good, where are heading in the right direction.

> > QEMU is asked to setup a QMP server on this pipe, but even if it is a
> > one-way only, qemu will write the QMP greeting message to the pipe.
> > This is done with:
> > -add-fd, to create a fdset which is use later.
> > -chardev 'file,path=/dev/fdset/1,append=true', this open a char device
> > on the write-end of the pipe, tell qemu that the FD is write-only, and
> > not to run truncate on it.
> > -mon, just start the QMP server on this new chardev.
> 
> Have you considered socketpair() ?  That avoids the oddnes of passing
> qemu a write-only fd.

I did considered to pass an open socket to QEMU, I didn't know that
socketpair() existed. But I don't think that would help with the current
version of QEMU (2.11 and older). There are only two ways to give a
file descriptor to qemu, these are:
  -chardev pipe,..
  -chardev file,...
And in both case, QEMU is only going to write to the fd anyway.

But, with the to be release QEMU 2.12, there is a new interface that
allow to pre-open a socket:
  -chardev socket,fd=?
See 
https://git.qemu.org/?p=qemu.git;a=commit;h=0935700f8544033ebbd41e1f13cd528f8a58d24d

With that, we could open our usual "/var/run/xen/qmp-libxl-$domid"
socket for QEMU. That would work when the retricted fonctionnality is
also available. But that cann't be use unconditionnaly.

-- 
Anthony PERARD

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

Re: [Xen-devel] [RFC v2 8/9] HACK libxl_exec: Check QEMU status via QMP instead of xenstore

[Ian Jackson]

Anthony PERARD writes ("[RFC v2 8/9] HACK libxl_exec: Check QEMU status via QMP 
instead of xenstore"):
> When QEMU is restricted, the qemu on the receiving side cann't write
> anything to xenstore once the migration is started. So it cann't tell
> libxl that it is ready to continue running the guest.
...
> This patch creates a pipe, give the write-end to qemu, and wait for
> something to be written to it. (We could check if it is actually the QMP
> greeting message.)

This is indeed the kind of thing I had in mind in our IRL
conversation.

> QEMU is asked to setup a QMP server on this pipe, but even if it is a
> one-way only, qemu will write the QMP greeting message to the pipe.
> This is done with:
> -add-fd, to create a fdset which is use later.
> -chardev 'file,path=/dev/fdset/1,append=true', this open a char device
> on the write-end of the pipe, tell qemu that the FD is write-only, and
> not to run truncate on it.
> -mon, just start the QMP server on this new chardev.

Have you considered socketpair() ?  That avoids the oddnes of passing
qemu a write-only fd.

Anyway, I'm not sure why this approach justifies HACK.  Are all the
things you are asking qemu to do not fully supported ?

> This patch copies most of "xswait" and call it "qmpwait". This is
> probably not the best way forward due to duplication.

Ah.  Indeed :-).

I haven't reviewed the code yet, only your description.

Ian.

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

Re: [Xen-devel] [RFC v2 8/9] HACK libxl_exec: Check QEMU status via QMP instead of xenstore

[Anthony PERARD]

On Mon, Apr 16, 2018 at 06:32:26PM +0100, Anthony PERARD wrote:
> When QEMU is restricted, the qemu on the receiving side cann't write
> anything to xenstore once the migration is started. So it cann't tell
> libxl that it is ready to continue running the guest.
> 
> In order to find out if QEMU is ready, we can issue QMP commands and
> check if it respond.
> 
> But there is no QMP socket to connect to before qemu is started. But we
> can uses different facility from qemu in order to setup some kind of
> callback before starting QEMU. For that, we open a file descriptor and
> give it to qemu.
> 
> This patch creates a pipe, give the write-end to qemu, and wait for
> something to be written to it. (We could check if it is actually the QMP
> greeting message.)
> 
> QEMU is asked to setup a QMP server on this pipe, but even if it is a
> one-way only, qemu will write the QMP greeting message to the pipe.
> This is done with:
> -add-fd, to create a fdset which is use later.
> -chardev 'file,path=/dev/fdset/1,append=true', this open a char device
> on the write-end of the pipe, tell qemu that the FD is write-only, and
> not to run truncate on it.
> -mon, just start the QMP server on this new chardev.
> 
> With that, qemu will start the QMP server on the write-only fd, which is
> enough to have the QMP greeting message. At this point, the QMP socket
> is ready, and I think qemu is in the main-loop and ready to start the
> emulation and respond to QMP commands.
> 
> This patch calls 'query-status', any response to that without error
> means that QEMU is ready. If the status is "running", QEMU would already
> have written the xenstore node if it could and is doing emulation.
> (Any subsequent QMP command 'cont', libxl__qmp_resume(),  is not
> changing anything.  If one adds '-S' to the QEMU command line, QEMU will
> have the status "prelaunch" as a response to 'query-status', then QEMU
> can be asked to start emulation with 'cont' via QMP.)
> 
> This patch copies most of "xswait" and call it "qmpwait". This is
> probably not the best way forward due to duplication.
> 
> Signed-off-by: Anthony PERARD 
> ---

This email should have:

Changes in RFC v2:
- Have QEMU throw a event instead of polling on connect() to the QMP
  socket

-- 
Anthony PERARD

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

Re: [Xen-devel] [RFC v2 8/9] HACK libxl_exec: Check QEMU status via QMP instead of xenstore

[Anthony PERARD]

On Mon, Apr 16, 2018 at 06:32:26PM +0100, Anthony PERARD wrote:
> -add-fd, to create a fdset which is use later.
> -chardev 'file,path=/dev/fdset/1,append=true', this open a char device
> on the write-end of the pipe, tell qemu that the FD is write-only, and
> not to run truncate on it.
> -mon, just start the QMP server on this new chardev.

There might be a better way than this. But the new way only exist in
2.12, and is not documented... (in --help).

With QEMU commit 0935700f8544033ebbd41e1f13cd528f8a58d24d (char: allow
passing pre-opened socket file descriptor at startup).

-- 
Anthony PERARD

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

[Xen-devel] [RFC v2 8/9] HACK libxl_exec: Check QEMU status via QMP instead of xenstore

[Anthony PERARD]

When QEMU is restricted, the qemu on the receiving side cann't write
anything to xenstore once the migration is started. So it cann't tell
libxl that it is ready to continue running the guest.

In order to find out if QEMU is ready, we can issue QMP commands and
check if it respond.

But there is no QMP socket to connect to before qemu is started. But we
can uses different facility from qemu in order to setup some kind of
callback before starting QEMU. For that, we open a file descriptor and
give it to qemu.

This patch creates a pipe, give the write-end to qemu, and wait for
something to be written to it. (We could check if it is actually the QMP
greeting message.)

QEMU is asked to setup a QMP server on this pipe, but even if it is a
one-way only, qemu will write the QMP greeting message to the pipe.
This is done with:
-add-fd, to create a fdset which is use later.
-chardev 'file,path=/dev/fdset/1,append=true', this open a char device
on the write-end of the pipe, tell qemu that the FD is write-only, and
not to run truncate on it.
-mon, just start the QMP server on this new chardev.

With that, qemu will start the QMP server on the write-only fd, which is
enough to have the QMP greeting message. At this point, the QMP socket
is ready, and I think qemu is in the main-loop and ready to start the
emulation and respond to QMP commands.

This patch calls 'query-status', any response to that without error
means that QEMU is ready. If the status is "running", QEMU would already
have written the xenstore node if it could and is doing emulation.
(Any subsequent QMP command 'cont', libxl__qmp_resume(),  is not
changing anything.  If one adds '-S' to the QEMU command line, QEMU will
have the status "prelaunch" as a response to 'query-status', then QEMU
can be asked to start emulation with 'cont' via QMP.)

This patch copies most of "xswait" and call it "qmpwait". This is
probably not the best way forward due to duplication.

Signed-off-by: Anthony PERARD 
---
 tools/libxl/libxl_aoutils.c  | 156 +++
 tools/libxl/libxl_dm.c   |  44 --
 tools/libxl/libxl_exec.c |  28 +--
 tools/libxl/libxl_internal.h |  30 +++
 4 files changed, 246 insertions(+), 12 deletions(-)

diff --git a/tools/libxl/libxl_aoutils.c b/tools/libxl/libxl_aoutils.c
index 9e493cd487..7d654996c3 100644
--- a/tools/libxl/libxl_aoutils.c
+++ b/tools/libxl/libxl_aoutils.c
@@ -626,6 +626,162 @@ void libxl__kill(libxl__gc *gc, pid_t pid, int sig, const 
char *what)
 what, (unsigned long)pid, sig);
 }
 
+/*- qmpwait -*/
+
+static libxl__ev_fd_callback qmpwait_fd_read_callback;
+static libxl__ev_time_callback qmpwait_timeout_callback;
+static void qmpwait_report_error(libxl__egc*, libxl__qmpwait_state*, int rc);
+
+void libxl__qmpwait_init(libxl__qmpwait_state *qmpwa)
+{
+libxl__ev_time_init(>time_ev);
+libxl__ev_fd_init(>notify_efd);
+}
+
+void libxl__qmpwait_stop(libxl__gc *gc, libxl__qmpwait_state *qmpwa)
+{
+libxl__ev_time_deregister(gc, >time_ev);
+libxl__ev_fd_deregister(gc, >notify_efd);
+libxl__carefd_close(qmpwa->notify_cfd);
+qmpwa->notify_cfd = NULL;
+}
+
+bool libxl__qmpwait_inuse(const libxl__qmpwait_state *qmpwa)
+{
+bool time_inuse = libxl__ev_time_isregistered(>time_ev);
+bool watch_inuse = libxl__ev_fd_isregistered(>notify_efd);
+assert(time_inuse == watch_inuse);
+return time_inuse;
+}
+
+int libxl__qmpwait_start(libxl__gc *gc, libxl__qmpwait_state *qmpwa)
+{
+int rc;
+
+rc = libxl__ev_time_register_rel(qmpwa->ao, >time_ev,
+ qmpwait_timeout_callback, 
qmpwa->timeout_ms);
+if (rc) goto err;
+
+rc = libxl__ev_fd_register(gc, >notify_efd,
+   qmpwait_fd_read_callback,
+   libxl__carefd_fd(qmpwa->notify_cfd),
+   POLLIN);
+if (rc) goto err;
+
+return 0;
+
+ err:
+libxl__qmpwait_stop(gc, qmpwa);
+return rc;
+}
+
+static void qmpwait_fd_read_callback(libxl__egc *egc, libxl__ev_fd *ev,
+ int fd, short events, short revents)
+{
+// checkout:
+// - datacopier_readable
+// - watchfd_callback
+
+libxl__qmpwait_state *qmpwa = CONTAINER_OF(ev, *qmpwa, notify_efd);
+libxl__spawn_state *ss = CONTAINER_OF(ev, *ss, qmpwait.notify_efd);
+STATE_AO_GC(qmpwa->ao);
+
+// need to handle POLLERR
+// POLLHUP and POLLIN might be both set.
+if (revents & POLLHUP && !(revents & POLLIN)) {
+LOG(DEBUG, "received POLLHUP on fd %d: read for %s",
+fd, qmpwa->what);
+libxl__ev_fd_deregister(gc, >notify_efd);
+if (!qmpwa->life_time_read)
+ss->failure_cb(egc, ss, ERROR_FAIL);
+return;
+}
+if (revents & ~(POLLIN|POLLHUP)) {
+LOG(ERROR, "unexpected poll event 0x%x on fd %d (expected POLLIN "
+"and/or POLLHUP) reading %s",
+