Re: [PATCH v5 09/21] libxl: add save/restore support for qemu-xen in stubdomain

[Jason Andryuk <jandryuk@xxxxxxxxx>]

On Thu, May 14, 2020 at 12:35 PM Ian Jackson <ian.jackson@xxxxxxxxxx> wrote:
>
> Jason Andryuk writes ("[PATCH v5 09/21] libxl: add save/restore support for 
> qemu-xen in stubdomain"):
> > From: Marek Marczykowski-Górecki <marmarek@xxxxxxxxxxxxxxxxxxxxxx>
> >
> > Rely on a wrapper script in stubdomain to attach FD 3/4 of qemu to
> > relevant consoles.
> >
> > Signed-off-by: Marek Marczykowski-Górecki <marmarek@xxxxxxxxxxxxxxxxxxxxxx>
> > Address TODO in dm_state_save_to_fdset: Only remove savefile for
> > non-stubdom.
> ...
> > +        if (is_stubdom) {
> > +            /* Linux stubdomain connects specific FD to 
> > STUBDOM_CONSOLE_RESTORE
> > +             */
> > +            flexarray_append(dm_args, "-incoming");
> > +            flexarray_append(dm_args, "fd:3");
>
> Would it be possible to use a different fixed fd number ?  Low numbers
> are particularly vulnerable to clashes with autoallocated numbers.
>
> I suggest randomly allocating one in the range [64,192>.  My random
> number generator picked 119.  So 118 and 119 ?

This makes sense and would be the easiest change.

> Also, why couldn't your wrapper script add this argument ?  If you do
> that there then there is one place that knows the fd number and a
> slightly less tortuous linkage between libxl and the script...

I like this idea, but there is a complication.  "-incoming" is only
added when performing a restore, so it cannot just be blindly added to
all qemu command lines in the stubdom.  Two options I see are to
either communicate a restore some other way (so the stubdom scripts
can add the appropriate option), or pass something though dm_args, but
let the script convert it into something usable.

There is "-incoming defer" where we can later specify
"migrate_incoming fd:119".  Another option is to `sed
s/defer/fd:119/`, but that is a little tricky since we need to look at
the preceding key to know if we should sed the second.  We could pass
only "-incoming" and require the stubdom script to modify that option.

I haven't tested any of this.

> It's not stated anywhere here that I can see but I think what is
> happening here is that your wrapper script knows the qemu savefile
> pathname and reads it directly.  Maybbe a comment would be
> worthwhile ?

The existing comment "Linux stubdomain connects specific FD to
STUBDOM_CONSOLE_RESTORE" is trying to state that.
STUBDOM_CONSOLE_RESTORE is defined as 2 for console 2 (/dev/hvc2), but
it is only a libxl_internal.h define.

Regards,
Jason