Re: [PATCH] RFC: Support network interface downscript

[Michal Privoznik <mprivozn@xxxxxxxxxx>]

On 5/14/20 5:19 PM, Chen Hanxiao wrote:
> https://gitlab.com/libvirt/libvirt/-/issues/13
>
> Add support for downscript:
>
> <interface type='ethernet'>
>      <mac address='00:11:22:33:44:55'/>
>      <script path='/etc/qemu-ifup'/>
>      <downscript path='/path/to/my/downscript'/>

Uh, this doesn't look very nice, but unfortunately 'script' is not 
future proof, so I guess 'downscript' is our only choice.

> </interface>
>
> But -net ...,script=/etc/qemu-ifup has some limitation:
> https://github.com/qemu/qemu/blob/035b448b84f3557206abc44d786c5d3db2638f7d/net/tap.c#L818
> Maybe virNetDevRunEthernetScript is another choice.
>
> Signed-off-by: Chen Hanxiao <chen_han_xiao@xxxxxxx>
> ---
>   src/conf/domain_conf.c    | 7 +++++++
>   src/conf/domain_conf.h    | 1 +
>   src/qemu/qemu_extdevice.c | 2 ++
>   3 files changed, 10 insertions(+)
>
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index c201fc9..5cb39a3 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -2520,6 +2520,7 @@ virDomainNetDefClear(virDomainNetDefPtr def)
>       VIR_FREE(def->teaming.persistent);
>       VIR_FREE(def->virtPortProfile);
>       VIR_FREE(def->script);
> +    VIR_FREE(def->downscript);
>       VIR_FREE(def->domain_name);
>       VIR_FREE(def->ifname);
>       VIR_FREE(def->ifname_guest);
> @@ -11977,6 +11978,7 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt,
>       g_autofree char *ifname_guest = NULL;
>       g_autofree char *ifname_guest_actual = NULL;
>       g_autofree char *script = NULL;
> +    g_autofree char *downscript = NULL;
>       g_autofree char *address = NULL;
>       g_autofree char *port = NULL;
>       g_autofree char *localaddr = NULL;
> @@ -12149,6 +12151,9 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt,
>               } else if (!script &&
>                          virXMLNodeNameEqual(cur, "script")) {
>                   script = virXMLPropString(cur, "path");
> +            } else if (!downscript &&
> +                       virXMLNodeNameEqual(cur, "downscript")) {
> +                downscript = virXMLPropString(cur, "path");
>               } else if (!domain_name &&
>                          virXMLNodeNameEqual(cur, "backenddomain")) {
>                   domain_name = virXMLPropString(cur, "name");
> @@ -12482,6 +12487,8 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt,
>   
>       if (script != NULL)
>           def->script = g_steal_pointer(&script);
> +    if (downscript != NULL)
> +        def->downscript = g_steal_pointer(&downscript);
>       if (domain_name != NULL)
>           def->domain_name = g_steal_pointer(&domain_name);
>       if (ifname != NULL)
> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
> index ddc75d8..e152c59 100644
> --- a/src/conf/domain_conf.h
> +++ b/src/conf/domain_conf.h
> @@ -1055,6 +1055,7 @@ struct _virDomainNetDef {
>           unsigned long sndbuf;
>       } tune;
>       char *script;
> +    char *downscript;
>       char *domain_name; /* backend domain name */
>       char *ifname; /* interface name on the host (<target dev='x'/>) */
>       int managed_tap; /* enum virTristateBool - ABSENT == YES */
> diff --git a/src/qemu/qemu_extdevice.c b/src/qemu/qemu_extdevice.c
> index 2096272..f8e440a 100644
> --- a/src/qemu/qemu_extdevice.c
> +++ b/src/qemu/qemu_extdevice.c
> @@ -234,6 +234,8 @@ qemuExtDevicesStop(virQEMUDriverPtr driver,
>   
>           if (slirp)
>               qemuSlirpStop(slirp, vm, driver, net);
> +        if (net->downscript)
> +            virNetDevRunEthernetScript(net->ifname, net->downscript);

This will run the script for all interface types, not only 'ethernet'. 
Also, it will run the script only when the domain is shut down 
(qemuExtDevicesStop() is called only from qemuProcessStop()), which is 
in contrast with the up script which is called on device hotplug. The 
downscript should be called on unplug too.

The code looks okayish, but in order to be merged it needs: RNG schema 
adjustment, some XML tests and documentation.

Michal