Re: [libvirt] [PATCH v4 2/5] libxl: add support for PVH

On Fri, Oct 19, 2018 at 08:06:18AM -0600, Jim Fehlig wrote: > On 10/19/18 3:11 AM, Daniel P. Berrangé wrote: >> On Thu, Oct 18, 2018 at 11:08:34AM -0600, Jim Fehlig wrote: >>> On 10/17/18 12:59 PM, Marek Marczykowski-Górecki wrote: >>>> On Sat, Oct 13, 2018 at 08:46:19AM -0600, Jim Fehlig wrote: >>>>> I had some couch time at the start of the weekend and was finally able to >>>>> try using this series with virt-install. As it turns out, reporting >>>>> duplicate <guest> blocks for <os_type> 'xen' is not quite right. Instead we >>>>> will want to report the additional <machine> under the existing 'xen' >>>>> <guest> blocks. >>>> >>>> Is that virt-install limitation? In that case, IMO virt-install should >>>> be fixed, instead of changing capabilities xml to match its limitations. >>> >>> Perhaps it is a virt-install limitation, but my suggestion was based more on >>> how the qemu driver reports the different machines >>> >>> <guest> >>> <os_type>hvm</os_type> >>> <arch name='x86_64'> >>> <wordsize>64</wordsize> >>> <emulator>/usr/bin/qemu-system-x86_64</emulator> >>> <machine maxCpus='255'>pc-i440fx-3.0</machine> >>> <machine maxCpus='288'>pc-q35-3.0</machine> >>> ... >>> </arch> >>> </guest> >>> >>> Compare that with reporting PV and PVH in different <guest> blocks, where >>> the <os_type> and <arch> are the same. It seems confusing from a consumers >>> POV >>> >>> <guest> >>> <os_type>xen</os_type> >>> <arch name='x86_64'> >>> <wordsize>64</wordsize> >>> <emulator>/usr/bin/qemu-system-x86_64</emulator> >>> <machine>xenpv</machine> >>> </arch> >>> </guest> >>> >>> <guest> >>> <os_type>xen</os_type> >>> <arch name='x86_64'> >>> <wordsize>64</wordsize> >>> <emulator>/usr/bin/qemu-system-x86_64</emulator> >>> <machine>xenpvh</machine> >>> </arch> >>> </guest> >>> >>> How should a consumer interpret that? Is the machine for os_type=xen, >>> arch=x86_64 a xenpv or a xenpvh? >> >> Yes, you are right - any pair of (os_type, arch) should be unique >> in the capabilities XML. So all machines should be reported in the >> same block. > > Our difficulty with that is xenpv and xenpvh machines have different > features. Marek pointed out that the qemu driver reports the "feature" > maxCpus as an attribute on the machine element, but we're hesitant to keep > adding attributes for each feature that is unique to a machine. > > Another option we discussed was reporting a superset of all features so that > e.g. (xen, x86_64) block would contain features supported by both PV and PVH > and then rejecting unsupported features when parsing domXML or starting the > VM. This option is rather distasteful. > > And we also have the option of adding VIR_DOMAIN_OSTYPE_XENPVH, which I've > shied away from but may be a better way to go in the end. Do you have any > suggestions we may have overlooked? Oooh, it looks like i've been mis-understanding PVH in all my previous reviews. I thought it was simply a "normal" Xen paravirtualized guest kernel. ie any 'pv' guest is also a valid 'pvh' guest. Looking at the docs https://wiki.xen.org/wiki/Xen_Project_Software_Overview#Guest_Types It appears I was wrong. It says a pvh guest kernel relies on hardware virt extensions for part of its work and paravirt for other parts. So really is a hybrid between pv and hvm.

With that in mind, we should indeed have a distinct OS type constant to express this.