|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Minios-devel] [UNIKRAFT PATCH v3 09/10] plat/xen: Add driver state functions to client API
Reviewed-by: Yuri Volchkov <yuri.volchkov@xxxxxxxxx>
But there is a couple of notes inline.
Costin Lupu <costin.lupu@xxxxxxxxx> writes:
> Extend the client API with functions for dealing with Xenbus
> driver states.
>
> Signed-off-by: Costin Lupu <costin.lupu@xxxxxxxxx>
> ---
> plat/xen/include/xenbus/client.h | 36 +++++++++++++
> plat/xen/xenbus/client.c | 107
> +++++++++++++++++++++++++++++++++++++++
> plat/xen/xenbus/exportsyms.uk | 3 ++
> 3 files changed, 146 insertions(+)
>
> diff --git a/plat/xen/include/xenbus/client.h
> b/plat/xen/include/xenbus/client.h
> index f3540b7..2add3be 100644
> --- a/plat/xen/include/xenbus/client.h
> +++ b/plat/xen/include/xenbus/client.h
> @@ -86,4 +86,40 @@ int xenbus_watch_wait_event(struct xenbus_watch *watch);
> */
> int xenbus_watch_notify_event(struct xenbus_watch *watch);
>
> +/*
> + * Driver states
> + */
> +
> +/*
> + * Returns the driver state found at the given Xenstore path.
> + *
> + * @param path Xenstore path
> + * @return The Xenbus driver state
> + */
> +XenbusState xenbus_read_driver_state(const char *path);
> +
> +/*
> + * Changes the state of a Xen PV driver
> + *
> + * @param xendev Xenbus device
> + * @param state The new Xenbus state
> + * @param xbt Xenbus transaction id
> + * @return 0 on success, a negative errno value on error.
> + */
> +int xenbus_switch_state(struct xenbus_device *xendev, XenbusState state,
> + xenbus_transaction_t xbt);
> +
> +/*
> + * Waits for the driver state found at the given Xenstore path to change by
> + * using watches.
> + *
> + * @param path Xenstore path
> + * @param state The returned Xenbus state
> + * @param watch Xenbus watch. It may be NULL, in which case a local watch
> + * will be created.
> + * @return 0 on success, a negative errno value on error.
> + */
> +int xenbus_wait_for_state_change(const char *path, XenbusState *state,
> + struct xenbus_watch *watch);
> +
> #endif /* __XENBUS_CLIENT_H__ */
> diff --git a/plat/xen/xenbus/client.c b/plat/xen/xenbus/client.c
> index 3dbca0f..d901222 100644
> --- a/plat/xen/xenbus/client.c
> +++ b/plat/xen/xenbus/client.c
> @@ -44,6 +44,7 @@
> #include <string.h>
> #include <uk/errptr.h>
> #include <uk/wait.h>
> +#include <xenbus/xs.h>
> #include <xenbus/client.h>
>
>
> @@ -130,3 +131,109 @@ int xenbus_watch_notify_event(struct xenbus_watch
> *watch)
>
> return 0;
> }
> +
> +XenbusState xenbus_read_driver_state(const char *path)
> +{
> + char state_path[strlen(path) + sizeof("/state")];
> + XenbusState state = XenbusStateUnknown;
> +
> + sprintf(state_path, "%s/state", path);
> + xs_read_integer(XBT_NIL, state_path, (int *) &state);
> +
> + return state;
> +}
> +
> +int xenbus_switch_state(struct xenbus_device *xendev, XenbusState state,
> + xenbus_transaction_t xbt)
Other function have xbt as a first parameter. Maybe in the follow up
patches move it here on the first place too? For the sake of consistency.
> +{
> + char state_path[strlen(xendev->nodename) + sizeof("/state")];
> + char new_state_str[2];
> + XenbusState crnt_state;
> + int need_transaction_end = 0; /* non-zero if local transaction */
> + int abort;
> + int err;
> +
> + if (xendev == NULL)
> + return -EINVAL;
> +
> + sprintf(state_path, "%s/state", xendev->nodename);
> +
> + do {
> + abort = 1;
> +
> + if (xbt == XBT_NIL) {
> + err = xs_transaction_start(&xbt);
> + if (err)
> + goto exit;
> + need_transaction_end = 1;
> + }
> +
> + /* check if state is already set */
> + err = xs_read_integer(xbt, xendev->nodename,
> + (int *) &crnt_state);
> + if (err || crnt_state == state)
> + goto exit;
> +
> + /* set new state */
> + snprintf(new_state_str, sizeof(new_state_str), "%d", state);
We never checked state for sanity. That's also fine to do in the next
patches.
> + err = xs_write(xbt, state_path, NULL, new_state_str);
> +
> + abort = 0;
> +exit:
> + if (need_transaction_end) {
> + int _err;
> +
> + _err = xs_transaction_end(xbt, abort);
> + if (!err)
> + err = _err;
> + xbt = XBT_NIL;
> + }
> + } while (err == -EAGAIN);
> +
> + if (err)
> + uk_printd(DLVL_ERR, "Error switching state to %s: %d\n",
> + xenbus_state_to_str(state), err);
> +
> + return err;
> +}
> +
> +int xenbus_wait_for_state_change(const char *path, XenbusState *state,
> + struct xenbus_watch *watch)
> +{
> + XenbusState crnt_state;
> + int err = 0, watch_is_local = 0;
> +
> + if (path == NULL || state == NULL) {
> + err = -EINVAL;
> + goto out;
> + }
> +
> + if (watch == NULL) {
> + /* create a local watch */
> + watch = xs_watch_path(XBT_NIL, path);
> + if (PTRISERR(watch)) {
> + err = PTR2ERR(watch);
> + goto out;
> + }
> + watch_is_local = 1;
> + }
> +
> + for (;;) {
> + err = xs_read_integer(XBT_NIL, path, (int *) &crnt_state);
> + if (err)
> + break;
> +
> + if (crnt_state != *state) {
> + *state = crnt_state;
> + break;
> + }
> +
> + xenbus_watch_wait_event(watch);
> + }
> +
> +out:
> + if (watch_is_local)
> + xs_unwatch(XBT_NIL, watch);
> +
> + return err;
> +}
> diff --git a/plat/xen/xenbus/exportsyms.uk b/plat/xen/xenbus/exportsyms.uk
> index 695a01a..8436a7a 100644
> --- a/plat/xen/xenbus/exportsyms.uk
> +++ b/plat/xen/xenbus/exportsyms.uk
> @@ -28,4 +28,7 @@ xenbus_devtype_to_str
> xenbus_str_to_devtype
> xenbus_watch_wait_event
> xenbus_watch_notify_event
> +xenbus_read_driver_state
> +xenbus_switch_state
> +xenbus_wait_for_state_change
>
> --
> 2.11.0
>
--
Yuri Volchkov
Software Specialist
NEC Europe Ltd
Kurfürsten-Anlage 36
D-69115 Heidelberg
_______________________________________________
Minios-devel mailing list
Minios-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/minios-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |