[libvirt] [PATCH 3/6] lxc: Converting full string entries in types only.

Julio Faracco posted 6 patches 6 years, 11 months ago
[libvirt] [PATCH 3/6] lxc: Converting full string entries in types only.
Posted by Julio Faracco 6 years, 11 months ago
This commit removes the full network entry setting: "lxc.network.X" to
type only. Like "type", "name", "flags", etc. So, here no matter if the
settings is "lxc.network.X" or "lxc.net.X.Y".

Signed-off-by: Julio Faracco <jcfaracco@gmail.com>
---
 src/lxc/lxc_native.c | 30 +++++++++++++++---------------
 1 file changed, 15 insertions(+), 15 deletions(-)

diff --git a/src/lxc/lxc_native.c b/src/lxc/lxc_native.c
index c144f3c52e..ed50415a93 100644
--- a/src/lxc/lxc_native.c
+++ b/src/lxc/lxc_native.c
@@ -562,7 +562,7 @@ lxcNetworkParseDataIPs(const char *name, virConfValuePtr value, lxcNetworkParseD
     if (VIR_ALLOC(ip) < 0)
         return -1;
 
-    if (STREQ(name, "lxc.network.ipv6"))
+    if (STREQ(name, "ipv6"))
         family = AF_INET6;
 
     ipparts = virStringSplit(value->str, "/", 2);
@@ -589,12 +589,11 @@ lxcNetworkParseDataIPs(const char *name, virConfValuePtr value, lxcNetworkParseD
 }
 
 static int
-lxcNetworkWalkCallback(const char *name, virConfValuePtr value, void *data)
+lxcNetworkParseDataSuffix(const char *name, virConfValuePtr value, lxcNetworkParseData *parseData)
 {
-    lxcNetworkParseData *parseData = data;
     int status;
 
-    if (STREQ(name, "lxc.network.type")) {
+    if (STREQ(name, "type")) {
         virDomainDefPtr def = parseData->def;
         size_t networks = parseData->networks;
         bool privnet = parseData->privnet;
@@ -619,30 +618,31 @@ lxcNetworkWalkCallback(const char *name, virConfValuePtr value, void *data)
         /* Keep the new value */
         parseData->type = value->str;
     }
-    else if (STREQ(name, "lxc.network.link"))
+    else if (STREQ(name, "link"))
         parseData->link = value->str;
-    else if (STREQ(name, "lxc.network.hwaddr"))
+    else if (STREQ(name, "hwaddr"))
         parseData->mac = value->str;
-    else if (STREQ(name, "lxc.network.flags"))
+    else if (STREQ(name, "flags"))
         parseData->flag = value->str;
-    else if (STREQ(name, "lxc.network.macvlan.mode"))
+    else if (STREQ(name, "macvlan.mode"))
         parseData->macvlanmode = value->str;
-    else if (STREQ(name, "lxc.network.vlan.id"))
+    else if (STREQ(name, "vlan.id"))
         parseData->vlanid = value->str;
-    else if (STREQ(name, "lxc.network.name"))
+    else if (STREQ(name, "name"))
         parseData->name = value->str;
-    else if (STREQ(name, "lxc.network.ipv4") ||
-             STREQ(name, "lxc.network.ipv6")) {
+    else if (STREQ(name, "ipv4") ||
+             STREQ(name, "ipv6")) {
         if (lxcNetworkParseDataIPs(name, value, parseData) < 0)
             return -1;
-    } else if (STREQ(name, "lxc.network.ipv4.gateway")) {
+    } else if (STREQ(name, "ipv4.gateway")) {
         parseData->gateway_ipv4 = value->str;
-    } else if (STREQ(name, "lxc.network.ipv6.gateway")) {
+    } else if (STREQ(name, "ipv6.gateway")) {
         parseData->gateway_ipv6 = value->str;
-    } else if (STRPREFIX(name, "lxc.network")) {
+    } else {
         VIR_WARN("Unhandled network property: %s = %s",
                  name,
                  value->str);
+        return -1;
     }
 
     return 0;
-- 
2.19.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/6] lxc: Converting full string entries in types only.
Posted by John Ferlan 6 years, 11 months ago
$SUBJ:

lxc: Introduce lxcNetworkParseDataSuffix

On 2/18/19 2:09 PM, Julio Faracco wrote:
> This commit removes the full network entry setting: "lxc.network.X" to
> type only. Like "type", "name", "flags", etc. So, here no matter if the
> settings is "lxc.network.X" or "lxc.net.X.Y".

Replace last sentence w/

This will handle entries regardless of whether they are prefixed by
"lxc.network." (today) or "lxc.net.X" (the future).

> 
> Signed-off-by: Julio Faracco <jcfaracco@gmail.com>
> ---
>  src/lxc/lxc_native.c | 30 +++++++++++++++---------------
>  1 file changed, 15 insertions(+), 15 deletions(-)
> 

Reviewed-by: John Ferlan <jferlan@redhat.com>

John

[...]

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/6] lxc: Converting full string entries in types only.
Posted by Julio Faracco 6 years, 11 months ago
Hi John,

I'm seeing clang warnings after applying patches.
Clang is complaining about this line:

> -    if (STREQ(name, "lxc.network.type")) {
> +    if (STREQ(name, "type")) {
>          virDomainDefPtr def = parseData->def;

I think it is irrelevant because there is no way to get a NULL pointer
from case structure.
During my tests, I'm being redirected to "Unhandled network property".
Technically, only lxcNetworkParseDataSuffix is using this method.
Btw, here is a possible fix for that...

--- a/src/lxc/lxc_native.c
+++ b/src/lxc/lxc_native.c
@@ -612,7 +612,7 @@ lxcNetworkParseDataIPs(const char *name,
     if (VIR_ALLOC(ip) < 0)
         return -1;

-    if (STREQ(name, "ipv6"))
+    if (STREQ_NULLABLE(name, "ipv6"))
         family = AF_INET6;

     ipparts = virStringSplit(value->str, "/", 2);

--
Julio Cesar Faracco

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/6] lxc: Converting full string entries in types only.
Posted by John Ferlan 6 years, 11 months ago

On 2/26/19 9:08 AM, Julio Faracco wrote:
> Hi John,
> 
> I'm seeing clang warnings after applying patches.
> Clang is complaining about this line:
> 
>> -    if (STREQ(name, "lxc.network.type")) {
>> +    if (STREQ(name, "type")) {
>>          virDomainDefPtr def = parseData->def;
> 
> I think it is irrelevant because there is no way to get a NULL pointer
> from case structure.
> During my tests, I'm being redirected to "Unhandled network property".
> Technically, only lxcNetworkParseDataSuffix is using this method.
> Btw, here is a possible fix for that...
> 

Hmm.. I don't typically compile with clang, although I know there are a
few on the team that do and they haven't posted anything yet. I did see
on the #virt channel that travis-ci's bot complained:

https://travis-ci.org/libvirt/libvirt/builds/498459437

However, in looking at the output, it wasn't a compilation failure,
rather it was a failure to 'mv mr.gmo-t mr.gmo' it seems which would be
unrelated.

I also just installed clang 7.0.1, ran CC=clang sh autogen.sh, and then
did a build, but didn't see any failure.

If anything - I would think a well placed sa_assert(suffix) in
lxcNetworkParseDataEntry would assert to the compiler that suffix isn't
NULL. Of course the fact that lxcNetworkParseDataIPs got called means
that virLXCNetworkConfigEntryTypeFromString had a valid string. So,
perhaps it's an older clang compiler that's issuing the message.

John

> --- a/src/lxc/lxc_native.c
> +++ b/src/lxc/lxc_native.c
> @@ -612,7 +612,7 @@ lxcNetworkParseDataIPs(const char *name,
>      if (VIR_ALLOC(ip) < 0)
>          return -1;
> 
> -    if (STREQ(name, "ipv6"))
> +    if (STREQ_NULLABLE(name, "ipv6"))
>          family = AF_INET6;
> 
>      ipparts = virStringSplit(value->str, "/", 2);
> 
> --
> Julio Cesar Faracco
> 

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