All of lore.kernel.org
 help / color / mirror / Atom feed
From: Dima Stepanov <dimastep@yandex-team.ru>
To: qemu-devel@nongnu.org
Cc: fam@euphon.net, kwolf@redhat.com, yc-core@yandex-team.ru,
	qemu-block@nongnu.org, mst@redhat.com, jasowang@redhat.com,
	dgilbert@redhat.com, mreitz@redhat.com, arei.gonglei@huawei.com,
	fengli@smartx.com, stefanha@redhat.com,
	marcandre.lureau@redhat.com, pbonzini@redhat.com,
	raphael.norwitz@nutanix.com
Subject: [PATCH v2 5/5] vhost: add device started check in migration set log
Date: Thu, 30 Apr 2020 16:36:20 +0300	[thread overview]
Message-ID: <d25241eb1fe7a55fc7dbe63ecedb4f1adf407837.1588252862.git.dimastep@yandex-team.ru> (raw)
In-Reply-To: <cover.1588252861.git.dimastep@yandex-team.ru>
In-Reply-To: <cover.1588252861.git.dimastep@yandex-team.ru>

If vhost-user daemon is used as a backend for the vhost device, then we
should consider a possibility of disconnect at any moment. If such
disconnect happened in the vhost_migration_log() routine the vhost
device structure will be clean up.
At the start of the vhost_migration_log() function there is a check:
  if (!dev->started) {
      dev->log_enabled = enable;
      return 0;
  }
To be consistent with this check add the same check after calling the
vhost_dev_set_log() routine. This in general help not to break a
migration due the assert() message. But it looks like that this code
should be revised to handle these errors more carefully.

In case of vhost-user device backend the fail paths should consider the
state of the device. In this case we should skip some function calls
during rollback on the error paths, so not to get the NULL dereference
errors.

Signed-off-by: Dima Stepanov <dimastep@yandex-team.ru>
---
 hw/virtio/vhost.c | 39 +++++++++++++++++++++++++++++++++++----
 1 file changed, 35 insertions(+), 4 deletions(-)

diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
index 3ee50c4..d5ab96d 100644
--- a/hw/virtio/vhost.c
+++ b/hw/virtio/vhost.c
@@ -787,6 +787,17 @@ static int vhost_dev_set_features(struct vhost_dev *dev,
 static int vhost_dev_set_log(struct vhost_dev *dev, bool enable_log)
 {
     int r, i, idx;
+
+    if (!dev->started) {
+        /*
+         * If vhost-user daemon is used as a backend for the
+         * device and the connection is broken, then the vhost_dev
+         * structure will be reset all its values to 0.
+         * Add additional check for the device state.
+         */
+        return -1;
+    }
+
     r = vhost_dev_set_features(dev, enable_log);
     if (r < 0) {
         goto err_features;
@@ -801,12 +812,19 @@ static int vhost_dev_set_log(struct vhost_dev *dev, bool enable_log)
     }
     return 0;
 err_vq:
-    for (; i >= 0; --i) {
+    /*
+     * Disconnect with the vhost-user daemon can lead to the
+     * vhost_dev_cleanup() call which will clean up vhost_dev
+     * structure.
+     */
+    for (; dev->started && (i >= 0); --i) {
         idx = dev->vhost_ops->vhost_get_vq_index(dev, dev->vq_index + i);
         vhost_virtqueue_set_addr(dev, dev->vqs + i, idx,
                                  dev->log_enabled);
     }
-    vhost_dev_set_features(dev, dev->log_enabled);
+    if (dev->started) {
+        vhost_dev_set_features(dev, dev->log_enabled);
+    }
 err_features:
     return r;
 }
@@ -832,7 +850,15 @@ static int vhost_migration_log(MemoryListener *listener, int enable)
     } else {
         vhost_dev_log_resize(dev, vhost_get_log_size(dev));
         r = vhost_dev_set_log(dev, true);
-        if (r < 0) {
+        /*
+         * The dev log resize can fail, because of disconnect
+         * with the vhost-user-blk daemon. Check the device
+         * state before calling the vhost_dev_set_log()
+         * function.
+         * Don't return error if device isn't started to be
+         * consistent with the check above.
+         */
+        if (dev->started && r < 0) {
             return r;
         }
     }
@@ -1739,7 +1765,12 @@ int vhost_dev_start(struct vhost_dev *hdev, VirtIODevice *vdev)
 fail_log:
     vhost_log_put(hdev, false);
 fail_vq:
-    while (--i >= 0) {
+    /*
+     * Disconnect with the vhost-user daemon can lead to the
+     * vhost_dev_cleanup() call which will clean up vhost_dev
+     * structure.
+     */
+    while ((--i >= 0) && (hdev->started)) {
         vhost_virtqueue_stop(hdev,
                              vdev,
                              hdev->vqs + i,
-- 
2.7.4



  parent reply	other threads:[~2020-04-30 13:57 UTC|newest]

Thread overview: 51+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-04-30 13:36 [PATCH v2 0/5] vhost-user reconnect issues during vhost initialization Dima Stepanov
2020-04-30 13:36 ` [PATCH v2 1/5] char-socket: return -1 in case of disconnect during tcp_chr_write Dima Stepanov
2020-05-06  8:54   ` Li Feng
2020-05-06  9:46   ` Marc-André Lureau
2020-04-30 13:36 ` [PATCH v2 2/5] vhost: introduce wrappers to set guest notifiers for virtio device Dima Stepanov
2020-05-04  0:36   ` Raphael Norwitz
2020-05-06  8:54     ` Dima Stepanov
2020-05-11  3:03   ` Jason Wang
2020-05-11  8:55     ` Dima Stepanov
2020-04-30 13:36 ` [PATCH v2 3/5] vhost-user-blk: add mechanism to track the guest notifiers init state Dima Stepanov
2020-05-04  1:06   ` Raphael Norwitz
2020-05-06  8:51     ` Dima Stepanov
2020-04-30 13:36 ` [PATCH v2 4/5] vhost: check vring address before calling unmap Dima Stepanov
2020-05-04  1:13   ` Raphael Norwitz
2020-05-11  3:05   ` Jason Wang
2020-05-11  9:11     ` Dima Stepanov
2020-05-12  3:26       ` Jason Wang
2020-05-12  9:08         ` Dima Stepanov
2020-05-13  3:00           ` Jason Wang
2020-05-13  9:36             ` Dima Stepanov
2020-05-14  7:28               ` Jason Wang
2020-04-30 13:36 ` Dima Stepanov [this message]
2020-05-06 22:08   ` [PATCH v2 5/5] vhost: add device started check in migration set log Raphael Norwitz
2020-05-07  7:15     ` Michael S. Tsirkin
2020-05-07 15:35     ` Dima Stepanov
2020-05-11  0:03       ` Raphael Norwitz
2020-05-11  9:43         ` Dima Stepanov
2020-05-11  3:15   ` Jason Wang
2020-05-11  9:25     ` Dima Stepanov
2020-05-12  3:32       ` Jason Wang
2020-05-12  3:47         ` Li Feng
2020-05-12  9:23           ` Dima Stepanov
2020-05-12  9:35         ` Dima Stepanov
2020-05-13  3:20           ` Jason Wang
2020-05-13  9:39             ` Dima Stepanov
2020-05-13  4:15           ` Michael S. Tsirkin
2020-05-13  5:56             ` Jason Wang
2020-05-13  9:47               ` Dima Stepanov
2020-05-14  7:34                 ` Jason Wang
2020-05-15 16:54                   ` Dima Stepanov
2020-05-16  3:20                     ` Li Feng
2020-05-18  2:52                       ` Jason Wang
2020-05-18  9:33                         ` Dima Stepanov
2020-05-18  9:27                       ` Dima Stepanov
2020-05-18  2:50                     ` Jason Wang
2020-05-18  9:41                       ` Dima Stepanov
2020-05-18  9:53                         ` Dr. David Alan Gilbert
2020-05-19  9:07                           ` Dima Stepanov
2020-05-19 10:24                             ` Dr. David Alan Gilbert
2020-05-19  9:59                     ` Michael S. Tsirkin
2020-05-19  9:13               ` Dima Stepanov

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=d25241eb1fe7a55fc7dbe63ecedb4f1adf407837.1588252862.git.dimastep@yandex-team.ru \
    --to=dimastep@yandex-team.ru \
    --cc=arei.gonglei@huawei.com \
    --cc=dgilbert@redhat.com \
    --cc=fam@euphon.net \
    --cc=fengli@smartx.com \
    --cc=jasowang@redhat.com \
    --cc=kwolf@redhat.com \
    --cc=marcandre.lureau@redhat.com \
    --cc=mreitz@redhat.com \
    --cc=mst@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=qemu-block@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    --cc=raphael.norwitz@nutanix.com \
    --cc=stefanha@redhat.com \
    --cc=yc-core@yandex-team.ru \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.