[libvirt] [PATCH 3/3] networkxml2conftest: Fix build on BSD

Martin Kletzander mkletzan at redhat.com
Fri Dec 30 16:06:40 UTC 2016


On Fri, Dec 30, 2016 at 04:00:21PM +0400, Roman Bogorodskiy wrote:
>  Martin Kletzander wrote:
>
>> Yet another way to fix the different loopback naming.  This variant
>> doesn't use multiple files, allows easy addition of more loopback
>> names and factoring out the naming code to a function (which would
>> programmatically get the loopback interface name) is very easy.  OTOH
>> we lose a way to regenerate the files (well, the *easy* way).
>>
>> Signed-off-by: Martin Kletzander <mkletzan at redhat.com>
>> ---
>>
>> Notes:
>>     The build is *NOT* tested on any non-Linux patch after this change.  I
>>     feel like it's easier for someone else to try it out than for me to
>>     spin up a new VM, install FreeBSD, figure out what's needed for
>>     building libvirt there and all that during the holidays.
>
>This sounds like a process with a very high entertainment value. :-)
>

I know, it's not like I haven't done that few times =D

>Unfortunately, I have a FreeBSD installation already, so... spoiler
>alert, test results are below.
>
>>     So please, if someone is bored out of their mind, feel free to test it
>>     out ;)
>>
>>  tests/networkxml2confdata/dhcp6-nat-network.conf   |  2 +-
>>  tests/networkxml2confdata/dhcp6-network.conf       |  2 +-
>>  .../dhcp6host-routed-network.conf                  |  2 +-
>>  tests/networkxml2confdata/isolated-network.conf    |  2 +-
>>  .../nat-network-dns-forward-plain.conf             |  2 +-
>>  .../nat-network-dns-forwarders.conf                |  2 +-
>>  .../networkxml2confdata/nat-network-dns-hosts.conf |  2 +-
>>  .../nat-network-dns-local-domain.conf              |  2 +-
>>  .../nat-network-dns-srv-record-minimal.conf        |  2 +-
>>  .../nat-network-dns-srv-record.conf                |  2 +-
>>  .../nat-network-dns-txt-record.conf                |  2 +-
>>  .../nat-network-name-with-quotes.conf              |  2 +-
>>  tests/networkxml2confdata/nat-network.conf         |  2 +-
>>  tests/networkxml2confdata/netboot-network.conf     |  2 +-
>>  .../networkxml2confdata/netboot-proxy-network.conf |  2 +-
>>  tests/networkxml2confdata/open-network.conf        |  2 +-
>>  tests/networkxml2confdata/ptr-domains-auto.conf    |  2 +-
>>  .../networkxml2confdata/routed-network-no-dns.conf |  2 +-
>>  tests/networkxml2confdata/routed-network.conf      |  2 +-
>>  tests/networkxml2conftest.c                        | 41 ++++++++++++++++++++--
>>  20 files changed, 58 insertions(+), 21 deletions(-)
>>
>> diff --git a/tests/networkxml2conftest.c b/tests/networkxml2conftest.c
>> index 7ff243b98d20..8ff2c0d33d34 100644
>> --- a/tests/networkxml2conftest.c
>> +++ b/tests/networkxml2conftest.c
>> @@ -19,15 +19,25 @@
>>  #define VIR_FROM_THIS VIR_FROM_NONE
>>
>>  static int
>> -testCompareXMLToConfFiles(const char *inxml, const char *outconf, dnsmasqCapsPtr caps)
>> +testCompareXMLToConfFiles(const char *inxml,
>> +                          const char *outconf,
>> +                          dnsmasqCapsPtr caps)
>>  {
>>      char *actual = NULL;
>> +    char *expected = NULL;
>>      int ret = -1;
>>      virNetworkDefPtr dev = NULL;
>>      virNetworkObjPtr obj = NULL;
>>      virCommandPtr cmd = NULL;
>>      char *pidfile = NULL;
>>      dnsmasqContext *dctx = NULL;
>> +    const char *loopback_name = "lo";
>> +    const char *loopback_placeholder = "@LOOPBACK_NAME@";
>> +    char *tmp = NULL;
>> +
>> +#ifndef __linux__
>> +    loname = "lo0";
>       ^^^^^^
>             should be 'loopback_name'.
>

🤦 so few rounds of refactoring will not be hidden under the covers.
OK, OK, I admit I didn't write it in one go =D

>After that I have networkxml2conftest runs smoothly for me.
>

Great!

>> +#endif
>>
>>      if (!(dev = virNetworkDefParseFile(inxml)))
>>          goto fail;
>> @@ -45,13 +55,40 @@ testCompareXMLToConfFiles(const char *inxml, const char *outconf, dnsmasqCapsPtr
>>                                     dctx, caps) < 0)
>>          goto fail;
>>
>> -    if (virTestCompareToFile(actual, outconf) < 0)
>> +    /* Regeneration option is sacrificed so that we can have one file for both
>> +     * Linux and non-Linux outputs */
>> +    if (virTestLoadFile(outconf, &expected) < 0)
>>          goto fail;
>>
>> +    tmp = strstr(expected, loopback_placeholder);
>> +    if (tmp) {
>> +        size_t placeholder_len = strlen(loopback_placeholder);
>> +        size_t loname_len = strlen(loopback_name);
>> +
>> +        if (loname_len > placeholder_len) {
>> +            fprintf(stderr, "%s", "Increase the loopback placeholder size");
>> +            goto fail;
>> +        }
>> +
>> +        if (!virStrncpy(tmp, loopback_name, strlen(loopback_name), strlen(tmp)))
>> +            goto fail;
>> +
>> +        memmove(tmp + loname_len, tmp + placeholder_len,
>> +                strlen(tmp + placeholder_len) + 1);
>> +    }
>> +
>> +    if (STRNEQ_NULLABLE(actual, expected)) {
>> +        virTestDifferenceFullNoRegenerate(stderr,
>> +                                          expected, outconf,
>> +                                          actual, NULL);
>> +        goto fail;
>> +    }
>> +
>>      ret = 0;
>>
>>   fail:
>>      VIR_FREE(actual);
>> +    VIR_FREE(expected);
>>      VIR_FREE(pidfile);
>>      virCommandFree(cmd);
>>      virObjectUnref(obj);
>
>Out of curiosity, can we use virStringReplace() here? I still have this

8-0 We have that?  I feel like I should've known... Or at least
should've tried looking for it.

>working with the following change (along with the loname fix):
>

Perfect.  ACK to your version if you're OK with it as well (the only
ACKs I've found in this mail are in the 'LOOPBACK' placeholder ;)), then
feel free to push it since you have the working commit handy.

Martin
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: Digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20161230/1e8b130a/attachment-0001.sig>


More information about the libvir-list mailing list