On Tue, Jul 7, 2026 at 1:22 PM Albert Esteve <[email protected]> wrote: > > Use g_autoptr(VirtioShmemGuard) to automatically commit the memory > region transaction opened by virtio_add_shmem_map_start(), removing > explicit virtio_add_shmem_map_end() calls on each exit path of > vhost_user_backend_handle_shmem_map(). This also allows the > send_reply_commit label to be folded into send_reply since the two > blocks are now identical. > > VirtioShmemGuard is a sentinel type whose non-NULL value triggers > the cleanup function on scope exit via GLib's g_autoptr infrastructure, > similar to those used in main-loop.h and rcu.h.
This patch is now superseded by https://lore.kernel.org/all/[email protected]/ I will drop this patch from the series and send the single remaining patch in v2. > > Signed-off-by: Albert Esteve <[email protected]> > --- > hw/virtio/vhost-user.c | 21 ++++----------------- > hw/virtio/virtio.c | 4 +++- > include/hw/virtio/virtio.h | 16 ++++++++++++++-- > 3 files changed, 21 insertions(+), 20 deletions(-) > > diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c > index f6bf9e0ba4..0306b06b48 100644 > --- a/hw/virtio/vhost-user.c > +++ b/hw/virtio/vhost-user.c > @@ -1933,6 +1933,7 @@ vhost_user_backend_handle_shmem_map(struct vhost_dev > *dev, > VhostUserPayload *payload, > int fd) > { > + g_autoptr(VirtioShmemGuard) shmem_guard = NULL; > VirtioSharedMemory *shmem; > VhostUserMMap *vu_mmap = &payload->mmap; > VirtioSharedMemoryMapping *existing; > @@ -1977,7 +1978,7 @@ vhost_user_backend_handle_shmem_map(struct vhost_dev > *dev, > } > } > > - virtio_add_shmem_map_start(); > + shmem_guard = virtio_add_shmem_map_start(); > > /* Create VirtioSharedMemoryMapping object */ > VirtioSharedMemoryMapping *mapping = virtio_shared_memory_mapping_new( > @@ -1986,7 +1987,7 @@ vhost_user_backend_handle_shmem_map(struct vhost_dev > *dev, > > if (!mapping) { > ret = -EFAULT; > - goto send_reply_commit; > + goto send_reply; > } > > /* Add the mapping to the shared memory region */ > @@ -1994,22 +1995,8 @@ vhost_user_backend_handle_shmem_map(struct vhost_dev > *dev, > error_report("Failed to add shared memory mapping"); > object_unref(OBJECT(mapping)); > ret = -EFAULT; > - goto send_reply_commit; > - } > - > -send_reply_commit: > - /* Send reply and commit after transaction started */ > - if (hdr->flags & VHOST_USER_NEED_REPLY_MASK) { > - payload->u64 = !!ret; > - hdr->size = sizeof(payload->u64); > - if (!vhost_user_send_resp(ioc, hdr, payload, &local_err)) { > - error_report_err(local_err); > - virtio_add_shmem_map_end(); > - return -EFAULT; > - } > + goto send_reply; > } > - virtio_add_shmem_map_end(); > - return 0; > > send_reply: > if (hdr->flags & VHOST_USER_NEED_REPLY_MASK) { > diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c > index e604781ffe..1db800fcd7 100644 > --- a/hw/virtio/virtio.c > +++ b/hw/virtio/virtio.c > @@ -3226,9 +3226,11 @@ VirtioSharedMemoryMapping > *virtio_shared_memory_mapping_new(uint8_t shmid, > return mapping; > } > > -void virtio_add_shmem_map_start(void) > +VirtioShmemGuard *virtio_add_shmem_map_start(void) > { > memory_region_transaction_begin(); > + /* Anything non-NULL causes the cleanup function to be called */ > + return (VirtioShmemGuard *)(uintptr_t)1; > } > > void virtio_add_shmem_map_end(void) > diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h > index 9935452486..b9e67f789c 100644 > --- a/include/hw/virtio/virtio.h > +++ b/include/hw/virtio/virtio.h > @@ -104,6 +104,8 @@ enum virtio_device_endian { > #define TYPE_VIRTIO_SHARED_MEMORY_MAPPING "virtio-shared-memory-mapping" > OBJECT_DECLARE_SIMPLE_TYPE(VirtioSharedMemoryMapping, > VIRTIO_SHARED_MEMORY_MAPPING) > > +typedef void VirtioShmemGuard; > + > /** > * VirtioSharedMemoryMapping: > * @parent: Parent QOM object > @@ -396,17 +398,27 @@ VirtioSharedMemoryMapping > *virtio_shared_memory_mapping_new(uint8_t shmid, > * Begins a transaction to add a shared memory mapping. Call this before > adding > * a shared memory mapping when the commit must be delayed until after a > reply > * is sent to a vhost-user backend. > + * > + * Returns: Non-NULL guard that triggers cleanup on scope exit. > */ > -void virtio_add_shmem_map_start(void); > +VirtioShmemGuard *virtio_add_shmem_map_start(void); > > /** > * virtio_add_shmem_map_end() - End a shmem map transaction > * > * Completes the transaction started by virtio_add_shmem_map_start() and > makes > - * the mapping visible to the guest. > + * the mapping visible to the guest. Prefer using g_autoptr(VirtioShmemGuard) > + * over calling this directly. > */ > void virtio_add_shmem_map_end(void); > > +static inline void virtio_shmem_guard_cleanup(VirtioShmemGuard *guard) > +{ > + virtio_add_shmem_map_end(); > +} > + > +G_DEFINE_AUTOPTR_CLEANUP_FUNC(VirtioShmemGuard, virtio_shmem_guard_cleanup) > + > /** > * virtio_add_shmem_map() - Add a memory mapping to a shared region > * @shmem: VirtioSharedMemory region > -- > 2.54.0 >
