On Mon, May 11, 2020 at 09:13:42AM +0200, Peter Krempa wrote:
It would be great if you could describe that 'pvscsi' indeed
is the
vmware paravirtual scsi controller even in qemu, because I had to dig
through the code to figure it out.
Sure, just in the commit message? Or elsewhere too?
Can the 'pvscsi' device be compiled out using any upstream
way? If not
we don't really need a capability for it since qemu-1.5.3 is the oldest
supported qemu and all other versions support it as well.
As Daniel P. Berrangé mentioned, this can be configured out via Kconfig.
Plus there are actually some files in tests/qemucapabilitiesdata that do
not advertise that device (e.g., *.s390x.replies).
The remainder of the patch looks okay to me, but it's missing a
qemuxml2argvtest case for the new controller. It looks like the
controller is using PCI addressing so the rest should be okay.
Sure thing, I'll be happy to add a test. Do I just take one of the
existing tests and modify the XML and expected resultant command?
If you figure out that the capability is required, I'd prefer if
the
addition of the capability is in a separate patch.
Can do. Given the (hopefully, given the patches from earlier today)
impending arrival of merge requests, and the fact that I already have
to redo the patch (since new capabilities have since been added), my
current plan is to just post the new commits in a merge request.
(Don't worry, I won't actually file the MR until the CI system is in
place.)
Cheers,
Chris.