On 12/16/20 8:17 PM, Laine Stump wrote:
On 12/16/20 1:45 PM, Michal Privoznik wrote:
During testing of my patch v6.10.0-rc1~221 it was found that
'ovs-vsctl get Interface $name name' or 'ovs-vsctl find Interface options:vhost-server-path=$path'
may return a string in double quotes, e.g. "vhost-user1". Later investigation of openvswitch code showed, that early versions (like 1.3.0) have somewhat restrictive set of safe characters (isalpha() || '_' || '-' || '.'), which is then refined with increasing version. For instance, version 2.11.4 has: isalnum() || '_' || '-' || '.'. If the string that ovs-vsctl wants to output contains any other character it is escaped. You want to be looking at ovsdb_atom_to_string() which handles outputting of a single string and calls string_needs_quotes() and possibly json_serialize_string() in openvswitch code base.
Since the interfaces are usually named "vhost-userN" we are facing a problem where with one version we get the name in double quotes and with another we get plain name without funny business.
Because of json involved I thought, let's make ovs-vsctl output into JSON format and then use our JSON parser, but guess what - ovs-vsctl ignores --format=json.
Yuck. Is this a known/reported problem? Is it true with current ovs-vsctl, or just with old versions? (I guess in the end it doesn't really matter, because we have to support old and new anyway :-/)
Honestly, I did not bother exactly because of the reason you're mentioning: we have to support both.
But with a little help of g_strdup_printf() it can be turned into JSON.
Fixes: e4c29e2904197472919d050c67acfd59f0144bbc Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1767013 Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/libvirt_private.syms | 1 + src/util/virnetdevopenvswitch.c | 47 +++++++++++++++++++++++++++++ src/util/virnetdevopenvswitch.h | 4 +++ tests/virnetdevopenvswitchtest.c | 52 ++++++++++++++++++++++++++++++++ 4 files changed, 104 insertions(+)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index c7c37d9689..583fc5800e 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -2676,6 +2676,7 @@ virNetDevOpenvswitchGetVhostuserIfname; virNetDevOpenvswitchInterfaceGetMaster; virNetDevOpenvswitchInterfaceParseStats; virNetDevOpenvswitchInterfaceStats; +virNetDevOpenvswitchMaybeUnescapeReply; virNetDevOpenvswitchRemovePort; virNetDevOpenvswitchSetMigrateData; virNetDevOpenvswitchSetTimeout; diff --git a/src/util/virnetdevopenvswitch.c b/src/util/virnetdevopenvswitch.c index 7eabaa763d..14fa294ae1 100644 --- a/src/util/virnetdevopenvswitch.c +++ b/src/util/virnetdevopenvswitch.c @@ -460,6 +460,48 @@ virNetDevOpenvswitchInterfaceGetMaster(const char *ifname, char **master) } +/** + * virNetDevOpenvswitchMaybeUnescapeReply: + * @reply: a string to unescape + * + * Depending on ovs-vsctl version a string might be escaped. For instance: + * -version 2.11.4 allows only is_alpha(), an underscore, a dash or a dot, + * -version 2.14.0 allows only is_alnum(), an underscore, a dash or a dot, + * any other character causes the string to be escaped. + * + * What this function does, is it checks whether @reply string consists solely + * from safe, not escaped characters (as defined by version 2.14.0) and if not + * an error is reported. If @reply is a string enclosed in double quotes, but + * otherwise safe those double quotes are removed. + * + * Returns: 0 on success, + * -1 otherwise (with error reported). + */ +int +virNetDevOpenvswitchMaybeUnescapeReply(char *reply) +{ + g_autoptr(virJSONValue) json = NULL; + g_autofree char *jsonStr = NULL; + const char *tmp = NULL; + size_t replyLen = strlen(reply); + + if (*reply != '"') + return 0; + + jsonStr = g_strdup_printf("{\"name\": %s}", reply); + if (!(json = virJSONValueFromString(jsonStr))) + return -1; + + if (!(tmp = virJSONValueObjectGetString(json, "name"))) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Malformed ovs-vsctl output")); + return -1; + }
Tricky!
There are other ways to unescape a string, but this one gets a vote for novelty :-)
Thank you sir!
Reviewed-by: Laine Stump <laine@redhat.com>
Michal