On 14.02.25 16:35, Ani Sinha wrote:
On Mon, Feb 3, 2025 at 3:50 AM Alexander Graf <g...@amazon.com> wrote:
Hey Ani!

On 28.01.25 22:31, Ani Sinha wrote:


[...]


diff --git a/hw/core/machine.c b/hw/core/machine.c
index c23b399496..0eaf8aa3ba 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -34,6 +34,7 @@
   #include "hw/virtio/virtio-pci.h"
   #include "hw/virtio/virtio-net.h"
   #include "hw/virtio/virtio-iommu.h"
+#include "hw/misc/vmfwupdate.h"
   #include "audio/audio.h"

   GlobalProperty hw_compat_9_2[] = {
@@ -252,6 +253,7 @@ GlobalProperty hw_compat_2_8[] = {
       { "virtio-pci", "x-pcie-pm-init", "off" },
       { "cirrus-vga", "vgamem_mb", "8" },
       { "isa-cirrus-vga", "vgamem_mb", "8" },
+    {TYPE_VMFWUPDATE, "disable", "1"},

If vmfwupdate is opt-in anyway, why disable for older machines?
Some older machines does not have enough slots for new fw-cfg files.
For those machines, we cannot add this device. Unless we disable the
device, QEMU would crash on an assertion.
This particular case was caught because the CI pipeline was failing.
There might be other cases where we need to disable it but CI does not
have a test case for it.


Please find a better way to fix that. You can have better error propagation when fw-cfg fills up so it's clear that the problem is the additional vmfwupdate device. But silently changing the behavior from "Device does what I want it to do" to "Device is attached, but not functional" based on machine version is a big no-go. You *must* fail loudly, because you can not honor the user's wish (expose vmfwupdate).


Alex


Reply via email to