Re: [libvirt] [PATCH 1/3] util: new function virNetDevOpenvswitchInterfaceGetMaster()

2018-07-02 Thread Laine Stump
On 07/02/2018 02:11 AM, Michal Prívozník wrote:
> On 07/02/2018 01:46 AM, Laine Stump wrote:
>> +virNetDevOpenvswitchAddTimeout(cmd);
>> +virCommandAddArgList(cmd, "iface-to-br", ifname, NULL);
>> +virCommandSetOutputBuffer(cmd, master);
>> +
>> +if (virCommandRun(cmd, ) < 0) {
>> +virReportError(VIR_ERR_INTERNAL_ERROR,
>> +   _("Unable to run command to get OVS master for "
>> + "interface %s"), ifname);
>> +goto cleanup;
>> +}
>> +
>> +/* non-0 exit code just means that the interface has no master in OVS */
>> +if (exitstatus != 0)
>> +VIR_FREE(*master);
> A-ha! We indeed need to do this. I was under impression that just like
> other functions of ours if they fail no allocation is done. But this is
> different so we need to call VIR_FREE().

Right. If ovs-vsctl is run and returns non-0, there still might have
been some sort of output to stdout (or there might not have been, but I
don't want to rely on it).

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 1/3] util: new function virNetDevOpenvswitchInterfaceGetMaster()

2018-07-02 Thread Michal Prívozník
On 07/02/2018 01:46 AM, Laine Stump wrote:
> This function retrieves the name of the OVS bridge that the given
> netdev is attached to. This separate function is necessary because OVS
> set the IFLA_MASTER attribute to "ovs-system" for all netdevs that are
> attached to an OVS bridge, so the standard method of retrieving the
> master can't be used.
> 
> Signed-off-by: Laine Stump 
> ---
>  src/libvirt_private.syms|  1 +
>  src/util/virnetdevopenvswitch.c | 55 
> +
>  src/util/virnetdevopenvswitch.h |  6 +
>  3 files changed, 62 insertions(+)
> 
> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index 98913a577a..386f53eeb9 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -2364,6 +2364,7 @@ virNetDevMidonetUnbindPort;
>  virNetDevOpenvswitchAddPort;
>  virNetDevOpenvswitchGetMigrateData;
>  virNetDevOpenvswitchGetVhostuserIfname;
> +virNetDevOpenvswitchInterfaceGetMaster;
>  virNetDevOpenvswitchInterfaceStats;
>  virNetDevOpenvswitchRemovePort;
>  virNetDevOpenvswitchSetMigrateData;
> diff --git a/src/util/virnetdevopenvswitch.c b/src/util/virnetdevopenvswitch.c
> index f86f698430..af3f2a773d 100644
> --- a/src/util/virnetdevopenvswitch.c
> +++ b/src/util/virnetdevopenvswitch.c
> @@ -404,6 +404,61 @@ virNetDevOpenvswitchInterfaceStats(const char *ifname,
>  return ret;
>  }
>  
> +
> +/**
> + * virNetDeOpenvswitchGetMaster:
> + * @ifname: name of interface we're interested in
> + * @master: used to return a string containing the name of @ifname's "master"
> + *  (this is the bridge or bond device that this device is attached 
> to)
> + *
> + * Returns 0 on success, -1 on failure (if @ifname has no master
> + * @master will be NULL, but return value will still be 0 (success)).
> + *
> + * NB: This function is needed because the IFLA_MASTER attribute of an
> + * interface in a netlink dump (see virNetDevGetMaster()) will always
> + * return "ovs-system" for any interface that is attached to an OVS
> + * switch. When that happens, virNetDevOpenvswitchInterfaceGetMaster()
> + * must be called to get the "real" master of the interface.
> + */
> +int
> +virNetDevOpenvswitchInterfaceGetMaster(const char *ifname, char **master)
> +{
> +virCommandPtr cmd = NULL;
> +int ret = -1;
> +int exitstatus;
> +
> +*master = NULL;
> +cmd = virCommandNew(OVSVSCTL);

I'd put an empty line in between these two ^^ because from data POV we
have two different blocks (one initializes variables, the other
constructs @cmd). But it is really a nit pick.

> +virNetDevOpenvswitchAddTimeout(cmd);
> +virCommandAddArgList(cmd, "iface-to-br", ifname, NULL);
> +virCommandSetOutputBuffer(cmd, master);
> +
> +if (virCommandRun(cmd, ) < 0) {
> +virReportError(VIR_ERR_INTERNAL_ERROR,
> +   _("Unable to run command to get OVS master for "
> + "interface %s"), ifname);
> +goto cleanup;
> +}
> +
> +/* non-0 exit code just means that the interface has no master in OVS */
> +if (exitstatus != 0)
> +VIR_FREE(*master);

A-ha! We indeed need to do this. I was under impression that just like
other functions of ours if they fail no allocation is done. But this is
different so we need to call VIR_FREE().


Michal

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


[libvirt] [PATCH 1/3] util: new function virNetDevOpenvswitchInterfaceGetMaster()

2018-07-01 Thread Laine Stump
This function retrieves the name of the OVS bridge that the given
netdev is attached to. This separate function is necessary because OVS
set the IFLA_MASTER attribute to "ovs-system" for all netdevs that are
attached to an OVS bridge, so the standard method of retrieving the
master can't be used.

Signed-off-by: Laine Stump 
---
 src/libvirt_private.syms|  1 +
 src/util/virnetdevopenvswitch.c | 55 +
 src/util/virnetdevopenvswitch.h |  6 +
 3 files changed, 62 insertions(+)

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 98913a577a..386f53eeb9 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -2364,6 +2364,7 @@ virNetDevMidonetUnbindPort;
 virNetDevOpenvswitchAddPort;
 virNetDevOpenvswitchGetMigrateData;
 virNetDevOpenvswitchGetVhostuserIfname;
+virNetDevOpenvswitchInterfaceGetMaster;
 virNetDevOpenvswitchInterfaceStats;
 virNetDevOpenvswitchRemovePort;
 virNetDevOpenvswitchSetMigrateData;
diff --git a/src/util/virnetdevopenvswitch.c b/src/util/virnetdevopenvswitch.c
index f86f698430..af3f2a773d 100644
--- a/src/util/virnetdevopenvswitch.c
+++ b/src/util/virnetdevopenvswitch.c
@@ -404,6 +404,61 @@ virNetDevOpenvswitchInterfaceStats(const char *ifname,
 return ret;
 }
 
+
+/**
+ * virNetDeOpenvswitchGetMaster:
+ * @ifname: name of interface we're interested in
+ * @master: used to return a string containing the name of @ifname's "master"
+ *  (this is the bridge or bond device that this device is attached to)
+ *
+ * Returns 0 on success, -1 on failure (if @ifname has no master
+ * @master will be NULL, but return value will still be 0 (success)).
+ *
+ * NB: This function is needed because the IFLA_MASTER attribute of an
+ * interface in a netlink dump (see virNetDevGetMaster()) will always
+ * return "ovs-system" for any interface that is attached to an OVS
+ * switch. When that happens, virNetDevOpenvswitchInterfaceGetMaster()
+ * must be called to get the "real" master of the interface.
+ */
+int
+virNetDevOpenvswitchInterfaceGetMaster(const char *ifname, char **master)
+{
+virCommandPtr cmd = NULL;
+int ret = -1;
+int exitstatus;
+
+*master = NULL;
+cmd = virCommandNew(OVSVSCTL);
+virNetDevOpenvswitchAddTimeout(cmd);
+virCommandAddArgList(cmd, "iface-to-br", ifname, NULL);
+virCommandSetOutputBuffer(cmd, master);
+
+if (virCommandRun(cmd, ) < 0) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _("Unable to run command to get OVS master for "
+ "interface %s"), ifname);
+goto cleanup;
+}
+
+/* non-0 exit code just means that the interface has no master in OVS */
+if (exitstatus != 0)
+VIR_FREE(*master);
+
+if (*master) {
+/* truncate at the first newline */
+char *nl = strchr(*master, '\n');
+if (nl)
+*nl = '\0';
+}
+
+VIR_DEBUG("OVS master for %s is %s", ifname, *master ? *master : "(none)");
+
+ret = 0;
+ cleanup:
+return ret;
+}
+
+
 /**
  * virNetDevOpenvswitchVhostuserGetIfname:
  * @path: the path of the unix socket
diff --git a/src/util/virnetdevopenvswitch.h b/src/util/virnetdevopenvswitch.h
index 6f6e620c22..331e483018 100644
--- a/src/util/virnetdevopenvswitch.h
+++ b/src/util/virnetdevopenvswitch.h
@@ -47,6 +47,9 @@ int virNetDevOpenvswitchAddPort(const char *brname,
 int virNetDevOpenvswitchRemovePort(const char *brname, const char *ifname)
 ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK;
 
+int virNetDevOpenvswitchInterfaceGetMaster(const char *ifname, char **master)
+ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK;
+
 int virNetDevOpenvswitchGetMigrateData(char **migrate, const char *ifname)
 ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK;
 
@@ -57,6 +60,9 @@ int virNetDevOpenvswitchInterfaceStats(const char *ifname,
virDomainInterfaceStatsPtr stats)
 ATTRIBUTE_NONNULL(1) ATTRIBUTE_RETURN_CHECK;
 
+int virNetDevOpenvswitchInterfaceGetMaster(const char *ifname, char **master)
+ATTRIBUTE_NONNULL(1) ATTRIBUTE_RETURN_CHECK;
+
 int virNetDevOpenvswitchGetVhostuserIfname(const char *path,
char **ifname)
 ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK ATTRIBUTE_NOINLINE;
-- 
2.14.4

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list