[PATCH v2] virProcessGetStatInfo: add a comment describing why we can not report error
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
participants (1)
-
Ani Sinha