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

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

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.
> 
> >                  break;
> >              default:
> >                  return ERROR_INVAL;
> 
> Regards,
> Jason

-- 
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