Re: [libvirt] [PATCH v5 04/10] Move iothreadspin information into iothreadids
On Fri, Apr 24, 2015 at 12:05:56 -0400, John Ferlan wrote:
Remove the iothreadspin array from cputune and replace with a cpumask
to be stored in the iothreadids list.
Adjust the test output because our printing goes in order of the iothreadids
list now.
Signed-off-by: John Ferlan jfer...@redhat.com
---
docs/formatdomain.html.in | 10 +-
src/conf/domain_conf.c | 112
++---
src/conf/domain_conf.h | 3 +-
src/qemu/qemu_cgroup.c | 16 +--
src/qemu/qemu_driver.c | 75 --
src/qemu/qemu_process.c| 10 +-
.../qemuxml2xmlout-cputune-iothreads.xml | 2 +-
7 files changed, 86 insertions(+), 142 deletions(-)
...
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index ddb0d83..129908d 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
...
@@ -20862,16 +20859,19 @@ virDomainDefFormatInternal(virDomainDefPtr def,
VIR_FREE(cpumask);
}
-for (i = 0; i def-cputune.niothreadspin; i++) {
+for (i = 0; i def-niothreadids; i++) {
char *cpumask;
-/* Ignore the iothreadpin which inherit from cpuset of vcpu. */
-if (virBitmapEqual(def-cpumask,
def-cputune.iothreadspin[i]-cpumask))
+
+/* Ignore iothreadids with no cpumask or a cpumask that
+ * inherits from cpuset of vcpu. */
+if (!def-iothreadids[i]-cpumask ||
+virBitmapEqual(def-cpumask, def-iothreadids[i]-cpumask))
continue;
I still think that comparing the cpu map with the default cpumap is
wrong. It should not be copied there in the first place. Additionally if
the user specifies the same CPU map as the default one, we should not
remove it.
virBufferAsprintf(buf, iothreadpin iothread='%u' ,
- def-cputune.iothreadspin[i]-id);
+ def-iothreadids[i]-iothread_id);
-if (!(cpumask =
virBitmapFormat(def-cputune.iothreadspin[i]-cpumask)))
+if (!(cpumask = virBitmapFormat(def-iothreadids[i]-cpumask)))
goto error;
virBufferAsprintf(buf, cpuset='%s'/\n, cpumask);
...
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 330ffdf..1fac0b8 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
...
@@ -6145,31 +6114,23 @@ qemuDomainPinIOThread(virDomainPtr dom,
}
if (flags VIR_DOMAIN_AFFECT_CONFIG) {
+virDomainIOThreadIDDefPtr iothrid;
+virBitmapPtr cpumask;
+
/* Coverity didn't realize that targetDef must be set if we got
here. */
sa_assert(persistentDef);
-if (iothread_id persistentDef-iothreads) {
+if (!(iothrid = virDomainIOThreadIDFind(persistentDef,
iothread_id))) {
virReportError(VIR_ERR_INVALID_ARG,
- _(iothread value out of range %d %d),
- iothread_id, persistentDef-iothreads);
+ _(iothreadid %d not found), iothread_id);
goto endjob;
}
-if (!persistentDef-cputune.iothreadspin) {
-if (VIR_ALLOC(persistentDef-cputune.iothreadspin) 0)
-goto endjob;
-persistentDef-cputune.niothreadspin = 0;
-}
-if (virDomainPinAdd(persistentDef-cputune.iothreadspin,
-persistentDef-cputune.niothreadspin,
-cpumap,
-maplen,
-iothread_id) 0) {
-virReportError(VIR_ERR_INTERNAL_ERROR, %s,
- _(failed to update or add iothreadspin xml
- of a persistent domain));
+if (!(cpumask = virBitmapNewData(cpumap, maplen)))
goto endjob;
-}
+
+virBitmapFree(iothrid-cpumask);
+iothrid-cpumask = cpumask;
Much nicer!
ret = virDomainSaveConfig(cfg-configDir, persistentDef);
goto endjob;
The code is much cleaner now. The cpu threads/pinning implementation is
horrible in this aspect.
ACK with the bitmap comparison removed.
Peter
signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v5 04/10] Move iothreadspin information into iothreadids
On 04/27/2015 10:20 AM, Peter Krempa wrote:
On Fri, Apr 24, 2015 at 12:05:56 -0400, John Ferlan wrote:
Remove the iothreadspin array from cputune and replace with a cpumask
to be stored in the iothreadids list.
Adjust the test output because our printing goes in order of the iothreadids
list now.
Signed-off-by: John Ferlan jfer...@redhat.com
---
docs/formatdomain.html.in | 10 +-
src/conf/domain_conf.c | 112
++---
src/conf/domain_conf.h | 3 +-
src/qemu/qemu_cgroup.c | 16 +--
src/qemu/qemu_driver.c | 75 --
src/qemu/qemu_process.c| 10 +-
.../qemuxml2xmlout-cputune-iothreads.xml | 2 +-
7 files changed, 86 insertions(+), 142 deletions(-)
...
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index ddb0d83..129908d 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
...
@@ -20862,16 +20859,19 @@ virDomainDefFormatInternal(virDomainDefPtr def,
VIR_FREE(cpumask);
}
-for (i = 0; i def-cputune.niothreadspin; i++) {
+for (i = 0; i def-niothreadids; i++) {
char *cpumask;
-/* Ignore the iothreadpin which inherit from cpuset of vcpu. */
-if (virBitmapEqual(def-cpumask,
def-cputune.iothreadspin[i]-cpumask))
+
+/* Ignore iothreadids with no cpumask or a cpumask that
+ * inherits from cpuset of vcpu. */
+if (!def-iothreadids[i]-cpumask ||
+virBitmapEqual(def-cpumask, def-iothreadids[i]-cpumask))
continue;
I still think that comparing the cpu map with the default cpumap is
wrong. It should not be copied there in the first place. Additionally if
the user specifies the same CPU map as the default one, we should not
remove it.
Removed the virBitmapEqual(def-cpumask, def-iothreadids[i]-cpumask)
check.
John
virBufferAsprintf(buf, iothreadpin iothread='%u' ,
- def-cputune.iothreadspin[i]-id);
+ def-iothreadids[i]-iothread_id);
-if (!(cpumask =
virBitmapFormat(def-cputune.iothreadspin[i]-cpumask)))
+if (!(cpumask = virBitmapFormat(def-iothreadids[i]-cpumask)))
goto error;
virBufferAsprintf(buf, cpuset='%s'/\n, cpumask);
...
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 330ffdf..1fac0b8 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
...
@@ -6145,31 +6114,23 @@ qemuDomainPinIOThread(virDomainPtr dom,
}
if (flags VIR_DOMAIN_AFFECT_CONFIG) {
+virDomainIOThreadIDDefPtr iothrid;
+virBitmapPtr cpumask;
+
/* Coverity didn't realize that targetDef must be set if we got
here. */
sa_assert(persistentDef);
-if (iothread_id persistentDef-iothreads) {
+if (!(iothrid = virDomainIOThreadIDFind(persistentDef,
iothread_id))) {
virReportError(VIR_ERR_INVALID_ARG,
- _(iothread value out of range %d %d),
- iothread_id, persistentDef-iothreads);
+ _(iothreadid %d not found), iothread_id);
goto endjob;
}
-if (!persistentDef-cputune.iothreadspin) {
-if (VIR_ALLOC(persistentDef-cputune.iothreadspin) 0)
-goto endjob;
-persistentDef-cputune.niothreadspin = 0;
-}
-if (virDomainPinAdd(persistentDef-cputune.iothreadspin,
-persistentDef-cputune.niothreadspin,
-cpumap,
-maplen,
-iothread_id) 0) {
-virReportError(VIR_ERR_INTERNAL_ERROR, %s,
- _(failed to update or add iothreadspin xml
- of a persistent domain));
+if (!(cpumask = virBitmapNewData(cpumap, maplen)))
goto endjob;
-}
+
+virBitmapFree(iothrid-cpumask);
+iothrid-cpumask = cpumask;
Much nicer!
ret = virDomainSaveConfig(cfg-configDir, persistentDef);
goto endjob;
The code is much cleaner now. The cpu threads/pinning implementation is
horrible in this aspect.
ACK with the bitmap comparison removed.
Peter
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
