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

2011/5/27 Daniel P. Berrange <berrange(a)redhat.com&gt;: > 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.