Re: [PATCH v6 1/8] vhost-user: Add VirtIO Shared Memory map request

[Albert Esteve <aesteve@redhat.com>]

On Mon, Aug 18, 2025 at 9:37 AM David Hildenbrand <david@redhat.com> wrote:
>
>
> > +    /* Map the shared memory region */
> > +    mapped_addr = mmap(NULL, len, prot_flags, MAP_SHARED, fd, fd_offset);
> > +    if (mapped_addr == MAP_FAILED) {
> > +        error_report("Failed to map shared memory region: %s", strerror(errno));
> > +        return NULL;
> > +    }
> > +
> > +    /* Create the VhostUserShmemObject */
> > +    shmem_obj = VHOST_USER_SHMEM_OBJECT(
> > +        object_new(TYPE_VHOST_USER_SHMEM_OBJECT));
> > +
> > +    /* Set up object properties */
> > +    shmem_obj->shmid = shmid;
> > +    shmem_obj->fd = fd;
> > +    shmem_obj->fd_offset = fd_offset;
> > +    shmem_obj->shm_offset = shm_offset;
> > +    shmem_obj->len = len;
> > +    shmem_obj->flags = flags;
> > +    shmem_obj->mapped_addr = mapped_addr;
> > +
> > +    /* Create MemoryRegion as a child of this object */
> > +    mr = g_new0(MemoryRegion, 1);
> > +    g_string_printf(mr_name, "vhost-user-shmem-%d-%" PRIx64, shmid, shm_offset);
> > +
> > +    /* Initialize MemoryRegion with the mapped memory */
> > +    memory_region_init_ram_device_ptr(mr, OBJECT(shmem_obj), mr_name->str,
> > +                                      len, mapped_addr);
>
> Oh, why did we go from memory_region_init_ram_from_fd() back to
> memory_region_init_ram_device_ptr() in this series?
>
> I thought having a wrapper object around the memory region would have
> been sufficient to handle the race?

Ah true :facepalm: I was under the impression that I needed mmap for
the read-write
flags, as I went a bit of back and forth for the last version. I was wrong, as
from_fd allows flags as an argument.

Let me send a new quick iteration for this patch with this fixed (and
other small things I saw
myself). Thanks for checking.

>
> --
> Cheers
>
> David / dhildenb
>