[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Minios-devel] [UNIKRAFT PATCH v2 09/10] plat/xen: Add driver state functions to client API
Hi Costin, one more note inline -Yuri. 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 | 128 > +++++++++++++++++++++++++++++++++++++++ > 2 files changed, 164 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..b45fd0c 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,130 @@ 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(state_path, (int *) &state); > + > + return state; > +} > + > +static XenbusState xenbus_intstr_to_state(const char *intstr) > +{ > + return (XenbusState) atoi(intstr); > +} > + > +int xenbus_switch_state(struct xenbus_device *xendev, XenbusState state, > + xenbus_transaction_t xbt) > +{ > + char state_path[strlen(xendev->nodename) + sizeof("/state")]; > + char *current_state_str, new_state_str[2]; > + XenbusState current_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 */ > + current_state_str = xs_read(xbt, state_path, NULL); > + if (PTRISERR(current_state_str)) { > + err = PTR2ERR(current_state_str); > + goto exit; > + } > + > + /* convert to int */ > + current_state = xenbus_intstr_to_state(current_state_str); > + free(current_state_str); > + > + if (current_state == state) > + goto exit; /* state already set */ > + > + /* set new state */ > + sprintf(new_state_str, "%d", state); > + 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) > +{ > + char *crnt_state_str; > + XenbusState crnt_state; > + int err = 0, watch_is_local = 0; > + > + if (path == NULL || state == NULL) { > + err = -EINVAL; > + goto out; > + } > + > + for (;;) { > + crnt_state_str = xs_read(XBT_NIL, path, NULL); > + if (PTRISERR(crnt_state_str)) { > + err = PTR2ERR(crnt_state_str); > + goto out; > + } > + > + /* convert to int */ > + crnt_state = xenbus_intstr_to_state(crnt_state_str); > + free(crnt_state_str); > + > + if (crnt_state != *state) { > + *state = crnt_state; > + break; > + } > + > + if (watch == NULL) { If watch was not registered, and the state of the device is changed about this point in time (after xs_read and before xs_watch_path), we would not get a notification about this change. It might be that we will stuck in xenbus_watch_wait_event forever. I propose to save the current number of watch->pending_events before the xs_read() call. And the watch itself has to be registered before xs_read too, if none provided by the caller. And later, instead of doing xenbus_watch_wait_event, do: uk_waitq_wait_event(&watch->wq, watch->pending_events != saved_counter_value)); > + /* create a local watch */ > + watch = xs_watch_path(XBT_NIL, path); > + if (PTRISERR(watch)) { > + err = PTR2ERR(watch); > + goto out; > + } > + > + watch_is_local = 1; > + } > + > + xenbus_watch_wait_event(watch); > + } > + > +out: > + if (watch_is_local) > + xs_unwatch(XBT_NIL, watch); > + > + return err; > +} > -- > 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 |