Re: [PATCH qemu-server 2/6] tests: hotplug: add some test cases

[Fiona Ebner <f.ebner@proxmox.com>]

Am 10.09.26 um 1:29 PM schrieb Dominik Csapak:
> diff --git a/src/test/hotplug/aarch64/disk-add-default-machine.conf.expected b/src/test/hotplug/aarch64/disk-add-default-machine.conf.expected
> new file mode 100644
> index 00000000..b25f9a70
> --- /dev/null
> +++ b/src/test/hotplug/aarch64/disk-add-default-machine.conf.expected

Not a fan of having the expected result be structured like this. I would
rather have separate files (or JSON), but not just loose separation with
comments.

> @@ -0,0 +1,28 @@
> +# recorded actions
> +storage activate_volumes ["local:8006/vm-8006-disk-1.qcow2"]
> +qmp object-add {"id":"iothread-virtio1","qom-type":"iothread"}
> +qmp object-del {"id":"throttle-drive-virtio1"}
> +qmp object-add {"id":"throttle-drive-virtio1","limits":{},"qom-type":"throttle-group"}
> +qmp blockdev-add {"detect-zeroes":"on","discard":"ignore","driver":"throttle","file":{"cache":{"direct":true,"no-flush":false},"detect-zeroes":"on","discard":"ignore","driver":"qcow2","file":{"aio":"io_uring","cache":{"direct":true,"no-flush":false},"detect-zeroes":"on","discard":"ignore","driver":"file","filename":"/var/lib/vz/images/8006/vm-8006-disk-1.qcow2","node-name":"ef4c217a7f08cf59741edc1790023f5","read-only":false},"node-name":"ff4c217a7f08cf59741edc1790023f5","read-only":false},"node-name":"drive-virtio1","read-only":false,"throttle-group":"throttle-drive-virtio1"}
> +hmp device_add driver=virtio-blk-pci,drive=drive-virtio1,id=virtio1,bus=pcie.0,addr=0xb,iothread=iothread-virtio1,write-cache=on
> +
> +# hotplug errors

This one could live in the test definition already, similar to how it's
done for the cfg2cmd tests. Then expected failures are easily
recognizable directly.

> +
> +# differences to a freshly started VM

This should always be empty in absence of bugs. I know you used it to
show off the fix for the iothread one, but I'd rather have the test
runner check and enforce this directly and not record anything in the
expected outputs. Otherwise, we have these extra lines in all test cases
for no real benefit.

> +
> +# resulting config
> +arch: aarch64
> +bios: ovmf
> +bootdisk: scsi0
> +cores: 2
> +efidisk0: local:8006/vm-8006-disk-2.qcow2,efitype=4m
> +memory: 2048
> +name: hotplug
> +net0: virtio=A2:C0:43:77:08:A0,bridge=vmbr0
> +ostype: l26
> +scsi0: local:8006/vm-8006-disk-0.qcow2,size=32G
> +scsihw: virtio-scsi-pci
> +smbios1: uuid=7b10d7af-b932-4c66-b2c3-3996152ec465
> +sockets: 1
> +virtio1: local:8006/vm-8006-disk-1.qcow2,iothread=1,size=32G
> +vmgenid: c773c261-d800-4348-9f5d-167fadd53cf8

Would just recording and the changes be more sensible to avoid lots of
redundant information? That could also already live in the test
definition with the test runner checking that nothing additional is
changed. In most cases it will directly overlap with what is in
[PENDING], so you don't even need to specify any additional expected
changes.

What do you think?