11:27 a.m.
https://bugzilla.redhat.com/show_bug.cgi?id=1624336
Details are in patch2, but essentially the issue is the check
for whether cold/hot plug of memory is supported occurs during
qemuDomainDefValidateMemoryHotplug; however, that is called
after virDomainDefCompatibleDevice, but is not called during
the qemuDomainAttachDeviceConfig processing.
Another solution would be to modify virDomainDefCompatibleDevice
to add a "if (def->mem.max_memory > 0 &&" check before the check
for whether the size fits and virDomainDefHasMemoryHotplug could
be called from qemuDomainAttachDeviceConfig, but that seems a bit
strange. The additional call is to avoid the equally strange message
that would appear "no free memory device slot available" because
nmems == mem.memory_slots == 0. If this solution is preferred I'm
fine with that, but figured I needed to start somewhere.
John Ferlan (2):
conf: Add the size of failed max_memory in error
conf: Add check/error for domain supports cold/hotplug
src/conf/domain_conf.c | 10 +++++++++-
1 file changed, 9 insertions(+), 1 deletion(-)
--
2.17.2
11:27 a.m.
New subject: [libvirt] [PATCH 1/2] conf: Add the size of failed max_memory in error
If virDomainDefCompatibleDevice fails because there is insufficient
domain def->mem.max_memory, then let's also print out that value in
the error message.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/conf/domain_conf.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index b70dca6c61..2a17214f38 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -28901,7 +28901,8 @@ virDomainDefCompatibleDevice(virDomainDefPtr def,
if ((virDomainDefGetMemoryTotal(def) + sz) > def->mem.max_memory) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_("Attaching memory device with size '%llu' would
"
- "exceed domain's maxMemory config"), sz);
+ "exceed domain's maxMemory config size
'%llu'"),
+ sz, def->mem.max_memory);
return -1;
}
}
--
2.17.2
11:27 a.m.
New subject: [libvirt] [PATCH 2/2] conf: Add check/error for domain supports cold/hotplug
https://bugzilla.redhat.com/show_bug.cgi?id=1624336
Add a check during virDomainDefCompatibleDevice whether the
domain supports cold/hotplug of a memory module even though
this duplicates the qemuDomainDefValidateMemoryHotplug check.
Without this check, the cold/hot plug would fail on the
subsequent mem_memory check (since it's 0). Adding a check
for max_memory > 0 would allow the subsequent hotplug check
to fail, but would cause coldplug to fail with the somewhat
opaque message "no free memory device slot available".
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/conf/domain_conf.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 2a17214f38..b83368a43a 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -28898,6 +28898,13 @@ virDomainDefCompatibleDevice(virDomainDefPtr def,
if (dev->type == VIR_DOMAIN_DEVICE_MEMORY) {
unsigned long long sz = dev->data.memory->size;
+ if (!virDomainDefHasMemoryHotplug(def)) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("cannot use/hotplug a memory device when domain "
+ "'maxMemory' is not defined"));
+ return -1;
+ }
+
if ((virDomainDefGetMemoryTotal(def) + sz) > def->mem.max_memory) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_("Attaching memory device with size '%llu' would
"
--
2.17.2
7:17 a.m.
On 12/7/18 6:27 PM, John Ferlan wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=1624336
Details are in patch2, but essentially the issue is the check
for whether cold/hot plug of memory is supported occurs during
qemuDomainDefValidateMemoryHotplug; however, that is called
after virDomainDefCompatibleDevice, but is not called during
the qemuDomainAttachDeviceConfig processing.
Another solution would be to modify virDomainDefCompatibleDevice
to add a "if (def->mem.max_memory > 0 &&" check before the check
for whether the size fits and virDomainDefHasMemoryHotplug could
be called from qemuDomainAttachDeviceConfig, but that seems a bit
strange. The additional call is to avoid the equally strange message
that would appear "no free memory device slot available" because
nmems == mem.memory_slots == 0. If this solution is preferred I'm
fine with that, but figured I needed to start somewhere.
John Ferlan (2):
conf: Add the size of failed max_memory in error
conf: Add check/error for domain supports cold/hotplug
src/conf/domain_conf.c | 10 +++++++++-
1 file changed, 9 insertions(+), 1 deletion(-)
ACK
Michal
2172
days inactive
2177
days old
3 comments
2 participants
participants (2)
-
John Ferlan -
Michal Privoznik