[libvirt PATCH 5/9] tests: Add simple test for virDomainMigrateCheckNotLocal

[Jiri Denemark]

On Tue, Aug 25, 2020 at 07:47:11 +0200, Martin Kletzander wrote:
> For this we need to make the function accessible (at least privately).  The
> behaviour will change in following patches and the test helps explaining the
> change.
> 
> Signed-off-by: Martin Kletzander <mkletzan at redhat.com>
> ---
>  scripts/apibuild.py      |  1 +
>  src/libvirt-domain.c     |  4 +-
>  src/libvirt_internal.h   |  2 +
>  src/libvirt_private.syms |  1 +
>  tests/meson.build        |  1 +
>  tests/virmigtest.c       | 90 ++++++++++++++++++++++++++++++++++++++++
>  6 files changed, 96 insertions(+), 3 deletions(-)
>  create mode 100644 tests/virmigtest.c
> 
> diff --git a/scripts/apibuild.py b/scripts/apibuild.py
> index 58ae76d29cfc..b94c0f6c09dd 100755
> --- a/scripts/apibuild.py
> +++ b/scripts/apibuild.py
> @@ -81,6 +81,7 @@ ignored_words = {
>  
>  ignored_functions = {
>      "virConnectSupportsFeature": "private function for remote access",
> +    "virDomainMigrateCheckNotLocal": "private function for migration",
>      "virDomainMigrateFinish": "private function for migration",
>      "virDomainMigrateFinish2": "private function for migration",
>      "virDomainMigratePerform": "private function for migration",
> diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
> index ad60a92da879..4d958ca5219d 100644
> --- a/src/libvirt-domain.c
> +++ b/src/libvirt-domain.c
> @@ -3269,8 +3269,7 @@ virDomainMigrateVersion3Params(virDomainPtr domain,
>                                          params, nparams, true, flags);
>  }
>  
> -
> -static int
> +int
>  virDomainMigrateCheckNotLocal(const char *dconnuri)
>  {
>      g_autoptr(virURI) tempuri = NULL;
> @@ -3286,7 +3285,6 @@ virDomainMigrateCheckNotLocal(const char *dconnuri)
>      return 0;
>  }
>  
> -
>  static int
>  virDomainMigrateUnmanagedProto2(virDomainPtr domain,
>                                  const char *dconnuri,

I believe the two empty lines around virDomainMigrateCheckNotLocal
should not be reduced to a single line. We tend to separate functions
with two empty lines (except for some cases where nobody noticed) :-)

...
> diff --git a/tests/virmigtest.c b/tests/virmigtest.c
> new file mode 100644
> index 000000000000..9539aadb5157
> --- /dev/null
> +++ b/tests/virmigtest.c
> @@ -0,0 +1,90 @@
...
> +#define VIR_FROM_THIS VIR_FROM_RPC
> +
> +VIR_LOG_INIT("tests.migtest");
> +
> +struct MigLocalData {

Eh, we never start a type with upper case.

> +    const char *uri;
> +    bool fail;
> +};
> +
> +extern int virDomainMigrateCheckNotLocal(const char *dconnuri);

Hmm, why is this needed? Looks like a leftover.

> +
> +static int testMigNotLocal(const void *args)
             ^
             |_________
                       |
Break the line here ---'

> +{
> +    int ret = -1;
> +    const struct MigLocalData *data = args;
...

With the nits addressed:

Reviewed-by: Jiri Denemark <jdenemar at redhat.com>