On Fri, Aug 14, 2026 at 07:56:42AM +0200, Philippe Mathieu-Daudé wrote: > Hi Peter, Michael, > > On 12/8/26 22:16, Michael Roth wrote: > > From: Peter Xu <[email protected]> > > > > So that there will be a verbal string returned when kvm not enabled, or > > kvm not compiled. > > > > Signed-off-by: Peter Xu <[email protected]> > > Reviewed-by: Xiaoyao Li <[email protected]> > > Reviewed-by: Fabiano Rosas <[email protected]> > > Reviewed-by: Michael Roth <[email protected]> > > Signed-off-by: Michael Roth <[email protected]> > > --- > > accel/kvm/kvm-all.c | 5 +++++ > > accel/stubs/kvm-stub.c | 1 + > > 2 files changed, 6 insertions(+) > > > > diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c > > index 120cab1e22..bda2e25a66 100644 > > --- a/accel/kvm/kvm-all.c > > +++ b/accel/kvm/kvm-all.c > > @@ -4758,6 +4758,11 @@ int kvm_create_guest_memfd(uint64_t size, uint64_t > > flags, Error **errp) > > .flags = flags, > > }; > > + if (!kvm_enabled()) { > > + error_setg(errp, "guest-memfd requires KVM accelerator"); > > + return -1; > > This doesn't sound right withing a KVM-specific method. We want to > assert() here. > > The call in ram_block_add() is already protected: > > 2147 static void ram_block_add(RAMBlock *new_block, Error **errp) > 2148 { > ... > 2185 if (new_block->flags & RAM_GUEST_MEMFD) { > 2188 if (!kvm_enabled()) { > 2189 error_setg(errp, "cannot set up private guest memory for > %s: KVM required", > 2190 object_get_typename(OBJECT(current_machine->cgs))); > 2191 goto out_free; > 2192 } > ... > 2203 new_block->guest_memfd = > kvm_create_guest_memfd(new_block->max_length, > 2204 0, errp); > > The other one is: > > 2823 int ram_block_rebind(Error **errp) > 2824 { > ... > 2829 RAMBLOCK_FOREACH(block) { > ... > 2834 block->guest_memfd = > kvm_create_guest_memfd(block->max_length, > 2835 0, errp); > > which is only called from KVM: > > 2776 static int kvm_reset_vmfd(MachineState *ms) > 2777 { > ... > 2827 /* rebind memory to new vm fd */ > 2828 ret = ram_block_rebind(&err); > > So maybe what we want is: > > -- >8 -- > diff --git a/system/physmem.c b/system/physmem.c > index b97016b1303..66ff74541aa 100644 > --- a/system/physmem.c > +++ b/system/physmem.c > @@ -2824,6 +2824,8 @@ int ram_block_rebind(Error **errp) > { > RAMBlock *block; > > + assert(kvm_enabled()); /* Only supported by KVM so far */ > + > qemu_mutex_lock_ramlist(); > > RAMBLOCK_FOREACH(block) { > --- > > Or less aggressive: > > -- >8 -- > diff --git a/system/physmem.c b/system/physmem.c > index b97016b1303..2988d1dd6c9 100644 > --- a/system/physmem.c > +++ b/system/physmem.c > @@ -2824,6 +2824,11 @@ int ram_block_rebind(Error **errp) > { > RAMBlock *block; > > + if (!kvm_enabled()) { > + error_setg(errp, "guest-memfd requires KVM accelerator"); > + return -1; > + } > + > qemu_mutex_lock_ramlist(); > > RAMBLOCK_FOREACH(block) { > --- > > WDYT?
Fine by me. IMHO it's normally more of an issue the other way round, if we used an assert() where we should use error_setg() (hence, user triggerable assert()s). Here we expect it to never happen, so either way should not happen.. If so, we could also assert() in ram_block_rebind(), as it's only used in kvm_reset_vmfd() only, so I don't see how it can be reached if KVM is not enabled first.. Thanks, -- Peter Xu
