|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH v2 4/5] vTPM: add vTPM device for HVM virtual machine
On Tue, Dec 30, 2014 at 11:45:31PM -0500, Quan Xu wrote:
> Signed-off-by: Quan Xu <quan.xu@xxxxxxxxx>
> ---
> tools/libxl/libxl.c | 62
> ++++++++++++++++++++++++++++++++++++++++++++
> tools/libxl/libxl_create.c | 6 +++++
> tools/libxl/libxl_dm.c | 16 ++++++++++++
> tools/libxl/libxl_internal.h | 3 +++
> 4 files changed, 87 insertions(+)
>
> diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
> index 18561fb..656d4b0 100644
> --- a/tools/libxl/libxl.c
> +++ b/tools/libxl/libxl.c
> @@ -2015,6 +2015,10 @@ void libxl__device_vtpm_add(libxl__egc *egc, uint32_t
> domid,
> flexarray_append(front, "handle");
> flexarray_append(front, GCSPRINTF("%d", vtpm->devid));
>
> + /*for para virtual machine*/
Space after "/*" and before "*/", and capitalise "for".
Please also fix similar issues below.
> + flexarray_append(front, "domain-type");
> + flexarray_append(front, GCSPRINTF("%d", LIBXL_DOMAIN_TYPE_PV));
> +
What is this used for?
> if (aodev->update_json) {
> lock = libxl__lock_domain_userdata(gc, domid);
> if (!lock) {
> @@ -2073,6 +2077,64 @@ out:
> return;
> }
>
> +void libxl__device_hvm_vtpm_add(libxl__gc *gc, uint32_t domid,
> + libxl_device_vtpm *vtpm)
> +{
> + flexarray_t *front;
> + flexarray_t *back;
> + libxl__device *device;
> + unsigned int rc;
> +
> + rc = libxl__device_vtpm_setdefault(gc, vtpm);
> + if (rc) goto out;
> +
> + front = flexarray_make(gc, 16, 1);
> + back = flexarray_make(gc, 16, 1);
> +
> + if (vtpm->devid == -1) {
> + if ((vtpm->devid = libxl__device_nextid(gc, domid, "vtpm")) < 0) {
> + rc = ERROR_FAIL;
> + goto out;
> + }
> + }
> +
> + GCNEW(device);
> + rc = libxl__device_from_vtpm(gc, domid, vtpm, device);
> + if ( rc != 0 ) goto out;
Coding style.
> + flexarray_append(back, "frontend-id");
> + flexarray_append(back, GCSPRINTF("%d", domid));
> + flexarray_append(back, "online");
> + flexarray_append(back, "1");
> + flexarray_append(back, "state");
> + flexarray_append(back, GCSPRINTF("%d", 1));
No need to use GCSPRINT.
> + flexarray_append(back, "handle");
> + flexarray_append(back, GCSPRINTF("%d", vtpm->devid));
> +
> + flexarray_append(back, "uuid");
> + flexarray_append(back, GCSPRINTF(LIBXL_UUID_FMT,
> LIBXL_UUID_BYTES(vtpm->uuid)));
Line too long.
> + flexarray_append(back, "resume");
> + flexarray_append(back, "False");
> +
> + flexarray_append(front, "backend-id");
> + flexarray_append(front, GCSPRINTF("%d", vtpm->backend_domid));
> + flexarray_append(front, "state");
> + flexarray_append(front, GCSPRINTF("%d", 1));
> + flexarray_append(front, "handle");
> + flexarray_append(front, GCSPRINTF("%d", vtpm->devid));
> +
> + flexarray_append(front, "domain-type");
> + flexarray_append(front, GCSPRINTF("%d", LIBXL_DOMAIN_TYPE_HVM));
> +
> + libxl__device_generic_add(gc, XBT_NULL, device,
> + libxl__xs_kvs_of_flexarray(gc, back,
> back->count),
> + libxl__xs_kvs_of_flexarray(gc, front,
> front->count),
> + NULL);
> +
> + rc = 0;
> +out:
> + return;
> +}
> +
There is one major issue with your implementation. You didn't handle
vtpm hot-plug and hot-unplug.
I think what you should do is to refactor libxl__device_vtpm_add to call
the right helpers (say libxl__device_vtpm_add_{pv,hvm}).
With this approach you don't need to worry about all the internal logic
of handling JSON template and locking etc. You might also be able to
nuke b_info->num_vtpms you added in previous patches.
Does this make sense?
> libxl_device_vtpm *libxl_device_vtpm_list(libxl_ctx *ctx, uint32_t domid,
> int *num)
> {
> GC_INIT(ctx);
> diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
> index c6f68fe..b2f61cb 100644
> --- a/tools/libxl/libxl_create.c
> +++ b/tools/libxl/libxl_create.c
> @@ -901,6 +901,12 @@ static void initiate_domain_create(libxl__egc *egc,
> d_config->nics[i].devid = ++last_devid;
> }
>
> + if (d_config->c_info.type == LIBXL_DOMAIN_TYPE_HVM &&
> + d_config->num_vtpms > 0) {
> + ret = libxl__device_vtpm_setdefault(gc, d_config->vtpms);
> + if (ret) goto error_out;
> + }
> +
> if (restore_fd >= 0) {
> LOG(DEBUG, "restoring, not running bootloader");
> domcreate_bootloader_done(egc, &dcs->bl, 0);
> diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
> index 3e191c3..337ac64 100644
> --- a/tools/libxl/libxl_dm.c
> +++ b/tools/libxl/libxl_dm.c
> @@ -414,6 +414,7 @@ static char **
> libxl__build_device_model_args_new(libxl__gc *gc,
> const libxl_device_nic *nics = guest_config->nics;
> const int num_disks = guest_config->num_disks;
> const int num_nics = guest_config->num_nics;
> + const int num_vtpms = guest_config->num_vtpms;
> const libxl_vnc_info *vnc = libxl__dm_vnc(guest_config);
> const libxl_sdl_info *sdl = dm_sdl(guest_config);
> const char *keymap = dm_keymap(guest_config);
> @@ -747,6 +748,15 @@ static char **
> libxl__build_device_model_args_new(libxl__gc *gc,
> abort();
> }
>
> + /*add vTPM parameters for HVM virtual machine*/
> + if (b_info->type == LIBXL_DOMAIN_TYPE_HVM &&
> + num_vtpms >0) {
"> 0".
> + flexarray_vappend(dm_args, "-tpmdev",
> + "xenstubdoms,id=xenvtpm0", NULL);
Hardcoded id? I think we need to be more flexible on this.
Wei.
> + flexarray_vappend(dm_args,"-device",
> + "tpm-tis,tpmdev=xenvtpm0", NULL);
> + }
> +
> ram_size = libxl__sizekb_to_mb(b_info->max_memkb - b_info->video_memkb);
> flexarray_append(dm_args, "-m");
> flexarray_append(dm_args, libxl__sprintf(gc, "%"PRId64, ram_size));
> @@ -1412,6 +1422,12 @@ retry_transaction:
> spawn->failure_cb = device_model_startup_failed;
> spawn->detached_cb = device_model_detached;
>
> + /* Plug vtpm devices*/
> + if (b_info->type == LIBXL_DOMAIN_TYPE_HVM &&
> + guest_config->num_vtpms > 0){
> + libxl__device_hvm_vtpm_add(gc, domid, guest_config->vtpms);
> + }
> +
> rc = libxl__spawn_spawn(egc, spawn);
> if (rc < 0)
> goto out_close;
> diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
> index 4361421..946b8cf 100644
> --- a/tools/libxl/libxl_internal.h
> +++ b/tools/libxl/libxl_internal.h
> @@ -2388,6 +2388,9 @@ _hidden void libxl__device_vtpm_add(libxl__egc *egc,
> uint32_t domid,
> libxl_device_vtpm *vtpm,
> libxl__ao_device *aodev);
>
> +void libxl__device_hvm_vtpm_add(libxl__gc *gc, uint32_t domid,
> + libxl_device_vtpm *vtpm);
> +
> /* Internal function to connect a vkb device */
> _hidden int libxl__device_vkb_add(libxl__gc *gc, uint32_t domid,
> libxl_device_vkb *vkb);
> --
> 1.8.3.2
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |