From: "Michael S. Tsirkin" <mst@redhat.com>
To: Raphael Norwitz <raphael.norwitz@nutanix.com>
Cc: Peter Turschmid <peter.turschm@nutanix.com>,
raphael.s.norwitz@gmail.com, qemu-devel@nongnu.org
Subject: Re: [PATCH v2 2/3] Refactor vhost_user_set_mem_table functions
Date: Thu, 6 Feb 2020 03:21:42 -0500 [thread overview]
Message-ID: <20200206032007-mutt-send-email-mst@kernel.org> (raw)
In-Reply-To: <1579143426-18305-3-git-send-email-raphael.norwitz@nutanix.com>
On Wed, Jan 15, 2020 at 09:57:05PM -0500, Raphael Norwitz wrote:
> vhost_user_set_mem_table() and vhost_user_set_mem_table_postcopy() have
> gotten convoluted, and have some identical code.
>
> This change moves the logic populating the VhostUserMemory struct and
> fds array from vhost_user_set_mem_table() and
> vhost_user_set_mem_table_postcopy() to a new function,
> vhost_user_fill_set_mem_table_msg().
>
> No functionality is impacted.
>
> Signed-off-by: Raphael Norwitz <raphael.norwitz@nutanix.com>
> Signed-off-by: Peter Turschmid <peter.turschm@nutanix.com>
Looks ok, but just cosmetics: let's have the flag say what
it does, not who uses it.
So s/postcopy/track_ramblocks/ ?
> ---
> hw/virtio/vhost-user.c | 143 +++++++++++++++++++++++--------------------------
> 1 file changed, 67 insertions(+), 76 deletions(-)
>
> diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
> index cce851a..af83fdd 100644
> --- a/hw/virtio/vhost-user.c
> +++ b/hw/virtio/vhost-user.c
> @@ -407,18 +407,79 @@ static int vhost_user_set_log_base(struct vhost_dev *dev, uint64_t base,
> return 0;
> }
>
> +static int vhost_user_fill_set_mem_table_msg(struct vhost_user *u,
> + struct vhost_dev *dev,
> + VhostUserMsg *msg,
> + int *fds, size_t *fd_num,
> + bool postcopy)
> +{
> + int i, fd;
> + ram_addr_t offset;
> + MemoryRegion *mr;
> + struct vhost_memory_region *reg;
> +
> + msg->hdr.request = VHOST_USER_SET_MEM_TABLE;
> +
> + for (i = 0; i < dev->mem->nregions; ++i) {
> + reg = dev->mem->regions + i;
> +
> + assert((uintptr_t)reg->userspace_addr == reg->userspace_addr);
> + mr = memory_region_from_host((void *)(uintptr_t)reg->userspace_addr,
> + &offset);
> + fd = memory_region_get_fd(mr);
> + if (fd > 0) {
> + if (postcopy) {
> + assert(*fd_num < VHOST_MEMORY_MAX_NREGIONS);
> + trace_vhost_user_set_mem_table_withfd(*fd_num, mr->name,
> + reg->memory_size,
> + reg->guest_phys_addr,
> + reg->userspace_addr,
> + offset);
> + u->region_rb_offset[i] = offset;
> + u->region_rb[i] = mr->ram_block;
> + } else if (*fd_num == VHOST_MEMORY_MAX_NREGIONS) {
> + error_report("Failed preparing vhost-user memory table msg");
> + return -1;
> + }
> + msg->payload.memory.regions[*fd_num].userspace_addr =
> + reg->userspace_addr;
> + msg->payload.memory.regions[*fd_num].memory_size =
> + reg->memory_size;
> + msg->payload.memory.regions[*fd_num].guest_phys_addr =
> + reg->guest_phys_addr;
> + msg->payload.memory.regions[*fd_num].mmap_offset = offset;
> + fds[(*fd_num)++] = fd;
> + } else if (postcopy) {
> + u->region_rb_offset[i] = 0;
> + u->region_rb[i] = NULL;
> + }
> + }
> +
> + msg->payload.memory.nregions = *fd_num;
> +
> + if (!*fd_num) {
> + error_report("Failed initializing vhost-user memory map, "
> + "consider using -object memory-backend-file share=on");
> + return -1;
> + }
> +
> + msg->hdr.size = sizeof(msg->payload.memory.nregions);
> + msg->hdr.size += sizeof(msg->payload.memory.padding);
> + msg->hdr.size += *fd_num * sizeof(VhostUserMemoryRegion);
> +
> + return 1;
> +}
> +
> static int vhost_user_set_mem_table_postcopy(struct vhost_dev *dev,
> struct vhost_memory *mem)
> {
> struct vhost_user *u = dev->opaque;
> int fds[VHOST_MEMORY_MAX_NREGIONS];
> - int i, fd;
> size_t fd_num = 0;
> VhostUserMsg msg_reply;
> int region_i, msg_i;
>
> VhostUserMsg msg = {
> - .hdr.request = VHOST_USER_SET_MEM_TABLE,
> .hdr.flags = VHOST_USER_VERSION,
> };
>
> @@ -433,48 +494,11 @@ static int vhost_user_set_mem_table_postcopy(struct vhost_dev *dev,
> u->region_rb_len = dev->mem->nregions;
> }
>
> - for (i = 0; i < dev->mem->nregions; ++i) {
> - struct vhost_memory_region *reg = dev->mem->regions + i;
> - ram_addr_t offset;
> - MemoryRegion *mr;
> -
> - assert((uintptr_t)reg->userspace_addr == reg->userspace_addr);
> - mr = memory_region_from_host((void *)(uintptr_t)reg->userspace_addr,
> - &offset);
> - fd = memory_region_get_fd(mr);
> - if (fd > 0) {
> - assert(fd_num < VHOST_MEMORY_MAX_NREGIONS);
> - trace_vhost_user_set_mem_table_withfd(fd_num, mr->name,
> - reg->memory_size,
> - reg->guest_phys_addr,
> - reg->userspace_addr, offset);
> - u->region_rb_offset[i] = offset;
> - u->region_rb[i] = mr->ram_block;
> - msg.payload.memory.regions[fd_num].userspace_addr =
> - reg->userspace_addr;
> - msg.payload.memory.regions[fd_num].memory_size = reg->memory_size;
> - msg.payload.memory.regions[fd_num].guest_phys_addr =
> - reg->guest_phys_addr;
> - msg.payload.memory.regions[fd_num].mmap_offset = offset;
> - fds[fd_num++] = fd;
> - } else {
> - u->region_rb_offset[i] = 0;
> - u->region_rb[i] = NULL;
> - }
> - }
> -
> - msg.payload.memory.nregions = fd_num;
> -
> - if (!fd_num) {
> - error_report("Failed initializing vhost-user memory map, "
> - "consider using -object memory-backend-file share=on");
> + if (vhost_user_fill_set_mem_table_msg(u, dev, &msg, fds, &fd_num,
> + true) < 0) {
> return -1;
> }
>
> - msg.hdr.size = sizeof(msg.payload.memory.nregions);
> - msg.hdr.size += sizeof(msg.payload.memory.padding);
> - msg.hdr.size += fd_num * sizeof(VhostUserMemoryRegion);
> -
> if (vhost_user_write(dev, &msg, fds, fd_num) < 0) {
> return -1;
> }
> @@ -545,7 +569,6 @@ static int vhost_user_set_mem_table(struct vhost_dev *dev,
> {
> struct vhost_user *u = dev->opaque;
> int fds[VHOST_MEMORY_MAX_NREGIONS];
> - int i, fd;
> size_t fd_num = 0;
> bool do_postcopy = u->postcopy_listen && u->postcopy_fd.handler;
> bool reply_supported = virtio_has_feature(dev->protocol_features,
> @@ -559,7 +582,6 @@ static int vhost_user_set_mem_table(struct vhost_dev *dev,
> }
>
> VhostUserMsg msg = {
> - .hdr.request = VHOST_USER_SET_MEM_TABLE,
> .hdr.flags = VHOST_USER_VERSION,
> };
>
> @@ -567,42 +589,11 @@ static int vhost_user_set_mem_table(struct vhost_dev *dev,
> msg.hdr.flags |= VHOST_USER_NEED_REPLY_MASK;
> }
>
> - for (i = 0; i < dev->mem->nregions; ++i) {
> - struct vhost_memory_region *reg = dev->mem->regions + i;
> - ram_addr_t offset;
> - MemoryRegion *mr;
> -
> - assert((uintptr_t)reg->userspace_addr == reg->userspace_addr);
> - mr = memory_region_from_host((void *)(uintptr_t)reg->userspace_addr,
> - &offset);
> - fd = memory_region_get_fd(mr);
> - if (fd > 0) {
> - if (fd_num == VHOST_MEMORY_MAX_NREGIONS) {
> - error_report("Failed preparing vhost-user memory table msg");
> - return -1;
> - }
> - msg.payload.memory.regions[fd_num].userspace_addr =
> - reg->userspace_addr;
> - msg.payload.memory.regions[fd_num].memory_size = reg->memory_size;
> - msg.payload.memory.regions[fd_num].guest_phys_addr =
> - reg->guest_phys_addr;
> - msg.payload.memory.regions[fd_num].mmap_offset = offset;
> - fds[fd_num++] = fd;
> - }
> - }
> -
> - msg.payload.memory.nregions = fd_num;
> -
> - if (!fd_num) {
> - error_report("Failed initializing vhost-user memory map, "
> - "consider using -object memory-backend-file share=on");
> + if (vhost_user_fill_set_mem_table_msg(u, dev, &msg, fds, &fd_num,
> + false) < 0) {
> return -1;
> }
>
> - msg.hdr.size = sizeof(msg.payload.memory.nregions);
> - msg.hdr.size += sizeof(msg.payload.memory.padding);
> - msg.hdr.size += fd_num * sizeof(VhostUserMemoryRegion);
> -
> if (vhost_user_write(dev, &msg, fds, fd_num) < 0) {
> return -1;
> }
> --
> 1.8.3.1
next prev parent reply other threads:[~2020-02-06 8:22 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-01-16 2:57 [PATCH v2 0/3] vhost-user: Lift Max Ram Slots Limitation Raphael Norwitz
2020-01-16 2:57 ` [PATCH v2 1/3] Fixed assert in vhost_user_set_mem_table_postcopy Raphael Norwitz
2020-02-06 8:17 ` Michael S. Tsirkin
2020-02-06 8:20 ` Michael S. Tsirkin
2020-02-09 17:17 ` Raphael Norwitz
2020-01-16 2:57 ` [PATCH v2 2/3] Refactor vhost_user_set_mem_table functions Raphael Norwitz
2020-02-06 8:21 ` Michael S. Tsirkin [this message]
2020-02-09 17:21 ` Raphael Norwitz
2020-01-16 2:57 ` [PATCH v2 3/3] Lift max memory slots limit imposed by vhost-user Raphael Norwitz
2020-02-06 8:32 ` Michael S. Tsirkin
2020-02-09 17:43 ` Raphael Norwitz
2020-02-20 7:03 ` Raphael Norwitz
2020-02-25 12:07 ` Michael S. Tsirkin
2020-01-31 21:21 ` [PATCH v2 0/3] vhost-user: Lift Max Ram Slots Limitation Raphael Norwitz
2020-02-06 8:33 ` Michael S. Tsirkin
2020-02-09 17:14 ` Raphael Norwitz
2020-02-10 16:04 ` Michael S. Tsirkin
2020-02-19 5:33 ` Raphael Norwitz
2020-02-19 10:08 ` Michael S. Tsirkin
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20200206032007-mutt-send-email-mst@kernel.org \
--to=mst@redhat.com \
--cc=peter.turschm@nutanix.com \
--cc=qemu-devel@nongnu.org \
--cc=raphael.norwitz@nutanix.com \
--cc=raphael.s.norwitz@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).