Re: [PATCH v1 01/12] libxl: add API wrapper for libxl_domain_create_restore

[Jim Fehlig <jfehlig@xxxxxxxx>]

On 3/17/21 4:57 AM, Olaf Hering wrote:
> Upcoming changes will use different LIBXL_API_VERSION variants.
>
> Prepare libxl_domain_create_restore, which got a new parameter
> "send_back_fd" in Xen 4.7. libvirt does not use this parameter.
>
> No functional change intended.
>
> Signed-off-by: Olaf Hering <olaf@xxxxxxxxx>
> ---
>   src/libxl/libxl_api.h    | 44 ++++++++++++++++++++++++++++++++++++++++
>   src/libxl/libxl_domain.c |  5 +++--
>   2 files changed, 47 insertions(+), 2 deletions(-)
>   create mode 100644 src/libxl/libxl_api.h
>
> diff --git a/src/libxl/libxl_api.h b/src/libxl/libxl_api.h
> new file mode 100644
> index 0000000000..64ccd7a428
> --- /dev/null
> +++ b/src/libxl/libxl_api.h

What do you think of a more descriptive name for this file? E.g. 
libxl_api_quirks.h or libxl_api_wrap.h?

> @@ -0,0 +1,44 @@
> +/*
> + * libxl_api.h: handle various libxl API variants

Or maybe libxl_api_variants.h?

> + *
> + * Copyright (C) 2021 SUSE LLC
> + *
> + * This library is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * This library is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with this library.  If not, see
> + * <http://www.gnu.org/licenses/>.
> + */
> +
> +#pragma once
> +
> +#include <libxl.h>
> +
> +static inline int
> +Libxl_Domain_Create_Restore(libxl_ctx *ctx,

libvirt doesn't use this format for function names. See 'Function names' under 
naming conventions

https://libvirt.org/coding-style.html#naming-conventions

I struggled with naming when toying with this in the past. Maybe 
libxlDomainCreateRestoreWrap? The 'Wrap' suffix compliments the libxl_api_wrap.h 
name suggestion.

Regards,
Jim

> +                            libxl_domain_config *d_config,
> +                            uint32_t *domid,
> +                            int restore_fd,
> +                            const libxl_domain_restore_params *params,
> +                            const libxl_asyncprogress_how *aop_console_how)
> +{
> +    int ret;
> +
> +#if LIBXL_API_VERSION < 0x040700
> +    ret = libxl_domain_create_restore(ctx, d_config, domid, restore_fd, params,
> +                                      NULL, aop_console_how);
> +#else
> +    ret = libxl_domain_create_restore(ctx, d_config, domid, restore_fd, -1,
> +                                      params, NULL, aop_console_how);
> +#endif
> +
> +    return ret;
> +}
> diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c
> index 014f6aceca..2fe1d34e19 100644
> --- a/src/libxl/libxl_domain.c
> +++ b/src/libxl/libxl_domain.c
> @@ -22,6 +22,7 @@
>   
>   #include <fcntl.h>
>   
> +#include "libxl_api.h"
>   #include "libxl_domain.h"
>   #include "libxl_capabilities.h"
>   
> @@ -1396,8 +1397,8 @@ libxlDomainStart(libxlDriverPrivatePtr driver,
>   #ifdef LIBXL_HAVE_SRM_V2
>           params.stream_version = restore_ver;
>   #endif
> -        ret = libxl_domain_create_restore(cfg->ctx, &d_config, &domid,
> -                                          restore_fd, &params, NULL,
> +        ret = Libxl_Domain_Create_Restore(cfg->ctx, &d_config, &domid,
> +                                          restore_fd, &params,
>                                             &aop_console_how);
>           libxl_domain_restore_params_dispose(&params);
>       }