On Mon, Oct 20, 2025 at 3:50 PM David Hildenbrand <[email protected]> wrote: > > > + * Returns: 0 on success, negative errno on failure > > + */ > > +static int > > +vhost_user_backend_handle_shmem_map(struct vhost_dev *dev, > > + QIOChannel *ioc, > > + VhostUserHeader *hdr, > > + VhostUserPayload *payload, > > + int fd) > > +{ > > + VirtioSharedMemory *shmem; > > + VhostUserMMap *vu_mmap = &payload->mmap; > > + VirtioSharedMemoryMapping *existing; > > + Error *local_err = NULL; > > + int ret = 0; > > + > > + if (fd < 0) { > > + error_report("Bad fd for map"); > > + ret = -EBADF; > > + goto send_reply; > > + } > > + > > + if (QSIMPLEQ_EMPTY(&dev->vdev->shmem_list)) { > > + error_report("Device has no VIRTIO Shared Memory Regions. " > > + "Requested ID: %d", vu_mmap->shmid); > > + ret = -EFAULT; > > + goto send_reply; > > + } > > + > > + shmem = virtio_find_shmem_region(dev->vdev, vu_mmap->shmid); > > + if (!shmem) { > > + error_report("VIRTIO Shared Memory Region at " > > + "ID %d not found or uninitialized", vu_mmap->shmid); > > + ret = -EFAULT; > > + goto send_reply; > > + } > > + > > + if ((vu_mmap->shm_offset + vu_mmap->len) < vu_mmap->len || > > + (vu_mmap->shm_offset + vu_mmap->len) > shmem->mr.size) { > > + error_report("Bad offset/len for mmap %" PRIx64 "+%" PRIx64, > > + vu_mmap->shm_offset, vu_mmap->len); > > + ret = -EFAULT; > > + goto send_reply; > > + } > > + > > + QTAILQ_FOREACH(existing, &shmem->mmaps, link) { > > + if (ranges_overlap(existing->offset, existing->len, > > + vu_mmap->shm_offset, vu_mmap->len)) { > > + error_report("VIRTIO Shared Memory mapping overlap"); > > + ret = -EFAULT; > > + goto send_reply; > > + } > > + } > > + > > + memory_region_transaction_begin(); > > My only comment would be whether the > memory_region_transaction_begin()/memory_region_transaction_commit() > should be hidden behind some > virtio_add_shmem_map_start()/virtio_add_shmem_map_end() helpers. > > Talking about memory regions in this function sounds odd given that it's > more an implementation detail hidden by other helpers. > > Then, we can also document why these functions exists, and what the > contract is for calling them.
I understand. I will send a follow up patch with this, and we can discuss the solution there. Thanks for giving it another spin! > > -- > Cheers > > David / dhildenb >
