10:06 a.m.
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(a)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