On Tue, May 31, 2011 at 11:52:20AM +0200, Matthias Bolte wrote:
> 2011/5/27 Daniel P. Berrange <berrange@xxxxxxxxxx>:
> > 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.
Thanks, I've pushed this series with those fixes
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list