Re: [Xen-devel] [PATCH 1/7] libxl, Introduce dm-version xenstore key.

[Anthony PERARD <anthony.perard@xxxxxxxxxx>]

On Fri, Oct 7, 2011 at 13:28, Ian Campbell <Ian.Campbell@xxxxxxxxxx> wrote:
> On Fri, 2011-10-07 at 13:10 +0100, Anthony PERARD wrote:
>> The key is /local/domain/$domid/dm-version.
>
> I've been wondering if we should introduce /libxl/$domid/ as a place for
> keeping tooltack internal droppings like this. The danger with putting
> stuff in /local/domain is that domains come to rely on them.

I was not sure about where to put the value. So
/libxl/$domid/dm-version is good to me.

>> This come with libxl__device_model_version_running helper function.
>>
>> Signed-off-by: Anthony PERARD <anthony.perard@xxxxxxxxxx>
>> ---
>> Âtools/libxl/libxl_create.c  |  Â5 +++++
>> Âtools/libxl/libxl_internal.c | Â 19 +++++++++++++++++++
>> Âtools/libxl/libxl_internal.h | Â Â5 +++++
>> Â3 files changed, 29 insertions(+), 0 deletions(-)
>>
>> diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
>> index e1e3258..3f33d90 100644
>> --- a/tools/libxl/libxl_create.c
>> +++ b/tools/libxl/libxl_create.c
>> @@ -175,6 +175,11 @@ int libxl__domain_build(libxl__gc *gc,
>>
>> Â Â Âgettimeofday(&start_time, NULL);
>>
>> + Â Âlocalents = libxl__calloc(gc, 3, sizeof (char *));
>> + Â Âi = 0;
>> + Â Âlocalents[i++] = "dm-version";
>> + Â Âlocalents[i++] = libxl__strdup(gc, 
>> libxl_device_model_version_to_string(dm_info->device_model_version));
>> +
>
> You don't seem to use this anywhere?

This is magic ! :-)

This pointer was not used in this functions, but still given to
libxl__build_post() (and written in xenstore to /local/domain).
My other option from this function was to write the value in /vm/
which does not seams to be a good place.

>> Â Â Âswitch (info->type) {
>> Â Â Âcase LIBXL_DOMAIN_TYPE_HVM:
>> Â Â Â Â Âret = libxl__build_hvm(gc, domid, info, dm_info, state);
>> diff --git a/tools/libxl/libxl_internal.c b/tools/libxl/libxl_internal.c
>> index 3b8a41f..e535c0c 100644
>> --- a/tools/libxl/libxl_internal.c
>> +++ b/tools/libxl/libxl_internal.c
>> @@ -318,3 +318,22 @@ int libxl__fd_set_cloexec(int fd)
>> Â Â Â}
>> Â Â Âreturn fcntl(fd, F_SETFD, flags | FD_CLOEXEC);
>> Â}
>> +
>> +libxl_device_model_version libxl__device_model_version_running(libxl__gc 
>> *gc,
>> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â uint32_t 
>> domid)
>> +{
>> + Â Âchar *path = NULL;
>> + Â Âchar *dm_version = NULL;
>> + Â Âlibxl_device_model_version value;
>> +
>> + Â Âpath = libxl__sprintf(gc, "/local/domain/%d/dm-version", domid);
>> + Â Âdm_version = libxl__xs_read(gc, XBT_NULL, path);
>> + Â Âif (!dm_version) {
>
> This would be a bug, since it would imply an inconsistent version of
> libxl was used to create the domain? (not sure what our policy around
> this actually is / should be).

I just want to give libxl the ability to run after an update. But it's
maybe better to print and return an error, so that will give a chance
to the user to update is xenstore :). (or use the tool that created
the domain).

>> + Â Â Â Âreturn LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN_TRADITIONAL;
>> + Â Â}
>> +
>> + Â Âif (libxl_device_model_version_from_string(dm_version, &value) < 0) {
>> + Â Â Â Âreturn LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN_TRADITIONAL;
>> + Â Â}
>> + Â Âreturn value;
>> +}
>> diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
>> index 935c899..4dd0f91 100644
>> --- a/tools/libxl/libxl_internal.h
>> +++ b/tools/libxl/libxl_internal.h
>> @@ -554,4 +554,9 @@ _hidden void libxl__json_object_free(libxl__gc *gc, 
>> libxl__json_object *obj);
>>
>> Â_hidden libxl__json_object *libxl__json_parse(libxl__gc *gc, const char *s);
>>
>> + Â/* Based on /local/domain/$domid/dm-version xenstore key
>> + Â * default is qemu xen traditional */
>> +_hidden libxl_device_model_version
>> +libxl__device_model_version_running(libxl__gc *gc, uint32_t domid);
>> +
>> Â#endif


-- 
Anthony PERARD

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel