All of lore.kernel.org
 help / color / mirror / Atom feed
From: no-reply@patchew.org
To: junyan.he@intel.com
Cc: famz@redhat.com, qemu-devel@nongnu.org,
	kwolf@redhat.comfamz@redhat.com, crosthwaite.peter@gmail.com,
	quintela@redhat.com, dgilbert@redhat.com, mreitz@redhat.com,
	pbonzini@redhat.com, rth@twiddle.net
Subject: Re: [Qemu-devel] [PATCH 00/10] RFC: Optimize nvdimm kind memory for snapshot.
Date: Thu, 15 Mar 2018 06:55:18 -0700 (PDT)	[thread overview]
Message-ID: <152112211727.51.6572002419251025407@71c20359a636> (raw)
In-Reply-To: <1520930033-18885-1-git-send-email-junyan.he@intel.com>

Hi,

This series seems to have some coding style problems. See output below for
more information:

Type: series
Message-id: 1520930033-18885-1-git-send-email-junyan.he@intel.com
Subject: [Qemu-devel] [PATCH 00/10] RFC: Optimize nvdimm kind memory for snapshot.

=== TEST SCRIPT BEGIN ===
#!/bin/bash

BASE=base
n=1
total=$(git log --oneline $BASE.. | wc -l)
failed=0

git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram

commits="$(git log --format=%H --reverse $BASE..)"
for c in $commits; do
    echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..."
    if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then
        failed=1
        echo
    fi
    n=$((n+1))
done

exit $failed
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 * [new tag]               patchew/1520930033-18885-1-git-send-email-junyan.he@intel.com -> patchew/1520930033-18885-1-git-send-email-junyan.he@intel.com
Switched to a new branch 'test'
856a5e9687 RFC: Enable nvdimm snapshot functions.
4c1adceb28 RFC: Add nvdimm snapshot saving to migration.
424778c4de RFC: Add a section_id parameter to save_live_iterate call.
f9abbe71cd RFC: Add get_current_snapshot_info to get the snapshot state.
f49a5b0838 RFC: Add save dependency functions to qemu_file
adf712a616 RFC: Add memory region snapshot bitmap get function.
3fa9484cb8 RFC: Set memory_region_set_log available for more client.
9d40dbb19e RFC: Implement save and support snapshot dependency in block driver layer.
c43e8a1da2 RFC: Implement qcow2's snapshot dependent saving function.
e022968c94 RFC: Add save and support snapshot dependency function to block driver.

=== OUTPUT BEGIN ===
Checking PATCH 1/10: RFC: Add save and support snapshot dependency function to block driver....
Checking PATCH 2/10: RFC: Implement qcow2's snapshot dependent saving function....
Checking PATCH 3/10: RFC: Implement save and support snapshot dependency in block driver layer....
Checking PATCH 4/10: RFC: Set memory_region_set_log available for more client....
Checking PATCH 5/10: RFC: Add memory region snapshot bitmap get function....
Checking PATCH 6/10: RFC: Add save dependency functions to qemu_file...
Checking PATCH 7/10: RFC: Add get_current_snapshot_info to get the snapshot state....
Checking PATCH 8/10: RFC: Add a section_id parameter to save_live_iterate call....
Checking PATCH 9/10: RFC: Add nvdimm snapshot saving to migration....
WARNING: line over 80 characters
#122: FILE: migration/nvdimm.c:70:
+------------------------------------------------------------------------------------

WARNING: line over 80 characters
#123: FILE: migration/nvdimm.c:71:
+| DIRTY_BITMAP_ID | total size | ram name size | ram name | ram size | bitmap size |

WARNING: line over 80 characters
#124: FILE: migration/nvdimm.c:72:
+------------------------------------------------------------------------------------

WARNING: line over 80 characters
#129: FILE: migration/nvdimm.c:77:
+---------------------------------------------------------------------------------------

WARNING: line over 80 characters
#130: FILE: migration/nvdimm.c:78:
+| DATA_ID | size | ram name size | ram name | ram size | data size | data... | END_ID |

WARNING: line over 80 characters
#131: FILE: migration/nvdimm.c:79:
+---------------------------------------------------------------------------------------

ERROR: do not use C99 // comments
#243: FILE: migration/nvdimm.c:191:
+    padding_sz -= sizeof(int32_t); // NVDIMM_SECTION_PADDING_ID

ERROR: do not use C99 // comments
#244: FILE: migration/nvdimm.c:192:
+    padding_sz -= sizeof(int32_t); // NVDIMM_PADDING_BYTE size

ERROR: do not use C99 // comments
#245: FILE: migration/nvdimm.c:193:
+    padding_sz -= sizeof(int32_t); // NVDIMM_SECTION_END_ID

ERROR: do not use C99 // comments
#344: FILE: migration/nvdimm.c:292:
+            data_sz += sizeof(int); // Zero page, just a ID

ERROR: do not use C99 // comments
#346: FILE: migration/nvdimm.c:294:
+            data_sz += ((1 << TARGET_PAGE_BITS) + sizeof(int)); // ID + page

ERROR: do not use C99 // comments
#350: FILE: migration/nvdimm.c:298:
+    total_sz = sizeof(unsigned int); // NVDIMM_SECTION_DIRTY_BITMAP_ID

ERROR: do not use C99 // comments
#351: FILE: migration/nvdimm.c:299:
+    total_sz += sizeof(uint64_t);    // the total size itself

ERROR: do not use C99 // comments
#352: FILE: migration/nvdimm.c:300:
+    total_sz += sizeof(int);         // ram name size

ERROR: do not use C99 // comments
#355: FILE: migration/nvdimm.c:303:
+    total_sz += sizeof(uint64_t); // ram size

ERROR: do not use C99 // comments
#356: FILE: migration/nvdimm.c:304:
+    total_sz += sizeof(uint64_t); // data size

ERROR: do not use C99 // comments
#358: FILE: migration/nvdimm.c:306:
+    total_sz += sizeof(unsigned int); // NVDIMM_SECTION_END_ID

WARNING: line over 80 characters
#414: FILE: migration/nvdimm.c:362:
+        memory_region_size(nvdimm_state->blocks[i]->mr) >> (TARGET_PAGE_BITS + 3);

WARNING: line over 80 characters
#421: FILE: migration/nvdimm.c:369:
+                                             snap, addr, 1 << TARGET_PAGE_BITS)) {

ERROR: do not use C99 // comments
#426: FILE: migration/nvdimm.c:374:
+    total_sz = sizeof(unsigned int); // NVDIMM_SECTION_DIRTY_BITMAP_ID

ERROR: do not use C99 // comments
#427: FILE: migration/nvdimm.c:375:
+    total_sz += sizeof(uint64_t);    // the total size itself

ERROR: do not use C99 // comments
#428: FILE: migration/nvdimm.c:376:
+    total_sz += sizeof(int);         // ram name size

ERROR: do not use C99 // comments
#431: FILE: migration/nvdimm.c:379:
+    total_sz += sizeof(uint64_t); // ram size

ERROR: do not use C99 // comments
#432: FILE: migration/nvdimm.c:380:
+    total_sz += sizeof(uint64_t); // bitmap size

ERROR: do not use C99 // comments
#434: FILE: migration/nvdimm.c:382:
+    total_sz += sizeof(uint64_t); // data size

ERROR: do not use C99 // comments
#436: FILE: migration/nvdimm.c:384:
+    total_sz += sizeof(unsigned int); // NVDIMM_SECTION_END_ID

WARNING: line over 80 characters
#453: FILE: migration/nvdimm.c:401:
+                                                 snap, addr, 1 << TARGET_PAGE_BITS)) {

ERROR: do not use C99 // comments
#552: FILE: migration/nvdimm.c:500:
+    if (QEMU_IS_ALIGNED(cur_pos, alignment)) { // Already aligned

ERROR: trailing statements should be on next line
#552: FILE: migration/nvdimm.c:500:
+    if (QEMU_IS_ALIGNED(cur_pos, alignment)) { // Already aligned

ERROR: g_free(NULL) is safe this check is probably not required
#594: FILE: migration/nvdimm.c:542:
+        if (nvdimm_state->cur_snapshot_id) {
+            g_free(nvdimm_state->cur_snapshot_id);

ERROR: g_free(NULL) is safe this check is probably not required
#597: FILE: migration/nvdimm.c:545:
+        if (nvdimm_state->blocks) {
+            g_free(nvdimm_state->blocks);

ERROR: g_free(NULL) is safe this check is probably not required
#676: FILE: migration/nvdimm.c:624:
+        if (nvdimm_state->depend_snapshot_id) {
+            g_free(nvdimm_state->depend_snapshot_id);

ERROR: do not use C99 // comments
#695: FILE: migration/nvdimm.c:643:
+                // Can not find the same block?

ERROR: g_free(NULL) is safe this check is probably not required
#896: FILE: migration/nvdimm.c:844:
+    if (bitmap_buf) {
+        g_free(bitmap_buf);

ERROR: g_free(NULL) is safe this check is probably not required
#983: FILE: migration/nvdimm.c:931:
+    if (buf) {
+        g_free(buf);

ERROR: g_free(NULL) is safe this check is probably not required
#1062: FILE: migration/nvdimm.c:1010:
+    if (buf) {
+        g_free(buf);

total: 27 errors, 9 warnings, 1050 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

Checking PATCH 10/10: RFC: Enable nvdimm snapshot functions....
ERROR: do not use C99 // comments
#47: FILE: migration/ram.c:1596:
+            // If snapshot and the block is nvdimm, let nvdimm do the job

ERROR: do not use C99 // comments
#58: FILE: migration/ram.c:2239:
+            // If snapshot and the block is nvdimm, let nvdimm do the job

total: 2 errors, 0 warnings, 51 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

=== OUTPUT END ===

Test command exited with code: 1


---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-devel@freelists.org

  parent reply	other threads:[~2018-03-15 13:56 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-03-13  8:33 [Qemu-devel] [PATCH 00/10] RFC: Optimize nvdimm kind memory for snapshot junyan.he
2018-03-13  8:33 ` [Qemu-devel] [PATCH 01/10] RFC: Add save and support snapshot dependency function to block driver junyan.he
2018-03-13  8:33 ` [Qemu-devel] [PATCH 02/10] RFC: Implement qcow2's snapshot dependent saving function junyan.he
2018-05-08 14:50   ` Eric Blake
2018-05-14 12:59     ` Stefan Hajnoczi
2018-03-13  8:33 ` [Qemu-devel] [PATCH 03/10] RFC: Implement save and support snapshot dependency in block driver layer junyan.he
2018-03-13  8:33 ` [Qemu-devel] [PATCH 04/10] RFC: Set memory_region_set_log available for more client junyan.he
2018-03-13  8:33 ` [Qemu-devel] [PATCH 05/10] RFC: Add memory region snapshot bitmap get function junyan.he
2018-03-13  8:33 ` [Qemu-devel] [PATCH 06/10] RFC: Add save dependency functions to qemu_file junyan.he
2018-03-13  8:33 ` [Qemu-devel] [PATCH 07/10] RFC: Add get_current_snapshot_info to get the snapshot state junyan.he
2018-03-13  8:33 ` [Qemu-devel] [PATCH 08/10] RFC: Add a section_id parameter to save_live_iterate call junyan.he
2018-03-13  8:33 ` [Qemu-devel] [PATCH 09/10] RFC: Add nvdimm snapshot saving to migration junyan.he
2018-03-13  8:33 ` [Qemu-devel] [PATCH 10/10] RFC: Enable nvdimm snapshot functions junyan.he
2018-03-15 13:55 ` no-reply [this message]
2018-03-15 14:15 ` [Qemu-devel] [PATCH 00/10] RFC: Optimize nvdimm kind memory for snapshot no-reply
2018-05-08  9:23 ` Stefan Hajnoczi
2018-03-14  1:20 junyan.he
2018-03-14  1:31 ` no-reply
2018-03-14  1:36 ` no-reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=152112211727.51.6572002419251025407@71c20359a636 \
    --to=no-reply@patchew.org \
    --cc=famz@redhat.com \
    --cc=junyan.he@intel.com \
    --cc=kwolf@redhat.comfamz \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.