WARNING - OLD ARCHIVES

This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
   
 
 
Xen 
 
Home Products Support Community News
 
   
 

xen-devel

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

To: Ian Campbell <Ian.Campbell@xxxxxxxxxx>
Subject: Re: [Xen-devel] [PATCH 1/7] libxl, Introduce dm-version xenstore key.
From: Anthony PERARD <anthony.perard@xxxxxxxxxx>
Date: Fri, 7 Oct 2011 14:29:45 +0100
Cc: Xen Devel <xen-devel@xxxxxxxxxxxxxxxxxxx>, Stefano Stabellini <Stefano.Stabellini@xxxxxxxxxxxxx>
Delivery-date: Fri, 07 Oct 2011 06:31:08 -0700
Dkim-signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=mime-version:sender:in-reply-to:references:from:date :x-google-sender-auth:message-id:subject:to:cc:content-type :content-transfer-encoding; bh=dqvuZ/MnePZ9ghZ32+fqLkQPLUMwcs14xbxGcJmcAfo=; b=u39bHbMkokFU2P2qd3XPbyn3x2UfCaSy0C+oMD6LHKqNdi7EiCOupvcu+o/erytxgQ +vZqs6gDBDnOoQ0tYSWOX4rwrKpdu2JJhG37NqP4wc7Dqhb/w5Zn4HRqPg0e/5ZDQVXi +WKZDQhieLQRYbC5jsX8XP0zEa5W7P6uDKC6s=
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
In-reply-to: <1317990495.21903.331.camel@xxxxxxxxxxxxxxxxxxxxxx>
List-help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-id: Xen developer discussion <xen-devel.lists.xensource.com>
List-post: <mailto:xen-devel@lists.xensource.com>
List-subscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
References: <1317989418-25463-1-git-send-email-anthony.perard@xxxxxxxxxx> <1317989418-25463-2-git-send-email-anthony.perard@xxxxxxxxxx> <1317990495.21903.331.camel@xxxxxxxxxxxxxxxxxxxxxx>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
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