[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH v2] qemu: Adding 'downscript' feature for QEMU network interfaces.



2017-05-16 19:16 GMT-03:00 Laine Stump <laine laine org>:
> On 05/05/2017 10:22 AM, Julio Faracco wrote:
>> This commit adds the support for 'downscript' feature:
>> - For QEMU command line with the option:
>>   '-net downscript=/etc/qemu-ifdown,...'.
>>
>> - For Domains with a network interface description:
>>   '<interface type='ethernet'>
>>      ...
>>      <downscript path='/etc/qemu-ifdown'/>
>>      ...
>>   </interface>'
>>
>> The options 'script' and 'downscript' accept the argument 'no' to disable
>> the script executions. The way that the code was implemented, the XML file
>> accepts '<[down]script path='no'>' to solve this problem.
>>
>> This commit updates the tests and documentation too.
>>
>> Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=825939
>
> I have a couple issues with this implementation:
>
> 1) the script in an interface's <script> element is executed by libvirt
> itself, not by qemu (historically - it used to be executed by qemu, but
> this was changed a few years ago as part of work to make <interface
> type='ethernet'> usable without running qemu as root). Execution by
> libvirt vs by qemu  makes a *big* difference, since the qemu process is
> generally running unprivileged with all capabilities dropped (i.e. the
> script probably wouldn't have sufficient permission to do what a lot of
> people would want to do).
>
> 2) the <downscript> (and also the existing <script>) only get the name
> of the tap device as a parameter on their commandline, but it's likely
> that people would often want more information about the domain and its
> configuration in order to do some intelligent things in the script.
> Rather than propagating a limited model, maybe we should think about
> deprecating <script path='blah'/> (it would still work, but would get a
> better replacement) with something like
>

I created this patch based on a feature request because this is the same
problem we are facing here in my team. We are using hooks to enable
some network enhancements. But this is not working for some specific
cases. It is so generic for network. We need to do several workarounds.
So, the best approach for us, is using script/downscript.

>    <script up='Bob' down='Loblaw'/>

It could be a solution.
The changes introduced by this patch are less drastic.
Probably, if we go this way, we will need to change lines and lines of code.

>
> (this is just a point for discussion - don't go off an implement it that
> way just because I suggested it! Somebody else may disagree...). What
> I'm thinking is that these scripts could take arguments similar to the
> network hooks, including the full domain XML and interface XML on stdin.
>
> (the problem with network hooks and <interface type='ethernet'> is of
> course that network hooks aren't executed for type='ethernet'> (maybe
> that should change, although I like the idea of specifying the name of
> the script in the XML, rather than having a network hook that is tried
> for *every single interface in every domain* (and thus taints all
> domains on the host)).
>

This idea sounds good. (it is interesting). At least, it will solve our problem.
Because our scripts are associated with interfaces and network settings.
But obviously, this is my scenario and my case.

Thanks Laine.

>
>
>>
>> Signed-off-by: Julio Faracco <jcfaracco gmail com>
>> ---
>>  docs/formatdomain.html.in                                  |  1 +
>>  docs/schemas/domaincommon.rng                              |  8 ++++++++
>>  src/conf/domain_conf.c                                     | 13 +++++++++++++
>>  src/conf/domain_conf.h                                     |  1 +
>>  src/qemu/qemu_parse_command.c                              |  4 ++++
>>  tests/qemuargv2xmldata/qemuargv2xml-net-eth-ifname.args    |  2 +-
>>  tests/qemuargv2xmldata/qemuargv2xml-net-eth-ifname.xml     |  1 +
>>  tests/qemuargv2xmldata/qemuargv2xml-net-eth.args           |  2 +-
>>  tests/qemuargv2xmldata/qemuargv2xml-net-eth.xml            |  1 +
>>  tests/qemuxml2argvdata/qemuxml2argv-net-eth-ifname.xml     |  1 +
>>  tests/qemuxml2argvdata/qemuxml2argv-net-eth.xml            |  1 +
>>  tests/qemuxml2xmloutdata/qemuxml2xmlout-net-eth-ifname.xml |  1 +
>>  tests/qemuxml2xmloutdata/qemuxml2xmlout-net-eth.xml        |  1 +
>>  13 files changed, 35 insertions(+), 2 deletions(-)
>>
>> diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
>> index 8c884f4..89fe86d 100644
>> --- a/docs/formatdomain.html.in
>> +++ b/docs/formatdomain.html.in
>> @@ -4663,6 +4663,7 @@
>>    &lt;interface type='ethernet'&gt;
>>      &lt;target dev='vnet7'/&gt;
>>      &lt;script path='/etc/qemu-ifup-mynet'/&gt;
>> +    &lt;downscript path='/etc/qemu-ifdown-mynet'/&gt;
>>    &lt;/interface&gt;
>>  &lt;/devices&gt;
>>  ...</pre>
>> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
>> index 281309e..2f88dda 100644
>> --- a/docs/schemas/domaincommon.rng
>> +++ b/docs/schemas/domaincommon.rng
>> @@ -2609,6 +2609,14 @@
>>          </element>
>>        </optional>
>>        <optional>
>> +        <element name="downscript">
>> +          <attribute name="path">
>> +            <ref name="filePath"/>
>> +          </attribute>
>> +          <empty/>
>> +        </element>
>> +      </optional>
>> +      <optional>
>>          <element name="backenddomain">
>>            <attribute name="name">
>>              <ref name="domainName"/>
>> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
>> index 0ff216e..32d5720 100644
>> --- a/src/conf/domain_conf.c
>> +++ b/src/conf/domain_conf.c
>> @@ -1935,6 +1935,7 @@ virDomainNetDefClear(virDomainNetDefPtr def)
>>      VIR_FREE(def->backend.vhost);
>>      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);
>> @@ -9589,6 +9590,7 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt,
>>      char *ifname_guest = NULL;
>>      char *ifname_guest_actual = NULL;
>>      char *script = NULL;
>> +    char *downscript = NULL;
>>      char *address = NULL;
>>      char *port = NULL;
>>      char *localaddr = NULL;
>> @@ -9761,6 +9763,9 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt,
>>              } else if (!script &&
>>                         xmlStrEqual(cur->name, BAD_CAST "script")) {
>>                  script = virXMLPropString(cur, "path");
>> +            } else if (!downscript &&
>> +                       xmlStrEqual(cur->name, BAD_CAST "downscript")) {
>> +                downscript = virXMLPropString(cur, "path");
>>              } else if (!domain_name &&
>>                         xmlStrEqual(cur->name, BAD_CAST "backenddomain")) {
>>                  domain_name = virXMLPropString(cur, "name");
>> @@ -10074,6 +10079,10 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt,
>>          def->script = script;
>>          script = NULL;
>>      }
>> +    if (downscript != NULL) {
>> +        def->downscript = downscript;
>> +        downscript = NULL;
>> +    }
>>      if (domain_name != NULL) {
>>          def->domain_name = domain_name;
>>          domain_name = NULL;
>> @@ -10356,6 +10365,7 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt,
>>      VIR_FREE(dev);
>>      virDomainActualNetDefFree(actual);
>>      VIR_FREE(script);
>> +    VIR_FREE(downscript);
>>      VIR_FREE(bridge);
>>      VIR_FREE(model);
>>      VIR_FREE(backend);
>> @@ -22158,6 +22168,9 @@ virDomainNetDefFormat(virBufferPtr buf,
>>
>>      virBufferEscapeString(buf, "<script path='%s'/>\n",
>>                            def->script);
>> +    if (def->downscript)
>> +        virBufferEscapeString(buf, "<downscript path='%s'/>\n",
>> +                              def->downscript);
>>      virBufferEscapeString(buf, "<backenddomain name='%s'/>\n", def->domain_name);
>>
>>      if (def->ifname &&
>> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
>> index 09fb7aa..9deca76 100644
>> --- a/src/conf/domain_conf.h
>> +++ b/src/conf/domain_conf.h
>> @@ -1024,6 +1024,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'/>) */
>>      virNetDevIPInfo hostIP;
>> diff --git a/src/qemu/qemu_parse_command.c b/src/qemu/qemu_parse_command.c
>> index af9063c..d773917 100644
>> --- a/src/qemu/qemu_parse_command.c
>> +++ b/src/qemu/qemu_parse_command.c
>> @@ -1060,6 +1060,10 @@ qemuParseCommandLineNet(virDomainXMLOptionPtr xmlopt,
>>              def->script = values[i];
>>              values[i] = NULL;
>>          } else if (def->type == VIR_DOMAIN_NET_TYPE_ETHERNET &&
>> +                   STREQ(keywords[i], "downscript") && STRNEQ(values[i], "")) {
>> +            def->downscript = values[i];
>> +            values[i] = NULL;
>> +        } else if (def->type == VIR_DOMAIN_NET_TYPE_ETHERNET &&
>>                     STREQ(keywords[i], "ifname")) {
>>              def->ifname = values[i];
>>              values[i] = NULL;
>> diff --git a/tests/qemuargv2xmldata/qemuargv2xml-net-eth-ifname.args b/tests/qemuargv2xmldata/qemuargv2xml-net-eth-ifname.args
>> index 4d74ae4..5d7129c 100644
>> --- a/tests/qemuargv2xmldata/qemuargv2xml-net-eth-ifname.args
>> +++ b/tests/qemuargv2xmldata/qemuargv2xml-net-eth-ifname.args
>> @@ -18,6 +18,6 @@ QEMU_AUDIO_DRV=none \
>>  -usb \
>>  -drive file=/dev/HostVG/QEMUGuest1,format=raw,if=ide,bus=0,unit=0 \
>>  -net nic,macaddr=00:11:22:33:44:55,vlan=0,model=rtl8139,name=net0 \
>> --net tap,ifname=nic02,script=/etc/qemu-ifup,vlan=0,name=hostnet0 \
>> +-net tap,ifname=nic02,script=/etc/qemu-ifup,downscript=/etc/qemu-ifdown,vlan=0,name=hostnet0 \
>>  -serial none \
>>  -parallel none
>> diff --git a/tests/qemuargv2xmldata/qemuargv2xml-net-eth-ifname.xml b/tests/qemuargv2xmldata/qemuargv2xml-net-eth-ifname.xml
>> index fa9a892..8e04efb 100644
>> --- a/tests/qemuargv2xmldata/qemuargv2xml-net-eth-ifname.xml
>> +++ b/tests/qemuargv2xmldata/qemuargv2xml-net-eth-ifname.xml
>> @@ -30,6 +30,7 @@
>>      <interface type='ethernet'>
>>        <mac address='00:11:22:33:44:55'/>
>>        <script path='/etc/qemu-ifup'/>
>> +      <downscript path='/etc/qemu-ifdown'/>
>>        <target dev='nic02'/>
>>        <model type='rtl8139'/>
>>        <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/>
>> diff --git a/tests/qemuargv2xmldata/qemuargv2xml-net-eth.args b/tests/qemuargv2xmldata/qemuargv2xml-net-eth.args
>> index 89eb4c1..0e3fa21 100644
>> --- a/tests/qemuargv2xmldata/qemuargv2xml-net-eth.args
>> +++ b/tests/qemuargv2xmldata/qemuargv2xml-net-eth.args
>> @@ -18,6 +18,6 @@ QEMU_AUDIO_DRV=none \
>>  -usb \
>>  -drive file=/dev/HostVG/QEMUGuest1,format=raw,if=ide,bus=0,unit=0 \
>>  -net nic,macaddr=00:11:22:33:44:55,vlan=0,model=rtl8139,name=net0 \
>> --net tap,script=/etc/qemu-ifup,vlan=0,name=hostnet0 \
>> +-net tap,script=/etc/qemu-ifup,downscript=/etc/qemu-ifdown,vlan=0,name=hostnet0 \
>>  -serial none \
>>  -parallel none
>> diff --git a/tests/qemuargv2xmldata/qemuargv2xml-net-eth.xml b/tests/qemuargv2xmldata/qemuargv2xml-net-eth.xml
>> index 57c4be8..d177ca8 100644
>> --- a/tests/qemuargv2xmldata/qemuargv2xml-net-eth.xml
>> +++ b/tests/qemuargv2xmldata/qemuargv2xml-net-eth.xml
>> @@ -30,6 +30,7 @@
>>      <interface type='ethernet'>
>>        <mac address='00:11:22:33:44:55'/>
>>        <script path='/etc/qemu-ifup'/>
>> +      <downscript path='/etc/qemu-ifdown'/>
>>        <model type='rtl8139'/>
>>        <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/>
>>      </interface>
>> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-net-eth-ifname.xml b/tests/qemuxml2argvdata/qemuxml2argv-net-eth-ifname.xml
>> index dd0d752..21d8259 100644
>> --- a/tests/qemuxml2argvdata/qemuxml2argv-net-eth-ifname.xml
>> +++ b/tests/qemuxml2argvdata/qemuxml2argv-net-eth-ifname.xml
>> @@ -26,6 +26,7 @@
>>      <interface type='ethernet'>
>>        <mac address='00:11:22:33:44:55'/>
>>        <script path='/etc/qemu-ifup'/>
>> +      <downscript path='/etc/qemu-ifdown'/>
>>        <target dev='nic02'/>
>>        <model type='rtl8139'/>
>>      </interface>
>> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-net-eth.xml b/tests/qemuxml2argvdata/qemuxml2argv-net-eth.xml
>> index 48acadf..9f40122 100644
>> --- a/tests/qemuxml2argvdata/qemuxml2argv-net-eth.xml
>> +++ b/tests/qemuxml2argvdata/qemuxml2argv-net-eth.xml
>> @@ -26,6 +26,7 @@
>>      <interface type='ethernet'>
>>        <mac address='00:11:22:33:44:55'/>
>>        <script path='/etc/qemu-ifup'/>
>> +      <downscript path='/etc/qemu-ifdown'/>
>>        <model type='rtl8139'/>
>>      </interface>
>>      <input type='mouse' bus='ps2'/>
>> diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-net-eth-ifname.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-net-eth-ifname.xml
>> index c36baa0..b71fd5a 100644
>> --- a/tests/qemuxml2xmloutdata/qemuxml2xmlout-net-eth-ifname.xml
>> +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-net-eth-ifname.xml
>> @@ -30,6 +30,7 @@
>>      <interface type='ethernet'>
>>        <mac address='00:11:22:33:44:55'/>
>>        <script path='/etc/qemu-ifup'/>
>> +      <downscript path='/etc/qemu-ifdown'/>
>>        <target dev='nic02'/>
>>        <model type='rtl8139'/>
>>        <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/>
>> diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-net-eth.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-net-eth.xml
>> index 898bda6..c55cd68 100644
>> --- a/tests/qemuxml2xmloutdata/qemuxml2xmlout-net-eth.xml
>> +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-net-eth.xml
>> @@ -30,6 +30,7 @@
>>      <interface type='ethernet'>
>>        <mac address='00:11:22:33:44:55'/>
>>        <script path='/etc/qemu-ifup'/>
>> +      <downscript path='/etc/qemu-ifdown'/>
>>        <model type='rtl8139'/>
>>        <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/>
>>      </interface>
>>
>


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]