Re: [PATCH v2 15/24] vhost-user: add op to enable or disable a single vring

2022-08-24 Thread Kangjie Xu 



在 2022/8/24 17:02, Jason Wang 写道:


在 2022/8/24 11:09, Kangjie Xu 写道:


在 2022/8/24 10:53, Jason Wang 写道:


在 2022/8/16 09:06, Kangjie Xu 写道:

The interface to set enable status for a single vring is lacked in
VhostOps, since the vhost_set_vring_enable_op will manipulate all
virtqueues in a device.

Resetting a single vq will rely on this interface.

Signed-off-by: Kangjie Xu 
Signed-off-by: Xuan Zhuo 
---
  hw/virtio/vhost-user.c    | 26 +++---
  include/hw/virtio/vhost-backend.h |  3 +++
  2 files changed, 22 insertions(+), 7 deletions(-)

diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
index 56033f7a92..8307976cda 100644
--- a/hw/virtio/vhost-user.c
+++ b/hw/virtio/vhost-user.c
@@ -1199,6 +1199,22 @@ static int vhost_user_set_vring_base(struct 
vhost_dev *dev,

  return vhost_set_vring(dev, VHOST_USER_SET_VRING_BASE, ring);
  }
  +static int vhost_user_set_single_vring_enable(struct vhost_dev 
*dev,

+  int index,
+  int enable)
+{
+    if (index < dev->vq_index || index >= dev->vq_index + 
dev->nvqs) {

+    return -EINVAL;
+    }
+
+    struct vhost_vring_state state = {
+    .index = index,
+    .num   = enable,
+    };
+
+    return vhost_set_vring(dev, VHOST_USER_SET_VRING_ENABLE, );
+}
+
  static int vhost_user_set_vring_enable(struct vhost_dev *dev, int 
enable)

  {
  int i;
@@ -1208,13 +1224,8 @@ static int 
vhost_user_set_vring_enable(struct vhost_dev *dev, int enable)

  }
    for (i = 0; i < dev->nvqs; ++i) {
-    int ret;
-    struct vhost_vring_state state = {
-    .index = dev->vq_index + i,
-    .num   = enable,
-    };
-
-    ret = vhost_set_vring(dev, VHOST_USER_SET_VRING_ENABLE, 
);



Then I'd squash this into previous patch or re-roder to let this 
patch (vhost_user_set_single_vring_enable()) to be first.


Thanks


Sorry, I don't get why we should re-order them, since these two 
patches are independent.



I meant it's not good to introduce some codes in patch 14 but delete 
them in patch 15 (the above part for example).


Thanks


I get your point, but in fact, it seems that the deleded codes here in 
patch 15 do not appear in patch 14


Patch 14 is about vhost_user_reset_vring(), patch 15 is about 
vhost_user_set_vring_enable().


Thanks.



Thanks

+    int ret = vhost_user_set_single_vring_enable(dev, 
dev->vq_index + i,

+ enable);
  if (ret < 0) {
  /*
   * Restoring the previous state is likely infeasible, 
as well as

@@ -2668,6 +2679,7 @@ const VhostOps user_ops = {
  .vhost_reset_vring = vhost_user_reset_vring,
  .vhost_reset_device = vhost_user_reset_device,
  .vhost_get_vq_index = vhost_user_get_vq_index,
+    .vhost_set_single_vring_enable = 
vhost_user_set_single_vring_enable,

  .vhost_set_vring_enable = vhost_user_set_vring_enable,
  .vhost_requires_shm_log = vhost_user_requires_shm_log,
  .vhost_migration_done = vhost_user_migration_done,
diff --git a/include/hw/virtio/vhost-backend.h 
b/include/hw/virtio/vhost-backend.h

index f23bf71a8d..38f6b752ff 100644
--- a/include/hw/virtio/vhost-backend.h
+++ b/include/hw/virtio/vhost-backend.h
@@ -83,6 +83,8 @@ typedef int (*vhost_reset_vring_op)(struct 
vhost_dev *dev,

  struct vhost_vring_state *ring);
  typedef int (*vhost_reset_device_op)(struct vhost_dev *dev);
  typedef int (*vhost_get_vq_index_op)(struct vhost_dev *dev, int 
idx);
+typedef int (*vhost_set_single_vring_enable_op)(struct vhost_dev 
*dev,
+    int index, int 
enable);

  typedef int (*vhost_set_vring_enable_op)(struct vhost_dev *dev,
   int enable);
  typedef bool (*vhost_requires_shm_log_op)(struct vhost_dev *dev);
@@ -158,6 +160,7 @@ typedef struct VhostOps {
  vhost_reset_device_op vhost_reset_device;
  vhost_reset_vring_op vhost_reset_vring;
  vhost_get_vq_index_op vhost_get_vq_index;
+    vhost_set_single_vring_enable_op vhost_set_single_vring_enable;
  vhost_set_vring_enable_op vhost_set_vring_enable;
  vhost_requires_shm_log_op vhost_requires_shm_log;
  vhost_migration_done_op vhost_migration_done;

Re: [PATCH v2 15/24] vhost-user: add op to enable or disable a single vring

2022-08-24 Thread Jason Wang 



在 2022/8/24 11:09, Kangjie Xu 写道:


在 2022/8/24 10:53, Jason Wang 写道:


在 2022/8/16 09:06, Kangjie Xu 写道:

The interface to set enable status for a single vring is lacked in
VhostOps, since the vhost_set_vring_enable_op will manipulate all
virtqueues in a device.

Resetting a single vq will rely on this interface.

Signed-off-by: Kangjie Xu 
Signed-off-by: Xuan Zhuo 
---
  hw/virtio/vhost-user.c    | 26 +++---
  include/hw/virtio/vhost-backend.h |  3 +++
  2 files changed, 22 insertions(+), 7 deletions(-)

diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
index 56033f7a92..8307976cda 100644
--- a/hw/virtio/vhost-user.c
+++ b/hw/virtio/vhost-user.c
@@ -1199,6 +1199,22 @@ static int vhost_user_set_vring_base(struct 
vhost_dev *dev,

  return vhost_set_vring(dev, VHOST_USER_SET_VRING_BASE, ring);
  }
  +static int vhost_user_set_single_vring_enable(struct vhost_dev *dev,
+  int index,
+  int enable)
+{
+    if (index < dev->vq_index || index >= dev->vq_index + dev->nvqs) {
+    return -EINVAL;
+    }
+
+    struct vhost_vring_state state = {
+    .index = index,
+    .num   = enable,
+    };
+
+    return vhost_set_vring(dev, VHOST_USER_SET_VRING_ENABLE, );
+}
+
  static int vhost_user_set_vring_enable(struct vhost_dev *dev, int 
enable)

  {
  int i;
@@ -1208,13 +1224,8 @@ static int vhost_user_set_vring_enable(struct 
vhost_dev *dev, int enable)

  }
    for (i = 0; i < dev->nvqs; ++i) {
-    int ret;
-    struct vhost_vring_state state = {
-    .index = dev->vq_index + i,
-    .num   = enable,
-    };
-
-    ret = vhost_set_vring(dev, VHOST_USER_SET_VRING_ENABLE, 
);



Then I'd squash this into previous patch or re-roder to let this 
patch (vhost_user_set_single_vring_enable()) to be first.


Thanks


Sorry, I don't get why we should re-order them, since these two 
patches are independent.



I meant it's not good to introduce some codes in patch 14 but delete 
them in patch 15 (the above part for example).


Thanks




Thanks

+    int ret = vhost_user_set_single_vring_enable(dev, 
dev->vq_index + i,

+ enable);
  if (ret < 0) {
  /*
   * Restoring the previous state is likely infeasible, 
as well as

@@ -2668,6 +2679,7 @@ const VhostOps user_ops = {
  .vhost_reset_vring = vhost_user_reset_vring,
  .vhost_reset_device = vhost_user_reset_device,
  .vhost_get_vq_index = vhost_user_get_vq_index,
+    .vhost_set_single_vring_enable = 
vhost_user_set_single_vring_enable,

  .vhost_set_vring_enable = vhost_user_set_vring_enable,
  .vhost_requires_shm_log = vhost_user_requires_shm_log,
  .vhost_migration_done = vhost_user_migration_done,
diff --git a/include/hw/virtio/vhost-backend.h 
b/include/hw/virtio/vhost-backend.h

index f23bf71a8d..38f6b752ff 100644
--- a/include/hw/virtio/vhost-backend.h
+++ b/include/hw/virtio/vhost-backend.h
@@ -83,6 +83,8 @@ typedef int (*vhost_reset_vring_op)(struct 
vhost_dev *dev,

  struct vhost_vring_state *ring);
  typedef int (*vhost_reset_device_op)(struct vhost_dev *dev);
  typedef int (*vhost_get_vq_index_op)(struct vhost_dev *dev, int idx);
+typedef int (*vhost_set_single_vring_enable_op)(struct vhost_dev *dev,
+    int index, int 
enable);

  typedef int (*vhost_set_vring_enable_op)(struct vhost_dev *dev,
   int enable);
  typedef bool (*vhost_requires_shm_log_op)(struct vhost_dev *dev);
@@ -158,6 +160,7 @@ typedef struct VhostOps {
  vhost_reset_device_op vhost_reset_device;
  vhost_reset_vring_op vhost_reset_vring;
  vhost_get_vq_index_op vhost_get_vq_index;
+    vhost_set_single_vring_enable_op vhost_set_single_vring_enable;
  vhost_set_vring_enable_op vhost_set_vring_enable;
  vhost_requires_shm_log_op vhost_requires_shm_log;
  vhost_migration_done_op vhost_migration_done;

Re: [PATCH v2 15/24] vhost-user: add op to enable or disable a single vring

2022-08-23 Thread Kangjie Xu 



在 2022/8/24 10:53, Jason Wang 写道:


在 2022/8/16 09:06, Kangjie Xu 写道:

The interface to set enable status for a single vring is lacked in
VhostOps, since the vhost_set_vring_enable_op will manipulate all
virtqueues in a device.

Resetting a single vq will rely on this interface.

Signed-off-by: Kangjie Xu 
Signed-off-by: Xuan Zhuo 
---
  hw/virtio/vhost-user.c    | 26 +++---
  include/hw/virtio/vhost-backend.h |  3 +++
  2 files changed, 22 insertions(+), 7 deletions(-)

diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
index 56033f7a92..8307976cda 100644
--- a/hw/virtio/vhost-user.c
+++ b/hw/virtio/vhost-user.c
@@ -1199,6 +1199,22 @@ static int vhost_user_set_vring_base(struct 
vhost_dev *dev,

  return vhost_set_vring(dev, VHOST_USER_SET_VRING_BASE, ring);
  }
  +static int vhost_user_set_single_vring_enable(struct vhost_dev *dev,
+  int index,
+  int enable)
+{
+    if (index < dev->vq_index || index >= dev->vq_index + dev->nvqs) {
+    return -EINVAL;
+    }
+
+    struct vhost_vring_state state = {
+    .index = index,
+    .num   = enable,
+    };
+
+    return vhost_set_vring(dev, VHOST_USER_SET_VRING_ENABLE, );
+}
+
  static int vhost_user_set_vring_enable(struct vhost_dev *dev, int 
enable)

  {
  int i;
@@ -1208,13 +1224,8 @@ static int vhost_user_set_vring_enable(struct 
vhost_dev *dev, int enable)

  }
    for (i = 0; i < dev->nvqs; ++i) {
-    int ret;
-    struct vhost_vring_state state = {
-    .index = dev->vq_index + i,
-    .num   = enable,
-    };
-
-    ret = vhost_set_vring(dev, VHOST_USER_SET_VRING_ENABLE, 
);



Then I'd squash this into previous patch or re-roder to let this patch 
(vhost_user_set_single_vring_enable()) to be first.


Thanks


Sorry, I don't get why we should re-order them, since these two patches 
are independent.


Thanks

+    int ret = vhost_user_set_single_vring_enable(dev, 
dev->vq_index + i,

+ enable);
  if (ret < 0) {
  /*
   * Restoring the previous state is likely infeasible, 
as well as

@@ -2668,6 +2679,7 @@ const VhostOps user_ops = {
  .vhost_reset_vring = vhost_user_reset_vring,
  .vhost_reset_device = vhost_user_reset_device,
  .vhost_get_vq_index = vhost_user_get_vq_index,
+    .vhost_set_single_vring_enable = 
vhost_user_set_single_vring_enable,

  .vhost_set_vring_enable = vhost_user_set_vring_enable,
  .vhost_requires_shm_log = vhost_user_requires_shm_log,
  .vhost_migration_done = vhost_user_migration_done,
diff --git a/include/hw/virtio/vhost-backend.h 
b/include/hw/virtio/vhost-backend.h

index f23bf71a8d..38f6b752ff 100644
--- a/include/hw/virtio/vhost-backend.h
+++ b/include/hw/virtio/vhost-backend.h
@@ -83,6 +83,8 @@ typedef int (*vhost_reset_vring_op)(struct 
vhost_dev *dev,

  struct vhost_vring_state *ring);
  typedef int (*vhost_reset_device_op)(struct vhost_dev *dev);
  typedef int (*vhost_get_vq_index_op)(struct vhost_dev *dev, int idx);
+typedef int (*vhost_set_single_vring_enable_op)(struct vhost_dev *dev,
+    int index, int enable);
  typedef int (*vhost_set_vring_enable_op)(struct vhost_dev *dev,
   int enable);
  typedef bool (*vhost_requires_shm_log_op)(struct vhost_dev *dev);
@@ -158,6 +160,7 @@ typedef struct VhostOps {
  vhost_reset_device_op vhost_reset_device;
  vhost_reset_vring_op vhost_reset_vring;
  vhost_get_vq_index_op vhost_get_vq_index;
+    vhost_set_single_vring_enable_op vhost_set_single_vring_enable;
  vhost_set_vring_enable_op vhost_set_vring_enable;
  vhost_requires_shm_log_op vhost_requires_shm_log;
  vhost_migration_done_op vhost_migration_done;

Re: [PATCH v2 15/24] vhost-user: add op to enable or disable a single vring

2022-08-23 Thread Jason Wang 



在 2022/8/16 09:06, Kangjie Xu 写道:

The interface to set enable status for a single vring is lacked in
VhostOps, since the vhost_set_vring_enable_op will manipulate all
virtqueues in a device.

Resetting a single vq will rely on this interface.

Signed-off-by: Kangjie Xu 
Signed-off-by: Xuan Zhuo 
---
  hw/virtio/vhost-user.c| 26 +++---
  include/hw/virtio/vhost-backend.h |  3 +++
  2 files changed, 22 insertions(+), 7 deletions(-)

diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
index 56033f7a92..8307976cda 100644
--- a/hw/virtio/vhost-user.c
+++ b/hw/virtio/vhost-user.c
@@ -1199,6 +1199,22 @@ static int vhost_user_set_vring_base(struct vhost_dev 
*dev,
  return vhost_set_vring(dev, VHOST_USER_SET_VRING_BASE, ring);
  }
  
+static int vhost_user_set_single_vring_enable(struct vhost_dev *dev,

+  int index,
+  int enable)
+{
+if (index < dev->vq_index || index >= dev->vq_index + dev->nvqs) {
+return -EINVAL;
+}
+
+struct vhost_vring_state state = {
+.index = index,
+.num   = enable,
+};
+
+return vhost_set_vring(dev, VHOST_USER_SET_VRING_ENABLE, );
+}
+
  static int vhost_user_set_vring_enable(struct vhost_dev *dev, int enable)
  {
  int i;
@@ -1208,13 +1224,8 @@ static int vhost_user_set_vring_enable(struct vhost_dev 
*dev, int enable)
  }
  
  for (i = 0; i < dev->nvqs; ++i) {

-int ret;
-struct vhost_vring_state state = {
-.index = dev->vq_index + i,
-.num   = enable,
-};
-
-ret = vhost_set_vring(dev, VHOST_USER_SET_VRING_ENABLE, );



Then I'd squash this into previous patch or re-roder to let this patch 
(vhost_user_set_single_vring_enable()) to be first.


Thanks



+int ret = vhost_user_set_single_vring_enable(dev, dev->vq_index + i,
+ enable);
  if (ret < 0) {
  /*
   * Restoring the previous state is likely infeasible, as well as
@@ -2668,6 +2679,7 @@ const VhostOps user_ops = {
  .vhost_reset_vring = vhost_user_reset_vring,
  .vhost_reset_device = vhost_user_reset_device,
  .vhost_get_vq_index = vhost_user_get_vq_index,
+.vhost_set_single_vring_enable = vhost_user_set_single_vring_enable,
  .vhost_set_vring_enable = vhost_user_set_vring_enable,
  .vhost_requires_shm_log = vhost_user_requires_shm_log,
  .vhost_migration_done = vhost_user_migration_done,
diff --git a/include/hw/virtio/vhost-backend.h 
b/include/hw/virtio/vhost-backend.h
index f23bf71a8d..38f6b752ff 100644
--- a/include/hw/virtio/vhost-backend.h
+++ b/include/hw/virtio/vhost-backend.h
@@ -83,6 +83,8 @@ typedef int (*vhost_reset_vring_op)(struct vhost_dev *dev,
  struct vhost_vring_state *ring);
  typedef int (*vhost_reset_device_op)(struct vhost_dev *dev);
  typedef int (*vhost_get_vq_index_op)(struct vhost_dev *dev, int idx);
+typedef int (*vhost_set_single_vring_enable_op)(struct vhost_dev *dev,
+int index, int enable);
  typedef int (*vhost_set_vring_enable_op)(struct vhost_dev *dev,
   int enable);
  typedef bool (*vhost_requires_shm_log_op)(struct vhost_dev *dev);
@@ -158,6 +160,7 @@ typedef struct VhostOps {
  vhost_reset_device_op vhost_reset_device;
  vhost_reset_vring_op vhost_reset_vring;
  vhost_get_vq_index_op vhost_get_vq_index;
+vhost_set_single_vring_enable_op vhost_set_single_vring_enable;
  vhost_set_vring_enable_op vhost_set_vring_enable;
  vhost_requires_shm_log_op vhost_requires_shm_log;
  vhost_migration_done_op vhost_migration_done;

[PATCH v2 15/24] vhost-user: add op to enable or disable a single vring

2022-08-15 Thread Kangjie Xu 
The interface to set enable status for a single vring is lacked in
VhostOps, since the vhost_set_vring_enable_op will manipulate all
virtqueues in a device.

Resetting a single vq will rely on this interface.

Signed-off-by: Kangjie Xu 
Signed-off-by: Xuan Zhuo 
---
 hw/virtio/vhost-user.c| 26 +++---
 include/hw/virtio/vhost-backend.h |  3 +++
 2 files changed, 22 insertions(+), 7 deletions(-)

diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
index 56033f7a92..8307976cda 100644
--- a/hw/virtio/vhost-user.c
+++ b/hw/virtio/vhost-user.c
@@ -1199,6 +1199,22 @@ static int vhost_user_set_vring_base(struct vhost_dev 
*dev,
 return vhost_set_vring(dev, VHOST_USER_SET_VRING_BASE, ring);
 }
 
+static int vhost_user_set_single_vring_enable(struct vhost_dev *dev,
+  int index,
+  int enable)
+{
+if (index < dev->vq_index || index >= dev->vq_index + dev->nvqs) {
+return -EINVAL;
+}
+
+struct vhost_vring_state state = {
+.index = index,
+.num   = enable,
+};
+
+return vhost_set_vring(dev, VHOST_USER_SET_VRING_ENABLE, );
+}
+
 static int vhost_user_set_vring_enable(struct vhost_dev *dev, int enable)
 {
 int i;
@@ -1208,13 +1224,8 @@ static int vhost_user_set_vring_enable(struct vhost_dev 
*dev, int enable)
 }
 
 for (i = 0; i < dev->nvqs; ++i) {
-int ret;
-struct vhost_vring_state state = {
-.index = dev->vq_index + i,
-.num   = enable,
-};
-
-ret = vhost_set_vring(dev, VHOST_USER_SET_VRING_ENABLE, );
+int ret = vhost_user_set_single_vring_enable(dev, dev->vq_index + i,
+ enable);
 if (ret < 0) {
 /*
  * Restoring the previous state is likely infeasible, as well as
@@ -2668,6 +2679,7 @@ const VhostOps user_ops = {
 .vhost_reset_vring = vhost_user_reset_vring,
 .vhost_reset_device = vhost_user_reset_device,
 .vhost_get_vq_index = vhost_user_get_vq_index,
+.vhost_set_single_vring_enable = vhost_user_set_single_vring_enable,
 .vhost_set_vring_enable = vhost_user_set_vring_enable,
 .vhost_requires_shm_log = vhost_user_requires_shm_log,
 .vhost_migration_done = vhost_user_migration_done,
diff --git a/include/hw/virtio/vhost-backend.h 
b/include/hw/virtio/vhost-backend.h
index f23bf71a8d..38f6b752ff 100644
--- a/include/hw/virtio/vhost-backend.h
+++ b/include/hw/virtio/vhost-backend.h
@@ -83,6 +83,8 @@ typedef int (*vhost_reset_vring_op)(struct vhost_dev *dev,
 struct vhost_vring_state *ring);
 typedef int (*vhost_reset_device_op)(struct vhost_dev *dev);
 typedef int (*vhost_get_vq_index_op)(struct vhost_dev *dev, int idx);
+typedef int (*vhost_set_single_vring_enable_op)(struct vhost_dev *dev,
+int index, int enable);
 typedef int (*vhost_set_vring_enable_op)(struct vhost_dev *dev,
  int enable);
 typedef bool (*vhost_requires_shm_log_op)(struct vhost_dev *dev);
@@ -158,6 +160,7 @@ typedef struct VhostOps {
 vhost_reset_device_op vhost_reset_device;
 vhost_reset_vring_op vhost_reset_vring;
 vhost_get_vq_index_op vhost_get_vq_index;
+vhost_set_single_vring_enable_op vhost_set_single_vring_enable;
 vhost_set_vring_enable_op vhost_set_vring_enable;
 vhost_requires_shm_log_op vhost_requires_shm_log;
 vhost_migration_done_op vhost_migration_done;
-- 
2.32.0