Re: [libvirt] [PATCH 6/7] Do not use vshPrintPinInfo in iothreadinfo

2015-03-26 Thread Peter Krempa 
On Wed, Mar 25, 2015 at 19:39:11 +0100, Ján Tomko wrote:
 Just format the bitmap via virBitmapFormat.
 ---
  tools/virsh-domain.c | 13 +++--
  1 file changed, 11 insertions(+), 2 deletions(-)
 
 diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
 index afd92b1..cb9cb9d 100644
 --- a/tools/virsh-domain.c
 +++ b/tools/virsh-domain.c
 @@ -6834,6 +6834,7 @@ cmdIOThreadInfo(vshControl *ctl, const vshCmd *cmd)
  size_t i;
  int maxcpu;
  unsigned int flags = VIR_DOMAIN_AFFECT_CURRENT;
 +virBitmapPtr map = NULL;
  
  VSH_EXCLUSIVE_OPTIONS_VAR(current, live);
  VSH_EXCLUSIVE_OPTIONS_VAR(current, config);
 @@ -6863,16 +6864,24 @@ cmdIOThreadInfo(vshControl *ctl, const vshCmd *cmd)
_(IOThread ID), _(CPU Affinity));
  vshPrintExtra(ctl, 
 ---\n);
  for (i = 0; i  niothreads; i++) {
 +char *mapstr = NULL;
 +virBitmapFree(map);
 +map = virBitmapNewData(info[i]-cpumap, info[i]-cpumaplen);
 +if (!map)
 +goto cleanup;
 +
 +if (!(mapstr = virBitmapFormat(map)))
 +goto cleanup;
  
  vshPrint(ctl,  %-15u , info[i]-iothread_id);
 -ignore_value(vshPrintPinInfo(info[i]-cpumap, info[i]-cpumaplen,
 - maxcpu, 0));
 +vshPrint(ctl,  %-15s , mapstr);
  vshPrint(ctl, \n);
  virDomainIOThreadInfoFree(info[i]);
  }
  VIR_FREE(info);
  
   cleanup:
 +virBitmapFree(map);
  virDomainFree(dom);
  return niothreads = 0;
  }

Since vshPrintPinInfo produces the same output, how about we kill the
reimplementation in vshPrintPinInfo and replace it with this code? And
keep the use of vshPrintPinInfo here?

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 6/7] Do not use vshPrintPinInfo in iothreadinfo

2015-03-26 Thread John Ferlan 


On 03/26/2015 05:58 AM, Peter Krempa wrote:
 On Wed, Mar 25, 2015 at 19:39:11 +0100, Ján Tomko wrote:
 Just format the bitmap via virBitmapFormat.
 ---
  tools/virsh-domain.c | 13 +++--
  1 file changed, 11 insertions(+), 2 deletions(-)

 diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
 index afd92b1..cb9cb9d 100644
 --- a/tools/virsh-domain.c
 +++ b/tools/virsh-domain.c
 @@ -6834,6 +6834,7 @@ cmdIOThreadInfo(vshControl *ctl, const vshCmd *cmd)
  size_t i;
  int maxcpu;
  unsigned int flags = VIR_DOMAIN_AFFECT_CURRENT;
 +virBitmapPtr map = NULL;
  
  VSH_EXCLUSIVE_OPTIONS_VAR(current, live);
  VSH_EXCLUSIVE_OPTIONS_VAR(current, config);
 @@ -6863,16 +6864,24 @@ cmdIOThreadInfo(vshControl *ctl, const vshCmd *cmd)
_(IOThread ID), _(CPU Affinity));
  vshPrintExtra(ctl, 
 ---\n);
  for (i = 0; i  niothreads; i++) {
 +char *mapstr = NULL;
 +virBitmapFree(map);
 +map = virBitmapNewData(info[i]-cpumap, info[i]-cpumaplen);
 +if (!map)
 +goto cleanup;
 +
 +if (!(mapstr = virBitmapFormat(map)))
 +goto cleanup;
  
  vshPrint(ctl,  %-15u , info[i]-iothread_id);
 -ignore_value(vshPrintPinInfo(info[i]-cpumap, info[i]-cpumaplen,
 - maxcpu, 0));
 +vshPrint(ctl,  %-15s , mapstr);
  vshPrint(ctl, \n);
  virDomainIOThreadInfoFree(info[i]);
  }
  VIR_FREE(info);
  
   cleanup:
 +virBitmapFree(map);
  virDomainFree(dom);
  return niothreads = 0;
  }
 
 Since vshPrintPinInfo produces the same output, how about we kill the
 reimplementation in vshPrintPinInfo and replace it with this code? And
 keep the use of vshPrintPinInfo here?
 
So then this should also be done for cmdVcpuPin and cmdEmulatorPin since
that was the model...

John

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

Re: [libvirt] [PATCH 6/7] Do not use vshPrintPinInfo in iothreadinfo

2015-03-26 Thread Peter Krempa 
On Thu, Mar 26, 2015 at 06:25:21 -0400, John Ferlan wrote:
 
 
 On 03/26/2015 05:58 AM, Peter Krempa wrote:
  On Wed, Mar 25, 2015 at 19:39:11 +0100, Ján Tomko wrote:
  Just format the bitmap via virBitmapFormat.
  ---
   tools/virsh-domain.c | 13 +++--
   1 file changed, 11 insertions(+), 2 deletions(-)
 
  diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
  index afd92b1..cb9cb9d 100644
  --- a/tools/virsh-domain.c
  +++ b/tools/virsh-domain.c
  @@ -6834,6 +6834,7 @@ cmdIOThreadInfo(vshControl *ctl, const vshCmd *cmd)
   size_t i;
   int maxcpu;
   unsigned int flags = VIR_DOMAIN_AFFECT_CURRENT;
  +virBitmapPtr map = NULL;
   
   VSH_EXCLUSIVE_OPTIONS_VAR(current, live);
   VSH_EXCLUSIVE_OPTIONS_VAR(current, config);
  @@ -6863,16 +6864,24 @@ cmdIOThreadInfo(vshControl *ctl, const vshCmd *cmd)
 _(IOThread ID), _(CPU Affinity));
   vshPrintExtra(ctl, 
  ---\n);
   for (i = 0; i  niothreads; i++) {
  +char *mapstr = NULL;
  +virBitmapFree(map);
  +map = virBitmapNewData(info[i]-cpumap, info[i]-cpumaplen);
  +if (!map)
  +goto cleanup;
  +
  +if (!(mapstr = virBitmapFormat(map)))
  +goto cleanup;
   
   vshPrint(ctl,  %-15u , info[i]-iothread_id);
  -ignore_value(vshPrintPinInfo(info[i]-cpumap, info[i]-cpumaplen,
  - maxcpu, 0));
  +vshPrint(ctl,  %-15s , mapstr);
   vshPrint(ctl, \n);
   virDomainIOThreadInfoFree(info[i]);
   }
   VIR_FREE(info);
   
cleanup:
  +virBitmapFree(map);
   virDomainFree(dom);
   return niothreads = 0;
   }
  
  Since vshPrintPinInfo produces the same output, how about we kill the
  reimplementation in vshPrintPinInfo and replace it with this code? And
  keep the use of vshPrintPinInfo here?
  
 So then this should also be done for cmdVcpuPin and cmdEmulatorPin since
 that was the model...

Or as I've said above, rewrite vshPrintPinInfo() with the internal code
and leave it used in all places (here too).

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 6/7] Do not use vshPrintPinInfo in iothreadinfo

2015-03-26 Thread John Ferlan 


On 03/25/2015 02:39 PM, Ján Tomko wrote:
 Just format the bitmap via virBitmapFormat.
 ---
  tools/virsh-domain.c | 13 +++--
  1 file changed, 11 insertions(+), 2 deletions(-)
 
 diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
 index afd92b1..cb9cb9d 100644
 --- a/tools/virsh-domain.c
 +++ b/tools/virsh-domain.c
 @@ -6834,6 +6834,7 @@ cmdIOThreadInfo(vshControl *ctl, const vshCmd *cmd)
  size_t i;
  int maxcpu;
  unsigned int flags = VIR_DOMAIN_AFFECT_CURRENT;
 +virBitmapPtr map = NULL;
  
  VSH_EXCLUSIVE_OPTIONS_VAR(current, live);
  VSH_EXCLUSIVE_OPTIONS_VAR(current, config);
 @@ -6863,16 +6864,24 @@ cmdIOThreadInfo(vshControl *ctl, const vshCmd *cmd)
_(IOThread ID), _(CPU Affinity));
  vshPrintExtra(ctl, 
 ---\n);
  for (i = 0; i  niothreads; i++) {
 +char *mapstr = NULL;

Considering the other discussion about Set/Add/Del IOThread and since
you're modifying the code anyway...

How about adding a check for:

if (info[i].iothread_id == 0)
continue;

That way we can prepare for a configuration that may have holes on
the delete and won't have some future issue with a 1.2.14 virsh
receiving something unexpected from a 1.2.15 daemon.

John
 +virBitmapFree(map);
 +map = virBitmapNewData(info[i]-cpumap, info[i]-cpumaplen);
 +if (!map)
 +goto cleanup;
 +
 +if (!(mapstr = virBitmapFormat(map)))
 +goto cleanup;
  
  vshPrint(ctl,  %-15u , info[i]-iothread_id);
 -ignore_value(vshPrintPinInfo(info[i]-cpumap, info[i]-cpumaplen,
 - maxcpu, 0));
 +vshPrint(ctl,  %-15s , mapstr);
  vshPrint(ctl, \n);
  virDomainIOThreadInfoFree(info[i]);
  }
  VIR_FREE(info);
  
   cleanup:
 +virBitmapFree(map);
  virDomainFree(dom);
  return niothreads = 0;
  }
 

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

Re: [libvirt] [PATCH 6/7] Do not use vshPrintPinInfo in iothreadinfo

2015-03-26 Thread Ján Tomko 
On Thu, Mar 26, 2015 at 08:50:24AM -0400, John Ferlan wrote:
 
 
 On 03/25/2015 02:39 PM, Ján Tomko wrote:
  Just format the bitmap via virBitmapFormat.
  ---
   tools/virsh-domain.c | 13 +++--
   1 file changed, 11 insertions(+), 2 deletions(-)
  
  diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
  index afd92b1..cb9cb9d 100644
  --- a/tools/virsh-domain.c
  +++ b/tools/virsh-domain.c
  @@ -6834,6 +6834,7 @@ cmdIOThreadInfo(vshControl *ctl, const vshCmd *cmd)
   size_t i;
   int maxcpu;
   unsigned int flags = VIR_DOMAIN_AFFECT_CURRENT;
  +virBitmapPtr map = NULL;
   
   VSH_EXCLUSIVE_OPTIONS_VAR(current, live);
   VSH_EXCLUSIVE_OPTIONS_VAR(current, config);
  @@ -6863,16 +6864,24 @@ cmdIOThreadInfo(vshControl *ctl, const vshCmd *cmd)
 _(IOThread ID), _(CPU Affinity));
   vshPrintExtra(ctl, 
  ---\n);
   for (i = 0; i  niothreads; i++) {
  +char *mapstr = NULL;
 
 Considering the other discussion about Set/Add/Del IOThread and since
 you're modifying the code anyway...
 
 How about adding a check for:
 
 if (info[i].iothread_id == 0)
 continue;
 
 That way we can prepare for a configuration that may have holes on
 the delete and won't have some future issue with a 1.2.14 virsh
 receiving something unexpected from a 1.2.15 daemon.

We would not need to include empty elements in the array - the holes
would be apparent from the presence of a thread with id=1, then id=3.

Otherwise the thread_id element would be redundant.

Jan


signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH 6/7] Do not use vshPrintPinInfo in iothreadinfo

2015-03-25 Thread Ján Tomko 
Just format the bitmap via virBitmapFormat.
---
 tools/virsh-domain.c | 13 +++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index afd92b1..cb9cb9d 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -6834,6 +6834,7 @@ cmdIOThreadInfo(vshControl *ctl, const vshCmd *cmd)
 size_t i;
 int maxcpu;
 unsigned int flags = VIR_DOMAIN_AFFECT_CURRENT;
+virBitmapPtr map = NULL;
 
 VSH_EXCLUSIVE_OPTIONS_VAR(current, live);
 VSH_EXCLUSIVE_OPTIONS_VAR(current, config);
@@ -6863,16 +6864,24 @@ cmdIOThreadInfo(vshControl *ctl, const vshCmd *cmd)
   _(IOThread ID), _(CPU Affinity));
 vshPrintExtra(ctl, 
---\n);
 for (i = 0; i  niothreads; i++) {
+char *mapstr = NULL;
+virBitmapFree(map);
+map = virBitmapNewData(info[i]-cpumap, info[i]-cpumaplen);
+if (!map)
+goto cleanup;
+
+if (!(mapstr = virBitmapFormat(map)))
+goto cleanup;
 
 vshPrint(ctl,  %-15u , info[i]-iothread_id);
-ignore_value(vshPrintPinInfo(info[i]-cpumap, info[i]-cpumaplen,
- maxcpu, 0));
+vshPrint(ctl,  %-15s , mapstr);
 vshPrint(ctl, \n);
 virDomainIOThreadInfoFree(info[i]);
 }
 VIR_FREE(info);
 
  cleanup:
+virBitmapFree(map);
 virDomainFree(dom);
 return niothreads = 0;
 }
-- 
2.0.5

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