[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Minios-devel] [UNIKRAFT PATCH v2 6/6] lib/ukblkdev: Stop and release an Unikraft block device





On 01.07.19 12:04, Roxana Nicolescu wrote:
A device can be stopped, which will not permit further requests.
Also, for closing the device, the device must pe stopped,
each queue must be release and all the data allocated must be freed.

Signed-off-by: Roxana Nicolescu <nicolescu.roxana1996@xxxxxxxxx>
---
  lib/ukblkdev/blkdev.c                   | 90 +++++++++++++++++++++++++++++++++
  lib/ukblkdev/exportsyms.uk              |  4 ++
  lib/ukblkdev/include/uk/blkdev.h        | 31 ++++++++++++
  lib/ukblkdev/include/uk/blkdev_core.h   | 13 +++++
  lib/ukblkdev/include/uk/blkdev_driver.h |  9 ++++
  5 files changed, 147 insertions(+)

diff --git a/lib/ukblkdev/blkdev.c b/lib/ukblkdev/blkdev.c
index 21346d50..1cce36cc 100644
--- a/lib/ukblkdev/blkdev.c
+++ b/lib/ukblkdev/blkdev.c
@@ -466,3 +466,93 @@ int uk_blkdev_sync_io(struct uk_blkdev *dev,
        return req->result;
  }
  #endif
+
+int uk_blkdev_stop(struct uk_blkdev *dev)
+{
+       int rc = 0;
+
+       UK_ASSERT(dev);
+       UK_ASSERT(dev->_data);
+       UK_ASSERT(dev->dev_ops);
+       UK_ASSERT(dev->dev_ops->dev_stop);
+       UK_ASSERT(dev->_data->state == UK_BLKDEV_RUNNING);
+
+       rc = dev->dev_ops->dev_stop(dev);
+       if (rc)
+               uk_pr_err("Failed to stop blkdev%"PRIu16" device %d\n",
+                               dev->_data->id, rc);
+       else {
+               uk_pr_info("Stopped blkdev%"PRIu16" device\n",
+                                               dev->_data->id);
+               dev->_data->state = UK_BLKDEV_CONFIGURED;
+       }
+
+       return rc;
+}
+
+void uk_blkdev_queue_release(struct uk_blkdev *dev, uint16_t queue_id)
+{
+       int rc = 0;
+
+       UK_ASSERT(dev != NULL);
+       UK_ASSERT(dev->_data);
+       UK_ASSERT(dev->dev_ops);
+       UK_ASSERT(dev->dev_ops->queue_release);
+       UK_ASSERT(queue_id < CONFIG_LIBUKBLKDEV_MAXNBQUEUES);
+       UK_ASSERT(dev->_data->state != UK_BLKDEV_RUNNING);
+       UK_ASSERT(!PTRISERR(dev->_queue[queue_id]));
+
+#if CONFIG_LIBUKBLKDEV_DISPATCHERTHREADS
+       /* TODO make sure if the dispatcher thread is not in the middle
+        * of the callback
+        **/
+       if (dev->_data->queue_handler[queue_id].callback)
+               _destroy_event_handler(&dev->_data->queue_handler[queue_id]);
+#endif
+
+       rc = dev->dev_ops->queue_release(dev, dev->_queue[queue_id]);
+       if (rc)
+               uk_pr_err("Failed to release blkdev%"PRIu16"-q%"PRIu16": %d\n",
+                               dev->_data->id, queue_id, rc);
+       else {
+               uk_pr_info("Released blkdev%"PRIu16"-q%"PRIu16"\n",
+                               dev->_data->id, queue_id);
+
+               dev->_queue[queue_id] = NULL;
+       }
+}

I think you should forward the error codes. It seems not to make sense to keep it internal.

+
+void uk_blkdev_drv_unregister(struct uk_blkdev *dev)
+{
+       uint16_t id;
+
+       UK_ASSERT(dev != NULL);
+       UK_ASSERT(dev->_data);
+       UK_ASSERT(dev->_data->state != UK_BLKDEV_RUNNING);

I think for unregister the device should be in UNCONFIGURED state.

+
+       id = dev->_data->id;
+
+       uk_free(dev->_data->a, dev->_data);
+       UK_TAILQ_REMOVE(&uk_blkdev_list, dev, _list);
+       blkdev_count--;
+
+       uk_pr_info("Unregistered blkdev%"PRIu16": %p\n",
+                                  id, dev);
+}
+
+void uk_blkdev_unconfigure(struct uk_blkdev *dev)
+{
+       uint16_t id;
+
+       UK_ASSERT(dev);
+       UK_ASSERT(dev->_data);
+       UK_ASSERT(dev->dev_ops);
+       UK_ASSERT(dev->dev_ops->dev_unconfigure);
+       UK_ASSERT(dev->_data->state != UK_BLKDEV_RUNNING);
+
+       id = dev->_data->id;
+       dev->_data->state = UK_BLKDEV_UNCONFIGURED;
+       dev->dev_ops->dev_unconfigure(dev);
+
+       uk_pr_info("Unconfigured blkdev%"PRIu16"\n", id);
+}

I let you decide if it also would make sense to forward errors (e.g., driver could complain that not all queues are released).

diff --git a/lib/ukblkdev/exportsyms.uk b/lib/ukblkdev/exportsyms.uk
index 85135b95..7c8a9fa1 100644
--- a/lib/ukblkdev/exportsyms.uk
+++ b/lib/ukblkdev/exportsyms.uk
@@ -24,3 +24,7 @@ uk_blkdev_sync_io
  uk_blkdev_sync_read
  uk_blkdev_sync_write
  uk_blkreq_is_done
+uk_blkdev_stop
+uk_blkdev_queue_release
+uk_blkdev_drv_unregister
+uk_blkdev_unconfigure
diff --git a/lib/ukblkdev/include/uk/blkdev.h b/lib/ukblkdev/include/uk/blkdev.h
index ad0f2629..e1ff3b46 100644
--- a/lib/ukblkdev/include/uk/blkdev.h
+++ b/lib/ukblkdev/include/uk/blkdev.h
@@ -471,6 +471,37 @@ int uk_blkdev_sync_io(struct uk_blkdev *dev,
#endif +/**
+ * Stop an Unikraft block device, and set its state to UK_BLKDEV_CONFIGURED
+ * state.
+ * The device can be restarted with a call to uk_blkdev_start().
+ *
+ * @param dev
+ *     The Unikraft Block Device.
+ */
+int uk_blkdev_stop(struct uk_blkdev *dev);
+
+/**
+ * Free a queue and its descriptors for an Unikraft block device.
+ * @param dev
+ *     The Unikraft Block Device.
+ * @param queue_id
+ *     The index of the queue to set up.
+ *     The value must be in range [0, nb_queue -1] previously supplied
+ *     to uk_blkdev_configure()
+ */
+void uk_blkdev_queue_release(struct uk_blkdev *dev, uint16_t queue_id)int 
uk_blkdev_queue_release(struct uk_blkdev *dev, uint16_t queue_id);

+
+/**
+ * Close a stopped Unikraft block device.
+ * The function frees all resources except for
+ * the ones needed by the UK_BLKDEV_UNCONFIGURED state.
+ *
+ * @param dev
+ *     The Unikraft Block Device.
+ */
+void uk_blkdev_unconfigure(struct uk_blkdev *dev);

You decide if
        int uk_blkdev_unconfigure(struct uk_blkdev *dev);
would make sense.

+
  #ifdef __cplusplus
  }
  #endif
diff --git a/lib/ukblkdev/include/uk/blkdev_core.h 
b/lib/ukblkdev/include/uk/blkdev_core.h
index 8bf08cdd..0774485d 100644
--- a/lib/ukblkdev/include/uk/blkdev_core.h
+++ b/lib/ukblkdev/include/uk/blkdev_core.h
@@ -202,14 +202,27 @@ typedef int (*uk_blkdev_queue_submit_one_t)(struct 
uk_blkdev *dev,
  typedef int (*uk_blkdev_queue_finish_reqs_t)(struct uk_blkdev *dev,
                struct uk_blkdev_queue *queue);
+/** Driver callback type to stop an Unikraft block device. */
+typedef int (*uk_blkdev_stop_t)(struct uk_blkdev *dev);
+
+/** Driver callback type to release a queue of an Unikraft block device. */
+typedef int (*uk_blkdev_queue_release_t)(struct uk_blkdev *dev,
+               struct uk_blkdev_queue *queue);
+
+/** Driver callback type to close an Unikraft block device. */
+typedef void (*uk_blkdev_unconfigure_t)(struct uk_blkdev *dev);
+
  struct uk_blkdev_ops {
        uk_blkdev_get_info_t                            get_info;
        uk_blkdev_configure_t                           dev_configure;
        uk_blkdev_queue_get_info_t                      queue_get_info;
        uk_blkdev_queue_configure_t                     queue_setup;
        uk_blkdev_start_t                               dev_start;
+       uk_blkdev_stop_t                                dev_stop;
        uk_blkdev_queue_intr_enable_t                   queue_intr_enable;
        uk_blkdev_queue_intr_disable_t                  queue_intr_disable;
+       uk_blkdev_queue_release_t                       queue_release;
+       uk_blkdev_unconfigure_t                         dev_unconfigure;
  };
/**
diff --git a/lib/ukblkdev/include/uk/blkdev_driver.h 
b/lib/ukblkdev/include/uk/blkdev_driver.h
index 534542a1..5183e64a 100644
--- a/lib/ukblkdev/include/uk/blkdev_driver.h
+++ b/lib/ukblkdev/include/uk/blkdev_driver.h
@@ -97,6 +97,15 @@ static inline void uk_blkdev_drv_queue_event(struct 
uk_blkdev *dev,
  #endif
  }
+/**
+ * Frees the data allocated for the Unikraft Block Device.
+ * Removes the block device from the list.
+ *
+ * @param dev
+ *     Unikraft block device
+ */
+void uk_blkdev_drv_unregister(struct uk_blkdev *dev);
+
  #ifdef __cplusplus
  }
  #endif


_______________________________________________
Minios-devel mailing list
Minios-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/minios-devel

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.