On Thu, May 08, 2025 at 07:51:29AM +0200, Peter Krempa wrote:
> On Wed, May 07, 2025 at 16:38:38 -0700, Matthew R. Ochs via Devel wrote:
> > diff --git
> > a/tests/qemuxmlconfdata/aarch64-nousb-minimal.aarch64-latest.abi-update.args
> >
> > b/tests/qemuxmlconfdata/aarch64-nousb-minimal.aarch64-latest.abi-update.args
> > index a3d0f7a4fcf5..24f770b92865 100644
> > ---
> > a/tests/qemuxmlconfdata/aarch64-nousb-minimal.aarch64-latest.abi-update.args
> > +++
> > b/tests/qemuxmlconfdata/aarch64-nousb-minimal.aarch64-latest.abi-update.args
> > @@ -10,7 +10,7 @@
> > XDG_CONFIG_HOME=/var/lib/libvirt/qemu/domain--1-aarch64test/.config \
> > -name guest=aarch64test,debug-threads=on \
> > -S \
> > -object
> > '{"qom-type":"secret","id":"masterKey0","format":"raw","file":"/var/lib/libvirt/qemu/domain--1-aarch64test/master-key.aes"}'
> > \
> > --machine borzoi,usb=off,dump-guest-core=off \
> > +-machine collie,usb=off,dump-guest-core=off \
> > -accel kvm \
> > -cpu host \
> > -m size=1048576k \
>
> I presume you did a s/borzoi/collie herer, without running the test
> suite?
They seems to have gone further than that: earlier in the diff you
can spot:
> diff --git a/tests/qemucapabilitiesdata/caps_8.2.0_aarch64.replies
> b/tests/qemucapabilitiesdata/caps_8.2.0_aarch64.replies
> index 203774ecff..ed72c6fdfa 100644
> --- a/tests/qemucapabilitiesdata/caps_8.2.0_aarch64.replies
> +++ b/tests/qemucapabilitiesdata/caps_8.2.0_aarch64.replies
> @@ -27289,7 +27289,7 @@
> "parent": "sys-bus-device"
> },
> {
> - "name": "borzoi-machine",
> + "name": "collie-machine",
> "parent": "spitz-common"
> },
> {
> --- a/tests/qemucapabilitiesdata/caps_8.2.0_aarch64.xml
> +++ b/tests/qemucapabilitiesdata/caps_8.2.0_aarch64.xml
> @@ -240,7 +240,7 @@
> <machine type='kvm' name='qcom-dc-scm-v1-bmc' maxCpus='2'
> defaultRAMid='ram' acpi='no'/>
> <machine type='kvm' name='mori-bmc' maxCpus='2'
> defaultCPU='cortex-a9-arm-cpu' defaultRAMid='ram' acpi='no'/>
> <machine type='kvm' name='ast2600-evb' maxCpus='2' defaultRAMid='ram'
> acpi='no'/>
> - <machine type='kvm' name='borzoi' maxCpus='1'
> defaultCPU='pxa270-c0-arm-cpu' acpi='no'/>
> + <machine type='kvm' name='collie' maxCpus='1'
> defaultCPU='pxa270-c0-arm-cpu' acpi='no'/>
> <machine type='kvm' name='tiogapass-bmc' maxCpus='1' defaultRAMid='ram'
> acpi='no'/>
> <machine type='kvm' name='spitz' maxCpus='1'
> defaultCPU='pxa270-c0-arm-cpu' acpi='no'/>
> <machine type='kvm' name='virt-2.7' maxCpus='255'
> defaultCPU='cortex-a15-arm-cpu' numaMemSupported='yes'
> defaultRAMid='mach-virt.ram' acpi='yes'/>
which seems to indicate that they have just applied a naive
s/borzoi/collie/ without considering the semantics. With this patch
applied:
$ grep collie tests/qemucapabilitiesdata/caps_8.2.0_aarch64.xml
<machine type='kvm' name='collie' maxCpus='1'
defaultCPU='pxa270-c0-arm-cpu' acpi='no'/>
<machine type='kvm' name='collie' maxCpus='1'
defaultCPU='sa1110-arm-cpu' defaultRAMid='strongarm.sdram' acpi='no'/>
<machine type='tcg' name='collie' maxCpus='1'
defaultCPU='pxa270-c0-arm-cpu' acpi='no'/>
<machine type='tcg' name='collie' maxCpus='1'
defaultCPU='sa1110-arm-cpu' defaultRAMid='strongarm.sdram' acpi='no'/>
i.e. the collie machine shows up twice, with completely different
attributes associated with it each time, in the resulting
capabilities data. This obviously will not make any sense to a user
or application parsing the information.
> When running, the commandline would change a bit more:
>
> +-machine collie,usb=off,dump-guest-core=off,memory-backend=strongarm.sdram \
>
> and also this needed to be added.
>
> +-object
> '{"qom-type":"memory-backend-ram","id":"strongarm.sdram","size":1073741824}' \
That was all present in my patch from earlier this year[1], which
this one is clearly based on.
And "based on" is a very generous term in this case, since what the
author appears to have done is take it outright, leaving the commit
message completely unchanged, ripping out the original authorship and
reposting it as their own - with some fresh new bugs sprinkled in for
good measure. Which needless to say is not just highly questionable
from a legal standpoint but also just deeply, deeply uncool.
I'm going to assume that this was done with no ill intent behind it,
especially since by CCing me to the series they did the exact
opposite of trying to cover their tracks ;)
Still, this contribution is completely unacceptable due to both
technical and legal factors and should be disregarded.
> Since these hunks are present in 2/2 you've probably forgot to commit
> them here :)
>
> Anyways I'll fix this.
>
> Reviewed-by: Peter Krempa <pkre...@redhat.com>
100% NACK. Please don't push any of this.
[1]
https://lists.libvirt.org/archives/list/devel@lists.libvirt.org/message/2PIQSKIV3E3B2W6YZQ3O5RHEK235OTBK/
--
Andrea Bolognani / Red Hat / Virtualization
