Re: [Xen-devel] [PATCH v8 21/21] libxl: add support for migrating HVM guests without a device model

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 06/11/15 16:05, Roger Pau Monne wrote:
> diff --git a/tools/libxl/libxl_stream_write.c 
> b/tools/libxl/libxl_stream_write.c
> index 52a60d7..0a6eaf9 100644
> --- a/tools/libxl/libxl_stream_write.c
> +++ b/tools/libxl/libxl_stream_write.c
> @@ -232,6 +232,9 @@ void libxl__stream_write_start(libxl__egc *egc,
>              stream->emu_sub_hdr.id = EMULATOR_QEMU_UPSTREAM;
>              break;
>  
> +        case LIBXL_DEVICE_MODEL_VERSION_NONE:
> +            break;
> +

This (in principle) leaves stream->emu_sub_hdr.id uninitialised
(although its value will be zero because of libxl__stream_write_init()).

I would be tempted to (ab)use EMULATOR_UNKNOWN here and have
setup_emulator_write() assert id != UNKNOWN, to catch calls which slip
through the cracks.

>          default:
>              rc = ERROR_FAIL;
>              LOG(ERROR, "Unknown emulator for HVM domain");
> @@ -359,6 +362,12 @@ static void write_emulator_xenstore_record(libxl__egc 
> *egc,
>      char *buf = NULL;
>      uint32_t len = 0;
>  
> +    if (libxl__device_model_version_running(gc, dss->domid) ==
> +        LIBXL_DEVICE_MODEL_VERSION_NONE) {
> +        emulator_xenstore_record_done(egc, stream);
> +        return;
> +    }
> +

libxl__device_model_version_running() performs a xenstore action, which
is a steep overhead for constant information.

It would be better to introduce a new field in libxl__stream_write_state
and cache libxl__device_model_version_running() once in
libxl__stream_write_start().  You can then also use that for an
assertion in setup_emulator_write()

~Andrew

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