Roxana
On Tue, Oct 15, 2019 at 4:52 PM Simon Kuenzer <simon.kuenzer@xxxxxxxxx
<mailto:simon.kuenzer@xxxxxxxxx>> wrote:
Hey, thanks a lot for the quick fix. A minor comment.
Thanks,
Simon
On 15.10.19 15:41, 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
<mailto:nicolescu.roxana1996@xxxxxxxxx>>
> ---
> lib/ukblkdev/blkdev.c | 90
+++++++++++++++++++++++++
> lib/ukblkdev/exportsyms.uk <http://exportsyms.uk>
| 4 ++
> lib/ukblkdev/include/uk/blkdev.h | 44 ++++++++++++
> lib/ukblkdev/include/uk/blkdev_core.h | 13 ++++
> lib/ukblkdev/include/uk/blkdev_driver.h | 9 +++
> 5 files changed, 160 insertions(+)
>
> diff --git a/lib/ukblkdev/blkdev.c b/lib/ukblkdev/blkdev.c
> index 8679e074..11e33b59 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);
> +
> + uk_pr_info("Trying to stop blkdev%"PRIu16" device\n",
> + dev->_data->id);
> + 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;
> +}
> +
> +int 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
> + 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;
> + }
> +
> + return rc;
> +}
> +
> +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_UNCONFIGURED);
> +
> + 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);
> +}
> +
> +int uk_blkdev_unconfigure(struct uk_blkdev *dev)
> +{
> + uint16_t q_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_CONFIGURED);
> + for (q_id = 0; q_id < CONFIG_LIBUKBLKDEV_MAXNBQUEUES; ++q_id)
> + UK_ASSERT(PTRISERR(dev->_queue[q_id]));
> +
> + dev->_data->state = UK_BLKDEV_UNCONFIGURED;
You should only go into UNCONFIGURED state, if you did not get an
error,
from the driver right?
> + return dev->dev_ops->dev_unconfigure(dev);
> +}
> diff --git a/lib/ukblkdev/exportsyms.uk <http://exportsyms.uk>
b/lib/ukblkdev/exportsyms.uk <http://exportsyms.uk>
> index 5c05994a..f3be6c51 100644
> --- a/lib/ukblkdev/exportsyms.uk <http://exportsyms.uk>
> +++ b/lib/ukblkdev/exportsyms.uk <http://exportsyms.uk>
> @@ -12,3 +12,7 @@ uk_blkdev_start
> uk_blkdev_queue_submit_one
> uk_blkdev_queue_finish_reqs
> uk_blkdev_sync_io
> +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..e1e9b069 100644
> --- a/lib/ukblkdev/include/uk/blkdev.h
> +++ b/lib/ukblkdev/include/uk/blkdev.h
> @@ -471,6 +471,50 @@ int uk_blkdev_sync_io(struct uk_blkdev *dev,
>
> #endif
>
> +/**
> + * Stop a Unikraft block device, and set its state to
UK_BLKDEV_CONFIGURED
> + * state. From now on, users cannot send any requests.
> + * If there are pending requests, this function will return
-EBUSY because
> + * the queues are not empty. If polling is used instead of
interrupts,
> + * make sure to clean the queue and process all the responses
before this
> + * operation is called.
> + * The device can be restarted with a call to uk_blkdev_start().
> + *
> + * @param dev
> + * The Unikraft Block Device.
> + * @return
> + * - 0: Success
> + * - (<0): on error returned by driver
> + */
> +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()
> + * @return
> + * - 0: Success
> + * - (<0): on error returned by driver
> + */
> +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.
> + * @return
> + * - 0: Success
> + * - (<0): on error returned by driver
> + */
> +int uk_blkdev_unconfigure(struct uk_blkdev *dev);
> +
> #ifdef __cplusplus
> }
> #endif
> diff --git a/lib/ukblkdev/include/uk/blkdev_core.h
b/lib/ukblkdev/include/uk/blkdev_core.h
> index b12e3eb1..ba9bf575 100644
> --- a/lib/ukblkdev/include/uk/blkdev_core.h
> +++ b/lib/ukblkdev/include/uk/blkdev_core.h
> @@ -203,14 +203,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 int (*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 a3c41da3..be807069 100644
> --- a/lib/ukblkdev/include/uk/blkdev_driver.h
> +++ b/lib/ukblkdev/include/uk/blkdev_driver.h
> @@ -106,6 +106,15 @@ static inline void
uk_blkdev_drv_queue_event(struct uk_blkdev *dev,
> #define uk_blkreq_finished(req) \
> (ukarch_store_n(&(req)->state.counter, UK_BLKDEV_REQ_FINISHED))
>
> +/**
> + * 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
>