Ok, it makes sense. It looks ok to me now.
Acked-by: Antonio Fischetti <antonio.fische...@intel.com> > -----Original Message----- > From: Aaron Conole [mailto:acon...@redhat.com] > Sent: Tuesday, May 31, 2016 7:20 PM > To: Fischetti, Antonio <antonio.fische...@intel.com> > Cc: Wojciechowicz, RobertX <robertx.wojciechow...@intel.com>; > dev@openvswitch.org > Subject: Re: [ovs-dev] [PATCH] ovsdb: Expose vhost-user socket > directory in ovsdb > > "Fischetti, Antonio" <antonio.fische...@intel.com> writes: > > > Hi Robert, > > one comment below. > > I've checked it applies cleanly to the latest master branch. > > Also with utilities/checkpatch.py is ok. > > > > Thanks, > > Antonio > > > >> -----Original Message----- > >> From: dev [mailto:dev-boun...@openvswitch.org] On Behalf Of Robert > >> Wojciechowicz > >> Sent: Monday, May 23, 2016 11:29 AM > >> To: dev@openvswitch.org > >> Subject: [ovs-dev] [PATCH] ovsdb: Expose vhost-user socket > directory > >> in ovsdb > >> > >> In order to correctly interoperate with Openstack and ODL, > >> the vhost-user socket directory must be exposed from OVS via > OVSDB. > >> Different distros may package OVS in different ways, > >> so the locations of these sockets may vary depending on how > >> ovs-vswitchd has been started. Some clients need information where > >> the sockets are located when instantiating Qemu virtual machines. > >> The full vhost-user socket directory is constructed from current > >> OVS working directory and optionally from specified subdirectory. > >> This patch exposes vhost-user socket directory in Open_vSwitch > >> table in other_config column in two following keys: > >> 1. ovs-run-dir - OVS working directory > >> 2. vhost-sock-dir - subdirectory of ovs-run-dir (might be empty) > >> > >> Signed-off-by: Robert Wojciechowicz > <robertx.wojciechow...@intel.com> > >> --- > >> lib/netdev-dpdk.c | 38 ++++++++++++++++++++++++++++++++++---- > >> lib/netdev-dpdk.h | 9 +++++++++ > >> vswitchd/bridge.c | 2 ++ > >> vswitchd/vswitch.xml | 11 +++++++++++ > >> 4 files changed, 56 insertions(+), 4 deletions(-) > >> > >> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c > >> index 9ffa7ff..4e68bd6 100644 > >> --- a/lib/netdev-dpdk.c > >> +++ b/lib/netdev-dpdk.c > >> @@ -138,9 +138,13 @@ BUILD_ASSERT_DECL((MAX_NB_MBUF / > >> ROUND_DOWN_POW2(MAX_NB_MBUF/MIN_NB_MBUF)) > >> > >> #ifdef VHOST_CUSE > >> static char *cuse_dev_name = NULL; /* Character device > >> cuse_dev_name. */ > >> +#else > >> +static char *sock_dir_subcomponent = NULL; /* Subdir of OVS run > >> dir > >> + for vhost-user > >> sockets */ > >> #endif > >> static char *vhost_sock_dir = NULL; /* Location of vhost-user > >> sockets */ > >> > >> + > >> /* > >> * Maximum amount of time in micro seconds to try and enqueue to > >> vhost. > >> */ > >> @@ -3086,9 +3090,6 @@ dpdk_init__(const struct smap > >> *ovs_other_config) > >> bool auto_determine = true; > >> int err = 0; > >> cpu_set_t cpuset; > >> -#ifndef VHOST_CUSE > >> - char *sock_dir_subcomponent; > >> -#endif > >> > >> if (!smap_get_bool(ovs_other_config, "dpdk-init", false)) { > >> VLOG_INFO("DPDK Disabled - to change this requires a > >> restart.\n"); > >> @@ -3119,10 +3120,11 @@ dpdk_init__(const struct smap > >> *ovs_other_config) > > > > [Antonio F] Just wondering if inside the previous lines > > where the vhu sock does not exist, should we consider to > > restore vhost_sock_dir to ovs_rundir(), like: > > > > err = stat(vhost_sock_dir, &s); > > if (err) { > > VLOG_ERR("vhost-user sock directory '%s' does not > exist.", > > vhost_sock_dir); > > > > + vhost_sock_dir = xasprintf("%s", ovs_rundir()); > > + sock_dir_subcomponent = ""; > > > > } > > > > ? > > Or is that ok to leave it as it is now? > > I haven't reviewed the whole patch, but I don't think this would be > an > appropriate change. As it stands, if a user sees this error message, > they may simply mkdir the missing subdirectory, and then dpdk vhost- > user > sockets will begin to work. With your change, not only will the > error > message be displayed, but user sockets will be popping up in > unexpected > places, meaning a restart will be required to resolve the issue > fully. > > >> VLOG_ERR("vhost-user sock directory request '%s/%s' > has > >> invalid" > >> "characters '..' - using %s instead.", > >> ovs_rundir(), sock_dir_subcomponent, > >> ovs_rundir()); > >> + sock_dir_subcomponent = ""; > >> } > >> - free(sock_dir_subcomponent); > >> } else { > >> vhost_sock_dir = sock_dir_subcomponent; > >> + sock_dir_subcomponent = ""; > >> #endif > >> } > >> > >> @@ -3244,6 +3246,34 @@ dpdk_init(const struct smap > *ovs_other_config) > >> } > >> } > >> > >> +void > >> +dpdk_set_config(const struct ovsrec_open_vswitch *cfg) > >> +{ > >> + struct smap dpdk_args; > >> + const struct ovsdb_datum *datum; > >> + size_t i; > >> + > >> + ovs_mutex_lock(&dpdk_mutex); > >> + > >> + smap_init(&dpdk_args); > >> + /* read current values from database */ > >> + datum = ovsrec_open_vswitch_get_other_config(cfg, > >> OVSDB_TYPE_STRING, > >> + > OVSDB_TYPE_STRING); > >> + for (i = 0; i < datum->n; i++) { > >> + smap_add(&dpdk_args, datum->keys[i].string, datum- > >> >values[i].string); > >> + } > >> + /* add default paths to the database */ > >> + smap_add_format(&dpdk_args, "ovs-run-dir", "%s", > ovs_rundir()); > >> + if (sock_dir_subcomponent) { > >> + smap_add_format(&dpdk_args, "vhost-sock-dir", "%s", > >> + sock_dir_subcomponent); > >> + } > >> + ovsrec_open_vswitch_set_other_config(cfg, &dpdk_args); > >> + smap_destroy(&dpdk_args); > >> + > >> + ovs_mutex_unlock(&dpdk_mutex); > >> +} > >> + > >> static const struct netdev_class dpdk_class = > >> NETDEV_DPDK_CLASS( > >> "dpdk", > >> diff --git a/lib/netdev-dpdk.h b/lib/netdev-dpdk.h > >> index ee748e0..08abcac 100644 > >> --- a/lib/netdev-dpdk.h > >> +++ b/lib/netdev-dpdk.h > >> @@ -3,6 +3,8 @@ > >> > >> #include <config.h> > >> > >> +#include "lib/vswitch-idl.h" > >> + > >> struct dp_packet; > >> struct smap; > >> > >> @@ -25,6 +27,7 @@ struct smap; > >> > >> void netdev_dpdk_register(void); > >> void free_dpdk_buf(struct dp_packet *); > >> +void dpdk_set_config(const struct ovsrec_open_vswitch *cfg); > >> int pmd_thread_setaffinity_cpu(unsigned cpu); > >> > >> #else > >> @@ -45,6 +48,12 @@ free_dpdk_buf(struct dp_packet *buf OVS_UNUSED) > >> /* Nothing */ > >> } > >> > >> +static inline void > >> +dpdk_set_config(const struct ovsrec_open_vswitch *cfg) > >> +{ > >> + /* Nothing */ > >> +} > >> + > >> static inline int > >> pmd_thread_setaffinity_cpu(unsigned cpu OVS_UNUSED) > >> { > >> diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c > >> index 41ec4ba..d248721 100644 > >> --- a/vswitchd/bridge.c > >> +++ b/vswitchd/bridge.c > >> @@ -393,6 +393,7 @@ bridge_init(const char *remote) > >> ovsdb_idl_omit(idl, &ovsrec_open_vswitch_col_db_version); > >> ovsdb_idl_omit(idl, &ovsrec_open_vswitch_col_system_type); > >> ovsdb_idl_omit(idl, &ovsrec_open_vswitch_col_system_version); > >> + ovsdb_idl_omit_alert(idl, > >> &ovsrec_open_vswitch_col_other_config); > >> > >> ovsdb_idl_omit_alert(idl, &ovsrec_bridge_col_datapath_id); > >> ovsdb_idl_omit_alert(idl, > &ovsrec_bridge_col_datapath_version); > >> @@ -2957,6 +2958,7 @@ bridge_run(void) > >> > >> if (cfg) { > >> ovsrec_open_vswitch_set_cur_cfg(cfg, cfg->next_cfg); > >> + dpdk_set_config(cfg); > >> discover_types(cfg); > >> } > >> > >> diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml > >> index 944d8ec..8d06d29 100644 > >> --- a/vswitchd/vswitch.xml > >> +++ b/vswitchd/vswitch.xml > >> @@ -311,6 +311,17 @@ > >> </p> > >> </column> > >> > >> + <column name="other_config" key="ovs-run-dir" > >> + type='{"type": "string"}'> > >> + <p> > >> + Specifies the Open vSwitch run directory. > >> + </p> > >> + <p> > >> + Defaults to the working directory of the application. > >> Changing this > >> + value requires restarting the daemon. > >> + </p> > >> + </column> > >> + > >> <column name="other_config" key="n-handler-threads" > >> type='{"type": "integer", "minInteger": 1}'> > >> <p> > >> -- > >> 1.8.3.1 > >> > >> _______________________________________________ > >> dev mailing list > >> dev@openvswitch.org > >> http://openvswitch.org/mailman/listinfo/dev > > _______________________________________________ > > dev mailing list > > dev@openvswitch.org > > http://openvswitch.org/mailman/listinfo/dev _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
