From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from gate001.proxmox.com (gate001.proxmox.com [45.144.208.40]) by lore.proxmox.com (Postfix) with ESMTPS id C60561FF0AF for ; Thu, 10 Sep 2026 16:24:33 +0200 (CEST) Received: from gate001.proxmox.com (localhost.localdomain [127.0.0.1]) by gate001.proxmox.com (Proxmox) with ESMTP id 5DD172158E; Thu, 10 Sep 2026 16:24:30 +0200 (CEST) Message-ID: <26e7412a-c382-4ae2-8830-26741784a8c1@proxmox.com> Date: Thu, 10 Sep 2026 16:24:25 +0200 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH qemu-server 2/6] tests: hotplug: add some test cases To: Dominik Csapak , pve-devel@lists.proxmox.com References: <20260910110832.2822954-1-d.csapak@proxmox.com> <20260910110832.2822954-3-d.csapak@proxmox.com> Content-Language: en-US From: Fiona Ebner In-Reply-To: <20260910110832.2822954-3-d.csapak@proxmox.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1789050256679 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.639 Adjusted score from AWL reputation of From: address DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment (newer systems) RCVD_IN_DNSWL_MED -2.3 Sender listed at https://www.dnswl.org/, medium trust SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Message-ID-Hash: WKUGTMHRFIUC2IUO6BSNXBZHC76XV5QB X-Message-ID-Hash: WKUGTMHRFIUC2IUO6BSNXBZHC76XV5QB X-MailFrom: f.ebner@proxmox.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; loop; banned-address; emergency; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header X-Mailman-Version: 3.3.10 Precedence: list List-Id: Proxmox VE development discussion List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: 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?