[PATCH] libxl: use LIBXL_API_VERSION 0x040200

[PATCH] libxl: use LIBXL_API_VERSION 0x040200

[Jim Fehlig]

To ensure the libvirt libxl driver will build with future versions
of Xen where the libxl API may change in incompatible ways,
explicitly use LIBXL_API_VERSION 0x040200. The libxl driver
does use new libxl APIs that have been added since Xen 4.2, but
currently it does not make use of any changes made to existing
APIs such as libxl_domain_create_restore or libxl_set_vcpuaffinity.
The version can be bumped if/when the libxl driver consumes the
changed APIs.

Further details can be found in the following discussion thread

https://www.redhat.com/archives/libvir-list/2016-April/msg00178.html
Signed-off-by: Jim Fehlig <jfehlig@suse.com>
---
 src/Makefile.am          |  1 +
 src/libxl/libxl_conf.h   | 12 ------------
 src/libxl/libxl_domain.c | 15 ---------------
 3 files changed, 1 insertion(+), 27 deletions(-)

diff --git a/src/Makefile.am b/src/Makefile.am
index 08ff301..259a474 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -1311,6 +1311,7 @@ endif ! WITH_DRIVER_MODULES
 
 libvirt_driver_libxl_impl_la_CFLAGS = 		\
 		$(LIBXL_CFLAGS)			\
+		-DLIBXL_API_VERSION=0x040200    \
 		-I$(srcdir)/access	\
 		-I$(srcdir)/conf	\
 		-I$(srcdir)/secret      \
diff --git a/src/libxl/libxl_conf.h b/src/libxl/libxl_conf.h
index 3c0eafb..24e2911 100644
--- a/src/libxl/libxl_conf.h
+++ b/src/libxl/libxl_conf.h
@@ -69,18 +69,6 @@
 # endif
 
 
-/* libxl interface for setting VCPU affinity changed in 4.5. In fact, a new
- * parameter has been added, representative of 'VCPU soft affinity'. If one
- * does not care about it (and that's libvirt case), passing NULL is the
- * right thing to do. To mark that change, LIBXL_HAVE_VCPUINFO_SOFT_AFFINITY
- * is defined. */
-# ifdef LIBXL_HAVE_VCPUINFO_SOFT_AFFINITY
-#  define libxl_set_vcpuaffinity(ctx, domid, vcpuid, map) \
-    libxl_set_vcpuaffinity((ctx), (domid), (vcpuid), (map), NULL)
-#  define libxl_set_vcpuaffinity_all(ctx, domid, max_vcpus, map) \
-    libxl_set_vcpuaffinity_all((ctx), (domid), (max_vcpus), (map), NULL)
-# endif
-
 typedef struct _libxlDriverPrivate libxlDriverPrivate;
 typedef libxlDriverPrivate *libxlDriverPrivatePtr;
 
diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c
index aed904b..192a506 100644
--- a/src/libxl/libxl_domain.c
+++ b/src/libxl/libxl_domain.c
@@ -981,9 +981,6 @@ libxlDomainStart(libxlDriverPrivatePtr driver, virDomainObjPtr vm,
     int managed_save_fd = -1;
     libxlDomainObjPrivatePtr priv = vm->privateData;
     libxlDriverConfigPtr cfg;
-#ifdef LIBXL_HAVE_DOMAIN_CREATE_RESTORE_PARAMS
-    libxl_domain_restore_params params;
-#endif
     virHostdevManagerPtr hostdev_mgr = driver->hostdevMgr;
     libxl_asyncprogress_how aop_console_how;
 
@@ -1070,20 +1067,8 @@ libxlDomainStart(libxlDriverPrivatePtr driver, virDomainObjPtr vm,
         ret = libxl_domain_create_new(cfg->ctx, &d_config,
                                       &domid, NULL, &aop_console_how);
     } else {
-#if defined(LIBXL_HAVE_DOMAIN_CREATE_RESTORE_SEND_BACK_FD)
-        params.checkpointed_stream = 0;
-        ret = libxl_domain_create_restore(cfg->ctx, &d_config, &domid,
-                                          restore_fd, -1, &params, NULL,
-                                          &aop_console_how);
-#elif defined(LIBXL_HAVE_DOMAIN_CREATE_RESTORE_PARAMS)
-        params.checkpointed_stream = 0;
-        ret = libxl_domain_create_restore(cfg->ctx, &d_config, &domid,
-                                          restore_fd, &params, NULL,
-                                          &aop_console_how);
-#else
         ret = libxl_domain_create_restore(cfg->ctx, &d_config, &domid,
                                           restore_fd, NULL, &aop_console_how);
-#endif
     }
     virObjectLock(vm);
 
-- 
2.1.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [PATCH] libxl: use LIBXL_API_VERSION 0x040200

[Dario Faggioli]

[-- Attachment #1.1: Type: text/plain, Size: 1083 bytes --]

On Wed, 2016-04-13 at 17:37 -0600, Jim Fehlig wrote:
> To ensure the libvirt libxl driver will build with future versions
> of Xen where the libxl API may change in incompatible ways,
> explicitly use LIBXL_API_VERSION 0x040200. The libxl driver
> does use new libxl APIs that have been added since Xen 4.2, but
> currently it does not make use of any changes made to existing
> APIs such as libxl_domain_create_restore or libxl_set_vcpuaffinity.
> The version can be bumped if/when the libxl driver consumes the
> changed APIs.
> 
> Further details can be found in the following discussion thread
> 
> https://www.redhat.com/archives/libvir-list/2016-April/msg00178.html
> Signed-off-by: Jim Fehlig <jfehlig@suse.com>
>
FWIW:

Reviewed-by: Dario Faggioli <dario.faggioli@citrix.com>

Regards,
Dario
-- 
<<This happens because I choose it to happen!>> (Raistlin Majere)
-----------------------------------------------------------------
Dario Faggioli, Ph.D, http://about.me/dario.faggioli
Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)


[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [libvirt] [PATCH] libxl: use LIBXL_API_VERSION 0x040200

[Martin Kletzander]

[-- Attachment #1.1: Type: text/plain, Size: 1500 bytes --]

On Wed, Apr 13, 2016 at 05:37:05PM -0600, Jim Fehlig wrote:
>To ensure the libvirt libxl driver will build with future versions
>of Xen where the libxl API may change in incompatible ways,
>explicitly use LIBXL_API_VERSION 0x040200. The libxl driver
>does use new libxl APIs that have been added since Xen 4.2, but
>currently it does not make use of any changes made to existing
>APIs such as libxl_domain_create_restore or libxl_set_vcpuaffinity.
>The version can be bumped if/when the libxl driver consumes the
>changed APIs.
>
>Further details can be found in the following discussion thread
>
>https://www.redhat.com/archives/libvir-list/2016-April/msg00178.html
>Signed-off-by: Jim Fehlig <jfehlig@suse.com>
>---
> src/Makefile.am          |  1 +
> src/libxl/libxl_conf.h   | 12 ------------
> src/libxl/libxl_domain.c | 15 ---------------
> 3 files changed, 1 insertion(+), 27 deletions(-)
>
>diff --git a/src/Makefile.am b/src/Makefile.am
>index 08ff301..259a474 100644
>--- a/src/Makefile.am
>+++ b/src/Makefile.am
>@@ -1311,6 +1311,7 @@ endif ! WITH_DRIVER_MODULES
>
> libvirt_driver_libxl_impl_la_CFLAGS = 		\
> 		$(LIBXL_CFLAGS)			\
>+		-DLIBXL_API_VERSION=0x040200    \

Adding it to LIBXL_CFLAGS in configure.ac would make it show after
configure, but that's something probably only I would find interesting
=)  I'm no libxl expert and I don't have it on my system, so I can't try
building it, hence only weak ACK from me.  If it build for you then it's
fine as nobody objects as I see.

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [PATCH] libxl: use LIBXL_API_VERSION 0x040200

[George Dunlap]

On Thu, Apr 14, 2016 at 12:37 AM, Jim Fehlig <jfehlig@suse.com> wrote:
> To ensure the libvirt libxl driver will build with future versions
> of Xen where the libxl API may change in incompatible ways,
> explicitly use LIBXL_API_VERSION 0x040200. The libxl driver
> does use new libxl APIs that have been added since Xen 4.2, but
> currently it does not make use of any changes made to existing
> APIs such as libxl_domain_create_restore or libxl_set_vcpuaffinity.
> The version can be bumped if/when the libxl driver consumes the
> changed APIs.
>
> Further details can be found in the following discussion thread
>
> https://www.redhat.com/archives/libvir-list/2016-April/msg00178.html
> Signed-off-by: Jim Fehlig <jfehlig@suse.com>
> ---
>  src/Makefile.am          |  1 +
>  src/libxl/libxl_conf.h   | 12 ------------
>  src/libxl/libxl_domain.c | 15 ---------------
>  3 files changed, 1 insertion(+), 27 deletions(-)
>
> diff --git a/src/Makefile.am b/src/Makefile.am
> index 08ff301..259a474 100644
> --- a/src/Makefile.am
> +++ b/src/Makefile.am
> @@ -1311,6 +1311,7 @@ endif ! WITH_DRIVER_MODULES
>
>  libvirt_driver_libxl_impl_la_CFLAGS =          \
>                 $(LIBXL_CFLAGS)                 \
> +               -DLIBXL_API_VERSION=0x040200    \
>                 -I$(srcdir)/access      \
>                 -I$(srcdir)/conf        \
>                 -I$(srcdir)/secret      \
> diff --git a/src/libxl/libxl_conf.h b/src/libxl/libxl_conf.h
> index 3c0eafb..24e2911 100644
> --- a/src/libxl/libxl_conf.h
> +++ b/src/libxl/libxl_conf.h
> @@ -69,18 +69,6 @@
>  # endif
>
>
> -/* libxl interface for setting VCPU affinity changed in 4.5. In fact, a new
> - * parameter has been added, representative of 'VCPU soft affinity'. If one
> - * does not care about it (and that's libvirt case), passing NULL is the
> - * right thing to do. To mark that change, LIBXL_HAVE_VCPUINFO_SOFT_AFFINITY
> - * is defined. */
> -# ifdef LIBXL_HAVE_VCPUINFO_SOFT_AFFINITY
> -#  define libxl_set_vcpuaffinity(ctx, domid, vcpuid, map) \
> -    libxl_set_vcpuaffinity((ctx), (domid), (vcpuid), (map), NULL)
> -#  define libxl_set_vcpuaffinity_all(ctx, domid, max_vcpus, map) \
> -    libxl_set_vcpuaffinity_all((ctx), (domid), (max_vcpus), (map), NULL)
> -# endif

Just checking with this -- the LIBXL_API_VERSION is meant for
*forward* compatibility, but I think the LIBXL_HAVE_* macros are for
*backwards* compatibility, isn't that right?  If you compile this
against an older version of Xen without LIBXL_HAVE_VCPU_SOFT_AFFINITY
(say, 4.4), will it break with this patch?

 -George

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [PATCH] libxl: use LIBXL_API_VERSION 0x040200

[Dario Faggioli]

[-- Attachment #1.1: Type: text/plain, Size: 2907 bytes --]

On Thu, 2016-04-14 at 09:33 +0100, George Dunlap wrote:
> On Thu, Apr 14, 2016 at 12:37 AM, Jim Fehlig <jfehlig@suse.com>
> wrote:
> > 
> > diff --git a/src/libxl/libxl_conf.h b/src/libxl/libxl_conf.h
> > index 3c0eafb..24e2911 100644
> > --- a/src/libxl/libxl_conf.h
> > +++ b/src/libxl/libxl_conf.h
> > @@ -69,18 +69,6 @@
> >  # endif
> > 
> > 
> > -/* libxl interface for setting VCPU affinity changed in 4.5. In
> > fact, a new
> > - * parameter has been added, representative of 'VCPU soft
> > affinity'. If one
> > - * does not care about it (and that's libvirt case), passing NULL
> > is the
> > - * right thing to do. To mark that change,
> > LIBXL_HAVE_VCPUINFO_SOFT_AFFINITY
> > - * is defined. */
> > -# ifdef LIBXL_HAVE_VCPUINFO_SOFT_AFFINITY
> > -#  define libxl_set_vcpuaffinity(ctx, domid, vcpuid, map) \
> > -    libxl_set_vcpuaffinity((ctx), (domid), (vcpuid), (map), NULL)
> > -#  define libxl_set_vcpuaffinity_all(ctx, domid, max_vcpus, map) \
> > -    libxl_set_vcpuaffinity_all((ctx), (domid), (max_vcpus), (map),
> > NULL)
> > -# endif
> Just checking with this -- the LIBXL_API_VERSION is meant for
> *forward* compatibility, but I think the LIBXL_HAVE_* macros are for
> *backwards* compatibility, isn't that right?  If you compile this
> against an older version of Xen without LIBXL_HAVE_VCPU_SOFT_AFFINITY
> (say, 4.4), will it break with this patch?
> 
Err... I'm not good at this things, but I don't think it will break.

So, libxl_set_vcpuaffinity(ctx,d,v,map,map) was defined in Xen 4.5. Xen
4.4 only has libxl_set_vcpuaffinity(ctx,d,v,map).

Libvirt only uses libxl_set_vcpuaffinity(ctx,d,v,map) (i.e., the
latter == the older one).

On anything below Xen 4.5 --which also means
LIBXL_HAVE_VCPU_SOFT_AFFINITY is not defined-- the variant with only
one map parameter is available, and therefore things should build and
work.

For anything above Xen 4.5 _and_ with LIBXL_API_VERSION<4.5 (which
would be the case, with this patch), libxl.h provides the stub for the
variant with only one map argument, so everything should again be fine.

Or am I missing something?

Perhaps, the point is that the hunk being removed above should really
have been using LIBXL_API_VERSION already, rather than
LIBXL_HAVE_VCPUINFO_SOFT_AFFINITY.

But, at the time, as far as I can recall, libvirt was not setting
LIBXL_API_VERSION to anything, and doing that was not considered
necessary. Therefore I did use LIBXL_HAVE_VCPUINFO_SOFT_AFFINITY for
the check, which is always present, starting from Xen 4.5 (or never
present, before that).

Regards,
Dario
-- 
<<This happens because I choose it to happen!>> (Raistlin Majere)
-----------------------------------------------------------------
Dario Faggioli, Ph.D, http://about.me/dario.faggioli
Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)


[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [PATCH] libxl: use LIBXL_API_VERSION 0x040200

[Jim Fehlig]

George Dunlap wrote:
> On Thu, Apr 14, 2016 at 12:37 AM, Jim Fehlig <jfehlig@suse.com> wrote:
>> To ensure the libvirt libxl driver will build with future versions
>> of Xen where the libxl API may change in incompatible ways,
>> explicitly use LIBXL_API_VERSION 0x040200. The libxl driver
>> does use new libxl APIs that have been added since Xen 4.2, but
>> currently it does not make use of any changes made to existing
>> APIs such as libxl_domain_create_restore or libxl_set_vcpuaffinity.
>> The version can be bumped if/when the libxl driver consumes the
>> changed APIs.
>>
>> Further details can be found in the following discussion thread
>>
>> https://www.redhat.com/archives/libvir-list/2016-April/msg00178.html
>> Signed-off-by: Jim Fehlig <jfehlig@suse.com>
>> ---
>>  src/Makefile.am          |  1 +
>>  src/libxl/libxl_conf.h   | 12 ------------
>>  src/libxl/libxl_domain.c | 15 ---------------
>>  3 files changed, 1 insertion(+), 27 deletions(-)
>>
>> diff --git a/src/Makefile.am b/src/Makefile.am
>> index 08ff301..259a474 100644
>> --- a/src/Makefile.am
>> +++ b/src/Makefile.am
>> @@ -1311,6 +1311,7 @@ endif ! WITH_DRIVER_MODULES
>>
>>  libvirt_driver_libxl_impl_la_CFLAGS =          \
>>                 $(LIBXL_CFLAGS)                 \
>> +               -DLIBXL_API_VERSION=0x040200    \
>>                 -I$(srcdir)/access      \
>>                 -I$(srcdir)/conf        \
>>                 -I$(srcdir)/secret      \
>> diff --git a/src/libxl/libxl_conf.h b/src/libxl/libxl_conf.h
>> index 3c0eafb..24e2911 100644
>> --- a/src/libxl/libxl_conf.h
>> +++ b/src/libxl/libxl_conf.h
>> @@ -69,18 +69,6 @@
>>  # endif
>>
>>
>> -/* libxl interface for setting VCPU affinity changed in 4.5. In fact, a new
>> - * parameter has been added, representative of 'VCPU soft affinity'. If one
>> - * does not care about it (and that's libvirt case), passing NULL is the
>> - * right thing to do. To mark that change, LIBXL_HAVE_VCPUINFO_SOFT_AFFINITY
>> - * is defined. */
>> -# ifdef LIBXL_HAVE_VCPUINFO_SOFT_AFFINITY
>> -#  define libxl_set_vcpuaffinity(ctx, domid, vcpuid, map) \
>> -    libxl_set_vcpuaffinity((ctx), (domid), (vcpuid), (map), NULL)
>> -#  define libxl_set_vcpuaffinity_all(ctx, domid, max_vcpus, map) \
>> -    libxl_set_vcpuaffinity_all((ctx), (domid), (max_vcpus), (map), NULL)
>> -# endif
> 
> Just checking with this -- the LIBXL_API_VERSION is meant for
> *forward* compatibility, but I think the LIBXL_HAVE_* macros are for
> *backwards* compatibility, isn't that right?  If you compile this
> against an older version of Xen without LIBXL_HAVE_VCPU_SOFT_AFFINITY
> (say, 4.4), will it break with this patch?

I've tested the patch (well, actually a variant that moves
'-DLIBXL_API_VERSION=0x040200' to configure.ac as suggested by Martin) by
building against 4.2, 4.3, 4.4, and 4.7/master. I think Dario already did a good
job explaining the removal of the above hunk.

Regards,
Jim

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [libvirt] [PATCH] libxl: use LIBXL_API_VERSION 0x040200

[Jim Fehlig]

Martin Kletzander wrote:
> On Wed, Apr 13, 2016 at 05:37:05PM -0600, Jim Fehlig wrote:
>> To ensure the libvirt libxl driver will build with future versions
>> of Xen where the libxl API may change in incompatible ways,
>> explicitly use LIBXL_API_VERSION 0x040200. The libxl driver
>> does use new libxl APIs that have been added since Xen 4.2, but
>> currently it does not make use of any changes made to existing
>> APIs such as libxl_domain_create_restore or libxl_set_vcpuaffinity.
>> The version can be bumped if/when the libxl driver consumes the
>> changed APIs.
>>
>> Further details can be found in the following discussion thread
>>
>> https://www.redhat.com/archives/libvir-list/2016-April/msg00178.html
>> Signed-off-by: Jim Fehlig <jfehlig@suse.com>
>> ---
>> src/Makefile.am          |  1 +
>> src/libxl/libxl_conf.h   | 12 ------------
>> src/libxl/libxl_domain.c | 15 ---------------
>> 3 files changed, 1 insertion(+), 27 deletions(-)
>>
>> diff --git a/src/Makefile.am b/src/Makefile.am
>> index 08ff301..259a474 100644
>> --- a/src/Makefile.am
>> +++ b/src/Makefile.am
>> @@ -1311,6 +1311,7 @@ endif ! WITH_DRIVER_MODULES
>>
>> libvirt_driver_libxl_impl_la_CFLAGS =         \
>>         $(LIBXL_CFLAGS)            \
>> +        -DLIBXL_API_VERSION=0x040200    \
> 
> Adding it to LIBXL_CFLAGS in configure.ac would make it show after
> configure, but that's something probably only I would find interesting
> =)

I think that is a good idea. I've sent a V2 that moves the define from
src/Makefile.am to configure.ac

https://www.redhat.com/archives/libvir-list/2016-April/msg00938.html

> I'm no libxl expert and I don't have it on my system, so I can't try
> building it, hence only weak ACK from me.  If it build for you then it's
> fine as nobody objects as I see.

I've built libvirt with the V2 patch against Xen 4.2, 4.3, 4.4, and 4.7/master.

Regards,
Jim

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel