Ben Pfaff <[email protected]> writes:
> It took me a few minutes to figure out what the code for deciding on the
> vhost socket directory was doing, because it was needlessly complicated.
> This simplifies it.
>
> Build tested only.
>
> Signed-off-by: Ben Pfaff <[email protected]>
> ---
> This is a repost of a patch first posted in April.
Somehow this dropped off my radar. Sorry.
> lib/dpdk.c | 64
> ++++++++++++++++++++++----------------------------------------
> 1 file changed, 23 insertions(+), 41 deletions(-)
>
> diff --git a/lib/dpdk.c b/lib/dpdk.c
> index 8da6c324407e..1bd31ba345f5 100644
> --- a/lib/dpdk.c
> +++ b/lib/dpdk.c
> @@ -42,29 +42,29 @@ static FILE *log_stream = NULL; /* Stream for DPDK
> log redirection */
>
> static char *vhost_sock_dir = NULL; /* Location of vhost-user sockets */
>
> -static int
> -process_vhost_flags(char *flag, const char *default_val, int size,
> - const struct smap *ovs_other_config,
> - char **new_val)
> +static char *
> +get_custom_vhost_sock_dir(const char *stem, const char *suffix)
> {
> - const char *val;
> - int changed = 0;
> + if (!suffix) {
> + return NULL;
> + }
>
> - val = smap_get(ovs_other_config, flag);
> + if (strstr(suffix, "..")) {
> + VLOG_ERR("ignoring vhost-sock-dir '%s' with invalid characters '..'",
> + suffix);
> + return NULL;
> + }
>
> - /* Process the vhost-sock-dir flag if it is provided, otherwise resort to
> - * default value.
> - */
> - if (val && (strlen(val) <= size)) {
> - changed = 1;
> - *new_val = xstrdup(val);
> - VLOG_INFO("User-provided %s in use: %s", flag, *new_val);
> - } else {
> - VLOG_INFO("No %s provided - defaulting to %s", flag, default_val);
> - *new_val = xstrdup(default_val);
> + struct stat s;
> + char *dir = xasprintf("%s/%s", stem, suffix);
> + if (stat(dir, &s)) {
> + VLOG_ERR("%s: ignoring requested vhost socket directory (%s)",
> + dir, ovs_strerror(errno));
> + free(dir);
> + return NULL;
> }
>
> - return changed;
> + return dir;
> }
>
> static char **
> @@ -311,7 +311,6 @@ dpdk_init__(const struct smap *ovs_other_config)
> bool auto_determine = true;
> int err = 0;
> cpu_set_t cpuset;
> - char *sock_dir_subcomponent;
>
> log_stream = fopencookie(NULL, "w+", dpdk_log_func);
> if (log_stream == NULL) {
> @@ -321,28 +320,11 @@ dpdk_init__(const struct smap *ovs_other_config)
> rte_openlog_stream(log_stream);
> }
>
> - if (process_vhost_flags("vhost-sock-dir", ovs_rundir(),
> - NAME_MAX, ovs_other_config,
> - &sock_dir_subcomponent)) {
> - struct stat s;
> - if (!strstr(sock_dir_subcomponent, "..")) {
> - vhost_sock_dir = xasprintf("%s/%s", ovs_rundir(),
> - sock_dir_subcomponent);
> -
> - err = stat(vhost_sock_dir, &s);
> - if (err) {
> - VLOG_ERR("vhost-user sock directory '%s' does not exist.",
> - vhost_sock_dir);
> - }
> - } else {
> - vhost_sock_dir = xstrdup(ovs_rundir());
> - VLOG_ERR("vhost-user sock directory request '%s/%s' has invalid"
> - "characters '..' - using %s instead.",
> - ovs_rundir(), sock_dir_subcomponent, ovs_rundir());
> - }
> - free(sock_dir_subcomponent);
> - } else {
> - vhost_sock_dir = sock_dir_subcomponent;
> + vhost_sock_dir = get_custom_vhost_sock_dir(ovs_rundir(),
> + smap_get(ovs_other_config,
> + "vhost-sock-dir"));
> + if (!vhost_sock_dir) {
> + vhost_sock_dir = xstrdup(ovs_rundir());
> }
Because of the way DPDK works, I think we need a VLOG_INFO here
indicating which path we chose for the vhost socket directory.
Otherwise, if the user changes the db value and forgets to restart, we
have no indicator to tell us where to look. Unless there's some other
way of knowing.
Otherwise, LGTM.
>
> argv = grow_argv(&argv, 0, 1);
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
