Re: [Xen-devel] [PATCH v4 12/16] libxl: use vchan for QMP access with Linux stubdomain

[Marek Marczykowski-Górecki <marmarek@xxxxxxxxxxxxxxxxxxxxxx>]

On Fri, Jan 24, 2020 at 02:58:56PM -0500, Rich Persaud wrote:
> On Jan 24, 2020, at 09:07, Jason Andryuk <jandryuk@xxxxxxxxx> wrote:
> > 
> > On Tue, Jan 21, 2020 at 6:46 PM Marek Marczykowski-Górecki
> > <marmarek@xxxxxxxxxxxxxxxxxxxxxx> wrote:
> > 
> >>>> +
> >>>> +    sdss->qmp_proxy_spawn.timeout_ms = LIBXL_DEVICE_MODEL_START_TIMEOUT 
> >>>> * 1000;
> >>>> +    sdss->qmp_proxy_spawn.midproc_cb = libxl__spawn_record_pid;
> >>>> +    sdss->qmp_proxy_spawn.confirm_cb = qmp_proxy_confirm;
> >>>> +    sdss->qmp_proxy_spawn.failure_cb = qmp_proxy_startup_failed;
> >>>> +    sdss->qmp_proxy_spawn.detached_cb = qmp_proxy_detached;
> >>>> +
> >>>> +    const int arraysize = 6;
> >>>> +    GCNEW_ARRAY(args, arraysize);
> >>>> +    args[nr++] = STUBDOM_QMP_PROXY_PATH;
> >>>> +    args[nr++] = GCSPRINTF("--state-path=%s", 
> >>>> sdss->qmp_proxy_spawn.xspath);
> >>>> +    args[nr++] = GCSPRINTF("%u", dm_domid);
> >>>> +    args[nr++] = GCSPRINTF("%s/device-model/%u/qmp-vchan", dom_path, 
> >>>> guest_domid);
> >>> Thinking of OpenXT"s qmp-helper, this path isn't useful.  But it is
> >>> for vchan-socket-proxy, so qmp-helper could just change to ignore it.
> >> For vchan we could use also a port number (and then it will encode it
> >> into a xenstore path). This is in fact how we use libvchan in Qubes. I
> >> opted for explicit path only because of lack of idea for some meaningful
> >> port number ;) But I'm open for suggestions.
> >> I guess that would be useful for Argo version then.
> > 
> > The argo version hard codes the port number, so it's not a command
> > line argument.  The port number would need to get passed to the
> > stubdom or it would need to be standardized.
> > 
> > I think the arguments for vchan-socket-proxy make sense.  Since it's
> > the one that's submitted upstream, it makes sense to use them.
> > 
> > Put another way, do we want this to support alternate implementations
> > for a qmp proxy?  Should the arguments be generic for that case?
> 
> 
> One advantage of the server+client approach of vchan-socket-proxy is the 
> absence of patches for Qemu.  OpenXT qmp-helper requires a Qemu patch for 
> Argo support.  If there was a qmp socket proxy with Argo support, unpatched 
> Qemu could be used with libxl and Argo access controls.
> 
> A generalized qmp-socket-proxy may be useful to other projects.  It would be 
> good if the design supported single-purpose (client or server) binaries, e.g. 
> common functions in a library shared by separate client and server source 
> files, with conditional compilation for vchan and Argo interfaces.

I don't think it's worth separating client and server sources in the
current shape. The whole file has less than 500 lines and majority of it
is the common code. After connection setup, data processing is
symmetric (the whole data_loop and its helper functions).

What may be worth doing, is adding a place to plug qemu->libxl data
filtering/sanitization. This data filtering should indeed be in a
separate source file and only linked into server binary.
I'm not yet sure what I'd like to filter data with. To be honest, I'm
a bit uncomfortable with processing untrusted data in C...
Does Argo have bindings for some other (memory safe) language? That
would be a strong argument to use Argo here exclusively.
Alternatively, I can think of delegating filtering to a separate process
(pass data over stdin/stdout pipes). That could be very flexible, but
could be also an overkill. Also: should such filter see data in both
directions? I think yes, to have some context what libxl could expect
(filter-out unexpected responses, responses not matching schema for a
particular message type etc).

-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?

Attachment: signature.asc
Description: PGP signature

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