Make QEMU installation relocatable

[PATCH 07/13] net: relocate paths to helpers and scripts

Posted by Paolo Bonzini 5 years, 5 months ago

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 include/net/net.h |  4 ++--
 net/tap.c         | 28 +++++++++++++++++++++-------
 2 files changed, 23 insertions(+), 9 deletions(-)

diff --git a/include/net/net.h b/include/net/net.h
index e7ef42d62b..897b2d7595 100644
--- a/include/net/net.h
+++ b/include/net/net.h
@@ -209,8 +209,8 @@ void netdev_add(QemuOpts *opts, Error **errp);
 int net_hub_id_for_client(NetClientState *nc, int *id);
 NetClientState *net_hub_port_find(int hub_id);
 
-#define DEFAULT_NETWORK_SCRIPT "/etc/qemu-ifup"
-#define DEFAULT_NETWORK_DOWN_SCRIPT "/etc/qemu-ifdown"
+#define DEFAULT_NETWORK_SCRIPT CONFIG_SYSCONFDIR "/qemu-ifup"
+#define DEFAULT_NETWORK_DOWN_SCRIPT CONFIG_SYSCONFDIR "/qemu-ifdown"
 #define DEFAULT_BRIDGE_HELPER CONFIG_QEMU_HELPERDIR "/qemu-bridge-helper"
 #define DEFAULT_BRIDGE_INTERFACE "br0"
 
diff --git a/net/tap.c b/net/tap.c
index 14dc904fca..a271046461 100644
--- a/net/tap.c
+++ b/net/tap.c
@@ -582,14 +582,20 @@ int net_init_bridge(const Netdev *netdev, const char *name,
                     NetClientState *peer, Error **errp)
 {
     const NetdevBridgeOptions *bridge;
-    const char *helper, *br;
+    g_autofree char *default_helper = NULL;
+    const char *helper;
+    const char *br;
     TAPState *s;
     int fd, vnet_hdr;
 
     assert(netdev->type == NET_CLIENT_DRIVER_BRIDGE);
     bridge = &netdev->u.bridge;
 
-    helper = bridge->has_helper ? bridge->helper : DEFAULT_BRIDGE_HELPER;
+    if (!bridge->has_helper) {
+        helper = default_helper = get_relocated_path(DEFAULT_BRIDGE_HELPER);
+    } else {
+        helper = bridge->helper;
+    }
     br     = bridge->has_br     ? bridge->br     : DEFAULT_BRIDGE_INTERFACE;
 
     fd = net_bridge_run_helper(helper, br, errp);
@@ -773,8 +779,8 @@ int net_init_tap(const Netdev *netdev, const char *name,
     const NetdevTapOptions *tap;
     int fd, vnet_hdr = 0, i = 0, queues;
     /* for the no-fd, no-helper case */
-    const char *script = NULL; /* suppress wrong "uninit'd use" gcc warning */
-    const char *downscript = NULL;
+    const char *script;
+    const char *downscript;
     Error *err = NULL;
     const char *vhostfdname;
     char ifname[128];
@@ -784,6 +790,8 @@ int net_init_tap(const Netdev *netdev, const char *name,
     tap = &netdev->u.tap;
     queues = tap->has_queues ? tap->queues : 1;
     vhostfdname = tap->has_vhostfd ? tap->vhostfd : NULL;
+    script = tap->has_script ? tap->script : NULL;
+    downscript = tap->has_downscript ? tap->downscript : NULL;
 
     /* QEMU hubs do not support multiqueue tap, in this case peer is set.
      * For -netdev, peer is always NULL. */
@@ -934,13 +942,19 @@ free_fail:
             return -1;
         }
     } else {
+        g_autofree char *default_script = NULL;
+        g_autofree char *default_downscript = NULL;
         if (tap->has_vhostfds) {
             error_setg(errp, "vhostfds= is invalid if fds= wasn't specified");
             return -1;
         }
-        script = tap->has_script ? tap->script : DEFAULT_NETWORK_SCRIPT;
-        downscript = tap->has_downscript ? tap->downscript :
-            DEFAULT_NETWORK_DOWN_SCRIPT;
+
+        if (!tap->has_script) {
+            script = default_script = get_relocated_path(DEFAULT_NETWORK_SCRIPT);
+        }
+        if (!tap->has_downscript) {
+            downscript = default_downscript = get_relocated_path(DEFAULT_NETWORK_SCRIPT);
+        }
 
         if (tap->has_ifname) {
             pstrcpy(ifname, sizeof ifname, tap->ifname);
-- 
2.26.2

Re: [PATCH 07/13] net: relocate paths to helpers and scripts

Posted by Philippe Mathieu-Daudé 5 years, 5 months ago

On 9/1/20 8:20 AM, Paolo Bonzini wrote:
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  include/net/net.h |  4 ++--
>  net/tap.c         | 28 +++++++++++++++++++++-------
>  2 files changed, 23 insertions(+), 9 deletions(-)
> 
> diff --git a/include/net/net.h b/include/net/net.h
> index e7ef42d62b..897b2d7595 100644
> --- a/include/net/net.h
> +++ b/include/net/net.h
> @@ -209,8 +209,8 @@ void netdev_add(QemuOpts *opts, Error **errp);
>  int net_hub_id_for_client(NetClientState *nc, int *id);
>  NetClientState *net_hub_port_find(int hub_id);
>  
> -#define DEFAULT_NETWORK_SCRIPT "/etc/qemu-ifup"
> -#define DEFAULT_NETWORK_DOWN_SCRIPT "/etc/qemu-ifdown"
> +#define DEFAULT_NETWORK_SCRIPT CONFIG_SYSCONFDIR "/qemu-ifup"
> +#define DEFAULT_NETWORK_DOWN_SCRIPT CONFIG_SYSCONFDIR "/qemu-ifdown"
>  #define DEFAULT_BRIDGE_HELPER CONFIG_QEMU_HELPERDIR "/qemu-bridge-helper"
>  #define DEFAULT_BRIDGE_INTERFACE "br0"
>  
> diff --git a/net/tap.c b/net/tap.c
> index 14dc904fca..a271046461 100644
> --- a/net/tap.c
> +++ b/net/tap.c
> @@ -582,14 +582,20 @@ int net_init_bridge(const Netdev *netdev, const char *name,
>                      NetClientState *peer, Error **errp)
>  {
>      const NetdevBridgeOptions *bridge;
> -    const char *helper, *br;
> +    g_autofree char *default_helper = NULL;
> +    const char *helper;
> +    const char *br;
>      TAPState *s;
>      int fd, vnet_hdr;
>  
>      assert(netdev->type == NET_CLIENT_DRIVER_BRIDGE);
>      bridge = &netdev->u.bridge;
>  
> -    helper = bridge->has_helper ? bridge->helper : DEFAULT_BRIDGE_HELPER;
> +    if (!bridge->has_helper) {
> +        helper = default_helper = get_relocated_path(DEFAULT_BRIDGE_HELPER);
> +    } else {
> +        helper = bridge->helper;
> +    }

Nitpicking, I find easier to review adding simply once after out of the
if() statement:

  helper = bridge->helper;

>      br     = bridge->has_br     ? bridge->br     : DEFAULT_BRIDGE_INTERFACE;
>  
>      fd = net_bridge_run_helper(helper, br, errp);
> @@ -773,8 +779,8 @@ int net_init_tap(const Netdev *netdev, const char *name,
>      const NetdevTapOptions *tap;
>      int fd, vnet_hdr = 0, i = 0, queues;
>      /* for the no-fd, no-helper case */
> -    const char *script = NULL; /* suppress wrong "uninit'd use" gcc warning */
> -    const char *downscript = NULL;
> +    const char *script;
> +    const char *downscript;
>      Error *err = NULL;
>      const char *vhostfdname;
>      char ifname[128];
> @@ -784,6 +790,8 @@ int net_init_tap(const Netdev *netdev, const char *name,
>      tap = &netdev->u.tap;
>      queues = tap->has_queues ? tap->queues : 1;
>      vhostfdname = tap->has_vhostfd ? tap->vhostfd : NULL;
> +    script = tap->has_script ? tap->script : NULL;
> +    downscript = tap->has_downscript ? tap->downscript : NULL;
>  
>      /* QEMU hubs do not support multiqueue tap, in this case peer is set.
>       * For -netdev, peer is always NULL. */
> @@ -934,13 +942,19 @@ free_fail:
>              return -1;
>          }
>      } else {
> +        g_autofree char *default_script = NULL;
> +        g_autofree char *default_downscript = NULL;
>          if (tap->has_vhostfds) {
>              error_setg(errp, "vhostfds= is invalid if fds= wasn't specified");
>              return -1;
>          }
> -        script = tap->has_script ? tap->script : DEFAULT_NETWORK_SCRIPT;
> -        downscript = tap->has_downscript ? tap->downscript :
> -            DEFAULT_NETWORK_DOWN_SCRIPT;
> +
> +        if (!tap->has_script) {
> +            script = default_script = get_relocated_path(DEFAULT_NETWORK_SCRIPT);
> +        }
> +        if (!tap->has_downscript) {
> +            downscript = default_downscript = get_relocated_path(DEFAULT_NETWORK_SCRIPT);
> +        }

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

>  
>          if (tap->has_ifname) {
>              pstrcpy(ifname, sizeof ifname, tap->ifname);
>

Re: [PATCH 07/13] net: relocate paths to helpers and scripts

Posted by Paolo Bonzini 5 years, 5 months ago

On 02/09/20 10:24, Philippe Mathieu-Daudé wrote:
>> -    helper = bridge->has_helper ? bridge->helper : DEFAULT_BRIDGE_HELPER;
>> +    if (!bridge->has_helper) {
>> +        helper = default_helper = get_relocated_path(DEFAULT_BRIDGE_HELPER);
>> +    } else {
>> +        helper = bridge->helper;
>> +    }
> Nitpicking, I find easier to review adding simply once after out of the
> if() statement:
> 
>   helper = bridge->helper;
> 

Even better,

-    helper = bridge->has_helper ? bridge->helper : DEFAULT_BRIDGE_HELPER;
+    helper = bridge->has_helper ? bridge->helper : NULL;

and move the get_relocated_path in net_bridge_run_helper.

Paolo