Re: [libvirt] [PATCHv3 10/12] qemu: conf: Add support for memory device cold(un)plug
On Fri, Mar 20, 2015 at 07:40:21 -0400, John Ferlan wrote:
I have tested your series with our qemu memory hot remove patch series,
here would be a possible error.
When hotplug a memory device, its size has been aligned. So the
compare for
size here would fail possiblely.
hmm.. Not sure that's necessary - although Peter can make the final
determination... Commit id '57b215a' doesn't modify each def-mems[i]
entry in qemuDomainAlignMemorySizes, rather it gets a value from
virDomainDefSetMemoryInitial and then does the rounding.
If the stored def-mems[i]-size value is/was modified, then I'd agree,
but it doesn't appear to be that way.
If there is a rounding of the value - then please just point it out
Yes, the stored def-mems[i]-size value was modified.
If you assign the size 524287 KiB, the stored value will be 524288.
Thanks,
Zhu
Ah - found it - patch 9 has:
+/* Align memory module sizes */
+for (i = 0; i def-nmems; i++)
+qemuDomainMemoryDeviceAlignSize(def-mems[i]);
+
Which I missed on my first foray through this. Once I cscope'd on
VIR_ROUND_UP() instead of -size, it became apparent
So yes, it seems the to be compared size needs a likewise adjustment.
Indeed, but the size needs to be aligned only for the active definition
as we only align that one, thus it belongs to patch 12/12.
I'll be adding the following diff to 12/12:
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 40041d5..9b8d11b 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -4189,6 +4189,8 @@ qemuDomainDetachMemoryDevice(virQEMUDriverPtr driver,
return -1;
}
+qemuDomainMemoryDeviceAlignSize(memdef);
+
if ((idx = virDomainMemoryFindByDef(vm-def, memdef)) 0) {
virReportError(VIR_ERR_OPERATION_INVALID, %s,
_(device not present in domain configuration));
Peter
signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv3 10/12] qemu: conf: Add support for memory device cold(un)plug
I have tested your series with our qemu memory hot remove patch series, here would be a possible error. When hotplug a memory device, its size has been aligned. So the compare for size here would fail possiblely. hmm.. Not sure that's necessary - although Peter can make the final determination... Commit id '57b215a' doesn't modify each def-mems[i] entry in qemuDomainAlignMemorySizes, rather it gets a value from virDomainDefSetMemoryInitial and then does the rounding. If the stored def-mems[i]-size value is/was modified, then I'd agree, but it doesn't appear to be that way. If there is a rounding of the value - then please just point it out Yes, the stored def-mems[i]-size value was modified. If you assign the size 524287 KiB, the stored value will be 524288. Thanks, Zhu Ah - found it - patch 9 has: +/* Align memory module sizes */ +for (i = 0; i def-nmems; i++) +qemuDomainMemoryDeviceAlignSize(def-mems[i]); + Which I missed on my first foray through this. Once I cscope'd on VIR_ROUND_UP() instead of -size, it became apparent So yes, it seems the to be compared size needs a likewise adjustment. John -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv3 10/12] qemu: conf: Add support for memory device cold(un)plug
On 03/18/2015 01:51 AM, Zhu Guihua wrote:
On 03/17/2015 10:19 PM, Peter Krempa wrote:
Add a few helpers that allow to operate with memory device definitions
on the domain config and use them to implement memory device coldplug in
the qemu driver.
---
Notes:
Version 2:
- no changes
src/conf/domain_conf.c | 100
+++
src/conf/domain_conf.h | 10 +
src/libvirt_private.syms | 4 ++
src/qemu/qemu_driver.c | 15 ++-
4 files changed, 127 insertions(+), 2 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 8c2234f..1a02e46 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -12811,6 +12811,106 @@ virDomainRNGRemove(virDomainDefPtr def,
}
+static int
+virDomainMemoryFindByDefInternal(virDomainDefPtr def,
+ virDomainMemoryDefPtr mem,
+ bool allowAddressFallback)
+{
+size_t i;
+
+for (i = 0; i def-nmems; i++) {
+virDomainMemoryDefPtr tmp = def-mems[i];
+
+/* address, if present */
+if (allowAddressFallback) {
+if (tmp-info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE)
+continue;
+} else {
+if (mem-info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE
+!virDomainDeviceInfoAddressIsEqual(tmp-info,
mem-info))
+continue;
+}
+
+/* alias, if present */
+if (mem-info.alias
+STRNEQ_NULLABLE(tmp-info.alias, mem-info.alias))
+continue;
+
+/* target info - always present */
+if (tmp-model != mem-model ||
+tmp-targetNode != mem-targetNode ||
+tmp-size != mem-size)
I have tested your series with our qemu memory hot remove patch series,
here would be a possible error.
When hotplug a memory device, its size has been aligned. So the compare for
size here would fail possiblely.
hmm.. Not sure that's necessary - although Peter can make the final
determination... Commit id '57b215a' doesn't modify each def-mems[i]
entry in qemuDomainAlignMemorySizes, rather it gets a value from
virDomainDefSetMemoryInitial and then does the rounding.
If the stored def-mems[i]-size value is/was modified, then I'd agree,
but it doesn't appear to be that way.
If there is a rounding of the value - then please just point it out
John
Thanks,
Zhu
+continue;
+
+/* source stuff - match with device */
+if (tmp-pagesize != mem-pagesize)
+continue;
+
+if (!virBitmapEqual(tmp-sourceNodes, mem-sourceNodes))
+continue;
+
+break;
+}
+
+if (i == def-nmems)
+return -1;
+
+return i;
+}
+
+
+int
+virDomainMemoryFindByDef(virDomainDefPtr def,
+ virDomainMemoryDefPtr mem)
+{
+return virDomainMemoryFindByDefInternal(def, mem, false);
+}
+
[...]
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv3 10/12] qemu: conf: Add support for memory device cold(un)plug
On 03/20/2015 06:39 AM, John Ferlan wrote:
On 03/18/2015 01:51 AM, Zhu Guihua wrote:
On 03/17/2015 10:19 PM, Peter Krempa wrote:
Add a few helpers that allow to operate with memory device definitions
on the domain config and use them to implement memory device coldplug in
the qemu driver.
---
Notes:
Version 2:
- no changes
src/conf/domain_conf.c | 100
+++
src/conf/domain_conf.h | 10 +
src/libvirt_private.syms | 4 ++
src/qemu/qemu_driver.c | 15 ++-
4 files changed, 127 insertions(+), 2 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 8c2234f..1a02e46 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -12811,6 +12811,106 @@ virDomainRNGRemove(virDomainDefPtr def,
}
+static int
+virDomainMemoryFindByDefInternal(virDomainDefPtr def,
+ virDomainMemoryDefPtr mem,
+ bool allowAddressFallback)
+{
+size_t i;
+
+for (i = 0; i def-nmems; i++) {
+virDomainMemoryDefPtr tmp = def-mems[i];
+
+/* address, if present */
+if (allowAddressFallback) {
+if (tmp-info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE)
+continue;
+} else {
+if (mem-info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE
+!virDomainDeviceInfoAddressIsEqual(tmp-info,
mem-info))
+continue;
+}
+
+/* alias, if present */
+if (mem-info.alias
+STRNEQ_NULLABLE(tmp-info.alias, mem-info.alias))
+continue;
+
+/* target info - always present */
+if (tmp-model != mem-model ||
+tmp-targetNode != mem-targetNode ||
+tmp-size != mem-size)
I have tested your series with our qemu memory hot remove patch series,
here would be a possible error.
When hotplug a memory device, its size has been aligned. So the compare for
size here would fail possiblely.
hmm.. Not sure that's necessary - although Peter can make the final
determination... Commit id '57b215a' doesn't modify each def-mems[i]
entry in qemuDomainAlignMemorySizes, rather it gets a value from
virDomainDefSetMemoryInitial and then does the rounding.
If the stored def-mems[i]-size value is/was modified, then I'd agree,
but it doesn't appear to be that way.
If there is a rounding of the value - then please just point it out
Yes, the stored def-mems[i]-size value was modified.
If you assign the size 524287 KiB, the stored value will be 524288.
Thanks,
Zhu
John
Thanks,
Zhu
+continue;
+
+/* source stuff - match with device */
+if (tmp-pagesize != mem-pagesize)
+continue;
+
+if (!virBitmapEqual(tmp-sourceNodes, mem-sourceNodes))
+continue;
+
+break;
+}
+
+if (i == def-nmems)
+return -1;
+
+return i;
+}
+
+
+int
+virDomainMemoryFindByDef(virDomainDefPtr def,
+ virDomainMemoryDefPtr mem)
+{
+return virDomainMemoryFindByDefInternal(def, mem, false);
+}
+
[...]
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv3 10/12] qemu: conf: Add support for memory device cold(un)plug
Add a few helpers that allow to operate with memory device definitions
on the domain config and use them to implement memory device coldplug in
the qemu driver.
---
Notes:
Version 2:
- no changes
src/conf/domain_conf.c | 100 +++
src/conf/domain_conf.h | 10 +
src/libvirt_private.syms | 4 ++
src/qemu/qemu_driver.c | 15 ++-
4 files changed, 127 insertions(+), 2 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 8c2234f..1a02e46 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -12811,6 +12811,106 @@ virDomainRNGRemove(virDomainDefPtr def,
}
+static int
+virDomainMemoryFindByDefInternal(virDomainDefPtr def,
+ virDomainMemoryDefPtr mem,
+ bool allowAddressFallback)
+{
+size_t i;
+
+for (i = 0; i def-nmems; i++) {
+virDomainMemoryDefPtr tmp = def-mems[i];
+
+/* address, if present */
+if (allowAddressFallback) {
+if (tmp-info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE)
+continue;
+} else {
+if (mem-info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE
+!virDomainDeviceInfoAddressIsEqual(tmp-info, mem-info))
+continue;
+}
+
+/* alias, if present */
+if (mem-info.alias
+STRNEQ_NULLABLE(tmp-info.alias, mem-info.alias))
+continue;
+
+/* target info - always present */
+if (tmp-model != mem-model ||
+tmp-targetNode != mem-targetNode ||
+tmp-size != mem-size)
+continue;
+
+/* source stuff - match with device */
+if (tmp-pagesize != mem-pagesize)
+continue;
+
+if (!virBitmapEqual(tmp-sourceNodes, mem-sourceNodes))
+continue;
+
+break;
+}
+
+if (i == def-nmems)
+return -1;
+
+return i;
+}
+
+
+int
+virDomainMemoryFindByDef(virDomainDefPtr def,
+ virDomainMemoryDefPtr mem)
+{
+return virDomainMemoryFindByDefInternal(def, mem, false);
+}
+
+
+int
+virDomainMemoryFindInactiveByDef(virDomainDefPtr def,
+ virDomainMemoryDefPtr mem)
+{
+int ret;
+
+if ((ret = virDomainMemoryFindByDefInternal(def, mem, false)) 0)
+ret = virDomainMemoryFindByDefInternal(def, mem, true);
+
+return ret;
+}
+
+
+int
+virDomainMemoryInsert(virDomainDefPtr def,
+ virDomainMemoryDefPtr mem)
+{
+int id = def-nmems;
+
+if (mem-info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE
+virDomainDefHasDeviceAddress(def, mem-info)) {
+virReportError(VIR_ERR_OPERATION_INVALID, %s,
+ _(Domain already contains a device with the same
+ address));
+return -1;
+}
+
+if (VIR_APPEND_ELEMENT(def-mems, def-nmems, mem) 0)
+return -1;
+
+return id;
+}
+
+
+virDomainMemoryDefPtr
+virDomainMemoryRemove(virDomainDefPtr def,
+ int idx)
+{
+virDomainMemoryDefPtr ret = def-mems[idx];
+VIR_DELETE_ELEMENT(def-mems, idx, def-nmems);
+return ret;
+}
+
+
char *
virDomainDefGetDefaultEmulator(virDomainDefPtr def,
virCapsPtr caps)
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 475a174..eb61aff 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -2860,6 +2860,16 @@ virDomainChrDefGetSecurityLabelDef(virDomainChrDefPtr
def, const char *model);
typedef const char* (*virEventActionToStringFunc)(int type);
typedef int (*virEventActionFromStringFunc)(const char *type);
+int virDomainMemoryInsert(virDomainDefPtr def, virDomainMemoryDefPtr mem)
+ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK;
+virDomainMemoryDefPtr virDomainMemoryRemove(virDomainDefPtr def, int idx)
+ATTRIBUTE_NONNULL(1);
+int virDomainMemoryFindByDef(virDomainDefPtr def, virDomainMemoryDefPtr mem)
+ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK;
+int virDomainMemoryFindInactiveByDef(virDomainDefPtr def,
+ virDomainMemoryDefPtr mem)
+ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK;
+
VIR_ENUM_DECL(virDomainTaint)
VIR_ENUM_DECL(virDomainVirt)
VIR_ENUM_DECL(virDomainBoot)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index f24b449..239bef4 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -335,6 +335,10 @@ virDomainLockFailureTypeToString;
virDomainMemballoonModelTypeFromString;
virDomainMemballoonModelTypeToString;
virDomainMemoryDefFree;
+virDomainMemoryFindByDef;
+virDomainMemoryFindInactiveByDef;
+virDomainMemoryInsert;
+virDomainMemoryRemove;
virDomainNetAppendIpAddress;
virDomainNetDefFormat;
virDomainNetDefFree;
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 300bce4..e948cca
Re: [libvirt] [PATCHv3 10/12] qemu: conf: Add support for memory device cold(un)plug
On 03/17/2015 10:19 PM, Peter Krempa wrote:
Add a few helpers that allow to operate with memory device definitions
on the domain config and use them to implement memory device coldplug in
the qemu driver.
---
Notes:
Version 2:
- no changes
src/conf/domain_conf.c | 100 +++
src/conf/domain_conf.h | 10 +
src/libvirt_private.syms | 4 ++
src/qemu/qemu_driver.c | 15 ++-
4 files changed, 127 insertions(+), 2 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 8c2234f..1a02e46 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -12811,6 +12811,106 @@ virDomainRNGRemove(virDomainDefPtr def,
}
+static int
+virDomainMemoryFindByDefInternal(virDomainDefPtr def,
+ virDomainMemoryDefPtr mem,
+ bool allowAddressFallback)
+{
+size_t i;
+
+for (i = 0; i def-nmems; i++) {
+virDomainMemoryDefPtr tmp = def-mems[i];
+
+/* address, if present */
+if (allowAddressFallback) {
+if (tmp-info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE)
+continue;
+} else {
+if (mem-info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE
+!virDomainDeviceInfoAddressIsEqual(tmp-info, mem-info))
+continue;
+}
+
+/* alias, if present */
+if (mem-info.alias
+STRNEQ_NULLABLE(tmp-info.alias, mem-info.alias))
+continue;
+
+/* target info - always present */
+if (tmp-model != mem-model ||
+tmp-targetNode != mem-targetNode ||
+tmp-size != mem-size)
I have tested your series with our qemu memory hot remove patch series,
here would be a possible error.
When hotplug a memory device, its size has been aligned. So the compare for
size here would fail possiblely.
Thanks,
Zhu
+continue;
+
+/* source stuff - match with device */
+if (tmp-pagesize != mem-pagesize)
+continue;
+
+if (!virBitmapEqual(tmp-sourceNodes, mem-sourceNodes))
+continue;
+
+break;
+}
+
+if (i == def-nmems)
+return -1;
+
+return i;
+}
+
+
+int
+virDomainMemoryFindByDef(virDomainDefPtr def,
+ virDomainMemoryDefPtr mem)
+{
+return virDomainMemoryFindByDefInternal(def, mem, false);
+}
+
[...]
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
