virProcessGetStatInfo() currently is unable to report error conditions because
that breaks libvirt's public best effort APIs. We add a comment in the function
to indicate this. Adding comment here prevents others from going down the path
of reporting error conditions in this functions in the future. It also reminds
us that at some point in the future we need to fix the code so that this
limitations no longer exists.

Please also see commit
105dace22cc7 ("Revert "report error when virProcessGetStatInfo() is unable to parse data"")

Signed-off-by: Ani Sinha <ani@anisinha.ca>
---
 src/util/virprocess.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/src/util/virprocess.c b/src/util/virprocess.c
index b559a4257e..9422829b8b 100644
--- a/src/util/virprocess.c
+++ b/src/util/virprocess.c
@@ -1784,6 +1784,12 @@ virProcessGetStatInfo(unsigned long long *cpuTime,
         virStrToLong_ullp(proc_stat[VIR_PROCESS_STAT_STIME], NULL, 10, &systime) < 0 ||
         virStrToLong_l(proc_stat[VIR_PROCESS_STAT_RSS], NULL, 10, &rss) < 0 ||
         virStrToLong_i(proc_stat[VIR_PROCESS_STAT_PROCESSOR], NULL, 10, &cpu) < 0) {
+        /* This function can not report error at present. Reporting error here
+         * causes some of libvirt's best effort public APIs to fail. This
+         * resuts in external API behavior change. Until we can fix this in
+         * a way so that public API behavior remains unchanged, we can only
+         * write a warning log here.
+         */
         VIR_WARN("cannot parse process status data");
     }
 
-- 
2.25.1

ping ...

On Fri, 21 Jan 2022, Ani Sinha wrote:

> virProcessGetStatInfo() currently is unable to report error conditions because
> that breaks libvirt's public best effort APIs. We add a comment in the function
> to indicate this. Adding comment here prevents others from going down the path
> of reporting error conditions in this functions in the future. It also reminds
> us that at some point in the future we need to fix the code so that this
> limitations no longer exists.
>
> Please also see commit
> 105dace22cc7 ("Revert "report error when virProcessGetStatInfo() is unable to parse data"")
>
> Signed-off-by: Ani Sinha <ani@anisinha.ca>
> ---
>  src/util/virprocess.c | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/src/util/virprocess.c b/src/util/virprocess.c
> index b559a4257e..9422829b8b 100644
> --- a/src/util/virprocess.c
> +++ b/src/util/virprocess.c
> @@ -1784,6 +1784,12 @@ virProcessGetStatInfo(unsigned long long *cpuTime,
>          virStrToLong_ullp(proc_stat[VIR_PROCESS_STAT_STIME], NULL, 10, &systime) < 0 ||
>          virStrToLong_l(proc_stat[VIR_PROCESS_STAT_RSS], NULL, 10, &rss) < 0 ||
>          virStrToLong_i(proc_stat[VIR_PROCESS_STAT_PROCESSOR], NULL, 10, &cpu) < 0) {
> +        /* This function can not report error at present. Reporting error here
> +         * causes some of libvirt's best effort public APIs to fail. This
> +         * resuts in external API behavior change. Until we can fix this in
> +         * a way so that public API behavior remains unchanged, we can only
> +         * write a warning log here.
> +         */
>          VIR_WARN("cannot parse process status data");
>      }
>
> --
> 2.25.1
>
>

Pinging again in case there is any interest ..

On Tue, Jan 25, 2022 at 4:34 PM Ani Sinha <ani@anisinha.ca> wrote:
>
> ping ...
>
> On Fri, 21 Jan 2022, Ani Sinha wrote:
>
> > virProcessGetStatInfo() currently is unable to report error conditions because
> > that breaks libvirt's public best effort APIs. We add a comment in the function
> > to indicate this. Adding comment here prevents others from going down the path
> > of reporting error conditions in this functions in the future. It also reminds
> > us that at some point in the future we need to fix the code so that this
> > limitations no longer exists.
> >
> > Please also see commit
> > 105dace22cc7 ("Revert "report error when virProcessGetStatInfo() is unable to parse data"")
> >
> > Signed-off-by: Ani Sinha <ani@anisinha.ca>
> > ---
> >  src/util/virprocess.c | 6 ++++++
> >  1 file changed, 6 insertions(+)
> >
> > diff --git a/src/util/virprocess.c b/src/util/virprocess.c
> > index b559a4257e..9422829b8b 100644
> > --- a/src/util/virprocess.c
> > +++ b/src/util/virprocess.c
> > @@ -1784,6 +1784,12 @@ virProcessGetStatInfo(unsigned long long *cpuTime,
> >          virStrToLong_ullp(proc_stat[VIR_PROCESS_STAT_STIME], NULL, 10, &systime) < 0 ||
> >          virStrToLong_l(proc_stat[VIR_PROCESS_STAT_RSS], NULL, 10, &rss) < 0 ||
> >          virStrToLong_i(proc_stat[VIR_PROCESS_STAT_PROCESSOR], NULL, 10, &cpu) < 0) {
> > +        /* This function can not report error at present. Reporting error here
> > +         * causes some of libvirt's best effort public APIs to fail. This
> > +         * resuts in external API behavior change. Until we can fix this in
> > +         * a way so that public API behavior remains unchanged, we can only
> > +         * write a warning log here.
> > +         */
> >          VIR_WARN("cannot parse process status data");
> >      }
> >
> > --
> > 2.25.1
> >
> >