From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:60183) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XVsgy-0002gO-7P for qemu-devel@nongnu.org; Sun, 21 Sep 2014 21:45:29 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XVsgw-0007SC-QY for qemu-devel@nongnu.org; Sun, 21 Sep 2014 21:45:28 -0400 Received: from mail-oi0-x22f.google.com ([2607:f8b0:4003:c06::22f]:57513) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XVsgw-0007Qf-F1 for qemu-devel@nongnu.org; Sun, 21 Sep 2014 21:45:26 -0400 Received: by mail-oi0-f47.google.com with SMTP id e131so2988598oig.6 for ; Sun, 21 Sep 2014 18:45:19 -0700 (PDT) MIME-Version: 1.0 In-Reply-To: <20140902191227.7f07b4f0@bahia.local> References: <1409568798-2292-1-git-send-email-junmuzi@gmail.com> <20140902191227.7f07b4f0@bahia.local> Date: Mon, 22 Sep 2014 09:45:18 +0800 Message-ID: From: jun muzi Content-Type: multipart/alternative; boundary=001a11c2cbd86febbd05039d9a25 Subject: Re: [Qemu-devel] [PATCH v2] qcow2: add update refcount table realization for update_refcount List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Greg Kurz Cc: kwolf@redhat.com, Fam Zheng , qemu-devel@nongnu.org, stefanha@redhat.com --001a11c2cbd86febbd05039d9a25 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Thanks. I will give a new version in v3 of qcow2 shrink. Jun Li 2014-9-3 =E4=B8=8A=E5=8D=881:12=E4=BA=8E "Greg Kurz" =E5=86=99=E9=81=93=EF=BC=9A > On Mon, 1 Sep 2014 18:52:48 +0800 > Jun Li wrote: > > > When every item of refcount block is NULL, free refcount block and rese= t > the > > corresponding item of refcount table with NULL. > > > > Signed-off-by: Jun Li > > --- > > > > The v2 do following change to modify some potential issue. > > > > +------- Here should start from "0". > > | > > for (k =3D 0; k < refcount_block_entries; k++) { > > if (refcount_block[k] !=3D cpu_to_be16(0)) { > > ... | | > > } | | > > } | +---- Using "0" is more > safe. > > | > > +-------- This should be "k" not "++k". > > --- > > block/qcow2-refcount.c | 31 +++++++++++++++++++++++++++++++ > > 1 file changed, 31 insertions(+) > > > > diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c > > index 43665b8..63f36e6 100644 > > --- a/block/qcow2-refcount.c > > +++ b/block/qcow2-refcount.c > > @@ -586,6 +586,37 @@ static int QEMU_WARN_UNUSED_RESULT > update_refcount(BlockDriverState *bs, > > if (refcount =3D=3D 0 && s->discard_passthrough[type]) { > > update_refcount_discard(bs, cluster_offset, > s->cluster_size); > > } > > + > > + /* When refcount block is NULL, update refcount table */ > > + if (block_index =3D=3D 0) { > > + int k =3D block_index; > > Hi, > > k =3D 0 is also done in the for block below... > > > + int refcount_block_entries =3D s->cluster_size / > sizeof(uint16_t); > > It's better for maintainance to count elements in an array this way: > > int refcount_block_entries =3D s->cluster_size / sizeof(refcount_block[0]= ); > > > + for (k =3D 0; k < refcount_block_entries; k++) { > > + if (refcount_block[k] !=3D cpu_to_be16(0)) { > > + break; > > + } > > + } > > + > > + if (k =3D=3D refcount_block_entries) { > > + qemu_vfree(refcount_block); > > + /* update refcount table */ > > + unsigned int refcount_table_index; > > + uint64_t data64 =3D cpu_to_be64(0); > > + refcount_table_index =3D cluster_index >> > (s->cluster_bits - > > + REFCOUNT_SHIFT); > > + ret =3D bdrv_pwrite_sync(bs->file, > > + s->refcount_table_offset + > > + refcount_table_index * > > + sizeof(uint64_t), > > + &data64, sizeof(data64)); > > + if (ret < 0) { > > + goto fail; > > + } > > + > > + s->refcount_table[refcount_table_index] =3D data64; > > + > > + } > > + } > > } > > > > ret =3D 0; > > Cheers. > > -- > Gregory Kurz kurzgreg@fr.ibm.com > gkurz@linux.vnet.ibm.com > Software Engineer @ IBM/Meiosys http://www.ibm.com > Tel +33 (0)562 165 496 > > "Anarchy is about taking complete responsibility for yourself." > Alan Moore. > > --001a11c2cbd86febbd05039d9a25 Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: base64 PHAgZGlyPSJsdHIiPlRoYW5rcy4gSSB3aWxsIGdpdmUgYSBuZXcgdmVyc2lvbiBpbiB2MyBvZiBx Y293MiBzaHJpbmsuPGJyPjwvcD4NCjxwIGRpcj0ibHRyIj5KdW4gTGk8L3A+DQo8ZGl2IGNsYXNz PSJnbWFpbF9xdW90ZSI+MjAxNC05LTMg5LiK5Y2IMToxMuS6jiAmcXVvdDtHcmVnIEt1cnomcXVv dDsgJmx0OzxhIGhyZWY9Im1haWx0bzpna3VyekBsaW51eC52bmV0LmlibS5jb20iPmdrdXJ6QGxp bnV4LnZuZXQuaWJtLmNvbTwvYT4mZ3Q75YaZ6YGT77yaPGJyIHR5cGU9ImF0dHJpYnV0aW9uIj48 YmxvY2txdW90ZSBjbGFzcz0iZ21haWxfcXVvdGUiIHN0eWxlPSJtYXJnaW46MCAwIDAgLjhleDti b3JkZXItbGVmdDoxcHggI2NjYyBzb2xpZDtwYWRkaW5nLWxlZnQ6MWV4Ij5PbiBNb24swqAgMSBT ZXAgMjAxNCAxODo1Mjo0OCArMDgwMDxicj4NCkp1biBMaSAmbHQ7PGEgaHJlZj0ibWFpbHRvOmp1 bm11emlAZ21haWwuY29tIj5qdW5tdXppQGdtYWlsLmNvbTwvYT4mZ3Q7IHdyb3RlOjxicj4NCjxi cj4NCiZndDsgV2hlbiBldmVyeSBpdGVtIG9mIHJlZmNvdW50IGJsb2NrIGlzIE5VTEwsIGZyZWUg cmVmY291bnQgYmxvY2sgYW5kIHJlc2V0IHRoZTxicj4NCiZndDsgY29ycmVzcG9uZGluZyBpdGVt IG9mIHJlZmNvdW50IHRhYmxlIHdpdGggTlVMTC48YnI+DQomZ3Q7PGJyPg0KJmd0OyBTaWduZWQt b2ZmLWJ5OiBKdW4gTGkgJmx0O2FkZHJlc3NAaGlkZGVuJmd0Ozxicj4NCiZndDsgLS0tPGJyPg0K Jmd0Ozxicj4NCiZndDsgVGhlIHYyIGRvIGZvbGxvd2luZyBjaGFuZ2UgdG8gbW9kaWZ5IHNvbWUg cG90ZW50aWFsIGlzc3VlLjxicj4NCiZndDs8YnI+DQomZ3Q7wqAgwqAgwqAgwqAgwqAgwqAgwqAg Ky0tLS0tLS0gSGVyZSBzaG91bGQgc3RhcnQgZnJvbSAmcXVvdDswJnF1b3Q7Ljxicj4NCiZndDvC oCDCoCDCoCDCoCDCoCDCoCDCoCB8PGJyPg0KJmd0O8KgIMKgIMKgZm9yIChrID0gMDsgayAmbHQ7 IHJlZmNvdW50X2Jsb2NrX2VudHJpZXM7IGsrKykgezxicj4NCiZndDvCoCDCoCDCoCDCoCDCoGlm IChyZWZjb3VudF9ibG9ja1trXSAhPSBjcHVfdG9fYmUxNigwKSkgezxicj4NCiZndDvCoCDCoCDC oCDCoCDCoC4uLsKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIHzCoCDCoCDCoCDCoCDCoCDCoCDCoCDC oCDCoHw8YnI+DQomZ3Q7wqAgwqAgwqAgwqAgwqB9wqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAg fMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgfDxicj4NCiZndDvCoCDCoCDCoH3CoCDCoCDCoCDC oCDCoCDCoCDCoCDCoCDCoCDCoCDCoCB8wqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqArLS0tLSBV c2luZyAmcXVvdDswJnF1b3Q7IGlzIG1vcmUgc2FmZS48YnI+DQomZ3Q7wqAgwqAgwqAgwqAgwqAg wqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgfDxicj4NCiZndDvCoCDCoCDCoCDCoCDCoCDCoCDC oCDCoCDCoCDCoCDCoCDCoCDCoCDCoCArLS0tLS0tLS0gVGhpcyBzaG91bGQgYmUgJnF1b3Q7ayZx dW90OyBub3QgJnF1b3Q7KytrJnF1b3Q7Ljxicj4NCiZndDsgLS0tPGJyPg0KJmd0O8KgIGJsb2Nr L3Fjb3cyLXJlZmNvdW50LmMgfCAzMSArKysrKysrKysrKysrKysrKysrKysrKysrKysrKysrPGJy Pg0KJmd0O8KgIDEgZmlsZSBjaGFuZ2VkLCAzMSBpbnNlcnRpb25zKCspPGJyPg0KJmd0Ozxicj4N CiZndDsgZGlmZiAtLWdpdCBhL2Jsb2NrL3Fjb3cyLXJlZmNvdW50LmMgYi9ibG9jay9xY293Mi1y ZWZjb3VudC5jPGJyPg0KJmd0OyBpbmRleCA0MzY2NWI4Li42M2YzNmU2IDEwMDY0NDxicj4NCiZn dDsgLS0tIGEvYmxvY2svcWNvdzItcmVmY291bnQuYzxicj4NCiZndDsgKysrIGIvYmxvY2svcWNv dzItcmVmY291bnQuYzxicj4NCiZndDsgQEAgLTU4Niw2ICs1ODYsMzcgQEAgc3RhdGljIGludCBR RU1VX1dBUk5fVU5VU0VEX1JFU1VMVCB1cGRhdGVfcmVmY291bnQoQmxvY2tEcml2ZXJTdGF0ZSAq YnMsPGJyPg0KJmd0O8KgIMKgIMKgIMKgIMKgIGlmIChyZWZjb3VudCA9PSAwICZhbXA7JmFtcDsg cy0mZ3Q7ZGlzY2FyZF9wYXNzdGhyb3VnaFt0eXBlXSkgezxicj4NCiZndDvCoCDCoCDCoCDCoCDC oCDCoCDCoCB1cGRhdGVfcmVmY291bnRfZGlzY2FyZChicywgY2x1c3Rlcl9vZmZzZXQsIHMtJmd0 O2NsdXN0ZXJfc2l6ZSk7PGJyPg0KJmd0O8KgIMKgIMKgIMKgIMKgIH08YnI+DQomZ3Q7ICs8YnI+ DQomZ3Q7ICvCoCDCoCDCoCDCoCAvKiBXaGVuIHJlZmNvdW50IGJsb2NrIGlzIE5VTEwsIHVwZGF0 ZSByZWZjb3VudCB0YWJsZSAqLzxicj4NCiZndDsgK8KgIMKgIMKgIMKgIGlmIChibG9ja19pbmRl eCA9PSAwKSB7PGJyPg0KJmd0OyArwqAgwqAgwqAgwqAgwqAgwqAgaW50IGsgPSBibG9ja19pbmRl eDs8YnI+DQo8YnI+DQpIaSw8YnI+DQo8YnI+DQprID0gMCBpcyBhbHNvIGRvbmUgaW4gdGhlIGZv ciBibG9jayBiZWxvdy4uLjxicj4NCjxicj4NCiZndDsgK8KgIMKgIMKgIMKgIMKgIMKgIGludCBy ZWZjb3VudF9ibG9ja19lbnRyaWVzID0gcy0mZ3Q7Y2x1c3Rlcl9zaXplIC8gc2l6ZW9mKHVpbnQx Nl90KTs8YnI+DQo8YnI+DQpJdCYjMzk7cyBiZXR0ZXIgZm9yIG1haW50YWluYW5jZSB0byBjb3Vu dCBlbGVtZW50cyBpbiBhbiBhcnJheSB0aGlzIHdheTo8YnI+DQo8YnI+DQppbnQgcmVmY291bnRf YmxvY2tfZW50cmllcyA9IHMtJmd0O2NsdXN0ZXJfc2l6ZSAvIHNpemVvZihyZWZjb3VudF9ibG9j a1swXSk7PGJyPg0KPGJyPg0KJmd0OyArwqAgwqAgwqAgwqAgwqAgwqAgZm9yIChrID0gMDsgayAm bHQ7IHJlZmNvdW50X2Jsb2NrX2VudHJpZXM7IGsrKykgezxicj4NCiZndDsgK8KgIMKgIMKgIMKg IMKgIMKgIMKgIMKgIGlmIChyZWZjb3VudF9ibG9ja1trXSAhPSBjcHVfdG9fYmUxNigwKSkgezxi cj4NCiZndDsgK8KgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIGJyZWFrOzxicj4NCiZndDsg K8KgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIH08YnI+DQomZ3Q7ICvCoCDCoCDCoCDCoCDCoCDCoCB9 PGJyPg0KJmd0OyArPGJyPg0KJmd0OyArwqAgwqAgwqAgwqAgwqAgwqAgaWYgKGsgPT0gcmVmY291 bnRfYmxvY2tfZW50cmllcykgezxicj4NCiZndDsgK8KgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIHFl bXVfdmZyZWUocmVmY291bnRfYmxvY2spOzxicj4NCiZndDsgK8KgIMKgIMKgIMKgIMKgIMKgIMKg IMKgIC8qIHVwZGF0ZSByZWZjb3VudCB0YWJsZSAqLzxicj4NCiZndDsgK8KgIMKgIMKgIMKgIMKg IMKgIMKgIMKgIHVuc2lnbmVkIGludCByZWZjb3VudF90YWJsZV9pbmRleDs8YnI+DQomZ3Q7ICvC oCDCoCDCoCDCoCDCoCDCoCDCoCDCoCB1aW50NjRfdCBkYXRhNjQgPSBjcHVfdG9fYmU2NCgwKTs8 YnI+DQomZ3Q7ICvCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCByZWZjb3VudF90YWJsZV9pbmRleCA9 IGNsdXN0ZXJfaW5kZXggJmd0OyZndDsgKHMtJmd0O2NsdXN0ZXJfYml0cyAtPGJyPg0KJmd0OyAr wqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAg wqBSRUZDT1VOVF9TSElGVCk7PGJyPg0KJmd0OyArwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgcmV0 ID0gYmRydl9wd3JpdGVfc3luYyhicy0mZ3Q7ZmlsZSw8YnI+DQomZ3Q7ICvCoCDCoCDCoCDCoCDC oCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoHMtJmd0O3JlZmNv dW50X3RhYmxlX29mZnNldCArPGJyPg0KJmd0OyArwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAg wqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqByZWZjb3VudF90YWJsZV9pbmRleCAqPGJy Pg0KJmd0OyArwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAg wqAgwqAgwqAgwqBzaXplb2YodWludDY0X3QpLDxicj4NCiZndDsgK8KgIMKgIMKgIMKgIMKgIMKg IMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgJmFtcDtkYXRhNjQsIHNp emVvZihkYXRhNjQpKTs8YnI+DQomZ3Q7ICvCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCBpZiAocmV0 ICZsdDsgMCkgezxicj4NCiZndDsgK8KgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIGdvdG8g ZmFpbDs8YnI+DQomZ3Q7ICvCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCB9PGJyPg0KJmd0OyArPGJy Pg0KJmd0OyArwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgcy0mZ3Q7cmVmY291bnRfdGFibGVbcmVm Y291bnRfdGFibGVfaW5kZXhdID0gZGF0YTY0Ozxicj4NCiZndDsgKzxicj4NCiZndDsgK8KgIMKg IMKgIMKgIMKgIMKgIH08YnI+DQomZ3Q7ICvCoCDCoCDCoCDCoCB9PGJyPg0KJmd0O8KgIMKgIMKg IH08YnI+DQomZ3Q7PGJyPg0KJmd0O8KgIMKgIMKgIHJldCA9IDA7PGJyPg0KPGJyPg0KQ2hlZXJz Ljxicj4NCjxicj4NCi0tPGJyPg0KR3JlZ29yeSBLdXJ6wqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAg wqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqA8YSBocmVmPSJtYWlsdG86a3VyemdyZWdA ZnIuaWJtLmNvbSI+a3VyemdyZWdAZnIuaWJtLmNvbTwvYT48YnI+DQrCoCDCoCDCoCDCoCDCoCDC oCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDC oDxhIGhyZWY9Im1haWx0bzpna3VyekBsaW51eC52bmV0LmlibS5jb20iPmdrdXJ6QGxpbnV4LnZu ZXQuaWJtLmNvbTwvYT48YnI+DQpTb2Z0d2FyZSBFbmdpbmVlciBAIElCTS9NZWlvc3lzwqAgwqAg wqAgwqAgwqAgwqAgwqAgwqAgwqAgPGEgaHJlZj0iaHR0cDovL3d3dy5pYm0uY29tIiB0YXJnZXQ9 Il9ibGFuayI+aHR0cDovL3d3dy5pYm0uY29tPC9hPjxicj4NClRlbCArMzMgKDApNTYyIDE2NSA0 OTY8YnI+DQo8YnI+DQomcXVvdDtBbmFyY2h5IGlzIGFib3V0IHRha2luZyBjb21wbGV0ZSByZXNw b25zaWJpbGl0eSBmb3IgeW91cnNlbGYuJnF1b3Q7PGJyPg0KwqAgwqAgwqAgwqAgQWxhbiBNb29y ZS48YnI+DQo8YnI+DQo8L2Jsb2NrcXVvdGU+PC9kaXY+DQo= --001a11c2cbd86febbd05039d9a25--