Re: [libvirt] [PATCHv7 15/18] conf: Add 'id' to virDomainResctrlDef

2018-11-12 Thread Wang, Huaqiang 



On 11/6/2018 3:15 AM, John Ferlan wrote:

On 10/22/18 4:01 AM, Wang Huaqiang wrote:

Adding element 'id' to virDomainResctrlDef tracking resource group
id, it reflects the attribute 'id' of of element  in XML.

virResctrlAlloc.id is a copy from virDomanResctrlDef.id.

Signed-off-by: Wang Huaqiang
---
  src/conf/domain_conf.c | 20 
  src/conf/domain_conf.h |  1 +
  2 files changed, 9 insertions(+), 12 deletions(-)


Not sure I see the need to duplicate the alloc->id value especially
since it's "invalid" have "resctrl->alloc == NULL", right?

Can this resctrl->id ever be different?  Not that I can see. I see this
is a desire to reduce an error case in Format processing, but if
virResctrlAllocGetID returned NULL there's other problems...

John


This patch should be removed and will be removed.

It is introduced in series v4, in that series the 'resctrl->alloc' is 
allowed

to be NULL representing a default monitor.
Now we changed our design and resctrl->alloc is always assigned with an
virResctrlAlloc data structure, then 'resctrl->id' is not necessary. We 
can keep the

resctrl ID in alloc->id.

Thanks for review.
Huaqiang


diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 01f795f..1aecd8c 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -2980,6 +2980,7 @@ virDomainResctrlDefFree(virDomainResctrlDefPtr resctrl)
  virObjectUnref(resctrl->alloc);
  virBitmapFree(resctrl->vcpus);
  VIR_FREE(resctrl->monitors);
+VIR_FREE(resctrl->id);
  VIR_FREE(resctrl);
  }
  
@@ -19145,6 +19146,9 @@ virDomainResctrlNew(xmlNodePtr node,

  if (VIR_ALLOC(resctrl) < 0)
  goto cleanup;
  
+if (VIR_STRDUP(resctrl->id, alloc_id) < 0)

+goto cleanup;
+
  if (!(resctrl->vcpus = virBitmapNewCopy(*vcpus))) {
  virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
 _("failed to copy 'vcpus'"));
@@ -27323,13 +27327,9 @@ virDomainCachetuneDefFormat(virBufferPtr buf,
  
  virBufferAsprintf(buf, "  
-if (!(flags & VIR_DOMAIN_DEF_FORMAT_INACTIVE)) {

-const char *alloc_id = virResctrlAllocGetID(resctrl->alloc);
-if (!alloc_id)
-goto cleanup;
+if (!(flags & VIR_DOMAIN_DEF_FORMAT_INACTIVE))
+virBufferAsprintf(buf, " id='%s'", resctrl->id);
  
-virBufferAsprintf(buf, " id='%s'", alloc_id);

-}
  virBufferAddLit(buf, ">\n");
  
  virBufferAddBuffer(buf, &childrenBuf);

@@ -27386,13 +27386,9 @@ virDomainMemorytuneDefFormat(virBufferPtr buf,
  
  virBufferAsprintf(buf, "  
-if (!(flags & VIR_DOMAIN_DEF_FORMAT_INACTIVE)) {

-const char *alloc_id = virResctrlAllocGetID(resctrl->alloc);
-if (!alloc_id)
-goto cleanup;
+if (!(flags & VIR_DOMAIN_DEF_FORMAT_INACTIVE))
+virBufferAsprintf(buf, " id='%s'", resctrl->id);
  
-virBufferAsprintf(buf, " id='%s'", alloc_id);

-}
  virBufferAddLit(buf, ">\n");
  
  virBufferAddBuffer(buf, &childrenBuf);

diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 60f6464..e190aa2 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -2248,6 +2248,7 @@ typedef struct _virDomainResctrlDef virDomainResctrlDef;
  typedef virDomainResctrlDef *virDomainResctrlDefPtr;
  
  struct _virDomainResctrlDef {

+char *id;
  virBitmapPtr vcpus;
  virResctrlAllocPtr alloc;
  



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

Re: [libvirt] [PATCHv7 15/18] conf: Add 'id' to virDomainResctrlDef

2018-11-05 Thread John Ferlan 



On 10/22/18 4:01 AM, Wang Huaqiang wrote:
> Adding element 'id' to virDomainResctrlDef tracking resource group
> id, it reflects the attribute 'id' of of element  in XML.
> 
> virResctrlAlloc.id is a copy from virDomanResctrlDef.id.
> 
> Signed-off-by: Wang Huaqiang 
> ---
>  src/conf/domain_conf.c | 20 
>  src/conf/domain_conf.h |  1 +
>  2 files changed, 9 insertions(+), 12 deletions(-)
> 

Not sure I see the need to duplicate the alloc->id value especially
since it's "invalid" have "resctrl->alloc == NULL", right?

Can this resctrl->id ever be different?  Not that I can see. I see this
is a desire to reduce an error case in Format processing, but if
virResctrlAllocGetID returned NULL there's other problems...

John

> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 01f795f..1aecd8c 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -2980,6 +2980,7 @@ virDomainResctrlDefFree(virDomainResctrlDefPtr resctrl)
>  virObjectUnref(resctrl->alloc);
>  virBitmapFree(resctrl->vcpus);
>  VIR_FREE(resctrl->monitors);
> +VIR_FREE(resctrl->id);
>  VIR_FREE(resctrl);
>  }
>  
> @@ -19145,6 +19146,9 @@ virDomainResctrlNew(xmlNodePtr node,
>  if (VIR_ALLOC(resctrl) < 0)
>  goto cleanup;
>  
> +if (VIR_STRDUP(resctrl->id, alloc_id) < 0)
> +goto cleanup;
> +
>  if (!(resctrl->vcpus = virBitmapNewCopy(*vcpus))) {
>  virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> _("failed to copy 'vcpus'"));
> @@ -27323,13 +27327,9 @@ virDomainCachetuneDefFormat(virBufferPtr buf,
>  
>  virBufferAsprintf(buf, "  
> -if (!(flags & VIR_DOMAIN_DEF_FORMAT_INACTIVE)) {
> -const char *alloc_id = virResctrlAllocGetID(resctrl->alloc);
> -if (!alloc_id)
> -goto cleanup;
> +if (!(flags & VIR_DOMAIN_DEF_FORMAT_INACTIVE))
> +virBufferAsprintf(buf, " id='%s'", resctrl->id);
>  
> -virBufferAsprintf(buf, " id='%s'", alloc_id);
> -}
>  virBufferAddLit(buf, ">\n");
>  
>  virBufferAddBuffer(buf, &childrenBuf);
> @@ -27386,13 +27386,9 @@ virDomainMemorytuneDefFormat(virBufferPtr buf,
>  
>  virBufferAsprintf(buf, "  
> -if (!(flags & VIR_DOMAIN_DEF_FORMAT_INACTIVE)) {
> -const char *alloc_id = virResctrlAllocGetID(resctrl->alloc);
> -if (!alloc_id)
> -goto cleanup;
> +if (!(flags & VIR_DOMAIN_DEF_FORMAT_INACTIVE))
> +virBufferAsprintf(buf, " id='%s'", resctrl->id);
>  
> -virBufferAsprintf(buf, " id='%s'", alloc_id);
> -}
>  virBufferAddLit(buf, ">\n");
>  
>  virBufferAddBuffer(buf, &childrenBuf);
> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
> index 60f6464..e190aa2 100644
> --- a/src/conf/domain_conf.h
> +++ b/src/conf/domain_conf.h
> @@ -2248,6 +2248,7 @@ typedef struct _virDomainResctrlDef virDomainResctrlDef;
>  typedef virDomainResctrlDef *virDomainResctrlDefPtr;
>  
>  struct _virDomainResctrlDef {
> +char *id;
>  virBitmapPtr vcpus;
>  virResctrlAllocPtr alloc;
>  
> 

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

[libvirt] [PATCHv7 15/18] conf: Add 'id' to virDomainResctrlDef

2018-10-22 Thread Wang Huaqiang 
Adding element 'id' to virDomainResctrlDef tracking resource group
id, it reflects the attribute 'id' of of element  in XML.

virResctrlAlloc.id is a copy from virDomanResctrlDef.id.

Signed-off-by: Wang Huaqiang 
---
 src/conf/domain_conf.c | 20 
 src/conf/domain_conf.h |  1 +
 2 files changed, 9 insertions(+), 12 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 01f795f..1aecd8c 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -2980,6 +2980,7 @@ virDomainResctrlDefFree(virDomainResctrlDefPtr resctrl)
 virObjectUnref(resctrl->alloc);
 virBitmapFree(resctrl->vcpus);
 VIR_FREE(resctrl->monitors);
+VIR_FREE(resctrl->id);
 VIR_FREE(resctrl);
 }
 
@@ -19145,6 +19146,9 @@ virDomainResctrlNew(xmlNodePtr node,
 if (VIR_ALLOC(resctrl) < 0)
 goto cleanup;
 
+if (VIR_STRDUP(resctrl->id, alloc_id) < 0)
+goto cleanup;
+
 if (!(resctrl->vcpus = virBitmapNewCopy(*vcpus))) {
 virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("failed to copy 'vcpus'"));
@@ -27323,13 +27327,9 @@ virDomainCachetuneDefFormat(virBufferPtr buf,
 
 virBufferAsprintf(buf, "alloc);
-if (!alloc_id)
-goto cleanup;
+if (!(flags & VIR_DOMAIN_DEF_FORMAT_INACTIVE))
+virBufferAsprintf(buf, " id='%s'", resctrl->id);
 
-virBufferAsprintf(buf, " id='%s'", alloc_id);
-}
 virBufferAddLit(buf, ">\n");
 
 virBufferAddBuffer(buf, &childrenBuf);
@@ -27386,13 +27386,9 @@ virDomainMemorytuneDefFormat(virBufferPtr buf,
 
 virBufferAsprintf(buf, "alloc);
-if (!alloc_id)
-goto cleanup;
+if (!(flags & VIR_DOMAIN_DEF_FORMAT_INACTIVE))
+virBufferAsprintf(buf, " id='%s'", resctrl->id);
 
-virBufferAsprintf(buf, " id='%s'", alloc_id);
-}
 virBufferAddLit(buf, ">\n");
 
 virBufferAddBuffer(buf, &childrenBuf);
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 60f6464..e190aa2 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -2248,6 +2248,7 @@ typedef struct _virDomainResctrlDef virDomainResctrlDef;
 typedef virDomainResctrlDef *virDomainResctrlDefPtr;
 
 struct _virDomainResctrlDef {
+char *id;
 virBitmapPtr vcpus;
 virResctrlAllocPtr alloc;
 
-- 
2.7.4

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