[libvirt] [PATCH 0/2] conf: qemu: support new Hyper-V enlightenments in Qemu-3.1

[John Ferlan]

On 11/14/18 8:24 AM, Vitaly Kuznetsov wrote:
> John Ferlan <jferlan at redhat.com> writes:
> 
>> On 11/14/18 4:04 AM, Andrea Bolognani wrote:
>>> On Tue, 2018-11-13 at 18:53 +0100, Vitaly Kuznetsov wrote:
>>>> The upcoming Qemu-3.1 release will bring us two new Hyper-V enlightenments:
>>>> hv_ipi and hv_evmcs. Support these in libvirt.
>>>>
>>>> Vitaly Kuznetsov (2):
>>>>   conf: qemu: add support for Hyper-V PV IPIs
>>>>   conf: qemu: add support for Hyper-V Enlightened VMCS
>>>
>>> I have pointed out a few very minor issues with the patches, as
>>> well as a couple areas where pre-existing issues could be fixed,
>>> although I don't consider the latter a requirement for merging
>>> the series ;)
>>>
>>> One thing that I would like to see, though, is the change being
>>> documented properly in the release notes (docs/news.xml). Please
>>> include that and respin.
>>>
>>
>> One other thing that I've really tried to see done is splitting the
>> "conf" and "qemu" patches.
>>
>> That way one deals with conf, docs, conf, util, xml2xmltest, while the
>> other deals with the qemu specific changes qemu, xml2argv.
>>
>> Sometimes it's "harder" to do that - such as may be the case with
>> various switches when a new case is added; however, in that case the
>> added case in the conf/docs/etc patch would go with the "default:" or
>> "last" case and then be moved with the subsequent qemu patch.
>>
> 
> I have to admit my overall knowledge of libvirt pretty limited, probably
> because of that I'm failing to see benefits of doing such split for
> Qemu/KVM-only features (as the whole Hyper-V emulation is

Generally speaking it's to keep conf/docs/XML changes separate from
hypervisor specific to make bisecting easier and patches smaller for
reviews.

I see I didn't ask the last time (commit f4c39db736 for PV TLB flush
support in Aug 2018), so either I had a lapse there or I was just
feeling it wasn't that important since things are so tightly coupled. I
look at a lot of patches and just try to be consistent in asks.

> Qemu/KVM-only). How is the first patch going to be tested? Compile-only?

Since you've updated tests/qemuxml2xmloutdata/hyperv.xml (and the
output). That means the XML parsing is being tested. So that's testing
that the newly added XML/RNG field can be parsed and output as expected.
There are cases I've reviewed where the formatting was wrong, but there
was no xml2xml to test that.

> Also, which of these patches should src/cpu/* hunks go to?

I would think the conf/docs/xml2xml since it seems they are defining
supported bits.

> 
> Would realy appreciate some guidance here) Thanks!
> 

In the long run it may not matter much - I'm not here to NAK something
because the split isn't done. Again, I'm trying to be consistent in how
I look at things.

John