[libvirt] [PATCH v2 3/3] virQEMUDriverDomainABIStability: Check for memoryBacking

Michal Privoznik posted 3 patches 7 years, 11 months ago
There is a newer version of this series
[libvirt] [PATCH v2 3/3] virQEMUDriverDomainABIStability: Check for memoryBacking
Posted by Michal Privoznik 7 years, 11 months ago
https://bugzilla.redhat.com/show_bug.cgi?id=1450349

Problem is, qemu fails to load guest memory image if these
attribute change on migration/restore from an image.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
---
 src/libvirt_private.syms |  6 ++++++
 src/qemu/qemu_conf.c     |  2 +-
 src/qemu/qemu_domain.c   | 40 ++++++++++++++++++++++++++++++++++++++++
 src/qemu/qemu_domain.h   |  1 +
 4 files changed, 48 insertions(+), 1 deletion(-)

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index d361454d5..a3ed2ad5b 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -404,11 +404,15 @@ virDomainLockFailureTypeFromString;
 virDomainLockFailureTypeToString;
 virDomainMemballoonModelTypeFromString;
 virDomainMemballoonModelTypeToString;
+virDomainMemoryAllocationTypeFromString;
+virDomainMemoryAllocationTypeToString;
 virDomainMemoryDefFree;
 virDomainMemoryFindByDef;
 virDomainMemoryFindInactiveByDef;
 virDomainMemoryInsert;
 virDomainMemoryRemove;
+virDomainMemorySourceTypeFromString;
+virDomainMemorySourceTypeToString;
 virDomainNetAppendIPAddress;
 virDomainNetDefClear;
 virDomainNetDefFormat;
@@ -690,6 +694,8 @@ virNodeDeviceEventUpdateNew;
 
 
 # conf/numa_conf.h
+virDomainMemoryAccessTypeFromString;
+virDomainMemoryAccessTypeToString;
 virDomainNumaCheckABIStability;
 virDomainNumaEquals;
 virDomainNumaFree;
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index 78f55c0e7..d8b88386d 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -910,7 +910,7 @@ virQEMUDriverCreateXMLConf(virQEMUDriverPtr driver)
     return virDomainXMLOptionNew(&virQEMUDriverDomainDefParserConfig,
                                  &virQEMUDriverPrivateDataCallbacks,
                                  &virQEMUDriverDomainXMLNamespace,
-                                 NULL);
+                                 &virQEMUDriverDomainABIStability);
 }
 
 
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 35fd79de8..c1ff8ca8a 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -5797,6 +5797,46 @@ qemuDomainUpdateMemoryDeviceInfo(virQEMUDriverPtr driver,
 }
 
 
+static bool
+qemuDomainABIStabilityCheck(const virDomainDef *src,
+                            const virDomainDef *dst)
+{
+    if (src->mem.source != dst->mem.source) {
+        virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+                       _("Target memoryBacking source '%s' doesn't "
+                         "match source memoryBacking source'%s'"),
+                       virDomainMemorySourceTypeToString(dst->mem.source),
+                       virDomainMemorySourceTypeToString(src->mem.source));
+        return false;
+    }
+
+    if (src->mem.access != dst->mem.access) {
+        virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+                       _("Target memoryBacking access '%s' doesn't "
+                         "match access memoryBacking access'%s'"),
+                       virDomainMemoryAccessTypeToString(dst->mem.access),
+                       virDomainMemoryAccessTypeToString(src->mem.access));
+        return false;
+    }
+
+    if (src->mem.allocation != dst->mem.allocation) {
+        virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+                       _("Target memoryBacking allocation '%s' doesn't "
+                         "match allocation memoryBacking allocation'%s'"),
+                       virDomainMemoryAllocationTypeToString(dst->mem.allocation),
+                       virDomainMemoryAllocationTypeToString(src->mem.allocation));
+        return false;
+    }
+
+    return true;
+}
+
+
+virDomainABIStability virQEMUDriverDomainABIStability = {
+    .domain = qemuDomainABIStabilityCheck,
+};
+
+
 bool
 qemuDomainDefCheckABIStability(virQEMUDriverPtr driver,
                                virDomainDefPtr src,
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index aebd91ad3..829f7746e 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -636,6 +636,7 @@ void qemuDomainCleanupRun(virQEMUDriverPtr driver,
 extern virDomainXMLPrivateDataCallbacks virQEMUDriverPrivateDataCallbacks;
 extern virDomainXMLNamespace virQEMUDriverDomainXMLNamespace;
 extern virDomainDefParserConfig virQEMUDriverDomainDefParserConfig;
+extern virDomainABIStability virQEMUDriverDomainABIStability;
 
 int qemuDomainUpdateDeviceList(virQEMUDriverPtr driver,
                                virDomainObjPtr vm, int asyncJob);
-- 
2.13.0

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 3/3] virQEMUDriverDomainABIStability: Check for memoryBacking
Posted by Daniel P. Berrange 7 years, 11 months ago
On Wed, May 24, 2017 at 04:45:57PM +0200, Michal Privoznik wrote:
> https://bugzilla.redhat.com/show_bug.cgi?id=1450349
> 
> Problem is, qemu fails to load guest memory image if these
> attribute change on migration/restore from an image.

[snip]

> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> index 35fd79de8..c1ff8ca8a 100644
> --- a/src/qemu/qemu_domain.c
> +++ b/src/qemu/qemu_domain.c
> @@ -5797,6 +5797,46 @@ qemuDomainUpdateMemoryDeviceInfo(virQEMUDriverPtr driver,
>  }
>  
>  
> +static bool
> +qemuDomainABIStabilityCheck(const virDomainDef *src,
> +                            const virDomainDef *dst)
> +{
> +    if (src->mem.source != dst->mem.source) {
> +        virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> +                       _("Target memoryBacking source '%s' doesn't "
> +                         "match source memoryBacking source'%s'"),
> +                       virDomainMemorySourceTypeToString(dst->mem.source),
> +                       virDomainMemorySourceTypeToString(src->mem.source));
> +        return false;
> +    }
> +
> +    if (src->mem.access != dst->mem.access) {
> +        virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> +                       _("Target memoryBacking access '%s' doesn't "
> +                         "match access memoryBacking access'%s'"),
> +                       virDomainMemoryAccessTypeToString(dst->mem.access),
> +                       virDomainMemoryAccessTypeToString(src->mem.access));
> +        return false;
> +    }
> +
> +    if (src->mem.allocation != dst->mem.allocation) {
> +        virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> +                       _("Target memoryBacking allocation '%s' doesn't "
> +                         "match allocation memoryBacking allocation'%s'"),
> +                       virDomainMemoryAllocationTypeToString(dst->mem.allocation),
> +                       virDomainMemoryAllocationTypeToString(src->mem.allocation));
> +        return false;
> +    }


Do we really need to blacklist all of these changes. I can understand that
changing the memory source would affect migration ABI, as it causes us to
use the memory backend command line config differently.

Assuming that matches though, I'm sceptical that changing 'access' or
'allocation' affects ABI.


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 3/3] virQEMUDriverDomainABIStability: Check for memoryBacking
Posted by Peter Krempa 7 years, 11 months ago
On Wed, May 24, 2017 at 16:06:56 +0100, Daniel Berrange wrote:
> On Wed, May 24, 2017 at 04:45:57PM +0200, Michal Privoznik wrote:
> > https://bugzilla.redhat.com/show_bug.cgi?id=1450349
> > 
> > Problem is, qemu fails to load guest memory image if these
> > attribute change on migration/restore from an image.
> 
> [snip]
> 
> > diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> > index 35fd79de8..c1ff8ca8a 100644
> > --- a/src/qemu/qemu_domain.c
> > +++ b/src/qemu/qemu_domain.c
> > @@ -5797,6 +5797,46 @@ qemuDomainUpdateMemoryDeviceInfo(virQEMUDriverPtr driver,
> >  }
> >  
> >  
> > +static bool
> > +qemuDomainABIStabilityCheck(const virDomainDef *src,
> > +                            const virDomainDef *dst)
> > +{
> > +    if (src->mem.source != dst->mem.source) {
> > +        virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> > +                       _("Target memoryBacking source '%s' doesn't "
> > +                         "match source memoryBacking source'%s'"),
> > +                       virDomainMemorySourceTypeToString(dst->mem.source),
> > +                       virDomainMemorySourceTypeToString(src->mem.source));
> > +        return false;
> > +    }
> > +
> > +    if (src->mem.access != dst->mem.access) {
> > +        virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> > +                       _("Target memoryBacking access '%s' doesn't "
> > +                         "match access memoryBacking access'%s'"),
> > +                       virDomainMemoryAccessTypeToString(dst->mem.access),
> > +                       virDomainMemoryAccessTypeToString(src->mem.access));
> > +        return false;
> > +    }
> > +
> > +    if (src->mem.allocation != dst->mem.allocation) {
> > +        virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> > +                       _("Target memoryBacking allocation '%s' doesn't "
> > +                         "match allocation memoryBacking allocation'%s'"),
> > +                       virDomainMemoryAllocationTypeToString(dst->mem.allocation),
> > +                       virDomainMemoryAllocationTypeToString(src->mem.allocation));
> > +        return false;
> > +    }
> 
> 
> Do we really need to blacklist all of these changes. I can understand that
> changing the memory source would affect migration ABI, as it causes us to
> use the memory backend command line config differently.
> 
> Assuming that matches though, I'm sceptical that changing 'access' or
> 'allocation' affects ABI.

If it does, a comment should state when it's required. (e.g. access may
need to be checked, since shared access may force usage of
memory-backend-file).

Allocation is indeed weird.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 3/3] virQEMUDriverDomainABIStability: Check for memoryBacking
Posted by Michal Privoznik 7 years, 11 months ago
On 05/24/2017 05:06 PM, Daniel P. Berrange wrote:
> On Wed, May 24, 2017 at 04:45:57PM +0200, Michal Privoznik wrote:
>> https://bugzilla.redhat.com/show_bug.cgi?id=1450349
>>
>> Problem is, qemu fails to load guest memory image if these
>> attribute change on migration/restore from an image.
> 
> [snip]
> 
>> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
>> index 35fd79de8..c1ff8ca8a 100644
>> --- a/src/qemu/qemu_domain.c
>> +++ b/src/qemu/qemu_domain.c
>> @@ -5797,6 +5797,46 @@ qemuDomainUpdateMemoryDeviceInfo(virQEMUDriverPtr driver,
>>  }
>>  
>>  
>> +static bool
>> +qemuDomainABIStabilityCheck(const virDomainDef *src,
>> +                            const virDomainDef *dst)
>> +{
>> +    if (src->mem.source != dst->mem.source) {
>> +        virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
>> +                       _("Target memoryBacking source '%s' doesn't "
>> +                         "match source memoryBacking source'%s'"),
>> +                       virDomainMemorySourceTypeToString(dst->mem.source),
>> +                       virDomainMemorySourceTypeToString(src->mem.source));
>> +        return false;
>> +    }
>> +
>> +    if (src->mem.access != dst->mem.access) {
>> +        virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
>> +                       _("Target memoryBacking access '%s' doesn't "
>> +                         "match access memoryBacking access'%s'"),
>> +                       virDomainMemoryAccessTypeToString(dst->mem.access),
>> +                       virDomainMemoryAccessTypeToString(src->mem.access));
>> +        return false;
>> +    }
>> +
>> +    if (src->mem.allocation != dst->mem.allocation) {
>> +        virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
>> +                       _("Target memoryBacking allocation '%s' doesn't "
>> +                         "match allocation memoryBacking allocation'%s'"),
>> +                       virDomainMemoryAllocationTypeToString(dst->mem.allocation),
>> +                       virDomainMemoryAllocationTypeToString(src->mem.allocation));
>> +        return false;
>> +    }
> 
> 
> Do we really need to blacklist all of these changes. I can understand that
> changing the memory source would affect migration ABI, as it causes us to
> use the memory backend command line config differently.
> 
> Assuming that matches though, I'm sceptical that changing 'access' or
> 'allocation' affects ABI.

Yep, you're right. Only memory source change causes trouble. Other twos
can be changed without any harm. v2 on its way.

Michal

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list