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

[Anthony PERARD <anthony.perard@xxxxxxxxxx>]

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.

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

We could start with "default" and "virtio-mmio" as options. "default" is
what we have now and usually mean emulated+xen-pv.

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

Thanks,

-- 
Anthony PERARD