[libvirt] [PATCH 1/4] Add an API for comparing the ABI of two guest configurations

[Matthias Bolte]

2011/5/27 Daniel P. Berrange <berrange at redhat.com>:
> To allow a client app to pass in custom XML during migration
> of a guest it is neccessary to ensure the guest ABI remains
> unchanged. The virDomainDefCheckABIStablity method accepts
> two virDomainDefPtr structs and compares everything in them
> that could impact the guest machine ABI
>
> * src/conf/domain_conf.c, src/conf/domain_conf.h,
>  src/libvirt_private.syms: Add virDomainDefCheckABIStablity
> * src/conf/cpu_conf.c, src/conf/cpu_conf.h: Add virCPUDefIsEqual
> * src/util/sysinfo.c, src/util/sysinfo.h: Add virSysinfoIsEqual
> ---
>  src/conf/cpu_conf.c      |   91 +++++
>  src/conf/cpu_conf.h      |    9 +-
>  src/conf/domain_conf.c   |  881 +++++++++++++++++++++++++++++++++++++++++++++-
>  src/conf/domain_conf.h   |   11 +-
>  src/libvirt_private.syms |    1 +
>  src/util/sysinfo.c       |   60 +++-
>  src/util/sysinfo.h       |   11 +-
>  7 files changed, 1051 insertions(+), 13 deletions(-)
>
> diff --git a/src/conf/cpu_conf.c b/src/conf/cpu_conf.c
> index 98d598a..77d0976 100644
> --- a/src/conf/cpu_conf.c
> +++ b/src/conf/cpu_conf.c

> @@ -446,3 +449,91 @@ no_memory:
>     virReportOOMError();
>     return -1;
>  }
> +
> +bool
> +virCPUDefIsEqual(virCPUDefPtr src,
> +                 virCPUDefPtr dst)
> +{
> +    bool identical = false;
> +    int i;

> +    for (i = 0 ; i < src->nfeatures ; i++) {
> +        if (STRNEQ(src->features[i].name, dst->features[i].name)) {
> +            virCPUReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> +                              _("Target CPU feature %s does not match source %s"),
> +                              dst->features[i].name, src->features[i].name);
> +            goto cleanup;
> +        }

I think you need to compare features[i].policy here too.

> +    }
> +
> +    identical = true;
> +
> +cleanup:
> +    return identical;
> +}

> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 8ff155b..a9a4655 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c

> +static bool virDomainNetDefCheckABIStability(virDomainNetDefPtr src,
> +                                             virDomainNetDefPtr dst)
> +{
> +    bool identical = false;
> +
> +    if (memcmp(src->mac, dst->mac, VIR_MAC_BUFLEN) != 0) {
> +        virDomainReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> +                             _("Target network card mac %02x:%02x:%02x:%02x:%02x:%02x"
> +                               "does not match source %02x:%02x:%02x:%02x:%02x:%02x"),
> +                             dst->mac[0], dst->mac[2], dst->mac[2],

Shouldn't this be dst->mac[0], dst->mac[1], dst->mac[2]?

> +                             dst->mac[3], dst->mac[4], dst->mac[5],
> +                             src->mac[0], src->mac[2], src->mac[2],

Copy-n-paste error here.

> diff --git a/src/util/sysinfo.c b/src/util/sysinfo.c
> index d929073..70da532 100644
> --- a/src/util/sysinfo.c
> +++ b/src/util/sysinfo.c

> @@ -326,4 +328,56 @@ virSysinfoFormat(virSysinfoDefPtr def, const char *prefix)
>     return virBufferContentAndReset(&buf);
>  }
>
> +bool virSysinfoIsEqual(virSysinfoDefPtr src,
> +                       virSysinfoDefPtr dst)
> +{
> +    bool identical = false;
> +
> +    if (!src && !dst)
> +        return true;
> +
> +    if ((src && !dst) || (!src && dst)) {
> +        virSmbiosReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> +                             _("Target sysinfo does not match source"));
> +        goto cleanup;
> +    }
> +
> +    if (src->type != dst->type) {
> +        virSmbiosReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> +                             _("Target sysinfo %s does not match source %s"),
> +                             virSysinfoTypeToString(dst->type),
> +                             virSysinfoTypeToString(src->type));
> +        goto cleanup;
> +    }
> +
> +#define CHECK_FIELD(name, desc)                                         \

This needs to be '# define CHECK_FIELD(name, desc)' to please syntax-check.

> +    do {                                                                \
> +        if (STRNEQ_NULLABLE(src->name, dst->name)) {                    \
> +            virSmbiosReportError(VIR_ERR_CONFIG_UNSUPPORTED,            \
> +                                 _("Target sysinfo " desc " %s does not match source %s"), \

Are you sure that this insertion of dest into the string in this way
will work properly with gettext? I think

  _("Target sysinfo %s %s does not match source %s"), desc

will work better.

> +                                 src->name, dst->name);                 \

You used STRNEQ_NULLABLE as src->name, dst->name could be NULL, so you
need to use

  NULLSTR(src->name), NULLSTR(dst->name)

here too.

> +        }                                                               \
> +    } while (0)
> +
> +    CHECK_FIELD(bios_vendor, "BIOS vendor");
> +    CHECK_FIELD(bios_version, "BIOS version");
> +    CHECK_FIELD(bios_date, "BIOS date");
> +    CHECK_FIELD(bios_release, "BIOS release");
> +
> +    CHECK_FIELD(system_manufacturer, "system vendor");
> +    CHECK_FIELD(system_product, "system product");
> +    CHECK_FIELD(system_version, "system version");
> +    CHECK_FIELD(system_serial, "system serial");
> +    CHECK_FIELD(system_uuid, "system uuid");
> +    CHECK_FIELD(system_sku, "system sku");
> +    CHECK_FIELD(system_family, "system family");
> +
> +#undef CHECK_FIELD

s/#undef CHECK_FIELD/# undef CHECK_FIELD/

> +
> +    identical = true;
> +
> +cleanup:
> +    return identical;
> +}
> +
>  #endif /* !WIN32 */

ACK with those nits fixed.

Matthias