| Message ID | 1515515975-28968-1-git-send-email-jianjay.zhou@huawei.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, 10 Jan 2018 00:39:35 +0800 Jay Zhou <jianjay.zhou@huawei.com> wrote: > Start QEMU with more DIMM devices than limit but without any > vhost backends and then hotplug a vhost user backend, the VM > will be crashed. > Instead of asserting in vhost_user_set_mem_table(), error number > is used to gracefully prevent device to start. This fixes the > crash issue. I'd rewrite commit message as following: -- QEMU will assert on vhsot-user backed virtio device hotplug if QEMU is using more RAM regions than VHOST_MEMORY_MAX_NREGIONS (for example if it were started with a lot of DIMM devices). Fix it by returning error instead of asserting and let callers of vhost_set_mem_table() handle error condition gracefully. -- > > Cc: qemu-stable@nongnu.org > Signed-off-by: Igor Mammedov <imammedo@redhat.com> > Signed-off-by: Jay Zhou <jianjay.zhou@huawei.com> > --- > hw/virtio/vhost-user.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c > index 093675e..8500562 100644 > --- a/hw/virtio/vhost-user.c > +++ b/hw/virtio/vhost-user.c > @@ -317,11 +317,14 @@ static int vhost_user_set_mem_table(struct vhost_dev *dev, > &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; > - assert(fd_num < VHOST_MEMORY_MAX_NREGIONS); > fds[fd_num++] = fd; > } > }
Hi Igor, > -----Original Message----- > From: Igor Mammedov [mailto:imammedo@redhat.com] > Sent: Wednesday, January 10, 2018 9:31 PM > To: Zhoujian (jay) <jianjay.zhou@huawei.com> > Cc: qemu-devel@nongnu.org; Huangweidong (C) <weidong.huang@huawei.com>; > mst@redhat.com; wangxin (U) <wangxinxin.wang@huawei.com>; qemu- > stable@nongnu.org; Gonglei (Arei) <arei.gonglei@huawei.com>; Liuzhe (Ahriy, > Euler) <liuzhe13@huawei.com> > Subject: Re: [Qemu-devel] [PATCH v5 1/4] vhost: remove assertion to prevent > crash > > On Wed, 10 Jan 2018 00:39:35 +0800 > Jay Zhou <jianjay.zhou@huawei.com> wrote: > > > Start QEMU with more DIMM devices than limit but without any vhost > > backends and then hotplug a vhost user backend, the VM will be > > crashed. > > Instead of asserting in vhost_user_set_mem_table(), error number is > > used to gracefully prevent device to start. This fixes the crash > > issue. > > I'd rewrite commit message as following: > -- > QEMU will assert on vhsot-user backed virtio device hotplug if QEMU is using > more RAM regions than VHOST_MEMORY_MAX_NREGIONS (for example if it were > started with a lot of DIMM devices). > > Fix it by returning error instead of asserting and let callers of > vhost_set_mem_table() handle error condition gracefully. > -- s/vhsot-user/vhost-user, otherwise much better than mine, will use it in the next version, thanks! Jay > > > > Cc: qemu-stable@nongnu.org > > Signed-off-by: Igor Mammedov <imammedo@redhat.com> > > Signed-off-by: Jay Zhou <jianjay.zhou@huawei.com> > > --- > > hw/virtio/vhost-user.c | 5 ++++- > > 1 file changed, 4 insertions(+), 1 deletion(-) > > > > diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c index > > 093675e..8500562 100644 > > --- a/hw/virtio/vhost-user.c > > +++ b/hw/virtio/vhost-user.c > > @@ -317,11 +317,14 @@ static int vhost_user_set_mem_table(struct vhost_dev > *dev, > > &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; > > - assert(fd_num < VHOST_MEMORY_MAX_NREGIONS); > > fds[fd_num++] = fd; > > } > > }
diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c index 093675e..8500562 100644 --- a/hw/virtio/vhost-user.c +++ b/hw/virtio/vhost-user.c @@ -317,11 +317,14 @@ static int vhost_user_set_mem_table(struct vhost_dev *dev, &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; - assert(fd_num < VHOST_MEMORY_MAX_NREGIONS); fds[fd_num++] = fd; } }