Re: [PATCH 0/4] migration: Remove extra type-checking from vmstate macros
Fabiano Rosas <[email protected]>
| Newsgroups | gmane.comp.emulators.qemu |
|---|---|
| Message-ID | <[email protected]> |
Peter Xu <[email protected]> writes: > On Wed, Jul 29, 2026 at 07:52:23PM -0300, Fabiano Rosas wrote: >> Hi, this is basically what I ranted about in: >> https://lore.kernel.org/r/[email protected] >> >> I'm replacing the per-integer-size type checks with a single "int that >> fits in 32bit" check. This allows several lines of duplicated code to >> be removed. >> >> I haven't changed the macro names in the device code yet. If this >> series gets positive feedback then I'll send per-subsystem patches >> doing that. >> >> CI run: https://gitlab.com/farosas/qemu/-/pipelines/2716814081 >> Also tested: >> - migration-test --full --thorough >> - x86_64 compat run forwards and backwards for previous 3 QEMU releases >> - s390x compat run forwards and backwards for previous 2 QEMU releases >> - ppc64 compat run forwards and backwards for previous QEMU release >> - migration-test smoke ASAN/UBSAN run >> >> Fabiano Rosas (4): >> migration: Remove VMSTATE_ARRAY_INT32_UNSAFE >> migration: Introduce VMStateOffset >> migration: Remove redundant flags >> migration: Remove duplicate vmstate macros > > Nice work! > > I think I was only looking at VBUFFER side and I thought it was fine > sticking with 32bit even signed or not, not a huge deal. But cleaning up > VARRAY whole thing together looks definitely an improvement. I definitely > like your version here. > > I assume with your series I can drop both of my patches here, right? > > [PATCH v2 1/5] migration: Fix possible overflow in vmstate_handle_alloc() > https://lore.kernel.org/r/[email protected] > (I'll still respin with the rest) > > [PATCH] vhost/migration: Fix incorrect size used in inflight->addr in VMSD > https://lore.kernel.org/r/[email protected] > > The only missing piece would be an multiply overflow check in > vmstate_handle_alloc(), if you could add that check too while rewritting > that in patch 1 then I think it'll cover all. > > Vladimir's ask in the separate email makes sense: I wonder if we can also > do one step further and merge VBUFFER into VARRAY. > > The other trivial thing is, while looking, I found one trivial macro > VMSTATE_PARTIAL_VBUFFER not used; can drop it altogether. I can look at it all. Just a heads-up, I'll be off for a couple of weeks starting tomorrow.