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

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?