On Mon, Oct 05, 2020 at 17:40:48 -0400, Nico Pache wrote:
I'm not super familiar with the code base so followed the 2
autodeflate commits as a guide.
I had a few questions regarding your feedback.
> The patch is also missing test XML addition for the qemuxml2argvtest and
qemuxml2xmltest.
can you please explain the qemuxml2xmltest files. I don't get what they do.
qemuxml2argvtest takes the input file from qemuxml2argvdata/testname.xml
which is a VM xml file and invokes internals which generate the command
line which would be used to start qemu. This is then checked against
qemuxml2argvdata/testname.args
Similarly qemuxml2xmltest takes the same input file as the above test
(literally the same, from the qemuxml2argv directory) and parses it and
formats it back applying any internal validation and default filling.
The output is compared against qemuxml2xmloutdata/testname.xml
Note that you just have to provide the input file and invocation macro
and you can then run the testsuite instructing it to generate the
expected output files:
VIR_TEST_REGENERATE_OUTPUT=1 tests/qemuxml2argvtest
And commit the result after verifying that it has the new bits
formatted. (Yes the testsuite will fail on the first round.)
You can either provide your own input file, but it needs have certain
settings (such as path to emulator) right to work with the tests so it's
usually simpler to just copy and modify an appropriate existing test
input file.
The added bonus of the above is that those files are also automatically
checked for conformance with the XML schema.
Please note that any new tests added to the qemuxml2* test suite should
use the DO_TEST_CAPS_* (such as DO_TEST_CAPS_LATEST, which invokes the
test with capabilities of the latest version of x86_64 qemu) macros to
invoke it rather than the now obsolete DO_TEST (without CAPS) where you
are enumerating the qemu capability flags manually.
Also do I commit the additional testing files with the src/qemu
changes or
in a separate commit?
Either of them is fine. They just must apply, compile and pass the tests
after every commit.
Will changing *virtio-balloon-pci.free-page-reporting* to
*virtio-balloon.free-page-reporting* encompass all the possible
virtio-balloon options?
yup, that sounds reasonable
Thanks in advance!
--Nico