1:50 p.m.
Note that this series applies only on top of the NUMA config unification series:
http://www.redhat.com/archives/libvir-list/2015-February/msg00532.html
Please see individual patches for explanation
Peter Krempa (3):
conf: ABI: Hugepage backing definition is not guest ABI
conf: ABI: Memballoon setting is not guest ABI
conf: numa: Check ABI stability of NUMA configuration
src/conf/domain_conf.c | 31 ++-----------------------------
src/conf/numa_conf.c | 37 +++++++++++++++++++++++++++++++++++++
src/conf/numa_conf.h | 3 +++
src/libvirt_private.syms | 1 +
4 files changed, 43 insertions(+), 29 deletions(-)
--
2.2.2
1:50 p.m.
New subject: [libvirt] [PATCH 1/3] conf: ABI: Hugepage backing definition is not guest ABI
The backing of the vm's memory isn't influencing the guest ABI thus
shouldn't be checked.
---
src/conf/domain_conf.c | 24 ------------------------
1 file changed, 24 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index fe73a5f..9a7060d 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -15969,30 +15969,6 @@ virDomainDefCheckABIStability(virDomainDefPtr src,
dst->mem.cur_balloon, src->mem.cur_balloon);
goto error;
}
- if (src->mem.nhugepages != dst->mem.nhugepages) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- _("Target domain huge pages count %zu does not match source
%zu"),
- dst->mem.nhugepages, src->mem.nhugepages);
- goto error;
- }
- for (i = 0; i < src->mem.nhugepages; i++) {
- virDomainHugePagePtr src_huge = &src->mem.hugepages[i];
- virDomainHugePagePtr dst_huge = &dst->mem.hugepages[i];
-
- if (src_huge->size != dst_huge->size) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- _("Target domain huge page size %llu "
- "does not match source %llu"),
- dst_huge->size, src_huge->size);
- goto error;
- }
-
- if (!virBitmapEqual(src_huge->nodemask, dst_huge->nodemask)) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("Target huge page nodemask does not match
source"));
- goto error;
- }
- }
if (src->vcpus != dst->vcpus) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
--
2.2.2
12:38 p.m.
New subject: [libvirt] [PATCH 1/3] conf: ABI: Hugepage backing definition is not guest ABI
On 02/16/2015 02:50 PM, Peter Krempa wrote:
The backing of the vm's memory isn't influencing the guest
ABI thus
shouldn't be checked.
---
src/conf/domain_conf.c | 24 ------------------------
1 file changed, 24 deletions(-)
Just to put it "officially there" - ACK...
some thoughts below FWIW
John
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index fe73a5f..9a7060d 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -15969,30 +15969,6 @@ virDomainDefCheckABIStability(virDomainDefPtr src,
dst->mem.cur_balloon, src->mem.cur_balloon);
goto error;
}
- if (src->mem.nhugepages != dst->mem.nhugepages) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- _("Target domain huge pages count %zu does not match source
%zu"),
- dst->mem.nhugepages, src->mem.nhugepages);
- goto error;
- }
Looks like this hunk was originally added by commit id 'f2fc1eee' when
it was one huge chunk... I'm not overly familiar with how hugepages
work, but agree that it doesn't seem to cause ABI issues.
- for (i = 0; i < src->mem.nhugepages; i++) {
- virDomainHugePagePtr src_huge = &src->mem.hugepages[i];
- virDomainHugePagePtr dst_huge = &dst->mem.hugepages[i];
-
- if (src_huge->size != dst_huge->size) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- _("Target domain huge page size %llu "
- "does not match source %llu"),
- dst_huge->size, src_huge->size);
- goto error;
- }
-
- if (!virBitmapEqual(src_huge->nodemask, dst_huge->nodemask)) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("Target huge page nodemask does not match
source"));
- goto error;
- }
- }
Looks like this hunk was added by commit id '136ad497' when the one
chunk was split with the extra mask capability. Again, I agree it
doesn't appear to cause ABI issues.
if (src->vcpus != dst->vcpus) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
1:50 p.m.
New subject: [libvirt] [PATCH RFC 2/3] conf: ABI: Memballoon setting is not guest ABI
The current balloon setting does not influence the ABI of the guest and
the driver should take the change just fine.
---
I personally don't think that this check is useful, but I'll happily drop the
patch if anybody thinks otherwise.
src/conf/domain_conf.c | 6 ------
1 file changed, 6 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 9a7060d..eee01b6 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -15963,12 +15963,6 @@ virDomainDefCheckABIStability(virDomainDefPtr src,
dst->mem.max_balloon, src->mem.max_balloon);
goto error;
}
- if (src->mem.cur_balloon != dst->mem.cur_balloon) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- _("Target domain current memory %lld does not match source
%lld"),
- dst->mem.cur_balloon, src->mem.cur_balloon);
- goto error;
- }
if (src->vcpus != dst->vcpus) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
--
2.2.2
1:54 p.m.
New subject: [libvirt] [PATCH RFC 2/3] conf: ABI: Memballoon setting is not guest ABI
On Mon, Feb 16, 2015 at 08:50:43PM +0100, Peter Krempa wrote:
The current balloon setting does not influence the ABI of the guest
and
the driver should take the change just fine.
---
I personally don't think that this check is useful, but I'll happily drop the
patch if anybody thinks otherwise.
src/conf/domain_conf.c | 6 ------
1 file changed, 6 deletions(-)
Yes, you can change the balloon target on the fly, so this isn't something
we need to keep as stable across migration. The question though is whether
libvirt/QEMU will correctly handle the changed balloon setting during
migration. ie does libvirt correctly issue a 'balloon' monitor command on
the target host if it has changed in the target XML ? If it does, then I
agree this check can go..
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 9a7060d..eee01b6 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -15963,12 +15963,6 @@ virDomainDefCheckABIStability(virDomainDefPtr src,
dst->mem.max_balloon, src->mem.max_balloon);
goto error;
}
- if (src->mem.cur_balloon != dst->mem.cur_balloon) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- _("Target domain current memory %lld does not match source
%lld"),
- dst->mem.cur_balloon, src->mem.cur_balloon);
- goto error;
- }
Regards,
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 :|
11:15 a.m.
New subject: [libvirt] [PATCH RFC 2/3] conf: ABI: Memballoon setting is not guest ABI
On Mon, Feb 16, 2015 at 19:54:23 +0000, Daniel Berrange wrote:
On Mon, Feb 16, 2015 at 08:50:43PM +0100, Peter Krempa wrote:
> The current balloon setting does not influence the ABI of the guest and
> the driver should take the change just fine.
> ---
> I personally don't think that this check is useful, but I'll happily drop
the
> patch if anybody thinks otherwise.
>
> src/conf/domain_conf.c | 6 ------
> 1 file changed, 6 deletions(-)
Yes, you can change the balloon target on the fly, so this isn't something
we need to keep as stable across migration. The question though is whether
libvirt/QEMU will correctly handle the changed balloon setting during
migration. ie does libvirt correctly issue a 'balloon' monitor command on
the target host if it has changed in the target XML ? If it does, then I
agree this check can go..
I think qemu should handle this just right as we pass it via the
monitor, but I didn't actually try it. I'll hold off this one until I
make sure that it actually is okay.
Peter
1:50 p.m.
New subject: [libvirt] [PATCH 3/3] conf: numa: Check ABI stability of NUMA configuration
Add helper to compare initial sizes of indivitual NUMA nodes and the map
of belonging vCPUs. Other configuration is not ABI.
---
src/conf/domain_conf.c | 3 +++
src/conf/numa_conf.c | 37 +++++++++++++++++++++++++++++++++++++
src/conf/numa_conf.h | 3 +++
src/libvirt_private.syms | 1 +
4 files changed, 44 insertions(+)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index eee01b6..15d4fe0 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -15964,6 +15964,9 @@ virDomainDefCheckABIStability(virDomainDefPtr src,
goto error;
}
+ if (!virDomainNumaCheckABIStability(src->numa, dst->numa))
+ goto error;
+
if (src->vcpus != dst->vcpus) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_("Target domain vCPU count %d does not match source
%d"),
diff --git a/src/conf/numa_conf.c b/src/conf/numa_conf.c
index 0a6f902..70044d5 100644
--- a/src/conf/numa_conf.c
+++ b/src/conf/numa_conf.c
@@ -814,6 +814,43 @@ virDomainNumaNew(void)
}
+bool
+virDomainNumaCheckABIStability(virDomainNumaPtr src,
+ virDomainNumaPtr tgt)
+{
+ size_t i;
+
+ if (virDomainNumaGetNodeCount(src) != virDomainNumaGetNodeCount(tgt)) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("Target NUMA node count '%zu' doesn't match
"
+ "source '%zu'"),
+ virDomainNumaGetNodeCount(tgt),
+ virDomainNumaGetNodeCount(src));
+ return false;
+ }
+
+ for (i = 0; i < virDomainNumaGetNodeCount(src); i++) {
+ if (src->mem_nodes[i].mem != tgt->mem_nodes[i].mem) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("Size of target NUMA node %zu (%llu) doesn't
"
+ "match source (%llu)"), i,
+ tgt->mem_nodes[i].mem, src->mem_nodes[i].mem);
+ return false;
+ }
+
+ if (!virBitmapEqual(src->mem_nodes[i].cpumask,
+ tgt->mem_nodes[i].cpumask)) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("Processor mask of target NUMA node %zu doesn't
"
+ "match source"), i);
+ return false;
+ }
+ }
+
+ return true;
+}
+
+
size_t
virDomainNumaGetNodeCount(virDomainNumaPtr numa)
{
diff --git a/src/conf/numa_conf.h b/src/conf/numa_conf.h
index 8b3c437..23265af 100644
--- a/src/conf/numa_conf.h
+++ b/src/conf/numa_conf.h
@@ -125,6 +125,9 @@ void virDomainNumaSetNodeMemorySize(virDomainNumaPtr numa,
bool virDomainNumaEquals(virDomainNumaPtr n1,
virDomainNumaPtr n2);
+bool virDomainNumaCheckABIStability(virDomainNumaPtr src,
+ virDomainNumaPtr tgt);
+
bool virDomainNumatuneHasPlacementAuto(virDomainNumaPtr numatune);
bool virDomainNumatuneHasPerNodeBinding(virDomainNumaPtr numatune);
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 8988b61..c02e089 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -627,6 +627,7 @@ virNodeDeviceObjUnlock;
# conf/numa_conf.h
+virDomainNumaCheckABIStability;
virDomainNumaEquals;
virDomainNumaFree;
virDomainNumaGetNodeCount;
--
2.2.2
6:37 p.m.
New subject: [libvirt] [PATCH 3/3] conf: numa: Check ABI stability of NUMA configuration
OK - I peeked after looking at 24/24 to see how difficult this series
would be (and before soaking my brain cells in liquid refreshment)...
The first two seemed OK and it doesn't appear there's anyone else
disagreeing...
For this one though...
On 02/16/2015 02:50 PM, Peter Krempa wrote:
Add helper to compare initial sizes of indivitual NUMA nodes and the
map
of belonging vCPUs. Other configuration is not ABI.
---
src/conf/domain_conf.c | 3 +++
src/conf/numa_conf.c | 37 +++++++++++++++++++++++++++++++++++++
src/conf/numa_conf.h | 3 +++
src/libvirt_private.syms | 1 +
4 files changed, 44 insertions(+)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index eee01b6..15d4fe0 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -15964,6 +15964,9 @@ virDomainDefCheckABIStability(virDomainDefPtr src,
goto error;
}
+ if (!virDomainNumaCheckABIStability(src->numa, dst->numa))
+ goto error;
+
if (src->vcpus != dst->vcpus) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_("Target domain vCPU count %d does not match source
%d"),
diff --git a/src/conf/numa_conf.c b/src/conf/numa_conf.c
index 0a6f902..70044d5 100644
--- a/src/conf/numa_conf.c
+++ b/src/conf/numa_conf.c
@@ -814,6 +814,43 @@ virDomainNumaNew(void)
}
+bool
+virDomainNumaCheckABIStability(virDomainNumaPtr src,
+ virDomainNumaPtr tgt)
+{
+ size_t i;
+
+ if (virDomainNumaGetNodeCount(src) != virDomainNumaGetNodeCount(tgt)) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("Target NUMA node count '%zu' doesn't match
"
+ "source '%zu'"),
+ virDomainNumaGetNodeCount(tgt),
+ virDomainNumaGetNodeCount(src));
+ return false;
+ }
+
+ for (i = 0; i < virDomainNumaGetNodeCount(src); i++) {
+ if (src->mem_nodes[i].mem != tgt->mem_nodes[i].mem) {
Use virDomainNumaGetNodeMemorySize() ?
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("Size of target NUMA node %zu (%llu) doesn't
"
+ "match source (%llu)"), i,
+ tgt->mem_nodes[i].mem, src->mem_nodes[i].mem);
+ return false;
+ }
+
+ if (!virBitmapEqual(src->mem_nodes[i].cpumask,
+ tgt->mem_nodes[i].cpumask)) {
Use virDomainNumaGetNodeCpumask to access each cpumask bitmap ?
This seems reasonable to me...
ACK with the accessor changes...
John
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("Processor mask of target NUMA node %zu doesn't
"
+ "match source"), i);
+ return false;
+ }
+ }
+
+ return true;
+}
+
+
size_t
virDomainNumaGetNodeCount(virDomainNumaPtr numa)
{
diff --git a/src/conf/numa_conf.h b/src/conf/numa_conf.h
index 8b3c437..23265af 100644
--- a/src/conf/numa_conf.h
+++ b/src/conf/numa_conf.h
@@ -125,6 +125,9 @@ void virDomainNumaSetNodeMemorySize(virDomainNumaPtr numa,
bool virDomainNumaEquals(virDomainNumaPtr n1,
virDomainNumaPtr n2);
+bool virDomainNumaCheckABIStability(virDomainNumaPtr src,
+ virDomainNumaPtr tgt);
+
bool virDomainNumatuneHasPlacementAuto(virDomainNumaPtr numatune);
bool virDomainNumatuneHasPerNodeBinding(virDomainNumaPtr numatune);
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 8988b61..c02e089 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -627,6 +627,7 @@ virNodeDeviceObjUnlock;
# conf/numa_conf.h
+virDomainNumaCheckABIStability;
virDomainNumaEquals;
virDomainNumaFree;
virDomainNumaGetNodeCount;
12:36 p.m.
New subject: [libvirt] [PATCH 3/3] conf: numa: Check ABI stability of NUMA configuration
On Thu, Feb 19, 2015 at 19:37:33 -0500, John Ferlan wrote:
OK - I peeked after looking at 24/24 to see how difficult this series
would be (and before soaking my brain cells in liquid refreshment)...
The first two seemed OK and it doesn't appear there's anyone else
disagreeing...
For this one though...
On 02/16/2015 02:50 PM, Peter Krempa wrote:
> Add helper to compare initial sizes of indivitual NUMA nodes and the map
> of belonging vCPUs. Other configuration is not ABI.
> ---
> src/conf/domain_conf.c | 3 +++
> src/conf/numa_conf.c | 37 +++++++++++++++++++++++++++++++++++++
> src/conf/numa_conf.h | 3 +++
> src/libvirt_private.syms | 1 +
> 4 files changed, 44 insertions(+)
>
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index eee01b6..15d4fe0 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -15964,6 +15964,9 @@ virDomainDefCheckABIStability(virDomainDefPtr src,
> goto error;
> }
>
> + if (!virDomainNumaCheckABIStability(src->numa, dst->numa))
> + goto error;
> +
> if (src->vcpus != dst->vcpus) {
> virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> _("Target domain vCPU count %d does not match source
%d"),
> diff --git a/src/conf/numa_conf.c b/src/conf/numa_conf.c
> index 0a6f902..70044d5 100644
> --- a/src/conf/numa_conf.c
> +++ b/src/conf/numa_conf.c
> @@ -814,6 +814,43 @@ virDomainNumaNew(void)
> }
>
>
> +bool
> +virDomainNumaCheckABIStability(virDomainNumaPtr src,
> + virDomainNumaPtr tgt)
> +{
> + size_t i;
> +
> + if (virDomainNumaGetNodeCount(src) != virDomainNumaGetNodeCount(tgt)) {
> + virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> + _("Target NUMA node count '%zu' doesn't
match "
> + "source '%zu'"),
> + virDomainNumaGetNodeCount(tgt),
> + virDomainNumaGetNodeCount(src));
> + return false;
> + }
> +
> + for (i = 0; i < virDomainNumaGetNodeCount(src); i++) {
> + if (src->mem_nodes[i].mem != tgt->mem_nodes[i].mem) {
Use virDomainNumaGetNodeMemorySize() ?
> + virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> + _("Size of target NUMA node %zu (%llu) doesn't
"
> + "match source (%llu)"), i,
> + tgt->mem_nodes[i].mem, src->mem_nodes[i].mem);
> + return false;
> + }
> +
> + if (!virBitmapEqual(src->mem_nodes[i].cpumask,
> + tgt->mem_nodes[i].cpumask)) {
Use virDomainNumaGetNodeCpumask to access each cpumask bitmap ?
This seems reasonable to me...
ACK with the accessor changes...
I've changed to the accessor-based approach and pushed this one along
with patch 1.
Thanks.
Peter
8:14 a.m.
On Mon, Feb 16, 2015 at 20:50:41 +0100, Peter Krempa wrote:
Note that this series applies only on top of the NUMA config
unification series:
http://www.redhat.com/archives/libvir-list/2015-February/msg00532.html
Please see individual patches for explanation
Peter Krempa (3):
conf: ABI: Hugepage backing definition is not guest ABI
conf: ABI: Memballoon setting is not guest ABI
conf: numa: Check ABI stability of NUMA configuration
src/conf/domain_conf.c | 31 ++-----------------------------
src/conf/numa_conf.c | 37 +++++++++++++++++++++++++++++++++++++
src/conf/numa_conf.h | 3 +++
src/libvirt_private.syms | 1 +
4 files changed, 43 insertions(+), 29 deletions(-)
Rebased version of this series including the depending series is
available at:
git fetch git://pipo.sk/pipo/libvirt.git/ memory-refactors
3597
days inactive
3601
days old
9 comments
3 participants
participants (3)
-
Daniel P. Berrange -
John Ferlan -
Peter Krempa