On Tue, Sep 17, 2019 at 05:52:24PM +0100, Ian Jackson wrote:
> Anthony PERARD writes ("[PATCH 06/35] libxl: Use ev_qmp for
> switch_qemu_xen_logdirty"):
> > Signed-off-by: Anthony PERARD <anthony.per...@citrix.com>
> ...
> > + rc = libxl__ev_time_register_rel(ao, &lds->timeout,
> > + switch_logdirty_timeout, 10 * 1000);
> > + if (rc) goto out;
> > +
> > + qmp->ao = ao;
> > + qmp->domid = domid;
> > + qmp->payload_fd = -1;
> > + qmp->callback = switch_qemu_xen_logdirty_done;
> > + libxl__qmp_param_add_bool(gc, &args, "enable", enable);
> > + rc = libxl__ev_qmp_send(gc, qmp, "xen-set-global-dirty-log", args);
>
> I hate to suggest this at this stage, but: maybe the timeout could be
> incorporated into libxl__ev_qmp ?
>
> I think every libxl__qmp caller is going to need a timeout ?
Yes, every user of libxl__ev_qmp should have a timeout set up. But
there are different way to set it up. When we have several command to
send via QMP should we have a new timeout for every command or set only
one when sending the first command, and only stopping the timeout when
the last command's response have been received? In this patch series
I've choose the second option.
I can see several cases:
- One QMP command
-> easy, one timeout for it.
- several commands, related or not.
-> one timeout per command? or one timeout which cover all of them?
one per commands means that a bad qemu could delay the operation
several time longer than if there were a single timeout covering
all the commands.
- other slow operation are done beside a qmp command
-> In this case, it might not be practical to have a timeout attach
to the qmp command, we are going to need a timeout for the other
operations.
I guess we could try to optimise the simpler case when there is only one
qmp operation, because that's the most common, but allow to send
commands without having libxl__qmp setting a timeout.
--
Anthony PERARD
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
