Merge ~mirespace/ubuntu/+source/sysstat:lp-1888345-sysstat-iostat-focal--outputJSONfix into ubuntu/+source/sysstat:ubuntu/focal-devel

[Miriam España Acebal]

Hello team,

Here you have the patch proposal for fixing LP: #1888345 for Focal. The number of megabytes written to the device per second was wrong when the output was requested on JSON format (iostat command).

It was made cherry-picking commit from upstream as recommended by sysstat repository's owner (https://github.com/sysstat/sysstat/issues/264) to create later the patch in d/p/.

Anyway, checking system behaviour before and after fixing (grabbing output for the same time, two shells):

Before (spoiler: 0.23 vs 483.45 wMB/s):
- Non JSON output: https://pastebin.canonical.com/p/vXJ2xQPXW3/
- JSON output: https://pastebin.canonical.com/p/ncGvCzgHCg/

After (spoiler: 0.03 vs 0.03 wMB/s):
- Non JSON output: https://pastebin.canonical.com/p/QhmWBHXCcG/
- JSON output: https://pastebin.canonical.com/p/XWQtNGHNkP/

SRU template is not present yet in the bug discussion on LP.

Anything I must change/add, please let me know... Thanks in advance!

[Bryce Harrington (bryce)]

Guessing that Christian will be doing the review for this one, but one thing to mention is that when we add patches to a package, our team's standard practice is to add some DEP3 metadata fields.

Here is the template that I use; I suggest saving this locally as you'll be using it often. The referenced link explains what the fields mean.

Description: ...
 ...
Author:
Origin: upstream|backport|vendor|other, https://...
Bug: https://...
Bug-Ubuntu: https://bugs.launchpad.net/bind/+bug/NNNNNN
Bug-Debian: ...
Reviewed-By: ...
Acked-By: ...
Forwarded: no | not-needed | https://salsa.debian.org/....
Applied-Upstream: <release that the fix is expected to be in>, commit:SHA
Last-Update: 2019-06-27

# See https://dep-team.pages.debian.net/deps/dep3/

Christian can give good guidance on what precisely should be included, but for this patch I am guessing at least "Origin", "Bug-Ubuntu:", "Bug:" should be added to the patch, after the "Signed-off-by:" line.

---

One other important note is that when doing SRUs, the version number is always incremented by a decimal fraction. So in this case the version number should be 12.2.0-2ubuntu0.1

---

review: Needs Fixing

[Bryce Harrington (bryce)]

Oh also, while it's not required I strongly encourage doing at least a preliminary draft of the SRU bug report template text before submitting the MP. The reason is that very often with SRUs that text is the hardest part, so getting extra review attention early can be very helpful.

Along with the DEP3 metadata template, I also keep a template for SRU text. I think this is copied from a webpage, and might be a bit dated, but here is what I use:

[Impact]
* An explanation of the effects of the bug on users and
  justification for backporting the fix to the stable release.

* In addition, it is helpful, but not required, to include an
  explanation of how the upload fixes this bug.

[Test Case]
* Detailed instructions how to reproduce the bug

* These should allow someone who is not familiar with the affected
  package to reproduce the bug and verify that the updated package fixes
  the problem.

[Where Problems Could Occur]
* Think about what the upload changes in the software. Imagine the change is
  wrong or breaks something else: how would this show up?

* It is assumed that any SRU candidate patch is well-tested before
  upload and has a low overall risk of regression, but it's important
  to make the effort to think about what ''could'' happen in the
  event of a regression.

* This must '''never''' be "None" or "Low", or entirely an argument as to why
  your upload is low risk.

* This both shows the SRU team that the risks have been considered,
  and provides guidance to testers in regression-testing the SRU.

[Other Info]

* Anything else you think is useful to include
* Anticipate questions from users, SRU, +1 maintenance, security teams and the Technical Board
  and address these questions in advance

[Original Report]

[Miriam España Acebal (mirespace)]

Hi Bryce, team!

thanks for all the comments.. I fixed them... I forgot to put the PPA: https://launchpad.net/~mirespace/+archive/ubuntu/srus/+packages

Thanks!

[Christian Ehrhardt  (paelzer)]

We did a session explaining a bit more in detail what was missing.
Thanks bryce for the pre-review we could perfectly use it to go step by step through the "why" these changes are needed.

The version is fixed

The new Deb-3 header is fine, although forwarded is not needed it is also not wrong.

We also went over adding a PPA reference here (done) and added the SRU Template.

Thereby it LGTM now.

review: Approve

[Christian Ehrhardt  (paelzer)]

Also fixed whitspace damage and fixed it

[Christian Ehrhardt  (paelzer)]

To ssh://git.launchpad.net/~usd-import-team/ubuntu/+source/sysstat
 * [new tag] upload/12.2.0-2ubuntu0.1 -> upload/12.2.0-2ubuntu0.1

Uploading to ubuntu (via ftp to upload.ubuntu.com):
  Uploading sysstat_12.2.0-2ubuntu0.1.dsc: done.
  Uploading sysstat_12.2.0-2ubuntu0.1.debian.tar.xz: done.
  Uploading sysstat_12.2.0-2ubuntu0.1_source.buildinfo: done.
  Uploading sysstat_12.2.0-2ubuntu0.1_source.changes: done.
Successfully uploaded packages.

Unmerged commits

58cb909... by Miriam España Acebal on 2021-08-09

changelog

c255d26... by Miriam España Acebal on 2021-08-09

update-maintainers

317c21a... by Miriam España Acebal on 2021-08-09

  * d/p/12-fix-wMB-json-output.patch: Correct values for the number of
    megabytes written to the device per second when using JSON output.
    Thanks to Sebastien GODARD <email address hidden> #264
    on upstream (LP: #1888345).

diff --git a/debian/changelog b/debian/changelog
index 856c122..d61cf3b 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,12 @@
+sysstat (12.2.0-2ubuntu0.1) focal; urgency=medium
+    
+  * d/p/12-fix-wMB-json-output.patch: Correct values for the number of
+    megabytes written to the device per second when using JSON output.
+    Thanks to Sebastien GODARD <[e-mail]> #264
+    on upstream (LP: #1888345).
+
+ -- Miriam España Acebal <[e-mail]>  Mon, 09 Aug 2021 16:08:06 +0200
+
 sysstat (12.2.0-2) unstable; urgency=medium
 
   * Add 11-Double-free-in-check_file_actlst.patch, taken from upstream,
diff --git a/debian/control b/debian/control
index 3f16f09..099e70b 100644
--- a/debian/control
+++ b/debian/control
@@ -1,7 +1,8 @@
 Source: sysstat
 Section: admin
 Priority: optional
-Maintainer: Robert Luberda <[e-mail]>
+Maintainer: Ubuntu Developers <[e-mail]>
+XSBC-Original-Maintainer: Robert Luberda <[e-mail]>
 Build-Depends: debhelper-compat (= 12), gettext, libsensors-dev, pkg-config
 Standards-Version: 4.4.1
 Rules-Requires-Root: no
diff --git a/debian/patches/12-fix-wMB-json-output.patch b/debian/patches/12-fix-wMB-json-output.patch
new file mode 100644
index 0000000..3591655
--- /dev/null
+++ b/debian/patches/12-fix-wMB-json-output.patch
@@ -0,0 +1,49 @@
+From: Sebastien GODARD <[e-mail]>
+Date: Sun Nov 24 08:55:40 2019 +0100
+Description: iostat: Fix wrong unit used in JSON output
+
+The values for the amount of data read/written or discarded were always
+expressed in blocks/s in the JSON output generated by iostat. It should
+take into account the unit (blocks, kB, MB) selected by the user.
+
+Signed-off-by: Sebastien GODARD <[e-mail]>
+
+Author: Miriam España Acebal <[e-mail]>
+Origin: upstream, https://github.com/sysstat/sysstat/commit/404eee1417dad8abe6ef49ea6e1469fe6cfdddbe
+Bug: https://github.com/sysstat/sysstat/issues/264
+Bug-Ubuntu:https://bugs.launchpad.net/ubuntu/+source/sysstat/+bug/1888345
+Forwarded: not-needed
+Applied-Upstream: Fixed in version 12.3.1, 404eee1417dad8abe6ef49ea6e1469fe6cfdddbe
+Last-Update: 2021-08-10
+
+---
+ 1 file changed, 5 insertions(+), 5 deletions(-)
+
+diff --git a/iostat.c b/iostat.c
+index 5aa7a062..41a1dbd5 100644
+--- a/iostat.c
++++ b/iostat.c
+@@ -857,11 +857,6 @@ void write_disk_stat_header(int *fctr, int *tab, int hpart)
+ {
+ 	char *units, *spc;
+ 
+-	if (DISPLAY_JSON_OUTPUT(flags)) {
+-		xprintf((*tab)++, "\"disk\": [");
+-		return;
+-	}
+-
+ 	if (DISPLAY_KILOBYTES(flags)) {
+ 		*fctr = 2;
+ 		units = "kB";
+@@ -881,6 +876,11 @@ void write_disk_stat_header(int *fctr, int *tab, int hpart)
+ 		spc = "";
+ 	}
+ 
++	if (DISPLAY_JSON_OUTPUT(flags)) {
++		xprintf((*tab)++, "\"disk\": [");
++		return;
++	}
++
+ 	if (!DISPLAY_HUMAN_READ(flags)) {
+ 		printf("Device       ");
+ 	}
diff --git a/debian/patches/series b/debian/patches/series
index 7caf071..2142bc1 100644
--- a/debian/patches/series
+++ b/debian/patches/series
@@ -8,3 +8,4 @@
 09-enable-colors.patch
 10-ignore-ut-failures.patch
 11-Double-free-in-check_file_actlst.patch
+12-fix-wMB-json-output.patch