Re: [libvirt] [PATCHv7 03/18] util: Refactor code for determining allocation path

[Huaqiang,Wang]

On 2018年11月05日 23:02, John Ferlan wrote:


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

The code for determining resctrl allocation path could be reused
for monitor. Refactor it for reuse.

Signed-off-by: Wang Huaqiang 
---
  src/util/virresctrl.c | 38 ++
  1 file changed, 30 insertions(+), 8 deletions(-)

diff --git a/src/util/virresctrl.c b/src/util/virresctrl.c
index 6530801..956aca8 100644
--- a/src/util/virresctrl.c
+++ b/src/util/virresctrl.c
@@ -2265,28 +2265,50 @@ virResctrlAllocAssign(virResctrlInfoPtr resctrl,
  }
  
  
+static char *

+virResctrlDeterminePath(const char *parentpath,
+const char *prefix,
+const char *id)
+{
+char *path = NULL;
+
+if (!id) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _("Resctrl ID must be set before determining resctrl "
+ "parentpath='%s'"), parentpath);

Add "for prefix='%s'" w/ prefix as argument especially since for Alloc's
the parent path is SYSFS_RESCTRL_PATH so it's perhaps not specific enough.

With this change that I can make for you,


Your change made the message be more clear.  Agree with your change.



Reviewed-by: John Ferlan 

John


Thanks for your review.
Huaqiang



[...]


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

Re: [libvirt] [PATCHv7 03/18] util: Refactor code for determining allocation path

[John Ferlan]

On 10/22/18 4:01 AM, Wang Huaqiang wrote:
> The code for determining resctrl allocation path could be reused
> for monitor. Refactor it for reuse.
> 
> Signed-off-by: Wang Huaqiang 
> ---
>  src/util/virresctrl.c | 38 ++
>  1 file changed, 30 insertions(+), 8 deletions(-)
> 
> diff --git a/src/util/virresctrl.c b/src/util/virresctrl.c
> index 6530801..956aca8 100644
> --- a/src/util/virresctrl.c
> +++ b/src/util/virresctrl.c
> @@ -2265,28 +2265,50 @@ virResctrlAllocAssign(virResctrlInfoPtr resctrl,
>  }
>  
>  
> +static char *
> +virResctrlDeterminePath(const char *parentpath,
> +const char *prefix,
> +const char *id)
> +{
> +char *path = NULL;
> +
> +if (!id) {
> +virReportError(VIR_ERR_INTERNAL_ERROR,
> +   _("Resctrl ID must be set before determining resctrl "
> + "parentpath='%s'"), parentpath);

Add "for prefix='%s'" w/ prefix as argument especially since for Alloc's
the parent path is SYSFS_RESCTRL_PATH so it's perhaps not specific enough.

With this change that I can make for you,

Reviewed-by: John Ferlan 

John

[...]

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

[libvirt] [PATCHv7 03/18] util: Refactor code for determining allocation path

[Wang Huaqiang]

The code for determining resctrl allocation path could be reused
for monitor. Refactor it for reuse.

Signed-off-by: Wang Huaqiang 
---
 src/util/virresctrl.c | 38 ++
 1 file changed, 30 insertions(+), 8 deletions(-)

diff --git a/src/util/virresctrl.c b/src/util/virresctrl.c
index 6530801..956aca8 100644
--- a/src/util/virresctrl.c
+++ b/src/util/virresctrl.c
@@ -2265,28 +2265,50 @@ virResctrlAllocAssign(virResctrlInfoPtr resctrl,
 }
 
 
+static char *
+virResctrlDeterminePath(const char *parentpath,
+const char *prefix,
+const char *id)
+{
+char *path = NULL;
+
+if (!id) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _("Resctrl ID must be set before determining resctrl "
+ "parentpath='%s'"), parentpath);
+return NULL;
+}
+
+if (virAsprintf(, "%s/%s-%s", parentpath, prefix, id) < 0)
+return NULL;
+
+return path;
+}
+
+
 int
 virResctrlAllocDeterminePath(virResctrlAllocPtr alloc,
  const char *machinename)
 {
-if (!alloc->id) {
-virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
-   _("Resctrl Allocation ID must be set before creation"));
+if (alloc->path) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _("Resctrl allocation path is already set to '%s'"),
+   alloc->path);
 return -1;
 }
 
 /* If the allocation is empty, then the path will be SYSFS_RESCTRL_PATH */
 if (virResctrlAllocIsEmpty(alloc)) {
-if (!alloc->path &&
-VIR_STRDUP(alloc->path, SYSFS_RESCTRL_PATH) < 0)
+if (VIR_STRDUP(alloc->path, SYSFS_RESCTRL_PATH) < 0)
 return -1;
 
 return 0;
 }
 
-if (!alloc->path &&
-virAsprintf(>path, "%s/%s-%s",
-SYSFS_RESCTRL_PATH, machinename, alloc->id) < 0)
+alloc->path = virResctrlDeterminePath(SYSFS_RESCTRL_PATH,
+  machinename, alloc->id);
+
+if (!alloc->path)
 return -1;
 
 return 0;
-- 
2.7.4

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