Re: [PATCH V6 1/2] libxl: Add support for Virtio disk configuration

[Oleksandr <olekstysh@xxxxxxxxx>]

On 21.12.21 18:46, Anthony PERARD wrote:


Hi Anthony, Juergen

> On Fri, Dec 17, 2021 at 06:50:02PM +0200, Oleksandr wrote:
> > On 17.12.21 17:26, Juergen Gross wrote:
> > > On 15.12.21 22:36, Oleksandr wrote:
> > > > On 15.12.21 17:58, Juergen Gross wrote:
> > > > > In practice we are having something like the "protocol" already today:
> > > > > the disk device name is encoding that ("xvd*" is a Xen PV disk, while
> > > > > "sd*" is an emulated SCSI disk, which happens to be presented to the
> > > > > guest as "xvd*", too). And this is an additional information not
> > > > > related to the backendtype.
> You mean in theory? ;-) In practice, xvd* is the same as hd* (or sd*?).
> I tried once to have xvd* mean PV disk only, but the patch was rejected.
> So at the moment, we always get an emulated disk, we can't have PV disk
> alone, at least on x86.
I guess this a question to Juergen?


> > > > > So we have basically the following configuration items, which are
> > > > > orthogonal to each other (some combinations might not make sense,
> > > > > but in theory most would be possible):
> > > > >
> > > > > 1. protocol: emulated (not PV), Xen (like today), virtio
> > > > >
> > > > > 2. backendtype: phy (blkback), qdisk (qemu), other (e.g. a daemon)
> > > > >
> > > > > 3. format: raw, qcow, qcow2, vhd, qed
> > > > >
> > > > > The combination virtio+phy would be equivalent to vhost, BTW. And
> > > > > virtio+other might even use vhost-user, depending on the daemon.
> > > > yes, BTW the combination virtio+other is close to our use-case.
> > > >
> > > >
> > > > Thank you for the detailed explanation, now I see your point why
> > > > using backendtype=virtio is not flexible option in the long term
> > > > and why we would want/need to an extra configuration option such as
> > > > protocol, etc. I think, it makes sense and would be correct.
> > > >
> > > > If we take a disk as an example, then from the configuration PoV we
> > > > will need to:
> > > > - add an optional "protocol" option
> > > I'm not sure regarding the name of the option. "protocol" was just a
> > > suggestion by me.
> > Yes, personally I would be fine with either "protocol" or "specification",
> > with a little preference for the former. What other people think of the
> > name?
> I don't have a better idea. "protocol" sound fine, as long as the description of
> this new field is about how a guest kernel will communicate with the
> backend.
yes, sure.


> We could start with "default" and "virtio-mmio" as options. "default" is
> what we have now and usually mean emulated+xen-pv.
ok, sounds good to me.


> > > > - add new backendtype: external/other/daemon/etc.
> > > > This seems to cover all possible combinations describe above
> > > > (although I agree that some of them might not make sense). Is my
> > > > understanding correct?
> > > Yes.
> > ok, thank you for confirming.
> >
> > > > Unfortunately, disk configuration/management code is spread over
> > > > multiple sources (including auto-generated) in the toolstack which
> > > > is not so easy to follow (at least to me
> > > > who is not familiar enough with all this stuff), but anyway may I
> > > > please clarify what is the minimum required amount of things that I
> > > > need to do in order to get this basic virtio-mmio
> > > > support series accepted?
> > > I'd say we should first get consensus that others agree with my
> > > suggestion.
> > This is fair. Personally I share your opinion (what you propose sounds
> > reasonable to me in general). Are there any other opinions? Any feedback
> > would be highly appreciated.
> The new proposed property sound good to me as well.
Great, thank you for the feedback.


> Thanks,
--
Regards,

Oleksandr Tyshchenko