Re: [Xen-devel] [RFC PATCH 05/17] libxl: use xenstore for pci hotplug qemu-in-linux-stubdom commands

[Jason Andryuk <jandryuk@xxxxxxxxx>]

On Wed, Aug 1, 2018 at 12:03 PM Marek Marczykowski-Górecki
<marmarek@xxxxxxxxxxxxxxxxxxxxxx> wrote:
>
> On Wed, Aug 01, 2018 at 10:29:07AM -0400, Jason Andryuk wrote:
> > On Mon, Jul 30, 2018 at 11:56 PM, Marek Marczykowski-Górecki
> > <marmarek@xxxxxxxxxxxxxxxxxxxxxx> wrote:
> > > From: Simon Gaiser <simon@xxxxxxxxxxxxxxxxxxxxxx>
> > >
> > > There is no QMP socket access, re-use the same mechanism as for MiniOS
> > > based stubdom.
> >
> > Later you add some QMP support.  Is this preferred because your QMP
> > support is unreliable?
>
> For now, until final decision how to connect QMP is made, yes.
> This is also somethings we already use Qubes for almost a year, so it
> got much more testing.
>
> But if we get stable QMP support, this patch could be dropped.
>
> > <snip>
> >
> > > @@ -1010,7 +1011,15 @@ static int do_pci_add(libxl__gc *gc, uint32_t 
> > > domid, libxl_device_pci *pcidev, i
> > >                  rc = qemu_pci_add_xenstore(gc, domid, pcidev);
> > >                  break;
> > >              case LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN:
> > > -                rc = libxl__qmp_pci_add(gc, domid, pcidev);
> > > +                dm_domid = libxl_get_stubdom_id(ctx, domid);
> > > +
> > > +                if (dm_domid != 0
> > > +                    && libxl__stubdomain_version_running(gc, dm_domid) ==
> > > +                         LIBXL_STUBDOMAIN_VERSION_LINUX) {
> > > +                    rc = qemu_pci_add_xenstore(gc, domid, pcidev);
> > > +                } else {
> > > +                    rc = libxl__qmp_pci_add(gc, domid, pcidev);
> > > +                }
> >
> > What about refactoring the whole switch statement to something like:
> > bool use_qmp = libxl__device_model_version_running(gc, domid)
> >                  == LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN &&
> >                libxl_get_stubdom_id(ctx, domid) == 0;
> > if (use_qmp) {
> >    rc = libxl__qmp_pci_add(gc, domid, pcidev);
> > } else {
> >     rc = qemu_pci_add_xenstore(gc, domid, pcidev);
> > }
>
> That indeed looks better. But I'd delay touching this patch until
> getting some plan on QMP.

Ok.  That sounds good.

Regards,
Jason

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